fix(ci): upgrade the review runner's qwen CLI to npm latest per run - #8265
Closed
wenshao wants to merge 4 commits into
Closed
fix(ci): upgrade the review runner's qwen CLI to npm latest per run#8265wenshao wants to merge 4 commits into
wenshao wants to merge 4 commits into
Conversation
Collaborator
Collaborator
|
Thanks for the PR! Template: the body uses its own headings (Why / What / Verification / Not in this PR) rather than the template's, but it covers everything the template asks for — what changed, why, how it was verified, and scope. Treating it as complete. ✓ Problem: observed, not theoretical. The fleet updater ( Direction: aligned. Stale review runners repost already-fixed output bugs, which is exactly the kind of noise this PR removes. No public-contract or product-surface impact — it's review infrastructure. Approach: minimal and well-shaped. Resolve Risk: no elevated risk signals (the Stage 1e revert-correlated paths don't apply to a workflow file). It is CI infrastructure that runs with secrets, but the script takes no PR-derived input, touches the same package + registry as before, and the worst case degrades to today's behavior (review on the installed version) or fails closed. Moving on to code review. 🔍 中文说明感谢贡献！ 模板： PR 正文用了自己的小标题（Why / What / Verification / Not in this PR），但模板要求的内容都覆盖了——改了什么、为什么、如何验证、范围。视为完整。✓ 问题： 已观测到，非理论性。fleet 更新器（ 方向： 对齐。停留在旧版本的 review runner 会重复发出已修复的输出 bug，正是本 PR 要消除的噪声。不涉及公开契约或产品面——属于 review 基础设施。 方案： 最小且设计良好。从 registry 解析 风险： 无升级风险信号（Stage 1e 的高回滚相关路径不适用于 workflow 文件）。它确实是带 secrets 运行的 CI 基础设施，但脚本不接收任何来自 PR 的输入，包与 registry 和之前一致，最坏情况退化为当前行为（用已安装版本 review）或失败关闭。 进入代码审查。🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewRead independently first, then against the diff — the implementation matches what I'd have written, and I didn't find a correctness or safety problem. The version comparison is sound: No injection surface: the script interpolates no PR-derived input — package name and registry are hardcoded — and Nothing blocking. The one carry-over from Stage 1 is the description's follow-up note ( Test evidenceThis is a workflow-only change; the PR's own CI can't exercise the new bash logic (it runs inside the review job that this very workflow bootstraps, on the self-hosted ECS runners — not in the unit suite). The real validation is the next PR-review run resolving The author reports verifying via Sandboxed lanes don't apply: CI on the reviewed commit (fetched once, not polled — the finalize job updates this table when CI settles):
Most checks are skipped, consistent with a workflow-only change that touches no application code; the ubuntu unit test is the gate still running and is expected to be unaffected. 中文说明代码审查先独立构思再对照 diff——实现与我会写的一致，未发现正确性或安全问题。 版本比较是可靠的： 无注入面：脚本不插入任何来自 PR 的输入——包名与 registry 均硬编码——且 无阻塞项。Stage 1 遗留的一点是描述里后续项说明（ 测试证据这是仅 workflow 的改动；PR 自身的 CI 无法执行新的 bash 逻辑（它运行在本 workflow 引导的 review 任务里、在自托管 ECS runner 上——不在单测套件中）。真正的验证是下一次 PR-review 运行时解析出 作者称通过 沙盒通道不适用： 被测 commit 的 CI（一次性获取，不轮询——CI 落定后 finalize 任务会更新此表）：多数检查被跳过，与仅改 workflow、未触及应用代码一致；ubuntu 单测是仍在运行的关口，预期不受影响。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — a tight, well-reasoned fix for a demonstrated problem; the only nit is the slightly off follow-up note about This one's easy to back. The problem is real and specific — a #8005 review posted by a runner still on 0.20.0 a week after the chunk-id fix shipped — and the root cause (install-if-missing + a fleet updater that only reaches The one thing I'd fix before this description gets copied elsewhere: Verdict: approve. The ubuntu unit test (the PR's own CI) is still in flight, so I'm not stamping it this second — approval is deferred until CI lands green on the reviewed commit, then it goes in pinned to that commit. 中文说明置信度：4/5 —— 针对一个已被证实的问题的紧凑、合理的修复；唯一的小问题是关于 这个 PR 很容易支持。问题真实而具体——#8005 的一条 review 由仍停留在 0.20.0 的 runner 发出，距离 chunk-id 修复发布已过去一周——根因（install-if-missing + 只覆盖 唯一想在这份描述被复制到别处之前修正的： 结论：批准。ubuntu 单测（PR 自身 CI）仍在运行，所以不立刻盖章——批准推迟到 CI 在被测 commit 上变绿后，再以 pinned 到该 commit 的方式提交。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Contributor
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 review job's install step short-circuited whenever a qwen binary already existed on the runner, and the fleet updater (update-ecs-runner-qwen.yml) only reaches the hosts carrying an ecs-update-* label. Any other ecs-qwen host keeps whatever version it was imaged with: #8005's review was posted by qwen 0.20.0 on ecs-qwen-runner-64c-11 a week after 0.21.x shipped, resurrecting the raw-chunk-id coverage disclosures that #7550 already fixed. Resolve latest from the registry and upgrade when the installed version is stale. Degraded paths warn instead of silently running old code: an unreachable registry or a failed install falls back to the installed qwen with a workflow warning, and a post-install version mismatch (a sudo-owned system install shadowing the per-run upgrade) is also surfaced as a warning.
wenshao
force-pushed
the
fix/ci-review-qwen-cli-update
branch
from
August 1, 2026 01:00
c2cc5bc to
c9f0d06
Compare
Collaborator
Author
|
@qwen-code /takeover |
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Collaborator
Author
OverviewTurns the review job's The diagnosis in the description is sound and well-evidenced: the old Main concern — please verify on a real ECS host before mergingThe upgrade is an unprivileged # The runner service resolves the system-wide qwen binary. Do not
# carry a runner user's custom npm prefix into the sudo install.
sudo env -u NPM_CONFIG_PREFIX npm install -g ... "@qwen-code/qwen-code@${VERSION}"That comment is direct evidence the qwen the runner resolves lives in a root-owned system prefix. So on exactly the hosts this PR targets, the new install lands in one of two states:
Both land on today's behavior plus an annotation. And in the second case the Note the old code path can't tell us whether an unprivileged Two ways out, either is fine:
A run log from an Suggestions
What's good
Follow-upsAgreed on both items in the description. One to add to the same batch: |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
Collaborator
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedMain concern — unprivileged
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+491
to
494
| if [ -n "$have" ] && [ "$have" = "$want" ]; then | ||
| echo "qwen $have (latest)" | ||
| exit 0 | ||
| fi |
Collaborator
There was a problem hiding this comment.
[Suggestion] This new step adds several bash branches (already-latest skip, registry-unreachable with/without an installed qwen, the sudo-then-unprivileged install fallback, both-fail with/without an installed qwen, the not-on-PATH error, and the version-shadowing warning) but none is covered by a test, even though this workflow already ships an extraction-based harness — scripts/tests/qwen-pr-review-workflow.test.js — that parses this YAML and exercises the real Run review bash against stubbed binaries. — Concrete cost: a future edit that breaks the [ "$have" = "$want" ] comparison (or a format drift between qwen --version and npm view … version) would either reinstall on every run or silently never upgrade a stale host — reproducing the exact stale-runner failure this PR fixes — while the suite stays green. Extending that harness to extract this step's run (the way retryLoopSource() does) and assert each branch's exit code and ::warning::/::error:: output would pin the behavior.
— qwen3.8-max-preview via Qwen Code /review
Collaborator
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8265One inline finding this round, from the automated reviewer. It is addressed. Feedback
Decision: implementedThis is an early review round, and the suggestion is valuable, codebase-consistent, and in scope: it pins precisely the stale-runner behavior the PR exists to fix, using the same extraction-based approach the file already uses for the review retry loop. No source/workflow change was needed — the step's ChangesExtended
The harness is load-bearing: mutating the workflow's Conflict notesNone — Verification
中文说明Autofix 审查轮次总结 — PR #8265本轮有一条来自自动审查器的行内意见，已处理。 反馈
决定：采纳实现当前处于早期审查轮次，该建议有价值、与代码库风格一致、且在范围内：它恰好固化了本 PR 存在的目的——防止陈旧 runner 的行为，并采用该文件已用于审查重试循环的同一种“抽取真实源码”方式。无需修改源码或工作流——该步骤的 改动扩展了
该夹具是“承重”的：将工作流中的 冲突说明无 —— 验证
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/模型 |
Collaborator
|
🔀 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 将重新运行。 |
qwen-code-ci-bot
approved these changes
Aug 1, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
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.
中文说明
未发现问题。LGTM！✅
未检查（工具限制，非阻断）：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-preview via Qwen Code /review
Collaborator
|
Thanks for digging into this. The stale-version symptom was real, but host inspection showed a different root cause, so I don't think the per-review upgrade should land. The The host configuration has now been repaired:
A regression review for #8005 is now running on Updating Qwen inside every review is the wrong ownership boundary here. Multiple runner services share one host, so concurrent reviews can race while mutating the same global npm installation. It also adds an npm-registry dependency to every review, and the unprivileged fallback can recreate runner-private installations. The review workflow should keep the existing install-if-missing behavior for hosted fallback; host provisioning/auditing should enforce the shared system PATH, and the release-to-fleet-updater trigger should keep the system installation current once per host. Closing this PR because the infrastructure issue has been corrected and the proposed implementation no longer matches the confirmed root cause. 中文说明感谢排查。Qwen 旧版本现象是真实的，但检查宿主机后确认根因与 PR 中的判断不同，因此不建议合入“每次 review 都升级 Qwen”的方案。
目前宿主机已完成修复：
#8005 的回归 review 已落到修复后的 每次 review 都执行 Qwen 升级不属于正确的维护边界。同一宿主机上有多个并发 runner，可能同时修改同一份全局 npm 安装；同时会让每次 review 都依赖 npm registry，非特权回退还可能重新制造 runner 私有安装。review workflow 应保留现有的缺失时安装逻辑用于 hosted fallback；宿主机 provisioning/审计负责保证统一系统 PATH，发布后的 fleet updater 每台宿主机更新一次全局安装。 由于基础设施配置已经修复，且当前实现不再对应已确认的根因，因此关闭此 PR。 |
Collaborator
|
One additional data point from real post-repair traffic: the runners are not merely finding an installed Qwen binary — they resolve the current npm The npm registry currently reports
For all three, workspace cleanup, checkout, and the Qwen availability step completed successfully before entering 中文说明补充一组修复后的真实任务证据：这些 runner 不只是能找到某个已安装的 Qwen，而是实际命中了 npm 当前的 npm registry 当前返回的
三条任务的 workspace 清理、checkout 和 Qwen 可用性检查都已成功，随后进入 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR #8005's review was posted in the pre-#7550 format — raw
chunk Ncoverage disclosures — a week after that fix shipped in v0.21.0. The run log shows why: the review executed onecs-qwen-runner-64c-11with qwen 0.20.0 (run 30672982031), because theInstall Qwen CLI if missingstep short-circuits whenever anyqwenbinary exists on the runner, and the fleet updater (update-ecs-runner-qwen.yml) only reaches the hosts carrying anecs-update-*label. Every otherecs-qwenhost keeps whatever version it was imaged with, indefinitely — so already-fixed output bugs keep resurfacing in posted reviews.What
Install Qwen CLI if missing→Install or update Qwen CLI:latestversion from the npm registry; if the installedqwen --versionalready matches, exit early (no reinstall on the fast path).npm install -gthe resolved version.::warning::, review proceeds on the installed version;::error::, fail;::warning::, review proceeds; no qwen → fail;qwen --versionstill not the wanted version (a sudo-owned system-wide install shadowing the per-run user-prefix upgrade) →::warning::naming both versions.Verification
bash -non the extracted step script.npm/qwencovering all five paths above (stale→upgrade, already-latest, registry-down×2, install-fail): output and exit codes match the table.Not in this PR (known follow-ups)
qwen-triage.yml(lines ~1117, ~2617) has the same install-if-missing pattern.update-ecs-runner-qwen.yml'srepository_dispatch: npm-publishedtrigger has no sender anywhere in the repo — the fleet updater only ever runs via manual dispatch.中文说明
背景
PR #8005 的 review 以 #7550 修复前的旧格式发出(裸的
chunk N覆盖披露),而该修复随 v0.21.0 发布已一周多。运行日志给出了原因:这次 review 在ecs-qwen-runner-64c-11上以 qwen 0.20.0 执行(run 30672982031)。Install Qwen CLI if missing步骤只要 runner 上存在任意qwen二进制就直接短路退出,而集群更新器(update-ecs-runner-qwen.yml)只覆盖带ecs-update-*label 的机器。其余ecs-qwen机器会无限期停留在装机时的版本——已修复的输出 bug 因此不断在已发布的 review 中复活。改动
Install Qwen CLI if missing→Install or update Qwen CLI:latest的具体版本号;本机qwen --version已一致则提前退出(快路径不重复安装)。npm install -g到解析出的版本。::warning::,用已装版本继续 review;::error::,失败;::warning::,继续;未装则失败;qwen --version仍不是目标版本(sudo 安装的系统级二进制 shadow 了本次用户级升级)→::warning::同时列出两个版本号。验证
bash -n语法检查。npm/qwen对上述五条路径(过期升级、已是 latest、registry 不可达 ×2、安装失败)做了功能测试:输出与退出码全部符合预期。不在本 PR 内(已知后续项)
qwen-triage.yml(约 1117、2617 行)存在同样的 install-if-missing 模式。update-ecs-runner-qwen.yml的repository_dispatch: npm-published触发器在仓库内没有任何发送方——集群更新器实际只能靠手动 dispatch 运行。