ci: review external PRs after CI with Copilot allowance fallback (#332)

* ci: request one Copilot review after required checks pass

* ci: route exhausted Copilot allowance to CodeRabbit

---------

Co-authored-by: kuddev <kuddev@users.noreply.github.com>
This commit is contained in:
Kuddevandkuddev authored and GitHub committed 2026-09-28 11:55:22 +08:00
1 parent 8087306b2b
commit 914f427332
6 files changed
+529 -1

No files matched your search

+20
View File
@@ -0,0 +1,20 @@
# yaml-language-server: $schema=https://www.coderabbit.ai/schema/schema.v2.json
language: en-US
reviews:
profile: chill
request_changes_workflow: false
auto_review:
enabled: false
auto_incremental_review: false
drafts: false
ignore_usernames:
- Kuddev
path_instructions:
- path: "**/*"
instructions: >-
Follow AGENTS.md and the closest module instructions. Prioritize actionable
correctness, regression, security, state/lifetime and compatibility issues.
Give the triggering scenario and concrete code evidence. Distinguish
confirmed defects from questions; avoid speculative redesign and formatting
comments. CI and maintainer review remain the merge constraints. Never
recommend weakening required checks, raising budgets, or removing tests.
+19
View File
@@ -0,0 +1,19 @@
# Review guidance
Review concrete regressions and missing boundary cases in the changed code. Explain
an actionable finding with the trigger, affected behavior, and file/line evidence.
Distinguish confirmed defects from questions; avoid speculative redesigns and style
comments already covered by formatting. Do not infer success from a passing build.
Follow `AGENTS.md` and the closest module instructions, `CONTRIBUTING.md`,
`docs/architecture.md`, and `docs/project-constraints.md`. Treat them as the
source of project policy rather than inventing a second rule set. Check ownership,
dependency direction, cancellation/lifetime, persistence compatibility, and tests
appropriate to the change. Consult `docs/internationalization.md` for UI text and
`packaging/AGENTS.md` for release changes. Verify performance claims against their
stated workload and separate platform tests from visual acceptance.
CI remains the deterministic merge gate. Do not recommend weakening required
checks, widening budgets, deleting tests, or treating this automated review as
maintainer approval. For CI changes, look for missing-platform, skipped-test,
stale-commit, cache-poisoning, fork-permission, and failure-propagation regressions.
+147
View File
@@ -0,0 +1,147 @@
name: AI review after CI
on:
workflow_run:
workflows: [Full native tests, Architecture contracts, PR size]
types: [completed]
# Metadata only: never check out or execute pull-request content with this token.
pull_request_target:
types: [ready_for_review]
workflow_dispatch:
inputs:
pull_number:
description: Existing PR to evaluate with the same eligibility and CI checks
required: true
type: string
permissions:
actions: read
checks: read
contents: read
issues: write
pull-requests: write
# A pending event may be replaced. Every run scans eligible open PRs, so the
# retained event still evaluates all PRs, and only one request writer runs at once.
concurrency:
group: ai-first-review
cancel-in-progress: false
jobs:
request-review:
if: github.event_name != 'workflow_run' || github.event.workflow_run.conclusion == 'success'
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- uses: actions/github-script@v7
env:
COPILOT_REVIEW_TOKEN: ${{ secrets.COPILOT_REVIEW_TOKEN }}
CODERABBIT_FALLBACK_ENABLED: ${{ vars.CODERABBIT_FALLBACK_ENABLED }}
with:
retries: 0
script: |
const marker = 'ai-review-requested';
const previousMarker = 'copilot-review-requested';
const bot = 'copilot-pull-request-reviewer[bot]';
const fallbackBot = 'coderabbitai[bot]';
const same = (a, b) => a?.toLowerCase() === b?.toLowerCase();
const report = (number, text) => core.info(`#${number}: ${text}`);
const eligible = pr => pr.state === 'open' && pr.draft === false &&
pr.user?.type === 'User' && !same(pr.user.login, context.repo.owner);
let pulls;
if (context.eventName === 'workflow_dispatch') {
const value = context.payload.inputs?.pull_number;
if (!/^[1-9][0-9]*$/.test(value || '')) throw new Error('Invalid PR number');
pulls = [(await github.rest.pulls.get({
...context.repo, pull_number: Number(value),
})).data];
} else {
pulls = await github.paginate(github.rest.pulls.list, {
...context.repo, state: 'open', per_page: 100,
});
}
const policies = new Map();
let reviewer;
const provider = async () => {
if (!reviewer) {
const token = process.env.COPILOT_REVIEW_TOKEN;
if (!token) throw new Error('Missing dedicated COPILOT_REVIEW_TOKEN secret');
reviewer = new github.constructor({auth: token, retry: {enabled: false}});
const user = (await reviewer.rest.users.getAuthenticated()).data;
if (!same(user.login, context.repo.owner)) throw new Error('Review token must belong to the repository owner');
}
// This account endpoint is also used by GitHub's Copilot clients.
// Unknown or changed responses stop requests; never infer exhaustion
// from an authentication failure, a rate limit, or a network error.
const data = (await reviewer.request('GET /copilot_internal/user')).data;
const quota = data.quota_snapshots?.premium_interactions;
if (!quota || typeof quota.unlimited !== 'boolean' ||
!Number.isFinite(quota.quota_remaining) || quota.quota_remaining < 0 ||
!Number.isFinite(Date.parse(quota.timestamp_utc)) ||
Math.abs(Date.now() - Date.parse(quota.timestamp_utc)) > 5 * 60 * 1000) {
throw new Error('Copilot allowance is missing, stale or unrecognized; no review requested');
}
if (quota.overage_permitted !== false) {
throw new Error('Paid overage must remain disabled before automated requests');
}
core.info(`Copilot allowance remaining: ${quota.quota_remaining}; reset: ${data.quota_reset_date_utc}`);
if (quota.unlimited || quota.quota_remaining > 0) return 'copilot';
if (process.env.CODERABBIT_FALLBACK_ENABLED !== 'true') {
throw new Error('Copilot exhausted; CodeRabbit installation must be verified before enabling fallback');
}
return 'coderabbit';
};
for (const pr of pulls) {
const number = pr.number;
if (!eligible(pr)) { report(number, 'Owner, bot, draft or closed PR; skipped.'); continue; }
const args = {...context.repo, pull_number: number};
const claimed = item => item.labels.some(label => [marker, previousMarker].includes(label.name)) ||
item.requested_reviewers.some(user => same(user.login, bot));
if (claimed(pr)) { report(number, 'Review already requested; skipped.'); continue; }
const reviews = await github.paginate(github.rest.pulls.listReviews, {...args, per_page: 100});
if (reviews.some(review => [bot, fallbackBot].some(login => same(review.user?.login, login)))) {
report(number, 'An AI reviewer has already reviewed this PR; manual re-review only.'); continue;
}
if (!policies.has(pr.base.ref)) {
const rules = await github.paginate('GET /repos/{owner}/{repo}/rules/branches/{branch}', {
...context.repo, branch: pr.base.ref, per_page: 100,
});
policies.set(pr.base.ref, rules.filter(rule => rule.type === 'required_status_checks')
.flatMap(rule => rule.parameters.required_status_checks));
}
const required = policies.get(pr.base.ref);
if (!required.length) { report(number, 'No required CI policy; manual review only.'); continue; }
const ref = pr.head.sha;
const checks = await github.paginate(github.rest.checks.listForRef, {
...context.repo, ref, filter: 'latest', per_page: 100,
});
const passed = required.every(rule => {
const matching = checks.filter(check => check.name === rule.context &&
(rule.integration_id == null || rule.integration_id === -1 || check.app?.id === rule.integration_id));
matching.sort((a, b) => b.id - a.id);
return matching[0]?.head_sha === ref && matching[0]?.status === 'completed' &&
matching[0]?.conclusion === 'success';
});
if (!passed) { report(number, 'Required CI incomplete or unsuccessful; deferred.'); continue; }
const selected = await provider();
// Guard a push, draft conversion or close racing the checks above.
const current = (await github.rest.pulls.get(args)).data;
if (!eligible(current) || current.head.sha !== ref || current.base.ref !== pr.base.ref || claimed(current)) {
report(number, 'PR changed while checking; deferred.'); continue;
}
// Persist the one-shot claim before the non-idempotent review request.
// On a timeout/error leave the claim in place: a maintainer must inspect
// the result before manually re-requesting, preventing quota retry loops.
await github.rest.issues.addLabels({
...context.repo, issue_number: number, labels: [marker, `${selected}-review-requested`],
});
if (selected === 'copilot') {
await reviewer.rest.pulls.requestReviewers({...args, reviewers: [bot]});
} else {
await reviewer.rest.issues.createComment({
...context.repo, issue_number: number,
body: '@coderabbitai review',
});
}
report(number, `Requested first ${selected} review after ${required.length} required checks passed.`);
}
+2 -1
View File
@@ -63,7 +63,8 @@ jobs:
scripts.tests.test_stable_release \ scripts.tests.test_stable_release \
scripts.tests.test_ci_cache \ scripts.tests.test_ci_cache \
scripts.tests.test_platform_cfg \ scripts.tests.test_platform_cfg \
scripts.tests.test_pr_size_workflow -v scripts.tests.test_pr_size_workflow \
scripts.tests.test_copilot_review_workflow -v
- name: Check platform cfg budget before native jobs - name: Check platform cfg budget before native jobs
run: python3 scripts/check_platform_cfg.py run: python3 scripts/check_platform_cfg.py
- name: Plan native validation - name: Plan native validation
@@ -0,0 +1,98 @@
# AI review after required CI, with an allowance fallback
## Status
Implemented; activation requires the default-branch workflow, a dedicated owner
credential, and verified CodeRabbit installation before enabling its fallback.
## Context
Reviews of every push spend allowance on drafts, failed builds, and changes that
already have an initial review. Native branch rules do not express the requested
combination of authorship, readiness, CI, review history, and allowance routing.
## Evidence
GitHub exposes effective required checks through the branch rules API and results
for a specific commit. The reviewer API accepts `copilot-pull-request-reviewer[bot]`.
A user-authenticated request produced a review, whereas an Actions-token probe
returned success without a Copilot run or review. API acceptance alone therefore
cannot establish delivery. A user credential is required for activation testing.
The authenticated Copilot client endpoint exposes a timestamped allowance snapshot,
including remaining credits and whether paid overage is permitted. This endpoint
is not a stable public billing contract; response drift must stop automation.
## Decision
The metadata-only [workflow](../../../../.github/workflows/copilot-review.yml)
requests one review for an open, non-draft PR by a human other than the repository
owner, after every required check on its current head succeeds. It reads effective
server-side branch rules rather than copying their check list. No required-check
policy means no automatic review.
Completed CI workflows and draft-to-ready transitions trigger eligibility scans.
A repository-wide concurrency group serializes requests; scanning open PRs covers
events replaced in GitHub's pending slot. Dispatch evaluates one existing PR with
the same guards. Previous Copilot or CodeRabbit reviews prevent automatic repeats.
The workflow's built-in token reads PR/CI metadata and writes claim labels.
`COPILOT_REVIEW_TOKEN` must belong to the repository owner and is used only for
account allowance and user-attributed review requests/comments; it needs no Checks,
Actions or Contents permission for metadata reads. A positive Copilot
allowance selects Copilot. Zero allowance selects CodeRabbit only when
`CODERABBIT_FALLBACK_ENABLED=true`, after installation has been verified. Missing,
negative, stale, unrecognized, or unreadable allowance data stops requests; an
ordinary permission/network error never implies exhaustion. Paid overage must be
disabled. Credits are not a fixed number of reviews, and consumption reporting can
lag; this router does not establish a provider-side spending cap.
A persistent `ai-review-requested` label is written before requesting either bot;
older `copilot-review-requested` claims remain recognized. On uncertain delivery or
failure, keep the claim and require manual inspection before retrying. No automatic
retry loops or default dual review. CodeRabbit's own automatic and incremental
reviews are disabled in the repository config; the fallback uses an explicit
`@coderabbitai review` request. Instructions prioritize evidence-backed defects.
The workflow never checks out PR code, runs artifacts, interpolates PR text into
commands, approves a PR, or merges changes. Human review and required CI remain
unchanged. The dedicated credential is a repository secret, never checked in;
provision and validate it separately from the workflow's source.
## Rejected alternatives
- Branch-wide rules or every-push reviews: cannot enforce the requested policy.
- Executing fork code with privileged credentials: unnecessary for metadata routing.
- Treating any Copilot error as exhausted credit: hides outages and permission bugs.
- In-memory deduplication or uncertain retries: can request two paid reviews.
- Hard-coded check names: drift from the effective merge requirements.
- Copying a developer's broad CLI credential into Actions without explicit setup.
## Consequences
Maintainers request subsequent reviews manually. A push can race the final API
call; rechecking head/draft/base/claim immediately before submission narrows but
cannot eliminate that race. An accepted request followed by asynchronous service
failure remains for manual inspection rather than risking duplicate reviews.
Unprotected target branches and legacy status-only checks require manual review.
This is supplemental automation, not another mandatory merge check.
## Validation
Offline fixtures execute the actual script: successful external PR; owner, bot,
draft and closed exclusions; missing/failed/skipped checks; newer pending attempts;
wrong app/head; racing changes; review history; claims; policy/quota failures;
positive versus exhausted allowance; disabled fallback; and no double request.
The required lint job runs the contract before scheduling native runners.
Actual owner-secret delivery and CodeRabbit response must be verified on GitHub
before claiming end-to-end activation. Do not consume the real allowance merely
to force exhaustion; the zero-credit routing boundary is covered with fixtures.
## Supersedes
None.
## Revisit when
GitHub exposes a stable allowance API or equivalent native routing, supports atomic
SHA-bound review requests, required-check providers change, or ownership changes.
@@ -0,0 +1,243 @@
from __future__ import annotations
import json
from datetime import datetime, timezone
from pathlib import Path
import shutil
import subprocess
import textwrap
import unittest
ROOT = Path(__file__).resolve().parents[2]
RUNNER = r'''
const input = JSON.parse(require('node:fs').readFileSync(0, 'utf8'));
const writes = [], logs = [];
process.env.COPILOT_REVIEW_TOKEN = input.missingToken ? '' : 'fixture-token';
process.env.CODERABBIT_FALLBACK_ENABLED = input.fallbackEnabled ? 'true' : 'false';
const list = () => {}, listReviews = () => {}, listForRef = () => {};
const github = {
rest: {
pulls: {
list, listReviews,
get: async () => ({data: input.current || input.pr}),
},
checks: {listForRef},
issues: {addLabels: async value => {
if (input.labelError) throw new Error('claim failed');
writes.push(['claim', value]);
}},
},
paginate: async (method, args) => {
if (method === list) return [input.pr];
if (method === listReviews) return input.reviews || [];
if (method === listForRef) return input.checks;
if (typeof method === 'string' && method.includes('/rules/branches/')) {
if (input.policyError) throw new Error('policy unavailable');
return input.rules;
}
throw new Error('unexpected API');
},
};
const reviewer = {
rest: {
users: {getAuthenticated: async () => ({data: {login: input.tokenOwner || 'owner'}})},
pulls: {requestReviewers: async value => {
writes.push(['request', value]);
if (input.requestError) throw new Error('request failed');
}},
issues: {createComment: async value => writes.push(['fallback', value])},
},
request: async () => {
if (input.quotaError) throw new Error('quota API failed');
return {data: {quota_snapshots: {premium_interactions: input.quota || {
unlimited: false, quota_remaining: 100, overage_permitted: false,
timestamp_utc: new Date().toISOString(),
}}}};
},
};
github.constructor = class {
constructor({auth}) {
if (auth !== 'fixture-token') throw new Error('Wrong credential');
return reviewer;
}
};
const context = {repo: {owner: 'owner', repo: 'project'}, eventName: input.event || 'workflow_run',
payload: {inputs: {pull_number: input.number || '7'}}};
const core = {info: value => logs.push(value)};
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
new AsyncFunction('github', 'context', 'core', input.script)(github, context, core)
.then(() => process.stdout.write(JSON.stringify({writes, logs})))
.catch(error => process.stdout.write(JSON.stringify({writes, logs, error: error.message})));
'''
class CopilotReviewWorkflowTests(unittest.TestCase):
@classmethod
def setUpClass(cls):
cls.node = shutil.which('node')
if not cls.node:
raise RuntimeError('Node.js is required for the Copilot workflow contract')
cls.workflow = (ROOT / '.github/workflows/copilot-review.yml').read_text(encoding='utf-8')
cls.script = textwrap.dedent(cls.workflow.split(' script: |\n', 1)[1])
def fixture(self):
return {
'pr': {'number': 7, 'state': 'open', 'draft': False,
'user': {'login': 'contributor', 'type': 'User'},
'head': {'sha': 'current'}, 'base': {'ref': 'main'},
'labels': [], 'requested_reviewers': []},
'rules': [{'type': 'required_status_checks', 'parameters': {
'required_status_checks': [{'context': 'native', 'integration_id': 15368},
{'context': 'architecture', 'integration_id': 15368}]}}],
'checks': [{'id': i, 'name': name, 'head_sha': 'current', 'status': 'completed',
'conclusion': 'success', 'app': {'id': 15368}}
for i, name in enumerate(('native', 'architecture'), 1)],
}
def run_gate(self, data):
result = subprocess.run([self.node, '-e', RUNNER],
input=json.dumps({'script': self.script, **data}),
text=True, encoding='utf-8', capture_output=True, check=True)
return json.loads(result.stdout)
def test_external_ready_pr_claimed_then_requested_after_all_required_ci(self):
result = self.run_gate(self.fixture())
self.assertEqual([row[0] for row in result['writes']], ['claim', 'request'])
self.assertEqual(result['writes'][1][1]['reviewers'], ['copilot-pull-request-reviewer[bot]'])
def test_owner_draft_closed_and_bot_prs_never_request(self):
for change in ({'user': {'login': 'OwNeR', 'type': 'User'}}, {'draft': True},
{'state': 'closed'}, {'user': {'login': 'dependabot', 'type': 'Bot'}}):
with self.subTest(change=change):
data = self.fixture()
data['pr'].update(change)
self.assertEqual(self.run_gate(data)['writes'], [])
def test_failed_pending_skipped_neutral_missing_wrong_app_and_old_checks_defer(self):
for change in ({'conclusion': 'failure'}, {'status': 'in_progress'},
{'conclusion': 'skipped'}, {'conclusion': 'neutral'},
{'name': 'unrelated'}, {'app': {'id': 123}}, {'head_sha': 'old'}):
with self.subTest(change=change):
data = self.fixture()
data['checks'][0].update(change)
self.assertEqual(self.run_gate(data)['writes'], [])
def test_newer_pending_attempt_cannot_reuse_old_success(self):
data = self.fixture()
data['checks'].append({**data['checks'][0], 'id': 99, 'status': 'queued', 'conclusion': None})
self.assertEqual(self.run_gate(data)['writes'], [])
def test_previous_review_on_any_commit_prevents_automatic_rereview(self):
data = self.fixture()
data['reviews'] = [{'user': {'login': 'copilot-pull-request-reviewer[bot]'}, 'commit_id': 'old'}]
self.assertEqual(self.run_gate(data)['writes'], [])
def test_pending_request_or_persistent_claim_prevents_duplicates(self):
for field, value in (
('labels', [{'name': 'copilot-review-requested'}]),
('requested_reviewers', [{'login': 'copilot-pull-request-reviewer[bot]'}]),
):
data = self.fixture()
data['pr'][field] = value
self.assertEqual(self.run_gate(data)['writes'], [])
def test_push_draft_close_base_change_or_claim_during_reads_defers(self):
for change in ({'head': {'sha': 'new'}}, {'draft': True}, {'state': 'closed'},
{'base': {'ref': 'release'}}, {'labels': [{'name': 'copilot-review-requested'}]}):
with self.subTest(change=change):
data = self.fixture()
data['current'] = {**data['pr'], **change}
self.assertEqual(self.run_gate(data)['writes'], [])
def test_missing_or_unreadable_policy_fails_closed(self):
data = self.fixture()
data['rules'] = []
self.assertEqual(self.run_gate(data)['writes'], [])
data['policyError'] = True
self.assertEqual(self.run_gate(data)['error'], 'policy unavailable')
def test_write_failure_does_not_retry_or_erase_claim(self):
data = self.fixture()
data['requestError'] = True
result = self.run_gate(data)
self.assertEqual(result['error'], 'request failed')
self.assertEqual([row[0] for row in result['writes']], ['claim', 'request'])
data['labelError'] = True
self.assertEqual(self.run_gate(data)['writes'], [])
def test_manual_backfill_uses_same_gate_and_validates_number(self):
data = {**self.fixture(), 'event': 'workflow_dispatch'}
self.assertEqual(len(self.run_gate(data)['writes']), 2)
data['number'] = '7; arbitrary code'
self.assertEqual(self.run_gate(data)['error'], 'Invalid PR number')
def test_quota_exhaustion_uses_only_enabled_coderabbit(self):
data = self.fixture()
data['quota'] = {'unlimited': False, 'quota_remaining': 0, 'overage_permitted': False,
'timestamp_utc': datetime.now(timezone.utc).isoformat()}
self.assertEqual(self.run_gate(data)['writes'], [])
data['fallbackEnabled'] = True
result = self.run_gate(data)
self.assertEqual([row[0] for row in result['writes']], ['claim', 'fallback'])
self.assertEqual(result['writes'][1][1]['body'], '@coderabbitai review')
def test_quota_errors_stale_data_paid_overage_and_wrong_identity_fail_closed(self):
for extra in ({'quotaError': True}, {'tokenOwner': 'other'},
{'quota': {'unlimited': False, 'quota_remaining': -1}},
{'quota': {'unlimited': False, 'quota_remaining': 10,
'overage_permitted': True, 'timestamp_utc': '2020-01-01T00:00:00Z'}}):
with self.subTest(extra=extra):
data = {**self.fixture(), **extra, 'fallbackEnabled': True}
result = self.run_gate(data)
self.assertEqual(result['writes'], [])
self.assertIn('error', result)
def test_paid_overage_or_stale_allowance_never_routes_to_either_bot(self):
for quota in (
{'unlimited': False, 'quota_remaining': 50, 'overage_permitted': True,
'timestamp_utc': datetime.now(timezone.utc).isoformat()},
{'unlimited': False, 'quota_remaining': 0, 'overage_permitted': False,
'timestamp_utc': '2020-01-01T00:00:00Z'},
):
data = {**self.fixture(), 'quota': quota, 'fallbackEnabled': True}
self.assertEqual(self.run_gate(data)['writes'], [])
def test_missing_user_token_does_not_spend_or_claim(self):
result = self.run_gate({**self.fixture(), 'missingToken': True})
self.assertEqual(result['writes'], [])
self.assertIn('Missing dedicated', result['error'])
def test_user_token_has_no_ci_reading_methods(self):
# The two clients deliberately expose disjoint APIs in the fixture.
result = self.run_gate(self.fixture())
self.assertEqual([row[0] for row in result['writes']], ['claim', 'request'])
self.assertNotIn('github-token: ${{ secrets.', self.workflow)
def test_available_copilot_does_not_trigger_enabled_fallback(self):
result = self.run_gate({**self.fixture(), 'fallbackEnabled': True})
self.assertEqual([row[0] for row in result['writes']], ['claim', 'request'])
def test_coderabbit_config_disables_independent_automatic_reviews(self):
config = (ROOT / '.coderabbit.yaml').read_text(encoding='utf-8')
for entry in ('enabled: false', 'auto_incremental_review: false', 'drafts: false', '- Kuddev'):
self.assertIn(entry, config)
def test_coderabbit_does_not_duplicate_previous_ai_review(self):
data = self.fixture()
data['reviews'] = [{'user': {'login': 'coderabbitai[bot]'}}]
self.assertEqual(self.run_gate(data)['writes'], [])
def test_workflow_never_executes_fork_content_or_approves_merges(self):
self.assertNotIn('actions/checkout', self.workflow)
self.assertIn('secrets.COPILOT_REVIEW_TOKEN', self.workflow)
self.assertNotIn('contents: write', self.workflow)
self.assertNotIn('createReview', self.script)
self.assertNotIn('synchronize', self.workflow)
self.assertIn('types: [ready_for_review]', self.workflow)
self.assertIn("github.event.workflow_run.conclusion == 'success'", self.workflow)
self.assertIn('retries: 0', self.workflow)
self.assertIn('cancel-in-progress: false', self.workflow)
if __name__ == '__main__':
unittest.main()