fix(native): retain signature roots until descriptor cleanup completes

Give each signature snapshot a private child-resource scope. Its composite
root lease closes those children before removing the directory and retains
both stages across failed retries. Bind local cleanup failures to their
exact owner so child release cannot clear a failed root removal.

Replace the mixed-outcome regression that expected premature root removal.
Cover real descriptor denial, primary/cancellation preservation, MCP close
and same-connection recovery. Expose underlying real codesign test errors
before asserting result fields.

Fixes #5052

src/native/SignatureTargetBinding.test.ts

@@ -223,7 +223,7 @@ it("waits for an in-flight signature binding before provider close", async () =>
   await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" });
 });
 
-it("tracks mixed snapshot descriptor and root cleanup through partial release and retry", async () => {
+it("releases retained descriptors before retrying their containing root", async () => {
   const target = await createTarget("observed", "expected");
   const resources = new ArtifactResourceScope();
   const original = OwnedFileHandle.prototype.close;
@@ -265,15 +265,16 @@ it("tracks mixed snapshot descriptor and root cleanup through partial release an
     createRoot,
   );
   expect(retried.ok).toBe(false);
-  if (retried.ok) throw new Error("Expected retained descriptor retry failure");
+  if (retried.ok) throw new Error("Expected retained root retry failure");
   expect(first.error.cleanupIncomplete).toBe(true);
-  expect(first.error.cleanupResources).toEqual([snapshot]);
-  expect(retried.error.cleanupResources).toEqual([snapshot]);
+  expect(first.error.cleanupResources).toEqual([roots[0]!]);
+  expect(retried.error.cleanupResources).toEqual([roots[0]!]);
   expect(roots).toHaveLength(1);
   expect(inspect).not.toHaveBeenCalled();
-  await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" });
+  expect((await stat(roots[0]!)).isDirectory()).toBe(true);
   await resources.close();
   expect(attempts).toBe(3);
+  await expect(stat(roots[0]!)).rejects.toMatchObject({ code: "ENOENT" });
   for (const failure of [first.error, retried.error]) {
     expect(failure.cleanupIncomplete).toBe(false);
     expect(failure.cleanupResources).toEqual([]);
@@ -283,6 +284,57 @@ it("tracks mixed snapshot descriptor and root cleanup through partial release an
   );
 });
 
+it("retains the snapshot root until its failed descriptor owner confirms release", async () => {
+  const target = await createTarget("observed", "expected");
+  const resources = new ArtifactResourceScope();
+  const original = OwnedFileHandle.prototype.close;
+  let blocked: OwnedFileHandle | undefined;
+  let allowClose = false;
+  const close = vi
+    .spyOn(OwnedFileHandle.prototype, "close")
+    .mockImplementation(function (this: OwnedFileHandle) {
+      blocked ??= this;
+      if (this === blocked && !allowClose)
+        return Promise.reject(new Error("snapshot descriptor remains open"));
+      return original.call(this);
+    });
+  const root = await createRoot();
+  const removeRoot = vi.fn(root.close);
+  onTestFinished(async () => {
+    allowClose = true;
+    await resources.close();
+    close.mockRestore();
+  });
+  const result = await withSignatureTarget(
+    target,
+    resources,
+    unusedInspection,
+    undefined,
+    async () => ({ path: root.path, close: removeRoot }),
+  );
+  expect(result.ok).toBe(false);
+  if (result.ok) throw new Error("Expected retained snapshot descriptor");
+  expect(removeRoot).not.toHaveBeenCalled();
+  expect((await stat(root.path)).isDirectory()).toBe(true);
+  const retry = await resources.close().then(
+    () => undefined,
+    (cause: unknown) => cause,
+  );
+  expect(retry).toBeDefined();
+  expect(removeRoot).not.toHaveBeenCalled();
+  expect((await stat(root.path)).isDirectory()).toBe(true);
+  expect(result.error.cleanupIncomplete).toBe(true);
+  allowClose = true;
+  await resources.close();
+  expect(removeRoot).toHaveBeenCalledTimes(1);
+  expect(result.error.cleanupIncomplete).toBe(false);
+  expect(result.error.cleanupResources).toEqual([]);
+  expect(resolvedAnalysisError(result.error)._tag).toBe(
+    "AnalysisArtifactChangedError",
+  );
+  await expect(stat(root.path)).rejects.toMatchObject({ code: "ENOENT" });
+});
+
 const opaqueReasons = [
   null,
   0,
@@ -339,12 +391,15 @@ for (const ownerIndex of [0, 1])
       const reason = { stage: "descriptor-close" };
       const original = OwnedFileHandle.prototype.close;
       let calls = 0;
+      let blocked: OwnedFileHandle | undefined;
+      let allowClose = false;
       const close = vi
         .spyOn(OwnedFileHandle.prototype, "close")
         .mockImplementation(async function (this: OwnedFileHandle) {
           const cancels = calls++ === ownerIndex;
+          if (cancels && cleanupFails) blocked = this;
           try {
-            if (cancels && cleanupFails)
+            if (this === blocked && !allowClose)
               throw new Error("controlled close failure");
             await original.call(this);
           } finally {
@@ -368,6 +423,7 @@ for (const ownerIndex of [0, 1])
         throw new Error("Expected descriptor finalization cancellation");
       expect(inspect).not.toHaveBeenCalled();
       expect(result.error.cleanupIncomplete).toBe(cleanupFails);
+      allowClose = true;
       await resources.close();
       expect(resolvedAnalysisError(result.error)).toMatchObject({
         _tag: "AnalysisCancelledError",

src/native/SignatureTargetBinding.ts

@@ -118,9 +118,20 @@ export const withSignatureTarget = async (
             : snapshotFailure(target, "create", cause),
         );
       }
+      const children = new ArtifactResourceScope();
       const owner: SignatureOwner = {
         kind: "lease",
-        lease: new AnalysisCleanupLease(() => root.close(), [root.path]),
+        lease: new AnalysisCleanupLease(
+          async () => {
+            await children.close();
+            await root.close();
+          },
+          {
+            get cleanupResources() {
+              return [...children.cleanupResources, root.path];
+            },
+          },
+        ),
         resource: root.path,
       };
       let result: Result<InspectSignature, AnalysisError>;
@@ -128,7 +139,7 @@ export const withSignatureTarget = async (
         const binding = await bindSignatureTarget(
           target,
           join(root.path, basename(target.path)),
-          resources,
+          children,
           signal,
         );
         if (!binding.ok) result = binding;
@@ -173,11 +184,19 @@ export const withSignatureTarget = async (
       cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined
         ? cause.cleanup
         : undefined;
-    if (retained !== undefined)
+    if (retained !== undefined) {
+      const failure = signatureCleanupFailure(
+        target,
+        retained.resources,
+        cause,
+        "retry",
+      );
+      bindArtifactFileCleanup(cause, failure);
       return combineCleanupFailure(
         err(signatureReadFailure(target, cause)),
-        signatureCleanupFailure(target, retained.resources, cause, "retry"),
+        failure,
       );
+    }
     return err(signatureReadFailure(target, cause));
   }
 };
@@ -202,8 +221,8 @@ const signatureCleanupFailure = (
   resources: readonly string[],
   cause: unknown,
   phase: string,
-): AnalysisError => {
-  const failure = new ProviderCleanupError(
+): AnalysisError =>
+  new ProviderCleanupError(
     NATIVE_MACOS_PROVIDER_IDENTITY.id,
     resources,
     {
@@ -213,9 +232,6 @@ const signatureCleanupFailure = (
     },
     { operation: "inspect_signature", cause },
   );
-  bindArtifactFileCleanup(cause, failure);
-  return failure;
-};
 
 const snapshotFailure = (
   target: BinaryTarget,

tests/boundary/mcp/nativeSignatureTargetBindingMcp.test.ts

@@ -1,8 +1,9 @@
 import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
-import { writeFile } from "node:fs/promises";
+import { lstat, mkdir, rm, writeFile } from "node:fs/promises";
 import { join } from "node:path";
 import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
-import { expect, it } from "vitest";
+import { expect, it, vi } from "vitest";
+import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js";
 import { NativeMacOSProvider } from "../../../src/native/NativeMacOSProvider.js";
 import {
   machoImage,
@@ -16,6 +17,112 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 import { NativeFixtureRunner } from "../../fixtures/nativeCommands.js";
 import { toolContract } from "../../../src/contracts/toolContracts.js";
 
+it("keeps a failed snapshot descriptor and root together through MCP close and recovery", async () => {
+  const root = await createTestTempDirectory("rea-signature-order-mcp-");
+  const path = join(root, "program");
+  await writeFile(path, machoImage({}));
+  const runner = new NativeFixtureRunner();
+  const run = vi.spyOn(runner, "run");
+  let created = 0;
+  let removed = 0;
+  const provider = new NativeMacOSProvider(
+    {},
+    runner,
+    "darwin",
+    undefined,
+    async () => {
+      const path = join(root, `snapshot-${String(++created)}`);
+      await mkdir(path, { mode: 0o700 });
+      return {
+        path,
+        close: async () => {
+          await rm(path, { recursive: true });
+          removed++;
+        },
+      };
+    },
+  );
+  const server = createServer({
+    kind: "session",
+    session: createTestBinarySession(provider),
+  });
+  const client = new Client({ name: "signature-order-test", version: "1" });
+  const [clientTransport, serverTransport] =
+    InMemoryTransport.createLinkedPair();
+  const original = OwnedFileHandle.prototype.close;
+  let blocked: OwnedFileHandle | undefined;
+  let allowClose = false;
+  let close: ReturnType<typeof vi.spyOn> | undefined;
+  try {
+    await server.connect(serverTransport);
+    await client.connect(clientTransport);
+    expect(
+      (await client.callTool({ name: "open_binary", arguments: { path } }))
+        .isError,
+    ).not.toBe(true);
+    close = vi
+      .spyOn(OwnedFileHandle.prototype, "close")
+      .mockImplementation(function (this: OwnedFileHandle) {
+        blocked ??= this;
+        if (this === blocked && !allowClose)
+          return Promise.reject(new Error("snapshot descriptor close denied"));
+        return original.call(this);
+      });
+    for (const name of [
+      "inspect_signature",
+      "inspect_signature",
+      "close_binary",
+    ]) {
+      const reply = await client.callTool({ name, arguments: {} });
+      const error = parseMcpToolError(reply).error;
+      expect(error.code).toBe("cleanup_incomplete");
+      expect(error.details).toMatchObject({
+        cleanup: "incomplete",
+        resources: expect.arrayContaining([
+          join(root, "snapshot-1"),
+          join(root, "snapshot-1", "program"),
+        ]),
+      });
+      expect(created).toBe(1);
+      expect(removed).toBe(0);
+      expect((await lstat(join(root, "snapshot-1"))).isDirectory()).toBe(true);
+      expect(run).not.toHaveBeenCalled();
+      await client.ping();
+    }
+    allowClose = true;
+    expect(
+      (await client.callTool({ name: "close_binary", arguments: {} })).isError,
+    ).not.toBe(true);
+    expect(removed).toBe(1);
+    await expect(lstat(join(root, "snapshot-1"))).rejects.toMatchObject({
+      code: "ENOENT",
+    });
+    expect(
+      (await client.callTool({ name: "open_binary", arguments: { path } }))
+        .isError,
+    ).not.toBe(true);
+    const recovered = await client.callTool({
+      name: "inspect_signature",
+      arguments: {},
+    });
+    expect(recovered.isError, JSON.stringify(recovered.content)).not.toBe(true);
+    expect(recovered.structuredContent).toMatchObject({
+      normalized_result: { signed: true, identifier: "com.example.fixture" },
+    });
+    expect(created).toBe(2);
+    expect(removed).toBe(2);
+  } finally {
+    allowClose = true;
+    try {
+      await client.close();
+      await server.close();
+    } finally {
+      close?.mockRestore();
+      run.mockRestore();
+    }
+  }
+});
+
 it.each(["mach-o", "mach-o-universal"] as const)(
   "rejects changed %s signature targets through MCP and accepts reopening the new version",
   async (format) => {

tests/boundary/providers/native/nativeSignatureTargetBinding.test.ts

@@ -68,7 +68,8 @@ describe("native signature target identity", () => {
     const result = await new NativeMacOSProvider({}, new Captures(), "darwin")
       .createClient(target)
       .execute("inspect_signature", {});
-    expect(result.ok && result.value.result).toMatchObject({
+    if (!result.ok) throw result.error;
+    expect(result.value.result).toMatchObject({
       signed: true,
       identifier: "com.example.fixture",
     });
@@ -477,11 +478,11 @@ it.skipIf(process.platform !== "darwin")(
     const result = await new NativeMacOSProvider({}, runner, "darwin")
       .createClient(target)
       .execute("inspect_signature", {});
-    expect(result.ok && result.value.result).toMatchObject({
+    if (!result.ok) throw result.error;
+    expect(result.value.result).toMatchObject({
       signed: true,
       identifier: "rea.original",
     });
-    if (!result.ok) throw result.error;
     expect(
       inspectSignatureSchema
         .parse(result.value.result)