fix(mcp): report selection after failed target opens (#8726)

* fix(mcp): report selection after failed target opens

* fix(mcp): retain resource root selection in open failures

* test(session): assert selection during cleanup failures

* test(mcp): delay failed open response across target switch

* test(session): distinguish no selection from pending cleanup

* test(session): assert selections during cleanup and cancellation

* test(mcp): join the failed-open gate before restoring its spy

The snapshot-switch gate waited only on its acknowledgement, so a request
that settled early hung until the runner timeout, and a later assertion
failure restored the spy while that request was still in flight. Race the
acknowledgement and join the launched open in finally before mockRestore.

---------

Co-authored-by: Cursor Agent <[email protected]>

docs/mcp-contracts.md

@@ -989,6 +989,15 @@ concrete version, selection source, and complete analysis profile through
 choices return typed selection details. A selected provider is never replaced
 automatically after a runtime failure.
 
+When an admitted `open_binary` attempt fails, `error.details.selection_at_failure`
+records the canonical binary path and SHA-256, resource root (including its
+caller-selected path), or absence of selection at
+the end of that serialized attempt. This is a past observation: a queued or
+later operation may already have changed the selection. No selection does not
+establish that cleanup is complete. Call `binary_session` for current state
+and cleanup status. Validation failures that do not enter the session have no
+such receipt. The original error and any cleanup recovery remain authoritative.
+
 `open_binary` resolves an Apple bundle directory (`.app`, `.appex`,
 `.framework`, `.xpc`, `.systemextension`, `.bundle` or `.plugin`, compared
 case-insensitively) to the program file its `Info.plist` names in

src/application/binary/BinarySession.ts

@@ -35,6 +35,8 @@ import type {
 } from "../AnalysisProvider.js";
 import type {
   BinarySessionPort,
+  BinaryOpenResult,
+  OpenSelectionReceipt,
   SavedAnalysisSnapshot,
 } from "./BinarySessionPort.js";
 import {
@@ -478,7 +480,7 @@ export class BinarySession
   open(
     path: string,
     options: BinarySessionOpenOptions = {},
-  ): Promise<Result<BinaryTarget, AnalysisError>> {
+  ): Promise<BinaryOpenResult> {
     return this.#open(
       (admitted) =>
         resolveSessionOpen({
@@ -498,7 +500,7 @@ export class BinarySession
   openResolvedTarget(
     resolved: Pick<ResolvedSessionOpen, "target" | "route">,
     options: Pick<BinarySessionOpenOptions, "signal" | "snapshot"> = {},
-  ): Promise<Result<BinaryTarget, AnalysisError>> {
+  ): Promise<BinaryOpenResult> {
     return this.#open(
       () =>
         Promise.resolve(
@@ -519,20 +521,44 @@ export class BinarySession
       admitted: BinarySessionOpenOptions,
     ) => Promise<Result<ResolvedSessionOpen, AnalysisError>>,
     options: BinarySessionOpenOptions,
-  ): Promise<Result<BinaryTarget, AnalysisError>> {
+  ): Promise<BinaryOpenResult> {
     return this.#serialize(async (admittedThrough) => {
-      const refused = this.#targetAcquisitionRefused("open_binary");
-      if (refused !== undefined) return refused;
-      const admitted = await this.#withLoadedSnapshot(options);
-      if (!admitted.ok) return admitted;
-      return this.#openWithinTransition(
-        admittedThrough,
-        () => resolve(admitted.value),
-        admitted.value,
-      );
+      const attempt = async (): Promise<
+        Result<BinaryTarget, AnalysisError>
+      > => {
+        const refused = this.#targetAcquisitionRefused("open_binary");
+        if (refused !== undefined) return refused;
+        const admitted = await this.#withLoadedSnapshot(options);
+        if (!admitted.ok) return admitted;
+        return this.#openWithinTransition(
+          admittedThrough,
+          () => resolve(admitted.value),
+          admitted.value,
+        );
+      };
+      const result = await attempt();
+      return result.ok
+        ? result
+        : { ...result, selection: this.#openSelectionReceipt() };
     });
   }
 
+  #openSelectionReceipt(): OpenSelectionReceipt {
+    if (this.#active !== undefined)
+      return {
+        kind: "binary",
+        path: this.#active.target.path,
+        sha256: this.#active.target.sha256,
+      };
+    if (this.#activeBundleResources !== undefined)
+      return {
+        kind: "resources",
+        path: this.#activeBundleResources.root.canonicalPath,
+        caller_path: this.#activeBundleResources.root.callerPath,
+      };
+    return { kind: "none" };
+  }
+
   /** Read the staged snapshot only after earlier lifecycle transitions have settled. */
   async #withLoadedSnapshot(
     options: BinarySessionOpenOptions,

src/application/binary/BinarySessionBundleResources.test.ts

@@ -157,6 +157,13 @@ it("opens a resource-only bundle without executable identity and keeps snapshot
     session_kind: "bundle_resources",
     resource_root: { canonical_path: opened.value.canonicalPath },
   });
+  const rejectedBinary = await session.open(join(rootPath, "missing-binary"));
+  if (rejectedBinary.ok) throw new Error("Expected a missing binary refusal");
+  expect(rejectedBinary.selection).toEqual({
+    kind: "resources",
+    path: opened.value.canonicalPath,
+    caller_path: opened.value.callerPath,
+  });
   const snapshot = session.exportAnalysisSnapshot();
   if (snapshot.ok)
     throw new Error("resource session exported a binary snapshot");
@@ -652,10 +659,15 @@ it("keeps an earlier resource close failure primary when binary candidate cleanu
   const result = await opening;
   expect(result.ok).toBe(false);
   if (result.ok) throw new Error("Expected failed binary open");
+  expect(result.selection).toEqual({ kind: "none" });
   expect(result.error).toBe(originalFailure);
   expect(result.error.cleanupPrimary).toBeUndefined();
   expect(binaryCloseAttempts).toBe(1);
   expect(resourceCloseAttempts).toBe(1);
+  expect(await session.status()).toMatchObject({
+    open: false,
+    cleanup_pending: true,
+  });
   expect(await session.close()).toMatchObject({ ok: true });
   expect(resourceCloseAttempts).toBe(2);
 });

src/application/binary/BinarySessionPort.ts

@@ -30,6 +30,24 @@ import type {
 import type { SavedAnalysisSnapshot } from "../../domain/savedAnalysisSnapshot.js";
 export type { SavedAnalysisSnapshot } from "../../domain/savedAnalysisSnapshot.js";
 
+/** Selection observed at the end of one serialized open attempt. */
+export type OpenSelectionReceipt =
+  | { readonly kind: "binary"; readonly path: string; readonly sha256: string }
+  | {
+      readonly kind: "resources";
+      readonly path: string;
+      readonly caller_path: string;
+    }
+  | { readonly kind: "none" };
+
+export type BinaryOpenResult =
+  | { readonly ok: true; readonly value: BinaryTarget }
+  | {
+      readonly ok: false;
+      readonly error: AnalysisError;
+      readonly selection: OpenSelectionReceipt;
+    };
+
 /** Target lifecycle used by CLI and MCP without exposing a concrete provider. */
 export interface BinarySessionPort
   extends AnalysisOperationPort, InvestigationRecordPort {
@@ -72,7 +90,7 @@ export interface BinarySessionPort
       readonly loadSnapshot?: AnalysisSnapshotLoader;
       readonly providerId?: AnalysisProviderSelector;
     },
-  ): Promise<Result<BinaryTarget, AnalysisError>>;
+  ): Promise<BinaryOpenResult>;
   /** Open a resource-only app root without selecting or inventing an executable target. */
   openBundleResources(
     path: string,

src/application/binary/BinarySessionRestoration.test.ts

@@ -98,6 +98,7 @@ it.each([
       );
       expect(result.ok).toBe(false);
       if (result.ok) return;
+      expect(result.selection).toEqual({ kind: "none" });
       expect(session.activeTarget()).toBeUndefined();
       expect(await session.status()).toMatchObject({
         open: false,
@@ -119,6 +120,7 @@ it.each([
       expect(projectAnalysisError(result.error).code).toBe(
         cancelled ? "cancelled" : "unreadable_output",
       );
+      expect(result.selection).toEqual({ kind: "none" });
       expect(projectAnalysisError(result.error)).toMatchObject({
         details: {
           diagnostics: {

src/contracts/sessionToolContracts.ts

@@ -68,7 +68,7 @@ export const exportEvidenceBundleInputSchema = z.strictObject({
 export const SESSION_TOOL_CONTRACTS = [
   session(
     "open_binary",
-    "Open a local executable, application bundle, archive, JavaScript, source map, plist, or analysis database after validation. format=dos-com explicitly interprets 1..65280 headerless bytes as a DOS COM analysis image. format=raw-binary interprets headerless bytes with a required architecture and entry_point; file offset 0 maps at that address. Omission preserves header-based detection. provider_id selects one deep provider or deterministic auto selection; the binding remains stable until close or an explicit switch, with no failure fallback. Reopening the same target and profile retains its active database and metadata edits. An optional analysis snapshot is imported atomically and must match the binary identity, concrete provider, and canonical analysis profile exactly; importing into a session with metadata mutations requires closing and reopening it first. Switching targets preserves investigation records. Call close_binary to release engine resources; reset_investigation explicitly clears investigation records.",
+    "Open a local executable, application bundle, archive, JavaScript, source map, plist, or analysis database after validation. format=dos-com explicitly interprets 1..65280 headerless bytes as a DOS COM analysis image. format=raw-binary interprets headerless bytes with a required architecture and entry_point; file offset 0 maps at that address. Omission preserves header-based detection. provider_id selects one deep provider or deterministic auto selection; the binding remains stable until close or an explicit switch, with no failure fallback. Reopening the same target and profile retains its active database and metadata edits. An optional analysis snapshot is imported atomically and must match the binary identity, concrete provider, and canonical analysis profile exactly; importing into a session with metadata mutations requires closing and reopening it first. Switching targets preserves investigation records. A failed admitted open reports the selection at failure in error.details.selection_at_failure; later calls can change it, so use binary_session for current state. Call close_binary to release engine resources; reset_investigation explicitly clears investigation records.",
     openBinaryInputSchema,
   ),
   session(

src/server/ToolResultDelivery.ts

@@ -194,8 +194,13 @@ export class ToolResultDelivery {
   toErrorToolResult(
     error: AnalysisError,
     targetContent: boolean,
+    details?: Readonly<Record<string, JsonValue>>,
   ): CallToolResult {
-    const projected = projectAnalysisError(error);
+    const base = projectAnalysisError(error);
+    const projected =
+      details === undefined
+        ? base
+        : { ...base, details: { ...base.details, ...details } };
     if (
       error instanceof NoBinaryOpenError &&
       this.noTargetRecovery !== undefined

src/server/registerSessionTools.ts

@@ -237,7 +237,13 @@ const registerOpenLifecycleTool = ({
             },
             openContract,
           )
-        : server.delivery.toCallToolResult(opened, openContract);
+        : server.delivery.toErrorToolResult(
+            opened.error,
+            openContract.returnsTargetContent,
+            {
+              selection_at_failure: opened.selection,
+            },
+          );
     },
   );
 };

src/server/toolLogging.ts

@@ -2,11 +2,13 @@ import type { Result } from "../domain/result.js";
 import type { Logger } from "pino";
 
 /** Log one MCP tool's duration and status without recording caller input. */
-export const logToolExecution = async <Value, Failure>(
+export const logToolExecution = async <
+  Outcome extends Result<unknown, unknown>,
+>(
   logger: Logger,
   tool: string,
-  execute: () => Promise<Result<Value, Failure>>,
-): Promise<Result<Value, Failure>> => {
+  execute: () => Promise<Outcome>,
+): Promise<Outcome> => {
   const startedAt = performance.now();
   const result = await execute();
   logger[result.ok ? "info" : "warn"](

tests/boundary/mcp/providerSelectionMcp.test.ts

@@ -6,11 +6,14 @@ import { join } from "node:path";
 import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
 import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv";
 import type { CallToolResult } from "@modelcontextprotocol/server";
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { z } from "zod";
 import { jsonObjectSchema } from "../../../src/domain/jsonValue.js";
 
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+import { createDeferred } from "../../fixtures/binarySession.js";
+import { awaitAcknowledgement } from "../../fixtures/lifecycleAcknowledgement.js";
+import type { BinarySession } from "../../../src/application/binary/BinarySession.js";
 
 import {
   createAnalysisExecution,
@@ -550,6 +553,68 @@ describe("binary_session detail over MCP", () => {
   });
 });
 
+const expectedOpenSelection = (path: string, sha256: string) => ({
+  kind: "binary",
+  path,
+  sha256,
+});
+
+const expectQueuedOpenReceipt = async (
+  mcp: Client,
+  session: BinarySession,
+  first: string,
+  second: string,
+  snapshotPath: string,
+) => {
+  const call = (name: string, args: Record<string, unknown>) =>
+    mcp.callTool({ name, arguments: args });
+  const originalOpen = session.open.bind(session);
+  const failedSettled = createDeferred<void>();
+  const deliverFailure = createDeferred<void>();
+  const intercept = vi
+    .spyOn(session, "open")
+    .mockImplementation(async (path, options) => {
+      const result = await originalOpen(path, options);
+      if (path === second && options?.loadSnapshot !== undefined) {
+        failedSettled.resolve();
+        await deliverFailure.promise;
+      }
+      return result;
+    });
+  // Launch after the release gate exists so a failed assertion still joins it.
+  const failedSwitch = call("open_binary", {
+    path: second,
+    provider_id: "beta",
+    snapshot_path: snapshotPath,
+  });
+  try {
+    await awaitAcknowledgement(
+      failedSettled.promise,
+      failedSwitch,
+      "failed snapshot open",
+    );
+    const nextReply = await call("open_binary", {
+      path: second,
+      provider_id: "beta",
+    });
+    expect(nextReply.isError).not.toBe(true);
+    const sha = (value: string) =>
+      createHash("sha256").update(value).digest("hex");
+    expect((await call("binary_session", {})).structuredContent).toMatchObject({
+      result: { path: second, sha256: sha("second fixture") },
+    });
+    deliverFailure.resolve();
+    const failedReply = await failedSwitch;
+    expect(
+      parseMcpToolError(failedReply).error.details?.selection_at_failure,
+    ).toEqual(expectedOpenSelection(first, sha("first fixture")));
+  } finally {
+    deliverFailure.resolve();
+    await failedSwitch.catch(() => undefined);
+    intercept.mockRestore();
+  }
+};
+
 it("reports confirmed snapshot binding differences and keeps the active session", async () => {
   const testDirectory = await createTestTempDirectory(
     "rea-snapshot-mismatch-mcp-",
@@ -613,9 +678,11 @@ it("reports confirmed snapshot binding differences and keeps the active session"
       provider_id: provider,
       snapshot_path: snapshotPath,
     });
-    expect(result.isError).toBe(true);
     const error = parseMcpToolError(result).error;
     expect(error.code).toBe("evidence_integrity_mismatch");
+    expect(error.details?.selection_at_failure).toEqual(
+      expectedOpenSelection(first, sha("first fixture")),
+    );
     expect(error.remediation.action).toBe(
       analysisErrorRemediationAction(new AnalysisSnapshotMismatchError("")),
     );
@@ -667,6 +734,8 @@ it("reports confirmed snapshot binding differences and keeps the active session"
     true,
   );
 
+  await expectQueuedOpenReceipt(mcp, session, first, second, snapshotPath);
+
   revision = 1;
   const raw = join(testDirectory, "raw.bin");
   const rawSnapshot = join(testDirectory, "raw-snapshot.json");

tests/composition/analysis-sessions/binarySession.test.ts

@@ -309,7 +309,11 @@ it("reports an external cleanup blocker until its exact owner confirms release",
       (_value, error) => error,
     );
     expect(await session.status()).toMatchObject({ cleanup_pending: true });
-    expect(await session.open(target)).toEqual(err(failure));
+    expect(await session.open(target)).toEqual({
+      ok: false,
+      error: failure,
+      selection: { kind: "none" },
+    });
     await session.close({ cleanupRetry: { operation: "close_binary" } });
     expect(released).toBe(false);
     expect(await session.status()).toMatchObject({ cleanup_pending: true });
@@ -1862,7 +1866,11 @@ describe("no replacement start", () => {
     });
     expect((await session.open(first)).ok).toBe(true);
 
-    expect(await session.open(second)).toEqual(err(cleanupError));
+    expect(await session.open(second)).toEqual({
+      ok: false,
+      error: cleanupError,
+      selection: { kind: "none" },
+    });
     expect(created).toBe(1);
     expect(await session.status()).toMatchObject({ open: false });
   });
@@ -1921,7 +1929,11 @@ it("blocks analysis rebinding until the external writer cleanup settles", async
   );
   expect(result).toEqual(resultOk(err(failure)));
   expect(session.activeTarget()).toBeUndefined();
-  expect(await session.open(target)).toEqual(err(failure));
+  expect(await session.open(target)).toEqual({
+    ok: false,
+    error: failure,
+    selection: { kind: "none" },
+  });
   const anotherMutation = vi.fn(async () => resultOk("unrelated command"));
   expect(
     await session.withAdmittedExternalMutation(

tests/composition/analysis-sessions/binarySessionCleanupRetry.test.ts

@@ -128,7 +128,11 @@ it.each(["close", "switch", "open after close"] as const)(
       transition === "close" ? session.close() : session.open(second);
     const initial =
       transition === "switch" ? await attempt() : await session.close();
-    expect(initial).toEqual(err(cleanupError));
+    expect(initial).toEqual(
+      transition === "switch"
+        ? { ...err(cleanupError), selection: { kind: "none" } }
+        : err(cleanupError),
+    );
     expect(session.activeTarget()).toBeUndefined();
     expect(await session.status()).toMatchObject({
       open: false,
@@ -149,7 +153,11 @@ it.each(["close", "switch", "open after close"] as const)(
     });
     await access(runtime);
 
-    expect(await attempt()).toEqual(err(cleanupError));
+    expect(await attempt()).toEqual(
+      transition === "close"
+        ? err(cleanupError)
+        : { ...err(cleanupError), selection: { kind: "none" } },
+    );
     expect(await session.status()).toMatchObject({
       analysis_run: { run_id: fixture.runIds[0] },
     });

tests/composition/analysis-sessions/providerBinding.test.ts

@@ -175,6 +175,7 @@ describe("target-bound provider routing", () => {
     expect(cancelled).toEqual({
       ok: false,
       error: new AnalysisCancelledError("open_binary", { cause: reason }),
+      selection: { kind: "binary", path: target, sha256: expect.any(String) },
     });
     if (!cancelled.ok) expect(cancelled.error.cause).toBe(reason);
     expect(alpha.clients[0]?.closed).toBe(false);

tests/process-global/mainStartupCancellation.test.ts

@@ -75,9 +75,13 @@ it.each([
       await new Promise<void>((resolve) =>
         signal.addEventListener("abort", () => resolve(), { once: true }),
       );
-      return err(
-        new AnalysisCancelledError("open_binary", { cause: signal.reason }),
-      );
+      return {
+        ok: false,
+        error: new AnalysisCancelledError("open_binary", {
+          cause: signal.reason,
+        }),
+        selection: { kind: "none" },
+      };
     });
     let closeFails = failedClose;
     const close = vi.spyOn(session, "close").mockImplementation(async () =>
@@ -139,7 +143,7 @@ it.each(["optional", "late-handle"] as const)(
   "closes resources settling after startup cancellation at %s",
   async (stage) => {
     const session = sessionOwner();
-    vi.spyOn(session, "open").mockResolvedValue(ok(target));
+    vi.spyOn(session, "open").mockResolvedValue({ ok: true, value: target });
     const closeSession = vi.spyOn(session, "close").mockResolvedValue(ok(null));
     const input = new PassThrough();
     const signals = new EventEmitter();