ci: run Windows merge queue tests on ECS - #8386
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: this is a CI infrastructure improvement, not a bug fix — the Windows ECS runner has completed onboarding validation (#8008, #8050, and the linked Actions run), and this assigns it its first production workload. The motivation is clear and evidenced. Direction: aligned. This follows the exact same pattern already established for the Linux ECS runners in this workflow — conditional Size: not applicable — Approach: the scope is tight. Every edit serves the stated goal: runner selection, self-hosted environment config, shell/encoding alignment, and the Windows-specific test command that skips bash-only Risk: no elevated risk signals. The job only triggers on Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个 CI 基础设施改进，不是 bug 修复——Windows ECS runner 已完成上线验证（#8008、#8050 及关联的 Actions 运行），本 PR 为其分配第一项正式负载。动机清晰且有证据支撑。 方向：对齐。与本 workflow 中 Linux ECS runner 已建立的模式完全一致——条件式 规模：不适用—— 方案：范围紧凑。每处编辑都服务于既定目标：runner 选择、自托管环境配置、shell/编码对齐、以及跳过 bash-only 风险：无升级风险信号。该 job 仅在 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: for "route the Windows merge-queue test to the validated ECS runner with fallback", I would change Findings: no blockers. A few observations, all positive:
TestingThis is a CI-only change with no user-visible behavior — real-scenario testing is N/A. CI signal for Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The Windows test is skipped as expected — it only runs on 中文说明代码审查独立方案： 对于"将 Windows 合并队列测试路由到已验证的 ECS runner 并保留回退"，我会：用现有 发现： 无阻塞项。几点观察，均为正面：
测试这是纯 CI 变更，无用户可见行为——真实场景测试不适用。
— Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, minimal CI change by a maintainer that follows the established Linux ECS pattern exactly; would merge without hesitation. This is the natural next step after the Windows runner validation work. The diff does one thing — routes the merge-queue Windows test to the ECS runner — and does it with the same kill-switch safety net the Linux jobs already have. The environment configuration is properly gated, the test command adaptation for Windows is well-reasoned (skipping bash-only Nothing to second-guess here. Approval deferred until CI lands green on 中文说明置信度：5/5 — 维护者提交的干净、最小化 CI 变更，完全遵循已建立的 Linux ECS 模式；毫不犹豫地合并。 这是 Windows runner 验证工作后的自然下一步。diff 只做一件事——将合并队列 Windows 测试路由到 ECS runner——并使用与 Linux job 相同的开关安全网。环境配置正确地加了条件守卫，Windows 测试命令的适配合理（跳过 bash-only 的 无需犹豫。批准延迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Review:
|
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. ✅
Windows ECS runner validation reportStatus: the ECS routing and runner setup are working, but the Windows test job is not green yet. I dispatched the merge-queue-only Windows job from temporary validation refs that enabled
Runner setup, checkout, Node setup, npm configuration, and dependency installation succeeded in every completed ECS run. No ECS run was lost to runner disconnects, timeouts, or resource exhaustion. The three post-fix ECS runs averaged 13m 26s; the hosted baseline took 45m 40s (about 3.4x longer). Validation found and fixed two concrete Windows gaps in this PR:
The final ECS run still fails on hard-coded POSIX path/executable-bit expectations and Windows filesystem semantics. The same-code hosted baseline is also red, with a larger overlapping failure set, so these are current Windows-suite compatibility issues rather than an ECS scheduling failure. Conclusion: the Windows ECS runner is viable and materially faster, but this check should not yet be treated as healthy/green. The remaining Windows test expectations need a separate compatibility pass before this becomes a reliable merge-queue gate. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — the changed job Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and its Windows runtime behavior was not exercised locally (review host is Linux).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：build-and-test — the changed job Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and its Windows runtime behavior was not exercised locally (review host is Linux)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
[Critical] ci.yml:721 — Dropping test:scripts removes the only CI execution of 9 Windows-only tests in scripts/tests/install-script.test.js (re-checked existing blocker: still stands)
— qwen3.7-max 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 冲突，直到移除标签或达到轮次上限。移除 |
Review:
|
Review feedback on the Windows ECS routing: dropping test:scripts removed the only Windows execution of 9 Windows-only install-script tests, and the job-wide PowerShell default plus narrowed test command changed the kill-switch fallback away from the known-good hosted configuration. Restore the full npm run test:ci on both paths (bash is available: pre-installed on hosted runners, exposed via the Git Bash PATH entry on ECS) and gate every ECS-specific adjustment on runner.environment: the PowerShell setup step (now also skip_ci-guarded), TEMP/TMP/LC_ALL env writes, and the Linux-style Node setup split that fails with an actionable error naming MAINTAINER_ECS_RUNNER_DISABLED. The windows-2022 fallback is byte-for-byte the pre-ECS job again.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8386One commit on the PR branch: Feedback points and decisions[Critical] Inline rc:3699888288 (@qwen-code-ci-bot) and the matching CHANGES_REQUESTED review by @doudouOUC — dropping @wenshao §1 (blocking) — RESOLVED. Same defect and same fix as the Critical above. @wenshao §2 — the job-wide @wenshao §3 — the kill switch no longer restored the known-good hosted configuration. — RESOLVED. Everything ECS-specific is now gated on
@wenshao §4 — diverges from the Linux ECS Node-setup convention. — RESOLVED (split implemented). @wenshao §5.1 — missing @wenshao §5.2 — the @wenshao §5.3 — @qwen-code-ci-bot CHANGES_REQUESTED review — "Not reviewed: build-and-test — the changed job was skipped in CI (merge_group-only) and not exercised locally (review host is Linux)". — NOT ACTIONABLE IN CODE. The job is merge-gate-only by design (that is how the required check context @yiliang114 validation report — INFORMATIONAL, no action requested. It confirms the ECS routing works (~3.4× faster than hosted) and that the remaining 11 red assertions are pre-existing Windows-suite compatibility issues tracked for a separate pass (the same-code hosted baseline is also red, with 42). Restoring VerificationCommands actually run this round (this change touches
中文说明Autofix 评审轮次总结 — PR #8386在 PR 分支上提交了一个 commit： 反馈点与处理决定[Critical] 行内评论 rc:3699888288（@qwen-code-ci-bot）以及 @doudouOUC 相同的 CHANGES_REQUESTED 评审 —— 去掉 @wenshao §1（阻断项） —— 已解决。与上述 Critical 是同一缺陷，同一修复。 @wenshao §2 —— job 级 @wenshao §3 —— kill switch 不再恢复已验证的托管配置。 —— 已解决。所有 ECS 专属调整现在都按
@wenshao §4 —— 与 Linux ECS 的 Node 安装约定不一致。 —— 已解决（实现了拆分）。 @wenshao §5.1 —— 新 setup 步骤缺少 @wenshao §5.2 —— 从 smoke workflow 拷贝时丢掉了关于 @wenshao §5.3 —— 相对 smoke workflow 少了 @qwen-code-ci-bot 的 CHANGES_REQUESTED 评审 —— "Not reviewed: build-and-test —— 被改动的 job 在 CI 中被跳过（仅 merge_group 触发），且未在本地演练（评审宿主是 Linux）"。 —— 代码层面不可处理。该 job 按设计只在 merge gate 运行（这正是保留必需检查上下文 @yiliang114 的验证报告 —— 信息性内容，未提出改动要求。报告确认 ECS 路由可用（比托管快约 3.4 倍），剩余 11 个红色断言是既有的 Windows 套件兼容性问题，将另行专项处理（同一代码的托管基线同样为红，且有 42 个失败）。恢复 验证本轮实际运行的命令（本次改动仅涉及
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/模型 |
Final Windows ECS validationI validated the final PR head
Both runs completed dependency installation, every workspace test suite, and the root script suite without failures. The script-suite summary was identical in both runs: 37/37 test files passed; 545 tests passed, 38 platform skips, 0 failures (583 total). The previously failing The Windows installer coverage remained enabled and passed in both runs: 9/9 Windows installer/uninstaller end-to-end tests passed. This includes checksum-verified local installation, tampered-archive rejection, unsafe install-path rejection, Aliyun VERSION-pointer resolution, npm fallback behavior and error context, plus PowerShell uninstall help and purge behavior. The compatibility fixes are test/CI-only: platform-correct paths and POSIX-only skips, deterministic directory-identity replacement, killed-process liveness waiting, removal of unrelated sidecar I/O from a 50,001-session truncation test, and exact Windows CMD fixture output. No production logic was changed. The runner retains the repository kill switch ( Conclusion: the complete Windows merge-queue test workload is green in two consecutive exact-head ECS runs, with full script and Windows installer coverage preserved. |
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8386 (reviews after 2026-08-04T11:41:18Z)Addressed the two new top-level reviews from @wenshao (12:43 and 14:11 UTC) and the automated review round 6 (COMMENTED with zero findings — nothing to fix; its note that the merge_group-only Windows job cannot be exercised from a Linux review host is expected, and the merge queue remains the verification point). Items already fixed at the previous head Fixed in this round (commit
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and the Windows runtime behavior of the changed job (routing, composite actions, cache step) cannot be exercised from this Linux review host; the first merge-queue run remains the verification point.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and all diff-touched script tests are green; the edits are Windows platform guards and path normalizations).
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and the Windows runtime behavior of the changed job (routing, composite actions, cache step) cannot be exercised from this Linux review host; the first merge-queue run remains the verification point。
未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and all diff-touched script tests are green; the edits are Windows platform guards and path normalizations)。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
…8386) The CDP acceptance test asserted a POSIX-only SIGKILL escalation, which fails deterministically on Windows where kill('SIGTERM') terminates the child directly — blocking the Windows merge-queue gate. Assert the platform-appropriate signal instead. Also address review suggestions: probe `unzip` alongside `zip`, pin the integration_cli guard's missing step-level `if:`, stop getWorkflowStep at unnamed steps, pin install-script.test.js out of the win32 excludes, add the stale-checkout guard to windows-runner-smoke.yml, pin the Node preflight warning branch and the guard reject path contiguously, and extend the smoke shell-parity loop to the npm cache step.
|
🤖 Addressed the latest review feedback (round 10/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 10/100 轮）。改动内容与我反驳保留之处如下： Autofix review round summary — PR #8386All nine inline findings (1 Critical, 8 Suggestions) were addressed in code in one commit ( Critical
Suggestions
Review-level (CHANGES_REQUESTED body)The review body's “Not reviewed” items (Windows/macOS jobs skipped in CI, actionlint embedded-shell limitation) are verification-environment notes, not code defects — no code change alters them. The actionable content was the Critical above; after push, the smoke re-dispatch and the first merge-queue run remain the authoritative Windows verification point. VerificationCommands actually run on this Linux runner, in order:
Not run: integration tests after 中文说明Autofix 评审轮次总结 — PR #8386全部九条 inline 发现（1 条 Critical、8 条 Suggestion）已在一个提交（ Critical
Suggestions
评审主体（CHANGES_REQUESTED 正文）评审正文中的“未审查”条目（Windows/macOS 作业在 CI 中被跳过、actionlint 内嵌 shell 的工具限制）属于验证环境的说明，不是代码缺陷——没有任何代码改动可以改变它们。其中可执行的内容就是上面的 Critical；推送之后，smoke 重新 dispatch 与第一次合并队列运行仍是 Windows 侧的权威验证点。 验证在本 Linux 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/模型 |
|
📊 Takeover milestone — round 10/100, in the current window. Census: 10 pushed fix(es), 5 no-change review(s), 0 timeout(s), 0 rejected attempt(s), 0 other round(s) (crash / model error / gate error / infra), 0 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 10/100 轮（当前窗口）。统计：推送修复 10 次、审阅无需改动 5 次、超时 0 次、验证拒绝 0 次、其他轮次（崩溃/模型错误/门错误/infra）0 次、base 更新 0 次。 轮次到这个量值得人工看一眼。可选：继续（无需操作）；若轮次持续累积，考虑拆分或缩减 PR；或释放接管（移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Test (windows-latest, Node 22.x) is merge_group-only and was skipped in CI; the Windows runtime behavior of the changed job (ECS routing, composite actions, cache step) cannot be exercised from this Linux review host; the first merge-queue run remains the verification point. Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and targeted local reruns of every PR-touched test file are green; the edits are Windows platform guards and path normalizations). Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge_group-only); this diff changes no production logic, so the Linux workspace and script suites that ran already cover every changed file. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Test (windows-latest, Node 22.x) is merge_group-only and was skipped in CI; the Windows runtime behavior of the changed job (ECS routing, composite actions, cache step) cannot be exercised from this Linux review host; the first merge-queue run remains the verification point。 未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and targeted local reruns of every PR-touched test file are green; the edits are Windows platform guards and path normalizations)。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (merge_group-only); this diff changes no production logic, so the Linux workspace and script suites that ran already cover every changed file。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
Windows verification on the current head is in: I dispatched The case that blocked the earlier runs is resolved: That closes the "smoke re-dispatch on the new head" precondition; the remaining Windows confirmation is the first merge-queue run, which the gate already treats as authoritative. Re-review when convenient. |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
|
⏸️ AutoFix paused: this PR reached its automatic round cap (10/10) and the loop will not manage it further — new feedback and base conflicts stay unhandled. Comment 中文说明⏸️ AutoFix 已暂停：本 PR 达到自动轮次上限（10/10），循环不再管理——新反馈与 base 冲突将无人处理。评论 |
…-runner # Conflicts: # packages/core/src/skills/skill-curator.test.ts
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and its Windows runtime behavior on the ECS runner (routing, composite actions, cache step) cannot be exercised from this Linux review host; the green smoke dispatch at HEAD (run 30973835494) exercises the same configuration, and the first merge-queue run remains the final verification point. Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and targeted local reruns of every PR-touched test file are green; the edits are Windows platform guards and path normalizations). Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI (merge_group-only) and its Windows runtime behavior on the ECS runner (routing, composite actions, cache step) cannot be exercised from this Linux review host; the green smoke dispatch at HEAD (run 30973835494) exercises the same configuration, and the first merge-queue run remains the final verification point。 未审查：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI; macOS execution of the changed test files did not run (Linux suites and targeted local reruns of every PR-touched test file are green; the edits are Windows platform guards and path normalizations)。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/windows-runner-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
Status check / assessmentWent through the full review history, threads, and CI state on this PR. Summary: Open items
Overall The PR grew well beyond "routing only" (27 files, +1099/-580) with a large wave of Windows test-compat fixes and fairly brittle YAML-pinning tests in Functionally this looks ready to merge once the stale review is refreshed and the one open comment-accuracy suggestion is applied or explicitly dismissed. |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at c9ac1582575423dc355f32f026853b09f7b5d726. This PR does the thing that is easy to get wrong — changing tests so they pass on a new platform — and it mostly gets it right. I went through the test changes asking of each one whether it fixes a platform assumption or quietly drops coverage.
What I verified
The genuinely good ones:
readManyFiles.test.tsmoves from mockingfs.statto performing a real directory rename insideshouldGitIgnoreFile, and addsexpect(result.files).toHaveLength(0). That is stronger, not weaker: the new assertion kills a mutant that deletes the inode-recheck guard, which the old stat-mocking shape could not, sincereadDirectoryreturns its info unconditionally.package-scripts.test.jschanging'@echo husky >> "%F%"'to'@echo(husky>>"%F%"'fixes a real cmd.exe bug —echo x >> fwrites the space before>into the file — rather than bending an assertion.install-script.test.jsgates on azipAvailableprobe that checks bothzipandunzip, and it is fail-closed:if (process.env.CI && platform !== 'win32' && !zipAvailable) throw. Tool absence on non-Windows CI is an error, not a silent skip. TheitOnWindowscount is unchanged, so the nine Windows installer cases did not go anywhere.lint.test.jsnormalizes paths for the path assertions while keeping the un-normalizedexpect(first).not.toBe(second)isolation assertion. That detail is the difference between normalizing and weakening.- Most new platform skips are for preconditions Windows physically cannot express — POSIX mode bits on NTFS, catchable POSIX signals, control bytes in filenames, renaming over an open file,
chmodon a directory — and each carries a comment saying so.
I also checked the three new composite actions for the usual self-hosted footguns and found none: no Defender manipulation, no Set-ExecutionPolicy, no downloading or installing of unverified tools, and nothing written to machine-level state (only GITHUB_ENV / GITHUB_PATH, which are job-scoped). All three fail closed. The explicit -Encoding utf8 in configure-windows-runner is the right call — PowerShell would otherwise write UTF-16LE that the runner cannot parse.
On the merge-queue side: only merge_group is touched, pull_request triggers are untouched, there is no pull_request_target, permissions is contents: read, and no secrets are injected. The comment at ci.yml:753-754 is now accurate that maintainers can still queue fork PRs, so the trust boundary is stated rather than assumed away.
Two things I would like settled before merge
1. drive.test.ts:99 is the only new skip in this PR with no stated reason, and it takes three tests with it. configure-windows-runner/action.yml deliberately puts C:\Program Files\Git\bin on PATH, so spawnSync('bash', …) is available on this runner. The first skipped test, reports the code for every exit path, uses only echo, exit 17, set -e; false and exit 0 — I read it, and there is no POSIX-only construct in it. Every other skip in this diff explains itself; this one does not, which means nobody will dare remove it later. Either narrow it to the case that genuinely depends on shell semantics using the capability-based it.skipIf(!hasBash) pattern already present in extract-step.test.ts, or record the observed Windows failure in a comment.
2. self-hosted-node only warns on a Node major-version mismatch, while the check it serves is named Test (windows-latest, Node 22.x). The action's own description says it "warns when the major version is not 22", so this is deliberate — but per your comment at ci.yml:752-753 that name is the required-status-check context. So if the ECS box is ever reprovisioned to Node 20 or 24, a required check reading "Node 22.x" goes green against a different runtime, with only an annotation to show it. Version drift on a long-lived runner is exactly the failure mode that is hardest to notice. Worth making it exit 1 — the error text already points at MAINTAINER_ECS_RUNNER_DISABLED as the escape hatch — or comparing against .nvmrc rather than a hardcoded "22". Note the parity is currently inverted: windows-runner-smoke.yml hard-checks the Node version, so the smoke job is stricter than the real gate.
One smaller thing I noticed
The win32 exclusion in vitest.config.ts is a glob, 'scripts/tests/qwen-*-workflow.test.js', not an enumeration. Spreading configDefaults.exclude is correct and non-Windows behaviour is unchanged, but the glob means any future qwen-<something>-workflow.test.js silently drops off Windows without anyone deciding that. Contrast the care taken in the other direction — there is a dedicated test pinning install-script.test.js out of the exclude list, with a comment that excluding it would silently drop coverage. The same discipline pointed at this glob would mean listing the files explicitly, so adding one is a decision rather than a default.
Verification boundary, stated plainly
Test (windows-latest, Node 22.x) is skipping on this PR because the job is merge_group-only, so there is no direct Windows evidence at this HEAD. The green Windows evidence is the smoke run at e6d5b344, and the only commit between that and c9ac1582 is docs(ci): clarify Windows runner trust boundary, whose diff is two comment lines in ci.yml — so that green run is functionally equivalent to what is here. Test (ubuntu-latest, Node 22.x) passes on c9ac1582 (run 30988426238).
Approving on that basis. The two items above are not blockers, but the second one touches a required gate, so I would rather it be decided than inherited.
|
Released in v0.21.6. |
7 participants
What this PR does
Routes the merge-queue Windows test to the validated self-hosted
ecs-winrunner by default while preserving the existingMAINTAINER_ECS_RUNNER_DISABLED=truefallback towindows-2022. It keeps the required check name unchanged and aligns the job with the PowerShell, locale, temporary-directory, line-ending, memory, and workspace-test configuration already proven by the Windows runner validation workflow.Why it's needed
The Windows ECS runner has completed its onboarding smoke and workspace compatibility validation. Assigning the existing merge-queue Windows test is the first production CI workload, reducing reliance on the hosted Windows pool without widening the workflow trigger or permissions.
Reviewer Test Plan
How to verify
Confirm workflow lint passes. When the PR enters the merge queue, verify
Test (windows-latest, Node 22.x)is assigned toecs-qwen-runner-win-hk-1, retains its existing check name, and completes the workspace test suites. SettingMAINTAINER_ECS_RUNNER_DISABLED=trueshould route subsequent runs back towindows-2022.Evidence (Before & After)
Before: the merge-queue Windows test always selected
windows-2022.After: the workflow selects the
self-hosted,Windows,X64, andecs-winlabels unless the existing ECS kill switch is enabled. The same environment and workspace test command passed on the ECS runner in https://github.com/QwenLM/qwen-code/actions/runs/30631136482.Tested on
actionlintandyamllintEnvironment (optional)
macOS local workflow validation; Windows ECS runner
ecs-qwen-runner-win-hk-1.Risk & Scope
merge_group, so its final routing is verified when this PR enters the merge queue. Additional Windows build and release jobs remain hosted.Linked Issues
N/A - follow-up to the Windows runner onboarding in #8008 and compatibility validation in #8050.
中文说明
本 PR 做了什么
默认将合并队列中的 Windows 测试调度到已经验证通过的自托管
ecs-winrunner，同时保留现有的MAINTAINER_ECS_RUNNER_DISABLED=true开关，以便回退到windows-2022。必需检查的名称保持不变，并将正式 job 对齐到 Windows runner 验证 workflow 已经跑通的 PowerShell、区域设置、临时目录、换行符、内存和 workspace 测试配置。为什么需要
Windows ECS runner 已经完成上线冒烟和 workspace 兼容性验证。让它承接现有的合并队列 Windows 测试，是第一项正式 CI 负载，可以减少对 GitHub 托管 Windows 池的依赖，同时不扩大 workflow 的触发范围或权限。
Reviewer 测试计划
如何验证
确认 workflow lint 通过。当 PR 进入合并队列时，确认
Test (windows-latest, Node 22.x)被调度到ecs-qwen-runner-win-hk-1，检查名称保持不变，并完成 workspace 测试套件。设置MAINTAINER_ECS_RUNNER_DISABLED=true后，后续运行应回退到windows-2022。前后对比证据
之前：合并队列 Windows 测试始终选择
windows-2022。之后：除非开启现有 ECS kill switch，否则 workflow 会选择
self-hosted、Windows、X64和ecs-win标签。同样的环境和 workspace 测试命令已经在 ECS runner 上通过：https://github.com/QwenLM/qwen-code/actions/runs/30631136482。已测试平台
actionlint和yamllint环境
macOS 本地 workflow 验证；Windows ECS runner
ecs-qwen-runner-win-hk-1。风险与范围
merge_group事件运行，因此最终调度要等本 PR 进入合并队列后验证。其他 Windows 构建和发布任务继续使用托管 runner。关联 Issue
不适用；这是 #8008 Windows runner 上线和 #8050 兼容性验证的后续步骤。