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>
This commit is contained in:
brennanb2025
2026-05-12 13:44:10 -07:00
co-authored by Orca
parent 07a78157cc
commit 99540e20fa
2 changed files with 17 additions and 1 deletions
@@ -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()
})
})
@@ -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)
})