morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #2928] [CLOSED] fix(process): count one unreadable child once during cleanup #5497

New issue

Closed

opened 2026-10-31 01:07:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2928
Author: @CodeAlex52
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/process-cleanup-count-dedupe

📝 Commits (5)

70a1893 fix(process): count one unreadable child once during cleanup
52a23ff fix(process): retain stronger cleanup failures
89d36df fix(process): count one unreadable child once during cleanup
014ce9f Merge remote-tracking branch 'refs/remotes/wave3/pr2928-latest' into codex/maintenance-pr-2928
e46862d Merge branch 'main' into fix/process-cleanup-count-dedupe

📊 Changes

3 files changed (+156 additions, -1 deletions)

View changed files

📝 src/process/ProcessOwnership.test.ts (+67 -0)
📝 src/process/ProcessOwnership.ts (+32 -1)
➕ src/process/cleanup-count.regression.test.ts (+57 -0)

📄 Description

Summary

Fixes #2540.

Cleanup now reports one unreadable live child once: the reason names one live process and the failure list keeps one entry per PID.

Problem and expected behavior

The cleanup plan seeded its unresolved list with the sweep failures (ProcessOwnership.ts:459) and then appended the per-group validation failures, which describe the same member again (:492-510), so cleanupValidationFailure counted the same PID twice:

process ownership token could not be read for 2 live process(es): environment_unavailable=2; live candidates 101=environment_unavailable, 101=environment_unavailable

Expected (per the issue's restoration check): one failure for PID 101, a count of one, cleanup still unsuccessful, and no signal to the unverified group. A control with two distinct unreadable live children must still report each PID once and count two.

Change and scope

src/process/ProcessOwnership.ts: when merging the per-group validation failures into the unresolved list, skip PIDs that the sweep already recorded. One observation per PID is kept; distinct PIDs are unaffected, and the signalability decision (failures.length === 0 && scanFailures.length === 0) is unchanged.

Verification

Added src/process/cleanup-count.regression.test.ts with the issue's reproduction and its control:

one unreadable child → failures has length 1, reason contains 1 live process(es), zero signal calls;
two distinct unreadable children → one failure per PID, reason contains 2 live process(es), zero signal calls.

I traced both fixtures through the cleanup plan and the failure merge by hand; I could not run Vitest in this environment (no local checkout), so please let CI run npm run test:focused -- src/process/cleanup-count.regression.test.ts and the static checks.

_{🔄 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/2928 **Author:** [@CodeAlex52](https://github.com/CodeAlex52) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/process-cleanup-count-dedupe` --- ### 📝 Commits (5) - [`70a1893`](https://github.com/morluto/rea/commit/70a18934e6ab6168f6997675040a1d1428f6d749) fix(process): count one unreadable child once during cleanup - [`52a23ff`](https://github.com/morluto/rea/commit/52a23ff69921c743415d81a014fcf39e5b2291be) fix(process): retain stronger cleanup failures - [`89d36df`](https://github.com/morluto/rea/commit/89d36dfbb05d8b8cf5e7c42d100f264cb221224d) fix(process): count one unreadable child once during cleanup - [`014ce9f`](https://github.com/morluto/rea/commit/014ce9fd1b0868bd0cf22cbdddaae210daaf24f1) Merge remote-tracking branch 'refs/remotes/wave3/pr2928-latest' into codex/maintenance-pr-2928 - [`e46862d`](https://github.com/morluto/rea/commit/e46862dc3af91fc39f7055145e42231858225cac) Merge branch 'main' into fix/process-cleanup-count-dedupe ### 📊 Changes **3 files changed** (+156 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/process/ProcessOwnership.test.ts` (+67 -0) 📝 `src/process/ProcessOwnership.ts` (+32 -1) ➕ `src/process/cleanup-count.regression.test.ts` (+57 -0) </details> ### 📄 Description ## Summary Fixes #2540. Cleanup now reports one unreadable live child once: the reason names one live process and the failure list keeps one entry per PID. ## Problem and expected behavior The cleanup plan seeded its unresolved list with the sweep failures (`ProcessOwnership.ts:459`) and then appended the per-group validation failures, which describe the same member again (`:492-510`), so `cleanupValidationFailure` counted the same PID twice: ```text process ownership token could not be read for 2 live process(es): environment_unavailable=2; live candidates 101=environment_unavailable, 101=environment_unavailable ``` Expected (per the issue's restoration check): one failure for PID 101, a count of one, cleanup still unsuccessful, and no signal to the unverified group. A control with two distinct unreadable live children must still report each PID once and count two. ## Change and scope `src/process/ProcessOwnership.ts`: when merging the per-group validation failures into the unresolved list, skip PIDs that the sweep already recorded. One observation per PID is kept; distinct PIDs are unaffected, and the signalability decision (`failures.length === 0 && scanFailures.length === 0`) is unchanged. ## Verification Added `src/process/cleanup-count.regression.test.ts` with the issue's reproduction and its control: - one unreadable child → `failures` has length 1, reason contains `1 live process(es)`, zero signal calls; - two distinct unreadable children → one failure per PID, reason contains `2 live process(es)`, zero signal calls. I traced both fixtures through the cleanup plan and the failure merge by hand; I could not run Vitest in this environment (no local checkout), so please let CI run `npm run test:focused -- src/process/cleanup-count.regression.test.ts` and the static checks. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:11 +00:00

[PR #5497] [MERGED] test: stabilize worker cancellation and trailer regressions #6939

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

No description provided.

Rows
Columns