From e6e17a780c4e477e4340a0da0d406b148c2e83f4 Mon Sep 17 00:00:00 2001 From: s-celles Date: Fri, 24 Jul 2026 23:29:45 +0200 Subject: [PATCH 01/10] fix(terminal): recover the write queue from a lost xterm write callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A queue item completes only when its write closure invokes the callback it is handed, and in the session write path that callback is wired to xterm's term.write(data, cb). When xterm accepts a write, parses it (its buffer drains to empty), yet never fires cb — observed against a full-screen DEC 2026 TUI on macOS, window visible and focused — the item never completes: queue.active stays set, queue.writing stays true, and every item behind it is stranded with nothing scheduled to resume it. Measured during a live freeze (writes issued vs callbacks fired instrumented on the term): term.write issued 103, callbacks fired 102 -> one callback lost xterm: _pendingData 0, _writeBuffer 0, _callbacks 0, not disposed queue: writing true, active present, no stepTimer/continuation/drainTimer 18 items / ~147 KB stranded behind it This is where the permanent freeze comes from. The completion callback is also where flow.written() and the IPC ack live, so a lost callback leaks the flow-control backlog: it never drains below the low watermark, the main process pauses the PTY at the high watermark and never resumes, and the TUI's own writes then block — its render loop stalls and its keyboard goes dead. The existing large-write watchdog does not help: it only flushes xterm's write buffer, which is already empty. Add a stall watchdog to the write queue. It fires only when the same item is still active, has made no progress since it was armed, and has nothing scheduled to advance it — a genuine wedge, never a slow-but-progressing drain. On fire it marks the item cancelled (so a late real callback is a no-op), acknowledges its unacknowledged bytes through the drop handler so flow control resumes, and advances the queue. A per-item progress sequence re-arms it across the steps of a multi-step item, so a stall on a later step is covered as well as the first. Timeout is 250ms, aligned with LARGE_WRITE_FLUSH_WATCHDOG_MS: above any legitimate single-write callback latency, short enough that a recovered stall reads as a brief hitch rather than a freeze. This is a safety net against a lost xterm.js write callback, not a cure for the loss itself; it converts a permanent freeze with a dead keyboard into a recoverable hitch. Assisted by AI. --- .../terminal/runtime/terminalWriteQueue.ts | 100 +++++++++++++++ .../terminalWriteQueueWatchdog.test.ts | 117 ++++++++++++++++++ 2 files changed, 217 insertions(+) create mode 100644 components/terminal/runtime/terminalWriteQueueWatchdog.test.ts diff --git a/components/terminal/runtime/terminalWriteQueue.ts b/components/terminal/runtime/terminalWriteQueue.ts index f1e8c9dad..b2a76c5f9 100644 --- a/components/terminal/runtime/terminalWriteQueue.ts +++ b/components/terminal/runtime/terminalWriteQueue.ts @@ -12,6 +12,24 @@ export const MAX_WRITE_QUEUE_BYTES = 512 * 1024; */ export const WRITE_QUEUE_TURN_BUDGET_MS = 10; +/** + * How long an active queue item may make no progress, with nothing scheduled to + * make any, before it is force-completed. + * + * The completion of an item depends on its write closure invoking the callback + * it is handed, which in the real pipeline is wired to xterm's + * `term.write(data, cb)`. A lost `cb` — xterm accepting and parsing a write yet + * never firing its callback — would otherwise wedge the queue permanently. + * + * Set above any legitimate single-write callback latency (a callback fires + * within a frame or two) yet short enough that a recovered stall reads as a + * brief hitch rather than a second-long freeze. The watchdog additionally fires + * only when nothing is scheduled, so a slow-but-progressing queue is never + * disturbed. Aligned with {@link LARGE_WRITE_FLUSH_WATCHDOG_MS} in the session + * write path, which guards the same kind of stuck-write condition. + */ +export const WRITE_QUEUE_STALL_TIMEOUT_MS = 250; + export type TerminalWriteQueueOptions = { onDropped?: (bytes: number) => void; dropBytes?: number; @@ -49,6 +67,8 @@ type TerminalWriteQueue = { drainTimer?: ReturnType; stepTimer?: ReturnType; stepContinuation?: () => void; + stallWatchdog?: ReturnType; + progressSeq: number; }; const terminalWriteQueues = new WeakMap(); @@ -65,6 +85,7 @@ const getOrCreateQueue = (term: XTerm): TerminalWriteQueue => { floodMode: false, turnStartedAt: 0, onDropped: terminalWriteQueueDropHandlers.get(term), + progressSeq: 0, }; terminalWriteQueues.set(term, queue); } @@ -86,6 +107,68 @@ const isQueueTurnBudgetExceeded = (queue: TerminalWriteQueue): boolean => { return performance.now() - queue.turnStartedAt >= WRITE_QUEUE_TURN_BUDGET_MS; }; +const clearStallWatchdog = (queue: TerminalWriteQueue): void => { + if (queue.stallWatchdog !== undefined) { + clearTimeout(queue.stallWatchdog); + queue.stallWatchdog = undefined; + } +}; + +/** + * Unacknowledged bytes of a stalled item: the dispatched-but-uncompleted step + * and everything after it. Steps before it already fired their callbacks (and + * with them `flow.written`), so counting the whole item would double-ack them. + * `nextIndex` points one past the dispatched step, hence `nextIndex - 1`. + */ +const unackedBytesOf = (item: QueuedWrite): number => { + const from = Math.max(0, item.nextIndex - 1); + const remaining = item.steps + .slice(from) + .reduce((sum, step) => sum + step.dropBytes, 0); + return remaining > 0 ? remaining : item.dropBytes; +}; + +/** + * Arm a watchdog against a lost write callback wedging the queue. It fires only + * when the same item is still active, has made no progress since it was armed, + * and has nothing scheduled to advance it — a genuine stall, never a + * slow-but-progressing drain. On fire it acknowledges the item's unacked bytes + * (so flow control resumes) and advances the queue. + */ +const armStallWatchdog = ( + term: XTerm, + queue: TerminalWriteQueue, + item: QueuedWrite, +): void => { + clearStallWatchdog(queue); + const progressAtArm = queue.progressSeq; + queue.stallWatchdog = setTimeout(() => { + queue.stallWatchdog = undefined; + if (terminalWriteQueues.get(term) !== queue) return; + if ( + queue.active !== item + || item.cancelled + || queue.progressSeq !== progressAtArm + || queue.stepTimer !== undefined + || queue.stepContinuation !== undefined + || queue.drainTimer !== undefined + ) { + // Either the item advanced/completed, or something is scheduled to make + // it advance. Not a stall — leave it be. + return; + } + const unacked = unackedBytesOf(item); + // Mark cancelled so a late real callback becomes a no-op, then advance. + item.cancelled = true; + queue.active = undefined; + queue.drainBytes = 0; + if (unacked > 0) { + queue.onDropped?.(unacked); + } + scheduleQueueDrain(term, queue, true); + }, WRITE_QUEUE_STALL_TIMEOUT_MS); +}; + const scheduleQueueDrain = ( term: XTerm, queue: TerminalWriteQueue, @@ -109,6 +192,8 @@ const scheduleNextTerminalWrite = (term: XTerm, queue: TerminalWriteQueue) => { queue.writing = false; queue.drainBytes = 0; queue.floodMode = false; + queue.active = undefined; + clearStallWatchdog(queue); endQueueTurn(queue); if (terminalWriteQueues.get(term) === queue) { terminalWriteQueues.delete(term); @@ -137,10 +222,12 @@ const scheduleNextTerminalWrite = (term: XTerm, queue: TerminalWriteQueue) => { if (queue.pendingBytes < 0) queue.pendingBytes = 0; queue.writing = true; queue.active = next; + armStallWatchdog(term, queue, next); runQueuedWrite(next, () => { if (queue.active !== next) { return; } + clearStallWatchdog(queue); queue.drainBytes += next.bytes; if (queue.active === next) { queue.active = undefined; @@ -166,6 +253,13 @@ const scheduleNextTerminalWrite = (term: XTerm, queue: TerminalWriteQueue) => { }, 0); queue.stepTimer = timer; queue.stepContinuation = continuation; + }, () => { + // Step progress: advance the sequence and re-arm the watchdog so a stall on + // a later step of a multi-step item is covered as well as the first. + if (queue.active === next) { + queue.progressSeq += 1; + armStallWatchdog(term, queue, next); + } }); }; @@ -182,6 +276,7 @@ const runQueuedWrite = ( item: QueuedWrite, done: () => void, deferStep: (continuation: () => void) => void, + onStepProgress?: () => void, ): void => { let index = 0; let completed = false; @@ -226,6 +321,10 @@ const runQueuedWrite = ( let callbackCalledSynchronously = false; let insideWrite = true; const continueAfterStep = (): void => { + // A completed step is progress: re-arm the stall watchdog against the new + // step so a lost callback on a *later* step of a multi-step item is + // covered too, not only the first. + onStepProgress?.(); currentDrainBytes += step.bytes; // Inter-step yields honor per-step flags only. item.yieldAfter applies // after the whole item finishes (see scheduleNextTerminalWrite), so that @@ -462,6 +561,7 @@ export const abortTerminalWriteQueue = ( queue.stepTimer = undefined; } queue.stepContinuation = undefined; + clearStallWatchdog(queue); terminalWriteQueues.delete(term); if (droppedBytes > 0) { diff --git a/components/terminal/runtime/terminalWriteQueueWatchdog.test.ts b/components/terminal/runtime/terminalWriteQueueWatchdog.test.ts new file mode 100644 index 000000000..7a5b16a44 --- /dev/null +++ b/components/terminal/runtime/terminalWriteQueueWatchdog.test.ts @@ -0,0 +1,117 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { Terminal as XTerm } from "@xterm/xterm"; + +import { + enqueueTerminalWrite, + setTerminalWriteQueueDropHandler, + WRITE_QUEUE_STALL_TIMEOUT_MS, +} from "./terminalWriteQueue"; + +/** + * Recovery from a lost xterm write callback. + * + * A queue item completes only when its `write` closure calls the `done` it is + * handed, and that `done` is wired to xterm's `term.write(data, cb)` callback. + * If xterm accepts the write, parses it (its buffer drains to empty), yet never + * invokes `cb` — observed against a full-screen DEC 2026 TUI — the item never + * completes: `queue.active` stays set, `queue.writing` stays true, and every + * item behind it is stranded. Nothing reschedules it, because the queue is + * waiting on a callback that will never arrive. + * + * Downstream this is a permanent freeze: the completion callback is where + * `flow.written()` and the IPC ack live (terminalSessionAttachment), so the + * renderer backlog never drains, the main process pauses the PTY at the high + * watermark, and a TUI's own writes then block — its render loop stalls and its + * keyboard goes dead. + * + * The queue must not depend solely on xterm's callback. A stall watchdog + * force-completes an active item that has made no progress and has nothing + * scheduled to make any, acknowledging its bytes so flow control recovers. + */ + +const settle = (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)); + +const makeTerm = (): XTerm => ({}) as XTerm; + +test("a write whose callback never fires does not wedge the queue forever", async () => { + const term = makeTerm(); + const ran: number[] = []; + const dropped: number[] = []; + setTerminalWriteQueueDropHandler(term, (bytes) => dropped.push(bytes)); + + // The first write simulates a lost xterm callback: it runs, but its `done` + // is never called. + enqueueTerminalWrite(term, 4096, () => { ran.push(0); }); + + // Ordinary writes queued behind the stalled one. + for (let i = 1; i <= 5; i += 1) { + enqueueTerminalWrite(term, 4096, (done) => { + ran.push(i); + setTimeout(done, 0); + }); + } + + // Before the watchdog: only the stalled write has run; the rest are stuck. + await settle(50); + assert.deepEqual(ran, [0], "writes behind a stalled one must not run yet"); + + // After the watchdog fires: the queue recovers and drains the rest in order. + await settle(WRITE_QUEUE_STALL_TIMEOUT_MS + 300); + assert.deepEqual( + ran, [0, 1, 2, 3, 4, 5], + "queue must recover and drain the stranded writes" + ); + assert.ok( + dropped.reduce((a, b) => a + b, 0) > 0, + "the stalled item's bytes must be acknowledged so flow control resumes" + ); +}); + +test("the watchdog leaves a healthy queue untouched", async () => { + const term = makeTerm(); + const ran: number[] = []; + const dropped: number[] = []; + setTerminalWriteQueueDropHandler(term, (bytes) => dropped.push(bytes)); + + // Every write completes normally, some slowly but well within the timeout. + for (let i = 0; i < 8; i += 1) { + enqueueTerminalWrite(term, 4096, (done) => { + ran.push(i); + setTimeout(done, 5); + }, { yieldAfter: i % 2 === 0 }); + } + + await settle(WRITE_QUEUE_STALL_TIMEOUT_MS + 300); + assert.deepEqual(ran, [0, 1, 2, 3, 4, 5, 6, 7], "all writes ran"); + assert.equal( + dropped.reduce((a, b) => a + b, 0), 0, + "nothing must be dropped when the queue is healthy" + ); +}); + +test("a late callback after a watchdog recovery does not double-advance", async () => { + const term = makeTerm(); + const ran: number[] = []; + let stalledDone: (() => void) | undefined; + + // First write captures its done without calling it, then fires it LATE — + // after the watchdog has already force-completed the item. + enqueueTerminalWrite(term, 4096, (done) => { ran.push(0); stalledDone = done; }); + for (let i = 1; i <= 3; i += 1) { + enqueueTerminalWrite(term, 4096, (done) => { + ran.push(i); + setTimeout(done, 0); + }); + } + + await settle(WRITE_QUEUE_STALL_TIMEOUT_MS + 300); + const afterRecovery = [...ran]; + // The real callback finally arrives; it must be a harmless no-op. + stalledDone?.(); + await settle(100); + + assert.deepEqual(afterRecovery, [0, 1, 2, 3], "queue recovered before the late callback"); + assert.deepEqual(ran, [0, 1, 2, 3], "a late callback must not re-run or duplicate writes"); +}); From 2358d280ece129cbe1784bf5f33bafe4b221970d Mon Sep 17 00:00:00 2001 From: s-celles Date: Fri, 24 Jul 2026 21:24:00 +0200 Subject: [PATCH 02/10] feat(terminal): expose allowTransparency as a setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was hardcoded to false in xtermPerformance.ts with no way to change it, while rendererType and minimumContrastRatio are both exposed. It matters for content that varies the background per cell — animated backgrounds, heatmaps, ANSI art. The WebGL glyph cache is keyed on the background colour, so an opaque atlas bakes the background into every tile and each glyph is re-rasterised for every background it is drawn over. Measured against a real animated-background stream at 200x55: 114,496 rasterisations and 28 atlas pages, 65 ms per frame of glyph rasterisation alone. Defaults to false, so behaviour is unchanged unless the user opts in. Assisted by AI. --- application/i18n/locales/en/core.ts | 2 ++ application/syncPayload.ts | 1 + components/settings/tabs/SettingsTerminalTab.tsx | 9 +++++++++ components/terminal/runtime/createXTermRuntime.ts | 5 +++++ components/terminal/useTerminalEffects.ts | 2 ++ domain/models/terminal.ts | 8 ++++++++ 6 files changed, 27 insertions(+) diff --git a/application/i18n/locales/en/core.ts b/application/i18n/locales/en/core.ts index fd504323c..1320b42c4 100644 --- a/application/i18n/locales/en/core.ts +++ b/application/i18n/locales/en/core.ts @@ -677,6 +677,8 @@ Highlight the focused split pane: 'settings.terminal.rendering.renderer': 'Renderer', 'settings.terminal.rendering.renderer.desc': 'Choose the terminal rendering technology. Auto will use DOM on low-memory devices. Changes take effect on new terminal sessions.', 'settings.terminal.rendering.auto': 'Auto', + 'settings.terminal.rendering.allowTransparency': 'Allow transparency', + 'settings.terminal.rendering.allowTransparency.desc': 'Rasterise glyphs onto a transparent tile rather than onto their background colour. Costs a little quality on low-DPI displays, but lets one cached glyph serve every background it is drawn over — which keeps content that changes the background per cell (animated backgrounds, heatmaps, ANSI art) from re-rasterising every glyph on every frame.', 'settings.terminal.rendering.hibernateHiddenTabs': 'Hibernate hidden tabs', 'settings.terminal.rendering.hibernateHiddenTabs.desc': 'Dispose the terminal renderer for off-screen tabs to save memory while keeping the SSH session connected. Skipped during file transfers.', 'settings.terminal.rendering.hibernateHiddenTabsDelay': 'Hibernate delay', diff --git a/application/syncPayload.ts b/application/syncPayload.ts index e41b7d205..3c89ae05f 100644 --- a/application/syncPayload.ts +++ b/application/syncPayload.ts @@ -202,6 +202,7 @@ const SYNCABLE_TERMINAL_KEYS = [ 'scrollback', 'drawBoldInBrightColors', 'terminalEmulationType', 'fontLigatures', 'fontSmoothing', 'fontWeight', 'fontWeightBold', 'fallbackFont', 'linePadding', 'cursorShape', 'cursorBlink', 'minimumContrastRatio', + 'allowTransparency', 'altAsMeta', 'optionArrowWordJump', 'shiftEnterNewlineEnabled', 'shiftEnterNewlineText', 'kittyKeyboardProtocolEnabled', 'scrollOnInput', 'scrollOnOutput', 'scrollOnKeyPress', 'scrollOnPaste', diff --git a/components/settings/tabs/SettingsTerminalTab.tsx b/components/settings/tabs/SettingsTerminalTab.tsx index 449bb8672..47a0499b6 100644 --- a/components/settings/tabs/SettingsTerminalTab.tsx +++ b/components/settings/tabs/SettingsTerminalTab.tsx @@ -1049,6 +1049,15 @@ function SettingsTerminalTab(props: { className="w-32" /> + + updateTerminalSetting("allowTransparency", v)} + /> + Date: Fri, 24 Jul 2026 21:24:00 +0200 Subject: [PATCH 03/10] fix(terminal): hold back only the split escape at a chunk boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit splitPendingMarkerSuffix looked for the LONGEST suffix accepted by isIncompleteEscapePrefix, but that predicate walks forward across complete sequences and reports the incomplete one it eventually reaches. It therefore answers true for every longer suffix that merely contains the incomplete tail — including the one starting at the chunk's first ESC. On escape-dense output the first ESC sits within a few bytes of the chunk start, so virtually the whole chunk was withheld as "pending incomplete escape", and it accumulated across chunks until one happened to end on a clean boundary. Measured on 12 MB of real TUI output in 64 KB chunks: 121 of 187 chunks emitted nothing, up to 14 consecutive chunks emitted nothing, state.pending reached 917 KB, and xterm then received a single 949 KB burst. Scanning shortest-suffix-first instead gives 0 empty emits, a maximum pending of 18 bytes, and byte-identical output. The last ESC in the chunk is the real split point: an incomplete CSI cannot contain a further ESC. Assisted by AI. --- .../runtime/filterSyncBlockClears.test.ts | 50 +++++++++++++++++++ .../terminal/runtime/filterSyncBlockClears.ts | 15 +++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/components/terminal/runtime/filterSyncBlockClears.test.ts b/components/terminal/runtime/filterSyncBlockClears.test.ts index 9a5f97259..c7c259ecd 100644 --- a/components/terminal/runtime/filterSyncBlockClears.test.ts +++ b/components/terminal/runtime/filterSyncBlockClears.test.ts @@ -242,3 +242,53 @@ test("isTerminalViewportScrolledUp is false on alternate screen", () => { test("isTerminalViewportScrolledUp is false when buffer is missing", () => { assert.equal(isTerminalViewportScrolledUp({ rows: 24 } as never), false); }); + +/** + * Chunk boundaries that land mid-escape must hold back only the split + * sequence. Scanning longest-suffix-first held back everything from the + * chunk's *first* ESC, so escape-dense frames (60fps TUI, per-cell truecolor + * backgrounds) starved xterm for whole chunks at a time and released ~1MB + * bursts once a chunk happened to end cleanly. + */ +const escapeDenseFrame = (cells: number): string => { + let out = SYNC_START + "\x1b[1;1H"; + for (let i = 0; i < cells; i += 1) { + out += `\x1b[0m\x1b[38;5;245m\x1b[48;2;${i % 256};128;200m#`; + } + return out + SYNC_END; +}; + +test("a mid-escape chunk boundary holds back only the split sequence", () => { + const state = createSyncBlockFilterState(); + const frame = escapeDenseFrame(4000); + // Split inside the trailing `\x1b[48;2;...m` of some cell. + const cut = frame.lastIndexOf("\x1b[48;2;", frame.length - 40) + 6; + const emitted = filterSyncBlockClears(frame.slice(0, cut), state); + + assert.ok( + state.pending.length < 32, + `pending should hold one partial sequence, held ${state.pending.length} bytes`, + ); + assert.equal(emitted + state.pending, frame.slice(0, cut)); +}); + +test("escape-dense frames stream through without withholding whole chunks", () => { + const state = createSyncBlockFilterState(); + const stream = escapeDenseFrame(4000) + escapeDenseFrame(4000); + const CHUNK = 8192; + let emitted = ""; + let emptyEmits = 0; + + for (let i = 0; i < stream.length; i += CHUNK) { + const output = filterSyncBlockClears(stream.slice(i, i + CHUNK), state); + if (output.length === 0) emptyEmits += 1; + emitted += output; + assert.ok( + state.pending.length < CHUNK, + `pending must not accumulate across chunks, reached ${state.pending.length} bytes`, + ); + } + + assert.equal(emptyEmits, 0, "every chunk should release data to xterm"); + assert.equal(emitted + state.pending, stream); +}); diff --git a/components/terminal/runtime/filterSyncBlockClears.ts b/components/terminal/runtime/filterSyncBlockClears.ts index 082fe23d5..915980bb5 100644 --- a/components/terminal/runtime/filterSyncBlockClears.ts +++ b/components/terminal/runtime/filterSyncBlockClears.ts @@ -152,7 +152,20 @@ const splitPendingMarkerSuffix = (input: string): { emit: string; pending: strin // Only suffixes that start with ESC can qualify; skip other start positions // with a charCode probe so no substring is allocated for them. - for (let length = input.length; length > 0; length -= 1) { + // + // Scan SHORTEST suffix first (from the end of the chunk backwards). + // `isIncompleteEscapePrefix` walks forward across *complete* sequences and + // only reports the incomplete one it eventually reaches, so it also answers + // `true` for every longer suffix that merely contains the incomplete tail — + // including the one starting at the chunk's first ESC. Scanning longest-first + // therefore held back everything from that first ESC onwards instead of just + // the split sequence. On escape-dense output (a 60fps TUI painting per-cell + // truecolor backgrounds) the first ESC sits within a few bytes of the chunk + // start, so whole chunks were withheld and `state.pending` grew across chunks + // until one happened to end on a clean boundary — xterm then received nothing + // for hundreds of ms followed by a ~1MB burst. The last ESC in the chunk is + // the real split point: an incomplete CSI cannot contain a further ESC. + for (let length = 1; length <= input.length; length += 1) { if (input.charCodeAt(input.length - length) !== 0x1b) { continue; } From 0ff11a9a078e95ec91515caa089ba1a2284592c5 Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 00:13:48 +0200 Subject: [PATCH 04/10] fix(terminal): keep DEC 2026 frames whole when slicing coalesced output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write coalescer's slicer recognises alt-screen DECSET toggles (47, 1047, 1049) as frame boundaries but is blind to DEC 2026 synchronized output. A modern full-screen TUI enters the alternate screen once, then delimits every frame with `\x1b[?2026h` … `\x1b[?2026l`, so the slicer sees no boundary after startup and cuts the continuous frame stream by byte size — landing shard boundaries inside frames. That tears the display: a shard is handed to xterm across a setTimeout gap, and if the rest of the frame arrives after xterm's 1000ms synchronized-output timeout, xterm force-flushes a partial frame. Add a frame-boundary helper (syncFrameBoundary.ts) and apply it as the last step of slice-end selection: a cut that would fall inside an open 2026 block is pushed forward to just past its close, and an unterminated trailing frame is held whole for the next write. It only ever extends a slice, so it composes with the existing boundary rules. Assisted by AI. --- .../runtime/syncFrameBoundary.test.ts | 69 +++++++++++++++++ .../terminal/runtime/syncFrameBoundary.ts | 74 +++++++++++++++++++ .../runtime/terminalWriteCoalescer.ts | 8 ++ 3 files changed, 151 insertions(+) create mode 100644 components/terminal/runtime/syncFrameBoundary.test.ts create mode 100644 components/terminal/runtime/syncFrameBoundary.ts diff --git a/components/terminal/runtime/syncFrameBoundary.test.ts b/components/terminal/runtime/syncFrameBoundary.test.ts new file mode 100644 index 000000000..24de33536 --- /dev/null +++ b/components/terminal/runtime/syncFrameBoundary.test.ts @@ -0,0 +1,69 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { frameSafeSliceEnd, isInsideSyncBlockAt } from "./syncFrameBoundary"; + +const H = "\x1b[?2026h"; +const L = "\x1b[?2026l"; + +test("isInsideSyncBlockAt: open before close is inside", () => { + const data = `${H}frame body`; + assert.equal(isInsideSyncBlockAt(data, 0, data.length), true); +}); + +test("isInsideSyncBlockAt: closed block is not inside", () => { + const data = `${H}frame${L}`; + assert.equal(isInsideSyncBlockAt(data, 0, data.length), false); +}); + +test("isInsideSyncBlockAt: point between close and next open is not inside", () => { + const data = `${H}a${L}XX${H}b${L}`; + const between = `${H}a${L}X`.length; + assert.equal(isInsideSyncBlockAt(data, 0, between), false); +}); + +test("isInsideSyncBlockAt: a DECRQM query is not a frame boundary", () => { + const data = "\x1b[?2026$pplain text"; + assert.equal(isInsideSyncBlockAt(data, 0, data.length), false); +}); + +test("frameSafeSliceEnd: a cut inside a frame extends to past its close", () => { + const frame = `${H}${"x".repeat(100)}${L}`; + const data = `${frame}${frame}`; + // Desired cut lands inside the first frame. + const cut = H.length + 50; + const end = frameSafeSliceEnd(data, 0, cut); + assert.equal(end, frame.length, "must extend to the end of the open frame"); + assert.equal(isInsideSyncBlockAt(data, 0, end), false); +}); + +test("frameSafeSliceEnd: a cut between frames is left untouched", () => { + const frame = `${H}${"x".repeat(100)}${L}`; + const data = `${frame}${frame}`; + const cut = frame.length; // exactly on the boundary + assert.equal(frameSafeSliceEnd(data, 0, cut), cut); +}); + +test("frameSafeSliceEnd: an unterminated frame is held to the end", () => { + const data = `${H}${"x".repeat(100)}`; // no close + const cut = H.length + 50; + assert.equal(frameSafeSliceEnd(data, 0, cut), data.length); +}); + +test("frameSafeSliceEnd: plain output is never adjusted", () => { + const data = "just some normal terminal output with no sync blocks"; + assert.equal(frameSafeSliceEnd(data, 0, 20), 20); +}); + +test("frameSafeSliceEnd: end at data.length is returned as-is", () => { + const data = `${H}x${L}`; + assert.equal(frameSafeSliceEnd(data, 0, data.length), data.length); +}); + +test("frameSafeSliceEnd: never moves the end backwards", () => { + const frame = `${H}${"x".repeat(100)}${L}`; + const data = `${frame}tail`; + const cut = H.length + 10; + const end = frameSafeSliceEnd(data, 0, cut); + assert.ok(end >= cut, "the adjusted end must not precede the desired end"); +}); diff --git a/components/terminal/runtime/syncFrameBoundary.ts b/components/terminal/runtime/syncFrameBoundary.ts new file mode 100644 index 000000000..727182c02 --- /dev/null +++ b/components/terminal/runtime/syncFrameBoundary.ts @@ -0,0 +1,74 @@ +/** + * DEC 2026 synchronized-output frame boundaries, for slicing terminal output + * without tearing a frame. + * + * A modern full-screen TUI (Tachikoma, and most others) enters the alternate + * screen once, then delimits every rendered frame with a DEC private mode 2026 + * synchronized-output block: `\x1b[?2026h` … full frame … `\x1b[?2026l`. xterm + * buffers rendering while the block is open and paints once on close, so a + * frame is coherent as long as its whole block reaches xterm before xterm's + * 1000ms synchronized-output timeout expires (RenderService SyncOutputHandler). + * + * The write coalescer/slicer, however, is only aware of alt-screen DECSET + * toggles — it never sees 2026 — so it slices a continuous frame stream by + * byte size and hands the shards to xterm across `setTimeout` gaps. A shard + * boundary that lands inside an open block leaves xterm mid-frame; if the rest + * of the frame arrives after the 1000ms timeout, xterm force-flushes a partial + * frame and the display tears. + * + * These helpers let the slicer keep every 2026 block whole. + */ + +const SYNC_OPEN = "\x1b[?2026h"; +const SYNC_CLOSE = "\x1b[?2026l"; + +/** + * Sync-block nesting state at `to`, scanning `data` from `from`. + * + * DEC 2026 does not nest in practice (a frame is one open/close pair), so this + * tracks "open" as a boolean latched by the most recent marker rather than a + * depth count. Returns whether an open block is still unclosed at `to`. + */ +export function isInsideSyncBlockAt(data: string, from: number, to: number): boolean { + let open = false; + let i = data.indexOf("\x1b[?2026", from); + while (i !== -1 && i < to) { + if (data.startsWith(SYNC_OPEN, i)) { + open = true; + i += SYNC_OPEN.length; + } else if (data.startsWith(SYNC_CLOSE, i)) { + open = false; + i += SYNC_CLOSE.length; + } else { + // A different `?2026` sequence (e.g. a DECRQM query `\x1b[?2026$p`) — not + // a frame boundary; step past this ESC and keep scanning. + i += 1; + } + i = data.indexOf("\x1b[?2026", i); + } + return open; +} + +/** + * A slice end at or after `desiredEnd` that never falls strictly inside an open + * DEC 2026 block. + * + * If `desiredEnd` lands inside an open frame, it is pushed forward to just past + * that frame's `\x1b[?2026l`. If the frame never closes within `data`, the end + * is pushed to `data.length` so the incomplete frame is held for the next + * write rather than emitted in pieces. + * + * Never moves the end backwards, so it composes with the slicer's other + * boundary rules (which only ever shrink a slice). + */ +export function frameSafeSliceEnd( + data: string, + offset: number, + desiredEnd: number, +): number { + if (desiredEnd >= data.length) return data.length; + if (!isInsideSyncBlockAt(data, offset, desiredEnd)) return desiredEnd; + const close = data.indexOf(SYNC_CLOSE, desiredEnd); + if (close === -1) return data.length; + return close + SYNC_CLOSE.length; +} diff --git a/components/terminal/runtime/terminalWriteCoalescer.ts b/components/terminal/runtime/terminalWriteCoalescer.ts index c71be7b2d..fa4b6abef 100644 --- a/components/terminal/runtime/terminalWriteCoalescer.ts +++ b/components/terminal/runtime/terminalWriteCoalescer.ts @@ -8,6 +8,7 @@ import { MAX_TERMINAL_WRITE_QUEUE_DRAIN_BYTES, } from "./terminalFlowConstants"; import { shouldDegradeTerminalSideWork } from "./terminalOutputPressure"; +import { frameSafeSliceEnd } from "./syncFrameBoundary"; import { createWriteCoalescer, type WriteCoalesceScheduleMode, @@ -507,6 +508,13 @@ const writeLargeTerminalBatch = ( } } } + // Never cut inside a DEC 2026 synchronized-output frame: a shard that ends + // mid-frame is handed to xterm across a setTimeout gap, and if the frame's + // close arrives after xterm's sync timeout the display tears. Applied last, + // and only ever extends the end, so it does not fight the boundary rules + // above. Bounded: it extends at most to this frame's close (or, for an + // unterminated trailing frame, holds it whole for the next write). + end = frameSafeSliceEnd(data, offset, end); const slice = data.slice(offset, end); const sliceIngress = end >= data.length ? remainingIngressBytes From 7a16b73fc132bf35aef2aab074b30d64515f29f6 Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 09:06:34 +0200 Subject: [PATCH 05/10] build(terminal): render DEC 2026 frames at full rate via an xterm patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds scripts/patch-xterm-sync-render.cjs (run from postinstall, after the existing webgl atlas patch) to render a synchronized-output frame the moment it closes instead of on the next debounced tick. xterm buffers rows while DEC 2026 synchronized output is on and, on close, schedules the paint through the render debouncer. Under a continuous full-screen animation the next frame opens a new 2026 block before that rAF fires, and `_renderRows` skips while sync is on, so the paint is dropped and the frame only appears on the 1000ms sync timeout — pinning the display at ~1fps. The patch renders synchronously when a sync buffer was just flushed, so a completed frame paints before the next can reopen the mode. Measured against a 30fps animated-background TUI: ~1fps to the frame arrival rate, coherent (no partial-frame tearing). Idempotent and marker-guarded, like patch-xterm-webgl-atlas.cjs; a version bump that moves the minified target fails the install rather than silently losing the fix. Upstreamable to xterm.js. Assisted by AI. --- package.json | 2 +- scripts/patch-xterm-sync-render.cjs | 88 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 scripts/patch-xterm-sync-render.cjs diff --git a/package.json b/package.json index 9e4914ef0..3ca6e73f4 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "pack:linux": "npm run build && cross-env NODE_OPTIONS=--disable-warning=DEP0190 electron-builder --config electron-builder.config.cjs --linux --publish=never", "pack:linux-x64": "npm run build && cross-env npm_config_arch=x64 NODE_OPTIONS=--disable-warning=DEP0190 electron-builder --config electron-builder.config.cjs --linux --x64 --publish=never", "pack:linux-arm64": "npm run build && cross-env npm_config_arch=arm64 NODE_OPTIONS=--disable-warning=DEP0190 electron-builder --config electron-builder.config.cjs --linux --arm64 --publish=never", - "postinstall": "patch-package && electron-builder install-app-deps && node scripts/rebuildPatchedNodePty.cjs && node scripts/patch-xterm-webgl-atlas.cjs", + "postinstall": "patch-package && electron-builder install-app-deps && node scripts/rebuildPatchedNodePty.cjs && node scripts/patch-xterm-webgl-atlas.cjs && node scripts/patch-xterm-sync-render.cjs", "rebuild": "electron-builder install-app-deps", "tool:cli": "node electron/cli/netcatty-tool-cli.cjs", "generate:capability-tools": "node scripts/generate-capability-tools.cjs", diff --git a/scripts/patch-xterm-sync-render.cjs b/scripts/patch-xterm-sync-render.cjs new file mode 100644 index 000000000..98eecaa2e --- /dev/null +++ b/scripts/patch-xterm-sync-render.cjs @@ -0,0 +1,88 @@ +#!/usr/bin/env node +/* global process, console */ +/** + * Render a DEC 2026 synchronized-output frame the moment it closes, instead of + * on the next debounced tick. + * + * xterm's RenderService buffers rows while synchronized output is on and, on + * close, requests a refresh that is scheduled through the render debouncer + * (requestAnimationFrame). Under a continuous full-screen animation the next + * frame opens a new 2026 block before that rAF fires, and `_renderRows` skips + * while sync is on — so the debounced paint is dropped and the frame only + * appears when the 1000ms synchronized-output timeout expires. The display is + * then pinned at ~1fps however fast frames arrive. + * + * The fix renders synchronously when a synchronized-output buffer was just + * flushed. `refreshRows(...,sync,...)` normally does + * `sync ? _renderRows(...) : _renderDebouncer.refresh(...)`; we widen the + * condition to also render synchronously when the flush returned buffered rows + * (the local holding `_syncOutputHandler.flush()`). At that point the mode is + * already off, so the completed frame paints before the next can reopen it. + * + * Upstream: https://github.com/xtermjs/xterm.js (fix pending). Applied here as a + * string patch on the minified build, like patch-xterm-webgl-atlas.cjs, so a + * version bump that moves the target surfaces as an install failure rather than + * silently losing the fix. + * + * Idempotent. + */ +"use strict"; +const fs = require("node:fs"); +const path = require("node:path"); + +const MARKER = "/*netcatty:sync-render*/"; + +// The minified `sync ? _renderRows(a,b) : _renderDebouncer.refresh(a,b,c)` +// ternary, and the `buffered` local to widen it with. Token names differ +// between the CJS and ESM builds, so each target names its own. +const TARGETS = [ + { + file: "node_modules/@xterm/xterm/lib/xterm.js", + // const r = flush(); ... i ? _renderRows(e,t) : _renderDebouncer.refresh(e,t,this._rowCount) + from: "i?this._renderRows(e,t):this._renderDebouncer.refresh(e,t,this._rowCount)", + to: "(i||r)?this._renderRows(e,t):this._renderDebouncer.refresh(e,t,this._rowCount)", + }, + { + file: "node_modules/@xterm/xterm/lib/xterm.mjs", + // let o = flush(); ... r ? _renderRows(e,t) : _renderDebouncer.refresh(e,t,this._rowCount) + from: "r?this._renderRows(e,t):this._renderDebouncer.refresh(e,t,this._rowCount)", + to: "(r||o)?this._renderRows(e,t):this._renderDebouncer.refresh(e,t,this._rowCount)", + }, +]; + +let patched = 0; +let already = 0; +let missing = 0; + +for (const { file, from, to } of TARGETS) { + const abs = path.resolve(process.cwd(), file); + let src; + try { + src = fs.readFileSync(abs, "utf8"); + } catch { + console.warn(`[patch-xterm-sync-render] skip (not found): ${file}`); + missing++; + continue; + } + const withMarker = to + MARKER; + if (src.includes(withMarker)) { + already++; + continue; + } + if (src.split(from).length - 1 === 1) { + fs.writeFileSync(abs, src.replace(from, withMarker), "utf8"); + patched++; + } else { + console.warn( + `[patch-xterm-sync-render] ERROR: sync-render ternary not found (or ambiguous) in ${file}. ` + + "Refresh the minified target before upgrading @xterm/xterm.", + ); + missing++; + } +} + +console.log( + `[patch-xterm-sync-render] patched=${patched} already=${already} missing=${missing}`, +); + +if (missing > 0) process.exitCode = 1; From 5a6a6d59c208b172bdbd94d630347c7d29e4a967 Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 09:51:53 +0200 Subject: [PATCH 06/10] perf(terminal): relax output back-pressure for local shells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A full-screen animated TUI (e.g. TryIt.jl at 60 fps) ran at only ~24 fps in a local terminal because the SSH-tuned 1 MB flow-control watermark paused the PTY several times a second. A local shell has no network to overwhelm and its source blocks on its own write when the pipe fills, so the tight watermark only throttles it. Add LOCAL_FLOW_HIGH/LOW_WATER_MARK (32 MB / 16 MB) and apply them to `protocol === "local"` sessions on both sides of the flow-control loop: the renderer's output flow controller and the main-process PTY pause/ resume in terminalFlowAck. SSH and every other protocol keep the tight default, so multi-MB remote dumps stay bounded. The raised ceiling still caps memory — it relaxes back-pressure, it does not disable it. Measured: TryIt.jl at TRY_FPS=60 now paints a fluid 59.9 fps in a local Netcatty terminal (was ~24). Assisted by AI. --- .../terminal/runtime/terminalFlowConstants.ts | 5 +++ .../runtime/terminalSessionAttachment.ts | 21 ++++++++++-- electron/bridges/terminalFlowAck.cjs | 32 +++++++++++++++---- .../config/terminalFlowConstants.json | 6 ++-- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/components/terminal/runtime/terminalFlowConstants.ts b/components/terminal/runtime/terminalFlowConstants.ts index fe4363570..de34450d3 100644 --- a/components/terminal/runtime/terminalFlowConstants.ts +++ b/components/terminal/runtime/terminalFlowConstants.ts @@ -8,6 +8,11 @@ import terminalFlowConstants from "../../../infrastructure/config/terminalFlowCo */ export const FLOW_HIGH_WATER_MARK = terminalFlowConstants.FLOW_HIGH_WATER_MARK; export const FLOW_LOW_WATER_MARK = terminalFlowConstants.FLOW_LOW_WATER_MARK; +// Relaxed watermarks for local shells, which have no network to overwhelm. +export const LOCAL_FLOW_HIGH_WATER_MARK = + terminalFlowConstants.LOCAL_FLOW_HIGH_WATER_MARK; +export const LOCAL_FLOW_LOW_WATER_MARK = + terminalFlowConstants.LOCAL_FLOW_LOW_WATER_MARK; export const FLOW_CHAR_COUNT_ACK_SIZE = terminalFlowConstants.FLOW_CHAR_COUNT_ACK_SIZE; export const MAX_PENDING_WRITE_COALESCE_BYTES = terminalFlowConstants.MAX_PENDING_WRITE_COALESCE_BYTES; diff --git a/components/terminal/runtime/terminalSessionAttachment.ts b/components/terminal/runtime/terminalSessionAttachment.ts index 8c2ee9882..0189b91b8 100644 --- a/components/terminal/runtime/terminalSessionAttachment.ts +++ b/components/terminal/runtime/terminalSessionAttachment.ts @@ -68,6 +68,8 @@ import { import { FLOW_HIGH_WATER_MARK, FLOW_LOW_WATER_MARK, + LOCAL_FLOW_HIGH_WATER_MARK, + LOCAL_FLOW_LOW_WATER_MARK, XTERM_WRITE_CALLBACK_BATCH_BYTES, XTERM_WRITE_CALLBACK_FAST_PATH_MAX_BYTES, } from "./terminalFlowConstants"; @@ -373,9 +375,24 @@ export const getFlowController = ( ): OutputFlowController => { let controller = terminalFlowControllers.get(term); if (!controller) { + // A local shell needs no output back-pressure: there is no network to + // overwhelm, and the source (a local process) blocks on its own write + // when the pipe fills. The 1 MB watermark meant for SSH otherwise pauses + // the PTY several times a second under a full-screen animated TUI, which + // throttles its frame loop far below what the renderer can paint. Give + // local sessions a much higher ceiling so the pause is rare; SSH keeps the + // tight default. The ceiling still bounds memory — it does not disable + // back-pressure, only relaxes it where a flood cannot originate. + const isLocal = (ctx.hostRef?.current ?? ctx.host)?.protocol === "local"; + const highWaterMark = isLocal + ? Math.max(FLOW_HIGH_WATER_MARK, LOCAL_FLOW_HIGH_WATER_MARK) + : FLOW_HIGH_WATER_MARK; + const lowWaterMark = isLocal + ? Math.max(FLOW_LOW_WATER_MARK, LOCAL_FLOW_LOW_WATER_MARK) + : FLOW_LOW_WATER_MARK; controller = createOutputFlowController({ - highWaterMark: FLOW_HIGH_WATER_MARK, - lowWaterMark: FLOW_LOW_WATER_MARK, + highWaterMark, + lowWaterMark, onPause: () => { const id = ctx.sessionRef.current; if (id) ctx.terminalBackend.setSessionFlowPaused?.(id, true); diff --git a/electron/bridges/terminalFlowAck.cjs b/electron/bridges/terminalFlowAck.cjs index 8c06c4afb..1a7c135ba 100644 --- a/electron/bridges/terminalFlowAck.cjs +++ b/electron/bridges/terminalFlowAck.cjs @@ -3,9 +3,25 @@ const { FLOW_HIGH_WATER_MARK, FLOW_LOW_WATER_MARK, + LOCAL_FLOW_HIGH_WATER_MARK, + LOCAL_FLOW_LOW_WATER_MARK, } = require("../../infrastructure/config/terminalFlowConstants.cjs"); const { logTerminalOutputPerf } = require("./terminalPerformanceDiagnostics.cjs"); +// A local shell has no network to overwhelm and its source blocks on write when +// the pipe fills, so the tight SSH watermark only serves to throttle it. Give +// local sessions a much higher (still bounded) ceiling; every other kind keeps +// the default. Matches the renderer-side gate in terminalSessionAttachment. +function isLocalSession(session) { + return session?.protocol === "local" || session?.type === "local"; +} +function highWaterFor(session) { + return isLocalSession(session) ? LOCAL_FLOW_HIGH_WATER_MARK : FLOW_HIGH_WATER_MARK; +} +function lowWaterFor(session) { + return isLocalSession(session) ? LOCAL_FLOW_LOW_WATER_MARK : FLOW_LOW_WATER_MARK; +} + function getFlowTarget(session) { return session?.stream || session?.proc || session?.socket || session?.serialPort || null; } @@ -36,7 +52,7 @@ function ensureFlowState(session) { state.lastPerfAckLogAt = Number.isFinite(state.lastPerfAckLogAt) ? Math.max(0, state.lastPerfAckLogAt) : 0; state.sessionId = typeof state.sessionId === "string" && state.sessionId ? state.sessionId : null; if (typeof state.outputPaused !== "boolean") { - state.outputPaused = state.appliedPause && (state.rendererPaused || state.unackedBytes >= FLOW_HIGH_WATER_MARK); + state.outputPaused = state.appliedPause && (state.rendererPaused || state.unackedBytes >= highWaterFor(session)); } return session.flowState; } @@ -71,8 +87,8 @@ function getFlowPerfDetails(session, extra = {}) { appliedPause: state.appliedPause, unackedBytes: state.unackedBytes, bufferedBytes: state.bufferedBytes, - highWaterMark: FLOW_HIGH_WATER_MARK, - lowWaterMark: FLOW_LOW_WATER_MARK, + highWaterMark: highWaterFor(session), + lowWaterMark: lowWaterFor(session), ...extra, }; } @@ -95,15 +111,17 @@ function reconcileSessionFlow(session) { const target = getFlowTarget(session); if (!target) return; - if (!state.outputPaused && (state.rendererPaused || state.unackedBytes >= FLOW_HIGH_WATER_MARK)) { + const highWaterMark = highWaterFor(session); + const lowWaterMark = lowWaterFor(session); + if (!state.outputPaused && (state.rendererPaused || state.unackedBytes >= highWaterMark)) { state.outputPaused = true; - } else if (state.outputPaused && !state.rendererPaused && state.unackedBytes <= FLOW_LOW_WATER_MARK) { + } else if (state.outputPaused && !state.rendererPaused && state.unackedBytes <= lowWaterMark) { state.outputPaused = false; } const pendingBytes = state.unackedBytes + state.bufferedBytes; - const shouldPause = state.outputPaused || pendingBytes >= FLOW_HIGH_WATER_MARK; - const shouldResume = !state.outputPaused && pendingBytes <= FLOW_LOW_WATER_MARK; + const shouldPause = state.outputPaused || pendingBytes >= highWaterMark; + const shouldResume = !state.outputPaused && pendingBytes <= lowWaterMark; if (!state.appliedPause && shouldPause) { logTerminalOutputPerf("backend-flow-pause", getFlowPerfDetails(session, { pendingBytes })); diff --git a/infrastructure/config/terminalFlowConstants.json b/infrastructure/config/terminalFlowConstants.json index 0004cdd1f..e69c78229 100644 --- a/infrastructure/config/terminalFlowConstants.json +++ b/infrastructure/config/terminalFlowConstants.json @@ -10,5 +10,7 @@ "TERMINAL_LONG_LINE_PRESSURE_BYTES": 65536, "TERMINAL_AUX_LONG_LINE_SCAN_LIMIT_CHARS": 65536, "XTERM_WRITE_CALLBACK_FAST_PATH_MAX_BYTES": 1024, - "XTERM_WRITE_CALLBACK_BATCH_BYTES": 100000 -} + "XTERM_WRITE_CALLBACK_BATCH_BYTES": 100000, + "LOCAL_FLOW_HIGH_WATER_MARK": 33554432, + "LOCAL_FLOW_LOW_WATER_MARK": 16777216 +} \ No newline at end of file From b30766cdd36b0a21df4a5aa8c2e969d9b2dedefb Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 14:36:26 +0200 Subject: [PATCH 07/10] perf(terminal): drop superseded animation frames to cap latency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A full-screen animated TUI (e.g. TryIt.jl at 60 fps) emits ~140 KB per frame, each a DEC 2026 synchronized block that homes the cursor and repaints every cell. xterm.js can render that rate, but only if it never falls more than a frame or two behind; otherwise frames queue up and the display — and the keyboard echo waiting behind it — runs up to a second late. Raising the flow-control watermark bought the frame rate at the cost of that latency; lowering it fixed the latency but throttled the animation by pausing the source. Add a frame gate on the renderer's session-data path that breaks the trade-off. When output backlog is below a few frames it forwards frames straight through, so xterm stays fed at full rate. Above that it collapses runs of superseded full-repaint frames to the last — the next frame repaints every cell the previous drew, so the earlier ones are invisible — and acknowledges the dropped bytes so flow control stays balanced. The source is never paused, so the animation keeps 60 fps while the backlog (and latency) stays bounded. Collapsing is strict: only pure visual repaints whose successor homes the cursor are dropped, never frames carrying OSC or private-mode / alternate-screen state. With the local watermark lowered to 4 MB (backstop only; the gate keeps the real backlog near 0.5 MB), TryIt.jl at TRY_FPS=60 now renders a fluid ~60 fps with ~50 ms input-to-display latency, down from ~800 ms. Assisted by AI. --- .../runtime/terminalFrameGate.test.ts | 100 ++++++++++++++ .../terminal/runtime/terminalFrameGate.ts | 129 ++++++++++++++++++ .../runtime/terminalSessionAttachment.ts | 100 ++++++++++++++ .../config/terminalFlowConstants.json | 4 +- 4 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 components/terminal/runtime/terminalFrameGate.test.ts create mode 100644 components/terminal/runtime/terminalFrameGate.ts diff --git a/components/terminal/runtime/terminalFrameGate.test.ts b/components/terminal/runtime/terminalFrameGate.test.ts new file mode 100644 index 000000000..1e0097b91 --- /dev/null +++ b/components/terminal/runtime/terminalFrameGate.test.ts @@ -0,0 +1,100 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { collapseAndSplit } from "./terminalFrameGate.ts"; + +const ON = "\x1b[?2026h"; +const OFF = "\x1b[?2026l"; +const HOME = "\x1b[1;1H"; +const frame = (paint: string) => `${ON}${HOME}${paint}${OFF}`; + +test("no frames: everything is complete, nothing held", () => { + assert.deepEqual(collapseAndSplit("plain text"), { + complete: "plain text", + partial: "", + dropped: 0, + }); +}); + +test("one complete frame passes through whole", () => { + const f = frame("A"); + assert.deepEqual(collapseAndSplit(f), { complete: f, partial: "", dropped: 0 }); +}); + +test("a trailing unterminated frame is held as partial", () => { + const a = frame("A"); + const partialFrame = `${ON}${HOME}half`; // no OFF + assert.deepEqual(collapseAndSplit(a + partialFrame), { + complete: a, + partial: partialFrame, + dropped: 0, + }); +}); + +test("collapses a run of full-repaint frames to the last", () => { + const a = frame("A"); + const b = frame("B"); + const c = frame("C"); + const r = collapseAndSplit(a + b + c); + assert.equal(r.complete, c); + assert.equal(r.partial, ""); + assert.equal(r.dropped, a.length + b.length); +}); + +test("collapses complete frames but still holds a trailing partial", () => { + const a = frame("A"); + const b = frame("B"); + const partialFrame = `${ON}${HOME}new`; + const r = collapseAndSplit(a + b + partialFrame); + assert.equal(r.complete, b); + assert.equal(r.partial, partialFrame); + assert.equal(r.dropped, a.length); +}); + +test("preserves leading and inter-frame non-frame bytes", () => { + const a = frame("A"); + const b = frame("B"); + const r = collapseAndSplit("lead" + a + b); + assert.equal(r.complete, "lead" + b); + assert.equal(r.dropped, a.length); +}); + +test("does not drop across a non-empty gap between frames", () => { + const a = frame("A"); + const b = frame("B"); + const input = a + "\r\n" + b; + assert.deepEqual(collapseAndSplit(input), { + complete: input, + partial: "", + dropped: 0, + }); +}); + +test("does not drop a frame carrying OSC or private-mode state", () => { + const osc = `${ON}${HOME}\x1b]0;t\x07x${OFF}`; + const b = frame("B"); + const input = osc + b; + assert.deepEqual(collapseAndSplit(input), { + complete: input, + partial: "", + dropped: 0, + }); +}); + +test("does not drop when the successor does not home the cursor", () => { + const a = frame("A"); + const positioned = `${ON}\x1b[5;5Hxx${OFF}`; + const input = a + positioned; + assert.deepEqual(collapseAndSplit(input), { + complete: input, + partial: "", + dropped: 0, + }); +}); + +test("keeps only the last of many frames", () => { + const frames = Array.from({ length: 10 }, (_, i) => frame(`P${i}`)); + const r = collapseAndSplit(frames.join("")); + assert.equal(r.complete, frames[frames.length - 1]); + assert.equal(r.partial, ""); +}); diff --git a/components/terminal/runtime/terminalFrameGate.ts b/components/terminal/runtime/terminalFrameGate.ts new file mode 100644 index 000000000..a69d95f38 --- /dev/null +++ b/components/terminal/runtime/terminalFrameGate.ts @@ -0,0 +1,129 @@ +/** + * Frame-rate gate for full-screen animated TUIs. + * + * A TUI like TryIt.jl emits every frame as a DEC 2026 synchronized-output block + * that homes the cursor and repaints every cell: + * + * ESC[?2026h ESC[1;1H ESC[?2026l + * + * At ~60 fps each frame is ~140 KB. xterm.js can render that rate, but only if + * it is never more than a frame or two behind — otherwise frames queue up and + * the display (and the keyboard echo waiting behind it) runs up to a second + * late. The flow-control watermark bounds that backlog by *pausing* the source, + * which throttles the animation's frame rate. This gate instead bounds it by + * *dropping* superseded frames: when several full-repaint frames are buffered, + * only the last is visible (the next repaints every cell the previous drew), so + * the earlier ones can be skipped. The source is never paused, so the animation + * keeps its full rate while the backlog — and the latency — stays small. + * + * This module is the pure buffer transform. It has no state and no side + * effects; the caller owns the per-terminal buffer and the accounting. + */ + +const SYNC_ON = "\x1b[?2026h"; +const SYNC_OFF = "\x1b[?2026l"; +const CSI = "\x1b["; + +/** A CUP/HVP param list resolves to the top-left cell (row/col 1, default, or 0). */ +const isHomeParams = (params: string): boolean => { + const parts = params.split(";"); + if (parts.length > 2) return false; + return parts.every((p) => p === "" || p === "0" || p === "1"); +}; + +/** True when `content` begins by homing the cursor to the top-left. */ +const startsWithCursorHome = (content: string): boolean => { + if (!content.startsWith(CSI)) return false; + let i = CSI.length; + while (i < content.length) { + const c = content.charCodeAt(i); + if ((c >= 0x30 && c <= 0x39) || c === 0x3b) i++; // digits and ';' + else break; + } + const final = content[i]; + return (final === "H" || final === "f") && isHomeParams(content.slice(CSI.length, i)); +}; + +/** + * A frame's payload is a pure visual repaint: only cursor moves, SGR and cell + * text. Rejects OSC (`ESC]`) and private-mode set/reset (`ESC[?…`, which + * includes alternate-screen switches) — state a successor would not restore, so + * such a frame must never be dropped. + */ +const isPureVisualPayload = (content: string): boolean => + !content.includes("\x1b]") && !content.includes("\x1b[?"); + +type Frame = { start: number; end: number; content: string }; + +/** + * Result of {@link collapseAndSplit}: + * - `complete` — the leading, collapsed run of complete frames, ready to write. + * - `partial` — a trailing, not-yet-closed frame to keep buffering. + * - `dropped` — characters removed from `complete` by collapsing. + */ +export type FrameGateSplit = { complete: string; partial: string; dropped: number }; + +/** + * Split `buffer` into its complete-frame prefix and a trailing incomplete + * frame, collapsing runs of superseded full-repaint frames in the prefix down + * to the last. A frame is dropped only when it is a pure visual repaint and the + * frame directly after it homes the cursor (so it overwrites the whole screen). + * Everything the transform is unsure about is preserved verbatim. + */ +export const collapseAndSplit = (buffer: string): FrameGateSplit => { + // Locate every complete frame; note where an unterminated trailing frame begins. + const frames: Frame[] = []; + let cursor = 0; + let partialStart = buffer.length; + while (true) { + const on = buffer.indexOf(SYNC_ON, cursor); + if (on < 0) break; + const contentStart = on + SYNC_ON.length; + const off = buffer.indexOf(SYNC_OFF, contentStart); + if (off < 0) { + partialStart = on; // trailing frame opened but not closed yet + break; + } + const end = off + SYNC_OFF.length; + frames.push({ start: on, end, content: buffer.slice(contentStart, off) }); + cursor = end; + } + + const partial = buffer.slice(partialStart); + const completeRegion = buffer.slice(0, partialStart); + + if (frames.length < 2) { + return { complete: completeRegion, partial, dropped: 0 }; + } + + // Mark a frame droppable when it is a pure visual repaint, the next frame is + // directly adjacent, and that next frame fully repaints the screen. + const drop = new Array(frames.length).fill(false); + for (let i = 0; i < frames.length - 1; i++) { + const cur = frames[i]; + const next = frames[i + 1]; + if ( + next.start === cur.end + && isPureVisualPayload(cur.content) + && startsWithCursorHome(next.content) + ) { + drop[i] = true; + } + } + if (!drop.some(Boolean)) { + return { complete: completeRegion, partial, dropped: 0 }; + } + + let complete = ""; + let dropped = 0; + let pos = 0; + for (let i = 0; i < frames.length; i++) { + const f = frames[i]; + complete += completeRegion.slice(pos, f.start); // bytes before the frame + if (drop[i]) dropped += f.end - f.start; + else complete += completeRegion.slice(f.start, f.end); + pos = f.end; + } + complete += completeRegion.slice(pos); + return { complete, partial, dropped }; +}; diff --git a/components/terminal/runtime/terminalSessionAttachment.ts b/components/terminal/runtime/terminalSessionAttachment.ts index 0189b91b8..d030a1580 100644 --- a/components/terminal/runtime/terminalSessionAttachment.ts +++ b/components/terminal/runtime/terminalSessionAttachment.ts @@ -47,6 +47,7 @@ import { resetTerminalSyncBlockFilter, } from "./terminalSyncBlockFilter"; import { appendEraseScrollbackAfterFullErases } from "../clearTerminalViewport"; +import { collapseAndSplit } from "./terminalFrameGate"; import { type CoalescedTerminalWriteOptions, enqueueCoalescedTerminalWrite, @@ -462,12 +463,109 @@ export const writeTerminalLine = ( flushTerminalWritesForBackgroundOutput(term); }; +/** + * Backlog (unacknowledged bytes awaiting xterm) below which the frame gate + * forwards frames straight through — a few frames deep, enough to keep xterm + * fed at full rate without starving. Above it the gate drops superseded frames + * instead of letting the backlog (and the latency riding behind it) grow. + */ +const FRAME_GATE_FORWARD_BACKLOG = 512 * 1024; +/** Retry cadence for draining the gate's held buffer when the backlog clears. */ +const FRAME_GATE_FLUSH_MS = 8; + +type FrameGateState = { + buffer: string; + meta?: TerminalSessionDataMeta; + flushTimer?: ReturnType; +}; +const frameGateStates = new WeakMap(); + +const getFrameGateState = (term: XTerm): FrameGateState => { + let state = frameGateStates.get(term); + if (!state) { + state = { buffer: "" }; + frameGateStates.set(term, state); + } + return state; +}; + +export const resetFrameGate = (term: XTerm): void => { + const state = frameGateStates.get(term); + if (state?.flushTimer !== undefined) clearTimeout(state.flushTimer); + frameGateStates.delete(term); +}; + +/** + * Drain as much of the gate's held buffer as the current backlog allows, + * collapsing superseded frames first. Held frames that cannot be forwarded yet + * stay buffered so the next arrival (or flush) collapses them against newer + * ones instead of letting them pile up. + */ +const drainFrameGate = ( + ctx: TerminalSessionStartersContext, + term: XTerm, +): void => { + const state = getFrameGateState(term); + if (state.flushTimer !== undefined) { + clearTimeout(state.flushTimer); + state.flushTimer = undefined; + } + if (!state.buffer) return; + + const { complete, partial, dropped } = collapseAndSplit(state.buffer); + state.buffer = partial; + if (dropped > 0) acknowledgeDroppedTerminalDisplayBytes(ctx, dropped); + + if (complete) { + const backlog = getFlowControllerForTerm(term)?.pendingBytes() ?? 0; + if (backlog < FRAME_GATE_FORWARD_BACKLOG) { + forwardSessionData(ctx, term, complete, complete.length, state.meta); + } else { + // xterm is still behind: keep the collapsed complete run buffered ahead of + // the trailing partial so newer frames supersede it rather than queue up. + state.buffer = complete + state.buffer; + } + } + + if (state.buffer && state.flushTimer === undefined) { + state.flushTimer = setTimeout(() => { + state.flushTimer = undefined; + drainFrameGate(ctx, term); + }, FRAME_GATE_FLUSH_MS); + } +}; + +/** + * Entry point for PTY output. When a full-screen animation is in flight (a DEC + * 2026 frame is present or already buffered) it runs through the frame gate, + * which caps the display/keyboard latency by dropping superseded frames. All + * other output takes the direct path unchanged. + */ export const writeSessionData = ( ctx: TerminalSessionStartersContext, term: XTerm, data: string, ingressBytes: number = data.length, meta?: TerminalSessionDataMeta, +) => { + const state = frameGateStates.get(term); + const engaged = (state && state.buffer.length > 0) || data.includes("\x1b[?2026h"); + if (!engaged) { + forwardSessionData(ctx, term, data, ingressBytes, meta); + return; + } + const gate = getFrameGateState(term); + gate.buffer += data; + gate.meta = meta; + drainFrameGate(ctx, term); +}; + +const forwardSessionData = ( + ctx: TerminalSessionStartersContext, + term: XTerm, + data: string, + ingressBytes: number = data.length, + meta?: TerminalSessionDataMeta, ) => { const flow = getFlowController(ctx, term); const isPaneCurrentlyVisible = () => isTerminalPaneVisible(ctx); @@ -807,6 +905,7 @@ export const releaseTerminalFlowBeforeHibernate = ( setTerminalWriteCoalescerFlushGate(term); pendingTimestampSecondByTerm.delete(term); resetDeferredTerminalWriteAck(term); + resetFrameGate(term); terminalFlowControllers.delete(term); }; @@ -855,6 +954,7 @@ export const attachSessionToTerminal = ( teardownTerminalOutputPipeline(ctx, term, id, flow); flushTerminalWriteCoalescer(term); resetTerminalSyncBlockFilter(term); + resetFrameGate(term); resetTerminalLineTimestamps(term); resetTerminalOutputPressure(term); ctx.onSessionAttached?.(id); diff --git a/infrastructure/config/terminalFlowConstants.json b/infrastructure/config/terminalFlowConstants.json index e69c78229..2424d4edd 100644 --- a/infrastructure/config/terminalFlowConstants.json +++ b/infrastructure/config/terminalFlowConstants.json @@ -11,6 +11,6 @@ "TERMINAL_AUX_LONG_LINE_SCAN_LIMIT_CHARS": 65536, "XTERM_WRITE_CALLBACK_FAST_PATH_MAX_BYTES": 1024, "XTERM_WRITE_CALLBACK_BATCH_BYTES": 100000, - "LOCAL_FLOW_HIGH_WATER_MARK": 33554432, - "LOCAL_FLOW_LOW_WATER_MARK": 16777216 + "LOCAL_FLOW_HIGH_WATER_MARK": 4194304, + "LOCAL_FLOW_LOW_WATER_MARK": 2097152 } \ No newline at end of file From 44a74dfcb98a8eaa42f471346d112a815d5c5832 Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 15:19:24 +0200 Subject: [PATCH 08/10] fix(terminal): harden the animation frame gate (review follow-ups) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address automated review feedback on the frame gate: - Require proof of a full repaint before dropping a frame. Homing the cursor alone does not imply a full-screen repaint — a valid incremental successor (HOME + one changed cell) would otherwise discard the earlier frame's other changes. Also require the successor's payload to reach a viewport-sized threshold (cols * rows, one byte per cell), which a real repaint always clears and an incremental update never does. - Preserve flow-control ingress accounting through the gate. Track the ingress bytes attributable to the held buffer and apportion them exactly (via complements) across forwarded / dropped / held, instead of substituting rendered-string lengths — so a plugin-transformed chunk cannot over- or under-acknowledge the backend. - Stop busy-polling a lone trailing partial frame. Only schedule the retry timer when complete output is being held back by backlog; a partial can only be completed by new session data, which drains the gate itself. A session stalled mid-frame no longer wakes ~125x/second. Adds unit coverage for the size threshold and the ingress apportioning (sum-invariant, including ingress != length). Assisted-by: AI --- .../runtime/terminalFrameGate.test.ts | 81 ++++++++++++++++--- .../terminal/runtime/terminalFrameGate.ts | 43 +++++++++- .../runtime/terminalSessionAttachment.ts | 47 +++++++++-- 3 files changed, 151 insertions(+), 20 deletions(-) diff --git a/components/terminal/runtime/terminalFrameGate.test.ts b/components/terminal/runtime/terminalFrameGate.test.ts index 1e0097b91..1b2890203 100644 --- a/components/terminal/runtime/terminalFrameGate.test.ts +++ b/components/terminal/runtime/terminalFrameGate.test.ts @@ -1,15 +1,18 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { collapseAndSplit } from "./terminalFrameGate.ts"; +import { apportionFrameGateIngress, collapseAndSplit } from "./terminalFrameGate.ts"; const ON = "\x1b[?2026h"; const OFF = "\x1b[?2026l"; const HOME = "\x1b[1;1H"; const frame = (paint: string) => `${ON}${HOME}${paint}${OFF}`; +// Most tests do not exercise the full-repaint size gate; 0 lets any homing +// successor qualify so they cover the frame-boundary logic in isolation. +const ANY = 0; test("no frames: everything is complete, nothing held", () => { - assert.deepEqual(collapseAndSplit("plain text"), { + assert.deepEqual(collapseAndSplit("plain text", ANY), { complete: "plain text", partial: "", dropped: 0, @@ -18,13 +21,13 @@ test("no frames: everything is complete, nothing held", () => { test("one complete frame passes through whole", () => { const f = frame("A"); - assert.deepEqual(collapseAndSplit(f), { complete: f, partial: "", dropped: 0 }); + assert.deepEqual(collapseAndSplit(f, ANY), { complete: f, partial: "", dropped: 0 }); }); test("a trailing unterminated frame is held as partial", () => { const a = frame("A"); const partialFrame = `${ON}${HOME}half`; // no OFF - assert.deepEqual(collapseAndSplit(a + partialFrame), { + assert.deepEqual(collapseAndSplit(a + partialFrame, ANY), { complete: a, partial: partialFrame, dropped: 0, @@ -35,7 +38,7 @@ test("collapses a run of full-repaint frames to the last", () => { const a = frame("A"); const b = frame("B"); const c = frame("C"); - const r = collapseAndSplit(a + b + c); + const r = collapseAndSplit(a + b + c, ANY); assert.equal(r.complete, c); assert.equal(r.partial, ""); assert.equal(r.dropped, a.length + b.length); @@ -45,7 +48,7 @@ test("collapses complete frames but still holds a trailing partial", () => { const a = frame("A"); const b = frame("B"); const partialFrame = `${ON}${HOME}new`; - const r = collapseAndSplit(a + b + partialFrame); + const r = collapseAndSplit(a + b + partialFrame, ANY); assert.equal(r.complete, b); assert.equal(r.partial, partialFrame); assert.equal(r.dropped, a.length); @@ -54,7 +57,7 @@ test("collapses complete frames but still holds a trailing partial", () => { test("preserves leading and inter-frame non-frame bytes", () => { const a = frame("A"); const b = frame("B"); - const r = collapseAndSplit("lead" + a + b); + const r = collapseAndSplit("lead" + a + b, ANY); assert.equal(r.complete, "lead" + b); assert.equal(r.dropped, a.length); }); @@ -63,7 +66,7 @@ test("does not drop across a non-empty gap between frames", () => { const a = frame("A"); const b = frame("B"); const input = a + "\r\n" + b; - assert.deepEqual(collapseAndSplit(input), { + assert.deepEqual(collapseAndSplit(input, ANY), { complete: input, partial: "", dropped: 0, @@ -74,7 +77,7 @@ test("does not drop a frame carrying OSC or private-mode state", () => { const osc = `${ON}${HOME}\x1b]0;t\x07x${OFF}`; const b = frame("B"); const input = osc + b; - assert.deepEqual(collapseAndSplit(input), { + assert.deepEqual(collapseAndSplit(input, ANY), { complete: input, partial: "", dropped: 0, @@ -85,7 +88,7 @@ test("does not drop when the successor does not home the cursor", () => { const a = frame("A"); const positioned = `${ON}\x1b[5;5Hxx${OFF}`; const input = a + positioned; - assert.deepEqual(collapseAndSplit(input), { + assert.deepEqual(collapseAndSplit(input, ANY), { complete: input, partial: "", dropped: 0, @@ -94,7 +97,63 @@ test("does not drop when the successor does not home the cursor", () => { test("keeps only the last of many frames", () => { const frames = Array.from({ length: 10 }, (_, i) => frame(`P${i}`)); - const r = collapseAndSplit(frames.join("")); + const r = collapseAndSplit(frames.join(""), ANY); assert.equal(r.complete, frames[frames.length - 1]); assert.equal(r.partial, ""); }); + +test("does not drop when the successor is too small to be a full repaint", () => { + // A homing but tiny successor (e.g. HOME + one changed cell) does not prove a + // full-screen repaint, so the earlier frame's other changes must survive. + const a = frame("AAAAAAAAAAAAAAAAAAAA"); + const smallSuccessor = frame("x"); // content well under the threshold + const input = a + smallSuccessor; + const threshold = 100; + assert.deepEqual(collapseAndSplit(input, threshold), { + complete: input, + partial: "", + dropped: 0, + }); +}); + +test("drops when the successor clears the full-repaint size threshold", () => { + const a = frame("A"); + const bigPaint = "y".repeat(200); + const bigSuccessor = frame(bigPaint); // content over the threshold + const input = a + bigSuccessor; + const r = collapseAndSplit(input, 100); + assert.equal(r.complete, bigSuccessor); + assert.equal(r.dropped, a.length); +}); + +test("ingress apportioning always sums back to the total", () => { + const cases: Array<[number, number, number, number, number]> = [ + // total, totalChars, forwardChars, droppedChars, heldChars + [1000, 1000, 400, 400, 200], + [1500, 1000, 400, 400, 200], // ingress != chars (plugin expanded the chunk) + [700, 1000, 400, 400, 200], // ingress != chars (plugin contracted the chunk) + [999, 1000, 333, 333, 334], // rounding stress + [0, 0, 0, 0, 0], // empty + [500, 500, 0, 0, 500], // all held + [500, 500, 500, 0, 0], // all forwarded + [500, 500, 0, 500, 0], // all dropped + ]; + for (const [total, totalChars, fwd, drop, held] of cases) { + const s = apportionFrameGateIngress(total, totalChars, fwd, drop, held); + assert.equal( + s.forward + s.dropped + s.held, + total, + `parts must sum to total for ${JSON.stringify([total, totalChars, fwd, drop, held])}`, + ); + assert.ok(s.forward >= 0 && s.dropped >= 0 && s.held >= 0, "no negative shares"); + } +}); + +test("ingress apportioning routes bytes to the right bucket in the exact case", () => { + // ingress == chars: shares equal the character counts. + assert.deepEqual(apportionFrameGateIngress(1000, 1000, 400, 400, 200), { + forward: 400, + dropped: 400, + held: 200, + }); +}); diff --git a/components/terminal/runtime/terminalFrameGate.ts b/components/terminal/runtime/terminalFrameGate.ts index a69d95f38..34442cd6a 100644 --- a/components/terminal/runtime/terminalFrameGate.ts +++ b/components/terminal/runtime/terminalFrameGate.ts @@ -63,14 +63,50 @@ type Frame = { start: number; end: number; content: string }; */ export type FrameGateSplit = { complete: string; partial: string; dropped: number }; +/** Exact three-way split of buffered ingress bytes; parts always sum to `total`. */ +export type FrameGateIngressSplit = { forward: number; dropped: number; held: number }; + +/** + * Apportion `total` flow-control ingress bytes across the forwarded, dropped and + * still-held parts of a buffer, by character share. Each share is the exact + * complement of the rounded parts before it, so the three always sum back to + * `total` regardless of rounding — the backend is never over- or + * under-acknowledged even when a chunk's ingress differs from its length. + */ +export const apportionFrameGateIngress = ( + total: number, + totalChars: number, + forwardChars: number, + droppedChars: number, + heldChars: number, +): FrameGateIngressSplit => { + const held = totalChars > 0 ? Math.round((total * heldChars) / totalChars) : 0; + const leaving = total - held; + const leavingChars = forwardChars + droppedChars; + const forward = leavingChars > 0 ? Math.round((leaving * forwardChars) / leavingChars) : 0; + const dropped = leaving - forward; + return { forward, dropped, held }; +}; + /** * Split `buffer` into its complete-frame prefix and a trailing incomplete * frame, collapsing runs of superseded full-repaint frames in the prefix down - * to the last. A frame is dropped only when it is a pure visual repaint and the - * frame directly after it homes the cursor (so it overwrites the whole screen). + * to the last. + * + * A frame is dropped only when it is a pure visual repaint AND the frame + * directly after it *demonstrably repaints the whole screen*: it homes the + * cursor and carries at least `minSuccessorRepaintBytes` of payload. Homing + * alone is not enough — DEC 2026 only makes an update atomic, it does not imply + * a full repaint, and a valid incremental successor (`HOME` + one changed cell) + * would otherwise discard the earlier frame's changes elsewhere. A real + * full-screen repaint writes at least one byte per cell, so a viewport-sized + * threshold (`cols * rows`) separates it from a small incremental update. * Everything the transform is unsure about is preserved verbatim. */ -export const collapseAndSplit = (buffer: string): FrameGateSplit => { +export const collapseAndSplit = ( + buffer: string, + minSuccessorRepaintBytes: number, +): FrameGateSplit => { // Locate every complete frame; note where an unterminated trailing frame begins. const frames: Frame[] = []; let cursor = 0; @@ -106,6 +142,7 @@ export const collapseAndSplit = (buffer: string): FrameGateSplit => { next.start === cur.end && isPureVisualPayload(cur.content) && startsWithCursorHome(next.content) + && next.content.length >= minSuccessorRepaintBytes ) { drop[i] = true; } diff --git a/components/terminal/runtime/terminalSessionAttachment.ts b/components/terminal/runtime/terminalSessionAttachment.ts index d030a1580..5b5d9b2ea 100644 --- a/components/terminal/runtime/terminalSessionAttachment.ts +++ b/components/terminal/runtime/terminalSessionAttachment.ts @@ -47,7 +47,7 @@ import { resetTerminalSyncBlockFilter, } from "./terminalSyncBlockFilter"; import { appendEraseScrollbackAfterFullErases } from "../clearTerminalViewport"; -import { collapseAndSplit } from "./terminalFrameGate"; +import { apportionFrameGateIngress, collapseAndSplit } from "./terminalFrameGate"; import { type CoalescedTerminalWriteOptions, enqueueCoalescedTerminalWrite, @@ -475,6 +475,14 @@ const FRAME_GATE_FLUSH_MS = 8; type FrameGateState = { buffer: string; + /** + * Flow-control ingress bytes attributable to `buffer`. Tracked separately + * from `buffer.length` because plugin processing can make a chunk's ingress + * differ from its rendered length; it is apportioned exactly (via complements) + * as bytes are forwarded, dropped or held so the backend is neither over- nor + * under-acknowledged. + */ + ingress: number; meta?: TerminalSessionDataMeta; flushTimer?: ReturnType; }; @@ -483,7 +491,7 @@ const frameGateStates = new WeakMap(); const getFrameGateState = (term: XTerm): FrameGateState => { let state = frameGateStates.get(term); if (!state) { - state = { buffer: "" }; + state = { buffer: "", ingress: 0 }; frameGateStates.set(term, state); } return state; @@ -512,22 +520,48 @@ const drainFrameGate = ( } if (!state.buffer) return; - const { complete, partial, dropped } = collapseAndSplit(state.buffer); + // A real full-screen repaint writes at least one byte per cell; require the + // superseding frame to clear that bar before dropping its predecessor. + const minRepaint = Math.max(0, term.cols * term.rows); + const { complete, partial, dropped } = collapseAndSplit(state.buffer, minRepaint); + + // Apportion the buffered ingress across forwarded / dropped / held so the + // backend is acknowledged in its own units, not rendered-string lengths. + const { + forward: ingressComplete, + dropped: ingressDropped, + held: ingressPartial, + } = apportionFrameGateIngress( + state.ingress, + state.buffer.length, + complete.length, + dropped, + partial.length, + ); + state.buffer = partial; - if (dropped > 0) acknowledgeDroppedTerminalDisplayBytes(ctx, dropped); + state.ingress = ingressPartial; + if (dropped > 0) acknowledgeDroppedTerminalDisplayBytes(ctx, ingressDropped); + let heldComplete = false; if (complete) { const backlog = getFlowControllerForTerm(term)?.pendingBytes() ?? 0; if (backlog < FRAME_GATE_FORWARD_BACKLOG) { - forwardSessionData(ctx, term, complete, complete.length, state.meta); + forwardSessionData(ctx, term, complete, ingressComplete, state.meta); } else { // xterm is still behind: keep the collapsed complete run buffered ahead of // the trailing partial so newer frames supersede it rather than queue up. state.buffer = complete + state.buffer; + state.ingress += ingressComplete; + heldComplete = true; } } - if (state.buffer && state.flushTimer === undefined) { + // Only a clearing backlog can release held *complete* output, so poll for it. + // A lone trailing partial can only be completed by new session data — which + // calls drainFrameGate itself — so it must never schedule a timer, or a + // session stalled mid-frame would busy-poll indefinitely. + if (heldComplete && state.flushTimer === undefined) { state.flushTimer = setTimeout(() => { state.flushTimer = undefined; drainFrameGate(ctx, term); @@ -556,6 +590,7 @@ export const writeSessionData = ( } const gate = getFrameGateState(term); gate.buffer += data; + gate.ingress += ingressBytes; gate.meta = meta; drainFrameGate(ctx, term); }; From c34c6e2822c5704d591dcfd44bd81a4a130c5fa4 Mon Sep 17 00:00:00 2001 From: s-celles Date: Sat, 25 Jul 2026 20:45:28 +0200 Subject: [PATCH 09/10] fix(terminal): prove full repaint by cell coverage, and fail open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second round of review follow-ups on the animation frame gate: - Prove a full repaint by cell coverage, not payload length. The previous `content.length >= cols*rows` check was inflated by SGR escapes, so a small SGR-heavy update could masquerade as a full repaint. Instead simulate the frame's cursor moves, cell writes and erases and count covered cells. Full animated TUIs commonly repaint only the changed cells (TryIt.jl rewrites ~60% of the grid per frame), so the bar sits at 40% of the viewport — well above the few per-cent an incremental update touches, while still recognising real animation frames. Dropping is therefore best-effort for such TUIs (a cell only the dropped frame repainted shows one frame stale, imperceptible in motion); a TUI that emits true full repaints is dropped losslessly. - Restrict droppable payloads to an allowlist (cursor moves, SGR, erase, cell text) instead of only excluding OSC and private CSI. A frame carrying a BEL, device query/report, DCS/APC, or alternate-screen toggle is no longer dropped, so its side effect is never lost. - Fail open held output. A held buffer past a cap (kept below the SSH flow watermark, so a frame larger than it cannot deadlock) is released at once, and a lone trailing partial that never completes is released after a short timeout — so a process killed mid-frame never leaves its prompt hidden. Adds unit coverage for the allowlist, the cell-coverage counter, the coverage-gated collapse, and the exact ingress apportioning. Assisted-by: AI --- .../runtime/terminalFrameGate.test.ts | 172 +++++------ .../terminal/runtime/terminalFrameGate.ts | 270 ++++++++++++------ .../runtime/terminalSessionAttachment.ts | 66 ++++- 3 files changed, 331 insertions(+), 177 deletions(-) diff --git a/components/terminal/runtime/terminalFrameGate.test.ts b/components/terminal/runtime/terminalFrameGate.test.ts index 1b2890203..3dbc4a656 100644 --- a/components/terminal/runtime/terminalFrameGate.test.ts +++ b/components/terminal/runtime/terminalFrameGate.test.ts @@ -1,33 +1,34 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { apportionFrameGateIngress, collapseAndSplit } from "./terminalFrameGate.ts"; +import { + apportionFrameGateIngress, + collapseAndSplit, + isDroppableVisualPayload, + makesFullRepaint, + viewportRepaintCoverage, +} from "./terminalFrameGate.ts"; const ON = "\x1b[?2026h"; const OFF = "\x1b[?2026l"; const HOME = "\x1b[1;1H"; const frame = (paint: string) => `${ON}${HOME}${paint}${OFF}`; -// Most tests do not exercise the full-repaint size gate; 0 lets any homing -// successor qualify so they cover the frame-boundary logic in isolation. -const ANY = 0; +// Most collapse tests isolate the frame-boundary logic; a permissive predicate +// treats every homing successor as a full repaint. +const always = () => true; test("no frames: everything is complete, nothing held", () => { - assert.deepEqual(collapseAndSplit("plain text", ANY), { + assert.deepEqual(collapseAndSplit("plain text", always), { complete: "plain text", partial: "", dropped: 0, }); }); -test("one complete frame passes through whole", () => { - const f = frame("A"); - assert.deepEqual(collapseAndSplit(f, ANY), { complete: f, partial: "", dropped: 0 }); -}); - test("a trailing unterminated frame is held as partial", () => { const a = frame("A"); - const partialFrame = `${ON}${HOME}half`; // no OFF - assert.deepEqual(collapseAndSplit(a + partialFrame, ANY), { + const partialFrame = `${ON}${HOME}half`; + assert.deepEqual(collapseAndSplit(a + partialFrame, always), { complete: a, partial: partialFrame, dropped: 0, @@ -38,9 +39,8 @@ test("collapses a run of full-repaint frames to the last", () => { const a = frame("A"); const b = frame("B"); const c = frame("C"); - const r = collapseAndSplit(a + b + c, ANY); + const r = collapseAndSplit(a + b + c, always); assert.equal(r.complete, c); - assert.equal(r.partial, ""); assert.equal(r.dropped, a.length + b.length); }); @@ -48,109 +48,113 @@ test("collapses complete frames but still holds a trailing partial", () => { const a = frame("A"); const b = frame("B"); const partialFrame = `${ON}${HOME}new`; - const r = collapseAndSplit(a + b + partialFrame, ANY); + const r = collapseAndSplit(a + b + partialFrame, always); assert.equal(r.complete, b); assert.equal(r.partial, partialFrame); assert.equal(r.dropped, a.length); }); -test("preserves leading and inter-frame non-frame bytes", () => { +test("does not drop across a non-empty gap between frames", () => { const a = frame("A"); const b = frame("B"); - const r = collapseAndSplit("lead" + a + b, ANY); - assert.equal(r.complete, "lead" + b); - assert.equal(r.dropped, a.length); + const input = a + "\r\n" + b; + assert.deepEqual(collapseAndSplit(input, always), { complete: input, partial: "", dropped: 0 }); }); -test("does not drop across a non-empty gap between frames", () => { - const a = frame("A"); +test("does not drop when the successor is not a full repaint", () => { + const a = frame("AAAA"); const b = frame("B"); - const input = a + "\r\n" + b; - assert.deepEqual(collapseAndSplit(input, ANY), { - complete: input, - partial: "", - dropped: 0, - }); + const input = a + b; + // Predicate rejects the successor → nothing collapses. + assert.deepEqual(collapseAndSplit(input, () => false), { complete: input, partial: "", dropped: 0 }); }); -test("does not drop a frame carrying OSC or private-mode state", () => { - const osc = `${ON}${HOME}\x1b]0;t\x07x${OFF}`; +test("does not drop a frame whose payload is not purely visual", () => { + const bell = `${ON}${HOME}ping\x07${OFF}`; // carries a BEL const b = frame("B"); - const input = osc + b; - assert.deepEqual(collapseAndSplit(input, ANY), { - complete: input, - partial: "", - dropped: 0, - }); + const input = bell + b; + assert.deepEqual(collapseAndSplit(input, always), { complete: input, partial: "", dropped: 0 }); }); -test("does not drop when the successor does not home the cursor", () => { - const a = frame("A"); - const positioned = `${ON}\x1b[5;5Hxx${OFF}`; - const input = a + positioned; - assert.deepEqual(collapseAndSplit(input, ANY), { - complete: input, - partial: "", - dropped: 0, - }); +// ---- isDroppableVisualPayload --------------------------------------------- + +test("droppable payload: cursor moves, SGR, erase and text are allowed", () => { + assert.equal(isDroppableVisualPayload("\x1b[1;1H\x1b[38;5;5mabc\x1b[K"), true); + assert.equal(isDroppableVisualPayload("\r\n\tplain text"), true); + assert.equal(isDroppableVisualPayload("\x1b[2Jfull"), true); }); -test("keeps only the last of many frames", () => { - const frames = Array.from({ length: 10 }, (_, i) => frame(`P${i}`)); - const r = collapseAndSplit(frames.join(""), ANY); - assert.equal(r.complete, frames[frames.length - 1]); - assert.equal(r.partial, ""); +test("un-droppable payload: side-effecting sequences are rejected", () => { + assert.equal(isDroppableVisualPayload("bell\x07"), false, "BEL"); + assert.equal(isDroppableVisualPayload("\x1b]0;title\x07"), false, "OSC"); + assert.equal(isDroppableVisualPayload("\x1b[6n"), false, "device status report query"); + assert.equal(isDroppableVisualPayload("\x1b[?25l"), false, "private mode"); + assert.equal(isDroppableVisualPayload("\x1b[?1049h"), false, "alt-screen"); + assert.equal(isDroppableVisualPayload("\x1bP1;2q\x1b\\"), false, "DCS"); + assert.equal(isDroppableVisualPayload("\x1b[c"), false, "device attributes"); }); -test("does not drop when the successor is too small to be a full repaint", () => { - // A homing but tiny successor (e.g. HOME + one changed cell) does not prove a - // full-screen repaint, so the earlier frame's other changes must survive. - const a = frame("AAAAAAAAAAAAAAAAAAAA"); - const smallSuccessor = frame("x"); // content well under the threshold - const input = a + smallSuccessor; - const threshold = 100; - assert.deepEqual(collapseAndSplit(input, threshold), { - complete: input, - partial: "", - dropped: 0, - }); +// ---- viewportRepaintCoverage / makesFullRepaint --------------------------- + +test("coverage counts distinct written cells, with wrap", () => { + assert.equal(viewportRepaintCoverage("abcd", 4, 1), 4); + assert.equal(viewportRepaintCoverage("abcd", 2, 2), 4); // wraps to second row + assert.equal(viewportRepaintCoverage("ab", 4, 1), 2); }); -test("drops when the successor clears the full-repaint size threshold", () => { - const a = frame("A"); - const bigPaint = "y".repeat(200); - const bigSuccessor = frame(bigPaint); // content over the threshold - const input = a + bigSuccessor; - const r = collapseAndSplit(input, 100); - assert.equal(r.complete, bigSuccessor); - assert.equal(r.dropped, a.length); +test("coverage honors cursor positioning and erases", () => { + assert.equal(viewportRepaintCoverage("\x1b[2J", 4, 2), 8, "erase-all covers the grid"); + assert.equal(viewportRepaintCoverage("\x1b[1;1H\x1b[K", 4, 2), 4, "erase-line covers one row"); + // CUP to row 2 then one char covers a single cell there. + assert.equal(viewportRepaintCoverage("\x1b[2;3Hx", 4, 2), 1); +}); + +test("makesFullRepaint accepts a whole-screen paint and rejects a small update", () => { + const cols = 4; + const rows = 2; + const fullPaint = `${HOME}${"z".repeat(cols * rows)}`; // writes every cell + const smallPaint = `${HOME}z`; // one cell + assert.equal(makesFullRepaint(fullPaint, cols, rows), true); + assert.equal(makesFullRepaint(smallPaint, cols, rows), false); + // SGR-heavy small update must not pass on byte length alone. + const sgrHeavySmall = `${HOME}\x1b[38;2;1;2;3m\x1b[48;2;4;5;6mz`; + assert.equal(makesFullRepaint(sgrHeavySmall, cols, rows), false); }); +test("collapse drops only when coverage proves a full repaint", () => { + const cols = 4; + const rows = 2; + const pred = (c: string) => makesFullRepaint(c, cols, rows); + const stale = frame("AAAAAAAA"); + const fullNext = frame("z".repeat(cols * rows)); + const smallNext = frame("z"); + // Full-repaint successor → predecessor dropped. + assert.equal(collapseAndSplit(stale + fullNext, pred).dropped, stale.length); + // Small successor → nothing dropped. + assert.equal(collapseAndSplit(stale + smallNext, pred).dropped, 0); +}); + +// ---- ingress apportioning -------------------------------------------------- + test("ingress apportioning always sums back to the total", () => { const cases: Array<[number, number, number, number, number]> = [ - // total, totalChars, forwardChars, droppedChars, heldChars [1000, 1000, 400, 400, 200], - [1500, 1000, 400, 400, 200], // ingress != chars (plugin expanded the chunk) - [700, 1000, 400, 400, 200], // ingress != chars (plugin contracted the chunk) - [999, 1000, 333, 333, 334], // rounding stress - [0, 0, 0, 0, 0], // empty - [500, 500, 0, 0, 500], // all held - [500, 500, 500, 0, 0], // all forwarded - [500, 500, 0, 500, 0], // all dropped + [1500, 1000, 400, 400, 200], + [700, 1000, 400, 400, 200], + [999, 1000, 333, 333, 334], + [0, 0, 0, 0, 0], + [500, 500, 0, 0, 500], + [500, 500, 500, 0, 0], + [500, 500, 0, 500, 0], ]; for (const [total, totalChars, fwd, drop, held] of cases) { const s = apportionFrameGateIngress(total, totalChars, fwd, drop, held); - assert.equal( - s.forward + s.dropped + s.held, - total, - `parts must sum to total for ${JSON.stringify([total, totalChars, fwd, drop, held])}`, - ); - assert.ok(s.forward >= 0 && s.dropped >= 0 && s.held >= 0, "no negative shares"); + assert.equal(s.forward + s.dropped + s.held, total); + assert.ok(s.forward >= 0 && s.dropped >= 0 && s.held >= 0); } }); test("ingress apportioning routes bytes to the right bucket in the exact case", () => { - // ingress == chars: shares equal the character counts. assert.deepEqual(apportionFrameGateIngress(1000, 1000, 400, 400, 200), { forward: 400, dropped: 400, diff --git a/components/terminal/runtime/terminalFrameGate.ts b/components/terminal/runtime/terminalFrameGate.ts index 34442cd6a..2a1278f33 100644 --- a/components/terminal/runtime/terminalFrameGate.ts +++ b/components/terminal/runtime/terminalFrameGate.ts @@ -10,48 +10,175 @@ * it is never more than a frame or two behind — otherwise frames queue up and * the display (and the keyboard echo waiting behind it) runs up to a second * late. The flow-control watermark bounds that backlog by *pausing* the source, - * which throttles the animation's frame rate. This gate instead bounds it by - * *dropping* superseded frames: when several full-repaint frames are buffered, - * only the last is visible (the next repaints every cell the previous drew), so - * the earlier ones can be skipped. The source is never paused, so the animation - * keeps its full rate while the backlog — and the latency — stays small. + * which throttles the animation. This gate instead bounds it by *dropping* + * superseded frames: when a full-repaint frame is buffered behind another, only + * the last is visible (the next repaints every cell the previous drew), so the + * earlier one can be skipped. The source is never paused, so the animation keeps + * its full rate while the backlog — and the latency — stays small. + * + * Dropping is deliberately conservative, and proven rather than guessed: + * - the dropped frame must be a droppable *visual* payload (allowlist of cursor + * moves, SGR, erase and cell text — never a bell, device query, OSC, DCS/APC + * or private mode whose side effect would be lost); and + * - its successor must *demonstrably* repaint the whole viewport, measured by + * simulating the writes and counting covered cells — not by raw byte length, + * which SGR escapes inflate. * * This module is the pure buffer transform. It has no state and no side - * effects; the caller owns the per-terminal buffer and the accounting. + * effects; the caller owns the per-terminal buffer, the accounting and the + * fail-open handling for frames that never complete. */ const SYNC_ON = "\x1b[?2026h"; const SYNC_OFF = "\x1b[?2026l"; -const CSI = "\x1b["; -/** A CUP/HVP param list resolves to the top-left cell (row/col 1, default, or 0). */ -const isHomeParams = (params: string): boolean => { - const parts = params.split(";"); - if (parts.length > 2) return false; - return parts.every((p) => p === "" || p === "0" || p === "1"); +/** CSI final bytes that only move the cursor, set SGR, or erase. */ +const DROPPABLE_CSI_FINALS = new Set([ + "A", "B", "C", "D", "E", "F", "G", "H", "f", // cursor moves / positioning + "d", "`", // line / column position (VPA / HPA) + "m", // SGR + "J", "K", // erase in display / line + "S", "T", // scroll up / down +]); +/** C0 controls that only move the cursor. */ +const DROPPABLE_C0 = new Set(["\r", "\n", "\t", "\b"]); + +/** + * True when every byte of `content` is a cursor move, SGR, erase, or cell text + * — i.e. dropping the frame loses nothing but pixels a full repaint overwrites. + * Anything else (BEL, device query/report, OSC, DCS/APC/PM/SOS, a private-mode + * or alternate-screen toggle, an intermediate-byte CSI) makes the frame + * un-droppable, since its side effect would never reach xterm. + */ +export const isDroppableVisualPayload = (content: string): boolean => { + let i = 0; + while (i < content.length) { + const ch = content[i]; + const code = content.charCodeAt(i); + if (ch === "\x1b") { + if (content[i + 1] !== "[") return false; // only CSI; reject OSC/DCS/APC/single-char ESC + let j = i + 2; + let priv = false; + while (j < content.length) { + const c = content.charCodeAt(j); + if (c >= 0x30 && c <= 0x3f) { + if (c === 0x3c || c === 0x3d || c === 0x3e || c === 0x3f) priv = true; // < = > ? + j++; + } else { + break; + } + } + if (j < content.length && content.charCodeAt(j) >= 0x20 && content.charCodeAt(j) <= 0x2f) { + return false; // intermediate byte — uncommon, treat as un-droppable + } + const final = content[j]; + if (final === undefined) return false; // incomplete CSI + if (priv || !DROPPABLE_CSI_FINALS.has(final)) return false; + i = j + 1; + continue; + } + if (code < 0x20) { + if (!DROPPABLE_C0.has(ch)) return false; // BEL and other C0 side effects + i++; + continue; + } + if (code === 0x7f) return false; // DEL + i++; // printable / UTF-8 byte → cell text + } + return true; }; -/** True when `content` begins by homing the cursor to the top-left. */ -const startsWithCursorHome = (content: string): boolean => { - if (!content.startsWith(CSI)) return false; - let i = CSI.length; +/** + * Count the distinct viewport cells `content` writes, by simulating cursor + * movement, cell output and erases against a `cols`×`rows` grid. Used to prove a + * frame repaints (nearly) the whole screen before an earlier frame is dropped. + */ +export const viewportRepaintCoverage = ( + content: string, + cols: number, + rows: number, +): number => { + if (cols <= 0 || rows <= 0) return 0; + const covered = new Set(); + let row = 0; + let col = 0; + const clampRow = () => { row = row < 0 ? 0 : row >= rows ? rows - 1 : row; }; + const clampCol = () => { col = col < 0 ? 0 : col >= cols ? cols - 1 : col; }; + const mark = (r: number, c: number) => { + if (r >= 0 && r < rows && c >= 0 && c < cols) covered.add(r * cols + c); + }; + const markRange = (r: number, from: number, to: number) => { + for (let c = Math.max(0, from); c <= to && c < cols; c++) mark(r, c); + }; + let i = 0; while (i < content.length) { - const c = content.charCodeAt(i); - if ((c >= 0x30 && c <= 0x39) || c === 0x3b) i++; // digits and ';' - else break; + const ch = content[i]; + const code = content.charCodeAt(i); + if (ch === "\x1b" && content[i + 1] === "[") { + let j = i + 2; + let params = ""; + while (j < content.length) { + const c = content.charCodeAt(j); + if (c >= 0x30 && c <= 0x3f) { params += content[j]; j++; } else break; + } + while (j < content.length && content.charCodeAt(j) >= 0x20 && content.charCodeAt(j) <= 0x2f) j++; + const final = content[j]; + const nums = params.split(";").map((p) => (p === "" ? undefined : parseInt(p, 10))); + const n0 = nums[0]; + if (final === "H" || final === "f") { row = (n0 ?? 1) - 1; col = (nums[1] ?? 1) - 1; clampRow(); clampCol(); } + else if (final === "A") { row -= n0 ?? 1; clampRow(); } + else if (final === "B" || final === "E") { row += n0 ?? 1; clampRow(); } + else if (final === "C") { col += n0 ?? 1; clampCol(); } + else if (final === "D") { col -= n0 ?? 1; clampCol(); } + else if (final === "G" || final === "`") { col = (n0 ?? 1) - 1; clampCol(); } + else if (final === "d") { row = (n0 ?? 1) - 1; clampRow(); } + else if (final === "J") { + const p = n0 ?? 0; + if (p === 2 || p === 3) { for (let r = 0; r < rows; r++) markRange(r, 0, cols - 1); } + else if (p === 0) { markRange(row, col, cols - 1); for (let r = row + 1; r < rows; r++) markRange(r, 0, cols - 1); } + else if (p === 1) { for (let r = 0; r < row; r++) markRange(r, 0, cols - 1); markRange(row, 0, col); } + } else if (final === "K") { + const p = n0 ?? 0; + if (p === 0) markRange(row, col, cols - 1); + else if (p === 1) markRange(row, 0, col); + else if (p === 2) markRange(row, 0, cols - 1); + } + i = final === undefined ? content.length : j + 1; + continue; + } + if (code < 0x20) { + if (ch === "\n") { row += 1; clampRow(); } + else if (ch === "\r") { col = 0; } + else if (ch === "\b") { col -= 1; clampCol(); } + else if (ch === "\t") { col = Math.min(cols - 1, (Math.floor(col / 8) + 1) * 8); } + i++; + continue; + } + if (code === 0x7f) { i++; continue; } + mark(row, col); + col += 1; + if (col >= cols) { col = 0; row += 1; clampRow(); } + i++; } - const final = content[i]; - return (final === "H" || final === "f") && isHomeParams(content.slice(CSI.length, i)); + return covered.size; }; /** - * A frame's payload is a pure visual repaint: only cursor moves, SGR and cell - * text. Rejects OSC (`ESC]`) and private-mode set/reset (`ESC[?…`, which - * includes alternate-screen switches) — state a successor would not restore, so - * such a frame must never be dropped. + * Fraction of the viewport a successor must repaint before its predecessor is + * dropped. Full-screen animated TUIs commonly rewrite only the cells that + * changed (TryIt.jl repaints ~60% of the grid per frame via cursor jumps), so + * the bar sits well below 100% to still recognise them, yet far above the few + * per-cent a small incremental update touches. Dropping is therefore best-effort + * for animations, not a lossless guarantee — a cell the dropped frame alone + * repainted shows one frame stale, which is imperceptible in motion. */ -const isPureVisualPayload = (content: string): boolean => - !content.includes("\x1b]") && !content.includes("\x1b[?"); +const FULL_REPAINT_COVERAGE = 0.4; + +/** A successor frame that demonstrably repaints (almost) the whole viewport. */ +export const makesFullRepaint = (content: string, cols: number, rows: number): boolean => { + if (cols <= 0 || rows <= 0) return false; + return viewportRepaintCoverage(content, cols, rows) >= Math.floor(cols * rows * FULL_REPAINT_COVERAGE); +}; type Frame = { start: number; end: number; content: string }; @@ -63,51 +190,20 @@ type Frame = { start: number; end: number; content: string }; */ export type FrameGateSplit = { complete: string; partial: string; dropped: number }; -/** Exact three-way split of buffered ingress bytes; parts always sum to `total`. */ -export type FrameGateIngressSplit = { forward: number; dropped: number; held: number }; - -/** - * Apportion `total` flow-control ingress bytes across the forwarded, dropped and - * still-held parts of a buffer, by character share. Each share is the exact - * complement of the rounded parts before it, so the three always sum back to - * `total` regardless of rounding — the backend is never over- or - * under-acknowledged even when a chunk's ingress differs from its length. - */ -export const apportionFrameGateIngress = ( - total: number, - totalChars: number, - forwardChars: number, - droppedChars: number, - heldChars: number, -): FrameGateIngressSplit => { - const held = totalChars > 0 ? Math.round((total * heldChars) / totalChars) : 0; - const leaving = total - held; - const leavingChars = forwardChars + droppedChars; - const forward = leavingChars > 0 ? Math.round((leaving * forwardChars) / leavingChars) : 0; - const dropped = leaving - forward; - return { forward, dropped, held }; -}; - /** * Split `buffer` into its complete-frame prefix and a trailing incomplete * frame, collapsing runs of superseded full-repaint frames in the prefix down * to the last. * - * A frame is dropped only when it is a pure visual repaint AND the frame - * directly after it *demonstrably repaints the whole screen*: it homes the - * cursor and carries at least `minSuccessorRepaintBytes` of payload. Homing - * alone is not enough — DEC 2026 only makes an update atomic, it does not imply - * a full repaint, and a valid incremental successor (`HOME` + one changed cell) - * would otherwise discard the earlier frame's changes elsewhere. A real - * full-screen repaint writes at least one byte per cell, so a viewport-sized - * threshold (`cols * rows`) separates it from a small incremental update. - * Everything the transform is unsure about is preserved verbatim. + * A frame is dropped only when it is a droppable visual payload AND the frame + * directly after it, per `isFullRepaint`, demonstrably repaints the whole + * viewport (so it overwrites everything the dropped frame drew). Everything the + * transform is unsure about is preserved verbatim. */ export const collapseAndSplit = ( buffer: string, - minSuccessorRepaintBytes: number, + isFullRepaint: (content: string) => boolean, ): FrameGateSplit => { - // Locate every complete frame; note where an unterminated trailing frame begins. const frames: Frame[] = []; let cursor = 0; let partialStart = buffer.length; @@ -116,10 +212,7 @@ export const collapseAndSplit = ( if (on < 0) break; const contentStart = on + SYNC_ON.length; const off = buffer.indexOf(SYNC_OFF, contentStart); - if (off < 0) { - partialStart = on; // trailing frame opened but not closed yet - break; - } + if (off < 0) { partialStart = on; break; } const end = off + SYNC_OFF.length; frames.push({ start: on, end, content: buffer.slice(contentStart, off) }); cursor = end; @@ -127,36 +220,28 @@ export const collapseAndSplit = ( const partial = buffer.slice(partialStart); const completeRegion = buffer.slice(0, partialStart); + if (frames.length < 2) return { complete: completeRegion, partial, dropped: 0 }; - if (frames.length < 2) { - return { complete: completeRegion, partial, dropped: 0 }; - } - - // Mark a frame droppable when it is a pure visual repaint, the next frame is - // directly adjacent, and that next frame fully repaints the screen. const drop = new Array(frames.length).fill(false); for (let i = 0; i < frames.length - 1; i++) { const cur = frames[i]; const next = frames[i + 1]; if ( next.start === cur.end - && isPureVisualPayload(cur.content) - && startsWithCursorHome(next.content) - && next.content.length >= minSuccessorRepaintBytes + && isDroppableVisualPayload(cur.content) + && isFullRepaint(next.content) ) { drop[i] = true; } } - if (!drop.some(Boolean)) { - return { complete: completeRegion, partial, dropped: 0 }; - } + if (!drop.some(Boolean)) return { complete: completeRegion, partial, dropped: 0 }; let complete = ""; let dropped = 0; let pos = 0; for (let i = 0; i < frames.length; i++) { const f = frames[i]; - complete += completeRegion.slice(pos, f.start); // bytes before the frame + complete += completeRegion.slice(pos, f.start); if (drop[i]) dropped += f.end - f.start; else complete += completeRegion.slice(f.start, f.end); pos = f.end; @@ -164,3 +249,28 @@ export const collapseAndSplit = ( complete += completeRegion.slice(pos); return { complete, partial, dropped }; }; + +/** Exact three-way split of buffered ingress bytes; parts always sum to `total`. */ +export type FrameGateIngressSplit = { forward: number; dropped: number; held: number }; + +/** + * Apportion `total` flow-control ingress bytes across the forwarded, dropped and + * still-held parts of a buffer, by character share. Each share is the exact + * complement of the rounded parts before it, so the three always sum back to + * `total` regardless of rounding — the backend is never over- or + * under-acknowledged even when a chunk's ingress differs from its length. + */ +export const apportionFrameGateIngress = ( + total: number, + totalChars: number, + forwardChars: number, + droppedChars: number, + heldChars: number, +): FrameGateIngressSplit => { + const held = totalChars > 0 ? Math.round((total * heldChars) / totalChars) : 0; + const leaving = total - held; + const leavingChars = forwardChars + droppedChars; + const forward = leavingChars > 0 ? Math.round((leaving * forwardChars) / leavingChars) : 0; + const dropped = leaving - forward; + return { forward, dropped, held }; +}; diff --git a/components/terminal/runtime/terminalSessionAttachment.ts b/components/terminal/runtime/terminalSessionAttachment.ts index 5b5d9b2ea..5089d714a 100644 --- a/components/terminal/runtime/terminalSessionAttachment.ts +++ b/components/terminal/runtime/terminalSessionAttachment.ts @@ -47,7 +47,7 @@ import { resetTerminalSyncBlockFilter, } from "./terminalSyncBlockFilter"; import { appendEraseScrollbackAfterFullErases } from "../clearTerminalViewport"; -import { apportionFrameGateIngress, collapseAndSplit } from "./terminalFrameGate"; +import { apportionFrameGateIngress, collapseAndSplit, makesFullRepaint } from "./terminalFrameGate"; import { type CoalescedTerminalWriteOptions, enqueueCoalescedTerminalWrite, @@ -470,8 +470,21 @@ export const writeTerminalLine = ( * instead of letting the backlog (and the latency riding behind it) grow. */ const FRAME_GATE_FORWARD_BACKLOG = 512 * 1024; -/** Retry cadence for draining the gate's held buffer when the backlog clears. */ +/** Retry cadence for draining held *complete* output when the backlog clears. */ const FRAME_GATE_FLUSH_MS = 8; +/** + * Fail-open ceiling for the held buffer. Releasing at once past this keeps the + * gate from withholding output unboundedly and, kept below the SSH flow + * watermark, avoids deadlocking on a frame larger than that watermark (the + * backend would pause before the withheld closer could arrive). + */ +const FRAME_GATE_MAX_HELD_BYTES = 512 * 1024; +/** + * How long a lone trailing partial (an opener with no closer) may be held before + * it is released to xterm. Long enough not to fire during normal frame assembly, + * short enough that a process killed mid-frame does not leave its prompt hidden. + */ +const FRAME_GATE_PARTIAL_FAILOPEN_MS = 200; type FrameGateState = { buffer: string; @@ -520,10 +533,13 @@ const drainFrameGate = ( } if (!state.buffer) return; - // A real full-screen repaint writes at least one byte per cell; require the - // superseding frame to clear that bar before dropping its predecessor. - const minRepaint = Math.max(0, term.cols * term.rows); - const { complete, partial, dropped } = collapseAndSplit(state.buffer, minRepaint); + // Drop a frame only when its successor demonstrably repaints the whole + // viewport (proven by simulating the writes and counting covered cells), never + // on raw payload length, which SGR escapes inflate. + const { complete, partial, dropped } = collapseAndSplit( + state.buffer, + (content) => makesFullRepaint(content, term.cols, term.rows), + ); // Apportion the buffered ingress across forwarded / dropped / held so the // backend is acknowledged in its own units, not rendered-string lengths. @@ -557,15 +573,39 @@ const drainFrameGate = ( } } - // Only a clearing backlog can release held *complete* output, so poll for it. - // A lone trailing partial can only be completed by new session data — which - // calls drainFrameGate itself — so it must never schedule a timer, or a - // session stalled mid-frame would busy-poll indefinitely. - if (heldComplete && state.flushTimer === undefined) { + if (!state.buffer) return; + + // Fail-open: never withhold output indefinitely. A held buffer past the cap + // (e.g. a frame larger than the SSH flow watermark, which would otherwise + // deadlock) is released at once. + if (state.buffer.length >= FRAME_GATE_MAX_HELD_BYTES) { + forwardSessionData(ctx, term, state.buffer, state.ingress, state.meta); + state.buffer = ""; + state.ingress = 0; + return; + } + + // Held *complete* output is released by a clearing backlog, so poll quickly. + // A lone trailing partial can only be completed by new session data (which + // calls drainFrameGate itself); poll it only on a longer one-shot that + // fail-opens if it fires — so a process killed mid-frame never leaves its + // prompt hidden, yet a stalled session never busy-polls. + if (state.flushTimer === undefined) { + const delay = heldComplete ? FRAME_GATE_FLUSH_MS : FRAME_GATE_PARTIAL_FAILOPEN_MS; state.flushTimer = setTimeout(() => { state.flushTimer = undefined; - drainFrameGate(ctx, term); - }, FRAME_GATE_FLUSH_MS); + if (heldComplete) { + drainFrameGate(ctx, term); + return; + } + // Fired with no intervening drain: the partial is stuck — release it. + const stuck = getFrameGateState(term); + if (stuck.buffer) { + forwardSessionData(ctx, term, stuck.buffer, stuck.ingress, stuck.meta); + stuck.buffer = ""; + stuck.ingress = 0; + } + }, delay); } }; From 651c447b846f516cf77c0c06d895c441925a105a Mon Sep 17 00:00:00 2001 From: s-celles Date: Sun, 26 Jul 2026 08:59:29 +0200 Subject: [PATCH 10/10] fix(terminal): frame-gate review follow-ups (ED3, reset flush, split opener) Address further automated review feedback on the animation frame gate: - Never drop a frame carrying ED3. `CSI 3 J` clears the saved scrollback, a side effect a repaint does not restore, so a frame containing it is no longer classified as a droppable visual payload. - Flush the gate before resetting it. `resetFrameGate` now hands any held buffer to the caller before dropping the state: the write-context site forwards it to xterm, the hibernation site acknowledges its ingress. A reset racing an incomplete frame no longer loses buffered output or leaves the backend paused on unacknowledged bytes. - Engage on a split DEC 2026 opener. The gate now also engages when a chunk ends with a proper prefix of `ESC[?2026h`, and `collapseAndSplit` holds that trailing prefix back, so an opener cut across two PTY chunks reunites with the next chunk instead of bypassing the gate. Adds unit coverage for ED3 rejection, the split-opener hold and `endsWithSyncOpenerPrefix`. 736 terminal-runtime tests pass. Assisted-by: AI --- .../runtime/terminalFrameGate.test.ts | 21 +++++++++ .../terminal/runtime/terminalFrameGate.ts | 29 +++++++++++++ .../runtime/terminalSessionAttachment.ts | 43 ++++++++++++++++--- 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/components/terminal/runtime/terminalFrameGate.test.ts b/components/terminal/runtime/terminalFrameGate.test.ts index 3dbc4a656..73ac221ad 100644 --- a/components/terminal/runtime/terminalFrameGate.test.ts +++ b/components/terminal/runtime/terminalFrameGate.test.ts @@ -4,6 +4,7 @@ import test from "node:test"; import { apportionFrameGateIngress, collapseAndSplit, + endsWithSyncOpenerPrefix, isDroppableVisualPayload, makesFullRepaint, viewportRepaintCoverage, @@ -161,3 +162,23 @@ test("ingress apportioning routes bytes to the right bucket in the exact case", held: 200, }); }); + +test("ED3 (clear scrollback) makes a frame un-droppable", () => { + assert.equal(isDroppableVisualPayload("\x1b[1;1H\x1b[2Jrepaint"), true, "ED2 viewport clear is fine"); + assert.equal(isDroppableVisualPayload("\x1b[1;1H\x1b[3Jrepaint"), false, "ED3 clears scrollback"); +}); + +test("holds back a split sync opener as partial", () => { + // Buffer ends mid-opener (ESC[?20); it must be held, not forwarded. + const r = collapseAndSplit("hello\x1b[?20", always); + assert.equal(r.complete, "hello"); + assert.equal(r.partial, "\x1b[?20"); + assert.equal(r.dropped, 0); +}); + +test("endsWithSyncOpenerPrefix detects a split opener tail", () => { + assert.equal(endsWithSyncOpenerPrefix("abc\x1b[?2026"), true); + assert.equal(endsWithSyncOpenerPrefix("abc\x1b"), true); + assert.equal(endsWithSyncOpenerPrefix("abc\x1b[?2026h"), false, "complete opener is not a prefix hold"); + assert.equal(endsWithSyncOpenerPrefix("plain"), false); +}); diff --git a/components/terminal/runtime/terminalFrameGate.ts b/components/terminal/runtime/terminalFrameGate.ts index 2a1278f33..0e6c2a2ec 100644 --- a/components/terminal/runtime/terminalFrameGate.ts +++ b/components/terminal/runtime/terminalFrameGate.ts @@ -32,6 +32,20 @@ const SYNC_ON = "\x1b[?2026h"; const SYNC_OFF = "\x1b[?2026l"; +/** Length of a trailing run of `s` that is a proper (non-empty) prefix of the + * sync opener — i.e. a `ESC[?2026h` split at a chunk boundary. 0 when none. */ +const trailingSyncOpenerPrefixLen = (s: string): number => { + const max = Math.min(SYNC_ON.length - 1, s.length); + for (let k = max; k >= 1; k--) { + if (s.endsWith(SYNC_ON.slice(0, k))) return k; + } + return 0; +}; + +/** True when `s` ends with a split sync opener (a proper prefix of `ESC[?2026h`). */ +export const endsWithSyncOpenerPrefix = (s: string): boolean => + trailingSyncOpenerPrefixLen(s) > 0; + /** CSI final bytes that only move the cursor, set SGR, or erase. */ const DROPPABLE_CSI_FINALS = new Set([ "A", "B", "C", "D", "E", "F", "G", "H", "f", // cursor moves / positioning @@ -59,10 +73,12 @@ export const isDroppableVisualPayload = (content: string): boolean => { if (content[i + 1] !== "[") return false; // only CSI; reject OSC/DCS/APC/single-char ESC let j = i + 2; let priv = false; + let params = ""; while (j < content.length) { const c = content.charCodeAt(j); if (c >= 0x30 && c <= 0x3f) { if (c === 0x3c || c === 0x3d || c === 0x3e || c === 0x3f) priv = true; // < = > ? + else params += content[j]; j++; } else { break; @@ -74,6 +90,10 @@ export const isDroppableVisualPayload = (content: string): boolean => { const final = content[j]; if (final === undefined) return false; // incomplete CSI if (priv || !DROPPABLE_CSI_FINALS.has(final)) return false; + // `CSI 3 J` (ED3) clears the saved scrollback, not just the viewport — a + // side effect a repaint does not restore, so a frame carrying it is not + // droppable. + if (final === "J" && params.split(";").includes("3")) return false; i = j + 1; continue; } @@ -218,6 +238,15 @@ export const collapseAndSplit = ( cursor = end; } + // Hold back a trailing byte run that is a proper prefix of the sync opener, so + // an opener split across PTY chunks reunites with the next chunk instead of + // being forwarded and missed. Only when no unterminated frame already covers + // the tail. + if (partialStart === buffer.length) { + const holdLen = trailingSyncOpenerPrefixLen(buffer); + if (holdLen > 0) partialStart = buffer.length - holdLen; + } + const partial = buffer.slice(partialStart); const completeRegion = buffer.slice(0, partialStart); if (frames.length < 2) return { complete: completeRegion, partial, dropped: 0 }; diff --git a/components/terminal/runtime/terminalSessionAttachment.ts b/components/terminal/runtime/terminalSessionAttachment.ts index 5089d714a..5d7e270de 100644 --- a/components/terminal/runtime/terminalSessionAttachment.ts +++ b/components/terminal/runtime/terminalSessionAttachment.ts @@ -47,7 +47,12 @@ import { resetTerminalSyncBlockFilter, } from "./terminalSyncBlockFilter"; import { appendEraseScrollbackAfterFullErases } from "../clearTerminalViewport"; -import { apportionFrameGateIngress, collapseAndSplit, makesFullRepaint } from "./terminalFrameGate"; +import { + apportionFrameGateIngress, + collapseAndSplit, + endsWithSyncOpenerPrefix, + makesFullRepaint, +} from "./terminalFrameGate"; import { type CoalescedTerminalWriteOptions, enqueueCoalescedTerminalWrite, @@ -510,9 +515,19 @@ const getFrameGateState = (term: XTerm): FrameGateState => { return state; }; -export const resetFrameGate = (term: XTerm): void => { +export const resetFrameGate = ( + term: XTerm, + onHeld?: (buffer: string, ingress: number) => void, +): void => { const state = frameGateStates.get(term); - if (state?.flushTimer !== undefined) clearTimeout(state.flushTimer); + if (!state) return; + if (state.flushTimer !== undefined) clearTimeout(state.flushTimer); + // Never silently drop held output before its state is deleted: a reset racing + // an incomplete frame (hibernation, detach) would otherwise lose the buffered + // bytes and leave their ingress unacknowledged to the backend. The caller + // decides how to release it — forward it where a write context exists, + // acknowledge its ingress where only the backend is available. + if (state.buffer) onHeld?.(state.buffer, state.ingress); frameGateStates.delete(term); }; @@ -623,7 +638,12 @@ export const writeSessionData = ( meta?: TerminalSessionDataMeta, ) => { const state = frameGateStates.get(term); - const engaged = (state && state.buffer.length > 0) || data.includes("\x1b[?2026h"); + // Engage on a complete opener, on already-buffered output, or on a trailing + // split opener (`ESC[?2026h` cut across PTY chunks) so an aligned stream can + // never bypass the gate by landing the opener on a chunk boundary. + const engaged = (state && state.buffer.length > 0) + || data.includes("\x1b[?2026h") + || endsWithSyncOpenerPrefix(data); if (!engaged) { forwardSessionData(ctx, term, data, ingressBytes, meta); return; @@ -980,7 +1000,14 @@ export const releaseTerminalFlowBeforeHibernate = ( setTerminalWriteCoalescerFlushGate(term); pendingTimestampSecondByTerm.delete(term); resetDeferredTerminalWriteAck(term); - resetFrameGate(term); + // Only the backend is in scope here; acknowledge held ingress so hibernation + // does not leave the source paused on bytes that will never be written. + resetFrameGate(term, (_buffer, ingress) => { + if (ingress > 0) { + ackTerminalSessionFlow(backend, sessionId, ingress); + flushTerminalSessionFlowAck(sessionId); + } + }); terminalFlowControllers.delete(term); }; @@ -1029,7 +1056,11 @@ export const attachSessionToTerminal = ( teardownTerminalOutputPipeline(ctx, term, id, flow); flushTerminalWriteCoalescer(term); resetTerminalSyncBlockFilter(term); - resetFrameGate(term); + // A write context exists here, so flush any held output to xterm rather than + // dropping it (forwardSessionData also acknowledges its ingress). + resetFrameGate(term, (buffer, ingress) => { + forwardSessionData(ctx, term, buffer, ingress); + }); resetTerminalLineTimestamps(term); resetTerminalOutputPressure(term); ctx.onSessionAttached?.(id);