morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #5862] [MERGED] fix(android): report a rejected relative JADX JAR instead of an unset one #7091

New issue

Closed

opened 2026-11-01 21:45:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:19 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/jadx-jar-rejected-selection

📝 Commits (2)

eaee2df fix(android): report a rejected relative JADX JAR instead of an unset one
68c7a60 Merge remote-tracking branch 'origin/main' into fix/jadx-jar-rejected-selection

📊 Changes

3 files changed (+90 additions, -1 deletions)

View changed files

📝 src/android/JadxConfiguration.ts (+10 -1)
📝 tests/acceptance/nonBinaryComposition.test.ts (+55 -0)
📝 tests/boundary/android/jadxDoctor.test.ts (+25 -0)

📄 Description

What broke: a supplied but non-absolute REA_JADX_MCP_JAR was reported as unset (not_configured, jar_configured: false). The rejected value was dropped, so the output was byte-identical to an unset variable.
Fix: resolveJar now separates unset (not_configured) from supplied but rejected (open_options_invalid, jar_configured: true, the exact jar_path, path_requirement: "absolute"). CLI, MCP and doctor all build their output from that one result.
Risk: low. The change is limited to one JADX admission branch, and a relative path is still refused. The visible changes are the failure_code and message for a supplied non-absolute value, including an empty string.

Summary

Fixes #5844.

Callers could not tell an omitted JADX JAR setting from a supplied value that failed validation, and they could not see what REA rejected. That loses the caller-selected input and the failure reason, which AGENTS.md asks REA to keep. Now the unset case stays distinct, and a rejected selection keeps its value and names the absolute-path constraint it failed.

Problem and expected behavior

Smallest trigger: REA_JADX_MCP_JAR=./tools/jadx.jar rea inspect-android-package x.apk. On main, resolveJar checked jar === undefined || !isAbsolute(jar) in one condition and threw not_configured with { jar_configured: false }. The broken rule: a supplied value must not be treated as absent. Doctor already counted the setting as configured: true (it checks !== undefined) but reported code: "not_configured", so its own output contradicted itself.

Expected: relative paths are still refused. The refusal keeps the selection and reports the failed requirement, and unset remains its own condition.

Change and scope

src/android/JadxConfiguration.ts (resolveJar, which owns parsing of this setting): unset stays not_configured / { jar_configured: false }. Any supplied non-absolute value fails with open_options_invalid, { jar_configured: true, jar_path, path_requirement: "absolute" }, and the message REA_JADX_MCP_JAR must be an absolute path; received "<value>". <existing remediation>. This follows the existing JAVA_HOME non-absolute branch in the same file. An empty string counts as a supplied value, which matches how doctor already computes configured.
Consumers need no changes. They already project the boundary failure: projectConfigurationFailure (CLI/MCP), JadxDoctor (kind jar with configured → config_drift, existing JAR remediation) and CapabilityInventory (still provider_missing, now with the precise reason).
Sibling settings: JAVA_HOME, REA_JADX_HEAP_MIB, REA_JADX_ACTIVE_PROCESSOR_COUNT and REA_JADX_OPERATION_TIMEOUT_MS in the same parser already keep the rejected value. Other providers' path settings (for example REA_TSHARK_COMMAND in PcapConfiguration) use their own parsers and already echo the rejected value. There is no shared helper with this gap.
Not included: accepting relative paths (the issue does not ask for it).

Follow the contribution scope. Ordinary fix within an existing boundary; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: resolveJar in the JADX configuration boundary, provider admission, before Java selection or engine launch.
Contribution scope fit and linked maintainer decision: existing Android/JADX admission; not applicable.
CLI and MCP/tool-catalog contract: same capability_unavailable error. For a supplied non-absolute JAR, details.diagnostics now has failure_code: "open_options_invalid", jar_configured: true, jar_path and path_requirement, and the message gains the rejected value. No schema or catalog change.
Provider, bridge, target-format, or platform compatibility: none. isAbsolute semantics are unchanged per platform.
Evidence, artifact, provenance, or reconstruction contract: the caller-selected value is now preserved verbatim in diagnostics.
Target selection, authority, upstream behavior, and cleanup details: none. The refusal still happens before any file access, Java probe or launch.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added:
- tests/acceptance/nonBinaryComposition.test.ts: compiled CLI plus MCP factory parity for relative, empty and unset REA_JADX_MCP_JAR, with exact diagnostics for each.
- tests/boundary/android/jadxDoctor.test.ts: doctor reports a relative JAR as configured: true, open_options_invalid, config_drift, with the rejected value in detail.
- Both new tests failed with the source fix stashed (2 failed / 24 passed) and pass with it.
Base reproduction: the issue's portable script against the real built CLI in an isolated HOME/XDG/TMPDIR scratch directory, with a synthetic ZIP named .apk and no JAR or JDK.
User-visible CLI output (inspect-android-package --format json, details.diagnostics plus message):

Before (main f801b2fbf):

relative: {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"}
empty:    {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"}
unset:    {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"}
relative_vs_unset_byte_identical=true

After:

relative: message "REA_JADX_MCP_JAR must be an absolute path; received \"./tools/jadx-headless-mcp-0.7.1-all.jar\". Set REA_JADX_MCP_JAR to the absolute path of ..."
          {"jar_configured": true, "jar_path": "./tools/jadx-headless-mcp-0.7.1-all.jar", "path_requirement": "absolute", "failure_kind": "jar", "failure_code": "open_options_invalid"}
empty:    {"jar_configured": true, "jar_path": "", "path_requirement": "absolute", "failure_kind": "jar", "failure_code": "open_options_invalid"}
unset:    {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"}
relative_vs_unset_byte_identical=false

After, rea doctor --provider jadx --format json with REA_JADX_MCP_JAR=./tools/jadx.jar:

{"id":"jadx","configured":true,"available":false,"registrationEnvironment":{},"checks":[{"name":"configuration","ok":false,"code":"open_options_invalid","detail":"REA_JADX_MCP_JAR must be an absolute path; received \"./tools/jadx.jar\". Set REA_JADX_MCP_JAR to ...","remediation":"Set REA_JADX_MCP_JAR to the absolute path of ...","classification":"config_drift"}]}

Remaining proof gaps:
- Doctor CLI output "before" is source-derived. The new doctor boundary test fails on the unfixed source, but I did not capture real doctor output on main.
- Not run: Windows, Linux, and a successful real-engine run (no JAR or JDK used).

For evidence-bearing changes:

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

Local runs on macOS arm64, Node v26:

Issue repro script on the real CLI: before, byte-identical relative/unset; after, distinct (output above).
rea doctor --provider jadx --format json with a relative JAR: output above.
VITEST_MAX_WORKERS=2 npm run test:focused -- tests/acceptance/nonBinaryComposition.test.ts tests/boundary/android/jadxDoctor.test.ts tests/boundary/android/jadxConfiguration.test.ts: 42 passed, 2 skipped (platform-gated).
The same two new tests with the fix stashed: 2 failed, 24 passed.
VITEST_MAX_WORKERS=2 npm run test:focused -- tests/boundary/process/jadxIntegration.test.ts src/doctorRuntime.test.ts tests/boundary/cli/doctorCancellation.test.ts src/application/CapabilityInventory.test.ts: 92 passed.
npm run check:fast: passed.
npm run format:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers matching failure_code: "not_configured" for a supplied non-absolute JAR now get open_options_invalid, the same code JAVA_HOME already uses for this condition. Unset behavior is unchanged.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 CLI only; no JADX engine run.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the echoed value is the caller's own configuration path, already echoed by the other JAR failure messages. No new file access or process launch.

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/5862 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/jadx-jar-rejected-selection` --- ### 📝 Commits (2) - [`eaee2df`](https://github.com/morluto/rea/commit/eaee2df0b7eaaef7d9ce3e122205b9f95cac2b87) fix(android): report a rejected relative JADX JAR instead of an unset one - [`68c7a60`](https://github.com/morluto/rea/commit/68c7a60bfdce142f16763b3712b15a7648e5b963) Merge remote-tracking branch 'origin/main' into fix/jadx-jar-rejected-selection ### 📊 Changes **3 files changed** (+90 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/android/JadxConfiguration.ts` (+10 -1) 📝 `tests/acceptance/nonBinaryComposition.test.ts` (+55 -0) 📝 `tests/boundary/android/jadxDoctor.test.ts` (+25 -0) </details> ### 📄 Description **What broke:** a supplied but non-absolute `REA_JADX_MCP_JAR` was reported as unset (`not_configured`, `jar_configured: false`). The rejected value was dropped, so the output was byte-identical to an unset variable. **Fix:** `resolveJar` now separates unset (`not_configured`) from supplied but rejected (`open_options_invalid`, `jar_configured: true`, the exact `jar_path`, `path_requirement: "absolute"`). CLI, MCP and doctor all build their output from that one result. **Risk:** low. The change is limited to one JADX admission branch, and a relative path is still refused. The visible changes are the `failure_code` and message for a supplied non-absolute value, including an empty string. ## Summary Fixes #5844. Callers could not tell an omitted JADX JAR setting from a supplied value that failed validation, and they could not see what REA rejected. That loses the caller-selected input and the failure reason, which AGENTS.md asks REA to keep. Now the unset case stays distinct, and a rejected selection keeps its value and names the absolute-path constraint it failed. ## Problem and expected behavior Smallest trigger: `REA_JADX_MCP_JAR=./tools/jadx.jar rea inspect-android-package x.apk`. On main, `resolveJar` checked `jar === undefined || !isAbsolute(jar)` in one condition and threw `not_configured` with `{ jar_configured: false }`. The broken rule: a supplied value must not be treated as absent. Doctor already counted the setting as `configured: true` (it checks `!== undefined`) but reported `code: "not_configured"`, so its own output contradicted itself. Expected: relative paths are still refused. The refusal keeps the selection and reports the failed requirement, and unset remains its own condition. ## Change and scope - `src/android/JadxConfiguration.ts` (`resolveJar`, which owns parsing of this setting): unset stays `not_configured` / `{ jar_configured: false }`. Any supplied non-absolute value fails with `open_options_invalid`, `{ jar_configured: true, jar_path, path_requirement: "absolute" }`, and the message `REA_JADX_MCP_JAR must be an absolute path; received "<value>". <existing remediation>`. This follows the existing `JAVA_HOME` non-absolute branch in the same file. An empty string counts as a supplied value, which matches how doctor already computes `configured`. - Consumers need no changes. They already project the boundary failure: `projectConfigurationFailure` (CLI/MCP), `JadxDoctor` (kind `jar` with configured → `config_drift`, existing JAR remediation) and `CapabilityInventory` (still `provider_missing`, now with the precise reason). - Sibling settings: `JAVA_HOME`, `REA_JADX_HEAP_MIB`, `REA_JADX_ACTIVE_PROCESSOR_COUNT` and `REA_JADX_OPERATION_TIMEOUT_MS` in the same parser already keep the rejected value. Other providers' path settings (for example `REA_TSHARK_COMMAND` in `PcapConfiguration`) use their own parsers and already echo the rejected value. There is no shared helper with this gap. - Not included: accepting relative paths (the issue does not ask for it). Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Ordinary fix within an existing boundary; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `resolveJar` in the JADX configuration boundary, provider admission, before Java selection or engine launch. - Contribution scope fit and linked maintainer decision: existing Android/JADX admission; not applicable. - CLI and MCP/tool-catalog contract: same `capability_unavailable` error. For a supplied non-absolute JAR, `details.diagnostics` now has `failure_code: "open_options_invalid"`, `jar_configured: true`, `jar_path` and `path_requirement`, and the message gains the rejected value. No schema or catalog change. - Provider, bridge, target-format, or platform compatibility: none. `isAbsolute` semantics are unchanged per platform. - Evidence, artifact, provenance, or reconstruction contract: the caller-selected value is now preserved verbatim in diagnostics. - Target selection, authority, upstream behavior, and cleanup details: none. The refusal still happens before any file access, Java probe or launch. - Generated metadata, package, or installation impact: none. Expected effects: - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added: - `tests/acceptance/nonBinaryComposition.test.ts`: compiled CLI plus MCP factory parity for relative, empty and unset `REA_JADX_MCP_JAR`, with exact diagnostics for each. - `tests/boundary/android/jadxDoctor.test.ts`: doctor reports a relative JAR as `configured: true`, `open_options_invalid`, `config_drift`, with the rejected value in `detail`. - Both new tests failed with the source fix stashed (2 failed / 24 passed) and pass with it. - Base reproduction: the issue's portable script against the real built CLI in an isolated HOME/XDG/TMPDIR scratch directory, with a synthetic ZIP named `.apk` and no JAR or JDK. - User-visible CLI output (`inspect-android-package --format json`, `details.diagnostics` plus message): Before (main `f801b2fbf`): ```text relative: {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"} empty: {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"} unset: {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"} relative_vs_unset_byte_identical=true ``` After: ```text relative: message "REA_JADX_MCP_JAR must be an absolute path; received \"./tools/jadx-headless-mcp-0.7.1-all.jar\". Set REA_JADX_MCP_JAR to the absolute path of ..." {"jar_configured": true, "jar_path": "./tools/jadx-headless-mcp-0.7.1-all.jar", "path_requirement": "absolute", "failure_kind": "jar", "failure_code": "open_options_invalid"} empty: {"jar_configured": true, "jar_path": "", "path_requirement": "absolute", "failure_kind": "jar", "failure_code": "open_options_invalid"} unset: {"jar_configured": false, "failure_kind": "jar", "failure_code": "not_configured"} relative_vs_unset_byte_identical=false ``` After, `rea doctor --provider jadx --format json` with `REA_JADX_MCP_JAR=./tools/jadx.jar`: ```json {"id":"jadx","configured":true,"available":false,"registrationEnvironment":{},"checks":[{"name":"configuration","ok":false,"code":"open_options_invalid","detail":"REA_JADX_MCP_JAR must be an absolute path; received \"./tools/jadx.jar\". Set REA_JADX_MCP_JAR to ...","remediation":"Set REA_JADX_MCP_JAR to the absolute path of ...","classification":"config_drift"}]} ``` - Remaining proof gaps: - Doctor CLI output "before" is source-derived. The new doctor boundary test fails on the unfixed source, but I did not capture real doctor output on main. - Not run: Windows, Linux, and a successful real-engine run (no JAR or JDK used). For evidence-bearing changes: - [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 Local runs on macOS arm64, Node v26: - Issue repro script on the real CLI: before, byte-identical relative/unset; after, distinct (output above). - `rea doctor --provider jadx --format json` with a relative JAR: output above. - `VITEST_MAX_WORKERS=2 npm run test:focused -- tests/acceptance/nonBinaryComposition.test.ts tests/boundary/android/jadxDoctor.test.ts tests/boundary/android/jadxConfiguration.test.ts`: 42 passed, 2 skipped (platform-gated). - The same two new tests with the fix stashed: 2 failed, 24 passed. - `VITEST_MAX_WORKERS=2 npm run test:focused -- tests/boundary/process/jadxIntegration.test.ts src/doctorRuntime.test.ts tests/boundary/cli/doctorCancellation.test.ts src/application/CapabilityInventory.test.ts`: 92 passed. - `npm run check:fast`: passed. - `npm run format:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers matching `failure_code: "not_configured"` for a supplied non-absolute JAR now get `open_options_invalid`, the same code `JAVA_HOME` already uses for this condition. Unset behavior is unchanged. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 CLI only; no JADX engine run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the echoed value is the caller's own configuration path, already echoed by the other JAR failure messages. No new file access or process launch. ## 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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] 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:45:19 +00:00