From 4a6ef33d9acadb01fa79f67492bdd431ac18ed3a Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:12:05 +0000 Subject: [PATCH 1/4] chore(ci): Move reviewer routing out of CODEOWNERS GitHub requests every matching code owner team when a PR opens, before the assign-reviewers workflow requests individuals, so the team-level 'only notify requested team members' setting never kicks in and the whole team gets pinged. Keep the same rules in .github/REVIEWERS, which only the workflow reads. Co-Authored-By: Francesco Gringl-Novy <2411343+mydea@users.noreply.github.com> --- .github/{CODEOWNERS => REVIEWERS} | 5 +++++ .github/workflows/assign-reviewers.yml | 2 +- scripts/assign-reviewers.mjs | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) rename .github/{CODEOWNERS => REVIEWERS} (92%) diff --git a/.github/CODEOWNERS b/.github/REVIEWERS similarity index 92% rename from .github/CODEOWNERS rename to .github/REVIEWERS index a6dc12f405db..e534da992d3d 100644 --- a/.github/CODEOWNERS +++ b/.github/REVIEWERS @@ -1,3 +1,8 @@ +# Reviewer routing for scripts/assign-reviewers.mjs, in CODEOWNERS syntax. +# This is deliberately not a CODEOWNERS file: GitHub requests every matching +# code owner team as soon as a PR opens, before the workflow requests anyone, +# so the whole team gets notified. + # Browser, replay, feedback, and related packages /packages/browser/ @getsentry/team-javascript-sdks-browser /packages/browser-utils/ @getsentry/team-javascript-sdks-browser diff --git a/.github/workflows/assign-reviewers.yml b/.github/workflows/assign-reviewers.yml index e8d11ae70c20..60e541dcb775 100644 --- a/.github/workflows/assign-reviewers.yml +++ b/.github/workflows/assign-reviewers.yml @@ -34,7 +34,7 @@ jobs: uses: actions/checkout@v7 with: sparse-checkout: | - .github/CODEOWNERS + .github/REVIEWERS scripts/assign-reviewers.mjs scripts/junior-requesters.mjs sparse-checkout-cone-mode: false diff --git a/scripts/assign-reviewers.mjs b/scripts/assign-reviewers.mjs index ee6122f71d39..ea5ca3c7cd0d 100644 --- a/scripts/assign-reviewers.mjs +++ b/scripts/assign-reviewers.mjs @@ -5,7 +5,7 @@ import { getJuniorRequesters, TEAM_SLUG } from './junior-requesters.mjs'; const REQUIRED_REVIEWERS = 3; // Managers who are members (and code owners) of the SDK teams, but should never be auto-requested for review. const NEVER_REQUEST = ['mydea', 'stephanie-anderson']; -const CODEOWNERS_PATH = '.github/CODEOWNERS'; +const REVIEWERS_PATH = '.github/REVIEWERS'; function patternToRegExp(pattern) { let p = pattern; @@ -120,7 +120,7 @@ export async function assignReviewers({ github, context, core }) { (await github.paginate(github.rest.teams.listMembersInOrg, { org: owner, team_slug })).map(m => m.login); const files = (await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number })).map(f => f.filename); - const codeowners = getOwnersForFiles(parseCodeowners(fs.readFileSync(CODEOWNERS_PATH, 'utf8')), files); + const codeowners = getOwnersForFiles(parseCodeowners(fs.readFileSync(REVIEWERS_PATH, 'utf8')), files); const codeownerPool = []; for (const codeowner of codeowners) { From 817898e3d74c71b7d619452714fe30580f234281 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:35:08 +0000 Subject: [PATCH 2/4] chore(ci): Replace manager review requests from team auto-assignment team-javascript-sdks will auto-assign one reviewer so GitHub doesn't ping the whole team. GitHub can pick a manager, so the workflow removes pending review requests from NEVER_REQUEST users and doesn't count them, which makes it pick someone else instead. Co-Authored-By: Francesco Gringl-Novy <2411343+mydea@users.noreply.github.com> --- scripts/assign-reviewers.mjs | 15 +++++++++++++-- scripts/assign-reviewers.test.mjs | 29 +++++++++++++++++++++++++++++ scripts/github-test-utils.mjs | 1 + 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/scripts/assign-reviewers.mjs b/scripts/assign-reviewers.mjs index ea5ca3c7cd0d..7fe9b0dcab6d 100644 --- a/scripts/assign-reviewers.mjs +++ b/scripts/assign-reviewers.mjs @@ -3,7 +3,8 @@ import * as fs from 'fs'; import { getJuniorRequesters, TEAM_SLUG } from './junior-requesters.mjs'; const REQUIRED_REVIEWERS = 3; -// Managers who are members (and code owners) of the SDK teams, but should never be auto-requested for review. +// Managers who are members of the SDK teams, but should never be auto-requested for review. +// GitHub's team auto-assignment can still pick them, so their pending requests are removed again. const NEVER_REQUEST = ['mydea', 'stephanie-anderson']; const REVIEWERS_PATH = '.github/REVIEWERS'; @@ -100,12 +101,22 @@ export async function assignReviewers({ github, context, core }) { // Whoever asked Junior for the PR is its de facto author. const requesters = await getJuniorRequesters({ github, core, pr: currentPr }); + const neverRequest = new Set(NEVER_REQUEST.map(login => login.toLowerCase())); + const unwantedRequests = currentPr.requested_reviewers + .map(user => user.login) + .filter(login => neverRequest.has(login.toLowerCase())); + if (unwantedRequests.length) { + core.info(`Removing review requests from: ${unwantedRequests.join(', ')}`); + await github.rest.pulls.removeRequestedReviewers({ owner, repo, pull_number, reviewers: unwantedRequests }); + } + const requestedReviewers = currentPr.requested_reviewers.filter(user => !neverRequest.has(user.login.toLowerCase())); + const reviews = await github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number }); // Requested reviewers drop off `requested_reviewers` once they submit a review, so count both. // Bots (AI reviewers) don't count towards the required reviewers. const existing = [ ...new Set( - [...currentPr.requested_reviewers, ...reviews.map(r => r.user)] + [...requestedReviewers, ...reviews.map(r => r.user)] .filter(user => user && user.type !== 'Bot' && user.login.toLowerCase() !== author.toLowerCase()) .map(user => user.login), ), diff --git a/scripts/assign-reviewers.test.mjs b/scripts/assign-reviewers.test.mjs index 5ff6877615e6..99ba7d786f8b 100644 --- a/scripts/assign-reviewers.test.mjs +++ b/scripts/assign-reviewers.test.mjs @@ -88,4 +88,33 @@ describe('assignReviewers', () => { expect.objectContaining({ reviewers: expect.arrayContaining(['b', 'c', 'd']) }), ); }); + + it('removes review requests from managers and requests someone else instead', async () => { + const { github, run } = setup(assignReviewers, { + user: { login: 'author', type: 'User' }, + requested_reviewers: [ + { login: 'Mydea', type: 'User' }, + { login: 'a', type: 'User' }, + ], + }); + await run(); + expect(github.rest.pulls.removeRequestedReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: ['Mydea'] }), + ); + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: [expect.any(String), expect.any(String)] }), + ); + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: expect.not.arrayContaining(['a', 'Mydea']) }), + ); + }); + + it('does not remove any review requests when no manager is requested', async () => { + const { github, run } = setup(assignReviewers, { + user: { login: 'author', type: 'User' }, + requested_reviewers: [{ login: 'a', type: 'User' }], + }); + await run(); + expect(github.rest.pulls.removeRequestedReviewers).not.toHaveBeenCalled(); + }); }); diff --git a/scripts/github-test-utils.mjs b/scripts/github-test-utils.mjs index 37857c50ea65..1feade5fe177 100644 --- a/scripts/github-test-utils.mjs +++ b/scripts/github-test-utils.mjs @@ -25,6 +25,7 @@ export function setup(run, pr) { listReviews: vi.fn(), listFiles: vi.fn(), requestReviewers: vi.fn(), + removeRequestedReviewers: vi.fn(), }, teams: { listMembersInOrg: vi.fn() }, users: { getByUsername: vi.fn(async ({ username }) => ({ data: profiles[username] })) }, From 903bf4695197113532846f54c529761ab0671f13 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:42:22 +0000 Subject: [PATCH 3/4] test: Assert each excluded reviewer separately --- scripts/assign-reviewers.test.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/assign-reviewers.test.mjs b/scripts/assign-reviewers.test.mjs index 99ba7d786f8b..a34ed2b035e6 100644 --- a/scripts/assign-reviewers.test.mjs +++ b/scripts/assign-reviewers.test.mjs @@ -105,7 +105,10 @@ describe('assignReviewers', () => { expect.objectContaining({ reviewers: [expect.any(String), expect.any(String)] }), ); expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( - expect.objectContaining({ reviewers: expect.not.arrayContaining(['a', 'Mydea']) }), + expect.objectContaining({ reviewers: expect.not.arrayContaining(['a']) }), + ); + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: expect.not.arrayContaining(['Mydea']) }), ); }); From 60c118439231358a61d3d5c7afb3f18f8357ee58 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:45:51 +0000 Subject: [PATCH 4/4] chore(ci): Keep manager review requests, just don't count them Instead of removing a manager that GitHub's auto-assignment picked, leave the request in place and top up to 3 other reviewers, so a PR can end up with 4. Co-Authored-By: Francesco Gringl-Novy <2411343+mydea@users.noreply.github.com> --- scripts/assign-reviewers.mjs | 9 +-------- scripts/assign-reviewers.test.mjs | 14 +------------- scripts/github-test-utils.mjs | 1 - 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/scripts/assign-reviewers.mjs b/scripts/assign-reviewers.mjs index 7fe9b0dcab6d..2120f7bdd5cd 100644 --- a/scripts/assign-reviewers.mjs +++ b/scripts/assign-reviewers.mjs @@ -4,7 +4,7 @@ import { getJuniorRequesters, TEAM_SLUG } from './junior-requesters.mjs'; const REQUIRED_REVIEWERS = 3; // Managers who are members of the SDK teams, but should never be auto-requested for review. -// GitHub's team auto-assignment can still pick them, so their pending requests are removed again. +// GitHub's team auto-assignment can still pick them, so their pending requests don't count towards the required reviewers. const NEVER_REQUEST = ['mydea', 'stephanie-anderson']; const REVIEWERS_PATH = '.github/REVIEWERS'; @@ -102,13 +102,6 @@ export async function assignReviewers({ github, context, core }) { const requesters = await getJuniorRequesters({ github, core, pr: currentPr }); const neverRequest = new Set(NEVER_REQUEST.map(login => login.toLowerCase())); - const unwantedRequests = currentPr.requested_reviewers - .map(user => user.login) - .filter(login => neverRequest.has(login.toLowerCase())); - if (unwantedRequests.length) { - core.info(`Removing review requests from: ${unwantedRequests.join(', ')}`); - await github.rest.pulls.removeRequestedReviewers({ owner, repo, pull_number, reviewers: unwantedRequests }); - } const requestedReviewers = currentPr.requested_reviewers.filter(user => !neverRequest.has(user.login.toLowerCase())); const reviews = await github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number }); diff --git a/scripts/assign-reviewers.test.mjs b/scripts/assign-reviewers.test.mjs index a34ed2b035e6..0628ad394373 100644 --- a/scripts/assign-reviewers.test.mjs +++ b/scripts/assign-reviewers.test.mjs @@ -89,7 +89,7 @@ describe('assignReviewers', () => { ); }); - it('removes review requests from managers and requests someone else instead', async () => { + it('does not count pending review requests from managers towards the required reviewers', async () => { const { github, run } = setup(assignReviewers, { user: { login: 'author', type: 'User' }, requested_reviewers: [ @@ -98,9 +98,6 @@ describe('assignReviewers', () => { ], }); await run(); - expect(github.rest.pulls.removeRequestedReviewers).toHaveBeenCalledWith( - expect.objectContaining({ reviewers: ['Mydea'] }), - ); expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( expect.objectContaining({ reviewers: [expect.any(String), expect.any(String)] }), ); @@ -111,13 +108,4 @@ describe('assignReviewers', () => { expect.objectContaining({ reviewers: expect.not.arrayContaining(['Mydea']) }), ); }); - - it('does not remove any review requests when no manager is requested', async () => { - const { github, run } = setup(assignReviewers, { - user: { login: 'author', type: 'User' }, - requested_reviewers: [{ login: 'a', type: 'User' }], - }); - await run(); - expect(github.rest.pulls.removeRequestedReviewers).not.toHaveBeenCalled(); - }); }); diff --git a/scripts/github-test-utils.mjs b/scripts/github-test-utils.mjs index 1feade5fe177..37857c50ea65 100644 --- a/scripts/github-test-utils.mjs +++ b/scripts/github-test-utils.mjs @@ -25,7 +25,6 @@ export function setup(run, pr) { listReviews: vi.fn(), listFiles: vi.fn(), requestReviewers: vi.fn(), - removeRequestedReviewers: vi.fn(), }, teams: { listMembersInOrg: vi.fn() }, users: { getByUsername: vi.fn(async ({ username }) => ({ data: profiles[username] })) },