From 8a6cc6c13846f7d38fbaf42a545e07ae06f4da17 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:10:33 -0400 Subject: [PATCH] fix(terminal): bring each pane back after a terminal service restart when it is shown (#28880) * fix(terminal): bring each pane back after a terminal service restart when it is shown After Settings -> "Restart terminal service", every pane that was open kept its dead process: typing did nothing and agents never resumed until Orca relaunched. A restart-replaced exit now arms the pane: the visible pane spawns its replacement at once, any other pane when it is next shown, through the same fresh spawn and cold-restore agent resume the hibernation wake uses, so old output stays above the new prompt. The exit keeps the agent status the resume reads, and clears the dead binding so the replacement binds as a first spawn. Local spawns wait for an in-flight restart so they reach the new daemon. Claude-Session: p2-headless * fix(terminal): resume from the live recovery record; skip a pane another flow rebound A restart exit is a process death to main's agent-status store, which clears the pane's row before the window sees the exit, so keeping the window's copy changed nothing. The resume reads the live recovery record every agent turn already writes, as a relaunch after a crash does. The replacement also stands down when the pane's stored binding already names another process. Claude-Session: p2-headless * fix(terminal): send every restart exit down one path A finished-turn agent pane took the hibernation wake, which gives up when its record changes before the pane is shown. Every restart exit now arms the replacement directly. Claude-Session: p2-headless --- src/main/daemon/daemon-restart-state.test.ts | 34 +++ src/main/daemon/daemon-restart-state.ts | 8 + src/main/ipc/pty/register-handlers.ts | 8 +- ...pty-connection-restart-replacement.test.ts | 275 ++++++++++++++++++ .../pty-connection/pty-exit-hibernate.ts | 14 +- .../restart-replacement-on-show.ts | 38 +++ .../session-reconcile-dispose.ts | 2 + .../workspace-layout-oracle-known-on-main.ts | 7 - 8 files changed, 374 insertions(+), 12 deletions(-) create mode 100644 src/main/daemon/daemon-restart-state.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-restart-replacement.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-connection/restart-replacement-on-show.ts diff --git a/src/main/daemon/daemon-restart-state.test.ts b/src/main/daemon/daemon-restart-state.test.ts new file mode 100644 index 00000000000..f9280702050 --- /dev/null +++ b/src/main/daemon/daemon-restart-state.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { runCoalescedDaemonRestart, settledDaemonRestart } from './daemon-restart-state' + +describe('settledDaemonRestart', () => { + it('is undefined with no restart running', () => { + expect(settledDaemonRestart()).toBeUndefined() + }) + + it('holds until the running restart settles, whether it succeeds or fails', async () => { + for (const outcome of ['resolve', 'reject'] as const) { + let finish!: () => void + const restart = runCoalescedDaemonRestart( + () => + new Promise((resolve, reject) => { + finish = () => + outcome === 'resolve' ? resolve({ killedCount: 1 }) : reject(new Error('x')) + }) + ) + restart.catch(() => {}) + let settled = false + void settledDaemonRestart()?.then(() => { + settled = true + }) + await Promise.resolve() + expect(settled).toBe(false) + + finish() + await restart.catch(() => {}) + await Promise.resolve() + expect(settled).toBe(true) + expect(settledDaemonRestart()).toBeUndefined() + } + }) +}) diff --git a/src/main/daemon/daemon-restart-state.ts b/src/main/daemon/daemon-restart-state.ts index e37fad69709..70918375898 100644 --- a/src/main/daemon/daemon-restart-state.ts +++ b/src/main/daemon/daemon-restart-state.ts @@ -9,6 +9,14 @@ export function isDaemonRestartInFlight(): boolean { return restartInFlight !== null } +/** Settles once an in-flight restart has swapped in its provider (or failed); undefined when none. */ +export function settledDaemonRestart(): Promise | undefined { + return restartInFlight?.then( + () => undefined, + () => undefined + ) +} + export function runCoalescedDaemonRestart( run: () => Promise ): Promise { diff --git a/src/main/ipc/pty/register-handlers.ts b/src/main/ipc/pty/register-handlers.ts index 4010711e28b..b40aececc21 100644 --- a/src/main/ipc/pty/register-handlers.ts +++ b/src/main/ipc/pty/register-handlers.ts @@ -1,3 +1,4 @@ +import { settledDaemonRestart } from '../../daemon/daemon-restart-state' import { getAppEnvironment } from '../../../shared/app-environment' import type { OrcaRuntimeService } from '../../runtime/orca-runtime' import type { Store } from '../../persistence' @@ -104,7 +105,12 @@ export function registerPtyHandlers( return undefined } // Why: during cold start the daemon provider swap overlaps first paint, so local spawns must wait; SSH/headless don't use the desktop daemon. - return options?.awaitLocalPtyStartup?.() + const startup = options?.awaitLocalPtyStartup?.() + // A pane replaced by a restart spawns while it runs; it must reach the new daemon, not the old. + const restart = settledDaemonRestart() + return startup && restart + ? Promise.all([startup, restart]).then(() => undefined) + : (startup ?? restart) } const getLocalPtyProviderStartupPromise = ( diff --git a/src/renderer/src/components/terminal-pane/pty-connection-restart-replacement.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-restart-replacement.test.ts new file mode 100644 index 00000000000..bbcd67e7ec1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-restart-replacement.test.ts @@ -0,0 +1,275 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + leafIdForPane, + createMockTransport, + createPane, + createManager +} from './pty-connection-test-pane-fixtures' +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 { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ + toast: { + info: toastInfo + } +})) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +// Why: the working→idle test invokes the real useNotificationDispatch hook outside React, so useCallback must pass through (safe suite-wide: no test here renders React). +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn( + (_environmentId: string, options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + } + ) +})) + +// Why: stub only getEagerPtyBufferHandle so tests can simulate a live eager buffer (adopt path) without standing up the real IPC dispatcher. +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) +} + +describe('connectPanePty', () => { + beforeEach(async () => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + await installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + /** A pane running an agent, with the live recovery record the app keeps for every agent turn. */ + async function connectWithAgent(opts: { visible: boolean; turnDone?: boolean }) { + const { connectPanePty } = await import('./pty-connection') + const { deliverPtyExitToHandlers } = await import('./pty-exit-delivery') + const transport = createMockTransport('pty-pane-2') + transportFactoryQueue.push(transport) + const deps = createDeps({ isVisibleRef: { current: opts.visible } }) + const pane = createPane(2) + const paneKey = `tab-1:${leafIdForPane(2)}` + mockStoreState.removeAgentStatus.mockImplementation((key: string) => { + delete mockStoreState.agentStatusByPaneKey[key] + }) + mockStoreState.sleepingAgentSessionsByPaneKey[paneKey] = { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + agent: 'claude', + providerSession: { key: 'session_id', id: 'sess-1' }, + prompt: 'fix it', + state: opts.turnDone ? 'done' : 'working', + capturedAt: 1, + updatedAt: 1, + origin: 'live' + } + // 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 + const binding = connectPanePty(...args) + await flushAsyncTicks() + const onPtyExit = createdTransportOptions[0]?.onPtyExit + if (typeof onPtyExit !== 'function') { + throw new Error('expected the transport exit handler') + } + const exit = async (replacedByRestart: boolean): Promise => { + deliverPtyExitToHandlers({ + ptyId: 'tab-pty', + code: -1, + replacedByRestart, + primary: (code) => onPtyExit('tab-pty', code), + sidecars: [] + }) + await flushAsyncTicks() + } + const connects = (): number => transport.connect.mock.calls.length + const lastCommand = (): string | undefined => transport.connect.mock.calls.at(-1)?.[0]?.command + return { binding, exit, connects, lastCommand } + } + + it('resumes a restart-replaced hidden pane once, when it is next shown', async () => { + const pane = await connectWithAgent({ visible: false }) + const before = pane.connects() + + await pane.exit(true) + expect(pane.connects()).toBe(before) + + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + expect(pane.connects()).toBe(before + 1) + expect(pane.lastCommand()).toContain('sess-1') + + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + expect(pane.connects()).toBe(before + 1) + }) + + it('resumes a restart-replaced pane at once when it is visible', async () => { + const pane = await connectWithAgent({ visible: true }) + const before = pane.connects() + + await pane.exit(true) + + expect(pane.connects()).toBe(before + 1) + expect(pane.lastCommand()).toContain('sess-1') + }) + + it('spawns nothing for a restart-replaced pane closed before it is shown', async () => { + const pane = await connectWithAgent({ visible: false }) + await pane.exit(true) + const before = pane.connects() + + pane.binding.dispose() + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(pane.connects()).toBe(before) + }) + + it('spawns nothing when another flow rebound the pane before it is shown', async () => { + const pane = await connectWithAgent({ visible: false }) + await pane.exit(true) + const before = pane.connects() + mockStoreState.terminalLayoutsByTabId = { + ...mockStoreState.terminalLayoutsByTabId, + 'tab-1': { + root: { type: 'leaf', leafId: leafIdForPane(2) }, + activeLeafId: leafIdForPane(2), + expandedLeafId: null, + ptyIdsByLeafId: { [leafIdForPane(2)]: 'pty-replacement' } + } + } + + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(pane.connects()).toBe(before) + }) + + it('resumes an agent whose turn had finished the same way, once, when shown', async () => { + const pane = await connectWithAgent({ visible: false, turnDone: true }) + const before = pane.connects() + await pane.exit(true) + expect(pane.connects()).toBe(before) + + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(pane.connects()).toBe(before + 1) + expect(pane.lastCommand()).toContain('sess-1') + }) + + it('leaves a pane lost to any other -1 exit without a process', async () => { + const pane = await connectWithAgent({ visible: false }) + const before = pane.connects() + await pane.exit(false) + + pane.binding.noteVisibilityResume() + await flushAsyncTicks() + + expect(pane.connects()).toBe(before) + }) +}) 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 e592c745ee2..7bc1ddf0402 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 @@ -19,6 +19,7 @@ import type { PtyPaneStartup } from '../pty-connection-types' import type { ConnectPanePtySession } from './connect-pane-pty-session' import { isPtyExitReplacedByRestart } from '../pty-exit-delivery' +import { armRestartReplacement } from './restart-replacement-on-show' // Why live+done arms too: kept from #16308's wake-sweep widening; hibernation itself only writes worktree-sleep notes. export function noteArmsHibernatedPaneWake(record: SleepingAgentSessionRecord): boolean { @@ -219,15 +220,15 @@ export function installPtyExitHibernate(session: ConnectPanePtySession): void { // Why: main clears gate state on PTY exit too; this only resets the // pane-local marker so a reused pane cannot skip re-marking a new PTY. session.releaseHiddenRendererPtyDelivery() + const replacedByRestart = isPtyExitReplacedByRestart(ptyId) // A synthetic host-loss exit only retires this transport. Keep the mounted // leaf↔PTY identity so reconnect/replay can adopt it after the host returns. - if (!isUnverifiedExit) { + // A restart's process is gone for good, so its replacement binds as a first spawn. + if (!isUnverifiedExit || replacedByRestart) { session.clearPanePtyFitBinding() } const isSuppressedExit = - session.deps.consumeSuppressedPtyExit(ptyId) || - preserveRendererBinding || - isPtyExitReplacedByRestart(ptyId) + session.deps.consumeSuppressedPtyExit(ptyId) || preserveRendererBinding || replacedByRestart if (!isSuppressedExit && !isUnverifiedExit) { session.clearExitedPanePtyLayoutBinding(ptyId) } @@ -262,6 +263,11 @@ export function installPtyExitHibernate(session: ConnectPanePtySession): void { // Why: the action that suppressed the exit owns whether the leaf binding // is a wake hint or should be discarded; runtime cleanup above is enough. session.manager.setPaneGpuRendering(session.pane.id, true) + if (replacedByRestart) { + // Nothing else will replace this process; the pane takes it back when shown. + armRestartReplacement(session, ptyId) + return + } const sleepingRecordEntry = session.getSleepingRecordForPane(useAppStore.getState()) if (sleepingRecordEntry && noteArmsHibernatedPaneWake(sleepingRecordEntry.record)) { // Why: hibernation killed this pane's PTY while hidden. The frozen TUI diff --git a/src/renderer/src/components/terminal-pane/pty-connection/restart-replacement-on-show.ts b/src/renderer/src/components/terminal-pane/pty-connection/restart-replacement-on-show.ts new file mode 100644 index 00000000000..411fa206b99 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection/restart-replacement-on-show.ts @@ -0,0 +1,38 @@ +import { useAppStore } from '@/store' +import type { ConnectPanePtySession } from './connect-pane-pty-session' + +/** + * A terminal service restart ended this pane's process and nothing replaced it. The pane gets its + * replacement the way a relaunch would (fresh shell, cold-restore agent resume, old output kept), + * but only once it is shown, so a restart never resumes every background agent at once. + */ +export function armRestartReplacement(session: ConnectPanePtySession, ptyId: string): void { + session.restartReplacementPtyId = ptyId + if (session.deps.isVisibleRef.current) { + // Deferred off the exit handler, as the hibernation wake is. + queueMicrotask(() => consumeRestartReplacement(session)) + } +} + +/** Spawns the armed replacement, unless the pane closed or another flow already rebound it. */ +export function consumeRestartReplacement(session: ConnectPanePtySession): void { + const ptyId: string | null = session.restartReplacementPtyId ?? null + if (!ptyId) { + return + } + session.restartReplacementPtyId = null + const currentPtyId = session.transport.getPtyId() + const storedPtyId = + useAppStore.getState().terminalLayoutsByTabId[session.deps.tabId]?.ptyIdsByLeafId?.[ + session.pane.leafId + ] + if ( + session.disposed || + session.deps.paneTransportsRef.current.get(session.pane.id) !== session.transport || + (currentPtyId !== null && currentPtyId !== ptyId) || + (storedPtyId !== undefined && storedPtyId !== ptyId) + ) { + return + } + void session.startFreshColdRestoreAgentResume() +} 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 8f2f9c91b0f..267074e766c 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 @@ -1,4 +1,5 @@ import { useAppStore } from '@/store' +import { consumeRestartReplacement } from './restart-replacement-on-show' import { shouldReconcileMissingSession, type HasPty } from '../terminal-dead-session-reconcile' import { cancelPendingSafeFitContinuations } from '@/lib/pane-manager/pane-tree-ops' import { PANE_PTY_RESIZE_HOLD_FLUSH_EVENT } from '@/lib/pane-manager/pane-pty-resize-hold' @@ -75,6 +76,7 @@ export function installSessionReconcileDispose(session: ConnectPanePtySession): session.claimPendingVisibleRemoteViewport() session.ptySizeReassertion.request({ fit: false }) session.consumeHibernatedAgentWake() + consumeRestartReplacement(session) session.resumeDeferredSpawnOnReveal() session.requestKnownWindowsShiftEnterReconfirmation() session.sampleVisiblePaneForegroundAgent() diff --git a/tests/e2e/workspace-layout-oracle-known-on-main.ts b/tests/e2e/workspace-layout-oracle-known-on-main.ts index bb4aae8c7f0..e32d096b537 100644 --- a/tests/e2e/workspace-layout-oracle-known-on-main.ts +++ b/tests/e2e/workspace-layout-oracle-known-on-main.ts @@ -24,13 +24,6 @@ type KnownOnMain = { const TRANSIENT = /\(transient\)$/ export const LAYOUT_ORACLE_KNOWN_ON_MAIN: readonly KnownOnMain[] = [ - // The in-app daemon restart keeps every pane, but nothing starts a new process in them. - { - scenario: 'live-daemon-restart', - check: 'marker', - step: 'after daemon restart', - cause: 'panes stay without a process after a daemon restart' - }, // A headless host lists a worktree with no saved tab bar (and, after a restart, every worktree) // under a synthesized `headless-terminals:` group, while the layout stream names the same group // by its saved or minted id: same tabs and order, two group ids. One projection in PR 6.