mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-12 02:19:43 +00:00
ci: Add opt-in auto-assignment of reviewer teams (#32216)
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
@@ -461,6 +461,70 @@ export async function postOrUpdateComment(pullRequestNumber, body, botMarker) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Request review from the given GitHub teams on a PR.
|
||||
*
|
||||
* Team slugs are the part after the org, e.g. `catalysts` for
|
||||
* `@n8n-io/catalysts`. Re-requesting an already-requested team is a no-op on
|
||||
* GitHub's side, so this is safe to call on every PR update.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @param { string[] } teamSlugs
|
||||
*/
|
||||
export async function requestTeamReviewers(pullRequestNumber, teamSlugs) {
|
||||
if (teamSlugs.length === 0) return;
|
||||
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
await octokit.rest.pulls.requestReviewers({
|
||||
owner,
|
||||
repo,
|
||||
pull_number: pullRequestNumber,
|
||||
team_reviewers: teamSlugs,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a label to a PR (issue). Adding a label that is already present is a
|
||||
* no-op on GitHub's side.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @param { string } label
|
||||
*/
|
||||
export async function addLabel(pullRequestNumber, label) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
await octokit.rest.issues.addLabels({
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
labels: [label],
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a label from a PR (issue). Removing a label that is not present
|
||||
* returns 404 from GitHub; that case is swallowed so the call is idempotent.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @param { string } label
|
||||
*/
|
||||
export async function removeLabel(pullRequestNumber, label) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
try {
|
||||
await octokit.rest.issues.removeLabel({
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
name: label,
|
||||
});
|
||||
} catch (ex) {
|
||||
if (ex?.status === 404) return;
|
||||
throw ex;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {string} tag
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
/**
|
||||
* Opt-in reviewer auto-assignment.
|
||||
*
|
||||
* When a PR carries the `Auto-assign reviewers` label, request review from
|
||||
* every team that owns a changed file (per .github/OWNERS). This is strictly
|
||||
* opt-in: without the label the script is a no-op, so it never adds friction
|
||||
* to PRs that don't want it.
|
||||
*
|
||||
* Assignment happens once: after requesting reviewers the trigger label is
|
||||
* swapped for the `Reviewers auto-assigned` marker, and any PR already carrying
|
||||
* that marker is skipped. This stops later `synchronize` events from
|
||||
* re-requesting (and re-notifying) the same teams.
|
||||
*
|
||||
* Complements `owners-review-recommendations.mjs`, which only *suggests*
|
||||
* reviewer teams in a comment — this one actually requests them.
|
||||
*/
|
||||
|
||||
import {
|
||||
addLabel,
|
||||
ensureEnvVar,
|
||||
getChangedFiles,
|
||||
readPrLabels,
|
||||
removeLabel,
|
||||
requestTeamReviewers,
|
||||
} from './github-helpers.mjs';
|
||||
import { assignOwnership, ownershipsToAllocations, parseOwnersFile } from './owners.mjs';
|
||||
|
||||
export const AUTO_ASSIGN_LABEL = 'Auto-assign reviewers';
|
||||
export const ASSIGNED_LABEL = 'Reviewers auto-assigned';
|
||||
|
||||
/**
|
||||
* Convert an OWNERS team handle (`@n8n-io/catalysts`) into the GitHub team slug
|
||||
* (`catalysts`) expected by the requestReviewers API.
|
||||
*
|
||||
* @param { string } team
|
||||
* @returns { string }
|
||||
*/
|
||||
export function teamHandleToSlug(team) {
|
||||
return team.replace(/^@[^/]+\//, '');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { string[] } labels
|
||||
* @returns { boolean }
|
||||
*/
|
||||
export function hasAutoAssignLabel(labels) {
|
||||
return labels.includes(AUTO_ASSIGN_LABEL);
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine the team slugs that own files in the changeset, de-duplicated and
|
||||
* stable-ordered (largest ownership first, matching allocation order).
|
||||
*
|
||||
* @param { Set<string> } changedFiles
|
||||
* @returns { string[] }
|
||||
*/
|
||||
export function resolveOwnerTeamSlugs(changedFiles) {
|
||||
const owners = parseOwnersFile();
|
||||
const ownerships = assignOwnership(changedFiles, owners);
|
||||
const allocations = ownershipsToAllocations(ownerships);
|
||||
|
||||
return allocations.map((allocation) => teamHandleToSlug(allocation.team));
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { number } pullRequestNumber
|
||||
*/
|
||||
export async function run(pullRequestNumber) {
|
||||
const labels = readPrLabels();
|
||||
|
||||
if (labels.includes(ASSIGNED_LABEL)) {
|
||||
console.log(
|
||||
`PR #${pullRequestNumber} already carries the "${ASSIGNED_LABEL}" label; reviewers were already assigned.`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!hasAutoAssignLabel(labels)) {
|
||||
console.log(
|
||||
`PR #${pullRequestNumber} is missing the "${AUTO_ASSIGN_LABEL}" label; skipping reviewer assignment.`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const changedFiles = await getChangedFiles(pullRequestNumber);
|
||||
const teamSlugs = resolveOwnerTeamSlugs(changedFiles);
|
||||
|
||||
if (teamSlugs.length === 0) {
|
||||
console.log(`No owning teams matched for PR #${pullRequestNumber}; nothing to assign.`);
|
||||
return;
|
||||
}
|
||||
|
||||
console.log(`Requesting review from team(s): ${teamSlugs.join(', ')}`);
|
||||
await requestTeamReviewers(pullRequestNumber, teamSlugs);
|
||||
|
||||
// Swap the trigger label for the done-marker so later runs no-op.
|
||||
await addLabel(pullRequestNumber, ASSIGNED_LABEL);
|
||||
await removeLabel(pullRequestNumber, AUTO_ASSIGN_LABEL);
|
||||
}
|
||||
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
const pullRequestNumber = parseInt(ensureEnvVar('PULL_REQUEST_NUMBER'));
|
||||
await run(pullRequestNumber);
|
||||
}
|
||||
@@ -0,0 +1,174 @@
|
||||
import { beforeEach, describe, it, mock } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners-assign-reviewers.test.mjs
|
||||
* */
|
||||
|
||||
/** @type {() => string[]} */
|
||||
let readPrLabelsImpl = () => [];
|
||||
/** @type {(pullRequestNumber: number) => Promise<Set<string>>} */
|
||||
let getChangedFilesImpl = async () => new Set();
|
||||
/** @type {(pullRequestNumber: number, teamSlugs: string[]) => Promise<void>} */
|
||||
let requestTeamReviewersImpl = async () => {};
|
||||
/** @type {(pullRequestNumber: number, label: string) => Promise<void>} */
|
||||
let addLabelImpl = async () => {};
|
||||
/** @type {(pullRequestNumber: number, label: string) => Promise<void>} */
|
||||
let removeLabelImpl = async () => {};
|
||||
|
||||
mock.module('./github-helpers.mjs', {
|
||||
namedExports: {
|
||||
ensureEnvVar: () => {}, // no-op in tests
|
||||
readPrLabels: () => readPrLabelsImpl(),
|
||||
getChangedFiles: (n) => getChangedFilesImpl(n),
|
||||
requestTeamReviewers: (pullRequestNumber, teamSlugs) =>
|
||||
requestTeamReviewersImpl(pullRequestNumber, teamSlugs),
|
||||
addLabel: (pullRequestNumber, label) => addLabelImpl(pullRequestNumber, label),
|
||||
removeLabel: (pullRequestNumber, label) => removeLabelImpl(pullRequestNumber, label),
|
||||
},
|
||||
});
|
||||
|
||||
/** @type {() => Array<{ filepath: string, team: string }>} */
|
||||
let parseOwnersFileImpl = () => [];
|
||||
/** @type {(files: Set<string>, owners: Array<{ filepath: string, team: string }>) => Map<string, string[]>} */
|
||||
let assignOwnershipImpl = () => new Map();
|
||||
/** @type {(ownerships: Map<string, string[]>) => Array<{ team: string, fileCount: number }>} */
|
||||
let ownershipsToAllocationsImpl = () => [];
|
||||
|
||||
mock.module('./owners.mjs', {
|
||||
namedExports: {
|
||||
parseOwnersFile: () => parseOwnersFileImpl(),
|
||||
assignOwnership: (files, owners) => assignOwnershipImpl(files, owners),
|
||||
ownershipsToAllocations: (ownerships) => ownershipsToAllocationsImpl(ownerships),
|
||||
},
|
||||
});
|
||||
|
||||
const {
|
||||
AUTO_ASSIGN_LABEL,
|
||||
ASSIGNED_LABEL,
|
||||
teamHandleToSlug,
|
||||
hasAutoAssignLabel,
|
||||
resolveOwnerTeamSlugs,
|
||||
run,
|
||||
} = await import('./owners-assign-reviewers.mjs');
|
||||
|
||||
describe('teamHandleToSlug', () => {
|
||||
it('strips the org prefix from an OWNERS team handle', () => {
|
||||
assert.equal(teamHandleToSlug('@n8n-io/catalysts'), 'catalysts');
|
||||
});
|
||||
|
||||
it('leaves a bare slug untouched', () => {
|
||||
assert.equal(teamHandleToSlug('catalysts'), 'catalysts');
|
||||
});
|
||||
|
||||
it('only strips the first segment', () => {
|
||||
assert.equal(teamHandleToSlug('@n8n-io/migrations-review'), 'migrations-review');
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasAutoAssignLabel', () => {
|
||||
it('returns true when the label is present', () => {
|
||||
assert.equal(hasAutoAssignLabel(['bug', AUTO_ASSIGN_LABEL]), true);
|
||||
});
|
||||
|
||||
it('returns false when the label is absent', () => {
|
||||
assert.equal(hasAutoAssignLabel(['bug', 'enhancement']), false);
|
||||
});
|
||||
|
||||
it('returns false for an empty label set', () => {
|
||||
assert.equal(hasAutoAssignLabel([]), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveOwnerTeamSlugs', () => {
|
||||
beforeEach(() => {
|
||||
parseOwnersFileImpl = () => [];
|
||||
assignOwnershipImpl = () => new Map();
|
||||
ownershipsToAllocationsImpl = () => [];
|
||||
});
|
||||
|
||||
it('maps allocations to team slugs in allocation order', () => {
|
||||
ownershipsToAllocationsImpl = () => [
|
||||
{ team: '@n8n-io/catalysts', fileCount: 3 },
|
||||
{ team: '@n8n-io/qa-dx', fileCount: 1 },
|
||||
];
|
||||
|
||||
assert.deepEqual(resolveOwnerTeamSlugs(new Set(['a.ts'])), ['catalysts', 'qa-dx']);
|
||||
});
|
||||
|
||||
it('returns an empty array when no team owns a file', () => {
|
||||
assert.deepEqual(resolveOwnerTeamSlugs(new Set(['a.ts'])), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe('run', () => {
|
||||
let requestTeamReviewers;
|
||||
let addLabel;
|
||||
let removeLabel;
|
||||
|
||||
beforeEach(() => {
|
||||
readPrLabelsImpl = () => [AUTO_ASSIGN_LABEL];
|
||||
getChangedFilesImpl = async () => new Set(['a.ts']);
|
||||
parseOwnersFileImpl = () => [];
|
||||
assignOwnershipImpl = () => new Map();
|
||||
ownershipsToAllocationsImpl = () => [{ team: '@n8n-io/catalysts', fileCount: 1 }];
|
||||
requestTeamReviewers = mock.fn(async () => {});
|
||||
requestTeamReviewersImpl = requestTeamReviewers;
|
||||
addLabel = mock.fn(async () => {});
|
||||
addLabelImpl = addLabel;
|
||||
removeLabel = mock.fn(async () => {});
|
||||
removeLabelImpl = removeLabel;
|
||||
});
|
||||
|
||||
it('requests review from the owning team slugs when the label is present', async () => {
|
||||
ownershipsToAllocationsImpl = () => [
|
||||
{ team: '@n8n-io/catalysts', fileCount: 2 },
|
||||
{ team: '@n8n-io/qa-dx', fileCount: 1 },
|
||||
];
|
||||
|
||||
await run(42);
|
||||
|
||||
assert.equal(requestTeamReviewers.mock.calls.length, 1);
|
||||
assert.equal(requestTeamReviewers.mock.calls[0].arguments[0], 42);
|
||||
assert.deepEqual(requestTeamReviewers.mock.calls[0].arguments[1], ['catalysts', 'qa-dx']);
|
||||
});
|
||||
|
||||
it('swaps the trigger label for the done-marker after assigning', async () => {
|
||||
await run(42);
|
||||
|
||||
assert.deepEqual(addLabel.mock.calls[0].arguments, [42, ASSIGNED_LABEL]);
|
||||
assert.deepEqual(removeLabel.mock.calls[0].arguments, [42, AUTO_ASSIGN_LABEL]);
|
||||
});
|
||||
|
||||
it('is a no-op when the trigger label is absent', async () => {
|
||||
readPrLabelsImpl = () => ['bug'];
|
||||
|
||||
await run(42);
|
||||
|
||||
assert.equal(requestTeamReviewers.mock.calls.length, 0);
|
||||
assert.equal(addLabel.mock.calls.length, 0);
|
||||
assert.equal(removeLabel.mock.calls.length, 0);
|
||||
});
|
||||
|
||||
it('is a no-op when the done-marker label is already present', async () => {
|
||||
readPrLabelsImpl = () => [AUTO_ASSIGN_LABEL, ASSIGNED_LABEL];
|
||||
|
||||
await run(42);
|
||||
|
||||
assert.equal(requestTeamReviewers.mock.calls.length, 0);
|
||||
assert.equal(addLabel.mock.calls.length, 0);
|
||||
assert.equal(removeLabel.mock.calls.length, 0);
|
||||
});
|
||||
|
||||
it('does not request reviewers or relabel when no team owns a changed file', async () => {
|
||||
ownershipsToAllocationsImpl = () => [];
|
||||
|
||||
await run(42);
|
||||
|
||||
assert.equal(requestTeamReviewers.mock.calls.length, 0);
|
||||
assert.equal(addLabel.mock.calls.length, 0);
|
||||
assert.equal(removeLabel.mock.calls.length, 0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,60 @@
|
||||
name: 'CI: Owners Assign Reviewers'
|
||||
|
||||
# Opt-in reviewer auto-assignment.
|
||||
#
|
||||
# When a PR carries the `Auto-assign reviewers` label, requests review from
|
||||
# every team that owns a changed file (based on .github/OWNERS). Without the
|
||||
# label this is a no-op — opting in is entirely up to the PR author.
|
||||
#
|
||||
# Assignment runs once: afterwards the trigger label is swapped for a
|
||||
# `Reviewers auto-assigned` marker, and any PR already carrying that marker is
|
||||
# skipped. This keeps later `synchronize` events from re-requesting the teams.
|
||||
#
|
||||
# Runs on `labeled` (so adding the label assigns immediately) and on
|
||||
# `synchronize` (so newly-touched files still get assigned before the marker
|
||||
# lands). The label checks live in the script, so other PRs exit early.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types:
|
||||
- labeled
|
||||
- synchronize
|
||||
- reopened
|
||||
branches:
|
||||
- master
|
||||
|
||||
jobs:
|
||||
assign-reviewers:
|
||||
name: Assign reviewers
|
||||
# Skipped for:
|
||||
# - PRs from forks (no token to assign with)
|
||||
# - Bot-authored PRs (Dependabot, Renovate, etc.)
|
||||
if: |
|
||||
github.event.pull_request.head.repo.full_name == github.repository &&
|
||||
github.event.pull_request.user.type != 'Bot'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
steps:
|
||||
- name: Generate App Token
|
||||
id: app-token
|
||||
uses: actions/create-github-app-token@29824e69f54612133e76f7eaac726eef6c875baf # v2.2.1
|
||||
with:
|
||||
app-id: ${{ secrets.N8N_ASSISTANT_APP_ID }}
|
||||
private-key: ${{ secrets.N8N_ASSISTANT_PRIVATE_KEY }}
|
||||
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Setup Node.js
|
||||
uses: ./.github/actions/setup-nodejs
|
||||
with:
|
||||
build-command: ''
|
||||
install-command: pnpm install --frozen-lockfile --dir ./.github/scripts --ignore-workspace
|
||||
|
||||
- name: Assign reviewer teams
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
|
||||
run: node .github/scripts/owners-assign-reviewers.mjs
|
||||
Reference in New Issue
Block a user