From 676d29f9eef794cea668f5f09f0ff510c848a513 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:11:39 +0800 Subject: [PATCH] fix(native): preserve qualified feature trace observations Use one containing-procedure parser beside its declared schema for navigation, address facets, and feature trace references. Keep the trace cache and reference type bound to parsed facts, returning typed malformed output before SDK projection instead of passing unchecked JSON onward. Retain same-address search alternatives such as Kuna's distinct string encodings. References identify target addresses, so observe and emit each target once while retaining duplicate rows from the provider observation. Remove address-only match collapse and redundant residual deduplication. Document these semantics. Validation: regression-first runs reproduced four malformed-resolution failures and three alternative/reference-replay failures. Focused MCP and consumer controls passed after repair. Real native engines were not run. --- docs/mcp-contracts.md | 5 + docs/native-investigation.md | 6 +- src/application/AnalysisContextQueries.ts | 19 +-- src/application/EnhancedLiteralTracing.ts | 28 ++-- src/application/EnhancedToolTypes.ts | 3 +- src/domain/hopperValues.ts | 19 +++ ...enhancedTools.traceFeatureCoverage.test.ts | 130 +++++++++++++++++- 7 files changed, 174 insertions(+), 36 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index c37f8153c7..734e908f27 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -174,6 +174,11 @@ visible. When a requested goal is not reached but an expansion is unresolved, `goal_status` is `unknown`; `not_reached` means traversal exhausted without unresolved relationship reads. Cancellation remains a tool error. +`trace_feature` validates each reference's containing-procedure resolution using +the same contract as navigation context. Explicit absence remains a successful +negative observation; malformed provider replies return `unreadable_output` +for `resolve_containing_procedure` before successful-result projection. + REA prepares complete MCP results within the pinned stdio client's 10 MiB receive-buffer budget, including both text and structured representations and room for the JSON-RPC envelope. Tool errors omit `structuredContent` from the diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 89e541c895..0b9e149124 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -265,7 +265,11 @@ Undecodable objects retain native display text with `decoding.available: false` and a reason, so one uncertain object does not block unrelated inspections. Function dossiers and `trace_feature` string matches retain this same string evidence, including decoding uncertainty; procedure matches keep their own -named observation fields without invented string qualifications. Native call edges retain +named observation fields without invented string qualifications. Tracing keeps +same-address search alternatives, including different string encodings, in their +original search order and shares one reference read per target address within +the call. Reference rows are emitted once per target address, preserving any +duplicates in the provider's observation. Native call edges retain Hopper's partial `CallReference` classification and exact endpoints; detailed reference flags remain unavailable rather than being invented. Caller and callee procedure inventories come from independent Hopper APIs; REA retains observed diff --git a/src/application/AnalysisContextQueries.ts b/src/application/AnalysisContextQueries.ts index dc8d1a0d81..6735bb308a 100644 --- a/src/application/AnalysisContextQueries.ts +++ b/src/application/AnalysisContextQueries.ts @@ -8,7 +8,7 @@ import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { analysisBookmarkSchema, - containingProcedureResolutionSchema, + parseContainingProcedureResolution, } from "../domain/hopperValues.js"; import { resolveAnalysisDocument } from "./AnalysisDocument.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -70,7 +70,7 @@ export const getNavigationContext = async ( executionOptions(signal), ); if (!procedure.ok) return procedure; - const resolution = readContainingProcedure(procedure.value.result); + const resolution = parseContainingProcedureResolution(procedure.value.result); if (!resolution.ok) return resolution; return ok({ document: documentName, @@ -114,7 +114,7 @@ export const inspectAddressContext = async ( const name = facet(results[0], operations[0]); const procedureResult = results[1]; const resolution = procedureResult?.ok - ? readContainingProcedure(procedureResult.value.result) + ? parseContainingProcedureResolution(procedureResult.value.result) : procedureResult; const procedure: Facet = resolution?.ok ? { state: "available", value: resolution.value } @@ -161,19 +161,6 @@ const isRequestLevelFailure = (error: AnalysisError): boolean => error._tag === "AnalysisTimeoutError" || error._tag === "NoBinaryOpenError"; -const readContainingProcedure = (value: JsonValue) => { - const parsed = containingProcedureResolutionSchema.safeParse(value); - return parsed.success - ? ok(parsed.data) - : err( - new AnalysisOutputError( - "resolve_containing_procedure", - "expected a containing-procedure resolution", - { cause: parsed.error }, - ), - ); -}; - const matchingBookmarks = (value: JsonValue, address: string): Facet => { if (!Array.isArray(value)) return { diff --git a/src/application/EnhancedLiteralTracing.ts b/src/application/EnhancedLiteralTracing.ts index 4ce765f033..0def62ee91 100644 --- a/src/application/EnhancedLiteralTracing.ts +++ b/src/application/EnhancedLiteralTracing.ts @@ -6,6 +6,10 @@ import type { import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { + parseContainingProcedureResolution, + type ContainingProcedureResolution, +} from "../domain/hopperValues.js"; import { err, ok, type Result } from "../domain/result.js"; import type { EnhancedResult, @@ -81,7 +85,6 @@ const literalMatches = async ( signal?: AbortSignal, ): Promise> => { const matches: TraceMatch[] = []; - const matched = new Set(); for (const [tool, type] of sources) { const result = await call( tool, @@ -106,14 +109,11 @@ const literalMatches = async ( ), ); for (const item of items.data) { - const key = `${type}\u0000${item.address}`; - if (matched.has(key)) continue; matches.push( type === "string" ? { ...item, type: "string" } : { ...item, type: "procedure" }, ); - matched.add(key); } } return ok({ matches }); @@ -128,9 +128,13 @@ const traceReferences = async ( > => { const references: TraceReference[] = []; const residual: string[] = []; + // References identify targets by address, independently of search alternatives. + const tracedTargets = new Set(); // A source address is resolved once however many matches reference it. - const containing = new Map(); + const containing = new Map(); for (const match of request.matches) { + if (tracedTargets.has(match.address)) continue; + tracedTargets.add(match.address); const observed = await observe( "xrefs", { address: match.address }, @@ -139,12 +143,10 @@ const traceReferences = async ( if (!observed.ok) return observed; // Only an explicit producer report of truncation is a coverage gap; a // provider that reports nothing keeps its previous, unqualified result. - const unknown = `References to ${match.address} were truncated by the provider; additional references may exist.`; - if ( - observed.value.referencesTruncated === true && - !residual.includes(unknown) - ) - residual.push(unknown); + if (observed.value.referencesTruncated === true) + residual.push( + `References to ${match.address} were truncated by the provider; additional references may exist.`, + ); const sources = observed.value.result; if (!Array.isArray(sources)) return err( @@ -169,7 +171,9 @@ const traceReferences = async ( request.signal, ); if (!resolved.ok) return resolved; - containingProcedure = resolved.value; + const parsed = parseContainingProcedureResolution(resolved.value); + if (!parsed.ok) return parsed; + containingProcedure = parsed.value; containing.set(source, containingProcedure); } references.push({ diff --git a/src/application/EnhancedToolTypes.ts b/src/application/EnhancedToolTypes.ts index 25121bc100..f5cbd713ea 100644 --- a/src/application/EnhancedToolTypes.ts +++ b/src/application/EnhancedToolTypes.ts @@ -3,6 +3,7 @@ import type { z } from "zod"; import { AnalysisInputError } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import type { ContainingProcedureResolution } from "../domain/hopperValues.js"; import { err, type Result } from "../domain/result.js"; import { enhancedInputSchemas, @@ -14,7 +15,7 @@ export type { LiteralFeatureMatch as TraceMatch } from "../domain/native/literal export type TraceReference = { readonly target_address: string; readonly source_address: string; - readonly containing_procedure: JsonValue; + readonly containing_procedure: ContainingProcedureResolution; }; /** One enhanced call whose input was parsed at its owning adapter boundary. */ diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index b50328e51d..41cd06e858 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -235,6 +235,25 @@ export const containingProcedureResolutionSchema = z.discriminatedUnion( }), ], ); +export type ContainingProcedureResolution = z.infer< + typeof containingProcedureResolutionSchema +>; + +/** Validate provider procedure facts before a workflow interprets absence. */ +export const parseContainingProcedureResolution = ( + value: JsonValue, +): Result => { + const parsed = containingProcedureResolutionSchema.safeParse(value); + return parsed.success + ? ok(parsed.data) + : err( + new AnalysisOutputError( + "resolve_containing_procedure", + "expected a containing-procedure resolution", + { cause: parsed.error }, + ), + ); +}; export const localVariableSchema = z .object({ diff --git a/tests/boundary/mcp/enhancedTools.traceFeatureCoverage.test.ts b/tests/boundary/mcp/enhancedTools.traceFeatureCoverage.test.ts index c38ec1403e..c9df3b2383 100644 --- a/tests/boundary/mcp/enhancedTools.traceFeatureCoverage.test.ts +++ b/tests/boundary/mcp/enhancedTools.traceFeatureCoverage.test.ts @@ -7,6 +7,7 @@ import { AnalysisOutputError } from "../../../src/domain/analysisErrorCore.js"; import { err, ok } from "../../../src/domain/result.js"; import { closeEnhancedToolResources, connect } from "./enhancedToolsHarness.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; afterEach(closeEnhancedToolResources); @@ -116,11 +117,6 @@ const expectedTrace = ( source_address: "0x21", containing_procedure: containing("0x21"), }, - ...extraMatches.map(() => ({ - target_address: "0x10", - source_address: "0x20", - containing_procedure: containing("0x20"), - })), ].filter( ({ target_address }) => !emptyReferenceTargets.includes(target_address), ), @@ -200,7 +196,7 @@ describe("trace_feature reference coverage", () => { ]); expect( output.normalized_result, - "every reference is still reported with its containing procedure", + "each target's reference set is reported once with its containing procedure", ).toEqual( expectedTrace( [], @@ -298,3 +294,125 @@ it("preserves string search qualifications in trace matches without fabricating (output.normalized_result as { matches: object[] }).matches[1], ).not.toHaveProperty("string"); }); + +it("retains each same-address string encoding and reuses the address reference observation", async () => { + const calls: string[] = []; + const baseline = provider({ calls }); + // Kuna's canonical inventory identity includes the encoding at each address. + const rows = ["ascii", "utf8", "utf16"].map((encoding) => ({ + address: "0x10", + value: "hello", + string: { + encoding, + encoding_status: "observed" as const, + termination: "present_or_not_required" as const, + byte_length: encoding === "utf16" ? 12 : 6, + }, + })); + const output = await trace({ + execute: (operation, input, options) => + operation === "search_strings" + ? Promise.resolve(ok(createAnalysisExecution(rows, PROVIDER))) + : baseline.execute(operation, input, options), + }); + expect(output.normalized_result).toMatchObject({ + matches: rows.map((row) => ({ ...row, type: "string" })), + references: [ + { + target_address: "0x10", + source_address: "0x20", + containing_procedure: containing("0x20"), + }, + ], + }); + expect(calls.filter((call) => call.startsWith("xrefs:"))).toEqual([ + "xrefs:0x10", + ]); + expect( + calls.filter((call) => call.startsWith("resolve_containing_procedure:")), + ).toEqual(["resolve_containing_procedure:0x20"]); +}); + +it("keeps duplicate reference rows reported by one provider observation", async () => { + const baseline = provider(); + const output = await trace({ + execute: (operation, input, options) => + operation === "xrefs" + ? Promise.resolve( + ok(createAnalysisExecution(["0x20", "0x20"], PROVIDER)), + ) + : baseline.execute(operation, input, options), + }); + expect(output.normalized_result).toMatchObject({ + references: ["0x10", "0x10", "0x11", "0x11"].map((address) => ({ + target_address: address, + source_address: "0x20", + containing_procedure: containing("0x20"), + })), + }); +}); + +it.each([ + null, + {}, + { query_address: "0x20", found: true, procedure: null }, + { query_address: "0x20", found: false, procedure: null }, +])( + "returns a typed failure for malformed trace reference resolution: %j", + async (resolution) => { + const baseline = provider(); + const client = await connect({ + execute: (operation, input, options) => + operation === "resolve_containing_procedure" + ? Promise.resolve(ok(createAnalysisExecution(resolution, PROVIDER))) + : baseline.execute(operation, input, options), + }); + const result = await client.callTool({ + name: "trace_feature", + arguments: { query: "hello" }, + }); + expect(JSON.stringify(result.content)).not.toContain( + "Output validation error", + ); + expect(parseMcpToolError(result).error).toMatchObject({ + code: "unreadable_output", + details: { operation: "resolve_containing_procedure" }, + }); + }, +); + +it("retains a validated containing procedure and its unavailable body facts", async () => { + const baseline = provider(); + const resolution = { + query_address: "0x20", + found: true, + procedure: { + address: "0x20", + name: "hello_caller", + classification: null, + body: { available: false, reason: "Provider has no function ranges." }, + }, + } as const; + const output = await trace({ + execute: (operation, input, options) => + operation === "resolve_containing_procedure" + ? Promise.resolve( + ok( + createAnalysisExecution( + { ...resolution, query_address: String(input.address) }, + PROVIDER, + ), + ), + ) + : baseline.execute(operation, input, options), + }); + expect(output.normalized_result).toMatchObject({ + references: [ + { source_address: "0x20", containing_procedure: resolution }, + { + source_address: "0x21", + containing_procedure: { ...resolution, query_address: "0x21" }, + }, + ], + }); +});