Skip to content

fix(butler): heartbeat-watch 阈值读取路径勘误(ADR-0074) - #233

Merged
randypanding merged 1 commit into
mainfrom
fix-watch-yaml-path
Aug 21, 2026
Merged

fix(butler): heartbeat-watch 阈值读取路径勘误(ADR-0074)#233
randypanding merged 1 commit into
mainfrom
fix-watch-yaml-path

Conversation

@randypanding

@randypanding randypanding commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

首版误读 butler.yaml 顶层键(实际嵌套 thresholds: 下)→ KeyError;且 Actions 默认 bash -e 令 $( ) 失败直灭脚本 exit 1 而非 infra 通道。修复:正确嵌套路径 + 显式捕获 exit 2。

Card: Cloudbird-Software/.github#168

Summary by CodeRabbit

  • Bug 修复
    • 优先从配置文件中的嵌套设置读取陈旧度阈值,手动覆盖值仍保持最高优先级。
    • 配置读取失败时将明确报告为基础设施错误,并以相应错误状态退出,便于及时发现监控配置问题。

Copilot AI lite review requested due to automatic review settings August 21, 2026 18:21
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b4efc25-119a-4f3f-b5e9-2f42b4b2ced3

📥 Commits

Reviewing files that changed from the base of the PR and between 1fd7ec0 and 64b5b65.

📒 Files selected for processing (1)
  • .github/workflows/butler-heartbeat-watch.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Butler 心跳监控

Layer / File(s) Summary
阈值读取与错误处理
.github/workflows/butler-heartbeat-watch.yml
默认阈值路径改为 thresholds.deadman_stale_hours。手动覆盖值仍优先使用。配置读取失败时输出错误并以 exit 2 退出。

Suggested labels: security, bug

Merge Risk: 🔵 Low · up to 64b5b

The workflow fix is localized and addresses the configuration-path and exit-code behavior, but merge readiness still requires confirmation that the required CODEOWNERS owner approval for this governance-path change is complete.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题使用了有效的 Conventional Commits 前缀 fix,长度为 47 个字符,并准确描述了心跳监控阈值读取路径修复。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-watch-yaml-path

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix heartbeat-watch threshold YAML path and fail-closed error handling

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Read stale threshold from the correct nested key: butler.yaml#thresholds.deadman_stale_hours.
• Prevent bash -e from aborting on $(...) failures by explicitly routing errors to infra (exit 2).
• Keep numeric validation and stale-check flow unchanged while improving failure diagnostics.
Diagram

graph TD
  A["butler-heartbeat-watch job"] --> B["Resolve STALE_H"] --> C["Parse butler.yaml"] --> D["Validate numeric"] --> E["Query last heartbeat"] --> F["Compare + report"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use yq to read YAML in shell
  • ➕ Avoids embedding Python one-liner logic in bash
  • ➕ Typically clearer error messages and simpler quoting
  • ➖ Adds dependency/installation step (or relies on runner preinstall)
  • ➖ Version differences across yq implementations can cause portability issues
2. Precompute threshold as workflow env/inputs
  • ➕ Eliminates runtime YAML parsing failure mode in Actions
  • ➕ Makes overrides explicit and discoverable in workflow config
  • ➖ Duplicates policy data outside the source-of-truth file
  • ➖ Requires extra update steps when policy changes

Recommendation: Current approach (Python yaml.safe_load) is appropriate since PyYAML is already implied by existing logic; the key improvement is explicitly catching command-substitution failures under bash -e and routing them to the intended infra failure path (exit 2). Consider yq only if YAML parsing expands further or readability becomes an issue.

Files changed (1) +8 / -1

Bug fix (1) +8 / -1
butler-heartbeat-watch.ymlFix nested threshold lookup and capture YAML read failures under bash -e +8/-1

Fix nested threshold lookup and capture YAML read failures under bash -e

• Corrects the stale-hours threshold read path to thresholds.deadman_stale_hours instead of a top-level key. Refactors command substitution into an explicit conditional so Python/YAML read errors are caught and surfaced as infra failures (exit 2) rather than silent bash -e termination (exit 1).

.github/workflows/butler-heartbeat-watch.yml

@coderabbitai coderabbitai Bot added bug Something isn't working security labels Aug 21, 2026
@randypanding
randypanding merged commit 94a8d2f into main Aug 21, 2026
14 checks passed
@randypanding
randypanding deleted the fix-watch-yaml-path branch August 21, 2026 18:24

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 the butler-heartbeat-watch workflow’s stale-threshold lookup so it reads the correct nested key from governance/policy/butler.yaml (thresholds.deadman_stale_hours) and routes YAML-read failures through the intended infra error path (exit 2), aligning with ADR-0074 fail-closed behavior.

Changes:

  • Corrects the threshold key path from a top-level lookup to thresholds.deadman_stale_hours.
  • Refactors threshold resolution to avoid unhandled $(...) failures and emit a dedicated infra error (exit 2) on read failures.

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

Comment on lines +49 to +53
# 顶层键 KeyError;且 Actions 默认 bash -e 会令 $( ) 失败直灭脚本 exit 1,
# 现显式捕获走 infra 通道 exit 2)
STALE_H="${STALE_HOURS_OVERRIDE:-}"
if [[ -z "$STALE_H" ]]; then
STALE_H=$(python3 -c 'import yaml; print(yaml.safe_load(open("governance/policy/butler.yaml", encoding="utf-8"))["thresholds"]["deadman_stale_hours"])' | tr -d '\r') || {
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Card: line formatted as code 📘 Rule violation § Compliance
Description
The PR description’s card metadata is formatted as inline code (wrapped in backticks), so it does
not start with the required literal Card:  prefix and may not be parsed by tooling. This violates
the required PR card metadata line format.
Code

.github/workflows/butler-heartbeat-watch.yml[R48-51]

+          # 阈值真源 = butler.yaml#thresholds.deadman_stale_hours(嵌套键——首版误读
+          # 顶层键 KeyError;且 Actions 默认 bash -e 会令 $( ) 失败直灭脚本 exit 1,
+          # 现显式捕获走 infra 通道 exit 2)
+          STALE_H="${STALE_HOURS_OVERRIDE:-}"
Relevance

●●● Strong

The card line is visibly backtick-wrapped, so it does not literally begin with the required
parseable Card:  prefix.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist rule requires a single parseable line that literally starts with Card:  in the PR
description. Since this PR modifies a workflow file under .github/, the PR description is expected
to carry compliant metadata; the provided description shows the card line wrapped in backticks,
which prevents it from matching the required prefix.

Rule 2825427: Require PR description to include a card metadata line
.github/workflows/butler-heartbeat-watch.yml[48-55]

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 must include exactly one plain-text line starting with `Card: ` followed by `<owner>/<repo>#<n>` (not wrapped in backticks / code formatting).

## Issue Context
This PR has a card reference, but it is formatted as inline code, which prevents the line from literally starting with `Card: `.

## Fix Focus Areas
- .github/workflows/butler-heartbeat-watch.yml[48-55]

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



Informational

2. Missing ADR-#### in description 📘 Rule violation § Compliance
Description
This PR modifies a file under .github/, but the PR description body does not contain an ADR-NNNN
reference as required. This can break governance traceability for standards/automation changes.
Code

.github/workflows/butler-heartbeat-watch.yml[R48-51]

+          # 阈值真源 = butler.yaml#thresholds.deadman_stale_hours(嵌套键——首版误读
+          # 顶层键 KeyError;且 Actions 默认 bash -e 会令 $( ) 失败直灭脚本 exit 1,
+          # 现显式捕获走 infra 通道 exit 2)
+          STALE_H="${STALE_HOURS_OVERRIDE:-}"
Relevance

● Weak

ADR-0074 is present in the PR title, matching the repository gate’s historical title-or-body check.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist rule requires an ADR-NNNN reference in the PR description body when paths like
.github/ are modified. This PR modifies .github/workflows/butler-heartbeat-watch.yml, so the
requirement is triggered, but the PR description text provided does not include any ADR-####
token.

Rule 2778538: Require ADR reference in PR description when governance or standards files change
.github/workflows/butler-heartbeat-watch.yml[48-55]

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 must include an `ADR-NNNN` token when files under `.github/` are modified.

## Issue Context
This PR changes `.github/workflows/butler-heartbeat-watch.yml`, so the ADR reference requirement applies.

## Fix Focus Areas
- .github/workflows/butler-heartbeat-watch.yml[48-55]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 16 rules
Review mode: ⚖️ Balanced: This changes runtime CI workflow behavior and error handling for a configuration-driven heartbeat path; although localized, it carries meaningful operational risk and is not trivial enough for lite.
ⓘ  1 issues published inline · 2 in summary

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

Comment on lines +48 to +51
# 阈值真源 = butler.yaml#thresholds.deadman_stale_hours(嵌套键——首版误读
# 顶层键 KeyError;且 Actions 默认 bash -e 会令 $( ) 失败直灭脚本 exit 1,
# 现显式捕获走 infra 通道 exit 2)
STALE_H="${STALE_HOURS_OVERRIDE:-}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. card: line formatted as code 📘 Rule violation § Compliance

The PR description’s card metadata is formatted as inline code (wrapped in backticks), so it does
not start with the required literal Card:  prefix and may not be parsed by tooling. This violates
the required PR card metadata line format.
Agent Prompt
## Issue description
PR description must include exactly one plain-text line starting with `Card: ` followed by `<owner>/<repo>#<n>` (not wrapped in backticks / code formatting).

## Issue Context
This PR has a card reference, but it is formatted as inline code, which prevents the line from literally starting with `Card: `.

## Fix Focus Areas
- .github/workflows/butler-heartbeat-watch.yml[48-55]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants