Skip to content

fix(ci): keep contended ECS E2E shards above the flat 60-minute ceiling (#10591) - #10597

Closed
qwen-code-dev-bot wants to merge 4 commits into
mainfrom
autofix/issue-10591
Closed

fix(ci): keep contended ECS E2E shards above the flat 60-minute ceiling (#10591)#10597
qwen-code-dev-bot wants to merge 4 commits into
mainfrom
autofix/issue-10591

Conversation

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

What this PR does

Extends the Linux E2E job's ceiling on the shared ECS runner pool from 60 to 90 minutes, scoped exactly to the pool routing: the same repository and kill-switch condition that decides whether a shard runs on the ecs-qwen pool or falls back to a hosted runner. Hosted fallbacks keep the pre-contention 60-minute bound. A lane-contract test in the scripts test suite evaluates the real timeout expression for both routings — the substitute-then-evaluate technique the platform-lane tests already use — so a regression to an unconditional ceiling fails in CI instead of reading as a passing constant.

Why it's needed

The post-merge E2E run 33345905817 on cd5d5af2fb failed before any test result was reported and was tracked as issue #10591. The issue body is the failure tracker's per-commit template, which it only uses when the failed jobs' logs carry no vitest FAIL lines — a job that died before tests reported. The run's wall clock (~63 minutes from push to filed issue) lines up with the flat 60-minute job ceiling expiring on a contended shared host, plus finalization and tracker latency.

The shared ECS pool has a documented history of stretching CI lanes past their pre-contention budgets: #10552 extended the main CI and Serve A/B ceilings (scoped to the pool) and serialized the Java SDK lane behind a host lock, and #10567 capped each E2E shard to one vitest fork on the pool — the test phase only. The Linux E2E job was the last shared-pool lane still on the old flat bound, so its install/build/bundle/sandbox-build phases under overlapping runs could still expire the whole shard before any test reported. This closes that gap the same way the sibling lanes were fixed.

Reviewer Test Plan

How to verify

  • Read the workflow change: the timeout expression reuses the exact routing condition of the job's runs-on, so the ceiling can never disagree with where the job runs (pool → 90, hosted fallback → 60).
  • Run the contract test: npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/e2e-workflow.test.js — the two new assertions evaluate the real expression for the pool routing (expects 90) and for both hosted fallbacks, fork and kill-switch (expect 60).
  • Mutation-check the witness if desired: set the expression to a flat 60 and the pool assertion fails; set both branches to '90' and the hosted assertion fails (both probes were run during development and restored).
  • Watch the next post-merge E2E runs on main: under host contention, shards that previously expired at 60 minutes without reporting any test result should now complete; genuinely wedged shards still fail at 90 instead of GitHub's 360-minute default.

Evidence (Before & After)

N/A (CI workflow change, not user-visible). Before: run 33345905817 failed before any test result was reported and the tracker filed per-commit issue #10591. After: the ECS-routed shards carry the contention budget their sibling pool lanes already have, pinned by a failing-on-regression contract test.

Tested on

OS Status
🍏 macOS ⚠️ not tested
🪟 Windows ⚠️ not tested
🐧 Linux ✅ tested

Environment (optional)

Verification ran on a shared ECS runner host: npm run build, npm run typecheck, npm run lint, node scripts/lint.js --actionlint, npx prettier --check on the touched files, focused vitest on the workflow contract test with mutation probes, and a full npm run test:scripts pass (four unrelated load-flakes re-run green in isolation; details in the E2E report). yamllint could not run locally (no pip); the new line matches the double-quoted-with-inner-single-quotes style already present and CI-green in the same file, which the yamllint config explicitly allows.

Risk & Scope

  • Main risk or tradeoff: a genuinely wedged ECS-routed shard now holds its runner up to 90 minutes instead of 60 — the same tradeoff already accepted for the ci.yml pool lanes in ci: stabilize tests under shared ECS host contention #10552, bounded well below GitHub's 360-minute default. Hosted runners are unaffected.
  • Not validated / out of scope: the exact step that expired in run 33345905817 cannot be proven without the run logs; this applies the repository's established contention remedy to the one lane that lacked it. The macOS E2E job (hosted-only, default ceiling), the nightly isolated legs, and any deeper pool serialization (e.g. host locks around install/build) are out of scope.
  • Breaking changes / migration notes: none; workflow-only change, no runtime behavior affected.

Linked Issues

Fixes #10591

中文说明

本 PR 做了什么

将 Linux E2E 任务在共享 ECS runner 资源池上的上限从 60 分钟提高到 90 分钟,并且严格限定在资源池路由上:使用与"任务跑在 ecs-qwen 池还是回退到托管 runner"完全相同的仓库名 + 紧急开关条件。托管回退路径保留争用前的 60 分钟上限。脚本测试套件中新增了一个通道契约测试,对两种路由分别求值真实的 timeout 表达式——沿用平台通道测试已在使用的"先替换再求值"手法——这样将来如果退化成无条件的固定上限,会在 CI 中直接失败,而不是看起来像个通过的常量。

为什么需要

cd5d5af2fb 上的合并后 E2E 运行 33345905817 在报告任何测试结果之前就失败了,被跟踪为 issue #10591。该 issue 正文是失败跟踪器的"按提交跟踪"模板,只有当失败任务的日志中没有任何 vitest FAIL 行时才会使用——即任务在测试产出结果之前就死掉了。该运行的墙上时钟(从推送到建 issue 约 63 分钟)与固定 60 分钟任务上限在争用的共享主机上到期、再加上收尾和跟踪器延迟相吻合。

共享 ECS 资源池有把 CI 通道撑到超过争用前预算的既有记录:#10552 提高了主 CI 与 Serve A/B 的上限(限定在资源池内),并用主机锁把 Java SDK 通道串行化;#10567 把每个 E2E 分片在资源池上的 vitest fork 数限制为 1——但只覆盖测试阶段。Linux E2E 任务是唯一仍停留在旧的固定上限上的共享池通道,因此在重叠运行之下,其安装/构建/打包/沙箱镜像构建阶段仍可能让整个分片在任何测试产出结果之前超时。本 PR 用与兄弟通道相同的方式补上了这个缺口。

审阅者测试计划

如何验证

  • 阅读工作流变更:timeout 表达式复用了该任务 runs-on 完全相同的路由条件,因此上限永远不会与任务实际运行的位置不一致(资源池 → 90,托管回退 → 60)。
  • 运行契约测试:npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/e2e-workflow.test.js —— 两个新断言分别对资源池路由求值真实表达式(期望 90),以及对 fork 和紧急开关两种托管回退求值(期望 60)。
  • 如需验证见证的有效性:把表达式改成固定的 60,资源池断言会失败;把两个分支都改成 '90',托管断言会失败(开发过程中两个探针都实际运行过并已恢复)。
  • 观察接下来 main 上的合并后 E2E 运行:在主机争用下,过去在 60 分钟未报告任何测试结果就超时的分片现在应当能够完成;真正卡死的分片仍会在 90 分钟失败,而不是 GitHub 默认的 360 分钟。

证据(前后对比)

N/A(CI 工作流变更,用户不可见)。变更前:运行 33345905817 在报告任何测试结果之前失败,跟踪器按提交建立了 issue #10591。变更后:ECS 路由的分片获得了其兄弟资源池通道早已拥有的争用预算,并由一个"退化即失败"的契约测试钉住。

测试平台

OS 状态
🍏 macOS ⚠️ 未测试
🪟 Windows ⚠️ 未测试
🐧 Linux ✅ 已测试

环境(可选)

验证在一台共享 ECS runner 主机上进行:npm run buildnpm run typechecknpm run lintnode scripts/lint.js --actionlint、对改动文件运行 npx prettier --check、对工作流契约测试做带变异探针的聚焦 vitest,以及一次完整的 npm run test:scripts(4 个与本次变更无关的负载性 flake 单独重跑后全绿,详见 E2E 报告)。yamllint 在本地无法运行(没有 pip);新行与同文件中已经存在且 CI 全绿的"双引号内含单引号"写法完全一致,yamllint 配置明确允许该形态。

风险与范围

  • 主要风险或取舍:真正卡死的 ECS 路由分片现在会占用 runner 最多 90 分钟而不是 60 分钟——这与 ci: stabilize tests under shared ECS host contention #10552 中 ci.yml 资源池通道已经接受的取舍相同,且远低于 GitHub 默认的 360 分钟上限。托管 runner 不受影响。
  • 未验证 / 超出范围:没有运行日志就无法证明 33345905817 具体在哪一步超时;本 PR 将仓库既定的争用补救手段应用到唯一缺少它的通道。macOS E2E 任务(仅托管、默认上限)、夜间隔离通道,以及更深层的资源池串行化(例如对安装/构建阶段加主机锁)均不在本 PR 范围内。
  • 破坏性变更 / 迁移说明:无;纯工作流变更,不影响任何运行时行为。

关联 Issue

Fixes #10591

…ng (#10591)

The post-merge E2E run 33345905817 on cd5d5af failed before any
test result was reported: the per-commit issue body from
main-ci-failure-issue.yml means no failed job log carried a vitest
FAIL line, and the run's ~63-minute wall clock lines up with the flat
timeout-minutes: 60 killing a shard still working on a contended
shared ECS host. ci.yml's pool lanes already got extended, ECS-scoped
ceilings for the same contention (#10552), and #10567 capped each E2E
shard to one vitest fork on the pool; the Linux E2E job was the last
shared-pool lane still on the pre-contention bound.

Extend the ceiling to 90 on the ECS routing only — the same condition
runs-on uses — so hosted fallbacks (forks and the kill-switch) keep
the pre-contention 60 and a genuine hang there does not burn the extra
30 minutes. A lane-contract test evaluates the real timeout expression
for both routings, the way ci-platform-lanes.test.js does, so a
regression to an unconditional ceiling fails there instead of reading
as a passing constant.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

E2E Report — fix(ci): keep contended ECS E2E shards above the flat 60-minute ceiling (#10591)

Problem

Issue #10591 tracks the post-merge E2E Tests run
33345905817
on commit cd5d5af2fb, which failed before any test result was reported.

Diagnosis (repository evidence — no GitHub credentials available to read the run logs)

  1. The issue body is the per-commit template emitted by .github/scripts/ci/main-failure-signature.mjs, which is only used when the failed jobs' logs contain no vitest/pytest FAIL lines — the helper documents this shape as an install/build break, i.e. a job that died before tests reported.
  2. Timeline: the commit was pushed at 2026-08-31 00:53:13 UTC and the issue was created at 01:56:49 UTC (~63 minutes later) — consistent with a shard expiring at the e2e-test-linux job's flat timeout-minutes: 60, plus run finalization and issue-tracker latency. A fast step error (docker preflight, npm failure) would normally end the run far sooner.
  3. The Linux E2E shards run on the shared ecs-qwen pool where overlapping runs and sibling workflows contend on the same hosts. The repository already fought this exact failure class: ci: stabilize tests under shared ECS host contention #10552 extended the ci.yml/serve-ab.yml pool ceilings (scoped to the ECS routing) and serialized sdk-java via a host lock; ci: serialize E2E tests on shared ECS runners #10567 capped each E2E shard at one vitest fork on self-hosted runners — test phase only. e2e-test-linux was the last shared-pool lane still carrying the pre-contention flat 60-minute ceiling.

Change

  • .github/workflows/e2e.yml: e2e-test-linux timeout-minutes becomes 90 on the ECS routing only — the same condition runs-on uses (github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true'). Hosted fallbacks (forks, kill-switch) keep 60, mirroring how ci: stabilize tests under shared ECS host contention #10552 scoped the ci.yml extension, so a genuine hang on a hosted runner does not burn the extra 30 minutes.
  • scripts/tests/e2e-workflow.test.js: a lane-contract test evaluates the real timeout expression for both routings using the substitute-then-evaluate technique from ci-platform-lanes.test.js, so a regression to an unconditional ceiling fails here instead of reading as a passing constant.

The exact failure step inside the job cannot be proven without the run logs; the extended, ECS-scoped ceiling is the repository's established remedy for this lane under contention, and fast-failure mechanisms (docker preflight ::error::, bounded sandbox-image retry) are untouched and would still surface a genuinely broken environment.

Mutation probes (guard witnessed by the round's own tests)

Probe Mutation Result
1 Revert workflow to flat timeout-minutes: 60 keeps a contended ECS shard above the install/build/test budget FAILED (expected 90, got 60); restored afterward
2 Hosted branch also 90 ('90' || '90') keeps the hosted fallback on the pre-contention ceiling FAILED (expected 60, got 90); restored afterward
restore Real scoped expression 11/11 green in scripts/tests/e2e-workflow.test.js

Workflow size ratchet

e2e.yml grew from baseline 16534 to 18577 bytes (+2043), within the gate's 4096-byte growth allowance (.github/workflows/.size-baseline, .github/scripts/check-workflow-size.sh).

Verification

  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0)
  • npm run lint (ESLint) — passed (exit 0)
  • node scripts/lint.js --actionlint — passed (run twice: before and after the final comment rewording)
  • npx prettier --check .github/workflows/e2e.yml scripts/tests/e2e-workflow.test.js — clean
  • npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/e2e-workflow.test.js — 11 passed (re-run after every mutation probe and after the final rewording)
  • Full npm run test:scripts — 1963 passed / 16 skipped / 4 failed; all 4 failures are in files unrelated to this change (qwen-autofix-workflow.test.js, upload-aliyun-oss-assets.test.js, no-utils-upward-import-config.test.js, vscode-companion-no-webui-config.test.js), each a 30s-timeout overrun at 34–52s wall under host contention; an isolated re-run of exactly those four files passed 249/249. Attributed to shared-ECS-host load, consistent with the contention this PR's family exists to mitigate — not to this change.
  • yamllint — not runnable on this runner (no pip); the new line follows the exact quoting precedent already present and CI-green in the same file (if: "${{ runner.environment == 'github-hosted' }}" — a double-quoted string containing single quotes, allowed by the config's allow-quoted-quotes: true).
  • Integration tests — not applicable: the change is CI workflow YAML, exercised by the workflow contract tests above, not by the bundled CLI or the integration harness.
中文说明

E2E 报告 — fix(ci): 让争用中的 ECS E2E 分片不再受固定 60 分钟上限卡死(#10591

问题

Issue #10591 跟踪的是提交 cd5d5af2fb 上的合并后 E2E Tests 运行 33345905817,该运行在报告任何测试结果之前就失败了。

诊断(基于仓库内证据——没有 GitHub 凭据,无法读取运行日志)

  1. Issue 正文是 .github/scripts/ci/main-failure-signature.mjs 输出的"按提交跟踪"模板,只有当失败任务的日志中没有任何 vitest/pytest FAIL 行时才会使用该模板——该辅助脚本把这种形态记录为安装/构建类中断,即任务在测试产出结果之前就死掉了。
  2. 时间线:提交于 2026-08-31 00:53:13 UTC 推送,issue 于 01:56:49 UTC 创建(约 63 分钟后)——与某个分片在 e2e-test-linux 任务固定的 timeout-minutes: 60 上超时、再加上运行收尾和 issue 跟踪器延迟相吻合。快速步骤报错(docker 预检、npm 失败)通常会让运行更早结束。
  3. Linux E2E 分片运行在共享的 ecs-qwen 资源池上,重叠的运行和同池的其他工作流会在同一批主机上相互争用。仓库此前已经处理过完全相同的失败类别:ci: stabilize tests under shared ECS host contention #10552 提高了 ci.yml/serve-ab.yml 资源池任务的上限(限定在 ECS 路由内),并用主机锁将 sdk-java 串行化;ci: serialize E2E tests on shared ECS runners #10567 把每个 E2E 分片在自托管 runner 上的 vitest fork 数限制为 1——但只覆盖了测试阶段。e2e-test-linux 是唯一一个仍保留争用前固定 60 分钟上限的共享池通道。

变更

  • .github/workflows/e2e.ymle2e-test-linuxtimeout-minutes 在 ECS 路由上变为 90——使用与 runs-on 完全相同的条件(github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true')。托管回退路径(fork、紧急开关)保持 60,与 ci: stabilize tests under shared ECS host contention #10552ci.yml 的限定方式一致,避免托管 runner 上的真卡死白白多烧 30 分钟。
  • scripts/tests/e2e-workflow.test.js:新增通道契约测试,沿用 ci-platform-lanes.test.js 的"先替换再求值"手法,对两种路由分别求值真实的 timeout 表达式——如果将来退化成无条件的固定上限,这里会直接失败,而不是看起来像个通过的常量。

没有运行日志就无法证明任务内部具体失败在哪一步;提高并按 ECS 限定的上限是仓库对该通道在争用下的既定补救手段,而快速失败机制(docker 预检 ::error::、有界的沙箱镜像重试)未被改动,真正损坏的环境依然会显式暴露。

变异探针(本回合提交的测试即为守卫的见证)

探针 变异 结果
1 把工作流恢复为固定 timeout-minutes: 60 keeps a contended ECS shard above the install/build/test budget 失败(期望 90,实际 60);随后恢复
2 托管分支也改成 90'90' || '90' keeps the hosted fallback on the pre-contention ceiling 失败(期望 60,实际 90);随后恢复
恢复 真实的按路由限定表达式 scripts/tests/e2e-workflow.test.js 11/11 全绿

工作流体积棘轮

e2e.yml 从基线 16534 字节增至 18577 字节(+2043),在门禁 4096 字节的增长 allowance 之内(见 .github/workflows/.size-baseline.github/scripts/check-workflow-size.sh)。

验证

  • npm run build — 通过(退出码 0)
  • npm run typecheck — 通过(退出码 0)
  • npm run lint(ESLint)— 通过(退出码 0)
  • node scripts/lint.js --actionlint — 通过(共运行两次:最终注释措辞调整前后各一次)
  • npx prettier --check .github/workflows/e2e.yml scripts/tests/e2e-workflow.test.js — 干净
  • npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/e2e-workflow.test.js — 11 通过(每次变异探针之后及最终措辞调整后均重跑)
  • 完整 npm run test:scripts — 1963 通过 / 16 跳过 / 4 失败;4 个失败全部位于与本变更无关的文件(qwen-autofix-workflow.test.jsupload-aliyun-oss-assets.test.jsno-utils-upward-import-config.test.jsvscode-companion-no-webui-config.test.js),均为 30 秒超时在主机争用下的实际耗时 34–52 秒超限;把这 4 个文件单独重跑则 249/249 全部通过。归因于共享 ECS 主机负载——正是本 PR 所属系列要缓解的争用——与本变更无关。
  • yamllint — 本 runner 上无法运行(没有 pip);新行遵循同文件中已经存在且 CI 全绿的完全相同的引号先例(if: "${{ runner.environment == 'github-hosted' }}"——双引号字符串内含单引号,配置的 allow-quoted-quotes: true 允许该形态)。
  • 集成测试 — 不适用:本变更是 CI 工作流 YAML,由上述工作流契约测试覆盖,不由打包后的 CLI 或集成测试框架执行。

🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 31, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 64a703c did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 64a703c 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed, not theoretical. Post-merge E2E run 33345905817 on cd5d5af2fb failed, and I confirmed via the API that the E2E Test (Linux) - sandbox:docker - shard 2/3 job was cancelled at ~62 minutes of wall clock — the flat 60-minute job ceiling expiring before that shard reported any test result. The docker shards in that run were the slow/variable ones (~14 min failure, ~35 min success, 62 min+ cancelled), so a contention-stretched shard really is hitting this ceiling. The tracker filed per-commit issue #10591 because no vitest FAIL line was ever produced.

Direction: aligned. This is the same contention remedy already accepted for the sibling pool lanes (#10552 raised the ci.yml pool ceilings to 90, #10567 capped E2E forks on the pool) — this PR just closes the one lane that was still on the old flat bound. CHANGELOG has no direct reference, but CI reliability is squarely in scope and the 90-minute value matches what ci.yml's pool lanes already run.

Size: not applicable — .github/workflows/e2e.yml + one contract test, no core paths.

Approach: the scope feels right and it's done the minimal way. The timeout expression reuses the exact boolean the job's runs-on already uses to pick pool vs hosted, so the ceiling can never disagree with where the job actually runs (pool → 90, hosted fallback stays 60). The added contract test evaluates the real expression for both routings rather than pinning a bare constant, so a future regression to an unconditional ceiling fails in CI. I don't see anything here that should be cut.

Risk: no elevated risk signals — neither changed file matches the revert-correlated paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:是已观测到的问题,不是理论担忧。cd5d5af2fb 上的合并后 E2E 运行 33345905817 失败了,我通过 API 确认 E2E Test (Linux) - sandbox:docker - shard 2/3 任务在墙上时钟约 62 分钟时被取消——即固定的 60 分钟任务上限在该分片报告任何测试结果之前到期。该运行中的 docker 分片明显偏慢且方差大(约 14 分钟失败、约 35 分钟成功、62 分钟+ 被取消),说明被争用拖长的分片确实撞到了这个上限。由于日志里没有任何 vitest FAIL 行,失败跟踪器按提交建立了 issue #10591

方向:对齐。这与兄弟资源池通道已经接受的争用补救手段一致(#10552 把 ci.yml 资源池上限提到 90,#10567 限制了资源池上 E2E 的 fork 数)——本 PR 只是补上仍停留在旧固定上限上的那一条通道。CHANGELOG 没有直接引用,但 CI 可靠性完全在范围内,且 90 分钟的取值与 ci.yml 资源池通道现行值一致。

规模:不适用——仅 .github/workflows/e2e.yml 加一个契约测试,不触及核心路径。

方案:范围合理,且做法已经是最小化。timeout 表达式复用了该任务 runs-on 用来选择资源池/托管的完全相同的布尔条件,因此上限永远不会与任务实际运行的位置不一致(资源池 → 90,托管回退保持 60)。新增的契约测试对两种路由分别求值真实表达式,而不是钉住一个裸常量,这样将来退化成无条件上限会在 CI 中失败。我没有看到任何应该砍掉的部分。

风险:无升级风险信号——两个改动文件都不命中与回滚相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 64a703cade01e200b116288ac3b10208c2768684 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Code review

Before reading the diff I'd have done exactly this: raise timeout-minutes only on the pool routing (never loosen the hosted fallback, where a genuine hang should still be caught early), and pin it with a test. The PR matches that plan.

Two things I verified closely, both clean:

  • The ceiling can't disagree with the runner. The new expression's predicate — github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true' — is character-for-character the same boolean runs-on uses to choose ecs-qwen vs ubuntu-latest. I traced the GitHub-expression precedence (&& binds tighter than ||, fromJSON yields a number): pool-routed → 90, fork → 60, kill-switch → 60. It mirrors the exact shape ci.yml's pool lanes already use (fromJSON(<pool-condition> && '90' || '60'), ci.yml line 219), and the "${{ … == '…' }}" double-quoted style matches existing CI-green lines in this same file.
  • The contract test actually pins the change. It reuses the substitute-then-evaluate technique from ci-platform-lanes.test.js, evaluates the real expression (not a hardcoded constant), and guards against unmodeled terms. It asserts pool → 90 and both hosted fallbacks (fork, kill-switch) → 60, so a regression to a flat value fails in CI either way. This file is wired into CI (test:scripts is part of test:ci), so the pin really runs.

No correctness bugs, no convention violations, no scope creep — every line is needed for the stated goal. Nothing blocking.

Testing

This is a CI workflow change with nothing user-visible, so real-scenario (tmux) testing is N/A. The evidence below is the PR's own CI, read through the API on the reviewed commit — I did not run any PR code. The contract test that pins the change runs inside the Test (ubuntu-latest, Node 22.x) job below, which is still in flight.

Final CI results for 64a703c (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Post Coverage Comment (ubuntu-latest, 22.x) ❌ failure
Test (ubuntu-latest, Node 22.x) ❌ failure
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ❌ failure
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Secret scan (TruffleHog) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

No check has failed at the time of writing; the Linux unit job (which runs the new contract test) and the no-AK integration lane are still running. One honest caveat: the deterministic half of the claim — "the expression evaluates to 90 on the pool and 60 on every hosted fallback, and stays that way" — is pinned by the contract test above. The other half — "90 minutes is enough headroom under real pool contention" — is an environmental tuning question that only future post-merge runs can answer, and no sandboxed lane can reproduce shared-host contention; the PR itself says as much, and 90 is the same value the sibling ci.yml pool lanes already run. So I'm not flagging a /verify//tmux lane here: neither would settle that residual, and the testable surface is already pinned in CI.

中文说明

代码审查

在看 diff 之前我的独立方案就是这样:只在资源池路由上提高 timeout-minutes(绝不放宽托管回退——那里真卡死就该尽早被抓出来),并用测试钉住。PR 与这个方案一致。

我重点核了两处,都干净:

  • 上限不会与 runner 不一致。 新表达式的谓词——github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true'——与 runs-on 用来选择 ecs-qwen 还是 ubuntu-latest 的布尔条件逐字符相同。我推演了 GitHub 表达式优先级(&& 高于 ||fromJSON 得到数字):资源池路由 → 90,fork → 60,紧急开关 → 60。它与 ci.yml 资源池通道现行写法完全一致(fromJSON(<资源池条件> && '90' || '60'),ci.yml 第 219 行),"${{ … == '…' }}" 双引号写法也与本文件中已经 CI 全绿的写法一致。
  • 契约测试确实钉住了改动。 它复用 ci-platform-lanes.test.js 的"先替换再求值"手法,求值的是真实表达式(而非硬编码常量),并防御未建模的项。它断言资源池 → 90、两种托管回退(fork、紧急开关)→ 60,因此无论退化成哪个固定值都会在 CI 中失败。该文件接入了 CI(test:scriptstest:ci 的一部分),这个钉扎确实会运行。

没有正确性 bug、没有规范违反、没有范围蔓延——每一行都是目标所必需的。无阻塞项。

测试

这是用户不可见的 CI 工作流变更,因此真实场景(tmux)测试为 N/A。下面的证据是被审提交上、通过 API 读取的 PR 自身 CI——我没有运行任何 PR 代码。钉住改动的契约测试就在下面的 Test (ubuntu-latest, Node 22.x) 任务里运行,该任务仍在进行中。

(CI 表格见上方标记区域,由 finalize 工作流在 CI 结束后原地更新。)

撰写时没有任何检查失败;Linux 单元任务(运行新契约测试)与无 AK 集成通道仍在运行。一个诚实的说明:该改动的确定性部分——"表达式在资源池上求值为 90、在所有托管回退上为 60,并保持如此"——已由契约测试钉住。另一半——"90 分钟在真实资源池争用下是否足够"——是只有未来合并后运行才能回答的环境调参问题,任何沙箱通道都无法复现共享主机争用;PR 本身也这么说了,而且 90 与兄弟 ci.yml 资源池通道现行值一致。因此这里不点名 /verify//tmux 通道:两者都无法解决这个残余不确定性,而可测试的部分已经在 CI 中被钉住。

Qwen Code · qwen3.8-max

Reviewed at 64a703cade01e200b116288ac3b10208c2768684 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — an observed, API-confirmed timeout failure fixed the same way its sibling lanes already were, scoped to the pool routing and pinned by a test that fails on regression.

Stepping back: this is exactly the kind of change I want to see. The problem is real — I confirmed run 33345905817's docker shard was cancelled at ~62 minutes, past the flat 60-minute ceiling, before it could report a single test result. The fix is the minimal version of itself: one conditional timeout-minutes that reuses the runs-on predicate so the ceiling can't drift from where the job runs, plus a contract test that evaluates the real expression for both routings. My independent proposal and the PR's approach are the same thing; I found no simpler path and nothing here I'd cut. The only residual — whether 90 minutes is enough headroom under genuine contention — is environmental, only answerable by watching post-merge runs, and the value matches what the ci.yml pool lanes already accept. If I'm maintaining this in six months I'll thank whoever pinned it with a failing-on-regression test.

Verdict: approve. The Linux unit job and the no-AK integration lane are still running on this commit, though, so approval is deferred until CI lands green on 64a703cade01e200b116288ac3b10208c2768684.

中文说明

置信度:5/5 —— 一次已观测到、并经 API 确认的超时失败,用其兄弟通道已经采用的方式修复,严格限定在资源池路由内,并由一个"退化即失败"的测试钉住。

退一步看:这正是我希望看到的改动。问题是真实的——我确认运行 33345905817 的 docker 分片在约 62 分钟(超过固定 60 分钟上限)时被取消,且尚未报告任何测试结果。修复本身就是最小形态:一个复用 runs-on 谓词的条件式 timeout-minutes,使上限不会与任务实际运行位置脱节;外加一个对两种路由分别求值真实表达式的契约测试。我的独立方案与 PR 做法一致;我找不到更简单的路径,也没有任何想砍掉的部分。唯一的残余不确定性——90 分钟在真实争用下是否足够——是环境性的,只能通过观察合并后运行来回答,且该取值与 ci.yml 资源池通道已接受的数值一致。六个月后维护这段代码时,我会感谢那个用"退化即失败"测试把它钉住的人。

结论:批准。 但该提交上的 Linux 单元任务与无 AK 集成通道仍在运行,因此批准将推迟到 CI 在该提交上全绿之后。

Qwen Code · qwen3.8-max

Reviewed at 64a703cade01e200b116288ac3b10208c2768684 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

中文说明

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment thread .github/workflows/e2e.yml
# at the old flat 60 on a contended shared host before its log reported
# any test result. Hosted fallbacks keep the pre-contention ceiling so
# a genuine hang there does not burn the extra 30 minutes.
timeout-minutes: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R1-1: [fails-closed] [regression] This change turns e2e-test-linux's timeout-minutes from the number 60 into this expression string, but the existing lane-contract test .github/scripts/ci-runner-routing.test.mjs:552 still pins assert.equal(job['timeout-minutes'], 60) for this exact job and is never updated by this PR. That file imports node:assert/strict (so assert.equal is strictEqual), which means the assertion now fails unconditionally in every routing state. The file is listed in HELPER_TESTS (.github/workflows/ci.yml:82) and executed via node --test by both helper steps — "GitHub CI helper checks" (ci.yml:436) and "Run .github/scripts helper tests" (ci.yml:586); this PR classifies into the full profile, so the required Test (ubuntu-latest, Node 22.x) check goes red on this PR's own CI run.

Witness:

$ node --test .github/scripts/ci-runner-routing.test.mjs   (at HEAD 64a703c)
# tests 23
# pass 22
# fail 1
not ok 5 - carries the pool hygiene and capability steps in order
  operator: strictEqual
  expected: 60
  actual: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"
  at: .github/scripts/ci-runner-routing.test.mjs:552

A/B on the contested assertion: BASE (pre-change e2e.yml) passes with value=60; the PR arm fails with the expression string.

Update .github/scripts/ci-runner-routing.test.mjs:552 in this same PR: replace the bare 60 pin with an evaluation of the new routing-scoped expression — the same substitute-then-evaluate technique this PR adds in scripts/tests/e2e-workflow.test.js — asserting 90 on the pool routing and 60 on both hosted fallbacks (fork and kill-switch), and fix the adjacent stale comment ("Pin the ci.yml pool precedent"), which now contradicts ci.yml's actual pool precedent of 90.

The updated pin must keep the two ceilings mirrored from .github/workflows/ci.yml:219 (90 on ecs-qwen routing, 60 hosted) and agree with the routing condition the timeout mirrors (runs-on at .github/workflows/e2e.yml:47). The updated assertion itself must go red if timeout-minutes regresses to an unconditional value (a flat 60 or flat 90 without the routing condition) — please remove the routing condition from the assertion and run that subtest to prove it.

中文说明

本改动把 e2e-test-linuxtimeout-minutes 从数字 60 改成了这个表达式字符串,但既有的通道契约测试 .github/scripts/ci-runner-routing.test.mjs:552 仍然对同一个任务钉死 assert.equal(job['timeout-minutes'], 60),且本 PR 没有更新它。该文件导入 node:assert/strict(因此 assert.equalstrictEqual),所以这条断言现在在任何路由状态下都会无条件失败。该文件位于 HELPER_TESTS(.github/workflows/ci.yml:82)中,由两个助手测试步骤通过 node --test 执行——"GitHub CI helper checks"(ci.yml:436)与 "Run .github/scripts helper tests"(ci.yml:586);本 PR 会被分类为 full profile,因此本 PR 自己的 CI 运行中必需的 Test (ubuntu-latest, Node 22.x) 检查会变红。

见证(在 HEAD 64a703c 上执行):node --test .github/scripts/ci-runner-routing.test.mjs → 23 个测试,22 通过,1 失败——失败子测试为 "carries the pool hygiene and capability steps in order",operator: strictEqual,expected: 60,actual 为新的表达式字符串。对该断言做 A/B:BASE(改动前的 e2e.yml)通过(值为 60),PR 侧失败。

请在同一个 PR 中更新 .github/scripts/ci-runner-routing.test.mjs:552:把裸的 60 钉子替换为对新的路由限定表达式求值——复用本 PR 在 scripts/tests/e2e-workflow.test.js 中新增的"先替换再求值"手法——断言资源池路由为 90、两种托管回退(fork 与紧急开关)均为 60,并修正旁边那条已过时的注释("Pin the ci.yml pool precedent"),它现在与 ci.yml 实际的资源池上限 90 相矛盾。

更新后的钉子必须保持与 .github/workflows/ci.yml:219 一致的两个上限(ecs-qwen 路由 90,托管 60),并与 timeout 所镜像的路由条件(.github/workflows/e2e.yml:47 的 runs-on)保持一致。更新后的断言本身必须在 timeout-minutes 退化为无条件值(不带路由条件的裸 60 或裸 90)时变红——请从断言中去掉路由条件并运行该子测试来证明这一点。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified real at the current head: node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs fails at line 552 (strictEqual expected 60, actual is the new routing-scoped expression string) — 23 tests, 22 pass, 1 fail. The full HELPER_TESTS set confirms this is the sole cause of the red required Test (ubuntu-latest, Node 22.x) check: 500 tests, exactly this one failure.

This round nevertheless does NOT implement the pin update: .github/scripts/ is outside this PR's footprint (the PR touches only .github/workflows/e2e.yml and scripts/tests/e2e-workflow.test.js), and the round-3 attempt at exactly this pin update was rejected by the autofix gate and reverted for that reason. Repeating the rejected change would fail the gate again, and there is no in-footprint change that can turn the check green while the pin still reads 60. This needs a maintainer decision on where the pin update lands:

  1. Allow this PR to expand into .github/scripts/, so a next round updates the pin here (substitute-then-evaluate: 90 on the pool routing, 60 on both hosted fallbacks, mirroring the runs-on condition; plus the stale "Pin the ci.yml pool precedent" comment); or
  2. A maintainer lands the pin update separately (direct commit, or a small standalone PR merged first), and this PR rebases onto it; or
  3. Revert this PR's timeout change entirely (not recommended — it abandons the Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 fix).

Leaving this thread open until a maintainer chooses.

中文说明

已在当前 head 上确认为真实问题:node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs 在第 552 行失败(strictEqual 期望 60,实际为新的路由限定表达式字符串)——23 个测试,22 通过,1 失败。完整 HELPER_TESTS 集合确认这是必需的 Test (ubuntu-latest, Node 22.x) 检查变红的唯一原因:共 500 个测试,仅此一处失败。

但本轮不实施该断言更新:.github/scripts/ 在本 PR 的 footprint 之外(本 PR 只改动 .github/workflows/e2e.ymlscripts/tests/e2e-workflow.test.js),且第 3 轮对同一断言的更新尝试已被 autofix gate 以该理由拒绝并回滚。重复被拒绝的改动只会再次被 gate 拒绝,而在断言仍钉死 60 的情况下,footprint 内没有任何改动能让该检查变绿。需要维护者决定断言更新落在哪里:

  1. 允许本 PR 扩展到 .github/scripts/,下一轮即可在此更新断言(先替换再求值:资源池路由 90、两种托管回退均为 60,与 runs-on 条件保持一致;同时修正过时的 "Pin the ci.yml pool precedent" 注释);或
  2. 由维护者单独落地断言更新(直接提交,或先合并一个小的独立 PR),之后本 PR 重新变基;或
  3. 整体回退本 PR 的 timeout 改动(不建议——等于放弃 Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 的修复)。

在维护者作出选择之前,本线程保持打开。

// Run 33345905817 (issue #10591) expired at the old flat 60-minute
// ceiling on a contended shared ECS host before its log reported any
// test result. The extended ceiling is
// scoped to the pool routing — the same condition `runs-on` uses:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-2: The new lane-contract test evaluates only the timeout-minutes expression and never reads runs-on, so the timeout↔routing equivalence this comment claims ("the same condition runs-on uses") is documented but unpinned. e2e.yml carries the routing predicate as two hand-written copies (runs-on at e2e.yml:47 and timeout-minutes at e2e.yml:54), unlike ci.yml where both fields derive from one source of truth (needs.classify_pr.outputs.ubuntu_runner). A future edit to the runs-on condition (adding a kill-switch term or widening pool eligibility) that forgets timeout-minutes leaves every test in this suite green: runs newly routed onto the contended ECS pool keep the 60-minute ceiling and silently re-create issue #10591 (a shard expires before its log reports any test result), or drift the other way and hosted runs get 90 minutes, burning the extra 30 the diff's comment explicitly exists to prevent.

Witness:

Mutation probe: flip != 'true' -> == 'true' on runs-on ONLY (timeout expression untouched)
  PR suite on mutated tree: 11/11 passed (green through a complete inversion of routing semantics)
  equivalence probe (assert timeout == 90 iff runs-on resolves to ecs-qwen): 2 failed | 1 passed
    poolRouted=false but timeout=90
    poolRouted=true but timeout=60   <- the exact #10591 re-creation
  flip check on pristine tree: 3 passed

In the same describe block, evaluate the runs-on expression with the same substitution sweep and assert agreement per scenario instead of asserting each half against test-chosen constants — e.g. assert timeoutMinutesOn(s) === 90 iff the evaluated runs-on expression resolves to the ["self-hosted", "linux", "x64", "ecs-qwen"] branch — so the two expressions answer to one oracle. Note that runs-on at .github/workflows/e2e.yml:47 is a single-quoted YAML scalar with doubled quotes, so after YAML parse the embedded literals carry single quotes; the check must compare the two evaluated results, not a third hand-copied condition. The new equivalence assertion must go red when != 'true' is flipped to == 'true' on only the runs-on expression — today's suite stays green under that mutation; please add it and prove it.

中文说明

新增的通道契约测试只对 timeout-minutes 表达式求值,从不读取 runs-on,因此这条注释所声称的 timeout↔路由等价关系("与 runs-on 使用相同的条件")只是写在注释里,并没有被测试钉住。e2e.yml 中的路由谓词以两份手写副本存在(runs-on 在 e2e.yml:47,timeout-minutes 在 e2e.yml:54),而不像 ci.yml 那样两个字段都从 needs.classify_pr.outputs.ubuntu_runner 这一唯一事实源派生。如果未来某次编辑修改了 runs-on 条件(新增紧急开关项或放宽资源池准入)却忘了 timeout-minutes,本套件的所有测试仍会全绿:新路由到争用 ECS 资源池的运行会继续使用 60 分钟上限,悄悄复现 issue #10591(分片在报告任何测试结果之前过期);反向漂移则会让托管运行获得 90 分钟,白白烧掉本 diff 注释明确要避免的额外 30 分钟。

见证:变异探针——只对 runs-on 表达式把 != 'true' 翻转为 == 'true'(timeout 表达式不动),本 PR 的套件在变异树上 11/11 全绿(路由语义被完全反转后依然通过);而等价性探针(断言 timeout == 90 当且仅当 runs-on 解析到 ecs-qwen 分支)在同一变异树上 2 失败 | 1 通过,包括 "poolRouted=false but timeout=90" 与 "poolRouted=true but timeout=60"(即 #10591 的复现形态);在纯净树上做翻转校验:3 通过。

请在同一个 describe 块中,用相同的替换扫描对 runs-on 表达式求值,并按场景断言两者一致,而不是各自对着测试选定的常量断言——例如断言 timeoutMinutesOn(s) === 90 当且仅当求值后的 runs-on 表达式解析到 ["self-hosted", "linux", "x64", "ecs-qwen"] 分支——让两个表达式对同一个裁决者负责。注意:.github/workflows/e2e.yml:47 的 runs-on 是内含双写引号的单引号 YAML 标量,YAML 解析后其中的字面量带单引号;检查应比较两个求值结果,而不是第三份手抄的条件。新的等价断言必须满足:只把 runs-on 表达式中的 != 'true' 翻转为 == 'true' 时变红——当前套件在该变异下保持全绿,请加上断言并运行证明。

— qwen3.8-max via Qwen Code /review (v0.22.3)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • scripts/tests/e2e-workflow.test.js:110 — [probe] timeoutMinutesOn duplicates the ci-platform-lanes.test.js substitute-then-evaluate skeleton (extract into workflow-helpers.js)
中文说明

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment thread .github/workflows/e2e.yml
# at the old flat 60 on a contended shared host before its log reported
# any test result. Hosted fallbacks keep the pre-contention ceiling so
# a genuine hang there does not burn the extra 30 minutes.
timeout-minutes: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R1-1: [fails-closed] [regression] This change turns e2e-test-linux's timeout-minutes from the number 60 into this expression string, but the existing lane-contract test .github/scripts/ci-runner-routing.test.mjs:552 still pins assert.equal(job['timeout-minutes'], 60) for this exact job and is never updated by this PR. That file imports node:assert/strict (so assert.equal is strictEqual), which means the assertion now fails unconditionally in every routing state. The file is listed in HELPER_TESTS (.github/workflows/ci.yml:82) and executed via node --test by both helper steps — "GitHub CI helper checks" (ci.yml:436) and "Run .github/scripts helper tests" (ci.yml:586); this PR classifies into the full profile, so the required Test (ubuntu-latest, Node 22.x) check goes red on this PR's own CI run.

Re-checked at f2b2a2f7 (this round's reviewed commit): the stale pin is untouched and the failure still fires.

Witness:

$ node --test .github/scripts/ci-runner-routing.test.mjs   (at HEAD f2b2a2f7)
# tests 23
# pass 22
# fail 1
not ok 5 - carries the pool hygiene and capability steps in order
  operator: strictEqual
  expected: 60
  actual: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"
  at: .github/scripts/ci-runner-routing.test.mjs:552

A/B on the contested assertion: BASE (pre-change e2e.yml) passes with value=60; the PR arm fails with the expression string.

Update .github/scripts/ci-runner-routing.test.mjs:552 in this same PR: replace the bare 60 pin with an evaluation of the new routing-scoped expression — the same substitute-then-evaluate technique this PR adds in scripts/tests/e2e-workflow.test.js — asserting 90 on the pool routing and 60 on both hosted fallbacks (fork and kill-switch), and fix the adjacent stale comment ("Pin the ci.yml pool precedent"), which now contradicts ci.yml's actual pool precedent of 90.

The updated pin must keep the two ceilings mirrored from .github/workflows/ci.yml:219 (90 on ecs-qwen routing, 60 hosted) and agree with the routing condition the timeout mirrors (runs-on at .github/workflows/e2e.yml:47). The updated assertion itself must go red if timeout-minutes regresses to an unconditional value (a flat 60 or flat 90 without the routing condition) — please remove the routing condition from the assertion and run that subtest to prove it.

中文说明

本改动把 e2e-test-linuxtimeout-minutes 从数字 60 改成了这个表达式字符串,但既有的通道契约测试 .github/scripts/ci-runner-routing.test.mjs:552 仍然对同一个任务钉死 assert.equal(job['timeout-minutes'], 60),且本 PR 没有更新它。该文件导入 node:assert/strict(因此 assert.equalstrictEqual),所以这条断言现在在任何路由状态下都会无条件失败。该文件位于 HELPER_TESTS(.github/workflows/ci.yml:82)中,由两个助手测试步骤通过 node --test 执行——"GitHub CI helper checks"(ci.yml:436)与 "Run .github/scripts helper tests"(ci.yml:586);本 PR 会被分类为 full profile,因此本 PR 自己的 CI 运行中必需的 Test (ubuntu-latest, Node 22.x) 检查会变红。

已在本轮审阅的提交 f2b2a2f7 上复核:该过时的断言原样存在,失败仍然触发。

见证(在 HEAD f2b2a2f 上执行):node --test .github/scripts/ci-runner-routing.test.mjs → 23 个测试,22 通过,1 失败——失败子测试为 "carries the pool hygiene and capability steps in order",operator: strictEqual,expected: 60,actual 为新的表达式字符串。对该断言做 A/B:BASE(改动前的 e2e.yml)通过(值为 60),PR 侧失败。

请在同一个 PR 中更新 .github/scripts/ci-runner-routing.test.mjs:552:把裸的 60 钉子替换为对新的路由限定表达式求值——复用本 PR 在 scripts/tests/e2e-workflow.test.js 中新增的"先替换再求值"手法——断言资源池路由为 90、两种托管回退(fork 与紧急开关)均为 60,并修正旁边那条已过时的注释("Pin the ci.yml pool precedent"),它现在与 ci.yml 实际的资源池上限 90 相矛盾。

更新后的钉子必须保持与 .github/workflows/ci.yml:219 一致的两个上限(ecs-qwen 路由 90,托管 60),并与 timeout 所镜像的路由条件(.github/workflows/e2e.yml:47 的 runs-on)保持一致。更新后的断言本身必须在 timeout-minutes 退化为无条件值(不带路由条件的裸 60 或裸 90)时变红——请从断言中去掉路由条件并运行该子测试来证明这一点。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified real at the current head: node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs fails at line 552 (strictEqual expected 60, actual is the new routing-scoped expression string) — 23 tests, 22 pass, 1 fail. The full HELPER_TESTS set confirms this is the sole cause of the red required Test (ubuntu-latest, Node 22.x) check: 500 tests, exactly this one failure.

This round nevertheless does NOT implement the pin update: .github/scripts/ is outside this PR's footprint (the PR touches only .github/workflows/e2e.yml and scripts/tests/e2e-workflow.test.js), and the round-3 attempt at exactly this pin update was rejected by the autofix gate and reverted for that reason. Repeating the rejected change would fail the gate again, and there is no in-footprint change that can turn the check green while the pin still reads 60. This needs a maintainer decision on where the pin update lands:

  1. Allow this PR to expand into .github/scripts/, so a next round updates the pin here (substitute-then-evaluate: 90 on the pool routing, 60 on both hosted fallbacks, mirroring the runs-on condition; plus the stale "Pin the ci.yml pool precedent" comment); or
  2. A maintainer lands the pin update separately (direct commit, or a small standalone PR merged first), and this PR rebases onto it; or
  3. Revert this PR's timeout change entirely (not recommended — it abandons the Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 fix).

Leaving this thread open until a maintainer chooses.

中文说明

已在当前 head 上确认为真实问题:node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs 在第 552 行失败(strictEqual 期望 60,实际为新的路由限定表达式字符串)——23 个测试,22 通过,1 失败。完整 HELPER_TESTS 集合确认这是必需的 Test (ubuntu-latest, Node 22.x) 检查变红的唯一原因:共 500 个测试,仅此一处失败。

但本轮不实施该断言更新:.github/scripts/ 在本 PR 的 footprint 之外(本 PR 只改动 .github/workflows/e2e.ymlscripts/tests/e2e-workflow.test.js),且第 3 轮对同一断言的更新尝试已被 autofix gate 以该理由拒绝并回滚。重复被拒绝的改动只会再次被 gate 拒绝,而在断言仍钉死 60 的情况下,footprint 内没有任何改动能让该检查变绿。需要维护者决定断言更新落在哪里:

  1. 允许本 PR 扩展到 .github/scripts/,下一轮即可在此更新断言(先替换再求值:资源池路由 90、两种托管回退均为 60,与 runs-on 条件保持一致;同时修正过时的 "Pin the ci.yml pool precedent" 注释);或
  2. 由维护者单独落地断言更新(直接提交,或先合并一个小的独立 PR),之后本 PR 重新变基;或
  3. 整体回退本 PR 的 timeout 改动(不建议——等于放弃 Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 的修复)。

在维护者作出选择之前,本线程保持打开。

// Run 33345905817 (issue #10591) expired at the old flat 60-minute
// ceiling on a contended shared ECS host before its log reported any
// test result. The extended ceiling is
// scoped to the pool routing — the same condition `runs-on` uses:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R1-2: The new lane-contract test evaluates only the timeout-minutes expression and never reads runs-on, so the timeout↔routing equivalence this comment claims ("the same condition runs-on uses") is documented but unpinned. e2e.yml carries the routing predicate as two hand-written copies (runs-on at e2e.yml:47 and timeout-minutes at e2e.yml:54), unlike ci.yml where both fields derive from one source of truth (needs.classify_pr.outputs.ubuntu_runner). A future edit to the runs-on condition (adding a kill-switch term or widening pool eligibility) that forgets timeout-minutes leaves every test in this suite green: runs newly routed onto the contended ECS pool keep the 60-minute ceiling and silently re-create issue #10591 (a shard expires before its log reports any test result), or drift the other way and hosted runs get 90 minutes, burning the extra 30 the diff's comment explicitly exists to prevent.

Re-checked at f2b2a2f7 (this round's reviewed commit): the equivalence is still unpinned.

Witness:

Mutation probe: flip != 'true' -> == 'true' on runs-on ONLY (timeout expression untouched)
  PR suite on mutated tree: 11/11 passed (green through a complete inversion of routing semantics)
  equivalence probe (assert timeout == 90 iff runs-on resolves to ecs-qwen): 2 failed | 1 passed
    poolRouted=false but timeout=90
    poolRouted=true but timeout=60   <- the exact #10591 re-creation
  flip check on pristine tree: 3 passed

In the same describe block, evaluate the runs-on expression with the same substitution sweep and assert agreement per scenario instead of asserting each half against test-chosen constants — e.g. assert timeoutMinutesOn(s) === 90 iff the evaluated runs-on expression resolves to the ["self-hosted", "linux", "x64", "ecs-qwen"] branch — so the two expressions answer to one oracle. Note that runs-on at .github/workflows/e2e.yml:47 is a single-quoted YAML scalar with doubled quotes, so after YAML parse the embedded literals carry single quotes; the check must compare the two evaluated results, not a third hand-copied condition. The new equivalence assertion must go red when != 'true' is flipped to == 'true' on only the runs-on expression — today's suite stays green under that mutation; please add it and prove it.

中文说明

新增的通道契约测试只对 timeout-minutes 表达式求值,从不读取 runs-on,因此这条注释所声称的 timeout↔路由等价关系("与 runs-on 使用相同的条件")只是写在注释里,并没有被测试钉住。e2e.yml 中的路由谓词以两份手写副本存在(runs-on 在 e2e.yml:47,timeout-minutes 在 e2e.yml:54),而不像 ci.yml 那样两个字段都从 needs.classify_pr.outputs.ubuntu_runner 这一唯一事实源派生。如果未来某次编辑修改了 runs-on 条件(新增紧急开关项或放宽资源池准入)却忘了 timeout-minutes,本套件的所有测试仍会全绿:新路由到争用 ECS 资源池的运行会继续使用 60 分钟上限,悄悄复现 issue #10591(分片在报告任何测试结果之前过期);反向漂移则会让托管运行获得 90 分钟,白白烧掉本 diff 注释明确要避免的额外 30 分钟。

已在本轮审阅的提交 f2b2a2f7 上复核:该等价关系仍未被测试钉住。

见证:变异探针——只对 runs-on 表达式把 != 'true' 翻转为 == 'true'(timeout 表达式不动),本 PR 的套件在变异树上 11/11 全绿(路由语义被完全反转后依然通过);而等价性探针(断言 timeout == 90 当且仅当 runs-on 解析到 ecs-qwen 分支)在同一变异树上 2 失败 | 1 通过,包括 "poolRouted=false but timeout=90" 与 "poolRouted=true but timeout=60"(即 #10591 的复现形态);在纯净树上做翻转校验:3 通过。

请在同一个 describe 块中,用相同的替换扫描对 runs-on 表达式求值,并按场景断言两者一致,而不是各自对着测试选定的常量断言——例如断言 timeoutMinutesOn(s) === 90 当且仅当求值后的 runs-on 表达式解析到 ["self-hosted", "linux", "x64", "ecs-qwen"] 分支——让两个表达式对同一个裁决者负责。注意:.github/workflows/e2e.yml:47 的 runs-on 是内含双写引号的单引号 YAML 标量,YAML 解析后其中的字面量带单引号;检查应比较两个求值结果,而不是第三份手抄的条件。新的等价断言必须满足:只把 runs-on 表达式中的 != 'true' 翻转为 == 'true' 时变红——当前套件在该变异下保持全绿,请加上断言并运行证明。

— qwen3.8-max via Qwen Code /review (v0.22.3)

@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind main, so it merged current main in via update-branch and will retry on the next scan. A stale base (a dependency or symbol main already changed) can fail the build without being the fix's fault; if it still fails once current, it hands off to a human.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:

Address-review summary — PR #10597 (issue #10591), round 4 (same-run verification repair)

Commits this round: f22f6f5642revert(ci): move the e2e lane-pin update out of this PR's footprint (#10591) (1 file changed, +2/−41). The rejected round-3 commit f31593cbde is preserved as instructed; the follow-up revert repairs its deterministic rejection. Cumulative PR diff vs main: .github/workflows/e2e.yml (+8/−2) and scripts/tests/e2e-workflow.test.js (+77) only — the .github/scripts/ area is net-zero again.

Deterministic rejection repair

The round-3 commit was rejected with: "This round modified CI/verification machinery in area(s) the PR itself never touched: ci-scripts .github/scripts/ci-runner-routing.test.mjs … Revert these files; if the feedback genuinely requires them, escalate it to a maintainer as an open question instead of implementing it."

Action taken: .github/scripts/ci-runner-routing.test.mjs restored to the pre-round state via git restore --source=f2b2a2f7ed and verified byte-identical to origin/main (git diff origin/main -- <file> empty). One additive commit; no history rewritten; nothing else touched.

Feedback triage

[rc:3891763181] / [rc:3892039740] — R1-1 [Critical], e2e.yml:54 — ESCALATED to a maintainer (not implemented)

The finding is real and re-reproduced on the restored tree this round: node --test .github/scripts/ci-runner-routing.test.mjs → 23 tests, 22 pass, 1 fail — the stale `ass

Why it was not pushed:

Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round.

build failed on the agent-committed fix

-wecom@0.22.3 build
> tsc --build


> @qwen-code/channel-feishu@0.22.3 build
> tsc --build


> @qwen-code/channel-qqbot@0.22.3 build
> tsc --build


> @qwen-code/channel-github@0.22.3 build
> tsc --build


> @qwen-code/channel-gitlab@0.22.3 build
> tsc --build


> @qwen-code/channel-plugin-example@0.22.3 build
> tsc --build


> @qwen-code/audio-capture@0.22.3 build:ts
> tsc --build


> @qwen-code/node-repl-mcp@0.1.1 build
> node build.mjs

node-repl-mcp: runtime assets copied to /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/node-repl/dist/runtime

> @qwen-code/acp-bridge@0.22.3 build
> node ../../scripts/build_package.js

Successfully copied files.

> @qwen-code/sdk@0.1.8 build
> node scripts/build.js

Compiling input files...
Processing src/index.ts
Writing src/index.ts -> dist/index.d.ts
Checking generated files...
�[1;33mCompiler option "skipLibCheck" is disabled to properly check generated output�[0m
Done in 41.99s
Compiling input files...
Processing src/daemon/transcript.ts
Writing src/daemon/transcript.ts -> dist/daemon/transcript.d.ts
Checking generated files...
�[1;33mCompiler option "skipLibCheck" is disabled to properly check generated output�[0m
Done in 11.03s
file:///home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/sdk-typescript/scripts/build.js:348
    throw new Error(
          ^

Error: Browser daemon SDK bundle is 220220 bytes; expected <= 220160
    at assertBrowserSafeBundle (file:///home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/sdk-typescript/scripts/build.js:348:11)
    at file:///home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/sdk-typescript/scripts/build.js:225:1
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)

Node.js v22.23.2
npm error Lifecycle script `build` failed with error:
npm error code 1
npm error path /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/sdk-typescript
npm error workspace @qwen-code/sdk@0.1.8
npm error location /home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/packages/sdk-typescript
npm error command failed
npm error command sh -c node scripts/build.js
node:internal/errors:983
  const err = new Error(message);
              ^

Error: Command failed: npm run build --workspace=packages/sdk-typescript
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:916:11)
    at execSync (node:child_process:988:15)
    at file:///home/github-runner/actions-runner-hk-j6c03lyei7s809zq1s6u-7/_work/qwen-code/qwen-code/scripts/build.js:91:3
    at ModuleJob.run (node:internal/modules/esm/module_job:343:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:681:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 1778884,
  stdout: null,
  stderr: null
}

Node.js v22.23.2
🔁 no failure identity in the head transcript — charged to the round
中文说明

🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证,但本 PR 落后于 main,因此已通过 update-branch 合入当前 main,并将在下次扫描时重试。过期的 base(main 已改动的依赖或符号)可能让构建失败而并非修复本身的错;若 base 更新后仍然失败,将移交人工处理。

验证门的拒绝原因与日志证据见上方英文部分(gate-rejection 不翻译)。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/33365133425


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R1-2 timeout↔runs-on mirror unpinned in tests — already reported (comment 3892039748)

Not explored to full depth (tool budget reached): "agent 1a": none — the only check I could not run is the vitest suite itself (no node_modules in the review worktree or parent checkout), which I substituted with the ver…; "agent 3c": none — but note the vitest suite could not be run end-to-end (no node_modules in the review worktree); the Critical was instead verified by reading the exact …; "agent 3b": none — the vitest suite could not be executed in this tree (no node_modules ), but the helper's computation was verified directly, and no check was cut short b….

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • scripts/tests/e2e-workflow.test.js:110 — [review] D3-1 timeoutMinutesOn duplicates the substitute-then-evaluate skeleton (4th copy; guards drifted)
中文说明

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未探索到全部深度(达到工具调用预算):"agent 1a"none — the only check I could not run is the vitest suite itself (no node_modules in the review worktree or parent checkout), which I substituted with the ver…"agent 3c"none — but note the vitest suite could not be run end-to-end (no node_modules in the review worktree); the Critical was instead verified by reading the exact …"agent 3b"none — the vitest suite could not be executed in this tree (no node_modules ), but the helper's computation was verified directly, and no check was cut short b…

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment thread .github/workflows/e2e.yml
# at the old flat 60 on a contended shared host before its log reported
# any test result. Hosted fallbacks keep the pre-contention ceiling so
# a genuine hang there does not burn the extra 30 minutes.
timeout-minutes: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R1-1: [fails-closed] [regression] This change turns e2e-test-linux's timeout-minutes from the number 60 into this expression string, but the existing lane-contract test .github/scripts/ci-runner-routing.test.mjs:552 still pins assert.equal(job['timeout-minutes'], 60) for this exact job and is never updated by this PR. That file imports node:assert/strict (so assert.equal is strictEqual), which means the assertion now fails unconditionally in every routing state. The file is listed in HELPER_TESTS (.github/workflows/ci.yml:82) and executed via node --test by both helper steps — "GitHub CI helper checks" (ci.yml:436) and "Run .github/scripts helper tests" (ci.yml:586); this PR classifies into the full profile, so the required Test (ubuntu-latest, Node 22.x) check goes red on this PR's own CI run and on every run after merge until the pin is updated.

Re-checked at 77eb9c1 (this round's reviewed commit): the stale pin is untouched and the failure still fires.

Witness:

$ node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs   (at HEAD 77eb9c1)
# tests 23
# pass 22
# fail 1
not ok 5 - carries the pool hygiene and capability steps in order
  operator: strictEqual
  expected: 60
  actual: "${{ fromJSON((github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true') && '90' || '60') }}"
  at: .github/scripts/ci-runner-routing.test.mjs:552

A/B on the contested assertion: BASE (pre-change e2e.yml, git show HEAD^2) carries the numeric 60 that satisfies the pin; the PR arm fails with the expression string.

Update .github/scripts/ci-runner-routing.test.mjs:552 in this same PR: replace the bare 60 pin with an evaluation of the new routing-scoped expression — the same substitute-then-evaluate technique this PR adds in scripts/tests/e2e-workflow.test.js — asserting 90 on the pool routing and 60 on both hosted fallbacks (fork and kill-switch), and fix the adjacent stale comment ("Pin the ci.yml pool precedent"), which now contradicts ci.yml's actual pool precedent of 90. Note: the round-3 autofix attempt at exactly this fix was rejected by the autofix gate because .github/scripts/ is outside this PR's footprint, and the change was reverted — if that gate still applies to this PR, this update needs a maintainer decision on where the pin change lands.

The replacement must respect the strict-assert import at .github/scripts/ci-runner-routing.test.mjs:17 (node:assert/strict, so compare like-typed values), must keep the pin as a node:test/node:assert assertion (ci.yml:579-583 notes vitest does not collect node:test files), and must keep the two ceilings mirrored from .github/workflows/ci.yml:219 (90 on ecs-qwen routing, 60 hosted), agreeing with the routing condition the timeout mirrors (runs-on at .github/workflows/e2e.yml:47). The updated assertion itself must go red if timeout-minutes regresses to an unconditional value (a flat 60 or flat 90 without the routing condition) — please remove the routing condition from the assertion and run that subtest to prove it.

中文说明

本改动把 e2e-test-linuxtimeout-minutes 从数字 60 改成了这个表达式字符串,但既有的通道契约测试 .github/scripts/ci-runner-routing.test.mjs:552 仍然对同一个任务钉死 assert.equal(job['timeout-minutes'], 60),且本 PR 没有更新它。该文件导入 node:assert/strict(因此 assert.equalstrictEqual),所以这条断言现在在任何路由状态下都会无条件失败。该文件位于 HELPER_TESTS(.github/workflows/ci.yml:82)中,由两个助手测试步骤通过 node --test 执行——"GitHub CI helper checks"(ci.yml:436)与 "Run .github/scripts helper tests"(ci.yml:586);本 PR 会被分类为 full profile,因此本 PR 自己的 CI 运行中必需的 Test (ubuntu-latest, Node 22.x) 检查会变红,且在更新该断言之前,合并后的每次运行也会变红。

已在 77eb9c1(本轮审阅的提交)上复核:该过时的断言原样存在,失败仍然触发。

见证(在 HEAD 77eb9c1 上执行):node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs → 23 个测试,22 通过,1 失败——失败子测试为 "carries the pool hygiene and capability steps in order",operator: strictEqual,expected: 60,actual 为新的表达式字符串。对该断言做 A/B:BASE(改动前的 e2e.yml,git show HEAD^2)是能满足该断言的数字 60;PR 侧因表达式字符串而失败。

请在同一个 PR 中更新 .github/scripts/ci-runner-routing.test.mjs:552:把裸的 60 钉子替换为对新的路由限定表达式求值——复用本 PR 在 scripts/tests/e2e-workflow.test.js 中新增的"先替换再求值"手法——断言资源池路由为 90、两种托管回退(fork 与紧急开关)均为 60,并修正旁边那条已过时的注释("Pin the ci.yml pool precedent"),它现在与 ci.yml 实际的资源池上限 90 相矛盾。注意:第 3 轮 autofix 曾尝试过完全相同的修复,但因 .github/scripts/ 超出本 PR 的改动范围而被 autofix 门拒绝并回退——如果该限制对本 PR 仍然生效,这次更新需要维护者决定断言改动落在哪里。

替换时必须尊重 .github/scripts/ci-runner-routing.test.mjs:17 的 strict 断言导入(node:assert/strict,即比较同类型值);断言必须保持为 node:test/node:assert 形式(ci.yml:579-583 注明 vitest 不会收集 node:test 文件);并保持与 .github/workflows/ci.yml:219 一致的两个上限(ecs-qwen 路由 90,托管 60),与 timeout 所镜像的路由条件(.github/workflows/e2e.yml:47 的 runs-on)保持一致。更新后的断言本身必须在 timeout-minutes 退化为无条件值(不带路由条件的裸 60 或裸 90)时变红——请从断言中去掉路由条件并运行该子测试来证明这一点。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified real at the current head: node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs fails at line 552 (strictEqual expected 60, actual is the new routing-scoped expression string) — 23 tests, 22 pass, 1 fail. The full HELPER_TESTS set confirms this is the sole cause of the red required Test (ubuntu-latest, Node 22.x) check: 500 tests, exactly this one failure.

This round nevertheless does NOT implement the pin update: .github/scripts/ is outside this PR's footprint (the PR touches only .github/workflows/e2e.yml and scripts/tests/e2e-workflow.test.js), and the round-3 attempt at exactly this pin update was rejected by the autofix gate and reverted for that reason, as this comment itself notes. Repeating the rejected change would fail the gate again, and there is no in-footprint change that can turn the check green while the pin still reads 60. This needs a maintainer decision on where the pin update lands:

  1. Allow this PR to expand into .github/scripts/, so a next round updates the pin here (substitute-then-evaluate: 90 on the pool routing, 60 on both hosted fallbacks, mirroring the runs-on condition; plus the stale "Pin the ci.yml pool precedent" comment); or
  2. A maintainer lands the pin update separately (direct commit, or a small standalone PR merged first), and this PR rebases onto it; or
  3. Revert this PR's timeout change entirely (not recommended — it abandons the Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 fix).

Leaving this thread open until a maintainer chooses.

中文说明

已在当前 head 上确认为真实问题:node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs 在第 552 行失败(strictEqual 期望 60,实际为新的路由限定表达式字符串)——23 个测试,22 通过,1 失败。完整 HELPER_TESTS 集合确认这是必需的 Test (ubuntu-latest, Node 22.x) 检查变红的唯一原因:共 500 个测试,仅此一处失败。

但本轮不实施该断言更新:.github/scripts/ 在本 PR 的 footprint 之外(本 PR 只改动 .github/workflows/e2e.ymlscripts/tests/e2e-workflow.test.js),且正如本评论所述,第 3 轮对同一断言的更新尝试已被 autofix gate 以该理由拒绝并回滚。重复被拒绝的改动只会再次被 gate 拒绝,而在断言仍钉死 60 的情况下,footprint 内没有任何改动能让该检查变绿。需要维护者决定断言更新落在哪里:

  1. 允许本 PR 扩展到 .github/scripts/,下一轮即可在此更新断言(先替换再求值:资源池路由 90、两种托管回退均为 60,与 runs-on 条件保持一致;同时修正过时的 "Pin the ci.yml pool precedent" 注释);或
  2. 由维护者单独落地断言更新(直接提交,或先合并一个小的独立 PR),之后本 PR 重新变基;或
  3. 整体回退本 PR 的 timeout 改动(不建议——等于放弃 Main CI failed: E2E Tests on cd5d5af2fbd7 #10591 的修复)。

在维护者作出选择之前,本线程保持打开。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@yiliang114

Copy link
Copy Markdown
Collaborator

Closing: the linked issue #10591 was closed as resolved by #10567 (E2E workers serialized on shared ECS hosts). The shared-host contention this timeout adjustment targeted is already addressed by the merged #10552 / #10567 / #10605 series, and main E2E is green.

@yiliang114 yiliang114 closed this Aug 31, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/10 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #10597 (issue #10591)

Feedback dispositions

Implemented

  • R1-2 (Suggestion)rc:3891763185, rc:3892039748 — RESOLVED IN CODE. The timeout↔runs-on equivalence is now pinned: the lane-contract block evaluates BOTH hand-written copies of the routing predicate (runs-on and timeout-minutes in .github/workflows/e2e.yml) through one shared substitute-then-evaluate helper and asserts, per routing scenario, that the ceiling is 90 if and only if runs-on resolves to the ecs-qwen pool branch. The two ceiling tests (90 pool / 60 hosted) are unchanged in what they pin. While implementing this, the block's previously duplicated evaluator was consolidated into the one shared helper (one guard regex instead of two), per the round's subtractive-fix guidance.

Escalated to maintainer (not implemented)

  • R1-1 (Critical)rc:3891763181, rc:3892039740, rc:3893130441 — verified real and reproduced at the current head, but the fix is outside this PR's footprint and a maintainer decision is required. Details:
    • Reproduced: node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs → 23 tests, 22 pass, 1 fail — carries the pool hygiene and capability steps in order, strictEqual expected 60, actual is the new routing-scoped expression string (assertion at .github/scripts/ci-runner-routing.test.mjs:552).
    • Scope check: the full HELPER_TESTS set (22 files, exactly what the CI Test job runs) reports 500 tests / 499 pass / 1 fail — the R1-1 pin is the SOLE failure, so it fully explains the red Test (ubuntu-latest, Node 22.x) check.
    • Why not implemented: the fix requires editing .github/scripts/ci-runner-routing.test.mjs, which is outside this PR's footprint (the PR touches only .github/workflows/e2e.yml and scripts/tests/e2e-workflow.test.js). The round-3 attempt at exactly this pin update was rejected by the autofix gate and reverted for that reason; repeating it would fail the gate again. No in-footprint change can turn the check green while the pin still reads 60. The threads are left OPEN with an explicit question to the maintainer (see comment replies): (1) allow this PR to expand into .github/scripts/, or (2) land the pin update separately and rebase this PR, or (3) revert the timeout change (not recommended).

No action

  • Review bodies rv:5063177930, rv:5063499272, rv:5064838103 — their actionable content is only "Not linted (tool limitation, not a blocker)" for .github/workflows/e2e.yml (actionlint embedded-shell source mapping unsupported); the findings themselves are the inline comments handled above. Nothing further to address.
  • Deferred notes embedded in rounds 2–3 (timeoutMinutesOn skeleton duplication) were marked "recorded, not requested in this round"; this round's consolidation of the two evaluators in e2e-workflow.test.js into one shared helper addresses the in-file part of that observation without touching other files.

Changes this round

  1. Merged origin/main (8 commits). This repairs the previous round's verification-gate rejection ("build failed on the agent-committed fix": Browser daemon SDK bundle is 220220 bytes; expected <= 220160). That failure class is removed by main's fix(sdk): make daemon bundle size advisory (#10630), which was not in the branch when the rejection happened; the merge also re-aligns the branch with the auto-updated base. No conflicts in this PR's files.
  2. scripts/tests/e2e-workflow.test.js (+36/−21): one shared substitute-then-evaluate helper for both routing expressions; new keeps the ceiling mirrored with the runs-on routing test asserting timeout ↔ runs-on agreement per scenario; commit 3c48c3bd2d.

Mutation probes (witness verification)

  • Flip != 'true'== 'true' on runs-on ONLY (the exact mutation R1-2 named, which previously left the suite green): the new mirror test FAILS (timeout and runs-on disagree for {"repository":"QwenLM/qwen-code","ecsDisabled":""} — 1 failed | 12 passed). Restored → 13/13 green.
  • Flatten timeout-minutes to unconditional 60: pool-ceiling test + mirror test FAIL (2 failed | 11 passed). Restored.
  • Flatten timeout-minutes to unconditional 90: hosted-ceiling test + mirror test FAIL (2 failed | 11 passed). Restored.

Failed-check notes

  • Test (ubuntu-latest, Node 22.x) — fully explained by the R1-1 stale pin (499/500 HELPER_TESTS pass; sole failure is the pin). Goes green once the pin update lands per the maintainer decision.
  • web-shell E2E Smoke (ubuntu-latest, Node 22.x) — failed on CI. This PR's diff touches neither web-shell nor ci.yml; the browser E2E harness is not among this round's allowed verification commands, so it is recorded here and left to the workflow's CI gate.
  • Post Coverage Comment — collateral of the failed Test job (failed within 11s; no coverage artifact to post while Test is red).
  • review-pr — CANCELLED (no action).

Verification

  • npm run build — PASSED (exit 0; the previous round's sdk-typescript bundle-size build failure no longer reproduces after merging main, per fix(sdk): make daemon browser bundle size advisory #10630)
  • npm run typecheck — PASSED
  • npm run lint — PASSED
  • focused vitest scripts/tests/e2e-workflow.test.js (touched file) — 13 passed (13)
  • mutation probes (3) — all witnesses verified as described above; tree restored after each probe
  • npm run test:scripts (full suite, no concurrent load) — 2017 passed | 16 skipped | 2 failed; both failures characterized as load/timing flakes unrelated to this PR:
    • upload-aliyun-oss-assets.test.js "kills a stalled attempt and retries it when attemptTimeoutMs is set" — passed 16/16 when rerun alone on this branch.
    • qwen-autofix-workflow.test.js — a 30s testTimeout, hitting a DIFFERENT test in every run ("behaviorally replays the stale-duplicate revalidation…" in the suite run, "upserts deferred findings…" in the first alone run). Rerun alone on this branch: 229/229 passed. Rerun alone on origin/main (f772343, detached checkout, then back): 229/229 passed. The suite reads neither file this PR changes (verified by grep), so there is no mechanism for this PR to affect it.
  • node --test --test-concurrency=1 over all HELPER_TESTS (the CI Test job's helper step) — 500 tests: 499 pass, 1 fail = the R1-1 stale pin (escalated above)
中文说明

Autofix 审阅轮次 — PR #10597(issue #10591

反馈处置

已实施

  • R1-2(Suggestion)rc:3891763185rc:3892039748 — 已在代码中解决。timeout↔runs-on 的等价关系现在被测试钉住:通道契约块通过一个共享的"先替换再求值"助手,对 .github/workflows/e2e.yml 中路由谓词的两份手写副本(runs-ontimeout-minutes)同时求值,并按场景断言:上限为 90 当且仅当 runs-on 解析到 ecs-qwen 资源池分支。两条上限测试(资源池 90 / 托管 60)钉住的内容不变。实现过程中,该块内原先重复的求值器被合并为一个共享助手(守卫正则从两份收敛为一份),符合本轮"减法优先"的指引。

升级给维护者(未实施)

  • R1-1(Critical)rc:3891763181rc:3892039740rc:3893130441 — 已在当前 head 上确认为真实问题并复现,但修复位于本 PR 的 footprint 之外,需要维护者决定。详情:
    • 复现:node --test --test-concurrency=1 .github/scripts/ci-runner-routing.test.mjs → 23 个测试,22 通过,1 失败 —— carries the pool hygiene and capability steps in orderstrictEqual 期望 60,实际为新的路由限定表达式字符串(断言位于 .github/scripts/ci-runner-routing.test.mjs:552)。
    • 范围核查:完整 HELPER_TESTS 集合(22 个文件,与 CI Test 任务实际执行的完全一致)报告 500 个测试 / 499 通过 / 1 失败 —— R1-1 的过时断言是唯一失败项,因此完整解释了变红的 Test (ubuntu-latest, Node 22.x) 检查。
    • 为何未实施:修复必须编辑 .github/scripts/ci-runner-routing.test.mjs,而该文件在本 PR 的 footprint 之外(本 PR 只改动 .github/workflows/e2e.ymlscripts/tests/e2e-workflow.test.js)。第 3 轮对同一断言的更新尝试已被 autofix gate 以该理由拒绝并回滚;重复该改动会再次被拒绝。在断言仍钉死 60 的情况下,footprint 内没有任何改动能让该检查变绿。三条线程保持打开,并向维护者提出明确问题(见评论回复):(1) 允许本 PR 扩展到 .github/scripts/;或 (2) 单独落地断言更新后本 PR 变基;或 (3) 回退 timeout 改动(不建议)。

无需处理

  • 审阅主体 rv:5063177930rv:5063499272rv:5064838103 — 其可操作内容仅为 .github/workflows/e2e.yml 的"未检查(工具限制,非阻断)"(actionlint 尚不支持 embedded-shell source mapping);发现本体即上面已处理的内联评论。无其他待办。
  • 第 2–3 轮内嵌的延后记录(timeoutMinutesOn 骨架重复)标注为"已记录,本轮不要求修改";本轮将 e2e-workflow.test.js 内的两个求值器合并为一个共享助手,在不触及其他文件的前提下处理了该观察中属于本文件的部分。

本轮改动

  1. 合并 origin/main(8 个提交)。这修复了上一轮验证 gate 的拒绝原因("build failed on the agent-committed fix":Browser daemon SDK bundle is 220220 bytes; expected <= 220160)。该失败类别已由 main 上的 fix(sdk): make daemon bundle size advisory (#10630) 消除,而该提交在拒绝发生时尚不在本分支上;合并同时让分支重新对齐自动更新后的 base。本 PR 的文件无冲突。
  2. scripts/tests/e2e-workflow.test.js(+36/−21):一个共享的"先替换再求值"助手同时服务两个路由表达式;新增 keeps the ceiling mirrored with the runs-on routing 测试,按场景断言 timeout ↔ runs-on 一致;提交 3c48c3bd2d

变异探针(见证验证)

  • 只对 runs-on!= 'true' 翻转为 == 'true'(即 R1-2 指出的、此前整套测试保持全绿的变异):新增的镜像测试失败(timeout and runs-on disagree for {"repository":"QwenLM/qwen-code","ecsDisabled":""} —— 1 失败 | 12 通过)。恢复后 → 13/13 全绿。
  • timeout-minutes 退化为无条件 60:资源池上限测试 + 镜像测试失败(2 失败 | 11 通过)。已恢复。
  • timeout-minutes 退化为无条件 90:托管上限测试 + 镜像测试失败(2 失败 | 11 通过)。已恢复。

失败检查说明

  • Test (ubuntu-latest, Node 22.x) —— 完全由 R1-1 的过时断言解释(HELPER_TESTS 499/500 通过,唯一失败即该断言)。待维护者决定断言更新落点后即可变绿。
  • web-shell E2E Smoke (ubuntu-latest, Node 22.x) —— CI 上失败。本 PR 的 diff 既不触及 web-shell 也不触及 ci.yml;浏览器 E2E 装置不属于本轮允许的验证命令,故在此记录,交由工作流的 CI gate 裁定。
  • Post Coverage Comment —— Test 任务失败的连带后果(11 秒内失败;Test 变红时无覆盖率产物可发布)。
  • review-pr —— CANCELLED(无需处理)。

验证

  • npm run build — 通过(exit 0;合并 main 后上一轮的 sdk-typescript 包体积构建失败不再复现,见 fix(sdk): make daemon browser bundle size advisory #10630
  • npm run typecheck — 通过
  • npm run lint — 通过
  • 聚焦 vitest scripts/tests/e2e-workflow.test.js(触及文件)— 13 通过(13)
  • 变异探针(3 个)— 所有见证均按上文验证;每个探针后均恢复原状
  • npm run test:scripts(全套,无并发负载)— 2017 通过 | 16 跳过 | 2 失败;两处失败均定性为与本 PR 无关的负载/时序抖动:
    • upload-aliyun-oss-assets.test.js 的 "kills a stalled attempt and retries it when attemptTimeoutMs is set" —— 在本分支上单独重跑 16/16 通过。
    • qwen-autofix-workflow.test.js —— 30 秒 testTimeout 超时,且每次运行命中的测试都不同(全套运行命中 "behaviorally replays the stale-duplicate revalidation…",首次单独运行命中 "upserts deferred findings…")。在本分支单独重跑:229/229 通过;在 origin/main(f772343c78,detached checkout 后已切回)单独重跑:229/229 通过。该套件不读取本 PR 改动的任何文件(已用 grep 验证),因此本 PR 不存在影响它的机制。
  • 对全部 HELPER_TESTS 执行 node --test --test-concurrency=1(与 CI Test 任务的助手步骤一致)— 500 个测试:499 通过,1 失败 = R1-1 的过时断言(已升级给维护者)

🦷 Gate advisory — this round resolves a Critical/Request-changes finding with test-only changes (machine-measured): the bite check cannot verify a test-side fix, so the resolution rests on the round summary alone. · 本轮以纯测试改动解决 Critical/Request-changes 反馈(门自动测量):bite 检查无法验证测试侧修复,该解决仅以轮次摘要为凭。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

⚠️ Review-thread resolution skipped — guard: live-head drift; resolved 0 of 2 selected thread(s), 2 left for a later round. · 评审线程关闭被跳过——守卫:live-head drift;选中 2 条,本轮关闭 0 条,其余 2 条留待后续轮次。

Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Main CI failed: E2E Tests on cd5d5af2fbd7

4 participants