From 3d896eee8a4866d07b23f541991c3ff831e2b1ac Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 15:49:55 -0400 Subject: [PATCH] fix: skip TUI render loop on non-TTY stdout to prevent CPU burn (#3095) (#3263) When gsd is spawned as an RPC bridge child process, stdout is a pipe (process.stdout.isTTY === undefined). The TUI render loop would run at ~4,600 renders/sec writing ANSI escape codes to the pipe, consuming 500%+ CPU per process while idle. Add isTTY guard to Terminal interface, ProcessTerminal.start(), TUI.start(), and requestRender() so the entire render pipeline is skipped on non-TTY stdout. RemoteTerminal (browser-backed) correctly reports isTTY=true. Co-authored-by: Claude Opus 4.6 --- .../src/modes/rpc/remote-terminal.ts | 6 + packages/pi-tui/src/terminal.ts | 14 ++ packages/pi-tui/src/tui.ts | 8 + src/tests/tui-non-tty-render-loop.test.ts | 143 ++++++++++++++++++ 4 files changed, 171 insertions(+) create mode 100644 src/tests/tui-non-tty-render-loop.test.ts diff --git a/packages/pi-coding-agent/src/modes/rpc/remote-terminal.ts b/packages/pi-coding-agent/src/modes/rpc/remote-terminal.ts index 84f78f950..4dda9b0c9 100644 --- a/packages/pi-coding-agent/src/modes/rpc/remote-terminal.ts +++ b/packages/pi-coding-agent/src/modes/rpc/remote-terminal.ts @@ -49,6 +49,12 @@ export class RemoteTerminal implements Terminal { return this._rows; } + get isTTY(): boolean { + // RemoteTerminal renders to a browser-based terminal emulator via + // the RPC bridge — it behaves like a real TTY for rendering purposes. + return true; + } + get kittyProtocolActive(): boolean { return false; } diff --git a/packages/pi-tui/src/terminal.ts b/packages/pi-tui/src/terminal.ts index 52bb27ad3..ff84a6283 100644 --- a/packages/pi-tui/src/terminal.ts +++ b/packages/pi-tui/src/terminal.ts @@ -9,6 +9,9 @@ const cjsRequire = createRequire(import.meta.url); * Minimal terminal interface for TUI */ export interface Terminal { + // Whether stdout is a real TTY (false for pipes, e.g. RPC bridge processes) + readonly isTTY: boolean; + // Start the terminal with input and resize handlers start(onInput: (data: string) => void, onResize: () => void): void; @@ -63,11 +66,22 @@ export class ProcessTerminal implements Terminal { private stdinDataHandler?: (data: string) => void; private writeLogPath = process.env.PI_TUI_WRITE_LOG || ""; + get isTTY(): boolean { + return !!process.stdout.isTTY; + } + get kittyProtocolActive(): boolean { return this._kittyProtocolActive; } start(onInput: (data: string) => void, onResize: () => void): void { + // Non-TTY stdout (pipe) — skip TUI initialization entirely. + // RPC bridge processes communicate via JSON, not terminal escape codes. + // Without this guard, the render loop burns 500%+ CPU. (issue #3095) + if (!this.isTTY) { + return; + } + this.inputHandler = onInput; this.resizeHandler = onResize; diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index d0154b0ce..8e3db6f05 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -399,6 +399,12 @@ export class TUI extends Container { start(): void { this.stopped = false; + // Non-TTY stdout (pipe) — skip TUI entirely to avoid burning CPU. + // RPC bridge processes have piped stdio; rendering ANSI escape codes + // to a pipe is pure waste and causes a runaway render loop. (issue #3095) + if (!this.terminal.isTTY) { + return; + } this.terminal.start( (data) => this.handleInput(data), () => this.requestRender(), @@ -458,6 +464,8 @@ export class TUI extends Container { } requestRender(force = false): void { + // Skip rendering on non-TTY stdout to prevent CPU burn (issue #3095) + if (!this.terminal.isTTY) return; if (force) { this.previousLines = []; this.previousWidth = -1; // -1 triggers widthChanged, forcing a full clear diff --git a/src/tests/tui-non-tty-render-loop.test.ts b/src/tests/tui-non-tty-render-loop.test.ts new file mode 100644 index 000000000..2e6e4677d --- /dev/null +++ b/src/tests/tui-non-tty-render-loop.test.ts @@ -0,0 +1,143 @@ +/** + * Test: RPC bridge TUI render loop must not burn CPU on non-TTY stdout. + * + * When gsd is spawned as an RPC bridge child process, stdout is a pipe + * (process.stdout.isTTY === undefined). The TUI render loop must not + * start in that scenario — otherwise it runs at ~4,600 renders/second + * consuming 500%+ CPU doing nothing useful. + * + * Regression test for: https://github.com/gsd-build/gsd-2/issues/3095 + */ +import { describe, it, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { ProcessTerminal } from "@gsd/pi-tui"; +import { TUI } from "@gsd/pi-tui"; +import type { Terminal } from "@gsd/pi-tui"; + +/** + * A mock terminal that tracks writes and render activity. + * Simulates a non-TTY environment (isTTY = false). + */ +class MockNonTTYTerminal implements Terminal { + public started = false; + public writeCount = 0; + public writtenData: string[] = []; + private _onInput?: (data: string) => void; + private _onResize?: () => void; + + /** Simulates non-TTY stdout */ + readonly isTTY = false; + + start(onInput: (data: string) => void, onResize: () => void): void { + this.started = true; + this._onInput = onInput; + this._onResize = onResize; + } + + stop(): void { + this.started = false; + } + + async drainInput(_maxMs?: number, _idleMs?: number): Promise {} + + write(data: string): void { + this.writeCount++; + this.writtenData.push(data); + } + + get columns(): number { return 80; } + get rows(): number { return 24; } + get kittyProtocolActive(): boolean { return false; } + + moveBy(_lines: number): void {} + hideCursor(): void {} + showCursor(): void {} + clearLine(): void {} + clearFromCursor(): void {} + clearScreen(): void {} + setTitle(_title: string): void {} +} + +/** + * A mock terminal that behaves like a real TTY. + */ +class MockTTYTerminal extends MockNonTTYTerminal { + override readonly isTTY = true as const; +} + +describe("TUI non-TTY render loop guard (issue #3095)", () => { + it("ProcessTerminal.start() should be a no-op when stdout is not a TTY", () => { + // ProcessTerminal.start() accesses process.stdout directly. + // We verify it exposes isTTY so callers can check before starting. + const terminal = new ProcessTerminal(); + // ProcessTerminal.isTTY should reflect process.stdout.isTTY + assert.equal( + typeof terminal.isTTY, + "boolean", + "ProcessTerminal must expose an isTTY property" + ); + }); + + it("TUI.start() must not render when terminal.isTTY is false", async () => { + const terminal = new MockNonTTYTerminal(); + const tui = new TUI(terminal); + + tui.start(); + + // Wait for any nextTick-scheduled renders to fire + await new Promise((resolve) => setTimeout(resolve, 50)); + + // The TUI should NOT have produced any render output on a non-TTY terminal + assert.equal( + terminal.writeCount, + 0, + `TUI rendered ${terminal.writeCount} times on non-TTY stdout — ` + + `this would cause the CPU burn described in #3095. ` + + `Expected 0 writes when isTTY is false.` + ); + + // Clean up + tui.stop(); + }); + + it("TUI.start() renders normally when terminal.isTTY is true", async () => { + const terminal = new MockTTYTerminal(); + const tui = new TUI(terminal); + + tui.start(); + + // Wait for nextTick-scheduled render + await new Promise((resolve) => setTimeout(resolve, 50)); + + // On a TTY terminal, at least one render should have occurred + assert.ok( + terminal.writeCount > 0, + "TUI should render at least once on a TTY terminal" + ); + + tui.stop(); + }); + + it("requestRender() must be a no-op when terminal.isTTY is false", async () => { + const terminal = new MockNonTTYTerminal(); + const tui = new TUI(terminal); + + tui.start(); + + // Force multiple render requests + tui.requestRender(); + tui.requestRender(); + tui.requestRender(); + + // Wait for any scheduled renders + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.equal( + terminal.writeCount, + 0, + "requestRender() must not write to non-TTY stdout" + ); + + tui.stop(); + }); +});