From d68eee37576ec2327f16adfcd3526b6ad61fa58c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:12:26 -0700 Subject: [PATCH] fix(runtime): retire an exited terminal before its stream end (#23492) * fix(runtime): retire an exited terminal before its stream end An exit's durable retirement became asynchronous, so onPtyExit released the terminal stream before the retirement landed. A paired client answers a stream end by re-activating its pane; that activation still found the exited leaf, materialized it under the same session id, and registerPty dropped the pending retirement. The exited split pane came back as a fresh shell. The exit now stages the retirement into the in-memory session and publishes it synchronously, then notifies exit listeners, and only then makes it durable. A failed durable write is logged and left in memory for the next profile write instead of being rolled back, since the process is gone either way. This removes the pending-retirement latch and its post-await incarnation fence: there is no longer a window for them to guard. * test(runtime): a failed exit retirement still reaches disk Pins the no-rollback contract through a real Store and SQLite authority: when the retirement's own durable write fails, the in-memory retirement is carried by the next unrelated profile write, and by the app-quit flush when no other write happens. The delayed authority fixture can now fail its next write, and the acknowledged-retirement fixture reads the database a relaunch would load and models the quit flush. * test(runtime): a stream end observes the exit retirement already published The re-activation check alone passes with the listener ordering reverted, because activation awaits before its lookup. Record the session binding and publication count at the moment the exit listener fires so the ordering itself is pinned. * fix(runtime): an exit cleanup fault still ends the terminal stream * perf(runtime): exits retired together share one durable write * test(runtime): a refused staging write still retires the pane and ends the stream * refactor(runtime): describe exit retirement as staged, not durably accepted The retirement result is staged in memory before any write, and the removable-surface comment and the replacement-admission test name still described the old publish-after-durable rule. --- config/reliability-gates.jsonc | 2 +- ...-host-admitted-terminal-membership.test.ts | 2 +- ...profile-state-delayed-authority-fixture.ts | 23 +- ...retirement-publication-during-read.test.ts | 9 +- ...wledged-terminal-tab-retirement-fixture.ts | 24 +- .../orca-runtime-fit-override-listeners.ts | 1 - ...-runtime-invalidate-all-handles-for-pty.ts | 1 - src/main/runtime/orca-runtime-on-pty-exit.ts | 319 +++++++++--------- ...me-persist-terminal-surface-retirements.ts | 129 +++---- src/main/runtime/orca-runtime-register-pty.ts | 1 - .../orca-runtime-terminal-retirement.test.ts | 20 +- .../exit-retirement-activation.spec.ts | 148 ++++++++ src/main/runtime/orca-runtime.test.ts | 1 + ...rminal-retirement-async-durability.test.ts | 63 +++- 14 files changed, 472 insertions(+), 271 deletions(-) create mode 100644 src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index e100d54e17a..4e4443354fa 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -8856,7 +8856,7 @@ "Windows ConPTY, SSH-hosted PTYs, app-quit killAll, and daemon dispose retain foreground-tree-only teardown.", "A process born in the capture second is SIGTERMed but not SIGKILLed because ps cannot prove its recycled-PID identity.", "A descendant orphaned before or during root ownership loss requires the separate crash-orphan sweep and is not recovered from a stale kill-time snapshot.", - "recordTerminalSurfaceRetirement advances the INCOMING session's terminal topology revision, so a tombstoned close whose write carries a revision at or below the store's own is rebased away instead of persisting. Pre-existing for every repo already past revision 0 and unchanged by the host-admitted membership fix, which only makes more repos reach that state sooner; the authoritative close path (persistTerminalSurfaceRetirements) computes from the store's session and is unaffected. Known, unaddressed, and deliberately out of scope here: the safe correction is per-tab rather than per-worktree rebase arbitration, because raising the incoming revision wholesale would re-open the host-tab loss whenever a close coincides with a host create.", + "recordTerminalSurfaceRetirement advances the INCOMING session's terminal topology revision, so a tombstoned close whose write carries a revision at or below the store's own is rebased away instead of persisting. Pre-existing for every repo already past revision 0 and unchanged by the host-admitted membership fix, which only makes more repos reach that state sooner; the authoritative close path (stageTerminalSurfaceRetirements) computes from the store's session and is unaffected. Known, unaddressed, and deliberately out of scope here: the safe correction is per-tab rather than per-worktree rebase arbitration, because raising the incoming revision wholesale would re-open the host-tab loss whenever a close coincides with a host create.", "The SSH relay reattach binding (src/main/ssh/ssh-relay-session.ts persistPtyBinding) is NOT flagged host-admitted, so a lease rebind that has to mint a missing tab raises no fence. It rebinds an existing lease rather than admitting a new surface, so it is believed not to need one, but that was not established either way. Known, unaddressed, and deliberately out of scope here.", "The host-created retention journey is macOS-local; the reported incident was on a physical Windows host, which we did not run against." ], diff --git a/src/main/persistence-host-admitted-terminal-membership.test.ts b/src/main/persistence-host-admitted-terminal-membership.test.ts index 3470215a515..83a6898c667 100644 --- a/src/main/persistence-host-admitted-terminal-membership.test.ts +++ b/src/main/persistence-host-admitted-terminal-membership.test.ts @@ -136,7 +136,7 @@ describe('host-admitted terminal membership survives a stale renderer replay', ( }) // Closing must still work afterwards. Closes are host-driven: the retirement is - // computed from the store's own session (see persistTerminalSurfaceRetirements), + // computed from the store's own session (see stageTerminalSurfaceRetirements), // which is what outranks the fence this create just raised. it('still lets the authoritative retirement path close the host-admitted tab', async () => { const store = await createStore() diff --git a/src/main/persistence/loading-store/profile-state-delayed-authority-fixture.ts b/src/main/persistence/loading-store/profile-state-delayed-authority-fixture.ts index cebd91b0538..26709b33e3f 100644 --- a/src/main/persistence/loading-store/profile-state-delayed-authority-fixture.ts +++ b/src/main/persistence/loading-store/profile-state-delayed-authority-fixture.ts @@ -29,12 +29,18 @@ export class DelayedAuthority implements AsyncProfileStateAuthority { | { started: ReturnType>; finish: ReturnType> } | undefined readonly captures: ProfileStateDomainReplacement[][] = [] + private failNext = false readonly close = vi.fn(async () => { this.inner.close() }) constructor(readonly inner: ProfileStateSqliteAuthority) {} + /** Fail the next profile write the way a full disk would, after any paused gate opens. */ + failNextWrite() { + this.failNext = true + } + pause() { const gate = { started: deferred(), finish: deferred() } this.next = gate @@ -51,17 +57,17 @@ export class DelayedAuthority implements AsyncProfileStateAuthority { } async writeSerializedState(payload: Buffer) { const captured = Buffer.from(payload) - await this.dispatch(() => this.inner.writeSerializedState(captured)) + await this.dispatch(() => this.inner.writeSerializedState(captured), true) } async writeSerializedDomains(replacements: readonly ProfileStateDomainReplacement[]) { const captured = structuredClone(replacements) this.captures.push([...captured]) - await this.dispatch(() => this.inner.writeSerializedDomains(captured)) + await this.dispatch(() => this.inner.writeSerializedDomains(captured), true) } async writeCompleteSerializedDomains(replacements: readonly ProfileStateDomainReplacement[]) { const captured = structuredClone(replacements) this.captures.push([...captured]) - await this.dispatch(() => this.inner.writeCompleteSerializedDomains(captured)) + await this.dispatch(() => this.inner.writeCompleteSerializedDomains(captured), true) } async writeSerializedAutomationRuns( replacements: readonly ProfileStateDomainReplacement[], @@ -69,7 +75,10 @@ export class DelayedAuthority implements AsyncProfileStateAuthority { ) { const captured = structuredClone(replacements) const capturedRuns = structuredClone(runs) - await this.dispatch(() => this.inner.writeSerializedAutomationRuns(captured, capturedRuns)) + await this.dispatch( + () => this.inner.writeSerializedAutomationRuns(captured, capturedRuns), + true + ) } async writeJsonExport(path: string) { return this.inner.writeJsonExport(path) @@ -83,11 +92,15 @@ export class DelayedAuthority implements AsyncProfileStateAuthority { async quarantineDatabase(root?: string, reason?: string) { return this.inner.quarantineDatabase(root, reason) } - private async dispatch(operation: () => void) { + private async dispatch(operation: () => void, write = false) { const gate = this.next this.next = undefined gate?.started.resolve() await gate?.finish.promise + if (write && this.failNext) { + this.failNext = false + throw new Error('profile_state_write_failed') + } operation() } } diff --git a/src/main/persistence/loading-store/pty-retirement-publication-during-read.test.ts b/src/main/persistence/loading-store/pty-retirement-publication-during-read.test.ts index dd6754c1d73..34b6e742804 100644 --- a/src/main/persistence/loading-store/pty-retirement-publication-during-read.test.ts +++ b/src/main/persistence/loading-store/pty-retirement-publication-during-read.test.ts @@ -29,9 +29,6 @@ class RetirementRuntime extends OrcaRuntimeService { generations(): number { return this.ptyLifecycleGenerationById.size } - retirements(): number { - return this.pendingPtySurfaceRetirementsByPtyId.size - } async closeTab(): Promise { const snapshot = this.snapshot() const tab = snapshot?.tabs[0] @@ -96,10 +93,8 @@ it.each(['read', 'replacement', 'legacy-replacement'] as const)( gate.finish.resolve() await exiting expect(readState().workspaceSession.terminalLayoutsByTabId[binding.tabId]).toBeUndefined() - expect(runtime.snapshot()?.tabs).toHaveLength(action === 'read' ? 0 : 1) - if (action === 'read') { - expect(runtime.retirements()).toBe(0) - } + // Why every action: the exit retired the leaf in memory before any of them could run. + expect(runtime.snapshot()?.tabs).toHaveLength(0) } ) diff --git a/src/main/runtime/acknowledged-terminal-tab-retirement-fixture.ts b/src/main/runtime/acknowledged-terminal-tab-retirement-fixture.ts index 51cd8b4cd8b..12b3340d2ac 100644 --- a/src/main/runtime/acknowledged-terminal-tab-retirement-fixture.ts +++ b/src/main/runtime/acknowledged-terminal-tab-retirement-fixture.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { vi } from 'vitest' import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { PersistedState } from '../../shared/persisted-state-types' import type { RuntimeSyncWindowGraph } from '../../shared/runtime-types' import { closeTerminalTabInWorkspaceSession } from '../../shared/workspace-session-terminal-tab-close' import { ProfileStateSqliteAuthority } from '../persistence/profile-state/profile-state-sqlite-authority' @@ -30,8 +31,9 @@ function deferred(): { promise: Promise; resolve: () => void } { export function createAcknowledgedTabRetirementFixture(bound = false) { const directory = mkdtempSync(join(tmpdir(), 'orca-close-ack-')) + const databasePath = join(directory, 'profile-state.db') const authority = new DelayedAuthority( - new ProfileStateSqliteAuthority(join(directory, 'profile-state.db'), 'ack-retirement') + new ProfileStateSqliteAuthority(databasePath, 'ack-retirement') ) const store = new Store({ dataFile: join(directory, 'orca-data.json'), @@ -149,6 +151,7 @@ export function createAcknowledgedTabRetirementFixture(bound = false) { store.setWorkspaceSession( advanceTerminalTopologyRevision(store.getWorkspaceSession(), ACK_WORKTREE) ) + let finalFlush: Promise | undefined const entered = deferred() const acknowledgement = deferred() const closeTerminalTab = vi.fn(async () => { @@ -184,15 +187,30 @@ export function createAcknowledgedTabRetirementFixture(bound = false) { acknowledgement, closeTerminalTab, publish, + /** Reads what a relaunch would load, independent of the store's in-memory state. */ + readDisk: (): PersistedState => { + const reader = new ProfileStateSqliteAuthority(databasePath, 'ack-retirement') + try { + return JSON.parse(reader.readSerializedState() ?? '{}') + } finally { + reader.close() + } + }, hasTab: () => store.getWorkspaceSession().tabsByWorktree[ACK_WORKTREE].some((tab) => tab.id === ACK_TAB), close: (options: { force?: boolean } = {}) => runtime.closeMobileSessionTab(`id:${ACK_WORKTREE}`, ACK_TAB, { reason: 'user', ...options }), + /** The app-quit flush; it finalizes persistence, so dispose must not flush again. */ + quit: () => (finalFlush ??= store.flushFinalOrThrowAsync()), dispose: async () => { runtime.setNotifier(null) runtime.syncWindowGraph(1, { tabs: [], leaves: [], mobileSessionTabs: [] }) - await store.flushPendingOrThrowAsync() - await store.freezeWritesAsync() + if (finalFlush) { + await finalFlush + } else { + await store.flushPendingOrThrowAsync() + await store.freezeWritesAsync() + } setRuntimeDesktopSurface(null) rmSync(directory, { recursive: true, force: true }) } diff --git a/src/main/runtime/orca-runtime-fit-override-listeners.ts b/src/main/runtime/orca-runtime-fit-override-listeners.ts index 6d4242d7758..5ef17ce4c2e 100644 --- a/src/main/runtime/orca-runtime-fit-override-listeners.ts +++ b/src/main/runtime/orca-runtime-fit-override-listeners.ts @@ -74,7 +74,6 @@ export class OrcaRuntimeWithFitOverrideListeners extends OrcaRuntimeWithStopRequ protected providerSnapshotsWithLiveModeTransition = new WeakSet() protected ptyLifecycleGenerationById = new Map() - protected pendingPtySurfaceRetirementsByPtyId = new Map() protected nextPtyLifecycleGeneration = 1 diff --git a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts index a348309a568..9d6e5d56ca4 100644 --- a/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts +++ b/src/main/runtime/orca-runtime-invalidate-all-handles-for-pty.ts @@ -140,7 +140,6 @@ export class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRuntimeWithRe options: { awaitsRegistration?: boolean } = {} ): void { this.invalidatePtyControllerInventoryForLifecycle(ptyId) - this.pendingPtySurfaceRetirementsByPtyId.delete(ptyId) const existingPty = this.ptysById.get(ptyId) if ( existingPty && diff --git a/src/main/runtime/orca-runtime-on-pty-exit.ts b/src/main/runtime/orca-runtime-on-pty-exit.ts index 52b7024b818..abcf2fcf57f 100644 --- a/src/main/runtime/orca-runtime-on-pty-exit.ts +++ b/src/main/runtime/orca-runtime-on-pty-exit.ts @@ -68,171 +68,172 @@ export class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDisconnecte pty?.incarnationId ?? `runtime:${this.runtimeId}:${this.getPtyLifecycleGeneration(ptyId)}` this.advancePtyLifecycleGeneration(ptyId) - this.notifyPtyExitListeners(ptyId) - const exactSurfaceByKey = new Map< - string, - Pick - >() - for (const [worktreeId, snapshot] of this.mobileSessionTabsByWorktree) { - for (const tab of snapshot.tabs) { - if ( - tab.type === 'terminal' && - (tab.ptyId === ptyId || tab.parentLayout?.ptyIdsByLeafId?.[tab.leafId] === ptyId) - ) { - exactSurfaceByKey.set(`${worktreeId}\0${tab.parentTabId}\0${tab.leafId}`, { - worktreeId, - parentTabId: tab.parentTabId, - leafId: tab.leafId - }) + let retirement: Promise | undefined + try { + const exactSurfaceByKey = new Map< + string, + Pick + >() + for (const [worktreeId, snapshot] of this.mobileSessionTabsByWorktree) { + for (const tab of snapshot.tabs) { + if ( + tab.type === 'terminal' && + (tab.ptyId === ptyId || tab.parentLayout?.ptyIdsByLeafId?.[tab.leafId] === ptyId) + ) { + exactSurfaceByKey.set(`${worktreeId}\0${tab.parentTabId}\0${tab.leafId}`, { + worktreeId, + parentTabId: tab.parentTabId, + leafId: tab.leafId + }) + } } } - } - for (const leaf of this.getLeavesForPty(ptyId)) { - exactSurfaceByKey.set(`${leaf.worktreeId}\0${leaf.tabId}\0${leaf.leafId}`, { - worktreeId: leaf.worktreeId, - parentTabId: leaf.tabId, - leafId: leaf.leafId - }) - } - const parsedPaneKey = parsePaneKey(pty?.paneKey ?? '') - if (pty?.tabId && parsedPaneKey) { - exactSurfaceByKey.set(`${pty.worktreeId}\0${pty.tabId}\0${parsedPaneKey.leafId}`, { - worktreeId: pty.worktreeId, - parentTabId: pty.tabId, - leafId: parsedPaneKey.leafId - }) - } - const exactSurfaces = [...exactSurfaceByKey.values()] - const pendingIncarnation = this.pendingPtyRegistrationIncarnations.get(ptyId) - const exitMatchesPendingRegistration = - this.pendingPtyRegistrationIncarnations.has(ptyId) && - (pendingIncarnation === null || - exitIncarnationId === null || - exitIncarnationId === undefined || - pendingIncarnation === exitIncarnationId) - if (exitMatchesPendingRegistration) { - // Why: reused surfaces can look registered while their replacement incarnation still awaits admission. - this.earlyExitedPtyIncarnations.set( - ptyId, - exitIncarnationId ?? pendingIncarnation ?? pty?.incarnationId ?? null - ) - } - // Why both kinds: a sleep keeps its wake hint, and a restart's replacement takes the pane. - const preservesIntentionallyStoppedSurface = - this.intentionalPtyStops.claimExit(ptyId, exitIncarnationId ?? pty?.incarnationId).length > 0 - advertisedUrlWatcher.unbindPty(ptyId) - // Clean up new mobile state for this PTY - this.mobileSubscribers.delete(ptyId) - this.terminalViewSubscribers.clearSubscribers(ptyId) - this.mobileDisplayModes.delete(ptyId) - this.resizeListeners.delete(ptyId) - this.lastRendererSizes.delete(ptyId) - this.recentPtyOutputById.delete(ptyId) - this.setupCompletionTokenByPtyId.delete(ptyId) - this.clearWaitBlockedCheckState(ptyId) - this.recentPtyPathCandidatesById.delete(ptyId) - this.ptyOutputSequenceById.delete(ptyId) - this.providerSequenceInitializedPtys.delete(ptyId) - this.providerSequenceOffsetByPtyId.delete(ptyId) - this.providerSnapshotPreferredPtys.delete(ptyId) - this.providerModeTrackersByPtyId.delete(ptyId) - this.providerModeSnapshotScansByPtyId.delete(ptyId) - this.providerBufferAcquisitionsByPtyId.delete(ptyId) - this.providerVisibleStateByPtyId.delete(ptyId) - this.providerVisibleRetryAtByPtyId.delete(ptyId) - this.agentPromptExplicitStatusFloorByPtyId.delete(ptyId) - this.ptyLifecycleGenerationById.delete(ptyId) - this.pendingPtySurfaceRetirementsByPtyId.delete(ptyId) - this.agentStatusOscProcessorsByPtyId.delete(ptyId) - this.terminalSpawnCommandsByPtyId.delete(ptyId) - this.disposePtyTitleTracker(ptyId) - this.oscTitleScanTailByPtyId.delete(ptyId) - this.osc7ScanTailByPtyId.delete(ptyId) - this.terminalCwdByPtyId.delete(ptyId) - this.terminalFileUriHostnameByPtyId.delete(ptyId) - this.wslDistroByPtyId.delete(ptyId) - // Why: a Claude agent-team leader whose PTY exits naturally (agent finished, - // process died, renderer reload) must release its team + nested panes map. - // Previously only explicit closeTerminal evicted it, so natural exits leaked - // one team per never-reused teamId for the runtime's lifetime. - const exitedTeamLeaderHandle = this.handleByPtyId.get(ptyId) - if (exitedTeamLeaderHandle) { - this.claudeAgentTeams.removeTeamForLeaderHandle(exitedTeamLeaderHandle) - } - // Layout state machine: clear `layouts` and `layoutQueues`. Any - // already-queued applyLayout work for this ptyId will run, but every - // applyLayout re-checks `layouts.has(ptyId)` (or fresh-subscribe) and - // short-circuits with `pty-exited`. - this.layouts.delete(ptyId) - this.layoutQueues.delete(ptyId) - this.freshSubscribeGuard.delete(ptyId) - this.cancelPendingDriverMutations(ptyId) - // Why: a cold restore can respawn under the same session id within the - // delayed-Enter window; the armed Enter would inject \r into the - // replacement and stamp rows it never received. - this.orchestrationMailboxNotifications.retirePty(ptyId) - // Why: the dead pty's terminal handle and any run bound to its panes still carry mailbox - // pointers; schedule a debounced repoint so they do not stay aimed at a retired session. - for (const leaf of this.getLeavesForPty(ptyId)) { - const mailboxHandle = this.handleByLeafKey.get(this.getLeafKey(leaf.tabId, leaf.leafId)) - if (mailboxHandle) { - this.mailPointerRepointScheduler.schedule(mailboxHandle) + for (const leaf of this.getLeavesForPty(ptyId)) { + exactSurfaceByKey.set(`${leaf.worktreeId}\0${leaf.tabId}\0${leaf.leafId}`, { + worktreeId: leaf.worktreeId, + parentTabId: leaf.tabId, + leafId: leaf.leafId + }) } - const boundRun = this._orchestrationDb?.getCurrentRunForPane?.(`${leaf.tabId}:${leaf.leafId}`) - if (boundRun) { - this.mailPointerRepointScheduler.schedule(`run:${boundRun.id}`) + const parsedPaneKey = parsePaneKey(pty?.paneKey ?? '') + if (pty?.tabId && parsedPaneKey) { + exactSurfaceByKey.set(`${pty.worktreeId}\0${pty.tabId}\0${parsedPaneKey.leafId}`, { + worktreeId: pty.worktreeId, + parentTabId: pty.tabId, + leafId: parsedPaneKey.leafId + }) + } + const exactSurfaces = [...exactSurfaceByKey.values()] + const pendingIncarnation = this.pendingPtyRegistrationIncarnations.get(ptyId) + const exitMatchesPendingRegistration = + this.pendingPtyRegistrationIncarnations.has(ptyId) && + (pendingIncarnation === null || + exitIncarnationId === null || + exitIncarnationId === undefined || + pendingIncarnation === exitIncarnationId) + if (exitMatchesPendingRegistration) { + // Why: reused surfaces can look registered while their replacement incarnation still awaits admission. + this.earlyExitedPtyIncarnations.set( + ptyId, + exitIncarnationId ?? pendingIncarnation ?? pty?.incarnationId ?? null + ) + } + // Why both kinds: a sleep keeps its wake hint, and a restart's replacement takes the pane. + const preservesIntentionallyStoppedSurface = + this.intentionalPtyStops.claimExit(ptyId, exitIncarnationId ?? pty?.incarnationId).length > + 0 + advertisedUrlWatcher.unbindPty(ptyId) + // Clean up new mobile state for this PTY + this.mobileSubscribers.delete(ptyId) + this.terminalViewSubscribers.clearSubscribers(ptyId) + this.mobileDisplayModes.delete(ptyId) + this.resizeListeners.delete(ptyId) + this.lastRendererSizes.delete(ptyId) + this.recentPtyOutputById.delete(ptyId) + this.setupCompletionTokenByPtyId.delete(ptyId) + this.clearWaitBlockedCheckState(ptyId) + this.recentPtyPathCandidatesById.delete(ptyId) + this.ptyOutputSequenceById.delete(ptyId) + this.providerSequenceInitializedPtys.delete(ptyId) + this.providerSequenceOffsetByPtyId.delete(ptyId) + this.providerSnapshotPreferredPtys.delete(ptyId) + this.providerModeTrackersByPtyId.delete(ptyId) + this.providerModeSnapshotScansByPtyId.delete(ptyId) + this.providerBufferAcquisitionsByPtyId.delete(ptyId) + this.providerVisibleStateByPtyId.delete(ptyId) + this.providerVisibleRetryAtByPtyId.delete(ptyId) + this.agentPromptExplicitStatusFloorByPtyId.delete(ptyId) + this.ptyLifecycleGenerationById.delete(ptyId) + this.agentStatusOscProcessorsByPtyId.delete(ptyId) + this.terminalSpawnCommandsByPtyId.delete(ptyId) + this.disposePtyTitleTracker(ptyId) + this.oscTitleScanTailByPtyId.delete(ptyId) + this.osc7ScanTailByPtyId.delete(ptyId) + this.terminalCwdByPtyId.delete(ptyId) + this.terminalFileUriHostnameByPtyId.delete(ptyId) + this.wslDistroByPtyId.delete(ptyId) + // Why: a Claude agent-team leader whose PTY exits naturally (agent finished, + // process died, renderer reload) must release its team + nested panes map. + // Previously only explicit closeTerminal evicted it, so natural exits leaked + // one team per never-reused teamId for the runtime's lifetime. + const exitedTeamLeaderHandle = this.handleByPtyId.get(ptyId) + if (exitedTeamLeaderHandle) { + this.claudeAgentTeams.removeTeamForLeaderHandle(exitedTeamLeaderHandle) + } + // Layout state machine: clear `layouts` and `layoutQueues`. Any + // already-queued applyLayout work for this ptyId will run, but every + // applyLayout re-checks `layouts.has(ptyId)` (or fresh-subscribe) and + // short-circuits with `pty-exited`. + this.layouts.delete(ptyId) + this.layoutQueues.delete(ptyId) + this.freshSubscribeGuard.delete(ptyId) + this.cancelPendingDriverMutations(ptyId) + // Why: a cold restore can respawn under the same session id within the + // delayed-Enter window; the armed Enter would inject \r into the + // replacement and stamp rows it never received. + this.orchestrationMailboxNotifications.retirePty(ptyId) + // Why: the dead pty's terminal handle and any run bound to its panes still carry mailbox + // pointers; schedule a debounced repoint so they do not stay aimed at a retired session. + for (const leaf of this.getLeavesForPty(ptyId)) { + const mailboxHandle = this.handleByLeafKey.get(this.getLeafKey(leaf.tabId, leaf.leafId)) + if (mailboxHandle) { + this.mailPointerRepointScheduler.schedule(mailboxHandle) + } + const boundRun = this._orchestrationDb?.getCurrentRunForPane?.( + `${leaf.tabId}:${leaf.leafId}` + ) + if (boundRun) { + this.mailPointerRepointScheduler.schedule(`run:${boundRun.id}`) + } } - } - if (this.terminalFitOverrides.has(ptyId)) { - this.terminalFitOverrides.delete(ptyId) - this.notifier?.terminalFitOverrideChanged(ptyId, 'desktop-fit', 0, 0) - this.notifyFitOverrideListeners(ptyId, 'desktop-fit', 0, 0) - } - // Why: clear driver state and notify the renderer so any lock banner on - // this dead pane unmounts. Without this, the pane shows a stuck banner - // until tab teardown, and `getDriver(deadPtyId)` would keep returning a - // stale `mobile{X}` to any caller that hasn't yet seen the exit IPC. - this.terminalDrivers.clear(ptyId) - this.remoteDesktopFloor.clearPty(ptyId) - this.disposeHeadlessTerminal(ptyId) - if (processDeathCertified) { - // The bounded verdict register also fences late graphs after the PTY record was pruned. - this.rememberPtyLivenessVerdict(ptyId, { status: 'exited' }) - } - if (pty) { - pty.connected = false - pty.runtimeSessionOwned = false - this.setPairedRendererSessionOwnership(pty.ptyId, false) - pty.disconnectedAt = Date.now() - pty.lastExitCode = exitCode - pty.lastExitCause = exitCause - // Why: the exited process's live frames say nothing about a replacement. - // A same-id respawn makes the leaf writable again before any new title, - // so leaving this true would let push delivery type into the new process - // on the dead one's idle. lastAgentStatus itself stays for `ps` display. - pty.lastAgentStatusObservedLive = false - this.resolvePtyExitWaiters(pty, ptyId) - this.pruneDisconnectedPtyTranscript(pty) - } - let retirement: Promise | undefined - if (preservesIntentionallyStoppedSurface || preservesAbnormalSshSurface) { - // Why: relay loss is recoverable; keep the HUB-owned pane addressable through the bounded reconnect grace. - this.touchMobileSessionSnapshotsForPty(ptyId, { immediate: true }) - } else { - // Why: permanent process exit is absence, not a starting/sleeping tab. - // Retire before publishing so paired clients never persist a ghost. - const pendingRetirement = {} - this.pendingPtySurfaceRetirementsByPtyId.set(ptyId, pendingRetirement) - retirement = this.retireMobileSessionSurfacesForPty(ptyId, incarnationId, exactSurfaces) - .catch((error) => { + if (this.terminalFitOverrides.has(ptyId)) { + this.terminalFitOverrides.delete(ptyId) + this.notifier?.terminalFitOverrideChanged(ptyId, 'desktop-fit', 0, 0) + this.notifyFitOverrideListeners(ptyId, 'desktop-fit', 0, 0) + } + // Why: clear driver state and notify the renderer so any lock banner on + // this dead pane unmounts. Without this, the pane shows a stuck banner + // until tab teardown, and `getDriver(deadPtyId)` would keep returning a + // stale `mobile{X}` to any caller that hasn't yet seen the exit IPC. + this.terminalDrivers.clear(ptyId) + this.remoteDesktopFloor.clearPty(ptyId) + this.disposeHeadlessTerminal(ptyId) + if (processDeathCertified) { + // The bounded verdict register also fences late graphs after the PTY record was pruned. + this.rememberPtyLivenessVerdict(ptyId, { status: 'exited' }) + } + if (pty) { + pty.connected = false + pty.runtimeSessionOwned = false + this.setPairedRendererSessionOwnership(pty.ptyId, false) + pty.disconnectedAt = Date.now() + pty.lastExitCode = exitCode + pty.lastExitCause = exitCause + // Why: the exited process's live frames say nothing about a replacement. + // A same-id respawn makes the leaf writable again before any new title, + // so leaving this true would let push delivery type into the new process + // on the dead one's idle. lastAgentStatus itself stays for `ps` display. + pty.lastAgentStatusObservedLive = false + this.resolvePtyExitWaiters(pty, ptyId) + this.pruneDisconnectedPtyTranscript(pty) + } + if (preservesIntentionallyStoppedSurface || preservesAbnormalSshSurface) { + // Why: relay loss is recoverable; keep the HUB-owned pane addressable through the bounded reconnect grace. + this.touchMobileSessionSnapshotsForPty(ptyId, { immediate: true }) + } else { + // Why: permanent process exit is absence, not a starting/sleeping tab. + // Retire before publishing so paired clients never persist a ghost. + try { + retirement = this.retireMobileSessionSurfacesForPty(ptyId, incarnationId, exactSurfaces) + } catch (error) { console.error('[runtime] failed to publish terminal retirement:', error) - }) - .finally(() => { - if (this.pendingPtySurfaceRetirementsByPtyId.get(ptyId) === pendingRetirement) { - this.pendingPtySurfaceRetirementsByPtyId.delete(ptyId) - } - }) + } + } + } finally { + // Why last: a stream end cues clients to re-activate the pane it ended, so the retirement + // must precede it; a cleanup fault must still end the stream. + this.notifyPtyExitListeners(ptyId) } const exitedSurfaces: { handle: string; paneKey: string | null }[] = [] diff --git a/src/main/runtime/orca-runtime-persist-terminal-surface-retirements.ts b/src/main/runtime/orca-runtime-persist-terminal-surface-retirements.ts index bafc280d957..5458543815a 100644 --- a/src/main/runtime/orca-runtime-persist-terminal-surface-retirements.ts +++ b/src/main/runtime/orca-runtime-persist-terminal-surface-retirements.ts @@ -1,95 +1,69 @@ // @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests. import { OrcaRuntimeWithTouchMobileSessionTabsForWorktree } from './orca-runtime-touch-mobile-session-tabs-for-worktree' import type { RetiredTerminalSurface } from './mobile-session-terminal-retirement' -import type { ExecutionHostId } from '../../shared/execution-host' import type { RuntimeMobileSessionRetiredTerminalSurface } from '../../shared/runtime-types' import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' -import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import { retireTerminalSurfaceFromPersistence } from './mobile-session-terminal-persistence-retirement' import { retireTerminalSurfacesFromSnapshot } from './mobile-session-terminal-retirement' import { attachRetirementProofsToSnapshot } from './mobile-session-terminal-retirement-proof' -import { cloneWorkspaceSessionState } from '../persistence/restoring-sessions/session-owner-fields' -import { rollbackWorkspaceSessionAfterFailedAsyncWrite } from '../persistence/restoring-sessions/workspace-session-write-rollback' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' export class OrcaRuntimeWithPersistTerminalSurfaceRetirements extends OrcaRuntimeWithTouchMobileSessionTabsForWorktree { /** - * Retires each surface in the session partition of the host that owns its worktree. + * Retires each surface in the in-memory session partition of the host that owns its worktree. * Why: an SSH pane's durable surface lives in that connection's partition; retiring it * against the local partition strands the real ghost and bumps a foreign host's epoch. - * Returns null when nothing may be published because persistence is unavailable or failed. + * `accepted` held the surface; `unpersisted` had no partition to hold it (or refused the write). */ - protected async persistTerminalSurfaceRetirements( - retiredSurfaces: readonly RetiredTerminalSurface[] - ): Promise<{ accepted: RetiredTerminalSurface[]; unpersisted: RetiredTerminalSurface[] } | null> { + protected stageTerminalSurfaceRetirements(retiredSurfaces: readonly RetiredTerminalSurface[]): { + accepted: RetiredTerminalSurface[] + unpersisted: RetiredTerminalSurface[] + } { + const accepted: RetiredTerminalSurface[] = [] + const unpersisted: RetiredTerminalSurface[] = [] + for (const surface of retiredSurfaces) { + const hostId = + this.tryGetWorkspaceSessionHostIdForWorktree(surface.worktreeId) ?? LOCAL_EXECUTION_HOST_ID + const current = this.store?.getWorkspaceSession?.(hostId) + if (!current) { + unpersisted.push(surface) + continue + } + const next = retireTerminalSurfaceFromPersistence(current, surface) + if (next === current) { + continue + } + try { + this.store.setWorkspaceSession(next, hostId) + accepted.push(surface) + } catch (error) { + // Why: the process is gone whether or not the profile admits the write (quit, maintenance). + console.error('[runtime] could not stage terminal retirement:', error) + unpersisted.push(surface) + } + } + return { accepted, unpersisted } + } + + // Why no rollback: the process is gone, so a failed write leaves the retirement for the next one. + protected async persistStagedTerminalSurfaceRetirements(): Promise { if (!this.store?.runDurableMutation) { - const hasPersistedSession = retiredSurfaces.some((surface) => - this.store?.getWorkspaceSession?.( - this.tryGetWorkspaceSessionHostIdForWorktree(surface.worktreeId) ?? - LOCAL_EXECUTION_HOST_ID - ) - ) - return hasPersistedSession ? null : { accepted: [], unpersisted: [...retiredSurfaces] } + return } try { - return await this.store.runDurableMutation(() => { - const accepted: RetiredTerminalSurface[] = [] - const unpersisted: RetiredTerminalSurface[] = [] - const originals = new Map() - const staged = new Map() - for (const surface of retiredSurfaces) { - const hostId = - this.tryGetWorkspaceSessionHostIdForWorktree(surface.worktreeId) ?? - LOCAL_EXECUTION_HOST_ID - const current = this.store.getWorkspaceSession?.(hostId) - if (!current) { - unpersisted.push(surface) - continue - } - if (!this.store.setWorkspaceSession) { - throw new Error('workspace_session_unavailable') - } - if (!originals.has(hostId)) { - originals.set(hostId, cloneWorkspaceSessionState(current)) - } - const next = retireTerminalSurfaceFromPersistence(current, surface) - if (next !== current) { - this.store.setWorkspaceSession(next, hostId) - staged.set(hostId, cloneWorkspaceSessionState(this.store.getWorkspaceSession(hostId))) - accepted.push(surface) - } - } - return { - value: { accepted, unpersisted }, - persist: staged.size > 0, - rollback: () => { - for (const [hostId, stagedSession] of staged) { - const current = this.store.getWorkspaceSession(hostId) - const rolledBack = rollbackWorkspaceSessionAfterFailedAsyncWrite( - originals.get(hostId), - stagedSession, - current - ) - if (rolledBack !== current) { - this.store.setWorkspaceSession(rolledBack, hostId) - } - } - } - } - }) + // Why if-dirty: an earlier write, or another exit's, may already carry this retirement. + await this.store.runDurableMutation(() => ({ value: undefined, persist: 'if-dirty' })) } catch (error) { - console.error('[runtime] failed to persist terminal retirement:', error) - return null + console.error('[runtime] terminal retirement is not yet durable:', error) } } - protected async retireMobileSessionSurfacesForPty( + // Why synchronous: the exit's stream end cues clients to re-activate, which must find the leaf gone. + protected retireMobileSessionSurfacesForPty( ptyId: string, incarnationId: string, exactSurfaces: readonly Pick[] - ): Promise { - // Reads can mint a new frame generation while this independent retirement waits for disk. - const pendingRetirement = this.pendingPtySurfaceRetirementsByPtyId.get(ptyId) + ): Promise | undefined { const terminalHandle = this.handleByPtyId.get(ptyId) ?? this.findHandleForPtyRecord(ptyId) ?? undefined const retiredSurfaceByKey = new Map() @@ -119,26 +93,18 @@ export class OrcaRuntimeWithPersistTerminalSurfaceRetirements extends OrcaRuntim } const retiredSurfaces = [...retiredSurfaceByKey.values()] if (retiredSurfaces.length === 0) { - return + return undefined } - const persisted = await this.persistTerminalSurfaceRetirements(retiredSurfaces) - const currentIncarnation = this.ptysById.get(ptyId)?.incarnationId - if ( - !persisted || - this.pendingPtySurfaceRetirementsByPtyId.get(ptyId) !== pendingRetirement || - (currentIncarnation && currentIncarnation !== incarnationId) - ) { - return - } - for (const surface of persisted.unpersisted) { + const staged = this.stageTerminalSurfaceRetirements(retiredSurfaces) + for (const surface of staged.unpersisted) { const repoId = getRepoIdFromWorktreeId(surface.worktreeId) this.terminalTopologyRevisionByRepoId.set( repoId, (this.terminalTopologyRevisionByRepoId.get(repoId) ?? 0) + 1 ) } - // Why: one repo epoch can cover multiple exits, but only surfaces individually accepted by persistence may disappear. - const removableRetiredSurfaces = [...persisted.accepted, ...persisted.unpersisted] + // Why: one repo epoch can cover multiple exits; a surface the session binds to another PTY or incarnation stays. + const removableRetiredSurfaces = [...staged.accepted, ...staged.unpersisted] for (const [worktreeId, snapshot] of this.mobileSessionTabsByWorktree) { // Why proofs aren't gated on `removable`: the exit is the attestation, and a surface the // renderer already de-persisted leaves persistence nothing to accept. Withholding the proof @@ -163,7 +129,7 @@ export class OrcaRuntimeWithPersistTerminalSurfaceRetirements extends OrcaRuntim snapshot, ptyId, exactSurfaces: removableSurfaces, - // Why: discovery is broad by PTY id, but publication may remove only surfaces whose durable retirement was accepted. + // Why: discovery is broad by PTY id, but publication may remove only surfaces the session retired. exactOnly: true, ...(retirementProofs.length > 0 ? { retirementProofs } : {}) }) @@ -175,6 +141,7 @@ export class OrcaRuntimeWithPersistTerminalSurfaceRetirements extends OrcaRuntim } this.publishRetiredTerminalSurfaceProofs(worktreeId, retirementProofs) } + return staged.accepted.length > 0 ? this.persistStagedTerminalSurfaceRetirements() : undefined } /** Ships durable retirement proofs on their own frame when no surface removal carries them. */ diff --git a/src/main/runtime/orca-runtime-register-pty.ts b/src/main/runtime/orca-runtime-register-pty.ts index ba7b47be72d..bd9c96a5c0f 100644 --- a/src/main/runtime/orca-runtime-register-pty.ts +++ b/src/main/runtime/orca-runtime-register-pty.ts @@ -27,7 +27,6 @@ export class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalidateAllHand isWsl?: boolean ): void { this.assertPtyDidNotExitBeforeRegistration(ptyId, binding?.incarnationId) - this.pendingPtySurfaceRetirementsByPtyId.delete(ptyId) this.invalidatePtyControllerInventoryForLifecycle(ptyId, connectionId) const existingPty = this.ptysById.get(ptyId) const replacementHandle = binding?.terminalHandle?.trim() diff --git a/src/main/runtime/orca-runtime-terminal-retirement.test.ts b/src/main/runtime/orca-runtime-terminal-retirement.test.ts index 4422dc2a1e6..d6f70fd4c6e 100644 --- a/src/main/runtime/orca-runtime-terminal-retirement.test.ts +++ b/src/main/runtime/orca-runtime-terminal-retirement.test.ts @@ -691,7 +691,7 @@ describe('OrcaRuntimeService terminal surface retirement', () => { expect(flushOrThrow).toHaveBeenCalledOnce() }) - it('does not publish absence when the durable retirement flush fails', async () => { + it('publishes absence and reports when the durable retirement flush fails', async () => { const session = makePersistedSplitSession() const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) const runtime = new OrcaRuntimeService( @@ -717,20 +717,20 @@ describe('OrcaRuntimeService terminal surface retirement', () => { await runtime.onPtyExit('pty-left', 0, 'incarnation-a') + // Why: the process is gone either way; a failed write is reported, not reinstated. expect((await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`)).tabs).toEqual([ - expect.objectContaining({ id: 'tab::left' }), expect.objectContaining({ id: 'tab::right' }) ]) - expect(events).toEqual([]) + expect(events).not.toEqual([]) expect(errorSpy).toHaveBeenCalledWith( - '[runtime] failed to persist terminal retirement:', + '[runtime] terminal retirement is not yet durable:', expect.any(Error) ) unsubscribe() errorSpy.mockRestore() }) - it('rolls back an in-memory retirement when the durable flush fails', async () => { + it('keeps the in-memory retirement when the durable flush fails', async () => { let session = makePersistedSplitSession() const original = structuredClone(session) const setWorkspaceSession = vi.fn((next: WorkspaceSessionState) => { @@ -755,10 +755,14 @@ describe('OrcaRuntimeService terminal surface retirement', () => { incarnationId: 'incarnation-a' }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + await runtime.onPtyExit('pty-left', 0, 'incarnation-a') - expect(session).toEqual(original) - expect(setWorkspaceSession).toHaveBeenLastCalledWith(original, LOCAL_EXECUTION_HOST_ID) - expect(setWorkspaceSession).toHaveBeenCalledTimes(2) + // Why: the next profile write carries the staged retirement; nothing may reinstate the leaf. + expect(session).not.toEqual(original) + expect(session.terminalLayoutsByTabId.tab?.ptyIdsByLeafId).toEqual({ right: 'pty-right' }) + expect(setWorkspaceSession).toHaveBeenCalledOnce() + errorSpy.mockRestore() }) }) diff --git a/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts b/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts new file mode 100644 index 00000000000..7da878ca2f7 --- /dev/null +++ b/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts @@ -0,0 +1,148 @@ +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from '../orca-runtime-test-mocks.spec' +import { + HEADLESS_LEAF_ID, + HEADLESS_SECOND_LEAF_ID, + TEST_WORKTREE_ID, + makeDeferred, + makeHeadlessTerminalLayout, + makeRuntimeStoreWithWorkspaceSession, + makeWorkspaceSessionWithHeadlessTerminal +} from '../orca-runtime-test-fixtures.spec' + +async function startSplitHost() { + const { runtimeStore, getSession } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal({ + tabsByWorktree: { + [TEST_WORKTREE_ID]: [ + { + id: 'host-tab', + ptyId: 'pty-a', + worktreeId: TEST_WORKTREE_ID, + title: 'Split Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { + 'host-tab': makeHeadlessTerminalLayout({ + [HEADLESS_LEAF_ID]: 'pty-a', + [HEADLESS_SECOND_LEAF_ID]: 'pty-b' + }) + } + }) + ) + const spawn = vi.fn(async (options: { sessionId?: string }) => ({ + id: options.sessionId ?? 'fresh-pty' + })) + const adoptStablePane = vi.fn(async () => null) + const runtime = new OrcaRuntimeService(runtimeStore) + runtime.setPtyController({ + spawn, + adoptStablePane, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + runtime.syncWindowGraph(0, { tabs: [], leaves: [] }) + const activate = (leafId: string, intent: 'user' | 'automatic') => + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab', leafId, { + notifyClients: false, + navigation: 'caller', + intent + }) + const terminalLeafIds = async (): Promise => + (await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`)).tabs.flatMap((tab) => + tab.type === 'terminal' ? [tab.leafId] : [] + ) + await activate(HEADLESS_LEAF_ID, 'user') + await activate(HEADLESS_SECOND_LEAF_ID, 'user') + expect(spawn).toHaveBeenCalledTimes(2) + expect(adoptStablePane).toHaveBeenCalledTimes(2) + return { runtime, runtimeStore, getSession, spawn, adoptStablePane, activate, terminalLeafIds } +} + +describe('OrcaRuntimeService', () => { + // Why: a paired mirror answers an exit's stream end by re-activating its pane. An activation + // that still finds the leaf respawns the exited session, and the retirement never publishes. + it('retires an exited split leaf before its stream end, while the durable write is pending', async () => { + const { runtime, runtimeStore, getSession, spawn, adoptStablePane, activate, terminalLeafIds } = + await startSplitHost() + + const disk = makeDeferred() + Object.assign(runtimeStore, { flushPendingOrThrowAsync: () => disk.promise }) + const published = vi.fn() + runtime.onMobileSessionTabsChanged(published) + // The stream end: the mirror's re-activation starts the moment the host releases it. + let reactivation: Promise | undefined + let observedAtStreamEnd: { binding?: string; publications: number } | undefined + runtime.subscribeToPtyExit('pty-b', () => { + observedAtStreamEnd = { + binding: + getSession().terminalLayoutsByTabId['host-tab']?.ptyIdsByLeafId?.[ + HEADLESS_SECOND_LEAF_ID + ], + publications: published.mock.calls.length + } + reactivation = activate(HEADLESS_SECOND_LEAF_ID, 'automatic').catch((error) => error) + }) + const exiting = runtime.onPtyExit('pty-b', 0, undefined, { providerExitObserved: true }) + + // Why: whatever answers the stream end, over any transport, must already see the leaf retired. + expect(observedAtStreamEnd).toEqual({ binding: undefined, publications: 1 }) + expect(reactivation).toBeDefined() + // Why: the refusal must come from the lookup, before any stable-pane adoption can revive it. + expect(await reactivation).toEqual(new Error('tab_not_found')) + expect(adoptStablePane).toHaveBeenCalledTimes(2) + expect(spawn).toHaveBeenCalledTimes(2) + expect(await terminalLeafIds()).toEqual([HEADLESS_LEAF_ID]) + expect( + getSession().terminalLayoutsByTabId['host-tab']?.ptyIdsByLeafId?.[HEADLESS_SECOND_LEAF_ID] + ).toBeUndefined() + + disk.resolve() + await exiting + expect(await terminalLeafIds()).toEqual([HEADLESS_LEAF_ID]) + }) + + // Why: the process is gone whether or not the profile admits the write (quit, maintenance). + it('retires the pane and ends the stream when the profile refuses the staging write', async () => { + const { runtime, runtimeStore, activate, terminalLeafIds } = await startSplitHost() + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + runtimeStore.setWorkspaceSession.mockImplementation(() => { + throw new Error('Profile maintenance or finalization is blocking new terminal snapshot work') + }) + const streamEnd = vi.fn() + runtime.subscribeToPtyExit('pty-b', streamEnd) + + await runtime.onPtyExit('pty-b', 0, undefined, { providerExitObserved: true }) + + expect(streamEnd).toHaveBeenCalledOnce() + expect(await terminalLeafIds()).toEqual([HEADLESS_LEAF_ID]) + await expect(activate(HEADLESS_SECOND_LEAF_ID, 'automatic')).rejects.toThrow('tab_not_found') + expect(errorSpy).toHaveBeenCalledWith( + '[runtime] could not stage terminal retirement:', + expect.any(Error) + ) + errorSpy.mockRestore() + }) + + // Why: the stream end now waits behind the exit cleanup; a cleanup fault must not strand it. + it('still ends the stream when exit cleanup throws before the retirement', () => { + const runtime = new OrcaRuntimeService() + Object.assign(runtime, { + disposeHeadlessTerminal: () => { + throw new Error('dispose_failed') + } + }) + const streamEnd = vi.fn() + runtime.subscribeToPtyExit('pty-a', streamEnd) + + expect(() => runtime.onPtyExit('pty-a', 0)).toThrow('dispose_failed') + expect(streamEnd).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 6d4cba4741d..070363fdb3f 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -79,6 +79,7 @@ await import('./orca-runtime-tests/mobile-session-tabs-part-10.spec') await import('./orca-runtime-tests/mobile-session-tabs-part-11.spec') await import('./orca-runtime-tests/mobile-session-tabs-part-12.spec') await import('./orca-runtime-tests/mobile-session-tabs-part-13.spec') +await import('./orca-runtime-tests/exit-retirement-activation.spec') await import('./orca-runtime-tests/mobile-creation-and-orchestration.spec') await import('./orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec') await import('./orca-runtime-tests/mobile-creation-and-orchestration-part-03.spec') diff --git a/src/main/runtime/terminal-retirement-async-durability.test.ts b/src/main/runtime/terminal-retirement-async-durability.test.ts index 038832782d7..28d2f1603a9 100644 --- a/src/main/runtime/terminal-retirement-async-durability.test.ts +++ b/src/main/runtime/terminal-retirement-async-durability.test.ts @@ -2,6 +2,7 @@ import { afterEach, expect, it, vi } from 'vitest' import { ACK_INCARNATION, ACK_LEAF, + ACK_SECOND_LEAF, ACK_TAB, createAcknowledgedTabRetirementFixture } from './acknowledged-terminal-tab-retirement-fixture' @@ -35,7 +36,7 @@ it('withholds the acknowledged close until its host retirement is durable', asyn expect(f.hasTab()).toBe(false) }) -it('publishes physical-exit retirement only after durability', async () => { +it('publishes physical-exit retirement before its durable write completes', async () => { const f = fixture() await f.store.flushPendingOrThrowAsync() const published = vi.fn() @@ -43,7 +44,8 @@ it('publishes physical-exit retirement only after durability', async () => { const gate = f.authority.pause() const exiting = f.runtime.onPtyExit('pty-a', 0, ACK_INCARNATION, { providerExitObserved: true }) await gate.started.promise - expect(published).not.toHaveBeenCalled() + // Why: a client re-activating the exited pane in this window must already find it retired. + expect(published).toHaveBeenCalled() gate.finish.resolve() await exiting expect(published).toHaveBeenCalled() @@ -53,7 +55,7 @@ it('publishes physical-exit retirement only after durability', async () => { unsubscribe() }) -it('does not publish a delayed exit over a newly admitted incarnation', async () => { +it('publishes nothing when the exit write lands after a replacement is admitted', async () => { const f = fixture() await f.store.flushPendingOrThrowAsync() const published = vi.fn() @@ -68,3 +70,58 @@ it('does not publish a delayed exit over a newly admitted incarnation', async () expect(published).not.toHaveBeenCalled() unsubscribe() }) + +async function exitWithFailedDurableWrite(f: ReturnType): Promise { + await f.store.flushPendingOrThrowAsync() + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined) + f.authority.failNextWrite() + await f.runtime.onPtyExit('pty-a', 0, ACK_INCARNATION, { providerExitObserved: true }) + expect(errorSpy).toHaveBeenCalledWith( + '[runtime] terminal retirement is not yet durable:', + expect.any(Error) + ) + errorSpy.mockRestore() + // The failed write left disk untouched, so a relaunch would still load the exited leaf. + expect(f.readDisk().workspaceSession.terminalLayoutsByTabId[ACK_TAB]?.ptyIdsByLeafId).toEqual({ + [ACK_LEAF]: 'pty-a', + [ACK_SECOND_LEAF]: 'pty-b' + }) +} + +it('persists a failed exit retirement with the next unrelated profile write', async () => { + const f = fixture() + await exitWithFailedDurableWrite(f) + f.store.addRepo({ + id: 'repo2', + path: '/tmp/other', + displayName: 'Other', + badgeColor: 'gray', + addedAt: 2 + }) + await f.store.flushPendingOrThrowAsync() + expect(f.readDisk().workspaceSession.terminalLayoutsByTabId[ACK_TAB]?.ptyIdsByLeafId).toEqual({ + [ACK_SECOND_LEAF]: 'pty-b' + }) +}) + +it('persists a failed exit retirement with the final quit flush', async () => { + const f = fixture() + await exitWithFailedDurableWrite(f) + await f.quit() + expect(f.readDisk().workspaceSession.terminalLayoutsByTabId[ACK_TAB]?.ptyIdsByLeafId).toEqual({ + [ACK_SECOND_LEAF]: 'pty-b' + }) +}) + +it('writes exits retired in the same tick once', async () => { + const f = fixture() + await f.store.flushPendingOrThrowAsync() + const writes = f.authority.captures.length + await Promise.all([ + f.runtime.onPtyExit('pty-a', 0, ACK_INCARNATION, { providerExitObserved: true }), + f.runtime.onPtyExit('pty-b', 0, undefined, { providerExitObserved: true }) + ]) + // Why: the first write already carries both retirements; the second has nothing left to fence. + expect(f.authority.captures.length - writes).toBe(1) + expect(f.readDisk().workspaceSession.terminalLayoutsByTabId[ACK_TAB]).toBeUndefined() +})