From a27c691fdd304da204fbdc0424e8caa92416ef39 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:05:58 -0700 Subject: [PATCH] fix(terminal): stop detached exit observers pinning evicted panes' xterm buffers (#16551) --- .../pty-connection/pane-pty-binding.ts | 5 + .../session-reconcile-dispose.ts | 2 + .../pty-transport-reattach-admission.test.ts | 27 +++ .../terminal-parked-pty-watcher.ts | 48 ++++- .../terminal-parked-tab-watchers.test.ts | 21 +- ...inal-parked-watcher-reconciliation.test.ts | 58 ++++++ .../terminal-parked-watcher-reconciliation.ts | 12 +- .../terminal-parked-watcher-registry.ts | 5 + ...arked-watcher-sleep-preserved-exit.test.ts | 173 ++++++++++++++++ ...l-parked-watcher-sole-newborn-exit.test.ts | 188 ++++++++++++++++++ .../use-terminal-pane-lifecycle.ts | 41 +++- 11 files changed, 561 insertions(+), 19 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-parked-watcher-sole-newborn-exit.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-binding.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-binding.ts index 326e2b345ed..86f9523324c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-binding.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-binding.ts @@ -22,4 +22,9 @@ export type PanePtyBinding = IDisposable & { markShortcutTerminalInputSent: () => void reconcileIfSessionDead: (liveSessionIds: Set, snapshotRequestedAt?: number) => void reconcileIfSessionMissing: (hasPty: HasPty, livenessRequestedAt?: number) => void + /** This session fresh-spawned the PTY (onPtySpawn fired for it) and never + * sent terminal input — the sole-newborn diagnostic case pty-exit-hibernate + * preserves on exit. Read at unmount so the parked sidecar can carry the + * fact as a plain value instead of a session reference. */ + isUntouchedFreshSpawnPty: (ptyId: string) => boolean } 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 657a01169bc..16541b4e72d 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 @@ -172,6 +172,8 @@ export function installSessionReconcileDispose(session: ConnectPanePtySession): }, reconcileIfSessionDead: session.reconcileIfSessionDead, reconcileIfSessionMissing: session.reconcileIfSessionMissing, + isUntouchedFreshSpawnPty: (ptyId) => + session.spawnedFreshPtyId === ptyId && !Number.isFinite(session.lastTerminalInputAt), dispose() { session.disposed = true // Why: a detached client stops observing the pane's bytes, so it must cede diff --git a/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts index 41014dae389..879db477f40 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-reattach-admission.test.ts @@ -258,6 +258,33 @@ describe('createIpcPtyTransport', () => { expect(transport.isConnected()).toBe(false) }) + it('respawns a sole-newborn preserved exit at reveal instead of replaying it', async () => { + const { bufferPreHandlerPtyExit, consumePreHandlerPtyState, clearPreHandlerPtyState } = + await import('./pty-pre-handler-buffer') + const { createIpcPtyTransport } = await import('./pty-transport') + const spawn = window.api.pty.spawn as unknown as ReturnType + spawn.mockResolvedValueOnce({ id: 'reveal-pty' }) + const onExitCallback = vi.fn() + const sessionId = 'sole-newborn-dead-session' + // The parked sidecar's sole-newborn guard consumed the buffered exit + // (pre-fix primary parity), so reveal must take the readmission/spawn path. + // exitedBeforeAttach here would drain the exit into the reattached session + // — where spawnedFreshPtyId is null — and close the kept tab on reveal. + bufferPreHandlerPtyExit(sessionId, 1) + consumePreHandlerPtyState(sessionId) + + const result = await createIpcPtyTransport({}).connect({ + url: '', + sessionId, + callbacks: { onExit: onExitCallback } + }) + + expect(result).not.toEqual(expect.objectContaining({ exitedBeforeAttach: true })) + expect(spawn).toHaveBeenCalledWith(expect.objectContaining({ sessionId })) + expect(onExitCallback).not.toHaveBeenCalled() + clearPreHandlerPtyState(sessionId) + }) + it('rejects a buffered dead-session exit before publishing its final frame', async () => { const { bufferPreHandlerPtyData, bufferPreHandlerPtyExit, clearPreHandlerPtyState } = await import('./pty-pre-handler-buffer') 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 b78a3a07e5c..1cd049a3878 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 @@ -4,7 +4,12 @@ import { useAppStore } from '@/store' import { closeTerminalTab } from '../terminal/terminal-tab-actions' import { startParkedTerminalByteWatcher } from './parked-terminal-byte-watcher' import { subscribeToPtyExit } from './pty-dispatcher' -import { discardPreHandlerPtyState, hasPreHandlerPtyExit } from './pty-pre-handler-buffer' +import { + consumePreHandlerPtyState, + discardPreHandlerPtyState, + hasPreHandlerPtyExit +} from './pty-pre-handler-buffer' +import { consumeCommittedPtyShutdownExit } from './pty-shutdown-exit-deferral' import { detachTerminalLayoutLeaf } from './terminal-layout-leaf-detach' import { isParkRestorableTerminalPty, @@ -55,6 +60,16 @@ export function startParkedPtyWatcher(args: { } const handlePtyExit = (_code: number, { hadPrimary }: { hadPrimary: boolean }): void => { useAppStore.getState().clearRuntimePaneTitle(tab.id, pane.paneId) + // Why: detach drops the session-bound exit observer (it pinned the disposed + // pane's xterm buffers), so this sidecar is the sole owner of a parked PTY's + // exit. A sleep/shutdown exit must keep the tab AND its layout — revival + // belongs to the wake path — and must leave the buffered exit in place as + // the tombstone that stops watcher syncs re-pinning the dead PTY. + if (!hadPrimary && isSleepPreservedParkedPtyExit(ptyId)) { + entry.disposersByPtyId.get(ptyId)?.() + entry.disposersByPtyId.delete(ptyId) + return + } if (entry.disposersByPtyId.size > 1) { discardPreHandlerPtyState(ptyId) collapseParkedExitedLeaf(tab.id, ptyId) @@ -67,6 +82,23 @@ export function startParkedPtyWatcher(args: { entry.disposersByPtyId.delete(ptyId) return } + // Why: parity with the session observer's sole-newborn guard (pty-exit-hibernate) — + // a worktree's only fresh-spawned shell nobody ever typed into can die on shell + // startup (e.g. a failing .envrc); keep its tab readable instead of closing it and + // stranding the user on Landing. Split siblings keep their own branch above. + if (pane.untouchedFreshSpawn) { + entry.disposersByPtyId.get(ptyId)?.() + entry.disposersByPtyId.delete(ptyId) + // Consume the buffered exit exactly as the pre-fix primary observer did — NOT + // the sleep branch's tombstone. A tombstone would send reveal down connectIpcPty's + // exitedBeforeAttach path, draining the exit into the reattached session (where + // spawnedFreshPtyId is null, so the guard fails) and closing the tab the moment + // the user reveals it. Consumption cannot let a later watcher sync re-register + // this dead PTY: paneIdByPtyId deliberately keeps the slot, so + // reconcileParkedWatcherPtyIds never computes it as added. + consumePreHandlerPtyState(ptyId) + return + } // Why: the empty entry prevents a pending pinned-close confirmation from restarting the dead PTY. entry.disposersByPtyId.get(ptyId)?.() @@ -105,6 +137,20 @@ export function startParkedPtyWatcher(args: { }) } +// Why these three markers: a pending renderer shutdown transaction, a +// suppressed intentional restart, or a committed sleep (host-initiated remote +// sleep marks it from the exit payload) all mean this exit is orchestrated — +// closing the tab would destroy state the wake/restart path owns. Host-sleep +// dispositions are remote-runtime-only and remote PTYs never reach this +// sidecar, so consumeCommittedPtyShutdownExit runs without an environment id. +function isSleepPreservedParkedPtyExit(ptyId: string): boolean { + const state = useAppStore.getState() + if (state.isPtyShutdownPending(ptyId) || state.suppressedPtyExitIds[ptyId]) { + return true + } + return consumeCommittedPtyShutdownExit(ptyId, null) +} + export function collapseParkedExitedLeaf(tabId: string, ptyId: string): void { const state = useAppStore.getState() const layout = state.terminalLayoutsByTabId[tabId] diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts index cd0448180ba..d64d19f7036 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts @@ -93,6 +93,8 @@ type MockStoreState = { setRuntimePaneTitle: ReturnType setTabLayout: ReturnType updateTabTitle: ReturnType + isPtyShutdownPending: ReturnType + suppressedPtyExitIds: Record } let mockStoreState: MockStoreState @@ -159,7 +161,9 @@ describe('terminal-parked-tab-watchers', () => { clearRuntimePaneTitle: vi.fn(), setRuntimePaneTitle: vi.fn(), setTabLayout: vi.fn(), - updateTabTitle: vi.fn() + updateTabTitle: vi.fn(), + isPtyShutdownPending: vi.fn(() => false), + suppressedPtyExitIds: {} } ;(globalThis as { window?: unknown }).window = { api: { pty: { write: ptyWrite } } } clearTerminalProviderSnapshotCapabilities() @@ -331,11 +335,12 @@ describe('terminal-parked-tab-watchers', () => { expect(getParkedTerminalWatcherTabIds()).toEqual([TAB_ID]) }) - it('collapses a dead split leaf even when a stale primary handler also observed the exit', () => { - // Why (regression, #ghost-blank-pane): a genuinely parked tab's PaneManager - // is already destroyed, so the retained primary exit handler's own - // split-collapse path is a no-op against the persisted layout — hadPrimary - // must not skip this sidecar's collapse for a surviving sibling leaf. + it('collapses a dead split leaf even when a concurrent primary handler also observed the exit', () => { + // Why (regression, #ghost-blank-pane): a primary can coexist with a parked + // sidecar — an eager pre-mount handle, or a reveal remount racing watcher + // disposal. Neither collapses the persisted parked layout (eager handlers + // never touch layout; detach dropped the session observer that once did) — + // hadPrimary must not skip this sidecar's collapse for a surviving leaf. capturePanes([ { ptyId: PTY_ID, paneId: 1, leafId: LEAF_ID, drivesTabTitle: true }, { ptyId: SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false } @@ -431,7 +436,9 @@ describe('terminal-parked-tab-watchers', () => { expect(getParkedTerminalWatcherTabIds()).toEqual([]) }) - it('does not queue a second close when a retained primary handled the parked exit', () => { + // Why still reachable post detach-fix: an eager pre-mount handle or a reveal + // remount's fresh registerExit can own the exit while this sidecar is live. + it('does not queue a second close when a concurrent primary handled the parked exit', () => { capturePanes([{ ptyId: PTY_ID, paneId: 1, leafId: LEAF_ID, drivesTabTitle: true }]) syncParked() diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.test.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.test.ts index 18e8f41cac6..f59d8b4500e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.test.ts @@ -85,3 +85,61 @@ describe('paired parked-watcher reconciliation', () => { }) }) }) + +// Why: the sole-newborn parity flag is a fact about the captured PTY, so the +// layout-fallback rescue must carry it only while the leaf still binds that PTY. +describe('untouchedFreshSpawn carry through the layout-fallback rescue', () => { + const soleLeafState = { + runtimePaneTitlesByTabId: {}, + terminalLayoutsByTabId: { + [TAB_ID]: { + root: { type: 'leaf' as const, leafId: FIRST_LEAF_ID }, + activeLeafId: FIRST_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [FIRST_LEAF_ID]: FIRST_PTY_ID } + } + } + } + + it('keeps the captured fact for the same PTY when the rescue runs', () => { + // Stale capture (an extra pane the layout no longer has) forces the rescue path. + captureParkedTerminalPaneCandidates(TAB_ID, WORKTREE_ID, [ + { + ptyId: FIRST_PTY_ID, + paneId: 1, + leafId: FIRST_LEAF_ID, + drivesTabTitle: true, + untouchedFreshSpawn: true + }, + { ptyId: OLD_SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false } + ]) + + const panes = resolveParkedTerminalPaneCandidates( + { id: TAB_ID, ptyId: FIRST_PTY_ID }, + soleLeafState + ) + + expect(panes).toHaveLength(1) + expect(panes[0].untouchedFreshSpawn).toBe(true) + }) + + it('drops the fact when the leaf re-minted a different PTY', () => { + captureParkedTerminalPaneCandidates(TAB_ID, WORKTREE_ID, [ + { + ptyId: OLD_SECOND_PTY_ID, + paneId: 1, + leafId: FIRST_LEAF_ID, + drivesTabTitle: true, + untouchedFreshSpawn: true + } + ]) + + const panes = resolveParkedTerminalPaneCandidates( + { id: TAB_ID, ptyId: FIRST_PTY_ID }, + soleLeafState + ) + + expect(panes).toHaveLength(1) + expect(panes[0].untouchedFreshSpawn).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.ts index 0b2939aade8..148c71a174d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.ts @@ -62,7 +62,17 @@ export function resolveParkedTerminalPaneCandidates( } return fallback.map((pane) => { const prior = captured?.panes.find((candidate) => candidate.leafId === pane.leafId) - return prior ? { ...pane, paneId: prior.paneId, drivesTabTitle: prior.drivesTabTitle } : pane + return prior + ? { + ...pane, + paneId: prior.paneId, + drivesTabTitle: prior.drivesTabTitle, + // Why: the fact belongs to the captured PTY; a re-minted id on this leaf is a different shell. + ...(prior.ptyId === pane.ptyId && prior.untouchedFreshSpawn + ? { untouchedFreshSpawn: true } + : {}) + } + : pane }) } diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts index f0184b2b7aa..7e305ac7fe3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts @@ -18,6 +18,11 @@ export type ParkedTerminalPaneCapture = { /** Stable terminal-layout leaf UUID (paneKey attribution). */ leafId: string drivesTabTitle: boolean + /** The unmounted pane fresh-spawned this PTY and no terminal input was ever + * sent. Plain-value carry of the session facts behind the sole-newborn + * guard (pty-exit-hibernate), so the parked sidecar preserves a dead + * never-typed shell's tab the same way the session observer did. */ + untouchedFreshSpawn?: boolean } export type CapturedTabPanes = { worktreeId: string; panes: ParkedTerminalPaneCapture[] } 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 new file mode 100644 index 00000000000..3e9ea0a25b8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sleep-preserved-exit.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// Why this file: detach drops the session-bound primary exit observer (it pinned +// the disposed pane's xterm buffers), so the parked watcher sidecar is the sole +// owner of a parked PTY's exit. Orchestrated sleep/shutdown exits must keep the +// tab and the layout the wake path restores. + +const WORKTREE_ID = 'repo::/worktree' +const TAB_ID = 'tab-1' +const PTY_ID = `${WORKTREE_ID}@@session-1` +const SECOND_PTY_ID = `${WORKTREE_ID}@@session-2` +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const SECOND_LEAF_ID = '22222222-2222-4222-8222-222222222222' + +const startedWatcherDisposers: ReturnType[] = [] +vi.mock('./parked-terminal-byte-watcher', () => ({ + startParkedTerminalByteWatcher: () => { + const dispose = vi.fn() + startedWatcherDisposers.push(dispose) + return dispose + } +})) + +type ExitCallback = (code: number, context: { hadPrimary: boolean }) => void +const exitCallbacksByPtyId = new Map() +vi.mock('./pty-dispatcher', () => ({ + subscribeToPtyExit: (ptyId: string, callback: ExitCallback) => { + exitCallbacksByPtyId.set(ptyId, callback) + return vi.fn() + } +})) + +const discardPreHandlerPtyState = vi.fn() +vi.mock('./pty-pre-handler-buffer', () => ({ + consumePreHandlerPtyState: vi.fn(), + discardPreHandlerPtyState: (ptyId: string) => discardPreHandlerPtyState(ptyId), + hasPreHandlerPtyExit: () => false +})) + +const closeTerminalTab = vi.fn() +vi.mock('../terminal/terminal-tab-actions', () => ({ + closeTerminalTab: (tabId: string, options?: unknown) => closeTerminalTab(tabId, options) +})) + +type MockStoreState = { + tabsByWorktree: Record + terminalLayoutsByTabId: Record + runtimePaneTitlesByTabId: Record> + clearTabLaunchAgent: ReturnType + clearRuntimePaneTitle: ReturnType + setTabLayout: ReturnType + updateTabTitle: ReturnType + isPtyShutdownPending: ReturnType + suppressedPtyExitIds: Record +} +let mockStoreState: MockStoreState +vi.mock('@/store', () => ({ + useAppStore: { getState: () => mockStoreState } +})) + +import { + consumeCommittedPtyShutdownExit, + markCommittedPtyShutdowns +} from './pty-shutdown-exit-deferral' +import { startParkedPtyWatcher } from './terminal-parked-pty-watcher' +import type { ParkedTabWatcherEntry } from './terminal-parked-watcher-registry' + +function startSplitWatchers(): ParkedTabWatcherEntry { + const entry: ParkedTabWatcherEntry = { + worktreeId: WORKTREE_ID, + tabPtyId: PTY_ID, + paneIdByPtyId: new Map(), + disposersByPtyId: new Map() + } + const tab = { id: TAB_ID, ptyId: PTY_ID } + for (const pane of [ + { ptyId: PTY_ID, paneId: 1, leafId: LEAF_ID, drivesTabTitle: true }, + { ptyId: SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false } + ]) { + startParkedPtyWatcher({ + worktreeId: WORKTREE_ID, + tab, + pane, + entry, + restoreTitleOnRegister: false, + restorePolicy: {} + }) + } + return entry +} + +describe('sleep-preserved parked exits (sole-owner sidecar)', () => { + beforeEach(() => { + mockStoreState = { + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + runtimePaneTitlesByTabId: {}, + clearTabLaunchAgent: vi.fn(), + clearRuntimePaneTitle: vi.fn(), + setTabLayout: vi.fn(), + updateTabTitle: vi.fn(), + isPtyShutdownPending: vi.fn(() => false), + suppressedPtyExitIds: {} + } + }) + + afterEach(() => { + startedWatcherDisposers.length = 0 + exitCallbacksByPtyId.clear() + vi.clearAllMocks() + // Drain any committed marker a test armed but did not consume. + consumeCommittedPtyShutdownExit(PTY_ID, null) + }) + + it.each([ + ['pending renderer shutdown', () => mockStoreState.isPtyShutdownPending.mockReturnValue(true)], + ['suppressed intentional restart', () => (mockStoreState.suppressedPtyExitIds[PTY_ID] = true)], + ['committed sleep marker', () => markCommittedPtyShutdowns([PTY_ID])] + ] as const)('keeps the tab and layout on a %s exit', (_marker, arm) => { + const entry = startSplitWatchers() + arm() + + exitCallbacksByPtyId.get(PTY_ID)?.(0, { hadPrimary: false }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(mockStoreState.setTabLayout).not.toHaveBeenCalled() + // The buffered exit stays as the restart tombstone. + expect(discardPreHandlerPtyState).not.toHaveBeenCalled() + expect(mockStoreState.clearRuntimePaneTitle).toHaveBeenCalledWith(TAB_ID, 1) + expect(startedWatcherDisposers[0]).toHaveBeenCalled() + expect(startedWatcherDisposers[1]).not.toHaveBeenCalled() + expect(entry.disposersByPtyId.has(PTY_ID)).toBe(false) + // The one-shot committed marker is consumed by the guard, never leaked. + expect(consumeCommittedPtyShutdownExit(PTY_ID, null)).toBe(false) + }) + + it('leaves the committed marker to the primary when one handled the exit', () => { + startSplitWatchers() + markCommittedPtyShutdowns([PTY_ID]) + + exitCallbacksByPtyId.get(PTY_ID)?.(0, { hadPrimary: true }) + + // hadPrimary skips the guard entirely: marker untouched, watcher disposed. + expect(consumeCommittedPtyShutdownExit(PTY_ID, null)).toBe(true) + expect(startedWatcherDisposers[0]).toHaveBeenCalled() + }) + + it('still collapses the dead leaf on an ordinary parked split exit', () => { + startSplitWatchers() + + exitCallbacksByPtyId.get(SECOND_PTY_ID)?.(0, { hadPrimary: false }) + + // No sleep marker: the pre-fix disposition is untouched. + expect(discardPreHandlerPtyState).toHaveBeenCalledWith(SECOND_PTY_ID) + }) + + it('still closes the tab on an ordinary exit of the last parked pane', () => { + const entry = startSplitWatchers() + exitCallbacksByPtyId.get(SECOND_PTY_ID)?.(0, { hadPrimary: false }) + expect(entry.disposersByPtyId.size).toBe(1) + + exitCallbacksByPtyId.get(PTY_ID)?.(0, { hadPrimary: false }) + + // The unarmed committed-marker probe in the guard must not block the close. + expect(closeTerminalTab).toHaveBeenCalledWith( + TAB_ID, + expect.objectContaining({ + hostCloseReason: 'pty-exit', + lifecyclePtyId: PTY_ID + }) + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sole-newborn-exit.test.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sole-newborn-exit.test.ts new file mode 100644 index 00000000000..3dddd996fed --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-sole-newborn-exit.test.ts @@ -0,0 +1,188 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// Why this file: the parked sidecar owns a parked PTY's exit, so it must mirror +// the session observer's sole-newborn guard (pty-exit-hibernate): a sole +// fresh-spawned pane nobody ever typed into keeps its tab on exit. The fact +// travels as the plain-value `untouchedFreshSpawn` capture flag — never a +// session reference. + +const WORKTREE_ID = 'repo::/worktree' +const TAB_ID = 'tab-1' +const PTY_ID = `${WORKTREE_ID}@@session-1` +const SECOND_PTY_ID = `${WORKTREE_ID}@@session-2` +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const SECOND_LEAF_ID = '22222222-2222-4222-8222-222222222222' + +const startedWatcherDisposers: ReturnType[] = [] +vi.mock('./parked-terminal-byte-watcher', () => ({ + startParkedTerminalByteWatcher: () => { + const dispose = vi.fn() + startedWatcherDisposers.push(dispose) + return dispose + } +})) + +type ExitCallback = (code: number, context: { hadPrimary: boolean }) => void +const exitCallbacksByPtyId = new Map() +vi.mock('./pty-dispatcher', () => ({ + subscribeToPtyExit: (ptyId: string, callback: ExitCallback) => { + exitCallbacksByPtyId.set(ptyId, callback) + return vi.fn() + } +})) + +const consumePreHandlerPtyState = vi.fn() +const discardPreHandlerPtyState = vi.fn() +vi.mock('./pty-pre-handler-buffer', () => ({ + consumePreHandlerPtyState: (ptyId: string) => consumePreHandlerPtyState(ptyId), + discardPreHandlerPtyState: (ptyId: string) => discardPreHandlerPtyState(ptyId), + hasPreHandlerPtyExit: () => false +})) + +const closeTerminalTab = vi.fn() +vi.mock('../terminal/terminal-tab-actions', () => ({ + closeTerminalTab: (tabId: string, options?: unknown) => closeTerminalTab(tabId, options) +})) + +type MockStoreState = { + tabsByWorktree: Record + terminalLayoutsByTabId: Record + runtimePaneTitlesByTabId: Record> + clearTabLaunchAgent: ReturnType + clearRuntimePaneTitle: ReturnType + setTabLayout: ReturnType + updateTabTitle: ReturnType + isPtyShutdownPending: ReturnType + suppressedPtyExitIds: Record +} +let mockStoreState: MockStoreState +vi.mock('@/store', () => ({ + useAppStore: { getState: () => mockStoreState } +})) + +import { startParkedPtyWatcher } from './terminal-parked-pty-watcher' +import type { + ParkedTabWatcherEntry, + ParkedTerminalPaneCapture +} from './terminal-parked-watcher-registry' + +function startWatchers(panes: ParkedTerminalPaneCapture[]): ParkedTabWatcherEntry { + const entry: ParkedTabWatcherEntry = { + worktreeId: WORKTREE_ID, + tabPtyId: PTY_ID, + paneIdByPtyId: new Map(), + disposersByPtyId: new Map() + } + const tab = { id: TAB_ID, ptyId: PTY_ID } + for (const pane of panes) { + startParkedPtyWatcher({ + worktreeId: WORKTREE_ID, + tab, + pane, + entry, + restoreTitleOnRegister: false, + restorePolicy: {} + }) + } + return entry +} + +const soleNewbornPane: ParkedTerminalPaneCapture = { + ptyId: PTY_ID, + paneId: 1, + leafId: LEAF_ID, + drivesTabTitle: true, + untouchedFreshSpawn: true +} + +describe('sole-newborn parked exits (sole-owner sidecar)', () => { + beforeEach(() => { + mockStoreState = { + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + runtimePaneTitlesByTabId: {}, + clearTabLaunchAgent: vi.fn(), + clearRuntimePaneTitle: vi.fn(), + setTabLayout: vi.fn(), + updateTabTitle: vi.fn(), + isPtyShutdownPending: vi.fn(() => false), + suppressedPtyExitIds: {} + } + }) + + afterEach(() => { + startedWatcherDisposers.length = 0 + exitCallbacksByPtyId.clear() + vi.clearAllMocks() + }) + + it('keeps the tab when the sole untouched fresh-spawn pane exits while parked', () => { + const entry = startWatchers([soleNewbornPane]) + + exitCallbacksByPtyId.get(PTY_ID)?.(1, { hadPrimary: false }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(mockStoreState.setTabLayout).not.toHaveBeenCalled() + expect(mockStoreState.clearRuntimePaneTitle).toHaveBeenCalledWith(TAB_ID, 1) + expect(startedWatcherDisposers[0]).toHaveBeenCalled() + expect(entry.disposersByPtyId.has(PTY_ID)).toBe(false) + // The exit is consumed (pre-fix primary parity), never left as a buffered + // tombstone — a tombstone would close the tab at reveal via exitedBeforeAttach. + expect(consumePreHandlerPtyState).toHaveBeenCalledWith(PTY_ID) + expect(discardPreHandlerPtyState).not.toHaveBeenCalled() + // The pane-id slot stays, so a watcher sync never re-registers the dead PTY. + expect(entry.paneIdByPtyId.get(PTY_ID)).toBe(1) + }) + + it.each([ + ['no fresh-spawn/never-typed capture', {}], + ['an explicit non-newborn capture', { untouchedFreshSpawn: false }] + ] as const)('still closes the tab with %s', (_case, flag) => { + const { untouchedFreshSpawn: _dropped, ...basePane } = soleNewbornPane + startWatchers([{ ...basePane, ...flag }]) + + exitCallbacksByPtyId.get(PTY_ID)?.(1, { hadPrimary: false }) + + expect(closeTerminalTab).toHaveBeenCalledWith(TAB_ID, expect.anything()) + expect(consumePreHandlerPtyState).not.toHaveBeenCalled() + }) + + it('does not extend the guard to a newborn split sibling — its leaf collapses as today', () => { + startWatchers([ + soleNewbornPane, + { ptyId: SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false } + ]) + + exitCallbacksByPtyId.get(PTY_ID)?.(1, { hadPrimary: false }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + // Split-sibling branch: pre-handler state discarded, not consumed-for-parity. + expect(discardPreHandlerPtyState).toHaveBeenCalledWith(PTY_ID) + expect(consumePreHandlerPtyState).not.toHaveBeenCalled() + }) + + it('leaves disposition to the primary when a mounted pane handled the exit', () => { + const entry = startWatchers([soleNewbornPane]) + + exitCallbacksByPtyId.get(PTY_ID)?.(1, { hadPrimary: true }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(consumePreHandlerPtyState).not.toHaveBeenCalled() + expect(startedWatcherDisposers[0]).toHaveBeenCalled() + expect(entry.disposersByPtyId.has(PTY_ID)).toBe(false) + }) + + it('becomes the sole-newborn guard after a split sibling already collapsed', () => { + const entry = startWatchers([ + soleNewbornPane, + { ptyId: SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false } + ]) + exitCallbacksByPtyId.get(SECOND_PTY_ID)?.(0, { hadPrimary: false }) + expect(entry.disposersByPtyId.size).toBe(1) + + exitCallbacksByPtyId.get(PTY_ID)?.(1, { hadPrimary: false }) + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(consumePreHandlerPtyState).toHaveBeenCalledWith(PTY_ID) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 16c00e6c64e..52a348b627d 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -653,7 +653,10 @@ export function retireMountedTerminalPaneSurface(args: { ) => void syncPanePtyLayoutBinding: (paneId: number, ptyId: string | null) => void clearTabPtyId: (tabId: string, ptyId: string) => void - transport?: { detach?: () => void; destroy?: () => void } + transport?: { + detach?: (options?: { preserveExitObserver?: boolean }) => void + destroy?: () => void + } }): void { args.retireAgentPaneAuthority(args.paneKey, { preserveSleepingAgentSession: true @@ -662,7 +665,10 @@ export function retireMountedTerminalPaneSurface(args: { args.syncPanePtyLayoutBinding(args.paneId, null) args.clearTabPtyId(args.tabId, args.ptyId) } - args.transport?.detach?.() + // preserveExitObserver:false — a retired surface keeps its PTY alive but starts no parked + // watcher, so a preserved observer would pin the disposed pane's xterm for the whole session + // (the entries only die on real PTY exit). The buffered exit drains into a successor mount. + args.transport?.detach?.({ preserveExitObserver: false }) } /** Wires mounted terminal panes to renderer state and terminal event handling. */ @@ -1494,7 +1500,10 @@ export function useTerminalPaneLifecycle({ if (transport && !isRetiredSurface) { if (isDetachedToTab) { // Why: detach hands the PTY to a new tab, so drop renderer listeners without process teardown. - transport.detach?.() + // preserveExitObserver:false for the same reason as unmount — the destination tab's + // registerExit re-owns the exit, and a preserved observer would pin this pane's xterm + // until then (forever if the destination never mounts). + transport.detach?.({ preserveExitObserver: false }) } else { const ptyId = suppressIntentionalPaneCloseExit( transport, @@ -1951,12 +1960,21 @@ export function useTerminalPaneLifecycle({ captureParkedTerminalPaneCandidates( tabId, worktreeId, - manager.getPanes().map((capturedPane) => ({ - ptyId: paneTransports.get(capturedPane.id)?.getPtyId() ?? null, - paneId: capturedPane.id, - leafId: capturedPane.leafId, - drivesTabTitle: manager.getActivePane()?.id === capturedPane.id - })) + manager.getPanes().map((capturedPane) => { + const capturedPtyId = paneTransports.get(capturedPane.id)?.getPtyId() ?? null + const capturedBinding = panePtyBindings.get(capturedPane.id) as + | (IDisposable & { isUntouchedFreshSpawnPty?: (ptyId: string) => boolean }) + | undefined + return { + ptyId: capturedPtyId, + paneId: capturedPane.id, + leafId: capturedPane.leafId, + drivesTabTitle: manager.getActivePane()?.id === capturedPane.id, + untouchedFreshSpawn: + capturedPtyId !== null && + (capturedBinding?.isUntouchedFreshSpawnPty?.(capturedPtyId) ?? false) + } + }) ) for (const transport of paneTransports.values()) { const ptyId = transport.getPtyId() @@ -1969,7 +1987,10 @@ export function useTerminalPaneLifecycle({ }) ) { // Why: tab-move rehome and web-mirror remount unmount a still-live tab; detach preserves the running PTY so the remount reattaches without restarting the shell. - transport.detach?.() + // preserveExitObserver:false — the session-bound exit observer closes over the disposed pane's xterm + // (~14MB/pane pinned in the ptyId-keyed dispatcher maps until exit). Detached exit is owned by the + // parked watcher sidecar; without a watcher it buffers and drains into the next mount's registerExit. + transport.detach?.({ preserveExitObserver: false }) } else { // Why: un-attached transports have no PTY ID; destroy so an in-flight spawn resolves to a killed PTY, not a revived stale binding after unmount. transport.destroy?.()