Repository navigation
no-push-without-self-review: a heading-then-separate-line Verdict format parses as no verdict, masking a stale held value as a same-turn lag #2444
Description
Important
The original diagnosis in this issue was wrong. It is preserved below for the record. The corrected account is in the first section.
Corrected diagnosis
hooks/no-push-without-self-review.py refused a push citing a blocking verdict while the most recent dispatch had returned a clean one. The cause is a report-format gap, not transcript lag.
VERDICT_LINE requires the literal Verdict: label on the same line as the phrase:
VERDICT_LINE = re.compile(
r"^[ \t]{0,3}(?:#{1,6}[ \t]*)?Verdict[ \t]*:[ \t]*(?:\*\*)?"
r"(Ready for merge|Needs (?:more )?work)\b",
re.I | re.M,
)A report ending
## Verdict
Ready for merge
Reviewed-Commit: <sha>
therefore parses as no verdict at all. parse_report returns (None, None), read_latest_review does not advance its held value, and the guard keeps reporting an older needs_work. Four consecutive clean reviews were discarded this way.
The dispatcher writes the report template into the review brief, so the brief is where this is fixed: require Verdict: <phrase> on one line.
A second cause, in the same evidence
The four dispatches spanned two branches, which git merge-base --is-ancestor settles:
5a45e7fd,629cca1b->fix/2409-driver-comments446fa0ee,cf08d05f->ums-2409-fail-open-lessons
read_latest_review holds one global (verdict, reviewed_commit) pair, not one per branch. So a review of either branch displaces the other's regardless of format, and a session driving two branches must review and push them in sequence. Fixing the format alone would not have made those pushes independent.
That second cause is why this issue stays open. The format gap is the answer to "why was the verdict discarded"; the global held value is the answer to "why does a two-branch session serialize", and nothing has been changed for it.
What the transcript reader actually does
It does lag within a turn, by seconds, and it does catch up. That is real and it is not what caused this. The lag theory was reached by reasoning about session mechanics; the format gap was found by running read_latest_review and printing parse_report's result per admitted result, which is the check that distinguishes them and which should have come first.
Original report (superseded --- the transcript-lag theory below is wrong)
The original body attributed the refusal to the transcript not being flushed for same-turn tool results, and its closing section asserted "Not a defect in / The verdict parsing itself. parse_report read all four reports correctly; only their visibility was wrong." Both claims are false: parse_report returned no verdict for two of the four reports, and re-running the reader minutes apart returned the same stale value rather than resolving.
Posted by Claude Code (AI agent) --- not written by a human.
Activity
d-morrison commentedon Aug 27, 2026
Corrected diagnosis
The transcript-lag theory in the issue body is wrong. Re-running
read_latest_reviewagainst the same transcript twice, minutes apart, returned the identical('needs_work', '446fa0eecaec6a58e2d65e1b8d24265e67e13138', True)both times -- a lag that would resolve on its own would not reproduce identically across two reads separated by real wall-clock time.The actual cause: the two later, clean dispatches wrote their verdict as
VERDICT_LINErequires the verdict phrase on the same line asVerdict:, optionally heading-prefixed (### Verdict: Ready for merge). A heading naming the section with the phrase on the following line does not match. Confirmed independently:So
parse_reportreturned no verdict for both later dispatches, andread_latest_review's held(verdict, reviewed_commit)pair never advanced past the second dispatch'sneeds_work. This is not a defect in the guard:VERDICT_LINE's same-line requirement is the documented, deliberate anchoring described in the guard's own docstring (no-push-without-self-review.py's "WHAT it said" section), and.claude/agents/adversarial-reviewer.mdalready specifies the one-line form for its own persona. The gap was a review brief that didn't repeat that requirement for a non-persona dispatch.What this changes
The "possible directions" in the issue body (distinguish no-verdict from blocking-verdict in the refusal message, or accept an in-flight-turn verdict) don't apply -- there was no lag to accommodate. The third direction, "document the constraint," is what actually fixes this, and it's documentation rather than code:
shared/workflow/adversarial-self-review.mdnow has a "A verdict phrase separated from its heading by a line break is no verdict" section, andmemories/mistake-patterns.mdPattern 17 records this as a case: verify a guard refusal by running its own reader over the transcript before theorizing about a cause like lag.Closing as resolved by documentation -- no code change needed, since the parser is behaving correctly per its own contract.
Posted by Claude Code (AI agent) --- not written by a human.
[-]no-push-without-self-review: a verdict returned in the SAME turn is invisible, so dispatch-then-push in one turn always blocks[/-][+]no-push-without-self-review: a heading-then-separate-line Verdict format parses as no verdict, masking a stale held value as a same-turn lag[/+]ums: same-line verdict parsing, backtick-delimiter generalization, me…
fix(review): close structured-payload fail-opens found in post-merge …
feat(review): support structured review-bot output in check-pr-fully-…