From 28477ce9816f30b4d10fab845f45753e4fa43e77 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 09:50:13 +0000 Subject: [PATCH 1/3] Re-request review when a release PR changes after approval Approvals are now only treated as satisfied if they were submitted against the release PR's current head commit. If new commits land after someone approves, they're re-requested instead of staying excluded indefinitely. --- .../release-pr-bot/manage-release-pr.js | 29 ++++++++++--- .../release-pr-bot/manage-release-pr.test.js | 43 +++++++++++++++++-- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/.github/actions/release-pr-bot/manage-release-pr.js b/.github/actions/release-pr-bot/manage-release-pr.js index 05d7fab..ed19c29 100644 --- a/.github/actions/release-pr-bot/manage-release-pr.js +++ b/.github/actions/release-pr-bot/manage-release-pr.js @@ -143,12 +143,23 @@ async function updateReleasePR({ github, owner, repo, pr, body, reviewers }) { }), ]) - const excludedLogins = new Set([ - ...currentReviews.users.map((u) => u.login), - ...submittedReviews - .filter((review) => review.state === 'APPROVED') - .map((review) => review.user?.login), - ]) + const excludedLogins = new Set(currentReviews.users.map((u) => u.login)) + const staleApprovers = new Set() + + const latestReviewByLogin = new Map() + for (const review of submittedReviews) { + const login = review.user?.login + if (login) latestReviewByLogin.set(login, review) + } + for (const [login, review] of latestReviewByLogin) { + if (review.state !== 'APPROVED') continue + if (review.commit_id === pr.head.sha) { + excludedLogins.add(login) + } else { + staleApprovers.add(login) + } + } + const newReviewers = reviewers.filter((r) => !excludedLogins.has(r)) if (newReviewers.length > 0) { @@ -158,6 +169,12 @@ async function updateReleasePR({ github, owner, repo, pr, body, reviewers }) { pull_number: pr.number, reviewers: newReviewers, }) + const reRequested = newReviewers.filter((r) => staleApprovers.has(r)) + if (reRequested.length > 0) { + console.log( + `Re-requested review from ${reRequested.join(', ')} (new commits since their approval)`, + ) + } console.log(`Added reviewers: ${newReviewers.join(', ')}`) } diff --git a/.github/actions/release-pr-bot/manage-release-pr.test.js b/.github/actions/release-pr-bot/manage-release-pr.test.js index a90b0d1..8161c5f 100644 --- a/.github/actions/release-pr-bot/manage-release-pr.test.js +++ b/.github/actions/release-pr-bot/manage-release-pr.test.js @@ -94,13 +94,50 @@ describe('manage-release-pr', () => { expect(github.rest.pulls.requestReviewers).not.toHaveBeenCalled() }) - it('does not re-request reviews from people who already approved', async () => { + it('does not re-request reviews from people who already approved the current commit', async () => { const github = makeGithub({ commits: [{ sha: 'abc123' }], mergedPRs: [unreleasedPR], - openPRs: [{ number: 42 }], + openPRs: [{ number: 42, head: { sha: 'head-sha-1' } }], + currentReviewers: [], + submittedReviews: [ + { user: { login: 'alice' }, state: 'APPROVED', commit_id: 'head-sha-1' }, + ], + }) + + await run({ github, context: makeContext() }) + + expect(github.rest.pulls.requestReviewers).not.toHaveBeenCalled() + }) + + it('re-requests review from someone who approved a stale commit', async () => { + const github = makeGithub({ + commits: [{ sha: 'abc123' }], + mergedPRs: [unreleasedPR], + openPRs: [{ number: 42, head: { sha: 'head-sha-2' } }], + currentReviewers: [], + submittedReviews: [ + { user: { login: 'alice' }, state: 'APPROVED', commit_id: 'head-sha-1' }, + ], + }) + + await run({ github, context: makeContext() }) + + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ pull_number: 42, reviewers: ['alice'] }), + ) + }) + + it('uses the latest review per person when they reviewed more than once', async () => { + const github = makeGithub({ + commits: [{ sha: 'abc123' }], + mergedPRs: [unreleasedPR], + openPRs: [{ number: 42, head: { sha: 'head-sha-1' } }], currentReviewers: [], - submittedReviews: [{ user: { login: 'alice' }, state: 'APPROVED' }], + submittedReviews: [ + { user: { login: 'alice' }, state: 'APPROVED', commit_id: 'stale-sha' }, + { user: { login: 'alice' }, state: 'APPROVED', commit_id: 'head-sha-1' }, + ], }) await run({ github, context: makeContext() }) From 8e584858f7c4e7675a2dd94474095c82a48117ce Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 09:59:13 +0000 Subject: [PATCH 2/3] Fix lint: avoid for...of and continue in updateReleasePR CI failed on no-restricted-syntax / no-continue (Airbnb config). Rewrote the latest-review lookup with reduce/forEach instead. --- .../actions/release-pr-bot/manage-release-pr.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/actions/release-pr-bot/manage-release-pr.js b/.github/actions/release-pr-bot/manage-release-pr.js index ed19c29..ba4d635 100644 --- a/.github/actions/release-pr-bot/manage-release-pr.js +++ b/.github/actions/release-pr-bot/manage-release-pr.js @@ -146,19 +146,20 @@ async function updateReleasePR({ github, owner, repo, pr, body, reviewers }) { const excludedLogins = new Set(currentReviews.users.map((u) => u.login)) const staleApprovers = new Set() - const latestReviewByLogin = new Map() - for (const review of submittedReviews) { + const latestReviewByLogin = submittedReviews.reduce((map, review) => { const login = review.user?.login - if (login) latestReviewByLogin.set(login, review) - } - for (const [login, review] of latestReviewByLogin) { - if (review.state !== 'APPROVED') continue + if (login) map.set(login, review) + return map + }, new Map()) + + Array.from(latestReviewByLogin.entries()).forEach(([login, review]) => { + if (review.state !== 'APPROVED') return if (review.commit_id === pr.head.sha) { excludedLogins.add(login) } else { staleApprovers.add(login) } - } + }) const newReviewers = reviewers.filter((r) => !excludedLogins.has(r)) From acb86434b499fb87e22d992c9ffd1024f98f005c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 10:22:10 +0000 Subject: [PATCH 3/3] Add test: only the reviewer with a stale approval gets re-requested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers two contributors who both approved, then one of them lands another commit — verifies the other reviewer's still-current approval is left alone while only the stale one is re-requested. --- .../release-pr-bot/manage-release-pr.test.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.github/actions/release-pr-bot/manage-release-pr.test.js b/.github/actions/release-pr-bot/manage-release-pr.test.js index 8161c5f..53f2ba5 100644 --- a/.github/actions/release-pr-bot/manage-release-pr.test.js +++ b/.github/actions/release-pr-bot/manage-release-pr.test.js @@ -145,6 +145,36 @@ describe('manage-release-pr', () => { expect(github.rest.pulls.requestReviewers).not.toHaveBeenCalled() }) + it('re-requests review only from the person whose approval went stale after a new commit', async () => { + const bobPR = { + number: 2, + title: 'Add feature', + merged_at: '2024-01-02T00:00:00Z', + merge_commit_sha: 'def456', + user: { login: 'bob' }, + } + + const github = makeGithub({ + commits: [{ sha: 'abc123' }, { sha: 'def456' }], + mergedPRs: [unreleasedPR, bobPR], + openPRs: [{ number: 42, head: { sha: 'head-sha-2' } }], + currentReviewers: [], + submittedReviews: [ + // alice reviewed before her second commit landed - now stale + { user: { login: 'alice' }, state: 'APPROVED', commit_id: 'head-sha-1' }, + // bob reviewed after that commit landed - still current + { user: { login: 'bob' }, state: 'APPROVED', commit_id: 'head-sha-2' }, + ], + }) + + await run({ github, context: makeContext() }) + + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledTimes(1) + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ pull_number: 42, reviewers: ['alice'] }), + ) + }) + it('excludes bots from reviewers', async () => { const github = makeGithub({ commits: [{ sha: 'abc123' }],