diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 16ce69d6b5..150cf71a97 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -201,7 +201,8 @@ retained session history or an explicit bundle for transfer. 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. Response budget settings are captured at startup; +room for the JSON-RPC envelope. Tool errors carry only their text +representation, so only that text counts against the 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 diff --git a/scripts/lib/real-ghidra-large-results.mjs b/scripts/lib/real-ghidra-large-results.mjs index 76738567e1..92dde5045a 100644 --- a/scripts/lib/real-ghidra-large-results.mjs +++ b/scripts/lib/real-ghidra-large-results.mjs @@ -64,11 +64,13 @@ export async function verifyGhidraLargeResults({ assert.equal(expected[0], 0x5a); assert.equal(expected.at(-1), 0xa5); const before = await call("binary_session"); - // This request fits the SDK receive limit, but repeating the producer's - // diagnostic in text and structured content cannot fit the response. + // An error reaches the wire once, as escaped JSON text. Each quote costs + // two request bytes but four response bytes once the echoed diagnostic is + // escaped again as text, so this request fits the SDK receive limit while + // the error response cannot. const missingSelector = "REA_MISSING_" + - "x".repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 2) + 64 * 1024); + '"'.repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 4) + 64 * 1024); const lookupError = await reject("procedure_address", { procedure: missingSelector, }); diff --git a/src/server/EvidenceMcpServer.ts b/src/server/EvidenceMcpServer.ts index f2e2396ddf..706ad05772 100644 --- a/src/server/EvidenceMcpServer.ts +++ b/src/server/EvidenceMcpServer.ts @@ -153,7 +153,8 @@ class ErrorEvidenceTransport implements Transport { return message; const structured = jsonObjectSchema.parse(message.result.structuredContent); const budget = this.delivery.resultBudgetBytes; - const encoded = encodeToolResult(structured, budget); + // The delivered error omits structuredContent, so only its text counts. + const encoded = encodeToolResult(structured, budget, "text"); if (encoded.ok) return message; const operation = tool ?? "mcp_tool_error"; const originalError = analysisErrorProjectionSchema.safeParse( diff --git a/src/server/toolResult.ts b/src/server/toolResult.ts index df7be01f8c..90f54dda18 100644 --- a/src/server/toolResult.ts +++ b/src/server/toolResult.ts @@ -65,7 +65,13 @@ export class ToolResultDelivery { /** Project an error without allocating oversized repeated MCP text. */ toErrorToolResult(error: AnalysisError): CallToolResult { const structuredContent = { error: projectAnalysisError(error) }; - const encoded = encodeToolResult(structuredContent, this.resultBudgetBytes); + // Errors reach the wire as text only; the structured copy is a private + // carrier the transport removes after its own budget check. + const encoded = encodeToolResult( + structuredContent, + this.resultBudgetBytes, + "text", + ); // The transport retains an oversized projected error before replacing it // with a recoverable delivery constraint, using this same budget. return { diff --git a/src/server/toolResultEncoding.test.ts b/src/server/toolResultEncoding.test.ts index c6b20d3f55..3225ddd8b9 100644 --- a/src/server/toolResultEncoding.test.ts +++ b/src/server/toolResultEncoding.test.ts @@ -41,6 +41,25 @@ describe("MCP result encoding budget", () => { }); }); + it.each([ + { error: { code: "invalid_request", message: 'δΈ­ζ–‡"\n'.repeat(30000) } }, + { error: { message: "πŸ˜€\ud800" } }, + ])("counts only escaped text for text-only error delivery", (candidate) => { + const textBytes = Buffer.byteLength( + JSON.stringify({ + content: [{ type: "text", text: JSON.stringify(candidate) }], + }), + ); + const encoded = encodeToolResult(candidate, textBytes, "text"); + if (!encoded.ok) throw new Error("Expected complete encoding"); + expect(encoded.text).toBe(JSON.stringify(candidate)); + expect(encoded.bytes).toBe(textBytes); + expect(encodeToolResult(candidate, textBytes - 1, "text")).toMatchObject({ + ok: false, + bytesAtLeast: textBytes, + }); + }); + it("stops before reading the remainder of an oversized result", () => { const candidate: JsonValue = { text: "x".repeat(1000000) }; Object.defineProperty(candidate, "remainder", { diff --git a/src/server/toolResultEncoding.ts b/src/server/toolResultEncoding.ts index c72b38859b..54d6efa6f3 100644 --- a/src/server/toolResultEncoding.ts +++ b/src/server/toolResultEncoding.ts @@ -13,6 +13,16 @@ const RESULT_ENVELOPE_BYTES = }), ) - 4; +const TEXT_RESULT_ENVELOPE_BYTES = Buffer.byteLength( + JSON.stringify({ content: [{ type: "text", text: "" }] }), +); + +/** + * MCP representations a result carries on the wire. Successful results repeat + * the JSON as structured content and escaped text; error results carry text only. + */ +export type ToolResultRepresentations = "structured-and-text" | "text"; + /** Result of encoding the complete repeated MCP representations within a wire budget. */ export type ToolResultEncoding = | { readonly ok: true; readonly text: string; readonly bytes: number } @@ -23,18 +33,23 @@ export type ToolResultEncoding = readonly constraint: "receive-buffer" | "string-length"; }; -/** Account for structured JSON and escaped text before allocating the complete text. */ +/** Account for the delivered JSON representations before allocating the complete text. */ export const encodeToolResult = ( value: JsonValue, budgetBytes: number, + representations: ToolResultRepresentations = "structured-and-text", ): ToolResultEncoding => { + const structured = representations === "structured-and-text"; const parts: string[] = []; - let bytes = RESULT_ENVELOPE_BYTES; - let codeUnits = RESULT_ENVELOPE_BYTES; + let bytes = structured ? RESULT_ENVELOPE_BYTES : TEXT_RESULT_ENVELOPE_BYTES; + let codeUnits = bytes; for (const part of bufferedJsonParts(jsonParts(value))) { const escapedPart = JSON.stringify(part); - bytes += Buffer.byteLength(part) + Buffer.byteLength(escapedPart) - 2; - codeUnits += part.length + escapedPart.length - 2; + bytes += + (structured ? Buffer.byteLength(part) : 0) + + Buffer.byteLength(escapedPart) - + 2; + codeUnits += (structured ? part.length : 0) + escapedPart.length - 2; if (bytes > budgetBytes || codeUnits > MCP_RESULT_STRING_LIMIT) return { ok: false, diff --git a/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts index ea2809b7ce..ba90eafdfe 100644 --- a/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts +++ b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts @@ -17,17 +17,22 @@ import { ToolResultDelivery } from "../../../src/server/toolResult.js"; const delivery = new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE); -const oversizedFailure = () => +const failureWithName = (nameBytes: number) => delivery.toErrorToolResult( new AnalysisInputError("procedure_address", undefined, [ { path: ["procedure"], reason: "invalid_value", - message: `Unknown Ghidra procedure name or address: REA_MISSING_${"x".repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 2) + 64 * 1024)}`, + message: `Unknown Ghidra procedure name or address: REA_MISSING_${"x".repeat(nameBytes)}`, }, ]), ); +// The delivered error carries its JSON once, as text, so only a diagnostic +// larger than the whole budget is oversized. +const oversizedFailure = () => + failureWithName(STDIO_DEFAULT_MAX_BUFFER_SIZE + 64 * 1024); + const exerciseDeliveryFailure = async ( recordEvidence: EvidenceWriter["recordEvidence"] | undefined, retention: "retained" | "unavailable" | "failed", @@ -143,3 +148,42 @@ it("retains the complete oversized diagnostic before removing its private struct ); expect(retained[0]?.raw_result).toEqual(oversizedFailure()); }); + +it("delivers an error whose text fits although structured and text copies would not", async () => { + const nameBytes = Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 2) + 64 * 1024; + const retained: Evidence[] = []; + const server = new EvidenceMcpServer( + { name: "mid-sized-error-test", version: "1" }, + { capabilities: {} }, + (evidence) => { + retained.push(evidence); + return ok("added"); + }, + delivery, + ); + server.registerTool("mid_failure", { inputSchema: {} }, async () => + failureWithName(nameBytes), + ); + const client = new Client({ name: "mid-sized-error-client", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const failure = await client.callTool({ + name: "mid_failure", + arguments: {}, + }); + expect(failure.isError).toBe(true); + expect(failure.structuredContent).toBeUndefined(); + expect(parseMcpToolError(failure)).toEqual( + failureWithName(nameBytes).structuredContent, + ); + expect(retained).toHaveLength(0); + expect(Buffer.byteLength(JSON.stringify(failure))).toBeLessThan( + STDIO_DEFAULT_MAX_BUFFER_SIZE, + ); + } finally { + await Promise.all([client.close(), server.close()]); + } +}); diff --git a/tests/process-global/mcpDeliveryEnvironment.test.ts b/tests/process-global/mcpDeliveryEnvironment.test.ts index 33bb18ed51..32c9061af3 100644 --- a/tests/process-global/mcpDeliveryEnvironment.test.ts +++ b/tests/process-global/mcpDeliveryEnvironment.test.ts @@ -62,8 +62,9 @@ it("keeps independent server budgets for successful results and oversized errors largeEnvironment.REA_MCP_MAX_RESPONSE_BYTES = String(smallBudget); vi.stubEnv("REA_MCP_MAX_RESPONSE_BYTES", "invalid-ambient-after-selection"); + // Error text alone exceeds the small budget and fits the large one. const diagnostic = "observed failure ".repeat( - Math.ceil((smallBudget / 2 + 65536) / 17), + Math.ceil((smallBudget + 65536) / 17), ); for (const server of [small, large]) server.registerTool("selected_failure", { inputSchema: {} }, async () =>