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,