What
Three places invoke the review gate and none of them states the condition an agent actually needs before merging. The shortest form is the one in the AGENTS.md commands block, which is the one that gets copied.
| Site |
Form |
What it accepts |
AGENTS.md:22 |
--strict-head-review |
any review at the head, including your own thread replies |
docs/AGENT_REVIEW_RECTIFY.md:134 |
--strict-head-review --required-review-author ... --wait-head-review-ms 180000 --allow-missing-head-review |
waits 3 min, then proceeds regardless |
.github/workflows/review-gate.yml:92 |
neither flag |
head-review check off entirely |
Why it matters
Replying to a review thread creates a review record attached to whatever the head is at that moment. So dispositioning the previous round of findings is by itself enough to satisfy --strict-head-review.
Observed on #210: the gate passed two minutes after a push. Five reviews existed at the head — all of them replies posted while dispositioning the previous round. The only genuine reviewer's latest review was on the previous head. Adding --required-review-author made the same gate correctly report No review was found for current head.
The perverse property is that the tighter the review discipline, the more reliably the condition is met by the person being reviewed.
Proposed
AGENTS.md:22 carries the full form: --strict-head-review --required-review-author chatgpt-codex-connector.docs/AGENT_REVIEW_RECTIFY.md:134 drops --allow-missing-head-review; a timeout there should fail rather than proceed, since the whole point of --wait-head-review-ms is to wait for the thing the waiver then ignores.- Consider defaulting
--required-review-author to DEFAULT_PR_FINDING_AUTHOR when --strict-head-review is passed, so the safe form is the short one. The script already computes prFindingAuthor that way for findings; the head-review check is the one place it does not.
The CI workflow is a separate question and probably correct as-is — a status check cannot block waiting on an asynchronous reviewer, and branch protection pairs it with native conversation resolution.
Not in scope
Loosening any existing check. This is about a condition that is weaker than it reads, not about a condition that is too strict.
What
Three places invoke the review gate and none of them states the condition an agent actually needs before merging. The shortest form is the one in the
AGENTS.mdcommands block, which is the one that gets copied.AGENTS.md:22--strict-head-reviewdocs/AGENT_REVIEW_RECTIFY.md:134--strict-head-review --required-review-author ... --wait-head-review-ms 180000 --allow-missing-head-review.github/workflows/review-gate.yml:92Why it matters
Replying to a review thread creates a review record attached to whatever the head is at that moment. So dispositioning the previous round of findings is by itself enough to satisfy
--strict-head-review.Observed on #210: the gate passed two minutes after a push. Five reviews existed at the head — all of them replies posted while dispositioning the previous round. The only genuine reviewer's latest review was on the previous head. Adding
--required-review-authormade the same gate correctly reportNo review was found for current head.The perverse property is that the tighter the review discipline, the more reliably the condition is met by the person being reviewed.
Proposed
AGENTS.md:22carries the full form:--strict-head-review --required-review-author chatgpt-codex-connector.docs/AGENT_REVIEW_RECTIFY.md:134drops--allow-missing-head-review; a timeout there should fail rather than proceed, since the whole point of--wait-head-review-msis to wait for the thing the waiver then ignores.--required-review-authortoDEFAULT_PR_FINDING_AUTHORwhen--strict-head-reviewis passed, so the safe form is the short one. The script already computesprFindingAuthorthat way for findings; the head-review check is the one place it does not.The CI workflow is a separate question and probably correct as-is — a status check cannot block waiting on an asynchronous reviewer, and branch protection pairs it with native conversation resolution.
Not in scope
Loosening any existing check. This is about a condition that is weaker than it reads, not about a condition that is too strict.