fix(ci): keep a fallback comment when the PR review runner dies - #9255
Conversation
A review job that dies abnormally never reaches its in-job fallback comment step: the runner worker crash in FinalizeJob on the PR QwenLM#8894 run (EACCES creating under the runner home directory) left the PR with no review and no explanation. - Probe write access to $HOME, $RUNNER_TEMP and the runner root at job start, repair single-directory ownership with the existing sudo pattern, and fail fast with a clear message when repair is impossible instead of burning the review budget to die at finalize. - Add a fallback-comment job on an ephemeral hosted runner that posts the retry guidance whenever review-pr fails. It derives the PR number from the event payload (dead job outputs do not survive a crash) and dedupes on a qwen-review-fallback comment marker plus this run's URL, so the in-job step, the ack comment, and re-runs never double-post.
|
Re-run of the gate at the round-3 head ( Template looks good ✓ Problem: observed, not theoretical — the review run for PR #8894 (job 95028756485) died in the runner worker's finalization phase with permission-denied on the runner user's own home directory. Because the crash happened outside the normal step flow, no fallback comment was posted and no cleanup ran; the PR sat silently with no review and no explanation. The linked run/job is concrete evidence. Direction: aligned — this hardens the repo's own review pipeline against a failure class it demonstrably has. Pure CI-infrastructure change, no product surface. Size: not applicable — no core ( Approach: scope feels right. Two defenses for two distinct failure modes — a preflight health probe for corruption already present at job start, and a dependent fallback job on an ephemeral hosted runner for mid-run deaths — plus marker/run-URL dedup reusing the ack-comment convention. The gate enumerates exactly the jobs whose failure leaves Risk: no elevated risk signals — neither changed file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在第 3 轮 autofix 后的 head（ 模板完整 ✓ 问题：已观测到的事故，不是理论假设——PR #8894 的 review 运行（job 95028756485）死于 runner worker 收尾阶段：在 runner 用户自己的 home 目录下创建文件时权限被拒。崩溃发生在正常步骤流之外，兜底评论没发、清理没跑，PR 停留在无 review、无解释的状态。PR 描述附了具体 run/job 链接作为证据。 方向：对齐——加固的是仓库自己的 review 流水线，针对的是已被证实存在的故障类别。纯 CI 基础设施改动，不涉及产品面。 规模：不适用——未触及核心（ 方案：范围合理。两种防御对应两种不同的故障模式——前置健康探测处理 job 启动时已存在的损坏；跑在临时托管 runner 上的依赖 job 兜底运行中途的死亡——外加复用 ack 评论惯例的 marker/run-URL 去重。gate 恰好枚举了"失败会导致 review-pr 被跳过"的全部 job，我在当前 head 上对照实际 job 图重新核对过（ 风险：无升级风险信号——两个改动文件均不匹配与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewFresh pass at the round-3 head. The earlier blocking findings from prior rounds stay resolved, and I found no new blockers — a few notes on what I checked and what remains as follow-ups:
sequenceDiagram
participant P1 as Upstream gate jobs
participant P2 as review-pr on self-hosted
participant P3 as fallback-comment on hosted
participant P4 as PR comments
P2->>P2: health probe, repair or fail fast
P2-->>P3: failure or timeout-cancel opens gate
P1-->>P3: any upstream failure opens gate too
P3->>P4: list bot comments, marker-filtered
alt same-run fallback exists
P3->>P3: skip on marker plus run URL
else none yet for this run
P3->>P4: post marker-headed retry guidance
end
Non-blocking follow-ups (none of these block this PR):
Testing evidence (this PR's own CI, read via the API)The Additionally, the sandboxed
Sandboxed verification would settle the one remaining gap: 中文说明代码审查在第 3 轮 head 上重新过了一遍。此前各轮的阻塞性发现保持已修复状态，本轮没有发现新的阻塞问题——以下是我核对过的点和遗留的跟进项：
非阻塞跟进项（均不阻塞本 PR）：
测试证据（通过 API 读取本 PR 自己的 CI）
另外，针对较早 head — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — an incident-driven fix that survived three review rounds, a sandboxed verification (236/236 assertions, executed A/B), and a fresh static pass at the round-3 head; what remains is follow-up work, not doubt about this diff. Stepping back: this PR answers a real, evidenced failure — a review run that died in the runner's finalization phase and left a PR with no review and no explanation — and it answers it with the two defenses that failure actually calls for. A probe that catches an already-broken runner in seconds instead of burning hours of review budget, and a dependent job on an ephemeral runner that survives whatever killed the review job and posts the explanation itself. I tried to talk myself out of it and the objections didn't hold: the gate is scoped to real failures (designed skips and rejections stay quiet — verified against the job graph and the precheck's exit semantics, and already observed live at this head), the token-carrying job executes no PR code, and every lookup failure fails toward silence-or-retry rather than toward a duplicate. Six months from now this reads as careful ops hygiene, not cleverness. The 4 rather than 5 is honest bookkeeping, not hesitation: the Approving, pinned to the reviewed commit. @wenshao the F1 follow-up (excluding the 中文说明置信度：4/5 —— 一个由真实事故驱动的修复，经历了三轮评审、一次沙箱验证（236/236 断言、执行级 A/B），以及在第 3 轮 head 上的全新静态审查；剩下的是跟进工作，而不是对这个 diff 的疑虑。 退一步看：这个 PR 回应的是一个有证据的真实故障——review 运行死于 runner 收尾阶段，PR 落得既无 review 也无解释——而它给出的正是该故障真正需要的两种防御：一个探测，在几秒钟内发现已经损坏的 runner，而不是烧掉数小时的 review 预算；一个跑在临时 runner 上的依赖 job，能在杀死 review job 的任何故障中幸存，并自己发出解释。我试着说服自己否决它，但反对意见都站不住：gate 只针对真实失败打开（设计内的跳过与拒绝保持安静——已对照 job 图和预检的退出语义核实，并且在当前 head 上已有实时观察），携带 token 的 job 不执行任何 PR 代码，每次查找失败都倒向沉默或重试而非重复评论。六个月后再看，这读起来是审慎的运维卫生，而不是炫技。 给 4 而不是 5 是如实记账，不是犹豫： 批准，钉在被审提交上。@wenshao F1 的跟进（把 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
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)
| # Two levels above the workspace (_work/owner/repo) is the runner | ||
| # root, whose _diag/pages dir is what FinalizeJob creates in. | ||
| RUNNER_ROOT="$(cd "$GITHUB_WORKSPACE/../.." && pwd)" |
There was a problem hiding this comment.
[Suggestion] The RUNNER_ROOT derivation is off by one level on the self-hosted pool: $GITHUB_WORKSPACE/../.. resolves to the runner's _work directory, not the runner root — the workspace sits three levels below it (<runner-root>/_work/<owner>/<repo>). The probe therefore never checks the directory this comment names ("the runner root, whose _diag/pages dir is what FinalizeJob creates in"), and the repair arm chowns _work instead of the runner root.
Failure scenario: a prior containerised job running as root leaves the runner root or _diag root-owned/mode-broken while _work stays writable (the same corruption family the comment cites) → all probes pass green → the job burns its full review budget and the worker still dies in FinalizeJob with EACCES — the exact mode this step was added to fail-fast on. Witness (probed on this pool): workspace/../.. → _work (lists _actions _PipelineMapping qwen-code _temp _tool); one level further up → the runner root (lists _diag bin config.sh externals _work).
The observed incident class stays covered ($HOME and $RUNNER_TEMP are probed), so this is a coverage gap in the defense, not the PR's core promise — but the fix is one line.
| # Two levels above the workspace (_work/owner/repo) is the runner | |
| # root, whose _diag/pages dir is what FinalizeJob creates in. | |
| RUNNER_ROOT="$(cd "$GITHUB_WORKSPACE/../.." && pwd)" | |
| # Three levels above the workspace (_work/owner/repo) is the runner | |
| # root, whose _diag/pages dir is what FinalizeJob creates in. | |
| RUNNER_ROOT="$(cd "$GITHUB_WORKSPACE/../../.." && pwd)" |
中文说明
RUNNER_ROOT 的推导在 self-hosted 池上差了一层:$GITHUB_WORKSPACE/../.. 解析到的是 runner 的 _work 目录,而不是 runner 根目录——workspace 位于根目录下方三层(<runner-root>/_work/<owner>/<repo>)。因此探测从未真正检查注释声称的目录("runner 根目录,其 _diag/pages 目录是 FinalizeJob 写入的位置"),修复分支 chown 的也是 _work 而非 runner 根目录。
失败场景:先前以 root 运行的容器化 job 把 runner 根目录或 _diag 留成 root 属主/权限损坏,而 _work 仍可写(正是注释引用的损坏家族)→ 所有探测通过 → job 烧完整个 review 预算,worker 仍在 FinalizeJob 阶段以 EACCES 死亡——正是本步骤要快速失败的故障模式。实测(就在本池):workspace/../.. 列出 _actions _PipelineMapping qwen-code _temp _tool(即 _work 目录);再上一层才列出 _diag bin config.sh externals _work。
已观测到的事故类别仍有覆盖($HOME 与 $RUNNER_TEMP 已被探测),所以这是防御上的覆盖缺口,而非本 PR 的核心承诺——但修复只需一行。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # root, whose _diag/pages dir is what FinalizeJob creates in. | ||
| RUNNER_ROOT="$(cd "$GITHUB_WORKSPACE/../.." && pwd)" | ||
| status=0 | ||
| for dir in "$HOME" "${RUNNER_TEMP:?}" "$RUNNER_ROOT"; do |
There was a problem hiding this comment.
[Suggestion] The probe list covers $RUNNER_ROOT but not the _diag subdirectory this step's own comment names as the FinalizeJob write site — and a writable parent does not prove an existing subdirectory writable (ownership is per-directory). Demonstrated on this pool: with _diag mode-broken under a writable parent, the parent probe reported green while a FinalizeJob-shaped write inside _diag failed with EACCES. Combined with the off-by-one derivation above, _diag is doubly unreached; and the repair is single-directory (no recursion), so a correlated corruption event gets the probed dirs repaired while a still-broken _diag keeps the job dying.
Probe "$RUNNER_ROOT/_diag" as well when it exists (if it doesn't, FinalizeJob creates it, which only needs the already-probed runner root).
中文说明
探测列表覆盖了 $RUNNER_ROOT,但没有覆盖本步骤注释明确指出的 FinalizeJob 写入位置 _diag 子目录——且父目录可写并不能证明已存在的子目录可写(属主是逐目录的)。已在本池演示:父目录可写而 _diag 权限损坏时,父目录探测为绿色,而模拟 FinalizeJob 的 _diag 内写入以 EACCES 失败。叠加上面的层级推导偏差,_diag 双重不可达;且修复是单目录的(不递归),关联性损坏事件下被探测的目录修好了,仍然损坏的 _diag 却让 job 继续死亡。
建议在 _diag 存在时一并探测 "$RUNNER_ROOT/_diag"(若不存在,FinalizeJob 会创建它,只需要已被探测的 runner 根目录可写)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| exit 0 | ||
| fi | ||
| fallback_bodies="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json comments \ | ||
| --jq '.comments[] | select(.body | contains("<!-- qwen-review-fallback -->")) | .body' 2>/dev/null)" || fallback_bodies="" |
There was a problem hiding this comment.
[Suggestion] The dedup matches the marker + run URL in comments by ANY author. The sibling ack dedup in this same file filters on the posting account as well, and upsert-bot-comment.sh documents author-scoping as the property that keeps "a participant posting the marker" from capturing the lookup.
Failure scenario: anyone who can comment on this public PR posts a comment containing <!-- qwen-review-fallback --> plus actions/runs/<run_id> (the run id is published by the ack comment at run start and is sequential). When that run's review job then dies abnormally — the exact case this job exists for — the case match hits the planted body and the job exits 0 without posting: the retry guidance is silently suppressed, and the HTML-comment marker renders invisibly, so maintainers won't see why.
Verified live: gh pr view --json comments is GraphQL (comment keys: author, authorAssociation, body, … — no .user field, so the fix uses .author.login), and the in-job fallback comment is posted as qwen-code-ci-bot via CI_BOT_PAT, so an author filter still matches it. The bot login is the bot_login constant review-config already exports — keep them in sync rather than hardcoding a second copy.
| --jq '.comments[] | select(.body | contains("<!-- qwen-review-fallback -->")) | .body' 2>/dev/null)" || fallback_bodies="" | |
| --jq '.comments[] | select(.body | contains("<!-- qwen-review-fallback -->")) | select(.author.login == "qwen-code-ci-bot") | .body' 2>/dev/null)" || fallback_bodies="" |
中文说明
去重匹配的是任意作者评论中的标记 + run URL。同文件中姊妹的 ack 去重还额外过滤了发布账号;upsert-bot-comment.sh 也明确记载作者限定是保证"参与者发标记无法劫持查找"的关键属性。
失败场景:任何能在这个公开 PR 上评论的人,都可以发一条包含 <!-- qwen-review-fallback --> 和 actions/runs/<run_id> 的评论(run id 在运行开始时由 ack 评论公布,且顺序递增)。当该 run 的 review job 随后异常死亡——正是本 job 存在的场景——case 匹配命中伪造内容,job 直接 exit 0 不再发布:重试指引被静默抑制,而 HTML 注释标记渲染不可见,维护者看不出原因。
已实测验证:gh pr view --json comments 是 GraphQL(评论字段为 author、authorAssociation、body 等——没有 .user 字段,所以修复用 .author.login);job 内兜底评论通过 CI_BOT_PAT 以 qwen-code-ci-bot 身份发布,因此作者过滤仍然能匹配到它。bot 登录名就是 review-config 已导出的 bot_login 常量——保持同步,不要再硬编码第二份。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| fallback_bodies="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json comments \ | ||
| --jq '.comments[] | select(.body | contains("<!-- qwen-review-fallback -->")) | .body' 2>/dev/null)" || fallback_bodies="" |
There was a problem hiding this comment.
[Suggestion] A dedup-lookup failure is swallowed into an empty result and the step proceeds to post — fail-open toward duplicates. A transient GitHub API 5xx or a secondary rate limit on the shared CI_BOT_PAT makes fallback_bodies="", the case misses the comment the in-job step already posted for this run, and the PR gets a second identical fallback comment — the exact duplicate this marker dedup was added to prevent. The house norm says the opposite: upsert-bot-comment.sh's header — "A FAILED lookup is never treated as an EMPTY result: posting on a failed listing is how a transient 5xx mints a permanent duplicate" — with bounded retry as the established remedy.
Fail closed on a persistent lookup error (bounded retry, then skip with a summary line): fail-open buys nothing here, because when no comment exists both choices post — the only branch where they differ is the one that mints a duplicate.
中文说明
去重查找失败被吞成空结果,步骤继续发布——失败时偏向重复。共享 CI_BOT_PAT 上一次瞬时的 GitHub API 5xx 或次级限流就会让 fallback_bodies="",case 错过 job 内步骤为本 run 已发的评论,PR 上出现第二条一模一样的兜底评论——正是这个标记去重要防止的重复。仓库既有规范恰好相反:upsert-bot-comment.sh 头部写明"失败的查找绝不能当作空结果:在失败的列表上发布,就是瞬时 5xx 铸成永久重复的方式",并以有限重试作为既定补救。
建议在查找持续失败时失败关闭(有限重试,然后写一行 summary 跳过):失败开放在此没有任何收益——没有评论时两种选择都会发布,唯一有分歧的分支恰好是铸成重复的那个。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| pr_state="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json state --jq '.state')" || { | ||
| echo "Could not verify PR #${PR_NUMBER}; skipping fallback comment." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| } |
There was a problem hiding this comment.
[Suggestion] The twin failure mode of the lookup above: if this gh pr view call fails (expired/revoked CI_BOT_PAT, API outage), the job exits 0 GREEN with no comment posted and one ambiguous step-summary line — "Could not verify PR #N" reads identically for a deleted PR and for dead credentials. The run page shows fallback-comment ✓ for the one job whose sole purpose went unachieved, and the repo's flaky-rerun automation keys on failure, so green hides from it too: the failure-notifier fails silently.
Red-on-own-failure is safe here: the gate already requires review-pr.result == 'failure', so the run is red anyway. Emit a ::error:: annotation (annotations surface at run level; step summaries don't) and exit non-zero when the state check itself cannot complete; reserve exit 0 for a positively-determined non-OPEN PR.
中文说明
上面查找的对称失效模式:如果这个 gh pr view 调用失败(CI_BOT_PAT 过期/被吊销、API 故障),job 以 0 绿色退出,评论没发,只在 step summary 留下一句含糊的话——"Could not verify PR #N" 对"PR 已删除"和"凭证已失效"读起来一模一样。运行页面上,唯一职责没达成的 job 却显示 fallback-comment ✓;仓库的 flaky 重跑自动化以 failure 为键,绿色也同样躲过它:失败通知者自己静默失败了。
自身失败时变红在这里是安全的:门控已要求 review-pr.result == 'failure',整个 run 本来就是红的。状态检查本身无法完成时,发一条 ::error:: 注解(注解在 run 级别可见,step summary 不可见)并以非零退出;exit 0 保留给确认为非 OPEN 的 PR。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if: |- | ||
| always() && | ||
| needs.review-pr.result == 'failure' && |
There was a problem hiding this comment.
[Suggestion] The gate is keyed only to review-pr's result, but the incident's own trigger can kill the chain's EARLIER self-hosted jobs first: authorize runs on self-hosted ecs-qwen for same-repo PRs (verified live: run 31899604037 → ecs-qwen-runner-64c-7), and review-config likewise for bot review_requested. A dependent of a failed job is marked skipped, so needs.review-pr.result is 'skipped', this gate never opens, and the PR is left with no review and no explanation — reached through the PR #8894 trigger itself. (delay-automatic-review is hosted, so authorize/review-config complete the exposure list.)
Widen the gate by keying on the upstream jobs DIRECTLY:
needs: ['review-config', 'authorize', 'review-pr']
if: |-
always() &&
(needs.review-pr.result == 'failure' ||
needs.authorize.result == 'failure' ||
needs.review-config.result == 'failure') &&
…keep the existing repository / review_mode gates…Do not key on review-pr.result != 'success' instead — 4 failed workflow_dispatch resolve runs in recent history (e.g. 31915250131) skip review-pr BY DESIGN and would have received false-alarm fallback comments.
中文说明
门控只以 review-pr 的结果为键,但事故自身的诱因可以先杀死链上更靠前的 self-hosted job:同仓库 PR 的 authorize 跑在 self-hosted ecs-qwen 上(实测:run 31899604037 → ecs-qwen-runner-64c-7),bot review_requested 的 review-config 同理。依赖 job 失败时,下游被标记为 skipped,于是 needs.review-pr.result 是 'skipped',这个门控永远不会打开,PR 再次落入"没有 review 也没有解释"——正是从 PR #8894 的诱因走进来的。(delay-automatic-review 是托管 runner,所以 authorize/review-config 就是暴露面的全部。)
建议直接以上游 job 为键放宽门控(见上方代码)。不要改用 review-pr.result != 'success'——近期历史里有 4 个失败的 workflow_dispatch resolve 运行(如 31915250131)按设计跳过 review-pr,那样会收到误报的兜底评论。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if [ "$pr_state" != "OPEN" ]; then | ||
| echo "Skipping fallback comment: PR #${PR_NUMBER} is ${pr_state}." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
[Suggestion] This job drops the in-job step's moved-head guard (~line 1665 compares EXPECTED_HEAD_SHA with the current head): comment/review-event runs use per-run concurrency groups that a new push does NOT cancel (cancel-in-progress is only set for pull_request_target synchronize/closed). So a commit landing mid-review, followed by an abnormal review death, makes this job post stale "retry with @qwen-code /review" pointing at a dead run while a fresh lifecycle review of the new head is already queued — noise, and potentially provoking a redundant manual /review.
issue_comment runs, gh run view --json headSha returns the tip of main, not the PR head (verified: 8 recent issue_comment runs all show head_branch=main) — a naive headSha vs headRefOid comparison would mismatch on essentially every comment-triggered run and silently suppress the fallback on the very path this job exists for. Apply the guard only where a comparable head exists (pull_request_target events), and degrade to POSTING, never suppressing, when the comparison cannot be made.
中文说明
本 job 丢掉了 job 内步骤的"head 已移动"防护(约 1665 行会比较 EXPECTED_HEAD_SHA 与当前 head):评论/review 事件的运行使用按 run 分组的并发组,新 push 不会取消它们(cancel-in-progress 只对 pull_request_target 的 synchronize/closed 生效)。因此 review 进行中有新提交落入、随后 review 异常死亡时,本 job 会发出一条指向已死 run 的过期"重试 @qwen-code /review",而新 head 的自动 review 已经排队——噪音,还可能诱发多余的手动 /review。
issue_comment 运行,gh run view --json headSha 返回的是 main 的尖端而不是 PR 的 head(已验证:最近 8 个 issue_comment 运行全部 head_branch=main)——朴素的 headSha 对 headRefOid 比较几乎在每个评论触发的 run 上都会失配,从而在本 job 存在的核心路径上静默抑制兜底评论。只在 head 可比较的事件(pull_request_target)上加该防护;无法比较时,退化为发布,绝不抑制。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # dedupes on this marker plus this run's URL. | ||
| body="$(printf '<!-- qwen-review-fallback -->\n\n%s' "$body")" |
There was a problem hiding this comment.
[Suggestion] The marker literal is now hardcoded at three byte-identity-load-bearing sites across two jobs (this printf, the dedup filter at ~1777, and the fallback job's printf at ~1793). This same file states the norm for marker contracts (~line 1500): "The marker is defined ONCE and used for both the body and the upsert lookup — the two must be byte-identical or the upsert posts duplicates." A future edit renaming the marker in one or two of the three sites silently breaks cross-job dedup — every failed run that reached its in-job step then gets two fallback comments, and no test would fail (nothing outside this workflow references the marker).
Define it once — e.g. a workflow-level env: block (this file currently has none) — and reference $FALLBACK_MARKER in both printfs and via --arg marker "$FALLBACK_MARKER" / contains($marker) in the jq filter.
中文说明
标记字面量现在硬编码在两个 job 的三处"字节必须一致"的位置(这个 printf、约 1777 行的去重过滤器、约 1793 行兜底 job 的 printf)。同文件对标记契约已有规范(约 1500 行):"标记只定义一次,正文和 upsert 查找都用它——两者必须字节一致,否则 upsert 会发重复评论"。未来若只改三处中的一两处,跨 job 去重会静默失效——每个跑过 job 内步骤的失败 run 都会收到两条兜底评论,而且不会有任何测试失败(workflow 之外没有任何地方引用这个标记)。
建议只定义一次——例如 workflow 级 env: 块(本文件目前还没有)——两个 printf 都引用 $FALLBACK_MARKER,jq 过滤器用 --arg marker "$FALLBACK_MARKER" / contains($marker)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| fallback-comment: | ||
| needs: ['review-pr'] |
There was a problem hiding this comment.
[Suggestion] The behavior this PR adds — the health-probe fail-fast, this job, and the cross-job marker + run-URL dedup contract — is pinned by zero tests; reverting any hunk leaves every suite green (verified: grep finds no test reference to qwen-review-fallback / fallback-comment / Verify runner directory health; the efficacy probe had nothing it could probe). The repo already pins this exact workflow's invariants elsewhere: scripts/tests/qwen-pr-review-workflow.test.js and .github/scripts/qwen-triage-workflow.test.mjs parse this file and assert step invariants (assertUnconditional over the adjacent "Restore workspace ownership" step), and deadline.test.ts pins this workflow's env exports for the same drift reason.
Failure scenario: a future edit renames the marker on one side only, drops the See [workflow logs](…) link from a body variant, inverts the if: gate, or softens the probe to warn-only (drops status=1 / exit "$status") — each silently breaks dedup (duplicate comments) or restores the exact incident class (hours burned to die at finalize), and nothing fails.
Extend one of those suites: assert the in-job step prepends the marker and every FAILURE_KIND body includes ${RUN_URL}; this job selects the same literal marker, dedupes on actions/runs/${GITHUB_RUN_ID}, keeps needs.review-pr.result == 'failure' plus the repository gate, and exits 0 on each skip path; and the health step probes $HOME/$RUNNER_TEMP/the runner root, sets status=1 on unrepaired dirs, and ends with exit "$status".
中文说明
本 PR 新增的行为——健康探测快速失败、这个 job、跨 job 的"标记 + run URL"去重契约——没有任何测试固定;回退任何一个 hunk,所有测试套件仍然是绿的(已验证:全树 grep 没有任何测试引用 qwen-review-fallback / fallback-comment / Verify runner directory health;有效性探针也无可探)。仓库已在别处固定了同一 workflow 的不变量:scripts/tests/qwen-pr-review-workflow.test.js 与 .github/scripts/qwen-triage-workflow.test.mjs 解析本文件并断言步骤不变量(对相邻的 "Restore workspace ownership" 步骤有 assertUnconditional),deadline.test.ts 也出于同样的漂移原因固定了本 workflow 的 env 导出。
失败场景:未来某次编辑只改一侧的标记名、从某个 body 分支丢掉 See [workflow logs](…) 链接、反转 if: 门控、或把探测软化成只告警(去掉 status=1 / exit "$status")——每一种都会静默破坏去重(重复评论)或复活事故类别(烧几小时然后在 finalize 死亡),而不会有任何测试失败。
建议在上面任一测试套件中补充断言:job 内步骤前置标记且每个 FAILURE_KIND 正文都含 ${RUN_URL};本 job 选择同一标记字面量、按 actions/runs/${GITHUB_RUN_ID} 去重、保留 needs.review-pr.result == 'failure' 与仓库门控、每条跳过路径都 exit 0;健康步骤探测 $HOME/$RUNNER_TEMP/runner 根目录,修复失败置 status=1,并以 exit "$status" 结束。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
Review-round fixes for the fallback-comment defenses: - Probe the actual runner root (three levels above the workspace, not two) and the _diag subdirectory FinalizeJob writes in; a writable parent does not prove an existing subdirectory writable. - Open the fallback gate on authorize/review-config failures too — the incident's trigger can kill those earlier self-hosted jobs first, and a failed dependency marks review-pr 'skipped', which the old gate never matched. Guarded against resolve dispatch runs, which skip review-pr by design. - Author-scope the dedup lookup (resolved dynamically like upsert-bot-comment.sh) so a planted marker cannot suppress the fallback, and fail closed with bounded retry when the lookup or the state check fails instead of fail-open toward duplicates or a green job that never posted. - Skip the stale fallback when the PR head moved, but only on pull_request_target events where the run head is comparable — comment/review runs report main's tip, and posting wins over silence when the comparison is unavailable. - Define the marker once in a workflow-level env and pin all of the above in the workflow test suite, executing the fallback step's real bash against a stubbed gh.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #9255 (round 1)All 9 Suggestion findings from the automated reviewer were verified against the Findings and dispositions
Changed files
Verification
中文说明已处理的评审反馈 — PR #9255（第 1 轮）自动评审者的 9 条 Suggestion 发现全部经过代码核实，并在一个提交中解决（ 发现与处置
变更文件
验证
🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):
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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：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)
| (needs.review-pr.result == 'failure' || | ||
| needs.authorize.result == 'failure' || | ||
| needs.review-config.result == 'failure') && | ||
| github.event.inputs.command != 'resolve' && |
There was a problem hiding this comment.
[Critical] The /resolve exclusion only covers workflow_dispatch runs (github.event.inputs.command != 'resolve'), but /resolve is also a first-class issue_comment command — authorize.if matches startsWith(github.event.comment.body, '@qwen-code /resolve') — and on issue_comment events github.event.inputs is empty, so this exclusion never fires there. — Failure scenario: a maintainer comments @qwen-code /resolve on a same-repo PR; authorize runs on the self-hosted ECS pool and fails (a permission-API 5xx under set -euo pipefail, or the incident-class runner corruption this PR hardens against). review-pr is skipped by design (its if has no /resolve branch), the gate opens via needs.authorize.result == 'failure', and the job posts "Qwen Code review did not complete successfully … retry with @qwen-code /review" on a run where no review was ever attempted — misdiagnosing a resolve run and recommending the wrong command. Suggested fix: mirror authorize's /resolve predicate for comment events, e.g. add !(github.event_name == 'issue_comment' && startsWith(github.event.comment.body, '@qwen-code /resolve')) && to the gate, and extend the gate test's resolve case to the comment-driven path.
中文说明
/resolve 排除项只覆盖了 workflow_dispatch 运行（github.event.inputs.command != 'resolve'），但 /resolve 也是一等的 issue_comment 命令——authorize.if 匹配 startsWith(github.event.comment.body, '@qwen-code /resolve')——而在 issue_comment 事件中 github.event.inputs 为空，该排除项在那里永远不会生效。— 故障场景：维护者在同仓库 PR 上评论 @qwen-code /resolve；authorize 在自托管 ECS 池上运行并失败（set -euo pipefail 下的权限 API 5xx，或本 PR 所要防御的事故类 runner 损坏）。review-pr 按设计被跳过（其 if 没有 /resolve 分支），gate 经 needs.authorize.result == 'failure' 打开，在一个从未尝试过 review 的运行上发布 "Qwen Code review did not complete successfully … retry with @qwen-code /review"——把 resolve 运行误诊为 review 失败，并推荐了错误的命令。建议修复：在 gate 中为评论事件镜像 authorize 的 /resolve 谓词，例如添加 !(github.event_name == 'issue_comment' && startsWith(github.event.comment.body, '@qwen-code /resolve')) &&，并在 gate 测试中把 resolve 用例扩展到评论驱动路径。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # the same run id. The PR number comes from the event payload, not the dead | ||
| # job's outputs, which do not survive a crash. | ||
| fallback-comment: | ||
| needs: ['review-config', 'authorize', 'review-pr'] |
There was a problem hiding this comment.
[Suggestion] The gate hand-enumerates upstream failures but omits delay-automatic-review (review-pr's third direct dependency, line 365) and precheck-pr (the fork-PR chain root): their failures mark review-pr skipped without opening the gate, against this job's own comment — "a skipped review is just as unexplained as a dead one". — Failure scenario: (a) delay-automatic-review's 'Re-check PR state' step runs gh pr view under set -euo pipefail with no retry; a transient GitHub API 5xx fails the job, review-pr is skipped (failed dependency + missing should_review), the gate sees only skipped/success results and stays closed — the automatic review is silently lost. (b) For fork PRs, precheck-pr fails before posting anything (its comment step is gated on decision == 'manual_required'): the chain skips and the gate never opens — silence. Note: round-1 R1-6 scoped this gate to the incident class and explicitly excluded hosted jobs; the trigger here is transient API failure, a different mechanism — if hosted-job omission is deliberate, say so in the job comment. Suggested fix: add delay-automatic-review and precheck-pr to needs: and needs.delay-automatic-review.result == 'failure' || needs.precheck-pr.result == 'failure' to the disjunction (both are skipped where they don't apply, so the gate stays correctly closed there).
中文说明
gate 手工枚举上游失败，但遗漏了 delay-automatic-review（review-pr 的第三个直接依赖，第 365 行）和 precheck-pr（fork PR 链路的根）：它们的失败会把 review-pr 标记为 skipped 却不打开 gate，与本 job 自己的注释——"a skipped review is just as unexplained as a dead one"（被跳过的 review 和死掉的同样缺乏解释）——相矛盾。— 故障场景：(a) delay-automatic-review 的 'Re-check PR state' 步骤在 set -euo pipefail 下无重试地运行 gh pr view；一次瞬时的 GitHub API 5xx 就会使该 job 失败，review-pr 被跳过（依赖失败 + 缺少 should_review），gate 只看到 skipped/success 结果而保持关闭——自动 review 悄无声息地丢失。(b) 对 fork PR，precheck-pr 在发布任何内容之前失败（其评论步骤以 decision == 'manual_required' 为条件）：整条链路被跳过且 gate 永不打开——一片沉默。注：round-1 的 R1-6 把该 gate 限定在事故类（incident class）并明确排除了托管 job；此处的触发机制是瞬时 API 失败，属不同机制——如果有意排除托管 job，请在 job 注释中说明。建议修复：把 delay-automatic-review 和 precheck-pr 加入 needs:，并在析取式中加入 needs.delay-automatic-review.result == 'failure' || needs.precheck-pr.result == 'failure'（两者在不适用时均为 skipped，gate 在这些路径上仍正确保持关闭）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # from the account CI_BOT_PAT posts as. | ||
| bot_login="" | ||
| fallback_bodies="" | ||
| for _attempt in 1 2 3; do |
There was a problem hiding this comment.
[Suggestion] This dedup lookup re-inlines the marker+author upsert protocol that .github/scripts/upsert-bot-comment.sh was created to consolidate — the 3-attempt retry with sleep 10, dynamic gh api user --jq '.login' resolution, author-scoped marker filter, and fail-closed-on-failed-listing. The script's header states it exists because "the previous per-step copies had already drifted", and the in-code comment here even says "same norm as upsert-bot-comment.sh". The ~10% that differs is deliberate (dedup on marker + this run's URL, skip instead of PATCH), but the prerequisite machinery is now maintained in two places, and the new tests pin the inline form textually, cementing the second implementation. — Concrete cost: when the shared script's protocol is next hardened (retry count, backoff, fail-closed semantics), nothing references it from this job — the inline copy silently stays stale and the two comment paths drift, the exact failure mode the shared script was extracted to prevent. Suggested fix: add a base-ref actions/checkout step to this job (it currently checks out no code) and route the lookup/post through upsert-bot-comment.sh — either accepting upsert semantics or extending the script with a post-if-absent mode plus a caller-supplied body predicate.
中文说明
该去重查找把 .github/scripts/upsert-bot-comment.sh 被抽出来统一的 marker+author upsert 协议重新内联了一遍——3 次重试 + sleep 10、动态 gh api user --jq '.login' 解析、按作者过滤的 marker 筛选、查询失败即拒绝发布（fail-closed）。脚本头部写明它存在的原因正是"先前各步骤的拷贝已经发生漂移"，而这里的代码注释甚至写着 "same norm as upsert-bot-comment.sh"。约 10% 的差异是有意为之（以 marker + 本 run URL 去重、跳过而非 PATCH），但前置机制现在要在两处手工维护，且新测试以文本方式钉住了内联形式，把第二份实现固化了下来。— 具体代价：当共享脚本的协议下次被加固（重试次数、退避、fail-closed 语义）时，本 job 没有任何东西引用它——内联拷贝会悄悄停留在旧版本，两条评论路径随之漂移，正是共享脚本被抽取出来要防止的失败模式。建议修复：给该 job 增加一个 base-ref 的 actions/checkout 步骤（目前不 checkout 任何代码），并把查找/发布改走 upsert-bot-comment.sh——要么接受 upsert 语义，要么给脚本扩展一个"不存在才发布"的模式并支持调用方提供的正文谓词。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Declined for this PR (not on the merits of consolidation). The suggested fix extends .github/scripts/upsert-bot-comment.sh with a post-if-absent mode plus a caller-supplied body predicate and adds a base-ref checkout to this job — that script is shared machinery used by the docs-only relay and the stale-badge supersede step, so a protocol change risks regressing both, and .github/scripts/ is outside this PR's footprint. The ~10% divergence here is deliberate (dedup on marker plus this run's URL, skip instead of PATCH), and the inline copy is pinned textually and executed by the suite, so it cannot silently rot within this PR. Recommended follow-up: a dedicated PR consolidating all three comment-protocol sites (the upsert script, this dedup, and the R2-9 in-job lookup) behind one script mode.
中文说明
对本 PR 予以拒绝（并非不认可合并本身）。建议的修复要给 .github/scripts/upsert-bot-comment.sh 扩展"不存在才发布"模式并支持调用方正文谓词，还要给该 job 增加 base-ref checkout——该脚本是 docs-only relay 与 stale-badge supersede 步骤共用的机制，改动其协议有让两者回归的风险，且 .github/scripts/ 不在本 PR 的足迹之内。此处约 10% 的差异是有意为之（以 marker 加本 run URL 去重、跳过而非 PATCH），且内联拷贝被套件以文本钉住并实际执行，在本 PR 生命周期内不会悄悄腐化。建议后续：用一个专门的 PR 把三处评论协议站点（upsert 脚本、此去重、以及 R2-9 的 job 内查找）统一到同一脚本模式之下。
| expect(run).not.toMatch(/GITHUB_WORKSPACE\/\.\.\/\.\."/); | ||
| expect(run).toContain('"$HOME"'); | ||
| expect(run).toContain('"${RUNNER_TEMP:?}"'); | ||
| expect(run).toContain('"$RUNNER_ROOT/_diag"'); |
There was a problem hiding this comment.
[Suggestion] The health probe's [ -d "$RUNNER_ROOT/_diag" ] guard polarity is unpinned — toContain('"$RUNNER_ROOT/_diag"') matches the dirs+= line regardless of the guard, and nothing executes or asserts the if [ -d … ] condition. — Failure scenario: verified surviving mutant — inverting the guard to if [ ! -d "$RUNNER_ROOT/_diag" ] leaves the suite 131/131 green. On a runner where _diag does not yet exist (the case the workflow comment addresses), the mutant probes a nonexistent directory: touch fails, repair fails, and the review job fails fast on a healthy runner — while a corrupt existing _diag goes unprobed. Suggested fix: pin the guard text (expect(run).toContain('if [ -d "$RUNNER_ROOT/_diag" ]; then')) or add an executed-shape probe test with a sandboxed directory tree containing/omitting _diag.
中文说明
健康探测的 [ -d "$RUNNER_ROOT/_diag" ] 守卫极性未被钉住——toContain('"$RUNNER_ROOT/_diag"') 无论守卫如何都会匹配到 dirs+= 那一行，且没有任何测试执行或断言 if [ -d … ] 条件本身。— 故障场景：经验证存活的变异体——把守卫反转为 if [ ! -d "$RUNNER_ROOT/_diag" ] 后测试套件仍为 131/131 全绿。在 _diag 尚不存在的 runner 上（正是 workflow 注释所说明的情况），变异体会探测一个不存在的目录：touch 失败、修复失败，review job 在一台健康 runner 上快速失败——而一个已损坏的 _diag 反而不会被探测到。建议修复：钉住守卫文本（expect(run).toContain('if [ -d "$RUNNER_ROOT/_diag" ]; then')），或增加一个执行形态的探测测试，用包含/不包含 _diag 的沙箱目录树来验证。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ' [ "${SCENARIO:-}" = "lookup_fail" ] && exit 1', | ||
| ' cat "$COMMENTS_FILE"; exit 0 ;;', | ||
| ' *headRefOid*)', | ||
| ' [ "${SCENARIO:-}" = "prview_fail" ] && exit 1', |
There was a problem hiding this comment.
[Suggestion] The head-moved guard's "posting wins over silence" degradation is only tested when BOTH head lookups come back empty; the partial-failure edges are untested, and the stub's prview_fail scenario here is dead infrastructure no test invokes. — Failure scenario: verified surviving mutant — dropping [ -n "$run_head" ] && from the guard leaves the suite 131/131 green (the only partial-failure test runs with PR_HEAD unset, so the surviving check short-circuits). With the mutant, a transient gh run view 5xx while gh pr view … headRefOid succeeds makes [ "" != "<sha>" ] true: the fallback is suppressed on a dead run — silence instead of the retry comment, on the pull_request_target path the job exists for. Suggested fix: add runFallbackStep('runview_fail', { eventName: 'pull_request_target', prHead: 'newsha' }) expecting a post, and a prview_fail scenario expecting a post — pinning "comparison unavailable → post" for each lookup independently.
中文说明
head 已移动守卫的"发布优先于沉默"降级只在两次 head 查询都返回空时被测试；部分失败的边界未被测试，且此处 stub 的 prview_fail 场景是没有任何测试调用的死代码。— 故障场景：经验证存活的变异体——从守卫中删去 [ -n "$run_head" ] && 后套件仍为 131/131 全绿（唯一的部分失败测试在 PR_HEAD 未设置时运行，存活的检查会短路）。带上该变异体后，当 gh run view 瞬时 5xx 而 gh pr view … headRefOid 成功时，[ "" != "<sha>" ] 为真：兜底评论在一个已死的 run 上被抑制——在本 job 存在的 pull_request_target 路径上，换来沉默而非重试指引。建议修复：增加 runFallbackStep('runview_fail', { eventName: 'pull_request_target', prHead: 'newsha' }) 并断言会发布，再增加一个 prview_fail 场景断言会发布——分别为每次查询钉住"比较不可用 → 发布"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // while review-pr is skipped by design. | ||
| expect(job.if).toContain("github.event.inputs.command != 'resolve'"); | ||
| expect(job.if).toContain("github.repository == 'QwenLM/qwen-code'"); | ||
| expect(job.if).toContain("github.event.inputs.review_mode == 'comment'"); |
There was a problem hiding this comment.
[Suggestion] The gate test pins review_mode == 'comment' but not the disjunction guarding it — github.event_name != 'workflow_dispatch' || appears in no assertion. — Failure scenario: verified surviving mutant — collapsing (github.event_name != 'workflow_dispatch' || github.event.inputs.review_mode == 'comment') to the bare github.event.inputs.review_mode == 'comment' leaves the suite 131/131 green. On pull_request_target and issue_comment events review_mode is null, so the mutant evaluates false and the fallback-comment job is skipped on exactly the events that produce dead review runs — the entire round-2 defense disabled while the test certifies the gate. Suggested fix: add expect(job.if).toContain("github.event_name != 'workflow_dispatch'") alongside the existing gate assertions.
中文说明
gate 测试钉住了 review_mode == 'comment'，却没有钉住保护它的析取式——github.event_name != 'workflow_dispatch' || 没有出现在任何断言中。— 故障场景：经验证存活的变异体——把 (github.event_name != 'workflow_dispatch' || github.event.inputs.review_mode == 'comment') 坍缩为裸的 github.event.inputs.review_mode == 'comment' 后套件仍为 131/131 全绿。在 pull_request_target 和 issue_comment 事件上 review_mode 为 null，变异体求值为 false，fallback-comment job 恰恰在会产生死 review run 的事件上被跳过——整个 round-2 防御被废除，而测试还为 gate 背书。建议修复：在现有 gate 断言旁增加 expect(job.if).toContain("github.event_name != 'workflow_dispatch'")。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| exit 1 | ||
| fi | ||
| case "$fallback_bodies" in | ||
| *"actions/runs/${GITHUB_RUN_ID}"*) |
There was a problem hiding this comment.
[Suggestion] The dedup match is an unanchored substring test, broader than its stated contract ("marker plus this run's URL"): a fallback comment from any run whose numeric id merely CONTAINS this run's id suppresses this run's comment. — Failure scenario: run C dies without posting its fallback. Later run E dies and posts; E's id contains C's id as a digit substring (e.g. C=17123456789, E=171234567890 — as run ids grow a digit longer, each older id gains superstring successors). When the old failed run C is re-run (re-runs keep the same run id, which the design relies on) and dies again, the lookup finds E's marker comment, the pattern matches E's longer URL, and the job exits 0 "already exists" — C's re-run death is never announced. Probe-verified: reproduced on the unmodified PR; the anchored fix flips it, and legit same-id dedup still works (all five marker bodies render the URL as [workflow logs](${RUN_URL}), so the id is always immediately followed by )).
| *"actions/runs/${GITHUB_RUN_ID}"*) | |
| *"actions/runs/${GITHUB_RUN_ID})"*) |
中文说明
去重匹配是一个未加界定的子串测试，比其声明的契约（"marker 加本 run 的 URL"）更宽：任何数字 id 包含本 run id 作为子串的 run 的兜底评论，都会抑制本 run 的评论。— 故障场景：run C 死亡且未及发布兜底评论。之后的 run E 死亡并发布了评论；E 的 id 以数字子串方式包含 C 的 id（例如 C=17123456789，E=171234567890——当 run id 增长一位数，每个较早的 id 都会获得若干超串后继）。当旧的失败 run C 被重跑（重跑保留同一 run id，设计依赖这一点）并再次死亡时，查找会找到 E 的 marker 评论，模式匹配上 E 更长的 URL，job 以 0 退出"already exists"——C 的重跑死亡永远不会被通告。已用探针验证：在未修改的 PR 上复现；加界修复可使其翻转，且合法的同 id 去重仍然有效（五处 marker 正文都把 URL 渲染为 [workflow logs](${RUN_URL})，id 后总是紧跟 )）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # Blank line after the marker or the prose renders as raw source — | ||
| # same HTML-block quirk as the ack marker. The fallback-comment job | ||
| # dedupes on this marker plus this run's URL. | ||
| body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")" |
There was a problem hiding this comment.
[Suggestion] The cross-job dedup is one-directional: the new fallback-comment job lists existing marker comments before posting, but this in-job step has no dedup check — so a re-run after a fallback post mints a duplicate failure comment for the same run, contrary to the job comment's claim that "the same check dedupes re-runs, which keep the same run id". (In-job→in-job re-run duplicates pre-date this PR; the fallback→in-job pair is newly introduced by this diff.) — Failure scenario: attempt 1 — the self-hosted runner dies mid-review (the incident class this PR targets); this step never runs; the fallback-comment job posts the marker-headed comment linking actions/runs/<id>. The maintainer re-runs failed jobs (same run id). Attempt 2 completes but the review step fails for an ordinary reason (timeout/quota/generic): this step's if: failure() fires and posts a second marker-headed comment linking the identical run URL, because nothing on its path lists existing comments. Probe-verified via extract-step (the step posts without any comment listing; a fail-open lookup flips it). Suggested fix: apply the same author-scoped marker+run-URL lookup before posting here (fail-open — posting wins over silence, the fallback job still catches a true miss), or correct the job comment claiming re-run dedup.
中文说明
跨 job 的去重是单向的：新的 fallback-comment job 在发布前会列出现有 marker 评论，但 job 内的这个步骤没有去重检查——于是在兜底评论发布之后重跑，会为同一个 run 制造重复的失败评论，与 job 注释中"the same check dedupes re-runs, which keep the same run id"（同一检查会为保留相同 run id 的重跑去重）的说法矛盾。（job 内→job 内的重跑重复在本 PR 之前就存在；fallback→job 内的这一对是本 diff 新引入的。）— 故障场景：第 1 次尝试——自托管 runner 在 review 中途死亡（本 PR 针对的事故类）；此步骤从未运行；fallback-comment job 发布了带 marker、链接 actions/runs/<id> 的评论。维护者重跑失败的 jobs（run id 不变）。第 2 次尝试完成但 review 步骤因普通原因失败（超时/配额/通用）：此步骤的 if: failure() 触发，由于其路径上没有任何东西列出已有评论，又发布了第二条带 marker、链接同一 run URL 的评论。已用 extract-step 探针验证（该步骤发布时不做任何评论列出；加上 fail-open 查找后可翻转）。建议修复：在此处发布前应用同样的按作者过滤的 marker+run-URL 查找（fail-open——发布优先于沉默，fallback job 仍会兜住真正的遗漏），或修正声称重跑去重的 job 注释。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Acknowledged — this is a real duplication path. Deferred to the next round by this round's batch bound (eight findings implemented, Critical first). Planned fix: a fail-open, author-scoped marker + run-URL lookup before the in-job gh pr comment, evaluated together with the R2-3 consolidation question — adding a third inline copy of the lookup protocol this round would deepen exactly the drift R2-3 flags. The job comment's re-run-dedup claim is accurate for the fallback job's own check; the gap is the fallback→in-job pair, which the next round addresses.
中文说明
已确认——这是一条真实的重复路径。受本轮批次上限（实现了 8 个发现、Critical 优先）推迟到下一轮。计划中的修复：在 job 内 gh pr comment 之前加 fail-open、按作者过滤的 marker+run-URL 查找，并与 R2-3 的合并问题一并评估——本轮再加第三份内联查找拷贝会加深 R2-3 所指出的漂移。job 注释中关于重跑去重的说法对 fallback job 自身的检查是准确的；缺口在 fallback→job 内这一对，下一轮处理。
| github.repository == 'QwenLM/qwen-code' && | ||
| (github.event_name != 'workflow_dispatch' || | ||
| github.event.inputs.review_mode == 'comment') | ||
| runs-on: 'ubuntu-latest' |
There was a problem hiding this comment.
[Suggestion] This job's load-bearing placement invariant — it must run on an ephemeral hosted runner, never the self-hosted ECS pool whose failure mode it exists to survive — is pinned by no test; the suite pins only needs and if fragments. The job comment ("runs on an ephemeral hosted runner, so it survives whatever killed the review job") is the sole enforcement. — Failure scenario: a future edit routes fallback-comment onto the ECS pool (a routine-looking consolidation — the pool every other compute job in this file uses); all 131 tests stay green (verified mutant). On the next PR #8894-class incident the fallback job dies the same death as review-pr, and the PR is again left with no review and no comment — the exact state this PR exists to eliminate. Suggested fix: expect(job['runs-on']).toBe('ubuntu-latest') in the resilience suite, with a comment naming the survival requirement.
| runs-on: 'ubuntu-latest' | |
| runs-on: 'ubuntu-latest' |
中文说明
该 job 承重的部署不变量——必须运行在临时托管 runner 上、绝不能运行在它所要幸存其故障模式的自托管 ECS 池上——没有任何测试钉住；套件只钉了 needs 和 if 的片段。job 注释（"runs on an ephemeral hosted runner, so it survives whatever killed the review job"）是唯一的约束。— 故障场景：未来某次编辑把 fallback-comment 改到 ECS 池上（看似例行整合——该文件里其他所有计算 job 都用这个池）；全部 131 个测试保持绿色（变异体已验证）。下一次 PR #8894 类事故发生时，fallback job 会与 review-pr 同样死亡，PR 再次落入既无 review 也无评论的境地——正是本 PR 要消灭的状态。建议修复：在 resilience 套件中加入 expect(job['runs-on']).toBe('ubuntu-latest')，并用注释写明存活要求。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # Blank line after the marker or the prose renders as raw source — | ||
| # same HTML-block quirk as the ack marker. The fallback-comment job | ||
| # dedupes on this marker plus this run's URL. | ||
| body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")" |
There was a problem hiding this comment.
[Suggestion] The ordering invariant that this step prepends FALLBACK_MARKER to the body BEFORE gh pr comment is unpinned — the tests assert the printf line exists (toContain) but not that it precedes the post, so a regression that strips the marker from the posted in-job comment ships green. — Failure scenario: a future edit moves the printf line below the gh pr comment block (mutating a dead variable); the suite passes 131/131 (verified mutant). At runtime the in-job comment then posts without the marker, so the fallback job's author+marker dedup filter never sees it — and every run where review-pr fails after this step already posted mints a duplicate "review did not complete" comment, the duplication the marker was introduced to prevent. Suggested fix: pin the order (expect(inJobStep.run.indexOf(printf)).toBeLessThan(inJobStep.run.indexOf('gh pr comment')) — an established pattern elsewhere in this file), or execute this step's script the way runFallbackStep executes the fallback job's and assert the posted body starts with the marker.
中文说明
该步骤在 gh pr comment 之前把 FALLBACK_MARKER 前置到正文的这一顺序不变量未被钉住——测试只断言 printf 行存在（toContain），未断言它先于发布，因此一个把 marker 从所发布的 job 内评论中去掉的回归可以绿着上线。— 故障场景：未来某次编辑把 printf 行移到 gh pr comment 块之下（变异一个死变量）；套件 131/131 通过（变异体已验证）。运行时 job 内评论随后会不带 marker 发布，fallback job 的按作者+marker 去重筛选永远看不到它——于是每个 review-pr 在此步骤已发布之后才失败的 run 都会制造重复的 "review did not complete" 评论，正是 marker 被引入要防止的重复。建议修复：钉住顺序（expect(inJobStep.run.indexOf(printf)).toBeLessThan(inJobStep.run.indexOf('gh pr comment'))——本文件其他地方已有的既定模式），或像 runFallbackStep 执行 fallback job 脚本那样执行此步骤的脚本，并断言发布的正文以 marker 开头。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Acknowledged — deferred to the next round by this round's batch bound. The fix is the suggested one-line ordering assertion (indexOf(printf) < indexOf('gh pr comment') on the in-job step, an established pattern in this suite) and ships with the R2-9 follow-up.
中文说明
已确认——受本轮批次上限推迟到下一轮。修复即所建议的一行顺序断言（对 job 内步骤断言 indexOf(printf) < indexOf('gh pr comment')，本套件中已有的既定模式），随 R2-9 的后续一并提交。
…enLM#9255) - Exclude comment-driven /resolve runs from the fallback gate: authorize runs on `@qwen-code /resolve` issue comments where github.event.inputs is empty, so the dispatch-only exclusion never fired there and a failed resolve run was misdiagnosed as a dead review recommending the wrong command. - Enumerate precheck-pr and delay-automatic-review failures in the gate: either failure marks review-pr 'skipped' (a transient API 5xx in delay's re-check step, or the fork-PR chain root dying before it posts anything), which the old gate never matched — silence, against its own "a skipped review is as unexplained as a dead one" norm. Both are 'skipped' where they do not apply, so the gate stays closed there. - Anchor the cross-job dedup on the run URL's closing paren: run ids grow digits over time, so the unanchored substring let a later run's fallback comment (id 123450) suppress an earlier run's (id 12345) re-run comment; every marker body renders the URL as [workflow logs](...runs/<id>), so the id is always followed by ')'. - Pin the previously surviving mutants in the workflow suite: the _diag probe guard polarity, the workflow_dispatch disjunction, the ephemeral-hosted-runner placement, and the fallback body's marker-link shape the anchor relies on; add executed coverage for each head-lookup partial failure and for a distinct run's fallback not suppressing this run's comment.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： PR #9255 — round-2 review feedback addressedFindings and decisions
Eight findings were implemented this round (the Critical plus seven Reproduction before fixingPer the review-first rule, the new assertions were added to the workflow test
After the workflow fixes, all 135 tests pass. Two surviving-mutant claims were Changes
Declined: R2-3 (route the dedup through
|
| 发现 | 严重级别 | 决定 |
|---|---|---|
R2-1 —— /resolve 排除遗漏了评论驱动的 resolve 运行 |
Critical | 已修复 |
R2-2 —— gate 遗漏了 delay-automatic-review 与 precheck-pr 的失败 |
Suggestion | 已修复 |
R2-3 —— 去重查找重新内联了 upsert-bot-comment.sh 协议 |
Suggestion | 拒绝（理由见下） |
R2-4 —— _diag 健康探测守卫极性未被钉住 |
Suggestion | 已修复 |
R2-5 —— head 已移动守卫的部分失败边界未被测试（prview_fail stub 为死代码） |
Suggestion | 已修复 |
| R2-6 —— 去重测试只演练了相同 run id | Suggestion | 已修复 |
R2-7 —— gate 测试未钉住 workflow_dispatch 析取式 |
Suggestion | 已修复 |
| R2-8 —— 去重匹配是未加界定的子串测试 | Suggestion | 已修复 |
| R2-9 —— 跨 job 去重是单向的（job 内步骤没有去重） | Suggestion | 推迟到下一轮（说明见下） |
| R2-10 —— 临时托管 runner 部署不变量未被钉住 | Suggestion | 已修复 |
| R2-11 —— marker 前置先于发布的顺序未被钉住 | Suggestion | 推迟到下一轮（说明见下） |
本轮实现了 8 个发现（Critical 加 7 个 Suggestion）；R2-9 与 R2-11 按每轮批次上限推迟，并通过评论回复跟踪；R2-3 附理由拒绝。
修复前先复现
按"先复现"原则，新的断言先加入 workflow 测试套件，然后在未修改的代码上运行：
- 新的
/resolvegate 测试失败（评论驱动的排除项不存在）——复现 R2-1； - gate 测试在缺失的
delay-automatic-review/precheck-pr枚举与未钉住的workflow_dispatch析取式上失败——复现 R2-2/R2-7； - 新的不同 run 场景失败：run id 为
12345时，一条已存在的、链接actions/runs/123450的兜底评论通过未加界定的子串匹配抑制了本 run 的评论——在未修改代码上复现了 R2-8 的探针。
workflow 修复后，全部 135 个测试通过。另外对两个"存活变异体"的说法做了临时变异并还原的抽查：从 head 守卫中去掉 [ -n "$run_head" ] && 恰好使新的 run-head 部分失败测试失败；把去重模式放宽为 *actions/runs/* 恰好使新的不同 run 测试失败。
变更内容
.github/workflows/qwen-code-pr-review.yml：
- R2-1：在
fallback-commentgate 中新增!(github.event_name == 'issue_comment' && startsWith(github.event.comment.body, '@qwen-code /resolve'))，镜像 authorize 的评论驱动/resolve分支；issue_comment事件上github.event.inputs为空，仅靠 dispatch 排除项在那里永不生效。 - R2-2：在
needs:中加入precheck-pr与delay-automatic-review，并在 gate 析取式中加入两者的needs.<job>.result == 'failure'；两者在不适用时均为skipped，gate 在这些路径上仍正确保持关闭。job 注释现在明确写出枚举（自托管：authorize / review-config；托管：precheck-pr / delay-automatic-review）。 - R2-8：把去重 case 模式加界为
*"actions/runs/${GITHUB_RUN_ID})"*——所有 marker 正文都把 URL 渲染为[workflow logs](${RUN_URL})，id 后总紧跟)；超串 run id 不再能抑制本 run 的评论。
scripts/tests/qwen-pr-review-workflow.test.js：
- R2-1/R2-2/R2-7/R2-10：重写 gate 测试，钉住五个 job 的
needs枚举、全部五个失败析取项、(github.event_name != 'workflow_dispatch' ||析取项以及runs-on: 'ubuntu-latest'；把 resolve 排除拆成专门测试，同时覆盖 dispatch 与评论驱动两条路径。 - R2-4：钉住守卫文本
if [ -d "$RUNNER_ROOT/_diag" ]; then。 - R2-5：为
pull_request_target上每种 head 查询部分失败新增执行形态场景（runview_fail且 PR head 存活、以及此前是死代码的prview_fail场景且 run head 存活），均断言会发布。 - R2-6：新增"只存在不同 run 的兜底评论"场景——用超串 id
123450对 run12345，同时也钉住了 R2-8 的界——断言会发布。 - R2-8：同 run 去重用例现在镜像先前兜底评论的渲染形态（
See [workflow logs](...actions/runs/12345).），锚点所依赖的兜底 job 正文链接形态也与 job 内各份一并钉住。
拒绝：R2-3（把去重改走 upsert-bot-comment.sh）
就合并本身而言并非不认可，但对本 PR 予以拒绝：
- 该修复需要给
.github/scripts/upsert-bot-comment.sh扩展"不存在才发布"模式并支持调用方正文谓词，还要给该 job 增加 base-ref checkout。该脚本是 docs-only relay 与 stale-badge supersede 步骤共用的 CI 机制，改动其协议有让两者回归的风险，且.github/scripts/不在本 PR 的足迹内。 - 约 10% 的差异是有意为之：此查找以 marker 加本 run URL 去重、跳过而非 PATCH。内联拷贝被套件以文本钉住并实际执行，在本 PR 生命周期内不会悄悄腐化。
- 建议后续：用一个专门的 PR 把三处评论协议站点（upsert 脚本、此去重、以及下面 R2-9 的 job 内查找）统一到同一脚本模式之下。
推迟到下一轮
- R2-9：真实的重复路径（review 中途 runner 死亡后兜底评论已发布，随后同一 run 的重跑因普通原因失败又产生第二条带 marker 的评论）。受本轮批次上限推迟；计划中的修复是在 job 内步骤加 fail-open、按作者过滤的 marker+run-URL 查找，并与 R2-3 一并评估——再加第三份内联查找拷贝会加深 R2-3 所指出的漂移。
- R2-11：已确认；一行顺序断言随下一轮提交。
冲突说明
--conflict false；未执行任何合并。
验证
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-pr-review-workflow—— 135 通过（135），本轮前为 131；修复前运行出现复现 R2-1/R2-2/R2-7/R2-8 的 3 个预期失败- 变异抽查（临时、已还原）：去掉
[ -n "$run_head" ] &&→ 1 失败（新的 run-head 测试）；放宽去重为*actions/runs/*→ 1 失败（新的不同 run 测试） npm run build—— 通过（exit 0）npm run typecheck—— 通过（exit 0）npm run lint—— 通过（exit 0，无 ESLint 发现）- 对两个变更文件运行
npx prettier --check—— 干净 - workflow 的 YAML 解析检查：
fallback-comment的 needs/if/runs-on 按预期解析 - 未运行：集成测试（该行为由 workflow 测试夹具演练，不经捆绑 CLI）；
npm run generate:settings-schema（未触碰 settings 源）
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/模型 qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (round 5 reported a finding).
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.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (round 5 reported a finding)。
未检查（工具限制，非阻断）：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)
| echo "Skipping fallback comment: PR #${PR_NUMBER} is ${pr_state}." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi | ||
| body="**Qwen Code review did not complete successfully.** The review job failed before it could post details. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})." |
There was a problem hiding this comment.
[Suggestion] R3-1: The body asserts "The review job failed before it could post details", but this job's gate also opens on failures of precheck-pr, review-config, authorize and delay-automatic-review — on those paths review-pr was skipped, never ran, and the comment misnames the failing component. — Failure scenario: a transient API failure kills precheck-pr → the chain is skipped → the gate opens via needs.precheck-pr.result == 'failure' → the maintainer is told the review job failed and starts triage at review-pr's empty logs instead of the actually-failed upstream job — misdirection in the incident moment this job exists for. Witness: traced against the gate if: (lines 1779-1784), review-pr.needs (line 365) and GitHub's skipped-on-needs-failure semantics; all four upstream-failure paths reach this same static body.
| body="**Qwen Code review did not complete successfully.** The review job failed before it could post details. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})." | |
| body="**Qwen Code review did not complete successfully.** The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})." |
中文说明
正文断言 "The review job failed before it could post details"，但该 job 的 gate 同样会在 precheck-pr、review-config、authorize、delay-automatic-review 失败时打开——在这些路径上 review-pr 是被跳过、从未运行的，评论会把出错的组件指错。故障场景：一次瞬时 API 失败杀死 precheck-pr → 整链被跳过 → gate 经 needs.precheck-pr.result == 'failure' 打开 → 维护者被告知 review job 失败，从而在 review-pr 的空日志上开始排查，而不是去看真正失败的上游 job——恰恰在这个 job 存在的事故时刻造成误导。证据：已对照 gate if:（1779-1784 行）、review-pr.needs（365 行）及 GitHub 的 needs 失败即跳过的语义核实，四条上游失败路径都会落到这段静态正文。建议改为中性表述（见 suggestion 块）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| sudo -n chown "$RUNNER_UID:$RUNNER_GID" "$dir" 2>/dev/null || true | ||
| sudo -n chmod u+rwx "$dir" 2>/dev/null || true | ||
| if touch "$probe" 2>/dev/null; then |
There was a problem hiding this comment.
[Suggestion] R3-3: The health probe's repair-vs-fail-fast decision (probe → repair → re-probe → status) is pinned only by static toContain/toMatch assertions; no test executes this step's real bash through the repair branches. — Failure scenario: named surviving mutants — moving status=1 to right after the first failed touch ships a false fail-fast (a repairable directory aborts review-pr on every run until manual runner repair); deleting the second touch re-check ships a missed fail-fast (an unusable directory is reported "repaired" and the job dies in FinalizeJob — the PR #8894 incident this PR exists to prevent). Witness: probe ran the step's real bash with a stub sudo against a chmod-555 directory — mutant (a): REPAIRABLE → status:1 "failing fast" (original: status:0 "repaired"); mutant (b): UNREPAIRABLE → status:0 "repaired" (original: status:1); the suite stays 135/135 green under both mutants.
Suggested fix: add an executed-shape test mirroring runFallbackStep's stub harness — stub sudo, a read-only tmp dir standing in for $HOME, a fake GITHUB_WORKSPACE nested three deep — covering healthy (exit 0) / repairable (exit 0 + "repaired") / unrepairable (exit 1 + ::error::) / _diag-absent.
中文说明
健康探测的"修复还是快速失败"决策（探测 → 修复 → 复探 → status）目前只被静态 toContain/toMatch 断言钉住，没有任何测试真正执行这段 bash 的修复分支。故障场景：已点名的可存活变异——把 status=1 挪到首次 touch 失败后会带来误报式快速失败（可修复的目录导致 review-pr 每次运行都中止，直到人工修 runner）；删掉第二次 touch 复探则会漏掉快速失败（不可用目录被报告为"已修复"，job 最终死在 FinalizeJob——正是本 PR 要防的 PR #8894 事故）。证据：探针用 stub sudo 对 chmod-555 目录真实执行了该步 bash——变异 (a)：可修复 → status:1（原版 status:0）；变异 (b)：不可修复 → status:0（原版 status:1）；两种变异下测试套件均为 135/135 全绿。建议修复：仿照 runFallbackStep 的 stub 框架补一个执行型测试，覆盖健康/可修复/不可修复/无 _diag 四种路径。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| bot_login="" | ||
| fallback_bodies="" | ||
| sleep 10 |
There was a problem hiding this comment.
[Suggestion] R3-4: The retry loop's post-failure reset lines are unobservable in every test scenario — the only lookup-failure scenario (lookup_fail) fails BOTH gh api user and the comments listing, so bot_login ends up empty via the failed command substitution itself and these resets never matter. A mutant deleting them survives. — Failure scenario: gh api user succeeds while the comments listing returns 5xx on all three attempts → in the mutant, bot_login stays non-empty with empty fallback_bodies, the post-loop [ -z "$bot_login" ] guard passes, the dedup case matches nothing, and the job posts on a failed listing — the exact "transient 5xx mints a permanent duplicate" outcome the step's own comment forbids. Witness: probe ran the step's real bash under partial failure — original exits 1 with ::error:: and posts nothing; mutant (resets deleted) exits 0 and POSTED a comment; suite 135/135 green under the mutant.
Suggested fix: add a comments_lookup_fail scenario (user lookup succeeds, *comments* exits 1) asserting status === 1 and posted === ''.
中文说明
重试循环在失败后的两行重置（bot_login="" / fallback_bodies=""）在所有测试场景中都不可观测——唯一的查询失败场景 lookup_fail 同时令 gh api user 与评论列表失败，bot_login 本来就因命令替换失败而为空，重置行从未起作用，删掉它们的变异可以存活。故障场景：gh api user 成功而评论列表三次尝试全部 5xx → 变异体中 bot_login 非空而 fallback_bodies 为空，循环后的 [ -z "$bot_login" ] 守卫通过、dedup case 匹配不到任何内容，job 在一次失败的列表之上发布评论——正是该步注释明令禁止的"瞬时 5xx 铸成永久重复评论"。证据：探针在部分失败下真实执行该步 bash——原版 exit 1 且未发布；变异体 exit 0 且发布了评论；变异下套件 135/135 全绿。建议修复：新增 comments_lookup_fail 场景（user 查询成功、评论列表失败），断言 status === 1 且 posted === ''。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| PR_NUMBER: '${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}' | ||
| RUN_URL: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}' |
There was a problem hiding this comment.
[Suggestion] R3-5: This RUN_URL env expression is read by no test — the executed tests inject a literal RUN_URL — so the invariant the dedup depends on (the rendered link ends exactly at the run id, matching *"actions/runs/${GITHUB_RUN_ID})"*) is verified only for the injected literal, not for this YAML. The marker got byte-identical pinning tests for exactly this drift class; the URL shape did not. — Failure scenario: appending any suffix here (e.g. an /attempts/1 deep-link, plausibly added when someone improves the log link) makes the posted body contain runs/12345/attempts/1) → the dedup pattern no longer matches on re-run → every re-run of a dead review silently posts a duplicate fallback comment, with no error signal anywhere. Witness: probe (step's real bash, stub gh, GITHUB_RUN_ID=12345): base shape → NO POST (dedup matches); /attempts/1 mutant → POSTED (duplicate); mutated YAML suite run 135/135 green.
Suggested fix: assert this step env RUN_URL matches /\/actions\/runs\/\$\{\{ github\.run_id \}\}'$/ (and ideally equals the in-job step's RUN_URL expression).
中文说明
这个 RUN_URL env 表达式没有任何测试读取——执行型测试注入的是字面量 RUN_URL——因此 dedup 所依赖的不变量（渲染出的链接恰好在 run id 处结束，才能匹配 *"actions/runs/${GITHUB_RUN_ID})"*）只对被注入的字面量成立，对这段真实 YAML 并无验证。隐藏标记针对同类漂移已有逐字节钉住的测试，URL 形状却没有。故障场景：在此行追加任何后缀（例如有人改进日志链接时加上 /attempts/1 深链），正文将包含 runs/12345/attempts/1) → dedup 模式在重跑时不再匹配 → 死 review 的每次重跑都会静默多发一条兜底评论，且没有任何报错信号。证据：探针（真实 bash + stub gh，GITHUB_RUN_ID=12345）：原始形状 → 不发布（dedup 命中）；/attempts/1 变异 → 发布（重复）；变异下套件 135/135 全绿。建议修复：断言该 env 以 run id 结尾，并与 job 内步骤的 RUN_URL 表达式一致。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # dedupes on this marker plus this run's URL. | ||
| body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")" |
There was a problem hiding this comment.
[Suggestion] R2-9 (carried from round 2, ruled still standing at this commit): the cross-job dedup is one-directional — the new fallback-comment job lists existing marker comments before posting, but this in-job step posts with no marker/run-URL lookup (its body contains exactly one gh read, --json state,headRefOid, before gh pr comment). The fallback job's header claims "the same check dedupes re-runs", but re-runs re-fire THIS step (if: failure()), which has no check. — Failure scenario: run N dies mid-review (runner death; this step never runs) → fallback-comment posts comment #1 (marker + run N URL) → "Re-run failed jobs" (same run id) → review-pr fails again at step level → this step posts comment #2 with the same run URL → two near-identical comments for one run. Witness: not run — GitHub Actions re-run orchestration is the platform's; verdict rests on a full read of this step's body at HEAD. Acknowledged and deferred by the author automation in round 2; re-reporting because the mechanism still stands.
Suggested fix: give this step the sibling's guard before gh pr comment — the same bot-login-scoped, marker + actions/runs/${GITHUB_RUN_ID}) lookup (bounded retry optional here; on a failed lookup it can skip and let fallback-comment handle it).
中文说明
R2-9（第 2 轮携带，本轮代码核实仍然成立）：跨 job 去重是单向的——新的 fallback-comment job 在发布前会列出已有标记评论，但 job 内的这一步发布前没有任何标记/run URL 查询（正文在 gh pr comment 之前只有一处 --json state,headRefOid 读取）。兜底 job 的头注释声称"同一检查为重跑去重"，但重跑重新触发的是这一步（if: failure()），而它没有检查。故障场景：run N 在 review 中途死亡（runner 崩溃，此步未运行）→ fallback-comment 发出评论 #1（标记 + run N URL）→ 点击"Re-run failed jobs"（run id 不变）→ review-pr 再次在步骤级失败 → 此步带着同一 run URL 发出评论 #2 → 同一 run 出现两条几乎相同的评论。证据：未运行——重跑编排属于 GitHub Actions 平台；结论基于对 HEAD 处该步正文的完整阅读。第 2 轮作者自动化已确认并延期处理，因机制仍在，本轮继续报告。建议修复：在 gh pr comment 前给此步加上同款守卫（机器人登录作用域的标记 + run URL 查询；查询失败时可跳过、交给 fallback-comment 兜底）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| (needs.review-pr.result == 'failure' || | ||
| needs.authorize.result == 'failure' || |
There was a problem hiding this comment.
[Suggestion] R3-6: The gate disjunction admits only result == 'failure'. A review-pr that exceeds its job-level timeout-minutes (line 415, default 360) is auto-cancelled by GitHub with result 'cancelled' — which opens neither this gate nor the in-job step (failure() is false for a cancelled job). Both defenses miss on the timeout path. The workflow proves the authors hold this model elsewhere: Clean review worktrees uses if: 'always()' precisely because "a cancelled or timed-out review may not reach the CLI's process cleanup" (line 1705). — Failure scenario: a hang outside the CLI's own timeout enforcement (eight steps precede Run review and only Install capture tools has its own timeout) runs into the 360-min cap → review-pr result 'cancelled' → no in-job comment, gate closed → the PR is left with no review and no explanation — the same hole as the PR #8894 incident. Frequency is low (the cap sits far above the review budget by design), but the mechanism is deterministic once triggered. Witness: not run — GitHub Actions job-timeout semantics (conclusion cancelled, failure() false) only execute on GitHub's runtime; the quoted lines 415, 1653-1657, 1779-1784 and 1705-1711 carry the trace.
Suggested fix: add needs.review-pr.result == 'cancelled' to the disjunction — a run-level concurrency cancel cancels the whole run including the queued fallback job, so a fallback job that lives to evaluate the gate and sees 'cancelled' is overwhelmingly the job-timeout case (residual edge: manual single-job cancel, whose outcome is a benign retry-guidance comment) — and pin the chosen semantics in the gate test; or document in the gate comment that cancelled/timed-out runs are intentionally left silent.
中文说明
gate 的析取只接受 result == 'failure'。review-pr 超过 job 级 timeout-minutes（415 行，默认 360）时会被 GitHub 自动取消、结果为 'cancelled'——既打不开这个 gate，也触发不了 job 内步骤（被取消的 job 上 failure() 为假）。超时路径上两层防御全部落空。workflow 在别处证明作者清楚这一模型：Clean review worktrees 用 if: 'always()'，理由正是"被取消/超时的 review 可能走不到 CLI 的进程清理"（1705 行）。故障场景：CLI 自身超时管辖之外的挂起（Run review 之前有八个步骤，只有 Install capture tools 有自己的超时）撞上 360 分钟上限 → review-pr 结果 'cancelled' → 无 job 内评论、gate 关闭 → PR 既无 review 也无解释——与 PR #8894 事故相同的空洞。频率低（上限按设计远高于 review 预算），但一旦触发机制是确定的。证据：未运行——job 超时语义只在 GitHub 运行时生效；引用 415、1653-1657、1779-1784、1705-1711 行为证。建议修复：在析取中加入 needs.review-pr.result == 'cancelled'（run 级并发取消会连同排队中的兜底 job 一起取消，因此活着评估 gate 并看到 'cancelled' 的兜底 job 几乎必然是 job 超时；残余边界是手动取消单个 job，其后果只是一条良性的重试指引评论），并在 gate 测试中钉住所选语义；或在 gate 注释中声明有意对被取消/超时的运行保持沉默。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if bot_login="$(gh api user --jq '.login')" \ | ||
| && [ -n "$bot_login" ] \ |
There was a problem hiding this comment.
[Suggestion] R3-7: The cross-job dedup's load-bearing identity agreement — the in-job comment must be posted by the same account this lookup resolves via gh api user — is pinned by no test. It works today because the in-job step (line 1660) happens to use the same CI_BOT_PAT; nothing asserts it. — Failure scenario: mutate line 1660 to github.token (a plausible edit — the adjacent ack step posts with GITHUB_TOKEN) → the in-job comment is authored by github-actions[bot], invisible to this author-scoped filter → on every ordinary failure (in-job step posts, job result stays failure, gate opens) this job posts a second "did not complete" comment for the same run. Witness: probe applied that mutant — all four workflow-parsing suites stayed green (164 + 178 tests); adding one assertion pinning inJobStep.env.GH_TOKEN to CI_BOT_PAT made the same mutant fail (expected '${{ secrets.GITHUB_TOKEN }}' to be '${{ secrets.CI_BOT_PAT }}'). Tree restored, baseline 135/135.
Suggested fix: in the scopes the dedup to the authenticated bot login test, add:
expect(inJobStep.env.GH_TOKEN).toBe("'${{ secrets.CI_BOT_PAT }}'");with a comment that the author-scoped dedup only sees comments posted by the same account.
中文说明
跨 job 去重的承重不变量——job 内评论必须由本查询经 gh api user 解析出的同一账户发布——没有任何测试钉住。它今天成立只是因为 job 内步骤（1660 行）恰好也用 CI_BOT_PAT，并无任何断言保证。故障场景：把 1660 行改成 github.token（完全可能的编辑——相邻的 ack 步骤就用 GITHUB_TOKEN）→ job 内评论的作者变成 github-actions[bot]，对这个按作者过滤的查询不可见 → 每次普通失败（job 内步骤已发布、job 结果仍为 failure、gate 打开）本 job 都会为同一 run 再发一条 "did not complete" 评论。证据：探针应用该变异——四个解析此 workflow 的测试套件全部保持绿色（164 + 178）；加上一行把 inJobStep.env.GH_TOKEN 钉到 CI_BOT_PAT 的断言后，同一变异即失败。树已还原，基线 135/135。建议修复：在 scopes the dedup... 测试中补上该断言，并注明作者作用域去重只能看到同一账户发布的评论。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ' *comments*)', | ||
| ' [ "${SCENARIO:-}" = "lookup_fail" ] && exit 1', | ||
| ' cat "$COMMENTS_FILE"; exit 0 ;;', |
There was a problem hiding this comment.
[Suggestion] R3-8: The stub gh routes on a substring and cats a plain-text fixture regardless of --json/--jq arguments, so the fallback job's dedup jq filter and head-guard queries are never executed anywhere in this suite — only three substrings of the filter are pinned statically. The block's own comment advertises "reverting any hunk must fail here", but query-shape mutants pass. (The live queries are currently correct — verified against real gh — so this is a test-fidelity gap, not a live bug.) — Failure scenario: two demonstrated surviving mutants: (a) .comments[] → .comments[0] — dedup inspects only the PR's FIRST comment; a prior marker comment is essentially never first, so cross-job and re-run dedup silently miss it and double-post; (b) --jq '.headSha' → --jq '.conclusion' — run_head becomes null, never equals the PR head SHA, and the stale-head guard skips EVERY pull_request_target fallback — suppressing the comment on exactly the path the job exists for. Witness: probe applied both mutants to the YAML one at a time — BASE 135/135, MUTANT(a) 135/135, MUTANT(b) 135/135; grep confirms no positive assertion references .comments[], --json comments, --json headSha or --jq '.headSha'.
Suggested fix: route the stub's comments listing through real JSON and the caller's --jq argument (e.g. jq -n --rawfile b "$COMMENTS_FILE" '{comments:[{author:{login:"qwen-code-ci-bot"},body:$b}]}' then apply the passed filter with jq), or add an executed test that runs the extracted --jq program with real jq against a gh-shaped fixture — with the matching comment SECOND, to pin .comments[] iteration.
中文说明
stub gh 按子串路由、并不顾 --json/--jq 参数直接 cat 纯文本 fixture，因此兜底 job 的 dedup jq 过滤器与 head 守卫查询在整个套件中从未被执行——只有过滤器的三个子串被静态钉住。该测试块自己的注释宣称"还原任何 hunk 都必须在这里失败"，但查询形状的变异可以通过。注意：线上查询目前是正确（已对真实 gh 验证）的，所以这是测试保真度缺口，而非线上 bug。故障场景：两个已演示的可存活变异：(a) .comments[] → .comments[0]——dedup 只看 PR 的第一条评论，而标记评论几乎从不排在第一，跨 job 与重跑去重因此静默失效、导致重复发布；(b) --jq '.headSha' → --jq '.conclusion'——run_head 变为 null，永远不等于 PR head SHA，陈旧 head 守卫会跳过所有 pull_request_target 兜底——恰恰在该 job 存在的路径上把评论吞掉。证据：探针将两个变异分别应用到 YAML——BASE 135/135、变异(a) 135/135、变异(b) 135/135；grep 确认没有任何正向断言引用 .comments[]、--json comments、--json headSha 或 --jq '.headSha'。建议修复：让 stub 的评论列表走真实 JSON 并应用调用方的 --jq 参数，或补一个执行型测试，用真实 jq 对 gh 形状的 fixture 运行提取出的 --jq 程序——把匹配评论放在第二条，以钉住 .comments[] 的迭代。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
Deferred to the next round, not dropped. This round implements eight findings — two of them source changes (the in-job dedup guard and the cancelled gate) plus six test additions — and the test-line budget for this window is already committed to those pins; the stub-gh rework (or an executed-jq test with the matching comment second) is the largest remaining harness change and deserves its own round. Two notes shape the follow-up: the live queries are verified correct (as this finding itself states — a test-fidelity gap, not a live bug), so nothing ships unprotected in the meantime; and the preferred shape is the finding's second option, an executed test that runs the extracted --jq program with real jq against a gh-shaped fixture, since rewiring the shared stub's routing touches all twelve existing scenarios.
中文说明
延期到下一轮，不是丢弃。本轮实现了 8 条发现——其中两条是源码变更（job 内去重守卫与 cancelled gate），六条是测试新增——本窗口的测试行预算已全部投入这些钉住；stub gh 的重构（或"把匹配评论放在第二条、用真实 jq 执行提取出的 --jq 程序"的执行型测试）是剩余最大的框架改动，值得单独一轮处理。两点说明决定跟进方式：线上查询已验证正确（该发现本身也指出这是测试保真度缺口、非线上 bug），因此在此期间没有任何行为处于无保护状态；首选形态是该发现的第二个选项——执行型测试，因为它不需要改动共享 stub 的路由（那会触及全部 12 个既有场景）。
| # dedupes on this marker plus this run's URL. | ||
| body="$(printf '%s\n\n%s' "$FALLBACK_MARKER" "$body")" | ||
| gh pr comment "$PR_NUMBER" \ |
There was a problem hiding this comment.
[Suggestion] R2-11 (carried from round 2, ruled still standing at this commit): the ordering invariant that this step prepends $FALLBACK_MARKER to the body BEFORE gh pr comment is pinned only by existence assertions (toContain of the printf line) — grep across every test file at HEAD finds no ordering assertion for this step, despite the round-2 reply stating the one-line ordering assertion would ship. The fallback job's side IS pinned (the executed test asserts posted.startsWith(marker)), but this in-job step is not executed by the harness. — Failure scenario: a regression moving the printf below gh pr comment keeps every test green (toContain still matches) and ships fallback comments without the marker on the in-job path — breaking the cross-job and re-run dedup that keys on it. Witness: grep at HEAD — FALLBACK_MARKER appears only in existence assertions and env injection; no indexOf/toBeLessThan ordering assertion exists anywhere.
Suggested fix: add the one-line ordering assertion the round-2 thread already proposed — an established pattern in this suite:
expect(
inJobStep.run.indexOf(`body="$(printf '%s\\n\\n%s' "$FALLBACK_MARKER" "$body")"`),
).toBeLessThan(inJobStep.run.indexOf('gh pr comment'));中文说明
R2-11（第 2 轮携带，本轮代码核实仍然成立）：这一步在 gh pr comment 之前把 $FALLBACK_MARKER 前置到正文的次序不变量，目前只被存在性断言（对 printf 行的 toContain）钉住——在 HEAD 处 grep 全部测试文件都找不到针对该步的次序断言，尽管第 2 轮回复声称这条一行次序断言会随本轮推送。兜底 job 一侧已有钉住（执行型测试断言 posted.startsWith(marker)），但 job 内这一步并未被执行型框架覆盖。故障场景：把 printf 挪到 gh pr comment 之后的回归能让所有测试保持绿色（toContain 依然匹配），并在 job 内路径发出没有标记的兜底评论——破坏以标记为键的跨 job 与重跑去重。证据：HEAD 处 grep——FALLBACK_MARKER 只出现在存在性断言与 env 注入中，任何地方都没有 indexOf/toBeLessThan 次序断言。建议修复：补上第 2 轮线程已提出的一行次序断言（本套件的既有模式，见代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 236 passed · 0 failed · 236 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：236 通过 · 0 失败 · 236 总计 Verification reportPR #9255 Deep Verification —
|
| cell | environment | observable oracle | base (337da2143c) |
head (95ada483) |
|---|---|---|---|---|
| Incident: review job dies before any comment | workflow structure | fallback-comment job exists |
absent (job list: precheck-pr, ack-review-request, review-config, delay-automatic-review, authorize, review-pr, resolve-pr; no marker anywhere) | present, marker env defined once |
| In-job fallback body | real extracted step bash vs stub gh |
posted body bytes | posts guidance, no marker | posts marker + \n\n + byte-identical base body (03-base-ab-injob.png) |
| Dead-run PR gets an explanation | executed Post fallback comment step |
gh pr comment count + body |
0 (nothing exists to post) | 1, marker-headed, run-linked (01-… scenario 01) |
| Roll-out transition: base-shape comment exists | head dedup vs base-shape fixture | post/suppress | n/a | posts a duplicate (measured, see F2) |
Structural arm: 6/6 assertions (base-structure.sh); body-diff arm: 6/6 (base-injob.mjs). Witness: 03-base-ab-injob.png.
Executed scenario matrix (head, 01-fallback-scenario-matrix.png)
The affected suite at head — 135/135 tests green in 4.13 s — is witnessed by 00-suite-gate-head.png.
16 scenarios, 50 scripted assertions, all pass — including the ones the PR's own harness cannot reach: my stub gh applies the caller's real --jq filter to full payloads, so the author-scope + marker filter execute, whereas the PR's stub pre-applies the filter semantics and never runs it.
| # | scenario | expected | observed |
|---|---|---|---|
| 01 | incident: ack + human + planted-marker + other-run comments, no same-run fallback | post once, marker-headed, run-linked | ✅ |
| 02 | same-run bot fallback exists | suppress, exit 0 | ✅ |
| 03 | only run 123450's fallback exists (anchoring: id superstring of 12345) | post | ✅ |
| 04 | only the ack comment (links run URL, no marker) | post | ✅ |
| 05 | participant-planted marker + same-run URL | post (author scope defeats planting) | ✅ |
| 06 | dedup lookup fails persistently | exit 1, no post, exactly 3 attempts | ✅ |
| 07 | comments listing fails persistently | exit 1, no post, 3 full re-resolutions | ✅ |
| 08 | lookup fails twice, succeeds on 3rd | recover, post once | ✅ |
| 09 | PR state lookup fails | exit 1, no post | ✅ |
| 10 | PR merged meanwhile | exit 0, no post | ✅ |
| 11 | pull_request_target, head moved | skip before any comment lookup | ✅ |
| 12 | issue_comment run, heads differ | guard not applied, post | ✅ |
| 13/14 | pull_request_target, run-head / pr-head lookup fails | degrade to POSTING | ✅ ✅ |
| 15 | PR number undeterminable | exit 0, zero gh calls |
✅ |
| 16 | base-shape (marker-less) same-run comment | not deduped → duplicate (F2) | ✅ (as designed today) |
Health probe (head, 02-health-probe-matrix.png)
7 scenarios on real directory trees laid out as <root>/_work/<owner>/<repo>, 19 assertions, all pass: healthy layout silent + exit 0; broken $HOME without sudo → warning + ::error:: + exit 1; broken + repairable → "repaired", exit 0; _diag present-and-broken is probed and named in the error (proving three-level resolution); _diag absent is skipped; unset RUNNER_TEMP aborts loudly via ${RUNNER_TEMP:?}; broken runner root names the root itself, not _work. Residual: ownership-corruption repair (chown on a root-owned dir) was validated at effect level via the chmod path only — real passwordless-sudo escalation is not available in this container.
Gate enumeration (static, complete)
review-pr needs {review-config, delay-automatic-review, authorize}; authorize needs {precheck-pr}; delay-automatic-review needs {authorize}. The transitive failure closure is exactly the four jobs the gate enumerates plus review-pr itself — complete. authorize.if only admits PR-scoped events (github.event.issue.pull_request for comments), so PR_NUMBER can never resolve to a plain issue through any gate-opening path; cancelled runs yield 'cancelled' results, which the failure-keyed gate correctly ignores.
Vacuity / mutation matrix (04-mutation-matrix.png)
8 single-hunk mutants of the workflow, each applied → suite run → restored clean (git-asserted):
| mutant | change | PR suite | classification |
|---|---|---|---|
| M1 anchor | drop ) from the dedup run-URL match |
KILLED ('another run's fallback' executed test) | pinned |
| M2 diag polarity | if [ -d _diag ] → if [ ! -d _diag ] |
KILLED (structural polarity test) | pinned |
| M3 gate keying | == 'failure' → != 'success' |
KILLED | pinned |
| M4 author scope | drop select(.author.login == "$bot_login") |
KILLED | pinned |
| M5 resolve exclusion | drop the issue_comment /resolve guard |
KILLED (1 failed | 134 passed) | pinned |
| M6 two levels | ../../.. → ../.. |
KILLED | pinned |
| M7 backoff | sleep 10 → sleep 1 |
SURVIVED | coverage gap — the bounded retry count is pinned (3 attempts asserted); the delay value is not. Not dead code: the sleep executes in production. |
| M8 job timeout | timeout-minutes: 5 → 4 |
SURVIVED | coverage gap — job-level cap not pinned. Not dead code: Actions enforces it. |
Positive controls: the suite's 6 kills above, and my independent harness run against M1 — which turns scenario 03 red (and cascades scenario 01's post assertions) — quoted beside the survivors.
Corrections
None — no inaccurate claims from earlier rounds needed correcting. (One correction of my own harness is disclosed under Methodology.)
Findings
F1 — qwen-autofix censuses count the new fallback comment as actionable feedback (medium-low)
The PR adds a new bot comment type but qwen-autofix.yml's two comment censuses do not know about it:
BOT_COMMENT_FILTER(line 3581) excludes known non-actionable bot markers fromN_ISSUE_COMMENTS;qwen-review-fallbackis absent.- The NEWEST-watermark computation (line ~4573) carries its own inline marker alternation; also absent.
Measured by executing the censuses' exact jq programs (sliced verbatim from qwen-autofix.yml) with the workflow's own variable values (05-autofix-census-finding.png, harness/autofix-census.mjs):
| probe | current filter | without fallback comment | patched filter (+marker) |
|---|---|---|---|
| actionable-feedback count | 2 (fallback counted) | 1 | 1 |
| NEWEST watermark | 2026-08-16T06:00 (advanced by fallback) | 04:00 | 04:00 |
Consequence: on a PR inside the autofix fleet whose review dies, the fallback comment alone selects the PR for a review-address round — a full agent cycle spent "addressing" a failure notification. The PR description's risk section says "other workflows on the same runner pool are untouched" — true for the pool, but the comment stream is shared state, and this cost is one the description did not name.
Reproduce: node tmp/pr9255-verify-20260816-080753/harness/autofix-census.mjs
Suggested fix (measured, different file — follow-up PR or amend)
Add qwen-review-fallback| to both alternations in qwen-autofix.yml (the BOT_COMMENT_FILTER assignment and the inline test("<!\-\- (...) ") in the NEWEST computation). The patched-filter cells above are the measurement: with the token added, the fallback comment is excluded from both the count and the watermark while every other fixture behaves identically (positive control in the same harness: the qwen-triage-marked comment is excluded by the current filter, the human MEMBER comment is counted). No test currently pins the full list — scripts/tests/qwen-autofix-workflow.test.js spot-checks it (toContain('qwen-triage'), toContain('qwen-review-suggestion-summary') near line 7897), so the fix should land with a matching toContain('qwen-review-fallback') there; without that fixture the axis stays unpinned.
F2 — roll-out transition mints one duplicate on same-run re-runs (low, one-time window)
Dedup matches marker + run URL, but base-version in-job comments carry no marker. Measured (scenario 16 + base-injob.mjs): a run that posted its in-job comment under base code, then fails again and re-runs under head code (same run_id), posts a second fallback comment. Self-limiting: it only affects re-runs of runs that started before merge, and the Risk & Scope section already accepts "one extra fallback-style comment" for crash → re-run sequences — but that acceptance was argued from the re-run posting behavior, not from this marker-blindness, so naming it explicitly.
Reproduce: node tmp/pr9255-verify-20260816-080753/harness/fallback-matrix.mjs (scenario 16).
Not covered
- yamllint — environmental: this container ships Python 3.11 with no
pipmodule, noensurepip, andpip3 install --user(the repo installer's path) fails with permission denied; the pinned yamllint 1.35.1 could not be installed. actionlint 1.7.12 (the repo's pinned version, proven live against a planted violation) parses the whole workflow and passes; YAML parse is additionally proven by every harness's extraction. - Per-commit verification — the checkout is depth 2; of the PR's 3 commits (metadata snapshot) only the head
95ada483is locally reachable, so per-commit attribution was out of reach. The aggregateHEAD^1..HEADdiff is what every cell above verifies. - Live end-to-end posting — no GitHub token exists in this sandbox and the fallback job has never run in production, so the replay of the new step is uncalibrated against a real emitted artifact (there is none yet); calibration here is the structural A/B plus the byte-diff of the in-job body. A stub
ghencodes the documented API shapes (comments[].author.login,--jqapplication); real-API pagination ofgh pr view --json commentsbeyond one page is likewise unverified — on very long-lived PRs an old marker comment could sit outside the fetched window (re-runs dedup on recent comments, so practical risk is low). - chown repair of root-owned directories on a real runner — this container has no sudo binary and we run uid 1000; repair was proven via the
chmodpath with an effect-faithful shim, and the probe'ssudo -n chowninvocation was observed, but actual passwordless-sudo escalation is a runner-side property. - Hosted-fallback geometry — on
ubuntu-latestfallback runners three levels above the workspace is the runner home; reasoned from layout, not measured (the probe is harmless there: fresh VM, everything writable). - Mid-run corruption — scoped out by the PR itself; the probe says so in its own comment.
- Repo-wide test gate — only the affected suite (
scripts/tests/qwen-pr-review-workflow.test.js, 135 tests) ran, per targeted-gate policy.
Methodology
Environment: the CI verify container itself (node:22-bookworm lane runtime), uid 1000, node v22.23.2, bash 5.2.15, gh 2.97.0 (unauthenticated), jq 1.6; npm ci + npm run build pre-done at the merge ref. Runtime facts measured in-place ($RUNNER_TEMP = /__w/_temp here, no sudo/zstd/shellcheck binaries shipped) rather than read from YAML.
Each run: block under test was extracted verbatim through a YAML parser (harness/extract.mjs) into harness/*.sh, checked with bash -n (proven live), the repo-pinned shellcheck 0.11.0 (zero findings above note: severity; 84 style notes of the same classes the repo's own scripts carry), and repo-pinned actionlint 1.7.12 on the full workflow (clean; proven live with a planted expression violation). The repo's shellcheck gate only scans standalone .sh files — workflow-embedded bash is never shellchecked by it, and actionlint's shellcheck integration is explicitly disabled in scripts/lint.js — a pre-existing coverage observation, not a PR defect.
The fallback-step harness (harness/fallback-matrix.mjs) executes the step's real bash via bash --noprofile --norc against a stub gh whose fidelity choice is the point: it runs the caller's own --jq program with real jq over realistic fixtures (mixed authors, ack comment, planted marker), so the dedup filter executes rather than being assumed — the PR's own executed tests stub this away. The health harness (harness/health-matrix.mjs) breaks real directories via mode bits and observes the extracted probe. The vacuity round mutated the workflow in place, ran the suite, and restored with git checkout after each mutant (clean-tree asserted every time); M1 additionally ran through my harness as positive control. The full round as printed — killed-by test names, survivors, and the positive control — is 04-mutation-matrix.png. One incident of my own: the first positive-control pass ran green because the harness executed a stale pre-extracted copy of the step instead of the live workflow — the exact "scenario never reached the code under test" failure; fixed to extract per run, after which M1 correctly turned scenario 03 red and the restored head returned to 50/50.
Raw logs: logs/ (per-mutant vitest logs, shellcheck output, census log, harness-on-M1); harnesses and counts in harness/ and counts/; base worktree removed after the A/B cells were captured.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round 3 — PR #9255Commit: Eight of the nine round-3 findings are resolved in code; one (R3-8, the stub- Feedback dispositions
Also disclosed, unchanged: the reverse audit did not converge within its round cap, and actionlint's embedded-shell source mapping for this workflow remains unsupported (tool limitation, carried from the review's own disclosure). VerificationCommands actually run this round (all from the PR checkout at commit
中文说明Autofix 第 3 轮 — PR #9255提交： 第 3 轮共 9 条发现，其中 8 条已在代码中解决；1 条（R3-8，stub 反馈处置
同时如实披露（未变化）：reverse audit 未能在其轮数上限内收敛；actionlint 对该 workflow 的 embedded-shell source mapping 仍不支持（工具限制，沿用审查自身的披露）。 验证本轮实际执行的命令（除特别说明外均在 PR checkout 的
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 /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 309 passed · 0 failed · 309 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：309 通过 · 0 失败 · 309 总计 Verification reportPR #9255 Deep Verification (round 4) —
|
| # | finding (round 3) | severity | status at head 3cad0992 |
|---|---|---|---|
| F1 | qwen-autofix censuses count the fallback comment as actionable feedback | medium-low | stands — re-measured, surface widened: six filter sites lack the marker (the round-3 report measured two; a full sweep of the file at the current base finds five inline alternations, not one). Repro: node tmp/pr9255-verify-20260816-094340/harness/autofix-census.mjs (22/22). |
| F2 | roll-out transition mints one duplicate on same-run re-runs (base-shape comments carry no marker) | low | stands — re-measured on both dedup paths: fallback job (matrix scenario 17) and in-job step (scenario 27) both post a duplicate over a marker-less same-run comment. |
No declined/deferred rows existed in the previous round.
Scope selection
Central claim: when the review job dies or fails without posting its in-job fallback comment, the new fallback-comment job posts exactly one explanatory comment — deduped against an already-posted comment for the same run, never suppressed by other comments (ack, other runs, participant-planted markers), fail-closed on lookup failures.
Secondary claims: (1) the preflight health probe detects unwritable runner directories, repairs when possible, fails fast otherwise; (2) the gate opens on any upstream failure that marks review-pr failed/skipped, and never on skips or /resolve runs.
Round-3-delta probes (new this round): the in-job cross-job dedup block — same-run suppression, defer-on-failure semantics (exit 0, never fail-open), the anchor on the in-job side — plus mutants of both dedup sites and of the defer branch, which the previous round had not individually exercised.
Budget went to: the executed scenario matrix (71 assertions), structural + byte-diff A/B, health matrix, gate-completeness analysis, the autofix census re-measurement, the 144-test suite gate, a 13-mutant matrix plus two refined re-mutants, and the lint gates.
Central claim — A/B table
| cell | environment | observable oracle | base (3186d4ea) |
head (3cad0992) |
|---|---|---|---|---|
| Incident: review job dies before any comment | workflow structure | fallback-comment job / marker / health step exist |
all absent; qwen-review-fallback literal nowhere in the file |
present; job-list delta exactly +fallback-comment (02-health-gate-base-ab.png, base-ab 15/15) |
| In-job fallback body | real extracted step bash vs stub gh |
posted body bytes | posts guidance, no marker | posts marker + \n\n + byte-identical base body (01-fallback-scenario-matrix.png, cells 29/30) |
| Dead-run PR gets an explanation | executed Post fallback comment step |
gh pr comment count + body |
0 (nothing exists to post) | 1, marker-headed, run-linked, retry guidance (scenario 01) |
| Roll-out transition: base-shape comment exists | head dedup vs marker-less fixture | post/suppress | n/a | posts a duplicate (F2, scenarios 17 + 27) |
Witnesses: 02-health-gate-base-ab.png (structure), 01-fallback-scenario-matrix.png (behavior, incl. the byte-diff cells).
Executed scenario matrix (head, 01-fallback-scenario-matrix.png)
71 scripted assertions, all pass. The stub gh applies the caller's real --jq program with real jq over full comment payloads (mixed authors, ack comment, participant-planted marker), so the dedup filter's author scope and marker test execute rather than being assumed.
| # | scenario | expected | observed |
|---|---|---|---|
| 01 | incident: human + ack + participant-planted marker + other-run fallback present, no same-run fallback | post once, marker-headed, run-linked | ✅ |
| 02 | same-run bot fallback exists | suppress, exit 0 | ✅ |
| 03 | only run 123450's fallback exists (id is a superstring of 12345) | post (anchor holds) | ✅ |
| 04 | only the ack comment (links the run URL, no marker) | post | ✅ |
| 05 | participant-planted marker + same-run URL | post (author scope defeats planting) | ✅ |
| 06 | user lookup fails persistently — with a same-run comment present | exit 1, no post, exactly 3 attempts, ::error:: |
✅ |
| 07 | comments listing fails persistently | exit 1, no post, 3 full re-resolutions | ✅ |
| 08 | comments listing fails twice, succeeds on 3rd | recover, post once, exactly 3 attempts | ✅ |
| 09 | user lookup fails twice, succeeds on 3rd | recover, post | ✅ |
| 10 | PR state lookup fails | exit 1, no post | ✅ |
| 11 | PR merged meanwhile | exit 0, no post, state named | ✅ |
| 12 | pull_request_target, head moved | skip before any comment lookup | ✅ |
| 13 | issue_comment run, heads differ | guard not applied, posts; zero run-view calls | ✅ |
| 14/15 | pull_request_target, run-head / pr-head lookup fails | degrade to POSTING | ✅ ✅ |
| 15b | pull_request_target, heads equal | posts | ✅ |
| 16 | PR number undeterminable | exit 0, zero gh calls |
✅ |
| 17 | base-shape (marker-less) same-run comment | not deduped → duplicate (F2) | ✅ (as designed today) |
| 18 | in-job default failure | body byte-pinned: marker + exact guidance | ✅ |
| 19 | in-job: same-run fallback (cross-job dedup) | suppress | ✅ |
| 20/21 | in-job: user/comments lookup fails | defer exit 0, no post, summary names the fallback job | ✅ ✅ |
| 22 | in-job: only another run's fallback | post (in-job anchor holds) | ✅ |
| 23 | in-job: state+head lookup fails | fail-open skip (fallback job is the net) | ✅ |
| 24/25 | in-job: PR closed / head moved past EXPECTED_HEAD_SHA | skip clean | ✅ ✅ |
| 26 | in-job: quota failure kind | quota-aware body, marker-prepended | ✅ |
| 27 | in-job: marker-less same-run comment | duplicate (F2 via in-job path) | ✅ |
| 28 | in-job: participant-planted marker | author scope holds | ✅ |
Health probe (head, 02-health-gate-base-ab.png)
9 scenarios on real directory trees laid out as <root>/_work/<owner>/<repo>, 21 assertions, all pass: healthy layout silent + exit 0; broken $HOME/RUNNER_TEMP/runner-root with failing sudo → ::error:: + fail fast; repairable → "repaired" + exit 0; _diag present-and-broken is probed and named in the error (three-level resolution); _diag absent is skipped; unset RUNNER_TEMP aborts loudly via ${RUNNER_TEMP:?}; sudo binary absent entirely still fails fast and clean (H9 — the || true absorbs command-not-found, diagnosis intact).
Gate enumeration (programmatic, complete — 02-health-gate-base-ab.png)
Computed the transitive needs closure of review-pr from the parsed YAML: exactly {review-config, delay-automatic-review, authorize, precheck-pr}; asserted the gate enumerates precisely that closure plus review-pr (failure for all upstreams, failure+cancelled for review-pr, never != 'success'), runs on ubuntu-latest with a 5-minute cap and pull-requests: write only, posts as CI_BOT_PAT — the same identity as the in-job step, which is what makes the author-scoped dedup see the in-job comment. 36/36 assertions.
Corrections
One correction of the previous verification round's own coverage claim (not of any review comment or PR text): the round-3 report said F1 concerned two census sites (BOT_COMMENT_FILTER and the NEWEST watermark). A full sweep of qwen-autofix.yml at the current base finds six sites carrying marker alternations — the BOT_COMMENT_FILTER variable plus five byte-identical inline alternations (lines 4573, 4659, 5013, 5112, 5212). The finding's substance is unchanged; its surface is wider, and the suggested fix must edit all six.
Findings
F1 — qwen-autofix censuses count the new fallback comment as actionable feedback (medium-low, STANDS)
The PR adds a new bot comment type (posted as qwen-code-ci-bot, which is REVIEW_BOT in qwen-autofix.yml and therefore passes every author gate); qwen-autofix.yml's six comment filters do not know its marker:
BOT_COMMENT_FILTER(line 3581) — feedsN_ISSUE_COMMENTS;
2–6. five inlinetest("<!\-\- (…) ")alternations (lines 4573, 4659, 5013, 5112, 5212) — NEWEST watermark, live re-check census, eval feedback list, over-budget summary, and the issue-level comment context handed to the agent.
Measured by executing the censuses' jq programs sliced verbatim from the file with the workflow's own variable values (04-autofix-census-f1.png, harness/autofix-census.mjs, 22/22):
| probe | current filter | without fallback comment | patched filters (+marker, all sites) |
|---|---|---|---|
actionable-feedback count (N_ISSUE_COMMENTS) |
2 (fallback counted) | 1 | 1 |
| NEWEST watermark | 2026-08-16T06:00 (advanced by fallback) | 04:30 | 04:30 |
live re-check census (LIVE_NEW) |
2 | 1 | 1 |
Positive controls in the same run: a qwen-triage-marked comment and the ack are excluded by the current filters; the human MEMBER comment is counted. Regex-execution across all six sites confirms the fallback body matches none of the current alternations and would match each once qwen-review-fallback| is added.
Consequence (unchanged from round 3): on a PR in the autofix fleet whose review dies, the fallback comment alone can select the PR for a review-address round — a full agent cycle spent "addressing" a failure notification — and the comment's body enters the context the agent is told to address. The PR's Risk & Scope says "other workflows on the same runner pool are untouched" — true for the pool; the comment stream is shared state, and this cost remains unnamed in the description.
Reproduce: node tmp/pr9255-verify-20260816-094340/harness/autofix-census.mjs
Suggested fix (measured; different file — follow-up PR)
Add qwen-review-fallback| to all six alternations in qwen-autofix.yml (the BOT_COMMENT_FILTER assignment at line 3581 and the five inline test("<!\-\- (…) ") sites). The patched-filter cells above are the measurement: with the token added at every site, the fallback comment is excluded from count, watermark, and live census while every other fixture behaves identically. The five inline alternations are byte-identical today (asserted), so one sed-shaped edit applies to all of them — but the count is six sites, and no test currently pins the full list: scripts/tests/qwen-autofix-workflow.test.js spot-checks it (toContain('qwen-triage') etc.), so the fix should land with a matching toContain('qwen-review-fallback') there; without that fixture the axis stays unpinned.
F2 — roll-out transition mints one duplicate on same-run re-runs (low, one-time window, STANDS)
Dedup matches marker + run URL, but base-version in-job comments carry no marker. Re-measured at the new head on both dedup paths (01-fallback-scenario-matrix.png, scenarios 17 and 27): a run that posted its in-job comment under base code, then fails again and re-runs under head code (same run_id), posts a second fallback comment — whether the re-run's comment comes from the fallback job or from the in-job step. Self-limiting: only re-runs of runs that started before merge. The Risk & Scope section accepts "one extra fallback-style comment" for crash → re-run sequences, but argued it from re-run posting behavior, not from this marker-blindness — naming the mechanism explicitly, as in round 3.
Reproduce: node tmp/pr9255-verify-20260816-094340/harness/fallback-matrix.mjs (scenarios 17, 27).
Vacuity / mutation matrix (03-mutation-matrix.png)
Single-hunk mutants of the workflow at head; each applied → full suite run → restored (git status --porcelain clean after every mutant). The suite at head is 144/144 green unmutated (00-suite-gate-head.png, 5.7 s).
| mutant | change | suite result | killing test (intended assertion) | classification |
|---|---|---|---|---|
| M1a | in-job dedup pattern disabled | KILLED | 'in-job step dedupes on a fallback comment this run already has' | pinned |
| M1a-clean | in-job pattern → suffix match | KILLED | same test | pinned |
| M1b | fallback-job dedup pattern disabled | KILLED | 'dedupes on the marker plus this run URL' | pinned |
| M1b-pure | fallback-job ) anchor dropped, same-run kept |
KILLED | 'still posts when only another run's fallback exists' — expected '' not to be '' |
pinned |
| M2 | _diag guard polarity inverted |
KILLED (2 tests) | 'probes the runner root three levels up…' | pinned |
| M3 | gate == 'failure' → != 'success' |
KILLED | 'opens the gate on any upstream failure…' | pinned |
| M4 | author scope dropped from both dedups | KILLED | 'scopes the dedup to the authenticated bot login…' | pinned |
| M5 | issue_comment /resolve exclusion dropped |
KILLED | 'never opens the gate on a /resolve run…' | pinned |
| M6 | runner root two levels up | KILLED | 'probes the runner root three levels up…' | pinned |
| M7 | in-job cross-job dedup block removed | KILLED (2 tests) | 'in-job step dedupes…' | pinned |
| M8 | in-job lookup failure: defer → fail-open post | KILLED | 'in-job step defers to the fallback job when its dedup lookup fails' | pinned |
| M9-fixed | fallback-job RUN_URL gains /attempts suffix |
KILLED | 'pins the run-URL shape the dedup anchors on' | pinned |
| M10 | marker env value changed | KILLED | 'defines the fallback marker once…' | pinned |
| M11 | backoff sleep 10 → sleep 1 |
SURVIVED | — | coverage gap — the bounded retry count is pinned (3 attempts asserted); the delay value is not. Not dead code: the sleep executes in production. (Pre-existing survivor, re-measured.) |
| M12 | job timeout-minutes: 5 → 4 |
SURVIVED | — | coverage gap — job-level cap not pinned. Not dead code: Actions enforces it. (Pre-existing survivor, re-measured.) |
Positive controls: the suite's own 11 kills above; and my independent harness run against M1b-pure — scenario 03 (superstring run) goes red while scenario 02 (same-run suppression) stays green, isolating exactly the anchor's job. Two harness incidents, both adjudicated: the first M1a/M1b attempts produced quote-structure side effects in the case pattern (the ) anchor cannot be removed without moving a quote boundary in bash), so refined mutants (M1a-clean, M1b-pure) were run and killed cleanly; the first M9 attempt mutated resolve-pr's identical RUN_URL line instead of the fallback job's (survived for that reason — discarded, replaced by M9-fixed). Completeness note: the suite has no in-job other-run fixture — the in-job side of the superstring exclusion is proven by my harness scenario 22 and by the byte-identity of the two patterns, not by a suite fixture.
Bounded residuals (not findings)
- Cross-job dedup race: if a failed run's fallback job is still in flight while a manual single-job re-run's in-job step also posts, one duplicate is theoretically possible in the seconds-wide window between the fallback job's dedup read and its post. Inherent to comment-based cross-job dedup (no atomic upsert across jobs); at most one extra comment, never a missing one.
- Body fidelity on re-runs: when the fallback job already posted a generic comment for a run, a later in-job step on the re-run suppresses its quota/timeout-specific body in favor of no duplicate. Dedup-over-fidelity is the designed tradeoff; the run link in the standing comment still reaches the logs.
- Manually cancelled upstream jobs (single-job cancel of
authorizeetc.) yield a skippedreview-prthat the failure-keyed gate does not admit — silent by design of the failure keying (a run-level cancel takes the fallback job down with it; the PR's own comment documents the tradeoff).
Not covered
- yamllint — environmental: no
pip3in this container (node scripts/lint.js --setupreportspip3: Permission denied;command -v pip3empty), so the pinned yamllint 1.35.1 could not be installed. actionlint 1.7.12 (repo-pinned, proven live against a planted expression violation) parses the whole workflow clean; YAML parse is additionally proven by every harness's extraction. - Per-commit verification / round-3 delta isolation — the checkout is depth 2; of the PR's 4 commits (metadata snapshot) only head
3cad0992is locally reachable, and95ada483(the previously verified head) is not, so commit 4's delta cannot be git-separated from the aggregate. Every claim above verifies the aggregateHEAD^1..HEADdiff; delta scoping used the previous report's descriptions plus the observed suite growth (135 → 144 tests). - Live end-to-end posting — no GitHub token exists in this sandbox and the fallback job has never run in production, so the replay of the new step is uncalibrated against a real emitted artifact (there is none yet); calibration here is the structural A/B plus the byte-diff of the in-job body. Real-API pagination of
gh pr view --json commentsbeyond one page is likewise unverified (on very long-lived PRs an old marker comment could sit outside the fetched window; re-runs dedup on recent comments, so practical risk is low). - chown repair of root-owned directories on a real runner — no sudo binary in this container; repair was proven via the
chmodeffect path, and H9 proves the probe degrades cleanly when sudo is absent entirely. Actual passwordless-sudo escalation is a runner-side property. - Hosted-fallback geometry — on
ubuntu-latestfallback runners three levels above the workspace is the runner home; reasoned from layout, not measured (the probe is harmless there: fresh VM, everything writable). - Mid-run corruption — scoped out by the PR itself.
- Merge freshness — verified against the merge ref as fetched (base tip
3186d4ea); whethermainmoved after the fetch is unobservable in-sandbox. - Repo-wide test gate — only the affected suite (
scripts/tests/qwen-pr-review-workflow.test.js, 144 tests) ran, per targeted-gate policy.
Methodology
Environment: the CI verify container itself (node:22-bookworm lane runtime), uid 1000, node v22.23.2, bash 5.2.15, jq 1.6; npm ci + npm run build pre-done at the merge ref. Runtime facts measured in-place ($RUNNER_TEMP = /__w/_temp, no sudo/pip3/zstd binaries shipped) rather than read from YAML.
Every run: block under test was extracted verbatim through a YAML parser (harness/extract.mjs) into harness/*-step.sh, checked with bash -n, and executed via bash --noprofile --norc -eo pipefail (the runner's documented default shell) with one documented pre-substitution (${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }} → 300 for the in-job block's sole expression context). The scenario harness (harness/fallback-matrix.mjs + harness/gh-stub.sh) drives the real bash against a stub gh whose fidelity choice is the point: it runs the caller's own --jq program with real jq over realistic fixtures, so the dedup filter executes rather than being assumed; per-kind attempt counters in the stub pin the retry counts. The health harness (harness/health-matrix.mjs) breaks real directory trees via mode bits. The census harness (harness/autofix-census.mjs) slices the jq programs line-anchored from qwen-autofix.yml and executes them under the workflow's own variable values. The mutation round mutated the workflow in place, ran the full suite per mutant, and restored with a byte-for-byte rewrite of the original content, git status --porcelain-asserted clean each time; mutants M1a/M1b/M9 required one refinement round each after cause analysis (documented above the matrix). Lint gates (05-lint-gates.png): repo-pinned actionlint 1.7.12 and shellcheck 0.11.0 installed via node scripts/lint.js --setup, each proven live against a planted violation before use; shellcheck ran with the repo's flag set over the extracted blocks. The first two harness bugs of this round (a counter snapshot read after cleanup; scenario names not matching the stub's injection keys) were found by the harnesses' own red lines, root-caused, fixed, and re-run — the green totals above are the re-runs.
Raw logs: logs/ (per-harness and per-mutant logs); harnesses in harness/; evidence captures in evidence/ produced with scripts/verify-capture.mjs.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
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.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：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)
| try { | ||
| stdout = execFileSync( | ||
| 'bash', | ||
| ['-c', useInJobStep ? inJobStep.run : step.run], |
There was a problem hiding this comment.
[Suggestion] The executed in-job harness runs the in-job step's run block verbatim, but that block still contains the raw GitHub Actions expression MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}" — bash raises bad substitution, leaves the variable unset, and continues. — Failure scenario: a maintainer extending the harness with an in-job FAILURE_KIND: 'timeout' scenario gets silently wrong behaviour — [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ] with the latter empty always takes the else branch, so the small-timeout body variant is unreachable by executed tests (verified by probe: the raw block posts the max-timeout body for both 60 and 360; substituting '300' first restores both arms). The file's own convention routes such values through env: ('Run review' does, and the expression-length suite pins it ${{ }}-free).
Suggested fix: substitute before executing, e.g. (useInJobStep ? inJobStep.run : step.run).replaceAll('${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}', '300'), or move the value into the in-job step's env: block mirroring MAX_TIMEOUT_MINUTES_VAR in 'Run review'.
中文说明
已执行的 in-job 测试框架原样运行 in-job 步骤的 run 块，但该块仍包含原生 GitHub Actions 表达式 MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"——bash 报 bad substitution、变量保持未设并继续执行。— 故障场景：维护者若为 in-job 增加 FAILURE_KIND: 'timeout' 场景，会得到静默错误的行为——后者为空时 [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ] 恒走 else 分支，小超时 body 变体无法被执行测试覆盖（探针已验证：原始块对 60 与 360 都发最大超时 body；先替换为 '300' 可恢复两个分支）。本文件既有约定是经由 env: 传值（'Run review' 即如此，且表达式长度套件钉住其不含 ${{ }}）。
建议修复：执行前先替换，如 (useInJobStep ? inJobStep.run : step.run).replaceAll('${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}', '300')；或将该值移入 in-job 步骤的 env: 块，仿照 'Run review' 的 MAX_TIMEOUT_MINUTES_VAR。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # than burning hours of review budget to die at finalize. Only catches | ||
| # corruption already present at job start; mid-run corruption is covered | ||
| # by the fallback-comment job instead. | ||
| - name: 'Verify runner directory health' |
There was a problem hiding this comment.
[Suggestion] The only self-healing repair for runner-pool corruption lives in review-pr, the last job of the chain — downstream of the self-hosted jobs (authorize / review-config) that the same corruption kills first, so the repair is usually unreachable while the failure persists. — Failure scenario: an ecs-qwen runner with a root-owned $HOME (the exact PR #8894 trigger) kills authorize/review-config (verified: authorize runs self-hosted for same-repo heads; review-config has no needs and schedules first); review-pr is skipped without starting, so this repair never runs and the corrupted runner stays in the pool to kill the retry. The diff's own rationale concedes the trigger "can kill the chain's earlier self-hosted jobs first" — it covers that with a comment, not with repair.
Suggested fix: run the same probe (at minimum probe+repair) in the other self-hosted jobs of this workflow (authorize, review-config), or extract it as a shared preflight for every ecs-qwen job in this file.
中文说明
针对 runner 池损坏的唯一自愈修复位于链条最末的 review-pr——处在同样会被该损坏杀死的自托管 job（authorize/review-config）下游，因此故障持续期间修复通常不可达。— 故障场景：$HOME 属主为 root 的 ecs-qwen runner（正是 PR #8894 的触发条件）杀死 authorize/review-config（已核实：同仓库 head 时 authorize 跑在自托管上；review-config 无 needs、最先被调度）；review-pr 未启动即被跳过，本修复从不执行，损坏的 runner 留在池中继续杀死重试。diff 自身的理由也承认触发条件"可能先杀死链条上更早的自托管 job"——但只以注释覆盖，未以修复覆盖。
建议修复：在本 workflow 其他自托管 job（authorize、review-config）中运行同样的探测（至少探测+修复），或抽取为本文件所有 ecs-qwen job 共享的前置检查。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // incident's trigger can kill the chain's self-hosted jobs first | ||
| // (authorize / review-config), and a transient API failure can kill the | ||
| // hosted ones (precheck-pr / delay-automatic-review). | ||
| expect(job.needs).toEqual([ |
There was a problem hiding this comment.
[Suggestion] The fallback gate's upstream coverage is pinned as a literal five-job list, never derived from review-pr's transitive needs closure — today's agreement is real but coincidental (verified set-equal; job.needs appears exactly once in the file). — Failure scenario: a future change adds a new required job upstream of review-pr without touching fallback-comment; that job fails transiently, review-pr is marked skipped, the gate checks only its five literal jobs — none failed — and no fallback comment posts, silently reintroducing the exact unexplained-skip this PR exists to eliminate with every test still green.
Suggested fix: compute the transitive needs closure of doc.jobs['review-pr'] and assert job.needs is set-equal to closure ∪ {'review-pr'} and that job.if contains a needs.<id>.result == 'failure' clause for each — the derived-check style this file already uses.
中文说明
兜底门禁对上游的覆盖被钉成一个字面量五-job 列表，从未由 review-pr 的传递 needs 闭包推导——当前的一致是真实但巧合的（已验证集合相等；job.needs 在文件中仅出现一次）。— 故障场景：未来在 review-pr 上游新增一个必需 job 却未改动 fallback-comment；该 job 瞬时失败，review-pr 被标记 skipped，门禁只检查那五个字面量 job——无一失败——于是不发兜底评论，在所有测试仍为绿色的情况下，悄悄重新引入本 PR 旨在消除的"无解释跳过"。
建议修复：计算 doc.jobs['review-pr'] 的传递 needs 闭包，断言 job.needs 与闭包 ∪ {'review-pr'} 集合相等，且 job.if 对其中每个 job 都含 needs.<id>.result == 'failure' 子句——即本文件已采用的推导式检查风格。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| # posting on a failed listing is how a transient 5xx mints one. | ||
| bot_login="$(gh api user --jq '.login' 2>/dev/null)" || bot_login="" | ||
| fallback_bodies="" | ||
| if [ -n "$bot_login" ] \ |
There was a problem hiding this comment.
[Suggestion] The in-job dedup's error path "comments listing fails while gh api user succeeds" is never executed — the only in-job scenarios run are already, default, and lookup_fail; the fallback-job twin (comments_lookup_fail) IS tested. — Failure scenario: verified surviving mutant — decoupling the failed lookup from the condition (hoisted assignment + || true) leaves the suite green; the condition then succeeds with an empty body on a transient 5xx and the step posts without dedup, minting a permanent duplicate on the exact re-run case this block was written for (attempt 1 died, the fallback job already posted for this run id, attempt 2 hits a transient listing failure).
Suggested fix: add one scenario — runFallbackStep('comments_lookup_fail', { useInJobStep: true }) expecting status 0, posted '', and summary containing deferring to the fallback-comment job.
中文说明
in-job 去重的错误路径"gh api user 成功但评论列表失败"从未被执行——in-job 只跑 already、default、lookup_fail 三个场景；其 fallback-job 孪生路径（comments_lookup_fail）却有测试。— 故障场景：已验证存活的变异体——把失败的查询与条件解耦（提升赋值 + || true）后套件仍全绿；瞬时 5xx 时条件以空 body 成功，步骤未经去重就发布，在正是本块为之编写的重跑场景上铸成永久重复（尝试 1 死亡、fallback job 已就该 run id 发布、尝试 2 遇到瞬时列表失败）。
建议修复：新增一个场景——runFallbackStep('comments_lookup_fail', { useInJobStep: true })，期望 status 0、posted 为 ''、summary 含 deferring to the fallback-comment job。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| continue | ||
| fi | ||
| echo "::warning::no write access to $dir; attempting single-directory repair" | ||
| sudo -n chown "$RUNNER_UID:$RUNNER_GID" "$dir" 2>/dev/null || true |
There was a problem hiding this comment.
[Suggestion] The repair branch's chown is unverified by any test: the stub sudo no-ops chown (it only honours chmod) and no textual pin mentions it, so deleting the chown line leaves the whole suite green (verified by probe). — Failure scenario: on the step's own incident trigger (root-owned directories), chmod u+rwx without chown grants write to root, not the runner user, so repair can never succeed in exactly the case the step was written for — every incident degrades to fail-fast requiring manual runner repair instead of self-healing, while CI stays green. The harness cannot create root-owned fixtures without root, but the stub could still observe the invocation.
Suggested fix: have the stub sudo append "$@" to a log file (like the gh stub's $CALLS) and assert in the repair test that a chown <uid>:<gid> <broken-dir> invocation occurred.
中文说明
修复分支的 chown 未被任何测试验证：stub sudo 对 chown 不做任何事（只执行 chmod），也没有文本钉住它，因此删除 chown 行后整个套件仍为绿色（探针已验证）。— 故障场景：在该步骤自身针对的事故触发条件（root 属主目录）下，缺少 chown 的 chmod u+rwx 把写权限给了 root 而非 runner 用户，修复在该步骤正是为之编写的场景中永远无法成功——每起事故都退化为需人工修复 runner 的快速失败而非自愈，CI 却保持绿色。框架在无 root 时无法构造 root 属主夹具，但 stub 仍可记录该调用。
建议修复：让 stub sudo 把 "$@" 追加到日志文件（如 gh stub 的 $CALLS），并在修复测试中断言出现过 chown <uid>:<gid> <broken-dir> 调用。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| echo "Skipping fallback comment: PR #${PR_NUMBER} is ${pr_state}." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi | ||
| body="**Qwen Code review did not complete successfully.** The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with \`@qwen-code /review\`. See [workflow logs](${RUN_URL})." |
There was a problem hiding this comment.
[Suggestion] Fallback comments are fire-and-forget: nothing updates, retracts, or consolidates a marker comment after posting (repo-wide, the marker's only consumers are the two dedup filters). — Failure scenario: a run dies, the fallback posts "retry with @qwen-code /review", the retry succeeds and the real review posts — a stale, still-actionable failure claim persists beside the completed review, and a contributor following its advice mints another full review run; on a persistently unhealthy pool each instructed retry and each synchronize run posts one more near-identical comment. This file already upserts the ack and the docs-only badge (upsert-bot-comment.sh --update-only); the fallback marker is the only singleton bot comment with no such path.
Suggested fix: route the post through upsert-bot-comment.sh (one marker comment per PR, updated with the latest dead run's URL), and/or supersede a stale fallback comment on the success path — at minimum, decide and document that staleness/accumulation is accepted.
中文说明
兜底评论是"发后即忘"：发布后没有任何机制更新、撤回或合并标记评论（全仓库范围内，该标记的唯一消费者是两处去重过滤器）。— 故障场景：某 run 死亡，兜底发布"请用 @qwen-code /review 重试"，重试成功、真正的 review 发出——一条仍然可操作的陈旧失败声明与已完成的 review 并存，贡献者照做又会触发一次完整 review；在持续不健康的 runner 池上，每次按其指引的重试、每次 synchronize 运行都会再发一条几乎相同的评论。本文件对确认评论与仅文档徽章都已采用 upsert（upsert-bot-comment.sh --update-only）；兜底标记是唯一没有此路径的单例 bot 评论。
建议修复：经由 upsert-bot-comment.sh 发布（每 PR 一条标记评论，更新为最新死亡 run 的 URL），和/或在成功路径上取代陈旧的兜底评论——至少明确并记录接受陈旧/累积。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ' case "$*" in', | ||
| ' *comments*)', | ||
| ' case "${SCENARIO:-}" in lookup_fail | comments_lookup_fail) exit 1 ;; esac', | ||
| ' cat "$COMMENTS_FILE"; exit 0 ;;', |
There was a problem hiding this comment.
[Suggestion] R3-8 (carried from round 3, ruled still standing at this commit): the executed harness's stub gh cats the fixture verbatim and never applies the --jq dedup filter, so the filter's semantics are unexecuted for BOTH steps — the comment claiming the stub "pre-applies the dedup filter's semantics" overstates the coverage. — Failure scenario: verified surviving mutant — changing the filter's projection | .body to | .id (in either step's run block) leaves the suite green, because the stub ignores --jq; at runtime the filter then returns comment node IDs, the case "$fallback_bodies" in *"actions/runs/${GITHUB_RUN_ID})"*) anchor never matches an ID, and the cross-job/re-run dedup silently stops matching — every re-run of a dead review double-posts, the exact regression this PR exists to prevent.
Suggested fix: have the stub apply the real filter — store the fixture as JSON, extract the --jq argument, and pipe through jq so filter mutants change the stub's output; or add one executed assertion that runs each step's literal --jq string through jq against a fixture.
中文说明
R3-8（承接自第 3 轮，在本 commit 判定为仍然存在）：已执行框架的 stub gh 原样输出夹具、从不应用 --jq 去重过滤器，因此两个步骤的过滤器语义都未被执行——声称 stub"预先应用去重过滤器语义"的注释夸大了覆盖。— 故障场景：已验证存活的变异体——把过滤器投影 | .body 改为 | .id（任一步骤的 run 块）后套件仍绿，因为 stub 忽略 --jq；运行时过滤器将返回评论节点 ID，case "$fallback_bodies" in *"actions/runs/${GITHUB_RUN_ID})"*) 锚点永远匹配不到 ID，跨 job/重跑去重悄悄失效——死掉 review 的每次重跑都会重复发布，正是本 PR 旨在防止的回归。
建议修复：让 stub 应用真实过滤器——以 JSON 存储夹具、提取 --jq 参数并交给 jq 处理，使过滤器变异能改变 stub 输出；或新增一个执行式断言，把每个步骤的字面量 --jq 字符串对夹具用 jq 运行一遍。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| runs-on: 'ubuntu-latest' | ||
| timeout-minutes: 5 |
There was a problem hiding this comment.
[Suggestion] The fallback job's timeout-minutes: 5 is pinned by no assertion, unlike the identical invariant this file already enforces for the capture-tools step (which pins both the value and the worst-case retry budget arithmetically). — Failure scenario: verified surviving mutant — shrinking it below the loop's wall-clock floor (3 × sleep 10 plus up to six gh calls with their own backoff) makes GitHub cancel the job mid-loop on exactly the degraded-API conditions the loop exists to survive — no comment, no fail-closed summary — while the suite stays green (probe: timeout-minutes: 0.4 left it 144/144; adding the pin flipped it red).
Suggested fix: add expect(job['timeout-minutes']).toBe(5); beside the existing runs-on pin (the timeout pin alone is the verifiable guard — the gh calls have no script-level timeout, so a budget formula has no finite inputs to compute from).
中文说明
兜底 job 的 timeout-minutes: 5 没有任何断言钉住，而本文件对 capture-tools 步骤已强制同样的不变量（既钉数值、又以算术钉最坏重试预算）。— 故障场景：已验证存活的变异体——把它缩小到循环墙钟下限（3 × sleep 10 加最多六次自带退避的 gh 调用）以下，GitHub 会在恰是该循环为之存在的 API 降级条件下于循环中途取消 job——无评论、无失败关闭摘要——而套件仍保持绿色（探针：timeout-minutes: 0.4 时仍 144/144；加上该钉住后转红）。
建议修复：在现有 runs-on 钉住旁添加 expect(job['timeout-minutes']).toBe(5);（仅超时钉住即为可验证的防线——gh 调用无脚本级超时，预算公式没有可计算的有限输入）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — CI green at 3cad099, approving per maintainer request.
|
Released in v0.21.13. |
…LM#9255) * fix(ci): keep a fallback comment when the PR review runner dies A review job that dies abnormally never reaches its in-job fallback comment step: the runner worker crash in FinalizeJob on the PR QwenLM#8894 run (EACCES creating under the runner home directory) left the PR with no review and no explanation. - Probe write access to $HOME, $RUNNER_TEMP and the runner root at job start, repair single-directory ownership with the existing sudo pattern, and fail fast with a clear message when repair is impossible instead of burning the review budget to die at finalize. - Add a fallback-comment job on an ephemeral hosted runner that posts the retry guidance whenever review-pr fails. It derives the PR number from the event payload (dead job outputs do not survive a crash) and dedupes on a qwen-review-fallback comment marker plus this run's URL, so the in-job step, the ack comment, and re-runs never double-post. * fix(ci): harden the PR review fallback comment (QwenLM#9255) Review-round fixes for the fallback-comment defenses: - Probe the actual runner root (three levels above the workspace, not two) and the _diag subdirectory FinalizeJob writes in; a writable parent does not prove an existing subdirectory writable. - Open the fallback gate on authorize/review-config failures too — the incident's trigger can kill those earlier self-hosted jobs first, and a failed dependency marks review-pr 'skipped', which the old gate never matched. Guarded against resolve dispatch runs, which skip review-pr by design. - Author-scope the dedup lookup (resolved dynamically like upsert-bot-comment.sh) so a planted marker cannot suppress the fallback, and fail closed with bounded retry when the lookup or the state check fails instead of fail-open toward duplicates or a green job that never posted. - Skip the stale fallback when the PR head moved, but only on pull_request_target events where the run head is comparable — comment/review runs report main's tip, and posting wins over silence when the comparison is unavailable. - Define the marker once in a workflow-level env and pin all of the above in the workflow test suite, executing the fallback step's real bash against a stubbed gh. * fix(ci): close the fallback-comment gate gaps from round-2 review (QwenLM#9255) - Exclude comment-driven /resolve runs from the fallback gate: authorize runs on `@qwen-code /resolve` issue comments where github.event.inputs is empty, so the dispatch-only exclusion never fired there and a failed resolve run was misdiagnosed as a dead review recommending the wrong command. - Enumerate precheck-pr and delay-automatic-review failures in the gate: either failure marks review-pr 'skipped' (a transient API 5xx in delay's re-check step, or the fork-PR chain root dying before it posts anything), which the old gate never matched — silence, against its own "a skipped review is as unexplained as a dead one" norm. Both are 'skipped' where they do not apply, so the gate stays closed there. - Anchor the cross-job dedup on the run URL's closing paren: run ids grow digits over time, so the unanchored substring let a later run's fallback comment (id 123450) suppress an earlier run's (id 12345) re-run comment; every marker body renders the URL as [workflow logs](...runs/<id>), so the id is always followed by ')'. - Pin the previously surviving mutants in the workflow suite: the _diag probe guard polarity, the workflow_dispatch disjunction, the ephemeral-hosted-runner placement, and the fallback body's marker-link shape the anchor relies on; add executed coverage for each head-lookup partial failure and for a distinct run's fallback not suppressing this run's comment. * fix(ci): close round-3 review gaps in the fallback-comment defenses (QwenLM#9255) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
… the failure comment (QwenLM#10114) The fallback-comment gate admits a cancelled review-pr on purpose: a job-level timeout is auto-CANCELLED by GitHub (failure() false), which opens neither the failure-only gate nor the in-job step, so silence there would leave a timed-out review unexplained (QwenLM#9255). But the same 'cancelled' result also arrives when a run or job is cancelled mid-review after the upstream chain finished, and that flavor got the full failure body — "The review pipeline failed before a review could be posted. A transient error is retried automatically…" — none of which is true for a cancellation. On PR QwenLM#9729, run 32875478404 was run-cancelled two minutes into the review with no successor run, so the QwenLM#9716 supersede guard correctly did not match, and the comment read as a pipeline outage to the PR author. The two flavors are not separable in needs — both reach the gate as review-pr 'cancelled' with upstream green — so the fix branches inside the step on the wired-in needs result: a cancelled review-pr now posts one body accurate for both flavors (no failure/auto-retry claims, retry instruction kept for the timeout flavor, run-URL markdown link kept for the cross-job dedup), and everything else keeps the failure body. The step runs under set -u, so a dropped env wiring fails the step loudly instead of silently reverting cancelled runs to the false body. Mutation-verified: neutralizing the cancelled branch fails both new tests; the restored workflow passes the suite at base parity. Fixes QwenLM#10109
4 participants
What this PR does
This PR makes the PR-review workflow resilient to a class of failures where the review job dies abnormally and the PR is left with no review and no explanation. It adds two defenses: a preflight health probe at job start verifies that the runner user can still create files in its own directories, attempts a narrowly scoped ownership repair using the same passwordless-sudo pattern already established in the workflow, and fails fast with a clear diagnostic when repair is impossible; and a new lightweight dependent job on an ephemeral hosted runner posts the usual retry-guidance fallback comment whenever the review job fails, deriving the PR number from the event payload (a dead job's outputs do not survive a crash) and skipping when a fallback comment for the same run already exists. To make that dedup reliable, fallback comments now carry a hidden HTML marker, following the convention the acknowledgement comment already uses.
Why it's needed
The review run for PR #8894 (job 95028756485) died when the self-hosted runner worker crashed in its job-finalization phase with a permission-denied error creating files under the runner user's home directory. Because the crash happened outside the normal step flow, none of the remaining steps ever ran: no fallback comment was posted, no worktree cleanup happened, and the PR sat silently with no review and no indication of what went wrong. The known trigger on the shared self-hosted pool is a prior containerised job running as root leaving root-owned or mode-broken directories behind — the workflow already documents and defends against this pattern for the workspace, but not for the runner's own directories. Even when the root cause cannot be prevented (for example, corruption that happens mid-run), the PR should still get an explanatory comment, and a job that starts on an already-broken runner should fail in seconds with a clear message instead of burning hours of review budget.
Reviewer Test Plan
How to verify
The two new pieces of shell were extracted from the workflow and exercised locally on macOS. The health probe was run against a directory made read-only: it detected the missing write access, attempted repair, and exited non-zero with a clear error when repair was unavailable. The fallback job's script was run against a mock
ghacross four comment scenarios: an acknowledgement comment carrying the run URL but not the marker (must not suppress posting), a fallback comment with the marker and the same run URL (must suppress), a fallback comment with the marker and a different run URL (must not suppress), and no comments at all (must post). All four behaved as designed. The full workflow passes yamllint at the same version CI uses, actionlint, and a plain YAML parse. End to end, the fallback job fires on any failed review run once this merges; the marker plus run-URL dedup is what keeps ordinary failures (where the in-job fallback step already posted) and re-runs quiet. A natural first live check is the next review failure: the PR should receive exactly one explanatory comment.Evidence (Before & After)
N/A (workflow change, no user-visible UI)
Tested on
Environment (optional)
Local simulation of the embedded scripts with mocked
gh; yamllint 1.35.1 (same as CI), actionlint, PyYAML parse. No live end-to-end run yet — the real path activates on the next failed review run after merge.Risk & Scope
Linked Issues
Incident reference: PR #8894 review run 31891414261 (no auto-close).
中文说明
这个 PR 做了什么
本 PR 让 PR review workflow 对一类故障具备容忍能力：review job 异常死亡时,PR 不再落得"既没有 review、也没有任何解释"的状态。具体加了两种防御:一是在 job 启动时做前置健康探测,验证 runner 用户仍能在自己的目录中创建文件,发现不可写时先用 workflow 中既有的免密 sudo 模式做范围受限的单目录属主修复,修不好就带着清晰的诊断信息快速失败;二是新增一个跑在临时托管 runner 上的轻量依赖 job,只要 review job 失败就由它兜底发布常规的重试指引评论——PR 号从事件载荷推导(死掉的 job 的 outputs 无法幸存),并且当同一 run 的兜底评论已存在时跳过。为了让去重可靠,兜底评论现在带一个隐藏的 HTML 标记,沿用确认评论已在使用的惯例。
为什么需要
PR #8894 的 review 运行(job 95028756485)死于 self-hosted runner worker 在 job 收尾阶段的崩溃:在 runner 用户 home 目录下创建文件时权限被拒。由于崩溃发生在正常步骤流之外,后续步骤一个都没跑:兜底评论没发、worktree 清理没做,PR 悄无声息地搁置着,没有 review 也没有任何说明。共享 self-hosted 池上的已知诱因是先前以 root 运行的容器化 job 留下 root 属主或权限损坏的目录——workflow 对 workspace 已有文档和防御,但对 runner 自身目录没有。即便根因无法预防(比如运行中途发生的损坏),PR 也应该收到一条解释性评论;而落在已损坏 runner 上的 job 应该在几秒内带着清晰信息失败,而不是烧掉几小时的 review 预算后才死掉。
Reviewer 测试计划
如何验证
两段新增 shell 已从 workflow 中提取出来,在 macOS 上本地演练:健康探测对着一个只读目录运行——检测到不可写、尝试修复、修复不可用时以清晰报错非零退出。兜底 job 脚本用 mock
gh跑了四种评论场景:只带 run URL 但不带标记的确认评论(不得抑制发布)、带标记且同 run URL 的兜底评论(必须抑制)、带标记但 run URL 不同的兜底评论(不得抑制)、完全没有评论(必须发布),四种行为均符合设计。整个 workflow 通过与 CI 相同版本的 yamllint、actionlint 以及 YAML 解析检查。端到端地,合并后任何一次失败的 review 运行都会触发兜底 job;"标记 + run URL"去重保证普通失败(job 内兜底步骤已发过)和重跑保持安静。第一个自然的线上验证是下一次 review 失败:PR 应当恰好收到一条解释性评论。证据(Before & After)
N/A(workflow 改动,无用户可见 UI)
测试平台
环境(可选)
用 mock
gh对内嵌脚本做本地模拟;yamllint 1.35.1(与 CI 相同)、actionlint、PyYAML 解析。尚无真实端到端运行——真实路径在合并后的下一次 review 失败时激活。风险与范围
关联 Issue
事故引用:PR #8894 的 review 运行 31891414261(不自动关闭)。