diff --git a/docs/testing.md b/docs/testing.md index f56710fd4..ce5e24bc6 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -366,10 +366,16 @@ behavior, and independent CLI/MCP database ownership. Large-result probes compile initialized host-native data sized from the pinned MCP SDK receive budget. Real byte reads, annotation edits and function dossiers exceed that budget while preserving the connection and active analysis run. -Each delivery error must identify its successfully retained Evidence record; -export must recover every source byte and complete annotation, with CLI parity +Successful-result and oversized-error delivery constraints must identify their +successfully retained Evidence records; +export must recover every source byte, complete annotation and original error +diagnostic, with CLI parity and an unchanged executable. The focused formatter check retains only the separate case where recording was not acknowledged. +Long ordinary procedure names and encoded address-space selectors must produce +normal validation errors without exhausting Java's regex stack or losing the +private bridge connection. Each rejection is followed by a real provider lookup; +address-like literal names still resolve exactly after annotation. Malformed annotation readback, memory completeness, and inventory data remain separate SDK/provider integration cases; success from a real provider cannot establish rejection of a contradictory provider response. diff --git a/scripts/lib/real-ghidra-large-results.mjs b/scripts/lib/real-ghidra-large-results.mjs index 6185904a9..76738567e 100644 --- a/scripts/lib/real-ghidra-large-results.mjs +++ b/scripts/lib/real-ghidra-large-results.mjs @@ -64,6 +64,24 @@ 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. + const missingSelector = + "REA_MISSING_" + + "x".repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 2) + 64 * 1024); + const lookupError = await reject("procedure_address", { + procedure: missingSelector, + }); + const lookupId = retainedReference(lookupError, "procedure_address"); + assert.equal( + lookupError.details.reported_limits.original_error_code, + "invalid_request", + ); + assert.equal( + await call("procedure_address", { procedure: entry.address }), + entry.address, + ); + const readError = await reject("read_bytes", { address: payload.address, length, @@ -116,6 +134,28 @@ export async function verifyGhidraLargeResults({ assert.equal(record.subject.digest.sha256, sha256); return record; }; + const originalError = bundle.records.find( + ({ evidence_id }) => evidence_id === lookupId, + ); + assert.ok(originalError); + assert.equal(originalError.operation, "mcp_tool_error"); + assert.equal(originalError.provider.id, "rea-mcp"); + assert.equal(originalError.subject, null); + assert.equal(originalError.parameters.tool_name, "procedure_address"); + assert.equal(originalError.normalized_result.error.code, "invalid_request"); + assert.equal(originalError.raw_result.isError, true); + assert.deepEqual( + originalError.raw_result.structuredContent, + originalError.normalized_result, + ); + assert.equal( + originalError.normalized_result.error.details.operation, + "procedure_address", + ); + assert.equal( + originalError.normalized_result.error.details.issues[0].message, + `Unknown Ghidra procedure name or address: ${missingSelector}`, + ); const readRecord = retained(readId); const read = readRecord.normalized_result; assert.equal(read.requested_bytes, length); diff --git a/src/server/EvidenceMcpServer.ts b/src/server/EvidenceMcpServer.ts new file mode 100644 index 000000000..9e9fbae49 --- /dev/null +++ b/src/server/EvidenceMcpServer.ts @@ -0,0 +1,206 @@ +import { + McpServer, + isCallToolResult, + isJSONRPCRequest, + isJSONRPCNotification, + isJSONRPCResultResponse, + type Implementation, + type JSONRPCMessage, + type McpServerOptions, + type RequestId, + type Transport, + type TransportSendOptions, +} from "@modelcontextprotocol/server"; + +import { analysisErrorProjectionSchema } from "../contracts/errorSchemas.js"; +import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js"; +import { parseMcpResponseBudget } from "../config/mcpResponseBudget.js"; +import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { createEvidence } from "../domain/evidence.js"; +import { jsonObjectSchema } from "../domain/jsonValue.js"; +import { toErrorToolResult } from "./toolResult.js"; +import { + encodeToolResult, + MCP_RESULT_BUDGET_BYTES, +} from "./toolResultEncoding.js"; + +/** Bind oversized MCP error recovery to this server's Evidence ledger. */ +export class EvidenceMcpServer extends McpServer { + constructor( + info: Implementation, + options: McpServerOptions, + private readonly recordEvidence: + | EvidenceWriter["recordEvidence"] + | undefined, + ) { + super(info, options); + } + + /** Preserve the SDK transport lifecycle while retaining oversized errors. */ + override connect(transport: Transport): Promise { + return super.connect( + new ErrorEvidenceTransport(transport, this.recordEvidence), + ); + } +} + +class ErrorEvidenceTransport implements Transport { + onclose: Transport["onclose"]; + onerror: Transport["onerror"]; + onmessage: Transport["onmessage"]; + readonly hasPerRequestStream?: boolean; + private readonly pendingTools = new Map(); + + constructor( + private readonly transport: Transport, + private readonly recordEvidence: + | EvidenceWriter["recordEvidence"] + | undefined, + ) { + if (transport.hasPerRequestStream !== undefined) + this.hasPerRequestStream = transport.hasPerRequestStream; + transport.onmessage = (message, extra) => { + if ( + isJSONRPCRequest(message) && + message.method === "tools/call" && + typeof message.params?.["name"] === "string" + ) + this.pendingTools.set(message.id, message.params["name"]); + // The SDK suppresses replies after cancellation, so send() cannot release + // this bookkeeping. Keep transport authentication outside retained data. + if ( + isJSONRPCNotification(message) && + message.method === "notifications/cancelled" + ) { + const id = message.params?.["requestId"]; + if (typeof id === "string" || typeof id === "number") + this.pendingTools.delete(id); + } + this.onmessage?.(message, extra); + }; + transport.onerror = (error) => this.onerror?.(error); + transport.onclose = () => { + this.pendingTools.clear(); + this.onclose?.(); + }; + } + + get sessionId(): string | undefined { + return this.transport.sessionId; + } + set sessionId(value: string | undefined) { + this.transport.sessionId = value; + } + + start(): Promise { + return this.transport.start(); + } + + async close(): Promise { + try { + await this.transport.close(); + } finally { + this.pendingTools.clear(); + } + } + + setProtocolVersion(version: string): void { + this.transport.setProtocolVersion?.(version); + } + + setSupportedProtocolVersions(versions: string[]): void { + this.transport.setSupportedProtocolVersions?.(versions); + } + + async send( + message: JSONRPCMessage, + options?: TransportSendOptions, + ): Promise { + const id = + "id" in message && + (typeof message.id === "string" || typeof message.id === "number") + ? message.id + : undefined; + const tool = id === undefined ? undefined : this.pendingTools.get(id); + try { + await this.transport.send(this.recoverError(message, tool), options); + } finally { + if (!isJSONRPCRequest(message) && id !== undefined) + this.pendingTools.delete(id); + } + } + + private recoverError( + message: JSONRPCMessage, + tool: string | undefined, + ): JSONRPCMessage { + if ( + !isJSONRPCResultResponse(message) || + !isCallToolResult(message.result) || + message.result.isError !== true || + message.result.structuredContent === undefined + ) + return message; + const structured = jsonObjectSchema.parse(message.result.structuredContent); + const configured = parseMcpResponseBudget( + process.env.REA_MCP_MAX_RESPONSE_BYTES, + ); + const budget = + configured.ok && configured.value !== undefined + ? configured.value - 1024 + : MCP_RESULT_BUDGET_BYTES; + const encoded = encodeToolResult(structured, budget); + if (encoded.ok) return message; + const operation = tool ?? "mcp_tool_error"; + const originalError = analysisErrorProjectionSchema.safeParse( + structured["error"], + ); + // Retain the already projected diagnostic. Transport authentication and raw + // request arguments are deliberately excluded: the application owns their + // explicit sensitivity policy, and its error projection is authoritative. + const evidence = createEvidence( + undefined, + { id: "rea-mcp", name: "REA MCP delivery", version: "1" }, + { + operation: "mcp_tool_error", + parameters: { tool_name: operation }, + result: structured, + rawResult: jsonObjectSchema.parse(message.result), + }, + ); + const recorded = this.recordEvidence?.(evidence); + const retained = recorded !== undefined && recorded.ok; + const failure = new AnalysisResourceConstraintError( + operation, + "transport", + "The tool failed, but its complete error response exceeds the MCP response budget.", + { + boundary: "mcp-response", + ...(originalError.success + ? { original_error_code: originalError.data.code } + : {}), + result_budget_bytes: budget, + response_bytes_at_least: encoded.bytesAtLeast, + constraint: encoded.constraint, + ...(retained + ? { + evidence_reference: { + kind: "retained-evidence", + evidence_id: evidence.evidence_id, + }, + } + : { retention: recorded === undefined ? "unavailable" : "failed" }), + ...(recorded !== undefined && !recorded.ok + ? { retention_error: projectAnalysisError(recorded.error) } + : {}), + }, + { + remediationAction: retained + ? "Export the retained original error through export_evidence_bundle to a caller-selected path. The connection remains usable." + : "Configure a larger REA_MCP_MAX_RESPONSE_BYTES and matching client receive buffer, then retry. The original error could not be retained by this server.", + }, + ); + return { ...message, result: toErrorToolResult(failure) }; + } +} diff --git a/src/server/createServer.ts b/src/server/createServer.ts index 73d724ffd..c105d7a97 100644 --- a/src/server/createServer.ts +++ b/src/server/createServer.ts @@ -7,7 +7,8 @@ import type { RecordedCrashService } from "../application/binaryDiagnostics/Reco import { registerBinaryDiagnosticsTools } from "./registerBinaryDiagnosticsTools.js"; import { createBinaryLayoutService } from "../composition/binaryDiagnostics.js"; import type { BinaryLayoutService } from "../application/binaryDiagnostics/BinaryLayoutService.js"; -import { McpServer } from "@modelcontextprotocol/server"; +import type { McpServer } from "@modelcontextprotocol/server"; +import { EvidenceMcpServer } from "./EvidenceMcpServer.js"; import { isAbsolute } from "node:path"; import type { AnalysisOperationPort } from "../application/AnalysisProvider.js"; @@ -145,7 +146,7 @@ const installSessionToolAvailability = ( }; const createMcpServer = (session: BinarySessionPort | undefined): McpServer => - new McpServer( + new EvidenceMcpServer( { name: PRODUCT_IDENTITY.mcpServerKey, version: PRODUCT_IDENTITY.packageVersion, @@ -157,6 +158,9 @@ const createMcpServer = (session: BinarySessionPort | undefined): McpServer => ? ACTIVE_TARGET_INSTRUCTIONS : TARGET_FREE_INSTRUCTIONS, }, + session === undefined + ? undefined + : (evidence) => session.recordEvidence(evidence), ); /** diff --git a/src/server/toolResult.ts b/src/server/toolResult.ts index 9313f6dfa..cb641abca 100644 --- a/src/server/toolResult.ts +++ b/src/server/toolResult.ts @@ -27,7 +27,7 @@ export const toCallToolResult = ( ): CallToolResult => result.ok ? successResult(result.value, contract, context) - : errorResult(result.error); + : toErrorToolResult(result.error); /** Deliver Evidence using the producer's explicit recording acknowledgment. */ export const toEvidenceToolResult = ( @@ -36,7 +36,7 @@ export const toEvidenceToolResult = ( recorded: Result | undefined, ): CallToolResult => recorded !== undefined && !recorded.ok - ? errorResult(recorded.error) + ? toErrorToolResult(recorded.error) : toCallToolResult( { ok: true, value: evidence }, contract, @@ -45,16 +45,23 @@ export const toEvidenceToolResult = ( : { retainedEvidenceId: evidence.evidence_id }, ); -const errorResult = (error: AnalysisError): CallToolResult => { +/** Project an error without allocating oversized repeated MCP text. */ +export const toErrorToolResult = (error: AnalysisError): CallToolResult => { const projected = projectAnalysisError(error); const structuredContent = { error: projected }; + const configured = parseMcpResponseBudget( + process.env.REA_MCP_MAX_RESPONSE_BYTES, + ); + const budget = + configured.ok && configured.value !== undefined + ? configured.value - 1024 + : MCP_RESULT_BUDGET_BYTES; + const encoded = encodeToolResult(structuredContent, budget); + // The session-bound transport retains an oversized projected error before + // replacing it with a recoverable delivery constraint. Avoid allocating its + // repeated text representation before that boundary has acknowledged it. return { - content: [ - { - type: "text", - text: JSON.stringify(structuredContent), - }, - ], + content: encoded.ok ? [{ type: "text", text: encoded.text }] : [], structuredContent, isError: true, }; @@ -71,14 +78,14 @@ const successResult = ( const configured = parseMcpResponseBudget( process.env.REA_MCP_MAX_RESPONSE_BYTES, ); - if (!configured.ok) return errorResult(configured.error); + if (!configured.ok) return toErrorToolResult(configured.error); const budget = configured.value === undefined ? MCP_RESULT_BUDGET_BYTES : configured.value - 1024; const encoded = encodeToolResult(candidate, budget); if (!encoded.ok) - return errorResult( + return toErrorToolResult( new AnalysisResourceConstraintError( contract.name, "transport", diff --git a/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts new file mode 100644 index 000000000..c5d6a206c --- /dev/null +++ b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts @@ -0,0 +1,115 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { + STDIO_DEFAULT_MAX_BUFFER_SIZE, + type CallToolResult, +} from "@modelcontextprotocol/server"; +import { expect, it } from "vitest"; +import { z } from "zod"; + +import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js"; +import type { EvidenceWriter } from "../../../src/application/investigation/InvestigationRecordPort.js"; +import { EvidenceIntegrityError } from "../../../src/domain/evidenceErrors.js"; +import { err } from "../../../src/domain/result.js"; +import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; +import { toErrorToolResult } from "../../../src/server/toolResult.js"; + +const oversizedFailure = () => + 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)}`, + }, + ]), + ); + +const exerciseDeliveryFailure = async ( + recordEvidence: EvidenceWriter["recordEvidence"] | undefined, +) => { + const server = new EvidenceMcpServer( + { name: "oversized-error-test", version: "1" }, + { capabilities: {} }, + recordEvidence, + ); + server.registerTool("large_failure", { inputSchema: {} }, async () => + oversizedFailure(), + ); + server.registerTool( + "healthy", + { inputSchema: {} }, + async (): Promise => ({ + content: [{ type: "text", text: "still connected" }], + }), + ); + const client = new Client({ name: "oversized-error-client", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + + const failure = await client.callTool({ + name: "large_failure", + arguments: {}, + }); + expect(failure.isError).toBe(true); + expect(failure.structuredContent).toMatchObject({ + error: { + code: "resource_constraint", + details: { + reported_limits: { + original_error_code: "invalid_request", + }, + }, + }, + }); + const reportedLimits = z + .object({ + error: z.object({ + code: z.literal("resource_constraint"), + details: z.object({ + reported_limits: z.record(z.string(), z.unknown()), + }), + }), + }) + .parse(failure.structuredContent).error.details.reported_limits; + expect(failure.structuredContent).toMatchObject({ + error: { + details: { + reported_limits: { + boundary: "mcp-response", + }, + }, + }, + }); + expect(reportedLimits).not.toHaveProperty("evidence_reference"); + expect(reportedLimits.retention).toBe( + recordEvidence === undefined ? "unavailable" : "failed", + ); + if (recordEvidence !== undefined) + expect(reportedLimits.retention_error).toMatchObject({ + code: "evidence_integrity_mismatch", + }); + expect(Buffer.byteLength(JSON.stringify(failure))).toBeLessThan( + STDIO_DEFAULT_MAX_BUFFER_SIZE, + ); + + const healthy = await client.callTool({ name: "healthy", arguments: {} }); + expect(healthy.content).toContainEqual({ + type: "text", + text: "still connected", + }); + } finally { + await Promise.all([client.close(), server.close()]); + } +}; + +it("keeps the SDK connection usable when oversized-error retention is unavailable or fails", async () => { + await exerciseDeliveryFailure(undefined); + await exerciseDeliveryFailure(() => + err( + new EvidenceIntegrityError("fixture ledger rejected the Evidence record"), + ), + ); +});