From b7ebccf8d3c06b637bbca4dc3038e8e5175b0bfc Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:08:30 +0800 Subject: [PATCH] fix(navigation): reject malformed containing-procedure results Move the unchanged resolution contract into its domain owner and use it when building navigation context. Preserve provider failures instead of guessing absence from error text or malformed result shapes. Return null only for an explicit valid negative resolution. Remove the obsolete error-message matcher and partial name parser. Cover four malformed result shapes, original error identity, valid positive/negative facts and the typed MCP unreadable-output response. Original source failed six cases. Validation: navigation source/MCP and advertised schema checks passed. Compact and full generated catalogs are byte-identical to the parent. Static, docs, discovery and duplication checks passed. --- .../AnalysisContextQueries.test.ts | 100 +++++++++++++++++- src/application/AnalysisContextQueries.ts | 44 +++----- src/contracts/toolOutputSchemaGroups.ts | 8 +- src/contracts/toolOutputSchemaPrimitives.ts | 19 +--- src/domain/hopperValues.ts | 17 +++ .../boundary/mcp/navigationContextMcp.test.ts | 10 +- 6 files changed, 148 insertions(+), 50 deletions(-) diff --git a/src/application/AnalysisContextQueries.test.ts b/src/application/AnalysisContextQueries.test.ts index 41ccaab44d..0336a03f75 100644 --- a/src/application/AnalysisContextQueries.test.ts +++ b/src/application/AnalysisContextQueries.test.ts @@ -106,7 +106,12 @@ describe("analysis context queries: navigation", () => { ok( createAnalysisExecution( operation === "resolve_containing_procedure" - ? { found: false, procedure: null, reason: "not_in_procedure" } + ? { + query_address: "", + found: false, + procedure: null, + reason: "not_in_procedure", + } : "", provider, ), @@ -139,6 +144,99 @@ describe("analysis context queries: navigation", () => { }); }); +describe("navigation procedure admission", () => { + it.each([ + { condition: "missing result", value: null }, + { condition: "missing discriminator", value: {} }, + { + condition: "missing query identity", + value: { found: false, procedure: null, reason: "not_in_procedure" }, + }, + { + condition: "missing procedure facts", + value: { + query_address: "0x401000", + found: true, + procedure: { address: "0x401000", name: "main" }, + }, + }, + ])( + "rejects $condition instead of reporting no procedure", + async ({ value }) => { + const result = await getNavigationContext( + { + execute: async (operation) => + ok( + createAnalysisExecution( + operation === "current_address" ? "0x401000" : value, + provider, + ), + ), + }, + { document: "fixture" }, + ); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisOutputError", + operation: "resolve_containing_procedure", + }, + }); + }, + ); + + it.each(["no procedure exists", "not in a procedure"])( + "preserves a provider failure containing %s", + async (message) => { + const failure = new AnalysisProtocolError(`Provider failed: ${message}`); + const result = await getNavigationContext( + { + execute: async (operation) => + operation === "current_address" + ? ok(createAnalysisExecution("0x401000", provider)) + : err(failure), + }, + { document: "fixture" }, + ); + expect(result).toEqual(err(failure)); + if (result.ok) throw new Error("expected original provider failure"); + expect(result.error).toBe(failure); + }, + ); + + it("returns a validated containing procedure name", async () => { + const result = await getNavigationContext( + { + execute: async (operation) => + ok( + createAnalysisExecution( + operation === "current_address" + ? "0x401000" + : { + query_address: "0x401000", + found: true, + procedure: { + address: "0x401000", + name: "main", + classification: null, + body: { + available: false, + reason: "Fixture has no body ranges", + }, + }, + }, + provider, + ), + ), + }, + { document: "fixture" }, + ); + expect(result).toEqual( + ok({ document: "fixture", address: "0x401000", procedure: "main" }), + ); + }); +}); + describe("analysis context queries: address facets", () => { it("keeps unsupported address facets local and filters bookmarks", async () => { const bookmarkInputs: Readonly>[] = []; diff --git a/src/application/AnalysisContextQueries.ts b/src/application/AnalysisContextQueries.ts index bd68462335..ba19e3aae4 100644 --- a/src/application/AnalysisContextQueries.ts +++ b/src/application/AnalysisContextQueries.ts @@ -6,7 +6,10 @@ import type { import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; import type { JsonValue } from "../domain/jsonValue.js"; -import { analysisBookmarkSchema } from "../domain/hopperValues.js"; +import { + analysisBookmarkSchema, + containingProcedureResolutionSchema, +} from "../domain/hopperValues.js"; import { resolveAnalysisDocument } from "./AnalysisDocument.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -66,18 +69,22 @@ export const getNavigationContext = async ( parameters(documentName, address.value.result), executionOptions(signal), ); - if (!procedure.ok) { - if (!isMissingCurrentProcedure(procedure.error)) return procedure; - return ok({ - document: documentName, - address: address.value.result, - procedure: null, - }); - } + if (!procedure.ok) return procedure; + const resolution = containingProcedureResolutionSchema.safeParse( + procedure.value.result, + ); + if (!resolution.success) + return err( + new AnalysisOutputError( + "resolve_containing_procedure", + "expected a containing-procedure resolution", + { cause: resolution.error }, + ), + ); return ok({ document: documentName, address: address.value.result, - procedure: containingProcedureName(procedure.value.result), + procedure: resolution.data.found ? resolution.data.procedure.name : null, }); }; @@ -147,28 +154,11 @@ const facet = ( remediation: `Call ${operation} directly to inspect provider availability and diagnostics.`, }; -const isMissingCurrentProcedure = (error: AnalysisError): boolean => - /(?:no procedure exists|not in a procedure)/iu.test(error.message); - const isRequestLevelFailure = (error: AnalysisError): boolean => error._tag === "AnalysisCancelledError" || error._tag === "AnalysisTimeoutError" || error._tag === "NoBinaryOpenError"; -const containingProcedureName = (value: JsonValue): string | null => { - if (typeof value !== "object" || value === null || Array.isArray(value)) - return null; - if (value.found !== true) return null; - const procedure = value.procedure; - if ( - typeof procedure !== "object" || - procedure === null || - Array.isArray(procedure) - ) - return null; - return typeof procedure.name === "string" ? procedure.name : null; -}; - const resolvedAddress = ( result: Result | undefined, fallback: string, diff --git a/src/contracts/toolOutputSchemaGroups.ts b/src/contracts/toolOutputSchemaGroups.ts index 16add16138..367c9170d7 100644 --- a/src/contracts/toolOutputSchemaGroups.ts +++ b/src/contracts/toolOutputSchemaGroups.ts @@ -46,6 +46,7 @@ import { referenceEdgeSchema, unresolvedCallSchema, procedureIdentitySchema, + containingProcedureResolutionSchema, } from "../domain/hopperValues.js"; import { nativeApiInspectionResultSchema } from "../domain/native/nativeApiBoundary.js"; import { @@ -84,7 +85,6 @@ import { nativeInvestigationTraceSchema } from "../domain/native/nativeInvestiga import { addressList, addressedEntry, - containingProcedureResolution, functionDossierOutput, graphNode, lifecycleResultOf, @@ -204,7 +204,9 @@ export const officialOutputSchemas: Readonly> = { }), ), procedure_pseudo_code: evidenceResultOf(nullableText), - resolve_containing_procedure: evidenceResultOf(containingProcedureResolution), + resolve_containing_procedure: evidenceResultOf( + containingProcedureResolutionSchema, + ), search_procedures: analysisDetailOutputSchemaOf( z.array(z.object({ address: z.string(), value: z.string() })), nativeInventorySummaryViewResultSchema.extend({ @@ -242,7 +244,7 @@ const literalTraceOutput = evidenceResultOf( z.object({ target_address: z.string(), source_address: z.string(), - containing_procedure: containingProcedureResolution, + containing_procedure: containingProcedureResolutionSchema, }), ), truncated: z.boolean(), diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index 555e88ad86..8dd970667d 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -1,10 +1,7 @@ import { procedureInfoSchema } from "../domain/native/procedureInfo.js"; import { z } from "zod"; -import { - procedureIdentitySchema, - functionDossierSchema, -} from "../domain/hopperValues.js"; +import { functionDossierSchema } from "../domain/hopperValues.js"; import { analysisProfileSchema } from "../domain/analysisProfile.js"; import { evidenceSchema, @@ -554,20 +551,6 @@ export const addressedEntry = z.object({ address: z.string(), name: z.string(), }); -export const containingProcedureResolution = z.discriminatedUnion("found", [ - z.object({ - query_address: z.string(), - found: z.literal(true), - procedure: procedureIdentitySchema, - }), - z.object({ - query_address: z.string(), - found: z.literal(false), - procedure: z.null(), - reason: z.enum(["outside_segments", "not_in_procedure"]), - }), -]); - const unavailable = z.object({ available: z.literal(false), reason: z.string(), diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index dfe42cf987..53232fba5d 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -216,6 +216,23 @@ export const procedureIdentitySchema = z "Function body entry membership disagrees with procedure identity", }); }); +/** An explicit provider result distinguishes a containing function from known absence. */ +export const containingProcedureResolutionSchema = z.discriminatedUnion( + "found", + [ + z.object({ + query_address: z.string(), + found: z.literal(true), + procedure: procedureIdentitySchema, + }), + z.object({ + query_address: z.string(), + found: z.literal(false), + procedure: z.null(), + reason: z.enum(["outside_segments", "not_in_procedure"]), + }), + ], +); export const localVariableSchema = z .object({ description: z.string(), diff --git a/tests/boundary/mcp/navigationContextMcp.test.ts b/tests/boundary/mcp/navigationContextMcp.test.ts index 7bcf1d363d..961f795930 100644 --- a/tests/boundary/mcp/navigationContextMcp.test.ts +++ b/tests/boundary/mcp/navigationContextMcp.test.ts @@ -45,8 +45,16 @@ it.each([ selection: { current_document: "fixture", current_address: null }, operation: "current_address", }, + { + selection: { + current_document: "fixture", + current_address: "0x401000", + resolve_containing_procedure: null, + }, + operation: "resolve_containing_procedure", + }, ])( - "delivers a typed MCP failure when $operation is not a string", + "delivers a typed MCP failure when $operation returns malformed output", async ({ selection, operation }) => { const result = await navigate(selection); expect(JSON.stringify(result.content)).not.toContain(