From aa57f4a61a81fdcf89593c448101347be3e13288 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 11 Jun 2026 01:03:46 +0000 Subject: [PATCH 1/2] fix(tui): handled stdout eio renderer failures Caught asynchronous stdout error events from ProcessTerminal writes and disabled future terminal rendering instead of letting the stream error escape as an uncaught exception. Made cleanup reentry no-op idempotently so fatal shutdown paths do not flood logs with recursive cleanup errors. Fixes #2284 --- packages/tui/CHANGELOG.md | 4 ++ packages/tui/src/terminal.ts | 44 ++++++++++++++++++++-- packages/tui/test/issue-2034-repro.test.ts | 21 +++++++++++ packages/utils/CHANGELOG.md | 4 ++ packages/utils/src/postmortem.ts | 6 +-- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index a80141946..4c702550f 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `ProcessTerminal` treating asynchronous stdout `EIO` errors as uncaught exceptions: stdout `error` events now mark the terminal dead, disable future renders, and keep the active session process alive ([#2284](https://github.com/can1357/oh-my-pi/issues/2284)). + ## [15.11.0] - 2026-06-10 ### Added diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 80c42db4d..3e61a4b8b 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -123,6 +123,27 @@ let activeTerminal: ProcessTerminal | null = null; // Track if a terminal was ever started (for emergency restore logic) let terminalEverStarted = false; +const stdoutErrorHandlers = new Set<(err: Error) => void>(); +let stdoutErrorListenerInstalled = false; + +function onStdoutError(err: Error): void { + for (const handler of stdoutErrorHandlers) handler(err); +} + +function registerStdoutErrorHandler(handler: (err: Error) => void): () => void { + stdoutErrorHandlers.add(handler); + if (!stdoutErrorListenerInstalled) { + process.stdout.on("error", onStdoutError); + stdoutErrorListenerInstalled = true; + } + return () => { + stdoutErrorHandlers.delete(handler); + if (stdoutErrorHandlers.size > 0 || !stdoutErrorListenerInstalled) return; + process.stdout.removeListener("error", onStdoutError); + stdoutErrorListenerInstalled = false; + }; +} + const STD_INPUT_HANDLE = -10; const ENABLE_VIRTUAL_TERMINAL_INPUT = 0x0200; /** UTF-8 codepage id for SetConsoleCP/SetConsoleOutputCP. */ @@ -344,6 +365,11 @@ export class ProcessTerminal implements Terminal { #stdinDataHandler?: (data: string) => void; #dead = false; #writeLogPath = $env.PI_TUI_WRITE_LOG || ""; + #stdoutErrorCleanup?: () => void; + #stdoutErrorHandler = (err: Error) => { + this.#markTerminalWriteFailed(err); + }; + #windowsVTInputRestore?: () => void; #appearanceCallbacks: Array<(appearance: TerminalAppearance) => void> = []; #appearance: TerminalAppearance | undefined; @@ -1182,6 +1208,18 @@ export class ProcessTerminal implements Terminal { if (process.stdin.setRawMode) { process.stdin.setRawMode(this.#wasRaw); } + this.#stdoutErrorCleanup?.(); + this.#stdoutErrorCleanup = undefined; + } + + #ensureStdoutErrorHandler(): void { + this.#stdoutErrorCleanup ??= registerStdoutErrorHandler(this.#stdoutErrorHandler); + } + + #markTerminalWriteFailed(err: unknown): void { + if (this.#dead) return; + this.#dead = true; + logger.warn("terminal write failed; disabling terminal rendering", { err }); } write(data: string): void { @@ -1200,6 +1238,7 @@ export class ProcessTerminal implements Terminal { // Skip control sequences when stdout isn't a TTY (piped output, tests, log // files). They serve no purpose there and would surface as visible noise. if (!process.stdout.isTTY) return; + this.#ensureStdoutErrorHandler(); // A console-sharing child process may have flipped the console codepage // away from UTF-8; repair it before any bytes hit WriteFile so no frame // is ever translated through an OEM codepage. See ensureWindowsConsoleUtf8. @@ -1219,15 +1258,14 @@ export class ProcessTerminal implements Terminal { // threshold. See #2034 and #2095. if (isConPTYHosted() && Buffer.byteLength(data, "utf8") > MAX_CONPTY_WRITE_CHUNK_BYTES) { for (const chunk of chunkForConPTY(data, MAX_CONPTY_WRITE_CHUNK_BYTES)) { + if (this.#dead) break; process.stdout.write(chunk); } } else { process.stdout.write(data); } } catch (err) { - // Any write failure means terminal is dead - no recovery possible - this.#dead = true; - logger.warn("terminal is dead - no recovery possible", { error: err, data }); + this.#markTerminalWriteFailed(err); } } diff --git a/packages/tui/test/issue-2034-repro.test.ts b/packages/tui/test/issue-2034-repro.test.ts index 4e35e1bd2..3f7dfc29d 100644 --- a/packages/tui/test/issue-2034-repro.test.ts +++ b/packages/tui/test/issue-2034-repro.test.ts @@ -273,5 +273,26 @@ describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { } expect(conptyChunks.join("")).toBe(payload); }); + + it("marks the terminal dead when stdout emits EIO after a write (#2284)", () => { + const writes = captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const err = Object.assign(new Error("EIO: i/o error, write"), { + code: "EIO", + fd: 5, + syscall: "write", + errno: -5, + }); + + try { + terminal.write("first frame"); + process.stdout.emit("error", err); + terminal.write("second frame"); + + expect(writes).toEqual(["first frame"]); + } finally { + terminal.stop(); + } + }); }); }); diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 927996369..0ac8d8f24 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed cleanup reentry noise during fatal shutdown: recursive cleanup requests now no-op idempotently instead of logging repeated `Cleanup invoked recursively` errors ([#2284](https://github.com/can1357/oh-my-pi/issues/2284)). + ## [15.11.0] - 2026-06-10 ### Added diff --git a/packages/utils/src/postmortem.ts b/packages/utils/src/postmortem.ts index 1e66b8875..501e67071 100644 --- a/packages/utils/src/postmortem.ts +++ b/packages/utils/src/postmortem.ts @@ -38,7 +38,6 @@ function runCleanup(reason: Reason): Promise { cleanupStage = "running"; break; case "running": - logger.error("Cleanup invoked recursively", { stack: new Error().stack }); return Promise.resolve(); case "complete": return Promise.resolve(); @@ -150,8 +149,9 @@ export function register(id: string, callback: (reason: Reason) => void | Promis }; if (cleanupStage !== "idle") { - // If cleanup is already running/completed, warn and run on microtask. - logger.warn("Cleanup invoked recursively", { id }); + // Cleanup is already in progress or complete; run late registrations once + // without re-entering the global cleanup pass. + logger.debug("Cleanup already started; running late callback once", { id }); try { callback(Reason.MANUAL); } catch (e) { From 3c1d5a88e644b53a710c4d2d05874a9459eb9bc1 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 11 Jun 2026 01:08:43 +0000 Subject: [PATCH 2/2] fix(tui): kept stdout error guard installed Kept the process-level stdout error listener installed after ProcessTerminal stop so delayed write failures cannot become unhandled stream errors. Added regression coverage for EIO events delivered after terminal shutdown. Fixes #2284 --- packages/tui/src/terminal.ts | 3 --- packages/tui/test/issue-2034-repro.test.ts | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/tui/src/terminal.ts b/packages/tui/src/terminal.ts index 3e61a4b8b..fefc54219 100644 --- a/packages/tui/src/terminal.ts +++ b/packages/tui/src/terminal.ts @@ -138,9 +138,6 @@ function registerStdoutErrorHandler(handler: (err: Error) => void): () => void { } return () => { stdoutErrorHandlers.delete(handler); - if (stdoutErrorHandlers.size > 0 || !stdoutErrorListenerInstalled) return; - process.stdout.removeListener("error", onStdoutError); - stdoutErrorListenerInstalled = false; }; } diff --git a/packages/tui/test/issue-2034-repro.test.ts b/packages/tui/test/issue-2034-repro.test.ts index 3f7dfc29d..d82f3ef5f 100644 --- a/packages/tui/test/issue-2034-repro.test.ts +++ b/packages/tui/test/issue-2034-repro.test.ts @@ -294,5 +294,21 @@ describe("issue #2034: chunk large terminal writes on Windows ConPTY", () => { terminal.stop(); } }); + + it("keeps stdout error events handled after stop for delayed write failures (#2284)", () => { + captureStdoutWrites(); + const terminal = new ProcessTerminal(); + const err = Object.assign(new Error("EIO: i/o error, write"), { + code: "EIO", + fd: 5, + syscall: "write", + errno: -5, + }); + + terminal.write("restore frame"); + terminal.stop(); + + expect(() => process.stdout.emit("error", err)).not.toThrow(); + }); }); });