fix(ci): self-heal failed checkouts on the reused review runners - #9220
Conversation
A checkout failure on the self-hosted review pool was terminal: either a transient network drop mid-fetch (curl 92 / early EOF), or a corrupt persisted workspace whose refs claim objects missing from its object store, after which every fetch dies in negotiation with 'remote did not send all necessary objects'. ecs-qwen-runner-64c-23 stayed in that state for two days (2026-08-13..15), failing seven review jobs on the same missing SHAs. Make the first checkout continue-on-error; on failure wipe the whole workspace (not just .git) and retry the identical checkout once. The workspace is disposable — later steps reinstall deps and tools.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Gate re-check on Template ✓ — all required sections, bilingual. Problem: observed, not theoretical. Independently confirmed on an earlier pass from the CI run annotations: 2026-08-13..15, seven Direction: in scope — this heals the repo's own review automation on its reused self-hosted pool; no product surface is touched. Size: not core infrastructure. Production change is +72/−0 in the workflow YAML; tests are +453/−1. Test lines dominate, and that is proportionate rather than bloat: the wipe step's payload is an Approach: minimal — first checkout made survivable, outcome-gated wipe, byte-identical retry once. Scope is still exactly right: only the Risk: changed files match no high-risk path pattern; no elevated risk signals. Moving on to code review. 🔍 中文说明在 模板 ✓ —— 各必需小节齐全,中英双语。 问题: 已观测到的,不是理论问题。早前的 pass 已从 CI run annotation 中独立确认:2026-08-13 至 15 日,七个 review-pr job 在同一台 runner(ecs-qwen-runner-64c-23)的 checkout 阶段挂掉,引用的都是完全相同的缺失 SHA —— 损坏的持久化 clone 令该机器连续两天对所有调度过来的 review 必挂。同窗口内另有两次失败是 fetch 中途的网络瞬断。两种失败形态都写进了自愈步骤自己的注释里。 方向: 在范围内 —— 修复的是仓库自己的 review 自动化在复用 self-hosted 池上的可靠性,不触及任何产品面。 规模: 非核心基础设施。生产改动为 workflow YAML +72/−0;测试 +453/−1。测试行数占大头,但这是相称的而非臃肿:wipe 步骤的载荷是 CI 里的 rm -rf,守卫正需要这样的重度 pin。 方案: 最小化 —— 第一次 checkout 可存活、按 outcome 触发 wipe、一次性字节级相同的重试。范围仍然恰好:只自愈 review-pr job 的 checkout;文件中第二个 "Checkout base branch" 属于跑在临时 ubuntu-latest 上的 resolve-pr,所以只有被修的 checkout 位于持久化 workspace 池上。本轮新增:merge 在同一文件里调和了 main 的 Verify runner directory health 步骤(#8894 同类)—— 两道防线互补而非重叠(health 在 job 启动时探测 runner 目录;自愈在 checkout 时作用于 workspace),合并后文件中的步骤顺序核验无误。 风险: 改动文件未命中任何高风险路径模式;无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewSecond full pass, on Both Criticals are fixed the way the reviewer prescribed — not sanded down.
The bot's earlier Critical on the same family (sudo argv pin vs canonicalized The guard chain end to end — canonicalize → strip trailing slashes → denylist → The merge itself is clean: main brought the TestingUnattended CI run — no PR code executed here; the evidence below is the PR's own CI on the reviewed commit, quoted via the API. At posting time the ubuntu unit lane — which runs Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the remaining behavioral claim: 中文说明代码审查第二次完整 pass,在 两个 Critical 都按评审者给出的方式修复 —— 没有打磨变钝。
机器人早先在同一问题族上的 Critical(软链 tmpdir 下 sudo argv pin 与规范化后的 整条守卫链 —— 规范化 → 去尾斜杠 → 拒绝名单 → merge 本身干净:main 把 测试无人值守 CI 运行 —— 此处未执行任何 PR 代码;以下证据是 PR 自身 CI 在被审 commit 上的结果,经 API 引用。发布时 ubuntu 单元 lane —— 运行 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — both of doudouOUC's Criticals are fixed exactly the way they were prescribed, without loosening a single assertion; the guard chain re-verifies end to end at the merged head; and this pass surfaced no new concern. What keeps it from a 5 is the standing open records — the R5-1 wipe-idiom duplication, and a root cause contained rather than fixed — plus the ubuntu lane still running at posting time. Stepping back: the shape of this PR has not changed since the first pass, and it remains the right shape — a permanent failure mode (one corrupted persisted workspace poisons a runner for days) turned into a self-healing one at bounded cost (one extra full clone after a first failure), with nothing changing when the first checkout succeeds. What this pass added is the reassurance that the review process itself worked: a human reviewer caught two ways the test suite would have been red on macOS, the fixes landed as probe-and-gate and spelling-consistency rather than deleted assertions, and the suite comes out of it strictly more honest — it now documents which of its claims depend on GNU userland instead of silently leaning on a ci.yml TMPDIR export. In six months this still reads like someone who has been burned by a CI runner before and does not want to be again. The bot's changes-request from the 中文说明置信度:4/5 —— doudouOUC 的两个 Critical 都严格按照其给出的方式修复,未放宽任何断言;守卫链在合并后的 head 上端到端复核通过;本轮未发现新问题。没到 5 分是因为尚存的记录项 —— R5-1 的 wipe 习语重复、以及被遏制而非修复的根因 —— 加上发布时 ubuntu lane 仍在运行。 退一步看:这个 PR 的形态从第一次 pass 起没有变,而且仍然是正确的形态 —— 把一种永久性失败模式(一次持久化 workspace 损坏就能让某台 runner 中毒数天)变成有界代价的自愈(第一次失败后多一次完整 clone),且第一次 checkout 成功时什么都不变。本轮新增的是对评审流程本身的确认:人类评审抓出了测试套件在 macOS 上会变红的两处,修复以"探测并跳过"和"拼写一致"的方式落下,而不是删除断言,套件因此变得更诚实 —— 它现在明确声明哪些断言依赖 GNU userland,而不是悄悄依赖 ci.yml 的 TMPDIR 导出。六个月后再看,这代码依然像一个被 CI runner 坑过、不想再被坑一次的人写的。 机器人 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit rounds 3–5 — the loop stopped after round 2: the Step 4 verifier's mutation probe executed rm -rf /* on the runner host and destroyed the review environment, and resuming agent rounds on the damaged host was declined.
Not reviewed: diff-coverage proof — the harness transcripts that record the Step 3 fan-out were destroyed in the same incident, so coverage can no longer be certified mechanically even though all 13 agents returned substantive receipts before it.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
中文说明
未审查:reverse audit rounds 3–5 — the loop stopped after round 2: the Step 4 verifier's mutation probe executed rm -rf /* on the runner host and destroyed the review environment, and resuming agent rounds on the damaged host was declined。
未审查:diff-coverage proof — the harness transcripts that record the Step 3 fan-out were destroyed in the same incident, so coverage can no longer be certified mechanically even though all 13 agents returned substantive receipts before it。
未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。
未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6t-17/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk-j6c03lyei7s809zq1s6t-17--work-qwen-code-qwen-code/subagents/b330460a-a575-4bd2-a3a4-f5fb6f4f5fb9'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9220All seven inline findings from the automated reviewer (six Feedback points and decisions
Verification
Commit: 中文说明Autofix 审查轮次 — PR #9220自动审查者的全部 7 条行内发现(第 1–2 轮共 7 条 反馈点与处理决定
验证
提交: Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — PR #9220No action taken this round: the workflow-prepared feedback contains no actionable items, so no code was changed and no commit was created. The branch stays at Feedback triage
Result
中文说明Autofix 评审轮次 —— PR #9220本轮未采取任何操作:工作流准备的反馈中没有可处理的事项,因此未修改任何代码,也未创建任何提交。分支保持在 反馈分诊
结果
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 176 passed · 0 failed · 176 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:176 通过 · 0 失败 · 176 总计 Verification reportPR #9220 Deep Verification — fix(ci): self-heal failed checkouts on the reused review runnersVerdict: 中文摘要结论:
Central claim and A/B proofCentral claim: on the reused self-hosted review pool, a failed base-branch checkout is no longer terminal — the job wipes the whole workspace once and retries the byte-identical checkout; on success nothing changes. Secondary claims: (1) the wipe script is safe ( The checkout step itself cannot run in this sandbox, so the A/B is a step-chain replay: both YAMLs are parsed,
14/14 scripted assertions. Recovery from a corrupt workspace: head 1/1, base 0/1; happy path preserved on both arms; double failure stays red. Byte-identical retry, proven two ways. The retry step is textually the base's original checkout body (the diff inserts the new head above it), so pin/ref/fetch-depth cannot drift by construction; the suite's equality assertion plus mutant m4 (retry Wipe script — real execution matrixThe exact
Corrections (to the PR description / test comments, not code-change requests)
Findings1. Suggestion (non-blocking): wipe lacks the pool idiom's suspicious-path guardThe wipe step's comment cites the "Pool wipe idiom (serve-ab.yml, qwen-triage.yml)", but adopts only the unguarded half of it. The qwen-triage.yml copies of this idiom (both the pre- and post-external-code wipes) carry a 2. Note: one new test passes for the wrong reason under the coarse revert — the fine mutant still kills itUnder mutant m1 (whole-file revert to base YAML) only 5 of the 6 heal tests go red: Not covered
MethodologyEnvironment: the CI verify container (node:22-bookworm lane runtime), uid 1000, node v22.23.2, no sudo/shellcheck/zstd preinstalled; actionlint 1.7.12 + shellcheck 0.11.0 installed via the repo's pinned Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.12)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Addresses the 16:40 review round on the checkout self-heal: - The wipe-failure test leaned on the real sudo, so it covered a different branch per lane; replace it with a PATH-stubbed sudo that forces both legs to fail hermetically, and pin the survivors left in place plus their oncall-visible warning. - The '|| sudo -n find' escalation leg survived deletion mutants: add a stub-sudo test proving the leg actually runs when user-mode find fails (leg-deletion and '||'->'&&' mutants both verified red). - Reuse the triage idiom's suspicious-path guard before wiping. - Count post-wipe survivors and warn with the count — triage exits 1 here, but the heal chain must stay alive for the retry. - Disclose in the step comment that the sudo leg only helps pool members with passwordless sudo.
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #9220 (review round 4)All seven round-4 Suggestions are addressed in one commit Feedback points and dispositions
No conflict work: VerificationCommands actually run, in order:
中文说明轮次总结 — PR #9220(review 第 4 轮)第 4 轮的全部 7 条 Suggestion 已在一个提交( 反馈点与处置
无冲突处理: 验证实际执行的命令(按顺序):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 3c": none — no check was cut short (~13 of ~35 tool calls used)..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 3c":none — no check was cut short (~13 of ~35 tool calls used).。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 138 passed · 0 failed · 138 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:138 通过 · 0 失败 · 138 总计 Verification reportPR #9220 Deep Verification (round 6) — fix(ci): self-heal failed checkouts on the reused review runnersVerdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证容器中执行(与 base A/B 重放对照、真实 git 损坏恢复、真实执行 harness、BSD userland 增量 A/B、突变矩阵、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。脚本断言:138 通过 · 0 失败 · 138 总计。
Previous-finding status (round 5 → round 6 head
|
| # | Round-5 finding | Severity | Status at round 6 |
|---|---|---|---|
| 1 | Lexical ..-escape reaches rm on the no-realpath fallback axis |
Note | stands, re-measured — wipe matrix W7.2: with realpath stubbed, $RWS/sub/../../w7-outside-canary string-matches the allowlist and the rm recorder logged -rf …/w7-rws/sub/../../w7-outside-canary/canary (raw .. argv, nothing actually deleted). The other 21 ladder shapes still refused without realpath (W7.1 21/21); the legitimate control still wipes on the same axis (W7.3). Blast radius unchanged: runner-set GITHUB_WORKSPACE + an image without coreutils realpath both required; this lane ships /usr/bin/realpath (measured). |
| 2 | Surviving mutants (M2/M10/M11 after round 5's delta killed M12) | Note | stand, re-measured — matrix re-run at the new head: M2/M10/M11 survive with unchanged classifications (Findings 2). No mutant regressed killed→survived; 15/19 killed. One NEW survivor discovered this round (probe mutant M17, Findings 3). |
| 3 | refuses to wipe when GITHUB_WORKSPACE is unset or empty names "unset" but the fixture exercises only empty |
Note | stands, re-measured — source scan: fixture passes only GITHUB_WORKSPACE: ''; live run with the variable truly unset refuses (exit 1, bash: line 8: GITHUB_WORKSPACE: parameter null or not set). Name/fixture drift, not a hole. |
| 4 | Intermittent actionlint+shellcheck stall (environmental) | Note | not reproduced, second round running — wrapper --actionlint 0.07 s, --shellcheck 12.44 s, direct per-arm actionlint 0.012 s (head) / 0.016 s (base); round 4 measured ~100 s stalls on identical input. Both gates proven live by planted-violation probes (L5/L7). |
Carried corrections: #2 (wipe preserves the directory, better than the plan's "recreates" — re-verified W1/F3) and #3 (runner default shell includes -o pipefail — re-verified F4) remain accurate. Correction #1 was partially addressed — see Corrections.
Round-6 delta: BSD-userland suite fix, proven load-bearing by A/B
The delta commit 90fa6bb4 (per its commit message; per-commit diffs are unreachable at depth 2) is test-file-only plus a YAML comment: it adds a hasGnuRealpath host probe (spawnSync('realpath', ['-m', '--', '/'])), gates the canonicalization test on it.skipIf(!hasGnuRealpath) instead of running it everywhere, and canonicalizes the lock fixture's RUNNER_WORKSPACE half (realpathSync(tmpdir())) so both halves of the allowlist comparison are spelled the same way on symlinked-tmpdir hosts. The YAML's heal script itself is unchanged in behavior; the only YAML delta is a comment recording that -m is GNU-only.
The A/B below reconstructs the pre-delta state by reverting exactly those two hunks in a scratch worktree, then drives both builds through the same environments:
| Cell | Build | Environment | Result (heal describe, 13 tests) |
|---|---|---|---|
| B1 | head | this GNU lane | 13 pass, 0 skip (probe reads true; coverage kept) |
| B2 | head | BSD realpath stub on PATH | 12 pass, 1 skip, 0 fail — skipped test is exactly refuses an allowlist-escaping .. path via canonicalization |
| B3 | head | BSD stub + symlinked TMPDIR | 12 pass, 1 skip, 0 fail |
| B4 | head | symlinked TMPDIR alone (GNU) | 13 pass (lock-fixture canonicalization holds on the macOS /var → /private/var shape) |
| B5 | pre-delta reconstruction | BSD stub | 1 failed / 12 passed — the failing test is the canonicalization test (the commit's reported shape) |
| B6 | pre-delta reconstruction | BSD stub + symlinked TMPDIR | 3 failed / 10 passed — canonicalization test + both lock-fixture tests, which throw before any assertion because the resolved workspace sits outside the raw allowlist root (the commit's second reported shape) |
| B7 | head (restored) | this GNU lane | 13 pass (harness integrity) |
Witness: 04-bsd-userland-delta-ab.png. Both failure shapes the commit message reports were reproduced byte-for-byte in cell counts, and the shipped head is green in all four environments. The delta's mutation claim ("deleting the canonicalization line still turns the test red on a GNU host") also holds at this head — mutant M1 is killed by exactly that test (mutation matrix).
The merge commit 952792a5 resolved a conflict in the test file against the newer main tip; the merged file's non-heal tests are exercised by the full-suite gates below (144 base tests green at head, in both directions).
Central claim and A/B proof
Central claim: on the reused self-hosted review pool, a failed base-branch checkout is no longer terminal — the job wipes the whole workspace once and retries the byte-identical checkout; on a successful first checkout nothing changes; a double failure stays red.
The checkout action cannot run in this sandbox, so the A/B is a step-chain replay: both YAMLs parsed with the repo's yaml package, every review-pr step up to (not including) Resolve PR context executed for real under the runner shell contract (bash --noprofile --norc -e -o pipefail, cwd = scratch workspace, pool geometry GITHUB_WORKSPACE one level inside RUNNER_WORKSPACE), actions/checkout stubbed by the bug's failure model (poison marker inside the workspace ⇒ the incident's exact remote did not send all necessary objects / Could not read … / Failed to traverse parents … lines until a wipe removes it; network marker ⇒ a network that stays down). This reproduces the shape of the failure, not its git-internal cause — see Not covered. Witness: 01-ab-replay-base-vs-head-corrupt-workspace.png.
A/B replay cells
| Cell | Environment | Oracle | Result |
|---|---|---|---|
| BASE/corrupt | base YAML, poisoned workspace | job status, attempts, poison survival | red after 1 attempt, poison survives, review never reached (expected red — the bug) |
| HEAD/corrupt | head YAML, poisoned workspace | attempts, poison gone, dir preserved, params identical, next step reached | green, 2 attempts, wipe fired, junk (node_modules) wiped too, retry params byte-identical (ref/fetch-depth), reached Resolve PR context |
| HEAD/clean | head YAML, healthy git repo + canary | heal skipped, canary survives | green, 1 attempt, wipe AND retry both SKIP, canary intact |
| BASE/clean | base YAML, A/A parity | identical end state | green, 1 attempt, canary intact |
| HEAD/double-fail | head YAML, network stays down | job red, exactly 2 attempts, wipe between them | red, 2 attempts, wipe fired at trace index between checkout and retry, review never reached |
28/28 assertions including stub fidelity (incident lines emitted while poisoned, success once healed) and control purity (base YAML has no heal steps). Recovery from a corrupt workspace: head 1/1, base 0/1.
Real-git corroboration (cause, not just shape)
02-real-git-corruption-heal.png: a file://-cloned workspace with its object store deleted but refs kept fails git fsck on missing blobs and git log dies (fatal: bad object HEAD) — the incident's corruption shape; the real extracted wipe script empties the workspace and preserves the directory; a fresh clone into it passes fsck and traverses all 3 commits. 13/13 assertions. Same honest caveat as round 5: an honest loopback server re-sends whatever the corrupt client lacks, so the terminal negotiation failure itself is modeled by the replay stub.
Wipe matrix — real execution of the extracted run: block
33/33 assertions. Witness: 03-wipe-matrix-guard-ladder-allowlist.png. Guard probes use a PATH-fronted rm recorder; a regressed guard is proven by its call log with nothing deleted.
| Cell | Scenario | Observed |
|---|---|---|
| W1 | legitimate wipe in pool geometry (real rm): nested dirs, .git, dotfiles, dir with space, symlink escaping to an outside canary |
emptied incl. dotfiles, directory preserved, exit 0, success warning, no survivors warning, outside canary untouched |
| W2/W3 | GITHUB_WORKSPACE unset / empty |
refused, error names the variable, rm never invoked (container ships GITHUB_WORKSPACE; the unset cell deletes it from the child env explicitly) |
| W4 | whole workspace locked (0o500) + failing sudo stub (both legs fail) | exit 0 (chain alive), wipe-failed warning, 2 entries survived naming both survivors incl. the one with a space |
| W5 | locked workspace + argv-recording sudo stub exiting 0 | sudo leg ran with -n find, argv targets exactly the workspace, then-branch warning |
| W6 | hostile ladder WITH realpath (22 shapes: roots, trailing slashes, //, dot-components, /tmp, /opt, symlink→/etc, ..-spellings) |
all refused, zero rm invocations |
| W7 | same ladder with realpath stubbed + ..-escape + legitimate control |
21/21 lexical shapes refused; the ..-escape reaches rm (Finding 1, raw argv recorded); legitimate workspace still wipes |
| W8 | allowlist boundary: WS==RWS exactly, prefix-sibling $RWS-evil, legitimate WS with trailing slash |
first two refused with zero rm; legitimate wiped |
| W9 | RUNNER_WORKSPACE edges: /, empty, unset, trailing-slash without realpath, nonexistent WS inside RWS |
refused/refused/refused/wiped/exit 0 as designed |
Mutation matrix — test pinning at the round-6 head
Control (unmutated heal describe): 13/13 green. Witness: 05-mutation-matrix-probe-mutants.png. 21/21 assertions (control + 19 mutants with attribution).
| Mutant | Change | Round 5 | Round 6 |
|---|---|---|---|
| M1 | drop WS realpath -m canonicalization |
killed | killed by refuses an allowlist-escaping .. path via canonicalization (the delta's mutation claim, re-verified) |
| M2 | drop denylist case |
survives | survives (Findings 2) |
| M3 | drop allowlist case |
killed | killed by both guard tests |
| M4 | drop RWS strip loop | killed | killed by the trailing-slash-RWS test |
| M5 | RUNNER_WORKSPACE :? → bare |
killed | killed by refuses to wipe when RUNNER_WORKSPACE is unset or empty |
| M6 | drop empty-RWS resolved to / refusal |
killed | killed by refuses to wipe when RUNNER_WORKSPACE resolves to / |
| M7 | delete || sudo -n find leg |
killed | killed by escalates to the sudo leg when the user-mode wipe fails |
| M8 | wipe condition || → && |
killed | killed by the same test |
| M9 | delete survivors count+warning | killed | killed by exits 0 and keeps survivors when BOTH wipe legs fail |
| M10 | survivors wc -l → wc -c |
survives | survives (Findings 2) |
| M11 | GITHUB_WORKSPACE :? → bare |
survives | survives (Findings 2) |
| M12 | drop WS strip loop | killed | killed by refuses a trailing-slash GITHUB_WORKSPACE when realpath is absent |
| M13 | drop continue-on-error |
killed | killed by makes the first checkout failure survivable and addressable |
| M14 | drop id: 'checkout' |
killed | killed by the same test |
| M15 | gates 'failure' → 'success' |
killed | killed by wipes and retries exactly when the first checkout fails |
| M16 | retry fetch-depth: 0 → 1 |
killed | killed by retries with the identical checkout |
| M17 | new: probe hasGnuRealpath forced false, run on GNU lane |
— | survives (12 pass + 1 skip, suite green) — Findings 3 |
| M18 | new: probe forced true, run under BSD realpath |
— | killed by the canonicalization test — the probe's BSD direction IS behaviorally pinned |
| M0 | coarse whole-file revert to base YAML | 11 red / 2 green | 11 red / 2 green — the two greens are the two toThrow() tests, green for the wrong reason (wipe step absent ⇒ wipe.run throws before any assertion) |
No mutant regressed from killed to survived. Regression direction: the base test file (144 tests) against the head YAML: 144/144 green; base parity at the base tip: 144/144 green (suites harness). Head full suite: 157 pass / 0 fail / 0 skip (06-head-suite-157-pass.png).
Corrections
- Description count drift (not a code-change request), new shape: the body now says "Thirteen new tests pin the chain … 131 tests pass". The thirteen claim is correct (13
it()blocks undercheckout self-heal, counted from source; round-5's "Five new tests" correction is addressed). The total is stale: the suite at head is 157/157 — the merge of main into the branch brought 26 pre-existing tests into this file (base parity measures 144 there vs 118 at round 5's base tip), so 144 + 13 = 157, not 131. - (Carried) The test plan says the wipe "recreates the directory"; the implementation preserves it (
find -mindepth 1 -maxdepth 1) — the better behavior, re-verified in W1/F3. - (Carried) The runner's documented Linux default shell includes
-o pipefail; the tests reproduce it (bash -e -o pipefail -c wipe.run, F4).
Findings
1. Note (non-blocking): the no-realpath fallback still admits a lexical ..-escape — stands
Re-measured (W7.2): with realpath stubbed, GITHUB_WORKSPACE=$RWS/sub/../../<outside> string-matches "$RWS"/* and reaches rm — the recorder logged -rf <raw .. path>/canary while nothing was deleted; with realpath present (W6) the same shape is refused by canonicalization. Reproduce: node tmp/pr9220-verify-20260816-133108/harness/wipe-matrix.mjs (cell W7). Blast radius unchanged and bounded: GITHUB_WORKSPACE is set by the runner from its registered work directory, never from PR input, and the residual additionally requires an image without coreutils realpath — the pool's node:22-bookworm container ships it (measured here: /usr/bin/realpath, -m exits 0). Recorded for awareness only.
2. Note: three surviving mutants, classified (completeness reporting, not merge conditions) — stands for M2/M10/M11
- M2 (denylist dropped) — redundant layer since the allowlist landed: a canonicalized path inside
"$RWS"/*can match a denylist arm only if the runner workspace itself sits under a guarded root; no realistic pool layout does. Belt-and-braces; no test can pin a unique outcome for it. - M10 (
wc -l→wc -c) — coverage gap: the survivors count value is asserted by no test (only the warning's presence and the names); control flow does not depend on the number. - M11 (WS
:?dropped) — redundant layer:set -uaborts the unset case and the""denylist arm catches empty; only the error text differs.
3. Note (new this round): the GNU-host truth of the hasGnuRealpath probe is unpinned — completeness only
Mutant M17 (probe forced false) runs green on this GNU lane — the canonicalization test simply skips, and nothing in the suite notices. By the design of skipIf, a probe that wrongly reads false on a GNU host would silently drop the canonicalization coverage everywhere, and a later M1-class regression could ship green with it. The other direction IS pinned: M18 (probe forced true) fails under a BSD userland exactly on the gated test. The probe runs the real realpath -m -- / binary, so breaking it requires editing the probe line itself — visible in any diff review — which is why this stays a completeness note rather than a merge condition.
4. Note: test name promises more than the fixture — stands
refuses to wipe when GITHUB_WORKSPACE is unset or empty exercises only GITHUB_WORKSPACE: ''. Behaviorally both cases refuse identically (re-proven: truly-unset run exits 1 naming the variable), so this is a name/fixture drift, not a hole.
5. Note: intermittent actionlint+shellcheck stall (environmental) — not reproduced, second round running
Round 4 measured ~100 s stalls; rounds 5 and 6 both measure sub-13 s on identical input with both liveness probes green. Non-determinism on identical input keeps the attribution environmental; all deterministic gates are green on both arms.
6. Note (observation): shellcheck style notes on the wipe block; no repo gate scans YAML run blocks
With the repo's shellcheck flags (--enable=all --severity=style), the extracted wipe block draws only style notes — SC2250 (brace the variables), SC2292 (prefer [[ ]]), SC2249 (default case arm; the denylist deliberately falls through to the allowlist layer) — and exits 0 at severity=warning (L6/L6b). Pre-existing configuration, not a PR defect: the repo's shellcheck gate scans tracked shell files only and its actionlint runs with -shellcheck=, so bash inside run: blocks is gated nowhere in CI — the vitest suite that executes the script verbatim is its only coverage, and it is green (L8).
Not covered
- Replay calibration. No token, no retrievable real run annotations — the replay remains uncalibrated: the stub reproduces the shape of the corrupt-workspace failure (its exact error lines), not the git negotiation that produces it. The real-git harness proves the corruption and the wipe+re-clone recovery with real git; the durable proof ("next real checkout failure heals") is only observable on the pool, as the PR itself states.
- Real sudo fallback execution. Container is uid 1000 with no
sudobinary; the leg is verified by argv-fidelity shim (W5), real permission-denied paths (W4), and the BSD/symlink probes, not by an actual rootfind. - Per-commit attribution. Depth-2 shallow checkout:
git rev-list HEAD^1..HEAD^2sees 1 of the snapshot's 10 commits (and--is-shallow-repositoryis true), so the aggregateHEAD^1..HEADdiff (2 files, +525/−1) is what was verified; per-commit behavior attribution was out of reach. - macOS lane behavior — simulated via the BSD realpath stub and symlinked TMPDIR (cells B2–B6), not run on Darwin.
- yamllint — re-measured as environmental:
pip3exists on PATH without an execute bit (wrapper fails withxargs: yamllint: Permission denied;--setupfails only at the yamllint install, L0/L3/L3b). actionlint covers the YAML structure and passed with liveness probes on both arms. - Repo-wide lint/test gates (targeted suite + changed files only, per scope).
- Harness self-corrections this round (documented, not findings about the PR): the first replay run had a trace-index arithmetic bug of mine (wipe correctly sat between checkout and retry at indices 3/4/5); the container ships
GITHUB_WORKSPACE, which leaked into the first "unset" cell until the env merge learned to delete keys; the W7.3/W8 legitimate cells first ran with the rm recorder still on PATH (no-op wipe); lint L0/L6 encodings adjusted (setup exits 1 on the yamllint image block; an extractedrun:block needs a shebang for shellcheck). All fixed and re-run from the same harnesses; the PR's behavior was never in doubt by these.
Methodology
Environment: CI verify container (node:22-bookworm lane runtime), uid 1000, node v22.23.2, /usr/bin/realpath present and GNU (-m exits 0), no sudo binary, /tmp a real directory. Harnesses in harness/ (replay-ab.mjs, real-corruption.mjs, wipe-matrix.mjs, bsd-delta.mjs, mutation-matrix.mjs, suites.mjs, lint-gates.mjs, findings-remeasure.mjs, tally.mjs): they parse the workflow with the repo's yaml package and execute every relevant run: block verbatim under bash --noprofile --norc -e -o pipefail with cwd = scratch workspace; the replay emulates runner semantics (continue-on-error, steps.<id>.outcome gates) in pool geometry. BSD/mutation/suite cells ran in scratch git worktrees at HEAD (tmp/scratch-tree, removed after; git worktree list shows the main tree only) and at HEAD^1 (tmp/base-tree, removed after); node_modules resolution for those trees walks up to the root install, which is a clean control because the PR touches no package.json/lockfile (no workspace symlinks are crossed by these test paths). Assertion totals: replay 28 + real-git 13 + wipe matrix 33 + BSD delta 10 + mutation 21 + suites 7 + lint gates 14 + findings re-measure 12 = 138 pass / 0 fail (assertions.json). Raw logs and per-cell vitest JSON: logs/; evidence images produced by scripts/verify-capture.mjs rerunning each harness as it printed.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — the macOS merge-queue lane did not run at this head (all check runs at the reviewed SHA were cancelled; the lane is merge-queue-only by design); the suite ran green on Linux (157/157) and the macOS gating fixes were traced in code.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — the macOS merge-queue lane did not run at this head (all check runs at the reviewed SHA were cancelled; the lane is merge-queue-only by design); the suite ran green on Linux (157/157) and the macOS gating fixes were traced in code。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round: no action requiredThis round ran in Critical-only mode (5 change-producing rounds already complete) and found nothing actionable:
No code changes were made and no commit was created. VerificationNo verification commands were run this round because no change was made — there was nothing to build, typecheck, lint, or test, and nothing was committed. CI at head 中文说明Autofix 轮次:无需任何操作本轮在 Critical-only 模式下运行(此前已完成 5 个产生改动的轮次),未发现任何可执行事项:
本轮未做任何代码改动,也未创建任何提交。 验证本轮未运行任何验证命令,因为没有产生任何改动 —— 没有需要 build、typecheck、lint 或测试的内容,也没有提交任何内容。head Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式:已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
There was a problem hiding this comment.
Re-reviewed the current head 952792a5 after the merge from main. No blockers found.
The checkout-heal chain is coherent end to end: the first checkout failure is survivable and addressable, the wipe and retry are gated on its actual outcome, the wipe canonicalizes and constrains the target under RUNNER_WORKSPACE, the retry is byte-identical to the first checkout, and a second checkout failure still fails the job. No downstream step consumes outputs from the failed first checkout.
Verification:
- checkout self-heal tests on stock macOS/BSD userland: 12 passed, with the GNU-only canonicalization case correctly skipped
- the same tests with GNU
realpath: 13 passed - extracted wipe script:
bash -nand ShellCheck clean - current-head Ubuntu and review workflow checks are green
The full workflow test file has one macOS-only runner-health fixture failure, but the same test fails unchanged on the base commit, so it is not introduced by this PR. Approved.
|
Released in v0.21.13. |
…ve-ab wipes (QwenLM#9277) * fix(ci): back-port the checkout-heal wipe guard to the triage and serve-ab wipes The "empty the workspace, keep the directory" idiom exists in three copies; only the review workflow's copy received the QwenLM#9220 hardening (canonicalization, trailing-slash strip, RUNNER_WORKSPACE allowlist). Measured on main for QwenLM#9265, the two triage guards let non-canonical spellings of the guarded roots through (/home/, /home/., //usr, /root/, /var/ all reached the rm), and serve-ab's wipe had no guard at all — even `/home` or an empty string arrived at `find … -exec rm -rf`. Port the reference guard to all three sites, keeping each site's exit contract: triage fails loud both before and after external code, serve-ab stays bare under the job's `-eo pipefail` so an unclearable workspace fails before either checkout builds on top of the leftovers. Pin each ported copy with its own tests: bad-path batteries under an rm recorder (the destructive primitive cannot fire under any edit), an allowlist-escaping `..` case gated on a GNU-realpath host probe (the lesson from 90fa6bb), a realpath-absent trailing-slash RUNNER_WORKSPACE case, and text pins on the ported layers. Every pin was mutation-verified red against a deletion of the layer it guards. * test(ci): pin guarded serve wipe * fix(ci): close wipe guard fallback gaps * fix(ci): fail closed without realpath * fix(ci): keep wipe guards portable * test(ci): pin wipe-guard RWS layers and unmask the pre-run battery - run the rewritten pre-run sweep battery under -e -o pipefail so a failing sweep can no longer report success (bare bash -c masked it) - pin the RWS '..' refusal and degenerate-root refusal text in all copies, and add RUNNER_WORKSPACE='/' exec cases to both copy suites - exercise both pre-run and post-run copies in the realpath-absent refusal test - replace the '..' escape vector with a symlink escape that only the realpath line can refuse, and correct the mutant-outcome comments - add the serve-ab wipe-before-checkouts ordering pin from the sister suite and a happy-path RWS canonicalization pin * test(ci): correct wipe-guard mutant-outcome comments for find -P The symlink-escape comments claimed that with the WS realpath line deleted, find reaches rm through the link target. GNU find's default -P mode does not descend symlink operands: the mutant passes every guard, wipes nothing, and exits 0, so only the non-zero-status assertion catches it — the rm-log assertion passes vacuously. Reword both twin comments (R5-1). --------- Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
The revived macOS lane ran the shared suite for the first time since the merge queue went dark and failed on exactly three tests, identical across three consecutive runs: the bite-check block and the baseline A/B green path die with `mapfile: command not found` (a bash >= 4.4 builtin; macOS ships 3.2), and the health-probe repair case trips over BSD `mktemp -u` attempting to create where GNU's print-only `-u` just names the canary. Both suites pin scripts that only ever execute on Linux runners — ubuntu-latest or the Linux ECS pool — so the defects cannot exist in production; the suites just cannot run those scripts on a macOS host. Follow the convention #9220's fix established for the realpath case in the same file: probe the host capability, not the platform, and skipIf the three dependent tests where it is absent. A Mac with a newer bash or GNU coreutils fronting PATH keeps the coverage; the Linux lane runs all three unconditionally. The large bite-check test body is re-indented by prettier around the new skipIf wrapper; no other token changed.
…-retry (QwenLM#9327) * refactor(ci): simplify the review checkout self-heal back to wipe-and-retry QwenLM#9220 fixed a real incident (a corrupt persisted workspace made seven review jobs fail checkout on the same missing SHAs), but eight review rounds grew the heal step from ~15 lines into ~60 lines of path-guard layers (realpath canonicalization, two trailing-slash strip loops, a denylist case, a RUNNER_WORKSPACE allowlist) plus ~450 lines of tests pinning their mutation resistance. Every removed layer defended against a mangled GITHUB_WORKSPACE. That variable is set by actions/runner; anything that could mangle it — a compromised runner, a step writing GITHUB_ENV — already executes arbitrary code on the machine and needs no wipe to do damage, so the guard cannot defend against the only actor able to trigger it. The realistic contract is the :? guard: fail loud on a dropped variable. Kept and still pinned by tests: the pool wipe idiom, the sudo fallback leg (exact argv), the never-fail exit contract with named survivors, the identical retry checkout, and the continue-on-error invariants. Also dropped with the guards: the GNU-only realpath flag and its host-probe test machinery. * test(ci): pin the runner-owned GITHUB_WORKSPACE premise before the workspace wipe Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): seal the live override channels into the review wipe step * fix(ci): refuse a redirected workspace and pin the clean-wipe silence Addresses the two doudouOUC findings on the simplified heal: - The wipe now validates the filesystem OBJECT at $WS, not just the string: find -P does not descend a symlinked start, so a redirected workspace logged 'wiped for a clean retry' while deleting nothing, and the secret-bearing review step would then run through the redirection. Refuse loud on a symlink or non-directory — POSIX-only, no false-positive surface (a legitimate workspace is always a runner-created plain directory), and it pins the only sudo-escalated wipe in the pool to a validated target. - The clean-wipe silence branch was unpinned: the reviewer's minimal mutant (dropping the if/fi pair) shipped an empty-list survivor warning on every heal with the suite green. The clean-wipe test now asserts the success annotation and the absence of the survivor warning; both mutants verified red. * test(ci): seal the wipe step's surviving override channels * test(ci): seal the wipe's surviving override channels, pin its signals Addresses the open review findings on the simplified heal: - The seal's premise covered declarative env, $GITHUB_ENV/$GITHUB_PATH run writes, and the pre-wipe action set, but three channels passed it unchecked: a wipe-step `shell:` or workflow/job `defaults:` wrapper re-targets the environment at exec time; SHELLOPTS rides the same bash-startup family as BASH_ENV/ENV yet sat outside the dangerous name class; and ACTIONS_ALLOW_UNSECURE_COMMANDS re-enables the legacy ::set-env:: / ::add-path:: spellings the run-text scan did not match. Each channel was reproduced green against the old seal (mutant probe) and now turns it red. - The both-legs-fail test now also pins the else-branch "could not wipe" warning, and a dedicated test pins the `[ ! -d ]` refusal for a nonexistent workspace — the plain-file test alone still passes a guard mutated to `[ -f ]`. - The non-sudo wipe leg keeps its stderr: the 2>/dev/null discarded exactly the diagnostics oncall needs when the wipe fails, and the sudo leg already ran unsuppressed. * fix(ci): refuse workspace wipe through symlinked path components (QwenLM#9327) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(ci): give the macOS and Windows lanes a trigger again Both lanes are gated on `merge_group`, and no merge queue is enabled on this repository — the `main` ruleset carries only deletion, non-fast-forward and pull_request rules, no status check is required, and merges land as squashes. The last `merge_group` run of anything was 2026-07-02. So the gate was an off switch: the lanes reported as "skipped" on every pull request, which reads as agreement, and nothing ever reached them afterwards. The only signal this repository has about a host that is not Linux with a GNU userland had been silently off for six weeks, which is how QwenLM#9220 shipped a GNU-only `realpath -m` in a workflow guard with the suite that pinned it red on every Mac. Three triggers now, in cost order. A pull request whose diff a new classifier recognises as platform-sensitive: shell scripts of every dialect, workflow and composite-action YAML and the scripts they call, the script layer and its tests, the test-runner configuration that decides which suites run where, the root manifests, and source paths whose segments name a platform-coupled subsystem. It is a net, not a proof — it cannot see a platform assumption inside an ordinary source file, and no path rule ever will — so every unknown answers "sensitive": an unreadable listing, an unparsable entry, a fork pull request, a truncated file list, or the classify job failing outright all end as "run the lanes". Only a confident `false` skips them. The merge queue, if it is ever enabled again, unchanged. And a nightly run on `main` for everything the path list cannot see. Every other job in the workflow excludes `schedule` explicitly, so a nightly is exactly two jobs, and 'Qwen Code CI' joins the workflows the main-failure watcher opens autofix issues for — a red lane nobody is told about is the same silence the queue gate produced. That watcher gains a trigger-level `branches: ['main']` filter so the CI workflow's pull-request completions do not raise an event there just to skip. The classifier runs in its own small hosted job rather than as a step in `classify_pr`: that job's outputs pick the Linux runner for the whole run, and this one needs a checkout — on a pool whose workspace other jobs have poisoned before. It checks out the pull request's BASE commit, never the head: it runs before any review and executes a script from the tree it checks out. Its listing goes through the existing classify-pr-profile.sh wrapper, extended with a mode argument, because that wrapper's whole point is that one PR is never listed twice and classified differently in two places. Twelve tests pin the wiring — the triggers, the fail-safe direction of the gate, the base-commit checkout, that a nightly stays two jobs, and the alerting — and ten more pin the classifier itself, including the substring traps (`Shellfish.tsx`, `cryptic.ts`, `plateauDetector.ts` must not drag both lanes in) and every fail-safe path. Mutation-checked: flipping the gate to `== 'true'`, dropping the schedule from a lane, dropping the nightly guard from the ubuntu job, pointing the checkout at the head, dropping the classifier's test from the helper list, and removing the schedule trigger are each caught. * fix(ci): gate the Windows lane's checkout verification per trigger The first thing the revived triggers hit was not a test failure but the lane's own plumbing. `test_windows` verifies its checkout with `verify-checkout-head`, and the input was written when this lane ran in the merge queue alone: `expected_sha: github.event.merge_group.head_sha`, with no event gate. On a pull request that expression is empty, the action refuses an empty SHA, and the lane went red in 63 seconds without running a test — the first Windows run in six weeks, failing on the trigger rather than on the code. Give it the event-aware shape the Ubuntu gate already uses, and skip it where there is nothing to verify: the scheduled and dispatch runs check out a branch by name, not a head commit. Pinned generally rather than by name: for both lanes, any step whose inputs read a `github.event.<event>` context must be gated to that event, in the step's own `if` or in the expression itself. Restoring the old spelling turns that test red. * fix(ci): close review round on the revived platform lanes (QwenLM#9370) * fix(ci): pin the Windows lane routing to the canonical trust policy (QwenLM#9370) * fix(ci): host-probe the macOS lane's bash and GNU dependencies (QwenLM#9370) The revived macOS lane ran the shared suite for the first time since the merge queue went dark and failed on exactly three tests, identical across three consecutive runs: the bite-check block and the baseline A/B green path die with `mapfile: command not found` (a bash >= 4.4 builtin; macOS ships 3.2), and the health-probe repair case trips over BSD `mktemp -u` attempting to create where GNU's print-only `-u` just names the canary. Both suites pin scripts that only ever execute on Linux runners — ubuntu-latest or the Linux ECS pool — so the defects cannot exist in production; the suites just cannot run those scripts on a macOS host. Follow the convention QwenLM#9220's fix established for the realpath case in the same file: probe the host capability, not the platform, and skipIf the three dependent tests where it is absent. A Mac with a newer bash or GNU coreutils fronting PATH keeps the coverage; the Linux lane runs all three unconditionally. The large bite-check test body is re-indented by prettier around the new skipIf wrapper; no other token changed. * fix(ci): stop the subsystem rule matching compounds that name something else The platform-sensitivity classifier split a path segment on dashes and underscores anywhere, so `packages/web-shell/**` matched the `shell` keyword — one of this repository's largest packages, a browser UI with no host coupling, summoning both expensive lanes on every change to it. That is the cost the gate exists to avoid, spent on the wrong diffs. A keyword now counts when it NAMES the thing: a whole path segment (`src/sandbox/**`, `platform/paths.ts`, `shell.ts`) or the head of a hyphen/underscore stem (`pty-host.ts`). Not a trailing part of a compound, which belongs to whatever the leading word names, and still not a substring inside a longer word. Pinned both directions, including a directory that IS named for the subsystem wherever it sits (`web-shell/components/shell/**` stays sensitive). Mutation-checked: dropping either rule, or restoring the split-anywhere spelling, turns the suite red. Also drops a wrong issue citation in the watcher test's comment: the nightly comes from this change, not from the wipe-guard back-port. * test(ci): pin the gate as a disjunction and the watcher's name binding Two mutation-survivable gaps in this PR's own tests. The clause-presence assertions left a connective mutation alive: `||` → `&&` between two event clauses keeps every asserted string in place and makes the gate unsatisfiable for every trigger — both lanes silently off again, which is the state this PR exists to end. Read the event group and require it to be a disjunction, allowing `&&` only inside the pull-request clause that binds to the classifier output. And the watcher's binding to this workflow is by display name: `workflow_run.workflows` matches the watched workflow's `name:`, so renaming ci.yml unhooks the nightly's alerting silently. Pin both sides. Both checked by mutation: flipping one `||` and renaming the workflow each turn a named test red. * fix(ci): close the classifier's CRLF gap and widen the lane step scan Three findings from this round, all in the direction of the tests and the parser being less clever than they claimed. The classifier's JSONL reader split on `\n` while its sibling splits on `/\r?\n/`. Every suffix rule here is end-anchored, so one carriage return on a CRLF listing would leave `build.sh\r` and classify a script-layer change as ordinary source. Matched to the sibling, with a fixture on both the JSON and the raw-line path. The per-trigger step scan serialized only `with:` inputs, so the same defect wearing an `env:` or `run:` key escaped it; it now reads all three. And the nightly blast-radius guard tested for the MENTION of an allowlisted event rather than the IMPOSSIBILITY of `schedule` — a job gated `pull_request || schedule` satisfied it while running every night. It now requires the absence of an explicit schedule clause too. * style(ci): format the runner-routing suite The Windows trust-policy matrix added last round left the file outside prettier's style, which the repository's lint step fails on; main's copy is clean. Formatting only — the nine assertions are unchanged and still pass. * fix(ci): gate the two mapfile-crossing gate tests on the host probe (QwenLM#9370) * fix(ci): keep pull requests off the persistent Windows pool and cover native audio (QwenLM#9370) A pull_request run executes the workflow YAML from the PR's own merge commit, so the test_windows runs-on trust clause it evaluated could be rewritten by any PR the lane admits. Every pull request now runs on hosted windows-2022 unconditionally; the pool is reached only by the post-approval merge queue, schedule and dispatch, guarded by the kill-switch. The routing tests and the exact-line pin are re-pointed at that enforceable shape. Also add audio to the platform-sensitivity classifier's subsystem keywords: packages/audio-capture is a node-gyp workspace compiled per-host on exactly the two revived lanes, but its native sources (.cc/.mm/.gyp) carried no rule and a PR touching only them skipped both lanes. The workspace directory now classifies sensitive; an ordinary .cc elsewhere stays ordinary source. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>










What this PR does
The automatic review job checks out the base branch on the reused self-hosted runner pool, and a failed checkout was terminal: the job died and nothing ever repaired the workspace it failed on. This PR makes that checkout self-healing: when the first attempt fails, the runner wipes the entire persisted workspace and retries the identical checkout once (same pinned action, same ref, same full-history fetch) before giving up. When the first attempt succeeds, nothing changes — the heal steps are gated on the first checkout's outcome and stay skipped.
Why it's needed
Between 2026-08-13 and 2026-08-15, seven review jobs landing on one runner (ecs-qwen-runner-64c-23) failed at checkout with
remote did not send all necessary objects, every time citing the SAME missing commit SHAs. The persisted workspace repository was corrupt — local refs claimed objects missing from the object store — so every fetch died in negotiation, and the machine was a guaranteed failure for every review dispatched to it for two days. Two further checkout failures in the same window were transient network drops mid-fetch (curl 92 HTTP/2 stream CANCEL/early EOF). Nothing recreates the workspace today, so one corruption permanently poisons a runner for this workflow; the workspace is fully disposable (later steps reinstall dependencies and tools), so wiping it and re-cloning is always a safe recovery.Reviewer Test Plan
How to verify
The happy path is unchanged: the heal steps only run when the first checkout fails. On failure, the wipe step deletes the whole workspace (guarded by
${GITHUB_WORKSPACE:?}so an unset variable fails loudly instead of removing something unintended, with a passwordless-sudo fallback for root-owned leftovers, matching the existing ownership-restore step), recreates the directory, and the retry runs the byte-identical checkout. Thirteen new tests pin the chain: the first checkout is survivable and addressable, the heal gates fire exactly onoutcome == 'failure'in the right order, the retry is identical to the first checkout (pin, ref, fetch-depth), the REAL wipe script empties and recreates a scratch workspace when executed, the guard refuses every suspicious and allowlist-escaping workspace path without ever invokingrm, and the script refuses to run whenGITHUB_WORKSPACEorRUNNER_WORKSPACEis empty. Runnpx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js— 131 tests pass. The suite is also exercised against a simulated BSD userland (arealpathonPATHthat rejects-m, the way Darwin's does) and against a symlinkedTMPDIR, because the canonicalization the guard relies on is a GNU-only flag: the assertions that depend on it are gated on a host probe, so the suite is green on a non-GNU host instead of failing there for a defect the Linux-only review pool cannot have. Durable confirmation will come from the next real checkout failure on the pool; note that until this merges, ecs-qwen-runner-64c-23 still needs a manual workspace wipe (or the runner drained), since the heal only ships with the workflow change.Evidence (Before & After)
N/A — CI infrastructure change, no user-visible output. Before: seven consecutive failed checkouts on one runner with identical missing-SHA annotations (
Could not read 0dcf7140…,Failed to traverse parents of commit …,remote did not send all necessary objects), visible in the workflow run annotations for 2026-08-13..15. After: the first such failure wipes the workspace once and the retry clones fresh; a second consecutive failure on a healed workspace is what would indicate a deeper problem.Tested on
Environment (optional)
Unit tests only: vitest against the workflow YAML, including executing the wipe script itself, on Linux — macOS is covered by simulating its userland (a non-GNU
realpath) and its symlinked temp directory, not by running on a Mac. The retry path will exercise for real on the self-hosted pool's next checkout failure.Risk & Scope
Linked Issues
None — observed directly in CI run annotations; no open issue tracks this.
中文说明
这个 PR 做了什么
自动 review job 在复用的 self-hosted runner 池上 checkout base 分支,此前 checkout 一旦失败就是终局:job 直接挂掉,而它失败所依赖的那个 workspace 永远不会被修复。本 PR 让这个 checkout 具备自愈能力:第一次失败时,runner 会清掉整个持久化 workspace,然后用完全相同的参数(同一个 pin 的 action、同一个 ref、同样完整历史 fetch)重试一次 checkout,再失败才算失败。第一次就成功时什么都不会变——自愈步骤以第一次 checkout 的 outcome 为条件,正常情况下全部跳过。
为什么需要
2026-08-13 到 08-15 之间,落在同一台 runner(ecs-qwen-runner-64c-23)上的 7 个 review job 全部在 checkout 阶段以
remote did not send all necessary objects失败,而且每次引用的都是完全相同的缺失 commit SHA。持久化 workspace 里的 git 仓库已损坏——本地 ref 指向对象库中不存在的对象——于是每次 fetch 都在协商阶段死掉,这台机器连续两天对所有调度过来的 review 都是必挂。同一窗口内另有两次 checkout 失败是 fetch 中途的网络瞬断(curl 92 HTTP/2 stream CANCEL/early EOF)。目前没有任何机制会重建 workspace,所以一次损坏就会让某台 runner 对这个 workflow 永久中毒;而 workspace 本身是完全可再生的(后续步骤会重新安装依赖和工具),因此清掉重clone永远是安全的恢复手段。Reviewer 测试计划
如何验证
正常路径不变:自愈步骤只在第一次 checkout 失败时运行。失败时,wipe 步骤删除整个 workspace(用
${GITHUB_WORKSPACE:?}保护,变量未设置时会响亮报错而不是误删,并带免密 sudo 兜底处理 root 属主残留,与现有的属主恢复步骤同一模式),重建目录,然后用字节级一致的参数重试 checkout。新增 13 个测试钉住整条链:第一次 checkout 可存活且可被引用、自愈条件恰好在outcome == 'failure'时按正确顺序触发、重试与第一次 checkout 完全一致(pin、ref、fetch-depth)、真实执行 wipe 脚本验证它能清空并重建临时 workspace、guard 对所有可疑路径以及能逃出 allowlist 的路径都拒绝执行且从不调用rm、以及GITHUB_WORKSPACE或RUNNER_WORKSPACE为空时脚本拒绝运行。运行npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/qwen-pr-review-workflow.test.js,131 个测试全部通过。此外还在模拟的 BSD userland(PATH 上放一个像 Darwin 那样拒绝-m的realpath)和软链TMPDIR下跑过:guard 依赖的规范化用的是 GNU 独有参数,因此依赖它的断言以主机探测为前提跳过——在非 GNU 主机上套件保持绿色,而不是为一个 Linux-only runner 池不可能出现的缺陷变红。持久的确认要等 runner 池上下一次真实的 checkout 失败;注意在本 PR 合入之前,ecs-qwen-runner-64c-23 仍需要手动清一次 workspace(或先下线该 runner),因为自愈是随 workflow 变更才生效的。Before / After 证据
N/A——CI 基础设施改动,无用户可见输出。Before:同一台 runner 上连续 7 次 checkout 失败,annotation 中的缺失 SHA 完全相同(
Could not read 0dcf7140…、Failed to traverse parents of commit …、remote did not send all necessary objects),见 2026-08-13..15 的 workflow run annotations。After:第一次此类失败会清一次 workspace,重试重新 clone;只有在自愈后的 workspace 上再次连续失败,才说明有更深层的问题。测试平台
环境(可选)
仅单元测试:vitest 解析 workflow YAML 并实际执行 wipe 脚本,运行在 Linux 上;macOS 是通过模拟其 userland(非 GNU 的
realpath)和软链临时目录覆盖的,并非在 Mac 上真机运行。重试路径会在 runner 池下一次真实 checkout 失败时得到验证。风险与范围
关联事项
无——直接从 CI run annotations 观察到,没有对应的 open issue。