t3005: Fix review-bot-gate to handle rate-limited bots via status check fallback#3006
Conversation
…e rate-limited When all review bots post rate-limit notices instead of real reviews, the gate now checks if any bot posted a formal GitHub commit status check with SUCCESS state (e.g., CodeRabbit's status context). If so, the PR is treated as reviewed — the bot completed its analysis even though the comment was a rate-limit notice. Both the workflow (.github/workflows/review-bot-gate.yml) and the helper script (.agents/scripts/review-bot-gate-helper.sh) are updated with the any_bot_has_success_status() fallback function. Closes #3005
|
Caution Review failedPull request was closed or merged during review WalkthroughAdded a new Changes
Sequence DiagramsequenceDiagram
participant WF as Workflow
participant Helper as Helper Script
participant GH as GitHub API
WF->>WF: Detect rate-limited bots in comments
alt Rate-Limited Bots Found
WF->>Helper: Call any_bot_has_success_status()
Helper->>GH: Query commit statuses (head SHA)
Helper->>GH: Query check-runs for known bots
alt SUCCESS Status Found
GH-->>Helper: Return SUCCESS conclusion
Helper-->>WF: Return 0 (success)
WF->>WF: Set status_fallback=true
WF->>WF: Treat PR as PASS (with fallback)
else No SUCCESS Status
GH-->>Helper: Return failure/pending
Helper-->>WF: Return 1 (failure)
WF->>WF: Fail gate with rate-limit error
end
else No Rate-Limited Bots
WF->>WF: Continue normal gate logic
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Mar 6 18:19:32 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fallback mechanism to check for bot success statuses when review comments indicate rate-limiting. The implementation is sound, and all original comments have been retained as they either align with the provided rules or address valid concerns not covered by the rules. My feedback includes suggestions to remove stderr suppression for better debuggability, optimize a loop for performance, and correct the handling of output streams.
| head_sha=$(gh pr view "$pr_number" --repo "$repo" \ | ||
| --json headRefOid -q '.headRefOid' 2>/dev/null || echo "") |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null can hide important errors from the gh command, such as authentication failures or invalid repository details, making debugging difficult. It's better to let stderr be printed so that underlying issues are visible. The || echo "" construct will still prevent the script from exiting on failure and ensure head_sha is empty.
| head_sha=$(gh pr view "$pr_number" --repo "$repo" \ | |
| --json headRefOid -q '.headRefOid' 2>/dev/null || echo "") | |
| head_sha=$(gh pr view "$pr_number" --repo "$repo" \ | |
| --json headRefOid -q '.headRefOid' || echo "") |
References
- Avoid blanket error suppression with
2>/dev/nullto ensure underlying errors like authentication or system issues remain visible for debugging.
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/statuses" \ | ||
| --paginate --jq '.[] | select(.state == "success") | .context' \ | ||
| 2>/dev/null || echo "") |
There was a problem hiding this comment.
As with the previous gh command, suppressing stderr with 2>/dev/null can hide important API errors (e.g., authentication, rate limiting, or an invalid SHA). Removing it will improve debuggability by making these errors visible, while || echo "" will still handle the failure case gracefully.
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/statuses" \ | |
| --paginate --jq '.[] | select(.state == "success") | .context' \ | |
| 2>/dev/null || echo "") | |
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/statuses" \ | |
| --paginate --jq '.[] | select(.state == "success") | .context' \ | |
| || echo "") |
References
- Avoid blanket error suppression with
2>/dev/nullto ensure underlying errors like authentication or system issues remain visible for debugging.
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/check-runs" \ | ||
| --paginate --jq '.check_runs[] | select(.conclusion == "success") | .name' \ | ||
| 2>/dev/null || echo "") |
There was a problem hiding this comment.
Similarly, 2>/dev/null should be removed here to avoid masking potential errors from the GitHub API when fetching check-runs. This ensures that any issues with the API call are visible for easier debugging.
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/check-runs" \ | |
| --paginate --jq '.check_runs[] | select(.conclusion == "success") | .name' \ | |
| 2>/dev/null || echo "") | |
| statuses=$(gh api "repos/${repo}/commits/${head_sha}/check-runs" \ | |
| --paginate --jq '.check_runs[] | select(.conclusion == "success") | .name' \ | |
| || echo "") |
References
- Avoid blanket error suppression with
2>/dev/nullto ensure underlying errors like authentication or system issues remain visible for debugging.
| local bot bot_base | ||
| for bot in "${KNOWN_BOTS[@]}"; do | ||
| bot_base=$(echo "$bot" | tr '[:upper:]' '[:lower:]') | ||
| if echo "$statuses_lower" | grep -qi "$bot_base"; then |
There was a problem hiding this comment.
Using echo and grep inside a for loop is inefficient as it spawns new processes for each iteration. You can achieve the same result more efficiently by using Bash's built-in string matching capabilities, which avoids the overhead of external processes.
| if echo "$statuses_lower" | grep -qi "$bot_base"; then | |
| if [[ "$statuses_lower" == *"$bot_base"* ]]; then |
| # Show status check fallback info | ||
| echo "" | ||
| echo "Status check fallback (GH#3005):" | ||
| if any_bot_has_success_status "$pr_number" "$repo" 2>&1; then |
There was a problem hiding this comment.
The 2>&1 redirection sends the diagnostic message from any_bot_has_success_status (which is printed to stderr) to stdout. This mixes diagnostic output with the formatted report from do_list, and the message will not be indented correctly. It's better to let the diagnostic message be printed to stderr as intended by removing 2>&1. The if statement will still correctly evaluate the function's exit code.
| if any_bot_has_success_status "$pr_number" "$repo" 2>&1; then | |
| if any_bot_has_success_status "$pr_number" "$repo"; then |
…w-bot-gate-helper Address PR #3006 review feedback (GH#3025): - Remove 2>/dev/null from gh pr view, gh api statuses, and gh api check-runs calls in any_bot_has_success_status() — the || echo "" fallback handles failures gracefully while letting auth/network errors remain visible for debugging - Remove 2>&1 from any_bot_has_success_status call in do_list() to prevent mixing diagnostic stderr with formatted report output - Finding #4 (echo|grep -> bash built-in) was already addressed in the merged PR via bidirectional [[ ]] matching
…w-bot-gate-helper (#3039) Address PR #3006 review feedback (GH#3025): - Remove 2>/dev/null from gh pr view, gh api statuses, and gh api check-runs calls in any_bot_has_success_status() — the || echo "" fallback handles failures gracefully while letting auth/network errors remain visible for debugging - Remove 2>&1 from any_bot_has_success_status call in do_list() to prevent mixing diagnostic stderr with formatted report output - Finding #4 (echo|grep -> bash built-in) was already addressed in the merged PR via bidirectional [[ ]] matching
1 participant
Summary
SUCCESSstatus check on the PR's head commit, the gate passes — the bot completed its analysis even though its comment was a rate-limit notice.github/workflows/review-bot-gate.yml) and the helper script (.agents/scripts/review-bot-gate-helper.sh) are updated with theany_bot_has_success_status()fallback functionHow it works
The new
any_bot_has_success_status()function:gh pr viewrepos/{owner}/{repo}/commits/{sha}/statusesfor success statusesrepos/{owner}/{repo}/commits/{sha}/check-runsfor success check runsGate logic order (updated)
Evidence
Wait for AI Review Bots= FAILURECloses #3005
Summary by CodeRabbit
Release Notes
New Features
Chores