fix(cli): Keep CDP MCP env reads at serve boundaries - #6564
Closed
yiliang114 wants to merge 1 commit into
Closed
Conversation
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: Observed CI failure — PR #6559 broke Direction: Aligned. The process-env-guard exists for a reason — keeping environment reads at the serve boundary makes workspace-scoped behavior isolatable and testable. Making Size: Not applicable — changes are in Approach: Scope is tight. 27+14 lines across 5 files, all directly serving the stated goal. The signature change from optional-to-required Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测的 CI 失败——PR #6559 导致 方向：对齐。process-env-guard 的存在是为了将环境变量读取限制在 serve 边界，使 workspace 级行为可隔离、可测试。将 规模：不适用——改动在 方案：范围紧凑。5 个文件 27+14 行，全部围绕修复目标。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
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
Code ReviewIndependent proposal: Given the problem (process-env-guard flags Comparison: The PR matches this approach exactly. The only addition is extracting No reuse issues. No new abstractions. No code duplication. Real-Scenario TestingBefore (installed build — main branch)Guard correctly flags After (this PR — pr-6564 branch)process-env-guard test: Guard passes — browser automation MCP behavior tests: All 3 browser automation MCP tests pass — token-protected endpoints correctly suppress browser automation MCP, the capability is advertised when CDP tunnel is enabled without token, and dynamic registration still works. 中文说明代码审查独立方案： 针对 process-env-guard 标记 对比： PR 方案完全一致。唯一的额外是将 真实场景测试Before（main 分支）： process-env-guard 测试失败—— After（PR 分支）：
修复有效，行为未变。 — Qwen Code · qwen3.7-max |
Collaborator
|
This is a clean, minimal CI fix. The process-env-guard exists to keep environment reads at well-defined serve boundaries — The before/after test evidence is clear — guard fails on main with Every edit in the diff is necessary for the stated goal. No scope creep, no drive-by refactors. The Approving. ✅ 中文说明这是一个干净、最小的 CI 修复。process-env-guard 的目的是将环境变量读取限制在明确定义的 serve 边界—— before/after 测试证据清晰——main 分支上 guard 标记了 diff 中的每处改动都服务于修复目标。无范围蔓延，无顺手重构。 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 9, 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. ✅
Collaborator
Author
|
Closing this as a duplicate of #6562, which fixes the same process.env guard failure and has been reviewed/approved. |
wenshao
reviewed
Jul 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
Downgraded from Approve to Comment: CI failing (review-pr). The code changes are clean and correct.
— qwen3.7-max via Qwen Code /review
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 optional browser automation MCP environment lookup behind the existing serve process boundaries. The CDP MCP helper now requires callers to pass the environment explicitly, while the bootstrap capabilities path, runtime capabilities path, and ACP WebSocket registration path pass
process.envfrom files that are already treated as environment boundaries.Why it's needed
PR #6559 failed CI because
packages/cli/src/serve/process-env-guard.test.tscaughtpackages/cli/src/serve/cdp-mcp-command.tsas a new directprocess.envreader. That helper was introduced by #6472 for the optional external CDP MCP adapter, but it lives under the workspace-scoped serve tree scanned by the guard. The guard is intentional: serve and acp-bridge code should not grow ad hoc global environment reads outside the approved boundary files, because that makes workspace-scoped runtime behavior harder to isolate and test. This fixes the CI failure by preserving the guard and moving the environment dependency back to the existing serve boundary callers, rather than expanding the allowlist.Reviewer Test Plan
How to verify
Run
npm -w packages/cli exec -- vitest run src/serve/process-env-guard.test.ts; before this change the guard reportspackages/cli/src/serve/cdp-mcp-command.tsas an offender, and after this change it passes. For behavior, verify that browser automation MCP is still advertised only when the CDP tunnel and external adapter command are enabled, remains hidden for token-protected endpoints or disabled ACP HTTP, and still dynamically registers the Chrome DevTools runtime MCP server fromQWEN_CDP_MCP_COMMAND.Evidence (Before & After)
N/A, non-UI CI guard fix.
Tested on
Environment (optional)
Local Node 22 workspace. Loopback-dependent tests were run outside the filesystem/network sandbox because the sandbox rejects localhost listeners with
EPERM.Risk & Scope
npm run test:ci; this PR used the focused failing guard plus related behavior tests, CLI typecheck, CLI lint, and whitespace checks.Linked Issues
References #6559 and #6472.