From 0c6852b392b34319a6fd2c6dffcac10cee51ab94 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:20:33 +0800 Subject: [PATCH 1/2] docs(ghidra): explain supported heap and CPU controls (#1278) Explain consumed heap settings, owned JVM environment overrides, CPU affinity and memory measurements in the Ghidra setup guide. Closes #1255 --- docs/installation.md | 63 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index 5ac6692cf8..a0be329205 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -493,14 +493,14 @@ installs, upgrades, or modifies Ghidra or Java. Each verified session uses an ephemeral temporary project and isolated home/cache/config/temp paths. REA passes `-readOnly`, `-deleteProject`, uses -Ghidra's default analysis and resource settings, and loads its packaged Java +Ghidra's default analysis settings, and loads its packaged Java bridge via `-scriptPath`; it never opens an existing user project. Linux and macOS use a current-user-only local bridge socket and descriptor. The project remains under the selected temporary directory. If its Unix socket pathname would exceed the host's byte limit, REA allocates a separate mode-0700 socket directory under `/tmp` and removes it on close, cancellation, or failure. Diagnostics retain the actual endpoint and both owned directories. -On macOS, REA starts the inspected JVM directly using Ghidra's own LaunchSupport +On Linux and macOS, REA starts the inspected JVM directly using Ghidra's own LaunchSupport configuration. Apple platform shell wrappers hide their environments from ownership inspection, so retaining those wrappers would prevent verified process-group cancellation during startup. @@ -534,6 +534,65 @@ is no fixed per-operation or response-size ceiling. Unresolved computed calls remain unknown, reference-kind provenance is preserved, and provider-specific pseudocode is never treated as original source or Hopper-equivalent text. +### Ghidra heap and CPU controls + +Set resource controls in the environment that launches the CLI or MCP server, +before opening a new Ghidra session. On Linux and macOS, REA chooses the first +nonempty setting in this order: `GHIDRA_HEADLESS_MAXMEM`, `GHIDRA_MAXMEM`, then +`2G`. The chosen value becomes the JVM's `-Xmx` argument. Use a JVM heap-size +value such as `512M` or `2G`; an invalid value causes Java startup to fail. +The supported Ghidra 12.1.4 Windows headless script consumes the same settings +with the same precedence. + +For a small target on a constrained host: + +```bash +export GHIDRA_HEADLESS_MAXMEM=512M +``` + +In PowerShell: + +```powershell +$env:GHIDRA_HEADLESS_MAXMEM = "512M" +``` + +An MCP client must pass this setting to its REA server process; changing an +unrelated terminal's environment does not reconfigure an existing server or +JVM. A 512 MiB heap was verified with small native fixtures, but larger targets +can require more memory. The Java heap limit does not bound JVM RSS, Node's +heap, or total memory used by the process family. The startup deadline controls +waiting time independently of these memory settings. + +REA clears `_JAVA_OPTIONS`, `JAVA_TOOL_OPTIONS`, `JDK_JAVA_OPTIONS`, and +`GHIDRA_JAVA_OPTIONS` inherited from the caller, and owns +`GHIDRA_HEADLESS_JAVA_OPTIONS`. Custom flags injected through these variables +are not supported configuration. On Windows, REA replaces `JDK_JAVA_OPTIONS` +with its own isolated home/temp settings and `-XX:-UsePerfData`; on Linux and +macOS it supplies isolated paths directly to Java. Use the supported heap +settings above instead of adding a second `-Xmx` through an injection variable. + +The Linux/macOS launch sets `-XX:ParallelGCThreads=2` and +`-XX:CICompilerCount=2`; the supported Windows headless script also sets these +counts. These constrain particular JVM thread pools, not all analysis threads +or CPU usage. REA does not pass Ghidra's `-max-cpu` option or expose a general +Ghidra CPU limit. When needed, apply host CPU affinity and scheduling controls +to the REA process and verify the resulting JVM's affinity. Select CPUs allowed +on the host; do not assume CPU numbers from another machine are valid. + +Verify controls at their consumer after the first Ghidra-backed query starts +the JVM: inspect that owned JVM's effective `-Xmx` and GC/compiler flags, CPU +affinity, and observed memory use. Do not infer them from the parent shell's +environment or the absence of an error. On Linux, `/proc//cmdline` +contains the launch arguments and `/proc//status` reports +`Cpus_allowed_list` and `VmRSS`. Inspect only the process belonging to this +session and retain the resource flags needed for the measurement rather than +copying full command lines or environments into reports. Heap and affinity +settings affect cost and latency; they are not part of REA's semantic analysis +profile identity. See [real-provider testing](testing.md) for bounded +verification workflows. + +### Ghidra diagnostics and verification + Unexpected request failures retain their original internal cause. CLI and MCP errors expose Error names, messages, and available codes under `details.diagnostics.failure_cause`; other rejection values retain their From 7e96f25f8b1380a13a086a68f8d5a70380468151 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Fri, 9 Oct 2026 21:54:32 +0800 Subject: [PATCH 2/2] fix(mcp): deliver typed errors without invalid structured output (#1277) * fix(mcp): deliver tool errors within advertised schema contracts Retain complete typed JSON error content while omitting non-conforming structured errors at the public transport boundary. Recover oversized diagnostics into Evidence before removing their private structured carrier. Update schema-validated client readers and cover actual stdio delivery. Closes #1261 Assisted-by: Kogasa (gensokyo-skills:kogasa-surprise-testing) * test(mcp): read typed errors in canonical Evidence regressions * test(mcp): validate public error diagnostics in acceptance workflows --- docs/mcp-contracts.md | 9 +++ scripts/lib/browser-screenshot-e2e.mjs | 5 +- scripts/lib/ghidra-reference-snapshot-e2e.mjs | 11 ++-- scripts/lib/mcp-verifier-results.mjs | 7 +++ .../lib/real-ghidra-address-boundaries.mjs | 3 +- scripts/lib/real-ghidra-boundaries.mjs | 33 ++++++----- .../lib/real-ghidra-snapshot-lifecycle.mjs | 7 ++- scripts/verify-native-calls.mjs | 7 ++- scripts/verify-package-mcp.mjs | 3 +- scripts/verify-real-ida.mjs | 3 +- scripts/verify/android/failure-recovery.mjs | 16 +++--- scripts/verify/electron/tool-contracts.mjs | 5 +- src/server/EvidenceMcpServer.ts | 13 ++++- tests/acceptance/nonBinaryComposition.test.ts | 15 ++--- .../tools/historicalCaptureResources.test.ts | 5 +- .../tools/inspectorLifecycle.test.ts | 5 +- .../tools/localToolContracts.test.ts | 3 +- tests/boundary/mcp/analysisViewMcp.test.ts | 10 ++-- .../mcp/applicationEvidenceMcp.test.ts | 18 +++--- .../mcp/artifactMcp.inlineInventory.test.ts | 5 +- tests/boundary/mcp/artifactMcp.test.ts | 3 +- tests/boundary/mcp/browserMcp.test.ts | 3 +- .../boundary/mcp/bundleComparisonMcp.test.ts | 3 +- .../boundary/mcp/contractPresentation.test.ts | 6 +- tests/boundary/mcp/dylibResolutionMcp.test.ts | 11 ++-- ...4-uses-complete-inventories-inline.test.ts | 3 +- ...ts-incomplete-function-dossiers-at.test.ts | 5 +- .../mcp/enhancedTools.inputValidation.test.ts | 3 +- tests/boundary/mcp/enhancedToolsHarness.ts | 10 +++- tests/boundary/mcp/ghidraEvidenceMcp.test.ts | 3 +- .../mcp/ghidraFailureDiagnostics.test.ts | 3 +- tests/boundary/mcp/ghidraInventoryMcp.test.ts | 7 ++- tests/boundary/mcp/ghidraRoutingMcp.test.ts | 3 +- tests/boundary/mcp/largeToolResult.test.ts | 3 +- tests/boundary/mcp/managedMcp.test.ts | 9 ++- .../mcp/nativeCallObservationMcp.test.ts | 3 +- .../nativeSignatureTargetBindingMcp.test.ts | 3 +- tests/boundary/mcp/officialTools.test.ts | 3 +- .../mcp/optionalProviderIsolation.test.ts | 3 +- .../mcp/oversizedErrorDeliveryMcp.test.ts | 55 ++++++++++++++----- .../processComparisonDiagnosticsMcp.test.ts | 11 ++-- .../mcp/processEvidenceIdentityMcp.test.ts | 5 +- .../boundary/mcp/providerSelectionMcp.test.ts | 5 +- .../mcp/reconstructionCoverageMcp.test.ts | 3 +- .../retainedApplicationEvidenceMcp.test.ts | 30 ++++------ tests/boundary/mcp/serverIdentity.test.ts | 3 +- tests/boundary/mcp/serverLifecycleMcp.test.ts | 4 +- tests/boundary/mcp/serverUnknownsMcp.test.ts | 2 + tests/boundary/mcp/serverWorkflowMcp.test.ts | 2 + tests/boundary/mcp/sessionMcp.test.ts | 2 + tests/boundary/mcp/sessionMcpHarness.ts | 2 + .../mcp/toolErrorSchemaDelivery.test.ts | 50 +++++++++++++++++ .../boundary/mcp/webNetworkCaptureMcp.test.ts | 3 +- tests/fixtures/localToolsMcp.ts | 6 +- tests/fixtures/mcpToolError.ts | 16 ++++++ .../mcpDeliveryEnvironment.test.ts | 11 +++- 56 files changed, 324 insertions(+), 156 deletions(-) create mode 100644 tests/boundary/mcp/toolErrorSchemaDelivery.test.ts create mode 100644 tests/fixtures/mcpToolError.ts diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 77619106c0..16ce69d6b5 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -77,6 +77,15 @@ inventories, and do not claim that no short-lived descendant existed. ## Progress and cancellation +Tool execution failures return `isError: true` and the complete canonical +`{ "error": ... }` diagnostic as JSON in text `content`. Read that diagnostic +for the error code, target details, partial observations, and remediation. +Failures omit `structuredContent`, since each advertised `outputSchema` +describes successful results. Successful replies retain their schema-validated +structured data and matching text projection. Oversized errors are retained +as Evidence before delivery of a bounded diagnostic with an export reference; +if retention is unavailable or fails, the diagnostic reports that reason. + REA accepts ordinary `tools/call` progress tokens. Updates are monotonic, rate-bounded to at most one intermediate update per 100 ms, and always allow a terminal update. Unknown totals are omitted; REA does not fabricate percentages. diff --git a/scripts/lib/browser-screenshot-e2e.mjs b/scripts/lib/browser-screenshot-e2e.mjs index 7f1edc0f83..92a27116fb 100644 --- a/scripts/lib/browser-screenshot-e2e.mjs +++ b/scripts/lib/browser-screenshot-e2e.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { mkdtemp, readFile, rm } from "node:fs/promises"; @@ -158,11 +159,11 @@ const verifyDefaultScreenshotDelivery = async ({ ); assert.equal(constrained.isError, true); assert.equal( - constrained.structuredContent?.error?.details?.resource, + requireMcpToolError(constrained)?.details?.resource, "transport", ); const reference = - constrained.structuredContent?.error?.details?.reported_limits + requireMcpToolError(constrained)?.details?.reported_limits ?.evidence_reference; assert.equal(reference?.kind, "retained-evidence"); await client.ping(); diff --git a/scripts/lib/ghidra-reference-snapshot-e2e.mjs b/scripts/lib/ghidra-reference-snapshot-e2e.mjs index d759ed623e..b0ce2318fb 100644 --- a/scripts/lib/ghidra-reference-snapshot-e2e.mjs +++ b/scripts/lib/ghidra-reference-snapshot-e2e.mjs @@ -1,3 +1,8 @@ +import { + requireMcpToolError, + requireMcpLifecycleResult, + requireMcpEvidenceResult, +} from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; @@ -15,10 +20,6 @@ import { } from "../../dist/domain/analysisSnapshot.js"; import { createEvidence } from "../../dist/domain/evidence.js"; import { createEvidenceBundle } from "../../dist/domain/evidenceBundle.js"; -import { - requireMcpLifecycleResult, - requireMcpEvidenceResult, -} from "./mcp-verifier-results.mjs"; /** Reject a valid older snapshot whose instruction-only collector omitted a real edge. */ export async function verifyLegacyGhidraReferenceSnapshot( @@ -124,7 +125,7 @@ export async function verifyLegacyGhidraReferenceSnapshot( }, }); assert.equal(reply.isError, true); - const mcpRejection = reply.structuredContent.error; + const mcpRejection = requireMcpToolError(reply); assert.equal(mcpRejection.code, "evidence_integrity_mismatch"); assert.match(mcpRejection.details.reason, /profile_mismatch/u); assert.match( diff --git a/scripts/lib/mcp-verifier-results.mjs b/scripts/lib/mcp-verifier-results.mjs index 6af6bd9c5a..7ac038a0a6 100644 --- a/scripts/lib/mcp-verifier-results.mjs +++ b/scripts/lib/mcp-verifier-results.mjs @@ -1,3 +1,4 @@ +import { analysisErrorProjectionSchema } from "../../dist/contracts/errorSchemas.js"; import { toolContract } from "../../dist/contracts/toolContracts.js"; /** Read the textual JSON projection from one MCP tool call. */ @@ -9,6 +10,12 @@ export const mcpTextValue = (result) => { const jsonValue = (result) => JSON.parse(mcpTextValue(result)); +/** Read the complete canonical diagnostic from an MCP tool failure. */ +export const requireMcpToolError = (result) => { + if (result.isError !== true) throw new Error("Expected an MCP tool failure"); + return analysisErrorProjectionSchema.parse(jsonValue(result).error); +}; + const requireOutput = (result, operation, contract) => { if (result.isError === true) throw new Error(`${operation} failed: ${mcpTextValue(result)}`); diff --git a/scripts/lib/real-ghidra-address-boundaries.mjs b/scripts/lib/real-ghidra-address-boundaries.mjs index d964b6248a..f5b54d5f47 100644 --- a/scripts/lib/real-ghidra-address-boundaries.mjs +++ b/scripts/lib/real-ghidra-address-boundaries.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; /** Exercise segmented-address truncation through real CLI/MCP reads and edits. */ @@ -40,7 +41,7 @@ export async function verifyGhidraAddressBoundaries(client, entry, cli) { { timeout: 240000 }, ); assert.equal(reply.isError, true, `${name} accepted ${address}`); - const error = reply.structuredContent?.error; + const error = requireMcpToolError(reply); assert.equal(error?.code, "invalid_request", JSON.stringify(reply)); assert.equal(error.details.operation, name); const issues = JSON.stringify(error.details.issues); diff --git a/scripts/lib/real-ghidra-boundaries.mjs b/scripts/lib/real-ghidra-boundaries.mjs index bba21691de..0a59c5fae1 100644 --- a/scripts/lib/real-ghidra-boundaries.mjs +++ b/scripts/lib/real-ghidra-boundaries.mjs @@ -1,3 +1,8 @@ +import { + requireMcpToolError, + mcpTextValue, + requireMcpOperationResult, +} from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; @@ -7,11 +12,9 @@ import { setTimeout as delay } from "node:timers/promises"; import { promisify } from "node:util"; import Ajv from "ajv/dist/2020.js"; import addFormats from "ajv-formats"; + import { analysisErrorProjectionSchema } from "../../dist/contracts/errorSchemas.js"; -import { - mcpTextValue, - requireMcpOperationResult, -} from "./mcp-verifier-results.mjs"; + import { verifyLegacyGhidraReferenceSnapshot } from "./ghidra-reference-snapshot-e2e.mjs"; import { verifyGhidraSnapshotLifecycle } from "./real-ghidra-snapshot-lifecycle.mjs"; import { verifyGhidraTargetAdmission } from "./real-ghidra-target-admission.mjs"; @@ -61,23 +64,19 @@ export async function verifyGhidraBoundaries( true, `${name} accepted ${JSON.stringify(args)}`, ); - // SDK input-schema rejections are text-only; application errors use the - // canonical structured error projection, separately from success schemas. - if (reply.structuredContent !== undefined) { - analysisErrorProjectionSchema.parse(reply.structuredContent.error); - assert.deepEqual( - reply.structuredContent, - JSON.parse(mcpTextValue(reply)), - ); - } + // SDK input rejections are prose; application errors carry canonical JSON + // text without publishing data outside the advertised success schema. + const error = mcpTextValue(reply).startsWith("{") + ? requireMcpToolError(reply) + : undefined; + assert.equal(reply.structuredContent, undefined); if (diagnostic !== undefined) { - const error = reply.structuredContent?.error; assert.equal(error?.code, "invalid_request"); assert.equal(error?.details?.operation, name); assert.match(JSON.stringify(error.details.issues), diagnostic); } rejectedCalls++; - return reply.structuredContent?.error; + return error; }; const cli = async (command, value, flags = []) => { const { stdout } = await promisify(execFile)( @@ -481,7 +480,7 @@ export async function verifyGhidraBoundaries( options, ); assert.equal(regexFailure.isError, true); - const regexError = regexFailure.structuredContent.error; + const regexError = requireMcpToolError(regexFailure); assert.equal(regexError.code, "resource_constraint"); assert.equal(regexError.details.operation, "search_strings"); assert.equal(regexError.details.resource, "memory"); @@ -503,7 +502,7 @@ export async function verifyGhidraBoundaries( options, ); assert.equal(compileFailure.isError, true); - const compileError = compileFailure.structuredContent.error; + const compileError = requireMcpToolError(compileFailure); assert.equal(compileError.code, "resource_constraint"); assert.equal(compileError.details.operation, "search_procedures"); assert.match(compileError.details.reason, /stack while compiling pattern/u); diff --git a/scripts/lib/real-ghidra-snapshot-lifecycle.mjs b/scripts/lib/real-ghidra-snapshot-lifecycle.mjs index f6456f2637..4a854c7314 100644 --- a/scripts/lib/real-ghidra-snapshot-lifecycle.mjs +++ b/scripts/lib/real-ghidra-snapshot-lifecycle.mjs @@ -1,10 +1,13 @@ +import { + requireMcpToolError, + requireMcpOperationResult, +} from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { access, mkdtemp, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseAnalysisSnapshot } from "../../dist/domain/analysisSnapshot.js"; import { parseEvidenceBundle } from "../../dist/domain/evidenceBundle.js"; -import { requireMcpOperationResult } from "./mcp-verifier-results.mjs"; /** Keep mutable Ghidra observations out of immutable snapshots across target opens. */ export async function verifyGhidraSnapshotLifecycle( @@ -31,7 +34,7 @@ export async function verifyGhidraSnapshotLifecycle( true, `${name} accepted a mutated snapshot binding`, ); - const error = reply.structuredContent?.error; + const error = requireMcpToolError(reply); assert.equal( error?.code, "evidence_integrity_mismatch", diff --git a/scripts/verify-native-calls.mjs b/scripts/verify-native-calls.mjs index 696b6db52f..62c9a2b19c 100644 --- a/scripts/verify-native-calls.mjs +++ b/scripts/verify-native-calls.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "./lib/mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; @@ -232,12 +233,12 @@ const checkHardenedRuntime = async (directory, binary) => { { timeout: CLI_TIMEOUT_MS }, ); assert.equal(deniedMcp.isError, true, JSON.stringify(deniedMcp)); - assert.equal(deniedMcp.structuredContent.error.code, denied.code); + assert.equal(requireMcpToolError(deniedMcp).code, denied.code); assert.match( - deniedMcp.structuredContent.error.details.reason, + requireMcpToolError(deniedMcp).details.reason, /^debugger-attach-denied: /u, ); - assert.ok(deniedMcp.structuredContent.error.details.partial_observation); + assert.ok(requireMcpToolError(deniedMcp).details.partial_observation); }); const allowed = join(directory, "debuggable"); const entitlements = join(directory, "get-task-allow.plist"); diff --git a/scripts/verify-package-mcp.mjs b/scripts/verify-package-mcp.mjs index b2011a27ae..7bb22715dc 100644 --- a/scripts/verify-package-mcp.mjs +++ b/scripts/verify-package-mcp.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "./lib/mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { join } from "node:path"; import { execFile } from "node:child_process"; @@ -50,7 +51,7 @@ const verifyMcpUnknownProvider = async (client, mcpOptions) => { ); if ( unknownProvider.isError !== true || - unknownProvider.structuredContent?.error?.details?.selection_reason !== + requireMcpToolError(unknownProvider)?.details?.selection_reason !== "unknown_provider" ) throw new Error("packaged MCP accepted an unknown analysis provider"); diff --git a/scripts/verify-real-ida.mjs b/scripts/verify-real-ida.mjs index f39c3993b3..ebf33b32f4 100644 --- a/scripts/verify-real-ida.mjs +++ b/scripts/verify-real-ida.mjs @@ -1,4 +1,5 @@ #!/usr/bin/env node +import { requireMcpToolError } from "./lib/mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; @@ -172,7 +173,7 @@ const mcp = async (cliDossier) => { arguments: { address: "not-an-address" }, }); assert.equal(invalid.isError, true); - assert.equal(invalid.structuredContent.error.code, "invalid_request"); + assert.equal(requireMcpToolError(invalid).code, "invalid_request"); await call("close_binary", {}); opened = false; report.checks.push( diff --git a/scripts/verify/android/failure-recovery.mjs b/scripts/verify/android/failure-recovery.mjs index e2c2bf3df6..adcf41d74b 100644 --- a/scripts/verify/android/failure-recovery.mjs +++ b/scripts/verify/android/failure-recovery.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "../../lib/mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { access, @@ -208,13 +209,12 @@ export async function verifyAndroidFailureRecovery({ { timeout: 150_000 }, ); assert.equal(response.isError, true); - assert.equal(response.structuredContent.error.code, "invalid_request"); - assert.deepEqual( - response.structuredContent.error.details.issues[0].path, - ["class_name"], - ); + assert.equal(requireMcpToolError(response).code, "invalid_request"); + assert.deepEqual(requireMcpToolError(response).details.issues[0].path, [ + "class_name", + ]); assert.match( - response.structuredContent.error.details.issues[0].message, + requireMcpToolError(response).details.issues[0].message, /search_android_classes/u, ); assert.doesNotMatch(JSON.stringify(response), /rea doctor/u); @@ -231,8 +231,8 @@ export async function verifyAndroidFailureRecovery({ }, { timeout: 150_000 }, ); - assert.equal(method.structuredContent.error.code, "invalid_request"); - assert.deepEqual(method.structuredContent.error.details.issues[0].path, [ + assert.equal(requireMcpToolError(method).code, "invalid_request"); + assert.deepEqual(requireMcpToolError(method).details.issues[0].path, [ "method_name", ]); await client.ping(); diff --git a/scripts/verify/electron/tool-contracts.mjs b/scripts/verify/electron/tool-contracts.mjs index 6e45365d74..c5afa9f1c6 100644 --- a/scripts/verify/electron/tool-contracts.mjs +++ b/scripts/verify/electron/tool-contracts.mjs @@ -1,3 +1,4 @@ +import { requireMcpToolError } from "../../lib/mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile, spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; @@ -172,11 +173,11 @@ window.loadFile("renderer.html");require("node:fs").writeFileSync(${JSON.stringi { timeout: 150_000 }, ); const capture = active.isError - ? active.structuredContent.error.details.partial_observation.capture + ? requireMcpToolError(active).details.partial_observation.capture : active.structuredContent.normalized_result; if (active.isError) { assert.equal( - active.structuredContent.error.code, + requireMcpToolError(active).code, "cleanup_incomplete", JSON.stringify(active), ); diff --git a/src/server/EvidenceMcpServer.ts b/src/server/EvidenceMcpServer.ts index 571a2d1ec5..f2e2396ddf 100644 --- a/src/server/EvidenceMcpServer.ts +++ b/src/server/EvidenceMcpServer.ts @@ -122,7 +122,18 @@ class ErrorEvidenceTransport implements Transport { : undefined; const tool = id === undefined ? undefined : this.pendingTools.get(id); try { - await this.transport.send(this.recoverError(message, tool), options); + const recovered = this.recoverError(message, tool); + if ( + isJSONRPCResultResponse(recovered) && + isCallToolResult(recovered.result) && + recovered.result.isError === true + ) { + // Keep the complete typed JSON in content. Error projections are not + // success outputSchema data; oversized recovery must consume their + // private structured carrier before it is removed from the wire. + const { structuredContent: _diagnostic, ...result } = recovered.result; + await this.transport.send({ ...recovered, result }, options); + } else await this.transport.send(recovered, options); } finally { if (!isJSONRPCRequest(message) && id !== undefined) this.pendingTools.delete(id); diff --git a/tests/acceptance/nonBinaryComposition.test.ts b/tests/acceptance/nonBinaryComposition.test.ts index 0cb337de7f..b333db20ec 100644 --- a/tests/acceptance/nonBinaryComposition.test.ts +++ b/tests/acceptance/nonBinaryComposition.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -67,8 +68,8 @@ for (const { jarConfigured, setting } of [ arguments: { path: apk }, }); expect(response.isError).toBe(true); - expect(response.structuredContent).toEqual({ error: cliResult.json }); - expect(JSON.stringify(response.structuredContent)).toContain(setting); + expect(parseMcpToolError(response)).toEqual({ error: cliResult.json }); + expect(JSON.stringify(parseMcpToolError(response))).toContain(setting); }, ); @@ -92,8 +93,8 @@ cliTest.skipIf(process.platform !== "linux")( arguments: { path }, }); expect(response.isError).toBe(true); - expect(response.structuredContent).toEqual({ error: cliResult.json }); - expect(JSON.stringify(response.structuredContent)).toContain( + expect(parseMcpToolError(response)).toEqual({ error: cliResult.json }); + expect(JSON.stringify(parseMcpToolError(response))).toContain( "REA_BINWALK_COMMAND", ); }, @@ -145,8 +146,8 @@ cliTest( arguments: { path, output_directory: join(root, "output") }, }); expect(response.isError).toBe(true); - expect(response.structuredContent).toEqual({ error: cliResult.json }); - expect(JSON.stringify(response.structuredContent)).toContain( + expect(parseMcpToolError(response)).toEqual({ error: cliResult.json }); + expect(JSON.stringify(parseMcpToolError(response))).toContain( "REA_WAKARU_COMMAND", ); }, @@ -183,7 +184,7 @@ cliTest( }); expect(result.exitCode).toBe(1); expect(response.isError).toBe(true); - expect(response.structuredContent).toEqual({ error: result.json }); + expect(parseMcpToolError(response)).toEqual({ error: result.json }); expect(result.json).toMatchObject({ code: "invalid_request", details: { diff --git a/tests/acceptance/tools/historicalCaptureResources.test.ts b/tests/acceptance/tools/historicalCaptureResources.test.ts index 057b46133f..7aea0fcce2 100644 --- a/tests/acceptance/tools/historicalCaptureResources.test.ts +++ b/tests/acceptance/tools/historicalCaptureResources.test.ts @@ -7,6 +7,7 @@ import { expect, it } from "vitest"; import { WEB_NETWORK_CAPTURE_LIMITS } from "../../../src/domain/webNetworkCapture.js"; import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; const execute = promisify(execFile); @@ -56,7 +57,7 @@ it("reports real HAR heap exhaustion through CLI and MCP and preserves server us sensitive_values: ["caller-marked"], }); expect(response.isError).toBe(true); - expect(response.structuredContent).toMatchObject({ + expect(parseMcpToolError(response)).toMatchObject({ error: { code: "resource_constraint", retryable: false, @@ -110,7 +111,7 @@ it("reports real HAR heap exhaustion through CLI and MCP and preserves server us capture_path: path, format: "har", }); - expect(oversized.structuredContent).toMatchObject({ + expect(parseMcpToolError(oversized)).toMatchObject({ error: { code: "invalid_request" }, }); }, 60_000); diff --git a/tests/acceptance/tools/inspectorLifecycle.test.ts b/tests/acceptance/tools/inspectorLifecycle.test.ts index 8dc8f27cf2..cf1c791806 100644 --- a/tests/acceptance/tools/inspectorLifecycle.test.ts +++ b/tests/acceptance/tools/inspectorLifecycle.test.ts @@ -6,6 +6,7 @@ import { expect, it, onTestFinished } from "vitest"; import { toolContract } from "../../../src/contracts/toolContracts.js"; import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; it("observes a real Node Inspector and releases attachments after cancellation and failures", async () => { @@ -62,7 +63,7 @@ it("observes a real Node Inspector and releases attachments after cancellation a target_id: "../missing-target", }); expect(missing.isError).toBe(true); - expect(missing.structuredContent).toMatchObject({ + expect(parseMcpToolError(missing)).toMatchObject({ error: { code: "target_unavailable", message: expect.stringContaining("Refresh target discovery"), @@ -114,7 +115,7 @@ it("observes a real Node Inspector and releases attachments after cancellation a inspector_endpoint: endpoint, }); expect(unreachable.isError).toBe(true); - expect(unreachable.structuredContent).toMatchObject({ + expect(parseMcpToolError(unreachable)).toMatchObject({ error: { code: "provider_unavailable", message: expect.stringContaining( diff --git a/tests/acceptance/tools/localToolContracts.test.ts b/tests/acceptance/tools/localToolContracts.test.ts index 218d111c54..7bfc9dfc0e 100644 --- a/tests/acceptance/tools/localToolContracts.test.ts +++ b/tests/acceptance/tools/localToolContracts.test.ts @@ -9,6 +9,7 @@ import { toolContract } from "../../../src/contracts/toolContracts.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { itWithCaptureCapability as captureTest } from "../../boundary/process/processCaptureCapability.js"; import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; const execute = promisify(execFile); @@ -180,7 +181,7 @@ captureTest( const failed = await call("capture_process_scenario", { executable: missing, }); - expect(failed.structuredContent).toMatchObject({ + expect(parseMcpToolError(failed)).toMatchObject({ error: { code: "process_capture_failed", message: expect.stringContaining(missing), diff --git a/tests/boundary/mcp/analysisViewMcp.test.ts b/tests/boundary/mcp/analysisViewMcp.test.ts index 7fbd8b4699..9d188e6c03 100644 --- a/tests/boundary/mcp/analysisViewMcp.test.ts +++ b/tests/boundary/mcp/analysisViewMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { Ajv2020 } from "ajv/dist/2020.js"; import { expect, it, onTestFinished } from "vitest"; @@ -161,7 +162,7 @@ it("retains an oversized selected observation and keeps subsequent views usable" }), }), }) - .parse(response.structuredContent).error.details + .parse(parseMcpToolError(response)).error.details .reported_limits.evidence_reference; expect( session.evidenceById(reference.evidence_id)?.normalized_result, @@ -219,11 +220,8 @@ it("returns a JavaScript summary without graph payloads from a retained referenc view: { kind: "summary" }, }, }); - expect(stale).toMatchObject({ - isError: true, - structuredContent: { - error: { details: { reason: "missing" } }, - }, + expect(parseMcpToolError(stale)).toMatchObject({ + error: { details: { reason: "missing" } }, }); }); diff --git a/tests/boundary/mcp/applicationEvidenceMcp.test.ts b/tests/boundary/mcp/applicationEvidenceMcp.test.ts index f0bcb36e45..6780c9d019 100644 --- a/tests/boundary/mcp/applicationEvidenceMcp.test.ts +++ b/tests/boundary/mcp/applicationEvidenceMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import type { Client } from "@modelcontextprotocol/client"; import { describe, expect, it } from "vitest"; @@ -122,16 +123,13 @@ async function assertRejectedEvidenceReferences( application: record, }, }); - expect(rejected).toMatchObject({ - isError: true, - structuredContent: { - // A recorded Evidence with the wrong operation or predicate is an - // invalid caller input, not a provider failure. - error: { - code: "invalid_request", - category: "invalid_input", - retryable: true, - }, + expect(parseMcpToolError(rejected)).toMatchObject({ + // A recorded Evidence with the wrong operation or predicate is an + // invalid caller input, not a provider failure. + error: { + code: "invalid_request", + category: "invalid_input", + retryable: true, }, }); } diff --git a/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts b/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts index f7c046075b..7405c30c25 100644 --- a/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts +++ b/tests/boundary/mcp/artifactMcp.inlineInventory.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -64,7 +65,7 @@ it("inspects a standalone keyed archive through MCP with original object identit arguments: { path: "other.plist" }, }); expect(rejected.isError).toBe(true); - expect(rejected.structuredContent).toMatchObject({ + expect(parseMcpToolError(rejected)).toMatchObject({ error: { code: "invalid_request", details: { @@ -83,7 +84,7 @@ it("inspects a standalone keyed archive through MCP with original object identit arguments: { root: "missing" }, }); expect(missingRoot.isError).toBe(true); - expect(missingRoot.structuredContent).toMatchObject({ + expect(parseMcpToolError(missingRoot)).toMatchObject({ error: { code: "invalid_request", details: { diff --git a/tests/boundary/mcp/artifactMcp.test.ts b/tests/boundary/mcp/artifactMcp.test.ts index 82837a162b..5b786a2ddb 100644 --- a/tests/boundary/mcp/artifactMcp.test.ts +++ b/tests/boundary/mcp/artifactMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; @@ -64,7 +65,7 @@ it.each([ arguments: {}, }); expect(result.isError).toBe(true); - expect(result.structuredContent).toEqual({ + expect(parseMcpToolError(result)).toEqual({ error: { code: "artifact_integrity_mismatch", category: "integrity_mismatch", diff --git a/tests/boundary/mcp/browserMcp.test.ts b/tests/boundary/mcp/browserMcp.test.ts index 2cd03b0896..379a27d936 100644 --- a/tests/boundary/mcp/browserMcp.test.ts +++ b/tests/boundary/mcp/browserMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; @@ -274,7 +275,7 @@ it("does not attach to a target outside the request's allowed origin scope", asy }, }); expect(result.isError).toBe(true); - expect(result.structuredContent).toMatchObject({ + expect(parseMcpToolError(result)).toMatchObject({ error: { details: { operation: "inspect_web_page", diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index 31ad6e6bf2..0ea1006b34 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -131,7 +132,7 @@ describe("bundle comparison MCP integration", () => { }, }); expect(result.isError).toBe(true); - expect(result.structuredContent).toMatchObject({ + expect(parseMcpToolError(result)).toMatchObject({ error: { code: "evidence_integrity_mismatch", message: expect.stringContaining( diff --git a/tests/boundary/mcp/contractPresentation.test.ts b/tests/boundary/mcp/contractPresentation.test.ts index 2a3c9f7ec2..9d1d9d6bb0 100644 --- a/tests/boundary/mcp/contractPresentation.test.ts +++ b/tests/boundary/mcp/contractPresentation.test.ts @@ -1,10 +1,12 @@ import { execFile } from "node:child_process"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; +import { isCallToolResult } from "@modelcontextprotocol/server"; import { expect, it } from "vitest"; import { z } from "zod"; import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; const execute = promisify(execFile); const fixture = fileURLToPath( @@ -41,6 +43,8 @@ it("preserves advertised schemas and handler meaning when presentation order cha expect(advertised.outputSchema).toEqual(canonical?.outputSchema); } for (const { name, response } of result.responses) { + if (!isCallToolResult(response)) + throw new Error(`Invalid tools/call fixture result for ${name}`); if (name === "reconcile_javascript_runtime") { expect(response.isError).not.toBe(true); expect(response.structuredContent).toMatchObject({ @@ -48,7 +52,7 @@ it("preserves advertised schemas and handler meaning when presentation order cha }); } else { expect(response.isError, name).toBe(true); - expect(response.structuredContent, name).toMatchObject({ + expect(parseMcpToolError(response), name).toMatchObject({ error: { details: { operation: name } }, }); } diff --git a/tests/boundary/mcp/dylibResolutionMcp.test.ts b/tests/boundary/mcp/dylibResolutionMcp.test.ts index 5fb3262b66..34db785e33 100644 --- a/tests/boundary/mcp/dylibResolutionMcp.test.ts +++ b/tests/boundary/mcp/dylibResolutionMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { chmod, mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -107,7 +108,7 @@ it("traces dylib resolution for an opened app bundle and rejects a changed targe arguments: {}, }); expect(changed.isError).toBe(true); - expect(JSON.stringify(changed.structuredContent)).toContain("integrity"); + expect(JSON.stringify(parseMcpToolError(changed))).toContain("integrity"); }); }); @@ -126,7 +127,7 @@ it("rejects targets that are not Mach-O", async () => { arguments: {}, }); expect(called.isError).toBe(true); - expect(called.structuredContent).toMatchObject({ + expect(parseMcpToolError(called)).toMatchObject({ error: { code: "unsupported_target", category: "unsupported_target" }, }); }); @@ -153,7 +154,7 @@ it("reports target kinds that artifact operations cannot inspect as unsupported" const open = { name: "open_binary", arguments: { path } }; expect((await client.callTool(open)).isError, path).not.toBe(true); const called = await client.callTool({ name: operation, arguments: {} }); - expect(called.structuredContent, operation).toMatchObject({ + expect(parseMcpToolError(called), operation).toMatchObject({ error: { code: "unsupported_target", message: expect.stringContaining(`${operation} at ${path}`), @@ -230,7 +231,7 @@ it.skipIf(process.getuid?.() === 0)( arguments: {}, }); expect(called.isError).toBe(true); - expect(JSON.stringify(called.structuredContent)).toMatch( + expect(JSON.stringify(parseMcpToolError(called))).toMatch( /Permission denied \(EACCES\)/u, ); } finally { @@ -442,7 +443,7 @@ it("reports caller-selected roots that cannot be traced as invalid input", async arguments: { roots: ["Contents/MacOS/App", root] }, }); expect(called.isError, root).toBe(true); - expect(called.structuredContent, root).toMatchObject({ + expect(parseMcpToolError(called), root).toMatchObject({ error: { code: "invalid_request", details: { diff --git a/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts b/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts index 7e024f604a..79002f40e2 100644 --- a/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts +++ b/tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { afterEach, describe, expect, it } from "vitest"; import { @@ -109,7 +110,7 @@ describe("enhanced MCP tools", () => { expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); expect(text?.type === "text" ? text.text : "").toBe( - JSON.stringify(result.structuredContent), + JSON.stringify(parseMcpToolError(result)), ); }); }); diff --git a/tests/boundary/mcp/enhancedTools.06-rejects-incomplete-function-dossiers-at.test.ts b/tests/boundary/mcp/enhancedTools.06-rejects-incomplete-function-dossiers-at.test.ts index d0ac870fc6..212e32824d 100644 --- a/tests/boundary/mcp/enhancedTools.06-rejects-incomplete-function-dossiers-at.test.ts +++ b/tests/boundary/mcp/enhancedTools.06-rejects-incomplete-function-dossiers-at.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { afterEach, describe, expect, it } from "vitest"; import { functionDossierSchema } from "../../../src/domain/hopperValues.js"; @@ -26,7 +27,7 @@ describe("analyze_function MCP producer validation", () => { expect(result.isError).toBe(true); expect(result.content).toContainEqual({ type: "text", - text: JSON.stringify(result.structuredContent), + text: JSON.stringify(parseMcpToolError(result)), }); }); @@ -55,7 +56,7 @@ describe("analyze_function MCP producer validation", () => { expect(result.isError).toBe(true); expect(result.content).toContainEqual({ type: "text", - text: JSON.stringify(result.structuredContent), + text: JSON.stringify(parseMcpToolError(result)), }); }); }); diff --git a/tests/boundary/mcp/enhancedTools.inputValidation.test.ts b/tests/boundary/mcp/enhancedTools.inputValidation.test.ts index f6c881c374..a88361a01e 100644 --- a/tests/boundary/mcp/enhancedTools.inputValidation.test.ts +++ b/tests/boundary/mcp/enhancedTools.inputValidation.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { afterEach, describe, expect, it } from "vitest"; import { closeEnhancedToolResources, connect } from "./enhancedToolsHarness.js"; @@ -41,7 +42,7 @@ describe("enhanced MCP input validation", () => { expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); expect(text?.type === "text" ? text.text : "").toBe( - JSON.stringify(result.structuredContent), + JSON.stringify(parseMcpToolError(result)), ); }); }); diff --git a/tests/boundary/mcp/enhancedToolsHarness.ts b/tests/boundary/mcp/enhancedToolsHarness.ts index 2bb3bd3cab..26bb194ab8 100644 --- a/tests/boundary/mcp/enhancedToolsHarness.ts +++ b/tests/boundary/mcp/enhancedToolsHarness.ts @@ -1,6 +1,10 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import type { CallToolResult } from "@modelcontextprotocol/server"; -import { type JsonValue } from "../../../src/domain/jsonValue.js"; +import { + jsonValueSchema, + type JsonValue, +} from "../../../src/domain/jsonValue.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import type { AnalysisOperationPort } from "../../../src/application/AnalysisProvider.js"; import { createServer } from "../../../src/server/createServer.js"; @@ -38,4 +42,6 @@ export const closeEnhancedToolResources = async (): Promise => { }; export const jsonResult = (result: CallToolResult): JsonValue => - parseEvidence(result.structuredContent).normalized_result; + result.isError === true + ? jsonValueSchema.parse(parseMcpToolError(result)) + : parseEvidence(result.structuredContent).normalized_result; diff --git a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts index fc2b08b157..4d6d3da237 100644 --- a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts +++ b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { describe, expect, it } from "vitest"; import { EnhancedTools } from "../../../src/application/EnhancedTools.js"; import { connectGhidraMcp, sessionEvidence } from "./ghidraMcpHarness.js"; @@ -52,7 +53,7 @@ it("rejects contradictory annotation readback across the provider and MCP bounda }, }); expect(reply.isError).toBe(true); - expect(reply.structuredContent).toMatchObject({ + expect(parseMcpToolError(reply)).toMatchObject({ error: { code: "unreadable_output" }, }); } diff --git a/tests/boundary/mcp/ghidraFailureDiagnostics.test.ts b/tests/boundary/mcp/ghidraFailureDiagnostics.test.ts index 90419ac7e2..d6d5eeb51d 100644 --- a/tests/boundary/mcp/ghidraFailureDiagnostics.test.ts +++ b/tests/boundary/mcp/ghidraFailureDiagnostics.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { expect, it } from "vitest"; import { logCliCommand } from "../../../src/cliLogging.js"; @@ -64,7 +65,7 @@ it("preserves queue failure diagnostics through the provider, CLI adapter and SD arguments: {}, }); expect(mcp.isError).toBe(true); - expect(mcp.structuredContent).toMatchObject({ error: parsed }); + expect(parseMcpToolError(mcp)).toMatchObject({ error: parsed }); expect(mcp.content).toContainEqual( expect.objectContaining({ type: "text", diff --git a/tests/boundary/mcp/ghidraInventoryMcp.test.ts b/tests/boundary/mcp/ghidraInventoryMcp.test.ts index ecf7ffed0f..23bb6035f5 100644 --- a/tests/boundary/mcp/ghidraInventoryMcp.test.ts +++ b/tests/boundary/mcp/ghidraInventoryMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { expect, it } from "vitest"; import { jsonValueSchema } from "../../../src/domain/jsonValue.js"; import { ok } from "../../../src/domain/result.js"; @@ -100,11 +101,11 @@ it("rejects contradictory provider output before emitting MCP Evidence", async ( arguments: probe.arguments, }); expect(rejected.isError, probe.name).toBe(true); - expect(rejected.structuredContent).toMatchObject({ + expect(parseMcpToolError(rejected)).toMatchObject({ error: { code: "unreadable_output" }, }); - expect(rejected.structuredContent).not.toHaveProperty("evidence_id"); - expect(rejected.structuredContent).not.toHaveProperty("result"); + expect(parseMcpToolError(rejected)).not.toHaveProperty("evidence_id"); + expect(parseMcpToolError(rejected)).not.toHaveProperty("result"); } } finally { await harness.close(); diff --git a/tests/boundary/mcp/ghidraRoutingMcp.test.ts b/tests/boundary/mcp/ghidraRoutingMcp.test.ts index 9848b3c6f3..f4ed96938a 100644 --- a/tests/boundary/mcp/ghidraRoutingMcp.test.ts +++ b/tests/boundary/mcp/ghidraRoutingMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { describe, expect, it } from "vitest"; import { connectGhidraMcp } from "./ghidraMcpHarness.js"; @@ -28,7 +29,7 @@ describe("Ghidra MCP capability routing", () => { arguments: { address: "0x1000", comment: "must not mutate" }, }); expect(denied.isError).toBe(true); - expect(denied.structuredContent).toMatchObject({ + expect(parseMcpToolError(denied)).toMatchObject({ error: { code: "capability_unavailable", category: "unsupported_provider", diff --git a/tests/boundary/mcp/largeToolResult.test.ts b/tests/boundary/mcp/largeToolResult.test.ts index 76122951a5..00afa2d7ac 100644 --- a/tests/boundary/mcp/largeToolResult.test.ts +++ b/tests/boundary/mcp/largeToolResult.test.ts @@ -25,6 +25,7 @@ import { registerOfficialTools } from "../../../src/server/registerOfficialTools import { ToolResultDelivery } from "../../../src/server/toolResult.js"; import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { encodeToolResult } from "../../../src/server/toolResultEncoding.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; const delivery = new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE); @@ -188,7 +189,7 @@ describe("large complete Evidence MCP delivery", () => { }), }), }) - .parse(result.structuredContent).error.details.reported_limits; + .parse(parseMcpToolError(result)).error.details.reported_limits; const retained = parseEvidence( records.get(limits.evidence_reference.evidence_id), ); diff --git a/tests/boundary/mcp/managedMcp.test.ts b/tests/boundary/mcp/managedMcp.test.ts index 2b71c69286..3d2524eb0f 100644 --- a/tests/boundary/mcp/managedMcp.test.ts +++ b/tests/boundary/mcp/managedMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { parseEvidence } from "../../../src/domain/evidence.js"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -218,11 +219,8 @@ const verifyManagedCatalogAndNativeWorkflow = async ( ], }, }); - expect(wrong).toMatchObject({ - isError: true, - structuredContent: { - error: { code: "evidence_integrity_mismatch" }, - }, + expect(parseMcpToolError(wrong)).toMatchObject({ + error: { code: "evidence_integrity_mismatch" }, }); }; @@ -426,6 +424,7 @@ const verifyImport = async ( }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null diff --git a/tests/boundary/mcp/nativeCallObservationMcp.test.ts b/tests/boundary/mcp/nativeCallObservationMcp.test.ts index e6fcb9ccb4..284d6ce438 100644 --- a/tests/boundary/mcp/nativeCallObservationMcp.test.ts +++ b/tests/boundary/mcp/nativeCallObservationMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createHash } from "node:crypto"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -260,7 +261,7 @@ it.each([ }); expect(called.isError).toBe(true); - expect(called.structuredContent).toMatchObject({ + expect(parseMcpToolError(called)).toMatchObject({ error: { code: expectedCode, details: { diff --git a/tests/boundary/mcp/nativeSignatureTargetBindingMcp.test.ts b/tests/boundary/mcp/nativeSignatureTargetBindingMcp.test.ts index 8222fa9f92..4468491c26 100644 --- a/tests/boundary/mcp/nativeSignatureTargetBindingMcp.test.ts +++ b/tests/boundary/mcp/nativeSignatureTargetBindingMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; @@ -40,7 +41,7 @@ it("rejects changed signature targets through MCP and accepts reopening the new arguments: {}, }); expect(changed.isError).toBe(true); - expect(changed.structuredContent).toMatchObject({ + expect(parseMcpToolError(changed)).toMatchObject({ error: { code: "artifact_changed", details: { path } }, }); expect((await open()).isError).not.toBe(true); diff --git a/tests/boundary/mcp/officialTools.test.ts b/tests/boundary/mcp/officialTools.test.ts index fa409a2092..95b7413135 100644 --- a/tests/boundary/mcp/officialTools.test.ts +++ b/tests/boundary/mcp/officialTools.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Ajv2020 } from "ajv/dist/2020.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { afterEach, describe, expect, it } from "vitest"; @@ -140,7 +141,7 @@ describe("official Hopper proxy tools", () => { expect(result.isError).toBe(true); expect(result.content[0]).toEqual({ type: "text", - text: JSON.stringify(result.structuredContent), + text: JSON.stringify(parseMcpToolError(result)), }); }); diff --git a/tests/boundary/mcp/optionalProviderIsolation.test.ts b/tests/boundary/mcp/optionalProviderIsolation.test.ts index b3215aefad..7ede388744 100644 --- a/tests/boundary/mcp/optionalProviderIsolation.test.ts +++ b/tests/boundary/mcp/optionalProviderIsolation.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { McpServer } from "@modelcontextprotocol/server"; @@ -138,7 +139,7 @@ mcpTest.for(failedAdapters)( arguments: example.input, }); expect(unavailable.isError).toBe(true); - expect(unavailable.structuredContent).toMatchObject({ + expect(parseMcpToolError(unavailable)).toMatchObject({ error: { details: { provider_id: providerId, diff --git a/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts index 6bcec278b8..ea2809b7ce 100644 --- a/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts +++ b/tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { STDIO_DEFAULT_MAX_BUFFER_SIZE, @@ -9,7 +10,8 @@ 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 type { Evidence } from "../../../src/domain/evidence.js"; +import { err, ok } from "../../../src/domain/result.js"; import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { ToolResultDelivery } from "../../../src/server/toolResult.js"; @@ -28,6 +30,7 @@ const oversizedFailure = () => const exerciseDeliveryFailure = async ( recordEvidence: EvidenceWriter["recordEvidence"] | undefined, + retention: "retained" | "unavailable" | "failed", ) => { const server = new EvidenceMcpServer( { name: "oversized-error-test", version: "1" }, @@ -57,7 +60,8 @@ const exerciseDeliveryFailure = async ( arguments: {}, }); expect(failure.isError).toBe(true); - expect(failure.structuredContent).toMatchObject({ + expect(failure.structuredContent).toBeUndefined(); + expect(parseMcpToolError(failure)).toMatchObject({ error: { code: "resource_constraint", details: { @@ -76,8 +80,8 @@ const exerciseDeliveryFailure = async ( }), }), }) - .parse(failure.structuredContent).error.details.reported_limits; - expect(failure.structuredContent).toMatchObject({ + .parse(parseMcpToolError(failure)).error.details.reported_limits; + expect(parseMcpToolError(failure)).toMatchObject({ error: { details: { reported_limits: { @@ -86,11 +90,17 @@ const exerciseDeliveryFailure = async ( }, }, }); - expect(reportedLimits).not.toHaveProperty("evidence_reference"); - expect(reportedLimits.retention).toBe( - recordEvidence === undefined ? "unavailable" : "failed", - ); - if (recordEvidence !== undefined) + if (retention === "retained") { + expect(reportedLimits.evidence_reference).toMatchObject({ + kind: "retained-evidence", + evidence_id: expect.any(String), + }); + expect(reportedLimits).not.toHaveProperty("retention"); + } else { + expect(reportedLimits).not.toHaveProperty("evidence_reference"); + expect(reportedLimits.retention).toBe(retention); + } + if (retention === "failed") expect(reportedLimits.retention_error).toMatchObject({ code: "evidence_integrity_mismatch", }); @@ -109,10 +119,27 @@ const exerciseDeliveryFailure = async ( }; 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"), - ), + await exerciseDeliveryFailure(undefined, "unavailable"); + await exerciseDeliveryFailure( + () => + err( + new EvidenceIntegrityError( + "fixture ledger rejected the Evidence record", + ), + ), + "failed", ); }); + +it("retains the complete oversized diagnostic before removing its private structured carrier", async () => { + const retained: Evidence[] = []; + await exerciseDeliveryFailure((evidence) => { + retained.push(evidence); + return ok("added"); + }, "retained"); + expect(retained).toHaveLength(1); + expect(retained[0]?.normalized_result).toEqual( + oversizedFailure().structuredContent, + ); + expect(retained[0]?.raw_result).toEqual(oversizedFailure()); +}); diff --git a/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts b/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts index 81722a3847..3d229b2409 100644 --- a/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts +++ b/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -70,7 +71,7 @@ it("names differing comparison contract fields through MCP and the CLI", async ( ), expected: ["working_directory"], }; - expect(response.structuredContent).toMatchObject({ + expect(parseMcpToolError(response)).toMatchObject({ error: { code: "invalid_request", details: { issues: [issue] } }, }); @@ -97,7 +98,7 @@ it("names the stale capture that exceeds max_capture_age_ms", async () => { }, }); expect(response.isError).toBe(true); - expect(response.structuredContent).toMatchObject({ + expect(parseMcpToolError(response)).toMatchObject({ error: { code: "invalid_request", details: { @@ -146,7 +147,7 @@ it("names the side and constraint when Evidence is not a usable capture", async name: "compare_process_captures", arguments: { left: unrelated, right: capture }, }); - expect(wrongKind.structuredContent).toMatchObject({ + expect(parseMcpToolError(wrongKind)).toMatchObject({ error: { code: "invalid_request", details: { @@ -170,7 +171,7 @@ it("names the side and constraint when Evidence is not a usable capture", async right: { ...capture, parameters: { side: "changed" } }, }, }); - expect(tampered.structuredContent).toMatchObject({ + expect(parseMcpToolError(tampered)).toMatchObject({ error: { code: "evidence_integrity_mismatch", message: expect.stringContaining( @@ -189,7 +190,7 @@ it("names the side and constraint when Evidence is not a usable capture", async name: "compare_process_captures", arguments: { left: capture, right: invalidResult }, }); - expect(malformed.structuredContent).toMatchObject({ + expect(parseMcpToolError(malformed)).toMatchObject({ error: { code: "evidence_integrity_mismatch", message: expect.stringContaining( diff --git a/tests/boundary/mcp/processEvidenceIdentityMcp.test.ts b/tests/boundary/mcp/processEvidenceIdentityMcp.test.ts index 1081327d58..ceaf37480e 100644 --- a/tests/boundary/mcp/processEvidenceIdentityMcp.test.ts +++ b/tests/boundary/mcp/processEvidenceIdentityMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -290,7 +291,7 @@ for (const name of providerNames) for (const { expected: _expected, ...call } of consumerCalls(invalid)) { const response = await client.callTool(call); expect(response.isError, call.name).toBe(true); - expect(response.structuredContent, call.name).toMatchObject({ + expect(parseMcpToolError(response), call.name).toMatchObject({ error: { code: "invalid_request" }, }); } @@ -329,7 +330,7 @@ it("separately rejects a tampered digest before admission to any consumer or ses )) { const response = await client.callTool(call); expect(response.isError, call.name).toBe(true); - expect(response.structuredContent).toMatchObject({ + expect(parseMcpToolError(response)).toMatchObject({ error: { code: "invalid_request" }, }); } diff --git a/tests/boundary/mcp/providerSelectionMcp.test.ts b/tests/boundary/mcp/providerSelectionMcp.test.ts index 6061231615..5d30a4e377 100644 --- a/tests/boundary/mcp/providerSelectionMcp.test.ts +++ b/tests/boundary/mcp/providerSelectionMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -228,4 +229,6 @@ const capability = (provider: ProviderIdentity): CapabilityDescriptor => ({ }); const structured = (result: CallToolResult): Record => - z.record(z.string(), z.unknown()).parse(result.structuredContent); + result.isError === true + ? parseMcpToolError(result) + : z.record(z.string(), z.unknown()).parse(result.structuredContent); diff --git a/tests/boundary/mcp/reconstructionCoverageMcp.test.ts b/tests/boundary/mcp/reconstructionCoverageMcp.test.ts index 8cf0458ffe..50dd2508ab 100644 --- a/tests/boundary/mcp/reconstructionCoverageMcp.test.ts +++ b/tests/boundary/mcp/reconstructionCoverageMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { describe, expect, it } from "vitest"; @@ -43,7 +44,7 @@ describe("reconstruction coverage MCP", () => { }, }); expect(unknown.isError).toBe(true); - expect(unknown.structuredContent).toMatchObject({ + expect(parseMcpToolError(unknown)).toMatchObject({ error: { code: "invalid_request", details: { diff --git a/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts b/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts index 3173a0a63a..e9c0dd2227 100644 --- a/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts +++ b/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { afterEach, expect, it } from "vitest"; import { Ajv2020 } from "ajv/dist/2020.js"; @@ -136,15 +137,12 @@ it("keeps references session-scoped and expires them when close_binary clears th }, }; const foreign = await second.client.callTool(request); - expect(foreign).toMatchObject({ - isError: true, - structuredContent: { - error: { - details: { - evidence_id: application.evidence_id, - reason: "missing", - actual: null, - }, + expect(parseMcpToolError(foreign)).toMatchObject({ + error: { + details: { + evidence_id: application.evidence_id, + reason: "missing", + actual: null, }, }, }); @@ -156,10 +154,9 @@ it("keeps references session-scoped and expires them when close_binary clears th arguments: {}, }); expect(closed.isError).not.toBe(true); - expect(await first.client.callTool(request)).toMatchObject({ - isError: true, - structuredContent: { error: { details: { reason: "missing" } } }, - }); + expect(parseMcpToolError(await first.client.callTool(request))).toMatchObject( + { error: { details: { reason: "missing" } } }, + ); for (const harness of [first, second]) { const portable = await harness.client.callTool({ name: "trace_application_feature", @@ -199,11 +196,8 @@ it("preserves validation for the referenced record's operation, predicate, and s application: retained(record.evidence_id), }, }); - expect(response).toMatchObject({ - isError: true, - structuredContent: { - error: { code: "invalid_request", category: "invalid_input" }, - }, + expect(parseMcpToolError(response)).toMatchObject({ + error: { code: "invalid_request", category: "invalid_input" }, }); } for (const reference of [ diff --git a/tests/boundary/mcp/serverIdentity.test.ts b/tests/boundary/mcp/serverIdentity.test.ts index 9486496ce5..1c28ddeeb4 100644 --- a/tests/boundary/mcp/serverIdentity.test.ts +++ b/tests/boundary/mcp/serverIdentity.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -275,7 +276,7 @@ describe("active-target availability over MCP", () => { name: "inspect_macho", arguments: {}, }); - expect(call.structuredContent).toMatchObject({ + expect(parseMcpToolError(call)).toMatchObject({ error: { code: "target_unavailable" }, }); await openTarget(client, plist); diff --git a/tests/boundary/mcp/serverLifecycleMcp.test.ts b/tests/boundary/mcp/serverLifecycleMcp.test.ts index ae29ee2fc8..da4520307c 100644 --- a/tests/boundary/mcp/serverLifecycleMcp.test.ts +++ b/tests/boundary/mcp/serverLifecycleMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import type { CallToolResult } from "@modelcontextprotocol/server"; import { afterEach, expect, it } from "vitest"; @@ -37,6 +38,7 @@ const text = (result: CallToolResult): string => { }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null @@ -61,7 +63,7 @@ it("projects remote failures without provider or bridge details", async () => { category: "execution_failure", }, }); - expect(text(result)).toBe(JSON.stringify(result.structuredContent)); + expect(text(result)).toBe(JSON.stringify(parseMcpToolError(result))); }); it("lets the SDK validate a tool call before invoking its handler", async () => { diff --git a/tests/boundary/mcp/serverUnknownsMcp.test.ts b/tests/boundary/mcp/serverUnknownsMcp.test.ts index 1d757ed86b..70d8417b69 100644 --- a/tests/boundary/mcp/serverUnknownsMcp.test.ts +++ b/tests/boundary/mcp/serverUnknownsMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import type { CallToolResult } from "@modelcontextprotocol/server"; import { afterEach, expect, it } from "vitest"; @@ -68,6 +69,7 @@ const providerWithCapabilities = ( }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null diff --git a/tests/boundary/mcp/serverWorkflowMcp.test.ts b/tests/boundary/mcp/serverWorkflowMcp.test.ts index e70ae0e566..aa57b58c4f 100644 --- a/tests/boundary/mcp/serverWorkflowMcp.test.ts +++ b/tests/boundary/mcp/serverWorkflowMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { ok as resultOk } from "../../../src/domain/result.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import type { CallToolResult } from "@modelcontextprotocol/server"; @@ -37,6 +38,7 @@ const connect = async (analysis: AnalysisOperationPort) => { }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null diff --git a/tests/boundary/mcp/sessionMcp.test.ts b/tests/boundary/mcp/sessionMcp.test.ts index 20f1424734..33eff767a1 100644 --- a/tests/boundary/mcp/sessionMcp.test.ts +++ b/tests/boundary/mcp/sessionMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { readFile, realpath, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; @@ -496,6 +497,7 @@ const text = (result: CallToolResult): string => { }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null diff --git a/tests/boundary/mcp/sessionMcpHarness.ts b/tests/boundary/mcp/sessionMcpHarness.ts index cb50879ea3..8e45de5c75 100644 --- a/tests/boundary/mcp/sessionMcpHarness.ts +++ b/tests/boundary/mcp/sessionMcpHarness.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -151,6 +152,7 @@ export const snapshotAndRecordUnknown = async ( }; const structured = (result: CallToolResult): Record => { + if (result.isError === true) return parseMcpToolError(result); if ( typeof result.structuredContent !== "object" || result.structuredContent === null diff --git a/tests/boundary/mcp/toolErrorSchemaDelivery.test.ts b/tests/boundary/mcp/toolErrorSchemaDelivery.test.ts new file mode 100644 index 0000000000..af1952d8fc --- /dev/null +++ b/tests/boundary/mcp/toolErrorSchemaDelivery.test.ts @@ -0,0 +1,50 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { Ajv2020 } from "ajv/dist/2020.js"; +import { expect, it, onTestFinished } from "vitest"; +import { z } from "zod"; + +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; + +it("delivers typed tool failures without violating the advertised success schema over stdio", async () => { + const root = await mkdtemp(join(tmpdir(), "rea-tool-error-schema-")); + onTestFinished(() => rm(root, { recursive: true, force: true })); + const missing = join(root, "not-present.bin"); + const { client, call } = await connectLocalToolsMcp(); + const tools = (await client.listTools()).tools; + const failures: string[] = []; + const requests = [ + { name: "list_documents", arguments: {} }, + { name: "open_binary", arguments: { path: missing } }, + { + name: "analyze_javascript_application", + arguments: { input_path: missing }, + }, + { name: "inspect_artifact", arguments: {} }, + ]; + for (const request of requests) { + const tool = tools.find(({ name }) => name === request.name); + if (tool?.outputSchema === undefined) + throw new Error(`${request.name} omitted its output schema`); + const reply = await client.callTool(request); + const diagnostic = parseMcpToolError(reply); + expect(diagnostic.error.message, request.name).not.toBe(""); + if ("path" in request.arguments || "input_path" in request.arguments) + expect(JSON.stringify(diagnostic), request.name).toContain(missing); + if (reply.structuredContent !== undefined) { + const validate = new Ajv2020({ + strict: false, + validateFormats: false, + }).compile(z.record(z.string(), z.unknown()).parse(tool.outputSchema)); + if (!validate(reply.structuredContent)) failures.push(request.name); + } + } + expect(failures).toEqual([]); + const successful = await call("get_evidence_bundle", {}); + expect(successful.isError).not.toBe(true); + expect(successful.structuredContent).toBeDefined(); + await client.ping(); +}); diff --git a/tests/boundary/mcp/webNetworkCaptureMcp.test.ts b/tests/boundary/mcp/webNetworkCaptureMcp.test.ts index 46ec0bf6ab..b0ec2fb5cc 100644 --- a/tests/boundary/mcp/webNetworkCaptureMcp.test.ts +++ b/tests/boundary/mcp/webNetworkCaptureMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { Ajv2020 } from "ajv/dist/2020.js"; import { expect, it, onTestFinished } from "vitest"; @@ -92,7 +93,7 @@ it("publishes valid SDK schemas and retains historical inline Evidence with the arguments: { ...args, sensitive_values: ["secret"], mysecret: true }, }); expect(invalidSensitive.isError).toBe(true); - expect(invalidSensitive.structuredContent).toMatchObject({ + expect(parseMcpToolError(invalidSensitive)).toMatchObject({ error: { code: "invalid_request", details: { issues: [{ path: [] }] } }, }); expect(JSON.stringify(invalidSensitive)).not.toContain("secret"); diff --git a/tests/fixtures/localToolsMcp.ts b/tests/fixtures/localToolsMcp.ts index 53a84b6c98..2b08642ac4 100644 --- a/tests/fixtures/localToolsMcp.ts +++ b/tests/fixtures/localToolsMcp.ts @@ -47,11 +47,7 @@ export async function connectLocalToolsMcp() { const call = async (name: string, arguments_: Record) => { const response = await client.callTool({ name, arguments: arguments_ }); const validate = validators.get(name); - if ( - response.isError !== true && - response.structuredContent !== undefined && - validate !== undefined - ) + if (response.structuredContent !== undefined && validate !== undefined) expect( validate(response.structuredContent), JSON.stringify(validate.errors), diff --git a/tests/fixtures/mcpToolError.ts b/tests/fixtures/mcpToolError.ts new file mode 100644 index 0000000000..d6b45fa920 --- /dev/null +++ b/tests/fixtures/mcpToolError.ts @@ -0,0 +1,16 @@ +import type { CallToolResult } from "@modelcontextprotocol/server"; +import { expect } from "vitest"; +import { z } from "zod"; + +import { analysisErrorProjectionSchema } from "../../src/contracts/errorSchemas.js"; + +/** Read and validate the complete typed diagnostic in an MCP tool failure. */ +export function parseMcpToolError(result: CallToolResult) { + expect(result.isError).toBe(true); + const text = result.content.find((item) => item.type === "text"); + if (text?.type !== "text") throw new Error("Tool error omitted text content"); + return z + .object({ error: analysisErrorProjectionSchema }) + .strict() + .parse(JSON.parse(text.text)); +} diff --git a/tests/process-global/mcpDeliveryEnvironment.test.ts b/tests/process-global/mcpDeliveryEnvironment.test.ts index acf0796b0b..33bb18ed51 100644 --- a/tests/process-global/mcpDeliveryEnvironment.test.ts +++ b/tests/process-global/mcpDeliveryEnvironment.test.ts @@ -9,6 +9,7 @@ import { ok } from "../../src/domain/result.js"; import { run } from "../../src/main.js"; import { createServer } from "../../src/server/createServer.js"; import type { EvidenceMcpServer } from "../../src/server/EvidenceMcpServer.js"; +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; const resources: Array<{ close(): Promise }> = []; afterEach(async () => { @@ -89,8 +90,10 @@ it("keeps independent server budgets for successful results and oversized errors }), ), ); + if (smallResult === undefined || largeResult === undefined) + throw new Error("Missing result from selected-budget server"); expect(smallResult?.isError).toBe(true); - expect(smallResult?.structuredContent).toMatchObject({ + expect(parseMcpToolError(smallResult)).toMatchObject({ error: { code: "resource_constraint", details: { @@ -110,7 +113,9 @@ it("keeps independent server budgets for successful results and oversized errors client.callTool({ name: "selected_failure", arguments: {} }), ), ); - expect(smallFailure?.structuredContent).toMatchObject({ + if (smallFailure === undefined || largeFailure === undefined) + throw new Error("Missing failure from selected-budget server"); + expect(parseMcpToolError(smallFailure)).toMatchObject({ error: { code: "resource_constraint", details: { @@ -122,7 +127,7 @@ it("keeps independent server budgets for successful results and oversized errors }, }, }); - expect(largeFailure?.structuredContent).toMatchObject({ + expect(parseMcpToolError(largeFailure)).toMatchObject({ error: { code: "invalid_request", details: { issues: [{ message: diagnostic }] },