ci: settle disputed review findings with a reaction

[rafaellehmkuhl]

What this changes

A disputed finding used to be put to the maintainers as a checkbox block in the review body. That never worked: a tick records no author, so no run could tell who made the call, which is why the block was reprinted every round for the life of the PR and only a /resolve ever closed anything. The decisions block on #2865 is a fair example — four disputes, carried round after round, none of them actionable.

Each disputed finding now gets a comment of its own, seeded with a 👍 and a 👎. A reaction names who left it, so one click is evidence the next round can apply. The block leaves the review body entirely; :speech_balloon: in the summary table stays the signal that a dispute is waiting.

/resolve is untouched and still works.

How a vote is counted

Only reactions from someone with admin or write on the repository — the same bar /resolve clears, checked through the same collaborators/{user}/permission lookup. The bot's own seeded reactions are excluded by author rather than by permission, since the token that posts them is the repository's own. Votes are deduplicated by login, so a maintainer who reacts both ways has said nothing.

Reactions that count	Verdict	Effect on the finding
more 👍 than 👎	accept	closes as resolved, exactly as /resolve does
more 👎 than 👍	reject	argument refused, finding carries on as plain open
equal	tie	decides nothing, stays as the review judged it
none	pending	decides nothing, stays as the review judged it

A reject settles the argument rather than the finding, so author_argument is dropped and the same question does not go back to the same people — but it is not a lock, and a later code change closes the finding on the usual terms.

Notes for the reviewer

• Reactions cannot trigger a workflow, so a vote is applied by the next /review. The decision comment says so.
• Votes are read as live state every round rather than banked as a delta, so a maintainer who changes their mind just moves their reaction. That holds up to the /review that applies the verdict and no further — an accepted finding is closed from then on — which is why the decision comment says so where it is read.
• A vote is keyed on the argument it answers, not on the finding id alone. A finding whose author_argument changes gets a fresh comment and a fresh tally, so a refused argument is not re-applied to the next one; the guidelines say to carry an unchanged argument forward verbatim, since rewording it for style would put the same dispute up twice.
• The re-review's "nothing was pushed, stop early" exit had to learn about this for the same reason it once had to learn about /resolve: a vote arrives precisely when nothing has been pushed.
• post is not wired into the initial-review workflow. Round 1 cannot produce a dispute — both the guidelines and that workflow's prompt require every finding to enter the ledger as open — so it would be a guaranteed no-op plus an API call. A round-1 dispute, if one ever became possible, gets its comment on the first /review.
• Conflicts with ci: collapse the whole review comment behind one summary line #2957, which rewrites some of the same guidelines paragraphs. Whichever merges second wants a rebase.

Test plan

• review-decisions.sh --self-check — nine cases over the tally rule, including a passer-by's click, an unreadable permission, and the same maintainer reacting both ways. Wired into ci.yml beside the other review-script checks.
• post dry-run against a stubbed gh: a finding that already has a comment is skipped, a new one is posted and both reactions seeded, and a malformed finding id is refused before it reaches the marker.
• read dry-run against a stubbed gh: the bot's seeded reactions, an unrelated eyes, a reactor with none, and a failed permission lookup are all discarded, the last with a warning rather than in silence.
• Comment-filter checks: a forged decision comment from a non-bot account and a prose mention of the marker are both ignored.
• Empty-input cases: no decision comments, nothing disputed, and an empty ledger.
• End to end on a live PR with a real dispute — needs this merged, since the workflow runs from the default branch.

[github-actions]

Automated PR Review — round 1

Warning

⚠️ IMPORTANT FIXES REQUIRED — 6 open findings: 2 major (1.1, 1.2) and 4 minor.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side — which nothing could ever act on, because a tick does not record who left it. This PR drops that list and instead posts one comment per disputed finding, pre-seeded with a thumbs-up and a thumbs-down. The next time someone asks for a re-review, a script counts the reactions left by people with write access to the repository and either closes the finding or records that the author's argument was refused. It also adds a self-test of the counting rule to CI and rewrites the reviewer's own instructions to match.

What still needs attention

#	Problem	What it means	Severity	Status
1.1	Vote counting may have no permission to check who voted	Maintainers click to settle a dispute and nothing happens, with no visible error anywhere.	major	❌
1.2	An old thumbs-down is re-applied to every later argument	Once an argument is voted down, the author can never put a different one about that finding to the maintainers again, and nobody is told.	major	❌
1.3	A vote cannot be taken back once it has been acted on	A maintainer told they can change their mind by moving their reaction finds it no longer does anything.	minor	❌
1.4	A thumbs-down, or a tie, is never acknowledged	Someone who votes against an argument gets no sign the system saw their click.	minor	❌
5.1	Whole comment thread is downloaded even with nothing to decide	Every re-review spends an extra round trip to GitHub that it almost never needs.	minor	❌
6.1	The voting comment misreads its own state	The comment shows one vote each way before anyone has voted and keeps asking for a decision after one was made.	minor	❌

Change map — what was established before judging

Claims (from the PR body and the commit message; each checked against the base checkout)

• "A tick records no author, so no run could tell who made the call." — verified. The checkbox block is defined only as prose in .github/claude-review/review-guidelines.md:185-201; nothing in claude-pr-review-continue.yml reads a checkbox, and resolutions.json (:249-251) is the only input that closes a finding without a code change.
• "The block was reprinted every round for the life of the PR." — verified. review-guidelines.md:126 requires every open finding to be reprinted, and step 3a of the re-review prompt (:350) keeps a disputed finding in the open set until /resolve.
• "Only reactions from someone with admin or write count, through the same collaborators/{user}/permission lookup /resolve clears." — partly verified. Same endpoint as claude-pr-review-continue.yml:80, same accepted values (review-decisions.sh:27), but not the same job or the same token — see finding 1.1.
• "The bot's own seeded reactions are excluded by author rather than by permission." — verified at review-decisions.sh:116-117 (.user.type != "Bot").
• "Votes are deduplicated by login." — verified: TALLY (:26-35) runs unique over logins on each side.
• "post is not wired into the initial-review workflow … round 1 cannot produce a dispute." — verified. claude-pr-review.yml:3-9 fires only on opened/ready_for_review, and its prompt requires every finding to enter the ledger as open, so no round-1 ledger can carry disputed.
• "Votes are read as live state … so a maintainer who changes their mind just moves their reaction." — contradicted once a vote has been applied; see finding 1.3.
• "/resolve is untouched and still works." — verified: review-command.sh, the authorize job and the resolutions plumbing are unchanged by the diff.

Failure site — the mechanism being replaced is the prose block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are in the diff, and the grep for leftovers (Decisions, raising_hand, checkbox, tick) finds none surviving outside the removed hunks.

Entry points (line numbers are the new file's own, .github/scripts/review-decisions.sh)

Function	Reached from	Frequency
open_votes (:42)	post (:62) and read_votes (:103)	per user action — twice per /review
permission_of (:53)	read_votes (:124), once per distinct reactor	per user action
post (:61)	publish step of claude-pr-review-continue.yml, after gh pr comment	per user action — each published re-review
read_votes (:101)	"Gather PR state and previous review" step, claude-pr-review-continue.yml (hunk @@ -252,6)	per user action — each /review
self_check (:139)	ci.yml:128 (new step)	one-shot per CI run
TALLY (:26)	read_votes (:134) and self_check (:141)	as above

Invariants

• Only an admin/write reactor's click counts. Enforced at one chokepoint — TALLY :27 — fed by permission_of; every producer (any GitHub user can react) passes through it, so the enumeration is closed by construction. Whether the lookup can answer at all in that job is finding 1.1.
• A decision comment is one this workflow posted. open_votes :43-49 requires github-actions[bot] and a body whose start is the marker. I checked jq's capture semantics directly: ^ anchors at string start, not line start, and a non-match yields no object at all, so the failure that once cost this pipeline a ledger (review-ledger.sh:9-12, marker quoted in prose) does not recur here.
• The bot's own seeds never count. Single consumer, :116-117.
• One decision comment per finding id, for the life of the PR. The have set, :62-68. This is the invariant finding 1.2 breaks.
• decisions.json carries no free text, which is what the new prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, and up/down are GitHub logins.

1. Correctness & Implementation Bugs — 4 findings

1.1 — The permission lookup runs in a job this repository says cannot make that call — major

permission_of (review-decisions.sh:53-59) calls repos/$REPO/collaborators/<login>/permission. It is reached from read_votes (:124), which the workflow runs in the "Gather PR state and previous review" step of the rereview job. That job's permissions block is contents: read (claude-pr-review-continue.yml:155-161).

The base workflow puts the identical lookup in a separate job and says why at :21-24: "the endpoint needs a token with push access, and the job that runs the model must not be given one". authorize declares contents: write (:38-44) for exactly that reason.

Both statements cannot be right. If the base comment is, then every lookup fails, permission_of substitutes unknown (:55), TALLY discards the reaction (:27), and every vote on every PR resolves to pending — the feature counts nothing, ever. The only trace is one ::warning:: per reactor (:56-57) in a log nobody opens, and the review then reports the vote as still open, which is exactly what it would report if nobody had clicked. The read dry run in the test plan ran against a stubbed gh, so it cannot distinguish the two either.

Settle it against the live API and then either correct the comment at :21-24 or give this lookup a token that can answer — keeping in mind why the base isolated it, namely that the model step deliberately has no GITHUB_TOKEN in its environment (:298-301). Independently of which way it goes, make the failure loud: when at least one +1/-1 reaction was found and no permission lookup succeeded at all, exit non-zero instead of reporting pending.

1.2 — A rejected vote is re-applied to every later argument on the same finding — major

post skips any finding that already has a decision comment (review-decisions.sh:62-68), and read re-tallies the live reactions on that comment every round (:101-136), so a verdict is permanent state keyed on the finding id alone.

The reachable sequence: round N disputes 1.1 and gets a comment; the maintainers vote it down; round N+1 applies reject — drop author_argument, carry as plain open, per the new prompt bullet. The author then offers a different argument. Step 3d of the prompt (unchanged in substance by the diff) still says an author's explanation makes a finding disputed, so round N+2 marks it disputed again with the new author_argument — and then post finds the id in have and posts nothing, while read returns the old reject from reactions left about the previous argument. The prompt applies it to any finding that is still open, so the new argument is dropped before a maintainer ever sees it. :speech_balloon: never appears, no comment is posted, and nobody is asked.

It fails in the safe direction — the finding stays open — but the author is silenced with no record, which is the same complaint the PR opens with.

Two workable shapes: key the vote on the argument rather than the id (a short digest of author_argument in the marker, so a new argument gets a fresh comment and a fresh tally), or, if one vote per finding is the intent, say so in the prompt and require the review to report a re-dispute it is refusing on a stale verdict, so at least the refusal is visible.

1.3 — An applied accept cannot be reversed, contrary to the stated live-state behaviour — minor

The PR body's notes say votes are live state "so a maintainer who changes their mind just moves their reaction". That holds only until a review applies the vote. Once accept lands, the ledger entry is resolved, and resolved is a closed status the open set excludes (review-guidelines.md:91-96, and the new prompt bullet's "only to a finding that is still open"). Moving the reaction to -1 afterwards flips decisions.json to reject, and nothing acts on it — the finding stays closed. Either state the one-way step where the promise is made (the decision comment body, review-decisions.sh:85-88, is where a maintainer will actually read it), or have the prompt reopen a resolved-by-vote finding whose verdict is no longer accept.

1.4 — A refused or tied vote is never acknowledged to the voter — minor

The new prompt bullet gives reporting rules for accept (name who accepted, in the since-last-round block) and for tie/pending (say the vote is still open, naming the split), but none for reject: applying it drops author_argument silently, and the finding looks exactly as it did before the vote. Worse for tie, the amended short-circuit fires when HEAD_SHA equals PREV_SHA and every accept and reject is reflected — a tie is not in that condition, so a /review typed straight after a split vote takes the early exit and reports nothing, which is the one case the prompt explicitly asked to be reported. Net effect: a thumbs-up that carries is visible, a thumbs-down and a split are not, and the maintainer cannot tell a counted vote from an ignored one. Add a reject reporting rule, and either fold an unreported tie into the exit condition or state that a tie is deliberately not worth a run.

5. Performance — 1 finding

5.1 — post walks the whole comment thread even when nothing is disputed — minor

post calls open_votes (review-decisions.sh:62) — a --paginate walk of every comment on the PR — before it looks at the ledger at all. Every published re-review pays that round trip, and the overwhelmingly common case is a ledger with no disputed entry, where the answer is thrown away. Reading the disputed ids first is local and free; return 0 when there are none, and fetch only when there is something to subtract. Note also that the same job already holds the full comment list on disk as bot-comments.json (claude-pr-review-continue.yml:203-204), gathered once precisely so every view derives from a single fetch (:188-196) — read could take it as an optional input instead of re-fetching.

6. UI / UX — 1 finding (2 sub-items)

6.1 — The decision comment misreports its own state — minor

The surface here is the comment post writes (review-decisions.sh:76-89), which is what a maintainer reads while deciding a dispute. Two breaches on one surface:

• The seeded reactions read as votes. The +1 and -1 seeded at :95-96 are counted in the reaction chips GitHub renders, so a fresh comment shows one vote each way, and the only line explaining the rule — "Only reactions from someone with write access to this repository are counted" (:87-88) — is, read literally, false about them: the token that posted them does have write access. A maintainer reading the chips as a live 1–1 tally concludes the vote is deadlocked when nobody has voted. One clause fixes it: say the two reactions already present were left by the bot and do not count.
• The comment never learns it was decided. Nothing revisits it, so after the vote is applied it still reads "React to this comment and the next /review applies the answer" (:85) on a finding that may already be closed — a standing block asking for a decision nobody needs to make, which is the shape this PR set out to remove. Have the round that applies a verdict edit the comment to record the outcome (or at minimum link it from the since-last-round entry).

Sections with nothing to report (8)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script mirrors the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check subcommand, case dispatch — and every added path has a call site in this PR; the one edit that looks unrelated, review-ledger.sh review.md > ledger.json, is what feeds post)

4. Security — ✅ (the diff rewrites this reviewer's own governing document — reviewed here, not adopted: this run was conducted under the base version of the guidelines and took no procedure from the diff. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; every API-sourced value reaches jq through --arg/--argjson/--input - and never a shell; the bot-author filter plus the string-anchored marker were both checked against jq's real capture behaviour, and the vote gate accepts the same admin/write bar /resolve already runs on)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0 — the diff contains no JS/TS — so no complexity or depth trigger exists this round and none was counted by eye; the decision rule lives in one TALLY expression that both the workflow path and --self-check execute, which is the pattern review-command.sh established)

8. Commit Hygiene — ✅ (one commit, ci:-prefixed in line with git log on this checkout; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; 256 additions is inside the reviewable range; no #N or closing keyword in the message, with the #2865/#2957 references kept in the PR body where they belong)

9. Tests — ✅ (nothing existing was removed or weakened; the new --self-check is wired in at ci.yml:128 beside the four sibling checks and drives the same TALLY string the workflow executes)

10. Documentation — ✅ (the script header documents both subcommands, the environment it reads and the token scope it needs; the guidelines and the re-review prompt were both updated in the same diff, and a grep for Decisions, raising_hand, checkbox and tick finds no orphaned reference to the removed block)

11. Nitpicks / Optional — ✅ (nothing beyond the findings above; the --self-check cases, the temp-file handling and the RETURN traps were read and are sound)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 1

Done

• .github/scripts/review-decisions.sh, .github/workflows/claude-pr-review-continue.yml (1.1 — permission lookup runs in a job without push access): settled against the docs. GET /repos/{owner}/{repo}/collaborators/{username}/permission is listed under Metadata: read for an installation token, which every job here holds, so the base comment claiming the endpoint needs push access was the wrong half — corrected in place. authorize keeps its wider scope, since the older docs did claim push and that gate failing closed would take /resolve with it. Independently, read no longer reports pending when the lookup never answers: if reactions were found and not one permission could be read, it exits non-zero with an ::error:: instead of counting nothing in silence.
• .github/scripts/review-decisions.sh, .github/claude-review/review-guidelines.md (1.2 — a rejected vote is re-applied to every later argument): the vote is keyed on the argument now, not the finding id. The marker carries a checksum of author_argument, post skips only an argument already put to a vote, and read tallies the newest comment per finding — the reactions on a superseded one answered a question nobody is being asked. The guidelines say to carry an unchanged argument forward verbatim, so rewording it for style cannot put the same dispute up twice.
• .github/scripts/review-decisions.sh (1.3 — an applied accept cannot be reversed): stated where a maintainer reads it. The decision comment now says to move your reaction while the vote is open, and that once a /review has closed the finding on it, moving it no longer reopens it.
• .github/workflows/claude-pr-review-continue.yml, .github/claude-review/review-guidelines.md (1.4 — a refused or tied vote is never acknowledged): a reject gets a reporting rule, naming the down logins in the since-last-round block. The tie half went the other way and is now explicit: a vote that decides nothing can never become "already reflected", so it never buys a run of its own, and the split is reported by the next review that runs for a reason of its own.
• .github/scripts/review-decisions.sh (5.1 — post walks the whole thread with nothing disputed): the ledger is read first and post returns before the fetch when nothing is disputed.
• .github/scripts/review-decisions.sh (6.1, first item — the seeded reactions read as votes): the comment now says the two reactions already on it were left by the bot so either answer is one click, and that neither counts.
• .github/claude-review/review-guidelines.md (6.1, second item — the comment never learns it was decided): took the minimum you named. A finding closed by a vote is reported naming who accepted it and linking the decision comment by its url, so the standing block is one click from the answer it got.

Done differently

• .github/scripts/review-decisions.sh (5.1, second part — have read take bot-comments.json instead of re-fetching): declined the reuse, kept the saving. bot-comments.json is a snapshot taken at the start of a run that can last 35 minutes, and post runs at the end of it, so the helper would carry two freshness contracts to save one paginated GET per /review. The waste the finding actually named — a walk on every publish with nothing disputed — is gone.

Questions for reviewers

• .github/workflows/claude-pr-review-continue.yml (1.1): the docs settle this on paper, not in production — no run has yet made the call from a contents: read job. The loud failure means the first real vote settles it and says so out loud, rather than counting nothing forever. If you would rather not find out that way, the alternative is moving read into authorize and passing decisions.json through a job output; say the word and it lands in this PR.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 2

Note

📝 MINOR SUGGESTIONS — 4 open (3 minor, 1 nit) and 4 closed since round 1.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side — which nothing could ever act on, because a tick does not record who left it. This PR drops that list and posts one comment per disagreement instead, pre-seeded with a thumbs-up and a thumbs-down. The next time someone asks for a re-review, a script counts the reactions left by people with write access to the repository and either closes the finding or records that the argument was refused. Since round 1 the vote is tied to the specific argument it answers rather than to the finding, a counting run that cannot check who voted now fails loudly instead of quietly counting nothing, and the comment tells the reader which of the reactions on it are real.

What still needs attention

#	Problem	What it means	Severity	Status
1.5	The comment posted and the comment counted can be different ones	If the author goes back to an argument that was already voted on once, the answer applied is the one given about a different argument, and nobody is asked again.	minor	❌
5.1	The whole comment thread is still downloaded on every re-review	Every re-review spends a round trip to GitHub fetching a list it already has on disk a moment earlier.	minor	:large_yellow_circle:
6.1	A refused argument leaves its voting comment standing	The comment keeps asking for a decision that was already made, and a later click on it can quietly close the finding anyway.	minor	:large_yellow_circle:
10.1	The script's own usage note describes the old behaviour	Anyone reading the file's header is told a rule the code no longer follows.	nit	❌

Since round 1 — 4 closed, 2 partially addressed, 2 new, comparing 91ec6b5 → fd7c39a

The increment is not usable as a delta this round. The author amended the single commit rather than adding one, so 91ec6b5 is no longer an ancestor of fd7c39a and the compare falls back to the merge base: incremental.diff reproduces the entire PR (review-decisions.sh appears as added, +244/-0). Every status below was therefore judged against pr.diff and the current file contents, not against the increment.

resolutions.json is [] — no /resolve has been issued on this PR, so nothing was closed by a maintainer command this round and there is no unmatched id to report back.

✅ 1.1 — Vote permission lookup runs in a job without push access (major, addressed)
The finding asked for two things. First, settle the contradiction and either correct the workflow comment or give the lookup a token that can answer: the header comment at claude-pr-review-continue.yml:19-27 no longer claims the endpoint needs push access, and now states it asks an installation token for metadata: read, with a stated reason for leaving authorize's wider scope alone. I cannot verify that claim from here — this job has no network — and the author says as much themselves. Second, and independently of which way it went, make the failure loud: read_votes now exits non-zero with an ::error:: when reactions were found and not one permission lookup answered (review-decisions.sh:149-152), and the workflow runs it without || :, so the step fails rather than reporting pending. That is what makes the unverified half safe: the first real vote settles it out loud instead of counting nothing forever.

✅ 1.2 — A rejected vote is re-applied to every later argument on the same finding (major, addressed)
The finding named the fix and the fix is the one that landed: the marker now carries a checksum of the argument (review-decisions.sh:88, written at :92), post skips only a finding-plus-argument pair it has already put to a vote (:78, :89), and read tallies the newest comment per finding (NEWEST, :40, used at :118), so reactions left about a superseded argument no longer decide the current one. The guidelines add the matching rule that an unchanged argument is carried forward verbatim (review-guidelines.md, hunk @@ -182,23). Two narrower holes the new keying opens are raised fresh as 1.5.

✅ 1.3 — An applied accept cannot be reversed, contrary to the stated live-state behaviour (minor, addressed)
The finding offered two routes and asked for either. The first landed, in the place the finding named: the decision comment body now says to move your reaction while the vote is open, "once a /review has closed the finding on it, moving it no longer reopens it" (review-decisions.sh:102-103) — which is where a maintainer actually reads it, rather than in the PR body where the promise was made.

✅ 1.4 — A refused or tied vote is never acknowledged to the voter (minor, addressed)
Both halves landed. reject gets a reporting rule — the since-last-round block names the down logins (prompt bullet, claude-pr-review-continue.yml, hunk @@ -341,17). For the tie the author took the second of the two options offered: the short-circuit paragraph now states outright that a tie or a pending can never become "already reflected" and deliberately never buys a run of its own, with the split reported by the next review that runs for a reason of its own (review-guidelines.md, hunk @@ -388,7).

:large_yellow_circle: 5.1 — post walks the whole comment thread even when nothing is disputed (minor, partially addressed)
The titled waste is gone: post reads the ledger first and returns before the fetch when nothing is disputed (review-decisions.sh:70-76). The second item the finding named — having read take the already-fetched comment list instead of re-fetching — was declined. The author's stated reason is that the file "is a snapshot taken at the start of a run that can last 35 minutes, and post runs at the end of it". That is true of post, and it is why post should keep fetching; it is not true of read, which the same PR wires into the gather step a few lines below the fetch. Details in section 5.

:large_yellow_circle: 6.1 — Decision comment misreports its own state (minor, partially addressed)
Sub-item one is done: the comment now says the two reactions already on it were left by the bot and that neither counts (review-decisions.sh:99-101). Sub-item two is done for one verdict of two — an accept is reported with a link to the comment it closed on, a reject names the voters but leaves the comment standing, unedited and still live. Details in section 6.

New this round: 1.5 (minor) and 10.1 (nit), both consequences of the round-1 fixes rather than of anything that was there before.

Discussion since round 1. One substantive comment, from @rafaellehmkuhl (#2960 (comment)), item-by-item against the round-1 findings; the bare /review that follows it is the command and is ignored. Its claims were checked against the diff rather than taken as evidence, and are recorded as verified or contradicted in the Change map. One thing in it is addressed to the maintainers rather than to me and is repeated here so it is not lost in a collapsed block: on 1.1 the author notes that the docs settle the permission question on paper only, and offers to move read into the authorize job and pass decisions.json through a job output instead, if the maintainers would rather not learn the answer from the first real vote.

Change map — what was established before judging

Claims (from the commit message and the author's follow-up comment; each checked against the code, never adopted)

• "A vote is keyed on the argument it answers rather than on the finding alone." — verified: review-decisions.sh:88 checksums author_argument, :92 writes it into the marker, :47-51 parses it back, :78/:89 dedupe on the pair. Incomplete in the two cases in finding 1.5.
• "read tallies the newest comment per finding." — verified: NEWEST at :40, applied at :118, with two self-check cases at :211-218.
• "Only a reactor with push access counts, which is the authority /resolve already runs on." — verified: TALLY:27 accepts admin/write only, the same values review-command.sh:27 accepts, fed by permission_of:56.
• "The endpoint asks an installation token for metadata: read, which every job here has." — not verifiable here: this job has no network, and the author states plainly that no run has yet made the call from a contents: read job. The loud-failure path at :149-152 is what makes either answer survivable, which is why 1.1 closes.
• "read no longer reports pending when the lookup never answers." — verified: :149-152 returns 1, and the workflow calls it with no || : (hunk @@ -252,6), so the gather step fails.
• "post returns before the fetch when nothing is disputed." — verified: :70-76, ahead of the open_votes call at :78.
• "bot-comments.json is a snapshot taken at the start of a run that can last 35 minutes, and post runs at the end of it." — verified for post (publish step, hunk @@ -438,6), contradicted for read: the fetch and the author filter are at claude-pr-review-continue.yml:196-204 and the read call is inserted at hunk @@ -252,6 of the same step, seconds later. See 5.1.
• "A tie never buys a run of its own, deliberately." — verified in the rewritten short-circuit bullet (review-guidelines.md, hunk @@ -388,7).
• "/resolve is untouched." — verified: review-command.sh, the authorize gate and the resolutions plumbing are unchanged; the only edit near them is the header comment rewrite.

Failure site — the mechanism being replaced is the checkbox block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are removed by the diff, and a grep for raising_hand, Decisions for a human, checkbox and Ticking a box across .github/ finds no surviving reference outside the removed hunks (the :raising_hand: at review-decisions.sh:93 is the new comment's own heading).

Entry points (line numbers are the new file's own, .github/scripts/review-decisions.sh)

Function	Reached from	Frequency
open_votes (:46)	post (:78) and read_votes (:118)	per user action — up to twice per /review
permission_of (:56)	read_votes (:139), once per distinct reactor	per user action
post (:64)	publish step of claude-pr-review-continue.yml (hunk @@ -438,6), after gh pr comment	per user action — each published re-review
read_votes (:116)	"Gather PR state and previous review" step, claude-pr-review-continue.yml (hunk @@ -252,6)	per user action — every /review, dispute or not
self_check (:164)	ci.yml:128 (new step)	one-shot per CI run
TALLY (:26) / NEWEST (:40)	read_votes (:118, :159) and self_check	as above

Invariants

• Only an admin/write reactor's click counts. One chokepoint, TALLY:27, fed by permission_of; every producer (any GitHub user can react) passes through it, so the enumeration is closed by construction.
• A decision comment is one this workflow posted. open_votes:48-49 requires github-actions[bot] and a body whose first line is the marker; the publish and gather steps both run on secrets.GITHUB_TOKEN (claude-pr-review-continue.yml:175, :401), so the login the filter expects is the one the comments carry.
• The bot's own seeds never count. Single consumer, :131-133.
• One decision comment per (finding, argument), for the life of the PR. The have set, :78/:89. This is the invariant finding 1.5 breaks, because the tally uses a different key.
• decisions.json carries no free text, which is what the new prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, and up/down are GitHub logins.

1. Correctness & Implementation Bugs — 1 finding (2 sub-items)

1.5 — The key a vote is posted under and the key it is counted under are not the same key — minor

post decides whether to ask by looking for the pair finding arg anywhere in the PR's history of decision comments (review-decisions.sh:78, :89), while read_votes decides what the answer is by taking the newest comment for that finding and ignoring the argument entirely (NEWEST, :40, used at :118). decisions.json then carries {id, url, verdict, up, down} with no arg at all (:154-161), so nothing downstream can tell whether the verdict it is applying answers the argument the ledger is currently showing. Two reachable consequences:

• An argument the author returns to is refused on someone else's verdict. Argument A gets comment Initial project structure #1; A is rejected; the author makes argument B and gets comment README uses npm and not yarn #2; the author later goes back to A. post finds 1.1 <cksumA> in have and posts nothing, read returns comment README uses npm and not yarn #2's verdict, and the prompt applies it to a finding whose live argument is A — dropping author_argument with no record. That is finding 1.2's harm exactly, in a much narrower window, which is why this is minor and not major.
• A live tally is discarded when the wording moves. The key is a cksum of a sentence a language model regenerates every round. The guidelines now require it carried forward verbatim (review-guidelines.md, hunk @@ -182,23), and copying a JSON field is fairly reliable, but nothing mechanical enforces it: a paraphrase on a finding whose vote is still tie or pending posts a second comment for the same dispute and strands the reactions already on the first, since only the newest counts. The failure the PR set out to remove — the same question re-asked every round — comes back as a new comment per round instead of a reprinted block, and this time it also throws away votes.

One change closes both: make the argument key data rather than prose. Have post write the arg it used back into the finding's ledger entry (or have the review carry it), emit it in decisions.json, and then compare it at both ends — read ignores a verdict whose arg is not the ledger's current one, and post compares against the newest comment for the finding rather than the whole history. If that is more than the case is worth, the alternative is to say so: a ponytail: comment at :86-88 naming the ceiling (vote identity depends on the argument sentence being reproduced byte for byte) and the upgrade path is what AGENTS.md asks for a deliberate corner-cut.

5. Performance — 1 finding (carried from round 1, partially addressed)

5.1 — The comment thread is still walked on every /review, from read rather than from post — minor

The half of this that named post is fixed: the disputed ids are read off the ledger first and post returns before the fetch when there are none (review-decisions.sh:70-76).

What remains is the same paginated walk on the other side. read_votes calls open_votes unconditionally (:118), and the workflow calls read on every /review (hunk @@ -252,6), so a PR that has never had a dispute — the overwhelmingly common case — pays a full --paginate walk of its issue comments to produce []. The same step already has that list: all-comments.ndjson at claude-pr-review-continue.yml:196, filtered to bot-comments.json at :203-204 with the identical author predicate open_votes applies, roughly fifty lines above the insertion point.

The author declined this, on the grounds that the snapshot can be 35 minutes stale by the time post runs. That is right about post, which is why post should keep its own fetch — but read runs inside the very step that takes the snapshot, so there is no freshness to trade there. Give open_votes an optional file argument, have read pass bot-comments.json and post keep fetching. The per-comment reactions fetches are unavoidable and are not what this asks about.

6. UI / UX — 1 finding (carried from round 1, partially addressed)

6.1 — The decision comment still does not learn that it was decided — minor

The surface is the comment post writes (review-decisions.sh:90-104), which is what a maintainer reads while deciding a dispute.

The first sub-item is closed: :99-101 now says the two reactions already present were left by the bot so either answer is one click, and that neither of them counts.

The second is closed for accept only. The guidelines now require a finding closed by a vote to be reported with a link to the comment it closed on (review-guidelines.md, hunk @@ -376,7), which is the minimum the finding named. A reject gets no such treatment: the prompt has the review name the down logins and nothing else (hunk @@ -341,17), and nothing ever revisits the comment. So after a refusal it still reads "React to this comment and the next /review applies the answer" (:95) about an argument the ledger has already dropped — and because read re-tallies it live every round, one later +1 flips the same comment to accept and the prompt closes the finding as resolved, with a resolution saying the author's argument was accepted by vote, when that argument no longer appears anywhere in the ledger or the review. A refused dispute is therefore one click away from being closed on an argument nobody can read.

Either edit the comment when a verdict is applied — record the outcome in the body, which also ends the re-asking — or, at minimum, link the comment from the reject report as well and have the prompt stop treating a comment whose verdict has already been applied as a live vote.

10. Documentation — 1 finding

10.1 — The script header still documents one decision comment per PR — nit

review-decisions.sh:13 reads post <ledger-file> # one comment per disputed finding, once per PR. Since the argument keying landed that is no longer true — it is one comment per disputed finding per argument, which is the whole point of :88-89. The workflow comment at hunk @@ -438,6 ("A comment of its own per disputed finding") reads the same way. One clause on each; the header is the first thing anyone touching this file reads, and it currently contradicts the behaviour the same commit introduced.

Sections with nothing to report (7)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions, so the inventory form does not apply)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script keeps the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check, case dispatch — and every added path has a call site in this PR: post from the publish step, read from the gather step, --self-check from ci.yml:128, and the review-ledger.sh redirect change exists to feed post)

4. Security — ✅ (the diff rewrites this reviewer's own governing document and its prompt — reviewed here, not adopted: this run was conducted under the base version of the guidelines and the base system prompt, and took no procedure, input file or output rule from the diff. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; the comment body is built in a temp file and posted via jq -Rs/--input -, every API-sourced value reaches jq through --arg/--argjson, permission_of interpolates only a GitHub login, and the ledger-sourced title and argument cannot forge a marker because capture is anchored at string start and only line 1 is read)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0, truncated: false — the diff contains no JS/TS, so no complexity or depth trigger exists this round and none was counted by eye; the decision rule is still one TALLY expression that both the workflow path and --self-check execute, and NEWEST joined it under the same pattern)

8. Commit Hygiene — ✅ (still one ci:-prefixed commit, amended rather than followed by a fixup, which is what AGENTS.md asks for; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; 303 additions is inside the reviewable range; no #N or closing keyword in the message)

9. Tests — ✅ (nothing existing was removed or weakened; the two new --self-check cases at :211-218 drive the same NEWEST string read_votes executes, beside the nine that drive TALLY, and the step sits with the four sibling checks in ci.yml)

11. Nitpicks / Optional — ✅ (nothing beyond the findings above; the cksum collision window, the RETURN traps, the early return 0 path and the reaction-seeding failure mode were read and are not worth a line)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 2

Done

• .github/scripts/review-decisions.sh, .github/workflows/claude-pr-review-continue.yml (1.5 + 6.1 — the key posted under is not the key counted on, and a refused argument's comment stays live): one change, at the read end, closes both. A decision comment now counts only while its arg is the checksum of the author_argument the ledger is currently showing (LIVE, :44-47, applied at :157), so read takes the ledger as a second argument. An argument that was accepted or refused has left the ledger, so its comment drops out of the tally and a later click on it cannot close a finding on a case nobody can read; a comment about a superseded argument cannot answer for the current one either. On the posting side have is built from the newest comment per finding rather than the whole history (:95), the same key the tally reads on, so an author who returns to an earlier argument is asked again instead of being answered by a verdict about something else. arg is not emitted in decisions.json: with the gate, every entry in it is by construction about the ledger's live argument, so there is nothing downstream left to compare.
• .github/scripts/review-decisions.sh (1.5, second part — a paraphrase strands the votes): took the alternative the finding offered. That ceiling is the prose key itself, so it is a ponytail: comment at :105-108 naming it and the upgrade path (a stable id emitted with the argument).
• .github/scripts/review-decisions.sh (6.1 — the comment misstates when it stops counting): the body said the vote closes when a /review closes the finding; it now says it stops counting once a /review settles the dispute either way, which is what the gate does (:123-126).
• .github/claude-review/review-guidelines.md, .github/workflows/claude-pr-review-continue.yml (6.1 — link the comment from the reject report): a refused argument is now reported naming who refused it and linking its decision comment, alongside the accept rule that already did.
• .github/scripts/review-decisions.sh, .github/workflows/claude-pr-review-continue.yml (5.1 — read re-fetches a list the step already has): open_votes takes an optional comments file, read is called with bot-comments.json, and post keeps its own fetch for the staleness reason the last round gave. The marker and author filter still run on the file, so they stay in one place whatever the source is. You were right that the freshness argument covers post and not read.
• .github/scripts/review-decisions.sh, .github/workflows/claude-pr-review-continue.yml (10.1 — the header documents the old rule): the usage block says one comment per disputed finding per argument and carries the new read signature; the workflow comment above the post call says the same.

Done differently

• .github/workflows/claude-pr-review-continue.yml (6.1, first option — edit the comment when a verdict is applied): declined the edit, took the effect. Whoever edits has to know the verdict was applied, and the only run that knows is the one whose review published it — read computes the tally in the gather step, 35 minutes before that, and an edit there would seal a vote a failed run never applied, losing it. The gate needs nothing to be written back: the ledger not showing the argument any more is the record, and the prompt bullet on decisions.json no longer claims a verdict is re-delivered every run, because an applied one is not.

Questions for reviewers

• .github/scripts/review-decisions.sh (1.5): the gate reads the live arguments out of previous-ledger.json, so a ledger that fails to parse would silently discard every vote on the PR — the one failure this file refuses to have. An empty ledger next to an existing decision comment now warns and counts the votes ungated rather than dropping them (:152-157), on the grounds that failing the step instead would wedge every later /review until someone edited the comment by hand. If you would rather it fail loud there, say so and it changes.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 3

Warning

⚠️ IMPORTANT FIXES REQUIRED — 2 open (1 major, 1 minor) and 8 closed, 4 of them this round.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side, which nothing could ever act on because a tick does not record who left it. This PR drops that list and posts one comment per disagreement instead, pre-seeded with a thumbs-up and a thumbs-down. The next time someone asks for a re-review, a script counts the reactions left by people with write access to the repository and either closes the finding or records that the argument was refused. Since round 2 a comment only counts while it is still asking about the disagreement the review is currently showing, so a click on a comment about something already settled no longer decides anything, and the counting side stopped re-downloading a list of comments it had already been handed.

What still needs attention

#	Problem	What it means	Severity	Status
1.7	Votes are counted unchecked when the review's own history is missing	If the record of past findings is ever lost, an old thumbs-up left about something else can close a real problem as agreed, and the review will report agreement nobody can trace.	major	❌
1.6	A disagreement with nothing to agree to is still put to a vote	A maintainer clicks to settle it and nothing ever happens, with no message anywhere saying the click was ignored.	minor	❌

Since round 2 — 4 closed, 2 new, comparing fd7c39a → a45580b

The increment is not usable as a delta this round, for the second round running. The author amended the single commit again rather than adding one, so fd7c39a is no longer an ancestor of a45580b and the compare falls back to the merge base: incremental.diff reproduces the entire PR (review-decisions.sh appears as added, +297/-0, the whole file). Every status below was judged against pr.diff and the current file contents, not against the increment.

resolutions.json is [] — no /resolve has been issued on this PR, so nothing was closed by a maintainer command this round and there is no unmatched id to report back.

✅ 1.5 — The key a vote is posted under and the key it is counted under are not the same key (minor, addressed)
The finding named a primary fix in three parts and offered one alternative for the part that was not worth building. Part one, read ignores a verdict whose arg is not the ledger's current one: read_votes now derives, per open dispute, the checksum of the author_argument the ledger is showing (review-decisions.sh:143-148) and filters the comments through LIVE (:47, applied :157). Part two, post compares against the newest comment for the finding rather than the whole history: have is now built through NEWEST (:95), the same key the tally reads on. Part three, emit arg in decisions.json, was deliberately not done, and the reasoning holds on the gated path — with the filter at :157 every entry the file carries is by construction about the live argument, so there is nothing left downstream to compare. It does not hold on the ungated path at :154-155, which is raised fresh as 1.7 rather than held against this finding. The second consequence the finding named — a paraphrase stranding the votes already cast — took the alternative the finding offered explicitly: a ponytail: comment at :105-108 naming the ceiling (the key is a checksum of a sentence a model regenerates each round) and the upgrade path (emit a stable id alongside the argument).

✅ 5.1 — post walks the whole comment thread even when nothing is disputed (minor, addressed)
The remaining half landed as the finding specified it. open_votes takes an optional comments file (:55-66), the workflow calls read decisions.json previous-ledger.json bot-comments.json (hunk @@ -252,6), and post keeps its own fetch (:95) for the staleness reason the previous round accepted. The marker and author filter still run on the file contents either way (:62-65), so the source cannot change what counts as a decision comment.

✅ 6.1 — Decision comment misreports its own state (minor, addressed)
The finding offered an edit-on-apply route or a stated minimum, and the minimum landed in full. Link the comment from the reject report as well: the contract bullet now requires a refused argument reported by name and url alongside the accepted one (review-guidelines.md, hunk @@ -376,7), and the prompt bullet says the same (hunk @@ -341,17). Stop treating a comment whose verdict has already been applied as a live vote: better than asked — the LIVE filter drops it in the script (:157), so the prompt is never handed it at all, and the comment body itself now says the vote stops counting once a /review settles the dispute "whether by closing the finding or by refusing the argument" (:124-126). The edit-on-apply route was declined with a reason I checked and accept: read computes the tally in the gather step and only the publish step 35 minutes later knows what the review actually applied, so an edit at tally time would seal a vote a failed run never used. The comment does still open with "React to this comment and the next /review applies the answer" (:117) after it has stopped counting, qualified only by the closing paragraph — that is the residue the finding accepted as the price of the minimum, and it is not being reopened.

✅ 10.1 — Script header still documents one decision comment per PR (nit, addressed)
review-decisions.sh:13-16 now reads post <ledger-file> # one comment per disputed finding, per argument and documents the new read <output-file> <ledger-file> [comments-file] signature including what the optional file is for. The workflow comment above the post call says "A comment of its own per disputed finding and argument" (hunk @@ -438,6).

New this round: 1.7 (major) and 1.6 (minor). Both are in the code the round-2 fixes added, and both are the same class of defect the fixes closed — the posting end and the counting end disagreeing about which comments are live votes — in the two cases the new gate does not cover.

Discussion since round 2. One substantive comment, from @rafaellehmkuhl (#2960 (comment)), item by item against the round-2 findings; the bare /review that follows it is the command and is ignored. Every claim in it was checked against the code rather than taken as evidence and is recorded as verified or contradicted in the Change map. It closes with a question addressed to the maintainers rather than to me, repeated here so it is not lost in a collapsed block: the author asks whether read should fail loudly when the ledger it gates on is empty, instead of warning and counting the votes ungated. My answer is finding 1.7 — do neither of those two things. Keep counting, as the author argues, but say in decisions.json that the gate did not run, so the review that consumes it can report the votes instead of closing findings on them.

Change map — what was established before judging

Claims (from the commit message and the author's follow-up comment; each checked against the code, never adopted)

• "A decision comment now counts only while its arg is the checksum of the author_argument the ledger is currently showing." — verified: the live map is built at review-decisions.sh:143-148, LIVE is defined at :47 and applied at :157, with two self-check cases at :262-267. Incomplete in the two cases in findings 1.6 and 1.7.
• "On the posting side have is built from the newest comment per finding rather than the whole history." — verified: :95, NEWEST at :42.
• "arg is not emitted in decisions.json: with the gate, every entry in it is by construction about the ledger's live argument." — verified for the gated path (:195-201 emits {id, url} plus the TALLY fields and nothing else), contradicted for the fallback at :154-155, where the gate is skipped and the same shape is emitted. That is finding 1.7.
• "Took the alternative the finding offered: a ponytail: comment naming the ceiling and the upgrade path." — verified: :105-108, in the form AGENTS.md asks for a deliberate corner-cut.
• "The body now says it stops counting once a /review settles the dispute either way." — verified: :124-126.
• "A refused argument is now reported naming who refused it and linking its decision comment." — verified: review-guidelines.md hunk @@ -376,7 and the decisions.json prompt bullet, hunk @@ -341,17.
• "open_votes takes an optional comments file, read is called with bot-comments.json, and post keeps its own fetch." — verified: :55-66, workflow hunk @@ -252,6, and :95. bot-comments.json is written at claude-pr-review-continue.yml:203-204 as a JSON array of whole comment objects with the identical author predicate, so jq -c '.[]' over it feeds open_votes exactly what the API path did.
• "Declined the edit-on-apply: only the run whose review published it knows the verdict was applied, and read runs 35 minutes earlier." — verified as a description of the pipeline: read sits in the gather step (hunk @@ -252,6), post in the publish step (hunk @@ -438,6), and nothing between them records which verdicts the model actually used.
• "An empty ledger beside a decision comment now warns and counts the votes ungated rather than dropping them." — verified: :151-158. This is finding 1.7.
• "Round 1 cannot produce a dispute, so post is deliberately not wired into the initial-review workflow." — verified: claude-pr-review.yml:112-113 requires every finding to enter the ledger as open, and the base guidelines say the same, so wiring it there would be a guaranteed no-op.
• "/resolve is untouched." — verified: review-command.sh, the authorize gate and the resolutions plumbing at claude-pr-review-continue.yml:238-253 are unchanged.

Failure site — the mechanism being replaced is the checkbox block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are removed by the diff, and a grep for raising_hand, Decisions for a human, checkbox and Ticking a box across .github/ finds no surviving reference outside the removed hunks; the :raising_hand: at review-decisions.sh:114 is the new comment's own heading.

Entry points (line numbers are the new file's own, .github/scripts/review-decisions.sh)

Function	Reached from	Frequency
open_votes (:55)	post (:95), unsourced; read_votes (:150), off bot-comments.json	per user action — up to twice per /review
permission_of (:70)	read_votes (:180), once per distinct reactor	per user action
post (:78)	publish step of claude-pr-review-continue.yml (hunk @@ -438,6), after gh pr comment, non-fatal	per user action — each published re-review
read_votes (:139)	"Gather PR state and previous review" step (hunk @@ -252,6), no `
self_check (:205)	ci.yml:128 (new step, beside the four sibling script checks)	one-shot per CI run
TALLY (:28) / NEWEST (:42) / LIVE (:47)	read_votes (:150, :157, :195-201), post (:95), self_check	as above

Invariants

• Only an admin/write reactor's click counts. One chokepoint, TALLY:28, fed by permission_of; every producer (any GitHub user can react) passes through it, so the enumeration is closed by construction.
• A decision comment is one this workflow posted. open_votes:62-65 requires github-actions[bot] and a body whose line 1 is the marker; both the publish and gather steps run on secrets.GITHUB_TOKEN (claude-pr-review-continue.yml:401, :175), so the login the filter expects is the one the comments carry. The filter runs on the file source too, so passing bot-comments.json does not widen it.
• A comment counts only while it answers the argument the ledger is still showing. New this round, single consumer at :157. Two producers escape it: a disputed entry with no author_argument never enters the live map at all (:148), which is finding 1.6, and the whole filter is skipped when the ledger is empty (:154-156), which is finding 1.7.
• One decision comment per (finding, argument), for the life of the PR. have at :95, now built on the same NEWEST key the tally reads on. This is the invariant round 2's finding 1.5 broke, and it now holds.
• decisions.json carries no free text, which is what the new prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, and up/down are GitHub logins.

1. Correctness & Implementation Bugs — 2 findings

1.7 — When the gate cannot run, the file still says it did — major

read_votes gates the tally on the arguments the ledger is currently showing (review-decisions.sh:157), except here:

if [ "$(jq 'length' "$ledger")" -eq 0 ] && [ "$(jq 'length' <<< "$votes")" -gt 0 ]; then
  echo "::warning::no findings in $ledger; counting every vote on the PR without checking its argument" >&2
else
  votes=$(jq -c --argjson live "$live" "$LIVE" <<< "$votes")
fi

(:154-158). The reasoning for counting rather than dropping is sound and I am not asking for it to be reversed — silently discarding every vote on the PR is the worse failure, and it is the one the author asked about. The defect is that nothing downstream is told. decisions.json is emitted in the identical shape either way (:195-201), the warning goes to the Actions log, and the prompt that consumes the file states the opposite as a guarantee: "It holds the votes still open on the arguments the ledger is currently showing, so a verdict already applied never comes back" (claude-pr-review-continue.yml, hunk @@ -341,17), immediately before "accept closes the finding exactly as a /resolve does". The two documents in this same diff already disagree about it — the guidelines' short-circuit asks the reviewer to check whether "every accept and reject in decisions.json is already reflected" in the ledger (hunk @@ -388,7), a state the gate makes unreachable and only this fallback can produce.

What it costs, in the state the fallback exists for — previous-ledger.json is [] while decision comments are still on the thread, which claude-pr-review-continue.yml:217 and :230-232 produce whenever the ledger block fails to parse or the bot's review comment is gone (the workflow's own comment at :246 notes anyone with write access can edit a bot comment):

• A dispute settled rounds ago is re-tallied live. One later +1 on that comment now reads as an accept, and the prompt closes the finding as resolved with a resolution saying the author's argument was accepted by vote — an argument no longer in the ledger and not in the review. That is finding 6.1's harm exactly, in the one path where the gate that closed it is switched off.
• Worse in the sub-case where the bot's review comment itself is missing: previous-review.md is then empty too, the reviewer runs as a first review and numbers its findings 1.1, 1.2, … from scratch, and an accept keyed to the old 1.1 lands on a brand-new, unrelated finding of the same id.

The fix is small and keeps the author's answer to their own question. Mark the fallback in the data rather than only in the log: set a flag on the entries ("gated": false, or a wrapper object carrying it once) at :155, and add one clause to the decisions.json prompt bullet saying an ungated verdict is reported in the since-last-round block and never closes a finding. One line in the script, one clause in the prompt, and the guarantee the bullet makes becomes true again.

1.6 — A dispute with no author_argument is put to a vote that can never be counted — minor

The two ends disagree about which disputed findings are votable:

• post selects every disputed entry whose id is shaped like a finding, with no check that it carries an argument (:84-86). It then reads the argument with // "" (:101), omits the argument line from the comment body when it is empty (:116), and keys the marker on cksum of the empty string (:109). Handling the empty case in three places is the tell that it is expected, not accidental — and the ledger is written by a language model, so a disputed entry that arrives without the field the guidelines ask for is exactly the input this has to survive.
• read_votes builds the live map only from entries where author_argument != null (:148). Such a finding therefore never enters it, LIVE compares the comment's numeric arg against null (:47) and drops it at :157.

So the comment is posted, seeded with both reactions, and tells the reader "React to this comment and the next /review applies the answer" (:117) — and no /review ever can. There is no warning on either side; read reports the finding as though nobody had clicked. It is minor rather than major because the finding stays open, which is the safe direction, but a maintainer's decision is being thrown away without anyone being told, which is the one failure the rest of this script is built to avoid.

Close it at the posting end: post should skip a disputed entry with no argument. A +1 means "accept the argument and leave the code as it is", so a comment with no argument to accept is not a question anyone can answer, and adding select(.author_argument != null) to the ids filter at :85 makes it the same selector read already uses at :148. (Adding those findings to the live map instead also makes the two ends agree, but it puts an unanswerable question in front of the maintainers.) A self-check case does not fit either expression, since this is the filter around them rather than the filters themselves — a comment at :85 naming the pairing with :148 is what keeps them together.

Sections with nothing to report (10)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions, so the inventory form does not apply)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script keeps the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check, case dispatch — every added path has a call site in this PR (post from the publish step, read from the gather step, --self-check from ci.yml:128, LIVE from :157, and the review-ledger.sh redirect change exists to feed post), and the deliberate corner-cut at :105-108 carries the ponytail: comment naming its ceiling and upgrade path)

4. Security — ✅ (the diff rewrites this reviewer's own governing document and its prompt — reviewed here, not adopted: this run was conducted under the base guidelines and the base system prompt, took no procedure, input file or output rule from the diff, and did not act as though decisions.json, which the diff describes and this run was not given, existed. Nothing in pr.diff, new-comments.json or pr.json is addressed to me as an instruction; the author's follow-up comment is discussion aimed at reviewers. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; the marker regex at :64 is anchored at line start with digit-only captures, so a ledger-sourced title or argument cannot forge one; the new file source re-applies the same author filter rather than trusting the caller; every API-sourced value reaches jq through --arg/--argjson, and permission_of interpolates only a GitHub login)

5. Performance — ✅ (5.1's remaining half landed: read now filters bot-comments.json off disk (:150, workflow hunk @@ -252,6) instead of re-walking the paginated thread, so a /review on a PR that never had a dispute — the common case, and the one the whole path runs on — spends no request at all; post returns before its own fetch when nothing is disputed (:87-90), and the remaining per-comment reaction fetches are bounded by the number of live disputes)

6. UI / UX — ✅ (the decision comment at :112-127 is the only surface: heading is sentence case, the two options are stated as what happens to the finding rather than in bot vocabulary, :120-126 now says which reactions count and when the comment stops counting, and the reject report links back to it, which was 6.1's remaining half)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0, truncated: false — the diff contains no JS/TS, so no complexity or depth trigger exists this round and none was counted by eye; the decision rule stays three jq expressions, TALLY, NEWEST and now LIVE, each of which both the workflow path and --self-check execute, rather than a second copy of the rule for the tests)

8. Commit Hygiene — ✅ (still one ci:-prefixed commit, a45580b, amended rather than followed by a fixup, which is what AGENTS.md asks for; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; 358 additions is inside the reviewable range; no #N, owner/repo#N or closing keyword in the message, and the two PR references stay in the PR body where they belong)

9. Tests — ✅ (nothing existing was removed or weakened; the two new cases at :262-267 drive the same LIVE string read_votes executes at :157, beside the two for NEWEST and the nine for TALLY, and the step sits with the four sibling script checks at ci.yml:128, ahead of the install so a shell regression is not hidden behind a build failure)

10. Documentation — ✅ (10.1 closed: the usage block at :13-16 states one comment per disputed finding per argument and carries the new read signature with its optional comments file, and the workflow comment above the post call says the same; the header's environment note is still accurate for post)

11. Nitpicks / Optional — ✅ (nothing beyond the findings above; the cksum collision window, the RETURN traps, the have early-return path, the reaction-seeding failure mode, and the header's issues: write note now being wider than what read alone needs were all read and are not worth a line)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 3

Done

• .github/scripts/review-decisions.sh, .github/workflows/claude-pr-review-continue.yml, .github/claude-review/review-guidelines.md (1.7 — when the gate cannot run, the file still says it did): the fallback is now marked in the data, not only in the log. read_votes carries gated (review-decisions.sh:144, set false at :161) and every entry in decisions.json states it (:206); the summary line prints (ungated) beside such a verdict. The prompt bullet now says what gated reports and that a false entry decides nothing — no close, no dropped author_argument, no resolved_by — and goes in the since-last-round block saying the vote could not be checked against the ledger and has to be recast or issued as a /resolve. Counting rather than discarding is unchanged, which was the part I asked about.
• .github/claude-review/review-guidelines.md (1.7, second half): the short-circuit's "already reflected" test now excludes an ungated verdict alongside tie and pending. Without that clause an ungated accept is never applied, so it never becomes reflected, and every /review would buy a full review for as long as the ledger stays unreadable.
• .github/scripts/review-decisions.sh (1.6 — a dispute with no author_argument is put to a vote that can never be counted): post's ids filter is now select(.status == "disputed" and .author_argument != null) (:87-88), the same selector read_votes:151 builds its live map from, with a comment naming the pairing. One note on the edge you did not name: author_argument: "" still posts and still counts, because "" passes != null at both ends, so the two ends agree on it and the // "" handling at :104 and :119 stays reachable. If an empty string should be treated as absent too, say so and both selectors become (.author_argument // "") != "".

Won't change (with reasoning)

• "The increment is not usable as a delta, for the second round running": this is the third amend, and it will keep happening. AGENTS.md's commit hygiene forbids leaving a commit that fixes an earlier commit on the same branch, so a review fix is a --fixup folded with --autosquash before the push, which is an amend by another name. pr.diff and the file contents are the authority for a single-commit PR; the compare API cannot be.

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 4

Note

📝 MINOR SUGGESTIONS — 3 open (all minor) and 10 closed, 2 of them this round.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side, which nothing could ever act on because a tick does not record who left it. This PR drops that list and posts one comment per disagreement instead, pre-seeded with a thumbs-up and a thumbs-down. The next time someone asks for a re-review, a script counts the reactions left by people with write access to the repository and either closes the finding or records that the argument was refused. Since round 3 that count also declares, in the file it writes rather than only in a log, whether it was able to check that each comment still asks about the disagreement the review is currently showing — so a run that has lost the record of past findings reports its votes instead of acting on them. What is left open is three small gaps between what the machinery does and what the people using it are told.

What still needs attention

#	Problem	What it means	Severity	Status
1.8	A vote can be taken on an argument that is never shown	A maintainer is asked to agree with an explanation the comment does not contain, and one click then closes a real problem as agreed, with nothing on record saying what was agreed to.	minor	❌
1.9	The reviewer's instructions contradict themselves about what can close a finding	One paragraph says a maintainer's vote closes a finding and the paragraph above it says nothing but the typed command can, so a vote may be quietly ignored.	minor	❌
6.2	A voting comment that has been overtaken still asks for votes	Once the disagreement moves on, the older comment keeps inviting clicks that no longer count, and nobody is told their decision was discarded.	minor	❌

Since round 3 — 2 closed, 3 new, comparing a45580b → 8b84f76

The increment is not usable as a delta this round, for the third round running. The author amended the single commit again, so a45580b is no longer an ancestor of 8b84f76 and the compare falls back to the merge base: incremental.diff reproduces the entire PR (review-decisions.sh appears as added, +305/-0, the whole file). Every status below was judged against pr.diff and the current file contents. The author's answer on this (#2960 (comment)) checks out and settles it — AGENTS.md:191 forbids leaving a commit that fixes an earlier commit on the same branch and :195-196 requires fixup! folded before pushing, so on a one-commit PR the amend is the required form and the compare API can never produce a usable increment here. Recorded once and not raised again in later rounds.

resolutions.json is [] — no /resolve has been issued on this PR, so nothing was closed by a maintainer command this round and there is no unmatched id to report back.

✅ 1.7 — When the gate cannot run, the file still says it did (major, addressed)
The finding asked for two things and both landed. Mark the fallback in the data rather than only in the log: read_votes carries gated (review-decisions.sh:144), sets it false on exactly the ungated branch (:161), every entry in decisions.json states it (:206), and the summary line prints (ungated) beside such a verdict (:209-210). The counting-rather-than-discarding behaviour the author asked about is unchanged, which is what the finding said it wanted. One clause in the prompt bullet: the bullet now says an entry with gated false decides nothing — "never close a finding, drop an author_argument or record a resolved_by on it" — and goes in the since-last-round block saying the vote could not be checked and has to be recast or issued as a /resolve (claude-pr-review-continue.yml, hunk @@ -341,17). The internal contradiction the finding named is gone too: the short-circuit's "already reflected" test now excludes an ungated verdict alongside tie and pending (review-guidelines.md, hunk @@ -388,7), which it had to, or an ungated accept would never be applied, never become reflected, and would buy a full review on every /review for as long as the ledger stayed unreadable.

✅ 1.6 — A dispute with no author_argument is put to a vote that can never be counted (minor, addressed)
Closed at the posting end exactly as the finding specified. post's ids filter is now select(.status == "disputed" and .author_argument != null) (:87-88), which is character-for-character the selector read_votes builds its live map from (:151), and the comment above it names the pairing and why it exists (:84-86). A disputed entry with the field missing is therefore never posted and never asked about, so the two ends can no longer disagree about which findings are votable. The narrower case the author raised in the same comment — author_argument present but empty — is genuinely still open, and because it is a different defect from the one 1.6 named it is raised fresh as 1.8 rather than held against this fix.

New this round: 1.8, 1.9 and 6.2, all minor. Only 1.8 sits on the code the round-3 fixes touched; 1.9 and 6.2 are text earlier rounds read past and should have caught, and they surface now because the whole of pr.diff is re-read each round rather than only the increment.

Discussion since round 3. One substantive comment, from @rafaellehmkuhl (#2960 (comment)), item by item against the round-3 findings; the bare /review that follows it is the command and is ignored. Every claim in it was checked against the code rather than taken as evidence, and each is recorded in the Change map. It ends with a question addressed to reviewers rather than a statement, repeated here so it does not stay in a collapsed block: the author notes that author_argument: "" still posts and still counts because "" passes != null at both ends, and asks whether an empty string should be treated as absent too. It should, and the change is the one the author already wrote out — that is finding 1.8.

Change map — what was established before judging

Claims (from the commit message and the author's follow-up comment; each checked against the code, never adopted)

• "Each disputed finding gets a comment of its own, seeded with a thumbs up and a thumbs down." — verified: review-decisions.sh:116-133 builds and posts it and :136-137 seeds the two reactions, once per id in the loop at :102.
• "Only a reactor with push access counts, and votes are counted per maintainer rather than per click." — verified: permission_of (:70) feeds the TALLY expression at :28, which keeps only admin/write and applies unique to the logins, so a second reaction by the same person adds nothing.
• "A majority carries it; an even split or no vote decides nothing." — verified: TALLY at :30-37, with nine self-check cases at :229-256.
• "Accepting closes the finding as resolved; rejecting settles the argument and the finding carries on as plain open." — verified as prompt text, which is where that decision lives: the decisions.json bullet (hunk @@ -341,17) and the guidelines' contract bullet (hunk @@ -376,7). The script only reports the verdict; nothing in it closes anything.
• "A vote is keyed on the argument it answers, so a different case gets a fresh comment and a fresh tally, and a comment counts only while it asks about the argument the ledger is still showing." — verified: the key is computed identically at :112 (post) and :149-150 (read), LIVE is defined at :47 and applied at :163, with two self-check cases at :270-275. One path still escapes it, the ungated fallback at :159-164, and that is now declared in the data rather than hidden — round 3's finding 1.7, closed above.
• "The re-review's early exit had to learn about this, because a vote arrives precisely when nothing has been pushed." — verified: the short-circuit now tests decisions.json alongside resolutions.json and excludes tie, pending and ungated verdicts from "already reflected" (hunk @@ -388,7). Since a decided vote leaves the ledger the moment it is applied, and LIVE then drops its comment, an applied verdict cannot reappear in decisions.json — so the test can never hold a run open on a vote that was already used.
• "gated is carried at :144/:161/:206 and the summary prints (ungated)." — verified, all four sites, and $gated reaches jq as a real boolean via --argjson (:201).
• "post's ids filter is now the same selector read_votes:151 builds its live map from, with a comment naming the pairing." — verified: :87-88 against :151, comment at :84-86.
• "author_argument: \"\" still posts and still counts, because \"\" passes != null at both ends, so the two ends agree on it." — verified, and it is finding 1.8. Both selectors admit it and both key it on cksum of the empty string, so the keys match and the tally is real; what the comment omits is the argument itself (:119).
• "The amend is what AGENTS.md's commit hygiene requires on a single-commit PR." — verified against AGENTS.md:191 and :195-196.
• "/resolve is untouched." — verified: review-command.sh, the authorize gate and the resolutions plumbing at claude-pr-review-continue.yml:238-253 are unchanged by this diff.

Failure site — the mechanism being replaced is the checkbox block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are removed by the diff, and a grep for raising_hand, Decisions for a human, checkbox and Ticking a box across .github/ finds no surviving reference outside the removed hunks and the unrelated issue templates; the :raising_hand: at review-decisions.sh:117 is the new comment's own heading. Every place in the guidelines that named /resolve as the only exit from a dispute (:99, :156, :200, :379, :391) is rewritten by this diff — but the same statement in the re-review prompt at claude-pr-review-continue.yml:343 is not, which is finding 1.9.

Entry points (line numbers are the new file's own, .github/scripts/review-decisions.sh)

Function	Reached from	Frequency
open_votes (:55)	post (:98), unsourced; read_votes (:153), off bot-comments.json	per user action — up to twice per /review
permission_of (:70)	read_votes (:186), once per distinct reactor	per user action
post (:78)	publish step of claude-pr-review-continue.yml (hunk @@ -438,6), after gh pr comment, non-fatal	per user action — each published re-review
read_votes (:142)	"Gather PR state and previous review" step (hunk @@ -252,6), no || :	per user action — every /review, dispute or not
self_check (:213)	ci.yml:128 (new step, beside the four sibling script checks)	one-shot per CI run
TALLY (:28) / NEWEST (:42) / LIVE (:47)	read_votes (:153, :163, :207), post (:98), self_check	as above

Invariants

• Only an admin/write reactor's click counts. One chokepoint, TALLY:28, fed by permission_of; every producer (any GitHub user can react) passes through it, so the enumeration is closed by construction. The bot's own seeded reactions are dropped earlier, by .user.type != "Bot" at :179, since the token that posted them is the repository's and would otherwise pass the permission test itself.
• A decision comment is one this workflow posted. open_votes:62-65 requires github-actions[bot] and a body whose line 1 is the marker, on the file source as well as the API one, so passing bot-comments.json cannot widen it. Both the gather and publish steps run on secrets.GITHUB_TOKEN (claude-pr-review-continue.yml:175, :401), so the login the filter expects is the one the comments carry.
• A comment counts only while it answers the argument the ledger is still showing. Single consumer at :163. Two producers used to escape it and one still does: a disputed entry with no author_argument no longer enters either end (:87-88 and :151, round 3's 1.6), while the ungated fallback (:159-164) still skips the filter and is now declared in the data instead (gated, round 3's 1.7). The empty-string case passes both selectors and is finding 1.8.
• One decision comment per (finding, argument), for the life of the PR. have at :98, built on the same NEWEST key the tally reads on. Holds; its cost is that a comment the ledger has moved past is never revisited or edited, which is finding 6.2.
• decisions.json carries no free text, which is what the prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, gated from a shell boolean passed as --argjson, and up/down are GitHub logins.

1. Correctness & Implementation Bugs — 2 findings

1.8 — An empty author_argument is put to a vote that shows no argument — minor

Round 3's 1.6 is closed: the two ends now use the same selector. What that selector admits is author_argument: "", which the author named in their follow-up and which the script handles deliberately in three places:

• post reads it with // "" (:104), computes the key as cksum of the empty string (:112), and suppresses the argument line entirely (:119, [ -z "$argument" ] || printf ...).
• read_votes puts it in the live map under the same key (:149-150), so unlike the missing-field case the tally is real and a click does decide.

The comment a maintainer then sees is the heading, the finding's title, and React to this comment and the next /review applies the answer: followed by ":+1: accept the argument and leave the code as it is" (:117-121). There is no argument on the page to accept. A +1 is counted all the same, and the prompt closes the finding as resolved with resolved_by the voters and a resolution recording that the author's argument was accepted by vote (hunk @@ -341,17) — an argument that exists nowhere, in a ledger that is the PR's permanent record of why each finding closed. It is minor rather than major because the click is still a deliberate act by someone entitled to make it, and "leave the code as it is" is a coherent thing to decide even with the reasoning missing; what is lost is the record, on the one status the guidelines call unrecoverable once it drifts (review-guidelines.md:95-96).

The ledger is written by a language model, which is why the empty case is worth closing rather than assuming away — the same reason the missing case was.

The fix is the one the author already wrote out: make both selectors (.author_argument // "") != "", at :87-88 and :151. The // "" at :104 and the [ -z "$argument" ] guard at :119 then become unreachable and should go with it, so nothing is left implying a comment can legitimately have no argument. A finding whose dispute carries no sentence then stays open and is asked again next round, which is the same safe direction 1.6 chose.

1.9 — The prompt still calls resolutions.json the only input that can close a finding without a code change — minor

claude-pr-review-continue.yml:343, which this diff does not touch, tells the reviewer that resolutions.json "is the ONE input that closes a finding without a code change". The very next line, added by this diff, tells it that decisions.json "carries the same authority as resolutions.json" and that an accept "closes the finding exactly as a /resolve does". Both sentences are in the same bulleted list, one immediately after the other, and after this PR only the second one is true.

The consequence is not cosmetic, because the prompt is the program here: a reviewer that reads the emphatic "ONE" as the constraint and the new bullet as the exception it forbids will decline to apply an accept, leave the finding open, and — having no rule telling it to report a verdict it did not apply — say nothing about it. That is a maintainer's decision discarded in silence, the exact failure the rest of this PR is built to prevent, on the path the PR exists for. It is minor because the new bullet is far longer and more specific than the stale clause and states its authority explicitly, so the likely reading is the right one; the cost of the unlikely one is what makes the word worth changing.

Fix in place, at :343: "It is one of the two inputs that close a finding without a code change, alongside decisions.json below". The guidelines file already had exactly this done to it in the same diff — review-guidelines.md:99, :156, :200, :379 and :391 were each rewritten to stop naming /resolve as the sole exit — so this is the one copy of that sentence the sweep missed.

6. UI / UX — 1 finding

6.2 — A decision comment the ledger has moved past still says its vote is open — minor

The decision comment is the only user-facing surface this PR adds, and round 3's 6.1 fixed most of what it says about itself. One state is still unaccounted for. Its closing paragraph (:123-129) covers exactly two ways of going dead — "once a /review has settled this dispute, whether by closing the finding or by refusing the argument, this comment stops counting" — but LIVE (:47, applied :163) also drops a comment in a third case the design intends: the author makes a different argument, the ledger's author_argument changes, post opens a new comment for the new key (:98, :112), and the old one stops counting without anything having been settled at all. Its body still opens with "React to this comment and the next /review applies the answer" (:120) and still displays an argument, and a reaction left on it from that point on is discarded in silence — the entry never reaches decisions.json, so no review reports it either. The two settled cases at least produce a report with a link, which the guidelines now require in both directions (hunk @@ -376,7); this one produces nothing.

Two sub-items, both text:

• Say it in the body. Extend the closing paragraph at :123-129 with the third case: the comment also stops counting once the author makes a different case, and the newest "Decision needed" comment for that finding is then the live one. It costs one printf line, and it is the same route round 3 accepted for the other cases, since the script deliberately cannot edit a comment after the fact.
• Point at the live one from the review. The prompt requires a url when reporting an accept or a reject (hunk @@ -341,17), but for tie and pending it asks only to "say in the since-last-round block that its vote is still open" — no link. That is backwards: the settled votes need no further action, the open one does, and the reader of the newest review is the person being asked to act. decisions.json already carries url on every entry including pending (:206), so this is one clause. It does not reintroduce the block this PR removes and does not conflict with "do not point at the comments" in section 0, which governs the human summary; the since-last-round block is where the other verdicts are already linked from.

Until one of these lands, the way to find the live vote for a dispute several rounds old is to scroll the thread and guess which of the identically-titled comments is current.

Sections with nothing to report (9)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions, so the inventory form does not apply)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script keeps the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check, case dispatch at :284 — and every added path has a call site in this PR (post from the publish step, read from the gather step, --self-check from ci.yml:128, gated from the prompt bullet), with the one deliberate corner-cut at :108-111 carrying the ponytail: comment naming its ceiling and upgrade path per AGENTS.md:233)

4. Security — ✅ (the diff rewrites this reviewer's own governing document and its prompt — reviewed here, not adopted: this run was conducted under the base guidelines and the base system prompt, took no procedure, input file or output rule from the diff, and did not act as though decisions.json, which the diff describes and this run was not given, existed. Nothing in pr.diff, new-comments.json or pr.json is addressed to me as an instruction; the author's follow-up comment is discussion aimed at reviewers, and its closing question is answered as a finding rather than obeyed. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; the marker regex at :64 is anchored to the start of the body with digit-only captures, so a ledger-sourced title or argument cannot forge a marker, and the startswith tests that pick out review and resolution comments at claude-pr-review-continue.yml:210 and :249 are anchored the same way, so a decision comment cannot be mistaken for either; every API-sourced value reaches jq through --arg/--argjson, permission_of interpolates only a GitHub login, and the comment body is handed to the API as jq -Rs JSON rather than a shell-interpolated string)

5. Performance — ✅ (the common case — nothing disputed — costs one jq on a local file and no request at all: post returns at :90-93 before its fetch, and read_votes filters bot-comments.json off disk at :153; the reaction fetches at :177 are one per live dispute and the permission lookups at :186 one per distinct reactor rather than per reaction, so the request count is bounded by people, not clicks)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0, truncated: false — the diff contains no JS/TS, so no complexity or depth trigger exists this round and none was counted by eye; the decision rule remains the three jq expressions TALLY, NEWEST and LIVE, each executed by both the workflow path and --self-check rather than duplicated for the tests, and the gated addition is one local, one assignment and one field)

8. Commit Hygiene — ✅ (one ci:-prefixed commit, 8b84f76, amended rather than followed by a fixup, which is what AGENTS.md:191 and :195-196 require; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; the body explains the mechanism and the alternative it rejected; 366 additions is inside the reviewable range; no #N, owner/repo#N or closing keyword in the message)

9. Tests — ✅ (nothing existing was removed or weakened; the self-check now drives all three expressions the workflow executes — nine TALLY cases at :229-256, two NEWEST at :260-267, two LIVE at :270-275 — and the step sits with the four sibling script checks at ci.yml:128, ahead of the install so a shell regression is not hidden behind a build failure; the gated branch is shell control flow around those expressions, which that harness cannot reach, and no new harness is being asked for)

10. Documentation — ✅ (the usage block at :13-17 still matches both entry points, and the read line's output description was updated in step with the code to [{id, url, gated, verdict, up, down}] per live vote (:15-16); the workflow comments above both call sites describe what each does, and the guidelines' own account of the mechanism was rewritten in the same diff — the one stale sentence left anywhere is the prompt clause in finding 1.9, reported there rather than twice)

11. Nitpicks / Optional — ✅ (nothing beyond the findings above; the cksum collision window, the RETURN traps, the hard return 1 at :196-199 when no reactor's permission can be read, re-raising an argument that was already refused and getting the earlier verdict back on the same comment, and a write-access user editing the marker on a comment that already has reactions were all read and are not worth a line — the last two are reachable only by someone who could /resolve the finding outright)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 4

Done

• .github/scripts/review-decisions.sh (1.8 — an empty author_argument is put to a vote that shows no argument): both ends now refuse it, and the selector is written once rather than twice. VOTABLE (:40-44) is select(.status == "disputed" and (.author_argument // "") != ""), used by post's ids (:91) and by the live map in read_votes (:156), so the two cannot drift apart on the next case the way they did on this one and on 1.6. The handling that assumed a comment could legitimately have no argument went with it: // "" at the old :104 and the [ -z "$argument" ] guard at the old :119. New self-check case covers all four shapes — argument, empty string, missing field, not disputed.
• .github/workflows/claude-pr-review-continue.yml (1.9 — the prompt still calls resolutions.json the only input that can close a finding): :357 now reads "one of the two inputs that close a finding without a code change, alongside decisions.json below". That was the one copy of the sentence the sweep through the guidelines missed.
• .github/scripts/review-decisions.sh (6.2, first sub-item — a comment the ledger has moved past still asks for votes): the closing paragraph now names the third way a comment goes dead (:131-134) — the author makes a different case, and the newest "Decision needed" comment for that finding is the live one.
• .github/workflows/claude-pr-review-continue.yml (6.2, second sub-item — point at the live one from the review): the tie/pending clause at :358 now requires the url too, saying why: an open vote is the one the reader can still act on, so it needs the link more than a settled one does.

Questions for reviewers

• .github/scripts/review-decisions.sh (1.8): the fix leaves an empty-argument dispute silently unposted — no comment, no vote, and nothing in the review saying why that dispute never reached a ballot. Same as the missing-field case since round 3, so it is not new, but the ledger is model-written and an empty author_argument is a ledger defect rather than a state the design intends. Worth a warning line from post, or is the finding staying open answer enough?

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 5

Note

📝 MINOR SUGGESTIONS — 1 open (minor) and 13 closed, 3 of them this round.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side, which nothing could ever act on because a tick does not record who left it. This PR drops that list and posts one comment per disagreement instead, pre-seeded with a thumbs-up and a thumbs-down; the next time someone asks for a re-review, a script counts the reactions left by people with write access and either closes the finding or records that the explanation was refused. This round closes the last three gaps between what that machinery does and what people are told about it: a disagreement with no written explanation is no longer put to a vote at all, the reviewer's own instructions no longer contradict themselves about what can close a finding, and a voting comment now says every way it can go dead while the review links the one that is still live. What remains is the flip side of the first of those.

What still needs attention

#	Problem	What it means	Severity	Status
1.10	A disagreement can be marked as awaiting a decision with no way to make one	The summary says a disagreement is waiting and that there is a comment to vote in, but for one shape of bad input no comment is posted and nothing anywhere says so, so the maintainer hunts for a ballot that does not exist.	minor	❌

Since round 4 — 3 closed, 1 new, comparing 8b84f76 → 2f43771

The increment is not usable as a delta this round, for the fourth round running. The author amended the single commit again (pr.json carries one commit, 2f43771), so 8b84f76 is no longer an ancestor of the head and the compare falls back to the merge base: incremental.diff reproduces the entire PR — its first line is === .github/claude-review/review-guidelines.md (modified, +28/-34) === and review-decisions.sh appears as added, +320/-0, the whole file. Every status below was judged against pr.diff and the current file contents. This was settled in round 4 and is recorded here only because the contract asks for the range: AGENTS.md:191 and :195-196 make the amend the required form on a one-commit PR, so the compare API can never produce a usable increment here. Not raised as a finding.

resolutions.json is [] — no /resolve has been issued on this PR, so nothing was closed by a maintainer command this round and there is no unmatched id to report back. Line numbers below are the new file's own; add 121 to get the pr.diff line.

✅ 1.8 — An empty author_argument is put to a vote that shows no argument (minor, addressed)
The finding asked for four things and all four landed, in the stronger form the author chose. Both selectors reject an empty argument: they are now one definition, VOTABLE at :44 — select(.status == "disputed" and (.author_argument // "") != "") — used by post's ids at :91 and by the live map in read_votes at :156, so the two ends cannot drift apart on the next case the way they did on this one and on 1.6. The handling that assumed a comment could legitimately have no argument is gone: argument is read without // "" (:107) and the argument line is printed unconditionally (:122, formerly guarded by [ -z "$argument" ]). A self-check covers it: the new case at :276-282 asserts ["1.1"] over the four shapes — an argument, an empty string, a missing field, and a non-disputed entry that carries one.

✅ 1.9 — The prompt still calls resolutions.json the only input that can close a finding (minor, addressed)
claude-pr-review-continue.yml:357 now reads "It is one of the two inputs that close a finding without a code change, alongside decisions.json below", where it said "the ONE input". That was the last copy of the sentence the sweep through the guidelines missed, and the contradiction with the decisions.json bullet immediately below it is gone.

✅ 6.2 — A decision comment the ledger has moved past still says its vote is open (minor, addressed)
Both sub-items landed. Say it in the body: the closing paragraph now names the third way a comment goes dead (:131-134) — "The same goes once the author makes a different case: the argument above stops being the one in question, and the newest 'Decision needed' comment for this finding is the live vote" — which is exactly the case LIVE (:53, applied :168) drops and the paragraph did not cover. Point at the live one from the review: the tie/pending clause at claude-pr-review-continue.yml:358 now requires the url as well, with the reason stated — an open vote is the one the reader can still act on. Every verdict in decisions.json is therefore reported with a link, in both directions, which is what the guidelines' contract bullet asks for.

New this round: 1.10, minor, on the code the 1.8 fix touched — it is the reporting side of the state that fix now refuses.

Discussion since round 4. One substantive comment, from @rafaellehmkuhl (#2960 (comment)); the bare /review that follows it is the command and is ignored. Every "Done" claim in it was checked against the file rather than taken as evidence, and each is recorded in the Change map — the four line references it gives (:40-44, :91, :156, :131-134, and :357/:358 in the workflow) all land where it says. Its closing question is addressed to reviewers, so it is answered here rather than left in a collapsed block: the author asks whether an empty-argument dispute being left silently unposted is worth a warning line from post, "or is the finding staying open answer enough?". It is worth the warning line — the finding staying open is not visible as an answer to anyone, because the review still marks the dispute :speech_balloon: and the guidelines this PR writes now promise that a comment to vote in is on the thread. That is finding 1.10, raised at minor rather than higher because /resolve still closes such a finding and the state needs a defective ledger entry to arise at all.

Change map — what was established before judging

Claims (from the commit message and the author's round-4 follow-up; each checked against the code, never adopted). Line numbers are .github/scripts/review-decisions.sh as the diff leaves it.

• "Each disputed finding gets a comment of its own, seeded with a thumbs up and a thumbs down." — verified: :118-135 builds it and :141-142 seeds the two reactions, once per id in the loop at :105.
• "Only a reactor with push access counts, and votes are counted per maintainer rather than per click." — verified: permission_of (:76) feeds TALLY (:28), which keeps only admin/write and applies unique to the logins.
• "A majority carries it; an even split or no vote decides nothing." — verified: TALLY:30-37, with nine self-check cases at :234-261.
• "Accepting closes the finding as resolved; rejecting settles the argument and the finding carries on as plain open." — verified as prompt text, which is where that decision lives (claude-pr-review-continue.yml:358, and the guidelines' contract bullet, hunk @@ -376,7). The script only reports a verdict; nothing in it closes anything.
• "A vote is keyed on the argument it answers, so a different case gets a fresh comment and a fresh tally." — verified: the key is computed identically at :115 (post) and :154-155 (read), LIVE is defined at :53 and applied at :168, with two self-check cases at :285-290.
• "Both ends now refuse an empty author_argument, and the selector is written once rather than twice." — verified: VOTABLE at :44, used at :91 and :156; the // "" and the [ -z "$argument" ] guard the previous round pointed at are both gone (:107, :122); the four-shape self-check is at :276-282.
• ":357 now says one of the two inputs, and :358 requires the url on tie and pending too." — verified, both in claude-pr-review-continue.yml.
• "The closing paragraph names the third way a comment goes dead." — verified at :131-134.
• "The re-review's early exit had to learn about this, because a vote arrives precisely when nothing has been pushed." — verified: the short-circuit now tests decisions.json alongside resolutions.json and excludes tie, pending and ungated verdicts from "already reflected" (guidelines, hunk @@ -388,7). A decided vote leaves the ledger the moment it is applied and LIVE then drops its comment, so an applied verdict cannot reappear and hold a run open.
• "post is not wired into the initial-review workflow, because round 1 cannot produce a dispute." — verified: claude-pr-review.yml:113 requires every round-1 finding to enter the ledger with status: "open", and that workflow's publish step still sends the ledger to /dev/null (:160) and calls nothing.
• "/resolve is untouched." — verified: review-command.sh, the authorize gate and the resolutions plumbing at claude-pr-review-continue.yml:238-253 are unchanged by this diff.

Failure site — the mechanism being replaced is the checkbox block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are removed by the diff, and rg for raising_hand, Decisions for a human, checkbox and Ticking a box across .github/ returns those three lines and nothing else, so no workflow prompt is left asking for a block the guidelines no longer define; the :raising_hand: at review-decisions.sh:120 is the new comment's own heading.

Entry points

Function	Reached from	Frequency
open_votes (:61)	post (:101), unsourced; read_votes (:158), off bot-comments.json	per user action — up to twice per /review
permission_of (:76)	read_votes (:191), once per distinct reactor	per user action
post (:84)	publish step of claude-pr-review-continue.yml (hunk @@ -438,6), after gh pr comment, non-fatal	per user action — each published re-review
read_votes (:147)	"Gather PR state and previous review" step (hunk @@ -252,6), no || :	per user action — every /review, dispute or not
self_check (:218)	ci.yml:128 (new step, beside the four sibling script checks)	one-shot per CI run
TALLY (:28) / VOTABLE (:44) / NEWEST (:48) / LIVE (:53)	post (:91, :101), read_votes (:156, :158, :168), self_check	as above

Invariants

• Only an admin/write reactor's click counts. One chokepoint, TALLY:28, fed by permission_of; every producer (any GitHub user can react) passes through it. The bot's own seeded reactions are dropped earlier, by .user.type != "Bot" at :184, since the token that posted them is the repository's and would pass the permission test itself.
• A decision comment is one this workflow posted. open_votes:68-71 requires github-actions[bot] and a body whose line 1 is the marker, on the file source as well as the API one. Checked this round rather than assumed: jq's ^ anchors to the start of the string, not of a line (jq -n '"prose\nXY-marker …" | [capture("^XY-marker …")]' returns []), so a review comment that merely quotes the marker in prose — which is how the ledger extractor once lost a real ledger — cannot be read as a ballot.
• A comment counts only while it answers the argument the ledger is still showing. Single consumer at :168. Both producers now go through one definition of votable (VOTABLE:44, this round's 1.8), and the one path that still skips the filter — the ungated fallback at :164-166, taken when the ledger is unreadable — declares itself in the data via gated (round 3's 1.7) instead of deciding anything.
• One decision comment per (finding, argument), for the life of the PR. have at :101, built on the same NEWEST key the tally reads on.
• Every dispute the review marks :speech_balloon: has a comment on the thread to vote in. This is new: the guidelines this diff writes state it as the reason the summary needs no block of its own (hunk @@ -182,23). Its single producer is post's ids at :90-92, and two inputs break it there — an author_argument that is empty or missing, and an id not shaped N.M — with no output in either case. That is finding 1.10.
• decisions.json carries no free text, which is what the prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, gated from a shell boolean passed as --argjson, and up/down are GitHub logins.

1. Correctness & Implementation Bugs — 1 finding

1.10 — A dispute refused by post is dropped without a word, while the review says its ballot is waiting — minor

Round 4's 1.8 is closed the right way round: an empty author_argument can no longer put a question to maintainers that the comment does not contain. What it leaves is the other end of that decision. post:90-96 selects the votable ids and then:

if [ -z "$ids" ]; then
  echo 'no disputed finding needs a decision comment'
  return 0
fi

With a ledger holding a disputed entry whose author_argument is "" or missing, ids is empty and that line runs — and it is not true. A disputed finding does need a decision comment; this one just cannot have one. When some findings are votable and others are not, the refused ones produce no output at all, not even a false line. The same silence covers the second filter on that expression, select(.id | test("^[0-9]+\\.[0-9]+$")), which drops a malformed id for a good reason (the id goes into the marker the reader parses back) and equally without saying so.

What the maintainer sees is the mismatch. The review's summary table still carries :speech_balloon: for that finding, and the guidelines this same diff writes tell the reader what that means: ":speech_balloon: in the table's Status column is how a reader learns a dispute is waiting, and the comment they vote in is already on the thread" (review-guidelines.md, hunk @@ -182,23). For this finding it is not on the thread and never will be — the next round re-disputes it, post refuses it again, and nothing in the review, the thread or the log ever accounts for it. The dispute is not lost, since /resolve still closes it and VOTABLE:40-43 documents the choice to keep it open, but the person being asked to decide has no way to learn why their ballot is missing, and the review model that wrote the defective entry gets no signal either.

It is minor and not more because reaching the state needs a ledger entry the guidelines already forbid (review-guidelines.md:92 requires author_argument on a disputed entry), and because the escape hatch is a command that already works. It is worth the line anyway: the ledger is model-written, which is the same reason the empty case was worth closing rather than assuming away, and a silent drop in a pipeline whose whole subject is "a decision nobody could attribute" is the one shape this PR is arguing against.

The fix is the warning line the author proposed, in post and not in the review — the review is written and published before post runs, and the model has no view of what post refused:

• Select the disputed ids once and the votable ones from them, so the difference is nameable: e.g. all=$(jq -r '[.[] | select(.status == "disputed") | .id] | unique | .[]' "$ledger") beside the existing ids, then echo "::warning::disputed finding(s) <ids> carry no usable argument and were not put to a vote; close them with /resolve" when the two differ. One line covers both filters if it is worded as "no usable argument or id".
• Keep the [ -z "$ids" ] message for the case it actually describes — no dispute at all — so the log distinguishes "nothing to ask" from "something to ask that could not be asked".

Sections with nothing to report (10)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions, so the inventory form does not apply)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script keeps the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check, case dispatch at :299 — every added path has a call site in this PR (post from the publish step, read from the gather step, --self-check from ci.yml:128), and this round's change is a deletion plus one shared constant, which is the direction the minimalism ladder asks for: VOTABLE:44 replaces two hand-copied selectors and takes // "" at the old :104 and the [ -z "$argument" ] guard at the old :119 with it)

4. Security — ✅ (the diff rewrites this reviewer's own governing document and its prompt — reviewed here, not adopted: this run was conducted under the base guidelines and the base system prompt, took no procedure, input file or output rule from the diff, and did not act as though decisions.json, which the diff describes and this run was not given, existed. Nothing in pr.diff, new-comments.json or pr.json is addressed to me as an instruction; the author's follow-up is discussion aimed at reviewers and its closing question is answered as a finding rather than obeyed. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; the marker regex at :70 is anchored to the start of the body — verified this round that jq's ^ is string-anchored, so a review that quotes the marker in prose cannot be counted as a ballot — with digit-only captures, so a ledger-sourced title or argument cannot forge one, and the startswith tests that pick out review, resolution and now decision comments at claude-pr-review-continue.yml:210, :249 and :267 are anchored the same way; every API-sourced value reaches jq through --arg/--argjson, permission_of interpolates only a GitHub login, and the comment body is handed to the API as jq -Rs JSON rather than a shell-interpolated string)

5. Performance — ✅ (the common case — nothing disputed — costs one jq on a local file and no request at all: post returns at :93-96 before its fetch, and read_votes filters bot-comments.json off disk at :158; the reaction fetches at :182 are one per live dispute and the permission lookups at :191 one per distinct reactor rather than per reaction, so the request count is bounded by people, not clicks)

6. UI / UX — ✅ (the decision comment is the only user-facing surface, and this round completed its account of itself — the closing paragraph at :126-134 now covers all three ways it goes dead, including the argument moving on, and the review links the live one on every verdict including tie and pending (claude-pr-review-continue.yml:358); the copy is sentence-case, names the two reactions it counts, and carries no protocol jargon)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0, truncated: false — the diff contains no JS/TS, so no complexity or depth trigger exists this round and none was counted by eye; the decision rule is now four jq constants, TALLY, VOTABLE, NEWEST and LIVE, each executed by both the workflow path and --self-check rather than duplicated for the tests, and VOTABLE removed the one selector this PR had written out twice)

8. Commit Hygiene — ✅ (one ci:-prefixed commit, 2f43771, amended rather than followed by a fixup, which is what AGENTS.md:191 and :195-196 require; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; the body explains the mechanism and the alternative it rejected; 382 additions is inside the reviewable range; no #N, owner/repo#N or closing keyword in the message, and the two PR references the author needs are in the PR body where they belong)

9. Tests — ✅ (nothing existing was removed or weakened; the self-check drives all four expressions the workflow executes — nine TALLY cases at :234-261, two NEWEST at :265-272, the new four-shape VOTABLE case at :276-282, two LIVE at :285-290 — and the step sits with the four sibling script checks at ci.yml:128, ahead of the install so a shell regression is not hidden behind a build failure)

10. Documentation — ✅ (the usage block at :13-17 still matches both entry points and the read line still describes its output as [{id, url, gated, verdict, up, down}] per live vote; the VOTABLE comment at :40-43 states why one definition serves both ends and what a dispute with no argument does instead, the ponytail: note at :111-114 still names the cksum ceiling and its upgrade path, and the stale prompt sentence round 4 found is fixed at claude-pr-review-continue.yml:357)

11. Nitpicks / Optional — ✅ (nothing beyond the finding above; section 0's "It has four parts" still reads correctly with Nothing else. as the fourth, the cksum collision window, the RETURN traps, the hard return 1 at :201-204 when no reactor's permission can be read — consistent with every other gh call in a step that runs set -euo pipefail — and an author re-raising a verbatim argument that was already refused and getting the earlier verdict back on the same comment were all read and are not worth a line)

Generated by Claude. This is advisory; a human reviewer must still approve.

[rafaellehmkuhl]

Review follow-up — round 5

Done

• .github/scripts/review-decisions.sh (1.10 — a dispute refused by post is dropped without a word): post now names what it refused. The votable-id expression became a constant, VOTABLE_IDS (:46-48), and UNVOTABLE_IDS (:50-52) is the disputed ids minus that set, so the difference is computed from one definition rather than a second hand-copied filter. When it is non-empty, :99-100 emits ::warning::disputed finding(s) <ids> carry no usable argument or id and were not put to a vote; close them with /resolve — one line covering both filters, as suggested. The [ -z "$ids" ] message is kept for the case it describes (:101-104): with a refused dispute on the ledger it no longer claims no dispute needs a comment, because the warning above already said one does and could not have it. Self-check case at :293-301, over the same four shapes as the 1.8 case, asserting ["1.2","bogus"].

[rafaellehmkuhl]

/review

[github-actions]

Automated PR Review — round 6

Tip

✅ READY TO MERGE — nothing open, and 14 closed, 1 of them this round.

When the reviewer and the PR author disagree about a finding, the review used to print a checklist asking a maintainer to pick a side, which nothing could ever act on because a tick does not record who left it. This PR drops that list and posts one comment per disagreement instead, pre-seeded with a thumbs-up and a thumbs-down; the next time someone asks for a re-review, a script counts the reactions left by people with write access and either closes the finding or records that the explanation was refused. This round closes the last gap between that machinery and what people are told about it: a disagreement the script cannot put to a vote — because nobody wrote down the explanation, or because its reference number is malformed — is now named in the run's log instead of vanishing, so the maintainer being asked to decide is not left hunting for a ballot that was never posted.

What still needs attention

Nothing. Every finding this PR has raised is closed, and this round found no new ones.

Since round 5 — 1 closed, 0 new, comparing 2f43771 → 4cbc4ab

The increment is not usable as a delta this round, for the fifth round running. The author amended the single commit again (pr.json carries one commit, 4cbc4ab), so 2f43771 is no longer an ancestor of the head and the compare falls back to the merge base: incremental.diff reproduces the entire PR — its file headers are review-guidelines.md (modified, +28/-34), review-decisions.sh (added, +339/-0), ci.yml (modified, +3/-0) and claude-pr-review-continue.yml (modified, +31/-8), which is pr.json's file list exactly. The status below was judged against pr.diff and the current file contents. This was settled in round 4 and is recorded here only because the contract asks for the range: AGENTS.md:191 and :195-196 make the amend the required form on a one-commit PR, so the compare API can never produce a usable increment here. Not raised as a finding.

resolutions.json is [] — no /resolve has been issued on this PR, so nothing was closed by a maintainer command this round and there is no unmatched id to report back. Line numbers below are .github/scripts/review-decisions.sh as the diff leaves it; add 121 to get the pr.diff line.

✅ 1.10 — A dispute refused by post is dropped without a word, while the review says its ballot is waiting (minor, addressed)
The finding asked for three things and all three landed. Make the difference nameable from one definition rather than a second hand-copied filter: the votable-id expression is now the constant VOTABLE_IDS (:46-48), still built on VOTABLE, and UNVOTABLE_IDS (:50-52) is ([.[] | select(.status == "disputed") | .id] | unique) - (VOTABLE_IDS) — the complement by construction, so the two sets cannot drift and no disputed id can fall through both. Say what was refused: :98 computes it and :99-100 emits ::warning::disputed finding(s) <ids> carry no usable argument or id and were not put to a vote; close them with /resolve, one line covering both filters, as the finding suggested. Keep the old message for the case it describes: :101-104 now prints no disputed finding needs a decision comment only when unvotable is empty, so with a refused dispute on the ledger it no longer claims nothing needed asking. Checked the arithmetic of the new expression against the self-check case the author added at :293-301: over 1.1 (argument, valid id), 1.2 (empty argument), bogus (argument, malformed id) and 1.4 (open, argument), VOTABLE_IDS is ["1.1"] and the difference is ["1.2","bogus"], which is what the case asserts.

Nothing new this round. All eleven guideline sections were re-run over the whole of pr.diff, not the increment; what that turned up and rejected is in the Change map and in the clean-section clauses below.

Discussion since round 5. One substantive comment, from @rafaellehmkuhl (#2960 (comment)); the bare /review that follows it is the command and is ignored. Every claim in it was checked against the file rather than taken as evidence, and every line reference it gives lands where it says (:46-48, :50-52, :99-100, :101-104, :293-301). One description in it is slightly off and the code is not: the new self-check case is said to run "over the same four shapes as the 1.8 case", but it substitutes a malformed id for that case's missing-author_argument entry. That is the right substitution — the missing-field shape is already covered where it is decided, in the VOTABLE case at :283-291, and UNVOTABLE_IDS inherits it as a difference — so it is recorded here rather than raised.

Change map — what was established before judging

Claims (from the PR body, the commit message and the author's round-5 follow-up; each checked against the code, never adopted). Line numbers are .github/scripts/review-decisions.sh as the diff leaves it.

• "Each disputed finding gets a comment of its own, seeded with a thumbs up and a thumbs down." — verified: :127-144 builds it and :150-151 seeds the two reactions, once per id in the loop at :114.
• "Only a reactor with push access counts, and votes are counted per maintainer rather than per click." — verified: permission_of (:84) feeds TALLY (:28-38), which keeps only admin/write and applies unique to the logins.
• "A majority carries it; an even split or no vote decides nothing." — verified: the verdict chain at :34-37, with nine self-check cases at :243-270.
• "Accepting closes the finding as resolved; rejecting settles the argument and the finding carries on as plain open." — verified as prompt text, which is where that decision lives (claude-pr-review-continue.yml:358, and the guidelines' contract bullet, hunk @@ -376,7). The script only reports a verdict; nothing in it closes anything.
• "A vote is keyed on the argument it answers, so a different case gets a fresh comment and a fresh tally." — verified: the key is computed identically at :124 (post) and :163-164 (read), LIVE is defined at :61 and applied at :177, with two self-check cases at :303-309.
• "post now names what it refused; the difference is computed from one definition." — verified: VOTABLE :44 → VOTABLE_IDS :46-48 → UNVOTABLE_IDS :50-52, consumed at :97 and :98, warned at :99-100, with the self-check at :293-301. This round's 1.10.
• "The re-review's early exit had to learn about this, because a vote arrives precisely when nothing has been pushed." — verified: the short-circuit now tests decisions.json alongside resolutions.json and excludes tie, pending and ungated verdicts from "already reflected" (guidelines, hunk @@ -388,7). Traced the consequence rather than taking the wording: an applied accept or reject leaves the finding non-disputed, so it drops out of the live map at :161-165 and LIVE drops its comment, which means any accept/reject still present in decisions.json is by construction one that has not been applied. The clause cannot hold a run open on a verdict already banked.
• "post is not wired into the initial-review workflow, because round 1 cannot produce a dispute." — verified: claude-pr-review.yml:113 requires every round-1 finding to enter the ledger with status: "open", that workflow's publish step still sends the ledger to /dev/null (:160) and calls nothing, and it triggers only on opened/ready_for_review (:9), so it cannot run on a PR that has decision comments.
• "/resolve is untouched." — verified: review-command.sh, the authorize gate and the resolutions plumbing at claude-pr-review-continue.yml:238-253 are unchanged by this diff; the only edit inside authorize is the comment block at :19-24.
• Test-plan lines for the post, read and comment-filter dry runs against a stubbed gh — not verifiable from the diff: those stubs are not in the tree, so they are the author's report of a local run and are recorded as such. What is in the tree and does run in CI is --self-check, covered above.

Failure site — the mechanism being replaced is the checkbox block at review-guidelines.md:185-201 plus its slot in the output contract at :338. Both are removed by the diff, and a grep for raising_hand, Decisions for a human, Ticking a box and checkbox across .github/ returns those lines and nothing else, so no workflow prompt is left asking for a block the guidelines no longer define; the :raising_hand: at review-decisions.sh:129 is the new comment's own heading. The site of this round's fix is post's early return, formerly a bare [ -z "$ids" ] message and now :97-105.

Entry points

Function	Reached from	Frequency
open_votes (:69)	post (:110), unsourced; read_votes (:167), off bot-comments.json	per user action — up to twice per /review
permission_of (:84)	read_votes (:200), once per distinct reactor	per user action
post (:92)	publish step of claude-pr-review-continue.yml (hunk @@ -438,6), after gh pr comment, non-fatal	per user action — each published re-review
read_votes (:156)	"Gather PR state and previous review" step (hunk @@ -252,6), no || :	per user action — every /review, dispute or not
self_check (:227)	ci.yml:128 (new step, beside the four sibling script checks)	one-shot per CI run
TALLY (:28) / VOTABLE (:44) / VOTABLE_IDS (:46) / UNVOTABLE_IDS (:50) / NEWEST (:56) / LIVE (:61)	post (:97, :98, :110), read_votes (:165, :167, :177), self_check	as above

Invariants

• Only an admin/write reactor's click counts. One chokepoint, TALLY:28, fed by permission_of; every producer (any GitHub user can react) passes through it. The bot's own seeded reactions are dropped earlier, by .user.type != "Bot" at :193, since the token that posted them is the repository's and would pass the permission test itself.
• A decision comment is one this workflow posted. open_votes:77-78 requires github-actions[bot] and a body whose line 1 is the marker, on the file source as well as the API one. jq's ^ anchors to the start of the string rather than of a line, so a review comment that merely quotes the marker in prose — which is how the ledger extractor once lost a real ledger — cannot be read as a ballot. Traced the forgery path once more this round now that the marker carries a computable arg: posting under that author requires a token this repository issues, which is write access, and write access already has /resolve. No escalation.
• A comment counts only while it answers the argument the ledger is still showing. Single consumer at :177. Both producers go through one definition of votable (VOTABLE:44), and the one path that skips the filter — the ungated fallback at :173-178, taken when the ledger is unreadable — declares itself in the data via gated instead of deciding anything.
• One decision comment per (finding, argument), for the life of the PR. have at :110, built on the same NEWEST key the tally reads on, tested at :125-126.
• Every dispute the review marks :speech_balloon: is either put to a vote or named in the run's log. Closed at both ends this round. The two producers are ids (:97) and unvotable (:98), and because the second is defined as the set difference of the first over the disputed entries, there is no third outcome an id can take. The remaining silent path is an API failure inside the loop, which self-heals — the next round finds no comment under that key and posts one — and is already announced by ::warning::could not put this round's disputed findings to a vote at the call site.
• decisions.json carries no free text, which is what the prompt bullet asserts. Holds: id is constrained by the marker regex to N.M, url comes from the API, verdict from TALLY, gated from a shell boolean passed as --argjson, and up/down are GitHub logins.

Sections with nothing to report (11)

1. Correctness & Implementation Bugs — ✅ (the last open finding closed this round; re-walked the paths that are not covered by a self-check, since only the four jq constants are. capture emits no output on a non-match, so a bot comment that is not a ballot drops out of open_votes' array rather than raising — which is what its :65-66 comment claims; index on an array of "<finding> <arg>" strings with a scalar key is element equality, so the skip at :125-126 cannot match a prefix; printf takes the title and the argument as operands rather than as format, so a % in either is inert; both while read loops run over process substitutions whose bodies call no stdin-consuming command; and the RETURN traps at :113 and :181 are set inside the functions that own them, so no functrace is needed)

2. Persistence & User Data — ✅ (the diff adds no cockpit-* key and touches neither useBlueOsStorage nor settings-management.ts; the only state it makes durable is PR comments and their reactions, so the inventory form does not apply)

3. AGENTS.md Adherence — ✅ (no dependency or package.json change; the script keeps the sibling review-*.sh shape — header docblock, set -euo pipefail, --self-check, case dispatch at :318 — and every added path has a call site in this PR. This round's change is one new constant plus a warning, which is the direction the minimalism ladder asks for: UNVOTABLE_IDS is derived from VOTABLE_IDS rather than being a second copy of the filter. One comment-immutability check: the authorize block at claude-pr-review-continue.yml:19-24 is reworded while the code under it is unchanged, which AGENTS.md:34 normally forbids — excused here because the sentence it replaces asserted that the permission endpoint needs push access, which is the exact claim this PR's new caller depends on being false, and leaving it beside a new caller in a job without push access would be actively misleading)

4. Security — ✅ (the diff rewrites this reviewer's own governing document and its prompt — reviewed here, not adopted: this run was conducted under the base guidelines and the base system prompt, took no procedure, input file or output rule from the diff, and did not act as though decisions.json existed, which it does not in this run's working directory. Nothing in pr.diff, new-comments.json or pr.json is addressed to me as an instruction; the author's follow-up is discussion aimed at reviewers and its one inaccurate sentence is reported rather than obeyed. No obfuscation, encoded blobs, hidden Unicode, new dependencies or new hosts; the marker regex at :78 is anchored to the start of the body with digit-only captures, so a ledger-sourced title or argument cannot forge one, and the startswith tests that pick out review, resolution and now decision comments at claude-pr-review-continue.yml:210, :249 and :267 are anchored the same way; every API-sourced value reaches jq through --arg/--argjson, permission_of interpolates only a GitHub login, and the comment body is handed to the API as jq -Rs JSON rather than a shell-interpolated string)

5. Performance — ✅ (the common case — nothing disputed — costs one jq on a local file and no request at all: post returns at :101-105 before its fetch, and read_votes filters bot-comments.json off disk at :167; this round adds a second local jq over the same file, no request. The reaction fetches at :191 are one per live dispute and the permission lookups at :200 one per distinct reactor rather than per reaction, so the request count is bounded by people, not clicks)

6. UI / UX — ✅ (the decision comment is the only user-facing surface and is unchanged this round; its closing paragraph at :135-143 covers all three ways it goes dead, the copy is sentence-case and names the two reactions it counts, and the review links the live one on every verdict including tie and pending. The new output this round is an Actions annotation, addressed to whoever opens the run rather than to the PR, which is where the finding asked for it: the review is written and published before post runs, so the model has no view of what post refused)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0 and functionsMeasured: 0, truncated: false — the diff contains no JS/TS, so no complexity or depth trigger exists this round and none was counted by eye; the decision rule is now five jq constants, each executed by both the workflow path and --self-check rather than duplicated for the tests, and this round's addition is defined in terms of an existing one instead of restating it)

8. Commit Hygiene — ✅ (one ci:-prefixed commit, 4cbc4ab, amended rather than followed by a fixup, which is what AGENTS.md:191 and :195-196 require; the guidelines, script, ci.yml and workflow edits are a single logical change that cannot land apart; the body gained a paragraph for this round's fix, naming both refusal cases and why the log is where they are named; 401 additions is inside the reviewable range for a change that is mostly one new file; no #N, owner/repo#N or closing keyword in the message, and the two PR references the author needs are in the PR body where they belong)

9. Tests — ✅ (nothing existing was removed or weakened; the self-check drives all five expressions the workflow executes — nine TALLY cases at :243-270, two NEWEST at :272-281, VOTABLE at :283-291, the new UNVOTABLE_IDS case at :293-301, two LIVE at :303-309 — and the step sits with the four sibling script checks at ci.yml:128, ahead of the install so a shell regression is not hidden behind a build failure)

10. Documentation — ✅ (the usage block at :13-17 still matches both entry points; the VOTABLE comment at :40-43 states why one definition serves both ends, :46-47 says why the id shape is part of votability, and :50-51 says why the complement is logged rather than dropped — the reason the finding gave, in the code that does it; the ponytail: note at :120-123 still names the cksum ceiling and its upgrade path)

11. Nitpicks / Optional — ✅ (nothing worth a line. Read and rejected: the marker string exists both as MARKER at :23 and as a literal in open_votes' regex at :78, which no self-check ties together, though a drift there fails loudly by counting nothing; a gh api failure between the comment POST at :146 and the reaction seeds at :150-151 leaves a comment whose text promises two reactions that are not on it, which needs an API failure inside a two-call window and costs a trip through the reaction picker; resolved_by will hold a string from a /resolve and a list from a vote, which nothing machine-reads; and a vote cast while the ledger is unreadable is gated: false, which the short-circuit rule deliberately refuses to let buy a run of its own — a compound corner whose other half, a lost ledger, is the larger pre-existing problem and not one this PR introduces)

Generated by Claude. This is advisory; a human reviewer must still approve.