docs: cap the review loop with a round budget; prove mid-loop fixes

[vivek7405]

Problem

The self-review loop has no cost ceiling and no proof discipline for its own repair work, and the two problems feed each other. Evidence from a live loop on a security-fix PR (2026-07-29, running 18 rounds): round 17 found that two of its five findings were defects introduced by round 16's fixes (a rejection handler missing the render-token check its siblings had, and a loading.${''}js literal typo published into docs), and the round-14 regression test had silently become non-discriminating one commit later, because a later fix changed the mechanism so reverting the original try/catch alone no longer redded anything, while the PR body still claimed it did. The loop was correctly catching its own repair work, but each catch cost a full round, and nothing bounded how long that could continue. The owner's constraint is explicit: long review cycles are unaffordable.

#1171 / #1173 fixed the OTHER unbounded axis (prose relitigation, via the substantive gate and the one bounded prose pass). Repair-regression rounds are substantive by definition, so that gate does not and should not end them. The fix for this axis is upstream: make mid-loop fixes cheap to prove correct, and cap the autonomous round count so a pathological PR stops and reports instead of looping.

Design / approach

Two mechanisms, both in .claude/skills/webjs-start-work/SKILL.md, both aimed at cost.

1. A hard round budget with a report-and-stop exit. After N substantive rounds (default 5; a user instruction can raise it per task), the loop STOPS, posts the round's findings as usual, withholds the flip to ready for review, and reports to the user: rounds run, findings open, and the loop's own read on why it is not converging (repair regressions, a genuinely deep surface, reviewer quality). This is a third exit alongside converged and BLOCKED, wired into the same taxonomy. Prior art: the 18-round loop above, asked to stop, independently improvised this exact shape (let the in-flight round land, fix only a real behavior bug from it, record prose findings without another round, file everything else, hand the merge decision to the user), which is evidence the shape is the natural one to codify. Rationale: an over-budget loop is signal about the CHANGE (too big, too entangled) or the FIXES (undisciplined), and the user deciding what to do next is cheaper than five more autonomous rounds. The existing "never interrupt the user mid-loop" rule stays true for spawn failures; the budget stop is not an interruption mid-round but a bounded end.
2. Mid-loop fixes get the original change's counterfactual discipline, cheaply. (a) A fix commit that changes shipped-source behavior runs the same commit-first red/green toggle the Definition of done mandates for the original change (the toggle costs seconds; the repair-regression round it prevents costs a full reviewer). (b) A behavioral fix to a surface an existing test claims to cover must re-run that test's counterfactual, because a fix can make an older test non-discriminating without failing it (the round-14/16 incident above). (c) Any PR-body claim of the form "reverting X reds Y" is dated by the commit it was true at, and a later commit touching the same mechanism either re-verifies or restates it. Stale-true-then-false claims are worse than no claim.

Explicitly out of scope: changing the substantive gate, the prose pass, or deep-review's cost profile (its maxAgents knob already exists). If round-1 cost itself becomes the complaint, that is a separate issue.

Implementation notes (for the implementing agent)

• Land AFTER docs: exit the review loop on a substantive gate plus one prose pass #1173 merges and edit on top of its text. docs: exit the review loop on a substantive gate plus one prose pass #1173 (branch docs/two-tier-review-loop-exit, PR open at filing time) rewrote the exit condition; these line refs are against that branch's state of .claude/skills/webjs-start-work/SKILL.md.
• Where to edit, all in ### How the loop works unless noted:
  • The loop intro (~L311, "Run rounds of self-review until ONE round finds nothing SUBSTANTIVE") gains the budget: "or the round budget (default 5 substantive rounds) is spent, which ends the loop as OVER BUDGET, reported to the user, never silently".
  • Step 3 (~L363) and the "LAST round" absolute (~L379): the budget is checked when a round's substantive findings would trigger the next delta round; the absolute's exit taxonomy ("The ONE case that ends the loop without converging") becomes two cases (BLOCKED, OVER BUDGET) and must be reworded from "ONE".
  • ### Reporting after the loop (~L387): add the over-budget report shape: rounds run, open findings with their PR-review links, the loop's read on the non-convergence cause, and NO "ready to merge". The <K> counting rule applies unchanged.
  • ## Failure handling (~L469): add the over-budget entry next to the blocked-reviewer one. The flip to ready is withheld, branch and card stay as they are.
  • The loop-speed bullet "No full test suite inside the loop" (~L315): add that the per-fix scope INCLUDES the red/green counterfactual toggle for a behavioral shipped-source fix, stated as being inside the seconds-per-fix cost envelope, so the bullet cannot be read as license to skip it (that reading is how the round-16 regressions shipped).
  • Step 2's "Fix it on the branch" bullet (~L365) and the Definition of done's counterfactual block (~L120-131): cross-reference each other; add the claim-freshness rule ("reverting X reds Y" claims are dated and re-verified when a later commit touches the mechanism) to the counterfactual block, since that is where a cold agent looks up counterfactual mechanics.
• Landmines / gotchas:
  • Both copies of the skill must end byte-identical: in-repo and ~/.claude/skills/webjs-start-work/SKILL.md (diff -q), the docs: make the self-review loop fast, parallel, and stall-proof #1159 rule.
  • test/hooks/review-loop-exit.test.mjs (added by docs: exit the review loop on a substantive gate plus one prose pass #1173) pins load-bearing exit-condition anchors with regexes, including "The ONE case that ends the loop without converging"-adjacent phrasings; rewording the exit taxonomy WILL red it until its anchors are updated, which is intended (update the anchors with the wording, and add anchors for the budget rule).
  • The routed directive in .claude/hooks/route-skills.sh (~L190) restates the loop exit for review prompts and must stay in lockstep (the same test asserts its phrases); decide whether the budget belongs in the directive (recommended: one clause) and keep bash -n green.
  • AGENTS.md invariant 11 (banned glyphs, WebJs casing) applies; the block-prose-punctuation.sh hook scans new content.
  • Do not weaken "Never stop mid-loop to report a failed spawn" (~L345): spawn failures still re-spawn silently; only the round BUDGET stops the loop.
• Tests + docs surfaces: node --test test/hooks/review-loop-exit.test.mjs test/hooks/route-skills.test.mjs green, with the review-loop-exit anchors extended for the budget text (counterfactual: revert the skill hunks, expect red). No packages/*/src change, no docs-site/scaffold/MCP surfaces (same N/A rationale as docs: make the self-review loop fast, parallel, and stall-proof #1159/docs: exit the review loop on a substantive gate plus one prose pass #1173).

Acceptance criteria

• The loop has three exits, each with a defined report: converged (nothing substantive, then the prose pass), BLOCKED (unproducible reviewer), OVER BUDGET (default 5 substantive rounds spent; user-raisable per task)
• An over-budget loop reports rounds run, open findings, and a non-convergence read, and never flips the PR to ready or says "ready to merge"
• A behavioral shipped-source fix commit inside the loop runs the commit-first red/green toggle, stated inside the loop-speed bullet so speed cannot be read as license to skip it
• A fix touching a surface an existing test claims to cover re-runs that test's counterfactual (the non-discriminating-test case)
• PR-body counterfactual claims are dated and re-verified or restated when a later commit touches their mechanism
• test/hooks/review-loop-exit.test.mjs anchors the new budget wording (revert reds it) and stays green with route-skills.test.mjs
• In-repo and ~/.claude skill copies byte-identical; router directive in lockstep and bash -n green