fix(terminal): close dense-output review gaps

This commit is contained in:
Neil
2026-08-04 21:15:43 -07:00
parent f0c927f03d
commit dbdabe1739
8 changed files with 157 additions and 41 deletions
+5 -6
View File
@@ -16426,9 +16426,7 @@ describe('registerPtyHandlers', () => {
.filter(([channel]) => channel === 'pty:data')
.map(([, payload]) => payload as { data: string; rawLength?: number; transformed?: true })
const delivered = payloads.map(({ data }) => data).join('')
expect(delivered.length).toBe(dense.length)
expect(delivered.slice(0, 512)).toBe(dense.slice(0, 512))
expect(delivered.slice(-512)).toBe(dense.slice(-512))
expect(delivered).toBe(dense)
expect(payloads.reduce((sum, payload) => sum + (payload.rawLength ?? 0), 0)).toBe(
dense.length
)
@@ -16576,7 +16574,7 @@ describe('registerPtyHandlers', () => {
})
})
it('rejects malformed and cross-window pty write IPC before provider writes', async () => {
it('accepts an explicit false backlog flag but rejects malformed and cross-window writes', async () => {
const mockProc = createMockProc()
spawnMock.mockReturnValue(mockProc.proc)
registerPtyHandlers(mainWindow as never)
@@ -16606,9 +16604,10 @@ describe('registerPtyHandlers', () => {
data: 'x',
rendererBacklogDropped: false
})
).toBe(false)
).toBe(true)
expect(writeAccepted(foreignWindowIpcEvent, { id: result.id, data: 'x' })).toBe(false)
expect(mockProc.proc.write).not.toHaveBeenCalled()
expect(mockProc.proc.write).toHaveBeenNthCalledWith(1, 'x')
expect(mockProc.proc.write).toHaveBeenNthCalledWith(2, 'x')
})
it('silently drops writes to a live PTY after ownership loss until pty:listSessions rebuilds it (frozen-terminal repro)', async () => {
+14 -16
View File
@@ -3107,8 +3107,8 @@ export function registerPtyHandlers(
(pending.droppedOutput === true
? pending.denseSgrDrop !== true
: pending.data.length <= INTERACTIVE_DENSE_BACKLOG_DROP_CHARS ||
!isDenseTerminalSgr(pending.data) ||
pending.projectionAdmissionIds !== undefined)
pending.projectionAdmissionIds !== undefined ||
!isDenseTerminalSgr(pending.data))
) {
return false
}
@@ -6732,7 +6732,7 @@ export function registerPtyHandlers(
}
}
type PtyWritePayload = { id: string; data: string; rendererBacklogDropped?: true }
type PtyWritePayload = { id: string; data: string; rendererBacklogDropped?: boolean }
type PtyViewportClaimPayload = { id: string; cols: number; rows: number }
const isPtyWritePayload = (value: unknown): value is PtyWritePayload =>
@@ -6742,7 +6742,7 @@ export function registerPtyHandlers(
(value as { id: string }).id.length > 0 &&
typeof (value as { data?: unknown }).data === 'string' &&
((value as { rendererBacklogDropped?: unknown }).rendererBacklogDropped === undefined ||
(value as { rendererBacklogDropped?: unknown }).rendererBacklogDropped === true)
typeof (value as { rendererBacklogDropped?: unknown }).rendererBacklogDropped === 'boolean')
const isPtyViewportClaimPayload = (value: unknown): value is PtyViewportClaimPayload =>
typeof value === 'object' &&
@@ -6764,6 +6764,14 @@ export function registerPtyHandlers(
!mainWindow.isDestroyed() &&
!(typeof mainWebContents.isDestroyed === 'function' && mainWebContents.isDestroyed())
const notePtyInputForWrite = (args: PtyWritePayload): void => {
notePtyInput(args.id, performance.now())
if (args.rendererBacklogDropped) {
interactiveDenseProtectionPtys.add(args.id)
}
prioritizePendingPtyDataForInput(args.id)
}
const writePtyInput = (args: PtyWritePayload): boolean | Promise<boolean> => {
// Why: mobile-presence-lock defense-in-depth — the renderer's onData guard can let one keystroke slip during the state-flip lag, so catch it server-side. See docs/mobile-presence-lock.md.
if (runtime?.getDriver(args.id).kind === 'mobile') {
@@ -6774,12 +6782,7 @@ export function registerPtyHandlers(
return false
}
try {
const now = performance.now()
notePtyInput(args.id, now)
if (args.rendererBacklogDropped) {
interactiveDenseProtectionPtys.add(args.id)
}
prioritizePendingPtyDataForInput(args.id)
notePtyInputForWrite(args)
if (visibleRendererPtys.has(args.id)) {
clearHiddenRendererResizeOutput(args.id)
}
@@ -6802,12 +6805,7 @@ export function registerPtyHandlers(
return false
}
try {
const now = performance.now()
notePtyInput(args.id, now)
if (args.rendererBacklogDropped) {
interactiveDenseProtectionPtys.add(args.id)
}
prioritizePendingPtyDataForInput(args.id)
notePtyInputForWrite(args)
if (visibleRendererPtys.has(args.id)) {
clearHiddenRendererResizeOutput(args.id)
}
@@ -12007,7 +12007,7 @@ describe('connectPanePty', () => {
enableMainAuthority()
const deps = createDeps({ isVisibleRef: { current: true } })
const terminalTarget = createKeyboardEventTarget()
const { pane, transport } = await connectHiddenPane(deps, terminalTarget.target)
const { pane, transport, dataCallback } = await connectHiddenPane(deps, terminalTarget.target)
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
typeof vi.fn
>
@@ -12017,22 +12017,18 @@ describe('connectPanePty', () => {
rows: 30,
seq: 64
})
const dense = Array.from(
const dense = `\x1b[c${Array.from(
{ length: 4_096 },
(_value, index) => `\x1b[38;5;${index % 216}mX\x1b[0m`
).join('')
const { writeTerminalOutput } =
await import('@/lib/pane-manager/pane-terminal-output-scheduler')
).join('')}`
vi.useFakeTimers()
try {
writeTerminalOutput(pane.terminal as never, dense, {
foreground: true,
latencySensitive: false
})
dataCallback(dense)
terminalTarget.dispatch(keyEvent({ key: 'a' }))
sendTerminalInputThroughPane(pane, 'a')
expect(transport.sendInput).toHaveBeenCalledWith('a', true)
expect(transport.sendInputImmediate).toHaveBeenCalledWith(DEFAULT_DA1_RESPONSE)
vi.advanceTimersByTime(499)
await flushAsyncTicks(4)
expect(getMainBufferSnapshot).not.toHaveBeenCalled()
@@ -6434,6 +6434,7 @@ export function connectPanePty(
// Why: every scheduler write claims one child so a split delivery is credited only after all children parse or discard.
ackCredit: takeCurrentTerminalDeliveryCredit() ?? undefined,
onBackgroundBacklogDropped: markHiddenOutputRestoreNeeded,
onDenseSgrBacklogDropped: salvageRendererQueriesFromDiscardedRestoreData,
latencySensitive:
!foreground || parseHiddenStartupOutput
? true
@@ -1450,16 +1450,19 @@ describe('pane terminal output scheduler', () => {
const firstCredit = vi.fn()
const secondCredit = vi.fn()
const echoCredit = vi.fn()
const droppedDenseOutput = vi.fn()
writeTerminalOutput(terminal, denseChunk, {
foreground: true,
latencySensitive: false,
ackCredit: firstCredit
ackCredit: firstCredit,
onDenseSgrBacklogDropped: droppedDenseOutput
})
writeTerminalOutput(terminal, denseChunk, {
foreground: true,
latencySensitive: false,
ackCredit: secondCredit
ackCredit: secondCredit,
onDenseSgrBacklogDropped: droppedDenseOutput
})
writeTerminalOutput(terminal, '__PRIOR_ECHO__', {
foreground: true,
@@ -1474,11 +1477,46 @@ describe('pane terminal output scheduler', () => {
vi.runAllTimers()
const output = terminal.write.mock.calls.map(([data]) => data).join('')
expect(output).toBe('__PRIOR_ECHO____ECHO__')
expect(output).toBe('\x18\x1b[0m__PRIOR_ECHO____ECHO__')
expect(output).not.toContain(denseChunk)
expect(firstCredit).toHaveBeenCalledTimes(1)
expect(secondCredit).toHaveBeenCalledTimes(1)
expect(echoCredit).toHaveBeenCalledTimes(1)
expect(droppedDenseOutput).toHaveBeenCalledWith(`${denseChunk}${denseChunk}`)
})
it('retains only a dense chunk partial escape before ordinary output', async () => {
vi.useFakeTimers()
const { prioritizeTerminalInput, writeTerminalOutput } = await loadScheduler()
const terminal = createTerminal()
const densePrefix = `${Array.from(
{ length: 2_048 },
(_value, index) => `\x1b[38;5;${index % 216}mX\x1b[0m`
).join('')}\x1b[`
const droppedDenseOutput = vi.fn()
const droppableDense = densePrefix.slice(0, -2)
writeTerminalOutput(terminal, droppableDense, {
foreground: true,
latencySensitive: false,
onDenseSgrBacklogDropped: droppedDenseOutput
})
writeTerminalOutput(terminal, densePrefix, {
foreground: true,
latencySensitive: false
})
writeTerminalOutput(terminal, 'c__ECHO__', {
foreground: true,
latencySensitive: false
})
expect(prioritizeTerminalInput(terminal)).toBe(true)
vi.runAllTimers()
expect(terminal.write.mock.calls.map(([data]) => data).join('')).toBe(
'\x18\x1b[0m\x1b[c__ECHO__'
)
expect(droppedDenseOutput).toHaveBeenCalledWith(`${droppableDense}${densePrefix}`)
})
it('admits only one dense SGR slice until xterm reports parse completion', async () => {
@@ -1506,6 +1544,23 @@ describe('pane terminal output scheduler', () => {
expect(terminal.write).toHaveBeenCalledTimes(2)
})
it('keeps draining background dense SGR without parse-clock blocking', async () => {
vi.useFakeTimers()
const { writeTerminalOutput } = await loadScheduler()
const terminal = createTerminal()
terminal.write.mockImplementation(() => undefined)
const dense = Array.from(
{ length: 4_096 },
(_value, index) => `\x1b[38;5;${index % 216}mX\x1b[0m`
).join('')
writeTerminalOutput(terminal, dense, { foreground: false })
vi.advanceTimersByTime(200)
expect(terminal.write.mock.calls.length).toBeGreaterThan(1)
expect(terminal.write.mock.calls.map(([data]) => data).join('')).toBe(dense)
})
it('releases dense parse pacing when terminal output is discarded', async () => {
vi.useFakeTimers()
const { discardTerminalOutput, writeTerminalOutput } = await loadScheduler()
@@ -23,6 +23,10 @@ import {
TERMINAL_OUTPUT_BACKLOG_MIN_CAP_CHARS,
terminalOutputBacklogCapChars
} from '../../../../shared/terminal-scrollback-policy'
import {
extractPartialEscapeTail,
MAX_PARTIAL_ESCAPE_TAIL_LENGTH
} from '../../../../shared/terminal-partial-escape-tail'
import { isDenseTerminalSgr } from '../../../../shared/terminal-sgr-load'
type TerminalOutputTarget = ForegroundTerminalOutputTarget
@@ -39,6 +43,7 @@ type WriteTerminalOutputOptions = {
/** Parse-deferred delivery ACK (terminal-pty-ack-gate). MUST be invoked when the chunk is parsed OR discarded by any drop path; fire-once, so double invocation is safe but omission permanently shrinks main's in-flight window. */
ackCredit?: () => void
onBackgroundBacklogDropped?: () => void
onDenseSgrBacklogDropped?: (data: string) => void
latencySensitive?: boolean
forceForegroundRefresh?: boolean
followupForegroundRefresh?: boolean
@@ -79,6 +84,7 @@ type QueueEntry = {
chunkIndex: number
queuedChars: number
onBackgroundBacklogDropped?: () => void
onDenseSgrBacklogDropped?: (data: string) => void
backgroundBacklogDropped: boolean
denseSgrQueuedChars: number
highPriority: boolean
@@ -322,6 +328,7 @@ function createQueueEntry(
chunkIndex: 0,
queuedChars: 0,
onBackgroundBacklogDropped: options.onBackgroundBacklogDropped,
onDenseSgrBacklogDropped: options.onDenseSgrBacklogDropped,
backgroundBacklogDropped: false,
denseSgrQueuedChars: 0,
highPriority: true,
@@ -793,18 +800,52 @@ function replaceBacklogWithWarning(
}
}
function dropDenseSgrChunksForInput(entry: QueueEntry): void {
function dropDenseSgrChunksForInput(entry: QueueEntry): boolean {
const retained: QueueChunk[] = []
let droppedChars = 0
let droppedRun: QueueChunk[] = []
const flushDroppedRun = (): void => {
if (droppedRun.length === 0) {
return
}
const finalChunk = droppedRun.at(-1)
if (!finalChunk) {
return
}
const data = droppedRun.map((chunk) => chunk.data).join('')
const tail = extractPartialEscapeTail(data)
const preservedTail = tail.length <= MAX_PARTIAL_ESCAPE_TAIL_LENGTH ? tail : ''
for (const chunk of droppedRun) {
chunk.ackCredit?.()
}
try {
entry.onDenseSgrBacklogDropped?.(data)
} catch (error) {
console.warn('[terminal] failed to salvage queries from dropped dense output', error)
}
retained.push({
...finalChunk,
data: `\x18\x1b[0m${preservedTail}`,
denseSgr: false,
onParsed: undefined,
ackCredit: undefined
})
droppedRun = []
}
for (let index = entry.chunkIndex; index < entry.chunks.length; index += 1) {
const chunk = entry.chunks[index]
if (chunk.denseSgr) {
droppedChars += chunk.data.length
chunk.ackCredit?.()
droppedRun.push(chunk)
} else {
flushDroppedRun()
retained.push(chunk)
}
}
flushDroppedRun()
if (droppedChars === 0) {
return false
}
recordRendererCrashBreadcrumb('terminal_input_dense_backlog_dropped', {
droppedChars,
retainedChars: entry.queuedChars - droppedChars
@@ -813,7 +854,7 @@ function dropDenseSgrChunksForInput(entry: QueueEntry): void {
clearForegroundCoalesce(entry)
entry.chunks = retained
entry.chunkIndex = 0
entry.queuedChars = Math.max(0, entry.queuedChars - droppedChars)
entry.queuedChars = retained.reduce((total, chunk) => total + chunk.data.length, 0)
entry.denseSgrQueuedChars = 0
entry.highPriority = retained.length > 0
entry.foregroundHold = false
@@ -821,6 +862,7 @@ function dropDenseSgrChunksForInput(entry: QueueEntry): void {
debugState.droppedBacklogCount++
}
recordQueueDebugPressure()
return true
}
function hasQueuedChunks(entry: QueueEntry): boolean {
@@ -966,7 +1008,7 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null
discardTerminalOutput(entry.terminal)
return null
}
const paceByParse = entry.denseSgrQueuedChars > 0
const paceByParse = entry.highPriority && entry.denseSgrQueuedChars > 0
const queuedWrite = takeQueuedChunk(
entry,
paceByParse ? DENSE_SGR_CHUNK_CHARS : BACKGROUND_CHUNK_CHARS
@@ -1129,6 +1171,7 @@ export function writeTerminalOutput(
) {
const queued = entry ?? createQueueEntry(terminal, options)
queued.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped
queued.onDenseSgrBacklogDropped ??= options.onDenseSgrBacklogDropped
queued.highPriority = true
queuedByTerminal.set(terminal, queued)
enqueueChunk(queued, data, {
@@ -1228,6 +1271,7 @@ export function writeTerminalOutput(
queuedByTerminal.set(terminal, queued)
} else {
queued.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped
queued.onDenseSgrBacklogDropped ??= options.onDenseSgrBacklogDropped
queued.highPriority = true
}
enqueueChunk(queued, data, {
@@ -1292,6 +1336,7 @@ export function writeTerminalOutput(
queuedByTerminal.set(terminal, entry)
} else {
entry.onBackgroundBacklogDropped = options.onBackgroundBacklogDropped
entry.onDenseSgrBacklogDropped ??= options.onDenseSgrBacklogDropped
}
enqueueChunk(entry, data, {
beforeWrite: options.beforeWrite,
@@ -1315,7 +1360,9 @@ export function prioritizeTerminalInput(terminal: TerminalOutputTarget): boolean
if (!entry || entry.denseSgrQueuedChars <= INPUT_ECHO_DENSE_BACKLOG_CHARS) {
return false
}
dropDenseSgrChunksForInput(entry)
if (!dropDenseSgrChunksForInput(entry)) {
return false
}
scheduleDrain(0)
return true
}
+7
View File
@@ -45,4 +45,11 @@ describe('dense terminal SGR classification', () => {
expect(`${stripped}__ECHO__`).toBe('\x18\x1b[0mred\x1b[38;5\x18\x1b[0m__ECHO__')
})
it('stops at an unterminated CSI tail', () => {
const tail = `\x1b[${'1;'.repeat(64 * 1024)}`
expect(isDenseTerminalSgr(tail)).toBe(false)
expect(stripTerminalSgr(`\x1b[31mred${tail}`)).toBe(`\x18\x1b[0mred${tail}\x18\x1b[0m`)
})
})
+15 -2
View File
@@ -1,25 +1,33 @@
const TERMINAL_SGR_CLASSIFICATION_MAX_CHARS = 16 * 1024
export function isDenseTerminalSgr(data: string): boolean {
let sgrSequences = 0
let textChars = 0
const limit = Math.min(data.length, TERMINAL_SGR_CLASSIFICATION_MAX_CHARS)
for (let index = 0; index < data.length; index += 1) {
for (let index = 0; index < limit; index += 1) {
if (data[index] !== '\x1b' || data[index + 1] !== '[') {
textChars += 1
continue
}
let cursor = index + 2
while (cursor < data.length) {
let terminated = false
while (cursor < limit) {
const code = data.charCodeAt(cursor)
if (code >= 0x40 && code <= 0x7e) {
if (data[cursor] === 'm') {
sgrSequences += 1
}
index = cursor
terminated = true
break
}
cursor += 1
}
if (!terminated) {
break
}
}
return sgrSequences >= 32 && sgrSequences * 2 >= textChars
@@ -34,6 +42,7 @@ export function stripTerminalSgr(data: string): string {
continue
}
let cursor = index + 2
let terminated = false
while (cursor < data.length) {
const code = data.charCodeAt(cursor)
if (code >= 0x40 && code <= 0x7e) {
@@ -42,10 +51,14 @@ export function stripTerminalSgr(data: string): string {
copyFrom = cursor + 1
}
index = cursor
terminated = true
break
}
cursor += 1
}
if (!terminated) {
break
}
}
// Boundary CAN/reset prevents carried parser or style state from swallowing retained bytes.