From aa98edf35ab918f53e56424dfd7f39a00eb5138e Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:40:33 -0700 Subject: [PATCH] fix(runtime): contain a parked resume replay that throws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handle-gap park releases its waiters from inside `useAppStore.subscribe`, and the replay it runs is `resumeSleepingAgentSessionsForWorktree` — a large synchronous sweep. Zustand notifies listeners in a plain loop, so a replay that threw escaped the `setState` that triggered it: measured, the exception left the store write, every listener registered after this module missed that write, and the sibling pane the same mirror frame made due was never released. The waiter's timer, map entry and subscription are already torn down before `run`, so containing the throw holds nothing back — it only stops one pane's failed recovery from taking the store notification and its siblings with it. --- ...rror-handle-gap-replay-containment.test.ts | 82 +++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 10 ++- 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-replay-containment.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-replay-containment.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-replay-containment.test.ts new file mode 100644 index 00000000000..b11d80ba188 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-replay-containment.test.ts @@ -0,0 +1,82 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { clearRuntimeEnvironmentConnectionGenerationsForTests } from '@/store/slices/runtime-status' +import { + HOST_MIRROR_HANDLE_GAP_DEADLINE_MS, + countParkedHostMirrorHandleGapPanesForTests, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +// What this pins: the parked replay is `resumeSleepingAgentSessionsForWorktree`, a large +// synchronous sweep, and it is released from inside `useAppStore.subscribe`. Zustand notifies +// listeners in a plain loop, so a replay that throws escapes the `setState` that triggered it: +// the listeners registered after this module never see the write, and every sibling pane the +// same mirror frame made due is left parked. The waiter's own state is torn down before `run`, +// so containing the throw holds nothing back. + +const initialAppStoreState = useAppStore.getState() +const ENV_ID = 'env-gap-containment' + +function seedTwoMirroredPanes(): void { + useAppStore.setState({ + tabsByWorktree: { + wt: [ + { id: 'tab-a', title: 'a', ptyId: null }, + { id: 'tab-b', title: 'b', ptyId: null } + ] as never + }, + ptyIdsByTabId: {} + }) +} + +describe('host-mirror handle-gap replay containment', () => { + beforeEach(() => { + vi.useFakeTimers() + useAppStore.setState(initialAppStoreState, true) + }) + + afterEach(() => { + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it('a replay that throws neither aborts the store write nor strands its sibling panes', () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + seedTwoMirroredPanes() + const siblingReplay = vi.fn() + parkUntilHostMirrorHandleLands(ENV_ID, 'wt', 'tab-a', () => { + throw new Error('replay blew up') + }) + parkUntilHostMirrorHandleLands(ENV_ID, 'wt', 'tab-b', siblingReplay) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(2) + + // A store subscriber registered after this module's, so it is notified after the drain. + const laterSubscriber = vi.fn() + const unsubscribe = useAppStore.subscribe(laterSubscriber) + + // One mirror frame lands both handles, making both waiters due on a single store write. + expect(() => + useAppStore.setState({ ptyIdsByTabId: { 'tab-a': ['pty-a'], 'tab-b': ['pty-b'] } }) + ).not.toThrow() + unsubscribe() + + expect(siblingReplay).toHaveBeenCalledTimes(1) + expect(laterSubscriber).toHaveBeenCalledTimes(1) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + }) + + it('a replay that throws on the deadline path does not escape the timer', () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + seedTwoMirroredPanes() + parkUntilHostMirrorHandleLands(ENV_ID, 'wt', 'tab-a', () => { + throw new Error('replay blew up') + }) + + expect(() => vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS)).not.toThrow() + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + }) +}) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts index 9582dcc9d6b..37cc8b2ba02 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -82,7 +82,15 @@ function releaseWaiter(key: string): void { clearTimeout(waiter.deadline) waitersByPane.delete(key) stopStoreSubscriptionIfIdle() - waiter.run() + // Why contained: the release path runs inside useAppStore.subscribe, so a replay that throws + // escapes the setState that triggered it — aborting the listener loop, so every subscriber + // after this one misses the write, and stranding the sibling panes the same frame made due. + // The waiter's own state is already torn down above, so nothing is held by swallowing here. + try { + waiter.run() + } catch (error) { + console.error(`[host-mirror] parked resume replay failed for ${key}:`, error) + } } function waiterIsReleased(waiter: HandleGapWaiter, state: HandleGapStoreState): boolean {