diff --git a/apps/desktop/src/store/session-pin-sync.test.ts b/apps/desktop/src/store/session-pin-sync.test.ts index 63878c1373cc..e9c7f76c1e8c 100644 --- a/apps/desktop/src/store/session-pin-sync.test.ts +++ b/apps/desktop/src/store/session-pin-sync.test.ts @@ -165,4 +165,32 @@ describe('watchSessionPins remote pull', () => { expect($pinnedSessionIds.get()).not.toContain('race') }) + + it('does not let a stale page undo a pin before the write fires', async () => { + // Regression: pullRemotePins ran before the push pass, so a stale page + // showing pinned=false immediately undid a just-made pin before writePin + // ever fired. The justChanged guard now skips ids in this window. + $sessions.set([row('stale', { pinned: false })]) + $pinnedSessionIds.set(['stale']) + await flush() + + expect(patch).toHaveBeenCalledWith('stale', true, undefined) + expect($pinnedSessionIds.get()).toContain('stale') + }) + + it('does not let a stale page re-pin a just-unpinned session', async () => { + // Regression: pullRemotePins ran before the push pass, so a stale page + // showing pinned=true immediately re-pinned a just-unpinned session + // before writePin(false) ever fired. + $sessions.set([row('stale-2', { pinned: true })]) + $pinnedSessionIds.set(['stale-2']) + await flush() + patch.mockClear() + + $pinnedSessionIds.set([]) + await flush() + + expect(patch).toHaveBeenCalledWith('stale-2', false, undefined) + expect($pinnedSessionIds.get()).not.toContain('stale-2') + }) }) diff --git a/apps/desktop/src/store/session-pin-sync.ts b/apps/desktop/src/store/session-pin-sync.ts index 5af6d3fdca5a..860f325baaa4 100644 --- a/apps/desktop/src/store/session-pin-sync.ts +++ b/apps/desktop/src/store/session-pin-sync.ts @@ -59,8 +59,14 @@ function writePin(id: string, pinned: boolean, profile?: null | string): Promise * 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. + * + * ``justChanged`` carries ids whose local state was just changed by the user + * (the symmetric difference between the pinned set and what we've successfully + * mirrored). Those ids must be skipped: the ``$sessions`` page is stale until + * the push pass writes the new value and the next refresh carries it, so + * adopting the server's old state would immediately undo the user's click. */ -function pullRemotePins(): void { +function pullRemotePins(justChanged: Set): void { const local = new Set($pinnedSessionIds.get()) for (const row of $sessions.get()) { @@ -72,6 +78,13 @@ function pullRemotePins(): void { // 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) + + // Skip ids whose local state was just changed by the user — the page is + // stale and the push pass will write the correct value. + if (justChanged.has(pinId) || justChanged.has(row.id)) { + continue + } + 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. @@ -99,8 +112,34 @@ function reconcile(): void { return } - pullRemotePins() + // Compute the set of ids whose local pin state was just changed by the user + // but hasn't been mirrored to the server yet. Pull must skip these: the + // $sessions page still carries the old server value and would immediately + // undo the user's click. This is the symmetric difference between what the + // local set says and what we've successfully mirrored. + const prePull = new Set($pinnedSessionIds.get()) + const justChanged = new Set() + + // Newly pinned (in local set, not yet mirrored) — the push pass will write + // pinned=true; pull must not see pinned=false and undo it. + for (const id of prePull) { + if (!mirrored.has(id) && !pending.has(id)) { + justChanged.add(id) + } + } + + // Newly unpinned (was mirrored/pending, no longer in local set) — the push + // pass will write pinned=false; pull must not see pinned=true and re-pin it. + for (const id of [...mirrored, ...pending]) { + if (!prePull.has(id)) { + justChanged.add(id) + } + } + + pullRemotePins(justChanged) + // Re-read after pull — pullRemotePins may have adopted/dropped pins via + // pinSession/unpinSession, and the push pass must see the updated set. const current = new Set($pinnedSessionIds.get()) // Unpinned: anything we were tracking that's no longer in the set.