feat(drill): 注入/独立验证/台账引擎(W4-C4 .github#223,ADR-0069) - #245
Conversation
|
Warning Review limit reached
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. 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 (6)
Comment |
PR Summary by Qodofeat(drill): add inject + independent gate verification + drill ledger aggregation
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
There was a problem hiding this comment.
Pull request overview
This PR implements the core “seed drill” engine for governance gate health checks (ADR-0069 / Card #223): injecting known defects into an isolated branch, independently verifying whether a target gate fails, and producing an append-only history source suitable for dashboard aggregation.
Changes:
- Extend
governance/drill/drill.pywithinject,record(append-only ledger), andredrateaggregation. - Add an independent verifier
governance/drill/verify_gate.pythat polls GitHub check-runs and outputs RED/GREEN/NO-SURFACE/MISSING-GATE/TIMEOUT with strict fail-closed behavior. - Add offline self-tests for selection determinism and verification semantics, and wire drill tests into the gate workflow’s script checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| governance/tests/test-drill.sh | Aggregates and fail-closes if governance/drill/tests/test-*.sh coverage disappears. |
| governance/drill/verify_gate.py | Independent verifier that polls check-runs (or offline JSON) and emits a drill verdict/exit code. |
| governance/drill/tests/test-verify.sh | Offline assertions for verifier verdict semantics and fail-closed behavior. |
| governance/drill/tests/test-select.sh | Offline assertions for deterministic selection and target pool constraints. |
| governance/drill/drill.py | Adds injection, ledger append, and red-rate aggregation capabilities to the drill engine. |
| .github/workflows/gate.yml | Registers the new drill scripts for bash syntax checking in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| data = b"\0" * s["size_bytes"] | ||
| with open(path, "wb") as f: | ||
| f.write(data) | ||
| _git({}, "add", path, cwd=tmp) |
| weeks = {} | ||
| for r in seeds: | ||
| wk = r["ts"][:10] | ||
| d = r.get("difficulty", "?") | ||
| weeks.setdefault(wk, {}).setdefault(d, 0) | ||
| weeks[wk][d] += 1 | ||
| fails = [r for r in rows if r.get("kind") == "failclose-drill"] | ||
| out = {"total_records": len(rows), "seed_drills": len(seeds), "verdicts": verdicts, | ||
| "red_rate": rate, "red_rate_note": None if denom else "N/A(尚无可判定演习)", | ||
| "difficulty_trend_by_day": {k: dict(sorted(v.items())) for k, v in sorted(weeks.items())}, | ||
| "failclose_drills": len(fails), |
| for l in lines: | ||
| prev = json.loads(l) # 既有行畸形=台账已损坏,拒绝追加(fail-closed) | ||
| if prev["ts"] >= rec["ts"]: | ||
| die(f"append-only 破坏: 新 ts {rec['ts']} 不晚于末行 {prev['ts']}") |
| f"{API}/repos/{org}/{repo}/commits/{sha}/check-runs?per_page=100", | ||
| headers={"Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json", | ||
| "User-Agent": "seed-drill-verify"}) | ||
| with urllib.request.urlopen(req) as r: |
| hits = match_gate(checks, gate) | ||
| if hits and hits[0].get("status") == "completed": | ||
| c = hits[0] | ||
| v = "RED" if c.get("conclusion") == "failure" else "GREEN" |
Code Review by Qodo
1. Non-success marked GREEN
|
| if hits and hits[0].get("status") == "completed": | ||
| c = hits[0] | ||
| v = "RED" if c.get("conclusion") == "failure" else "GREEN" | ||
| return {"verdict": v, "check_name": c.get("name"), |
There was a problem hiding this comment.
3. Non-success marked green 🐞 Bug ≡ Correctness
verify_gate.py 在目标 check-run status=completed 时,将除 failure 之外的所有 conclusion 都判为 GREEN,导致 cancelled/timed_out/neutral/skipped 等非 success 的情况被误判为“关卡通过”。这会把中间态/异常态当成“关卡死了”触发错误的 GREEN(退出码1) 与错误台账口径。
Agent Prompt
### Issue description
`verify_gate.py` currently maps any completed check run whose `conclusion != "failure"` to `GREEN`. GitHub check-run conclusions include multiple non-success terminal states (cancelled/timed_out/neutral/skipped/action_required/startup_failure/stale, etc.). Treating those as `GREEN` produces false “gate is green” alarms.
### Issue Context
The verifier is intended to assert “this defect should make gate X fail”. For completed runs:
- `failure` => `RED`
- `success` => `GREEN`
- other completed conclusions should be treated as an intermediate/indeterminate verdict (e.g., `TIMEOUT`/`MISSING-GATE`/new `INDETERMINATE`) with exit code 3, because they are not evidence that the gate is passing.
### Fix Focus Areas
- governance/drill/verify_gate.py[56-62]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| f"{API}/repos/{org}/{repo}/commits/{sha}/check-runs?per_page=100", | ||
| headers={"Authorization": f"Bearer {token}", "Accept": "application/vnd.github+json", | ||
| "User-Agent": "seed-drill-verify"}) | ||
| with urllib.request.urlopen(req) as r: |
There was a problem hiding this comment.
4. No api timeout handling 🐞 Bug ☼ Reliability
verify_gate.py 调用 urllib.request.urlopen 拉取 check-runs 时未设置 timeout 且未捕获网络异常,可能在网络抖动/代理问题/连接卡死时无限阻塞或直接抛异常终止,导致验证步骤与上游 workflow 不可预测地挂起或无结构化输出。
Agent Prompt
### Issue description
`fetch_check_runs()` uses `urllib.request.urlopen(req)` without a timeout and without exception handling. In CI, a stuck TCP connection or slow proxy can block indefinitely (ignoring the intended `--timeout` polling window), or crash the process with an unhandled exception.
### Issue Context
The verifier already has a polling loop and a user-facing `--timeout` budget. Network calls should respect that budget by:
- passing a per-request timeout
- catching `URLError`/`HTTPError` and continuing the poll (or returning an indeterminate verdict) rather than crashing
- optionally backing off / limiting retries
### Fix Focus Areas
- governance/drill/verify_gate.py[27-34]
- governance/drill/verify_gate.py[45-80]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| path = os.path.join(tmp, *s["payload_path"].format(DATE=date).split("/")) | ||
| os.makedirs(os.path.dirname(path), exist_ok=True) |
There was a problem hiding this comment.
5. Inject path traversal risk 🐞 Bug ⛨ Security
drill.py 的 inject 用 samples.registry.yaml 的 payload_path 直接 split/join 到临时工作区路径,未禁止绝对路径或 '..' 片段,样本库一旦被误写/被恶意提交可导致写入临时目录外任意路径。即使最终 git add/commit 失败,也会在 runner 上发生越界写文件。
Agent Prompt
### Issue description
`cmd_inject()` constructs the destination path from `payload_path` without sanitization. A `payload_path` containing absolute paths or `..` can escape the clone directory and write arbitrary files on the machine running the command.
### Issue Context
`validate_samples()` currently checks only that `payload_path` is non-empty and contains `{DATE}`. It does not enforce that the path is relative and normalized.
### Fix Focus Areas
- governance/drill/drill.py[87-90]
- governance/drill/drill.py[169-181]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| rec = json.loads(a.json) | ||
| except Exception as e: | ||
| die(f"--json 解析失败: {e}") | ||
| if not rec.get("ts") or not rec.get("kind"): |
There was a problem hiding this comment.
6. Record json type unchecked 🐞 Bug ☼ Reliability
drill.py 的 record 对 --json 只做 json.loads 并假设其为 dict 且 ts 可比较字符串;传入 null/false/数组或非字符串 ts 会触发 AttributeError/TypeError/KeyError,导致非预期栈追踪而非一致的 fail-closed 错误信息。
Agent Prompt
### Issue description
`cmd_record()` assumes the parsed JSON is a dict with string fields. If `--json` is a top-level scalar (`null`, `false`) or a list, `.get` will crash; if `ts` types differ across lines, comparisons can raise `TypeError`. This breaks the intended consistent fail-closed behavior.
### Issue Context
There is an established repo pattern to harden JSON validation by explicitly asserting top-level objects (rejecting `None`/`False`/scalars) before continuing.
### Fix Focus Areas
- governance/drill/drill.py[210-227]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- drill.py 补全 inject(git 隔离分支注入,push 3 次重试)/record(append-only 台账:时间戳单调+run 去重)/redrate(红率聚合+难度按周分布+零分母 null 口径) - verify_gate.py 独立验证步骤(与样本库分离——ADR-0069 决策 2):RED=演习成功/ GREEN=演习失败/NO-SURFACE/MISSING-GATE/TIMEOUT,静默期防假阴性 - tests: test-select(确定性/holdout 隔离/钉选)+ test-verify(红绿判定核)+ governance/tests/test-drill.sh CI 聚合入口;gate.yml bash -n 登记 - 首演实录(PR body 详):注入 drill/seed-20260822 → push 面 NO-SURFACE(局限 记录)→ draft PR#239 → org-hygiene 变红=演习成功 → 即关即删 Card: #223
1b689da to
1320285
Compare
动机
样本选出来之后要能注入、能独立判定红绿、能留痕进 dashboard 数据源(ADR-0069 决策 2/3)。本 PR 补全演习引擎:git 隔离分支注入 + 独立验证脚本 + append-only 台账与红率聚合。
变更清单
governance/drill/drill.py补全:inject(浅克隆→演习分支→写缺陷→push 3 次重试,只开分支不开 PR)、record(append-only:时间戳严格递增 + 同 run 去重)、redrate(红率 + 难度按周分布 + 零分母 null 诚实口径 +--fail-unhealthy告警出口)governance/drill/verify_gate.py:独立验证步骤(与样本库分离)——不读样本库、不注入,只断言"该缺陷应触发关卡 X 失败":RED=演习成功(0) / GREEN=演习失败(1) / NO-SURFACE / MISSING-GATE / TIMEOUT(3),静默期 120s 防"workflow 排队中"假阴性test-select.sh(同 seed 确定性、holdout 永不入池、github-scope 只打治理总仓、钉选通道)+test-verify.sh(红绿判定核 8 断言)+governance/tests/test-drill.shCI 聚合入口;gate.yml bash -n 登记首次演习实录(AC-1 证据,2026-08-21 UTC)
select --seed 20260822 --sample-id hygiene-gitleaks-aws-key --target-repo .githubinject→ 分支drill/seed-20260822@78ee220(缺陷文件drill/leak-aws-20260821.ini){"verdict": "RED", "check_name": "org-hygiene / hygiene", "conclusion": "failure"}——gitleaks 抓到伪造 AWS 键,关卡活着,红=演习成功AC 映射
history.jsonl首条 seed-drill 记录:verdict=red, surface=draft-pr, limitation=NO-SURFACE 实测);绿→P0 的自动化开单在 PR4 workflow(GREEN 判定路径已被 test-verify 断言)redrate:红率 = red/(red+green)(首演后 =1.0),难度按周分布可见,--fail-unhealthy红率<100% 非零退出测试方法
bash governance/tests/test-drill.sh→ 3 套 24 断言全绿(本 PR 时点);台账操作见 history.jsonl 首演两行实录风险与回滚
注入只落演习分支(不进 agent 工作区);台账 append-only 由 record 命令强制。回滚:摘除脚本即停,台账留档无害。
Card: #223