fix(ci): isolate and bound release validation workloads - #10765
Merged
Conversation
Pin every release validation job to the hk4 host label, move the nightly and preview crons out of the pool's peak hours and apart from each other, give the web-shell Vitest project the same shared-ECS timeout ceiling as cli/core, and retry timing-only workspace test failures twice. Claude-Session: https://claude.ai/code/session_01AWWgJEqafyAT1Mc75T8N7h
Collaborator
Collaborator
Author
|
Follow-ups deliberately left out of this PR, in the order I would do them:
中文说明这个 PR 有意没做的后续，按我建议的顺序：
|
Collaborator
Author
|
Why these two cron times, for the record:
The cost is that the preview cutoff moves from Tuesday 23:59 UTC (07:59 Wednesday Beijing) to Tuesday 17:00 UTC (01:00 Wednesday Beijing). Everything merged during the Beijing Tuesday workday is still included; PRs merged on a US Tuesday afternoon now roll into the following week's preview. If that matters, Tuesday 23:00 UTC also works (nightly is done by ~22:30, and 07:00 Beijing is just ahead of the peak), at the price of about one hour of margin before PR CI ramps up. |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Observed, with unusually good evidence. Seven failed release runs are listed by run ID, every failure is timing-only (no test failed twice across runs), and the per-host job counts show the failing shards all landed on hk3/hk5 while those hosts ran 12–18 concurrent jobs, while the one successful run sat on hk4 with ~6. This is a diagnosed failure class, not a theoretical concern. Direction: Aligned — failing nightlies block the release lane, and this is a direct continuation of #10667 and #10619 (same failure class, same lane). Keeping publishing on hosted runners and the Size: No core paths touched. 88 changed lines total: 42 CI workflow/config, 8 docs, 8 test config, 30 tests. Approach: The scope feels right — pinning validation to the least-contended host, de-overlapping the two crons, raising the timeout ceiling in the one package still on vitest's 5s default, and Risk: No elevated risk signals (no high-risk paths matched). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到，且证据相当充分。PR 列出了 7 次失败运行的 run ID，所有失败都是时间类的（没有用例在两次运行中重复失败），按机器统计的 job 数显示失败分片全部落在正同时跑 12–18 个 job 的 hk3/hk5 上，而唯一成功的一次在只有约 6 个并发 job 的 hk4。这是已诊断的失败类别，不是理论性问题。 方向：对齐——nightly 失败会卡住 release 链路，本 PR 是 #10667 和 #10619 的直接延续（同一失败类别、同一链路）。发布保留在托管 runner、不动 规模：未触及核心路径。共 88 行改动：42 行 CI workflow/配置、8 行文档、8 行测试配置、30 行测试。 方案：范围合理——把校验固定到争抢最少的机器、错开两个 cron、给仍在 vitest 默认 5 秒超时的那个包提高上限、release 链路加 风险：无升级风险信号（未命中高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewIndependent baseline first: for "release validation times out on contended hosts" I would have done exactly this — a dedicated label pinning validation to the least-contended host (hosted fallback untouched), the two crons moved into quiet hours and away from each other, the 5s-default timeout raised in the one package still missing the ECS branch, retries only on the release lane, and a guard so cron and classification cannot drift apart again. The PR matches that proposal; I don't see a simpler shape that covers the failure class. What I verified in the diff:
One suggestion, non-blocking: the new comment says dropping the plain TestingThis is an unattended run, so the evidence below is the PR's own CI fetched via the API — no PR code was built or executed here. CI on this commit is red, but not because of this diff: all four failing lanes die in the same The
中文说明代码审查先说独立判断：对"争抢机器导致 release 校验超时"这个问题，我会做的正是这一套——用专门标签把校验固定到争抢最少的机器（托管兜底不动）、把两个 cron 挪到空闲时段并彼此错开、给仍缺 ECS 分支的那个包提高 5 秒默认超时、只在 release 链路加重试，再加一个防止 cron 与判定再次脱节的守卫。PR 与该方案一致；我没有看到更简单、又能覆盖这一失败类别的形态。 diff 中已核实：
一条建议，不阻塞：新增注释说从 hk4 runner 上移除普通 测试这是无人值守运行，以下证据来自 PR 自身 CI 的 API 抓取——未构建或执行任何 PR 代码。本提交的 CI 是红的，但不是这个 diff 造成的：四条失败全部死在同一个 （CI 结果表格见上方，四条失败共享同一段 tsc 错误摘录。） — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 3/5 — clean review on an otherwise-solid PR; approval is held only because this commit's CI is red from a pre-existing main breakage, not from the diff. Stepping back: the motivation is seven failed release runs with a coherent diagnosis and run IDs, the implementation is the minimal set of complementary fixes, and every edit in the diff earns its place — no drive-bys. My independent proposal and the PR's approach are the same shape, so there is no simpler path I can offer. The schedule-pin test is the part I'd be glad to find in six months. The one nit stands: the comment (and PR body) claim about making hk4 release-only "without touching this file" is wrong under AND label matching and should be corrected, but it doesn't affect today's behavior. Why no approval in this run, plainly: all four red lanes fail in 中文说明置信度：3/5 —— 对一个本身扎实的 PR 审查干净；暂不批准，仅因为当前提交的 CI 因 main 上已有的损坏而红，与 diff 无关。 退一步看：动机是 7 次失败的 release 运行、一份自洽的诊断和 run ID；实现是互补修复的最小组合，diff 里每一处改动都有存在理由，没有夹带。我的独立方案与 PR 的做法同形，没有更简单的路可提。schedule 对照测试是半年后最让人庆幸会存在的那部分。唯一的 nit 仍然成立：注释（和 PR 正文）里"无需改动本文件即可让 hk4 只跑 release"的说法在 AND 标签匹配下是错的，应当修正，但不影响当前行为。 本轮为何不批准，直说：四条红检查全部死在 — Qwen Code · qwen3.8-max Reviewed at |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
yiliang114
changed the title
Collaborator
Author
|
Updated this PR through
Verification: the focused Release workflow and package-script suites pass with 64 tests passed / 1 skipped; changed-file ESLint, Prettier, and This PR intentionally does not claim to fix the independent tar |
Collaborator
|
Verification report at head Static review
Live verification (diff applies cleanly onto local main
Non-blocking note: this PR touches the same Not approved: CI at head still pending and no gate approvals on record. No Critical found — no blocking feedback. |
yiliang114
commented
Sep 2, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review findings (self-PR, cannot self-approve):
- Cron moves are safe: both schedules land in the ECS pool's quiet hours, and the new test pins every schedule cron string verbatim against prepare's comparisons, so a cron edited without its comparison can no longer silently no-op the schedule.
- Concurrency design is right: scheduled runs serialize on one group with cancel-in-progress=false (a delayed preview queues behind the nightly instead of overlapping on the reserved host), while manual runs keep per-run unique groups so a dry run can never supersede a pending real release in GitHub's non-FIFO queue.
- Pinning validation to the dedicated hk4 label (registered in actionlint.yaml, deliberately not the shared ecs-qwen label) is documented with the exit hatch: infra can drop ecs-qwen from hk4 without making the jobs unroutable; the routing test carries the new runs-on expression.
- VITEST_RETRY semantics are correct: nightly/preview absorb transient timing failures (retry 2), stable stays strict (0); the flag is pinned in the runner test along with the env expression.
- quality_scripts gains the same ECS vitest ceilings as the workspace leg (test-extended to both steps), and web-shell's jsdom timeouts join the clamp-parity suite (imported under the RUNNER_NAME stub like the others).
No blocking issues found.
qwen-code-dev-bot
approved these changes
Sep 2, 2026
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at head f875f060.
- The schedule move into the ECS pool's quiet hours keeps prepare's nightly/preview classification correct because the comparison strings moved with the crons, and a new test closes that drift class permanently by asserting every cron in the workflow appears verbatim in the prepare comparison. The dedicated concurrency block serializes scheduled validations (
cancel-in-progress: false) while giving manual runs unique groups, so a queued dry run can never supersede a pending real release. - Retries are graded sensibly: nightly/preview may absorb two transient timing failures, stable releases keep
--retry=0strictness — pinned by the exact-string test. The web-shell timeout ceilings match the existing core/cli ECS branch and are folded into the parity ratchet, and the scripts lane clamp copies the workspace lane's tunable binding verbatim. - Runner routing: the new
ecs-qwen-hk4-hostlabel is registered for actionlint, every validation job's expression updated in lockstep, and the hosted fallback behindMAINTAINER_ECS_RUNNER_DISABLEDis preserved (with the deliberate choice to drop the shared label requirement documented). - No prior review rounds or open threads; CI on this head has no failures (Test/review pipeline running); per the channel convention the call is on the review itself.
chiga0
approved these changes
Sep 2, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Tier: Standard (CI/infra change; concurrency lifecycle present; no persisted format, no new permissions or secrets)
Scope: .github/workflows/release.yml, .github/actionlint.yaml, packages/web-shell/vitest.config.ts, three contract-test files, docs/developers/development/npm.md. NOT reviewed — no working tree available for local execution (rung 1-3 not applicable to CI-config-only change with no runnable product surface).
No blocking findings.
Approval blockers: none.
Checked:
Class 1 — cron contract: 0 21 * * * and 0 17 * * 2 appear in both on.schedule and the prepare vars-step comparisons. New test classifies each schedule cron by the exact string it fires with enforces this contract mechanically; a cron edit without the matching comparison string edit fails the test suite. Clean.
Class 2 — runner label: ecs-qwen-hk4-host added to actionlint.yaml allowlist; MAINTAINER_ECS_RUNNER_DISABLED hosted fallback preserved. Three jobs intentionally staying on ubuntu-latest (quality aggregator, publish, notify_failure) are not in validationJobs and correctly excluded. Clean.
Class 5 — test validity: the routing test updates ecsRunsOn to the new label and exercises all 8 validation jobs; any drift in runs-on fails that assertion. VITEST_RETRY test validates the exact expression string. unit-vitest-configs.test.ts stubs RUNNER_NAME=ecs-qwen-parity, imports web-shell config under that stub, and asserts testTimeout/hookTimeout match the autofix gate clamps (60000). Scripts bounds test extended to cover quality_scripts step. All new tests are specific enough to fail on regression.
Class 10 — stated intent: PR claims "both workspace and scripts lanes use the existing ECS worker bound" — confirmed, quality_scripts now has the four VITEST_* bounds env vars. Scripts job intentionally omits VITEST_RETRY (no retry for scripts tests; not claimed in PR description). Concurrency group structure (release-scheduled-validation for scheduled, unique group per run_id for manual, cancel-in-progress: false) matches PR description and validated by dedicated test. Clean.
UTC-date shift documented: nightly at 21:00 UTC labels one calendar day earlier than the old midnight schedule; PR body and docs both state this explicitly.
Cross-check: existing reviewer (qwen-code-dev-bot, approved) and author self-comment both confirm the cron contract and routing consistency. No inline comments filed by anyone. No misses to record.
Reviewed with AI assistance.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
approved these changes
Sep 2, 2026
qwen-code-dev-bot
pushed a commit
that referenced
this pull request
Sep 2, 2026
main's 7df5ac6 (#10765) landed the same Vitest worker bound this branch added to the release scripts lane, and pinned it with its own contract test asserting the four VITEST_* variables literally in both lanes. Resolve both conflicts to main's shape: its fix is the superset (dedicated hk4 host, serialized concurrency, rescheduled crons, nightly/preview retry), and its workspace lane now carries a lane-only VITEST_RETRY that an env-block alias would leak into the scripts lane. That drops this branch's YAML anchor dedup of the two env blocks, so the reviewer Suggestion it answered stays open for a maintainer to call: re-applying it now means rewriting the contract test #10765 just merged. What survives is the orthogonal deflake in the upload-stall test. Its 400ms attempt bound raced node's cold-start latency, so the SIGKILL could land before the shim's first appendFileSync and lose that attempt's log line. Measured under 20-way single-CPU contention, 400ms loses the log entirely (ENOENT reading the attempt count) while 2000ms passes, worst case 12.2s inside the test's 30s budget.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR makes release validation use a bounded, separately routable execution lane instead of relying on whichever shared ECS host happens to be least busy.
ecs-qwen-hk4-hostlabel without also requiring the sharedecs-qwenlabel. This keeps the workflow routable after infrastructure removes the shared label from hk4; the existing hosted-runner fallback remains available throughMAINTAINER_ECS_RUNNER_DISABLED.Why it's needed
Recent Release runs showed large host-dependent timing differences. The self-hosted fleet exposes many logical runners per physical machine, so multiple PR, E2E, integration, and release jobs could saturate one host even though GitHub treated them as independent runners. The Tuesday preview and daily nightly were also scheduled one minute apart, creating two full validation DAGs at the same time.
The repository-side changes bound each Vitest process, route validation through a dedicated label, and prevent the two scheduled releases from overlapping. To complete the host reservation, infrastructure should remove the generic
ecs-qwenlabel from the hk4 runners after this lands; the workflow intentionally no longer depends on that label.Reviewer Test Plan
How to verify
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/release-workflow.test.js scripts/tests/package-scripts.test.js. Expect 64 tests passing and one skipped.ecs-qwen-*runner name and verify both test and hook timeouts resolve to 60000.ecs-qwen-hk4-*runner, the workspace lanes use retry 2, and the scripts lane has the same worker cap as the workspace lanes.Evidence (Before & After)
N/A — CI configuration only. The Release runs used for diagnosis include 33576013293, 33574663722, 33520204570, 33455887802, 33361682601, and 33282281913; run 33401719747 was the successful comparison point.
Tested on
Environment (optional)
Local Linux-compatible Node.js workspace; focused workflow-contract tests and changed-file lint/format checks.
Risk & Scope
MAINTAINER_ECS_RUNNER_DISABLED=trueto use hosted runners during host maintenance.write after enderror from run 33576013293 or the shared extension-store lock from run 33574663722. Those need separate deterministic fixes rather than broader retries.Linked Issues
Related to #10755 and #10757. Follows up on #10667 and #10619.
中文说明
这个 PR 做了什么
这个 PR 为 release 校验建立了有界、可独立路由的执行通道，不再依赖共享 ECS 池中哪台机器恰好比较空闲。
ecs-qwen-hk4-host标签，不再同时要求共享的ecs-qwen标签。这样基础设施从 hk4 移除共享标签后，workflow 仍然可以正常路由；现有的MAINTAINER_ECS_RUNNER_DISABLED托管 runner 兜底保持不变。为什么需要
最近的 Release run 表现出明显的主机相关耗时差异。自托管资源池在每台物理机上注册了多个逻辑 runner，因此 PR、E2E、integration 和 release job 可能同时压满同一台物理机，而 GitHub 会把它们当成互相独立的 runner。周二 preview 和每日 nightly 此前也只相差一分钟，会同时启动两套完整校验 DAG。
仓库侧改动会限制每个 Vitest 进程的并发，通过专用标签路由校验，并阻止两次定时发布互相重叠。为了真正完成主机预留，基础设施需要在本 PR 合入后从 hk4 runner 移除通用的
ecs-qwen标签；workflow 已经刻意不再依赖该标签。Reviewer Test Plan
如何验证
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/release-workflow.test.js scripts/tests/package-scripts.test.js，预期 64 个测试通过、1 个跳过。ecs-qwen-*runner 名加载 Web Shell 配置，确认测试和 hook 超时都解析为 60000。ecs-qwen-hk4-*runner，workspace 通道使用 retry 2，并且 scripts 通道与 workspace 通道使用相同 worker 上限。前后证据
N/A——仅 CI 配置改动。诊断参考的 Release run 包括 33576013293、33574663722、33520204570、33455887802、33361682601 和 33282281913；33401719747 作为成功对照。
测试平台
环境（可选）
本地 Linux 兼容 Node.js workspace；执行了聚焦 workflow 契约测试以及改动文件的 lint/format 检查。
风险与范围
MAINTAINER_ECS_RUNNER_DISABLED=true使用托管 runner。write after end，或者 run 33574663722 的共享 extension-store 锁。这些问题需要单独的确定性修复，不能依赖更宽泛的重试。关联 Issue
关联 #10755 和 #10757；跟进 #10667 和 #10619。