From 5c45337a6f397c4f802f6990583db737eb034bfa Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:10:48 -0700 Subject: [PATCH] fix(terminal): make Codex restart replace the pane's process instead of reattaching it (#22737) * fix(terminal): make Codex restart replace the pane's process instead of reattaching it A spawn for a pane that still has a live process is treated as a reattach. Both Codex restart paths raced that: the open-pane restart killed the old PTY without waiting and then spawned, and the unmounted-tab restart spawned before killing. Either way the "fresh" spawn could re-adopt the old Codex (dialog comes back) or hit the half-killed session and leave a plain shell. pty:spawn now accepts replacesPtyId. Main stops that PTY and waits for the exit before the spawn resolves the pane owner, so the existing dead-owner path launches fresh and records the current Codex home. Both restart paths send it and no longer kill the old PTY themselves. Fixes #18174 * fix(terminal): send the replaced PTY once and keep the hidden-pane replacement Two follow-ups to the restart handoff: - The replaced PTY id rode the transport options, which outlive the first spawn. A later fresh spawn from the same pane (for example a hibernation wake) re-sent it, so main stopped that id again; SSH relay ids restart at pty-1, so it could name another pane's PTY. It is now a connection input consumed by the first fresh spawn only. - The hidden-tab restart still treated a changed binding after the spawn as a reason to stand down and reap the replacement. Main has already stopped the old PTY by then, so its exit can clear the tab binding (a background-launch exit observer does) or a pane can mount, and standing down left the pane with no process. It now keeps the replacement unless the tab or leaf was actually taken over. * fix(terminal): hold the pane while a restart stops its old process A restart spawn stopped the pane's old process before reserving the pane, so a hidden tab revealed during the stop could reattach the dying process (and the restart could then join that reattach and return the old id). The replacing spawn now reserves the pane before the stop, so any spawn for the pane in that window joins the replacement; a failed stop still settles the reservation. The hidden-tab restart also tombstones the old PTY's buffered exit before spawning, so a reveal mid-restart reconnects by pane identity instead of replaying that exit into the pane. * fix(terminal): label a restart's replaced-PTY exit and keep its stop owed until sent Main now marks the PTY it stops for a replacing spawn and stamps `replacedByRestart` on that PTY's exit (provider-observed, synthetic, and SSH-unregistered stops all reach the renderer through the same finalize step). A failed stop clears the mark with nothing sent; an undelivered mark expires. The renderer classifies the labeled exit before any consumer runs: parked-tab watchers end their subscription instead of collapsing the leaf or closing the tab, the pre-attach buffer discards instead of queuing a death for a later mount, and a mounted pane treats it as an intentional restart. With that, the hidden-tab restart no longer tears down its parked watchers and buffered output before spawning; it releases them only after the swap, so a stop that fails leaves the still-running Codex fully observed. The visible restart's pane session now holds the replaced PTY until a spawn request actually carries it (the IPC transport claims it as it sends). If the pane is closed or parked first, disposal stops that PTY with an ordinary kill instead of orphaning it. * test(terminal): type restart test fakes and merge a duplicate import Co-Authored-By: Claude --------- Co-authored-by: Claude --- src/main/ipc/pty-pane-restart-replace.test.ts | 283 ++++++++++++++++++ src/main/ipc/pty/delivery/exit.ts | 52 +++- src/main/ipc/pty/ipc/renderer-kill.ts | 167 ++++++----- src/main/ipc/pty/ipc/spawn-begin.ts | 81 +++-- src/main/ipc/pty/ipc/spawn-run.ts | 25 +- src/main/ipc/pty/ipc/spawn-types.ts | 3 + src/main/ipc/pty/pane/spawn-reservation.ts | 17 ++ src/main/ipc/pty/register-handlers.ts | 28 +- src/main/ipc/pty/session.ts | 3 + src/preload/api/pty-api.ts | 4 + src/preload/api/pty-bridge-session-control.ts | 2 + .../pty-bridge-stream-and-serialization.ts | 3 + .../codex-detached-pane-restart.test.ts | 117 +++++++- .../codex-detached-pane-restart.ts | 40 ++- .../terminal-pane/ipc-pty-spawn-request.ts | 5 +- .../pane-restart-transport-handoff.test.ts | 81 +++++ .../pane-restart-transport-handoff.ts | 22 ++ .../pty-connection-hibernation-wake.test.ts | 117 +++++++- .../terminal-pane/pty-connection-types.ts | 2 + .../pty-connection/connect-pane-pty.ts | 9 + .../pty-connection/fresh-spawn-start.ts | 3 + .../pty-connection/pty-exit-hibernate.ts | 6 +- .../session-reconcile-dispose.ts | 7 + .../terminal-pane/pty-dispatcher.ts | 1 + .../terminal-pane/pty-exit-delivery.ts | 31 +- .../terminal-pane/pty-transport-types.ts | 3 + .../terminal-parked-pty-watcher.ts | 7 + ...arked-watcher-sleep-preserved-exit.test.ts | 20 ++ .../use-terminal-pane-process-exit-actions.ts | 4 +- 29 files changed, 985 insertions(+), 158 deletions(-) create mode 100644 src/main/ipc/pty-pane-restart-replace.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.ts diff --git a/src/main/ipc/pty-pane-restart-replace.test.ts b/src/main/ipc/pty-pane-restart-replace.test.ts new file mode 100644 index 00000000000..e8e91ea7d45 --- /dev/null +++ b/src/main/ipc/pty-pane-restart-replace.test.ts @@ -0,0 +1,283 @@ +import { describe, expect, it, vi } from 'vitest' +import { setupPtyIpcSuite, type PtyIpcSuiteFixtures } from './pty-ipc-test-harness' +import { SessionNotFoundError } from '../daemon/daemon-errors' +import { makePaneKey } from '../../shared/stable-pane-id' +import { registerPtyHandlers, setLocalPtyProvider } from './pty' + +vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) +vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) +vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) +vi.mock('node:child_process', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) +) +vi.mock('../opencode/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) +) +vi.mock('../mimo/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) +) +vi.mock('../agent-hooks/server', () => + import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) +) +vi.mock('../pi/titlebar-extension-service', () => + import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) +) +vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) +vi.mock('../wsl', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) +) +vi.mock('../telemetry/client', () => + import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) +) +vi.mock('../telemetry/classify-error', () => + import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) +) +vi.mock('../cli/linux-terminal-orca-cli-shim', () => + import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) +) +vi.mock('../memory/pty-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) +) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => + import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) +) +vi.mock('../codex/codex-pane-account-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) +) +vi.mock('../codex/codex-state-db-backfill-recovery', () => + import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) +) + +const worktreeId = 'repo-1::/tmp/restart' +const cwd = '/tmp/restart' +const tabId = 'tab-restart' +const leafId = '12121212-1212-4212-8212-121212121212' +const paneKey = makePaneKey(tabId, leafId) + +type RestartHarness = ReturnType + +function registerWithFakes( + mainWindow: PtyIpcSuiteFixtures['mainWindow'], + runtime: RestartHarness['runtime'], + store: RestartHarness['store'] +): void { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: spawn and kill read only the window, runtime and store members these fakes define. + const args = [ + mainWindow, + runtime, + undefined, + undefined, + undefined, + store + ] as unknown as Parameters + registerPtyHandlers(...args) +} + +function installRestartHarness( + options: { shutdownFails?: boolean; shutdownGate?: Promise } = {} +) { + let oldSessionAlive = true + const control = { shutdownFails: options.shutdownFails ?? false } + const providerSpawn = vi.fn(async (spawnOptions: { attachOnly?: boolean }) => { + if (!spawnOptions.attachOnly) { + return { id: 'pty-new', incarnationId: 'inc-new' } + } + if (!oldSessionAlive) { + throw new SessionNotFoundError('pty-old') + } + return { id: 'pty-old', incarnationId: 'inc-old', isReattach: true } + }) + const shutdown = vi.fn(async () => { + await options.shutdownGate + if (control.shutdownFails) { + throw new Error('daemon unreachable') + } + oldSessionAlive = false + }) + const provider = { + spawn: providerSpawn, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown, + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}), + listProcesses: vi.fn(async () => []), + attach: vi.fn(), + getDefaultShell: vi.fn(), + getProfiles: vi.fn() + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the restart path calls only the provider members this fake defines. + setLocalPtyProvider(provider as unknown as Parameters[0]) + let session = { + tabsByWorktree: { [worktreeId]: [{ id: tabId, worktreeId, ptyId: 'pty-old' }] }, + terminalLayoutsByTabId: { + [tabId]: { + root: { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: 'pty-old' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-old' } + } + const store = { + getWorkspaceSession: vi.fn(() => session), + setWorkspaceSession: vi.fn((next) => { + session = next + }), + flushOrThrow: vi.fn(), + persistPtyBinding: vi.fn(), + getFolderWorkspace: vi.fn(() => undefined), + getFolderWorkspaces: vi.fn(() => []), + getProjectGroups: vi.fn(() => []), + getRepos: vi.fn(() => []) + } + const runtime = { + setPtyController: vi.fn(), + resolveTerminalPane: vi.fn(() => { + throw new Error('terminal_not_found') + }), + markPtyStopRequested: vi.fn(), + createPreAllocatedTerminalHandle: vi.fn(() => 'term-restart'), + preAllocateHandleForPty: vi.fn(() => 'term-restart'), + registerPreAllocatedHandleForPty: vi.fn(), + beginPtyRegistration: vi.fn(), + cancelPendingPtyRegistration: vi.fn(), + assertPtyRegistrationAllowed: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + seedHeadlessTerminal: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn() + } + return { providerSpawn, shutdown, store, runtime, control } +} + +function restartSpawnArgs(extra: { replacesPtyId?: string } = {}) { + return { + cols: 80, + rows: 24, + cwd, + command: 'codex', + launchAgent: 'codex', + worktreeId, + tabId, + leafId, + env: { ORCA_PANE_KEY: paneKey, ORCA_TAB_ID: tabId, ORCA_WORKTREE_ID: worktreeId }, + ...extra + } +} + +describe('pty:spawn replacing a pane owner', () => { + const { handlers, mainWindow } = setupPtyIpcSuite() + + function exitPayloads(id: string): Record[] { + return mainWindow.webContents.send.mock.calls + .filter(([channel, payload]) => channel === 'pty:exit' && payload?.id === id) + .map(([, payload]) => payload) + } + + it('reattaches a live pane owner when the spawn does not name it as replaced', async () => { + const { providerSpawn, store, runtime } = installRestartHarness() + registerWithFakes(mainWindow, runtime, store) + + const spawned = await handlers.get('pty:spawn')!(null, restartSpawnArgs()) + + expect(spawned).toMatchObject({ id: 'pty-old', isReattach: true }) + expect(providerSpawn).toHaveBeenCalledTimes(1) + }) + + it('stops the replaced owner and launches fresh instead of reattaching it', async () => { + const { providerSpawn, shutdown, store, runtime } = installRestartHarness() + registerWithFakes(mainWindow, runtime, store) + + const spawned = await handlers.get('pty:spawn')!( + null, + restartSpawnArgs({ replacesPtyId: 'pty-old' }) + ) + + expect(spawned).toMatchObject({ id: 'pty-new' }) + expect(shutdown).toHaveBeenCalledWith('pty-old', expect.objectContaining({ immediate: true })) + expect(spawned).not.toHaveProperty('isReattach', true) + const freshLaunch = providerSpawn.mock.calls.find(([options]) => !options.attachOnly)?.[0] + expect(freshLaunch).toMatchObject({ command: 'codex' }) + expect(shutdown.mock.invocationCallOrder[0]).toBeLessThan( + providerSpawn.mock.invocationCallOrder[0]! + ) + }) + + it('labels the replaced owner exit so the renderer keeps the pane', async () => { + const { store, runtime } = installRestartHarness() + registerWithFakes(mainWindow, runtime, store) + + await handlers.get('pty:spawn')!(null, restartSpawnArgs({ replacesPtyId: 'pty-old' })) + + expect(exitPayloads('pty-old')).toEqual([ + expect.objectContaining({ id: 'pty-old', replacedByRestart: true }) + ]) + }) + + it('never labels an ordinary close', async () => { + const { store, runtime } = installRestartHarness() + registerWithFakes(mainWindow, runtime, store) + + await handlers.get('pty:kill')!(null, { id: 'pty-old' }) + + expect(exitPayloads('pty-old')).toHaveLength(1) + expect(exitPayloads('pty-old')[0]).not.toHaveProperty('replacedByRestart') + }) + + it('hands a spawn for the pane that arrives mid-stop the replacement, not the dying owner', async () => { + let finishShutdown!: () => void + const shutdownGate = new Promise((resolve) => { + finishShutdown = resolve + }) + const { providerSpawn, shutdown, store, runtime } = installRestartHarness({ shutdownGate }) + registerWithFakes(mainWindow, runtime, store) + + const restart = handlers.get('pty:spawn')!(null, restartSpawnArgs({ replacesPtyId: 'pty-old' })) + await vi.waitFor(() => expect(shutdown).toHaveBeenCalledTimes(1)) + // A hidden tab revealed now reconnects its pane while the old owner is still alive. + const reveal = handlers.get('pty:spawn')!(null, restartSpawnArgs()) + finishShutdown() + + await expect(restart).resolves.toMatchObject({ id: 'pty-new' }) + await expect(reveal).resolves.toMatchObject({ id: 'pty-new', isReattach: true }) + expect(providerSpawn.mock.calls.filter(([options]) => !options.attachOnly)).toHaveLength(1) + }) + + it('refuses to launch a second process when the replaced owner could not be stopped', async () => { + const { providerSpawn, store, runtime, control } = installRestartHarness({ + shutdownFails: true + }) + registerWithFakes(mainWindow, runtime, store) + + await expect( + handlers.get('pty:spawn')!(null, restartSpawnArgs({ replacesPtyId: 'pty-old' })) + ).rejects.toThrow('daemon unreachable') + expect(providerSpawn).not.toHaveBeenCalled() + expect(exitPayloads('pty-old')).toEqual([]) + // The pane is released: a later spawn still reaches the surviving owner instead of hanging. + await expect(handlers.get('pty:spawn')!(null, restartSpawnArgs())).resolves.toMatchObject({ + id: 'pty-old', + isReattach: true + }) + // The failed restart left no label behind: a later close of the same PTY reads as a close. + control.shutdownFails = false + await handlers.get('pty:kill')!(null, { id: 'pty-old' }) + expect(exitPayloads('pty-old')).toHaveLength(1) + expect(exitPayloads('pty-old')[0]).not.toHaveProperty('replacedByRestart') + }) +}) diff --git a/src/main/ipc/pty/delivery/exit.ts b/src/main/ipc/pty/delivery/exit.ts index aa26bb914bc..9aba356b3c3 100644 --- a/src/main/ipc/pty/delivery/exit.ts +++ b/src/main/ipc/pty/delivery/exit.ts @@ -7,8 +7,57 @@ import { allocatePtyLifecycleSequence } from '../host-env/types' import { makePtyDataPayload, sendPtyDataToRenderer } from './payload' import { getRendererInFlightCharsForPty } from './accounting' import { clearFlushTimerIfIdle } from './flush' +import { ptyIncarnationById } from '../provider/ownership-state' import type { PtyIpcSession } from '../session' +export type ReplacedPtyStop = { + incarnationId: string | undefined + expiryTimer?: NodeJS.Timeout +} + +/** Labels the exit of a PTY that main stops so a new process can take its pane. Settle with + * whether the stop succeeded; a failed stop leaves no label behind. */ +export function markReplacedPtyStop( + session: PtyIpcSession, + id: string +): (stopped: boolean) => void { + clearTimeout(session.replacedPtyStopsById.get(id)?.expiryTimer) + const mark: ReplacedPtyStop = { incarnationId: ptyIncarnationById.get(id) } + session.replacedPtyStopsById.set(id, mark) + return (stopped) => { + if (session.replacedPtyStopsById.get(id) !== mark) { + return + } + if (!stopped) { + session.replacedPtyStopsById.delete(id) + return + } + // Why a window: an SSH exit can reach the renderer after the stop settles; bound it like a synthetic kill. + mark.expiryTimer = setTimeout(() => { + if (session.replacedPtyStopsById.get(id) === mark) { + session.replacedPtyStopsById.delete(id) + } + }, SYNTHETIC_KILL_EXIT_DUPLICATE_WINDOW_MS) + mark.expiryTimer.unref?.() + } +} + +function consumeReplacedPtyStop( + session: PtyIpcSession, + payload: { id: string; incarnationId?: string } +): boolean { + const mark = session.replacedPtyStopsById.get(payload.id) + if ( + !mark || + (mark.incarnationId && payload.incarnationId && mark.incarnationId !== payload.incarnationId) + ) { + return false + } + clearTimeout(mark.expiryTimer) + session.replacedPtyStopsById.delete(payload.id) + return true +} + export function rememberSyntheticKillExit( session: PtyIpcSession, id: string, @@ -157,7 +206,8 @@ export function finalizePtyExitForRenderer( ...payload, ...(session.reversibleStopOwnersByPtyId.has(payload.id) ? { preserveRendererBinding: true } - : {}) + : {}), + ...(consumeReplacedPtyStop(session, payload) ? { replacedByRestart: true } : {}) }) } diff --git a/src/main/ipc/pty/ipc/renderer-kill.ts b/src/main/ipc/pty/ipc/renderer-kill.ts index 9d8dfa8c30b..8f73e72aa59 100644 --- a/src/main/ipc/pty/ipc/renderer-kill.ts +++ b/src/main/ipc/pty/ipc/renderer-kill.ts @@ -26,7 +26,33 @@ export type PtyKillIpcDeps = { * implementation from `killPtyFromRuntimeController`. Both have to record an undelivered SSH stop, * and this is the one ordinary tab close actually reaches. */ export function installPtyKillIpcHandler(deps: PtyKillIpcDeps): void { - const ipcMain = getPtyIpc() + getPtyIpc().handle('pty:kill', (_event, args: { id: string; keepHistory?: boolean }) => + stopRendererOwnedPty(deps, args) + ) +} + +/** Stops a pane's PTY for the spawn replacing it. `markReplaced` labels the exit so the renderer + * reads it as a handoff, not the pane dying; a failed stop removes the label with nothing sent. */ +export async function stopReplacedPanePty( + deps: PtyKillIpcDeps, + id: string, + markReplaced: (id: string) => (stopped: boolean) => void +): Promise { + const settle = markReplaced(id) + try { + await stopRendererOwnedPty(deps, { id }) + } catch (err) { + settle(false) + throw err + } + settle(true) +} + +/** Stops a renderer-owned PTY and settles only once its shutdown has been observed or synthesized. */ +export async function stopRendererOwnedPty( + deps: PtyKillIpcDeps, + args: { id: string; keepHistory?: boolean } +): Promise { const { store, runtime, @@ -35,77 +61,74 @@ export function installPtyKillIpcHandler(deps: PtyKillIpcDeps): void { rememberSyntheticKillExit, sendPtyExitToRenderer } = deps - - ipcMain.handle('pty:kill', async (_event, args: { id: string; keepHistory?: boolean }) => { - if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) { - // Why: runtime terminal handles belong to terminal.close; unowned PTY routing could target the local provider. - throw new Error('Invalid PTY provider id') - } - runtime?.markPtyStopRequested?.(args.id) - const ownedConnectionId = ptyOwnership.get(args.id) - const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null - const connectionId = ownedConnectionId ?? parsedSshId?.connectionId - // Why: wait for daemon startup before selecting the local provider, else a fallback shutdown falsely succeeds and orphans a restored daemon PTY (#7742). - const startupPromise = getLocalPtyProviderStartupPromise(connectionId) - if (startupPromise) { - await startupPromise - } - // Why stated rather than inferred: this IPC serves both the ordinary tab close and pane - // hibernation, and only hibernation passes keepHistory. Recording a replayable kill for a - // hibernating pane would destroy it on the next handshake. - const reversible = args.keepHistory === true - const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id) - if (!provider && connectionId) { - // Why: detached SSH PTYs intentionally keep ownership after their - // provider is unregistered; hydrated app-scoped ids can also arrive - // before ownership is rebuilt. Tombstone instead of falling back local. - const incarnationId = finishPtyShutdown(args.id, connectionId, store) - // The relay was never asked, so the remote shell is still running. Keep the order. - recordUndeliveredSshPtyKill({ - store, - ptyId: args.id, - connectionId, - reversible, - incarnationId - }) - runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON) - runtime?.onPtyExit(args.id, -1, incarnationId) - rememberSyntheticKillExit(args.id, incarnationId) - sendPtyExitToRenderer({ - id: args.id, - code: -1, - ...(incarnationId ? { incarnationId } : {}) - }) - return - } - const shutdownProvider = provider ?? getProviderForPty(args.id) - let providerExitObserved = false - try { - providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, { - immediate: true, - keepHistory: args.keepHistory ?? false - }) - } catch (err) { - if (!isPtyAlreadyGoneError(err)) { - // Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry. - // The renderer has already discarded the tab, so nothing here retries — the durable order - // is what lets the next handshake to this host finish the close. - recordUndeliveredSshPtyKill({ store, ptyId: args.id, connectionId, reversible }) - throw err - } - /* session already dead — cleanup below handles the rest */ - } - // Why: some shutdown paths do not emit onExit through the provider listener. - // Explicit cleanup is idempotent and covers already-dead PTYs. + if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) { + // Why: runtime terminal handles belong to terminal.close; unowned PTY routing could target the local provider. + throw new Error('Invalid PTY provider id') + } + runtime?.markPtyStopRequested?.(args.id) + const ownedConnectionId = ptyOwnership.get(args.id) + const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null + const connectionId = ownedConnectionId ?? parsedSshId?.connectionId + // Why: wait for daemon startup before selecting the local provider, else a fallback shutdown falsely succeeds and orphans a restored daemon PTY (#7742). + const startupPromise = getLocalPtyProviderStartupPromise(connectionId) + if (startupPromise) { + await startupPromise + } + // Why stated rather than inferred: this IPC serves both the ordinary tab close and pane + // hibernation, and only hibernation passes keepHistory. Recording a replayable kill for a + // hibernating pane would destroy it on the next handshake. + const reversible = args.keepHistory === true + const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id) + if (!provider && connectionId) { + // Why: detached SSH PTYs intentionally keep ownership after their + // provider is unregistered; hydrated app-scoped ids can also arrive + // before ownership is rebuilt. Tombstone instead of falling back local. const incarnationId = finishPtyShutdown(args.id, connectionId, store) - if (!providerExitObserved) { - runtime?.onPtyExit(args.id, -1, incarnationId) - rememberSyntheticKillExit(args.id, incarnationId) - sendPtyExitToRenderer({ - id: args.id, - code: -1, - ...(incarnationId ? { incarnationId } : {}) - }) + // The relay was never asked, so the remote shell is still running. Keep the order. + recordUndeliveredSshPtyKill({ + store, + ptyId: args.id, + connectionId, + reversible, + incarnationId + }) + runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON) + runtime?.onPtyExit(args.id, -1, incarnationId) + rememberSyntheticKillExit(args.id, incarnationId) + sendPtyExitToRenderer({ + id: args.id, + code: -1, + ...(incarnationId ? { incarnationId } : {}) + }) + return + } + const shutdownProvider = provider ?? getProviderForPty(args.id) + let providerExitObserved = false + try { + providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, { + immediate: true, + keepHistory: args.keepHistory ?? false + }) + } catch (err) { + if (!isPtyAlreadyGoneError(err)) { + // Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry. + // The renderer has already discarded the tab, so nothing here retries — the durable order + // is what lets the next handshake to this host finish the close. + recordUndeliveredSshPtyKill({ store, ptyId: args.id, connectionId, reversible }) + throw err } - }) + /* session already dead — cleanup below handles the rest */ + } + // Why: some shutdown paths do not emit onExit through the provider listener. + // Explicit cleanup is idempotent and covers already-dead PTYs. + const incarnationId = finishPtyShutdown(args.id, connectionId, store) + if (!providerExitObserved) { + runtime?.onPtyExit(args.id, -1, incarnationId) + rememberSyntheticKillExit(args.id, incarnationId) + sendPtyExitToRenderer({ + id: args.id, + code: -1, + ...(incarnationId ? { incarnationId } : {}) + }) + } } diff --git a/src/main/ipc/pty/ipc/spawn-begin.ts b/src/main/ipc/pty/ipc/spawn-begin.ts index 0b59d347e4d..bc0e86ccd2d 100644 --- a/src/main/ipc/pty/ipc/spawn-begin.ts +++ b/src/main/ipc/pty/ipc/spawn-begin.ts @@ -14,6 +14,24 @@ import { } from '../pane/spawn-reservation' import { resolveStablePaneOwner } from '../pane/stable-owner' import type { PtyIpcSpawnState } from './spawn-state' +import type { PtySpawnIpcArgs } from './spawn-types' + +/** The pane key a spawn names before preflight; null when it names no stable pane. */ +function resolveEarlyPaneKey(args: PtySpawnIpcArgs): string | null { + const leafId = + typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null + return typeof args.worktreeId === 'string' && + typeof args.tabId === 'string' && + isValidTerminalTabId(args.tabId) && + args.tabId.length <= 512 && + leafId + ? makePaneKey(args.tabId, leafId) + : null +} + +export function resolveEarlyPaneSpawnReservationKey(args: PtySpawnIpcArgs): string | null { + return makePaneSpawnReservationKey(args.worktreeId, args.connectionId, resolveEarlyPaneKey(args)) +} export async function beginPtyIpcSpawn( ctx: PtyIpcSpawnState @@ -23,16 +41,7 @@ export async function beginPtyIpcSpawn( ctx.codexHomeLaunchStartedSequence = !args.connectionId ? allocatePtyLifecycleSequence() : undefined - const initialLeafId = - typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null - const initialPaneKey = - typeof args.worktreeId === 'string' && - typeof args.tabId === 'string' && - isValidTerminalTabId(args.tabId) && - args.tabId.length <= 512 && - initialLeafId - ? makePaneKey(args.tabId, initialLeafId) - : null + const initialPaneKey = resolveEarlyPaneKey(args) const initialStablePanePtyId = (() => { try { return !args.connectionId && initialPaneKey @@ -59,39 +68,29 @@ export async function beginPtyIpcSpawn( ctx.spawnTiming = createPtySpawnTiming() ctx.cwd = ctx.deps.resolvePtySpawnStartupCwd(args.worktreeId, args.cwd) - const earlyLeafId = - typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null - const earlyPaneKey = - typeof args.worktreeId === 'string' && - typeof args.tabId === 'string' && - isValidTerminalTabId(args.tabId) && - args.tabId.length <= 512 && - earlyLeafId - ? makePaneKey(args.tabId, earlyLeafId) - : null - const earlyReservationKey = makePaneSpawnReservationKey( - args.worktreeId, - args.connectionId, - earlyPaneKey - ) - const pendingRuntimeCreate = earlyReservationKey - ? pendingRuntimePaneCreatesByOwnerKey.get(earlyReservationKey) - : undefined - if (pendingRuntimeCreate) { - await pendingRuntimeCreate.promise - } - const existingPaneSpawn = earlyReservationKey - ? paneSpawnReservationsByOwnerKey.get(earlyReservationKey) - : undefined - if (existingPaneSpawn) { - return { ...(await existingPaneSpawn.promise), isReattach: true } + const earlyReservationKey = resolveEarlyPaneSpawnReservationKey(args) + // Why: a replacing spawn reserved its pane before stopping the owner; joining itself would deadlock. + const heldReservation = ctx.paneSpawnReservation + if (!heldReservation) { + const pendingRuntimeCreate = earlyReservationKey + ? pendingRuntimePaneCreatesByOwnerKey.get(earlyReservationKey) + : undefined + if (pendingRuntimeCreate) { + await pendingRuntimeCreate.promise + } + const existingPaneSpawn = earlyReservationKey + ? paneSpawnReservationsByOwnerKey.get(earlyReservationKey) + : undefined + if (existingPaneSpawn) { + return { ...(await existingPaneSpawn.promise), isReattach: true } + } } ctx.earlyStablePaneOwner = - earlyPaneKey && args.worktreeId + initialPaneKey && args.worktreeId ? resolveStablePaneOwner( ctx.deps.runtime, ctx.deps.store, - earlyPaneKey, + initialPaneKey, args.worktreeId, args.connectionId ) @@ -99,9 +98,9 @@ export async function beginPtyIpcSpawn( ctx.earlyWorktreeId = args.worktreeId // Reserve early so renderer/runtime materialization cannot start duplicate provider spawns. ctx.paneSpawnReservationKey = earlyReservationKey - ctx.paneSpawnReservation = ctx.paneSpawnReservationKey - ? reservePaneSpawn(ctx.paneSpawnReservationKey) - : null + ctx.paneSpawnReservation = + heldReservation ?? + (ctx.paneSpawnReservationKey ? reservePaneSpawn(ctx.paneSpawnReservationKey) : null) ctx.finishTerminalInstall = (): void => {} ctx.stablePaneOwner = null ctx.stablePaneBindingPersisted = false diff --git a/src/main/ipc/pty/ipc/spawn-run.ts b/src/main/ipc/pty/ipc/spawn-run.ts index 82e2d33f383..7efeb23538d 100644 --- a/src/main/ipc/pty/ipc/spawn-run.ts +++ b/src/main/ipc/pty/ipc/spawn-run.ts @@ -1,6 +1,6 @@ -import { rejectPaneSpawnReservation } from '../pane/spawn-reservation' +import { rejectPaneSpawnReservation, reserveIdlePaneSpawn } from '../pane/spawn-reservation' import { ptySizes } from '../delivery/visibility-state' -import { beginPtyIpcSpawn } from './spawn-begin' +import { beginPtyIpcSpawn, resolveEarlyPaneSpawnReservationKey } from './spawn-begin' import { preparePtyIpcSpawnPreflight } from './spawn-preflight' import { assemblePtyIpcSpawnEnv } from './spawn-env' import { buildPtyIpcSpawnOptions } from './spawn-options' @@ -31,13 +31,26 @@ function restoreProvisionalPtySize(ctx: PtyIpcSpawnState): void { } export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArgs) { - triggerPtySpawnPushTargetMaterialization(deps, args) const ctx = createPtyIpcSpawnState(deps, args) - const early = await beginPtyIpcSpawn(ctx) - if (early) { - return early + const replacedPaneKey = + args.replacesPtyId !== undefined ? resolveEarlyPaneSpawnReservationKey(args) : null + if (replacedPaneKey) { + // Why: hold the pane across the stop, so a spawn for it arriving meanwhile (a hidden tab + // revealed mid-restart) joins the replacement instead of reattaching the owner being stopped. + ctx.paneSpawnReservation = await reserveIdlePaneSpawn(replacedPaneKey) + ctx.paneSpawnReservationKey = replacedPaneKey } try { + if (args.replacesPtyId !== undefined) { + // Why: stop before resolving the pane owner, so the spawn below finds a dead owner and + // launches fresh instead of reattaching the process this restart exists to replace. + await deps.stopReplacedPty(args.replacesPtyId) + } + triggerPtySpawnPushTargetMaterialization(deps, args) + const early = await beginPtyIpcSpawn(ctx) + if (early) { + return early + } await preparePtyIpcSpawnPreflight(ctx) await assemblePtyIpcSpawnEnv(ctx) const earlyReserved = await buildPtyIpcSpawnOptions(ctx).catch((error: unknown) => { diff --git a/src/main/ipc/pty/ipc/spawn-types.ts b/src/main/ipc/pty/ipc/spawn-types.ts index c80bcb53819..e14b469d1fb 100644 --- a/src/main/ipc/pty/ipc/spawn-types.ts +++ b/src/main/ipc/pty/ipc/spawn-types.ts @@ -51,6 +51,8 @@ export type PtySpawnIpcArgs = { // Why: closes the SIGKILL race (INVESTIGATION.md) by letting main sync-flush the binding before pty:spawn returns; only the Ctrl+T daemon-host path threads these. tabId?: string leafId?: string + // Why: a pane with a live owner is otherwise reattached, so a restart names the PTY it replaces. + replacesPtyId?: string // Why: renderer-threaded launch telemetry (telemetry-plan.md§Agent launch semantics); loosely typed because the main-side schema validator is the single enforcement point. telemetry?: { agent_kind?: unknown @@ -121,4 +123,5 @@ export type PtySpawnIpcDeps = { trustedTerminalHandleEnv: Set sendPtySpawnedToRenderer: (id: string) => void syncPtyBackgroundedDelivery: (id: string, caller: string) => void + stopReplacedPty: (id: string) => Promise } diff --git a/src/main/ipc/pty/pane/spawn-reservation.ts b/src/main/ipc/pty/pane/spawn-reservation.ts index 47c968b9b5a..936854347aa 100644 --- a/src/main/ipc/pty/pane/spawn-reservation.ts +++ b/src/main/ipc/pty/pane/spawn-reservation.ts @@ -38,6 +38,23 @@ export function reservePaneSpawn(paneKey: string): PaneSpawnReservation { return reservation } +/** Reserves the pane once no runtime create or other spawn holds it. */ +export async function reserveIdlePaneSpawn(ownerKey: string): Promise { + for (;;) { + const pendingCreate = pendingRuntimePaneCreatesByOwnerKey.get(ownerKey) + if (pendingCreate) { + await pendingCreate.promise + continue + } + const pendingSpawn = paneSpawnReservationsByOwnerKey.get(ownerKey) + if (pendingSpawn) { + await pendingSpawn.promise.catch(() => {}) + continue + } + return reservePaneSpawn(ownerKey) + } +} + export function clearPaneSpawnReservation( paneKey: string, reservation: PaneSpawnReservation diff --git a/src/main/ipc/pty/register-handlers.ts b/src/main/ipc/pty/register-handlers.ts index 0193f71a1f5..7794a8ef60a 100644 --- a/src/main/ipc/pty/register-handlers.ts +++ b/src/main/ipc/pty/register-handlers.ts @@ -13,7 +13,12 @@ import { localProvider } from './provider/registry' import { finishPtyShutdown } from './provider/liveness' import type { GetSelectedCodexHomePath, PrepareClaudeAuth } from './host-env/types' import { installPtyInspectIpcHandlers } from './ipc/inspect' -import { installPtyKillIpcHandler } from './ipc/renderer-kill' +import { + installPtyKillIpcHandler, + stopReplacedPanePty, + type PtyKillIpcDeps +} from './ipc/renderer-kill' +import { markReplacedPtyStop } from './delivery/exit' import { installPtyWriteIpcHandlers } from './ipc/write' import { installPtySpawnIpcHandler } from './ipc/spawn' import { installPtyRuntimeController } from './runtime/controller' @@ -239,6 +244,14 @@ export function registerPtyHandlers( }) installPtySnapshotIpcHandlers({ runtime, pendingData: session.pendingData }) + const killDeps: PtyKillIpcDeps = { + store, + runtime, + getLocalPtyProviderStartupPromise, + shutdownProviderAndDetectExit: session.shutdownProviderAndDetectExit, + rememberSyntheticKillExit: session.rememberSyntheticKillExit, + sendPtyExitToRenderer: session.sendPtyExitToRenderer + } installPtySpawnIpcHandler({ runtime, store, @@ -260,17 +273,12 @@ export function registerPtyHandlers( session.transitionSpawnHiddenRendererPtyDeliveryState, trustedTerminalHandleEnv: session.trustedTerminalHandleEnv, sendPtySpawnedToRenderer: session.sendPtySpawnedToRenderer, - syncPtyBackgroundedDelivery: session.syncPtyBackgroundedDelivery + syncPtyBackgroundedDelivery: session.syncPtyBackgroundedDelivery, + stopReplacedPty: (id) => + stopReplacedPanePty(killDeps, id, (ptyId) => markReplacedPtyStop(session, ptyId)) }) installPtyWriteIpcHandlers({ mainWindow, runtime }) installPtyResizeVisibilityIpc(session) installPtyInspectIpcHandlers({ getLocalPtyProviderStartupPromise }) - installPtyKillIpcHandler({ - store, - runtime, - getLocalPtyProviderStartupPromise, - shutdownProviderAndDetectExit: session.shutdownProviderAndDetectExit, - rememberSyntheticKillExit: session.rememberSyntheticKillExit, - sendPtyExitToRenderer: session.sendPtyExitToRenderer - }) + installPtyKillIpcHandler(killDeps) } diff --git a/src/main/ipc/pty/session.ts b/src/main/ipc/pty/session.ts index a7054e54d3b..30c6735cde2 100644 --- a/src/main/ipc/pty/session.ts +++ b/src/main/ipc/pty/session.ts @@ -10,6 +10,7 @@ import type { PtyRendererDeliveryStateReport } from '../../../shared/pty-renderer-delivery-health' import type { PtyRendererDeliveryDebugSnapshot } from './delivery/debug' +import type { ReplacedPtyStop } from './delivery/exit' import { PtyProducerFlowController } from '../pty-producer-flow-control' import { PtyPendingDataDrainQueue, type PendingPtyData } from '../pty-pending-data-drain-queue' import type { SshPtyOutputIntake } from '../ssh-pty-output-intake' @@ -101,6 +102,7 @@ export type PtyIpcSession = { { cleanupTimer: NodeJS.Timeout; incarnationId: string | undefined } > reversibleStopOwnersByPtyId: Map + replacedPtyStopsById: Map retiredRejectedPtyIds: Map pendingSerializeRequests: Map< string, @@ -230,6 +232,7 @@ export function createPtyIpcSession(args: { backgroundedDeliverySyncByPty: new Map(), syntheticKillExitPtyIds: new Map(), reversibleStopOwnersByPtyId: new Map(), + replacedPtyStopsById: new Map(), retiredRejectedPtyIds: new Map(), pendingSerializeRequests: new Map(), canSendPtyDataToRenderer: unsetSessionFn, diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index 7e5809899c4..ca273aee07d 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -46,6 +46,8 @@ export type PtyApi = { // Why: main sync-flushes the (worktreeId,tabId,leafId→ptyId) binding before pty:spawn returns to close a SIGKILL race (INVESTIGATION.md). tabId?: string leafId?: string + // Why: a pane with a live owner is otherwise reattached; a restart names the PTY main must stop first. + replacesPtyId?: string // Why: main fires `agent_started` only on spawn success, so launch metadata rides this field (telemetry-plan.md §Agent launch semantics). telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }) => Promise<{ @@ -213,6 +215,8 @@ export type PtyApi = { incarnationId?: string /** Set only when the owning relay disowned this id; never a claim that the process died. */ ptySourceDisowned?: true + /** Main stopped this PTY so a new process could take its pane; the pane is not dying. */ + replacedByRestart?: true }) => void ) => () => void onSpawned: (callback: (data: { id: string }) => void) => () => void diff --git a/src/preload/api/pty-bridge-session-control.ts b/src/preload/api/pty-bridge-session-control.ts index 009b9048d72..9854d2d86fd 100644 --- a/src/preload/api/pty-bridge-session-control.ts +++ b/src/preload/api/pty-bridge-session-control.ts @@ -44,6 +44,8 @@ export const ptySessionControlApi = { // Why: closes the SIGKILL race (INVESTIGATION.md) — main sync-flushes the (worktreeId, tabId, leafId → ptyId) binding before pty:spawn returns. tabId?: string leafId?: string + // Why: a pane with a live owner is otherwise reattached; a restart names the PTY main must stop first. + replacesPtyId?: string // Why: loose typing on purpose — renderer owns launch metadata, main owns whether the launch happened and validates (telemetry-plan.md §Agent launch semantics). telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }): Promise<{ diff --git a/src/preload/api/pty-bridge-stream-and-serialization.ts b/src/preload/api/pty-bridge-stream-and-serialization.ts index 65612b7e3d7..49e2189fd53 100644 --- a/src/preload/api/pty-bridge-stream-and-serialization.ts +++ b/src/preload/api/pty-bridge-stream-and-serialization.ts @@ -74,6 +74,8 @@ export const ptyStreamAndSerializationApi = { incarnationId?: string /** Set only when the owning relay disowned this id; never a claim that the process died. */ ptySourceDisowned?: true + /** Main stopped this PTY so a new process could take its pane; the pane is not dying. */ + replacedByRestart?: true }) => void ): (() => void) => { const listener = ( @@ -84,6 +86,7 @@ export const ptyStreamAndSerializationApi = { preserveRendererBinding?: boolean incarnationId?: string ptySourceDisowned?: true + replacedByRestart?: true } ) => callback(data) ipcRenderer.on('pty:exit', listener) diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts index 70682d481fb..8aea1440364 100644 --- a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts @@ -3,6 +3,15 @@ import { useAppStore } from '@/store' import { registerRuntimeTerminalTab } from '@/runtime/sync-runtime-graph' import { awaitsCodexRestartAnswer, blocksCodexPaneInput } from '../codex-restart-notice-state' import { ptyDataHandlers } from './pty-dispatcher' +import { deliverPtyExitToHandlers } from './pty-exit-delivery' +import { + bufferPreHandlerPtyData, + clearPreHandlerPtyState, + drainPreHandlerPtyData, + hasPreHandlerPtyExit, + isPreHandlerPtyStateDiscarded +} from './pty-pre-handler-buffer' +import { parkedWatchersByTabId } from './terminal-parked-watcher-registry' import { sweepUnclaimedCodexPaneRestarts } from './codex-detached-pane-restart' import { hasAddedPendingCodexPaneRestart, @@ -113,6 +122,7 @@ describe('codex detached pane restart executor', () => { worktreeId: 'wt1', tabId: 'tab-1', leafId: LEAF_ID, + replacesPtyId: OLD_PTY, initiallyHidden: true }) ) @@ -125,7 +135,8 @@ describe('codex detached pane restart executor', () => { ORCA_WORKSPACE_ID: 'wt1' }) ) - expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + // Main stops the replaced PTY inside the spawn; a renderer kill would race its adoption. + expect(window.api.pty.kill).not.toHaveBeenCalled() const state = useAppStore.getState() expect(state.ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) @@ -138,6 +149,47 @@ describe('codex detached pane restart executor', () => { expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId[NEW_PTY])).toBe(false) }) + it('adopts the replacement when the replaced PTY exit clears the tab binding mid-spawn', async () => { + seedQueuedRestart() + // Main stops the replaced PTY before replying, so its exit reaches the renderer first; a + // background-launch exit sidecar answers that exit by clearing the tab's binding. + vi.mocked(window.api.pty.spawn).mockImplementation(async () => { + useAppStore.getState().clearTabPtyId('tab-1', OLD_PTY) + return { id: NEW_PTY } + }) + + await sweepUnclaimedCodexPaneRestarts() + + const state = useAppStore.getState() + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(state.ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) + expect(state.terminalLayoutsByTabId['tab-1']?.ptyIdsByLeafId).toEqual({ [LEAF_ID]: NEW_PTY }) + expect(blocksCodexPaneInput(state.codexRestartNoticeByPtyId[NEW_PTY])).toBe(false) + }) + + it('keeps the replaced PTY exit away from a tab revealed mid-restart', async () => { + // Earlier restarts in this file tombstone the same id; start from a live PTY's state. + clearPreHandlerPtyState(OLD_PTY) + seedQueuedRestart() + let revealView: { exitReplayed: boolean; sessionAdmitted: boolean } | null = null + vi.mocked(window.api.pty.spawn).mockImplementation(async () => { + // Main stops the replaced PTY before replying, labeled as a replacement; no pane owns it yet. + deliverPtyExitToHandlers({ ptyId: OLD_PTY, code: 0, replacedByRestart: true, sidecars: [] }) + // What a pane revealed now consults before reconnecting under the layout's old id. + revealView = { + exitReplayed: hasPreHandlerPtyExit(OLD_PTY), + sessionAdmitted: !isPreHandlerPtyStateDiscarded(OLD_PTY) + } + return { id: NEW_PTY } + }) + + await sweepUnclaimedCodexPaneRestarts() + + // Neither: the reveal reconnects by pane identity, which main answers with the replacement. + expect(revealView).toEqual({ exitReplayed: false, sessionAdmitted: false }) + expect(useAppStore.getState().ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) + }) + it('executes via the store subscription without a lifecycle timeout', async () => { const uninstall = installCodexDetachedPaneRestartExecutor() try { @@ -145,7 +197,10 @@ describe('codex detached pane restart executor', () => { expect(window.api.pty.spawn).not.toHaveBeenCalled() await vi.waitFor(() => expect(window.api.pty.spawn).toHaveBeenCalledTimes(1)) - await vi.waitFor(() => expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY)) + await vi.waitFor(() => + expect(useAppStore.getState().ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) + ) + expect(window.api.pty.kill).not.toHaveBeenCalled() expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({}) } finally { @@ -254,7 +309,9 @@ describe('codex detached pane restart executor', () => { ptyIdsByLeafId: { [LEAF_ID]: NEW_PTY } }) ) - expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + expect(window.api.pty.spawn).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ replacesPtyId: OLD_PTY }) + ) }) it('rebinds only the codex leaf of a split and keeps the sibling', async () => { @@ -285,8 +342,11 @@ describe('codex detached pane restart executor', () => { [LEAF_ID]: NEW_PTY, [SIBLING_LEAF]: 'wt1@@sibling' }) - // Split-pane safety: only the codex pane's PTY dies. - expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(OLD_PTY) + // Split-pane safety: only the codex pane's PTY is replaced. + expect(window.api.pty.spawn).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ replacesPtyId: OLD_PTY }) + ) + expect(window.api.pty.kill).not.toHaveBeenCalled() expect(useAppStore.getState().ptyIdsByTabId['tab-1']).toEqual([NEW_PTY, 'wt1@@sibling']) }) @@ -311,12 +371,10 @@ describe('codex detached pane restart executor', () => { } }) - it('reaps a detached spawn and requeues when a pane mounts during the spawn', async () => { + it('keeps the replacement when a pane mounts during the spawn, since main already stopped the old PTY', async () => { seedQueuedRestart() const pendingSpawn = deferred<{ id: string }>() - const pendingKill = deferred() vi.mocked(window.api.pty.spawn).mockReturnValue(pendingSpawn.promise) - vi.mocked(window.api.pty.kill).mockReturnValue(pendingKill.promise) const restart = sweepUnclaimedCodexPaneRestarts() await vi.waitFor(() => expect(window.api.pty.spawn).toHaveBeenCalledTimes(1)) @@ -330,12 +388,13 @@ describe('codex detached pane restart executor', () => { }) try { pendingSpawn.resolve({ id: NEW_PTY }) - await vi.waitFor(() => expect(window.api.pty.kill).toHaveBeenCalledExactlyOnceWith(NEW_PTY)) - - expect(useAppStore.getState().ptyIdsByTabId['tab-1']).toEqual([OLD_PTY]) - expect(useAppStore.getState().pendingCodexPaneRestartIds).toEqual({ [OLD_PTY]: true }) await restart - pendingKill.resolve() + + expect(window.api.pty.kill).not.toHaveBeenCalled() + const state = useAppStore.getState() + expect(state.ptyIdsByTabId['tab-1']).toEqual([NEW_PTY]) + expect(state.terminalLayoutsByTabId['tab-1']?.ptyIdsByLeafId?.[LEAF_ID]).toBe(NEW_PTY) + expect(state.pendingCodexPaneRestartIds).toEqual({}) } finally { unregister() } @@ -404,6 +463,38 @@ describe('codex detached pane restart executor', () => { expect(awaitsCodexRestartAnswer(state.codexRestartNoticeByPtyId[OLD_PTY])).toBe(true) }) + it('keeps parked watchers and buffered output for the old PTY when main cannot stop it', async () => { + clearPreHandlerPtyState(OLD_PTY) + seedQueuedRestart() + bufferPreHandlerPtyData(OLD_PTY, 'still running') + const disposeWatcher = vi.fn() + parkedWatchersByTabId.set('tab-1', { + worktreeId: 'wt1', + tabPtyId: OLD_PTY, + paneIdByPtyId: new Map([[OLD_PTY, 1]]), + disposersByPtyId: new Map([[OLD_PTY, disposeWatcher]]) + }) + vi.mocked(window.api.pty.spawn).mockRejectedValue(new Error('daemon unreachable')) + + try { + await sweepUnclaimedCodexPaneRestarts() + + // No exit was sent, so the still-running Codex keeps every renderer observer it had. + expect(disposeWatcher).not.toHaveBeenCalled() + expect(parkedWatchersByTabId.get('tab-1')?.disposersByPtyId.has(OLD_PTY)).toBe(true) + expect(isPreHandlerPtyStateDiscarded(OLD_PTY)).toBe(false) + const replayed: string[] = [] + drainPreHandlerPtyData(OLD_PTY, (data) => replayed.push(data)) + expect(replayed).toEqual(['still running']) + expect( + awaitsCodexRestartAnswer(useAppStore.getState().codexRestartNoticeByPtyId[OLD_PTY]) + ).toBe(true) + } finally { + parkedWatchersByTabId.delete('tab-1') + clearPreHandlerPtyState(OLD_PTY) + } + }) + it('kills now and defers the Codex respawn to mount when the layout leaf is unknown', async () => { seedQueuedRestart({ leafId: null }) diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts index 6665bbaa02a..1578a09fc24 100644 --- a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts @@ -207,6 +207,8 @@ async function executeDetachedCodexPaneRestart( } // Hidden replacements converge on mount; provider sizing must not delay ownership transfer. + // Main stops `ptyId` first and labels its exit as a replacement, so parked watchers and the + // pre-attach buffer leave the pane alone; a failed stop sends no exit and changes nothing here. const spawned = await window.api.pty.spawn({ cols: 80, rows: 24, @@ -219,22 +221,20 @@ async function executeDetachedCodexPaneRestart( worktreeId, tabId: tab.id, leafId, + replacesPtyId: ptyId, ...(tab.shellOverride ? { shellOverride: tab.shellOverride } : {}), ...(projectRuntime ? { projectRuntime } : {}), initiallyHidden: true }) + // Why adopt rather than stand down: main stopped `ptyId` before replying, so the replacement is + // the pane's only process — even if that exit already cleared the old binding or a pane mounted. const store = useAppStore.getState() - if (!isLocatedCodexPaneCurrent(store, located, ptyId)) { + if (!isLocatedCodexPaneLeafAdoptable(store, located, ptyId, spawned.id)) { reopenCurrentCodexRestartPrompt(located, ptyId) reapUnboundCodexPty(spawned.id, 'stale detached spawn') return } - if (hasRegisteredRuntimeTerminalTab(tab.id, worktreeId) || ptyDataHandlers.has(ptyId)) { - store.queueCodexPaneRestarts([ptyId]) - reapUnboundCodexPty(spawned.id, 'mounted-owner handoff spawn') - return - } store.updateTabPtyId(tab.id, spawned.id, ptyId) if (!useAppStore.getState().ptyIdsByTabId[tab.id]?.includes(spawned.id)) { // Why: the tab was retired while the spawn was in flight; without a binding @@ -248,8 +248,7 @@ async function executeDetachedCodexPaneRestart( // new PTY; the restart it recorded is now done, so the block must lift. store.clearCodexRestartNotice(spawned.id) store.clearCodexRestartNotice(ptyId) - - killReplacedCodexPanePty(ptyId) + releaseReplacedCodexPanePty(ptyId) } function isLocatedCodexPaneCurrent( @@ -274,6 +273,22 @@ function isLocatedCodexPaneCurrent( ) } +function isLocatedCodexPaneLeafAdoptable( + state: AppState, + located: LocatedCodexPane, + replacedPtyId: string, + replacementPtyId: string +): boolean { + const currentTab = state.tabsByWorktree[located.worktreeId]?.find( + (candidate) => candidate.id === located.tab.id + ) + if (!currentTab || (currentTab.generation ?? 0) !== located.generation || !located.leafId) { + return false + } + const boundPtyId = state.terminalLayoutsByTabId[located.tab.id]?.ptyIdsByLeafId?.[located.leafId] + return boundPtyId === replacedPtyId || boundPtyId === replacementPtyId +} + function reopenCurrentCodexRestartPrompt(located: LocatedCodexPane, replacedPtyId: string): void { const state = useAppStore.getState() const currentTab = state.tabsByWorktree[located.worktreeId]?.find( @@ -331,12 +346,17 @@ function reapUnboundCodexPty(ptyId: string, reason: string): void { discardPreHandlerPtyState(ptyId) } -function killReplacedCodexPanePty(ptyId: string): void { - // Why the disposal: a parked tab's exit sidecar treats any exit as the pane +function releaseReplacedCodexPanePty(ptyId: string): void { + // Why the disposal: a parked tab's exit sidecar treats an unlabeled exit as the pane // dying — it would collapse the just-rebound leaf or close the whole tab. disposeParkedTerminalWatchersForPtyIds([ptyId]) for (const snapshot of unregisterPtyDataHandlers([ptyId])) { snapshot.commit() } + discardPreHandlerPtyState(ptyId) +} + +function killReplacedCodexPanePty(ptyId: string): void { + releaseReplacedCodexPanePty(ptyId) reapUnboundCodexPty(ptyId, 'replaced Codex pane PTY') } diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts index 226207d9060..ff69ba35a86 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts @@ -39,6 +39,8 @@ export async function spawnIpcPty( } = transportOptions const shouldSendLocalCwdFallback = cwdFallback === 'worktree' && !connectionId && !admittedSessionId + // Why: a reattach under an admitted session id must never stop the PTY it is reattaching. + const replacesPtyId = admittedSessionId ? null : (connectOptions.claimReplacedPtyId?.() ?? null) return window.api.pty.spawn({ cols: connectOptions.cols ?? 80, rows: connectOptions.rows ?? 24, @@ -75,10 +77,11 @@ export async function spawnIpcPty( worktreeId, ...(tabId ? { tabId } : {}), ...(leafId ? { leafId } : {}), + ...(replacesPtyId ? { replacesPtyId } : {}), ...(shellOverride ? { shellOverride } : {}), ...(projectRuntime ? { projectRuntime } : {}), ...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}), ...(terminalKittyKeyboardProtocol === true ? { terminalKittyKeyboardProtocol: true } : {}), ...(telemetry ? { telemetry } : {}) - }) as Promise + }) } diff --git a/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.test.ts b/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.test.ts new file mode 100644 index 00000000000..3f1ab4efa70 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.test.ts @@ -0,0 +1,81 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { installIpcPtyWindow, restorePtySpecWindow } from './pty-transport-test-harness' + +describe('releasePaneTransportForRestart', () => { + const originalWindow = (globalThis as { window?: typeof window }).window + + beforeEach(() => { + vi.resetModules() + installIpcPtyWindow(originalWindow, { data: () => {}, exit: () => {} }) + }) + + afterEach(() => { + restorePtySpecWindow(originalWindow) + }) + + async function attachedTransport(ptyId: string) { + const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createIpcPtyTransport({}) + transport.attach({ existingPtyId: ptyId, callbacks: {} }) + return transport + } + + it('hands the old PTY to the replacement spawn instead of killing it', async () => { + const { releasePaneTransportForRestart } = await import('./pane-restart-transport-handoff') + const transport = await attachedTransport('wt1@@old') + + const replacesPtyId = releasePaneTransportForRestart(transport) + + expect(window.api.pty.kill).not.toHaveBeenCalled() + expect(replacesPtyId).toBe('wt1@@old') + + const { createIpcPtyTransport } = await import('./pty-transport') + await createIpcPtyTransport({ command: 'codex', worktreeId: 'wt1' }).connect({ + url: '', + claimReplacedPtyId: () => 'wt1@@old', + callbacks: {} + }) + expect(window.api.pty.spawn).toHaveBeenCalledWith( + expect.objectContaining({ command: 'codex', replacesPtyId: 'wt1@@old' }) + ) + }) + + it('leaves the replaced PTY with its owner when the connect stands down before spawning', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const claimReplacedPtyId = vi.fn(() => 'wt1@@old') + + await createIpcPtyTransport({ command: 'codex', worktreeId: 'wt1' }).connect({ + url: '', + claimReplacedPtyId, + shouldContinue: () => false, + callbacks: {} + }) + + expect(window.api.pty.spawn).not.toHaveBeenCalled() + expect(claimReplacedPtyId).not.toHaveBeenCalled() + }) + + it('keeps kill-then-connect for a remote-runtime PTY whose host ignores the replace field', async () => { + const { releasePaneTransportForRestart } = await import('./pane-restart-transport-handoff') + const transport = await attachedTransport('remote:env-1@@term-1') + + expect(releasePaneTransportForRestart(transport)).toBeNull() + expect(window.api.pty.kill).toHaveBeenCalledWith('remote:env-1@@term-1') + }) + + it('never sends a replace field on a session reattach', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + vi.mocked(window.api.pty.spawn).mockResolvedValue({ id: 'wt1@@live', isReattach: true }) + + await createIpcPtyTransport({}).connect({ + url: '', + sessionId: 'wt1@@live', + claimReplacedPtyId: () => 'wt1@@old', + callbacks: {} + }) + + expect(window.api.pty.spawn).toHaveBeenCalledWith( + expect.not.objectContaining({ replacesPtyId: expect.anything() }) + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.ts b/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.ts new file mode 100644 index 00000000000..93eea063ed4 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pane-restart-transport-handoff.ts @@ -0,0 +1,22 @@ +import type { PtyTransport } from './pty-transport-types' +import { isRemoteRuntimePtyId } from './pty-connection/paired-parked-terminal-restore' + +/** + * Releases a pane's transport for a restart and returns the PTY its replacement spawn must name. + * + * Why: main adopts a pane's live owner on spawn, so a local restart must not kill the old PTY + * itself — the replacement spawn names it and main stops it before launching. Remote-runtime + * hosts do not read that field, so they keep the kill-then-connect path. + */ +export function releasePaneTransportForRestart(transport: PtyTransport | undefined): string | null { + const existingPtyId = transport?.getPtyId() + const replacesPtyId = + existingPtyId && transport?.detach && !isRemoteRuntimePtyId(existingPtyId) + ? existingPtyId + : null + if (replacesPtyId) { + transport?.detach?.({ preserveExitObserver: false }) + } + transport?.destroy?.() + return replacesPtyId +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection-hibernation-wake.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-hibernation-wake.test.ts index 340d174db9d..7aa4de64a1d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-hibernation-wake.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-hibernation-wake.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { flushAsyncTicks, createDeferred } from './pty-connection-test-async' import { sendTerminalInputThroughPane } from './pty-connection-test-dom' import { + LEAF_2, leafIdForPane, createMockTransport, createPane, @@ -11,7 +12,7 @@ import { import { buildPaneConnectionDeps } from './pty-connection-test-deps' import { createInitialStoreState } from './pty-connection-test-store-fixtures' import type { StoreState } from './pty-connection-test-store-state' -import type { MockTransport } from './pty-connection-test-pane-fixtures' +import type { ConnectCallbacks, MockTransport } from './pty-connection-test-pane-fixtures' import { installTerminalTestGlobals, restoreTerminalTestGlobals @@ -271,6 +272,120 @@ describe('connectPanePty', () => { expect(transport.connect.mock.calls.length).toBe(connectCallsAfterWake) }) + it('names the replaced PTY on the restart spawn only, never on a later wake of the same pane', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-restarted') + transportFactoryQueue.push(transport) + const deps = createDeps({ + tabId: 'tab-restart-wake', + startup: { command: 'claude', launchAgent: 'claude' }, + replacesPtyId: 'pty-replaced', + consumeSuppressedPtyExit: vi.fn(() => true), + isVisibleRef: { current: false } + }) + const pane = createPane(2) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixtures implement the pane, manager and deps members connectPanePty reads. + const args = [pane, createManager(1), deps] as unknown as Parameters + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the pane binding exposes the wake hook this test drives. + const binding = connectPanePty(...args) as unknown as { + wakeHibernatedAgentIfArmed: (claimedProviderSessions?: Set) => string | null + dispose: () => void + } + await flushAsyncTicks() + + expect(transport.connect).toHaveBeenCalledTimes(1) + const restartConnect: { claimReplacedPtyId?: () => string | null } | undefined = + transport.connect.mock.calls[0]?.[0] + // The IPC transport takes the id as it sends the spawn; this mock transport does it here. + expect(restartConnect?.claimReplacedPtyId?.()).toBe('pty-replaced') + // Why: transport options outlive the first spawn, so the field must not ride them. + expect(createdTransportOptions[0]).not.toHaveProperty('replacesPtyId') + + const paneKey = `tab-restart-wake:${leafIdForPane(2)}` + mockStoreState.sleepingAgentSessionsByPaneKey[paneKey] = { + paneKey, + tabId: 'tab-restart-wake', + worktreeId: 'wt-1', + agent: 'claude', + providerSession: { key: 'session_id', id: 'sess-restart-wake' }, + prompt: 'test prompt', + state: 'done', + capturedAt: 1, + updatedAt: 1, + origin: 'worktree-sleep' + } + mockStoreState.suppressedPtyExitIds['pty-restarted'] = true + const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined + onPtyExit?.('pty-restarted') + await flushAsyncTicks() + expect(binding.wakeHibernatedAgentIfArmed(new Set())).not.toBeNull() + await flushAsyncTicks() + + expect(transport.connect).toHaveBeenCalledTimes(2) + expect(transport.connect.mock.calls[1]?.[0]).not.toHaveProperty('claimReplacedPtyId') + binding.dispose() + await flushAsyncTicks() + expect(window.api.pty.kill).not.toHaveBeenCalledWith('pty-replaced') + }) + + it('keeps an established split pane mounted when main labels its exit as a restart replacement', async () => { + const { connectPanePty } = await import('./pty-connection') + const { deliverPtyExitToHandlers } = await import('./pty-exit-delivery') + let onData: ((data: string) => void) | undefined + const transport = createMockTransport('pty-pane-2') + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + onData = callbacks.onData + return 'pty-pane-2' + }) + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixtures implement the pane, manager and deps members connectPanePty reads. + const args = [createPane(2), manager, deps] as unknown as Parameters + connectPanePty(...args) + const onPtyExit = createdTransportOptions[0]?.onPtyExit + expect(onPtyExit).toBeTypeOf('function') + await flushAsyncTicks() + onData?.('codex prompt') + + deliverPtyExitToHandlers({ + ptyId: 'pty-pane-2', + code: 0, + replacedByRestart: true, + primary: (code) => { + if (typeof onPtyExit === 'function') { + onPtyExit('pty-pane-2', code) + } + }, + sidecars: [] + }) + + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.clearExitedPanePtyLayoutBinding).not.toHaveBeenCalled() + }) + + it('stops the replaced PTY itself when the pane is disposed before a spawn carried the stop', async () => { + const { connectPanePty } = await import('./pty-connection') + transportFactoryQueue.push(createMockTransport('pty-restarted')) + const deps = createDeps({ + tabId: 'tab-restart-closed', + startup: { command: 'codex', launchAgent: 'codex' }, + replacesPtyId: 'pty-replaced' + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixtures implement the pane, manager and deps members connectPanePty reads. + const args = [createPane(2), createManager(1), deps] as unknown as Parameters< + typeof connectPanePty + > + // Closing the tab (or parking it) disposes the pane before its deferred connect runs. + connectPanePty(...args).dispose() + await flushAsyncTicks() + + expect(window.api.pty.kill).toHaveBeenCalledWith('pty-replaced') + }) + it('latches a navigation-free wake that lands before the hibernation kill arms the pane', async () => { // Race (#7906): the edge-triggered wake can land after the sleeping record but before the kill sets hibernatedWakePtyId; without a latch it'd be dropped, leaving a frozen terminal. const { connectPanePty } = await import('./pty-connection') diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index 27a24c73621..27f377f30c4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -66,6 +66,8 @@ export type PtyConnectionDeps = { /** Releases a deferred split's detach fence when its initial spawn yields no PTY. */ onDeferredCwdSpawnFailed?: () => void startup?: PtyPaneStartup + /** The pane's previous PTY; main stops it before this connection's first fresh spawn. */ + replacesPtyId?: string restoredLeafId?: string | null restoredPtyIdByLeafId?: Record /** Park intent sampled at render time, before the host disposes the tab's diff --git a/src/renderer/src/components/terminal-pane/pty-connection/connect-pane-pty.ts b/src/renderer/src/components/terminal-pane/pty-connection/connect-pane-pty.ts index 02e613dc1f1..663bb3b345b 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/connect-pane-pty.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/connect-pane-pty.ts @@ -149,6 +149,15 @@ export function connectPanePty( // mutation does not propagate back. session.paneStartup = session.deps.startup ?? null session.deps.startup = undefined + // Why the session holds it until a spawn request carries it: the pane already dropped every other + // reference to this PTY, so the stop is owed until main takes it or dispose kills it. + session.pendingReplacedPtyId = session.deps.replacesPtyId ?? null + session.deps.replacesPtyId = undefined + session.claimPendingReplacedPtyId = (): string | null => { + const ptyId: string | null = session.pendingReplacedPtyId + session.pendingReplacedPtyId = null + return ptyId + } // Why: paneKey crosses PTY env, hook IPC, retained rows, and reload/replay. // Use the stable layout leaf UUID, not the renderer-local numeric pane id. diff --git a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts index 66e95d98556..29bc438a01c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts @@ -111,6 +111,9 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { ...(coldRestoreOverride ? { launchToken: coldRestoreOverride.launchToken } : {}), ...(coldRestoreOverride ? { launchAgent: coldRestoreOverride.agent } : {}), ...(session.shouldDeclareHiddenAtSpawn() ? { initiallyHidden: true } : {}), + ...(session.pendingReplacedPtyId + ? { claimReplacedPtyId: session.claimPendingReplacedPtyId } + : {}), shouldContinue: () => !session.disposed && (findTerminalTabForPane(useAppStore.getState(), session.deps.worktreeId, session.deps.tabId) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts b/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts index 669d46300b6..cdf4b869b12 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts @@ -20,6 +20,7 @@ import { import type { PtyPaneStartup } from '../pty-connection-types' import type { ConnectPanePtySession } from './connect-pane-pty-session' +import { isPtyExitReplacedByRestart } from '../pty-exit-delivery' /** PTY exit handling, hibernated-pane wake targets, and post-exit focus transfer. */ export function installPtyExitHibernate(session: ConnectPanePtySession): void { @@ -219,7 +220,10 @@ export function installPtyExitHibernate(session: ConnectPanePtySession): void { // Why: the negotiating application died with its PTY; any replacement // session starts with kitty keyboard flags at zero. session.kittyKeyboardModes.reset() - const isSuppressedExit = session.deps.consumeSuppressedPtyExit(ptyId) || preserveRendererBinding + const isSuppressedExit = + session.deps.consumeSuppressedPtyExit(ptyId) || + preserveRendererBinding || + isPtyExitReplacedByRestart(ptyId) if (!isSuppressedExit && !isUnverifiedExit) { session.clearExitedPanePtyLayoutBinding(ptyId) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/session-reconcile-dispose.ts b/src/renderer/src/components/terminal-pane/pty-connection/session-reconcile-dispose.ts index 959f7edf43f..af21b11f25e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/session-reconcile-dispose.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/session-reconcile-dispose.ts @@ -177,6 +177,13 @@ export function installSessionReconcileDispose(session: ConnectPanePtySession): dispose() { session.disposed = true session.startupTiming?.finish('disposed') + const unsentReplacedPtyId: string | null = session.claimPendingReplacedPtyId() + if (unsentReplacedPtyId) { + // Why: no spawn will carry this stop now, and the pane no longer references the PTY. + void Promise.resolve() + .then(() => window.api.pty.kill(unsentReplacedPtyId)) + .catch((err: unknown) => console.warn('[terminal] failed to stop a replaced PTY:', err)) + } // A successor can claim the numeric pane slot before this retired // binding's disposal callback runs; do not clear its pane-scoped error. const currentPaneTransport = session.deps.paneTransportsRef.current.get(session.pane.id) diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index ccb38ae2f55..5547b94e837 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -206,6 +206,7 @@ function attachPtySecondaryPushListeners(unsubscribes: (() => void)[]): void { // Why forwarded: pty ids are reused, so a buffered exit needs the lifetime it describes to // tell "this pane's shell died" from "the id's previous owner died" (#16970). ...(payload.incarnationId ? { incarnationId: payload.incarnationId } : {}), + ...(payload.replacedByRestart === true ? { replacedByRestart: true } : {}), ...(primary ? { primary } : {}), sidecars: sidecars ? Array.from(sidecars) : [] }) diff --git a/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts b/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts index a01e09c2d8f..a072c90e82a 100644 --- a/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts +++ b/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts @@ -1,7 +1,8 @@ import { bufferPreHandlerPtyExit, clearPreHandlerPtyState, - consumePreHandlerPtyState + consumePreHandlerPtyState, + discardPreHandlerPtyState } from './pty-pre-handler-buffer' type PtyExitDelivery = { @@ -9,12 +10,36 @@ type PtyExitDelivery = { code: number /** Which lifetime of `ptyId` died. Absent when the execution host predates the field. */ incarnationId?: string + /** Main stopped this PTY so a new process could take its pane. */ + replacedByRestart?: boolean primary?: (code: number) => void sidecars: readonly ((code: number, context: { hadPrimary: boolean }) => void)[] } +// Why scoped to one delivery: consumers read it synchronously while main's label is in hand, so +// nothing is left behind to expire or to mislabel a later exit of the same id. +const replacedExitsInDelivery = new Set() + +/** Whether the exit being delivered hands the pane to a replacement rather than ending it. */ +export function isPtyExitReplacedByRestart(ptyId: string): boolean { + return replacedExitsInDelivery.has(ptyId) +} + /** Delivers one exit to its primary owner and every observational sidecar. */ export function deliverPtyExitToHandlers(delivery: PtyExitDelivery): void { + if (!delivery.replacedByRestart) { + deliverClassifiedPtyExit(delivery) + return + } + replacedExitsInDelivery.add(delivery.ptyId) + try { + deliverClassifiedPtyExit(delivery) + } finally { + replacedExitsInDelivery.delete(delivery.ptyId) + } +} + +function deliverClassifiedPtyExit(delivery: PtyExitDelivery): void { let firstError: unknown let hasError = false try { @@ -27,6 +52,10 @@ export function deliverPtyExitToHandlers(delivery: PtyExitDelivery): void { // must not become a new pre-handler event for a future mount. consumePreHandlerPtyState(delivery.ptyId) } + } else if (delivery.replacedByRestart) { + // Why: a buffered exit would read as this pane's death to a mount mid-restart; discarding + // also stops that mount reattaching the dead id, so it spawns by pane and gets the replacement. + discardPreHandlerPtyState(delivery.ptyId) } else { bufferPreHandlerPtyExit(delivery.ptyId, delivery.code, delivery.incarnationId) } 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 246657f20c1..c7753abbd3e 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -165,6 +165,9 @@ export type PtyTransport = { launchToken?: string launchAgent?: TuiAgent startupCommandDelivery?: StartupCommandDelivery + /** Taken only as the spawn request is sent; main stops the returned PTY before resolving the + * pane's owner. Never taken on a session reattach, so the caller still holds it. */ + claimReplacedPtyId?: () => string | null /** Reject a stale restored identity before this transport can publish global PTY handlers. */ admitPtyId?: (ptyId: string) => boolean /** Reject a stale pane after any pre-spawn test gate but before creating a PTY. */ diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts b/src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts index cf63f88be9f..158d03e8bb0 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts @@ -5,6 +5,7 @@ import { useAppStore } from '@/store' import { closeTerminalTab } from '../terminal/terminal-tab-actions' import { startParkedTerminalByteWatcher } from './parked-terminal-byte-watcher' import { subscribeToPtyExit } from './pty-dispatcher' +import { isPtyExitReplacedByRestart } from './pty-exit-delivery' import { consumePreHandlerPtyState, discardPreHandlerPtyState, @@ -60,6 +61,12 @@ export function startParkedPtyWatcher(args: { return } const handlePtyExit = (code: number, { hadPrimary }: { hadPrimary: boolean }): void => { + if (isPtyExitReplacedByRestart(ptyId)) { + // Why: the pane lives on under its replacement PTY; only this watcher's subscription ends. + entry.disposersByPtyId.get(ptyId)?.() + entry.disposersByPtyId.delete(ptyId) + return + } useAppStore.getState().clearRuntimePaneTitle(tab.id, pane.paneId) // A negative code is a synthetic loss sentinel, not a death certificate. // Preserve the tab so host shutdown/reconnect cannot be mistaken for an diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts index 3e9ea0a25b8..0349c6aa061 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts @@ -63,6 +63,7 @@ import { markCommittedPtyShutdowns } from './pty-shutdown-exit-deferral' import { startParkedPtyWatcher } from './terminal-parked-pty-watcher' +import { deliverPtyExitToHandlers } from './pty-exit-delivery' import type { ParkedTabWatcherEntry } from './terminal-parked-watcher-registry' function startSplitWatchers(): ParkedTabWatcherEntry { @@ -134,6 +135,25 @@ describe('sleep-preserved parked exits (sole-owner sidecar)', () => { expect(consumeCommittedPtyShutdownExit(PTY_ID, null)).toBe(false) }) + it('keeps the tab on an exit main labeled as a restart replacement', () => { + const entry = startSplitWatchers() + exitCallbacksByPtyId.get(SECOND_PTY_ID)?.(0, { hadPrimary: false }) + discardPreHandlerPtyState.mockClear() + + // The last parked pane's PTY is replaced; an ordinary exit here would close the tab. + deliverPtyExitToHandlers({ + ptyId: PTY_ID, + code: 0, + replacedByRestart: true, + sidecars: [exitCallbacksByPtyId.get(PTY_ID)!] + }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(mockStoreState.clearRuntimePaneTitle).not.toHaveBeenCalledWith(TAB_ID, 1) + expect(startedWatcherDisposers[0]).toHaveBeenCalled() + expect(entry.disposersByPtyId.size).toBe(0) + }) + it('leaves the committed marker to the primary when one handled the exit', () => { startSplitWatchers() markCommittedPtyShutdowns([PTY_ID]) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-process-exit-actions.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-process-exit-actions.ts index 391a796776e..0d3a8c5f382 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-process-exit-actions.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-process-exit-actions.ts @@ -4,6 +4,7 @@ import { CODEX_ACCOUNT_RESTART_STARTUP } from '@/lib/codex-session-restart' import { makePaneKey } from '../../../../shared/stable-pane-id' import { connectPanePty } from './pty-connection' import { bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' +import { releasePaneTransportForRestart } from './pane-restart-transport-handoff' import { clearPaneTerminalError } from './terminal-error-accumulation' import { resolveTerminalProcessExitRestartStartup } from './terminal-process-exit-restart' import type { PaneProcessExit, PtyConnectionDeps } from './pty-connection-types' @@ -78,7 +79,7 @@ export function useTerminalPaneProcessExitActions(controller: TerminalPaneCloseC panePtyBinding?.dispose() panePtyBindingsRef.current.delete(paneId) syncPanePtyLayoutBinding(paneId, null) - transport?.destroy?.() + const replacesPtyId = releasePaneTransportForRestart(transport) paneTransportsRef.current.delete(paneId) setCacheTimerStartedAt(makePaneKey(tabId, pane.leafId), null) setTerminalError(null) @@ -88,6 +89,7 @@ export function useTerminalPaneProcessExitActions(controller: TerminalPaneCloseC worktreeId, cwd, startup: restartStartup, + ...(replacesPtyId ? { replacesPtyId } : {}), mountFollowsTerminalPark: false, paneTransportsRef, paneMode2031Ref,