From cc7d03d19c511eed1bbb2d12f5964404d74ad798 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:25:20 -0400 Subject: [PATCH] fix(daemon): keep every pane in main's layout through a terminal service restart (#28852) Restarting the terminal service ends each session with a synthetic -1 exit. Main read those as ordinary deaths and retired every terminal surface from the saved layout, while the window kept drawing them. Phones and the CLI lost the tabs, and the next launch restored a split as a single pane. The restart now labels those exits as replaced, and main records them in its existing intentional-stop register, the one terminal restarts already use. The runtime keeps the surface and the window receives the same replaced label. Claude-Session: p2-headless --- .../daemon-init-restart-sequence.test.ts | 4 +- src/main/daemon/daemon-provider-restart.ts | 4 +- .../daemon/daemon-pty-listener-emission.ts | 5 +- src/main/daemon/daemon-pty-router-events.ts | 2 + .../daemon/daemon-pty-session-inventory.ts | 8 ++- .../degraded-daemon-pty-provider.test.ts | 8 ++- .../daemon/degraded-daemon-pty-provider.ts | 8 +-- src/main/ipc/pty/provider/bind-listeners.ts | 50 ++++++++++++------- src/main/providers/pty-provider-contract.ts | 2 + .../terminal-intentional-stop-exit.test.ts | 35 +++++++++++-- .../runtime/terminal-intentional-stops.ts | 3 +- .../terminal-parked-pty-watcher.ts | 4 +- ...arked-watcher-sleep-preserved-exit.test.ts | 2 +- .../helpers/terminal-restart-persistence.ts | 5 +- .../workspace-layout-oracle-markers.ts | 16 ++++-- .../workspace-layout-oracle-session.ts | 8 ++- .../workspace-layout-oracle-known-on-main.ts | 19 +++---- 17 files changed, 123 insertions(+), 60 deletions(-) diff --git a/src/main/daemon/daemon-init-restart-sequence.test.ts b/src/main/daemon/daemon-init-restart-sequence.test.ts index df3b48417cb..e9cfcd704e9 100644 --- a/src/main/daemon/daemon-init-restart-sequence.test.ts +++ b/src/main/daemon/daemon-init-restart-sequence.test.ts @@ -74,7 +74,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { // killedCount must be 3 — proves the count was taken *before* fanout cleared the set (a swapped-order bug reports 0). expect(result.killedCount).toBe(3) - expect(originalAdapter.fanoutSyntheticExits).toHaveBeenCalledWith(-1) + expect(originalAdapter.fanoutSyntheticExits).toHaveBeenCalledWith(-1, { + replacedByRestart: true + }) // Ordering invariant: synthetic exits must reach the renderer *before* listeners are torn down (Step 1 before 2). expect(order).toEqual(['fanout', 'unbind']) }) diff --git a/src/main/daemon/daemon-provider-restart.ts b/src/main/daemon/daemon-provider-restart.ts index f6d11d868f8..83f476c8396 100644 --- a/src/main/daemon/daemon-provider-restart.ts +++ b/src/main/daemon/daemon-provider-restart.ts @@ -83,9 +83,9 @@ async function runRestartDaemon(expectedDaemonPid?: number): Promise(listeners: readonly T[], emit: (listener: T) export function createPtyExitPayload( id: string, - { code, incarnationId, cause }: Omit + { code, incarnationId, cause, replacedByRestart }: Omit ): DaemonPtyRouterExitEvent { return { id, code, ...(incarnationId ? { incarnationId } : {}), - ...(cause ? { cause } : {}) + ...(cause ? { cause } : {}), + ...(replacedByRestart ? { replacedByRestart } : {}) } } diff --git a/src/main/daemon/daemon-pty-router-events.ts b/src/main/daemon/daemon-pty-router-events.ts index 92b626d36e9..eb12c2d7cfe 100644 --- a/src/main/daemon/daemon-pty-router-events.ts +++ b/src/main/daemon/daemon-pty-router-events.ts @@ -15,4 +15,6 @@ export type DaemonPtyRouterExitEvent = { incarnationId?: PtyIncarnationId /** Absent from a daemon predating exit causes; readers fall back to `unknown`. */ cause?: TerminalExitCause + /** Main ended the session to restart the daemon; the pane stays for its next process. */ + replacedByRestart?: true } diff --git a/src/main/daemon/daemon-pty-session-inventory.ts b/src/main/daemon/daemon-pty-session-inventory.ts index dc3c816f1ec..5f3064dbec6 100644 --- a/src/main/daemon/daemon-pty-session-inventory.ts +++ b/src/main/daemon/daemon-pty-session-inventory.ts @@ -137,7 +137,7 @@ export abstract class DaemonPtySessionInventory extends DaemonPtyProcessInspecti // Why: the daemon's kill-all-and-shutdown path suppresses onExit fanout (session.ts:246-252), so synthesize pty:exit // for every live session before teardown or renderer panes black-hole writes to a disposed adapter forever. - fanoutSyntheticExits(code: number): void { + fanoutSyntheticExits(code: number, exit: { replacedByRestart?: true } = {}): void { const ids = [...this.activeSessionIds] this.activeSessionIds.clear() this.sessionsAwaitingDaemonRecovery.clear() @@ -158,7 +158,11 @@ export abstract class DaemonPtySessionInventory extends DaemonPtyProcessInspecti // Why: don't catch listener throws — matches the natural onExit fanout so synthetic exits keep the same error semantics. emitPtyListeners(this.exitListeners, (listener) => listener( - createPtyExitPayload(id, { code, incarnationId: this.sessionIncarnations.get(id) }) + createPtyExitPayload(id, { + code, + incarnationId: this.sessionIncarnations.get(id), + ...exit + }) ) ) this.sessionIncarnations.delete(id) diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index eed79501220..161a676d2b8 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -666,10 +666,14 @@ describe('DegradedDaemonPtyProvider', () => { provider.onExit(exitSpy) await provider.discoverDaemonSessions() - provider.fanoutCurrentDaemonSyntheticExits(-1) + provider.fanoutCurrentDaemonSyntheticExits(-1, { replacedByRestart: true }) expect(exitSpy).toHaveBeenCalledOnce() - expect(exitSpy).toHaveBeenCalledWith({ id: 'current-session', code: -1 }) + expect(exitSpy).toHaveBeenCalledWith({ + id: 'current-session', + code: -1, + replacedByRestart: true + }) expect(provider.getSessionIdsRoutedTo('current-daemon')).toEqual([]) expect(provider.hasPty('legacy-session')).toBe(true) }) diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index f1db303ddfd..bf021bce8f8 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -33,7 +33,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { private ownerRecovery: DegradedDaemonOwnerRecovery private unsubscribers: (() => void)[] = [] private dataListeners: ((payload: PtyDataEvent) => void)[] = [] - private exitListeners: ((payload: { id: string; code: number }) => void)[] = [] + private exitListeners: Parameters[0][] = [] constructor(opts: { current: DaemonPtyAdapter @@ -267,7 +267,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { return trackedUnsubscribe } - onExit(callback: (payload: { id: string; code: number }) => void): () => void { + onExit(callback: Parameters[0]): () => void { this.exitListeners.push(callback) return () => { const idx = this.exitListeners.indexOf(callback) @@ -317,13 +317,13 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { return listProviderSessionIds(this.sessionProviders, target) } - fanoutCurrentDaemonSyntheticExits(code: number): void { + fanoutCurrentDaemonSyntheticExits(code: number, exit: { replacedByRestart?: true } = {}): void { for (const id of this.getSessionIdsRoutedTo('current-daemon')) { this.sessionProviders.delete(id) // Why: restart kills listed sessions even when the adapter did not track them active. // oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration for (const listener of [...this.exitListeners]) { - listener({ id, code }) + listener({ id, code, ...exit }) } } } diff --git a/src/main/ipc/pty/provider/bind-listeners.ts b/src/main/ipc/pty/provider/bind-listeners.ts index ec13ad0d36e..5d455b43539 100644 --- a/src/main/ipc/pty/provider/bind-listeners.ts +++ b/src/main/ipc/pty/provider/bind-listeners.ts @@ -88,28 +88,40 @@ export function bindProviderListeners(session: PtyIpcSession): void { return } const syntheticExit = session.consumeSyntheticKillExit(payload.id, payload.incarnationId) - if (!isLocalProvider) { - clearProviderPtyState(payload.id) - ptyOwnership.delete(payload.id) - if (syntheticExit) { - session.runtime?.markPtyStopRequested(payload.id) + // Main restarted the daemon: the pane outlives its process, for the runtime and the window. + const settleRestartStop = payload.replacedByRestart + ? session.runtime?.intentionalPtyStops?.mark( + payload.id, + 'replaced', + payload.incarnationId ?? null + ) + : undefined + try { + if (!isLocalProvider) { + clearProviderPtyState(payload.id) + ptyOwnership.delete(payload.id) + if (syntheticExit) { + session.runtime?.markPtyStopRequested(payload.id) + } + session.runtime?.onPtyExit(payload.id, payload.code, payload.incarnationId, { + providerExitObserved: true, + ...(payload.cause ? { cause: payload.cause } : {}) + }) } - session.runtime?.onPtyExit(payload.id, payload.code, payload.incarnationId, { - providerExitObserved: true, - ...(payload.cause ? { cause: payload.cause } : {}) + // The control reply can overtake stream data; the physical exit must retire that late output. + if (syntheticExit) { + return + } + // Why not the whole payload: the exit cause is a main-process fact for the + // runtime's records; the renderer's pty:exit contract stays as it was. + session.sendPtyExitToRenderer({ + id: payload.id, + code: payload.code, + ...(payload.incarnationId ? { incarnationId: payload.incarnationId } : {}) }) + } finally { + settleRestartStop?.(true) } - // The control reply can overtake stream data; the physical exit must retire that late output. - if (syntheticExit) { - return - } - // Why not the whole payload: the exit cause is a main-process fact for the - // runtime's records; the renderer's pty:exit contract stays as it was. - session.sendPtyExitToRenderer({ - id: payload.id, - code: payload.code, - ...(payload.incarnationId ? { incarnationId: payload.incarnationId } : {}) - }) }) ) } diff --git a/src/main/providers/pty-provider-contract.ts b/src/main/providers/pty-provider-contract.ts index 1f17bc05a44..b16728b4423 100644 --- a/src/main/providers/pty-provider-contract.ts +++ b/src/main/providers/pty-provider-contract.ts @@ -264,6 +264,8 @@ export type IPtyProvider = { incarnationId?: PtyIncarnationId /** Absent when the provider predates exit causes; readers must not infer one from `code`. */ cause?: TerminalExitCause + /** Main ended the session to restart the daemon; the pane stays for its next process. */ + replacedByRestart?: true }) => void ): () => void } diff --git a/src/main/runtime/terminal-intentional-stop-exit.test.ts b/src/main/runtime/terminal-intentional-stop-exit.test.ts index 041d9710936..95cac3c8c5b 100644 --- a/src/main/runtime/terminal-intentional-stop-exit.test.ts +++ b/src/main/runtime/terminal-intentional-stop-exit.test.ts @@ -84,9 +84,7 @@ function createHarness(opts: { lateProviderExit?: boolean; folder?: boolean } = }) ptyOwnership.set(PTY_ID, 'local') ptyIncarnationById.set(PTY_ID, INCARNATION_ID) - let emitProviderExit: - | ((payload: { id: string; code: number; incarnationId?: string }) => void) - | undefined + let emitProviderExit: Parameters[0] | undefined const provider = { onData: () => () => {}, onExit: (listener: typeof emitProviderExit) => { @@ -124,6 +122,14 @@ function createHarness(opts: { lateProviderExit?: boolean; folder?: boolean } = deps, emitProviderExit: (incarnationId: string) => emitProviderExit?.({ id: PTY_ID, code: 0, incarnationId }), + /** The exit main synthesizes for each session when it restarts the daemon. */ + emitDaemonRestartExit: (replacedByRestart?: true) => + emitProviderExit?.({ + id: PTY_ID, + code: -1, + incarnationId: INCARNATION_ID, + ...(replacedByRestart ? { replacedByRestart } : {}) + }), boundPtyId: () => store.getWorkspaceSession().terminalLayoutsByTabId[TAB_ID]?.ptyIdsByLeafId?.[LEAF_ID] ?? null, tabIds: () => (store.getWorkspaceSession().tabsByWorktree[WORKTREE_ID] ?? []).map((t) => t.id), @@ -210,6 +216,29 @@ describe('intentional stops keep the pane through the exit', () => { ]) }) + it('keeps the pane when main restarts the daemon under it', () => { + const harness = createHarness() + + harness.emitDaemonRestartExit(true) + + expect(harness.tabIds()).toEqual([TAB_ID]) + expect(harness.boundPtyId()).toBe(PTY_ID) + expect(harness.rendererExits()).toEqual([ + { id: PTY_ID, code: -1, incarnationId: INCARNATION_ID, replacedByRestart: true } + ]) + }) + + it('retires the pane on a daemon-wide exit that no restart claims', () => { + const harness = createHarness() + + harness.emitDaemonRestartExit() + + expect(harness.boundPtyId()).toBeNull() + expect(harness.rendererExits()).toEqual([ + { id: PTY_ID, code: -1, incarnationId: INCARNATION_ID } + ]) + }) + it('keeps the pane through the synthetic exit and the provider exit that follows it', async () => { const harness = createHarness({ lateProviderExit: true }) diff --git a/src/main/runtime/terminal-intentional-stops.ts b/src/main/runtime/terminal-intentional-stops.ts index 88ac59a40d4..836743fde67 100644 --- a/src/main/runtime/terminal-intentional-stops.ts +++ b/src/main/runtime/terminal-intentional-stops.ts @@ -3,7 +3,8 @@ import { SYNTHETIC_KILL_EXIT_DUPLICATE_WINDOW_MS } from '../ipc/pty/delivery/vis /** * Why main stopped a PTY on purpose. Both kinds keep the pane's binding through the exit: * - `reversible`: sleep or hibernation; the binding is the wake hint. - * - `replaced`: a restart stops the old process, or a reattach finds it gone; a new one takes over. + * - `replaced`: a restart stops the old process, or a reattach finds it gone; a new one takes over, + * at once or (after a daemon restart) when the pane next connects. */ export type TerminalIntentionalStopKind = 'reversible' | 'replaced' 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 248ed6f9b6d..7541630a278 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 @@ -61,13 +61,15 @@ export function startParkedPtyWatcher(args: { return } const handlePtyExit = (code: number, { hadPrimary }: { hadPrimary: boolean }): void => { + // Why first: a daemon restart's replacement starts only when the pane next connects, so the + // dead process's title (a 'working' agent pins worktree status) must not wait for it. + useAppStore.getState().clearRuntimePaneTitle(tab.id, pane.paneId) 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 // explicit close by either this watcher or the orphan sweep. 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 0349c6aa061..7cf83aa2e21 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 @@ -149,7 +149,7 @@ describe('sleep-preserved parked exits (sole-owner sidecar)', () => { }) expect(closeTerminalTab).not.toHaveBeenCalled() - expect(mockStoreState.clearRuntimePaneTitle).not.toHaveBeenCalledWith(TAB_ID, 1) + expect(mockStoreState.clearRuntimePaneTitle).toHaveBeenCalledWith(TAB_ID, 1) expect(startedWatcherDisposers[0]).toHaveBeenCalled() expect(entry.disposersByPtyId.size).toBe(0) }) diff --git a/tests/e2e/helpers/terminal-restart-persistence.ts b/tests/e2e/helpers/terminal-restart-persistence.ts index 46ba4efda9f..42d8af829df 100644 --- a/tests/e2e/helpers/terminal-restart-persistence.ts +++ b/tests/e2e/helpers/terminal-restart-persistence.ts @@ -69,7 +69,8 @@ export async function bootstrapFirstLaunch( */ export async function bootstrapRestoredLaunch( page: Page, - expectedWorktreeId: string + expectedWorktreeId: string, + paneCount = 1 ): Promise { await waitForSessionReady(page) await expect @@ -80,7 +81,7 @@ export async function bootstrapRestoredLaunch( // restored terminal surface is what we're about to assert against, so make // sure it exists before any content/layout assertion races. await waitForActiveTerminalManager(page, 30_000) - await waitForPaneCount(page, 1, 30_000) + await waitForPaneCount(page, paneCount, 30_000) } export async function setPaneTitleFromTerminalMenu(page: Page, title: string): Promise { diff --git a/tests/e2e/helpers/workspace-layout-oracle-markers.ts b/tests/e2e/helpers/workspace-layout-oracle-markers.ts index e285e2acbb5..026ff74e61a 100644 --- a/tests/e2e/helpers/workspace-layout-oracle-markers.ts +++ b/tests/e2e/helpers/workspace-layout-oracle-markers.ts @@ -35,12 +35,18 @@ export async function checkPaneMarkers( continue } const marker = `ORACLE_MARK_${Date.now().toString(36)}_${index}` - await client.call('terminal.send', { - terminal: terminal.handle, - text: `echo ${marker}`, - enter: true - }) checked += 1 + try { + await client.call('terminal.send', { + terminal: terminal.handle, + text: `echo ${marker}`, + enter: true + }) + } catch (error) { + // A pane whose process is gone is a finding, not a harness fault. + failures.push(`pane ${terminal.tabId}:${terminal.leafId} refused ${marker}: ${String(error)}`) + continue + } const deadline = Date.now() + 10_000 let shown = false while (!shown && Date.now() < deadline) { diff --git a/tests/e2e/helpers/workspace-layout-oracle-session.ts b/tests/e2e/helpers/workspace-layout-oracle-session.ts index 5978956f9d0..e2cdb58bf08 100644 --- a/tests/e2e/helpers/workspace-layout-oracle-session.ts +++ b/tests/e2e/helpers/workspace-layout-oracle-session.ts @@ -122,8 +122,12 @@ export async function runOracleScenario( run.client = new RuntimeClient(session.userDataDir, 30_000) await subscribeWindowLayout(next.page, feed) await run.oracle.retarget(target(next.page, run.client)) - await (reopen ?? bootstrapRestoredLaunch)(next.page, worktreeId) - await waitForPaneCount(next.page, paneCount, 30_000).catch(() => {}) + if (reopen) { + await reopen(next.page, worktreeId) + await waitForPaneCount(next.page, paneCount, 30_000).catch(() => {}) + } else { + await bootstrapRestoredLaunch(next.page, worktreeId, paneCount) + } const restartsTerminals = coldDaemon || options.terminalsRestart const after = await run.oracle.step( coldDaemon ? 'after cold relaunch' : 'after relaunch', diff --git a/tests/e2e/workspace-layout-oracle-known-on-main.ts b/tests/e2e/workspace-layout-oracle-known-on-main.ts index f5bc67ebda4..bb4aae8c7f0 100644 --- a/tests/e2e/workspace-layout-oracle-known-on-main.ts +++ b/tests/e2e/workspace-layout-oracle-known-on-main.ts @@ -24,20 +24,13 @@ type KnownOnMain = { const TRANSIENT = /\(transient\)$/ export const LAYOUT_ORACLE_KNOWN_ON_MAIN: readonly KnownOnMain[] = [ - // The in-app daemon restart drops the worktree's tabs from main while the window keeps them. - ...(['view', 'expected', 'restart', 'marker'] as const).map((check) => ({ + // The in-app daemon restart keeps every pane, but nothing starts a new process in them. + { scenario: 'live-daemon-restart', - check, - cause: 'daemon restart empties the runtime layout' - })), - // The relaunch after it has no tab bar for the worktree, so its group is minted again (Linux CI). - ...(['rules', 'client'] as const).map((check) => ({ - scenario: 'live-daemon-restart', - check, - step: 'after relaunch', - platform: 'linux' as const, - cause: 'daemon restart empties the runtime layout' - })), + 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.