fix(artifacts): preserve cancellation during inventory cleanup

src/artifacts/inventory/ArtifactInventoryFinalization.test.ts

@@ -0,0 +1,187 @@
+import { createHash } from "node:crypto";
+import { writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { TextReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js";
+import { afterEach, expect, it, vi } from "vitest";
+import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js";
+import { ArtifactProvider } from "../ArtifactProvider.js";
+import { ArtifactReaderFailure } from "../ArtifactReader.js";
+import { ArtifactResourceScope } from "../ArtifactResourceScope.js";
+import { ZipArtifactReader } from "../ZipArtifactReader.js";
+import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js";
+import { projectAnalysisError } from "../../domain/analysisErrorProjection.js";
+import {
+  scanCanonicalArtifactInventoryForExtractionInScope,
+  scanCanonicalArtifactInventoryInScope,
+} from "./scanCanonical.js";
+
+afterEach(() => vi.restoreAllMocks());
+
+const zipFixture = async () => {
+  const directory = await createTestTempDirectory(
+    "rea-inventory-finalization-",
+  );
+  const path = join(directory, "input.zip");
+  const writer = new ZipWriter(new Uint8ArrayWriter());
+  await writer.add("member.txt", new TextReader("retained member evidence"));
+  const bytes = await writer.close();
+  await writeFile(path, bytes);
+  return { path, sha256: createHash("sha256").update(bytes).digest("hex") };
+};
+
+it.each([
+  { transfer: false, readerFails: false, rootFails: false, cancelAt: "reader" },
+  { transfer: false, readerFails: false, rootFails: false, cancelAt: "root" },
+  { transfer: false, readerFails: true, rootFails: true, cancelAt: "reader" },
+  { transfer: false, readerFails: true, rootFails: true, cancelAt: "root" },
+  { transfer: true, readerFails: false, rootFails: false, cancelAt: "reader" },
+  { transfer: true, readerFails: false, rootFails: true, cancelAt: "reader" },
+  { transfer: true, readerFails: true, rootFails: false, cancelAt: "reader" },
+])(
+  "retains completed inventory and exact cleanup owners: %j",
+  async (scenario) => {
+    const { path, sha256 } = await zipFixture();
+    const scope = new ArtifactResourceScope();
+    const controller = new AbortController();
+    const reason = { request: "cancel during inventory finalization" };
+    let retry = false;
+    const readerClose = ZipArtifactReader.prototype.close;
+    const reader = vi
+      .spyOn(ZipArtifactReader.prototype, "close")
+      .mockImplementation(async function (this: ZipArtifactReader) {
+        if (scenario.cancelAt === "reader") controller.abort(reason);
+        if (scenario.readerFails && !retry)
+          throw new Error("reader close denied");
+        await readerClose.call(this);
+      });
+    const rootClose = OwnedFileHandle.prototype.close;
+    const root = vi
+      .spyOn(OwnedFileHandle.prototype, "close")
+      .mockImplementation(async function (this: OwnedFileHandle) {
+        if (scenario.cancelAt === "root") controller.abort(reason);
+        if (scenario.rootFails && !retry) throw new Error("root close denied");
+        await rootClose.call(this);
+      });
+    const options = { resourceScope: scope, signal: controller.signal };
+    const failure: unknown = await scope
+      .run(async () => {
+        if (scenario.transfer)
+          await scanCanonicalArtifactInventoryForExtractionInScope(
+            path,
+            options,
+          );
+        else await scanCanonicalArtifactInventoryInScope(path, options);
+      })
+      .catch((cause: unknown) => cause);
+    try {
+      expect(failure).toBeInstanceOf(ArtifactReaderFailure);
+      if (!(failure instanceof ArtifactReaderFailure))
+        throw new Error("Expected inventory failure");
+      const earlierCleanupFailure =
+        scenario.readerFails && scenario.cancelAt === "root";
+      expect(failure.reason).toBe(earlierCleanupFailure ? "io" : "cancelled");
+      expect(failure.cancellation?.reason).toBe(
+        earlierCleanupFailure ? undefined : reason,
+      );
+      expect(failure.partialObservation).toMatchObject({
+        kind: "artifact-inventory",
+        inventory: {
+          manifest: { root_sha256: sha256, occurrence_count: 2 },
+          coverage: { status: "complete" },
+        },
+      });
+      expect(reader).toHaveBeenCalledTimes(1);
+      expect(root).toHaveBeenCalledTimes(1);
+      expect(failure.cleanupLeases).toHaveLength(
+        Number(scenario.readerFails) + Number(scenario.rootFails),
+      );
+      expect(scope.hasPending).toBe(scenario.readerFails || scenario.rootFails);
+      retry = true;
+      await scope.retryCleanup();
+      expect(scope.hasPending).toBe(false);
+      expect(failure.cleanupLeases?.every((lease) => lease.released)).toBe(
+        true,
+      );
+      expect(reader).toHaveBeenCalledTimes(scenario.readerFails ? 2 : 1);
+      expect(root).toHaveBeenCalledTimes(scenario.rootFails ? 2 : 1);
+    } finally {
+      retry = true;
+      await scope.close();
+    }
+  },
+);
+
+it("keeps an earlier inventory failure primary when cleanup is cancelled", async () => {
+  const { path } = await zipFixture();
+  const scope = new ArtifactResourceScope();
+  const controller = new AbortController();
+  const primary = new ArtifactReaderFailure(
+    "format",
+    "member enumeration failed",
+  );
+  const enumeration = vi
+    .spyOn(ZipArtifactReader.prototype, "entries")
+    .mockImplementation(async function* () {
+      throw primary;
+    });
+  const close = ZipArtifactReader.prototype.close;
+  vi.spyOn(ZipArtifactReader.prototype, "close").mockImplementation(
+    async function (this: ZipArtifactReader) {
+      controller.abort({ request: "later cleanup cancellation" });
+      await close.call(this);
+    },
+  );
+  try {
+    await expect(
+      scope.run(() =>
+        scanCanonicalArtifactInventoryInScope(path, {
+          resourceScope: scope,
+          signal: controller.signal,
+        }),
+      ),
+    ).rejects.toBe(primary);
+    expect(enumeration).toHaveBeenCalledTimes(1);
+    expect(scope.hasPending).toBe(false);
+  } finally {
+    await scope.close();
+  }
+});
+
+it.each(["inventory_artifact", "inspect_artifact"] as const)(
+  "reports finalization cancellation and retained inventory through provider %s",
+  async (operation) => {
+    const { path, sha256 } = await zipFixture();
+    const controller = new AbortController();
+    const reason = { request: "provider inventory cancellation" };
+    const close = ZipArtifactReader.prototype.close;
+    vi.spyOn(ZipArtifactReader.prototype, "close").mockImplementation(
+      async function (this: ZipArtifactReader) {
+        controller.abort(reason);
+        await close.call(this);
+      },
+    );
+    const client = new ArtifactProvider({}).createClient({
+      kind: "archive",
+      format: "zip",
+      path,
+      sha256,
+    });
+    try {
+      const result = await client.execute(
+        operation,
+        {},
+        { signal: controller.signal },
+      );
+      expect(result.ok).toBe(false);
+      if (result.ok) throw new Error("Expected provider cancellation");
+      expect(projectAnalysisError(result.error).code).toBe("cancelled");
+      expect(result.error.cause).toBe(reason);
+      expect(result.error.partialObservation).toMatchObject({
+        kind: "artifact-inventory",
+        inventory: { manifest: { root_sha256: sha256, occurrence_count: 2 } },
+      });
+    } finally {
+      expect((await client.close()).ok).toBe(true);
+    }
+  },
+);

src/artifacts/inventory/scanCanonical.ts

@@ -259,8 +259,11 @@ const scanCanonical = async (
         : "release",
   });
   if (cleanup.failure !== undefined) throw cleanup.failure;
-  if (outcome.kind === "failed") throw outcome.cause;
-  return { snapshot: outcome.snapshot, transferredSource: cleanup.transferred };
+  if (cleanup.outcome.kind === "failed") throw cleanup.outcome.cause;
+  return {
+    snapshot: cleanup.outcome.snapshot,
+    transferredSource: cleanup.transferred,
+  };
 };
 
 type InventoryScanOutcome =
@@ -289,8 +292,35 @@ const cleanupInventoryOwners = async ({
 }): Promise<{
   readonly failure: ArtifactReaderFailure | undefined;
   readonly transferred: TransferredArtifactInventorySource | undefined;
+  readonly outcome: InventoryScanOutcome;
 }> => {
+  let finalOutcome = outcome;
   let cleanupFailure: ArtifactReaderFailure | undefined;
+  // Finalization still belongs to this operation. Preserve completed facts when
+  // cancellation arrives while an owner is being released, but keep an earlier
+  // operation or cleanup failure primary.
+  const observeCancellation = (): void => {
+    if (
+      finalOutcome.kind === "completed" &&
+      cleanupFailure === undefined &&
+      options.signal?.aborted === true
+    )
+      finalOutcome = {
+        kind: "failed",
+        cause: new ArtifactReaderFailure(
+          "cancelled",
+          "Artifact inventory cancelled during finalization",
+          {
+            cause: options.signal.reason,
+            partialObservation: {
+              kind: "artifact-inventory",
+              inventory: finalOutcome.snapshot,
+            },
+          },
+        ),
+      };
+  };
+  observeCancellation();
   const readerOwners: ArtifactResourceOwner[] = [...ownedReaders]
     .reverse()
     .map((reader) => ({
@@ -308,10 +338,11 @@ const cleanupInventoryOwners = async ({
         };
   for (const owner of readerOwners) {
     const attempt = await options.resourceScope.release(owner);
+    observeCancellation();
     if (attempt.kind === "released") continue;
     cleanupFailure = options.resourceScope.cleanupFailure(
       cleanupFailure === undefined
-        ? outcome
+        ? finalOutcome
         : { kind: "failed", cause: cleanupFailure },
       [attempt],
       outcome.kind === "completed"
@@ -321,6 +352,7 @@ const cleanupInventoryOwners = async ({
   }
   if (
     rootDisposition === "transfer" &&
+    finalOutcome.kind === "completed" &&
     cleanupFailure === undefined &&
     rootSource !== undefined &&
     rootOwner !== undefined &&
@@ -329,13 +361,15 @@ const cleanupInventoryOwners = async ({
     return {
       failure: undefined,
       transferred: { owner: rootOwner, admission: readerAdmission },
+      outcome: finalOutcome,
     };
   if (rootOwner !== undefined) {
     const attempt = await options.resourceScope.release(rootOwner);
+    observeCancellation();
     if (attempt.kind === "failed")
       cleanupFailure = options.resourceScope.cleanupFailure(
         cleanupFailure === undefined
-          ? outcome
+          ? finalOutcome
           : { kind: "failed", cause: cleanupFailure },
         [attempt],
         outcome.kind === "completed"
@@ -343,7 +377,11 @@ const cleanupInventoryOwners = async ({
           : undefined,
       );
   }
-  return { failure: cleanupFailure, transferred: undefined };
+  return {
+    failure: cleanupFailure,
+    transferred: undefined,
+    outcome: finalOutcome,
+  };
 };
 
 interface SnapshotBuildInput {