governance: auto-fix 上限执法 + 额度/成本熔断（P2-8，ADR-0040） - #116
Merged
Conversation
|
Warning Review limit reached
Next review available in: 14 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 (7)
Comment |
PR Summary by QodoEnforce auto-fix limits and organization cost circuit breakers
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1. False breaker looks unavailable
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+10
to
+13
| workflow_dispatch: | ||
| # 注入入口（T2 注入式测试：79%/85%/100% 全场景不依赖真实超支）：空=真实 API/真源 | ||
| inputs: | ||
| usage_minutes_override: |
There was a problem hiding this comment.
4. Dispatch grants org-wide mutations 🐞 Bug ⛨ Security
Any repository writer who can manually dispatch these workflows can supply hard-stop or all-PR inputs that execute with the organization-admin GOVERNANCE_TOKEN. For example, usage_minutes_override can trip the organization breaker, while all_prs=true and max_attempts=0 can close every listed open PR without requiring dry-run.
Agent Prompt
## Issue description
Manual dispatch inputs permit repository writers to invoke destructive organization-wide actions with an admin secret.
## Issue Context
GitHub permits users with repository write access to trigger `workflow_dispatch`; dry-run defaults to disabled and the supplied inputs directly control enforcement.
## Fix Focus Areas
- .github/workflows/cost-check.yml[10-32]
- .github/workflows/cost-check.yml[50-58]
- .github/workflows/auto-fix-limit.yml[10-32]
- .github/workflows/auto-fix-limit.yml[51-59]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+244
to
+248
| elif [[ $BREAKER_SET -eq 0 ]]; then | ||
| # 复位确认：变量未置位 + 用量 < 硬停档 + P0 issue 开着 → 自动关闭（人工复位已发生且留痕在评论） | ||
| for row in $(gov_open_issues cost-circuit-breaker | cut -f1); do | ||
| mutate "$GH" issue close "$row" --repo "$GOV_REPO" --comment \ | ||
| "复位确认：$CB_VARIABLE 已未置位且用量 ${PCT_MIN}% < ${AM_STOP}%——全流程恢复（agent 派发/automerge 前置检查放行）。自动关闭。" \ |
There was a problem hiding this comment.
5. New breaker p0 closes immediately 🐞 Bug ≡ Correctness
BREAKER_SET is read before set_breaker and never updated, so the first hard-stop run enters the BREAKER_SET == 0 reset branch after creating the P0 and immediately closes it. The same branch can also close breaker issues when the variable read failed and its state is unknown.
Agent Prompt
## Issue description
The first breaker activation immediately triggers the reset cleanup using stale pre-activation state.
## Issue Context
`BREAKER_SET` is initialized from the variable before hard-stop processing. `set_breaker` does not update it, and the reset branch does not require both metrics to be below hard-stop or the state read to have succeeded.
## Fix Focus Areas
- governance/cost-check.sh[123-132]
- governance/cost-check.sh[176-203]
- governance/cost-check.sh[234-251]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+170
to
+172
| mutate "$GH" api -X POST "orgs/$ORG/actions/variables/$CB_VARIABLE" \ | ||
| -f name="$CB_VARIABLE" -F value=true -f visibility=all >/dev/null 2>&1 \ | ||
| || infra "org 变量 $CB_VARIABLE 置位失败" |
There was a problem hiding this comment.
6. Breaker creation uses wrong endpoint 🐞 Bug ≡ Correctness
When the variable does not yet exist, set_breaker POSTs to the item endpoint ending in /$CB_VARIABLE instead of the organization variable collection. The fallback therefore fails and the hard-stop can leave AUTO_MERGE_DISABLED unset.
Agent Prompt
## Issue description
The breaker fallback cannot create a missing organization variable because it POSTs to the wrong REST path.
## Issue Context
PATCH uses the item path, but creation must POST to `/orgs/{org}/actions/variables` with name, value, and visibility in the request body.
## Fix Focus Areas
- governance/cost-check.sh[167-173]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+125
to
+131
| VERR=$("$GH" api "orgs/$ORG/actions/variables/$CB_VARIABLE" --jq .value 2>&1) || true | ||
| if [[ "$VERR" == *"true"* && "$VERR" != *"Not Found"* ]]; then | ||
| BREAKER_SET=1 | ||
| elif grep -q "Not Found" <<<"$VERR"; then | ||
| : | ||
| else | ||
| infra "org 变量 $CB_VARIABLE 读取失败（非 404）——熔断状态未知" |
There was a problem hiding this comment.
7. False breaker looks unavailable 🐞 Bug ≡ Correctness
Both scripts accept only a value containing true or a 404 response, so a valid manually reset value of false is recorded as an infrastructure failure. Cost checks then exit 2 instead of confirming reset, and auto-fix scans also remain permanently red after the documented PATCH-to-false reset.
Agent Prompt
## Issue description
A valid organization variable value of `false` is treated as an API failure.
## Issue Context
The documented reset procedure explicitly PATCHes the variable to false. Parse the command status separately and accept exact true and false values; handle 404 from a reliable status check rather than matching error text.
## Fix Focus Areas
- governance/cost-check.sh[123-132]
- governance/auto-fix-limit.sh[92-103]
- governance/cost-check.sh[191-194]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+172
to
+176
| mutate "$GH" pr edit "$num" --repo "$ORG/$repo" --add-label "$AF_EXHAUSTED_LABEL" >/dev/null 2>&1 || true | ||
| mutate "$GH" pr close "$num" --repo "$ORG/$repo" --comment \ | ||
| "auto-fix 上限触发（ADR-0040）：$CHECK_NAME 失败 $FAILS 次 ≥ 上限 $MAX_ATTEMPTS。自动关闭并不再重试；失败历史见 .github 仓说明 issue。人工续作：修复后新开 PR（勿 reopen 本 PR）。" \ | ||
| >/dev/null 2>&1 || true # 竞态（本轮已被关/合）不算 infra；issue 通道已有去重兜底 | ||
| EXHAUSTED_CLOSED=$((EXHAUSTED_CLOSED+1)) |
There was a problem hiding this comment.
9. Failed closures count as enforced 🐞 Bug ☼ Reliability
Auto-merge removal, label application, and PR closure failures are swallowed, after which EXHAUSTED_CLOSED is incremented unconditionally. The workflow can therefore report that an over-limit PR was closed and return the over-limit status while the PR remains open and mergeable.
Agent Prompt
## Issue description
Failed PR enforcement mutations are reported as successful closures.
## Issue Context
Only benign, explicitly identified races should be tolerated. Other failures must increment infrastructure errors, and the closure counter should update only after closure is confirmed.
## Fix Focus Areas
- governance/auto-fix-limit.sh[167-177]
- governance/auto-fix-limit.sh[203-218]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+148
to
+149
| if ! ROW=$("$GH" api "repos/$ORG/$repo/commits/$sha/check-runs" \ | ||
| --jq "([.check_runs[] | select(.name == \"$CHECK_NAME\")] | sort_by(.id) | last | [.conclusion // \"none\", (.html_url // \"-\")]) // [\"absent\", \"-\"] | @tsv" 2>/dev/null); then |
There was a problem hiding this comment.
10. Missing checks escape counting 🐞 Bug ≡ Correctness
When a commit has no matching check run, last is null but the expression wraps it as `[null, "-"], so the outer // ["absent", "-"]` fallback never runs. Historical commits with no gate check are consequently not recognized as absent and evade the script's intended fail-closed attempt count.
Agent Prompt
## Issue description
An empty matching check-run list does not produce the expected `absent` conclusion.
## Issue Context
Apply the null fallback to `last` before reading `.conclusion`, or branch explicitly on an empty result, and test commits with zero matching runs.
## Fix Focus Areas
- governance/auto-fix-limit.sh[145-160]
- governance/policy/automation-limits.yaml[21-23]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+158
to
+163
| if mutate "$GH" api -X DELETE "repos/$ORG/$r/pulls/$n/auto-merge" >/dev/null 2>&1; then | ||
| act "熔断执法: 撤销 $r#$n 的 auto-merge" | ||
| fi | ||
| done < <("$GH" pr list --repo "$ORG/$r" --state open --limit 200 \ | ||
| --json number,autoMergeRequest \ | ||
| --jq '.[] | [.number, (if .autoMergeRequest != null then "1" else "0" end)] | @tsv' 2>/dev/null) |
There was a problem hiding this comment.
11. Breaker removal failures are silent 🐞 Bug ☼ Reliability
strip_all_automerge neither checks failure of each repository's PR-list process substitution nor records failed DELETE requests. A hard-stop can therefore leave existing auto-merges enabled while the P0 body falsely states that all were removed.
Agent Prompt
## Issue description
The hard-stop silently ignores failures while enumerating PRs or deleting auto-merge requests.
## Issue Context
Fetch and validate each PR list outside process substitution, record API failures through `infra`, and verify or fail on each DELETE before claiming complete enforcement.
## Fix Focus Areas
- governance/cost-check.sh[143-165]
- governance/cost-check.sh[176-189]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+140
to
+142
| if [[ "${#SHAS[@]}" -ge 100 ]]; then | ||
| # >100 commit 的 agent PR 本身即失控形态——直接按超限处置（fail-closed 方向） | ||
| FAILS=$MAX_ATTEMPTS; FAIL_ROWS="(commit 数 ≥100，逐条省略——按失控处置)" |
There was a problem hiding this comment.
12. Exactly 100 commits force closure 🐞 Bug ≡ Correctness
The commit request returns at most 100 entries, but the script treats >= 100 as proof that the PR is over limit. A PR with exactly 100 commits and no failed gate attempts is therefore forcibly closed, even though the comment says only PRs with more than 100 commits should receive that treatment.
Agent Prompt
## Issue description
A PR containing exactly 100 commits is classified as an overflow and closed regardless of check results.
## Issue Context
Request a sentinel 101st commit or paginate before deciding that the PR exceeds 100 commits; process exactly 100 normally if that is the intended boundary.
## Fix Focus Areas
- governance/auto-fix-limit.sh[133-143]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+161
to
+163
| done < <("$GH" pr list --repo "$ORG/$r" --state open --limit 200 \ | ||
| --json number,autoMergeRequest \ | ||
| --jq '.[] | [.number, (if .autoMergeRequest != null then "1" else "0" end)] | @tsv' 2>/dev/null) |
There was a problem hiding this comment.
14. Enforcement stops after 200 prs 🐞 Bug ☼ Reliability
Both breaker scanners explicitly limit each repository to 200 open PRs, despite promising to revoke auto-merge from all open PRs. Any matching auto-merge request beyond that limit remains active during a hard stop.
Agent Prompt
## Issue description
Organization-wide breaker enforcement scans only the first 200 open PRs per repository.
## Issue Context
Paginate until exhaustion in both scripts and treat any page failure as infrastructure failure before claiming complete revocation.
## Fix Focus Areas
- governance/cost-check.sh[143-165]
- governance/auto-fix-limit.sh[105-118]
- governance/auto-fix-limit.sh[197-200]
ⓘ 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.
摘要（ADR-0040 实现载体，工作卡 #93 / 计划 #81 P2-8）
无人值守自动化的两道生存护栏：
governance/auto-fix-limit.sh（小时级 cron）：统计 agent PR 各 commit 上gatecheck run 的失败结论数（真源 = Checks API，无内存态——崩溃/重启计数续接，T3 由构造保证）；达上限 → 撤 auto-merge + 关 PR +auto-fix-limit-exhausted标签 + 说明 issue（含失败 run 链接）；reopen 的超限 PR 下轮再关。governance/cost-check.sh（6h cron）：GET /orgs/{org}/settings/billing/usage（旧端点 410 已迁移）汇总当月 Actions 分钟 vspolicy/automation-limits.yaml声明预算：≥80% 告警 issue（同日去重）；≥100% 置 org 变量AUTO_MERGE_DISABLED+ 撤全部 open PR auto-merge + P0 issue。复位仅人工（owner PATCH 变量 + P0 留评论），脚本观察到复位后自动关 P0。LLM_TOKENS_USED），数据源（llm-gateway usage）pending。刻意不改
governance/expected-state.json：熔断变量是运行态标志非期望状态（纳入会造成复位后被 drift-check 反向改回的对账冲突——ADR-0040 后果节）。变更文件
governance/policy/automation-limits.yaml（新）——阈值真源：max_attempts=3 / 20000 分钟月预算 / 80·100 档位 / 熔断变量与复位人governance/auto-fix-limit.sh（新）——上限执法扫描器governance/cost-check.sh（新）——用量检查 + 熔断置位/复位确认.github/workflows/auto-fix-limit.yml（新）——cron 18 * * * * + dispatch 注入（max_attempts/check_name/repos/all_prs/dry_run）.github/workflows/cost-check.yml（新）——cron 42 */6 * * * + dispatch 注入（usage/quota/llm/dry_run——T2 注入式测试入口）AGENTS.md——硬规则 (a)-(d)（熔断前置检查/重试上限/禁 reopen/infra issue 补盲）+ 索引 + 常用命令.github/workflows/gate.yml——bash -n 纳入两个新脚本本地已验证（dry-run，零写入）
待执行（合并后，按卡内测试方法）
@randypanding