mirror of
https://github.com/morluto/rea.git
synced 2026-10-12 02:19:24 +00:00
fix(mcp): retain oversized errors before transport delivery
This commit is contained in:
+8
-2
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<void> {
|
||||
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<RequestId, string>();
|
||||
|
||||
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<void> {
|
||||
return this.transport.start();
|
||||
}
|
||||
|
||||
async close(): Promise<void> {
|
||||
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<void> {
|
||||
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) };
|
||||
}
|
||||
}
|
||||
@@ -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),
|
||||
);
|
||||
|
||||
/**
|
||||
|
||||
+18
-11
@@ -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<unknown, AnalysisError> | 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",
|
||||
|
||||
@@ -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<CallToolResult> => ({
|
||||
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"),
|
||||
),
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user