From d21e2677b05592fd4656d9b5075c65a3cae63570 Mon Sep 17 00:00:00 2001 From: Thomas Willson Date: Thu, 15 Oct 2026 11:48:26 -0700 Subject: [PATCH] ReviewSLA: don't re-request a review from a new assignee who already approved On an out-of-office reassignment, the new assignee was requested as a reviewer on every PR they took over, including ones they had already approved. On StaflLib#2393 that asked willson556 to review again 22 minutes after his approval. Skip the request where their approval stands; they're still made assignee. Co-Authored-By: Claude Opus 5.5 --- ReviewSLA/run.js | 2 ++ ReviewSLA/run.test.js | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/ReviewSLA/run.js b/ReviewSLA/run.js index af6529a..892291a 100644 --- a/ReviewSLA/run.js +++ b/ReviewSLA/run.js @@ -96,6 +96,8 @@ async function reassign({ github, core, owner, inputs, teams, clock, stack, out, await github.rest.issues.removeAssignees({ owner, repo: p.repo, issue_number: p.number, assignees: [login] }); await github.rest.issues.addAssignees({ owner, repo: p.repo, issue_number: p.number, assignees: [chosen.login] }); p.assignees = [...p.assignees.filter((l) => l !== login), chosen.login]; + // Someone whose approval already stands owes nothing here, so a request would only ask them again. + if (approvedAt(p, chosen.login)) continue; try { await github.rest.pulls.requestReviewers({ owner, repo: p.repo, pull_number: p.number, reviewers: [chosen.login] }); } catch (error) { diff --git a/ReviewSLA/run.test.js b/ReviewSLA/run.test.js index 620cac8..ab58d1d 100644 --- a/ReviewSLA/run.test.js +++ b/ReviewSLA/run.test.js @@ -227,6 +227,20 @@ test('an assignee who is out keeps the PRs they already approved', async () => { assert.strictEqual(notice.issue_number, 2); }); +test('the new assignee isn\'t asked again on a PR they\'ve already approved', async () => { + // mhamilton approved #1 without being assigned; #2 above it still needs their review. + const w = world({ + prs: [ + onePr({ timeline: [reviewItem('mhamilton', 'APPROVED', '2026-10-01T17:30:00Z')] }), + { number: 2, head: 'feature/b', base: 'feature/a', assignees: ['ghopper', 'kjohnson'] }, + ], + out: [['Person ghopper', '2026-10-01']], + }); + await go(w, { now: '2026-10-01T18:00:00Z' }); + assert.deepStrictEqual(w.calls.assign, [[1, 'mhamilton'], [2, 'mhamilton']]); + assert.deepStrictEqual(w.calls.requested, [[2, 'mhamilton']]); +}); + test('on a public repo, the reassignment notice doesn\'t say the person is out', async () => { const w = world({ prs: [onePr()], out: [['Person ghopper', '2026-10-01']], isPrivate: false }); await go(w, { now: '2026-10-01T18:00:00Z' });