From eb38e4c4024a7d66001b3e69f8309c85492f05c0 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:51:19 +0000 Subject: [PATCH] feat(mcp): return structured typed tool results --- src/contracts/toolOutputSchemas.ts | 16 +++++- src/domain/nativeInspection.ts | 5 +- src/domain/processCapture.ts | 9 +-- src/domain/processComparison.ts | 9 +-- src/server/toolResult.ts | 19 ++++++- tests/artifactMcp.test.ts | 61 ++++++++++++++++++++- tests/contractSnapshot.test.ts | 31 +++++++++++ tests/enhancedTools.test.ts | 26 +++++---- tests/integration/serverIntegration.test.ts | 29 +++++++++- tests/officialTools.test.ts | 2 +- tests/sessionMcp.test.ts | 14 ++++- tests/toolResult.test.ts | 14 ++++- 12 files changed, 203 insertions(+), 32 deletions(-) diff --git a/src/contracts/toolOutputSchemas.ts b/src/contracts/toolOutputSchemas.ts index 1187e3350..7d9455ab3 100644 --- a/src/contracts/toolOutputSchemas.ts +++ b/src/contracts/toolOutputSchemas.ts @@ -181,8 +181,22 @@ const symbolDiscoveryOutput = (property: "classes" | "protocols") => const analysisErrorProjectionSchema = z .object({ tag: z.enum(ANALYSIS_ERROR_TAGS), + category: z.enum([ + "invalid_input", + "permission_required", + "unsupported_provider", + "integrity_mismatch", + "truncated", + "cancelled", + "timeout", + "unavailable", + "execution_failure", + ]), message: z.string(), - details: z.record(z.string(), z.union([z.string(), z.number(), z.null()])), + details: z.record( + z.string(), + z.union([z.string(), z.number(), z.boolean(), z.null()]), + ), }) .strict(); const graphNode = z.discriminatedUnion("status", [ diff --git a/src/domain/nativeInspection.ts b/src/domain/nativeInspection.ts index 7e93dc3b6..970d1d9df 100644 --- a/src/domain/nativeInspection.ts +++ b/src/domain/nativeInspection.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { jsonValueSchema } from "./jsonValue.js"; const hexAddress = z.string().regex(/^0x[a-fA-F0-9]+$/u); const coverage = (item: Schema) => @@ -112,7 +113,7 @@ export const inspectSignatureSchema = z.object({ hash_algorithms: z.array(z.string()), authorities: z.array(z.string()), designated_requirement: z.string().nullable(), - entitlements: z.unknown().nullable(), + entitlements: jsonValueSchema.nullable(), timestamp: z.string().nullable(), hardened_runtime: z.boolean().nullable(), provenance: z.array(nativeCommandInvocationSchema), @@ -121,7 +122,7 @@ export const inspectSignatureSchema = z.object({ export const inspectPlistSchema = z.object({ format: z.enum(["xml", "binary", "json", "unknown"]), - value: z.unknown(), + value: jsonValueSchema, bundle: z.object({ identifier: z.string().nullable(), executable: z.string().nullable(), diff --git a/src/domain/processCapture.ts b/src/domain/processCapture.ts index 945fcd879..2ad10ed0a 100644 --- a/src/domain/processCapture.ts +++ b/src/domain/processCapture.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { jsonValueSchema } from "./jsonValue.js"; import { LEGACY_PROCESS_CAPTURE_MESSAGE, @@ -200,10 +201,10 @@ export const processCaptureSchema: z.ZodType = z.object({ pty_backend: z.literal("node-pty"), started_at: z.iso.datetime(), completed_at: z.iso.datetime(), - scenario: z.record(z.string(), z.unknown()), - comparison_contract: z.record(z.string(), z.unknown()), - shim_plan: z.array(z.unknown()), - replay_plan: z.record(z.string(), z.unknown()), + scenario: z.record(z.string(), jsonValueSchema), + comparison_contract: z.record(z.string(), jsonValueSchema), + shim_plan: z.array(jsonValueSchema), + replay_plan: z.record(z.string(), jsonValueSchema), full_scenario_sha256: z.string().regex(/^[a-f0-9]{64}$/u), comparison_contract_sha256: z.string().regex(/^[a-f0-9]{64}$/u), executable_sha256: z.string().regex(/^[a-f0-9]{64}$/u), diff --git a/src/domain/processComparison.ts b/src/domain/processComparison.ts index 0c9b3e105..def58ebe1 100644 --- a/src/domain/processComparison.ts +++ b/src/domain/processComparison.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import type { ProcessCapture } from "./processCapture.js"; import { validateProcessCapture } from "./processCapture.js"; +import { jsonValueSchema } from "./jsonValue.js"; /** Comparison classification that never equates incomplete evidence. */ export const comparisonStatusSchema = z.enum([ @@ -42,8 +43,8 @@ export const processCaptureComparisonSchema = z index: z.number().int().nonnegative(), left_at_ms: z.number().int().nonnegative().nullable(), right_at_ms: z.number().int().nonnegative().nullable(), - left: z.unknown().nullable(), - right: z.unknown().nullable(), + left: jsonValueSchema.nullable(), + right: jsonValueSchema.nullable(), }), ]), limitations: z.array(z.string()), @@ -165,8 +166,8 @@ const firstCollectionDivergence = ( index, left_at_ms: eventTime(leftValue), right_at_ms: eventTime(rightValue), - left: leftValue, - right: rightValue, + left: jsonValueSchema.parse(leftValue), + right: jsonValueSchema.parse(rightValue), }; } return null; diff --git a/src/server/toolResult.ts b/src/server/toolResult.ts index cd965b433..9205732da 100644 --- a/src/server/toolResult.ts +++ b/src/server/toolResult.ts @@ -1,6 +1,10 @@ import type { CallToolResult } from "@modelcontextprotocol/server"; -import { projectAnalysisError, type AnalysisError } from "../domain/errors.js"; +import { + AnalysisOutputError, + projectAnalysisError, + type AnalysisError, +} from "../domain/errors.js"; import type { Result } from "../domain/result.js"; import type { JsonValue } from "../domain/jsonValue.js"; import type { ToolContract } from "../contracts/toolContracts.js"; @@ -22,9 +26,10 @@ const errorResult = (error: AnalysisError): CallToolResult => { content: [ { type: "text", - text: `${projected.tag}: ${projected.message}`, + text: projected.message, }, ], + structuredContent: { error: projected }, isError: true, }; }; @@ -40,9 +45,17 @@ const successResult = ( content: [ { type: "text", - text: "AnalysisOutputError: Analysis output does not match the tool contract", + text: "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", }, ], + structuredContent: { + error: projectAnalysisError( + new AnalysisOutputError( + contract.name, + "output does not match the tool contract", + ), + ), + }, isError: true, }; } diff --git a/tests/artifactMcp.test.ts b/tests/artifactMcp.test.ts index 8c6e8bbe4..eb83a1d7a 100644 --- a/tests/artifactMcp.test.ts +++ b/tests/artifactMcp.test.ts @@ -1,8 +1,10 @@ -import { mkdtemp, writeFile } from "node:fs/promises"; +import { createHash } from "node:crypto"; +import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { createPackageWithOptions } from "@electron/asar"; import { TextReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js"; import { describe, expect, it } from "vitest"; import { z } from "zod"; @@ -16,6 +18,63 @@ import { createServer } from "../src/server/createServer.js"; import { observed } from "./fixtures/analysisExecution.js"; describe("artifact graph MCP integration", () => { + it("returns ASAR integrity path and hashes as structured error data", async () => { + const root = await mkdtemp(join(tmpdir(), "rea-asar-integrity-mcp-")); + const source = join(root, "source"); + const original = "console.log('ok');\n"; + const changed = "changed();\n"; + await mkdir(source); + await writeFile(join(source, "main.js"), original); + const archive = join(root, "fixture.asar"); + await createPackageWithOptions(source, archive, { unpack: "*.js" }); + await writeFile(join(`${archive}.unpacked`, "main.js"), changed); + + const session = new BinarySession(new ArtifactProvider()); + const server = createServer(session, session); + const client = new Client({ name: "asar-integrity-test", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const opened = await client.callTool({ + name: "open_binary", + arguments: { path: archive }, + }); + expect(opened.isError).not.toBe(true); + + const result = await client.callTool({ + name: "inventory_artifact", + arguments: {}, + }); + expect(result.isError).toBe(true); + expect(result.structuredContent).toEqual({ + error: { + tag: "ArtifactOperationError", + category: "integrity_mismatch", + message: + "Artifact is invalid or has changed. Get a fresh copy and try again.", + details: { + operation: "inventory_artifact", + reason: "integrity", + logicalPath: "main.js", + declaredSha256: createHash("sha256").update(original).digest("hex"), + calculatedSha256: createHash("sha256") + .update(changed) + .digest("hex"), + unpacked: true, + }, + }, + }); + } finally { + await Promise.allSettled([ + client.close(), + server.close(), + session.close(), + ]); + } + }); + it("rejects altered payloads that reuse session Evidence IDs", async () => { const session = new BinarySession(() => ({ health: () => Promise.resolve(), diff --git a/tests/contractSnapshot.test.ts b/tests/contractSnapshot.test.ts index eea4777a7..e8897606c 100644 --- a/tests/contractSnapshot.test.ts +++ b/tests/contractSnapshot.test.ts @@ -14,6 +14,19 @@ import { sessionOutputSchemas, } from "../src/contracts/toolOutputSchemas.js"; +const emptySchemaPaths = (value: unknown, path = "$"): string[] => { + if (Array.isArray(value)) + return value.flatMap((item, index) => + emptySchemaPaths(item, `${path}[${String(index)}]`), + ); + if (typeof value !== "object" || value === null) return []; + const entries = Object.entries(value); + if (entries.length === 0) return [path]; + return entries.flatMap(([key, item]) => + emptySchemaPaths(item, `${path}.${key}`), + ); +}; + describe("tool contract surface", () => { it("advertises complete typed schemas and annotations for all analysis tools", () => { const contracts = [ @@ -122,4 +135,22 @@ describe("tool contract surface", () => { expect(contract.description).toMatch(/[.;]/u); } }); + + it("publishes no unconstrained output-schema holes across all 68 tools", () => { + const contracts = [ + ...OFFICIAL_TOOL_CONTRACTS, + ...ENHANCED_TOOL_CONTRACTS, + ...NATIVE_TOOL_CONTRACTS, + ...ARTIFACT_TOOL_CONTRACTS, + ...SESSION_TOOL_CONTRACTS, + ]; + expect(contracts).toHaveLength(68); + for (const contract of contracts) { + const schema = z.toJSONSchema(contract.outputSchema, { + target: "draft-07", + unrepresentable: "any", + }); + expect(emptySchemaPaths(schema), contract.name).toEqual([]); + } + }); }); diff --git a/tests/enhancedTools.test.ts b/tests/enhancedTools.test.ts index 628af8cd3..e7f208c8b 100644 --- a/tests/enhancedTools.test.ts +++ b/tests/enhancedTools.test.ts @@ -312,7 +312,9 @@ describe("enhanced MCP tools", () => { status: "error", error: { tag: "AnalysisOutputError", - message: "Invalid analysis output for decompile: failed", + category: "execution_failure", + message: + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", details: { operation: "decompile", reason: "failed" }, }, }, @@ -352,7 +354,11 @@ describe("enhanced MCP tools", () => { { address: "0x1", status: "error", - error: { tag: "AnalysisOutputError" }, + error: { + category: "execution_failure", + message: + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", + }, }, ], }, @@ -551,8 +557,8 @@ describe("enhanced MCP tools", () => { }); expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); - expect(text?.type === "text" ? text.text : "").toContain( - "non-advancing pagination offset", + expect(text?.type === "text" ? text.text : "").toBe( + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", ); }); @@ -598,8 +604,8 @@ describe("enhanced MCP tools", () => { }); expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); - expect(text?.type === "text" ? text.text : "").toContain( - "HopperProtocolError", + expect(text?.type === "text" ? text.text : "").toBe( + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", ); }); @@ -619,8 +625,8 @@ describe("enhanced MCP tools", () => { }); expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); - expect(text?.type === "text" ? text.text : "").toContain( - "AnalysisOutputError", + expect(text?.type === "text" ? text.text : "").toBe( + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", ); }); @@ -655,8 +661,8 @@ describe("enhanced MCP tools", () => { }); expect(result.isError).toBe(true); const text = result.content.find((item) => item.type === "text"); - expect(text?.type === "text" ? text.text : "").toContain( - "AnalysisOutputError", + expect(text?.type === "text" ? text.text : "").toBe( + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", ); }); diff --git a/tests/integration/serverIntegration.test.ts b/tests/integration/serverIntegration.test.ts index 45cf5153e..72b0039b0 100644 --- a/tests/integration/serverIntegration.test.ts +++ b/tests/integration/serverIntegration.test.ts @@ -252,6 +252,17 @@ describe("full MCP integration with multi-tool sequences", () => { }, }); expect(unavailable.isError).toBe(true); + expect(structured(unavailable)).toMatchObject({ + error: { + tag: "AnalysisCapabilityUnavailableError", + category: "unsupported_provider", + details: { + providerId: "partial", + operation: "procedure_pseudo_code", + reason: "Decompiler is not installed.", + }, + }, + }); expect(received[0]).not.toHaveProperty("unknown_registry_approved"); expect( structured( @@ -354,7 +365,7 @@ describe("full MCP integration with multi-tool sequences", () => { expect(names).not.toContain("open_binary"); }); - it("propagates remote Hopper errors with structured content", async () => { + it("projects remote failures without provider or bridge details", async () => { const client = await connect({ execute: () => Promise.resolve(err(new HopperRemoteError(-32000, "bridge timeout"))), @@ -365,8 +376,20 @@ describe("full MCP integration with multi-tool sequences", () => { arguments: {}, }); expect(result.isError).toBe(true); - expect(text(result)).toContain("HopperRemoteError"); - expect(text(result)).toContain("bridge timeout"); + expect(structured(result)).toMatchObject({ + error: { + tag: "HopperRemoteError", + category: "execution_failure", + details: { + code: -32000, + safeMessage: "bridge timeout", + diagnosticType: "remote", + }, + }, + }); + expect(text(result)).toBe( + "Analysis could not complete. Retry once; if it continues, run `rea doctor`.", + ); }); it("handles concurrent tool calls without corruption", async () => { diff --git a/tests/officialTools.test.ts b/tests/officialTools.test.ts index 9aaa1a837..64a024c62 100644 --- a/tests/officialTools.test.ts +++ b/tests/officialTools.test.ts @@ -199,7 +199,7 @@ describe("official Hopper proxy tools", () => { expect(result.content).toEqual([ { type: "text", - text: "HopperRemoteError: Hopper request failed (-1): denied", + text: "Analysis could not complete. Retry once; if it continues, run `rea doctor`.", }, ]); }); diff --git a/tests/sessionMcp.test.ts b/tests/sessionMcp.test.ts index 21144d061..e123d6757 100644 --- a/tests/sessionMcp.test.ts +++ b/tests/sessionMcp.test.ts @@ -58,7 +58,9 @@ describe("target-free MCP lifecycle", () => { arguments: {}, }); expect(before.isError).toBe(true); - expect(text(before)).toContain("NoBinaryOpenError"); + expect(text(before)).toBe( + "No app is open. Ask the user which app to investigate, then call open_binary with its local path.", + ); expect((await mcp.listTools()).tools).toHaveLength(68); const deniedCapture = await mcp.callTool({ name: "capture_process_scenario", @@ -69,7 +71,15 @@ describe("target-free MCP lifecycle", () => { }, }); expect(deniedCapture.isError).toBe(true); - expect(text(deniedCapture)).toContain("process capture is disabled"); + expect(structured(deniedCapture)).toMatchObject({ + error: { + tag: "ProcessCaptureError", + category: "permission_required", + }, + }); + expect(text(deniedCapture)).toBe( + "Process capture is disabled. Set `REA_PROCESS_CAPTURE_ENABLED=true`, configure approved roots, then restart REA.", + ); expect( (await mcp.callTool({ name: "open_binary", arguments: { path: first } })) .isError, diff --git a/tests/toolResult.test.ts b/tests/toolResult.test.ts index cffd8e73a..a9d098526 100644 --- a/tests/toolResult.test.ts +++ b/tests/toolResult.test.ts @@ -26,9 +26,21 @@ describe("tool result projection", () => { content: [ { type: "text", - text: "AnalysisOutputError: Analysis output does not match the tool contract", + text: "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", }, ], + structuredContent: { + error: { + tag: "AnalysisOutputError", + category: "execution_failure", + message: + "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`.", + details: { + operation: "provider_neutral_fixture", + reason: "output does not match the tool contract", + }, + }, + }, isError: true, }); });