Skip to content

fix: shipping-pr declares PR clean before CodeRabbit review completes - #24

Merged
ether-moon merged 2 commits into
mainfrom
fix-coderabbit-review-polling
May 19, 2026
Merged

fix: shipping-pr declares PR clean before CodeRabbit review completes#24
ether-moon merged 2 commits into
mainfrom
fix-coderabbit-review-polling

Conversation

@ether-moon

@ether-moon ether-moon commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • shipping-pr could exit with a "clean" verdict before CodeRabbit finished reviewing a pushed fix, silently ending the review ping-pong one round short.
  • Root cause: Step 4 waited a fixed 10 min for CodeRabbit; real latency runs ~15 min (confirmed on a live PR — push→review took 14.5 min). On timeout, Step 7 saw no review comments and declared the PR clean.
  • Fix: poll the CodeRabbit commit status — a real per-commit completion signal CodeRabbit publishes itself — and never declare clean until that review is confirmed complete.

Changes

  • polling.md — Step 4 rewritten: adaptive poll of the CodeRabbit commit status (context == "CodeRabbit") on HEAD until it leaves pending; chunked re-entry under the Bash 10-min limit; emits REVIEW_VERIFIED.
  • blocker-resolution.md — Step 5 carries REVIEW_VERIFIED; Step 7 adds a fail-safe branch: when CodeRabbit is active and its review for HEAD has not completed, exit 1 "not verified" instead of 0 "clean".
  • SKILL.md--review-timeout default 10 → 30 min (safety cap, not the expected wait); examples, success criteria, and troubleshooting summary updated.
  • troubleshooting.md — replaced the obsolete commit_id-filter pitfall with "Treating a CodeRabbit review timeout as clean".
  • Added design spec at docs/superpowers/specs/2026-05-19-shipping-pr-coderabbit-review-gate-design.md.

Test Plan

  • jq status-filter unit tests across success/pending/failure/error/absent/empty/mixed inputs (7/7)
  • Filter verified against a live PR's real commits/{sha}/status response → success
  • bash -n syntax check on the new Step 4 and Step 7 snippets
  • Step 7 convergence truth-table (stuck / not-verified / clean) — 5/5
  • Real-world smoke test: run /skill-set:pr:ship on an actual PR with CodeRabbit enabled

Summary by CodeRabbit

  • Documentation
    • Extended CodeRabbit review timeout from 10 to 30 minutes to allow more time for review completion detection
    • Improved review gate logic to ensure incomplete reviews are not incorrectly marked as clean
    • Clarified documentation on review completion polling behavior and termination conditions
    • Enhanced troubleshooting guidance for handling CodeRabbit timeout scenarios

Review Change Stack

shipping-pr could declare a PR clean before CodeRabbit finished its
review. Step 4 used a fixed 10-min timeout shorter than CodeRabbit's
real latency (~15 min observed); on timeout Step 7 treated the absent
review as "nothing to fix" and exited clean.

Step 4 now polls the `CodeRabbit` commit status until it leaves
pending — a real per-commit completion signal instead of a fixed
guess. Step 7 refuses a clean verdict unless that review completed,
exiting "not verified" otherwise. --review-timeout default raised
10 -> 30 min as a safety cap; troubleshooting.md updated.
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d0752669-4e5b-4b26-985f-e3996bf106c8

📥 Commits

Reviewing files that changed from the base of the PR and between 0ef31be and bc74d94.

📒 Files selected for processing (5)
  • docs/superpowers/specs/2026-05-19-shipping-pr-coderabbit-review-gate-design.md
  • plugins/skill-set/skills/shipping-pr/SKILL.md
  • plugins/skill-set/skills/shipping-pr/reference/blocker-resolution.md
  • plugins/skill-set/skills/shipping-pr/reference/polling.md
  • plugins/skill-set/skills/shipping-pr/reference/troubleshooting.md

📝 Walkthrough

Walkthrough

The PR documents a fix to the shipping-pr skill's review-gate logic. Step 4 switches from a fixed 10-minute incremental-review wait to an adaptive CodeRabbit commit-status polling approach that detects true completion, Step 7 adds a fail-safe that prevents clean exits when CodeRabbit review never completes, and the default --review-timeout increases from 10 to 30 minutes. All changes are documentation and specification.

Changes

CodeRabbit Review Gate Fix

Layer / File(s) Summary
Design specification and problem analysis
docs/superpowers/specs/2026-05-19-shipping-pr-coderabbit-review-gate-design.md
New design spec document explaining the false-clean problem (Step 4 timeout before CodeRabbit review arrives causing Step 7 to incorrectly mark clean), identifies CodeRabbit commit-status as the reliable completion signal, and proposes adaptive polling with a 30-minute cap and a REVIEW_VERIFIED flag to gate Step 7's clean exit.
CodeRabbit commit-status polling (Step 4)
plugins/skill-set/skills/shipping-pr/reference/polling.md
Step 4 replaces fixed-timeout review-object polling with adaptive commit-status polling on TARGET_SHA for the CodeRabbit context. Implements chunked re-entry (9-minute headroom under 10-minute Bash limit) with persistent REVIEW_DEADLINE to enforce 30-minute overall cap, setting REVIEW_VERIFIED based on terminal-state detection.
State propagation and fail-safe logic (Steps 5–7)
plugins/skill-set/skills/shipping-pr/reference/blocker-resolution.md
Step 5 propagates REVIEW_VERIFIED unchanged as an unverified condition (not a hard blocker). Step 7 implements the fail-safe: when WAIT_CR==true and REVIEW_VERIFIED==false, exits non-clean (exit 1) with an explicit message indicating CodeRabbit review did not complete, preventing premature clean verdicts during pending review.
Configuration defaults and user documentation
plugins/skill-set/skills/shipping-pr/SKILL.md, plugins/skill-set/skills/shipping-pr/reference/troubleshooting.md
Updates --review-timeout default from 10 to 30 minutes, revises Step 4 description to "review-completion wait", tightens success criteria to forbid clean exits during pending CodeRabbit review, adds Common Mistakes guidance on timeout misdiagnosis, and revises troubleshooting.md to direct users to poll the CodeRabbit commit status and exit non-verified if still pending at timeout.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • ether-moon/skill-set#18: Introduced the initial shipping-pr skill workflow docs; this PR revises the core Step 4 polling semantics (commit-status terminal wait vs. incremental-review wait) and adds the Step 7 fail-safe using REVIEW_VERIFIED to fix the false-clean issue.

Poem

🐰 A review gate stands tall and wise,
No more false cleans beneath Bash skies,
CodeRabbit's heartbeat we now await,
Thirty minutes to verify the state,
Step 7 won't let pending slip by!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically summarizes the main problem being fixed: the shipping-pr skill incorrectly declares PRs clean before CodeRabbit review completes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-coderabbit-review-polling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ether-moon
ether-moon merged commit c4352cf into main May 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant