morluto/rea

Fork 0

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

[GH-ISSUE #5206] [Bug] Process exit refresh can discard reparented descendants and report cleanup complete #3193

New issue

Closed

opened 2026-10-31 00:37:42 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

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

Owner

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

Before submitting

Searched open and closed issues and reviewed the related closed PR #4346.
Removed credentials and sensitive target data.

Area / interface / failure stage

Process capture or runtime observation; shared cleanup/lineage boundaries used by CLI/MCP providers; cleanup, cancellation, or lifecycle.

Observed behavior

In a controlled production-boundary probe, a process-table refresh showed a new reparented process, but cleanup returned { "cleaned": true, "signaled": false } without inspecting that row's token. A related lineage probe returned status: "verified" with no descendants after observing the same kind of replacement row.

Reproduction

Built runtime from source 39275420ea18ad803eccfcc2702865b98ed36e44, Node 24.18.0, macOS arm64. The source-owned ProcessOwnershipHost fixtures launch and signal no actual target processes.

Cleanup fixture:

Initial table: PID 100 / PGID 100 with stale launcher command, plus child PID 101 / PPID 100 / PGID 100. Requested launcher command differs, forcing exit settlement.
Refreshed table: only PID 102 / PPID 1 / PGID 102, representing a grandchild that became a detached leader and was reparented while 100 and 101 exited.
The fixture would return the requested run token for 102 if queried. Actual result: no token reads, no signal calls, cleaned: true.

Lineage fixture: initial table has the expected launcher 100 and child 101. Its token batch returns the expected launcher token and an unavailable child token. The liveness refresh contains launcher 100 plus reparented PID 102 / PGID 102. Actual result: verified, descendants: []; 102 is never token-read.

Executed reproducible Node probes against the built production exports; both outcomes repeated after the fixture was tightened to make 102 its own group leader. Source and built-module digests, snapshots, configured fixture tokens, actual token reads, and outputs were recorded separately.

Expected behavior

Fresh exit proof for one process must not discard other observed rows or imply complete owned cleanup/lineage. Carry the full refreshed table into bounded ownership reconciliation, inspect new candidates with native identity and token facts, and retain uncertainty when that examination cannot complete. Positive foreign ownership and PID reuse must remain fail-closed.

Impact and scope

The current cleanup path can report completion while an observed replacement remains unexamined. The fixture configures the replacement with the expected token, but production never reads it; the token is a controlled input, not an observed native fact. Real OS fork/reparent timing and real-provider impact were not reproduced. Native Linux/Windows behavior remains unverified. No claim of signaling an unrelated process is made.

Relevant evidence and analysis

settleLauncherExit recognizes descendants through old PIDs/groups and current ancestry; the reparented row matches none of them. createOwnedCleanupPlan then accepts an empty root plan. In lineage, processIsGone reduces the fresh table to a boolean before the consumer publishes its old descendant list.

Related #4322 reports the opposite symptom, a false ownership failure after exit. Clearing that failure without retaining refreshed membership would expose this completion gap further. PR #4346 remains closed/unmerged; its review history contains corresponding counterexamples. A shared snapshot/ownership correction is required; no production fix is claimed here.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5206 ### Before submitting - [x] Searched open and closed issues and reviewed the related closed PR #4346. - [x] Removed credentials and sensitive target data. ### Area / interface / failure stage Process capture or runtime observation; shared cleanup/lineage boundaries used by CLI/MCP providers; cleanup, cancellation, or lifecycle. ### Observed behavior In a controlled production-boundary probe, a process-table refresh showed a new reparented process, but cleanup returned `{ "cleaned": true, "signaled": false }` without inspecting that row's token. A related lineage probe returned `status: "verified"` with no descendants after observing the same kind of replacement row. ### Reproduction Built runtime from source `39275420ea18ad803eccfcc2702865b98ed36e44`, Node 24.18.0, macOS arm64. The source-owned `ProcessOwnershipHost` fixtures launch and signal no actual target processes. Cleanup fixture: 1. Initial table: PID 100 / PGID 100 with stale launcher command, plus child PID 101 / PPID 100 / PGID 100. Requested launcher command differs, forcing exit settlement. 2. Refreshed table: only PID 102 / PPID 1 / PGID 102, representing a grandchild that became a detached leader and was reparented while 100 and 101 exited. 3. The fixture would return the requested run token for 102 if queried. Actual result: no token reads, no signal calls, `cleaned: true`. Lineage fixture: initial table has the expected launcher 100 and child 101. Its token batch returns the expected launcher token and an unavailable child token. The liveness refresh contains launcher 100 plus reparented PID 102 / PGID 102. Actual result: `verified`, `descendants: []`; 102 is never token-read. Executed reproducible Node probes against the built production exports; both outcomes repeated after the fixture was tightened to make 102 its own group leader. Source and built-module digests, snapshots, configured fixture tokens, actual token reads, and outputs were recorded separately. ### Expected behavior Fresh exit proof for one process must not discard other observed rows or imply complete owned cleanup/lineage. Carry the full refreshed table into bounded ownership reconciliation, inspect new candidates with native identity and token facts, and retain uncertainty when that examination cannot complete. Positive foreign ownership and PID reuse must remain fail-closed. ### Impact and scope The current cleanup path can report completion while an observed replacement remains unexamined. The fixture configures the replacement with the expected token, but production never reads it; the token is a controlled input, not an observed native fact. Real OS fork/reparent timing and real-provider impact were not reproduced. Native Linux/Windows behavior remains unverified. No claim of signaling an unrelated process is made. ### Relevant evidence and analysis [`settleLauncherExit`](https://github.com/morluto/rea/blob/39275420ea18ad803eccfcc2702865b98ed36e44/src/process/ProcessOwnership.ts#L360) recognizes descendants through old PIDs/groups and current ancestry; the reparented row matches none of them. [`createOwnedCleanupPlan`](https://github.com/morluto/rea/blob/39275420ea18ad803eccfcc2702865b98ed36e44/src/process/ProcessOwnership.ts#L440) then accepts an empty root plan. In lineage, [`processIsGone`](https://github.com/morluto/rea/blob/39275420ea18ad803eccfcc2702865b98ed36e44/src/process/ProcessOwnershipObservation.ts#L564) reduces the fresh table to a boolean before the consumer publishes its old descendant list. Related #4322 reports the opposite symptom, a false ownership failure after exit. Clearing that failure without retaining refreshed membership would expose this completion gap further. PR #4346 remains closed/unmerged; its review history contains corresponding counterexamples. A shared snapshot/ownership correction is required; no production fix is claimed here.

bsakaguchi

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

closed this issue
added the
triage

bug
labels

bsakaguchi commented

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

Author

Owner

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

Thanks for the detailed write-up. The fail-open gap is real: when settleLauncherExit only matches old PIDs/groups, a reparented row like PID 102 silently falls through and cleanup reports complete without ever reading its token. Agree that the full refreshed table should be carried into bounded ownership reconciliation instead of being collapsed to a boolean, and that uncertainty must be retained (fail-closed) when the examination cannot complete. This also connects to #4322's false-ownership failure — fixing one side without the shared snapshot correction would just move the symptom. Hope the shared snapshot/ownership correction lands soon.

@XinwangTao commented on GitHub (Oct 19, 2026): Thanks for the detailed write-up. The fail-open gap is real: when settleLauncherExit only matches old PIDs/groups, a reparented row like PID 102 silently falls through and cleanup reports complete without ever reading its token. Agree that the full refreshed table should be carried into bounded ownership reconciliation instead of being collapsed to a boolean, and that uncertainty must be retained (fail-closed) when the examination cannot complete. This also connects to #4322's false-ownership failure — fixing one side without the shared snapshot correction would just move the symptom. Hope the shared snapshot/ownership correction lands soon.

bsakaguchi commented

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

Author

Owner

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

Hi @morluto , I'd like to take this if you're open to an outside fix, since I see a lot of fix(process) work landing right now. My plan is to treat rows that are new in the refreshed table as candidates whose token gets read (unreadable means unverified, not cleaned), keep the refreshed membership in lineage instead of a boolean, and add deterministic fixtures for both cases.

@PranavV205 commented on GitHub (Oct 19, 2026): Hi @morluto , I'd like to take this if you're open to an outside fix, since I see a lot of `fix(process)` work landing right now. My plan is to treat rows that are new in the refreshed table as candidates whose token gets read (unreadable means unverified, not cleaned), keep the refreshed membership in lineage instead of a boolean, and add deterministic fixtures for both cases.

bsakaguchi commented

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

Author

Owner

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

49e67aa1d addresses the native producing boundary with a run-scoped ownership ledger carried from acquisition through settlement, finalization, and retries. Related generations survive reparenting, group changes, smaller observations, and omitted table rows. First-seen unreadable identities remain explicit lookup uncertainty and are re-probed; neither historical relation nor a bare PID authorizes signaling. Current matching token/native generation and membership are required.

Swept cleanup now validates/signals all planned groups before a bounded final family check. Electron uses that shared operation, and capture no longer has a separate verification seam. Controls cover delayed post-SIGKILL absence, a helper moving groups during the last signal, retrying its newly proven group without repeating released peers, omitted unreadable identity, and retaining the temporary root while ownership is unresolved. Ownership/consumer focused coverage: 513 passed, 1 skipped in 35 files; static/docs/discovery checks passed on macOS with Node 24.18.0.

Keeping this broader issue open. The original identityless/custom-host refresh path has a weaker contract and is not upgraded into native proof by these changes. Native Linux exit/reparent/PID-reuse behavior and Windows cleanup were not qualified. This remains point-in-time observation, not atomic containment, and a child never observed before escape is not covered by retained history.

@morluto commented on GitHub (Oct 19, 2026): [49e67aa1d](https://github.com/morluto/rea/commit/49e67aa1d963bda337c261f4813ecb7128e2a50c) addresses the native producing boundary with a run-scoped ownership ledger carried from acquisition through settlement, finalization, and retries. Related generations survive reparenting, group changes, smaller observations, and omitted table rows. First-seen unreadable identities remain explicit lookup uncertainty and are re-probed; neither historical relation nor a bare PID authorizes signaling. Current matching token/native generation and membership are required. Swept cleanup now validates/signals all planned groups before a bounded final family check. Electron uses that shared operation, and capture no longer has a separate verification seam. Controls cover delayed post-SIGKILL absence, a helper moving groups during the last signal, retrying its newly proven group without repeating released peers, omitted unreadable identity, and retaining the temporary root while ownership is unresolved. Ownership/consumer focused coverage: 513 passed, 1 skipped in 35 files; static/docs/discovery checks passed on macOS with Node 24.18.0. Keeping this broader issue open. The original identityless/custom-host refresh path has a weaker contract and is not upgraded into native proof by these changes. Native Linux exit/reparent/PID-reuse behavior and Windows cleanup were not qualified. This remains point-in-time observation, not atomic containment, and a child never observed before escape is not covered by retained history.

bsakaguchi commented

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

Author

Owner

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

Cross-referencing with sibling Linux process ownership report #4322 ([Bug] Linux exit race reports an exited owned process as a token mismatch). Both reports isolate race conditions during /proc-based process exit polling, reparented descendants, and ownership token lifecycle verification.

Consolidated under Cluster AC (Linux Process Exit Races, Descendant Reparenting & Token Mismatches) in tracker #4330.

@matbrgz commented on GitHub (Oct 19, 2026): Cross-referencing with sibling Linux process ownership report #4322 (`[Bug] Linux exit race reports an exited owned process as a token mismatch`). Both reports isolate race conditions during `/proc`-based process exit polling, reparented descendants, and ownership token lifecycle verification. Consolidated under **Cluster AC (Linux Process Exit Races, Descendant Reparenting & Token Mismatches)** in tracker #4330.

bsakaguchi commented

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

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Nice — #5675's token-check on identityless exit-refresh replacements closes the exact gap reported here: a reparented row can no longer be skipped while cleanup still reports complete. Good that the refreshed membership is carried into reconciliation rather than collapsed to a boolean, and that unreadable identity stays fail-closed instead of being treated as cleaned. Suggest pinning the reparented-row fixture (PID 102 detached leader with expected token available) in the contract tests so future changes can't regress back to reporting clean without reading that token.

@XinwangTao commented on GitHub (Oct 21, 2026): Nice — #5675's token-check on identityless exit-refresh replacements closes the exact gap reported here: a reparented row can no longer be skipped while cleanup still reports complete. Good that the refreshed membership is carried into reconciliation rather than collapsed to a boolean, and that unreadable identity stays fail-closed instead of being treated as cleaned. Suggest pinning the reparented-row fixture (PID 102 detached leader with expected token available) in the contract tests so future changes can't regress back to reporting clean without reading that token.

bsakaguchi referenced this issue

2026-10-31 01:09:12 +00:00

[PR #3467] [CLOSED] test(browser): cover WebMCP cleanup outcomes #5758

bsakaguchi referenced this issue

2026-10-31 01:09:14 +00:00

[PR #3470] [CLOSED] fix(browser): shut down the WebMCP domain before detaching CDP targets #5762