fix(ci): coordinate Docker sandbox workloads on shared ECS hosts - #10605
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The linked issue #10595 is an auto-filed main-CI failure (run 33351032808): the docker shard's fixed-session route test returned HTTP 500 three times over ~30s while the same commit's non-docker shard passed, and a sibling shard logged Docker API Direction: aligned — this is the repo's own CI reliability on the shared self-hosted pool. No product surface involved, so no CHANGELOG signal applies; the area is clearly relevant. Size: no core paths touched. ~183 production lines (e2e.yml 116, release.yml 58, build_sandbox.js 9) vs ~133 test lines. Below every escalation threshold. Author is a maintainer, same-repo branch. Approach: matches what I would have proposed for this failure shape — build the image once per commit and reuse it (commit-qualified tag), pin the test process to the resolved immutable image ID, and coordinate via a host-wide reader/writer lock so builds/cleanup take exclusive access while all shards test concurrently under shared access. Keeping image choice, build, ID capture, and the tests in one shell step is what closes the retag/prune window. Release-lane participation closes the second build/prune path on the pool, and folding its two vitest steps into one is required for lock continuity — neither is scope creep. One non-blocking item I'll flag in the code review: the per-commit coordinator lock files under Risk: no high-risk-path matches (all changes under Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的问题，不是理论性的。关联 issue #10595 是自动提交的 main CI 失败（run 33351032808）：docker shard 的 fixed-session 路由测试在约 30 秒内三次返回 HTTP 500，而同一 commit 的非 docker shard 通过；兄弟 shard 还在 Buildx 检查 builder 容器时记录了 Docker API 方向：对齐——这是仓库自身在共享 self-hosted 池上的 CI 可靠性。不涉及产品面，CHANGELOG 信号不适用，该领域显然相关。 规模：未触及核心路径。约 183 行生产代码（e2e.yml 116、release.yml 58、build_sandbox.js 9）对约 133 行测试代码。低于所有升级阈值。作者是 maintainer，同仓库分支。 方案：与我对这个故障形态的独立提议一致——每 commit 构建一次镜像并复用（带 commit 的 tag），把测试进程固定到解析出的不可变 image ID，并通过主机级读写锁协调：构建/清理取独占访问，所有 shard 以共享访问并发测试。镜像选择、构建、ID 捕获与测试放在同一个 shell step 中，正是消除 retag/prune 空窗的关键。release 通道参与协议，关闭了池上第二条构建/清理路径；其两个 vitest step 合并为一个是锁连续性的必要要求——都不是范围蔓延。一个非阻塞项会在代码审查中指出： 风险：无高风险路径命中（全部改动在 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewMy independent proposal for this failure shape, written before reading the diff: build the image once per commit, reuse it across shards, pin the test process to the resolved immutable image ID, and serialize only the build/prune phases with a host-level lock while tests stay concurrent. This PR does exactly that, with a readers/writer split that is the right structure — the comparison found no simpler path it missed. Things I verified against the code rather than taking on trust:
One non-blocking observation for a follow-up: the per-commit coordinator lock files ( TestingThis is an unattended CI run — per policy I did not build or execute any PR code. The evidence below is the PR's own CI on the reviewed commit, quoted by check name and conclusion. The updated workflow contract tests run in the still-in-flight unit lane ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 What no PR check can settle: the central claim — that the lock protocol actually removes shared-daemon contention on the ECS pool. e2e.yml has no Real-scenario testing: N/A — CI infrastructure with no user-visible surface (and this is an unattended run). 中文说明代码审查：我在读 diff 之前的独立方案是——每 commit 构建一次镜像并跨 shard 复用、把测试进程固定到解析出的不可变 image ID、用主机级锁只对构建/清理阶段串行化而测试保持并发。本 PR 正是这样做的，且读写锁分离是正确结构——对比之下没有发现它遗漏的更简路径。 逐项核实（而非听信描述）： 一个非阻塞的后续建议：按 commit 命名的协调锁文件会在常驻池主机的 测试：这是无人值守 CI 运行——按策略我没有构建或执行任何 PR 代码。证据是所审提交上 PR 自己的 CI（按检查名与结论引用）。更新后的工作流契约测试位于仍在运行的单元测试通道（ 任何 PR 检查都无法定论的部分：核心主张——锁协议确实消除了 ECS 池上的共享 daemon 竞争——e2e.yml 没有 真实场景测试：N/A——纯 CI 基础设施，无用户可见面（且为无人值守运行）。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean review with one non-blocking follow-up (lock-file sweep); the only open item is CI still in flight. Stepping back: this is a maintainer-authored fix for an observed failure with a linked failing run, and the mechanism is the minimal one that actually removes the race instead of papering over it — no timeout bumps, no matrix reduction. The lock protocol is more machinery than a naive fix, but each piece earns its place: the readers/writer split is exactly what keeps the three docker shards concurrent, and the single-step design is what makes the image-ID pin trustworthy. The contract tests pin the load-bearing invariants (lock ordering, bounded retry, ID pin, uninterrupted lock lifetime, release-lane participation), which is what keeps shell-in-YAML maintainable in six months. The remaining uncertainty — the lock-file accumulation nit, and the writer-preference tradeoff the author already disclosed — is non-blocking. Not verified, stated plainly: live shared-ECS behavior (one build per commit/host, sibling shards reusing the image, no more repeated 500s) only becomes observable after merge, since e2e.yml never runs on PRs. That is inherent to the change, not a gap in this review. Approval is deferred until CI lands green on 中文说明置信度：4/5 —— 审查干净，仅一个非阻塞后续项（锁文件清理）；唯一未决事项是 CI 仍在运行。 退一步看：这是维护者针对有失败 run 佐证的已观测问题提交的修复，其机制是真正消除竞争的最小方案——没有提高超时、没有缩减矩阵。锁协议比朴素修复多一些机制，但每一部分都有存在理由：读写锁分离正是三个 docker shard 保持并发的原因，单 step 设计正是 image ID 固定可信的原因。契约测试钉住了关键不变量（锁顺序、有界重试、ID 固定、锁生命周期不中断、release 通道参与），这正是写在 YAML 里的 shell 六个月后仍可维护的原因。剩余的不确定——锁文件累积的小问题、作者已披露的写者优先权衡——均非阻塞。 明确说明未验证的部分：共享 ECS 上的真实行为（每 commit/每主机一次构建、兄弟 shard 复用镜像、不再出现连续 500）只能在合并后观察到，因为 e2e.yml 不在 PR 上运行。这是该改动的固有属性，不是本次审查的缺口。 审批推迟到 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
approved these changes
Aug 31, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
yiliang114
enabled auto-merge
August 31, 2026 07:15
Collaborator
Local verification report — verbatim workflow shell against a real Docker daemonVerified at head How this was verifiedThe interesting part of this PR is a shell lock protocol embedded in YAML, so the harness executes the verbatim Results1. Contract tests —
2. Same-commit, three shards, one daemon — three concurrent shards (0.3 s apart, same
3. Writers wait for active readers, without queue-jumping — a different-commit shard started 3 s into the readers' test phase polled the write lock and began its prune+build at 15:47:57.362, strictly after the last reader released at 15:47:56.994. The release step (verbatim, including
4. Failure faces are loud, cleanup is safe — with the writer deadline shrunk 1800 s → 6 s (only for this probe) and a reader holding the shared lock throughout, the writer exits 1 with the exact
5. The pinned-ID handoff works end to end with the real CLI — this PR's actual
6. Static checks — actionlint: e2e.yml 0 findings before and after; release.yml 9 before and 9 after (all pre-existing: YAML-anchor false positives + one SC2129, none introduced here). Prettier: clean on all six changed files. Not covered here: a full build of the repository's real sandbox Dockerfile on this host. It was attempted with the verbatim workflow command but abandoned mid- Also confirmed from the repo rather than taken on faith: Findings (none blocking)F1 — latent footgun, worth a follow-up issue: an image-ID F2 — contract-test coverage gap (minor): deleting the pre-build labeled cleanup line inside Run E2E tests survives both contract suites (the post-step cleanup is pinned by the routing test; the build-path one is not). Consequence is small since the post step still bounds image age; a one-line assertion would pin it. F3 — observation, no action needed: flock(2) documents that the exclusive→shared downgrade may not be atomic. If a polling writer ever did slip in, the builder's subsequent blocking F4 — operational assumption this environment cannot check: the protocol coordinates only if the logical runners on one physical ECS host share One tradeoff to keep in mind operationally (already disclosed in the PR body, verified fail-loud in scenario 4): if Docker test traffic keeps the shared lock held past 30 minutes, a new commit's build errors out explicitly rather than degrading — that error names the lock, so it will be diagnosable if the pool ever gets there. Tested on
Linux 6.12 (Debian 13), Docker 26.1.5 / buildx 0.13.1 (default builder, no Buildx setup — the self-hosted path), util-linux flock 2.41, Node 22. 中文版报告本地验证报告 — 真实 Docker daemon 上逐字执行 workflow shell验证 head： 验证方式本 PR 的核心是嵌在 YAML 里的 shell 锁协议，因此 harness 执行的是逐字提取的 结果1. 契约测试 — 2. 同 commit 三 shard 争用一个 daemon — 三个并发 shard（间隔 0.3 秒、相同 3. writer 等待活跃 reader，且不插队 — reader 测试进行 3 秒后启动的另一 commit shard 轮询写锁，其 prune+build 于 15:47:57.362 开始，严格晚于最后一个 reader 15:47:56.994 的释放。release 步骤（逐字执行，含 4. 失败面是显式报错，清理是安全的 — 将 writer 期限从 1800 秒缩到 6 秒（仅此探针）、reader 全程持共享锁时，writer 以 exit 1 输出 5. 固定 image ID 的交接用真实 CLI 端到端可用 — 用本 PR 真实的 6. 静态检查 — actionlint：e2e.yml 前后均 0；release.yml 前后均 9（全部为既有的 YAML anchor 误报 + 一处 SC2129，无新增）。Prettier：6 个改动文件全部通过。三个提取脚本 本报告未覆盖：在本机完整构建仓库真实的 sandbox Dockerfile。曾用 workflow 原命令尝试，但在 builder 阶段的 另外从仓库确认（而非采信 PR 描述）： 发现（均不阻塞）F1 — 潜在坑，建议开 follow-up issue：image ID 形式的 F2 — 契约测试覆盖缺口（轻微）： 删掉 Run E2E tests 内 build 前的 labeled 清理行可同时通过两套契约（收尾步骤的清理被 routing 测试盯住，build 路径的没有）。由于收尾步骤仍会限制镜像年龄，后果很小；补一行断言即可。 F3 — 观察项，无需行动： flock(2) 文档指出独占→共享的降级可能非原子。若轮询 writer 真的插入，builder 随后的阻塞 F4 — 本环境无法检验的运行前提： 协议生效要求同一物理 ECS 上的逻辑 runner 共享 一个需要记住的运维权衡（PR 描述已披露、场景 4 已验证失败面）：若 Docker 测试流量让共享锁持续超过 30 分钟，新 commit 的构建会显式报错而非静默降级——错误信息点名了锁，届时可以直接定位。 测试平台
Linux 6.12（Debian 13）、Docker 26.1.5 / buildx 0.13.1（默认 builder、无 Buildx setup——即 self-hosted 路径）、util-linux flock 2.41、Node 22。 |
wenshao
approved these changes
Aug 31, 2026
This was referenced Aug 31, 2026
This was referenced Aug 31, 2026
pull Bot
pushed a commit
to TKaxv-7S/qwen-code
that referenced
this pull request
Aug 31, 2026
…LM#10624) * fix(cli): shorten sandbox hostname for image-ID container names A regular container name derived from an image ID (sha256-<64hex> plus the -<8hex> suffix = 80 chars) exceeds Linux HOST_NAME_MAX (64), so `docker run` fails with `sethostname: invalid argument` (exit 125) whenever QWEN_SANDBOX_IMAGE is an image ID -- the documented shape after QwenLM#10605. Keep the full name for --name and derive --hostname from a short hash when the container name is too long. Follow-up to F1 in the QwenLM#10605 verification report. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): preserve valid sandbox hostnames Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-dev-bot
added a commit
that referenced
this pull request
Aug 31, 2026
…#10604) main's #10605 (6608cd3) replaced the separate CLI and interactive Docker integration steps with a single 'Run Docker Integration Tests' step that coordinates sandbox builds and docker daemon access on shared hosts. The release.yml conflicts are resolved toward main: the consolidated step is kept intact, and this branch's explanatory RUNNER_ENVIRONMENT comment moves into the consolidated step's env block, joining the two non-Docker steps' comments that merged cleanly.
Collaborator
Post-merge addendum — the live ECS runs, and a forensic re-read of the failing runA pre-merge verification report was already posted above by a parallel session, and this PR merged as
Nothing here changes the merge decision. The protocol is behaving in production exactly as designed, including the one property the pre-merge report had to leave open. The merged tree is byte-identical to the tree that was tested. All six changed files hash the same at
1. The first six live ECS runs: 18 docker shards, and it worksRunner names on the pool are
This also settles the one thing the pre-merge report explicitly could not check (its F4): the logical runners on a physical host really do share Build-phase time, as a side effect: 13 post-merge builds average 5 m 35 s (min 4 m 01 s, max 8 m 02 s), against 7 m 11 s / 7 m 31 s / 7 m 29 s for the three self-contending builds in the last clean pre-merge run, and 9 m 53 s / 9 m 58 s plus one outright build failure in the failing run. 2. The failing run, re-read from the logs
Every factual claim in the PR body holds:
Two honest notes:
3. Failure modes on the verbatim workflow shellExtracted
4. Two contract-test gaps (non-blocking, follow-up material)A 10-mutation sweep against the three contract suites killed 8. The two survivors:
5. One number worth keeping an eye onThe shared lock is held for the whole docker test phase, and a following commit's writer polls against a 30-minute deadline:
So the real headroom is roughly 7–10 minutes, and — per §3 — shards 2–3 of the following commit spend that same budget queued rather than getting a fresh one. Nothing has come close yet (0 timeouts in 18 shards) and the failure is diagnosable when it comes, but the margin is thinner than "30 minutes" suggests if the docker suite keeps growing. Environment: macOS host, Docker 29.5.2; protocol probes in 中文说明合入后补充报告 —— 首批真实 ECS 运行，以及对失败 run 的日志复核本 PR 上方已有一份并发会话发布的合入前验证报告；本 PR 于 08:44 UTC 以
这些都不改变合入结论。 协议在生产环境中的行为与设计完全一致，包括合入前报告只能存疑的那一条。 合入后的树与被测试的树逐字节一致：六个改动文件在 1. 首批六轮真实 ECS 运行：18 个 docker shard，协议生效pool 上的 runner 命名为
这同时也解决了合入前报告明确无法检验的那一条（其 F4）：同一物理宿主上的逻辑 runner 确实共享 构建耗时的附带结果：合入后 13 次构建平均 5 分 35 秒（最短 4 分 01 秒，最长 8 分 02 秒）；对照合入前最后一轮全绿中三个互相争用的构建 7 分 11 秒 / 7 分 31 秒 / 7 分 29 秒，以及失败 run 中的 9 分 53 秒 / 9 分 58 秒外加一次彻底失败。 2. 失败 run 的日志复核PR 描述中的事实性断言全部成立：
两点需要如实说明：
3. 逐字 workflow shell 上的失败面探针提取出的
4. 两个契约测试缺口（不阻塞，可作为后续项）针对三套契约测试做了 10 个变异，杀死 8 个。两个存活的：
5. 一个值得持续关注的数字共享锁在整个 docker 测试阶段被持有，而后续 commit 的 writer 按 30 分钟期限轮询：
也就是说真实余量约为 7–10 分钟，并且按 §3，后续 commit 的第 2、3 个 shard 是把同一份预算耗在排队上、而不是各自获得新的一份。目前尚未接近（18 个 shard 零超时），真到那天报错也可定位；但如果 docker 套件继续变大，这个余量比「30 分钟」听上去要薄。 环境： macOS 宿主，Docker 29.5.2；协议探针跑在 |
This was referenced Sep 2, 2026
Collaborator
|
Released in v0.23.0. |
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
3 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 coordinates Docker sandbox preparation and execution across the logical self-hosted runners that share one physical ECS Docker daemon, without reducing the three-way E2E shard concurrency.
Why it's needed
The failing job is E2E Test (Linux) - sandbox:docker - shard 3/3 in run 33351032808. Its fixed-session route test returned HTTP 500 three times over 30.3 seconds; that timing is consistent with three 10-second ACP channel initialization timeouts, while the same commit's non-Docker shard passed the case.
The three Docker shards ran as separate logical runners on the same physical ECS host and independently created Buildx builders and built the same mutable sandbox tag. A sibling shard logged a Docker API
context deadline exceededwhile Buildx inspected its builder container, and all three builds later unpacked/retagged the image at the same time. That shared-daemon build pressure is the concrete difference from the passing non-Docker path. Raising test timeouts or reducing the matrix would hide the symptom; it would not remove the competing builders, mutable tag, or global prune race.The new lock protocol separates coordination from execution: same-commit shards briefly serialize only the image check/build, then retain shared daemon access while their tests run concurrently. A different commit or release build polls for exclusive access without queueing ahead of same-commit readers, so it cannot build or prune until active Docker tests finish.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: the linked failing run launched three independent Buildx setup/build paths against one daemon and one mutable tag; the Docker shard's route test ended in repeated 500 responses after approximately 10 seconds per attempt.
After: local workflow contracts, runner-routing contracts, formatting, JavaScript syntax, extracted shell syntax, E2E workflow actionlint, and diff checks pass. Live shared-ECS behavior requires the first post-merge workflow run.
Tested on
Environment (optional)
Local workflow parsing and contract tests with the repository's existing dependencies; no local access to the shared ECS Docker daemon.
Risk & Scope
Linked Issues
Fixes #10595
中文说明
本 PR 做了什么
本 PR 对同一台物理 ECS 上多个逻辑 self-hosted runner 共享 Docker daemon 的 sandbox 镜像准备与测试执行进行协调，同时保留现有的三个 E2E shard 并发。
为什么需要
失败任务是 run 33351032808 中的 E2E Test (Linux) - sandbox:docker - shard 3/3。其中固定 session 的 route 测试重试三次后仍返回 HTTP 500，总耗时 30.3 秒；这个时长与三次 10 秒 ACP channel initialize timeout 一致，而同一 commit 的非 Docker shard 可以通过该用例。
三个 Docker shard 以不同逻辑 runner 的身份运行在同一台物理 ECS 上，各自创建 Buildx builder，并同时构建同一个可变 sandbox tag。相邻 shard 的日志在 Buildx 检查 builder container 时直接出现 Docker API
context deadline exceeded，随后三个构建又在同一时间解包并重打该镜像 tag。共享 daemon 上的构建压力是它与正常通过的非 Docker 路径之间的确定差异。单纯提高测试超时或降低 matrix 并发只会掩盖症状，无法消除多个 builder、可变 tag 和全局 prune 竞态。新的锁协议将协调与执行分开：同一 commit 的 shard 只在镜像检查/构建阶段短暂串行，随后在测试期间并发持有 daemon 共享锁。不同 commit 或 release 的构建以不进入 writer 等待队列的方式轮询独占锁，因此不会排在同 commit reader 前面，也不能在 Docker 测试运行期间构建或 prune。
评审者测试计划
如何验证
证据（修复前/后）
修复前：链接中的失败 run 在同一个 daemon 和同一个可变 tag 上启动了三套独立的 Buildx setup/build；Docker shard 的 route 测试每次约 10 秒后返回 500，重试后仍失败。
修复后：本地 workflow 契约、runner routing 契约、格式检查、JavaScript 语法、抽取出的 shell 语法、E2E workflow actionlint 和 diff 检查均通过。共享 ECS 上的真实行为需要通过合入后的第一轮 workflow 确认。
测试平台
环境（可选）
使用仓库已有依赖在本地完成 workflow 解析与契约测试；本地无法访问共享 ECS Docker daemon。
风险与范围
关联 Issue
Fixes #10595