From 2d00ac3e45c66763e558cc2d232b88ead3ee1bdb Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 10 Jun 2026 01:28:22 +0200 Subject: [PATCH] fix(tui): restored terminal state on crash and unwedged paste-mode input emergency restore leaves the alt screen and disables mouse tracking; bracketed paste gets an inactivity watchdog and byte cap so a lost end marker cannot eat input forever; split-escape flush window raised to 50ms; kitty printable dedup expires; resetDisplay repaints on the alt screen; input scanning is index-based instead of O(n^2) slicing; appearance poll no longer clears selection every 2s. --- packages/tui/src/stdin-buffer.ts | 130 ++++++++++++++---- packages/tui/src/terminal.ts | 24 +++- packages/tui/src/tui.ts | 16 ++- packages/tui/test/stdin-buffer.test.ts | 102 +++++++++++++- packages/tui/test/terminal-appearance.test.ts | 22 +-- 5 files changed, 250 insertions(+), 44 deletions(-) diff --git a/packages/tui/src/stdin-buffer.ts b/packages/tui/src/stdin-buffer.ts index c5189a885..3cde729b7 100644 --- a/packages/tui/src/stdin-buffer.ts +++ b/packages/tui/src/stdin-buffer.ts @@ -21,6 +21,14 @@ import { EventEmitter } from "events"; const ESC = "\x1b"; const BRACKETED_PASTE_START = "\x1b[200~"; const BRACKETED_PASTE_END = "\x1b[201~"; +// Paste-mode recovery bounds: a lost/corrupted end marker (ssh/tmux +// truncation) must not hang input forever or grow memory unboundedly. +const PASTE_INACTIVITY_TIMEOUT_MS = 1000; +const PASTE_MAX_BYTES = 64 * 1024 * 1024; +// A buggy double-report (CSI-u event plus the bare printable for the same +// keypress) arrives in the same terminal write; a bare char that shows up +// later than this window is a real keystroke and must not be swallowed. +const KITTY_PRINTABLE_DEDUP_WINDOW_MS = 25; /** * Check if a string is a complete escape sequence or needs more data @@ -202,41 +210,41 @@ function parseUnmodifiedKittyPrintableCodepoint(sequence: string): number | unde function extractCompleteSequences(buffer: string): { sequences: string[]; remainder: string } { const sequences: string[] = []; + const length = buffer.length; let pos = 0; - while (pos < buffer.length) { - const remaining = buffer.slice(pos); - - // Try to extract a sequence starting at this position - if (remaining.startsWith(ESC)) { - // Find the end of this escape sequence - let seqEnd = 1; - while (seqEnd <= remaining.length) { - const candidate = remaining.slice(0, seqEnd); + // Index-based scanning: this is the input hot path. Slicing the remaining + // buffer (or Array.from-ing it) per iteration would make plain-text bursts + // O(n²) — a 100KB non-bracketed paste must stay O(n). + while (pos < length) { + if (buffer.charCodeAt(pos) === 0x1b) { + // Find the end of this escape sequence by growing the candidate. + let end = pos + 1; + let consumed = false; + while (end <= length) { + const candidate = buffer.slice(pos, end); const status = isCompleteSequence(candidate); - - if (status === "complete") { - sequences.push(candidate); - pos += seqEnd; - break; - } else if (status === "incomplete") { - seqEnd++; - } else { - // Should not happen when starting with ESC - sequences.push(candidate); - pos += seqEnd; - break; + if (status === "incomplete") { + end++; + continue; } + // "complete" — or "not-escape", which should not happen when + // starting with ESC; both consume the candidate. + sequences.push(candidate); + pos = end; + consumed = true; + break; } - if (seqEnd > remaining.length) { - return { sequences, remainder: remaining }; + if (!consumed) { + return { sequences, remainder: buffer.slice(pos) }; } } else { // Not an escape sequence - take one Unicode scalar, not a UTF-16 code unit. - const char = Array.from(remaining)[0] ?? ""; - sequences.push(char); - pos += char.length; + const codePoint = buffer.codePointAt(pos)!; + const charLength = codePoint > 0xffff ? 2 : 1; + sequences.push(buffer.slice(pos, pos + charLength)); + pos += charLength; } } @@ -249,6 +257,17 @@ export type StdinBufferOptions = { * After this time, a genuinely incomplete escape is flushed. */ timeout?: number; + /** + * Paste-mode inactivity watchdog (default: 1000ms). If no input arrives for + * this long while waiting for the bracketed-paste end marker, the paste is + * assumed truncated: accumulated bytes are delivered and input recovers. + */ + pasteTimeout?: number; + /** + * Paste-mode byte cap (default: 64 MiB). Exceeding it aborts paste mode the + * same way, bounding memory when the end marker never arrives. + */ + pasteByteLimit?: number; }; export type StdinBufferEventMap = { @@ -264,14 +283,21 @@ export class StdinBuffer extends EventEmitter { #buffer: string = ""; #timeout?: NodeJS.Timeout; readonly #timeoutMs: number; + readonly #pasteTimeoutMs: number; + readonly #pasteByteLimit: number; #pasteMode: boolean = false; #pasteChunks: string[] = []; #pasteOverlap: string = ""; + #pasteBytes = 0; + #pasteWatchdog?: NodeJS.Timeout; #pendingKittyPrintableCodepoint: number | undefined; + #pendingKittyPrintableAtMs = 0; constructor(options: StdinBufferOptions = {}) { super(); this.#timeoutMs = options.timeout ?? 75; + this.#pasteTimeoutMs = options.pasteTimeout ?? PASTE_INACTIVITY_TIMEOUT_MS; + this.#pasteByteLimit = options.pasteByteLimit ?? PASTE_MAX_BYTES; } process(data: string | Buffer): void { @@ -326,6 +352,7 @@ export class StdinBuffer extends EventEmitter { this.#pasteMode = true; this.#pasteChunks = []; this.#pasteOverlap = ""; + this.#pasteBytes = 0; this.#consumePasteChunk(firstChunk); return; } @@ -360,8 +387,14 @@ export class StdinBuffer extends EventEmitter { const probe = this.#pasteOverlap + chunk; if (probe.indexOf(BRACKETED_PASTE_END) === -1) { this.#pasteChunks.push(chunk); + this.#pasteBytes += chunk.length; const keep = BRACKETED_PASTE_END.length - 1; this.#pasteOverlap = probe.length > keep ? probe.slice(probe.length - keep) : probe; + if (this.#pasteBytes > this.#pasteByteLimit) { + this.#abortPaste(); + return; + } + this.#armPasteWatchdog(); return; } @@ -372,9 +405,11 @@ export class StdinBuffer extends EventEmitter { const pastedContent = flat.slice(0, endIndex); const remaining = flat.slice(endIndex + BRACKETED_PASTE_END.length); + this.#clearPasteWatchdog(); this.#pasteMode = false; this.#pasteChunks = []; this.#pasteOverlap = ""; + this.#pasteBytes = 0; this.#pendingKittyPrintableCodepoint = undefined; this.emit("paste", pastedContent); @@ -384,14 +419,53 @@ export class StdinBuffer extends EventEmitter { } } + /** Re-arm the paste-mode inactivity watchdog after each chunk. */ + #armPasteWatchdog(): void { + if (this.#pasteWatchdog) clearTimeout(this.#pasteWatchdog); + this.#pasteWatchdog = setTimeout(() => { + this.#pasteWatchdog = undefined; + this.#abortPaste(); + }, this.#pasteTimeoutMs); + } + + #clearPasteWatchdog(): void { + if (this.#pasteWatchdog) { + clearTimeout(this.#pasteWatchdog); + this.#pasteWatchdog = undefined; + } + } + + /** + * Recover from a paste whose end marker never arrived (dropped or corrupted + * in transit, or past the byte cap): exit paste mode and deliver the + * accumulated bytes as a paste, so they are neither lost, replayed as + * keystrokes, nor accumulated forever while input appears dead. + */ + #abortPaste(): void { + this.#clearPasteWatchdog(); + const content = this.#pasteChunks.join(""); + this.#pasteMode = false; + this.#pasteChunks = []; + this.#pasteOverlap = ""; + this.#pasteBytes = 0; + this.emit("paste", content); + } + #emitDataSequence(sequence: string): void { const rawCodepoint = sequence.length === 1 ? sequence.codePointAt(0) : undefined; - if (rawCodepoint !== undefined && rawCodepoint === this.#pendingKittyPrintableCodepoint) { + if ( + rawCodepoint !== undefined && + rawCodepoint === this.#pendingKittyPrintableCodepoint && + Date.now() - this.#pendingKittyPrintableAtMs <= KITTY_PRINTABLE_DEDUP_WINDOW_MS + ) { this.#pendingKittyPrintableCodepoint = undefined; return; } this.#pendingKittyPrintableCodepoint = parseUnmodifiedKittyPrintableCodepoint(sequence); + if (this.#pendingKittyPrintableCodepoint !== undefined) { + this.#pendingKittyPrintableAtMs = Date.now(); + } this.emit("data", sequence); } @@ -416,10 +490,12 @@ export class StdinBuffer extends EventEmitter { clearTimeout(this.#timeout); this.#timeout = undefined; } + this.#clearPasteWatchdog(); this.#buffer = ""; this.#pasteMode = false; this.#pasteChunks = []; this.#pasteOverlap = ""; + this.#pasteBytes = 0; this.#pendingKittyPrintableCodepoint = undefined; } diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 46b6afff7..26f852f09 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -134,6 +134,11 @@ export function emergencyTerminalRestore(): void { const terminal = activeTerminal; if (terminal) { terminal.stop(); + // stop() never touches the alternate screen — the TUI owns that + // state and exits it on the normal shutdown path. A crash while a + // fullscreen overlay is up would otherwise strand the shell on the + // alt buffer. Safe no-op when the alt screen is not active. + terminal.write("\x1b[?1049l"); terminal.showCursor(); } else if (terminalEverStarted) { // Blind restore only if we know a terminal was started but lost track of it @@ -147,6 +152,8 @@ export function emergencyTerminalRestore(): void { "\x1b[?5522l" + // Disable enhanced paste notifications "\x1b[4;0m" + // Disable modifyOtherKeys fallback + "\x1b[?1006l\x1b[?1003l\x1b[?1000l" + // Disable mouse tracking (fullscreen overlays) + "\x1b[?1049l" + // Leave the alternate screen (fullscreen overlays) "\x1b[?25h", // Show cursor ); if (process.stdin.setRawMode) { @@ -450,7 +457,12 @@ export class ProcessTerminal implements Terminal { * to handle the case where the response arrives split across multiple events. */ #setupStdinBuffer(): void { - this.#stdinBuffer = new StdinBuffer({ timeout: 10 }); + // 50ms balances two failure modes: a bare ESC keypress on legacy + // terminals waits this long before it is delivered, while a CSI key + // escape split across stdin reads (laggy ssh/tmux links) leaks as + // literal typed text if the flush fires between the fragments. 10ms + // proved too tight for split escapes (#1238 covered only probe replies). + this.#stdinBuffer = new StdinBuffer({ timeout: 50 }); // Kitty protocol response pattern: \x1b[?u const kittyResponsePattern = /^\x1b\[\?(\d+)u$/; @@ -815,6 +827,9 @@ export class ProcessTerminal implements Terminal { /** * Start periodic OSC 11 re-queries for terminals without Mode 2031 (Warp, Alacritty, WezTerm). * Self-disables once Mode 2031 fires (push-based is better than polling). + * The interval is deliberately long: each poll's OSC 11 + DA1 write clears + * an active text selection on several terminals, so polling exists only to + * eventually notice a rare OS theme switch, not to track it promptly. */ #startOsc11Poll(): void { this.#stopOsc11Poll(); @@ -824,7 +839,7 @@ export class ProcessTerminal implements Terminal { return; } this.#queryBackgroundColor(); - }, 2_000); + }, 30_000); this.#osc11PollTimer.unref(); } @@ -1016,6 +1031,11 @@ export class ProcessTerminal implements Terminal { this.#safeWrite("\x1b[?2004l"); this.#safeWrite("\x1b[?5522l"); + // Disable mouse tracking (enabled only by fullscreen overlays; safe + // no-ops otherwise). Covers crash paths that reach stop() without the + // TUI's own overlay teardown running. + this.#safeWrite("\x1b[?1006l\x1b[?1003l\x1b[?1000l"); + // Disable Mode 2031 appearance change notifications this.#safeWrite("\x1b[?2031l"); diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 9badfc9d8..6a3297445 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -1725,6 +1725,12 @@ export class TUI extends Container { this.#imageBudget.beginPass(); const rawFrame = this.render(width); this.#imageBudget.endPass(); + // Ghostty initial-image deferral must run before any render state is + // consumed (#resizeEventPending, hardware-cursor state, commit + // re-anchoring): the early return abandons this frame and the deferred + // render recomposes from scratch, so consuming state here would + // misclassify a pending resize as an ordinary diff and corrupt the paint. + if (this.#maybeDeferGhosttyInitialImagePaint()) return; // Strip cursor markers immediately (they are internal sentinels and // must never reach the terminal, the committed prefix, or the audit); // the visible marker is chosen after the window top is known. @@ -1853,7 +1859,6 @@ export class TUI extends Container { // Load newly-displayed image data once, before this frame's placements // (and any emitter) reference it. `a=t` produces no display, so writing // it ahead of the synchronized paint is artifact-free. - if (this.#maybeDeferGhosttyInitialImagePaint()) return; const imageTransmits = this.#imageBudget.takeTransmits(); if (imageTransmits.length > 0) { let transmitBuffer = ""; @@ -2279,8 +2284,13 @@ export class TUI extends Container { #emitAltFrame(lines: string[], width: number, height: number): void { const fitted: string[] = new Array(height); for (let r = 0; r < height; r++) fitted[r] = lines[r] ?? ""; - // Skip an identical repaint (the modal is mostly static between keystrokes). - if (this.#altPreviousLines.length === height) { + // Skip an identical repaint (the modal is mostly static between + // keystrokes) — unless a forced repaint (resetDisplay, + // requestRender(true)) is pending: the redraw gesture must repair a + // corrupted modal even when our cached frame is byte-identical. + const force = this.#forceViewportRepaintOnNextRender; + this.#forceViewportRepaintOnNextRender = false; + if (!force && this.#altPreviousLines.length === height) { let same = true; for (let r = 0; r < height; r++) { if (fitted[r] !== this.#altPreviousLines[r]) { diff --git a/packages/tui/test/stdin-buffer.test.ts b/packages/tui/test/stdin-buffer.test.ts index d88c1a42f..a20cccc34 100644 --- a/packages/tui/test/stdin-buffer.test.ts +++ b/packages/tui/test/stdin-buffer.test.ts @@ -5,7 +5,7 @@ * MIT License - Copyright (c) 2025 opentui */ -import { beforeEach, describe, expect, it } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { StdinBuffer } from "@oh-my-pi/pi-tui/stdin-buffer"; describe("StdinBuffer", () => { @@ -22,6 +22,13 @@ describe("StdinBuffer", () => { }); }); + afterEach(() => { + // Kill pending flush/watchdog timers: a stale timer from a prior test's + // buffer would otherwise emit into the current test's emittedSequences + // (the data listener closes over the reassigned module variable). + buffer.destroy(); + }); + // Helper to process data through the buffer function processInput(data: string | Buffer): void { buffer.process(data); @@ -129,6 +136,21 @@ describe("StdinBuffer", () => { }); }); + describe("Kitty Printable Dedup Window", () => { + it("swallows the immediate bare duplicate of a kitty printable", () => { + // Buggy double-report: CSI-u event plus the bare char in one write. + processInput("\x1b[97ua"); + expect(emittedSequences).toEqual(["\x1b[97u"]); + }); + + it("does not swallow a real keystroke after the dedup window expires", async () => { + processInput("\x1b[97u"); + await Bun.sleep(50); + processInput("a"); + expect(emittedSequences).toEqual(["\x1b[97u", "a"]); + }); + }); + describe("Mouse Events", () => { it("should handle mouse press event", () => { processInput("\x1b[<0;10;5M"); @@ -211,6 +233,35 @@ describe("StdinBuffer", () => { }); }); + describe("Large Plain-Text Bursts", () => { + it("splits a large non-bracketed burst into per-character events quickly", () => { + // Pins the O(n) scan: the prior per-iteration slice/Array.from made + // this O(n²) — a 64KB burst would blow the test timeout. + const content = "0123456789abcdef".repeat(4096); // 64 KB + processInput(content); + expect(emittedSequences.length).toBe(content.length); + expect(emittedSequences[0]).toBe("0"); + expect(emittedSequences[emittedSequences.length - 1]).toBe("f"); + }); + + it("keeps escape parsing and surrogate pairs intact inside a burst", () => { + processInput("abc🙂\x1b[A\u{1f389}def\x1b[<35;20;5m\x1b"); + expect(emittedSequences).toEqual([ + "a", + "b", + "c", + "🙂", + "\x1b[A", + "\u{1f389}", + "d", + "e", + "f", + "\x1b[<35;20;5m", + ]); + expect(buffer.getBuffer()).toBe("\x1b"); + }); + }); + describe("Flush", () => { it("should flush incomplete sequences", () => { processInput("\x1b[<35"); @@ -358,6 +409,55 @@ describe("StdinBuffer", () => { }); }); + describe("Paste Recovery", () => { + it("recovers from a lost end marker via the inactivity watchdog", async () => { + buffer = new StdinBuffer({ timeout: 10, pasteTimeout: 20 }); + const pastes: string[] = []; + const data: string[] = []; + buffer.on("paste", d => pastes.push(d)); + buffer.on("data", s => data.push(s)); + + buffer.process("\x1b[200~lost marker content"); + expect(pastes).toEqual([]); + + await Bun.sleep(60); + expect(pastes).toEqual(["lost marker content"]); + + // Input is alive again after recovery. + buffer.process("a"); + expect(data).toEqual(["a"]); + }); + + it("re-arms the watchdog while paste chunks keep arriving", async () => { + buffer = new StdinBuffer({ timeout: 10, pasteTimeout: 50 }); + const pastes: string[] = []; + buffer.on("paste", d => pastes.push(d)); + + buffer.process("\x1b[200~part1 "); + await Bun.sleep(20); + buffer.process("part2"); + await Bun.sleep(20); + expect(pastes).toEqual([]); // still inside the re-armed window + + buffer.process("\x1b[201~"); + expect(pastes).toEqual(["part1 part2"]); + }); + + it("aborts paste mode when the byte cap is exceeded", () => { + buffer = new StdinBuffer({ timeout: 10, pasteByteLimit: 8 }); + const pastes: string[] = []; + const data: string[] = []; + buffer.on("paste", d => pastes.push(d)); + buffer.on("data", s => data.push(s)); + + buffer.process("\x1b[200~0123456789abcdef"); + expect(pastes).toEqual(["0123456789abcdef"]); + + buffer.process("x"); + expect(data).toEqual(["x"]); + }); + }); + describe("Destroy", () => { it("should clear buffer on destroy", () => { processInput("\x1b[<35"); diff --git a/packages/tui/test/terminal-appearance.test.ts b/packages/tui/test/terminal-appearance.test.ts index e0a5007de..655fb059b 100644 --- a/packages/tui/test/terminal-appearance.test.ts +++ b/packages/tui/test/terminal-appearance.test.ts @@ -195,8 +195,8 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { const afterInitial = queryCount(); - // Advance 2s — poll should fire and send another query - vi.advanceTimersByTime(2000); + // Advance one poll interval — poll should fire and send another query + vi.advanceTimersByTime(30_000); expect(queryCount()).toBe(afterInitial + 1); // Complete poll's OSC 11 + DA1 (only one DA1 sentinel — keyboard probe is one-shot) @@ -212,8 +212,8 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { const afterMode2031 = queryCount(); - // Advance 4s — no additional poll queries should fire - vi.advanceTimersByTime(4000); + // Advance two more poll intervals — no additional poll queries should fire + vi.advanceTimersByTime(60_000); expect(queryCount()).toBe(afterMode2031); terminal.stop(); @@ -228,21 +228,21 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { process.stdin.emit("data", "\x1b[?1;2c"); process.stdin.emit("data", "\x1b[?1;2c"); - // Poll fires at 2s while Mode 2031 support is still unknown. + // Poll fires at the first interval while Mode 2031 support is still unknown. const afterInitial = queryCount(); - vi.advanceTimersByTime(2000); + vi.advanceTimersByTime(30_000); expect(queryCount()).toBe(afterInitial + 1); // Drain the poll's OSC 11 reply so it is no longer pending. process.stdin.emit("data", "\x1b]11;rgb:ffff/ffff/ffff\x07"); // DECRQM confirms Mode 2031 support — push notifications supersede polling, // so the poll must stop (its repeated OSC 11/DA1 writes otherwise clobber - // the user's active text selection every 2s). + // the user's active text selection on every poll). process.stdin.emit("data", "\x1b[?2031;3$y"); const afterConfirm = queryCount(); // Advance well past several poll intervals — no further OSC 11 queries fire. - vi.advanceTimersByTime(6000); + vi.advanceTimersByTime(90_000); expect(queryCount()).toBe(afterConfirm); terminal.stop(); @@ -259,7 +259,7 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { process.stdin.emit("data", "\x1b[?1;2c"); const afterInitial = queryCount(); - vi.advanceTimersByTime(4000); + vi.advanceTimersByTime(90_000); expect(queryCount()).toBe(afterInitial); @@ -335,7 +335,7 @@ describe("ProcessTerminal OSC 11 appearance detection", () => { process.stdin.emit("data", "\x1b]11;rgb:1c1c/1c1c/1c1c\x07"); // DA1 reply arrives split: the prefix appears as one event and then the StdinBuffer - // flush timeout (10ms) elapses before the rest of the response is delivered. + // flush timeout (50ms) elapses before the rest of the response is delivered. // xterm-style "VT420 with extensions" response: \x1b[?62;6;7;14;...;52c process.stdin.emit("data", "\x1b[?62"); vi.advanceTimersByTime(50); @@ -616,7 +616,7 @@ describe("ProcessTerminal DECRQM + in-band resize (DEC 2026/2048)", () => { it("reassembles an in-band resize report split past the flush window without leaking the tail", () => { // The reported bug: resizing rapidly keeps the event loop busy, so the - // StdinBuffer flush timeout (10ms) fires after the `\x1b[48;…` prefix but + // StdinBuffer flush timeout (50ms) fires after the `\x1b[48;…` prefix but // before the terminator. The tail then arrives as bare characters that // leaked into the editor as literal text (e.g. `8;125;1156;1125t`). vi.useFakeTimers();