Skip to content

fix: re-pin 激活暴露的两个真 bug——policy job checkout 空输出检出 caller 仓 + flaky-retry 注释数字混入(.github #90/#94) - #43

Merged
randypanding merged 2 commits into
mainfrom
fix/v1.5.6-two-livebugs
Aug 21, 2026
Merged

fix: re-pin 激活暴露的两个真 bug——policy job checkout 空输出检出 caller 仓 + flaky-retry 注释数字混入(.github #90/#94)#43
randypanding merged 2 commits into
mainfrom
fix/v1.5.6-two-livebugs

Conversation

@randypanding

@randypanding randypanding commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

摘要

v1.5.5 re-pin 系列(Use-up-Plan PR#52 等 7 仓)在 harden-runner SHA 手误修复后跑到运行时,暴露两个此前从未被消费所以从未被测试的 bug:

1. dep-review.yml policy job 检出 caller 仓(#90

policy job 的 checkout 步骤仍引用 ${{ steps.self.outputs.repo }}/.ref——该输出在 v1.5.3 钉源改版(workflow_ref→ciw-ref)时已随旧解析步骤移除 → 空值 → actions/checkout 落缺省(caller 仓 + PR merge ref)→ can't open file .../ci-workflows/scripts/dep-supply-chain-check.py → fail-closed exit 2(日志实锤:git remote add origin https://github.com/Cloudbird-Software/Use-up-Plan)。

修复:checkout 直取 repository: Cloudbird-Software/CI-Workflows + ref: ${{ inputs.ciw-ref }}(与 job outputs 同源)。

2. flaky-retry.sh policy 提取把注释数字混入(#94

retry_max: 2 # 自动重试上限(总运行 ≤3) 行——grep -oE '[0-9]+' 取出 2 3 两个数 → RETRY_MAX='2 3'TOTAL=... 算术崩溃 → TOTAL: unbound variable(Use-up-Plan check job 日志实锤)。self-test 走 FLAKY_POLICY_INLINE 分支,从未覆盖 curl+sed 分支。

修复:提取前 sed 's/#.*//' 剥注释。

验证

  • 本地复现两 bug 的根因路径(testing.yaml 提取管线实测输出 '2
    3')
  • 合并发 v1.5.6 + 7 仓 re-pin 更新后:check/deps 全绿
  • flaky-retry curl 分支实测:make check 经重试包装跑通并入账

C1:.github/ 与 scripts/ 路径,ADR-0039/0043 域。

Summary by CodeRabbit

  • Bug Fixes
    • 提升依赖审查流程的稳定性,确保始终使用指定版本的策略配置。
    • 修复策略配置中带有行尾注释时重试次数解析不正确的问题。
    • 保持现有重试、失败处理及 flaky 事件记录行为不变。

…已随钉源改版移除,空值导致检出 caller 仓)
…3' 算术崩溃 TOTAL unbound——self-test 走 inline 分支从未覆盖 curl 分支)
@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: 0c0c24dc-af4c-4bb3-9b71-aec1b39d59ad

📥 Commits

Reviewing files that changed from the base of the PR and between ea21f86 and 84d29da.

📒 Files selected for processing (2)
  • .github/workflows/dep-review.yml
  • scripts/flaky-retry.sh

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


📝 Walkthrough

Walkthrough

Changes

本次变更固定 policy job 的策略仓库检出来源,并在解析 retry_max 前移除行尾注释。

CI 行为更新

Layer / File(s) Summary
固定策略仓库检出
.github/workflows/dep-review.yml
policy job 从固定的 Cloudbird-Software/CI-Workflows 仓库检出,并使用 inputs.ciw-ref 作为 ref。
解析带注释的重试配置
scripts/flaky-retry.sh
解析 retry_max 时先移除配置行中的注释,再提取数值。

Suggested labels: security, bug

Merge Risk: ⚪ Minimal · up to 84d29

This localized change corrects repository checkout selection and retry-policy parsing; no actionable merge-blocking risk remains beyond normal checks.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning 标题使用了有效的 Conventional Commits 前缀,但长度为 98 个字符,超过 50 个字符限制。 将标题压缩到 50 个字符以内,并保留 fix 前缀及两个主要修复点。
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/v1.5.6-two-livebugs

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

@coderabbitai coderabbitai Bot added bug Something isn't working security labels Aug 21, 2026
@randypanding
randypanding merged commit ca53520 into main Aug 21, 2026
13 of 14 checks passed
@randypanding
randypanding deleted the fix/v1.5.6-two-livebugs branch August 21, 2026 06:29
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix dep-review policy checkout pin and flaky-retry retry_max parsing

🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Fix dep-review policy job to checkout CI-Workflows by explicit ciw-ref pin.
• Prevent flaky-retry.sh from parsing comment digits into retry_max (arithmetic crash).
Diagram

graph TD
  A["Caller repo"] --> B["dep-review.yml"] --> C["policy job"] --> D["Checkout CI-Workflows@ciw-ref"]
  E["flaky-retry.sh"] --> F["GitHub Contents API"] --> G["testing.yaml policy"] --> H["retry_max parsed (no comments)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parse YAML with a real parser (yq/python) instead of grep/sed
  • ➕ Much more robust against formatting changes (comments, indentation, multi-line values).
  • ➕ Clearer failure modes and easier validation of schema.
  • ➖ Adds a dependency/tooling requirement (yq) or a heavier runtime (python) to a minimal shell script path.
  • ➖ May be overkill for a single scalar if policy format is tightly controlled.
2. Enforce comment-free scalar lines in policy via validation
  • ➕ Keeps shell extraction trivial and avoids adding tooling.
  • ➕ Shifts complexity to policy linting where it’s more visible.
  • ➖ Still brittle if other formatting variations appear (quoting, anchors, etc.).
  • ➖ Requires additional validation step and policy author discipline.

Recommendation: The chosen fixes are appropriate for a hot bugfix: explicitly check out Cloudbird-Software/CI-Workflows at inputs.ciw-ref to avoid defaulting to the caller repo, and strip #... comments before numeric extraction to prevent RETRY_MAX becoming multi-value. If the policy YAML is expected to evolve, consider migrating the curl-branch parsing to a real YAML parser (yq/python) to eliminate grep/sed fragility.

Files changed (2) +3 / -3

Bug fix (2) +3 / -3
dep-review.ymlFix policy job checkout to use explicit CI-Workflows repo + ciw-ref +2/-2

Fix policy job checkout to use explicit CI-Workflows repo + ciw-ref

• Updates the 'policy' job checkout step to stop using removed 'steps.self.outputs.*' values (which could become empty and default checkout to the caller repo). The workflow now checks out 'Cloudbird-Software/CI-Workflows' at 'inputs.ciw-ref', aligning with the reusable-workflow pinning model.

.github/workflows/dep-review.yml

flaky-retry.shStrip YAML comments before extracting retry_max +1/-1

Strip YAML comments before extracting retry_max

• Hardens retry policy extraction by removing inline '#' comments before running digit extraction. This prevents comment numbers from being included in 'RETRY_MAX' (e.g., '2 # ≤3' becoming '2 3') and avoids arithmetic failures downstream.

scripts/flaky-retry.sh

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Informational

1. Non-portable grep whitespace 🐞 Bug ⚙ Maintainability
Description
scripts/flaky-retry.sh matches the retry_max line using grep -E '^\s+retry_max:', but \s is not
supported by many grep implementations (non-GNU/BSD/busybox), so policy parsing can fail and the
script will exit 2 (fail-closed) in those environments.
Code

scripts/flaky-retry.sh[43]

+      | grep -E '^\s+retry_max:' | head -1 | sed 's/#.*//' | grep -oE '[0-9]+')
Relevance

●●● Strong

Clear portability bug; replacing nonportable \s with POSIX [[:space:]] is a trivial deterministic
fix.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR-modified parsing pipeline still uses \s in an ERE, which is not reliably supported;
multiple references show grep -E may not recognize \s and recommend [[:space:]] for
portability. The script itself advertises local self-test usage, making this a practical portability
footgun.

scripts/flaky-retry.sh[1-7]
scripts/flaky-retry.sh[41-46]
🌐 Demonstrates that grep/egrep may not recognize \s (no matches) and recommends using POSIX [[:space:]] instead for whitespace matching.

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

### Issue description
`grep -E '^\s+retry_max:'` is not portable across grep implementations; in some environments `\s` is treated as a literal `s`, causing the `retry_max` line not to match and the script to fail-closed.

### Issue Context
This script documents local execution (`--self-test`), so portability beyond GNU grep matters.

### Fix Focus Areas
- scripts/flaky-retry.sh[40-45]

### Suggested change
Replace `'^\s+retry_max:'` with a POSIX character class, e.g. `grep -E '^[[:space:]]+retry_max:'` (or `grep -E '^[[:blank:]]+retry_max:'` if you only want spaces/tabs).

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


Grey Divider

Context sources
✅ Web pages:
  +7 more
Review mode: ⚖️ Balanced: This is a small but behavior-changing fix in CI workflow checkout and policy parsing; it affects fail-closed automation and warrants a careful single-pass review, though it is not dense enough for extended review.

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 added a commit that referenced this pull request Aug 26, 2026
对近一周(#21..#124)全部 PR 复盘后的机械债清理:仅删除 AST 级验证
「全仓零引用」的未用导入/未用名,不改任何判定逻辑、阈值、白名单或
policy 数据。逐文件出处:

- pipeline/adversary/cnb_bridge.py:删未用 `from typing import Any`(#73/#74)
- pipeline/adversary/golden_set.py:删未用 `from typing import Any`(#80/#82/#83)
- pipeline/adversary/holdout_registry.py:删未用 `from typing import Any`(#81/#82)
- pipeline/adversary/e2e/e2e-runner.py:删未用 `from typing import Any`(#89)
- pipeline/adversary/llm_verifier.py:删未用 `import math`;可选库导入行去掉
  未用名 extract_score(call_verifier/create_openai_client 均在用,保留)(#72/#76)
- pipeline/entropy/tests/test_e2e.py:删未用 `import sys`(#56)
- pipeline/selftest-c/tests/test_registry.py:删未用 `import copy`(#103)
- pipeline/trust-gate/tests/test_adjudicate.py:删未用 `import copy`(#63)
- pipeline/trust-gate/tests/test_cli.py:from-import 去掉未用名
  PREDICATES/UNLOCK_STATE(保留 trust_gate 可导入性冒烟导入与 noqa 惯例)(#63)
- scripts/dep-supply-chain-check.py:删未用 `import copy`(#36/#43)

刻意不动(已核验非死代码):各模块 `from __future__ import annotations`;
fuzz/sast/symbolic 的 `_yamlmini` 双模式导入守卫(noqa F401,保证包路径);
golden_set 等 try-import yaml 的环境 fail-closed 守卫;org-gate /
suppression-gate / adversary-gate 等关卡 workflow 与 policy/suppressions.yaml
基线数据——门语义一概不变。

验证:
- py_compile 全部 scripts/pipeline *.py 通过;bash -n 全部 *.sh 通过
- workflows/policy/pipeline 共 62 个 YAML 解析通过
- scripts/test-integrity-fixtures/run.sh、scripts/suppression-budget-selftest.sh 通过
- python -m unittest:trust-gate test_adjudicate+test_cli 17 例、
  selftest-c tests.test_registry 14 例、entropy tests.test_e2e 10 例——全绿

Co-authored-by: randypanding <randypanding@users.noreply.github.com>
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.

1 participant