fix(javascript)!: retain nested artifact readers in caller scopes

Require the caller artifact resource scope when reading JavaScript artifacts. Transfer failed nested ASAR cleanup to that scope so later work retries the same owner before admission. Preserve traversal failures and attempt every remaining iterator during unwind.

Validation: reconstruction, integrity, content identity, cancellation and nested-reader cleanup boundary checks.

BREAKING CHANGE: readJavaScriptArtifactFiles now requires ArtifactResourceScope as its third argument; the optional abort signal is fourth.

src/application/javascript/JavaScriptArtifactReconstruction.ts

@@ -96,7 +96,12 @@ export const reconstructJavaScriptArtifact = async (
         "read_javascript_artifacts",
         "Reading inventoried JavaScript application sources",
       );
-      const files = await readJavaScriptArtifactFiles(reader, snapshot, signal);
+      const files = await readJavaScriptArtifactFiles(
+        reader,
+        snapshot,
+        scope,
+        signal,
+      );
       await reportPhase(
         "parse_javascript_sources",
         `Parsing and projecting ${String(files.files.length)} application source files`,

src/artifacts/javascript/JavaScriptArtifactFiles.ts

@@ -4,6 +4,7 @@ import type { Readable } from "node:stream";
 
 import { compareUnicodeCodePoints } from "../../domain/unicodeCodePointOrder.js";
 import { AsarArtifactReader } from "../AsarArtifactReader.js";
+import type { ArtifactResourceScope } from "../ArtifactResourceScope.js";
 import {
   ArtifactPathRegistry,
   normalizeArtifactPath,
@@ -39,6 +40,7 @@ interface ExpectedContainer {
 }
 
 interface ReadContext {
+  readonly resources: ArtifactResourceScope;
   readonly expected: ReadonlyMap<string, ExpectedFile>;
   readonly expectedContainers: ReadonlyMap<string, ExpectedContainer>;
   readonly registry: ArtifactPathRegistry;
@@ -59,6 +61,7 @@ interface ReadTextInput {
 export const readJavaScriptArtifactFiles = async (
   reader: ArtifactReader,
   snapshot: ArtifactInventorySnapshot,
+  resources: ArtifactResourceScope,
   signal?: AbortSignal,
 ): Promise<JavaScriptArtifactFileSet> => {
   if (reader instanceof AsarArtifactReader) {
@@ -79,6 +82,7 @@ export const readJavaScriptArtifactFiles = async (
   const inventory = expectedInventory(snapshot);
   const expected = inventory.files;
   const context: ReadContext = {
+    resources,
     expected,
     expectedContainers: inventory.containers,
     registry: new ArtifactPathRegistry(),
@@ -122,7 +126,10 @@ const visitReader = async (
       iterator: reader.entries(context.signal)[Symbol.asyncIterator](),
     },
   ];
-  const ownedReaders: ArtifactReader[] = [];
+  const ownedReaders: {
+    readonly reader: ArtifactReader;
+    readonly path: string;
+  }[] = [];
   let failure: { readonly cause: unknown } | undefined;
   try {
     while (stack.length > 0) {
@@ -174,7 +181,7 @@ const visitReader = async (
         );
         context.containers.push(inventory);
         const nested = new AsarArtifactReader(entry.adapterKey);
-        ownedReaders.push(nested);
+        ownedReaders.push({ reader: nested, path });
         stack.push({
           reader: nested,
           prefix: path,
@@ -205,24 +212,33 @@ const visitReader = async (
   } catch (cause: unknown) {
     failure = { cause };
   } finally {
-    for (const { iterator } of stack) {
+    for (const { iterator, prefix, reader } of stack.reverse()) {
       try {
         await iterator.return?.();
       } catch (cause: unknown) {
-        failure ??= { cause };
+        failure = {
+          cause: ArtifactReaderFailure.withCleanup(
+            failure === undefined ? cause : failure.cause,
+            ArtifactReaderFailure.cleanupObservation(
+              cause,
+              `JavaScript artifact traversal for ${prefix || reader.format}`,
+            ),
+          ),
+        };
       }
     }
     for (const owned of ownedReaders.reverse()) {
-      try {
-        await owned.close();
-      } catch (cause: unknown) {
+      const resource = `nested ASAR reader for ${owned.path}`;
+      const cleanup = await context.resources.release({
+        kind: "reader",
+        reader: owned.reader,
+        resource,
+      });
+      if (cleanup.kind === "failed") {
         failure = {
           cause: ArtifactReaderFailure.withCleanup(
-            failure?.cause ?? cause,
-            ArtifactReaderFailure.cleanupObservation(
-              cause,
-              `nested ASAR reader for ${prefix}`,
-            ),
+            failure === undefined ? cleanup.cause : failure.cause,
+            ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource),
           ),
         };
       }

tests/boundary/filesystem/javascriptArtifactContentIdentity.test.ts

@@ -5,7 +5,7 @@ import { createPackage } from "@electron/asar";
 import { describe, expect, it } from "vitest";
 
 import { scanArtifactInventory } from "../../fixtures/artifactInventory.js";
-import { readJavaScriptArtifactFiles } from "../../../src/artifacts/javascript/JavaScriptArtifactFiles.js";
+import { readJavaScriptArtifactFiles } from "../../support/javascriptApplicationScope.js";
 import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js";
 import { DirectoryArtifactReader } from "../../../src/artifacts/DirectoryArtifactReader.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";

tests/boundary/filesystem/javascriptArtifactReconstruction.test.ts

@@ -6,8 +6,10 @@ import { expect, it } from "vitest";
 
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 
-import { readJavaScriptArtifactFiles } from "../../../src/artifacts/javascript/JavaScriptArtifactFiles.js";
-import { reconstructJavaScriptArtifact } from "../../support/javascriptApplicationScope.js";
+import {
+  reconstructJavaScriptArtifact,
+  readJavaScriptArtifactFiles,
+} from "../../support/javascriptApplicationScope.js";
 import { scanArtifactInventory } from "../../fixtures/artifactInventory.js";
 import {
   type ArtifactEntry,

tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts

@@ -13,7 +13,7 @@ import { analyzeJavaScriptApplication } from "../../support/javascriptApplicatio
 import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js";
 import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
 import { createJavaScriptArtifactReader } from "../../../src/artifacts/javascript/JavaScriptArtifactReader.js";
-import { readJavaScriptArtifactFiles } from "../../../src/artifacts/javascript/JavaScriptArtifactFiles.js";
+import { readJavaScriptArtifactFiles } from "../../support/javascriptApplicationScope.js";
 import { scanCanonicalArtifactInventory } from "../../fixtures/artifactInventory.js";
 import { createStrippedAsarAddon } from "../../fixtures/strippedAsarAddon.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";

tests/boundary/filesystem/javascriptNestedReaderCleanup.test.ts

@@ -0,0 +1,76 @@
+import { mkdir, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+
+import { createPackage } from "@electron/asar";
+import { expect, it, onTestFinished, vi } from "vitest";
+
+import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js";
+import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js";
+import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js";
+import { reconstructJavaScriptArtifact } from "../../../src/application/javascript/JavaScriptArtifactReconstruction.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+it("retains a failed nested reader through blocked work and scope cleanup", async () => {
+  const root = await createTestTempDirectory("rea-js-nested-reader-cleanup-");
+  const bundle = join(root, "bundle");
+  const contents = join(root, "asar-contents");
+  const archive = join(bundle, "app.asar");
+  await Promise.all([mkdir(bundle), mkdir(contents)]);
+  await writeFile(join(contents, "package.json"), '{"main":"index.js"}\n');
+  await writeFile(join(contents, "index.js"), "module.exports = 1;\n");
+  await createPackage(contents, archive);
+
+  const scope = new ArtifactResourceScope();
+  const originalClose = AsarArtifactReader.prototype.close;
+  const closedReaders: AsarArtifactReader[] = [];
+  const closeFailure = new Error("nested ASAR close failed");
+  let blockReadPhaseClose = true;
+  onTestFinished(async () => {
+    blockReadPhaseClose = false;
+    await scope.close().catch(() => undefined);
+    vi.restoreAllMocks();
+  });
+  vi.spyOn(AsarArtifactReader.prototype, "close").mockImplementation(
+    async function (this: AsarArtifactReader) {
+      closedReaders.push(this);
+      // Inventory closes its nested reader first. Fail the next, read-phase
+      // reader close after its native handle remains available for scope retry.
+      if (this === closedReaders[1] && blockReadPhaseClose) throw closeFailure;
+      await originalClose.call(this);
+    },
+  );
+
+  const input = { input_path: bundle };
+  const firstFailure = await reconstructJavaScriptArtifact(input, scope).catch(
+    (cause: unknown) => cause,
+  );
+  expect(firstFailure).toBeInstanceOf(ArtifactReaderFailure);
+  expect(firstFailure).toMatchObject({
+    cleanup: {
+      reason: closeFailure.message,
+      resources: [expect.stringContaining("app.asar")],
+    },
+  });
+  expect(closedReaders).toHaveLength(2);
+  const retainedReader = closedReaders[1];
+  expect(retainedReader).toBeDefined();
+  expect(retainedReader).not.toBe(closedReaders[0]);
+
+  const blockedWork = await reconstructJavaScriptArtifact(input, scope).catch(
+    (cause: unknown) => cause,
+  );
+  expect(blockedWork).toMatchObject({
+    reason: "unavailable",
+    cleanup: {
+      reason: closeFailure.message,
+      resources: [expect.stringContaining("app.asar")],
+    },
+  });
+  expect(closedReaders).toHaveLength(3);
+  expect(closedReaders[2]).toBe(retainedReader);
+
+  blockReadPhaseClose = false;
+  await scope.close();
+  expect(closedReaders).toHaveLength(4);
+  expect(closedReaders[3]).toBe(retainedReader);
+});

tests/composition/javascript-applications/javascriptArtifactReconstruction.test.ts

@@ -11,7 +11,10 @@ import { expect, it } from "vitest";
 
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 
-import { reconstructJavaScriptArtifact } from "../../support/javascriptApplicationScope.js";
+import {
+  reconstructJavaScriptArtifact,
+  readJavaScriptArtifactFiles,
+} from "../../support/javascriptApplicationScope.js";
 import {
   analyzeJavaScriptArtifactFiles,
   analyzeAndProjectJavaScriptArtifactFiles,
@@ -22,7 +25,6 @@ import {
   createJavaScriptSemanticGraphProjection,
 } from "../../../src/application/javascript/JavaScriptSemanticGraphBuilder.js";
 import { createJavaScriptArtifactReader } from "../../../src/artifacts/javascript/JavaScriptArtifactReader.js";
-import { readJavaScriptArtifactFiles } from "../../../src/artifacts/javascript/JavaScriptArtifactFiles.js";
 import {
   scanCanonicalArtifactInventory,
   scanArtifactInventory,

tests/support/javascriptApplicationScope.ts

@@ -3,6 +3,19 @@ import type { ExecutionOptions } from "../../src/application/AnalysisProvider.js
 import { analyzeJavaScriptApplication as analyzeOwned } from "../../src/application/javascript/JavaScriptApplicationService.js";
 import { reconstructJavaScriptArtifact as reconstructOwned } from "../../src/application/javascript/JavaScriptArtifactReconstruction.js";
 import type { ProgressReporter } from "../../src/application/ProgressReporter.js";
+import { readJavaScriptArtifactFiles as readOwned } from "../../src/artifacts/javascript/JavaScriptArtifactFiles.js";
+import type { ArtifactReader } from "../../src/artifacts/ArtifactReader.js";
+import type { ArtifactInventorySnapshot } from "../../src/domain/artifactInventorySnapshot.js";
+
+/** Read a fixture's nested containers with an awaited cleanup owner. */
+export const readJavaScriptArtifactFiles = (
+  reader: ArtifactReader,
+  snapshot: ArtifactInventorySnapshot,
+  signal?: AbortSignal,
+) =>
+  withArtifactResourceScope((scope) =>
+    readOwned(reader, snapshot, scope, signal),
+  );
 
 /** Run a JavaScript analysis with an awaited, test-owned artifact scope. */
 export const analyzeJavaScriptApplication = (