fix(terminal): bind recovery to xterm lifecycle

This commit is contained in:
Brennan Benson
2026-07-14 03:14:36 -07:00
parent 93a41314b9
commit 2d56ebee77
8 changed files with 221 additions and 26 deletions
@@ -863,6 +863,7 @@ describe('connectPanePty', () => {
ackColdRestore: vi.fn(),
onClearBufferRequest: vi.fn(() => vi.fn()),
onSerializeBufferRequest: vi.fn(() => vi.fn()),
sendSerializedBuffer: vi.fn(),
declarePendingPaneSerializer: vi.fn().mockResolvedValue(1),
settlePaneSerializer: vi.fn().mockResolvedValue(undefined),
clearPendingPaneSerializer: vi.fn().mockResolvedValue(undefined)
@@ -5637,7 +5638,7 @@ describe('connectPanePty', () => {
expect(transport.resize).toHaveBeenLastCalledWith(65, 63)
})
it('adopts a still-live background PTY via attach instead of reattaching when an eager buffer exists', async () => {
it('adopts a live eager PTY and withholds snapshots after its renderer dies', async () => {
// Why: background automation tabs spawn the agent PTY eagerly and register an
// eager buffer, then never mount until opened. On first mount the restored
// ptyId equals the tab ptyId, so without this guard the pane mis-routes into
@@ -5681,6 +5682,19 @@ describe('connectPanePty', () => {
)
const { hasPtySerializer } = await import('./pty-buffer-serializer')
expect(hasPtySerializer(eagerPtyId)).toBe(true)
const serializeRequestHandler = (
window.api.pty.onSerializeBufferRequest as unknown as {
mock: { calls: [[(request: { requestId: string; ptyId: string }) => void]] }
}
).mock.calls[0]?.[0]
const { notifyUndeliverableWrite } =
await import('@/lib/pane-manager/terminal-write-pipeline-health')
notifyUndeliverableWrite(pane.terminal, 'replay-wedged')
serializeRequestHandler?.({ requestId: 'dead-renderer', ptyId: eagerPtyId })
await flushAsyncTicks()
expect(window.api.pty.sendSerializedBuffer).toHaveBeenCalledWith('dead-renderer', null)
})
it('does not adopt another tab live eager PTY from a stale restored leaf binding', async () => {
@@ -56,6 +56,7 @@ import {
} from '@/lib/pane-manager/terminal-write-pipeline-health'
import {
captureTerminalPaneRecoveryGeneration,
registerTerminalPaneRecoveryInstance,
requestTerminalPaneRecovery
} from './terminal-pane-recovery'
import {
@@ -954,6 +955,7 @@ export function connectPanePty(
// Why: recovery ownership belongs to this xterm instance. A request that
// settles after remount must not remount its already-replaced successor.
const terminalRecoveryGeneration = captureTerminalPaneRecoveryGeneration(deps.tabId)
const terminalRecoveryInstance = registerTerminalPaneRecoveryInstance(deps.tabId)
exposeE2eTerminalPtyOutputDebug()
let disposed = false
let connectFrame: number | null = null
@@ -3375,6 +3377,7 @@ export function connectPanePty(
ptyId: undeliverablePtyId,
reason: 'input-undeliverable',
terminalRecoveryGeneration,
terminalRecoveryInstanceId: terminalRecoveryInstance.id,
// Why: pty:hasPty answers null for ids the local registry doesn't own,
// and a disconnected remote pane would otherwise remount-churn on every
// cooldown window while typing. Local panes keep the lenient gate.
@@ -3397,7 +3400,8 @@ export function connectPanePty(
tabId: deps.tabId,
ptyId: transport.getPtyId() ?? storePtyId,
reason,
terminalRecoveryGeneration
terminalRecoveryGeneration,
terminalRecoveryInstanceId: terminalRecoveryInstance.id
})
}
)
@@ -3945,7 +3949,15 @@ export function connectPanePty(
ptyId,
async (opts) => {
try {
if (isTerminalWritePipelineCertifiedDead(pane.terminal)) {
return null
}
await waitForTerminalOutputParsed(pane.terminal)
// Certification can land while the serializer waits for an older
// write; never publish a fossil frame from a dead renderer.
if (isTerminalWritePipelineCertifiedDead(pane.terminal)) {
return null
}
// Why: alt-screen TUIs (vim, claude-code) hold transient state in
// the alternate screen. The hydration path requests
// altScreenForcesZeroRows so normal-buffer scrollback isn't bled
@@ -6179,7 +6191,8 @@ export function connectPanePty(
tabId: deps.tabId,
ptyId: transport.getPtyId() ?? storePtyId,
reason: 'restore-blocked',
terminalRecoveryGeneration
terminalRecoveryGeneration,
terminalRecoveryInstanceId: terminalRecoveryInstance.id
})
}
return false
@@ -7544,6 +7557,9 @@ export function connectPanePty(
reconcileIfSessionMissing,
dispose() {
disposed = true
// A normal park/reconnect/remount does not advance the recovery epoch;
// invalidate this concrete xterm so its delayed retry cannot hit the next.
terminalRecoveryInstance.unregister()
unregisterUndeliverableWriteHandler()
// Why: the post-spawn reconcile polls across frames; cancel its pending
// rAF so a torn-down pane cannot keep fitting/resizing after disposal.
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
_resetTerminalPaneRecoveryForTests,
captureTerminalPaneRecoveryGeneration,
registerTerminalPaneRecoveryInstance,
requestTerminalPaneRecovery
} from './terminal-pane-recovery'
@@ -237,6 +238,69 @@ describe('requestTerminalPaneRecovery', () => {
expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(2)
})
it('cancels a retry when a non-recovery lifecycle replaces its xterm', async () => {
vi.useFakeTimers()
vi.setSystemTime(0)
const originalInstance = registerTerminalPaneRecoveryInstance('tab-1')
await requestTerminalPaneRecovery({
tabId: 'tab-1',
ptyId: 'pty-1',
reason: 'write-stalled',
terminalRecoveryGeneration: captureTerminalPaneRecoveryGeneration('tab-1'),
terminalRecoveryInstanceId: originalInstance.id
})
originalInstance.unregister()
const wedgedReplacement = registerTerminalPaneRecoveryInstance('tab-1')
await requestTerminalPaneRecovery({
tabId: 'tab-1',
ptyId: 'pty-1',
reason: 'replay-wedged',
terminalRecoveryGeneration: captureTerminalPaneRecoveryGeneration('tab-1'),
terminalRecoveryInstanceId: wedgedReplacement.id
})
expect(vi.getTimerCount()).toBe(1)
// Cold parking, SSH reconnect, and ordinary remounts dispose the binding
// without changing the recovery epoch; disposal owns timer invalidation.
wedgedReplacement.unregister()
const healthySuccessor = registerTerminalPaneRecoveryInstance('tab-1')
expect(vi.getTimerCount()).toBe(0)
await vi.advanceTimersByTimeAsync(600_000)
expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(1)
healthySuccessor.unregister()
})
it('keeps a sibling pane retry when the first requesting split is disposed', async () => {
vi.useFakeTimers()
vi.setSystemTime(0)
await requestTerminalPaneRecovery({
tabId: 'tab-1',
ptyId: 'pty-1',
reason: 'write-stalled'
})
const recoveryGeneration = captureTerminalPaneRecoveryGeneration('tab-1')
const firstSplit = registerTerminalPaneRecoveryInstance('tab-1')
const secondSplit = registerTerminalPaneRecoveryInstance('tab-1')
for (const instance of [firstSplit, secondSplit]) {
await requestTerminalPaneRecovery({
tabId: 'tab-1',
ptyId: `pty-${instance.id}`,
reason: 'replay-wedged',
terminalRecoveryGeneration: recoveryGeneration,
terminalRecoveryInstanceId: instance.id
})
}
firstSplit.unregister()
expect(vi.getTimerCount()).toBe(1)
await vi.advanceTimersByTimeAsync(15_000)
expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(2)
secondSplit.unregister()
})
it('budgets tabs independently', async () => {
expect(
await requestTerminalPaneRecovery({ tabId: 'tab-1', ptyId: 'pty-1', reason: 'write-stalled' })
@@ -23,9 +23,12 @@ type RecoveryRequest = {
tabId: string
ptyId: string | null
reason: TerminalPaneRecoveryReason
/** Identifies the xterm generation making the request. A replaced xterm
* must not schedule or complete recovery for its successor. */
/** Identifies the tab recovery epoch making the request. A successful
* recovery must immediately invalidate every pre-remount request. */
terminalRecoveryGeneration?: number
/** Identifies the concrete mounted xterm making the request. Disposal
* invalidates delayed work even when the tab's recovery epoch is unchanged. */
terminalRecoveryInstanceId?: number
/** Remote panes (runtime mirrors, app-SSH) must prove the PTY alive before
* an input-undeliverable remount: pty:hasPty answers null for ids the local
* registry doesn't own, and treating null as "proceed" would let a
@@ -46,7 +49,15 @@ const RECOVERY_COOLDOWN_MS = 15_000
const recoveryTimestampsByTabId = new Map<string, number[]>()
const recoveryGenerationByTabId = new Map<string, number>()
const pendingRetryTimerByTabId = new Map<string, ReturnType<typeof setTimeout>>()
const activeTerminalRecoveryInstanceIds = new Set<number>()
const pendingRetryByTabId = new Map<
string,
{
timer: ReturnType<typeof setTimeout>
requestsByInstanceId: Map<number | undefined, RecoveryRequest>
}
>()
let nextTerminalRecoveryInstanceId = 0
type RecoveryBudget =
| { allowed: true }
@@ -81,38 +92,73 @@ export function captureTerminalPaneRecoveryGeneration(tabId: string): number {
return recoveryGenerationByTabId.get(tabId) ?? 0
}
function requestBelongsToCurrentRecoveryGeneration(request: RecoveryRequest): boolean {
export function registerTerminalPaneRecoveryInstance(tabId: string): {
id: number
unregister: () => void
} {
const id = ++nextTerminalRecoveryInstanceId
activeTerminalRecoveryInstanceIds.add(id)
return {
id,
unregister: () => {
activeTerminalRecoveryInstanceIds.delete(id)
const pendingRetry = pendingRetryByTabId.get(tabId)
pendingRetry?.requestsByInstanceId.delete(id)
if (pendingRetry?.requestsByInstanceId.size === 0) {
cancelPendingRecoveryRetry(tabId)
}
}
}
}
function isCurrentTerminalRecoveryRequest(request: RecoveryRequest): boolean {
return (
request.terminalRecoveryGeneration === undefined ||
request.terminalRecoveryGeneration === captureTerminalPaneRecoveryGeneration(request.tabId)
(request.terminalRecoveryGeneration === undefined ||
request.terminalRecoveryGeneration ===
captureTerminalPaneRecoveryGeneration(request.tabId)) &&
(request.terminalRecoveryInstanceId === undefined ||
activeTerminalRecoveryInstanceIds.has(request.terminalRecoveryInstanceId))
)
}
function scheduleRecoveryRetry(request: RecoveryRequest, delayMs: number): void {
if (!requestBelongsToCurrentRecoveryGeneration(request)) {
if (!isCurrentTerminalRecoveryRequest(request)) {
return
}
if (pendingRetryTimerByTabId.has(request.tabId)) {
const pendingRetry = pendingRetryByTabId.get(request.tabId)
if (pendingRetry) {
// Multiple split panes share a tab-wide remount. Keep one request per
// concrete xterm so disposing one pane cannot cancel a sibling's heal.
pendingRetry.requestsByInstanceId.set(request.terminalRecoveryInstanceId, request)
return
}
const requestsByInstanceId = new Map<number | undefined, RecoveryRequest>([
[request.terminalRecoveryInstanceId, request]
])
const timer = setTimeout(
() => {
pendingRetryTimerByTabId.delete(request.tabId)
if (!requestBelongsToCurrentRecoveryGeneration(request)) {
pendingRetryByTabId.delete(request.tabId)
const currentRequest = [...requestsByInstanceId.values()].find(
isCurrentTerminalRecoveryRequest
)
if (!currentRequest) {
return
}
void requestTerminalPaneRecovery(request)
void requestTerminalPaneRecovery(currentRequest)
},
Math.max(delayMs, 1_000)
)
pendingRetryTimerByTabId.set(request.tabId, timer)
pendingRetryByTabId.set(request.tabId, {
timer,
requestsByInstanceId
})
}
function cancelPendingRecoveryRetry(tabId: string): void {
const timer = pendingRetryTimerByTabId.get(tabId)
if (timer !== undefined) {
clearTimeout(timer)
pendingRetryTimerByTabId.delete(tabId)
const pendingRetry = pendingRetryByTabId.get(tabId)
if (pendingRetry !== undefined) {
clearTimeout(pendingRetry.timer)
pendingRetryByTabId.delete(tabId)
}
}
@@ -125,7 +171,7 @@ function cancelPendingRecoveryRetry(tabId: string): void {
* exited"), and remounting there would race it.
*/
export async function requestTerminalPaneRecovery(request: RecoveryRequest): Promise<boolean> {
if (!requestBelongsToCurrentRecoveryGeneration(request)) {
if (!isCurrentTerminalRecoveryRequest(request)) {
return false
}
const budget = recoveryBudget(request.tabId, Date.now())
@@ -160,7 +206,7 @@ export async function requestTerminalPaneRecovery(request: RecoveryRequest): Pro
}
// Re-check the budget across the await: a concurrent detector may have
// already consumed it for this tab.
if (!requestBelongsToCurrentRecoveryGeneration(request)) {
if (!isCurrentTerminalRecoveryRequest(request)) {
return false
}
const recheck = recoveryBudget(request.tabId, Date.now())
@@ -223,8 +269,10 @@ export async function requestTerminalPaneRecovery(request: RecoveryRequest): Pro
export function _resetTerminalPaneRecoveryForTests(): void {
recoveryTimestampsByTabId.clear()
recoveryGenerationByTabId.clear()
for (const timer of pendingRetryTimerByTabId.values()) {
clearTimeout(timer)
activeTerminalRecoveryInstanceIds.clear()
nextTerminalRecoveryInstanceId = 0
for (const pendingRetry of pendingRetryByTabId.values()) {
clearTimeout(pendingRetry.timer)
}
pendingRetryTimerByTabId.clear()
pendingRetryByTabId.clear()
}
@@ -8,11 +8,29 @@ import {
resolvePaneLinkCwd,
resolvePaneSeedCwd,
resolveQueuedInitialCwd,
resetTerminalKeyboardProtocolAfterInterrupt,
shouldDetachPaneTransportOnUnmount,
splitPaneWithOneShotStartup,
suppressIntentionalPaneCloseExit
} from './use-terminal-pane-lifecycle'
describe('resetTerminalKeyboardProtocolAfterInterrupt', () => {
it('does not write to an xterm whose pipeline is certified dead', async () => {
const { _resetWritePipelineHealthForTests, notifyUndeliverableWrite } =
await import('@/lib/pane-manager/terminal-write-pipeline-health')
const terminal = { write: vi.fn() }
try {
notifyUndeliverableWrite(terminal, 'replay-wedged')
resetTerminalKeyboardProtocolAfterInterrupt(terminal as never)
expect(terminal.write).not.toHaveBeenCalled()
} finally {
_resetWritePipelineHealthForTests(terminal)
}
})
})
describe('splitPaneWithOneShotStartup', () => {
it('only exposes startup to the intentional split and clears it afterwards', () => {
const deps: { startup?: { command: string; env?: Record<string, string> } | null } = {
@@ -15,7 +15,10 @@ import {
resolveTerminalCursorInactiveStyle
} from '@/lib/pane-manager/pane-terminal-options'
import { normalizeDesktopTerminalScrollbackRows } from '../../../../shared/terminal-scrollback-policy'
import { configureTerminalOutputBacklogCap } from '@/lib/pane-manager/pane-terminal-output-scheduler'
import {
configureTerminalOutputBacklogCap,
writeTerminalOutput
} from '@/lib/pane-manager/pane-terminal-output-scheduler'
import { normalizeTerminalLineHeight } from '../../../../shared/terminal-line-height-settings'
import { normalizeTerminalTuiMouseWheelMultiplier } from '@/lib/pane-manager/pane-terminal-mouse-wheel'
import { buildWindowsPtyCompatibilityOptions } from '@/lib/pane-manager/windows-pty-compatibility'
@@ -132,6 +135,17 @@ import {
shouldClearLaunchAgentForClosedPane
} from './terminal-pane-close-identity'
export function resetTerminalKeyboardProtocolAfterInterrupt(terminal: Terminal): void {
// Use the guarded output path so a certified/throwing xterm cannot escape a
// keyboard handler or retain more writes while pane recovery is delayed.
writeTerminalOutput(terminal, RESET_KITTY_KEYBOARD_PROTOCOL, {
foreground: true,
// The interrupt itself already took the synchronous input path. Queue the
// renderer reset so it cannot flush a PTY backlog inside the key handler.
latencySensitive: false
})
}
export function recordRuntimeCreatedTerminalPaneSplit(
createdPane: unknown,
args: {
@@ -981,7 +995,7 @@ export function useTerminalPaneLifecycle({
pane.terminal.input(TERMINAL_INTERRUPT_INPUT)
// Why: CLIs such as Codex can die on SIGINT before restoring
// xterm's renderer-side Kitty flags, leaving the shell corrupted.
pane.terminal.write(RESET_KITTY_KEYBOARD_PROTOCOL)
resetTerminalKeyboardProtocolAfterInterrupt(pane.terminal)
} else {
pendingTerminalInterruptKeyup = false
}
@@ -249,6 +249,22 @@ describe('pane terminal output scheduler', () => {
}
})
it('does not probe a certified terminal while waiting for parsed output', async () => {
const { waitForTerminalOutputParsed } = await loadScheduler()
const { _resetWritePipelineHealthForTests, notifyUndeliverableWrite } =
await import('./terminal-write-pipeline-health')
const terminal = createTerminal()
try {
notifyUndeliverableWrite(terminal, 'replay-wedged')
await waitForTerminalOutputParsed(terminal)
expect(terminal.write).not.toHaveBeenCalled()
} finally {
_resetWritePipelineHealthForTests(terminal)
}
})
it('credits an empty write immediately', async () => {
const { writeTerminalOutput } = await loadScheduler()
const terminal = createTerminal()
@@ -1484,6 +1484,11 @@ export function registerTerminalBacklogRecovery(
export function waitForTerminalOutputParsed(terminal: TerminalOutputTarget): Promise<void> {
flushTerminalOutput(terminal)
if (isTerminalWritePipelineCertifiedDead(terminal)) {
// A dead pipeline cannot settle; recovery owns it and serializers must not
// enqueue probe writes while a bounded remount retry is pending.
return Promise.resolve()
}
return new Promise((resolve) => {
let settled = false