[GH-ISSUE #4939] [Bug] CLI finalizers silently omit cancellation during final service close #3064

Closed
opened 2026-10-31 00:35:48 +00:00 by bsakaguchi · 1 comment
Owner

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

Before submitting

  • Searched open/closed issues and PRs for finishAnalysisCommand (19 results, exhausted). Related forwarding proposal #4652 and broader lifecycle tracker #4227; #4683 concerns provider root cleanup, not this final service-close window.
  • Removed credentials and sensitive target data.

Area / interface / failure stage

CLI / CLI / Cleanup, cancellation, or lifecycle.

Observed behavior

The registered inspect-wasm-artifact command returns complete successful Evidence when interruption arrives during its final awaited service.close(). The command's signal handler observes SIGINT (exit status 130), but the finalizer never receives that signal, so its post-close cancellation check is disabled.

Reproduction actually run

Against 2ae4e1608f, registered the real WASM CLI command with a real WasmArtifactService and a controlled valid provider result. The provider's close triggers the registered SIGINT callback through an injected CommandCancellationHost, after result production. Executed five controls: one failed (late interruption returned Evidence instead of typed cancellation with partial Evidence); four passed (uninterrupted completion, primary failure plus interruption, close failure, interrupted close failure). This is a controlled production CLI/service/finalizer seam, not an OS signal or real WABT reproduction.

Expected behavior

After the last awaited finalization, return typed cancellation with completed Evidence when interrupted. Keep an existing primary failure or failed cleanup authoritative. Require each finalizer caller to supply its cancellation context so omission cannot silently disable this lifecycle rule. Preserve fallback resource diagnostics.

Impact and scope

Source audit found 18 of 34 production finalizer calls omit the command signal: ADB, Android, Apktool, PE diagnostics, packet capture/field discovery, EVM, Go, JavaScript recovery, JEB, pcap, signal decoding, SQLite, WASM, module tracing, source-location tracing and network capture. Only the registered WASM close interleaving was executed; natural frequency, native-provider behavior and other platform signals are NOT_ENOUGH_INFO. Existing browser/Electron/Cutter/Firmware/Flutter/Rust forwarding paths must remain intact.

Version / runtime / target

Source 2ae4e1608f, Node 24.18.0, macOS arm64. Authored WASM Evidence fixture; no target execution, binary mutation, or native-provider acquisition.

finalizer signature and post-close check; registered WASM call. docs/tool-design.md#own-the-lifecycle requires cancellation through awaited cleanup and preservation of completed observations.

PR #4652 (Tapraj-Pathak, 8c1cbe0f0e) identifies forwarding omissions. Its added fifth-position signals conflict with the current fifth-position optional string fallback; its helper-only test passes a sixth-position signal and cannot establish registration correctness. Proposed structural fix: make AbortSignal the required fifth parameter, move optional fallback sixth, migrate all 34 callers, preserve existing forwarded signals and add the actual registered-command control. This report tracks the verified integration/type-contract gap rather than claiming a novel independent forwarding proposal.

Originally created by @morluto on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4939 ### Before submitting - [x] Searched open/closed issues and PRs for `finishAnalysisCommand` (19 results, exhausted). Related forwarding proposal #4652 and broader lifecycle tracker #4227; #4683 concerns provider root cleanup, not this final service-close window. - [x] Removed credentials and sensitive target data. ### Area / interface / failure stage CLI / CLI / Cleanup, cancellation, or lifecycle. ### Observed behavior The registered `inspect-wasm-artifact` command returns complete successful Evidence when interruption arrives during its final awaited `service.close()`. The command's signal handler observes SIGINT (exit status 130), but the finalizer never receives that signal, so its post-close cancellation check is disabled. ### Reproduction actually run Against 2ae4e1608f290c95ff7cff28c4d16a79b301d03d, registered the real WASM CLI command with a real WasmArtifactService and a controlled valid provider result. The provider's close triggers the registered SIGINT callback through an injected CommandCancellationHost, after result production. Executed five controls: one failed (late interruption returned Evidence instead of typed cancellation with partial Evidence); four passed (uninterrupted completion, primary failure plus interruption, close failure, interrupted close failure). This is a controlled production CLI/service/finalizer seam, not an OS signal or real WABT reproduction. ### Expected behavior After the last awaited finalization, return typed cancellation with completed Evidence when interrupted. Keep an existing primary failure or failed cleanup authoritative. Require each finalizer caller to supply its cancellation context so omission cannot silently disable this lifecycle rule. Preserve fallback resource diagnostics. ### Impact and scope Source audit found 18 of 34 production finalizer calls omit the command signal: ADB, Android, Apktool, PE diagnostics, packet capture/field discovery, EVM, Go, JavaScript recovery, JEB, pcap, signal decoding, SQLite, WASM, module tracing, source-location tracing and network capture. Only the registered WASM close interleaving was executed; natural frequency, native-provider behavior and other platform signals are NOT_ENOUGH_INFO. Existing browser/Electron/Cutter/Firmware/Flutter/Rust forwarding paths must remain intact. ### Version / runtime / target Source 2ae4e1608f290c95ff7cff28c4d16a79b301d03d, Node 24.18.0, macOS arm64. Authored WASM Evidence fixture; no target execution, binary mutation, or native-provider acquisition. ### Relevant source / related proposed repair [finalizer signature and post-close check](https://github.com/morluto/rea/blob/2ae4e1608f290c95ff7cff28c4d16a79b301d03d/src/cli/finishAnalysisCommand.ts); [registered WASM call](https://github.com/morluto/rea/blob/2ae4e1608f290c95ff7cff28c4d16a79b301d03d/src/cli/wasmCommands.ts). `docs/tool-design.md#own-the-lifecycle` requires cancellation through awaited cleanup and preservation of completed observations. PR #4652 (Tapraj-Pathak, 8c1cbe0f0ebbc7e4a11449450f4aba6f80475dc8) identifies forwarding omissions. Its added fifth-position signals conflict with the current fifth-position optional string fallback; its helper-only test passes a sixth-position signal and cannot establish registration correctness. Proposed structural fix: make AbortSignal the required fifth parameter, move optional fallback sixth, migrate all 34 callers, preserve existing forwarded signals and add the actual registered-command control. This report tracks the verified integration/type-contract gap rather than claiming a novel independent forwarding proposal.
bsakaguchi 2026-10-31 00:35:48 +00:00
  • closed this issue
  • added the
    triage
    bug
    labels
Author
Owner

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

Fixed in 279c590ef6 on main.

The shared CLI finalizer now requires AbortSignal before optional resource diagnostics. All 34 production calls use the same signal as their operations; 18 previously omitted it. This removes silent opt-out and the placeholder argument before a sixth-position signal. The network-capture resource fallback remains intact. Existing primary and cleanup failures retain precedence; successful final close checks cancellation and preserves completed Evidence.

Credit: adapted Tapraj-Pathak's forwarding proposal #4652 (verified commit 8c1cbe0f0e), with co-author attribution. Its fifth-position signal additions needed the corresponding finalizer signature correction. The registered-command regression exercises this wiring; a helper-only check cannot prove it.

Executed validation: before the fix, the new registered WASM late-close interrupt failed while four controls passed. Afterward 163 tests across 14 files passed: registered WASM success/interruption/primary-failure/cleanup-failure/mixed interruption controls, finalizer boundaries, Android, packet/signal/JEB/Apktool command workflows, web retained cleanup, Flutter cleanup/retry, direct cleanup composition and EVM reason translation. Typecheck/lint, docs with five conformance tests, discovery of all 1,572 files, and commit/push hooks passed. Source review confirmed all signal/fallback migrations; no remaining blockers found.

Six bounded completed tasks compared the registered WASM command plus real service/finalizer with controlled provider data, before and after. Entire replies and normalized Evidence matched; cl100k serialized reply estimate stayed 830. Warm medians: elapsed 0.151874 → 0.154182 s; CPU 0.169239 → 0.165026 s; sampled family peak RSS 150,503,424 → 109,846,528 bytes. No speed or memory improvement claimed: only two alternating warm repeats, short process lifetimes, 10 ms RSS sampling, no cache flush. Actual model/host/schema tokens and cancelled/failed-cleanup costs unmeasured. Elapsed includes parent reply validation; CPU is wait4 family usage.

Limits: the new interleaving uses the actual CLI registration, service and finalizer with a controlled provider and injected signal host. No real WABT/EVM engine, OS signal, Linux/Windows platform qualification or natural failure frequency is asserted. Heavy CPU/string-limit stress and profiling stayed stopped. Broader lifecycle audit remains incomplete.

<!-- gh-comment-id:6244416637 --> @morluto commented on GitHub (Oct 18, 2026): Fixed in 279c590ef6bb737d4a48ea696a0ae5eb30010b3d on main. The shared CLI finalizer now requires AbortSignal before optional resource diagnostics. All 34 production calls use the same signal as their operations; 18 previously omitted it. This removes silent opt-out and the placeholder argument before a sixth-position signal. The network-capture resource fallback remains intact. Existing primary and cleanup failures retain precedence; successful final close checks cancellation and preserves completed Evidence. Credit: adapted Tapraj-Pathak's forwarding proposal #4652 (verified commit 8c1cbe0f0ebbc7e4a11449450f4aba6f80475dc8), with co-author attribution. Its fifth-position signal additions needed the corresponding finalizer signature correction. The registered-command regression exercises this wiring; a helper-only check cannot prove it. Executed validation: before the fix, the new registered WASM late-close interrupt failed while four controls passed. Afterward 163 tests across 14 files passed: registered WASM success/interruption/primary-failure/cleanup-failure/mixed interruption controls, finalizer boundaries, Android, packet/signal/JEB/Apktool command workflows, web retained cleanup, Flutter cleanup/retry, direct cleanup composition and EVM reason translation. Typecheck/lint, docs with five conformance tests, discovery of all 1,572 files, and commit/push hooks passed. Source review confirmed all signal/fallback migrations; no remaining blockers found. Six bounded completed tasks compared the registered WASM command plus real service/finalizer with controlled provider data, before and after. Entire replies and normalized Evidence matched; cl100k serialized reply estimate stayed 830. Warm medians: elapsed 0.151874 → 0.154182 s; CPU 0.169239 → 0.165026 s; sampled family peak RSS 150,503,424 → 109,846,528 bytes. No speed or memory improvement claimed: only two alternating warm repeats, short process lifetimes, 10 ms RSS sampling, no cache flush. Actual model/host/schema tokens and cancelled/failed-cleanup costs unmeasured. Elapsed includes parent reply validation; CPU is wait4 family usage. Limits: the new interleaving uses the actual CLI registration, service and finalizer with a controlled provider and injected signal host. No real WABT/EVM engine, OS signal, Linux/Windows platform qualification or natural failure frequency is asserted. Heavy CPU/string-limit stress and profiling stayed stopped. Broader lifecycle audit remains incomplete.
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#3064
No description provided.