feat(serve): add workspace MCP management

[ytahdn]

What this PR does

This PR adds workspace-scoped MCP management to Web Shell and the daemon. It introduces a plugin management entry with extension and MCP tabs, persisted user/workspace MCP discovery that can run without a chat session, typed SDK status and control operations, and management flows for approval, reconnect, OAuth authentication, enablement, add, edit, and delete. Changes are reconciled into live sessions while project .mcp.json servers and non-persisted runtime servers remain outside the management list.

The MCP UI now provides aligned cards and empty states, source filtering, server/tool/resource details, polling for OAuth completion, safe destructive confirmations, atomic settings mutations, redacted-secret preservation, and compatible source metadata through the additive configOrigin field.

Why it's needed

MCP administration was previously tied too closely to session lifecycle, so the management page could lose discovery state, fail operations when no session existed, or require repeated initialization. Users also lacked a consistent place to inspect and manage persisted MCP servers. A workspace-level control path allows administration after daemon restart, works with or without connected sessions, and propagates configuration changes to every active runtime.

Reviewer Test Plan

How to verify

1. Start the daemon without opening a chat session, open Plugins > MCP, and confirm persisted user and workspace servers load and can be initialized, refreshed, approved, authenticated, reconnected, enabled, edited, added, and deleted.
2. Open a live chat session, perform the same MCP operations from the management page, and confirm the session receives the updated server and tool state without restarting or navigating away from the session URL.
3. Confirm project .mcp.json servers and non-persisted runtime MCP servers do not appear in the management list, while extension-provided MCP servers remain visible but cannot be individually disabled or deleted.
4. Start OAuth authentication and complete it in the browser. Confirm the detail page polls until success, stops if the server is removed, and reports daemon errors instead of polling for ten minutes.
5. Add or edit a server with a description and JSON configuration. Confirm the form description overrides a JSON description, masked secrets are preserved, and saving uses configuration reload rather than a new initialization.
6. Navigate from a chat session to Plugins and back. Confirm the original session remains reachable.

Evidence (Before & After)

Before: MCP management depended on a live session and could reinitialize repeatedly, lose status across navigation, or expose operations that the backend could not perform.

After: MCP management has a workspace-level lifecycle, works with zero or multiple live sessions, propagates changes to those sessions, and provides consistent management UI and status behavior.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

Targeted validation completed: CLI MCP tests (249 passed), workspace settings tests (14 passed), SDK daemon event tests (106 passed), package-level typechecks for CLI, ACP bridge, SDK, and Web Shell, targeted builds for ACP bridge, SDK, and WebUI, and git diff validation. Per request, no Web Shell UI test or repository-wide build was run.

Risk & Scope

• Main risk or tradeoff: workspace MCP state now coordinates a daemon-owned discovery runtime, cached status, and live session reconciliation, so lifecycle and concurrent management paths are the highest-risk areas.
• Not validated / out of scope: Windows and Linux manual verification, Web Shell UI automation, repository-wide build, and management of project .mcp.json or transient runtime MCP servers.
• Breaking changes / migration notes: no intentional breaking change. The legacy source field keeps its existing runtime meaning; configOrigin is additive and should be used by new consumers for precise provenance.

Linked Issues

N/A

中文说明

本 PR 的改动

本 PR 为 Web Shell 和 daemon 增加工作区级 MCP 管理能力。新增带有“扩展”和“MCP”Tab 的插件管理入口；持久化的用户级、工作区级 MCP 可在没有聊天 session 时完成发现；SDK 增加类型化的状态与控制操作；管理页支持审批、重新连接、OAuth 认证、启用、新增、编辑和删除。配置变更会同步到所有存活 session，同时项目 .mcp.json 和未落盘的运行时 MCP 不进入管理列表。

MCP 页面同时补齐统一的卡片与空状态、来源筛选、服务器/工具/资源详情、OAuth 完成轮询、安全的删除二次确认、原子设置修改、敏感字段遮罩保留，以及通过新增 configOrigin 字段提供兼容且精确的来源信息。

为什么需要

此前 MCP 管理与 session 生命周期耦合过深，管理页面可能丢失发现状态、在没有 session 时无法操作，或者重复初始化。用户也缺少统一查看和管理持久化 MCP 服务器的入口。工作区级控制链路可以在 daemon 重启后直接管理，在没有或存在多个 session 时都能工作，并把配置变更传播到所有活跃运行实例。

Reviewer 测试计划

验证方式

1. 启动 daemon 但不打开聊天 session，进入“插件 > MCP”，确认用户和工作区持久化服务器可加载，并可执行初始化、刷新、审批、认证、重新连接、启用、编辑、新增和删除。
2. 打开一个聊天 session，从管理页执行相同操作，确认 session 无需重启即可获得更新后的服务器和工具状态，并且导航不会改变 session URL。
3. 确认项目 .mcp.json 和未落盘的运行时 MCP 不展示；扩展提供的 MCP 可以展示，但不能单独禁用或删除。
4. 发起 OAuth 认证并在浏览器完成，确认详情页轮询至成功；服务器被移除时立即停止；daemon 返回错误时展示真实错误，而不是继续轮询十分钟。
5. 新增或编辑带描述和 JSON 配置的服务器，确认表单描述覆盖 JSON 描述、遮罩后的敏感字段得到保留，并且保存走配置重载而不是再次初始化。
6. 从聊天 session 进入插件页再返回，确认原 session 仍可访问。

前后对比证据

修改前：MCP 管理依赖存活 session，可能重复初始化、导航后丢失状态，或展示后端不支持的操作。

修改后：MCP 管理具有工作区级生命周期，可在零个或多个存活 session 下工作，操作结果同步到 session，并提供一致的管理 UI 与状态行为。

测试平台

OS	状态
🍏 macOS	✅ 已测试
🪟 Windows	⚠️ 未测试
🐧 Linux	⚠️ 未测试

环境

已完成定向验证：CLI MCP 测试 249 个通过、工作区设置测试 14 个通过、SDK daemon 事件测试 106 个通过、CLI/ACP bridge/SDK/Web Shell 包级类型检查、ACP bridge/SDK/WebUI 定向构建以及 git diff 检查。按要求未运行 Web Shell UI 测试或仓库全量 build。

风险与范围

• 主要风险或权衡：工作区 MCP 状态需要协调 daemon 所有的发现运行实例、状态缓存和活跃 session，同步生命周期与并发管理路径风险最高。
• 未验证或不在范围内：Windows 和 Linux 手动验证、Web Shell UI 自动化、仓库全量 build，以及项目 .mcp.json 和临时运行时 MCP 的管理。
• 破坏性变更或迁移说明：没有有意的破坏性变更。旧 source 字段保持原有运行时含义；新增 configOrigin，新的消费者应使用它获取精确来源。

关联 Issue

无

[github-actions]

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。

[qwen-code-ci-bot]

🖼️ web-shell visual preview

Auto-rendered from this PR head fdfcbf4 against a mock daemon (no real backend). Refreshes on every push.

Screenshots · light / dark

view	light	dark
Model Dialog
Permission Panel
Session Transcript
Slash Menu
Theme Dialog

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

[qwen-code-ci-bot]

Thanks for the PR @ytahdn!

Template looks good — all required sections present, bilingual body included, test plan concrete. ✓

Problem (observed, not theoretical): the PR addresses a real lifecycle-coupling issue — MCP management previously depended on a live chat session, which could lose discovery state and fail operations when no session existed. The PR body explains the failure modes clearly and the reviewer test plan covers the before/after scenarios. No before/after reproduction is attached, but the failure mode is an architectural property of the previous session-scoped design rather than a specific bug path, so the framing is reasonable.

Direction: aligned. Workspace-level MCP management continues the multi-workspace daemon direction that the recent PRs in this area (runtime workspace removal #6745, persisted workspace registrations #6716, channel workers grouped by workspace #6635, workspace-qualified ACP transport #6621) have been building toward. Decoupling MCP administration from session lifecycle is the natural next step. CHANGELOG has no direct prior MCP-management entry, but the workspace/daemon cluster of recent work makes the area clearly relevant.

Size: feat(serve) touching core paths — no hard block, but flagged for maintainer awareness.

• Production logic: +4,189 / -1,510 = 5,699 lines (> 500 → maintainer awareness; > 1,000 → splitting advisory)
• Test code: +1,957 / -7 = 1,964 lines
• Generated / schema: 0
• Core paths (packages/core/src/**): minimal — only a 10-line read-only getServerStatus getter in mcp-client-manager.ts. The bulk sits in packages/cli/src/acp-integration/acpAgent.ts (the MCP lifecycle engine), packages/acp-bridge/src/bridge.ts (status cache + discovery orchestration), packages/cli/src/serve/routes/workspace-mcp-control.ts (HTTP routes), and the new McpManagerPage.tsx (~1,773 lines). The cross-package surface (core + cli + acp-bridge + sdk-typescript + web-shell + webui) is wide, which is the main thing the maintainer should weigh.

Approach:

• The scope is internally coherent — every piece serves "workspace MCP management": discovery runtime, status cache, control routes, UI, and secret preservation. Splitting it further would leave each piece depending on the others being merged first.
• One observation for discussion: McpManagerPage.tsx is a single 1,773-line React component covering list, detail, edit, add, OAuth polling, and confirm flows. If a future iteration breaks it into smaller subcomponents (list / detail / forms), that would be an incremental cleanup rather than a pre-merge block. Not raising as a concern — flagging for the maintainer's awareness given the file's size.
• The PR also removes the old McpDialog.tsx (~712 lines) and McpDialog.module.css (~368 lines) and replaces them with a management page — a deliberate scope swap rather than drive-by churn, consistent with the stated goal.
• Secret redaction (redactMcpServerSecrets / restoreRedactedMcpSecrets) was moved from inline in acpAgent.ts into a shared mcp-server-secrets.ts module and extended to cover oauth.clientSecret. This is a clean refactor driven by the new edit flow — no concern.

Re-run note (a78cd5e): two follow-up commits since the previous review at eb228d1 — ed9ea7fcc (align MCP action expectation in App.test.tsx) and a78cd5e6b (address MCP review findings: defensive Array.isArray guard in bridge, createWorkspaceMcpDiscoveryConfig error wrapping, and try-catch for tool/resource refresh in the management page). Gate analysis unchanged — all findings above still hold.

Moving on to code review and real-scenario testing. 🔍

中文说明

感谢 @ytahdn 的贡献！

模板完整 ✓ —— 所有章节齐全，含双语正文，测试计划具体。

问题（已观察到，非理论性）： PR 解决一个真实的生命周期耦合问题——此前 MCP 管理依赖存活聊天 session，可能在无 session 时丢失发现状态、操作失败。正文对失败模式的描述清楚，reviewer 测试计划覆盖 before/after 场景。未附复现，但该失败模式是此前 session-scoped 设计的架构属性，而非具体 bug 路径，所以 framing 合理。

方向： 对齐。工作区级 MCP 管理延续了近期多工作区 daemon 方向（运行时工作区移除 #6745、持久化工作区注册 #6716、按工作区分组的 channel worker #6635、工作区限定的 ACP 传输 #6621）。把 MCP 管理从 session 生命周期中解耦是自然的下一步。CHANGELOG 中没有直接对应的 MCP 管理条目，但该领域与近期的 workspace/daemon 集群明显相关。

规模： feat(serve) 触及核心路径——未触发硬阻断，但需提请关注。

• 生产代码：+4,189 / -1,510 = 5,699 行（> 500 需维护者关注；> 1,000 建议考虑拆分）
• 测试代码：+1,957 / -7 = 1,964 行
• 生成 / schema：0
• 核心路径（packages/core/src/**）：影响极小——mcp-client-manager.ts 仅新增 10 行只读 getServerStatus getter。主体在 acpAgent.ts（MCP 生命周期引擎）、bridge.ts（状态缓存与发现编排）、workspace-mcp-control.ts（HTTP 路由）以及新增的 McpManagerPage.tsx（约 1,773 行）。跨包面（core + cli + acp-bridge + sdk-typescript + web-shell + webui）较宽，是维护者主要需要考量的点。

方案：

• 范围内部内聚——每个部分都服务于"工作区 MCP 管理"：发现运行实例、状态缓存、控制路由、UI、敏感字段保留。进一步拆分会导致每部分都需要等其它部分先合入。
• 一个供讨论的观察：McpManagerPage.tsx 是单个 1,773 行的 React 组件，覆盖列表、详情、编辑、新增、OAuth 轮询、确认流程。如果未来迭代把它拆成更小的子组件（list / detail / forms），会是增量清理而非合入前的阻断。不作为问题提出——仅因文件规模提醒维护者关注。
• 同步删除旧的 McpDialog.tsx（约 712 行）和 McpDialog.module.css（约 368 行），替换为管理页——是目标驱动的范围置换而非顺手改动，与声明的目标一致。
• 敏感字段脱敏（redactMcpServerSecrets / restoreRedactedMcpSecrets）从 acpAgent.ts 内联代码搬到独立的 mcp-server-secrets.ts，并扩展覆盖 oauth.clientSecret。这是为新的编辑流程驱动的重构——无问题。

Re-run 说明（a78cd5e6b）： 自上次 review（eb228d1）以来新增两个跟进提交——ed9ea7fcc（对齐 App.test.tsx 中的 MCP 操作预期）和 a78cd5e6b（处理 MCP review 发现：bridge 中的防御性 Array.isArray 检查、createWorkspaceMcpDiscoveryConfig 错误包装、管理页中工具/资源刷新的 try-catch）。Gate 分析不变——以上所有结论仍然成立。

接下来进入代码审查与真实场景测试。🔍

— Qwen Code · qwen3.7-max

_{Reviewed at a78cd5e6b7d5fcd786154b6ac7c3c5217add2d1f · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review (re-run on a78cd5e)

Two follow-up commits landed since the previous review at eb228d1:

1. ed9ea7fcc — "fix(web-shell): align MCP action expectation" — fixes the App.test.tsx failure that was the sole correctness blocker. The mock now includes removable: true and the assertion correctly expects Authenticate to be absent when requiresAuth is not set. Clean one-line fix matching the analysis from the previous Stage 2.

2. a78cd5e6b — "fix(serve): address MCP review findings" — three targeted defensive improvements:

   • Bridge Array.isArray(rawStatus.servers) guard in bridge.ts: early-returns when the status response lacks a servers array, preventing crashes from malformed daemon responses. Good defensive coding — the previous code assumed the array was always present.
   • createWorkspaceMcpDiscoveryConfig error wrapping in acpAgent.ts: catches creation failures and pushes them to syncErrors instead of letting the exception propagate uncaught. This is the right pattern — it matches how the surrounding code aggregates errors with AggregateError.
   • Try-catch for loadServerData in management page in McpManagerPage.tsx: prevents tool/resource refresh errors from breaking the post-management status polling loop. The comment notes that errors are still recorded inside loadServerData, so observability is preserved.

All three fixes are minimal, targeted, and directly address findings from the previous review. No scope creep or drive-by changes.

Previous Stage 2 concerns — status:

#	Concern	Status
1	App.test.tsx failing on new HEAD	Fixed (ed9ea7f)
2	restoreRedactedMcpSecrets generic type cast	Open (hygiene, non-blocking)
3	Partial-failure semantics in reconcileMcpServerAcrossLiveConfigs	Open (documented in PR risk section, non-blocking)
4	workspaceMcpDiscoveryConfig never torn down	Open (likely intentional, non-blocking)

Core path review: unchanged — getServerStatus in mcp-client-manager.ts is still a clean 10-line read-only getter.

Real-scenario testing

All affected test suites pass on the current HEAD (a78cd5e6b). The previously failing App.test.tsx test now passes. Total: 1,838 tests across 6 suites, 0 failures.

=== Web Shell App.test.tsx ===
 ✓ App.test.tsx (103 tests) 2223ms

 Test Files  1 passed (1)
      Tests  103 passed (103)
   Duration  3.59s

=== ACP bridge bridge.test.ts ===
 ✓ src/bridge.test.ts (403 tests) 3719ms

 Test Files  1 passed (1)
      Tests  403 passed (403)
   Duration  6.95s

=== CLI server.test.ts + workspace-settings.test.ts ===
 Test Files  2 passed (2)
      Tests  740 passed (740)
   Duration  22.45s

=== SDK daemonEvents + DaemonClient + acpRouteTable ===
 ✓ test/unit/acpRouteTable.test.ts  (103 tests) 16ms
 ✓ test/unit/DaemonClient.test.ts  (270 tests) 1917ms

 Test Files  3 passed (3)
      Tests  479 passed (479)
   Duration  2.33s

=== Core mcp-client-manager.test.ts ===
 Test Files  1 passed (1)
      Tests  113 passed (113)
   Duration  5.79s

I did not run a before/after tmux drive of the live web-shell MCP management UI — that requires a running daemon with OAuth-configured MCP servers, which is outside what's feasible in this CI environment. The unit-test evidence covers the correctness surface (the previous failure was a unit-level assertion mismatch, now resolved), and the PR body already flags that Web Shell UI manual testing was not run on Linux/Windows.

中文说明

代码审查（在 a78cd5e 上 re-run）

自上次 review（eb228d1）以来新增两个跟进提交：

1. ed9ea7fcc — "fix(web-shell): align MCP action expectation"：修复了 App.test.tsx 失败——此前唯一的正确性阻断。mock 现在包含 removable: true，断言正确预期在没有 requiresAuth 时 Authenticate 不应出现。干净的一行修复，与上次 Stage 2 分析完全吻合。

2. a78cd5e6b — "fix(serve): address MCP review findings"：三个有针对性的防御性改进：

   • Bridge Array.isArray(rawStatus.servers) 守卫（bridge.ts）：当 status 响应缺少 servers 数组时提前返回，防止畸形 daemon 响应导致崩溃。
   • createWorkspaceMcpDiscoveryConfig 错误包装（acpAgent.ts）：捕获创建失败并推入 syncErrors，而不是让异常未捕获地传播。
   • 管理页中 loadServerData 的 try-catch（McpManagerPage.tsx）：防止工具/资源刷新错误打断管理操作后的状态轮询。

三个修复都是最小化、有针对性的，直接对应上次 review 的发现。没有范围蔓延或顺手改动。

上次 Stage 2 关注——状态：

#	关注	状态
1	App.test.tsx 在新 HEAD 上失败	已修复（ed9ea7fcc）
2	restoreRedactedMcpSecrets 泛型类型断言	待处理（代码卫生，不阻断）
3	reconcileMcpServerAcrossLiveConfigs 部分失败语义	待处理（已在 PR 风险章节记录，不阻断）
4	workspaceMcpDiscoveryConfig 从不清理	待处理（可能是有意的，不阻断）

核心路径审查： 不变——mcp-client-manager.ts 中的 getServerStatus 仍然是干净的 10 行只读 getter。

真实场景测试

所有受影响的测试套件在当前 HEAD（a78cd5e6b）上通过。此前失败的 App.test.tsx 测试现在通过。总计：6 个套件 1,838 个测试，0 失败。

我没有在 tmux 中做 Web Shell MCP 管理 UI 的 before/after 实操驱动——那需要带 OAuth 配置的 MCP 服务器运行中的 daemon，超出 CI 环境能力。单元测试证据覆盖了正确性面（此前的失败是单元级断言不匹配，现已修复），PR 正文已标记 Linux/Windows 上未进行 Web Shell UI 手动测试。

— Qwen Code · qwen3.7-max

_{Reviewed at a78cd5e6b7d5fcd786154b6ac7c3c5217add2d1f · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — solid across every stage; the one correctness blocker from the previous review is fixed, all 1,838 tests pass, and three non-blocking hygiene items remain for future cleanup.

Stepping back from the stages, my honest read on this re-run:

• The previous review at eb228d1 deferred at 3/5 because of one App.test.tsx failure. That failure is now fixed (ed9ea7fcc) — the mock adds removable: true and the assertion correctly expects Authenticate to be absent when requiresAuth is not set. The fix matches the analysis from the previous Stage 2 exactly.
• The second follow-up (a78cd5e6b) adds three targeted defensive improvements: a bridge Array.isArray guard for malformed daemon responses, error wrapping for createWorkspaceMcpDiscoveryConfig, and a try-catch for tool/resource refresh in the management page. Each is minimal, focused, and directly addresses a review finding. No scope creep.
• All 1,838 tests across the six affected suites pass: web-shell (103), acp-bridge (403), CLI server + workspace-settings (740), SDK daemon events + DaemonClient + route table (479), and core mcp-client-manager (113).
• The three remaining Stage 2 concerns — generic type cast in restoreRedactedMcpSecrets, partial-failure semantics in reconcileMcpServerAcrossLiveConfigs, and workspaceMcpDiscoveryConfig lifetime — are hygiene. None makes the PR unsafe to ship. The partial-failure semantics concern is already documented in the PR's own risk section. The generic type cast is a future-caller-honesty question, not a today-correctness bug. The config lifetime is likely intentional (workspace MCP outlives individual sessions).
• Stage 0 flagged the PR's size (5,699 production lines across 6 packages) for maintainer awareness. The fork-refactor approval guardrail doesn't apply (title is feat, not refactor). The PR has been through 5+ review rounds with both automated and human reviewer participation — the maintainer is not unaware. The size escalation is a policy cap, not a genuine correctness concern, and the re-run policy says not to defer on hygiene when no blocking uncertainty remains.

Approving. The architecture earns its lines, the tests pass, the follow-up commits are clean, and the remaining concerns are cleanup items for future iterations.

中文说明

置信度：4/5 —— 各阶段扎实；上次 review 的唯一正确性阻断已修复，1,838 个测试全部通过，三个不阻断的代码卫生项留待后续清理。

从所有阶段退一步看，这次 re-run 的真实判断：

• 上次 review（eb228d1）因一个 App.test.tsx 失败在 3/5 时转交。该失败现已修复（ed9ea7fcc）——mock 添加了 removable: true，断言正确预期在没有 requiresAuth 时 Authenticate 不应出现。修复与上次 Stage 2 分析完全吻合。
• 第二个跟进提交（a78cd5e6b）新增三个有针对性的防御性改进：bridge 的 Array.isArray 守卫防止畸形 daemon 响应、createWorkspaceMcpDiscoveryConfig 错误包装、管理页中工具/资源刷新的 try-catch。每个都是最小化、聚焦的，直接对应 review 发现。没有范围蔓延。
• 六个受影响套件共 1,838 个测试全部通过：web-shell（103）、acp-bridge（403）、CLI server + workspace-settings（740）、SDK daemon events + DaemonClient + route table（479）、core mcp-client-manager（113）。
• Stage 2 剩余的三个关注——restoreRedactedMcpSecrets 的泛型类型断言、reconcileMcpServerAcrossLiveConfigs 的部分失败语义、workspaceMcpDiscoveryConfig 的生命周期——都是代码卫生。不会让 PR 在当前形式下不安全。部分失败语义已在 PR 自己的风险章节记录。泛型类型断言是未来调用者诚实性问题，不是今天的正确性 bug。Config 生命周期可能是有意的（工作区 MCP 超过单个 session）。
• Stage 0 因 PR 规模（5,699 行生产代码跨 6 个包）标记了维护者关注。fork-refactor 审批护栏不适用（标题是 feat 不是 refactor）。PR 已经历 5+ 轮 review，有自动化和人类 reviewer 参与——维护者并非不知情。规模升级是策略上限，不是真实的正确性顾虑，而 re-run 策略说在没有阻断性不确定性时不要因代码卫生而 defer。

Approve。架构赚到了它的行数，测试通过，跟进提交干净，剩余关注是未来迭代的清理项。

— Qwen Code · qwen3.7-max

_{Reviewed at a78cd5e6b7d5fcd786154b6ac7c3c5217add2d1f · re-run with @qwen-code /triage}

[wenshao]

Code review — feat(serve): add workspace MCP management (#6954)

Reviewed the full diff (54 files, +6139/−1516) across acp-bridge, cli/serve, core, sdk-typescript, web-shell, and webui. Overall this is a well-architected feature: durable config as the source of truth, an optional daemon-owned management runtime, reconcile-to-live-sessions on every mutating op, secret redaction on read + restore-on-write, an async OAuth flow that stops blocking a request for 10 minutes, complete EN/ZH i18n, and a design doc. Good work.

It is not mergeable as-is, though — the branch is red on one of its own new tests, and there are a couple of correctness rough edges worth fixing.

🔴 Blocking: CI is red (the PR's own new test fails, deterministically)

Test (ubuntu-latest) fails at packages/web-shell/client/App.test.tsx:1882:

App session callbacks > shows server operations without duplicating tools and resources tabs
AssertionError: expected '…EditReconnectDisable' to contain 'Authenticate'

The new test's fixture is a disconnected workspace_settings server with no requiresAuth and no removable:

{ name: 'filesystem', configOrigin: 'workspace_settings', disabled: false,
  mcpStatus: 'disconnected', resourceCount: 1 }

But serverActions() (McpManagerPage.tsx) gates those two actions:

• authenticate requires server.mcpStatus !== 'disconnected' || server.requiresAuth → false here.
• remove (Delete) requires server.removable → undefined here.

So the menu renders exactly Edit / Reconnect / Disable, and the toContain('Authenticate') / toContain('Delete') assertions can never pass. This is a pure logic-vs-fixture mismatch (not a flake). Fix by making the fixture consistent with the gating (add requiresAuth: true and removable: true) — or, if the intent is that workspace servers always expose Authenticate/Delete, change serverActions() and keep the fixture. Either way, the branch needs to go green before merge.

Correctness

• loadServerData() throw marks a succeeded action as failed. loadServerData collects tool/resource failures into loadErrorsByServer and then throw new AggregateError(...) (McpManagerPage.tsx ~line 598). It is awaited inside runAction's main try (~line 955), so if a manage/reconnect action succeeds but the follow-up tools/resources fetch fails, the catch overwrites the success notice with mcp.action.failed — even though the server was approved/enabled/authenticated/reconnected. The per-server error is already surfaced via loadErrorsByServer, so this is double-reporting a non-fatal detail fetch as a hard action failure. Consider loadServerData(...).catch(() => {}) at that call site, or move it out of the action's try.

• mcpAuthenticationResults is never cleared except on the next authenticate. In acpAgent.ts, clear-auth, disable, enable, and reload don't reset the stored {state:'succeeded'|'failed', error}, so buildWorkspaceMcpStatus keeps reporting the stale authenticationState/authenticationError indefinitely. It's mostly latent today (steady-state UI only renders pending/needsApproval/rejected, and a re-authenticate clears the map first), but e.g. clear-auth leaving a lingering succeeded is surprising. Worth clearing on clear-auth/disable/reload.

• The atomic MCP-mutation lock is bypassed by full-map writes. withMcpServerMutationLock is only taken when mcpServerMutation is present (workspace-settings.ts). A POST /workspace/settings with key:'mcpServers' and no mutation still does an unlocked read-modify-write (prepareSettingWrite reads existing, restores secrets, persists), so it can race a concurrent locked set/remove and lose data. The bundled UI always sends mutations, so this is latent, but the route is a public API — consider taking the lock for any mcpServers write, not just mutation writes.

Security

• Secret handling is solid: keys redacted (values → __redacted__) on read, sentinel restored from prior on write, keys with no prior are dropped (so the literal sentinel is never persisted), and broadcastSettingsChanged is fed the redacted publicValue. The shared mcp-server-secrets.ts extraction (with OAuth clientSecret coverage) is a nice consolidation, and OAuth token logging is fingerprint-only. 👍
• Note for reviewers: adding mcpServers to WEB_SHELL_SETTINGS means the Web settings route can now persist arbitrary command-executing MCP configs to user/workspace settings. That's the feature's intent and the workspace-qualified route is trust-gated, but it widens the Web write surface — worth confirming the daemon's auth/trust posture is sound with that in mind.

Performance

• OAuth polling is a fixed 1.5s interval for up to 400 attempts (~10 min), and each mcp.reload() triggers a full workspace MCP status refresh whose bridge side re-queries tools + resources for every connected server (cacheWorkspaceMcpDetails). During a long OAuth wait that's a lot of child ext-method traffic. A status-only poll (defer detail fetches until completion) or a small backoff would cut it substantially. Same fixed-interval shape in startDiscovery (40× / 60s).

Tests / minor

• Per the automated review: initializeWorkspaceMcp / initializeMcp are added structurally alongside the well-tested reload* variants but have no dedicated unit tests (DaemonClient, workspace actions). Cheap to add given the reload tests are right there.
• packages/web-shell/client/utils/errorDetail.ts is missing the SPDX license header that mcp-server-secrets.ts and most siblings carry (inconsistent across web-shell utils already, so minor).

Verdict

Architecturally sound and a genuinely useful capability, with good security hygiene. The blocker is the red CI on the PR's own new test; beyond that the loadServerData throw and the stale auth-state are the two I'd want addressed before merge, with the mutation-lock gap and polling cost as good follow-ups.

[qwen-code-ci-bot]

Not reviewed: reverse audit — worktree was cleaned up during review, reverse audit could not run.

Not reviewed: build-and-test — npm ci timed out in worktree; build and test results unavailable.

Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-15/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-15--work-qwen-code-qwen-code/subagents/80673a63-3250-4061-b1d3-097ec754954b (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-15/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-15--work-qwen-code-qwen-code/subagents/80673a63-3250-4061-b1d3-097ec754954b'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

Qwen Code review did not complete successfully. Qwen review exited with status 1. See workflow logs.

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to the diff; see the terminal output.

— qwen3.7-max via Qwen Code /review

[ytahdn]

Review triage for head a78cd5e:

The latest automated review reports no blockers. This PR has already gone through well over five review rounds, so per the repository review-round policy this round only landed correctness-blocking fixes and records remaining Suggestions for follow-up instead of widening the PR again.

Fixed in a78cd5e:

• Preserve and aggregate reload synchronization failures: feat(serve): add workspace MCP management #6954 (comment)
• Prevent successful MCP actions from being reported as failed when only tool/resource refresh fails.
• Defensively ignore malformed MCP status responses without ending OAuth pending state.

Rejected as inconsistent with the agreed product behavior:

• Disconnected servers without requiresAuth should show Reconnect, not Authenticate at the same time: feat(serve): add workspace MCP management #6954 (comment)
• The MCP tab intentionally shows the aligned empty state while loading; adding a new loading skeleton/spinner would reverse the requested behavior: feat(serve): add workspace MCP management #6954 (comment)
• Extension servers are emitted as non-removable by the backend, so the removable extension scenario is not reachable through the supported API: feat(serve): add workspace MCP management #6954 (comment)

Deferred to follow-up work:

• Inline status-panel OAuth polling and reconnect copy: feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment)
• Settings mutation hardening and serialization: feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment)
• Pool restart post-verification: feat(serve): add workspace MCP management #6954 (comment)
• Additional initialize/event unit coverage: feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment)
• Discovery lifecycle cleanup/coalescing and helper deduplication: feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment)
• Relaxing the workspace config ToolRegistry precondition: feat(serve): add workspace MCP management #6954 (comment)
• Avoiding an explicit undefined OAuth clientSecret property: feat(serve): add workspace MCP management #6954 (comment)
• Additional auth-failure and legacy-source presentation: feat(serve): add workspace MCP management #6954 (comment), feat(serve): add workspace MCP management #6954 (comment)
• OAuth start-timeout retry semantics: feat(serve): add workspace MCP management #6954 (comment)

All required CI checks are green. The remaining merge gate is maintainer review/decision.

[ytahdn]

@qwen-code /triage

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

@qwen-code /resolve

[qwen-code-dev-bot]

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution Summary — PR #6954

Base branch

main (fetched as origin/main)

Conflicted file

packages/cli/src/acp-integration/acpAgent.ts — one conflict region (lines 2746–2761)

Conflict analysis

Both sides added independent private fields to the QwenAgent class declaration:

HEAD (PR branch — feat/web-shell-mcp-page): Added three workspace MCP discovery fields plus two readonly Maps for MCP authentication tracking (pendingMcpAuthentications, mcpAuthenticationResults). These are core to the PR's workspace MCP management feature.

origin/main: Added one readonly Map generationControllers (mapping request IDs to { sessionId, controller: AbortController }). This supports a new session generation start/cancel feature landed on main.

Resolution

Kept both sets of fields. They are orthogonal features on the same class with no semantic interaction. Ordering: PR's MCP fields first, then main's generationControllers — this preserves logical grouping (MCP-related declarations together) and places the newer main addition after, matching the declaration order a fresh author would choose.

Verified all field names are referenced beyond their declarations (10, 19, and 5 occurrences respectively), confirming neither side is dead code.

Commit

chore: merge main into feat/web-shell-mcp-page (fdfcbf4)

[wenshao]

✅ Local verification report — workspace MCP management

Maintainer-run local build + real test verification of this PR, done in an isolated git worktree at the current PR head fdfcbf45a and baselined against current origin/main (ad65ba3bd). All package test suites touched by the PR pass, no regressions were introduced, and the new McpManagerPage was rendered end-to-end (real Tailwind/shadcn + i18n, daemon hooks mocked) with screenshots below.

The head advanced from a78cd5e6b to fdfcbf45a (a main merge) mid-verification. I re-ran everything at fdfcbf45a. The PR's own MCP files (workspace-mcp-control.ts, mcp-server-secrets.ts, workspace-settings.ts, mcp-client-manager.ts, McpManagerPage.tsx, PluginManagerPage.tsx, actions.ts, useDaemonMcp.ts) are byte-identical at both heads; the merge only added unrelated main content.

Test results (all green at fdfcbf45a)

Package · suite	Result
core · mcp-client-manager.test.ts (incl. new getServerStatus)	113 passed
cli · acp-integration/acpAgent.test.ts (workspace-MCP ops)	249 passed
cli · serve/routes/workspace-settings.test.ts + workspace-service/facade.test.ts	95 passed
cli · serve/server.test.ts (incl. new /workspace/mcp/* routes ×56)	683 passed, 51 pre-existing¹
acp-bridge · bridge.test.ts + status	403 passed
sdk-typescript · DaemonClient + acpRouteTable + daemonEvents + DaemonSessionClient	524 passed
webui · workspace/actions.test.ts + DaemonWorkspaceProvider.test.tsx	24 passed
web-shell · App.test.tsx + ExtensionsManagerPage.test.tsx	123 passed

≈ 2,214 tests passing across the changed surface.

¹ The 51 server.test.ts failures are pre-existing and environmental, not caused by this PR. Controlled experiment (same machine, same node_modules): the failing set is byte-identical between the PR and current origin/main.

	passed	failed	total
PR fdfcbf45a	683	51	734
origin/main ad65ba3bd	688	51	739

diff of the two failing-name sets ⇒ identical. They are all unrelated routes (POST /session returns 500 on this box, etc.) that fail on a clean main checkout too. The PR adds new passing tests and regresses nothing.

Typecheck (all tsc --noEmit clean at fdfcbf45a, deps built in dependency order)

sdk-typescript ✅ · acp-bridge ✅ · webui ✅ · web-shell ✅ · core build ✅

Note on the cli package typecheck (environmental, not a PR defect)

Raw cli tsc --noEmit inside the symlinked verification worktree emits false cross-package "property does not exist" errors because tsc resolves some @qwen-code/acp-bridge / @qwen-code/sdk subpath types against composite .d.ts declarations rather than the source paths map. This is not PR-specific:

• current origin/main emits 52 such errors in the identical setup; the PR emits 72 (31 shared).
• the 13-error PR delta all references symbols this PR adds and that are present in source — verified directly: AcpSessionBridge.initializeWorkspaceMcp/reloadWorkspaceMcp (bridgeTypes.ts:861,864), manageMcpServer(action: 'approve' | …) (bridgeTypes.ts:1171), and ServeWorkspaceMcpServerStatus.{configOrigin,approvalState,authenticationState,removable,…} (status.ts:220).
• the packages that define those symbols (acp-bridge, sdk-typescript) typecheck clean.

CI's dependency-ordered build resolves these; the PR's own CI typechecks pass.

UI — real render of the new McpManagerPage

The PR flagged Web Shell UI as not automation-tested. I rendered the real McpManagerPage component in a standalone Vite harness (real globals.css Tailwind v4 + shadcn tokens, real i18n; only the useMcp/useSettings daemon hooks mocked) and drove it with Playwright over representative multi-state fixtures (connected · needs-approval · OAuth-authenticating · disabled · extension-provided · disconnected-with-error). Zero console/page errors across all interactions.

Server list — light & dark

Light	Dark

Source filter → Extension (narrows to the extension-provided server; extension servers are visible but not individually removable — matches Test Plan #3)

Server detail — Basic Information & Tools browser (drill-down loads tools/resources on demand; invalid tool flagged Blocked)

Overview	Tools

Add MCP Server dialog (name / description / scope select / JSON config — light & dark, via Radix portal)

Light	Dark

Method

• Isolated git worktree detached at fdfcbf45a; node_modules symlinked from a built checkout; workspace deps resolve to source via each package's vitest/tsconfig paths aliases.
• server.test.ts baseline via a second worktree pinned to origin/main ad65ba3bd, same environment.
• Screenshots: real McpManagerPage in a Vite dev harness + Playwright (deviceScaleFactor: 2), light & dark.

Verdict

LGTM from a local-verification standpoint. Backend routes, secret redaction/restore, workspace-scoped MCP lifecycle, SDK typing, and the new Web Shell UI all behave correctly and are covered by green suites; no regression against current main. The only caveat is the environmental cli raw-tsc noise documented above, which CI does not hit.

🇨🇳 中文版本（点击展开）

✅ 本地验证报告 —— 工作区 MCP 管理

以维护者身份对本 PR 做了本地构建 + 真实测试验证：在隔离的 git worktree 中检出当前 PR HEAD fdfcbf45a，并与当前 origin/main（ad65ba3bd）做对照基线。PR 涉及的所有包测试套件全部通过，未引入任何回归；同时对新增的 McpManagerPage 做了端到端真实渲染（真实 Tailwind/shadcn + i18n，仅 mock daemon hooks），截图见下。

验证过程中 HEAD 从 a78cd5e6b 前进到 fdfcbf45a（一次 main 合并）。我已在 fdfcbf45a 重跑了全部验证。PR 自身的 MCP 文件（workspace-mcp-control.ts、mcp-server-secrets.ts、workspace-settings.ts、mcp-client-manager.ts、McpManagerPage.tsx、PluginManagerPage.tsx、actions.ts、useDaemonMcp.ts）在两个 HEAD 下逐字节相同，合并仅带入了无关的 main 内容。

测试结果（fdfcbf45a 全绿）

包 · 套件	结果
core · mcp-client-manager.test.ts（含新增 getServerStatus）	113 通过
cli · acp-integration/acpAgent.test.ts（工作区 MCP 操作）	249 通过
cli · serve/routes/workspace-settings.test.ts + workspace-service/facade.test.ts	95 通过
cli · serve/server.test.ts（含新增 /workspace/mcp/* 路由 ×56）	683 通过，51 为既有失败¹
acp-bridge · bridge.test.ts + status	403 通过
sdk-typescript · DaemonClient + acpRouteTable + daemonEvents + DaemonSessionClient	524 通过
webui · workspace/actions.test.ts + DaemonWorkspaceProvider.test.tsx	24 通过
web-shell · App.test.tsx + ExtensionsManagerPage.test.tsx	123 通过

改动面上 约 2,214 条测试通过。

¹ server.test.ts 的 51 条失败是既有的、环境相关的，与本 PR 无关。 对照实验（同机器、同 node_modules）：PR 与当前 origin/main 的失败用例集合逐字节相同。

	通过	失败	总计
PR fdfcbf45a	683	51	734
origin/main ad65ba3bd	688	51	739

两份失败用例名单 diff 结果完全一致，均为无关路由（如本机 POST /session 返回 500），在干净的 main 上同样失败。本 PR 只新增了通过的测试，未造成任何回归。

类型检查（fdfcbf45a 下各包 tsc --noEmit 干净，依赖按序构建）

sdk-typescript ✅ · acp-bridge ✅ · webui ✅ · web-shell ✅ · core 构建 ✅

关于 cli 包类型检查的说明（环境问题，非 PR 缺陷）

在使用符号链接 node_modules 的验证 worktree 中，cli 的原始 tsc --noEmit 会报出跨包 “property does not exist” 的假错误——因为 tsc 对部分 @qwen-code/acp-bridge / @qwen-code/sdk 子路径类型走了 composite .d.ts 声明，而非源码 paths 映射。这并非本 PR 特有：

• 当前 origin/main 在完全相同的环境下报 52 条同类错误；PR 报 72 条（31 条共有）。
• PR 多出的 13 条全部指向本 PR 新增且源码中确实存在 的符号，已逐一核对：AcpSessionBridge.initializeWorkspaceMcp/reloadWorkspaceMcp（bridgeTypes.ts:861,864）、manageMcpServer(action: 'approve' | …)（bridgeTypes.ts:1171）、ServeWorkspaceMcpServerStatus.{configOrigin,approvalState,authenticationState,removable,…}（status.ts:220）。
• 定义这些符号的包（acp-bridge、sdk-typescript）类型检查干净。

CI 的按依赖顺序构建可正确解析这些；PR 自身的 CI 类型检查通过。

UI —— 新增 McpManagerPage 真实渲染

PR 明确指出 Web Shell UI 未做自动化测试。我在独立 Vite harness 中渲染了真实的 McpManagerPage 组件（真实 globals.css Tailwind v4 + shadcn tokens、真实 i18n，仅 mock useMcp/useSettings），并用 Playwright 以覆盖多种状态的样例数据驱动（已连接 · 待审批 · OAuth 认证中 · 已禁用 · 扩展提供 · 断开并报错）。所有交互零 console/page 报错。

服务器列表 —— 浅色与深色

浅色	深色

来源筛选 → Extension（仅保留扩展提供的服务器；扩展服务器可见但不能单独删除 —— 对应测试计划 #3）

服务器详情 —— 基本信息与工具浏览（进入详情按需加载 tools/resources；非法工具标记为 Blocked）

概览	工具

新增 MCP Server 对话框（名称 / 描述 / 范围选择 / JSON 配置 —— 浅色与深色，经 Radix portal）

浅色	深色

方法

• 隔离 git worktree，detached 于 fdfcbf45a；node_modules 从已构建的 checkout 符号链接；工作区依赖通过各包 vitest/tsconfig 的 paths alias 解析到源码。
• server.test.ts 基线来自第二个固定在 origin/main ad65ba3bd 的 worktree，环境一致。
• 截图：真实 McpManagerPage 跑在 Vite dev harness + Playwright（deviceScaleFactor: 2），浅色与深色。

结论

从本地验证角度 LGTM。 后端路由、密钥遮罩/还原、工作区级 MCP 生命周期、SDK 类型、以及新增 Web Shell UI 行为均正确，且被全绿套件覆盖；相对当前 main 无回归。唯一需要说明的是上文所述的 cli 原始 tsc 环境噪声，CI 不会触发该问题。

[chiga0]

Code Review Overview (AI Generated)

PR: #6954 — feat(serve): add workspace MCP management
Type: New Feature
Change size: +6150/-1516 across 54 files
HEAD: fdfcbf45

Findings Summary

• Critical: 0
• Major: 3 (architecture/UX quality, not correctness blockers)
• Minor: 5
• Nit: 3

Cross-Validation

Finding	Other Reviewer	My Assessment
Latest review: LGTM	qwen-code-ci-bot	Confirmed — no blocking correctness issues
Earlier CHANGES_REQUESTED (dismissed)	qwen-code-ci-bot	Resolved in subsequent fix commits

Review

Comprehensive feature adding workspace-scoped MCP management across daemon, bridge, SDK, and Web Shell. The design is sound: management operations go through ensureChannel() with proper withWorkspaceControl accounting, configuration changes persist first then reconcile across live runtimes, and secret redaction is correctly extracted into a shared module (mcp-server-secrets.ts).

Key strengths verified:

• Secret handling: Backend redactMcpServersSetting correctly redacts before sending to frontend. restoreRedactedMcpSecrets preserves originals when __redacted__ is unchanged. Broadcasts use redacted values.
• OAuth flow: Handles server removal during polling, auth failure detection, daemon errors, URL validation (isHttpUrl), and finite iteration limits (400 × 1.5s = 10min max).
• Navigation state: Session preserved between chat ↔ MCP management. /mcp command opens management page directly.
• Mutation serialization: mcpServerMutationQueues correctly chains operations per workspace/scope, preventing concurrent config mutations.
• Status caching: mergeManagedWorkspaceMcpStatus preserves cached state across child process restarts, with proper handling of fresh children.

Major Findings (non-blocking)

M1. Monolithic 1777-line McpManagerPage component

Single function component with 20+ useState calls, all business logic, and all rendering. Not a correctness issue but makes the component hard to test, review, and maintain. Recommend extracting useMcpDiscovery, useMcpServerActions, useMcpServerForm hooks and ServerList, ServerDetail, ToolDetailView sub-components in a follow-up.

M2. __redacted__ placeholders visible in edit form textarea

Users see __redacted__ magic strings in the JSON config editor with no visual indication these are protected fields. If accidentally modified, the backend treats it as a new secret value. Backend behavior is correct (secrets never leak to frontend), but UX could surprise users. Recommend either stripping env/headers/oauth.clientSecret from the editable JSON and providing separate masked inputs, or adding a visible warning banner.

M3. OAuth authentication runs as detached async task with no shutdown cancellation

The authenticate action spawns a detached IIFE that continues executing against a partially-torn-down agent if shutdown occurs mid-authentication. Common pattern in long-running daemons but reconcileMcpServerAcrossLiveConfigs could iterate configs whose tool registries are already stopped. An AbortController would be cleaner.

Minor Findings

1. busyServer blocks ALL server actions during OAuth poll — overly conservative; should be per-server.
2. disable action has no confirmation dialog — unlike remove, executes immediately. Disabling interrupts all active connections.
3. originatorClientId defaults to 'daemon' sentinel — audit trails become ambiguous. Should be a documented constant or undefined.
4. workspaceMcpDiscoveryConfig lacks explicit teardown — pool drain handles connections but Config's tool registry/event listeners aren't explicitly disposed.
5. reconcileMcpServerAcrossLiveConfigs is not atomic — sequential operations leave partial state on failure. By design (per-runtime independence) but can cause cross-session inconsistency.

Final Verdict

APPROVE. The feature is well-designed with correct secret handling, proper OAuth flow management, and solid status caching. The Major findings are architecture/UX quality issues, not correctness blockers. The test coverage (249 CLI MCP tests, 106 SDK tests, bridge tests for channel lifecycle and status merging) is thorough.

---

This review was generated by QoderWork AI