review: findings widen one invariant across rounds and burn the round cap

[justinhelmer]

What happened

PR #2164 (fix for #2152: the coding child's push guard must certify the pushed changed set) took eight review verdicts across two ship attempts, and the second attempt ended at the round cap with the PR still not merge-ready. From 22:20Z on, four consecutive Changes requested verdicts each named a new case of the same invariant, at the same site (src/core/harness/pi/toolRules.ts):

Verdict (UTC)	Head	Finding	Line
21:22	ce8a962	a receipt stays valid across an untracked HEAD change or a later failed formatter	279, 302
22:20	57812d3	a compound push can mutate the tree after it is checked; dirty receipts do not identify the checked contents	329, 290
22:31	785cccc	an explicit refspec can push a tree other than the one that earned the receipt	392
22:45	7d9719f	only the first refspec is checked; a multi-refspec push publishes unchecked trees	407
22:55	ff0b416	configured remote.<name>.push refspecs let an omitted-refspec push publish unchecked trees	420

The ship parent (run 9c396421, thread key slack:C0BRRHKFLCB:1790021390.458049) ended with round cap reached at 22:55Z. Each round cost a fix child plus a review (about 10–15 minutes and roughly $1), and the attempt had to be recovered by hand with a directive coding run and a re-issue.

Every one of those findings is a case of one invariant: a push publishes only trees that earned the receipt, for every way the pushed tree can be selected (the checkout at the time of the check, each explicit refspec's source, each configured refspec's source, and the omitted-refspec default). The first Changes requested at that site could have named the invariant and its cases; instead each round widened the previous finding by one case, and the cap counted each widening as a full round.

Why it matters

• The round cap exists to stop a unit that is not converging. A unit that fixes exactly what it was told and is then told one more case each round is converging on the reviewer's schedule, and the cap ends it anyway.
• The cost is paid per round: a fix child, a review, a CI run and a wind-down slot each time.
• The recovery is manual (a directive fix and a re-issue), which is the path 0073 retires.

Product shape (either, or both)

1. A finding names its invariant and all the cases the reviewer can see. When a review requests changes at a site, its finding states the invariant the code must hold and enumerates the cases it checked, not the first failing case only. The review agent's spec (agent-review.md) gets the row; the verdict block carries the invariant so a later round can be checked against it.
2. A round that only widens a prior finding does not count toward the cap. A Changes requested whose every finding is at the same site as a prior round's finding and names the same invariant is a continuation: the fix round runs, the cap is not decremented, and the card says so ("widened F3; round cap unchanged").

The first shape prevents the pattern; the second stops it from ending a unit. Spec home: docs/reference/specs/agent-review.md (finding completeness) and agent-ship.md (what the cap counts).

Receipt runnable when

worker:bot ≥ <merge sha> and event: a PR whose second Changes requested verdict names the same file and invariant as the first; the card shows the round as a continuation, or the first verdict already listed the later case.

[justinhelmer]

Sixth round on the same site, 23:14Z (attempt 3, round 1, head cd30118, after the configured-refspec fix): Changes requested — configured bare-push destinations can bypass the run's branch and protected-branch policy.

F1: inspectGitPushSources discards each configured refspec’s destination. For example, remote.origin.push=HEAD:main followed by git push origin validates the formatted HEAD tree and is allowed without applying the own-branch/protected-branch checks to main. Preserve and validate every configured destination—or fail closed—before authorizing the push.

The widening moved from which trees a push publishes to which destinations a configured refspec may name; both are cases of the one invariant the first round could have stated (a push publishes only gated trees to the run's own branch, for every way the refspec resolves).

[justinhelmer]

Seventh round on the same site, 23:31Z (attempt 3, round 2, head 12328f9): Changes requested — configured push URLs still let literal origin publish the gated tree outside the run's repo (inspectGitPushSources checks refspecs, mirror mode, follow-tags and push.default, never remote.origin.pushurl; F2 major at toolRules.ts:347).

Two facts for the product shape: (a) the operator had steered the fix round at 23:18Z to close the whole invariant and listed remote.<name>.pushurl and a second remote among the cases; the child's push still omitted it, so a folded steer is not a completeness guarantee — the enumeration has to be a table the reviewer checks rows against, not prose the child reads; (b) the reviewer again named one missing case rather than the table, so shape 1 (the finding names the invariant and every case it checked) still has no owner in the round.

[justinhelmer]

Eighth round on the same site, 23:46Z (attempt 3, round 3, head faa1f00): Changes requested — the pushurl fix protects only bare pushes; explicit refspec pushes can still be redirected. The next verdict is the attempt's last before the round cap.

[justinhelmer]

Ninth round, 23:56Z (attempt 3, round 4, head 7b8734a): Changes requested — the pushurl path is fixed, but origin's ordinary URL can still be redirected after the receipt is earned. Attempt 3 ended at the round cap. The unit is parked until after the 1.260.3 roll; the next attempt starts from a directive fix that carries the enumeration table (source × destination × remote URL) rather than one case.

[justinhelmer]

Tenth round on the same site, 06:14Z (attempt 4, round 1, head 863394c — after the whole-invariant table landed for sources, destinations and remotes): Changes requested — a backgrounded or piped push can still race a mutation after the guard's tree inspection (pushCanFollowAnotherCommand checks only the text before the push; git push origin HEAD:<ref> & npm version patch passes; F1 major at toolRules.ts:298). A new axis this time (command composition) rather than a new case of the last one; the operator steered the fix round to enumerate every compound form (&, |, ||, ;, subshell/group, eval, xargs, sh -c, multiple pushes) so the eleventh round has nothing left.