From 140063bec142036b75a4f7017005f90bd10c0c8e Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:18:40 +0200 Subject: [PATCH 1/3] fix(skills): flip PR to ready before waiting on CodeRabbit to avoid a draft deadlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit never reviews a draft PR. The monitor loop could enter or continue its CodeRabbit/threads wait while the PR was still draft, deadlocking silently (observed 2026-07-16). Adds a hard ordering invariant in §5 (flip ready as soon as draft CI is green, before any wait), plus a belt-and-braces re-check at the top of every monitor loop pass: if still isDraft with CI green, run `gh pr ready` again — covers a loop started pre-flip or a rebase/force-push that reverted the PR to draft. Closes #4450 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- .claude/skills/pull-request/SKILL.md | 10 +++++++- .../pull-request/references/monitor-loop.md | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/.claude/skills/pull-request/SKILL.md b/.claude/skills/pull-request/SKILL.md index fc0217e5b..967d6b997 100644 --- a/.claude/skills/pull-request/SKILL.md +++ b/.claude/skills/pull-request/SKILL.md @@ -97,12 +97,14 @@ PR title must follow `type(scope): description` (conventional commits). Link the - Checkbox sections (Validation, Guardrails): check each box that applies (`- [x]`), leave unchecked only what genuinely does not apply - Follow any instructions in the template (e.g. "Delete this section if not applicable") -Once **draft CI passes** and the PR is ready for human review, convert to ready: +Once **draft CI passes**, convert to ready **immediately, before anything else**: ```bash gh pr ready ``` +> **Ordering invariant (hard requirement):** CodeRabbit never reviews a draft PR. Flipping to ready is a mandatory prerequisite for entering — or continuing — any wait on CodeRabbit/review threads. Do it the moment draft CI is green, BEFORE starting section 6. Waiting on CodeRabbit while the PR is still draft is a guaranteed silent deadlock (observed 2026-07-16, issue #4450). +> > Some bots (e.g. CodeRabbit) trigger on ready, not on CI completion. After converting, do a **preliminary review pass** before entering the main loop: > > ```bash @@ -115,6 +117,9 @@ gh pr ready ## 6. Monitor loop (autonomous) +The PR must already be ready (see §5) before this loop starts — CodeRabbit +never reviews a draft, so entering this loop while still draft deadlocks. + After `gh pr ready`, run an autonomous polling loop until either CI is green with zero unresolved threads for 3 consecutive passes (~9 min) or the safety limit (10 iterations) trips. @@ -126,6 +131,9 @@ PR= ``` Per pass: +0. **Draft guard** (belt-and-braces) → if still `isDraft: true` and CI is + green, `gh pr ready "$PR"` immediately — covers a loop that started + pre-flip, or a rebase/force-push that reverted the PR to draft 1. Wait CI → fix + /verify + commit + push if red, else continue 2. Check mergeable — `CONFLICTING` stops, `UNKNOWN` retries 3. Grace `sleep 180` + adaptive recheck of pending review checks diff --git a/.claude/skills/pull-request/references/monitor-loop.md b/.claude/skills/pull-request/references/monitor-loop.md index 9e1e58b2d..821fdde6f 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -13,12 +13,18 @@ PR= After `gh pr ready`, run this loop yourself — do not wait for the user. +**Ordering invariant:** the PR must be ready (not draft) before this loop +starts — CodeRabbit never reviews a draft PR, so waiting on it while draft is +a guaranteed silent deadlock. Step 0 below is a defensive re-check, not a +substitute for flipping to ready in §5 before entering the loop. + ## Loop procedure ```text consecutive_zero = 0 REPEAT: + 0. Draft guard (belt-and-braces) → see 6a-0. If still draft with CI green, flip to ready now. 1. Wait for CI → sleep 30 then gh pr checks "$PR" --watch 2. If CI fails → fix, /verify, commit, push, consecutive_zero=0, GOTO 1 2b. Check mergeable status → gh pr view "$PR" --json mergeable --jq .mergeable @@ -34,6 +40,25 @@ REPEAT: else GOTO 3 ``` +## 6a-0. Draft guard (belt-and-braces) + +Run at the top of **every** pass, before waiting on CI or reading threads. +Covers the race where the loop started before the ready-flip landed, or a +rebase/force-push reverted the PR to draft: + +```bash +STATE=$(gh pr view "$PR" --json isDraft,statusCheckRollup) +IS_DRAFT=$(echo "$STATE" | jq -r '.isDraft') +CI_GREEN=$(echo "$STATE" | jq -e '[.statusCheckRollup[].conclusion] | all(. == "SUCCESS" or . == "NEUTRAL" or . == "SKIPPED")' >/dev/null 2>&1 && echo true || echo false) + +if [ "$IS_DRAFT" = "true" ] && [ "$CI_GREEN" = "true" ]; then + gh pr ready "$PR" +fi +``` + +If still draft with CI **not** green, do nothing here — fall through to 6a, +fix CI first, and this check runs again next pass. + ## 6a. Wait for CI After any push, wait 30s then watch: From d10b42b1bea1ad4b20ac2af8abd4c0623703d5d4 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:28:50 +0200 Subject: [PATCH 2/3] fix(skills): make the draft-guard CI-green check reject empty rollups and accept neutral/skipped Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- .claude/skills/pull-request/references/monitor-loop.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/skills/pull-request/references/monitor-loop.md b/.claude/skills/pull-request/references/monitor-loop.md index 821fdde6f..acf9d5035 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -49,7 +49,9 @@ rebase/force-push reverted the PR to draft: ```bash STATE=$(gh pr view "$PR" --json isDraft,statusCheckRollup) IS_DRAFT=$(echo "$STATE" | jq -r '.isDraft') -CI_GREEN=$(echo "$STATE" | jq -e '[.statusCheckRollup[].conclusion] | all(. == "SUCCESS" or . == "NEUTRAL" or . == "SKIPPED")' >/dev/null 2>&1 && echo true || echo false) +# green = rollup non-empty (an empty rollup right after a force-push/rebase must NOT read as green) AND +# every check is SUCCESS/NEUTRAL/SKIPPED (all non-blocking; a bare FAILURE/CANCELLED or still-pending check stays not-green) +CI_GREEN=$(echo "$STATE" | jq -r '[.statusCheckRollup[]? | (.conclusion // .state)] | length > 0 and all(. as $s | ["SUCCESS","NEUTRAL","SKIPPED"] | index($s) != null)') if [ "$IS_DRAFT" = "true" ] && [ "$CI_GREEN" = "true" ]; then gh pr ready "$PR" From ed7180b98abe7beb4e6ecc5176b22dcba39f9bbd Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:32:41 +0200 Subject: [PATCH 3/3] fix(skills): handle gh pr ready failure and re-run the draft guard after CI goes green Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- .../pull-request/references/monitor-loop.md | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.claude/skills/pull-request/references/monitor-loop.md b/.claude/skills/pull-request/references/monitor-loop.md index acf9d5035..c5073a046 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -27,6 +27,10 @@ REPEAT: 0. Draft guard (belt-and-braces) → see 6a-0. If still draft with CI green, flip to ready now. 1. Wait for CI → sleep 30 then gh pr checks "$PR" --watch 2. If CI fails → fix, /verify, commit, push, consecutive_zero=0, GOTO 1 + 2a. Re-run draft guard → see 6a-0. CI is confirmed green here (whether it started + green or just turned green in step 1) — re-checking closes + the gap where a pass starting draft+red would otherwise sail + straight through to mergeability/grace/review-wait still draft. 2b. Check mergeable status → gh pr view "$PR" --json mergeable --jq .mergeable if "CONFLICTING" → report to user and STOP if "UNKNOWN" → sleep 30, retry (up to 3 times), then proceed @@ -54,12 +58,24 @@ IS_DRAFT=$(echo "$STATE" | jq -r '.isDraft') CI_GREEN=$(echo "$STATE" | jq -r '[.statusCheckRollup[]? | (.conclusion // .state)] | length > 0 and all(. as $s | ["SUCCESS","NEUTRAL","SKIPPED"] | index($s) != null)') if [ "$IS_DRAFT" = "true" ] && [ "$CI_GREEN" = "true" ]; then - gh pr ready "$PR" + if ! gh pr ready "$PR"; then + sleep 5 + if ! gh pr ready "$PR"; then + echo "gh pr ready failed twice on PR $PR — stopping to avoid a draft deadlock." >&2 + exit 1 + fi + fi fi ``` +If `gh pr ready` fails, retry once (5s later); if it still fails, stop and +report to user — never fall through to mergeability/grace/review-wait with +the PR still draft. Continue only once the PR is confirmed non-draft. + If still draft with CI **not** green, do nothing here — fall through to 6a, -fix CI first, and this check runs again next pass. +fix CI first. Once CI turns green (either at pass start or mid-pass in 6a), +this guard re-runs at step 2a before mergeability/grace/review-wait — never +"next pass." ## 6a. Wait for CI