diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index 78d96a1f62..16c4a435e1 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -15,7 +15,7 @@ The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes ## Approval scoring -The weighted approval workflow publishes the `weighted approval` commit status on the pull request head. Branch rules must require this status with GitHub Actions as its expected source; a context-only requirement can accept a same-named status from another integration. The status succeeds at two approval points, remains pending below two points or while the pull request is a draft, fails while a write-capable reviewer has an effective `CHANGES_REQUESTED` review, and reports an error when policy evaluation fails. +The weighted approval workflow publishes the `weighted approval` commit status on the pull request head. Branch rules must require this status with GitHub Actions as its expected source; a context-only requirement can accept a same-named status from another integration. A completed evaluation returns `success` at two approval points and `pending` below two points, while the pull request is a draft, or while a write-capable reviewer has an effective `CHANGES_REQUESTED` review. If evaluation fails, the publisher writes an `error` status. Reviewers whose calculated base repository permission is `write` or `admin` count. The [approval policy](approval-policy.json) gives `@07akioni`, `@imccyu`, `@tianyicui`, `@tianyicui-bot`, `@turtle1999`, and `@turtle2099` two points each; every other write-capable reviewer gets one point. The pull-request author and reviewers without write permission do not count. diff --git a/.github/review-ownership/check-approval.mjs b/.github/review-ownership/check-approval.mjs index 78f81acb56..d8e40f7731 100644 --- a/.github/review-ownership/check-approval.mjs +++ b/.github/review-ownership/check-approval.mjs @@ -129,7 +129,7 @@ export async function listPullRequestReviews(api, repository, pullNumber) { /** * Evaluate approval points from current reviews and repository permissions. * @param {{event: unknown, policySource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise}} options Runtime inputs. - * @returns {Promise<{pull: {repository: string, number: number, headSha: string}, state: 'failure' | 'pending' | 'success', description: string, points: number, requiredPoints: number, approvals: Array<{login: string, points: number}>, blockers: string[], ignoredReviewers: string[]}>} Approval decision and status payload fields. + * @returns {Promise<{pull: {repository: string, number: number, headSha: string}, state: 'pending' | 'success', description: string, points: number, requiredPoints: number, approvals: Array<{login: string, points: number}>, blockers: string[], ignoredReviewers: string[]}>} Approval decision and status payload fields. */ export async function evaluateApproval({ event, policySource, api }) { const pull = pullRequestFromEvent(event) @@ -169,7 +169,7 @@ export async function evaluateApproval({ event, policySource, api }) { return next }, 0) if (blockers.length > 0) { - return approvalResult(pull, policy.requiredPoints, approvals, blockers, ignoredReviewers, 'failure', + return approvalResult(pull, policy.requiredPoints, approvals, blockers, ignoredReviewers, 'pending', `${blockers.length} blocking change request${blockers.length === 1 ? '' : 's'}`) } const state = points >= policy.requiredPoints ? 'success' : 'pending' diff --git a/.github/review-ownership/check-approval.test.mjs b/.github/review-ownership/check-approval.test.mjs index da8bc6ee4d..a79454273d 100644 --- a/.github/review-ownership/check-approval.test.mjs +++ b/.github/review-ownership/check-approval.test.mjs @@ -206,11 +206,13 @@ test('ignores a reviewer whose collaborator permission lookup returns 404', asyn assert.deepEqual(result.ignoredReviewers, ['former-writer']) }) -test('blocks on a write-capable change request but ignores the author and read-only blockers', async () => { - const result = await evaluateApproval({ +test('keeps a write-capable change request pending but ignores the author and read-only blockers', async () => { + const statuses = [] + const result = await runApprovalCheck({ event: pullRequestEvent({ author: 'author' }), policySource, - api: async (path) => { + runUrl: 'https://github.example/actions/runs/1', + api: async (path, options = {}) => { if (path.includes('/reviews?')) { return [ review('turtle1999', 'APPROVED'), @@ -222,13 +224,25 @@ test('blocks on a write-capable change request but ignores the author and read-o if (path.includes('/collaborators/turtle1999/permission')) return { permission: 'admin' } if (path.includes('/collaborators/blocker/permission')) return { permission: 'write' } if (path.includes('/collaborators/reader/permission')) return { permission: 'read' } + if (path.includes('/statuses/')) { + statuses.push(options.body) + return {} + } throw new Error(`unexpected API path ${path}`) }, + write: () => {}, }) - assert.equal(result.state, 'failure') + assert.equal(result.state, 'pending') + assert.equal(result.description, '1 blocking change request.') assert.equal(result.points, 2) assert.deepEqual(result.blockers, ['blocker']) assert.deepEqual(result.ignoredReviewers, ['reader']) + assert.deepEqual(statuses, [{ + state: 'pending', + context: 'weighted approval', + description: '1 blocking change request.', + target_url: 'https://github.example/actions/runs/1', + }]) }) test('keeps drafts pending without reading reviews', async () => {