diff --git a/apps/desktop/src/app/chat/sidebar/session-index.test.ts b/apps/desktop/src/app/chat/sidebar/session-index.test.ts index b714a0fa12..5a0d6af709 100644 --- a/apps/desktop/src/app/chat/sidebar/session-index.test.ts +++ b/apps/desktop/src/app/chat/sidebar/session-index.test.ts @@ -119,6 +119,27 @@ describe('resolvePinnedSessions', () => { expect(resolvePinnedSessions([], index, sessions, new Set(['other'])).map(s => s.id)).toEqual(['foreign']) }) + it('does not reshuffle Show-all pins that already live in the local set', () => { + // Foreign-profile rows used to miss the per-profile local copy and fall + // through the recency-ordered fallback, so a click (last_active bump) + // reshuffled the whole Pinned section. A connection-wide local set keeps + // the hand-picked order even when recency changes. + const sessions = [ + row('foreign', { last_active: 1, pinned: true, profile: 'k9' }), + row('local', { last_active: 50, pinned: true, profile: 'default' }) + ] + const index = buildSessionByAnyId(sessions, [], []) + + expect(resolvePinnedSessions(['foreign', 'local'], index, sessions).map(s => s.id)).toEqual(['foreign', 'local']) + + const clicked = [ + row('foreign', { last_active: 99, pinned: true, profile: 'k9' }), + row('local', { last_active: 50, pinned: true, profile: 'default' }) + ] + + expect(resolvePinnedSessions(['foreign', 'local'], index, clicked).map(s => s.id)).toEqual(['foreign', 'local']) + }) + it('ignores rows from a backend that predates the pinned flag', () => { // `pinned` undefined means "no opinion", never "pinned". const sessions = [row('a')] diff --git a/apps/desktop/src/lib/connection-scoped.test.ts b/apps/desktop/src/lib/connection-scoped.test.ts new file mode 100644 index 0000000000..8a020bc963 --- /dev/null +++ b/apps/desktop/src/lib/connection-scoped.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' + +import { connectionScopeSuffix } from './connection-scoped' + +const remote = (profile: string, baseUrl = 'https://gw.example:8443') => ({ + baseUrl, + mode: 'remote' as const, + profile +}) + +describe('connectionScopeSuffix', () => { + it('is empty for a local connection', () => { + expect(connectionScopeSuffix({ baseUrl: 'http://127.0.0.1:8000', mode: 'local', profile: 'default' })).toBe('') + }) + + it('includes the profile by default so profile-local lists stay apart', () => { + expect(connectionScopeSuffix(remote('default'))).toBe( + `.remote.${encodeURIComponent('https://gw.example:8443')}.default` + ) + expect(connectionScopeSuffix(remote('k9'))).not.toBe(connectionScopeSuffix(remote('default'))) + }) + + it('is stable across profile switch when includeProfile is false', () => { + const a = connectionScopeSuffix(remote('default'), false) + const b = connectionScopeSuffix(remote('k9'), false) + + expect(a).toBe(`.remote.${encodeURIComponent('https://gw.example:8443')}`) + expect(a).toBe(b) + }) + + it('still isolates different gateways when includeProfile is false', () => { + expect(connectionScopeSuffix(remote('default', 'https://gw-a.example'), false)).not.toBe( + connectionScopeSuffix(remote('default', 'https://gw-b.example'), false) + ) + }) +}) diff --git a/apps/desktop/src/lib/connection-scoped.ts b/apps/desktop/src/lib/connection-scoped.ts index 4831b9f90d..abca2bb827 100644 --- a/apps/desktop/src/lib/connection-scoped.ts +++ b/apps/desktop/src/lib/connection-scoped.ts @@ -17,8 +17,10 @@ import { readKey, writeKey } from './storage' // but its storage key follows the active connection. The local connection // keeps the BARE key — byte-identical behavior for single-backend users, the // same contract as `backendScopeKey` in @hermes/shared — while a remote -// connection gets `.remote..`, the -// shape `workspaceCwdKey` already established. +// connection gets `.remote..` by +// default (the shape `workspaceCwdKey` already established). Gateway-wide +// mirrors (pins) pass `{ includeProfile: false }` so a profile switch cannot +// fragment the cache and re-assert a stale copy. // // Legacy globally-keyed values are deliberately NOT migrated into remote // scopes: those keys accumulated writes from every window, so ownership of @@ -34,14 +36,32 @@ export interface ConnectionScopeDescriptor { profile?: null | string } +export interface ConnectionScopeOptions { + /** + * When false, a remote suffix is `.remote.` only. Use this for + * gateway-wide mirrors (pins) so a profile switch cannot reload a stale + * per-profile copy. Defaults to true — session order and other + * profile-local lists stay isolated. + */ + includeProfile?: boolean +} + /** The storage-key suffix for a connection. Local (and unknown) connections * map to the bare key; remote connections get their own namespace. */ -export function connectionScopeSuffix(connection: ConnectionScopeDescriptor | null | undefined): string { +export function connectionScopeSuffix( + connection: ConnectionScopeDescriptor | null | undefined, + includeProfile = true +): string { if (connection?.mode !== 'remote') { return '' } const base = encodeURIComponent(connection.baseUrl || 'remote') + + if (!includeProfile) { + return `.remote.${base}` + } + const profile = encodeURIComponent(connection.profile || 'default') return `.remote.${base}.${profile}` @@ -51,24 +71,34 @@ interface ScopedEntry { $value: WritableAtom codec: Codec fallback: T + includeProfile: boolean key: string + /** Last suffix this entry loaded or persisted under. */ + suffix: string /** True while a rescope is applying a loaded value — the persistence * subscriber must not echo that read back into storage. */ applying: boolean } +let activeConnection: ConnectionScopeDescriptor | null | undefined let activeSuffix = '' +let activeGatewaySuffix = '' const registry: ScopedEntry[] = [] const scopeListeners = new Set<() => void>() +function suffixFor(entry: Pick, 'includeProfile'>): string { + return connectionScopeSuffix(activeConnection, entry.includeProfile) +} + /** The suffix for the connection the window is currently on. */ export function activeConnectionScopeSuffix(): string { return activeSuffix } -/** Observe scope changes (fires BEFORE the scoped atoms repaint, so - * per-connection bookkeeping can reset ahead of the reload). */ +/** Observe gateway-identity changes (fires BEFORE scoped atoms that + * follow the connection reload, so pin-sync bookkeeping can reset). + * Profile-only switches do not fire: pin state is gateway-wide. */ export function onConnectionScopeChange(listener: () => void): () => void { scopeListeners.add(listener) @@ -76,7 +106,7 @@ export function onConnectionScopeChange(listener: () => void): () => void { } function loadEntry(entry: ScopedEntry): T { - const raw = readKey(entry.key + activeSuffix) + const raw = readKey(entry.key + suffixFor(entry)) if (raw === null) { return entry.fallback @@ -94,8 +124,22 @@ function loadEntry(entry: ScopedEntry): T { * Reads seed from the current scope's key; writes land under it. When the * window's connection changes, every scoped atom reloads from the new scope. */ -export function connectionScopedAtom(key: string, fallback: T, codec: Codec = Codecs.json()): WritableAtom { - const entry: ScopedEntry = { $value: atom(fallback), applying: false, codec, fallback, key } +export function connectionScopedAtom( + key: string, + fallback: T, + codec: Codec = Codecs.json(), + options?: ConnectionScopeOptions +): WritableAtom { + const includeProfile = options?.includeProfile !== false + const entry: ScopedEntry = { + $value: atom(fallback), + applying: false, + codec, + fallback, + includeProfile, + key, + suffix: connectionScopeSuffix(activeConnection, includeProfile) + } entry.$value.set(loadEntry(entry)) registry.push(entry) @@ -115,7 +159,7 @@ export function connectionScopedAtom(key: string, fallback: T, codec: Codec { // The bare key still belongs to the local connection. expect(readKey('hermes.desktop.pinnedSessions')).toBe(JSON.stringify(['local-1'])) - // The remote pin landed under its own scope, not the shared key. - const scoped = readKey( - `hermes.desktop.pinnedSessions.remote.${encodeURIComponent('https://vps-a.example:8443')}.default` - ) + // The remote pin landed under its own gateway scope, not the shared key + // and not a per-profile fragment. + const scoped = readKey(`hermes.desktop.pinnedSessions.remote.${encodeURIComponent('https://vps-a.example:8443')}`) expect(scoped).toBe(JSON.stringify(['a-1'])) }) @@ -117,6 +116,19 @@ describe('connection-scoped sidebar lists (#77318)', () => { expect($pinnedSessionIds.get()).toEqual(['a-1']) }) + it('keeps pins across a profile switch on the same gateway, but not session order', () => { + setConnection(remoteA) + pinSession('a-1') + $sidebarSessionOrderIds.set(['s1']) + $sidebarSessionOrderManual.set(true) + + setConnection({ ...remoteA, profile: 'k9' } as unknown as HermesConnection) + + expect($pinnedSessionIds.get()).toEqual(['a-1']) + expect($sidebarSessionOrderIds.get()).toEqual([]) + expect($sidebarSessionOrderManual.get()).toBe(false) + }) + it('scopes the manual session order and its flag per connection', () => { $sidebarSessionOrderIds.set(['s1', 's2']) $sidebarSessionOrderManual.set(true) diff --git a/apps/desktop/src/store/layout.ts b/apps/desktop/src/store/layout.ts index 5369912c33..1887396ab2 100644 --- a/apps/desktop/src/store/layout.ts +++ b/apps/desktop/src/store/layout.ts @@ -97,7 +97,13 @@ export const $sidebarWidth: ReadableAtom = computed($paneStates, states // one localStorage area. A global key here is how one gateway's pins bleed // into another window's sidebar (#77318). The local connection keeps the // bare legacy key; remote connections get their own namespaced keys. -export const $pinnedSessionIds = connectionScopedAtom(SIDEBAR_PINNED_STORAGE_KEY, [] as string[], Codecs.stringArray) +// +// Pins omit the profile from that key: `sessions.pinned` is gateway-wide, +// and a per-profile localStorage copy is how an unpin in profile A comes +// back when the window rescopes to B (stale ids flush as pinned=true). +export const $pinnedSessionIds = connectionScopedAtom(SIDEBAR_PINNED_STORAGE_KEY, [] as string[], Codecs.stringArray, { + includeProfile: false +}) export const $sidebarSessionOrderIds = connectionScopedAtom( SIDEBAR_SESSION_ORDER_STORAGE_KEY, [] as string[], diff --git a/apps/desktop/src/store/session-pin-connection-scope.test.ts b/apps/desktop/src/store/session-pin-connection-scope.test.ts new file mode 100644 index 0000000000..c9ef71fdf3 --- /dev/null +++ b/apps/desktop/src/store/session-pin-connection-scope.test.ts @@ -0,0 +1,110 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { HermesConnection } from '@/global' +import { connectionScopeSuffix } from '@/lib/connection-scoped' +import { readKey } from '@/lib/storage' +import type { SessionInfo } from '@/types/hermes' + +const patch = vi.fn<(id: string, pinned: boolean, profile?: null | string) => Promise<{ ok: boolean }>>(() => + Promise.resolve({ ok: true }) +) + +vi.mock('@/hermes', () => ({ + setApiRequestProfile: () => {}, + setSessionPinnedRemote: (id: string, pinned: boolean, profile?: null | string) => patch(id, pinned, profile) +})) + +import { $pinnedSessionIds, pinSession, unpinSession } from '@/store/layout' +import { $sessions, setConnection } from '@/store/session' + +import { resetSessionPinMirror, watchSessionPins } from './session-pin-sync' + +const PIN_KEY = 'hermes.desktop.pinnedSessions' + +const remote = (profile: string, baseUrl = 'https://gw.example:8443'): HermesConnection => + ({ + baseUrl, + mode: 'remote', + profile, + token: 't', + wsUrl: 'ws://x' + }) as unknown as HermesConnection + +const row = (id: string, extra: Partial = {}): SessionInfo => + ({ id, message_count: 1, source: 'cli', started_at: 0, title: id, ...extra }) as SessionInfo + +const flush = () => Promise.resolve() + +beforeAll(() => { + ;(globalThis as { window?: unknown }).window ??= {} + ;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = {} + watchSessionPins() +}) + +beforeEach(() => { + window.localStorage.clear() + setConnection(remote('default')) + $sessions.set([]) + $pinnedSessionIds.set([]) + resetSessionPinMirror() + patch.mockClear() +}) + +afterEach(() => { + $sessions.set([]) + $pinnedSessionIds.set([]) + resetSessionPinMirror() +}) + +describe('desktop pin list is connection-scoped, not profile-scoped', () => { + it('keeps the pin storage key stable across a profile switch', () => { + setConnection(remote('default')) + pinSession('s1') + + const gatewayKey = `${PIN_KEY}${connectionScopeSuffix(remote('default'), false)}` + + expect(readKey(gatewayKey)).toBe(JSON.stringify(['s1'])) + + setConnection(remote('k9')) + expect($pinnedSessionIds.get()).toEqual(['s1']) + expect(readKey(gatewayKey)).toBe(JSON.stringify(['s1'])) + expect(readKey(`${PIN_KEY}${connectionScopeSuffix(remote('k9'))}`)).toBeNull() + }) + + it('still isolates pin sets between two different remote gateways', () => { + setConnection(remote('default', 'https://gw-a.example')) + pinSession('a-1') + + setConnection(remote('default', 'https://gw-b.example')) + expect($pinnedSessionIds.get()).toEqual([]) + pinSession('b-1') + + setConnection(remote('k9', 'https://gw-a.example')) + expect($pinnedSessionIds.get()).toEqual(['a-1']) + + setConnection(remote('default', 'https://gw-b.example')) + expect($pinnedSessionIds.get()).toEqual(['b-1']) + }) + + it('lets an unpin survive a profile rescope instead of flushing pin=true', async () => { + $sessions.set([row('s1', { pinned: false, profile: 'k9' })]) + + setConnection(remote('default')) + pinSession('s1') + await flush() + + setConnection(remote('k9')) + expect($pinnedSessionIds.get()).toEqual(['s1']) + + unpinSession('s1') + await flush() + patch.mockClear() + + setConnection(remote('default')) + await flush() + + expect($pinnedSessionIds.get()).not.toContain('s1') + expect(patch).not.toHaveBeenCalledWith('s1', true, expect.anything()) + expect(patch).not.toHaveBeenCalledWith('s1', true, 'k9') + }) +})