From ea923287751043c7acfaa35a0acd10fe32f785d6 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Sat, 10 Oct 2026 14:33:57 +0800 Subject: [PATCH] fix(mcp): validate doctor probe against its discovered output contract (#1461) Validate the target-free probe against the named definition discovered from the complete production catalog. Preserve initialized server and inventory observations when the SDK rejects the probe, and retain the schema failure reason, absolute deadline and owned-child cleanup. The negative stdio replay regression fails on original main and passes the fix; actual production-boundary fault injection confirms rejection. Nine doctor/help tests, serial full static checks and real small-target CLI/MCP parity pass. Successful doctor reports remain byte-identical. No performance or model-token reduction is claimed. Closes #1454. Refs #1065, #723, #1433. --- src/mcpDoctor.ts | 26 +++++++--- tests/boundary/mcp/mcpDoctor.test.ts | 77 +++++++++++++++++++++++++++- tests/fixtures/mcpDoctorReply.mjs | 48 +++++++++++++++++ 3 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/mcpDoctorReply.mjs diff --git a/src/mcpDoctor.ts b/src/mcpDoctor.ts index 0f5de39fce..62a5882807 100644 --- a/src/mcpDoctor.ts +++ b/src/mcpDoctor.ts @@ -6,6 +6,7 @@ import { resolve } from "node:path"; import { CATALOG_IDENTITY } from "./catalogIdentity.js"; import { PROMPT_CONTRACTS } from "./contracts/promptContracts.js"; import { TOOL_CONTRACTS } from "./contracts/toolContracts.js"; +import { err, ok } from "./domain/result.js"; import { PRODUCT_IDENTITY } from "./identity.js"; import { MCP_STARTUP_POLICY } from "./mcpStartupPolicy.js"; @@ -131,11 +132,22 @@ const inspectProductionMcpSession = async ( signal: AbortSignal, ) => { const request = requestOptions(deadline, signal); - const [tools, prompts, requestFlow] = await Promise.all([ + const [tools, prompts] = await Promise.all([ client.listTools(undefined, request), client.listPrompts(undefined, request), - client.callTool({ name: "binary_session", arguments: {} }, request), ]); + const sessionTool = tools.tools.find(({ name }) => name === "binary_session"); + const requestFlow = await client + .callTool( + { name: "binary_session", arguments: {} }, + { + ...requestOptions(deadline, signal), + ...(sessionTool === undefined ? {} : { toolDefinition: sessionTool }), + }, + ) + .then(ok, (cause: unknown) => + err({ kind: "request-flow" as const, detail: errorMessage(cause) }), + ); const inventory = { tools: compareInventory( TOOL_CONTRACTS.map(({ name }) => name), @@ -147,7 +159,7 @@ const inspectProductionMcpSession = async ( ), }; const observedIdentity = parseIdentityToolResult( - requestFlow.structuredContent, + requestFlow.ok ? requestFlow.value.structuredContent : undefined, ); const serverVersion = client.getServerVersion(); const protocolVersion = client.getNegotiatedProtocolVersion(); @@ -173,8 +185,10 @@ const inspectProductionMcpSession = async ( }, { name: "request-flow", - ok: requestFlow.isError !== true, - detail: "binary_session target-free request", + ok: requestFlow.ok && requestFlow.value.isError !== true, + detail: requestFlow.ok + ? "binary_session target-free request" + : requestFlow.error.detail, }, ]; return { @@ -194,7 +208,7 @@ const inspectProductionMcpSession = async ( }, request_flow: { tool: "binary_session", - ok: requestFlow.isError !== true, + ok: requestFlow.ok && requestFlow.value.isError !== true, }, checks, }; diff --git a/tests/boundary/mcp/mcpDoctor.test.ts b/tests/boundary/mcp/mcpDoctor.test.ts index 9858e4ff2e..e9b68b36f5 100644 --- a/tests/boundary/mcp/mcpDoctor.test.ts +++ b/tests/boundary/mcp/mcpDoctor.test.ts @@ -1,4 +1,6 @@ -import { readFile } from "node:fs/promises"; +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { readFile, writeFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -43,6 +45,79 @@ describe("production MCP doctor", () => { ); }, 30_000); + it("rejects a schema-invalid probe while retaining the discovered inventory", async () => { + const root = await createTestTempDirectory("rea-mcp-doctor-reply-"); + const snapshotPath = join(root, "reply.json"); + const environment = { PATH: process.env.PATH ?? "" }; + const client = new Client({ name: "doctor-reply-capture", version: "1" }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [resolve("scripts/rea.mjs"), "mcp"], + cwd: process.cwd(), + env: environment, + stderr: "pipe", + }); + transport.stderr?.on("data", () => undefined); + try { + await client.connect(transport); + const tools = await client.listTools(); + const prompts = await client.listPrompts(); + const toolDefinition = tools.tools.find( + ({ name }) => name === "binary_session", + ); + if (toolDefinition === undefined) + throw new Error("Production binary_session contract is missing"); + const reply = await client.callTool( + { name: "binary_session", arguments: {} }, + { toolDefinition }, + ); + await writeFile( + snapshotPath, + JSON.stringify({ + serverInfo: client.getServerVersion(), + protocolVersion: client.getNegotiatedProtocolVersion(), + tools, + prompts, + reply, + }), + ); + } finally { + await client.close(); + await transport.close(); + } + // Replay real production metadata through a synthetic stdio producer. + for (const mode of ["valid", "invalid"]) { + const result = await runProductionMcpDoctor({ + command: process.execPath, + args: [ + resolve("tests/fixtures/mcpDoctorReply.mjs"), + snapshotPath, + mode, + ], + cwd: process.cwd(), + environment, + }); + expect(result.healthy).toBe(mode === "valid"); + expect(result.inventory?.tools.observed).toBe( + CATALOG_IDENTITY.counts.mcp_tools, + ); + expect(result.inventory?.prompts.observed).toBe( + CATALOG_IDENTITY.counts.mcp_prompts, + ); + expect(result.checks).toContainEqual({ + name: "initialize", + ok: true, + detail: expect.any(String), + }); + if (mode === "invalid") + expect(result.checks).toContainEqual({ + name: "request-flow", + ok: false, + detail: expect.stringMatching(/output schema/iu), + }); + } + }, 30_000); + it("reports exact missing, unexpected, and duplicate inventory names", () => { expect(compareInventory(["a", "b"], ["b", "c", "c"])).toEqual({ expected: 2, diff --git a/tests/fixtures/mcpDoctorReply.mjs b/tests/fixtures/mcpDoctorReply.mjs new file mode 100644 index 0000000000..14dad1240e --- /dev/null +++ b/tests/fixtures/mcpDoctorReply.mjs @@ -0,0 +1,48 @@ +import { readFile } from "node:fs/promises"; +import { createInterface } from "node:readline"; + +const snapshot = JSON.parse(await readFile(process.argv[2], "utf8")); +const reply = structuredClone(snapshot.reply); +if (process.argv[3] === "invalid") { + reply.structuredContent.schema_violation_control = true; + reply.content = reply.content.map((item) => + item.type === "text" + ? { ...item, text: JSON.stringify(reply.structuredContent) } + : item, + ); +} + +const input = createInterface({ input: process.stdin }); +input.on("line", (line) => { + const request = JSON.parse(line); + if (request.id === undefined) return; + let result; + switch (request.method) { + case "initialize": + result = { + protocolVersion: snapshot.protocolVersion, + capabilities: { tools: {}, prompts: {} }, + serverInfo: snapshot.serverInfo, + }; + break; + case "tools/list": + result = snapshot.tools; + break; + case "prompts/list": + result = snapshot.prompts; + break; + case "tools/call": + if (request.params.name === "binary_session") result = reply; + break; + case "ping": + result = {}; + break; + } + const response = + result === undefined + ? { error: { code: -32601, message: "Unknown fixture method or tool" } } + : { result }; + process.stdout.write( + `${JSON.stringify({ jsonrpc: "2.0", id: request.id, ...response })}\n`, + ); +});