mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-11 22:50:06 +00:00
ci: Replace CODEOWNERS with required-review enforcement in OWNERS (#37120)
Co-authored-by: Claude Fable 5 <[email protected]>
This commit is contained in:
Charlie Kolb
co-authored by
Claude Fable 5
Claude Fable 5
parent
086739d160
commit
6de8eacbc6
+1
-1
@@ -13,7 +13,7 @@ This folder contains n8n's GitHub Actions infrastructure.
|
||||
| `workflows/` | GitHub Actions workflows |
|
||||
| `actions/` | Reusable composite actions |
|
||||
| `scripts/` | Release & Docker automation |
|
||||
| `CODEOWNERS` | Team review ownership |
|
||||
| `scripts/owners/` | Team review ownership scripts; the OWNERS file is at the repo root (`required` entries gate merges) |
|
||||
|
||||
### Workflow Naming
|
||||
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
# Temporary: kept side by side with the top-level OWNERS file while the
|
||||
# "Required Reviews" enforcement is on trial. Do not add entries here;
|
||||
# add them to OWNERS. See "Transition from CODEOWNERS" in WORKFLOWS.md.
|
||||
packages/@n8n/db/src/migrations/ @n8n-io/migrations-review
|
||||
/OWNERS @n8n-io/qa-dx
|
||||
.github/workflows @n8n-io/qa-dx
|
||||
.github/scripts @n8n-io/qa-dx
|
||||
.github/actions @n8n-io/qa-dx
|
||||
|
||||
+57
-6
@@ -10,7 +10,7 @@ Complete reference for n8n's `.github/` folder.
|
||||
.github/
|
||||
├── WORKFLOWS.md # This document
|
||||
├── CI-TELEMETRY.md # Telemetry & metrics guide
|
||||
├── CODEOWNERS # Team ownership for PR reviews
|
||||
├── CODEOWNERS # Temporary, side by side with OWNERS during the trial
|
||||
├── pull_request_template.md # PR description template
|
||||
├── pull_request_title_conventions.md # Title format rules (Angular)
|
||||
├── actionlint.yml # Workflow linter config
|
||||
@@ -21,6 +21,7 @@ Complete reference for n8n's `.github/` folder.
|
||||
│ ├── config.yml # Routes to community/security
|
||||
│ └── 01-bug.yml # Structured bug report form
|
||||
├── scripts/ # Automation scripts
|
||||
│ ├── owners/ # Owners scripts (the OWNERS file lives at the repo root)
|
||||
│ ├── bump-versions.mjs # Calculate next version
|
||||
│ ├── update-changelog.mjs # Generate CHANGELOG
|
||||
│ ├── trim-fe-packageJson.js # Strip frontend devDeps
|
||||
@@ -603,13 +604,63 @@ See **[CI-TELEMETRY.md](CI-TELEMETRY.md)** for:
|
||||
|
||||
---
|
||||
|
||||
## CODEOWNERS
|
||||
## OWNERS
|
||||
|
||||
Team ownership mappings in `CODEOWNERS`:
|
||||
Team ownership lives in the top-level `OWNERS` file (this replaces the
|
||||
GitHub-native `CODEOWNERS` file; see the transition note below). The scripts
|
||||
that consume it live in `.github/scripts/owners/`. Line format:
|
||||
|
||||
| Path Pattern | Team |
|
||||
|--------------------------------------------------------------|----------------------------|
|
||||
| `packages/@n8n/db/src/migrations/` | @n8n-io/migrations-review |
|
||||
```
|
||||
<pattern> <@org/team> [required]
|
||||
```
|
||||
|
||||
Patterns are a catch-all (`*`), a directory prefix (`packages/x/`), or an
|
||||
exact file path. Matching is last-match-wins, so specific rules must come
|
||||
after general rules.
|
||||
|
||||
The format is strict, enforced by `node .github/scripts/owners/owners.mjs --check`:
|
||||
|
||||
- Tokens on a line come in a fixed order: pattern, one team, then options
|
||||
such as `required`.
|
||||
- Directory patterns end with `/` and must be existing directories; all other
|
||||
patterns must be existing files. Duplicate patterns are rejected.
|
||||
|
||||
Team existence is not checked by `--check` (it needs an org read token, which
|
||||
fork PRs do not have); a separate workflow covers it (DEVP-891).
|
||||
|
||||
The file drives four workflows:
|
||||
|
||||
| Workflow | Purpose |
|
||||
|-----------------------------------|--------------------------------------------------------------------------|
|
||||
| `ci-owners-validation.yml` | Validates OWNERS (syntax, dead paths) via `owners.mjs --check` |
|
||||
| `ci-owners-review-recommendations.yml` | Advisory PR comment: reviewer teams, line stats, required reviews |
|
||||
| `ci-owners-assign-reviewers.yml` | Opt-in reviewer auto-assignment (label-triggered) |
|
||||
| `ci-owners-required-reviews.yml` | Enforces `required` entries via the "Required Reviews" commit status |
|
||||
|
||||
### Required reviews
|
||||
|
||||
An entry with the `required` option makes team approval mandatory: when a PR
|
||||
changes a file whose winning entry carries `required`, a member of each listed
|
||||
team must approve the PR. `ci-owners-required-reviews.yml` evaluates this on
|
||||
PR changes and review events, and reports a commit status
|
||||
named **Required Reviews** on the head SHA. The ruleset for `master` must list
|
||||
that status as a required check for the block to take effect. Merge-queue runs
|
||||
report success on the queue head without re-evaluating: a PR cannot enter the
|
||||
queue unless the status is green on its head, and the queue does not change
|
||||
approvals.
|
||||
|
||||
The workflow reads OWNERS and its scripts from the base branch only, so a PR
|
||||
cannot lift its own review requirement.
|
||||
|
||||
### Transition from CODEOWNERS
|
||||
|
||||
During a trial period, `.github/CODEOWNERS` stays in place next to OWNERS:
|
||||
GitHub's native code-owner enforcement keeps gating merges while the
|
||||
"Required Reviews" status runs side by side. The two must agree — CODEOWNERS
|
||||
holds exactly the `required` entries of OWNERS (plus the OWNERS file itself)
|
||||
and must not gain new entries; new ownership goes into OWNERS. After the
|
||||
trial, delete `.github/CODEOWNERS`, remove "Require review from Code Owners"
|
||||
from the master ruleset, and delete this section (tracked in DEVP-887).
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -424,6 +424,73 @@ export async function getPrFiles(pullRequestNumber) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all reviews submitted on a PR, in submission order.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @returns { Promise<Array<{ user: { login: string } | null, state: string, submitted_at?: string }>> }
|
||||
* */
|
||||
export async function getPrReviews(pullRequestNumber) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
return await octokit.paginate(octokit.rest.pulls.listReviews, {
|
||||
owner,
|
||||
repo,
|
||||
pull_number: pullRequestNumber,
|
||||
per_page: 100,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Test whether a user is an active member of an org team.
|
||||
*
|
||||
* Team slugs are the part after the org, e.g. `catalysts` for
|
||||
* `@n8n-io/catalysts`. Requires a token with org members read access
|
||||
* (the plain GITHUB_TOKEN cannot read team membership). Returns false
|
||||
* for pending invitations and for teams that do not exist.
|
||||
*
|
||||
* @param { string } teamSlug
|
||||
* @param { string } username
|
||||
* @returns { Promise<boolean> }
|
||||
* */
|
||||
export async function isTeamMember(teamSlug, username) {
|
||||
const { octokit, owner } = initGithub();
|
||||
|
||||
try {
|
||||
const { data } = await octokit.rest.teams.getMembershipForUserInOrg({
|
||||
org: owner,
|
||||
team_slug: teamSlug,
|
||||
username,
|
||||
});
|
||||
return data.state === 'active';
|
||||
} catch (ex) {
|
||||
if (ex?.status === 404) return false;
|
||||
throw ex;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create (or overwrite) a commit status on the given SHA. A ruleset can list
|
||||
* the status context as a required check to gate merges on it.
|
||||
*
|
||||
* @param { string } sha
|
||||
* @param {{ state: 'success' | 'failure' | 'pending' | 'error', context: string, description: string, targetUrl?: string }} status
|
||||
*/
|
||||
export async function setCommitStatus(sha, { state, context, description, targetUrl }) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
await octokit.rest.repos.createCommitStatus({
|
||||
owner,
|
||||
repo,
|
||||
sha,
|
||||
state,
|
||||
context,
|
||||
// The API rejects descriptions longer than 140 characters.
|
||||
description: description.length > 140 ? `${description.slice(0, 139)}…` : description,
|
||||
target_url: targetUrl,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Post a PR comment, or update the existing one if a previous run already
|
||||
* left one identified by the provided bot marker.
|
||||
|
||||
@@ -1,160 +0,0 @@
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
/**
|
||||
* @typedef Owner
|
||||
* @property { string } filepath
|
||||
* @property { string } team
|
||||
* */
|
||||
|
||||
/**
|
||||
* @typedef { Map<string, string[]> } Ownerships
|
||||
* */
|
||||
|
||||
/**
|
||||
* @typedef Allocation
|
||||
* @property { string } team
|
||||
* @property { number } fileCount
|
||||
* */
|
||||
|
||||
// Resolve relative to this file so the path works regardless of cwd
|
||||
// (workflow runs from repo root; `npm test` runs from .github/scripts).
|
||||
export const OWNERS_FILE = join(import.meta.dirname, "..", "OWNERS");
|
||||
|
||||
/**
|
||||
* Parse OWNERS file content into Owner records. Lines without an `@n8n-io/*`
|
||||
* team are skipped.
|
||||
*
|
||||
* @param { string } content
|
||||
* @returns { Owner[] }
|
||||
* */
|
||||
export function parseOwnersContent(content) {
|
||||
return content.split("\n")
|
||||
.filter(line => line.includes("@n8n-io"))
|
||||
.map(line => ({
|
||||
filepath: line.match(/^\S+/)?.at(0),
|
||||
team: line.match(/@n8n-io\/.*/)?.at(0)
|
||||
}))
|
||||
.filter(/** @returns { owner is Owner } */ (owner) =>
|
||||
owner.filepath !== undefined && owner.team !== undefined
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Read and parse the .github/OWNERS file.
|
||||
*
|
||||
* @param { string } [path] Optional override; defaults to OWNERS_FILE.
|
||||
* @returns { Owner[] }
|
||||
* */
|
||||
export function parseOwnersFile(path = OWNERS_FILE) {
|
||||
const content = readFileSync(path, "utf8");
|
||||
return parseOwnersContent(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test whether `file` is matched by a CODEOWNERS-style pattern.
|
||||
*
|
||||
* The OWNERS file uses three pattern shapes, all handled here:
|
||||
* "*" catch-all (matches any file)
|
||||
* "packages/x/" directory pattern (matches every file under packages/x/ recursively)
|
||||
* "path/to/f.ts" exact path
|
||||
*
|
||||
* If richer globs are ever introduced to OWNERS (e.g. `*.ts`, `**\/foo`),
|
||||
* extend this helper rather than reaching for a dependency.
|
||||
*
|
||||
* @param { string } file
|
||||
* @param { string } pattern
|
||||
* @returns { boolean }
|
||||
* */
|
||||
export function matchesPattern(file, pattern) {
|
||||
if (pattern === "*") return true;
|
||||
if (pattern.endsWith("/")) return file.startsWith(pattern);
|
||||
return file === pattern;
|
||||
}
|
||||
|
||||
/**
|
||||
* Map each changed file to the team that owns it, applying CODEOWNERS
|
||||
* last-match-wins semantics. Files that match no rule are omitted.
|
||||
*
|
||||
* @param { Set<string> } files
|
||||
* @param { Owner[] } owners
|
||||
* @returns { Ownerships } team -> files it owns in this changeset
|
||||
* */
|
||||
export function assignOwnership(files, owners) {
|
||||
/** @type { Ownerships } */
|
||||
const teamToFiles = new Map();
|
||||
|
||||
for (const file of files) {
|
||||
// Walk rules in reverse so the *last* matching rule wins.
|
||||
for (let i = owners.length - 1; i >= 0; i--) {
|
||||
if (matchesPattern(file, owners[i].filepath)) {
|
||||
const team = owners[i].team;
|
||||
const bucket = teamToFiles.get(team);
|
||||
|
||||
if (bucket) {
|
||||
bucket.push(file);
|
||||
} else {
|
||||
teamToFiles.set(team, [file]);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return teamToFiles;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { Ownerships } ownerships
|
||||
* @returns { Allocation[] }
|
||||
* */
|
||||
export function ownershipsToAllocations(ownerships) {
|
||||
return Array.from(ownerships).map(([team, files]) => ({
|
||||
team,
|
||||
fileCount: files.length,
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a newline-delimited list of changed file paths from disk.
|
||||
* Empty/whitespace-only lines are skipped.
|
||||
*
|
||||
* @param { string } path
|
||||
* @returns { Set<string> }
|
||||
* */
|
||||
export function readChangedFilesList(path) {
|
||||
return new Set(
|
||||
readFileSync(path, "utf8")
|
||||
.split("\n")
|
||||
.map(line => line.trim())
|
||||
.filter(Boolean)
|
||||
);
|
||||
}
|
||||
|
||||
// CLI: `node owners.mjs <changed-files-list>`
|
||||
// Reads the given file (one changed path per line), runs ownership
|
||||
// allocation against .github/OWNERS. Prints out an object with ownerships for files
|
||||
// and the ownership counts per team as JSON on stdout.
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
const path = process.argv[2];
|
||||
if (!path) {
|
||||
console.error("Usage: node owners.mjs <changed-files-list>");
|
||||
console.error(" <changed-files-list>: path to a file containing one changed path per line");
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const files = readChangedFilesList(path);
|
||||
const ownerships = assignOwnership(files, parseOwnersFile());
|
||||
const totalFiles = files.size;
|
||||
|
||||
const allocations = Array.from(ownerships)
|
||||
.map(([team, ownedFiles]) => ({
|
||||
team,
|
||||
fileCount: ownedFiles.length,
|
||||
share: totalFiles === 0 ? 0 : Math.round((ownedFiles.length / totalFiles) * 100),
|
||||
files: ownedFiles,
|
||||
}))
|
||||
.sort((a, b) => b.fileCount - a.fileCount);
|
||||
|
||||
console.log(JSON.stringify({ totalFiles, allocations }, null, 4));
|
||||
}
|
||||
@@ -1,173 +0,0 @@
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
assignOwnership,
|
||||
ownershipsToAllocations,
|
||||
parseOwnersContent,
|
||||
parseOwnersFile,
|
||||
} from './owners.mjs';
|
||||
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test ./.github/scripts/owners.test.mjs
|
||||
* */
|
||||
|
||||
describe('parseOwnersContent', () => {
|
||||
it('parses well-formed OWNERS lines into Owner records', () => {
|
||||
const content = [
|
||||
'# header comment',
|
||||
'',
|
||||
'* @n8n-io/catalysts',
|
||||
'packages/cli/ @n8n-io/cli-team',
|
||||
'packages/foo/bar.ts @n8n-io/some-team',
|
||||
].join('\n');
|
||||
|
||||
assert.deepEqual(parseOwnersContent(content), [
|
||||
{ filepath: '*', team: '@n8n-io/catalysts' },
|
||||
{ filepath: 'packages/cli/', team: '@n8n-io/cli-team' },
|
||||
{ filepath: 'packages/foo/bar.ts', team: '@n8n-io/some-team' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('skips lines without an @n8n-io team', () => {
|
||||
const content = [
|
||||
'# comment',
|
||||
'* @other-org/team',
|
||||
'pkg/ @n8n-io/keepers',
|
||||
'',
|
||||
].join('\n');
|
||||
|
||||
assert.deepEqual(parseOwnersContent(content), [
|
||||
{ filepath: 'pkg/', team: '@n8n-io/keepers' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns an empty array when no lines reference @n8n-io', () => {
|
||||
assert.deepEqual(parseOwnersContent('# nothing here\n* @someone-else'), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseOwnersFile', () => {
|
||||
it('reads the real OWNERS file into well-formed Owner records', () => {
|
||||
const owners = parseOwnersFile();
|
||||
|
||||
assert.ok(owners.length > 0, 'OWNERS file should not be empty');
|
||||
assert.ok(
|
||||
owners.every(o => o.team && o.filepath),
|
||||
'every parsed entry should have both team and filepath',
|
||||
);
|
||||
assert.ok(
|
||||
owners.every(o => o.team?.startsWith('@n8n-io/')),
|
||||
'every parsed team should belong to the @n8n-io org',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('assignOwnership', () => {
|
||||
it('assigns every file to the catch-all team when only `*` is defined', () => {
|
||||
const files = new Set(['a.ts', 'packages/cli/src/index.ts', 'docs/readme.md']);
|
||||
const owners = [{ filepath: '*', team: '@n8n-io/catalysts' }];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/catalysts')?.sort(),
|
||||
[...files].sort(),
|
||||
);
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('applies last-match-wins: a later specific rule overrides the catch-all', () => {
|
||||
const files = new Set([
|
||||
'README.md',
|
||||
'packages/cli/src/index.ts',
|
||||
'packages/cli/src/lib/foo.ts',
|
||||
]);
|
||||
const owners = [
|
||||
{ filepath: '*', team: '@n8n-io/catalysts' },
|
||||
{ filepath: 'packages/cli/', team: '@n8n-io/cli-team' },
|
||||
];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/catalysts'), ['README.md']);
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/cli-team')?.sort(),
|
||||
['packages/cli/src/index.ts', 'packages/cli/src/lib/foo.ts'].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('matches a directory pattern recursively', () => {
|
||||
const files = new Set([
|
||||
'packages/cli/src/deep/nested/file.ts',
|
||||
'packages/cli/package.json',
|
||||
]);
|
||||
const owners = [{ filepath: 'packages/cli/', team: '@n8n-io/cli-team' }];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/cli-team')?.sort(),
|
||||
[...files].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('matches an exact file pattern only against that file', () => {
|
||||
const files = new Set([
|
||||
'packages/cli/src/controllers/ai.controller.ts',
|
||||
'packages/cli/src/controllers/other.controller.ts',
|
||||
]);
|
||||
const owners = [
|
||||
{
|
||||
filepath: 'packages/cli/src/controllers/ai.controller.ts',
|
||||
team: '@n8n-io/ai-team',
|
||||
},
|
||||
];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/ai-team'), [
|
||||
'packages/cli/src/controllers/ai.controller.ts',
|
||||
]);
|
||||
// the other controller matched no rule, so it must be omitted entirely
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('omits files that match no rule (no catch-all present)', () => {
|
||||
const files = new Set(['unowned/file.ts', 'packages/cli/src/x.ts']);
|
||||
const owners = [{ filepath: 'packages/cli/', team: '@n8n-io/cli-team' }];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/cli-team'), ['packages/cli/src/x.ts']);
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('returns an empty Map when there are no changed files', () => {
|
||||
const owners = [{ filepath: '*', team: '@n8n-io/catalysts' }];
|
||||
const result = assignOwnership(new Set(), owners);
|
||||
|
||||
assert.equal(result.size, 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ownershipsToAllocations', () => {
|
||||
it('converts a Map of team -> files into Allocation[] with fileCount', () => {
|
||||
const ownerships = new Map([
|
||||
['@n8n-io/cli-team', ['a.ts', 'b.ts', 'c.ts']],
|
||||
['@n8n-io/catalysts', ['README.md']],
|
||||
]);
|
||||
|
||||
const result = ownershipsToAllocations(ownerships);
|
||||
|
||||
assert.deepEqual(result, [
|
||||
{ team: '@n8n-io/cli-team', fileCount: 3 },
|
||||
{ team: '@n8n-io/catalysts', fileCount: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns an empty array for an empty Map', () => {
|
||||
assert.deepEqual(ownershipsToAllocations(new Map()), []);
|
||||
});
|
||||
});
|
||||
+11
-15
@@ -2,7 +2,7 @@
|
||||
* 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
|
||||
* every team that owns a changed file (per 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.
|
||||
*
|
||||
@@ -11,7 +11,7 @@
|
||||
* 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*
|
||||
* Complements `review-recommendations.mjs`, which only *suggests*
|
||||
* reviewer teams in a comment — this one actually requests them.
|
||||
*/
|
||||
|
||||
@@ -22,23 +22,19 @@ import {
|
||||
readPrLabels,
|
||||
removeLabel,
|
||||
requestTeamReviewers,
|
||||
} from './github-helpers.mjs';
|
||||
import { assignOwnership, ownershipsToAllocations, parseOwnersFile } from './owners.mjs';
|
||||
} from '../github-helpers.mjs';
|
||||
import {
|
||||
assignOwnership,
|
||||
ownershipsToAllocations,
|
||||
parseOwnersFile,
|
||||
teamHandleToSlug,
|
||||
} from './owners.mjs';
|
||||
|
||||
export { teamHandleToSlug };
|
||||
|
||||
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 }
|
||||
+8
-6
@@ -4,7 +4,7 @@ import assert from 'node:assert/strict';
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners-assign-reviewers.test.mjs
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners/assign-reviewers.test.mjs
|
||||
* */
|
||||
|
||||
/** @type {() => string[]} */
|
||||
@@ -18,7 +18,7 @@ let addLabelImpl = async () => {};
|
||||
/** @type {(pullRequestNumber: number, label: string) => Promise<void>} */
|
||||
let removeLabelImpl = async () => {};
|
||||
|
||||
mock.module('./github-helpers.mjs', {
|
||||
mock.module('../github-helpers.mjs', {
|
||||
namedExports: {
|
||||
ensureEnvVar: () => {}, // no-op in tests
|
||||
readPrLabels: () => readPrLabelsImpl(),
|
||||
@@ -30,9 +30,9 @@ mock.module('./github-helpers.mjs', {
|
||||
},
|
||||
});
|
||||
|
||||
/** @type {() => Array<{ filepath: string, team: string }>} */
|
||||
/** @type {() => Array<import('./owners.mjs').OwnersEntry>} */
|
||||
let parseOwnersFileImpl = () => [];
|
||||
/** @type {(files: Set<string>, owners: Array<{ filepath: string, team: string }>) => Map<string, string[]>} */
|
||||
/** @type {(files: Set<string>, entries: Array<import('./owners.mjs').OwnersEntry>) => Map<string, string[]>} */
|
||||
let assignOwnershipImpl = () => new Map();
|
||||
/** @type {(ownerships: Map<string, string[]>) => Array<{ team: string, fileCount: number }>} */
|
||||
let ownershipsToAllocationsImpl = () => [];
|
||||
@@ -40,8 +40,10 @@ let ownershipsToAllocationsImpl = () => [];
|
||||
mock.module('./owners.mjs', {
|
||||
namedExports: {
|
||||
parseOwnersFile: () => parseOwnersFileImpl(),
|
||||
assignOwnership: (files, owners) => assignOwnershipImpl(files, owners),
|
||||
assignOwnership: (files, entries) => assignOwnershipImpl(files, entries),
|
||||
ownershipsToAllocations: (ownerships) => ownershipsToAllocationsImpl(ownerships),
|
||||
// Mirrors the real implementation; the mock replaces the whole module.
|
||||
teamHandleToSlug: (team) => team.replace(/^@[^/]+\//, ''),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -52,7 +54,7 @@ const {
|
||||
hasAutoAssignLabel,
|
||||
resolveOwnerTeamSlugs,
|
||||
run,
|
||||
} = await import('./owners-assign-reviewers.mjs');
|
||||
} = await import('./assign-reviewers.mjs');
|
||||
|
||||
describe('teamHandleToSlug', () => {
|
||||
it('strips the org prefix from an OWNERS team handle', () => {
|
||||
@@ -0,0 +1,346 @@
|
||||
import { readFileSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
/**
|
||||
* @typedef OwnersEntry
|
||||
* @property { string } pattern
|
||||
* @property { string } team
|
||||
* @property { boolean } required A member of the team must approve before merge.
|
||||
* @property { number } line 1-based line number in the OWNERS file.
|
||||
* */
|
||||
|
||||
/**
|
||||
* @typedef { Map<string, string[]> } Ownerships
|
||||
* */
|
||||
|
||||
/**
|
||||
* @typedef Allocation
|
||||
* @property { string } team
|
||||
* @property { number } fileCount
|
||||
* */
|
||||
|
||||
const REPO_ROOT = join(import.meta.dirname, "..", "..", "..");
|
||||
|
||||
// Resolve relative to this file so the path works regardless of cwd
|
||||
// (workflow runs from repo root; `npm test` runs from .github/scripts).
|
||||
export const OWNERS_FILE = join(REPO_ROOT, "OWNERS");
|
||||
|
||||
// GitHub team handle, e.g. `@n8n-io/catalysts`.
|
||||
const TEAM_TOKEN = /^@[\w.-]+\/[\w.-]+$/;
|
||||
|
||||
/**
|
||||
* Line grammar: `<pattern> <@org/team> [required]`
|
||||
* New option keywords go here.
|
||||
*/
|
||||
const OPTION_TOKENS = new Set(["required"]);
|
||||
|
||||
/**
|
||||
* @param { string } line
|
||||
* @returns { string }
|
||||
*/
|
||||
function stripComment(line) {
|
||||
const commentStart = line.match(/(^|\s)#/);
|
||||
return commentStart ? line.slice(0, commentStart.index) : line;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse OWNERS file content into structured entries.
|
||||
*
|
||||
* Throws on lines that do not follow the grammar, so a malformed OWNERS
|
||||
* file fails loudly in every consumer instead of being silently ignored.
|
||||
*
|
||||
* @param { string } content
|
||||
* @returns { OwnersEntry[] }
|
||||
* */
|
||||
export function parseOwnersContent(content) {
|
||||
/** @type { OwnersEntry[] } */
|
||||
const entries = [];
|
||||
|
||||
content.split("\n").forEach((rawLine, index) => {
|
||||
const lineNumber = index + 1;
|
||||
const line = stripComment(rawLine).trim();
|
||||
if (!line) return;
|
||||
|
||||
const [pattern, ...tokens] = line.split(/\s+/);
|
||||
/** @type { string | null } */
|
||||
let team = null;
|
||||
let required = false;
|
||||
let sawOption = false;
|
||||
|
||||
for (const token of tokens) {
|
||||
if (TEAM_TOKEN.test(token)) {
|
||||
if (sawOption) {
|
||||
throw new Error(`OWNERS line ${lineNumber}: team "${token}" must come before options`);
|
||||
}
|
||||
if (team) {
|
||||
throw new Error(`OWNERS line ${lineNumber}: only one team per pattern is supported`);
|
||||
}
|
||||
team = token;
|
||||
} else if (token === "required") {
|
||||
required = true;
|
||||
sawOption = true;
|
||||
} else if (OPTION_TOKENS.has(token)) {
|
||||
throw new Error(`OWNERS line ${lineNumber}: option "${token}" is not handled`);
|
||||
} else {
|
||||
throw new Error(`OWNERS line ${lineNumber}: unknown token "${token}"`);
|
||||
}
|
||||
}
|
||||
|
||||
if (!team) {
|
||||
throw new Error(`OWNERS line ${lineNumber}: no team for pattern "${pattern}"`);
|
||||
}
|
||||
|
||||
entries.push({ pattern, team, required, line: lineNumber });
|
||||
});
|
||||
|
||||
return entries;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read and parse the OWNERS file.
|
||||
*
|
||||
* @param { string } [path] Optional override; defaults to OWNERS_FILE.
|
||||
* @returns { OwnersEntry[] }
|
||||
* */
|
||||
export function parseOwnersFile(path = OWNERS_FILE) {
|
||||
const content = readFileSync(path, "utf8");
|
||||
return parseOwnersContent(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { string } pattern
|
||||
* @returns { 'file' | 'directory' | null }
|
||||
* */
|
||||
function getPathKind(pattern) {
|
||||
const stats = statSync(join(REPO_ROOT, pattern), { throwIfNoEntry: false });
|
||||
if (!stats) return null;
|
||||
return stats.isDirectory() ? "directory" : "file";
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate parsed OWNERS entries beyond the line grammar:
|
||||
* - no duplicate patterns
|
||||
* - directory patterns (trailing `/`) point at existing directories,
|
||||
* all other patterns (except `*`) at existing files
|
||||
*
|
||||
* @param { OwnersEntry[] } entries
|
||||
* @param { (pattern: string) => 'file' | 'directory' | null } [pathKind]
|
||||
* @returns { string[] } validation errors, empty when the file is valid
|
||||
* */
|
||||
export function validateOwners(entries, pathKind = getPathKind) {
|
||||
const errors = [];
|
||||
/** @type { Map<string, number> } */
|
||||
const seenPatterns = new Map();
|
||||
|
||||
for (const entry of entries) {
|
||||
const firstLine = seenPatterns.get(entry.pattern);
|
||||
if (firstLine !== undefined) {
|
||||
errors.push(`duplicate pattern "${entry.pattern}" (lines ${firstLine} and ${entry.line})`);
|
||||
} else {
|
||||
seenPatterns.set(entry.pattern, entry.line);
|
||||
}
|
||||
|
||||
if (entry.pattern !== "*") {
|
||||
const kind = pathKind(entry.pattern);
|
||||
const expectedKind = entry.pattern.endsWith("/") ? "directory" : "file";
|
||||
|
||||
if (kind === null) {
|
||||
errors.push(`pattern "${entry.pattern}" (line ${entry.line}) does not exist in the repository`);
|
||||
} else if (kind !== expectedKind) {
|
||||
errors.push(
|
||||
kind === "directory"
|
||||
? `pattern "${entry.pattern}" (line ${entry.line}) is a directory; add a trailing "/"`
|
||||
: `pattern "${entry.pattern}" (line ${entry.line}) is a file; remove the trailing "/"`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert an OWNERS team handle (`@n8n-io/catalysts`) into the GitHub team slug
|
||||
* (`catalysts`) expected by the teams API.
|
||||
*
|
||||
* @param { string } team
|
||||
* @returns { string }
|
||||
*/
|
||||
export function teamHandleToSlug(team) {
|
||||
return team.replace(/^@[^/]+\//, "");
|
||||
}
|
||||
|
||||
/**
|
||||
* Test whether `file` is matched by a CODEOWNERS-style pattern.
|
||||
*
|
||||
* The OWNERS file uses three pattern shapes, all handled here:
|
||||
* "*" catch-all (matches any file)
|
||||
* "packages/x/" directory pattern (matches every file under packages/x/ recursively)
|
||||
* "path/to/f.ts" exact path
|
||||
*
|
||||
* If richer globs are ever introduced to OWNERS (e.g. `*.ts`, `**\/foo`),
|
||||
* extend this helper rather than reaching for a dependency.
|
||||
*
|
||||
* @param { string } file
|
||||
* @param { string } pattern
|
||||
* @returns { boolean }
|
||||
* */
|
||||
export function matchesPattern(file, pattern) {
|
||||
if (pattern === "*") return true;
|
||||
if (pattern.endsWith("/")) return file.startsWith(pattern);
|
||||
return file === pattern;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the entry that owns `file`, applying CODEOWNERS last-match-wins
|
||||
* semantics. Returns undefined when no entry matches.
|
||||
*
|
||||
* @param { string } file
|
||||
* @param { OwnersEntry[] } entries
|
||||
* @returns { OwnersEntry | undefined }
|
||||
* */
|
||||
export function findOwningEntry(file, entries) {
|
||||
for (let i = entries.length - 1; i >= 0; i--) {
|
||||
if (matchesPattern(file, entries[i].pattern)) return entries[i];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Map each changed file to the team that owns it, applying CODEOWNERS
|
||||
* last-match-wins semantics. Files that match no rule are omitted.
|
||||
*
|
||||
* @param { Set<string> } files
|
||||
* @param { OwnersEntry[] } entries
|
||||
* @returns { Ownerships } team -> files it owns in this changeset
|
||||
* */
|
||||
export function assignOwnership(files, entries) {
|
||||
/** @type { Ownerships } */
|
||||
const teamToFiles = new Map();
|
||||
|
||||
for (const file of files) {
|
||||
const entry = findOwningEntry(file, entries);
|
||||
if (!entry) continue;
|
||||
|
||||
const bucket = teamToFiles.get(entry.team);
|
||||
|
||||
if (bucket) {
|
||||
bucket.push(file);
|
||||
} else {
|
||||
teamToFiles.set(entry.team, [file]);
|
||||
}
|
||||
}
|
||||
|
||||
return teamToFiles;
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine which teams must approve the changeset: a team is required when
|
||||
* its `required` entry wins (last-match) for a changed file.
|
||||
*
|
||||
* @param { Set<string> } files
|
||||
* @param { OwnersEntry[] } entries
|
||||
* @returns { Ownerships } required team -> files that triggered the requirement
|
||||
* */
|
||||
export function resolveRequiredTeams(files, entries) {
|
||||
/** @type { Ownerships } */
|
||||
const teamToFiles = new Map();
|
||||
|
||||
for (const file of [...files].sort()) {
|
||||
const entry = findOwningEntry(file, entries);
|
||||
if (!entry?.required) continue;
|
||||
|
||||
const bucket = teamToFiles.get(entry.team);
|
||||
|
||||
if (bucket) {
|
||||
bucket.push(file);
|
||||
} else {
|
||||
teamToFiles.set(entry.team, [file]);
|
||||
}
|
||||
}
|
||||
|
||||
return teamToFiles;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { Ownerships } ownerships
|
||||
* @returns { Allocation[] }
|
||||
* */
|
||||
export function ownershipsToAllocations(ownerships) {
|
||||
return Array.from(ownerships).map(([team, files]) => ({
|
||||
team,
|
||||
fileCount: files.length,
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a newline-delimited list of changed file paths from disk.
|
||||
* Empty/whitespace-only lines are skipped.
|
||||
*
|
||||
* @param { string } path
|
||||
* @returns { Set<string> }
|
||||
* */
|
||||
export function readChangedFilesList(path) {
|
||||
return new Set(
|
||||
readFileSync(path, "utf8")
|
||||
.split("\n")
|
||||
.map(line => line.trim())
|
||||
.filter(Boolean)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @returns { never }
|
||||
*/
|
||||
function exitWithUsage() {
|
||||
console.error("Usage: node owners.mjs <changed-files-list> | --check");
|
||||
console.error(" <changed-files-list>: path to a file containing one changed path per line");
|
||||
console.error(" --check: validate the OWNERS file (syntax, duplicates, dead paths)");
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
function runCheck() {
|
||||
/** @type { import('./owners.mjs').OwnersEntry[] } */
|
||||
let entries;
|
||||
try {
|
||||
entries = parseOwnersFile();
|
||||
} catch (error) {
|
||||
console.error(`OWNERS is invalid: ${error.message}`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const errors = validateOwners(entries);
|
||||
if (errors.length > 0) {
|
||||
for (const error of errors) console.error(`OWNERS is invalid: ${error}`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const requiredCount = entries.filter((entry) => entry.required).length;
|
||||
console.log(`OWNERS is valid: ${entries.length} entries, ${requiredCount} with required review.`);
|
||||
}
|
||||
|
||||
// CLI: `node owners.mjs <changed-files-list>` prints ownership allocations for
|
||||
// the given changed paths as JSON. `node owners.mjs --check` validates OWNERS.
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
const arg = process.argv[2];
|
||||
if (!arg) exitWithUsage();
|
||||
|
||||
if (arg === "--check") {
|
||||
runCheck();
|
||||
} else {
|
||||
const files = readChangedFilesList(arg);
|
||||
const ownerships = assignOwnership(files, parseOwnersFile());
|
||||
const totalFiles = files.size;
|
||||
|
||||
const allocations = Array.from(ownerships)
|
||||
.map(([team, ownedFiles]) => ({
|
||||
team,
|
||||
fileCount: ownedFiles.length,
|
||||
share: totalFiles === 0 ? 0 : Math.round((ownedFiles.length / totalFiles) * 100),
|
||||
files: ownedFiles,
|
||||
}))
|
||||
.sort((a, b) => b.fileCount - a.fileCount);
|
||||
|
||||
console.log(JSON.stringify({ totalFiles, allocations }, null, 4));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,336 @@
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
assignOwnership,
|
||||
findOwningEntry,
|
||||
ownershipsToAllocations,
|
||||
parseOwnersContent,
|
||||
parseOwnersFile,
|
||||
resolveRequiredTeams,
|
||||
teamHandleToSlug,
|
||||
validateOwners,
|
||||
} from './owners.mjs';
|
||||
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test ./.github/scripts/owners/owners.test.mjs
|
||||
* */
|
||||
|
||||
/** @param {Partial<import('./owners.mjs').OwnersEntry>} entry */
|
||||
function entry(entry) {
|
||||
return { team: '@n8n-io/some-team', required: false, line: 1, pattern: '*', ...entry };
|
||||
}
|
||||
|
||||
describe('parseOwnersContent', () => {
|
||||
it('parses well-formed OWNERS lines into entries', () => {
|
||||
const content = [
|
||||
'# header comment',
|
||||
'',
|
||||
'* @n8n-io/catalysts',
|
||||
'packages/cli/ @n8n-io/cli-team',
|
||||
'packages/foo/bar.ts @n8n-io/some-team',
|
||||
].join('\n');
|
||||
|
||||
assert.deepEqual(parseOwnersContent(content), [
|
||||
{ pattern: '*', team: '@n8n-io/catalysts', required: false, line: 3 },
|
||||
{ pattern: 'packages/cli/', team: '@n8n-io/cli-team', required: false, line: 4 },
|
||||
{ pattern: 'packages/foo/bar.ts', team: '@n8n-io/some-team', required: false, line: 5 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('parses the `required` option after the team', () => {
|
||||
const entries = parseOwnersContent('pkg/ @n8n-io/keepers required');
|
||||
|
||||
assert.deepEqual(entries, [
|
||||
{ pattern: 'pkg/', team: '@n8n-io/keepers', required: true, line: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('throws on a second team', () => {
|
||||
assert.throws(
|
||||
() => parseOwnersContent('pkg/ @n8n-io/keepers @n8n-io/others'),
|
||||
/OWNERS line 1: only one team per pattern is supported/,
|
||||
);
|
||||
});
|
||||
|
||||
it('ignores inline comments', () => {
|
||||
const entries = parseOwnersContent('pkg/ @n8n-io/keepers # the why');
|
||||
|
||||
assert.equal(entries[0].team, '@n8n-io/keepers');
|
||||
});
|
||||
|
||||
it('throws on an unknown token with the line number', () => {
|
||||
assert.throws(
|
||||
() => parseOwnersContent('# comment\npkg/ @n8n-io/keepers banana'),
|
||||
/OWNERS line 2: unknown token "banana"/,
|
||||
);
|
||||
});
|
||||
|
||||
it('throws when a pattern has no team', () => {
|
||||
assert.throws(() => parseOwnersContent('pkg/ required'), /OWNERS line 1: no team/);
|
||||
});
|
||||
|
||||
it('throws when a team comes after an option', () => {
|
||||
assert.throws(
|
||||
() => parseOwnersContent('pkg/ @n8n-io/keepers required @n8n-io/others'),
|
||||
/OWNERS line 1: team "@n8n-io\/others" must come before options/,
|
||||
);
|
||||
});
|
||||
|
||||
it('returns an empty array for comment-only content', () => {
|
||||
assert.deepEqual(parseOwnersContent('# nothing here\n\n'), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseOwnersFile', () => {
|
||||
it('reads the real OWNERS file into well-formed entries', () => {
|
||||
const entries = parseOwnersFile();
|
||||
|
||||
assert.ok(entries.length > 0, 'OWNERS file should not be empty');
|
||||
assert.ok(
|
||||
entries.every((e) => e.team && e.pattern),
|
||||
'every parsed entry should have both a team and a pattern',
|
||||
);
|
||||
assert.ok(
|
||||
entries.every((e) => e.team.startsWith('@n8n-io/')),
|
||||
'every parsed team should belong to the @n8n-io org',
|
||||
);
|
||||
});
|
||||
|
||||
it('the real OWNERS file passes validation', () => {
|
||||
assert.deepEqual(validateOwners(parseOwnersFile()), []);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateOwners', () => {
|
||||
// Path kind stub matching the pattern shape, for tests about other rules.
|
||||
const kindFromShape = (pattern) => (pattern.endsWith('/') ? 'directory' : 'file');
|
||||
|
||||
it('reports duplicate patterns with both line numbers', () => {
|
||||
const entries = [
|
||||
entry({ pattern: 'pkg/', line: 3 }),
|
||||
entry({ pattern: 'pkg/', line: 7 }),
|
||||
];
|
||||
|
||||
const errors = validateOwners(entries, kindFromShape);
|
||||
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /duplicate pattern "pkg\/" \(lines 3 and 7\)/);
|
||||
});
|
||||
|
||||
it('reports patterns that do not exist, except the catch-all', () => {
|
||||
const entries = [entry({ pattern: '*' }), entry({ pattern: 'gone/', line: 2 })];
|
||||
|
||||
const errors = validateOwners(entries, () => null);
|
||||
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /pattern "gone\/" \(line 2\) does not exist/);
|
||||
});
|
||||
|
||||
it('reports a directory pattern that points at a file', () => {
|
||||
const errors = validateOwners([entry({ pattern: 'pkg/', line: 1 })], () => 'file');
|
||||
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /pattern "pkg\/" \(line 1\) is a file; remove the trailing "\/"/);
|
||||
});
|
||||
|
||||
it('reports a file pattern that points at a directory', () => {
|
||||
const errors = validateOwners([entry({ pattern: 'pkg', line: 1 })], () => 'directory');
|
||||
|
||||
assert.equal(errors.length, 1);
|
||||
assert.match(errors[0], /pattern "pkg" \(line 1\) is a directory; add a trailing "\/"/);
|
||||
});
|
||||
|
||||
it('returns no errors for a valid set of entries', () => {
|
||||
const entries = [
|
||||
entry({ pattern: '*' }),
|
||||
entry({ pattern: 'pkg/', line: 2 }),
|
||||
entry({ pattern: 'pkg/a.ts', line: 3 }),
|
||||
];
|
||||
|
||||
assert.deepEqual(validateOwners(entries, kindFromShape), []);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
describe('findOwningEntry', () => {
|
||||
it('returns the last matching entry', () => {
|
||||
const entries = [
|
||||
entry({ pattern: '*', team: '@n8n-io/catalysts' }),
|
||||
entry({ pattern: 'pkg/', team: '@n8n-io/keepers' }),
|
||||
];
|
||||
|
||||
assert.equal(findOwningEntry('pkg/a.ts', entries), entries[1]);
|
||||
assert.equal(findOwningEntry('other.ts', entries), entries[0]);
|
||||
});
|
||||
|
||||
it('returns undefined when nothing matches', () => {
|
||||
assert.equal(findOwningEntry('a.ts', [entry({ pattern: 'pkg/' })]), undefined);
|
||||
});
|
||||
});
|
||||
|
||||
describe('assignOwnership', () => {
|
||||
it('assigns every file to the catch-all team when only `*` is defined', () => {
|
||||
const files = new Set(['a.ts', 'packages/cli/src/index.ts', 'docs/readme.md']);
|
||||
const owners = [entry({ pattern: '*', team: '@n8n-io/catalysts' })];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/catalysts')?.sort(),
|
||||
[...files].sort(),
|
||||
);
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('applies last-match-wins: a later specific rule overrides the catch-all', () => {
|
||||
const files = new Set([
|
||||
'README.md',
|
||||
'packages/cli/src/index.ts',
|
||||
'packages/cli/src/lib/foo.ts',
|
||||
]);
|
||||
const owners = [
|
||||
entry({ pattern: '*', team: '@n8n-io/catalysts' }),
|
||||
entry({ pattern: 'packages/cli/', team: '@n8n-io/cli-team' }),
|
||||
];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/catalysts'), ['README.md']);
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/cli-team')?.sort(),
|
||||
['packages/cli/src/index.ts', 'packages/cli/src/lib/foo.ts'].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('matches a directory pattern recursively', () => {
|
||||
const files = new Set([
|
||||
'packages/cli/src/deep/nested/file.ts',
|
||||
'packages/cli/package.json',
|
||||
]);
|
||||
const owners = [entry({ pattern: 'packages/cli/', team: '@n8n-io/cli-team' })];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(
|
||||
result.get('@n8n-io/cli-team')?.sort(),
|
||||
[...files].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('matches an exact file pattern only against that file', () => {
|
||||
const files = new Set([
|
||||
'packages/cli/src/controllers/ai.controller.ts',
|
||||
'packages/cli/src/controllers/other.controller.ts',
|
||||
]);
|
||||
const owners = [
|
||||
entry({
|
||||
pattern: 'packages/cli/src/controllers/ai.controller.ts',
|
||||
team: '@n8n-io/ai-team',
|
||||
}),
|
||||
];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/ai-team'), [
|
||||
'packages/cli/src/controllers/ai.controller.ts',
|
||||
]);
|
||||
// the other controller matched no rule, so it must be omitted entirely
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('omits files that match no rule (no catch-all present)', () => {
|
||||
const files = new Set(['unowned/file.ts', 'packages/cli/src/x.ts']);
|
||||
const owners = [entry({ pattern: 'packages/cli/', team: '@n8n-io/cli-team' })];
|
||||
|
||||
const result = assignOwnership(files, owners);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/cli-team'), ['packages/cli/src/x.ts']);
|
||||
assert.equal(result.size, 1);
|
||||
});
|
||||
|
||||
it('returns an empty Map when there are no changed files', () => {
|
||||
const owners = [entry({ pattern: '*', team: '@n8n-io/catalysts' })];
|
||||
const result = assignOwnership(new Set(), owners);
|
||||
|
||||
assert.equal(result.size, 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveRequiredTeams', () => {
|
||||
const owners = [
|
||||
entry({ pattern: '*', team: '@n8n-io/catalysts' }),
|
||||
entry({ pattern: '.github/', team: '@n8n-io/qa-dx' }),
|
||||
entry({ pattern: '.github/workflows/', team: '@n8n-io/qa-dx', required: true }),
|
||||
entry({ pattern: 'db/migrations/', team: '@n8n-io/migrations-review', required: true }),
|
||||
];
|
||||
|
||||
it('collects the teams of `required` entries that win for a changed file', () => {
|
||||
const result = resolveRequiredTeams(
|
||||
new Set(['.github/workflows/ci.yml', 'db/migrations/1-init.ts', 'src/a.ts']),
|
||||
owners,
|
||||
);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/qa-dx'), ['.github/workflows/ci.yml']);
|
||||
assert.deepEqual(result.get('@n8n-io/migrations-review'), ['db/migrations/1-init.ts']);
|
||||
assert.equal(result.size, 2);
|
||||
});
|
||||
|
||||
it('ignores files whose winning entry is not required', () => {
|
||||
const result = resolveRequiredTeams(new Set(['OWNERS', 'src/a.ts']), owners);
|
||||
|
||||
assert.equal(result.size, 0);
|
||||
});
|
||||
|
||||
it('a later non-required entry overrides an earlier required one', () => {
|
||||
const result = resolveRequiredTeams(new Set(['db/migrations/1-init.ts']), [
|
||||
entry({ pattern: 'db/', team: '@n8n-io/migrations-review', required: true }),
|
||||
entry({ pattern: 'db/migrations/', team: '@n8n-io/catalysts' }),
|
||||
]);
|
||||
|
||||
assert.equal(result.size, 0);
|
||||
});
|
||||
|
||||
it('lists files deterministically (sorted) per team', () => {
|
||||
const result = resolveRequiredTeams(
|
||||
new Set(['.github/workflows/z.yml', '.github/workflows/a.yml']),
|
||||
owners,
|
||||
);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/qa-dx'), [
|
||||
'.github/workflows/a.yml',
|
||||
'.github/workflows/z.yml',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ownershipsToAllocations', () => {
|
||||
it('converts a Map of team -> files into Allocation[] with fileCount', () => {
|
||||
const ownerships = new Map([
|
||||
['@n8n-io/cli-team', ['a.ts', 'b.ts', 'c.ts']],
|
||||
['@n8n-io/catalysts', ['README.md']],
|
||||
]);
|
||||
|
||||
const result = ownershipsToAllocations(ownerships);
|
||||
|
||||
assert.deepEqual(result, [
|
||||
{ team: '@n8n-io/cli-team', fileCount: 3 },
|
||||
{ team: '@n8n-io/catalysts', fileCount: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns an empty array for an empty Map', () => {
|
||||
assert.deepEqual(ownershipsToAllocations(new Map()), []);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,213 @@
|
||||
/**
|
||||
* Required-review enforcement for OWNERS entries marked `required`.
|
||||
*
|
||||
* Computes the teams whose approval the changeset needs (per OWNERS)
|
||||
* and reports the verdict as a "Required Reviews" commit status on the PR
|
||||
* head SHA. A ruleset that lists this status as a required check blocks the
|
||||
* merge until a member of each required team has approved the PR. Merge-queue
|
||||
* runs do not reach this script: the workflow reports success on the queue
|
||||
* head directly, because entering the queue already required a green status.
|
||||
*
|
||||
* The job itself always succeeds when the evaluation runs; the commit status
|
||||
* carries the verdict. The status is set to pending before the evaluation
|
||||
* starts, so a crash (e.g. the team membership API is unavailable) cannot
|
||||
* leave an earlier green status in effect — the gate fails closed.
|
||||
*/
|
||||
|
||||
import {
|
||||
getChangedFiles,
|
||||
getEventFromGithubEventPath,
|
||||
getPrReviews,
|
||||
getPullRequestById,
|
||||
isTeamMember,
|
||||
setCommitStatus,
|
||||
} from '../github-helpers.mjs';
|
||||
import { parseOwnersFile, resolveRequiredTeams, teamHandleToSlug } from './owners.mjs';
|
||||
|
||||
export const STATUS_CONTEXT = 'Required Reviews';
|
||||
const TARGET_BRANCH = 'master';
|
||||
|
||||
/**
|
||||
* A PR review as returned by the reviews API.
|
||||
*
|
||||
* @typedef Review
|
||||
* @property { { login: string } | null } user
|
||||
* @property { string } state
|
||||
* @property { string } [submitted_at]
|
||||
*/
|
||||
|
||||
/**
|
||||
* Resolve the PR number from the triggering event.
|
||||
*
|
||||
* @param { string } eventName
|
||||
* @param { any } event Parsed GITHUB_EVENT_PATH payload.
|
||||
* @param { string | undefined } pullRequestNumberEnv PULL_REQUEST_NUMBER (workflow_dispatch input).
|
||||
* @returns { number }
|
||||
*/
|
||||
export function resolvePullRequestNumber(eventName, event, pullRequestNumberEnv) {
|
||||
if (event?.pull_request?.number) return event.pull_request.number;
|
||||
|
||||
const parsed = parseInt(pullRequestNumberEnv ?? '');
|
||||
if (Number.isNaN(parsed)) {
|
||||
throw new Error(`Cannot resolve a PR number for event "${eventName}"`);
|
||||
}
|
||||
return parsed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reduce a PR's review history to the latest meaningful state per reviewer,
|
||||
* mirroring how GitHub itself treats reviews: a later APPROVED or
|
||||
* CHANGES_REQUESTED (or a dismissal) replaces the reviewer's earlier state;
|
||||
* COMMENTED and PENDING reviews carry no state.
|
||||
*
|
||||
* @param { Review[] } reviews
|
||||
* @returns { Map<string, string> } login -> latest review state
|
||||
*/
|
||||
export function latestReviewStates(reviews) {
|
||||
const sorted = reviews
|
||||
.filter((review) => review.user?.login)
|
||||
.filter((review) => ['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes(review.state))
|
||||
.toSorted((a, b) => new Date(a.submitted_at ?? 0) - new Date(b.submitted_at ?? 0));
|
||||
|
||||
const states = new Map();
|
||||
for (const review of sorted) {
|
||||
states.set(review.user.login, review.state);
|
||||
}
|
||||
return states;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { Review[] } reviews
|
||||
* @returns { Set<string> } logins whose latest review state is APPROVED
|
||||
*/
|
||||
export function collectApprovers(reviews) {
|
||||
return new Set(
|
||||
[...latestReviewStates(reviews)]
|
||||
.filter(([, state]) => state === 'APPROVED')
|
||||
.map(([login]) => login),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { string[] } missingTeams Team handles without an approving member.
|
||||
* @param { number } requiredCount Total number of required teams.
|
||||
* @returns {{ state: 'success' | 'failure', description: string }}
|
||||
*/
|
||||
export function buildStatus(missingTeams, requiredCount) {
|
||||
if (requiredCount === 0) {
|
||||
return { state: 'success', description: 'No team approval is required for these changes' };
|
||||
}
|
||||
|
||||
if (missingTeams.length === 0) {
|
||||
return {
|
||||
state: 'success',
|
||||
description: `All required team approvals are present (${requiredCount} team${requiredCount === 1 ? '' : 's'})`,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
state: 'failure',
|
||||
description: `Missing approval from: ${missingTeams.map(teamHandleToSlug).join(', ')}`,
|
||||
};
|
||||
}
|
||||
|
||||
function statusTargetUrl() {
|
||||
const { GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID } = process.env;
|
||||
if (!GITHUB_SERVER_URL || !GITHUB_REPOSITORY || !GITHUB_RUN_ID) return undefined;
|
||||
return `${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { number } pullRequestNumber
|
||||
* @returns { Promise<{ state: 'success' | 'failure', description: string }> }
|
||||
*/
|
||||
async function evaluateRequiredReviews(pullRequestNumber) {
|
||||
const changedFiles = await getChangedFiles(pullRequestNumber);
|
||||
const requiredTeams = resolveRequiredTeams(changedFiles, parseOwnersFile());
|
||||
|
||||
/** @type { string[] } */
|
||||
const missingTeams = [];
|
||||
|
||||
if (requiredTeams.size > 0) {
|
||||
const approvers = collectApprovers(await getPrReviews(pullRequestNumber));
|
||||
console.log(`Current approvals: ${[...approvers].join(', ') || '(none)'}`);
|
||||
|
||||
for (const [team, files] of requiredTeams) {
|
||||
const slug = teamHandleToSlug(team);
|
||||
// Per-approver membership checks instead of fetching the roster:
|
||||
// approvers are few, teams can be large.
|
||||
const teamApprovers = [];
|
||||
for (const login of approvers) {
|
||||
if (await isTeamMember(slug, login)) teamApprovers.push(login);
|
||||
}
|
||||
const verdict = teamApprovers.length > 0 ? `approved by ${teamApprovers.join(', ')}` : 'approval missing';
|
||||
|
||||
console.log(`${team}: ${verdict} — owns ${files.length} changed file(s):`);
|
||||
for (const file of files) console.log(` - ${file}`);
|
||||
|
||||
if (teamApprovers.length === 0) missingTeams.push(team);
|
||||
}
|
||||
} else {
|
||||
console.log('No changed file matches a `required` OWNERS entry.');
|
||||
}
|
||||
|
||||
return buildStatus(missingTeams, requiredTeams.size);
|
||||
}
|
||||
|
||||
export async function run() {
|
||||
const eventName = process.env.GITHUB_EVENT_NAME ?? 'workflow_dispatch';
|
||||
const event = getEventFromGithubEventPath();
|
||||
|
||||
const pullRequestNumber = resolvePullRequestNumber(
|
||||
eventName,
|
||||
event,
|
||||
process.env.PULL_REQUEST_NUMBER,
|
||||
);
|
||||
const pullRequest = await getPullRequestById(pullRequestNumber);
|
||||
|
||||
if (pullRequest.base.ref !== TARGET_BRANCH) {
|
||||
console.log(`PR #${pullRequestNumber} targets "${pullRequest.base.ref}", not "${TARGET_BRANCH}"; skipping.`);
|
||||
return;
|
||||
}
|
||||
|
||||
const statusSha = pullRequest.head.sha;
|
||||
const targetUrl = statusTargetUrl();
|
||||
|
||||
// Pending first: it replaces any earlier verdict on this SHA, so a crash
|
||||
// during evaluation cannot leave a stale green status in effect.
|
||||
await setCommitStatus(statusSha, {
|
||||
state: 'pending',
|
||||
context: STATUS_CONTEXT,
|
||||
description: 'Evaluating required reviews',
|
||||
targetUrl,
|
||||
});
|
||||
|
||||
/** @type { { state: 'success' | 'failure', description: string } } */
|
||||
let status;
|
||||
try {
|
||||
status = await evaluateRequiredReviews(pullRequestNumber);
|
||||
} catch (evaluationError) {
|
||||
// Best effort: nicer than a stuck pending status. The pending status
|
||||
// already blocks the merge if this write fails too.
|
||||
try {
|
||||
await setCommitStatus(statusSha, {
|
||||
state: 'error',
|
||||
context: STATUS_CONTEXT,
|
||||
description: 'Evaluation failed; see the workflow run',
|
||||
targetUrl,
|
||||
});
|
||||
} catch {}
|
||||
throw evaluationError;
|
||||
}
|
||||
|
||||
console.log(`Setting "${STATUS_CONTEXT}" on ${statusSha} to ${status.state}: ${status.description}`);
|
||||
await setCommitStatus(statusSha, {
|
||||
...status,
|
||||
context: STATUS_CONTEXT,
|
||||
targetUrl,
|
||||
});
|
||||
}
|
||||
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
await run();
|
||||
}
|
||||
@@ -0,0 +1,281 @@
|
||||
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/required-reviews.test.mjs
|
||||
* */
|
||||
|
||||
/** @type {() => any} */
|
||||
let eventImpl = () => ({});
|
||||
/** @type {(pullRequestNumber: number) => Promise<any>} */
|
||||
let getPullRequestByIdImpl = async () => ({});
|
||||
/** @type {(pullRequestNumber: number) => Promise<Set<string>>} */
|
||||
let getChangedFilesImpl = async () => new Set();
|
||||
/** @type {(pullRequestNumber: number) => Promise<any[]>} */
|
||||
let getPrReviewsImpl = async () => [];
|
||||
/** @type {(teamSlug: string, username: string) => Promise<boolean>} */
|
||||
let isTeamMemberImpl = async () => false;
|
||||
/** @type {(sha: string, status: any) => Promise<void>} */
|
||||
let setCommitStatusImpl = async () => {};
|
||||
|
||||
mock.module('../github-helpers.mjs', {
|
||||
namedExports: {
|
||||
getEventFromGithubEventPath: () => eventImpl(),
|
||||
getPullRequestById: (n) => getPullRequestByIdImpl(n),
|
||||
getChangedFiles: (n) => getChangedFilesImpl(n),
|
||||
getPrReviews: (n) => getPrReviewsImpl(n),
|
||||
isTeamMember: (slug, username) => isTeamMemberImpl(slug, username),
|
||||
setCommitStatus: (sha, status) => setCommitStatusImpl(sha, status),
|
||||
},
|
||||
});
|
||||
|
||||
/** @type {() => Array<import('./owners.mjs').OwnersEntry>} */
|
||||
let parseOwnersFileImpl = () => [];
|
||||
/** @type {(files: Set<string>, entries: any[]) => Map<string, string[]>} */
|
||||
let resolveRequiredTeamsImpl = () => new Map();
|
||||
|
||||
mock.module('./owners.mjs', {
|
||||
namedExports: {
|
||||
parseOwnersFile: () => parseOwnersFileImpl(),
|
||||
resolveRequiredTeams: (files, entries) => resolveRequiredTeamsImpl(files, entries),
|
||||
// Mirrors the real implementation; the mock replaces the whole module.
|
||||
teamHandleToSlug: (team) => team.replace(/^@[^/]+\//, ''),
|
||||
},
|
||||
});
|
||||
|
||||
const {
|
||||
STATUS_CONTEXT,
|
||||
buildStatus,
|
||||
collectApprovers,
|
||||
latestReviewStates,
|
||||
resolvePullRequestNumber,
|
||||
run,
|
||||
} = await import('./required-reviews.mjs');
|
||||
|
||||
describe('resolvePullRequestNumber', () => {
|
||||
it('reads the PR number from pull_request payloads', () => {
|
||||
assert.equal(
|
||||
resolvePullRequestNumber('pull_request', { pull_request: { number: 42 } }, undefined),
|
||||
42,
|
||||
);
|
||||
});
|
||||
|
||||
it('falls back to the PULL_REQUEST_NUMBER env value for workflow_dispatch', () => {
|
||||
assert.equal(resolvePullRequestNumber('workflow_dispatch', {}, '7'), 7);
|
||||
});
|
||||
|
||||
it('throws when no source yields a PR number', () => {
|
||||
assert.throws(() => resolvePullRequestNumber('workflow_dispatch', {}, undefined), /Cannot resolve/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('latestReviewStates', () => {
|
||||
it('keeps only the latest meaningful state per reviewer', () => {
|
||||
const states = latestReviewStates([
|
||||
{ user: { login: 'ana' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
{ user: { login: 'ana' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' },
|
||||
{ user: { login: 'ben' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
{ user: { login: 'ben' }, state: 'DISMISSED', submitted_at: '2026-01-03T00:00:00Z' },
|
||||
]);
|
||||
|
||||
assert.equal(states.get('ana'), 'APPROVED');
|
||||
assert.equal(states.get('ben'), 'DISMISSED');
|
||||
});
|
||||
|
||||
it('ignores COMMENTED and PENDING reviews', () => {
|
||||
const states = latestReviewStates([
|
||||
{ user: { login: 'ana' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
{ user: { login: 'ana' }, state: 'COMMENTED', submitted_at: '2026-01-02T00:00:00Z' },
|
||||
{ user: { login: 'ben' }, state: 'PENDING' },
|
||||
]);
|
||||
|
||||
assert.equal(states.get('ana'), 'APPROVED');
|
||||
assert.equal(states.has('ben'), false);
|
||||
});
|
||||
|
||||
it('sorts by submission time, not input order', () => {
|
||||
const states = latestReviewStates([
|
||||
{ user: { login: 'ana' }, state: 'APPROVED', submitted_at: '2026-01-02T00:00:00Z' },
|
||||
{ user: { login: 'ana' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
]);
|
||||
|
||||
assert.equal(states.get('ana'), 'APPROVED');
|
||||
});
|
||||
|
||||
it('skips reviews without a user', () => {
|
||||
assert.equal(latestReviewStates([{ user: null, state: 'APPROVED' }]).size, 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectApprovers', () => {
|
||||
it('returns reviewers whose latest state is APPROVED', () => {
|
||||
const approvers = collectApprovers([
|
||||
{ user: { login: 'ana' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
{ user: { login: 'ben' }, state: 'CHANGES_REQUESTED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
]);
|
||||
|
||||
assert.deepEqual([...approvers], ['ana']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildStatus', () => {
|
||||
it('succeeds when nothing is required', () => {
|
||||
const status = buildStatus([], 0);
|
||||
|
||||
assert.equal(status.state, 'success');
|
||||
assert.match(status.description, /No team approval is required/);
|
||||
});
|
||||
|
||||
it('succeeds when all required teams approved', () => {
|
||||
const status = buildStatus([], 2);
|
||||
|
||||
assert.equal(status.state, 'success');
|
||||
assert.match(status.description, /2 teams/);
|
||||
});
|
||||
|
||||
it('fails with the missing team slugs', () => {
|
||||
const status = buildStatus(['@n8n-io/qa-dx', '@n8n-io/migrations-review'], 2);
|
||||
|
||||
assert.equal(status.state, 'failure');
|
||||
assert.equal(status.description, 'Missing approval from: qa-dx, migrations-review');
|
||||
});
|
||||
});
|
||||
|
||||
describe('run', () => {
|
||||
let setCommitStatus;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.GITHUB_EVENT_NAME = 'pull_request';
|
||||
delete process.env.PULL_REQUEST_NUMBER;
|
||||
|
||||
eventImpl = () => ({ pull_request: { number: 42 } });
|
||||
getPullRequestByIdImpl = async () => ({
|
||||
base: { ref: 'master' },
|
||||
head: { sha: 'head-sha' },
|
||||
user: { login: 'author' },
|
||||
});
|
||||
getChangedFilesImpl = async () => new Set(['a.ts']);
|
||||
getPrReviewsImpl = async () => [];
|
||||
isTeamMemberImpl = async () => false;
|
||||
parseOwnersFileImpl = () => [];
|
||||
resolveRequiredTeamsImpl = () => new Map();
|
||||
setCommitStatus = mock.fn(async () => {});
|
||||
setCommitStatusImpl = setCommitStatus;
|
||||
});
|
||||
|
||||
it('sets a pending status first, then a success status when nothing is required', async () => {
|
||||
await run();
|
||||
|
||||
assert.equal(setCommitStatus.mock.calls.length, 2);
|
||||
|
||||
const [pendingSha, pendingStatus] = setCommitStatus.mock.calls[0].arguments;
|
||||
assert.equal(pendingSha, 'head-sha');
|
||||
assert.equal(pendingStatus.context, STATUS_CONTEXT);
|
||||
assert.equal(pendingStatus.state, 'pending');
|
||||
|
||||
const [sha, status] = setCommitStatus.mock.calls[1].arguments;
|
||||
assert.equal(sha, 'head-sha');
|
||||
assert.equal(status.context, STATUS_CONTEXT);
|
||||
assert.equal(status.state, 'success');
|
||||
});
|
||||
|
||||
it('sets a failure status when a required team has not approved', async () => {
|
||||
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
|
||||
getPrReviewsImpl = async () => [
|
||||
{ user: { login: 'outsider' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
];
|
||||
isTeamMemberImpl = async () => false;
|
||||
|
||||
await run();
|
||||
|
||||
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
|
||||
assert.equal(status.state, 'failure');
|
||||
assert.match(status.description, /Missing approval from: qa-dx/);
|
||||
});
|
||||
|
||||
it('sets an error status and rethrows when the evaluation fails', async () => {
|
||||
getChangedFilesImpl = async () => {
|
||||
throw new Error('API unavailable');
|
||||
};
|
||||
|
||||
await assert.rejects(run(), /API unavailable/);
|
||||
|
||||
assert.equal(setCommitStatus.mock.calls.length, 2);
|
||||
assert.equal(setCommitStatus.mock.calls[0].arguments[1].state, 'pending');
|
||||
assert.equal(setCommitStatus.mock.calls[1].arguments[1].state, 'error');
|
||||
});
|
||||
|
||||
it('sets a success status when a member of each required team approved', async () => {
|
||||
resolveRequiredTeamsImpl = () =>
|
||||
new Map([
|
||||
['@n8n-io/qa-dx', ['a.ts']],
|
||||
['@n8n-io/migrations-review', ['m.ts']],
|
||||
]);
|
||||
getPrReviewsImpl = async () => [
|
||||
{ user: { login: 'poly' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
];
|
||||
// `poly` is a member of both required teams.
|
||||
isTeamMemberImpl = async (slug, username) => username === 'poly';
|
||||
|
||||
await run();
|
||||
|
||||
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
|
||||
assert.equal(status.state, 'success');
|
||||
});
|
||||
|
||||
it('checks membership per approver and required team', async () => {
|
||||
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
|
||||
getPrReviewsImpl = async () => [
|
||||
{ user: { login: 'poly' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
];
|
||||
const isTeamMember = mock.fn(async () => true);
|
||||
isTeamMemberImpl = isTeamMember;
|
||||
|
||||
await run();
|
||||
|
||||
assert.deepEqual(
|
||||
isTeamMember.mock.calls.map((call) => call.arguments),
|
||||
[['qa-dx', 'poly']],
|
||||
);
|
||||
});
|
||||
|
||||
it('does not check membership when there is no approval', async () => {
|
||||
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
|
||||
const isTeamMember = mock.fn(async () => true);
|
||||
isTeamMemberImpl = isTeamMember;
|
||||
|
||||
await run();
|
||||
|
||||
assert.equal(isTeamMember.mock.calls.length, 0);
|
||||
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
|
||||
assert.equal(status.state, 'failure');
|
||||
});
|
||||
|
||||
it('does not count an approval that was later dismissed', async () => {
|
||||
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['a.ts']]]);
|
||||
getPrReviewsImpl = async () => [
|
||||
{ user: { login: 'poly' }, state: 'APPROVED', submitted_at: '2026-01-01T00:00:00Z' },
|
||||
{ user: { login: 'poly' }, state: 'DISMISSED', submitted_at: '2026-01-02T00:00:00Z' },
|
||||
];
|
||||
isTeamMemberImpl = async (slug, username) => username === 'poly';
|
||||
|
||||
await run();
|
||||
|
||||
const [, status] = setCommitStatus.mock.calls.at(-1).arguments;
|
||||
assert.equal(status.state, 'failure');
|
||||
});
|
||||
|
||||
it('skips PRs that do not target master without setting a status', async () => {
|
||||
getPullRequestByIdImpl = async () => ({
|
||||
base: { ref: 'release-candidate/2.9.x' },
|
||||
head: { sha: 'head-sha' },
|
||||
});
|
||||
|
||||
await run();
|
||||
|
||||
assert.equal(setCommitStatus.mock.calls.length, 0);
|
||||
});
|
||||
|
||||
});
|
||||
+54
-20
@@ -4,14 +4,21 @@
|
||||
* Posts (or updates) a single PR comment that combines:
|
||||
* - Recommended reviewer teams based on file ownership
|
||||
* - A breakdown of changed lines by category (source code, test files, misc)
|
||||
* - The required team reviews (OWNERS entries with `required`), with a
|
||||
* prompt to request review from those teams
|
||||
*
|
||||
* Advisory only — does not gate merging.
|
||||
* Advisory only — does not gate merging. Enforcement of required reviews
|
||||
* lives in required-reviews.mjs.
|
||||
*/
|
||||
|
||||
import { minimatch } from 'minimatch';
|
||||
import { ensureEnvVar, getPrFiles, postOrUpdateComment } from './github-helpers.mjs';
|
||||
import { assignOwnership, ownershipsToAllocations, parseOwnersFile } from './owners.mjs';
|
||||
import { MISC_PATTERNS, SIZE_LIMIT, TEST_PATTERNS } from './quality/check-pr-size.mjs';
|
||||
import { ensureEnvVar, getPrFiles, postOrUpdateComment } from '../github-helpers.mjs';
|
||||
import { categorizeFile, SIZE_LIMIT } from '../quality/check-pr-size.mjs';
|
||||
import {
|
||||
assignOwnership,
|
||||
ownershipsToAllocations,
|
||||
parseOwnersFile,
|
||||
resolveRequiredTeams,
|
||||
} from './owners.mjs';
|
||||
|
||||
/** @typedef {import('./owners.mjs').Allocation} Allocation */
|
||||
|
||||
@@ -41,19 +48,9 @@ function createEmptyLineStats() {
|
||||
* @param {{ filename: string, additions: number, deletions: number }} file
|
||||
*/
|
||||
function addFileToLineStats(stats, file) {
|
||||
const isTest = TEST_PATTERNS.some((p) => minimatch(file.filename, p));
|
||||
const isMisc = !isTest && MISC_PATTERNS.some((p) => minimatch(file.filename, p));
|
||||
|
||||
if (isTest) {
|
||||
stats.testFilesAdded += file.additions;
|
||||
stats.testFilesRemoved += file.deletions;
|
||||
} else if (isMisc) {
|
||||
stats.miscAdded += file.additions;
|
||||
stats.miscRemoved += file.deletions;
|
||||
} else {
|
||||
stats.sourceCodeAdded += file.additions;
|
||||
stats.sourceCodeRemoved += file.deletions;
|
||||
}
|
||||
const category = categorizeFile(file.filename);
|
||||
stats[`${category}Added`] += file.additions;
|
||||
stats[`${category}Removed`] += file.deletions;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -192,6 +189,35 @@ export function buildOverviewTable(allocations, changedFiles, totalLineStats, li
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the section that lists required team approvals. Returns null when no
|
||||
* approval is required.
|
||||
*
|
||||
* Suggests a team review request instead of naming an individual: GitHub then
|
||||
* assigns reviewers according to the team's own review settings (assignment
|
||||
* algorithm, excluded members), which the API does not let us read.
|
||||
*
|
||||
* @param { Map<string, string[]> } requiredTeamFiles Required team handle -> files that triggered the requirement.
|
||||
* @returns { string | null }
|
||||
*/
|
||||
export function buildRequiredReviewsSection(requiredTeamFiles) {
|
||||
if (requiredTeamFiles.size === 0) return null;
|
||||
|
||||
const plural = requiredTeamFiles.size > 1;
|
||||
|
||||
return [
|
||||
'### Required reviews',
|
||||
'',
|
||||
'Some changed files have a `required` owner in `OWNERS`. A member of each of these teams must approve this PR before it can merge:',
|
||||
'',
|
||||
'| Team | Files |',
|
||||
'| --- | ---: |',
|
||||
...[...requiredTeamFiles].map(([team, files]) => `| ${team} | ${files.length} |`),
|
||||
'',
|
||||
`Request a review from the team${plural ? 's' : ''} — GitHub assigns reviewers according to the team's review settings. The \`Auto-assign reviewers\` label does this for all owning teams.`,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Construct the full PR comment body from reviewer allocations and line stats.
|
||||
*
|
||||
@@ -200,14 +226,19 @@ export function buildOverviewTable(allocations, changedFiles, totalLineStats, li
|
||||
* @param { LineStats } lineStats
|
||||
* @param { Map<string, LineStats> } lineStatsByTeam
|
||||
* @param { Allocation[] } [otherAllocations]
|
||||
* @param { string | null } [requiredSection]
|
||||
* @returns { string }
|
||||
*/
|
||||
export function buildComment(allocations, changedFiles, lineStats, lineStatsByTeam = new Map(), otherAllocations = []) {
|
||||
export function buildComment(allocations, changedFiles, lineStats, lineStatsByTeam = new Map(), otherAllocations = [], requiredSection = null) {
|
||||
const body = [
|
||||
BOT_MARKER,
|
||||
buildOverviewTable(allocations, changedFiles, lineStats, lineStatsByTeam, otherAllocations),
|
||||
];
|
||||
|
||||
if (requiredSection) {
|
||||
body.push('', requiredSection);
|
||||
}
|
||||
|
||||
if (lineStats.sourceCodeAdded > SIZE_LIMIT) {
|
||||
body.push('', `❗ Source code additions (${lineStats.sourceCodeAdded.toLocaleString()}) exceed the ${SIZE_LIMIT.toLocaleString()}-line limit.`);
|
||||
}
|
||||
@@ -243,7 +274,10 @@ export async function run(pullRequestNumber) {
|
||||
const topAllocations = sortedAllocations.slice(0, 3);
|
||||
const otherAllocations = sortedAllocations.slice(3);
|
||||
|
||||
const body = buildComment(topAllocations, changedFiles, lineStats, lineStatsByTeam, otherAllocations);
|
||||
const requiredTeamFiles = resolveRequiredTeams(changedFiles, owners);
|
||||
const requiredSection = buildRequiredReviewsSection(requiredTeamFiles);
|
||||
|
||||
const body = buildComment(topAllocations, changedFiles, lineStats, lineStatsByTeam, otherAllocations, requiredSection);
|
||||
|
||||
await postOrUpdateComment(pullRequestNumber, body, BOT_MARKER);
|
||||
}
|
||||
+72
-7
@@ -4,7 +4,7 @@ import assert from 'node:assert/strict';
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners-review-recommendations.test.mjs
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners/review-recommendations.test.mjs
|
||||
* */
|
||||
|
||||
/** @type {(pullRequestNumber: number) => Promise<Array<{ filename: string, additions: number, deletions: number, previous_filename?: string }>>} */
|
||||
@@ -12,7 +12,7 @@ let getPrFilesImpl = async () => [];
|
||||
/** @type {(pullRequestNumber: number, body: string, botMarker: string) => Promise<void>} */
|
||||
let postOrUpdateCommentImpl = async () => {};
|
||||
|
||||
mock.module('./github-helpers.mjs', {
|
||||
mock.module('../github-helpers.mjs', {
|
||||
namedExports: {
|
||||
ensureEnvVar: () => {}, // no-op in tests
|
||||
initGithub: () => {}, // no-op in tests
|
||||
@@ -24,23 +24,32 @@ mock.module('./github-helpers.mjs', {
|
||||
},
|
||||
});
|
||||
|
||||
/** @type {() => Array<{ filepath: string, team: string }>} */
|
||||
/** @type {() => Array<import('./owners.mjs').OwnersEntry>} */
|
||||
let parseOwnersFileImpl = () => [];
|
||||
/** @type {(files: Set<string>, owners: Array<{ filepath: string, team: string }>) => Map<string, string[]>} */
|
||||
/** @type {(files: Set<string>, entries: any[]) => Map<string, string[]>} */
|
||||
let assignOwnershipImpl = () => new Map();
|
||||
/** @type {(ownerships: Map<string, string[]>) => Array<{ team: string, fileCount: number }>} */
|
||||
let ownershipsToAllocationsImpl = () => [];
|
||||
/** @type {(files: Set<string>, entries: any[]) => Map<string, string[]>} */
|
||||
let resolveRequiredTeamsImpl = () => new Map();
|
||||
|
||||
mock.module('./owners.mjs', {
|
||||
namedExports: {
|
||||
parseOwnersFile: () => parseOwnersFileImpl(),
|
||||
assignOwnership: (files, owners) => assignOwnershipImpl(files, owners),
|
||||
assignOwnership: (files, entries) => assignOwnershipImpl(files, entries),
|
||||
ownershipsToAllocations: (ownerships) => ownershipsToAllocationsImpl(ownerships),
|
||||
resolveRequiredTeams: (files, entries) => resolveRequiredTeamsImpl(files, entries),
|
||||
},
|
||||
});
|
||||
|
||||
const { buildOverviewTable, buildComment, computeAllocationLineStats, computeLineStats, run } =
|
||||
await import('./owners-review-recommendations.mjs');
|
||||
const {
|
||||
buildOverviewTable,
|
||||
buildComment,
|
||||
buildRequiredReviewsSection,
|
||||
computeAllocationLineStats,
|
||||
computeLineStats,
|
||||
run,
|
||||
} = await import('./review-recommendations.mjs');
|
||||
|
||||
const BOT_MARKER = '<!-- pr-recommendations -->';
|
||||
const EMPTY_LINE_STATS = {
|
||||
@@ -209,6 +218,7 @@ describe('computeLineStats', () => {
|
||||
['fixtures/ directory', 'src/fixtures/data.json'],
|
||||
['__mocks__/ directory', 'src/__mocks__/axios.ts'],
|
||||
['packages/testing/**', 'packages/testing/playwright/spec.ts'],
|
||||
['dot-directory *.test.mjs', '.github/scripts/owners/owners.test.mjs'],
|
||||
]) {
|
||||
it(`classifies ${label} as testFiles`, () => {
|
||||
const stats = computeLineStats([{ filename, additions: 7, deletions: 2 }]);
|
||||
@@ -307,6 +317,38 @@ describe('computeAllocationLineStats', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildRequiredReviewsSection', () => {
|
||||
it('returns null when no approval is required', () => {
|
||||
assert.equal(buildRequiredReviewsSection(new Map()), null);
|
||||
});
|
||||
|
||||
it('lists each required team with its file count', () => {
|
||||
const section = buildRequiredReviewsSection(
|
||||
new Map([
|
||||
['@n8n-io/qa-dx', ['a.yml', 'b.yml']],
|
||||
['@n8n-io/migrations-review', ['m.ts']],
|
||||
]),
|
||||
);
|
||||
|
||||
assert.match(section, /### Required reviews/);
|
||||
assert.match(section, /\| @n8n-io\/qa-dx \| 2 \|/);
|
||||
assert.match(section, /\| @n8n-io\/migrations-review \| 1 \|/);
|
||||
});
|
||||
|
||||
it('prompts to request review from the team, plural when several teams are required', () => {
|
||||
const singular = buildRequiredReviewsSection(new Map([['@n8n-io/qa-dx', ['a.yml']]]));
|
||||
const plural = buildRequiredReviewsSection(
|
||||
new Map([
|
||||
['@n8n-io/qa-dx', ['a.yml']],
|
||||
['@n8n-io/migrations-review', ['m.ts']],
|
||||
]),
|
||||
);
|
||||
|
||||
assert.match(singular, /Request a review from the team —/);
|
||||
assert.match(plural, /Request a review from the teams —/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('run', () => {
|
||||
let postOrUpdateComment;
|
||||
|
||||
@@ -317,6 +359,7 @@ describe('run', () => {
|
||||
parseOwnersFileImpl = () => [];
|
||||
assignOwnershipImpl = () => new Map();
|
||||
ownershipsToAllocationsImpl = () => [];
|
||||
resolveRequiredTeamsImpl = () => new Map();
|
||||
});
|
||||
|
||||
it('calls postOrUpdateComment with the PR number, generated body, and bot marker', async () => {
|
||||
@@ -385,4 +428,26 @@ describe('run', () => {
|
||||
assert.match(body, /## PR review overview/);
|
||||
assert.match(body, /\| @n8n-io\/cli-team \| 1 \| 100% \| \+50 \/ -5 \|/);
|
||||
});
|
||||
|
||||
it('omits the required reviews section when no approval is required', async () => {
|
||||
await run(42);
|
||||
|
||||
const body = postOrUpdateComment.mock.calls[0].arguments[1];
|
||||
assert.doesNotMatch(body, /### Required reviews/);
|
||||
});
|
||||
|
||||
it('includes the required reviews section with the team review prompt', async () => {
|
||||
getPrFilesImpl = async () => [{ filename: '.github/workflows/ci.yml', additions: 1, deletions: 0 }];
|
||||
parseOwnersFileImpl = () => [
|
||||
{ pattern: '.github/workflows/', team: '@n8n-io/qa-dx', required: true, line: 1 },
|
||||
];
|
||||
resolveRequiredTeamsImpl = () => new Map([['@n8n-io/qa-dx', ['.github/workflows/ci.yml']]]);
|
||||
|
||||
await run(42);
|
||||
|
||||
const body = postOrUpdateComment.mock.calls[0].arguments[1];
|
||||
assert.match(body, /### Required reviews/);
|
||||
assert.match(body, /\| @n8n-io\/qa-dx \| 1 \|/);
|
||||
assert.match(body, /Request a review from the team —/);
|
||||
});
|
||||
});
|
||||
@@ -6,7 +6,7 @@
|
||||
},
|
||||
"packageManager": "[email protected]",
|
||||
"scripts": {
|
||||
"test": "node --test --experimental-test-module-mocks ./*.test.mjs ./docker/*.test.mjs ./quality/*.test.mjs ./slack/*.test.mjs ./stale/*.test.mjs ../../scripts/licenses/*.test.mjs ../../scripts/mutation-health/*.test.mjs",
|
||||
"test": "node --test --experimental-test-module-mocks ./*.test.mjs ./docker/*.test.mjs ./owners/*.test.mjs ./quality/*.test.mjs ./slack/*.test.mjs ./stale/*.test.mjs ../../scripts/licenses/*.test.mjs ../../scripts/mutation-health/*.test.mjs",
|
||||
"generate-sbom": "FETCH_LICENSE=true cdxgen -t pnpm --no-install-deps --profile license-compliance --spec-version 1.6 -o ../../sbom-source.cdx.json ../../compiled/",
|
||||
"enrich-sbom": "node ../../scripts/licenses/enrich-sbom.mjs ../../sbom-source.cdx.json",
|
||||
"render-licenses-md": "node ../../scripts/licenses/render-licenses-md.mjs ../../sbom-source.cdx.json ../../packages/cli/THIRD_PARTY_LICENSES.md ../../compiled/node_modules",
|
||||
|
||||
@@ -49,6 +49,22 @@ export const MISC_PATTERNS = [
|
||||
|
||||
export const EXCLUDE_PATTERNS = [...TEST_PATTERNS, ...MISC_PATTERNS];
|
||||
|
||||
// Without `dot`, `**` refuses dot segments, so nothing under `.github/`
|
||||
// would ever match a pattern.
|
||||
const MATCH_OPTIONS = { dot: true };
|
||||
|
||||
/**
|
||||
* Categorize a changed file for line statistics.
|
||||
*
|
||||
* @param { string } filename
|
||||
* @returns { 'testFiles' | 'misc' | 'sourceCode' }
|
||||
*/
|
||||
export function categorizeFile(filename) {
|
||||
if (TEST_PATTERNS.some((pattern) => minimatch(filename, pattern, MATCH_OPTIONS))) return 'testFiles';
|
||||
if (MISC_PATTERNS.some((pattern) => minimatch(filename, pattern, MATCH_OPTIONS))) return 'misc';
|
||||
return 'sourceCode';
|
||||
}
|
||||
|
||||
const BOT_MARKER = '<!-- pr-size-check -->';
|
||||
|
||||
/**
|
||||
@@ -86,7 +102,7 @@ export async function hasValidOverride(comments, getPermission) {
|
||||
*/
|
||||
export function countFilteredAdditions(files, excludePatterns) {
|
||||
return files
|
||||
.filter((file) => !excludePatterns.some((pattern) => minimatch(file.filename, pattern)))
|
||||
.filter((file) => !excludePatterns.some((pattern) => minimatch(file.filename, pattern, MATCH_OPTIONS)))
|
||||
.reduce((sum, file) => sum + file.additions, 0);
|
||||
}
|
||||
|
||||
|
||||
@@ -158,6 +158,15 @@ describe('countFilteredAdditions', () => {
|
||||
assert.equal(countFilteredAdditions([], EXCLUDE_PATTERNS), 0);
|
||||
});
|
||||
|
||||
it('excludes test and misc files inside dot-directories', () => {
|
||||
const files = [
|
||||
{ filename: '.github/scripts/owners/owners.mjs', additions: 50 },
|
||||
{ filename: '.github/scripts/owners/owners.test.mjs', additions: 100 },
|
||||
{ filename: '.github/WORKFLOWS.md', additions: 100 },
|
||||
];
|
||||
assert.equal(countFilteredAdditions(files, EXCLUDE_PATTERNS), 50);
|
||||
});
|
||||
|
||||
it('applies EXCLUDE_PATTERNS to common test file extensions', () => {
|
||||
const files = [
|
||||
{ filename: 'src/service.ts', additions: 50 },
|
||||
|
||||
@@ -1,23 +0,0 @@
|
||||
# .github/workflows/ci-codeowners-validation.yml
|
||||
name: 'CI: Validate CODEOWNERS'
|
||||
|
||||
# Only run when CODEOWNERS or packages change
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- '.github/CODEOWNERS'
|
||||
- 'packages/**'
|
||||
|
||||
jobs:
|
||||
validate:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- uses: mszostok/codeowners-validator@7f3f5e28c6d7b8dfae5731e54ce2272ca384592f #v0.7.4
|
||||
with:
|
||||
# Start with safe checks only. Add "owners" and
|
||||
# experimental_checks: "notowned" once the file has settled
|
||||
# and skip patterns are configured.
|
||||
checks: 'files,duppatterns,syntax'
|
||||
github_access_token: '${{ secrets.GITHUB_TOKEN }}'
|
||||
@@ -3,7 +3,7 @@ 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
|
||||
# every team that owns a changed file (based on 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
|
||||
@@ -60,4 +60,4 @@ jobs:
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
|
||||
run: node .github/scripts/owners-assign-reviewers.mjs
|
||||
run: node .github/scripts/owners/assign-reviewers.mjs
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
name: 'CI: Owners Required Reviews'
|
||||
run-name: "${{ github.event_name == 'workflow_dispatch' && format('CI: Owners Required Reviews (PR #{0})', inputs.pr_number) || '' }}"
|
||||
|
||||
# Enforces the `required` entries in OWNERS: when a PR changes a file
|
||||
# whose winning OWNERS entry carries `required`, a member of the owning team
|
||||
# must approve the PR before merge.
|
||||
#
|
||||
# Replaces the GitHub-native CODEOWNERS enforcement, which could not express
|
||||
# team ownership metadata beyond "request a review".
|
||||
#
|
||||
# Triggers
|
||||
# - pull_request (opened/synchronize/reopened/ready_for_review):
|
||||
# re-evaluates whenever the changeset can change. Skipped for fork PRs
|
||||
# (no secrets): their status stays "expected" — fail closed — until the
|
||||
# first review event reports it.
|
||||
# - pull_request_review (submitted/dismissed): re-evaluates when approvals
|
||||
# change. A privileged event (base repo context, secrets), so it covers
|
||||
# fork PRs too — and approvals can only change through review events.
|
||||
# - merge_group: reports success on the queue head without re-evaluating.
|
||||
# A PR cannot enter the queue unless this status is green on its head,
|
||||
# and the queue does not change approvals.
|
||||
# - workflow_dispatch: manual re-check (e.g. after a team membership change,
|
||||
# or a fork PR push after an approval).
|
||||
#
|
||||
# The checkout below always pins the base branch, never the PR merge ref: the
|
||||
# scripts and the OWNERS file are trusted input, so a PR cannot lift its own
|
||||
# review requirement or reach the status-writing token.
|
||||
#
|
||||
# Output
|
||||
# - A commit status named "Required Reviews" on the PR head SHA (or
|
||||
# merge-group head SHA). Add this name to a ruleset's required-checks list
|
||||
# to gate merges on it. The first job step sets the status to pending, so
|
||||
# a run that crashes at any later point cannot leave a stale verdict in
|
||||
# effect. The job itself only fails on errors.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened, ready_for_review]
|
||||
branches:
|
||||
- master
|
||||
pull_request_review:
|
||||
types: [submitted, dismissed]
|
||||
merge_group:
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pr_number:
|
||||
description: 'Pull request number to re-evaluate'
|
||||
required: true
|
||||
type: string
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
concurrency:
|
||||
group: >-
|
||||
owners-required-reviews-${{ github.event.pull_request.number
|
||||
|| github.event.merge_group.head_sha
|
||||
|| inputs.pr_number
|
||||
|| github.ref }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
# The queue requires this status on the merge-group head, but there is
|
||||
# nothing new to evaluate there: entering the queue already required the
|
||||
# status to be green on the PR head, and queueing does not change approvals.
|
||||
pass-in-merge-queue:
|
||||
name: Pass in merge queue
|
||||
if: github.event_name == 'merge_group'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
statuses: write
|
||||
steps:
|
||||
- env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
SHA: ${{ github.event.merge_group.head_sha }}
|
||||
run: |
|
||||
gh api "repos/${GITHUB_REPOSITORY}/statuses/${SHA}" \
|
||||
-f state=success \
|
||||
-f context='Required Reviews' \
|
||||
-f description='Approvals were verified before the PR entered the queue'
|
||||
|
||||
required-reviews:
|
||||
name: Check required reviews
|
||||
# Fork PRs are skipped on pull_request (no secrets for the app token);
|
||||
# pull_request_review covers them. pull_request_review has no `branches`
|
||||
# filter; the script also skips non-master bases, the guard here keeps
|
||||
# the early pending status off non-master heads.
|
||||
if: >-
|
||||
github.event_name != 'merge_group' &&
|
||||
(github.event_name != 'pull_request' ||
|
||||
github.event.pull_request.head.repo.full_name == github.repository) &&
|
||||
(github.event_name != 'pull_request_review' ||
|
||||
github.event.pull_request.base.ref == 'master')
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
contents: read
|
||||
statuses: write
|
||||
steps:
|
||||
# Fail closed before any fallible step: replace a possibly-green status
|
||||
# with pending, so a failure in token generation, checkout, or setup
|
||||
# cannot leave a stale verdict in effect. The script sets the verdict.
|
||||
# Skipped for workflow_dispatch (no SHA in the event); there the script
|
||||
# sets pending itself.
|
||||
- name: Set pending status
|
||||
if: github.event_name != 'workflow_dispatch'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
SHA: ${{ github.event.pull_request.head.sha }}
|
||||
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
|
||||
run: |
|
||||
gh api "repos/${GITHUB_REPOSITORY}/statuses/${SHA}" \
|
||||
-f state=pending \
|
||||
-f context='Required Reviews' \
|
||||
-f description='Evaluating required reviews' \
|
||||
-f "target_url=${RUN_URL}"
|
||||
|
||||
- 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 }}
|
||||
# Scope the token to what the script needs; without these inputs it
|
||||
# would inherit every permission of the app installation.
|
||||
permission-members: read
|
||||
permission-pull-requests: read
|
||||
permission-statuses: write
|
||||
|
||||
# Always check out the base branch, never the PR merge ref: the OWNERS
|
||||
# file and the scripts must be trusted input. The 'master' fallback
|
||||
# covers workflow_dispatch, so a dispatch from another branch cannot
|
||||
# run that branch's policy.
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: ${{ github.event.pull_request.base.ref || 'master' }}
|
||||
persist-credentials: false
|
||||
|
||||
- name: Setup Node.js
|
||||
uses: ./.github/actions/setup-nodejs
|
||||
with:
|
||||
build-command: ''
|
||||
install-command: pnpm install --frozen-lockfile --dir ./.github/scripts --ignore-workspace
|
||||
|
||||
- name: Evaluate required reviews
|
||||
env:
|
||||
# App token: reading org team membership is beyond GITHUB_TOKEN.
|
||||
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
PULL_REQUEST_NUMBER: ${{ inputs.pr_number || '' }}
|
||||
STATUS_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
# Bootstrap: the checkout is the trusted base branch. Until the
|
||||
# enforcement script exists there, there is no policy to enforce.
|
||||
if [ ! -f .github/scripts/owners/required-reviews.mjs ]; then
|
||||
echo "No enforcement script on the base branch; nothing to enforce."
|
||||
if [ -n "$STATUS_SHA" ]; then
|
||||
gh api "repos/${GITHUB_REPOSITORY}/statuses/${STATUS_SHA}" \
|
||||
-f state=success \
|
||||
-f context='Required Reviews' \
|
||||
-f description='Owners enforcement is not on the base branch yet'
|
||||
fi
|
||||
exit 0
|
||||
fi
|
||||
node .github/scripts/owners/required-reviews.mjs
|
||||
@@ -1,10 +1,12 @@
|
||||
name: 'CI: Owners Review Recommendations'
|
||||
|
||||
# Posts (or updates) a PR comment with recommended reviewer teams (based on
|
||||
# file ownership in .github/OWNERS) and a breakdown of changed lines by
|
||||
# category (source code, test files, misc).
|
||||
# file ownership in OWNERS), a breakdown of changed lines by category
|
||||
# (source code, test files, misc), and the required team reviews with a
|
||||
# prompt to request review from those teams.
|
||||
#
|
||||
# Advisory only — does not gate merging.
|
||||
# Advisory only — does not gate merging. Enforcement of required reviews
|
||||
# lives in ci-owners-required-reviews.yml.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
@@ -49,4 +51,4 @@ jobs:
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}
|
||||
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
|
||||
run: node .github/scripts/owners-review-recommendations.mjs
|
||||
run: node .github/scripts/owners/review-recommendations.mjs
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
name: 'CI: Validate OWNERS'
|
||||
|
||||
# Validates OWNERS strictly: line syntax (pattern, one team,
|
||||
# then options), duplicate patterns, and patterns that no longer point at an
|
||||
# existing file or directory. Runs on OWNERS changes and on changes to the
|
||||
# owned paths, so a move or delete that orphans an OWNERS entry fails here.
|
||||
#
|
||||
# Team names are not checked here: that needs an org read token, which fork
|
||||
# PRs do not have. A separate workflow covers it (see DEVP-891).
|
||||
|
||||
# The paths cover every top-level path that OWNERS entries point at, so a
|
||||
# move or delete of an owned path cannot slip through unvalidated.
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- '.github/**'
|
||||
- 'assets/**'
|
||||
- 'docker/**'
|
||||
- 'packages/**'
|
||||
- 'patches/**'
|
||||
- 'scripts/**'
|
||||
- 'security/**'
|
||||
- '.code-health-baseline.json'
|
||||
- 'OWNERS'
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
validate:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
persist-credentials: false
|
||||
|
||||
# owners.mjs only uses node built-ins, so no dependency install is needed.
|
||||
- run: node .github/scripts/owners/owners.mjs --check
|
||||
@@ -115,6 +115,7 @@ jobs:
|
||||
workflows: .github/**
|
||||
workflow-scripts:
|
||||
.github/scripts/**
|
||||
OWNERS
|
||||
scripts/licenses/**
|
||||
scripts/mutation-health/**
|
||||
cubic.yaml
|
||||
|
||||
+24
-7
@@ -1,6 +1,20 @@
|
||||
# n8n CODEOWNERS
|
||||
# n8n OWNERS
|
||||
#
|
||||
# Format: <pattern> <@org/team> [required]
|
||||
#
|
||||
# Examples:
|
||||
# packages/foo/ @n8n-io/team-a advisory reviewer recommendation
|
||||
# packages/foo/bar.ts @n8n-io/team-b required team approval blocks merge
|
||||
#
|
||||
# Last-match-wins: specific rules MUST come AFTER general rules.
|
||||
#
|
||||
# The `required` option makes team approval mandatory: a member of the team
|
||||
# must approve the PR before merge. This is enforced by the
|
||||
# "Required Reviews" commit status (ci-owners-required-reviews.yml).
|
||||
# All other entries are advisory (reviewer recommendations and opt-in
|
||||
# reviewer auto-assignment).
|
||||
#
|
||||
# Validated by ci-owners-validation.yml (`node .github/scripts/owners/owners.mjs --check`).
|
||||
|
||||
# Default catch-all (ensures every file gets at least one reviewer)
|
||||
* @n8n-io/catalysts
|
||||
@@ -9,7 +23,7 @@
|
||||
|
||||
packages/core/ @n8n-io/catalysts
|
||||
packages/workflow/ @n8n-io/catalysts
|
||||
packages/workflow/styker.config.mjs @n8n-io/qa-dx
|
||||
packages/workflow/stryker.config.mjs @n8n-io/qa-dx
|
||||
packages/@n8n/config/ @n8n-io/catalysts
|
||||
packages/@n8n/backend-common/ @n8n-io/catalysts
|
||||
packages/@n8n/backend-network/ @n8n-io/catalysts
|
||||
@@ -30,13 +44,15 @@ packages/@n8n/extension-sdk/ @n8n-io/catalysts
|
||||
packages/@n8n/eslint-config/ @n8n-io/qa-dx
|
||||
packages/@n8n/typescript-config/ @n8n-io/qa-dx
|
||||
|
||||
packages/@n8n/db/src/migrations/ @n8n-io/migrations-review
|
||||
packages/@n8n/db/src/migrations/ @n8n-io/migrations-review required
|
||||
|
||||
# Top-level paths
|
||||
scripts/ @n8n-io/qa-dx
|
||||
patches/ @n8n-io/qa-dx
|
||||
assets/ @n8n-io/adore
|
||||
security/ @n8n-io/qa-dx
|
||||
.code-health-baseline.json @n8n-io/qa-dx required
|
||||
OWNERS @n8n-io/qa-dx required
|
||||
|
||||
# @n8n/cli
|
||||
packages/@n8n/cli/ @n8n-io/adore
|
||||
@@ -126,7 +142,6 @@ packages/cli/src/controllers/cta.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/folder.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/tags.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/binary-data.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/dynamic-templates.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/posthog.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/translation.controller.ts @n8n-io/adore
|
||||
packages/cli/src/controllers/project.controller.ts @n8n-io/ligo
|
||||
@@ -143,7 +158,6 @@ packages/cli/src/services/role-cache.service.ts @n8n-io/iam
|
||||
packages/cli/src/services/password.utility.ts @n8n-io/iam
|
||||
packages/cli/src/services/public-api-key.service.ts @n8n-io/iam
|
||||
packages/cli/src/services/security-settings.service.ts @n8n-io/iam
|
||||
packages/cli/src/services/ssrf/ @n8n-io/catalysts
|
||||
packages/cli/src/services/static-auth-service.ts @n8n-io/iam
|
||||
packages/cli/src/services/access.service.ts @n8n-io/iam
|
||||
packages/cli/src/services/ai.service.ts @n8n-io/ai
|
||||
@@ -153,7 +167,6 @@ packages/cli/src/services/annotation-tag.service.ee.ts @n8n-io/ai
|
||||
packages/cli/src/services/folder.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/tag.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/cta.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/dynamic-templates.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/frontend.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/banner.service.ts @n8n-io/adore
|
||||
packages/cli/src/services/project.service.ee.ts @n8n-io/ligo
|
||||
@@ -169,7 +182,6 @@ packages/frontend/editor-ui/ @n8n-io/frontend
|
||||
packages/frontend/editor-ui/src/features/ai/ @n8n-io/ai
|
||||
packages/frontend/editor-ui/src/features/credentials/ @n8n-io/iam
|
||||
packages/frontend/editor-ui/src/features/execution/ @n8n-io/ligo
|
||||
packages/frontend/editor-ui/src/features/project-roles/ @n8n-io/iam
|
||||
packages/frontend/editor-ui/src/features/integrations/ @n8n-io/ligo
|
||||
|
||||
packages/frontend/@n8n/design-system/ @n8n-io/design
|
||||
@@ -225,7 +237,12 @@ packages/extensions/insights/ @n8n-io/ligo
|
||||
# CI/CD
|
||||
|
||||
.github/ @n8n-io/qa-dx
|
||||
.github/workflows/ @n8n-io/qa-dx required
|
||||
.github/scripts/ @n8n-io/qa-dx required
|
||||
.github/actions/ @n8n-io/qa-dx required
|
||||
.github/poutine-rules/ @n8n-io/qa-dx required
|
||||
docker/ @n8n-io/qa-dx
|
||||
docker/get-n8n.sh @n8n-io/qa-dx required
|
||||
|
||||
# QA
|
||||
|
||||
Reference in New Issue
Block a user