fix(native): retain command cleanup across source finalization (#10009)

src/native/NativeMacOSProvider.ts

@@ -1055,6 +1055,19 @@ class NativeMacOSClient implements AnalysisClient {
     // Verify failed captures too: a disappearing target is not a codesign
     // execution failure, and later captures must never use another version.
     const after = await verifySignatureTarget(this.target, binding, signal);
+    if (!capture.ok && !after.ok) {
+      const primary =
+        after.error instanceof AnalysisCancelledError
+          ? analysisErrorWithCancellation(capture.error, after.error)
+          : after.error;
+      return err(
+        nativeFailureWithCommandCleanup(
+          primary,
+          capture.error,
+          "inspect_signature",
+        ),
+      );
+    }
     return after.ok ? capture : after;
   }
 
@@ -1817,6 +1830,7 @@ const nativeMachoCompletion = (
       : new AnalysisCancelledError(operation, {
           cause: completion.cancellation.reason,
           ...(completedFailure === undefined ||
+          completedFailure.cleanupIncomplete ||
           (cause instanceof ArtifactReaderFailure &&
             cause.cleanup !== undefined)
             ? {}
@@ -1831,6 +1845,40 @@ const nativeMachoCompletion = (
   return { observation, completedFailure, completionCancellation };
 };
 
+/** Every native reclassification retains the command and independent file owners. */
+const nativeFailureWithCommandCleanup = (
+  primary: AnalysisError,
+  producer: AnalysisError,
+  operation: string,
+  descriptorCleanup?: AnalysisError,
+): AnalysisError => {
+  if (!producer.cleanupIncomplete) return primary;
+  const cleanup = descriptorCleanup ?? producer;
+  return cleanup === primary
+    ? primary
+    : analysisErrorWithCleanupFailure(
+        primary,
+        cleanup,
+        operation,
+      ).bindCleanupDependencies([
+        primary,
+        producer,
+        ...(descriptorCleanup === undefined ? [] : [descriptorCleanup]),
+      ]);
+};
+
+/** The exact callback error remains an owner even if verification reclassifies it. */
+const nativeMachoProducerFailure = (
+  cause: unknown,
+): AnalysisError | undefined => {
+  if (cause instanceof AnalysisError) return cause;
+  if (!(cause instanceof NativeMachoSourceVerificationFailure))
+    return undefined;
+  return cause.outcome.kind === "failed"
+    ? nativeMachoProducerFailure(cause.outcome.cause)
+    : nativeMachoCompletedFailure(cause.outcome.value);
+};
+
 const nativeMachoSourceFailure = (
   cause: unknown,
   target: BinaryTarget,
@@ -1839,6 +1887,31 @@ const nativeMachoSourceFailure = (
 ): AnalysisError => {
   const { observation, completedFailure, completionCancellation } =
     nativeMachoCompletion(cause, target, parameters, operation);
+  const source = artifactFilePrimaryFacts(cause);
+  const producerFailure =
+    completedFailure ?? nativeMachoProducerFailure(source.cause);
+  // Classification and ownership are independent: neither a changed source nor
+  // a released descriptor settles the command's retained cleanup capability.
+  const retainOwnedCleanup = (
+    selected: AnalysisError,
+    descriptorCleanup?: AnalysisError,
+  ): AnalysisError => {
+    if (!producerFailure?.cleanupIncomplete) return selected;
+    const primary =
+      selected === descriptorCleanup
+        ? (descriptorCleanup.cleanupPrimary ?? producerFailure)
+        : selected === completionCancellation
+          ? producerFailure
+          : selected;
+    if (completionCancellation !== undefined)
+      analysisErrorWithCancellation(primary, completionCancellation);
+    return nativeFailureWithCommandCleanup(
+      primary,
+      producerFailure,
+      operation,
+      descriptorCleanup,
+    );
+  };
   if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) {
     const physical =
       cause instanceof ArtifactFileCleanupFailure
@@ -1887,17 +1960,15 @@ const nativeMachoSourceFailure = (
     if (observation !== undefined)
       completionCancellation?.retainPartialObservation(observation);
     bindArtifactFileCleanup(cause, failure);
-    return failure;
+    return retainOwnedCleanup(failure, failure);
   }
   if (completionCancellation !== undefined) {
     if (observation !== undefined)
       completionCancellation.retainPartialObservation(observation);
-    return completionCancellation;
+    return retainOwnedCleanup(completionCancellation);
   }
-  return nativeMachoSourcePrimary(
-    artifactFilePrimaryFacts(cause),
-    target.path,
-    operation,
+  return retainOwnedCleanup(
+    nativeMachoSourcePrimary(source, target.path, operation),
   );
 };
 

src/native/NativeMachoSourceProvider.test.ts

@@ -1,9 +1,14 @@
+import { EventEmitter } from "node:events";
 import { open, writeFile, type FileHandle } from "node:fs/promises";
 import { join } from "node:path";
 import { expect, it, onTestFinished, vi } from "vitest";
 import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
 import { ProviderCleanupError } from "../domain/providerCleanupError.js";
 import { NativeMacOSProvider } from "./NativeMacOSProvider.js";
+import { NativeCommandFailure, XcrunCommandRunner } from "./CommandRunner.js";
+import { ProviderProcessSupervisor } from "../process/ProviderProcess.js";
+import { err } from "../domain/result.js";
+import { primaryAnalysisError } from "../domain/analysisErrorBase.js";
 import { MachoCommandNamesLimitError } from "./MachoCommandNames.js";
 import {
   NativeFixtureRunner,
@@ -490,3 +495,165 @@ it.each([
     }
   },
 );
+
+it.each(
+  (["inspect_macho", "list_architectures"] as const).flatMap((operation) => [
+    ...[false, true].flatMap((cancelDuringClose) =>
+      [false, true].map((failDescriptorClose) => ({
+        operation,
+        cancelDuringClose,
+        failDescriptorClose,
+        changeTarget: false,
+      })),
+    ),
+    {
+      operation,
+      cancelDuringClose: false,
+      failDescriptorClose: false,
+      changeTarget: true,
+    },
+  ]),
+)(
+  "keeps $operation command cleanup live through finalization (cancel: $cancelDuringClose, failed close: $failDescriptorClose, changed target: $changeTarget)",
+  async ({
+    operation,
+    cancelDuringClose,
+    failDescriptorClose,
+    changeTarget,
+  }) => {
+    const root = await createTestTempDirectory("rea-macho-command-owner-");
+    const path = join(root, "fixture.macho");
+    await writeFile(path, inspectionImage());
+    const target = await nativeMachoTargetForFile(path);
+    // The exited event handle and cleanup callback are inert; no process starts.
+    const process = Object.assign(new EventEmitter(), {
+      exitCode: 1,
+      signalCode: null,
+      stdout: null,
+      stderr: null,
+      kill: () => {
+        throw new Error("Exited fixture must not be signalled");
+      },
+    });
+    let mayClean = false;
+    let cleanupAttempts = 0;
+    const owner = new ProviderProcessSupervisor({
+      process,
+      ownsProcessLifetime: true,
+      cleanup: async () => {
+        cleanupAttempts++;
+        return mayClean
+          ? { cleaned: true, signaled: false }
+          : { cleaned: false, reason: "worker remains owned" };
+      },
+    });
+    process.emit("close", 1, null);
+    const physical = new Error("physical command read failure");
+    const failure = new NativeCommandFailure(
+      "lipo",
+      "io",
+      1,
+      { cause: physical, cleanupOwner: owner },
+      {
+        stdout: "partial native output",
+        stderr: "read failed",
+        stdoutBytes: 21,
+        stderrBytes: 11,
+        exitCode: 1,
+        signal: null,
+        truncated: false,
+      },
+      "worker remains owned",
+      ["native-command-process"],
+    );
+    const runner = new XcrunCommandRunner({}, async () => {
+      if (changeTarget) await writeFile(path, "changed Mach-O source bytes");
+      return err(failure);
+    });
+    const client = new NativeMacOSProvider({}, runner, "darwin").createClient(
+      target,
+    );
+    const controller = new AbortController();
+    const reason = { request: "cancel during descriptor close" };
+    let descriptorCloseFails = failDescriptorClose;
+    const probe = await open(path, "r");
+    const prototype = Object.getPrototypeOf(probe) as FileHandle;
+    await probe.close();
+    const original = prototype.stat;
+    const stat = vi
+      .spyOn(prototype, "stat")
+      .mockImplementationOnce(async function (
+        this: FileHandle,
+        ...args: Parameters<FileHandle["stat"]>
+      ) {
+        const close = this.close.bind(this);
+        this.close = async () => {
+          if (cancelDuringClose) controller.abort(reason);
+          if (descriptorCloseFails) throw new Error("descriptor remains owned");
+          await close();
+        };
+        return Reflect.apply(original, this, args);
+      });
+    onTestFinished(async () => {
+      stat.mockRestore();
+      descriptorCloseFails = false;
+      mayClean = true;
+      expect((await client.close()).ok).toBe(true);
+    });
+    const result = await client.execute(
+      operation,
+      {},
+      { signal: controller.signal },
+    );
+    if (result.ok) throw new Error("Expected command failure");
+    expect(result.error._tag).toBe("ProviderAdapterError");
+    const primary = primaryAnalysisError(result.error);
+    if (changeTarget) {
+      expect(primary._tag).toBe("AnalysisArtifactChangedError");
+      expect(primary.diagnostics).toMatchObject({
+        previous_error: {
+          details: {
+            captured_output: {
+              stdout: "partial native output",
+              stderr: "read failed",
+            },
+          },
+        },
+      });
+    } else {
+      expect(primary.cause).toBeInstanceOf(NativeCommandFailure);
+      if (!(primary.cause instanceof NativeCommandFailure))
+        throw new Error("Expected the runner's physical command failure");
+      expect(primary.cause.cause).toBe(failure);
+      expect(primary.capturedOutput).toMatchObject({
+        stdout: "partial native output",
+        stderr: "read failed",
+      });
+    }
+    expect(result.error.cleanupIncomplete).toBe(true);
+    expect(result.error.cleanupResources.toSorted()).toEqual(
+      [...(failDescriptorClose ? [path] : []), "native-command-process"].sort(),
+    );
+    expect(primary.secondaryFailures).toEqual(
+      cancelDuringClose
+        ? [
+            expect.objectContaining({
+              _tag: "AnalysisCancelledError",
+              cause: reason,
+            }),
+          ]
+        : [],
+    );
+    descriptorCloseFails = false;
+    const blocked = await client.close();
+    expect(blocked.ok).toBe(false);
+    expect(result.error.cleanupIncomplete).toBe(true);
+    expect(result.error.cleanupResources).toEqual(["native-command-process"]);
+    expect(cleanupAttempts).toBe(1);
+    mayClean = true;
+    expect((await client.close()).ok).toBe(true);
+    expect(cleanupAttempts).toBe(2);
+    expect(result.error.cleanupIncomplete).toBe(false);
+    expect(result.error.cleanupResources).toEqual([]);
+  },
+);

src/native/SignatureTargetBinding.test.ts

@@ -1,3 +1,4 @@
+import { EventEmitter } from "node:events";
 import { createHash } from "node:crypto";
 import {
   chmod,
@@ -19,7 +20,12 @@ import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
 import type { BinaryTarget } from "../domain/binaryTargetTypes.js";
 import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
 import { ok, err } from "../domain/result.js";
-import type { NativeCommandRunner } from "./CommandRunner.js";
+import {
+  NativeCommandFailure,
+  XcrunCommandRunner,
+  type NativeCommandRunner,
+} from "./CommandRunner.js";
+import { ProviderProcessSupervisor } from "../process/ProviderProcess.js";
 import type { NativeCallTracer } from "./LldbCallTracer.js";
 import { NativeMacOSProvider } from "./NativeMacOSProvider.js";
 import {
@@ -27,7 +33,10 @@ import {
   withSignatureTarget,
 } from "./SignatureTargetBinding.js";
 import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
-import { resolvedAnalysisError } from "../domain/analysisErrorBase.js";
+import {
+  primaryAnalysisError,
+  resolvedAnalysisError,
+} from "../domain/analysisErrorBase.js";
 import { ProviderCleanupError } from "../domain/providerCleanupError.js";
 import { AnalysisOutputError } from "../domain/analysisErrorCore.js";
 import { signatureInspectionFixture } from "../../tests/fixtures/nativeCommands.js";
@@ -645,3 +654,180 @@ it("does not adopt an inspection's arbitrary thrown cleanup marker", async () =>
   expect(result.error.cleanupResources).toEqual([]);
   await resources.close();
 });
+
+it.each(["io", "cancelled"] as const)(
+  "preserves a codesign %s failure when later target verification observes cancellation",
+  async (kind) => {
+    const target = await createTarget("registered source bytes");
+    const controller = new AbortController();
+    const reason = { caller: "cancel after codesign result", kind };
+    const output = {
+      stdout: "physical output",
+      stderr: "physical diagnostic",
+      stdoutBytes: 15,
+      stderrBytes: 19,
+      exitCode: null,
+      signal: null,
+      truncated: false,
+    };
+    const failure = new NativeCommandFailure(
+      "codesign",
+      kind,
+      null,
+      { cancellation: { reason } },
+      output,
+      "command cleanup remains unknown",
+      ["command-owner"],
+    );
+    const runner: NativeCommandRunner = {
+      run: async () => {
+        controller.abort(reason);
+        return err(failure);
+      },
+    };
+    const client = new NativeMacOSProvider(
+      {},
+      runner,
+      "darwin",
+      () => unusedTracer,
+      createRoot,
+    ).createClient(target);
+    try {
+      const result = await client.execute(
+        "inspect_signature",
+        {},
+        { signal: controller.signal },
+      );
+      if (result.ok) throw new Error("Expected codesign failure");
+      expect(result.error._tag).toBe(
+        kind === "cancelled"
+          ? "AnalysisCancelledError"
+          : "ProviderAdapterError",
+      );
+      expect(result.error.capturedOutput).toMatchObject({
+        stdout: output.stdout,
+        stderr: output.stderr,
+      });
+      expect(result.error.cleanupIncomplete).toBe(true);
+      expect(result.error.cleanupResources).toEqual(["command-owner"]);
+      if (kind === "cancelled") expect(result.error.cause).toBe(reason);
+      else {
+        expect(result.error.cause).toBe(failure);
+        expect(result.error.secondaryFailures).toContainEqual(
+          expect.objectContaining({
+            _tag: "AnalysisCancelledError",
+            cause: reason,
+          }),
+        );
+      }
+    } finally {
+      await client.close();
+    }
+  },
+);
+
+it.each(["unchanged", "cancelled", "changed"] as const)(
+  "keeps a codesign cleanup marker live after %s target verification",
+  async (outcome) => {
+    const target = await createTarget("registered source bytes");
+    const controller = new AbortController();
+    const reason = { request: "cancel after native command failure" };
+    const process = Object.assign(new EventEmitter(), {
+      exitCode: 1,
+      signalCode: null,
+      stdout: null,
+      stderr: null,
+      kill: () => {
+        throw new Error("Exited fixture must not be signalled");
+      },
+    });
+    let mayClean = false;
+    let cleanupAttempts = 0;
+    const owner = new ProviderProcessSupervisor({
+      process,
+      ownsProcessLifetime: true,
+      cleanup: async () => {
+        cleanupAttempts++;
+        return mayClean
+          ? { cleaned: true, signaled: false }
+          : { cleaned: false, reason: "codesign worker remains owned" };
+      },
+    });
+    process.emit("close", 1, null);
+    const failure = new NativeCommandFailure(
+      "codesign",
+      "io",
+      1,
+      { cleanupOwner: owner },
+      {
+        stdout: "physical output",
+        stderr: "physical diagnostic",
+        stdoutBytes: 15,
+        stderrBytes: 19,
+        exitCode: 1,
+        signal: null,
+        truncated: false,
+      },
+      "codesign worker remains owned",
+      ["native-command-process"],
+    );
+    const runner = new XcrunCommandRunner({}, async () => {
+      if (outcome === "cancelled") controller.abort(reason);
+      if (outcome === "changed")
+        await writeFile(target.path, "changed source bytes");
+      return err(failure);
+    });
+    const client = new NativeMacOSProvider(
+      {},
+      runner,
+      "darwin",
+      () => unusedTracer,
+      createRoot,
+    ).createClient(target);
+    onTestFinished(async () => {
+      mayClean = true;
+      expect((await client.close()).ok).toBe(true);
+    });
+    const result = await client.execute(
+      "inspect_signature",
+      {},
+      { signal: controller.signal },
+    );
+    if (result.ok) throw new Error("Expected codesign failure");
+    const primary = primaryAnalysisError(result.error);
+    expect(primary._tag).toBe(
+      outcome === "changed"
+        ? "AnalysisArtifactChangedError"
+        : "ProviderAdapterError",
+    );
+    if (outcome !== "changed") {
+      if (!(primary.cause instanceof NativeCommandFailure))
+        throw new Error("Expected physical codesign failure");
+      expect(primary.cause.cause).toBe(failure);
+      expect(primary.capturedOutput).toMatchObject({
+        stdout: "physical output",
+        stderr: "physical diagnostic",
+      });
+    }
+    expect(result.error.cleanupIncomplete).toBe(true);
+    expect(result.error.cleanupResources).toEqual(["native-command-process"]);
+    expect(primary.secondaryFailures).toEqual(
+      outcome === "cancelled"
+        ? [
+            expect.objectContaining({
+              _tag: "AnalysisCancelledError",
+              cause: reason,
+            }),
+          ]
+        : [],
+    );
+    expect((await client.close()).ok).toBe(false);
+    expect(result.error.cleanupIncomplete).toBe(true);
+    expect(cleanupAttempts).toBe(1);
+    mayClean = true;
+    expect((await client.close()).ok).toBe(true);
+    expect(cleanupAttempts).toBe(2);
+    expect(result.error.cleanupIncomplete).toBe(false);
+    expect(result.error.cleanupResources).toEqual([]);
+  },
+);