ci: require an approving review for non-self-authored PRs - #241
Conversation
No native branch-protection/ruleset feature does this -- bypass lists are actor-based (whoever merges bypasses), not author-based, so a bypass for getappz would let getappz merge ANYONE's PR without review, not just their own. Reimplemented as a required status check instead: trusted authors (TRUSTED_PR_AUTHORS, default getappz) skip the requirement (avoids the self-approval deadlock GitHub enforces platform-wide), everyone else -- Devin, any future external contributor -- needs an actual approving review before merge.
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow that runs on pull request and review events, allows configured trusted authors to bypass approval, and fails for other authors until an approving review exists. ChangesReview Enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/require-review-for-others.yml:
- Around line 46-48: Update the approval check in the workflow’s gh api
invocation to paginate all pull-request reviews, restrict approvals to reviews
targeting the current head commit, and evaluate each reviewer’s most recent
review state so stale approvals cannot satisfy the requirement. Preserve the
existing conditional that proceeds only when at least one current approving
reviewer remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 37eb4332-a531-4c2e-be8f-145b03152072
📒 Files selected for processing (1)
.github/workflows/require-review-for-others.yml
| approvals=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" \ | ||
| --jq '[.[] | select(.state=="APPROVED")] | length') | ||
| if [ "$approvals" -gt 0 ]; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files .github/workflows/require-review-for-others.yml
echo "----"
wc -l .github/workflows/require-review-for-others.yml
echo "----"
cat -n .github/workflows/require-review-for-others.yml | sed -n '1,140p'Repository: getappz/agentflare
Length of output: 2610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow logic and any local usage of the Reviews API or review state handling.
rg -n "pulls/.*/reviews|APPROVED|submitted_at|commit_id|head.sha|per_page=100|gh api --paginate" .github/workflows -SRepository: getappz/agentflare
Length of output: 411
Require a current approving review, not any historical one. .github/workflows/require-review-for-others.yml:46-48 counts every APPROVED review object, so an older approval can still pass after new commits, and this call only reads the first reviews page. Filter to the current head.sha, paginate, and evaluate each reviewer’s latest state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/require-review-for-others.yml around lines 46 - 48, Update
the approval check in the workflow’s gh api invocation to paginate all
pull-request reviews, restrict approvals to reviews targeting the current head
commit, and evaluate each reviewer’s most recent review state so stale approvals
cannot satisfy the requirement. Preserve the existing conditional that proceeds
only when at least one current approving reviewer remains.
|
Simplifying: folding this into auto-merge.yml's existing author check instead of a separate required-status-check (which has a self-modifying exploit gap under pull_request triggers) — see auto-merge.yml. Popular repos don't enforce required-review-count for solo maintainers; they just don't auto-merge non-self PRs and review those by hand. |
1 participant
Summary
.github/workflows/require-review-for-others.yml: a required-status-check that auto-passes for trusted PR authors (TRUSTED_PR_AUTHORS, defaultgetappz) and requires an actual approving review (checked via the Reviews API, independent of branch-protection review-count settings) for everyone else@getappzcan never approve@getappz's own PR (GitHub blocks self-approval at the platform level, confirmed this session on chore: add CODEOWNERS #239/ci: auto-enable merge-when-ready on every PR open #240). Rulesets' bypass lists are actor-based (whoever performs the merge bypasses), not author-based, so a bypass for@getappzwould let them merge anyone's PR without review, including external ones — the opposite of the goalapp/devin-ai-integrationdirectly against this repo (not a fork — Devin has write access). Once this check is required, Devin's PRs (and any future external contributor's) need your explicit approval; your own PRs keep auto-merging freelygh variable set TRUSTED_PR_AUTHORS --body "getappz,other-login"to add more trusted authors (e.g. once anagentflare-botidentity exists)Follow-up needed after merge
This workflow only becomes enforced once its check name ("Require review (non-self authors)") is added to branch protection's required status checks — I'll do that once this lands and the check has run at least once so GitHub knows its exact name.
Test plan
Summary by CodeRabbit