mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 16:03:26 +00:00
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 <help@stably.ai>
This commit is contained in:
brennanb2025
Orca
@@ -145,6 +145,7 @@ type MockTransport = {
|
||||
sendInputAccepted?: ReturnType<typeof vi.fn>
|
||||
resize: ReturnType<typeof vi.fn>
|
||||
getPtyId: ReturnType<typeof vi.fn>
|
||||
getConnectionId: ReturnType<typeof vi.fn>
|
||||
serializeBuffer?: ReturnType<typeof vi.fn>
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string>) => 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<string>): 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) {
|
||||
|
||||
@@ -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<string>) => 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())
|
||||
})
|
||||
})
|
||||
@@ -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<string>) => 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<string>
|
||||
}): 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<ReconcilableBinding>
|
||||
listSessions: () => Promise<{ id: string; cwd: string; title: string }[]>
|
||||
}): Promise<void> {
|
||||
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)
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<ReconcilableBinding>
|
||||
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<ReconcilableBinding>,
|
||||
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])
|
||||
|
||||
|
||||
Reference in New Issue
Block a user