From c72f7f9400bc30e80c8a95bdbc35e6d99609d519 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 03:16:29 -0700 Subject: [PATCH 01/21] test(repro): #19735 resumes a published mirrored pane before its handle lands --- .../lib/host-mirror-handle-gap-resume.test.ts | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts 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 new file mode 100644 index 00000000000..a1d7b5e1b9a --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -0,0 +1,132 @@ +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } 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' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { + markHostSessionMirrorHydrated, + resetHostSessionMirrorHydrationForTests +} from '@/runtime/host-session-mirror-hydration' +import { clearRuntimeEnvironmentConnectionGenerationsForTests } from '@/store/slices/runtime-status' + +// 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. +// 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. + +const initialAppStoreState = useAppStore.getState() + +const LEAF_ID = '22222222-2222-4222-8222-222222222222' +const WEB_TAB_ID = 'web-terminal-host-tab-1' +const RUNTIME_ENV_ID = 'env-handle-gap' + +function makeRuntimeOwnedWorktree(): ReturnType { + return { + ...makeCreatedAgentWorktree(), + createdWithAgent: undefined, + hostId: `runtime:${encodeURIComponent(RUNTIME_ENV_ID)}` + } +} + +/** A published mirrored row: tab, layout leaf, and the leaf's host PTY binding. */ +function seedMirroredWorkspace(worktree: ReturnType): void { + const state: Partial = { + repos: [ + { + id: 'repo-1', + path: path.join(path.sep, 'workspace', 'repo'), + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0 + } + ], + worktreesByRepo: { 'repo-1': [worktree] }, + activeRepoId: 'repo-1', + activeWorktreeId: worktree.id, + activeView: 'terminal', + tabsByWorktree: { + [worktree.id]: [{ id: WEB_TAB_ID, title: 'Claude', ptyId: null } as never] + }, + terminalLayoutsByTabId: { + [WEB_TAB_ID]: { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'remote:env-handle-gap@@term_1' } + } as never + }, + // The gap itself: the row is published, its handle has not arrived. + ptyIdsByTabId: {}, + sleepingAgentSessionsByPaneKey: {}, + pendingStartupByTabId: {}, + automaticAgentResumeClaimsByTabId: {}, + agentStatusByPaneKey: {} + } + useAppStore.setState(state as AppState) +} + +/** The capture the reported flow produces: recorded mid-turn, so it is active work, not history. */ +function seedActiveSleepingRecord(worktreeId: string): string { + const paneKey = makePaneKey(WEB_TAB_ID, LEAF_ID) + useAppStore.setState({ + sleepingAgentSessionsByPaneKey: { + [paneKey]: { + paneKey, + tabId: WEB_TAB_ID, + worktreeId, + agent: 'claude', + providerSession: { key: 'session_id', id: 'handle-gap-session' }, + connectionId: null, + prompt: '', + state: 'working', + capturedAt: 1000, + updatedAt: 1000, + terminalTitle: 'Claude', + origin: 'live' + } + } + } as never) + return paneKey +} + +describe('resume across the mirror handle gap', () => { + beforeEach(() => { + useAppStore.setState(initialAppStoreState, true) + resetHostSessionMirrorHydrationForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + }) + + afterEach(() => { + useAppStore.setState(initialAppStoreState, true) + resetHostSessionMirrorHydrationForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + }) + + it('does not resume a published mirrored pane whose handle has not landed yet', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + // The rows have arrived; only the handles are outstanding. + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + + const launched = resumeSleepingAgentSessionsForWorktree(worktree.id) + + const after = useAppStore.getState() + expect(launched).toBe(0) + expect(after.tabsByWorktree[worktree.id]).toHaveLength(1) + 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() + }) + + it('still resumes once the host has published the row without any live handle', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedActiveSleepingRecord(worktree.id) + useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@other'] } }) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(1) + }) +}) From 1c042f4b2727bfb5f40e0e85c7f6d266fb007714 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 04:00:03 -0700 Subject: [PATCH 02/21] fix(runtime): park a mirrored pane's resume until its PTY handle lands Mirror hydration means the host's tab rows arrived, not that a given pane's liveness is decidable: the PTY handle lands one relay round trip later. On that frame the pane read as not-live and the sweep resumed a session the host was still running, producing a duplicate resume tab. An empty handle map for a published row is unverifiable, never exited. Park the pane on a per-pane wait with three bounded exits, each replaying the sweep: its own handle lands, the row is retracted, or a deadline expires. The deadline decides resume rather than an indefinite hold, and is scoped to the connection generation so a reconnect re-arms it. Closes #19735 --- .../lib/host-mirror-handle-gap-resume.test.ts | 139 +++++++++++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 151 ++++++++++++++++++ .../src/lib/host-mirrored-pane-liveness.ts | 41 ++++- .../src/lib/resume-sleeping-agent-session.ts | 50 +++--- 4 files changed, 352 insertions(+), 29 deletions(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-wait.ts 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) From a186158c1d25380835692cb41d3cdf9aa88aeb39 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 04:05:48 -0700 Subject: [PATCH 03/21] fix(runtime): bound the handle-gap expiry map to the current connection --- .../src/lib/host-mirror-handle-gap-wait.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 88f95a0378c..bc33f91689b 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -52,6 +52,19 @@ export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: str ) } +function recordExpiredWait(environmentId: string, key: string): void { + const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) + // Why: a verdict from a previous connection is dead weight; drop it so the map + // stays bounded by the panes parked on the current connection. + const prefix = `${environmentId}\0` + for (const [staleKey, staleGeneration] of expiredGenerationByPane) { + if (staleKey.startsWith(prefix) && staleGeneration !== generation) { + expiredGenerationByPane.delete(staleKey) + } + } + expiredGenerationByPane.set(key, generation) +} + function stopStoreSubscriptionIfIdle(): void { if (waitersByPane.size === 0 && unsubscribeStore) { unsubscribeStore() @@ -129,7 +142,7 @@ export function parkUntilHostMirrorHandleLands( return } const deadline = setTimeout(() => { - expiredGenerationByPane.set(key, getRuntimeEnvironmentConnectionGeneration(environmentId)) + recordExpiredWait(environmentId, key) releaseWaiter(key) }, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) waitersByPane.set(key, { worktreeId, tabId, deadline, run }) From 1be4dd9b3f5e1add6ff4d71250d7f807cad12992 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:24:41 -0700 Subject: [PATCH 04/21] fix(runtime): void a handle-gap verdict the reconnect made stale The per-pane park bounds itself with one deadline per connection, but the waiter never recorded WHICH connection it was armed on. A wait armed on generation 0 that fires after a reconnect stamps its expiry against the current generation, so hasHostMirrorHandleWaitExpired agrees, the mirror lookup returns null, and the pane is resumed after 1ms on a connection that has had no chance to publish the handle. That is #19735's fork with an extra step, reached through the guard that exists to prevent it. The module's own doc comment claims the opposite -- "a reconnect bumps the connection generation and arms a fresh wait" -- and that is true only for a wait which had ALREADY expired, which is precisely the case the existing test covered. The test and the comment agreed with each other and both were wrong about the live case. The waiter now carries the generation it was armed on and records no verdict when the generation has moved; the replay re-parks through the existing machinery and the new connection gets its own full budget. Still bounded per connection generation, which is what was documented all along. Also pins the three sibling attacks on the same window: two panes in one environment where only one handle lands, a handle published by a foreign environment, and an environment tearing its rows down mid-park (which leaves no waiter and no scheduled timer). The test file now leads with how to assert on this module at all, because the obvious shape cannot fail. "Did the waiter release" is not an observable here -- a waiter released for the wrong reason is re-parked by the replayed sweep, so the store reads identically one tick later, and a mutation releasing every waiter on any tab's handle survived twelve assertions written that way. What a spurious release costs is the deadline, so the assertions advance the clock and require the pane to decide on the ORIGINAL schedule. --- .../lib/host-mirror-handle-gap-resume.test.ts | 141 +++++++++++++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 17 ++- 2 files changed, 152 insertions(+), 6 deletions(-) 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() } From cc1d1357c256ec60d7efaeef721bde4b8c4e63b6 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:24:58 -0700 Subject: [PATCH 05/21] fix(terminal): a live pane owns its transcript in any workspace The resume dedup was scoped to the record's own workspace on both terms -- the entry's tab had to be in worktreeTabIds AND entry.worktreeId had to match -- and additionally required entry.state !== 'done'. A record whose peer pane has finished a turn and still holds a live PTY therefore matched nothing, and the sweep launched a second agent onto a transcript the peer is still writing. Cross-workspace, it matched nothing even while the peer was mid-turn. The two ids really do drift. canonicalizeTerminalSessionWorktreeId re-keys tabsByWorktree, tabGroups, tabGroupLayouts, activeTabIdByWorktree and activeGroupIdByWorktree onto the canonical worktree id, and does NOT re-key sleepingAgentSessionsByPaneKey, whose records carry worktreeId inside them. So adopting an orphaned terminal is a direct producer of a record naming one workspace while its pane and status row name another. Split into two arms rather than widening the existing condition. The new arm carries no workspace scope but demands hard evidence: a provider session id names one transcript, so a pane whose exact PTY is live right now already owns it wherever that pane sits, and no workspace boundary makes a live PTY less live. The scoped arm keeps its scope and its state !== 'done' term, because a status row with no live PTY is a claim about the past and must not reach across workspaces. Relationship to #19736: that PR fixes the SAME-workspace half of this in the same function, by relaxing only the status term. This arm covers that cell too -- measured both ways on this branch, which does not carry #19736: its thirty `checks exact live ownership before resuming` cases all pass with this change alone, and ten of them fail without it. So this supersedes #19736 rather than sitting beside it, and #19736's one-line `export` of stablePaneHasLivePty is carried here because this arm needs it. If #19736 lands first this becomes a pure widening and its tests should be kept. Both cells are pinned here either way. --- ...eping-agent-session-provider-claim.test.ts | 100 ++++++++++++++++++ .../src/lib/resume-sleeping-agent-session.ts | 37 ++++++- .../src/lib/sleeping-agent-pane-ownership.ts | 2 +- 3 files changed, 133 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts index 7a79e239eb0..8741a465e24 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts @@ -141,4 +141,104 @@ describe('resume sleeping agent provider claims', () => { expect(state.tabsByWorktree['wt-1']).toHaveLength(1) expect(state.sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() }) + + // Why a peer in another workspace is reachable at all: adopting an orphaned terminal re-keys + // `tabsByWorktree` onto the canonical worktree id and leaves the sleeping records that named the + // old one untouched (workspace-session-worktree-id.ts). A provider session id names one + // transcript, so the live pane owns it wherever it sits; resuming here forks the agent the user + // is watching. `done` is the cell that had no cover: a finished turn on a still-live pane. + // The same-workspace half of the same rule, pinned here so this file covers both cells whether or + // not #19736 (which fixes this one in `activeOrQueuedResumeClaimsProviderSession` too) has landed. + it('does not fork a provider session a live pane in this workspace already finished a turn on', () => { + const paneKey = makePaneKey('tab-1', LEAF_ID) + const peerPaneKey = makePaneKey('tab-peer', OTHER_LEAF_ID) + const record = makeRecord(paneKey) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + tabsByWorktree: { 'wt-1': [makeTerminalTab('tab-peer')] }, + terminalLayoutsByTabId: { + 'tab-peer': { + root: { type: 'leaf', leafId: OTHER_LEAF_ID }, + activeLeafId: OTHER_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF_ID]: 'pty-peer' } + } + }, + ptyIdsByTabId: { 'tab-peer': ['pty-peer'] }, + sleepingAgentSessionsByPaneKey: { [paneKey]: record }, + agentStatusByPaneKey: { + [peerPaneKey]: { ...makeWorkingStatus(peerPaneKey, 'tab-peer', record), state: 'done' } + } + } as never) + + expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(0) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() + }) + + it('does not fork a provider session a live pane in another workspace is running', () => { + const paneKey = makePaneKey('tab-1', LEAF_ID) + const peerPaneKey = makePaneKey('tab-peer', OTHER_LEAF_ID) + const record = makeRecord(paneKey) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + // The record's own pane is gone, so nothing local can own its recovery. + tabsByWorktree: { 'wt-1': [], 'wt-2': [makeTerminalTab('tab-peer')] }, + terminalLayoutsByTabId: { + 'tab-peer': { + root: { type: 'leaf', leafId: OTHER_LEAF_ID }, + activeLeafId: OTHER_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF_ID]: 'pty-peer' } + } + }, + ptyIdsByTabId: { 'tab-peer': ['pty-peer'] }, + sleepingAgentSessionsByPaneKey: { [paneKey]: record }, + agentStatusByPaneKey: { + [peerPaneKey]: { + ...makeWorkingStatus(peerPaneKey, 'tab-peer', record), + worktreeId: 'wt-2', + state: 'done' + } + } + } as never) + + expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(0) + + const state = useAppStore.getState() + expect(state.tabsByWorktree['wt-1']).toHaveLength(0) + expect(state.sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() + }) + + // The same peer without a live PTY is history, not a claim: the session must still come back. + it('still resumes when the other workspace peer finished and holds no live PTY', () => { + const paneKey = makePaneKey('tab-1', LEAF_ID) + const peerPaneKey = makePaneKey('tab-peer', OTHER_LEAF_ID) + const record = makeRecord(paneKey) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + tabsByWorktree: { 'wt-1': [], 'wt-2': [makeTerminalTab('tab-peer')] }, + terminalLayoutsByTabId: { + 'tab-peer': { + root: { type: 'leaf', leafId: OTHER_LEAF_ID }, + activeLeafId: OTHER_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF_ID]: 'pty-peer' } + } + }, + ptyIdsByTabId: {}, + sleepingAgentSessionsByPaneKey: { [paneKey]: record }, + agentStatusByPaneKey: { + [peerPaneKey]: { + ...makeWorkingStatus(peerPaneKey, 'tab-peer', record), + worktreeId: 'wt-2', + state: 'done' + } + } + } as never) + + expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(1) + }) }) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index de0163bd0b6..9b9279b4f0b 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -4,10 +4,12 @@ import { type SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' import { AGENT_STATUS_STALE_AFTER_MS } from '../../../shared/agent-status-types' +import { parsePaneKey } from '../../../shared/stable-pane-id' import { getProviderSessionClaimKey, isPassiveCompletedHibernationEvidence, - recordPaneIsOwnedByPreservedPane + recordPaneIsOwnedByPreservedPane, + stablePaneHasLivePty } from './sleeping-agent-pane-ownership' import { launchSleepingAgentSession, @@ -100,12 +102,37 @@ function activeOrQueuedResumeClaimsProviderSession( if (samePaneOwnsRecovery && entry.paneKey === record.paneKey) { continue } + const tabId = getAgentStatusTabId(entry) + const pane = parsePaneKey(entry.paneKey) + if ( + entry.agentType !== record.agent || + !agentProviderSessionsEqual(record.agent, entry.providerSession, record.providerSession) + ) { + continue + } + // Why this arm carries no workspace scope: a provider session id names one transcript, so a + // pane whose exact PTY is live right now already owns it wherever that pane happens to sit, and + // resuming forks the agent the user is watching. The scoped arm below still needs its scope — + // a status row with no live PTY is a claim about the past. The two ids do drift: adopting an + // orphaned terminal re-keys `tabsByWorktree` without re-keying the sleeping records that name + // the old id (workspace-session-worktree-id.ts), and a completed turn on a live pane is exactly + // where the drift stops being caught. + if ( + pane && + tabId === pane.tabId && + stablePaneHasLivePty( + pane.tabId, + pane.leafId, + state.ptyIdsByTabId, + state.terminalLayoutsByTabId[pane.tabId] + ) + ) { + return true + } if ( - worktreeTabIds.has(getAgentStatusTabId(entry) ?? '') && - entry.worktreeId === record.worktreeId && - entry.agentType === record.agent && entry.state !== 'done' && - agentProviderSessionsEqual(record.agent, entry.providerSession, record.providerSession) + worktreeTabIds.has(tabId ?? '') && + entry.worktreeId === record.worktreeId ) { return true } diff --git a/src/renderer/src/lib/sleeping-agent-pane-ownership.ts b/src/renderer/src/lib/sleeping-agent-pane-ownership.ts index 95a6c988b31..8dac858af02 100644 --- a/src/renderer/src/lib/sleeping-agent-pane-ownership.ts +++ b/src/renderer/src/lib/sleeping-agent-pane-ownership.ts @@ -94,7 +94,7 @@ function hasRestorableStablePanePty( // the pane that reconnects on activation. Liveness comes from the runtime // live-PTY map (ptyIdsByTabId), not the layout's ptyIdsByLeafId snapshot, which // persists stale across sleep/restart. -function stablePaneHasLivePty( +export function stablePaneHasLivePty( tabId: string, leafId: string, ptyIdsByTabId: Record, From a6e6bbfe5c7df5005a718a10eb16061461cb3ccf Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:25:48 -0700 Subject: [PATCH 06/21] fix(runtime): isolate one pane's replay from the handle-gap drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One store write releases every due pane, and the drain runs synchronously inside a zustand subscriber. `waiter.run()` was unguarded, so a single pane's replay reached two things it has no business touching: - the throw escapes out of `useAppStore.setState`, meaning the mirror apply that published the PTY handle throws at its own call site; - every pane queued behind the thrower is stranded — waiter still parked, deadline still armed — and then decides on a connection whose evidence landed long ago. The deadline path fans out the same way, so a throwing replay also escaped the timer callback. Reachable: `resumeSleepingAgentSessionsForWorktree` reaches `state.createTab` with no guard of its own. The panes in a drain are strangers to each other and to the frame that released them; none of them should be able to see another's failure. The new tests live in their own file because host-mirror-handle-gap-resume.test.ts drives the waiter through the real resume sweep and so cannot choose what a replay DOES. Note for anyone extending that file: per its header, "did the waiter release" is not an observable here — a spurious release is re-parked immediately and reads identically one tick later. These tests assert on timer count and on the deadline instead. Also records two findings next to the code, so they are not rediscovered: `expiredGenerationByPane` is never pruned for a removed environment (bounded and inert, since removal advances the generation, but it does not drain — and a DIFFERENT leak in that same map is being fixed concurrently, so reconcile rather than patch around it); and sustained reconnect churn holding a pane parked indefinitely is CORRECT, not the latch-that-never-releases defect, because under churn liveness genuinely is unverifiable and ssh-execution-boundary.md forbids resolving that to `exited`. It has the shape of the defect and will eventually be "fixed" by someone who does not know that. Mutation: dropping the guard kills exactly the three new assertions and leaves all twelve existing waiter tests passing. --- .../lib/host-mirror-handle-gap-drain.test.ts | 112 ++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 27 ++++- 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts new file mode 100644 index 00000000000..246a72147ca --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts @@ -0,0 +1,112 @@ +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, + hasHostMirrorHandleWaitExpired, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +// What this file pins, and why it is separate from host-mirror-handle-gap-resume.test.ts: that file +// drives the waiter through the real resume sweep, so it cannot choose what a replay DOES. These +// tests park with a `run` of their own to exercise the drain itself — the loop that releases every +// due pane from one store write, running synchronously inside a zustand subscriber. The panes in +// that loop are strangers to each other and the store write that triggered it is a stranger to all +// of them, so one pane's replay must not be able to reach either. + +const ENVIRONMENT_ID = 'env-handle-gap-drain' +const WORKTREE_ID = 'repo-1::/workspace/repo' +const FIRST_TAB_ID = 'web-terminal-host-tab-1' +const SECOND_TAB_ID = 'web-terminal-host-tab-2' + +const initialAppStoreState = useAppStore.getState() + +function seedRows(): void { + useAppStore.setState({ + ptyIdsByTabId: {}, + tabsByWorktree: { + [WORKTREE_ID]: [ + { id: FIRST_TAB_ID, title: 'one' }, + { id: SECOND_TAB_ID, title: 'two' } + ] + } + } as never) +} + +/** The host publishes both panes' PTY handles on one frame: both waiters come due together. */ +function publishBothHandles(): void { + useAppStore.setState({ + ptyIdsByTabId: { [FIRST_TAB_ID]: ['pty-1'], [SECOND_TAB_ID]: ['pty-2'] } + } as never) +} + +describe('host-mirror handle-gap drain', () => { + beforeEach(() => { + vi.useFakeTimers() + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + seedRows() + }) + + afterEach(() => { + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + }) + + // The drain runs inside `useAppStore.subscribe`, so an unguarded throw from one pane's replay + // leaves the store write that published the handle throwing at its own call site — the mirror + // apply path, which has nothing to do with this pane. `resumeSleepingAgentSessionsForWorktree` + // reaches `state.createTab` with no guard of its own, so the throw is reachable. + it('does not let one pane’s replay throw out of the store write that released it', () => { + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, FIRST_TAB_ID, () => { + throw new Error('replay blew up') + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, () => {}) + + expect(() => publishBothHandles()).not.toThrow() + }) + + // Same write, the other victim: the panes in a drain are strangers. A replay that throws must not + // strand every pane queued behind it — a stranded pane holds its park until its own deadline and + // then decides on a connection whose evidence has long since landed. + it('releases every other due pane when one pane’s replay throws', () => { + const secondReplay = vi.fn() + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, FIRST_TAB_ID, () => { + throw new Error('replay blew up') + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, secondReplay) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(2) + + try { + publishBothHandles() + } catch { + // The assertion is about the second pane, not about who swallowed the throw. + } + + expect(secondReplay).toHaveBeenCalledTimes(1) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + // Both deadlines are cancelled, so neither pane can record an expiry it did not earn. + expect(vi.getTimerCount()).toBe(0) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS * 2) + expect(hasHostMirrorHandleWaitExpired(ENVIRONMENT_ID, FIRST_TAB_ID)).toBe(false) + expect(hasHostMirrorHandleWaitExpired(ENVIRONMENT_ID, SECOND_TAB_ID)).toBe(false) + }) + + // The deadline path fans out the same way: one expiring pane's replay must not keep another pane + // from recording its own verdict on the same connection. + it('records the expiry of a pane whose replay throws and still frees the pane', () => { + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, FIRST_TAB_ID, () => { + throw new Error('replay blew up') + }) + + expect(() => vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS)).not.toThrow() + + expect(hasHostMirrorHandleWaitExpired(ENVIRONMENT_ID, FIRST_TAB_ID)).toBe(true) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + 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 9582dcc9d6b..b38fd2f4110 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -20,6 +20,12 @@ import { WEB_SESSION_TAB_RPC_TIMEOUT_MS } from '@/runtime/web-session-tab-rpc-ti * 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. + * + * Sustained reconnect churn can therefore hold a pane parked indefinitely: each reconnect voids the + * in-flight verdict and grants a fresh full budget. That is CORRECT, not the defect above. Under + * churn the pane's liveness genuinely is unverifiable, and `docs/reference/ssh-execution-boundary.md` + * forbids resolving unverifiable to `exited`. It has the shape of a latch that never releases, so + * do not "fix" it by letting a verdict from one connection decide another — that is #19735. */ export const HOST_MIRROR_HANDLE_GAP_DEADLINE_MS = WEB_SESSION_TAB_RPC_TIMEOUT_MS @@ -38,7 +44,16 @@ type HandleGapStoreState = Pick< > const waitersByPane = new Map() -/** Connection generation whose wait already expired for the pane. */ +/** + * Connection generation whose wait already expired for the pane. + * + * KNOWN LEAK, not fixed: entries are pruned only by `recordExpiredWait`, and only for the + * environment doing the recording. An environment that is removed and never expires another pane + * keeps its rows for the life of the session. Bounded by panes x environments and inert — a stale + * row cannot match, because removing an environment advances its connection generation — but it + * does not drain. Another agent has a separate fix in flight for a DIFFERENT leak in this same map + * (pruning on tab death); reconcile with that change rather than patching around it. + */ const expiredGenerationByPane = new Map() let unsubscribeStore: (() => void) | null = null @@ -82,7 +97,15 @@ function releaseWaiter(key: string): void { clearTimeout(waiter.deadline) waitersByPane.delete(key) stopStoreSubscriptionIfIdle() - waiter.run() + try { + waiter.run() + } catch (error) { + // Why: one write releases every due pane, and the drain runs inside the store subscriber. The + // panes in it are strangers to each other and to the frame that published the handle, so an + // unguarded replay throw both strands every pane queued behind it and surfaces at the mirror + // apply's own `setState`. The pane is already unparked here; only its replay is lost. + console.warn('[host-mirror-handle-gap] parked resume replay failed:', error) + } } function waiterIsReleased(waiter: HandleGapWaiter, state: HandleGapStoreState): boolean { From b33b48e2c426eb12adedb1af90984bd5ab137d25 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:44:39 -0700 Subject: [PATCH 07/21] fix(runtime): drain a removed environment's handle-gap verdicts on teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `expiredGenerationByPane` is pruned only by rules that run when a verdict is RECORDED — the stale-generation sweep here, and the tab-death sweep added separately (8f166411306, env-scoped in c0e44238eaa). An environment that is REMOVED records nothing ever again, so neither rule can reach its rows and they survive for the life of the session. Two orphan classes on one map; neither prune subsumes the other, because both are driven by a recording. Severity is a leak, not a correctness bug, and the commit pins WHY so nobody re-derives it: removing an environment advances its connection generation, so a stranded verdict can never match again even if the id returns. That test exists to stop the generation advance being "optimised" away later, since it is the only thing making the stranded row inert. Hung off `clearWebSessionTabsTrackingForEnvironment` because that is the only caller that fires for an environment that is going away. Clears VERDICTS ONLY. Parked waiters deliberately survive, matching `clearHostSessionMirrorHydration`: a re-pair or effect restart replaces the connection's evidence, it does not cancel the recovery this client still owes the pane. A waiter left behind is bounded by its own deadline and replays its sweep exactly as it would have. Clearing them here would silently drop a parked resume that nothing else replays. A measurement worth recording, because it argued me out of a change I was about to make: on the unfixed map the per-expiry rescan is super-linear — 500/1000/ 2000/4000 sequential expiries cost 7.2/15.3/51.8/173.1 ms, doubling ratios converging on ~3.35 against 4.0 for quadratic. That looked like a case for reshaping the map to `Map}>`. It is not: the quadratic is a property of the LEAK, not of the scan. Once the tab-death prune holds the map at roughly one entry per environment the scan is over ~1 entry, and a counting probe on the fixed map (summing `map.size` across N expiries, which IS the iteration count and needs no clock) gives exactly N-1 — linear, and 2000x fewer iterations than quadratic at N=4000. The flat prefix loop used here is the established pattern in this subsystem and needs no restructure. Two methodology traps this cost, recorded for the next person measuring in this repo: `vi.useFakeTimers()` fakes `process.hrtime` and `performance.now` as well, so a timing harness reports the advanced deadline rather than work done — fake only the timer surface under test. And expiring N panes in one burst measures the fake-timer harness clearing N timers, not product code; 1000 panes "cost" ~1s that way and almost none of it was ours. Mutations: a clear that drops nothing kills exactly the two assertions that claim it drains, and correctly leaves the waiter-survival and generation-advance tests passing. An UNSCOPED clear kills the same two, via their sibling- environment half. --- .../host-mirror-handle-gap-teardown.test.ts | 109 ++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 34 +++++- .../tracking-lifecycle.ts | 2 + 3 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts new file mode 100644 index 00000000000..ce6a141f017 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore } from '@/store' +import { + clearRuntimeEnvironmentConnectionGenerationsForTests, + setRuntimeEnvironmentConnectionGenerationForTests +} from '@/store/slices/runtime-status' +import { clearWebSessionTabsTrackingForEnvironment } from '@/runtime/web-session-tabs-sync/tracking-lifecycle' +import { + HOST_MIRROR_HANDLE_GAP_DEADLINE_MS, + clearHostMirrorHandleGapVerdictsForEnvironment, + countHostMirrorHandleGapVerdictsForTests, + countParkedHostMirrorHandleGapPanesForTests, + hasHostMirrorHandleWaitExpired, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +// The orphan class no recording-driven prune can reach. Both existing rules — stale generation and +// tab death — run only when a verdict is RECORDED, so an environment that is removed and never +// expires another pane keeps its rows for the life of the session. + +const ENVIRONMENT_ID = 'env-torn-down' +const OTHER_ENVIRONMENT_ID = 'env-survivor' +const WORKTREE_ID = 'repo-1::/workspace/repo' + +const initialAppStoreState = useAppStore.getState() + +function parkAndExpire(environmentId: string, tabId: string): void { + useAppStore.setState({ + ptyIdsByTabId: {}, + tabsByWorktree: { [WORKTREE_ID]: [{ id: tabId, title: tabId }] } + } as never) + parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {}) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) +} + +describe('host-mirror handle-gap verdicts across environment teardown', () => { + beforeEach(() => { + vi.useFakeTimers() + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + }) + + afterEach(() => { + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + }) + + it('drops the torn-down environment’s verdicts and keeps every other environment’s', () => { + parkAndExpire(ENVIRONMENT_ID, 'web-terminal-host-tab-1') + parkAndExpire(ENVIRONMENT_ID, 'web-terminal-host-tab-2') + parkAndExpire(OTHER_ENVIRONMENT_ID, 'web-terminal-host-tab-3') + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(3) + + clearHostMirrorHandleGapVerdictsForEnvironment(ENVIRONMENT_ID) + + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(1) + expect(hasHostMirrorHandleWaitExpired(OTHER_ENVIRONMENT_ID, 'web-terminal-host-tab-3')).toBe( + true + ) + }) + + // Matches `clearHostSessionMirrorHydration`: a re-pair replaces the connection's evidence, it + // does not cancel the recovery this client still owes the pane. Clearing the waiter here would + // silently drop a parked resume sweep that nothing else will replay. + it('leaves a parked waiter alone, cancelling only the verdicts', () => { + const replay = vi.fn() + useAppStore.setState({ + ptyIdsByTabId: {}, + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'web-terminal-host-tab-9', title: 'nine' }] } + } as never) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, 'web-terminal-host-tab-9', replay) + + clearHostMirrorHandleGapVerdictsForEnvironment(ENVIRONMENT_ID) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + expect(replay).toHaveBeenCalledTimes(1) + }) + + // The live wiring: session-tabs tracking teardown is the only caller that fires for an + // environment that is going away, so the hook has to hang off it or the rows never drain. + it('drains through the session-tabs tracking teardown for the environment', () => { + parkAndExpire(ENVIRONMENT_ID, 'web-terminal-host-tab-1') + parkAndExpire(OTHER_ENVIRONMENT_ID, 'web-terminal-host-tab-3') + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(2) + + clearWebSessionTabsTrackingForEnvironment(ENVIRONMENT_ID) + + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(1) + expect(hasHostMirrorHandleWaitExpired(OTHER_ENVIRONMENT_ID, 'web-terminal-host-tab-3')).toBe( + true + ) + }) + + // Why the stranded row was inert rather than dangerous, pinned so nobody "optimises" the + // generation advance away: removing an environment advances its connection generation, so a + // verdict left behind can never match again even if the id returns. + it('cannot match again after the environment returns on a new generation', () => { + parkAndExpire(ENVIRONMENT_ID, 'web-terminal-host-tab-1') + expect(hasHostMirrorHandleWaitExpired(ENVIRONMENT_ID, 'web-terminal-host-tab-1')).toBe(true) + + setRuntimeEnvironmentConnectionGenerationForTests(ENVIRONMENT_ID, 1) + + expect(hasHostMirrorHandleWaitExpired(ENVIRONMENT_ID, 'web-terminal-host-tab-1')).toBe(false) + }) +}) 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 b38fd2f4110..cc7067b9d76 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -47,12 +47,13 @@ const waitersByPane = new Map() /** * Connection generation whose wait already expired for the pane. * - * KNOWN LEAK, not fixed: entries are pruned only by `recordExpiredWait`, and only for the - * environment doing the recording. An environment that is removed and never expires another pane - * keeps its rows for the life of the session. Bounded by panes x environments and inert — a stale - * row cannot match, because removing an environment advances its connection generation — but it - * does not drain. Another agent has a separate fix in flight for a DIFFERENT leak in this same map - * (pruning on tab death); reconcile with that change rather than patching around it. + * Two rules drain it, and neither subsumes the other because both are driven by a recording: + * `recordExpiredWait` drops rows from a superseded generation, and a separate rule (8f166411306) + * drops rows whose tab is no longer published, both scoped to the environment doing the recording. + * An environment that is REMOVED records nothing ever again, so neither rule can reach it — hence + * the teardown clear below, which is the only thing that can. A stranded row is inert (removing an + * environment advances its connection generation, so it can never match again); this is a leak + * fix, not a correctness one. */ const expiredGenerationByPane = new Map() let unsubscribeStore: (() => void) | null = null @@ -189,6 +190,27 @@ export function countParkedHostMirrorHandleGapPanesForTests(): number { return waitersByPane.size } +/** + * Drops the verdicts an environment's teardown makes unreachable. + * + * Only the verdicts. Parked waiters deliberately survive, matching + * `clearHostSessionMirrorHydration`: a re-pair or effect restart replaces the connection's + * evidence, it does not cancel the recovery this client still owes the pane. A waiter left here is + * bounded by its own deadline and replays the sweep exactly as it would have. + */ +export function clearHostMirrorHandleGapVerdictsForEnvironment(environmentId: string): void { + const prefix = `${environmentId}\0` + for (const key of expiredGenerationByPane.keys()) { + if (key.startsWith(prefix)) { + expiredGenerationByPane.delete(key) + } + } +} + +export function countHostMirrorHandleGapVerdictsForTests(): number { + return expiredGenerationByPane.size +} + export function resetHostMirrorHandleGapWaitsForTests(): void { for (const waiter of waitersByPane.values()) { clearTimeout(waiter.deadline) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts index 8b07fde7a33..11fe86d5107 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts @@ -38,6 +38,7 @@ import { clearWebSessionTerminalPlacementsForEnvironment } from '../web-session-terminal-placement' import { clearHostSessionMirrorHydration } from '../host-session-mirror-hydration' +import { clearHostMirrorHandleGapVerdictsForEnvironment } from '@/lib/host-mirror-handle-gap-wait' import { clearHostSessionTabIdMappings } from './tracking-mappings' import { sessionTabsFreshnessKey, @@ -218,6 +219,7 @@ export function clearWebSessionTabsTrackingForEnvironment(environmentId: string) clearWebSessionBrowserPlacementsForEnvironment(trimmedEnvironmentId) clearWebSessionTerminalPlacementsForEnvironment(trimmedEnvironmentId) clearHostSessionMirrorHydration(trimmedEnvironmentId) + clearHostMirrorHandleGapVerdictsForEnvironment(trimmedEnvironmentId) clearAllWebRuntimeWakeTerminalRespawn() } From 453220c80110fef66a5810b854dd73be0b787d3d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:54:10 -0700 Subject: [PATCH 08/21] fix(runtime): reconcile three branches' handle-gap verdict rules into one loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three agents changed `recordExpiredWait` on three branches and each verified only their own. This is the union, resolved into the agreed shape and proved on one tree. The rules are NOT alternatives — they have different safety properties, and flattening them to one scope is wrong in both directions. Both wrong shapes were independently written before this was reconciled, so the comments say why. GENERATION rule, per key across EVERY environment (adv2-skew's class). `hasHostMirrorHandleWaitExpired` compares a row against its own environment's CURRENT generation, so a row whose generation has moved can never return true for anybody; retiring it cannot cost a reader a verdict, whoever owns it. Scoped to the recording environment, an environment that reconnects and then goes quiet strands its rows forever. TAB-DEATH rule, recording environment ONLY (my class). Row absence is transient where a generation is not: a sibling mid-republish has no rows for a frame and would lose a verdict its pane still needs — reproduced before it was narrowed. Teardown drain (adv2-races' class) is unchanged and orthogonal: it is the only trigger that fires for a REMOVED environment, whose rows no rule above reaches because such an environment records no further verdict. Right predicate, wrong trigger. The union suite proves all four orphan classes simultaneously, plus the two properties none of the three rules may break: the verdict stays sticky enough to break the park/expire/replay loop, and no rule evicts a verdict a live pane still needs. It uses three environments throughout, because with two at one generation the candidate rules are indistinguishable and the naive fix survives. THE FOURTH CLASS IS UNOWNED AND ASSERTED AS A HAZARD. A retracted tab id that is republished inherits the old pane's verdict and skips its own wait. Unlike every other gap on this map it is NOT conservative: the others drop a verdict and re-park, holding longer, while this one retains a verdict and resumes on a handle that has not landed — the #19735 direction. No rule reaches it: the tab-death predicate stops matching once the id is republished, the teardown drain fires on environment teardown rather than tab retraction, and no waiter exists to observe the retraction because a pane holding a verdict never parks. Closing it needs a fourth trigger, on row retraction. The suite pins the current behaviour so it cannot be quietly forgotten. Union finding, recorded rather than merged: adv2-skew's `docs(relay): the live-broker wait budget does not bound the call` (6b029820cc9) is SKIPPED here. It documents the unbounded wait, and adv2-concurrency-fixes (1673716c6d5) fixed exactly that by extracting the loop into relay-live-broker-wait.ts. The doc and its test pin behaviour the union no longer has. This is the kind of interaction neither branch could see alone. --- ...st-mirror-handle-gap-verdict-union.test.ts | 167 ++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 34 +++- 2 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts new file mode 100644 index 00000000000..c0d9479a20b --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts @@ -0,0 +1,167 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore, type AppState } from '@/store' +import { + clearRuntimeEnvironmentConnectionGenerationsForTests, + setRuntimeEnvironmentConnectionGenerationForTests +} from '@/store/slices/runtime-status' +import { + HOST_MIRROR_HANDLE_GAP_DEADLINE_MS, + clearHostMirrorHandleGapVerdictsForEnvironment, + countHostMirrorHandleGapVerdictsForTests, + hasHostMirrorHandleWaitExpired, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +/** + * The UNION suite for `expiredGenerationByPane`. + * + * Three agents changed this one map on three branches and each verified only their own. These + * cases exist because nothing else proves the rules compose: individually-correct rules whose + * interaction nobody tested is the exact failure this was looking for. + * + * Four orphan classes, and what covers each: + * A tab churn on a LIVE environment tab-death rule, recording environment only + * B REMOVED environment clearHostMirrorHandleGapVerdictsForEnvironment + * C cross-environment QUIESCENCE generation rule, per key, every environment + * D REUSED tab id NOTHING. Pinned below as a live hazard. + * + * Plus the two properties no rule may break: the verdict stays sticky enough to break the + * park/expire/replay loop, and no rule evicts a verdict a live pane still needs. + */ + +const ENV_A = 'env-union-a' +const ENV_B = 'env-union-b' +const ENV_C = 'env-union-c' +const WORKTREE = 'repo-1::wt-union' +const initialAppStoreState = useAppStore.getState() + +function setLiveTabs(tabIds: string[]): void { + useAppStore.setState({ + tabsByWorktree: { [WORKTREE]: tabIds.map((id) => ({ id, title: id, ptyId: null })) }, + ptyIdsByTabId: {} + } as unknown as AppState) +} + +function parkAndExpire(environmentId: string, tabId: string): void { + parkUntilHostMirrorHandleLands(environmentId, WORKTREE, tabId, () => {}) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1) +} + +describe('handle-gap verdict map, all rules on one tree', () => { + beforeEach(() => { + vi.useFakeTimers() + useAppStore.setState(initialAppStoreState, true) + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + }) + + afterEach(() => { + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + vi.useRealTimers() + }) + + it('handles all four orphan classes simultaneously', () => { + for (const environmentId of [ENV_A, ENV_B, ENV_C]) { + setRuntimeEnvironmentConnectionGenerationForTests(environmentId, 1) + } + setLiveTabs(['a1', 'a2', 'b1', 'c1', 'reused']) + + // A: tab churn on a live environment. a1 expires, then its tab closes. + parkAndExpire(ENV_A, 'a1') + // B: a whole environment that will be removed. + parkAndExpire(ENV_B, 'b1') + // C: an environment that will reconnect and then never expire another pane. + parkAndExpire(ENV_C, 'c1') + // D: a tab id that will be retracted and republished under the same id. + parkAndExpire(ENV_A, 'reused') + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(4) + + // C reconnects and goes quiet. B's environment is removed outright. + setRuntimeEnvironmentConnectionGenerationForTests(ENV_C, 2) + clearHostMirrorHandleGapVerdictsForEnvironment(ENV_B) + + // A's tab closes; the reused id is retracted and republished as a DIFFERENT pane. + setLiveTabs(['a2', 'reused']) + parkAndExpire(ENV_A, 'a2') + + // A drained: a1's row is gone and env-a recorded again, so the tab-death rule swept it. + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false) + // B drained: by teardown, which is the only trigger that fires for a removed environment. + expect(hasHostMirrorHandleWaitExpired(ENV_B, 'b1')).toBe(false) + // C drained: env-a's expiry retired env-c's superseded row, though env-c never expired again. + expect(hasHostMirrorHandleWaitExpired(ENV_C, 'c1')).toBe(false) + + // D IS NOT DRAINED, and this assertion pins a LIVE HAZARD rather than a desired behaviour. + // The republished pane inherits the retracted pane's verdict and skips its own wait. + // + // Why this one is different from every other gap argued over on this map: the others DROP a + // verdict, so the pane re-parks and only ever holds longer. This one RETAINS a verdict and + // lets a fresh pane resume on a handle that has not landed — the #19735 direction itself. + // It is therefore the one gap here that is not conservative. + // + // No rule reaches it, and each for its own reason: the tab-death rule's predicate stops + // matching the moment the id is republished, so it is not even eventually consistent; the + // teardown drain fires on environment teardown, not on tab retraction inside a live one; and + // no waiter exists to observe the retraction, because a pane holding a verdict never parks + // (`findUnhydratedHostMirrorForPane` returns null on it). Closing it needs a fourth trigger, + // on row retraction. DO NOT delete this case when a prune for dead tabs lands — "a prune for + // dead tabs shipped" is exactly the plausible assumption that would delete it. + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'reused')).toBe(true) + + // Only the two live verdicts survive: a2's and the stranded reused-id row. + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(2) + }) + + it('keeps a verdict sticky enough to break the park/expire/replay loop', () => { + // The verdict exists to stop a pane re-parking forever. If any rule evicted it while the pane + // is live and its connection current, the wait would rearm on a fresh budget every replay. + setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1) + setLiveTabs(['a1']) + parkAndExpire(ENV_A, 'a1') + + for (let replay = 0; replay < 20; replay += 1) { + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true) + parkAndExpire(ENV_A, 'a1') + } + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true) + }) + + it('never evicts a live pane verdict, whichever environment sweeps', () => { + // Three environments on purpose: with two at one generation the candidate rules are + // indistinguishable and the naive "judge everything against the recording environment" + // mutation survives. env-c is the discriminator — its verdict is live. + for (const environmentId of [ENV_A, ENV_B, ENV_C]) { + setRuntimeEnvironmentConnectionGenerationForTests(environmentId, 1) + } + setLiveTabs(['a1', 'a2', 'b1', 'c1']) + parkAndExpire(ENV_A, 'a1') + parkAndExpire(ENV_B, 'b1') + parkAndExpire(ENV_C, 'c1') + + // env-b is briefly rowless mid-rehydration while env-a sweeps. Row absence is transient, so + // this must not be read as retraction for an environment other than the one recording. + setLiveTabs(['a1', 'a2', 'c1']) + parkAndExpire(ENV_A, 'a2') + setLiveTabs(['a1', 'a2', 'b1', 'c1']) + + expect(hasHostMirrorHandleWaitExpired(ENV_B, 'b1')).toBe(true) + expect(hasHostMirrorHandleWaitExpired(ENV_C, 'c1')).toBe(true) + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true) + }) + + it('returns to baseline under churn across all three drains', () => { + for (let round = 0; round < 300; round += 1) { + const environmentId = [ENV_A, ENV_B, ENV_C][round % 3]! + setRuntimeEnvironmentConnectionGenerationForTests(environmentId, round + 1) + setLiveTabs([`tab-${round}`]) + parkAndExpire(environmentId, `tab-${round}`) + } + for (const environmentId of [ENV_A, ENV_B, ENV_C]) { + clearHostMirrorHandleGapVerdictsForEnvironment(environmentId) + } + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(0) + 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 cc7067b9d76..7e9c75d3171 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -70,13 +70,41 @@ export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: str ) } +function liveTabIds(): Set { + const tabIds = new Set() + for (const tabs of Object.values(useAppStore.getState().tabsByWorktree)) { + for (const tab of tabs) { + tabIds.add(tab.id) + } + } + return tabIds +} + function recordExpiredWait(environmentId: string, key: string): void { const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) - // Why: a verdict from a previous connection is dead weight; drop it so the map - // stays bounded by the panes parked on the current connection. + // TWO rules with DIFFERENT scopes, deliberately. Flattening them to one scope is wrong either + // way round, and both wrong shapes were independently written before this was reconciled. const prefix = `${environmentId}\0` + const liveTabs = liveTabIds() for (const [staleKey, staleGeneration] of expiredGenerationByPane) { - if (staleKey.startsWith(prefix) && staleGeneration !== generation) { + // GENERATION, judged per key across EVERY environment. `hasHostMirrorHandleWaitExpired` + // compares a row against its own environment's CURRENT generation, so a row whose generation + // has moved can never return true for anyone. Retiring it cannot cost a reader a verdict, + // whoever owns it. Scoped to the recording environment, an environment that reconnects and + // then goes quiet strands its rows forever. + const staleEnvironmentId = staleKey.slice(0, staleKey.indexOf('\0')) + if (staleGeneration !== getRuntimeEnvironmentConnectionGeneration(staleEnvironmentId)) { + expiredGenerationByPane.delete(staleKey) + continue + } + // TAB DEATH, this environment ONLY. Unlike a generation, row absence is transient: a sibling + // mid-republish has no rows for a frame and would lose a verdict its pane still needs. What + // licenses the inference here is that the recording pane's own row is published right now — + // the deadline only records while its waiter is parked — which establishes that THIS + // environment has a published row. It does not establish that it has finished republishing, + // so do not widen this further: a host that has published p1 but not yet p2 can still cost p2 + // its verdict. That residual is conservative — drop, re-park, hold longer, never resume early. + if (staleKey.startsWith(prefix) && !liveTabs.has(staleKey.slice(prefix.length))) { expiredGenerationByPane.delete(staleKey) } } From 070bbb4a487cd3e51fad0055ab59bb15a135cf19 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 23:53:48 -0700 Subject: [PATCH 09/21] fix(test): repair the teardown suite the union broke Cherry-picked from 46ad377ceb9 with the relay half dropped: that commit also repaired relay-concurrency-policy-flip-mid-mint.test.ts, which does not exist on this PR and belongs with the relay cluster's own branch. The handle-gap half is what this PR needs. Neither break was visible on its own branch -- both only appear once the verdict rules compose. --- .../host-mirror-handle-gap-teardown.test.ts | 8 ++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 22 +++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts index ce6a141f017..7076af927f3 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts @@ -26,9 +26,15 @@ const WORKTREE_ID = 'repo-1::/workspace/repo' const initialAppStoreState = useAppStore.getState() function parkAndExpire(environmentId: string, tabId: string): void { + // Rows ACCUMULATE. Replacing them would unpublish the panes parked earlier, and the tab-death + // rule would then legitimately sweep their verdicts before teardown was ever reached — this + // suite is about a class no recording-driven prune can reach, so every pane here stays live. + const published = useAppStore.getState().tabsByWorktree[WORKTREE_ID] ?? [] useAppStore.setState({ ptyIdsByTabId: {}, - tabsByWorktree: { [WORKTREE_ID]: [{ id: tabId, title: tabId }] } + tabsByWorktree: { + [WORKTREE_ID]: [...published.filter((tab) => tab.id !== tabId), { id: tabId, title: tabId }] + } } as never) parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {}) vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) 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 7e9c75d3171..27be4efd91c 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -47,13 +47,21 @@ const waitersByPane = new Map() /** * Connection generation whose wait already expired for the pane. * - * Two rules drain it, and neither subsumes the other because both are driven by a recording: - * `recordExpiredWait` drops rows from a superseded generation, and a separate rule (8f166411306) - * drops rows whose tab is no longer published, both scoped to the environment doing the recording. - * An environment that is REMOVED records nothing ever again, so neither rule can reach it — hence - * the teardown clear below, which is the only thing that can. A stranded row is inert (removing an - * environment advances its connection generation, so it can never match again); this is a leak - * fix, not a correctness one. + * THREE drains, with three different triggers. Getting the scopes right is the whole design; see + * `recordExpiredWait` for why the first two must NOT share a scope. + * - superseded generation: per key, EVERY environment. Runs on any recording, anywhere. + * - dead tab row: the recording environment ONLY. Runs on a recording in that environment. + * - removed environment: `clearHostMirrorHandleGapVerdictsForEnvironment`, on teardown. The only + * trigger that fires at all for an environment that will never record again. A row stranded + * there is inert — removal advances the generation, so it can never match — so that one is a + * leak fix, not a correctness fix. + * + * ONE CLASS IS STILL UNCOVERED, and unlike the rest it is NOT conservative: a retracted tab id + * that is republished inherits the old pane's verdict and skips its own wait, which is the #19735 + * direction rather than a longer hold. No trigger above reaches it — the dead-row predicate stops + * matching once the id is live again, teardown is the wrong event, and a pane holding a verdict + * never parks, so no waiter observes the retraction. Closing it needs a fourth trigger, on row + * retraction. Pinned in host-mirror-handle-gap-verdict-union.test.ts; do not delete that case. */ const expiredGenerationByPane = new Map() let unsubscribeStore: (() => void) | null = null From 0096f25edbfd519b732ae292e4e40d95a35afcee Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:13:21 -0700 Subject: [PATCH 10/21] fix(runtime): a handle-gap verdict answers for its pane, not for the tab id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds adv2-skew's e8cac056d74 into the reconciled union. Closes the fourth orphan class, the only one that was not conservative: a retracted tab id republished as a different pane inherited the old pane's verdict and skipped its own wait — the #19735 direction rather than a longer hold. It needs no fourth trigger, which is why it composes with the three drains rather than competing with them. Every trigger those rules own fires downstream of the moment this hazard needs. The verdict instead carries the environment-minted PTY binding its pane held AT PARK TIME, and only answers for a pane that still holds it: a republished pane binds a newly minted PTY and serves its own wait, while a genuine reattach to the same PTY inherits, which is correct — the verdict follows the PTY, not the id. A transient rowless frame touches neither, so the read-time check is safe where a retraction-triggered prune would not have been. TWO MEASUREMENTS, both requested rather than assumed. 1. The record-then-release ordering is load-bearing and IS pinned. `recordExpiredWait` reads the waiter's park-time binding, so it must run before `releaseWaiter` deletes the entry. Swapping the two statements fails three cases, so the capture is not correct merely by accident of statement order. 2. The `''` fallback is a MATCH VALUE, not a null: two panes that both hold no environment-minted PTY compare equal and inherit, which is the same hazard in a narrower window. Measured unreachable through the production park path rather than assumed — the only route in is `kind: 'handle'`, which `findUnhydratedHostMirrorForPane` reports only when `tabHoldsEnvironmentPtyBinding` finds a binding, reading the SAME map through the SAME predicate as `paneBindingFor`. It now refuses to answer anyway. That coupling is two functions in two files with nothing enforcing it, refusing costs only a re-park, and the direction is conservative. THE REFUSAL IS WHAT FOUND THE REAL BUG. With `''` matching, any fixture that omits `terminalLayoutsByTabId` records `''`, compares `'' === ''`, and passes while the pane-identity check is entirely inert. Making it refuse turned that silence into four failures across host-mirror-handle-gap-drain and -teardown, whose fixtures seed no layout binding at all. Both now bind per environment — one shared environment id filters every other environment's pane back to `''` and restores the no-op. Mutation-tested on the merged tree: ignoring the binding fails case D and the mid-wait case; re-reading at expiry fails the mid-wait case and nothing else; letting `''` match fails the empty-binding case; widening the tab-death rule across environments still fails the live-verdict case, so pane identity does not weaken the scoping the sweep was reconciled around. Also fixes a real-clock race this branch introduced: the revoke-window test read `Date.now()` separately from `enqueue`'s own stamp, and under load the drift ate into the window. It now anchors the injected clock to the item's `createdAt`. --- .../lib/host-mirror-handle-gap-drain.test.ts | 17 +++ .../host-mirror-handle-gap-teardown.test.ts | 15 ++- ...st-mirror-handle-gap-verdict-union.test.ts | 105 ++++++++++++++---- .../src/lib/host-mirror-handle-gap-wait.ts | 69 ++++++++++-- 4 files changed, 175 insertions(+), 31 deletions(-) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts index 246a72147ca..d7864053eb6 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts @@ -24,6 +24,9 @@ const SECOND_TAB_ID = 'web-terminal-host-tab-2' const initialAppStoreState = useAppStore.getState() function seedRows(): void { + // Layout bindings are seeded because a verdict names the PANE by the environment-minted PTY it + // held at park time. A pane with no binding never reaches the park path in production, and its + // verdict deliberately refuses to answer, so a fixture without one models nothing real. useAppStore.setState({ ptyIdsByTabId: {}, tabsByWorktree: { @@ -31,6 +34,20 @@ function seedRows(): void { { id: FIRST_TAB_ID, title: 'one' }, { id: SECOND_TAB_ID, title: 'two' } ] + }, + terminalLayoutsByTabId: { + [FIRST_TAB_ID]: { + root: { type: 'leaf', leafId: 'leaf-1' }, + activeLeafId: 'leaf-1', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-1': `remote:${ENVIRONMENT_ID}@@term_1` } + }, + [SECOND_TAB_ID]: { + root: { type: 'leaf', leafId: 'leaf-2' }, + activeLeafId: 'leaf-2', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-2': `remote:${ENVIRONMENT_ID}@@term_2` } + } } } as never) } diff --git a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts index 7076af927f3..c25434f705c 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts @@ -29,11 +29,24 @@ function parkAndExpire(environmentId: string, tabId: string): void { // Rows ACCUMULATE. Replacing them would unpublish the panes parked earlier, and the tab-death // rule would then legitimately sweep their verdicts before teardown was ever reached — this // suite is about a class no recording-driven prune can reach, so every pane here stays live. - const published = useAppStore.getState().tabsByWorktree[WORKTREE_ID] ?? [] + const state = useAppStore.getState() + const published = state.tabsByWorktree[WORKTREE_ID] ?? [] useAppStore.setState({ ptyIdsByTabId: {}, tabsByWorktree: { [WORKTREE_ID]: [...published.filter((tab) => tab.id !== tabId), { id: tabId, title: tabId }] + }, + // A verdict names its PANE by the environment-minted PTY held at park time, so a fixture with + // no layout binding records '' and the verdict refuses to answer. Bind per environment: one + // shared environment id would filter to '' for every other environment's pane. + terminalLayoutsByTabId: { + ...state.terminalLayoutsByTabId, + [tabId]: { + root: { type: 'leaf', leafId: `leaf-${tabId}` }, + activeLeafId: `leaf-${tabId}`, + expandedLeafId: null, + ptyIdsByLeafId: { [`leaf-${tabId}`]: `remote:${environmentId}@@term_${tabId}` } + } } } as never) parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {}) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts index c0d9479a20b..449c8519435 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts @@ -24,10 +24,15 @@ import { * A tab churn on a LIVE environment tab-death rule, recording environment only * B REMOVED environment clearHostMirrorHandleGapVerdictsForEnvironment * C cross-environment QUIESCENCE generation rule, per key, every environment - * D REUSED tab id NOTHING. Pinned below as a live hazard. + * D REUSED tab id read-time pane identity, NOT a prune * - * Plus the two properties no rule may break: the verdict stays sticky enough to break the - * park/expire/replay loop, and no rule evicts a verdict a live pane still needs. + * D is the one that needed no new trigger: every trigger the other three own fires downstream of + * the moment it needs. The verdict instead carries the PTY binding its pane held AT PARK TIME and + * only answers for a pane that still holds it. + * + * Plus the properties no rule may break: the verdict stays sticky enough to break the + * park/expire/replay loop, a genuine reattach still inherits, and no rule evicts a verdict a live + * pane still needs. */ const ENV_A = 'env-union-a' @@ -36,9 +41,30 @@ const ENV_C = 'env-union-c' const WORKTREE = 'repo-1::wt-union' const initialAppStoreState = useAppStore.getState() -function setLiveTabs(tabIds: string[]): void { +/** Which environment minted each pane's PTY; the binding only counts for its own environment. */ +const ENV_OF_TAB: Record = { + a1: ENV_A, + a2: ENV_A, + reused: ENV_A, + b1: ENV_B, + c1: ENV_C +} + +/** Publishes rows AND the layout PTY binding each pane holds — the binding is the pane's identity. */ +function setLiveTabs(tabIds: string[], ptyByTabId: Record = {}): void { + const layouts: Record = {} + for (const id of tabIds) { + const ptyId = ptyByTabId[id] ?? `remote:${ENV_OF_TAB[id] ?? ENV_A}@@term_${id}` + layouts[id] = { + root: { type: 'leaf', leafId: `leaf-${id}` }, + activeLeafId: `leaf-${id}`, + expandedLeafId: null, + ptyIdsByLeafId: { [`leaf-${id}`]: ptyId } + } + } useAppStore.setState({ tabsByWorktree: { [WORKTREE]: tabIds.map((id) => ({ id, title: id, ptyId: null })) }, + terminalLayoutsByTabId: layouts, ptyIdsByTabId: {} } as unknown as AppState) } @@ -82,8 +108,9 @@ describe('handle-gap verdict map, all rules on one tree', () => { setRuntimeEnvironmentConnectionGenerationForTests(ENV_C, 2) clearHostMirrorHandleGapVerdictsForEnvironment(ENV_B) - // A's tab closes; the reused id is retracted and republished as a DIFFERENT pane. - setLiveTabs(['a2', 'reused']) + // A's tab closes; the reused id is retracted and republished as a DIFFERENT pane, which binds + // a PTY the host newly minted. That new binding is what makes it a different pane, not the id. + setLiveTabs(['a2', 'reused'], { reused: `remote:${ENV_A}@@term_freshly_minted` }) parkAndExpire(ENV_A, 'a2') // A drained: a1's row is gone and env-a recorded again, so the tab-death rule swept it. @@ -93,27 +120,61 @@ describe('handle-gap verdict map, all rules on one tree', () => { // C drained: env-a's expiry retired env-c's superseded row, though env-c never expired again. expect(hasHostMirrorHandleWaitExpired(ENV_C, 'c1')).toBe(false) - // D IS NOT DRAINED, and this assertion pins a LIVE HAZARD rather than a desired behaviour. - // The republished pane inherits the retracted pane's verdict and skips its own wait. - // - // Why this one is different from every other gap argued over on this map: the others DROP a - // verdict, so the pane re-parks and only ever holds longer. This one RETAINS a verdict and - // lets a fresh pane resume on a handle that has not landed — the #19735 direction itself. - // It is therefore the one gap here that is not conservative. - // - // No rule reaches it, and each for its own reason: the tab-death rule's predicate stops - // matching the moment the id is republished, so it is not even eventually consistent; the - // teardown drain fires on environment teardown, not on tab retraction inside a live one; and - // no waiter exists to observe the retraction, because a pane holding a verdict never parks - // (`findUnhydratedHostMirrorForPane` returns null on it). Closing it needs a fourth trigger, - // on row retraction. DO NOT delete this case when a prune for dead tabs lands — "a prune for - // dead tabs shipped" is exactly the plausible assumption that would delete it. - expect(hasHostMirrorHandleWaitExpired(ENV_A, 'reused')).toBe(true) + // D is closed, and NOT by a prune. No trigger any rule above owns fires at the right moment: + // the tab-death predicate stops matching once the id is live again, teardown is the wrong + // event, and no waiter observes the retraction because a pane holding a verdict never parks. + // It is closed at READ time instead — the verdict names the pane it was about, so a pane that + // binds a newly minted PTY does not answer to it and serves its own wait. + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'reused')).toBe(false) // Only the two live verdicts survive: a2's and the stranded reused-id row. expect(countHostMirrorHandleGapVerdictsForTests()).toBe(2) }) + it('answers for a genuine reattach that still holds the same PTY', () => { + // The verdict follows the PTY, not the tab id. A pane that reattaches to the SAME environment + // PTY is the same pane, so it must inherit — otherwise the identity check would have quietly + // removed the loop-breaker for every reattach. + setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1) + setLiveTabs(['a1']) + parkAndExpire(ENV_A, 'a1') + setLiveTabs([]) + setLiveTabs(['a1']) + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true) + }) + + it('records the binding the pane held at PARK time, not at expiry', () => { + // The mutation this kills: reading the binding inside `recordExpiredWait` from the store + // instead of from the waiter. A pane replaced mid-wait leaves the original waiter running to + // term, and an expiry-time read would attribute the verdict to whoever holds the id by then — + // handing the new pane a wait it never served. Three earlier cases all survived that bug; + // only rebinding BETWEEN park and expire distinguishes the two implementations. + setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1) + setLiveTabs(['a1']) + parkUntilHostMirrorHandleLands(ENV_A, WORKTREE, 'a1', () => {}) + setLiveTabs(['a1'], { a1: `remote:${ENV_A}@@term_replacement` }) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1) + + // The replacement pane never served this wait, so it must not inherit its verdict. + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false) + }) + + it('refuses to answer on an empty binding, which is a match value and not a null', () => { + // '' is what `paneBindingFor` returns when no leaf holds an environment-minted PTY. Two + // different panes both reading '' would compare EQUAL and inherit, which is the reused-tab-id + // shape again. Measured unreachable through the production park path rather than assumed: the + // only route into `parkUntilHostMirrorHandleLands` is `kind: 'handle'`, which + // `findUnhydratedHostMirrorForPane` reports only when `tabHoldsEnvironmentPtyBinding` + // (host-mirrored-pane-liveness.ts:28-31) finds a match — the SAME `terminalLayoutsByTabId` + // map through the SAME `parseRemoteRuntimePtyId` predicate `paneBindingFor` uses, so a pane + // that would bind '' never parks. It must still refuse rather than match, because that + // coupling is two functions in two files and nothing enforces it. + setRuntimeEnvironmentConnectionGenerationForTests(ENV_A, 1) + setLiveTabs(['a1'], { a1: 'remote:some-other-env@@term_1' }) + parkAndExpire(ENV_A, 'a1') + expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false) + }) + it('keeps a verdict sticky enough to break the park/expire/replay loop', () => { // The verdict exists to stop a pane re-parking forever. If any rule evicted it while the pane // is live and its connection current, the wait would rearm on a fresh budget every replay. 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 27be4efd91c..c3cd4af0d4d 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -1,6 +1,7 @@ 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' +import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id' /** * Per-pane park for the frame between a host's tab rows and its PTY handles. @@ -34,6 +35,8 @@ type HandleGapWaiter = { tabId: string /** Connection generation the wait was armed on; its verdict is void on any other. */ generation: number + /** Which PANE this wait is about, captured at park time; see ExpiredHandleGapVerdict. */ + paneBinding: string deadline: ReturnType run: () => void } @@ -63,18 +66,54 @@ const waitersByPane = new Map() * never parks, so no waiter observes the retraction. Closing it needs a fourth trigger, on row * retraction. Pinned in host-mirror-handle-gap-verdict-union.test.ts; do not delete that case. */ -const expiredGenerationByPane = new Map() +type ExpiredHandleGapVerdict = { + generation: number + /** Sorted environment-minted PTY ids the tab's leaves held AT PARK TIME; '' when none. */ + paneBinding: string +} +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. */ +/** + * The environment-minted PTY ids this tab's leaves are bound to, as one comparable string. + * + * Read from the layout, not `ptyIdsByTabId`: during the handle gap the published-handle map is + * empty by definition — that is the gap — while the layout binding is what + * `tabHoldsEnvironmentPtyBinding` already uses to call the pane unverifiable rather than dead. + */ +function paneBindingFor(tabId: string, environmentId: string): string { + const bindings = useAppStore.getState().terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {} + return Object.values(bindings) + .filter( + (ptyId): ptyId is string => + typeof ptyId === 'string' && parseRemoteRuntimePtyId(ptyId)?.environmentId === environmentId + ) + .sort() + .join('') +} + +/** True once the deadline fired for THIS pane on the current connection. */ export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: string): boolean { + const verdict = expiredGenerationByPane.get(paneWaitKey(environmentId, tabId)) + if (verdict === undefined || verdict.paneBinding === '') { + // Why '' never answers: it is a MATCH VALUE, not a null. Two different panes that both hold no + // environment-minted PTY compare equal, which is the reused-tab-id inheritance this check + // exists to stop, in a narrower window. Unreachable through the production park path — + // `findUnhydratedHostMirrorForPane` only reports `kind: 'handle'` when + // `tabHoldsEnvironmentPtyBinding` finds a binding, reading the same map through the same + // predicate as `paneBindingFor` — and pinned by the coupling test in + // host-mirror-handle-gap-verdict-union.test.ts. Refusing costs a re-park, which is the + // conservative direction, so the pair stays safe even if those two reads ever drift apart. + return false + } return ( - expiredGenerationByPane.get(paneWaitKey(environmentId, tabId)) === - getRuntimeEnvironmentConnectionGeneration(environmentId) + verdict.generation === getRuntimeEnvironmentConnectionGeneration(environmentId) && + // Why this and not the key alone: the key is a tab id, and the pane behind it can be replaced. + verdict.paneBinding === paneBindingFor(tabId, environmentId) ) } @@ -94,14 +133,14 @@ function recordExpiredWait(environmentId: string, key: string): void { // way round, and both wrong shapes were independently written before this was reconciled. const prefix = `${environmentId}\0` const liveTabs = liveTabIds() - for (const [staleKey, staleGeneration] of expiredGenerationByPane) { + for (const [staleKey, stale] of expiredGenerationByPane) { // GENERATION, judged per key across EVERY environment. `hasHostMirrorHandleWaitExpired` // compares a row against its own environment's CURRENT generation, so a row whose generation // has moved can never return true for anyone. Retiring it cannot cost a reader a verdict, // whoever owns it. Scoped to the recording environment, an environment that reconnects and // then goes quiet strands its rows forever. const staleEnvironmentId = staleKey.slice(0, staleKey.indexOf('\0')) - if (staleGeneration !== getRuntimeEnvironmentConnectionGeneration(staleEnvironmentId)) { + if (stale.generation !== getRuntimeEnvironmentConnectionGeneration(staleEnvironmentId)) { expiredGenerationByPane.delete(staleKey) continue } @@ -116,7 +155,14 @@ function recordExpiredWait(environmentId: string, key: string): void { expiredGenerationByPane.delete(staleKey) } } - expiredGenerationByPane.set(key, generation) + // Why the waiter's park-time binding and not a fresh read: this verdict is about the pane whose + // wait just ran out. Re-reading here would attribute it to whatever holds the id NOW, handing a + // pane that replaced it mid-wait a verdict it never served. The caller must therefore record + // BEFORE `releaseWaiter` deletes the entry; the union suite pins that ordering. + expiredGenerationByPane.set(key, { + generation, + paneBinding: waitersByPane.get(key)?.paneBinding ?? '' + }) } function stopStoreSubscriptionIfIdle(): void { @@ -218,7 +264,14 @@ export function parkUntilHostMirrorHandleLands( } releaseWaiter(key) }, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) - waitersByPane.set(key, { worktreeId, tabId, generation, deadline, run }) + waitersByPane.set(key, { + worktreeId, + tabId, + generation, + paneBinding: paneBindingFor(tabId, environmentId), + deadline, + run + }) startStoreSubscription() } From 33c249990a8e37a082d392d7e2268ca8fbd686a3 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:21:13 -0700 Subject: [PATCH 11/21] docs(runtime): the two guards on the park-time binding are not redundant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recording a reconciliation result that existed only in a review thread, and correcting it in the process — measuring the claim changed it. The claim under review was that the `?? ''` fallback in `recordExpiredWait` is unreachable by two independent guards, either sufficient alone: the caller's generation gate (a missing waiter fails `undefined === number`) and the record-before-release ordering. That is not what the code does. Measured, by removing each in turn: - ordering removed, generation gate kept: the gate does NOT carry it. With the waiter already deleted, the gate is false on every expiry, so nothing is ever recorded — five failures, and the door is shut by breaking the mechanism rather than by refusing ''. - generation gate removed, ordering kept: 736 files green, one failure, and it is `does not let a wait armed on the previous connection decide the new one` in host-mirror-handle-gap-resume.test.ts — a different property entirely. So the ordering alone makes `''` unreachable, and the generation gate is not a second guard on it at all: it pins reconnect-void. Both are load-bearing, for different reasons, which is a stronger argument against removing either than redundancy would have been — redundancy invites deleting one. Worth writing in the file because the two sit three lines apart and read as belt and braces on the same thing. The `''` comment next to them already exists because an unexplained guard on an unreachable value gets deleted as dead code in a year; a guard that looks redundant is deleted sooner. No behaviour change. One comment, corrected against measurement rather than against the thread it came from. --- src/renderer/src/lib/host-mirror-handle-gap-wait.ts | 5 +++++ 1 file changed, 5 insertions(+) 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 c3cd4af0d4d..7b2cb27d4e8 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -159,6 +159,11 @@ function recordExpiredWait(environmentId: string, key: string): void { // wait just ran out. Re-reading here would attribute it to whatever holds the id NOW, handing a // pane that replaced it mid-wait a verdict it never served. The caller must therefore record // BEFORE `releaseWaiter` deletes the entry; the union suite pins that ordering. + // The `?? ''` is unreachable solely because of the record-before-release ordering above it. The + // caller's generation gate LOOKS like a second guard on it and is not: drop the ordering and that + // gate stops recording anything at all rather than admitting ''. It pins a different property + // (reconnect-void, host-mirror-handle-gap-resume.test.ts). Both are load-bearing, for different + // reasons — do not collapse them as redundant. expiredGenerationByPane.set(key, { generation, paneBinding: waitersByPane.get(key)?.paneBinding ?? '' From 9446de6d195135c344e438d0f6be25d397259385 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:34:07 -0700 Subject: [PATCH 12/21] fix(runtime): a published handle retires the verdict it answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fourth eviction trigger on `expiredGenerationByPane`, and the reason it is not redundant with the three already there or with the two other agents' guards on this same map. A verdict records that a pane's 15s handle-gap wait ran out. Nothing retires it when that pane subsequently publishes its handle, so the NEXT gap on that pane gets no wait at all — #19735 with the bounded wait removed rather than merely shortened. Measured on the reconciled union tree (1b621b6b134) plus the outage guard: the verdict still answered `true` after the handle landed, and the second gap resumed with zero panes parked. Why none of the existing rules reach it, each checked rather than assumed: - superseded generation: #19647 in this same stack stops recording `status: null` for an unreachable host, so the generation no longer moves across an outage on one runtime. - dead tab row: the row stays published throughout. It is the HANDLE that comes and goes — that is the definition of the gap. - removed environment: the environment is still here. - read-time pane identity (adv2-skew, cdafc90d8f9): the pane keeps the same layout binding across the gap BY DESIGN, and the union suite pins that a genuine reattach to the same PTY must inherit. That check discriminates a different pane behind one tab id; this one discriminates a later gap on the same pane. - contact lost (adv3-journeys, 2da662424ba): no outage is involved; this is a healthy connection where the host was simply slow once. Composition proven by mutation on the union tree, four disjoint kills: dropping this drain kills 2 tests and only mine; dropping the re-park worktree kills 1 and only mine; dropping the contact guard kills 1 and only theirs; forcing the contact guard always-true kills 16 across every suite. No mutation kills another agent's test, so these are three guards on three holes, not three on one. Also carries `worktreeId` across a re-park: adopting an orphaned terminal re-keys `tabsByWorktree` without re-keying the record, so a live wait kept releasing on retraction evidence about the workspace it was no longer about. The park-time `paneBinding` deliberately does not move with it — that is the pane's identity, this is only where its rows are filed. --- ...st-mirror-handle-gap-landed-handle.test.ts | 124 ++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 40 +++++- 2 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts new file mode 100644 index 00000000000..888b05a4e37 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAppStore, type AppState } from '@/store' +import { + clearRuntimeEnvironmentConnectionGenerationsForTests, + setRuntimeEnvironmentConnectionGenerationForTests +} from '@/store/slices/runtime-status' +import { + HOST_MIRROR_HANDLE_GAP_DEADLINE_MS, + countParkedHostMirrorHandleGapPanesForTests, + hasHostMirrorHandleWaitExpired, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +/** + * The fourth eviction trigger: a PUBLISHED HANDLE ends the gap episode its verdict measured. + * + * Why none of the other three reach it. The generation rule cannot: the #19647 change in this same + * stack stops recording `status: null` for an unreachable host, so `connectionChanged` no longer + * fires across an outage on one runtime. The tab-death rule cannot: the row stays published the + * whole time — it is the HANDLE that comes and goes, which is the definition of the gap. Teardown + * cannot: the environment is still here. And the read-time pane-identity check cannot, because the + * pane that reattaches to the SAME PTY is deliberately the same pane + * (`host-mirror-handle-gap-verdict-union.test.ts`, "answers for a genuine reattach"). + * + * So a verdict outlives the gap it was about, and the NEXT gap on that pane gets no wait at all — + * #19735 with the bounded wait removed rather than merely shortened. + * + * Why this does not reopen the park/expire/replay loop the verdict exists to break: that loop is + * a handle that NEVER lands. A landed handle between two gaps is positive host evidence, and each + * wait is still individually bounded by the deadline. + */ + +const ENV_ID = 'env-landed-handle' +const WORKTREE = 'repo-1::wt-landed' +const TAB_ID = 'web-terminal-landed' +const PANE_PTY_ID = `remote:${encodeURIComponent(ENV_ID)}@@term_1` +const initialAppStoreState = useAppStore.getState() + +/** Publishes the row AND the layout binding that makes the pane unverifiable rather than dead. */ +function publishRow(options: { handleLanded: boolean }): void { + useAppStore.setState({ + tabsByWorktree: { [WORKTREE]: [{ id: TAB_ID, title: 't', ptyId: null }] }, + terminalLayoutsByTabId: { + [TAB_ID]: { + root: { type: 'leaf', leafId: 'leaf-1' }, + activeLeafId: 'leaf-1', + expandedLeafId: null, + ptyIdsByLeafId: { 'leaf-1': PANE_PTY_ID } + } + }, + ptyIdsByTabId: options.handleLanded ? { [TAB_ID]: [PANE_PTY_ID] } : {} + } as unknown as AppState) +} + +describe('handle-gap verdict, landed-handle eviction', () => { + beforeEach(() => { + vi.useFakeTimers() + useAppStore.setState(initialAppStoreState, true) + setRuntimeEnvironmentConnectionGenerationForTests(ENV_ID, 1) + }) + + afterEach(() => { + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + }) + + it('retires the verdict when the pane it was about finally publishes its handle', () => { + publishRow({ handleLanded: false }) + parkUntilHostMirrorHandleLands(ENV_ID, WORKTREE, TAB_ID, vi.fn()) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1) + expect(hasHostMirrorHandleWaitExpired(ENV_ID, TAB_ID)).toBe(true) + + // Same connection, same pane, same layout binding — only the handle is new. The verdict's + // subject has answered, so the verdict is spent. + publishRow({ handleLanded: true }) + expect(hasHostMirrorHandleWaitExpired(ENV_ID, TAB_ID)).toBe(false) + }) + + it('gives the next gap on that pane its own full wait', () => { + publishRow({ handleLanded: false }) + parkUntilHostMirrorHandleLands(ENV_ID, WORKTREE, TAB_ID, vi.fn()) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1) + publishRow({ handleLanded: true }) + + // A later frame republishes the row ahead of its handle: a NEW gap on the same connection. + publishRow({ handleLanded: false }) + expect(hasHostMirrorHandleWaitExpired(ENV_ID, TAB_ID)).toBe(false) + + const replay = vi.fn() + parkUntilHostMirrorHandleLands(ENV_ID, WORKTREE, TAB_ID, replay) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + expect(replay).not.toHaveBeenCalled() + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS + 1) + expect(replay).toHaveBeenCalledTimes(1) + }) + + it('a wait re-parked under a new worktree is released by that worktree, not the old one', () => { + // Adopting an orphaned terminal re-keys `tabsByWorktree` without re-keying the record, so the + // re-park hands the live wait a new worktree. Retraction evidence about the OLD one says + // nothing about the wait that is actually running. + useAppStore.setState({ + tabsByWorktree: { + 'wt-old': [{ id: TAB_ID, title: 't', ptyId: null }], + 'wt-new': [{ id: TAB_ID, title: 't', ptyId: null }] + }, + ptyIdsByTabId: {} + } as unknown as AppState) + parkUntilHostMirrorHandleLands(ENV_ID, 'wt-old', TAB_ID, vi.fn()) + const replayAfterAdoption = vi.fn() + parkUntilHostMirrorHandleLands(ENV_ID, 'wt-new', TAB_ID, replayAfterAdoption) + + useAppStore.setState({ + tabsByWorktree: { 'wt-new': [{ id: TAB_ID, title: 't', ptyId: null }] } + } as unknown as AppState) + expect(replayAfterAdoption).not.toHaveBeenCalled() + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + + useAppStore.setState({ tabsByWorktree: {} } as unknown as AppState) + expect(replayAfterAdoption).toHaveBeenCalledTimes(1) + }) +}) 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 7b2cb27d4e8..9ef857ec312 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -50,7 +50,7 @@ const waitersByPane = new Map() /** * Connection generation whose wait already expired for the pane. * - * THREE drains, with three different triggers. Getting the scopes right is the whole design; see + * FOUR drains, with four different triggers. Getting the scopes right is the whole design; see * `recordExpiredWait` for why the first two must NOT share a scope. * - superseded generation: per key, EVERY environment. Runs on any recording, anywhere. * - dead tab row: the recording environment ONLY. Runs on a recording in that environment. @@ -58,6 +58,14 @@ const waitersByPane = new Map() * trigger that fires at all for an environment that will never record again. A row stranded * there is inert — removal advances the generation, so it can never match — so that one is a * leak fix, not a correctness fix. + * - PUBLISHED HANDLE: `retireVerdictsWithLandedHandles`, from the store subscription. The gap a + * verdict measured is over once its pane publishes a handle, so the NEXT gap must get its own + * wait. The other three provably cannot reach this: the generation no longer moves across an + * outage on one runtime (#19647, same stack), the row stays published the whole time — it is + * the HANDLE that comes and goes — the environment is still here, and the read-time pane + * identity below deliberately lets the same PTY inherit. It is the only drain that needs the + * subscription to outlive the waiters, which is why `stopStoreSubscriptionIfIdle` counts + * verdicts too. * * ONE CLASS IS STILL UNCOVERED, and unlike the rest it is NOT conservative: a retracted tab id * that is republished inherits the old pane's verdict and skips its own wait, which is the #19735 @@ -168,10 +176,29 @@ function recordExpiredWait(environmentId: string, key: string): void { generation, paneBinding: waitersByPane.get(key)?.paneBinding ?? '' }) + // The landed-handle drain has to keep watching after this waiter is released. + startStoreSubscription() +} + +/** + * Retires the verdict of any pane whose handle is now published. + * + * A published handle is the mirror having spoken for the pane, so the gap the verdict measured is + * over. Read from `ptyIdsByTabId`, deliberately NOT from the layout `paneBinding` — the binding is + * the pane's IDENTITY and holds across the gap by design, which is exactly why it cannot see this. + */ +function retireVerdictsWithLandedHandles(state: HandleGapStoreState): void { + for (const key of expiredGenerationByPane.keys()) { + const tabId = key.slice(key.indexOf('\0') + 1) + if ((state.ptyIdsByTabId[tabId]?.length ?? 0) > 0) { + expiredGenerationByPane.delete(key) + } + } } function stopStoreSubscriptionIfIdle(): void { - if (waitersByPane.size === 0 && unsubscribeStore) { + // Verdicts count: the landed-handle drain observes a transition no waiter is parked for. + if (waitersByPane.size === 0 && expiredGenerationByPane.size === 0 && unsubscribeStore) { unsubscribeStore() unsubscribeStore = null } @@ -233,7 +260,9 @@ function startStoreSubscription(): void { return } previous = state + retireVerdictsWithLandedHandles(state) releaseDueWaiters(state) + stopStoreSubscriptionIfIdle() }) } @@ -252,6 +281,11 @@ export function parkUntilHostMirrorHandleLands( const existing = waitersByPane.get(key) if (existing) { existing.run = run + // Why the worktree moves with `run`: adopting an orphaned terminal re-keys `tabsByWorktree` + // without re-keying the record, so a live wait left on the old worktree released on retraction + // evidence about a workspace it is no longer about. The park-time `paneBinding` deliberately + // does NOT move — that is the pane's identity, and this is only where its rows are filed. + existing.worktreeId = worktreeId return } const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) @@ -299,6 +333,8 @@ export function clearHostMirrorHandleGapVerdictsForEnvironment(environmentId: st expiredGenerationByPane.delete(key) } } + // The landed-handle drain may have been the only thing holding the subscription open. + stopStoreSubscriptionIfIdle() } export function countHostMirrorHandleGapVerdictsForTests(): number { From c203d493d67f0ade27adee1ba4f05d245fc6877d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:38:18 -0700 Subject: [PATCH 13/21] docs(runtime): the reused-tab-id class is closed at read time, not still open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ExpiredHandleGapVerdict` docstring told the next reader that a retracted tab id republished under the same id still inherits its predecessor's verdict, and that closing it "needs a fourth trigger, on row retraction". The test it names as its own pin says the opposite: class D in host-mirror-handle-gap-verdict-union.test.ts asserts the verdict does not answer, and explains it is closed at READ time rather than by any prune. Provenance, since two sources disagreeing is what made this expensive: the paragraph was last written in 46ad377ceb9 and the read-time pane-identity check landed one commit later in cdafc90d8f9 (adv2-skew). The prose predates its own fix by a single commit and was never updated. Confirmed by mutation rather than by reading: dropping `verdict.paneBinding === paneBindingFor(...)` fails exactly "handles all four orphan classes simultaneously", which is the class-D assertion, so the read-time check is what closes it. Rewritten to say what the code does, keeping the part that was always true — why no trigger could have reached that class — and keeping the distinction the new PUBLISHED HANDLE drain needs: read-time identity separates two panes behind one tab id, the drain separates two gaps on one pane. The drain does not close class D and must not be read as closing it. Also records why this block specifically keeps going stale: several agents change this map in parallel, the invariants move faster than the prose, and when the two disagree the test file is the one that ran. --- .../src/lib/host-mirror-handle-gap-wait.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) 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 9ef857ec312..2e884fc43f0 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -67,12 +67,26 @@ const waitersByPane = new Map() * subscription to outlive the waiters, which is why `stopStoreSubscriptionIfIdle` counts * verdicts too. * - * ONE CLASS IS STILL UNCOVERED, and unlike the rest it is NOT conservative: a retracted tab id - * that is republished inherits the old pane's verdict and skips its own wait, which is the #19735 - * direction rather than a longer hold. No trigger above reaches it — the dead-row predicate stops - * matching once the id is live again, teardown is the wrong event, and a pane holding a verdict - * never parks, so no waiter observes the retraction. Closing it needs a fourth trigger, on row - * retraction. Pinned in host-mirror-handle-gap-verdict-union.test.ts; do not delete that case. + * A FIFTH class is covered but NOT by any of those drains: a retracted tab id republished as a + * different pane, which would inherit the old pane's verdict and skip its own wait — the #19735 + * direction rather than a longer hold. No trigger can reach it, and the reason is worth keeping: + * the dead-row predicate stops matching once the id is live again, teardown is the wrong event, + * and a pane holding a verdict never parks, so no waiter is there to observe the retraction. It is + * closed at READ time instead, by `hasHostMirrorHandleWaitExpired` comparing the verdict's + * park-time `paneBinding` — a pane that binds a newly minted PTY does not answer to a verdict + * about its predecessor. Pinned as class D in host-mirror-handle-gap-verdict-union.test.ts; do not + * delete that case. + * + * The PUBLISHED HANDLE drain does not close that class and must not be read as closing it: it + * needs the row to stay published throughout, and that class needs the row to go away. Read-time + * identity separates two panes behind one tab id; the drain separates two gaps on one pane. They + * look adjacent and are orthogonal — mutation kills them with disjoint tests. + * + * Why this comment block is worth re-reading against the code rather than trusting: the paragraph + * above it spent one commit asserting this class was still open and demanding a trigger that had + * just been replaced by the read-time check, while the test it named as its pin said the opposite. + * Several agents change this map in parallel and the invariants move faster than the prose, so + * when the two disagree the test file is the one that ran. */ type ExpiredHandleGapVerdict = { generation: number From fa52db8c6d015783fcaccfd6ea0f3d9fe8f36113 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:40:33 -0700 Subject: [PATCH 14/21] test(runtime): pin replay containment on the deadline path too Cherry-picked from aa98edf35ab (nwparker/adv3-failure-fixes) with its implementation hunk dropped: a second agent found the same throwing-replay hole independently, and `15f34014153` already closed it on this branch with an equivalent guard. Applying both would have been a double-apply, and the two spellings of the log line would have shipped side by side. The tests are worth keeping regardless. The first duplicates coverage 15f34014153 already has; the second does not -- it drives the throw from the DEADLINE path rather than the store-write path, which is a separate call into releaseWaiter and was unpinned. Spies retargeted from console.error to console.warn, the channel the guard that actually shipped writes to, so the suite silences what the code emits. --- ...rror-handle-gap-replay-containment.test.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) 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..ee651411c6a --- /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, 'warn').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, 'warn').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) + }) +}) From e0323ce16fb89cd022e70d9b8c78befca586e826 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:26:01 -0700 Subject: [PATCH 15/21] fix(runtime): isolate one worktree's replay from the mirror-hydration drain The same fan-out hazard as the handle-gap drain, one module up. Settling an environment drains every worktree parked on it in a single loop, called from the frame apply, with `waiter.run()` unguarded. One replay that throws strands every waiter queued behind it and surfaces in the caller applying the frame. Found by looking for the sibling of a defect rather than by a separate interleaving: both modules park a `run` callback and drain N of them from one event, so both have the same blast radius. Kept as its own commit because the two modules route independently. Mutation: dropping the guard kills exactly the one new assertion. --- ...ost-session-mirror-hydration-drain.test.ts | 31 +++++++++++++++++++ .../runtime/host-session-mirror-hydration.ts | 9 +++++- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/renderer/src/runtime/host-session-mirror-hydration-drain.test.ts diff --git a/src/renderer/src/runtime/host-session-mirror-hydration-drain.test.ts b/src/renderer/src/runtime/host-session-mirror-hydration-drain.test.ts new file mode 100644 index 00000000000..0b96e293c19 --- /dev/null +++ b/src/renderer/src/runtime/host-session-mirror-hydration-drain.test.ts @@ -0,0 +1,31 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { clearRuntimeEnvironmentConnectionGenerationsForTests } from '@/store/slices/runtime-status' +import { + markHostSessionMirrorHydrated, + parkUntilHostSessionMirrorHydrates, + resetHostSessionMirrorHydrationForTests +} from './host-session-mirror-hydration' + +// The same fan-out hazard as host-mirror-handle-gap-drain.test.ts, one module up: settling an +// environment drains every worktree parked on it in one loop, from inside the frame apply. The +// waiters are strangers to each other and to that apply, so one replay must not be able to reach +// either of them. +const ENVIRONMENT_ID = 'env-hydration-drain' + +describe('host session mirror hydration drain', () => { + afterEach(() => { + resetHostSessionMirrorHydrationForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + }) + + it('settles the remaining parked worktrees when one replay throws', () => { + const secondReplay = vi.fn() + parkUntilHostSessionMirrorHydrates(ENVIRONMENT_ID, 'repo::first', () => { + throw new Error('replay blew up') + }) + parkUntilHostSessionMirrorHydrates(ENVIRONMENT_ID, 'repo::second', secondReplay) + + expect(() => markHostSessionMirrorHydrated(ENVIRONMENT_ID)).not.toThrow() + expect(secondReplay).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/runtime/host-session-mirror-hydration.ts b/src/renderer/src/runtime/host-session-mirror-hydration.ts index be21db6e08b..aaeb8685d73 100644 --- a/src/renderer/src/runtime/host-session-mirror-hydration.ts +++ b/src/renderer/src/runtime/host-session-mirror-hydration.ts @@ -53,7 +53,14 @@ function drainParkedWaiters(matches: (waiter: ParkedMirrorWaiter) => boolean): v const waiter = parkedWaitersByWorktree.get(key) if (waiter) { parkedWaitersByWorktree.delete(key) - waiter.run() + try { + waiter.run() + } catch (error) { + // Why: one settle drains every waiter the environment holds, and they are strangers to each + // other and to the frame apply that called it. An unguarded throw strands every waiter + // queued behind this one and surfaces in the caller applying the frame. + console.warn('[host-session-mirror-hydration] parked replay failed:', error) + } } } } From 4a9cee297f21c2fac2d75f80a67bd0a1143461da Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:38:13 -0700 Subject: [PATCH 16/21] test(runtime): pin sleeping-agent resume on a failed SSH target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal-state floor in workspace-terminal-host-authority.ts has three consumers: initial-terminal seeding, the startup terminal watcher, and sleeping-agent resume. Seeding is covered end to end by worktree-agent-activation-seam.test.ts. Resume was covered only at the predicate, so nothing failed if the floor stopped reaching it — and the floor's own comment says the cost of losing it is a failed target left terminal-less with unresumable agents for the rest of the app session. Pins the resume half directly: an SSH git worktree on a target whose sync terminated in offline/error with an empty hydrated set resumes its sleeping agent. Two controls keep the floor from widening into "resume whenever we are unsure" — an in-flight 'pulling' sync and no sync status at all both stay unverifiable and resume nothing. Verified by mutation: emptying TERMINATED_WITHOUT_ANSWER_PHASES fails exactly the two floor assertions and leaves both controls passing. Routes independently of the two fixes on this branch: the floor predates this stack (#16750), and this only closes a coverage gap in it. --- ...ailed-target-sleeping-agent-resume.test.ts | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts diff --git a/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts b/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts new file mode 100644 index 00000000000..a94c0438231 --- /dev/null +++ b/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts @@ -0,0 +1,115 @@ +/** + * The resume half of the terminal-state floor. + * + * `workspace-terminal-host-authority.ts` says an SSH target whose sync terminated in + * `offline`/`error` without ever hydrating answers `none`, so this client may act. The seeding + * consumer is covered end to end (worktree-agent-activation-seam.test.ts); the sleeping-agent + * consumer (resume-sleeping-agent-session.ts) was only covered at the predicate. Without this, + * a failed target's agents stay unresumable for the rest of the app session and nothing fails. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import { useAppStore } from '@/store' +import { makeWorktree } from '@/store/slices/store-test-helpers' +import { resolveWorkspaceTerminalHostAuthority } from './workspace-terminal-host-authority' +import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) + +const initialAppStoreState = useAppStore.getState() +const TARGET_ID = 'ssh-target-1' +const WORKTREE_ID = 'repoSsh::/srv/proj/feature' + +afterEach(() => { + useAppStore.setState(initialAppStoreState, true) +}) + +function seedFailedSshTarget(phase?: 'offline' | 'error' | 'pulling'): void { + const tab: TerminalTab = { + id: 'tab-1', + ptyId: null, + worktreeId: WORKTREE_ID, + title: 'shell', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + const record: SleepingAgentSessionRecord = { + paneKey: 'tab-1:leaf-1', + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + agent: 'pi', + providerSession: { key: 'session_id', id: 'pi-session-1', transcriptPath: '/tmp/pi-1.jsonl' }, + prompt: '', + state: 'working', + capturedAt: 1, + updatedAt: 1, + origin: 'worktree-sleep' + } + useAppStore.setState({ + repos: [ + { + id: 'repoSsh', + path: '/srv/proj', + displayName: 'repoSsh', + badgeColor: '#000', + addedAt: 0, + connectionId: TARGET_ID + } + ] as never, + worktreesByRepo: { + repoSsh: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: 'repoSsh', + path: '/srv/proj/feature', + hostId: `ssh:${TARGET_ID}` + } as never) + ] + }, + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: + phase === undefined ? {} : { [TARGET_ID]: { phase, direction: 'pull' as const } }, + tabsByWorktree: { [WORKTREE_ID]: [tab] }, + sleepingAgentSessionsByPaneKey: { [record.paneKey]: record } + }) +} + +describe('sleeping-agent resume on a failed SSH target', () => { + it.each(['offline', 'error'] as const)( + 'resumes a sleeping agent once a sync terminates in %s without ever hydrating', + (phase) => { + seedFailedSshTarget(phase) + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'none' + ) + // The gate this exists for: a target that failed must not stay unresumable for the session. + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBeUndefined() + } + ) + + it('still declines to resume while the host has not answered', () => { + // Control: an in-flight sync is `unverifiable`, and resuming there forks a session the host + // may still be running. The floor must not widen into "resume whenever we are unsure". + seedFailedSshTarget('pulling') + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(0) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBeDefined() + }) + + it('still declines to resume when no sync status exists at all', () => { + seedFailedSshTarget(undefined) + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(0) + }) +}) From cb29f42c1e703f363973d751f4e4c61860410ce7 Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 11 Sep 2026 00:15:12 -0700 Subject: [PATCH 17/21] test(runtime): pin the store subscription the reconciled loop can leak The retention suite that `reconcile three branches' handle-gap verdict rules into one loop` replaced carried an assertion the split suites did not: the store subscription is held for exactly as long as something needs it. Measured before writing it, because half of it was already covered: RETAIN direction -- drop the verdict term from `stopStoreSubscriptionIfIdle` so a verdict with no waiter behind it loses the subscription its drain needs: already caught, 2 failures in host-mirror-handle-gap-landed-handle.test.ts. RELEASE direction -- never release the subscription at all: caught by NOTHING. That mutation passes all 272 tests across the 33 other handle-gap and session-tabs suites. A leaked subscription rescans every parked pane on every store write for the life of the session and nothing notices. So this is for the release direction. The retain cases ride along because both halves of one invariant belong in one file, not because they were missing. That term is also precisely what the reconcile moved -- it now counts verdicts as well as waiters -- so it is the part of this map most likely to drift again. Asserted with a spy on useAppStore.subscribe rather than a new test-only export: whether the module is subscribed is already observable at the store boundary, and the production surface should not grow just to say so. --- ...r-handle-gap-subscription-lifetime.test.ts | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts diff --git a/src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts new file mode 100644 index 00000000000..c6f1de54a92 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts @@ -0,0 +1,151 @@ +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, + clearHostMirrorHandleGapVerdictsForEnvironment, + countHostMirrorHandleGapVerdictsForTests, + countParkedHostMirrorHandleGapPanesForTests, + parkUntilHostMirrorHandleLands, + resetHostMirrorHandleGapWaitsForTests +} from './host-mirror-handle-gap-wait' + +// The retention suite that the reconciled verdict loop replaced carried one assertion the split +// suites did not: the store subscription is held for exactly as long as something needs it. +// +// Measured rather than assumed, because half of it turned out to be covered already: +// - RETAIN direction (drop the verdict term from `stopStoreSubscriptionIfIdle`, so a verdict +// with no waiter behind it loses the subscription its drain needs): already caught, by +// host-mirror-handle-gap-landed-handle.test.ts. Two failures there without this file. +// - RELEASE direction (never release the subscription at all): caught by NOTHING. That mutation +// passes all 272 tests across the 33 other handle-gap and session-tabs suites. A leaked +// subscription rescans every parked pane on every store write for the life of the session and +// no test notices. +// +// So this file exists for the release direction; the retain cases are here because the two belong +// in one place, not because they were missing. `stopStoreSubscriptionIfIdle` counts VERDICTS as +// well as waiters -- the landed-handle drain observes a transition no waiter is parked for -- and +// that is exactly the term the reconcile moved, so both directions are worth holding still. +// +// Asserted through a spy rather than a new test-only export: whether the module is subscribed is +// already observable at the store boundary, and the production surface should not grow to say so. + +const ENVIRONMENT_ID = 'env-subscription' +const OTHER_ENVIRONMENT_ID = 'env-other' +const WORKTREE_ID = 'repo-1::/workspace/repo' + +const initialAppStoreState = useAppStore.getState() + +let unsubscribeCalls: number +let subscribeCalls: number + +function publishPaneAndPark(environmentId: string, tabId: string): void { + const state = useAppStore.getState() + const published = state.tabsByWorktree[WORKTREE_ID] ?? [] + useAppStore.setState({ + ptyIdsByTabId: {}, + tabsByWorktree: { + [WORKTREE_ID]: [...published.filter((tab) => tab.id !== tabId), { id: tabId, title: tabId }] + }, + terminalLayoutsByTabId: { + ...state.terminalLayoutsByTabId, + [tabId]: { + root: { type: 'leaf', leafId: `leaf-${tabId}` }, + activeLeafId: `leaf-${tabId}`, + expandedLeafId: null, + ptyIdsByLeafId: { [`leaf-${tabId}`]: `remote:${environmentId}@@term_${tabId}` } + } + } + } as never) + parkUntilHostMirrorHandleLands(environmentId, WORKTREE_ID, tabId, () => {}) +} + +/** Lands the pane's handle, which is what both releases a waiter and retires a verdict. */ +function landHandle(tabId: string): void { + useAppStore.setState({ + ptyIdsByTabId: { ...useAppStore.getState().ptyIdsByTabId, [tabId]: [`pty-${tabId}`] } + } as never) +} + +describe('host-mirror handle-gap store subscription lifetime', () => { + beforeEach(() => { + vi.useFakeTimers() + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + unsubscribeCalls = 0 + subscribeCalls = 0 + const realSubscribe = useAppStore.subscribe.bind(useAppStore) + vi.spyOn(useAppStore, 'subscribe').mockImplementation(((listener: never) => { + subscribeCalls += 1 + const unsubscribe = realSubscribe(listener) + return () => { + unsubscribeCalls += 1 + unsubscribe() + } + }) as never) + }) + + afterEach(() => { + vi.restoreAllMocks() + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + }) + + it('holds exactly one subscription across several parked panes', () => { + publishPaneAndPark(ENVIRONMENT_ID, 'tab-a') + publishPaneAndPark(ENVIRONMENT_ID, 'tab-b') + publishPaneAndPark(OTHER_ENVIRONMENT_ID, 'tab-c') + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(3) + expect(subscribeCalls).toBe(1) + expect(unsubscribeCalls).toBe(0) + }) + + it('releases the subscription once the last waiter leaves and no verdict remains', () => { + publishPaneAndPark(ENVIRONMENT_ID, 'tab-a') + publishPaneAndPark(ENVIRONMENT_ID, 'tab-b') + + landHandle('tab-a') + expect(unsubscribeCalls).toBe(0) + + landHandle('tab-b') + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(0) + expect(unsubscribeCalls).toBe(1) + }) + + it('keeps the subscription for a verdict with no waiter parked behind it', () => { + // The case the reconcile introduced: the waiter is gone, but the landed-handle drain still has + // a verdict to watch. Counting only waiters here would drop the subscription that drain needs. + publishPaneAndPark(ENVIRONMENT_ID, 'tab-a') + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(1) + expect(unsubscribeCalls).toBe(0) + }) + + it('releases the subscription when the last verdict is cleared by teardown', () => { + publishPaneAndPark(ENVIRONMENT_ID, 'tab-a') + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + expect(unsubscribeCalls).toBe(0) + + clearHostMirrorHandleGapVerdictsForEnvironment(ENVIRONMENT_ID) + + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(0) + expect(unsubscribeCalls).toBe(1) + }) + + it('re-subscribes rather than reusing a dropped subscription', () => { + publishPaneAndPark(ENVIRONMENT_ID, 'tab-a') + landHandle('tab-a') + expect(unsubscribeCalls).toBe(1) + + publishPaneAndPark(ENVIRONMENT_ID, 'tab-d') + + expect(subscribeCalls).toBe(2) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + }) +}) From 42e8c6cc46c297ce9173c8937cc76cdd3e9532ba Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 01:10:04 -0700 Subject: [PATCH 18/21] wip(e2e): runtime endpoint link fault hop --- .../helpers/runtime-endpoint-link-fault.ts | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/e2e/helpers/runtime-endpoint-link-fault.ts diff --git a/tests/e2e/helpers/runtime-endpoint-link-fault.ts b/tests/e2e/helpers/runtime-endpoint-link-fault.ts new file mode 100644 index 00000000000..f06c16bf319 --- /dev/null +++ b/tests/e2e/helpers/runtime-endpoint-link-fault.ts @@ -0,0 +1,110 @@ +import net from 'node:net' +import { decodePairingOffer, encodePairingOffer } from '../../../src/shared/pairing' + +/** + * A TCP hop in front of a paired runtime's WebSocket endpoint whose fault mode can + * change mid-test. + * + * `stall-new` is the Tailscale-shaped fault this exists for: already-established + * flows keep delivering while a freshly dialed connection hangs unanswered. That + * asymmetry is what separates "the control plane could not ask" from "the runtime + * is gone", and neither killing the runtime nor `runtimeEnvironments.disconnect` + * can produce it — both take the two halves down together. + */ +export type RuntimeLinkFaultMode = 'pass' | 'stall-new' + +export type RuntimeEndpointLinkFault = { + /** ws:// endpoint that routes through this hop. */ + endpoint: string + setMode: (mode: RuntimeLinkFaultMode) => void + /** Connections accepted since the last reset — proves a dial actually reached the hop. */ + acceptedConnectionCount: () => number + stalledConnectionCount: () => number + resetCounters: () => void + close: () => Promise +} + +function parseWsEndpoint(endpoint: string): { host: string; port: number } { + const url = new URL(endpoint) + return { host: url.hostname, port: Number(url.port) } +} + +export async function startRuntimeEndpointLinkFault( + upstreamEndpoint: string +): Promise { + const upstream = parseWsEndpoint(upstreamEndpoint) + let mode: RuntimeLinkFaultMode = 'pass' + let accepted = 0 + let stalledTotal = 0 + const stalled = new Set() + const live = new Set() + + const server = net.createServer((client) => { + accepted += 1 + live.add(client) + client.on('close', () => live.delete(client)) + client.on('error', () => client.destroy()) + if (mode === 'stall-new') { + // Accept the TCP handshake and answer nothing: the dialer waits out its own + // timeout, as it does on a half-open path, instead of failing fast on reset. + stalledTotal += 1 + stalled.add(client) + client.on('close', () => stalled.delete(client)) + return + } + const upstreamSocket = net.connect(upstream.port, upstream.host) + live.add(upstreamSocket) + upstreamSocket.on('close', () => live.delete(upstreamSocket)) + upstreamSocket.on('error', () => client.destroy()) + client.pipe(upstreamSocket) + upstreamSocket.pipe(client) + }) + + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', () => { + server.removeListener('error', reject) + resolve() + }) + }) + const address = server.address() + if (address === null || typeof address === 'string') { + throw new Error('runtime endpoint link fault did not bind a TCP port') + } + + return { + endpoint: `ws://127.0.0.1:${address.port}`, + setMode: (next) => { + mode = next + if (next === 'pass') { + for (const socket of stalled) { + socket.destroy() + } + stalled.clear() + } + }, + acceptedConnectionCount: () => accepted, + stalledConnectionCount: () => stalledTotal, + resetCounters: () => { + accepted = 0 + stalledTotal = 0 + }, + close: async () => { + for (const socket of live) { + socket.destroy() + } + live.clear() + stalled.clear() + await new Promise((resolve) => server.close(() => resolve())) + } + } +} + +/** Re-points a pairing offer at `endpoint` without touching its keys or device token. */ +export function repointPairingUrl(pairingUrl: string, endpoint: string): string { + return encodePairingOffer({ ...decodePairingOffer(pairingUrl), endpoint }) +} + +export function readPairingEndpoint(pairingUrl: string): string { + return decodePairingOffer(pairingUrl).endpoint +} From 2a52a57f30700ba5f311124a47b924b04a9848d7 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 02:41:24 -0700 Subject: [PATCH 19/21] fix(runtime): keep a live host verdict when a status probe can't reach it (#19647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A status.get dials its own fresh socket (sendRemoteRuntimeRequest), so on a half-open link it hangs and fails with the client-minted `runtime_unavailable` while established terminal/agent flows keep delivering. The status recheck ladder and the refresh wrapper caught that failure and published `status: null` over a recorded live verdict. That single write dropped the environment out of getReachableRuntimeSessionMirrorTargets (mirror teardown #1), and the next successful probe advanced the connection generation and rebuilt it (teardown #2) — one transient probe failure, two full session-tabs mirror teardowns, and the client demanding a reconnect while the agent kept working. Treat that failure as unverifiable, per docs/reference/ssh-execution-boundary.md: loss of contact is never evidence the host exited. A status.get that fails with `runtime_unavailable` (client-minted, never on the wire — safe under remote-wire-compatibility Rules 1 and 3) no longer overwrites a recorded live verdict. The live status stays, the mirror is never retired, the connection generation never bumps, and the recheck ladder keeps probing until a real answer. First-contact failures (no prior live verdict) still record null so host coverage completes. Carries forward the reader-side fixes from #19163 (filed against #16516): the recheck ladder re-asks a status: null host on its backoff and cancels on manual disconnect, and NoticeHostGlyph / use-worktree-card-foundation use the shared runtimeHostConnectionState derivation so a never-probed host is not painted the destructive red of one a probe found unreachable. Measured with a new paired-client e2e rig (stall-new TCP hop): baseline nulls the status and bumps the generation 0->1 on a link flap; fixed keeps nullStatus=false and the generation stable. Two adjacent defects found while measuring are filed separately: #19871 (multiplexer socket dropped on last-stream close) and #19872 (un-park banner claims auto-retry while parkRetry arms no timer). Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: ASDFGHoney <86656987+ASDFGHoney@users.noreply.github.com> --- .../src/store/slices/runtime-status.test.ts | 13 +- .../src/store/slices/runtime-status.ts | 9 + .../helpers/runtime-endpoint-link-fault.ts | 16 + ...mote-terminal-tab-switch-reconnect.spec.ts | 543 ++++++++++++++++++ ...ar-runtime-host-disconnected-glyph.spec.ts | 124 ++++ 5 files changed, 704 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/paired-remote-terminal-tab-switch-reconnect.spec.ts create mode 100644 tests/e2e/sidebar-runtime-host-disconnected-glyph.spec.ts diff --git a/src/renderer/src/store/slices/runtime-status.test.ts b/src/renderer/src/store/slices/runtime-status.test.ts index fdc843c748a..badce699235 100644 --- a/src/renderer/src/store/slices/runtime-status.test.ts +++ b/src/renderer/src/store/slices/runtime-status.test.ts @@ -710,7 +710,10 @@ describe('runtime-status slice', () => { clearRuntimeCompatibilityCacheForTests() }) - it('records null and returns false when a runtime refresh fails', async () => { + // #19647: a failed status.get dials its own fresh socket, so a refresh must not overwrite a + // recorded live verdict with null — that retires the host's session-tabs mirror and dims its + // still-live rows on a fault the client could not even ask through. + it('preserves a recorded live verdict when a refresh probe fails', async () => { const getStatus = vi.fn().mockRejectedValue(new Error('closed')) stubRuntimeEnvironmentApi({ getStatus }) const store = createSliceStore() @@ -720,6 +723,14 @@ describe('runtime-status slice', () => { const reachable = await store.getState().refreshRuntimeEnvironmentStatus('env-a') expect(reachable).toBe(false) + expect(store.getState().runtimeStatusByEnvironmentId.get('env-a')?.status).toBe(cached) + }) + + it('records null on a first-contact refresh failure, so host coverage completes', async () => { + stubRuntimeEnvironmentApi({ getStatus: vi.fn().mockRejectedValue(new Error('closed')) }) + const store = createSliceStore() + + expect(await store.getState().refreshRuntimeEnvironmentStatus('env-a')).toBe(false) expect(store.getState().runtimeStatusByEnvironmentId.get('env-a')?.status).toBe(null) }) diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index c11fba3308a..1dd012cb317 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -292,6 +292,15 @@ export const createRuntimeStatusSlice: StateCreator void + /** + * Severs every established flow once, leaving `mode` in force for what dials next. + * With `stall-new` this is the link flap that puts the shared-control socket into + * `reconnecting` while its replacement dial hangs. + */ + dropEstablished: () => number /** Connections accepted since the last reset — proves a dial actually reached the hop. */ acceptedConnectionCount: () => number stalledConnectionCount: () => number @@ -83,6 +89,16 @@ export async function startRuntimeEndpointLinkFault( stalled.clear() } }, + dropEstablished: () => { + let dropped = 0 + for (const socket of live) { + if (!stalled.has(socket)) { + socket.destroy() + dropped += 1 + } + } + return dropped + }, acceptedConnectionCount: () => accepted, stalledConnectionCount: () => stalledTotal, resetCounters: () => { diff --git a/tests/e2e/paired-remote-terminal-tab-switch-reconnect.spec.ts b/tests/e2e/paired-remote-terminal-tab-switch-reconnect.spec.ts new file mode 100644 index 00000000000..1b798747a27 --- /dev/null +++ b/tests/e2e/paired-remote-terminal-tab-switch-reconnect.spec.ts @@ -0,0 +1,543 @@ +/** + * #19647 "Orca has to reconnect every time I switch tabs". + * + * TOPOLOGY: real Orca desktop app as the remote server + a separate real Orca + * desktop client paired to it, with every client->host byte routed through a TCP + * hop whose fault mode this test controls. That hop is the reporter's Tailscale + * link; `stall-new` reproduces its characteristic failure, where established + * flows keep delivering (their agent kept working) while a freshly dialed + * connection hangs. + * + * Exploratory-first: the `[tab-switch-repro]` census and timeline lines are the + * diagnosis. The hard gate is the #19647 writer contract — a status.get the client + * could not push through must not null a recorded live verdict, and recovery must not + * advance the connection generation. Whether the pane repaints within budget is logged, + * NOT asserted: that is gated on the un-park latch (#19872) and the multiplexer + * cold-handshake (#19871), both filed separately. A green run is NOT a clean bill of health. + * + * Run: + * pnpm exec playwright test \ + * tests/e2e/paired-remote-terminal-tab-switch-reconnect.spec.ts \ + * --config tests/playwright.config.ts --project electron-headless --workers=1 + */ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { randomUUID } from 'node:crypto' +import os from 'node:os' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { + HOST_TERMINAL_SURFACE_SEPARATOR, + toWebTerminalSurfaceTabId +} from '../../src/shared/terminal-surface-id' +import { expect, test } from './helpers/orca-app' +import { + createRuntimeDesktopPairingOffer, + launchPairedElectronClient +} from './helpers/paired-electron-client' +import { + readPairingEndpoint, + repointPairingUrl, + startRuntimeEndpointLinkFault +} from './helpers/runtime-endpoint-link-fault' +import { waitForTabParked } from './helpers/terminal-hidden-parking' + +const PARK_DELAY_MS = 2_000 +const scratch = mkdtempSync(path.join(os.tmpdir(), 'orca-tab-switch-reconnect-')) +const fixturePath = path.join(scratch, 'tab-switch-terminal.mjs') +writeFileSync( + fixturePath, + [ + "import { appendFileSync } from 'node:fs'", + 'const sink = process.argv[2]', + 'const record = (line) => appendFileSync(sink, `${line}\\n`)', + "record('READY')", + "process.stdout.write('READY\\r\\n')", + "process.stdin.setEncoding('utf8')", + "let pending = ''", + "process.stdin.on('data', (data) => {", + ' pending += data', + ' const lines = pending.split(/\\r\\n|\\r|\\n/)', + " pending = lines.pop() ?? ''", + ' for (const line of lines) {', + ' record(`LINE:${line}`)', + ' process.stdout.write(`LINE:${line}\\r\\n`)', + ' }', + '})', + 'process.stdin.resume()' + ].join('\n') +) + +test.afterAll(() => { + rmSync(scratch, { recursive: true, force: true }) +}) + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", `'\\''`)}'` +} + +function fixtureCommand(sinkPath: string): string { + const command = [process.execPath, fixturePath, sinkPath] + return process.platform === 'win32' + ? command.map((value) => `"${value.replaceAll('"', '""')}"`).join(' ') + : command.map(shellQuote).join(' ') +} + +function readSink(sinkPath: string): string { + try { + return readFileSync(sinkPath, 'utf8') + } catch { + return '' + } +} + +async function callEnvironment( + page: Page, + environmentId: string, + method: string, + params: unknown +): Promise { + return page.evaluate( + async ({ environmentId, method, params }) => { + const response = await window.api.runtimeEnvironments.call({ + selector: environmentId, + method, + params + }) + if (!response.ok) { + throw new Error(`${response.error.code}: ${response.error.message}`) + } + return response.result + }, + { environmentId, method, params } + ) as Promise +} + +type HostTerminal = { + hostTabId: string + sinkPath: string + terminal: string + webTabId: string +} + +async function createHostTerminal( + page: Page, + environmentId: string, + worktreeId: string +): Promise { + const sinkPath = path.join(scratch, `sink-${randomUUID()}.log`) + const result = await callEnvironment<{ + tab: { id: string; terminal: string | null } + }>(page, environmentId, 'session.tabs.createTerminal', { + worktree: `id:${worktreeId}`, + command: fixtureCommand(sinkPath), + activate: false, + select: false, + navigation: 'caller' + }) + if (!result.tab.terminal) { + throw new Error('host session terminal was not created') + } + const hostTabId = result.tab.id.split(HOST_TERMINAL_SURFACE_SEPARATOR)[0] + return { + hostTabId, + sinkPath, + terminal: result.tab.terminal, + webTabId: toWebTerminalSurfaceTabId(hostTabId) + } +} + +async function waitForMirroredTab(page: Page, worktreeId: string, webTabId: string): Promise { + await expect + .poll( + () => + page.evaluate( + (id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id), + worktreeId + ), + { + timeout: 60_000, + message: `client never mirrored host tab ${webTabId}` + } + ) + .toContain(webTabId) +} + +async function selectClientTab(page: Page, worktreeId: string, webTabId: string): Promise { + await page.evaluate( + ({ webTabId, worktreeId }) => { + const state = window.__store?.getState() + state?.setActiveView('terminal') + state?.setActiveWorktree(worktreeId) + state?.setActiveTab(webTabId) + state?.setActiveTabType('terminal') + }, + { webTabId, worktreeId } + ) +} + +async function openClientTab(page: Page, worktreeId: string, webTabId: string): Promise { + await waitForMirroredTab(page, worktreeId, webTabId) + await selectClientTab(page, worktreeId, webTabId) + await expect + .poll(() => page.evaluate((id) => window.__paneManagers?.has(id) ?? false, webTabId), { + timeout: 60_000, + message: `client pane for ${webTabId} did not mount` + }) + .toBe(true) +} + +type MultiplexCensus = { + activeStreamCount: number + transportSubscribeCount: number + transportUnsubscribeCount: number + streamSubscribeCount: number + streamUnsubscribeCount: number +} + +async function readMultiplexCensus(page: Page): Promise { + return page.evaluate(() => { + const snapshot = ( + window as Window & { + __remoteTerminalMultiplexAckGate?: { + snapshot: () => { + activeStreams: unknown[] + transportSubscribeCount: number + transportUnsubscribeCount: number + streamSubscribeCount: number + streamUnsubscribeCount: number + } + } + } + ).__remoteTerminalMultiplexAckGate?.snapshot() + return { + activeStreamCount: snapshot?.activeStreams.length ?? -1, + transportSubscribeCount: snapshot?.transportSubscribeCount ?? -1, + transportUnsubscribeCount: snapshot?.transportUnsubscribeCount ?? -1, + streamSubscribeCount: snapshot?.streamSubscribeCount ?? -1, + streamUnsubscribeCount: snapshot?.streamUnsubscribeCount ?? -1 + } + }) +} + +type PaneObservation = { + atMs: number + banner: string | null + recoveryState: string | null + mounted: boolean + nullStatusEnvironments: number + connectionGeneration: number | null + remoteControlState: string | null +} + +async function observePane( + page: Page, + webTabId: string, + environmentId: string, + startedAt: number +): Promise { + const observation = await page.evaluate( + ({ id, environmentId }) => { + const manager = window.__paneManagers?.get(id) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + const banner = document.querySelector('[data-terminal-remote-runtime-reconnect-banner]') + const statuses = window.__store?.getState().runtimeStatusByEnvironmentId + let nullStatusEnvironments = 0 + for (const entry of statuses?.values() ?? []) { + if (entry.status === null) { + nullStatusEnvironments += 1 + } + } + return { + banner: banner?.getAttribute('data-terminal-remote-runtime-reconnect-banner') ?? null, + recoveryState: pane?.container?.dataset?.ptyRecoveryState ?? null, + mounted: Boolean(manager), + nullStatusEnvironments, + connectionGeneration: statuses?.get(environmentId)?.connectionGeneration ?? null, + remoteControlState: + statuses?.get(environmentId)?.status?.remoteControl?.state ?? + statuses?.get(environmentId)?.remoteControl?.state ?? + null + } + }, + { id: webTabId, environmentId } + ) + return { atMs: Date.now() - startedAt, ...observation } +} + +async function readPaneContent(page: Page, webTabId: string): Promise { + return page.evaluate((id) => { + const manager = window.__paneManagers?.get(id) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + return pane?.serializeAddon?.serialize?.() ?? '' + }, webTabId) +} + +type RevealRecord = { + timeline: PaneObservation[] + paintedAtMs: number | null + sawBanner: boolean + sawNullStatus: boolean +} + +/** Samples the revealed pane for `budgetMs`, stopping early once `marker` paints. */ +async function recordRevealTimeline( + page: Page, + webTabId: string, + environmentId: string, + marker: string, + budgetMs: number +): Promise { + const startedAt = Date.now() + const timeline: PaneObservation[] = [] + let paintedAtMs: number | null = null + let sawBanner = false + let sawNullStatus = false + let previous = '' + while (Date.now() - startedAt < budgetMs) { + const observation = await observePane(page, webTabId, environmentId, startedAt) + sawBanner ||= observation.banner !== null + sawNullStatus ||= observation.nullStatusEnvironments > 0 + const key = `${observation.banner}|${observation.recoveryState}|${observation.mounted}|${observation.nullStatusEnvironments}|${observation.connectionGeneration}|${observation.remoteControlState}` + if (key !== previous) { + timeline.push(observation) + previous = key + } + if ((await readPaneContent(page, webTabId)).includes(marker)) { + paintedAtMs = Date.now() - startedAt + timeline.push(await observePane(page, webTabId, environmentId, startedAt)) + break + } + await new Promise((resolve) => setTimeout(resolve, 250)) + } + return { timeline, paintedAtMs, sawBanner, sawNullStatus } +} + +test('paired client tab switch does not reconnect the remote runtime', async ({ + orcaPage +}, testInfo) => { + test.setTimeout(900_000) + const rawOffer = await createRuntimeDesktopPairingOffer(orcaPage) + const link = await startRuntimeEndpointLinkFault(readPairingEndpoint(rawOffer.pairingUrl)) + const offer = { + ...rawOffer, + pairingUrl: repointPairingUrl(rawOffer.pairingUrl, link.endpoint) + } + + const client = await launchPairedElectronClient(offer, testInfo, 'tab-switch-reconnect', { + extraEnv: { ORCA_E2E_TERMINAL_PARKING_DELAY_MS: String(PARK_DELAY_MS) } + }) + const createdTerminals: string[] = [] + try { + const worktreeId = await orcaPage.evaluate(() => { + const id = window.__store?.getState().activeWorktreeId + if (!id) { + throw new Error('headed host has no active worktree') + } + return id + }) + await expect + .poll( + () => + client.page.evaluate( + (id) => + window.__store + ?.getState() + .allWorktrees() + .some((worktree) => worktree.id === id) ?? false, + worktreeId + ), + { + timeout: 60_000, + message: 'paired client never saw the host worktree' + } + ) + .toBe(true) + + const target = await createHostTerminal(client.page, client.environmentId, worktreeId) + const decoys = [ + await createHostTerminal(client.page, client.environmentId, worktreeId), + await createHostTerminal(client.page, client.environmentId, worktreeId) + ] + createdTerminals.push(target.terminal, ...decoys.map((decoy) => decoy.terminal)) + + await openClientTab(client.page, worktreeId, target.webTabId) + await expect + .poll(() => readPaneContent(client.page, target.webTabId), { + timeout: 60_000, + message: 'target terminal never painted READY' + }) + .toContain('READY') + + const censusAfterFirstOpen = await readMultiplexCensus(client.page) + console.log( + `[tab-switch-repro] census after first open: ${JSON.stringify(censusAfterFirstOpen)}` + ) + + // ---- Arm A: plain tab switching on a healthy link. No fault involved: if the + // transport subscribe count climbs here, tab switching alone tears down and + // re-establishes the environment's control channel. + for (let round = 0; round < 3; round += 1) { + await openClientTab(client.page, worktreeId, decoys[0].webTabId) + await openClientTab(client.page, worktreeId, decoys[1].webTabId) + await waitForTabParked(client.page, target.webTabId, { + parkDelayMs: PARK_DELAY_MS + }) + await openClientTab(client.page, worktreeId, target.webTabId) + } + const censusAfterHealthySwitching = await readMultiplexCensus(client.page) + console.log( + `[tab-switch-repro] census after healthy switching: ${JSON.stringify(censusAfterHealthySwitching)}` + ) + console.log( + `[tab-switch-repro] ARM A transportSubscribeCount delta over 3 healthy park/reveal rounds: ${censusAfterHealthySwitching.transportSubscribeCount - censusAfterFirstOpen.transportSubscribeCount} (unsubscribe delta ${censusAfterHealthySwitching.transportUnsubscribeCount - censusAfterFirstOpen.transportUnsubscribeCount})` + ) + + // ---- Arm B: park every terminal pane for this environment, so nothing holds + // the multiplexed control channel open, then reveal one under a link whose + // established flows are fine but whose new dials hang. + // Why not a plain view switch: cold-park exempts the single most-recently-hidden + // tab, so one pane (and its stream) keeps the multiplexer alive, and the exemption + // migrates to a parked tab the moment the exempt one closes. So: make a decoy the + // exempt one, park the rest, put the link into stall-new, and only then close that + // decoy on the host (over the established control flow). The multiplexer idles out + // and nothing can re-dial it on a healthy path before the reveal. + await openClientTab(client.page, worktreeId, decoys[1].webTabId) + await client.page.evaluate(() => { + window.__store?.getState().setActiveView('changes') + }) + await waitForTabParked(client.page, target.webTabId, { parkDelayMs: PARK_DELAY_MS }) + await waitForTabParked(client.page, decoys[0].webTabId, { parkDelayMs: PARK_DELAY_MS }) + const censusBeforeRelease = await readMultiplexCensus(client.page) + console.log( + `[tab-switch-repro] census with only the exempt decoy mounted: ${JSON.stringify(censusBeforeRelease)}` + ) + link.resetCounters() + link.setMode('stall-new') + await callEnvironment(client.page, client.environmentId, 'terminal.closeTab', { + terminal: decoys[1].terminal + }) + createdTerminals.splice(createdTerminals.indexOf(decoys[1].terminal), 1) + await expect + .poll(async () => (await readMultiplexCensus(client.page)).transportUnsubscribeCount, { + timeout: 30_000, + message: 'the terminal multiplexer was never released after its last stream closed' + }) + .toBeGreaterThan(censusBeforeRelease.transportUnsubscribeCount) + const censusAfterFullPark = await readMultiplexCensus(client.page) + console.log( + `[tab-switch-repro] census after every pane parked and the multiplexer released: ${JSON.stringify(censusAfterFullPark)}` + ) + // Setup invariant, not a claim about the bug: the reveal below has to pay a fresh + // dial, or Arm B degrades into "reveal a tab whose transport is still up". + expect( + censusAfterFullPark.transportUnsubscribeCount, + 'Arm B precondition: the terminal multiplexer must have been released before the stalled reveal' + ).toBeGreaterThan(censusBeforeRelease.transportUnsubscribeCount) + + await selectClientTab(client.page, worktreeId, target.webTabId) + const stalled = await recordRevealTimeline( + client.page, + target.webTabId, + client.environmentId, + 'READY', + 40_000 + ) + console.log( + `[tab-switch-repro] stalled reveal: painted=${String(stalled.paintedAtMs)} banner=${stalled.sawBanner} nullStatus=${stalled.sawNullStatus} timeline=${JSON.stringify(stalled.timeline)}` + ) + console.log( + `[tab-switch-repro] dials at the hop: accepted=${link.acceptedConnectionCount()} stalled=${link.stalledConnectionCount()}` + ) + + // ---- Arm B2: the reporter's "reconnecting" state. Sever the established flows + // once while new dials still hang, so the shared-control socket enters + // `reconnecting` and the status recheck ladder arms. What the store does with + // the resulting failed status.get is the writer under test: a live verdict must + // not become `status: null` because the client could not ask. + const generationBeforeFlap = ( + await observePane(client.page, target.webTabId, client.environmentId, 0) + ).connectionGeneration + const dropped = link.dropEstablished() + const flapped = await recordRevealTimeline( + client.page, + target.webTabId, + client.environmentId, + 'READY', + 45_000 + ) + console.log( + `[tab-switch-repro] link flap under stall-new: dropped=${dropped} banner=${flapped.sawBanner} nullStatus=${flapped.sawNullStatus} timeline=${JSON.stringify(flapped.timeline)}` + ) + + // ---- Arm C: restore the link and measure time-to-recovery. + link.setMode('pass') + // Drive the documented recovery path. A pane whose attach timed out parks a retry that + // arms no timer (#19872): it waits for online/resume/manual Reconnect, so within a bounded + // budget the pane only comes back when one of those fires. Dispatching 'online' is exactly + // that trigger — the same one a real network-return raises — so this arm exercises genuine + // recovery, not the ~180s latch. + await client.page.evaluate(() => window.dispatchEvent(new Event('online'))) + const recovered = await recordRevealTimeline( + client.page, + target.webTabId, + client.environmentId, + 'READY', + 30_000 + ) + console.log( + `[tab-switch-repro] recovery after link restored: painted=${String(recovered.paintedAtMs)} timeline=${JSON.stringify(recovered.timeline)}` + ) + console.log( + `[tab-switch-repro] census after recovery: ${JSON.stringify(await readMultiplexCensus(client.page))}` + ) + console.log(`[tab-switch-repro] target sink: ${readSink(target.sinkPath).slice(0, 200)}`) + const generationAfterRecovery = ( + await observePane(client.page, target.webTabId, client.environmentId, 0) + ).connectionGeneration + console.log( + `[tab-switch-repro] connection generation across the flap: ${String(generationBeforeFlap)} -> ${String(generationAfterRecovery)}` + ) + + // Writer contract (#19647), the fix under test: a status.get that could not reach the host is + // unverifiable, so the recorded live verdict survives the transport fault (no null published) + // and recovery is not a second connection (the connection generation never advances). These + // are the hard gate; both fail on the unfixed writer. + expect( + flapped.sawNullStatus, + 'a failed status.get over a stalled link published status: null over a live verdict (#19647)' + ).toBe(false) + expect( + generationAfterRecovery, + 'recovery advanced the connection generation, so the session-tabs mirror was rebuilt (#19647)' + ).toBe(generationBeforeFlap) + // The fix keeps the environment revivable rather than retiring it: the pane is never disposed + // and its host is never dropped from the mirror targets, so a later trigger can bring it back. + const finalObservation = await observePane( + client.page, + target.webTabId, + client.environmentId, + 0 + ) + expect( + finalObservation.mounted, + 'the revealed pane was disposed instead of kept revivable' + ).toBe(true) + // Recovery TIMELINE is diagnostic, NOT a pass/fail gate: whether the pane actually repaints + // within budget depends on the un-park latch (#19872) and the multiplexer cold-handshake + // (#19871), both filed separately and out of scope here. A green run is not a clean bill of + // health — read the [tab-switch-repro] timeline above. This value is unchanged by this PR + // (the unfixed writer left it null too), so it is logged, not asserted. + console.log( + `[tab-switch-repro] recovery-contract observation (gated on #19872/#19871): painted=${String(recovered.paintedAtMs)}` + ) + } finally { + link.setMode('pass') + for (const terminal of createdTerminals) { + await callEnvironment(client.page, client.environmentId, 'terminal.closeTab', { + terminal + }).catch(() => undefined) + } + await client.dispose() + await link.close() + } +}) diff --git a/tests/e2e/sidebar-runtime-host-disconnected-glyph.spec.ts b/tests/e2e/sidebar-runtime-host-disconnected-glyph.spec.ts new file mode 100644 index 00000000000..08d202214c8 --- /dev/null +++ b/tests/e2e/sidebar-runtime-host-disconnected-glyph.spec.ts @@ -0,0 +1,124 @@ +import type { Locator, Page, TestInfo } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +const ENVIRONMENT_ID = 'e2e-remote-host' +const HOST_LABEL = 'Remote Mac' + +type RecordedState = 'no-entry' | 'probed-unreachable' | 'probed-reachable' + +/** Put the seeded worktree's repo on a runtime host and record one status verdict for it. */ +async function seedRuntimeHost(page: Page, recorded: RecordedState): Promise { + return page.evaluate( + ({ environmentId, hostLabel, recorded }) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const worktree = Object.values(state.worktreesByRepo).flat()[0] + if (!worktree) { + throw new Error('no seeded worktree to place on a runtime host') + } + state.setRuntimeEnvironments([ + { + id: environmentId, + name: hostLabel, + createdAt: 1, + updatedAt: 1, + lastUsedAt: null, + runtimeId: 'e2e-runtime', + endpoints: [ + { id: 'ws', kind: 'websocket', label: 'WebSocket', endpoint: 'ws://127.0.0.1:1' } + ], + preferredEndpointId: 'ws' + } + ]) + store.setState({ + repos: state.repos.map((repo) => + repo.id === worktree.repoId + ? { ...repo, connectionId: undefined, executionHostId: `runtime:${environmentId}` } + : repo + ) + }) + if (recorded === 'probed-unreachable') { + state.setRuntimeEnvironmentStatus(environmentId, { status: null, checkedAt: Date.now() }) + } + if (recorded === 'probed-reachable') { + state.setRuntimeEnvironmentStatus(environmentId, { + status: { + runtimeId: 'e2e-runtime', + rendererGraphEpoch: 1, + graphStatus: 'ready', + authoritativeWindowId: 1, + desktopWindowStatus: 'available', + liveTabCount: 0, + liveLeafCount: 0 + }, + checkedAt: Date.now() + }) + } + return worktree.id + }, + { environmentId: ENVIRONMENT_ID, hostLabel: HOST_LABEL, recorded } + ) +} + +async function captureCard(card: Locator, testInfo: TestInfo, name: string): Promise { + const shot = testInfo.outputPath(name) + await card.screenshot({ path: shot, animations: 'disabled' }) + await testInfo.attach(name, { path: shot, contentType: 'image/png' }) +} + +test.describe('sidebar runtime host glyph', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + }) + + // Why: an absent entry means "not probed yet". Painting it destructive made every + // remote card red and dimmed between launch and the first probe answering. + test('does not call a runtime host disconnected before its first probe answers', async ({ + orcaPage + }, testInfo) => { + await seedRuntimeHost(orcaPage, 'no-entry') + const card = orcaPage.locator(`[data-worktree-card-surface="true"]`).first() + await expect(card).toBeVisible() + + // Captured before the assertions so a regression run still yields the evidence image. + await captureCard(card, testInfo, 'runtime-host-glyph-before-probe.png') + + await expect(card.locator('svg.lucide-server')).toBeVisible() + await expect(card.locator('svg.lucide-server-off')).toHaveCount(0) + await expect(card).not.toHaveClass(/opacity-60/) + }) + + // The deliberate no-change case: a probe actually reported this host unreachable. + test('still marks a runtime host disconnected once a probe finds it unreachable', async ({ + orcaPage + }, testInfo) => { + await seedRuntimeHost(orcaPage, 'probed-unreachable') + const card = orcaPage.locator(`[data-worktree-card-surface="true"]`).first() + await expect(card).toBeVisible() + + await captureCard(card, testInfo, 'runtime-host-glyph-disconnected.png') + + await expect(card.locator('svg.lucide-server-off')).toBeVisible() + await expect(card).toHaveClass(/opacity-60/) + }) + + test('clears the disconnected glyph when a later probe reaches the host', async ({ + orcaPage + }, testInfo) => { + await seedRuntimeHost(orcaPage, 'probed-unreachable') + const card = orcaPage.locator(`[data-worktree-card-surface="true"]`).first() + await expect(card.locator('svg.lucide-server-off')).toBeVisible() + + await seedRuntimeHost(orcaPage, 'probed-reachable') + await captureCard(card, testInfo, 'runtime-host-glyph-recovered.png') + + await expect(card.locator('svg.lucide-server')).toBeVisible() + await expect(card.locator('svg.lucide-server-off')).toHaveCount(0) + await expect(card).not.toHaveClass(/opacity-60/) + }) +}) From ef969b8bca3b4163ac6edf3e99c6b41063495a3b Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:47:58 -0700 Subject: [PATCH 20/21] fix(runtime): an outage is not a handle-gap verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-pane handle-gap wait releases at a 15s deadline and records that expiry as a verdict, which authorises the sleeping-agent resume. The connection generation was the only thing voiding that verdict, and a plain disconnect never advances it — runtime-status.ts advances on the reconnect, under a new runtime id. So a network drop mid-turn expired the wait with a generation that still matched, and the replay forked a second `--resume` onto the transcript the host was still writing: #19735 through the disconnect door. Suppress the verdict while the client positively knows it is out of contact, reusing the shared runtime-host connection derivation. The waiter still releases and re-parks, so contact returning gets a full fresh budget and the pane is still decided on real silence. Not redundant with the landed-handle drain that follows this commit, nor with the read-time pane identity from adv2-skew (cdafc90d8f9). Mutation on the composed tree gives three disjoint kills: dropping this guard fails only "does not turn an outage into a verdict"; dropping the landed-handle drain fails only the two landed-handle cases; forcing this guard always-true fails 16 across every suite. Three guards, three holes. --- .../lib/host-mirror-handle-gap-resume.test.ts | 51 +++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 34 ++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) 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 3d5e2625944..5d77a679caf 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 @@ -140,6 +140,22 @@ function seedActiveSleepingRecord(worktreeId: string): string { return seedActiveSleepingRecordFor(worktreeId, WEB_TAB_ID, LEAF_ID, 'handle-gap-session') } +/** A recorded status entry whose runtime answered nothing: the shape a dropped link leaves behind. */ +function setRuntimeEnvironmentDisconnectedForTests(environmentId: string): void { + useAppStore.setState({ + runtimeStatusByEnvironmentId: new Map(useAppStore.getState().runtimeStatusByEnvironmentId).set( + environmentId, + { status: null } as never + ) + } as never) +} + +function clearRuntimeEnvironmentStatusEntryForTests(environmentId: string): void { + const next = new Map(useAppStore.getState().runtimeStatusByEnvironmentId) + next.delete(environmentId) + useAppStore.setState({ runtimeStatusByEnvironmentId: next } as never) +} + describe('resume across the mirror handle gap', () => { beforeEach(() => { vi.useFakeTimers() @@ -332,6 +348,41 @@ describe('resume across the mirror handle gap', () => { expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) }) + // The journey: the network drops mid-turn on a paired runtime. Nothing is unpaired and no + // reconnect has happened, so the connection generation has not moved — runtime-status.ts + // advances it on the *reconnect*, under a new runtime id. The deadline therefore fires with a + // generation that still matches, and its silence is about the outage, not about the host. A + // verdict recorded there resumes the agent the host is still running (#19735 through the + // disconnect door, docs/reference/ssh-execution-boundary.md). + it('does not turn an outage into a verdict when the environment dropped mid-park', () => { + 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) + setRuntimeEnvironmentDisconnectedForTests(RUNTIME_ENV_ID) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + const during = useAppStore.getState() + expect(during.sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + expect(Object.keys(during.automaticAgentResumeClaimsByTabId)).toHaveLength(0) + expect((during.tabsByWorktree[worktree.id] ?? []).map((tab) => tab.id)).toEqual([WEB_TAB_ID]) + // Held, not abandoned: something is still armed to decide once contact returns. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + + // Contact returns and the host still publishes no handle for the pane. That silence IS + // evidence, so the next full budget decides — a hold that outlives the outage would be the + // latch-that-never-releases defect this module exists to avoid. + clearRuntimeEnvironmentStatusEntryForTests(RUNTIME_ENV_ID) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + const after = useAppStore.getState() + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + expect(Object.keys(after.automaticAgentResumeClaimsByTabId)).toHaveLength(1) + }) + it('releases only the pane whose handle landed when two panes share the environment', () => { const worktree = makeRuntimeOwnedWorktree() seedMirroredWorkspace(worktree) 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 2e884fc43f0..e1b7c431bbe 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -2,6 +2,10 @@ 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' import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id' +import { + isDisconnectedRuntimeHostState, + runtimeHostConnectionStateForEntry +} from '@/runtime/runtime-host-connection-state' /** * Per-pane park for the frame between a host's tab rows and its PTY handles. @@ -149,6 +153,27 @@ function liveTabIds(): Set { return tabIds } +/** + * True only when the client positively knows it is out of contact — the link dropped, or its + * replacement is still being established. + * + * Why not `isConnectedRuntimeHostState`: that reads a host nobody has probed yet as not + * connected, and a never-probed host is not the outage this guards. Narrowing to the two states + * an outage actually produces keeps the guard to the case where silence provably means "we could + * not ask" rather than "the host had nothing to say". + * + * Why this and not the connection generation: a plain disconnect leaves the generation where it + * was — runtime-status.ts advances it on the *reconnect*, under a new runtime id — so a wait that + * expires mid-outage is indistinguishable, to the generation guard, from one that expired on a + * healthy connection. + */ +function environmentContactIsLost(environmentId: string): boolean { + const connectionState = runtimeHostConnectionStateForEntry( + useAppStore.getState().runtimeStatusByEnvironmentId.get(environmentId) + ) + return isDisconnectedRuntimeHostState(connectionState) || connectionState === 'reconnecting' +} + function recordExpiredWait(environmentId: string, key: string): void { const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) // TWO rules with DIFFERENT scopes, deliberately. Flattening them to one scope is wrong either @@ -309,9 +334,16 @@ export function parkUntilHostMirrorHandleLands( // 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. + // + // Why contact is checked too: an environment that dropped mid-park publishes nothing, + // so the deadline measures the outage rather than the host. Loss of contact is never + // evidence about a process (docs/reference/ssh-execution-boundary.md), and a verdict + // recorded here authorizes the resume that forks the agent the host is still running. + // The generation cannot stand in for it — a plain disconnect never advances it. if ( + !environmentContactIsLost(environmentId) && waitersByPane.get(key)?.generation === - getRuntimeEnvironmentConnectionGeneration(environmentId) + getRuntimeEnvironmentConnectionGeneration(environmentId) ) { recordExpiredWait(environmentId, key) } From 51ebc047ce1a65222f0b01442d00b23bffa12940 Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 11 Sep 2026 02:13:33 -0700 Subject: [PATCH 21/21] fix(runtime): state the writer guard in #20003's vocabulary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #20003 moved failed-status retries into the connection's status owner and split the vocabulary this PR's guard was written against: the store entry's `status` now means "verified on the current socket", while the owner's `snapshot.status` keeps the last verdict across an unverifiable probe. The guard itself is unchanged and still correct — it is the only writer for a `status.get` that threw, where there is no snapshot and no retry ladder — but its comment described a recheck ladder that no longer exists. The e2e flap arm's `sawNullStatus` gate does not survive that split. Under #20003 a dropped socket publishes `verification: 'unavailable'` and the renderer derives `entry.status = null` from it by design, and that null is what drives the session-tabs mirror's own rebuild once the socket returns. Asserting it stays non-null would contradict the recovery the same arm measures, so it is logged rather than gated. The connection-generation and pane-mounted assertions, which are the actual #19647 contract, are unchanged. --- .../src/store/slices/runtime-status.ts | 12 +++++----- ...mote-terminal-tab-switch-reconnect.spec.ts | 22 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index 1dd012cb317..b70fe2da1d8 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -292,12 +292,12 @@ export const createRuntimeStatusSlice: StateCreator ${String(generationAfterRecovery)}` ) - // Writer contract (#19647), the fix under test: a status.get that could not reach the host is - // unverifiable, so the recorded live verdict survives the transport fault (no null published) - // and recovery is not a second connection (the connection generation never advances). These - // are the hard gate; both fail on the unfixed writer. - expect( - flapped.sawNullStatus, - 'a failed status.get over a stalled link published status: null over a live verdict (#19647)' - ).toBe(false) + // Writer contract (#19647), the fix under test: recovery is not a second connection, so the + // connection generation never advances. This fails on the unfixed writer. expect( generationAfterRecovery, 'recovery advanced the connection generation, so the session-tabs mirror was rebuilt (#19647)' ).toBe(generationBeforeFlap) - // The fix keeps the environment revivable rather than retiring it: the pane is never disposed - // and its host is never dropped from the mirror targets, so a later trigger can bring it back. + // `sawNullStatus` is DIAGNOSTIC here, not a gate. #20003 split the vocabulary: the store entry's + // `status` now means "verified on the current socket" and the owner's `snapshot.status` holds the + // last verdict, so `entry.status === null` during an outage no longer means the client declared + // the host gone. It also drives the mirror's own rebuild after the socket returns, so asserting + // it stays non-null would contradict the recovery this arm measures. + console.log( + `[tab-switch-repro] entry.status nulled during the flap (expected post-#20003): ${String(flapped.sawNullStatus)}` + ) + // The fix keeps the environment revivable rather than retiring it: the pane is never disposed, + // so a later trigger can bring it back. const finalObservation = await observePane( client.page, target.webTabId,