From 4eaf3a117b3f4863a994b9be743ec5bc1501e16c Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:31:02 +0800 Subject: [PATCH] fix(mcp): retain protocol framing for oversized tool errors (#5371) Preserve the SDK resultType when bounded error recovery replaces an oversized response. Verify both revisions through the public serving entry, retained Evidence and same-connection recovery. Refs #5273 Co-authored-by: akushonkamen --- docs/mcp-contracts.md | 11 +- src/server/EvidenceMcpServer.test.ts | 230 ++++++++++++++++----------- src/server/EvidenceMcpServer.ts | 10 +- 3 files changed, 152 insertions(+), 99 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 912d531ea2..409d7d0aa7 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -101,8 +101,9 @@ unresolved relationship reads. Cancellation remains a tool error. REA prepares complete MCP results within the pinned stdio client's 10 MiB receive-buffer budget, including both text and structured representations and -room for the JSON-RPC envelope. Tool errors carry only their text -representation and `isError` flag, so only those count against the budget. Response budget settings are captured at startup; +room for the JSON-RPC envelope. Tool errors omit `structuredContent` from the +wire; their text, `isError`, provenance marker and negotiated protocol framing +count toward the wire budget. Response budget settings are captured at startup; restart or recreate the server to apply changes. If a result cannot fit, REA returns `resource_constraint` with `details.resource: "transport"` before constructing a document-sized string. Analysis Evidence remains complete in the current @@ -112,6 +113,12 @@ that reference for a summary, one section/module, or a stable page, or call `export_evidence_bundle` with a destination path. Complete bundle exports stream canonical JSON into an atomically published file. +On protocol revision `2026-07-28`, completed replies retain the SDK's +`resultType: "complete"`, including the replacement for an oversized error. +This marks completion of the MCP exchange; Evidence coverage, partial facts +and unknowns retain their separate meaning. Legacy replies keep their existing +framing. + For oversized native function Evidence, `details.reported_limits.continuation` contains a ready-to-call `inspect_analysis_view` request for the retained pseudocode's first page. Use its `name` and `arguments` directly, then follow diff --git a/src/server/EvidenceMcpServer.test.ts b/src/server/EvidenceMcpServer.test.ts index ce3e7a2746..18ee8d117e 100644 --- a/src/server/EvidenceMcpServer.test.ts +++ b/src/server/EvidenceMcpServer.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { + isJSONRPCResultResponse, + type JSONRPCResultResponse, +} from "@modelcontextprotocol/server"; +import { serveStdio } from "@modelcontextprotocol/server/stdio"; import { z } from "zod"; import { InvestigationRecords } from "../application/investigation/InvestigationRecords.js"; @@ -43,104 +48,137 @@ describe("MCP request bookkeeping", () => { }); describe("SDK validation error delivery", () => { - it("bounds oversized errors, retains their source, and preserves fitting and valid calls", async () => { - const delivery = new ToolResultDelivery(4096); - const records = new InvestigationRecords(); - const server = new EvidenceMcpServer( - { name: "sdk-validation-budget-test", version: "1" }, - { capabilities: {} }, - (evidence) => records.recordEvidence(evidence), - delivery, - ); - server.registerTool( - "procedure_pseudo_code", - { inputSchema: z.strictObject({ required: z.string() }) }, - async () => ({ content: [{ type: "text", text: "valid call" }] }), - ); - const client = new Client({ - name: "sdk-validation-budget-client", - version: "1", - }); - const [clientTransport, serverTransport] = - InMemoryTransport.createLinkedPair(); - const oversizedKey = `unexpected_${"x".repeat(6000)}`; - try { - await server.connect(serverTransport); - await client.connect(clientTransport); - - const oversized = await client.callTool({ - name: "procedure_pseudo_code", - arguments: { required: "present", [oversizedKey]: true }, - }); - expect(oversized.isError).toBe(true); - expect(oversized.structuredContent).toBeUndefined(); - expect(oversized._meta).toEqual({ - provenance: { untrusted_content: true, source: "analysis_target" }, - }); - expect(Buffer.byteLength(JSON.stringify(oversized))).toBeLessThan(4096); - const oversizedError = z - .object({ - error: z.object({ - code: z.string(), - details: z.object({ - reported_limits: z.record(z.string(), z.unknown()), - }), - }), - }) - .parse(JSON.parse(firstText(oversized) ?? "null")); - expect(oversizedError.error.code).toBe("resource_constraint"); - const reportedLimits = oversizedError.error.details.reported_limits; - expect(reportedLimits).toMatchObject({ - evidence_reference: { - kind: "retained-evidence", - evidence_id: expect.any(String), - }, - diagnostic_truncated: true, - }); - expect(reportedLimits).not.toHaveProperty("retention"); - const bundle = records.exportEvidenceBundle(); - expect(bundle.records).toHaveLength(1); - expect(bundle.records[0]?.operation).toBe("mcp_tool_error"); - expect(bundle.records[0]?.raw_result).toMatchObject({ - isError: true, - content: [ - { type: "text", text: expect.stringContaining(oversizedKey) }, - ], - }); - const evidenceReference = z - .object({ evidence_id: z.string() }) - .parse(reportedLimits.evidence_reference); - expect(bundle.records[0]?.evidence_id).toBe( - evidenceReference.evidence_id, + it.each(["2025-11-25", "2026-07-28"])( + "bounds and retains errors through the public stdio entry on %s", + async (revision) => { + const delivery = new ToolResultDelivery(4096); + const records = new InvestigationRecords(); + const server = new EvidenceMcpServer( + { name: "sdk-validation-budget-test", version: "1" }, + { capabilities: {} }, + (evidence) => records.recordEvidence(evidence), + delivery, ); + server.registerTool( + "procedure_pseudo_code", + { inputSchema: z.strictObject({ required: z.string() }) }, + async () => ({ content: [{ type: "text", text: "valid call" }] }), + ); + const client = new Client( + { + name: "sdk-validation-budget-client", + version: "1", + }, + revision === "2026-07-28" + ? { versionNegotiation: { mode: { pin: revision } } } + : { supportedProtocolVersions: [revision] }, + ); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const oversizedKey = `unexpected_${"x".repeat(6000)}`; + const serving = serveStdio(() => server, { transport: serverTransport }); + const replies: JSONRPCResultResponse[] = []; + try { + await client.connect(clientTransport); + expect(client.getNegotiatedProtocolVersion()).toBe(revision); + const receive = clientTransport.onmessage; + clientTransport.onmessage = (message) => { + if (isJSONRPCResultResponse(message)) replies.push(message); + receive?.(message); + }; - const fitting = await client.callTool({ - name: "procedure_pseudo_code", - arguments: { unexpected: true }, - }); - expect(fitting.isError).toBe(true); - expect(firstText(fitting)).toContain("Input validation error"); + // The normal reply and SDK validation error already negotiate and decode + // correctly before oversized recovery replaces an SDK-encoded envelope. + const valid = await client.callTool({ + name: "procedure_pseudo_code", + arguments: { required: "present" }, + }); + expect(valid.content).toContainEqual({ + type: "text", + text: "valid call", + }); + const fitting = await client.callTool({ + name: "procedure_pseudo_code", + arguments: { unexpected: true }, + }); + expect(fitting.isError).toBe(true); + expect(firstText(fitting)).toContain("Input validation error"); - const quotedKey = '"\\'.repeat(350); - const quoted = await client.callTool({ - name: "procedure_pseudo_code", - arguments: { required: "present", [quotedKey]: true }, - }); - expect(quoted.isError).toBe(true); - expect(firstText(quoted)).toContain("Input validation error"); - expect(Buffer.byteLength(JSON.stringify(quoted))).toBeLessThan(4096); - expect(records.exportEvidenceBundle().records).toHaveLength(1); + const oversized = await client.callTool({ + name: "procedure_pseudo_code", + arguments: { required: "present", [oversizedKey]: true }, + }); + expect(oversized.isError).toBe(true); + expect(oversized.structuredContent).toBeUndefined(); + expect(oversized._meta).toEqual({ + provenance: { untrusted_content: true, source: "analysis_target" }, + }); + expect(Buffer.byteLength(JSON.stringify(oversized))).toBeLessThan(4096); + const oversizedError = z + .object({ + error: z.object({ + code: z.string(), + details: z.object({ + reported_limits: z.record(z.string(), z.unknown()), + }), + }), + }) + .parse(JSON.parse(firstText(oversized) ?? "null")); + expect(oversizedError.error.code).toBe("resource_constraint"); + const reportedLimits = oversizedError.error.details.reported_limits; + expect(reportedLimits).toMatchObject({ + evidence_reference: { + kind: "retained-evidence", + evidence_id: expect.any(String), + }, + diagnostic_truncated: true, + }); + expect(reportedLimits).not.toHaveProperty("retention"); + const bundle = records.exportEvidenceBundle(); + expect(bundle.records).toHaveLength(1); + expect(bundle.records[0]?.operation).toBe("mcp_tool_error"); + expect(bundle.records[0]?.raw_result).toMatchObject({ + isError: true, + content: [ + { type: "text", text: expect.stringContaining(oversizedKey) }, + ], + }); + const evidenceReference = z + .object({ evidence_id: z.string() }) + .parse(reportedLimits.evidence_reference); + expect(bundle.records[0]?.evidence_id).toBe( + evidenceReference.evidence_id, + ); - const valid = await client.callTool({ - name: "procedure_pseudo_code", - arguments: { required: "present" }, - }); - expect(valid.content).toContainEqual({ - type: "text", - text: "valid call", - }); - } finally { - await Promise.all([client.close(), server.close()]); - } - }); + const quotedKey = '"\\'.repeat(350); + const quoted = await client.callTool({ + name: "procedure_pseudo_code", + arguments: { required: "present", [quotedKey]: true }, + }); + expect(quoted.isError).toBe(true); + expect(firstText(quoted)).toContain("Input validation error"); + expect(Buffer.byteLength(JSON.stringify(quoted))).toBeLessThan(4096); + expect(records.exportEvidenceBundle().records).toHaveLength(1); + + const recovered = await client.callTool({ + name: "procedure_pseudo_code", + arguments: { required: "present" }, + }); + expect(recovered.content).toContainEqual({ + type: "text", + text: "valid call", + }); + for (const reply of replies) { + expect(reply.result.resultType).toBe( + revision === "2026-07-28" ? "complete" : undefined, + ); + expect( + Buffer.byteLength(JSON.stringify(reply)) + 1, + ).toBeLessThanOrEqual(4096); + } + } finally { + await Promise.all([client.close(), serving.close()]); + } + }, + ); }); diff --git a/src/server/EvidenceMcpServer.ts b/src/server/EvidenceMcpServer.ts index 33ee1f24be..8598afd809 100644 --- a/src/server/EvidenceMcpServer.ts +++ b/src/server/EvidenceMcpServer.ts @@ -451,7 +451,15 @@ class ErrorEvidenceTransport implements Transport { ); return { ...message, - result: this.delivery.toErrorToolResult(failure, targetContent), + result: { + ...this.delivery.toErrorToolResult(failure, targetContent), + // The SDK stamps `resultType` on modern-era results before this + // transport sees them; the replacement must keep the stamp or a + // 2026-07-28 client discards REA's structured error entirely. + ...(typeof message.result.resultType === "string" + ? { resultType: message.result.resultType } + : {}), + }, }; } }