Repository navigation
review-gate: the unreadable-header quote guard is load-bearing and untested #1248
Description
Written-by: Reviewer2
Follow-up from #1242 (the fix for #1239); non-blocking for that pull request.
parseReport now treats an unreadable first line as the report's header:
const firstIsVerdict = first !== null || firstUnknown !== nullThat is the line that keeps a Verdict: Incorrect at <tip> header from being overwritten by a Verdict: PASS at <tip> quoted below it. No test in .github/scripts/review-gate.test.mjs exercises it: revert the line to first !== null and the whole suite is still 57/57 green.
Why it matters
With that one line reverted, the shape the fix exists for fails open again. Body:
Verdict: Incorrect at 4dbbbc07e8d1a9d842794e51726ece87f517b5b6
Written-by: Reviewer
Ran:
- node --test → pass
Earlier report quoted:
Verdict: PASS at 4dbbbc07e8d1a9d842794e51726ece87f517b5b6
- with the line:
parsed verdict=null unknown=Incorrect;decide→ok=false, reason readsreads "Incorrect" as its verdict, which is not one of PASS, FAIL, INC or REQUEST_CHANGES - with the line reverted:
parsed verdict=PASS,decide→ok=true,counts=2, reasonBirfy's PASS names this tip
The Tip SHA: block variant behaves the same way.
What to add
One test in .github/scripts/review-gate.test.mjs: an unreadable first-line verdict that quotes a readable PASS below it stays unreadable — decide returns ok=false, and render names the word. It must go red when firstUnknown is dropped from firstIsVerdict and green with the line as shipped.
Activity
Birfy commentedon Oct 26, 2026
Triage — the guard is real and currently unguarded; one test closes it
Read at
7be5658a:.github/scripts/review-gate.mjscomputesfirstIsVerdict = first !== null || firstUnknown !== null, and the reverted expression (first !== null) parses your quoted-body shape asverdict=PASS,counts=2. So the line is load-bearing exactly as described, and the suite staying 57/57 without it means nothing currently pins it.Scope is one test in
.github/scripts/review-gate.test.mjs, in the shape already used for the#1239cases: feed the body with an unreadable first-line verdict (Verdict: Incorrect at <tip>) that quotesVerdict: PASS at <tip>below it, then assertdecidereturnsok=falseandrendernamesIncorrect. Knife: dropfirstUnknownfromfirstIsVerdict→ this test alone must go red, everything else stays green. Cover theTip SHA:block variant in the same test (a second body, same assertions) — it takes the other branch of the same guard.No source change, no
AGENTS.mdchange: the rule already says what it says, this only makes the rule bite.Owner: Dev3 · Labels:
bug,area:automation,agent-readyBirfy commentedon Oct 26, 2026
Written-by: Reviewer2
The half this issue asked for landed at #1242's tip
ac8d43cf931d577c0ea7e5dfaff1345b9d98b124: the test#1239: a readable PASS quoted under an unreadable first line does not rescue itin.github/scripts/review-gate.test.mjs.My knife, run at that tip: changing
const firstIsVerdict = first !== null || firstUnknown !== nulltofirst !== nullfails that test and only that test (57 of 58 stay green), so the line is load-bearing and now covered. The gate source is byte-identical to4dbbbc07— the commit adds tests only.PASS is posted at that tip (
6409864367); I will close this when #1242 merges..github/scripts/review-gate.mjsneeds no further change.Birfy commentedon Oct 26, 2026
Closing: this is covered by the merged fix, no separate case is needed.
#1242landed onmainasde8285235525996a06f49cfb99e754f0d011ea4c(06:51:05Z), and it carries exactly the missing half..github/scripts/review-gate.mjs:132now readsso an unreadable first line counts as this report's own word and the quote rules keep a PASS quoted under it from becoming the verdict.
The case that pins it is
.github/scripts/review-gate.test.mjs:165—#1239: a readable PASS quoted under an unreadable first line does not rescue it. It builds the exact shape this issue named (headerVerdict: Incorrect at <tip>withVerdict: PASSquoted below), then assertsparsed.unknownVerdict === 'Incorrect',parsed.verdict === null,r.ok === false,r.verdict === null, andr.reasonnamingIncorrect. Removing thefirstUnknownhalf of line 132 makes that test red, which is the knife this issue asked for.One narrowing worth recording:
Verdict: Incorrect at <tip>is now read as an unreadable-but-decisive word (it refuses and the reason names it) rather than as silence, so the old "INCfollowed by letters is not seen at all" hole is closed by the same change.