mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 00:02:05 +00:00
Harden xterm write pipeline against sync-throw wedge that freezes panes
A synchronous exception escaping xterm's WriteBuffer loop permanently wedges that terminal: _innerWrite has no try/catch around the parse action or the write-completion callback, the tail re-schedule never runs, and write() only re-arms on an empty buffer. The pane stops rendering and, if a replay was in flight, the replay guard latches and pty-connection's onData silently eats every keystroke — matching the field reports (Discord #performance, issue #2836: content visible, shell alive, daemon output.log flat). Both vectors verified against vendored xterm 6.1.0-beta.287 in xterm-write-buffer-stall.repro.test.ts. Three layers of defense: - Guard every write-completion callback Orca hands xterm at the two choke points (writeForegroundTerminalChunk, writeBackgroundTerminalChunk), with settle and onParsed guarded separately so a WebGL/renderer failure during viewport settle cannot starve the replay-guard release. - Guard all throwing-capable custom parser handlers (DA1, OSC 10/11, CSI ?h/?l mode reports, OSC 52 clipboard, OSC 7 cwd), degrading a throw to "not handled" — same escape class as terminal-link-provider-guard.ts. - Replay-guard watchdog: each engagement releases exactly once, from xterm's completion or a 10s watchdog, so a lost completion (wedged pipeline, disposed-terminal race) cannot latch the guard on a live pane; replayIntoTerminalAsync resolves on either path so restore chains cannot hang. Force-releases record a crash breadcrumb. All guard trips record rate-capped crash breadcrumbs, so the next field occurrence names the throwing stack instead of failing silently. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -1,6 +1,27 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { ManagedPane } from '@/lib/pane-manager/pane-manager'
|
||||
import { isPaneReplaying, replayIntoTerminal, type ReplayingPanesRef } from './replay-guard'
|
||||
import {
|
||||
isPaneReplaying,
|
||||
replayIntoTerminal,
|
||||
replayIntoTerminalAsync,
|
||||
type ReplayingPanesRef
|
||||
} from './replay-guard'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
recordRendererCrashBreadcrumb: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/crash-diagnostics', () => ({
|
||||
recordRendererCrashBreadcrumb: mocks.recordRendererCrashBreadcrumb
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.recordRendererCrashBreadcrumb.mockClear()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
function makeRef(): ReplayingPanesRef {
|
||||
return { current: new Map() } as ReplayingPanesRef
|
||||
@@ -169,3 +190,96 @@ describe('replay-guard', () => {
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('replay-guard watchdog', () => {
|
||||
it('force-releases the guard when xterm never completes the write (wedged pipeline repro)', () => {
|
||||
// Why: a sync throw escaping xterm's WriteBuffer loop drops the pending
|
||||
// write completion forever (xterm-write-buffer-stall.repro.test.ts).
|
||||
// Without the watchdog the guard latches and pty-connection.ts onData
|
||||
// silently eats every keystroke on a live pane.
|
||||
vi.useFakeTimers()
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const ref = makeRef()
|
||||
const { pane } = makeFakePane(1)
|
||||
|
||||
replayIntoTerminal(pane, ref, 'restored bytes', 10_000)
|
||||
expect(isPaneReplaying(ref, 1)).toBe(true)
|
||||
|
||||
vi.advanceTimersByTime(9_999)
|
||||
expect(isPaneReplaying(ref, 1)).toBe(true)
|
||||
|
||||
vi.advanceTimersByTime(1)
|
||||
expect(isPaneReplaying(ref, 1)).toBe(false)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_replay_guard_watchdog_release',
|
||||
{ paneId: 1 }
|
||||
)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not double-decrement when the completion arrives after the watchdog fired', () => {
|
||||
vi.useFakeTimers()
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const ref = makeRef()
|
||||
const { pane, terminal } = makeFakePane(1)
|
||||
|
||||
// First replay's completion is lost; second replay is healthy but slow.
|
||||
replayIntoTerminal(pane, ref, 'lost completion', 1_000)
|
||||
replayIntoTerminal(pane, ref, 'slow completion', 60_000)
|
||||
terminal.pendingCallbacks.shift() // drop the first completion entirely
|
||||
expect(isPaneReplaying(ref, 1)).toBe(true)
|
||||
|
||||
vi.advanceTimersByTime(1_000)
|
||||
// Watchdog released only the lost engagement; the healthy one still holds.
|
||||
expect(isPaneReplaying(ref, 1)).toBe(true)
|
||||
|
||||
terminal.flush()
|
||||
expect(isPaneReplaying(ref, 1)).toBe(false)
|
||||
expect(ref.current.has(1)).toBe(false)
|
||||
|
||||
// A very late duplicate release must be a no-op.
|
||||
vi.advanceTimersByTime(120_000)
|
||||
expect(ref.current.has(1)).toBe(false)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not fire the watchdog after a normal completion', () => {
|
||||
vi.useFakeTimers()
|
||||
const ref = makeRef()
|
||||
const { pane, terminal } = makeFakePane(1)
|
||||
|
||||
replayIntoTerminal(pane, ref, 'healthy', 1_000)
|
||||
terminal.flush()
|
||||
expect(isPaneReplaying(ref, 1)).toBe(false)
|
||||
|
||||
vi.advanceTimersByTime(60_000)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('resolves replayIntoTerminalAsync via the watchdog so restore chains cannot hang', async () => {
|
||||
vi.useFakeTimers()
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const ref = makeRef()
|
||||
const { pane } = makeFakePane(1)
|
||||
|
||||
const replayDone = replayIntoTerminalAsync(pane, ref, 'restored bytes', 1_000)
|
||||
let resolved = false
|
||||
void replayDone.then(() => {
|
||||
resolved = true
|
||||
})
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(resolved).toBe(true)
|
||||
expect(isPaneReplaying(ref, 1)).toBe(false)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { ManagedPane } from '@/lib/pane-manager/pane-manager'
|
||||
import { writeForegroundTerminalChunk } from '@/lib/pane-manager/pane-terminal-foreground-render-settle'
|
||||
import { recordRendererCrashBreadcrumb } from '@/lib/crash-diagnostics'
|
||||
|
||||
// Why: xterm.js auto-responds to terminal query sequences (DA1 `CSI c`,
|
||||
// DECRQM `CSI ? Ps $ p`, OSC 10/11 color queries, focus events, CPR) by
|
||||
@@ -27,10 +28,60 @@ import { writeForegroundTerminalChunk } from '@/lib/pane-manager/pane-terminal-f
|
||||
|
||||
export type ReplayingPanesRef = React.RefObject<Map<number, number>>
|
||||
|
||||
// Why a watchdog: the decrement above only runs when xterm completes the
|
||||
// write. A wedged WriteBuffer (sync throw escaping a parse handler or a
|
||||
// write-completion callback — see xterm-write-buffer-stall.repro.test.ts) or
|
||||
// a disposed-terminal race can drop that completion forever, leaving the
|
||||
// guard latched on a live pane — which silently eats every keystroke
|
||||
// (Discord #performance / issue #2836). Real replays parse in well under a
|
||||
// second, so a 10s ceiling only ever fires on a genuinely lost completion.
|
||||
const REPLAY_GUARD_WATCHDOG_MS = 10_000
|
||||
|
||||
export function isPaneReplaying(ref: ReplayingPanesRef, paneId: number): boolean {
|
||||
return (ref.current.get(paneId) ?? 0) > 0
|
||||
}
|
||||
|
||||
/**
|
||||
* Engage the replay counter for one write and return the release function.
|
||||
* Release runs exactly once — from xterm's write completion or, failing
|
||||
* that, from the watchdog — so a lost completion cannot latch the guard.
|
||||
*/
|
||||
function engageReplayGuard(
|
||||
map: Map<number, number>,
|
||||
paneId: number,
|
||||
watchdogMs: number,
|
||||
onRelease?: () => void
|
||||
): () => void {
|
||||
map.set(paneId, (map.get(paneId) ?? 0) + 1)
|
||||
let released = false
|
||||
let watchdog: ReturnType<typeof setTimeout> | null = null
|
||||
const release = (reason: 'parsed' | 'watchdog'): void => {
|
||||
if (released) {
|
||||
return
|
||||
}
|
||||
released = true
|
||||
if (watchdog !== null) {
|
||||
clearTimeout(watchdog)
|
||||
watchdog = null
|
||||
}
|
||||
const remaining = (map.get(paneId) ?? 1) - 1
|
||||
if (remaining <= 0) {
|
||||
map.delete(paneId)
|
||||
} else {
|
||||
map.set(paneId, remaining)
|
||||
}
|
||||
if (reason === 'watchdog') {
|
||||
console.error(
|
||||
`[terminal] replay guard force-released for pane ${paneId} — xterm never completed the replay write (wedged write pipeline?)`
|
||||
)
|
||||
recordRendererCrashBreadcrumb('terminal_replay_guard_watchdog_release', { paneId })
|
||||
}
|
||||
onRelease?.()
|
||||
}
|
||||
watchdog = setTimeout(() => release('watchdog'), watchdogMs)
|
||||
return () => release('parsed')
|
||||
}
|
||||
|
||||
/** Writes `data` into the pane's terminal with the replay guard engaged,
|
||||
* so xterm's auto-replies to embedded query sequences do not leak to the
|
||||
* shell as input. The counter increments/decrements so nested replays
|
||||
@@ -38,53 +89,39 @@ export function isPaneReplaying(ref: ReplayingPanesRef, paneId: number): boolean
|
||||
export function replayIntoTerminal(
|
||||
pane: ManagedPane,
|
||||
replayingPanesRef: ReplayingPanesRef,
|
||||
data: string
|
||||
data: string,
|
||||
watchdogMs: number = REPLAY_GUARD_WATCHDOG_MS
|
||||
): void {
|
||||
if (!data) {
|
||||
return
|
||||
}
|
||||
const map = replayingPanesRef.current
|
||||
map.set(pane.id, (map.get(pane.id) ?? 0) + 1)
|
||||
const onParsed = (): void => {
|
||||
const remaining = (map.get(pane.id) ?? 1) - 1
|
||||
if (remaining <= 0) {
|
||||
map.delete(pane.id)
|
||||
} else {
|
||||
map.set(pane.id, remaining)
|
||||
}
|
||||
}
|
||||
const releaseParsed = engageReplayGuard(replayingPanesRef.current, pane.id, watchdogMs)
|
||||
// Why: hidden/snapshot replay bypasses the live foreground write path, but
|
||||
// WebGL/canvas renderers still need a post-parse repaint to drop stale cells.
|
||||
writeForegroundTerminalChunk(pane.terminal, data, {
|
||||
forceViewportRefresh: true,
|
||||
followupViewportRefresh: true,
|
||||
onParsed
|
||||
onParsed: releaseParsed
|
||||
})
|
||||
}
|
||||
|
||||
export function replayIntoTerminalAsync(
|
||||
pane: ManagedPane,
|
||||
replayingPanesRef: ReplayingPanesRef,
|
||||
data: string
|
||||
data: string,
|
||||
watchdogMs: number = REPLAY_GUARD_WATCHDOG_MS
|
||||
): Promise<void> {
|
||||
if (!data) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
const map = replayingPanesRef.current
|
||||
map.set(pane.id, (map.get(pane.id) ?? 0) + 1)
|
||||
return new Promise((resolve) => {
|
||||
// Why resolve on either release path: callers await this to sequence
|
||||
// restore steps; a lost write completion must not hang the restore chain.
|
||||
const releaseParsed = engageReplayGuard(replayingPanesRef.current, pane.id, watchdogMs, resolve)
|
||||
writeForegroundTerminalChunk(pane.terminal, data, {
|
||||
forceViewportRefresh: true,
|
||||
followupViewportRefresh: true,
|
||||
onParsed: () => {
|
||||
const remaining = (map.get(pane.id) ?? 1) - 1
|
||||
if (remaining <= 0) {
|
||||
map.delete(pane.id)
|
||||
} else {
|
||||
map.set(pane.id, remaining)
|
||||
}
|
||||
resolve()
|
||||
}
|
||||
onParsed: releaseParsed
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
resolveEffectiveTerminalAppearance
|
||||
} from '@/lib/terminal-theme'
|
||||
import { buildFontFamily } from './layout-serialization'
|
||||
import { guardParserHandler } from './terminal-parser-handler-guard'
|
||||
import { captureScrollState, restoreScrollState, safeFit } from '@/lib/pane-manager/pane-tree-ops'
|
||||
import {
|
||||
normalizeTerminalFastScrollSensitivity,
|
||||
@@ -55,50 +56,56 @@ export function installMode2031Handlers(deps: Mode2031HandlerDeps): IDisposable[
|
||||
// continue processing the same sequence, so compound sequences like
|
||||
// `CSI ?25;2031h` still update cursor visibility correctly.
|
||||
return [
|
||||
deps.parser.registerCsiHandler({ prefix: '?', final: 'h' }, (params) => {
|
||||
if (hasMode2031(params)) {
|
||||
// Why: a restored xterm buffer may contain `CSI ?2031h` emitted by
|
||||
// the previous session's TUI (e.g. Claude Code). Replaying that
|
||||
// buffer runs this handler, and without the guard we'd push
|
||||
// `CSI ?997;1n` via transport.sendInput into a fresh shell that has
|
||||
// no TUI consuming it — zsh then echoes the literal escape sequence
|
||||
// onto the prompt. The replay guard in pty-connection.ts only covers
|
||||
// xterm's own onData auto-replies, not handler-triggered sends, so
|
||||
// gate explicitly here. We also skip recording the subscribe bit:
|
||||
// the fresh shell is not actually subscribed, so a later theme flip
|
||||
// must not push either. A real TUI that starts up after restore will
|
||||
// re-emit `?2031h` itself and register normally.
|
||||
//
|
||||
// Why this broad guard is safe across all replay sources: the only
|
||||
// replay path that can carry raw `?2031h` is cold-restore scrollback
|
||||
// (pty-connection.ts), which is disk-replayed PTY output against a
|
||||
// fresh shell — the case this guard targets. Daemon snapshot payloads
|
||||
// (`rehydrateSequences + SerializeAddon.serialize()`) and persisted
|
||||
// scrollback (`SerializeAddon.serialize()`) never contain `?2031`:
|
||||
// SerializeAddon's _serializeModes whitelists only ?1h/?66h/?2004h/
|
||||
// [4h/?6h/?45h/?1004h/?7l/mouse modes/?25l, and buildRehydrateSequences
|
||||
// emits only ?1049h/?2004h/?1h/mouse reporting modes. If xterm ever
|
||||
// adds ?2031 to that whitelist, this guard would start suppressing
|
||||
// legitimate subscribes during snapshot reattach — revisit then.
|
||||
if (deps.isReplaying()) {
|
||||
return false
|
||||
deps.parser.registerCsiHandler(
|
||||
{ prefix: '?', final: 'h' },
|
||||
guardParserHandler('csi-mode2031-subscribe', (params) => {
|
||||
if (hasMode2031(params)) {
|
||||
// Why: a restored xterm buffer may contain `CSI ?2031h` emitted by
|
||||
// the previous session's TUI (e.g. Claude Code). Replaying that
|
||||
// buffer runs this handler, and without the guard we'd push
|
||||
// `CSI ?997;1n` via transport.sendInput into a fresh shell that has
|
||||
// no TUI consuming it — zsh then echoes the literal escape sequence
|
||||
// onto the prompt. The replay guard in pty-connection.ts only covers
|
||||
// xterm's own onData auto-replies, not handler-triggered sends, so
|
||||
// gate explicitly here. We also skip recording the subscribe bit:
|
||||
// the fresh shell is not actually subscribed, so a later theme flip
|
||||
// must not push either. A real TUI that starts up after restore will
|
||||
// re-emit `?2031h` itself and register normally.
|
||||
//
|
||||
// Why this broad guard is safe across all replay sources: the only
|
||||
// replay path that can carry raw `?2031h` is cold-restore scrollback
|
||||
// (pty-connection.ts), which is disk-replayed PTY output against a
|
||||
// fresh shell — the case this guard targets. Daemon snapshot payloads
|
||||
// (`rehydrateSequences + SerializeAddon.serialize()`) and persisted
|
||||
// scrollback (`SerializeAddon.serialize()`) never contain `?2031`:
|
||||
// SerializeAddon's _serializeModes whitelists only ?1h/?66h/?2004h/
|
||||
// [4h/?6h/?45h/?1004h/?7l/mouse modes/?25l, and buildRehydrateSequences
|
||||
// emits only ?1049h/?2004h/?1h/mouse reporting modes. If xterm ever
|
||||
// adds ?2031 to that whitelist, this guard would start suppressing
|
||||
// legitimate subscribes during snapshot reattach — revisit then.
|
||||
if (deps.isReplaying()) {
|
||||
return false
|
||||
}
|
||||
deps.paneMode2031.set(deps.paneId, true)
|
||||
deps.onSubscribe()
|
||||
}
|
||||
deps.paneMode2031.set(deps.paneId, true)
|
||||
deps.onSubscribe()
|
||||
}
|
||||
return false
|
||||
}),
|
||||
return false
|
||||
})
|
||||
),
|
||||
// Why no replay guard on the unsubscribe branch: clearing stale bookkeeping
|
||||
// is harmless. We only push CSI 997 on subscribe, never on unsubscribe, so
|
||||
// even if a cold-restore replay carries `?2031l`, this handler just deletes
|
||||
// map entries that a later real `?2031h` will re-populate normally.
|
||||
deps.parser.registerCsiHandler({ prefix: '?', final: 'l' }, (params) => {
|
||||
if (hasMode2031(params)) {
|
||||
deps.paneMode2031.delete(deps.paneId)
|
||||
deps.paneLastThemeMode.delete(deps.paneId)
|
||||
}
|
||||
return false
|
||||
})
|
||||
deps.parser.registerCsiHandler(
|
||||
{ prefix: '?', final: 'l' },
|
||||
guardParserHandler('csi-mode2031-unsubscribe', (params) => {
|
||||
if (hasMode2031(params)) {
|
||||
deps.paneMode2031.delete(deps.paneId)
|
||||
deps.paneLastThemeMode.delete(deps.paneId)
|
||||
}
|
||||
return false
|
||||
})
|
||||
)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
terminalOscColorQuerySlotsForBody,
|
||||
type TerminalOscColorQuerySlot
|
||||
} from '../../../../shared/terminal-osc-color-reply'
|
||||
import { guardParserHandler } from './terminal-parser-handler-guard'
|
||||
|
||||
export const DEFAULT_DA1_RESPONSE = '\x1b[?1;2c'
|
||||
export const CONPTY_DA1_RESPONSE = '\x1b[?61;4c'
|
||||
@@ -118,37 +119,46 @@ export function installTerminalCapabilityReplyHandlers(
|
||||
deps: TerminalCapabilityRepliesDeps
|
||||
): IDisposable {
|
||||
const disposables = [
|
||||
deps.parser.registerCsiHandler({ final: 'c' }, (params) => {
|
||||
if (!isPrimaryDeviceAttributesQuery(params)) {
|
||||
return false
|
||||
}
|
||||
// Why: restored scrollback may contain old DA1 queries; answering those
|
||||
// into the fresh shell recreates the stray-input leak this handler fixes.
|
||||
if (!deps.isReplaying()) {
|
||||
deps.sendInput(deps.da1Response ?? DEFAULT_DA1_RESPONSE)
|
||||
}
|
||||
return true
|
||||
}),
|
||||
deps.parser.registerOscHandler(10, (data) => {
|
||||
const slots = terminalOscColorQuerySlotsForBody(10, data.trim())
|
||||
if (!slots) {
|
||||
return false
|
||||
}
|
||||
if (deps.isReplaying()) {
|
||||
deps.parser.registerCsiHandler(
|
||||
{ final: 'c' },
|
||||
guardParserHandler('csi-da1', (params) => {
|
||||
if (!isPrimaryDeviceAttributesQuery(params)) {
|
||||
return false
|
||||
}
|
||||
// Why: restored scrollback may contain old DA1 queries; answering those
|
||||
// into the fresh shell recreates the stray-input leak this handler fixes.
|
||||
if (!deps.isReplaying()) {
|
||||
deps.sendInput(deps.da1Response ?? DEFAULT_DA1_RESPONSE)
|
||||
}
|
||||
return true
|
||||
}
|
||||
return sendTerminalOscColorQueryRepliesForSlots(slots, deps.terminal, deps.sendInput)
|
||||
}),
|
||||
deps.parser.registerOscHandler(11, (data) => {
|
||||
const slots = terminalOscColorQuerySlotsForBody(11, data.trim())
|
||||
if (!slots) {
|
||||
return false
|
||||
}
|
||||
if (deps.isReplaying()) {
|
||||
return true
|
||||
}
|
||||
return sendTerminalOscColorQueryRepliesForSlots(slots, deps.terminal, deps.sendInput)
|
||||
})
|
||||
})
|
||||
),
|
||||
deps.parser.registerOscHandler(
|
||||
10,
|
||||
guardParserHandler('osc-10-color-query', (data) => {
|
||||
const slots = terminalOscColorQuerySlotsForBody(10, data.trim())
|
||||
if (!slots) {
|
||||
return false
|
||||
}
|
||||
if (deps.isReplaying()) {
|
||||
return true
|
||||
}
|
||||
return sendTerminalOscColorQueryRepliesForSlots(slots, deps.terminal, deps.sendInput)
|
||||
})
|
||||
),
|
||||
deps.parser.registerOscHandler(
|
||||
11,
|
||||
guardParserHandler('osc-11-color-query', (data) => {
|
||||
const slots = terminalOscColorQuerySlotsForBody(11, data.trim())
|
||||
if (!slots) {
|
||||
return false
|
||||
}
|
||||
if (deps.isReplaying()) {
|
||||
return true
|
||||
}
|
||||
return sendTerminalOscColorQueryRepliesForSlots(slots, deps.terminal, deps.sendInput)
|
||||
})
|
||||
)
|
||||
]
|
||||
|
||||
return {
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Terminal } from '@xterm/headless'
|
||||
import {
|
||||
_resetParserHandlerReportsForTests,
|
||||
guardParserHandler
|
||||
} from './terminal-parser-handler-guard'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
recordRendererCrashBreadcrumb: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/crash-diagnostics', () => ({
|
||||
recordRendererCrashBreadcrumb: mocks.recordRendererCrashBreadcrumb
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.recordRendererCrashBreadcrumb.mockClear()
|
||||
_resetParserHandlerReportsForTests()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
describe('guardParserHandler', () => {
|
||||
it('passes arguments and return value through for healthy handlers', () => {
|
||||
const handler = vi.fn((data: string) => data === 'handled')
|
||||
const guarded = guardParserHandler('test-handler', handler)
|
||||
expect(guarded('handled')).toBe(true)
|
||||
expect(guarded('other')).toBe(false)
|
||||
expect(handler).toHaveBeenCalledTimes(2)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('degrades a throwing handler to "not handled" and reports a breadcrumb', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const guarded = guardParserHandler('exploding-handler', () => {
|
||||
throw new TypeError('synthetic handler failure')
|
||||
})
|
||||
expect(guarded()).toBe(false)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_parser_handler_error',
|
||||
expect.objectContaining({
|
||||
handler: 'exploding-handler',
|
||||
errorName: 'TypeError',
|
||||
errorMessage: 'synthetic handler failure'
|
||||
})
|
||||
)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('caps repeated reports per handler', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const guarded = guardParserHandler('spammy-handler', () => {
|
||||
throw new Error('always fails')
|
||||
})
|
||||
for (let i = 0; i < 20; i++) {
|
||||
guarded()
|
||||
}
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(5)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the real xterm write pipeline alive through a throwing handler (inverse of the wedge repro)', () => {
|
||||
// Why: xterm-write-buffer-stall.repro.test.ts proves an UNguarded throwing
|
||||
// handler permanently wedges the WriteBuffer. This is the fix's proof:
|
||||
// the same poison sequence through a GUARDED handler keeps completing
|
||||
// writes.
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
vi.useFakeTimers()
|
||||
const term = new Terminal({ allowProposedApi: true })
|
||||
const completed: string[] = []
|
||||
term.parser.registerCsiHandler(
|
||||
{ final: 'z' },
|
||||
guardParserHandler('poisoned-csi', () => {
|
||||
throw new Error('synthetic parser handler failure')
|
||||
})
|
||||
)
|
||||
|
||||
term.write('\x1b[z', () => {
|
||||
completed.push('poisoned')
|
||||
})
|
||||
term.write('after', () => {
|
||||
completed.push('after')
|
||||
})
|
||||
expect(() => vi.runAllTimers()).not.toThrow()
|
||||
|
||||
term.write('later', () => {
|
||||
completed.push('later')
|
||||
})
|
||||
vi.runAllTimers()
|
||||
expect(completed).toEqual(['poisoned', 'after', 'later'])
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_parser_handler_error',
|
||||
expect.objectContaining({ handler: 'poisoned-csi' })
|
||||
)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,44 @@
|
||||
import { recordRendererCrashBreadcrumb } from '@/lib/crash-diagnostics'
|
||||
|
||||
// Why: xterm's EscapeSequenceParser invokes custom CSI/OSC handlers
|
||||
// synchronously inside WriteBuffer._innerWrite, which has no try/catch. A
|
||||
// handler throw skips the loop's tail re-schedule, and write() only re-arms
|
||||
// on an EMPTY buffer — one throw permanently freezes the pane (output stops;
|
||||
// a pending replay guard never releases and silently eats keystrokes).
|
||||
// Verified against vendored xterm 6.1.0-beta.287 in
|
||||
// xterm-write-buffer-stall.repro.test.ts. Same escape class as
|
||||
// terminal-link-provider-guard.ts, applied to parser handlers.
|
||||
const MAX_REPORTS_PER_HANDLER = 5
|
||||
const reportCountsByHandler = new Map<string, number>()
|
||||
|
||||
/**
|
||||
* Wrap a custom parser handler so a synchronous throw is reported and
|
||||
* degraded to "not handled" (xterm falls through to the previous/default
|
||||
* handler) instead of wedging the terminal's write pipeline.
|
||||
*/
|
||||
export function guardParserHandler<HandlerArgs extends unknown[]>(
|
||||
handlerName: string,
|
||||
handler: (...args: HandlerArgs) => boolean
|
||||
): (...args: HandlerArgs) => boolean {
|
||||
return (...args: HandlerArgs): boolean => {
|
||||
try {
|
||||
return handler(...args)
|
||||
} catch (error: unknown) {
|
||||
const reported = reportCountsByHandler.get(handlerName) ?? 0
|
||||
if (reported < MAX_REPORTS_PER_HANDLER) {
|
||||
reportCountsByHandler.set(handlerName, reported + 1)
|
||||
console.error(`[terminal] parser handler "${handlerName}" threw`, error)
|
||||
recordRendererCrashBreadcrumb('terminal_parser_handler_error', {
|
||||
handler: handlerName,
|
||||
errorName: error instanceof Error ? error.name : typeof error,
|
||||
errorMessage: error instanceof Error ? error.message : String(error)
|
||||
})
|
||||
}
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function _resetParserHandlerReportsForTests(): void {
|
||||
reportCountsByHandler.clear()
|
||||
}
|
||||
@@ -58,6 +58,7 @@ import {
|
||||
import { handleOsc52ClipboardRequest } from './osc52-clipboard'
|
||||
import { showOsc52ClipboardBlockedToast } from './osc52-clipboard-blocked-toast'
|
||||
import { parseOsc7 } from './parse-osc7'
|
||||
import { guardParserHandler } from './terminal-parser-handler-guard'
|
||||
import { resolveTerminalJisYenInput } from './terminal-jis-yen-input'
|
||||
import { installTerminalImeCompositionTracker } from './terminal-ime-composition-tracker'
|
||||
import {
|
||||
@@ -780,12 +781,15 @@ export function useTerminalPaneLifecycle({
|
||||
// both the enabled and disabled paths so xterm doesn't fall
|
||||
// through to any other OSC 52 handler and so our intentional drop
|
||||
// in the disabled path is explicit.
|
||||
const osc52Disposable = pane.terminal.parser.registerOscHandler(52, (data) =>
|
||||
handleOsc52ClipboardRequest(data, {
|
||||
allowClipboardWrite: settingsRef.current?.terminalAllowOsc52Clipboard === true,
|
||||
writeClipboardText: window.api.ui.writeClipboardText,
|
||||
onBlockedWrite: showOsc52ClipboardBlockedToast
|
||||
})
|
||||
const osc52Disposable = pane.terminal.parser.registerOscHandler(
|
||||
52,
|
||||
guardParserHandler('osc-52-clipboard', (data) =>
|
||||
handleOsc52ClipboardRequest(data, {
|
||||
allowClipboardWrite: settingsRef.current?.terminalAllowOsc52Clipboard === true,
|
||||
writeClipboardText: window.api.ui.writeClipboardText,
|
||||
onBlockedWrite: showOsc52ClipboardBlockedToast
|
||||
})
|
||||
)
|
||||
)
|
||||
osc52DisposablesRef.current.set(pane.id, osc52Disposable)
|
||||
|
||||
@@ -811,14 +815,17 @@ export function useTerminalPaneLifecycle({
|
||||
confirmed: false
|
||||
})
|
||||
}
|
||||
const osc7Disposable = pane.terminal.parser.registerOscHandler(7, (data) => {
|
||||
const parsedCwd = parseOsc7(data, { uncHost: osc7UncHost })
|
||||
if (parsedCwd) {
|
||||
const confirmed = !isPaneReplaying(replayingPanesRef, pane.id)
|
||||
paneCwdRef.current.set(pane.id, { cwd: parsedCwd, confirmed })
|
||||
}
|
||||
return true
|
||||
})
|
||||
const osc7Disposable = pane.terminal.parser.registerOscHandler(
|
||||
7,
|
||||
guardParserHandler('osc-7-cwd', (data) => {
|
||||
const parsedCwd = parseOsc7(data, { uncHost: osc7UncHost })
|
||||
if (parsedCwd) {
|
||||
const confirmed = !isPaneReplaying(replayingPanesRef, pane.id)
|
||||
paneCwdRef.current.set(pane.id, { cwd: parsedCwd, confirmed })
|
||||
}
|
||||
return true
|
||||
})
|
||||
)
|
||||
osc7DisposablesRef.current.set(pane.id, osc7Disposable)
|
||||
|
||||
// Why: let host-handled keys bypass xterm's kitty CSI-u encoder.
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { runGuardedWriteCompletionStep } from './xterm-write-callback-guard'
|
||||
|
||||
export type ForegroundTerminalOutputTarget = {
|
||||
buffer?: {
|
||||
active?: {
|
||||
@@ -131,18 +133,24 @@ export function writeForegroundTerminalChunk(
|
||||
const beforeWriteViewport = options.forceViewportRefresh
|
||||
? captureViewportSnapshot(terminal)
|
||||
: null
|
||||
try {
|
||||
terminal.write(data, () => {
|
||||
if (beforeWriteViewport) {
|
||||
settleForegroundRender(terminal, beforeWriteViewport, options)
|
||||
}
|
||||
options.onParsed?.()
|
||||
})
|
||||
} catch {
|
||||
// Why guarded steps: this callback runs inside xterm's WriteBuffer loop,
|
||||
// where an escaping throw permanently wedges the terminal (see
|
||||
// xterm-write-callback-guard.ts). Guard settle and onParsed separately so a
|
||||
// renderer/WebGL failure during settle can't starve the replay-guard release.
|
||||
const runCompletionSteps = (): void => {
|
||||
if (beforeWriteViewport) {
|
||||
settleForegroundRender(terminal, beforeWriteViewport, options)
|
||||
runGuardedWriteCompletionStep('foreground-render-settle', () =>
|
||||
settleForegroundRender(terminal, beforeWriteViewport, options)
|
||||
)
|
||||
}
|
||||
options.onParsed?.()
|
||||
if (options.onParsed) {
|
||||
runGuardedWriteCompletionStep('foreground-on-parsed', options.onParsed)
|
||||
}
|
||||
}
|
||||
try {
|
||||
terminal.write(data, runCompletionSteps)
|
||||
} catch {
|
||||
runCompletionSteps()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
captureTerminalWriteScrollIntent,
|
||||
enforceTerminalWriteScrollIntent
|
||||
} from './terminal-scroll-intent'
|
||||
import { runGuardedWriteCompletionStep } from './xterm-write-callback-guard'
|
||||
|
||||
type TerminalOutputTarget = ForegroundTerminalOutputTarget
|
||||
|
||||
@@ -636,26 +637,34 @@ function writeBackgroundTerminalChunk(
|
||||
data: string,
|
||||
onParsed?: TerminalOutputParsedCallback
|
||||
): void {
|
||||
// Why guarded: these callbacks run inside xterm's WriteBuffer loop, where an
|
||||
// escaping throw permanently wedges the terminal (see
|
||||
// xterm-write-callback-guard.ts).
|
||||
const runOnParsed = onParsed
|
||||
? (): void => runGuardedWriteCompletionStep('background-on-parsed', onParsed)
|
||||
: undefined
|
||||
const scrollIntent = captureTerminalWriteScrollIntent(terminal)
|
||||
if (!scrollIntent) {
|
||||
if (!onParsed || terminal.write.length < 2) {
|
||||
if (!runOnParsed || terminal.write.length < 2) {
|
||||
terminal.write(data)
|
||||
onParsed?.()
|
||||
runOnParsed?.()
|
||||
return
|
||||
}
|
||||
terminal.write(data, onParsed)
|
||||
terminal.write(data, runOnParsed)
|
||||
return
|
||||
}
|
||||
const runScrollIntentThenParsed = (): void => {
|
||||
runGuardedWriteCompletionStep('background-scroll-intent', () =>
|
||||
enforceTerminalWriteScrollIntent(terminal, scrollIntent)
|
||||
)
|
||||
runOnParsed?.()
|
||||
}
|
||||
if (terminal.write.length < 2) {
|
||||
terminal.write(data)
|
||||
enforceTerminalWriteScrollIntent(terminal, scrollIntent)
|
||||
onParsed?.()
|
||||
runScrollIntentThenParsed()
|
||||
return
|
||||
}
|
||||
terminal.write(data, () => {
|
||||
enforceTerminalWriteScrollIntent(terminal, scrollIntent)
|
||||
onParsed?.()
|
||||
})
|
||||
terminal.write(data, runScrollIntentThenParsed)
|
||||
}
|
||||
|
||||
function writeForegroundTerminalChunkWithIntent(
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
_resetWriteCompletionReportsForTests,
|
||||
runGuardedWriteCompletionStep
|
||||
} from './xterm-write-callback-guard'
|
||||
import { writeForegroundTerminalChunk } from './pane-terminal-foreground-render-settle'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
recordRendererCrashBreadcrumb: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/crash-diagnostics', () => ({
|
||||
recordRendererCrashBreadcrumb: mocks.recordRendererCrashBreadcrumb
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.recordRendererCrashBreadcrumb.mockClear()
|
||||
_resetWriteCompletionReportsForTests()
|
||||
})
|
||||
|
||||
describe('runGuardedWriteCompletionStep', () => {
|
||||
it('contains a synchronous throw and reports a breadcrumb', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
expect(() =>
|
||||
runGuardedWriteCompletionStep('test-step', () => {
|
||||
throw new RangeError('synthetic settle failure')
|
||||
})
|
||||
).not.toThrow()
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_write_completion_error',
|
||||
expect.objectContaining({
|
||||
context: 'test-step',
|
||||
errorName: 'RangeError',
|
||||
errorMessage: 'synthetic settle failure'
|
||||
})
|
||||
)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('caps repeated reports per context so a throw-per-write loop cannot spam breadcrumbs', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
for (let i = 0; i < 20; i++) {
|
||||
runGuardedWriteCompletionStep('spammy-step', () => {
|
||||
throw new Error('always fails')
|
||||
})
|
||||
}
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledTimes(5)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('runs non-throwing steps transparently', () => {
|
||||
const step = vi.fn()
|
||||
runGuardedWriteCompletionStep('ok-step', step)
|
||||
expect(step).toHaveBeenCalledTimes(1)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('writeForegroundTerminalChunk completion guarding', () => {
|
||||
it('still releases onParsed when the settle step throws (replay-guard latch protection)', () => {
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
try {
|
||||
const pendingCallbacks: (() => void)[] = []
|
||||
// Why a getter that throws only after the write is dispatched: it
|
||||
// models renderer/buffer state failing between parse start and the
|
||||
// post-parse viewport settle (refreshVisibleRowsNow self-catches, so
|
||||
// the viewport comparison is the escaping surface).
|
||||
let bufferAccessPoisoned = false
|
||||
const realBuffer = { active: { cursorY: 0, baseY: 0, viewportY: 0 } }
|
||||
const terminal = {
|
||||
rows: 24,
|
||||
get buffer() {
|
||||
if (bufferAccessPoisoned) {
|
||||
throw new Error('synthetic buffer access failure')
|
||||
}
|
||||
return realBuffer
|
||||
},
|
||||
write: (_data: string, cb?: () => void) => {
|
||||
if (cb) {
|
||||
pendingCallbacks.push(cb)
|
||||
}
|
||||
}
|
||||
}
|
||||
const onParsed = vi.fn()
|
||||
|
||||
writeForegroundTerminalChunk(terminal, 'restored bytes', {
|
||||
forceViewportRefresh: true,
|
||||
onParsed
|
||||
})
|
||||
bufferAccessPoisoned = true
|
||||
// Simulate xterm completing the parse: the completion callback must not
|
||||
// let the settle throw escape into the WriteBuffer, and onParsed (the
|
||||
// replay-guard release) must still run.
|
||||
expect(() => pendingCallbacks.forEach((cb) => cb())).not.toThrow()
|
||||
expect(onParsed).toHaveBeenCalledTimes(1)
|
||||
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
|
||||
'terminal_write_completion_error',
|
||||
expect.objectContaining({ context: 'foreground-render-settle' })
|
||||
)
|
||||
} finally {
|
||||
errorSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,40 @@
|
||||
import { recordRendererCrashBreadcrumb } from '@/lib/crash-diagnostics'
|
||||
|
||||
// Why: xterm's WriteBuffer._innerWrite invokes write-completion callbacks with
|
||||
// no try/catch; a synchronous throw skips the loop's tail re-schedule, and
|
||||
// write() only re-arms processing when the buffer is EMPTY — which a stalled
|
||||
// buffer never is again. One escaping throw therefore permanently freezes the
|
||||
// pane: output stops rendering and a pending replay guard never releases, so
|
||||
// the pane silently eats every keystroke while the shell stays alive
|
||||
// (Discord #performance / issue #2836). Verified against the vendored xterm
|
||||
// 6.1.0-beta.287 in xterm-write-buffer-stall.repro.test.ts.
|
||||
const MAX_REPORTS_PER_CONTEXT = 5
|
||||
const reportCountsByContext = new Map<string, number>()
|
||||
|
||||
/**
|
||||
* Run one step of a write-completion callback so a synchronous throw cannot
|
||||
* escape into xterm's WriteBuffer. Steps are guarded individually so an
|
||||
* earlier step's failure (e.g. a WebGL refresh during viewport settle) cannot
|
||||
* starve a later step (e.g. the replay-guard release).
|
||||
*/
|
||||
export function runGuardedWriteCompletionStep(context: string, step: () => void): void {
|
||||
try {
|
||||
step()
|
||||
} catch (error: unknown) {
|
||||
const reported = reportCountsByContext.get(context) ?? 0
|
||||
if (reported >= MAX_REPORTS_PER_CONTEXT) {
|
||||
return
|
||||
}
|
||||
reportCountsByContext.set(context, reported + 1)
|
||||
console.error(`[terminal] write-completion step "${context}" threw`, error)
|
||||
recordRendererCrashBreadcrumb('terminal_write_completion_error', {
|
||||
context,
|
||||
errorName: error instanceof Error ? error.name : typeof error,
|
||||
errorMessage: error instanceof Error ? error.message : String(error)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
export function _resetWriteCompletionReportsForTests(): void {
|
||||
reportCountsByContext.clear()
|
||||
}
|
||||
Reference in New Issue
Block a user