From f54b14bfe06bad620889008cb65ab3277003e76c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 00:08:45 -0500 Subject: [PATCH 1/6] test(mcpdo): run the daemon suites on Windows against a named pipe (#2922) The daemon test harness derives the endpoint from getDaemonSocketPath instead of a hard-coded /mcpdod.sock, keys fake-daemon tokens by directory, passes dir alongside a pipe endpoint, and tears a spawned daemon down by its token/lock (present on every platform) with an EBUSY-tolerant rmSync. unixDaemonOnly is gone; the few remaining win32 skips are POSIX-only behaviour (socket files, sun_path, mode bits), each with its reason at the site. mcpdo's per-file coverage threshold is enforced on Windows again. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .../mcpdo/__tests__/daemon-coverage.test.ts | 193 ++- .../mcpdo/__tests__/daemon-handshake.test.ts | 48 +- clients/mcpdo/__tests__/daemon-paths.test.ts | 86 +- .../mcpdo/__tests__/daemon-private.test.ts | 34 +- clients/mcpdo/__tests__/daemon-stream.test.ts | 75 +- .../mcpdo/__tests__/directory-read.test.ts | 228 ++-- .../__tests__/elicitation-client.test.ts | 19 +- .../__tests__/helpers/daemon-endpoint.ts | 85 ++ .../mcpdo/__tests__/helpers/fake-daemon.ts | 14 +- clients/mcpdo/__tests__/helpers/platform.ts | 26 +- clients/mcpdo/__tests__/load.test.ts | 1037 ++++++++--------- .../mcpdo/__tests__/mcp-connection.test.ts | 21 +- clients/mcpdo/__tests__/mcp-coverage.test.ts | 839 +++++++------ .../mcpdo/__tests__/mcp-elicitation.test.ts | 21 +- .../__tests__/oauth-error-envelope.test.ts | 7 +- .../__tests__/rate-limit-envelope.test.ts | 29 +- .../__tests__/skills-budget-hint.test.ts | 204 ++-- clients/mcpdo/vitest.config.ts | 25 +- 18 files changed, 1498 insertions(+), 1493 deletions(-) create mode 100644 clients/mcpdo/__tests__/helpers/daemon-endpoint.ts diff --git a/clients/mcpdo/__tests__/daemon-coverage.test.ts b/clients/mcpdo/__tests__/daemon-coverage.test.ts index 6aad1c1cdb..1f50f87383 100644 --- a/clients/mcpdo/__tests__/daemon-coverage.test.ts +++ b/clients/mcpdo/__tests__/daemon-coverage.test.ts @@ -33,7 +33,14 @@ import { expectCliSuccess, expectCliFailure, } from "../../cli/__tests__/helpers/assertions.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { isWindows } from "./helpers/platform.js"; +import { + daemonEndpoint, + daemonMayBeLive, + releaseEndpoint, + removeDaemonDir, + stopDaemonIn, +} from "./helpers/daemon-endpoint.js"; import { readDaemonTokenFile } from "../src/daemon/auth.js"; import { FAKE_DAEMON_TOKEN, @@ -41,8 +48,10 @@ import { publishFakeDaemonToken, } from "./helpers/fake-daemon.js"; -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("daemon coverage", () => { +// Every client call passes `dir` alongside `socketPath`: on Windows the +// endpoint is a named pipe, from which the client cannot derive the token's +// directory (see `daemonTokenDir` and helpers/daemon-endpoint.ts). +describe("daemon coverage", () => { let server: DaemonServer | undefined; let dir: string | undefined; let configPath: string | undefined; @@ -53,7 +62,9 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { server = undefined; } if (dir) { - fs.rmSync(dir, { recursive: true, force: true }); + // Some cases spawn a real daemon process into `dir`. + await stopDaemonIn(dir); + removeDaemonDir(dir); dir = undefined; } if (configPath) { @@ -138,7 +149,9 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { process.env.MCP_INSPECTOR_DAEMON_DIR = freshDir(); try { const defs = new DaemonServer(); - expect(defs.socketPath).toContain("mcpdod.sock"); + expect(defs.socketPath).toBe( + daemonEndpoint(process.env.MCP_INSPECTOR_DAEMON_DIR!), + ); } finally { if (prev === undefined) delete process.env.MCP_INSPECTOR_DAEMON_DIR; else process.env.MCP_INSPECTOR_DAEMON_DIR = prev; @@ -267,9 +280,11 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { client.destroy(); }); - it("removes a stale socket before binding", async () => { + // POSIX-only: a stale endpoint is a leftover socket FILE; a named pipe + // vanishes with its server, so Windows never has one to remove. + it.skipIf(isWindows)("removes a stale socket before binding", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); fs.writeFileSync(sock, ""); server = new DaemonServer({ dir: d, idleMs: 0 }); await server.start(); @@ -283,7 +298,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { const result = await callDaemon<{ stopping: boolean }>( "daemon/stop", {}, - { socketPath: server.socketPath }, + { socketPath: server.socketPath, dir: server.dir }, ); expect(result.stopping).toBe(true); // Allow async stop to finish. @@ -345,7 +360,12 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { callDaemon( "ping", {}, - { socketPath: server.socketPath, timeoutMs: 0, signal: ac.signal }, + { + socketPath: server.socketPath, + dir: server.dir, + timeoutMs: 0, + signal: ac.signal, + }, ), ).rejects.toThrow(/cancelled/); }); @@ -355,7 +375,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { callDaemon( "ping", {}, - { socketPath: path.join(freshDir(), "missing.sock") }, + { socketPath: daemonEndpoint(freshDir()), dir: dir! }, ), ).rejects.toThrow(CliExitCodeError); @@ -363,13 +383,17 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { server = new DaemonServer({ dir: d, idleMs: 0 }); await server.start(); await expect( - callDaemon("connections/use", {}, { socketPath: server.socketPath }), + callDaemon( + "connections/use", + {}, + { socketPath: server.socketPath, dir: server.dir }, + ), ).rejects.toThrow(/requires a connection name/); }); it("callDaemon reassembles a multi-byte UTF-8 character split across chunks", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const value = "hΓ©llo πŸ‘‹ wΓΆrld"; const splitter = net.createServer((socket) => { socket.on("error", () => {}); @@ -387,21 +411,17 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { }); }); await new Promise((resolve) => splitter.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { const result = await callDaemon<{ value: string }>( "ping", {}, - { socketPath: sock, timeoutMs: 2000 }, + { socketPath: sock, dir: d, timeoutMs: 2000 }, ); expect(result.value).toBe(value); } finally { splitter.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }); @@ -416,7 +436,8 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { "ping", {}, { - socketPath: path.join(d, "absent.sock"), + socketPath: daemonEndpoint(d), + dir: d, signal: ac.signal, timeoutMs: 2000, }, @@ -426,31 +447,27 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { it("callDaemon rejects malformed response JSON", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const bad = net.createServer((socket) => { socket.on("error", () => {}); // Past the handshake, so the malformed line is the RESPONSE. onRequest(socket, () => socket.write("not-json\n")); }); await new Promise((resolve) => bad.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { await expect( - callDaemon("ping", {}, { socketPath: sock, timeoutMs: 2000 }), + callDaemon("ping", {}, { socketPath: sock, dir: d, timeoutMs: 2000 }), ).rejects.toThrow(SyntaxError); } finally { bad.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }); it("callDaemon ignores mismatched response ids then accepts a match", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const echo = net.createServer((socket) => { socket.on("error", () => {}); onRequest(socket, (buf) => { @@ -464,27 +481,23 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { }); }); await new Promise((resolve) => echo.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { const result = await callDaemon<{ ok: boolean }>( "ping", {}, - { socketPath: sock, timeoutMs: 2000 }, + { socketPath: sock, dir: d, timeoutMs: 2000 }, ); expect(result.ok).toBe(true); } finally { echo.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }); it("callDaemon skips blank lines and defaults missing exitCode", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const echo = net.createServer((socket) => { socket.on("error", () => {}); onRequest(socket, (buf) => { @@ -500,18 +513,14 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { }); }); await new Promise((resolve) => echo.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { await expect( - callDaemon("ping", {}, { socketPath: sock, timeoutMs: 2000 }), + callDaemon("ping", {}, { socketPath: sock, dir: d, timeoutMs: 2000 }), ).rejects.toMatchObject({ exitCode: 1 }); } finally { echo.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }); @@ -522,7 +531,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { server = new DaemonServer({ dir: d, idleMs: 0 }); await server.start(); - fs.unlinkSync(server.socketPath); + releaseEndpoint(server.socketPath); fs.unlinkSync(path.join(d, "mcpdod.lock")); await server.stop("stop"); server = undefined; @@ -539,6 +548,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { expect(second).toBe(first); await first; expect(fs.existsSync(server.socketPath)).toBe(false); + expect(daemonMayBeLive(server.dir)).toBe(false); server = undefined; }); @@ -604,23 +614,19 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { it("callDaemon times out a hung server", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const hung = net.createServer((socket) => { socket.on("error", () => {}); }); await new Promise((resolve) => hung.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { await expect( - callDaemon("ping", {}, { socketPath: sock, timeoutMs: 100 }), + callDaemon("ping", {}, { socketPath: sock, dir: d, timeoutMs: 100 }), ).rejects.toThrow(/timed out/); } finally { hung.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }, 5000); @@ -628,19 +634,19 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { // rpc/connect callers pass 0 because the daemon enforces the configured // MCP timeouts; a fixed 60s local timer falsely failed long tool calls. const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const sockets: net.Socket[] = []; const silent = net.createServer((socket) => { sockets.push(socket); socket.on("error", () => {}); }); await new Promise((resolve) => silent.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { const pending = callDaemon( "ping", {}, - { socketPath: sock, timeoutMs: 0 }, + { socketPath: sock, dir: d, timeoutMs: 0 }, ); let settled = false; // void: observer only; the promise itself is asserted on below. @@ -653,11 +659,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { await expect(pending).rejects.toThrow(/closed the connection/); } finally { silent.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }, 5000); @@ -802,7 +804,11 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { daemonScript: resolveDaemonScriptPath(), }); expect(ensured.spawned).toBe(true); - await callDaemon("daemon/stop", {}, { socketPath: ensured.socketPath }); + await callDaemon( + "daemon/stop", + {}, + { socketPath: ensured.socketPath, dir: d }, + ); await new Promise((r) => setTimeout(r, 150)); }); @@ -841,12 +847,12 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { it("waitForDaemonExit falls back to socket reachability without a pid", async () => { const d = freshDir(); - await waitForDaemonExit(undefined, path.join(d, "absent.sock"), 500, 10); + await waitForDaemonExit(undefined, daemonEndpoint(d), 500, 10); }); it("ensureDaemon waits out a stopping daemon and spawns a fresh one", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); // The "old daemon": a process that takes a moment to exit, and a socket // that answers ping with stopping:true (as the real dispatch does). const oldDaemon = spawn( @@ -870,16 +876,12 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { }); }); await new Promise((r) => stoppingSocket.listen(sock, r)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); // Mid-shutdown the socket goes away before the process does β€” the gap // where spawning too early would die on the still-held lock. setTimeout(() => { stoppingSocket.close(); - try { - fs.unlinkSync(sock); - } catch { - // already gone - } + releaseEndpoint(sock); }, 200); try { const ensured = await ensureDaemon({ @@ -887,7 +889,11 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { daemonScript: resolveDaemonScriptPath(), }); expect(ensured.spawned).toBe(true); - await callDaemon("daemon/stop", {}, { socketPath: ensured.socketPath }); + await callDaemon( + "daemon/stop", + {}, + { socketPath: ensured.socketPath, dir: d }, + ); await new Promise((r) => setTimeout(r, 150)); } finally { oldDaemon.kill(); @@ -949,7 +955,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { "ensureDaemon refuses a running daemon reporting %s", async (_label, extra, named) => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const old = await fakeDaemon(sock, { pong: true, pid: process.pid, @@ -974,7 +980,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { // concurrent first invocation from another release can win the lock. // This "daemon" binds the socket and reports another release. const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const script = path.join(d, "other-release-daemon.mjs"); fs.writeFileSync( script, @@ -1012,7 +1018,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { it("ensureDaemon reuses a running daemon of the same release", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const root = JSON.parse( fs.readFileSync( new URL("../../../package.json", import.meta.url), @@ -1084,13 +1090,13 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { // connections is owned by a live process. ensureDaemon must never unlink // it and install a replacement daemon β€” it must surface the ping failure. const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const occupant = net.createServer((socket) => { socket.on("error", () => {}); socket.end(); }); await new Promise((resolve) => occupant.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { await expect( ensureDaemon({ dir: d, daemonScript: resolveDaemonScriptPath() }), @@ -1121,6 +1127,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { {}, { socketPath: server.socketPath, + dir: d, token: "good", }, ); @@ -1191,27 +1198,23 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { it("callDaemon fails immediately when the peer closes without a response", async () => { const d = freshDir(); - const sock = path.join(d, "mcpdod.sock"); + const sock = daemonEndpoint(d); const peer = net.createServer((socket) => { socket.on("error", () => {}); // Accept then FIN with no NDJSON reply. socket.end(); }); await new Promise((resolve) => peer.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { await expect( - callDaemon("ping", {}, { socketPath: sock, timeoutMs: 60_000 }), + callDaemon("ping", {}, { socketPath: sock, dir: d, timeoutMs: 60_000 }), ).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" }, }); } finally { peer.close(); - try { - fs.unlinkSync(sock); - } catch { - // ignore - } + releaseEndpoint(sock); } }); @@ -1227,7 +1230,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { serverConfig: { type: "stdio", command, args }, serverIdentity: "s", }, - { socketPath: server.socketPath, timeoutMs: 15000 }, + { socketPath: server.socketPath, dir: server.dir, timeoutMs: 15000 }, ); const token = readDaemonTokenFile(d); const used = await server.handle({ @@ -1267,7 +1270,7 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { await callDaemon( "disconnect", { name: "s" }, - { socketPath: server.socketPath }, + { socketPath: server.socketPath, dir: server.dir }, ); // Idle timer armed β€” remaining countdown is positive and ≀ configured idleMs. const remaining = server.registry.idleRemainingMs(); @@ -1277,26 +1280,14 @@ describe.skipIf(unixDaemonOnly)("daemon coverage", () => { }); }); -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("mcp connection coverage", () => { +describe("mcp connection coverage", () => { let configPath: string | undefined; let storageDir: string | undefined; afterEach(async () => { if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon("daemon/stop", {}, { socketPath, timeoutMs: 2000 }); - } catch { - // ignore - } - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/daemon-handshake.test.ts b/clients/mcpdo/__tests__/daemon-handshake.test.ts index b3103fb732..ddc8b96c22 100644 --- a/clients/mcpdo/__tests__/daemon-handshake.test.ts +++ b/clients/mcpdo/__tests__/daemon-handshake.test.ts @@ -29,7 +29,7 @@ import { createDaemonHello, } from "../src/daemon/handshake.js"; import type { ConnectParams } from "../src/daemon/protocol.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { daemonEndpoint } from "./helpers/daemon-endpoint.js"; import { publishFakeDaemonToken } from "./helpers/fake-daemon.js"; /** The victim's token, published in its daemon dir. */ @@ -95,9 +95,9 @@ function helloOf(line: string): { id: string; nonce: string } { const unverified = { envelope: { code: "daemon_unverified" } }; -// Skipped on win32: needs a Unix socket (helpers/platform.ts, #2748). The -// handshake itself is platform-independent; Windows is the platform it is for. -describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { +// The handshake is platform-independent, and Windows is the platform it is +// for: a named pipe lives in a machine-wide namespace (see handshake.ts). +describe("daemon hello handshake", () => { let dir: string | undefined; const servers: net.Server[] = []; let daemon: DaemonServer | undefined; @@ -118,7 +118,12 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { function freshSock(): string { dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-hello-")); - return path.join(dir, "mcpdod.sock"); + return daemonEndpoint(dir); + } + + function freshDir(): string { + freshSock(); + return dir!; } async function listenImpostor(sock: string, answer: Answer) { @@ -201,12 +206,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { "callDaemon('connect') refuses an impostor answering with %s, having sent only the hello", async (_label, answer) => { const sock = freshSock(); - publishFakeDaemonToken(sock, VICTIM_TOKEN); + publishFakeDaemonToken(dir!, VICTIM_TOKEN); const { received } = await listenImpostor(sock, answer); let elicited = false; await expect( callDaemon("connect", connectParams, { socketPath: sock, + dir, timeoutMs: 2000, onElicitation: () => { elicited = true; @@ -223,7 +229,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { "streamDaemon refuses an impostor answering with %s, having sent only the hello", async (_label, answer) => { const sock = freshSock(); - publishFakeDaemonToken(sock, VICTIM_TOKEN); + publishFakeDaemonToken(dir!, VICTIM_TOKEN); const { received } = await listenImpostor(sock, answer); const data: unknown[] = []; await expect( @@ -231,6 +237,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { { method: "logging/tail" }, { socketPath: sock, + dir, timeoutMs: 2000, onData: (d) => { data.push(d); @@ -253,13 +260,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { delete process.env.MCP_INSPECTOR_DAEMON_TOKEN; try { await expect( - callDaemon("connect", connectParams, { socketPath: sock }), + callDaemon("connect", connectParams, { socketPath: sock, dir }), ).rejects.toMatchObject({ ...unverified, message: expect.stringContaining("No connection daemon token found"), }); await expect( - streamDaemon({}, { socketPath: sock, onData: () => {} }), + streamDaemon({}, { socketPath: sock, dir, onData: () => {} }), ).rejects.toMatchObject(unverified); // ensureDaemon finds the endpoint occupied and must not ping it. await expect(ensureDaemon({ dir: dir! })).rejects.toMatchObject( @@ -276,13 +283,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { it("without a token, nothing listening is still reported as unreachable", async () => { const sock = freshSock(); await expect( - callDaemon("ping", {}, { socketPath: sock, timeoutMs: 2000 }), + callDaemon("ping", {}, { socketPath: sock, dir, timeoutMs: 2000 }), ).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" } }); }); it("ensureDaemon refuses an impostor on the endpoint even with a token on disk", async () => { const sock = freshSock(); - publishFakeDaemonToken(sock, VICTIM_TOKEN); + publishFakeDaemonToken(dir!, VICTIM_TOKEN); const { received } = await listenImpostor(sock, (line, socket) => reply(socket, { id: helloOf(line).id, @@ -295,13 +302,13 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { }); it("the genuine daemon proves its token and serves calls and streams", async () => { - const sock = freshSock(); - daemon = new DaemonServer({ dir: path.dirname(sock), idleMs: 0 }); + const d = freshDir(); + daemon = new DaemonServer({ dir: d, idleMs: 0 }); await daemon.start(); const pong = await callDaemon<{ pong: boolean }>( "ping", {}, - { socketPath: daemon.socketPath, timeoutMs: 2000 }, + { socketPath: daemon.socketPath, dir: d, timeoutMs: 2000 }, ); expect(pong.pong).toBe(true); // A stream over an unknown connection is refused by the daemon itself β€” @@ -309,13 +316,18 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { await expect( streamDaemon( { name: "missing", method: "logging/tail" }, - { socketPath: daemon.socketPath, timeoutMs: 2000, onData: () => {} }, + { + socketPath: daemon.socketPath, + dir: d, + timeoutMs: 2000, + onData: () => {}, + }, ), ).rejects.not.toMatchObject(unverified); }); it("the daemon answers hello with the HMAC of the nonce, without the token, even while stopping", async () => { - const d = path.dirname(freshSock()); + const d = freshDir(); daemon = new DaemonServer({ dir: d, idleMs: 0 }); await daemon.start(); const token = readDaemonTokenFile(d)!; @@ -345,7 +357,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { it("the daemon rejects a malformed hello nonce", async () => { const srv = new DaemonServer({ - dir: path.dirname(freshSock()), + dir: freshDir(), idleMs: 0, requiredToken: "t", }); @@ -367,7 +379,7 @@ describe.skipIf(unixDaemonOnly)("daemon hello handshake", () => { delete process.env.MCP_INSPECTOR_DAEMON_TOKEN; try { const srv = new DaemonServer({ - dir: path.dirname(freshSock()), + dir: freshDir(), idleMs: 0, }); const answer = await srv.handle({ diff --git a/clients/mcpdo/__tests__/daemon-paths.test.ts b/clients/mcpdo/__tests__/daemon-paths.test.ts index 7852a60614..b484ca1083 100644 --- a/clients/mcpdo/__tests__/daemon-paths.test.ts +++ b/clients/mcpdo/__tests__/daemon-paths.test.ts @@ -12,7 +12,7 @@ import { getDaemonSocketPath, } from "../src/daemon/paths.js"; import { writeFormattedResult } from "@inspector/core/cli/handlers/format-output.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { isWindows } from "./helpers/platform.js"; describe("daemon paths", () => { const backup: Record = {}; @@ -23,6 +23,8 @@ describe("daemon paths", () => { "MCP_STORAGE_DIR", "HOME", "TMPDIR", + "TEMP", + "TMP", ]) { if (key in backup) { if (backup[key] === undefined) delete process.env[key]; @@ -38,23 +40,28 @@ describe("daemon paths", () => { else process.env[key] = value; } - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( - "prefers MCP_INSPECTOR_DAEMON_DIR over MCP_STORAGE_DIR", - () => { - const a = path.join(os.tmpdir(), "daemon-a"); - const b = path.join(os.tmpdir(), "daemon-b"); - setEnv("MCP_STORAGE_DIR", b); - setEnv("MCP_INSPECTOR_DAEMON_DIR", a); - expect(getDaemonDir()).toBe(path.resolve(a)); + it("prefers MCP_INSPECTOR_DAEMON_DIR over MCP_STORAGE_DIR", () => { + const a = path.join(os.tmpdir(), "daemon-a"); + const b = path.join(os.tmpdir(), "daemon-b"); + setEnv("MCP_STORAGE_DIR", b); + setEnv("MCP_INSPECTOR_DAEMON_DIR", a); + expect(getDaemonDir()).toBe(path.resolve(a)); + if (isWindows) { + // The endpoint is a pipe keyed by the directory, not a file inside it. + expect(getDaemonSocketPath()).toMatch( + /^\\\\\.\\pipe\\mcp-conn-[0-9a-f]{16}$/, + ); + expect(getDaemonSocketPath()).toBe(getDaemonSocketPath(path.resolve(a))); + expect(getDaemonSocketPath()).not.toBe( + getDaemonSocketPath(path.resolve(b)), + ); + } else { expect(getDaemonSocketPath()).toBe( path.join(path.resolve(a), "mcpdod.sock"), ); - expect(getDaemonLockPath()).toBe( - path.join(path.resolve(a), "mcpdod.lock"), - ); - }, - ); + } + expect(getDaemonLockPath()).toBe(path.join(path.resolve(a), "mcpdod.lock")); + }); it("falls back to MCP_STORAGE_DIR then ~/.mcp-inspector", () => { const storage = path.join(os.tmpdir(), "daemon-storage"); @@ -72,8 +79,9 @@ describe("daemon paths", () => { fs.rmSync(dir, { recursive: true, force: true }); }); - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( + // POSIX-only: asserts 0700 mode bits, and `ensureDaemonDir`'s ownership and + // mode checks are a no-op on Windows (no getuid, no UNIX mode semantics). + it.skipIf(isWindows)( "tightens a pre-existing loose daemon directory to 0700 and rejects symlinks", () => { // mkdirSync never re-modes an existing dir; ~/.mcp-inspector commonly @@ -94,26 +102,25 @@ describe("daemon paths", () => { }, ); - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( - "createPrivateDaemonDir nests under a short 0700 tmpdir layout", - () => { - const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-t-")); - setEnv("TMPDIR", tmp + path.sep); - const dir = createPrivateDaemonDir(); - // $TMPDIR/mcp-conn-/<8-hex>; short enough that mcpdod.sock stays inside - // the platform sun_path limit even for macOS /var/folders tmpdirs. - expect(dir.startsWith(tmp)).toBe(true); - expect(path.basename(dir)).toMatch(/^[0-9a-f]{8}$/); - expect(path.basename(path.dirname(dir))).toMatch(/^mcp-conn-/); - expect(fs.statSync(dir).isDirectory()).toBe(true); - if (process.platform !== "win32") { - expect(fs.statSync(dir).mode & 0o777).toBe(0o700); - expect(fs.statSync(path.dirname(dir)).mode & 0o777).toBe(0o700); - } - fs.rmSync(tmp, { recursive: true, force: true }); - }, - ); + it("createPrivateDaemonDir nests under a short 0700 tmpdir layout", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-t-")); + // os.tmpdir() reads TMPDIR on POSIX and TEMP/TMP on Windows. + setEnv("TMPDIR", tmp + path.sep); + setEnv("TEMP", tmp); + setEnv("TMP", tmp); + const dir = createPrivateDaemonDir(); + // $TMPDIR/mcp-conn-/<8-hex>; short enough that mcpdod.sock stays inside + // the platform sun_path limit even for macOS /var/folders tmpdirs. + expect(dir.startsWith(tmp)).toBe(true); + expect(path.basename(dir)).toMatch(/^[0-9a-f]{8}$/); + expect(path.basename(path.dirname(dir))).toMatch(/^mcp-conn-/); + expect(fs.statSync(dir).isDirectory()).toBe(true); + if (process.platform !== "win32") { + expect(fs.statSync(dir).mode & 0o777).toBe(0o700); + expect(fs.statSync(path.dirname(dir)).mode & 0o777).toBe(0o700); + } + fs.rmSync(tmp, { recursive: true, force: true }); + }); it("createPrivateDaemonDir refuses a symlinked mcp-conn root", () => { if (process.platform === "win32") return; @@ -142,8 +149,9 @@ describe("daemon paths", () => { fs.rmSync(tmp, { recursive: true, force: true }); }); - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( + // POSIX-only: `sun_path` bounds a socket FILE; Windows named pipes have no + // such limit, which the named-pipe case below asserts on every platform. + it.skipIf(isWindows)( "assertSocketPathWithinLimit rejects paths over the sun_path limit", () => { expect(() => diff --git a/clients/mcpdo/__tests__/daemon-private.test.ts b/clients/mcpdo/__tests__/daemon-private.test.ts index 8a7a6b76d1..77791f8d40 100644 --- a/clients/mcpdo/__tests__/daemon-private.test.ts +++ b/clients/mcpdo/__tests__/daemon-private.test.ts @@ -28,7 +28,7 @@ import { createPrivateBinding, formatPrivateEnvExports, } from "../src/connection/private-env.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; describe("daemon IPC token", () => { it("compares tokens in constant time", () => { @@ -110,8 +110,9 @@ describe("mcpdo private", () => { }); }); -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { +// Every call passes `dir`: on Windows the endpoint is a named pipe, from which +// the client cannot derive the token's directory (see `daemonTokenDir`). +describe("private daemon end-to-end", () => { let server: DaemonServer | undefined; let dir: string | undefined; @@ -121,7 +122,9 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { server = undefined; } if (dir) { - fs.rmSync(dir, { recursive: true, force: true }); + // A daemon `ensureDaemon` spawned is a separate process. + await stopDaemonIn(dir); + removeDaemonDir(dir); dir = undefined; } }); @@ -140,7 +143,12 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { callDaemon( "ping", {}, - { socketPath: server.socketPath, timeoutMs: 2000, token: "wrong" }, + { + socketPath: server.socketPath, + dir, + timeoutMs: 2000, + token: "wrong", + }, ), ).rejects.toMatchObject({ envelope: { code: "daemon_unverified" } }); // And the daemon still refuses a request carrying the wrong token, for a @@ -152,18 +160,18 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { error: { code: "daemon_auth_failed" }, }); - // Tokenless call discovers the published token file next to the socket. + // Tokenless call discovers the token file published in the daemon dir. const discovered = await callDaemon<{ pong: boolean }>( "ping", {}, - { socketPath: server.socketPath, timeoutMs: 2000 }, + { socketPath: server.socketPath, dir, timeoutMs: 2000 }, ); expect(discovered.pong).toBe(true); const pong = await callDaemon<{ pong: boolean }>( "ping", {}, - { socketPath: server.socketPath, timeoutMs: 2000, token }, + { socketPath: server.socketPath, dir, timeoutMs: 2000, token }, ); expect(pong.pong).toBe(true); }); @@ -260,7 +268,7 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { const pong = await callDaemon<{ pong: boolean }>( "ping", {}, - { socketPath, timeoutMs: 2000, token: "winner-token" }, + { socketPath, dir, timeoutMs: 2000, token: "winner-token" }, ); expect(pong.pong).toBe(true); } finally { @@ -322,14 +330,14 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { callDaemon( "ping", {}, - { socketPath, timeoutMs: 2000, token: "wrong" }, + { socketPath, dir, timeoutMs: 2000, token: "wrong" }, ), ).rejects.toMatchObject({ envelope: { code: "daemon_unverified" } }); const pong = await callDaemon<{ pong: boolean }>( "ping", {}, - { socketPath, timeoutMs: 2000, token }, + { socketPath, dir, timeoutMs: 2000, token }, ); expect(pong.pong).toBe(true); @@ -341,9 +349,9 @@ describe.skipIf(unixDaemonOnly)("private daemon end-to-end", () => { serverConfig: { type: "stdio", command, args }, serverIdentity: "s", }, - { socketPath, timeoutMs: 15000, token }, + { socketPath, dir, timeoutMs: 15000, token }, ); - await callDaemon("daemon/stop", {}, { socketPath, token }); + await callDaemon("daemon/stop", {}, { socketPath, dir, token }); } finally { if (prevDir === undefined) delete process.env[DAEMON_DIR_ENV]; else process.env[DAEMON_DIR_ENV] = prevDir; diff --git a/clients/mcpdo/__tests__/daemon-stream.test.ts b/clients/mcpdo/__tests__/daemon-stream.test.ts index b914fc9365..1bbe87cf47 100644 --- a/clients/mcpdo/__tests__/daemon-stream.test.ts +++ b/clients/mcpdo/__tests__/daemon-stream.test.ts @@ -12,11 +12,11 @@ import { CliExitCodeError, EXIT_CODES, } from "@inspector/core/cli/error-handler.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { isWindows } from "./helpers/platform.js"; +import { daemonEndpoint } from "./helpers/daemon-endpoint.js"; import { onRequest, publishFakeDaemonToken } from "./helpers/fake-daemon.js"; -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { +describe("streamDaemon + ipc-glue", () => { let dir: string | undefined; let server: net.Server | undefined; const sockets = new Set(); @@ -40,7 +40,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { function freshSock(): string { dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-stream-")); - return path.join(dir, "mcpdod.sock"); + return daemonEndpoint(dir); } async function listen( @@ -54,7 +54,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { onSocket(socket); }); await new Promise((resolve) => server!.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(dir!); } it("resolves immediately on an already-aborted signal without dialing", async () => { @@ -68,6 +68,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { { method: "logging/tail" }, { socketPath: sock, + dir, timeoutMs: 2000, signal: ac.signal, onData: () => {}, @@ -108,6 +109,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { { method: "logging/tail" }, { socketPath: sock, + dir, timeoutMs: 5000, onData: (d) => { data.push(d); @@ -152,6 +154,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { { method: "logging/tail" }, { socketPath: sock, + dir, timeoutMs: 5000, // First callback stalls on the gate; the client must stop reading // instead of queueing further frames behind an unbounded chain. @@ -204,6 +207,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { { method: "logging/tail" }, { socketPath: sock, + dir, timeoutMs: 5000, onData: (d) => { seen.push(d); @@ -226,7 +230,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 2000, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, + ), ).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" }, }); @@ -244,7 +251,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 2000, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, + ), ).rejects.toThrow(); }); @@ -264,7 +274,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 2000, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, + ), ).rejects.toMatchObject({ exitCode: EXIT_CODES.USAGE }); }); @@ -276,7 +289,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 2000, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, + ), ).rejects.toThrow(); }); @@ -291,6 +307,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { {}, { socketPath: sock, + dir, timeoutMs: 5000, signal: ac.signal, onData: () => {}, @@ -314,6 +331,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { {}, { socketPath: sock, + dir, timeoutMs: 5000, signal: ac.signal, onData: () => {}, @@ -332,7 +350,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 80, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 80, onData: () => {} }, + ), ).rejects.toThrow(/timed out/); }, 5000); @@ -353,7 +374,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 0, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 0, onData: () => {} }, + ), ).resolves.toBeUndefined(); }, 5000); @@ -368,7 +392,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { await expect( streamDaemon( {}, - { socketPath: sock, timeoutMs: 60_000, onData: () => {} }, + { socketPath: sock, dir, timeoutMs: 60_000, onData: () => {} }, ), ).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" }, @@ -387,7 +411,10 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); }); await expect( - streamDaemon({}, { socketPath: sock, timeoutMs: 2000, onData: () => {} }), + streamDaemon( + {}, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, + ), ).rejects.toMatchObject({ envelope: { code: "daemon_unreachable" }, message: expect.stringMatching(/closed the stream before it ended/), @@ -398,7 +425,7 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { const sock = freshSock(); const prevDir = process.env.MCP_INSPECTOR_DAEMON_DIR; const prevToken = process.env.MCP_INSPECTOR_DAEMON_TOKEN; - process.env.MCP_INSPECTOR_DAEMON_DIR = path.dirname(sock); + process.env.MCP_INSPECTOR_DAEMON_DIR = dir!; delete process.env.MCP_INSPECTOR_DAEMON_TOKEN; try { let seenToken: string | undefined; @@ -451,24 +478,32 @@ describe.skipIf(unixDaemonOnly)("streamDaemon + ipc-glue", () => { }); await streamDaemon( {}, - { socketPath: sock, timeoutMs: 2000, onData: () => {} }, + { socketPath: sock, dir, timeoutMs: 2000, onData: () => {} }, ); }); - it("removeStaleDaemonSocket handles absent, dead, and live sockets", async () => { + it("removeStaleDaemonSocket handles absent and live endpoints", async () => { const sock = freshSock(); await removeStaleDaemonSocket(sock); - fs.writeFileSync(sock, ""); - await removeStaleDaemonSocket(sock); - expect(fs.existsSync(sock)).toBe(false); - await listen(sock, () => {}); await expect(removeStaleDaemonSocket(sock)).rejects.toThrow( /already running/, ); }); + // POSIX-only: a dead endpoint is a leftover socket FILE; a named pipe + // vanishes with its server, so Windows has nothing stale to remove. + it.skipIf(isWindows)( + "removeStaleDaemonSocket unlinks a dead socket file", + async () => { + const sock = freshSock(); + fs.writeFileSync(sock, ""); + await removeStaleDaemonSocket(sock); + expect(fs.existsSync(sock)).toBe(false); + }, + ); + it("acceptDaemonConnection rejects invalid request lines", async () => { const sock = freshSock(); const chunks: string[] = []; diff --git a/clients/mcpdo/__tests__/directory-read.test.ts b/clients/mcpdo/__tests__/directory-read.test.ts index a506af7998..adaf547aac 100644 --- a/clients/mcpdo/__tests__/directory-read.test.ts +++ b/clients/mcpdo/__tests__/directory-read.test.ts @@ -21,8 +21,7 @@ import { deleteConfigFile, } from "../../cli/__tests__/helpers/fixtures.js"; import { expectCliSuccess } from "../../cli/__tests__/helpers/assertions.js"; -import { callDaemon } from "../src/daemon/client.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; /** Budget for each mcpdo command, matching the sibling suites' `runMcp` calls. */ const COMMAND_TIMEOUT_MS = 20_000; @@ -54,135 +53,112 @@ function pageOf(result: McpResult): DirectoryPage { } } -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)( - "mcpdo resources/directory/read (#2767)", - () => { - let storageDir: string | undefined; - let configPath: string | undefined; +describe("mcpdo resources/directory/read (#2767)", () => { + let storageDir: string | undefined; + let configPath: string | undefined; - afterEach(async () => { - if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon( - "daemon/stop", - {}, - { socketPath, timeoutMs: 2000 }, - ); - } catch { - // already stopped - } - // `daemon/stop` answers before shutdown finishes, so wait for the - // socket to go before deleting the directory a live daemon holds. - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); - storageDir = undefined; - } - if (configPath) { - deleteConfigFile(configPath); - configPath = undefined; - } - }); + afterEach(async () => { + if (storageDir) { + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); + storageDir = undefined; + } + if (configPath) { + deleteConfigFile(configPath); + configPath = undefined; + } + }); - it( - "reads a directory by positional URI or --uri, and pages with --cursor", - async () => { - const server = createTestServerHttp({ - serverInfo: createTestServerInfo("skills-directory", "1.0.0"), - skills: true, + it( + "reads a directory by positional URI or --uri, and pages with --cursor", + async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo("skills-directory", "1.0.0"), + skills: true, + }); + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { + skills: { type: "streamable-http", url: server.url }, + }, }); - try { - await server.start(); - configPath = createTestConfig({ - mcpServers: { - skills: { type: "streamable-http", url: server.url }, - }, - }); - storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcpdo-dirread-")); - const env = { - MCP_STORAGE_DIR: storageDir, - MCP_INSPECTOR_DAEMON_DIR: storageDir, - }; + storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcpdo-dirread-")); + const env = { + MCP_STORAGE_DIR: storageDir, + MCP_INSPECTOR_DAEMON_DIR: storageDir, + }; - expectCliSuccess( - await runMcp( - ["connect", "skills", "--config", configPath, "--format", "json"], - { env, timeout: COMMAND_TIMEOUT_MS }, - ), - ); + expectCliSuccess( + await runMcp( + ["connect", "skills", "--config", configPath, "--format", "json"], + { env, timeout: COMMAND_TIMEOUT_MS }, + ), + ); - const positional = pageOf( - await runMcp( - [ - "@skills", - "resources/directory/read", - DIRECTORY_URI, - "--format", - "json", - ], - { env, timeout: COMMAND_TIMEOUT_MS }, - ), - ); - expect(positional.resources.length).toBeGreaterThan(0); - expect( - positional.resources.every((r) => - r.uri.startsWith(`${DIRECTORY_URI}/`), - ), - ).toBe(true); - // The result is returned whole, so #2740's cacheable fields reach it. - expect(positional.ttlMs).toBeTypeOf("number"); - expect(positional.cacheScope).toBeTypeOf("string"); + const positional = pageOf( + await runMcp( + [ + "@skills", + "resources/directory/read", + DIRECTORY_URI, + "--format", + "json", + ], + { env, timeout: COMMAND_TIMEOUT_MS }, + ), + ); + expect(positional.resources.length).toBeGreaterThan(0); + expect( + positional.resources.every((r) => + r.uri.startsWith(`${DIRECTORY_URI}/`), + ), + ).toBe(true); + // The result is returned whole, so #2740's cacheable fields reach it. + expect(positional.ttlMs).toBeTypeOf("number"); + expect(positional.cacheScope).toBeTypeOf("string"); - const flag = pageOf( - await runMcp( - [ - "@skills", - "resources/directory/read", - "--uri", - DIRECTORY_URI, - "--format", - "json", - ], - { env, timeout: COMMAND_TIMEOUT_MS }, - ), - ); - expect(flag.resources).toEqual(positional.resources); + const flag = pageOf( + await runMcp( + [ + "@skills", + "resources/directory/read", + "--uri", + DIRECTORY_URI, + "--format", + "json", + ], + { env, timeout: COMMAND_TIMEOUT_MS }, + ), + ); + expect(flag.resources).toEqual(positional.resources); - // The fixture pages its directory, so the first page hands back a - // cursor; passing it back must return a different page. - const cursor = positional.nextCursor; - if (cursor === undefined) { - throw new Error( - "the skills fixture's first page has no nextCursor", - ); - } - const next = pageOf( - await runMcp( - [ - "@skills", - "resources/directory/read", - DIRECTORY_URI, - "--cursor", - cursor, - "--format", - "json", - ], - { env, timeout: COMMAND_TIMEOUT_MS }, - ), - ); - expect(next.resources.length).toBeGreaterThan(0); - expect(next.resources).not.toEqual(positional.resources); - } finally { - await server.stop(); + // The fixture pages its directory, so the first page hands back a + // cursor; passing it back must return a different page. + const cursor = positional.nextCursor; + if (cursor === undefined) { + throw new Error("the skills fixture's first page has no nextCursor"); } - }, - TEST_TIMEOUT_MS, - ); - }, -); + const next = pageOf( + await runMcp( + [ + "@skills", + "resources/directory/read", + DIRECTORY_URI, + "--cursor", + cursor, + "--format", + "json", + ], + { env, timeout: COMMAND_TIMEOUT_MS }, + ), + ); + expect(next.resources.length).toBeGreaterThan(0); + expect(next.resources).not.toEqual(positional.resources); + } finally { + await server.stop(); + } + }, + TEST_TIMEOUT_MS, + ); +}); diff --git a/clients/mcpdo/__tests__/elicitation-client.test.ts b/clients/mcpdo/__tests__/elicitation-client.test.ts index 8cbb552643..73f8a20f7b 100644 --- a/clients/mcpdo/__tests__/elicitation-client.test.ts +++ b/clients/mcpdo/__tests__/elicitation-client.test.ts @@ -8,7 +8,7 @@ import type { ElicitationRequestFrame, ElicitationResponseFrame, } from "../src/daemon/protocol.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { daemonEndpoint } from "./helpers/daemon-endpoint.js"; import { FAKE_DAEMON_TOKEN, answerHelloLine, @@ -21,8 +21,7 @@ import { * response, answered via `onElicitation` (or auto-cancelled without one), * with the connect timeout cleared once the exchange starts. */ -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { +describe("callDaemon elicitation duplex", () => { let dir: string | undefined; let server: net.Server | undefined; const sockets = new Set(); @@ -42,7 +41,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { function freshSock(): string { dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-elicit-client-")); - return path.join(dir, "mcpdod.sock"); + return daemonEndpoint(dir); } async function listen( @@ -56,7 +55,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { onSocket(socket); }); await new Promise((resolve) => server!.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(dir!); } /** The client's opening `hello`, answered before its real request. */ @@ -109,6 +108,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { { method: "tools/call" }, { socketPath: sock, + dir, timeoutMs: 5000, onElicitation: async (frame) => { seenFrames.push(frame); @@ -172,7 +172,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { const result = await callDaemon<{ done: boolean }>( "rpc", { method: "tools/call" }, - { socketPath: sock, timeoutMs: 5000 }, + { socketPath: sock, dir, timeoutMs: 5000 }, ); expect(result).toEqual({ done: true }); @@ -229,7 +229,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { const result = await callDaemon<{ done: boolean }>( "rpc", { method: "tools/call" }, - { socketPath: sock, timeoutMs: 5000, onElicitation }, + { socketPath: sock, dir, timeoutMs: 5000, onElicitation }, ); expect(result).toEqual({ done: true }); }); @@ -271,6 +271,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { { method: "tools/call" }, { socketPath: sock, + dir, timeoutMs: 5000, onElicitation: async () => { throw new Error("prompt blew up"); @@ -291,7 +292,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { const promise = callDaemon( "rpc", { method: "tools/call" }, - { socketPath: sock, timeoutMs: 60_000, signal: ac.signal }, + { socketPath: sock, dir, timeoutMs: 60_000, signal: ac.signal }, ); ac.abort(); await expect(promise).rejects.toThrow("cancelled"); @@ -326,7 +327,7 @@ describe.skipIf(unixDaemonOnly)("callDaemon elicitation duplex", () => { const result = await callDaemon<{ done: boolean }>( "rpc", { method: "tools/call" }, - { socketPath: sock, timeoutMs: 5000 }, + { socketPath: sock, dir, timeoutMs: 5000 }, ); expect(result).toEqual({ done: true }); // Force a client-side 'error' after the call already settled; the diff --git a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts new file mode 100644 index 0000000000..57306a5398 --- /dev/null +++ b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts @@ -0,0 +1,85 @@ +/** + * The daemon endpoint as the test harness sees it, on every platform (#2922). + * + * Production derives the endpoint from the daemon directory: a Unix socket + * file `/mcpdod.sock` on POSIX, a named pipe `\\.\pipe\mcp-conn-` + * on win32 (`getDaemonSocketPath`). A suite that hard-codes the `.sock` path + * binds nothing on Windows (`EACCES`), and one that waits for that file to + * vanish never sees a pipe at all. These helpers keep the suites on the + * production derivation instead: + * + * - {@link daemonEndpoint} β€” where a daemon (or a fake) for `dir` listens. + * - {@link releaseEndpoint} β€” what a test does after closing a fake's server: + * remove the socket file on POSIX; a pipe disappears with its server. + * - {@link stopDaemonIn} β€” teardown for a daemon an mcpdo command spawned. The + * liveness signal is the published `mcpdod.token` and the `mcpdod.lock`, + * which exist on every platform (unlike a socket file) and which the daemon + * removes last on shutdown, after the socket. + * - {@link removeDaemonDir} β€” `rmSync` with retries: a stopping daemon process + * still holds its `mcpdod.log` until it exits, which Windows reports as + * `EBUSY`/`EPERM` rather than letting the directory go. + * + * A pipe has no directory, so a client given only a pipe cannot find the + * token; every `callDaemon` here passes `dir` alongside `socketPath` (see + * `daemonTokenDir`). + */ +import * as fs from "node:fs"; +import { callDaemon } from "../../src/daemon/client.js"; +import { + getDaemonLockPath, + getDaemonSocketPath, + getDaemonTokenPath, +} from "../../src/daemon/paths.js"; + +/** The IPC endpoint a daemon (or a fake) for `dir` listens on. */ +export function daemonEndpoint(dir: string): string { + return getDaemonSocketPath(dir); +} + +/** Remove a closed fake's socket file (POSIX); a named pipe needs nothing. */ +export function releaseEndpoint(endpoint: string): void { + if (process.platform !== "win32") fs.rmSync(endpoint, { force: true }); +} + +/** Whether a daemon for `dir` may still be running. */ +export function daemonMayBeLive(dir: string): boolean { + return ( + fs.existsSync(getDaemonTokenPath(dir)) || + fs.existsSync(getDaemonLockPath(dir)) + ); +} + +/** + * Stop the daemon serving `dir`, if any, and wait for its shutdown to finish. + * `daemon/stop` answers before shutdown completes, so the wait is what keeps + * the caller from deleting a directory a live daemon still holds. + */ +export async function stopDaemonIn( + dir: string, + waitMs: number = 5000, +): Promise { + if (!daemonMayBeLive(dir)) return; + try { + await callDaemon( + "daemon/stop", + {}, + { socketPath: daemonEndpoint(dir), dir, timeoutMs: 2000 }, + ); + } catch { + // already stopped, or a stale token left by a daemon that died + } + const deadline = Date.now() + waitMs; + while (daemonMayBeLive(dir) && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 50)); + } +} + +/** Delete a daemon directory, retrying while an exiting daemon releases it. */ +export function removeDaemonDir(dir: string): void { + fs.rmSync(dir, { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 100, + }); +} diff --git a/clients/mcpdo/__tests__/helpers/fake-daemon.ts b/clients/mcpdo/__tests__/helpers/fake-daemon.ts index 8334086018..57d55535ef 100644 --- a/clients/mcpdo/__tests__/helpers/fake-daemon.ts +++ b/clients/mcpdo/__tests__/helpers/fake-daemon.ts @@ -11,23 +11,23 @@ */ import * as fs from "node:fs"; import type * as net from "node:net"; -import * as path from "node:path"; import { computeDaemonProof } from "../../src/daemon/auth.js"; +import { getDaemonTokenPath } from "../../src/daemon/paths.js"; /** Token the fakes hold, unless a test passes its own. */ export const FAKE_DAEMON_TOKEN = "fake-daemon-token"; /** - * Publish `token` as `mcpdod.token` next to a fake's Unix socket, where a - * client given only `socketPath` looks for it. + * Publish `token` as `mcpdod.token` in the fake's daemon directory, where a + * client passed that `dir` looks for it. Keyed by the directory rather than + * the endpoint: on Windows the endpoint is a named pipe, which has no + * directory to derive (see `helpers/daemon-endpoint.ts`). */ export function publishFakeDaemonToken( - socketPath: string, + dir: string, token: string = FAKE_DAEMON_TOKEN, ): void { - fs.writeFileSync(path.join(path.dirname(socketPath), "mcpdod.token"), token, { - mode: 0o600, - }); + fs.writeFileSync(getDaemonTokenPath(dir), token, { mode: 0o600 }); } /** The proof a genuine daemon holding `token` returns for one hello line. */ diff --git a/clients/mcpdo/__tests__/helpers/platform.ts b/clients/mcpdo/__tests__/helpers/platform.ts index aa97254e8e..8d9a6b28d8 100644 --- a/clients/mcpdo/__tests__/helpers/platform.ts +++ b/clients/mcpdo/__tests__/helpers/platform.ts @@ -1,19 +1,15 @@ /** - * Why some mcpdo suites skip on Windows (#2748), stated once. + * Which mcpdo cases skip on Windows, and why that set is small (#2748, #2922). * - * mcpdo is experimental, and its connection daemon was built and tested - * against a Unix domain socket. Production code already derives a named - * pipe on win32 (`getDaemonSocketPath`), but these suites were written for - * the socket: they bind or poll `/mcpdod.sock` directly (which Windows - * refuses with `EACCES`), wait for that file to vanish before tearing down - * (a pipe never appears there, so a live daemon keeps the temp dir open and - * `rmSync` fails `EBUSY`), and assert POSIX-only premises such as `0700` - * directories and the `sun_path` limit. + * The connection daemon's endpoint is a Unix domain socket on POSIX and a + * named pipe on win32 (`getDaemonSocketPath`). The suites derive the endpoint + * from that function and tear a daemon down through + * `helpers/daemon-endpoint.ts`, so every suite that needs a live daemon runs + * on the `windows` CI job too. * - * Making them pass on Windows means porting the daemon's test harness to - * named pipes. That is real work, so it is not folded into a fix for the gate. - * Until it is done, a suite that needs a live daemon or a socket file is - * skipped on win32 by name at its own site with a pointer here. The POSIX runs - * keep every one of them and the coverage gate they carry. + * What is left behind this flag is behaviour that does not exist on Windows at + * all β€” a socket FILE on disk (stale-file removal, `sun_path` length limits), + * POSIX mode bits (`0700` directories, `0600` files, the exec bit) and + * `getuid`-based ownership checks. Each skip names its reason at its own site. */ -export const unixDaemonOnly = process.platform === "win32"; +export const isWindows = process.platform === "win32"; diff --git a/clients/mcpdo/__tests__/load.test.ts b/clients/mcpdo/__tests__/load.test.ts index 058bd5d29b..d1debc7a54 100644 --- a/clients/mcpdo/__tests__/load.test.ts +++ b/clients/mcpdo/__tests__/load.test.ts @@ -15,7 +15,6 @@ import { deleteConfigFile, } from "../../cli/__tests__/helpers/fixtures.js"; import { expectCliSuccess } from "../../cli/__tests__/helpers/assertions.js"; -import { callDaemon } from "../src/daemon/client.js"; import { resolveLoadFlags } from "../src/connection/load-flags.js"; import { formatLoadSummaryHuman } from "../src/connection/format-human.js"; import { writeRpcOutcome } from "../src/connection/dispatch.js"; @@ -26,7 +25,7 @@ import { } from "@inspector/core/cli/handlers/load-test.js"; import { EXIT_CODES } from "@inspector/core/cli/error-handler.js"; import { PLAIN } from "@inspector/core/cli/style.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; /** * `mcpdo tools/call --load` (#2744): the flag rules, the summary rendering, @@ -994,291 +993,519 @@ const DAEMON_COMMAND_MS = 20_000; */ const DAEMON_TEST_MS = 3 * DAEMON_COMMAND_MS + 5_000; -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)( - "mcpdo tools/call --load", - { timeout: DAEMON_TEST_MS }, - () => { - let configPath: string | undefined; - let storageDir: string | undefined; - - afterEach(async () => { - if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon( - "daemon/stop", - {}, - { socketPath, timeoutMs: 2000 }, - ); - } catch { - // already stopped - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); - storageDir = undefined; - } - if (configPath) { - deleteConfigFile(configPath); - configPath = undefined; - } - }); +describe("mcpdo tools/call --load", { timeout: DAEMON_TEST_MS }, () => { + let configPath: string | undefined; + let storageDir: string | undefined; - function env(): Record { - storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-load-e2e-")); - return { - MCP_STORAGE_DIR: storageDir, - MCP_INSPECTOR_DAEMON_DIR: storageDir, - MCP_ALLOW_DEFAULT_CONNECTION: "1", - }; + afterEach(async () => { + if (storageDir) { + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); + storageDir = undefined; + } + if (configPath) { + deleteConfigFile(configPath); + configPath = undefined; } + }); - it("loads a tool over a live connection and leaves it usable", async () => { - configPath = createSampleTestConfig(); - const e = env(); - expectCliSuccess( - await runMcp(["connect", "test-stdio", "--config", configPath], { - env: e, - timeout: DAEMON_COMMAND_MS, - }), - ); + function env(): Record { + storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-load-e2e-")); + return { + MCP_STORAGE_DIR: storageDir, + MCP_INSPECTOR_DAEMON_DIR: storageDir, + MCP_ALLOW_DEFAULT_CONNECTION: "1", + }; + } - const loaded = await runMcp( - [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-requests", - "12", - "--load-concurrency", - "3", - "--format", - "json", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(loaded); - expect(JSON.parse(loaded.stdout)).toMatchObject({ - tool: "echo", - concurrency: 3, - requestLimit: 12, - requests: 12, - ok: 12, - errors: 0, - errorRate: 0, - }); + it("loads a tool over a live connection and leaves it usable", async () => { + configPath = createSampleTestConfig(); + const e = env(); + expectCliSuccess( + await runMcp(["connect", "test-stdio", "--config", configPath], { + env: e, + timeout: DAEMON_COMMAND_MS, + }), + ); - // The queue slot is released: an ordinary call runs afterwards. - expectCliSuccess( - await runMcp( - ["tools/call", "echo", "message:=after", "--format", "json"], - { - env: e, - timeout: DAEMON_COMMAND_MS, - }, - ), - ); + const loaded = await runMcp( + [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-requests", + "12", + "--load-concurrency", + "3", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expectCliSuccess(loaded); + expect(JSON.parse(loaded.stdout)).toMatchObject({ + tool: "echo", + concurrency: 3, + requestLimit: 12, + requests: 12, + ok: 12, + errors: 0, + errorRate: 0, }); - it("fails before loading when the tool does not exist", async () => { - configPath = createSampleTestConfig(); - const e = env(); - expectCliSuccess( - await runMcp(["connect", "test-stdio", "--config", configPath], { + // The queue slot is released: an ordinary call runs afterwards. + expectCliSuccess( + await runMcp( + ["tools/call", "echo", "message:=after", "--format", "json"], + { env: e, timeout: DAEMON_COMMAND_MS, - }), - ); - const result = await runMcp( - ["tools/call", "no_such_tool", "--load", "--load-requests", "2"], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expect(result.exitCode).toBe(5); - expect(result.stderr).toContain( - "Tool 'no_such_tool' not found on server.", - ); - }); + }, + ), + ); + }); - it("runs a --load-scenario mix over a live connection (#2839)", async () => { - const e = env(); - await connectStdio(e); - const dir = fs.mkdtempSync(path.join(storageDir!, "scenario-")); - const file = path.join(dir, "scenario.json"); - fs.writeFileSync( + it("fails before loading when the tool does not exist", async () => { + configPath = createSampleTestConfig(); + const e = env(); + expectCliSuccess( + await runMcp(["connect", "test-stdio", "--config", configPath], { + env: e, + timeout: DAEMON_COMMAND_MS, + }), + ); + const result = await runMcp( + ["tools/call", "no_such_tool", "--load", "--load-requests", "2"], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expect(result.exitCode).toBe(5); + expect(result.stderr).toContain("Tool 'no_such_tool' not found on server."); + }); + + it("runs a --load-scenario mix over a live connection (#2839)", async () => { + const e = env(); + await connectStdio(e); + const dir = fs.mkdtempSync(path.join(storageDir!, "scenario-")); + const file = path.join(dir, "scenario.json"); + fs.writeFileSync( + file, + JSON.stringify([ + { method: "tools/call", tool: "echo", args: { message: "hi" } }, + { method: "resources/read", uri: "test://env" }, + ]), + ); + const result = await runMcp( + [ + "tools/call", + "--load", + "--load-scenario", file, - JSON.stringify([ - { method: "tools/call", tool: "echo", args: { message: "hi" } }, - { method: "resources/read", uri: "test://env" }, - ]), - ); - const result = await runMcp( - [ - "tools/call", - "--load", - "--load-scenario", - file, - "--load-requests", - "6", - "--format", - "json", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(result); - const summary = JSON.parse(result.stdout) as LoadSummary; - expect(summary).toMatchObject({ requests: 6, ok: 6 }); - expect(summary.steps!.map((s) => [s.name, s.ok])).toEqual([ - ["tools/call echo", 3], - ["resources/read test://env", 3], - ]); + "--load-requests", + "6", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expectCliSuccess(result); + const summary = JSON.parse(result.stdout) as LoadSummary; + expect(summary).toMatchObject({ requests: 6, ok: 6 }); + expect(summary.steps!.map((s) => [s.name, s.ok])).toEqual([ + ["tools/call echo", 3], + ["resources/read test://env", 3], + ]); + }); + + it("sends --load-args-file rows through the daemon (#2840)", async () => { + const e = env(); + await connectStdio(e); + const file = path.join(storageDir!, "args.ndjson"); + // Every second row breaks echo's `message: string` schema, so the + // failure count shows the rows reached the daemon's calls. + fs.writeFileSync(file, '{"message":"ok-{{seq}}"}\n{"message":5}\n'); + const result = await runMcp( + [ + "tools/call", + "echo", + "--load", + "--load-args-file", + file, + "--load-requests", + "4", + "--load-concurrency", + "1", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expectCliSuccess(result); + const summary = JSON.parse(result.stdout) as LoadSummary; + expect(summary.requests).toBe(4); + expect(summary.ok).toBe(2); + expect(summary.toolErrors + summary.errors).toBe(2); + }); + + it("runs --load-steps over a live connection (#2841)", async () => { + const e = env(); + await connectStdio(e); + const result = await runMcp( + [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-steps", + "1,2", + "--load-step-duration", + "0.2", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expectCliSuccess(result); + const summary = JSON.parse(result.stdout) as LoadSummary; + expect(summary.stages!.map((s) => [s.stage, s.level])).toEqual([ + [1, 1], + [2, 2], + ]); + expect(summary.requests).toBeGreaterThan(0); + }); + + it("shows progress on stderr in text mode only (#2838)", async () => { + const e = env(); + await connectStdio(e); + const args = [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-duration", + "0.4", + "--load-progress", + "0.1", + "--load-interval", + "0.2", + ]; + const text = await runMcp(args, { env: e, timeout: DAEMON_COMMAND_MS }); + expectCliSuccess(text); + expect(text.stderr).toMatch(/\[load \d+\.\ds\] \d+ calls/); + const json = await runMcp([...args, "--format", "json"], { + env: e, + timeout: DAEMON_COMMAND_MS, + }); + expectCliSuccess(json); + expect(json.stderr).not.toContain("[load"); + const summary = JSON.parse(json.stdout) as LoadSummary; + expect(summary.intervals!.length).toBeGreaterThanOrEqual(2); + // A backed-up stderr drops the advisory lines rather than queue them. + Object.defineProperty(process.stderr, "writableNeedDrain", { + configurable: true, + get: () => true, }); + try { + const drained = await runMcp(args, { + env: e, + timeout: DAEMON_COMMAND_MS, + }); + expectCliSuccess(drained); + expect(drained.stderr).not.toContain("[load"); + } finally { + delete (process.stderr as { writableNeedDrain?: boolean }) + .writableNeedDrain; + } + }); - it("sends --load-args-file rows through the daemon (#2840)", async () => { - const e = env(); - await connectStdio(e); - const file = path.join(storageDir!, "args.ndjson"); - // Every second row breaks echo's `message: string` schema, so the - // failure count shows the rows reached the daemon's calls. - fs.writeFileSync(file, '{"message":"ok-{{seq}}"}\n{"message":5}\n'); - const result = await runMcp( + async function connectStdio(e: Record): Promise { + configPath = createSampleTestConfig(); + expectCliSuccess( + await runMcp(["connect", "test-stdio", "--config", configPath], { + env: e, + timeout: DAEMON_COMMAND_MS, + }), + ); + } + + it("warns on stderr when a --load-rate run falls short (#2837)", async () => { + const e = env(); + await connectStdio(e); + // No single worker starts 100k calls a second. + const result = await runMcp( + [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-rate", + "100000", + "--load-concurrency", + "1", + "--load-duration", + "0.3", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expectCliSuccess(result); + const summary = JSON.parse(result.stdout); + expect(summary).toMatchObject({ rate: 100000, plannedStarts: 30000 }); + expect(summary.droppedSlots).toBeGreaterThan(0); + expect(result.stderr).toContain("Warning: --load-rate 100000/s"); + }); + + it("prints the summary, then fails a breached threshold with the CLI's code", async () => { + const e = env(); + await connectStdio(e); + // A real call never completes in a microsecond, so this p99 always fails. + const breached = await runMcp( + [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-requests", + "4", + "--load-max-p99", + "0.001", + "--format", + "json", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expect(breached.exitCode).toBe(EXIT_CODES.LOAD_THRESHOLD_EXCEEDED); + expect(JSON.parse(breached.stdout)).toMatchObject({ requests: 4, ok: 4 }); + expect(breached.stderr).toContain("load_p99_exceeded"); + + // A threshold the run meets leaves the exit code alone. + expectCliSuccess( + await runMcp( [ "tools/call", "echo", + "message:=hi", "--load", - "--load-args-file", - file, "--load-requests", "4", - "--load-concurrency", - "1", - "--format", - "json", + "--load-max-error-rate", + "0", ], { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(result); - const summary = JSON.parse(result.stdout) as LoadSummary; - expect(summary.requests).toBe(4); - expect(summary.ok).toBe(2); - expect(summary.toolErrors + summary.errors).toBe(2); + ), + ); + }); + + it("compares against a saved baseline and fails a regression (#2910)", async () => { + const e = env(); + await connectStdio(e); + const run = [ + "tools/call", + "echo", + "message:=hi", + "--load", + "--load-requests", + "4", + "--load-concurrency", + "1", + "--format", + "json", + ]; + const saved = await runMcp(run, { env: e, timeout: DAEMON_COMMAND_MS }); + expectCliSuccess(saved); + const baselinePath = path.join(storageDir!, "baseline.json"); + fs.writeFileSync(baselinePath, saved.stdout); + const compared = await runMcp([...run, "--load-baseline", baselinePath], { + env: e, + timeout: DAEMON_COMMAND_MS, }); + expectCliSuccess(compared); + expect(compared.stderr).toContain(`Baseline ${baselinePath}: p50 `); + + // A baseline no real call can match: microsecond latencies. + const fast = JSON.parse(saved.stdout) as LoadSummary; + fast.latencyMs = { ...fast.latencyMs!, p50: 0.001, p99: 0.001 }; + fs.writeFileSync(baselinePath, JSON.stringify(fast)); + const regressed = await runMcp( + [...run, "--load-baseline", baselinePath, "--load-max-regression", "10%"], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expect(regressed.exitCode).toBe(EXIT_CODES.LOAD_THRESHOLD_EXCEEDED); + expect(JSON.parse(regressed.stdout)).toMatchObject({ requests: 4 }); + expect(regressed.stderr).toContain("load_regression"); + }); + + it("refuses more than one session over stdio", async () => { + const e = env(); + await connectStdio(e); + const result = await runMcp( + [ + "tools/call", + "echo", + "--load", + "--load-requests", + "2", + "--load-connections", + "2", + ], + { env: e, timeout: DAEMON_COMMAND_MS }, + ); + expect(result.exitCode).toBe(EXIT_CODES.USAGE); + expect(result.stderr).toContain( + "--load-connections above 1 requires an HTTP or SSE connection", + ); + }); - it("runs --load-steps over a live connection (#2841)", async () => { + it("spreads a run over extra sessions that never become connections", async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo("load-sessions", "1.0.0"), + tools: [createEchoTool()], + }); + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { remote: { type: "streamable-http", url: server.url } }, + }); const e = env(); - await connectStdio(e); - const result = await runMcp( + expectCliSuccess( + await runMcp(["connect", "remote", "--config", configPath], { + env: e, + timeout: DAEMON_COMMAND_MS, + }), + ); + const loaded = await runMcp( [ "tools/call", "echo", "message:=hi", "--load", - "--load-steps", - "1,2", - "--load-step-duration", - "0.2", + "--load-requests", + "9", + "--load-concurrency", + "3", + "--load-connections", + "3", "--format", "json", ], { env: e, timeout: DAEMON_COMMAND_MS }, ); - expectCliSuccess(result); - const summary = JSON.parse(result.stdout) as LoadSummary; - expect(summary.stages!.map((s) => [s.stage, s.level])).toEqual([ - [1, 1], - [2, 2], - ]); - expect(summary.requests).toBeGreaterThan(0); - }); + expectCliSuccess(loaded); + expect(JSON.parse(loaded.stdout)).toMatchObject({ + requests: 9, + ok: 9, + errors: 0, + connections: { requested: 3, opened: 3, failed: 0 }, + }); - it("shows progress on stderr in text mode only (#2838)", async () => { - const e = env(); - await connectStdio(e); - const args = [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-duration", - "0.4", - "--load-progress", - "0.1", - "--load-interval", - "0.2", - ]; - const text = await runMcp(args, { env: e, timeout: DAEMON_COMMAND_MS }); - expectCliSuccess(text); - expect(text.stderr).toMatch(/\[load \d+\.\ds\] \d+ calls/); - const json = await runMcp([...args, "--format", "json"], { + // The extras belonged to the run: only the connection itself is listed. + const listed = await runMcp(["connections/list", "--format", "json"], { env: e, timeout: DAEMON_COMMAND_MS, }); - expectCliSuccess(json); - expect(json.stderr).not.toContain("[load"); - const summary = JSON.parse(json.stdout) as LoadSummary; - expect(summary.intervals!.length).toBeGreaterThanOrEqual(2); - // A backed-up stderr drops the advisory lines rather than queue them. - Object.defineProperty(process.stderr, "writableNeedDrain", { - configurable: true, - get: () => true, - }); - try { - const drained = await runMcp(args, { - env: e, - timeout: DAEMON_COMMAND_MS, - }); - expectCliSuccess(drained); - expect(drained.stderr).not.toContain("[load"); - } finally { - delete (process.stderr as { writableNeedDrain?: boolean }) - .writableNeedDrain; - } - }); + expectCliSuccess(listed); + expect( + ( + JSON.parse(listed.stdout) as { connections: { name: string }[] } + ).connections.map((c) => c.name), + ).toEqual(["remote"]); + } finally { + await server.stop(); + } + }); - async function connectStdio(e: Record): Promise { - configPath = createSampleTestConfig(); + it("fans a triggered update out to subscribed sessions (#2913)", async () => { + const URI = "test://fanout"; + const subscribed: TestServerContext[] = []; + const server = createTestServerHttp({ + serverInfo: createTestServerInfo("load-fanout", "1.0.0"), + subscriptions: true, + resources: [{ uri: URI, name: "fanout", text: "v" }], + onResourceSubscribe: (_uri, context) => subscribed.push(context), + tools: [ + { + name: "bump", + description: "Updates the resource", + inputSchema: {}, + handler: async () => { + await Promise.all( + subscribed.map((c) => + c.server.server + .sendResourceUpdated({ uri: URI }) + .catch(() => {}), + ), + ); + return { content: [{ type: "text", text: "bumped" }] }; + }, + }, + ], + }); + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { remote: { type: "streamable-http", url: server.url } }, + }); + const e = env(); expectCliSuccess( - await runMcp(["connect", "test-stdio", "--config", configPath], { + await runMcp(["connect", "remote", "--config", configPath], { env: e, timeout: DAEMON_COMMAND_MS, }), ); - } - - it("warns on stderr when a --load-rate run falls short (#2837)", async () => { - const e = env(); - await connectStdio(e); - // No single worker starts 100k calls a second. - const result = await runMcp( + const loaded = await runMcp( [ "tools/call", - "echo", - "message:=hi", + "bump", "--load", - "--load-rate", - "100000", - "--load-concurrency", + "--load-requests", + "2", + "--load-subscribe", + URI, + "--load-subscribers", + "2", + "--load-subscribe-wait", "1", - "--load-duration", - "0.3", "--format", "json", ], { env: e, timeout: DAEMON_COMMAND_MS }, ); - expectCliSuccess(result); - const summary = JSON.parse(result.stdout); - expect(summary).toMatchObject({ rate: 100000, plannedStarts: 30000 }); - expect(summary.droppedSlots).toBeGreaterThan(0); - expect(result.stderr).toContain("Warning: --load-rate 100000/s"); - }); + expectCliSuccess(loaded); + expect(JSON.parse(loaded.stdout)).toMatchObject({ + requests: 2, + ok: 2, + fanout: { + mode: "triggered", + subscribed: 2, + updates: 2, + delivered: 4, + missed: 0, + }, + }); + } finally { + await server.stop(); + } + }); - it("prints the summary, then fails a breached threshold with the CLI's code", async () => { + it("churns sessions that never become connections (#2912)", async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo("load-churn", "1.0.0"), + tools: [createEchoTool()], + }); + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { remote: { type: "streamable-http", url: server.url } }, + }); const e = env(); - await connectStdio(e); - // A real call never completes in a microsecond, so this p99 always fails. - const breached = await runMcp( + expectCliSuccess( + await runMcp(["connect", "remote", "--config", configPath], { + env: e, + timeout: DAEMON_COMMAND_MS, + }), + ); + const loaded = await runMcp( [ "tools/call", "echo", @@ -1286,311 +1513,59 @@ describe.skipIf(unixDaemonOnly)( "--load", "--load-requests", "4", - "--load-max-p99", - "0.001", + "--load-concurrency", + "1", + "--load-churn", "--format", "json", ], { env: e, timeout: DAEMON_COMMAND_MS }, ); - expect(breached.exitCode).toBe(EXIT_CODES.LOAD_THRESHOLD_EXCEEDED); - expect(JSON.parse(breached.stdout)).toMatchObject({ requests: 4, ok: 4 }); - expect(breached.stderr).toContain("load_p99_exceeded"); - - // A threshold the run meets leaves the exit code alone. - expectCliSuccess( - await runMcp( - [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-requests", - "4", - "--load-max-error-rate", - "0", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ), - ); - }); - - it("compares against a saved baseline and fails a regression (#2910)", async () => { - const e = env(); - await connectStdio(e); - const run = [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-requests", - "4", - "--load-concurrency", - "1", - "--format", - "json", - ]; - const saved = await runMcp(run, { env: e, timeout: DAEMON_COMMAND_MS }); - expectCliSuccess(saved); - const baselinePath = path.join(storageDir!, "baseline.json"); - fs.writeFileSync(baselinePath, saved.stdout); - const compared = await runMcp([...run, "--load-baseline", baselinePath], { + expectCliSuccess(loaded); + expect(JSON.parse(loaded.stdout)).toMatchObject({ + requests: 4, + ok: 4, + churn: { callsPerSession: 1, opened: 4, failed: 0 }, + }); + const listed = await runMcp(["connections/list", "--format", "json"], { env: e, timeout: DAEMON_COMMAND_MS, }); - expectCliSuccess(compared); - expect(compared.stderr).toContain(`Baseline ${baselinePath}: p50 `); - - // A baseline no real call can match: microsecond latencies. - const fast = JSON.parse(saved.stdout) as LoadSummary; - fast.latencyMs = { ...fast.latencyMs!, p50: 0.001, p99: 0.001 }; - fs.writeFileSync(baselinePath, JSON.stringify(fast)); - const regressed = await runMcp( - [ - ...run, - "--load-baseline", - baselinePath, - "--load-max-regression", - "10%", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expect(regressed.exitCode).toBe(EXIT_CODES.LOAD_THRESHOLD_EXCEEDED); - expect(JSON.parse(regressed.stdout)).toMatchObject({ requests: 4 }); - expect(regressed.stderr).toContain("load_regression"); - }); - - it("refuses more than one session over stdio", async () => { - const e = env(); - await connectStdio(e); - const result = await runMcp( - [ - "tools/call", - "echo", - "--load", - "--load-requests", - "2", - "--load-connections", - "2", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expect(result.exitCode).toBe(EXIT_CODES.USAGE); - expect(result.stderr).toContain( - "--load-connections above 1 requires an HTTP or SSE connection", - ); - }); - - it("spreads a run over extra sessions that never become connections", async () => { - const server = createTestServerHttp({ - serverInfo: createTestServerInfo("load-sessions", "1.0.0"), - tools: [createEchoTool()], - }); - try { - await server.start(); - configPath = createTestConfig({ - mcpServers: { remote: { type: "streamable-http", url: server.url } }, - }); - const e = env(); - expectCliSuccess( - await runMcp(["connect", "remote", "--config", configPath], { - env: e, - timeout: DAEMON_COMMAND_MS, - }), - ); - const loaded = await runMcp( - [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-requests", - "9", - "--load-concurrency", - "3", - "--load-connections", - "3", - "--format", - "json", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(loaded); - expect(JSON.parse(loaded.stdout)).toMatchObject({ - requests: 9, - ok: 9, - errors: 0, - connections: { requested: 3, opened: 3, failed: 0 }, - }); - - // The extras belonged to the run: only the connection itself is listed. - const listed = await runMcp(["connections/list", "--format", "json"], { - env: e, - timeout: DAEMON_COMMAND_MS, - }); - expectCliSuccess(listed); - expect( - ( - JSON.parse(listed.stdout) as { connections: { name: string }[] } - ).connections.map((c) => c.name), - ).toEqual(["remote"]); - } finally { - await server.stop(); - } - }); - - it("fans a triggered update out to subscribed sessions (#2913)", async () => { - const URI = "test://fanout"; - const subscribed: TestServerContext[] = []; - const server = createTestServerHttp({ - serverInfo: createTestServerInfo("load-fanout", "1.0.0"), - subscriptions: true, - resources: [{ uri: URI, name: "fanout", text: "v" }], - onResourceSubscribe: (_uri, context) => subscribed.push(context), - tools: [ - { - name: "bump", - description: "Updates the resource", - inputSchema: {}, - handler: async () => { - await Promise.all( - subscribed.map((c) => - c.server.server - .sendResourceUpdated({ uri: URI }) - .catch(() => {}), - ), - ); - return { content: [{ type: "text", text: "bumped" }] }; - }, - }, - ], - }); - try { - await server.start(); - configPath = createTestConfig({ - mcpServers: { remote: { type: "streamable-http", url: server.url } }, - }); - const e = env(); - expectCliSuccess( - await runMcp(["connect", "remote", "--config", configPath], { - env: e, - timeout: DAEMON_COMMAND_MS, - }), - ); - const loaded = await runMcp( - [ - "tools/call", - "bump", - "--load", - "--load-requests", - "2", - "--load-subscribe", - URI, - "--load-subscribers", - "2", - "--load-subscribe-wait", - "1", - "--format", - "json", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(loaded); - expect(JSON.parse(loaded.stdout)).toMatchObject({ - requests: 2, - ok: 2, - fanout: { - mode: "triggered", - subscribed: 2, - updates: 2, - delivered: 4, - missed: 0, - }, - }); - } finally { - await server.stop(); - } - }); - - it("churns sessions that never become connections (#2912)", async () => { - const server = createTestServerHttp({ - serverInfo: createTestServerInfo("load-churn", "1.0.0"), - tools: [createEchoTool()], - }); - try { - await server.start(); - configPath = createTestConfig({ - mcpServers: { remote: { type: "streamable-http", url: server.url } }, - }); - const e = env(); - expectCliSuccess( - await runMcp(["connect", "remote", "--config", configPath], { - env: e, - timeout: DAEMON_COMMAND_MS, - }), - ); - const loaded = await runMcp( - [ - "tools/call", - "echo", - "message:=hi", - "--load", - "--load-requests", - "4", - "--load-concurrency", - "1", - "--load-churn", - "--format", - "json", - ], - { env: e, timeout: DAEMON_COMMAND_MS }, - ); - expectCliSuccess(loaded); - expect(JSON.parse(loaded.stdout)).toMatchObject({ - requests: 4, - ok: 4, - churn: { callsPerSession: 1, opened: 4, failed: 0 }, - }); - const listed = await runMcp(["connections/list", "--format", "json"], { - env: e, - timeout: DAEMON_COMMAND_MS, - }); - expect( - ( - JSON.parse(listed.stdout) as { connections: { name: string }[] } - ).connections.map((c) => c.name), - ).toEqual(["remote"]); - } finally { - await server.stop(); - } - }); + expect( + ( + JSON.parse(listed.stdout) as { connections: { name: string }[] } + ).connections.map((c) => c.name), + ).toEqual(["remote"]); + } finally { + await server.stop(); + } + }); - it.each([ - [ - ["tools/call", "echo", "--load-requests", "2"], - "--load-requests requires --load.", - ], - [["tools/call", "--load"], "--load requires a tool name."], - [ - ["tools/call", "echo", "--load", "--load-concurrency", "0"], - "--load-concurrency must be a positive integer.", - ], - [ - ["tools/call", "echo", "--load-max-p99", "10"], - "--load-max-p99 requires --load.", - ], - [ - ["tools/call", "echo", "--load", "--load-max-error-rate", "5"], - "--load-max-error-rate must be a number from 0 to 1.", - ], - [ - ["tools/call", "echo", "--load", "--load-connections", "0"], - "--load-connections must be a positive integer.", - ], - ])("rejects %j before reaching a daemon", async (args, message) => { - const result = await runMcp(args, { env: env() }); - expect(result.exitCode).not.toBe(0); - expect(result.stderr).toContain(message); - }); - }, -); + it.each([ + [ + ["tools/call", "echo", "--load-requests", "2"], + "--load-requests requires --load.", + ], + [["tools/call", "--load"], "--load requires a tool name."], + [ + ["tools/call", "echo", "--load", "--load-concurrency", "0"], + "--load-concurrency must be a positive integer.", + ], + [ + ["tools/call", "echo", "--load-max-p99", "10"], + "--load-max-p99 requires --load.", + ], + [ + ["tools/call", "echo", "--load", "--load-max-error-rate", "5"], + "--load-max-error-rate must be a number from 0 to 1.", + ], + [ + ["tools/call", "echo", "--load", "--load-connections", "0"], + "--load-connections must be a positive integer.", + ], + ])("rejects %j before reaching a daemon", async (args, message) => { + const result = await runMcp(args, { env: env() }); + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain(message); + }); +}); diff --git a/clients/mcpdo/__tests__/mcp-connection.test.ts b/clients/mcpdo/__tests__/mcp-connection.test.ts index bc970ff4f6..9253ab5c68 100644 --- a/clients/mcpdo/__tests__/mcp-connection.test.ts +++ b/clients/mcpdo/__tests__/mcp-connection.test.ts @@ -10,11 +10,9 @@ import { } from "../../cli/__tests__/helpers/fixtures.js"; import { expectCliSuccess } from "../../cli/__tests__/helpers/assertions.js"; import { resolveDaemonScriptPath } from "../src/daemon/ensure.js"; -import { callDaemon } from "../src/daemon/client.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("mcp connection CLI", () => { +describe("mcp connection CLI", () => { let configPath: string | undefined; let storageDir: string | undefined; @@ -25,19 +23,8 @@ describe.skipIf(unixDaemonOnly)("mcp connection CLI", () => { afterEach(async () => { if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon("daemon/stop", {}, { socketPath, timeoutMs: 2000 }); - } catch { - // already stopped - } - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/mcp-coverage.test.ts b/clients/mcpdo/__tests__/mcp-coverage.test.ts index 23f6ad32e2..ba29365793 100644 --- a/clients/mcpdo/__tests__/mcp-coverage.test.ts +++ b/clients/mcpdo/__tests__/mcp-coverage.test.ts @@ -14,8 +14,12 @@ import { expectCliFailure, } from "../../cli/__tests__/helpers/assertions.js"; import { resolveDaemonScriptPath } from "../src/daemon/ensure.js"; -import { callDaemon } from "../src/daemon/client.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { isWindows } from "./helpers/platform.js"; +import { + daemonMayBeLive, + removeDaemonDir, + stopDaemonIn, +} from "./helpers/daemon-endpoint.js"; describe("mcp.ts coverage", () => { let configPath: string | undefined; @@ -27,23 +31,12 @@ describe("mcp.ts coverage", () => { afterEach(async () => { if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon("daemon/stop", {}, { socketPath, timeoutMs: 2000 }); - } catch { - // already stopped - } - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - // Cleared before the removal, so one failed `rmSync` cannot hand the + // Cleared before the removal, so one failed removal cannot hand the // same directory to every later test's teardown as well (#2748). const dir = storageDir; storageDir = undefined; - fs.rmSync(dir, { recursive: true, force: true }); + await stopDaemonIn(dir); + removeDaemonDir(dir); } if (configPath) { deleteConfigFile(configPath); @@ -60,449 +53,437 @@ describe("mcp.ts coverage", () => { }; } - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( - "covers RPC registrations, metadata parse, and --plain", - async () => { - configPath = createSampleTestConfig(); - const e = env(); + it("covers RPC registrations, metadata parse, and --plain", async () => { + configPath = createSampleTestConfig(); + const e = env(); - const connected = await runMcp( - ["connect", "test-stdio", "--config", configPath, "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(connected); - - const withMeta = await runMcp( - [ - "tools/list", - "--metadata", - "client=connection-cov", - "--metadata", - "count=1", - // Object value must JSON.stringify (not String β†’ "[object Object]"). - "--metadata", - 'nested={"a":1}', - "--plain", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(withMeta); + const connected = await runMcp( + ["connect", "test-stdio", "--config", configPath, "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(connected); + + const withMeta = await runMcp( + [ + "tools/list", + "--metadata", + "client=connection-cov", + "--metadata", + "count=1", + // Object value must JSON.stringify (not String β†’ "[object Object]"). + "--metadata", + 'nested={"a":1}', + "--plain", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(withMeta); - const badMeta = await runMcp(["tools/list", "--metadata", "novalue"], { - env: e, - }); - expectCliFailure(badMeta); + const badMeta = await runMcp(["tools/list", "--metadata", "novalue"], { + env: e, + }); + expectCliFailure(badMeta); - const emptyMeta = await runMcp(["tools/list", "--metadata", "k="], { - env: e, - }); - expectCliFailure(emptyMeta); - - const read = await runMcp( - [ - "resources/read", - "demo://resource/static/document/architecture.md", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(read); + const emptyMeta = await runMcp(["tools/list", "--metadata", "k="], { + env: e, + }); + expectCliFailure(emptyMeta); + + const read = await runMcp( + [ + "resources/read", + "demo://resource/static/document/architecture.md", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(read); - // Same Commander action as subscribe (uri positional / --uri); prefer - // unsubscribe so we don't open a long-lived stream in this suite. - const unsub = await runMcp( - ["resources/unsubscribe", "test://env", "--format", "json"], - { env: e, timeout: 20000 }, - ); - // Default test server does not advertise subscriptions. - expectCliFailure(unsub); - expect(unsub.stderr).toMatch(/unsubscribe|Method not found/i); - - const prompt = await runMcp( - [ - "prompts/get", - "simple_prompt", - "--prompt-args", - "unused=1", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(prompt); - - // #2859: prompt argument values reach the server verbatim β€” no JSON - // normalisation (`1.50` β†’ `"1.5"`), and an empty value is legal. - const verbatim = await runMcp( - [ - "prompts/get", - "args_prompt", - "--prompt-args", - "city=1.50", - "state=", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(verbatim); - expect(verbatim.stdout).toContain( - "This is a prompt with arguments: city=1.50, state=", - ); + // Same Commander action as subscribe (uri positional / --uri); prefer + // unsubscribe so we don't open a long-lived stream in this suite. + const unsub = await runMcp( + ["resources/unsubscribe", "test://env", "--format", "json"], + { env: e, timeout: 20000 }, + ); + // Default test server does not advertise subscriptions. + expectCliFailure(unsub); + expect(unsub.stderr).toMatch(/unsubscribe|Method not found/i); + + const prompt = await runMcp( + [ + "prompts/get", + "simple_prompt", + "--prompt-args", + "unused=1", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(prompt); + + // #2859: prompt argument values reach the server verbatim β€” no JSON + // normalisation (`1.50` β†’ `"1.5"`), and an empty value is legal. + const verbatim = await runMcp( + [ + "prompts/get", + "args_prompt", + "--prompt-args", + "city=1.50", + "state=", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(verbatim); + expect(verbatim.stdout).toContain( + "This is a prompt with arguments: city=1.50, state=", + ); - const completeBad = await runMcp( - ["prompts/complete", "--complete-ref-type", "nope"], - { env: e }, - ); - expectCliFailure(completeBad); - - const complete = await runMcp( - [ - "prompts/complete", - "--complete-ref-type", - "ref/prompt", - "--complete-ref", - "simple_prompt", - "--complete-arg-name", - "name", - "--complete-arg-value", - "s", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - // Completion support varies; assert the command ran (not a usage parse error). - expect(complete.stderr).not.toMatch(/complete-ref-type/); - expect([0, 1]).toContain(complete.exitCode); + const completeBad = await runMcp( + ["prompts/complete", "--complete-ref-type", "nope"], + { env: e }, + ); + expectCliFailure(completeBad); + + const complete = await runMcp( + [ + "prompts/complete", + "--complete-ref-type", + "ref/prompt", + "--complete-ref", + "simple_prompt", + "--complete-arg-name", + "name", + "--complete-arg-value", + "s", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + // Completion support varies; assert the command ran (not a usage parse error). + expect(complete.stderr).not.toMatch(/complete-ref-type/); + expect([0, 1]).toContain(complete.exitCode); - const logOk = await runMcp( - ["logging/setLevel", "debug", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(logOk); + const logOk = await runMcp( + ["logging/setLevel", "debug", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(logOk); + + const logBad = await runMcp(["logging/setLevel", "--log-level", "nope"], { + env: e, + }); + expectCliFailure(logBad); - const logBad = await runMcp(["logging/setLevel", "--log-level", "nope"], { + const taskGet = await runMcp( + ["tasks/get", "missing-task", "--format", "json"], + { env: e, - }); - expectCliFailure(logBad); - - const taskGet = await runMcp( - ["tasks/get", "missing-task", "--format", "json"], - { - env: e, - timeout: 20000, - }, - ); - expectCliFailure(taskGet); + timeout: 20000, + }, + ); + expectCliFailure(taskGet); - const taskCancel = await runMcp( - ["tasks/cancel", "--task-id", "missing-task", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliFailure(taskCancel); + const taskCancel = await runMcp( + ["tasks/cancel", "--task-id", "missing-task", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliFailure(taskCancel); - const taskResult = await runMcp( - ["tasks/result", "missing-task", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliFailure(taskResult); + const taskResult = await runMcp( + ["tasks/result", "missing-task", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliFailure(taskResult); - const taskUpdateNoBody = await runMcp( - ["tasks/update", "missing-task", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliFailure(taskUpdateNoBody); - expect(taskUpdateNoBody.stderr).toMatch(/--input-responses/); - - const taskUpdateBadJson = await runMcp( - [ - "tasks/update", - "missing-task", - "--input-responses", - "not-json", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliFailure(taskUpdateBadJson); - expect(taskUpdateBadJson.stderr).toMatch(/--input-responses is invalid/); - - const taskUpdate = await runMcp( - [ - "tasks/update", - "missing-task", - "--input-responses", - '{"req-1":"answer"}', - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliFailure(taskUpdate); + const taskUpdateNoBody = await runMcp( + ["tasks/update", "missing-task", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliFailure(taskUpdateNoBody); + expect(taskUpdateNoBody.stderr).toMatch(/--input-responses/); + + const taskUpdateBadJson = await runMcp( + [ + "tasks/update", + "missing-task", + "--input-responses", + "not-json", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliFailure(taskUpdateBadJson); + expect(taskUpdateBadJson.stderr).toMatch(/--input-responses is invalid/); + + const taskUpdate = await runMcp( + [ + "tasks/update", + "missing-task", + "--input-responses", + '{"req-1":"answer"}', + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliFailure(taskUpdate); - const roots = await runMcp( - ["roots/set", "--roots-json", "[]", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(roots); - - const called = await runMcp( - [ - "tools/call", - "--tool-name", - "echo", - "--tool-arg", - "message=cov", - "--tool-metadata", - "src=test", - "--format", - "json", - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(called); + const roots = await runMcp( + ["roots/set", "--roots-json", "[]", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(roots); + + const called = await runMcp( + [ + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=cov", + "--tool-metadata", + "src=test", + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(called); - const templates = await runMcp( - ["resources/templates/list", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(templates); + const templates = await runMcp( + ["resources/templates/list", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(templates); - const prompts = await runMcp(["prompts/list", "--format", "json"], { - env: e, - timeout: 20000, - }); - expectCliSuccess(prompts); + const prompts = await runMcp(["prompts/list", "--format", "json"], { + env: e, + timeout: 20000, + }); + expectCliSuccess(prompts); - const tasks = await runMcp(["tasks/list", "--format", "json"], { - env: e, - timeout: 20000, - }); - expectCliSuccess(tasks); - expect(JSON.parse(tasks.stdout)).toHaveProperty("tasks"); + const tasks = await runMcp(["tasks/list", "--format", "json"], { + env: e, + timeout: 20000, + }); + expectCliSuccess(tasks); + expect(JSON.parse(tasks.stdout)).toHaveProperty("tasks"); - const rootsList = await runMcp(["roots/list", "--format", "json"], { - env: e, - timeout: 20000, - }); - expectCliSuccess(rootsList); - expect(JSON.parse(rootsList.stdout)).toHaveProperty("roots"); - - const show = await runMcp( - [ - "servers/show", - "test-stdio", - "--config", - configPath, - "--format", - "json", - ], + const rootsList = await runMcp(["roots/list", "--format", "json"], { + env: e, + timeout: 20000, + }); + expectCliSuccess(rootsList); + expect(JSON.parse(rootsList.stdout)).toHaveProperty("roots"); + + const show = await runMcp( + [ + "servers/show", + "test-stdio", + "--config", + configPath, + "--format", + "json", + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(show); + // Entry provenance rides along, mirroring servers/list. + expect(JSON.parse(show.stdout).source).toMatchObject({ kind: "config" }); + + // No name: falls back to the MRU connection's entry (test-stdio is + // connected above and MCP_ALLOW_DEFAULT_CONNECTION opts non-TTY in). + const showMru = await runMcp( + ["servers/show", "--config", configPath, "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(showMru); + expect(JSON.parse(showMru.stdout).name).toBe("test-stdio"); + + // MRU-inferred name missing from the shell's source: the error must + // explain the name came from the MRU connection, not look like a typo. + const otherConfig = createTestConfig({ + mcpServers: { unrelated: { type: "stdio", command: "true" } }, + }); + try { + const crossCatalog = await runMcp( + ["servers/show", "--config", otherConfig], { env: e, timeout: 20000 }, ); - expectCliSuccess(show); - // Entry provenance rides along, mirroring servers/list. - expect(JSON.parse(show.stdout).source).toMatchObject({ kind: "config" }); - - // No name: falls back to the MRU connection's entry (test-stdio is - // connected above and MCP_ALLOW_DEFAULT_CONNECTION opts non-TTY in). - const showMru = await runMcp( - ["servers/show", "--config", configPath, "--format", "json"], - { env: e, timeout: 20000 }, + expectCliFailure(crossCatalog); + expect(crossCatalog.stderr).toMatch( + /most-recently-used connection 'test-stdio' has no entry in config/, ); - expectCliSuccess(showMru); - expect(JSON.parse(showMru.stdout).name).toBe("test-stdio"); + } finally { + deleteConfigFile(otherConfig); + } - // MRU-inferred name missing from the shell's source: the error must - // explain the name came from the MRU connection, not look like a typo. - const otherConfig = createTestConfig({ - mcpServers: { unrelated: { type: "stdio", command: "true" } }, - }); - try { - const crossCatalog = await runMcp( - ["servers/show", "--config", otherConfig], - { env: e, timeout: 20000 }, - ); - expectCliFailure(crossCatalog); - expect(crossCatalog.stderr).toMatch( - /most-recently-used connection 'test-stdio' has no entry in config/, - ); - } finally { - deleteConfigFile(otherConfig); - } - - // Skills support is optional; the default test server may not advertise - // it. Either way, the RPC action itself should run (not a usage error). - const skillsList = await runMcp(["skills/list", "--format", "json"], { - env: e, - timeout: 20000, - }); - expect([0, 1]).toContain(skillsList.exitCode); + // Skills support is optional; the default test server may not advertise + // it. Either way, the RPC action itself should run (not a usage error). + const skillsList = await runMcp(["skills/list", "--format", "json"], { + env: e, + timeout: 20000, + }); + expect([0, 1]).toContain(skillsList.exitCode); - const skillsListVerify = await runMcp( - ["skills/list", "--verify", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expect([0, 1]).toContain(skillsListVerify.exitCode); + const skillsListVerify = await runMcp( + ["skills/list", "--verify", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expect([0, 1]).toContain(skillsListVerify.exitCode); - const skillsGet = await runMcp( - ["skills/get", "test://skill", "--verify", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expect([0, 1]).toContain(skillsGet.exitCode); + const skillsGet = await runMcp( + ["skills/get", "test://skill", "--verify", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expect([0, 1]).toContain(skillsGet.exitCode); - const skillsGetFlagUri = await runMcp( - ["skills/get", "--uri", "test://skill", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expect([0, 1]).toContain(skillsGetFlagUri.exitCode); + const skillsGetFlagUri = await runMcp( + ["skills/get", "--uri", "test://skill", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expect([0, 1]).toContain(skillsGetFlagUri.exitCode); - await runMcp( - ["disconnect", "--connection", "test-stdio", "--format", "json"], - { - env: e, - }, - ); - await runMcp(["daemon", "stop", "--format", "json"], { env: e }); - }, - ); + await runMcp( + ["disconnect", "--connection", "test-stdio", "--format", "json"], + { + env: e, + }, + ); + await runMcp(["daemon", "stop", "--format", "json"], { env: e }); + }); - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( - "covers ad-hoc connect options and servers/list catalog env", - async () => { - configPath = createSampleTestConfig(); - const e = env(); - const { command, args } = getTestMcpServerCommand(); - - const adHoc = await runMcp( - [ - "connect", - "--connection", - "opts", - "--transport", - "stdio", - "--cwd", - process.cwd(), - "-e", - "COV_FLAG=1", - "--connect-timeout", - "15000", - "--era", - "auto", - "--elicit", - "url", - "--format", - "json", - command, - ...args, - ], - { env: e, timeout: 20000 }, - ); - expectCliSuccess(adHoc); + it("covers ad-hoc connect options and servers/list catalog env", async () => { + configPath = createSampleTestConfig(); + const e = env(); + const { command, args } = getTestMcpServerCommand(); + + const adHoc = await runMcp( + [ + "connect", + "--connection", + "opts", + "--transport", + "stdio", + "--cwd", + process.cwd(), + "-e", + "COV_FLAG=1", + "--connect-timeout", + "15000", + "--era", + "auto", + "--elicit", + "url", + "--format", + "json", + command, + ...args, + ], + { env: e, timeout: 20000 }, + ); + expectCliSuccess(adHoc); - // --ema is rejected for ad-hoc targets: EMA needs per-server OAuth - // client id/secret, which only a catalog entry can supply. - const emaAdHoc = await runMcp( - ["connect", "--ema", "--format", "json", command, ...args], - { env: e, timeout: 20000 }, - ); - expectCliFailure(emaAdHoc); - expect(emaAdHoc.stderr).toMatch(/--ema cannot be used with an ad-hoc/); + // --ema is rejected for ad-hoc targets: EMA needs per-server OAuth + // client id/secret, which only a catalog entry can supply. + const emaAdHoc = await runMcp( + ["connect", "--ema", "--format", "json", command, ...args], + { env: e, timeout: 20000 }, + ); + expectCliFailure(emaAdHoc); + expect(emaAdHoc.stderr).toMatch(/--ema cannot be used with an ad-hoc/); - // Invalid --era is rejected before any connection is attempted. - const badEra = await runMcp( - ["connect", "--era", "bogus", "--format", "json", command, ...args], - { env: e, timeout: 20000 }, - ); - expectCliFailure(badEra); - expect(badEra.stderr).toMatch(/Invalid --era/); + // Invalid --era is rejected before any connection is attempted. + const badEra = await runMcp( + ["connect", "--era", "bogus", "--format", "json", command, ...args], + { env: e, timeout: 20000 }, + ); + expectCliFailure(badEra); + expect(badEra.stderr).toMatch(/Invalid --era/); - // Invalid --elicit is rejected before any connection is attempted. - const badElicit = await runMcp( - ["connect", "--elicit", "bogus", "--format", "json", command, ...args], - { env: e, timeout: 20000 }, - ); - expectCliFailure(badElicit); - expect(badElicit.stderr).toMatch(/Invalid --elicit/); + // Invalid --elicit is rejected before any connection is attempted. + const badElicit = await runMcp( + ["connect", "--elicit", "bogus", "--format", "json", command, ...args], + { env: e, timeout: 20000 }, + ); + expectCliFailure(badElicit); + expect(badElicit.stderr).toMatch(/Invalid --elicit/); - await runMcp(["disconnect", "--connection", "opts", "--format", "json"], { - env: e, - }); + await runMcp(["disconnect", "--connection", "opts", "--format", "json"], { + env: e, + }); - // Ad-hoc HTTP with --server-url and no positional rest (empty-rest branch). - const urlOnly = await runMcp( - [ - "connect", - "--connection", - "urlonly", - "--transport", - "http", - "--server-url", - "http://127.0.0.1:9/mcp", - "--header", - "X-Test: 1", - "--connect-timeout", - "100", - "--format", - "json", - ], - { env: e, timeout: 10000 }, - ); - expectCliFailure(urlOnly); - // Unreachable HTTP should classify as exit 4 when the error is network-shaped. - expect([1, 4]).toContain(urlOnly.exitCode); - - const listed = await runMcp(["servers/list", "--format", "json"], { - env: { - ...e, - MCP_CATALOG_PATH: configPath, - }, - }); - expectCliSuccess(listed); + // Ad-hoc HTTP with --server-url and no positional rest (empty-rest branch). + const urlOnly = await runMcp( + [ + "connect", + "--connection", + "urlonly", + "--transport", + "http", + "--server-url", + "http://127.0.0.1:9/mcp", + "--header", + "X-Test: 1", + "--connect-timeout", + "100", + "--format", + "json", + ], + { env: e, timeout: 10000 }, + ); + expectCliFailure(urlOnly); + // Unreachable HTTP should classify as exit 4 when the error is network-shaped. + expect([1, 4]).toContain(urlOnly.exitCode); + + const listed = await runMcp(["servers/list", "--format", "json"], { + env: { + ...e, + MCP_CATALOG_PATH: configPath, + }, + }); + expectCliSuccess(listed); - // Whitespace --config β†’ trim || undefined branch on servers/list. - const emptyConfig = await runMcp( - ["servers/list", "--config", " ", "--format", "json"], - { env: { ...e, MCP_CATALOG_PATH: configPath } }, - ); - expectCliSuccess(emptyConfig); + // Whitespace --config β†’ trim || undefined branch on servers/list. + const emptyConfig = await runMcp( + ["servers/list", "--config", " ", "--format", "json"], + { env: { ...e, MCP_CATALOG_PATH: configPath } }, + ); + expectCliSuccess(emptyConfig); - await runMcp(["daemon", "stop", "--format", "json"], { env: e }); - }, - ); + await runMcp(["daemon", "stop", "--format", "json"], { env: e }); + }); - // Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). - it.skipIf(unixDaemonOnly)( - "treats a single path-like token as ad-hoc stdio, a bare word as a catalog name", - async () => { - configPath = createSampleTestConfig(); - const e = { ...env(), MCP_CATALOG_PATH: configPath }; + it("treats a single path-like token as ad-hoc stdio, a bare word as a catalog name", async () => { + configPath = createSampleTestConfig(); + const e = { ...env(), MCP_CATALOG_PATH: configPath }; - // Bare word: catalog/config lookup β€” fails as a catalog miss, before - // any daemon or spawn work. - const bareWord = await runMcp( - ["connect", "no-such-catalog-entry", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliFailure(bareWord); - expect(bareWord.stderr).toMatch(/not found/i); + // Bare word: catalog/config lookup β€” fails as a catalog miss, before + // any daemon or spawn work. + const bareWord = await runMcp( + ["connect", "no-such-catalog-entry", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliFailure(bareWord); + expect(bareWord.stderr).toMatch(/not found/i); - // Path-like token: ad-hoc stdio target β€” never touches the catalog, so - // the failure is a spawn/connect failure, not a catalog miss. - const pathToken = await runMcp( - ["connect", "./no-such-server-binary", "--format", "json"], - { env: e, timeout: 20000 }, - ); - expectCliFailure(pathToken); - expect(pathToken.stderr).not.toMatch(/not found\. Available servers/); - }, - ); + // Path-like token: ad-hoc stdio target β€” never touches the catalog, so + // the failure is a spawn/connect failure, not a catalog miss. + const pathToken = await runMcp( + ["connect", "./no-such-server-binary", "--format", "json"], + { env: e, timeout: 20000 }, + ); + expectCliFailure(pathToken); + expect(pathToken.stderr).not.toMatch(/not found\. Available servers/); + }); it("bare mcpdo / --help print usage without an ErrorEnvelope", async () => { // Bare invocation: Commander writes help to stderr (help-after-error). @@ -576,8 +557,9 @@ describe("mcp.ts coverage", () => { ); }); - // Windows named pipes have no sun_path limit, so the check is a no-op there. - it.skipIf(process.platform === "win32")( + // POSIX-only: Windows named pipes have no sun_path limit, so the check is a + // no-op there. + it.skipIf(isWindows)( "an over-limit daemon socket path fails connections/list and MRU inference like connect does (#2823)", async () => { configPath = createSampleTestConfig(); @@ -635,7 +617,8 @@ describe("mcp.ts coverage", () => { message: "Daemon is not running.", }); - // Socket must not have been created by status/list. - expect(fs.existsSync(path.join(storageDir!, "mcpdod.sock"))).toBe(false); + // No daemon was started by status/list: it would have published its + // token and lock (the socket file exists on POSIX only). + expect(daemonMayBeLive(storageDir!)).toBe(false); }); }); diff --git a/clients/mcpdo/__tests__/mcp-elicitation.test.ts b/clients/mcpdo/__tests__/mcp-elicitation.test.ts index 401e26353d..9930fd46e1 100644 --- a/clients/mcpdo/__tests__/mcp-elicitation.test.ts +++ b/clients/mcpdo/__tests__/mcp-elicitation.test.ts @@ -9,9 +9,8 @@ import { expectCliFailure, } from "../../cli/__tests__/helpers/assertions.js"; import { resolveDaemonScriptPath } from "../src/daemon/ensure.js"; -import { callDaemon } from "../src/daemon/client.js"; import type { ElicitationPendingInfo } from "../src/daemon/protocol.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; /** * End-to-end non-interactive elicitation: a real daemon, a real composable @@ -20,8 +19,7 @@ import { unixDaemonOnly } from "./helpers/platform.js"; * parked (`elicitationPending`), and `elicitation/respond` resuming the call * to its final result. */ -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("mcp non-interactive elicitation (e2e)", () => { +describe("mcp non-interactive elicitation (e2e)", () => { let storageDir: string | undefined; let configPath: string | undefined; const ttyDescriptors: Array<{ @@ -38,19 +36,8 @@ describe.skipIf(unixDaemonOnly)("mcp non-interactive elicitation (e2e)", () => { if (desc) Object.defineProperty(stream, "isTTY", desc); } if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon("daemon/stop", {}, { socketPath, timeoutMs: 2000 }); - } catch { - // already stopped - } - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/oauth-error-envelope.test.ts b/clients/mcpdo/__tests__/oauth-error-envelope.test.ts index de82b917fa..3f38595d02 100644 --- a/clients/mcpdo/__tests__/oauth-error-envelope.test.ts +++ b/clients/mcpdo/__tests__/oauth-error-envelope.test.ts @@ -6,7 +6,7 @@ import { OAuthError } from "@modelcontextprotocol/client"; import { EXIT_CODES } from "@inspector/core/cli/error-handler.js"; import { DaemonServer } from "../src/daemon/server.js"; import { relayedEnvelope } from "../src/daemon/client.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir } from "./helpers/daemon-endpoint.js"; /** * An OAuth protocol failure's envelope across mcpdo's daemon socket (#2842). @@ -14,8 +14,7 @@ import { unixDaemonOnly } from "./helpers/platform.js"; * tests cover the hop, where the authorization server's `error` / * `error_description` fields would otherwise be dropped. */ -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("mcpdo oauth_error envelope", () => { +describe("mcpdo oauth_error envelope", () => { let dir: string | undefined; let server: DaemonServer | undefined; @@ -26,7 +25,7 @@ describe.skipIf(unixDaemonOnly)("mcpdo oauth_error envelope", () => { server = undefined; } if (dir) { - fs.rmSync(dir, { recursive: true, force: true }); + removeDaemonDir(dir); dir = undefined; } }); diff --git a/clients/mcpdo/__tests__/rate-limit-envelope.test.ts b/clients/mcpdo/__tests__/rate-limit-envelope.test.ts index f28052d978..be61d49ab7 100644 --- a/clients/mcpdo/__tests__/rate-limit-envelope.test.ts +++ b/clients/mcpdo/__tests__/rate-limit-envelope.test.ts @@ -10,7 +10,7 @@ import { EXIT_CODES, } from "@inspector/core/cli/error-handler.js"; import { DaemonServer } from "../src/daemon/server.js"; -import { callDaemon, relayedEnvelope } from "../src/daemon/client.js"; +import { relayedEnvelope } from "../src/daemon/client.js"; import { streamDaemon } from "../src/daemon/stream-client.js"; import { runMcp } from "./helpers/mcp-runner.js"; import { @@ -18,7 +18,11 @@ import { type ThrottlingServer, } from "../../cli/__tests__/helpers/throttling-server.js"; import { expectCliSuccess } from "../../cli/__tests__/helpers/assertions.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { + daemonEndpoint, + removeDaemonDir, + stopDaemonIn, +} from "./helpers/daemon-endpoint.js"; import { onRequest, publishFakeDaemonToken } from "./helpers/fake-daemon.js"; /** @@ -27,8 +31,7 @@ import { onRequest, publishFakeDaemonToken } from "./helpers/fake-daemon.js"; * the daemon socket first, so these tests cover that hop on each side and then * the whole path end to end. */ -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)("mcpdo rate-limited request envelope", () => { +describe("mcpdo rate-limited request envelope", () => { let dir: string | undefined; let server: DaemonServer | undefined; let throttling: ThrottlingServer | undefined; @@ -40,15 +43,8 @@ describe.skipIf(unixDaemonOnly)("mcpdo rate-limited request envelope", () => { server = undefined; } if (dir) { - const socketPath = path.join(dir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon("daemon/stop", {}, { socketPath, timeoutMs: 2000 }); - } catch { - // already stopped - } - } - fs.rmSync(dir, { recursive: true, force: true }); + await stopDaemonIn(dir); + removeDaemonDir(dir); dir = undefined; } await throttling?.stop(); @@ -131,7 +127,8 @@ describe.skipIf(unixDaemonOnly)("mcpdo rate-limited request envelope", () => { it("keeps the relayed fields when a stream's setup is refused", async () => { // `resources/subscribe` runs over `streamDaemon`, whose first frame is an // ordinary daemon response β€” a throttled subscribe must keep its fields. - const sock = path.join(freshDir(), "stream.sock"); + const d = freshDir(); + const sock = daemonEndpoint(d); const refuser = net.createServer((socket) => { socket.on("error", () => {}); onRequest(socket, (buf) => { @@ -154,11 +151,11 @@ describe.skipIf(unixDaemonOnly)("mcpdo rate-limited request envelope", () => { }); }); await new Promise((resolve) => refuser.listen(sock, resolve)); - publishFakeDaemonToken(sock); + publishFakeDaemonToken(d); try { const error = await streamDaemon( {}, - { socketPath: sock, timeoutMs: 2000, onData: () => {} }, + { socketPath: sock, dir: d, timeoutMs: 2000, onData: () => {} }, ).catch((e: unknown) => e); expect(error).toBeInstanceOf(CliExitCodeError); expect((error as CliExitCodeError).exitCode).toBe( diff --git a/clients/mcpdo/__tests__/skills-budget-hint.test.ts b/clients/mcpdo/__tests__/skills-budget-hint.test.ts index 0cd1087448..5a0186ef83 100644 --- a/clients/mcpdo/__tests__/skills-budget-hint.test.ts +++ b/clients/mcpdo/__tests__/skills-budget-hint.test.ts @@ -24,8 +24,7 @@ import { deleteConfigFile, } from "../../cli/__tests__/helpers/fixtures.js"; import { expectCliSuccess } from "../../cli/__tests__/helpers/assertions.js"; -import { callDaemon } from "../src/daemon/client.js"; -import { unixDaemonOnly } from "./helpers/platform.js"; +import { removeDaemonDir, stopDaemonIn } from "./helpers/daemon-endpoint.js"; /** Budget for each mcpdo command, matching the sibling suites' `runMcp` calls. */ const COMMAND_TIMEOUT_MS = 20_000; @@ -51,123 +50,96 @@ function reportsOf(result: McpResult): SkillVerifyReport[] { } } -// Skipped on win32: needs a live daemon / Unix socket (helpers/platform.ts, #2748). -describe.skipIf(unixDaemonOnly)( - "mcpdo skills catalog-budget hint (#2633)", - () => { - let storageDir: string | undefined; - let configPath: string | undefined; +describe("mcpdo skills catalog-budget hint (#2633)", () => { + let storageDir: string | undefined; + let configPath: string | undefined; - afterEach(async () => { - if (storageDir) { - const socketPath = path.join(storageDir, "mcpdod.sock"); - if (fs.existsSync(socketPath)) { - try { - await callDaemon( - "daemon/stop", - {}, - { socketPath, timeoutMs: 2000 }, - ); - } catch { - // already stopped - } - // `daemon/stop` answers before shutdown finishes, so wait for the - // socket to go before deleting the directory a live daemon still - // holds β€” the same wait `mcp-coverage.test.ts` makes. - const deadline = Date.now() + 2000; - while (fs.existsSync(socketPath) && Date.now() < deadline) { - await new Promise((r) => setTimeout(r, 50)); - } - } - fs.rmSync(storageDir, { recursive: true, force: true }); - storageDir = undefined; - } - if (configPath) { - deleteConfigFile(configPath); - configPath = undefined; - } - }); + afterEach(async () => { + if (storageDir) { + await stopDaemonIn(storageDir); + removeDaemonDir(storageDir); + storageDir = undefined; + } + if (configPath) { + deleteConfigFile(configPath); + configPath = undefined; + } + }); - it( - "names mcpdo's remedies, and the suggested command verifies the skipped skill", - async () => { - const server = createTestServerHttp({ - serverInfo: createTestServerInfo("skills-budget", "1.0.0"), - skills: true, - }); - try { - await server.start(); - configPath = createTestConfig({ - mcpServers: { - skills: { - type: "streamable-http", - url: server.url, - skillCatalogMaxSkills: 1, - }, + it( + "names mcpdo's remedies, and the suggested command verifies the skipped skill", + async () => { + const server = createTestServerHttp({ + serverInfo: createTestServerInfo("skills-budget", "1.0.0"), + skills: true, + }); + try { + await server.start(); + configPath = createTestConfig({ + mcpServers: { + skills: { + type: "streamable-http", + url: server.url, + skillCatalogMaxSkills: 1, }, - }); - storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcpdo-budget-")); - const env = { - MCP_STORAGE_DIR: storageDir, - MCP_INSPECTOR_DAEMON_DIR: storageDir, - }; - - expectCliSuccess( - await runMcp( - ["connect", "skills", "--config", configPath, "--format", "json"], - { env, timeout: COMMAND_TIMEOUT_MS }, - ), - ); + }, + }); + storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcpdo-budget-")); + const env = { + MCP_STORAGE_DIR: storageDir, + MCP_INSPECTOR_DAEMON_DIR: storageDir, + }; - const listed = await runMcp( - ["@skills", "skills/list", "--verify", "--format", "json"], + expectCliSuccess( + await runMcp( + ["connect", "skills", "--config", configPath, "--format", "json"], { env, timeout: COMMAND_TIMEOUT_MS }, - ); - const skipped = reportsOf(listed).find((report) => - report.incomplete?.includes("catalog budget"), - ); - if (!skipped) - throw new Error(`no skipped skill in: ${listed.output}`); - // mcpdo's own remedies β€” not the web's settings screen, nor the - // one-shot CLI's flags and `--method` syntax. - expect(skipped.incomplete).toContain( - "raise it with skillCatalogMaxSkills / skillCatalogMaxBytes in the connection's catalog entry", - ); - expect(skipped.incomplete).toContain( - "`mcpdo @ skills/get --verify`", - ); - expect(skipped.incomplete).not.toMatch( - /--method|--skill-catalog-max|Skills settings/, - ); - expect(skipped.incomplete).not.toContain(skipped.uri); + ), + ); - // Run the suggested command back, placeholders filled the way a user - // would fill them, minus the `mcpdo` program name itself. - const command = /`([^`]+)`/.exec(skipped.incomplete ?? "")?.[1] ?? ""; - const argv = command - .split(/\s+/) - .slice(1) - .map((arg) => - arg === "@" - ? "@skills" - : arg === "" - ? skipped.uri - : arg, - ); - const got = await runMcp([...argv, "--format", "json"], { - env, - timeout: COMMAND_TIMEOUT_MS, - }); - const reports = reportsOf(got); - expect(reports).toHaveLength(1); - expect(reports[0].uri).toBe(skipped.uri); - expect(reports[0].incomplete ?? "").not.toMatch(/catalog budget/); - expect(reports[0].files.length).toBeGreaterThan(0); - } finally { - await server.stop(); - } - }, - TEST_TIMEOUT_MS, - ); - }, -); + const listed = await runMcp( + ["@skills", "skills/list", "--verify", "--format", "json"], + { env, timeout: COMMAND_TIMEOUT_MS }, + ); + const skipped = reportsOf(listed).find((report) => + report.incomplete?.includes("catalog budget"), + ); + if (!skipped) throw new Error(`no skipped skill in: ${listed.output}`); + // mcpdo's own remedies β€” not the web's settings screen, nor the + // one-shot CLI's flags and `--method` syntax. + expect(skipped.incomplete).toContain( + "raise it with skillCatalogMaxSkills / skillCatalogMaxBytes in the connection's catalog entry", + ); + expect(skipped.incomplete).toContain( + "`mcpdo @ skills/get --verify`", + ); + expect(skipped.incomplete).not.toMatch( + /--method|--skill-catalog-max|Skills settings/, + ); + expect(skipped.incomplete).not.toContain(skipped.uri); + + // Run the suggested command back, placeholders filled the way a user + // would fill them, minus the `mcpdo` program name itself. + const command = /`([^`]+)`/.exec(skipped.incomplete ?? "")?.[1] ?? ""; + const argv = command + .split(/\s+/) + .slice(1) + .map((arg) => + arg === "@" ? "@skills" : arg === "" ? skipped.uri : arg, + ); + const got = await runMcp([...argv, "--format", "json"], { + env, + timeout: COMMAND_TIMEOUT_MS, + }); + const reports = reportsOf(got); + expect(reports).toHaveLength(1); + expect(reports[0].uri).toBe(skipped.uri); + expect(reports[0].incomplete ?? "").not.toMatch(/catalog budget/); + expect(reports[0].files.length).toBeGreaterThan(0); + } finally { + await server.stop(); + } + }, + TEST_TIMEOUT_MS, + ); +}); diff --git a/clients/mcpdo/vitest.config.ts b/clients/mcpdo/vitest.config.ts index 7dbcac6c61..fc25a99f86 100644 --- a/clients/mcpdo/vitest.config.ts +++ b/clients/mcpdo/vitest.config.ts @@ -33,22 +33,15 @@ export default defineConfig({ // covered modules. Exercised by spawning real processes (daemon spawn in // tests, smoke), which v8 coverage can't observe from the parent. exclude: ["src/mcp-bin.ts", "src/daemon/run.ts"], - // The per-file gate is enforced off Windows only (#2748). On win32 the - // daemon / Unix-socket suites skip (see __tests__/helpers/platform.ts), - // which leaves the daemon modules they cover far below 90 there β€” a - // statement about the skip, not about the code. Coverage is still - // collected and reported on Windows; CI's Linux `coverage` job is the - // authoritative gate, and every POSIX `local:gate` still enforces it. - thresholds: - process.platform === "win32" - ? undefined - : { - perFile: true, - lines: 90, - statements: 90, - functions: 90, - branches: 90, - }, + // Enforced on every platform: the daemon suites run on Windows too, + // against a named pipe (#2922; see __tests__/helpers/daemon-endpoint.ts). + thresholds: { + perFile: true, + lines: 90, + statements: 90, + functions: 90, + branches: 90, + }, }, }, }); From 5666ed01f4973dc1a3e878d844be23828ef9e95d Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 00:09:02 -0500 Subject: [PATCH 2/6] docs: the windows job now runs mcpdo's daemon suites and gate (#2922) Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- docs/quality-gate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/quality-gate.md b/docs/quality-gate.md index 24d004583b..81bd6e7cde 100644 --- a/docs/quality-gate.md +++ b/docs/quality-gate.md @@ -18,7 +18,7 @@ One more CI check runs outside that table: **`.github/workflows/dco.yml`** fails **`.github/workflows/codeql.yml`** runs CodeQL code scanning (`actions` and `javascript-typescript`, default query suite) on every push to and pull request into `main` and `v2/main`, plus a weekly schedule ([#2545](https://github.com/modelcontextprotocol/inspector/issues/2545)). It replaced CodeQL *default* setup, which scans only the default branch, so `v2/main` work went unscanned until the milestone-merge PR, which is where the v2.9.0 ReDoS ([#2540](https://github.com/modelcontextprotocol/inspector/issues/2540)) surfaced. Default setup is a repository setting and must stay **off**: GitHub rejects an advanced-setup upload while it is on. The job holds `security-events: write`, so `verify:action-pins` counts it as credentialed and its actions are SHA-pinned. It has no local-gate counterpart. -**The `windows` job in `main.yml`** ([#2748](https://github.com/modelcontextprotocol/inspector/issues/2748)) is the only CI job on a Windows runner. It runs the three `local:gate` stages that had gone red on an untouched Windows checkout: `test:scripts`, `coverage:web` and `coverage:mcpdo`. Each step runs even when the one before it failed, so one red stage still leaves the other two with a real verdict. It is not a second copy of `build`/`coverage`, so the smokes, Storybook and the other clients' suites stay Linux-only. Cases bound to POSIX semantics are skipped on win32 **by name at the test site**, each with its reason: mode bits, `chmod`-unwritable directories, process-group signals, Linux mount detection, and mcpdo's Unix-socket daemon suites (`clients/mcpdo/__tests__/helpers/platform.ts`). Everything else runs there. Web's per-file coverage gate holds on Windows and is enforced there too. mcpdo's is dropped on win32 in `clients/mcpdo/vitest.config.ts`, because the skipped daemon suites are the ones that cover the daemon modules, so `coverage:mcpdo` on Windows is a test verdict and the Linux `coverage` job stays the authoritative per-file gate. A `local:gate` run on Windows gets the same treatment from the same configs. +**The `windows` job in `main.yml`** ([#2748](https://github.com/modelcontextprotocol/inspector/issues/2748)) is the only CI job on a Windows runner. It runs the three `local:gate` stages that had gone red on an untouched Windows checkout: `test:scripts`, `coverage:web` and `coverage:mcpdo`. Each step runs even when the one before it failed, so one red stage still leaves the other two with a real verdict. It is not a second copy of `build`/`coverage`, so the smokes, Storybook and the other clients' suites stay Linux-only. Cases bound to POSIX semantics are skipped on win32 **by name at the test site**, each with its reason: mode bits, `chmod`-unwritable directories, process-group signals, Linux mount detection, and the few mcpdo cases about a socket FILE or the `sun_path` limit (`clients/mcpdo/__tests__/helpers/platform.ts`). Everything else runs there β€” including mcpdo's daemon suites, which reach the daemon over its named pipe through `clients/mcpdo/__tests__/helpers/daemon-endpoint.ts` ([#2922](https://github.com/modelcontextprotocol/inspector/issues/2922)). Web's and mcpdo's per-file coverage gates both hold on Windows and are enforced there too. A `local:gate` run on Windows gets the same treatment from the same configs. The local gate runs **every check** `main.yml` runs, and is not a mirror. One of its steps has no GitHub CI counterpart: From 4642f28ba80a354c43348ccd11fc4f265bc1be00 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 01:24:07 -0500 Subject: [PATCH 3/6] test(mcpdo): wait for a stopped daemon's process to exit before removing its dir (#2922) The daemon chdir()s into its directory and exits only once its event loop drains, after it has already removed its token and lock. Windows refuses to remove a directory that is a live process's cwd (EBUSY), so teardown now waits on the pid the daemon reports in ping. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .../__tests__/helpers/daemon-endpoint.ts | 55 ++++++++++++++----- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts index 57306a5398..ad9b5b8cf6 100644 --- a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts +++ b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts @@ -11,10 +11,10 @@ * - {@link daemonEndpoint} β€” where a daemon (or a fake) for `dir` listens. * - {@link releaseEndpoint} β€” what a test does after closing a fake's server: * remove the socket file on POSIX; a pipe disappears with its server. - * - {@link stopDaemonIn} β€” teardown for a daemon an mcpdo command spawned. The - * liveness signal is the published `mcpdod.token` and the `mcpdod.lock`, - * which exist on every platform (unlike a socket file) and which the daemon - * removes last on shutdown, after the socket. + * - {@link stopDaemonIn} β€” teardown for a daemon an mcpdo command spawned: + * stop it, then wait for its published `mcpdod.token` and `mcpdod.lock` + * (present on every platform, unlike a socket file) to go AND for its + * process to exit. * - {@link removeDaemonDir} β€” `rmSync` with retries: a stopping daemon process * still holds its `mcpdod.log` until it exits, which Windows reports as * `EBUSY`/`EPERM` rather than letting the directory go. @@ -49,27 +49,52 @@ export function daemonMayBeLive(dir: string): boolean { ); } +/** Whether process `pid` is still running. */ +function processAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + // EPERM: it exists but belongs to someone else β€” still alive. + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + /** - * Stop the daemon serving `dir`, if any, and wait for its shutdown to finish. - * `daemon/stop` answers before shutdown completes, so the wait is what keeps - * the caller from deleting a directory a live daemon still holds. + * Stop the daemon serving `dir`, if any, and wait for its process to exit. + * + * `daemon/stop` answers before shutdown completes, and the daemon removes its + * token and lock before its process actually exits (it exits once its event + * loop drains). On POSIX that gap is harmless. On Windows it is not: the + * daemon `chdir()`s into `dir` (see `daemon/run.ts`), and a directory that is + * some process's cwd cannot be removed (`EBUSY`). So the wait is on the pid + * the daemon reports in `ping`, not only on its files. + * + * The default budget keeps ping + stop + this wait + {@link removeDaemonDir}'s + * retries inside the shared 30s `hookTimeout` (vitest.shared.mts). */ export async function stopDaemonIn( dir: string, - waitMs: number = 5000, + waitMs: number = 15_000, ): Promise { if (!daemonMayBeLive(dir)) return; + const options = { socketPath: daemonEndpoint(dir), dir, timeoutMs: 2000 }; + let pid: number | undefined; + try { + ({ pid } = await callDaemon<{ pid?: number }>("ping", {}, options)); + } catch { + // unreachable: a stale token left by a daemon that died + } try { - await callDaemon( - "daemon/stop", - {}, - { socketPath: daemonEndpoint(dir), dir, timeoutMs: 2000 }, - ); + await callDaemon("daemon/stop", {}, options); } catch { - // already stopped, or a stale token left by a daemon that died + // already stopped } const deadline = Date.now() + waitMs; - while (daemonMayBeLive(dir) && Date.now() < deadline) { + const running = () => + daemonMayBeLive(dir) || + (pid !== undefined && pid !== process.pid && processAlive(pid)); + while (running() && Date.now() < deadline) { await new Promise((r) => setTimeout(r, 50)); } } From 0b1f033bfe51c1dfa42b35c957e15061cfb7e760 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 01:53:54 -0500 Subject: [PATCH 4/6] test(mcpdo): retry removing a daemon dir while a self-stopped daemon drains (#2922) A daemon that stopped on its own (idle reap after disconnect, or a test's daemon stop) has already removed its token, so teardown has no pid to wait on while the process still has the directory as its cwd. Retry the removal on EBUSY/EPERM/ENOTEMPTY for a bounded time instead. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .../mcpdo/__tests__/daemon-coverage.test.ts | 4 +- .../mcpdo/__tests__/daemon-private.test.ts | 2 +- .../mcpdo/__tests__/directory-read.test.ts | 2 +- .../__tests__/helpers/daemon-endpoint.ts | 47 ++++++++++++++----- clients/mcpdo/__tests__/load.test.ts | 2 +- .../mcpdo/__tests__/mcp-connection.test.ts | 2 +- clients/mcpdo/__tests__/mcp-coverage.test.ts | 2 +- .../mcpdo/__tests__/mcp-elicitation.test.ts | 2 +- .../__tests__/oauth-error-envelope.test.ts | 2 +- .../__tests__/rate-limit-envelope.test.ts | 2 +- .../__tests__/skills-budget-hint.test.ts | 2 +- 11 files changed, 47 insertions(+), 22 deletions(-) diff --git a/clients/mcpdo/__tests__/daemon-coverage.test.ts b/clients/mcpdo/__tests__/daemon-coverage.test.ts index 1f50f87383..6e42dfa90d 100644 --- a/clients/mcpdo/__tests__/daemon-coverage.test.ts +++ b/clients/mcpdo/__tests__/daemon-coverage.test.ts @@ -64,7 +64,7 @@ describe("daemon coverage", () => { if (dir) { // Some cases spawn a real daemon process into `dir`. await stopDaemonIn(dir); - removeDaemonDir(dir); + await removeDaemonDir(dir); dir = undefined; } if (configPath) { @@ -1287,7 +1287,7 @@ describe("mcp connection coverage", () => { afterEach(async () => { if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/daemon-private.test.ts b/clients/mcpdo/__tests__/daemon-private.test.ts index 77791f8d40..3c85802475 100644 --- a/clients/mcpdo/__tests__/daemon-private.test.ts +++ b/clients/mcpdo/__tests__/daemon-private.test.ts @@ -124,7 +124,7 @@ describe("private daemon end-to-end", () => { if (dir) { // A daemon `ensureDaemon` spawned is a separate process. await stopDaemonIn(dir); - removeDaemonDir(dir); + await removeDaemonDir(dir); dir = undefined; } }); diff --git a/clients/mcpdo/__tests__/directory-read.test.ts b/clients/mcpdo/__tests__/directory-read.test.ts index adaf547aac..2b39fe6e12 100644 --- a/clients/mcpdo/__tests__/directory-read.test.ts +++ b/clients/mcpdo/__tests__/directory-read.test.ts @@ -60,7 +60,7 @@ describe("mcpdo resources/directory/read (#2767)", () => { afterEach(async () => { if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts index ad9b5b8cf6..1b935157bd 100644 --- a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts +++ b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts @@ -15,8 +15,8 @@ * stop it, then wait for its published `mcpdod.token` and `mcpdod.lock` * (present on every platform, unlike a socket file) to go AND for its * process to exit. - * - {@link removeDaemonDir} β€” `rmSync` with retries: a stopping daemon process - * still holds its `mcpdod.log` until it exits, which Windows reports as + * - {@link removeDaemonDir} β€” `rmSync`, retried while an exiting daemon still + * holds the directory (its cwd, its `mcpdod.log`), which Windows reports as * `EBUSY`/`EPERM` rather than letting the directory go. * * A pipe has no directory, so a client given only a pipe cannot find the @@ -75,7 +75,7 @@ function processAlive(pid: number): boolean { */ export async function stopDaemonIn( dir: string, - waitMs: number = 15_000, + waitMs: number = 8_000, ): Promise { if (!daemonMayBeLive(dir)) return; const options = { socketPath: daemonEndpoint(dir), dir, timeoutMs: 2000 }; @@ -99,12 +99,37 @@ export async function stopDaemonIn( } } -/** Delete a daemon directory, retrying while an exiting daemon releases it. */ -export function removeDaemonDir(dir: string): void { - fs.rmSync(dir, { - recursive: true, - force: true, - maxRetries: 10, - retryDelay: 100, - }); +/** Errors Windows reports while some process still holds the directory. */ +const HELD = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]); + +/** + * Delete a daemon directory, retrying while an exiting daemon releases it. + * + * A daemon that stopped on its own β€” idle self-reap after a `disconnect`, or + * a test's own `daemon stop` β€” has already removed its token and lock, so + * {@link stopDaemonIn} has no pid to wait on, yet its process can still be + * draining its event loop with `dir` as its cwd (Windows: `EBUSY`). Retry + * until it lets go; the budget fits the shared 30s `hookTimeout` together + * with {@link stopDaemonIn}'s. + */ +export async function removeDaemonDir( + dir: string, + waitMs: number = 15_000, +): Promise { + const started = Date.now(); + for (;;) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (!code || !HELD.has(code) || Date.now() - started >= waitMs) { + throw new Error( + `could not remove ${dir} after ${Date.now() - started}ms: ${String(error)}`, + { cause: error }, + ); + } + } + await new Promise((r) => setTimeout(r, 100)); + } } diff --git a/clients/mcpdo/__tests__/load.test.ts b/clients/mcpdo/__tests__/load.test.ts index d1debc7a54..8c9ddc465c 100644 --- a/clients/mcpdo/__tests__/load.test.ts +++ b/clients/mcpdo/__tests__/load.test.ts @@ -1000,7 +1000,7 @@ describe("mcpdo tools/call --load", { timeout: DAEMON_TEST_MS }, () => { afterEach(async () => { if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/mcp-connection.test.ts b/clients/mcpdo/__tests__/mcp-connection.test.ts index 9253ab5c68..2ef3c90993 100644 --- a/clients/mcpdo/__tests__/mcp-connection.test.ts +++ b/clients/mcpdo/__tests__/mcp-connection.test.ts @@ -24,7 +24,7 @@ describe("mcp connection CLI", () => { afterEach(async () => { if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/mcp-coverage.test.ts b/clients/mcpdo/__tests__/mcp-coverage.test.ts index ba29365793..829ef8b2c1 100644 --- a/clients/mcpdo/__tests__/mcp-coverage.test.ts +++ b/clients/mcpdo/__tests__/mcp-coverage.test.ts @@ -36,7 +36,7 @@ describe("mcp.ts coverage", () => { const dir = storageDir; storageDir = undefined; await stopDaemonIn(dir); - removeDaemonDir(dir); + await removeDaemonDir(dir); } if (configPath) { deleteConfigFile(configPath); diff --git a/clients/mcpdo/__tests__/mcp-elicitation.test.ts b/clients/mcpdo/__tests__/mcp-elicitation.test.ts index 9930fd46e1..d0114b2915 100644 --- a/clients/mcpdo/__tests__/mcp-elicitation.test.ts +++ b/clients/mcpdo/__tests__/mcp-elicitation.test.ts @@ -37,7 +37,7 @@ describe("mcp non-interactive elicitation (e2e)", () => { } if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { diff --git a/clients/mcpdo/__tests__/oauth-error-envelope.test.ts b/clients/mcpdo/__tests__/oauth-error-envelope.test.ts index 3f38595d02..662b5849b0 100644 --- a/clients/mcpdo/__tests__/oauth-error-envelope.test.ts +++ b/clients/mcpdo/__tests__/oauth-error-envelope.test.ts @@ -25,7 +25,7 @@ describe("mcpdo oauth_error envelope", () => { server = undefined; } if (dir) { - removeDaemonDir(dir); + await removeDaemonDir(dir); dir = undefined; } }); diff --git a/clients/mcpdo/__tests__/rate-limit-envelope.test.ts b/clients/mcpdo/__tests__/rate-limit-envelope.test.ts index be61d49ab7..eeda2c3fb2 100644 --- a/clients/mcpdo/__tests__/rate-limit-envelope.test.ts +++ b/clients/mcpdo/__tests__/rate-limit-envelope.test.ts @@ -44,7 +44,7 @@ describe("mcpdo rate-limited request envelope", () => { } if (dir) { await stopDaemonIn(dir); - removeDaemonDir(dir); + await removeDaemonDir(dir); dir = undefined; } await throttling?.stop(); diff --git a/clients/mcpdo/__tests__/skills-budget-hint.test.ts b/clients/mcpdo/__tests__/skills-budget-hint.test.ts index 5a0186ef83..3d0caeacf2 100644 --- a/clients/mcpdo/__tests__/skills-budget-hint.test.ts +++ b/clients/mcpdo/__tests__/skills-budget-hint.test.ts @@ -57,7 +57,7 @@ describe("mcpdo skills catalog-budget hint (#2633)", () => { afterEach(async () => { if (storageDir) { await stopDaemonIn(storageDir); - removeDaemonDir(storageDir); + await removeDaemonDir(storageDir); storageDir = undefined; } if (configPath) { From f9dcfc9d17d44d3af12688bddf779427948aa922 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 02:19:54 -0500 Subject: [PATCH 5/6] test(mcpdo): reach each paths.ts platform arm on every host (#2922) With mcpdo's per-file gate enforced on Windows, the POSIX arms of paths.ts (socket-file path, sun_path limits, assertTrustedPrivateRoot's checks) need covering there too. Fake the platform (and stand in getuid where Windows has none) instead of skipping; the sun_path case now runs on every host. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/mcpdo/__tests__/daemon-paths.test.ts | 70 ++++++++++++++++---- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/clients/mcpdo/__tests__/daemon-paths.test.ts b/clients/mcpdo/__tests__/daemon-paths.test.ts index b484ca1083..06b3c40d32 100644 --- a/clients/mcpdo/__tests__/daemon-paths.test.ts +++ b/clients/mcpdo/__tests__/daemon-paths.test.ts @@ -14,6 +14,20 @@ import { import { writeFormattedResult } from "@inspector/core/cli/handlers/format-output.js"; import { isWindows } from "./helpers/platform.js"; +/** Run `fn` with `process.platform` reporting `platform`. */ +function withPlatform(platform: NodeJS.Platform, fn: () => T): T { + const real = Object.getOwnPropertyDescriptor( + process, + "platform", + ) as PropertyDescriptor; + Object.defineProperty(process, "platform", { value: platform }); + try { + return fn(); + } finally { + Object.defineProperty(process, "platform", real); + } +} + describe("daemon paths", () => { const backup: Record = {}; @@ -149,21 +163,53 @@ describe("daemon paths", () => { fs.rmSync(tmp, { recursive: true, force: true }); }); - // POSIX-only: `sun_path` bounds a socket FILE; Windows named pipes have no - // such limit, which the named-pipe case below asserts on every platform. - it.skipIf(isWindows)( - "assertSocketPathWithinLimit rejects paths over the sun_path limit", - () => { - expect(() => - assertSocketPathWithinLimit("/tmp/short/mcpdod.sock"), - ).not.toThrow(); - const long = "/" + "x".repeat(150) + "/mcpdod.sock"; - expect(() => assertSocketPathWithinLimit(long)).toThrow( - /too long for this platform/, - ); + // Each platform arm below is otherwise reached only on its own OS, and the + // per-file coverage gate runs on Linux, macOS and Windows alike (#2922), so + // the platform is faked rather than the case skipped. + it.each([ + ["linux", 107], + ["darwin", 103], + ] as const)( + "assertSocketPathWithinLimit enforces the %s sun_path limit (%i bytes)", + (platform, limit) => { + withPlatform(platform, () => { + const at = "/" + "x".repeat(limit - 1); + expect(Buffer.byteLength(at)).toBe(limit); + expect(() => assertSocketPathWithinLimit(at)).not.toThrow(); + expect(() => assertSocketPathWithinLimit(at + "x")).toThrow( + /too long for this platform/, + ); + }); }, ); + it("derives a socket file inside the daemon dir off Windows", () => { + withPlatform("linux", () => { + expect(getDaemonSocketPath("/some/daemon/dir")).toBe( + path.join("/some/daemon/dir", "mcpdod.sock"), + ); + }); + }); + + it("assertTrustedPrivateRoot rejects a non-directory and accepts a root it owns", () => { + // The checks run wherever `process.getuid` exists; Windows has none, so + // stand in the owner's uid there to reach them on every host. + const realGetuid = process.getuid; + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-conn-trust-")); + try { + const root = path.join(tmp, "root"); + fs.mkdirSync(root, { mode: 0o755 }); + process.getuid = realGetuid ?? (() => fs.lstatSync(root).uid); + const file = path.join(tmp, "not-a-dir"); + fs.writeFileSync(file, ""); + expect(() => assertTrustedPrivateRoot(file)).toThrow(/not a directory/); + expect(() => assertTrustedPrivateRoot(root)).not.toThrow(); + } finally { + process.getuid = realGetuid; + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + it("uses a deterministic named-pipe path on Windows with no sun_path limit", () => { const realPlatform = Object.getOwnPropertyDescriptor( process, From 2090e96dd6df3cf74f4e2d97fe0998d6df5b387a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 12 Nov 2026 02:48:02 -0500 Subject: [PATCH 6/6] test(mcpdo): skip the shutdown wait when nothing answers ping (#2922) A closed fake server leaves its published token behind, so stopDaemonIn waited out its full budget in every fake-server teardown. A daemon answers ping even while stopping, so an unanswered ping means there is no shutdown to wait for; a process still draining is removeDaemonDir's. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- clients/mcpdo/__tests__/helpers/daemon-endpoint.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts index 1b935157bd..573869d3d1 100644 --- a/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts +++ b/clients/mcpdo/__tests__/helpers/daemon-endpoint.ts @@ -83,7 +83,10 @@ export async function stopDaemonIn( try { ({ pid } = await callDaemon<{ pid?: number }>("ping", {}, options)); } catch { - // unreachable: a stale token left by a daemon that died + // Nothing answers (a daemon answers `ping` even while stopping): the + // token is stale β€” a closed fake's, or a dead daemon's β€” and there is no + // shutdown to wait for. A process still draining is removeDaemonDir's. + return; } try { await callDaemon("daemon/stop", {}, options);