# Queue Piece D review, round 2 (#1508, row 12)

Filbert, 2026-09-27. Round 1: `queue-d-review-r1-2026-09-27.md`
(sha256 a2dc2302…). This round checks C1 only, per Sage's ruling to fix it
in D.

## Verdict

**Approved.** The review covers `build.patch` sha256
8a19f7fa03e2ee6c3bd9ce7b2e296acb21655471c4cc56ba19f25464c8129209 (base
8ffbd73b, applied at cdcedb27), with `build-manifest.sha256` 647d0170…
and `build.md` e77c8c5e…. The C1 fix is correct. One mutant of mine
survives: an approval from an earlier round counted in the current one.
The code handles that case correctly; the tests just don't check it. n1
gives a four-line test that kills the mutant. I'd add it before the
commit, and it doesn't need a round 3. I checked it myself (below).

## What I checked

All of this ran in a scratch clone, `/tmp/fqd2`, at cdcedb27 with push
disabled, on frozen 0444 copies of the three inputs.

- **Manifest and suites.** The manifest checks 10/10. `node --test
  packages/queue/tests/` passes 142/142, and the ledger tests pass 51/51
  (Darkwing's 58 includes the helper patch, which I didn't apply).
  `scripts/test-queue.sh` reports 27/0 with the "outside the canonical
  root" skip line.
- **What changed.** I compared all ten manifest files with round 1's tree.
  Only `queue.mjs`, `README.md` and `review.test.mjs` differ, as Darkwing
  said.
- **The fix in `queue.mjs`.**
  - `requireApprovals` is the round-1 inline check moved into a helper.
    in-review→done's messages are unchanged.
  - in-review→in-progress still checks only `ownerOrPriv`, which is right
    for the changes path.
  - in-review→waiting-on-jason runs `ownerOrPriv`, then `refuseUnresolved`,
    then `requireApprovals` when the last round is a comment round.
    `unresolvedAttempts` skips rounds that aren't comment rounds, and v1
    entries can't open one. So v1 replay is unaffected.
  - There's no exemption for jason or sage, as agreed.
- **The tests.** The new test runs my round-1 reproduction and now
  refuses at the step that used to pass. It covers a reviewer who asked
  for changes, a second round, and reviewers cleared after the round
  opened. The rewritten unresolved test keeps waiting-on-jason→done
  covered through a late post that turns an abandoned attempt into a
  conflict. That's a better route than the old one.
- **The README** matches the code.
- **Mutations.** I wrote eight mutants of my own for this round. The
  suite kills seven:
  - G1: sage bypasses the approval check on waiting-on-jason;
  - G2: a `changes` verdict counts as an approval;
  - G3: the check reads the first round, not the last;
  - G4: one approval is enough;
  - G5: the owner-or-privileged check dropped from waiting-on-jason;
  - G6: the unresolved check dropped from waiting-on-jason→done;
  - G7: the approval check dropped from in-review→done.

  G8 survives: approvals are collected from every round's receipts
  instead of the current round's. See n1.
- **My plan.** Table line 1470 and 8.9's Receipts paragraph (plan sha256
  293747cd…) describe the code as built.

## Non-blocking

- **n1. Add a test that an earlier round's approval doesn't count (kills
  G8).** In the new test, filbert approves round 1 and rocko asks for
  changes. In round 2 both approve before anything checks, so an old
  approval counting in round 2 goes unnoticed. In
  `packages/queue/tests/review.test.mjs`, replace the round-2 `for` loop
  (three lines) with:

  ```js
  ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "91", "--candidate", head, "--op", "record-9-rock02"], "rocko"));
  // Filbert's round 1 approval doesn't carry into round 2.
  no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by filbert$/m);
  ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "90", "--candidate", head, "--op", "record-9-filb02"], "filbert"));
  ```

  I checked it in a copy of the scratch clone: `review.test.mjs` passes
  20/20 on the candidate and fails 1 under G8.
- **n2. An owner who is also a listed reviewer blocks the row.** `set
  reviewers` and `assign` don't stop the owner from being on the list.
  `record` refuses the owner, but `requireApprovals` still waits for the
  owner's approval. The row then can't reach done or waiting-on-jason
  until a privileged actor runs `set reviewers`. That fails closed, and it
  was already true of done in round 1. The refusal names the owner as
  missing, which could confuse. Don't fix it by dropping the owner from
  the required list: with reviewers `[owner]`, that list would be empty
  and the row would pass with no review. A later fix could refuse the
  owner at `set reviewers` and `assign`. That's a follow-up, not D.