From 99540e20fa5e5bb08e87491c2cf0b72da380d0e8 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Tue, 12 May 2026 13:44:10 -0700 Subject: [PATCH] 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 --- src/renderer/src/lib/session-write-subscriber.test.ts | 8 ++++++++ src/renderer/src/lib/session-write-subscriber.ts | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/lib/session-write-subscriber.test.ts b/src/renderer/src/lib/session-write-subscriber.test.ts index 19676188812..19701600e3b 100644 --- a/src/renderer/src/lib/session-write-subscriber.test.ts +++ b/src/renderer/src/lib/session-write-subscriber.test.ts @@ -127,5 +127,13 @@ describe('createSessionWriteSubscriber', () => { 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() }) }) diff --git a/src/renderer/src/lib/session-write-subscriber.ts b/src/renderer/src/lib/session-write-subscriber.ts index a7893d4ef34..b25fa82a1b2 100644 --- a/src/renderer/src/lib/session-write-subscriber.ts +++ b/src/renderer/src/lib/session-write-subscriber.ts @@ -60,7 +60,15 @@ export function createSessionWriteSubscriber({ } timer = setTimeout(() => { timer = null - persist(buildWorkspaceSessionPayload(state)) + // 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) })