diff --git a/src/mcpDoctor.ts b/src/mcpDoctor.ts index 62a5882807..d7bf1e366f 100644 --- a/src/mcpDoctor.ts +++ b/src/mcpDoctor.ts @@ -1,6 +1,7 @@ import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { Formatter, Help, z } from "incur"; +import { performance } from "node:perf_hooks"; import { resolve } from "node:path"; import { CATALOG_IDENTITY } from "./catalogIdentity.js"; @@ -74,14 +75,15 @@ interface McpDoctorCheck { /** Diagnose the actual production stdio adapter with one startup-bounded SDK session. */ export const runProductionMcpDoctor = async (options: McpDoctorOptions) => { const deadline = - Date.now() + (options.deadlineMs ?? MCP_STARTUP_POLICY.doctorDeadlineMs); + performance.now() + + (options.deadlineMs ?? MCP_STARTUP_POLICY.doctorDeadlineMs); const controller = new AbortController(); const timer = setTimeout( () => controller.abort(new Error("Production MCP doctor deadline expired")), - Math.max(1, deadline - Date.now()), + Math.max(1, Math.ceil(deadline - performance.now())), ); const stderr = captureStderr(); - const transport = new TrackedStdioClientTransport({ + const transport = new OwnedStdioClientTransport({ command: options.command, args: [...options.args], cwd: options.cwd, @@ -90,21 +92,20 @@ export const runProductionMcpDoctor = async (options: McpDoctorOptions) => { }); transport.stderr?.on("data", stderr.append); const client = new Client({ name: "rea-mcp-doctor", version: "1" }); + let result: ProductionMcpDoctorResult; + let cleanupFailure: { readonly cause: unknown } | undefined; try { await client.connect( transport, requestOptions(deadline, controller.signal), ); - return { - ...(await inspectProductionMcpSession( - client, - deadline, - controller.signal, - )), - diagnostics: stderr.value(), - }; + result = await inspectProductionMcpSession( + client, + deadline, + controller.signal, + ); } catch (cause: unknown) { - return { + result = { healthy: false, adapter: "production-stdio" as const, server: null, @@ -118,14 +119,71 @@ export const runProductionMcpDoctor = async (options: McpDoctorOptions) => { detail: errorMessage(cause), }, ], - diagnostics: stderr.value(), }; } finally { clearTimeout(timer); - await closeMcpChild(client, transport); + try { + await client.close(); + } catch (cause: unknown) { + cleanupFailure = { cause }; + } + try { + // An interrupted connect can clear the client's transport reference + // before its startup child is closed. The transport retains that child. + await transport.close(); + } catch (cause: unknown) { + cleanupFailure = { + cause: + cleanupFailure === undefined || cleanupFailure.cause === cause + ? cause + : new AggregateError( + [cleanupFailure.cause, cause], + `MCP client and transport cleanup failed: ${errorMessage(cleanupFailure.cause)}; ${errorMessage(cause)}`, + ), + }; + } } + if (cleanupFailure !== undefined) + result = { + ...result, + healthy: false, + checks: [ + ...result.checks, + { + name: "cleanup", + ok: false, + detail: `MCP stdio cleanup failed: ${errorMessage(cleanupFailure.cause)}`, + }, + ], + }; + return { ...result, diagnostics: stderr.value() }; }; +type ProductionMcpDoctorResult = + | Awaited> + | { + readonly healthy: false; + readonly adapter: "production-stdio"; + readonly server: null; + readonly inventory: null; + readonly identity: null; + readonly request_flow: { + readonly tool: "binary_session"; + readonly ok: false; + }; + readonly checks: readonly McpDoctorCheck[]; + }; + +/** Join SDK-triggered and caller-triggered close calls for the same child. */ +class OwnedStdioClientTransport extends StdioClientTransport { + #closePromise: Promise | undefined; + + override close(): Promise { + this.#closePromise ??= super.close(); + return this.#closePromise; + } +} + const inspectProductionMcpSession = async ( client: Client, deadline: number, @@ -385,7 +443,7 @@ const isOutputFormat = (value: string | undefined): value is OutputFormat => const requestOptions = (deadline: number, signal: AbortSignal) => ({ signal, - timeout: Math.max(1, deadline - Date.now()), + timeout: Math.max(1, Math.ceil(deadline - performance.now())), }); const targetFreeEnvironment = ( @@ -428,61 +486,3 @@ const isRecord = (value: unknown): value is Readonly> => const errorMessage = (cause: unknown): string => cause instanceof Error ? cause.message : String(cause); - -class TrackedStdioClientTransport extends StdioClientTransport { - spawnedPid: number | null = null; - - override async start(): Promise { - await super.start(); - this.spawnedPid = this.pid; - } -} - -const closeMcpChild = async ( - client: Client, - transport: TrackedStdioClientTransport, -): Promise => { - const pid = transport.spawnedPid ?? transport.pid; - await Promise.allSettled([client.close()]); - await Promise.allSettled([transport.close()]); - if (pid === null || !(await processExists(pid))) return; - await terminateProcess(pid, "SIGTERM"); - if (await waitForProcessExit(pid, 1_000)) return; - await terminateProcess(pid, "SIGKILL"); - await waitForProcessExit(pid, 1_000); -}; - -const terminateProcess = async ( - pid: number, - signal: NodeJS.Signals, -): Promise => { - try { - process.kill(pid, signal); - } catch (cause: unknown) { - if (!(cause instanceof Error && "code" in cause && cause.code === "ESRCH")) - throw cause; - } -}; - -const waitForProcessExit = async ( - pid: number, - timeoutMs: number, -): Promise => { - const deadline = Date.now() + timeoutMs; - do { - if (!(await processExists(pid))) return true; - await new Promise((resolve) => setTimeout(resolve, 20)); - } while (Date.now() < deadline); - return !(await processExists(pid)); -}; - -const processExists = async (pid: number): Promise => { - try { - process.kill(pid, 0); - return true; - } catch (cause: unknown) { - if (cause instanceof Error && "code" in cause && cause.code === "ESRCH") - return false; - throw cause; - } -};