fix(ghidra): keep and name a runtime directory that close could not remove

When removing Ghidra's private runtime directory failed, close_binary
reported only a generic `provider-client` resource, and a later close,
after the permission was restored, returned success while the directory
and its contents remained. The client dropped its reference to the root
before removing it, and PrivateRuntimeRoot cached the rejected removal.

Keep each runtime or socket root until its removal succeeds, report a
failure as a ProviderCleanupError naming the leftover paths, let
PrivateRuntimeRoot retry a failed removal, and pass such typed cleanup
failures through closeAnalysisClient instead of replacing them.

Fixes #1190

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/application/binary/AnalysisClientCleanup.test.ts

@@ -0,0 +1,45 @@
+import { expect, it } from "vitest";
+
+import { projectAnalysisError } from "../../domain/analysisErrorProjection.js";
+import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
+import { err } from "../../domain/result.js";
+import type { AnalysisClient } from "../AnalysisProvider.js";
+import { closeAnalysisClient } from "./AnalysisClientCleanup.js";
+
+const clientClosingWith = (cause: unknown): AnalysisClient => ({
+  execute: () => Promise.resolve(err(new ProviderCleanupError("x", [], {}))),
+  close: () => Promise.reject(cause),
+});
+
+it("keeps the leftover resources a typed cleanup failure names", async () => {
+  const leftover = "/tmp/rea-ghidra-fixture";
+  const failure = new ProviderCleanupError("ghidra", [leftover], {
+    reason: "runtime directory retained",
+  });
+
+  const closed = await closeAnalysisClient(
+    clientClosingWith(failure),
+    "ghidra",
+  );
+
+  expect(closed.ok).toBe(false);
+  if (!closed.ok)
+    expect(projectAnalysisError(closed.error)).toMatchObject({
+      code: "cleanup_incomplete",
+      details: { resources: [leftover] },
+    });
+});
+
+it("still reports an untyped close rejection as the provider client", async () => {
+  const closed = await closeAnalysisClient(
+    clientClosingWith(new Error("unexpected")),
+    "ghidra",
+  );
+
+  expect(closed.ok).toBe(false);
+  if (!closed.ok)
+    expect(projectAnalysisError(closed.error)).toMatchObject({
+      code: "cleanup_incomplete",
+      details: { resources: ["provider-client"] },
+    });
+});

src/application/binary/AnalysisClientCleanup.ts

@@ -1,6 +1,6 @@
 import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
 import { err, ok, type Result } from "../../domain/result.js";
-import type { AnalysisError } from "../../domain/analysisErrorBase.js";
+import { AnalysisError } from "../../domain/analysisErrorBase.js";
 import type { AnalysisClient, ExecutionOptions } from "../AnalysisProvider.js";
 
 /** Close one provider client and normalize unexpected adapter rejection. */
@@ -17,6 +17,9 @@ export const closeAnalysisClient = async (
     await client.close();
     return ok(null);
   } catch (cause: unknown) {
+    // A typed cleanup failure already names its leftover resources.
+    if (cause instanceof AnalysisError && cause.cleanupIncomplete)
+      return err(cause);
     return err(
       new ProviderCleanupError(
         providerId,

src/ghidra/GhidraClient.ts

@@ -3,6 +3,7 @@ import type { Socket } from "node:net";
 import { tmpdir } from "node:os";
 
 import type { JsonValue } from "../domain/jsonValue.js";
+import { ProviderCleanupError } from "../domain/providerCleanupError.js";
 import { err, ok, type Result } from "../domain/result.js";
 import { silentLogger, type Logger } from "../logger.js";
 import { PendingOperations } from "../process/PendingOperations.js";
@@ -534,17 +535,8 @@ export class GhidraClient {
       this.#lastDiagnostics = this.#diagnostics();
       this.#process = undefined;
       this.#launch = undefined;
-      const runtimeRoot = this.#runtimeRoot;
-      const socketRoot = this.#socketRoot;
-      this.#runtimeRoot = undefined;
-      this.#socketRoot = undefined;
       try {
-        const removed = await Promise.allSettled([
-          runtimeRoot?.close(),
-          socketRoot?.close(),
-        ]);
-        const failed = removed.find((result) => result.status === "rejected");
-        if (failed?.status === "rejected") throw failed.reason;
+        await this.#removeRuntimeRoots();
       } catch (cause: unknown) {
         if (forceStop)
           forcedStopFailure ??= this.#failure(
@@ -565,6 +557,47 @@ export class GhidraClient {
     }
   }
 
+  /**
+   * Remove the runtime and socket roots, keeping each one that could not be
+   * removed so a later close retries it rather than reporting success.
+   */
+  async #removeRuntimeRoots(): Promise<void> {
+    const roots = [
+      {
+        root: this.#runtimeRoot,
+        release: () => (this.#runtimeRoot = undefined),
+      },
+      { root: this.#socketRoot, release: () => (this.#socketRoot = undefined) },
+    ];
+    const leftovers: { readonly path: string; readonly reason: string }[] = [];
+    let firstCause: unknown;
+    for (const { root, release } of roots) {
+      if (root === undefined) continue;
+      try {
+        await root.close();
+        release();
+      } catch (cause: unknown) {
+        firstCause ??= cause;
+        leftovers.push({
+          path: root.path,
+          reason: cause instanceof Error ? cause.message : String(cause),
+        });
+      }
+    }
+    if (leftovers.length > 0)
+      throw new ProviderCleanupError(
+        "ghidra",
+        leftovers.map(({ path }) => path),
+        {
+          reason:
+            "Ghidra's private runtime directory could not be removed; it is retained for a later close.",
+          leftover_paths: leftovers.map(({ path }) => path),
+          failures: leftovers.map(({ path, reason }) => `${path}: ${reason}`),
+        },
+        { cause: firstCause },
+      );
+  }
+
   #redactAuthentication(value: string): string {
     for (const token of this.#authenticationTokens)
       value = value.replaceAll(token, "[REDACTED]");

src/process/PrivateRuntimeRoot.ts

@@ -86,12 +86,19 @@ export class PrivateRuntimeRoot {
     }
   }
 
-  /** Remove the runtime root; concurrent and repeated callers share cleanup. */
+  /**
+   * Remove the runtime root; concurrent callers share one attempt. A failed
+   * attempt is not cached, so a later close retries the removal.
+   */
   close(): Promise<void> {
-    this.#closePromise ??=
+    this.#closePromise ??= (
       this.windowsRuntime === undefined
         ? rm(this.path, { recursive: true, force: true })
-        : Promise.resolve().then(() => this.windowsRuntime?.close());
+        : Promise.resolve().then(() => this.windowsRuntime?.close())
+    ).catch((cause: unknown) => {
+      this.#closePromise = undefined;
+      throw cause;
+    });
     return this.#closePromise;
   }
 }

tests/boundary/providers/ghidra/ghidraClient.test.ts

@@ -1,7 +1,7 @@
 import { spawn, type ChildProcess } from "node:child_process";
 import { createHash } from "node:crypto";
 import { readFileSync } from "node:fs";
-import { access, mkdir } from "node:fs/promises";
+import { access, chmod, mkdir } from "node:fs/promises";
 import { join } from "node:path";
 import { fileURLToPath } from "node:url";
 
@@ -386,6 +386,35 @@ describe("GhidraClient established requests", () => {
   });
 });
 
+describe("GhidraClient runtime removal failures", () => {
+  it.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
+    "reports a runtime root it could not remove and removes it on a later close",
+    async () => {
+      const launcher = new FixtureLauncher();
+      const client = clientFor(launcher);
+      await expect(client.start()).resolves.toMatchObject({ ok: true });
+      const runtimeRoot = launcher.runtimeRoots[0] ?? "";
+      // Without write permission its entries cannot be removed.
+      await chmod(runtimeRoot, 0o500);
+      try {
+        await expect(client.close()).rejects.toMatchObject({
+          cleanupIncomplete: true,
+          cleanupResources: [runtimeRoot],
+          diagnostics: { leftover_paths: [runtimeRoot] },
+        });
+        await expect(access(runtimeRoot)).resolves.toBeUndefined();
+      } finally {
+        await chmod(runtimeRoot, 0o700);
+      }
+
+      await expect(client.close()).resolves.toBeUndefined();
+      await expect(access(runtimeRoot)).rejects.toMatchObject({
+        code: "ENOENT",
+      });
+    },
+  );
+});
+
 describe("GhidraClient cleanup and diagnostics", () => {
   it("makes double close idempotent and releases its process runtime", async () => {
     const launcher = new FixtureLauncher();