fix(ci): resolve latest and retry the triage runner's qwen CLI installs - #8367
fix(ci): resolve latest and retry the triage runner's qwen CLI installs#8367wenshao wants to merge 7 commits into
Conversation
The triage job's Ensure-qwen step short-circuited on any pre-installed qwen — the same bug the review runner fixed in c9f0d06 — so a stale runner image kept its version forever AND pinned the action's own unconditional reinstall to that stale version (`qwen_cli_version` is read from this step's output). A triage run then died on npm exit 217 inside the action's install, before any triage ran. The step now mirrors the review runner's shape, plus what this runner fleet has since taught: - resolve @latest from the registry and upgrade only when stale; - run the global install from RUNNER_TEMP — the persistent workspace still holds the previous run's checked-out tree, and npm reads a cwd .npmrc into a root-privileged install (the hazard this workflow's other install sites already document); - retry the install 3 times with backoff (the shared runners produce transient npm crashes; one flake must not kill the job); - fall back to the installed version when the registry or the install is what's broken, still emitting the version output the action pin needs on every success path. The workflow's two other @latest install sites gain the same 3-attempt retry. Verified: the edited step extracted verbatim with `qwen review extract-step` and driven under stubbed npm/qwen through all three branches (already-latest skip; install failing thrice then falling back with exit 0 and the version output intact; upgrade succeeding with the PATH-shadow warning). shellcheck clean on the extracted script; actionlint finding count identical to main (21 pre-existing info/style, 0 introduced); workflow suite 23/23.
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with solid evidence — the linked #8366 triage run died on npm exit 217 mid-install, and the stale-image short-circuit is the same bug the review runner already fixed in c9f0d06. Not theoretical; this is a recurring failure on the shared runners. Direction: aligned. This is the project's own triage reliability, and the fix mirrors an accepted fix family (c9f0d06). No CHANGELOG signal needed for a CI-only change; the area is clearly in scope. Size: not applicable — single workflow file ( Approach: scope feels right. The main Risk: no elevated risk signals — Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分——关联的 #8366 triage run 在安装中途以 npm 退出码 217 死掉，而过时镜像短路正是 review runner 在 c9f0d06 已修掉的同一个 bug。不是理论问题，而是共享 runner 上的反复故障。 方向：对齐。这是项目自身 triage 的可靠性修复，且与已接受的修复家族（c9f0d0657）一致。纯 CI 改动无需 CHANGELOG 信号，领域显然在范围内。 规模：不适用——单个 workflow 文件（ 方案：范围合理。主 风险：无升级风险信号—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewI read the diff against the surrounding workflow. Independent baseline first: for a stale-image install step whose output pins a downstream reinstall, I'd (1) resolve the registry's The implementation matches that baseline cleanly. A few things I checked specifically:
No correctness, security, or regression concerns. Comments explain the why (c9f0d06 bug family, the Test evidence (this PR's own CI)This is an unattended CI run, so I'm quoting the PR's CI signal via the API rather than running anything. CI for the reviewed commit is still in flight — the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Note on coverage: this is a workflow-YAML change, so the unit suite doesn't directly pin the install step's runtime behaviour — the relevant pre-merge signals are the author's The product sandboxed lanes don't fit this change: 中文说明代码审查我对照周边 workflow 读了 diff。先独立给出基线：对于一个输出会钉住下游重装、且镜像可能过时的安装步骤，我会（1）显式解析 registry 的 实现与该基线干净吻合。我重点核对了几处：
无正确性、安全性或回归问题。注释解释为什么（c9f0d0657 修复家族、 测试证据（本 PR 自己的 CI）这是无人值守的 CI 运行，因此我通过 API 引用 PR 的 CI 信号，而非自行运行任何内容。被审 commit 的 CI 仍在进行中—— 覆盖说明：这是 workflow-YAML 改动，单元测试套件并不直接钉住安装步骤的运行时行为——相关的合并前信号是作者的 产品沙箱通道不适用于本改动： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, focused CI fix for an observed recurring failure; I'd merge without hesitation. Stepping back: the problem is real and evidenced (the #8366 exit-217 run, the same bug family as c9f0d06), the fix is the minimal set needed — resolve/upgrade/fallback where the output feeds the action pin, plain retry at the two sibling sites — and the fallback degrades to today's behaviour rather than making anything worse. My independent baseline and the PR's approach line up; I didn't find a simpler path it missed. The comments tell a future maintainer why each hazard exists, which is exactly what this file needs. If I had to maintain this in six months I'd thank the author. The only thing standing between this and an immediate approval is procedural, not a code concern: the PR's own CI ( One informational note for the maintainer (not a reservation): because 中文说明置信度：5/5 —— 干净、聚焦的 CI 修复，针对一个已观测到的反复故障；我会毫不犹豫地合并。 退一步看：问题真实且有证据（#8366 的 exit-217 run，与 c9f0d06 同一修复家族），修复是所需的最小集合——在输出喂给 action 钉版处做解析/升级/回退，在另外两处安装点做单纯重试——且回退退化到当前行为而非使任何情况更糟。我的独立基线与 PR 方案一致；没找到它遗漏的更简路径。注释告诉未来的维护者每个隐患为什么存在，正是本文件所需要的。如果六个月后由我维护，我会感谢作者。 唯一挡在立即批准之前的是程序性的，而非代码顾虑：PR 自己的 CI（ 一条给维护者的信息性说明（非保留意见）：由于 — Qwen Code · qwen3.8-max-preview Reviewed at |
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. |
The triage workflow's own test pinned the previous step body verbatim (the single version-output echo), which the repair necessarily rewrote — found by CI, since the local run had only exercised the review workflow's test file. The pin now asserts the properties the step owes rather than its text: a version-output emission on every exit-0 path (>=4 — the action pin reads it, and a path that forgets it re-pins the reinstall to an empty string, which the action treats as latest), plus the repaired shape itself: resolve-then-compare, the bounded retry loop, the RUNNER_TEMP cwd, and the registry-down fallback. scripts/tests: triage 121/121, review-workflow 23/23. The generate-release-notes / pr-self-report-label / qwen-autofix failures in a whole-dir run reproduce identically on origin/main (environmental, pre-existing).
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted. Test Plan (not a blocker): @qwen-code/qwen-code@0.21.2 — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 Test Plan（非阻断）：@qwen-code/qwen-code@0.21.2 — no such file or directory。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| # cover the transient npm crashes the shared runners produce (a | ||
| # triage run died on exit 217 mid-install); the fallback covers the | ||
| # registry being the thing that is broken. | ||
| want="$(npm view --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest' version 2>/dev/null | tail -n 1 || true)" |
There was a problem hiding this comment.
[Suggestion] The npm view resolution call that gates the whole upgrade gets zero retries, while the install it gates gets three (the for attempt in 1 2 3 loop ten lines below). A single transient flake on this one call silently disables the upgrade and re-pins the action to the stale installed version. — Failure scenario: a transient exit-217-class npm crash or DNS blip during this call is swallowed by || true, so want=''; the registry-down fallback then emits version=$have (the stale installed version) and exits 0 without ever attempting an install. The action's unconditional --prefer-offline reinstall re-pins to that stale version — reproducing the exact stale-dist-tag regression this PR exists to fix. The identical flake on the install path is absorbed by the 3-attempt loop.
want=''
for attempt in 1 2 3; do
want="$(npm view --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest' version 2>/dev/null | tail -n 1 || true)"
[ -n "$want" ] && break
[ "$attempt" -lt 3 ] && sleep $((attempt * 5))
done中文说明
门控整个升级的 npm view 解析调用没有重试，而它所门控的安装却有 3 次重试（下方十行的 for attempt in 1 2 3）。这一次调用上的单次瞬态故障就会悄悄禁用升级，并把 action 重新钉到过时的已装版本。— 故障场景：该调用期间发生一次 exit-217 类 npm 崩溃或 DNS 抖动，被 || true 吞掉，于是 want=''；registry 不可达回退随后输出 version=$have（过时已装版本）并 exit 0，完全不尝试安装。action 的无条件 --prefer-offline 重装会重新钉到该过时版本——恰好复现本 PR 要修复的 stale-dist-tag 回归。同样的故障若发生在安装路径上，会被 3 次重试循环吸收。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| set -uo pipefail | ||
| # Resolve latest and upgrade when stale — the previous shape |
There was a problem hiding this comment.
[Suggestion] Changing set -euo pipefail to set -uo pipefail is a no-op for error semantics and is misleading. This step has no shell: and its job no defaults:, so GitHub invokes it as bash -e {0} — errexit stays active regardless of the set line (the line adds -u/pipefail but does not disable -e). — Failure scenario: the line reads as "errexit off" but errexit is on (a failing actual="$(qwen --version)" aborts the step with exit 1 rather than falling through). A future maintainer reasoning from the false contract — or "fixing" the perceived mismatch by adding set +e to match the apparent intent — would activate an empty-pin fall-through path that today cannot fire.
set -euo pipefail中文说明
把 set -euo pipefail 改成 set -uo pipefail 对错误语义是空操作，且具有误导性。本步骤没有 shell:，其 job 也没有 defaults:，所以 GitHub 以 bash -e {0} 调用——无论 set 行如何，errexit 都保持开启（该行只增加 -u/pipefail，并未关闭 -e）。— 故障场景：该行看起来像「errexit 关闭」，实际 errexit 是开的（actual="$(qwen --version)" 失败时会以 exit 1 中止步骤，而非继续往下走）。未来若有维护者依据这个虚假契约去推理，或为了「消除」这个看似的不一致而加上 set +e 去迎合表面意图，就会激活一条今天根本无法触发的空钉版（empty-pin）直通路径。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| fi | ||
| if [ -z "$want" ]; then | ||
| if [ -n "$have" ]; then | ||
| echo "::warning::npm registry unreachable; triaging with installed qwen $have" |
There was a problem hiding this comment.
[Suggestion] The registry-down fallback warning hardcodes npm registry unreachable for every failure mode, and 2>/dev/null on the npm view call (line 591) discards the real npm error. — Failure scenario: when npm crashes during npm view (the exit-217 transient failure that motivated this PR) or the registry returns 5xx, want is empty and the warning asserts "npm registry unreachable", sending the oncall to investigate DNS/network when the real cause (npm crash / server error) was thrown away by 2>/dev/null.
# capture the cause instead of asserting one
npm_err="$(npm view --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest' version 2>&1 >/dev/null | tail -n 1 || true)"
...
echo "::warning::npm registry query failed (${npm_err:-unknown}); triaging with installed qwen $have"中文说明
registry 不可达回退的警告对所有失败模式都硬编码为 npm registry unreachable，而 npm view 调用（591 行）上的 2>/dev/null 丢弃了真实的 npm 错误。— 故障场景：当 npm 在 npm view 期间崩溃（即促成本 PR 的 exit-217 瞬态故障）或 registry 返回 5xx 时，want 为空，警告却断言「npm registry unreachable」，把 oncall 引去排查 DNS/网络，而真正的原因（npm 崩溃 / 服务端错误）已被 2>/dev/null 丢掉。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| echo "::warning::qwen install attempt $attempt failed (wanted $want)" | ||
| sleep $((attempt * 10)) |
There was a problem hiding this comment.
[Suggestion] All three retry loops run sleep $((attempt * 10)) unconditionally inside the loop body, so after the 3rd and final failed attempt they sleep 30s with no subsequent attempt (same shape at the tmux lane ~1170 and the verify lane ~2453). — Failure scenario: on a full install failure (the exact exit-217 / registry-outage scenario the loops exist for), the loop sleeps 10+20+30=60s total, the final 30s buying no retry spacing; every fully-failed run stalls 30s before the fallback/exit 1, at all three sites.
echo "::warning::qwen install attempt $attempt failed (wanted $want)"
if [ "$attempt" -lt 3 ]; then sleep $((attempt * 10)); fi(The if form is set -e-safe for the two lane sites that keep set -euo pipefail; an &&-list would trip errexit when the test is false.)
中文说明
三个重试循环都在循环体内无条件执行 sleep $((attempt * 10))，因此在第 3 次（最后一次）失败后仍会 sleep 30 秒，而后面已没有第 4 次尝试（tmux 通道 ~1170 和 verify 通道 ~2453 形状相同）。— 故障场景：当安装彻底失败（正是循环要应对的 exit-217 / registry 宕机场景）时，循环共 sleep 10+20+30=60 秒，其中最后 30 秒换不来任何重试间隔；每一次彻底失败的运行都会在回退/exit 1 前空等 30 秒，三处皆然。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedAll four inline suggestions from the automated reviewer are implemented in one commit. 1. Retry the
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| [ -n "$installed" ] || { echo '::error::installing @qwen-code/qwen-code@latest failed after 3 attempts'; exit 1; } | ||
| qwen --version | ||
| tmux -V |
There was a problem hiding this comment.
[Suggestion] The 10-line retry-install block is duplicated verbatim across the workflow (lines 615–623, 1172–1181, 2456–2465), and the copies have already drifted — the Ensure qwen CLI step carries --loglevel=error --no-audit and pins @$want, while these two siblings do not. — Failure scenario: when the retry policy next needs a change (bumping to 5 attempts, adding --no-audit), the fix must be applied to three locations in lockstep; missing one silently diverges the lanes. The drift is already present.
Note: the tmux/verify installs run before checkout, so a repo-local .github/scripts/install-qwen.sh would not be on disk there — extraction needs a pre-checkout vehicle (composite action or script fetched at a pinned ref).
| [ -n "$installed" ] || { echo '::error::installing @qwen-code/qwen-code@latest failed after 3 attempts'; exit 1; } | |
| qwen --version | |
| tmux -V | |
| # TODO: extract the retry-install into a composite action | |
| # (e.g. .github/actions/install-qwen/action.yml) callable | |
| # from all three sites — the triage lane (post-checkout) | |
| # and the tmux/verify lanes (pre-checkout). | |
| [ -n "$installed" ] || { echo '::error::installing @qwen-code/qwen-code@latest failed after 3 attempts'; exit 1; } | |
| qwen --version | |
| tmux -V |
中文说明
[建议] 10 行重试安装块在 workflow 中逐字复制了三份（615–623、1172–1181、2456–2465 行），且副本已经漂移——Ensure qwen CLI 步骤带有 --loglevel=error --no-audit 并钉住 @$want，而这两个兄弟步骤没有。— 故障场景：当下次需要更改重试策略（增加到 5 次、添加 --no-audit）时，必须同步修改三处；漏掉一处就会悄悄分化各 lane。漂移已经存在。
注意：tmux/verify 安装在 checkout 之前运行，因此仓库内的 .github/scripts/install-qwen.sh 在那里不存在——提取需要 checkout 前的载体（composite action 或按固定 ref 获取的脚本）。
— qwen3.8-max-preview via Qwen Code /review
| // stale-dist-tag bug the pin exists to prevent). Three success paths, | ||
| // three emissions: already-latest, fallback-to-installed (twice: registry | ||
| // down, install failed), and fresh-install. |
There was a problem hiding this comment.
[Suggestion] Comment says "three emissions" but the step has four emission sites and the assertion below checks toBeGreaterThanOrEqual(4). — Failure scenario: a reader reconciling the comment's "three emissions" with the assertion's 4 must re-count the workflow to discover the comment is wrong; if a fifth path is added later, the stale "three" makes the intended invariant harder to audit.
| // stale-dist-tag bug the pin exists to prevent). Three success paths, | |
| // three emissions: already-latest, fallback-to-installed (twice: registry | |
| // down, install failed), and fresh-install. | |
| // stale-dist-tag bug the pin exists to prevent). Three success paths, | |
| // four emission sites (fallback-to-installed is reached twice: registry | |
| // down, install failed), plus fresh-install. |
中文说明
[建议] 注释写的是 "three emissions"（三处输出），但步骤实际有四个输出位，下方断言检查的是 toBeGreaterThanOrEqual(4)。— 故障场景：读者将注释中的 "three emissions" 与断言的 4 对比时，必须重新计数 workflow 才能发现注释写错了；如果以后添加第五条路径，过时的 "three" 会使预期不变量更难审计。
— qwen3.8-max-preview via Qwen Code /review
| // and the registry-down fallback that still triages on the installed CLI. | ||
| const ensure = step('Ensure qwen CLI'); | ||
| expect(ensure).toContain("@qwen-code/qwen-code@latest' version"); | ||
| expect(ensure).toContain('for attempt in 1 2 3; do'); |
There was a problem hiding this comment.
[Suggestion] The retry loops added to Install tmux runner tools (~line 1172) and Install verify runner tools (~line 2456) have no test coverage; only the Ensure qwen CLI step's retry is pinned here. — Failure scenario: a future edit removes or restructures the retry loop in either sibling step. The existing tests for those steps check for the npm install command and RUNNER_TEMP cwd but not for the retry loop or the failed after 3 attempts error guard. The next transient npm crash (the exit-217 class) kills the whole job instead of retrying, and the test suite passes green (verified: 121/121 pass with the sibling loops unasserted).
| expect(ensure).toContain('for attempt in 1 2 3; do'); | |
| expect(ensure).toContain('for attempt in 1 2 3; do'); | |
| // Also pin the sibling retry loops (added in the same diff): | |
| const tmuxInstall = stepIn('tmux-testing', 'Install tmux runner tools'); | |
| expect(tmuxInstall).toContain('for attempt in 1 2 3; do'); | |
| expect(tmuxInstall).toContain('failed after 3 attempts'); |
中文说明
[建议] 添加到 Install tmux runner tools（约 1172 行）和 Install verify runner tools（约 2456 行）的重试循环没有测试覆盖；此处仅钉住了 Ensure qwen CLI 步骤的重试。— 故障场景：未来编辑删除或重构了任一兄弟步骤中的重试循环。这些步骤的现有测试检查了 npm install 命令和 RUNNER_TEMP 工作目录，但没有检查重试循环或 failed after 3 attempts 错误守卫。下一次瞬态 npm 崩溃（exit-217 类）将杀死整个 job 而不是重试，且测试套件全绿通过（已验证：121/121 通过，兄弟循环未被断言）。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedrc:3698661220 — Suggestion: extract retry-install into a composite action (
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted. Test Plan (not a blocker): @qwen-code/qwen-code@0.21.2 — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 Test Plan（非阻断）：@qwen-code/qwen-code@0.21.2 — no such file or directory。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); | ||
| expect(ensure).toContain('triaging with installed qwen'); |
There was a problem hiding this comment.
[Suggestion] The rewritten Ensure qwen CLI step has two hard-fail paths that no test pins: echo '::error::qwen is not installed and the npm registry query failed'; exit 1 (qwen-triage.yml:612-613) and echo "::error::qwen is not installed and installing $want failed after 3 attempts"; exit 1 (qwen-triage.yml:630-631). The new/changed tests assert only the success and fallback-to-installed paths; the emissions >= 4 regex counts only the four success-path version= writes. — Failure scenario: probe-confirmed — softening both exit 1 → exit 0 keeps all 121 tests green (the emissions test counts only success paths), so the step would pass with no qwen and no version output, re-pinning the action reinstall to an empty string (the action treats it as latest, resurrecting the stale-dist-tag bug) and losing the fail-fast ::error:: diagnostic. Note: asserting on the message text alone does not catch an exit-code softening — assert on exit 1 / the ::error:: prefix.
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); | |
| expect(ensure).toContain('triaging with installed qwen'); | |
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); | |
| expect(ensure).toContain('triaging with installed qwen'); | |
| // The two hard-fail paths must keep failing the job — pin the exit code, | |
| // not just the message: a softened `exit 0` would otherwise ship undetected. | |
| expect(ensure).toContain('::error::'); | |
| expect(ensure).toContain('exit 1'); |
中文说明
重写后的 Ensure qwen CLI 步骤有两条必须让 job 失败的路径，但没有任何测试钉住它们：echo '::error::qwen is not installed and the npm registry query failed'; exit 1（qwen-triage.yml:612-613）和 echo "::error::qwen is not installed and installing $want failed after 3 attempts"; exit 1（qwen-triage.yml:630-631）。新增/修改的测试只断言成功路径与回退到已装版本的路径；emissions >= 4 正则只统计四条成功路径的 version= 输出。— 失败场景：经探针实测确认——把两处 exit 1 软化为 exit 0，全部 121 个测试仍全绿（emissions 测试只数成功路径），于是该步骤会在没有 qwen、也没有 version 输出的情况下通过，把 action 的重装版本钉成空字符串（action 会当作 latest，复活 stale-dist-tag bug），并丢掉预期的 fail-fast ::error:: 诊断。注意：只断言错误信息文本抓不到退出码被软化的情况——要断言 exit 1 / ::error:: 前缀。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8367One inline suggestion was open this round (round 2). It is implemented and the Inline suggestions
No required, declined, deferred, or escalated items this round. No conflict Verification
The change is test-only ( 中文说明已处理的审查反馈 — PR #8367本轮（第 2 轮）有一条行内建议处于待处理状态。已实现，且该发现已在代码中解决。 行内建议
本轮无必须项、无拒绝项、无延迟项、无升级项。无冲突（ 验证
本次改动仅涉及测试（ 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/模型 |
Code review —
|
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8367Addressed the maintainer code review of Feedback dispositions1. The round-3 test does not pin what its comment says it pins — FIXED (Required). Confirmed against the code: the Fix: replaced the two unanchored assertions in
Proven by mutation: softening either 2. New hard-fail corner that This is a genuine resilience concern: when the registry query fails all 3 attempts and nothing is pre-installed, the step now exits 1 without attempting an install, whereas 3. Unvalidated The review confirms "Neither is reachable today … this is defense-in-depth, not a live bug" (the registry is flag-pinned and Nits
Conflict notes
Verification
中文说明Autofix 审查轮次 — PR #8367处理了维护者对 反馈处理结论1. round-3 新加的测试没有钉住它注释所声称的东西 — 已修复（必改项）。 已对照代码确认： 修复：在
已通过变异验证：把任意一处 2. 相对 main 新增的硬失败角落 — 延后到后续 PR。 这是一个真实的韧性隐患：当 registry 查询三次全败且本机没有预装 qwen 时，步骤现在直接 exit 1、从不尝试安装；而 main 会直接执行 3. 未校验的 审查已确认「当前两者都不可达……属纵深防御，并非现实 bug」（registry 已用 flag 钉住， 小问题
冲突说明
验证
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. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted. Test Plan (not a blocker): @qwen-code/qwen-code@0.21.2 — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。 Test Plan（非阻断）：@qwen-code/qwen-code@0.21.2 — no such file or directory。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| // The step used to short-circuit on any pre-installed qwen — the same bug | ||
| // the review runner fixed in c9f0d0657 — and its stale version output then | ||
| // pinned the action reinstall to a stale release. Pin the repaired shape: | ||
| // resolve-then-compare, a bounded retry loop, the RUNNER_TEMP cwd (npm |
There was a problem hiding this comment.
[Suggestion] The comment below claims this test pins "resolve-then-compare", but no assertion covers the staleness comparison [ "$have" = "$want" ] or the qwen $have (latest) early-exit (qwen-triage.yml ~601) — only the resolve half (the npm view line, the retry loop) and the fallback paths are asserted. — Failure scenario: a one-line mutation = → != inverts the PR's headline behaviour: on a stale image (have != want, the exact case this PR fixes) the step enters the early-exit block, emits version=$have and exits 0, so triage runs the stale CLI and the action reinstall is pinned to the stale version — resurrecting the c9f0d06 bug the comment cites. Every test stays green (the emission-count regex still finds 4 textual sites).
Add an assertion for the comparison, e.g.:
expect(ensure).toContain('[ "$have" = "$want" ]');中文说明
下方注释声称本测试钉住了 “resolve-then-compare”，但没有任何断言覆盖版本比较 [ "$have" = "$want" ] 或 qwen $have (latest) 提前退出分支（qwen-triage.yml ~601）——只断言了 resolve 部分（npm view 行、重试循环）和回退路径。 — 故障场景：单行变异 = → != 会反转本 PR 的核心行为：在过时镜像上（have != want，正是本 PR 要修的场景）步骤会进入提前退出分支，输出 version=$have 并以 0 退出，于是 triage 用过时 CLI 运行、action 重装被钉到过时版本——复活注释所引用的 c9f0d06 bug。所有测试仍为绿（emission 计数正则仍能匹配到 4 处文本）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| expect(installStep).toContain('qwen --version'); | ||
| expect(installStep).toContain('tmux -V'); | ||
| expect(installStep).toContain('for attempt in 1 2 3; do'); | ||
| expect(installStep).toContain('failed after 3 attempts'); |
There was a problem hiding this comment.
[Suggestion] This lane's new retry assertion pins the error message but not the guard's exit 1 (qwen-triage.yml ~1181). The Ensure qwen CLI test in this same PR deliberately anchors each hard-fail message to its own exit 1, with a comment explaining why ("a softened exit 0 would otherwise ship undetected"); the two retry lanes (here and the verify lane at ~4017) omit that anchoring. — Failure scenario: a future edit softens the guard [ -n "$installed" ] || { …; exit 1; } to exit 0. On a stale-image runner (the PR's stated premise) all three installs fail, the step continues, and the immediately-following qwen --version succeeds against the stale pre-installed binary — so the lane silently runs against the stale qwen this PR exists to eliminate. Both toContain assertions stay green because the message string is still present.
| expect(installStep).toContain('failed after 3 attempts'); | |
| expect(installStep).toMatch(/failed after 3 attempts'; exit 1/); |
中文说明
本 lane 新增的重试断言只钉住了错误消息，没钉住守卫的 exit 1（qwen-triage.yml ~1181）。同一 PR 里 Ensure qwen CLI 的测试刻意把每个硬失败消息锚定到它自己的 exit 1，并有注释解释原因（“否则被软化的 exit 0 会悄然上线”）；两个重试 lane（此处与 ~4017 的 verify lane）漏掉了这一锚定。 — 故障场景：未来某次编辑把守卫 [ -n "$installed" ] || { …; exit 1; } 软化为 exit 0。在过时镜像 runner 上（本 PR 的前提）三次安装全失败、步骤继续执行，紧随其后的 qwen --version against 过时预装二进制成功——于是该 lane 悄然 against 本 PR 要消除的过时 qwen 运行。两个 toContain 断言仍为绿，因为消息字符串仍在。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const ensure = step('Ensure qwen CLI'); | ||
| expect(ensure).toContain("@qwen-code/qwen-code@latest' version"); | ||
| expect(ensure).toContain('for attempt in 1 2 3; do'); | ||
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); |
There was a problem hiding this comment.
[Suggestion] No assertion covers the install command's target "@qwen-code/qwen-code@$want" (qwen-triage.yml ~617) — the line that installs the resolved version rather than re-resolving @latest. Probe-confirmed: applying the mutation below leaves both relevant tests green, and adding the assertion flips the result. — Failure scenario: change the install line from "@qwen-code/qwen-code@$want" back to '@qwen-code/qwen-code@latest' (the old shape). On a runner whose npm cache holds a stale @latest dist-tag resolution, the install resolves a different (stale) release than npm view just resolved; the post-install shadow warning fires but the step still outputs the stale version= to GITHUB_OUTPUT, pinning the action reinstall to the stale release. Every test stays green.
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); | |
| expect(ensure).toContain('cd "${RUNNER_TEMP:?}"'); | |
| expect(ensure).toContain('"@qwen-code/qwen-code@$want"'); |
中文说明
没有断言覆盖安装命令的目标 "@qwen-code/qwen-code@$want"（qwen-triage.yml ~617）——正是这一行安装已解析的版本、而非重新解析 @latest。已用探针确认：施加下面的变异后两个相关测试仍为绿，加上该断言后结果翻转。 — 故障场景：把安装行从 "@qwen-code/qwen-code@$want" 改回 '@qwen-code/qwen-code@latest'（旧形态）。在 npm 缓存持有过时 @latest dist-tag 解析的 runner 上，安装会解析出与刚才 npm view 不同的（过时）版本；安装后的遮蔽警告会触发，但步骤仍把过时 version= 写入 GITHUB_OUTPUT，把 action 重装钉到过时版本。所有测试仍为绿。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| // reads a cwd .npmrc from the persistent workspace into a global install), | ||
| // and the registry-down fallback that still triages on the installed CLI. | ||
| const ensure = step('Ensure qwen CLI'); | ||
| expect(ensure).toContain("@qwen-code/qwen-code@latest' version"); |
There was a problem hiding this comment.
[Suggestion] No assertion covers the --registry=https://registry.npmjs.org flag on the npm view resolution command (qwen-triage.yml ~593). That line runs from the persistent workspace (not RUNNER_TEMP), so its --registry flag is the sole defense against a cwd .npmrc registry override. The assertion below matches both the flagged and the flagless form (verified), so it does not pin the flag — a gap between the test's name ("never trusts a cwd .npmrc") and its actual coverage. — Failure scenario: if the --registry flag is removed from the npm view line, a .npmrc left in the persistent workspace redirects the dist-tag resolution; want resolves to a stale version, and if the runner already has it, have == want triggers the early exit and the action reinstalls the stale release — the bug this PR fixes. The existing assertion still matches the flagless line, so every test stays green.
| expect(ensure).toContain("@qwen-code/qwen-code@latest' version"); | |
| expect(ensure).toContain("@qwen-code/qwen-code@latest' version"); | |
| expect(ensure).toContain("npm view --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest' version"); |
中文说明
没有断言覆盖 npm view 解析命令上的 --registry=https://registry.npmjs.org 标志（qwen-triage.yml ~593）。该行从持久工作区运行（而非 RUNNER_TEMP），因此它的 --registry 标志是抵御 cwd .npmrc registry 覆盖的唯一防线。下方断言对带标志和不带标志两种形态都匹配（已验证），所以并未钉住该标志——这是测试名（“never trusts a cwd .npmrc”）与其实际覆盖之间的缺口。 — 故障场景：若从 npm view 行移除 --registry 标志，持久工作区里残留的 .npmrc 会重定向 dist-tag 解析；want 解析到过时版本，若 runner 已装有该版本，have == want 触发提前退出、action 重装过时版本——正是本 PR 修的 bug。现有断言仍匹配无标志行，所有测试仍为绿。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
Rechecked this after #8343 and #8373. I do not think this PR is still needed as-is. Current CI evidence:
This PR is in the same problem family, but it assigns ownership differently. The exact per-job By resolving and globally installing |
4 participants
What this PR does
Fixes the triage job's
Ensure qwen CLIstep to resolve@latestfrom the registry, upgrade only when stale, run the global install fromRUNNER_TEMP, retry it 3 times with backoff, and fall back to the pre-installed version (still emitting theversionoutput the action pin needs) when the registry or the install is the broken part. The workflow's two other@latestinstall sites gain the same 3-attempt retry.Why it's needed
The step short-circuited on any pre-installed qwen — the identical bug the review runner fixed in c9f0d06 — with a second-order consequence the review runner didn't have: this step's output feeds
qwen_cli_version, which pins the composite action's own unconditional reinstall. A stale image therefore kept its version forever and dictated what the action reinstalled. Observed on #8366's triage run: the action's install of the stale-pinned0.21.2died with npm exit 217 on a shared runner (actions-runner-test-13) before any triage ran. Retries absorb that class of transient npm crash; theRUNNER_TEMPcwd closes the same.npmrc-into-global-install hazard this workflow's other install sites already document.Reviewer Test Plan
How to verify
The edited step was extracted verbatim with the repo's own
qwen review extract-stepand driven under stubbednpm/qwenthrough all three branches:Plus:
shellcheckclean on the extracted script;actionlintfinding count byte-identical to main (21 pre-existing info/style, 0 introduced);npx vitest run scripts/tests/qwen-pr-review-workflow.test.js→ 23/23.Evidence (Before & After)
Before: #8366 triage run —
Installing Qwen Code from npm: @qwen-code/qwen-code@0.21.2→Process completed with exit code 217, triage never ran. After: stub-driven transcript above; the real proof arrives with this PR's own triage run.Tested on
Environment (optional)
Extracted-step harness (
qwen review extract-step+ PATH stubs), bash; no model involved.Risk & Scope
Linked Issues
None. Failure observed on #8366's triage check; same fix family as c9f0d06.
中文说明
这个 PR 做了什么
修复 triage 任务的
Ensure qwen CLI步骤：从 registry 解析@latest、仅在过时时升级、在RUNNER_TEMP下执行全局安装、失败重试 3 次（带退避）、registry 或安装本身故障时回退到预装版本（且所有成功路径都照常输出 action 钉版所需的version）。workflow 中另外两处@latest安装点补上同样的 3 次重试。为什么需要
该步骤此前遇到任何预装 qwen 就短路——与 review runner 在 c9f0d06 修掉的完全是同一个 bug——且多一层后果：此步骤的输出喂给
qwen_cli_version，钉住 composite action 自己的无条件重装。于是过时镜像不仅永远保持旧版本，还决定了 action 重装什么。在 #8366 的 triage run 上实际观察到：action 安装被钉住的旧版0.21.2时在共享 runner（actions-runner-test-13）上以 npm 退出码 217 死掉，triage 完全没跑。重试吸收这类瞬态 npm 崩溃；RUNNER_TEMP工作目录关闭本 workflow 其他安装点早已注释过的.npmrc混入全局安装的隐患。Reviewer 测试计划
如何验证
改动后的步骤用仓库自己的
qwen review extract-step原样抽出，在 stub 的npm/qwen下驱动全部三条分支：另：抽出脚本
shellcheck干净；actionlint发现数与 main 逐字节一致（21 处既有 info/style，0 新增）；npx vitest run scripts/tests/qwen-pr-review-workflow.test.js→ 23/23。证据（Before & After）
Before：#8366 的 triage run——
Installing Qwen Code from npm: @qwen-code/qwen-code@0.21.2→Process completed with exit code 217，triage 未运行。After：上述 stub 驱动记录；真实验证由本 PR 自己的 triage run 完成。测试平台
环境（可选）
抽取步骤测试法（
qwen review extract-step+ PATH stub），bash；不涉及模型。风险与范围
关联 Issue
无。故障在 #8366 的 triage check 上观察到；与 c9f0d06 同一修复家族。