ci: auto-update ECS runners on stable publish and harden update job - #8343
Conversation
The update-ecs-runner-qwen workflow declares a repository_dispatch npm-published trigger, but nothing in the repo ever sent that event, so the self-hosted ECS runners only picked up new qwen releases when someone remembered to run the update workflow by hand. Emit the dispatch from the release workflow after a stable (npm_tag=latest) publish, carrying the released version; dispatch failure only warns so an already-published release cannot be failed by it. Also harden the update job against npm ENOTEMPTY rename failures: all runner processes of a region share one machine, so a concurrent global npm install from another job can race the npm rename of the package dir. Clear stale npm trash dirs and retry up to three times with backoff.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with strong evidence — specific run IDs (30711479656, 30723886544), machine name, and timestamps document a three-part failure chain: missing dispatch sender, cache-driven downgrade via Direction: CI infrastructure for the project's own ECS runners — squarely within scope. The release workflow change is additive ( Size: not applicable — no core paths touched. All changes are in Approach: three focused changes, each mapping 1:1 to a documented failure link. The scope feels right — cutting any one of them would leave a link in the chain unpatched. No unrelated changes or drive-by refactors. Risk: no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分——具体 run ID（30711479656、30723886544）、机器名、时间戳记录了三段式故障链：缺少 dispatch 发送方、 方向：项目自有 ECS runner 的 CI 基础设施——完全在范围内。release workflow 改动是附加的（ 规模：不适用——未触及核心路径。所有改动在 方案：三处聚焦改动，每处对应一个已记录的故障环节。范围合理——砍掉任何一处都会留下未修复的故障链。无无关改动或顺手重构。 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the three-part failure chain (no dispatch sender → cache-driven downgrade → ENOTEMPTY trash blocking recovery), I would: (1) add a Findings: clean. A few observations, none blocking:
No correctness bugs, security holes, or convention violations found. TestingThis is a CI/workflow-only change with no user-visible behavior — real-scenario testing is N/A. CI evidence from the PR's own checks on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The ubuntu unit suite passed. macOS/Windows tests and integration tests were skipped (expected for a workflow-only change). The web-shell E2E smoke test is still running — it exercises the web shell package, not workflow files, so it is not expected to be affected. 中文说明代码审查独立方案： 针对三段式故障链（无 dispatch 发送方 → 缓存驱动降级 → ENOTEMPTY 残留阻塞恢复），我的方案是：(1) 在 release workflow 添加仅限 stable 发布的 发现： 干净。几点观察，均非阻塞：
未发现正确性 bug、安全漏洞或规范违反。 测试纯 CI/workflow 改动，无用户可见行为——真实场景测试不适用。 CI 证据来自 PR 自身在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. The PR documents a real, observed failure chain with run IDs and timestamps, then fixes each link with a minimal, well-gated change. The dispatch step can't fail a release, the retry loop handles the race window correctly, and the version pin degrades gracefully. Tests pin the right invariants — especially the three-clause dispatch gate, which is the only thing standing between a nightly build and the ECS fleet. No scope creep, no over-engineering. If I had to maintain this in six months, I'd thank the author for the comments explaining why each gate exists. Approval deferred until CI lands green on 中文说明置信度：5/5 —— 每个阶段都干净，毫不犹豫地合并。 PR 用 run ID 和时间戳记录了真实的故障链，然后用最小化、门控良好的改动修复了每个环节。dispatch 步骤不会导致发布失败，重试循环正确处理了竞态窗口，版本固定优雅降级。测试固定了正确的不变量——尤其是三条件 dispatch 门控，它是阻止 nightly 构建被推到 ECS 集群的唯一保护。无范围蔓延，无过度工程。如果半年后维护这些代码，我会感谢作者用注释解释了每个门控存在的原因。 审批推迟至 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. |
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 — CI landed green after the review. ✅
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/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
| echo "::warning::npm install attempt ${attempt} failed; retrying" | ||
| sleep $((attempt * 10)) |
There was a problem hiding this comment.
[Suggestion] The retry loop body runs unconditionally on every iteration, including the last. When the third install fails it still logs ::warning::npm install attempt 3 failed; retrying (stating a retry that never happens), then sleeps 30s even though no fourth attempt follows, and falls through to exit 1 with no terminal annotation. — Concrete cost: on a sustained failure an oncall sees "retrying" as the final word and waits an extra 30s for a retry that never comes, with nothing stating the retries were exhausted; the house pattern in web-shell-visuals-publish.yml follows its loop with an explicit ::error::... after retries line, which this loop omits.
| echo "::warning::npm install attempt ${attempt} failed; retrying" | |
| sleep $((attempt * 10)) | |
| if [[ "${attempt}" -lt 3 ]]; then | |
| echo "::warning::npm install attempt ${attempt} failed; retrying" | |
| sleep $((attempt * 10)) | |
| else | |
| echo "::error::npm install of @qwen-code/qwen-code@${VERSION} failed after 3 attempts" | |
| fi |
中文说明
[Suggestion]（建议）重试循环体在每次迭代（包括最后一次）都无条件执行。当第三次安装失败时，仍会输出 ::warning::npm install attempt 3 failed; retrying（声明了一个永远不会发生的重试），然后在并不会出现第四次尝试的情况下 sleep 30 秒，并以 exit 1 结束且没有任何终止标注。— 具体代价：在持续性故障下，oncall 看到的最后一句话是 "retrying"，并会多等 30 秒等待一个永不到来的重试，且没有任何内容说明重试已耗尽；仓库中的同类写法（web-shell-visuals-publish.yml）会在循环后跟一行明确的 ::error::... after retries，而此循环遗漏了。
— qwen3.8-max-preview via Qwen Code /review
| ${{ github.repository == 'QwenLM/qwen-code' && | ||
| needs.prepare.outputs.is_dry_run == 'false' && | ||
| needs.prepare.outputs.npm_tag == 'latest' }} |
There was a problem hiding this comment.
[Suggestion] This if: gate is the sole protection preventing a nightly/preview/dry-run release from firing the fleet-moving npm-published dispatch, but nothing pins it. The repo has an established convention of testing analogous workflow safety invariants (e.g. scripts/tests/install-script.test.js asserts the OSS "latest" pointer flips only after every asset is verified; scripts/tests/qwen-fleet-shepherd-workflow.test.js executes extracted bash to prove gate/dry-run logic). — Failure scenario: a future edit that flips npm_tag == 'latest' to != 'latest', or drops the is_dry_run == 'false' clause, would pass review green and move the ECS fleet to a non-stable version — the exact outcome the step's own comment forbids; the triggered update workflow installs whatever version it is handed, so there is no downstream guard. Consider adding a workflow-content test (e.g. scripts/tests/release-workflow.test.js) asserting this step's if: contains all three clauses. (The gate as written is correct — per AGENTS.md a missing test is a Suggestion, not a blocker.)
中文说明
[Suggestion]（建议）这个 if: 门控是阻止 nightly/preview/dry-run 发布触发会移动 runner 集群的 npm-published dispatch 的唯一防护，但没有任何测试 pin 住它。仓库已有测试同类工作流安全不变量的约定（例如 scripts/tests/install-script.test.js 断言 OSS "latest" 指针仅在全部资产校验后才翻转；scripts/tests/qwen-fleet-shepherd-workflow.test.js 会执行抽取出的 bash 以验证门控/dry-run 逻辑）。— 失败场景：未来某次编辑把 npm_tag == 'latest' 改成 != 'latest'，或删掉 is_dry_run == 'false' 子句，都能绿通过审并把 ECS 集群切到非稳定版本——正是该步骤自身注释所禁止的结果；被触发的 update workflow 会安装它收到的任何版本，因此没有下游防护。建议新增一个 workflow 内容测试（如 scripts/tests/release-workflow.test.js），断言此步骤的 if: 包含全部三个子句。（门控本身是正确的——按 AGENTS.md，缺少测试属于 Suggestion，而非 blocker。）
— qwen3.8-max-preview via Qwen Code /review
| continue-on-error: true | ||
| env: | ||
| GITHUB_TOKEN: '${{ secrets.CI_BOT_PAT }}' |
There was a problem hiding this comment.
[Suggestion] With continue-on-error: true, a dispatch failure is masked as a successful step, so the publish job completes success and the notify_failure job (gated on needs.publish.result == 'failure') never fires. — Failure scenario: if gh api .../dispatches fails (transient GitHub 5xx, or an expired/rotated CI_BOT_PAT), the package is published and the release shows green, but the npm-published dispatch never fires, so the ECS fleet silently stays on the previous qwen version indefinitely — reproducing the exact problem this PR set out to fix. The only signal is a ::warning:: annotation on a step inside an otherwise-green run, routed to no one, so the documented "re-run the update workflow manually" fallback is never prompted. Keeping continue-on-error is the right call (a dispatch failure must not fail an already-published release); consider making the fallback reachable — e.g. emit ::error:: so it surfaces in run-level failure digests, or record explicitly at the notify_failure condition that this step's failures are deliberately invisible and the fleet must be reconciled via dispatch history.
中文说明
[Suggestion]（建议）由于 continue-on-error: true，dispatch 失败会被掩盖为成功步骤，因此 publish job 以 success 完成，而 notify_failure job（以 needs.publish.result == 'failure' 为门控）永远不会触发。— 失败场景：如果 gh api .../dispatches 失败（GitHub 瞬时 5xx，或 CI_BOT_PAT 过期/被轮换），包已发布、release 显示为绿色，但 npm-published dispatch 从未发出，于是 ECS 集群会无限期地静默停留在上一个 qwen 版本——恰好重现了本 PR 要解决的问题。唯一的信号是一个绿色 run 中某步骤上的 ::warning:: 标注，它不会通知任何人，因此文档中"手动重跑 update workflow"的兜底永远不会被触发。保留 continue-on-error 是对的（dispatch 失败不应拖败一个已发布的 release）；建议让这个兜底可达——例如输出 ::error:: 使其出现在 run 级失败摘要中，或在 notify_failure 的门控处明确记录：此步骤的失败是刻意对告警路径不可见的，需要通过 dispatch 历史来核对集群版本。
— qwen3.8-max-preview via Qwen Code /review
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
The triage job's qwen-code-action runs an unconditional global npm install with --prefer-offline on every run. On the shared self-hosted ECS box that resolves the latest dist-tag from the persistent npm cache, which lags npm publishes: after the runners were updated to 0.21.3, the next triage job resolved latest as the cached 0.21.2 and downgraded the box, which then also re-created the stale npm trash dir that blocks the update workflow with ENOTEMPTY. Capture the version the Ensure qwen CLI step verified and pass it as qwen_cli_version, so the action's redundant reinstall targets the exact installed version (no dist-tag resolution) and can never downgrade the shared runner.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8343Addressed all three automated-reviewer suggestions. No Critical findings were Feedback points
Conflict notesNone — Verification
中文说明Autofix 审查轮次 — PR #8343已处理自动审查器提出的全部三条建议。本次没有 Critical（阻断级）发现（审查结论为“无阻断问题”），也没有需要解决的 base 冲突（ 反馈要点
冲突说明无 — 验证
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; the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.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; the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| // The packages are published before this step runs, so it must not fail | ||
| // the release; but the failure must still surface (as an error, not a | ||
| // warning) so the fleet can be reconciled via a manual re-run. | ||
| expect(workflow).toContain('continue-on-error: true'); |
There was a problem hiding this comment.
[Suggestion] The continue-on-error: true assertion is unscoped. release.yml contains two continue-on-error: true lines — the pre-existing 'Auto-label internal CI PRs' step (line 528) and this new dispatch step (line 623) — and toContain matches anywhere in the whole-file string, so this assertion is already satisfied by the auto-label step and cannot detect removal of continue-on-error: true from the dispatch step. A probe confirmed it: deleting only release.yml:623 leaves this test green. — Failure scenario: a future edit drops continue-on-error: true from just the dispatch step; the test still passes via line 528, and on the next stable release a transient gh api dispatch failure then fails the publish job after the packages are already published — the regression this test's comment claims to guard.
| expect(workflow).toContain('continue-on-error: true'); | |
| expect(workflow).toContain( | |
| 'continue-on-error: true\n' + | |
| ' env:\n' + | |
| " GITHUB_TOKEN: '${{ secrets.CI_BOT_PAT }}'", | |
| ); |
中文说明
continue-on-error: true 断言未限定作用范围。release.yml 中有两处 continue-on-error: true——已有的 'Auto-label internal CI PRs' 步骤（第 528 行）与本 dispatch 步骤（第 623 行）——而 toContain 匹配整个文件字符串的任意位置，因此该断言已被 auto-label 步骤满足，无法检测 dispatch 步骤的 continue-on-error: true 被删除。探针已证实：仅删除 release.yml:623 后该测试仍为绿色。— 失败场景：未来某次编辑仅从 dispatch 步骤移除 continue-on-error: true，测试仍因第 528 行通过；下一次 stable 发布时一次瞬时的 gh api dispatch 失败会在包已发布之后让 publish job 失败——正是本测试注释声称要防的回归。上面的 suggestion 把断言限定到 dispatch 步骤独有的片段。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| # --prefer-offline and resolves the dist-tag from the runner's | ||
| # persistent npm cache, which lags npm publishes and has downgraded | ||
| # the shared ECS box back to a stale release. | ||
| qwen_cli_version: '${{ steps.ensure_qwen.outputs.version }}' |
There was a problem hiding this comment.
[Suggestion] The triage version-pin — this PR's central bugfix — has no test, while the PR adds readFileSync/string-match tests for the other two workflow changes and the repo has an established convention of pinning workflow invariants this way (scripts/tests/qwen-triage-workflow.test.js, ~5500 lines, reads this same workflow into workflow). A repo-wide grep for qwen_cli_version / ensure_qwen / the version=$(qwen --version) capture matches only the action's docs — nothing in scripts/tests/. — Failure scenario: a future edit deletes the qwen_cli_version: line, or breaks the echo "version=$(qwen --version)" >> "${GITHUB_OUTPUT}" capture so the output resolves empty; the action's unconditional global reinstall then silently falls back to latest resolved through the runner's stale npm cache — the exact downgrade this PR fixes — with no failing test. Suggested fix (in scripts/tests/qwen-triage-workflow.test.js):
it('pins the action reinstall to the version the job already runs', () => {
expect(workflow).toContain("id: 'ensure_qwen'");
expect(workflow).toContain(
'echo "version=$(qwen --version)" >> "${GITHUB_OUTPUT}"',
);
expect(workflow).toContain(
"qwen_cli_version: '${{ steps.ensure_qwen.outputs.version }}'",
);
});中文说明
triage 的版本固定——本 PR 的核心修复——没有测试；而本 PR 为另外两处 workflow 改动都加了 readFileSync/字符串匹配测试，且仓库已有用这种方式固定 workflow 不变量的约定（scripts/tests/qwen-triage-workflow.test.js，约 5500 行，读取的正是同一个 workflow 并存入 workflow）。全仓库 grep qwen_cli_version / ensure_qwen / version=$(qwen --version) 捕获，只命中该 action 的文档，scripts/tests/ 中没有任何命中。— 失败场景：未来某次编辑删除 qwen_cli_version: 行，或破坏 echo "version=$(qwen --version)" >> "${GITHUB_OUTPUT}" 捕获使输出解析为空；action 的无条件全局重装会静默回退到经 runner 陈旧 npm 缓存解析的 latest——正是本 PR 修复的降级——且没有任何测试失败。建议的修复（在 scripts/tests/qwen-triage-workflow.test.js）见上方代码块。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| " needs.prepare.outputs.is_dry_run == 'false' &&\n" + | ||
| " needs.prepare.outputs.npm_tag == 'latest' }}", | ||
| ); | ||
| expect(workflow).toContain("-f 'event_type=npm-published'"); |
There was a problem hiding this comment.
[Suggestion] This dispatch test pins the event type but not the version payload. The payload line -f "client_payload[version]=${RELEASE_VERSION}" (release.yml:631) is the load-bearing half of the release→update integration: the update workflow reads github.event.client_payload.version and falls back to @qwen-code/qwen-code@latest when it is empty (update-ecs-runner-qwen.yml:36-39). A probe confirmed it: deleting the payload line leaves this test green. — Failure scenario: a future edit drops the payload line; the test stays green (event type still pinned) but the dispatch no longer carries the released version, so the fleet installs whatever latest resolves to instead of the just-published release — a wrong-version fleet update whenever the two diverge.
| expect(workflow).toContain("-f 'event_type=npm-published'"); | |
| expect(workflow).toContain("-f 'event_type=npm-published'"); | |
| expect(workflow).toContain( | |
| '-f "client_payload[version]=${RELEASE_VERSION}"', | |
| ); |
中文说明
该 dispatch 测试只固定了事件类型，未固定版本 payload。payload 行 -f "client_payload[version]=${RELEASE_VERSION}"（release.yml:631）是 release→update 集成的承重一半：update workflow 读取 github.event.client_payload.version，为空时回退到 @qwen-code/qwen-code@latest（update-ecs-runner-qwen.yml:36-39）。探针已证实：删除该 payload 行后测试仍为绿色。— 失败场景：未来某次编辑删除 payload 行，测试仍绿（事件类型仍被固定），但 dispatch 不再携带发布版本，runner 集群会安装 latest 解析出的版本而非刚发布的版本——两者不一致时即一次错误版本的集群更新。上面的 suggestion 同时固定事件类型与版本 payload。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Address Review Summary — PR #8343Feedback points and decisionsrc:3697143050 — Scope 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/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.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; the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/update-ecs-runner-qwen.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review
| it('annotates a retry and a terminal failure distinctly', () => { | ||
| // The final attempt must not log a "retrying" warning that never | ||
| // retries; a sustained failure ends with an explicit exhausted error. |
There was a problem hiding this comment.
[Suggestion] This test's comment claims to guard that "the final attempt must not log a 'retrying' warning that never retries", but the two toContain assertions only pin the two echo strings as substrings anywhere in the file — the loop bound, the -lt 3 guard placement, and the trash-cleanup line are unpinned. — Failure scenario: moving echo "::warning::...retrying" above the if [[ "${attempt}" -lt 3 ]] guard makes attempt 3 log a "retrying" warning that never retries (the exact bug the comment names) while both assertions stay green; for attempt in 1 2 3 → 1 2 ships green while the error text still says "after 3 attempts"; and deleting sudo rm -rf "${PKG_DIR}"/.qwen-code-* is caught by no test. All three mutants were verified to ship green by a mutation probe. Pin the structure alongside the messages:
expect(workflow).toContain('for attempt in 1 2 3; do');
expect(workflow).toContain('if [[ "${attempt}" -lt 3 ]]; then');
expect(workflow).toContain('sudo rm -rf "${PKG_DIR}"/.qwen-code-*');中文说明
此测试的注释声称要保护“最后一次尝试不会记录一个永不重试的 'retrying' 警告”，但两个 toContain 断言仅把两条 echo 字符串作为文件任意位置的子串固定下来——循环上界、-lt 3 守卫的位置以及垃圾清理行都未被固定。— 失败场景：把 echo "::warning::...retrying" 移到 if [[ "${attempt}" -lt 3 ]] 守卫之上，会使第 3 次尝试记录一个永不重试的 "retrying" 警告（正是注释所指出的 bug），而两个断言仍为绿；for attempt in 1 2 3 → 1 2 在错误文案仍写 "after 3 attempts" 的情况下绿灯通过；删除 sudo rm -rf "${PKG_DIR}"/.qwen-code-* 也没有任何测试能捕获。以上三种变异均经变异探针验证可绿灯通过。建议在固定消息的同时固定结构：
expect(workflow).toContain('for attempt in 1 2 3; do');
expect(workflow).toContain('if [[ "${attempt}" -lt 3 ]]; then');
expect(workflow).toContain('sudo rm -rf "${PKG_DIR}"/.qwen-code-*');— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[rc:3697396797] Pin retry-loop structure in workflow test — ImplementedFeedback: The retry-annotation test only pinned the two echo strings as substrings, leaving the loop bound ( Change: Added three structural
Files changed: No conflicts. Verification
中文说明已处理的审查反馈[rc:3697396797] 在工作流测试中固定重试循环结构 — 已实现反馈： 重试注解测试仅将两条 echo 字符串作为子串固定，循环上界（ 变更： 在现有测试中新增三条结构性
变更文件： 无冲突。 验证
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/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — dispatch gating is correct (stable only, non-dry-run, QwenLM/qwen-code), security surface clean (payload from package.json, version via npm view, trash path hardcoded). Retry logic with backoff is sound.
Non-blocking: add exit 0/exit 1 control-flow assertions to the update workflow test.
|
@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: 69 passed · 0 failed · 69 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：69 通过 · 0 失败 · 69 总计 Verification reportPR #8343 Deep Verification —
|
| Scenario (stub npm fails on…) | Arm | exit | npm calls | cleanups | warnings | terminal err | sleeps | oracle |
|---|---|---|---|---|---|---|---|---|
| A: attempt 1 | base | 217 | 1 | – | – | – | – | blocked forever (the bug) |
| A: attempt 1 | head | 0 | 2 | 2 | 1 | 0 | 10 |
recovered ← load-bearing flip |
| D: attempts 1,2 | base | 217 | 1 | – | – | – | – | blocked |
| D: attempts 1,2 | head | 0 | 3 | 3 | 2 | 0 | 10 20 |
recovered on attempt 3 |
| B: attempts 1,2,3 | base | 217 | 1 | – | – | – | – | blocked |
| B: attempts 1,2,3 | head | 1 | 3 | 3 | 2 | 1 | 10 20 |
clean terminal failure |
| C: none (clean) | base | 0 | 1 | – | – | – | – | ok |
| C: none (clean) | head | 0 | 1 | 1 | 0 | 0 | – | no regression |
27/27 assertions pass (harness/ab-retry.sh, log logs/ab-retry.txt). The two load-bearing flips are A and D (217 → 0). Scenario B confirms the PR's own test claim that the final attempt logs a terminal ::error::…failed after 3 attempts and not a "retrying" warning (2 warnings, not 3). Scenario C confirms cleanup still runs once and no needless retry occurs on the happy path. Witness: evidence/01-ab-retry-loop-base-vs-head.png.
Verification notes (not corrections)
- The triage version pin's accepting end is documented, not assumed.
qwen_cli_versionis passed as awith:input (notenv:) toQwenLM/qwen-code-action@6d08e91…. This repo's own autogenerated action docs (docs/users/integration-github-action.md,BEGIN_AUTOGEN_INPUTS) listqwen_cli_versionas a declared input — "Optional, default:latest… a specific version number, or a git branch, tag, or commit." The triage step captures a clean0.21.3(measured below), which is exactly "a specific version number," so the action installs that exact version with no dist-tag resolution. This matters because the same workflow's comment documents that an unrecognized input (settings_json) was previously silently dropped — hadqwen_cli_versionnot been a real input, the pin would silently no-op and the downgrade would continue. It is real. - The dispatch gate reads populated outputs.
needs.prepare.outputs.{is_dry_run,npm_tag,release_version}all exist and are wired to real step outputs (steps.vars.outputs.is_dry_run,steps.version.outputs.NPM_TAG,steps.version.outputs.RELEASE_VERSION), so the gate fires for real stable publishes rather than evaluating empty strings to a silent no-op.
Secondary claims (wire-oracle + wiring)
Triage Ensure qwen CLI version capture and release Trigger ECS runner qwen update dispatch were extracted verbatim and executed (harness/wire-oracle.sh, log logs/wire-oracle.txt; 17/17 pass). Witness: evidence/02-wire-oracle-triage-dispatch.png.
- Installed branch (production path): ran the extracted step against the real
qwenin this container (/usr/local/bin/qwen, the lane's own runtime).qwen --versionprints exactly0.21.3\n— single line, no ANSI, empty stderr, exit 0 — andGITHUB_OUTPUTreceives exactly oneversion=0.21.3line, a valid exact npm spec. - Not-installed branch: with
qwenabsent from a minimal PATH,npm install -g --loglevel=error --no-audit @qwen-code/qwen-code@latestis invoked once, then the version is captured (0.21.3). - Dispatch success: exit 0, no
::error::; the stubghreceived exactlyapi repos/QwenLM/qwen-code/dispatches --method POST -f event_type=npm-published -f client_payload[version]=0.21.3. - Dispatch failure: exit 1, emits
::error::npm-published dispatch failed;…'Update ECS Runner Qwen'…(surfaces, whilecontinue-on-error: truekeeps the already-published release from failing).
Cross-workflow wiring was parsed from YAML, not grepped (harness/wiring.mjs, log logs/wiring.txt; 20/20 pass). Witness: evidence/03-cross-workflow-wiring.png.
- Triage: a step
id: ensure_qwenwritesversion=$(qwen --version)toGITHUB_OUTPUT; the same job'sRun Qwen Triageaction step consumes it aswith.qwen_cli_version = ${{ steps.ensure_qwen.outputs.version }}, ordered afterensure_qwen. - Release↔update: dispatch sends
event_type=npm-publishedand the update workflow'son.repository_dispatch.typesincludesnpm-published; the payload key matches on both sides — release writesclient_payload[version], update'sResolve versionreadsgithub.event.client_payload.version(keyversion==version), falling back to@latestwhen empty. - Gate: all three clauses present (
github.repository == 'QwenLM/qwen-code',is_dry_run == 'false',npm_tag == 'latest'),continue-on-error: true, andGITHUB_TOKEN: secrets.CI_BOT_PAT(correct — the defaultGITHUB_TOKENcannot sendrepository_dispatch).
Vacuity check on the new tests
The 4 tests this PR adds are string-containment assertions (expect(workflow).toContain(…)) — they prove the YAML text changed, not that the bash behaves. They are nonetheless non-vacuous: with the three workflow files reverted to base in a scratch worktree, all 4 fail for the right reason (each failure names the exact PR-added string it asserts), while the pre-existing update test still passes (valid control). Witness: evidence/04-vacuity-new-tests-vs-base.png; log logs/vacuity.txt.
| Test (added by PR) | Result on base | Failure names |
|---|---|---|
| release › dispatch gate (3 clauses) | FAIL (load-bearing) | `if: |
release › continue-on-error + ::error:: |
FAIL (load-bearing) | continue-on-error: true … env: |
| update › retry loop + terminal error | FAIL (load-bearing) | echo "::warning::npm install attempt … |
| triage › ensure_qwen version pin | FAIL (load-bearing) | id: 'ensure_qwen' |
| update › installs without npm prefix (pre-existing control) | PASS (control valid) | — |
At HEAD the full suite is green: 124/124 tests pass across the 3 files (logs/workflow-tests-head.txt). The behavioral evidence above is what the string tests cannot provide.
Findings
No blocking or correctness findings. Every executed assertion passed; the central claim is proven load-bearing and both secondary claims are proven at the bash + wiring level.
Informational notes (not merge conditions):
- SC2312 on the triage capture is mitigated, not a defect.
echo "version=$(qwen --version)"raises shellcheck's optional SC2312 (command substitution can mask a return value). In context it cannot: the preceding bareqwen --versionline runs underset -euo pipefail. Measured — with a stubqwenwhose--versionexits 1, the step exits 1 andGITHUB_OUTPUTis empty, i.e.set -estops the step at the bare line before the echo. (logs/shellcheck-wrapped.txt.) - Version pin is triage-only by design. As the PR's Risk section states, triage jobs no longer self-upgrade a stale box; recovery is the update workflow's job (which this PR unblocks). Accepted tradeoff, documented.
Not covered
- End-to-end dispatch trigger. A real
repository_dispatchfiring the update workflow only happens on the next stable publish; it cannot be exercised here (no token, no real release). The bash, the gating, and the cross-workflow payload wiring are verified; the live trigger is not. - The literal concurrent-install race was not reproduced. The A/B replays the documented failure shape (stub npm emitting the exact
ENOTEMPTY … .qwen-code-g9EoLCuD, exit 217, permanent block on base) — not the real trigger (twonpm install -gprocesses racing npm's rename). The referenced failing run logs (30711479656, 30723886544) are not retrievable without a token, so the replay is uncalibrated against real CI logs; it proves the handling, not the trigger. - The action's internal install code was not executed (separate repo, no token). Confirmed via this repo's autogenerated docs that
qwen_cli_versionis a declared input accepting a specific version, but the action's actualnpm installpath (whether it still passes--prefer-offlineonce pinned) was not run. - Per-commit attribution is out of reach. The checkout is depth 2;
git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata lists 7. Verified the aggregateHEAD^1..HEADdiff; individual commits were not exercised separately. - yamllint not run — the
node:22-bookwormcontainer shipspython3without thepipmodule (measured:No module named pip), soyamllintcannot be installed. YAML validity is proven instead byjs-yamlparsing all three workflows (extractor + wiring harness). - Repo-wide gates not run. Targeted the changed surface only: eslint clean on the 3 changed test files, shellcheck clean on the changed run blocks, actionlint clean on the 3 changed workflows (head and base both 0 issues). Repo-wide
npm run lint:ci/ shellcheck / yamllint were not run. - Other action-consuming workflows (
qwen-ci-flaky-rerun,qwen-issue-followup-bot,qwen-code-pr-review) use the same action SHA without the version pin. Whether they run on the shared self-hosted ECS box (where the persistent-cache downgrade applies) was not investigated; the PR scopes the pin to the triage job.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at the merge commit 55b2be7a1 (depth 2), npm ci + npm run build already complete. This container is a live sample of the triage/verify lane runtime, so the triage Ensure qwen CLI step was measured against the real installed qwen (0.21.3), not a guess. Each changed step's run: block was extracted verbatim with js-yaml (harness/extract.mjs) from HEAD and HEAD^1 and written to .sh files; the harnesses then drove them via bash --noprofile --norc (GitHub's runner contract) with the step's own set -euo pipefail, stubbing only the external commands (sudo, npm, sleep, gh) on a controlled PATH (absolute, since the script cds away). Base is a clean control: the extracted base block calls only the stubs and the PR changes no dependency manifest. Linters used the repo's pinned versions installed by node scripts/lint.js --setup (actionlint 1.7.12, shellcheck 0.11.0); each gate was proven live by planting a violation it caught before the clean result was trusted (evidence/05-lint-gates.png). Raw per-harness output lives in logs/; rerunnable harnesses in harness/. Assertion counts: ab-retry.sh 27 + wire-oracle.sh 17 + wiring.mjs 20 + vacuity.sh 5 = 69 pass / 0 fail.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Review Summary — PR #8343Verdict: No blockers. LGTM. Changes reviewed
Minor observations (non-blocking)
Risk assessmentLow. All three changes degrade gracefully to the status quo (manual update) on failure. No user-facing code paths are touched. The |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
Released in v0.21.4. |
5 participants
What this PR does
Three changes that keep the self-hosted ECS runners' qwen CLI current and stop them from being silently downgraded. First, the Release workflow now emits a
repository_dispatch(npm-published, carrying the released version) after a successful stable publish, so the existing Update ECS Runner Qwen workflow fires automatically instead of waiting for a manual run; the step is gated to real stable publishes (npm_tag == 'latest', non-dry-run, QwenLM/qwen-code only), excludes nightly/preview so the fleet is never moved to a prerelease, and iscontinue-on-errorwith a warning so an already-published release can never be failed by it. Second, the update job now clears stale npm trash dirs (/usr/local/lib/node_modules/@qwen-code/.qwen-code-*) and retries the global install up to three times with backoff, so leftover trash from crashed installs and concurrent-install races can no longer block it withENOTEMPTY. Third, the triage job now pins the qwen-code-action's unconditional global reinstall to the exact version the job already runs (captured from the Ensure qwen CLI step added by #8337), so the action can never downgrade the shared runner.Why it's needed
Investigation into why the runners kept serving 0.21.2 after 0.21.3 shipped found a three-part failure chain. (1) The
repository_dispatch: ['npm-published']trigger on the update workflow has never had a sender anywhere in the repo — all ~30 historical runs were manualworkflow_dispatch, so after every npm publish the runners kept serving the previous version until someone ran the update by hand. (2) The qwen-code-action used by the triage job runsnpm install --global --prefer-offline @qwen-code/qwen-code@latestunconditionally on every run; on the shared ECS box--prefer-offlineresolves thelatestdist-tag from the persistent npm cache, which lags npm publishes — after the box was updated to 0.21.3 at 17:28 UTC (verified by a job at 18:17), triage jobs resolvedlatestas the cached 0.21.2 and downgraded the box again (every job from 21:05 onward printed 0.21.2 on the same machineiZt4ne2fxw78cka1n2cbkoZ). #8337 added an if-missing pre-step but the action's internal install still runs unconditionally, so it does not stop the downgrade. (3) Each downgrade/re-upgrade cycle leaves npm trash dirs behind, and the trash name npm generates for a given package is deterministic — the 17:58 and 23:43 update failures both died on the identical rename target.qwen-code-g9EoLCuD, so once that stale dir exists the update workflow is blocked forever (ENOTEMPTY, exit 217) and cannot self-recover.Reviewer Test Plan
How to verify
Confirm the dispatch step gating: it fires only for real stable publishes from QwenLM/qwen-code (
npm_tag == 'latest', non-dry-run) and iscontinue-on-error, and the payload matches what the update workflow reads (github.event.client_payload.version←client_payload[version]=<release_version>; an empty version falls back to npmlatest). Confirm the update job's retry loop exits 0 on first success, cleans/usr/local/lib/node_modules/@qwen-code/.qwen-code-*before each attempt, and fails the step after three failed attempts, with the existing Verify version step still asserting the final version. Confirm the triage job captures the installed version in the Ensure qwen CLI step (id: ensure_qwen,versionoutput) and passes it asqwen_cli_version, so the action's reinstall targets an exact version (no dist-tag resolution, no cache-driven downgrade) — this composes with #8337's step rather than replacing it. After merge: re-run the Update ECS Runner Qwen workflow once to clear the stale trash dir and restore 0.21.3 on both machines; the next stable release should then dispatch the update automatically, and triage jobs must never change the installed version again.Evidence (Before & After)
N/A — CI/workflow-only change with no user-visible UI. Before: 0.21.3 published at 17:22 UTC, manual update at 17:28 verified 0.21.3, triage jobs downgraded the box back to 0.21.2 by 21:05 (stale npm cache +
--prefer-offline), and the 17:58 and 23:43 manual update re-runs both failed with the identicalENOTEMPTY ... '.qwen-code-g9EoLCuD'rename error (runs 30711479656, 30723886544). After: stable publishes dispatch the update automatically, the update job clears stale trash and retries through concurrent-install races, and the triage action's reinstall is pinned to the installed version.Tested on
Environment (optional)
Local validation:
yaml.safe_loadon all three workflows,actionlint(only pre-existing shellcheck style warnings, identical count before/after on each file),bash -non the changed run scripts, andgh api --helpconfirming thekey[subkey]=valuenested-parameter syntax.Risk & Scope
npm install -g @lateststeps can still race concurrent installs, but they resolve fresh metadata (no--prefer-offline) so they upgrade rather than downgrade.Linked Issues
N/A — stems from maintainer investigation of stale runner versions after the v0.21.3 release. Complements #8337.
中文说明
本 PR 做了什么
三处改动，让自建 ECS runner 上的 qwen CLI 保持最新、并阻止其被静默降级。第一，Release workflow 在 stable 发布成功后发送
repository_dispatch（npm-published，携带发布版本号），自动触发已有的 Update ECS Runner Qwen workflow，不再需要手动运行；该步骤仅限真实 stable 发布（npm_tag == 'latest'、非 dry-run、仅 QwenLM/qwen-code），排除 nightly/preview 以免 runner 集群被切到预发布版本，并设置continue-on-error仅告警，确保已发布的 release 不会被它拖败。第二，update job 现在会清理 npm 残留临时目录（/usr/local/lib/node_modules/@qwen-code/.qwen-code-*）并重试全局安装最多三次（带退避），使崩溃残留和并发安装竞态不再以ENOTEMPTY阻塞更新。第三，triage job 现在把 qwen-code-action 的无条件全局重装固定到本 job 已在运行的确切版本（取自 #8337 新增的 Ensure qwen CLI 步骤），使该 action 永远无法降级共享 runner。为什么需要
排查"0.21.3 发布后 runner 仍提供 0.21.2"发现三段式故障链。(1) update workflow 声明的
repository_dispatch: ['npm-published']触发器在全仓库没有任何发送方——历史约 30 次运行全部是手动workflow_dispatch，因此每次 npm 发布后 runner 都停留在旧版本，直到有人手动跑更新。(2) triage job 使用的 qwen-code-action 每次运行都无条件执行npm install --global --prefer-offline @qwen-code/qwen-code@latest；在共享 ECS 机器上，--prefer-offline从持久化 npm 缓存解析latestdist-tag，而缓存滞后于 npm 发布——机器在 UTC 17:28 被更新到 0.21.3（18:17 的 job 验证过）之后，triage job 把latest解析成缓存里的 0.21.2 并把机器降级回去（21:05 起同一台机器iZt4ne2fxw78cka1n2cbkoZ上每个 job 都打印 0.21.2）。#8337 加了"缺失才装"的预步骤，但 action 内部安装依旧无条件执行，挡不住降级。(3) 每次降级/升级循环都会留下 npm 临时目录，而 npm 对给定包生成的临时目录名是确定的——17:58 与 23:43 两次 update 失败都死在完全相同的 rename 目标.qwen-code-g9EoLCuD上，因此一旦这个残留目录存在，update workflow 就被永久阻塞（ENOTEMPTY，exit 217），无法自愈。Reviewer 测试计划
如何验证
确认 dispatch 步骤的门控：仅对来自 QwenLM/qwen-code 的真实 stable 发布触发（
npm_tag == 'latest'、非 dry-run）且为continue-on-error；payload 与 update workflow 读取的字段一致（github.event.client_payload.version←client_payload[version]=<release_version>；版本为空时回退 npmlatest）。确认 update job 的重试循环首次成功即退出 0、每次尝试前清理/usr/local/lib/node_modules/@qwen-code/.qwen-code-*、三次失败后让步骤失败，且现有的 Verify version 步骤仍校验最终版本。确认 triage job 在 Ensure qwen CLI 步骤捕获已安装版本（id: ensure_qwen、version输出）并以qwen_cli_version传入，使 action 的重装针对确切版本（不解析 dist-tag、不会被缓存驱动降级）——与 #8337 的步骤组合而非替换。合并后：手动重跑一次 Update ECS Runner Qwen workflow 以清除残留临时目录、把两台机器恢复到 0.21.3；此后下一次 stable 发布应自动派发更新，且 triage job 不再改变已安装版本。证据（前后对比）
N/A——纯 CI/workflow 改动，无用户可见 UI。改动前：0.21.3 于 UTC 17:22 发布，17:28 手动更新验证到 0.21.3，triage job 在 21:05 前把机器降级回 0.21.2（陈旧 npm 缓存 +
--prefer-offline），17:58 与 23:43 两次手动重跑都失败于完全相同的ENOTEMPTY ... '.qwen-code-g9EoLCuD'rename 错误（run 30711479656、30723886544）。改动后：stable 发布自动派发更新，update job 清理残留并重试穿过并发安装竞态，triage action 的重装被固定到已安装版本。测试平台
环境（可选）
本地校验：三个 workflow 通过
yaml.safe_load，actionlint仅报预存的 shellcheck 风格告警（每个文件改动前后数量一致），改动的 run 脚本通过bash -n，并通过gh api --help确认key[subkey]=value嵌套参数语法。风险与范围
npm install -g @latest步骤仍可能与并发安装竞争，但它们解析新鲜元数据（无--prefer-offline），只会升级而非降级。关联 Issue
N/A——源于 maintainer 对 v0.21.3 发布后 runner 版本滞后的排查。与 #8337 互补。