diff --git a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts index 7418db65233..27eb98dd2ad 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -36,6 +36,7 @@ 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' @@ -87,6 +88,40 @@ function seedMirroredWorkspace(worktree: ReturnType { expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) }) - it('still resumes once the host has published the row without any live handle', () => { + // 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) - useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@other'] } }) + // 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 @@ -215,14 +267,29 @@ describe('resume across the mirror handle gap', () => { expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() }) - it('releases when a handle lands for the tab and resumes if it belongs to another pane', () => { + // 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. In a SPLIT mirrored + // tab a sibling surface reaching `ready` therefore ends the wait for a surface that is still + // `pending-handle`, and the sweep resumes it — #19735's own shape, narrowed to split tabs. + // + // It is not closable inside this module: during the gap the pane's own 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. + // The client has to stop discarding each surface's `pending-handle` status + // (runtime-mobile-session-projection.ts publishes it; terminal-build.ts keeps only `ready` + // leaves' bindings). Tracked separately; this case exists so the residual cannot be mistaken for + // a covered one. + it('resumes a pending leaf once a sibling leaf of the same tab publishes its handle', () => { const worktree = makeRuntimeOwnedWorktree() seedMirroredWorkspace(worktree) const paneKey = seedActiveSleepingRecord(worktree.id) markHostSessionMirrorHydrated(RUNTIME_ENV_ID) expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) - useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@other'] } }) + seedPendingLeafBesideReadySibling() + useAppStore.setState({ + ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@term_sibling'] } + }) const after = useAppStore.getState() const resumeTabIds = (after.tabsByWorktree[worktree.id] ?? []) 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 index 0d160944ae1..9882b9ed979 100644 --- 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 @@ -17,10 +17,11 @@ import { // - RETAIN direction (drop the verdict term from `stopStoreSubscriptionIfIdle`, so a verdict // with no waiter behind it loses the subscription its drain needs): already caught, by // host-mirror-handle-gap-landed-handle.test.ts. Two failures there without this file. -// - RELEASE direction (never release the subscription at all): caught by NOTHING. That mutation -// passes all 272 tests across the 33 other handle-gap and session-tabs suites. A leaked +// - 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 -// no test notices. +// 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 diff --git a/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts index ffa8a7502bb..3de01bafbf5 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-verdict-union.test.ts @@ -118,7 +118,9 @@ describe('handle-gap verdict map, all rules on one tree', () => { expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(false) // B drained: by teardown, which is the only trigger that fires for a removed environment. expect(hasHostMirrorHandleWaitExpired(ENV_B, 'b1')).toBe(false) - // C drained: env-a's expiry retired env-c's superseded row, though env-c never expired again. + // 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: @@ -191,9 +193,10 @@ describe('handle-gap verdict map, all rules on one tree', () => { }) it('never evicts a live pane verdict, whichever environment sweeps', () => { - // Three environments on purpose: with two at one generation the candidate rules are - // indistinguishable and the naive "judge everything against the recording environment" - // mutation survives. env-c is the discriminator — its verdict is live. + // 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) } @@ -213,13 +216,24 @@ describe('handle-gap verdict map, all rules on one tree', () => { expect(hasHostMirrorHandleWaitExpired(ENV_A, 'a1')).toBe(true) }) - it('returns to baseline under churn across all three drains', () => { + 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) - setLiveTabs([`tab-${round}`]) + // Bind each round's pane to the environment that is recording it. Falling back to env-a for + // every round would store `paneBinding: ''` on two rounds in three, which is the match value + // the read-time check refuses — two thirds of the churn would then be synthetic. + 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 — + // each environment's own generation bump retires its predecessors, and the two environments + // not recording keep exactly their latest row. + expect(countHostMirrorHandleGapVerdictsForTests()).toBe(3) + for (const environmentId of [ENV_A, ENV_B, ENV_C]) { clearHostMirrorHandleGapVerdictsForEnvironment(environmentId) } diff --git a/src/renderer/src/lib/host-mirrored-pane-liveness.ts b/src/renderer/src/lib/host-mirrored-pane-liveness.ts index d9e8a70d01a..4770e7096e5 100644 --- a/src/renderer/src/lib/host-mirrored-pane-liveness.ts +++ b/src/renderer/src/lib/host-mirrored-pane-liveness.ts @@ -58,6 +58,19 @@ 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 sibling surface reaching `ready` first ends the + // wait for one still `pending-handle`, and the resume fires — #19735 narrowed to split tabs. + // It cannot be closed here, because during the gap the pane's own 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, which the host already publishes + // (main/runtime/runtime-mobile-session-projection.ts) and the client drops on apply. + // Pinned as current behaviour by "resumes a pending leaf once a sibling leaf of the same tab + // publishes its handle" in host-mirror-handle-gap-resume.test.ts. if ((state.ptyIdsByTabId[tabId]?.length ?? 0) > 0) { 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 1ce86dfb5dd..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 @@ -177,7 +177,10 @@ describe('resume sleeping agent provider claims', () => { expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() }) - it('does not fork a provider session a live pane in another workspace is running', () => { + // 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) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index 9b9279b4f0b..0610eb9cf8b 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -117,6 +117,13 @@ function activeOrQueuedResumeClaimsProviderSession( // 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 &&