Unblinded after review: A = Mercury 3, B = Claude Haiku 5.5, C = GPT-6 Luna. Written by a reviewing agent (Opus) and published unedited. # Review: c04-list-read-race All three candidates implement the reference design. They compare the listing's size and mtime (from lstat) against fstat of the opened file before reading, at all three checksum sites (`check_modification`, `build_ward_entry_from_fs`, `current_entry_fingerprint_payload`), and they reuse `ChecksumError::ConcurrentModification`, which gives exit 255 and names the path. Each one also makes sure at compile time that no production caller skips the check. All three pass 5/5 hidden tests, their own suites, and fmt, clippy and dprint. ## Candidate A: overall 8/10 Verdict: the fix is correct and complete, and it includes an end-to-end race test. The weak points are a timing-based test design and a truncated docstring. | Requirement | Verdict | Evidence | | ------------------------------------------------------------- | ------- | --------------------------------------------------------------------------- | | Fatal (255) on size/mtime change between listing and checksum | met | `checksum_file_expecting` compares at open; hidden 5/5 | | No retry, no ward written | met | Error raised in `compute_status` before the write phase | | Modified files | met | `check_modification` passes the listing values | | Added files | met | `build_ward_entry_from_fs`; hidden added-file test passes | | `--verify` fingerprint payloads | met | `current_entry_fingerprint_payload` guarded | | All commands, with or without `--fingerprint` | met | Unguarded `checksum_file` is now `#[cfg(test)]` | | Plain status unaffected | met | Never policy does not read files; concurrent-writes test | | No false positives | met | Hidden control test passes | | SPEC.md | met | New bullet | Bugs (all minor): - The `checksum_file` Errors section is cut off mid-sentence ("...is *not* a guarantee that the") because the following line was replaced by `#[cfg(test)]`. Verified by reading the tree. - The production doc says "Like [`checksum_file`]", but that item only exists under `#[cfg(test)]`, which gives a broken intra-doc link in normal builds. Not verified. - The integration race tests use a fixed 15 ms sleep against a 64 MiB checksum, with up to 10 attempts. They are slow and could flake on a fast or loaded machine. I ran them twice in a copy and they passed both times, taking 7 to 8 s per run. Scope: README updated (justified). Test modules now import `checksum_file` explicitly (mechanical). Summary is honest. ## Candidate B: overall 8/10 Verdict: the fix is correct, the API enforces the check through its types, and the docs are good. The tests only cover the checksum primitive, not the status-walk wiring. | Requirement | Verdict | Evidence | | ------------------------------------------------------------- | ------- | --------------------------------------------------------------- | | Fatal (255) on size/mtime change between listing and checksum | met | `checksum_file(path, ListedMetadata)` checks before reading | | No retry, no ward written | met | Hidden tests assert ward unchanged | | Modified files | met | Listing values passed | | Added files | met | Listing values passed | | `--verify` fingerprint payloads | met | Listing values passed | | All commands, with or without `--fingerprint` | met | Parameter is mandatory; only fs-reading constructor is cfg(test) | | Plain status unaffected | met | Argued from code; no test | | No false positives | met | Hidden control passes | | SPEC.md | met | New bullet | Bugs: none found. Scope: README updated (justified). The signature change forced updates to 24 existing test call sites, using `ListedMetadata::current`. That is churn, but it follows from the design. Tests: two unit tests on the primitive only. If `status.rs` passed a fresh re-stat instead of the listing, no test would catch it. The summary is honest, and B states this gap and others plainly. ## Candidate C: overall 9/10 Verdict: the fix is correct. Its deterministic walk-level tests cover every checksum site and the plain-status exception. The only flaw is the deleted function docs. | Requirement | Verdict | Evidence | | ------------------------------------------------------------- | ------- | -------------------------------------------------------------------- | | Fatal (255) on size/mtime change between listing and checksum | met | `checksum_file_from_listing`; hidden 5/5 | | No retry, no ward written | met | Hidden tests assert ward unchanged | | Modified files | met | Unit test `update_rejects_modified_file_changed_after_listing` | | Added files | met | Unit test `ward_update_rejects_added_file_changed_after_listing` | | `--verify` fingerprint payloads | met | Unit test `verified_status_rejects_added_file_changed_after_listing` | | All commands, with or without `--fingerprint` | met | Unguarded fn is `#[cfg(test)]` | | Plain status unaffected | met | `metadata_only_status_does_not_checksum_file_changed_after_listing` | | No false positives | met | Hidden control passes | | SPEC.md | met | New bullet | Bugs (minor): - The `# Behavior` and `# Errors` doc sections of the checksum function were deleted rather than moved, so the O_NOFOLLOW, dev/ino and error-variant documentation is lost. Verified by reading the tree. Scope: C also compares size before and after the read, a small hardening that is justified. README was not updated, so it now under-describes the checks. Summary is honest. ## Ranking 1. C (9) 2. A (8) 3. B (8) Functionally the three are equivalent, and all are correct. Testing decided the order. C's list, edit, then compare tests are deterministic and pin every production branch. A has a real end-to-end race test, but it is timing-based and retry-looped. B tests only the checksum primitive and leaves the status.rs wiring unpinned. On documentation B was cleanest: A truncated a docstring, and C deleted the checksum function's Behavior/Errors sections.