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 a1d7b5e1b9a..dcebb3953c4 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 @@ -1,5 +1,5 @@ import path from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore, type AppState } from '@/store' import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' import { makeCreatedAgentWorktree } from '@/lib/worktree-activation-created-agent-test-state' @@ -8,7 +8,15 @@ import { markHostSessionMirrorHydrated, resetHostSessionMirrorHydrationForTests } from '@/runtime/host-session-mirror-hydration' -import { clearRuntimeEnvironmentConnectionGenerationsForTests } from '@/store/slices/runtime-status' +import { + clearRuntimeEnvironmentConnectionGenerationsForTests, + setRuntimeEnvironmentConnectionGenerationForTests +} from '@/store/slices/runtime-status' +import { + HOST_MIRROR_HANDLE_GAP_DEADLINE_MS, + countParkedHostMirrorHandleGapPanesForTests, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' // The window this pins: a paired runtime publishes a workspace's tab rows and its PTY handles on // separate frames, so there is a frame where the row exists and `ptyIdsByTabId` is still empty. @@ -92,15 +100,20 @@ function seedActiveSleepingRecord(worktreeId: string): string { describe('resume across the mirror handle gap', () => { beforeEach(() => { + vi.useFakeTimers() useAppStore.setState(initialAppStoreState, true) resetHostSessionMirrorHydrationForTests() + resetHostMirrorHandleGapWaitsForTests() clearRuntimeEnvironmentConnectionGenerationsForTests() }) afterEach(() => { + // Why first: the store reset below retracts every row, which would replay a still-parked wait. + resetHostMirrorHandleGapWaitsForTests() useAppStore.setState(initialAppStoreState, true) resetHostSessionMirrorHydrationForTests() clearRuntimeEnvironmentConnectionGenerationsForTests() + vi.useRealTimers() }) it('does not resume a published mirrored pane whose handle has not landed yet', () => { @@ -118,6 +131,8 @@ describe('resume across the mirror handle gap', () => { expect(Object.keys(after.pendingStartupByTabId)).toHaveLength(0) // The record survives: the next frame carries the handle and decides for real. expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + // And something is armed to decide it — a hold with nothing armed is the defect, not the fix. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) }) it('still resumes once the host has published the row without any live handle', () => { @@ -129,4 +144,124 @@ describe('resume across the mirror handle gap', () => { expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(1) }) + + // The three exits of the per-pane park. A park with no bounded release is the + // latch-that-never-releases defect, so each one must replay the sweep. + + it("releases when the pane's own handle lands and keeps the pane it now owns", () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@term_1'] } }) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + // The released waiter must not fire again at the deadline. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + const after = useAppStore.getState() + expect(after.tabsByWorktree[worktree.id]).toHaveLength(1) + expect(Object.keys(after.automaticAgentResumeClaimsByTabId)).toHaveLength(0) + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + }) + + it('releases when a handle lands for the tab and resumes if it belongs to another pane', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@other'] } }) + + const after = useAppStore.getState() + const resumeTabIds = (after.tabsByWorktree[worktree.id] ?? []) + .map((tab) => tab.id) + .filter((id) => id !== WEB_TAB_ID) + expect(resumeTabIds).toHaveLength(1) + expect(after.automaticAgentResumeClaimsByTabId[resumeTabIds[0]!]?.providerSession).toEqual({ + key: 'session_id', + id: 'handle-gap-session' + }) + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + }) + + it('releases when the host retracts the row and resumes into a fresh tab', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + useAppStore.setState({ tabsByWorktree: { [worktree.id]: [] } }) + + const after = useAppStore.getState() + const tabs = after.tabsByWorktree[worktree.id] ?? [] + expect(tabs).toHaveLength(1) + expect(after.automaticAgentResumeClaimsByTabId[tabs[0]!.id]?.providerSession).toEqual({ + key: 'session_id', + id: 'handle-gap-session' + }) + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + }) + + it('releases at the deadline and resumes rather than holding the pane forever', () => { + 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 - 1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + vi.advanceTimersByTime(1) + + const after = useAppStore.getState() + const resumeTabIds = (after.tabsByWorktree[worktree.id] ?? []) + .map((tab) => tab.id) + .filter((id) => id !== WEB_TAB_ID) + expect(resumeTabIds).toHaveLength(1) + expect(after.automaticAgentResumeClaimsByTabId[resumeTabIds[0]!]?.providerSession).toEqual({ + key: 'session_id', + id: 'handle-gap-session' + }) + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + }) + + it('keeps the original deadline when a second sweep re-parks the same pane', () => { + 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) + // A re-activation mid-wait must not push the decision out another full budget. + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) + }) + + it('re-arms the wait after a reconnect instead of inheriting the expired verdict', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) + + // A host restart: the same row, a new connection, its handle unknown again. + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + setRuntimeEnvironmentConnectionGenerationForTests(RUNTIME_ENV_ID, 1) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + }) }) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts new file mode 100644 index 00000000000..88f95a0378c --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -0,0 +1,151 @@ +import { useAppStore } from '@/store' +import { getRuntimeEnvironmentConnectionGeneration } from '@/store/slices/runtime-status' +import { WEB_SESSION_TAB_RPC_TIMEOUT_MS } from '@/runtime/web-session-tab-rpc-timeout' + +/** + * Per-pane park for the frame between a host's tab rows and its PTY handles. + * + * Why: mirror hydration says "the rows arrived", not "this pane's liveness is + * decidable" — the handle lands one relay round trip later. A pane whose leaf + * is still bound to a PTY of the same environment, with no published handle, + * is `unverifiable` (docs/reference/ssh-execution-boundary.md); resuming on it + * forked a session the host was still running (#19735). + * + * The wait is bounded because mirror settlement has already happened and will + * not replay a parked sweep again. Three exits, each replaying the sweep: + * - the pane's own handle lands (`ptyIdsByTabId[tabId]` non-empty); + * - the row is retracted (the host has spoken: the pane is gone); + * - the deadline expires. A handle that has not landed within the RPC budget + * is not coming on this connection, so the pane is released to ordinary + * recovery: a resume after a bounded wait is defensible, an indefinite hold + * is the latch-that-never-releases defect. A reconnect bumps the connection + * generation and arms a fresh wait. + */ +export const HOST_MIRROR_HANDLE_GAP_DEADLINE_MS = WEB_SESSION_TAB_RPC_TIMEOUT_MS + +type HandleGapWaiter = { + worktreeId: string + tabId: string + deadline: ReturnType + run: () => void +} + +type HandleGapStoreState = Pick< + ReturnType, + 'ptyIdsByTabId' | 'tabsByWorktree' +> + +const waitersByPane = new Map() +/** Connection generation whose wait already expired for the pane. */ +const expiredGenerationByPane = new Map() +let unsubscribeStore: (() => void) | null = null + +function paneWaitKey(environmentId: string, tabId: string): string { + return `${environmentId}\0${tabId}` +} + +/** True once the deadline fired for this pane on the current connection. */ +export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: string): boolean { + return ( + expiredGenerationByPane.get(paneWaitKey(environmentId, tabId)) === + getRuntimeEnvironmentConnectionGeneration(environmentId) + ) +} + +function stopStoreSubscriptionIfIdle(): void { + if (waitersByPane.size === 0 && unsubscribeStore) { + unsubscribeStore() + unsubscribeStore = null + } +} + +function releaseWaiter(key: string): void { + const waiter = waitersByPane.get(key) + if (!waiter) { + return + } + clearTimeout(waiter.deadline) + waitersByPane.delete(key) + stopStoreSubscriptionIfIdle() + waiter.run() +} + +function waiterIsReleased(waiter: HandleGapWaiter, state: HandleGapStoreState): boolean { + if ((state.ptyIdsByTabId[waiter.tabId]?.length ?? 0) > 0) { + return true + } + const tabs = state.tabsByWorktree[waiter.worktreeId] ?? [] + return !tabs.some((tab) => tab.id === waiter.tabId) +} + +function releaseDueWaiters(state: HandleGapStoreState): void { + // Why: drain from a snapshot — a replay can re-park the pane, and that new + // waiter belongs to the next store write, not this one. + const dueKeys: string[] = [] + for (const [key, waiter] of waitersByPane) { + if (waiterIsReleased(waiter, state)) { + dueKeys.push(key) + } + } + for (const key of dueKeys) { + releaseWaiter(key) + } +} + +function startStoreSubscription(): void { + if (unsubscribeStore) { + return + } + let previous: HandleGapStoreState = useAppStore.getState() + unsubscribeStore = useAppStore.subscribe((state) => { + // Why: only these two slices can release a waiter; title, status, and + // usage ticks must not rescan every parked pane. + if ( + state.ptyIdsByTabId === previous.ptyIdsByTabId && + state.tabsByWorktree === previous.tabsByWorktree + ) { + return + } + previous = state + releaseDueWaiters(state) + }) +} + +/** + * Parks `run` until the pane's handle lands, its row is retracted, or the + * deadline expires. Re-parking an already-parked pane replaces `run` but keeps + * the original deadline, so a replay that re-parks cannot extend the wait. + */ +export function parkUntilHostMirrorHandleLands( + environmentId: string, + worktreeId: string, + tabId: string, + run: () => void +): void { + const key = paneWaitKey(environmentId, tabId) + const existing = waitersByPane.get(key) + if (existing) { + existing.run = run + return + } + const deadline = setTimeout(() => { + expiredGenerationByPane.set(key, getRuntimeEnvironmentConnectionGeneration(environmentId)) + releaseWaiter(key) + }, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + waitersByPane.set(key, { worktreeId, tabId, deadline, run }) + startStoreSubscription() +} + +export function countParkedHostMirrorHandleGapPanesForTests(): number { + return waitersByPane.size +} + +export function resetHostMirrorHandleGapWaitsForTests(): void { + for (const waiter of waitersByPane.values()) { + clearTimeout(waiter.deadline) + } + waitersByPane.clear() + expiredGenerationByPane.clear() + unsubscribeStore?.() + unsubscribeStore = null +} diff --git a/src/renderer/src/lib/host-mirrored-pane-liveness.ts b/src/renderer/src/lib/host-mirrored-pane-liveness.ts index d39c7bad418..d9e8a70d01a 100644 --- a/src/renderer/src/lib/host-mirrored-pane-liveness.ts +++ b/src/renderer/src/lib/host-mirrored-pane-liveness.ts @@ -1,15 +1,34 @@ import type { useAppStore } from '@/store' import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id' import { parsePaneKey } from '../../../shared/stable-pane-id' import { isWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' import { hasHostSessionMirrorHydrated } from '@/runtime/host-session-mirror-hydration' +import { hasHostMirrorHandleWaitExpired } from './host-mirror-handle-gap-wait' import { getRuntimeEnvironmentIdForWorktree } from './worktree-runtime-owner' type AppStoreState = ReturnType -export type UnhydratedHostMirror = { - /** Null when no paired runtime claims the workspace, so nothing will ever answer for the pane. */ - environmentId: string | null +export type UnhydratedHostMirror = + /** The host's tab rows have not arrived; mirror settlement replays the sweep. */ + | { + kind: 'mirror' + /** Null when no paired runtime claims the workspace, so nothing will ever answer for the pane. */ + environmentId: string | null + } + /** The rows arrived but this pane's PTY handle has not; a bounded per-pane wait replays. */ + | { kind: 'handle'; environmentId: string; tabId: string } + +/** The layout still binds a leaf of this tab to a PTY the environment minted. */ +function tabHoldsEnvironmentPtyBinding( + state: AppStoreState, + tabId: string, + environmentId: string +): boolean { + const bindings = state.terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {} + return Object.values(bindings).some( + (ptyId) => parseRemoteRuntimePtyId(ptyId)?.environmentId === environmentId + ) } /** @@ -19,7 +38,9 @@ export type UnhydratedHostMirror = { * Why: a `web-terminal-*` tab exists only because a host published it, and its * PTY handle arrives one relay round trip later. An empty local handle map is * therefore "unverifiable", never "exited" — the incident's replacement - * `codex resume` forked a session the host still held. + * `codex resume` forked a session the host still held. Mirror hydration only + * says the rows landed, so a pane still bound to this environment's PTY with + * no handle yet gets its own bounded wait (#19735). */ export function findUnhydratedHostMirrorForPane( record: SleepingAgentSessionRecord, @@ -41,8 +62,14 @@ export function findUnhydratedHostMirrorForPane( return null } const environmentId = getRuntimeEnvironmentIdForWorktree(state, record.worktreeId) - if (environmentId && hasHostSessionMirrorHydrated(environmentId, record.worktreeId)) { - return null + if (!environmentId || !hasHostSessionMirrorHydrated(environmentId, record.worktreeId)) { + return { kind: 'mirror', environmentId } } - return { environmentId } + if ( + tabHoldsEnvironmentPtyBinding(state, tabId, environmentId) && + !hasHostMirrorHandleWaitExpired(environmentId, tabId) + ) { + return { kind: 'handle', environmentId, tabId } + } + return null } diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index e0b5b79ac6e..de0163bd0b6 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -14,7 +14,11 @@ import { type ResumeSleepingAgentSessionsOptions } from './sleeping-agent-session-launch' import { isStructuredAgentSyntheticSleepingRecord } from './structured-agent-synthetic-sleeping-record' -import { findUnhydratedHostMirrorForPane } from './host-mirrored-pane-liveness' +import { + findUnhydratedHostMirrorForPane, + type UnhydratedHostMirror +} from './host-mirrored-pane-liveness' +import { parkUntilHostMirrorHandleLands } from './host-mirror-handle-gap-wait' import { resolveWorkspaceTerminalHostAuthority } from './workspace-terminal-host-authority' import { parkUntilHostSessionMirrorHydrates } from '@/runtime/host-session-mirror-hydration' @@ -148,27 +152,37 @@ function isInvalidWorktreeActivationRecord(record: SleepingAgentSessionRecord): ) } -function parkWorktreeResumeSweepUntilHostMirrorHydrates( +function replayParkedWorktreeResumeSweep( worktreeId: string, - environmentId: string | null, options: ResumeSleepingAgentSessionsOptions | undefined ): void { - if (!environmentId) { + // Why: the mirror can settle long after the user moved on, so a replayed + // resume must not steal the surface they are looking at now. + const isActive = useAppStore.getState().activeWorktreeId === worktreeId + // Why `skipClaimKeys` is dropped: it is a park-time snapshot of in-place + // wakes, and a latch that has since failed must stay resumable here. + resumeSleepingAgentSessionsForWorktree(worktreeId, { + ...(options?.onSessionLaunched ? { onSessionLaunched: options.onSessionLaunched } : {}), + ...(isActive ? {} : { suppressNavigation: true }) + }) +} + +function parkWorktreeResumeSweepUntilHostMirrorAnswers( + worktreeId: string, + mirror: UnhydratedHostMirror, + options: ResumeSleepingAgentSessionsOptions | undefined +): void { + const replay = (): void => replayParkedWorktreeResumeSweep(worktreeId, options) + if (mirror.kind === 'handle') { + parkUntilHostMirrorHandleLands(mirror.environmentId, worktreeId, mirror.tabId, replay) + return + } + if (!mirror.environmentId) { // No paired runtime owns the workspace, so no verdict is coming; the next // activation re-runs this sweep once one does. return } - parkUntilHostSessionMirrorHydrates(environmentId, worktreeId, () => { - // Why: the mirror can settle long after the user moved on, so a replayed - // resume must not steal the surface they are looking at now. - const isActive = useAppStore.getState().activeWorktreeId === worktreeId - // Why `skipClaimKeys` is dropped: it is a park-time snapshot of in-place - // wakes, and a latch that has since failed must stay resumable here. - resumeSleepingAgentSessionsForWorktree(worktreeId, { - ...(options?.onSessionLaunched ? { onSessionLaunched: options.onSessionLaunched } : {}), - ...(isActive ? {} : { suppressNavigation: true }) - }) - }) + parkUntilHostSessionMirrorHydrates(mirror.environmentId, worktreeId, replay) } export function resumeSleepingAgentSessionsForWorktree( @@ -219,11 +233,7 @@ export function resumeSleepingAgentSessionsForWorktree( // Why: pane ownership is undecidable until the mirror answers, and every // branch below — launch and clear alike — trusts that verdict. Take no // action on the record; the replay re-runs this pass with real evidence. - parkWorktreeResumeSweepUntilHostMirrorHydrates( - worktreeId, - unhydratedMirror.environmentId, - options - ) + parkWorktreeResumeSweepUntilHostMirrorAnswers(worktreeId, unhydratedMirror, options) continue } const isPaneOwned = recordPaneIsOwnedByPreservedPane(record, currentState)