[PR #1201] [MERGED] fix(ghidra): keep and name a runtime directory that close could not remove #2714

Closed
opened 2026-10-17 21:09:23 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1201
Author: @kaoru0822-kitauji
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/ghidra-runtime-cleanup-retry


📝 Commits (1)

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

📊 Changes

5 files changed (+132 additions, -15 deletions)

View changed files

➕ src/application/binary/AnalysisClientCleanup.test.ts (+45 -0)
📝 src/application/binary/AnalysisClientCleanup.ts (+4 -1)
📝 src/ghidra/GhidraClient.ts (+43 -10)
📝 src/process/PrivateRuntimeRoot.ts (+10 -3)
📝 tests/boundary/providers/ghidra/ghidraClient.test.ts (+30 -1)

📄 Description

Summary

If removing Ghidra's private runtime directory failed (the issue reproduces this by revoking write permission), close_binary reported cleanup_incomplete with only resources: ["provider-client"] and "provider client close rejected unexpectedly", and no directory path. Then, once the permission was restored, a second close_binary returned success although the rea-ghidra-* directory (target snapshot, project, logs, socket) was still there, and server shutdown left it as well.

Fixes #1190

Cause

  1. GhidraClient.#cleanup cleared #runtimeRoot/#socketRoot before trying to remove them, so after a failure no later close knew about the directory.
  2. PrivateRuntimeRoot.close() cached its removal promise, so even a retained reference would have returned the same rejection rather than retrying.
  3. closeAnalysisClient replaced every close rejection with a generic ProviderCleanupError(["provider-client"]).

Change

  • src/ghidra/GhidraClient.ts: #removeRuntimeRoots() removes the runtime and socket roots one by one, releasing each reference only after its removal succeeds. On failure it throws ProviderCleanupError("ghidra", <leftover paths>, { reason, leftover_paths, failures }), where each failure is path: OS message. The forced-stop path (active-request cancellation) keeps its existing wrapping. In both cases the roots stay owned, so a later close or server shutdown retries them.
  • src/process/PrivateRuntimeRoot.ts: concurrent close() calls still share one attempt, but a rejected attempt is no longer cached, so the next close retries.
  • src/application/binary/AnalysisClientCleanup.ts: an AnalysisError that already reports incomplete cleanup (cleanupIncomplete) is passed through unchanged. Untyped rejections, and Ghidra's own session errors, are still wrapped as provider-client as before.

Resulting projection for the issue's first close: cleanup_incomplete, details.resources: ["/…/rea-ghidra-XXXX"], diagnostics.leftover_paths and the EACCES failure text. The second close, after the permission is restored, removes the directory and succeeds.

Tests

  • tests/boundary/providers/ghidra/ghidraClient.test.ts (GhidraClient runtime removal failures): with the fake Ghidra launcher, the runtime root is made unwritable after start. close() rejects with cleanupIncomplete, cleanupResources: [runtimeRoot], and leftover_paths, and the directory remains. After restoring the permission, close() resolves and the directory is gone. Fails on main, where the second close resolved and left the directory. Skipped on Windows and as root.
  • src/application/binary/AnalysisClientCleanup.test.ts: a typed cleanup failure keeps its leftover path in details.resources (fails on main); an untyped rejection still reports provider-client.

Verification

  • npm run check:fast, npm run jscpd
  • npm run test:focused -- all Ghidra tests plus every test touching PrivateRuntimeRoot, closeAnalysisClient, ProviderCleanupError, or cleanup_incomplete (38 files: 456 passed, 117 skipped)

Not verified against a real Ghidra installation (none here). The change is in REA's own runtime-directory lifecycle, which the fake-launcher boundary test exercises through the real GhidraClient.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1201 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/ghidra-runtime-cleanup-retry` --- ### 📝 Commits (1) - [`b79d379`](https://github.com/morluto/rea/commit/b79d379c25bcc8ed61571c41a94b912b5704a067) fix(ghidra): keep and name a runtime directory that close could not remove ### 📊 Changes **5 files changed** (+132 additions, -15 deletions) <details> <summary>View changed files</summary> ➕ `src/application/binary/AnalysisClientCleanup.test.ts` (+45 -0) 📝 `src/application/binary/AnalysisClientCleanup.ts` (+4 -1) 📝 `src/ghidra/GhidraClient.ts` (+43 -10) 📝 `src/process/PrivateRuntimeRoot.ts` (+10 -3) 📝 `tests/boundary/providers/ghidra/ghidraClient.test.ts` (+30 -1) </details> ### 📄 Description ## Summary If removing Ghidra's private runtime directory failed (the issue reproduces this by revoking write permission), `close_binary` reported `cleanup_incomplete` with only `resources: ["provider-client"]` and "provider client close rejected unexpectedly", and no directory path. Then, once the permission was restored, a second `close_binary` returned success although the `rea-ghidra-*` directory (target snapshot, project, logs, socket) was still there, and server shutdown left it as well. Fixes #1190 ## Cause 1. `GhidraClient.#cleanup` cleared `#runtimeRoot`/`#socketRoot` before trying to remove them, so after a failure no later close knew about the directory. 2. `PrivateRuntimeRoot.close()` cached its removal promise, so even a retained reference would have returned the same rejection rather than retrying. 3. `closeAnalysisClient` replaced every close rejection with a generic `ProviderCleanupError(["provider-client"])`. ## Change - `src/ghidra/GhidraClient.ts`: `#removeRuntimeRoots()` removes the runtime and socket roots one by one, releasing each reference only after its removal succeeds. On failure it throws `ProviderCleanupError("ghidra", <leftover paths>, { reason, leftover_paths, failures })`, where each failure is `path: OS message`. The forced-stop path (active-request cancellation) keeps its existing wrapping. In both cases the roots stay owned, so a later close or server shutdown retries them. - `src/process/PrivateRuntimeRoot.ts`: concurrent `close()` calls still share one attempt, but a rejected attempt is no longer cached, so the next close retries. - `src/application/binary/AnalysisClientCleanup.ts`: an `AnalysisError` that already reports incomplete cleanup (`cleanupIncomplete`) is passed through unchanged. Untyped rejections, and Ghidra's own session errors, are still wrapped as `provider-client` as before. Resulting projection for the issue's first close: `cleanup_incomplete`, `details.resources: ["/…/rea-ghidra-XXXX"]`, `diagnostics.leftover_paths` and the `EACCES` failure text. The second close, after the permission is restored, removes the directory and succeeds. ## Tests - `tests/boundary/providers/ghidra/ghidraClient.test.ts` (`GhidraClient runtime removal failures`): with the fake Ghidra launcher, the runtime root is made unwritable after start. `close()` rejects with `cleanupIncomplete`, `cleanupResources: [runtimeRoot]`, and `leftover_paths`, and the directory remains. After restoring the permission, `close()` resolves and the directory is gone. Fails on `main`, where the second close resolved and left the directory. Skipped on Windows and as root. - `src/application/binary/AnalysisClientCleanup.test.ts`: a typed cleanup failure keeps its leftover path in `details.resources` (fails on `main`); an untyped rejection still reports `provider-client`. ## Verification - `npm run check:fast`, `npm run jscpd` - `npm run test:focused --` all Ghidra tests plus every test touching `PrivateRuntimeRoot`, `closeAnalysisClient`, `ProviderCleanupError`, or `cleanup_incomplete` (38 files: 456 passed, 117 skipped) Not verified against a real Ghidra installation (none here). The change is in REA's own runtime-directory lifecycle, which the fake-launcher boundary test exercises through the real `GhidraClient`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-17 21:09:23 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#2714
No description provided.