refactor(browser): share composed artifact cleanup ownership

src/browser/WebArtifactCleanup.test.ts

@@ -0,0 +1,71 @@
+import { expect, it, vi } from "vitest";
+import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
+import { LocalWebScriptArtifacts } from "./assets/LocalWebScriptArtifacts.js";
+import { closeWebArtifactResources } from "./WebArtifactCleanup.js";
+
+it("closes a shared script/resource scope once", async () => {
+  const resources = new ArtifactResourceScope();
+  const close = vi.spyOn(resources, "close").mockResolvedValue();
+  const scripts = new LocalWebScriptArtifacts(
+    "trace_web_module_imports",
+    resources,
+  );
+
+  await closeWebArtifactResources("web-module-artifacts", resources, scripts);
+
+  expect(close).toHaveBeenCalledTimes(1);
+});
+
+it.each([
+  ["web-module-artifacts", "trace_web_module_imports", "web module artifacts"],
+  [
+    "web-source-location-artifacts",
+    "trace_web_source_location",
+    "web source-location artifacts",
+  ],
+] as const)(
+  "attempts both cleanup owners and retries them for %s",
+  async (owner, operation, resource) => {
+    const resources = new ArtifactResourceScope();
+    let resourceCloseCalls = 0;
+    resources.addCloseHook(async () => {
+      resourceCloseCalls++;
+      if (resourceCloseCalls === 1)
+        throw new Error("artifact close refused once");
+    });
+    const scripts = new LocalWebScriptArtifacts(
+      "trace_web_module_imports",
+      new ArtifactResourceScope(),
+    );
+    let scriptCloseCalls = 0;
+    vi.spyOn(scripts, "close").mockImplementation(async () => {
+      scriptCloseCalls++;
+      if (scriptCloseCalls === 1) throw new Error("script close refused once");
+    });
+
+    await expect(
+      closeWebArtifactResources(owner, resources, scripts),
+    ).rejects.toMatchObject({
+      operation,
+      cleanupResources: [resource, "web scripts"],
+      diagnostics: {
+        failures: [
+          {
+            message: "artifact close refused once",
+            resource: [resource],
+          },
+          {
+            message: "script close refused once",
+            resource: ["web scripts"],
+          },
+        ],
+      },
+    });
+    expect(resourceCloseCalls).toBe(1);
+    expect(scriptCloseCalls).toBe(1);
+
+    await closeWebArtifactResources(owner, resources, scripts);
+    expect(resourceCloseCalls).toBe(2);
+    expect(scriptCloseCalls).toBe(2);
+  },
+);

src/browser/WebArtifactCleanup.ts

@@ -1,8 +1,51 @@
 import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
+import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
+import type { WebScriptArtifactPort } from "../application/WebScriptArtifactPort.js";
+import { LocalWebScriptArtifacts } from "./assets/LocalWebScriptArtifacts.js";
 import { AnalysisError } from "../domain/analysisErrorBase.js";
 import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
 import { ProviderCleanupError } from "../domain/providerCleanupError.js";
 
+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 error = webArtifactCleanupFailures(owner, operation, failures);
+  if (error !== undefined) throw error;
+};
+
 export const webArtifactCleanupFailures = (
   owner: string,
   operation: string,

src/browser/assets/WebSourceLocationArtifacts.ts

@@ -8,7 +8,7 @@ import {
   type WebSourceLocationInput,
 } from "../../domain/webSourceLocation.js";
 import { err, ok } from "../../domain/result.js";
-import { webArtifactCleanupFailures } from "../WebArtifactCleanup.js";
+import { closeWebArtifactResources } from "../WebArtifactCleanup.js";
 import { LocalWebScriptArtifacts } from "./LocalWebScriptArtifacts.js";
 import { decodeWebArtifact, webArtifactReadError } from "./WebArtifactReads.js";
 
@@ -30,34 +30,11 @@ export class LocalWebSourceLocationArtifacts implements WebSourceLocationArtifac
   }
 
   async close(): Promise<void> {
-    const closeScript =
-      this.scripts instanceof LocalWebScriptArtifacts &&
-      this.scripts.resources === this.#resources
-        ? undefined
-        : this.scripts.close?.bind(this.scripts);
-    const results = await Promise.allSettled([
-      Promise.resolve().then(() => this.#resources.close()),
-      ...(closeScript === undefined
-        ? []
-        : [Promise.resolve().then(closeScript)]),
-    ]);
-    const failures = results.flatMap((result, index) =>
-      result.status === "rejected"
-        ? [
-            {
-              cause: result.reason,
-              resource:
-                index === 0 ? "web source-location artifacts" : "web scripts",
-            },
-          ]
-        : [],
-    );
-    const error = webArtifactCleanupFailures(
+    return closeWebArtifactResources(
       "web-source-location-artifacts",
-      "trace_web_source_location",
-      failures,
+      this.#resources,
+      this.scripts,
     );
-    if (error !== undefined) throw error;
   }
 
   /** 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 { webArtifactCleanupFailures } from "../WebArtifactCleanup.js";
+import { closeWebArtifactResources } from "../WebArtifactCleanup.js";
 import type { WebModuleTraceInput } from "../../domain/webModuleTrace.js";
 import { LocalWebScriptArtifacts } from "../assets/LocalWebScriptArtifacts.js";
 import {
@@ -33,33 +33,11 @@ export class LocalWebModuleArtifacts implements WebModuleArtifactPort {
   }
 
   async close(): Promise<void> {
-    const closeScript =
-      this.scripts instanceof LocalWebScriptArtifacts &&
-      this.scripts.resources === this.#resources
-        ? undefined
-        : this.scripts.close?.bind(this.scripts);
-    const results = await Promise.allSettled([
-      Promise.resolve().then(() => this.#resources.close()),
-      ...(closeScript === undefined
-        ? []
-        : [Promise.resolve().then(closeScript)]),
-    ]);
-    const failures = results.flatMap((result, index) =>
-      result.status === "rejected"
-        ? [
-            {
-              cause: result.reason,
-              resource: index === 0 ? "web module artifacts" : "web scripts",
-            },
-          ]
-        : [],
-    );
-    const error = webArtifactCleanupFailures(
+    return closeWebArtifactResources(
       "web-module-artifacts",
-      "trace_web_module_imports",
-      failures,
+      this.#resources,
+      this.scripts,
     );
-    if (error !== undefined) throw error;
   }
 
   /** Read selected source and optional map without website requests. */