mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
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.
This commit is contained in:
+20
-6
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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,
|
||||
|
||||
Vendored
+48
@@ -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`,
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user