fix(serve): add mcp_guardrails to E2E capabilities expectation#4268
Merged
Conversation
PR #4247 (`feat(serve): MCP client guardrails`) added the always-on `mcp_guardrails` capability to `SERVE_CAPABILITY_REGISTRY` and updated the unit-level `EXPECTED_STAGE1_FEATURES` in `packages/cli/src/serve/server.test.ts`, but missed the matching integration-test expectation. The E2E `qwen serve — capabilities envelope > advertises all baseline capabilities` assertion has been failing on `main` since #4247 landed. Append `'mcp_guardrails'` to the expected `caps.features` array, in the same position as `SERVE_CAPABILITY_REGISTRY` and the unit-level constant (after `session_metadata`, before the conditional `require_auth`). No production code changes.
Contributor
📋 Review SummaryThis is a straightforward test-only fix that adds 🔍 General Feedback
🎯 Specific FeedbackNo specific issues identified in this review. Verification ChecklistI verified the following:
✅ Highlights
|
wenshao
commented
May 18, 2026
Collaborator
Author
There was a problem hiding this comment.
No issues found. LGTM! ✅
Verified: mcp_guardrails is correctly positioned after session_metadata in the integration test, matching the order in SERVE_CAPABILITY_REGISTRY (capabilities.ts:87) and EXPECTED_STAGE1_FEATURES (server.test.ts:109). This is a minimal, surgical test-only fix that realigns the integration test with already-merged production code.
— mimo-v2.5-pro via Qwen Code /review
wenshao
commented
May 18, 2026
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
No issues found. LGTM! ✅
— DeepSeek/deepseek-v4-pro via Qwen Code /review
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. |
doudouOUC
approved these changes
May 18, 2026
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — 变更正确且最小化，可以合并。
验证
三处来源的 mcp_guardrails 位置一致（均在 session_metadata 之后、条件性 require_auth 之前）：
capabilities.ts能力注册表（第 87 行）server.test.tsEXPECTED_STAGE1_FEATURES（第 109 行）- 本 PR
qwen-serve-routes.test.ts（第 217 行）
集成测试使用 toEqual 做严格顺序匹配，插入位置正确。
小建议（非阻塞）
这是第二次出现"注册能力时更新了单元测试但遗漏了集成测试"的情况（#4245 也是类似修复）。可考虑在 capabilities.ts 注册表附近加注释提醒同时更新两个测试文件，或抽取共享常量消除手动同步。
2 tasks
wenshao
pushed a commit
that referenced
this pull request
May 18, 2026
PRs #4249 (workspace memory + agents CRUD) and #4269 (workspace file read routes) added `workspace_memory`, `workspace_agents`, and `workspace_file_read` to `SERVE_CAPABILITY_REGISTRY` and updated the unit-level `EXPECTED_STAGE1_FEATURES` in `packages/cli/src/serve/server.test.ts`, but missed the matching integration-test expectation. The E2E `qwen serve — capabilities envelope > advertises all baseline capabilities` assertion has been failing on `main` since those PRs landed. Append the three tags in the same positions as `SERVE_CAPABILITY_REGISTRY` and the unit-level constant (`workspace_memory` + `workspace_agents` after `workspace_providers`, `workspace_file_read` after `mcp_guardrails`). No production code changes — same shape as #4268.
2 tasks
wenshao
pushed a commit
that referenced
this pull request
May 19, 2026
Two regressions introduced by #4271 (MCP guardrail push events) had been failing every main E2E run since the PR landed. Both fixes are in integration tests; no source changes. 1. `qwen serve — capabilities envelope > advertises all baseline capabilities`. `mcp_guardrail_events` was added to `SERVE_CAPABILITY_REGISTRY` and to the unit baseline list (`packages/cli/src/serve/server.test.ts:119`) but not to the integration test's hand-maintained list. Same drift class as #4268 / #4284. Fix: append the tag in registry order. 2. `MCP child amplification (P1 baseline) > clientCount matches external pgrep observation`. The test (added by #4271, never passed CI) asserted `pgrep_observed === MCP_SERVERS_CONFIGURED`, ignoring that an ACP child runs TWO `Config` objects — bootstrap (`runAcpAgent` → `config.initialize`) + per-session (`newSessionConfig` → `config.initialize`) — each with its own `McpClientManager`. After one session, pgrep observes 2×N grandchildren while `/workspace/mcp` snapshot (`buildWorkspaceMcpStatus(this.config)`) reads only the bootstrap manager (=N). Fix: encode the 2× architectural amplification literally so a future follow-up that unifies the managers fails this assertion and forces an explicit update; keep `clientCount === MCP_SERVERS_CONFIGURED` and the original `clientCount ≤ pgrep` over-report guard intact. Verified locally: both tests pass on first attempt (no retries) via `vitest run --root ./integration-tests cli/qwen-serve-routes.test.ts cli/qwen-serve-baseline.test.ts`. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
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.
Summary
'mcp_guardrails'to the expectedcaps.featuresarray inintegration-tests/cli/qwen-serve-routes.test.ts(theqwen serve — capabilities envelope > advertises all baseline capabilitiesassertion).feat(serve): MCP client guardrails) added the always-onmcp_guardrailscapability toSERVE_CAPABILITY_REGISTRYinpackages/cli/src/serve/capabilities.tsand updated the unit-levelEXPECTED_STAGE1_FEATURESconstant inpackages/cli/src/serve/server.test.ts, but missed the matching integration-test expectation. The E2E test onmainhas been failing since feat(serve): MCP client guardrails (#4175 Wave 3 PR 14) #4247 landed (run 26014877520). This PR realigns the integration test with the registered capability list.SERVE_CAPABILITY_REGISTRYorder (aftersession_metadata, before the conditionalrequire_auth). No production code changes.Validation
Commands run:
Expected result: the
advertises all baseline capabilitiesassertion passes; no regressions in either file.Observed result:
integration-tests/cli/qwen-serve-routes.test.ts: 26 passed (26) in 3.67s — the previously failingqwen serve — capabilities envelope > advertises all baseline capabilitiestest now passes (2ms).packages/cli/src/serve/server.test.ts: 156 passed (156) in 9.24s — capability-registry assertions (returns a fresh ordered registered feature list,exposes 'modes' metadata on mcp_guardrails, etc.) all pass, confirmingEXPECTED_STAGE1_FEATURESand the integration-test array agree on ordering.Quickest reviewer verification path: re-run the failing E2E job linked above (
E2E Test - macOS/E2E Test (Linux) - sandbox:none).Evidence — failing assertion before this PR:
Scope / Risk
Testing Matrix
Testing matrix notes:
npm run build && npm run bundle+ targeted vitest runs of the affected integration and unit test files. The change is platform-agnostic (only an in-memory string array literal in a test) and the matching CI job runs on bothmacos-latestandubuntu-latest.Linked Issues / Bugs