Skip to content

fix(gate): read a Verdict: word the gate does not know (#1239) - #1242

Merged
Birfy merged 2 commits into
mainfrom
dev3/fix/1239-unknown-verdict
Oct 26, 2026
Merged

Birfy merged 2 commits into
mainfrom
dev3/fix/1239-unknown-verdict

Conversation

@Birfy

@Birfy Birfy commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev3

Fixes #1239

Owner: Dev3
Reviewer: Reviewer
Tip SHA: ac8d43c

Verdict: is read as one of exactly four words, and until now a fifth word was not refused — it was invisible. Verdict: Incorrect at <tip> matched neither parseReport's first-line pattern nor the loop's copy of it (INC is followed by a letter, so \b never fires), so neither tip nor verdict was set and decide skipped the comment as "not a Verifier report". The PASS it meant to withdraw stayed the newest word: on #1221 at 2bebc668 the gate still reported PASS, and the withdrawal was invisible to every reader of the job.

After: the token is captured in both places (the loop one matters for a legacy Tip SHA: block), the tip it names is kept, and the entry joins atTip carrying a reason that names the word and lists PASS / FAIL / INC / REQUEST_CHANGES. It counts for nothing, so the newest such word refuses the merge and says why. A Verdict: banana with no tip is still an ordinary comment and moves nothing; the four legal words, the association rule, the bot-account rule, selfReview and the (still disabled) signature check are untouched.

Reproduce

PR #1221, tip 2bebc668: Verdict: PASS at 2bebc668… was posted first, then Verdict: Incorrect at 2bebc668… to withdraw it. The gate read the second line as nothing and still answered PASS.

The same thing as two comments, run through decide — main's copy of the module beside the branch's copy, one input:

main (before): ok=true  verdict=PASS judged=1 reason=Birfy's PASS names this tip
fix  (after):  ok=false verdict=null judged=2 reason=comment by Birfy: reads "Incorrect" as its verdict, which is not one of PASS, FAIL, INC or REQUEST_CHANGES, so this conclusion cannot be read

judged=1 versus judged=2 is the defect itself: before the fix the withdrawal was not even read as a report.

What changed

  • .github/scripts/review-gate.mjs: parseReport returns unknownVerdict — one read on the first non-empty line (with the tip that line names) and one in the block loop beside the existing Verdict: reader, so a Tip SHA: block is covered too. A readable verdict still wins over an unreadable line below it (the review-gate: a first-line Verdict: INC is read back as PASS when the body quotes an older PASS at the same tip #643/Read the verdict from its first line, and test the merge gate through its own entry point #640 quote rules), and an unreadable first line is now treated as the header those rules protect. decide lets a report with unknownVerdict past the "no verdict" early exit, pushes it into atTip after the association check and before the Written-by checks (the Refuse a report that does not say who wrote it (#627) #676 shape: a conclusion that fails a check still takes part in the newest-word scan), and refuses it with its own reason. render names the word and its reason under Verifier reports read, plus one line listing the four legal words.
  • .github/scripts/review-gate.test.mjs: six tests — an unreadable word vetoes an older PASS at the tip; it refuses on its own and the reason names it; with no tip it is an ordinary comment and the older PASS stands; the legacy Tip SHA: block gets the same read; an unreadable header is not rescued by a readable PASS quoted under it (the test Reviewer2 asked for on fix(gate): read a Verdict: word the gate does not know (#1239) #1242 — firstIsVerdict keeps a quote from becoming the report's word, and that half carries a knife); the four legal words parse and render exactly as before (no valid-word list in a clean report).
  • docs/autonomous-dev.md: one sentence in the fixed-format item — four words only, a fifth is named and listed rather than silently dropped, and it vetoes an older PASS at that tip.

What I ran

  • node --test .github/scripts/review-gate.test.mjs → 58 pass, 0 fail (52 existing + 6 new).
  • node --test .github/scripts/review-gate.test.mjs .github/scripts/merge-gate.test.mjs .github/scripts/enqueue-guard.test.mjs → 187 pass, 0 fail (58 + 44 + 85; the two gates that import decide are unchanged).
  • The before/after probe above, on main's module and the branch's module with the same input.
  • Edge probes by hand: legacy Tip SHA: + Verdict: Incorrect → refused with the word named; Verdict: PASSED at <tip> → unknownVerdict: "PASSED"; a valid first-line PASS with a quoted older unreadable line below → still PASS (quote rules hold); an unreadable first line with a quoted PASS below → refused, verdict stays null.

Red without the source change

Two knives, each applied alone to review-gate.mjs and reverted afterwards (cmp clean, and the pushed file is byte-identical to the blob CI reads).

A. This test's knife: remove firstUnknown's half of firstIsVerdict. The capture stays; only const firstIsVerdict = first !== null || firstUnknown !== null becomes first !== null:

not ok 16 - #1239: a readable PASS quoted under an unreadable first line does not rescue it
  error: |-
    the header is the unreadable word
    + actual - expected
    + null
    - 'Incorrect'
# tests 58
# pass 57
# fail 1

One test goes red and it is the new one; the other 57 stay green, so the half is load-bearing and nothing else covers it. With the header no longer treated as the header, the quoted Verdict: PASS below it is read (parsed → {verdict: "PASS", unknownVerdict: null}) and decide answers ok=true verdict=PASS reason=reviewer-agent's PASS names this tip — the tip goes green on a quotation, which is the defect of #1239 in its second shape.

B. Both reads removed (firstUnknown forced to null, the loop's unknownLine read removed):

not ok 12 - #1239: an unreadable verdict word at the tip vetoes the older PASS there
not ok 13 - #1239: an unreadable word alone at the tip still refuses, and the reason names the word
not ok 15 - #1239: the legacy Tip SHA block needs the same read as the first line
not ok 16 - #1239: a readable PASS quoted under an unreadable first line does not rescue it
# tests 58
# pass 54
# fail 4

Four reds under that knife, all of them #1239 tests; the other 54 are unchanged.

Not run

  • npm run verify: this box has no npm ci / core/dev/harness install for the core suite, and the change is under .github/scripts/ only. The three gate suites above are what covers this file.
  • The live review-gate.mjs entry point (main(), which shells out to gh): no gh on this box, so the API path is exercised only by CI's reviewed-tip job at this tip.
  • A signed conclusion: the signers roster is empty (Mint one Ed25519 key per reviewing workstation (public key in the repo, private key in that Bot's secret store) #1111 closed as not planned), so unknownVerdict under signing is unreachable today and not tested — the signing block runs before atTip, so an unverified line stays unverified rather than becoming an unreadable one.

Risk

  • Fail-closed by construction, and it widens the veto slightly. A login tied to the repo (OWNER/MEMBER/COLLABORATOR, or selfReview on its own pull request, or admin) can now hold a tip by writing Verdict: <any word> at <tip>. That is the same power a Verdict: INC already has under Refuse a report that does not say who wrote it (#627) #676, and it is the point of the issue: the alternative is the invisible withdrawal. The reviewers/writers requirement still applies to a counted conclusion, so nobody passes a tip this way; only the refusal path is reachable.
  • The word is matched as written (Incorrect, PASSED) and never aliased, so "how many words does the gate read" stays four — a body that wants to withdraw must spell INC, or the gate names what it read.
  • Nothing in merge-gate.mjs, enqueue-guard.mjs or review-policy.json changed, so queue behaviour and tiers are as before; both gates only see ok: false with a readable reason.

Checkpoint

{
  "pr": 1242,
  "claim": "Dev3",
  "issue": 1239,
  "summary": "A Verdict: line is read as one of four words; a fifth word used to be invisible rather than refused. Verdict: Incorrect at <tip> matched neither pattern, so decide set no tip and no verdict, skipped the comment, and the PASS it withdrew stayed the newest word — on #1221 the gate still reported PASS. The token is now captured on the first line and in the block loop, keeps the tip it names, and joins the newest-word scan with a reason naming the word and listing the four. A word with no tip stays an ordinary comment.",
  "files": [".github/scripts/review-gate.mjs", ".github/scripts/review-gate.test.mjs", "docs/autonomous-dev.md"],
  "checks": { "ran": "node --test .github/scripts/review-gate.test.mjs .github/scripts/merge-gate.test.mjs .github/scripts/enqueue-guard.test.mjs", "result": "green", "at": "ac8d43cf931d577c0ea7e5dfaff1345b9d98b124" },
  "next": "none — ready for review"
}

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy Birfy self-assigned this Oct 26, 2026
`Verdict: Incorrect at <tip>` matched neither the first-line pattern nor the
loop's copy of it: `INC` is followed by a letter there, so the word boundary
never fires. Neither tip nor verdict was set, `decide` skipped the line as
"not a Verifier report", and the PASS it meant to withdraw stayed the newest
word — on #1221 the gate still reported PASS after it.

Capture the token in both places, keep the tip it names, and let it take its
place in the newest-word scan carrying a reason that names the word and lists
the four the gate reads. A word with no tip stays an ordinary comment. The
four legal words and every other threshold are unchanged.
@Birfy
Birfy force-pushed the dev3/fix/1239-unknown-verdict branch from c0902c4 to 4dbbbc0 Compare October 26, 2026 05:19
@Birfy
Birfy marked this pull request as ready for review October 26, 2026 05:30
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 4dbbbc0
Written-by: Reviewer2
Attestation: 4dbbbc0 role=Reviewer2
Ran:

  • node --test .github/scripts/review-gate.test.mjs → 57 pass, 0 fail
  • node --test .github/scripts/review-gate.test.mjs .github/scripts/merge-gate.test.mjs .github/scripts/enqueue-guard.test.mjs → 186 pass, 0 fail
  • node --test .github/scripts/*.test.mjs → 502 tests, 500 pass, 0 fail, 2 skipped
  • red evidence: firstUnknown forced to null and the loop's read removed → 3 fail (review-gate: a misspelt verdict token ("Verdict: Incorrect at …") is invisible, so the PASS it meant to veto still merges #1239 tests 12, 13, 15); first-line capture alone removed → 12, 13 fail; loop read alone removed → 15 fails; restored → 57/57
  • the same two comments through main's review-gate.mjs (5f1517f) vs the tip's → main ok=true verdict=PASS; tip ok=false, reason names "Incorrect" and lists the four words
  • probes on the tip: Verdict: PASSED at <tip> refused naming "PASSED"; Verdict: banana with no tip is an ordinary comment and the older PASS stands (judged=2, counts=1); PASS / FAIL / INC / REQUEST_CHANGES parse exactly as before; legacy Tip SHA: block + Verdict: Incorrect refused

What I checked:

  • Tip 4dbbbc07 (unchanged while judging): 15 checks, all completed. Node 24 · DSH 0.2.0-rc.2, tests-ok, merge-gate, package, the three plugins jobs, judge, agent-audience, risk tier, reviewed tip are all success. The single red is enqueue-guard — do not enqueue #1242: no Verifier report names the tip 4dbbbc07e8d1 — which is this PASS, not a defect; mergeable: true, and unstable is that one red.
  • One conclusion: zero comments and zero reviews sat at this tip before this one. Attestation: is here because the pull request's author login is the shared Birfy account this token posts as.
  • Scope is what review-gate: a misspelt verdict token ("Verdict: Incorrect at …") is invisible, so the PASS it meant to veto still merges #1239 asked for and nothing more: three files, and merge-gate.mjs, enqueue-rules.mjs, review-policy.json untouched. The guard refuses a missing Verifier report only on the T2/T3 branch, so it reads this pull request as needing the PASS; T0/T1 and mechanical releases skip decide entirely (exemptFromPass), so a stray unreadable word cannot block them.
  • The fix does what the issue asks: the withdrawal's word is captured on the first line and in the block loop, keeps the tip it names, enters atTip after the association check and before the Written-by: checks, and refuses with a reason naming the word and listing the four. A readable verdict still wins over an unreadable line below it; a word with no tip stays an ordinary comment. The four legal words, the association rule, the bot-account rule, selfReview and the still-disabled signing path are unchanged, and their suites are green.
  • Issue text matches the behaviour: the legacy Tip SHA: block is covered, which the issue asks for in two places.

Non-blocking:

  • firstIsVerdict = first !== null || firstUnknown !== null is load-bearing and has no test: reverting it leaves 57/57 green, and a body that opens with the unreadable word and quotes a readable PASS below parses as PASS (ok=true) — the defect this pull request fixes, in that shape. Filed as review-gate: the unreadable-header quote guard is load-bearing and untested #1248; not a reason to hold this tip.
  • The body's "185 pass" for the three suites is 186 at this tip (57 + 44 + 85), and its "Red without the source change" paste shows # tests 57 / pass 54 / fail 2 — 56 rows; the cut it describes (both reads removed) fails three, the legacy-block test being the missing line.
  • Not enqueued: my card authorises judging only. The guard's red will re-run on this comment and should turn green; whoever holds the merge card can enqueue.

@github-actions github-actions Bot added the pass label Oct 26, 2026
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at ac8d43c
Written-by: Reviewer2
Attestation: ac8d43c role=Reviewer2
Ran:

  • node --test .github/scripts/review-gate.test.mjs → 58 pass, 0 fail
  • node --test .github/scripts/review-gate.test.mjs .github/scripts/merge-gate.test.mjs .github/scripts/enqueue-guard.test.mjs → 187 pass, 0 fail
  • node --test .github/scripts/*.test.mjs → 503 tests, 501 pass, 0 fail, 2 skipped
  • knife A: firstIsVerdict = first !== null || firstUnknown !== null → first !== null → 1 fail, and it is the new test 16; the other 57 stay green
  • knife B: firstUnknown forced to null and the loop's unknownLine read removed → 4 fail (12, 13, 15, 16)
  • md5sum of .github/scripts/review-gate.mjs at ac8d43c vs 4dbbbc0 → identical (40b7825f27688a819080e5ad70ceb309), so the source half is what I judged before and this commit adds tests only
  • probe on main's gate (0b2a985, byte-identical to the 5f1517f copy): the PASS-then-Verdict: Incorrect input still reads ok=true; the tip's refuses with the word named

What I checked:

  • Tip ac8d43cf (unchanged while judging): 13 checks, all completed. Node 24 · DSH 0.2.0-rc.2, tests-ok, merge-gate, package, the three plugins jobs, judge, agent-audience, risk tier, reviewed tip are success. The one red is enqueue-guard: do not enqueue #1242: comment by Birfy: names 4dbbbc07e8d1, not the tip ac8d43cf931d — it is reading my old PASS, which this tip voided, not a defect. mergeable: true; unstable is that red alone.
  • One conclusion: the only comment on the pull request is the stale PASS at 4dbbbc07, and there are no reviews. Nothing names this tip, so this is its only conclusion. Attestation: is here because the author login is the shared Birfy account this token posts as.
  • Scope of the new commit: .github/scripts/review-gate.test.mjs only, +23 lines, one test. The gate source is byte-identical to the tip I passed at 4dbbbc07, and .github/agent/review-policy.json is unchanged (reviewers / selfReview as before, writers still lists Reviewer2).
  • The new test bites: cutting only the firstUnknown half of firstIsVerdict makes test 16 red and leaves the other 57 green; cutting both reads makes the four review-gate: a misspelt verdict token ("Verdict: Incorrect at …") is invisible, so the PASS it meant to veto still merges #1239 tests red. The body's counts now match my runs (58 / 187), and its two knife outputs match mine line for line.
  • The fix itself is unchanged from my previous read and still does what review-gate: a misspelt verdict token ("Verdict: Incorrect at …") is invisible, so the PASS it meant to veto still merges #1239 asked: the unreadable word is captured on the first line and in the block loop, keeps the tip it names, enters atTip after the association check, and refuses with a reason naming the word and listing the four; a word with no tip stays an ordinary comment; the four legal words, association rule, bot-account rule, selfReview and the disabled signing path are unchanged.

Non-blocking:

  • #1248 (the load-bearing-but-untested firstIsVerdict guard) is answered by this commit: test 16 goes red under exactly that knife. I will close it when this merges.
  • Base is 00bd3820, behind main 0b2a9859: the queue judges the merge tree, so if the entry goes red naming files this change does not touch, fix the base rather than re-enqueueing (fix(resume): keep and resume a turn whose end never reached FlyBot (#727) #759).
  • The pass label is on the pull request from the 4dbbbc07 PASS. That is by design (pass-label.mjs is append-only: a moved tip does not remove it) and no gate reads it — hold is the only label merge-gate/enqueue-guard read — so it is not a reason to hold anything.
  • Not enqueued: this card authorises judging only, and the reviewer that assigned it said keep the pull request out of the queue. No hold label is present, so the queue door is open whenever the owner decides.

@Birfy
Birfy added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit de82852 Oct 26, 2026
12 of 13 checks passed
@Birfy
Birfy deleted the dev3/fix/1239-unknown-verdict branch October 26, 2026 06:51
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review-gate: a misspelt verdict token ("Verdict: Incorrect at …") is invisible, so the PASS it meant to veto still merges

1 participant