From 98e150d12db81879ad29ac953827f1916973596b Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:10:56 +0000 Subject: [PATCH] fix(mcp): exit the CLI cleanly on a broken-pipe stdout/stderr error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit process.stdout/stderr had no "error" listener, so a broken pipe (EPIPE) — the downstream reader in `... | head` closing after a few bytes while the CLI is still writing a large payload — surfaced as an uncaught write EPIPE and crashed the whole CLI with a stack trace. flushStdio() only drained via the "drain" event, which never fires once the stream has errored. Attach an "error" listener to both streams (gated on the launched-CLI entrypoint) that exits cleanly: success on EPIPE (a normal end to a piped command), a conventional failure code otherwise. Normal, fully-drained output is unchanged. Closes #8691 --- packages/loopover-mcp/bin/loopover-mcp.ts | 18 +++++++ test/unit/mcp-cli-broken-pipe.test.ts | 57 +++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/unit/mcp-cli-broken-pipe.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index cebff0b461..b016c15160 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -1682,6 +1682,24 @@ function flushStdio(): Promise { return Promise.all([drained(process.stdout), drained(process.stderr)]).then(() => undefined); } +/* v8 ignore start -- an "error" on stdout/stderr only occurs in the real launched process writing to an OS + pipe: a reader that closes early (`... | head`) breaks the pipe, and the next unguarded write would surface + as an uncaught `Error: write EPIPE` and crash the whole CLI with a stack trace instead of exiting cleanly. + flushStdio() above only drains via "drain", which never fires once the stream has errored, so the listener + must be attached separately. The in-process unit importer never spawns a subprocess and never binds these, + so this stays unhit there; mcp-cli-broken-pipe.test.ts drives the real subprocess path out-of-band. */ +if (runAsCliEntrypoint) { + const onStreamError = (error: NodeJS.ErrnoException) => { + // A broken pipe (EPIPE) is the downstream reader closing early — a normal, expected end to a piped + // command, not a failure — so exit with success. Any other stream error is unexpected but still must not + // crash as an uncaught exception, so exit with a conventional failure code instead. + process.exit(error.code === "EPIPE" ? 0 : 1); + }; + process.stdout.on("error", onStreamError); + process.stderr.on("error", onStreamError); +} +/* v8 ignore stop */ + /* v8 ignore next 11 -- the CLI dispatch runs only in the launched process (runAsCliEntrypoint); an in-process unit importer keeps it false and drives runCli/maintainCli directly instead (mcp-cli-plan-issues.test.ts). */ if (runAsCliEntrypoint && cliArgs[0] !== "--stdio") { diff --git a/test/unit/mcp-cli-broken-pipe.test.ts b/test/unit/mcp-cli-broken-pipe.test.ts new file mode 100644 index 0000000000..9253285592 --- /dev/null +++ b/test/unit/mcp-cli-broken-pipe.test.ts @@ -0,0 +1,57 @@ +import { spawn } from "node:child_process"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { bin, run } from "./support/mcp-cli-harness"; + +// #8691: bin/loopover-mcp.ts attaches no "error" listener to process.stdout/stderr, so a broken pipe +// (EPIPE — the downstream reader in `... | head` closes after a few bytes, while the CLI is still writing a +// large payload) surfaced as an uncaught `Error: write EPIPE` and crashed the whole CLI with a stack trace +// instead of exiting cleanly. `tools` is a deterministic, offline, large (~27KB, many separate writes) +// payload — piping it into a reader that grabs a handful of bytes and exits reliably reproduces the race. +// These tests spawn the real compiled dist CLI (like the rest of the mcp-cli-*.test.ts harness), so they +// require `npm run build:mcp` to have run first — CI's "Build MCP" step and local `npm run test:ci` do that. + +const configEnv = () => ({ ...process.env, LOOPOVER_CONFIG_DIR: mkdtempSync(join(tmpdir(), "loopover-cli-brokenpipe-")) }); + +/** Spawn `loopover-mcp tools` with its stdout wired straight into `head`'s stdin (a real OS pipe), so when + * head reads a few bytes and exits the CLI's next write hits a closed pipe — exactly the `| head` scenario. + * Resolves the CLI's own exit code/signal plus its stderr (where an uncaught-exception crash would print). */ +function runPipedIntoEarlyClosingReader(): Promise<{ code: number | null; signal: NodeJS.Signals | null; stderr: string }> { + return new Promise((resolve, reject) => { + const reader = spawn("head", ["-c", "5"], { stdio: ["pipe", "ignore", "ignore"] }); + // The parent-side handle to head's stdin can emit its own EPIPE once head exits; it is irrelevant to what + // the CLI subprocess sees, so swallow it rather than let it reject the test's own process. + reader.stdin.on("error", () => {}); + const cli = spawn(process.execPath, [bin, "tools"], { stdio: ["ignore", reader.stdin, "pipe"], env: configEnv() }); + let stderr = ""; + cli.stderr.on("data", (chunk) => (stderr += chunk)); + cli.on("error", reject); + cli.on("exit", (code, signal) => { + if (!reader.killed) reader.kill(); + resolve({ code, signal, stderr }); + }); + }); +} + +describe("loopover-mcp CLI survives a broken pipe (#8691)", () => { + it("exits cleanly — not via an uncaught EPIPE crash — when its large output is piped into a reader that closes early", async () => { + const { code, signal, stderr } = await runPipedIntoEarlyClosingReader(); + // Clean exit: a documented exit code, not a signal-kill, and no uncaught-exception stack trace on stderr. + expect(signal).toBeNull(); + expect(code).toBe(0); + expect(stderr).not.toMatch(/EPIPE/); + expect(stderr).not.toMatch(/Unhandled 'error'/); + expect(stderr).not.toMatch(/at afterWriteDispatched/); + }, 15_000); + + it("leaves normal (fully-drained) command output unaffected", () => { + // The same command read to completion still emits its full, unchanged output and exits successfully — the + // new error listener must not perturb the ordinary, non-broken-pipe path. + const output = run(["tools"]); + expect(output).toMatch(/Discovery & planning/); + expect(output).toContain("loopover_local_status"); + expect(output.length).toBeGreaterThan(10_000); + }); +});