morluto/rea

Fork 0

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

[PR #5018] [MERGED] fix(tests): preserve temporary fixture ownership #6736

New issue

Closed

opened 2026-11-01 21:42:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:42:41 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5018
Author: @gensokyo-dev
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/18/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/ci-audit-fail-closed-20261018

📝 Commits (2)

60b2149 fix(tests): detect all private temporary-root residues
122dbcd fix(tests): normalize fixture ownership at creation

📊 Changes

7 files changed (+134 additions, -12 deletions)

View changed files

📝 docs/testing.md (+5 -0)
➕ scripts/lib/test-temporary-ownership.d.mts (+2 -0)
➕ scripts/lib/test-temporary-ownership.mjs (+18 -0)
📝 scripts/verify-test-temp-hygiene.mjs (+3 -2)
➕ tests/boundary/filesystem/testTempHygiene.test.ts (+97 -0)
📝 tests/fixtures/temporaryDirectory.ts (+4 -9)
📝 tests/support/workspace/workspaceFixture.ts (+5 -1)

📄 Description

Summary

The shared temporary-directory helper accepts labels such as the existing reference-unicode-, but the hygiene verifier only recognizes rea- and bb-session-. Retained fixtures using other labels could therefore pass the hygiene check.

Normalize prefixes once in both shared fixture factories, adding rea- only when needed and retaining the caller's label. Factories and verifier share one ownership rule. Existing owned prefixes, third-party caches, failure diagnostics, and exact-root cleanup retain their behavior. A real tsx loader run verifies cache compatibility.

Ordinary test-infrastructure fix; maintainer scope decision not applicable. No product CLI/MCP, provider, package, or generated-contract changes. No separate issue: found during this CI/test audit.

Evidence and validation

Base c12d9eda8: actual shared factories left unrecognized fixture names; controlled successful children retaining those fixtures incorrectly exited 0. A failed child omitted the residues from diagnostics. All 7 new regressions fail against the base factories/verifier and pass with the fix.
Coverage includes both factories, unchanged existing prefixes, clean/retained fixtures, a third-party cache, an external directory symlink, combined test failure/residue diagnostics, exact-root removal, and external-target preservation.
Real Vitest hygiene run: 44 tests passed, including ciTestProjects, the Unicode-prefix consumer, and new subprocess-boundary scenarios.
Baseline CI infrastructure audit: 418 distinct tests checked; one direct invocation lacked npm's environment, then all 8 tests in that file passed under npm exec.
npm run verify:test-discovery: all 1,580 test files discovered exactly once.
npm run check: passed.
npm run docs:check: passed, including 5 README consistency tests.
Current-head CI: all 37 jobs passed for 122dbcda1, including portable inventory/report merging and selected platform/provider lanes. Codex's latest manual review reported no major issues; Greptile's review of this head found no blocking issue (5/5).

Same 26-test real hygiene workload, Node 24.18.0/npm 11.16.0 on Linux x64, one worker:

Revision	Elapsed	User/system CPU	Sampled family RSS peak
Base	40.09 s	12.95 / 9.64 s	634 MiB
Fix	27.06 s	11.71 / 4.63 s	640 MiB

Single observations on a shared host; file-cache warmth and competing work were uncontrolled. RSS was sampled every 100 ms and may double-count shared pages or miss short-lived/reparented children. No speedup claim. Agent token cost and cold/warm isolation were not measured. Local validation used Linux x64; other hosts rely on CI. Static checks ran one task at a time with two Go threads; sampled family RSS peaked at 4.57 GiB.

Author readiness

Human owner: @gensokyo-dev

The owner explicitly authorized ready-for-review publication, delegated completion of these declarations, and merge in the task instructions. Codex performed the diff review and validation; a separate human review was not independently observed.

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.

_{🔄 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/5018 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/18/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/ci-audit-fail-closed-20261018` --- ### 📝 Commits (2) - [`60b2149`](https://github.com/morluto/rea/commit/60b2149c5c0f99f963a5b366632e3876b4968e27) fix(tests): detect all private temporary-root residues - [`122dbcd`](https://github.com/morluto/rea/commit/122dbcda18ffba01e8d14ee12f966c8cbe68b96e) fix(tests): normalize fixture ownership at creation ### 📊 Changes **7 files changed** (+134 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+5 -0) ➕ `scripts/lib/test-temporary-ownership.d.mts` (+2 -0) ➕ `scripts/lib/test-temporary-ownership.mjs` (+18 -0) 📝 `scripts/verify-test-temp-hygiene.mjs` (+3 -2) ➕ `tests/boundary/filesystem/testTempHygiene.test.ts` (+97 -0) 📝 `tests/fixtures/temporaryDirectory.ts` (+4 -9) 📝 `tests/support/workspace/workspaceFixture.ts` (+5 -1) </details> ### 📄 Description ## Summary The shared temporary-directory helper accepts labels such as the existing `reference-unicode-`, but the hygiene verifier only recognizes `rea-` and `bb-session-`. Retained fixtures using other labels could therefore pass the hygiene check. Normalize prefixes once in both shared fixture factories, adding `rea-` only when needed and retaining the caller's label. Factories and verifier share one ownership rule. Existing owned prefixes, third-party caches, failure diagnostics, and exact-root cleanup retain their behavior. A real `tsx` loader run verifies cache compatibility. Ordinary test-infrastructure fix; maintainer scope decision not applicable. No product CLI/MCP, provider, package, or generated-contract changes. No separate issue: found during this CI/test audit. ## Evidence and validation - Base `c12d9eda8`: actual shared factories left unrecognized fixture names; controlled successful children retaining those fixtures incorrectly exited 0. A failed child omitted the residues from diagnostics. All 7 new regressions fail against the base factories/verifier and pass with the fix. - Coverage includes both factories, unchanged existing prefixes, clean/retained fixtures, a third-party cache, an external directory symlink, combined test failure/residue diagnostics, exact-root removal, and external-target preservation. - Real Vitest hygiene run: 44 tests passed, including `ciTestProjects`, the Unicode-prefix consumer, and new subprocess-boundary scenarios. - Baseline CI infrastructure audit: 418 distinct tests checked; one direct invocation lacked npm's environment, then all 8 tests in that file passed under `npm exec`. - `npm run verify:test-discovery`: all 1,580 test files discovered exactly once. - `npm run check`: passed. - `npm run docs:check`: passed, including 5 README consistency tests. - [Current-head CI](https://github.com/morluto/rea/actions/runs/39122743401): all 37 jobs passed for `122dbcda1`, including portable inventory/report merging and selected platform/provider lanes. [Codex's latest manual review](https://github.com/morluto/rea/pull/5018#issuecomment-6249311488) reported no major issues; [Greptile's review of this head](https://github.com/morluto/rea/pull/5018#issuecomment-6249200535) found no blocking issue (5/5). Same 26-test real hygiene workload, Node 24.18.0/npm 11.16.0 on Linux x64, one worker: | Revision | Elapsed | User/system CPU | Sampled family RSS peak | | --- | ---: | ---: | ---: | | Base | 40.09 s | 12.95 / 9.64 s | 634 MiB | | Fix | 27.06 s | 11.71 / 4.63 s | 640 MiB | Single observations on a shared host; file-cache warmth and competing work were uncontrolled. RSS was sampled every 100 ms and may double-count shared pages or miss short-lived/reparented children. No speedup claim. Agent token cost and cold/warm isolation were not measured. Local validation used Linux x64; other hosts rely on CI. Static checks ran one task at a time with two Go threads; sampled family RSS peaked at 4.57 GiB. ## Author readiness Human owner: @gensokyo-dev The owner explicitly authorized ready-for-review publication, delegated completion of these declarations, and merge in the task instructions. Codex performed the diff review and validation; a separate human review was not independently observed. - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:42:41 +00:00

closed this issue
added the
pull-request
label

bsakaguchi changed title from ~~[PR #5018] [MERGED] fix(tests): preserve temporary fixture ownership~~ to [PR #5018] fix(tests): preserve temporary fixture ownership

2026-11-03 19:45:53 +00:00

bsakaguchi changed title from ~~[PR #5018] fix(tests): preserve temporary fixture ownership~~ to [PR #5018] [MERGED] fix(tests): preserve temporary fixture ownership

2026-11-03 20:47:59 +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#6736

No description provided.

Rows
Columns