fix release integration env controls#5121
Conversation
|
Thanks for the PR! Template looks good ✓ — all required sections present with bilingual description. On direction: this is a straightforward CI bugfix. Release workflow broke after #4914 made debug logging opt-in; the integration tests and Docker sandbox weren't updated to match. Fixing broken release infra is clearly in scope — you can't ship if your release pipeline is red. On approach: the scope is tight and appropriate. Three surgical changes: set One minor observation: the Moving on to code review and testing. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有章节齐全,中英双语。 方向:这是一个明确的 CI bugfix。#4914 把 debug logging 改成 opt-in 后 release workflow 挂了;integration tests 和 Docker sandbox 没跟着改。修复 release 基础设施显然在范围内——release pipeline 红了就没法发版。 方案:范围紧凑合理。三处精准改动:在断言 debug 文件存在的测试中设置 一个小观察: 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
There was a problem hiding this comment.
Pull request overview
Restores explicit environment controls needed by release integration tests after debug file logging became opt-in, and ensures those controls are preserved when running the CLI inside the Docker sandbox.
Changes:
- Forward
QWEN_DEBUG_LOG_FILEand MCP-related control env vars into the sandbox container (including path mapping forQWEN_CODE_MCP_APPROVALS_PATH). - Make the
QWEN_RUNTIME_DIRdebug-log assertion test explicitly opt into file logging viaQWEN_DEBUG_LOG_FILE=1. - Ensure
simple-mcp-serverintegration test restoresQWEN_CODE_LEGACY_MCP_BLOCKINGafter execution.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/cli/src/utils/sandbox.ts | Passes through release/integration env controls into the Docker sandbox so the sandboxed CLI observes the same test configuration as the host. |
| integration-tests/cli/simple-mcp-server.test.ts | Saves/restores QWEN_CODE_LEGACY_MCP_BLOCKING to avoid cross-test environment leakage. |
| integration-tests/cli/qwen-config-dir.test.ts | Explicitly enables debug file logging for the runtime-dir assertions and cleans up the env var afterward. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewThe diff is clean and minimal — three files, 26 additions, zero deletions. Nothing to flag:
No correctness bugs, no security issues, no over-engineering. Real-scenario testingRan both affected integration test files before and after applying the PR fix. Same command in both runs: Before (main branch, without fix)After (this PR)Test 4a goes from failing (3 retries, all expecting debug files that never appear) to passing cleanly. The MCP test continues to pass, and notably the full combined run shows the env-var save/restore doesn't cause cross-test interference. 中文说明代码审查diff 干净且最小化——三个文件,26 行新增,0 行删除。无问题需要指出:
无正确性 bug,无安全问题,无过度工程。 实际场景测试在应用 PR 修复前后分别运行了两个受影响的 integration 测试文件,使用相同命令。 修复前:test 4a 失败(3 次重试均期望 debug 文件但不存在)。 — Qwen Code · qwen3.7-max |
|
This is a clean bugfix that does exactly what it says. The release workflow broke because #4914 made debug logging opt-in but two consumers weren't updated: the integration test that asserts debug file existence, and the Docker sandbox that needs test-control env vars forwarded. The PR fixes both with the minimum necessary changes. What makes this a good fix: it doesn't just patch the symptom (set the env var and move on) — it also closes a latent state-leak bug in the MCP test where The before/after is unambiguous: test 4a goes from hard-failing (3 retries, 0 debug files) to passing cleanly. All 8 tests pass in the combined run. No reservations. Approving. 中文说明这是一个干净的 bugfix,完全如描述所述。Release workflow 因为 #4914 把 debug logging 改成 opt-in 而挂了,但两个消费方没跟着改:断言 debug 文件存在的 integration test,和需要转发测试控制环境变量的 Docker sandbox。PR 用最小的必要改动同时修复了两者。 这个修复好在哪里:不只是打补丁(设个环境变量就完事)——还顺手修了一个潜在的 MCP test 状态泄漏 bug, 修复前后对比明确:test 4a 从硬失败(3 次重试,0 个 debug 文件)变为正常通过。组合运行 8 个测试全部通过。 没有顾虑。批准。 — Qwen Code · qwen3.7-max |
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. |
What this PR does
Restores the release integration test controls that became implicit after debug file logging was made opt-in.
This makes the QWEN_RUNTIME_DIR debug-log assertion explicitly enable file logging, and passes the release integration environment controls into the Docker sandbox so the sandboxed CLI sees the same test setup as the host process.
Why it's needed
The scheduled release workflow started failing after #4914 changed debug log file creation to require
QWEN_DEBUG_LOG_FILE=1. The existing QWEN_HOME/QWEN_RUNTIME_DIR integration test still expected debug files without opting in, and Docker integration lost the MCP test controls at the sandbox boundary.Reviewer Test Plan
How to verify
Run the affected no-sandbox integration files and confirm
qwen-config-dirandsimple-mcp-serverpass. In Docker CI, confirm the same tests no longer fail becauseQWEN_DEBUG_LOG_FILE,QWEN_CODE_LEGACY_MCP_BLOCKING, andQWEN_CODE_MCP_APPROVALS_PATHare available inside the sandbox.Evidence (Before & After)
Before: release run 27516870586 failed in
qwen-config-dir.test.tsbecause no debug log files were created, and Dockersimple-mcp-server.test.tscould not observe the expected MCP tool call. After:QWEN_SANDBOX=false npx vitest run --root ./integration-tests cli/qwen-config-dir.test.ts cli/simple-mcp-server.test.ts --poolOptions.threads.maxThreads 1passes with 2 files and 8 tests.Tested on
Environment (optional)
Local macOS, Node.js v22.22.0.
npm installcompleted and ranbuild/bundle;npx prettier --check integration-tests/cli/qwen-config-dir.test.ts integration-tests/cli/simple-mcp-server.test.ts packages/cli/src/utils/sandbox.ts;npx eslint integration-tests/cli/qwen-config-dir.test.ts integration-tests/cli/simple-mcp-server.test.ts packages/cli/src/utils/sandbox.ts;npm run typecheck.Risk & Scope
Linked Issues
Fixes #5117
中文说明
这个 PR 做了什么
恢复 release integration tests 依赖的测试控制变量,适配 #4914 之后 debug file logging 需要显式 opt-in 的行为。
这个改动让 QWEN_RUNTIME_DIR 的 debug log 断言显式设置
QWEN_DEBUG_LOG_FILE=1,并把 release integration 需要的环境变量传进 Docker sandbox,确保 sandbox 内 CLI 看到的测试配置和宿主进程一致。为什么需要
#4914 把 debug log file 创建改成需要
QWEN_DEBUG_LOG_FILE=1后,定时 release workflow 开始失败。原来的 QWEN_HOME/QWEN_RUNTIME_DIR integration test 没有显式 opt-in 却仍然期待 debug 文件;Docker integration 还会在 sandbox 边界丢失 MCP 测试控制变量。Reviewer Test Plan
如何验证
运行受影响的 no-sandbox integration 文件,确认
qwen-config-dir和simple-mcp-server通过。在 Docker CI 中,确认同一组测试不再因为QWEN_DEBUG_LOG_FILE、QWEN_CODE_LEGACY_MCP_BLOCKING和QWEN_CODE_MCP_APPROVALS_PATH没有进入 sandbox 而失败。证据(修改前/修改后)
修改前:release run 27516870586 中
qwen-config-dir.test.ts因未生成 debug log files 失败,Dockersimple-mcp-server.test.ts无法观察到预期 MCP tool call。修改后:QWEN_SANDBOX=false npx vitest run --root ./integration-tests cli/qwen-config-dir.test.ts cli/simple-mcp-server.test.ts --poolOptions.threads.maxThreads 1通过,2 个文件、8 个测试。测试平台
环境(可选)
本地 macOS,Node.js v22.22.0。
npm install完成并触发build/bundle;已运行npx prettier --check integration-tests/cli/qwen-config-dir.test.ts integration-tests/cli/simple-mcp-server.test.ts packages/cli/src/utils/sandbox.ts、npx eslint integration-tests/cli/qwen-config-dir.test.ts integration-tests/cli/simple-mcp-server.test.ts packages/cli/src/utils/sandbox.ts、npm run typecheck。风险和范围
关联 Issues
Fixes #5117