Merge remote-tracking branch 'origin/main' into codex/address-context-recovery

# Conflicts:
#	src/application/AnalysisContextQueries.ts

scripts/verify-real-ghidra-com.mjs

@@ -16,6 +16,10 @@ import { Client } from "@modelcontextprotocol/client";
 import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
 import { Ajv2020 } from "ajv/dist/2020.js";
 import { parseEvidence } from "../dist/domain/evidence.js";
+import {
+  ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY,
+  ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY,
+} from "../dist/application/AnalysisContextQueries.js";
 import { functionDossierSchema } from "../dist/domain/native/functionDossier.js";
 import { nativeInstructionSchema } from "../dist/domain/native/nativeInstruction.js";
 import { inspectVerifierGhidraInstallation } from "./lib/ghidra-installation-inspection.mjs";
@@ -276,6 +280,70 @@ try {
   assert.equal(updated.annotations.inline_comment, changes.inline_comment);
   assert.equal(updated.dossier.procedure.name, changes.name);
   assert.ok(updated.dossier.pseudocode.includes(changes.name));
+  // Exercise the documented recovery through the public MCP catalog on real
+  // Ghidra, rather than assuming the provider exposes direct comment methods.
+  const contextCall = async (name, args) => {
+    const definition = tools.find((tool) => tool.name === name);
+    assert.ok(definition?.outputSchema, `${name} must be advertised`);
+    const response = await client.callTool(
+      { name, arguments: args },
+      { timeout: 240000, toolDefinition: definition },
+    );
+    return requireMcpOutput(response, name);
+  };
+  const context = (
+    await contextCall("inspect_address_context", {
+      address: fixture.entry,
+    })
+  ).result;
+  assert.equal(context.procedure.state, "available");
+  assert.equal(context.procedure.value.found, true);
+  for (const facet of [context.comment, context.inline_comment]) {
+    assert.equal(facet.state, "unavailable");
+    assert.equal(facet.remediation, ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY);
+  }
+  assert.equal(
+    context.bookmarks.remediation,
+    ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY,
+  );
+  const summary = parseEvidence(
+    await contextCall("analyze_function", {
+      procedure: context.procedure.value.procedure.address,
+    }),
+  ).normalized_result;
+  const comments = parseEvidence(
+    await contextCall("inspect_analysis_view", {
+      source: {
+        kind: "retained-evidence",
+        evidence_id: summary.parent_evidence_id,
+      },
+      view: { kind: "native", facet: "comments" },
+    }),
+  ).normalized_result;
+  for (const [kind, text] of [
+    ["comment", changes.comment],
+    ["inline", changes.inline_comment],
+  ])
+    assert.ok(
+      comments.item.some(
+        (item) =>
+          item.address === context.procedure.value.query_address &&
+          item.kind === kind &&
+          item.text === text,
+      ),
+    );
+  const outside = (
+    await contextCall("inspect_address_context", {
+      address: unlistedAddress,
+    })
+  ).result;
+  assert.equal(outside.procedure.state, "available");
+  assert.equal(outside.procedure.value.found, false);
+  assert.equal(outside.comment.state, "unavailable");
+  assert.equal(
+    outside.comment.remediation,
+    ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY,
+  );
   assert.equal(
     await call("procedure_address", { procedure: changes.name }),
     fixture.entry,
@@ -386,6 +454,7 @@ try {
       fresh_decompiler: true,
       clear_comments: true,
       cli_mcp_parity: true,
+      address_context_comment_recovery: true,
     },
     context_input_errors_preserved: true,
     original_unchanged: true,

src/application/AnalysisContextQueries.ts

@@ -6,6 +6,7 @@ import type {
 import type { AnalysisError } from "../domain/analysisErrorBase.js";
 import {
   AnalysisCancelledError,
+  AnalysisCapabilityUnavailableError,
   AnalysisInputError,
   AnalysisOutputError,
 } from "../domain/analysisErrorCore.js";
@@ -31,6 +32,14 @@ type Facet =
       readonly error?: AnalysisErrorProjection;
     };
 
+/** Recovery for a comment inside an observed containing function. */
+export const ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY =
+  'If analyze_function and inspect_analysis_view are available and analyze_function uses the same document, analyze result.procedure.value.procedure.address (summary), then page view {kind:"native", facet:"comments"} using normalized_result.parent_evidence_id. Match result.procedure.value.query_address and kind; respect coverage and limitations.';
+
+/** Fallback when an unavailable capability has no narrower recovery. */
+export const ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY =
+  "Check binary_session for other available operations. This facet remains unknown.";
+
 const parameters = (
   document: string | undefined,
   address?: string,
@@ -150,8 +159,12 @@ export const inspectAddressContext = async (
   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 commentRecovery =
+    resolution?.ok && resolution.value.found
+      ? ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY
+      : undefined;
+  const comment = facet(results[2], operations[2], commentRecovery);
+  const inlineComment = facet(results[3], operations[3], commentRecovery);
   const bookmarks = facet(results[4], operations[4]);
   return ok({
     address: input.address,
@@ -173,14 +186,16 @@ export const inspectAddressContext = async (
 const facet = (
   result: Result<AnalysisExecution, AnalysisError> | undefined,
   operation: AnalysisOperation,
+  capabilityRecovery?: string,
 ): Facet =>
   result?.ok === true
     ? { state: "available", value: result.value.result }
-    : unavailableFacet(result?.error, operation);
+    : unavailableFacet(result?.error, operation, capabilityRecovery);
 
 const unavailableFacet = (
   error: AnalysisError | undefined,
   operation: AnalysisOperation,
+  capabilityRecovery?: string,
 ): Facet => {
   if (error instanceof AnalysisInputError) {
     const projected = projectAnalysisError(error);
@@ -195,7 +210,12 @@ const unavailableFacet = (
   return {
     state: "unavailable",
     reason: error?.message ?? `${operation} returned no result`,
-    remediation: `Call ${operation} directly to inspect provider availability and diagnostics.`,
+    remediation:
+      error instanceof AnalysisCapabilityUnavailableError && !error.retryable
+        ? (error.remediationAction ??
+          capabilityRecovery ??
+          ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY)
+        : `Call ${operation} directly to inspect provider availability and diagnostics.`,
   };
 };
 

tests/boundary/mcp/addressContextBookmarks.test.ts

@@ -5,6 +5,10 @@ import { expect, it } from "vitest";
 import { createTestBinarySession } from "../../fixtures/binarySession.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js";
+import {
+  ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY,
+  ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY,
+} from "../../../src/application/AnalysisContextQueries.js";
 import {
   AnalysisCapabilityUnavailableError,
   AnalysisCancelledError,
@@ -155,3 +159,127 @@ it.each([false, true])(
     }
   },
 );
+
+it.each([
+  "containing",
+  "outside",
+  "unknown",
+  "retryable",
+  "provider",
+  "general",
+] as const)(
+  "offers function-comment recovery only when containment is known: %s",
+  async (containment) => {
+    const identity = { id: "fixture", name: "Fixture", version: "1" };
+    const directory = await createTestTempDirectory("rea-context-recovery-");
+    const target = join(directory, "fixture.com");
+    await writeFile(target, Buffer.from([0xb8, 0x00, 0x4c, 0xcd, 0x21]));
+    const calls: string[] = [];
+    const session = createTestBinarySession(() => ({
+      execute: async (operation) => {
+        calls.push(operation);
+        if (
+          operation === "comment" ||
+          operation === "inline_comment" ||
+          operation === "list_bookmarks" ||
+          (operation === "resolve_containing_procedure" &&
+            containment === "unknown")
+        )
+          return err(
+            new AnalysisCapabilityUnavailableError(
+              identity.id,
+              operation,
+              "not declared",
+              {
+                retryable:
+                  containment === "retryable" && operation !== "list_bookmarks",
+                ...(containment === "provider" && operation !== "list_bookmarks"
+                  ? { remediationAction: "Provider-specific recovery." }
+                  : {}),
+                ...(containment === "general"
+                  ? { userMessage: "General provider discovery guidance." }
+                  : {}),
+              },
+            ),
+          );
+        const value =
+          operation === "resolve_containing_procedure"
+            ? containment === "containing" ||
+              containment === "retryable" ||
+              containment === "provider" ||
+              containment === "general"
+              ? {
+                  query_address: "0x401001",
+                  found: true,
+                  procedure: {
+                    address: "0x401000",
+                    name: "entry",
+                    classification: null,
+                    body: { available: false, reason: "not recorded" },
+                  },
+                }
+              : {
+                  query_address: "0x401001",
+                  found: false,
+                  procedure: null,
+                  reason: "not_in_procedure",
+                }
+            : "entry";
+        return ok(createAnalysisExecution(value, identity));
+      },
+      close: async () => ok(null),
+    }));
+    const server = createServer({ kind: "session", session });
+    const client = new Client({ name: "context-recovery", version: "1" });
+    const [clientTransport, serverTransport] =
+      InMemoryTransport.createLinkedPair();
+    try {
+      await server.connect(serverTransport);
+      await client.connect(clientTransport);
+      expect(
+        (
+          await client.callTool({
+            name: "open_binary",
+            arguments: { path: target, format: "dos-com" },
+          })
+        ).isError,
+      ).not.toBe(true);
+      const response = await client.callTool({
+        name: "inspect_address_context",
+        arguments: { address: "0x401001", document: "fixture" },
+      });
+      expect(response.isError).not.toBe(true);
+      const { result } = response.structuredContent as {
+        result: {
+          comment: { state: string; remediation: string };
+          inline_comment: { state: string; remediation: string };
+          bookmarks: { remediation: string };
+        };
+      };
+      for (const [name, facet] of [
+        ["comment", result.comment],
+        ["inline_comment", result.inline_comment],
+      ] as const) {
+        expect(facet.state).toBe("unavailable");
+        if (containment === "provider") {
+          expect(facet.remediation).toBe("Provider-specific recovery.");
+        } else if (containment === "retryable") {
+          expect(facet.remediation).toContain(name);
+        } else if (containment === "containing" || containment === "general") {
+          expect(facet.remediation).toBe(
+            ADDRESS_CONTEXT_FUNCTION_COMMENT_RECOVERY,
+          );
+        } else {
+          expect(facet.remediation).toBe(ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY);
+        }
+      }
+      expect(result.bookmarks.remediation).toBe(
+        ADDRESS_CONTEXT_UNAVAILABLE_RECOVERY,
+      );
+      expect(calls).not.toContain("analyze_function");
+    } finally {
+      await client.close();
+      await server.close();
+    }
+  },
+);