Skip to content

Fluid full-screen animated TUIs: drop superseded frames to keep 60 fps and a responsive keyboard#2489

Open
s-celles wants to merge 10 commits into
binaricat:mainfrom
s-celles:perf/fluid-terminal-v2
Open

Fluid full-screen animated TUIs: drop superseded frames to keep 60 fps and a responsive keyboard#2489
s-celles wants to merge 10 commits into
binaricat:mainfrom
s-celles:perf/fluid-terminal-v2

Conversation

@s-celles

@s-celles s-celles commented Jul 25, 2026

Copy link
Copy Markdown

Summary

Makes full-screen animated TUIs (e.g. TryIt.jl
at 60 fps) render fluidly in a local terminal and keep the keyboard
responsive. The flow-control watermark alone is a pure throughput↔latency dial —
high gives 60 fps with ~800 ms input lag, low gives a responsive keyboard at
~30 fps. This adds a renderer-side frame gate that drops superseded
full-screen frames instead of pausing the source, so the animation keeps full
rate while the backlog (and the latency behind it) stays bounded.

Measured on an M4 MacBook Air with TryIt.jl at TRY_FPS=60: ~60 fps with
~50 ms input-to-display latency, down from ~800 ms
, backlog capped near
0.5 MB.

Type of Change

  • Bug fix
  • New feature
  • Refactor / code cleanup
  • Documentation update
  • Build / CI change
  • Other (please describe):

Related Issue

Related to #2487.

Changes Made

  • Drop superseded animation frames (terminalFrameGate) — the core fix.
    When several full-screen full-repaint frames are buffered, only the last is
    visible, so the earlier ones are collapsed away and their bytes acknowledged.
    The source is never paused. Collapsing is strict: only pure visual repaints
    whose successor demonstrably repaints the whole screen (homes the cursor and
    writes ≥ 1 byte per cell) are dropped, never frames carrying OSC or
    private-mode / alternate-screen state. Flow-control ingress accounting is
    apportioned exactly through the gate.
  • Relax output back-pressure for local shells — a local shell has no network
    to overwhelm, so it gets a higher backstop watermark; SSH and every other
    protocol keep the tight default.
  • Keep DEC 2026 frames whole when slicing coalesced output — the batch
    slicer never cuts inside a synchronized-output frame.
  • Recover the write queue from a lost xterm write callback — a stall
    watchdog force-completes an item whose term.write callback never fires.
  • Expose allowTransparency as a setting.

Cross-repo: the display side needs xtermjs/xterm.js#6073 (issue #6071) — under
a continuous animation xterm.js defers a just-closed DEC 2026 frame to a
debounced paint and caps the display at ~1 fps. Until that ships in a release,
scripts/patch-xterm-sync-render.cjs applies it as an idempotent postinstall
string-patch on the bundled build; it no-ops once the upstream fix is present.

Happy to split this into smaller PRs (e.g. the frame gate on its own) if you'd
prefer to review it in pieces.

Screenshots / Demo

TryIt.jl at TRY_FPS=60 in a local terminal renders a fluid ~60 fps animated
background with immediate arrow-key / Ctrl+T response (was either laggy at
60 fps or throttled to ~30 fps before).

Testing

  • I have tested these changes locally (npm run dev)
  • Linting passes (npm run lint)
  • Tests pass (npm test) — terminal runtime suite green, incl. new
    terminalFrameGate unit coverage (frame collapse, size threshold, exact
    ingress apportioning)
  • Generated capability tool specs are updated when applicable (npm run generate:capability-tools) — N/A
  • No new console errors or warnings

Checklist

  • My code follows the existing project style
  • I have added or updated relevant documentation — code comments only; no user-facing docs touched
  • I have not introduced any breaking changes (or I have described them above)

Assisted-by: AI

s-celles added 7 commits July 25, 2026 08:17
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.
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.
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.
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.
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.
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.
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b30766cdd3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +519 to +522
if (complete) {
const backlog = getFlowControllerForTerm(term)?.pendingBytes() ?? 0;
if (backlog < FRAME_GATE_FORWARD_BACKLOG) {
forwardSessionData(ctx, term, complete, complete.length, state.meta);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve ingress accounting through the frame gate

When plugin processing expands or contracts a DEC 2026 chunk, ingressBytes deliberately differs from the rendered string length, but the gate discards that accounting and forwards complete.length; it also acknowledges collapsed output using the dropped character count. Such transformed frames therefore over- or under-ack the backend, which can prematurely resume output or leave the session paused at the high-water mark. Retain and apportion the original ingress units while buffering and dropping frames.

Useful? React with 👍 / 👎.

Comment on lines +105 to +109
if (
next.start === cur.end
&& isPureVisualPayload(cur.content)
&& startsWithCursorHome(next.content)
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require proof of a full repaint before dropping frames

DEC 2026 only makes updates atomic; it does not imply that a frame repaints every cell, and homing the cursor does not establish that either. Under backlog, a valid incremental successor such as HOME + one changed cell satisfies this condition, causing the preceding frame's changes elsewhere on screen to be silently discarded. Only collapse frames when the successor demonstrably clears or overwrites the complete viewport.

Useful? React with 👍 / 👎.

Comment on lines +160 to +168
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Do not force-complete slow xterm writes

If an xterm write callback is delayed beyond 250 ms on a slow machine or during a large parse, this treats an in-progress write as dropped, acknowledges its bytes, and starts subsequent writes while the original remains active inside xterm. The queue cannot distinguish that legitimate latency from a permanently lost callback, so this can defeat backpressure and skip the original item's completion bookkeeping during exactly the heavy-output scenario being optimized. Recovery needs a signal that xterm has actually drained or a substantially safer lifecycle mechanism rather than elapsed time alone.

Useful? React with 👍 / 👎.

Comment on lines +530 to +534
if (state.buffer && state.flushTimer === undefined) {
state.flushTimer = setTimeout(() => {
state.flushTimer = undefined;
drainFrameGate(ctx, term);
}, FRAME_GATE_FLUSH_MS);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop polling incomplete frames without a progress trigger

When state.buffer contains only an unterminated DEC 2026 frame, each retry produces the same partial value and unconditionally schedules another retry eight milliseconds later. No timer can complete that frame—only new session data can—so a stalled or disconnected session with a partial frame generates roughly 125 wakeups per second indefinitely while its tab remains mounted. Schedule polling only for complete output held by backlog, since new arrivals already invoke drainFrameGate themselves.

Useful? React with 👍 / 👎.

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
@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44a74dfcb9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +542 to +543
state.buffer = partial;
state.ingress = ingressPartial;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fail open incomplete synchronized frames

When a DEC 2026 opener is not followed promptly by its closer, partial is retained without being forwarded, acknowledged, timed out, or size-limited. A process interrupted or killed mid-frame therefore causes its prompt and all subsequent output to remain invisible indefinitely; additionally, an SSH frame larger than the 1 MiB flow watermark can deadlock because the backend pauses before the withheld closer can arrive. Add a timeout/size-based fail-open path that releases incomplete frames to xterm.

Useful? React with 👍 / 👎.

Comment on lines +143 to +145
&& isPureVisualPayload(cur.content)
&& startsWithCursorHome(next.content)
&& next.content.length >= minSuccessorRepaintBytes

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Verify repainted cells before dropping the prior frame

When backlog collapse runs, next.content.length >= cols * rows does not demonstrate a full-screen repaint because escape sequences are included in that length. For example, on an 80x24 terminal, updating only 100 cells with per-cell truecolor SGR sequences can exceed 1,920 characters while leaving most cells untouched; the preceding frame is then dropped and those untouched cells remain stale. Count actual cell writes/coverage or require a semantic clear/full repaint instead of raw payload length.

Useful? React with 👍 / 👎.

Comment on lines +53 to +54
const isPureVisualPayload = (content: string): boolean =>
!content.includes("\x1b]") && !content.includes("\x1b[?");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict droppable frames to visual control sequences

When a frame is superseded under backlog, this blacklist classifies every payload without OSC or private CSI as purely visual, even though it can contain BEL, DCS/APC commands, terminal queries, and other non-visual side effects. A synchronized frame carrying a bell or device-control request can therefore be dropped before xterm processes it, losing the notification or response. Parse an allowlist of cursor, SGR, erase, and cell-output operations rather than excluding only two prefixes.

Useful? React with 👍 / 👎.

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
@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c34c6e2822

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

queue.active = undefined;
queue.drainBytes = 0;
if (unacked > 0) {
queue.onDropped?.(unacked);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid double-acking writes after watchdog recovery

When an xterm callback is delayed beyond 250 ms rather than permanently lost, this handler reports the write as dropped and acknowledges its bytes, but cancelling the queue item does not cancel the in-flight write closure. When the real callback eventually arrives, terminalSessionAttachment.ts still calls flow.written() and sends the IPC acknowledgement before invoking the now-ignored queue callback, so the same bytes are acknowledged twice. Under a slow large write this understates the renderer backlog and can prevent flow control from pausing subsequent output.

Useful? React with 👍 / 👎.

"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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject ED3 frames from visual-only dropping

Allowing every J sequence classifies CSI 3 J as a droppable visual erase, although ED3 clears saved scrollback rather than merely repainting viewport pixels. If such a synchronized frame is followed by a frame that passes the repaint threshold, collapseAndSplit removes the ED3 frame entirely, leaving history that the application explicitly requested xterm to erase. Inspect the CSI parameters here and preserve stateful erase variants instead of treating all J commands alike.

Useful? React with 👍 / 👎.

setTerminalWriteCoalescerFlushGate(term);
pendingTimestampSecondByTerm.delete(term);
resetDeferredTerminalWriteAck(term);
resetFrameGate(term);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Flush the frame gate before resetting it

If hibernation or detachment races with an incomplete DEC 2026 frame, its bytes live only in FrameGateState.buffer; the preceding coalescer/queue flush and flow release do not see them. Deleting the state here therefore loses already-delivered terminal output that was not included in the hibernation snapshot, and its ingress bytes are never acknowledged to the backend. Forward or explicitly drop-and-ack the gate buffer before clearing it.

Useful? React with 👍 / 👎.

Comment on lines +625 to +626
const state = frameGateStates.get(term);
const engaged = (state && state.buffer.length > 0) || data.includes("\x1b[?2026h");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track synchronized-output openers across chunks

When the eight-byte DEC 2026 opener is split across two PTY data events, neither event contains the complete string, so both take the direct path and the frame gate never engages for that frame. PTY chunk boundaries are arbitrary, and a consistently aligned animated stream can therefore bypass the frame-dropping behavior this change is intended to provide. Preserve a short possible-opener suffix between calls or use the existing stateful escape parsing rather than relying on a chunk-local includes check.

Useful? React with 👍 / 👎.

…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
@binaricat

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 651c447b84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +165 to +166
if (unacked > 0) {
queue.onDropped?.(unacked);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid double-acking watchdog-recovered writes

When an xterm write callback arrives after the 250 ms watchdog, this onDropped call immediately invokes controller.written and the IPC acknowledgement through the registered drop handler. The original callback is not cancelled, however, and writeSessionDataImmediate subsequently performs the same flow accounting before its queue continuation becomes a no-op. This double acknowledgement can consume bytes belonging to later writes and resume a flooding PTY while output remains queued; make the completion accounting idempotent across watchdog and late-callback paths.

Useful? React with 👍 / 👎.

Comment on lines +616 to +620
// 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 = "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Flush held frames before writing session-exit output

If a process exits after opening a DEC 2026 block but before sending its closer, onSessionExit writes the [session closed] line immediately while this fail-open timer remains armed. The timer then forwards the older partial frame 200 ms later, reversing terminal-output order and potentially overwriting or hiding the exit message. Reset or synchronously flush the frame gate before lifecycle lines are enqueued.

Useful? React with 👍 / 👎.

@s-celles

Copy link
Copy Markdown
Author

@binaricat could you take the lead on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants