Skip to content

ci(workflow): Skip PR review reminders when already approved#20629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JPeer264 merged 1 commit into from
May 4, 2026
Merged

ci(workflow): Skip PR review reminders when already approved #20629

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions scripts/pr-review-reminder.mjs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -174,6 +174,16 @@ export default async function run({ github, context, core }) {
const pendingTeams = requested.teams; // team reviewers
if (pendingReviewers.length === 0 && pendingTeams.length === 0) continue;

// Skip if the PR already has at least one approval — no need to nudge remaining reviewers
const reviews = await github.paginate(github.rest.pulls.listReviews, {
owner,
repo,
pull_number: pr.number,
per_page: 100,
});
const hasApproval = reviews.some(r => r.state === 'APPROVED');

@cursorcursorBotMay 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in CursorFix in Web

Reviewed by Cursor Bugbot for commit c2c0bec. Configure here.

if (hasApproval) continue;
Comment on lines +184 to +185

@sentrysentryBotMay 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@JPeer264JPeer264May 4, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good bot. But in our workflow that is super rare. To keep it simple I'll leave it as is


// Fetch the PR timeline to determine when each review was (last) requested
const timeline = await github.paginate(github.rest.issues.listEventsForTimeline, {
owner,
Expand Down
Loading