fix(test): mount channel plugin fixture in Docker - #10318
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewRe-run at the maintainer's request — the head commit is unchanged, and I re-verified the mechanism against the current tree:
Test evidence (this PR's own CI, via API)This is an unattended CI run — I do not build or run PR code; the table below quotes the PR's own checks on the reviewed commit, fetched through the checks API. Since the first pass, the main Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 One structural note still stands: even a fully green PR CI will not exercise the scenario this PR fixes — the That gap is being closed now: a sandboxed verification run was launched alongside this re-run and will post its report in this thread — it settles whether 中文说明代码审查应维护者要求重跑——提交未变，我对当前代码树重新核验了机制：
测试证据（本 PR 的 CI，通过 API 获取）这是无人值守的 CI 运行——我不会构建或运行 PR 代码；下表引用被审提交上本 PR 自己的检查结果（经 checks API 获取）。相比首轮，主 此前的结构性说明仍然成立：即使 PR CI 全绿，也不会覆盖本修复针对的场景—— 该缺口正在被补上：本次重跑同时启动了沙箱验证运行，报告会发布在本线程——它将定论 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — verified release failure, traced root cause, one-line fix reusing an existing read-only mount mechanism; nothing left to second-guess. Re-run at the maintainer's request; the head commit is unchanged and my assessment stands. This is exactly the shape of fix the gate should wave through: the failure is real (reproduced three times in the release run's Docker integration job), the root cause — a dangling symlink target inside the sandbox — is confirmed by reading the mount code rather than asserted, and the fix is the smallest possible change, one env var in one test, using machinery that already exists, is unit-tested, and defaults to read-only. My independent proposal before reading the diff was identical. Every line in the diff is necessary; nothing unrelated rode along. Status since the first pass: the main The two honest caveats from the first pass stand, neither blocking: final confirmation of the docker scenario arrives with the in-flight verification run or the next release run / post-merge E2E, and the sandboxed session gains read-only sight of the repository — fine for a throwaway test sandbox, worth knowing. CI is not fully settled on this commit yet, so approval is deferred until it lands green on 中文说明置信度：5/5 —— 发版失败已核实，根因已追踪，单行修复复用了既有的只读挂载机制；没有可再质疑之处。 应维护者要求重跑；提交未变，结论不变。这正是审查门应当快速放行的修复形态：失败真实存在（在发版运行的 Docker 集成 job 中复现三次），根因（沙箱内悬空的软链接目标）是通过阅读挂载代码确认而非仅凭断言，修复是最小改动——单个测试中的一个环境变量，使用已有、经过单元测试且默认只读的机制。我在看 diff 之前的独立方案也相同。diff 中每一行都是必要的，没有夹带无关改动。 首轮之后的进展：主 首轮的两点非阻塞说明仍然成立：docker 场景的最终确认将由进行中的验证运行或下次发版运行/合并后 E2E 给出；沙箱会话将获得仓库的只读可见性——对一次性测试沙箱而言可以接受，但值得知晓。 该提交的 CI 尚未完全结束，因此批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
wenshao
approved these changes
Aug 27, 2026
Collaborator
|
@qwen-code /triage |
yiliang114
enabled auto-merge
August 27, 2026 16:58
Collaborator
|
Sandboxed verification: Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 52 passed · 0 failed · 52 total Flakiness gate: not applicable — no runnable changed test files (1 out-of-scope file(s) noted in the log) 中文 — 判定：
|
| Cell | SANDBOX_MOUNTS |
docker run repo volume |
stderr trace | Result |
|---|---|---|---|---|
| A1 (head semantics) | <repo> |
--volume <repo>:<repo>:ro present |
SANDBOX_MOUNTS: <repo> -> <repo> (ro) |
pass |
| A0 (base semantics) | unset | absent | absent | pass |
| A2 | relative/path |
n/a | FatalSandboxError: ... must be absolute, non-zero exit |
pass |
| A3 | /nonexistent-... |
n/a | FatalSandboxError: Missing mount path ..., non-zero exit |
pass |
The :ro + identity-target shape is exactly what the symlink needs: the container sees the repo at the same absolute path the host-side symlink points to, read-only (no write exposure of the repo to the sandboxed test process).
Integration matrix, head vs base (02-integration-matrix-head-vs-base.png, per-cell logs logs/cell-*.log)
Base control = git show HEAD^1:integration-tests/cli/... extracted next to the head file; the two differ by exactly the added line (asserted in-harness). 8/8 assertions.
| Cell | Arm | QWEN_SANDBOX |
Expected | Observed | Assertion |
|---|---|---|---|---|---|
| B | head | false |
4/4 pass (reviewer plan step 2) | 4/4 pass, first test 4.9 s (daemon+WS+full session flow) | pass |
| C | base | false |
4/4 pass (added line inert) | 4/4 pass | pass |
| D1 | head | docker |
env-blocked red | red: expected 'Could not create task "review".' to contain 'Created and selected task "review"' |
pass (expected red) |
| D0 | base | docker |
same red | byte-identical red (asserted) | pass (expected red) |
| E | n/a | docker |
root-cause probe | ACP child boot prints Missing sandbox command 'docker' (from QWEN_SANDBOX) |
pass |
D1 reproduces the wire shape of the original CI failure (same user-visible symptom, same assertion), not its cause: here the trigger is the absent docker runtime, there it was the absent repo mount. The D0≡D1 symmetry is the environmental-block evidence — the PR's line changes nothing in a container without docker, so the block is not attributable to the change.
Env-propagation chain (static proof the line reaches the sandbox builder)
- Test sets
SANDBOX_MOUNTSin the daemon env (qwen-serve-channel-workers.test.ts:274). - Daemon base env = launch env minus loader vars (
run-qwen-serve.ts:3140), extended intoruntimeEffectiveEnv(run-qwen-serve.ts:4632). - Channel worker fork clones the daemon env, deleting only token keys (
channel-worker-supervisor.ts:650-668) — and workers runqwen channel daemon-worker, a command path with no sandbox hop, which is why the other three tests (which create no nested sessions) neither set nor need the variable. - The
qwen --acpchild is spawned fromruntimeEffectiveEnvwith scrub list{QWEN_SERVER_TOKEN, QWEN_CODE_SIMPLE, EXTERNAL_TOOL_GUARD_TOKEN}(acp-bridge/src/spawnChannel.ts:662-664, 694) —SANDBOX_MOUNTSsurvives. - The child's main entry loads sandbox config when
QWEN_SANDBOXis set (gemini.tsx:538,670) andstart_sandboxreadsprocess.env['SANDBOX_MOUNTS'](sandbox.ts:559).
Cell E proved step 5 fires for the ACP child; steps 1-4 are the code paths quoted above.
Targeted gate (reviewer plan step 3)
packages/cli: npx vitest run src/serve/sandbox.test.ts → 27/27 pass (logs/unit-gate-sandbox.log), including the six parseSandboxMountSpec tests pinning the to=from, opts=ro defaults. Liveness proven: a scratch copy with the two ro expectations inverted fails exactly those two tests (2 failed | 25 passed, exit 1), so the green run is meaningful; the scratch file was removed afterwards.
Findings
No blocking findings. Non-blocking observations:
- Coverage gap (pre-existing, not introduced by this PR): nothing unit-pins the
SANDBOX_MOUNTS→--volumeassembly instart_sandbox(thesandbox.test.tssuite covers the parser and other arg paths, but not this block). The shim A/B in this round pins it behaviorally; a maintainer may want it as a permanent unit test. Severity: Suggestion-level note; not a merge condition. - The fix has never been executed end-to-end by anyone yet: the author states the Docker E2E was left to CI (PR body), and this round could not run it either. The residual risk is confined to standard docker bind-mount semantics on the CI runner; the chain above shows every link on this side of the docker boundary holds. The CI Docker lane is the deciding oracle.
Not covered
- The Docker E2E flip itself (base red → head green inside a real container). Environmental, proven by A/A: no
docker/podman/nerdctlbinary in the verify container;/var/run/docker.sockexists but isroot:992mode 660 and this job runs as uid 1000 (connectEACCES), no sudo. Cells D1/D0 and E show both arms blocked identically, so the block is not PR-caused. The Docker integration lane must adjudicate the central claim. - The other three tests under
QWEN_SANDBOX=docker(they create no nested sessions; architecturally they need no mount, and cells B/C show them green without docker involvement, but a docker-lane run of the whole file would confirm). - Release run 33085036437's original failure logs (no GitHub token in this environment); the PR's account of it is treated as hypothesis, and cell D1 independently reproduced the same symptom shape from a different cause.
- Windows/macOS behavior; the full
packages/cliunit suite and repo-wide gates (PR's own CI covers them). - Per-commit attribution: single commit
a469a392a5, matching the snapshot'scommitsarray; nothing to bisect.
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD = merge, HEAD^1 = base tip, HEAD^2 = verified head), npm ci + npm run build pre-run. Harnesses live in harness/ (sandbox-mounts-ab.mjs drives the real dist/cli.js through the real sandbox hop with a recording docker shim; integration-matrix.mjs runs the head test file and its HEAD^1 extraction under both sandbox modes plus the ACP-child boot probe). Raw per-cell logs in logs/; evidence captures evidence/01-docker-shim-ab-mount-flip.png and evidence/02-integration-matrix-head-vs-base.png produced by scripts/verify-capture.mjs. Expected docker-mode reds are encoded as passing assertions (the symmetry is the claim), so fail=0 in assertions.json counts only unexpected outcomes.
Flakiness gate log
integration test, out of gate scope: integration-tests/cli/qwen-serve-channel-workers.test.ts
verdict: n/a
summary: no runnable changed test files (1 out-of-scope file(s) noted in the log)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. |
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
approved these changes
Aug 27, 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. ✅
Collaborator
|
Released in v0.22.3. |
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
Keeps the repository-backed mock channel extension available when the integration fixture creates an ACP session inside the Docker sandbox. The fixture now adds the repository root to the sandbox mount list; the existing mount parser makes this read-only by default.
Why it's needed
The fixture installs the mock extension into its temporary Qwen home as an absolute symlink to the repository. The host-side channel worker can load that symlink, but the nested Docker sandbox previously mounted only the temporary workspace, Qwen home, runtime directory, and
/tmp. Inside the container the repository target was absent, extension discovery failed withENOENT, and/session new reviewreturned onlyCould not create task "review".This caused the Docker integration job in release run 33085036437 to fail on all retries.Reviewer Test Plan
How to verify
QWEN_SANDBOX=docker npx vitest run --root ./integration-tests cli/qwen-serve-channel-workers.test.ts -t 'controls a real mock-plugin worker after a channel-less boot' --retry=0. Confirm both the channel worker and the nested ACP session load the extension, and the first/session new reviewcreates and selects the task.QWEN_SANDBOX=false. Confirm all four channel-worker scenarios still pass.Evidence (Before & After)
N/A — integration-test fixture only; no user-visible UI change.
Tested on
Environment (optional)
Node 22.22.0 and npm 10.9.4. The local Docker client was installed, but its daemon was not running, so the Docker E2E itself remains for CI/reviewer verification.
Risk & Scope
Linked Issues
Related to #10312.
中文说明
本 PR 做了什么
确保集成测试 fixture 在 Docker 沙箱中创建 ACP 会话时，仍能访问仓库内的 mock channel 扩展。fixture 现在将仓库根目录加入沙箱挂载列表；现有挂载解析逻辑默认以只读方式挂载。
为什么需要
该 fixture 使用绝对软链接，将临时 Qwen home 中的 mock 扩展指向仓库目录。宿主侧 channel worker 能加载这个软链接，但嵌套 Docker 沙箱此前只挂载临时 workspace、Qwen home、runtime 目录和
/tmp。容器内缺少仓库目标路径，扩展发现因此以ENOENT失败，/session new review最终只返回Could not create task "review".。这导致发版运行 33085036437 的 Docker 集成 job 在所有重试中均失败。Reviewer Test Plan
如何验证
QWEN_SANDBOX=docker npx vitest run --root ./integration-tests cli/qwen-serve-channel-workers.test.ts -t 'controls a real mock-plugin worker after a channel-less boot' --retry=0。确认 channel worker 和嵌套 ACP 会话都能加载扩展，并且第一次/session new review能创建并选中任务。QWEN_SANDBOX=false运行同一测试文件，确认四个 channel worker 场景仍全部通过。前后对比证据
N/A —— 仅修改集成测试 fixture，没有用户可见 UI 变化。
测试平台
环境（可选）
Node 22.22.0、npm 10.9.4。本地已安装 Docker 客户端，但 daemon 未运行，因此 Docker E2E 留给 CI/Reviewer 验证。
风险与范围
关联 Issue
关联 #10312。