Fix: retain writer authority through reader-fence test setup (#174)

[flyingrobots]

Problem and outcome

The reader-fence process fixture released migration writer authority before each collector scenario reacquired it. A hosted run failed at that nonblocking acquisition with Busy. This PR returns the existing authority with the migrated store and retains it through collection, eliminating the release/reacquire gap.

Change kind: test-isolation bug fix. Scope is two reader-fence laws and their fixture, with receipts and documentation. Production code is unchanged. This independently merges onto main and does not require #161.

Invariant and approach

Collector preparation keeps cooperating writer authority continuously. Each law checks the real public adapter refuses a competing writer with exact WriterLockAcquireError::Busy at fixture handoff. The existing reader-fence assertions, kernel-observed blocked reader, SIGKILL/reap, persistent inode and post-release snapshot admission remain intact.

A concurrent subprocess can inherit open lock descriptions until exec, bridging a drop/reacquire gap. That source-supported schedule explains how the old setup can fail, but no syscall trace proves it caused the original hosted failure. Retaining existing authority eliminates the handoff rather than relying on that hypothesis. Production explicit-unlock changes, retries, sleeps, unsafe fork hooks and global test serialization were rejected as unnecessary or weaker fixes.

Evidence

The evidence record preserves the first hosted failure, a deterministic RED and fixed GREEN. Regression commit 6b1f1db fails the named continuous-exclusion assertion on unfixed main 34d70909b0cd93f6b020a59d4d40f43b07971cd8, with the law run alone before any child spawn. Both complete process scenarios then pass in copied-Docker debug/release runs at fixed code ff9d972efd67198391c6b1973d9f33538c1c5ed3.

This new assertion is fixture-contract evidence using the actual runtime lock adapter, not a reproduced hosted fork/exec schedule or a new product locking promise. The full copied-Docker required chain passes at ff9d972efd67198391c6b1973d9f33538c1c5ed3. Receipt-only successor e1300cf4a866bd34cba4ebf7dd50233ca264dada trims timestamp-line whitespace that failed the first hosted documentation check, and documents that normalization; runtime code is unchanged. Full candidate whitespace and Docker Markdown checks pass. Exact-successor independent review and hosted run 37153901813 remain pending. Earlier green runs do not waive the preserved failure.

Compatibility and implications

No public API, format, dependency, recovery policy, content identity, security boundary or production lock behavior changes. No benchmark claim. The fixture deliberately retains writer authority longer to cover the existing collector schedule; normal release points remain explicit. Existing medium-size resource-enforcement, platform admission, fixed-schedule and power-loss limitations remain documented. No risk waiver or retry-to-green is requested.

Closes #174. Refs #113, #160, #161, #132.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 583edacf-d5b7-4c4f-bd0f-91be381d576b

📥 Commits

Reviewing files that changed from the base of the PR and between 34d7090 and e1300cf.

📒 Files selected for processing (7)

• CHANGELOG.md
• docs/testing-evidence/reader-fence-process.md
• docs/testing-evidence/reader-fence-process/continuous-authority-green.txt
• docs/testing-evidence/reader-fence-process/continuous-authority-red.txt
• docs/testing-evidence/reader-fence-process/hosted-busy-failure.txt
• tests/reader_fence_process.rs
• tests/reader_fence_process/fixture.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[flyingrobots]

Code Lawyer finding

Severity	File/lines	Issue	Evidence	Acceptance
P4	docs/testing-evidence/reader-fence-process/hosted-busy-failure.txt:3,5,8,9,12,14	Timestamp-only raw-log lines retain trailing spaces, failing the repository whitespace gate.	Independent review and hosted Documentation run 37153756851 both identify the same six lines. Runtime diagnostics are unchanged.	Trim line-end whitespace in the excerpt, document that normalization, preserve the original raw log, and pass the full candidate diff whitespace check and final hosted docs job.

@codex This is static receipt formatting, not a runtime locking defect. No runtime assertion is appropriate for this fix.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[flyingrobots]

Hosted infrastructure failure preserved

Exact head e1300cf4a866bd34cba4ebf7dd50233ca264dada passes documentation and dependency jobs. Rust job 111293250277 stopped before compiling/running the first xtask because downloading the existing ambient-authority dependency from crates.io failed with curl [16] Error in the HTTP2 framing layer. No runtime test produced a verdict in that attempt. The raw job log is retained.

Re-executing this failed infrastructure job at the same SHA is required to obtain its missing validation result; it is not acceptance of a flaky test or a retry of the earlier Busy runtime failure. Both first failures remain in the review record. No production code or dependency pins change.

[flyingrobots]

Independent exact-head review: Keep PR #175 / issue #174

Reviewed head: e1300cf4a866bd34cba4ebf7dd50233ca264dada; tree 7868c563d34f01f0d33b7001705aab594f51d657. Base: main 34d70909b0cd93f6b020a59d4d40f43b07971cd8. Runtime fix: ff9d972efd67198391c6b1973d9f33538c1c5ed3, tree cfc9342ea52cfed1d72b998272bf868f6b22bcca. Regression: 6b1f1dbcf27a2520cb626b25dedf527d372c2112.

This is the authorized independent Codex review under the agy-review and Code Lawyer protocols. All source inspection is read-only. No host Rust tests, mutation, source editing, publishing, configuration change, fetch, merge or delegation was performed. Only this report was written. The explicit read-only task overrides the skill's ordinary fetch/publication workflow. Clean status and exact local/remote head/base were verified; the final head was reconfirmed after inspection.

Findings and disposition

No actionable runtime or evidence defect remains. The fix keeps actual migration-owned writer authority through fixture handoff and collector preparation, eliminating the unnecessary close/reopen window. It preserves both original reader-fence schedules and the #113 acceptance observations. The regression establishes the new fixture contract using the real adapter, and the documentation correctly avoids claiming that the inherited-descriptor explanation was proved for the hosted failure.

One P4 formatting defect was found at the initial fix head: timestamp-only lines 3, 5, 8, 9, 12 and 14 in docs/testing-evidence/reader-fence-process/hosted-busy-failure.txt retained trailing spaces. git diff --check failed, matching the repository's whole-tree whitespace gate at .github/workflows/ci.yml:122–123. The parent published this finding before remediation at comment 5973477486, and the initial hosted documentation job also failed. Successor e1300cf... trims only those spaces and explicitly documents the normalization. Both target-relative and actual CI-equivalent whole-tree whitespace checks now pass. The raw original failure remains retained. P4's source fix is verified; final hosted documentation confirmation is still part of the parent's separate exact-head gate. The failed initial job is not represented as green.

Verification Checklist

Scope, history and review surfaces

• Entire seven-file target-relative diff read: CHANGELOG, evidence prose, three raw receipts, the integration target and fixture. The reader harness, production source, Cargo manifests/lockfile, toolchain, fuzz/xtask, AGENTS and binding testing standards have no delta from target main.
• Applicable AGENTS, Testing Standards and enforcement profile remain binding. Change kind, fixture-contract subject, exact typed oracle, medium-size declarations, platform/feature restrictions, deletion criterion and existing enforcement gaps are explicit. This is one independently mergeable issue/PR with no prerequisite on Fix: validate completed migration namespaces and restart evidence (#111) #161 and no unrelated product work.
• All three commits audited: 6b1f1db... adds the same pre-child competing-writer assertion to both laws on the unfixed fixture; ff9d972... returns retained authority and removes both reacquisitions while adding focused receipts/prose; e1300cf... is evidence whitespace/provenance only. There are no merges in the PR history, so there is no new two-parent integration resolution to audit.
• Live GitHub GraphQL review collection exhausted. hasNextPage=false for global comments, reviews and threads, including nested comments if any. At final inspection there are zero threads/reviews and four global comments: hosted Codex limit, CodeRabbit processing the original fix head, the P4 finding, and hosted Codex environment notice. None supplies effective approval. No actionable external finding was silently omitted; CodeRabbit's unfinished optional review is not counted as completed.
• Original Prove reader-fence process death and collector exclusion in external harness #113 full review and subsequent calibration closure remain binding. Their source-pinned historical patches/receipts remain intact. The changed assertions are limited to the new continuous fixture-authority outcome, and its dedicated deterministic RED is assessed below. No historical calibration is relabeled as a fresh exact-head experiment.

Every changed runtime path and preserved production boundary

Behavior	File:line path	Result
Continuous writer authority from initialization/migration to collector	tests/reader_fence_process/fixture.rs:13–15 initializes; 32–37 moves initialization writer lock into migration and executes it; 41 returns directory plus same migration authority. Authority owns inventory at filesystem_migration_authority.rs:39–46; inventory owns the actual writer guard at filesystem_inventory_reader.rs:28–34/83–89.	No close, clone, replacement lock acquisition or authority gap. Returning a tuple moves existing ownership without adding a product API.
Both fixture consumers reject a competing writer	First law reader_fence_process.rs:27–34; second 70–77 → FilesystemWriterLock::try_acquire at filesystem_writer_lock.rs:72–83 → root and writer-file authority at 96–115/123–155.	Exact WriterLockAcquireError::Busy required. Unknown failures and successful competing admission fail. The probe is before this law spawns its reader; any unexpectedly acquired temporary guard is dropped before assertion failure. Both paths implement the same rule.
Live snapshot excludes collection; real death releases fence	First law 35–49 → Reader::spawn at reader.rs:55–82 → child serve at 26–40 → public FilesystemRetentionSnapshot::load at filesystem_retention_snapshot.rs:122–161 → ReaderFence::acquire at reader_fence.rs:35–50. Parent receives readiness before exclusive refusal; reader.rs:119–127 kills/reaps and attests SIGKILL; same collector descriptor then acquires.	Real writer authority now exists before child spawn and remains held throughout. Reader acquisition requires no writer authority, so this longer fixture guard does not block legitimate snapshot admission. Exact EWOULDBLOCK and post-death positive acquisition are unchanged.
Persistent empty inode preservation	First law 36 captures metadata; 50–55 compares original device/inode and zero length after kill and acquisition. Production shared fence owns a file and performs no unlink on drop (reader_fence.rs:17–27).	Assertions/expected values unchanged. Holding migration authority longer retains finite existing handles but mutates no published artifact after migration. Historical distinct-outcome calibration remains applicable to this unchanged observation.
Collector excludes a new public snapshot, then admits it	Second law 78–88: exclusive fence before spawn → reader.rs:92–116/180–189 exact child kernel queue witness → drop collector at 85 and migration writer guard at 86 → public snapshot readiness and normal completion.	Exclusive fence remains the blocking mechanism. Kernel witness, early-ready refusal, release and positive admission are unchanged. No elapsed-time-only negative oracle introduced.
Stable reader coordinates and typed failures	Snapshot 122–155 performs namespace/record/root identity admission and error mapping; reader fence 45–69 validates handle/entry empty regular-file identity around shared flock; snapshot source 66–103 → retention_view_collector.rs:99–117 double collection; snapshot errors preserve sources at filesystem_retention_snapshot_error.rs:53–61.	Production paths unchanged. New fixture ownership neither bypasses admission nor weakens verification. Catalog generation and root/manifest evidence remain separately owned contracts.
Normal and failed teardown	First law explicitly drops collector/writer at 56–57 before store removal; second drops them at 85–86 before successful child finish/removal. Reader startup failure and Drop kill/reap/remove socket at reader.rs:68–81/149–154; owned scratch cleanup at sandbox 49–58.	Actual authority remains an owned guard through preparation and releases at explicit existing schedule points. A test error cannot make an empty wait or child failure pass. No Drop durability claim, retry, sleep, global serialization, unsafe hook or production unlock change.

The original fixture was intentionally repository-only for platform admission. That boundary is preserved: RepositoryInitializationStorage::admit_unchecked and migration's repository constructor drive real protocol operations without certifying production platform eligibility. This PR adds no format/parser; no new fuzz target, golden vector or production model is warranted. #99 incomplete retention-stage preservation, recoverability and cooperating-writer concurrency contracts remain untouched.

Regression, calibration and receipt provenance

• RED is actual runtime observation, not setup or compilation failure. continuous-authority-red.txt:42–59 executes only the collector law, with one other test filtered out, and fails its named continuous-exclusion assertion at regression source line 72. At 6b1f1db..., fixture still drops authority and the assertion precedes the old acquisition and every child spawn. With the law run alone, the old fixture admits the contender deterministically. This avoids a concurrent inherited holder accidentally satisfying the negative oracle.
• The identical continuous-authority assertion appears in both laws over the same fixture/adapter boundary. The sole-law RED calibrates that shared contract; it is not advertised as independent falsification of every spelling or as a reproduced fork/exec schedule. The two original reader outcomes retain their earlier separate calibrated coverage.
• Fixed GREEN runs both whole process scenarios in debug and release, each showing two passes and no filtered laws. The raw log recompiles Keep after the source change, and release uses optimized artifacts. No test expectation was edited between RED and GREEN. Unlike the regression assertion, this focused pass also executes all preserved reader process observations.
• Independently compared committed RED/GREEN receipts with raw 174-red.log and 174-focused-green.log. They are byte-equal after only the declared isolated source/target prefix substitutions and trailing empty-tail normalization. Original raw compiler/runtime diagnostics are preserved.
• Independently read both copied-source archives and compared their test/fixture/harness blobs to exact Git revisions: 174-source.tar equals regression 6b1f1db...; 174-final.tar equals runtime fix ff9d972.... This supports the source coordinates rather than trusting a receipt label. The final successor has no runtime/test delta from the fixed archive.
• The final hosted failure excerpt is an exact contiguous original diagnostic excerpt after trimming its line-end whitespace. Its first line starts at the named failing test; subsequent original timestamps, Error: Busy, passing companion law, failed result and exit 101 are retained. 161-hosted-rust-failure.log remains the unnormalized source.
• The hosted failure itself does not identify a syscall, duplicated descriptor or precise interleaving. The inheritance mechanism remains an explicitly qualified possible schedule. A passing delayed-exec strace run is explicitly not negative evidence for a race. Neither document nor PR claims production locking was defective or that every source of Busy has been proven eliminated.

Constants, numbers and documentation claims

• No timing, size, format, retry or buffer constant changes. Existing 20-second fail-only watchdog, 1,048,576-byte catalog policy limit, generation-one fixture, zero-length fence, signal nine and default three reader attempts remain as in the original review. No performance guarantee or new enforced budget is inferred.
• Two law passes in each profile agree with the fixed raw output; one selected failing law and one filtered law agree with RED. Historical failed hosted run 37153036962/job 111290654504 and its source e206298... agree with the retained log. Counts describe executed experiments, not correctness coverage.
• New CHANGELOG entry accurately states fixture guard retention, handoff removal and unchanged production semantics. Evidence paragraphs at lines 41/43/45/47 accurately identify possible causality, fixture-contract oracle, source coordinates, commands, platforms, deletion criterion and limits. Paragraphs use one physical line.
• Prove reader-fence process death and collector exclusion in external harness #113 calibration paths remain pinned to their original ee21b01... source. The current guard change does not retroactively alter historical source or assertions. Resource, unsupported platform, fixed-schedule, full-GC and power-loss limits remain intact. No unrelated requirements entry was marked complete or roadmap checkbox rewritten.
• P4 final normalization is explicitly documented at evidence line 41; only receipt whitespace and its prose changed in e1300cf.... Both target-relative diff whitespace check and the exact whole-tree CI command pass on the final head. Initial failure remains part of the audit record.

Execution evidence and final gate boundary

Executed by this reviewer: read-only Git clean/head/tree/diff/history checks, exact whole-tree whitespace check, complete current GitHub review/head/base retrieval, archive-to-Git blob comparison, raw-to-committed receipt comparisons and source inspection. No Rust test or mutation was executed by this reviewer.

Inspected authored execution: deterministic RED, focused debug/release GREEN, and 174-validation.log. The full copied-Docker trace starts with tree cfc9342..., Rust 1.96.0, Linux aarch64, actual ext4 /dev/loop0 scratch and tmpfs negative fixture. It contains golden/conformance/source checks, debug/optimized process-death campaigns, formatting, feature checks/Clippy, full debug/release workspace tests, docs/doctests/MSRV and fuzz compilation/Clippy, finishing at successful final fuzz Clippy. The parent reports terminal session 82691 exit zero. The final whitespace-only successor has no runtime/tooling semantic delta, so that execution remains valid for the unchanged fixed code. It is not falsely relabeled as a different tree's execution.

Separately pending: completion of all final-successor hosted checks, any later optional provider feedback, effective approval admission and branch-protection/merge requirements. The parent reports final documentation/dependency jobs green, but final Rust job 111293250277 failed before the first xtask compilation because downloading ambient-authority from crates.io returned an HTTP/2 framing error. Its preserved 175-rust-failure.log was inspected and confirms infrastructure failure before runtime execution, not another Busy observation. It is excluded from runtime evidence and does not count as a successful required Rust gate. The initial hosted documentation failure is likewise excluded from green claims. This report provides exact-head independent approval; it does not declare the merge gate open while the separate final execution gate remains unsatisfied.

Unexercised and unclaimed: exact original hosted inheritance schedule, arbitrary fork/exec interleavings, physical power loss, complete GC, hostile noncooperating writers and full resource-sandbox compliance. These bounded limits do not invalidate the verified fixture continuity outcome or justify unrelated hardening.

Reviewed SHA: e1300cf4a866bd34cba4ebf7dd50233ca264dada.

APPROVE

[flyingrobots]

Code Lawyer closure — exact candidate e1300cf

Item	Source / severity	Fix and evidence	Disposition
Collector preparation drops then reacquires writer authority	Hosted runtime failure; test-isolation defect #174	6b1f1dbcf27a2520cb626b25dedf527d372c2112 adds a deterministic competing-admission assertion observed RED on the old fixture. ff9d972efd67198391c6b1973d9f33538c1c5ed3 retains the same migration-owned authority. Both original kernel reader schedules pass in debug/release.	Fixture contract fixed; exact untraced hosted interleaving remains unproved, explicitly documented.
Timestamp-only receipt lines violate whitespace gate	Independent review / P4	e1300cf4a866bd34cba4ebf7dd50233ca264dada trims only line-end whitespace and documents normalization. Raw failure retained; whole candidate whitespace and Docker Markdown checks pass.	Closed; initial docs failure preserved.
Full validation	Required local and hosted evidence	Full copied-Docker chain exits zero at fixed runtime tree cfc9342ea52cfed1d72b998272bf868f6b22bcca. Final successor changes receipt/docs only. All four final-head jobs in run 37153901813 pass.	Passed.
Hosted dependency-download failure	Infrastructure, before tests	First final-head Rust attempt fails fetching existing ambient-authority with HTTP/2 framing error. Raw log and comment 5973491008 preserved. Attempt two executes the missing Rust validation at identical SHA.	Not a runtime failure or retry-to-green of the Busy test.
Independent review	Authorized Codex fallback	Exact-head APPROVE and complete checklist.	Effective source approval.
Review queue and protections	Complete paginated bodies/comments/threads	No unresolved actionable threads or changes-requested reviews. Hosted Codex is unavailable; CodeRabbit's unfinished old-head review is not counted as approval. Normal signed merge only; no bypass.	Closed under authorized independent-review workflow.

No production source, API, format, recovery policy, dependency, benchmark promise or #99 scope changes. Original reader exclusion, SIGKILL/reap, fence identity and post-release admission observations remain intact. Existing platform/resource/power-loss limits remain explicit. The target is main 34d70909b0cd93f6b020a59d4d40f43b07971cd8; final head and checks are rechecked immediately before the authorized merge.

MERGE GATE: OPEN. Maintainer authorization is already recorded; use a normal merge matching the reviewed head.