Skip to content

fix(butler): 熔断变量创建端点勘误(POST 集合端点+value 字符串)(W1-C5 #168,ADR-0057) - #181

Merged
randypanding merged 1 commit into
mainfrom
w1c5-fix-breaker-endpoint
Aug 21, 2026
Merged

fix(butler): 熔断变量创建端点勘误(POST 集合端点+value 字符串)(W1-C5 #168,ADR-0057)#181
randypanding merged 1 commit into
mainfrom
w1c5-fix-breaker-endpoint

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

动机

deadman trip 首次演习(run 32481546304)抓出潜伏 bug:创建 org 变量的 POST 打到了 /actions/variables/<name>(正确=集合端点 /actions/variables,带名 POST=404),且 -F value=true 发送布尔(API 要字符串=422)。两错叠加 → 变量从未真正创建成功。

变更

  • .github/workflows/butler-deadman-trip.yml:POST 集合端点 + -f value;复位指引同步
  • governance/cost-check.sh set_breaker():同款潜伏 bug 一并修复(该路径首次真实触发前从未执行过);复位指引同步
  • docs/deadman-setup.md:复位命令 -F-f

验证

  • 本地实测(owner PAT,与 CI 同 API):旧写法 404/422 失败;新写法 POST orgs/…/actions/variables -f name -f value -f visibility → 变量创建成功(当前已置位=true,由本次演习链路置上)
  • bash -n cost-check.sh ✓
  • 合并后将重新 dispatch butler-deadman-trip 完整闭环:变量已存在 → PATCH 生效 → run exit 1(tripped 语义)→ 人工复位 + P0 P0 dead-man trip:管家缺席,自动合并已停(AUTO_MERGE_DISABLED=true) #180 留痕关闭(AC-3 演习证据补全)

AC 映射(#168 AC-3)

回滚:revert 本 PR。

…304 抓出;同修 cost-check set_breaker 同款潜伏 bug)(W1-C5 #168,ADR-0057/ADR-0040)
Copilot AI lite review requested due to automatic review settings August 21, 2026 12:25
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

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.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 43b9b72f-9265-4fbf-b15f-5dea70294eaa

📥 Commits

Reviewing files that changed from the base of the PR and between 8d20ca6 and 9bb30cb.

📒 Files selected for processing (3)
  • .github/workflows/butler-deadman-trip.yml
  • docs/deadman-setup.md
  • governance/cost-check.sh

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix org breaker variable creation in deadman-trip workflow and cost-check

🐞 Bug fix 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Fix org variable creation by POSTing to the collection endpoint and sending string values.
• Apply the same fix to cost-check breaker logic to avoid silent non-creation.
• Update runbook/reset commands to use consistent gh api flags.
Diagram

graph TD
  W[".github/workflows/butler-deadman-trip.yml"] -->|"PATCH else POST"| API(("GitHub Actions Variables API")) --> VARS[("Org Actions variables")]
  C["governance/cost-check.sh:set_breaker()"] -->|"PATCH else POST"| API
  D["docs/deadman-setup.md"] -. "reset guidance" .-> W

  subgraph Legend
    direction LR
    _file["File/Job"] ~~~ _api(("API")) ~~~ _db[("Store")]
  end
Loading
High-Level Assessment

The PR’s approach is the right minimal fix: keep the existing PATCH-then-POST fallback but correct the POST target to the collection endpoint and send value as a string. Alternatives like swapping to raw curl calls or introducing a different persistence mechanism (issue/comment state) would add complexity without improving correctness for this incident.

Files changed (3) +15 / -8

Bug fix (2) +14 / -7
butler-deadman-trip.ymlCorrect org variable POST endpoint and send value as string +6/-3

Correct org variable POST endpoint and send value as string

• Fixes the fallback variable-creation call to POST to 'orgs/$ORG/actions/variables' (collection endpoint) instead of the per-variable endpoint. Updates 'value' submission to use '-f' (string) and aligns the embedded reset/runbook snippet accordingly.

.github/workflows/butler-deadman-trip.yml

cost-check.shFix set_breaker() variable creation endpoint and value type +8/-4

Fix set_breaker() variable creation endpoint and value type

• Applies the same endpoint and value-type correction to 'set_breaker()' so a missing variable is actually created (POST to collection) and the value is accepted as a string. Also updates the printed manual reset guidance to use '-f value=false'.

governance/cost-check.sh

Documentation (1) +1 / -1
deadman-setup.mdFix reset command to use string value flag +1/-1

Fix reset command to use string value flag

• Updates the documented reset command to use '-f value=false' (string) instead of '-F' to match the API’s expected type and the corrected workflow behavior.

docs/deadman-setup.md

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a latent bug in the “breaker” org variable mutation logic used by the butler dead-man trip and cost-check paths, ensuring org variable creation uses the correct REST collection endpoint and that value is sent as a string (per API expectations). It also updates reset/runbook instructions to match the corrected gh api flag usage.

Changes:

  • Fix org variable creation to POST orgs/{org}/actions/variables (collection endpoint), not .../variables/{name}.
  • Send value=true|false as a string via -f/--raw-field rather than typed -F/--field to avoid 422 type errors.
  • Update operational reset instructions in both workflow output text and docs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
governance/cost-check.sh Corrects breaker variable PATCH/POST logic to use the proper POST endpoint and string value.
docs/deadman-setup.md Updates the documented reset command to use -f value=false (string).
.github/workflows/butler-deadman-trip.yml Fixes org variable creation endpoint and ensures value is sent as a string; aligns reset guidance text.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread governance/cost-check.sh
Comment on lines 184 to +185
set_breaker() { # PATCH 已有 / POST 新建(404 时)
# 端点勘误(2026-08-21 deadman 演习实测):创建 org 变量必须 POST 到集合端点
Comment on lines 69 to +70
# 1) 置共用熔断变量(PATCH 已有 / 404 时 POST 新建——同 cost-check.sh set_breaker)
if ! gh api -X PATCH "orgs/$ORG/actions/variables/$CB" -f name="$CB" -F value=true >/dev/null 2>&1; then
if ! gh api -X POST "orgs/$ORG/actions/variables/$CB" -f name="$CB" -F value=true -f visibility=all >/dev/null 2>&1; then
# 端点勘误(2026-08-21 首次演习实测):POST 须打集合端点 /actions/variables
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (0)

Grey Divider


Informational

1. PR body missing ADR-#### 📘 Rule violation § Compliance
Description
This PR touches governance-related paths (.github/, governance/), but the provided PR
description/body does not contain an ADR-#### reference token as required. Missing the ADR
reference reduces auditability for governance changes.
Code

.github/workflows/butler-deadman-trip.yml[R70-72]

+          #    端点勘误(2026-08-21 首次演习实测):POST 须打集合端点 /actions/variables
+          #    (不带变量名,带名=404);value 为字符串类型(-F 布尔=422)。
+          #    首演 run 32481546304 因旧写法 INFRA(变量未置位),本修复后重演闭环。
Relevance

● Weak

PR title contains ADR-0057; historical ADR gate precedent accepts title references, so the
missing-body claim is not applicable.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2778538 requires an ADR-#### token in the PR description/body for PRs touching .github/ or
governance/. The diff shows edits in those paths, so the rule applies; the provided PR
description/body lacks an ADR token (title reference does not satisfy the rule as written).

Rule 2778538: Require ADR reference in PR description for governance-related changes
.github/workflows/butler-deadman-trip.yml[69-75]
governance/cost-check.sh[184-193]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
PR description/body must include an `ADR-####` token when governance-related paths are modified.

## Issue Context
This PR modifies files under `.github/` and `governance/`, triggering the ADR-reference requirement.

## Fix Focus Areas
- .github/workflows/butler-deadman-trip.yml[69-75]
- governance/cost-check.sh[184-193]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. cost-check.sh requires org token 📘 Rule violation ⛨ Security
Description
governance/cost-check.sh and the related workflow perform authenticated gh api operations using
a caller-provided GH_TOKEN (sourced from secrets.GOVERNANCE_TOKEN / described as an org-admin
token) instead of obtaining an approved single-repo-scoped token via scripts/ghcb or
scripts/gh-app-token.sh. This increases the risk of over-scoped, over-privileged credentials being
used in CI/automation runs.
Code

governance/cost-check.sh[R190-192]

+       -f name="$CB_VARIABLE" -f value=true >/dev/null 2>&1; then
+    mutate "$GH" api -X POST "orgs/$ORG/actions/variables" \
+      -f name="$CB_VARIABLE" -f value=true -f visibility=all >/dev/null 2>&1 \
Relevance

● Weak

A same-day, closely matching token-scope finding on this butler workflow was explicitly rejected.

PR-#173

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2778539 requires automation GitHub authentication to be obtained exclusively via scripts/ghcb
or scripts/gh-app-token.sh with explicit single-repo scope. The cited script
governance/cost-check.sh explicitly requires GH_TOKEN=<org admin token> and uses it for
authenticated gh api operations (including in the changed breaker/mutation code path), while the
workflow sets GH_TOKEN from secrets.GOVERNANCE_TOKEN and then executes gh api calls,
demonstrating that the current implementation relies on a generic secret-provided token rather than
invoking an approved token-generation script.

Rule 2778539: Agent operations must obtain GitHub tokens via approved scripts with single-repo scope
governance/cost-check.sh[13-50]
governance/cost-check.sh[184-194]
.github/workflows/butler-deadman-trip.yml[43-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Automation scripts and workflows interacting with GitHub must obtain tokens via `scripts/ghcb` (preferred) or `scripts/gh-app-token.sh` (legacy) with explicit single-repo scope, rather than relying on a generic `GH_TOKEN` (including one described as an org-admin token or sourced directly from `secrets.GOVERNANCE_TOKEN`).

## Issue Context
Rule 2778539 requires agent/automation GitHub authentication to be obtained exclusively through the approved token scripts with single-repo scope. `governance/cost-check.sh` documents and enforces that `GH_TOKEN` must be set and uses it for `gh api` calls (including the modified breaker mutation path). The workflow also sets `GH_TOKEN` from `secrets.GOVERNANCE_TOKEN` and then runs authenticated `gh api` calls, indicating the automation path is using a potentially over-privileged secret token rather than an approved scoped token.

## Fix Focus Areas
- governance/cost-check.sh[13-50]
- governance/cost-check.sh[184-194]
- .github/workflows/butler-deadman-trip.yml[43-77]
- scripts/ghcb[1-41]
- scripts/gh-app-token.sh[37-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 15 rules
Review mode: ⚖️ Balanced: This changes runtime workflow and shell behavior on a fail-closed breaker path with operational blast radius; although localized, it is not clearly trivial enough for lite.

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@randypanding
randypanding merged commit 10ab858 into main Aug 21, 2026
11 checks passed
@randypanding
randypanding deleted the w1c5-fix-breaker-endpoint branch August 21, 2026 12:39
randypanding pushed a commit that referenced this pull request Aug 21, 2026
…冲突解为 §16→§17(本卡)→§18 段序(§18 头注释已预留 §17 编号给 W1-C3);REPOS.yaml 保留 archive/holdout 新条目+entry_protocol 申报
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants