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: c02-fp-repeated-edits

All three candidates pass all 13 hidden tests and their own suites. fmt is clean, and none adds a clippy diagnostic (4 = baseline). No SPEC.md exists at this commit, so no SPEC update was needed.

Experiments were run on copies of each tree under `/tmp/tmp.HqvFavfZzX` (scripts `scen.sh`, `s4.sh`, `touch.sh`):

| Scenario | A | B | C |
|---|---|---|---|
| S1 unreadable added file, default `status` | error 255 | `A locked`, exit 1 | error 255 |
| S2 unreadable M? file, default `status` | error 255 | `M? f`, exit 1 | error 255 |
| S3 `status --diff` then `update --verify --fingerprint` | accepted | accepted | accepted |
| S4 removed entry, ward entry rewritten before update | rejected | **accepted** | rejected |
| S5 file to dir type change: parity, then edit inside | ok / rejected | ok / rejected | ok / rejected |
| S6 `--verify`, added file swapped with same size+mtime | rejected | accepted (same as reference) | rejected |
| S7 default policy, M? file swapped with same size+mtime | rejected | accepted (same as reference, documented) | rejected |
| S8 names with `|` and newline, no change in between | accepted | accepted | accepted |

## Candidate A: overall 7/10

**Verdict:** A strong, well-tested fix. It binds the full old-to-new ward transition for every changed entry. To keep parity, default `status` now checksums every added and M? file, and many docs still describe a metadata-only default.

| Requirement | Verdict | Evidence |
|---|---|---|
| M? second edit rejected | met | old+new WardEntry with sha256 hashed; S7 also rejects same-metadata swap |
| `--verify` M second edit rejected | met | hidden + own tests |
| Added file edit rejected | met | Added always carries checksummed ward_entry |
| Symlink re-point rejected | met | target in new entry |
| Existing mismatch error, nothing written | met | own tests check bytes / no `.treeward` |
| Injective encoding | met | length prefixes, presence/variant tags, fixed-width ints |
| Parity under all policies and kinds | met | hidden parity, S3, S5, own Display-vs-WardUpdate unit test |
| Touch-only accepted | met | hidden parity_touch_only |

**Bugs**
- minor (verified): default `status` is no longer metadata-only. It fails with a permission error on unreadable added or M? files (S1/S2), and on an uninitialized tree it checksums every file.
- minor (verified): README line 120 and `cli.rs` help still say "Fast metadata-only check (default)" and "only reads directory listings and .treeward files". Both are now false.

**Scope:** The CLAUDE.md bullet update is small and accurate. The `StatusEntry` population contract changed, and the existing test that guarded "Display does not checksum" was rewritten to assert the opposite. That reverses a documented design choice.

**Tests:** The best of the three. Unit tests cover content, verify, added, symlink, removed, purpose parity, and injectivity. Integration tests run the attack under all three policies, init, symlink, hostile names, and a happy-path matrix.

**Summary honesty:** Mostly accurate. It says a metadata-only touch "still reports Unchanged". Under the default policy a touch reports `M?`, and the test it cites asserts `M?`.

## Candidate B: overall 7/10

**Verdict:** A clean fix that closely matches the reference and keeps default `status` metadata-only. Its weaknesses are thin unit-only tests, no binding of prior ward state for removals, and a README that fails dprint.

| Requirement | Verdict | Evidence |
|---|---|---|
| M? second edit rejected | met | size+mtime observed; same-metadata swap accepted like the reference, and documented |
| `--verify` M second edit rejected | met | sha256 included when the status-side policy checksummed |
| Added file edit rejected | met | size/mtime observed |
| Symlink re-point rejected | met | observed target |
| Existing mismatch error, nothing written | met | unit test checks `.treeward` |
| Injective encoding | met | domain prefix, length prefixes, tags, sha presence flag |
| Parity under all policies and kinds | met | ward-only checksums kept out of `observed`; hidden parity, S3, S5 |
| Touch-only accepted | met | hidden parity_touch_only |

**Bugs**
- minor (verified, S4): removals hash `Absent`, so a change to the removed entry's ward data between `status` and `update` is accepted.
- minor (verified): README is not dprint-formatted. CI requires `dprint fmt`. The new list also has a stray blank line.

**Scope:** It adds a public `observed` field to every `StatusEntry` variant. The `diffing.rs` fixture churn uses `ObservedEntry::Dir` for file entries, which is harmless but sloppy.

**Tests:** Four unit tests in `update.rs`. They cover the default policy, verify, symlink, and delimiter collision. There are no integration tests, no init rejection test, and no always-verify test.

**Summary honesty:** Very candid. It admits dprint was not run, admits init rejection was not exercised, and reports a flaky earlier run.

## Candidate C: overall 8/10

**Verdict:** A complete fix with a careful encoding. Its integration tests cover all three policies for every attack. It documents its main cost honestly: default `status` now reads changed files.

| Requirement | Verdict | Evidence |
|---|---|---|
| M? second edit rejected | met | M? files checksummed for the fingerprint; S7 rejects same-metadata swap |
| `--verify` M second edit rejected | met | hidden + own all-policy tests |
| Added file edit rejected | met | added files checksummed in Display; init tests across policies |
| Symlink re-point rejected | met | target in new state |
| Existing mismatch error, nothing written | met | tests check `.treeward` unchanged / absent |
| Injective encoding | met | record marker, framed fields, tags, domain prefix |
| Parity under all policies and kinds | met | hash built in purpose-independent deterministic traversal; S3, S5, own mixed-kinds test with type change |
| Touch-only accepted | met | hidden parity_touch_only |

**Bugs**
- minor (verified, S1/S2): same cost as A. Default `status` reads added and M? files, and errors on unreadable ones. Here the cost is documented in help and README.
- minor: the README rewrite dropped the existing "two purposes" (TOCTOU / idempotency) list. Nobody asked for that removal.

**Scope:** The extensive help and README rewrites are justified by the behavior change. Removing the list was not.

**Tests:** Integration tests over default, `--verify`, and `--always-verify`. They cover rewritten modified files, re-targeted symlinks, changed added files, init accept/reject, and mixed entry kinds. One unit test covers encoding framing.

**Summary honesty:** Accurate. Its claims match objective.json.

## Ranking

1. **C** (8)
2. **A** (7)
3. **B** (7)

None of the candidates has a critical or major bug.

## Comparison

All three meet every stated requirement with an injective encoding and status/update parity. A and C chose a stronger guarantee: they bind content even under the default policy. The price is that default `status` stops being metadata-only. C documented that tradeoff thoroughly and honestly. A left contradicting "fast metadata-only" docs in place, reversed a guarding test, and misstated touch behavior in its summary. B matches the reference most closely and keeps the default command cheap, but its tests are thin, removals bind no prior state, and its README fails dprint. A's much stronger test suite puts it ahead of B.