perf(session): gate session-write subscriber on relevant field changes (#1720)

* perf(session): gate session-write subscriber on relevant field changes

The App-level Zustand subscriber that debounces buildWorkspaceSessionPayload
fires on every store update (agent status, usage refreshes, runtime title
ticks, …). Each fire reset the 150ms timer, and when the timer eventually
expired the rebuild crossed 70-110ms with many tabs open, tripping
setTimeout violation warnings. Add a shallow reference-equality gate over
the fields actually consumed by the payload so the timer only resets when
those fields change. The field list is co-located with
WorkspaceSessionSnapshot and locked to it via a compile-time exhaustiveness
check, so adding a future snapshot field will fail to typecheck rather
than silently skip the gate.

Co-authored-by: Orca <help@stably.ai>

* test(session): regression test for session-write debounce gate

Extract the subscriber into createSessionWriteSubscriber so a vitest can
drive the real Zustand store and assert which mutations cause a session
write. The gate against unrelated updates (agent status, cache timers,
runtime title ticks) is load-bearing for setTimeout violation budgets
and the failure mode is silent — without this test, future store
additions could re-introduce the regression unnoticed.

Six cases lock in the contract: no write while not ready, exactly one
write when ready flips, no write on unrelated mutations, exactly one
write on a relevant mutation, coalescing within a debounce window, and
cleanup cancels a pending timer.

Co-authored-by: Orca <help@stably.ai>

* perf(session): rebuild session payload from latest store state in debounce

Replace the closed-over `state` snapshot captured at timer-schedule time
with `store.getState()` inside the setTimeout callback. Today this is
behaviorally equivalent because `buildWorkspaceSessionPayload` reads only
SESSION_RELEVANT_FIELDS (the same fields gating the timer reset), but a
future refactor that adds a non-relevant field read to the payload builder
would silently start emitting stale values without this guard.

Also tighten the cleanup test: mutate the store after `cleanup()` and assert
no persist, so a regression where the timer is cancelled but the listener
is left subscribed would now fail rather than pass.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/App.tsx

@@ -49,6 +49,7 @@ import {
 import { useGlobalFileDrop } from './hooks/useGlobalFileDrop'
 import { registerUpdaterBeforeUnloadBypass } from './lib/updater-beforeunload'
 import { buildWorkspaceSessionPayload } from './lib/workspace-session'
+import { createSessionWriteSubscriber } from './lib/session-write-subscriber'
 import { applyDocumentTheme } from './lib/document-theme'
 import { isEditableTarget } from './lib/editable-target'
 import {
@@ -416,25 +417,10 @@ function App(): React.JSX.Element {
   // Using a Zustand subscribe() outside React removes ~15 subscriptions from
   // App's render cycle, eliminating re-renders on every tab/file/browser change.
   useEffect(() => {
-    let timer: number | null = null
-    const unsub = useAppStore.subscribe((state) => {
-      if (!state.workspaceSessionReady) {
-        return
-      }
-      if (timer) {
-        window.clearTimeout(timer)
-      }
-      timer = window.setTimeout(() => {
-        timer = null
-        void window.api.session.set(buildWorkspaceSessionPayload(state))
-      }, 150)
+    return createSessionWriteSubscriber({
+      store: useAppStore,
+      persist: (payload) => void window.api.session.set(payload)
     })
-    return () => {
-      unsub()
-      if (timer) {
-        window.clearTimeout(timer)
-      }
-    }
   }, [])
 
   // On shutdown, capture terminal scrollback buffers and flush to disk.

src/renderer/src/lib/session-write-subscriber.test.ts

@@ -0,0 +1,139 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { WorkspaceSessionState } from '../../../shared/types'
+import { useAppStore, type AppState } from '@/store'
+import { createSessionWriteSubscriber } from './session-write-subscriber'
+
+// Why: useAppStore is a module-level singleton — tests must snapshot and
+// restore the full state around each case so cross-test pollution can't mask
+// a real regression in the gate logic this suite exists to lock down.
+let initialState: AppState
+
+describe('createSessionWriteSubscriber', () => {
+  beforeEach(() => {
+    initialState = useAppStore.getState()
+    vi.useFakeTimers()
+  })
+
+  afterEach(() => {
+    vi.useRealTimers()
+    useAppStore.setState(initialState, true)
+  })
+
+  it('does not write while workspaceSessionReady is false', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ tabsByWorktree: { 'wt-1': [] } })
+    vi.advanceTimersByTime(200)
+
+    expect(persist).not.toHaveBeenCalled()
+    cleanup()
+  })
+
+  it('writes exactly once after workspaceSessionReady flips to true', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ workspaceSessionReady: true })
+    vi.advanceTimersByTime(200)
+
+    expect(persist).toHaveBeenCalledTimes(1)
+    cleanup()
+  })
+
+  it('ignores mutations to fields outside SESSION_RELEVANT_FIELDS', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ workspaceSessionReady: true })
+    vi.advanceTimersByTime(200)
+    expect(persist).toHaveBeenCalledTimes(1)
+    persist.mockClear()
+
+    // setAgentStatus / setCacheTimerStartedAt mutate fields that are NOT in
+    // SESSION_RELEVANT_FIELDS — the gate must skip the timer reset entirely.
+    useAppStore.getState().setAgentStatus('tab-1:1', {
+      state: 'working',
+      prompt: 'Fix tests',
+      agentType: 'codex'
+    })
+    useAppStore.getState().setCacheTimerStartedAt('tab-1:pane-1', Date.now())
+    vi.advanceTimersByTime(200)
+
+    expect(persist).not.toHaveBeenCalled()
+    cleanup()
+  })
+
+  it('writes exactly once when a relevant field changes', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ workspaceSessionReady: true })
+    vi.advanceTimersByTime(200)
+    expect(persist).toHaveBeenCalledTimes(1)
+    persist.mockClear()
+
+    useAppStore.setState({
+      tabsByWorktree: {
+        'wt-1': [
+          {
+            id: 'tab-1',
+            ptyId: null,
+            worktreeId: 'wt-1',
+            title: 'shell',
+            customTitle: null,
+            color: null,
+            sortOrder: 0,
+            createdAt: 1
+          }
+        ]
+      }
+    })
+    vi.advanceTimersByTime(200)
+
+    expect(persist).toHaveBeenCalledTimes(1)
+    cleanup()
+  })
+
+  it('coalesces multiple relevant mutations within a debounce window', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ workspaceSessionReady: true })
+    vi.advanceTimersByTime(200)
+    persist.mockClear()
+
+    useAppStore.setState({ activeRepoId: 'repo-1' })
+    vi.advanceTimersByTime(50)
+    useAppStore.setState({ activeWorktreeId: 'wt-1' })
+    vi.advanceTimersByTime(50)
+    useAppStore.setState({ activeTabId: 'tab-1' })
+    vi.advanceTimersByTime(200)
+
+    expect(persist).toHaveBeenCalledTimes(1)
+    cleanup()
+  })
+
+  it('cleanup unsubscribes and cancels a pending timer', () => {
+    const persist = vi.fn<(payload: WorkspaceSessionState) => void>()
+    const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist })
+
+    useAppStore.setState({ workspaceSessionReady: true })
+    vi.advanceTimersByTime(200)
+    persist.mockClear()
+
+    useAppStore.setState({ activeTabId: 'tab-1' })
+    cleanup()
+    vi.advanceTimersByTime(200)
+
+    expect(persist).not.toHaveBeenCalled()
+
+    // Why: without this second mutation, the assertion above only proves the
+    // pending timer was cancelled — a regression where cleanup() forgot to
+    // unsub() would still pass. Mutating after cleanup verifies the listener
+    // was detached and no new timer is queued.
+    useAppStore.setState({ activeTabId: 'tab-2' })
+    vi.advanceTimersByTime(200)
+    expect(persist).not.toHaveBeenCalled()
+  })
+})

src/renderer/src/lib/session-write-subscriber.ts

@@ -0,0 +1,81 @@
+import type { AppState } from '../store'
+import type { WorkspaceSessionState } from '../../../shared/types'
+import { buildWorkspaceSessionPayload, SESSION_RELEVANT_FIELDS } from './workspace-session'
+
+export type SessionWriteSubscriberDeps = {
+  store: {
+    subscribe: (listener: (state: AppState) => void) => () => void
+    getState: () => AppState
+  }
+  persist: (payload: WorkspaceSessionState) => void
+  debounceMs?: number
+}
+
+/**
+ * Why: factored out so a vitest can drive the real Zustand store and assert
+ * which mutations cause a session write — the gate against unrelated updates
+ * (agent status, usage, runtime title ticks) is load-bearing for setTimeout
+ * violation budgets and the failure mode is silent.
+ */
+export function createSessionWriteSubscriber({
+  store,
+  persist,
+  debounceMs = 150
+}: SessionWriteSubscriberDeps): () => void {
+  let timer: ReturnType<typeof setTimeout> | null = null
+  // Why: the subscriber fires on every store update (agent status, usage
+  // refreshes, runtime title ticks, …). Without this gate each fire reset
+  // the debounce, and when it finally expired buildWorkspaceSessionPayload
+  // crossed 70-110ms with many tabs, tripping setTimeout violations. Compare
+  // each session-feeding field by reference against the prior snapshot and
+  // skip both the timer reset and the rebuild when none changed. `null`
+  // sentinel guarantees the very first fire always proceeds.
+  let prev: Record<string, unknown> | null = null
+
+  const unsub = store.subscribe((state) => {
+    if (!state.workspaceSessionReady) {
+      return
+    }
+    let changed = false
+    if (prev === null) {
+      changed = true
+    } else {
+      for (const key of SESSION_RELEVANT_FIELDS) {
+        if (prev[key] !== state[key]) {
+          changed = true
+          break
+        }
+      }
+    }
+    if (!changed) {
+      return
+    }
+    const next: Record<string, unknown> = {}
+    for (const key of SESSION_RELEVANT_FIELDS) {
+      next[key] = state[key]
+    }
+    prev = next
+    if (timer !== null) {
+      clearTimeout(timer)
+    }
+    timer = setTimeout(() => {
+      timer = null
+      // Why: rebuild from the freshest store state rather than the snapshot
+      // captured when this timer was scheduled. Today this is equivalent
+      // because buildWorkspaceSessionPayload reads only SESSION_RELEVANT_FIELDS
+      // (the same fields gating the timer reset), so the captured `state` is
+      // already current for those fields. Calling getState() guards against a
+      // future refactor that adds a non-relevant field read to the payload
+      // builder — without this, such a change would silently start emitting
+      // stale values for that field.
+      persist(buildWorkspaceSessionPayload(store.getState()))
+    }, debounceMs)
+  })
+
+  return () => {
+    unsub()
+    if (timer !== null) {
+      clearTimeout(timer)
+    }
+  }
+}

src/renderer/src/lib/workspace-session.test.ts

@@ -1,5 +1,9 @@
 import { describe, expect, it } from 'vitest'
-import { buildWorkspaceSessionPayload } from './workspace-session'
+import {
+  buildWorkspaceSessionPayload,
+  SESSION_RELEVANT_FIELDS,
+  type WorkspaceSessionSnapshot
+} from './workspace-session'
 import type { AppState } from '../store'
 
 function createSnapshot(overrides: Partial<AppState> = {}): AppState {
@@ -140,3 +144,49 @@ describe('buildWorkspaceSessionPayload', () => {
     expect(payload.activeTabTypeByWorktree).toEqual({ 'wt-2': 'terminal' })
   })
 })
+
+describe('SESSION_RELEVANT_FIELDS', () => {
+  // Why: this list gates the App-level session-write debounce subscriber.
+  // If a future field is added to WorkspaceSessionSnapshot but not to the
+  // gate, the subscriber would silently stop noticing changes to that field
+  // and persist stale data. Listing every key here as a fixture and asserting
+  // the gate covers them catches the drift at test time. The compile-time
+  // _exhaustive check in workspace-session.ts is the primary line of defense;
+  // this test is the runtime backstop.
+  const fixture: Record<keyof WorkspaceSessionSnapshot, true> = {
+    activeRepoId: true,
+    activeWorktreeId: true,
+    activeTabId: true,
+    tabsByWorktree: true,
+    terminalLayoutsByTabId: true,
+    activeTabIdByWorktree: true,
+    openFiles: true,
+    activeFileIdByWorktree: true,
+    activeTabTypeByWorktree: true,
+    browserTabsByWorktree: true,
+    browserPagesByWorkspace: true,
+    activeBrowserTabIdByWorktree: true,
+    browserUrlHistory: true,
+    unifiedTabsByWorktree: true,
+    groupsByWorktree: true,
+    layoutByWorktree: true,
+    activeGroupIdByWorktree: true,
+    sshConnectionStates: true,
+    repos: true,
+    worktreesByRepo: true,
+    lastKnownRelayPtyIdByTabId: true,
+    lastVisitedAtByWorktreeId: true
+  }
+
+  it('contains every key of WorkspaceSessionSnapshot', () => {
+    const fixtureKeys = Object.keys(fixture)
+    expect(
+      fixtureKeys.every((k) => (SESSION_RELEVANT_FIELDS as readonly string[]).includes(k))
+    ).toBe(true)
+    expect(SESSION_RELEVANT_FIELDS.length).toBe(fixtureKeys.length)
+  })
+
+  it('has no duplicate entries', () => {
+    expect(new Set(SESSION_RELEVANT_FIELDS).size).toBe(SESSION_RELEVANT_FIELDS.length)
+  })
+})

src/renderer/src/lib/workspace-session.ts

@@ -8,7 +8,7 @@ import type {
 import type { AppState } from '../store'
 import type { OpenFile } from '../store/slices/editor'
 
-type WorkspaceSessionSnapshot = Pick<
+export type WorkspaceSessionSnapshot = Pick<
   AppState,
   | 'activeRepoId'
   | 'activeWorktreeId'
@@ -34,6 +34,44 @@ type WorkspaceSessionSnapshot = Pick<
   | 'lastVisitedAtByWorktreeId'
 >
 
+// Why: the App-level Zustand subscriber that debounces session writes uses
+// this list as a shallow-equality gate so it only resets the timer when a
+// field that actually feeds buildWorkspaceSessionPayload changes. Keeping
+// the list co-located with WorkspaceSessionSnapshot means a future field
+// added to the snapshot type fails the _exhaustive check below at compile
+// time, preventing the gate from silently going stale.
+export const SESSION_RELEVANT_FIELDS = [
+  'activeRepoId',
+  'activeWorktreeId',
+  'activeTabId',
+  'tabsByWorktree',
+  'terminalLayoutsByTabId',
+  'activeTabIdByWorktree',
+  'openFiles',
+  'activeFileIdByWorktree',
+  'activeTabTypeByWorktree',
+  'browserTabsByWorktree',
+  'browserPagesByWorkspace',
+  'activeBrowserTabIdByWorktree',
+  'browserUrlHistory',
+  'unifiedTabsByWorktree',
+  'groupsByWorktree',
+  'layoutByWorktree',
+  'activeGroupIdByWorktree',
+  'sshConnectionStates',
+  'repos',
+  'worktreesByRepo',
+  'lastKnownRelayPtyIdByTabId',
+  'lastVisitedAtByWorktreeId'
+] as const satisfies readonly (keyof WorkspaceSessionSnapshot)[]
+
+type _MissingSessionField = Exclude<
+  keyof WorkspaceSessionSnapshot,
+  (typeof SESSION_RELEVANT_FIELDS)[number]
+>
+const _exhaustive: [_MissingSessionField] extends [never] ? true : never = true
+void _exhaustive
+
 /** Build the editor-file portion of the workspace session for persistence.
  *  Only edit-mode files are saved — diffs and conflict views are transient. */
 export function buildEditorSessionData(