diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 44a5eb96a0..3da235feb3 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -980,7 +980,9 @@ current/containing procedure. Its capability inventory exposes a `current_selection` mode and an `explicit_document` mode; the latter works when the caller supplies `document` and the provider lacks `current_document`. A cursor outside a procedure is represented as -`procedure: null`. `inspect_address_context` requires an explicit address and +`procedure: null` only after a valid explicit absence result. Malformed +procedure resolution returns typed unreadable output; provider errors never +become absence from their message text. `inspect_address_context` requires an explicit address and returns name, procedure, comment, inline-comment, and bookmark facets; unsupported facets are local `unavailable` outcomes. Use `current_document`, `current_address`, and `current_procedure` for direct single-field lookups; use diff --git a/src/application/AnalysisContextQueries.test.ts b/src/application/AnalysisContextQueries.test.ts index 41ccaab44d..3f876ff260 100644 --- a/src/application/AnalysisContextQueries.test.ts +++ b/src/application/AnalysisContextQueries.test.ts @@ -13,6 +13,7 @@ import { AnalysisOutputError, AnalysisProtocolError, } from "../domain/analysisErrorCore.js"; +import type { JsonValue } from "../domain/jsonValue.js"; import { err, ok } from "../domain/result.js"; const provider = { id: "fixture", name: "Fixture", version: "1" }; @@ -106,7 +107,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, ), @@ -117,6 +123,52 @@ describe("analysis context queries: navigation", () => { ); }); + it.each([ + null, + {}, + { found: false, procedure: null }, + { query_address: "0x401000", found: true, procedure: null }, + ])( + "keeps malformed containing-procedure output unknown: %j", + async (value) => { + const analysis: AnalysisOperationPort = { + execute: async (operation) => + ok( + createAnalysisExecution( + operation === "resolve_containing_procedure" + ? value + : operation === "current_document" + ? "fixture" + : "0x401000", + provider, + ), + ), + }; + expect(await getNavigationContext(analysis, {})).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisOutputError", + operation: "resolve_containing_procedure", + }, + }); + }, + ); + + it("does not infer procedure absence from a provider error message", async () => { + const failure = new AnalysisProtocolError( + "not in a procedure: malformed reply", + ); + const analysis: AnalysisOperationPort = { + execute: async (operation) => + operation === "resolve_containing_procedure" + ? err(failure) + : ok(createAnalysisExecution("0x401000", provider)), + }; + expect( + await getNavigationContext(analysis, { document: "fixture" }), + ).toEqual(err(failure)); + }); + it("uses an explicit document without passing it to current_document and preserves procedure failures", async () => { const calls: string[] = []; const failure = new AnalysisProtocolError("provider response malformed"); @@ -164,7 +216,15 @@ describe("analysis context queries: address facets", () => { { query_address: "0x401000", found: true, - procedure: { address: "0x401000", name: "main" }, + procedure: { + address: "0x401000", + name: "main", + classification: null, + body: { + available: false, + reason: "Fixture omits body ranges.", + }, + }, }, provider, ), diff --git a/src/application/AnalysisContextQueries.ts b/src/application/AnalysisContextQueries.ts index bd68462335..e3f108931b 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,13 @@ 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 = readContainingProcedure(procedure.value.result); + if (!resolution.ok) return resolution; return ok({ document: documentName, address: address.value.result, - procedure: containingProcedureName(procedure.value.result), + procedure: resolution.value.found ? resolution.value.procedure.name : null, }); }; @@ -114,7 +112,13 @@ export const inspectAddressContext = async ( if (!result.ok && isRequestLevelFailure(result.error)) return result; } const name = facet(results[0], operations[0]); - const procedure = facet(results[1], operations[1]); + const procedureResult = results[1]; + const resolution = procedureResult?.ok + ? readContainingProcedure(procedureResult.value.result) + : procedureResult; + const procedure: Facet = resolution?.ok + ? { state: "available", value: resolution.value } + : unavailableFacet(resolution?.error, operations[1]); const comment = facet(results[2], operations[2]); const inlineComment = facet(results[3], operations[3]); const bookmarks = facet(results[4], operations[4]); @@ -129,7 +133,7 @@ export const inspectAddressContext = async ( bookmarks.state === "available" ? matchingBookmarks( bookmarks.value, - resolvedAddress(results[1], input.address), + resolution?.ok ? resolution.value.query_address : input.address, ) : bookmarks, }); @@ -141,45 +145,32 @@ const facet = ( ): Facet => result?.ok === true ? { state: "available", value: result.value.result } - : { - state: "unavailable", - reason: result?.error.message ?? `${operation} returned no result`, - remediation: `Call ${operation} directly to inspect provider availability and diagnostics.`, - }; + : unavailableFacet(result?.error, operation); -const isMissingCurrentProcedure = (error: AnalysisError): boolean => - /(?:no procedure exists|not in a procedure)/iu.test(error.message); +const unavailableFacet = ( + error: AnalysisError | undefined, + operation: AnalysisOperation, +): Facet => ({ + state: "unavailable", + reason: error?.message ?? `${operation} returned no result`, + remediation: `Call ${operation} directly to inspect provider availability and diagnostics.`, +}); 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, -): string => { - if (result?.ok !== true) return fallback; - const value = result.value.result; - if (typeof value !== "object" || value === null || Array.isArray(value)) - return fallback; - return typeof value.query_address === "string" - ? value.query_address - : fallback; +const readContainingProcedure = (value: JsonValue) => { + const parsed = containingProcedureResolutionSchema.safeParse(value); + return parsed.success + ? ok(parsed.data) + : err( + new AnalysisOutputError( + "resolve_containing_procedure", + "expected a valid found or absent containing-procedure result", + ), + ); }; const matchingBookmarks = (value: JsonValue, address: string): Facet => { diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index 555e88ad86..d7113d36f5 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,19 +551,7 @@ 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"]), - }), -]); +export { containingProcedureResolutionSchema as containingProcedureResolution } from "../domain/hopperValues.js"; const unavailable = z.object({ available: z.literal(false), diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index dfe42cf987..1ed9d464e9 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -216,6 +216,24 @@ export const procedureIdentitySchema = z "Function body entry membership disagrees with procedure identity", }); }); +/** Exact containing-procedure outcome; valid absence is distinct from unreadable output. */ +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..a7b04a7822 100644 --- a/tests/boundary/mcp/navigationContextMcp.test.ts +++ b/tests/boundary/mcp/navigationContextMcp.test.ts @@ -58,3 +58,40 @@ it.each([ }); }, ); + +it.each([ + null, + {}, + { found: true, procedure: null }, + { query_address: "0x401000", found: false, procedure: null }, +])( + "rejects malformed procedure resolution rather than reporting absence: %j", + async (resolution) => { + const result = await navigate({ + current_document: "fixture", + current_address: "0x401000", + resolve_containing_procedure: resolution, + }); + expect(parseMcpToolError(result).error).toMatchObject({ + code: "unreadable_output", + details: { operation: "resolve_containing_procedure" }, + }); + }, +); + +it("keeps explicit procedure absence successful", async () => { + const result = await navigate({ + current_document: "fixture", + current_address: "0x401000", + resolve_containing_procedure: { + query_address: "0x401000", + found: false, + procedure: null, + reason: "not_in_procedure", + }, + }); + expect(result.isError).not.toBe(true); + expect(result.structuredContent).toMatchObject({ + result: { document: "fixture", address: "0x401000", procedure: null }, + }); +});