mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
Merge pull request #1288 from morluto/fix/mcp-error-text-budget
fix(mcp): measure tool errors against their text-only delivery
This commit is contained in:
8 files changed
+103
-14
No files matched your search
@@ -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
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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", {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()]);
|
||||
}
|
||||
});
|
||||
@@ -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 () =>
|
||||
|
||||
Reference in new issue
Block a user