diff --git a/apps/desktop/src/store/session-pin-sync.test.ts b/apps/desktop/src/store/session-pin-sync.test.ts index e6613d8658c..63878c1373c 100644 --- a/apps/desktop/src/store/session-pin-sync.test.ts +++ b/apps/desktop/src/store/session-pin-sync.test.ts @@ -93,3 +93,76 @@ describe('watchSessionPins', () => { expect(patch).not.toHaveBeenCalled() }) }) + +describe('watchSessionPins remote pull', () => { + it('adopts a pin another app made', async () => { + $sessions.set([row('remote', { pinned: true })]) + await flush() + + expect($pinnedSessionIds.get()).toContain('remote') + }) + + it('adopts a remote pin on the durable lineage root, not the live tip', async () => { + $sessions.set([row('tip', { _lineage_root_id: 'root', pinned: true })]) + await flush() + + expect($pinnedSessionIds.get()).toEqual(['root']) + }) + + it('does not echo an adopted pin back as a redundant write', async () => { + $sessions.set([row('adopted', { pinned: true })]) + await flush() + + expect(patch).not.toHaveBeenCalled() + }) + + it('drops a local pin the server reports as unpinned', async () => { + $pinnedSessionIds.set(['gone']) + $sessions.set([row('gone', { pinned: true })]) + await flush() + patch.mockClear() + + // Another app unpinned it; our next refresh carries the new truth. + $sessions.set([row('gone', { pinned: false })]) + await flush() + + expect($pinnedSessionIds.get()).not.toContain('gone') + }) + + it('leaves the local set alone when the backend omits the flag', async () => { + $pinnedSessionIds.set(['legacy']) + // No `pinned` key at all — a runtime predating the column. + $sessions.set([row('legacy')]) + await flush() + + expect($pinnedSessionIds.get()).toContain('legacy') + }) + + it('ignores a stale page that contradicts a write still in flight', async () => { + let settle: (v: { ok: boolean }) => void = () => {} + + patch.mockImplementationOnce(() => new Promise(resolve => (settle = resolve))) + + $sessions.set([row('race')]) + $pinnedSessionIds.set(['race']) + await flush() + expect(patch).toHaveBeenCalledWith('race', true, undefined) + + // A list request issued before the PATCH lands still says pinned=false. + // Honouring it would silently undo the pin the user just made. + $sessions.set([row('race', { pinned: false })]) + await flush() + + expect($pinnedSessionIds.get()).toContain('race') + + // Once the write is acked, later server truth is honoured again. + settle({ ok: true }) + await flush() + await flush() + + $sessions.set([row('race', { pinned: false }), row('other')]) + await flush() + + expect($pinnedSessionIds.get()).not.toContain('race') + }) +}) diff --git a/apps/desktop/src/store/session-pin-sync.ts b/apps/desktop/src/store/session-pin-sync.ts index 35235e6dc64..5af6d3fdca5 100644 --- a/apps/desktop/src/store/session-pin-sync.ts +++ b/apps/desktop/src/store/session-pin-sync.ts @@ -1,35 +1,106 @@ /** - * Mirror the sidebar's localStorage pins into the backend "keep" flag. + * Reconcile the sidebar's pins with the backend "keep" flag, both directions. * - * Pins live in `$pinnedSessionIds` (localStorage) and drive the sidebar UI. - * The `sessions.auto_archive` sweep, however, runs backend-side and is blind to - * localStorage — so without this bridge it could hide a pinned chat. This - * watcher PATCHes `pinned` on the session REST endpoint whenever the pinned set - * changes, and re-asserts the whole current set at boot, which transparently - * migrates pre-existing pins (no flag, no user action — the sweep just starts - * honouring them). It never touches the sidebar's own display; localStorage - * stays the source of truth there. + * Pins drive the sidebar UI out of `$pinnedSessionIds` (localStorage), but the + * durable record is `sessions.pinned` in each profile's state.db. Two things + * depend on the backend copy: the `sessions.auto_archive` sweep runs + * server-side and would otherwise hide a pinned chat, and a second Desktop app + * pointed at the same gateway has its own, separate localStorage. + * + * Push: PATCH `pinned` whenever the local set changes, and re-assert the whole + * set at boot — which transparently migrates pre-existing pins with no user + * action. + * + * Pull: session rows now carry `pinned`, and the list endpoints back-fill + * pinned conversations past their LIMIT, so a row's absence from a page no + * longer says anything about its pin state. That makes the server row + * authoritative: adopt pins this app hasn't seen, and drop local pins the + * server says are gone. Only rows actually present in the payload are + * consulted, so a backend predating the flag (`pinned === undefined`) leaves + * the local set untouched. */ import { setSessionPinnedRemote } from '@/hermes' -import { $pinnedSessionIds } from '@/store/layout' -import { $sessions, sessionMatchesStoredId } from '@/store/session' +import { $pinnedSessionIds, pinSession, unpinSession } from '@/store/layout' +import { $sessions, sessionMatchesStoredId, sessionPinId } from '@/store/session' // pin ids we've successfully PATCHed pinned=true this session. const mirrored = new Set() // pin ids awaiting their row so we can resolve the owning profile before PATCH. const pending = new Set() +// Writes we've issued but not yet had acked, id -> value written. A list page +// already in flight when we PATCH still carries the old value, so it must not +// be read as the server disagreeing with us. Cleared when the write settles — +// the request's own lifetime is the guard, so nothing can leave one open. +const unconfirmed = new Map() function profileFor(pinId: string): null | string | undefined { return $sessions.get().find(row => sessionMatchesStoredId(row, pinId))?.profile } +/** PATCH the flag, guarding reads against pages that predate the write. */ +function writePin(id: string, pinned: boolean, profile?: null | string): Promise { + unconfirmed.set(id, pinned) + + return setSessionPinnedRemote(id, pinned, profile).then( + () => { + unconfirmed.delete(id) + }, + (err: unknown) => { + unconfirmed.delete(id) + throw err + } + ) +} + +/** + * Adopt the server's pin state for every row in the current page. + * + * Runs before the push pass so a remote pin is already in the local set by the + * time we reconcile — it gets marked as mirrored rather than echoed straight + * back as a redundant PATCH. + */ +function pullRemotePins(): void { + const local = new Set($pinnedSessionIds.get()) + + for (const row of $sessions.get()) { + // A backend without the flag has no opinion; never act on `undefined`. + if (typeof row.pinned !== 'boolean') { + continue + } + + // Pins are keyed on the durable lineage root so they survive compression + // tip rotation; the row may surface under either identity. + const pinId = sessionPinId(row) + const heldLocally = local.has(pinId) || local.has(row.id) + + // A write of ours the page hasn't caught up to yet is newer than the page. + const awaited = unconfirmed.has(pinId) ? unconfirmed.get(pinId) : unconfirmed.get(row.id) + + if (awaited !== undefined && awaited !== row.pinned) { + continue + } + + if (row.pinned && !heldLocally) { + pinSession(pinId) + // Already true server-side; record it so the push pass doesn't re-PATCH. + mirrored.add(pinId) + } else if (!row.pinned && heldLocally) { + unpinSession(local.has(pinId) ? pinId : row.id) + mirrored.delete(pinId) + mirrored.delete(row.id) + } + } +} + function reconcile(): void { // Config/session REST is only reachable through the Electron bridge. if (!window.hermesDesktop) { return } + pullRemotePins() + const current = new Set($pinnedSessionIds.get()) // Unpinned: anything we were tracking that's no longer in the set. @@ -37,7 +108,7 @@ function reconcile(): void { if (!current.has(id)) { mirrored.delete(id) pending.delete(id) - void setSessionPinnedRemote(id, false, profileFor(id)).catch(() => {}) + void writePin(id, false, profileFor(id)).catch(() => {}) } } @@ -59,7 +130,7 @@ function reconcile(): void { pending.delete(id) mirrored.add(id) - void setSessionPinnedRemote(id, true, row.profile).catch(() => { + void writePin(id, true, row.profile).catch(() => { // Let a later reconcile retry the mirror. mirrored.delete(id) pending.add(id) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 65979f5e20e..bfc75cf6b3f 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -472,6 +472,13 @@ export interface SessionInfo { output_tokens: number /** Parent conversation when this row is a /branch fork. */ parent_session_id?: null | string + /** Durable server-side pin flag (`sessions.pinned`). The list endpoints + * back-fill pinned conversations past their LIMIT, so a pinned row is + * always present in a page — which makes this authoritative for the + * sidebar's Pinned section and lets a second app adopt pins made + * elsewhere. Undefined against a backend predating the flag; treat that as + * "no opinion" and leave the local pin set alone. */ + pinned?: boolean preview: null | string source: null | string started_at: number