diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index 96564b14b..3676f674b 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -425,6 +425,20 @@ try { json(text(current)).normalized_result !== "fixture" ) throw new Error("packaged MCP bridge call failed"); + const batch = await client.callTool( + { name: "batch_decompile", arguments: { addresses: ["0x1000"] } }, + mcpOptions, + ); + const batchResult = json(text(batch)).normalized_result; + if ( + batch.isError === true || + batchResult?.total !== 1 || + batchResult?.succeeded !== 1 || + batchResult?.failed !== 0 || + batchResult?.items?.[0]?.status !== "ok" || + batchResult?.items?.[0]?.pseudocode !== "return 0;" + ) + throw new Error("packaged MCP structured batch result failed"); const closed = await client.callTool( { name: "close_binary", arguments: {} }, mcpOptions, diff --git a/scripts/verify-real-hopper.mjs b/scripts/verify-real-hopper.mjs index 51cd77f1f..a173596e4 100644 --- a/scripts/verify-real-hopper.mjs +++ b/scripts/verify-real-hopper.mjs @@ -185,12 +185,18 @@ const verifyCurrentTarget = async (client, options) => { boundedResult === null || typeof boundedResult !== "object" || Array.isArray(boundedResult) || - Object.keys(boundedResult).length !== 1 + boundedResult.total !== 1 || + boundedResult.succeeded !== 1 || + boundedResult.failed !== 0 || + !Array.isArray(boundedResult.items) || + boundedResult.items.length !== 1 || + boundedResult.items[0]?.address !== firstAddress || + boundedResult.items[0]?.status !== "ok" ) { throw new Error("batch_decompile returned an invalid result"); } const boundedPseudocode = requirePseudocode( - boundedResult[firstAddress], + boundedResult.items[0].pseudocode, "batch_decompile", ); const dossier = requireFunctionDossier( diff --git a/src/application/EnhancedTools.ts b/src/application/EnhancedTools.ts index 9f65c3a99..ede9c5f7a 100644 --- a/src/application/EnhancedTools.ts +++ b/src/application/EnhancedTools.ts @@ -8,9 +8,11 @@ import { AnalysisInputError, AnalysisCancelledError, AnalysisOutputError, + projectAnalysisError, type AnalysisError, } from "../domain/errors.js"; import { + addressDistance, parseDocuments, parseFunctionDossier, parseAddressedPage, @@ -134,26 +136,40 @@ export class EnhancedTools { addresses: readonly string[], signal?: AbortSignal, ): EnhancedResult { - if (addresses.length === 0) return ok({ error: "No addresses provided" }); - - const entries = await Promise.all( + const items = await Promise.all( addresses.map(async (address) => { const result = await this.#call( "procedure_pseudo_code", { procedure: address }, signal, ); - return [ - address, - result.ok - ? result.value === null || result.value === "" - ? "No output" - : result.value - : `Error: ${result.error.message}`, - ] as const; + if (!result.ok) + return { + address, + status: "error" as const, + error: projectAnalysisError(result.error), + }; + if (typeof result.value !== "string" || result.value.length === 0) + return { + address, + status: "error" as const, + error: projectAnalysisError( + new AnalysisOutputError( + "procedure_pseudo_code", + "provider returned empty pseudocode", + ), + ), + }; + return { address, status: "ok" as const, pseudocode: result.value }; }), ); - return ok(Object.fromEntries(entries)); + const succeeded = items.filter(({ status }) => status === "ok").length; + return ok({ + items, + total: items.length, + succeeded, + failed: items.length - succeeded, + }); } async #callGraph( @@ -190,7 +206,8 @@ export class EnhancedTools { if (!result.ok) { graph[level].push({ address: current.address, - error: result.error.message, + status: "error", + error: projectAnalysisError(result.error), }); continue; } @@ -198,12 +215,14 @@ export class EnhancedTools { if (!related.ok) { graph[level].push({ address: current.address, - error: related.error.message, + status: "error", + error: projectAnalysisError(related.error), }); continue; } graph[level].push({ address: current.address, + status: "ok", calls: [...related.value], }); if (current.depth + 1 < input.depth) { @@ -226,21 +245,34 @@ export class EnhancedTools { } async #findXrefs(name: string, signal?: AbortSignal): EnhancedResult { - const resolved = await this.#call( - "address_name", - { address: name }, + const names = await this.#allAddressed("list_names", signal); + if (!names.ok) return names; + const resolved = names.value.find((entry) => entry.name === name); + if (resolved === undefined) + return ok({ status: "unresolved", name, reason: "name_not_found" }); + + const xrefs = await this.#call( + "xrefs", + { address: resolved.address }, signal, ); - if (!resolved.ok) return resolved; - const address = resolveAddress(resolved.value); - if (address === undefined) - return ok({ error: `Could not resolve name: ${name}` }); - - const xrefs = await this.#call("xrefs", { address }, signal); if (!xrefs.ok) return xrefs; - return ok( - Array.isArray(xrefs.value) ? { xrefs: xrefs.value } : xrefs.value, - ); + if ( + !Array.isArray(xrefs.value) || + xrefs.value.some((xref) => typeof xref !== "string") + ) + return err( + new AnalysisOutputError( + "xrefs", + "provider returned an invalid address list", + ), + ); + return ok({ + status: "resolved", + name, + address: resolved.address, + xrefs: xrefs.value, + }); } async #binaryOverview( @@ -491,17 +523,3 @@ const invalidInput = (name: EnhancedToolName, cause: Error): EnhancedResult => }), ), ); - -const resolveAddress = (value: JsonValue): string | undefined => { - if (typeof value === "string" && value.length > 0) return value; - if (typeof value !== "object" || value === null || Array.isArray(value)) { - return undefined; - } - const candidate = value.address ?? value.name; - return typeof candidate === "string" && candidate.length > 0 - ? candidate - : undefined; -}; - -const addressDistance = (start: string, end: string): number => - Math.max(0, Number.parseInt(end, 16) - Number.parseInt(start, 16)); diff --git a/src/contracts/toolContracts.ts b/src/contracts/toolContracts.ts index 20d7661e3..36bcc93fb 100644 --- a/src/contracts/toolContracts.ts +++ b/src/contracts/toolContracts.ts @@ -365,12 +365,12 @@ export const ENHANCED_TOOL_CONTRACTS = [ ), enhanced( "batch_decompile", - "Decompile up to 20 explicit procedure symbols or addresses concurrently. Per-item strings may contain errors or no-output markers, so validate each result; use analyze_function for a richer single-function dossier.", + "Decompile up to 20 explicit procedure symbols or addresses concurrently. Returns ordered per-item ok/error variants and aggregate counts; use analyze_function for a richer single-function dossier.", enhancedInputSchemas.batch_decompile, ), enhanced( "get_call_graph", - "Traverse Hopper's caller or callee relationships from one symbol or address for at most five levels. Nodes preserve per-procedure errors; indirect calls may be missing and results are not a whole-program CFG.", + "Traverse Hopper's caller or callee relationships from one symbol or address for at most five levels. Every node has an ok/error status and failures use safe typed projections; indirect calls may be missing and results are not a whole-program CFG.", enhancedInputSchemas.get_call_graph, ), enhanced( @@ -380,7 +380,7 @@ export const ENHANCED_TOOL_CONTRACTS = [ ), enhanced( "find_xrefs_to_name", - "Resolve a name through Hopper and return analyzed references to its address. Use when starting from a selector or symbol; resolution failure is returned explicitly and xrefs remain untyped.", + "Resolve an exact name through Hopper's exhaustively paged name inventory and return a resolved or unresolved result. Unresolved names use the stable name_not_found reason; xrefs remain untyped.", enhancedInputSchemas.find_xrefs_to_name, ), enhanced( diff --git a/src/contracts/toolOutputSchemas.ts b/src/contracts/toolOutputSchemas.ts index a1988ef65..a747a4036 100644 --- a/src/contracts/toolOutputSchemas.ts +++ b/src/contracts/toolOutputSchemas.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { ANALYSIS_ERROR_TAGS } from "../domain/errors.js"; import { evidenceSchema } from "../domain/evidence.js"; import { processCaptureComparisonSchema, @@ -177,9 +178,25 @@ const symbolDiscoveryOutput = (property: "classes" | "protocols") => [property]: z.array(addressedEntry), }), ); -const graphNode = z.union([ - z.object({ address: z.string(), calls: z.array(z.string()) }), - z.object({ address: z.string(), error: z.string() }), +/** Exact public projection of an expected analysis failure. */ +export const analysisErrorProjectionSchema = z + .object({ + tag: z.enum(ANALYSIS_ERROR_TAGS), + message: z.string(), + details: z.record(z.string(), z.union([z.string(), z.number(), z.null()])), + }) + .strict(); +const graphNode = z.discriminatedUnion("status", [ + z.object({ + address: z.string(), + status: z.literal("ok"), + calls: z.array(z.string()), + }), + z.object({ + address: z.string(), + status: z.literal("error"), + error: analysisErrorProjectionSchema, + }), ]); const functionDossierOutput = resultOf(functionDossierSchema); @@ -242,10 +259,25 @@ export const enhancedOutputSchemas: Readonly> = { get_objc_classes: symbolDiscoveryOutput("classes"), get_objc_protocols: symbolDiscoveryOutput("protocols"), batch_decompile: resultOf( - z.union([ - z.object({ error: z.literal("No addresses provided") }), - z.record(z.string(), z.string()), - ]), + z.object({ + items: z.array( + z.discriminatedUnion("status", [ + z.object({ + address: z.string(), + status: z.literal("ok"), + pseudocode: z.string().min(1), + }), + z.object({ + address: z.string(), + status: z.literal("error"), + error: analysisErrorProjectionSchema, + }), + ]), + ), + total: z.number().int().min(0), + succeeded: z.number().int().min(0), + failed: z.number().int().min(0), + }), ), get_call_graph: resultOf(z.record(z.string(), z.array(graphNode))), analyze_swift_types: resultOf( @@ -261,9 +293,18 @@ export const enhancedOutputSchemas: Readonly> = { }), ), find_xrefs_to_name: resultOf( - z.union([ - z.object({ xrefs: addressList }), - z.object({ error: z.string() }), + z.discriminatedUnion("status", [ + z.object({ + status: z.literal("resolved"), + name: z.string(), + address: z.string(), + xrefs: addressList, + }), + z.object({ + status: z.literal("unresolved"), + name: z.string(), + reason: z.literal("name_not_found"), + }), ]), ), binary_overview: resultOf( diff --git a/src/domain/errors.ts b/src/domain/errors.ts index ed67d576f..be080dca2 100644 --- a/src/domain/errors.ts +++ b/src/domain/errors.ts @@ -1,27 +1,34 @@ -export type AnalysisErrorTag = - | "AnalysisProtocolError" - | "AnalysisInputError" - | "AnalysisOutputError" - | "AnalysisCapabilityUnavailableError" - | "AnalysisCancelledError" - | "AnalysisTimeoutError" - | "ProviderSelectionError" - | "ProviderAdapterError" - | "ArtifactOperationError" - | "ProcessCaptureError" - | "EvidenceIntegrityError" - | "EvidenceLimitError" - | "EvidenceFileError" - | "UnknownRegistryError" - | "HopperTimeoutError" - | "HopperCancelledError" - | "HopperProtocolError" - | "HopperRemoteError" - | "HopperProcessError" - | "HopperStartError" - | "ConfigurationError" - | "NoBinaryOpenError" - | "BinaryTargetError"; +import type { JsonValue } from "./jsonValue.js"; + +/** Stable tags exposed by safe analysis-error projections. */ +export const ANALYSIS_ERROR_TAGS = [ + "AnalysisProtocolError", + "AnalysisInputError", + "AnalysisOutputError", + "AnalysisCapabilityUnavailableError", + "AnalysisCancelledError", + "AnalysisTimeoutError", + "ProviderSelectionError", + "ProviderAdapterError", + "ArtifactOperationError", + "ProcessCaptureError", + "EvidenceIntegrityError", + "EvidenceLimitError", + "EvidenceFileError", + "UnknownRegistryError", + "HopperTimeoutError", + "HopperCancelledError", + "HopperProtocolError", + "HopperRemoteError", + "HopperProcessError", + "HopperStartError", + "ConfigurationError", + "NoBinaryOpenError", + "BinaryTargetError", +] as const; + +/** Stable tag for an expected analysis failure. */ +export type AnalysisErrorTag = (typeof ANALYSIS_ERROR_TAGS)[number]; /** Base class for expected analysis, provider, and session failures. */ export abstract class AnalysisError extends Error { @@ -280,7 +287,8 @@ export class BinaryTargetError extends AnalysisError { } } -export interface AnalysisErrorProjection { +export interface AnalysisErrorProjection + extends Readonly> { readonly tag: AnalysisErrorTag; readonly message: string; readonly details: Readonly>; diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index 7d5e417bb..93771394d 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -24,6 +24,10 @@ export interface AddressedPage { readonly hasMore: boolean; } +/** Return the non-negative byte distance between hexadecimal addresses. */ +export const addressDistance = (start: string, end: string): number => + Math.max(0, Number.parseInt(end, 16) - Number.parseInt(start, 16)); + const procedureMapSchema = z.record(z.string(), z.string()); const addressedNamesSchema = z.array( z.object({ address: z.string(), name: z.string() }), diff --git a/tests/enhancedTools.test.ts b/tests/enhancedTools.test.ts index 765c1ca71..628af8cd3 100644 --- a/tests/enhancedTools.test.ts +++ b/tests/enhancedTools.test.ts @@ -5,9 +5,11 @@ import { afterEach, describe, expect, it } from "vitest"; import type { AnalysisOperationPort } from "../src/application/AnalysisProvider.js"; import { EnhancedTools } from "../src/application/EnhancedTools.js"; import { ENHANCED_TOOL_CONTRACTS } from "../src/contracts/toolContracts.js"; -import { observed as ok } from "./fixtures/analysisExecution.js"; +import { AnalysisOutputError } from "../src/domain/errors.js"; import { jsonValueSchema, type JsonValue } from "../src/domain/jsonValue.js"; +import { err } from "../src/domain/result.js"; import { createServer } from "../src/server/createServer.js"; +import { observed as ok } from "./fixtures/analysisExecution.js"; const PROCEDURES = { "0x1": "_TtC7Fixture5Class", @@ -39,6 +41,7 @@ const fixturePort = (): AnalysisOperationPort => ({ "0x10": "_OBJC_CLASS_$_Fixture", "0x11": "_OBJC_CLASS_$_Fixture", "0x12": "_OBJC_PROTOCOL_$_FixtureDelegate", + "0x13": "entry", }), ), ); @@ -63,9 +66,7 @@ const fixturePort = (): AnalysisOperationPort => ({ case "procedure_callers": return Promise.resolve(ok(["0x9"])); case "address_name": - return Promise.resolve( - ok({ address: "0x1", name: arguments_.address ?? "" }), - ); + return Promise.resolve(ok(arguments_.address ?? null)); case "xrefs": return Promise.resolve(ok(["0x20", "0x21"])); case "resolve_containing_procedure": @@ -217,16 +218,29 @@ describe("enhanced MCP tools", () => { // Legacy behavior treats any `_OBJC_` label, including protocol labels, as a class. expect(results[1]).toMatchObject({ count: 2 }); expect(results[2]).toMatchObject({ count: 1 }); - expect(results[3]).toEqual({ "0x1": "pseudo:0x1", "0x2": "pseudo:0x2" }); + expect(results[3]).toEqual({ + items: [ + { address: "0x1", status: "ok", pseudocode: "pseudo:0x1" }, + { address: "0x2", status: "ok", pseudocode: "pseudo:0x2" }, + ], + total: 2, + succeeded: 2, + failed: 0, + }); expect(results[4]).toEqual({ - "0": [{ address: "0x1", calls: ["0x2", "0x3"] }], + "0": [{ address: "0x1", status: "ok", calls: ["0x2", "0x3"] }], "1": [ - { address: "0x2", calls: ["0x1"] }, - { address: "0x3", calls: [] }, + { address: "0x2", status: "ok", calls: ["0x1"] }, + { address: "0x3", status: "ok", calls: [] }, ], }); expect(results[5]).toMatchObject({ total: 6 }); - expect(results[6]).toEqual({ xrefs: ["0x20", "0x21"] }); + expect(results[6]).toEqual({ + status: "resolved", + name: "entry", + address: "0x13", + xrefs: ["0x20", "0x21"], + }); expect(results[7]).toMatchObject({ document: "fixture", segment_count: 1, @@ -275,6 +289,84 @@ describe("enhanced MCP tools", () => { expect(maximum).toBe(20); }); + it("returns ordered typed batch failures and zero counts for empty input", async () => { + const tools = new EnhancedTools({ + execute: (_name, arguments_) => + arguments_.procedure === "0x2" + ? Promise.resolve(err(new AnalysisOutputError("decompile", "failed"))) + : Promise.resolve(ok("pseudo")), + }); + + const result = await tools.execute("batch_decompile", { + addresses: ["0x1", "0x2"], + }); + const empty = await tools.execute("batch_decompile", { addresses: [] }); + + expect(result).toEqual({ + ok: true, + value: { + items: [ + { address: "0x1", status: "ok", pseudocode: "pseudo" }, + { + address: "0x2", + status: "error", + error: { + tag: "AnalysisOutputError", + message: "Invalid analysis output for decompile: failed", + details: { operation: "decompile", reason: "failed" }, + }, + }, + ], + total: 2, + succeeded: 1, + failed: 1, + }, + }); + expect(empty).toEqual({ + ok: true, + value: { items: [], total: 0, succeeded: 0, failed: 0 }, + }); + }); + + it("returns typed graph failures and stable unresolved-name results", async () => { + const tools = new EnhancedTools({ + execute: (name) => + name === "procedure_callees" + ? Promise.resolve(err(new AnalysisOutputError(name, "failed"))) + : Promise.resolve(ok(page({}))), + }); + + const graph = await tools.execute("get_call_graph", { + address: "0x1", + direction: "forward", + depth: 1, + }); + const unresolved = await tools.execute("find_xrefs_to_name", { + name: "missing", + }); + + expect(graph).toMatchObject({ + ok: true, + value: { + "0": [ + { + address: "0x1", + status: "error", + error: { tag: "AnalysisOutputError" }, + }, + ], + }, + }); + expect(unresolved).toEqual({ + ok: true, + value: { + status: "unresolved", + name: "missing", + reason: "name_not_found", + }, + }); + }); + it("follows procedure pagination for whole-binary workflows", async () => { const offsets: number[] = []; const client = await connect({ diff --git a/tests/errors.test.ts b/tests/errors.test.ts index 2a087c6b8..9d319ea18 100644 --- a/tests/errors.test.ts +++ b/tests/errors.test.ts @@ -6,6 +6,8 @@ import { AnalysisInputError, AnalysisOutputError, AnalysisProtocolError, + AnalysisTimeoutError, + ArtifactOperationError, BinaryTargetError, ConfigurationError, EvidenceFileError, @@ -20,8 +22,10 @@ import { NoBinaryOpenError, ProviderAdapterError, ProviderSelectionError, + UnknownRegistryError, projectAnalysisError, type AnalysisError, + type AnalysisErrorTag, } from "../src/domain/errors.js"; import { ProcessCaptureError } from "../src/application/ProcessHarness.js"; @@ -34,35 +38,67 @@ describe("analysis error projection", () => { it("exhaustively projects stable tags without causes or local paths", () => { const secretCause = new Error("secret-token"); - const errors: readonly AnalysisError[] = [ - new AnalysisProtocolError("protocol"), - new AnalysisInputError("overview", { cause: secretCause }), - new AnalysisOutputError("overview", "invalid shape", { + const byTag = { + AnalysisProtocolError: new AnalysisProtocolError("protocol failed"), + AnalysisInputError: new AnalysisInputError("overview", { cause: secretCause, }), - new AnalysisCapabilityUnavailableError("fixture", "overview", "absent"), - new AnalysisCancelledError("overview"), - new ProviderSelectionError("overview"), - new ProviderAdapterError("fixture", "overview", { cause: secretCause }), - new ProcessCaptureError("capture refused", { cause: secretCause }), - new EvidenceIntegrityError("integrity", { cause: secretCause }), - new EvidenceLimitError("records", 10), - new EvidenceFileError("read", "outside-root", { cause: secretCause }), - new HopperTimeoutError(100), - new HopperCancelledError(), - new HopperProtocolError("wire", { cause: secretCause }), - new HopperRemoteError(7, "safe"), - new HopperProcessError(1), - new HopperStartError({ cause: secretCause }), - new ConfigurationError("configuration", { cause: secretCause }), - new NoBinaryOpenError(), - new BinaryTargetError("/secret/local/path", "invalid", { + AnalysisOutputError: new AnalysisOutputError( + "overview", + "invalid shape", + { + cause: secretCause, + }, + ), + AnalysisCapabilityUnavailableError: + new AnalysisCapabilityUnavailableError("fixture", "overview", "absent"), + AnalysisCancelledError: new AnalysisCancelledError("overview"), + AnalysisTimeoutError: new AnalysisTimeoutError("overview", 50), + ProviderSelectionError: new ProviderSelectionError("overview"), + ProviderAdapterError: new ProviderAdapterError("fixture", "overview", { cause: secretCause, }), - ]; + ArtifactOperationError: new ArtifactOperationError( + "inventory_artifact", + "integrity", + ), + ProcessCaptureError: new ProcessCaptureError("capture failed", { + cause: secretCause, + }), + EvidenceIntegrityError: new EvidenceIntegrityError("integrity failed", { + cause: secretCause, + }), + EvidenceLimitError: new EvidenceLimitError("records", 10), + EvidenceFileError: new EvidenceFileError("read", "outside-root", { + cause: secretCause, + }), + UnknownRegistryError: new UnknownRegistryError("revision-conflict", { + cause: secretCause, + }), + HopperTimeoutError: new HopperTimeoutError(100), + HopperCancelledError: new HopperCancelledError(), + HopperProtocolError: new HopperProtocolError("wire failed", { + cause: secretCause, + }), + HopperRemoteError: new HopperRemoteError(7, "safe"), + HopperProcessError: new HopperProcessError(1), + HopperStartError: new HopperStartError({ cause: secretCause }), + ConfigurationError: new ConfigurationError("configuration failed", { + cause: secretCause, + }), + NoBinaryOpenError: new NoBinaryOpenError(), + BinaryTargetError: new BinaryTargetError( + "/secret/local/path", + "invalid", + { + cause: secretCause, + }, + ), + } satisfies Readonly>; + const errors = Object.values(byTag); const projected = errors.map(projectAnalysisError); expect(new Set(projected.map(({ tag }) => tag)).size).toBe(errors.length); - expect(projected).toHaveLength(20); + expect(projected).toHaveLength(23); expect(JSON.stringify(projected)).not.toContain("secret-token"); expect(JSON.stringify(projected)).not.toContain("/secret/local/path"); expect(