From da606ff77718d9a3b10426fcf68c098138e6dcfa Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 12:26:27 +0000 Subject: [PATCH 1/7] 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) --- docs/mcp-contracts.md | 9 +++ scripts/lib/browser-screenshot-e2e.mjs | 5 +- scripts/lib/ghidra-reference-snapshot-e2e.mjs | 7 ++- scripts/lib/mcp-verifier-results.mjs | 8 +++ .../lib/real-ghidra-address-boundaries.mjs | 3 +- scripts/lib/real-ghidra-boundaries.mjs | 30 +++++----- .../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 ++--- 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 +- 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 | 3 + 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/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 ++++++ 50 files changed, 295 insertions(+), 139 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 994f301a71..f4db6cfd44 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 9eed016c4d..14232e14ea 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"; @@ -161,11 +162,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 905ed6b021..7dcb81a48a 100644 --- a/scripts/lib/ghidra-reference-snapshot-e2e.mjs +++ b/scripts/lib/ghidra-reference-snapshot-e2e.mjs @@ -1,3 +1,7 @@ +import { + requireMcpToolError, + requireMcpResult, +} from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; @@ -15,7 +19,6 @@ import { } from "../../dist/domain/analysisSnapshot.js"; import { createEvidence } from "../../dist/domain/evidence.js"; import { createEvidenceBundle } from "../../dist/domain/evidenceBundle.js"; -import { requireMcpResult } from "./mcp-verifier-results.mjs"; /** Reject a valid older snapshot whose instruction-only collector omitted a real edge. */ export async function verifyLegacyGhidraReferenceSnapshot( @@ -120,7 +123,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 f9e2d70806..8375bea879 100644 --- a/scripts/lib/mcp-verifier-results.mjs +++ b/scripts/lib/mcp-verifier-results.mjs @@ -1,3 +1,5 @@ +import { analysisErrorProjectionSchema } from "../../dist/contracts/errorSchemas.js"; + /** Read the textual JSON projection from one MCP tool call. */ export const mcpTextValue = (result) => { const text = result.content?.find((item) => item.type === "text")?.text; @@ -7,6 +9,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); +}; + /** Return the compact result projection from one successful MCP tool call. */ export const requireMcpResult = (result, operation) => { if (result.isError === true) { diff --git a/scripts/lib/real-ghidra-address-boundaries.mjs b/scripts/lib/real-ghidra-address-boundaries.mjs index 032e65be84..eb9f024808 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 2aa9a11066..2535b10df3 100644 --- a/scripts/lib/real-ghidra-boundaries.mjs +++ b/scripts/lib/real-ghidra-boundaries.mjs @@ -1,3 +1,8 @@ +import { + requireMcpToolError, + mcpTextValue, + requireMcpResult, +} from "./mcp-verifier-results.mjs"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; @@ -7,8 +12,7 @@ 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, requireMcpResult } 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"; @@ -58,23 +62,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)( @@ -478,7 +478,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"); @@ -500,7 +500,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 7333b72f50..78cdd1aab0 100644 --- a/scripts/lib/real-ghidra-snapshot-lifecycle.mjs +++ b/scripts/lib/real-ghidra-snapshot-lifecycle.mjs @@ -1,10 +1,13 @@ +import { + requireMcpToolError, + requireMcpResult, +} 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 { requireMcpResult } 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 3b5be1b5d6..215d4aabbe 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 e457f1b19e..0076baa42f 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 42f241d38a..f099113808 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, @@ -190,13 +191,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); @@ -213,8 +213,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 7404238dc9..588580dd47 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.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 9e9fbae493..efbf03f8cc 100644 --- a/src/server/EvidenceMcpServer.ts +++ b/src/server/EvidenceMcpServer.ts @@ -124,7 +124,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/boundary/mcp/analysisViewMcp.test.ts b/tests/boundary/mcp/analysisViewMcp.test.ts index d85fa05ec7..8675fa314f 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"; @@ -164,7 +165,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, @@ -222,11 +223,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 ba2944aa6a..ae9b8ca4b0 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"; @@ -178,16 +179,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 6d14f9df41..4259a6e71c 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"; @@ -62,7 +63,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: { @@ -81,7 +82,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 8dfb3d8dd2..d62e85178d 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 { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -62,7 +63,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 a61576ee77..8ace88edba 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 { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { afterEach, expect, it } from "vitest"; import { z } from "zod"; @@ -275,7 +276,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 ab1343bd7c..039074572f 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 { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -130,7 +131,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/dylibResolutionMcp.test.ts b/tests/boundary/mcp/dylibResolutionMcp.test.ts index 2bb0d70760..c73d5d5488 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 5c4af15dc5..17ec274917 100644 --- a/tests/boundary/mcp/enhancedToolsHarness.ts +++ b/tests/boundary/mcp/enhancedToolsHarness.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import type { CallToolResult } from "@modelcontextprotocol/server"; import { jsonValueSchema, @@ -40,6 +41,8 @@ export const closeEnhancedToolResources = async (): Promise => { }; export const jsonResult = (result: CallToolResult): JsonValue => { + if (result.isError === true) + return jsonValueSchema.parse(parseMcpToolError(result)); if (result.structuredContent === undefined) throw new Error("Tool result omitted structured content"); const structured = jsonValueSchema.safeParse(result.structuredContent); diff --git a/tests/boundary/mcp/ghidraEvidenceMcp.test.ts b/tests/boundary/mcp/ghidraEvidenceMcp.test.ts index 7506a7d0b3..60cd801553 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 15bcb13205..74a4ed35b2 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"; @@ -66,7 +67,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 3a484cae19..9a2f1cbe7d 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"; @@ -98,11 +99,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 800a2b1429..10ec4e2cb9 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/managedMcp.test.ts b/tests/boundary/mcp/managedMcp.test.ts index f54aa37ac9..cbe8c4bf5b 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 { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -230,11 +231,8 @@ const verifyManagedCatalogAndNativeWorkflow = async ( ], }, }); - expect(wrong).toMatchObject({ - isError: true, - structuredContent: { - error: { code: "evidence_integrity_mismatch" }, - }, + expect(parseMcpToolError(wrong)).toMatchObject({ + error: { code: "evidence_integrity_mismatch" }, }); }; @@ -446,6 +444,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 7ae612b9ff..0a3995db1c 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"; @@ -258,7 +259,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 f7701838a4..be331a7cc8 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 b7e4f00145..532e562d96 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 f6f7a3a1ec..687d026203 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 c5d6a206c6..6db36be568 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 { toErrorToolResult } from "../../../src/server/toolResult.js"; @@ -26,6 +28,7 @@ const oversizedFailure = () => const exerciseDeliveryFailure = async ( recordEvidence: EvidenceWriter["recordEvidence"] | undefined, + retention: "retained" | "unavailable" | "failed", ) => { const server = new EvidenceMcpServer( { name: "oversized-error-test", version: "1" }, @@ -54,7 +57,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: { @@ -73,8 +77,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: { @@ -83,11 +87,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", }); @@ -106,10 +116,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 c4b709a32e..83d7df22f9 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"; @@ -294,7 +295,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" }, }); } @@ -333,7 +334,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 af80423b86..9b1ec229e7 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"; @@ -233,4 +234,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 297247d2a8..a350ee828b 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 { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { describe, expect, it } from "vitest"; @@ -42,7 +43,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 1dfe0093a8..716462663c 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" }, }); } const tampered = { diff --git a/tests/boundary/mcp/serverIdentity.test.ts b/tests/boundary/mcp/serverIdentity.test.ts index e69f0f1e60..c7dab3ab1b 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 { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -285,7 +286,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 af5ba7c138..1e99fc54bf 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 c08aedfd13..158277444d 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 93368a451b..e24bec3ce5 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 { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import type { CallToolResult } from "@modelcontextprotocol/server"; import { afterEach, expect, it } from "vitest"; @@ -35,6 +36,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 dbd5d14b5e..df277e67f5 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"; @@ -492,6 +493,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 7df364a74e..cf9f241b76 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 fdbe0753e9..4b64a1a5f0 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"; @@ -93,7 +94,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)); +} From fd46cff017f0110aaab1216764cf01a71b341766 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 13:03:28 +0000 Subject: [PATCH 2/7] test(mcp): read typed errors in canonical Evidence regressions --- tests/boundary/mcp/contractPresentation.test.ts | 6 +++++- tests/boundary/mcp/largeToolResult.test.ts | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) 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/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), ); From a8b8288396cf2e053db768ef03f4db756aa8f71e Mon Sep 17 00:00:00 2001 From: felipeatp Date: Fri, 9 Oct 2026 04:26:40 -0300 Subject: [PATCH 3/7] fix: keep the Ghidra session root free of dot-prefixed path elements Ghidra validates every project path element with NamingUtilities.checkName and aborts the headless analyzer on any element starting with '.', so an inherited TMPDIR such as ~/.cache/ failed every Ghidra operation with a generic analysis failure. Select the session root through a helper that prefers the inherited base, then a dot-free platform temp directory, then the nearest dot-free ancestor. --- src/ghidra/GhidraClient.ts | 4 +- src/ghidra/GhidraSessionRoot.test.ts | 61 ++++++++++++++++++++++++++++ src/ghidra/GhidraSessionRoot.ts | 55 +++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 src/ghidra/GhidraSessionRoot.test.ts create mode 100644 src/ghidra/GhidraSessionRoot.ts diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index 5442586c84..375a4f0644 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -1,6 +1,5 @@ import { randomBytes, randomUUID } from "node:crypto"; import type { Socket } from "node:net"; -import { tmpdir } from "node:os"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import type { JsonValue } from "../domain/jsonValue.js"; @@ -47,9 +46,10 @@ import { } from "./GhidraTransport.js"; import { GhidraWire } from "./GhidraClientWire.js"; import { completeGhidraStartupHandshake } from "./GhidraClientStartup.js"; +import { ghidraSessionRoot } from "./GhidraSessionRoot.js"; const SHUTDOWN_TIMEOUT_MS = 10_000; -const SESSION_ROOT = tmpdir(); +const SESSION_ROOT = ghidraSessionRoot(); export type { GhidraClientOptions, diff --git a/src/ghidra/GhidraSessionRoot.test.ts b/src/ghidra/GhidraSessionRoot.test.ts new file mode 100644 index 0000000000..bc5369dd3f --- /dev/null +++ b/src/ghidra/GhidraSessionRoot.test.ts @@ -0,0 +1,61 @@ +import { join, sep } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { ghidraSessionRoot } from "./GhidraSessionRoot.js"; + +const hasDotPrefixElement = (directory: string): boolean => + directory.split(sep).some((element) => element.startsWith(".")); + +const POSIX = process.platform !== "win32"; +const posixFallback = join(sep, "tmp"); +const posixBase = (...segments: string[]) => join(sep, ...segments); + +describe("ghidraSessionRoot", () => { + it("keeps an inherited base that Ghidra accepts", () => { + const base = posixBase("var", "tmp", "rea-run"); + expect(ghidraSessionRoot({ base, fallback: posixFallback })).toBe(base); + }); + + it("keeps a base whose elements only contain an interior dot", () => { + const base = posixBase("opt", "rea.run", "session"); + expect(ghidraSessionRoot({ base, fallback: posixFallback })).toBe(base); + }); + + it("relocates a dot-prefixed base to the supplied dot-free fallback", () => { + const base = posixBase("home", "operator", ".cache", "scratch"); + expect(ghidraSessionRoot({ base, fallback: posixFallback })).toBe( + posixFallback, + ); + }); + + it("relocates a base that is itself a hidden directory", () => { + const base = posixBase("home", "operator", ".tmp"); + expect(ghidraSessionRoot({ base, fallback: posixBase("var", "tmp") })).toBe( + posixBase("var", "tmp"), + ); + }); + + it("ignores a fallback that is itself dotted", () => { + const base = posixBase("home", "operator", ".cache", "scratch"); + expect( + ghidraSessionRoot({ + base, + fallback: posixBase("home", "operator", ".tmp"), + }), + ).toBe(posixBase("home", "operator")); + }); + + it("never returns a path with a dot-prefixed element", () => { + const directory = ghidraSessionRoot({ + base: posixBase("home", "operator", ".cache", "scratch"), + fallback: posixBase("home", "operator", ".cache"), + }); + expect(hasDotPrefixElement(directory)).toBe(false); + }); + + it.skipIf(!POSIX)("prefers the platform temp directory by default", () => { + expect( + ghidraSessionRoot({ base: posixBase("home", "operator", ".cache") }), + ).toBe(posixBase("tmp")); + }); +}); diff --git a/src/ghidra/GhidraSessionRoot.ts b/src/ghidra/GhidraSessionRoot.ts new file mode 100644 index 0000000000..82c88c0bf3 --- /dev/null +++ b/src/ghidra/GhidraSessionRoot.ts @@ -0,0 +1,55 @@ +import { dirname, isAbsolute, resolve, sep } from "node:path"; +import { tmpdir } from "node:os"; + +/** Session roots may not contain a dot-prefixed path element. */ +const hasDotPrefixElement = (directory: string): boolean => + resolve(directory) + .split(sep) + .some((element) => element.startsWith(".")); + +/** Nearest ancestor of `directory`, inclusive, that has no dot-prefixed element. */ +const nearestSafeAncestor = (directory: string): string | undefined => { + let current = resolve(directory); + for (;;) { + if (!hasDotPrefixElement(current)) return current; + const parent = dirname(current); + if (parent === current) return undefined; + current = parent; + } +}; + +/** Coordinates accepted by {@link ghidraSessionRoot}. */ +export interface GhidraSessionRootOptions { + /** Inherited base directory, normally `os.tmpdir()`. */ + readonly base?: string; + /** Platform-specific dot-free fallback directory passed to `mkdtemp`. */ + readonly fallback?: string; + /** Platform override used by deterministic boundary tests. */ + readonly platform?: NodeJS.Platform; +} + +/** + * Select a session root usable as the parent of a Ghidra project path. + * + * Ghidra validates every project path element with `NamingUtilities.checkName` + * and aborts the headless analyzer on any element that starts with `.`, so an + * inherited `TMPDIR` such as `~/.cache/` fails every Ghidra operation + * with a generic analysis failure. Prefer the inherited base when it is usable, + * then a dot-free platform temp directory, then the nearest dot-free ancestor. + */ +export const ghidraSessionRoot = ( + options: GhidraSessionRootOptions = {}, +): string => { + const base = resolve(options.base ?? tmpdir()); + if (!hasDotPrefixElement(base)) return base; + const platform = options.platform ?? process.platform; + const fallback = + options.fallback ?? (platform === "win32" ? undefined : "/tmp"); + if ( + fallback !== undefined && + isAbsolute(fallback) && + !hasDotPrefixElement(fallback) + ) + return resolve(fallback); + return nearestSafeAncestor(base) ?? base; +}; From 1e83d270c2098b1bcb59591698a257a05d2296fc Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 13:20:49 +0000 Subject: [PATCH 4/7] test(ghidra): use host-native absolute session root fixtures --- src/ghidra/GhidraSessionRoot.test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ghidra/GhidraSessionRoot.test.ts b/src/ghidra/GhidraSessionRoot.test.ts index bc5369dd3f..c1c1ecf60e 100644 --- a/src/ghidra/GhidraSessionRoot.test.ts +++ b/src/ghidra/GhidraSessionRoot.test.ts @@ -1,4 +1,4 @@ -import { join, sep } from "node:path"; +import { join, resolve, sep } from "node:path"; import { describe, expect, it } from "vitest"; import { ghidraSessionRoot } from "./GhidraSessionRoot.js"; @@ -7,8 +7,8 @@ const hasDotPrefixElement = (directory: string): boolean => directory.split(sep).some((element) => element.startsWith(".")); const POSIX = process.platform !== "win32"; -const posixFallback = join(sep, "tmp"); -const posixBase = (...segments: string[]) => join(sep, ...segments); +const posixFallback = join(resolve(sep), "tmp"); +const posixBase = (...segments: string[]) => join(resolve(sep), ...segments); describe("ghidraSessionRoot", () => { it("keeps an inherited base that Ghidra accepts", () => { @@ -53,6 +53,15 @@ describe("ghidraSessionRoot", () => { expect(hasDotPrefixElement(directory)).toBe(false); }); + it("uses the nearest safe ancestor when the platform has no fallback", () => { + expect( + ghidraSessionRoot({ + base: posixBase("Users", "operator", ".cache", "scratch"), + platform: "win32", + }), + ).toBe(posixBase("Users", "operator")); + }); + it.skipIf(!POSIX)("prefers the platform temp directory by default", () => { expect( ghidraSessionRoot({ base: posixBase("home", "operator", ".cache") }), From 09058ebc4a68c76d865a92dbddb65811ba8344f7 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 13:42:08 +0000 Subject: [PATCH 5/7] test(mcp): validate public error diagnostics in acceptance workflows --- .../tools/historicalCaptureResources.test.ts | 5 +++-- tests/acceptance/tools/inspectorLifecycle.test.ts | 5 +++-- tests/acceptance/tools/localToolContracts.test.ts | 3 ++- tests/process-global/mcpDeliveryEnvironment.test.ts | 11 ++++++++--- 4 files changed, 16 insertions(+), 8 deletions(-) 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/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 }] }, From 1e341973cfad1dbf06755f3a086ffc23d42a723c Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 14:21:38 +0000 Subject: [PATCH 6/7] test(ghidra): size oversized annotations for canonical delivery --- scripts/lib/real-ghidra-large-results.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/lib/real-ghidra-large-results.mjs b/scripts/lib/real-ghidra-large-results.mjs index 76738567e1..15fbcd0873 100644 --- a/scripts/lib/real-ghidra-large-results.mjs +++ b/scripts/lib/real-ghidra-large-results.mjs @@ -87,9 +87,12 @@ export async function verifyGhidraLargeResults({ length, }); const readId = retainedReference(readError, "read_bytes"); + // Two annotation strings repeated in text and structured content must + // exceed the receive budget even without obsolete Evidence envelopes. + // The request contains only the two strings and remains below that budget. const comment = "REA_BIG_COMMENT:" + - "x".repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 10)); + "x".repeat(Math.ceil(STDIO_DEFAULT_MAX_BUFFER_SIZE / 4)); const annotationError = await reject("annotate_native_function", { procedure: entry.address, name: "rea_frame_probe", From 1c3766a8bcc8a2b6af679cdaa92a73fec42b3372 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Fri, 9 Oct 2026 14:37:06 +0000 Subject: [PATCH 7/7] test(ghidra): preserve selected verification resource limits --- scripts/lib/native-value-e2e.mjs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/lib/native-value-e2e.mjs b/scripts/lib/native-value-e2e.mjs index 1b21b2ad52..341c0b4504 100644 --- a/scripts/lib/native-value-e2e.mjs +++ b/scripts/lib/native-value-e2e.mjs @@ -25,6 +25,15 @@ const environment = () => ({ ...(process.env.JAVA_HOME === undefined ? {} : { JAVA_HOME: process.env.JAVA_HOME }), + ...(process.env.GHIDRA_HEADLESS_MAXMEM === undefined + ? {} + : { GHIDRA_HEADLESS_MAXMEM: process.env.GHIDRA_HEADLESS_MAXMEM }), + ...(process.env.GHIDRA_MAXMEM === undefined + ? {} + : { GHIDRA_MAXMEM: process.env.GHIDRA_MAXMEM }), + ...(process.env._JAVA_OPTIONS === undefined + ? {} + : { _JAVA_OPTIONS: process.env._JAVA_OPTIONS }), }); const verifyGraph = (input, target, procedure, globalAddress) => {