From 5e7bca95d9852057b89bfd8bd163219bd2aec306 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 28 Jun 2026 04:00:22 -0700 Subject: [PATCH] fix(tui): coalesce render frames while stdout backpressure is unresolved (#31486) (#54171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the previous frame's stdout.write has not drained (the outer terminal parser is overwhelmed by a wide CR+LF burst — CJK + ANSI tool output on a high-context session), the renderer kept writing a new frame every tick. That piled writes onto an already-backed-up pipe and kept the macrotask queue hot, starving the stdin 'readable' callback — the observed stdin freeze where the agent loop keeps running but keystrokes/Ctrl-C are dead. onRender now coalesces: while pendingWriteStart is non-null (prior write's drain callback hasn't fired) it skips the frame and retries on the drain tick instead of writing. A MAX_COALESCED_BACKPRESSURE_FRAMES ceiling forces a write through after N skips so a terminal whose drain callback never fires (OSError EIO on flush) self-heals once the pipe recovers rather than wedging forever. TTY-only; piped stdout has no flow control. Coalesce counter resets on every real write. This is the stdout-backpressure strand left open after #54046 fixed the swallowed-exception strand. --- .../packages/hermes-ink/src/ink/constants.ts | 13 ++ .../src/ink/ink-backpressure.test.ts | 194 ++++++++++++++++++ ui-tui/packages/hermes-ink/src/ink/ink.tsx | 37 +++- 3 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts diff --git a/ui-tui/packages/hermes-ink/src/ink/constants.ts b/ui-tui/packages/hermes-ink/src/ink/constants.ts index 1846997c0cb..c0753143931 100644 --- a/ui-tui/packages/hermes-ink/src/ink/constants.ts +++ b/ui-tui/packages/hermes-ink/src/ink/constants.ts @@ -4,3 +4,16 @@ export const FRAME_INTERVAL_MS = 16 // Keep clock-driven animations at full speed when terminal focus changes. // We still pause entirely when there are no keepAlive subscribers. export const BLURRED_FRAME_INTERVAL_MS = FRAME_INTERVAL_MS + +// Issue #31486 (stdout-backpressure strand): when the previous frame's +// stdout.write has NOT drained yet (terminal parser overwhelmed by a wide +// CR+LF burst — CJK + ANSI tool output on a high-context session), piling +// another write on the backed-up pipe both wastes the frame and keeps the +// macrotask queue churning, starving the stdin 'readable' callback. We +// instead COALESCE: skip the frame and retry on the drain tick. This ceiling +// caps how many consecutive frames we'll coalesce before forcing a write +// through, so a terminal whose drain callback never fires (e.g. EIO on +// flush) can't wedge the renderer permanently — it self-heals once the pipe +// recovers. ~10 frames at the drain-tick cadence is a few hundred ms of +// breathing room, well under any human-perceptible render stall. +export const MAX_COALESCED_BACKPRESSURE_FRAMES = 10 diff --git a/ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts b/ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts new file mode 100644 index 00000000000..0c76c08babf --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts @@ -0,0 +1,194 @@ +import { EventEmitter } from 'events' + +import React from 'react' +import { describe, expect, it, vi } from 'vitest' + +import Text from './components/Text.js' +import { MAX_COALESCED_BACKPRESSURE_FRAMES } from './constants.js' +import Ink from './ink.js' + +// Regression for issue #31486 (stdout-backpressure strand): when the +// previous frame's stdout.write has not drained (the terminal parser is +// overwhelmed — a wide CR+LF burst on a high-context session), the renderer +// must COALESCE rather than pile another write on the backed-up pipe. Piling +// writes keeps the macrotask queue hot and starves the stdin 'readable' +// callback, which is the observed freeze. The coalesce must be bounded: after +// MAX_COALESCED_BACKPRESSURE_FRAMES skipped frames it forces a write through +// so a terminal whose drain callback never fires can't wedge the renderer. + +/** + * A TTY whose write() reports backpressure (returns false) and WITHHOLDS the + * drain callback until fireDrain() is called — simulating a wedged terminal + * parser. Each write records its drain callback so the test controls timing. + */ +class WedgedTty extends EventEmitter { + chunks: string[] = [] + columns = 20 + rows = 5 + isTTY = true + private pendingDrains: Array<(err?: Error | null) => void> = [] + + write(chunk: string | Uint8Array, cb?: (err?: Error | null) => void): boolean { + this.chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')) + + if (cb) { + // Hold the callback — do NOT fire it. This leaves the renderer's + // pendingWriteStart non-null, the backpressure signal it coalesces on. + this.pendingDrains.push(cb) + } + + // Report backpressure. + return false + } + + /** Fire all withheld drain callbacks, simulating the pipe recovering. */ + fireDrain(): void { + const drains = this.pendingDrains + this.pendingDrains = [] + + for (const cb of drains) { + cb() + } + } + + get pendingDrainCount(): number { + return this.pendingDrains.length + } +} + +/** A normal fast TTY: write succeeds and drains synchronously. */ +class FastTty extends EventEmitter { + chunks: string[] = [] + columns = 20 + rows = 5 + isTTY = true + + write(chunk: string | Uint8Array, cb?: (err?: Error | null) => void): boolean { + this.chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')) + cb?.() + + return true + } +} + +const makeInk = (stdout: WedgedTty | FastTty) => { + const stdin = new EventEmitter() as unknown as NodeJS.ReadStream + const stderr = new FastTty() + + return new Ink({ + exitOnCtrlC: false, + patchConsole: false, + stderr: stderr as unknown as NodeJS.WriteStream, + stdin, + stdout: stdout as unknown as NodeJS.WriteStream + }) +} + +describe('Ink stdout backpressure coalescing (issue #31486)', () => { + it('coalesces frames while the previous write has not drained', () => { + vi.useFakeTimers() + + try { + const stdout = new WedgedTty() + const ink = makeInk(stdout) + + ink.render(React.createElement(Text, null, 'hello')) + ink.onRender() + + // First frame wrote (and reported backpressure; drain withheld). + const writesAfterFirst = stdout.chunks.length + expect(writesAfterFirst).toBeGreaterThan(0) + expect(stdout.pendingDrainCount).toBe(1) + + // Subsequent renders while the write is still pending must coalesce — + // no new bytes written, a retry timer scheduled instead. + ink.render(React.createElement(Text, null, 'world')) + ink.onRender() + expect(stdout.chunks.length).toBe(writesAfterFirst) + + ink.onRender() + expect(stdout.chunks.length).toBe(writesAfterFirst) + + ink.unmount() + } finally { + vi.useRealTimers() + } + }) + + it('resumes writing once the wedged pipe drains', () => { + vi.useFakeTimers() + + try { + const stdout = new WedgedTty() + const ink = makeInk(stdout) + + ink.render(React.createElement(Text, null, 'hello')) + ink.onRender() + const writesAfterFirst = stdout.chunks.length + + // Backed up: this render coalesces. + ink.render(React.createElement(Text, null, 'changed')) + ink.onRender() + expect(stdout.chunks.length).toBe(writesAfterFirst) + + // Pipe recovers — drain callback fires, clearing pendingWriteStart. + stdout.fireDrain() + + // The retry tick now finds the pipe drained and writes the pending frame. + vi.runAllTimers() + expect(stdout.chunks.length).toBeGreaterThan(writesAfterFirst) + + ink.unmount() + } finally { + vi.useRealTimers() + } + }) + + it('forces a write through after the coalesce ceiling so it never wedges forever', () => { + vi.useFakeTimers() + + try { + const stdout = new WedgedTty() + const ink = makeInk(stdout) + + ink.render(React.createElement(Text, null, 'hello')) + ink.onRender() + const writesAfterFirst = stdout.chunks.length + + // Mark content dirty and drive renders. The drain callback NEVER fires + // (pendingDrainCount stays > 0). After MAX_COALESCED_BACKPRESSURE_FRAMES + // coalesced retries, the renderer must force a write through. + ink.render(React.createElement(Text, null, 'forced')) + + // Drive enough retry ticks to exceed the ceiling. + for (let i = 0; i <= MAX_COALESCED_BACKPRESSURE_FRAMES + 2; i++) { + vi.advanceTimersByTime(4) + } + + // A write was forced through despite the never-firing drain callback. + expect(stdout.chunks.length).toBeGreaterThan(writesAfterFirst) + + ink.unmount() + } finally { + vi.useRealTimers() + } + }) + + it('never coalesces on a fast terminal that drains synchronously', () => { + const stdout = new FastTty() + const ink = makeInk(stdout) + + ink.render(React.createElement(Text, null, 'a')) + ink.onRender() + const afterA = stdout.chunks.length + expect(afterA).toBeGreaterThan(0) + + // Each changed render writes immediately — synchronous drain clears the + // backpressure signal before the next frame, so nothing is coalesced. + ink.render(React.createElement(Text, null, 'b')) + ink.onRender() + expect(stdout.chunks.length).toBeGreaterThan(afterA) + + ink.unmount() + }) +}) diff --git a/ui-tui/packages/hermes-ink/src/ink/ink.tsx b/ui-tui/packages/hermes-ink/src/ink/ink.tsx index d8c95fcc703..4c175721466 100644 --- a/ui-tui/packages/hermes-ink/src/ink/ink.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/ink.tsx @@ -18,7 +18,7 @@ import { colorize } from './colorize.js' import App from './components/App.js' import type { CursorAdvanceNotifier } from './components/CursorAdvanceContext.js' import type { CursorDeclaration, CursorDeclarationSetter } from './components/CursorDeclarationContext.js' -import { FRAME_INTERVAL_MS } from './constants.js' +import { FRAME_INTERVAL_MS, MAX_COALESCED_BACKPRESSURE_FRAMES } from './constants.js' import * as dom from './dom.js' import { markDirty } from './dom.js' import { KeyboardEvent } from './events/keyboard-event.js' @@ -205,6 +205,11 @@ export default class Ink { // (callback fired). private pendingWriteStart: number | null = null private lastDrainMs = 0 + // Issue #31486: count of consecutive frames skipped because the previous + // write hadn't drained. Reset to 0 whenever a frame actually writes (or the + // pipe has drained). Capped by MAX_COALESCED_BACKPRESSURE_FRAMES so a + // never-firing drain callback can't coalesce forever. + private coalescedBackpressureFrames = 0 private lastYogaCounters: { ms: number visited: number @@ -703,6 +708,36 @@ export default class Ink { this.drainTimer = null } + // Issue #31486 (stdout-backpressure strand): if the PREVIOUS frame's + // stdout.write still hasn't drained (callback hasn't fired — + // pendingWriteStart is non-null), the outer terminal is consuming bytes + // slower than we're producing them. Piling another write on the backed-up + // pipe is wasted work AND keeps the macrotask queue hot, which is what + // starves the stdin 'readable' callback and wedges input. Coalesce: + // skip this frame's render+write entirely and retry on the drain tick. + // The ceiling guarantees forward progress — after N coalesced frames we + // force the write through, so a terminal whose drain callback NEVER fires + // (e.g. OSError EIO on flush) self-heals once the pipe recovers instead of + // coalescing forever. Only on a TTY; piped stdout has no flow control and + // pendingWriteStart is never set there. + if ( + this.options.stdout.isTTY && + this.pendingWriteStart !== null && + this.coalescedBackpressureFrames < MAX_COALESCED_BACKPRESSURE_FRAMES + ) { + this.coalescedBackpressureFrames += 1 + this.isRendering = false + // Retry at the same cadence as a scroll drain tick. Don't use + // scheduleRender — lodash throttle's leading edge would re-enter here. + this.drainTimer = setTimeout(() => this.onRender(), FRAME_INTERVAL_MS >> 2) + + return + } + + // Either we wrote, or we hit the ceiling and are forcing a write through. + // Reset the coalesce counter so the next backpressure episode starts fresh. + this.coalescedBackpressureFrames = 0 + // Flush deferred interaction-time update before rendering so we call // Date.now() at most once per frame instead of once per keypress. // Done before the render to avoid dirtying state that would trigger