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() +})