morluto/rea

Fork 0

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

[GH-ISSUE #1441] [Bug] JADX cleanup-failure fixture loses the reason for incomplete teardown #489

New issue

Closed

opened 2026-10-15 18:25:30 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-15 18:25:30 +00:00

Owner

Originally created by @N0zoM1z0 on GitHub (Oct 10, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/1441

The compiled CI shard failed teardown of retains partial JADX replies when owned cleanup also fails in tests/boundary/process/androidAnalysisMcp.test.ts. The actual owned subprocess fixture returns an incomplete stop disposition, but its cleanup hook asserts only .status, discarding the reason needed to diagnose the failed ownership cleanup.

Searched open/closed issues and PRs for the fixture and cleanup failure.
No credentials, private host paths or private target source included.

Observed evidence: original failed job, at head c550c127, failed at tests/fixtures/android/jadx.ts:77 with expected 'incomplete' not to be 'incomplete'. The failure occurred after the expected partial-observation assertions; the owned teardown result is the failing boundary. The same-head rerun succeeded and #1429 subsequently merged. That distinguishes this failure from the documentation change, but does not establish its cleanup cause.

Local scope: sixteen serial repetitions of the original cleanup-failure process-boundary case passed at frozen main 0b9815b6, with two CPUs, a 768 MiB caller heap and a 2 GiB sampled process-family guard. The local diagnostic assertion retains the entire typed stop result. No actual JADX engine is used: this is a synthetic protocol producer running through the production owned-process seam. Its passing repetitions do not reproduce the CI failure or certify native JADX behavior.

Expected follow-up:

Retain the full ProviderProcessStopResult in the teardown assertion so another failure exposes the meaningful reason.
Reproduce the failing lifecycle/ownership observation under controlled conditions and fix the owning fixture or process boundary as the evidence warrants. Child exit, protocol closure and verified group cleanup are separate facts.
Keep ownership verification, partial-observation coverage and failed-cleanup retention active. A successful retry or changed assertion alone must not be claimed as a proven root-cause fix.

Refs #1065; related ownership work #1011. Root cause remains unverified.

Originally created by @N0zoM1z0 on GitHub (Oct 10, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/1441 The compiled CI shard failed teardown of `retains partial JADX replies when owned cleanup also fails` in `tests/boundary/process/androidAnalysisMcp.test.ts`. The actual owned subprocess fixture returns an incomplete stop disposition, but its cleanup hook asserts only `.status`, discarding the reason needed to diagnose the failed ownership cleanup. - [x] Searched open/closed issues and PRs for the fixture and cleanup failure. - [x] No credentials, private host paths or private target source included. Observed evidence: [original failed job](https://github.com/morluto/rea/actions/runs/38023755847/job/114130328487), at head `c550c127`, failed at `tests/fixtures/android/jadx.ts:77` with `expected 'incomplete' not to be 'incomplete'`. The failure occurred after the expected partial-observation assertions; the owned teardown result is the failing boundary. The [same-head rerun](https://github.com/morluto/rea/actions/runs/38023755847/job/114132236592) succeeded and #1429 subsequently merged. That distinguishes this failure from the documentation change, but does not establish its cleanup cause. Local scope: sixteen serial repetitions of the original cleanup-failure process-boundary case passed at frozen main `0b9815b6`, with two CPUs, a 768 MiB caller heap and a 2 GiB sampled process-family guard. The local diagnostic assertion retains the entire typed stop result. No actual JADX engine is used: this is a synthetic protocol producer running through the production owned-process seam. Its passing repetitions do not reproduce the CI failure or certify native JADX behavior. Expected follow-up: 1. Retain the full `ProviderProcessStopResult` in the teardown assertion so another failure exposes the meaningful reason. 2. Reproduce the failing lifecycle/ownership observation under controlled conditions and fix the owning fixture or process boundary as the evidence warrants. Child exit, protocol closure and verified group cleanup are separate facts. 3. Keep ownership verification, partial-observation coverage and failed-cleanup retention active. A successful retry or changed assertion alone must not be claimed as a proven root-cause fix. Refs #1065; related ownership work #1011. Root cause remains unverified.

bsakaguchi

2026-10-15 18:25:30 +00:00

closed this issue
added the
triage

bug

area: tests
labels

bsakaguchi commented

2026-10-15 18:25:31 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 10, 2026):

The diagnostic follow-up #1443 is now merged as 4e3d3354397374116577555200319975fc2ff86e after 19 successful checks on its exact final head. The fixture includes the complete typed stop disposition in the assertion message; a controlled Vitest failure-output probe verifies the reason survives reporting.

The original cleanup failure remains unreproduced: sixteen serial repetitions on frozen main passed, and the current seven process-boundary cases pass. This uses a synthetic JADX protocol producer and does not certify a real JADX engine. No retry, skip or relaxed cleanup requirement was added. Keeping this issue open for the root cause; a recurrence should now retain the actionable stop reason.

@N0zoM1z0 commented on GitHub (Oct 10, 2026): The diagnostic follow-up #1443 is now merged as `4e3d3354397374116577555200319975fc2ff86e` after 19 successful checks on its exact final head. The fixture includes the complete typed stop disposition in the assertion message; a controlled Vitest failure-output probe verifies the reason survives reporting. The original cleanup failure remains unreproduced: sixteen serial repetitions on frozen main passed, and the current seven process-boundary cases pass. This uses a synthetic JADX protocol producer and does not certify a real JADX engine. No retry, skip or relaxed cleanup requirement was added. Keeping this issue open for the root cause; a recurrence should now retain the actionable stop reason.

bsakaguchi commented

2026-10-15 18:25:31 +00:00

Author

Owner

@CassieFox23 commented on GitHub (Oct 10, 2026):

TL;DR: I ran REA's real Java engine path on Linux 100 times, and teardown finished every time. I also forced a failed cleanup, and REA reported the reason. I could not make the CI failure appear, so the root cause is still unknown.

I used the production path, not the test fixture. The code was main at 6db97d516, the latest commit when I started. The engine jar was jadx-headless-mcp 0.7.1 (Apache-2.0). Its SHA-256 matches REA's audited release. The Java runtime was Temurin 21.0.12.1. The APK was Coffee 2.27 (Apache-2.0). I only analysed it and never ran it. A heavy test job ran on the same machine the whole time. The load average is the count of processes running or waiting for a CPU, over one minute, out of 24 threads. It stayed between 3.9 and 9.1.

Results

Check	Runs	Result
`rea inspect-android-package` and four other APK commands, real engine	30	All exit 0. No cleanup message. The Java process group was gone 1 s after exit. No workspace left.
Same, with `strace` watching `kill`	10	All exit 0. REA sent SIGKILL to the Java process group each time.
Provider in one process, normal finish, stop result logged	30	`verified-cleanup` 30 of 30, in 68 to 84 ms.
Same, with failing requests: 10 missing class, 16 cancelled, 4 finished first	30	`verified-cleanup` 30 of 30, in 62 to 211 ms.
Forced failure: cleanup made to fail	4 cases	Each gave `cleanup_incomplete` with the reason. Two also kept the partial replies. A later close then cleaned up.
Existing process-boundary tests (40 tests) and transport tests (4), Node 24.18.0	5	All pass. The fixture's stop poll (the retry from `2596f8da1`) returned `incomplete` 0 of 50 times, 0 of 5 for the case that failed in CI.

Forced failure. I wrapped spawnOwnedProviderProcess at the launcher hook that production code already has. Only my own Java processes were touched. In the first case, a real "class not found" error came back with three engine replies. Then I made cleanup fail. REA returned cleanup_incomplete and kept the replies. In REA's real error output, the stop reason sits at details.diagnostics.provider_cleanup.reason. The field details.cleanup_reason is not present. The field details.diagnostics.reason holds only a generic line: "Provider jadx adapter failed during close_binary". In another case, cleanup claimed success while the Java process stayed alive. After about 2 s the reason was "verified process-group cleanup did not stop the launcher". A retry then cleaned up.

Not run

JDK 25 (not installed here), and macOS and Windows.
A cleanup failure that REA produces on its own. I made all four failures happen. They show how REA reports a failure, not what caused the CI failure.
Heavier load than a 1-minute average of about 9 on 24 threads.
The CI runner itself.
The one-shot CLI with a failed cleanup. I only forced failures through the provider, so I did not check what the CLI prints or its exit code.

Where the reason lives in the code

The function ProviderProcessSupervisor.stop() in src/process/ProviderProcess.ts gives back { status: "incomplete", reason } when REA cannot prove cleanup. Then JadxMcpTransport.close() throws a ProviderCleanupError. After that, JadxProvider wraps the error and keeps the partial replies.

Commands

# build, with HOME, npm cache and logs under /tmp/p1441
git archive refs/heads/main | tar -x -C /tmp/p1441/main
npm ci && npm run build

# every REA run
env -i HOME=/tmp/p1441/home PATH=<node 24.18.0>:/usr/bin:/bin TMPDIR=/tmp/p1441/tmp \
  JAVA_HOME=<Temurin 21.0.12.1> REA_JADX_MCP_JAR=<engine jar 0.7.1> \
  nice -n 10 timeout 300 node scripts/rea.mjs inspect-android-package <apk>

# test repeats, with a copy of tests/fixtures/android/jadx.ts that also logs each poll
node node_modules/vitest/vitest.mjs run --project process-boundary \
  tests/boundary/process/androidAnalysisMcp.test.ts tests/boundary/process/jadxIntegration.test.ts
node node_modules/vitest/vitest.mjs run --project boundary tests/boundary/android/jadxMcpTransport.test.ts

Test setup

Box: Nobara 44 (Fedora 44 base), Linux 7.2.9
CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
RAM: 32 GB DDR4-3600
Node: 24.18.0
Head / base: 6db97d516 / none (I tested main itself)
Other: Temurin 21.0.12.1 (GPL-2.0-only WITH Classpath-exception-2.0, per its licence file)
Other: Coffee 2.27 (Apache-2.0)
APK SHA-256: ae4688fe48e751517e7a1d56aed27bff10b87daac0d7e28993fb193a7dc8f223

🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

@CassieFox23 commented on GitHub (Oct 10, 2026): **TL;DR:** I ran REA's real Java engine path on Linux 100 times, and teardown finished every time. I also forced a failed cleanup, and REA reported the reason. I could not make the CI failure appear, so the root cause is still unknown. I used the production path, not the test fixture. The code was main at 6db97d516, the latest commit when I started. The engine jar was [jadx-headless-mcp](https://github.com/1013503897/jadx-headless-mcp) 0.7.1 ([Apache-2.0](https://github.com/1013503897/jadx-headless-mcp/blob/main/LICENSE)). Its SHA-256 matches REA's audited release. The Java runtime was Temurin 21.0.12.1. The APK was Coffee 2.27 ([Apache-2.0](https://github.com/mueller-ma/Coffee/blob/main/LICENSE)). I only analysed it and never ran it. A heavy test job ran on the same machine the whole time. The load average is the count of processes running or waiting for a CPU, over one minute, out of 24 threads. It stayed between 3.9 and 9.1. ### Results | Check | Runs | Result | | --- | --- | --- | | `rea inspect-android-package` and four other APK commands, real engine | 30 | All exit 0. No cleanup message. The Java process group was gone 1 s after exit. No workspace left. | | Same, with `strace` watching `kill` | 10 | All exit 0. REA sent SIGKILL to the Java process group each time. | | Provider in one process, normal finish, stop result logged | 30 | `verified-cleanup` 30 of 30, in 68 to 84 ms. | | Same, with failing requests: 10 missing class, 16 cancelled, 4 finished first | 30 | `verified-cleanup` 30 of 30, in 62 to 211 ms. | | Forced failure: cleanup made to fail | 4 cases | Each gave `cleanup_incomplete` with the reason. Two also kept the partial replies. A later close then cleaned up. | | Existing process-boundary tests (40 tests) and transport tests (4), Node 24.18.0 | 5 | All pass. The fixture's stop poll (the retry from 2596f8da1) returned `incomplete` 0 of 50 times, 0 of 5 for the case that failed in CI. | **Forced failure.** I wrapped `spawnOwnedProviderProcess` at the launcher hook that production code already has. Only my own Java processes were touched. In the first case, a real "class not found" error came back with three engine replies. Then I made cleanup fail. REA returned `cleanup_incomplete` and kept the replies. In REA's real error output, the stop reason sits at `details.diagnostics.provider_cleanup.reason`. The field `details.cleanup_reason` is not present. The field `details.diagnostics.reason` holds only a generic line: "Provider jadx adapter failed during close_binary". In another case, cleanup claimed success while the Java process stayed alive. After about 2 s the reason was "verified process-group cleanup did not stop the launcher". A retry then cleaned up. ### Not run - JDK 25 (not installed here), and macOS and Windows. - A cleanup failure that REA produces on its own. I made all four failures happen. They show how REA reports a failure, not what caused the CI failure. - Heavier load than a 1-minute average of about 9 on 24 threads. - The CI runner itself. - The one-shot CLI with a failed cleanup. I only forced failures through the provider, so I did not check what the CLI prints or its exit code. <details> <summary>Where the reason lives in the code</summary> The function `ProviderProcessSupervisor.stop()` in `src/process/ProviderProcess.ts` gives back `{ status: "incomplete", reason }` when REA cannot prove cleanup. Then `JadxMcpTransport.close()` throws a `ProviderCleanupError`. After that, `JadxProvider` wraps the error and keeps the partial replies. </details> <details> <summary>Commands</summary> ```sh # build, with HOME, npm cache and logs under /tmp/p1441 git archive refs/heads/main | tar -x -C /tmp/p1441/main npm ci && npm run build # every REA run env -i HOME=/tmp/p1441/home PATH=<node 24.18.0>:/usr/bin:/bin TMPDIR=/tmp/p1441/tmp \ JAVA_HOME=<Temurin 21.0.12.1> REA_JADX_MCP_JAR=<engine jar 0.7.1> \ nice -n 10 timeout 300 node scripts/rea.mjs inspect-android-package <apk> # test repeats, with a copy of tests/fixtures/android/jadx.ts that also logs each poll node node_modules/vitest/vitest.mjs run --project process-boundary \ tests/boundary/process/androidAnalysisMcp.test.ts tests/boundary/process/jadxIntegration.test.ts node node_modules/vitest/vitest.mjs run --project boundary tests/boundary/android/jadxMcpTransport.test.ts ``` </details> <details> <summary>Test setup</summary> - Box: Nobara 44 (Fedora 44 base), Linux 7.2.9 - CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads - RAM: 32 GB DDR4-3600 - Node: 24.18.0 - Head / base: 6db97d516 / none (I tested main itself) - Other: Temurin 21.0.12.1 (`GPL-2.0-only WITH Classpath-exception-2.0`, per its licence file) - Other: Coffee 2.27 ([Apache-2.0](https://github.com/mueller-ma/Coffee/blob/main/LICENSE)) - APK SHA-256: `ae4688fe48e751517e7a1d56aed27bff10b87daac0d7e28993fb193a7dc8f223` </details> 🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

bsakaguchi commented

2026-10-15 18:25:31 +00:00

Author

Owner

@CassieFox23 commented on GitHub (Oct 10, 2026):

TL;DR: With a forced failed teardown, the test now prints the reason, and before the change it did not. Ten more real-engine runs all cleaned up. Why the CI teardown failed is still unknown.

I tested the head of #1639 (daafe0495, merged as afc9d819a (#1639)) against main before it (926c2d6ba). My earlier real-engine results are in this comment.

Results

Check	Result
Forced failure, before the change	The message hides the reason.
Forced failure, with the change	The message names the reason. Both messages are in the raw output below.
Fixture tests (`androidAnalysisMcp`, `jadxIntegration`; 40 tests), plus `jadxConfiguration` (9 tests)	All pass. Ten runs each on the PR head and on main, Node 24.18.0. One run each on Node 22.23.1 and 26.11.1.
Real JADX engine, 10 full lifecycles on the PR head	`verified-cleanup` 10 of 10, in 70 to 83 ms. The Java process group was gone 1 s later.

The change touches only tests/fixtures/android/jadx.ts, so it cannot change how the real engine behaves. The 10 real runs agree.

Not run

A cleanup failure that happens on its own. I forced the one above.
The real engine through the test fixture. The fixture uses a synthetic engine.
The CI runner, JDK 25, macOS and Windows.

The test change only makes the next failure easier to read. What made the CI teardown incomplete is still open, as the merge note on #1639 says.

Thanks, Totopo27, for the fix.

Raw output of the forced failure

Main before (926c2d6ba). The message in this issue's body is from before #1443. Main printed this one:

AssertionError: expected { status: 'incomplete', …(1) } to not match object { status: 'incomplete' }
Compared values have no visual difference.
 ❯ tests/fixtures/android/jadx.ts:89:18
Caused by: Error: Matcher did not succeed in time.

PR head (daafe0495):

Error: Owned process cleanup incomplete: FORCED: ownership verification failed (pr1639 probe)
 ❯ tests/fixtures/android/jadx.ts:100:18
Caused by: Error: Matcher did not succeed in time.

Commands

git archive <sha> | tar -x -C /tmp/p2a-jadx/<tree>
npm ci && npm run build

# forced failure: in both trees, the teardown supervisor's cleanup hook returns
#   { cleaned: false, reason: "FORCED: ownership verification failed (pr1639 probe)" }
vitest run --project process-boundary tests/boundary/process/androidAnalysisMcp.test.ts \
  -t "retains partial JADX replies when owned cleanup also fails"

# repeats
vitest run --project process-boundary tests/boundary/process/androidAnalysisMcp.test.ts tests/boundary/process/jadxIntegration.test.ts
vitest run --project boundary tests/boundary/android/jadxConfiguration.test.ts

Test setup

Box: Nobara 44 (Fedora 44 base), Linux 7.2.9
CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
RAM: 32 GB DDR4-3600
Node: 22.23.1, 24.18.0, 26.11.1
Head / base: daafe0495 / 926c2d6ba (main before the merge)
Other: the engine jar was jadx-headless-mcp, at version 0.7.1, under Apache-2.0
Other: Temurin 21.0.12.1 (GPL-2.0-only WITH Classpath-exception-2.0)
Other: Coffee 2.27 (Apache-2.0)
Load average: 1.0 to 11.3 during the test repeats, and 6.0 to 7.0 during the real-engine runs. Other jobs ran on the box.
Load average counts the processes running or waiting for a CPU, over one minute, out of 24 threads.

🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

@CassieFox23 commented on GitHub (Oct 10, 2026): **TL;DR:** With a forced failed teardown, the test now prints the reason, and before the change it did not. Ten more real-engine runs all cleaned up. Why the CI teardown failed is still unknown. I tested the head of #1639 (daafe0495, merged as afc9d819a (#1639)) against main before it (926c2d6ba). My earlier real-engine results are in [this comment](https://github.com/morluto/rea/issues/1441#issuecomment-6101857535). ### Results | Check | Result | | --- | --- | | Forced failure, before the change | The message hides the reason. | | Forced failure, with the change | The message names the reason. Both messages are in the raw output below. | | Fixture tests (`androidAnalysisMcp`, `jadxIntegration`; 40 tests), plus `jadxConfiguration` (9 tests) | All pass. Ten runs each on the PR head and on main, Node 24.18.0. One run each on Node 22.23.1 and 26.11.1. | | Real JADX engine, 10 full lifecycles on the PR head | `verified-cleanup` 10 of 10, in 70 to 83 ms. The Java process group was gone 1 s later. | The change touches only `tests/fixtures/android/jadx.ts`, so it cannot change how the real engine behaves. The 10 real runs agree. ### Not run - A cleanup failure that happens on its own. I forced the one above. - The real engine through the test fixture. The fixture uses a synthetic engine. - The CI runner, JDK 25, macOS and Windows. The test change only makes the next failure easier to read. What made the CI teardown incomplete is still open, as the merge note on #1639 says. Thanks, Totopo27, for the fix. <details> <summary>Raw output of the forced failure</summary> Main before (926c2d6ba). The message in this issue's body is from before #1443. Main printed this one: ``` AssertionError: expected { status: 'incomplete', …(1) } to not match object { status: 'incomplete' } Compared values have no visual difference. ❯ tests/fixtures/android/jadx.ts:89:18 Caused by: Error: Matcher did not succeed in time. ``` PR head (daafe0495): ``` Error: Owned process cleanup incomplete: FORCED: ownership verification failed (pr1639 probe) ❯ tests/fixtures/android/jadx.ts:100:18 Caused by: Error: Matcher did not succeed in time. ``` </details> <details> <summary>Commands</summary> ```sh git archive <sha> | tar -x -C /tmp/p2a-jadx/<tree> npm ci && npm run build # forced failure: in both trees, the teardown supervisor's cleanup hook returns # { cleaned: false, reason: "FORCED: ownership verification failed (pr1639 probe)" } vitest run --project process-boundary tests/boundary/process/androidAnalysisMcp.test.ts \ -t "retains partial JADX replies when owned cleanup also fails" # repeats vitest run --project process-boundary tests/boundary/process/androidAnalysisMcp.test.ts tests/boundary/process/jadxIntegration.test.ts vitest run --project boundary tests/boundary/android/jadxConfiguration.test.ts ``` </details> <details> <summary>Test setup</summary> - Box: Nobara 44 (Fedora 44 base), Linux 7.2.9 - CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads - RAM: 32 GB DDR4-3600 - Node: 22.23.1, 24.18.0, 26.11.1 - Head / base: daafe0495 / 926c2d6ba (main before the merge) - Other: the engine jar was [jadx-headless-mcp](https://github.com/1013503897/jadx-headless-mcp/blob/main/LICENSE), at version 0.7.1, under Apache-2.0 - Other: Temurin 21.0.12.1 (`GPL-2.0-only WITH Classpath-exception-2.0`) - Other: Coffee 2.27 ([Apache-2.0](https://github.com/mueller-ma/Coffee/blob/main/LICENSE)) - Load average: 1.0 to 11.3 during the test repeats, and 6.0 to 7.0 during the real-engine runs. Other jobs ran on the box. - Load average counts the processes running or waiting for a CPU, over one minute, out of 24 threads. </details> 🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

bsakaguchi referenced this issue

2026-10-16 21:13:29 +00:00

[PR #493] [MERGED] docs: prioritize reusable analysis primitives #2025

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#489

No description provided.

Rows
Columns