From 4a4062d09e48813cbf99883eddfb4a14f10b097d Mon Sep 17 00:00:00 2001 From: chichuan Date: Tue, 25 Aug 2026 19:20:57 +0800 Subject: [PATCH] fix(ci): bind required checks to GitHub Actions --- .changes/reviewer-router-synchronous-merge.md | 2 +- .github/workflows/reviewer-router.yml | 30 +++++++++-- docs/automation.md | 6 ++- docs/ci-pr-gates.md | 10 ++-- .../reviewer_router_required_rulesets_test.go | 50 ++++++++++++++++--- 5 files changed, 80 insertions(+), 18 deletions(-) diff --git a/.changes/reviewer-router-synchronous-merge.md b/.changes/reviewer-router-synchronous-merge.md index 5b0c2b3c..176c3517 100644 --- a/.changes/reviewer-router-synchronous-merge.md +++ b/.changes/reviewer-router-synchronous-merge.md @@ -2,4 +2,4 @@ category: Fixed --- -- **Reviewer Router merge recovery** — retries exact App-owned merge intents through a SHA-bound synchronous merge after GitHub has enforced approval and required checks. +- **Reviewer Router merge recovery** — retries exact App-owned merge intents through a SHA-bound synchronous merge after GitHub has enforced approval and nine GitHub Actions source-bound required checks. diff --git a/.github/workflows/reviewer-router.yml b/.github/workflows/reviewer-router.yml index 858800f3..f2e2eb64 100644 --- a/.github/workflows/reviewer-router.yml +++ b/.github/workflows/reviewer-router.yml @@ -850,6 +850,10 @@ jobs: 'CLI Smoke', 'Mock MCP', ]; + // GitHub Actions is the sole reviewed producer for every required + // context. A context-only ruleset can be satisfied by another App + // or a legacy commit status with the same name. + const requiredCheckIntegrationID = 15368; function hasExactStringSet(values, expected) { return ( Array.isArray(values) && @@ -858,6 +862,23 @@ jobs: expected.every(value => values.includes(value)) ); } + function hasExactRequiredStatusChecks(checks) { + if ( + !Array.isArray(checks) || + checks.length !== requiredCheckContexts.length + ) { + return false; + } + const contexts = checks.map(check => check?.context); + return ( + new Set(contexts).size === requiredCheckContexts.length && + checks.every( + check => + requiredCheckContexts.includes(check?.context) && + check?.integration_id === requiredCheckIntegrationID, + ) + ); + } function hasExactMainRulesetScope(ruleset) { const includes = ruleset?.conditions?.ref_name?.include; const excludes = ruleset?.conditions?.ref_name?.exclude; @@ -917,13 +938,12 @@ jobs: return false; } const parameters = ruleset.rules[0].parameters; - const contexts = parameters?.required_status_checks?.map( - check => check?.context, - ); return ( parameters?.strict_required_status_checks_policy === true && parameters.do_not_enforce_on_create === false && - hasExactStringSet(contexts, requiredCheckContexts) + hasExactRequiredStatusChecks( + parameters.required_status_checks, + ) ); } // GitHub's read projection omits the entire parameters property @@ -1077,7 +1097,7 @@ jobs: !isExactMainQualityRuleset(qualityRulesets[0]) ) { throw new Error( - 'Reviewer Router App requires the exact non-bypassable main-quality ruleset with nine strict checks.', + 'Reviewer Router App requires the exact non-bypassable main-quality ruleset with nine strict GitHub Actions checks.', ); } for (const ruleset of activeMainRulesets) { diff --git a/docs/automation.md b/docs/automation.md index d0fdd576..9ab57f66 100644 --- a/docs/automation.md +++ b/docs/automation.md @@ -155,8 +155,10 @@ before calling the synchronous PR merge endpoint with the exact current head SHA. The preflight requires exactly one repository-owned `main-protection` ruleset with one latest-head approval and exactly one repository-owned `main-quality` ruleset with the reviewed nine strict checks. The App must report -`never` on both and on every other non-writer ruleset, so deletion or weakening -of either gate fails closed before merge. HTTP 405 means the PR is not ready, +`never` on both and on every other non-writer ruleset. Every required context +must be bound to the GitHub Actions App (`integration_id=15368`); a missing, +different, or duplicate context/source entry fails closed together with +deletion or weakening of either gate. HTTP 405 means the PR is not ready, while 409 means its revision changed; either remains open for the next event. Other failures make reconciliation red. A concurrent native merge is accepted only after the final diff --git a/docs/ci-pr-gates.md b/docs/ci-pr-gates.md index 703a2d69..2730c99e 100644 --- a/docs/ci-pr-gates.md +++ b/docs/ci-pr-gates.md @@ -214,8 +214,10 @@ each attempt it revalidates the App's ruleset boundary and PR intent, supplies the current head SHA, and treats server-declared not-ready or concurrent-revision responses as retriable. The live preflight requires the exact repository-owned approval ruleset and exact nine-check strict quality -ruleset, with the App unable to bypass either; a missing, disabled, or weakened -gate fails closed before merge. GitHub—not the workflow—decides whether the +ruleset, with every context bound to the GitHub Actions App +(`integration_id=15368`) and the Reviewer Router App unable to bypass either; +a missing, disabled, incorrectly sourced, or weakened gate fails closed before +merge. GitHub—not the workflow—decides whether the merge is admissible. A staggered twice-hourly schedule provides eventual recovery, and a manual `workflow_dispatch` from `main` is the immediate idempotent retry path. @@ -403,7 +405,9 @@ tool、parameter、mapping、positional execution、constraint 与 safety 语义 The `main` quality ruleset must enable strict required-status-check policy (`strict_required_status_checks_policy=true`) so a PR is revalidated whenever -`main` advances. It must require these exact contexts and no legacy aliases: +`main` advances. Every entry must select the GitHub Actions App +(`integration_id=15368`), not “any source”. It must require these exact +context/source pairs and no legacy aliases: - `Lint` - `Test` diff --git a/test/scripts/reviewer_router_required_rulesets_test.go b/test/scripts/reviewer_router_required_rulesets_test.go index 7c33cd12..5af09901 100644 --- a/test/scripts/reviewer_router_required_rulesets_test.go +++ b/test/scripts/reviewer_router_required_rulesets_test.go @@ -21,6 +21,23 @@ const reviewerRouterRequiredRulesetHelpers = `function hasExactStringSet(values, expected.every(value => values.includes(value)) ); } +function hasExactRequiredStatusChecks(checks) { + if ( + !Array.isArray(checks) || + checks.length !== requiredCheckContexts.length + ) { + return false; + } + const contexts = checks.map(check => check?.context); + return ( + new Set(contexts).size === requiredCheckContexts.length && + checks.every( + check => + requiredCheckContexts.includes(check?.context) && + check?.integration_id === requiredCheckIntegrationID, + ) + ); +} function hasExactMainRulesetScope(ruleset) { const includes = ruleset?.conditions?.ref_name?.include; const excludes = ruleset?.conditions?.ref_name?.exclude; @@ -80,13 +97,12 @@ function isExactMainQualityRuleset(ruleset) { return false; } const parameters = ruleset.rules[0].parameters; - const contexts = parameters?.required_status_checks?.map( - check => check?.context, - ); return ( parameters?.strict_required_status_checks_policy === true && parameters.do_not_enforce_on_create === false && - hasExactStringSet(contexts, requiredCheckContexts) + hasExactRequiredStatusChecks( + parameters.required_status_checks, + ) ); }` @@ -122,6 +138,7 @@ func TestReviewerRouterRequiresExactApprovalAndQualityRulesets(t *testing.T) { t.Errorf("approval/quality ruleset helper count = %d, want 1", got) } for _, marker := range []string{ + "const requiredCheckIntegrationID = 15368;", "protectionRulesets.length !== 1", "protectionRulesets[0].current_user_can_bypass !== 'never'", "!isExactMainProtectionRuleset(protectionRulesets[0])", @@ -129,7 +146,7 @@ func TestReviewerRouterRequiresExactApprovalAndQualityRulesets(t *testing.T) { "qualityRulesets[0].current_user_can_bypass !== 'never'", "!isExactMainQualityRuleset(qualityRulesets[0])", "exact non-bypassable main-protection approval ruleset", - "exact non-bypassable main-quality ruleset with nine strict checks", + "exact non-bypassable main-quality ruleset with nine strict GitHub Actions checks", } { if !strings.Contains(script, marker) { t.Errorf("reconciliation script is missing live ruleset contract marker %q", marker) @@ -153,6 +170,7 @@ const requiredCheckContexts = [ 'CLI Smoke', 'Mock MCP', ]; +const requiredCheckIntegrationID = 15368; ` + reviewerRouterRequiredRulesetHelpers + ` const clone = value => JSON.parse(JSON.stringify(value)); const scope = { @@ -193,7 +211,10 @@ const quality = { parameters: { do_not_enforce_on_create: false, strict_required_status_checks_policy: true, - required_status_checks: requiredCheckContexts.map(context => ({context})), + required_status_checks: requiredCheckContexts.map(context => ({ + context, + integration_id: requiredCheckIntegrationID, + })), }, }], }; @@ -249,8 +270,23 @@ candidate = clone(quality); candidate.rules[0].parameters.required_status_checks[8].context = 'Lint'; invalidQuality.push(['duplicate check', candidate]); candidate = clone(quality); -candidate.rules[0].parameters.required_status_checks.push({context: 'Unexpected'}); +candidate.rules[0].parameters.required_status_checks.push({ + context: 'Unexpected', + integration_id: requiredCheckIntegrationID, +}); invalidQuality.push(['extra check', candidate]); +candidate = clone(quality); +delete candidate.rules[0].parameters.required_status_checks[0].integration_id; +invalidQuality.push(['missing integration', candidate]); +candidate = clone(quality); +candidate.rules[0].parameters.required_status_checks[0].integration_id = 1; +invalidQuality.push(['wrong integration', candidate]); +candidate = clone(quality); +candidate.rules[0].parameters.required_status_checks[8] = { + context: 'Lint', + integration_id: 1, +}; +invalidQuality.push(['duplicate context from another integration', candidate]); for (const [name, value] of invalidQuality) { if (isExactMainQualityRuleset(value)) { throw new Error(name + ' main-quality ruleset was accepted');