morluto/rea

Fork 0

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

[PR #3735] [MERGED] test(rizin): own fake provider cleanup in shared fixtures #5873

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:10:05 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3735
Author: @Kenmege
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/rizin-startup-uncertainty-reap

📝 Commits (3)

461dc76 fix(test): reap the Rizin provider left by startup cleanup uncertainty
349be10 test(rizin): own fake provider cleanup in shared fixtures
5d85cfe chore(ci): refresh Rizin fixture audit against current main

📊 Changes

5 files changed (+234 additions, -91 deletions)

View changed files

📝 tests/boundary/providers/RizinDebugSessionCoverage.fixtures.ts (+75 -20)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts (+148 -48)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts (+7 -11)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts (+2 -6)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts (+2 -6)

📄 Description

Summary

The Rizin coverage harness discarded managers after deliberately uncertain cleanup while their real fake-provider children could remain alive. Linux probes on main a40ee741c reproduced the startup child surviving teardown for one second and found three further live children after per-test cleanup in the partial-timeout, truncated-output and cancellation cases. Persistence after worker exit was not measured for those three.

The shared harness now owns its native launches, reaps them with the existing bounded ProviderProcessSupervisor, and requires observed child exit. All four coverage files use its manager factory; custom injected spawners remain selected by the caller. Unsuccessful cleanup retains the owned process and every fixture directory for recovery. The original uncertainty, partial Evidence and inherited-output assertions remain intact. No product behavior, provider policy or public contract changes.

Validation performed

Integrated latest main 9d6651e5b; head 5d85cfe4e. Five fixture/test files changed. Linux x64, Node 24.18.0/npm 11.16.0, two CPU cores, serial heavy workflows, 1.5 GiB Node heap.

Seven focused files: 87 passed, 1 Darwin-only case skipped. Includes all four Rizin coverage files and native supervisor TERM/KILL escalation, bounded waits, incomplete cleanup/retry and output tests.
Independent post-cleanup observation of part4: all eight real fake-provider children had exited; the observer's safety reaper sent no termination. The old harness failed three corresponding postconditions. All probe processes were reaped through their retained owned handles.
Controlled removal of reaping and replacement of waiting with signal-only cleanup: two expected failures each. Restored source passes. Regression failure paths also reap their observed child.
Cleanup failure/recovery retains a live child and its files, admits another fixture, then verifies actual exit and removal of both directories after recovery.
npm ci, build:cached, all eight check:ci tasks and architecture guards passed. Normal pre-push check:fast passed. Current-head Greptile (5/5) and Codex reviews completed without findings.

Completed focused workflow on the integrated head (including an artifact rebuild): wall 31.88s, child user/system CPU 43.58/6.28s, sampled peak family RSS 3.09 GiB. Earlier warm-artifact execution of the same seven test files: 7.17s, 5.07/2.48s, 705 MiB. Fresh Vitest/provider processes; tool and file caches warm. RSS sums live descendants with a 100ms pause; shared pages can repeat and short/reparented children can be missed. Wait-accounted CPU excludes the measurement wrapper. The failed baseline has no equivalent completed cleanup result; no speedup or lower peak-memory claim is made. Agent tokens are unmeasured; no agent-facing response changes.

Native macOS arm64 / Node 24.18.0: 88 passed, zero skips across all seven files, including the Darwin-only ownership/cancellation case. Hosted proof uses diagnostic commit 2583c25d2, whose only difference from PR head 5d85cfe4e is the manual CI workflow. Its gate verifies that source identity, actual Darwin host, fresh JSON report and exact counts; the uploaded report was downloaded and independently checked. Hosted CPU and peak memory were not measured.

This verifies real fixture processes, not a real Rizin debugger.

Ownership and readiness

Original contributor: @Kenmege. The user explicitly delegated the follow-up implementation, technical review, ready-for-review update and merge in this session. Technical review was performed by the assistant; a separate human reread is not claimed.

Final diff, ownership boundaries, validation evidence and limitations technically reviewed.
Ready for initial maintainer review under the user's explicit authorization.

Before merge

Concrete fixture problem and existing process authority established.
Current-head reviews and native macOS evidence recorded.
Complete current-head CI passed, including all five shards, Portable tests and the required selection gate. Optional lanes were skipped according to scope. Greptile and Codex reviewed 5d85cfe4e without findings.

_{🔄 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/3735 **Author:** [@Kenmege](https://github.com/Kenmege) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/rizin-startup-uncertainty-reap` --- ### 📝 Commits (3) - [`461dc76`](https://github.com/morluto/rea/commit/461dc769782d84b263ec5b2bb696925e68c4a37a) fix(test): reap the Rizin provider left by startup cleanup uncertainty - [`349be10`](https://github.com/morluto/rea/commit/349be102ee93d21a2e0b4c787bc9daef148bbcc3) test(rizin): own fake provider cleanup in shared fixtures - [`5d85cfe`](https://github.com/morluto/rea/commit/5d85cfe4ebab0e91b04e6e9d7dadf72738c45677) chore(ci): refresh Rizin fixture audit against current main ### 📊 Changes **5 files changed** (+234 additions, -91 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/providers/RizinDebugSessionCoverage.fixtures.ts` (+75 -20) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts` (+148 -48) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts` (+7 -11) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts` (+2 -6) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts` (+2 -6) </details> ### 📄 Description ## Summary The Rizin coverage harness discarded managers after deliberately uncertain cleanup while their real fake-provider children could remain alive. Linux probes on main `a40ee741c` reproduced the startup child surviving teardown for one second and found three further live children after per-test cleanup in the partial-timeout, truncated-output and cancellation cases. Persistence after worker exit was not measured for those three. The shared harness now owns its native launches, reaps them with the existing bounded `ProviderProcessSupervisor`, and requires observed child exit. All four coverage files use its manager factory; custom injected spawners remain selected by the caller. Unsuccessful cleanup retains the owned process and every fixture directory for recovery. The original uncertainty, partial Evidence and inherited-output assertions remain intact. No product behavior, provider policy or public contract changes. ## Validation performed Integrated latest main `9d6651e5b`; head `5d85cfe4e`. Five fixture/test files changed. Linux x64, Node 24.18.0/npm 11.16.0, two CPU cores, serial heavy workflows, 1.5 GiB Node heap. - Seven focused files: **87 passed, 1 Darwin-only case skipped**. Includes all four Rizin coverage files and native supervisor TERM/KILL escalation, bounded waits, incomplete cleanup/retry and output tests. - Independent post-cleanup observation of part4: **all eight real fake-provider children had exited**; the observer's safety reaper sent no termination. The old harness failed three corresponding postconditions. All probe processes were reaped through their retained owned handles. - Controlled removal of reaping and replacement of waiting with signal-only cleanup: **two expected failures each**. Restored source passes. Regression failure paths also reap their observed child. - Cleanup failure/recovery retains a live child and its files, admits another fixture, then verifies actual exit and removal of both directories after recovery. - `npm ci`, `build:cached`, all eight `check:ci` tasks and architecture guards passed. Normal pre-push `check:fast` passed. Current-head Greptile (5/5) and Codex reviews completed without findings. Completed focused workflow on the integrated head (including an artifact rebuild): wall **31.88s**, child user/system CPU **43.58/6.28s**, sampled peak family RSS **3.09 GiB**. Earlier warm-artifact execution of the same seven test files: **7.17s**, **5.07/2.48s**, **705 MiB**. Fresh Vitest/provider processes; tool and file caches warm. RSS sums live descendants with a 100ms pause; shared pages can repeat and short/reparented children can be missed. Wait-accounted CPU excludes the measurement wrapper. The failed baseline has no equivalent completed cleanup result; no speedup or lower peak-memory claim is made. Agent tokens are unmeasured; no agent-facing response changes. Native macOS arm64 / Node 24.18.0: **88 passed, zero skips across all seven files**, including the Darwin-only ownership/cancellation case. [Hosted proof](https://github.com/morluto/rea/actions/runs/38758103354) uses diagnostic commit `2583c25d2`, whose only difference from PR head `5d85cfe4e` is the manual CI workflow. Its gate verifies that source identity, actual Darwin host, fresh JSON report and exact counts; the uploaded report was downloaded and independently checked. Hosted CPU and peak memory were not measured. This verifies real fixture processes, not a real Rizin debugger. ## Ownership and readiness Original contributor: @Kenmege. The user explicitly delegated the follow-up implementation, technical review, ready-for-review update and merge in this session. Technical review was performed by the assistant; a separate human reread is not claimed. - [x] Final diff, ownership boundaries, validation evidence and limitations technically reviewed. - [x] Ready for initial maintainer review under the user's explicit authorization. ## Before merge - [x] Concrete fixture problem and existing process authority established. - [x] Current-head reviews and native macOS evidence recorded. - [x] [Complete current-head CI](https://github.com/morluto/rea/actions/runs/38757303381) passed, including all five shards, Portable tests and the required selection gate. Optional lanes were skipped according to scope. Greptile and Codex reviewed `5d85cfe4e` without findings. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:05 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns