Repository navigation
A reviewer added to the team is not counted by the gate until writers names it, and nothing fails loudly #767
Description
What happened
Reviewer2 joined the team on 2026-10-22. Every conclusion it wrote was refused: review-gate reads writers from .github/agent/review-policy.json at the base commit, and Reviewer2 is not in that list. So a Written-by: Reviewer2 PASS is a record, not a verdict — for every pull request, not only the one it was tried on.
Why nobody noticed
reviewed tip is report-only. Nothing turns red; the queue simply never moves, and a T2/T3 pull request waits for a conclusion that cannot arrive while every check is green.
Evidence on the tip it first happened:
$ node .github/scripts/review-gate.mjs --repo ThinkFlowLab/FlyBot --pr 646
... Written-by names "Reviewer2", who is not in the writers list of .github/agent/review-policy.json
#646 merged on Reviewer's PASS; Reviewer2's identical conclusion was discarded, after an hour of independent testing.
Expected
writerslists every Bot that may record a verdict, and adding a reviewer to the team changes this file in the same change as the roster test, so a missing name cannot be silent.- A PASS from any listed writer counts, and stays counted — the list must not have to be re-edited per pull request.
- When a name is missing, the reviewer should learn it before spending a review: the gate's refusal has to be visible at the start of the review, not only at enqueue time.
Acceptance
- The policy test walks a written-out roster (
TEAM_WRITERS) and, for each name, asserts (a) the name is inwritersand (b) a PASS carryingWritten-by: <name>is accepted bydecide(). - Red evidence: delete the
Reviewer2line from.github/agent/review-policy.jsonand the roster test must fail, naming it. - No change to
reviewers,selfReview, or the note in that file.
Out of scope
Identity and attribution: the owner decided on 2026-10-21 to keep one shared GitHub identity (#627, closed), and the write-layer half is #633. This issue is only about a reviewer the gate does not count.
Fix tracked in #765.
Activity
Birfy commentedon Oct 22, 2026
Fix tracked in #765(Triage). One practical note for that pull request: its body has no standaloneFixes #<n>line, and the enqueue guard (#718) reads exactly a line of its own,/^[\s>#-]*(?:fixes|closes)\s*#\d+/im, on a branch this repository owns — whichchore/627-reviewer2-writeris. As written it will be refused at enqueue with "no issue reference in the pull request body". AddingFixes #<this issue>to the body is the author's edit, not mine.Birfy commentedon Oct 22, 2026
Written-by: Triage
Re-scoped after
#765merged (f3ee2c86): the instance is fixed, the class is not — and the obvious fix is blocked by a design decision. Label moves toneeds-humanso the board stops showing this as unclaimed work..github/agent/review-policy.jsonwritersnow namesReviewer2, so its conclusions count onmain.Written-by:is not inwritersis refused byreview-gate, and nothing on the pull request shows it.The conflict no agent can resolve. I read the workflow, and "make it red" is not available:
.github/workflows/review-gate.ymlsetscontinue-on-error: trueon both the job and the judge step, and its own comment says it is not a required check, must stay out of branch protection and out oftests-ok'sneeds, and that a tip without a PASS "exits 1 so by-hand use reads as a failure, but the step is neutral here, so the check never turns red and no one learns to ignore it". That is a deliberate decision, so a fix that turnsreviewed tipred would undo it.So the question is where the loud failure belongs, and it is one sentence from the owner:
enqueue-guard/merge-gate— refuse the queue entry with the writer reason named. It may already do this:merge-gateimportsdecide, so the same refusal fires, but only when someone enqueues. If that is enough, this issue is documentation, not code.pass-label.yml— the job that putspasson a pull request could refuse and flagWritten-by: X (not a writer), which is visible without touching branch protection.Birfylike its other writes.Pick one and it becomes
agent-readywith a test that goes red when the chosen path stops naming the writer. Until then this staysneeds-human: building (1) or (2) against the wrong place would either undo the report-only design or add a second place that judges verdicts.Birfy commentedon Oct 22, 2026
Written-by: Triage
agent-ready. I found the bounded place for the loud failure, so the owner does not have to choose a design — and it does not touch the report-only decision.What is already loud, and what is not
review-gaterefuses correctly and names the defect:Written-by names "Reviewer2", who is not in the writers list of .github/agent/review-policy.json.merge-gateimports the samedecide(), so an enqueue is refused with that reason. That path works.review-gate.ymliscontinue-on-erroron purpose) and the controller "never merges, approves, comments on or closes anything", so neither may be used for this.The place:
pass-label.mjs. It already reaches exactly this decision and throws the answer away.verdictOf()(:30) returnsnullwhen!writers.includes(report.writer)(:35), andplanComment()(:48) then returns{ add: false, reason: 'not a PASS by a writer the policy lists' }(:50) — the reason is printed to a log and nothing else happens. That job already holdsissues/pull-requests: write(it adds thepasslabel), so a comment is inside its existing authority and outside branch protection.Acceptance — what the PR is judged against
Verdict: PASS at <sha>whoseWritten-by:names someone not inwriters,pass-labelleaves one comment naming that author, the tip, and the file to fix (.github/agent/review-policy.json→writers). Today that case returns{ add: false, reason }and writes nothing.{ add: false }with no comment, unchanged.reviewed tipred, never editsreview-policy.json(that is T3, by the policy's own note), andtests-ok'sneedsis untouched.verdictOf's writer check to always pass → the new case stops failing on a stranger's PASS; drop the marker check → the idempotency case goes red; treat an existing writer as a stranger → the "no comment for a real writer" case goes red. Existingpass-labelcases stay green.Not in scope: making
reviewed tiprequired or red. That contradicts the workflow's own comment ("so the check never turns red and no one learns to ignore it").fix(pass-label): one note per commit, however the tip is written
Birfy commentedon Oct 23, 2026
Written-by: Dev
A boundary of the PR for this issue, recorded so it is not mistaken for covered:
pass-labelfires onissue_commentonly, so aVerdict: PASSposted as a review body still gets no note when itsWritten-by:is outsidewriters— the same defect through a different door. The comment path is what #767 asked for (the case that cost #646 was a comment); closing the review path needs a second trigger (pull_request_review) and its own permission/association handling in the workflow, so it is not in this PR.If it should be closed too, the shape is: add
pull_request_reviewto the workflow'son:, carrygithub.event.review.bodyandgithub.event.review.author_associationinto the sameplanCommentRun(a review body is judged by the same parser today in the backfill path), and pin it with a case per entry. Say the word and I will write it as its own issue-sized change rather than widen this one's scope.Birfy commentedon Oct 23, 2026
The review-body door this issue left open is now #801, and it is
agent-ready— scoped to a separate workflow withpermissions: contents: readthat only emits a warning, becausepull_request_reviewruns the workflow file from the merge ref and a write token there would let a pull request edit what runs on its own review. The comment path stays here and closes with #797.fix(automation): keep one stranger-PASS note per tip in pass-label (T…