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..ab67ff19931 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-drain.test.ts @@ -0,0 +1,211 @@ +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. +// +// Both release paths are here on purpose: the store-write drain and the deadline both funnel into +// `releaseWaiter`, and a guard added to one is easy to forget on the other. One mutation — +// rethrowing from that catch — kills the first and last cases together, which is the point: they +// are the two entry points, not two behaviours. The two middle cases are about what one replay can +// do to the pane queued behind it while the drain is mid-loop, and neither involves a throw. + +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 { + // 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. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + ptyIdsByTabId: {}, + tabsByWorktree: { + [WORKTREE_ID]: [ + { 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) +} + +/** The host publishes both panes' PTY handles on one frame: both waiters come due together. */ +function publishBothHandles(): void { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + ptyIdsByTabId: { + [FIRST_TAB_ID]: [`remote:${ENVIRONMENT_ID}@@term_1`], + [SECOND_TAB_ID]: [`remote:${ENVIRONMENT_ID}@@term_2`] + } + } as never) +} + +describe('host-mirror handle-gap drain', () => { + beforeEach(() => { + vi.useFakeTimers() + // The replays below throw on purpose; the module logs and swallows, which is the behaviour + // under test, so the log itself is noise. + vi.spyOn(console, 'warn').mockImplementation(() => {}) + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + seedRows() + }) + + afterEach(() => { + vi.restoreAllMocks() + resetHostMirrorHandleGapWaitsForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + useAppStore.setState(initialAppStoreState, true) + vi.useRealTimers() + }) + + // The drain runs inside `useAppStore.subscribe`, and zustand notifies listeners in a plain loop + // with no queue, so an unguarded throw from one pane's replay reaches three strangers at once: + // the `setState` that published the handle (the mirror apply, which has nothing to do with this + // pane), every sibling pane the same frame made due, and every listener registered after this + // module's. `resumeSleepingAgentSessionsForWorktree` reaches `state.createTab` with no guard of + // its own, so the throw is reachable. + it('does not let one pane’s replay throw reach the store write, its siblings, or later listeners', () => { + const siblingReplay = vi.fn() + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, FIRST_TAB_ID, () => { + throw new Error('replay blew up') + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, siblingReplay) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(2) + + // Registered after this module's subscription, so it is notified after the drain. + const laterListener = vi.fn() + const unsubscribe = useAppStore.subscribe(laterListener) + + expect(() => publishBothHandles()).not.toThrow() + unsubscribe() + + expect(siblingReplay).toHaveBeenCalledTimes(1) + expect(laterListener).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 drain is re-entrant: `resumeSleepingAgentSessionsForWorktree` reaches `createTab`, zustand + // notifies with no queue, and the nested pass drains the same map the outer loop is still + // walking. One store write must still mean one replay per pane. (Not one deadline per pane: the + // re-park after a release always takes a fresh budget, whichever way this goes — what a second + // release actually costs is running a whole worktree resume sweep again off one frame.) + it('replays a pane once per store write even when an earlier replay re-enters the drain', () => { + const siblingReplay = vi.fn(() => { + // What the real replay does when the sweep still finds the pane undecided. + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, siblingReplay) + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, FIRST_TAB_ID, () => { + // Only `tabsByWorktree` and `ptyIdsByTabId` re-enter: the subscription's slice guard drops + // everything else, so a `clearSleepingAgentSession` write would never reach the drain. + useAppStore.setState({ tabsByWorktree: { ...useAppStore.getState().tabsByWorktree } }) + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, siblingReplay) + + publishBothHandles() + + expect(siblingReplay).toHaveBeenCalledTimes(1) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + expect(vi.getTimerCount()).toBe(1) + }) + + // The other way the snapshot goes stale, and the one object identity cannot see: re-parking a + // STILL-PARKED pane mutates the waiter in place, so its `worktreeId` can move between the moment + // the drain judged it retracted and the moment it releases. + // + // Staged the only way production can reach it. A replay is + // `resumeSleepingAgentSessionsForWorktree` closed over ONE worktree and re-parks only under that + // worktree, so a waiter's worktree can only move when a DIFFERENT waiter's replay sweeps the + // workspace the row was adopted into. Here the second tab has already been re-keyed onto the + // canonical id — `canonicalizeTerminalSessionWorktreeId` re-keys `tabsByWorktree` and leaves the + // sleeping record naming the old one — so the first pane's sweep legitimately owns it, while the + // live waiter is still filed under the id its record named. + it('does not release on retraction evidence a mid-drain adoption has already made stale', () => { + const adoptedReplay = vi.fn() + const ADOPTING_WORKTREE_ID = 'repo-1::/workspace/adopted' + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + tabsByWorktree: { + [ADOPTING_WORKTREE_ID]: [ + { id: FIRST_TAB_ID, title: 'one' }, + { id: SECOND_TAB_ID, title: 'two' } + ] + } + } as never) + // Parked first so the drain reaches it first: `Map` preserves insertion order, and this pane's + // replay is what makes the next entry's snapshot verdict stale. If that order ever inverted the + // test would fail rather than pass quietly — the second pane would release before the adoption. + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, ADOPTING_WORKTREE_ID, FIRST_TAB_ID, () => { + // The sweep for the adopting workspace, re-parking the pane it now owns. No store write: a + // sweep that parks every record it finds launches nothing, which is exactly this case. + parkUntilHostMirrorHandleLands( + ENVIRONMENT_ID, + ADOPTING_WORKTREE_ID, + SECOND_TAB_ID, + adoptedReplay + ) + }) + parkUntilHostMirrorHandleLands(ENVIRONMENT_ID, WORKTREE_ID, SECOND_TAB_ID, adoptedReplay) + + // The frame that starts the drain. The second tab is absent from the worktree its waiter is + // filed under, so the snapshot reads retraction — evidence the adoption above makes obsolete + // before the release loop reaches it. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + ptyIdsByTabId: { [FIRST_TAB_ID]: [`remote:${ENVIRONMENT_ID}@@term_1`] } + } as never) + + expect(adoptedReplay).not.toHaveBeenCalled() + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + }) + + // The other entry into `releaseWaiter`. Here the throw would escape the timer callback instead of + // the store write, and the verdict must still be recorded — a pane whose replay failed has still + // used up its budget, and dropping the verdict re-parks it on a fresh one forever. + 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-landed-handle.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts new file mode 100644 index 00000000000..c031ec1b052 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-landed-handle.test.ts @@ -0,0 +1,128 @@ +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 { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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) + + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + tabsByWorktree: { 'wt-new': [{ id: TAB_ID, title: 't', ptyId: null }] } + } as unknown as AppState) + expect(replayAfterAdoption).not.toHaveBeenCalled() + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ tabsByWorktree: {} } as unknown as AppState) + expect(replayAfterAdoption).toHaveBeenCalledTimes(1) + }) +}) 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..2101451b32e --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -0,0 +1,481 @@ +import path from 'node:path' +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' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { + markHostSessionMirrorHydrated, + resetHostSessionMirrorHydrationForTests +} from '@/runtime/host-session-mirror-hydration' +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. +// 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 SIBLING_LEAF_ID = '44444444-4444-4444-8444-444444444444' +const SECOND_TAB_ID = 'web-terminal-host-tab-2' +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: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + [worktree.id]: [{ id: WEB_TAB_ID, title: 'Claude', ptyId: null } as never] + }, + terminalLayoutsByTabId: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + [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: {} + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState(state as AppState) +} + +/** + * The first frame of a SPLIT mirrored tab whose sibling surface is already `ready` while the + * record's own surface is still `pending-handle` and has never been bound. + * + * Why "never been bound" and not "went pending": `retainPendingTerminalBindings` + * (web-session-tabs-sync/terminal-build.ts) carries a pending surface's PRIOR binding forward, so a + * leaf that has ever held a handle keeps it across the gap and this shape cannot arise from one. It + * needs a cold start or a re-pair — no existing layout to retain from. + * + * No wait is armed here for a separate reason: `ptyIdsByTabId[tab]` is non-empty, so the pane reads + * decidable at the tab-granular gate before the per-pane wait is ever considered. That is the + * residual, and it is decided on the first frame. + * + * And not "the tab published a handle no leaf is bound to": `ptyIdsByTabId[tab]` is built from the + * very map written to `terminalLayoutsByTabId[tab].ptyIdsByLeafId`, so those two cannot disagree + * about which PTY ids exist. + */ +function seedSplitTabWithOnlySiblingReady( + worktree: ReturnType +): void { + seedMirroredWorkspace(worktree) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + terminalLayoutsByTabId: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + [WEB_TAB_ID]: { + root: { + type: 'split', + direction: 'row', + first: { type: 'leaf', leafId: LEAF_ID }, + second: { type: 'leaf', leafId: SIBLING_LEAF_ID } + }, + activeLeafId: SIBLING_LEAF_ID, + expandedLeafId: null, + // Only the ready sibling is bound; the record's leaf has never held a handle. + ptyIdsByLeafId: { [SIBLING_LEAF_ID]: 'remote:env-handle-gap@@term_sibling' } + } as never + }, + ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@term_sibling'] } + } as never) +} + +/** A second published mirrored row in the same environment, with its own leaf binding. */ +function seedSecondMirroredPane(worktreeId: string): void { + const before = useAppStore.getState() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + tabsByWorktree: { + [worktreeId]: [ + ...(before.tabsByWorktree[worktreeId] ?? []), + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the literal names every field this suite reads; the cast only supplies the rest of the declared shape. + { id: SECOND_TAB_ID, title: 'Claude 2', ptyId: null } as never + ] + }, + terminalLayoutsByTabId: { + ...before.terminalLayoutsByTabId, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + [SECOND_TAB_ID]: { + root: { type: 'leaf', leafId: SECOND_LEAF_ID }, + activeLeafId: SECOND_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [SECOND_LEAF_ID]: 'remote:env-handle-gap@@term_2' } + } as never + } + } as never) +} + +/** The capture the reported flow produces: recorded mid-turn, so it is active work, not history. */ +function seedActiveSleepingRecordFor( + worktreeId: string, + tabId: string, + leafId: string, + sessionId: string +): string { + const paneKey = makePaneKey(tabId, leafId) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + sleepingAgentSessionsByPaneKey: { + ...useAppStore.getState().sleepingAgentSessionsByPaneKey, + [paneKey]: { + paneKey, + tabId, + worktreeId, + agent: 'claude', + providerSession: { key: 'session_id', id: sessionId }, + connectionId: null, + prompt: '', + state: 'working', + capturedAt: 1000, + updatedAt: 1000, + terminalTitle: 'Claude', + origin: 'live' + } + } + } as never) + return paneKey +} + +function seedActiveSleepingRecord(worktreeId: string): string { + return seedActiveSleepingRecordFor(worktreeId, WEB_TAB_ID, LEAF_ID, 'handle-gap-session') +} + +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', () => { + 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() + // And something is armed to decide it — a hold with nothing armed is the defect, not the fix. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + }) + + // The counterweight to the park, and the reason the hydration short-circuit could not simply be + // dropped: a pane with nothing outstanding must still resume. Here no leaf of the published row + // binds a PTY this environment minted, so there is no handle on its way and no wait to arm — + // parking would be the latch-that-never-releases defect, since mirror settlement has already run + // and will not replay the sweep a second time. + it('still resumes a published row no leaf of which binds this environment', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedActiveSleepingRecord(worktree.id) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + terminalLayoutsByTabId: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + [WEB_TAB_ID]: { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: {} + } as never + } + } as never) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(1) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + }) + + // 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() + }) + + // KNOWN RESIDUAL, pinned as current behaviour rather than as desired behaviour. The gate this + // wait sits behind is tab-granular (`host-mirrored-pane-liveness.ts`: any published handle for + // the tab makes the pane decidable), while everything below it is leaf-aware. A split tab whose + // sibling surface is `ready` while this one is still `pending-handle` therefore reads decidable, + // no wait is armed at all, and the sweep resumes a pane the host has not answered for — #19735's + // own shape, narrowed to a split tab's first frame. + // + // It is not closable inside this module: such a leaf has NO binding, and the binding is what + // names the pane in a verdict, so a leaf-keyed wait has nothing to key on. It needs the + // per-surface `pending-handle` status the host publishes (runtime-mobile-session-projection.ts) + // and the client consumes without retaining per leaf. Tracked separately; this case exists so + // the residual cannot be mistaken for a covered one. + it('resumes a pending leaf when a sibling leaf of the same tab holds the only handle', () => { + const worktree = makeRuntimeOwnedWorktree() + seedSplitTabWithOnlySiblingReady(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(1) + // The residual in one assertion: nothing was ever parked for this pane. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + + 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() + }) + + // 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. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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-subscription-lifetime.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts new file mode 100644 index 00000000000..9882b9ed979 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-subscription-lifetime.test.ts @@ -0,0 +1,155 @@ +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 else. With +// `stopStoreSubscriptionIfIdle` neutered, the three cases below are the only failures in the +// handle-gap and session-tabs tree: 326 tests across the other 37 files still pass. A leaked +// subscription rescans every parked pane on every store write for the life of the session and +// nothing else 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] ?? [] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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 { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the subscriber is invoked with the store state pair; the narrowed listener type is what this suite asserts on. + 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) + }) +}) 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..1e769707fa1 --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-teardown.test.ts @@ -0,0 +1,130 @@ +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 { + // 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 state = useAppStore.getState() + const published = state.tabsByWorktree[WORKTREE_ID] ?? [] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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, () => {}) + 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() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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-verdict-union.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts new file mode 100644 index 00000000000..61fae8cbe3d --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts @@ -0,0 +1,247 @@ +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 read-time pane identity, NOT a prune + * + * 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' +const ENV_B = 'env-union-b' +const ENV_C = 'env-union-c' +const WORKTREE = 'repo-1::wt-union' +const initialAppStoreState = useAppStore.getState() + +/** 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 } + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + useAppStore.setState({ + tabsByWorktree: { [WORKTREE]: tabIds.map((id) => ({ id, title: id, ptyId: null })) }, + terminalLayoutsByTabId: layouts, + 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, 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. + 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: env-c reconnected at :109, so this read is false on the read-time generation gate alone + // and says nothing about whether the drain ran. The drain is what the COUNT below proves — it + // is the only assertion here that distinguishes "retired" from "stranded but unreachable". + expect(hasHostMirrorHandleWaitExpired(ENV_C, 'c1')).toBe(false) + + // 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. + 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', () => { + // env-b is the discriminator, and it is the only assertion here that is not a control: its row + // goes absent at the moment env-a records, so widening the tab-death rule past the recording + // environment deletes a verdict whose pane is merely mid-republish. env-a's and env-c's rows + // are published throughout and hold under every candidate rule. + 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('holds the verdict map at one live row per environment under churn', () => { + for (let round = 0; round < 300; round += 1) { + const environmentId = [ENV_A, ENV_B, ENV_C][round % 3]! + setRuntimeEnvironmentConnectionGenerationForTests(environmentId, round + 1) + // Bind each round's pane to the environment that is recording it. No assertion here reads + // `paneBinding` and neither prune rule inspects it, so this changes no outcome — but falling + // back to env-a stored the empty match value on two rounds in three, and a fixture that + // models a state the production park path cannot reach is not churn worth running. + setLiveTabs([`tab-${round}`], { [`tab-${round}`]: `remote:${environmentId}@@term_${round}` }) + parkAndExpire(environmentId, `tab-${round}`) + } + + // The assertion the loop exists for, and it has to come BEFORE teardown: the clear below + // deletes every key in the map by construction, so `toBe(0)` after it holds whether the drains + // work or are deleted outright. 300 expiries must leave one live verdict per environment. + // What this pins is that the prune loop runs AT ALL — without it the map holds 300. It does + // not isolate which rule prunes: with one tab live per round the generation rule and the + // tab-death rule each sweep the recording environment's predecessor on their own, so removing + // either alone still reads 3. The generation rule is separately isolated by the count in + // `handles all four orphan classes simultaneously`, where only it can retire env-c's row. + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(3) + + 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 new file mode 100644 index 00000000000..e2d3a58367d --- /dev/null +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -0,0 +1,405 @@ +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. + * + * 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. + * + * 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 + +type HandleGapWaiter = { + worktreeId: string + 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 +} + +type HandleGapStoreState = Pick< + ReturnType, + 'ptyIdsByTabId' | 'tabsByWorktree' +> + +const waitersByPane = new Map() +/** + * Connection generation whose wait already expired for the pane. + * + * 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. + * - 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. + * - 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. + * + * 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. + * + * KNOWN LEAK, deliberately not drained: a verdict whose row the host retracts for good on an + * environment that stays paired and never records again. The generation has not moved, teardown + * never fires, the retracted row can never publish a handle, and the tab-death rule only runs from + * inside a later recording. That entry outlives the session, and because + * `stopStoreSubscriptionIfIdle` counts verdicts, so does the store subscription — a no-op rescan on + * every write to the two `HandleGapStoreState` slices above. It cannot answer: the STORED binding is + * non-empty, so the `''` early return below does not catch it; what does is the compare against a + * fresh `paneBindingFor`, which reads '' for a row that is gone. So it costs work, not correctness. + * The obvious drain — drop a verdict whose binding no longer matches — is NOT safe: it would break + * the genuine reattach, where + * the binding goes away and comes back and the verdict must still answer + * (host-mirror-handle-gap-verdict-union.test.ts, "answers for a genuine reattach"). + * + * 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 + /** 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}` +} + +/** + * 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 ( + 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) + ) +} + +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) + // 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, 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 (stale.generation !== 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) + } + } + // 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. + // 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 ?? '' + }) + // 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 { + // 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 + } +} + +function releaseWaiter(key: string): void { + const waiter = waitersByPane.get(key) + if (!waiter) { + return + } + clearTimeout(waiter.deadline) + waitersByPane.delete(key) + stopStoreSubscriptionIfIdle() + 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 { + 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 due: [string, HandleGapWaiter][] = [] + for (const [key, waiter] of waitersByPane) { + if (waiterIsReleased(waiter, state)) { + due.push([key, waiter]) + } + } + // TWO guards, because a replay earlier in this loop reaches `createTab` and so re-enters this + // drain through zustand, which notifies with no queue. Each guard catches a different way the + // snapshot goes stale mid-loop, and neither covers the other. + for (const [key, waiter] of due) { + // ONE: the map no longer holds the waiter this entry is about. The nested pass released it and + // its replay re-parked, so the key names a NEW waiter that this store write never judged. + // Releasing by key would replay that pane a second time off a single write. + if (waitersByPane.get(key) !== waiter) { + continue + } + // TWO: the same waiter, re-judged against the same frame. `parkUntilHostMirrorHandleLands` + // re-parks a still-parked pane by MUTATING this object — `worktreeId` moves with `run` when + // adopting an orphaned terminal re-keys the rows — so identity survives it and the verdict + // taken above can be about a workspace the waiter is no longer filed under. Releasing on that + // is retraction evidence about the wrong workspace, the defect the `existing.worktreeId` + // assignment exists to prevent. Re-judging costs nothing: a waiter that is no longer due stays + // parked, bounded by its own deadline and judged again on the next write. + // The live store and not `state`, and NO TEST CAN TELL THE DIFFERENCE — deliberately. The two + // agree on every sequence the sweep can produce: a replay's only write is `createTab`, which + // appends a freshly minted tab id, so it can neither make an absent tab id present nor touch + // `ptyIdsByTabId`. They are kept apart anyway because if they ever did diverge `state` is the + // staler one, and its error is to RELEASE a pane whose row has come back — the direction this + // module exists to refuse. Holding on possibly-stale evidence costs a frame; acting on it is + // #19735. Do not "simplify" this to `state` on the grounds that nothing fails. + if (!waiterIsReleased(waiter, useAppStore.getState())) { + continue + } + 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 + retireVerdictsWithLandedHandles(state) + releaseDueWaiters(state) + stopStoreSubscriptionIfIdle() + }) +} + +/** + * 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 + // 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) + const deadline = setTimeout(() => { + // 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, + generation, + paneBinding: paneBindingFor(tabId, environmentId), + deadline, + run + }) + startStoreSubscription() +} + +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) + } + } + // The landed-handle drain may have been the only thing holding the subscription open. + stopStoreSubscriptionIfIdle() +} + +export function countHostMirrorHandleGapVerdictsForTests(): number { + return expiredGenerationByPane.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..17e252de349 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, @@ -37,12 +58,34 @@ export function findUnhydratedHostMirrorForPane( } // Why: a published PTY handle for the tab is the mirror having spoken for it, // whatever the individual leaf's fate. + // + // TAB-GRANULAR, and everything below this line is leaf-aware — the asymmetry is a known residual, + // not an oversight. For a single-leaf tab (every agent tab Orca creates) it is exact: the mirror + // builds `ptyIdsByTabId[tab]` out of the same map it writes to the layout's `ptyIdsByLeafId` + // (web-session-tabs-sync/terminal-build.ts), so a non-empty entry means this leaf is bound and + // live. For a SPLIT mirrored tab it is not. A leaf that has ever been bound keeps its binding + // across the gap — `retainPendingTerminalBindings` carries it — so the residual needs a leaf that + // was NEVER bound, i.e. a cold start or a re-pair with no layout to retain from. There, a sibling + // surface that reaches `ready` first publishes a handle for the tab while this leaf has none, the + // pane reads decidable, and the resume fires: #19735 narrowed to a split tab's first frame. + // It cannot be closed here, because such a leaf holds no binding and the binding is what names a + // pane in a handle-gap verdict. Closing it means keeping each surface's `pending-handle` status + // per leaf, which the host already publishes + // (main/runtime/runtime-mobile-session-projection.ts) and the client consumes but does not retain. + // Pinned as current behaviour by "resumes a pending leaf when a sibling leaf of the same tab + // holds the only handle" in host-mirror-handle-gap-resume.test.ts. if ((state.ptyIdsByTabId[tabId]?.length ?? 0) > 0) { 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-provider-claim.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts index 7a79e239eb0..b0449ce92bf 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,110 @@ 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) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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() + }) + + // The load-bearing half of the pair: this is the only case that proves the live arm carries no + // workspace scope. The peer is `done` here too — a finished turn on a pane whose shell is still + // up — so "live" means the PTY, not the agent. + it('does not fork a provider session a live pane in another 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) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the seeded slice names only the store fields this suite drives; the rest of AppState keeps its defaults. + 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 e0b5b79ac6e..0610eb9cf8b 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -4,17 +4,23 @@ 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, 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' @@ -96,12 +102,44 @@ 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. + // What this trades, stated because it reads as a regression: `entry.state` is ignored, so a + // FINISHED agent whose shell is still up releases its record and will not auto-resume. That is + // the intended side of the trade, not an oversight. A live PTY is positive evidence the host + // holds the transcript, and a bare `done` row cannot be told apart from a REPL idling at its + // prompt with the process still attached. Nothing is killed: the pane, its shell and the + // transcript survive, the record was only a queued respawn, and the user can resume by hand. + // Forking the transcript is not recoverable; declining to auto-resume is. + 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 } @@ -148,27 +186,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 +267,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) 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, 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..1232681d980 --- /dev/null +++ b/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts @@ -0,0 +1,117 @@ +/** + * 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({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + repos: [ + { + id: 'repoSsh', + path: '/srv/proj', + displayName: 'repoSsh', + badgeColor: '#000', + addedAt: 0, + connectionId: TARGET_ID + } + ] as never, + worktreesByRepo: { + repoSsh: [ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture carries the fields this suite drives; the cast only supplies the rest of the declared shape. + 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) + }) +}) 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) + } } } } 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() }