fix(terminal): guard each write-completion step separately

The parsed and write-failure callbacks ran their completion steps as a
bare sequence, so a throw in any step skipped every later one. Skipping
the dense-SGR release leaves the pacing slot claimed forever, which wedges
the terminal: nothing else clears it.

Run each step through the existing write-completion guard, as the
background write path already does.
This commit is contained in:
Neil
2026-09-19 15:07:09 -07:00
parent 2b2d64ce7b
commit 3fefb261f7
3 changed files with 69 additions and 10 deletions
@@ -1,4 +1,5 @@
import { writeForegroundTerminalChunk } from './pane-terminal-foreground-render-settle'
import { runGuardedWriteCompletionStep } from './xterm-write-callback-guard'
import { registerTerminalOutputAckCredits } from './pane-terminal-output-ack-credit'
import {
armTerminalWriteStallWatch,
@@ -132,8 +133,12 @@ export function flushTerminalOutputImpl(
} catch {
// Why: pre-write hooks/setup failed before xterm owned these bytes; cancel the watch, but consumed + abandoned chunks still credit delivery.
cancelTerminalWriteStallWatch(terminal)
ackCreditsParsed?.()
denseSgrRelease?.()
if (ackCreditsParsed) {
runGuardedWriteCompletionStep('flush-abort-ack-credits', ackCreditsParsed)
}
if (denseSgrRelease) {
runGuardedWriteCompletionStep('flush-abort-dense-release', denseSgrRelease)
}
fireQueuedAckCredits(entry)
clearForegroundRelease(entry)
recordQueueDebugPressure()
@@ -82,10 +82,19 @@ export function composeParsedCallback(
try {
onParsed?.()
} finally {
ackCreditsParsed?.()
denseSgrRelease?.()
pacer?.()
settleTerminalWriteStallWatch(terminal)
// Why guarded per step: one throwing step would skip every later one, and a skipped dense release pins inFlight so the pane never drains again.
if (ackCreditsParsed) {
runGuardedWriteCompletionStep('parsed-ack-credits', ackCreditsParsed)
}
if (denseSgrRelease) {
runGuardedWriteCompletionStep('parsed-dense-release', denseSgrRelease)
}
if (pacer) {
runGuardedWriteCompletionStep('parsed-pacer', pacer)
}
runGuardedWriteCompletionStep('parsed-stall-settle', () =>
settleTerminalWriteStallWatch(terminal)
)
}
}
}
@@ -98,8 +107,12 @@ export function composeWriteFailureCallback(
return () => {
try {
// A rejected write still consumed the main-owned delivery window.
ackCreditsParsed?.()
denseSgrRelease?.()
if (ackCreditsParsed) {
runGuardedWriteCompletionStep('write-failure-ack-credits', ackCreditsParsed)
}
if (denseSgrRelease) {
runGuardedWriteCompletionStep('write-failure-dense-release', denseSgrRelease)
}
} finally {
// Why: a synchronous rejection proves undeliverability but nothing about parse progress; recover without extending replay guards.
failTerminalWriteStallWatch(terminal)
@@ -179,8 +192,12 @@ export function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background'
} catch {
// Why: beforeWrite or write setup can fail before xterm owns the bytes; cancel the armed watch without claiming parser failure.
cancelTerminalWriteStallWatch(entry.terminal)
ackCreditsParsed?.()
denseSgrRelease?.()
if (ackCreditsParsed) {
runGuardedWriteCompletionStep('drain-abort-ack-credits', ackCreditsParsed)
}
if (denseSgrRelease) {
runGuardedWriteCompletionStep('drain-abort-dense-release', denseSgrRelease)
}
fireQueuedAckCredits(entry)
entry.chunks.length = 0
entry.chunkIndex = 0
@@ -518,6 +518,43 @@ describe('pane terminal output scheduler', () => {
expect(queuedByTerminal.has(terminal)).toBe(true)
})
it('releases the dense pacing slot when a parsed ack credit throws', async () => {
vi.useFakeTimers()
const { writeTerminalOutput } = await loadScheduler()
const terminal = createTerminal()
const parsed: (() => void)[] = []
terminal.write.mockImplementation((_data: string, callback?: () => void) => {
if (callback) {
parsed.push(callback)
}
})
const dense = Array.from(
{ length: 300 },
(_, index) => `\x1b[${30 + (index % 8)}mX\x1b[0m`
).join('')
writeTerminalOutput(terminal, dense, {
foreground: false,
ackCredit: () => {
throw new Error('ack credit failed')
}
})
writeTerminalOutput(terminal, dense, { foreground: false })
vi.advanceTimersByTime(50)
expect(terminal.write).toHaveBeenCalledTimes(1)
parsed.shift()?.()
vi.advanceTimersByTime(0)
// A throwing credit must not skip the dense release, or inFlight stays
// pinned and the terminal never drains again.
expect(terminal.write).toHaveBeenCalledTimes(2)
expect(mocks.recordRendererCrashBreadcrumb).toHaveBeenCalledWith(
'terminal_write_completion_error',
expect.objectContaining({ context: 'parsed-ack-credits' })
)
})
it('promotes large background backlogs to high-priority drains', async () => {
vi.useFakeTimers()
const { writeTerminalOutput } = await loadScheduler()