fix(desktop): keep pin list identity gateway-wide

Pin localStorage was keyed per connection and profile, so an unpin
reloaded a stale copy on switch and re-asserted pinned=true.
Scope pins by connection only so they survive rescope and stay isolated per gateway.
This commit is contained in:
Jeremy McKeehen
2026-08-19 10:57:55 -07:00
committed by Teknium
parent e0210ab6c9
commit ff57f173d8
6 changed files with 262 additions and 19 deletions
@@ -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')]
@@ -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)
)
})
})
+72 -14
View File
@@ -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 `<key>.remote.<encoded baseUrl>.<encoded profile>`, the
// shape `workspaceCwdKey` already established.
// connection gets `<key>.remote.<encoded baseUrl>.<encoded profile>` 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.<baseUrl>` 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<T> {
$value: WritableAtom<T>
codec: Codec<T>
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<any>[] = []
const scopeListeners = new Set<() => void>()
function suffixFor(entry: Pick<ScopedEntry<unknown>, '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<T>(entry: ScopedEntry<T>): 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<T>(entry: ScopedEntry<T>): 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<T>(key: string, fallback: T, codec: Codec<T> = Codecs.json<T>()): WritableAtom<T> {
const entry: ScopedEntry<T> = { $value: atom<T>(fallback), applying: false, codec, fallback, key }
export function connectionScopedAtom<T>(
key: string,
fallback: T,
codec: Codec<T> = Codecs.json<T>(),
options?: ConnectionScopeOptions
): WritableAtom<T> {
const includeProfile = options?.includeProfile !== false
const entry: ScopedEntry<T> = {
$value: atom<T>(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<T>(key: string, fallback: T, codec: Codec<T
return
}
writeKey(entry.key + activeSuffix, entry.codec.encode(value))
writeKey(entry.key + suffixFor(entry), entry.codec.encode(value))
})
return entry.$value
@@ -135,21 +179,35 @@ export function rescopeConnectionScopedStores(connection: ConnectionScopeDescrip
}
const next = connectionScopeSuffix(connection)
const nextGateway = connectionScopeSuffix(connection, false)
if (next === activeSuffix) {
return
}
activeConnection = connection
activeSuffix = next
// Bookkeeping listeners first: pin-sync's mirrored/pending sets describe
// the PREVIOUS backend and must be gone before the reload below triggers
// their reconcile against the new scope's lists.
for (const listener of scopeListeners) {
listener()
// Pin-sync's mirrored/pending sets describe the PREVIOUS gateway. Fire
// only when the connection (not the profile) changes — pins are
// gateway-wide, so a profile switch must not reset bookkeeping and then
// re-PATCH a leftover per-profile copy.
if (nextGateway !== activeGatewaySuffix) {
activeGatewaySuffix = nextGateway
for (const listener of scopeListeners) {
listener()
}
}
for (const entry of registry) {
const entrySuffix = suffixFor(entry)
if (entrySuffix === entry.suffix) {
continue
}
entry.suffix = entrySuffix
entry.applying = true
try {
@@ -87,10 +87,9 @@ describe('connection-scoped sidebar lists (#77318)', () => {
// 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)
+7 -1
View File
@@ -97,7 +97,13 @@ export const $sidebarWidth: ReadableAtom<number> = 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[],
@@ -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> = {}): 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')
})
})