From d49a5746363c1b8a25feabf51c348557a749cabb Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:18:21 +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 pull-request skill's monitor loop could enter (or continue) the CodeRabbit/threads wait while the PR was still draft, deadlocking silently — observed 2026-07-16: a converging PR sat in draft with CI green and the CodeRabbit check pending indefinitely, until manually un-drafted. - SKILL.md §5: emphasize the ordering invariant — `gh pr ready` MUST run as soon as draft CI is green, before section 6 starts waiting on any review signal. - SKILL.md §6 / monitor-loop.md: add a belt-and-braces draft guard at the top of every poll pass — if `isDraft: true` and CI is green, flip to ready immediately. Covers a loop started before the flip, or a rebase/force-push that reverted the PR back to draft. Closes #3956 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- .claude/skills/pull-request/SKILL.md | 10 ++++++++-- .../pull-request/references/monitor-loop.md | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.claude/skills/pull-request/SKILL.md b/.claude/skills/pull-request/SKILL.md index 2b10de815..4694ab6ed 100644 --- a/.claude/skills/pull-request/SKILL.md +++ b/.claude/skills/pull-request/SKILL.md @@ -97,13 +97,15 @@ 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: +**Ordering invariant — as soon as draft CI is green, flip to ready before doing anything else:** ```bash gh pr ready ``` -> Some bots (e.g. CodeRabbit) trigger on ready, not on CI completion. After converting, do a **preliminary review pass** before entering the main loop: +CodeRabbit (and some other review bots) never review a draft PR — they trigger on ready, not on CI completion. Entering or continuing the CodeRabbit/threads wait while the PR is still draft is a guaranteed silent deadlock (observed 2026-07-16: a converging PR sat in draft, CI green, CodeRabbit check pending indefinitely, until manually un-drafted). `gh pr ready` MUST run before section 6 starts waiting on any review signal — never after. + +> After converting, do a **preliminary review pass** before entering the main loop: > > ```bash > sleep 180 @@ -126,6 +128,10 @@ PR= ``` Per pass: +0. Draft guard — if `isDraft: true` AND CI green, `gh pr ready` immediately, + then continue the pass. CodeRabbit never reviews a draft; this catches a + loop that started before the 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 b03739905..5cca71b8e 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -19,6 +19,7 @@ After `gh pr ready`, run this loop yourself — do not wait for the user. consecutive_zero = 0 REPEAT: + 0. Draft guard → if PR is still draft AND CI is green, flip to ready now (see 6-guard) 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 → STATUS=$(gh pr view $PR --json mergeable --jq .mergeable) @@ -34,6 +35,25 @@ REPEAT: else GOTO 3 ``` +## 6-guard. Draft ordering guard (belt-and-braces) + +CodeRabbit never reviews a draft PR. Section 5 already flips the PR to ready +before this loop starts, but run this check at the top of **every** pass +anyway — it covers a loop entered before that flip, or a rebase/force-push +that reverted the PR back to draft: + +```bash +STATUS=$(gh pr view "$PR" --json isDraft,statusCheckRollup) +IS_DRAFT=$(echo "$STATUS" | jq -r '.isDraft') +CI_GREEN=$(echo "$STATUS" | jq -r '[.statusCheckRollup[] | (.conclusion // .state)] | all(. == "SUCCESS")') + +if [ "$IS_DRAFT" = "true" ] && [ "$CI_GREEN" = "true" ]; then + gh pr ready "$PR" +fi +``` + +If still draft with CI red, do nothing — wait for CI to go green first (step 1), then this check flips it on the next pass. + ## 6a. Wait for CI After any push, wait 30s then watch: From f187af196f66b5cd4aec8947c9cd4d023810ae71 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:26:48 +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 5cca71b8e..35d33d7b5 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -45,7 +45,9 @@ that reverted the PR back to draft: ```bash STATUS=$(gh pr view "$PR" --json isDraft,statusCheckRollup) IS_DRAFT=$(echo "$STATUS" | jq -r '.isDraft') -CI_GREEN=$(echo "$STATUS" | jq -r '[.statusCheckRollup[] | (.conclusion // .state)] | all(. == "SUCCESS")') +# 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 "$STATUS" | 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 5c705cce4a7451e0a500e2f75c93679ff94937ce Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 17 Jul 2026 10:31:03 +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 | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/.claude/skills/pull-request/references/monitor-loop.md b/.claude/skills/pull-request/references/monitor-loop.md index 35d33d7b5..8cd8f12ed 100644 --- a/.claude/skills/pull-request/references/monitor-loop.md +++ b/.claude/skills/pull-request/references/monitor-loop.md @@ -22,6 +22,9 @@ REPEAT: 0. Draft guard → if PR is still draft AND CI is green, flip to ready now (see 6-guard) 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. CI just turned green → re-run the draft guard now, before proceeding (see 6-guard) — + catches a pass that started draft+CI-red so the PR doesn't + stay draft into the review wait below 2b. Check mergeable status → STATUS=$(gh pr view $PR --json mergeable --jq .mergeable) if STATUS == "UNKNOWN" → sleep 10, retry up to 3 times if STATUS == "CONFLICTING" → report to user and STOP @@ -38,9 +41,10 @@ REPEAT: ## 6-guard. Draft ordering guard (belt-and-braces) CodeRabbit never reviews a draft PR. Section 5 already flips the PR to ready -before this loop starts, but run this check at the top of **every** pass -anyway — it covers a loop entered before that flip, or a rebase/force-push -that reverted the PR back to draft: +before this loop starts, but run this check at the top of **every** pass — +and again the moment CI turns green mid-pass (step 2a) — anyway: it covers a +loop entered before that flip, a rebase/force-push that reverted the PR back +to draft, or a pass that started draft with CI red: ```bash STATUS=$(gh pr view "$PR" --json isDraft,statusCheckRollup) @@ -50,11 +54,21 @@ IS_DRAFT=$(echo "$STATUS" | jq -r '.isDraft') CI_GREEN=$(echo "$STATUS" | 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" + gh pr ready "$PR" || { sleep 5; gh pr ready "$PR"; } || { + echo "ERROR: gh pr ready failed twice — PR still draft, cannot proceed to review wait." >&2 + exit 1 + } fi ``` -If still draft with CI red, do nothing — wait for CI to go green first (step 1), then this check flips it on the next pass. +If `gh pr ready` fails, retry once after a short sleep; if the retry also +fails, stop here — do not fall through to the review wait with the PR still +draft (that deadlocks on CodeRabbit, which never reviews a draft). + +If still draft with CI red, do nothing — wait for CI to go green first (step +1). The instant CI turns green, step 2a re-runs this same guard within the +same pass (not "next pass") so a still-draft PR flips ready before +mergeability, grace period, or review-wait ever run. ## 6a. Wait for CI