[PR #6282] [MERGED] fix(ghidra): skip the Java probe without an installation #7254

New issue

Closed

opened 2026-11-01 21:46:30 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:30 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/6200-hermes-sticky-profile-flake

📝 Commits (1)

81b9f2a fix(ghidra): skip the Java probe without an installation

📊 Changes

4 files changed (+126 additions, -14 deletions)

View changed files

📝 src/ghidra/GhidraInstallation.test.ts (+45 -0)
📝 src/ghidra/GhidraInstallation.ts (+5 -2)
📝 tests/acceptance/setup/hermesClientSetup.test.ts (+16 -12)
📝 tests/boundary/cli/setupProviderReadiness.test.ts (+60 -0)

📄 Description

What broke: rea setup and rea doctor ran the host java from PATH even when Ghidra was not configured. When that probe's owned-process cleanup failed, setup exited 1 with cleanup_incomplete. This made the Hermes setup acceptance tests fail at random on Linux CI.
Fix: The Ghidra inspection now runs Java only when an installation is selected (GHIDRA_INSTALL_DIR, or a Ghidra launcher found on PATH). This is the same rule that already applies to a relative JAVA_HOME. Hermes test exit-code assertions now print the CLI JSON when they fail.
Risk: Low. An unconfigured Ghidra still reports only the not_configured configuration check, as before. Its unavailable diagnostics now show java_version/java_home as null instead of facts from the host JVM. No process-ownership policy changed.

Summary

Fixes #6200

Setup and doctor inspect every deep-analysis provider. For Ghidra, inspectGhidraInstallation still ran java -XshowSettings:properties -version when no installation directory was set or found. Nothing used the result: the configuration check fails first, and doctor shows only that check. But a failure while the probe was cleaned up still aborted the whole command. So rea setup --client hermes could exit 1 because of a JVM it never needed.

Problem and expected behavior

Observed in main CI run 39702707721, Test shard 3/8 (Hermes custom sticky-profile selection preserves the root configuration). The repeat rea setup --client hermes --yes --json returned:

code: cleanup_incomplete  (operation: setup)
captured_output: exit_code 0, java.home = /usr/lib/jvm/temurin-17-jdk-amd64 ...
process_cleanup.reason: "process ownership token could not be read for 1 live process(es):
  environment_errno_EACCES=1; live candidates 3850=environment_errno_EACCES"
  diagnostic: "EACCES: permission denied, open '/proc/3850/environ'"

Java itself exited 0. The token sweep after the probe found a new live process whose environment it could not read. It failed closed, as designed. Expected: setup for a client with no Ghidra configuration does not depend on the host JVM or on unrelated processes on the machine.

The two runs cited in the issue (39703491153, the dry run, and 39705641554, the apply step) failed the same way in the same test family, on the same shard and within the same half hour. Their output was not captured, because the assertions recorded only exitCode. Linking them to this cause is an inference.

Change and scope

src/ghidra/GhidraInstallation.ts: probe Java only when installDir !== null and JAVA_HOME is unset or absolute. This keeps optional-provider checks gated on configuration, as Hopper's demo runtime, ILSpy and JADX already are. With this change, a doctor run with no configured provider starts no host provider process.
tests/acceptance/setup/hermesClientSetup.test.ts: exit-code assertions pass stdout as the message, following the convention in other setup acceptance tests. The inline apply call is bound to applied so it reports too.
Not changed: the process-ownership sweep policy (fail closed for an unreadable related or unclassified process). A maintainer kept that policy in 0e62094b6 for the same class of failure.
Not addressed: an EACCES /proc/<pid>/environ on a same-UID, unrelated process that procfs does not classify as non-dumpable or credential-restricted can still fail cleanup for probes that do run, such as a configured Ghidra. The PID 3850 process could not be identified from the log.

Contribution scope: ordinary fix within existing boundaries; no maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: Ghidra installation inspection (installationCoordinates). It runs before doctor and setup project anything.
Contribution scope fit and linked maintainer decision: not applicable (bug fix inside an existing provider check).
CLI and MCP/tool-catalog contract: unchanged schemas. When Ghidra is unconfigured, doctor/setup no longer fail with a Java probe cleanup_incomplete. Ghidra's unavailable diagnostics report java_version: null and java_home: null (or the configured JAVA_HOME) instead of values from the host JVM.
Provider, bridge, target-format, or platform compatibility: configured or PATH-discovered Ghidra installations still run the Java probe on all platforms.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: removes one host process launch (and its ownership sweep) from setup and doctor when Ghidra is unconfigured.
Generated metadata, package, or installation impact: none.
Launch or attach to a process.

(Now launched in fewer cases.)

Evidence and regression coverage

Tests added or updated:
- src/ghidra/GhidraInstallation.test.ts: "does not run Java without a Ghidra installation" (linux, linux with absolute JAVA_HOME, darwin, win32). Fails 4/4 on the base revision because probeJava is called.
- tests/boundary/cli/setupProviderReadiness.test.ts: "setup and doctor do not execute host Java while Ghidra is unconfigured". A recording java comes first on PATH, then the test runs the compiled rea setup --dry-run and rea doctor. Fails on the base revision: the marker file is written.
Base reproduction or other evidence: the CI JSON above (run 39702707721). Locally on macOS, rea setup --client hermes --dry-run --json with a recording java on PATH showed one java ... -XshowSettings:properties -version invocation before the fix and none after. I did not reproduce the Linux /proc EACCES race locally: no Linux runner was available. 200 standalone suffix dry runs at concurrency 8, and 9 full runs of the Hermes test file, all passed on macOS before and after the change.
User-visible CLI/MCP output: the doctor and setup JSON for an unconfigured Ghidra are unchanged (only the configuration check is shown).
Remaining proof gaps: no Linux run of the new tests outside CI. Not measured: JVM start-up time and memory saved per setup/doctor call (expected to be a few hundred ms of CPU with a JDK on PATH, but this is an estimate).
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- src/ghidra/GhidraInstallation.test.ts with the source fix reverted: 4 new cases failed. With the fix: 97 passed.
npm run test:focused -- tests/boundary/cli/setupProviderReadiness.test.ts with the source fix reverted: new test failed (marker written). With the fix: 3 passed.
npm run test:local -- Ghidra installation/doctor/provider/snapshot-cache tests, src/setupEnvironment.test.ts, src/application/Doctor*.test.ts, src/application/Setup*.test.ts: 12 files, 398 passed.
npm run test:focused -- all 18 tests/acceptance/setup/*.test.ts, tests/boundary/cli/doctor*.test.ts, tests/boundary/cli/setup*.test.ts, tests/boundary/filesystem/ghidraJavaProbe.test.ts, tests/boundary/providers/ghidra/ghidraClient.test.ts and ghidraJavaLaunch.test.ts: 24 files, 254 passed, 1 skipped.
tests/acceptance/setup/hermesClientSetup.test.ts: 6 runs before the change and 3 runs after it (final version), all 15 tests passed each time. After rebasing on latest main, the Hermes file plus the CLI regression: 18 passed.
npm run check:fast: passed. npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS only, locally. No real Ghidra installation was exercised. Linux behavior is covered by CI.
Package or release metadata impact: none.
Security, privacy, process, or containment review: fewer host processes are launched. The process-ownership policy is unchanged.

Author readiness

Human owner: @kaoru0822-kitauji

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 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/6282 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/6200-hermes-sticky-profile-flake` --- ### 📝 Commits (1) - [`81b9f2a`](https://github.com/morluto/rea/commit/81b9f2adb091a64ae40aac1c489c3a01eb241cb1) fix(ghidra): skip the Java probe without an installation ### 📊 Changes **4 files changed** (+126 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/GhidraInstallation.test.ts` (+45 -0) 📝 `src/ghidra/GhidraInstallation.ts` (+5 -2) 📝 `tests/acceptance/setup/hermesClientSetup.test.ts` (+16 -12) 📝 `tests/boundary/cli/setupProviderReadiness.test.ts` (+60 -0) </details> ### 📄 Description **What broke:** `rea setup` and `rea doctor` ran the host `java` from PATH even when Ghidra was not configured. When that probe's owned-process cleanup failed, setup exited 1 with `cleanup_incomplete`. This made the Hermes setup acceptance tests fail at random on Linux CI. **Fix:** The Ghidra inspection now runs Java only when an installation is selected (GHIDRA_INSTALL_DIR, or a Ghidra launcher found on PATH). This is the same rule that already applies to a relative JAVA_HOME. Hermes test exit-code assertions now print the CLI JSON when they fail. **Risk:** Low. An unconfigured Ghidra still reports only the `not_configured` configuration check, as before. Its unavailable diagnostics now show `java_version`/`java_home` as null instead of facts from the host JVM. No process-ownership policy changed. ## Summary Fixes #6200 Setup and doctor inspect every deep-analysis provider. For Ghidra, `inspectGhidraInstallation` still ran `java -XshowSettings:properties -version` when no installation directory was set or found. Nothing used the result: the configuration check fails first, and doctor shows only that check. But a failure while the probe was cleaned up still aborted the whole command. So `rea setup --client hermes` could exit 1 because of a JVM it never needed. ## Problem and expected behavior Observed in main CI run [39702707721](https://github.com/morluto/rea/actions/runs/39702707721), Test shard 3/8 (`Hermes custom sticky-profile selection preserves the root configuration`). The repeat `rea setup --client hermes --yes --json` returned: ``` code: cleanup_incomplete (operation: setup) captured_output: exit_code 0, java.home = /usr/lib/jvm/temurin-17-jdk-amd64 ... process_cleanup.reason: "process ownership token could not be read for 1 live process(es): environment_errno_EACCES=1; live candidates 3850=environment_errno_EACCES" diagnostic: "EACCES: permission denied, open '/proc/3850/environ'" ``` Java itself exited 0. The token sweep after the probe found a new live process whose environment it could not read. It failed closed, as designed. Expected: setup for a client with no Ghidra configuration does not depend on the host JVM or on unrelated processes on the machine. The two runs cited in the issue ([39703491153](https://github.com/morluto/rea/actions/runs/39703491153), the dry run, and [39705641554](https://github.com/morluto/rea/actions/runs/39705641554), the apply step) failed the same way in the same test family, on the same shard and within the same half hour. Their output was not captured, because the assertions recorded only `exitCode`. Linking them to this cause is an inference. ## Change and scope - `src/ghidra/GhidraInstallation.ts`: probe Java only when `installDir !== null` and JAVA_HOME is unset or absolute. This keeps optional-provider checks gated on configuration, as Hopper's demo runtime, ILSpy and JADX already are. With this change, a doctor run with no configured provider starts no host provider process. - `tests/acceptance/setup/hermesClientSetup.test.ts`: exit-code assertions pass `stdout` as the message, following the convention in other setup acceptance tests. The inline apply call is bound to `applied` so it reports too. - Not changed: the process-ownership sweep policy (fail closed for an unreadable related or unclassified process). A maintainer kept that policy in 0e62094b6 for the same class of failure. - Not addressed: an EACCES `/proc/<pid>/environ` on a same-UID, unrelated process that procfs does not classify as non-dumpable or credential-restricted can still fail cleanup for probes that do run, such as a configured Ghidra. The PID 3850 process could not be identified from the log. Contribution scope: ordinary fix within existing boundaries; no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: Ghidra installation inspection (`installationCoordinates`). It runs before doctor and setup project anything. - Contribution scope fit and linked maintainer decision: not applicable (bug fix inside an existing provider check). - CLI and MCP/tool-catalog contract: unchanged schemas. When Ghidra is unconfigured, doctor/setup no longer fail with a Java probe `cleanup_incomplete`. Ghidra's unavailable diagnostics report `java_version: null` and `java_home: null` (or the configured JAVA_HOME) instead of values from the host JVM. - Provider, bridge, target-format, or platform compatibility: configured or PATH-discovered Ghidra installations still run the Java probe on all platforms. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: removes one host process launch (and its ownership sweep) from setup and doctor when Ghidra is unconfigured. - Generated metadata, package, or installation impact: none. - [x] Launch or attach to a process. (Now launched in fewer cases.) ## Evidence and regression coverage - Tests added or updated: - `src/ghidra/GhidraInstallation.test.ts`: "does not run Java without a Ghidra installation" (linux, linux with absolute JAVA_HOME, darwin, win32). Fails 4/4 on the base revision because `probeJava` is called. - `tests/boundary/cli/setupProviderReadiness.test.ts`: "setup and doctor do not execute host Java while Ghidra is unconfigured". A recording `java` comes first on PATH, then the test runs the compiled `rea setup --dry-run` and `rea doctor`. Fails on the base revision: the marker file is written. - Base reproduction or other evidence: the CI JSON above (run 39702707721). Locally on macOS, `rea setup --client hermes --dry-run --json` with a recording `java` on PATH showed one `java ... -XshowSettings:properties -version` invocation before the fix and none after. I did not reproduce the Linux `/proc` EACCES race locally: no Linux runner was available. 200 standalone suffix dry runs at concurrency 8, and 9 full runs of the Hermes test file, all passed on macOS before and after the change. - User-visible CLI/MCP output: the doctor and setup JSON for an unconfigured Ghidra are unchanged (only the `configuration` check is shown). - Remaining proof gaps: no Linux run of the new tests outside CI. Not measured: JVM start-up time and memory saved per setup/doctor call (expected to be a few hundred ms of CPU with a JDK on PATH, but this is an estimate). - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/ghidra/GhidraInstallation.test.ts` with the source fix reverted: 4 new cases failed. With the fix: 97 passed. - `npm run test:focused -- tests/boundary/cli/setupProviderReadiness.test.ts` with the source fix reverted: new test failed (marker written). With the fix: 3 passed. - `npm run test:local --` Ghidra installation/doctor/provider/snapshot-cache tests, `src/setupEnvironment.test.ts`, `src/application/Doctor*.test.ts`, `src/application/Setup*.test.ts`: 12 files, 398 passed. - `npm run test:focused --` all 18 `tests/acceptance/setup/*.test.ts`, `tests/boundary/cli/doctor*.test.ts`, `tests/boundary/cli/setup*.test.ts`, `tests/boundary/filesystem/ghidraJavaProbe.test.ts`, `tests/boundary/providers/ghidra/ghidraClient.test.ts` and `ghidraJavaLaunch.test.ts`: 24 files, 254 passed, 1 skipped. - `tests/acceptance/setup/hermesClientSetup.test.ts`: 6 runs before the change and 3 runs after it (final version), all 15 tests passed each time. After rebasing on latest main, the Hermes file plus the CLI regression: 18 passed. - `npm run check:fast`: passed. `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS only, locally. No real Ghidra installation was exercised. Linux behavior is covered by CI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: fewer host processes are launched. The process-ownership policy is unchanged. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 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-11-01 21:46:30 +00:00