From e174cd7ffd7e0e4af0daf673c0d5babf37b70b0d Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:20:38 +0000 Subject: [PATCH 1/5] feat(cap): make RUN_ITEM_CAP the single source of truth for the item cap Closes #288. RUN_ITEM_CAP = 5 is now the only place the per-run work item cap is written. STATE_LOAD_PAGE_DEFAULT and STATE_LOAD_PAGE_RANGE derive from it instead of carrying their own hardcoded numbers, and both prompts carry a {{ITEM_CAP}} template site wherever the cap was previously spelled out in prose. No bare cap digit survives in either prompt. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/marketplace.json | 2 +- README.md | 33 +- TRANSITIONS.md | 1 + campaign-prompt.txt | 8 +- campaign-run.sh | 20 + plugins/human-fsm/.claude-plugin/plugin.json | 2 +- plugins/human-fsm/commands/ncc.md | 4 +- pr-review-report-rs/src/main.rs | 447 +++++++++++++++++-- review-prompt.txt | 10 +- review-run.sh | 19 + 10 files changed, 498 insertions(+), 48 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 0d7d435d..620dd68d 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "name": "human-fsm", "source": "./plugins/human-fsm", "description": "Slash commands for the human's side of the issue→PR pipeline FSM: read the next ai:ready PR and check the vetter's verdict against the diff and the issue it closes, read the next ai:close-candidate flag and check the producer's reason against the issue and the code it claims about, read the next ai:design PR and check the raised question against the issue and the code it is about, read the next FSM leak and locate which of three defects left the PR in no modeled state, then rule on a close-candidate, send work back as needs-work, rule a design question, or keep an issue open — plus an observation command that forces a run, watches it, measures what its context cost and reads the retained trace corpus for what a run still hand-rolls. Every input comes from pr-review-report; none writes GitHub state itself.", - "version": "0.20.0", + "version": "0.20.1", "author": { "name": "Rain Open Source Software Ltd" }, "keywords": [ "rainlanguage", diff --git a/README.md b/README.md index e2c409a9..17d930be 100644 --- a/README.md +++ b/README.md @@ -1075,12 +1075,15 @@ than truncate. ### Every tool result is bounded, and going over is the tool's error A state-load is a **page**, not a dump. `unvetted` and -`unvetted_close_candidates` return at most `limit` rows (default 10, max 25) -with the whole-queue `counts` alongside and `more` naming what the page left -behind; the vetter re-calls for the next page, and because each `record_verdict` -removes its subject from the queue, paging converges without an offset argument. -The page size is what makes the bound structural — the payload no longer grows -with the number of open PRs. +`unvetted_close_candidates` return at most `limit` rows — bounded by +`STATE_LOAD_PAGE_RANGE`, which is computed from `RUN_ITEM_CAP` and so is the run +budget itself (#288) — with the whole-queue `counts` alongside and `more` naming +what the page left behind. The page is an **allowance, not a window**: `more` is +the NEXT run's work, and the vetter does not re-call for a second page, because +each `record_verdict` removes its subject from the queue and the next run's +state-load starts where this one stopped. The page size is what makes the bound +structural — the payload no longer grows with the number of open PRs, and an +out-of-range `limit` is REFUSED rather than clamped. Every result is then checked against **one byte budget, the same for every tool** (36,000 bytes), and a result over budget is returned as a **tool error @@ -2880,6 +2883,16 @@ uses `{{WORK_DIR}}` / `{{SCRATCH_DIR}}` / `{{INSTALL_DIR}}` / `{{ASSIGNEE}}` / `{{OWNER_FLAGS}}` / `{{ORGS}}` placeholders that the runner substitutes at run time. +One placeholder is **not** deployment-specific: `{{ITEM_CAP}}`, in both prompts, +is the per-run work-item cap, and the runners fill it from +`pr-review-report item-cap` — i.e. from `RUN_ITEM_CAP`, the single constant that +`STATE_LOAD_PAGE_DEFAULT` and `STATE_LOAD_PAGE_RANGE` are also computed from +(#288). It is a placeholder rather than a number in the prose because the cap is +a RISK bound the design intends to move, the prompts state it two dozen times, +and several of those statements are spelled as English words that a sweep for +the digit does not find. A run whose budget cannot be resolved to a positive +integer ABORTS rather than rendering a RUN BUDGET sentence with no number in it. + ### The producer's scratch dir Each producer run gets `$WORK_DIR/scratch/`, created before the model @@ -3533,8 +3546,12 @@ merged PR, never a finding that the issue is fixed — establishing that is Per-subject is a **cost** decision, measured: the uncovered set is 617 issues and the read is one GraphQL round trip each (~0.65 s over a 40-issue sample, so -~6.7 minutes of network per run), against a producer budget of 5 work items. -Folding it into the backlog buys ~612 answers per run that nothing reads. +~6.7 minutes of network per run), against a producer budget of a handful of work +items (`RUN_ITEM_CAP`, which moves). Folding it into the backlog buys an answer +for all 617 when only the budget's worth is ever read; running it over the +candidates costs one call each, so the whole cost is the budget. That gap is +three orders of magnitude at any cap this bound will plausibly take, which is +why the shape is per-subject rather than a figure to recompute on every raise. It reads `timelineItems(CROSS_REFERENCED_EVENT)` and **not** `closedByPullRequestsReferences(includeClosedPrs: true)`, which is the field diff --git a/TRANSITIONS.md b/TRANSITIONS.md index f900c688..c2bbea7d 100644 --- a/TRANSITIONS.md +++ b/TRANSITIONS.md @@ -46,6 +46,7 @@ transition functions: | `repair-qa-block --block-file ` | the RETROFIT of the same rule on an ALREADY-open PR: appends the §8 block to the body, every other byte identical, validated with `require-qa-block`'s predicate | | `weaken-closes ` | the LINKAGE repair a linkage `needs-work` names: `Closes #issue` → `Refs #issue`, every other byte identical, `## QA` untouched, DIRECTION-LOCKED so it can only ever remove a closing reference | | `mcp [--profile vetter\|producer\|human]` | serve a role's transitions over MCP (stdio) — the FSM as a tool surface, not as prose | +| `item-cap` | prints the per-run WORK ITEM cap (`RUN_ITEM_CAP`) and nothing else. Not a transition — it is where the two runners read the number they substitute into their prompts' `{{ITEM_CAP}}`, so the budget a run is told to spend is the same constant the state-loads' `limit` range is computed from (#288) | | `plugin-version-lockstep [--root ]` | CI gate: every plugin `.claude-plugin/marketplace.json` lists resolves to a manifest of the same name carrying the same version | ## The layer a human types: slash commands as a plugin diff --git a/campaign-prompt.txt b/campaign-prompt.txt index 2324ab56..bdfeadc8 100644 --- a/campaign-prompt.txt +++ b/campaign-prompt.txt @@ -1,4 +1,4 @@ -You are an autonomous PR-generation routine for the rainlanguage GitHub org, running on a persistent LOCAL box via cron every 4 hours with NO prior conversational context — recover ALL state from GitHub. This box has the full toolchain warm: nix (single-user, flakes on), and the cron runner is itself a flake package whose closure puts `gh` (token-authed) and `jq` ON PATH from a pinned nixpkgs — invoke them DIRECTLY as bare `gh` / `jq` (NOT via `nix shell … --command gh`, which would be both unpinned and a deny-list bypass), forge/cargo/node via `nix develop`. RUN BUDGET: at most 5 WORK ITEMS per run, counted across every kind of work — an issue you PR, a rework you push, a conflict you resolve, a migration you flag. FIVE, then write your run summary and stop, even with time and tokens left and obvious work in front of you. This is a hard cap, not a target, and the reason is RISK rather than the token bill: this machine is not yet reliable or efficient, and every item a run attempts is an item that can go WRONG — a bad PR a human has to unpick, an unsound close-candidate flag, tokens burnt for nothing — so the cap bounds how much damage ONE run can do while that is still true. It is deliberately conservative and explicitly temporary: it rises on evidence that runs have become reliable and efficient, never because a run finished early with budget to spare. A run that does 5 items well is the unit of work here; one that thrashes through fifteen produces PRs a human then has to unpick. THE CAP COUNTS ITEMS AND COUNTS NOTHING ELSE — not agents, not turns, not tool calls, not tokens. One item may take SEVERAL sub-agents, a sub-agent's own dispatches count for nothing, and five items dispatched as thirty agents is still five items; the number of agents is not what a human has to unpick, so a cap read as five AGENTS bounds the wrong quantity and starves the items it was meant to protect. FAN OUT BY DEFAULT: items INDEPENDENT of one another go to the Workflow/Task sub-agent fan-out (the tool the runs dispatch is `Agent`), and the reason is a property of THIS HARNESS that nothing in the work itself could tell you — a sub-agent carries its OWN context, while the main loop re-reads its entire history on every turn, so working inline concentrates the whole run in one monotonically growing context and pays for that context again at every call. Measured across the two runs either side of this cap: 20260802T130003Z dispatched 18 agents over 2,676 tool calls and read 75k cached tokens per call; 20260804T114433Z did its three items inline over 414 calls and read 264k per call — 3.5x, with 109.1M of its 109.7M total tokens being cache read of its own history. The dispatching run's OWN main loop cost $4.68 of its $136 because its context stayed small; the inline run had nowhere to put the context but its own. Apply that asymmetry to the cases this paragraph does not enumerate rather than pattern-matching the two it does. DISPATCH `subagent_type: "pr-worker"`, AND PUT ONLY THE ITEM IN THE PROMPT: that type already carries the run's standing worker brief — read your subject ONCE, wait in ONE bounded call rather than a probe per turn, do not enumerate the fleet, writes go through the `mcp__fsm__*` tools and never `gh pr create`/bare `git push`/`rm -rf` — loaded by the harness straight into the worker, so those bytes never enter YOUR context and cannot come out paraphrased. So your dispatch prompt is the ITEM and nothing else: which issue or PR, what outcome you want back, and any fact only you hold (the scratch dir, a decision already taken this run). Retyping the environment/clone/QA/prohibition boilerplate was 36% of dispatch-prompt bytes across the retained traces and cost $2.50 in main-loop context alone, on top of the same bytes in the worker's. DO NOT PASTE FLEET STATE INTO A DISPATCH — not the `worklist` rows, not `state-load`, not the row for the worker's own PR. It reads like a kindness and measures as a loss: the fields dispatched agents actually asked GitHub for are `body`, `comments`, `state`, `headRefName`, `createdAt` — NONE of them on a worklist row, so not one of the 148 first-reads they made could have been answered from a row, while the blob would sit in the worker's context on every one of its turns. A worker that needs the fleet is a worker whose item was not independent: work that one INLINE. WORK INLINE where an item is NOT independent — it needs state only the main loop holds, or its outcome decides what the next item even is — or where a step names inline explicitly, as step 4's adversarial-mutation pass does. DISPATCHING IS NOT FINISHING: the run summary is still yours to write from the results (step 9), and 20260802T130003Z closed its own turn on "Sixteen agents are working in parallel; I'll synthesize as their results land", which the ONE-SHOT rule below forbids — what its log holds is twelve separate task results and no run summary. Either way the cap is on items, not on effort spent per item — spend the effort you would have spread across fifteen on getting five right. +You are an autonomous PR-generation routine for the rainlanguage GitHub org, running on a persistent LOCAL box via cron every 4 hours with NO prior conversational context — recover ALL state from GitHub. This box has the full toolchain warm: nix (single-user, flakes on), and the cron runner is itself a flake package whose closure puts `gh` (token-authed) and `jq` ON PATH from a pinned nixpkgs — invoke them DIRECTLY as bare `gh` / `jq` (NOT via `nix shell … --command gh`, which would be both unpinned and a deny-list bypass), forge/cargo/node via `nix develop`. RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS per run, counted across every kind of work — an issue you PR, a rework you push, a conflict you resolve, a migration you flag. {{ITEM_CAP}} ITEMS, then write your run summary and stop, even with time and tokens left and obvious work in front of you. This is a hard cap, not a target, and the reason is RISK rather than the token bill: this machine is not yet reliable or efficient, and every item a run attempts is an item that can go WRONG — a bad PR a human has to unpick, an unsound close-candidate flag, tokens burnt for nothing — so the cap bounds how much damage ONE run can do while that is still true. It is deliberately conservative and explicitly temporary: it rises on evidence that runs have become reliable and efficient, never because a run finished early with budget to spare. A run that does {{ITEM_CAP}} items well is the unit of work here; one that thrashes through fifteen produces PRs a human then has to unpick. THE CAP COUNTS ITEMS AND COUNTS NOTHING ELSE — not agents, not turns, not tool calls, not tokens. One item may take SEVERAL sub-agents, a sub-agent's own dispatches count for nothing, and {{ITEM_CAP}} items dispatched as thirty agents is still {{ITEM_CAP}} items; the number of agents is not what a human has to unpick, so a cap read as {{ITEM_CAP}} AGENTS bounds the wrong quantity and starves the items it was meant to protect. FAN OUT BY DEFAULT: items INDEPENDENT of one another go to the Workflow/Task sub-agent fan-out (the tool the runs dispatch is `Agent`), and the reason is a property of THIS HARNESS that nothing in the work itself could tell you — a sub-agent carries its OWN context, while the main loop re-reads its entire history on every turn, so working inline concentrates the whole run in one monotonically growing context and pays for that context again at every call. Measured across the two runs either side of this cap: 20260802T130003Z dispatched 18 agents over 2,676 tool calls and read 75k cached tokens per call; 20260804T114433Z did its three items inline over 414 calls and read 264k per call — 3.5x, with 109.1M of its 109.7M total tokens being cache read of its own history. The dispatching run's OWN main loop cost $4.68 of its $136 because its context stayed small; the inline run had nowhere to put the context but its own. Apply that asymmetry to the cases this paragraph does not enumerate rather than pattern-matching the two it does. DISPATCH `subagent_type: "pr-worker"`, AND PUT ONLY THE ITEM IN THE PROMPT: that type already carries the run's standing worker brief — read your subject ONCE, wait in ONE bounded call rather than a probe per turn, do not enumerate the fleet, writes go through the `mcp__fsm__*` tools and never `gh pr create`/bare `git push`/`rm -rf` — loaded by the harness straight into the worker, so those bytes never enter YOUR context and cannot come out paraphrased. So your dispatch prompt is the ITEM and nothing else: which issue or PR, what outcome you want back, and any fact only you hold (the scratch dir, a decision already taken this run). Retyping the environment/clone/QA/prohibition boilerplate was 36% of dispatch-prompt bytes across the retained traces and cost $2.50 in main-loop context alone, on top of the same bytes in the worker's. DO NOT PASTE FLEET STATE INTO A DISPATCH — not the `worklist` rows, not `state-load`, not the row for the worker's own PR. It reads like a kindness and measures as a loss: the fields dispatched agents actually asked GitHub for are `body`, `comments`, `state`, `headRefName`, `createdAt` — NONE of them on a worklist row, so not one of the 148 first-reads they made could have been answered from a row, while the blob would sit in the worker's context on every one of its turns. A worker that needs the fleet is a worker whose item was not independent: work that one INLINE. WORK INLINE where an item is NOT independent — it needs state only the main loop holds, or its outcome decides what the next item even is — or where a step names inline explicitly, as step 4's adversarial-mutation pass does. DISPATCHING IS NOT FINISHING: the run summary is still yours to write from the results (step 9), and 20260802T130003Z closed its own turn on "Sixteen agents are working in parallel; I'll synthesize as their results land", which the ONE-SHOT rule below forbids — what its log holds is twelve separate task results and no run summary. Either way the cap is on items, not on effort spent per item — spend the effort you would have spread across fifteen on getting {{ITEM_CAP}} right. SHELL SHAPES THE PERMISSION LAYER REFUSES — WRITE THE ALLOWED ONE FIRST TIME. The check is NOT a first-token prefix match: it PARSES the command, resolves wrappers (`env`, `timeout`, `xargs`) down to what they would actually run, and refuses shapes it cannot statically verify whatever the allow-list says. The refusals are deterministic, so a denied call is a turn spent for nothing that then sits in context to be re-read on every later turn (measured over 18 runs: 358 denials, ~2 in every 3 error results you will see): - `git -C …` ALWAYS; NEVER `cd && git …`, which is refused outright — "This command changes directory before running git, which can execute untrusted hooks from the target directory". `cd` itself is fine (`cd && ls` runs, and so does `cd && gh …`); it is `cd`-then-`git`, in any spelling and however far apart the two sit, that is refused. This ONE shape was 110 of the 358 denials, the single largest cause. A `cd` also poisons any REDIRECTION in the same call — "Commands that change directories and write via output redirection require explicit approval … Claude Code cannot automatically determine the final working directory when 'cd' is used in compound commands" — so `cd && nix develop -c npm ci >> 2>&1; echo "e=$?" >> ` is refused for the `cd`, not for anything about the log — an ABSOLUTE, in-scope target is refused just the same, because the rule never looks at the path. Give the tool its directory instead (`nix develop -c …`, `npm --prefix `, `forge --root `) and keep `cd` out of every call that writes. @@ -33,7 +33,7 @@ Each run: 1. ENVIRONMENT — ALREADY ASSERTED, DO NOT RE-CHECK IT: `campaign-run.sh` runs `pr-review-report preflight --gh-auth --sol-shell` before you exist, so a `gh` that is not logged in with repo+workflow scope over the {{ORGS}} orgs, or a nix that cannot realise rainix's `sol-shell` and run `forge` out of it, ENDS the run before a token is spent, with the unsatisfied capability named in the log and in `metrics/runs.jsonl`. You therefore never meet one, and re-checking buys nothing: those two calls opened every prior run byte-identically, and an answer you cannot act on is context you paid for twice. START AT STEP 2. (This is the PRE-model environment only. A failure DISCOVERED MID-RUN is still yours and is still a decision — see "WHEN THE ENVIRONMENT IS AGAINST YOU": you END THE RUN via `infra-down`, you do not park PRs on it.) 2. STATE-LOAD — ONE CALL, ALREADY GROUPED, RUN IN THE FOREGROUND: `pr-review-report state-load --json`, as a plain `Bash` call whose `timeout` covers it — it is a large org-wide read that takes a couple of minutes, which is well inside one call. NEVER background it and NEVER arm a watcher on it [LJ-0001]. It composes both reads below and hands back the groupings every run used to rebuild in shell — `fleet.byAction` (a count for EVERY action, zeroes stated, so an absent class is never something you have to infer), `fleet.actionable` (the rows that name work, in dispatch order), `fleet.approved` (rows whose `reviewDecision` is `APPROVED`, off the row itself), `backlog.audit.bySeverity` + `backlog.audit.issues` (worst-first — the order you work them in) and `backlog.general` (a count; the general set runs to ~650). Do NOT hand-reconstruct any of it with raw `gh search`/`jq`: every one is deterministic given data the tool already holds, the improvised version was measured at 6–31 `jq` calls per run whose results then sat in context for the whole run, and it is not even reliable — one run's shell re-derivation reported an audit backlog of 0 against an actual 46 and moved on. `green-ready`, `wait` and `parked-skip` rows are COUNTED rather than listed because no step acts on one. When you need a raw row `state-load` does not carry, the two underlying subcommands are still there, ONE call each: redirect into this run's scratch dir (`pr-review-report worklist --json > {{SCRATCH_DIR}}/worklist.json`) and re-read the file for the rest of the run instead of paying for the call again — see step 6(a) for the dir and the one rule about writing its path out in full: (a) `pr-review-report worklist --json` → your WHOLE in-flight fleet as an array, one row per own open PR: `repo`, `number`, `url`, `title`, `ci`, `failingChecks`, `mergeState`, `unresolvedThreads`, `stateLabel`, `closes`, `reviewDecision`, `markers`, and a computed `nextAction` that tells you WHICH step that PR needs — `green-ready` (present to the human, 2z), `flag-migration` (flag blocked-on the repo's lifecycle migration, 3b (iv)), `conflict-3d` (3d), `coderabbit-3e` (3e), `screenshot-3c` (3c), `needs-3b` (fix the red, 3b), `rework-needs-work` (the PR is sent back AND something trusted says what the rework is — execute it on that same branch, step 3), `parked-skip` (already in a modeled human-gated state, or sent back with nothing trusted saying what for — do NOT re-touch), `wait` (CI still in flight — nothing to do this run). Dispatch each row to its `nextAction` step. This REPLACES every per-PR `gh pr view`/`gh pr checks` enumeration in 2z and 3b — fetch a single PR directly ONLY for a detail the row does not carry, and `reviewDecision` is now ON the row, so the approved set in 2z is not one of them. (Cache: `worklist` reads through a bounded cache; pass `--no-cache` only if you need a guaranteed-fresh reload mid-run.) - (b) `pr-review-report uncovered-issues --json` → OPEN issues org-wide NOT already covered by an open PR, each with `number`, `repository`, `url`, `title`, `labels` — deduped by closing-keyword extraction (`Closes`/`Fixes`/`Resolves #N` in open-PR title+body), keyed per repo. This IS your deduped new-PR candidate set for step 4; its `labels` carry severity (`critical`/`high`/`medium`/`low`/`info`) and source (`audit`, `pass0`..`pass6`, `mutation-test`) that you prioritize by there. It REPLACES both the raw issue enumeration AND the step-3 dedup. COVERED IS NOT FIXED: this split is computed from OPEN PRs only, so an issue whose fix has ALREADY LANDED on `main` with no open PR pointing at it is `uncovered` here and enters the set — that is how a producer run re-implemented a guard merged 25 hours earlier (`rainlanguage/rain.dia#60` vs merged `#33`). So for the CANDIDATES YOU ACTUALLY TAKE (never the whole set — it is ~617 issues against a 5-item budget, which is the entire reason this is per-subject), run `pr-review-report already-fixed [...]`: exit 4 = a MERGED PR referencing that issue landed AFTER the issue was filed, 1 = it could not tell, 0 = clear. A non-zero exit is a REASON TO READ that merged PR, never a finding — if it did fix the issue, that is the step-7a `flag-close-candidate` path instead of a new PR; if it did not, work the issue and say so in the PR body. The same subcommand takes a PR ref and checks each issue that PR CLOSES, which is how you detect a PR of yours that a merged PR has SUPERSEDED (step 3's PR close-candidate route) — run it on the PRs your fleet sweep touches, for the same per-subject reason. + (b) `pr-review-report uncovered-issues --json` → OPEN issues org-wide NOT already covered by an open PR, each with `number`, `repository`, `url`, `title`, `labels` — deduped by closing-keyword extraction (`Closes`/`Fixes`/`Resolves #N` in open-PR title+body), keyed per repo. This IS your deduped new-PR candidate set for step 4; its `labels` carry severity (`critical`/`high`/`medium`/`low`/`info`) and source (`audit`, `pass0`..`pass6`, `mutation-test`) that you prioritize by there. It REPLACES both the raw issue enumeration AND the step-3 dedup. COVERED IS NOT FIXED: this split is computed from OPEN PRs only, so an issue whose fix has ALREADY LANDED on `main` with no open PR pointing at it is `uncovered` here and enters the set — that is how a producer run re-implemented a guard merged 25 hours earlier (`rainlanguage/rain.dia#60` vs merged `#33`). So for the CANDIDATES YOU ACTUALLY TAKE (never the whole set — it is ~617 issues against a {{ITEM_CAP}}-item budget, which is the entire reason this is per-subject), run `pr-review-report already-fixed [...]`: exit 4 = a MERGED PR referencing that issue landed AFTER the issue was filed, 1 = it could not tell, 0 = clear. A non-zero exit is a REASON TO READ that merged PR, never a finding — if it did fix the issue, that is the step-7a `flag-close-candidate` path instead of a new PR; if it did not, work the issue and say so in the PR body. The same subcommand takes a PR ref and checks each issue that PR CLOSES, which is how you detect a PR of yours that a merged PR has SUPERSEDED (step 3's PR close-candidate route) — run it on the PRs your fleet sweep touches, for the same per-subject reason. 2z. KEEP ALL YOUR OPEN PRS GREEN — APPROVED ONES FIRST (the merge cron is retired; landing is INTERACTIVE-ONLY — the human merges, or the interactive assistant merges on the human's explicit per-PR word; this routine NEVER runs `gh pr merge`). The APPROVED set is `fleet.approved` from step 2 — GitHub-native review state, a PR being APPROVED iff its `reviewDecision == APPROVED` (human approval is a native GitHub review, not a ledger line), over the open non-draft PRs authored by {{ASSIGNEE}} that the fleet read already covers. Do NOT re-derive it with `gh search prs --review approved`: that search opened all seven measured runs and returned EMPTY in every one, which is a round trip and a context payload buying a number the row already states. Your job for each APPROVED PR is to make and keep it LANDABLE, at top priority before all other work: - STALE-CI GUARD: if the head's check runs predate the PR's last push OR predate org gates they must satisfy (e.g. a static check added to the rainix reusables after the last run), retrigger with an --allow-empty commit — recorded greens are per-commit snapshots, not current policy. - RED → your step-3b work at top priority: fix and green it. A red prod-pin/testProdDeploy* check is the MIGRATION case (step 3b (iv)) — no code fix and NO deploy: the repo has not migrated to the split release lifecycle, and the PR is blocked on that migration. An APPROVED PR is human-sacred and cannot be label-flagged, so for the approved set report it as blocked-on-migration in the run summary (and make sure the repo's migration issue exists per 3b (iv)); an unapproved PR gets the 3b (iv) flag itself. @@ -43,7 +43,7 @@ Each run: - Report every approved PR's landability state (GREEN-READY / fixing / blocked-on-migration / needs-human) in the run summary — a green approved PR is an invitation for the human to merge, never something you land yourself. FLEET GREENNESS: the same duty extends to EVERY open PR of yours, approved or not — your whole fleet stays green and mergeable at all times, because the human review queue only presents green+mergeable PRs (reds and conflicts are invisible to it, so an un-green PR is undone work nobody will ever see). After the approved set, sweep the REST of your open PRs (they are the step-2 `worklist` rows — dispatch each by its `nextAction`; do NOT re-enumerate the fleet or re-check CI per PR): apply the STALE-CI GUARD fleet-wide (an org gate added to the rainix reusables reds old heads on their next run — retrigger stale heads BEFORE the human trips over phantom greens), fix reds via 3b (its back-off rules apply unchanged), resolve conflicts via 3d. BEHIND-BASE is also fleet-currency work: a PR whose `mergeStateStatus` is `BEHIND` (branch behind its base but NOT conflicting) is reviewed against stale code and may fail an up-to-date-branch protection — bring it current by merging the BASE branch INTO the PR branch (same discipline as 3d: `git merge` the base, NEVER rebase/force, union any test conflicts, build-verify, plain `git push`), so the human reviews it against current main. Prioritize approved > cheapest-to-green > rest; if the fleet is too big for one run, continue where the last run left off (oldest-unattended first) rather than re-sweeping from the top. 3. The COVERED/UNCOVERED split is ALREADY COMPUTED — it is the `pr-review-report uncovered-issues --json` set from step 2 (closing-keyword extraction over open-PR title+body, keyed per repo). Do NOT rebuild it with `gh search prs`+`jq`, and NEVER byte-offset-grep the raw JSON or hand-map references PR-by-PR — that hand-rolled dedup pathology burned the entire run and is FORBIDDEN. Work straight from the `uncovered-issues` candidate set; the run's value is PRs opened, not re-deriving coverage the tool already handed you. IN-RUN DEDUP: the issues a PR covers become COVERED the moment you open it — including the extra issues a multi-issue PR closes. Before starting each new issue, confirm you have not already covered it THIS run (a broad PR you opened minutes ago may already close it); never open a second PR overlapping one you just opened. If you catch a redundant PR you made, log the narrower one as a PR close-candidate noting which PR supersedes it. SEND-BACK WORK-ORDERS ARE A TYPED ROW, NOT A SEARCH: step 2 already states them — every fleet row whose `nextAction` is `rework-needs-work` is an open PR of yours labelled `ai:needs-work` that ALSO carries a TRUSTED rework instruction, and each one is YOUR WORK ORDER: read the instruction with `pr-review-report trusted-comments ` and rework that SAME PR/branch per it (never open a duplicate PR for the issue) as part of step 2z's fleet duty. Do NOT re-derive the set with `gh search prs --label ai:needs-work`: the label ALONE cannot tell the two send-back shapes apart, so that query answers a different question from the one you need, and a second read of the fleet is a second answer free to disagree with the one `state-load` just handed you — which is the whole reason state-load exists. The row is trustworthy for the reason you would have checked yourself: the tool author-verifies the instruction against the shared account, so the same text from a non-trusted author is a spoof, produces NO `rework-needs-work` row, and is not a work order. There is ONE send-back state whoever ruled (#133): a vetter verdict (`Reviewed : needs-work — …`) and a human ruling (a `👤 human` comment, a "Rework note", or both) all write `ai:needs-work` and all ask you for exactly the same thing, and the `👤 human` comment is how you tell that a HUMAN wrote this one — read it, it is the ruling and its reasoning. A needs-work-labelled PR with NO trusted instruction behind it is the OTHER shape and never appears as `rework-needs-work`: it counts as `parked-skip`, it is PARKED for a human, and you open NO new PR for it (re-attempting blind just piles up dead PRs). DESIGN RULINGS ARE THE SAME SEND-BACK (#219): a human answering a design question writes the SAME `ai:needs-work` + trusted work order a needs-work does — the ruling comment reads `Ruled : design — `, and it asks you for exactly what a needs-work asks: rework that SAME PR/branch per the note and push. There is no separate design channel, no design label of the human's, and no parked design PR to tiptoe around — one label, one pickup, one flow. THE SEND-BACK IS TRANSIENT, AND THE PUSH IS THE TRANSITION: after you push the rework commit there is NOTHING ELSE TO CALL, for either kind of send-back. The head moves, so the vetter's stale verdict and the human's sha-pinned ruling both stop being current, the PR is un-vetted, and the vetter re-judges it from scratch WITH the human's note in front of it; `record-verdict` clears the stale `ai:needs-work` as it writes the new verdict. (`reworked-reject` is GONE — do not look for it.) Never hand-edit a label with raw `gh`. THE OTHER EXIT: if the rework is not worth doing at all — the issue is a duplicate, superseded, or the approach was rejected outright rather than sent back for a fix — say so on the PR and treat it as a close-candidate rather than pushing a token commit; a needs-work has two exits, rework and close, and a whitespace push is neither (a LINKAGE send-back is a rework — see below — not a third exit). BODY-ONLY SEND-BACK — THE MISSING QA §8 BLOCK: a needs-work whose ONLY ground is that the PR BODY lacks QA-GUIDE §8's evidence block is not a code defect and is not fixed by touching code; it is fixed by writing the block. `gh pr edit` stays forbidden — the sanctioned move is `pr-review-report repair-qa-block --block-file `. Write the four-line block to `{{SCRATCH_DIR}}/qa-block-.md` with the Write tool, TRANSCRIBED from an adversarial-mutation-test pass you actually ran against this PR's change (if you no longer have that output, re-run the skill on the PR's diff first — never invent evidence to clear a label). The subcommand APPENDS the block and leaves every other byte of the body identical, and it validates what it writes with the SAME predicate the `require-qa-block` PR-open gate uses, so a block it accepts is a block that gate accepts. It REFUSES (exit 4) when the PR already has a `## QA` section that DIFFERS: that is the other send-back ("the block's claims don't hold"), which you fix by re-running the evidence and only then re-calling with `--replace` — never by rewriting the sentence. Re-running the identical call is a no-op, not an error. LINKAGE SEND-BACK — `Closes` SHOULD BE `Refs`: a needs-work whose ground is that the PR over-claims an issue (sound code, a genuine independently-valuable SUBSET, but the body says `Closes #N` where it must say `Refs #N`) is likewise not a code defect. There is no `ai:relink` verdict any more — a linkage error is a `needs-work` whose note names the reference — and the move is the `mcp__fsm__weaken_closes` TOOL: `{"pr": "/#", "issue": }` (dry-run first with `"dry_run": true` if you want to see the plan). It rewrites every `Closes #N` in the body to `Refs #N` and leaves every other byte identical, it NEVER touches the `## QA` block, and it is DIRECTION-LOCKED: it can only ever weaken a linkage, so there is no argument and no spelling by which you can ADD a `Closes` — the covered/uncovered split in step 2 is computed from `closingIssuesReferences`, and a producer that could strengthen one would be marking its own homework. Running it twice is running it once. It REFUSES (exit 4) an issue the body does not reference at all — that means the note named a number this PR does not link, so re-read the note rather than picking the nearest reference. THEN RE-ARM THE VETTER: a body edit moves no commit, so the PR is still vetted-at-head and the vetter will SKIP it — after a repair, push an `--allow-empty` commit to that branch (the subcommand prints a NOTE when this applies), which un-vets the PR for the next run. (`close` verdicts need no handling here — the duplicate's issue stays covered by the superseding open PR.) -3b. FIX YOUR OWN RED PRs **FIRST** — EXISTING in-flight work takes PRIORITY over opening new PRs. A red PR is a fix you ALREADY made that is merely stuck on CI; finishing it (driving it green so the human can land it) is higher-leverage than starting anything new, and a red PR is never landable until it is green. So BEFORE step 4 opens ANY new PR, spend the run budget on your fixable red PRs first — each red you work is one of the run's 5 items. Your red PRs are the step-2 `worklist` rows with `nextAction: needs-3b` (plus `flag-migration` rows → 3b (iv)); each row already carries its `ci` + `failingChecks`, so do NOT re-list the fleet or re-run `gh pr checks` per PR. Work the RED ones in audit-before-general, then severity (by the linked issue's labels) order, UP TO THE BUDGET — a red PR that is a hand-off (back-off / unfixable env-infra red, per below) costs no budget to leave, and one beyond the budget waits for the next run. Open NEW PRs (step 4) ONLY with the budget left AFTER your reds are handled; if reds consume the whole run, that is correct — open no new PR this run. For each red PR, in order: +3b. FIX YOUR OWN RED PRs **FIRST** — EXISTING in-flight work takes PRIORITY over opening new PRs. A red PR is a fix you ALREADY made that is merely stuck on CI; finishing it (driving it green so the human can land it) is higher-leverage than starting anything new, and a red PR is never landable until it is green. So BEFORE step 4 opens ANY new PR, spend the run budget on your fixable red PRs first — each red you work is one of the run's {{ITEM_CAP}} items. Your red PRs are the step-2 `worklist` rows with `nextAction: needs-3b` (plus `flag-migration` rows → 3b (iv)); each row already carries its `ci` + `failingChecks`, so do NOT re-list the fleet or re-run `gh pr checks` per PR. Work the RED ones in audit-before-general, then severity (by the linked issue's labels) order, UP TO THE BUDGET — a red PR that is a hand-off (back-off / unfixable env-infra red, per below) costs no budget to leave, and one beyond the budget waits for the next run. Open NEW PRs (step 4) ONLY with the budget left AFTER your reds are handled; if reds consume the whole run, that is correct — open no new PR this run. For each red PR, in order: (0) BACK-OFF — at most ONE fix attempt per failing check, EVER. The PR's own commit history is your only cross-run memory: check it FIRST (`gh pr view -R rainlanguage/ --json commits` + `git log`). If you (`{{ASSIGNEE}}`) have ALREADY pushed ANY commit to this PR aimed at this same failing check — whether or not it is tagged — and the check is STILL red, do NOT push again: a fix that built green locally yet stays red in CI is almost certainly CI-only (a fork/secret/infra/cache flake your local run cannot reproduce); STOP. If the check is failing for ENVIRONMENTAL reasons your code cannot touch, that is the `infra-down` case at the top of this prompt — `pr-review-report infra-down ""` and END THE RUN; do not label the PR and do not move on to other work. If instead the blocker is something only a PERSON can supply or decide — a CI secret that exists nowhere, a cross-repo contract change — that is a question, not an outage: `pr-review-report flag-design ""`, then move on. MANDATORY TAGGING: every step-3b fix commit's subject MUST start with `fix(ci): [3b-attempt]` — required, not optional, so the next run recognizes it; an untagged fix defeats this back-off and risks an infinite one-commit-per-run loop, so NEVER push a step-3b fix without that tag. A still-red `[3b-attempt]` commit = HAND OFF, never re-push — with ONE carve-out: a confirmed TRANSIENT infra flake (per (i): cachix/network/timeout signature in the log) may be EMPTY-COMMIT re-triggered again on a later run (still capped at one per check per run), because re-running is the only valid action and no code change can help; keep re-triggering only while the log still shows that same transient signature, STOP the instant it changes to a real (code) error or goes green, and if the same transient flake survives ~3 consecutive empty-retrigger runs, HAND OFF and note a likely sustained infra outage for a human. (This carve-out is for empty re-triggers ONLY; a still-red CODE fix is still HAND OFF, never re-push.) (i) READ THE FAILING LOG FIRST (`gh pr checks ` → `gh run view -R rainlanguage/ --log-failed`) and confirm the failure is REAL and establish its SOURCE: this PR's own code, a pre-existing `main` red, or CI infra. A real red caused by THIS PR's code → reproduce + minimal-fix per (ii). Otherwise it routes by what the log shows — decide which: • TRANSIENT INFRA FLAKE that a plain re-run is likely to clear — no code cause, e.g. a cachix substitute-download error (`unable to download …nar.zst`, `HTTP/2 framing layer`, `some substitutes … failed (usually … networking issues)`), a network/DNS reset, a runner OOM/timeout, or a transient 5xx from a registry. A code change CANNOT fix this and there is nothing to reproduce locally; the CORRECT action is to RE-TRIGGER CI with an EMPTY COMMIT: `git -C -c commit.gpgsign=false commit --allow-empty --no-verify -m "fix(ci): empty-commit retrigger (flaky) [3b-attempt]"` then the `mcp__fsm__push` TOOL (as 3b (iii); fast-forward — NEVER force). That empty commit IS this run's ONE attempt for that check (back-off (0)). It is explicitly NOT the forbidden "no-op fix": that ban forbids fabricating a fake CODE change to pretend a real failure is fixed; an empty commit touches no code and is the ONLY valid response to a genuine transient flake. Never empty-commit on a guess — only when the log positively shows a transient signature. @@ -82,7 +82,7 @@ Each run: `pr-review-report flag-close-candidate ": "` — the SOLE sanctioned close-candidate path. It applies the `ai:close-candidate` LABEL + a trusted `🤖 ai:producer` reason comment, REFUSES (exit 3) if the subject carries any `human:*` ruling label (the human's decision is sacred), and is idempotent (re-flag = no-op). Never use raw `gh issue edit`/`gh issue comment` for this — the subcommand is the one path (it labels the issue, which you otherwise cannot). RECENCY IS ENFORCED (already-fixed-on-main only): the tool REFUSES (exit 4) a reason that cites no MERGED commit sha / PR number, and refuses again if that commit/PR date does NOT post-date the issue's `createdAt`. "This code is on main today" is NOT the claim — the claim is "a change landed that resolved this AFTER it was reported"; code that predates the report cannot be the fix. So before flagging, identify the change that actually fixed it (`git log -S'' -- ` / the merged PR) and cite THAT; if you cannot find one, the issue is probably still live — or the honest category is `invalid`/`duplicate`/`wont-fix`, which are judgements and are not date-gated. CITATION EVIDENCE IS APPENDED TO YOUR FLAG, AND YOU READ IT BACK: the tool fetches the cited PR/commit's OWN diff and writes onto the comment how many files it touches, its `+a/-d` on every path your reason names, and which symbols your reason names that its changed lines do not contain. It refuses on exactly ONE reading of that line, and only this one (exit 5): the reason named path(s) or symbol(s), and the cited change contains NOT ONE of them — no named path in its file list, no named symbol in its changed lines. A partial miss NEVER refuses — a sound reason routinely names current-main symbols the cited change never touched, and a fix by deletion leaves its evidence on the removed side — but it is printed where you will see it, and a citation that CANNOT be what you claimed is now visible the moment you write it. `rain.dia#22` is the case: the flag said merged PR #48 "landed testRoundTripEmpty (line 27) and testRoundTrip31Bytes (line 32)" when #48's touch on that file is `+2/-2` (an import rewrite) and PR #33 is what added those tests. If the line says the paths you named are NOT TOUCHED BY IT, or names the very symbols your claim turns on as absent, you have cited the wrong change — find the right one and re-flag with it rather than leaving a provenance trail that sends the next reader to a PR that did not fix this. DO NOT CITE THE SHA YOU READ MAIN AT. `already-fixed-on-main: …Foo.svelte:40 at abc1234` looks like it carries a commit anchor and does not: `abc1234` dates the TREE YOU LOOKED AT, not the change that fixed anything, and because every recent main sha post-dates the issue the recency check passes without meaning a thing. That is a bare `file:line` in disguise, which is the claim shape this gate refuses outright. It is the shape of every commit-anchored flag on record — `raindex#588`/`#574`/`#573`/`#570` all cite `bb83031`, a build-script rename touching `foundry.toml` and `script/Build.sol`, as the fix for four unrelated Svelte UI bugs; `raindex#928` cites `7ba0fa8` in the words "tauri-app/ existed AT 7ba0fa8", naming the state BEFORE the deletion it credits. Cite the change that landed the fix (`git log -S'' -- `), and if its path or name has since been renamed, cite a path or symbol THAT CHANGE ITSELF contains. REACHABILITY (all already-fixed-on-main evidence, not only GUI): the cited code must be shown to RUN — a caller chain, a test that exercises it, or the screenshot below. A definition with no construction site, a symbol nothing outside its own file and tests references, or anything inside a commented-out block is NOT evidence that behavior changed (a `successMessage` constant can exist and never reach the DOM; a chart component can be 100% commented out). GUI-CLAIM PROOF (mandatory): when the `already-fixed-on-main` evidence is about RENDERED behavior a USER SEES (a `.svelte` component, a visible element, user-visible styling in packages/webapp|packages/ui-components, a non-`.svelte` service whose output is displayed, or a `site/` dashboard section), a `file:line`/grep is NOT sufficient — a code path can exist without producing the claimed rendered result (an author can wire `readableMsg` and still render a blank toast). RENDER the current-main behavior via step 5's harness and prove it: push `shots/-issue-.png` to the raindex `pr-screenshots` branch and EMBED the raw URL in the flag reason (`already-fixed-on-main: ; screenshot `), READING your own PNG to confirm it shows the claimed result before you flag. If the flow genuinely cannot be rendered in the harness (e.g. a wallet-gated path it can't reach), state that explicitly in the reason instead (`already-fixed-on-main: ; screenshot-not-possible: `) — where the reason is THE ATTEMPT THAT FAILED (what you rendered, where it stopped), never a judgement of what the render would have shown. "Pixel-identical", "no visible effect", "cosmetic only" are the CONCLUSIONS the render exists to support or refute, so they waive the evidence with the claim it was supposed to produce; the vetter rejects a why-not of that shape here exactly as it does on a PR (2026-07-29: cyclo.site#431's "rendered output is pixel-identical" waiver, sent back by a human). An unattempted render is not "not possible", it is not done. A bare code claim about GUI behavior with NEITHER a screenshot NOR a FAILED-ATTEMPT why-not is not a valid close-candidate — do not flag it; render it, or name the attempt that could not. The `ai:close-candidate` LABEL is the queue key — the human's triage view is `gh search issues {{OWNER_FLAGS}} --state open --label ai:close-candidate` (minus `human:keep-open`), which is self-cleaning: a closed/fixed issue drops out automatically and a `human:keep-open` ruling excludes it for good. You still NEVER close; flagging is the entire action. 8. Skip issues that are too large / need design / aren't reproducible / are research spikes / are pure umbrella trackers (brief note). If a fix won't converge after a focused attempt, skip it; don't grind for an hour on one. If a coverage target is already mutation-saturated, report it and open NO PR (never a no-test or scan-record PR). -9. Take tractable, genuinely-uncovered issues UP TO THE RUN'S REMAINING BUDGET (5 items total per run, shared with rework/conflict/migration work — if greenness work already used the budget, this step does nothing this run). Quality over quantity: every PR goes to a human reviewer, and a run that opens one PR a human merges beats one that opens five they have to triage. Because earlier runs/sessions have already PR'd most tractable issues, expect each run to mostly find things already covered (dedup) and do little — that's correct; focus on newly-filed issues and ones a prior run skipped. +9. Take tractable, genuinely-uncovered issues UP TO THE RUN'S REMAINING BUDGET ({{ITEM_CAP}} items total per run, shared with rework/conflict/migration work — if greenness work already used the budget, this step does nothing this run). Quality over quantity: every PR goes to a human reviewer, and a run that opens one PR a human merges beats one that opens a pile they have to triage. Because earlier runs/sessions have already PR'd most tractable issues, expect each run to mostly find things already covered (dedup) and do little — that's correct; focus on newly-filed issues and ones a prior run skipped. 7b. DESIGN-CANDIDATE RECORD (the only channel for "this existing design looks wrong / this issue would entrench it"): a GitHub comment on the issue, same mechanics as 7a (verify open, never duplicate): `gh issue comment -R rainlanguage/ --body "Design question — awaiting human design ruling before any PR: . First-principles question it fails: . (automated design-gate note)"`. The human's triage view is the `awaiting human design ruling` comment search. Never act on the design yourself — no redesign PRs without a human ruling. diff --git a/campaign-run.sh b/campaign-run.sh index 484daf28..0ded922d 100755 --- a/campaign-run.sh +++ b/campaign-run.sh @@ -383,6 +383,25 @@ export SCRATCH_DIR # preflight, and the mkdir failure itself) have no scratch dir to reclaim and must not acquire one. trap '[ -n "${SCRATCH_DIR:-}" ] && rm -rf "$SCRATCH_DIR"' EXIT +# --- the RUN BUDGET the prompt states (#288) --------------------------------------------------- +# The per-run WORK ITEM cap has ONE definition — `RUN_ITEM_CAP` in pr-review-report — and the prompt +# DERIVES every statement of it from `{{ITEM_CAP}}` rather than spelling the number in prose. Some +# of those statements read as English words rather than digits, so a sweep for the digit does not +# find them and a raise leaves them behind at the old value; a prompt is natural language, so what +# is left behind is not a parse error but a CONTRADICTORY instruction the run resolves its own way. +# +# The guard is the point, not the assignment. An empty substitution does not fail: it renders "at +# most WORK ITEMS per run" and hands the model a budget with no number in it — the same silent +# degradation the worker-brief guard below exists for, one stale binary on PATH away. So a value +# that is not a positive integer ABORTS the run instead of reaching the model. +ITEM_CAP="$(pr-review-report item-cap 2>/dev/null)" +case "$ITEM_CAP" in + '' | *[!0-9]* | 0) + echo "$(date -u +%FT%TZ) campaign run ABORT: \`pr-review-report item-cap\` gave no usable run budget (got '$ITEM_CAP') — the prompt's {{ITEM_CAP}} would render empty" | _log + exit 1 + ;; +esac + # substitute deployment values into the (path-free) prompt template at runtime PROMPT="$(sed -e "s#{{WORK_DIR}}#$WORK_DIR#g" \ -e "s#{{ASSIGNEE}}#$PR_ASSIGNEE#g" \ @@ -390,6 +409,7 @@ PROMPT="$(sed -e "s#{{WORK_DIR}}#$WORK_DIR#g" \ -e "s#{{ORGS}}#$ORGS_HUMAN#g" \ -e "s#{{INSTALL_DIR}}#$DIR#g" \ -e "s#{{SCRATCH_DIR}}#$SCRATCH_DIR#g" \ + -e "s#{{ITEM_CAP}}#$ITEM_CAP#g" \ "$DIR/campaign-prompt.txt")" # --- the STANDING BRIEF every dispatched worker starts with (#200) ----------------------------- diff --git a/plugins/human-fsm/.claude-plugin/plugin.json b/plugins/human-fsm/.claude-plugin/plugin.json index 01b1d92b..861de612 100644 --- a/plugins/human-fsm/.claude-plugin/plugin.json +++ b/plugins/human-fsm/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "human-fsm", "displayName": "Human FSM transitions", - "version": "0.20.0", + "version": "0.20.1", "description": "Slash commands for the human's side of the rainlanguage issue→PR pipeline finite state machine: the reads that precede a ruling, the transitions that make it, and /observe-run — force a run, watch it, measure its context, and read the retained trace corpus for what a run still hand-rolls. Every input arrives from pr-review-report — each ruling as one subcommand, /nr, /ncc, /ndd and /nm as typed MCP reads — and the guards (vocabulary, mandatory note, provenance anchor, stranded-flag refusal, subject type, terminal-is-moot, idempotence) live in that binary, not here.", "author": { "name": "Rain Open Source Software Ltd", diff --git a/plugins/human-fsm/commands/ncc.md b/plugins/human-fsm/commands/ncc.md index b9a38859..11c2b6e1 100644 --- a/plugins/human-fsm/commands/ncc.md +++ b/plugins/human-fsm/commands/ncc.md @@ -259,8 +259,8 @@ Each field of the row is one read a human otherwise does by hand: `queue.more` and `counts` frame the row. A flag you expected and did not get is usually in `counts.unvetted` — the vetter has not judged it, and under the -5-item run cap it may wait; a flag the vetter would REJECT never arrives here at -all. `strandedFlags` is a label parking an issue with nothing consuming it: no +per-run item cap it may wait; a flag the vetter would REJECT never arrives here +at all. `strandedFlags` is a label parking an issue with nothing consuming it: no producer comment behind it, or a reject whose label is live anyway. The vetter's state-load clears both, so one listed here is a clearance that has not run yet or could not write — the label is still parking the issue either way. It is not diff --git a/pr-review-report-rs/src/main.rs b/pr-review-report-rs/src/main.rs index 5dd2e160..42babc23 100644 --- a/pr-review-report-rs/src/main.rs +++ b/pr-review-report-rs/src/main.rs @@ -24121,6 +24121,38 @@ const MAX_BASIS_WORDS: usize = 12; const GC_MAX_AGE_DEFAULT: u64 = 30; const GC_MAX_AGE_RANGE: std::ops::RangeInclusive = 1..=365; +/// THE PER-RUN WORK-ITEM CAP — the one definition of it, and the only place it is spelled (#288). +/// +/// An ITEM is one unit of work a run may attempt: for the producer an issue it PRs, a rework it +/// pushes, a conflict it resolves, a migration it flags; for the vetter a PR it vets or a +/// close-candidate flag it rules on. It is a RISK CONTROL, not a throughput preference — this +/// machine is not yet reliable or efficient, and every item a run attempts is an item that can go +/// WRONG (a bad PR a human unpicks, an unsound flag, a wrong verdict acted on, tokens burnt for +/// nothing), so the cap bounds how much damage ONE run can do while that is still true. It is +/// deliberately conservative and explicitly temporary: it RISES on evidence from the run logs that +/// runs have become reliable and efficient, never because a run finished early with budget to +/// spare. A number the design intends to move repeatedly is the number that must not be copied. +/// +/// EVERY OTHER SURFACE DERIVES FROM THIS ONE, on both sides of the boundary: +/// +/// - IN THIS CRATE: [`STATE_LOAD_PAGE_DEFAULT`] and [`STATE_LOAD_PAGE_RANGE`] are computed from it, +/// so the page the MCP boundary hands back and the budget the run is told to spend cannot +/// disagree. The two state-load descriptions and their `limit` schemas are `format!`ed from it, +/// so the English a model reads off the schema is the number the validator enforces. +/// - IN THE PROMPTS: `campaign-prompt.txt` and `review-prompt.txt` carry `{{ITEM_CAP}}`, which the +/// runners substitute from `pr-review-report item-cap` — this constant, printed. A prompt is +/// natural language read by a model, so a stale restatement is not a parse error but a +/// CONTRADICTORY instruction each run resolves its own way. The template makes the restatement +/// unspellable, and `the_prompts_state_no_item_cap_of_their_own` fails the build if one is +/// written back in — as a digit or as an English word, which a `grep -n` for the digit misses. +/// +/// NOT everything spelled `3` or `5` nearby is this number. The four `NEXT_*_MAX_ROWS` page caps +/// bound a HUMAN's inbox and are sized by row STALENESS — each ruling changes the queue, so a page +/// is stale past its head (see [`NEXT_READY_MAX_ROWS`]). A human is not a cron. Deriving them from +/// here would enshrine a coincidence as a coupling and move a human's page every time the crons' +/// risk bound moves, so they stay their own constants at their own value. +const RUN_ITEM_CAP: usize = 5; + /// How many rows ONE state-load page carries, and the bounds a caller may move it within (#78). /// /// The vetter judges ONE PR at a time and each `record_verdict` removes that PR from the next @@ -24129,22 +24161,20 @@ const GC_MAX_AGE_RANGE: std::ops::RangeInclusive = 1..=365; /// STRUCTURAL: at 25 rows a state-load cannot reach [`MCP_MAX_RESULT_BYTES`] even with GitHub's /// longest legal titles, so the size of the queue stops being able to break the state-load. /// -/// RUN BUDGET (2026-08-03): a page is now an ALLOWANCE, not a window onto a longer queue. A vetter -/// run spends at most 5 ITEMS — a PR vetted or a close-candidate flag ruled on, ONE budget shared -/// across both state-loads. It is a RISK CONTROL, not a throughput preference: the FSM is not yet -/// reliable or efficient, every item a run attempts is an item that can go wrong (a wrong verdict -/// a human acts on, a sound flag stripped, tokens burnt for nothing), and 5 bounds how much damage -/// ONE run can do while that is still true. So a state-load handing back 10 or 25 is handing back -/// work the run must not do, and the per-tool half of "stop at 5" is a rule the surface enforces -/// rather than one the prompt merely asserts. The SHARING is necessarily the prompt's to enforce: -/// each tool call is bounded on its own, and neither can see what the other already spent. The -/// bound is deliberately conservative and explicitly temporary — raising it is moving THIS number, -/// gated on evidence from the run logs that runs have become reliable and efficient, never on a -/// run having finished early with budget to spare. It is the SAME number `campaign-prompt.txt` and -/// `review-prompt.txt` state in prose: a page smaller than the prompt's cap silently caps the run -/// lower than the rule says, so the two move together. -const STATE_LOAD_PAGE_DEFAULT: usize = 5; -const STATE_LOAD_PAGE_RANGE: std::ops::RangeInclusive = 1..=5; +/// RUN BUDGET (2026-08-03): a page is an ALLOWANCE, not a window onto a longer queue. A vetter run +/// spends at most [`RUN_ITEM_CAP`] ITEMS — a PR vetted or a close-candidate flag ruled on, ONE +/// budget shared across both state-loads. So a state-load handing back 10 or 25 is handing back +/// work the run must not do, and the per-tool half of "stop at the cap" is a rule the surface +/// ENFORCES rather than one the prompt merely asserts. The SHARING is necessarily the prompt's to +/// enforce: each tool call is bounded on its own, and neither can see what the other already spent. +/// +/// Both DERIVE from [`RUN_ITEM_CAP`] rather than restating it, because the two directions of +/// disagreement both fail SILENTLY: a page above the cap hands back work the run must not do, and +/// a page below it starves a run told to spend more items than it can fetch rows for — these tools +/// REFUSE an out-of-range `limit` rather than clamping it, so nothing errors and the run just +/// quietly does less than the rule says (#288). +const STATE_LOAD_PAGE_DEFAULT: usize = RUN_ITEM_CAP; +const STATE_LOAD_PAGE_RANGE: std::ops::RangeInclusive = 1..=RUN_ITEM_CAP as u64; /// The byte budget ONE tool result must fit in — the contract this server holds itself to, checked /// on every result before it is handed back (#78), and sized so that OUR error always arrives before @@ -31950,12 +31980,12 @@ fn mcp_all_tools() -> Value { { "name": "unvetted", "narrows": "limit", - "description": "State-load: ONE PAGE of the open PRs to vet, vet-first order. Per PR: headRefOid, labels, reviewDecision, humanSacred, vettedAtHead, ci, mergeable. `counts` is whole-queue; `more` is how many vet-able PRs this page left behind — the NEXT run's work: a run spends at most 5 ITEMS in total, shared with the flags from unvetted_close_candidates, so never re-call for a second page. `openThreads` lists the PRs withheld because a review thread is unresolved. Human-decided and vetted-at-head PRs are already excluded. A DRAFT is not vetted either, but it is not skipped: this call SENDS IT BACK itself, as ai:needs-work with the work order that the producer confirm the PR is not a draft if it intends to merge something, and `draftNeedsWork` names the PRs that happened to (`sentBack: false` = the write did not land). A draft ALREADY in a modeled ai:* state is left alone instead — the send-back would strip that label, and for ai:close-candidate/ai:design the label IS the human's queue — counted as `skipDraftInState` and inventoried by that state, not here. It also runs the ai:blocked-on clearance check: a flag whose typed deps are all merged/closed is cleared in-place and the PR appears here un-vetted; `blockedOn` lists the PRs still held (open deps named); `blockedOnManualReview` lists the flags the machine cannot judge (no typed refs / unresolvable ref) — those need a human, never a verdict.", + "description": format!("State-load: ONE PAGE of the open PRs to vet, vet-first order. Per PR: headRefOid, labels, reviewDecision, humanSacred, vettedAtHead, ci, mergeable. `counts` is whole-queue; `more` is how many vet-able PRs this page left behind — the NEXT run's work: a run spends at most {RUN_ITEM_CAP} ITEMS in total, shared with the flags from unvetted_close_candidates, so never re-call for a second page. `openThreads` lists the PRs withheld because a review thread is unresolved. Human-decided and vetted-at-head PRs are already excluded. A DRAFT is not vetted either, but it is not skipped: this call SENDS IT BACK itself, as ai:needs-work with the work order that the producer confirm the PR is not a draft if it intends to merge something, and `draftNeedsWork` names the PRs that happened to (`sentBack: false` = the write did not land). A draft ALREADY in a modeled ai:* state is left alone instead — the send-back would strip that label, and for ai:close-candidate/ai:design the label IS the human's queue — counted as `skipDraftInState` and inventoried by that state, not here. It also runs the ai:blocked-on clearance check: a flag whose typed deps are all merged/closed is cleared in-place and the PR appears here un-vetted; `blockedOn` lists the PRs still held (open deps named); `blockedOnManualReview` lists the flags the machine cannot judge (no typed refs / unresolvable ref) — those need a human, never a verdict."), "inputSchema": { "type": "object", "properties": { "include_skipped": {"type": "boolean", "description": "Also list the excluded PRs and why (digest rows: pr, action, unresolvedThreads)."}, - "limit": {"type": "integer", "description": "Rows per list, 1-5 (default 5) — a run's whole work budget is 5 items across both state-loads."} + "limit": {"type": "integer", "description": format!("Rows per list, 1-{RUN_ITEM_CAP} (default {RUN_ITEM_CAP}) — a run's whole work budget is {RUN_ITEM_CAP} items across both state-loads.")} } } }, @@ -32056,12 +32086,12 @@ fn mcp_all_tools() -> Value { { "name": "unvetted_close_candidates", "narrows": "limit", - "description": "State-load: ONE PAGE of the producer close-candidate flags on open SUBJECTS — issues AND pull requests (#211; the row's url says which) — to vet. Per subject: flagAt, flagReason (the producer's stated evidence), labels, humanSacred, vettedAtFlag. `counts` is whole-queue; `more` is how many this page left behind — the NEXT run's work: a run spends at most 5 ITEMS in total, shared with the PRs from unvetted, so never re-call for a second page. Already-vetted-at-flag subjects are excluded, as are PRs whose label is the vetter's own `close` verdict (counts.skipVetterClose — the human's queue, not a claim to judge). A subject a human has RULED is not excluded and not skipped: the ruling is a transition whose write only half landed, so this call COMPLETES it — the recorded close executed, or the flag the ruling contradicts retired (counts.completedHumanRuling / humanRulingCompletionFailed).", + "description": format!("State-load: ONE PAGE of the producer close-candidate flags on open SUBJECTS — issues AND pull requests (#211; the row's url says which) — to vet. Per subject: flagAt, flagReason (the producer's stated evidence), labels, humanSacred, vettedAtFlag. `counts` is whole-queue; `more` is how many this page left behind — the NEXT run's work: a run spends at most {RUN_ITEM_CAP} ITEMS in total, shared with the PRs from unvetted, so never re-call for a second page. Already-vetted-at-flag subjects are excluded, as are PRs whose label is the vetter's own `close` verdict (counts.skipVetterClose — the human's queue, not a claim to judge). A subject a human has RULED is not excluded and not skipped: the ruling is a transition whose write only half landed, so this call COMPLETES it — the recorded close executed, or the flag the ruling contradicts retired (counts.completedHumanRuling / humanRulingCompletionFailed)."), "inputSchema": { "type": "object", "properties": { "include_skipped": {"type": "boolean", "description": "Also list the excluded issues and why."}, - "limit": {"type": "integer", "description": "Rows per list, 1-5 (default 5) — a run's whole work budget is 5 items across both state-loads."} + "limit": {"type": "integer", "description": format!("Rows per list, 1-{RUN_ITEM_CAP} (default {RUN_ITEM_CAP}) — a run's whole work budget is {RUN_ITEM_CAP} items across both state-loads.")} } } }, @@ -43632,6 +43662,11 @@ enum Cmd { #[arg(long)] limit: Option, }, + /// Print the per-run WORK ITEM cap ([`RUN_ITEM_CAP`]) and nothing else, so a caller can `$(…)` + /// it. `campaign-run.sh` and `review-run.sh` substitute it into their prompt templates' + /// `{{ITEM_CAP}}`, which is what makes the prose a run reads and the page this binary's MCP + /// boundary will hand it the SAME number rather than two that have to be kept in step (#288). + ItemCap, /// CI gate: every plugin `.claude-plugin/marketplace.json` lists resolves to a real manifest /// carrying the SAME version. Exit 2 if a listing is wrong, 3 if the gate cannot be evaluated. PluginVersionLockstep { @@ -46992,8 +47027,11 @@ fn retire_blocked_infra_mode(dry_run: bool) -> i32 { // // PER-SUBJECT AND NOT FOLDED INTO `uncovered-issues`, which is a cost decision, measured: the // uncovered set is 617 issues and the read is one GraphQL round trip each (~0.65s measured over a -// 40-issue sample), while the producer's run budget is 5 work items. Running it over the backlog -// buys ~612 answers per run that nothing reads. Running it over the candidates costs five calls. +// 40-issue sample), while the producer's run budget is [`RUN_ITEM_CAP`] work items. Running it over +// the backlog buys an answer for all 617 when only the budget's worth is ever read; running it over +// the candidates costs one call each, so the whole cost is the budget. The gap is three orders of +// magnitude at any cap this bound will plausibly take, which is why the shape is per-subject rather +// than a figure that has to be recomputed every time the cap moves. // // The recency rule is [`landed_after_filed`] — the SAME function `already_fixed_recency_gate` // enforces at flag time, so the two ends of a run cannot disagree about what "post-dates" means. @@ -47617,6 +47655,10 @@ fn main() { include_skipped, limit, } => unvetted_mode(json, include_skipped, limit), + Cmd::ItemCap => { + println!("{RUN_ITEM_CAP}"); + 0 + } Cmd::PluginVersionLockstep { root } => plugin_version_lockstep_mode(&root), Cmd::RequireQaBlock => require_qa_block_mode(), Cmd::RepairQaBlock { @@ -52567,7 +52609,7 @@ mod prompt_section_tests { #[test] fn the_preamble_is_the_first_paragraph_and_stops_at_the_blank_line() { const PROMPT: &str = "\ -RUN BUDGET: at most 5 WORK ITEMS +RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS FAN OUT BY DEFAULT, and here is why \nSHELL SHAPES: a LATER paragraph @@ -52575,7 +52617,7 @@ ONE-SHOT, NOT A LOOP "; let preamble = producer_preamble(PROMPT); assert_eq!( - preamble, "RUN BUDGET: at most 5 WORK ITEMS\nFAN OUT BY DEFAULT, and here is why", + preamble, "RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS\nFAN OUT BY DEFAULT, and here is why", "a wrapped paragraph is joined back with its own newlines, not flattened" ); // A whitespace-only line ENDS the paragraph — it is blank to a reader, and a separator that @@ -52595,7 +52637,7 @@ ONE-SHOT, NOT A LOOP #[test] #[should_panic(expected = "campaign-prompt.txt has no preamble paragraph")] fn a_missing_preamble_panics_rather_than_yielding_an_empty_haystack() { - producer_preamble("\nRUN BUDGET: at most 5 WORK ITEMS\n"); + producer_preamble("\nRUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS\n"); } } @@ -53009,11 +53051,16 @@ mod settings_tests { }; let budget = producer_preamble(&prompt); - // THE CAP'S UNIT. "fan-out for the 3" numerically couples agents to the cap, and the + // THE CAP'S UNIT. "fan-out for the " numerically couples agents to the cap, and the // disclaimer beside it contrasted items with EFFORT — never with AGENTS — so the question // "may this item take two agents?" had no answer in the text. + // + // The forbidden string stops at "the" and carries NO number. Spelling the cap into it makes + // the assertion permanently true the moment the cap moves — the phrasing it exists to + // forbid just gets written with the new number and sails past — which is #288's defect + // inside the guard against it. assert!( - !prompt.contains("Use the Workflow/Task sub-agent fan-out for the 3"), + !prompt.contains("Use the Workflow/Task sub-agent fan-out for the"), "the cap must not be phrased so that the fan-out is sized by the item count" ); assert!( @@ -59223,6 +59270,15 @@ mod cli_tests { assert!(Cli::try_parse_from(["prr", "require-qa-block", "extra"]).is_err()); } + // The runners read this in a `$(…)`, so it takes no arguments and prints one number: a + // spelling that accepts any would let a caller ask for a budget other than the one in force, + // which is the disagreement the subcommand exists to make impossible. + #[test] + fn item_cap_cli() { + assert_eq!(parse(&["prr", "item-cap"]), Cmd::ItemCap); + assert!(Cli::try_parse_from(["prr", "item-cap", "5"]).is_err()); + } + // A CI gate must run locally against any checkout, so its root is an argument with a working // default rather than a path baked into a workflow. #[test] @@ -67923,6 +67979,343 @@ mod vetter_state_load_tests { } } +/// THE CAP'S PIN (#288) — what keeps [`RUN_ITEM_CAP`] a single source of TRUTH rather than a +/// single source of SELF-CONSISTENCY. +/// +/// Derivation alone is not a test. Once every surface is computed from one constant, an expectation +/// computed from that same constant agrees with it at EVERY value, so it can no longer tell 5 from +/// 3 and it survives a mutation of the number it exists to pin. So the values here are LITERALS, +/// deliberately written twice: moving the cap is meant to fail this file loudly and be confirmed +/// once, which is the opposite of a surface left behind silently. +#[cfg(test)] +mod run_item_cap_tests { + use super::*; + + /// How many whitespace/hyphen tokens before an `item`-noun the scan looks back over. + /// + /// ONE is not enough, and the miss is not hypothetical: the run-summary instruction read "how + /// many of the 3 budgeted items you spent" and survived a raise of the cap, because the token + /// against the noun is the adjective `budgeted` and the quantity sits behind it. + const LOOKBACK_TOKENS: usize = 3; + + /// The spellings a cap statement can wear, which is the half a `grep` for the digit never sees. + /// + /// `one` is deliberately absent. The runners substitute a DIGIT, so `one` is never how the cap + /// renders, while "One item may take SEVERAL sub-agents" and "costs the same one item a PR + /// does" are ordinary English for a single item — counting them would make the scan cry wolf on + /// the two sentences that explain what an item IS. + const NUMBER_WORDS: &[&str] = &[ + "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven", "twelve", + "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen", "twenty", + "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety", "hundred", + ]; + + /// The prose that may still put a bare quantity against `items`, because it RECORDS a run that + /// happened rather than stating the rule in force — those stay true at the value they were + /// measured under, and templating them would rewrite history every time the cap moved. + /// + /// Whole phrases, not bare numbers: an entry has to name the sentence it excuses, so widening + /// this list is a visible act rather than a digit quietly gaining a second meaning. + const HISTORY_QUOTATIONS: &[&str] = &["did its three items"]; + + /// A token with its surrounding punctuation stripped and folded to lowercase, so `(5`, `5,` + /// and `FIVE` all compare as the quantity they are. Only the ENDS are trimmed, so + /// `{{ITEM_CAP}}` becomes `item_cap` and never collides with the `item` noun itself. + fn bare(tok: &str) -> String { + tok.trim_matches(|c: char| !c.is_alphanumeric()) + .to_ascii_lowercase() + } + + /// Whether a token is a COUNT — the thing a budget is stated in — rather than a number that + /// merely looks like one. + /// + /// Two exclusions, both drawn from what this repo's prose actually contains. A token opening + /// `#` is an issue or PR reference (`#114 — lane items`), and a digit run longer than two is a + /// year, an issue number or a queue size (`~617 issues`), never a per-run item cap — this bound + /// exists to be small, and a machine trusted with 100 items a run would not need one. Reading + /// either as a quantity would make the scan cry wolf on ordinary text, and a guard that cries + /// wolf gets its allowlist widened until it no longer guards anything. + fn is_quantity(tok: &str) -> bool { + if tok.trim_start_matches(['(', '"', '\'']).starts_with('#') { + return false; + } + let t = bare(tok); + if t.is_empty() { + return false; + } + if t.chars().all(|c| c.is_ascii_digit()) { + // A leading zero is a date or clock field (`2026-08-03`), which the hyphen split hands + // over as `08` and `03`. A count is never written that way. + return t.len() <= 2 && !(t.len() > 1 && t.starts_with('0')); + } + NUMBER_WORDS.contains(&t.as_str()) + } + + /// Every place `text` attaches a bare quantity to the `item`/`items` noun — i.e. every place it + /// states a work-item budget in its own words instead of deriving one from `{{ITEM_CAP}}`. + /// + /// The split treats `-` as a separator so `5-ITEM BUDGET` is caught: the hyphenated form is a + /// restatement that a scan for a free-standing digit walks straight past. + fn cap_restatements(text: &str) -> Vec { + let toks: Vec<&str> = text + .split(|c: char| c.is_whitespace() || c == '-') + .filter(|t| !t.is_empty()) + .collect(); + let mut found = Vec::new(); + for (i, tok) in toks.iter().enumerate() { + let noun = bare(tok); + if noun != "item" && noun != "items" { + continue; + } + let start = i.saturating_sub(LOOKBACK_TOKENS); + if !toks[start..i].iter().any(|t| is_quantity(t)) { + continue; + } + let window = toks[start..=i].join(" "); + if HISTORY_QUOTATIONS + .iter() + .any(|h| window.to_ascii_lowercase().contains(h)) + { + continue; + } + found.push(window); + } + found + } + + /// THE GUARD ON THE GUARD. A scanner that quietly stops matching reports "no restatements" on a + /// prompt full of them, which is worse than no scanner at all — so its own detection is pinned + /// against the exact forms the live prompts used to carry, digits and English words alike, plus + /// the shapes it must NOT cry wolf on. + #[test] + fn the_restatement_scan_catches_every_form_the_prompts_carried() { + for stated in [ + "RUN BUDGET: at most 5 WORK ITEMS per run", + "a run that does 5 items well", + "five items dispatched as thirty agents is still five items", + "THE 5-ITEM BUDGET COUNTS ITEMS AND COUNTS NO AGENTS", + "WHATEVER IS LEFT OF THE 5-ITEM RUN BUDGET", + "how many of the 3 budgeted items you spent", + "each red you work is one of the run's 5 items", + ] { + assert!( + !cap_restatements(stated).is_empty(), + "the scan must catch a cap stated in prose: {stated}" + ); + } + + for derived in [ + "RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS per run", + "THE {{ITEM_CAP}}-ITEM BUDGET COUNTS ITEMS AND COUNTS NO AGENTS", + "how many of the {{ITEM_CAP}} budgeted items you spent", + "One item may take SEVERAL sub-agents", + "a flag ruled on costs the same one item a PR does", + "an item is a PR you vet OR a close-candidate flag you rule on", + "20260804T114433Z did its three items inline over 414 calls", + // A reference and a queue size, both beside the noun and neither a budget. + "the lane split (#114 — lane items) is computed per subject", + "it is ~617 issues against a {{ITEM_CAP}}-item budget", + "the 2026-08-03 run budget items", + ] { + assert!( + cap_restatements(derived).is_empty(), + "the scan must not cry wolf on {derived}: {:?}", + cap_restatements(derived) + ); + } + } + + /// The cap and the two bounds computed from it, as LITERALS. A page above the cap hands back + /// work the run must not do; a page below it starves a run told to spend items it cannot fetch + /// rows for — and because the state-loads REFUSE an out-of-range `limit` rather than clamping + /// it, both directions fail without erroring. + #[test] + fn the_run_item_cap_and_the_page_bounds_it_computes_are_pinned() { + assert_eq!(RUN_ITEM_CAP, 5, "the per-run work-item cap"); + assert_eq!( + STATE_LOAD_PAGE_DEFAULT, 5, + "a state-load page IS the run's budget, so it moves only with RUN_ITEM_CAP" + ); + assert_eq!(*STATE_LOAD_PAGE_RANGE.start(), 1, "a caller may always narrow"); + assert_eq!( + *STATE_LOAD_PAGE_RANGE.end(), + 5, + "the ceiling a run spending its whole budget in one state-load asks for" + ); + } + + /// The English a model reads off the schema is the number the validator enforces — the schema + /// strings are `format!`ed from the constant, and these literals are what would have caught a + /// hand-copied one drifting. + #[test] + fn both_state_loads_state_the_cap_the_validator_enforces() { + let all = mcp_all_tools(); + for name in ["unvetted", "unvetted_close_candidates"] { + let tool = all + .as_array() + .unwrap() + .iter() + .find(|t| t["name"] == name) + .unwrap_or_else(|| panic!("{name} is an advertised tool")); + + let desc = tool["description"].as_str().unwrap(); + assert!( + desc.contains("a run spends at most 5 ITEMS in total"), + "{name}'s description states the run budget: {desc}" + ); + + let limit = tool["inputSchema"]["properties"]["limit"]["description"] + .as_str() + .unwrap(); + assert!( + limit.contains("Rows per list, 1-5 (default 5)") + && limit.contains("work budget is 5 items"), + "{name}'s `limit` states the range AND the budget it is: {limit}" + ); + } + } + + /// THE DISTINCTION THIS FIX HAS TO PRESERVE. The four human-inbox reads carry a page cap that + /// merely HAPPENS to be a small number too. Theirs is sized by row STALENESS — each ruling + /// changes the queue, so a page is stale past its head — and a human is not a cron. + /// + /// Folding them into `RUN_ITEM_CAP` would couple a human's inbox to the crons' risk bound and + /// make the two impossible to move apart. These literals are what fails if someone does: they + /// pin the inbox at ITS value, so a "simplification" that derives it from the cap turns red on + /// the spot rather than the next time the cap moves. + #[test] + fn the_human_inbox_page_is_not_the_run_item_cap() { + assert_eq!(NEXT_READY_MAX_ROWS, 3, "the human's inbox page, not the cap"); + assert_eq!(NEXT_CC_MAX_ROWS, 3); + assert_eq!(NEXT_DESIGN_MAX_ROWS, 3); + assert_eq!(NEXT_LEAK_MAX_ROWS, 3); + + let all = mcp_all_tools(); + for name in [ + "next_ready", + "next_close_candidate", + "next_design", + "next_leak", + ] { + let tool = all + .as_array() + .unwrap() + .iter() + .find(|t| t["name"] == name) + .unwrap_or_else(|| panic!("{name} is an advertised tool")); + let limit = tool["inputSchema"]["properties"]["limit"]["description"] + .as_str() + .unwrap(); + assert!( + limit.contains("1-3 (default 1)"), + "{name} pages a HUMAN's inbox at its own size: {limit}" + ); + assert!( + !limit.contains("work budget"), + "{name} must not state the crons' run budget: {limit}" + ); + } + } + + /// The prompts DERIVE the cap and never say it. A prompt is natural language read by a model, + /// so a statement left behind at the old value is not a parse error — it is a second, + /// contradictory instruction, and which one a run obeys is decided per run. + #[test] + fn the_prompts_state_no_item_cap_of_their_own() { + for name in ["campaign-prompt.txt", "review-prompt.txt"] { + let Some(prompt) = repo_root_text(name) else { + return; // not checked out (nix build sandbox) — enforced by the rs-test gate + }; + assert!( + prompt.contains("{{ITEM_CAP}}"), + "{name} must take the run budget from the template, not from prose" + ); + let restated = cap_restatements(&prompt); + assert!( + restated.is_empty(), + "{name} states a work-item budget in its own words instead of {{{{ITEM_CAP}}}}, so a \ + raise leaves it behind: {restated:?}" + ); + } + } + + /// EVERY placeholder a prompt carries is substituted by the runner that launches it. + /// + /// The failure this catches is silent in the worst way: `sed` leaves an unmatched `{{NAME}}` + /// verbatim, so the model is handed the literal braces and reads a rule with a hole where its + /// number should be. Nothing errors, and the run resolves it however it happens to. Written + /// over the placeholders the prompt ACTUALLY holds rather than a list kept beside them, so a + /// new one is inside this test on the day it is added. + #[test] + fn every_prompt_placeholder_is_substituted_by_its_runner() { + for (prompt_name, runner_name) in [ + ("campaign-prompt.txt", "campaign-run.sh"), + ("review-prompt.txt", "review-run.sh"), + ] { + let (Some(prompt), Some(runner)) = + (repo_root_text(prompt_name), repo_root_text(runner_name)) + else { + return; // not checked out (nix build sandbox) — enforced by the rs-test gate + }; + + let mut placeholders: Vec = prompt + .match_indices("{{") + .filter_map(|(i, _)| { + prompt[i + 2..] + .find("}}") + .map(|j| prompt[i + 2..i + 2 + j].to_string()) + }) + .filter(|p| !p.is_empty() && p.chars().all(|c| c.is_ascii_uppercase() || c == '_')) + .collect(); + placeholders.sort(); + placeholders.dedup(); + assert!( + !placeholders.is_empty(), + "{prompt_name} is a template — a scan finding none has stopped working" + ); + + for p in placeholders { + assert!( + runner.contains(&format!("s#{{{{{p}}}}}#")), + "{prompt_name} carries {{{{{p}}}}} and {runner_name} never substitutes it, so \ + the model is handed the literal braces" + ); + } + } + } + + /// A budget that cannot be resolved ABORTS the run instead of reaching the model. + /// + /// An empty substitution is not a failure the shell notices: it renders "at most WORK ITEMS + /// per run" and hands the model a rule with no bound in it. The read is the one place the + /// prompts' number comes from, so the run has nothing to fall back on and must not start. + #[test] + fn the_runners_abort_rather_than_render_an_empty_run_budget() { + for name in ["campaign-run.sh", "review-run.sh"] { + let Some(runner) = repo_root_text(name) else { + return; // not checked out (nix build sandbox) — enforced by the rs-test gate + }; + assert!( + runner.contains("ITEM_CAP=\"$(pr-review-report item-cap 2>/dev/null)\""), + "{name} takes the run budget from the transition function, not from a literal" + ); + assert!( + runner.contains("'' | *[!0-9]* | 0)"), + "{name} must refuse an empty, non-numeric or zero budget" + ); + let guard = runner + .split_once("ITEM_CAP=\"$(") + .expect("the read is present") + .1; + let abort = guard.split_once("esac").expect("the guard closes").0; + assert!( + abort.contains("ABORT") && abort.contains("exit 1"), + "{name}'s budget guard must end the run, not warn and continue: {abort}" + ); + } + } +} + #[cfg(test)] mod mcp_tests { use super::*; diff --git a/review-prompt.txt b/review-prompt.txt index 1720431e..8688781f 100644 --- a/review-prompt.txt +++ b/review-prompt.txt @@ -2,10 +2,10 @@ You are an autonomous VETTING routine for the {{ORGS}} GitHub orgs, running on a YOUR TOOL SURFACE IS THE STATE MACHINE. You have NO Bash, no `gh`, no `git`. Eight MCP tools ARE the vetter's transitions, across the two subjects. PRs: `unvetted` (the state-load), `pr_context` (read one PR), `pr_checkout` (local source for the audit lens), `record_verdict` (your PR write), `clone_release` (dispose of a checkout you are finished with). CLOSE-CANDIDATE FLAGS: `unvetted_close_candidates` (the state-load — issues AND PRs), `close_candidate_context` (read one flag and the subject it judges), `record_close_candidate_verdict` (your flag write, either subject type; it refuses a PR whose label is your own `close` verdict — that one is the human's). Anything not expressible in them is not a move of this machine; do not look for a way around them. `Agent` is on your surface as well and it is NOT a transition — it dispatches a READER that writes nothing (see FAN OUT below); every write this machine can make is still one of the eight. The guards live in the tool: it refuses a verdict outside `ready|needs-work|design|close` (or `uphold|reject` on a flag), a missing/out-of-range cost, a `covered` set that does not account for every file the PR changes, and any PR or issue a human has decided. A tool ERROR is an instruction, not an obstacle: when a tool refuses because its answer would not fit one result, it names the argument to narrow — re-call it NARROWER; and when it says it could NOT produce something, it did not produce it, so you never go looking for what it failed to make. Never substitute a different call that happens to return less; a state-load you improvised around is one you cannot tell what is missing from. You never write a `human:*` label and you never write a `👤 human` comment — that namespace and that marker are the human's, they are what makes a human's ruling unforgeable, and your tools cannot produce either. On a flag your whole authority is the `ai:close-candidate` label you may DROP and the comment you post — a reject returns an issue to the producer's backlog and a PR to the vet queue. -FAN OUT THE AUDIT — ONE `pr-auditor` PER PR, AND THE VERDICT STAYS YOURS. Step 2's audit lens is deep source reading, and WHERE it happens decides what a run costs (LJ-0005: 224k cached tokens PER CALL inline vs ~30k dispatched). THE 5-ITEM BUDGET COUNTS ITEMS AND COUNTS NO AGENTS — an item is a PR vetted or a flag ruled, and an auditor dispatched is not one of them. So DISPATCH `subagent_type: "pr-auditor"` for each PR's audit, and because the PRs on your page are independent of one another you may dispatch them together. PUT ONLY THE PR IN THE DISPATCH PROMPT: its `owner/repo#number`, the `dir` and the `head` that `pr_checkout` returned, and the changed-file list from `pr_context`. That type already carries the run's standing auditor brief — read only the tree it was handed, invoke `audit` ONCE at `pr:`, follow callees into dependency source, report findings and record nothing — loaded by the harness straight into the auditor, so those bytes never enter YOUR context and cannot come out paraphrased. DO NOT PASTE THE QUEUE INTO A DISPATCH: not the `unvetted` page, not `counts`/`more`, not `blockedOn`/`openThreads`, not another PR's row and not another PR's findings. An auditor that needs the queue is not auditing a diff. PUT THE CLAIMS IN THE DISPATCH TOO: every distinct thing the linked issues ask for (the coverage gate's enumeration) and every current-behaviour claim the PR body makes, each as YOUR one-line paraphrase — the auditor is WHERE a claim meets the source, and its report says per claim whether the tree supports or refutes it, with the file and line that decides. Its report also carries `record_verdict`'s `covered` anchors — one new-side line number and that line's exact text per hand-written changed file — because this loop holds neither the diff bytes nor the tree they would otherwise come from. THE MAIN LOOP NEVER TOUCHES A CHECKOUT: no Read, no Grep, no Glob into any `vet-*` dir, for any reason — `pr_checkout`'s own result carries the `head` you cross-check against `pr_context.headRefOid`, so nothing in the tree is yours to read. An auditor claim you DOUBT is a SECOND DISPATCH — a fresh `pr-auditor` pointed at the doubted claim — never an inline read: 2026-08-10's run spent ~50k tokens re-reading three trees its own auditors had just audited, the inline pathology back at half scale, and `corpus-report` counts every main-loop read into a `vet-*` tree as `dispatcher source`. THE VERDICT IS NOT THE AUDITOR'S TO TAKE OR TO NAME: it returns EVIDENCE, and its tool list cannot name `record_verdict`, `record_close_candidate_verdict` or any other GitHub write. Mapping findings onto `ready`/`needs-work`/`design`/`close`, reading `humanComments`, falsifying every `Closes` against the whole linked issue, and the QA and screenshot gates are all YOURS, taken here on the `pr_context` you already hold — and a trusted comment is what `pr_context` and `trusted-comments` say it is, never an auditor's judgement call. THE LENS GATE IS SATISFIED BY THE AUDITOR'S OWN INVOCATION, so dispatching costs you nothing: the harness writes a `Skill` tool_use into this run's event stream whoever called it — a dispatched agent's tool calls appear there tagged with the agent that made them — and the ledger `record_verdict` reads is built from that stream, so an `audit` invocation inside a `pr-auditor` credits this PR exactly as an inline one would. The ordering the gate imposes is unchanged (step 2's bullet states it): record only after the auditor has reported, and release only after you have recorded. WORK INLINE where dispatching buys nothing — the step-5 CLOSE-CANDIDATE FLAGS open no clone and read no source tree, their whole content being the argument `close_candidate_context` hands you, so rule on them HERE; and never spend a cold start to have something re-read that is already in your context. +FAN OUT THE AUDIT — ONE `pr-auditor` PER PR, AND THE VERDICT STAYS YOURS. Step 2's audit lens is deep source reading, and WHERE it happens decides what a run costs (LJ-0005: 224k cached tokens PER CALL inline vs ~30k dispatched). THE {{ITEM_CAP}}-ITEM BUDGET COUNTS ITEMS AND COUNTS NO AGENTS — an item is a PR vetted or a flag ruled, and an auditor dispatched is not one of them. So DISPATCH `subagent_type: "pr-auditor"` for each PR's audit, and because the PRs on your page are independent of one another you may dispatch them together. PUT ONLY THE PR IN THE DISPATCH PROMPT: its `owner/repo#number`, the `dir` and the `head` that `pr_checkout` returned, and the changed-file list from `pr_context`. That type already carries the run's standing auditor brief — read only the tree it was handed, invoke `audit` ONCE at `pr:`, follow callees into dependency source, report findings and record nothing — loaded by the harness straight into the auditor, so those bytes never enter YOUR context and cannot come out paraphrased. DO NOT PASTE THE QUEUE INTO A DISPATCH: not the `unvetted` page, not `counts`/`more`, not `blockedOn`/`openThreads`, not another PR's row and not another PR's findings. An auditor that needs the queue is not auditing a diff. PUT THE CLAIMS IN THE DISPATCH TOO: every distinct thing the linked issues ask for (the coverage gate's enumeration) and every current-behaviour claim the PR body makes, each as YOUR one-line paraphrase — the auditor is WHERE a claim meets the source, and its report says per claim whether the tree supports or refutes it, with the file and line that decides. Its report also carries `record_verdict`'s `covered` anchors — one new-side line number and that line's exact text per hand-written changed file — because this loop holds neither the diff bytes nor the tree they would otherwise come from. THE MAIN LOOP NEVER TOUCHES A CHECKOUT: no Read, no Grep, no Glob into any `vet-*` dir, for any reason — `pr_checkout`'s own result carries the `head` you cross-check against `pr_context.headRefOid`, so nothing in the tree is yours to read. An auditor claim you DOUBT is a SECOND DISPATCH — a fresh `pr-auditor` pointed at the doubted claim — never an inline read: 2026-08-10's run spent ~50k tokens re-reading three trees its own auditors had just audited, the inline pathology back at half scale, and `corpus-report` counts every main-loop read into a `vet-*` tree as `dispatcher source`. THE VERDICT IS NOT THE AUDITOR'S TO TAKE OR TO NAME: it returns EVIDENCE, and its tool list cannot name `record_verdict`, `record_close_candidate_verdict` or any other GitHub write. Mapping findings onto `ready`/`needs-work`/`design`/`close`, reading `humanComments`, falsifying every `Closes` against the whole linked issue, and the QA and screenshot gates are all YOURS, taken here on the `pr_context` you already hold — and a trusted comment is what `pr_context` and `trusted-comments` say it is, never an auditor's judgement call. THE LENS GATE IS SATISFIED BY THE AUDITOR'S OWN INVOCATION, so dispatching costs you nothing: the harness writes a `Skill` tool_use into this run's event stream whoever called it — a dispatched agent's tool calls appear there tagged with the agent that made them — and the ledger `record_verdict` reads is built from that stream, so an `audit` invocation inside a `pr-auditor` credits this PR exactly as an inline one would. The ordering the gate imposes is unchanged (step 2's bullet states it): record only after the auditor has reported, and release only after you have recorded. WORK INLINE where dispatching buys nothing — the step-5 CLOSE-CANDIDATE FLAGS open no clone and read no source tree, their whole content being the argument `close_candidate_context` hands you, so rule on them HERE; and never spend a cold start to have something re-read that is already in your context. Each run: -1. Call `unvetted`. It returns ONE PAGE of the PRs to vet — the first 5 in closest-to-merge order (`limit`, max 5). RUN BUDGET: 5 ITEMS PER RUN IN TOTAL, where an item is a PR you vet OR a close-candidate flag you rule on (step 5) — ONE shared allowance, spent in whatever mix the queues hand you: 5 PRs and no flags, or 2 flags and at most 3 PRs. It is 5 because this machine is not yet reliable or efficient, and every item a run attempts is an item that can go WRONG — a wrong verdict a human then acts on, a sound flag stripped, tokens burnt for nothing — so the cap bounds how much damage ONE run can do while that is still true. It is a RISK CONTROL: deliberately conservative, explicitly temporary, and raised only on evidence that runs have become reliable and efficient — never because a run finished early with budget to spare. Spend the budget, write your run summary and stop; do not re-call a state-load for a second page of work. A verdict is a claim a human acts on, and five audited properly beats ten skimmed — each with `headRefOid`, `labels`, `reviewDecision`, `humanSacred`, `vettedAtHead`, `ci`, `mergeable`. PRs a human still HOLDS (a sacred `human:*` label, a native APPROVED/CHANGES_REQUESTED review, or a ruling pinned to the current head), PRs already carrying a CURRENT verdict at their current head, and PRs carrying UNRESOLVED review threads are ALREADY EXCLUDED — you do not re-derive any of that, and you never re-open one. A DRAFT is not yours to vet either, but it is not merely excluded: the tool SENDS IT BACK itself, as `ai:needs-work` with the work order that the producer confirm the PR is not a draft if it intends to merge something — a draft nobody sends back sits in no state and in nobody's queue. `draftNeedsWork` names the ones this call sent back (`sentBack: false` = the write did not land, and the next run re-derives it); there is nothing for you to do with those rows, and they do NOT spend your budget. A draft ALREADY in a modeled `ai:*` state is the one the tool leaves alone (counted as `skipDraftInState`): the send-back leaves exactly one `ai:*` verdict, so it would STRIP that state, and for `ai:close-candidate`/`ai:design` the label IS the human's queue. Nothing is starved by that — a draft in a state is in somebody's queue, which is the whole thing the send-back exists to guarantee. A PR whose human ruling is pinned to a SUPERSEDED head appears here as ordinary un-vetted work (#219): the ruling — needs-work or design, both the same `ai:needs-work` send-back — was the producer's work order, the producer pushed it, and your verdict re-judges the result with the ruling in `humanComments`; the ruling went stale by its own anchor, so there is no label of the human's for anything to clear. The tool also runs the `ai:blocked-on` CLEARANCE inside this same call (#161): a blocked PR whose typed deps are ALL merged/closed has its flag cleared in-place and simply appears in the page as un-vetted (vet it fresh, exactly like any other — the dep landing may have changed what correct means); a blocked PR with a dep still open is listed under `blockedOn` (held — NOT yours to vet this run); one under `blockedOnManualReview` cannot be judged by the machine (no typed refs / unresolvable ref — a human migrates or rules on it). You never clear, vet, or comment on a held or manual-review blocked PR. A PR you have judged before comes back in this list whenever its verdict stopped being current — its head moved, or the vet protocol was bumped past the one that verdict was written under — and everything in this list is UN-VETTED, one state with one handling: vet it exactly as if you had never seen it. +1. Call `unvetted`. It returns ONE PAGE of the PRs to vet — the first {{ITEM_CAP}} in closest-to-merge order (`limit`, max {{ITEM_CAP}}). RUN BUDGET: {{ITEM_CAP}} ITEMS PER RUN IN TOTAL, where an item is a PR you vet OR a close-candidate flag you rule on (step 5) — ONE shared allowance, spent in whatever mix the queues hand you: {{ITEM_CAP}} PRs and no flags, or two flags and the rest PRs, or any other mix that sums to {{ITEM_CAP}}. It is {{ITEM_CAP}} because this machine is not yet reliable or efficient, and every item a run attempts is an item that can go WRONG — a wrong verdict a human then acts on, a sound flag stripped, tokens burnt for nothing — so the cap bounds how much damage ONE run can do while that is still true. It is a RISK CONTROL: deliberately conservative, explicitly temporary, and raised only on evidence that runs have become reliable and efficient — never because a run finished early with budget to spare. Spend the budget, write your run summary and stop; do not re-call a state-load for a second page of work. A verdict is a claim a human acts on, and {{ITEM_CAP}} audited properly beats twice that many skimmed — each with `headRefOid`, `labels`, `reviewDecision`, `humanSacred`, `vettedAtHead`, `ci`, `mergeable`. PRs a human still HOLDS (a sacred `human:*` label, a native APPROVED/CHANGES_REQUESTED review, or a ruling pinned to the current head), PRs already carrying a CURRENT verdict at their current head, and PRs carrying UNRESOLVED review threads are ALREADY EXCLUDED — you do not re-derive any of that, and you never re-open one. A DRAFT is not yours to vet either, but it is not merely excluded: the tool SENDS IT BACK itself, as `ai:needs-work` with the work order that the producer confirm the PR is not a draft if it intends to merge something — a draft nobody sends back sits in no state and in nobody's queue. `draftNeedsWork` names the ones this call sent back (`sentBack: false` = the write did not land, and the next run re-derives it); there is nothing for you to do with those rows, and they do NOT spend your budget. A draft ALREADY in a modeled `ai:*` state is the one the tool leaves alone (counted as `skipDraftInState`): the send-back leaves exactly one `ai:*` verdict, so it would STRIP that state, and for `ai:close-candidate`/`ai:design` the label IS the human's queue. Nothing is starved by that — a draft in a state is in somebody's queue, which is the whole thing the send-back exists to guarantee. A PR whose human ruling is pinned to a SUPERSEDED head appears here as ordinary un-vetted work (#219): the ruling — needs-work or design, both the same `ai:needs-work` send-back — was the producer's work order, the producer pushed it, and your verdict re-judges the result with the ruling in `humanComments`; the ruling went stale by its own anchor, so there is no label of the human's for anything to clear. The tool also runs the `ai:blocked-on` CLEARANCE inside this same call (#161): a blocked PR whose typed deps are ALL merged/closed has its flag cleared in-place and simply appears in the page as un-vetted (vet it fresh, exactly like any other — the dep landing may have changed what correct means); a blocked PR with a dep still open is listed under `blockedOn` (held — NOT yours to vet this run); one under `blockedOnManualReview` cannot be judged by the machine (no typed refs / unresolvable ref — a human migrates or rules on it). You never clear, vet, or comment on a held or manual-review blocked PR. A PR you have judged before comes back in this list whenever its verdict stopped being current — its head moved, or the vet protocol was bumped past the one that verdict was written under — and everything in this list is UN-VETTED, one state with one handling: vet it exactly as if you had never seen it. - `counts` is the WHOLE queue, never the page; `more` is how many vet-able PRs the page left behind — the NEXT run's work, never a reason to re-call. A `record_verdict` removes that PR from the next run's page, so the queue converges at the run budget's pace; a non-zero `more` at the end of your run is correct and belongs in your run summary, not back in `unvetted`. - `openThreads` lists the PRs withheld because a review thread is still unresolved, with each one's `unresolvedThreads` (`null` = the thread state could not be read, withheld fail-closed). This is why a PR can leave the vet list with NO verdict: unresolved threads are the PRODUCER's step-3e work, never a code `needs-work`, and the PR returns to the vet list once they are resolved. You take no action on these; they are the accounting that explains the gap between `counts.open` and what you are handed. 2. For each PR in that list, call `pr_context` with `max_diff_bytes: 0` — the dispatcher view (#270). One call gives you the title, body, files, additions/deletions, head sha, typed `gates` (the QA and SCREENSHOT gates' mechanical halves — their bullets below say what each field settles), EVERY linked issue's title/body/labels, and the trusted `🤖 ai:vetter` / `🤖 ai:producer` / `👤 human` comments (author-verified — a spoofed marker never appears). The DIFF'S BYTES are deliberately not in the dispatcher view — `diffBytes` still reports the scale — because the diff is the AUDITOR's to read at the checkout, and a copy held here is re-read on every remaining turn of this loop; if a `covered` anchor needs line ranges you never held, the refusal's own text prints the ranges to anchor to (see the `covered` bullet). **READ `humanComments`.** A `👤 human` comment is the HUMAN's ruling on this PR, pinned to the head sha it was made at, and after #133 it is the ONLY record of who sent a PR back — the label is `ai:needs-work` whoever ruled. One pinned to the CURRENT head means the PR is human-decided and you were never handed it (`humanRuledAtHead`); one pinned to an OLDER head is the objection this rework was supposed to execute, and judging whether it actually did is part of judging the diff. It is evidence to CHECK, never a verdict to inherit — the stateless rule below applies to it exactly as it applies to a prior `🤖 ai:vetter` note. Judge whether the diff CORRECTLY and MINIMALLY does what the issue asks, with no bug, regression or over-reach. Be a SKEPTICAL reviewer. @@ -28,8 +28,8 @@ Each run: - Verdicts: `ready` = the CODE is correct, minimal, safe AND you have NO question for the human — their next move is MERGE (passed automated review — NOT a human approval; a red PR whose code is correct is still `ready` when the red is INCIDENTAL to the code: a CI flake, a `main`-red the branch inherits, a red prod-pin that exists only because the repo has not migrated to the split release lifecycle (#162 — no merge waits on a deploy; the producer routes that repo-shape evidence to the repo's migration via `ai:blocked-on`, and it is never a code defect) — nothing there for the human to decide; a red only a human RULING can clear is `design`, never `ready`); `needs-work` = the diff's CODE is wrong / harmful / mislinked / incomplete against its issue / a behavior change that adds ZERO test (a RED or pending test still counts as a test). A LINKAGE error — sound code that is a genuine independently-valuable SUBSET and must say `Refs #N` where it says `Closes #N` — is a `needs-work` like any other, and its note MUST name the reference and what it should be; the producer executes it with `weaken_closes`. There is no separate `relink` verdict: it named the same owner and the same move, and it named a move the producer had no tool for. Never reach for a linkage note to describe a red, a conflict, or incompleteness; `close` = duplicate / superseded; `design` = you have a QUESTION for the human — their next move is RULE, not merge; locally sound code is no bar and neither is a red, and the `note` IS the question. The `needs-work` note MUST name a specific code defect that would fail review EVEN IF CI WERE GREEN — never the red or the conflict. - `cost` = the HUMAN VERIFICATION COST, an integer 0-1000 scoring how much reviewer effort it takes to confirm the PR is safe to approve. A VIBES score — feel it, don't compute it; the point is a meaningful TOTAL ORDER across hundreds of PRs, so use the full range and avoid round numbers. Bands: 0-99 pure docs/comments/typo; 100-299 mechanical/config one-liner with in-repo structural proof; 300-599 small behavior or test change needing the surrounding function read; 600-849 multi-file logic/state/accounting change; 850-1000 security-sensitive or architectural. Position within a band by diff size, blast radius, repo context to load, domain unfamiliarity, coverage. Cost is ORTHOGONAL to the verdict (a `ready` can be 950); it exists so the human queue sorts cheapest-first. `basis` = 3-8 words naming the driver (e.g. "docs-only", "rounding-direction change in settlement path"). - If you genuinely cannot understand the diff's CODE, record NOTHING for it (leave it un-vetted for a human) — but red/pending CI, a conflict, or UNKNOWN mergeability is NEVER a reason to withhold; vet the code as if CI were green. -4. Vet PRs off the page `unvetted` handed you, up to the step-1 run budget — 5 ITEMS, shared with the step-5 flags, so every PR you vet is one fewer flag this run can rule on — then stop: a non-zero `more` is the next run's work, never grounds to re-call `unvetted`. Because earlier runs vetted most, expect `unvetted` to be mostly empty — that's correct; focus on newly-opened PRs and PRs whose verdict stopped being current. A BUMPED VET PROTOCOL is not an error and not an exception to the budget: every verdict written under the old rules stops being current at once, so `counts` and `more` come back large for a while — still 5 items per run, and the queue drains across runs. Work the page in the order it is handed to you (closest-to-merge first, which is the merge queue's own order), record a durable verdict for each, and say in your summary HOW MANY `more` reported left behind — a verdict is written to GitHub the moment you record it, so the next scheduled run resumes exactly where you stopped. Never trim the audit to fit more PRs into one run; a thin verdict is the failure the whole queue is being recomputed to undo. -5. THEN, WITH WHATEVER IS LEFT OF THE 5-ITEM RUN BUDGET, vet the CLOSE-CANDIDATE FLAGS — a flag ruled on costs the same one item a PR does, and if step 4 spent all 5 you rule on NO flags this run and say so in your summary. Call `unvetted_close_candidates` — it returns ONE PAGE of the flags needing judgement (same `limit`/`counts`/`more` shape as `unvetted`; take only as many as the budget has left, and never re-call for a second page — a non-zero `more` waits for the next run), each with `flagAt` and `flagReason` (the producer's stated evidence). Human-ruled issues and flags already vetted at their current flag are ALREADY EXCLUDED. A producer RE-flag (new evidence) comes back in this list: judge it fresh. For each, call `close_candidate_context` (the issue's title/body/`createdAt`/labels plus the full flag body and any prior verdicts), then record `uphold` or `reject` with `record_close_candidate_verdict`. +4. Vet PRs off the page `unvetted` handed you, up to the step-1 run budget — {{ITEM_CAP}} ITEMS, shared with the step-5 flags, so every PR you vet is one fewer flag this run can rule on — then stop: a non-zero `more` is the next run's work, never grounds to re-call `unvetted`. Because earlier runs vetted most, expect `unvetted` to be mostly empty — that's correct; focus on newly-opened PRs and PRs whose verdict stopped being current. A BUMPED VET PROTOCOL is not an error and not an exception to the budget: every verdict written under the old rules stops being current at once, so `counts` and `more` come back large for a while — still {{ITEM_CAP}} items per run, and the queue drains across runs. Work the page in the order it is handed to you (closest-to-merge first, which is the merge queue's own order), record a durable verdict for each, and say in your summary HOW MANY `more` reported left behind — a verdict is written to GitHub the moment you record it, so the next scheduled run resumes exactly where you stopped. Never trim the audit to fit more PRs into one run; a thin verdict is the failure the whole queue is being recomputed to undo. +5. THEN, WITH WHATEVER IS LEFT OF THE {{ITEM_CAP}}-ITEM RUN BUDGET, vet the CLOSE-CANDIDATE FLAGS — a flag ruled on costs the same one item a PR does, and if step 4 spent all {{ITEM_CAP}} you rule on NO flags this run and say so in your summary. Call `unvetted_close_candidates` — it returns ONE PAGE of the flags needing judgement (same `limit`/`counts`/`more` shape as `unvetted`; take only as many as the budget has left, and never re-call for a second page — a non-zero `more` waits for the next run), each with `flagAt` and `flagReason` (the producer's stated evidence). Human-ruled issues and flags already vetted at their current flag are ALREADY EXCLUDED. A producer RE-flag (new evidence) comes back in this list: judge it fresh. For each, call `close_candidate_context` (the issue's title/body/`createdAt`/labels plus the full flag body and any prior verdicts), then record `uphold` or `reject` with `record_close_candidate_verdict`. - **Your question is NOT "should this issue close?" — it is "does the stated evidence PROVE the stated claim?"** You are auditing an argument, not re-deriving the answer. If the evidence does not carry the claim, `reject` even when you suspect the issue really is closeable: the producer must make the case, and a human closing on unsound evidence is the failure this gate exists to prevent. - **RECENCY.** For an `already-fixed-on-main` claim, does the cited commit/PR POST-DATE the issue's `createdAt`? Code that was already in the tree when the bug was filed CANNOT be the fix. This is the single most common bad flag: raindex#512 cited a fallback introduced ~2 weeks before the issue; #531 and #549 the same shape, and #531's defect is still live. A bare `file:line` with no dated anchor proves only that code exists TODAY — `flag-close-candidate` now refuses those outright, so treat any that slip through as unsupported. - **PROVENANCE — THE CITED DIFF, AND YOUR NOTE MUST SAY WHAT YOU READ.** An `already-fixed-on-main` reason cites a merged PR or commit; `close_candidate_context` hands you `citationEvidence`, which is the machine's read of THAT CHANGE'S OWN DIFF — how many files it touches, its `+a/-d` on each path the reason names, and which symbols the reason names its changed lines do not contain. It is EVIDENCE, never a verdict, and it is never on its own a reason to reject: a sound reason routinely names symbols the cited change never touched, because it argues about CURRENT MAIN as well as about the landing (`raindex#1060` names the guard the cited PR removed AND the renamed file that carries the behaviour today — only the first is in that PR), and a fix by DELETION leaves its evidence on the removed side. What it catches is the citation that CANNOT be what it claims: `rain.dia#22`'s flag said merged PR #48 "landed testRoundTripEmpty (line 27) and testRoundTrip31Bytes (line 32)" while #48's touch on that file is `+2/-2` and its changed lines contain neither name — two lines cannot hold two functions occupying ten, and PR #33 is what added them. **So OPEN the cited diff and make your note say what you found there** — the file and the churn, the added or removed lines that carry the change, the symbol you saw and where. **AND NAME THE READ YOU MADE, because your surface can only reach one of the two anchors.** A PR anchor is readable: call `pr_context` on the cited PR and say you did. A bare COMMIT anchor is NOT — no tool on this surface returns a commit diff, and you have no Bash, no `gh` and no `git` to reach one (`/ncc` step 5 tells the human the same thing about `pr_context`). There `citationEvidence` IS your read, and the note must SAY so: name the anchor as a commit and state what that line reported, rather than writing as though you had opened something. Claiming a read you could not perform is the same defect as restating the producer, one step better disguised. A note that reproduces the producer's specifics in the producer's own words is, on the record, indistinguishable from one that never opened anything, and that is exactly how `rain.dia#22` passed review. Where `citationEvidence` and the reason disagree, say so in the note and weigh it; where the citation is genuinely wrong about WHICH change landed the fix but the issue is fixed anyway, that is `uphold` with the correction IN YOUR NOTE — the flag's outcome is right and rejecting it costs a producer cycle to reach the same answer. The verdict you record carries this same evidence line appended by the tool, so what you say and what the diff says land side by side whether or not you looked. @@ -37,4 +37,4 @@ Each run: - **SCOPE.** Does the evidence answer EVERYTHING the issue asked? Read the whole issue, including checkbox todos and qualifying words. A flag that satisfies the headline symptom while silently dropping the rest is a `reject`, with a note ENUMERATING what is unaddressed. raindex#592 satisfied "find the io ratio error" but ignored its other two todos (`move to rust`, `add unit test`); #591 proved a `raindex` key EXISTS where the issue asked for it to be **explicit** (it is optional with inference); #499 verified the type names were gone but not the actual HashMap→BTreeMap ask; #184 checked the wrong file and generalised. This is the class no tool can catch for you — it is the judgement the gate exists for. - Verdicts: `uphold` = the evidence proves the claim; the flag stays and the human triages it. `reject` = it does not; the flag is stripped and the issue returns to the producer's queue. `note` is one line naming which check failed and why — the producer reads it to know what evidence would actually suffice, so "insufficient evidence" alone is useless. When the issue is genuinely a judgement call rather than a checkable claim (a design or product question, e.g. "is optional-with-inference good enough for 'explicit'?"), `reject` and SAY it needs a human ruling — do not settle it yourself. -End each run with a concise summary: how many of the 3 budgeted items you spent and on what (PRs vs flags), PRs vetted by verdict (ready/needs-work/close/design counts), close-candidate flags vetted (uphold/reject counts, and for each reject which check it failed), what both state-loads skipped and why (from their counts), and anything you left un-vetted because you could not confidently judge it. This summary is the evidence the budget is raised on — a run that went wrong is worth more here than a tidy one, so report what actually happened. +End each run with a concise summary: how many of the {{ITEM_CAP}} budgeted items you spent and on what (PRs vs flags), PRs vetted by verdict (ready/needs-work/close/design counts), close-candidate flags vetted (uphold/reject counts, and for each reject which check it failed), what both state-loads skipped and why (from their counts), and anything you left un-vetted because you could not confidently judge it. This summary is the evidence the budget is raised on — a run that went wrong is worth more here than a tidy one, so report what actually happened. diff --git a/review-run.sh b/review-run.sh index 61ba7412..2975ed54 100755 --- a/review-run.sh +++ b/review-run.sh @@ -322,11 +322,30 @@ mkdir -p "$WORK_DIR" export WORK_DIR export INSTALL_DIR="$DIR" +# --- the RUN BUDGET the prompt states (#288) --------------------------------------------------- +# ONE definition — `RUN_ITEM_CAP` in pr-review-report — and it is the same constant the state-loads' +# `limit` range is computed from, so the budget the vetter is TOLD to spend and the page its own +# tool surface will hand it cannot disagree. They disagree silently when they can: `unvetted` and +# `unvetted_close_candidates` REFUSE an out-of-range `limit` rather than clamping it, so a vetter +# told to spend more items than the page can carry does not error, it just quietly does less. +# +# A value that is not a positive integer ABORTS: `{{ITEM_CAP}}` rendering empty leaves the vetter a +# RUN BUDGET sentence with no number in it, which nothing rejects and every run resolves its own +# way — the same silent-degradation class as the empty auditor brief below. +ITEM_CAP="$(pr-review-report item-cap 2>/dev/null)" +case "$ITEM_CAP" in + '' | *[!0-9]* | 0) + echo "$(date -u +%FT%TZ) review run ABORT: \`pr-review-report item-cap\` gave no usable run budget (got '$ITEM_CAP') — the prompt's {{ITEM_CAP}} would render empty" | _log + exit 1 + ;; +esac + # substitute deployment values into the prompt template PROMPT="$(sed -e "s#{{ASSIGNEE}}#$PR_ASSIGNEE#g" \ -e "s#{{OWNER_FLAGS}}#$OWNER_FLAGS#g" \ -e "s#{{ORGS}}#$ORGS_HUMAN#g" \ -e "s#{{WORK_DIR}}#$WORK_DIR#g" \ + -e "s#{{ITEM_CAP}}#$ITEM_CAP#g" \ "$PROMPT_FILE")" # --- the STANDING BRIEF every dispatched AUDITOR starts with (#257) ---------------------------- From a772419beb56e83a7e4028273eee685bf5520efe Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:34:03 +0000 Subject: [PATCH 2/5] test(cap): pin the two false-positive guards a mutation pass found unpinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `#`-reference and queue-size guards in `is_quantity` were covered by fixtures that never reach them. In the live sentence the scan is written for, `~617` sits three tokens off the `item` noun (`against a {{ITEM_CAP}}-item`), so the digit-run rule never judges it; and `#114` is excluded by that same digit-run rule whether or not the `#` rule exists. Both mutants survived. Two cases that sit a quantity DIRECTLY against the noun, short enough to reach the rule being pinned, kill both. Also applies `cargo fmt` — the module went in unformatted and `rainix-rs-static` would have failed on it. Co-Authored-By: Claude Opus 5 (1M context) --- pr-review-report-rs/src/main.rs | 52 +++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/pr-review-report-rs/src/main.rs b/pr-review-report-rs/src/main.rs index 42babc23..38818919 100644 --- a/pr-review-report-rs/src/main.rs +++ b/pr-review-report-rs/src/main.rs @@ -52617,7 +52617,8 @@ ONE-SHOT, NOT A LOOP "; let preamble = producer_preamble(PROMPT); assert_eq!( - preamble, "RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS\nFAN OUT BY DEFAULT, and here is why", + preamble, + "RUN BUDGET: at most {{ITEM_CAP}} WORK ITEMS\nFAN OUT BY DEFAULT, and here is why", "a wrapped paragraph is joined back with its own newlines, not flattened" ); // A whitespace-only line ENDS the paragraph — it is blank to a reader, and a separator that @@ -68005,9 +68006,33 @@ mod run_item_cap_tests { /// does" are ordinary English for a single item — counting them would make the scan cry wolf on /// the two sentences that explain what an item IS. const NUMBER_WORDS: &[&str] = &[ - "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven", "twelve", - "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen", "twenty", - "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety", "hundred", + "two", + "three", + "four", + "five", + "six", + "seven", + "eight", + "nine", + "ten", + "eleven", + "twelve", + "thirteen", + "fourteen", + "fifteen", + "sixteen", + "seventeen", + "eighteen", + "nineteen", + "twenty", + "thirty", + "forty", + "fifty", + "sixty", + "seventy", + "eighty", + "ninety", + "hundred", ]; /// The prose that may still put a bare quantity against `items`, because it RECORDS a run that @@ -68116,6 +68141,14 @@ mod run_item_cap_tests { "the lane split (#114 — lane items) is computed per subject", "it is ~617 issues against a {{ITEM_CAP}}-item budget", "the 2026-08-03 run budget items", + // THE TWO ABOVE DO NOT REACH THE GUARDS THEY NAME, which a mutation pass is how you + // find out: in the live sentence `~617` sits three tokens off the noun (`against a + // {{ITEM_CAP}}-item`) so the digit-run rule never judges it, and `#114` is excluded by + // that same digit-run rule whether or not the `#` rule exists. Each guard needs a + // quantity ADJACENT to the noun and short enough to reach it, or it is pinned by + // nothing — both of these survived until they were written. + "the uncovered set is 617 items", + "the lane split (#51 — lane items) is computed per subject", ] { assert!( cap_restatements(derived).is_empty(), @@ -68136,7 +68169,11 @@ mod run_item_cap_tests { STATE_LOAD_PAGE_DEFAULT, 5, "a state-load page IS the run's budget, so it moves only with RUN_ITEM_CAP" ); - assert_eq!(*STATE_LOAD_PAGE_RANGE.start(), 1, "a caller may always narrow"); + assert_eq!( + *STATE_LOAD_PAGE_RANGE.start(), + 1, + "a caller may always narrow" + ); assert_eq!( *STATE_LOAD_PAGE_RANGE.end(), 5, @@ -68185,7 +68222,10 @@ mod run_item_cap_tests { /// the spot rather than the next time the cap moves. #[test] fn the_human_inbox_page_is_not_the_run_item_cap() { - assert_eq!(NEXT_READY_MAX_ROWS, 3, "the human's inbox page, not the cap"); + assert_eq!( + NEXT_READY_MAX_ROWS, 3, + "the human's inbox page, not the cap" + ); assert_eq!(NEXT_CC_MAX_ROWS, 3); assert_eq!(NEXT_DESIGN_MAX_ROWS, 3); assert_eq!(NEXT_LEAK_MAX_ROWS, 3); From 69f7848ecab2f9d14a30f7ed84180570adc31bab Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:37:03 +0000 Subject: [PATCH 3/5] style: denofmt the two markdown files this branch edited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rainix-rs-static` runs `pre-commit run --all-files`, and its `denofmt` hook rewrites both: the new `item-cap` row in TRANSITIONS.md carried a trailing column space, and shortening ncc.md's cap sentence to `per-run item cap` left the paragraph re-wrapped at the wrong width. Reproduced with CI's exact command against the same pinned rainix rev, which is the only way to see these — a bare `cargo fmt` does not touch markdown. Co-Authored-By: Claude Opus 5 (1M context) --- TRANSITIONS.md | 2 +- plugins/human-fsm/commands/ncc.md | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/TRANSITIONS.md b/TRANSITIONS.md index c2bbea7d..3e6bf328 100644 --- a/TRANSITIONS.md +++ b/TRANSITIONS.md @@ -46,7 +46,7 @@ transition functions: | `repair-qa-block --block-file ` | the RETROFIT of the same rule on an ALREADY-open PR: appends the §8 block to the body, every other byte identical, validated with `require-qa-block`'s predicate | | `weaken-closes ` | the LINKAGE repair a linkage `needs-work` names: `Closes #issue` → `Refs #issue`, every other byte identical, `## QA` untouched, DIRECTION-LOCKED so it can only ever remove a closing reference | | `mcp [--profile vetter\|producer\|human]` | serve a role's transitions over MCP (stdio) — the FSM as a tool surface, not as prose | -| `item-cap` | prints the per-run WORK ITEM cap (`RUN_ITEM_CAP`) and nothing else. Not a transition — it is where the two runners read the number they substitute into their prompts' `{{ITEM_CAP}}`, so the budget a run is told to spend is the same constant the state-loads' `limit` range is computed from (#288) | +| `item-cap` | prints the per-run WORK ITEM cap (`RUN_ITEM_CAP`) and nothing else. Not a transition — it is where the two runners read the number they substitute into their prompts' `{{ITEM_CAP}}`, so the budget a run is told to spend is the same constant the state-loads' `limit` range is computed from (#288) | | `plugin-version-lockstep [--root ]` | CI gate: every plugin `.claude-plugin/marketplace.json` lists resolves to a manifest of the same name carrying the same version | ## The layer a human types: slash commands as a plugin diff --git a/plugins/human-fsm/commands/ncc.md b/plugins/human-fsm/commands/ncc.md index 11c2b6e1..f6874423 100644 --- a/plugins/human-fsm/commands/ncc.md +++ b/plugins/human-fsm/commands/ncc.md @@ -260,12 +260,12 @@ Each field of the row is one read a human otherwise does by hand: `queue.more` and `counts` frame the row. A flag you expected and did not get is usually in `counts.unvetted` — the vetter has not judged it, and under the per-run item cap it may wait; a flag the vetter would REJECT never arrives here -at all. `strandedFlags` is a label parking an issue with nothing consuming it: no -producer comment behind it, or a reject whose label is live anyway. The vetter's -state-load clears both, so one listed here is a clearance that has not run yet -or could not write — the label is still parking the issue either way. It is not -yours to rule on: leave it and let the next vetter run take it, or look at -`clearanceFailed` on that state-load if it persists. +at all. `strandedFlags` is a label parking an issue with nothing consuming it: +no producer comment behind it, or a reject whose label is live anyway. The +vetter's state-load clears both, so one listed here is a clearance that has not +run yet or could not write — the label is still parking the issue either way. It +is not yours to rule on: leave it and let the next vetter run take it, or look +at `clearanceFailed` on that state-load if it persists. `archivedRepoFlags` is a separate list and a harder state: the flag's REPO is archived, so no ruling can be written on it at all — a label will not move, a From 14f88cd9e7305863ffc00ff2c93f56d2d7c4512e Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:54:39 +0000 Subject: [PATCH 4/5] fix(cap): refuse a zero budget by digit, and check BOTH files in each loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, all of them the same shape — a guard that reads as covering a case while saying nothing about the neighbouring one. `'' | *[!0-9]* | 0` refused the string `0` and let `00` through: all digits, not the string `0`, and it renders "at most 00 WORK ITEMS", which is the zero budget the guard exists to refuse wearing two characters instead of one. Positivity is now decided by FINDING A NONZERO DIGIT rather than by excluding a list of spellings of zero, so every spelling of it is out by construction. The test that was supposed to hold that line asserted on the guard's TEXT, so it could only ever confirm the pattern list was the pattern list. It now lifts the `case` block verbatim out of the shipped script and RUNS it under `sh` against both the values that must abort and the values that must not. `let ... else { return }` in three loops over two files exited the whole test on the first file it could not read instead of skipping that file — so the second file was never checked even when it was there. `continue`. Also pins the direction of `cap_restatements`' window: the count belongs to the noun that FOLLOWS it, and the live prompt says "{{ITEM_CAP}} items dispatched as thirty agents" — the one sentence whose job is to say the cap counts items and not agents, and exactly what a forward window would flag. Co-Authored-By: Claude Opus 5 (1M context) --- campaign-run.sh | 11 +++++- pr-review-report-rs/src/main.rs | 65 +++++++++++++++++++++++++++++---- review-run.sh | 11 +++++- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/campaign-run.sh b/campaign-run.sh index 0ded922d..3c8b6e84 100755 --- a/campaign-run.sh +++ b/campaign-run.sh @@ -394,12 +394,21 @@ trap '[ -n "${SCRATCH_DIR:-}" ] && rm -rf "$SCRATCH_DIR"' EXIT # most WORK ITEMS per run" and hands the model a budget with no number in it — the same silent # degradation the worker-brief guard below exists for, one stale binary on PATH away. So a value # that is not a positive integer ABORTS the run instead of reaching the model. +# +# "Positive" is decided by finding a NONZERO DIGIT, not by excluding the string `0`: `00` is all +# digits and is not `0`, so an exclusion list lets it through and renders "at most 00 WORK ITEMS", +# which is the zero budget this guard exists to refuse wearing two characters instead of one. ITEM_CAP="$(pr-review-report item-cap 2>/dev/null)" case "$ITEM_CAP" in - '' | *[!0-9]* | 0) + '' | *[!0-9]*) echo "$(date -u +%FT%TZ) campaign run ABORT: \`pr-review-report item-cap\` gave no usable run budget (got '$ITEM_CAP') — the prompt's {{ITEM_CAP}} would render empty" | _log exit 1 ;; + *[1-9]*) ;; + *) + echo "$(date -u +%FT%TZ) campaign run ABORT: \`pr-review-report item-cap\` gave a ZERO run budget (got '$ITEM_CAP') — a run told to spend no items must not start" | _log + exit 1 + ;; esac # substitute deployment values into the (path-free) prompt template at runtime diff --git a/pr-review-report-rs/src/main.rs b/pr-review-report-rs/src/main.rs index 38818919..83d2c923 100644 --- a/pr-review-report-rs/src/main.rs +++ b/pr-review-report-rs/src/main.rs @@ -68076,11 +68076,19 @@ mod run_item_cap_tests { NUMBER_WORDS.contains(&t.as_str()) } - /// Every place `text` attaches a bare quantity to the `item`/`items` noun — i.e. every place it + /// Every place `text` puts a bare quantity BEFORE the `item`/`items` noun — i.e. every place it /// states a work-item budget in its own words instead of deriving one from `{{ITEM_CAP}}`. /// /// The split treats `-` as a separator so `5-ITEM BUDGET` is caught: the hyphenated form is a /// restatement that a scan for a free-standing digit walks straight past. + /// + /// LOOKING BACKWARD IS THE RULE, NOT A LIMITATION TO LIFT. English attaches a count to the noun + /// that FOLLOWS it, so a window opened after the noun reads the count of the NEXT noun instead: + /// the live sentence `{{ITEM_CAP}} items dispatched as thirty agents` would be flagged for the + /// `thirty` that belongs to `agents`, and it is the one sentence in the prompt whose whole job + /// is to say the cap counts items and NOT agents. A post-noun form (`items per run: 5`) is a + /// different construction, and catching it means stopping at the next noun rather than widening + /// this window — do not add a forward scan without that. The case below pins it. fn cap_restatements(text: &str) -> Vec { let toks: Vec<&str> = text .split(|c: char| c.is_whitespace() || c == '-') @@ -68149,6 +68157,10 @@ mod run_item_cap_tests { // nothing — both of these survived until they were written. "the uncovered set is 617 items", "the lane split (#51 — lane items) is computed per subject", + // THE DIRECTION, PINNED. A count sitting AFTER the noun belongs to the next noun, not + // to this one — widening the window forward flags `thirty` here, on the one sentence + // that exists to say the cap counts items and not agents. + "{{ITEM_CAP}} items dispatched as thirty agents is still {{ITEM_CAP}} items", ] { assert!( cap_restatements(derived).is_empty(), @@ -68264,7 +68276,7 @@ mod run_item_cap_tests { fn the_prompts_state_no_item_cap_of_their_own() { for name in ["campaign-prompt.txt", "review-prompt.txt"] { let Some(prompt) = repo_root_text(name) else { - return; // not checked out (nix build sandbox) — enforced by the rs-test gate + continue; // not checked out (nix build sandbox) — enforced by the rs-test gate }; assert!( prompt.contains("{{ITEM_CAP}}"), @@ -68295,7 +68307,7 @@ mod run_item_cap_tests { let (Some(prompt), Some(runner)) = (repo_root_text(prompt_name), repo_root_text(runner_name)) else { - return; // not checked out (nix build sandbox) — enforced by the rs-test gate + continue; // not checked out (nix build sandbox) — enforced by the rs-test gate }; let mut placeholders: Vec = prompt @@ -68324,6 +68336,32 @@ mod run_item_cap_tests { } } + /// TEST HELPER: run a runner's OWN budget guard against one value, in a shell, and report + /// whether it aborted. The `case` block is lifted verbatim out of the script, so what is + /// exercised is the guard that ships rather than a copy of it kept here. + /// + /// Asserting on the guard's TEXT is what let `00` through: a pattern list reads as covering the + /// case it names and says nothing whatever about the ones it does not, so `'' | *[!0-9]* | 0` + /// looked like "refuses zero" while `00` — all digits, and not the string `0` — sailed past it + /// into a rendered budget of nothing. + fn budget_guard_rejects(runner: &str, value: &str) -> bool { + let after = runner + .split_once("case \"$ITEM_CAP\" in") + .expect("the budget guard is present") + .1; + let block = after.split_once("esac").expect("the guard closes").0; + // `_log` is the runner's own logger, which the guard pipes its abort line into. + let script = format!( + "_log() {{ cat >/dev/null; }}\nITEM_CAP='{value}'\ncase \"$ITEM_CAP\" in{block}esac\nexit 0\n" + ); + !std::process::Command::new("sh") + .arg("-c") + .arg(script) + .status() + .expect("sh runs") + .success() + } + /// A budget that cannot be resolved ABORTS the run instead of reaching the model. /// /// An empty substitution is not a failure the shell notices: it renders "at most WORK ITEMS @@ -68333,16 +68371,27 @@ mod run_item_cap_tests { fn the_runners_abort_rather_than_render_an_empty_run_budget() { for name in ["campaign-run.sh", "review-run.sh"] { let Some(runner) = repo_root_text(name) else { - return; // not checked out (nix build sandbox) — enforced by the rs-test gate + continue; // not checked out (nix build sandbox) — enforced by the rs-test gate }; assert!( runner.contains("ITEM_CAP=\"$(pr-review-report item-cap 2>/dev/null)\""), "{name} takes the run budget from the transition function, not from a literal" ); - assert!( - runner.contains("'' | *[!0-9]* | 0)"), - "{name} must refuse an empty, non-numeric or zero budget" - ); + // EVERY shape that must not reach the model. `00`/`000` are the ones an exclusion list + // misses — all digits, neither of them the string `0`, and both render a ZERO budget, + // which is the same instruction as an empty one written with a character in it. + for bad in ["", "0", "00", "000", "x", "5x", " 5", "-1", "1.5", "5 5"] { + assert!( + budget_guard_rejects(&runner, bad), + "{name} must refuse {bad:?} rather than render it into the RUN BUDGET sentence" + ); + } + for good in ["1", "5", "10", "25"] { + assert!( + !budget_guard_rejects(&runner, good), + "{name} must accept the positive budget {good:?}" + ); + } let guard = runner .split_once("ITEM_CAP=\"$(") .expect("the read is present") diff --git a/review-run.sh b/review-run.sh index 2975ed54..c626f912 100755 --- a/review-run.sh +++ b/review-run.sh @@ -332,12 +332,21 @@ export INSTALL_DIR="$DIR" # A value that is not a positive integer ABORTS: `{{ITEM_CAP}}` rendering empty leaves the vetter a # RUN BUDGET sentence with no number in it, which nothing rejects and every run resolves its own # way — the same silent-degradation class as the empty auditor brief below. +# +# "Positive" is decided by finding a NONZERO DIGIT, not by excluding the string `0`: `00` is all +# digits and is not `0`, so an exclusion list lets it through and renders "at most 00 WORK ITEMS", +# which is the zero budget this guard exists to refuse wearing two characters instead of one. ITEM_CAP="$(pr-review-report item-cap 2>/dev/null)" case "$ITEM_CAP" in - '' | *[!0-9]* | 0) + '' | *[!0-9]*) echo "$(date -u +%FT%TZ) review run ABORT: \`pr-review-report item-cap\` gave no usable run budget (got '$ITEM_CAP') — the prompt's {{ITEM_CAP}} would render empty" | _log exit 1 ;; + *[1-9]*) ;; + *) + echo "$(date -u +%FT%TZ) review run ABORT: \`pr-review-report item-cap\` gave a ZERO run budget (got '$ITEM_CAP') — a run told to spend no items must not start" | _log + exit 1 + ;; esac # substitute deployment values into the prompt template From e977883b8d6ed595dc17c414518226dfbf4ed600 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Mon, 17 Aug 2026 04:29:54 +0000 Subject: [PATCH 5/5] Pay for the templating: cut the guide passage the producer cannot act on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cap is a RATCHET at main s exact size — main is 153919 of 153919 — so this branch s +289 fails by construction. The 289 is the templating itself: {{ITEM_CAP}} is 11 bytes more than 5, at 23 sites. That is the change, so the bytes come from prose instead. Cut from QA-GUIDE.md (charged with the prompts because campaign-prompt.txt NAMES it, which is the part I had wrong when I called 6213 bytes of headroom): the sentence describing require-qa-block as a PreToolUse hook over gh pr create. The producer cannot take that path — gh pr create is forbidden to it and it opens PRs only through open_pr — so by the gate s own test, what would the run get wrong without this line, the answer is nothing. 344 cut against 289 needed. The plugin cap is the same shape: 121990, and per-run item cap was 2 bytes over it. run item cap says the same thing and fits. denofmt reflowed TRANSITIONS.md, README.md and agents/ncc.md around the shorter lines; byte totals unchanged, both caps still satisfied. prompt 153864/153919, plugin 121988/121990. KNOWN NOT FIXED: rs-test (macos-latest) fails render_component_tests::the_server_answers_a_real_request, which binds a TCP socket on 127.0.0.1. It touches nothing this branch changes and main is green on its last six runs, so it is read as a runner flake and left to the re-run. ## QA - Discriminating tests: n/a — the change is prose bytes and markdown reflow; the gates that judge it are the two prompt-cap CI jobs, measured directly above. - Mutations applied: n/a — no branch to mutate. The measurement is the check: restoring the cut passage puts the total at 154208, which is the failure this commit answers. - Oracle: the cap job s own arithmetic, read from its CI log rather than recomputed — it charges files the prompt NAMES, one hop, which is why QA-GUIDE.md is in the total and why my glob-only count was wrong. - Category check: four CI failures. Covered: prompt cap, plugin prompt cap, rs-static (denofmt). NOT covered and stated above: rs-test (macos-latest). Co-Authored-By: Claude Opus 5 (1M context) --- QA-GUIDE.md | 17 ++++++----------- README.md | 1 + TRANSITIONS.md | 2 +- plugins/human-fsm/agents/ncc.md | 6 +++--- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/QA-GUIDE.md b/QA-GUIDE.md index 53979ed7..2c1d2a9a 100644 --- a/QA-GUIDE.md +++ b/QA-GUIDE.md @@ -103,17 +103,12 @@ All four lines are required. A line your change cannot have takes `n/a` **with the reason** (a docs-only diff has no mutations to apply); an absent line is not an option. -That is enforced where the PR is opened, not only where it is judged, on both -paths a PR of ours can be opened on. The cron producer opens PRs with the -`open_pr` MCP tool, which reads the body file and REFUSES (exit 3) before -anything is created; every other session goes through -`pr-review-report require-qa-block`, wired as a PreToolUse hook, which refuses a -`gh pr create` whose body has no `## QA` section. Both name the lines that are -missing, and both decide with the SAME predicate, so a body one accepts the -other accepts. So "a PR without its QA evidence does not get opened" is literal: -it costs one retry inside the run instead of a round trip through the vetter's -queue. The gate checks that the block is PRESENT; whether its claims hold is -still the vetter's call. +That is enforced where the PR is opened, not only where it is judged. You open +PRs with the `open_pr` MCP tool, which reads the body file and REFUSES (exit 3) +before anything is created, so "a PR without its QA evidence does not get +opened" is literal: it costs one retry inside the run instead of a round trip +through the vetter's queue. The gate checks that the block is PRESENT; whether +its claims hold is still the vetter's call. Both gates are on PR-OPEN, so neither can reach a PR already open without the block. `pr-review-report repair-qa-block --block-file ` diff --git a/README.md b/README.md index da7d4864..22b0d197 100644 --- a/README.md +++ b/README.md @@ -2960,6 +2960,7 @@ a RISK bound the design intends to move, the prompts state it two dozen times, and several of those statements are spelled as English words that a sweep for the digit does not find. A run whose budget cannot be resolved to a positive integer ABORTS rather than rendering a RUN BUDGET sentence with no number in it. + ### Timing the `gh` calls — `PRR_GH_TIMING` Set `PRR_GH_TIMING` to anything but empty or `0` and every `gh` the binary runs diff --git a/TRANSITIONS.md b/TRANSITIONS.md index 5b121691..e4267e9e 100644 --- a/TRANSITIONS.md +++ b/TRANSITIONS.md @@ -46,7 +46,7 @@ transition functions: | `repair-qa-block --block-file ` | the RETROFIT of the same rule on an ALREADY-open PR: appends the §8 block to the body, every other byte identical, validated with `require-qa-block`'s predicate | | `weaken-closes ` | the LINKAGE repair a linkage `needs-work` names: `Closes #issue` → `Refs #issue`, every other byte identical, `## QA` untouched, DIRECTION-LOCKED so it can only ever remove a closing reference | | `mcp [--profile vetter\|producer\|human]` | serve a role's transitions over MCP (stdio) — the FSM as a tool surface, not as prose | -| `item-cap` | prints the per-run WORK ITEM cap (`RUN_ITEM_CAP`) and nothing else. Not a transition — it is where the two runners read the number they substitute into their prompts' `{{ITEM_CAP}}`, so the budget a run is told to spend is the same constant the state-loads' `limit` range is computed from (#288) | +| `item-cap` | prints the per-run WORK ITEM cap (`RUN_ITEM_CAP`) and nothing else. Not a transition — it is where the two runners read the number they substitute into their prompts' `{{ITEM_CAP}}`, so the budget a run is told to spend is the same constant the state-loads' `limit` range is computed from (#288) | | `plugin-version-lockstep [--root ]` | CI gate: every plugin `.claude-plugin/marketplace.json` lists resolves to a manifest of the same name carrying the same version | ## The layer a human types: slash commands as a plugin diff --git a/plugins/human-fsm/agents/ncc.md b/plugins/human-fsm/agents/ncc.md index 9b071348..431bb503 100644 --- a/plugins/human-fsm/agents/ncc.md +++ b/plugins/human-fsm/agents/ncc.md @@ -309,9 +309,9 @@ Each field of the row is one read a human otherwise does by hand: that the flag is live rather than moot. `queue.more` and `counts` frame the row. A flag you expected and did not get is -usually in `counts.unvetted` — the vetter has not judged it, and under the -per-run item cap it may wait; a flag the vetter would REJECT never arrives here at -all. `strandedFlags` is a label parking an issue with nothing consuming it: no +usually in `counts.unvetted` — the vetter has not judged it, and under the run +item cap it may wait; a flag the vetter would REJECT never arrives here at all. +`strandedFlags` is a label parking an issue with nothing consuming it: no producer comment behind it, or a reject whose label is live anyway. The vetter's state-load clears both, so one listed here is a clearance that has not run yet or could not write — the label is still parking the issue either way. It is not