Uh oh!
There was an error while loading. Please reload this page.
ci(workflow): Skip PR review reminders when already approved - #20629
Conversation
If a PR already has at least one approval, there's no need to nudge the remaining requested reviewers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c2c0bec. Configure here.
| pull_number: pr.number, | ||
| per_page: 100, | ||
| }); | ||
| const hasApproval = reviews.some(r => r.state === 'APPROVED'); |
There was a problem hiding this comment.
Stale approval check ignores superseding reviews
Medium Severity
The reviews.some(r => r.state === 'APPROVED') check looks at all review objects ever submitted on the PR, not just the latest from each reviewer. GitHub's listReviews API preserves every review chronologically — if a reviewer first approves and later submits a "Changes Requested" review, both objects are retained with their original states. This means the check finds the stale APPROVED review and incorrectly skips the reminder, even though no current approval exists. The fix would be to consider only each reviewer's most recent review when determining approval status.
Reviewed by Cursor Bugbot for commit c2c0bec. Configure here.
| const hasApproval = reviews.some(r => r.state === 'APPROVED'); | ||
| if (hasApproval) continue; |
There was a problem hiding this comment.
Bug: The check for existing approvals (hasApproval) doesn't differentiate between current and stale approvals, causing reminders to be skipped for reviewers who are re-requested after new commits are pushed.
Severity: MEDIUM
Suggested Fix
To fix this, the logic should not rely solely on the presence of an APPROVED review. It should correlate reviews with the latest commit hash or consider the review submission timestamp relative to the last commit. Alternatively, filter out reviews that predate the most recent commit on the PR before checking for an APPROVED state. This ensures that only approvals on the current version of the code are considered.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: scripts/pr-review-reminder.mjs#L184-L185
Potential issue: The script determines if a PR has an approval by checking if
`reviews.some(r => r.state === 'APPROVED')`. This logic is flawed because it doesn't
account for stale approvals. On repositories without the "Dismiss stale pull request
approvals" setting, if new commits are pushed after an approval and the same reviewer is
re-requested, their old `APPROVED` review persists. The script will incorrectly detect
this as a current approval, set `hasApproval` to `true`, and skip sending a reminder,
effectively silencing reminders for that reviewer indefinitely.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Good bot. But in our workflow that is super rare. To keep it simple I'll leave it as is
size-limit report 📦
|
Uh oh!
There was an error while loading. Please reload this page.
Only allow PR nudges when the PR has not yet been approved.