diff --git a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts index dcebb3953c4..3d5e2625944 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -22,11 +22,21 @@ import { // separate frames, so there is a frame where the row exists and `ptyIdsByTabId` is still empty. // An empty handle map for a row the host is still publishing is `unverifiable`, never `exited` // (docs/reference/ssh-execution-boundary.md), so nothing may be resumed off it. +// +// HOW TO ASSERT ON THIS MODULE, because the obvious way cannot fail. "Did the waiter release" is +// NOT an observable here: a waiter released for the wrong reason is immediately re-parked by the +// replayed sweep, so the store, the record and the parked count all read identically one tick +// later. A mutation that released every waiter on any tab's handle survived twelve tests written +// that way. What a spurious release actually costs is the deadline — the re-park starts a fresh +// budget — so the assertion has to advance the clock: park, advance part of the budget, do the +// thing, then advance to the ORIGINAL deadline and require the pane to decide on schedule. const initialAppStoreState = useAppStore.getState() const LEAF_ID = '22222222-2222-4222-8222-222222222222' const WEB_TAB_ID = 'web-terminal-host-tab-1' +const SECOND_LEAF_ID = '33333333-3333-4333-8333-333333333333' +const SECOND_TAB_ID = 'web-terminal-host-tab-2' const RUNTIME_ENV_ID = 'env-handle-gap' function makeRuntimeOwnedWorktree(): ReturnType { @@ -74,17 +84,45 @@ function seedMirroredWorkspace(worktree: ReturnType { beforeEach(() => { vi.useFakeTimers() @@ -264,4 +306,95 @@ describe('resume across the mirror handle gap', () => { expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() }) + + // Why this is not the test above: there the wait had already expired before the reconnect, so + // the stale verdict was a map entry. Here the wait is still armed when the generation moves, and + // its deadline then fires on a connection that has had no chance at all to publish the handle. + it('does not let a wait armed on the previous connection decide the new one', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + // The host reconnects one millisecond before the wait's own deadline. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS - 1) + setRuntimeEnvironmentConnectionGenerationForTests(RUNTIME_ENV_ID, 1) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + vi.advanceTimersByTime(1) + + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(0) + // Re-armed, not held: the new connection gets its own budget and then decides. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) + }) + + it('releases only the pane whose handle landed when two panes share the environment', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedSecondMirroredPane(worktree.id) + const firstPaneKey = seedActiveSleepingRecordFor(worktree.id, WEB_TAB_ID, LEAF_ID, 'session-1') + const secondPaneKey = seedActiveSleepingRecordFor( + worktree.id, + SECOND_TAB_ID, + SECOND_LEAF_ID, + 'session-2' + ) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(2) + + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@term_1'] } }) + + // The first pane owns its live PTY; the second is still undecided, not resumed. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + const after = useAppStore.getState() + expect(after.sleepingAgentSessionsByPaneKey[firstPaneKey]).toBeDefined() + expect(after.sleepingAgentSessionsByPaneKey[secondPaneKey]).toBeDefined() + expect(Object.keys(after.automaticAgentResumeClaimsByTabId)).toHaveLength(0) + + // Why the clock matters: releasing the second pane here and letting the replay re-park it + // would look identical right now and silently restart its budget. Its own deadline still has + // to land on the original schedule. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[secondPaneKey]).toBeUndefined() + }) + + it('does not release or reschedule a park because another environment published a handle', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + useAppStore.setState({ + ptyIdsByTabId: { 'web-terminal-other-env-tab': ['remote:env-other@@term_1'] } + }) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + // The unrelated handle must not have restarted this pane's budget. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + }) + + it('leaves no waiter or timer behind when the environment tears its rows down mid-park', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + // Teardown drops every row the environment owned. + useAppStore.setState({ tabsByWorktree: {}, terminalLayoutsByTabId: {} } as never) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + // Nothing may still be scheduled against the torn-down environment. + expect(vi.getTimerCount()).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 bc33f91689b..9582dcc9d6b 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -26,6 +26,8 @@ export const HOST_MIRROR_HANDLE_GAP_DEADLINE_MS = WEB_SESSION_TAB_RPC_TIMEOUT_MS type HandleGapWaiter = { worktreeId: string tabId: string + /** Connection generation the wait was armed on; its verdict is void on any other. */ + generation: number deadline: ReturnType run: () => void } @@ -141,11 +143,22 @@ export function parkUntilHostMirrorHandleLands( existing.run = run return } + const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) const deadline = setTimeout(() => { - recordExpiredWait(environmentId, key) + // Why the generation is re-read: a reconnect mid-park makes this wait's silence + // evidence about a connection that is gone. Recording it would let a wait armed + // milliseconds before the reconnect authorize a resume on the new one — the #19735 + // fork with an extra step. Release without a verdict instead; the replay re-parks + // and the new connection gets its own full budget. + if ( + waitersByPane.get(key)?.generation === + getRuntimeEnvironmentConnectionGeneration(environmentId) + ) { + recordExpiredWait(environmentId, key) + } releaseWaiter(key) }, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) - waitersByPane.set(key, { worktreeId, tabId, deadline, run }) + waitersByPane.set(key, { worktreeId, tabId, generation, deadline, run }) startStoreSubscription() }