From e3a62a415f6b819afd5b19c15baa274771b3a551 Mon Sep 17 00:00:00 2001 From: Kingsword Date: Tue, 1 Sep 2026 13:32:23 +0800 Subject: [PATCH] fix(tui): handle terminal stream failures safely - stop notification writes after EIO/EPIPE failures - clean up stream error listeners after terminal teardown - preserve default unhandled rejection behavior --- packages/zcode-tui/src/index.ts | 37 +++++++++- packages/zcode-tui/src/notifications.ts | 15 ++++ packages/zcode-tui/src/stream-error-guard.ts | 19 ++++++ test/notifications.test.ts | 72 ++++++++++++++++++++ 4 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 packages/zcode-tui/src/stream-error-guard.ts diff --git a/packages/zcode-tui/src/index.ts b/packages/zcode-tui/src/index.ts index 8e03ab1..df427b7 100644 --- a/packages/zcode-tui/src/index.ts +++ b/packages/zcode-tui/src/index.ts @@ -134,6 +134,7 @@ import { type NotificationSettings, type TurnNotificationKind } from "./notifications.ts"; +import { watchStreamErrors } from "./stream-error-guard.ts"; import { readTuiMode, resolveTuiMode, @@ -671,6 +672,7 @@ class ZCodeTui { private backgroundHandoffInterruptInFlight = false; private updateCheckAbortController?: AbortController; private loginRequired: boolean; + private removeStreamErrorGuards?: () => void; constructor(private readonly options: TuiOptions) { this.animateTurnTimer = turnTimerAnimationEnabled(); @@ -794,6 +796,7 @@ class ZCodeTui { const onSigint = () => this.handleSignal("SIGINT"); const onSigterm = () => this.handleSignal("SIGTERM"); const onSighup = () => this.handleSignal("SIGHUP"); + this.installStreamErrorGuards(); process.once("SIGINT", onSigint); process.once("SIGTERM", onSigterm); if (process.platform !== "win32") process.once("SIGHUP", onSighup); @@ -856,9 +859,17 @@ class ZCodeTui { process.off("SIGINT", onSigint); process.off("SIGTERM", onSigterm); if (process.platform !== "win32") process.off("SIGHUP", onSighup); - if (startAttempted && !this.stopped) { - this.stop(); - await this.done; + try { + if (startAttempted && !this.stopped) { + this.stop(); + await this.done; + } + } finally { + // Leave the guards installed through the I/O turn containing the final + // terminal restore writes, which can report stream errors asynchronously. + await new Promise((resolve) => setImmediate(resolve)); + this.removeStreamErrorGuards?.(); + this.removeStreamErrorGuards = undefined; } } } @@ -868,6 +879,26 @@ class ZCodeTui { if (!this.stopped) this.stop(); } + /** + * process.stdout/stderr stream errors (EIO when the pty is gone, EPIPE when + * the reader disconnected) are emitted asynchronously, so the synchronous + * try/catch around every terminal write cannot observe them. Without a + * listener Node rethrows them and kills the process — a doomed notification + * write after the pane closes would crash the whole TUI. Absorb stream + * death here, stop notification writes, and let the run loop unwind through + * the normal stop path. + */ + private installStreamErrorGuards(): void { + if (this.removeStreamErrorGuards) return; + this.removeStreamErrorGuards = watchStreamErrors([process.stdout, process.stderr], () => { + this.notifications.markTerminalUnavailable(); + if (!this.stopped) { + process.exitCode = 1; + this.stop(); + } + }); + } + private async resolveTerminalColorScheme(): Promise { if (!this.colorsEnabled || this.themePreference !== "auto") return; try { diff --git a/packages/zcode-tui/src/notifications.ts b/packages/zcode-tui/src/notifications.ts index 0ce888d..cde71aa 100644 --- a/packages/zcode-tui/src/notifications.ts +++ b/packages/zcode-tui/src/notifications.ts @@ -277,6 +277,7 @@ export class TurnNotifier { private active = false; private focusReporting = false; private terminalFocus: TerminalFocusState = "unknown"; + private terminalUnavailable = false; constructor(private readonly options: TurnNotifierOptions) { this.env = options.env ?? process.env; @@ -359,11 +360,25 @@ export class TurnNotifier { } } + /** + * Marks the terminal as unusable after an async stream failure (EIO/EPIPE). + * writeTerminal's synchronous try/catch cannot observe those; the stream + * error handler calls this so later writes are dropped instead of racing + * another doomed write against the dying stream. + */ + markTerminalUnavailable(): void { + this.terminalUnavailable = true; + this.focusReporting = false; + this.terminalFocus = "unknown"; + } + private writeTerminal(data: string): boolean { + if (this.terminalUnavailable) return false; try { this.options.writeTerminal(data); return true; } catch { + this.terminalUnavailable = true; return false; } } diff --git a/packages/zcode-tui/src/stream-error-guard.ts b/packages/zcode-tui/src/stream-error-guard.ts new file mode 100644 index 0000000..1ad7a9a --- /dev/null +++ b/packages/zcode-tui/src/stream-error-guard.ts @@ -0,0 +1,19 @@ +export interface StreamErrorSource { + on(event: "error", listener: (error: Error) => void): this; + off(event: "error", listener: (error: Error) => void): this; +} + +export function watchStreamErrors( + sources: readonly StreamErrorSource[], + onError: (error: Error) => void +): () => void { + const uniqueSources = [...new Set(sources)]; + for (const source of uniqueSources) source.on("error", onError); + + let active = true; + return () => { + if (!active) return; + active = false; + for (const source of uniqueSources) source.off("error", onError); + }; +} diff --git a/test/notifications.test.ts b/test/notifications.test.ts index 8372c58..0c65f4e 100644 --- a/test/notifications.test.ts +++ b/test/notifications.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { EventEmitter } from "node:events"; import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -20,6 +21,7 @@ import { writeNotificationSettings, type NativeNotificationSender } from "../packages/zcode-tui/src/notifications.ts"; +import { watchStreamErrors } from "../packages/zcode-tui/src/stream-error-guard.ts"; const temporaryDirectories: string[] = []; @@ -253,6 +255,76 @@ describe("TUI turn notifications", () => { expect(writes).toEqual([]); }); + test("treats synchronous terminal write failures as sticky", async () => { + const writes: string[] = []; + let failing = false; + const notifier = new TurnNotifier({ + settings: { method: "auto", condition: "always" }, + writeTerminal: (data) => { + if (failing) { + const error = new Error("write EIO") as NodeJS.ErrnoException; + error.code = "EIO"; + throw error; + } + writes.push(data); + } + }); + + notifier.start(); + expect(await notifier.notify("completed", "Done")).toBe(true); + expect(writes).toEqual(["\x07"]); + + failing = true; + expect(await notifier.notify("completed", "Done")).toBe(false); + failing = false; + // The failure is sticky: writes stay suppressed even after the underlying + // stream would accept data again. + expect(await notifier.notify("completed", "Done")).toBe(false); + expect(writes).toEqual(["\x07"]); + + notifier.stop(); + expect(writes).toEqual(["\x07"]); + }); + + test("stops terminal writes after an asynchronous stream error", async () => { + const writes: string[] = []; + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const ignoreError = () => {}; + stdout.on("error", ignoreError); + stderr.on("error", ignoreError); + const notifier = new TurnNotifier({ + settings: { method: "auto", condition: "always" }, + writeTerminal: (data) => writes.push(data) + }); + let handledErrors = 0; + const dispose = watchStreamErrors([stdout, stderr], () => { + handledErrors += 1; + notifier.markTerminalUnavailable(); + }); + + notifier.start(); + expect(await notifier.notify("completed", "Done")).toBe(true); + await new Promise((resolve) => { + setImmediate(() => { + stdout.emit("error", Object.assign(new Error("write EIO"), { code: "EIO" })); + resolve(); + }); + }); + + expect(handledErrors).toBe(1); + expect(await notifier.notify("completed", "Done")).toBe(false); + notifier.stop(); + expect(writes).toEqual(["\x07"]); + + dispose(); + dispose(); + expect(stdout.listenerCount("error")).toBe(1); + expect(stderr.listenerCount("error")).toBe(1); + stderr.emit("error", new Error("detached")); + expect(handledErrors).toBe(1); + }); + test("uses BEL for automatic SSH notifications and supports disabling notifications", async () => { let commandRuns = 0; const writes: string[] = [];