governance: merge queue 改 repo 级声明与对账（P2-7 修正，ADR-0042） - #118
Merged
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Comment |
PR Summary by Qodo将 merge queue 治理迁移至仓库级声明与对账
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Merge method always mismatches
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "template-service" | ||
| ], | ||
| "params": { | ||
| "merge_method": "squash", |
There was a problem hiding this comment.
2. Merge method always mismatches 🐞 Bug ≡ Correctness
The expected state stores merge_method as lowercase squash, while GitHub’s merge-queue REST representation uses the uppercase enum SQUASH. Consequently §14 reports parameter drift for the GraphQL-created ruleset even when it has exactly the intended merge method.
Agent Prompt
## Issue description
The expected merge-queue method uses lowercase `squash`, but the REST ruleset response uses the uppercase merge-queue enum `SQUASH`, causing permanent false drift.
## Issue Context
GraphQL creates the rule with `mergeMethod: SQUASH`, and drift-check compares the REST response directly with expected-state JSON.
## Fix Focus Areas
- governance/expected-state.json[89-96]
- governance/drift-check.sh[575-580]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+576
to
+579
| got_p=$(jq -c '.rules[] | select(.type == "merge_queue") | .parameters | ||
| | {merge_method, check_response_timeout_minutes, max_entries_to_build, | ||
| min_entries_to_merge, max_entries_to_merge, min_entries_to_merge_wait_minutes, | ||
| grouping_strategy}' <<<"$detail") |
There was a problem hiding this comment.
3. Disabled queue passes reconciliation 🐞 Bug ≡ Correctness
Section 14 compares only the merge_queue rule parameters and never verifies ruleset enforcement, target, or default-branch conditions. A ruleset changed to disabled evaluation or made inapplicable to the default branch therefore still produces an OK result despite the expected merge queue no longer being enforced.
Agent Prompt
## Issue description
The merge-queue drift check validates only queue parameters, allowing disabled or inapplicable rulesets to pass.
## Issue Context
The creation mutation explicitly requires active enforcement, a branch target, and the default-branch condition, so these fields are part of the effective desired state even though §14 ignores them.
## Fix Focus Areas
- governance/drift-check.sh[573-581]
- governance/apply.sh[151-151]
- governance/expected-state.json[83-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| jq -e --arg r "$r" '.merge_queue.repos // [] | index($r) != null' "$EXPECTED" >/dev/null && continue | ||
| RS=$(api "https://api.github.com/repos/$ORG/$r/rulesets?per_page=100") | ||
| jq -e 'type == "array"' <<<"$RS" >/dev/null 2>&1 || continue | ||
| jq -e '.[] | select(.name == "merge-queue")' <<<"$RS" >/dev/null 2>&1 && drift "repo '$r' 存在未声明的 merge-queue ruleset（expected-state.merge_queue.repos 未列——扩围须修订 ADR-0042）" |
There was a problem hiding this comment.
4. Renamed queues evade detection 🐞 Bug ≡ Correctness
The undeclared-repository check identifies merge queues solely by the arbitrary ruleset name merge-queue, rather than by a contained rule whose type is merge_queue. An undeclared repository can therefore enable a queue under any other ruleset name without producing drift.
Agent Prompt
## Issue description
Unauthorized merge queues evade detection when their repository ruleset has a name other than `merge-queue`.
## Issue Context
Ruleset names are labels; merge-queue behavior is represented by a rule with type `merge_queue`. The list response does not include full rules, so matching candidates must be fetched and inspected by rule type.
## Fix Focus Areas
- governance/drift-check.sh[583-588]
- governance/drift-check.sh[569-580]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+586
to
+587
| RS=$(api "https://api.github.com/repos/$ORG/$r/rulesets?per_page=100") | ||
| jq -e 'type == "array"' <<<"$RS" >/dev/null 2>&1 || continue |
There was a problem hiding this comment.
5. Undeclared scan fails open 🐞 Bug ☼ Reliability
When an undeclared repository’s rulesets request fails or returns an error object, line 587 silently skips that repository without recording drift. The overall check can therefore exit successfully while being unable to determine whether that repository has an unauthorized merge queue.
Agent Prompt
## Issue description
The undeclared merge-queue scan silently skips repositories whose rulesets cannot be read, allowing an unverifiable run to pass.
## Issue Context
The declared-repository branch already records the same non-array response as drift. Apply equivalent fail-closed behavior to the undeclared scan.
## Fix Focus Areas
- governance/drift-check.sh[583-588]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+145
to
+146
| SRC_ID=$(curl -sS -H "Authorization: Bearer ${GH_TOKEN}" -H "Content-Type: application/json" -d '{"query":"{repository(owner:\"Cloudbird-Software\", name:\"'"$r"'\" ){id}}"}' https://api.github.com/graphql | jq -r '.data.repository.id') | ||
| RID=$(curl -sS -H "Authorization: Bearer ${GH_TOKEN}" -H "Content-Type: application/json" -d '{"query":"query($r:String!){repository(owner:\"Cloudbird-Software\", name:$r){rulesets(first:50){nodes{name databaseId}}}}","variables":{"r":"'"$r"'"}}' https://api.github.com/graphql | jq -r '.data.repository.rulesets.nodes[]? | select(.name == "merge-queue") | .databaseId') |
There was a problem hiding this comment.
6. Configured organization is ignored 🐞 Bug ≡ Correctness
Step 6 hardcodes Cloudbird-Software in both GraphQL lookups instead of using the script’s configurable ORG. Running with an overridden organization makes earlier steps operate on the requested organization while merge-queue creation fails against, or targets repositories in, Cloudbird-Software.
Agent Prompt
## Issue description
The merge-queue GraphQL calls ignore the configured `ORG` and always query Cloudbird-Software.
## Issue Context
`apply.sh` explicitly supports overriding `ORG`, and all preceding REST operations use that value. Pass owner and repository name as GraphQL variables rather than interpolating literals.
## Fix Focus Areas
- governance/apply.sh[26-32]
- governance/apply.sh[143-146]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+151
to
+152
| MUT='mutation($src: ID!) { createRepositoryRuleset(input: { sourceId: $src, name: "merge-queue", target: BRANCH, enforcement: ACTIVE, conditions: { refName: { include: ["~DEFAULT_BRANCH"], exclude: [] } }, rules: [ { type: PULL_REQUEST, parameters: { pullRequest: { requiredApprovingReviewCount: 0, dismissStaleReviewsOnPush: true, requireCodeOwnerReview: false, requireLastPushApproval: false, requiredReviewThreadResolution: false, allowedMergeMethods: [SQUASH] } } }, { type: REQUIRED_STATUS_CHECKS, parameters: { requiredStatusChecks: { requiredStatusChecks: [{ context: "gate", integrationId: null }], strictRequiredStatusChecksPolicy: false } } }, { type: MERGE_QUEUE, parameters: { mergeQueue: { mergeMethod: SQUASH, checkResponseTimeoutMinutes: 60, maxEntriesToBuild: 5, minEntriesToMerge: 1, maxEntriesToMerge: 1, minEntriesToMergeWaitMinutes: 0, groupingStrategy: ALLGREEN } } } ] }) { ruleset { databaseId } } }' | ||
| RESP=$(jq -n --arg q "$MUT" --arg src "$SRC_ID" '{query:$q, variables:{src:$src}}') |
There was a problem hiding this comment.
7. Apply ignores declared parameters 🐞 Bug ⚙ Maintainability
The creation mutation hardcodes every merge-queue parameter rather than reading .merge_queue.params from expected-state.json. A future parameter change in the declared single source creates the old configuration, after which drift-check immediately rejects the result.
Agent Prompt
## Issue description
Merge-queue creation duplicates all expected parameters as hardcoded GraphQL values instead of consuming the expected-state declaration.
## Issue Context
The expected-state file describes itself as the source used by both drift-check and apply. Drift-check already reads these parameters dynamically, while apply does not.
## Fix Focus Areas
- governance/apply.sh[151-153]
- governance/expected-state.json[83-97]
- governance/drift-check.sh[575-580]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
摘要
实测发现 org rulesets API 不支持 merge_queue 规则类型（REST+GraphQL 均 422 "Invalid rules"；且 merge_queue 必须与 pull_request/required_status_checks 同集、mergeMethod 须为 repo 允许的 squash）——PR #117 的 org 级
rulesets/merge-queue.json无法生效且会让 drift-check §1 误报。本 PR 修正为 repo 级声明模式（与 P1-1 repo settings 对账同型）。变更
governance/rulesets/merge-queue.json（org 级声明不可行）expected-state.json增merge_queue段（repos + params，单一真源）drift-check.sh新增 §14：repo 级 merge-queue ruleset 存在性+参数对账；未声明仓出现 merge-queue ruleset = 漂移（扩围须修订 ADR-0042）apply.sh新增 step6：幂等创建（GraphQL 写入——REST 不支持该规则类型；存在即跳过，参数对账由 §14 执法）现状
两仓 ruleset 已实际创建（agent-registry id=21076429 / template-service id=21076438，GraphQL，SQUASH+串行保守参数）；merge_group 触发已合入两仓（agent-registry#60 / template-service#18）。
验证
C1：governance/ 路径，ADR-0042 背书（org→repo 级范围为实施性修正，ADR 修订随批）。