diff --git a/.github/tests/auto-reviewers.test.cjs b/.github/tests/auto-reviewers.test.cjs new file mode 100644 index 00000000..f96313dc --- /dev/null +++ b/.github/tests/auto-reviewers.test.cjs @@ -0,0 +1,187 @@ +const assert = require('node:assert/strict'); +const { readFileSync } = require('node:fs'); +const { resolve } = require('node:path'); +const { test } = require('node:test'); + +const workflow = readFileSync(resolve(__dirname, '..', 'workflows', 'auto-reviewers.yml'), 'utf8'); +const blocks = [...workflow.matchAll(/ script: \|\r?\n((?: [^\r\n]*(?:\r?\n|$)|\r?\n)+)/g)]; +assert.equal(blocks.length, 2, 'Expected the team-reading and reviewer-requesting scripts.'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const scripts = blocks.map(match => new AsyncFunction( + 'github', 'context', 'core', 'process', + match[1].replace(/^ /gm, '') +)); +const context = { + repo: { owner: 'mini-software', repo: 'MiniExcel' }, + payload: { pull_request: { number: 1040 } } +}; + +function harness(options = {}) { + const logs = []; + const core = { + info: message => logs.push(message), + notice: message => logs.push(message) + }; + const requested = (options.requested ?? []).map(login => ({ login })); + const reviews = (options.reviewed ?? []).map(login => ({ user: { login } })); + const batches = []; + const pull = { + state: 'open', + draft: false, + base: { ref: 'master' }, + user: { login: 'author' }, + ...options.pull + }; + const listReviews = async () => {}; + const github = { + rest: { + pulls: { + get: async params => { + assert.ok(Number.isSafeInteger(params.pull_number)); + return { data: pull }; + }, + listRequestedReviewers: async () => ({ data: { users: requested } }), + listReviews, + requestReviewers: async ({ reviewers }) => { + if (options.error) { + throw options.error; + } + batches.push(reviewers); + if (options.omit) { + return; + } + if (options.reviewImmediately) { + reviews.push(...reviewers.map(login => ({ user: { login } }))); + } else { + requested.push(...reviewers.map(login => ({ login }))); + } + } + } + }, + paginate: async (method, params) => { + assert.equal(method, listReviews); + assert.equal(params.per_page, 100); + return [...reviews]; + } + }; + const run = (members, customContext = context, input = '') => scripts[1]( + github, customContext, core, + { env: { TEAM_REVIEWERS: JSON.stringify(members), INPUT_PR_NUMBER: input } } + ); + return { run, logs, batches }; +} + +test('team reader requires an explicit organization credential', async () => { + await assert.rejects(scripts[0]({}, context, {}, { env: {} }), /CORE_REVIEWERS_TOKEN/); +}); + +test('team reader dynamically paginates active members and excludes bots', async () => { + const listMembersInOrg = async () => {}; + const github = { + rest: { teams: { listMembersInOrg } }, + paginate: async (method, params) => { + assert.equal(method, listMembersInOrg); + assert.deepEqual(params, { + org: 'mini-software', team_slug: 'core-reviewers', per_page: 100 + }); + return [{ login: 'new-member', type: 'User' }, { login: 'bot', type: 'Bot' }]; + } + }; + const result = await scripts[0](github, context, { info() {} }, { + env: { CORE_REVIEWERS_TOKEN: 'test-only' } + }); + assert.deepEqual(JSON.parse(result), ['new-member']); + github.paginate = async () => []; + await assert.rejects(scripts[0](github, context, {}, { + env: { CORE_REVIEWERS_TOKEN: 'test-only' } + }), /no active human members/); +}); + +test('excludes the author, requested users, reviewed users and duplicate names case-insensitively', async () => { + const h = harness({ requested: ['requested'], reviewed: ['reviewed'] }); + await h.run(['AUTHOR', 'REQUESTED', 'REVIEWED', 'new-member', 'NEW-MEMBER']); + assert.deepEqual(h.batches, [['new-member']]); +}); + +test('requests all twenty eligible members in batches of at most fifteen', async () => { + const members = Array.from({ length: 20 }, (_, index) => `member-${index}`); + const h = harness(); + await h.run(members); + assert.deepEqual(h.batches.map(batch => batch.length), [15, 5]); + assert.deepEqual(h.batches.flat(), members); +}); + +test('repeated events do not send duplicate review requests', async () => { + const h = harness(); + await h.run(['first', 'second']); + await h.run(['first', 'second', 'new-member']); + assert.deepEqual(h.batches, [['first', 'second'], ['new-member']]); +}); + +test('reports when no additional reviewer is eligible', async () => { + const h = harness(); + await h.run(['author']); + assert.deepEqual(h.batches, []); + assert.match(h.logs[0], /already been requested or reviewed/); +}); + +test('skips closed, draft and non-master PRs explicitly', async () => { + for (const pull of [{ state: 'closed' }, { draft: true }, { base: { ref: 'other' } }]) { + const h = harness({ pull }); + await h.run(['reviewer']); + assert.deepEqual(h.batches, []); + assert.match(h.logs[0], /Skipping/); + } +}); + +test('manual dispatch accepts a PR number and rejects invalid inputs', async () => { + const manualContext = { ...context, payload: {} }; + const h = harness(); + await h.run(['reviewer'], manualContext, '1040'); + assert.deepEqual(h.batches, [['reviewer']]); + for (const input of ['', '-1', '0', '1.5', 'text', '9007199254740992']) { + await assert.rejects(h.run(['reviewer'], manualContext, input), /positive integer/); + } +}); + +test('API permission failures propagate instead of reporting success', async () => { + const error = new Error('Resource not accessible by integration'); + const h = harness({ error }); + await assert.rejects(h.run(['reviewer']), error); + assert.deepEqual(h.logs, []); +}); + +test('silently omitted API requests fail verification', async () => { + const h = harness({ omit: true }); + await assert.rejects(h.run(['reviewer']), /did not confirm review requests for: reviewer/); + assert.deepEqual(h.logs, []); +}); + +test('review completed during the request is counted as confirmed', async () => { + const h = harness({ reviewImmediately: true }); + await h.run(['reviewer']); + assert.match(h.logs[0], /Requested 1/); +}); + +test('malformed team output fails instead of treating it as an empty list', async () => { + await assert.rejects(scripts[1]( + { + rest: { pulls: { + get: async () => ({ data: { + state: 'open', draft: false, base: { ref: 'master' }, user: { login: 'author' } + } }), + listRequestedReviewers: async () => ({ data: { users: [] } }), + listReviews: async () => {} + } }, + paginate: async () => [] + }, + context, {}, { env: { TEAM_REVIEWERS: 'not-json' } } + ), SyntaxError); +}); + +test('fork-capable workflow never checks out or executes PR code', () => { + assert.match(workflow, /pull_request_target:/); + assert.match(workflow, /pull-requests: write/); + assert.doesNotMatch(workflow, /actions\/checkout|head\.sha|head\.ref|pull_request\.title|pull_request\.body/); + assert.match(workflow, /github-token: \$\{\{ secrets\.CORE_REVIEWERS_TOKEN \}\}/); +}); diff --git a/.github/tests/review-approval-policy.test.cjs b/.github/tests/review-approval-policy.test.cjs new file mode 100644 index 00000000..631f9f6c --- /dev/null +++ b/.github/tests/review-approval-policy.test.cjs @@ -0,0 +1,208 @@ +const assert = require('node:assert/strict'); +const { readFileSync } = require('node:fs'); +const { resolve } = require('node:path'); +const { test } = require('node:test'); + +const workflow = readFileSync(resolve(__dirname, '..', 'workflows', 'review-approval-policy.yml'), 'utf8'); +const signal = readFileSync(resolve(__dirname, '..', 'workflows', 'review-changed.yml'), 'utf8'); +const block = workflow.match(/ script: \|\r?\n((?: [^\r\n]*(?:\r?\n|$)|\r?\n)+)/); +assert.ok(block, 'Expected an embedded approval policy script.'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const evaluate = new AsyncFunction('github', 'context', 'core', block[1].replace(/^ /gm, '')); + +function pull(number = 1, labels = [], sha = `sha-${number}`) { + return { + number, draft: false, head: { sha }, + user: { login: 'author' }, labels: labels.map(name => ({ name })) + }; +} + +function review(id, login, state = 'APPROVED', type = 'User') { + return { id, state, user: { login, type } }; +} + +function harness(pulls, reviews = {}, options = {}) { + const statuses = []; + const logs = []; + const permissionCalls = []; + const list = async () => {}; + const listReviews = async () => {}; + const github = { + rest: { + pulls: { list, listReviews }, + repos: { + createCommitStatus: async status => { + statuses.push(status); + }, + getCollaboratorPermissionLevel: async ({ username }) => { + permissionCalls.push(username); + if (options.permissionError) { + throw options.permissionError; + } + return { data: { permission: options.permissions?.[username] ?? 'write' } }; + } + } + }, + paginate: async (method, params) => { + assert.equal(params.per_page, 100); + if (method === list) { + assert.equal(params.base, 'master'); + assert.equal(params.state, 'open'); + return pulls; + } + assert.equal(method, listReviews); + if (options.reviewError) { + throw options.reviewError; + } + return [...(reviews[params.pull_number] ?? [])]; + } + }; + const context = { + repo: { owner: 'mini-software', repo: 'MiniExcel' }, + serverUrl: 'https://github.com', runId: 123, + payload: { workflow_run: { pull_requests: [], head_repository: { full_name: 'untrusted/fork' } } } + }; + const run = () => evaluate(github, context, { info: message => logs.push(message) }); + return { run, statuses, logs, permissionCalls }; +} + +test('ordinary PR requires exactly one eligible approval', async () => { + for (const [reviews, expected] of [[[], 'pending'], [[review(1, 'reviewer')], 'success']]) { + const h = harness([pull()], { 1: reviews }); + await h.run(); + assert.equal(h.statuses[0].state, 'pending'); + assert.equal(h.statuses.at(-1).state, expected); + assert.equal(h.statuses.at(-1).context, 'Review approval policy'); + } +}); + +test('major-change PR remains blocked at two and passes at three approvals', async () => { + for (const count of [0, 1, 2, 3]) { + const h = harness([pull(1, ['MAJOR-CHANGE'])], { + 1: Array.from({ length: count }, (_, index) => review(index + 1, `reviewer-${index}`)) + }); + await h.run(); + assert.equal(h.statuses.at(-1).state, count === 3 ? 'success' : 'pending'); + if (count < 3) { + assert.match(h.statuses.at(-1).description, new RegExp(`${count}/3`)); + } + } +}); + +test('invited users, comments, author approvals, bots and Read users do not count', async () => { + const h = harness([pull(1, ['major-change'])], { + 1: [ + review(1, 'AUTHOR'), review(2, 'bot', 'APPROVED', 'Bot'), + review(3, 'read-user'), review(4, 'commenter', 'COMMENTED'), + review(5, 'valid') + ] + }, { permissions: { 'read-user': 'read' } }); + await h.run(); + assert.match(h.statuses.at(-1).description, /1\/3/); + assert.deepEqual(h.permissionCalls, ['read-user', 'valid']); +}); + +test('counts Write, Maintain and Admin approvals', async () => { + const h = harness([pull(1, ['major-change'])], { + 1: [review(1, 'writer'), review(2, 'maintainer'), review(3, 'admin')] + }, { permissions: { writer: 'write', maintainer: 'maintain', admin: 'admin' } }); + await h.run(); + assert.equal(h.statuses.at(-1).state, 'success'); +}); + +test('later dismissed or changes-requested reviews supersede earlier approvals', async () => { + for (const state of ['DISMISSED', 'CHANGES_REQUESTED']) { + const h = harness([pull()], { + 1: [review(2, 'REVIEWER', state), review(1, 'reviewer')] + }); + await h.run(); + assert.match(h.statuses.at(-1).description, /0\/1/); + } +}); + +test('comments do not cancel approvals and duplicate approvals count only once', async () => { + const h = harness([pull(1, ['major-change'])], { + 1: [review(1, 'reviewer'), review(2, 'REVIEWER'), review(3, 'Reviewer', 'COMMENTED')] + }); + await h.run(); + assert.match(h.statuses.at(-1).description, /1\/3/); +}); + +test('a new approval after a change request is counted', async () => { + const h = harness([pull()], { + 1: [review(1, 'reviewer', 'CHANGES_REQUESTED'), review(2, 'reviewer')] + }); + await h.run(); + assert.equal(h.statuses.at(-1).state, 'success'); +}); + +test('a label change raises the requirement without trusting old success', async () => { + const current = pull(); + const h = harness([current], { 1: [review(1, 'reviewer')] }); + await h.run(); + assert.equal(h.statuses.at(-1).state, 'success'); + current.labels.push({ name: 'major-change' }); + await h.run(); + assert.equal(h.statuses.at(-2).state, 'pending'); + assert.match(h.statuses.at(-1).description, /1\/3/); +}); + +test('PRs sharing a SHA cannot overwrite a stricter PR status with ordinary success', async () => { + const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], { + 1: [review(1, 'one')], + 2: [review(2, 'one'), review(3, 'two')] + }); + await h.run(); + assert.equal(h.statuses.length, 2); + assert.equal(h.statuses.at(-1).sha, 'shared'); + assert.match(h.statuses.at(-1).description, /#2: 2\/3/); + assert.deepEqual(h.permissionCalls, ['one', 'two']); +}); + +test('all PRs sharing a SHA must satisfy their own approval requirements', async () => { + const h = harness([pull(1, [], 'shared'), pull(2, ['major-change'], 'shared')], { + 1: [review(1, 'one')], + 2: [review(2, 'one'), review(3, 'two'), review(4, 'three')] + }); + await h.run(); + assert.equal(h.statuses.at(-1).state, 'success'); +}); + +test('review and permission API failures leave checks pending and propagate', async () => { + for (const option of ['reviewError', 'permissionError']) { + const error = new Error('GitHub API access failed'); + const h = harness([pull()], { 1: [review(1, 'reviewer')] }, { [option]: error }); + await assert.rejects(h.run(), error); + assert.deepEqual(h.statuses.map(status => status.state), ['pending']); + } +}); + +test('draft PRs are skipped and no open PRs are reported explicitly', async () => { + const draft = pull(); + draft.draft = true; + for (const pulls of [[], [draft]]) { + const h = harness(pulls); + await h.run(); + assert.deepEqual(h.statuses, []); + assert.match(h.logs[0], /No open/); + } +}); + +test('commit status descriptions obey the GitHub length limit', async () => { + const h = harness(Array.from({ length: 20 }, (_, index) => pull(index + 1, ['major-change'], 'shared'))); + await h.run(); + assert.equal(h.statuses.at(-1).description.length, 140); +}); + +test('trusted workflow uses no PR code, artifact content, team credential or write-capable review trigger', () => { + assert.match(workflow, /pull_request_target:/); + assert.match(workflow, /workflow_run:/); + assert.match(workflow, /workflows: \[PR review changed\]/); + assert.match(workflow, /statuses: write/); + assert.match(workflow, /pull-requests: read/); + assert.doesNotMatch(workflow, /actions\/checkout|\$\{\{[^}]*head\.(?:sha|ref)|download-artifact|secrets\.|pull_request_review:/); + assert.match(signal, /pull_request_review:/); + assert.match(signal, /types: \[submitted, dismissed\]/); + assert.match(signal, /permissions: \{\}/); + assert.doesNotMatch(signal, /actions\/checkout|\$\{\{|secrets\.|write/); +}); diff --git a/.github/workflows/auto-reviewers.yml b/.github/workflows/auto-reviewers.yml new file mode 100644 index 00000000..cda6a71f --- /dev/null +++ b/.github/workflows/auto-reviewers.yml @@ -0,0 +1,117 @@ +name: Request core reviewers + +on: + pull_request_target: + branches: [master] + types: [opened, reopened, ready_for_review, synchronize] + workflow_dispatch: + inputs: + pull_request_number: + description: Open PR targeting master to refresh reviewers + required: true + type: string + +permissions: + pull-requests: write + +concurrency: + group: core-reviewers-${{ github.event.pull_request.number || inputs.pull_request_number }} + cancel-in-progress: false + +jobs: + request-reviewers: + if: github.event_name == 'workflow_dispatch' || !github.event.pull_request.draft + runs-on: ubuntu-latest + steps: + - name: Read current team members + id: team + uses: actions/github-script@v7 + env: + CORE_REVIEWERS_TOKEN: ${{ secrets.CORE_REVIEWERS_TOKEN }} + with: + github-token: ${{ secrets.CORE_REVIEWERS_TOKEN }} + result-encoding: string + script: | + if (!process.env.CORE_REVIEWERS_TOKEN) { + throw new Error('Configure CORE_REVIEWERS_TOKEN with organization Members: read permission.'); + } + + const members = await github.paginate(github.rest.teams.listMembersInOrg, { + org: context.repo.owner, + team_slug: 'core-reviewers', + per_page: 100 + }); + const reviewers = members + .filter(member => member.type !== 'Bot') + .map(member => member.login); + if (reviewers.length === 0) { + throw new Error('The core-reviewers team has no active human members.'); + } + core.info(`Loaded ${reviewers.length} active core reviewers.`); + return JSON.stringify(reviewers); + + - name: Request missing reviewers + uses: actions/github-script@v7 + env: + TEAM_REVIEWERS: ${{ steps.team.outputs.result }} + INPUT_PR_NUMBER: ${{ inputs.pull_request_number }} + with: + script: | + const input = context.payload.pull_request?.number ?? process.env.INPUT_PR_NUMBER; + const pull_number = Number(input); + if (!/^[1-9]\d*$/.test(String(input)) || !Number.isSafeInteger(pull_number)) { + throw new Error('A positive integer pull request number is required.'); + } + const params = { ...context.repo, pull_number }; + const { data: pull } = await github.rest.pulls.get(params); + if (pull.state !== 'open' || pull.draft || pull.base.ref !== 'master') { + core.notice('Skipping: the PR must be open, ready for review, and target master.'); + return; + } + + const { data: requested } = await github.rest.pulls.listRequestedReviewers(params); + const reviews = await github.paginate(github.rest.pulls.listReviews, { + ...params, + per_page: 100 + }); + const excluded = new Set([ + pull.user.login, + ...requested.users.map(user => user.login), + ...reviews.map(review => review.user.login) + ].map(login => login.toLowerCase())); + const reviewers = JSON.parse(process.env.TEAM_REVIEWERS); + const missing = []; + for (const login of reviewers) { + const key = login.toLowerCase(); + if (!excluded.has(key)) { + missing.push(login); + excluded.add(key); + } + } + if (missing.length === 0) { + core.info('All eligible team members have already been requested or reviewed.'); + return; + } + + // GitHub can omit reviewers from oversized requests, so limit each batch. + for (let offset = 0; offset < missing.length; offset += 15) { + await github.rest.pulls.requestReviewers({ + ...params, + reviewers: missing.slice(offset, offset + 15) + }); + } + + const { data: updated } = await github.rest.pulls.listRequestedReviewers(params); + const updatedReviews = await github.paginate(github.rest.pulls.listReviews, { + ...params, + per_page: 100 + }); + const confirmed = new Set([ + ...updated.users.map(user => user.login), + ...updatedReviews.map(review => review.user.login) + ].map(login => login.toLowerCase())); + const unconfirmed = missing.filter(login => !confirmed.has(login.toLowerCase())); + if (unconfirmed.length > 0) { + throw new Error(`GitHub did not confirm review requests for: ${unconfirmed.join(', ')}`); + } + core.info(`Requested ${missing.length} additional core reviewers.`); diff --git a/.github/workflows/review-approval-policy.yml b/.github/workflows/review-approval-policy.yml new file mode 100644 index 00000000..fd049713 --- /dev/null +++ b/.github/workflows/review-approval-policy.yml @@ -0,0 +1,105 @@ +name: Review approval policy + +on: + pull_request_target: + branches: [master] + types: [opened, reopened, synchronize, ready_for_review, converted_to_draft, labeled, unlabeled, edited, closed] + workflow_run: + workflows: [PR review changed] + types: [completed] + workflow_dispatch: + +permissions: + pull-requests: read + statuses: write + +concurrency: + group: review-approval-policy + cancel-in-progress: false + +jobs: + check-approvals: + runs-on: ubuntu-latest + steps: + - name: Refresh approval requirements + uses: actions/github-script@v7 + with: + script: | + const statusContext = 'Review approval policy'; + const pulls = await github.paginate(github.rest.pulls.list, { + ...context.repo, + state: 'open', + base: 'master', + per_page: 100 + }); + const groups = new Map(); + for (const pull of pulls.filter(pull => !pull.draft)) { + const group = groups.get(pull.head.sha) ?? []; + group.push(pull); + groups.set(pull.head.sha, group); + } + if (groups.size === 0) { + core.info('No open, ready-for-review PRs target master.'); + return; + } + const writeStatus = (sha, state, description) => github.rest.repos.createCommitStatus({ + ...context.repo, + sha, + state, + context: statusContext, + description: description.slice(0, 140), + target_url: `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}` + }); + for (const sha of groups.keys()) { + await writeStatus(sha, 'pending', 'Checking current labels and eligible approvals.'); + } + + const permissions = new Map(); + for (const [sha, group] of groups) { + const results = []; + for (const pull of group) { + const required = pull.labels.some(label => label.name.toLowerCase() === 'major-change') ? 3 : 1; + const reviews = await github.paginate(github.rest.pulls.listReviews, { + ...context.repo, + pull_number: pull.number, + per_page: 100 + }); + const latest = new Map(); + for (const review of reviews.sort((left, right) => left.id - right.id)) { + if (['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes(review.state)) { + latest.set(review.user.login.toLowerCase(), review); + } + } + let approvals = 0; + for (const [login, review] of latest) { + if (review.state !== 'APPROVED' || review.user.type === 'Bot' || + login === pull.user.login.toLowerCase()) { + continue; + } + if (!permissions.has(login)) { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: review.user.login + }); + permissions.set(login, ['write', 'maintain', 'admin'].includes(data.permission)); + } + if (permissions.get(login)) { + approvals++; + } + } + results.push({ number: pull.number, required, approvals }); + } + + // Commit statuses are shared by PRs with the same head SHA. + const waiting = results.filter(result => result.approvals < result.required); + if (waiting.length > 0) { + const description = waiting + .map(result => `#${result.number}: ${result.approvals}/${result.required} eligible approvals`) + .join('; '); + await writeStatus(sha, 'pending', description); + core.info(description); + } else { + await writeStatus(sha, 'success', `Approval policy satisfied for ${group.length} open PR(s).`); + core.info(`Approval policy satisfied for ${group.map(pull => `#${pull.number}`).join(', ')}.`); + } + } diff --git a/.github/workflows/review-changed.yml b/.github/workflows/review-changed.yml new file mode 100644 index 00000000..5a9f7880 --- /dev/null +++ b/.github/workflows/review-changed.yml @@ -0,0 +1,14 @@ +name: PR review changed + +on: + pull_request_review: + types: [submitted, dismissed] + +permissions: {} + +jobs: + notify: + runs-on: ubuntu-latest + steps: + - name: Signal trusted approval refresh + run: echo "Review changed; the trusted approval policy workflow will refresh open PRs." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 872cee50..6b192e0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,86 @@ The issue template will help you on the way once again: * Unit tests for the changes have been added * Documentation has been updated where necessary +### Automatic review requests + +PRs targeting `master` automatically request active members of the +[`core-reviewers` team](https://github.com/orgs/mini-software/teams/core-reviewers) +when opened, reopened, marked ready for review, or updated. The PR author, +already requested reviewers, and anyone who has already submitted a review are +excluded. Draft PRs do not request reviews. Merge approval requirements are +configured separately from review invitations. + +Maintainers can update the reviewer list by adding or removing team members; +pending organization invitations must be accepted before members are included. +Removing a member does not revoke existing review requests or their separately +granted repository permissions. + +Before enabling the [reviewer workflow](.github/workflows/auto-reviewers.yml), +configure a fine-grained personal access token with `mini-software` as the resource +owner and only the organization **Members: read** permission. Store it as the +repository Actions secret `CORE_REVIEWERS_TOKEN` and ensure any required +organization approval is complete. Rotate the token before it expires. The token +only reads team membership; review requests use the workflow's `GITHUB_TOKEN` +with **Pull requests: write** permission. + +After the workflow is merged into `master`, use its **Run workflow** action with +a PR number to refresh reviewers on an existing PR. Team changes are otherwise +picked up on the next supported PR event, not immediately when membership changes. +The workflow uses `pull_request_target` to support fork PRs without checking out +or executing PR code. Missing credentials, team access errors, and unconfirmed +review requests fail the workflow explicitly. + +### Approval requirements + +The review policy for PRs targeting `master` is: + +* Ordinary changes: at least **one approval** from someone other than the author. +* Important changes with the `major-change` label: at least **three approvals**. + +Approvals must come from users with Write or higher repository permissions. +Invitations, comments, author approvals, bot reviews, and dismissed approvals do +not count. A later approval or change request supersedes that user's earlier +decision; a comment does not cancel an approval. Existing native protections +still apply, including blocking unresolved change requests. + +Maintainers should add `major-change` before review for public API changes, +architecture changes, breaking changes, or other high-impact modifications. +Do not remove the label merely to bypass the three-approval requirement. +This is a project policy, not an ASF-wide requirement for every GitHub PR. + +The [approval workflow](.github/workflows/review-approval-policy.yml) publishes +the commit status **Review approval policy**. It refreshes all open, non-draft +PRs on PR/label changes and after the +[read-only review notification workflow](.github/workflows/review-changed.yml) +completes. This two-workflow design supports fork reviews without granting the +notification workflow secrets or write permissions, or running PR code in the +privileged workflow. Neither workflow requires `CORE_REVIEWERS_TOKEN`. + +Changes are processed asynchronously. Wait for the approval workflow to finish +after changing labels or reviews, then check the status before merging. +Maintainers can manually run **Review approval policy** to refresh existing PRs. +PRs sharing a head commit also share its status, so all such open, non-draft PRs +must meet their individual approval requirements. + +To activate enforcement without a gap: + +1. Merge these workflows into `master` while retaining the current three-approval + native rule. +2. Run **Review approval policy** and verify its commit status on open PRs. +3. In the `main` ruleset, require the **Review approval policy** status check from + **GitHub Actions**, then lower the native required approvals to **one** in the + same ruleset update. Preserve the existing bypass policy. + +Do not lower the native approval count before the required status check is +configured. Team-based automatic invitations additionally require the secret +described above, but tiered approval enforcement does not. + +Run the reviewer and approval workflow tests with: + +```sh +node --test .github/tests/auto-reviewers.test.cjs .github/tests/review-approval-policy.test.cjs +``` + ## AI Usage The use of AI agents is allowed for generating bugfixes, unit tests and documentation.