Approve uncontested PRs after a week#1669
Approve uncontested PRs after a week#1669rjmunro wants to merge 3 commits intoSofie-Automation:mainfrom
Conversation
* Skip draft PRs - Added check to skip PRs marked as draft * Use updated_at instead of created_at - Now checks when the PR was last updated (including new commits) rather than when it was originally created * Check for "Changes Requested" - Skips PRs where any reviewer has explicitly requested changes
WalkthroughA new GitHub Actions workflow is introduced that automatically approves open pull requests meeting specific criteria: no review changes requested, no prior bot approval, and 7+ days of inactivity. It queries PRs via the REST API, filters based on status and age, and submits approval reviews. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub<br/>(Scheduler)
participant Workflow as Workflow<br/>(Runner)
participant API as GitHub<br/>REST API
participant Bot as Bot<br/>(Approver)
GitHub->>Workflow: Trigger workflow<br/>(daily or manual)
Workflow->>API: Query open PRs<br/>(sorted by updated_at)
API-->>Workflow: Return PR list
Workflow->>Workflow: Filter:<br/>Skip drafts,<br/>Check inactivity (7+ days)
Workflow->>API: Get review history<br/>for each PR
API-->>Workflow: Return reviews
Workflow->>Workflow: Evaluate reviews:<br/>Changes requested?<br/>Already approved by bot?
alt Uncontested & Inactive
Workflow->>Bot: Prepare approval
Bot->>API: Submit APPROVE review
API-->>Bot: Confirmation
else Has Changes or Already Approved
Workflow->>Workflow: Skip PR
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/approve-uncontested-prs.yml (2)
67-78: Consider adding a log message when skipping already-approved PRs.Currently there's no indication when a PR is skipped because it was already approved by the bot. Adding a log improves observability and debugging.
📝 Proposed enhancement for logging
if (!alreadyApproved) { // Submit an approval review await github.rest.pulls.createReview({ owner: context.repo.owner, repo: context.repo.repo, pull_request_number: pr.number, body: '✅ Auto-approving: This PR has been inactive for 7+ days with no objections.', event: 'APPROVE' }); console.log(`Approved PR #${pr.number}: ${pr.title}`); + } else { + console.log(`Skipping PR #${pr.number}: Already approved by bot`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/approve-uncontested-prs.yml around lines 67 - 78, Add a log message for the branch where a PR is skipped due to alreadyApproved being truthy: detect the branch that currently does nothing when alreadyApproved is true and insert a console.log (or processLogger) that clearly states the PR number/title was skipped because it was already approved; reference the alreadyApproved variable and the existing approval path that calls github.rest.pulls.createReview to mirror the same PR identifiers in the log so skipped vs approved actions are observable.
32-79: Add error handling to prevent one PR's failure from stopping the entire workflow.If an API call fails for a single PR (e.g., network issue, rate limiting), the workflow will crash and skip remaining PRs. Wrapping the loop body in try-catch ensures all PRs are attempted.
🛡️ Proposed fix with error handling
for (const pr of pullRequests) { + try { // Skip draft PRs if (pr.draft) { console.log(`Skipping PR #${pr.number}: Draft PR`); continue; } // ... rest of the loop body ... console.log(`Approved PR #${pr.number}: ${pr.title}`); } + } catch (error) { + console.error(`Error processing PR #${pr.number}: ${error.message}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/approve-uncontested-prs.yml around lines 32 - 79, Wrap the body of the for (const pr of pullRequests) loop in a try-catch so that any API failures (e.g., github.rest.pulls.listReviews, github.rest.pulls.createReview) for a single PR do not abort the whole workflow; inside try keep existing logic (draft check, lastActivity, hasChangesRequested, alreadyApproved, createReview) and in catch log the error with context (include pr.number and pr.title) and continue to the next PR so processing keeps going for remaining pullRequests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/approve-uncontested-prs.yml:
- Around line 62-65: The check for existing approvals can throw when review.user
is null; update the alreadyApproved predicate to guard review.user before
accessing review.user.login (e.g., use review.user && review.user.login ===
'github-actions[bot]' or optional chaining like review.user?.login ===
'github-actions[bot]') and keep the review.state === 'APPROVED' check so
deleted-user reviews don't crash the workflow.
- Around line 52-60: The current hasChangesRequested check uses the entire
reviews array and can pick up historical CHANGES_REQUESTED entries; change the
logic to consider only each reviewer’s latest review: build a map from
review.author.login (or review.user.login) to their most recent review (compare
review.submitted_at or review.id), then set hasChangesRequested =
Array.from(latestReviews.values()).some(r => r.state === 'CHANGES_REQUESTED');
keep references to the existing reviews variable and the hasChangesRequested/
pr.number logic so the skip message remains unchanged.
- Around line 24-30: The current call to github.rest.pulls.list only returns a
single page (default 30 PRs) and will miss PRs in large repositories; replace
the single-page call with GitHub's paginator (use github.paginate) to fetch all
pull requests, e.g. call github.paginate against github.rest.pulls.list (or its
endpoint) with the same params (owner, repo, state, sort, direction) and collect
the results into the pullRequests array used later; update references to the
destructured { data: pullRequests } to use the full array returned by paginate
so downstream logic (the code that iterates over pullRequests) processes every
PR.
---
Nitpick comments:
In @.github/workflows/approve-uncontested-prs.yml:
- Around line 67-78: Add a log message for the branch where a PR is skipped due
to alreadyApproved being truthy: detect the branch that currently does nothing
when alreadyApproved is true and insert a console.log (or processLogger) that
clearly states the PR number/title was skipped because it was already approved;
reference the alreadyApproved variable and the existing approval path that calls
github.rest.pulls.createReview to mirror the same PR identifiers in the log so
skipped vs approved actions are observable.
- Around line 32-79: Wrap the body of the for (const pr of pullRequests) loop in
a try-catch so that any API failures (e.g., github.rest.pulls.listReviews,
github.rest.pulls.createReview) for a single PR do not abort the whole workflow;
inside try keep existing logic (draft check, lastActivity, hasChangesRequested,
alreadyApproved, createReview) and in catch log the error with context (include
pr.number and pr.title) and continue to the next PR so processing keeps going
for remaining pullRequests.
| const { data: pullRequests } = await github.rest.pulls.list({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| state: 'open', | ||
| sort: 'updated', | ||
| direction: 'asc' | ||
| }); |
There was a problem hiding this comment.
Missing pagination handling will skip PRs in repositories with many open PRs.
The pulls.list API returns only 30 results by default (max 100 per page). If the repository has more open PRs than that, older PRs beyond the first page will never be processed.
Use github.paginate to iterate through all pages:
🔧 Proposed fix using pagination
- // Fetch all open pull requests, sorted by last update (oldest first)
- const { data: pullRequests } = await github.rest.pulls.list({
+ // Fetch all open pull requests, sorted by last update (oldest first)
+ const pullRequests = await github.paginate(github.rest.pulls.list, {
owner: context.repo.owner,
repo: context.repo.repo,
state: 'open',
sort: 'updated',
- direction: 'asc'
+ direction: 'asc',
+ per_page: 100
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: pullRequests } = await github.rest.pulls.list({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| sort: 'updated', | |
| direction: 'asc' | |
| }); | |
| const pullRequests = await github.paginate(github.rest.pulls.list, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| sort: 'updated', | |
| direction: 'asc', | |
| per_page: 100 | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/approve-uncontested-prs.yml around lines 24 - 30, The
current call to github.rest.pulls.list only returns a single page (default 30
PRs) and will miss PRs in large repositories; replace the single-page call with
GitHub's paginator (use github.paginate) to fetch all pull requests, e.g. call
github.paginate against github.rest.pulls.list (or its endpoint) with the same
params (owner, repo, state, sort, direction) and collect the results into the
pullRequests array used later; update references to the destructured { data:
pullRequests } to use the full array returned by paginate so downstream logic
(the code that iterates over pullRequests) processes every PR.
| // Check if any reviewer has requested changes | ||
| const hasChangesRequested = reviews.some( | ||
| review => review.state === 'CHANGES_REQUESTED' | ||
| ); | ||
|
|
||
| if (hasChangesRequested) { | ||
| console.log(`Skipping PR #${pr.number}: Has changes requested`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Logic bug: Historical CHANGES_REQUESTED reviews block auto-approval even after they're resolved.
The GitHub API returns all reviews in history. If a reviewer requested changes but later approved (or their review was dismissed), both reviews appear in the list. The current logic will skip the PR because it finds a historical CHANGES_REQUESTED review, even though the objection was resolved.
Check only the most recent review per reviewer:
🐛 Proposed fix to check only latest review per reviewer
- // Check if any reviewer has requested changes
- const hasChangesRequested = reviews.some(
- review => review.state === 'CHANGES_REQUESTED'
- );
+ // Get the most recent review state per reviewer
+ const latestReviewByUser = new Map();
+ for (const review of reviews) {
+ if (review.user && review.state !== 'COMMENTED') {
+ latestReviewByUser.set(review.user.login, review.state);
+ }
+ }
+
+ // Check if any reviewer's latest review requested changes
+ const hasChangesRequested = [...latestReviewByUser.values()].some(
+ state => state === 'CHANGES_REQUESTED'
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if any reviewer has requested changes | |
| const hasChangesRequested = reviews.some( | |
| review => review.state === 'CHANGES_REQUESTED' | |
| ); | |
| if (hasChangesRequested) { | |
| console.log(`Skipping PR #${pr.number}: Has changes requested`); | |
| continue; | |
| } | |
| // Get the most recent review state per reviewer | |
| const latestReviewByUser = new Map(); | |
| for (const review of reviews) { | |
| if (review.user && review.state !== 'COMMENTED') { | |
| latestReviewByUser.set(review.user.login, review.state); | |
| } | |
| } | |
| // Check if any reviewer's latest review requested changes | |
| const hasChangesRequested = [...latestReviewByUser.values()].some( | |
| state => state === 'CHANGES_REQUESTED' | |
| ); | |
| if (hasChangesRequested) { | |
| console.log(`Skipping PR #${pr.number}: Has changes requested`); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/approve-uncontested-prs.yml around lines 52 - 60, The
current hasChangesRequested check uses the entire reviews array and can pick up
historical CHANGES_REQUESTED entries; change the logic to consider only each
reviewer’s latest review: build a map from review.author.login (or
review.user.login) to their most recent review (compare review.submitted_at or
review.id), then set hasChangesRequested =
Array.from(latestReviews.values()).some(r => r.state === 'CHANGES_REQUESTED');
keep references to the existing reviews variable and the hasChangesRequested/
pr.number logic so the skip message remains unchanged.
| // Check if already approved by this action to avoid spamming | ||
| const alreadyApproved = reviews.some( | ||
| review => review.user.login === 'github-actions[bot]' && review.state === 'APPROVED' | ||
| ); |
There was a problem hiding this comment.
Add null safety for review.user to prevent runtime errors.
If a reviewer's account has been deleted, review.user will be null, causing review.user.login to throw a TypeError and crash the workflow.
🛡️ Proposed fix with null check
// Check if already approved by this action to avoid spamming
const alreadyApproved = reviews.some(
- review => review.user.login === 'github-actions[bot]' && review.state === 'APPROVED'
+ review => review.user?.login === 'github-actions[bot]' && review.state === 'APPROVED'
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if already approved by this action to avoid spamming | |
| const alreadyApproved = reviews.some( | |
| review => review.user.login === 'github-actions[bot]' && review.state === 'APPROVED' | |
| ); | |
| // Check if already approved by this action to avoid spamming | |
| const alreadyApproved = reviews.some( | |
| review => review.user?.login === 'github-actions[bot]' && review.state === 'APPROVED' | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/approve-uncontested-prs.yml around lines 62 - 65, The
check for existing approvals can throw when review.user is null; update the
alreadyApproved predicate to guard review.user before accessing
review.user.login (e.g., use review.user && review.user.login ===
'github-actions[bot]' or optional chaining like review.user?.login ===
'github-actions[bot]') and keep the review.state === 'APPROVED' check so
deleted-user reviews don't crash the workflow.
|
I suspect that relying on the Many of which don't affect whether anyone has any interest in reviewing the PR. (I havent verified that list) In a way I'm not even sure that I like pushing commits to reset this timer, as I am sometimes rebasing PRs when they have a merge conflict while the conflict is fresh in my memory, which will cause these unreviewed PRs to be delayed. I think my ideal criteria for this would be to look at when the pr was marked as 'ready for review' as the base time. (I believe the timeline/events can be queried to determine that). |
2 participants
About the Contributor
This pull request is posted on behalf of myself.
Type of Contribution
This is a CI / code management feature.
Current Behavior
n/a
New Behavior
The policy has been suggested that PRs should require a single approval and allow a week for people to object before they are merged, or if they are urgent, they can get 2 approvals. This PR adds a daily check that grants an approval to PRs that have been open for a week without objection, meaning the policy can be enforced by setting required approvals to 2 on the main branch.
Testing
It's basically impossible to test this without making it live.
Affected areas
This PR affects Github workflow
Time Frame
Other Information
Status
Summary
Introduces a new GitHub Actions workflow (
approve-uncontested-prs.yml) that automatically grants approvals to pull requests meeting specific criteria, enabling a policy where main branch protection requires 2 approvals while allowing eligible single-approval PRs to be auto-approved after 7 days of inactivity without objections.Changes
File:
.github/workflows/approve-uncontested-prs.yml(80 new lines)Adds a scheduled GitHub Actions workflow with the following behavior:
workflow_dispatchpull-requests: writescope for review submissiongithub-actions[bot]to avoid duplicate approvalsupdated_atactivity) for 7+ days, submitting an APPROVE review with the message: "✅ Auto-approving: This PR has been inactive for 7+ days with no objections."Key implementation details:
updated_atfield instead ofcreated_atto account for new commits on older PRs, preventing premature approval of code pushed to stale PRsactions/github-script@v7for PR and review queries