Skip to content

Commit 9d417b2

Browse files
bpamiriclaude
andauthored
ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard (#2797)
* fix(ci): skip COMMENTED reviews in Reviewer A guard The post-submission guard in bot-review-a.yml scans for "bogus" wheels-bot reviews on the head SHA and dismisses any with a body shorter than 200 chars or missing the canonical `wheels-bot:review-a` marker. GitHub's dismiss API only accepts APPROVED or CHANGES_REQUESTED reviews; passing a COMMENTED review id returns HTTP 422 ("Can not dismiss a commented pull request review") and crashes the step, red-X'ing the Reviewer A check even when A's real substantive review landed cleanly seconds earlier. wheels-bot itself occasionally posts placeholder COMMENTED reviews mid- cycle as it probes the `gh pr review` CLI before issuing the real one (observed bodies: "placeholder test - ignore", "placeholder2 - updating", "test with dollar sign: see \$reincludeGlobals function"). Those leak out as public COMMENTED reviews, which is what the guard is meant to clean up — but it can't dismiss them via this API, so it has to skip them. Add `select(.state != "COMMENTED")` to the jq pipeline that selects actionable reviews. COMMENTED reviews don't gate merging anyway — only APPROVED and CHANGES_REQUESTED do — so leaving them in PR history is acceptable noise. The guard now only acts on what it can actually dismiss. Observed on PR #2795 commit 0db188a, job run 26296842347. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * ci: allow-list APPROVED + CHANGES_REQUESTED in Reviewer A guard Tighten the previous COMMENTED-state filter to an explicit allow-list: APPROVED and CHANGES_REQUESTED are the only review states that (a) GitHub's dismiss API accepts and (b) gate merging. Switching from "!= DISMISSED && != COMMENTED" to "state IN (APPROVED, CHANGES_REQUESTED)" also covers PENDING reviews, which return the same HTTP 422 from the dismiss endpoint if they ever land on a head SHA. Follow-up to bd76e53 per Reviewer A's allow-list suggestion on #2797. Reviewer B confirmed PENDING is reachable and the closed-set form is strictly safer than the deny-list. No behavioral change for the COMMENTED case fixed in bd76e53 — that state is still excluded. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c3c685d commit 9d417b2

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

.github/workflows/bot-review-a.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,16 @@ jobs:
143143
run: |
144144
set -euo pipefail
145145
146+
# Allow-list APPROVED and CHANGES_REQUESTED only: those are the
147+
# exclusive set of states that (a) GitHub's dismiss API accepts and
148+
# (b) gate merging. Feeding the dismiss API any other state returns
149+
# HTTP 422 — e.g. "Can not dismiss a commented pull request review"
150+
# for COMMENTED, the same shape for PENDING — which would fail this
151+
# step and red-X the check. Originally observed on PR #2795 commit
152+
# 0db188a5 when a COMMENTED placeholder leaked through.
146153
reviews=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \
147154
| jq -c --arg sha "$HEAD_SHA" \
148-
'[.[] | select(.user.login == "wheels-bot[bot]") | select(.commit_id == $sha) | select(.state != "DISMISSED")]')
155+
'[.[] | select(.user.login == "wheels-bot[bot]") | select(.commit_id == $sha) | select(.state == "APPROVED" or .state == "CHANGES_REQUESTED")]')
149156
150157
count=$(echo "$reviews" | jq 'length')
151158
if [[ "$count" == "0" ]]; then

0 commit comments

Comments
 (0)