Skip to content

feat(acp): add session-scoped runtime MCP - #7645

Closed
qqqys wants to merge 1 commit into
QwenLM:mainfrom
qqqys:feat/session-scoped-client-mcp
Closed

feat(acp): add session-scoped runtime MCP#7645
qqqys wants to merge 1 commit into
QwenLM:mainfrom
qqqys:feat/session-scoped-client-mcp

Conversation

@qqqys

@qqqys qqqys commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a session-owned runtime MCP route alongside the existing workspace-owned route. A daemon host can now add or remove an SDK MCP server for one selected live session without mutating bootstrap state, fanning out to siblings, or publishing workspace-wide events.

Carries the originating session identity through reverse MCP messages and verifies that the claimed session belongs to the ACP channel before forwarding it to the host-side sender.

Why it's needed

Daemon-managed channels need private agent tools that belong to the channel's live session. Reusing the workspace route would expose those tools to unrelated terminal and web sessions in the same runtime. This provides the generic isolation boundary required by the later channel-loop integration while remaining independent of channel behavior.

Reviewer Test Plan

How to verify

  • Create two thread-scoped sessions on one live ACP channel, add and remove a runtime MCP server for the second session, and confirm both requests carry only that session identity.
  • Request a session-scoped add for an unknown session and confirm it fails instead of falling back to the live workspace channel.
  • In the ACP child, register managers for two sessions and confirm only the selected manager receives the add and remove calls; security-sensitive runtime configuration remains stripped.
  • Send a reverse MCP frame with a valid session identity and confirm the host sender receives it; malformed or foreign session identities must fail before the sender runs.
  • Run cd packages/acp-bridge && npx vitest run src/bridgeClient.test.ts src/bridge.test.ts.
  • Run cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts.
  • Run npm run build, npm run typecheck, and npm run lint.

Evidence (Before & After)

N/A — protocol and routing infrastructure only; no UI changes.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Node.js 22 workspace install; package-level Vitest suites and repository build/type/lint checks.

Risk & Scope

  • Main risk or tradeoff: The new route is intentionally live-session scoped; callers must recreate the runtime server after that session is replaced or restored.
  • Not validated / out of scope: No channel loop server is registered here, and no browser or TUI behavior changes in this PR.
  • Breaking changes / migration notes: None. Existing workspace-scoped runtime MCP registration and senders that ignore the optional context remain compatible.

Linked Issues

Related to #7628 and #7641.

中文说明

本 PR 做了什么

在现有 workspace 归属的运行时 MCP 路由之外,新增 session 归属的运行时 MCP 路由。daemon host 现在可以只为一个指定的 live session 添加或移除 SDK MCP server,不会修改 bootstrap 状态、扩散到兄弟 session,也不会发布 workspace 级事件。

反向 MCP 消息会携带来源 session 身份;在转交给 host 侧 sender 之前,还会校验该 session 确实归当前 ACP channel 所有。

为什么需要

daemon-managed channel 需要只属于其 live session 的私有 agent tools。如果复用 workspace 路由,这些工具会暴露给同一 runtime 中无关的终端和 Web session。这个 PR 提供后续 channel-loop 集成所需的通用隔离边界,同时不包含任何 channel 业务。

Reviewer 测试计划

如何验证

  • 在同一个 live ACP channel 上创建两个 thread-scoped session,只为第二个 session 添加并移除运行时 MCP server,确认两个请求都只携带该 session 身份。
  • 对不存在的 session 发起 session-scoped add,确认请求失败而不是回退到 live workspace channel。
  • 在 ACP child 中为两个 session 注册 manager,确认只有目标 manager 收到 add/remove;安全敏感的运行时配置仍会被剥离。
  • 发送带合法 session 身份的反向 MCP frame,确认 host sender 收到该身份;畸形或属于其他 channel 的 session 身份必须在 sender 执行前失败。
  • 运行 cd packages/acp-bridge && npx vitest run src/bridgeClient.test.ts src/bridge.test.ts
  • 运行 cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts
  • 运行 npm run buildnpm run typechecknpm run lint

证据(前后对比)

N/A——仅协议和路由基础设施,没有 UI 变化。

测试平台

OS 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

Node.js 22 workspace 安装;执行 package 级 Vitest suite 与仓库 build/type/lint 检查。

风险与范围

  • 主要风险或取舍:新路由刻意绑定 live session;该 session 被替换或恢复后,调用方必须重新创建运行时 server。
  • 未验证 / 范围外:本 PR 不注册 channel loop server,也不改变浏览器或 TUI 行为。
  • 破坏性变更 / 迁移说明:无。现有 workspace-scoped 运行时 MCP 注册,以及忽略可选 context 的 sender,保持兼容。

关联 Issue

关联 #7628#7641

@qqqys

qqqys commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

E2E / routing verification report

This PR is protocol infrastructure; the in-process channel loop server is intentionally deferred to the follow-up integration PR.

  • ✅ Selected-session routing: two live sessions shared one ACP child; add/remove requests targeted only the selected session id.
  • ✅ No fallback: an unknown session failed with SessionNotFoundError without invoking the live channel.
  • ✅ Child isolation: only the selected session's MCP manager received add/remove, while trust and cwd inputs were stripped.
  • ✅ Reverse path ownership: valid session context reached the host sender; malformed and foreign-channel session ids were rejected before invocation.
  • packages/acp-bridge: 524 focused tests passed before the final ownership case; the final focused rerun passed all 78 bridge-client tests.
  • packages/cli: all 318 ACP agent tests passed; the final selected-session focused rerun also passed.
  • ✅ Repository build, typecheck, and lint passed.

Live channel-loop tool execution is not expected in this PR because no loop MCP server is registered yet. The follow-up E2E will exercise natural-language loop creation in a daemon-managed channel and verify sibling-session invisibility.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: This is a feature addition, not a bug fix — the motivation is clear. Daemon-managed channels need private MCP tools scoped to a single live session; the existing workspace-scoped route would expose them to unrelated sessions in the same runtime. The linked issues (#7628, #7641) confirm this is a planned building block for the channel-loop integration.

Direction: Aligned. Session-scoped MCP routing is a natural extension of the existing workspace-scoped route, and the ACP bridge already has the requestSessionStatus / sessionOrThrow patterns for session-targeted operations. CHANGELOG has no direct reference to session-scoped MCP, but the daemon/session/MCP area is actively evolving and clearly relevant.

Size: Cross-package (acp-bridge + cli). Production logic: 471 lines (bridge.ts 23, bridgeClient.ts 21, bridgeOptions.ts 8, bridgeTypes.ts 69, status.ts 2, acpAgent.ts 348). Test: 267 lines. Under the 500-line maintainer-awareness threshold.

Approach: The scope feels right. The PR does two things and both are needed: (1) adds session-scoped add/remove ext methods that reuse the same validation and security-stripping logic as the workspace route (extracted into shared helpers — a clean dedup, not a drive-by refactor), and (2) forwards the originating session identity through reverse MCP messages with ownership verification. No unrelated changes. The type extraction in bridgeTypes.ts is a natural consequence of sharing the result types between workspace and session routes.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这是一个功能新增,不是 bug 修复——动机清晰。daemon 管理的 channel 需要只属于单个 live session 的私有 MCP 工具;现有的 workspace 级路由会将这些工具暴露给同一 runtime 中无关的 session。关联的 issue(#7628#7641)确认这是 channel-loop 集成的计划组件。

方向:对齐。session 级 MCP 路由是现有 workspace 级路由的自然扩展,ACP bridge 已有 requestSessionStatus / sessionOrThrow 模式用于 session 定向操作。CHANGELOG 没有直接提及 session-scoped MCP,但 daemon/session/MCP 领域正在活跃演进,明显相关。

规模:跨包(acp-bridge + cli)。生产逻辑:471 行(bridge.ts 23、bridgeClient.ts 21、bridgeOptions.ts 8、bridgeTypes.ts 69、status.ts 2、acpAgent.ts 348)。测试:267 行。低于 500 行维护者关注阈值。

方案:范围合理。PR 做了两件事且都是必要的:(1) 新增 session 级 add/remove ext method,复用与 workspace 路由相同的校验和安全剥离逻辑(提取为共享 helper——是干净的去重,不是顺手重构);(2) 在反向 MCP 消息中转发来源 session 身份并进行归属校验。没有无关改动。bridgeTypes.ts 中的类型提取是 workspace 和 session 路由共享结果类型的自然结果。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 46eeb21e2420a2b66d591504e063081a5a03a963 · re-run with @qwen-code /triage

@qqqys

qqqys commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Closing temporarily to keep this dependent series strictly serialized. The branch and verified commit are retained; this PR will be rebased onto main and reopened after #7641 merges.

@qqqys qqqys closed this Jul 24, 2026
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: Given the goal (session-scoped MCP routing for daemon channels), I would: (1) add two new ext methods (sessionMcpRuntimeAdd/Remove) that look up the target session via sessionOrThrow and call its own McpClientManager, (2) extract the existing workspace handler's validation and security-stripping into shared helpers to avoid duplication, (3) add bridge-level methods routing through the existing requestSessionStatus pattern, (4) forward the originating session identity through reverse MCP messages with an ownsSession ownership check, and (5) extract the inline result types since they're now shared.

Comparison with the diff: The PR matches this proposal almost exactly. The implementation is clean and follows established patterns throughout.

Key observations:

  • Security stripping is preserved. The session-scoped add path uses the same readRuntimeMcpAddRequest helper as the workspace route — trust, authProviderType, includeTools, excludeTools, cwd, env, oauth, headers are all stripped. The test explicitly passes trust: true and cwd: '/untrusted' and verifies the manager receives only { type: 'sdk' }.

  • Session isolation is correct. The session-scoped handlers call sessionOrThrow(sessionId) and operate on that session's Config only — no fan-out to siblings, no workspace bootstrap mutation. The test verifies the other session's manager is never called.

  • Backward compatible. The sessionId in reverse MCP messages is optional; the ClientMcpMessageContext parameter on the sender is optional. Existing consumers that ignore the context continue to work.

  • Ownership verification. The ownsSession check in bridgeClient.ts rejects session IDs not owned by the current ACP channel before the sender runs. The default () => true preserves existing behavior for consumers that don't wire it up.

  • Workspace route unchanged in behavior. The refactored workspace handlers use the shared helpers but preserve the fan-out to active sessions and the same error mapping.

No critical blockers or AGENTS.md violations found.

Testing

Unit tests (as specified in the PR's test plan):

packages/acp-bridge: 520 tests passed (bridge.test.ts + bridgeClient.test.ts)
packages/cli: 317 tests passed (acpAgent.test.ts)

Typecheck: tsc --noEmit clean for both packages/acp-bridge and packages/cli.

Lint: ESLint clean on all 6 changed production files.

Build + smoke test (this PR is protocol infrastructure — no UI changes, so the tmux test verifies the build is functional):

$ node dist/cli.js -p 'say hello in one word' 2>&1
Warning: QWEN_HOME points to "/home/github-runner/actions-runner-25/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens,
settings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
Warning: QWEN_HOME points to "/home/github-runner/actions-runner-25/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens,
settings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
Hello

CLI starts and responds correctly with the PR build.

中文说明

代码审查

独立方案: 给定目标(为 daemon channel 提供 session 级 MCP 路由),我会:(1) 新增两个 ext method(sessionMcpRuntimeAdd/Remove),通过 sessionOrThrow 查找目标 session 并调用其自身的 McpClientManager;(2) 将现有 workspace handler 的校验和安全剥离逻辑提取为共享 helper 以避免重复;(3) 在 bridge 层通过现有 requestSessionStatus 模式添加路由方法;(4) 在反向 MCP 消息中转发来源 session 身份并进行 ownsSession 归属校验;(5) 提取内联结果类型(因为现在被 workspace 和 session 路由共享)。

与 diff 的对比: PR 几乎完全匹配此方案。实现干净,全程遵循既有模式。

关键观察:

  • 安全剥离已保留。 session 级 add 路径使用与 workspace 路由相同的 readRuntimeMcpAddRequest helper——trustauthProviderTypeincludeToolsexcludeToolscwdenvoauthheaders 全部被剥离。测试明确传入 trust: truecwd: '/untrusted',并验证 manager 只收到 { type: 'sdk' }

  • session 隔离正确。 session 级 handler 调用 sessionOrThrow(sessionId) 并只操作该 session 的 Config——不扩散到兄弟 session,不修改 workspace bootstrap 状态。测试验证了另一个 session 的 manager 从未被调用。

  • 向后兼容。 反向 MCP 消息中的 sessionId 是可选的;sender 上的 ClientMcpMessageContext 参数也是可选的。忽略 context 的现有消费者继续正常工作。

  • 归属校验。 bridgeClient.ts 中的 ownsSession 检查在 sender 执行前拒绝不属于当前 ACP channel 的 session ID。默认的 () => true 为未接入的消费者保留了现有行为。

  • workspace 路由行为不变。 重构后的 workspace handler 使用共享 helper,但保留了对活跃 session 的扩散和相同的错误映射。

未发现关键阻塞问题或 AGENTS.md 违规。

测试

单元测试(按 PR 测试计划):

packages/acp-bridge: 520 个测试通过(bridge.test.ts + bridgeClient.test.ts)
packages/cli: 317 个测试通过(acpAgent.test.ts)

类型检查: tsc --noEmitpackages/acp-bridgepackages/cli 均通过。

Lint: ESLint 对全部 6 个变更的生产文件通过。

构建 + 冒烟测试(本 PR 是协议基础设施——无 UI 变化,tmux 测试验证构建可用):

$ node dist/cli.js -p 'say hello in one word' 2>&1
Hello

CLI 使用 PR 构建正常启动并响应。

Qwen Code · qwen3.8-max-preview

Reviewed at 46eeb21e2420a2b66d591504e063081a5a03a963 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean implementation that follows established patterns throughout; the only gap is that full end-to-end verification of the session-scoped routing requires the daemon channel setup that isn't wired yet, so confidence rests on comprehensive unit tests rather than live behavior.

This is a well-built PR. The approach matches what I'd propose independently — extract the workspace handler's validation and security-stripping into shared helpers, add session-scoped ext methods that reuse them via sessionOrThrow, and thread the session identity through reverse MCP messages with ownership verification. No simpler path was missed.

What I like: the security stripping is identical between workspace and session routes (same helper, same test proving trust and cwd are stripped), the session isolation is verified by asserting the sibling session's manager is never called, and the backward compatibility is preserved through optional parameters. The workspace route's behavior is unchanged — same fan-out, same error mapping — just expressed through the shared helpers now.

The author has several related PRs open for the channel/daemon infrastructure effort. This one is well-scoped as a standalone building block: it adds the routing primitive without coupling to channel-loop behavior, and the follow-up integration can build on it cleanly.

All 837 unit tests pass, typecheck and lint are clean, and the build produces a working CLI.

中文说明

置信度:4/5 — 实现干净,全程遵循既有模式;唯一的差距是 session 级路由的完整端到端验证需要尚未接入的 daemon channel 环境,因此置信度主要基于全面的单元测试而非实际运行行为。

这是一个构建良好的 PR。方案与我独立提出的一致——将 workspace handler 的校验和安全剥离逻辑提取为共享 helper,添加通过 sessionOrThrow 复用这些 helper 的 session 级 ext method,并在反向 MCP 消息中传递 session 身份并进行归属校验。没有遗漏更简路径。

亮点:workspace 和 session 路由的安全剥离完全一致(相同 helper,相同测试证明 trustcwd 被剥离);session 隔离通过断言兄弟 session 的 manager 从未被调用来验证;向后兼容通过可选参数保留。workspace 路由的行为不变——相同的扩散、相同的错误映射——只是现在通过共享 helper 表达。

作者有多个相关的 channel/daemon 基础设施 PR 处于 open 状态。这个 PR 作为独立的构建块范围合理:添加了路由原语而不耦合 channel-loop 行为,后续集成可以干净地基于它构建。

全部 837 个单元测试通过,类型检查和 lint 干净,构建产出可用的 CLI。

Qwen Code · qwen3.8-max-preview

Reviewed at 46eeb21e2420a2b66d591504e063081a5a03a963 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants