[GH-ISSUE #2428] test(reference): isolate source cleanup fault injection from Git metadata reads #814

New issue

Closed

opened 2026-10-15 18:28:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 18:28:42 +00:00

Owner

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

Originally assigned to: @N0zoM1z0 on GitHub.

The latest main passes CI but a cleanup-ownership test fails reproducibly for developers whose temporary directory is inside a Git repository. The test intercepts every node:fs/promises.open, counts all opens as source-file work, and injects a close failure into every successfully opened handle. Reference-source import legitimately reads enclosing Git metadata as well.

Baseline and actual failure

Baseline: 37b7550ecc916acc8619fa72e5c05af45702733f.

Latest main CI https://github.com/morluto/rea/actions/runs/38328721720 passed all 36 execution jobs. Its merged Linux report has 1,003 files / 10,316 cases (10,224 passed, 92 skipped). This is an environment-dependent test isolation defect, not evidence of a production cleanup regression.

npm run test:focused -- tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts fails locally without coverage or other concurrently selected tests: 1 failed / 3 passed. The failing case is retains partial file bytes and retries the same live owner before new work. It expects openCalls === 1, but observes 8. The complete coverage run independently encounters the same failure.

Tracing the mocked open boundary shows one target source.ts open plus seven legitimate probes under the enclosing temporary-directory .git marker (packed-refs, HEAD, and loose-reference candidates). No external Git marker was removed or modified.

Minimal deterministic counterexample

A clean, task-owned TMPDIR under /var/tmp, outside the ambient repository, makes the original four-case file pass.

Then change one condition: create a task-owned enclosing .git/HEAD containing a valid 40-hex-digit detached HEAD, and import its nested source/ directory. With the original fault injector, the no-enclosing-repository control passes, while the Git variant fails: cleanup resources contain both source/source.ts and .git/HEAD, instead of only the intended source file. The broad mock has manufactured an extra cleanup failure. This five-case counterexample fails 1 / passes 4 without relying on host Git state.

Boundary and oracle

ReferenceSourceImport.ts runs source traversal and readReferenceSourceVcs together. ReferenceSourceVcsAdapter.ts deliberately discovers the nearest enclosing repository and reads its metadata with the owned regular-file boundary. The test's intended oracle is narrower: retain the successfully read source bytes, retry the same live source owner, prevent a second source open while cleanup is pending, and close that source handle after retry succeeds. Auxiliary Git reads are valid independent work.

Proposed correction and acceptance

Scope the open counter and close-failure injection to the exact test-owned source path; delegate auxiliary opens normally.
Retain a deterministic enclosing-Git variant alongside the existing control.
Keep the existing partial-byte, cleanup-resource and retry-count assertions. Add direct file-handle lifetime observations so dropping ownership or skipping close cannot turn this correction into a vacuous pass.
Validate both normal and coverage runs, repeat on a clean temporary tree and the original ambient-Git environment, and test the guard with controlled incorrect owner behavior.
Run static/discovery/document checks and complete PR CI before merging. Preserve production VCS discovery and cleanup behavior.

Originally created by @N0zoM1z0 on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2428 Originally assigned to: @N0zoM1z0 on GitHub. The latest main passes CI but a cleanup-ownership test fails reproducibly for developers whose temporary directory is inside a Git repository. The test intercepts every `node:fs/promises.open`, counts all opens as source-file work, and injects a close failure into every successfully opened handle. Reference-source import legitimately reads enclosing Git metadata as well. ## Baseline and actual failure Baseline: `37b7550ecc916acc8619fa72e5c05af45702733f`. Latest main CI https://github.com/morluto/rea/actions/runs/38328721720 passed all 36 execution jobs. Its merged Linux report has 1,003 files / 10,316 cases (10,224 passed, 92 skipped). This is an environment-dependent test isolation defect, not evidence of a production cleanup regression. `npm run test:focused -- tests/boundary/filesystem/referenceSourceCleanupOwnership.test.ts` fails locally without coverage or other concurrently selected tests: 1 failed / 3 passed. The failing case is `retains partial file bytes and retries the same live owner before new work`. It expects `openCalls === 1`, but observes 8. The complete coverage run independently encounters the same failure. Tracing the mocked open boundary shows one target `source.ts` open plus seven legitimate probes under the enclosing temporary-directory `.git` marker (`packed-refs`, `HEAD`, and loose-reference candidates). No external Git marker was removed or modified. ## Minimal deterministic counterexample A clean, task-owned TMPDIR under `/var/tmp`, outside the ambient repository, makes the original four-case file pass. Then change one condition: create a task-owned enclosing `.git/HEAD` containing a valid 40-hex-digit detached HEAD, and import its nested `source/` directory. With the original fault injector, the no-enclosing-repository control passes, while the Git variant fails: cleanup resources contain both `source/source.ts` and `.git/HEAD`, instead of only the intended source file. The broad mock has manufactured an extra cleanup failure. This five-case counterexample fails 1 / passes 4 without relying on host Git state. ## Boundary and oracle `ReferenceSourceImport.ts` runs source traversal and `readReferenceSourceVcs` together. `ReferenceSourceVcsAdapter.ts` deliberately discovers the nearest enclosing repository and reads its metadata with the owned regular-file boundary. The test's intended oracle is narrower: retain the successfully read source bytes, retry the same live source owner, prevent a second source open while cleanup is pending, and close that source handle after retry succeeds. Auxiliary Git reads are valid independent work. ## Proposed correction and acceptance - Scope the open counter and close-failure injection to the exact test-owned source path; delegate auxiliary opens normally. - Retain a deterministic enclosing-Git variant alongside the existing control. - Keep the existing partial-byte, cleanup-resource and retry-count assertions. Add direct file-handle lifetime observations so dropping ownership or skipping close cannot turn this correction into a vacuous pass. - Validate both normal and coverage runs, repeat on a clean temporary tree and the original ambient-Git environment, and test the guard with controlled incorrect owner behavior. - Run static/discovery/document checks and complete PR CI before merging. Preserve production VCS discovery and cleanup behavior.

bsakaguchi

2026-10-15 18:28:42 +00:00