morluto/rea

Fork 0

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

[PR #6983] fix(process): settle exits during Linux credential observation #7547

New issue

Open

opened 2026-11-03 13:38:36 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:38:36 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6983
Author: @kwakhyun
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: codex/fix-linux-cleanup-followup

📝 Commits (1)

81df2d7 fix(process): settle exits during Linux credential observation

📊 Changes

2 files changed (+211 additions, -38 deletions)

View changed files

📝 src/process/ProcessOwnership.procIdentity.test.ts (+192 -28)
📝 src/process/ProcessOwnershipObservation.ts (+19 -10)

📄 Description

Summary

On Linux, a process can exit between the first native identity read and a later credential read. The observer then returned its earlier live identity, even when the later procfs read failed because the process was gone. Route those missing reads through the existing corroborated-absence check so cleanup can settle the exited candidate.

Related: #4322 and the credential-bracket fallback left open in #6220. This does not claim to resolve all of #4322 or reproduce #6220's entire nondumpable-churn workload. The producer defect remains on base 48c203f3567e7b79af562349eecb6720a96de667, which already includes #5270 and #5675.

Problem and expected behavior

A readable /proc/PID/stat followed by ENOENT/ESRCH from /proc/PID/status or the closing /proc/PID/stat must not silently become the old live observation. As with a missing first stat, absence requires a signal-zero probe that itself reports ESRCH. A successful, false, or permission-denied probe remains unavailable.

Change and scope

Forward missing status/closing-stat errors to the existing Linux absence classification; share its missing-procfs predicate and generalize its free-form diagnostic prefixes.
Retain the existing identity-only fallback for EACCES/EPERM, without attaching unverified credential facts.
Add controlled boundary cases and two real Linux child regressions. The latter control when an owned child exits, then use actual procfs reads and the kernel's signal-zero result after reap.

This is an ordinary fix within the existing observation boundary; a new maintainer capability/scope decision is not applicable. No new retries, waits, cleanup policy, or signal authority. At most one existing signal-zero probe is now reached for a missing optional credential read. Performance was not benchmarked.

Contract and boundary impact

Semantic owner: createSystemProcessOwnershipHost().processIdentities, before ownership facts and cleanup consume the observation. Existing absent/unavailable consumers need no changes.
CLI/MCP schemas, tool catalog, providers, evidence persistence, package/install metadata: unchanged. Free-form missing-procfs diagnostics now describe the broader lookup.
Platform: Linux credential observations only. PID alone still grants no termination authority; live/unknown cases remain conservative.
Read existing files or captured data (existing procfs observation).
Launch or attach to a process (existing cleanup workflows and owned test children).

Evidence and regression coverage

With the new tests and unmodified producer at d9660eb03271d0ce1859cf8720ca460701dd3322: six controlled cases fail; both real Linux child cases also fail, returning the first readable identity after the child exited normally with code 0. The producer is unchanged between that revision and the final base.
With the fix: both native exit cases and the live/unknown/permission controls pass. Existing ownership, PID-replacement, retained-descendant, cleanup-retry, and finalization regressions are included below.
The bounded 100-child ack/exit diagnostic corresponding to #4322 completed with zero first-cleanup, post-exit, or final-cleanup failures. This is one diagnostic run, not a failure-rate estimate or proof that every #4322 path is resolved.
Observed results and inferred scope remain distinct; unavailable coverage is explicit.

Validation performed

Toolchain: Node 24.22.1, npm 11.16.0. Linux runs use Debian bookworm arm64, uid 1000, 1 CPU, 1536 MiB, and one Vitest worker. CLI/MCP tests execute the compiled JavaScript runtime with separately installed Linux dependencies.

npm run check — passed (typecheck, lint/module boundaries, formatting, knip, metadata, architecture guards).
npm run build:cached — passed. The pre-push npm run check:fast also passed; hooks remained enabled.
Exact Linux source selection below — 311 passed in 21 files, including both real child regressions.
Exact Linux boundary/acceptance selection below — 53 passed, 1 macOS-only skip in 8 files, including compiled CLI/MCP finalization and cleanup retry.
node scripts/run-exact-tests.mjs run src/process/ProcessOwnership.procIdentity.test.ts --maxWorkers=1 --no-file-parallelism on macOS — 21 passed, 2 Linux-only skips.
node /evidence/issue4322-linux/repro-ack-exit.mjs (local bounded diagnostic using compiled modules) — 100/100 completed, no incomplete first/post-exit/final cleanup; no forced fixture kills. This diagnostic is separate from the committed regression tests.

Final Linux and static results cover commit 81df2d78fcc032ce9f602b670ffd349c1a807149. An initial full check on the earlier base failed in the unrelated releaseProposal.test.ts types; the same failure reproduced on a pristine checkout. Updating to main 48c203f3, which includes the upstream correction, cleared it. No release-related edit is included here.

Executed Linux test selections

node scripts/run-exact-tests.mjs run \
  src/process/ProcessOwnership.baselineAbsence.test.ts \
  src/process/ProcessOwnership.exitScope.test.ts \
  src/process/ProcessOwnership.exiting.test.ts \
  src/process/ProcessOwnership.finalSweep.test.ts \
  src/process/ProcessOwnership.identity.test.ts \
  src/process/ProcessOwnership.lineage.test.ts \
  src/process/ProcessOwnership.nativeExit.test.ts \
  src/process/ProcessOwnership.observation.test.ts \
  src/process/ProcessOwnership.part2.test.ts \
  src/process/ProcessOwnership.part3.test.ts \
  src/process/ProcessOwnership.procIdentity.test.ts \
  src/process/ProcessOwnership.runId.test.ts \
  src/process/ProcessOwnership.sweepExit.test.ts \
  src/process/ProcessOwnership.test.ts \
  src/process/ProcessOwnership.tokenAvailability.test.ts \
  src/process/ProcessOwnership.validation.test.ts \
  src/process/ProcessOwnershipFacts.test.ts \
  src/process/LinuxProcessStat.test.ts \
  src/process/LinuxProcessStatus.test.ts \
  src/process/capture/ProcessCaptureOwnershipEvidence.test.ts \
  src/process/ProviderProcess.launcherExit.test.ts \
  --maxWorkers=1 --no-file-parallelism \
  --reporter=default --reporter=json \
  --outputFile.json=/tmp/issue4322-linux-source.json

node scripts/run-exact-tests.mjs run \
  tests/boundary/process/providerProcess.test.ts \
  tests/boundary/process/processCleanupRetryOwnership.test.ts \
  tests/boundary/process/processLineageRetention.test.ts \
  tests/boundary/process/processBaselineAbsence.test.ts \
  tests/boundary/mcp/processCleanupRetryMcp.test.ts \
  tests/boundary/mcp/reverseEngineeringEnvironmentMcp.test.ts \
  tests/boundary/mcp/reverseEngineeringProviderEnvironmentMcp.test.ts \
  tests/acceptance/process/processFinalization.test.ts \
  --maxWorkers=1 --no-file-parallelism \
  --reporter=default --reporter=json \
  --outputFile.json=/tmp/issue4322-linux-boundary.json

Compatibility, safety, and release

No breaking API or migration. New native tests are Linux-only; macOS runs the controlled cases and skips those two tests. Windows, real Hopper/Ghidra engines, and the nondumpable synthetic-churn workload were not run. The runtime tests use owned fixture processes; they do not establish real-provider coverage.

Latest review and CI evidence

Re-reviewed unchanged head 81df2d78. Greptile's review is 5/5 with no actionable findings; there are no inline review threads. No Codex review is recorded.

CI run 40082480319 is not green:

Static checks passed, and the source log explicitly records 23/23 passing tests in ProcessOwnership.procIdentity.test.ts, including the two actual Linux child cases.
Build package aborted with free(): invalid next size (normal) / exit 134 in node scripts/ci/portable-build-cache.mjs key, before production build or ownership code executes. The same command succeeds locally on macOS; the runner crash's underlying cause has not been diagnosed.
Source tests exited in a Vitest worker running javascriptSemanticAnalysis.reference_lifetimes.test.ts. Running that file alone with one worker reproduced the same unexpected-worker-exit error both on this PR and in a pristine 48c203f3 worktree using the same pinned Node/dependencies. These JavaScript sources, runner configuration and dependency manifests are unchanged by the PR. This establishes a failure independent of this patch, not its underlying cause.
Portable-test aggregation then lacked the source/shard reports; the required CI gate failed. Those are downstream failures, not additional failed assertions in this change.

The API refused a failed-job rerun with HTTP 403 (Resource not accessible by integration); a maintainer with Actions write access must rerun it. The local ownership/CLI/MCP evidence above remains valid, but full remote qualification remains open.

Author readiness

Human owner: @kwakhyun

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

AI-assisted contribution. The human owner has reviewed the final diff and evidence, accepts the recorded CI limitations, and has declared this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. (Ordinary fix; no new capability.)
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed. (No contract or metadata changes.)
Required remote checks are complete.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6983 **Author:** [@kwakhyun](https://github.com/kwakhyun) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/fix-linux-cleanup-followup` --- ### 📝 Commits (1) - [`81df2d7`](https://github.com/morluto/rea/commit/81df2d78fcc032ce9f602b670ffd349c1a807149) fix(process): settle exits during Linux credential observation ### 📊 Changes **2 files changed** (+211 additions, -38 deletions) <details> <summary>View changed files</summary> 📝 `src/process/ProcessOwnership.procIdentity.test.ts` (+192 -28) 📝 `src/process/ProcessOwnershipObservation.ts` (+19 -10) </details> ### 📄 Description ## Summary On Linux, a process can exit between the first native identity read and a later credential read. The observer then returned its earlier live identity, even when the later procfs read failed because the process was gone. Route those missing reads through the existing corroborated-absence check so cleanup can settle the exited candidate. Related: #4322 and the credential-bracket fallback left open in #6220. This does not claim to resolve all of #4322 or reproduce #6220's entire nondumpable-churn workload. The producer defect remains on base `48c203f3567e7b79af562349eecb6720a96de667`, which already includes #5270 and #5675. ## Problem and expected behavior A readable `/proc/PID/stat` followed by ENOENT/ESRCH from `/proc/PID/status` or the closing `/proc/PID/stat` must not silently become the old live observation. As with a missing first stat, absence requires a signal-zero probe that itself reports ESRCH. A successful, false, or permission-denied probe remains unavailable. ## Change and scope - Forward missing status/closing-stat errors to the existing Linux absence classification; share its missing-procfs predicate and generalize its free-form diagnostic prefixes. - Retain the existing identity-only fallback for EACCES/EPERM, without attaching unverified credential facts. - Add controlled boundary cases and two real Linux child regressions. The latter control when an owned child exits, then use actual procfs reads and the kernel's signal-zero result after reap. This is an ordinary fix within the existing observation boundary; a new maintainer capability/scope decision is not applicable. No new retries, waits, cleanup policy, or signal authority. At most one existing signal-zero probe is now reached for a missing optional credential read. Performance was not benchmarked. ## Contract and boundary impact - Semantic owner: `createSystemProcessOwnershipHost().processIdentities`, before ownership facts and cleanup consume the observation. Existing absent/unavailable consumers need no changes. - CLI/MCP schemas, tool catalog, providers, evidence persistence, package/install metadata: unchanged. Free-form missing-procfs diagnostics now describe the broader lookup. - Platform: Linux credential observations only. PID alone still grants no termination authority; live/unknown cases remain conservative. - [x] Read existing files or captured data (existing procfs observation). - [x] Launch or attach to a process (existing cleanup workflows and owned test children). ## Evidence and regression coverage - With the new tests and unmodified producer at `d9660eb03271d0ce1859cf8720ca460701dd3322`: six controlled cases fail; both real Linux child cases also fail, returning the first readable identity after the child exited normally with code 0. The producer is unchanged between that revision and the final base. - With the fix: both native exit cases and the live/unknown/permission controls pass. Existing ownership, PID-replacement, retained-descendant, cleanup-retry, and finalization regressions are included below. - The bounded 100-child ack/exit diagnostic corresponding to #4322 completed with zero first-cleanup, post-exit, or final-cleanup failures. This is one diagnostic run, not a failure-rate estimate or proof that every #4322 path is resolved. - [x] Observed results and inferred scope remain distinct; unavailable coverage is explicit. ## Validation performed Toolchain: Node 24.22.1, npm 11.16.0. Linux runs use Debian bookworm arm64, uid 1000, 1 CPU, 1536 MiB, and one Vitest worker. CLI/MCP tests execute the compiled JavaScript runtime with separately installed Linux dependencies. - `npm run check` — passed (typecheck, lint/module boundaries, formatting, knip, metadata, architecture guards). - `npm run build:cached` — passed. The pre-push `npm run check:fast` also passed; hooks remained enabled. - Exact Linux source selection below — **311 passed in 21 files**, including both real child regressions. - Exact Linux boundary/acceptance selection below — **53 passed, 1 macOS-only skip in 8 files**, including compiled CLI/MCP finalization and cleanup retry. - `node scripts/run-exact-tests.mjs run src/process/ProcessOwnership.procIdentity.test.ts --maxWorkers=1 --no-file-parallelism` on macOS — **21 passed, 2 Linux-only skips**. - `node /evidence/issue4322-linux/repro-ack-exit.mjs` (local bounded diagnostic using compiled modules) — **100/100 completed**, no incomplete first/post-exit/final cleanup; no forced fixture kills. This diagnostic is separate from the committed regression tests. Final Linux and static results cover commit `81df2d78fcc032ce9f602b670ffd349c1a807149`. An initial full check on the earlier base failed in the unrelated `releaseProposal.test.ts` types; the same failure reproduced on a pristine checkout. Updating to main `48c203f3`, which includes the upstream correction, cleared it. No release-related edit is included here. <details> <summary>Executed Linux test selections</summary> ```sh node scripts/run-exact-tests.mjs run \ src/process/ProcessOwnership.baselineAbsence.test.ts \ src/process/ProcessOwnership.exitScope.test.ts \ src/process/ProcessOwnership.exiting.test.ts \ src/process/ProcessOwnership.finalSweep.test.ts \ src/process/ProcessOwnership.identity.test.ts \ src/process/ProcessOwnership.lineage.test.ts \ src/process/ProcessOwnership.nativeExit.test.ts \ src/process/ProcessOwnership.observation.test.ts \ src/process/ProcessOwnership.part2.test.ts \ src/process/ProcessOwnership.part3.test.ts \ src/process/ProcessOwnership.procIdentity.test.ts \ src/process/ProcessOwnership.runId.test.ts \ src/process/ProcessOwnership.sweepExit.test.ts \ src/process/ProcessOwnership.test.ts \ src/process/ProcessOwnership.tokenAvailability.test.ts \ src/process/ProcessOwnership.validation.test.ts \ src/process/ProcessOwnershipFacts.test.ts \ src/process/LinuxProcessStat.test.ts \ src/process/LinuxProcessStatus.test.ts \ src/process/capture/ProcessCaptureOwnershipEvidence.test.ts \ src/process/ProviderProcess.launcherExit.test.ts \ --maxWorkers=1 --no-file-parallelism \ --reporter=default --reporter=json \ --outputFile.json=/tmp/issue4322-linux-source.json node scripts/run-exact-tests.mjs run \ tests/boundary/process/providerProcess.test.ts \ tests/boundary/process/processCleanupRetryOwnership.test.ts \ tests/boundary/process/processLineageRetention.test.ts \ tests/boundary/process/processBaselineAbsence.test.ts \ tests/boundary/mcp/processCleanupRetryMcp.test.ts \ tests/boundary/mcp/reverseEngineeringEnvironmentMcp.test.ts \ tests/boundary/mcp/reverseEngineeringProviderEnvironmentMcp.test.ts \ tests/acceptance/process/processFinalization.test.ts \ --maxWorkers=1 --no-file-parallelism \ --reporter=default --reporter=json \ --outputFile.json=/tmp/issue4322-linux-boundary.json ``` </details> ## Compatibility, safety, and release No breaking API or migration. New native tests are Linux-only; macOS runs the controlled cases and skips those two tests. Windows, real Hopper/Ghidra engines, and the nondumpable synthetic-churn workload were not run. The runtime tests use owned fixture processes; they do not establish real-provider coverage. ## Latest review and CI evidence Re-reviewed unchanged head `81df2d78`. [Greptile's review](https://github.com/morluto/rea/pull/6983#issuecomment-6382629023) is **5/5 with no actionable findings**; there are no inline review threads. No Codex review is recorded. [CI run 40082480319](https://github.com/morluto/rea/actions/runs/40082480319) is **not green**: - Static checks passed, and the source log explicitly records **23/23** passing tests in `ProcessOwnership.procIdentity.test.ts`, including the two actual Linux child cases. - [Build package](https://github.com/morluto/rea/actions/runs/40082480319/job/120839282812) aborted with `free(): invalid next size (normal)` / exit 134 in `node scripts/ci/portable-build-cache.mjs key`, before production build or ownership code executes. The same command succeeds locally on macOS; the runner crash's underlying cause has not been diagnosed. - [Source tests](https://github.com/morluto/rea/actions/runs/40082480319/job/120839282831) exited in a Vitest worker running `javascriptSemanticAnalysis.reference_lifetimes.test.ts`. Running that file alone with one worker reproduced the same unexpected-worker-exit error both on this PR and in a pristine `48c203f3` worktree using the same pinned Node/dependencies. These JavaScript sources, runner configuration and dependency manifests are unchanged by the PR. This establishes a failure independent of this patch, not its underlying cause. - Portable-test aggregation then lacked the source/shard reports; the required CI gate failed. Those are downstream failures, not additional failed assertions in this change. The API refused a failed-job rerun with HTTP 403 (`Resource not accessible by integration`); a maintainer with Actions write access must rerun it. The local ownership/CLI/MCP evidence above remains valid, but full remote qualification remains open. ## Author readiness Human owner: @kwakhyun - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. AI-assisted contribution. The human owner has reviewed the final diff and evidence, accepts the recorded CI limitations, and has declared this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. (Ordinary fix; no new capability.) - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. (No contract or metadata changes.) - [ ] Required remote checks are complete. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-03 13:38:36 +00:00

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

No description provided.

Rows
Columns