diff --git a/apps/desktop/main/index.ts b/apps/desktop/main/index.ts index b734fbee..016b0817 100644 --- a/apps/desktop/main/index.ts +++ b/apps/desktop/main/index.ts @@ -123,8 +123,14 @@ import { } from "./services/secure-storage-policy"; import type { ServiceSupervisor } from "./services/supervisor"; import { TransportBridge } from "./transport-bridge"; +import { installStdioResilience } from "./stdio-resilience"; import { createMainWindow } from "./window"; +// FIRST, before anything can log. The CLI spawns this process with +// `stdio: "inherit"`, so the window outlives the terminal that launched it and +// a closed terminal turns the next console.* into an uncaught EPIPE. +installStdioResilience(); + applyBrandIdentity(app, { platform: process.platform }); const bootTimingStartedAt = process.hrtime.bigint(); diff --git a/apps/desktop/main/stdio-resilience.test.ts b/apps/desktop/main/stdio-resilience.test.ts new file mode 100644 index 00000000..745c4a61 --- /dev/null +++ b/apps/desktop/main/stdio-resilience.test.ts @@ -0,0 +1,74 @@ +import { PassThrough } from "node:stream"; +import { describe, expect, it } from "vitest"; + +import { installStdioResilience } from "./stdio-resilience"; + +/** + * The bug these pin: the app is spawned `stdio: "inherit"` from the `copilot` + * CLI, so closing the launching terminal leaves the GUI writing to a dead + * pipe, and an unguarded `console.error` then crashes the main process. + * + * Note what makes this testable at all: the assertion is about whether the + * stream THROWS, so each case emits a real `error` event rather than checking + * that a listener was registered. "A listener exists" is exactly the shape of + * assertion that passes over a listener which re-throws everything. + */ +describe("installStdioResilience", () => { + const epipe = (): Error => + Object.assign(new Error("write EPIPE"), { code: "EPIPE" }); + + it("swallows EPIPE so a dead pipe cannot crash the process", () => { + const stream = new PassThrough(); + const dispose = installStdioResilience([stream]); + + expect(() => stream.emit("error", epipe())).not.toThrow(); + + dispose(); + }); + + it("re-throws anything that is not EPIPE", () => { + const stream = new PassThrough(); + const dispose = installStdioResilience([stream]); + const enospc = Object.assign(new Error("no space left"), { + code: "ENOSPC", + }); + + // A full disk is a real failure. Swallowing every stream error to fix + // EPIPE is how the next one goes unnoticed. + expect(() => stream.emit("error", enospc)).toThrow("no space left"); + + dispose(); + }); + + it("guards every stream it is given, not just the first", () => { + const out = new PassThrough(); + const err = new PassThrough(); + const dispose = installStdioResilience([out, err]); + + expect(() => out.emit("error", epipe())).not.toThrow(); + expect(() => err.emit("error", epipe())).not.toThrow(); + + dispose(); + }); + + it("removes only its own listeners when disposed", () => { + const stream = new PassThrough(); + const before = stream.listenerCount("error"); + const dispose = installStdioResilience([stream]); + expect(stream.listenerCount("error")).toBe(before + 1); + + dispose(); + + expect(stream.listenerCount("error")).toBe(before); + }); + + it("leaves the stream unguarded again after disposal", () => { + const stream = new PassThrough(); + installStdioResilience([stream])(); + + // With no listener, Node's EventEmitter throws an emitted 'error'. This is + // the control case: it proves the passing case above is the guard working + // and not PassThrough being tolerant on its own. + expect(() => stream.emit("error", epipe())).toThrow("write EPIPE"); + }); +}); diff --git a/apps/desktop/main/stdio-resilience.ts b/apps/desktop/main/stdio-resilience.ts new file mode 100644 index 00000000..fa13f7a0 --- /dev/null +++ b/apps/desktop/main/stdio-resilience.ts @@ -0,0 +1,68 @@ +/** + * Keep a broken stdout/stderr pipe from killing the app. + * + * WHY THIS EXISTS, because it looks like defensive noise until you know the + * launch shape. `tools/cli/bin/copilot.mjs` spawns the app with + * `stdio: "inherit"`, so the GUI process writes straight to the terminal that + * ran `copilot`. That terminal is not the app's lifetime — the window outlives + * it. Close the terminal (or end the ssh session, or quit the shell) and the + * app keeps running with stdout pointing at a pipe whose reader is gone. + * + * The next `console.error` in the main process then raises `EPIPE`, and with no + * listener on the stream Node turns it into an uncaught exception that takes + * down the whole app. Observed exactly that way: + * + * Uncaught Exception: + * Error: write EPIPE + * at afterWriteDispatched (node:internal/stream_base_commons:159:15) + * ... + * at console.error (node:internal/console/constructor:444:26) + * at replyWithError (node:electron/js2c/browser_init) + * + * Read that trace bottom-up: the main process was REPORTING an IPC error, and + * the reporting killed it. A failure in the error path is the worst place to + * have one, because it converts a handled problem into a crash. + * + * There are ~30 `console.*` call sites in `main/`, so guarding them one by one + * is not the fix; attaching a listener to the stream is. A stream with an + * `error` listener no longer throws, and EPIPE genuinely has nothing to do + * about it — the reader is gone and there is nowhere to log the fact that + * there is nowhere to log. Anything that is NOT EPIPE is re-thrown, because a + * disk-full or permission error on stderr is a real failure and swallowing + * every stream error to fix one of them is how you lose the next one. + */ + +import type { Writable } from "node:stream"; + +/** The one error code that means "the reader went away, and that is fine". */ +const BROKEN_PIPE = "EPIPE"; + +/** + * Attach EPIPE-tolerant error handling to the process's output streams. + * + * Call this as early in main as possible — before any logging, because the + * write it protects can be the first one. Returns a disposer so a test can + * remove the listeners it added rather than leaking them across cases. + */ +export function installStdioResilience( + streams: readonly Writable[] = [process.stdout, process.stderr], +): () => void { + const attached: Array<[Writable, (error: Error) => void]> = []; + + for (const stream of streams) { + const onError = (error: Error): void => { + if ((error as NodeJS.ErrnoException).code === BROKEN_PIPE) { + return; + } + throw error; + }; + stream.on("error", onError); + attached.push([stream, onError]); + } + + return () => { + for (const [stream, onError] of attached) { + stream.off("error", onError); + } + }; +}