test(cli): isolate sandbox-relaunch tests from ambient QWEN_SANDBOX_IMAGE - #7093
Conversation
…MAGE The suite already saves/clears QWEN_SANDBOX/SANDBOX/QWEN_CODE_SIMPLE but not QWEN_SANDBOX_IMAGE. main() treats that env var as a custom sandbox image and takes the custom-image relaunch branch, skipping the host-update capability computation — so 'passes host update capability into a container sandbox' fails (getInstallationInfo never called) in any environment that exports a resolved image, e.g. the autofix runner (its resolve-sandbox-image step appends QWEN_SANDBOX_IMAGE to GITHUB_ENV). This blocked two otherwise-complete verified pushes by the autofix loop. Save/clear/restore it like its siblings.
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual body included. Problem: Observed, with evidence. The test Direction: Aligned. Making tests hermetic against ambient environment variables is clearly within scope — the suite already does this for Size: Not applicable — test file only ( Approach: Scope is minimal and correct. The change follows the existing save/clear/restore pattern in the same Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必要章节齐全,包含双语正文。 问题:已观测到,有证据。 当环境中设置了 方向:对齐。让测试对环境变量保持 hermetic 显然在范围内——该套件已经对 规模:不适用——仅测试文件( 方案:范围最小且正确。改动完全遵循同一 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: To make the Comparison with diff: The PR's approach is identical to my independent proposal. The variable declaration, Reuse check: No new logic introduced — purely following the existing pattern. Nothing to reuse-check. Issues found: None. The change is correct, minimal, and idiomatic. Real-Scenario TestingThree-way matrix verified locally: All three scenarios behave exactly as expected. The fix is confirmed. 中文说明代码审查独立方案: 让 与 diff 的对比: PR 的方案与我的独立方案完全一致。变量声明、 复用检查: 未引入新逻辑——纯粹遵循既有模式。无需复用检查。 发现的问题: 无。改动正确、最小、符合惯例。 真实场景测试本地验证了三方对照矩阵,三个场景表现均与预期完全一致。修复已确认。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; textbook test hermeticity fix. This is exactly the kind of PR you want to see: a real, observed CI problem with linked evidence, a minimal fix that follows the existing pattern in the same test suite, and a clear before/after reproduction. The three-way matrix (unfixed+env, fixed+env, fixed+clean) all behave as expected. Stage 0 (core module): Not applicable — test file only, no production code touched. Stage 1 (gate): Template complete, problem observed with evidence (two linked CI runs, reproduced locally), direction aligned, scope minimal. No concerns. Stage 2 (review + test): Code review found zero issues — the change is a mechanical extension of the existing save/clear/restore pattern. Real-scenario testing confirmed all three matrix scenarios pass/fail as expected. No blockers. No suggestions. Approved. ✅ 中文说明置信度:5/5 — 各阶段均无问题;教科书级的测试 hermeticity 修复。 这正是你希望看到的 PR:一个真实的、已观测到的 CI 问题(有链接证据),一个遵循同测试套件既有模式的最小修复,以及清晰的 before/after 复现。三方矩阵(未修复+环境变量、已修复+环境变量、已修复+干净环境)全部表现如预期。 阶段 0(核心模块): 不适用——仅测试文件,未触及生产代码。 阶段 1(门控): 模板完整,问题有证据(关联两个 CI 运行,本地复现),方向对齐,范围最小。无顾虑。 阶段 2(审查+测试): 代码审查未发现任何问题——改动是既有保存/清除/恢复模式的机械性扩展。真实场景测试确认三种矩阵场景均如预期通过/失败。 无阻塞项。无建议。已通过。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: Agent 1a: Line-by-line correctness — never opened its brief (/home/runner/work/qwen-code/qwen-code/.qwen/tmp/qwen-review-pr-7093-fetch-prompts/1a.brief.md), so it reviewed without the instructions it was launched to follow.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
What this PR does
Makes the CLI's sandbox-relaunch tests hermetic against an ambient
QWEN_SANDBOX_IMAGEenvironment variable. The suite already saves and clearsQWEN_SANDBOX,SANDBOX, andQWEN_CODE_SIMPLEto guarantee a consistent environment; this addsQWEN_SANDBOX_IMAGEto that same save/clear/restore pattern.Why it's needed
When
QWEN_SANDBOX_IMAGEis present, the CLI's startup treats it as a custom sandbox image and takes the custom-image relaunch branch, which intentionally skips the host-update capability computation. The testpasses host update capability into a container sandboxasserts that computation happens, so in any environment that exports a resolved image the test fails with "expected spy to be called … Number of calls: 0" — the autofix runner is exactly such an environment (its resolve-sandbox-image step appendsQWEN_SANDBOX_IMAGEtoGITHUB_ENV, so every later step inherits it). This single non-hermetic test blocked two otherwise-complete verified pushes by the autofix loop (its deterministic verify gate runs the changed-files test suite), and misled the agents into "fails identically on main" conclusions, since they verified base-branch copies inside the same contaminated environment. Regular CI never exports the variable, which is why the test is green there and the failure looked runner-specific.Reviewer Test Plan
How to verify
Controlled three-way matrix, run locally:
QWEN_SANDBOX_IMAGEset (reproduces the runner):QWEN_SANDBOX_IMAGE=ghcr.io/qwenlm/qwen-code:9.9.9 npx vitest run --root packages/cli src/gemini.test.tsx -t "passes host update capability"→ the test fails withexpected "spy" to be called with arguments: [ Any<String>, true ] … Number of calls: 0.QWEN_SANDBOX_IMAGEset → passes (the suite now clears the variable per-test and restores it after).Real-world before evidence: autofix verify-gate runs 29555723913 (PR #7062) and 29559202978 (PR #6984) each fail with exactly this one test out of the full suite.
Evidence (Before & After)
(Note:
gemini import boundary > does not statically import ACP…fails on this machine on pristine main as well — pre-existing local-environment issue, unrelated to this diff.)Tested on
Environment (optional)
vitest via a fresh
npm ciworktree checkout of main; verified fail→pass with the variable exported and pass with a clean env.Risk & Scope
QWEN_SANDBOX_IMAGEjob-wide is left as is (it is needed by the sandboxed agent); other suites that read this variable already clear it themselves.Linked Issues
Found while validating #6998 / #7076 live: the autofix loop's verified pushes on #6984 and #7062 were blocked solely by this test.
中文说明
本 PR 做了什么
让 CLI 的 sandbox-relaunch 测试对环境中的
QWEN_SANDBOX_IMAGE变量保持 hermetic。该测试套件已经对QWEN_SANDBOX、SANDBOX、QWEN_CODE_SIMPLE做了保存/清除/恢复以保证一致的测试环境;本 PR 把QWEN_SANDBOX_IMAGE加入同一模式。为什么需要
当
QWEN_SANDBOX_IMAGE存在时,CLI 启动逻辑将其视为自定义沙箱镜像并走 custom-image relaunch 分支,该分支有意跳过 host-update 能力计算。而测试passes host update capability into a container sandbox恰恰断言该计算发生,因此任何导出了已解析镜像的环境中该测试都会以 "expected spy to be called … Number of calls: 0" 失败 —— autofix runner 正是这种环境(其 resolve-sandbox-image 步骤把QWEN_SANDBOX_IMAGE追加进GITHUB_ENV,后续所有步骤继承)。这一个非 hermetic 测试挡住了 autofix 回路两次本已完成的 verified push(其确定性 verify gate 会跑 changed-files 测试),还误导 agent 得出"在 main 上也同样失败"的结论 —— 因为它们在同一被污染环境里验证 base 分支副本。常规 CI 从不导出该变量,所以在那里是绿的,失败看起来像 runner 特有。评审验证方案
如何验证
本地三方对照:
QWEN_SANDBOX_IMAGE(复现 runner):QWEN_SANDBOX_IMAGE=ghcr.io/qwenlm/qwen-code:9.9.9 npx vitest run --root packages/cli src/gemini.test.tsx -t "passes host update capability"→ 失败,Number of calls: 0。真实世界修复前证据:autofix verify-gate 运行 29555723913(PR #7062)与 29559202978(PR #6984)在全量套件中各只挂这一个测试。
证据(前后对比)
(注:
gemini import boundary > does not statically import ACP…在本机的未改动 main 上同样失败 —— 本地环境既有问题,与本 diff 无关。)测试情况
环境(可选)
在 main 的全新
npm ciworktree 检出上用 vitest 验证:导出变量时 fail→pass,干净环境 pass。风险与范围
QWEN_SANDBOX_IMAGE保持现状(沙箱化 agent 需要);其它读取该变量的套件已自行清理。关联 Issue
在 #6998 / #7076 的实战验证中发现:autofix 回路对 #6984 与 #7062 的 verified push 仅被此测试阻塞。