fix(ci): stop the fallback comment from denying a review it already posted - #9462
Conversation
…osted The review job can fail AFTER posting its review — the CLI exiting silently, a cleanup step dying — and both fallback sites then announce that review as one that could not be posted, retry instruction attached. Measured on PR #9342: the review posted at 11:56:34Z, review-pr failed at 12:00:53Z ("Qwen review completed but produced no output"), and the comment landed at 12:01:00Z saying the pipeline "failed before a review could be posted. … retry with @qwen-code /review" — a fresh ~3-hour review, asked for beside the review that had just landed. The autofix takeover loop reads the same feed a human does. Both sites now check, before composing a body, whether a review this run posted is already on the PR. The check is scoped three ways so a stale review can never buy silence on a genuinely dead pipeline: the bot's own account, the head this run reviewed, and a submission at or after this run started. Where the proof is unavailable — no start time, no head, a failed listing — the guard declines to fire and the comment posts, the same call the head-moved guard already makes. The job-level step now reads state and headRefOid in one `gh pr view` (the in-job step already did), which is where its head value comes from. Tests run the steps' real bash over review fixtures, because the guard IS a filter: silence when this run posted the review, and posting for each near-miss on its own — an earlier run's review at the same head, another account's, one of a different head, a PENDING one, none at all, an unavailable start time, and a failed reviews listing. One existing assertion tightened: "no `gh run view`" was the proxy for "no head comparison on comment runs", and the new guard asks that same command for startedAt on every event, so it now pins the head lookups themselves. The stub's state,headRefOid branch learned the pr_closed scenario its state-only sibling already knew.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@wenshao Thanks for the PR — the incident write-up (timestamps from #9342, run link, the autofix-loop consequence) is exactly the kind of evidence a reviewer wants. Before this can move to code review, the PR body is missing two required sections from the PR template:
## Risk & Scope— main risk or tradeoff, what is not validated / out of scope, breaking changes. Much of this is already argued in the prose (e.g. the deliberate decision not to touch the failure classification, the fail-open direction of the guard), so filling the section should just be a matter of lifting those sentences into it.## Linked Issues— no closing keyword is needed here, but the related work (#9342, the #8894 incident class this test suite is named after) belongs in this section rather than only inline in the prose.
Everything else checks out: What / Why / Reviewer Test Plan (How to verify, Evidence, Tested on) and the bilingual summary are all present. Once the two sections are added, re-trigger with @qwen-code /triage and the gate will continue from here.
中文说明
@wenshao 感谢提交——事故描述（#9342 的时间线、run 链接、autofix 循环被误导的后果）正是 reviewer 需要的证据。但在进入代码审查之前，PR 正文缺少模板中的两个必填章节：
## Risk & Scope（风险与范围）——主要风险或权衡、未验证/超出范围的部分、破坏性变更。正文中其实已经论述了大部分内容（例如刻意不动底层失败分类、守卫在证据不可得时选择照常发帖），补这一节基本只是把相关句子挪进来。## Linked Issues（关联 Issue）——这里不需要自动关闭关键字，但相关工作（#9342、本测试套件命名所来自的 #8894 事故类别）应放在这个章节里，而不仅散落在正文中。
其余部分都完整：What / Why / Reviewer Test Plan（验证方式、证据、测试平台）以及中文总结均已提供。补上这两个章节后使用 @qwen-code /triage 重新触发，审查会从这一步继续。
— Qwen Code · qwen3.8-max
|
@qwen-code /triage Both sections are added: 两节已补齐： |
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Re-run against
Moving on to code review. 🔍 中文说明针对
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no blockers. My independent take before reading the diff: attribute "did this run already post a review?" via the reviews API, scoped to the bot's own account and a submission window opening at the run's start, fail-open whenever the evidence is unavailable. The PR matches that baseline and improves on it in the one place my proposal was naive: the bot account is shared — release finalize, triage finalize, and the triage skill all approve under it — so matching any in-window bot review would let a foreign LGTM buy silence for a genuinely dead run. The guard instead matches positively on what only this pipeline's composed reviews carry (the What I verified against
Test evidence — the PR's own CI, read via the API (per policy this review never executes PR code). On CI on the reviewed commit:
Not verified here: live GitHub API behavior — the guard's Real-scenario (tmux): N/A — CI-only change with no user-visible surface. The observable behavior (the fallback comment appearing or not) is asserted directly by the suite above. 中文说明代码审查——无阻塞项。 读 diff 前的独立方案：通过 reviews API 判定「本次运行是否已发布过 review」，范围限定为 bot 自身账号、以运行开始为起点的提交窗口，证据不可得时放行（照发）。PR 与该基线一致，并在我的方案天真之处做了改进：bot 账号是共享的——release finalize、triage finalize、triage skill 都以它发 approval——因此匹配窗口内任意 bot review 会让外来 LGTM 为真正死掉的运行买到静默。守卫改为正向匹配只有本流水线组成的 review 才携带的东西（ 针对
测试证据——来自 PR 自己的 CI，经 API 读取（按规约本审查不执行 PR 代码）。 真实场景（tmux）：N/A——纯 CI 改动，无用户可见面。可观测行为（兜底评论出现与否）由上述套件直接断言。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a minimal, well-evidenced fix that matches the approach I'd have proposed independently and improves on it where my proposal was naive; the one reservation is the live-API behavior no offline test can cover, which is inherent to the change (and has a verify run in flight) rather than doubt about it. Stepping back: this is the incident class this pipeline keeps paying down — prose asserting the opposite of the evidence beside it — and the PR measured its incident precisely (timestamps, run link, the autofix-loop consequence) instead of asserting it abstractly. Ten review rounds plus three takeover rounds is a long road, but the convergence is real: the final round on this head found nothing, the leftover Suggestions are explicitly deferred to a follow-up, and every load-bearing decision in the shipped design — attribute by time, not head; anchor on Actions taken in this pass, transparently:
中文说明置信度：4/5 —— 一个最小且证据充分的修复，与我独立提出的方案一致，并在我的方案天真之处做了改进；唯一的保留是任何离线测试都无法覆盖的真实 API 行为——这是改动本身的属性（且已有 verify 运行在途），而非对它的怀疑。 退后一步看：这正是本流水线反复偿还的事故类别——散文断言与其旁边的证据相反——而本 PR 精确度量了它的事故（时间戳、run 链接、autofix 循环后果），而非抽象断言。十轮评审加三轮接管是一段长路，但收敛是真实的：该 head 上的最后一轮未发现任何问题，遗留的 Suggestions 已显式延期至后续跟进，且最终设计中每个承重决定——按时间而非 head 归因；锚定 本次执行的操作，透明列出：
— Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
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 — stopped before round 4 by the review time budget.
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.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未检查（工具限制，非阻断）：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.14)
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | ||
| if [ -n "$run_started" ] && [ -n "$current_head" ]; then |
There was a problem hiding this comment.
[Critical] The time anchor of this guard resets on job re-runs. gh run view --json startedAt returns the LATEST attempt's start, not the run's first start — and re-runs keep the same run id (the dedup comment just above relies on exactly that). So when attempt 1 of this run posts its review and the job fails after the post (the incident shape this PR fixes, measured on PR 9342), a re-run re-anchors startedAt to attempt 2's start: attempt 1's review no longer satisfies select(.submitted_at >= "$run_started"), and if attempt 2 then fails before posting, this step posts 'The review pipeline failed before a review could be posted … retry with @qwen-code /review' directly beneath the review attempt 1 already posted — the exact contradiction this PR exists to eliminate. Measured against this repo's own data:
gh api repos/QwenLM/qwen-code/actions/runs/32219268680
→ created_at 2026-08-19T05:23:57Z, run_attempt 2, run_started_at 2026-08-19T05:51:26Z
attempt-1 jobs created 05:23:57–05:24:10Z; attempt-2 jobs created from 05:51:27Z
re-run 32218596441: created_at 05:13:04Z → run_started_at 05:22:05Z (same reset)
Run-level startedAt equals the re-executed attempt's start while createdAt stays attempt-stable. Note the new stale fixture pins exactly this shape as the intended posting behavior from attempt 2's re-anchored window. Anchor on an attempt-stable timestamp instead — --json createdAt --jq '.createdAt' — at both sites: a review submitted after the run was created still cannot belong to an earlier run, so the stale-review-from-earlier-run protection is preserved. The test stub's *startedAt* dispatch pattern needs a matching update (with RUN_STARTED fixture values re-derived from createdAt).
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | |
| if [ -n "$run_started" ] && [ -n "$current_head" ]; then | |
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json createdAt --jq '.createdAt' 2>/dev/null)" || run_started="" | |
| if [ -n "$run_started" ] && [ -n "$current_head" ]; then |
中文说明
【Critical】 该守卫的时间锚点在作业重跑（re-run）时会被重置。gh run view --json startedAt 返回的是最新一次 attempt 的开始时间，而不是整个 run 最初的开始时间——而重跑保持同一个 run id（上方的 dedup 注释正是依赖这一点）。因此，当本次运行的 attempt 1 已发布 review、作业在发布之后才失败（本 PR 要修的正是这种形态，实测于 PR 9342），重跑会把 startedAt 重新锚定到 attempt 2 的开始时间：attempt 1 的 review 不再满足 select(.submitted_at >= "$run_started")；若 attempt 2 又是在发布前失败，这一步就会在 attempt 1 已发布的 review 正下方发出「The review pipeline failed before a review could be posted … retry with @qwen-code /review」——这正是本 PR 要消除的自相矛盾。已在本仓库数据上实测：
gh api repos/QwenLM/qwen-code/actions/runs/32219268680
→ created_at 2026-08-19T05:23:57Z, run_attempt 2, run_started_at 2026-08-19T05:51:26Z
attempt-1 jobs created 05:23:57–05:24:10Z; attempt-2 jobs created from 05:51:27Z
re-run 32218596441: created_at 05:13:04Z → run_started_at 05:22:05Z（同样的重置）
run 级 startedAt 等于重跑 attempt 的开始时间，而 createdAt 在各 attempt 间保持不变。另请注意：新增的 stale 夹具从 attempt 2 的重锚窗口看，恰好把这种形态钉成了「应当照常发帖」。建议改为锚定 attempt 稳定的时间戳——两处站点均改用 --json createdAt --jq '.createdAt'：review 只要在 run 创建之后提交就不可能属于更早的 run，因此「更早 run 的陈旧 review 不能买到静默」这一保护仍然成立。测试桩里的 *startedAt* 分发模式需要同步更新（RUN_STARTED 夹具值也需改为按 createdAt 取值）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | ||
| if [ -n "$run_started" ] && [ -n "$pr_head" ]; then |
There was a problem hiding this comment.
[Critical] Same defect as the in-job copy: the time anchor resets on job re-runs. gh run view --json startedAt returns the LATEST attempt's start, not the run's first start, and re-runs keep the same run id. When attempt 1 of this run posts its review and fails after the post, a re-run re-anchors startedAt to attempt 2's start; attempt 1's review then fails select(.submitted_at >= "$run_started"), and if attempt 2 fails before posting, this job posts 'The review pipeline failed before a review could be posted … retry with @qwen-code /review' directly beneath the review attempt 1 already posted. Measured against this repo's own data: re-run 32219268680 has attempt-1 jobs created 05:23:57–05:24:10Z but run-level run_started_at reads 05:51:26Z (the re-executed attempt's start, 05:51:27Z); re-run 32218596441 shows the same reset (05:13:04Z → 05:22:05Z), while createdAt stays attempt-stable. Anchor on --json createdAt --jq '.createdAt' instead, at both sites: a review submitted after the run was created still cannot belong to an earlier run, so the stale-review protection is preserved. The test stub's *startedAt* dispatch pattern needs a matching update.
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | |
| if [ -n "$run_started" ] && [ -n "$pr_head" ]; then | |
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json createdAt --jq '.createdAt' 2>/dev/null)" || run_started="" | |
| if [ -n "$run_started" ] && [ -n "$pr_head" ]; then |
中文说明
【Critical】 与 in-job 那份副本相同的缺陷：时间锚点在作业重跑时会被重置。gh run view --json startedAt 返回的是最新一次 attempt 的开始时间而非 run 最初的开始时间，且重跑保持同一个 run id。当本次运行的 attempt 1 已发布 review、作业在发布之后失败时，重跑会把 startedAt 重新锚定到 attempt 2 的开始时间；于是 attempt 1 的 review 不再满足 select(.submitted_at >= "$run_started")；若 attempt 2 在发布前失败，这个作业就会在 attempt 1 已发布的 review 正下方发出「The review pipeline failed before a review could be posted … retry with @qwen-code /review」。已在本仓库数据上实测：re-run 32219268680 的 attempt-1 作业创建于 05:23:57–05:24:10Z，但 run 级 run_started_at 读数为 05:51:26Z（即重跑 attempt 的开始时间 05:51:27Z）；re-run 32218596441 显示同样的重置（05:13:04Z → 05:22:05Z），而 createdAt 在各 attempt 间保持不变。建议两处站点均改为锚定 --json createdAt --jq '.createdAt'：review 只要在 run 创建之后提交就不可能属于更早的 run，陈旧 review 的防护仍然成立。测试桩里的 *startedAt* 分发模式需要同步更新。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | ||
| if [ -n "$run_started" ] && [ -n "$current_head" ]; then | ||
| posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.commit_id == \"$current_head\") | select(.submitted_at >= \"$run_started\") | .id" 2>/dev/null)" || posted_reviews="" |
There was a problem hiding this comment.
[Suggestion] Both lookups this guard relies on swallow every failure (2>/dev/null, || var="") and write nothing anywhere, so a failed guard degrades to the exact bug this PR fixes with zero trace. A transient API 5xx or secondary rate limit hitting gh run view or the reviews listing empties run_started/posted_reviews, the guard declines to fire, and the contradictory fallback comment posts — and the oncall investigating the recurrence finds no log line distinguishing 'the guard ran, nothing matched' from 'the lookup died'. Every adjacent lookup in these same steps announces its failures ('Fallback comment dedup lookup failed; deferring…', '::error::could not verify PR…'). Emit a ::warning:: and/or a step-summary line such as 'Already-posted guard unavailable (lookup failed); proceeding to post' before continuing, at both sites — no behavior change.
中文说明
【Suggestion】 该守卫依赖的两处查询都把失败完全吞掉（2>/dev/null、|| var=""），且不留任何痕迹，因此守卫失效时会悄无声息地退化成本 PR 要修的那个 bug。一次瞬时的 API 5xx 或次级限流命中 gh run view 或 reviews 列表查询时，run_started/posted_reviews 变为空，守卫拒绝触发，自相矛盾的兜底评论照常发出——而事后排查的 oncall 找不到任何日志能区分「守卫执行了但没有匹配」与「查询本身挂了」。同一批步骤里所有相邻查询都会宣告自己的失败（'Fallback comment dedup lookup failed; deferring…'、'::error::could not verify PR…'）。建议在两处站点查询失败时先发一条 ::warning:: 和/或 step-summary（如 'Already-posted guard unavailable (lookup failed); proceeding to post'）再继续——不改变任何行为。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| run_started="$(gh run view "${GITHUB_RUN_ID:?}" --repo "$GITHUB_REPOSITORY" --json startedAt --jq '.startedAt' 2>/dev/null)" || run_started="" | ||
| if [ -n "$run_started" ] && [ -n "$pr_head" ]; then | ||
| posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.commit_id == \"$pr_head\") | select(.submitted_at >= \"$run_started\") | .id" 2>/dev/null)" || posted_reviews="" |
There was a problem hiding this comment.
[Suggestion] Same silent-failure gap as the in-job copy: both lookups swallow every failure (2>/dev/null, || var="") with no trace, so a transient API failure here degrades to the exact bug this PR fixes, with nothing in the logs distinguishing 'no matching review' from 'lookup died' — while every adjacent lookup in this step announces its failures ('::error::fallback comment dedup lookup failed after retries', '::error::could not verify PR…'). Emit a ::warning:: and/or a step-summary line on lookup failure before continuing — no behavior change.
中文说明
【Suggestion】 与 in-job 副本相同的静默失败缺口：两处查询都把失败完全吞掉（2>/dev/null、|| var=""）且不留痕迹，因此这里一次瞬时的 API 失败就会退化成本 PR 要修的那个 bug，日志中没有任何东西能区分「没有匹配的 review」与「查询挂了」——而本步骤里所有相邻查询都会宣告自己的失败（'::error::fallback comment dedup lookup failed after retries'、'::error::could not verify PR…'）。建议查询失败时先发一条 ::warning:: 和/或 step-summary 再继续——不改变任何行为。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| ' *state,headRefOid*)', | ||
| ' [ "${SCENARIO:-}" = "state_fail" ] && exit 1', | ||
| ' printf "OPEN\\t%s\\n" "${PR_HEAD:-}"; exit 0 ;;', | ||
| ' state=OPEN; [ "${SCENARIO:-}" = "pr_closed" ] && state=MERGED', |
There was a problem hiding this comment.
[Suggestion] This migration leaves the stub's *state*) branch unreachable. The fallback job's standalone gh pr view --json state call was that branch's only consumer, and the new --json state,headRefOid form matches the earlier *state,headRefOid*) branch in case order. I parsed the workflow and enumerated every gh call in both extracted step scripts: zero standalone --json state queries remain, so nothing can reach *state*) — its state_fail/pr_closed handling now exists in two branches, and this diff itself demonstrates the trap: pr_closed had to be migrated into the live branch because the dead one no longer sees the call. A future edit adding a scenario to the dead branch would be silently a no-op, and a reader cannot tell which branch is authoritative. Delete the dead *state*) branch (the three lines right after the *headRefOid*) branch).
中文说明
【Suggestion】 这次迁移让桩里的 *state*) 分支变得不可达。兜底作业原先独立的 gh pr view --json state 调用是该分支唯一的消费者，而新的 --json state,headRefOid 形式按 case 顺序会先命中前面的 *state,headRefOid*) 分支。我解析了 workflow 并枚举两个被抽取步骤脚本里的所有 gh 调用：独立的 --json state 查询已经为零，没有任何调用能到达 *state*)——它的 state_fail/pr_closed 处理现在存在于两个分支里，而本 diff 恰好演示了这个陷阱：pr_closed 不得不迁到活分支，因为死分支已经看不到该调用了。未来若有人往死分支里加场景，会悄无声息地变成空操作，读者也无法判断哪个分支才是权威的。建议删除死掉的 *state*) 分支（紧跟在 *headRefOid*) 分支之后的三行）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // review-pr failed 12:00:53Z, the comment landed 12:01:00Z asking for a | ||
| // fresh ~3-hour review; the autofix takeover loop reads the same feed a | ||
| // human does. The guard is a FILTER (author scope, head, submission | ||
| // time), so these run the step's real bash over review fixtures. |
There was a problem hiding this comment.
[Suggestion] These tests run the extracted in-job script verbatim, but bash chokes on MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}" (workflow line 1776): the assignment is skipped with a bad substitution error and MAX_TIMEOUT_MINUTES stays unset, so for that line the tests do not actually run 'the step's real bash' as this comment claims. Measured: the full suite prints exactly 8 bash: line 52: …: bad substitution lines — this diff's new tests add 7 of them (the eighth new test exits at the guard before reaching the line; the fallback-job block contains no ${{ }}). Every suite run now normalizes eight lines of bash error output mixed with any real failure's diagnostics, and any future in-job test of the FAILURE_KIND=timeout body — the only branch that reads this variable — evaluates [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ] with the variable unset, errors, and falls into the wrong branch, so a timeout-variant test would pass or fail for the wrong reason. Substitute the expression before executing, e.g. in runFallbackStep: (useInJobStep ? inJobStep.run : step.run).replaceAll('${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}', '<repo default>') — using the repo's real default so future timeout-variant tests are meaningful. Verified: the fix flips 8 errors → 0 with the suite still green.
中文说明
【Suggestion】 这些测试原样执行抽取出的 in-job 脚本，但 bash 会在 MAX_TIMEOUT_MINUTES="${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}"（workflow 第 1776 行）上报 bad substitution 错误并跳过该赋值，MAX_TIMEOUT_MINUTES 保持未设置——因此对这一行而言，测试并未像此注释所宣称的那样运行「步骤的真实 bash」。实测：完整套件恰好打印 8 行 bash: line 52: …: bad substitution——本 diff 的新测试贡献了其中 7 行（第 8 个新测试在到达该行前就已在守卫处退出；兜底作业的代码块不含任何 ${{ }}）。现在每次运行套件都会把 8 行 bash 错误输出当作常态混入 stderr，与真实失败的诊断信息混在一起；而未来任何针对 FAILURE_KIND=timeout 分支（唯一读取该变量的分支）的 in-job 测试，都会在变量未设置的情况下执行 [ "$TIMEOUT_MINUTES" -lt "$MAX_TIMEOUT_MINUTES" ]，报错后落入错误分支——timeout 变体测试会因为错误的原因通过或失败。建议在执行前先替换该表达式，例如在 runFallbackStep 中：(useInJobStep ? inJobStep.run : step.run).replaceAll('${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }}', '<repo default>')——用仓库的真实默认值，让未来的 timeout 变体测试有意义。已验证：该修复把 8 个错误降为 0，套件仍然全绿。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…ay when it cannot run
Round 1's two blockers, both re-verified against this repo's own run data.
The time anchor reset on job re-runs. `gh run view --json startedAt`
returns the LATEST attempt's start while the run id stays the same — the
dedup above relies on that stability — so a re-run pushed attempt 1's
review outside "this run": runs 32219268680 (created 05:23:57Z,
startedAt 05:51:26Z) and 32218596441 (05:13:04Z → 05:22:05Z) both show
the ~9-28 minute shift. Attempt 1 posts its review, the job fails after
the post, someone re-runs it, attempt 2 fails before posting — and the
guard, anchored on attempt 2's start, lets the contradictory comment
through. Exactly the shape this PR exists to stop, on the path most
likely to reach it. Both sites now anchor on `createdAt`, which is
attempt-stable; a review submitted after the run was created still
cannot belong to an earlier run, so the stale-review protection is
unchanged.
The guard also swallowed its own lookup failures. A transient failure
in either call emptied the value, the guard declined, and the false
comment posted with nothing in the log separating "the guard ran,
nothing matched" from "the lookup died" — while every sibling lookup in
these steps announces its failures. Both unavailable paths now emit a
`::warning::` and a step-summary line before posting. No behavior
change: posting was, and remains, the fail-open direction.
Tests: a re-run fixture per site, where the stub answers `createdAt` and
`startedAt` with DIFFERENT values and attempt 1's review sits between
them — reverting either site to `startedAt` fails exactly these two; and
a per-site assertion that both unavailable paths announce themselves.
Also from round 1, both verified before taking: the stub's standalone
`*state*)` branch is dead (no `--json state` call remains in either
extracted step) and is removed, so its scenarios cannot be edited into a
no-op; and the harness now substitutes `${{ vars.* }}` before running
the in-job script, which bash rejected as a bad substitution — the
assignment was skipped, `MAX_TIMEOUT_MINUTES` stayed unset, and eight
error lines rode every suite run, so "the step's real bash" was not
quite true for that line.
|
All six findings from round 1 are taken in f4a9053 — nothing deferred. The two blockers were right, and I re-derived the evidence before changing anything. The silent-failure Suggestion is the same defect class this PR is about, so it was not a nit: both unavailable paths now emit The two test-hygiene Suggestions were verified before taking. I enumerated every Suite: 169 pass in this file (the one failure, 中文说明第 1 轮六条全部采纳（f4a9053472），无延后。 两条阻断项属实，我在动手前独立复核了证据： 「静默失败」那条与本 PR 要修的是同一类缺陷，因此不算小节：两条不可用路径现在都会先输出 两条测试卫生项也先核后改：我枚举了两个抽出脚本里所有 测试：本文件 169 通过（唯一失败 |
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: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only.
Not explored to full depth (tool budget reached): "agent 2": none — no check was cut short.; "agent 4": none — no check was cut short..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
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.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-pr-review-workflow.test.js:3459 — [review] new test comment says the guard asks gh run view for startedAt, but the guard queries createdAtscripts/tests/qwen-pr-review-workflow.test.js:3285 — [review] reviews stub runs the host's jq; on a jq-less host the silence tests go red and the 'still posts' filter cases pass vacuouslyscripts/tests/qwen-pr-review-workflow.test.js:3285 — [review] --paginate is unpinned; deleting it from the guard survives the suite while production reads only page 1scripts/tests/qwen-pr-review-workflow.test.js:3281 — [review] gh api stub dispatches on the repos/ prefix alone; endpoint drift escapes the suite
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only。
未探索到全部深度（达到工具调用预算）："agent 2"：none — no check was cut short.；"agent 4"：none — no check was cut short.。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # head this run reviewed, and a submission at or after this run | ||
| # started, so a stale review from an earlier run cannot silence a | ||
| # genuinely dead one; an unavailable start time or head declines to | ||
| # fire and posts. |
There was a problem hiding this comment.
[Suggestion] The opening paragraph of this guard comment still describes the round-1 startedAt anchor — "a submission at or after this run started", "an unavailable start time" — but the code below queries createdAt, and the very next paragraph says "The anchor is the run's CREATION, never its start" (the runtime warning likewise announces "no run creation time"). A maintainer editing this guard gets two contradictory anchors from the same block; trusting the first paragraph, they would conclude job re-runs break the guard — precisely the round-1 defect this revision removed. Suggest replacing "started" with "was created" and "start time" with "creation time" in the opening paragraph, here and in the fallback-comment job's twin.
| # head this run reviewed, and a submission at or after this run | |
| # started, so a stale review from an earlier run cannot silence a | |
| # genuinely dead one; an unavailable start time or head declines to | |
| # fire and posts. | |
| # head this run reviewed, and a submission at or after this run | |
| # was created, so a stale review from an earlier run cannot silence a | |
| # genuinely dead one; an unavailable creation time or head declines to | |
| # fire and posts. |
中文说明
【Suggestion】 该守卫注释的首段仍在描述第一轮的 startedAt 锚点——「a submission at or after this run started」「an unavailable start time」——但其下方代码查询的是 createdAt，紧邻的下一段也写明「The anchor is the run's CREATION, never its start」（运行时警告同样提示「no run creation time」）。维护者编辑这段守卫时会从同一个注释块里得到两个互相矛盾的锚点；若相信首段，就会认为作业重跑会破坏守卫——而这正是本轮修订所移除的第一轮缺陷。建议将首段的「started」改为「was created」、「start time」改为「creation time」，此处与 fallback-comment 作业中的孪生副本同步修改。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # reviewed, and a submission at or after this run started. When the | ||
| # start time or the head is unavailable the guard declines to fire — | ||
| # posting wins over silence, the same call the head-moved guard |
There was a problem hiding this comment.
[Suggestion] Same stale-anchor wording as the in-job twin: this opening sentence says "a submission at or after this run started" and "when the start time or the head is unavailable", while the next paragraph states "The anchor is the run's CREATION, never its start" and the code queries createdAt. The same 10-line block gives a maintainer two contradictory anchors for the condition that decides re-run behavior. Suggest "at or after this run was created" / "creation time" here as well.
| # reviewed, and a submission at or after this run started. When the | |
| # start time or the head is unavailable the guard declines to fire — | |
| # posting wins over silence, the same call the head-moved guard | |
| # reviewed, and a submission at or after this run was created. When the | |
| # creation time or the head is unavailable the guard declines to fire — | |
| # posting wins over silence, the same call the head-moved guard |
中文说明
【Suggestion】 与 in-job 那份副本相同的陈旧锚点措辞：首句写「a submission at or after this run started」「when the start time or the head is unavailable」，而下一段写明「The anchor is the run's CREATION, never its start」，代码查询的也是 createdAt。同一个 10 行注释块为决定重跑行为的条件给出了两个互相矛盾的锚点。此处同样建议改为「at or after this run was created」/「creation time」。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| elif [ -n "$posted_reviews" ]; then | ||
| echo "Skipping fallback comment: this run already posted a review on ${current_head}." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
[Suggestion] This skip message claims "this run already posted a review", but the guard's window (bot account + head + submitted_at >= createdAt) also matches a SIBLING run's review — reviews carry no run id, so the guard cannot distinguish this run's post from another run's, and the workflow's own concurrency comment acknowledges overlapping same-head runs ("an occasional duplicate review"). The suppression itself is correct — posting the fallback body would contradict the review sitting above it; only the attribution is wrong: an oncall investigating a silent dead run reads in this step's summary that THIS run posted when it never did. Suggest rewording to what the guard actually proved, here and in the fallback-comment job's twin.
| elif [ -n "$posted_reviews" ]; then | |
| echo "Skipping fallback comment: this run already posted a review on ${current_head}." >> "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi | |
| elif [ -n "$posted_reviews" ]; then | |
| echo "Skipping fallback comment: a bot review already exists on ${current_head} submitted at or after this run was created." >> "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi |
中文说明
【Suggestion】 这条跳过消息声称「this run already posted a review」，但守卫的窗口（bot 账号 + head + submitted_at >= createdAt）同样会匹配兄弟运行发布的 review——review 上不携带 run id，守卫无法区分本次运行的发布与另一次运行的发布，而本 workflow 自己的并发注释也承认同 head 运行会重叠（「an occasional duplicate review」）。静默本身是正确的——此时再发兜底正文会与上方的 review 自相矛盾；只是归属错了：值班排查一次静默的死运行，会在这一步的 summary 里读到「本次运行已发布过 review」，而它从未发布。建议把措辞改为守卫实际证明的事实，此处与 fallback-comment 作业中的孪生副本同步修改。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.commit_id == \"$pr_head\") | select(.submitted_at >= \"$run_created\") | .id" 2>/dev/null)"; then |
There was a problem hiding this comment.
[Suggestion] In this job the guard compares review commit_id against pr_head fetched fresh at fallback time, not the head this run reviewed. On every event except pull_request_target — where the head-moved guard above deliberately does not run — a push landing after the review was posted makes select(.commit_id == "$pr_head") match nothing, and the contradictory comment posts anyway. Executed through this repo's own harness (the step's real bash against a stub gh):
arm1 (fresh head == reviewed head): guard fires, posted '' — summary "already posted a review"
arm2 (push landed, pr_head=B, review on A): posted "<!-- qwen-review-fallback --> … failed before a review could be posted … retry with `@qwen-code /review`"
— the exact #9342 shape this guard exists to stop, re-opened for the non-PRT trigger + post + push + fail-after-post interleaving. The in-job twin is immune (its unconditional EXPECTED_HEAD_SHA check exits first), but this job has no record of the reviewed head — review-pr declares no job outputs. Suggest exposing expected_head_sha as a review-pr job output (job outputs survive job failure; empty only when the review step never ran, i.e. nothing was posted) and preferring it over the fresh head in the filter when non-empty — or correcting the comment's "the head this run reviewed" claim and documenting the residual.
中文说明
【Suggestion】 在这个作业里，守卫把 review 的 commit_id 与兜底时刻新取的 pr_head 比较，而不是本次运行实际所审的 head。在除 pull_request_target 之外的所有事件上——上方的 head-moved 守卫在这些事件上刻意不运行——review 发布之后若有 push 落地，select(.commit_id == "$pr_head") 就什么都匹配不到，自相矛盾的评论照样发出。已用本仓库自己的测试装置执行验证（步骤的真实 bash + 桩 gh``）：arm1（新 head == 所审 head）守卫触发、保持静默；arm2（push 落地，pr_head=B 而 review 的 commit_id=A）步骤发出了「failed before a review could be posted … retry with @qwen-code /review」——正是本守卫要消除的 #9342 形态，在「非 PRT 触发 + 已发布 + push + 发布后失败」这一子交错下被重新打开。in-job 那份副本不受影响（其无条件的 EXPECTED_HEAD_SHA 检查先行退出），但本作业无所审 head 的任何记录——review-pr没有声明任何 job outputs。建议把expected_head_sha暴露为review-pr` 的 job output（job output 在作业失败后仍然存在；仅当 review 步骤从未运行——即从未发布——时才为空），并在过滤器中非空时优先于新取的 head；或者修正注释里「the head this run reviewed」的说法并明确记录这一残留窗口。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| ' *createdAt*)', | ||
| ' [ "${SCENARIO:-}" = "runstart_fail" ] && exit 1', | ||
| ' printf "%s" "${RUN_CREATED:-}"; exit 0 ;;', |
There was a problem hiding this comment.
[Suggestion] The stub's case "$*" branches match substrings in order, so any gh run view query whose arguments contain createdAt — including a combined --json createdAt,startedAt --jq '.startedAt' — is answered from this *createdAt*) branch with RUN_CREATED, and the *startedAt*) discriminator below is unreachable for it. Mutation-tested against the suite: switching both guard sites to --json createdAt,startedAt --jq '.startedAt' leaves survives a job re-run green (the stub's $* contains createdAt, so the guard is handed RUN_CREATED 09:08:38Z and MID_RERUN 10:00:00Z compares ≥ it), while in production run_created would hold the re-executed attempt's start — the round-1 regression. The wholesale startedAt swap IS killed by the suite, so only this partial variant escapes; note the combined-field idiom is established in this very diff (--json state,headRefOid), which makes the shape a plausible future edit. Suggest answering run view the way the stub already answers the reviews lookup — run the caller's real --jq over a JSON object holding both fields ({"createdAt": "$RUN_CREATED", "startedAt": "$RUN_STARTED_ATTEMPT"}), so any query shape is answered per-field and the partial mutant is killed.
中文说明
【Suggestion】 桩里的 case "$*" 分支按顺序做子串匹配，因此任何参数中含 createdAt 的 gh run view 查询——包括组合式 --json createdAt,startedAt --jq '.startedAt'——都会命中这个 *createdAt*) 分支并得到 RUN_CREATED，下方的 *startedAt*) 判别分支对它永不可达。已对套件做变异实测：把两处守卫改成 --json createdAt,startedAt --jq '.startedAt' 后 survives a job re-run 仍然通过（桩的 $* 含 createdAt，守卫拿到 RUN_CREATED 09:08:38Z，MID_RERUN 10:00:00Z 比较 ≥ 成立），而生产中 run_created 将持有重跑 attempt 的开始时间——即第一轮回归。整串换成 startedAt 的变异能被套件杀死，只有这种部分变体漏网；注意组合字段写法在本 diff 中已有先例（--json state,headRefOid），这一形态是合理的未来编辑。建议让 run view 采用桩应答 reviews 查询的同一方式——对同时持有两个字段的 JSON 对象（{"createdAt": "$RUN_CREATED", "startedAt": "$RUN_STARTED_ATTEMPT"}）执行调用方自己的 --jq，使任何查询形态都按字段应答，从而杀死该部分变体。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| elif [ -n "$posted_reviews" ]; then | ||
| echo "Skipping fallback comment: this run already posted a review on ${pr_head}." >> "$GITHUB_STEP_SUMMARY" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
[Suggestion] Same attribution wording as the in-job twin: "this run already posted a review" — but the guard's window also matches a sibling run's review (reviews carry no run id), so this step summary can tell the oncall that THIS run posted when the review actually came from an overlapping run. The suppression behavior stays correct; reword the message to what the guard proved.
| elif [ -n "$posted_reviews" ]; then | |
| echo "Skipping fallback comment: this run already posted a review on ${pr_head}." >> "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi | |
| elif [ -n "$posted_reviews" ]; then | |
| echo "Skipping fallback comment: a bot review already exists on ${pr_head} submitted at or after this run was created." >> "$GITHUB_STEP_SUMMARY" | |
| exit 0 | |
| fi |
中文说明
【Suggestion】 与 in-job 那份副本相同的归属措辞：「this run already posted a review」——但守卫的窗口同样会匹配兄弟运行的 review（review 不携带 run id），因此这条 step summary 可能告诉值班「本次运行已发布过 review」，而那条 review 实际来自一次重叠的运行。静默行为本身正确；只需把消息改为守卫实际证明的事实。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…rd proved Round 2's six, all taken. The fallback JOB compared review commit ids against the PR's head at fallback time, not the head the run reviewed. On every trigger but pull_request_target the head-moved guard above deliberately does not run, so a push landing between the post and this step leaves that value pointing at bytes no review ever covered: the match fails and the contradictory comment posts anyway — the #9342 shape, re-opened for the trigger + post + push + fail-after-post interleaving. `review-pr` now publishes the head its review step recorded as a job output, and the guard reads it, falling back to the fresh head only when the job died before that step (a run that posted nothing either). The in-job twin needs none of this — its unconditional head-moved check exits first — and that asymmetry is now pinned per site rather than left to be rediscovered. Both skip messages claimed "this run already posted a review". Reviews carry no run id, so the window (bot account + head + submitted at or after this run was created) also matches an overlapping sibling run's review, which this workflow's own concurrency note says can happen. The suppression is right either way — a review IS sitting above the comment — but the oncall reading the summary was told something the guard never proved; both now say what it did. The guard's opening paragraphs still described the round-1 `startedAt` anchor while the code (and the paragraph below it, and the runtime warning) said creation. A maintainer reading top-down got the anchor that re-runs break — the defect round 1 removed. Test stub: `gh run view` now answers by running the caller's own --jq over an object carrying both timestamps, instead of a `case` on "$*" that matched substrings in order. A combined `--json createdAt,startedAt --jq '.startedAt'` was answered from the createdAt branch, leaving the re-run pin green for a guard reading the attempt-scoped field — the exact regression it exists to catch.
|
Round 2's six are all taken in 12dc118 — nothing deferred. The reviewed-head finding is the one that mattered, and its probe was right. The fallback job had no record of what this run reviewed, so it compared against the PR's head at fallback time; with the head-moved guard deliberately inert on non- The attribution wording was a fair catch on my own prose. Reviews carry no run id, so the window also matches an overlapping sibling run's review — the suppression stays correct (a review IS sitting above the comment), but the summary told the oncall something the guard never proved. Both messages now say what it did prove. Same for the two opening paragraphs still describing the round-1 The stub finding is the sharpest of the four hygiene ones, because it attacks the pin rather than the code: a Suite: 171 pass in this file (the one failure, 中文说明第 2 轮六条全部采纳（12dc11827f），无延后。 「审查时的 head」那条最关键，其探针结论正确：兜底作业没有本轮实际审查了哪个 head 的记录，只能拿兜底时刻的 PR head 比对；而在非 措辞归因那条是对我自己散文的公正指正：review 不携带 run id，所以该时间窗同样会匹配并发同胞运行发布的 review——抑制行为依然正确（评论上方确实有一份 review），但 summary 告诉 oncall 的是守卫从未证明的事。两处消息现在只陈述已证明的内容。两段仍在描述第 1 轮 桩那条是四条卫生项里最锋利的，因为它攻击的是钉本身：对 测试：本文件 171 通过（唯一失败在原始树同样复现）； |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- N3-2 guard lookups swallow stderr (2>/dev/null) with no diagnostic trace — already reported (comment 3811217409, comment 3811217413)
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.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-code-pr-review.yml:1793 — [review] the already-posted guard is duplicated verbatim across the two fallback sites — deferred (code-age rule: anchored code unchanged since round 2)scripts/tests/qwen-pr-review-workflow.test.js:3462 — [review] test comment says the guard queries startedAt but it queries createdAt — round 2 deferred the same finding; code unchanged.github/workflows/qwen-code-pr-review.yml:2019 — [review] guard lookups are single-shot while the sibling dedup loop retries 3× — deferred (code-age rule: anchored code unchanged since round 2)
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # harness, that interleaving reproduces the #9342 shape exactly. | ||
| # The job output is empty only when review-pr died before its review | ||
| # step ran, which is also a run that posted nothing. | ||
| guard_head="${REVIEWED_HEAD_SHA:-$pr_head}" |
There was a problem hiding this comment.
[Critical] The already-posted guard attributes a review to this run through the head the LAST attempt reviewed, so after a "Re-run failed jobs" the run's own attempt-1 review becomes invisible to the guard — and the fallback posts the contradictory "failed before a review could be posted … retry" comment directly beneath the review the same run already posted. Two demonstrated entrances: (1) attempt 1 posts its review on head H1 and the job fails after the post; the re-run keeps the run id; attempt 2 dies before the review step writes expected_head_sha (the runner-health fail-fast this workflow exists for, a checkout failure, or the step's own fail "Failed to determine state") — the re-executed job recomputes the output from attempt 2's empty step state, so REVIEWED_HEAD_SHA is '' and guard_head falls back to the (possibly moved) pr_head; (2) a push lands and attempt 2's review step re-runs, writing expected_head_sha=H2 (the new head), before the CLI dies — the same miss with the output populated. In both, .commit_id == <last-attempt head> misses attempt 1's review on H1 even though its submitted_at passes the attempt-stable createdAt window, and the in-job twin has the same hole via an empty EXPECTED_HEAD_SHA skipping its head-moved check. Probe against the steps' real bash (stub gh, the step's own --jq), with an attempt-1 bot review on H1 submitted after run creation and the head moved to H2:
fallback job (REVIEWED_HEAD_SHA=''): posted "<!-- qwen-review-fallback -->… failed before a review could be posted"
control (head unmoved): posted '' — "a bot review already exists on H1OLDSHA"
populated arm (head = H2): posted=YES in both twins
with the commit_id clause dropped: posted=no — "Skipping fallback comment: a bot review already exists"
The re-run premise is measured on this repo (run 32219268680: a previously-succeeded dependent job was re-executed by "Re-run failed jobs"). The createdAt anchor closed the time axis of the round-1 blockers; the head axis still breaks, and the comment blocks at lines 415-420 and 2029-2034 state the false inference ("a run that never got that far never posted a review either" — attempt 2 dying early does not mean the RUN posted nothing). Attribute reviews to the RUN instead of one attempt's head — e.g. drop the select(.commit_id == …) clause from both filters (the bot-account clause plus submitted_at >= run_created already scopes a review to this run), or collect the reviewed heads of all attempts — and correct those two comment blocks.
中文说明
【Critical】 该守卫通过「最后一次 attempt 所审的 head」把 review 归属到本 run，因此「Re-run failed jobs」之后，本 run 自己 attempt 1 发布的 review 会从守卫视野中消失——兜底评论会在同一 run 刚刚发布的 review 正下方发出自相矛盾的「failed before a review could be posted … retry」。两条已证实的入口：(1) attempt 1 在 head H1 上发布 review 后作业才失败；重跑保持同一 run id；attempt 2 在 review 步骤写入 expected_head_sha 之前死掉（本 workflow 专门防御的 runner-health fail-fast、checkout 失败、或步骤自身的 fail "Failed to determine state"）——重跑的作业从 attempt 2 的空步骤状态重新计算 output，REVIEWED_HEAD_SHA 为 ''，guard_head 回落到（可能已移动的）pr_head；(2) 一个 push 落地后 attempt 2 的 review 步骤重跑、写入 expected_head_sha=H2（新 head），随后 CLI 死掉——output 有值但同样错过。两种形态下 .commit_id == <最后 attempt 的 head> 都匹配不到 attempt 1 在 H1 上的 review（尽管其 submitted_at 通过了 attempt 稳定的 createdAt 窗口）；in-job 的孪生守卫同样有洞：EXPECTED_HEAD_SHA 为空时其 head-moved 检查被跳过。以步骤的真实 bash（桩 gh、步骤自己的 --jq）做的探针，attempt-1 bot review 在 H1、提交于 run 创建之后、head 已移至 H2：
fallback job（REVIEWED_HEAD_SHA=''）：posted "<!-- qwen-review-fallback -->… failed before a review could be posted"
对照（head 未移动）： posted '' — "a bot review already exists on H1OLDSHA"
output 有值分支（head = H2）： 两个孪生站点均 posted=YES
去掉 commit_id 子句后： posted=no — "Skipping fallback comment: a bot review already exists"
重跑前提已在本仓库实测（run 32219268680：一个先前已成功的依赖作业被「Re-run failed jobs」重新执行）。createdAt 锚点关闭了第 1 轮两个 blocker 的时间轴；head 轴仍然会断，且 415-420 与 2029-2034 两处注释陈述了错误推断（「走不到那一步的 run 也不可能发过 review」——attempt 2 早死并不意味着该 RUN 没发过）。建议把 review 归属到 RUN 而不是某一个 attempt 的 head——例如从两处过滤器中去掉 select(.commit_id == …) 子句（bot 账号子句加 submitted_at >= run_created 已足以把 review 限定到本 run），或收集所有 attempt 审过的 head——并同步更正上述两处注释。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.commit_id == \"$guard_head\") | select(.submitted_at >= \"$run_created\") | .id" 2>/dev/null)"; then |
There was a problem hiding this comment.
[Suggestion] Both guard comment blocks assert "a review submitted after the run was created still cannot belong to an earlier run", but this workflow's per-run concurrency groups deliberately allow two concurrent runs reviewing the same head (the concurrency comment itself accepts "an occasional duplicate review"). An earlier-created overlapping run's review — bot author, same commit_id, submitted after this run's createdAt — therefore satisfies this run's guard and silences its fallback: lifecycle run A created T0 reviews head X; a maintainer @qwen-code /review comment creates run B at T1 (per-run group — neither run disturbs the other); A submits at T2 > T1; B dies before posting → B's guard matches A's review on all three clauses and exits silent, so the failure of the explicitly-requested run is never announced. The harm is bounded — a fresh bot review of the same head is on the PR, so the suppressed comment would itself have contradicted it — but these comment blocks are the guard's only documentation and are written as load-bearing reasoning (measured run ids and timestamps); a future edit trusting "cannot belong to an earlier run" builds on a guarantee the concurrency model disproves. State the actual guarantee instead: a match proves this head is covered by a bot review submitted while this run was alive, and under per-run concurrency an earlier-created overlapping run's review can also match — accepted because silence then coincides with an existing review of the same head. If this silence ever becomes harmful, the class-closing fix is to embed the run URL/ID in the review body and match on it, exactly like the marker+RUN_URL dedup above.
中文说明
【Suggestion】 两处守卫注释都断言「在 run 创建之后提交的 review 仍不可能属于更早的 run」，但本 workflow 的 per-run 并发组刻意允许两个并发运行审同一个 head（并发注释自己也接受「偶尔的重复 review」）。因此一个创建更早、时间重叠的 run 的 review——bot 账号、相同 commit_id、提交于本 run 的 createdAt 之后——可以满足本 run 的守卫并让其兜底静默：生命周期 run A 于 T0 创建并审 head X；维护者评论触发的 run B 于 T1 创建（per-run 组——互不干扰）；A 于 T2 > T1 提交；B 在发帖前死掉 → B 的守卫三个子句全部匹配 A 的 review 并静默退出，被显式请求的那次运行的失败无人播报。危害有界——PR 上已有同 head 的新鲜 bot review，被压掉的评论本身就会与之矛盾——但这两处注释是该守卫唯一的文档，且以承重推理的口吻写成（附实测 run id 与时间戳）；未来若有人信任「不可能属于更早的 run」，就会在一个被并发模型证伪的保证上做建设。建议改为陈述真实保证：匹配只证明该 head 已被一份在本 run 存活期间提交的 bot review 覆盖，且在 per-run 并发下一个创建更早的重叠 run 的 review 也可能匹配——之所以接受，是因为此时静默与同 head 已有 review 并存。若这种静默将来有害，彻底关闭此类的做法是把 run URL/ID 嵌入 review 正文并按它匹配，与上方 marker+RUN_URL 的 dedup 一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| GH_TOKEN: '${{ secrets.CI_BOT_PAT }}' | ||
| 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 }}' | ||
| REVIEWED_HEAD_SHA: "${{ needs.review-pr.outputs.expected_head_sha || '' }}" |
There was a problem hiding this comment.
[Suggestion] The new cross-job wiring chain — the review-pr job output expected_head_sha feeding this env var — is pinned by no test: the behaviour tests inject REVIEWED_HEAD_SHA directly into the step's environment (bypassing this YAML env: block), and nothing asserts doc.jobs['review-pr'].outputs.expected_head_sha either. Probe: with both wiring hunks (the outputs: declaration and this env line) deleted, the suite still passes 172/172 — blind to the chain in both directions — while the same suite pins same-shaped wiring elsewhere (inJobStep.env.GH_TOKEN, step.env.RUN_URL, job.needs). A future edit that renames or drops either side of the chain ships green while REVIEWED_HEAD_SHA is permanently empty in production; the guard then falls back to pr_head, and on the common issue_comment trigger — where the head-moved guard deliberately does not run — the review-post → push → fail-after-post interleaving matches nothing and re-ships the exact #9342 contradiction this PR closes. A structural pin beside the existing ones would close it:
expect(doc.jobs['review-pr'].outputs?.expected_head_sha).toBe(
'${{ steps.review.outputs.expected_head_sha }}',
);
expect(step.env.REVIEWED_HEAD_SHA).toBe(
"${{ needs.review-pr.outputs.expected_head_sha || '' }}",
);中文说明
【Suggestion】 新的跨作业接线链——review-pr 作业 output expected_head_sha 馈入此环境变量——没有任何测试钉住：行为测试把 REVIEWED_HEAD_SHA 直接注入步骤环境（绕过了这段 YAML env:），也没有任何断言检查 doc.jobs['review-pr'].outputs.expected_head_sha。探针：删掉两处接线 hunk（outputs: 声明与本 env 行）后套件仍 172/172 全绿——对该链两个方向都失明——而同一套件在别处钉住了同形态的接线（inJobStep.env.GH_TOKEN、step.env.RUN_URL、job.needs）。未来若有编辑重命名或删掉链条任一侧，测试全绿的同时生产环境里 REVIEWED_HEAD_SHA 恒为空；守卫回落至 pr_head，而在常见的 issue_comment 触发下（head-moved 守卫刻意不运行），「发布 review → push → 发布后失败」的交错将匹配不到任何 review，从而重新放出本 PR 要关闭的 #9342 式自相矛盾。建议在既有接线断言旁补一个结构性钉住（见上方代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
… run attribute Round 3's blocker, and the second time the head clause re-opened the contradiction this PR exists to close. Two entrances this round, both after a "Re-run failed jobs": attempt 2 dies before the review step writes its head, so the guard falls back to a head attempt 1 never reviewed; or a push lands and attempt 2 records the NEW head — in both, attempt 1's own review no longer matches `.commit_id`, and the fallback posts "failed before a review could be posted … retry" beneath the review the same run had posted. Rather than patch the head lookup a third time, the head clause is gone. What the guard proves is now narrower and stable: a bot review of this PR was submitted while this run was alive — bot account plus the attempt-stable `createdAt` window. That closes both entrances at once and takes the round-2 cross-job wiring with it (review-pr's `expected_head_sha` output and the env line that read it), so there is no untested chain left whose silent breakage would restore the fresh-head comparison. The job-level step no longer needs the PR head either and reverts to its state-only query; the test stub's state-only branch, removed in round 1 as dead, has a caller again. The comment blocks now state the guarantee the concurrency model actually supports. They claimed a review inside the window "cannot belong to an earlier run", but per-run concurrency groups deliberately allow overlapping runs on the same head, so an earlier-created run's review can match and this run's failure then goes unannounced. That is accepted, and said plainly: the silence coincides with a bot review a reader can see — the very state that makes the comment's claim false — while the bot-author and creation-time clauses still rule out silence with no review at all. Tests: the moved-head case flips from "posts" to "silences" and is pinned per site (a review on ANY head inside the window silences); re-introducing a head clause fails exactly that test; and a structural pin asserts the wiring is absent rather than merely unused.
|
Round 3's blocker is fixed in a1380d0, and it deserved a design change rather than a third patch. The head clause was the defect, twice. Round 2 caught it comparing against a head a push had moved; round 3 caught it comparing against a head a re-run recomputed — both entrances ending in the same contradictory comment beneath the review the same run had posted. The head is simply not a stable attribute of a run: pushes move it, re-runs recompute it, and a job that died before its review step never recorded it at all. So the clause is gone. What the guard proves is now narrower and stable — a bot review of this PR was submitted while this run was alive — which closes both of your entrances at once. That also deletes the round-2 cross-job wiring ( The concurrency finding is right and I stated the real guarantee instead of the convenient one. Per-run groups deliberately allow overlapping runs on the same head, so an earlier-created run's review can fall inside this run's window and silence its fallback. Both comment blocks now say so, with the reason it is accepted: the silence coincides with a bot review a reader can see — precisely the state that makes this comment's claim false — while the bot-author and creation-time clauses still rule out silence with no review at all. If that ever becomes harmful, your run-URL-in-the-body idea is the class-closing fix, and it belongs in the composer, not here. Mutation-verified: re-introducing any head clause fails the new attribute-by-time test at both sites. Suite: 171 pass in this file (the one failure, 中文说明第 3 轮的阻断项已在 a1380d0 修复——它值得一次设计调整，而不是第三个补丁。 head 子句本身才是缺陷，而且这是第二次。 第 2 轮抓到它比对的是被推送移动过的 head；第 3 轮抓到它比对的是被重跑重新计算的 head——两条入口通向同一个结果：在同一次运行已发布的 review 下方贴出自相矛盾的评论。head 根本不是运行的稳定属性：推送会移动它、重跑会重算它、在 review 步骤之前就死掉的作业压根没记录过它。所以该子句被移除。守卫现在证明的事更窄也更稳定——本 PR 上有一份 bot review 在本次运行存活期间提交——这一次性关掉了你给出的两条入口。 这同时删掉了第 2 轮引入的跨 job 接线（ 并发那条说得对，我把真实保证写了进去，而不是那个方便的说法。 per-run 并发组本就允许同 head 的重叠运行，因此更早创建的运行发布的 review 可能落入本次运行的时间窗并使其兜底静默。两处注释现在都如实写明，并给出接受它的理由：此时的静默与「读者能看到的一份 bot review」同时成立——而那正是使本评论断言为假的状态；同时 bot 账号与创建时间两个子句仍然排除「完全没有 review 的静默」。若将来这种静默造成危害，你提出的「在 review 正文内嵌 run URL」是彻底关闭该类问题的修法，那属于 composer，不属于这里。 已变异验证：在任一站点重新引入 head 子句都会让新的「按时间归因」测试失败。测试：本文件 171 通过（唯一失败在原始树同样复现）；姊妹套件全绿；prettier 与 eslint 干净。 |
|
Qwen Code review did not complete successfully. A transient error is retried automatically; if you are seeing this, retry with |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Re-check: 3 blockers resolved
All three Critical blockers from prior rounds are confirmed fixed at HEAD a1380d019f:
-
startedAt→createdAt(comments 3811217387, 3811217401) — Both the fallback-comment job and the in-job step now query--json createdAt --jq '.createdAt'instead ofstartedAt. ThecreatedAtfield is attempt-stable across re-runs, whilestartedAtmoves to the re-executed attempt's start. Verified on the actual workflow lines 1797 and 2032. -
Head-independent attribution (comment 3813388372) — The
select(.commit_id == ...)clause has been dropped from both guard filters. The guard now attributes by TIME (bot account +submitted_at >= run_created), not by head. The workflow comments explicitly state "It is deliberately NOT keyed on the reviewed head." -
Test coverage — 8 new tests cover both sites (fallback job + in-job step): the incident shape, each near-miss (stale/foreign/pending/none), re-run stability, fail-open paths, and the absence of
commit_idin both sites. 90 tests pass (2 failures are environment-specific:jqnot available on Windows, and a pre-existingrepairs a single unwritable directoryfailure noted by the author).
No new findings. The remaining open inline comments are Suggestions about wording and edge cases already discussed in prior rounds.
中文说明
重新检查：3 个阻断问题均已解决
本轮审查确认此前三轮的所有 3 个 Critical 级别阻断问题已在 HEAD a1380d019f 修复：
-
startedAt改为createdAt（评论 3811217387、3811217401）——兜底作业和 in-job 步骤现在均查询--json createdAt --jq '.createdAt'而非startedAt。createdAt字段在各次重跑之间保持稳定，而startedAt会随重跑 attempt 的开始时间而漂移。已在 workflow 第 1797 和 2032 行验证。 -
按时间而非 head 归属（评论 3813388372）——两处守卫过滤器中的
select(.commit_id == ...)子句已被移除。守卫现在按时间（bot 账号 +submitted_at >= run_created）而非 head 来归属 review。workflow 注释明确说明"故意不锁定审过的 head"。 -
测试覆盖——新增 8 个测试覆盖两个站点（兜底作业 + in-job 步骤）：事故形态、四种近似情形（陈旧/他人账号/PENDING/无 review）、重跑稳定性、故障开放路径、以及两处均无
commit_id。90 条测试通过（2 条失败为环境问题：当前 Windows 系统无jq，以及作者已标注的既有repairs a single unwritable directory失败）。
无新发现。剩余的未结行内评论均为建议级，涉及措辞和边缘情况，已在之前轮次中讨论过。
— qwen3.8-max via Qwen Code /review (v0.21.13)
…d of failing them The stub answers the guard's reviews and run-view lookups by running the caller's own `--jq` filter — that filter IS the thing under test — so those cases need jq on PATH. A reviewer running the suite on Windows without jq saw them as failures of the guard rather than as untested, which is the wrong signal in the wrong direction. Probed once per run and skipped honestly. Measured with a jq that exits 127: the file goes from 31 failures to 26 failures plus 13 skips — the 26 are the retry-loop cases, which have parsed the review log with jq since long before this change and are equally untestable without it. GitHub's windows-latest image ships jq, so CI coverage is unchanged either way; what changes is what a jq-less machine reports.
|
Thanks for the re-check — and for the Windows datapoint, which was the one actionable thing in it. Fixed in 35bcbca. The jq failure was a wrong signal, not a broken guard. The stub answers the reviews and run-view lookups by running the caller's own One incident worth flagging, unrelated to the code but visible on this PR. A 中文说明感谢复核——其中唯一可执行的一点是 Windows 数据点，已在 35bcbca 修复。 jq 失败是错误信号，而非守卫损坏。 桩是用调用方自己的 另有一起与代码无关、但出现在本 PR 上的事件值得点出。 14:35:50Z 出现一条 |
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.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R4-2 test comment lists a head clause the shipped filter does not have (describe-level header) — dropped as overlap: same line as existing comment 3811217427 at scripts/tests/qwen-pr-review-workflow.test.js:3524
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only.
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.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-pr-review-workflow.test.js:3482 — [review] assertion comment says the guard asks for startedAt but it queries createdAt — rounds 2 and 3 deferred the same finding; code unchangedscripts/tests/qwen-pr-review-workflow.test.js:3300 — [probe] endpoint drift in the guard is an unobservable mutation — deferred (code-age rule: anchored code unchanged since round 3)scripts/tests/qwen-pr-review-workflow.test.js:3300 — [probe] the guard's --paginate flag is pinned by no assertion — deferred (code-age rule: anchored code unchanged since round 3)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # The head this run REVIEWED, not the PR's head at fallback time: on | ||
| # every trigger but pull_request_target the head-moved guard above | ||
| # deliberately does not run, so a push landing between the post and | ||
| # this step leaves `pr_head` pointing at bytes no review ever | ||
| # covered — the reviewed head's review then matches nothing and the | ||
| # contradictory comment posts anyway. Executed through this file's | ||
| # harness, that interleaving reproduces the #9342 shape exactly. | ||
| # The job output is empty only when review-pr died before its review | ||
| # step ran, which is also a run that posted nothing. |
There was a problem hiding this comment.
[Suggestion] Leftover paragraph from the abandoned head-keyed revision. It describes pr_head, "the reviewed head's review", and a review-pr "job output" — none of which exist: the shipped filter has no head clause, pr_head appears nowhere else in this workflow, and review-pr declares no outputs: (the new carries no cross-job head wiring to drift test pins both absences). It sits directly below the paragraph stating the guard is "deliberately NOT keyed on the reviewed head", contradicting it, and the in-job sibling carries no such paragraph. A maintainer debugging the next #9342-class contradiction reads this and either hunts for deleted wiring or "restores" head keying — re-opening the exact regression that test pins shut. Delete the paragraph; the shipped guard is fully explained by the preceding paragraphs.
| # The head this run REVIEWED, not the PR's head at fallback time: on | |
| # every trigger but pull_request_target the head-moved guard above | |
| # deliberately does not run, so a push landing between the post and | |
| # this step leaves `pr_head` pointing at bytes no review ever | |
| # covered — the reviewed head's review then matches nothing and the | |
| # contradictory comment posts anyway. Executed through this file's | |
| # harness, that interleaving reproduces the #9342 shape exactly. | |
| # The job output is empty only when review-pr died before its review | |
| # step ran, which is also a run that posted nothing. |
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // The reviews lookup runs the step's REAL --jq filter over the | ||
| // fixture: the guard under test IS that filter (author scope, head, | ||
| // submission time), so a stub that pre-applied it would pin nothing. |
There was a problem hiding this comment.
[Suggestion] This comment lists a head clause the shipped filter deliberately does not have — the filter is author scope + submission time only, and the attributes by TIME, not by head test exists precisely to pin that head is NOT a clause. It became wrong when the head clause was dropped. A maintainer making the test match the comment re-introduces head keying — the failure mode that test catches. Drop head, from the parenthetical.
| // The reviews lookup runs the step's REAL --jq filter over the | |
| // fixture: the guard under test IS that filter (author scope, head, | |
| // submission time), so a stub that pre-applied it would pin nothing. | |
| // The reviews lookup runs the step's REAL --jq filter over the | |
| // fixture: the guard under test IS that filter (author scope, | |
| // submission time), so a stub that pre-applied it would pin nothing. |
— qwen3.8-max via Qwen Code /review (v0.21.14)
| REVIEWS_JSON: reviews, | ||
| RUN_CREATED: runCreated, | ||
| RUN_STARTED_ATTEMPT: runStartedAttempt, | ||
| REVIEWED_HEAD_SHA: reviewedHead, |
There was a problem hiding this comment.
[Suggestion] The harness injects REVIEWED_HEAD_SHA: reviewedHead, here and declares reviewedHead = '' at line 3247, but nothing reads REVIEWED_HEAD_SHA — neither step's run block references it (zero occurrences in the workflow), which the new carries no cross-job head wiring to drift test pins. The wiring became dead when the head mechanism was dropped. It advertises harness support for reviewed-head scenarios the guard deliberately does not consume; a future author passing reviewedHead: 'X' sees silently no effect and may misdiagnose the guard as head-aware. Remove the reviewedHead = '' parameter (line 3247) and this REVIEWED_HEAD_SHA: reviewedHead, entry.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # The anchor is the run's CREATION, never its start. Re-running a | ||
| # failed job keeps the run id — the dedup above relies on exactly | ||
| # that — but moves run-level `startedAt` to the re-executed attempt: | ||
| # measured on this repo's runs 32219268680 (created 05:23:57Z, | ||
| # startedAt 05:51:26Z) and 32218596441 (05:13:04Z → 05:22:05Z). | ||
| # Anchoring there would push attempt 1's review outside "this run" | ||
| # on every re-run, so a re-run that fails before posting would | ||
| # contradict the review attempt 1 had posted — the very shape this | ||
| # guard exists to stop. `createdAt` is attempt-stable, and a review | ||
| # submitted after the run was created still cannot belong to an | ||
| # earlier run. |
There was a problem hiding this comment.
[Suggestion] The in-job block now states the createdAt-not-startedAt rationale twice: the "The window is anchored on createdAt, not startedAt …" passage about ten lines above and this paragraph, both citing the same two measured runs (32219268680, 32218596441). The fallback-comment job's block states it once. This round's comment rewrite added the first passage and cleaned the fallback block but left this second copy behind. A future correction must now be made in two places, and the two copies already phrase the claim differently — a one-sided edit leaves them silently disagreeing about the anchor. Delete this paragraph, keeping the first statement so the in-job block matches the fallback block's single-copy shape.
| # The anchor is the run's CREATION, never its start. Re-running a | |
| # failed job keeps the run id — the dedup above relies on exactly | |
| # that — but moves run-level `startedAt` to the re-executed attempt: | |
| # measured on this repo's runs 32219268680 (created 05:23:57Z, | |
| # startedAt 05:51:26Z) and 32218596441 (05:13:04Z → 05:22:05Z). | |
| # Anchoring there would push attempt 1's review outside "this run" | |
| # on every re-run, so a re-run that fails before posting would | |
| # contradict the review attempt 1 had posted — the very shape this | |
| # guard exists to stop. `createdAt` is attempt-stable, and a review | |
| # submitted after the run was created still cannot belong to an | |
| # earlier run. |
— qwen3.8-max via Qwen Code /review (v0.21.14)
Round 5's four, all leftovers of the round-3 design change rather than new behavior. The job-level block still explained why it compared against the head this run reviewed — naming `pr_head`, "the reviewed head's review" and a `review-pr` job output, none of which survive: the shipped filter is author scope plus the creation-time window, and the wiring was deleted with the head clause. A maintainer reading it would look for a comparison that is not there. The in-job block stated the createdAt-not-startedAt rationale twice, once with the measured run ids and once without; the measured one stays. Same in the tests: the stub's comment listed a head clause the filter deliberately does not have (`attributes by TIME, not by head` is the test that pins its absence), and the harness still declared and injected `reviewedHead`/`REVIEWED_HEAD_SHA`, which nothing reads since the wiring went — a knob that looks live and cannot be.
|
Round 5's four are taken in 7619a82. All four were leftovers of the round-3 design change, and the round is a fair audit of it: when a clause is removed, its rationale and its test knobs do not remove themselves.
No behavior change in this commit — comments and one dead test option. Suite unchanged: 171 pass in this file (plus the known local 中文说明第 5 轮四条已在 7619a82 处理。四条都是第 3 轮设计改动的残留，而这一轮正是对它的公正审计：子句被移除时，它的理由和它的测试旋钮不会自己消失。
本提交无行为变更（注释 + 一个死的测试选项）。测试不变：本文件 171 通过（外加已知本机 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- guard lookups swallow stderr (2>/dev/null) with no diagnostic trace — already reported (comment 3811217409, comment 3811217413)
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.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-pr-review-workflow.test.js:3482 — [review] Comment says the guard queries startedAt but it queries createdAt — rounds 2, 3 and 4 deferred the same finding; code unchangedscripts/tests/qwen-pr-review-workflow.test.js:3247 — [review] Harness option expectedHead is declared and read but set by no caller (dead switch) — code unchanged since round 4
中文说明
已审查。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| # The anchor is the run's CREATION, never its start. Re-running a | ||
| # failed job keeps the run id — the dedup above relies on exactly | ||
| # that — but moves run-level `startedAt` to the re-executed attempt: | ||
| # measured on this repo's runs 32219268680 (created 05:23:57Z, | ||
| # startedAt 05:51:26Z) and 32218596441 (05:13:04Z → 05:22:05Z). |
There was a problem hiding this comment.
[Suggestion] R4-4: Round 4 flagged that this block states the createdAt-not-startedAt rationale twice; the fix in 7619a82 deleted a different third paragraph ("Anchoring there would push attempt 1's review outside this run…"), leaving the flagged pair in place. The "What a match proves" paragraph above (~lines 1772-1778) already says the window is anchored on createdAt, not startedAt — citing the same two measured runs 32219268680 and 32218596441 — and this trailing paragraph restates it with slightly different wording ("the dedup above relies on that" vs "relies on exactly that"). The fallback-comment job's twin block states the rationale once. A future correction to the anchor rationale must now edit both copies, and a one-sided edit leaves this single block internally disagreeing about which timestamp the window anchors on. Delete this trailing paragraph so the in-job block matches the fallback block's single-copy shape.
| # The anchor is the run's CREATION, never its start. Re-running a | |
| # failed job keeps the run id — the dedup above relies on exactly | |
| # that — but moves run-level `startedAt` to the re-executed attempt: | |
| # measured on this repo's runs 32219268680 (created 05:23:57Z, | |
| # startedAt 05:51:26Z) and 32218596441 (05:13:04Z → 05:22:05Z). |
中文说明
【Suggestion】 R4-4：第 4 轮曾指出本代码块把 createdAt 而非 startedAt 的理由讲了两遍；7619a82cad 的修复删除的是另一段（"Anchoring there would push attempt 1's review outside this run…"），被指出的那一对仍然都在。上方 "What a match proves" 段（约 1772-1778 行）已说明窗口锚定在 createdAt 而非 startedAt——并引用了同样的两个实测 run 32219268680 与 32218596441——而这段结尾段落又用略有不同的措辞复述了一遍（"the dedup above relies on that" 对 "relies on exactly that"）。fallback-comment job 的孪生代码块只陈述了一次该理由。将来若要修正锚点理由，必须同时改两处；单边修改会让这一个注释块内部对「窗口锚定在哪个时间戳」自相矛盾。删除这段结尾段落，使 in-job 块与 fallback 块的单份陈述形态一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
Round 6 reports no blocker. R4-4 is a fair catch — the round-5 edit removed a neighbouring paragraph rather than the duplicated rationale, so the repetition it flagged is still there. It is comment text with no behavioral effect, and this stack's standing rule takes only Criticals past round 5, so it is recorded and deferred rather than spent as another round. Same for the re-reported 中文说明第 6 轮无阻断项。 R4-4 指出得对——第 5 轮的编辑删掉的是相邻段落，而非被点名的重复理由段，因此该重复仍在。它是注释文本、无行为影响；本栈现行规则是第 5 轮之后只采纳 Critical，故记录延后，而不是再消耗一轮。重报的 |
|
@qwen-code /takeover |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summaryThis round closed the one still-actionable inline finding (R4-4, re-reported in round 5) plus the last surviving items of the reviewer's deferred lists — all leftovers of the round-3 design change (attribute by time, drop the head clause). The batch is subtractive: net −6 lines, comments and one dead test knob only, no behavior change. Commit: Implemented this round
Re-verified as already resolved in prior commits (no change needed)
Declined / deferred / escalatedNone. Conflict
Verification
中文说明本轮总结本轮关闭了唯一仍处于可处理状态的行内发现（R4-4，第 5 轮重新上报），以及审查者延后列表中最后的遗留项——它们都是第 3 轮设计变更（按时间归因、移除 head 子句）的残留。本批改动是收敛性的：净 −6 行，仅涉及注释与一个死掉的测试旋钮，无行为变更。提交： 本轮实施
已复核确认先前提交已解决（无需改动）
拒绝 / 延后 / 升级无。 冲突
验证
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/模型 |
39a93c7
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-code-pr-review.yml:1766 — [review] the already-posted guard is duplicated verbatim across the two fallback sites; extraction would need a checkout added to the fallback job (also deferred in rounds 2, 3 and 6)
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.submitted_at >= \"$run_created\") | .id" 2>/dev/null)"; then |
There was a problem hiding this comment.
[Critical] The already-posted guard's window (bot account + submitted_at >= run createdAt) matches ANY review posted by the bot account — but this account posts other kinds of reviews too. finalize-release.yml:252-260 unconditionally approves release PRs as CI_BOT_PAT directly, and qwen-triage-finalize.yml:458 posts a deferred APPROVE under QWEN_CODE_BOT_TOKEN || CI_BOT_PAT (the same account whenever QWEN_CODE_BOT_TOKEN is unset). When such an approval lands inside the window, the guard exits 0 and the fallback stays silent for a genuinely dead review run — exactly the "silence with NO review at all" state the guard's comment claims the author + creation-time clauses rule out: the review pipeline's review is still absent, the claim "failed before a review could be posted" remains true, the retry instruction remains needed, and the visible LGTM approval now actively masks the dead pipeline.
Concrete shape, probed against this step's real bash: every release PR triggers a review run (release PRs are same-repo branches authored by github-actions[bot]; precheck-pr is fork-only and authorize unconditionally admits non-review_requested pull_request_target events), finalize-release's second-bot approval is submitted at or after the review run's creation, and if review-pr then fails before posting, the guard matches the approval and skips. The triage leg is the same shape whenever QWEN_CODE_BOT_TOKEN is unset — its green gate counts only event == "pull_request" runs (qwen-triage-finalize.yml:160,184), structurally excluding the review run's pull_request_target run, so its approval can land while the review run is alive. The accepted-trade-off paragraph argues only about an overlapping run's review of THIS workflow; it does not argue this sibling state, so the defense does not transfer.
witness: probe on the extracted real step bash (stub gh applying the step's own --jq filter):
ARM bot-approve-in-window: exit=0 posted=no — "Skipping fallback comment: a bot review of this PR was submitted after this run was created."
controls: different author → posted=yes; same account out-of-window → posted=yes
fixed copy (foreign approvals excluded): release-approve posted=yes, triage-approve posted=yes,
while the pipeline's own in-window review still posts=no
Key the guard on evidence only this pipeline's review carries instead of the account-wide window — the robust option is having the review step record the id of the review it posted and checking that evidence here; alternatively exclude the known foreign approval bodies from the match. The identical copy in the fallback-comment job (lines 2031-2032) needs the same treatment.
中文说明
【Critical】 已发布守卫的窗口（bot 账号 + submitted_at >= run createdAt）会匹配该 bot 账号发布的任何 review——但该账号还会发布其他种类的 review。finalize-release.yml:252-260 直接以 CI_BOT_PAT 无条件批准 release PR；qwen-triage-finalize.yml:458 以 QWEN_CODE_BOT_TOKEN || CI_BOT_PAT 发布延迟 APPROVE（当 QWEN_CODE_BOT_TOKEN 未配置时即同一账号）。当这类批准落在窗口内时，守卫 exit 0、兜底保持静默——而 review 流水线其实已经死了。这正是守卫注释声称 author + creation-time 子句已排除的「完全没有任何 review 的静默」状态：本流水线的 review 并未发布，「failed before a review could be posted」的断言仍然为真，重试指引仍然需要，而可见的 LGTM 批准反而把死掉的流水线掩盖了。
具体形态（已对步骤的真实 bash 做探针验证）：每个 release PR 都会触发一次 review run（release PR 是 github-actions[bot] 创建的同仓库分支；precheck-pr 仅针对 fork，authorize 无条件放行非 review_requested 的 pull_request_target 事件）；finalize-release 的第二 bot 批准提交于该 review run 创建之后；若 review-pr 在发布前失败，守卫就会匹配到该批准并跳过发帖。triage 分支在 QWEN_CODE_BOT_TOKEN 未配置时同形——其绿灯门槛只统计 event == "pull_request" 的 run（qwen-triage-finalize.yml:160,184），在结构上排除了 review run 的 pull_request_target run，因此其批准可以在 review run 存活期间落地。注释中「可接受的折衷」一段只论证了本 workflow 的重叠 run 的 review，未论证这种同账号的外来批准，因此该辩护不能迁移过来。
探针证据（抽出步骤真实 bash，桩 gh 套用步骤自己的 --jq 过滤器）：ARM bot-approve-in-window：exit=0 posted=no —— "Skipping fallback comment: a bot review of this PR was submitted after this run was created."；对照：不同作者 → posted=yes，同账号但窗口外 → posted=yes；修正副本（排除外来批准）：release-approve posted=yes、triage-approve posted=yes，而本流水线自己窗口内的 review 仍然 posted=no。
建议把守卫锚定到只有本流水线 review 才携带的证据上，而不是账号级窗口——最稳健的做法是让 review 步骤记录其发布的 review id，这里检查该证据；或者从匹配中排除已知的外来批准正文。fallback-comment 作业中的同款副本（2031-2032 行）需要同步处理。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #9462Feedback points and dispositions[rc:3820618375] Critical — already-posted guard matches foreign same-account approvals → Resolved in codeThe claim: the guard's window (bot account + Reproduced before changing anything: the claim is checkable and was probed. Two new tests run the step's real bash with a stub Fix: the guard's Both guard comment blocks now document the exclusion and name the sibling workflows, and the existing "stays silent" test gives the pipeline's own review a realistic report body so the exclusions cannot silently swallow it. Post-fix evidence: the new tests pass; a direct jq probe over the extracted real filter confirmed the finding's witness shape (foreign approvals excluded; the pipeline's own in-window review still matches — alone AND mixed with foreign approvals; stale/different-author controls unchanged); both guard copies verified byte-identical. [rv:4981569331] CHANGES_REQUESTED review — Declined (nothing requested this round)The review's items are explicitly marked "Not linted (tool limitation, not a blocker)" and "recorded, not requested in this round" (the duplicated-guard extraction, already deferred in rounds 2, 3 and 6). No checkable defect claim to reproduce; nothing to implement this round. Recorded here so the deferral stays visible. Changes
ConflictNone ( Verification
中文说明Autofix 轮次总结 — PR #9462反馈点及处置[rc:3820618375] Critical — already-posted 守卫会匹配同账号的外来批准 → 已在代码中解决该观点认为：守卫的窗口（bot 账号 + 修改前先复现：该缺陷可检验，且已做探针验证。两个新测试用桩 修复：守卫的 两处守卫的注释块均已记录该排除条款并点名相关兄弟 workflow；既有的 "stays silent" 测试现在为本流水线自己的 review 赋予真实的报告正文，防止排除条款悄悄把它吞掉。 修复后证据：新测试通过；对抽出的真实过滤器直接做 jq 探针，确认了该发现的见证形态（外来批准被排除；本流水线自己窗口内的 review 仍然匹配——单独存在以及与外来批准混合存在时均如此；stale/不同作者的对照不变）；两处守卫副本经核验逐字节一致。 [rv:4981569331] CHANGES_REQUESTED review — 拒绝（本轮无请求事项）该 review 的条目均明确标注 "Not linted (tool limitation, not a blocker)" 以及 "recorded, not requested in this round"（重复守卫的抽取，已在第 2、3、6 轮延后）。没有可检验的缺陷断言可供复现；本轮无可实施事项。记录于此以保持延后事项的可见性。 变更
冲突无（ 验证
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/模型 |
|
Verified the round-2 fix and found the exclusion list already incomplete — reporting rather than pushing, since the takeover loop owns the branch. A third foreign approval body exists, posted under the same account, and it is not excluded. The structural point is that the list cannot be finished by enumeration. Three sources are already known (two workflows plus a skill reference), one of them is not even a workflow file, and any of the three can be reworded without anyone thinking about this filter — and every such drift fails in the dangerous direction: a masked dead pipeline, not a noisy comment. Suggested direction: match on evidence only this pipeline's review carries, rather than excluding what it does not. Every body 中文说明已核验第 2 轮的修复，并发现排除清单本身就不完整——因分支由 takeover 循环管理，我只报告不推送。 存在第三种同账号外来批准正文，且未被排除。 结构性问题在于：这份清单无法靠枚举收尾。 目前已知三个来源（两个工作流 + 一个 skill 参考文档），其中一个甚至不是工作流文件；三者中任何一处改写措辞都不会有人想到这个过滤器，而每一次漂移都朝危险方向失败——被掩盖的死流水线，而不是一条吵闹的评论。 建议方向：匹配只有本流水线 review 才携带的证据，而不是排除它不携带的东西。 |
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: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only.
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.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-pr-review-workflow.test.js:3548 — [review] test hardcodes FOREIGN_APPROVAL_BODIES instead of deriving them from the producer workflows.github/workflows/qwen-code-pr-review.yml:1806 — [review] the already-posted guard is duplicated verbatim across the two fallback sites, embedding exclusion literals the file's convention says to define once (workflow-level env: next to FAL….github/workflows/qwen-code-pr-review.yml:1806 — [review] the exclusion literals are exact-string copies of producer-owned bodies (finalize-release.yml, qwen-triage-finalize.yml) with no back-reference on either producer side
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI (merge_group-gated) and its suite ran locally on Linux only。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| echo "::warning::already-posted guard unavailable (no run creation time); posting the fallback comment" | ||
| echo "Already-posted guard unavailable (run creation time missing); proceeding to post." >> "$GITHUB_STEP_SUMMARY" | ||
| elif ! posted_reviews="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/reviews" --paginate \ | ||
| --jq ".[] | select(.user.login == \"$bot_login\") | select(.submitted_at >= \"$run_created\") | select(.body != \"Automated second approval for the release version bump.\") | select(.body != \"LGTM, looks ready to ship — CI landed green after the review. ✅\") | .id" 2>/dev/null)"; then |
There was a problem hiding this comment.
[Critical] R7-1: the foreign-approval exclusion list is still incomplete — a third same-account approval body, LGTM, looks ready to ship. ✅, matches the guard window and silences the fallback for a genuinely dead run. .qwen/skills/triage/references/pr-workflow.md (lines 82-84 and 699-701) instructs the triage agent to approve with exactly that body via gh api .../reviews -f event=APPROVE, under QWEN_CODE_BOT_TOKEN || CI_BOT_PAT — the same account this guard resolves whenever that secret is unset. The two select(.body != ...) clauses exclude only the release-second-approval and triage-finalize bodies, so the triage approval passes the author and window clauses. This is not hypothetical: PR #9342 carries that exact body twice from qwen-code-ci-bot, once as the APPROVED review at 2026-08-19T00:10:21Z. A review run created before such an approval that then dies before posting its own review is silenced by it — the failure shape this PR exists to stop, through a third door. The identical copy at line 2050 has the same hole.
The entrance space is unbounded — three producers are already known (two workflows plus a skill reference doc an LLM executes), and the live reviews on PR #9342 also carry a fourth freeform variant ("LGTM, looks ready to ship — five rounds converged…") that any exclusion list, including one extended with the third body, would miss. Prefer matching on evidence only this pipeline's review carries — every composed review body ends with the via Qwen Code /review (v…) attribution, while the foreign approvals carry none — over enumerating bodies it does not carry. That fails in the safe direction: if the attribution ever changes, the guard stops firing and the comment posts (the pre-PR status quo, not a masked failure). Verify every composed body, including a zero-findings APPROVE, carries the attribution before adopting; if exclusions stay for now, at minimum add the third body at both sites and extend the test's FOREIGN_APPROVAL_BODIES.
witness — probe on the steps' real bash (stub gh applying the step's own --jq):
SHIPPED: site=fallback job body="LGTM, looks ready to ship. ✅" → status=0 posted=NO
site=in-job step body="LGTM, looks ready to ship. ✅" → status=0 posted=NO
("Skipping fallback comment: a bot review of this PR was submitted after this run was created.")
+ third exclusion clause: both sites → posted=YES
live: gh api repos/QwenLM/qwen-code/pulls/9342/reviews
→ review 4967239026 APPROVED 2026-08-19T00:10:21Z user=qwen-code-ci-bot
body="LGTM, looks ready to ship. ✅"
中文说明
【Critical】 R7-1：外来批准正文的排除清单仍不完整——第三个同账号批准正文 LGTM, looks ready to ship. ✅ 会匹配守卫窗口，使兜底评论在真正死掉的流水线面前保持静默。.qwen/skills/triage/references/pr-workflow.md（第 82-84 行与第 699-701 行）指示 triage agent 通过 gh api .../reviews -f event=APPROVE 以该正文发布批准，所用令牌为 QWEN_CODE_BOT_TOKEN || CI_BOT_PAT——当该 secret 未配置时即本守卫所解析的同一账号。现有两条 select(.body != ...) 只排除了 release 第二批准与 triage-finalize 的正文，因此该 triage 批准能通过作者与时间窗两个子句。这并非假设：PR #9342 上就有 qwen-code-ci-bot 以该正文发布的两条 review，其中一条是 2026-08-19T00:10:21Z 的 APPROVED。若一次 review run 在该批准落地之前创建、又在发布自己的 review 之前死掉，就会被它静音——正是本 PR 要消除的失败形态，从第三扇门进来。第 2050 行的同款副本有同样的洞。
入口空间是无界的——目前已知三个来源（两个工作流 + 一个由 LLM 执行的 skill 参考文档），且 PR #9342 的实际 review 数据中还有第四种自由措辞变体（"LGTM, looks ready to ship — five rounds converged…"），任何排除清单（包括补上第三个正文的清单）都会漏掉它。建议改为匹配只有本流水线 review 才携带的证据——每份合成的 review 正文都以 via Qwen Code /review (v…) 署名结尾，而外来批准两者皆无——而不是枚举它不携带的正文。该方向朝安全侧失败：若署名将来变化，守卫不再触发、评论照发（回到本 PR 之前的状态，而不是掩盖死流水线）。采纳前请确认每一份合成正文（包括零发现的 APPROVE）都携带该署名；若暂保留排除清单，至少在两处站点补上第三个正文，并同步扩充测试中的 FOREIGN_APPROVAL_BODIES。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The foreign-approval exclusion list shipped incomplete: the triage skill's commit-pinned APPROVE body also posts under the same account, matches the guard's author and window clauses, and silenced the fallback for a genuinely dead run — the failure shape this guard exists to stop. The producer set is open, so no exclusion list can be finished; every miss fails in the dangerous direction. Match positively instead: a review silences the fallback only if its body carries what only this pipeline's composed reviews carry — the "via Qwen Code /review" attribution footer or the invisible qwen-review-ledger marker. Every composed body carries at least one (a zero-findings APPROVE included); no foreign approval carries either. A marker that ever changes shape stops the guard firing and the comment posts — the pre-guard status quo, not a masked dead run.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #9462 (already-posted guard, round 9)One Critical finding, one carrying review, one maintainer report — all three name the same defect. Fixed at the root; nothing else was actionable this round. Feedback points and dispositions[rc:3821576772] Critical R7-1 — the foreign-approval exclusion list is incomplete; Reproduced before changing anything: ran the shipped The fix is the one the finding and the maintainer comment both recommend: stop enumerating what the pipeline's reviews are NOT, match on what only they ARE. Both guard sites now keep only reviews whose body carries the Changes:
Pre-round gate evidence: with the pre-round workflow restored temporarily, exactly the two new foreign-approval tests fail (one per site); with the fix, all pass. [rv:4982729106] CHANGES_REQUESTED review — "Partially reviewed — gaps disclosed" → Its finding (R7-1) is the Critical above; disclosures acknowledged. The review's carried finding is R7-1, fixed as above. Its two disclosures are noted and unchanged here: the macOS test matrix leg is merge_group-gated and this runner is Linux (the jq-gated tests probe for jq and skip honestly where absent), and actionlint's embedded-shell source mapping is unsupported (tool limitation — the real-bash test harness is this file's executable coverage). The three items the review deferred under the convergence posture (test deriving [ic:5355135806] Maintainer report (@wenshao) — same defect plus the structural direction → Fixed along that direction. The report's structural point — the list cannot be finished by enumeration, and every drift fails toward masking a dead pipeline — is exactly what the fix implements. Its suggested evidence pair (footer or ledger marker) was verified against the compose path before adoption, as the report asked. Conflict: none ( VerificationCommands actually run this round, in order:
Not run (not applicable): 中文说明本轮总结 — PR #9462（already-posted 守卫，第 9 轮）一条 Critical 发现、一份携带该发现的 review、一条维护者报告——三者指向同一个缺陷。已从根因修复；本轮没有其他需要处理的内容。 反馈点与处置[rc:3821576772] Critical R7-1 — 外来批准正文排除清单不完整， 修改前先复现：用 jq 1.6 将已合入的 修复方式正是该发现与维护者评论共同推荐的方向：不再枚举本流水线 review「不是什么」，改为匹配「只有它才是什么」。两处守卫站点现在只保留正文携带 改动：
前轮门禁证据：临时还原本轮之前的 workflow 后，恰好是两条新的外来批准测试失败（每站点一条）；应用修复后全部通过。 [rv:4982729106] CHANGES_REQUESTED review —— "Partially reviewed — gaps disclosed" → 其发现（R7-1）即上述 Critical；已确认其披露项。 该 review 携带的发现即 R7-1，已按上文修复。其两项披露在此记录、不做改动：macOS 测试矩阵腿受 merge_group 门控且本 runner 为 Linux（依赖 jq 的测试会探测 jq，缺失时如实跳过）；actionlint 的 embedded-shell source mapping 尚不支持（工具限制——真实 bash 测试骨架即该文件的执行覆盖）。review 在收敛姿态下延后的三项（测试从 producer 推导 [ic:5355135806] 维护者报告（@wenshao）—— 同一缺陷及结构性方向 → 按该方向修复。 报告的结构性观点——清单无法靠枚举收尾、且每一次漂移都朝掩盖死流水线的方向失败——正是本次修复所实现的。其建议的证据对（署名或 ledger 标记）在采纳前已按报告要求对合成路径完成核验。 冲突： 无（ 验证本轮实际执行的命令，按顺序：
未执行（不适用）： 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.
Reviewed — no blockers.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D9-1 already-posted guard duplicated verbatim across the two fallback sites — already recorded in the rounds 2, 3, 6, 7 and 8 deferral lists
- D9-2 the guard's --paginate flag is pinned by no assertion — already recorded in the rounds 2, 4 and 6 deferral lists
- D9-4 guard skip path does not record which review IDs bought the silence — already recorded in the round 6 deferral list
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.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-code-pr-review.yml:1811 — [review] marker literals are unpinned copies of the CLI composer constantsscripts/tests/qwen-pr-review-workflow.test.js:3299 — [probe] stub serves the reviews fixture to any repos/* endpoint; the guard's URL is unpinned
中文说明
已审查——无阻断问题。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken this round — the only new feedback is the automated reviewer's
No inline comments, no issue-level feedback, no failed or still-red checks were present this round (all CI checks are SUCCESS or SKIPPED). This is round 9; per the repo's review policy, Suggestion-level items past the early rounds stay deferred to follow-ups rather than growing a converging PR. No code changes were made and nothing was committed. 中文说明本轮未采取任何操作——唯一的新反馈是自动评审器发布的
本轮没有行内评论、没有 issue 级反馈、也没有失败或持续飘红的检查（所有 CI 检查均为 SUCCESS 或 SKIPPED）。当前已是第 9 轮评审；按照本仓库的评审政策，超出早期轮次的建议级事项应保持延后、交由后续跟进处理，而不是让一个正在收敛的 PR 继续膨胀。本轮未修改任何代码，也未提交任何内容。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D9-1 already-posted guard duplicated verbatim across the two fallback sites — already recorded in the rounds 2, 3, 6, 7, 8 and 9 deferral/duplicate lists
- D10-1 the guard's marker literals are unpinned hand-copies of the CLI composer constants (FOOTER_MARKER / qwen-review-ledger prefix) — already recorded in the round 9 deferral list
- D9-2 the guard's --paginate flag is pinned by no assertion (mutation probe: deleting it keeps the suite green) — already recorded in the rounds 2, 4, 6 and 9 deferral/duplicate lists
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.
中文说明
已审查。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— glm-5.3 via Qwen Code /review (v0.21.11)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken this round — the only new feedback is the automated reviewer's round-10
This is review round 10; the last two reviews both reported no blockers, and per the repo's review policy Suggestion-level items past the early rounds stay deferred to follow-ups rather than growing a converging PR. No code changes were made and nothing was committed. 中文说明本轮未采取任何操作——唯一的新反馈是自动评审器第 10 轮发布的
当前已是第 10 轮评审；最近两轮评审均报告无阻断问题，按照本仓库的评审政策，超出早期轮次的建议级事项应保持延后、交由后续跟进处理，而不是让一个正在收敛的 PR 继续膨胀。本轮未修改任何代码，也未提交任何内容。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 84 passed · 0 failed · 84 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：84 通过 · 0 失败 · 84 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9462 deep verification —
|
| scenario (review fixture) | base (no guard) | head (guard) |
|---|---|---|
| incident: bot, footer+ledger, after creation | POST, body carries the false claim | SILENT + "a bot review of this PR was submitted" in summary |
| attribution off (ledger marker only) | POST | SILENT |
| pre-ledger bundle (footer only) | POST | SILENT |
| re-run window (between createdAt and startedAt) | POST | SILENT |
| moved head (review on OLDSHA, PR at NEWSHA) | POST | SILENT |
| stale review (before this run was created) | POST | POST |
| foreign account (marker, in window) | POST | POST |
| foreign approval (bot LGTM, no marker) | POST | POST |
| PENDING review (submitted_at null) | POST | POST |
| no reviews at all | POST | POST |
| run creation time unavailable | POST | POST + ::warning::already-posted guard + summary line |
| reviews listing fails | POST | POST + ::warning::already-posted guard + summary line |
(identical at both sites — fallback-comment job and in-job step — 24 cells + skip-line/warning/false-claim checks = 64 assertions). The base incident cell is the control oracle: it ships "Qwen Code review did not complete successfully … retry with @qwen-code /review" beside a review the fixture says was posted; head replaces that with silence.
Marker invariant (why the positive match is sound): compose-review.ts appends the ledger marker to every PR-review body it returns (only local, PR-less reviews get none, and those are never posted); the attribution footer rides when review.attribution is on, so at least one marker rides every posted composed body. The one escape (a mid-compose plan-file corruption makes ledgerMarkerFor catch → null while attribution is off) fails open — the comment posts, i.e. the pre-guard status quo, not a masked dead run. The foreign same-account producers (finalize-release.yml ×2, qwen-triage-finalize.yml, triage skill pr-workflow.md ×2) all post fixed bodies containing neither marker and no PR-derived text, so nothing attacker-writable can mint a silencing body today.
Reviewer Test Plan walk
| plan step | result |
|---|---|
npx vitest run scripts/tests/qwen-pr-review-workflow.test.js — "165 pass" |
runs green; actual count at the final tree is 174 passed (174) (the 165 figure predates later rounds adding tests) |
| "Eight new tests execute the steps' real bash" | 16 new test instances exist (7 per site × 2 sites + 2 shared); all pass; they do execute the real bash |
{site} stays silent when THIS run already posted its review |
present, passes; M0/M1..M3 prove it non-vacuous |
… still posts when no review can be attributed to this run (stale/foreign-account/PENDING/none) |
present, passes; each case pinned by M1/M2 |
… posts when this run's start time is unavailable |
present under the renamed title "…creation time is unavailable"; passes; pinned by M5 |
… posts when the reviews lookup itself fails |
present, passes; pinned by M5/M6 |
pre-existing assertion tightened (not.toContain('run view') → head lookups) |
verified in diff and live: the guard calls gh run view … createdAt on every event; M4/M7 pin the lookups' semantics |
"Local failures unrelated to this change" (repairs a single unwritable directory, 5 autofix) |
do not reproduce in this container: the full file is 174/174 green here, and qwen-autofix-workflow.test.js is 192/192 (with one unhandled vitest-worker RPC timeout — infrastructure noise, not a test failure). Environment-dependent, not a PR regression |
| sibling suites "green (42 pass)" | green; actual count 55 passed |
Corrections
- The body says "The job-level step also now reads
stateandheadRefOidin onegh pr view… which is where its head value comes from", and its stub note explains thestate,headRefOidbranch as serving "the job-level step's new combined read". At the shipped head this is stale: round 3 removed the head clause entirely and reverted the fallback-comment job to a state-only query (--json state --jq '.state', line 1997); only the in-job step keeps the combined read (line 1725). The final test file's own comment ("Live again: the fallback job reverted to a state-only query …") matches the code. The description's round-1 wiring no longer exists — correctly, since the guard no longer keys on the head.
Findings
No blocking findings. Informational only:
-
Stale description counts (severity: nit). "165 pass" → 174; "eight new tests" → 16 instances; sibling suites "42" → 55; the combined-read/stub notes above. All are description drift across the PR's ten rounds; the shipped code and tests are internally consistent and every behavioral claim I tested holds.
-
Fail-open is pinned more broadly than the PR states (positive). M5 (silence-on-unavailable) was killed by 11 tests, not the 2 new ones: seven pre-existing tests run without a
runCreatedfixture, so their stub answerscreatedAtwith''and the shipped guard fail-OPENs there — turning that into silence voids their posts too. The fail-open direction is therefore defended by the old suite as well.
Mutation matrix (vacuity + coverage)
Control (unmutated head in scratch worktree): 174 passed, 0 failed. Every mutant killed exactly its predicted test set (exact-set equality; mutation-matrix.mjs, captured in 02-mutation-matrix-all-killed.png):
| mutant | killed by (predicted = observed) |
|---|---|
| M0 revert guard (both sites) | 7 (both "stays silent", both "survives a re-run", both "says so in the log", "attributes by TIME") — failures are behavioral (expected '<!\-\- qwen-review-fallback …' to be '') |
| M1 drop author clause | 2 ("still posts … not attributed" ×2, foreign case) |
| M2 drop time-window clause | 2 (same test, stale/pending cases) |
| M3 drop marker clause | 2 ("still posts … foreign approvals" ×2) |
| M4 createdAt→startedAt | 2 ("survives a job re-run" ×2) |
| M5 exit silent when creation time unavailable | 11 (4 predicted + 7 pre-existing fail-open pins) |
| M6a drop ::warning (creation-time branch) | 2 ("says so in the log" ×2) |
| M6b drop ::warning (reviews-failed branch) | 2 ("says so in the log" ×2) |
| M7 re-add head clause (in-job only) | 2 ("attributes by TIME", "carries no cross-job head wiring") |
| M8 break dedup anchor (control outside the guard) | 2 (both dedup tests) |
Zero survivors; the reverted run fails the intended behavioral assertion (M0 quotes expected-vs-actual posted body), and M8 proves the runner catches regressions outside the new tests.
Targeted gates
- head suite: 174 passed (174); base suite: 158 passed (158) → +16 tests, +0 failures.
- jq-less regime (jq exits 127): base 25 failed/133 passed, head 25 failed/134 passed/15 skipped; the 25 failing names are byte-identical on both arms (pre-existing retry-loop cases, equally untestable without jq), 0 head-only, 0 base-only; the 15 skips are exactly the jq-driven guard cases (
03-nojq-attribution-clean.png). - jq filter semantics against real jq: 8/8 (
04-jq-semantics-boundary.png) —>=keeps the boundary, drops 1s-before, drops nullsubmitted_at(PENDING), null body doesn't crash, footer/ledger match, foreign LGTM doesn't. - Lint:
bash -nclean on all four extracted scripts (base+head × 2 sites); shellcheck--shell=bashclean on all four (live control caught a planted violation); actionlint clean on the changed workflow (live control caught a planted undefined-variable); ESLint clean on the changed test file (live control caught a planted unused var). yamllint could not be installed (nopip3in this container) — see Not covered.
Not covered
- Real GitHub API shapes: no token in this sandbox, so
gh run view --json createdAtand RESTsubmitted_atformats were not observed live. The guard's lexicographic>=is valid only while both stay fixed-formatYYYY-MM-DDTHH:MM:SSZ— the same comparison pattern this workflow family already uses (qwen-triage.yml.submitted_at >= $since). If a format ever drifted, the failure direction is bounded: an unavailable/failed listing posts (fail-open), and a silently-wrong comparison degrades to the pre-guard status quo (comment posts) rather than masking a dead run. - yamllint gate not run (no
pip3in container); YAML structural validity is nonetheless proven by two independent parsers (actionlint and theyamlpackage used by the harness and the suite). - Per-commit attribution: depth-2 checkout holds only the merge commit, base tip, and PR head (snapshot lists 10 commits, 1 reachable). The aggregate
HEAD^1..HEADdiff was verified; per-commit attribution is out of reach. - End-to-end GitHub posting (a real comment on a real PR) — by design the harness stubs
gh; the PR's own scope statement agrees. - The author-claimed local failures (unwritable-dir test, 5 autofix tests) do not reproduce here; I did not chase the author's environment.
Methodology
Environment: node:22-bookworm CI container, merge-ref checkout (HEAD merge commit, HEAD^1 base tip, HEAD^2 PR head), npm ci+npm run build pre-run. ab-harness.mjs and mutation-matrix.mjs (scratch git worktree at HEAD with root node_modules symlinked, removed after) drive the steps' real bash extracted verbatim from base/head YAML through an independent stub gh that applies each call's own --jq to JSON fixtures with real jq; nojq-attribution.mjs and jq-semantics.mjs are the remaining scripted checks. Raw logs (ab-run*.log, mut-*.json, base-*.json, head-nojq.json, matrix-run.log) and harness sources live beside this report; evidence images in evidence/.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/qwen-pr-review-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-pr-review-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/qwen-pr-review-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
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. ✅
Superseded: the stage-1a template gate this review enforced now passes (Risk & Scope and Linked Issues were added); triage re-run re-gated the PR at f9333d0.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 Reviewed the latest feedback — no changes needed. Why, point by point: No action taken this round — there is no actionable feedback newer than the last evaluation (2026-08-20T16:57:29Z):
The PR stays as-is at 中文说明🤖 已审阅最新反馈——无需改动。逐点说明原因如下： 本轮未做任何改动——自上次评估（2026-08-20T16:57:29Z）之后没有可操作的反馈：
PR 保持在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at HEAD f9333d0. Approving — no new issues found.
The 22 open bot threads are all superseded by later commits (the bot posts them but they were never resolved by hand); I checked each remaining Critical against the code as it stands:
Both recent Criticals — fixed by this diff. The foreign-approval concern (release/triage APPROVEs from the same bot account buying silence, incl. the R7-1 LGTM, looks ready to ship. ✅ triage body) is resolved by dropping body-enumeration for a positive match on what only this pipeline's reviews carry: contains("via Qwen Code /review") or contains("qwen-review-ledger"). I ran the shipped filter against a fixture covering every shape — a composed body with the footer, the ledger marker, both, and neither; a foreign LGTM; a release approval; a wrong-author, a stale (pre-createdAt), and a PENDING (submitted_at: null) review. Only the three composed bodies matched; every foreign/stale/pending case fell through to posting. null >= "…" is false in jq, so PENDING is correctly excluded, and the |-piped contains(a) or contains(b) binds as intended (both test .body).
Other open threads — addressed. Skip-message wording now reads "a bot review of this PR was submitted after this run was created" (no longer over-claims "this run posted"); the dead *state*) stub branch is live again since the fallback job reverted to a state-only query; the ${{ vars.QWEN_REVIEW_MAX_TIMEOUT_MINUTES }} bad substitution is fixed by substituting before bash runs; both failure paths now emit ::warning:: + a step-summary line (no more silent degrade); the head-keyed leftovers and duplicated anchor rationale were removed.
Design is sound and fails safe. Scoped to the bot account + a submission >= createdAt (stable across job re-runs, unlike startedAt), deliberately not keyed on the moving head. Every unavailable input (no creation time, listing failure) declines to fire and posts — the pre-guard status quo, never a masked dead run.
CI green (qwen-pr-review-workflow.test.js 165 pass via Test ubuntu-latest).
|
Released in v0.22.0. |
4 participants
What this PR does
Stops both
/reviewfallback-comment sites from announcing a review that the same run had already posted. The review job can fail after the post — the CLI exiting silently, a cleanup step dying — and the fallback body's claim ("The review pipeline failed before a review could be posted") then contradicts the review sitting directly above it, with a retry instruction attached. Before composing a body, each site now asks whether a review this run posted is already on the PR, and stays silent when the answer is yes.The check is scoped three ways so a stale review can never buy silence on a genuinely dead pipeline: the bot's own account, the head this run reviewed, and a submission at or after this run started. Where that proof is unavailable — no start time, no head, a failed listing — the guard declines to fire and the comment posts, which is the same call the existing head-moved guard makes when its comparison is unavailable. The job-level step also now reads
stateandheadRefOidin onegh pr view(the in-job step already did), which is where its head value comes from.Why it's needed
Measured on PR #9342, run 32119559927: the review posted at 11:56:34Z;
review-prfailed at 12:00:53Z withTransient review failure (Qwen review completed but produced no output.); the fallback comment landed at 12:01:00Z telling the reader the pipeline "failed before a review could be posted … retry with@qwen-code /review" — a fresh ~3-hour review, asked for beside the review that had just landed.The cost is not only a confusing pair of messages. The autofix takeover loop reads the same feed a human does, and on that PR the red check drove it to merge main and re-run the full CI. A reviewer who trusts the comment spends another review cycle; a reader who trusts the review wonders which of the two to believe. It is the same defect class this review pipeline keeps paying down: prose asserting the opposite of the evidence beside it.
This PR fixes the false claim. It deliberately does not touch the underlying classification — a run that posts its review and then exits without stdout is still a red check — because that decision (does a posted review make the run a success?) deserves its own change; the empty-output classifier cannot currently tell "died" from "posted, then went quiet", and I have no reproduction of the second attempt's silence yet.
Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-pr-review-workflow.test.js— 165 pass. Eight new tests execute the steps' real bash (extracted from the YAML) against a stubghthat applies the step's own--jqfilter to review fixtures, because the guard is that filter:{fallback job,in-job step} stays silent when THIS run already posted its review— the incident shape; asserts no comment and the skip line in the step summary.… still posts when no review can be attributed to this run— each near-miss on its own: an earlier run's review at the same head, another account's review, a review of a different head, a PENDING (unsubmitted) one, and none at all.… posts when this run's start time is unavailableand… posts when the reviews lookup itself fails— the fail-open direction, so a lookup failure never reads as "a review exists".One pre-existing assertion was tightened rather than deleted:
expect(r.calls).not.toContain('run view')was the proxy for "no head comparison on comment runs", and the new guard asks that same command forstartedAton every event; it now pins the head lookups (headSha,--json headRefOid) themselves. The stub'sstate,headRefOidbranch also learned thepr_closedscenario itsstate-only sibling already knew — without that, the job-level step's new combined read always sawOPEN.Local failures unrelated to this change (verified by re-running the same tests on a pristine tree):
repairs a single unwritable directory instead of failing fastin this file, and 5 tests inscripts/tests/qwen-autofix-workflow.test.js.scripts/tests/{qwen-resolve-workflow,review-worktree-cleanup-workflow,serve-ab-workflow}.test.jsare green (42 pass).Evidence (Before & After)
N/A — CI-only change, no user-visible surface. The observable behavior is the presence or absence of a bot comment, which the tests above assert directly.
Tested on
Risk & Scope
gh, which is how every other executable test in this file works.Linked Issues
PR #8894 incident classthe touched test suite is named after — the incident that established the fallback comment and its dedup discipline.中文说明
这个 PR 做什么：让
/review的两个兜底评论站点不再宣称「未能发布 review」——而实际上同一次运行早已发布过。review 作业可能在发布之后才失败（CLI 静默退出、清理步骤挂掉），此时兜底正文的断言与其正上方的 review 自相矛盾，还附带重试指引。现在两处在组装正文前先检查：本次运行是否已在该 PR 上发布过 review；是则静默。检查按三个维度收窄，使陈旧 review 无法为「真正死掉的流水线」买到静默：bot 自己的账号、本次运行所审的 head、以及提交时间不早于本次运行开始。当证据不可得（无开始时间、无 head、查询失败）时守卫不触发、照常发帖——与既有 head-moved 守卫在比较不可得时的选择一致。
为什么需要：实测于 PR #9342（run 32119559927）：11:56:34Z review 已发布 → 12:00:53Z 作业失败（
Qwen review completed but produced no output.）→ 12:01:00Z 兜底评论称「failed before a review could be posted……retry with @qwen-code /review」，在刚落地的 review 旁边索要一次约 3 小时的重跑。代价不止是自相矛盾的两条消息：autofix takeover 循环与人读同一个信息流，在该 PR 上红检查驱动它合并 main 并重跑了整轮 CI。这与本流水线反复偿还的是同一类缺陷：散文断言与其旁边的证据相反。本 PR 只修这条虚假断言，刻意不动底层分类（发布后无 stdout 仍是红检查）——「已发布 review 是否应让该次运行判为成功」值得单独一个改动；当前的空输出判定分不清「进程死了」与「发完就没声了」，且我尚无第二次尝试静默的复现。
验证：
npx vitest run scripts/tests/qwen-pr-review-workflow.test.js—— 165 通过。新增 8 个测试直接执行从 YAML 抽出的真实 bash，用桩gh对 review 夹具套用步骤自己的--jq过滤器（守卫本身就是那个过滤器）：事故形态静默；四种近似情形（同 head 的旧 review、他人账号、不同 head、PENDING）与无 review 时照常发帖；开始时间不可得、reviews 查询失败时同样发帖。一条既有断言被收紧而非删除（not.toContain('run view')原是「评论型运行不做 head 比较」的代理，新守卫在所有事件上都会用同一命令问 startedAt，故改为直接钉 head 查询）。桩里的state,headRefOid分支补上了它state兄弟早已有的pr_closed场景。本机既有失败（原始树复现，与本改动无关）：本文件的repairs a single unwritable directory，以及qwen-autofix-workflow.test.js的 5 条。风险与范围：主要权衡是这个守卫在一种形态下用少量「沉默」换掉错误陈述——「发布了 review 然后死掉」的运行不再被播报；这正是意图所在，而该运行自己的红色检查仍在说作业失败，失败并未变得不可见，只是那句描述错误的话没有了。反方向的风险（让真正死掉的流水线沉默）被三个维度的收窄限制住，且每个子句都有各自的钉住测试。未验证/超出范围：底层分类未动——发布后无 stdout 仍是红检查；在 #9342 上正是这个红检查驱动 autofix 合并 main 并重跑 CI，所以本 PR 去掉的是那句虚假断言与重试指引，而不是那次浪费的重跑。要让「已发布 review」计入运行结果，需要空输出判定能区分「进程死了」与「发完就没声了」，我尚无复现（该运行第二次尝试的三分钟静默原因不明，怀疑是增量锚点的 same-sha 停轮，未证实）。此外未验证的是真实 GitHub API 行为——测试以桩
gh驱动步骤的真实 bash，这与该文件中其他可执行测试的做法一致。破坏性变更：无；不涉及 workflow 的输入、输出、secrets 或权限，唯一行为变化是给两个发帖步骤各加一个跳过条件。关联：#9342（本次误报的实测现场）、#9255（引入本 PR 所加固的兜底作业）、以及本测试套件命名所源自的
PR #8894 incident class。