fix(adb): retain and resolve owned hash cleanup

Keep exact returned hash and directory failures linked to their cleanup owners. Retain input staging while its descriptor remains unresolved, and resolve the linked errors only after successful release. Preserve typed cleanup and partial acquisition facts through file, package, and bugreport consumers instead of wrapping or flattening them away.

Validated 189 affected tests across 13 files, build, typecheck, lint, and docs. Three larger integration cases were excluded to respect the CPU limit. New real-file fault controls reproduce on the old source and verify persistent failure, owner identity, cancellation, retained output, and recovery. No physical ADB device or real JADX engine verification. Refs #4266, #4274.

src/adb/AdbProvider.ts

@@ -378,6 +378,8 @@ interface PendingAdbDirectoryCleanup {
     | "install_adb_package";
   readonly path: string;
   readonly primary: AnalysisError;
+  readonly reportedError: AnalysisError;
+  readonly prerequisite?: AnalysisError;
   readonly remove: () => Promise<void>;
 }
 
@@ -385,6 +387,7 @@ interface PendingAdbFileCleanup {
   readonly operation: string;
   readonly path: string;
   readonly owner: OwnedFileHandle;
+  readonly reportedError: AnalysisError;
 }
 
 interface OwnedDirectoryIdentity {
@@ -583,8 +586,6 @@ export class AdbProvider {
     try {
       return await hashStableFile(path, signal);
     } catch (cause: unknown) {
-      if (cause instanceof OwnedRegularFileCleanupFailure)
-        this.#pendingFileCleanup.add({ operation, path, owner: cause.owner });
       if (cause instanceof OwnedRegularFileCleanupFailure) {
         const primary = signal?.aborted
           ? new AnalysisCancelledError(operation, { cause: cause.cause })
@@ -594,10 +595,11 @@ export class AdbProvider {
                 cause.cause instanceof Error
                   ? cause.cause.message
                   : String(cause.cause),
-              diagnostics: (cause instanceof StableFileHashCleanupFailure &&
+              diagnostics:
+                cause instanceof StableFileHashCleanupFailure &&
                 cause.completedHash !== undefined
                   ? { completed_hash: cause.completedHash }
-                  : {}),
+                  : {},
             });
         const cleanup = new ProviderCleanupError(
           ADB_PROVIDER_IDENTITY.id,
@@ -614,7 +616,18 @@ export class AdbProvider {
           },
           { operation, cause: cause.cleanupCause },
         );
-        throw analysisErrorWithCleanupFailure(primary, cleanup, operation);
+        const reported = analysisErrorWithCleanupFailure(
+          primary,
+          cleanup,
+          operation,
+        );
+        this.#pendingFileCleanup.add({
+          operation,
+          path,
+          owner: cause.owner,
+          reportedError: reported,
+        });
+        throw reported;
       }
       if (signal?.aborted === true)
         throw new AnalysisCancelledError(operation, { cause });
@@ -676,6 +689,7 @@ export class AdbProvider {
       try {
         await owner.owner.close();
         this.#pendingFileCleanup.delete(owner);
+        owner.reportedError.resolveCleanup();
       } catch (cause: unknown) {
         failures.push(
           new ProviderCleanupError(
@@ -688,9 +702,21 @@ export class AdbProvider {
       }
     }
     for (const owner of this.#pendingDirectoryCleanup) {
+      if (owner.prerequisite?.cleanupIncomplete === true) {
+        failures.push(
+          new ProviderCleanupError(
+            ADB_PROVIDER_IDENTITY.id,
+            [owner.path],
+            { reason: "A dependent owned resource still needs cleanup" },
+            { operation: owner.operation, cause: owner.prerequisite },
+          ),
+        );
+        continue;
+      }
       try {
         await owner.remove();
         this.#pendingDirectoryCleanup.delete(owner);
+        owner.reportedError.resolveCleanup();
       } catch (cause: unknown) {
         const cleanup = new ProviderCleanupError(
           ADB_PROVIDER_IDENTITY.id,
@@ -793,7 +819,13 @@ export class AdbProvider {
     }
 
     if (root !== undefined) {
+      const prerequisite =
+        !result.ok && result.error.cleanupIncomplete ? result.error : undefined;
       const remove = async () => {
+        if (prerequisite?.cleanupIncomplete === true)
+          throw new Error(
+            "A staged file still has an unresolved cleanup owner",
+          );
         await verifyOwnedDirectory(root!.path, identity);
         await root!.close();
       };
@@ -841,14 +873,15 @@ export class AdbProvider {
           operation,
           path: root.path,
           primary,
+          reportedError:
+            primary instanceof AnalysisCancelledError
+              ? primary
+              : analysisErrorWithCleanupFailure(primary, cleanup, operation),
+          ...(prerequisite === undefined ? {} : { prerequisite }),
           remove,
         };
         this.#pendingDirectoryCleanup.add(cleanupOwner);
-        return err(
-          primary instanceof AnalysisCancelledError
-            ? primary
-            : analysisErrorWithCleanupFailure(primary, cleanup, operation),
-        );
+        return err(cleanupOwner.reportedError);
       }
     }
 
@@ -1650,14 +1683,6 @@ export class AdbProvider {
         { cause },
       );
       if (!packageDirectoryCreated) return err(primary);
-      const owner: PendingAdbDirectoryCleanup = {
-        operation,
-        path: packageDirectory,
-        primary,
-        remove: () =>
-          removeOwnedDirectory(packageDirectory, packageDirectoryIdentity),
-      };
-      this.#pendingDirectoryCleanup.add(owner);
       const cleanup = new ProviderCleanupError(
         ADB_PROVIDER_IDENTITY.id,
         [packageDirectory],
@@ -1666,31 +1691,49 @@ export class AdbProvider {
         },
         { operation, cause },
       );
-      return err(analysisErrorWithCleanupFailure(primary, cleanup, operation));
+      const reported = analysisErrorWithCleanupFailure(
+        primary,
+        cleanup,
+        operation,
+      );
+      this.#pendingDirectoryCleanup.add({
+        operation,
+        path: packageDirectory,
+        primary,
+        reportedError: reported,
+        remove: () =>
+          removeOwnedDirectory(packageDirectory, packageDirectoryIdentity),
+      });
+      return err(reported);
     }
     const failOwnedPackage = async (
       primary: AnalysisError,
     ): Promise<Result<AnalysisExecution, AnalysisError>> => {
-      const owner: PendingAdbDirectoryCleanup = {
-        operation,
-        path: packageDirectory,
-        primary,
-        remove: () =>
-          removeOwnedDirectory(packageDirectory, packageDirectoryIdentity),
-      };
+      const remove = () =>
+        removeOwnedDirectory(packageDirectory, packageDirectoryIdentity);
       try {
-        await owner.remove();
+        await remove();
       } catch (cause: unknown) {
-        this.#pendingDirectoryCleanup.add(owner);
         const cleanup = new ProviderCleanupError(
           ADB_PROVIDER_IDENTITY.id,
           [packageDirectory],
           { reason: cause instanceof Error ? cause.message : String(cause) },
           { operation, cause },
         );
-        return err(
-          analysisErrorWithCleanupFailure(primary, cleanup, operation),
+        const reported = analysisErrorWithCleanupFailure(
+          primary,
+          cleanup,
+          operation,
         );
+        this.#pendingDirectoryCleanup.add({
+          operation,
+          path: packageDirectory,
+          primary,
+          reportedError: reported,
+          prerequisite: primary,
+          remove,
+        });
+        return err(reported);
       }
       return err(primary);
     };
@@ -1717,8 +1760,8 @@ export class AdbProvider {
       stage: "resolve_paths" | "pull" | "digest";
       message: string;
     }[] = [];
-    const cancelledPackagePull = (
-      error: AnalysisCancelledError,
+    const partialPackagePull = (
+      error: AnalysisError,
       localPath: string,
       devicePath: string,
       stage: "pull" | "digest",
@@ -1739,8 +1782,8 @@ export class AdbProvider {
               stage,
               message:
                 stage === "pull"
-                  ? `Pull cancelled; any incomplete file is retained at ${localPath}. Remaining APKs were not attempted.`
-                  : `Digesting the pulled file was cancelled; the file is retained at ${localPath}. Remaining APKs were not attempted.`,
+                  ? `${error instanceof AnalysisCancelledError ? "Pull cancelled" : "Pull failed"}; any incomplete file is retained at ${localPath}. Remaining APKs were not attempted.`
+                  : `${error instanceof AnalysisCancelledError ? "Digesting the pulled file was cancelled" : "Digesting the pulled file failed"}; the file is retained at ${localPath}. Remaining APKs were not attempted.`,
             },
           ],
           coverage: "partial",
@@ -1760,7 +1803,7 @@ export class AdbProvider {
       );
       if (!pulled.ok) {
         if (pulled.error instanceof AnalysisCancelledError)
-          return cancelledPackagePull(
+          return partialPackagePull(
             pulled.error,
             localPath,
             entry.device_path,
@@ -1785,8 +1828,11 @@ export class AdbProvider {
           role_basis: "file_name",
         });
       } catch (cause: unknown) {
-        if (cause instanceof AnalysisCancelledError)
-          return cancelledPackagePull(
+        if (
+          cause instanceof AnalysisCancelledError ||
+          (cause instanceof AnalysisError && cause.cleanupIncomplete)
+        )
+          return partialPackagePull(
             cause,
             localPath,
             entry.device_path,
@@ -1888,7 +1934,10 @@ export class AdbProvider {
         ),
       );
     } catch (cause) {
-      if (cause instanceof AnalysisCancelledError) {
+      if (
+        cause instanceof AnalysisCancelledError ||
+        (cause instanceof AnalysisError && cause.cleanupIncomplete)
+      ) {
         cause.retainPartialObservation({
           provider_id: "adb",
           operation,
@@ -2629,7 +2678,10 @@ export class AdbProvider {
           }),
         );
       } catch (cause) {
-        if (cause instanceof AnalysisCancelledError) {
+        if (
+          cause instanceof AnalysisCancelledError ||
+          (cause instanceof AnalysisError && cause.cleanupIncomplete)
+        ) {
           if (retainedObservation !== undefined)
             cause.retainPartialObservation(retainedObservation);
           return err(cause);
@@ -2658,24 +2710,25 @@ export class AdbProvider {
         : collectedResult.error;
       if (retainedObservation !== undefined)
         primary.retainPartialObservation(retainedObservation);
+      const reported = analysisErrorWithCleanupFailure(
+        primary,
+        new ProviderCleanupError(
+          ADB_PROVIDER_IDENTITY.id,
+          [stagingDirectory],
+          { reason: cause instanceof Error ? cause.message : String(cause) },
+          { operation, cause },
+        ),
+        operation,
+      );
       this.#pendingDirectoryCleanup.add({
         operation,
         path: stagingDirectory,
         primary,
+        reportedError: reported,
+        prerequisite: primary,
         remove: () => removeOwnedDirectory(stagingDirectory, stagingIdentity),
       });
-      return err(
-        analysisErrorWithCleanupFailure(
-          primary,
-          new ProviderCleanupError(
-            ADB_PROVIDER_IDENTITY.id,
-            [stagingDirectory],
-            { reason: cause instanceof Error ? cause.message : String(cause) },
-            { operation, cause },
-          ),
-          operation,
-        ),
-      );
+      return err(reported);
     }
     return collectedResult;
   }

tests/boundary/providers/adb/adbHashCleanupConsumers.test.ts

@@ -0,0 +1,148 @@
+import { readFile, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { afterEach, expect, onTestFinished, test, vi } from "vitest";
+
+import { AdbProvider } from "../../../../src/adb/AdbProvider.js";
+import { OwnedFileHandle } from "../../../../src/filesystem/OwnedFileHandle.js";
+import {
+  execFileBytes,
+  execFileOutput,
+} from "../../../../src/process/ExecFileOutput.js";
+import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js";
+import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js";
+
+vi.mock(
+  "../../../../src/process/ExecFileOutput.js",
+  async (importOriginal) => ({
+    ...(await importOriginal<
+      typeof import("../../../../src/process/ExecFileOutput.js")
+    >()),
+    execFileBytes: vi.fn(),
+    execFileOutput: vi.fn(),
+  }),
+);
+
+afterEach(() => vi.restoreAllMocks());
+
+const fixture = async () => {
+  const root = await createTestTempDirectory("rea-adb-hash-consumer-");
+  const binary = join(root, "adb");
+  vi.mocked(execFileOutput).mockImplementation(async () => {
+    const stdout = `Android Debug Bridge version 1.0.41\nVersion 34.0.5-stub\nInstalled as ${binary}\n`;
+    return {
+      stdout,
+      stderr: "",
+      stdoutBytes: Buffer.byteLength(stdout),
+      stderrBytes: 0,
+    };
+  });
+  vi.mocked(execFileBytes).mockImplementation(async (_binary, args) => {
+    const last = args.at(-1)!;
+    if (args.includes("pull")) {
+      await writeFile(last, "pulled bytes");
+      return { stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) };
+    }
+    if (args.includes("path") && args.includes("pm"))
+      return {
+        stdout: Buffer.from("package:/data/app/example/base.apk\n"),
+        stderr: Buffer.alloc(0),
+      };
+    if (args.includes("bugreport")) {
+      const staging = last;
+      const archive = join(staging, "report.zip");
+      await writeFile(archive, "pulled bytes");
+      return {
+        stdout: Buffer.from(`Bug report copied to ${archive}\n`),
+        stderr: Buffer.alloc(0),
+      };
+    }
+    return { stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) };
+  });
+  const provider = new AdbProvider({ environment: { REA_ADB_PATH: binary } });
+  onTestFinished(() => provider.close());
+  const failFirstClose = () => {
+    const originalClose = OwnedFileHandle.prototype.close;
+    let failed = false;
+    vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation(
+      async function (this: OwnedFileHandle) {
+        if (!failed) {
+          failed = true;
+          throw new Error("hash descriptor close denied");
+        }
+        return originalClose.call(this);
+      },
+    );
+  };
+  return { root, provider, failFirstClose };
+};
+
+test.each([
+  {
+    label: "single-file pull",
+    retainedPath: (root: string) => join(root, "payload.bin"),
+    request: (root: string) => ({
+      operation: "pull_adb_file" as const,
+      input: {
+        serial: "device",
+        device_path: "/sdcard/payload.bin",
+        output_directory: root,
+      },
+    }),
+    expected: { digest_status: "unknown", device_path: "/sdcard/payload.bin" },
+  },
+  {
+    label: "package pull",
+    retainedPath: (root: string) => join(root, "example", "base.apk"),
+    request: (root: string) => ({
+      operation: "pull_adb_package" as const,
+      input: {
+        serial: "device",
+        package: "example",
+        output_directory: root,
+      },
+    }),
+    expected: {
+      package_name: "example",
+      coverage: "partial",
+      failures: [
+        { device_path: "/data/app/example/base.apk", stage: "digest" },
+      ],
+    },
+  },
+  {
+    label: "bugreport collection",
+    retainedPath: (root: string) => join(root, "report.zip"),
+    request: (root: string) => ({
+      operation: "collect_adb_bugreport" as const,
+      input: { serial: "device", output_directory: root },
+    }),
+    expected: { digest_status: "unknown", serial: "device" },
+  },
+])(
+  "preserves the linked hash cleanup error through $label",
+  async ({ request, expected, retainedPath }) => {
+    const f = await fixture();
+    f.failFirstClose();
+    const result = await f.provider.execute(request(f.root));
+    expect(result.ok).toBe(false);
+    if (result.ok) throw new Error("expected hash descriptor cleanup failure");
+    const returnedError = result.error;
+    const originalCause = returnedError.cause;
+    expect(returnedError.cleanupIncomplete).toBe(true);
+    expect(returnedError.diagnostics).toMatchObject({
+      primary_error: {
+        details: { diagnostics: { completed_hash: { bytes: 12 } } },
+      },
+    });
+    expect(
+      projectAnalysisError(returnedError).details?.partial_observation,
+    ).toMatchObject({ result: expected });
+
+    await f.provider.close();
+    expect(returnedError.cleanupIncomplete).toBe(false);
+    expect(returnedError.cause).toBe(originalCause);
+    await expect(readFile(retainedPath(f.root))).resolves.toEqual(
+      Buffer.from("pulled bytes"),
+    );
+  },
+);

tests/boundary/providers/adb/adbInputSnapshotOwnership.test.ts

@@ -11,6 +11,8 @@ import { basename, dirname, join } from "node:path";
 import { afterEach, expect, onTestFinished, test, vi } from "vitest";
 
 import { AdbProvider } from "../../../../src/adb/AdbProvider.js";
+import { AnalysisError } from "../../../../src/domain/analysisErrorBase.js";
+import { OwnedFileHandle } from "../../../../src/filesystem/OwnedFileHandle.js";
 import {
   execFileBytes,
   execFileOutput,
@@ -117,6 +119,7 @@ test("retains failed snapshot cleanup ownership and retries it on close", async
   const result = await f.provider.execute(f.request);
   expect(result.ok).toBe(false);
   if (result.ok) throw new Error("expected snapshot cleanup failure");
+  expect(result.error.cleanupIncomplete).toBe(true);
   expect(result.error.cleanupResources).toContain(dirname(f.snapshotPath()));
   const evidence = parseEvidence(result.error.partialObservation);
   expect(evidence).toMatchObject({
@@ -134,12 +137,102 @@ test("retains failed snapshot cleanup ownership and retries it on close", async
     await readFile(f.source),
   );
 
+  vi.mocked(rm).mockImplementationOnce(async (path, options) => {
+    if (basename(String(path)).startsWith("rea-adb-input-"))
+      throw new Error("snapshot cleanup retry denied");
+    return actualFs.rm(path, options);
+  });
+  await expect(f.provider.close()).rejects.toMatchObject({
+    cleanupIncomplete: true,
+  });
+  expect(result.error.cleanupIncomplete).toBe(true);
+
   await f.provider.close();
+  expect(result.error.cleanupIncomplete).toBe(false);
   await expect(lstat(dirname(f.snapshotPath()))).rejects.toMatchObject({
     code: "ENOENT",
   });
 });
 
+test("resolves a hash cleanup error only after the exact retained descriptor closes", async () => {
+  const f = await fixture();
+  const actualFs =
+    await vi.importActual<typeof import("node:fs/promises")>(
+      "node:fs/promises",
+    );
+  let stagedPath: string | undefined;
+  vi.mocked(copyFile).mockImplementationOnce(
+    async (source, destination, mode) => {
+      stagedPath = String(destination);
+      await actualFs.copyFile(source, destination, mode);
+    },
+  );
+  const closeFailure = new Error("digest descriptor close denied");
+  const owners: OwnedFileHandle[] = [];
+  let denialsRemaining = 2;
+  const originalClose = OwnedFileHandle.prototype.close;
+  const close = vi
+    .spyOn(OwnedFileHandle.prototype, "close")
+    .mockImplementation(async function (this: OwnedFileHandle) {
+      owners.push(this);
+      if (denialsRemaining > 0) {
+        denialsRemaining -= 1;
+        throw closeFailure;
+      }
+      return originalClose.call(this);
+    });
+  try {
+    const result = await f.provider.execute(f.request);
+    expect(result.ok).toBe(false);
+    if (result.ok) throw new Error("expected digest cleanup failure");
+    const reported = result.error;
+    const originalCause = reported.cause;
+    expect(reported.cleanupIncomplete).toBe(true);
+    expect(reported.cause).toBeInstanceOf(AggregateError);
+    const hashFailure = (reported.cause as AggregateError).errors[0];
+    if (!(hashFailure instanceof AnalysisError))
+      throw new Error("Missing original typed hash failure");
+    expect(hashFailure).toMatchObject({
+      diagnostics: {
+        primary_error: {
+          details: {
+            diagnostics: {
+              completed_hash: { bytes: 17, sha256: f.digest },
+            },
+          },
+        },
+      },
+    });
+    expect(owners).toHaveLength(1);
+    if (stagedPath === undefined) throw new Error("Missing staged input path");
+
+    await expect(f.provider.close()).rejects.toMatchObject({
+      cleanupIncomplete: true,
+      cleanupResources: [stagedPath, dirname(stagedPath)],
+    });
+    expect(reported.cleanupIncomplete).toBe(true);
+    expect(reported.cause).toBe(originalCause);
+    await expect(readFile(stagedPath)).resolves.toEqual(
+      await readFile(f.source),
+    );
+
+    await f.provider.close();
+    expect(owners).toHaveLength(3);
+    expect(owners[0]).toBe(owners[1]);
+    expect(owners[1]).toBe(owners[2]);
+    expect(reported.cleanupIncomplete).toBe(false);
+    expect(hashFailure.cleanupIncomplete).toBe(false);
+    expect(reported.cause).toBe(originalCause);
+    await expect(lstat(dirname(stagedPath))).rejects.toMatchObject({
+      code: "ENOENT",
+    });
+  } finally {
+    denialsRemaining = 0;
+    close.mockRestore();
+    await f.provider.close();
+  }
+});
+
 test.each([false, true])(
   "preserves cancellation and push receipt during final snapshot cleanup (cleanup failure: %s)",
   async (failCleanup) => {
@@ -163,6 +256,7 @@ test.each([false, true])(
     expect(result.ok).toBe(false);
     if (result.ok) throw new Error("expected cancellation");
     expect(result.error._tag).toBe("AnalysisCancelledError");
+    const originalCause = result.error.cause;
     const evidence = parseEvidence(result.error.partialObservation);
     expect(evidence).toMatchObject({
       provider: { id: "adb" },
@@ -176,13 +270,16 @@ test.each([false, true])(
         device_path: "/data/local/tmp/payload.bin",
       },
     });
-    if (failCleanup)
+    if (failCleanup) {
+      expect(result.error.cleanupIncomplete).toBe(true);
       expect(result.error.cleanupResources).toContain(
         dirname(f.snapshotPath()),
       );
-    else expect(result.error.cleanupIncomplete).toBe(false);
+    } else expect(result.error.cleanupIncomplete).toBe(false);
     expect(await readFile(f.source)).toEqual(Buffer.from("immutable payload"));
     await f.provider.close();
+    expect(result.error.cleanupIncomplete).toBe(false);
+    expect(result.error.cause).toBe(originalCause);
   },
 );