From 0399ffa2c6e78379bd9a264c5ab53f6b77fd90dc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:20:12 -0700 Subject: [PATCH] fix(terminal): re-bind a host-exit-settled remote pane on the host's next snapshot (#33062) * fix(terminal): hand a host-exit-settled remote pane back when the host binds a new process After #31968 settles a host-session pane on the host's exit record, a host restart can bind the new process just after the probe read the old one's exit. The pane then stayed ended until the tab was reopened. Keep watching the host's accepted snapshots after that settle; once one publishes a provably different live process for the pane, remount the tab so it attaches to it. * fix(terminal): let a new host process reopen recovery after a failed rebind * fix(terminal): watch for a host rebind only on a leaf-addressed pane * fix(terminal): catch a host rebind published while the exit probe confirms * fix(terminal): keep the host-rebind remount under the recovery window cap --- .../pty-connection/pane-transport-options.ts | 12 ++ .../terminal-pane/pty-transport-types.ts | 2 + ...ime-pty-transport-host-exit-record.test.ts | 113 +++++++++++++++++- .../remote-runtime-pty-transport.ts | 38 ++++++ .../remote-terminal-host-rebind-after-exit.ts | 57 +++++++++ ...inal-pane-recovery-host-rebind-cap.test.ts | 57 +++++++++ .../terminal-pane/terminal-pane-recovery.ts | 5 +- .../web-session-terminal-handle-events.ts | 5 +- .../terminals/terminal-tab-recovery-ledger.ts | 9 +- src/shared/terminal-tab-types.ts | 2 + 10 files changed, 292 insertions(+), 8 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/remote-terminal-host-rebind-after-exit.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-pane-recovery-host-rebind-cap.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts index c73567da815..3f1d192aa71 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts @@ -1,5 +1,6 @@ import { useAppStore } from '@/store' import { shouldRetainDisposedPaneSpawn } from './disposed-spawn-retention' +import { requestTerminalPaneRecovery } from '../terminal-pane-recovery' import type { IpcPtyTransportOptions } from '../pty-transport-types' import type { ConnectPanePtySession } from './connect-pane-pty-session' @@ -70,6 +71,17 @@ export function buildPaneTransportOptions(session: ConnectPanePtySession): IpcPt onPtyExit: session.onExit, onPtySpawn: session.onPtySpawn, onPtyRebind: session.onPtyRebind, + // Why a remount: the exit already tore down this mount's PTY state, so a fresh mount attaches. + onHostReboundExitedPty: (ptyId) => + void requestTerminalPaneRecovery({ + tabId: session.deps.tabId, + ptyId, + reason: 'host-rebound-exited-pane', + // Why: a new host process is fresh evidence a failed attempt can't refuse; the cap still holds. + trigger: 'host-evidence', + terminalRecoveryGeneration: session.terminalRecoveryGeneration, + terminalRecoveryInstanceId: session.terminalRecoveryInstance.id + }), retainDisposedSpawn: () => shouldRetainDisposedPaneSpawn( useAppStore.getState(), diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 22099a5c2ae..22e3afbc7d8 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -323,6 +323,8 @@ export type IpcPtyTransportOptions = { retainDisposedSpawn?: () => boolean /** Rebind an existing pane after its provider replaces the PTY identity. */ onPtyRebind?: (ptyId: string, replacedPtyId: string, incarnationId?: string | null) => void + /** The host bound a new live process to a pane this transport already ended on its exit record. */ + onHostReboundExitedPty?: (ptyId: string) => void onBell?: () => void onAgentBecameIdle?: (title: string, meta?: AgentIdleEdgeMeta) => void onAgentBecameWorking?: () => void diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-exit-record.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-exit-record.test.ts index 95ad606abf3..a29dbb9dec2 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-exit-record.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-exit-record.test.ts @@ -63,14 +63,18 @@ function answerShow(terminal: Record): string[] { return shown } -async function attachRestoredHostTab(onPtyExit: (ptyId: string, exitCode?: number) => void) { +async function attachRestoredHostTab( + onPtyExit: (ptyId: string, exitCode?: number) => void, + onHostReboundExitedPty?: (ptyId: string) => void +) { resolvedPaneHandle = 'terminal-ended' const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const transport = createRemoteRuntimePtyTransport('hub-env', { worktreeId: 'wt-1', tabId: 'web-terminal-host-tab-1', leafId: 'pane:1', - onPtyExit + onPtyExit, + ...(onHostReboundExitedPty ? { onHostReboundExitedPty } : {}) }) transport.attach({ existingPtyId: HOST_TAB_PTY_ID, callbacks: {} }) await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) @@ -78,6 +82,40 @@ async function attachRestoredHostTab(onPtyExit: (ptyId: string, exitCode?: numbe return transport } +async function publishHostSurface( + surface: { terminal: string; incarnationId?: string } | null, + snapshotVersion: number +): Promise { + const tab = { + type: 'terminal' as const, + id: 'host-tab-1::pane:1', + parentTabId: 'host-tab-1', + leafId: 'pane:1', + title: 'Terminal', + isActive: true + } + // Why dynamic: the harness resets modules, so the transport's channel is the freshly loaded one. + const { queueAcceptedWebSessionTerminalSnapshot } = + await import('../../runtime/web-session-terminal-handle-events') + queueAcceptedWebSessionTerminalSnapshot( + { + worktree: 'wt-1', + publicationEpoch: 'epoch-ready', + snapshotVersion, + activeGroupId: null, + activeTabId: tab.id, + activeTabType: 'terminal', + tabs: [ + surface + ? { ...tab, status: 'ready', ...surface } + : { ...tab, status: 'pending-handle', terminal: null } + ] + }, + 'hub-env' + ) + await new Promise((resolve) => setTimeout(resolve, 0)) +} + describe('remote runtime pane whose host session already ended (#31539)', () => { beforeEach(() => { resetRemoteRuntimeTransport() @@ -232,6 +270,77 @@ describe('remote runtime pane whose host session already ended (#31539)', () => transport.destroy?.() }) + it('hands a settled pane back once a host snapshot publishes its new process', async () => { + const onHostReboundExitedPty = vi.fn() + const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty) + answerShow({ connected: false, verdict: 'exited', incarnationId: 'inc-1' }) + refuseStream('terminal_handle_stale') + await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('ended')) + + await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-1' }, 4) + await publishHostSurface({ terminal: 'terminal-ended' }, 5) + await publishHostSurface(null, 6) + expect(onHostReboundExitedPty).not.toHaveBeenCalled() + + await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-2' }, 7) + await publishHostSurface({ terminal: 'terminal-later' }, 8) + expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-ended']]) + transport.destroy?.() + }) + + it('hands a settled pane back to a new handle the host binds to it', async () => { + const onHostReboundExitedPty = vi.fn() + const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty) + answerShow({ connected: false, exitCause: { kind: 'operator_close' } }) + refuseStream('no_connected_pty') + await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('ended')) + + await publishHostSurface({ terminal: 'terminal-restarted' }, 4) + + expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-restarted']]) + transport.destroy?.() + }) + + it('hands a settled pane back to a process the host published while the exit was confirming', async () => { + const onHostReboundExitedPty = vi.fn() + const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty) + answerShow({ connected: false, verdict: 'exited', incarnationId: 'inc-1' }) + let answerPane: (value: unknown) => void = () => {} + const answer = runtimeCall.getMockImplementation() + runtimeCall.mockImplementation(async (args: { method: string; params?: unknown }) => + args.method === 'terminal.resolvePane' + ? new Promise((resolve) => { + answerPane = resolve + }) + : answer?.(args) + ) + refuseStream('terminal_handle_stale') + await vi.waitFor(() => + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.resolvePane' }) + ) + ) + + await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-2' }, 4) + answerPane({ + ok: true, + result: { + terminal: { + handle: 'terminal-ended', + tabId: 'host-tab-1', + leafId: 'pane:1', + worktreeId: 'wt-1', + incarnationId: 'inc-1' + } + } + }) + + await vi.waitFor(() => + expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-ended']]) + ) + transport.destroy?.() + }) + it('settles a pane whose restore attach is refused as stale before it binds', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const onPtyExit = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 0720769eb9f..ce6ea995f2b 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -60,6 +60,10 @@ import { getRemoteRuntimeTerminalMultiplexer } from '../../runtime/remote-runtim import { REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE } from '../../runtime/remote-runtime-terminal-snapshot-state' import { restoredSnapshotPaintsPrintableContent } from './restored-snapshot-coverage' import { hostRecordsTerminalExit, sameWhenBothKnown } from './remote-terminal-host-exit-record' +import { + watchHostRebindAfterExit, + type HostRebindAfterExitWatch +} from './remote-terminal-host-rebind-after-exit' import type { RemoteRuntimeMultiplexedTerminal, RemoteRuntimeSnapshotOutcome @@ -226,6 +230,7 @@ export function createRemoteRuntimePtyTransport( onPtyExit, onPtySpawn, onPtyRebind, + onHostReboundExitedPty, onTitleChange, onBell, onAgentBecameIdle, @@ -271,6 +276,7 @@ export function createRemoteRuntimePtyTransport( let recoveryReplacementPolicy: HostHandleReplacementPolicy = 'reuse' let recoveryReplacementPolicyHandle: string | null = null let stopWaitingForPublishedHandle: (() => void) | null = null + let stopWatchingHostRebind: (() => void) | null = null let publishedHandleWaitEpoch: number | null = null // Why: a spent auto-recovery window is the evidence that licenses reattaching the fenced handle; explicit retries must not erase it. let autoRecoveryWindowSpent = false @@ -2209,6 +2215,8 @@ export function createRemoteRuntimePtyTransport( stopWaitingForPublishedHandle?.() stopWaitingForPublishedHandle = null publishedHandleWaitEpoch = null + stopWatchingHostRebind?.() + stopWatchingHostRebind = null } function isCurrentRemoteTerminal(targetHandle: string, targetPtyId: string | null): boolean { @@ -2262,6 +2270,7 @@ export function createRemoteRuntimePtyTransport( probeLifecycle === lifecycleEpoch && handle === targetHandle && sameWhenBothKnown(authoritativePtyIncarnationId, exited.incarnationId) + let rebindWatch = startHostRebindWatch() void (async () => { const { terminal } = await callRuntime<{ terminal?: RuntimeTerminalShow }>('terminal.show', { terminal: targetHandle @@ -2276,17 +2285,46 @@ export function createRemoteRuntimePtyTransport( return } remotePtyId ??= exitedPtyId + const exitedIncarnationId = terminal.incarnationId ?? authoritativePtyIncarnationId const cause = terminal.exitCause retireRemoteTerminalId(cause?.kind === 'exited' ? cause.exitCode : undefined) + if (rebindWatch && !destroyed) { + stopWatchingHostRebind = rebindWatch.dispose + const armed = rebindWatch + rebindWatch = null + armed.arm(targetHandle, exitedIncarnationId) + } })() .catch(() => {}) .finally(() => { + rebindWatch?.dispose() if (hostExitProbeHandle === targetHandle) { hostExitProbeHandle = null } }) } + // Why: a restart's new process can land just after the probe read the old one's exit (#31968). + function startHostRebindWatch(): HostRebindAfterExitWatch | null { + if (destroyed || !onHostReboundExitedPty || !worktreeId || !tabId || !leafId) { + return null + } + if (!isWebTerminalSurfaceTabId(tabId)) { + return null + } + const watchLifecycle = lifecycleEpoch + const environmentId = currentRuntimeEnvironmentId + return watchHostRebindAfterExit( + { environmentId, worktreeId, hostTabId: toHostSessionTabId(tabId), leafId }, + (nextHandle) => { + stopWatchingHostRebind = null + if (!destroyed && terminalEnded && watchLifecycle === lifecycleEpoch) { + onHostReboundExitedPty(toRemoteRuntimePtyId(nextHandle, environmentId)) + } + } + ) + } + async function hostPaneStillHoldsExitedTerminal( targetHandle: string, exited: Pick diff --git a/src/renderer/src/components/terminal-pane/remote-terminal-host-rebind-after-exit.ts b/src/renderer/src/components/terminal-pane/remote-terminal-host-rebind-after-exit.ts new file mode 100644 index 00000000000..d9f9c0c71a4 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-terminal-host-rebind-after-exit.ts @@ -0,0 +1,57 @@ +import { + subscribeAcceptedWebSessionTerminalHandle, + type WebSessionTerminalHandleUpdate +} from '../../runtime/web-session-terminal-handle-events' +import { sameWhenBothKnown } from './remote-terminal-host-exit-record' + +export type HostRebindAfterExitWatch = { + /** Starts judging updates against the settled exit, the one seen while probing included. */ + arm: (exitedHandle: string, exitedIncarnationId: string | null) => void + dispose: () => void +} + +/** Fires once the host publishes a provably different live process for an exit-settled pane. */ +export function watchHostRebindAfterExit( + args: { environmentId: string; worktreeId: string; hostTabId: string; leafId: string }, + onRebound: (handle: string) => void +): HostRebindAfterExitWatch { + let stopped = false + let exited: { handle: string; incarnationId: string | null } | null = null + // Why buffered: the replacement can be published while the exit probe is still confirming. + let latest: WebSessionTerminalHandleUpdate | null = null + const judge = (update: WebSessionTerminalHandleUpdate): void => { + if (stopped || !exited) { + return + } + if (!update.surfacePresent) { + dispose() + return + } + const next = update.terminalHandle + if ( + !next || + (next === exited.handle && sameWhenBothKnown(update.incarnationId, exited.incarnationId)) + ) { + return + } + dispose() + onRebound(next) + } + const stop = subscribeAcceptedWebSessionTerminalHandle(args, (update) => { + latest = update + judge(update) + }) + function dispose(): void { + stopped = true + stop() + } + return { + arm(exitedHandle, exitedIncarnationId) { + exited = { handle: exitedHandle, incarnationId: exitedIncarnationId } + if (latest) { + judge(latest) + } + }, + dispose + } +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery-host-rebind-cap.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery-host-rebind-cap.test.ts new file mode 100644 index 00000000000..e81ae1e972f --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery-host-rebind-cap.test.ts @@ -0,0 +1,57 @@ +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { + _resetTerminalPaneRecoveryForTests, + captureTerminalPaneRecoveryGeneration +} from './terminal-pane-recovery' +import { requestAndSettle } from './terminal-recovery-ledger-test-driver' +import { + recoveryLedgerMocks as mocks, + resetRecoveryLedgerStore, + setTerminalTabs +} from './terminal-recovery-ledger-test-store' + +vi.mock('@/store', async () => { + const store = await import('./terminal-recovery-ledger-test-store') + return { useAppStore: { getState: () => store.recoveryLedgerStoreState() } } +}) + +vi.mock('@/lib/crash-breadcrumb-recorder', async () => { + const store = await import('./terminal-recovery-ledger-test-store') + return { recordRendererCrashBreadcrumb: store.recoveryLedgerMocks.recordRendererCrashBreadcrumb } +}) + +beforeEach(() => { + _resetTerminalPaneRecoveryForTests() + resetRecoveryLedgerStore() + setTerminalTabs([{ id: 'tab-1' }]) +}) + +afterEach(() => { + vi.useRealTimers() +}) + +it('stops re-binding at the window cap when the host keeps contradicting its exit record', async () => { + vi.useFakeTimers() + const rebind = () => + requestAndSettle( + { + tabId: 'tab-1', + ptyId: 'remote:env@@terminal-1', + reason: 'host-rebound-exited-pane', + trigger: 'host-evidence', + terminalRecoveryGeneration: captureTerminalPaneRecoveryGeneration('tab-1') + }, + // Each remounted pane is settled as exited again by the same contradicting host. + 'failed' + ) + + const outcomes: boolean[] = [] + for (let attempt = 0; attempt < 5; attempt += 1) { + vi.setSystemTime(attempt * 20_000) + outcomes.push(await rebind()) + } + + expect(outcomes).toEqual([true, true, true, false, false]) + await vi.advanceTimersByTimeAsync(10 * 60_000) + expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(3) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts index edc9d18eb67..9928a6f075c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts @@ -101,7 +101,10 @@ function shouldScheduleRecoveryRetry( return request.terminalRecoveryGeneration !== undefined } if (decline.declinedBy === 'window-cap') { - return request.reason !== 'reattach-unverifiable' + // Why: a host that keeps contradicting itself must stop at the cap and leave the pane ended. + return ( + request.reason !== 'reattach-unverifiable' && request.reason !== 'host-rebound-exited-pane' + ) } // 'settled-failure' deliberately schedules nothing: a retry timer would be // the counting loop again. Only a new trigger reopens that reason. diff --git a/src/renderer/src/runtime/web-session-terminal-handle-events.ts b/src/renderer/src/runtime/web-session-terminal-handle-events.ts index cbd1cda8cce..3418f1f7fb6 100644 --- a/src/renderer/src/runtime/web-session-terminal-handle-events.ts +++ b/src/renderer/src/runtime/web-session-terminal-handle-events.ts @@ -7,6 +7,8 @@ import { hostSnapshotAffirmsWorktreeContents } from './host-session-snapshot-aut export type WebSessionTerminalHandleUpdate = { surfacePresent: boolean terminalHandle: string | null + /** The published handle's process lifetime, when the host stamps one. */ + incarnationId?: string | null } type TerminalHandleSubscriber = { @@ -49,7 +51,8 @@ function resolveSubscriberUpdate( mirroredSurfaces.find((surface) => surface.status === 'ready') return { surfacePresent: true, - terminalHandle: readySurface?.terminal ?? null + terminalHandle: readySurface?.terminal ?? null, + ...(readySurface?.incarnationId ? { incarnationId: readySurface.incarnationId } : {}) } } diff --git a/src/renderer/src/store/terminals/terminal-tab-recovery-ledger.ts b/src/renderer/src/store/terminals/terminal-tab-recovery-ledger.ts index 8f4e8c1c174..a872dd3693d 100644 --- a/src/renderer/src/store/terminals/terminal-tab-recovery-ledger.ts +++ b/src/renderer/src/store/terminals/terminal-tab-recovery-ledger.ts @@ -31,9 +31,10 @@ export const RECOVERY_COOLDOWN_MS = 15_000 export const RECOVERY_SETTLEMENT_TIMEOUT_MS = 31_000 /** Why a request exists at all. Only 'automatic' is subject to the - * settled-failure refusal: a user pressing Retry, or an external lifecycle - * remount, IS the new trigger the refusal is waiting for. */ -export type TerminalRecoveryTrigger = 'automatic' | 'user' | 'external' + * settled-failure refusal: a user pressing Retry, an external lifecycle + * remount, or a host publishing a new process IS the new trigger the refusal + * is waiting for. 'host-evidence' still counts against the window cap. */ +export type TerminalRecoveryTrigger = 'automatic' | 'user' | 'external' | 'host-evidence' export type TerminalRecoveryRemountRequest = { reason: TerminalPaneRecoveryReason @@ -133,7 +134,7 @@ export function admitTerminalRecoveryRemount( retryInMs: recent[0] + RECOVERY_WINDOW_MS - request.now } } - if (request.trigger === 'user') { + if (request.trigger === 'user' || request.trigger === 'host-evidence') { // The user asking again IS the new evidence. Only the window cap — the // backstop against a loop neither side can see — survives it. return ADMITTED diff --git a/src/shared/terminal-tab-types.ts b/src/shared/terminal-tab-types.ts index e61fcd91c9f..3d15923d0b8 100644 --- a/src/shared/terminal-tab-types.ts +++ b/src/shared/terminal-tab-types.ts @@ -22,6 +22,8 @@ export type TerminalPaneRecoveryReason = // binding. pty:data for the old id then lands in the pre-handler buffer, which // ACKs it — main's delivery health stays green while the pane shows nothing. | 'spawn-left-pane-unbound' + // A pane ended on its host's exit record, then the host bound a new process to it. + | 'host-rebound-exited-pane' export type TerminalPaneRecoveryOutcome = | 'pending'