Fix release PR review re-requests for stale commits

.github/actions/release-pr-bot/manage-release-pr.js

@@ -143,12 +143,24 @@ 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 = submittedReviews.reduce((map, review) => {
+    const login = review.user?.login
+    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))
 
   if (newReviewers.length > 0) {
@@ -158,6 +170,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(', ')}`)
   }
 

.github/actions/release-pr-bot/manage-release-pr.test.js

@@ -94,20 +94,87 @@ 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' }],
+      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', commit_id: 'stale-sha' },
+        { 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 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' }],