t1382: Enforce wait-for-review-bots gate before PR merge#2739
t1382: Enforce wait-for-review-bots gate before PR merge#2739alex-solovyev merged 1 commit intomainfrom
Conversation
Add three-layer defense to prevent PRs from being merged before AI code review bots (CodeRabbit, Gemini Code Assist, etc.) post their findings: 1. CI workflow (.github/workflows/review-bot-gate.yml) — runs as a required status check, re-triggers on review/comment events 2. Helper script (review-bot-gate-helper.sh) — check/wait/list commands for agent and manual use 3. Agent rules in full-loop.md, pr-loop.md, pr.md, and build.txt — mandatory wait gate before merge step Process gap: PR #1 on aidevops-cloudron-app was merged before review bots posted, losing all security findings. Bots typically post within 2-5 minutes; the gate polls up to 10 minutes before timing out. Skip mechanism: add 'skip-review-gate' label for docs-only PRs or repos without review bots configured. Closes #2735
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 introduces a robust mechanism to prevent premature merging of pull requests, thereby safeguarding against the oversight of valuable AI code review bot feedback, particularly security-related findings. By integrating a multi-layered 'Review Bot Gate' into the CI/CD pipeline and agent workflows, the system now ensures that automated code analysis has completed and been considered before a PR can be finalized. This enhancement directly addresses a past incident where critical security insights were missed due to early merges. 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
|
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR introduces a comprehensive Review Bot Gate system to enforce waiting for AI review bots (CodeRabbit, Gemini Code Assist, Augment, Copilot) to comment on PRs before merging. It implements a three-layer gate via CI workflow, agent script helper, and branch protection guidance, with polling, skip-label bypass, and timeout handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub API
participant CI as CI: review-bot-gate.yml
participant Agent as Agent Script
participant Bots as Review Bots
Dev->>GH: Create/update PR
CI->>GH: Triggered: check for bot comments
CI->>GH: Scan PR reviews, comments
CI->>GH: Detect KNOWN_BOTS presence
alt Skip Label Present
CI->>GH: Return SKIP status
CI-->>Dev: Gate bypassed (docs-only)
else Bots Found
CI->>GH: Export gate_passed=true
CI-->>Dev: PASS - review findings
else No Bots
Agent->>GH: Begin polling (do_wait)
loop Poll every 60s until timeout
Agent->>GH: Check for bot comments
Bots-->>GH: Post review comments
Agent->>GH: Detect KNOWN_BOTS match
end
alt Timeout (10 min)
Agent-->>Dev: WAITING - proceed with warning
else Bot Posts Within Timeout
Agent-->>Dev: PASS - mandatory review required
end
end
Dev->>Dev: Review bot findings
Dev->>GH: Merge PR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: Mon Mar 2 23:13:29 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
Adding |
![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 crucial 'review bot gate' to prevent premature merging of pull requests before AI reviewers have posted their findings. However, a security audit identified a high-severity command injection vulnerability in review-bot-gate-helper.sh due to unsafe arithmetic expansion of user-controlled variables, and a medium-severity argument injection risk in the usage of the GitHub CLI. These issues could allow an attacker to execute arbitrary commands or bypass the intended security gate. Furthermore, the review-bot-gate-helper.sh script suppresses stderr on gh CLI calls, which can mask underlying issues and hinder debugging. Remediation for the security vulnerabilities involves strict input validation and the use of the -- separator for CLI commands, and the stderr suppression should be removed. The documentation updates are clear and comprehensive.
| while [[ "$elapsed" -lt "$max_wait" ]]; do | ||
| local result | ||
| result=$(do_check "$pr_number" "$repo" 2>/dev/null) || true | ||
|
|
||
| if [[ "$result" == "PASS" || "$result" == "SKIP" ]]; then | ||
| echo "$result" | ||
| return 0 | ||
| fi | ||
|
|
||
| echo "[${elapsed}s/${max_wait}s] Still waiting for review bots..." >&2 | ||
| sleep "$poll_interval" | ||
| elapsed=$((elapsed + poll_interval)) |
There was a problem hiding this comment.
This block contains a high-severity command injection vulnerability due to unsafe arithmetic expansion. Variables like max_wait or poll_interval, if derived from user-controlled input and containing command substitutions, could lead to arbitrary command execution within the [[ ... -lt ... ]] or $((...)) contexts. Additionally, on line 148, the stderr of the do_check function is being redirected to /dev/null. This suppresses diagnostic messages and potential gh command errors, making debugging difficult as the user only sees [Xs/Ys] Still waiting... without context. It is crucial to implement strict input validation for max_wait and poll_interval to prevent command injection, and to remove 2>/dev/null from the do_check call to improve error visibility.
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| reviews=$(gh api "repos/${repo}/pulls/${pr_number}/reviews" \ | ||
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") | ||
|
|
||
| # 2. Issue comments (some bots post as comments, not reviews) | ||
| local comments | ||
| comments=$(gh api "repos/${repo}/issues/${pr_number}/comments" \ | ||
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") | ||
|
|
||
| # 3. Review comments (inline code comments) | ||
| local review_comments | ||
| review_comments=$(gh api "repos/${repo}/pulls/${pr_number}/comments" \ | ||
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") |
There was a problem hiding this comment.
The gh api calls in this function use 2>/dev/null to suppress stderr. This is problematic as it can hide important errors like authentication failures, invalid repository or PR numbers, or network issues. If such an error occurs, the command will silently produce no output, and the script will incorrectly assume no comments or reviews exist.
This violates a general rule for this repository. Please remove 2>/dev/null from all three gh api calls in this function. The existing || echo "" is sufficient to handle cases where the API returns no items, preventing the script from exiting due to set -e.
| reviews=$(gh api "repos/${repo}/pulls/${pr_number}/reviews" \ | |
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") | |
| # 2. Issue comments (some bots post as comments, not reviews) | |
| local comments | |
| comments=$(gh api "repos/${repo}/issues/${pr_number}/comments" \ | |
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") | |
| # 3. Review comments (inline code comments) | |
| local review_comments | |
| review_comments=$(gh api "repos/${repo}/pulls/${pr_number}/comments" \ | |
| --paginate --jq '.[].user.login' 2>/dev/null || echo "") | |
| reviews=$(gh api "repos/${repo}/pulls/${pr_number}/reviews" \ | |
| --paginate --jq '.[].user.login' || echo "") | |
| # 2. Issue comments (some bots post as comments, not reviews) | |
| local comments | |
| comments=$(gh api "repos/${repo}/issues/${pr_number}/comments" \ | |
| --paginate --jq '.[].user.login' || echo "") | |
| # 3. Review comments (inline code comments) | |
| local review_comments | |
| review_comments=$(gh api "repos/${repo}/pulls/${pr_number}/comments" \ | |
| --paginate --jq '.[].user.login' || echo "") |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| labels=$(gh pr view "$pr_number" --repo "$repo" \ | ||
| --json labels -q '.labels[].name' 2>/dev/null || echo "") |
There was a problem hiding this comment.
This gh pr view command is potentially vulnerable to argument injection. If the pr_number or repo variables start with a hyphen (e.g., -Rattacker/repo), they might be interpreted as flags by the gh command, allowing an attacker to manipulate its behavior, such as redirecting the check to a different repository to bypass the review gate. Additionally, using 2>/dev/null here hides potential errors from the gh pr view command. If the command fails (e.g., due to authentication issues or an invalid PR number), this function will silently fail to retrieve labels and incorrectly determine that the skip-review-gate label is not present, potentially causing the script to wait unnecessarily. It is recommended to use the -- separator before user-controlled arguments to prevent argument injection and to remove the stderr redirection to allow errors to be visible for debugging.
| labels=$(gh pr view "$pr_number" --repo "$repo" \ | |
| --json labels -q '.labels[].name' 2>/dev/null || echo "") | |
| labels=$(gh pr view "$pr_number" --repo "$repo" \ | |
| --json labels -q '.labels[].name' || echo "") |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
|
|
||
| # Default repo from current git context | ||
| if [[ -z "$repo" ]]; then | ||
| repo=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || echo "") |
There was a problem hiding this comment.
The gh repo view command suppresses stderr with 2>/dev/null. While there is an error check immediately following this line, it's better to let gh print its own specific error message (e.g., 'not a git repository', 'no remote configured'). This provides more context to the user than the generic 'Could not determine repo' message.
Please remove 2>/dev/null to align with the project's general rules on error visibility.
| repo=$(gh repo view --json nameWithOwner -q .nameWithOwner 2>/dev/null || echo "") | |
| repo=$(gh repo view --json nameWithOwner -q .nameWithOwner || echo "") |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
1 participant
Summary
.github/workflows/review-bot-gate.yml) as a required status check, re-triggers on review/comment eventsreview-bot-gate-helper.sh) for check/wait/list commands for agent and manual usefull-loop.md,pr-loop.md,pr.md, andbuild.txt— mandatory wait gate before merge stepProcess gap: PR #1 on aidevops-cloudron-app was merged before review bots posted, losing all security findings. Bots typically post within 2-5 minutes; the gate polls up to 10 minutes before timing out.
Closes #2735
Summary by CodeRabbit
New Features
Documentation