diff --git a/scripts/assign-reviewers.mjs b/scripts/assign-reviewers.mjs index 60586290e710..fe43374e1b25 100644 --- a/scripts/assign-reviewers.mjs +++ b/scripts/assign-reviewers.mjs @@ -87,6 +87,32 @@ export function pickReviewers({ existing, codeownerPool, teamPool, exclude }) { return picked; } +/** + * Returns the logins that set their GitHub status to busy ("limited availability"). GitHub's own team review + * assignment skips these users too. User status is only exposed via GraphQL. + */ +export async function getBusyLogins({ github, core, logins }) { + const unique = [...new Map(logins.map(login => [login.toLowerCase(), login])).values()]; + if (!unique.length) { + return []; + } + + const query = `query(${unique.map((_, i) => `$l${i}: String!`).join(', ')}) { + ${unique.map((_, i) => `u${i}: user(login: $l${i}) { login status { indicatesLimitedAvailability } }`).join('\n ')} + }`; + const variables = Object.fromEntries(unique.map((login, i) => [`l${i}`, login])); + + try { + const data = await github.graphql(query, variables); + return Object.values(data) + .filter(user => user?.status?.indicatesLimitedAvailability) + .map(user => user.login); + } catch (error) { + core.warning(`Could not read reviewer availability, not skipping busy users: ${error.message}`); + return []; + } +} + export async function assignReviewers({ github, context, core }) { const { owner, repo } = context.repo; const pull_number = context.payload.pull_request.number; @@ -137,15 +163,19 @@ export async function assignReviewers({ github, context, core }) { } } + const teamPool = await listTeamMembers(TEAM_SLUG); + const busy = await getBusyLogins({ github, core, logins: [...codeownerPool, ...teamPool] }); + const reviewers = pickReviewers({ existing, codeownerPool, - teamPool: await listTeamMembers(TEAM_SLUG), - exclude: [author, ...requesters, ...NEVER_REQUEST], + teamPool, + exclude: [author, ...requesters, ...NEVER_REQUEST, ...busy], }); core.info(`Codeowners: ${codeowners.join(', ') || '(none)'}`); core.info(`Existing reviewers: ${existing.join(', ') || '(none)'}`); + core.info(`Busy, skipped: ${busy.join(', ') || '(none)'}`); if (!reviewers.length) { core.info('No eligible reviewers left to request.'); diff --git a/scripts/assign-reviewers.test.mjs b/scripts/assign-reviewers.test.mjs index 0628ad394373..896ffc38f236 100644 --- a/scripts/assign-reviewers.test.mjs +++ b/scripts/assign-reviewers.test.mjs @@ -1,6 +1,12 @@ -import { describe, expect, it } from 'vitest'; - -import { assignReviewers, getOwnersForFiles, parseCodeowners, pickReviewers } from './assign-reviewers.mjs'; +import { describe, expect, it, vi } from 'vitest'; + +import { + assignReviewers, + getBusyLogins, + getOwnersForFiles, + parseCodeowners, + pickReviewers, +} from './assign-reviewers.mjs'; import { attribution, junior, setup } from './github-test-utils.mjs'; const CODEOWNERS = ` @@ -80,7 +86,47 @@ describe('pickReviewers', () => { }); }); +describe('getBusyLogins', () => { + const core = { warning: vi.fn() }; + + it('returns users whose status indicates limited availability', async () => { + const github = { + graphql: vi.fn(async () => ({ + u0: { login: 'a', status: { indicatesLimitedAvailability: true } }, + u1: { login: 'b', status: { indicatesLimitedAvailability: false } }, + u2: { login: 'c', status: null }, + })), + }; + expect(await getBusyLogins({ github, core, logins: ['a', 'b', 'c', 'A'] })).toEqual(['a']); + expect(github.graphql).toHaveBeenCalledWith(expect.any(String), { l0: 'A', l1: 'b', l2: 'c' }); + }); + + it('skips the query when there are no logins', async () => { + const github = { graphql: vi.fn() }; + expect(await getBusyLogins({ github, core, logins: [] })).toEqual([]); + expect(github.graphql).not.toHaveBeenCalled(); + }); + + it('treats nobody as busy when the query fails', async () => { + const github = { graphql: vi.fn(async () => Promise.reject(new Error('boom'))) }; + expect(await getBusyLogins({ github, core, logins: ['a'] })).toEqual([]); + expect(core.warning).toHaveBeenCalledWith(expect.stringContaining('boom')); + }); +}); + describe('assignReviewers', () => { + it('does not request a review from busy users', async () => { + const { github, run } = setup(assignReviewers, { user: { login: 'author', type: 'User' } }); + github.graphql.mockResolvedValue({ u0: { login: 'a', status: { indicatesLimitedAvailability: true } } }); + await run(); + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: expect.arrayContaining(['b', 'c', 'd']) }), + ); + expect(github.rest.pulls.requestReviewers).toHaveBeenCalledWith( + expect.objectContaining({ reviewers: expect.not.arrayContaining(['a']) }), + ); + }); + it('does not request a review from the requester of a Junior PR', async () => { const { github, run } = setup(assignReviewers, { user: junior, body: attribution('**Alice Example**') }); await run(); diff --git a/scripts/github-test-utils.mjs b/scripts/github-test-utils.mjs index 37857c50ea65..3ad406f7aa0c 100644 --- a/scripts/github-test-utils.mjs +++ b/scripts/github-test-utils.mjs @@ -10,6 +10,7 @@ const profiles = { export function setup(run, pr) { const github = { paginate: vi.fn(async method => (method === github.rest.teams.listMembersInOrg ? Object.values(profiles) : [])), + graphql: vi.fn(async () => ({})), rest: { pulls: { get: vi.fn(async () => ({ @@ -35,7 +36,7 @@ export function setup(run, pr) { repo: { owner: 'getsentry', repo: 'sentry-javascript' }, payload: { pull_request: { number: 1 } }, }; - const core = { info: vi.fn() }; + const core = { info: vi.fn(), warning: vi.fn() }; return { github, run: () => run({ github, context, core }) }; }