From 3e29bd68bd25b29fca4aaedc862fd6422deeb11d Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Sat, 27 Jun 2026 12:52:16 -0700 Subject: [PATCH] Recover frozen terminal panes after a backgrounded agent exits When a terminal pane's PTY/daemon session is reaped while its worktree is surface-hidden, the renderer never observes the exit, so the pane stays mounted bound to a dead session: on return it shows a stale frozen frame and silently swallows keyboard input. On visibility resume, reconcile any pane bound to a genuinely-reaped local/daemon session by routing it through the same teardown an observed exit runs. As defense-in-depth, re-check liveness once per resume on the first keystroke. SSH and web-runtime (remote:) panes are deliberately not reconciled (the flat listSessions IPC cannot prove their liveness). Co-authored-by: Orca --- .../terminal-pane/pty-connection.test.ts | 292 ++++++++++++++++++ .../terminal-pane/pty-connection.ts | 97 ++++++ .../terminal-dead-session-reconcile.test.ts | 108 +++++++ .../terminal-dead-session-reconcile.ts | 76 +++++ .../use-terminal-pane-lifecycle.test.ts | 38 +++ .../use-terminal-pane-lifecycle.ts | 37 +++ 6 files changed, 648 insertions(+) create mode 100644 src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index ac1c52a4174..58efa975f9f 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -145,6 +145,7 @@ type MockTransport = { sendInputAccepted?: ReturnType resize: ReturnType getPtyId: ReturnType + getConnectionId: ReturnType serializeBuffer?: ReturnType } @@ -276,6 +277,7 @@ function createMockTransport(initialPtyId: string | null = null): MockTransport sendInput: vi.fn(() => true), resize: vi.fn(() => true), getPtyId: vi.fn(() => ptyId), + getConnectionId: vi.fn(() => null), serializeBuffer: undefined } as MockTransport const sendInput = transport.sendInput as unknown as (data: string) => boolean @@ -580,6 +582,7 @@ describe('connectPanePty', () => { }, pty: { signal: vi.fn(), + listSessions: vi.fn().mockResolvedValue([]), getMainBufferSnapshot: vi.fn().mockResolvedValue(null), getForegroundProcess: vi.fn().mockResolvedValue(null), hasChildProcesses: vi.fn().mockResolvedValue(false), @@ -9478,4 +9481,293 @@ describe('connectPanePty', () => { expect(deps.setCacheTimerStartedAt).toHaveBeenCalledWith(makePaneKey('tab-1', LEAF_1), null) expect(mockStoreState.removeAgentStatus).not.toHaveBeenCalled() }) + + describe('reconcileIfSessionDead', () => { + it('closes a split pane bound to a dead local session (same teardown as onExit)', async () => { + const { connectPanePty } = await import('./pty-connection') + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + const transport = createMockTransport('pty-pane-2') + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + 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')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + // Why: clear the freshly-split early-return guard so onExit reaches the + // close branch, matching an established split pane the user has used. + capturedDataCallback.current?.('shell prompt') + + binding.reconcileIfSessionDead(new Set(['pty-pane-1'])) + + expect(manager.closePane).toHaveBeenCalledWith(2) + }) + + it('routes the last pane through onPtyExitRef when its session is dead', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-pane-1') + transportFactoryQueue.push(transport) + const manager = createManager(1) + const deps = createDeps() + + const binding = connectPanePty(createPane(1) as never, manager as never, deps as never) + + // The last pane reattaches to the tab's persisted ptyId ('tab-pty'). + binding.reconcileIfSessionDead(new Set(['some-other-live'])) + + expect(deps.onPtyExitRef.current).toHaveBeenCalledWith('tab-pty') + expect(manager.closePane).not.toHaveBeenCalled() + }) + + it('is a no-op when the bound session is still live', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('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')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + + binding.reconcileIfSessionDead(new Set(['pty-pane-1', 'pty-pane-2'])) + + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('is a no-op for remote: web-runtime ids', async () => { + const { connectPanePty } = await import('./pty-connection') + enableActiveRuntimeEnvironment('env-1') + const transport = createMockTransport('remote:env-1:pane-2') + transport.getConnectionId.mockReturnValue(null) + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + + binding.reconcileIfSessionDead(new Set()) + + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('is a no-op for SSH/non-local ids (non-null connectionId)', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-pane-2') + transport.getConnectionId.mockReturnValue('ssh-target-1') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + + binding.reconcileIfSessionDead(new Set()) + + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('respects suppression: a suppressed dead session keeps the pane mounted', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-pane-2') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + consumeSuppressedPtyExit: vi.fn(() => true), + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + + binding.reconcileIfSessionDead(new Set(['pty-pane-1'])) + + expect(deps.consumeSuppressedPtyExit).toHaveBeenCalledWith('pty-pane-2') + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('does not act on a stale id after a reattach changed the bound id', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('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')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + + // Reattach to a new live session between snapshot and apply: the snapshot + // marked the OLD id dead, but getPtyId now returns the new (live) id. + transport.getPtyId.mockReturnValue('pty-pane-2-reattached') + binding.reconcileIfSessionDead(new Set(['pty-pane-1', 'pty-pane-2-reattached'])) + + expect(manager.closePane).not.toHaveBeenCalled() + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + }) + + it('closes a split pane exactly once across reconcile + a racing real exit', async () => { + const { connectPanePty } = await import('./pty-connection') + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + const transport = createMockTransport('pty-pane-2') + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + 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')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + capturedDataCallback.current?.('shell prompt') + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string) => void) + | undefined + expect(onPtyExit).toBeTypeOf('function') + + binding.reconcileIfSessionDead(new Set(['pty-pane-1'])) + // A racing real/synthetic pty:exit for the SAME id must not close twice. + onPtyExit?.('pty-pane-2') + + expect(manager.closePane).toHaveBeenCalledTimes(1) + expect(manager.closePane).toHaveBeenCalledWith(2) + }) + + it('closes a genuinely-dead non-suppressed pane once (not misread as suppressed)', async () => { + const { connectPanePty } = await import('./pty-connection') + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + const transport = createMockTransport('pty-pane-2') + transport.connect.mockImplementation( + async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-pane-2' + } + ) + transportFactoryQueue.push(transport) + const manager = createManager(2) + // consumeSuppressedPtyExit always returns false for this genuinely-dead id. + const deps = createDeps({ + restoredLeafId: LEAF_2, + consumeSuppressedPtyExit: vi.fn(() => false), + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + + const binding = connectPanePty(createPane(2) as never, manager as never, deps as never) + capturedDataCallback.current?.('shell prompt') + const onPtyExit = createdTransportOptions[0]?.onPtyExit as + | ((ptyId: string) => void) + | undefined + + binding.reconcileIfSessionDead(new Set(['pty-pane-1'])) + onPtyExit?.('pty-pane-2') + + // Observable outcome: single close, pane was treated as dead (closed). + expect(manager.closePane).toHaveBeenCalledTimes(1) + expect(manager.closePane).toHaveBeenCalledWith(2) + }) + }) + + describe('input liveness re-check IPC gating (perf)', () => { + // Why (perf regression guard): listSessions() is a renderer→main→daemon + // round-trip. The input-driven liveness re-check must fire at most once per + // resume window, never once per keystroke, or every healthy local pane puts + // a process-enumeration round-trip on the typing hot path. + async function connectActivePaneWithInput(): Promise<{ + binding: { noteVisibilityResume: () => void } + typeKeystroke: (data?: string) => void + }> { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('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')]]) } + }) + const pane = createPane(2) + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + return { + binding, + // Drives the real xterm onData (terminal input) handler. + typeKeystroke: (data = 'a') => sendTerminalInputThroughPane(pane, data) + } + } + + it('fires listSessions at most once across many keystrokes in one resume window', async () => { + const listSessions = vi.mocked(window.api.pty.listSessions) + listSessions.mockClear() + const { typeKeystroke } = await connectActivePaneWithInput() + + for (let i = 0; i < 25; i++) { + typeKeystroke('x') + } + + expect(listSessions).toHaveBeenCalledTimes(1) + }) + + it('re-arms one re-check after a visibility resume', async () => { + const listSessions = vi.mocked(window.api.pty.listSessions) + listSessions.mockClear() + const { binding, typeKeystroke } = await connectActivePaneWithInput() + + typeKeystroke('a') + typeKeystroke('b') + expect(listSessions).toHaveBeenCalledTimes(1) + + // Resume re-arms exactly one more re-check. + binding.noteVisibilityResume() + typeKeystroke('c') + typeKeystroke('d') + expect(listSessions).toHaveBeenCalledTimes(2) + }) + + it('never fires listSessions for a remote: web-runtime pane (liveness owned by host snapshot)', async () => { + const listSessions = vi.mocked(window.api.pty.listSessions) + listSessions.mockClear() + const { connectPanePty } = await import('./pty-connection') + // Remote panes report a null connectionId but a remote:-prefixed ptyId, so + // the SSH/connectionId guard alone would not exclude them — the remote + // prefix guard must. + const transport = createMockTransport('remote:env-1@@terminal-2') + transport.getConnectionId.mockReturnValue(null) + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + const pane = createPane(2) + connectPanePty(pane as never, manager as never, deps as never) + + sendTerminalInputThroughPane(pane, 'x') + sendTerminalInputThroughPane(pane, 'y') + + expect(listSessions).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index e6930cfbd9d..f1f57e49ec5 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -20,6 +20,7 @@ import { hasCachedWindowsTerminalCapabilities } from '@/lib/windows-terminal-capabilities' import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding' +import { shouldReconcileDeadSession } from './terminal-dead-session-reconcile' import type { PtyConnectionDeps } from './pty-connection-types' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' @@ -506,6 +507,8 @@ let inactiveForegroundImmediateBudgetWindowStart = 0 type PanePtyBinding = IDisposable & { syncProcessTracking: () => void + noteVisibilityResume: () => void + reconcileIfSessionDead: (liveSessionIds: Set) => void } function isAgentTaskCompleteNotificationEnabled(): boolean { @@ -1293,7 +1296,17 @@ export function connectPanePty( } }) + // Why: the transport's own exit handler (pty-transport.ts) normally makes + // onExit run-at-most-once by clearing connected/ptyId + unregistering BEFORE + // calling it. reconcileIfSessionDead drives onExit directly (bypassing that), + // so this binding-local one-shot guards the body so reconcile and any racing + // real/synthetic pty:exit for the same id close the pane exactly once. + let onExitHasRun = false const onExit = (ptyId: string): void => { + if (onExitHasRun) { + return + } + onExitHasRun = true agentCompletionCoordinator.dispose() clearPanePtyFitBinding() const isSuppressedExit = deps.consumeSuppressedPtyExit(ptyId) @@ -1925,6 +1938,11 @@ export function connectPanePty( clearPendingTerminalInputIntent() return } + // Why (Defect #2): a keystroke against a pane whose daemon session was + // reaped while hidden is silently dropped — sendInput still returns true. + // Kick off a fire-and-forget liveness re-check so the dead pane is cleaned + // up without relying on (the never-occurring) sendInput false return. + recheckLivenessAfterInput() const intent = pendingTerminalInputIntent // Why: real xterm can deliver the terminal byte even when our DOM keydown // listener missed the press. Exact Ctrl+C/Escape bytes are still safe to @@ -4134,10 +4152,89 @@ export function connectPanePty( scheduleRuntimeGraphSync() }) + // Why: on visibility resume a pane may still be bound to a daemon session + // reaped while hidden (the missed-exit defect). Route it through the SAME + // teardown a real onExit runs. Re-validate identity at apply time so a + // reattach racing the listSessions snapshot is never clobbered, and respect + // the remote/SSH guards. Suppression semantics come for free via onExit + // (which consults consumeSuppressedPtyExit) plus the one-shot guard above. + const reconcileIfSessionDead = (liveSessionIds: Set): void => { + if (disposed || onExitHasRun) { + return + } + const currentPtyId = transport.getPtyId() + if ( + !currentPtyId || + !shouldReconcileDeadSession({ + ptyId: currentPtyId, + connectionId: transport.getConnectionId?.(), + liveSessionIds + }) + ) { + return + } + onExit(currentPtyId) + } + + // Why (perf): the only moment a daemon session can be reaped behind the + // renderer's back is while the pane was surface-hidden. So the input-driven + // re-check is only useful in the window right after a resume — once it (or + // the resume pass) has confirmed liveness for this resume, re-polling on + // every subsequent keystroke is pure waste: listSessions() is a + // renderer→main→daemon round-trip (DaemonPtyAdapter.listProcesses requests + // `listSessions` from the daemon subprocess), so an ungated per-keystroke + // re-check would put a process-enumeration round-trip on the typing hot path + // for every healthy local pane. Fire at most ONCE per resume window; reset + // on the next hide→show. This preserves the "reduces not eliminates the + // first-keystroke drop" intent — the first keystroke after a resume still + // triggers exactly one re-check. + let livenessRecheckFiredSinceResume = false + + // Why (Defect #2 defense-in-depth): in the broken state sendInput returns + // true (connected/ptyId still set) so the dropped keystroke is invisible to + // the renderer. A fire-and-forget liveness re-check on the FIRST input after + // a resume cleans the pane up promptly instead of waiting for the resume + // pass alone. It REDUCES but cannot eliminate the first-keystroke drop (that + // byte is already gone daemon-side). + const recheckLivenessAfterInput = (): void => { + if (disposed || onExitHasRun || livenessRecheckFiredSinceResume) { + return + } + const currentPtyId = transport.getPtyId() + const currentConnectionId = transport.getConnectionId?.() + if ( + !currentPtyId || + // Why: `remote:` web-runtime liveness is owned by the host snapshot, not + // listSessions; skip here so a remote pane's keystrokes never put a local + // daemon round-trip on the typing hot path (reconcile would no-op anyway). + isRemoteRuntimePtyId(currentPtyId) || + (currentConnectionId !== null && currentConnectionId !== undefined) + ) { + return + } + // Why: set BEFORE the IPC so concurrent keystrokes coalesce to one in-flight + // request rather than fanning out a round-trip per byte typed. + livenessRecheckFiredSinceResume = true + void window.api.pty + .listSessions() + .then((sessions) => { + reconcileIfSessionDead(new Set(sessions.map((session) => session.id))) + }) + // Why: a rejected listing is "unknown" — never close a pane on it. + .catch(() => {}) + } + return { syncProcessTracking() { agentCompletionCoordinator.startProcessTracking() }, + // Why: re-arm the once-per-resume input re-check when the pane becomes + // visible again. Called from the lifecycle visibility effect; the gate + // keeps the typing hot path off the listSessions IPC between resumes. + noteVisibilityResume() { + livenessRecheckFiredSinceResume = false + }, + reconcileIfSessionDead, dispose() { disposed = true if (terminalKeyTargetSupportsEvents) { diff --git a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts new file mode 100644 index 00000000000..215b833086d --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it, vi } from 'vitest' +import { + reconcileDeadSessions, + shouldReconcileDeadSession +} from './terminal-dead-session-reconcile' + +describe('shouldReconcileDeadSession', () => { + it('reconciles a local, non-remote id genuinely absent from the live set', () => { + expect( + shouldReconcileDeadSession({ + ptyId: 'wt@@dead', + connectionId: null, + liveSessionIds: new Set(['wt@@alive']) + }) + ).toBe(true) + }) + + it('does not reconcile when the bound id is still live', () => { + expect( + shouldReconcileDeadSession({ + ptyId: 'wt@@alive', + connectionId: null, + liveSessionIds: new Set(['wt@@alive']) + }) + ).toBe(false) + }) + + it('skips a mid-spawn pane with no bound id', () => { + expect( + shouldReconcileDeadSession({ + ptyId: null, + connectionId: null, + liveSessionIds: new Set() + }) + ).toBe(false) + }) + + it('skips remote: web-runtime ids', () => { + expect( + shouldReconcileDeadSession({ + ptyId: 'remote:env-1:abc', + connectionId: null, + liveSessionIds: new Set() + }) + ).toBe(false) + }) + + it('skips SSH/non-local ids (non-null connectionId)', () => { + expect( + shouldReconcileDeadSession({ + ptyId: 'wt@@ssh-dead', + connectionId: 'ssh-target-1', + liveSessionIds: new Set(['wt@@alive']) + }) + ).toBe(false) + }) + + it('reconciles a genuinely-absent local id even when the live set is empty (no zero-total skip)', () => { + expect( + shouldReconcileDeadSession({ + ptyId: 'wt@@dead', + connectionId: null, + liveSessionIds: new Set() + }) + ).toBe(true) + }) +}) + +describe('reconcileDeadSessions', () => { + function createBinding() { + return { reconcileIfSessionDead: vi.fn<(liveSessionIds: Set) => void>() } + } + + it('invokes each binding with the resolved live-session id set', async () => { + const bindingA = createBinding() + const bindingB = createBinding() + await reconcileDeadSessions({ + bindings: [bindingA, bindingB], + listSessions: async () => [ + { id: 'wt@@alive', cwd: '/a', title: 'a' }, + { id: 'wt@@other', cwd: '/b', title: 'b' } + ] + }) + const expectedSet = new Set(['wt@@alive', 'wt@@other']) + expect(bindingA.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet) + expect(bindingB.reconcileIfSessionDead).toHaveBeenCalledWith(expectedSet) + }) + + it('treats a rejected listSessions as "unknown" and reconciles nothing', async () => { + const binding = createBinding() + await reconcileDeadSessions({ + bindings: [binding], + listSessions: async () => { + throw new Error('IPC failed') + } + }) + expect(binding.reconcileIfSessionDead).not.toHaveBeenCalled() + }) + + it('treats a resolved empty list as authoritative (still reconciles)', async () => { + const binding = createBinding() + await reconcileDeadSessions({ + bindings: [binding], + listSessions: async () => [] + }) + expect(binding.reconcileIfSessionDead).toHaveBeenCalledWith(new Set()) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts new file mode 100644 index 00000000000..6c479883150 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts @@ -0,0 +1,76 @@ +// Renderer-side liveness reconciliation for terminal panes whose daemon PTY +// session was reaped while the worktree was surface-hidden. On visibility +// resume the missed `pty:exit` left the pane mounted bound to a dead session; +// this routes such panes through the same exit teardown an observed exit runs. +// See design-docs/terminal-dead-pane-on-bg-exit.md. + +const REMOTE_PTY_ID_PREFIX = 'remote:' + +/** + * A pane binding that exposes its bound transport identity plus a reconcile + * hook. Kept structural so the lifecycle fan-out can call it without importing + * the full `PanePtyBinding` shape from pty-connection. + */ +export type ReconcilableBinding = { + reconcileIfSessionDead?: (liveSessionIds: Set) => void +} + +/** + * PURE decision: should the pane bound to `ptyId` be reconciled (torn down) + * given the resolved set of live session ids? + * + * Reconcile ONLY when every guard passes: + * - `ptyId` is non-null (a mid-spawn pane has no id to prove dead). + * - `ptyId` is not `remote:`-prefixed (web-runtime liveness is owned by the + * host snapshot, not `listSessions`). + * - `connectionId === null` — the id is local/daemon-backed. SSH-backed ids + * (non-null connectionId) are deferred: the flat `listSessions` shape cannot + * authoritatively prove an SSH session gone (see design Section 1). + * - the id is genuinely absent from the resolved live set. + */ +export function shouldReconcileDeadSession(args: { + ptyId: string | null | undefined + connectionId: string | null | undefined + liveSessionIds: Set +}): boolean { + const { ptyId, connectionId, liveSessionIds } = args + if (ptyId === null || ptyId === undefined) { + return false + } + if (ptyId.startsWith(REMOTE_PTY_ID_PREFIX)) { + return false + } + // Why: only local/daemon-backed ids (connectionId null/undefined) are + // reconcilable; a non-null connectionId means SSH, which is deferred. + if (connectionId !== null && connectionId !== undefined) { + return false + } + return !liveSessionIds.has(ptyId) +} + +/** + * Thin orchestration: fetch the live session listing once and invoke each + * binding's `reconcileIfSessionDead` with the resolved set. + * + * Why a rejected `listSessions()` is swallowed (no reconcile): the local + * provider listing is NOT `.catch`-wrapped in main, so a rejection means the + * listing is unknown — closing a pane on an IPC/listing failure is unacceptable. + * A RESOLVED list (even empty `[]`) is AUTHORITATIVE for local ids: a local id + * absent from it is genuinely reaped, so there is no zero-total skip. + */ +export async function reconcileDeadSessions(args: { + bindings: Iterable + listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> +}): Promise { + let sessions: { id: string }[] + try { + sessions = await args.listSessions() + } catch { + // Why: a rejected listing is "unknown" — never close a pane on it. + return + } + const liveSessionIds = new Set(sessions.map((session) => session.id)) + for (const binding of args.bindings) { + binding.reconcileIfSessionDead?.(liveSessionIds) + } +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index e5aaba85a86..049312db69e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import { mapRestoredPaneTitlesByPaneId, + scheduleVisibilityReconcilePass, shouldDetachPaneTransportOnUnmount, splitPaneWithOneShotStartup, suppressIntentionalPaneCloseExit @@ -182,3 +183,40 @@ describe('suppressIntentionalPaneCloseExit', () => { expect(suppressPtyExit).not.toHaveBeenCalled() }) }) + +describe('scheduleVisibilityReconcilePass', () => { + it('schedules a reconcile pass over the bindings when becoming visible', async () => { + const reconcileIfSessionDead = vi.fn() + const listSessions = vi + .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() + .mockResolvedValue([{ id: 'live-1', cwd: '/a', title: 'a' }]) + + const scheduled = scheduleVisibilityReconcilePass({ + isVisible: true, + bindings: [{ reconcileIfSessionDead }], + listSessions + }) + + expect(scheduled).toBe(true) + // Fire-and-forget: let the async listSessions resolve before asserting. + await Promise.resolve() + await Promise.resolve() + expect(listSessions).toHaveBeenCalledTimes(1) + expect(reconcileIfSessionDead).toHaveBeenCalledWith(new Set(['live-1'])) + }) + + it('self-gates: does not schedule when hiding (isVisible false)', () => { + const listSessions = vi + .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() + .mockResolvedValue([]) + + const scheduled = scheduleVisibilityReconcilePass({ + isVisible: false, + bindings: [{ reconcileIfSessionDead: vi.fn() }], + listSessions + }) + + expect(scheduled).toBe(false) + expect(listSessions).not.toHaveBeenCalled() + }) +}) 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 fe67c1ad83a..0d44bce6518 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 @@ -70,6 +70,7 @@ import { installMouseHideWhileTyping } from './mouse-hide-while-typing' import type { EffectiveMacOptionAsAlt } from '@/lib/keyboard-layout/detect-option-as-alt' import { resolveEffectiveTerminalAppearance } from '@/lib/terminal-theme' import { connectPanePty } from './pty-connection' +import { reconcileDeadSessions, type ReconcilableBinding } from './terminal-dead-session-reconcile' import type { PtyTransport } from './pty-transport' import { getRemoteRuntimePtyEnvironmentId } from '@/runtime/runtime-terminal-stream' import { getConnectionId } from '@/lib/connection-context' @@ -361,6 +362,26 @@ export function shouldDetachPaneTransportOnUnmount(args: { ) } +/** + * Self-gating dead-session reconcile pass scheduled from the isVisible effect. + * Why self-gate: the effect fires on BOTH isVisible true and false, but we only + * reconcile on resume (becoming visible), never on hide. Returns true when the + * pass was scheduled so the resume-unit test can assert the gate. + */ +export function scheduleVisibilityReconcilePass(args: { + isVisible: boolean + bindings: Iterable + listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> +}): boolean { + if (!args.isVisible) { + return false + } + // Why: fire-and-forget so the async listSessions IPC never blocks the + // synchronous WebGL/fit resume work the user sees first. + void reconcileDeadSessions({ bindings: args.bindings, listSessions: args.listSessions }) + return true +} + export function useTerminalPaneLifecycle({ tabId, worktreeId, @@ -1504,9 +1525,25 @@ export function useTerminalPaneLifecycle({ for (const panePtyBinding of panePtyBindingsRef.current.values()) { const bindingWithVisibility = panePtyBinding as IDisposable & { syncProcessTracking?: () => void + noteVisibilityResume?: () => void } bindingWithVisibility.syncProcessTracking?.() + // Why: re-arm the once-per-resume input liveness re-check so the typing + // hot path stays off the listSessions IPC between resumes (the re-check + // is only useful right after a hidden→visible flip). + if (isVisible) { + bindingWithVisibility.noteVisibilityResume?.() + } } + // Why: the reconcile pass self-gates on becoming visible (resume) — the + // effect also fires on hide — and runs fire-and-forget alongside + // syncProcessTracking. reconcileDeadSessions re-validates identity at apply + // time so a racing reattach is not clobbered. + scheduleVisibilityReconcilePass({ + isVisible, + bindings: panePtyBindingsRef.current.values() as Iterable, + listSessions: () => window.api.pty.listSessions() + }) // eslint-disable-next-line react-hooks/exhaustive-deps -- Why: visibility flips must refresh existing PTY process tracking even though the ref object identity is stable. }, [isVisible, isVisibleRef, panePtyBindingsRef])