From 5dfb9d050c2cc44a61b183dae765bef4d22a53b7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:26:01 +0800 Subject: [PATCH] fix(analysis)!: scope call-graph caveats to their observed relationships Return deduplicated execution caveats once and preserve provider-owned relationship diagnostics and completeness on graph nodes. Keep complete relationship reads exhaustive despite contextual caveats, including portable call-path reconstruction. Preserve omitted diagnostic fields in historical Evidence and bind reported diagnostics into semantic identity. Revise graph workflow and Ghidra adapter profiles so old cached representations cannot replay under the new contracts. Remove stale duplicate-lookup test claims and redundant fixture validation. Prune million-entry duplicate cases while retaining argument-stack and deep-chain regressions; bounded Flameox captures identified validation and canonical hashing costs. Validation: static and architecture checks, generated docs, 364 focused tests before pruning, 21 call-path tests after pruning, public MCP schema validation and targeted profile tests. Real engine behavior was not locally exercised. BREAKING CHANGE: get_call_graph normalized_result now contains { graph, limitations }; traversal-depth groups are under graph. --- docs/mcp-contracts.md | 9 +- scripts/lib/real-hopper-workflows.mjs | 2 +- src/application/CallPathTracing.ts | 5 + src/application/EnhancedTools.ts | 62 ++++++++-- src/application/InvestigationProviders.ts | 4 +- .../binary/AnalysisSnapshotCache.test.ts | 66 +++++++++- src/contracts/toolOutputSchemaGroups.ts | 7 +- src/domain/callPath.test.ts | 77 +++++------- src/domain/callPath.ts | 4 + src/domain/evidence.test.ts | 33 +++++ src/domain/relationshipCoverage.ts | 28 +++-- src/ghidra/GhidraAnalysisProfile.ts | 2 + src/ghidra/GhidraProvider.behavior.test.ts | 6 +- src/ghidra/GhidraProviderCapabilities.ts | 20 ++- src/ghidra/GhidraProviderClient.ts | 9 +- src/ida/IdaCallGraph.ts | 41 +++--- ...3-returns-typed-graph-failures-and.test.ts | 23 ++-- .../enhancedTools.callPathsAndBatches.test.ts | 83 ++++++++++--- .../boundary/process/jadxIntegration.test.ts | 2 +- .../providers/ida/calleeRelationships.test.ts | 117 ++++++++++-------- .../providers/ida/selectorProvenance.test.ts | 19 +-- 21 files changed, 433 insertions(+), 186 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 93f890f201..decb3b4d44 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -346,9 +346,12 @@ directly to `compare_functions`, and `inspect_artifact` Evidence to `compare_artifacts`. Use `get_evidence_bundle` when the task needs broader retained session history or an explicit bundle for transfer. -`get_call_graph` keeps provider-limited relationship reads on their node as -`limitations`, alongside any observed `calls`. An empty call list with such a -limitation is incomplete coverage, not proof that the function has no calls. +`get_call_graph` returns a `graph` keyed by traversal depth and a top-level +`limitations` array with deduplicated execution caveats. Each node keeps its +relationship coverage and read-specific `limitations`, alongside observed +`calls`. When a provider does not classify its caveats, they remain on that +node and coverage is `unknown`. An empty call list with partial or unknown +coverage is not proof that the function has no calls. `trace_call_path` reports each limited expansion in `residual_unknowns` and sets `truncated: true`; any path already supported by observed edges remains visible. Tool errors remain in their existing error or failure fields. diff --git a/scripts/lib/real-hopper-workflows.mjs b/scripts/lib/real-hopper-workflows.mjs index c0b6595edc..9c71353adf 100644 --- a/scripts/lib/real-hopper-workflows.mjs +++ b/scripts/lib/real-hopper-workflows.mjs @@ -37,7 +37,7 @@ export async function verifyHopperWorkflows(call, invalid, procedure) { ); assert.equal(cycle.length, 2, "fixture omitted its source-owned call cycle"); const cycleGraph = await call("get_call_graph", { address: cycle[0].value }); - const cycleNodes = Object.values(cycleGraph).flat(); + const cycleNodes = Object.values(cycleGraph.graph).flat(); assert.equal(cycleNodes.length, 2); assert.deepEqual( cycleNodes.map((node) => node.address).sort(), diff --git a/src/application/CallPathTracing.ts b/src/application/CallPathTracing.ts index 65628cf326..825c69f445 100644 --- a/src/application/CallPathTracing.ts +++ b/src/application/CallPathTracing.ts @@ -157,6 +157,11 @@ const expandAddress = async ( reason: "Provider did not report relationship coverage.", }; state.relationshipCoverage.set(address, coverage); + for (const diagnostic of "diagnostics" in coverage && + Array.isArray(coverage.diagnostics) + ? coverage.diagnostics + : []) + state.limitations.add(diagnostic); if (coverage.status === "partial") for (const limitation of coverage.limitations) state.residual.add( diff --git a/src/application/EnhancedTools.ts b/src/application/EnhancedTools.ts index 89c65a41f3..a5d09ecc2a 100644 --- a/src/application/EnhancedTools.ts +++ b/src/application/EnhancedTools.ts @@ -33,6 +33,7 @@ import { inspectNativeDispatch } from "./native/NativeDispatchMetadataInspection import { traceNativeValues } from "./native/NativeValueTrace.js"; import { traceNativeUiAction } from "./native/NativeUiActionTrace.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; +import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; import { invalidEnhancedInput, @@ -343,15 +344,26 @@ export class EnhancedTools { const relation = input.direction === "forward" ? "callees" : "callers"; const tool = input.direction === "forward" ? "procedure_callees" : "procedure_callers"; - const resolved = await resolveProcedureAddress( - this.#call.bind(this), - input.address, + const limitations = new Set(); + const resolved = await this.#observe( + "procedure_address", + { procedure: input.address }, signal, ); if (!resolved.ok) return resolved; - const discovered = new Set([resolved.value]); + for (const limitation of resolved.value.limitations) + limitations.add(limitation); + const startAddress = resolved.value.result; + if (typeof startAddress !== "string" || startAddress.length === 0) + return err( + new AnalysisOutputError( + "procedure_address", + "provider returned no canonical procedure address", + ), + ); + const discovered = new Set([startAddress]); const queue: Array<{ address: string; depth: number }> = [ - { address: resolved.value, depth: 0 }, + { address: startAddress, depth: 0 }, ]; let queueIndex = 0; const graph: Record = {}; @@ -385,17 +397,40 @@ export class EnhancedTools { }); continue; } + for (const limitation of result.value.limitations) + limitations.add(limitation); + const reportedCoverage = result.value.relationshipCoverage; + const coverage = reportedCoverage ?? { + status: "unknown" as const, + reason: "Provider did not report relationship coverage.", + }; + const relationshipLimitations = + reportedCoverage === undefined + ? [ + ...result.value.limitations, + "Provider did not report relationship coverage.", + ] + : [ + ...(reportedCoverage.status === "partial" + ? reportedCoverage.limitations + : reportedCoverage.status === "unknown" + ? [reportedCoverage.reason] + : []), + ...("diagnostics" in reportedCoverage + ? reportedCoverage.diagnostics + : []), + ]; + const nodeLimitations = [...new Set(relationshipLimitations)].sort( + compareUnicodeCodePoints, + ); graph[level].push({ address: current.address, status: "ok", calls: [...related.value], - relationship_coverage: result.value.relationshipCoverage ?? { - status: "unknown", - reason: "Provider did not report relationship coverage.", - }, - ...(result.value.limitations.length === 0 + relationship_coverage: coverage, + ...(nodeLimitations.length === 0 ? {} - : { limitations: [...result.value.limitations] }), + : { limitations: nodeLimitations }), }); for (const address of related.value) { if (!discovered.has(address)) { @@ -404,7 +439,10 @@ export class EnhancedTools { } } } - return ok(graph); + return ok({ + graph, + limitations: [...limitations].sort(compareUnicodeCodePoints), + }); } async #analyzeSwiftTypes( diff --git a/src/application/InvestigationProviders.ts b/src/application/InvestigationProviders.ts index d1da371f5d..883d3878c2 100644 --- a/src/application/InvestigationProviders.ts +++ b/src/application/InvestigationProviders.ts @@ -18,7 +18,9 @@ export const workflowAnalysisProfile = ( ): AnalysisProfileCommitment => createAnalysisProfile(REA_WORKFLOW_PROVIDER, { upstream_analysis_profile: upstream, - ...(operation === "binary_overview" || operation === "batch_decompile" + ...(operation === "binary_overview" || + operation === "batch_decompile" || + operation === "get_call_graph" ? { workflow_revision: 2 } : {}), }); diff --git a/src/application/binary/AnalysisSnapshotCache.test.ts b/src/application/binary/AnalysisSnapshotCache.test.ts index 4d57e8a3d4..48c2a54bd1 100644 --- a/src/application/binary/AnalysisSnapshotCache.test.ts +++ b/src/application/binary/AnalysisSnapshotCache.test.ts @@ -14,6 +14,7 @@ import { createAnalysisSnapshotWorkflowEntry, parseAnalysisSnapshot, serializeAnalysisSnapshot, + snapshotEvidenceForQuery, snapshotBinding, snapshotTarget, } from "../../domain/analysisSnapshot.js"; @@ -30,6 +31,7 @@ import { SNAPSHOT_CACHE_ENTRY_CEILING, isSnapshotCacheable, } from "./AnalysisSnapshotCache.js"; +import { enhancedOutputSchemas } from "../../contracts/toolOutputSchemaGroups.js"; it("exports independent nested query, workflow and Evidence payloads", () => { const cache = new AnalysisSnapshotCache(); @@ -505,7 +507,10 @@ describe("relationship coverage snapshot replay", () => { const cache = new AnalysisSnapshotCache(); const operation = "procedure_callees" as const; const parameters = { procedure: "0x1000", document: "fixture" }; - const coverage = { status: "complete" as const }; + const coverage = { + status: "complete" as const, + diagnostics: ["The provider retained an unresolved call target."], + }; const execution = createAnalysisExecution( ["0x2000"], ANALYSIS_SNAPSHOT_PROVIDER, @@ -561,8 +566,8 @@ describe("relationship coverage snapshot replay", () => { const tamperedEntry = tampered.entries[0]; if (tamperedEntry === undefined) throw new Error("snapshot entry missing"); tamperedEntry.execution.relationship_coverage = { - status: "partial", - limitations: ["forged coverage"], + status: "complete", + diagnostics: ["forged coverage"], }; expect( new AnalysisSnapshotCache().import(tampered, undefined, () => ok(0)).ok, @@ -601,3 +606,58 @@ describe("relationship coverage snapshot replay", () => { }); }); }); + +it("does not replay pre-envelope call graphs under the revised workflow profile", () => { + const cache = new AnalysisSnapshotCache(); + const target = ANALYSIS_SNAPSHOT_TARGET; + const profile = ANALYSIS_SNAPSHOT_PROFILE; + const operation = "get_call_graph" as const; + const parameters = { address: "0x1000", direction: "forward" }; + const oldProfile = workflowAnalysisProfile(profile); + const revisedProfile = workflowAnalysisProfile(profile, operation); + const oldResult = { "0": [] }; + cache.recordWorkflow({ + target, + profile, + operation, + parameters, + execution: { + ...createAnalysisExecution(oldResult, REA_WORKFLOW_PROVIDER, { + subject: target, + rawResult: oldResult, + }), + analysisProfile: oldProfile, + }, + }); + const evidence = createEvidence(target, REA_WORKFLOW_PROVIDER, { + operation, + confidence: "derived", + parameters, + result: oldResult, + rawResult: oldResult, + analysisProfile: oldProfile, + }); + const exported = cache.export( + target, + profile, + createEvidenceBundle([evidence]), + ); + if (!exported.ok) throw exported.error; + const snapshot = parseAnalysisSnapshot(exported.value); + + expect( + snapshotEvidenceForQuery(snapshot, { + target, + bindingProfile: profile, + operation, + parameters, + provider: REA_WORKFLOW_PROVIDER, + evidenceProfile: revisedProfile, + }), + ).toBeUndefined(); + expect( + enhancedOutputSchemas.get_call_graph?.safeParse({ + normalized_result: oldResult, + }).success, + ).toBe(false); +}); diff --git a/src/contracts/toolOutputSchemaGroups.ts b/src/contracts/toolOutputSchemaGroups.ts index 3c97b3acac..3366be8ec7 100644 --- a/src/contracts/toolOutputSchemaGroups.ts +++ b/src/contracts/toolOutputSchemaGroups.ts @@ -304,7 +304,12 @@ export const enhancedOutputSchemas: Readonly> = { failed: z.number().int().min(0), }), ), - get_call_graph: evidenceResultOf(z.record(z.string(), z.array(graphNode))), + get_call_graph: evidenceResultOf( + z.strictObject({ + graph: z.record(z.string(), z.array(graphNode)), + limitations: z.array(z.string()), + }), + ), analyze_swift_types: evidenceResultOf( z.object({ total: z.number().int().min(0), diff --git a/src/domain/callPath.test.ts b/src/domain/callPath.test.ts index ca9515e39f..e0f343fca5 100644 --- a/src/domain/callPath.test.ts +++ b/src/domain/callPath.test.ts @@ -1,14 +1,9 @@ import { describe, expect, it } from "vitest"; -import { enhancedInputSchemas } from "../contracts/enhancedInputs.js"; import { buildCallPath } from "./callPath.js"; -import { - callPathInputSchema, - callPathResultSchema, -} from "./callPathSchemas.js"; +import { callPathResultSchema, type CallPathInput } from "./callPathSchemas.js"; import { createEvidence, type Evidence } from "./evidence.js"; -import { functionDossierSchema } from "./hopperValues.js"; -import { jsonValueSchema } from "./jsonValue.js"; +import type { JsonValue } from "./jsonValue.js"; import type { RelationshipCoverage } from "./relationshipCoverage.js"; const observe = ( @@ -21,7 +16,7 @@ const observe = ( readonly relationshipCoverage?: RelationshipCoverage; } = {}, ): Evidence => { - const value = functionDossierSchema.parse({ + const value = { procedure: { address, name: `fn_${address.slice(2)}`, @@ -56,16 +51,14 @@ const observe = ( native_value_flow: null, basic_blocks: [], limitations: [...(options.dossierLimitations ?? [])], - }); + } satisfies JsonValue; return createEvidence( { path: "/tmp/a", sha256: "a".repeat(64), format: "mach-o" }, { id: provider, name: provider, version: "1" }, { operation: "analyze_function", - parameters: enhancedInputSchemas.analyze_function.parse({ - procedure: address, - }), - result: jsonValueSchema.parse(value), + parameters: { procedure: address }, + result: value, confidence: "derived", authority: "shipped-artifact", ...(options.evidenceLimitations === undefined @@ -78,15 +71,16 @@ const observe = ( ); }; -const run = (functions: Evidence[], overrides: Record = {}) => - buildCallPath( - callPathInputSchema.parse({ - functions, - start: { address: "0x1000" }, - goal: { address: "0x4000" }, - ...overrides, - }), - ); +const run = ( + functions: Evidence[], + overrides: Partial> = {}, +) => + buildCallPath({ + functions, + start: { address: "0x1000" }, + goal: { address: "0x4000" }, + ...overrides, + }); describe("call path reconstruction", () => { it("returns deterministic shortest-first cited paths", () => { @@ -178,7 +172,7 @@ describe("call path reconstruction", () => { describe("call path topology and capacity", () => { it("retains every distinct generic limitation beyond the push argument limit", () => { const limitations = Array.from( - { length: 1_000_000 }, + { length: 150_000 }, (_, index) => `context-${index.toString().padStart(7, "0")}`, ); const result = run([ @@ -335,26 +329,23 @@ describe("call path topology and capacity", () => { }); describe("explicit call relationship coverage", () => { - it.each([150_000, 1_000_000])( - "accumulates %i partial reasons without argument-stack exhaustion", - (length) => { - const result = run([ - observe("0x1000", [], "rea-workflow", { - relationshipCoverage: { - status: "partial", - limitations: Array(length).fill("omitted"), - }, - evidenceLimitations: [], - dossierLimitations: [], - }), - ]); - expect(result).toMatchObject({ - status: "unknown", - search_scope: { exhaustive: false }, - limitations: ["omitted"], - }); - }, - ); + it("accumulates 150,000 partial reasons without argument-stack exhaustion", () => { + const result = run([ + observe("0x1000", [], "rea-workflow", { + relationshipCoverage: { + status: "partial", + limitations: Array(150_000).fill("omitted"), + }, + evidenceLimitations: [], + dossierLimitations: [], + }), + ]); + expect(result).toMatchObject({ + status: "unknown", + search_scope: { exhaustive: false }, + limitations: ["omitted"], + }); + }); it("retains contextual limits on a found path without reducing complete coverage", () => { const limitation = "Provider results reflect the current database state."; diff --git a/src/domain/callPath.ts b/src/domain/callPath.ts index 439a589490..2186c80102 100644 --- a/src/domain/callPath.ts +++ b/src/domain/callPath.ts @@ -244,6 +244,10 @@ const inspectSearch = ({ limitations.push(limitation); else if (coverage?.status === "unknown") limitations.push(coverage.reason); + for (const diagnostic of coverage !== null && "diagnostics" in coverage + ? coverage.diagnostics + : []) + limitations.push(diagnostic); // The requested goal is a terminal for this search. Its outgoing edges // cannot add a path to the goal, so its dossier limits are contextual // but do not make the searched start-to-goal closure incomplete. diff --git a/src/domain/evidence.test.ts b/src/domain/evidence.test.ts index fc2f199216..c5898aecc7 100644 --- a/src/domain/evidence.test.ts +++ b/src/domain/evidence.test.ts @@ -19,6 +19,39 @@ const TARGET: BinaryTarget = { const PROVIDER = { id: "fixture", name: "Fixture provider", version: "1" }; const PROFILE = createAnalysisProfile(PROVIDER, { loader: "default" }); +it("preserves omitted relationship diagnostics and authenticates reported diagnostics", () => { + const observation = { + operation: "procedure_callees", + parameters: { procedure: "0x1000" }, + result: ["0x2000"], + }; + const historical = createEvidence(TARGET, PROVIDER, { + ...observation, + relationshipCoverage: { status: "complete" }, + }); + expect(historical.relationship_coverage).toEqual({ status: "complete" }); + expect(parseEvidence(JSON.parse(JSON.stringify(historical)))).toEqual( + historical, + ); + const diagnosed = createEvidence(TARGET, PROVIDER, { + ...observation, + relationshipCoverage: { + status: "complete", + diagnostics: ["Only resolved direct call references are observed."], + }, + }); + expect(parseEvidence(JSON.parse(JSON.stringify(diagnosed)))).toEqual( + diagnosed, + ); + expect(diagnosed.evidence_id).not.toBe(historical.evidence_id); + expect(() => + parseEvidence({ + ...historical, + relationship_coverage: diagnosed.relationship_coverage, + }), + ).toThrow("semantic identifier"); +}); + it("preserves identity and bytes when reusing an owned immutable result", () => { const value = { nested: { values: ["observed", 1] } }; const mutable = createEvidence(TARGET, PROVIDER, { diff --git a/src/domain/relationshipCoverage.ts b/src/domain/relationshipCoverage.ts index 5568533435..1de973669e 100644 --- a/src/domain/relationshipCoverage.ts +++ b/src/domain/relationshipCoverage.ts @@ -1,16 +1,24 @@ import { z } from "zod"; +const diagnostics = z.array(z.string().min(1)).min(1); +const completeCoverage = z.strictObject({ status: z.literal("complete") }); +const partialCoverage = z.strictObject({ + status: z.literal("partial"), + limitations: z.array(z.string().min(1)).min(1), +}); +const unknownCoverage = z.strictObject({ + status: z.literal("unknown"), + reason: z.string().min(1), +}); + /** Explicit coverage state for a provider's observed caller/callee set. */ -export const relationshipCoverageSchema = z.discriminatedUnion("status", [ - z.strictObject({ status: z.literal("complete") }), - z.strictObject({ - status: z.literal("partial"), - limitations: z.array(z.string().min(1)).min(1), - }), - z.strictObject({ - status: z.literal("unknown"), - reason: z.string().min(1), - }), +export const relationshipCoverageSchema = z.union([ + completeCoverage, + completeCoverage.extend({ diagnostics }), + partialCoverage, + partialCoverage.extend({ diagnostics }), + unknownCoverage, + unknownCoverage.extend({ diagnostics }), ]); export type RelationshipCoverage = z.infer; diff --git a/src/ghidra/GhidraAnalysisProfile.ts b/src/ghidra/GhidraAnalysisProfile.ts index b8bec6c61e..a736cd0bf4 100644 --- a/src/ghidra/GhidraAnalysisProfile.ts +++ b/src/ghidra/GhidraAnalysisProfile.ts @@ -105,6 +105,7 @@ export const resolveGhidraAnalysisProfile = async ( load_image_observations: "source-mappings-entry-context-v2", function_body_evidence: "complete-inclusive-ranges-v1", function_references: "complete-body-and-entry-reference-manager-v2", + call_relationship_coverage: "resolved-call-reference-diagnostics-v1", string_inventory_evidence: "defined-data-coverage-v1", location_resolution: "explicit-address-exact-entry-symbol-first-v3", instruction_flow_evidence: "decoded-return-pcode-v1", @@ -189,6 +190,7 @@ export const resolveGhidraAnalysisProfile = async ( load_image_observations: "source-mappings-entry-context-v2", function_body_evidence: "complete-inclusive-ranges-v1", function_references: "complete-body-and-entry-reference-manager-v2", + call_relationship_coverage: "resolved-call-reference-diagnostics-v1", string_inventory_evidence: "defined-data-coverage-v1", location_resolution: "explicit-address-exact-entry-symbol-first-v3", instruction_flow_evidence: "decoded-return-pcode-v1", diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index 1c13280138..eb3ca11716 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -586,6 +586,7 @@ describe("Ghidra client projection", () => { expect(resolved.value.profile.parameters).toMatchObject({ language_id: "auto-from-header", compiler_spec_id: "auto-default", + call_relationship_coverage: "resolved-call-reference-diagnostics-v1", }); const result = await ghidra @@ -788,8 +789,11 @@ describe("Ghidra relationship coverage", () => { if (!execution.ok) throw execution.error; expect(execution.value.relationshipCoverage).toEqual({ status: "complete", + diagnostics: [ + "Only resolved Ghidra call references are returned; unresolved computed or indirect calls remain unknown, while function classifications distinguish thunks and externals.", + ], }); - expect(execution.value.limitations).toContain( + expect(execution.value.limitations).not.toContain( "Only resolved Ghidra call references are returned; unresolved computed or indirect calls remain unknown, while function classifications distinguish thunks and externals.", ); await client.close(); diff --git a/src/ghidra/GhidraProviderCapabilities.ts b/src/ghidra/GhidraProviderCapabilities.ts index b64b49d7f5..f3f94c6457 100644 --- a/src/ghidra/GhidraProviderCapabilities.ts +++ b/src/ghidra/GhidraProviderCapabilities.ts @@ -2,6 +2,7 @@ import type { CapabilityDescriptor, ProviderIdentity, } from "../application/AnalysisProvider.js"; +import type { RelationshipCoverage } from "../domain/relationshipCoverage.js"; import { hasWindowsNativeAuthority, windowsNativeAuthorityUnavailableReason, @@ -43,6 +44,20 @@ const stringInventoryLimitation = const searchLimitation = "Regex mode uses Java Pattern semantics. Matcher or compiler stack exhaustion returns a resource constraint without closing the session; literal mode avoids regex recursion and returns complete matching values."; +const resolvedCallReferenceLimitation = + "Only resolved Ghidra call references are returned; unresolved computed or indirect calls remain unknown, while function classifications distinguish thunks and externals."; + +/** Relationship-only caveats belong to each observed caller/callee result. */ +export const relationshipCoverageFor = ( + operation: string, +): RelationshipCoverage | undefined => + operation === "procedure_callers" || operation === "procedure_callees" + ? { + status: "complete", + diagnostics: [resolvedCallReferenceLimitation], + } + : undefined; + /** Limitation text for one admitted operation, including the common base. */ export const limitationsFor = (operation: string): readonly string[] => { const common = [ @@ -135,10 +150,7 @@ export const limitationsFor = (operation: string): readonly string[] => { ]; case "procedure_callers": case "procedure_callees": - return [ - ...common, - "Only resolved Ghidra call references are returned; unresolved computed or indirect calls remain unknown, while function classifications distinguish thunks and externals.", - ]; + return common; case "xrefs": return [ ...common, diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index d44998c6fb..b7ba24e0a7 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -85,6 +85,7 @@ import { healthLimitations, windowsP0Limitations, limitationsFor, + relationshipCoverageFor, } from "./GhidraProviderCapabilities.js"; import type { GhidraSessionError } from "./GhidraSessionError.js"; import type { GhidraSessionInfo } from "./GhidraSessionValues.js"; @@ -823,14 +824,14 @@ export const createGhidraProviderClient = (input: { if (!attested.ok) return attested; normalized = attested.value; } + const relationshipCoverage = relationshipCoverageFor(operation); return ok( createAnalysisExecution(normalized, committedProfile.provider, { rawResult: called.value, analysisProfile: committedProfile, - ...(operation === "procedure_callers" || - operation === "procedure_callees" - ? { relationshipCoverage: { status: "complete" as const } } - : {}), + ...(relationshipCoverage === undefined + ? {} + : { relationshipCoverage }), limitations: [ ...limitationsFor(operation), ...observationLimitations, diff --git a/src/ida/IdaCallGraph.ts b/src/ida/IdaCallGraph.ts index 930f14877e..f37d26af5b 100644 --- a/src/ida/IdaCallGraph.ts +++ b/src/ida/IdaCallGraph.ts @@ -62,6 +62,7 @@ export const unavailableCallGraph = (reason: string): CallGraphCallees => ({ coverage: { status: "unknown", reason: `Upstream callgraph was unavailable (${reason}); callee edges remain unknown.`, + diagnostics: [UNRECOGNIZED_CALLEE_LIMITATION], }, }); @@ -101,6 +102,7 @@ export const readCallGraphCallees = async ( BigInt(left) < BigInt(right) ? -1 : BigInt(left) > BigInt(right) ? 1 : 0, ); const limitations = [UNRECOGNIZED_CALLEE_LIMITATION]; + const diagnostics = [UNRECOGNIZED_CALLEE_LIMITATION]; const coverageLimitations: string[] = []; const truncated = row.truncated === true || @@ -124,11 +126,12 @@ export const readCallGraphCallees = async ( try { const raw = await call("lookup_funcs", { queries: targets }); const parsed = modernLookupBatchSchema.safeParse(raw); - if (!parsed.success) - limitations.push( - "IDA observed callee addresses but returned an unreadable name lookup result; the addresses are retained.", - ); - else { + if (!parsed.success) { + const limitation = + "IDA observed callee addresses but returned an unreadable name lookup result; the addresses are retained."; + limitations.push(limitation); + diagnostics.push(limitation); + } else { const rows = new Map(); for (const item of parsed.data) { const query = idaAddressSchema.safeParse(item.query); @@ -139,18 +142,19 @@ export const readCallGraphCallees = async ( const item = rows.get(target); if (item?.fn?.address === target && item.error === null) names.set(target, item.fn.name); - else - limitations.push( - `IDA observed callee address ${target} but could not resolve its name${item?.error == null ? "" : `: ${item.error}`}; the address is retained.`, - ); + else { + const limitation = `IDA observed callee address ${target} but could not resolve its name${item?.error == null ? "" : `: ${item.error}`}; the address is retained.`; + limitations.push(limitation); + diagnostics.push(limitation); + } } } } catch (cause: unknown) { if (cause instanceof AnalysisCancelledError) throw cause; if (!expectedReadFailure(cause)) throw cause; - limitations.push( - `IDA observed callee addresses but the name lookup failed (${errorReason(cause)}); the addresses are retained.`, - ); + const limitation = `IDA observed callee addresses but the name lookup failed (${errorReason(cause)}); the addresses are retained.`; + limitations.push(limitation); + diagnostics.push(limitation); } } return { @@ -161,9 +165,14 @@ export const readCallGraphCallees = async ( }; }), limitations, - coverage: - coverageLimitations.length === 0 - ? { status: "complete" } - : { status: "partial", limitations: coverageLimitations }, + coverage: { + ...(coverageLimitations.length === 0 + ? { status: "complete" as const } + : { + status: "partial" as const, + limitations: coverageLimitations, + }), + diagnostics, + }, }; }; diff --git a/tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts b/tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts index 511dec47c1..aaf1068ead 100644 --- a/tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts +++ b/tests/boundary/mcp/enhancedTools.03-returns-typed-graph-failures-and.test.ts @@ -37,17 +37,20 @@ describe("enhanced MCP tools", () => { expect(graph).toMatchObject({ ok: true, value: { - "0": [ - { - address: "0x1", - status: "error", - error: { - category: "execution_failure", - message: - "Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.", + graph: { + "0": [ + { + address: "0x1", + status: "error", + error: { + category: "execution_failure", + message: + "Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.", + }, }, - }, - ], + ], + }, + limitations: [], }, }); expect(unresolved).toEqual({ diff --git a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts index 54f61f8e0f..c13ea3c56b 100644 --- a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts +++ b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts @@ -56,21 +56,23 @@ describe("enhanced MCP tools", () => { expect(graph.isError).not.toBe(true); expect(graph.structuredContent).toMatchObject({ normalized_result: { - "0": [ - { - address: "0x1", - status: "ok", - calls: ["0x2"], - limitations: [limitation], - relationship_coverage: { - status: "partial", + graph: { + "0": [ + { + address: "0x1", + status: "ok", + calls: ["0x2"], limitations: [limitation], + relationship_coverage: { + status: "partial", + limitations: [limitation], + }, }, - }, - ], + ], + }, + limitations: [limitation], }, }); - const trace = await client.callTool({ name: "trace_call_path", arguments: { start: "0x1", goal: "0x2" }, @@ -97,7 +99,11 @@ describe("enhanced MCP tools", () => { return relationshipExecution(String(arguments_.procedure)); if (name !== "procedure_callees") throw new Error(`Unexpected operation: ${name}`); - return relationshipExecution([], [caveat], { status: "complete" }); + return relationshipExecution( + arguments_.procedure === "0x1" ? ["0x2"] : [], + [caveat], + { status: "complete" }, + ); }, }); @@ -107,14 +113,20 @@ describe("enhanced MCP tools", () => { }); expect(graph.structuredContent).toMatchObject({ normalized_result: { - "0": [ - { - limitations: [caveat], - relationship_coverage: { status: "complete" }, - }, - ], + graph: { + "0": [ + { status: "ok", relationship_coverage: { status: "complete" } }, + ], + "1": [ + { status: "ok", relationship_coverage: { status: "complete" } }, + ], + }, + limitations: [caveat], }, }); + expect(JSON.stringify(graph.structuredContent)?.split(caveat)).toHaveLength( + 2, + ); const trace = await client.callTool({ name: "trace_call_path", @@ -129,6 +141,41 @@ describe("enhanced MCP tools", () => { }, }); }); + + it("keeps unclassified legacy caveats on nodes with unknown coverage", async () => { + const caveat = "This legacy provider does not classify call limitations."; + const client = await connect({ + execute: async (name, arguments_) => { + if (name === "procedure_address") + return relationshipExecution(String(arguments_.procedure)); + if (name !== "procedure_callees") + throw new Error(`Unexpected operation: ${name}`); + return relationshipExecution([], [caveat]); + }, + }); + + const graph = await client.callTool({ + name: "get_call_graph", + arguments: { address: "0x1", direction: "forward" }, + }); + expect(graph.structuredContent).toMatchObject({ + normalized_result: { + graph: { + "0": [ + { + status: "ok", + relationship_coverage: { status: "unknown" }, + limitations: [ + "Provider did not report relationship coverage.", + caveat, + ], + }, + ], + }, + limitations: [caveat], + }, + }); + }); }); describe("enhanced MCP batches", () => { diff --git a/tests/boundary/process/jadxIntegration.test.ts b/tests/boundary/process/jadxIntegration.test.ts index 7f64229f26..86e2c5ab71 100644 --- a/tests/boundary/process/jadxIntegration.test.ts +++ b/tests/boundary/process/jadxIntegration.test.ts @@ -277,7 +277,7 @@ it("fetches all class pages rather than silently retaining the first upstream pa await verifyCleanup(launches); }); -it("preserves duplicate class-name rows and reports lookup ambiguity", async () => { +it("preserves duplicate class-name rows and occurrence counts", async () => { const { service, apk, launches, provider } = await setup( "duplicate-class-name", ); diff --git a/tests/boundary/providers/ida/calleeRelationships.test.ts b/tests/boundary/providers/ida/calleeRelationships.test.ts index 7ba797de46..9d8ed638d5 100644 --- a/tests/boundary/providers/ida/calleeRelationships.test.ts +++ b/tests/boundary/providers/ida/calleeRelationships.test.ts @@ -40,34 +40,41 @@ it("shows the callee edges of an ARM64 procedure in the call graph", async () => result.value, "the root lists both observed callees instead of an empty observed list", ).toMatchObject({ - "0": [ - { - address: "0x1000", - status: "ok", - calls: ["0x2000", "0x3000"], - limitations: expect.arrayContaining([ - "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", - ]), - }, - ], - "1": [ - { - address: "0x2000", - status: "ok", - calls: [], - limitations: expect.arrayContaining([ - "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", - ]), - }, - { - address: "0x3000", - status: "ok", - calls: [], - limitations: expect.arrayContaining([ - "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", - ]), - }, - ], + graph: { + "0": [ + { + address: "0x1000", + status: "ok", + calls: ["0x2000", "0x3000"], + limitations: expect.arrayContaining([ + "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", + ]), + }, + ], + "1": [ + { + address: "0x2000", + status: "ok", + calls: [], + limitations: expect.arrayContaining([ + "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", + ]), + }, + { + address: "0x3000", + status: "ok", + calls: [], + limitations: expect.arrayContaining([ + "This relationship read does not establish calls to targets IDA does not recognize as functions; imports and other non-function call targets remain unknown.", + ]), + }, + ], + }, + limitations: expect.arrayContaining([ + expect.stringContaining( + "Provider version identifies the REA IDA adapter.", + ), + ]), }); expect( requestedRoots(producer), @@ -183,15 +190,17 @@ it("preserves missing callgraph coverage through direct, graph, and trace workfl }); if (!graph.ok) throw graph.error; expect(graph.value).toMatchObject({ - "0": [ - { - status: "ok", - calls: [], - limitations: expect.arrayContaining([ - expect.stringContaining("callgraph was unavailable"), - ]), - }, - ], + graph: { + "0": [ + { + status: "ok", + calls: [], + limitations: expect.arrayContaining([ + expect.stringContaining("callgraph was unavailable"), + ]), + }, + ], + }, }); const trace = await application.execute("trace_call_path", { @@ -239,13 +248,18 @@ it("records a failed ARM64 call graph read as the node's error, not as a complet result.value, "the transport-truncated graph is a recorded failure and no callee list is invented", ).toEqual({ - "0": [ - { - address: "0x1000", - status: "error", - error: expect.objectContaining({ code: "unreadable_output" }), - }, - ], + graph: { + "0": [ + { + address: "0x1000", + status: "error", + error: expect.objectContaining({ code: "unreadable_output" }), + }, + ], + }, + limitations: expect.arrayContaining([ + expect.stringContaining("Live results are never replayed"), + ]), }); } finally { expect((await client.close()).ok).toBe(true); @@ -273,12 +287,15 @@ it("preserves an explicit root cap below the edge limit through every relationsh }); if (!graph.ok) throw graph.error; expect(graph.value).toMatchObject({ - "0": [ - { - calls: ["0x2000"], - limitations: expect.arrayContaining([limitation]), - }, - ], + graph: { + "0": [ + { + calls: ["0x2000"], + limitations: expect.arrayContaining([limitation]), + }, + ], + }, + limitations: expect.arrayContaining([limitation]), }); const trace = await application.execute("trace_call_path", { start: "main", diff --git a/tests/boundary/providers/ida/selectorProvenance.test.ts b/tests/boundary/providers/ida/selectorProvenance.test.ts index e18a1d3af9..40e92ae1cb 100644 --- a/tests/boundary/providers/ida/selectorProvenance.test.ts +++ b/tests/boundary/providers/ida/selectorProvenance.test.ts @@ -58,14 +58,17 @@ describe("provider-derived selectors in IDA composed workflows", () => { result.value, "the resolved root stays ok and only the derived node carries the producer fault", ).toMatchObject({ - "0": [{ address: "0x1000", status: "ok" }], - "1": [ - { - address: "0x2000", - status: "error", - error: { code: "unreadable_output" }, - }, - ], + graph: { + "0": [{ address: "0x1000", status: "ok" }], + "1": [ + { + address: "0x2000", + status: "error", + error: { code: "unreadable_output" }, + }, + ], + }, + limitations: expect.any(Array), }); } finally { expect((await client.close()).ok).toBe(true);