[GH-ISSUE #4787] [Bug] WABT runtime cleanup remains incomplete after a successful retry #2490

New issue

Closed

opened 2026-10-17 21:05:37 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-17 21:05:37 +00:00

Owner

Originally created by @xu91102 on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4787

Observed behavior

On main 5d6029ea62a65ec34fd739ee40223c77c6fdb63b, the built inspect-wasm-artifact CLI still returns cleanup_incomplete and lists a runtime root after its own close retry has successfully removed that exact directory.

I reproduced this with the unchanged official WABT 1.0.42 macOS arm64 executables, Node 24.18.0, and a harmless authored 20-byte core module. Fault injection denied only the first fs.promises.rm for the command's own rea-wabt-* root. The second call delegated to the real filesystem and succeeded. This is controlled cleanup failure, not a naturally occurring OS denial.

Input / cleanup	Observed CLI result	Confirmed filesystem result
Valid module / permitted	exit 0, complete Evidence	root removed
Same module / first removal denied, retry succeeds	exit 1, `cleanup_incomplete`, root still listed	second removal succeeds, root absent, isolated temp directory empty
WABT-rejected module / first removal denied, retry succeeds	`cleanup_incomplete`, original `invalid_request` only in diagnostics	root absent after the successful retry

Each case above was executed once. The valid completed Evidence is already retained as a partial observation; the defect is the stale live ownership and failure classification after recovery.

Reproduction

Build the checkout and set REA_WABT_BIN_DIRECTORY to the real 1.0.42 tool directory.
Write these valid module bytes to selected.wasm: 0061736d01000000000a066f7061717565616263.
Use a Node preload with syncBuiltinESMExports() to wrap fs.promises.rm. Restrict the wrapper to a fresh isolated TMPDIR and a direct child whose basename starts rea-wabt-. On the first matching call, throw an authored EACCES before invoking the original. On later calls, await the original and record existsSync(path).
Run node --import ./cleanup-preload.mjs scripts/rea.mjs inspect-wasm-artifact ./selected.wasm --json.
The trace records first denial, then a successful second removal with existsSync(path) === false; the JSON response still contains details.cleanup: "incomplete" and that removed path in details.resources.

The archive SHA-256 for the real toolchain matched GitHub release metadata: 3f654779b436c628db3ca4323c51538815f5fbc887a6d5f3bb1c18d36281e240. The tools validated/decoded authored modules only; no WASM target was instantiated or executed.

Expected behavior and producing boundary

A failed cleanup must remain owned while the exact resource is live. When that same owner confirms successful retry, returned errors should stop advertising it as unresolved. Preserve the original typed input/cancellation failure, completed partial Evidence and historical cleanup diagnostics; recovery must not clear another still-failing owner.

Source-derived cause: WabtArtifactProvider retains the runtime through ArtifactResourceScope.release, but constructs an unbound ProviderCleanupError. A subsequent successful provider.close() removes the retained owner without updating the originating error. The existing AnalysisCleanupLease/group mechanism provides exact-owner resolution for other producers.

Related closed #3935 fixed lost Evidence on repeated cleanup failure; #4254 fixed cancellation during successful final cleanup. The recovered-root trace above exercises a separate outcome. Actual open PR diffs #2406, #2694, #4433 and #4485 do not change this final root/error ownership boundary.

Scope and verification limits

Observed on macOS arm64 through the real compiled CLI, real WABT 1.0.42 and real temporary-directory removal with one injected denial. No natural permission failure, Windows/Linux, real application prevalence or performance result is claimed. A focused fix and same-connection recovery controls are being prepared; there is no upstream PR yet.

Local validation update and overlap

An independently reviewed local fix now binds root/error cleanup to exact leases, preserves the typed primary failure and exact late-cancellation reason, and keeps a failed snapshot descriptor owned before removing its parent root. Deduplicated focused coverage is 16 files /159 PASS plus5 README checks; affected test files were rerun rather than counted twice. Full check, docs and unique discovery gates passed.

Real WABT1.0.42 on macOS arm64 passed the official42-case compiled CLI/stdio SDK lane. Four additional actual CLI controls covered normal input, a first-removal failure followed by real recovery, rejected input with recovery, and actual SIGINT after root removal. The SIGINT response kept the same full Evidence as the successful control. One official SDK stdio connection handled two blocked calls on the same retained root, then two recovered successful calls; provider roots were absent and the isolated temp directory was empty after awaited connection teardown. These remain controlled filesystem fault tests, not natural failure-frequency, Windows/Linux or WASM-execution results.

PR #4799 now changes the same runtime-root/error binding and primary-failure composition. I read its full actual diff and am holding my local commit read-only instead of opening a duplicate PR. My additional descriptor-ordering, cancellation and real-tool validation above ran against my local snapshot, not the head of #4799; they do not certify all of that PR's behavior. There is no PR or upstream CI from my account for this issue, and the local branch was not pushed.

Upstream resolution readback

The maintainer independently fixed this root in commit 22a0746f, and closed this issue as completed at2026-10-17T13:03:07Z. I verified the production diff and that the commit is an ancestor of captured main bd3210eb. PR #4799 was subsequently closed without merging. This was not a merge of my unpublished local commit and has no coauthor credit to me. My extended local controls have not run on that upstream commit, so this source readback does not certify their full range. The local snapshot remains read-only and no duplicate PR will be opened.

Originally created by @xu91102 on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4787 ### Observed behavior On main `5d6029ea62a65ec34fd739ee40223c77c6fdb63b`, the built `inspect-wasm-artifact` CLI still returns `cleanup_incomplete` and lists a runtime root after its own close retry has successfully removed that exact directory. I reproduced this with the unchanged official WABT 1.0.42 macOS arm64 executables, Node 24.18.0, and a harmless authored 20-byte core module. Fault injection denied only the first `fs.promises.rm` for the command's own `rea-wabt-*` root. The second call delegated to the real filesystem and succeeded. This is controlled cleanup failure, not a naturally occurring OS denial. | Input / cleanup | Observed CLI result | Confirmed filesystem result | | --- | --- | --- | | Valid module / permitted | exit 0, complete Evidence | root removed | | Same module / first removal denied, retry succeeds | exit 1, `cleanup_incomplete`, root still listed | second removal succeeds, root absent, isolated temp directory empty | | WABT-rejected module / first removal denied, retry succeeds | `cleanup_incomplete`, original `invalid_request` only in diagnostics | root absent after the successful retry | Each case above was executed once. The valid completed Evidence is already retained as a partial observation; the defect is the stale live ownership and failure classification after recovery. ### Reproduction 1. Build the checkout and set `REA_WABT_BIN_DIRECTORY` to the real 1.0.42 tool directory. 2. Write these valid module bytes to `selected.wasm`: `0061736d01000000000a066f7061717565616263`. 3. Use a Node preload with `syncBuiltinESMExports()` to wrap `fs.promises.rm`. Restrict the wrapper to a fresh isolated `TMPDIR` and a direct child whose basename starts `rea-wabt-`. On the first matching call, throw an authored `EACCES` before invoking the original. On later calls, await the original and record `existsSync(path)`. 4. Run `node --import ./cleanup-preload.mjs scripts/rea.mjs inspect-wasm-artifact ./selected.wasm --json`. 5. The trace records first denial, then a successful second removal with `existsSync(path) === false`; the JSON response still contains `details.cleanup: "incomplete"` and that removed path in `details.resources`. The archive SHA-256 for the real toolchain matched GitHub release metadata: `3f654779b436c628db3ca4323c51538815f5fbc887a6d5f3bb1c18d36281e240`. The tools validated/decoded authored modules only; no WASM target was instantiated or executed. ### Expected behavior and producing boundary A failed cleanup must remain owned while the exact resource is live. When that same owner confirms successful retry, returned errors should stop advertising it as unresolved. Preserve the original typed input/cancellation failure, completed partial Evidence and historical cleanup diagnostics; recovery must not clear another still-failing owner. Source-derived cause: `WabtArtifactProvider` retains the runtime through `ArtifactResourceScope.release`, but constructs an unbound `ProviderCleanupError`. A subsequent successful `provider.close()` removes the retained owner without updating the originating error. The existing `AnalysisCleanupLease`/group mechanism provides exact-owner resolution for other producers. Related closed #3935 fixed lost Evidence on repeated cleanup failure; #4254 fixed cancellation during successful final cleanup. The recovered-root trace above exercises a separate outcome. Actual open PR diffs #2406, #2694, #4433 and #4485 do not change this final root/error ownership boundary. ### Scope and verification limits Observed on macOS arm64 through the real compiled CLI, real WABT 1.0.42 and real temporary-directory removal with one injected denial. No natural permission failure, Windows/Linux, real application prevalence or performance result is claimed. A focused fix and same-connection recovery controls are being prepared; there is no upstream PR yet. ### Local validation update and overlap An independently reviewed local fix now binds root/error cleanup to exact leases, preserves the typed primary failure and exact late-cancellation reason, and keeps a failed snapshot descriptor owned before removing its parent root. Deduplicated focused coverage is 16 files /159 PASS plus5 README checks; affected test files were rerun rather than counted twice. Full check, docs and unique discovery gates passed. Real WABT1.0.42 on macOS arm64 passed the official42-case compiled CLI/stdio SDK lane. Four additional actual CLI controls covered normal input, a first-removal failure followed by real recovery, rejected input with recovery, and actual SIGINT after root removal. The SIGINT response kept the same full Evidence as the successful control. One official SDK stdio connection handled two blocked calls on the same retained root, then two recovered successful calls; provider roots were absent and the isolated temp directory was empty after awaited connection teardown. These remain controlled filesystem fault tests, not natural failure-frequency, Windows/Linux or WASM-execution results. PR #4799 now changes the same runtime-root/error binding and primary-failure composition. I read its full actual diff and am holding my local commit read-only instead of opening a duplicate PR. My additional descriptor-ordering, cancellation and real-tool validation above ran against my local snapshot, **not** the head of #4799; they do not certify all of that PR's behavior. There is no PR or upstream CI from my account for this issue, and the local branch was not pushed. ### Upstream resolution readback The maintainer independently fixed this root in commit [`22a0746f`](https://github.com/morluto/rea/commit/22a0746f9031c28a027b01f0bd206c6a63cc0a4b), and closed this issue as completed at2026-10-17T13:03:07Z. I verified the production diff and that the commit is an ancestor of captured main `bd3210eb`. PR #4799 was subsequently closed **without merging**. This was not a merge of my unpublished local commit and has no coauthor credit to me. My extended local controls have not run on that upstream commit, so this source readback does not certify their full range. The local snapshot remains read-only and no duplicate PR will be opened.

bsakaguchi closed this issue

2026-10-17 21:05:37 +00:00

bsakaguchi commented

2026-10-17 21:05:39 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Fixed on main in 22a0746f9.

Runtime and descriptor failures now follow their exact cleanup leases through retry and error translation; confirmed recovery clears live resource claims. Primary typed failures, captured diagnostics, completed partial Evidence and finalization abort reasons remain available. Command supervisors are retained until stop is confirmed before runtime removal (#4798).

Validation: 88 focused tests across 10 files; post-fixture-refactor 28 provider tests; build, static and documentation checks. The actual CLI and real stdio MCP verifier passed 51 cases with official WABT 1.0.42, including authored exact-root/descriptor denials followed by real removal/close and same-connection retry. The baseline reproduced stale cleanup claims for valid input, rejected input and descriptor failure.

Limits: permission denials were controlled injections; incomplete command stop/mixed retries were source controls using exact supervisors, not a real failed OS kill. No WASM target was instantiated. Twelve small ordinary CLI cost tasks retained byte-identical replies and Evidence IDs; no performance improvement is claimed.

@morluto commented on GitHub (Oct 17, 2026): Fixed on main in 22a0746f9. Runtime and descriptor failures now follow their exact cleanup leases through retry and error translation; confirmed recovery clears live resource claims. Primary typed failures, captured diagnostics, completed partial Evidence and finalization abort reasons remain available. Command supervisors are retained until stop is confirmed before runtime removal (#4798). Validation: 88 focused tests across 10 files; post-fixture-refactor 28 provider tests; build, static and documentation checks. The actual CLI and real stdio MCP verifier passed 51 cases with official WABT 1.0.42, including authored exact-root/descriptor denials followed by real removal/close and same-connection retry. The baseline reproduced stale cleanup claims for valid input, rejected input and descriptor failure. Limits: permission denials were controlled injections; incomplete command stop/mixed retries were source controls using exact supervisors, not a real failed OS kill. No WASM target was instantiated. Twelve small ordinary CLI cost tasks retained byte-identical replies and Evidence IDs; no performance improvement is claimed.

bsakaguchi referenced this issue

2026-10-31 00:30:55 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-10-31 01:05:19 +00:00

[PR #2492] feat(setup): support CodeBuddy as a setup client #5270

bsakaguchi referenced this issue

2026-10-31 01:07:41 +00:00

[PR #3041] [MERGED] feat(setup): add WorkBuddy client support #5575