morluto/rea

Fork 0

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

[GH-ISSUE #5199] [Bug] Stopping the sampler can return before its ps command exits #3189

New issue

Closed

opened 2026-10-31 00:37:39 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:37:39 +00:00

Owner

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

Before submitting

Searched open and closed issues for related reports.
Removed credentials and sensitive target data.

Area

Process capture or runtime observation

Observed behavior

In a controlled POSIX subprocess regression, awaiting the sampler's stop function returned before its owned ps command finished handling SIGTERM. This is separate from the interrupted-observation coverage gap in #5194.

Reproduction

On source revision ba866a26ecbe179a717e746cbc0d1605ae616ad5, add a process-global regression that puts a source-owned fake ps executable first on PATH. It acknowledges that its SIGTERM handler is installed, then on SIGTERM waits 150 ms, writes a completion marker, and exits. Start the production sampler, await the ready marker, await sampler stop, and read the completion marker.

Ran with pinned Node: npm run test:run -- --project process-global tests/process-global/processSamplerCommandFinalization.test.ts. The marker read failed with ENOENT; cleanup separately waited for the handler before removing its directory. One pinned baseline run reproduced this. The test exercises the Darwin sampling branch explicitly.

Expected behavior

The owner should await its issued command's exit before completing sampler shutdown. See ownership and completion invariants.

Impact and scope

Observed only with a controlled delayed SIGTERM handler on macOS arm64, Node 24.18.0. Natural system ps timing, real Linux behavior, and Windows behavior were not reproduced. Shutdown can otherwise finish while an owned observation command remains active.

REA version / runtime / target / interface / failure stage

Source ba866a26ecbe179a717e746cbc0d1605ae616ad5; macOS arm64; Node 24.18.0. Source-owned Node subprocess fixture; internal production sampler boundary. Cleanup, cancellation, or lifecycle.

Relevant evidence and analysis

The post-stop readFile(donePath) rejected with ENOENT. ProcessSampling.ts calls execFileOutput with only { signal }. ExecFileOutput.ts documents that native abort rejects before exit; its existing stopSignal option supervises exit. Proposed fix: use that option at the sampler's command owner, retaining the existing cancellation classification.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5199 ### Before submitting - [x] Searched open and closed issues for related reports. - [x] Removed credentials and sensitive target data. ### Area Process capture or runtime observation ### Observed behavior In a controlled POSIX subprocess regression, awaiting the sampler's stop function returned before its owned `ps` command finished handling SIGTERM. This is separate from the interrupted-observation coverage gap in #5194. ### Reproduction On source revision `ba866a26ecbe179a717e746cbc0d1605ae616ad5`, add a process-global regression that puts a source-owned fake `ps` executable first on PATH. It acknowledges that its SIGTERM handler is installed, then on SIGTERM waits 150 ms, writes a completion marker, and exits. Start the production sampler, await the ready marker, await sampler stop, and read the completion marker. Ran with pinned Node: `npm run test:run -- --project process-global tests/process-global/processSamplerCommandFinalization.test.ts`. The marker read failed with ENOENT; cleanup separately waited for the handler before removing its directory. One pinned baseline run reproduced this. The test exercises the Darwin sampling branch explicitly. ### Expected behavior The owner should await its issued command's exit before completing sampler shutdown. See [ownership and completion invariants](https://github.com/morluto/rea/blob/ba866a26ecbe179a717e746cbc0d1605ae616ad5/docs/tool-design.md#own-the-lifecycle). ### Impact and scope Observed only with a controlled delayed SIGTERM handler on macOS arm64, Node 24.18.0. Natural system `ps` timing, real Linux behavior, and Windows behavior were not reproduced. Shutdown can otherwise finish while an owned observation command remains active. ### REA version / runtime / target / interface / failure stage Source `ba866a26ecbe179a717e746cbc0d1605ae616ad5`; macOS arm64; Node 24.18.0. Source-owned Node subprocess fixture; internal production sampler boundary. Cleanup, cancellation, or lifecycle. ### Relevant evidence and analysis The post-stop `readFile(donePath)` rejected with `ENOENT`. [ProcessSampling.ts](https://github.com/morluto/rea/blob/ba866a26ecbe179a717e746cbc0d1605ae616ad5/src/process/capture/ProcessSampling.ts#L127) calls `execFileOutput` with only `{ signal }`. [ExecFileOutput.ts](https://github.com/morluto/rea/blob/ba866a26ecbe179a717e746cbc0d1605ae616ad5/src/process/ExecFileOutput.ts#L11) documents that native abort rejects before exit; its existing `stopSignal` option supervises exit. Proposed fix: use that option at the sampler's command owner, retaining the existing cancellation classification.

bsakaguchi

2026-10-31 00:37:39 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-31 00:37:40 +00:00

Author

Owner

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

Fixed on main in ca71e91d23.

The root cause was treating an abort rejection as command completion. The sampler and ownership-query callers now use the existing supervised stop contract, which waits for exit and escalates after its grace period. The adjacent Darwin reader had a separate ownership gap: close tracked compilation but omitted admitted queries. Token and identity queries now share one command owner; close rejects new command admission, drains existing commands, and only then removes the helper root. Failed root removal retains its exact owner for retry without rerunning queries.

Validation: 66 relevant tests passed across focused runs, including native macOS helper controls and CLI/MCP cancellation; the final 8-test finalization subset passed after adding mixed query outcomes and failed-cleanup retry. The unchanged shared command owner also passed its 30 cancellation/timeout controls. npm run check, npm run docs:check, and discovery of all 1,637 test files passed. All seven original counterexamples were observed failing before their producing fixes (five delayed command cases and two held-helper close cases).

Scope: delayed SIGTERM uses real source-owned POSIX subprocesses on macOS. Concurrent reader close uses explicit production-boundary gates and real directory removal; normal native macOS helper reads also passed. The FreeBSD route was exercised with an explicit host selection on macOS, not a real BSD kernel. Native Windows and current real Linux behavior remain unverified.

Three completed real CLI captures retained identical terminal/normal-exit/verified-cleanup oracles. Repeated medians against the earlier e8da baseline: elapsed 1.949→1.921 s; family CPU 1.839→1.784 s; sampled family peak RSS 515.0→514.1 MB; estimated full-reply cl100k tokens 2063→2062.5 (7,642 bytes both). These are small, non-alternating samples with warm filesystem caches, not a speedup claim; actual agent task tokens, true cold runs, and MCP discovery costs were not measured. Cancellation intentionally waits for physical completion.

The Node abort behavior was researched through Context7 and DeepWiki and verified against pinned Node 24.18.0 source. No related cleanup remains in this command-owner slice.

@morluto commented on GitHub (Oct 19, 2026): Fixed on `main` in ca71e91d23672646af917ede8ae82c7fea395341. The root cause was treating an abort rejection as command completion. The sampler and ownership-query callers now use the existing supervised stop contract, which waits for exit and escalates after its grace period. The adjacent Darwin reader had a separate ownership gap: close tracked compilation but omitted admitted queries. Token and identity queries now share one command owner; close rejects new command admission, drains existing commands, and only then removes the helper root. Failed root removal retains its exact owner for retry without rerunning queries. Validation: 66 relevant tests passed across focused runs, including native macOS helper controls and CLI/MCP cancellation; the final 8-test finalization subset passed after adding mixed query outcomes and failed-cleanup retry. The unchanged shared command owner also passed its 30 cancellation/timeout controls. `npm run check`, `npm run docs:check`, and discovery of all 1,637 test files passed. All seven original counterexamples were observed failing before their producing fixes (five delayed command cases and two held-helper close cases). Scope: delayed SIGTERM uses real source-owned POSIX subprocesses on macOS. Concurrent reader close uses explicit production-boundary gates and real directory removal; normal native macOS helper reads also passed. The FreeBSD route was exercised with an explicit host selection on macOS, not a real BSD kernel. Native Windows and current real Linux behavior remain unverified. Three completed real CLI captures retained identical terminal/normal-exit/verified-cleanup oracles. Repeated medians against the earlier e8da baseline: elapsed 1.949→1.921 s; family CPU 1.839→1.784 s; sampled family peak RSS 515.0→514.1 MB; estimated full-reply cl100k tokens 2063→2062.5 (7,642 bytes both). These are small, non-alternating samples with warm filesystem caches, not a speedup claim; actual agent task tokens, true cold runs, and MCP discovery costs were not measured. Cancellation intentionally waits for physical completion. The Node abort behavior was researched through Context7 and DeepWiki and verified against [pinned Node 24.18.0 source](https://github.com/nodejs/node/blob/v24.18.0/lib/child_process.js#L705-L714). No related cleanup remains in this command-owner slice.

bsakaguchi referenced this issue

2026-10-31 01:08:49 +00:00

[PR #3298] [CLOSED] fix(javascript): honor node-ID seed matching options #5710

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

No description provided.

Rows
Columns