fix(desktop): hold the pin write guard until a page confirms it

The guard that stops a stale list page from reverting a fresh pin was
released on the PATCH's own ack. A list request issued just before the
write is slower than the write, so it lands after the ack still carrying
the old value, with no guard left to fence it: the pin flips back and the
next reconcile pushes that wrong value to the server, making it durable.

Keep the guard until a page actually confirms the value written, with a
cooldown so a row that never returns can't fence itself forever, and drop
it outright when the write fails — the server never changed, so it stays
authoritative.

Also reset the mirror bookkeeping on a gateway switch. mirrored/pending
are per-backend facts; carrying them across a re-home told us the pins
were already pushed to a backend that has never seen them.

apps/desktop/src/store/gateway-switch.ts

@@ -20,6 +20,7 @@ import {
   setSessions,
   setSessionsLoading
 } from '@/store/session'
+import { resetSessionPinMirror } from '@/store/session-pin-sync'
 import { clearAllSessionStates } from '@/store/session-states'
 
 // True while a soft gateway-mode apply is mid-flight (wipe → re-dial). Lets the
@@ -43,6 +44,10 @@ export function wipeSessionListsForGatewaySwitch(): void {
   // The next backend is a different runtime — don't carry the old one's
   // "batched sidebar endpoint missing" capability verdict across the switch.
   resetSidebarBatchCapability()
+  // Pins are mirrored per-backend. The next gateway has its own state.db and
+  // has never seen them, so drop the "already pushed" bookkeeping and let the
+  // next reconcile re-assert the whole set against the new backend.
+  resetSessionPinMirror()
   setSessions([])
   setSessionProfilesTruncated({})
   setCronSessions([])

apps/desktop/src/store/session-pin-sync.test.ts

@@ -13,7 +13,7 @@ vi.mock('@/hermes', () => ({
 import { $pinnedSessionIds } from '@/store/layout'
 import { $sessions } from '@/store/session'
 
-import { watchSessionPins } from './session-pin-sync'
+import { resetSessionPinMirror, watchSessionPins } from './session-pin-sync'
 
 const row = (id: string, extra: Partial<SessionInfo> = {}): SessionInfo =>
   ({ id, message_count: 1, source: 'cli', started_at: 0, title: id, ...extra }) as SessionInfo
@@ -30,6 +30,10 @@ beforeAll(() => {
 beforeEach(() => {
   $sessions.set([])
   $pinnedSessionIds.set([])
+  // The mirror/pending/unconfirmed maps are module-global, so one test's
+  // bookkeeping would otherwise suppress the next test's PATCH (or fence out
+  // its page). Same reset the gateway switch uses.
+  resetSessionPinMirror()
   patch.mockClear()
 })
 
@@ -198,14 +202,89 @@ describe('watchSessionPins remote pull', () => {
 
     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')])
+    expect($pinnedSessionIds.get()).toContain('race')
+  })
+
+  it('still ignores a pre-write page that lands AFTER the ack (#76919)', async () => {
+    // The ack is not proof: a list request issued before the PATCH is slower
+    // than the PATCH itself, so it can arrive afterwards still carrying the
+    // old value. Reverting on it un-pins the session AND pushes the wrong
+    // value back to the server, making the mistake durable.
+    $sessions.set([row('acked')])
+    $pinnedSessionIds.set(['acked'])
+    await flush()
+    await flush()
+    expect(patch).toHaveBeenCalledWith('acked', true, undefined)
+    patch.mockClear()
+
+    // Post-ack, but this page predates the write.
+    $sessions.set([row('acked', { pinned: false })])
     await flush()
 
-    expect($pinnedSessionIds.get()).not.toContain('race')
+    expect($pinnedSessionIds.get()).toContain('acked')
+    expect(patch).not.toHaveBeenCalled()
+  })
+
+  it('releases the guard once a page confirms the written value', async () => {
+    $sessions.set([row('confirmed')])
+    $pinnedSessionIds.set(['confirmed'])
+    await flush()
+    await flush()
+
+    // The server catches up and echoes our value back.
+    $sessions.set([row('confirmed', { pinned: true })])
+    await flush()
+    patch.mockClear()
+
+    // With the write confirmed, a genuine remote unpin is authoritative again.
+    $sessions.set([row('confirmed', { pinned: false })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).not.toContain('confirmed')
+  })
+
+  it('stops fencing once the guard cooldown expires', async () => {
+    vi.useFakeTimers()
+
+    try {
+      $sessions.set([row('stale')])
+      $pinnedSessionIds.set(['stale'])
+      await flush()
+      await flush()
+
+      // No page ever confirms the write. The guard must not fence forever —
+      // after the cooldown the server's answer wins again.
+      vi.advanceTimersByTime(11_000)
+
+      $sessions.set([row('stale', { pinned: false })])
+      await flush()
+
+      expect($pinnedSessionIds.get()).not.toContain('stale')
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('keeps the pin and retries when the write itself fails', async () => {
+    patch.mockImplementationOnce(() => Promise.reject(new Error('offline')))
+
+    $sessions.set([row('failed')])
+    $pinnedSessionIds.set(['failed'])
+    await flush()
+    await flush()
+    patch.mockClear()
+
+    // The PATCH never landed, so the server legitimately still says unpinned —
+    // but that's OUR undelivered intent, not a remote decision. The pin stays
+    // and the next reconcile retries it rather than silently dropping it.
+    $sessions.set([row('failed', { pinned: false })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).toContain('failed')
+    expect(patch).toHaveBeenCalledWith('failed', true, undefined)
   })
 })

apps/desktop/src/store/session-pin-sync.ts

@@ -17,7 +17,8 @@
  * 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.
+ * the local set untouched — and a page that predates one of our own writes is
+ * fenced out until a later page confirms the value we wrote.
  */
 
 import { setSessionPinnedRemote } from '@/hermes'
@@ -28,11 +29,17 @@ import { $sessions, sessionMatchesStoredId, sessionPinId } from '@/store/session
 const mirrored = new Set<string>()
 // pin ids awaiting their row so we can resolve the owning profile before PATCH.
 const pending = new Set<string>()
-// 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<string, boolean>()
+// Writes we've issued, id -> the value we wrote and when. A list page already
+// in flight when we PATCH still carries the OLD value, and it can land after
+// our ack — so the ack is not proof the page we're reading is newer than the
+// write. Hold the guard until a page actually CONFIRMS the written value,
+// with a cooldown so a row that never comes back can't fence itself forever.
+const unconfirmed = new Map<string, { at: number; value: boolean }>()
+
+// How long an unconfirmed write outranks a page that contradicts it. Long
+// enough to cover a list request issued just before the PATCH (those are the
+// slow ones), short enough that a genuine server-side change still wins.
+const WRITE_GUARD_MS = 10_000
 
 function profileFor(pinId: string): null | string | undefined {
   return $sessions.get().find(row => sessionMatchesStoredId(row, pinId))?.profile
@@ -40,13 +47,18 @@ function profileFor(pinId: string): null | string | undefined {
 
 /** PATCH the flag, guarding reads against pages that predate the write. */
 function writePin(id: string, pinned: boolean, profile?: null | string): Promise<void> {
-  unconfirmed.set(id, pinned)
+  unconfirmed.set(id, { at: Date.now(), value: pinned })
 
   return setSessionPinnedRemote(id, pinned, profile).then(
     () => {
-      unconfirmed.delete(id)
+      // Deliberately NOT cleared here: a list request issued before this PATCH
+      // can still land after the ack carrying the pre-write value. The guard
+      // is released by pullRemotePins when a page confirms the written value,
+      // or by the cooldown if none ever does.
     },
     (err: unknown) => {
+      // A failed write leaves the server on the old value, so the guard would
+      // be fencing out the truth. Drop it and let the page win.
       unconfirmed.delete(id)
       throw err
     }
@@ -76,11 +88,22 @@ function pullRemotePins(): void {
     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)
+    // A write of ours this page may predate. Confirmed (page agrees) → release
+    // the guard, the server has caught up. Contradicted but still inside the
+    // cooldown → the page was almost certainly issued before our PATCH, so our
+    // write is newer: skip the row. Contradicted past the cooldown → no page
+    // ever confirmed us, so stop fencing and let the server win.
+    const guardKey = unconfirmed.has(pinId) ? pinId : unconfirmed.has(row.id) ? row.id : null
+    const guard = guardKey ? unconfirmed.get(guardKey) : undefined
 
-    if (awaited !== undefined && awaited !== row.pinned) {
-      continue
+    if (guard && guardKey) {
+      if (guard.value === row.pinned) {
+        unconfirmed.delete(guardKey)
+      } else if (Date.now() - guard.at < WRITE_GUARD_MS) {
+        continue
+      } else {
+        unconfirmed.delete(guardKey)
+      }
     }
 
     // Local intent still waiting on its PATCH (row unresolved when the push
@@ -160,3 +183,20 @@ export function watchSessionPins(): void {
   $pinnedSessionIds.listen(reconcile)
   $sessions.listen(reconcile)
 }
+
+/**
+ * Forget what we've mirrored, because the backend we mirrored it TO is gone.
+ *
+ * `mirrored` / `pending` / `unconfirmed` all mean "relative to the gateway we
+ * are talking to". After a soft switch the next backend has its own state.db
+ * and has never seen these pins, but `mirrored` would report them as already
+ * pushed and suppress the PATCHes — so the user's pins silently fail to reach
+ * the new gateway (and its auto-archive sweep is free to hide them). Dropping
+ * the bookkeeping makes the next reconcile re-assert the whole set, which is
+ * the same path that migrates pre-existing pins at boot.
+ */
+export function resetSessionPinMirror(): void {
+  mirrored.clear()
+  pending.clear()
+  unconfirmed.clear()
+}