test(mcp): use one temporary directory owner

Merge main and keep comment-facet recovery distinct from generic provider guidance.

docs/mcp-contracts.md

@@ -1025,7 +1025,11 @@ analysis client. `open_binary.provider_id` accepts a concrete provider ID or
 concrete version, selection source, and complete analysis profile through
 `analysis_provider_binding`. Ambiguity and unknown, unavailable, or unsupported
 choices return typed selection details. A selected provider is never replaced
-automatically after a runtime failure.
+automatically after a runtime failure. When the bound route does not declare an
+operation, its error points to `binary_session` with `detail: "complete"` for
+candidate capabilities and an explicit `open_binary` provider choice. A
+candidate's availability does not guarantee support for the selected target;
+switching may discard provider session edits, and retained Evidence does not restore them.
 
 When an admitted `open_binary` attempt fails, `error.details.selection_at_failure`
 records the canonical binary path and SHA-256, resource root (including its

src/application/AnalysisContextQueries.ts

@@ -220,6 +220,7 @@ const unavailableFacet = (
       remediation:
         error.retryable ||
         error.remediationAction !== undefined ||
+        error.fallbackRemediationAction !== undefined ||
         error.continuingCallerGuidance?.remediationAction !== undefined
           ? projected.remediation.action
           : "Review provider availability in binary_session with detail complete and select another supported provider if appropriate. After switching, if a name is known and search_procedures is available, use it to find candidates and verify their identity and coordinates.",

src/application/DirectAnalysis.ts

@@ -25,8 +25,10 @@ import {
   retainAnalysisResultSources,
   type AnalysisResultWithSources,
 } from "./investigation/recordEvidenceSources.js";
-import { resolveInvestigationEvidenceClosure } from "./investigation/InvestigationEvidence.js";
-import { createInvestigationEvidenceClosureChecker } from "./investigation/InvestigationEvidence.js";
+import {
+  resolveInvestigationEvidenceClosure,
+  createInvestigationEvidenceClosureChecker,
+} from "./investigation/InvestigationEvidence.js";
 import type { Evidence } from "../domain/evidence.js";
 import type { NativeToolName } from "../contracts/native/nativeToolContracts.js";
 import type { EnhancedToolName } from "../contracts/enhancedInputs.js";

src/application/binary/BinarySessionExecution.ts

@@ -56,6 +56,19 @@ interface PrepareSessionExecutionInput {
   ) => AnalysisExecution | undefined;
 }
 
+/** Recovery for an operation absent from the selected binary route. */
+export const undeclaredOperationGuidance = (
+  operation: string,
+  entryCommentAvailable = false,
+): string => {
+  const entryComment =
+    entryCommentAvailable &&
+    (operation === "set_comment" || operation === "set_inline_comment")
+      ? `If the address is exactly a function entry and an entry comment is intended, use MCP annotate_native_function with procedure set to that entry and ${operation === "set_comment" ? "comment" : "inline_comment"}, or CLI rea annotate-native-function with the target, entry and ${operation === "set_comment" ? "--comment" : "--inline-comment"}. This edits session metadata, not arbitrary-address comments; whether CLI edits persist after the invocation depends on the provider and selected checkpoint mode. `
+      : "";
+  return `${entryComment}In MCP, inspect binary_session with detail: "complete" for candidate availability, target support, and capabilities. If a suitable candidate is available, explicitly call open_binary with the same target and its provider_id. In CLI, use rea providers to inspect readiness and capabilities; target support remains unknown until a target is selected. Select an available candidate with --provider and check the result. Switching may discard provider session edits; retained Evidence does not restore them. Candidate availability does not guarantee support for this target.`;
+};
+
 /** Resolve capability, profile, and snapshot state before invoking a provider. */
 export const prepareSessionExecution = (
   input: PrepareSessionExecutionInput,
@@ -73,19 +86,34 @@ export const prepareSessionExecution = (
   if (capability?.available !== true) {
     const selectionError = unboundOperationError(operation, active.route);
     if (selectionError !== undefined) return err(selectionError);
+    const entryCommentAvailable =
+      active.route.binding !== null &&
+      active.route.capabilities.get("annotate_native_function")?.available ===
+        true &&
+      active.route.capabilities.get("annotate_native_function")?.provider.id ===
+        active.route.binding.identity.id;
+    const undeclaredGuidance =
+      capability === undefined
+        ? undeclaredOperationGuidance(operation, entryCommentAvailable)
+        : undefined;
     return err(
       new AnalysisCapabilityUnavailableError(
         active.route.binding?.identity.id ?? active.route.identity.id,
         operation,
         capability?.reason ?? "operation is not declared by this provider",
-        {
-          ...(capability?.userMessage === undefined
-            ? {}
-            : { userMessage: capability.userMessage }),
-          ...(capability?.remediation === undefined
-            ? {}
-            : { remediationAction: capability.remediation }),
-        },
+        undeclaredGuidance === undefined
+          ? {
+              ...(capability?.userMessage === undefined
+                ? {}
+                : { userMessage: capability.userMessage }),
+              ...(capability?.remediation === undefined
+                ? {}
+                : { remediationAction: capability.remediation }),
+            }
+          : {
+              userMessage: `The selected provider does not declare ${operation}.`,
+              fallbackRemediationAction: undeclaredGuidance,
+            },
       ),
     );
   }

src/domain/analysisErrorCore.ts

@@ -220,6 +220,8 @@ export class AnalysisCapabilityUnavailableError extends AnalysisError {
   readonly _tag = "AnalysisCapabilityUnavailableError";
   override readonly userMessage: string | undefined;
   readonly remediationAction: string | undefined;
+  /** General recovery used only when a consumer has no more specific action. */
+  readonly fallbackRemediationAction: string | undefined;
   readonly siblingOperations: readonly string[] | undefined;
   readonly siblingReasons: Readonly<Record<string, string>> | undefined;
   /** Incomplete cleanup must settle before the operation can be retried. */
@@ -232,6 +234,7 @@ export class AnalysisCapabilityUnavailableError extends AnalysisError {
     options?: AnalysisErrorOptions & {
       readonly userMessage?: string;
       readonly remediationAction?: string;
+      readonly fallbackRemediationAction?: string;
       readonly siblingOperations?: readonly string[];
       readonly siblingReasons?: Readonly<Record<string, string>>;
       readonly retryable?: boolean;
@@ -243,6 +246,7 @@ export class AnalysisCapabilityUnavailableError extends AnalysisError {
     );
     this.userMessage = options?.userMessage;
     this.remediationAction = options?.remediationAction;
+    this.fallbackRemediationAction = options?.fallbackRemediationAction;
     this.siblingOperations = options?.siblingOperations;
     this.siblingReasons = options?.siblingReasons;
     this.retryable = options?.retryable === true && !this.cleanupIncomplete;

src/domain/analysisErrorPresentation.ts

@@ -90,6 +90,11 @@ export const analysisErrorRemediationAction = (
     return "Set environment.service_workers to allow to permit this capture, or remove the service worker registration through the browser. REA never removes existing registrations.";
   if (error instanceof AnalysisCapabilityUnavailableError && error.retryable)
     return "Review details.reason, then retry the same analysis after the condition clears.";
+  if (
+    error instanceof AnalysisCapabilityUnavailableError &&
+    error.fallbackRemediationAction !== undefined
+  )
+    return error.fallbackRemediationAction;
   if (error instanceof EvidenceFileError && error.published !== undefined)
     return error.published.path === undefined
       ? "Resolve the reported staging cleanup failure. Publication completed; its path identity is excluded."

src/domain/analysisErrorProjection.test.ts

@@ -311,6 +311,32 @@ describe("capability failure retry disposition", () => {
     ).toBe("Selected runtime diagnostic");
   });
 
+  it("uses generic recovery only when no more specific recovery exists", () => {
+    const project = (options: {
+      fallbackRemediationAction: string;
+      remediationAction?: string;
+      retryable?: boolean;
+    }) =>
+      projectAnalysisError(
+        new AnalysisCapabilityUnavailableError(
+          "fixture",
+          "inspect",
+          "unavailable",
+          options,
+        ),
+      ).remediation.action;
+    expect(project({ fallbackRemediationAction: "Generic" })).toBe("Generic");
+    expect(
+      project({
+        fallbackRemediationAction: "Generic",
+        remediationAction: "Specific",
+      }),
+    ).toBe("Specific");
+    expect(
+      project({ fallbackRemediationAction: "Generic", retryable: true }),
+    ).toContain("retry the same analysis");
+  });
+
   it("retains cleanup ownership before allowing a transient operation to retry", () => {
     expect(
       projectAnalysisError(

src/domain/explicitSensitiveFailure.test.ts

@@ -88,6 +88,30 @@ it.each([
   expect(JSON.stringify(projected)).not.toContain("explicit-secret");
 });
 
+it("preserves sanitized capability recovery specificity", () => {
+  for (const [options, expected] of [
+    [{ remediationAction: "Repair explicit-secret" }, "Repair [REDACTED]"],
+    [
+      { fallbackRemediationAction: "Select explicit-secret" },
+      "Select [REDACTED]",
+    ],
+  ] as const) {
+    const error = new AnalysisCapabilityUnavailableError(
+      "provider",
+      "inspect",
+      "unsupported",
+      options,
+    );
+    const sanitized = redactExplicitFailure(error, ["explicit-secret"]);
+    expect(projectAnalysisError(sanitized).remediation.action).toBe(expected);
+    if (!(sanitized instanceof AnalysisCapabilityUnavailableError))
+      throw new Error("expected capability failure");
+    expect(sanitized.remediationAction === undefined).toBe(
+      "fallbackRemediationAction" in options,
+    );
+  }
+});
+
 it.each([
   ["/metadata/private~1~0/numeric", ["private/~"], "/metadata"],
   ["/metadata/private/child", ["private"], "/metadata"],

src/domain/explicitSensitiveFailure.ts

@@ -286,6 +286,14 @@ const redactFailure = (
         ...(error.userMessage === undefined
           ? {}
           : { userMessage: text(error.userMessage) }),
+        ...(error.remediationAction === undefined
+          ? {}
+          : { remediationAction: text(error.remediationAction) }),
+        ...(error.fallbackRemediationAction === undefined
+          ? {}
+          : {
+              fallbackRemediationAction: text(error.fallbackRemediationAction),
+            }),
         ...(error.siblingOperations === undefined
           ? {}
           : { siblingOperations: error.siblingOperations }),

src/providers/AnalysisClientFailure.test.ts

@@ -101,6 +101,7 @@ it("preserves sibling operations, sibling reasons, and remediation action when r
     {
       userMessage: "Export failed",
       remediationAction: "Retry without project requirement",
+      fallbackRemediationAction: "Review provider choices",
       siblingOperations: ["procedure_assembly", "analyze_function"],
       siblingReasons: {
         analyze_function: "analyze_function export failed",
@@ -126,6 +127,7 @@ it("preserves sibling operations, sibling reasons, and remediation action when r
   expect(attributed.remediationAction).toBe(
     "Retry without project requirement",
   );
+  expect(attributed.fallbackRemediationAction).toBe("Review provider choices");
   expect(attributed.userMessage).toBe("Export failed");
   expect(attributed.retryable).toBe(true);
 });

src/providers/AnalysisClientFailure.ts

@@ -231,6 +231,9 @@ export const analysisClientFailureForOperation = (
         ...(cause.remediationAction === undefined
           ? {}
           : { remediationAction: cause.remediationAction }),
+        ...(cause.fallbackRemediationAction === undefined
+          ? {}
+          : { fallbackRemediationAction: cause.fallbackRemediationAction }),
         ...(cause.siblingOperations === undefined
           ? {}
           : { siblingOperations: cause.siblingOperations }),

tests/boundary/mcp/providerSelectionMcp.test.ts

@@ -1,6 +1,6 @@
 import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
 import { createHash } from "node:crypto";
-import { rm, writeFile } from "node:fs/promises";
+import { writeFile } from "node:fs/promises";
 import { join } from "node:path";
 
 import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
@@ -24,6 +24,7 @@ import {
 } from "../../../src/application/AnalysisProvider.js";
 import { AnalysisProviderRegistry } from "../../../src/application/binary/AnalysisProviderRegistry.js";
 import { composeBinarySession } from "../../../src/application/binary/BinarySessionComposition.js";
+import { undeclaredOperationGuidance } from "../../../src/application/binary/BinarySessionExecution.js";
 import { createAnalysisProfile } from "../../../src/domain/analysisProfile.js";
 import { analysisErrorRemediationAction } from "../../../src/domain/analysisErrorPresentation.js";
 import { AnalysisSnapshotMismatchError } from "../../../src/domain/evidenceErrors.js";
@@ -48,12 +49,8 @@ import {
 } from "../../../src/contracts/toolOutputSchemaPrimitives.js";
 
 const resources: Array<{ close(): Promise<unknown> }> = [];
-let directory: string | undefined;
 afterEach(async () => {
   await Promise.all(resources.splice(0).map((resource) => resource.close()));
-  if (directory !== undefined)
-    await rm(directory, { recursive: true, force: true });
-  directory = undefined;
 });
 
 it("delivers provider-owned diagnostic keys without narrowing their JSON facts", async () => {
@@ -107,9 +104,51 @@ it("delivers provider-owned diagnostic keys without narrowing their JSON facts",
   expect(JSON.parse(text.text)).toEqual(response.structuredContent);
 });
 
+const expectUnknownProviderKeepsBinding = async (
+  mcp: Client,
+  session: ReturnType<typeof composeBinarySession>,
+  workspace: string,
+  target: string,
+  opened: Awaited<ReturnType<Client["callTool"]>>,
+): Promise<void> => {
+  const unknown = await mcp.callTool({
+    name: "open_binary",
+    arguments: { path: target, provider_id: "missing" },
+  });
+  expect(unknown.isError).toBe(true);
+  const refusal = structured(unknown);
+  expect(refusal).toMatchObject({
+    error: {
+      details: {
+        selection_reason: "unknown_provider",
+        target_at_open_completion: structured(opened).result,
+        provider_id_at_open_completion: "beta",
+      },
+    },
+  });
+  expect(
+    structured(
+      await mcp.callTool({
+        name: "binary_session",
+        arguments: {},
+      }),
+    ).result,
+  ).toMatchObject({
+    analysis_provider_binding: { provider: { id: "beta" } },
+  });
+  await checkQueuedSwitchReceipt(
+    mcp,
+    session,
+    target,
+    workspace,
+    opened,
+    refusal,
+  );
+};
+
 describe("provider selection over MCP", () => {
   it("keeps open_binary and binary_session binding semantics exact", async () => {
-    directory = await createTestTempDirectory("rea-provider-mcp-");
+    const directory = await createTestTempDirectory("rea-provider-mcp-");
     const target = join(directory, "fixture.hop");
     await writeFile(target, "fixture");
     const starts: string[] = [];
@@ -199,6 +238,30 @@ describe("provider selection over MCP", () => {
     });
     expect(starts).toEqual(["beta"]);
 
+    const unsupported = await mcp.callTool({
+      name: "read_bytes",
+      arguments: { address: "0x1000", length: 4 },
+    });
+    expect(unsupported.isError).toBe(true);
+    expect(structured(unsupported)).toMatchObject({
+      error: {
+        code: "capability_unavailable",
+        message: expect.stringContaining("read_bytes"),
+        details: { provider_id: "beta", operation: "read_bytes" },
+        remediation: {
+          action: undeclaredOperationGuidance("read_bytes"),
+        },
+      },
+    });
+    const remediation = undeclaredOperationGuidance("read_bytes");
+    expect(parseMcpToolError(unsupported).error.message).not.toContain(
+      remediation,
+    );
+    expect(remediation).toMatch(
+      /binary_session.*complete.*open_binary.*provider_id/u,
+    );
+    expect(starts).toEqual(["beta"]);
+
     const status = structured(
       await mcp.callTool({
         name: "binary_session",
@@ -223,44 +286,18 @@ describe("provider selection over MCP", () => {
       ],
     });
 
-    const unknown = await mcp.callTool({
-      name: "open_binary",
-      arguments: { path: target, provider_id: "missing" },
-    });
-    expect(unknown.isError).toBe(true);
-    const refusal = structured(unknown);
-    expect(refusal).toMatchObject({
-      error: {
-        details: {
-          selection_reason: "unknown_provider",
-          target_at_open_completion: structured(opened).result,
-          provider_id_at_open_completion: "beta",
-        },
-      },
-    });
-    expect(
-      structured(
-        await mcp.callTool({
-          name: "binary_session",
-          arguments: {},
-        }),
-      ).result,
-    ).toMatchObject({
-      analysis_provider_binding: { provider: { id: "beta" } },
-    });
-    await checkQueuedSwitchReceipt(
+    await expectUnknownProviderKeepsBinding(
       mcp,
       session,
-      target,
       directory,
+      target,
       opened,
-      refusal,
     );
   });
 });
 
 it("offers admitted providers and retains unconfigured rejections", async () => {
-  directory = await createTestTempDirectory("rea-provider-mcp-mixed-");
+  const directory = await createTestTempDirectory("rea-provider-mcp-mixed-");
   const target = join(directory, "fixture.hop");
   await writeFile(target, "fixture");
   const starts: string[] = [];
@@ -462,7 +499,7 @@ const connectDetailFixture = async (starts: string[]) => {
 
 describe("binary_session detail over MCP", () => {
   it("defaults to a summary that keeps every decision fact and names what complete detail adds", async () => {
-    directory = await createTestTempDirectory("rea-session-detail-mcp-");
+    const directory = await createTestTempDirectory("rea-session-detail-mcp-");
     const target = join(directory, "fixture.hop");
     await writeFile(target, "fixture");
     const starts: string[] = [];
@@ -792,6 +829,171 @@ it("reports confirmed snapshot binding differences and keeps the active session"
   });
 });
 
+it.each([true, false])(
+  "offers an entry-comment handoff only when the selected provider admits it (%s)",
+  async (entryCommentAvailable) => {
+    const directory = await createTestTempDirectory("rea-comment-recovery-");
+    const target = join(directory, "fixture.hop");
+    await writeFile(target, "fixture");
+    const starts: string[] = [];
+    const selected = candidate("beta", starts);
+    const identity = selected.identity();
+    const annotation = capability(identity);
+    const session = composeBinarySession(
+      new AnalysisProviderRegistry([
+        {
+          ...selected,
+          capabilities: () => [
+            capability(identity),
+            entryCommentAvailable
+              ? { ...annotation, operation: "annotate_native_function" }
+              : {
+                  ...annotation,
+                  operation: "annotate_native_function",
+                  available: false,
+                  reason: "Selected analysis is read-only.",
+                  availabilityCode: "open_options_invalid",
+                },
+          ],
+        },
+      ]),
+    );
+    const server = createServer(
+      { kind: "session", session },
+      { logger: silentLogger },
+    );
+    const mcp = new Client({ name: "comment-recovery", version: "1" });
+    const [clientTransport, serverTransport] =
+      InMemoryTransport.createLinkedPair();
+    resources.push(mcp, server, { close: () => session.close() });
+    await server.connect(serverTransport);
+    await mcp.connect(clientTransport);
+    expect(
+      (
+        await mcp.callTool({
+          name: "open_binary",
+          arguments: { path: target, provider_id: "beta" },
+        })
+      ).isError,
+    ).not.toBe(true);
+    for (const [operation, field, flag] of [
+      ["set_comment", "comment", "--comment"],
+      ["set_inline_comment", "inline_comment", "--inline-comment"],
+    ] as const) {
+      const reply = await mcp.callTool({
+        name: operation,
+        arguments: { address: "0x1000", comment: "note" },
+      });
+      const error = parseMcpToolError(reply).error;
+      expect(error.code).toBe("capability_unavailable");
+      expect(error.details).toMatchObject({ provider_id: "beta", operation });
+      expect(error.remediation.action).toBe(
+        undeclaredOperationGuidance(operation, entryCommentAvailable),
+      );
+      if (entryCommentAvailable) {
+        expect(error.remediation.action).toContain("annotate_native_function");
+        expect(error.remediation.action).toContain(field);
+        expect(error.remediation.action).toContain(flag);
+      } else
+        expect(error.remediation.action).not.toContain(
+          "annotate_native_function",
+        );
+    }
+    expect(starts).toEqual([]);
+    const after = structured(
+      await mcp.callTool({ name: "binary_session", arguments: {} }),
+    );
+    expect(after.result).toMatchObject({
+      analysis_provider_binding: { provider: { id: "beta" } },
+    });
+  },
+);
+
+it("keeps containing-function recovery for undeclared comment facets", async () => {
+  const directory = await createTestTempDirectory(
+    "rea-undeclared-comment-facet-",
+  );
+  const target = join(directory, "fixture.hop");
+  await writeFile(target, "fixture");
+  const starts: string[] = [];
+  const selected = candidate("beta", starts);
+  const identity = selected.identity();
+  const session = composeBinarySession(
+    new AnalysisProviderRegistry([
+      {
+        ...selected,
+        capabilities: () => [
+          capability(identity),
+          {
+            ...capability(identity),
+            operation: "resolve_containing_procedure",
+          },
+        ],
+        createClient: (_target, profile) => ({
+          execute: async (operation) =>
+            ok(
+              createAnalysisExecution(
+                operation === "resolve_containing_procedure"
+                  ? {
+                      query_address: "0x1001",
+                      found: true,
+                      procedure: {
+                        address: "0x1000",
+                        name: "entry",
+                        classification: null,
+                        body: { available: false, reason: "not recorded" },
+                      },
+                    }
+                  : "entry",
+                profile?.provider ?? identity,
+              ),
+            ),
+          close: async () => ok(null),
+        }),
+      },
+    ]),
+  );
+  const server = createServer(
+    { kind: "session", session },
+    { logger: silentLogger },
+  );
+  const mcp = new Client({ name: "undeclared-comment-facet", version: "1" });
+  const [clientTransport, serverTransport] =
+    InMemoryTransport.createLinkedPair();
+  resources.push(mcp, server, { close: () => session.close() });
+  await server.connect(serverTransport);
+  await mcp.connect(clientTransport);
+  expect(
+    (
+      await mcp.callTool({
+        name: "open_binary",
+        arguments: { path: target, provider_id: "beta" },
+      })
+    ).isError,
+  ).not.toBe(true);
+  const reply = await mcp.callTool({
+    name: "inspect_address_context",
+    arguments: { address: "0x1001", document: "fixture" },
+  });
+  expect(reply.isError).not.toBe(true);
+  expect(structured(reply)).toMatchObject({
+    result: {
+      comment: {
+        state: "unavailable",
+        remediation: expect.stringContaining("inspect_analysis_view"),
+      },
+      inline_comment: {
+        state: "unavailable",
+        remediation: expect.stringContaining("inspect_analysis_view"),
+      },
+      bookmarks: {
+        state: "unavailable",
+        remediation: expect.stringContaining("open_binary"),
+      },
+    },
+  });
+});
+
 const candidate = (
   id: string,
   starts: string[],
@@ -995,7 +1197,9 @@ it("preserves a selected Ghidra project's read-only guidance when an edit is req
 });
 
 it("retains attributed mixed profile cleanup through registered MCP calls and partial retry", async () => {
-  directory = await createTestTempDirectory("rea-profile-completion-mcp-");
+  const directory = await createTestTempDirectory(
+    "rea-profile-completion-mcp-",
+  );
   const path = join(directory, "fixture.hop");
   await writeFile(path, "fixture");
   const starts: string[] = [];