fix(browser): retain composed artifact cleanup owners across retries

src/browser/WebArtifactCleanup.test.ts

@@ -1,7 +1,8 @@
 import { expect, it, vi } from "vitest";
 import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
 import { LocalWebScriptArtifacts } from "./assets/LocalWebScriptArtifacts.js";
-import { closeWebArtifactResources } from "./WebArtifactCleanup.js";
+import { ProviderCleanupError } from "../domain/providerCleanupError.js";
+import { WebArtifactCleanup } from "./WebArtifactCleanup.js";
 
 it("closes a shared script/resource scope once", async () => {
   const resources = new ArtifactResourceScope();
@@ -11,7 +12,11 @@ it("closes a shared script/resource scope once", async () => {
     resources,
   );
 
-  await closeWebArtifactResources("web-module-artifacts", resources, scripts);
+  await new WebArtifactCleanup(
+    "web-module-artifacts",
+    resources,
+    scripts,
+  ).close();
 
   expect(close).toHaveBeenCalledTimes(1);
 });
@@ -43,9 +48,8 @@ it.each([
       if (scriptCloseCalls === 1) throw new Error("script close refused once");
     });
 
-    await expect(
-      closeWebArtifactResources(owner, resources, scripts),
-    ).rejects.toMatchObject({
+    const cleanup = new WebArtifactCleanup(owner, resources, scripts);
+    await expect(cleanup.close()).rejects.toMatchObject({
       operation,
       cleanupResources: [resource, "web scripts"],
       diagnostics: {
@@ -64,8 +68,88 @@ it.each([
     expect(resourceCloseCalls).toBe(1);
     expect(scriptCloseCalls).toBe(1);
 
-    await closeWebArtifactResources(owner, resources, scripts);
+    await cleanup.close();
     expect(resourceCloseCalls).toBe(2);
     expect(scriptCloseCalls).toBe(2);
   },
 );
+
+it.each(["resources", "scripts"] as const)(
+  "retries only unresolved %s and resolves prior aggregate errors",
+  async (failed) => {
+    const resources = new ArtifactResourceScope();
+    const scripts = new LocalWebScriptArtifacts(
+      "trace_web_module_imports",
+      new ArtifactResourceScope(),
+    );
+    const resourceClose = vi.spyOn(resources, "close").mockResolvedValue();
+    const scriptClose = vi.spyOn(scripts, "close").mockResolvedValue();
+    const denied = failed === "resources" ? resourceClose : scriptClose;
+    denied.mockRejectedValueOnce(new Error("controlled close refusal"));
+    const cleanup = new WebArtifactCleanup(
+      "web-module-artifacts",
+      resources,
+      scripts,
+    );
+    const first = await cleanup.close().catch((cause: unknown) => cause);
+    expect(first).toMatchObject({
+      cleanupIncomplete: true,
+      cleanupResources: [
+        failed === "resources" ? "web module artifacts" : "web scripts",
+      ],
+    });
+    await cleanup.close();
+    expect(first).toMatchObject({
+      cleanupIncomplete: false,
+      cleanupResources: [],
+    });
+    expect(resourceClose).toHaveBeenCalledTimes(failed === "resources" ? 2 : 1);
+    expect(scriptClose).toHaveBeenCalledTimes(failed === "scripts" ? 2 : 1);
+    await cleanup.close();
+    expect(denied).toHaveBeenCalledTimes(2);
+  },
+);
+
+it("retains exact child labels until every failed owner releases", async () => {
+  const resources = new ArtifactResourceScope();
+  const scripts = new LocalWebScriptArtifacts(
+    "trace_web_module_imports",
+    new ArtifactResourceScope(),
+  );
+  const resourceClose = vi.spyOn(resources, "close").mockResolvedValue();
+  const scriptClose = vi.spyOn(scripts, "close").mockResolvedValue();
+  resourceClose.mockRejectedValueOnce(
+    new ProviderCleanupError("fixture", ["selected-map-handle"], {}),
+  );
+  scriptClose.mockRejectedValueOnce(
+    new ProviderCleanupError("fixture", ["captured-script-handle"], {}),
+  );
+  const cleanup = new WebArtifactCleanup(
+    "web-module-artifacts",
+    resources,
+    scripts,
+  );
+  const first = await cleanup.close().catch((cause: unknown) => cause);
+  expect(first).toMatchObject({
+    cleanupResources: ["selected-map-handle", "captured-script-handle"],
+  });
+  scriptClose.mockRejectedValueOnce(
+    new ProviderCleanupError("fixture", ["captured-script-handle"], {}),
+  );
+  const second = await cleanup.close().catch((cause: unknown) => cause);
+  expect(first).toMatchObject({
+    cleanupIncomplete: true,
+    cleanupResources: ["captured-script-handle"],
+  });
+  await cleanup.close();
+  expect(first).toMatchObject({
+    cleanupIncomplete: false,
+    cleanupResources: [],
+  });
+  expect(second).toMatchObject({
+    cleanupIncomplete: false,
+    cleanupResources: [],
+  });
+  expect(resourceClose).toHaveBeenCalledTimes(2);
+  expect(scriptClose).toHaveBeenCalledTimes(3);
+});

src/browser/WebArtifactCleanup.ts

@@ -1,3 +1,7 @@
+import {
+  AnalysisCleanupLease,
+  AnalysisCleanupLeaseGroup,
+} from "../artifacts/AnalysisCleanupLease.js";
 import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
 import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
 import type { WebScriptArtifactPort } from "../application/WebScriptArtifactPort.js";
@@ -10,43 +14,89 @@ type WebArtifactOwner =
   | "web-source-location-artifacts"
   | "web-module-artifacts";
 
-/** Close a composed artifact reader without closing a shared script scope twice. */
-export const closeWebArtifactResources = async (
-  owner: WebArtifactOwner,
-  resources: ArtifactResourceScope,
-  scripts: WebScriptArtifactPort,
-): Promise<void> => {
-  const sourceLocation = owner === "web-source-location-artifacts";
-  const operation = sourceLocation
-    ? "trace_web_source_location"
-    : "trace_web_module_imports";
-  const resource = sourceLocation
-    ? "web source-location artifacts"
-    : "web module artifacts";
-  const closeScript =
-    scripts instanceof LocalWebScriptArtifacts &&
-    scripts.resources === resources
-      ? undefined
-      : scripts.close?.bind(scripts);
-  const results = await Promise.allSettled([
-    Promise.resolve().then(() => resources.close()),
-    ...(closeScript === undefined ? [] : [Promise.resolve().then(closeScript)]),
-  ]);
-  const failures = results.flatMap((result, index) =>
-    result.status === "rejected"
-      ? [
-          {
-            cause: result.reason,
-            resource: index === 0 ? resource : "web scripts",
-          },
-        ]
-      : [],
+const cleanupOwner = (resource: string, close: () => Promise<void>) => {
+  let retained: readonly string[] = [resource];
+  const lease = new AnalysisCleanupLease(
+    async () => {
+      try {
+        await close();
+      } catch (cause: unknown) {
+        retained =
+          cause instanceof AnalysisError && cause.cleanupResources.length > 0
+            ? cause.cleanupResources
+            : ArtifactReaderFailure.cleanupObservation(cause, resource)
+                .resources;
+        throw cause;
+      }
+    },
+    {
+      get cleanupResources() {
+        return retained;
+      },
+    },
   );
-  const error = webArtifactCleanupFailures(owner, operation, failures);
-  if (error !== undefined) throw error;
+  return { resource, lease };
 };
 
-export const webArtifactCleanupFailures = (
+/** Retain each composed artifact owner's release across partial cleanup retries. */
+export class WebArtifactCleanup {
+  readonly #owners: readonly {
+    readonly resource: string;
+    readonly lease: AnalysisCleanupLease;
+  }[];
+  readonly #operation: string;
+
+  constructor(
+    private readonly owner: WebArtifactOwner,
+    resources: ArtifactResourceScope,
+    scripts: WebScriptArtifactPort,
+  ) {
+    const sourceLocation = owner === "web-source-location-artifacts";
+    this.#operation = sourceLocation
+      ? "trace_web_source_location"
+      : "trace_web_module_imports";
+    const resource = sourceLocation
+      ? "web source-location artifacts"
+      : "web module artifacts";
+    const closeScript =
+      scripts instanceof LocalWebScriptArtifacts &&
+      scripts.resources === resources
+        ? undefined
+        : scripts.close?.bind(scripts);
+    this.#owners = [
+      cleanupOwner(resource, () => resources.close()),
+      ...(closeScript === undefined
+        ? []
+        : [cleanupOwner("web scripts", closeScript)]),
+    ];
+  }
+
+  async close(): Promise<void> {
+    const pending = this.#owners.filter(({ lease }) => !lease.released);
+    const results = await Promise.allSettled(
+      pending.map(({ lease }) => lease.release()),
+    );
+    const failures = results.flatMap((result, index) =>
+      result.status === "rejected"
+        ? [{ cause: result.reason as unknown, ...pending[index]! }]
+        : [],
+    );
+    const error = webArtifactCleanupFailures(
+      this.owner,
+      this.#operation,
+      failures,
+    );
+    if (error !== undefined) {
+      new AnalysisCleanupLeaseGroup(
+        error,
+        failures.map(({ lease }) => lease),
+      );
+      throw error;
+    }
+  }
+}
+
+const webArtifactCleanupFailures = (
   owner: string,
   operation: string,
   failures: readonly { readonly resource: string; readonly cause: unknown }[],

src/browser/assets/WebSourceLocationArtifacts.ts

@@ -8,7 +8,7 @@ import {
   type WebSourceLocationInput,
 } from "../../domain/webSourceLocation.js";
 import { err, ok } from "../../domain/result.js";
-import { closeWebArtifactResources } from "../WebArtifactCleanup.js";
+import { WebArtifactCleanup } from "../WebArtifactCleanup.js";
 import { LocalWebScriptArtifacts } from "./LocalWebScriptArtifacts.js";
 import { decodeWebArtifact, webArtifactReadError } from "./WebArtifactReads.js";
 
@@ -16,6 +16,7 @@ import { decodeWebArtifact, webArtifactReadError } from "./WebArtifactReads.js";
 export class LocalWebSourceLocationArtifacts implements WebSourceLocationArtifactPort {
   readonly scripts: WebScriptArtifactPort;
   readonly #resources: ArtifactResourceScope;
+  readonly #cleanup: WebArtifactCleanup;
 
   constructor(
     scripts?: WebScriptArtifactPort,
@@ -27,14 +28,15 @@ export class LocalWebSourceLocationArtifacts implements WebSourceLocationArtifac
     this.scripts =
       scripts ??
       new LocalWebScriptArtifacts("trace_web_source_location", resources);
+    this.#cleanup = new WebArtifactCleanup(
+      "web-source-location-artifacts",
+      resources,
+      this.scripts,
+    );
   }
 
   async close(): Promise<void> {
-    return closeWebArtifactResources(
-      "web-source-location-artifacts",
-      this.#resources,
-      this.scripts,
-    );
+    return this.#cleanup.close();
   }
 
   /** Preserve actual map identity separately from the selected deployment URL. */

src/browser/modules/WebModuleArtifacts.ts

@@ -7,7 +7,7 @@ import type { ExecutionOptions } from "../../application/AnalysisProvider.js";
 import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js";
 import { jsonObjectSchema } from "../../domain/jsonValue.js";
 import { err, ok } from "../../domain/result.js";
-import { closeWebArtifactResources } from "../WebArtifactCleanup.js";
+import { WebArtifactCleanup } from "../WebArtifactCleanup.js";
 import type { WebModuleTraceInput } from "../../domain/webModuleTrace.js";
 import { LocalWebScriptArtifacts } from "../assets/LocalWebScriptArtifacts.js";
 import {
@@ -19,6 +19,7 @@ import {
 export class LocalWebModuleArtifacts implements WebModuleArtifactPort {
   readonly scripts: WebScriptArtifactPort;
   readonly #resources: ArtifactResourceScope;
+  readonly #cleanup: WebArtifactCleanup;
 
   constructor(
     scripts?: WebScriptArtifactPort,
@@ -30,14 +31,15 @@ export class LocalWebModuleArtifacts implements WebModuleArtifactPort {
     this.scripts =
       scripts ??
       new LocalWebScriptArtifacts("trace_web_module_imports", resources);
+    this.#cleanup = new WebArtifactCleanup(
+      "web-module-artifacts",
+      resources,
+      this.scripts,
+    );
   }
 
   async close(): Promise<void> {
-    return closeWebArtifactResources(
-      "web-module-artifacts",
-      this.#resources,
-      this.scripts,
-    );
+    return this.#cleanup.close();
   }
 
   /** Read selected source and optional map without website requests. */