From 84c06a030f2ab5501051bbfdd9d1963f59fcd57e Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:41:58 +0800 Subject: [PATCH 1/9] Shut down when stdin closes or hits EOF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A host that dies (SIGKILL) or closes the stdio pipe leaves squiggles and every LSP server it spawned alive indefinitely. SIGINT/SIGTERM were the only shutdown triggers, and the MCP TS SDK never surfaces stdin EOF — StdioServerTransport only wires 'data'/'error' listeners. Watch process.stdin end/close and server.onclose (the SDK reassigns transport.onclose inside connect(), so hooking the transport would clobber its handler), guarded so shutdown runs exactly once; all five triggers share one rejection-safe wrapper. Also suppress EPIPE on stdout/stderr: after host death those pipes are dead, and the resulting 'error' event used to kill the process mid-shutdown, before the LSP children could be reaped. Fixes #2 --- src/index.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index b454fae..1f900d6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,6 +20,12 @@ const require = createRequire(import.meta.url); const { version } = require("../package.json"); async function main(): Promise { + // Once the host dies, writes to stdout/stderr emit EPIPE as an unhandled + // "error" event that would kill the process mid-shutdown, orphaning the + // LSP children. Suppressing is standard for stdio servers. + process.stdout.on("error", () => {}); + process.stderr.on("error", () => {}); + if (process.argv[2] === "init") { const result = runInit(process.argv.slice(3), process.cwd()); process.stdout.write(result.message + "\n"); @@ -83,15 +89,29 @@ async function main(): Promise { await server.connect(transport); log("MCP server connected via stdio"); + let shuttingDown = false; const shutdown = async () => { + if (shuttingDown) return; + shuttingDown = true; log("Shutting down..."); if (manager) await manager.shutdownAll(); await server.close(); process.exit(0); }; + const requestShutdown = () => { + shutdown().catch((err) => { + logError("Shutdown failed", err); + process.exit(1); + }); + }; - process.on("SIGINT", shutdown); - process.on("SIGTERM", shutdown); + process.on("SIGINT", requestShutdown); + process.on("SIGTERM", requestShutdown); + // The SDK assigns transport.onclose inside connect(), so hook the server + // instead — stdin EOF/close is the only signal guaranteed on host death. + server.onclose = requestShutdown; + process.stdin.on("end", requestShutdown); + process.stdin.on("close", requestShutdown); } main().catch((err) => { From 28a6a1036e508bbd9a32ff2ebbf0a1c553a610c8 Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:44:19 +0800 Subject: [PATCH 2/9] Add integration test for issue #2 shutdown wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spawn squiggles with a dummy LSP config, trigger lazy LSP spawn via a tools/call, close the host stdin, and assert both processes exit. The dummy fixture answers initialize only (it never answers shutdown), so the only path to reaping it is squiggles's kill() — exactly the path that used to be skipped when EPIPE killed the parent mid-shutdown. Verified the test fails on the pre-fix build (squiggles hangs past 15 s, LSP orphaned) and passes on the fixed build. --- .../__shutdown_fixtures__/dummy-lsp.mjs | 35 ++++++ src/__tests__/index.shutdown.test.ts | 114 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs create mode 100644 src/__tests__/index.shutdown.test.ts diff --git a/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs b/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs new file mode 100644 index 0000000..3ff8e66 --- /dev/null +++ b/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs @@ -0,0 +1,35 @@ +// Dummy LSP for the shutdown integration test. +// Writes its PID to argv[2] so the test can verify the process was reaped +// after squiggles exits. Answers initialize only, then hangs on everything +// (including shutdown) so the ONLY way to exit is squiggles's kill() — the +// path that used to be skipped when the EPIPE bug killed the parent mid-shutdown. +import process from "node:process"; +import { writeFileSync } from "node:fs"; + +writeFileSync(process.argv[2], String(process.pid)); + +function send(msg) { + const json = JSON.stringify(msg); + process.stdout.write(`Content-Length: ${Buffer.byteLength(json)}\r\n\r\n${json}`); +} + +let buf = Buffer.alloc(0); +process.stdin.on("data", (chunk) => { + buf = Buffer.concat([buf, chunk]); + for (;;) { + const sep = buf.indexOf("\r\n\r\n"); + if (sep === -1) break; + const header = buf.slice(0, sep).toString(); + const m = /Content-Length: (\d+)/i.exec(header); + if (!m) { buf = buf.slice(sep + 4); continue; } + const len = parseInt(m[1], 10); + if (buf.length < sep + 4 + len) break; + const body = buf.slice(sep + 4, sep + 4 + len).toString(); + buf = buf.slice(sep + 4 + len); + const msg = JSON.parse(body); + if (msg.method === "initialize") { + send({ jsonrpc: "2.0", id: msg.id, result: { capabilities: { textDocumentSync: 1 } } }); + } + } +}); +setInterval(() => {}, 1000); \ No newline at end of file diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts new file mode 100644 index 0000000..236dd85 --- /dev/null +++ b/src/__tests__/index.shutdown.test.ts @@ -0,0 +1,114 @@ +// Integration test for issue #2: closing the host stdin must shut down +// squiggles AND the LSP server it spawned (no orphan processes). +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { spawn, type ChildProcess } from "node:child_process"; +import * as fs from "node:fs"; +import * as path from "node:path"; + +const DIST_ENTRY = path.resolve(__dirname, "..", "..", "dist", "index.js"); +const FIXTURES = path.join(__dirname, "__shutdown_fixtures__"); +const DUMMY_LSP = path.join(FIXTURES, "dummy-lsp.mjs"); + +const TEST_ROOT = path.join(FIXTURES, "project"); +const PROJECT_DIR = TEST_ROOT; + +function alive(pid: number): boolean { + try { process.kill(pid, 0); return true; } catch { return false; } +} + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +beforeEach(() => { + fs.mkdirSync(PROJECT_DIR, { recursive: true }); + fs.writeFileSync( + path.join(PROJECT_DIR, "squiggles.yaml"), + [ + "servers:", + " dummy:", + ` command: ["node", "${DUMMY_LSP}", "${path.join(PROJECT_DIR, "dummy-lsp.pid")}"]`, + ' filePatterns: ["*.txt"]', + "", + ].join("\n"), + ); + fs.writeFileSync(path.join(PROJECT_DIR, "sample.txt"), "hello\n"); + fs.rmSync(path.join(PROJECT_DIR, "dummy-lsp.pid"), { force: true }); +}); + +afterEach(() => { + fs.rmSync(TEST_ROOT, { recursive: true, force: true }); +}); + +describe("squiggles shutdown on stdin close (issue #2)", () => { + it("exits and reaps its LSP child when the host closes stdin", async () => { + if (!fs.existsSync(DIST_ENTRY)) { + throw new Error( + `dist/index.js not found at ${DIST_ENTRY} — run \`npm run build\` before this test`, + ); + } + + const squiggles: ChildProcess = spawn("node", [DIST_ENTRY, PROJECT_DIR], { + stdio: ["pipe", "pipe", "pipe"], + }); + + const pidFile = path.join(PROJECT_DIR, "dummy-lsp.pid"); + const responses = new Map(); + let outBuf = ""; + squiggles.stderr?.on("data", () => {}); + squiggles.stdout?.on("data", (chunk) => { + outBuf += chunk.toString(); + let i; + while ((i = outBuf.indexOf("\n")) !== -1) { + const line = outBuf.slice(0, i).trim(); + outBuf = outBuf.slice(i + 1); + if (!line) continue; + try { + const msg = JSON.parse(line); + if (typeof msg.id === "number") responses.set(msg.id, msg); + } catch { /* ignore non-JSON noise */ } + } + }); + + const send = (obj: unknown) => squiggles.stdin!.write(JSON.stringify(obj) + "\n"); + const waitFor = async (cond: () => boolean, ms: number, what: string) => { + const start = Date.now(); + while (Date.now() - start < ms) { + if (cond()) return; + await sleep(50); + } + throw new Error(`timeout waiting for ${what}`); + }; + + try { + // MCP handshake. + send({ jsonrpc: "2.0", id: 1, method: "initialize", params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "test", version: "0" } } }); + await waitFor(() => responses.has(1), 5000, "initialize response"); + send({ jsonrpc: "2.0", method: "notifications/initialized" }); + + // Trigger lazy LSP spawn via a tool call. We don't await a response — + // closing stdin will close the transport first. + send({ jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "document_symbols", arguments: { file: "sample.txt" } } }); + + await waitFor(() => fs.existsSync(pidFile), 10000, "dummy LSP spawn"); + const lspPid = parseInt(fs.readFileSync(pidFile, "utf-8"), 10); + expect(Number.isFinite(lspPid)).toBe(true); + expect(alive(lspPid)).toBe(true); + + // Host closes its stdin pipe — the SIGKILL/pipe-close signal issue #2. + squiggles.stdin!.end(); + + // squiggles must exit. + const exit = await new Promise((resolve) => { + squiggles.once("exit", (code) => resolve(code)); + setTimeout(() => resolve(null), 15000); + }); + expect(exit).not.toBeNull(); + expect(exit).toBe(0); + + // The dummy LSP child must have been reaped too. + await sleep(300); + expect(alive(lspPid)).toBe(false); + } finally { + if (squiggles.exitCode === null) squiggles.kill("SIGKILL"); + } + }, 30000); +}); \ No newline at end of file From e0268adaf8551ab272a760325bc9ed4b11d6132d Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:48:12 +0800 Subject: [PATCH 3/9] Make the lightweight shutdown test discriminating: assert the shutdown log line appears, not just a clean exit. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous form of the test passed on the pre-fix build too, because on a project with no LSPs Node's event loop drains naturally once stdin is gone and the process exits 0 by accident — never entering our shutdown path. Asserting that stderrBuf contains 'Shutting down...' catches the actual regression: only the fixed build takes the explicit shutdown path. --- src/__tests__/index.shutdown.test.ts | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts index 236dd85..c00c54f 100644 --- a/src/__tests__/index.shutdown.test.ts +++ b/src/__tests__/index.shutdown.test.ts @@ -111,4 +111,71 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { if (squiggles.exitCode === null) squiggles.kill("SIGKILL"); } }, 30000); + + it("exits when stdin closes before any MCP traffic", async () => { + if (!fs.existsSync(DIST_ENTRY)) { + throw new Error( + `dist/index.js not found at ${DIST_ENTRY} — run \`npm run build\` before this test`, + ); + } + + // No-LSP project so we don't accidentally spawn a child to clean up. + const noLspDir = path.join(FIXTURES, "project-nolsp"); + fs.mkdirSync(noLspDir, { recursive: true }); + + const child = spawn(process.execPath, [DIST_ENTRY, noLspDir], { + stdio: ["pipe", "pipe", "pipe"], + }); + // Drain output so the child never blocks on a full pipe. + child.stdout?.on("data", () => {}); + let stderrBuf = ""; + child.stderr?.on("data", (chunk) => { + stderrBuf += chunk.toString(); + }); + + try { + // Wait until squiggles is past server.connect() and its stdin + // listeners are wired — closes a tiny race where EOF could land + // before the listeners are registered. + await new Promise((resolve, reject) => { + const deadline = setTimeout( + () => reject(new Error("child never reached 'MCP server connected'")), + 5000, + ); + const tick = setInterval(() => { + if (stderrBuf.includes("MCP server connected via stdio")) { + clearTimeout(deadline); + clearInterval(tick); + resolve(); + } + }, 20); + }); + + child.stdin!.end(); + + // Assert the process actually went through the shutdown path: the + // base build accidentally exits cleanly on a no-LSP project because + // Node's event loop drains once every fd is idle — but it never logs + // "Shutting down..." because no shutdown handler exists. Only the + // fixed build takes the explicit path. + const result = await new Promise<{ code: number | string | null; loggedShutdown: boolean }>( + (resolve) => { + child.once("exit", (code, signal) => { + resolve({ + code: code ?? signal ?? null, + loggedShutdown: stderrBuf.includes("Shutting down..."), + }); + }); + setTimeout(() => resolve({ code: "timeout", loggedShutdown: false }), 5000); + }, + ); + + expect(result.code).not.toBe("timeout"); + expect(result.code).toBe(0); + expect(result.loggedShutdown).toBe(true); + } finally { + if (child.exitCode === null) child.kill("SIGKILL"); + fs.rmSync(noLspDir, { recursive: true, force: true }); + } + }, 15000); }); \ No newline at end of file From 6cb0dd3118aa34dd640d80163f200796656ff2ef Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:56:24 +0800 Subject: [PATCH 4/9] Clean up timers and unify process.execPath in shutdown tests - raceWithTimeout helper: exit-listener timeouts are always cleared when the child wins the race (previously the 15s/5s timers outlived the test and fired into a finished run) - wait-for-connect polling: the interval is cleared on the timeout path too, not just on success - replace the fixed 300ms reap sleep with a deadline-bounded poll - heavy test's finally now also kills a leftover dummy LSP on failure - spawn and dummy-LSP config use process.execPath instead of "node" --- src/__tests__/index.shutdown.test.ts | 93 ++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 25 deletions(-) diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts index c00c54f..e1726be 100644 --- a/src/__tests__/index.shutdown.test.ts +++ b/src/__tests__/index.shutdown.test.ts @@ -18,6 +18,33 @@ function alive(pid: number): boolean { const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); +// Runs `setup` with a settle callback; whichever fires first — settle or the +// timer — wins, and the timer is always cleared so it can't leak past the +// end of the test. +function raceWithTimeout( + setup: (settle: (value: T) => void) => void, + ms: number, + onTimeout: () => T, +): Promise { + return new Promise((resolve) => { + let settled = false; + const timer = setTimeout(() => { + if (!settled) { + settled = true; + resolve(onTimeout()); + } + }, ms); + const settle = (v: T) => { + if (!settled) { + settled = true; + clearTimeout(timer); + resolve(v); + } + }; + setup(settle); + }); +} + beforeEach(() => { fs.mkdirSync(PROJECT_DIR, { recursive: true }); fs.writeFileSync( @@ -25,7 +52,7 @@ beforeEach(() => { [ "servers:", " dummy:", - ` command: ["node", "${DUMMY_LSP}", "${path.join(PROJECT_DIR, "dummy-lsp.pid")}"]`, + ` command: ["${process.execPath}", "${DUMMY_LSP}", "${path.join(PROJECT_DIR, "dummy-lsp.pid")}"]`, ' filePatterns: ["*.txt"]', "", ].join("\n"), @@ -46,7 +73,7 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { ); } - const squiggles: ChildProcess = spawn("node", [DIST_ENTRY, PROJECT_DIR], { + const squiggles: ChildProcess = spawn(process.execPath, [DIST_ENTRY, PROJECT_DIR], { stdio: ["pipe", "pipe", "pipe"], }); @@ -97,18 +124,30 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { squiggles.stdin!.end(); // squiggles must exit. - const exit = await new Promise((resolve) => { - squiggles.once("exit", (code) => resolve(code)); - setTimeout(() => resolve(null), 15000); - }); + const exit = await raceWithTimeout( + (settle) => squiggles.once("exit", (code) => settle(code)), + 15000, + () => null, + ); expect(exit).not.toBeNull(); expect(exit).toBe(0); - // The dummy LSP child must have been reaped too. - await sleep(300); + // The dummy LSP child must have been reaped too (poll instead of a + // fixed sleep so a slow reap doesn't flake and a fast one doesn't wait). + const reapDeadline = Date.now() + 3000; + while (alive(lspPid) && Date.now() < reapDeadline) { + await sleep(50); + } expect(alive(lspPid)).toBe(false); } finally { if (squiggles.exitCode === null) squiggles.kill("SIGKILL"); + // Best-effort: never leave the dummy LSP behind on a failed run. + if (fs.existsSync(pidFile)) { + const leftover = parseInt(fs.readFileSync(pidFile, "utf-8"), 10); + if (Number.isFinite(leftover) && alive(leftover)) { + try { process.kill(leftover, "SIGKILL"); } catch { /* already gone */ } + } + } } }, 30000); @@ -137,19 +176,22 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { // Wait until squiggles is past server.connect() and its stdin // listeners are wired — closes a tiny race where EOF could land // before the listeners are registered. - await new Promise((resolve, reject) => { - const deadline = setTimeout( - () => reject(new Error("child never reached 'MCP server connected'")), - 5000, - ); - const tick = setInterval(() => { - if (stderrBuf.includes("MCP server connected via stdio")) { - clearTimeout(deadline); - clearInterval(tick); - resolve(); - } - }, 20); - }); + let tick: NodeJS.Timeout | undefined; + const connected = await raceWithTimeout( + (settle) => { + tick = setInterval(() => { + if (stderrBuf.includes("MCP server connected via stdio")) { + settle(true); + } + }, 20); + }, + 5000, + () => false, + ); + clearInterval(tick); + if (!connected) { + throw new Error("child never reached 'MCP server connected'"); + } child.stdin!.end(); @@ -158,16 +200,17 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { // Node's event loop drains once every fd is idle — but it never logs // "Shutting down..." because no shutdown handler exists. Only the // fixed build takes the explicit path. - const result = await new Promise<{ code: number | string | null; loggedShutdown: boolean }>( - (resolve) => { + const result = await raceWithTimeout( + (settle) => { child.once("exit", (code, signal) => { - resolve({ + settle({ code: code ?? signal ?? null, loggedShutdown: stderrBuf.includes("Shutting down..."), }); }); - setTimeout(() => resolve({ code: "timeout", loggedShutdown: false }), 5000); }, + 5000, + () => ({ code: "timeout" as number | string | null, loggedShutdown: false }), ); expect(result.code).not.toBe("timeout"); From a6f70f2362a1c8775a3f81e4373565d57689ae13 Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:05:03 +0800 Subject: [PATCH 5/9] Escape YAML config paths with JSON.stringify in shutdown test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interpolating bare paths into a double-quoted YAML flow scalar breaks the moment a path contains a quote or backslash. Use the same escaping as renderConfig in init.ts (JSON.stringify per element) — JSON string escaping is a valid YAML flow scalar. --- src/__tests__/index.shutdown.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts index e1726be..0bbbe3b 100644 --- a/src/__tests__/index.shutdown.test.ts +++ b/src/__tests__/index.shutdown.test.ts @@ -47,13 +47,16 @@ function raceWithTimeout( beforeEach(() => { fs.mkdirSync(PROJECT_DIR, { recursive: true }); + // Paths are escaped with JSON.stringify — same convention as renderConfig + // in init.ts — so quotes/backslashes in them can't corrupt the YAML. + const flow = (items: string[]) => `[${items.map((s) => JSON.stringify(s)).join(", ")}]`; fs.writeFileSync( path.join(PROJECT_DIR, "squiggles.yaml"), [ "servers:", " dummy:", - ` command: ["${process.execPath}", "${DUMMY_LSP}", "${path.join(PROJECT_DIR, "dummy-lsp.pid")}"]`, - ' filePatterns: ["*.txt"]', + ` command: ${flow([process.execPath, DUMMY_LSP, path.join(PROJECT_DIR, "dummy-lsp.pid")])}`, + ` filePatterns: ${flow(["*.txt"])}`, "", ].join("\n"), ); From 67604b144955974e372fd30909319921e204c1ec Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:43:52 +0800 Subject: [PATCH 6/9] Register stdin shutdown listeners before MCP connect --- src/index.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 1f900d6..0f245cb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -86,8 +86,6 @@ async function main(): Promise { }); const transport = new StdioServerTransport(); - await server.connect(transport); - log("MCP server connected via stdio"); let shuttingDown = false; const shutdown = async () => { @@ -107,11 +105,16 @@ async function main(): Promise { process.on("SIGINT", requestShutdown); process.on("SIGTERM", requestShutdown); - // The SDK assigns transport.onclose inside connect(), so hook the server - // instead — stdin EOF/close is the only signal guaranteed on host death. - server.onclose = requestShutdown; + // Register stdin listeners before connect(): StdioServerTransport starts + // consuming stdin during connect(), and EOF/close are one-shot events. process.stdin.on("end", requestShutdown); process.stdin.on("close", requestShutdown); + // The SDK assigns transport.onclose inside connect(), so hook the server + // instead of clobbering the transport's internal close handler. + server.onclose = requestShutdown; + + await server.connect(transport); + log("MCP server connected via stdio"); } main().catch((err) => { From 1c5ecffd6534bb368e75400be7cc2ca6ad61dde9 Mon Sep 17 00:00:00 2001 From: Haoyan Zhang <78358414+Bigzhangbig@users.noreply.github.com> Date: Fri, 25 Sep 2026 16:44:12 +0800 Subject: [PATCH 7/9] Cover immediate stdin EOF race in shutdown integration test --- src/__tests__/index.shutdown.test.ts | 29 +++++++--------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts index 0bbbe3b..57b2310 100644 --- a/src/__tests__/index.shutdown.test.ts +++ b/src/__tests__/index.shutdown.test.ts @@ -4,9 +4,11 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { spawn, type ChildProcess } from "node:child_process"; import * as fs from "node:fs"; import * as path from "node:path"; +import { fileURLToPath } from "node:url"; -const DIST_ENTRY = path.resolve(__dirname, "..", "..", "dist", "index.js"); -const FIXTURES = path.join(__dirname, "__shutdown_fixtures__"); +const TEST_DIR = path.dirname(fileURLToPath(import.meta.url)); +const DIST_ENTRY = path.resolve(TEST_DIR, "..", "..", "dist", "index.js"); +const FIXTURES = path.join(TEST_DIR, "__shutdown_fixtures__"); const DUMMY_LSP = path.join(FIXTURES, "dummy-lsp.mjs"); const TEST_ROOT = path.join(FIXTURES, "project"); @@ -176,26 +178,9 @@ describe("squiggles shutdown on stdin close (issue #2)", () => { }); try { - // Wait until squiggles is past server.connect() and its stdin - // listeners are wired — closes a tiny race where EOF could land - // before the listeners are registered. - let tick: NodeJS.Timeout | undefined; - const connected = await raceWithTimeout( - (settle) => { - tick = setInterval(() => { - if (stderrBuf.includes("MCP server connected via stdio")) { - settle(true); - } - }, 20); - }, - 5000, - () => false, - ); - clearInterval(tick); - if (!connected) { - throw new Error("child never reached 'MCP server connected'"); - } - + // Close stdin immediately, without waiting for server.connect(). + // This covers the race where StdioServerTransport starts consuming + // stdin during connect() and EOF arrives before connect() resolves. child.stdin!.end(); // Assert the process actually went through the shutdown path: the From 377e2c4675bb4a913b8efea72f1cb4f0c05aa533 Mon Sep 17 00:00:00 2001 From: Carl Dawson Date: Fri, 25 Sep 2026 14:30:05 +0100 Subject: [PATCH 8/9] Remove comments from shutdown wiring --- src/index.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/index.ts b/src/index.ts index 0f245cb..c5b486f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,11 +20,9 @@ const require = createRequire(import.meta.url); const { version } = require("../package.json"); async function main(): Promise { - // Once the host dies, writes to stdout/stderr emit EPIPE as an unhandled - // "error" event that would kill the process mid-shutdown, orphaning the - // LSP children. Suppressing is standard for stdio servers. - process.stdout.on("error", () => {}); - process.stderr.on("error", () => {}); + const ignoreWriteErrors = () => {}; + process.stdout.on("error", ignoreWriteErrors); + process.stderr.on("error", ignoreWriteErrors); if (process.argv[2] === "init") { const result = runInit(process.argv.slice(3), process.cwd()); @@ -105,12 +103,8 @@ async function main(): Promise { process.on("SIGINT", requestShutdown); process.on("SIGTERM", requestShutdown); - // Register stdin listeners before connect(): StdioServerTransport starts - // consuming stdin during connect(), and EOF/close are one-shot events. process.stdin.on("end", requestShutdown); process.stdin.on("close", requestShutdown); - // The SDK assigns transport.onclose inside connect(), so hook the server - // instead of clobbering the transport's internal close handler. server.onclose = requestShutdown; await server.connect(transport); From da45948a3198c2b736959ee35110bfed576ebac2 Mon Sep 17 00:00:00 2001 From: Carl Dawson Date: Fri, 25 Sep 2026 14:30:05 +0100 Subject: [PATCH 9/9] Simplify shutdown integration tests --- .../__shutdown_fixtures__/dummy-lsp.mjs | 43 ++- src/__tests__/index.shutdown.test.ts | 294 +++++++----------- 2 files changed, 130 insertions(+), 207 deletions(-) diff --git a/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs b/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs index 3ff8e66..cb33e82 100644 --- a/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs +++ b/src/__tests__/__shutdown_fixtures__/dummy-lsp.mjs @@ -1,35 +1,32 @@ -// Dummy LSP for the shutdown integration test. -// Writes its PID to argv[2] so the test can verify the process was reaped -// after squiggles exits. Answers initialize only, then hangs on everything -// (including shutdown) so the ONLY way to exit is squiggles's kill() — the -// path that used to be skipped when the EPIPE bug killed the parent mid-shutdown. -import process from "node:process"; import { writeFileSync } from "node:fs"; writeFileSync(process.argv[2], String(process.pid)); -function send(msg) { - const json = JSON.stringify(msg); +function send(message) { + const json = JSON.stringify(message); process.stdout.write(`Content-Length: ${Buffer.byteLength(json)}\r\n\r\n${json}`); } -let buf = Buffer.alloc(0); +let buffer = Buffer.alloc(0); + process.stdin.on("data", (chunk) => { - buf = Buffer.concat([buf, chunk]); + buffer = Buffer.concat([buffer, chunk]); + for (;;) { - const sep = buf.indexOf("\r\n\r\n"); - if (sep === -1) break; - const header = buf.slice(0, sep).toString(); - const m = /Content-Length: (\d+)/i.exec(header); - if (!m) { buf = buf.slice(sep + 4); continue; } - const len = parseInt(m[1], 10); - if (buf.length < sep + 4 + len) break; - const body = buf.slice(sep + 4, sep + 4 + len).toString(); - buf = buf.slice(sep + 4 + len); - const msg = JSON.parse(body); - if (msg.method === "initialize") { - send({ jsonrpc: "2.0", id: msg.id, result: { capabilities: { textDocumentSync: 1 } } }); + const separator = buffer.indexOf("\r\n\r\n"); + if (separator === -1) return; + + const length = Number(/Content-Length: (\d+)/i.exec(buffer.subarray(0, separator).toString())?.[1]); + const bodyStart = separator + 4; + if (buffer.length < bodyStart + length) return; + + const message = JSON.parse(buffer.subarray(bodyStart, bodyStart + length).toString()); + buffer = buffer.subarray(bodyStart + length); + + if (message.method === "initialize") { + send({ jsonrpc: "2.0", id: message.id, result: { capabilities: { textDocumentSync: 1 } } }); } } }); -setInterval(() => {}, 1000); \ No newline at end of file + +setInterval(() => {}, 1000); diff --git a/src/__tests__/index.shutdown.test.ts b/src/__tests__/index.shutdown.test.ts index 57b2310..d2b5766 100644 --- a/src/__tests__/index.shutdown.test.ts +++ b/src/__tests__/index.shutdown.test.ts @@ -1,212 +1,138 @@ -// Integration test for issue #2: closing the host stdin must shut down -// squiggles AND the LSP server it spawned (no orphan processes). -import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { spawn, type ChildProcess } from "node:child_process"; +import { describe, it, expect, beforeAll, beforeEach, afterEach } from "vitest"; +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { once } from "node:events"; import * as fs from "node:fs"; import * as path from "node:path"; -import { fileURLToPath } from "node:url"; +import { createInterface } from "node:readline"; +import { setTimeout as sleep } from "node:timers/promises"; -const TEST_DIR = path.dirname(fileURLToPath(import.meta.url)); -const DIST_ENTRY = path.resolve(TEST_DIR, "..", "..", "dist", "index.js"); -const FIXTURES = path.join(TEST_DIR, "__shutdown_fixtures__"); +const ENTRY = path.resolve(import.meta.dirname, "../../dist/index.js"); +const FIXTURES = path.join(import.meta.dirname, "__shutdown_fixtures__"); const DUMMY_LSP = path.join(FIXTURES, "dummy-lsp.mjs"); +const PROJECT = path.join(FIXTURES, "project"); +const EMPTY_PROJECT = path.join(FIXTURES, "empty-project"); +const PID_FILE = path.join(PROJECT, "dummy-lsp.pid"); -const TEST_ROOT = path.join(FIXTURES, "project"); -const PROJECT_DIR = TEST_ROOT; +let running: ChildProcessWithoutNullStreams | undefined; -function alive(pid: number): boolean { - try { process.kill(pid, 0); return true; } catch { return false; } -} +function startSquiggles(projectDir: string) { + const child = spawn(process.execPath, [ENTRY, projectDir]); + const answered = new Set(); + let stderr = ""; -const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); - -// Runs `setup` with a settle callback; whichever fires first — settle or the -// timer — wins, and the timer is always cleared so it can't leak past the -// end of the test. -function raceWithTimeout( - setup: (settle: (value: T) => void) => void, - ms: number, - onTimeout: () => T, -): Promise { - return new Promise((resolve) => { - let settled = false; - const timer = setTimeout(() => { - if (!settled) { - settled = true; - resolve(onTimeout()); - } - }, ms); - const settle = (v: T) => { - if (!settled) { - settled = true; - clearTimeout(timer); - resolve(v); - } - }; - setup(settle); + running = child; + child.stderr.setEncoding("utf8").on("data", (chunk: string) => { + stderr += chunk; + }); + createInterface({ input: child.stdout }).on("line", (line) => { + const message = JSON.parse(line); + if (typeof message.id === "number") answered.add(message.id); }); + + return { + child, + answered, + get stderr() { + return stderr; + }, + send(message: object) { + child.stdin.write(`${JSON.stringify(message)}\n`); + }, + }; +} + +async function exitCode(child: ChildProcessWithoutNullStreams, ms: number): Promise { + const [code] = await once(child, "close", { signal: AbortSignal.timeout(ms) }); + return code; +} + +async function waitFor(condition: () => boolean, ms: number, what: string): Promise { + const deadline = Date.now() + ms; + while (!condition()) { + if (Date.now() > deadline) throw new Error(`Timed out waiting for ${what}`); + await sleep(50); + } +} + +function lspPid(): number | undefined { + try { + return Number(fs.readFileSync(PID_FILE, "utf8")) || undefined; + } catch { + return undefined; + } +} + +function alive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } } +beforeAll(() => { + if (!fs.existsSync(ENTRY)) throw new Error(`${ENTRY} not found, run npm run build first`); +}); + beforeEach(() => { - fs.mkdirSync(PROJECT_DIR, { recursive: true }); - // Paths are escaped with JSON.stringify — same convention as renderConfig - // in init.ts — so quotes/backslashes in them can't corrupt the YAML. - const flow = (items: string[]) => `[${items.map((s) => JSON.stringify(s)).join(", ")}]`; + fs.mkdirSync(PROJECT, { recursive: true }); + fs.mkdirSync(EMPTY_PROJECT, { recursive: true }); fs.writeFileSync( - path.join(PROJECT_DIR, "squiggles.yaml"), + path.join(PROJECT, "squiggles.yaml"), [ "servers:", " dummy:", - ` command: ${flow([process.execPath, DUMMY_LSP, path.join(PROJECT_DIR, "dummy-lsp.pid")])}`, - ` filePatterns: ${flow(["*.txt"])}`, + ` command: ${JSON.stringify([process.execPath, DUMMY_LSP, PID_FILE])}`, + ` filePatterns: ["*.txt"]`, "", ].join("\n"), ); - fs.writeFileSync(path.join(PROJECT_DIR, "sample.txt"), "hello\n"); - fs.rmSync(path.join(PROJECT_DIR, "dummy-lsp.pid"), { force: true }); + fs.writeFileSync(path.join(PROJECT, "sample.txt"), "hello\n"); }); afterEach(() => { - fs.rmSync(TEST_ROOT, { recursive: true, force: true }); + running?.kill("SIGKILL"); + const pid = lspPid(); + if (pid && alive(pid)) process.kill(pid, "SIGKILL"); + fs.rmSync(PROJECT, { recursive: true, force: true }); + fs.rmSync(EMPTY_PROJECT, { recursive: true, force: true }); }); -describe("squiggles shutdown on stdin close (issue #2)", () => { - it("exits and reaps its LSP child when the host closes stdin", async () => { - if (!fs.existsSync(DIST_ENTRY)) { - throw new Error( - `dist/index.js not found at ${DIST_ENTRY} — run \`npm run build\` before this test`, - ); - } +describe("when stdin closes", () => { + it("exits and kills its LSP servers", async () => { + const squiggles = startSquiggles(PROJECT); - const squiggles: ChildProcess = spawn(process.execPath, [DIST_ENTRY, PROJECT_DIR], { - stdio: ["pipe", "pipe", "pipe"], + squiggles.send({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "test", version: "0" } }, }); - - const pidFile = path.join(PROJECT_DIR, "dummy-lsp.pid"); - const responses = new Map(); - let outBuf = ""; - squiggles.stderr?.on("data", () => {}); - squiggles.stdout?.on("data", (chunk) => { - outBuf += chunk.toString(); - let i; - while ((i = outBuf.indexOf("\n")) !== -1) { - const line = outBuf.slice(0, i).trim(); - outBuf = outBuf.slice(i + 1); - if (!line) continue; - try { - const msg = JSON.parse(line); - if (typeof msg.id === "number") responses.set(msg.id, msg); - } catch { /* ignore non-JSON noise */ } - } + await waitFor(() => squiggles.answered.has(1), 5000, "initialize response"); + squiggles.send({ jsonrpc: "2.0", method: "notifications/initialized" }); + squiggles.send({ + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "document_symbols", arguments: { file: "sample.txt" } }, }); + await waitFor(() => lspPid() !== undefined, 10_000, "dummy LSP to start"); + const pid = lspPid()!; + expect(alive(pid)).toBe(true); - const send = (obj: unknown) => squiggles.stdin!.write(JSON.stringify(obj) + "\n"); - const waitFor = async (cond: () => boolean, ms: number, what: string) => { - const start = Date.now(); - while (Date.now() - start < ms) { - if (cond()) return; - await sleep(50); - } - throw new Error(`timeout waiting for ${what}`); - }; - - try { - // MCP handshake. - send({ jsonrpc: "2.0", id: 1, method: "initialize", params: { protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "test", version: "0" } } }); - await waitFor(() => responses.has(1), 5000, "initialize response"); - send({ jsonrpc: "2.0", method: "notifications/initialized" }); - - // Trigger lazy LSP spawn via a tool call. We don't await a response — - // closing stdin will close the transport first. - send({ jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: "document_symbols", arguments: { file: "sample.txt" } } }); - - await waitFor(() => fs.existsSync(pidFile), 10000, "dummy LSP spawn"); - const lspPid = parseInt(fs.readFileSync(pidFile, "utf-8"), 10); - expect(Number.isFinite(lspPid)).toBe(true); - expect(alive(lspPid)).toBe(true); - - // Host closes its stdin pipe — the SIGKILL/pipe-close signal issue #2. - squiggles.stdin!.end(); - - // squiggles must exit. - const exit = await raceWithTimeout( - (settle) => squiggles.once("exit", (code) => settle(code)), - 15000, - () => null, - ); - expect(exit).not.toBeNull(); - expect(exit).toBe(0); - - // The dummy LSP child must have been reaped too (poll instead of a - // fixed sleep so a slow reap doesn't flake and a fast one doesn't wait). - const reapDeadline = Date.now() + 3000; - while (alive(lspPid) && Date.now() < reapDeadline) { - await sleep(50); - } - expect(alive(lspPid)).toBe(false); - } finally { - if (squiggles.exitCode === null) squiggles.kill("SIGKILL"); - // Best-effort: never leave the dummy LSP behind on a failed run. - if (fs.existsSync(pidFile)) { - const leftover = parseInt(fs.readFileSync(pidFile, "utf-8"), 10); - if (Number.isFinite(leftover) && alive(leftover)) { - try { process.kill(leftover, "SIGKILL"); } catch { /* already gone */ } - } - } - } - }, 30000); - - it("exits when stdin closes before any MCP traffic", async () => { - if (!fs.existsSync(DIST_ENTRY)) { - throw new Error( - `dist/index.js not found at ${DIST_ENTRY} — run \`npm run build\` before this test`, - ); - } - - // No-LSP project so we don't accidentally spawn a child to clean up. - const noLspDir = path.join(FIXTURES, "project-nolsp"); - fs.mkdirSync(noLspDir, { recursive: true }); - - const child = spawn(process.execPath, [DIST_ENTRY, noLspDir], { - stdio: ["pipe", "pipe", "pipe"], - }); - // Drain output so the child never blocks on a full pipe. - child.stdout?.on("data", () => {}); - let stderrBuf = ""; - child.stderr?.on("data", (chunk) => { - stderrBuf += chunk.toString(); - }); + squiggles.child.stdin.end(); + + expect(await exitCode(squiggles.child, 15_000)).toBe(0); + await waitFor(() => !alive(pid), 3000, "dummy LSP to exit"); + }, 30_000); + + it("exits through shutdown before any requests", async () => { + const squiggles = startSquiggles(EMPTY_PROJECT); - try { - // Close stdin immediately, without waiting for server.connect(). - // This covers the race where StdioServerTransport starts consuming - // stdin during connect() and EOF arrives before connect() resolves. - child.stdin!.end(); - - // Assert the process actually went through the shutdown path: the - // base build accidentally exits cleanly on a no-LSP project because - // Node's event loop drains once every fd is idle — but it never logs - // "Shutting down..." because no shutdown handler exists. Only the - // fixed build takes the explicit path. - const result = await raceWithTimeout( - (settle) => { - child.once("exit", (code, signal) => { - settle({ - code: code ?? signal ?? null, - loggedShutdown: stderrBuf.includes("Shutting down..."), - }); - }); - }, - 5000, - () => ({ code: "timeout" as number | string | null, loggedShutdown: false }), - ); - - expect(result.code).not.toBe("timeout"); - expect(result.code).toBe(0); - expect(result.loggedShutdown).toBe(true); - } finally { - if (child.exitCode === null) child.kill("SIGKILL"); - fs.rmSync(noLspDir, { recursive: true, force: true }); - } - }, 15000); -}); \ No newline at end of file + squiggles.child.stdin.end(); + + expect(await exitCode(squiggles.child, 5000)).toBe(0); + expect(squiggles.stderr).toContain("Shutting down..."); + }, 15_000); +});