refactor(mcp): migrate the legacy client path to SDK v2

[me2seeks]

Summary

• migrate the production MCP client from the monolithic v1 SDK to the v2 client/core packages while explicitly retaining legacy protocol negotiation
• publish one immutable, manager-owned callable Tool snapshot whose revision and opaque bindings are the discovery and execution authority
• preserve managed-network approval and durable raw model arguments while carrying Runtime invocation context through bound calls

Refs #1650

This is PR 1 of 3 for the dual-era rollout. It upgrades the current legacy-only path to SDK v2 while keeping the existing configuration, transport selection, and user-visible protocol behavior unchanged.

What changed

• Production clients set versionNegotiation: { mode: "legacy" } explicitly. The fixture matrix keeps a real v1.29 stdio peer and uses v2 server packages for remote legacy transports.
• Tool discovery uses bounded low-level pagination instead of the SDK response cache. Definitions and output schemas are fully validated before a candidate snapshot is published.
• McpToolProvider exposes one toolSnapshot() seam and one bound callTool(binding, rawArgs, context) seam. The old optional binding route, string-based call route, and host-side snapshot reconstruction are gone.
• The MCP manager owns the immutable { revision, tools } snapshot. It increments the revision only when the callable binding set changes, atomically replaces successful refreshes, and keeps the previous callable snapshot after a failed refresh.
• Each advertised Tool carries a main-process-only opaque binding to its validated connection generation and full definition. Removed, changed, disconnected, or replaced bindings fail locally before another request is sent.
• The Runtime adapter requests managed-network expansion immediately before provider dispatch. ToolRuntime records and replays the model's raw schema arguments; bindings and server identity remain execution-only metadata.
• The Runtime Host Client Capability adapter implements the same bound snapshot contract and preserves session, turn, tool-call, cwd, timeout, and abort context through the invocation broker.
• Desktop and Runtime Host consume the manager's authoritative snapshot revision directly when publishing MCP capability changes.
• Deferred-result classification still happens after one wire call and before successful structured-output validation. A response is never recovered by invoking a Tool a second time.
• Connection, refresh, Tool error, and stdio diagnostics retain explicit size, redaction, and control-character boundaries.
• The unreachable modern SEP-2243 header path remains out of this legacy-only PR. It belongs to the later automatic-negotiation work.

Regression coverage

• A real SSE fixture emits notifications/tools/list_changed: a valid refresh publishes one new immutable snapshot, while an invalid refresh preserves the prior callable snapshot.
• A real SQLite ToolRuntime replay proves raw model arguments are durable and the opaque binding never enters Runtime events or replay input.
• Direct and nested Code Mode tests prove managed-network approval happens before provider dispatch.
• Manager tests retain pagination, refresh-race, stale-binding, validation, schema, timeout, abort, and diagnostic coverage.

Verification

• npm test — all workspace tests passed before the final conflict-free rebase
• npm run format:check
• npm run lint
• npm run typecheck
• npm run check:third-party-notices
• npm run test:scripts — 126 passed, 2 skipped
• After rebasing onto a956756a:
  • npm run build:test
  • npm --workspace @maka/mcp test — 63 passed
  • focused Runtime MCP, Code Mode, and SQLite boundary tests — 31 passed
  • Runtime Host Client Capability tests — 28 passed
  • npm --workspace @maka/desktop run test:dist — 1430 passed

Review focus

• the advertised descriptor, opaque binding, and revision come from one immutable manager-owned snapshot
• managed-network authority remains before every remote MCP dispatch
• durable T1/T2 state contains only the model's raw Tool arguments
• refresh, reconnect, disconnect, and replacement races cannot publish or execute stale bindings
• Runtime Host and Desktop consume the authoritative revision without a parallel identity counter

Modern negotiation, protocol configuration/status UI, modern Tool-list subscriptions, input_required, and narrowed Streamable HTTP-to-SSE fallback remain follow-up work.

[Astro-Han]

P1. The interface rewrite covers only one implementer. packages/runtime-host/src/server/client-capability-coordinator.ts:600 still implements McpToolProvider with the old surface (tools() plus the three-argument callTool), and the new callTool(binding, { ... }) drops the context: McpToolInvocationContext that main's packages/runtime/src/mcp-tools.ts now requires. Rebase is not a mechanical conflict resolution: runtime-host calls will fail typecheck or get rejected at runtime with "Client Capability invocation context is missing". The desktop snapshot watcher also moved from main.ts to boot.ts on main.

P2. createClient pins versionNegotiation: { mode: 'legacy' }, so getProtocolEra() never returns 'modern' and enforceMcpHeaders is always false. The whole SEP-2243 path ships unreachable: sep-2243.ts (~270 lines), the header checks, the snapshot partition, and the console.warn allow entry in scripts/check-console.mjs. No manager-level test can reach it. Either delete the path until 'auto' negotiation lands, or switch the mode and add a test that exercises exclusion and pre-call rejection.

P3. refreshToolLoop throws MCP server "x" connection changed during tool refresh: undefined when failure is undefined, because safeMcpOperationError defaults includeCause to true.

P3. mcpBoundToolSnapshotKey deep-clones and stringifies every descriptor on each status emit. The binding string already captures the same change set; JSON.stringify(bindings) would do.

[me2seeks]

Thanks — I rebased onto the current main and addressed all four findings on the new head 8de0614b.

• I updated the Runtime Host Client Capability adapter to the bound-tool provider surface and preserved the full invocation context through the broker. The Desktop watcher stays in main's boot.ts.
• I removed the unreachable modern SEP-2243 header path from this legacy-only PR. Negotiation remains explicitly pinned to legacy.
• Refresh errors no longer append : undefined when there is no underlying failure.
• The Desktop snapshot key now serializes only opaque bindings.

I also kept the existing Runtime Host dependency boundary instead of adding an allowlist exception: the adapter derives its opaque binding type from McpToolProvider.

On the current head, build, format, lint, typecheck, notices, MCP 62/62, Runtime MCP 7/7, Runtime Host seam 24/24, and Desktop surface 13/13 all pass. The full Runtime suite passed 3130 tests with 3 skipped. A full Runtime Host run had one unrelated five-second auxiliary-model timeout (663/664); that test passed immediately in isolation.

[Astro-Han]

Thanks for the substantial revision here. The central direction looks right: MCP discovery should publish one validated, immutable tool snapshot, and execution should use an opaque binding from that same snapshot. That is a stronger authority boundary than resolving serverId + toolName again at call time.

The previous review concerns around Runtime Host context propagation, unreachable SEP-2243 code, refresh diagnostics, and Desktop binding identity appear to be addressed. The remaining work is primarily integrating this design with constraints that have since landed on main.

Recommended final architecture

I suggest keeping the bound-snapshot design and completing it, rather than reintroducing a compatibility facade during the rebase:

MCP connection
    → complete pagination and schema validation
    → immutable ToolSnapshot { revision, [{ descriptor, opaque binding }] }
    → Runtime MakaTool closure
    → managed-network approval
    → callTool(binding, raw model args, context)

The responsibilities would remain:

• MCP manager: connection generation, discovery, validation, atomic snapshot publication, and binding validity.
• Runtime adapter: sandbox/network authority, timeout/abort context, and model-output projection.
• ToolRuntime: durable T1/T2 records using the model's original arguments.
• Runtime Host/Desktop: consume the authoritative snapshot revision instead of independently reconstructing tool identity.

In that end state, the old parallel routes can disappear: optional bindTool?, string-based callTool(serverId, toolName, ...), wrapped durable arguments, and host-side snapshot reconstruction.

Findings

P1 — Preserve the managed-network authority from current main

The PR head calls the provider binding directly from mcp-tools.ts. Current main requires managed executions whose network boundary is disabled to go through requestSandboxBoundary before any remote MCP call. Resolving the conflict in favor of the PR side would let Code Mode bypass that authority.

Please keep the network expansion check in the Runtime adapter immediately before callTool(binding, ...), and restore its focused regression test.

P1 — Persist the model's raw provider arguments, not host approval metadata

The branch still defines permissionArgs as { serverId, toolName, arguments }, and several tests pin that wrapped shape into durable tool-call state. Current main intentionally removed this behavior: replay must show the model the original arguments described by the tool schema.

The opaque binding and server identity should remain execution-only metadata. ToolRuntime should persist the original provider arguments. The MCP-specific T1/T2 tests can be collapsed to one SQLite replay test proving both properties:

• raw provider arguments survive replay;
• opaque bindings never enter durable state.

P1 — Preserve an authoritative callable-snapshot revision for Runtime Host

The new manager removes toolSnapshotRevision(), while current Desktop Runtime Host wiring still consumes that revision to publish MCP capability changes.

The clean solution is not to restore a separate counter beside the snapshot. Make revision part of the immutable snapshot itself, incrementing only when the callable binding set changes. Runtime Host can then consume that authoritative revision directly.

P3 — Exercise the real notifications/tools/list_changed path

Current tests call refreshTools() directly. A small integration test should make the fixture emit notifications/tools/list_changed, then verify that:

• a successful refresh atomically publishes a new snapshot;
• a failed refresh preserves the previous callable snapshot.

Test cleanup

The persisted-argument/T1/T2 cases in mcp-tools.test.ts currently assert the obsolete wrapped argument shape. The ordering and non-persistence boundaries are useful, but the cases do not all need to remain separate.

I suggest replacing them with one focused SQLite replay test that proves raw provider arguments are durable while opaque bindings remain execution-only. The manager's pagination, refresh-race, stale-binding, validation, and diagnostic tests remain valuable and should stay.

Overall, I do not think this needs another architectural rewrite. Please rebase onto current main, retain the bound-snapshot design, carry forward the three mainline constraints above, remove the obsolete wrapped-argument expectations, and rerun CI on the resolved merge head. Thanks!

中文对照

感谢对这个 PR 做了大幅调整。核心方向是正确的：MCP 工具发现应当发布一份经过验证、不可变的工具快照，实际执行则使用同一份快照生成的 opaque binding。相比在调用时重新通过 serverId + toolName 查找，这提供了更可靠的权威边界。

上次 review 提到的 Runtime Host context 透传、不可达的 SEP-2243 代码、refresh 错误信息以及 Desktop binding identity，看起来都已经处理。现在主要需要把这套设计与后来进入 main 的约束正确整合。

建议的最终架构

建议继续完成 bound snapshot 方案，不要为了方便 rebase 又引入兼容 facade：

MCP 连接
    → 完整分页和 schema 校验
    → 不可变 ToolSnapshot { revision, [{ descriptor, opaque binding }] }
    → Runtime MakaTool closure
    → managed-network 授权
    → callTool(binding, 模型原始参数, context)

职责保持如下：

• MCP manager：连接 generation、工具发现、校验、原子发布 snapshot，以及 binding 有效性。
• Runtime adapter：sandbox/network authority、timeout/abort context，以及模型输出转换。
• ToolRuntime：使用模型原始参数写入 durable T1/T2。
• Runtime Host/Desktop：直接消费权威 snapshot revision，不再自行重建工具 identity。

最终可以删除旧的平行路径：可选 bindTool?、基于字符串的 callTool(serverId, toolName, ...)、包装后的持久化参数，以及 Host 侧重新构造 snapshot identity。

Findings

P1 — 保留当前 main 的 managed-network authority

PR head 的 mcp-tools.ts 会直接调用 provider binding。当前 main 要求：managed execution 的网络边界尚未启用时，任何远程 MCP 调用都必须先经过 requestSandboxBoundary。如果解决冲突时直接采用 PR 侧实现，Code Mode 可能绕过这一授权边界。

请在 Runtime adapter 中、callTool(binding, ...) 之前保留 network expansion 检查，并恢复对应的 focused regression test。

P1 — 持久化模型的原始 provider 参数，而不是宿主审批元数据

当前分支仍通过 permissionArgs 生成 { serverId, toolName, arguments }，并有多项测试将这个包装结构钉入 durable tool-call state。当前 main 已经有意移除了这一行为：模型 replay 时必须看到工具 schema 所描述的原始参数。

opaque binding 和 server identity 应仅作为执行期元数据存在。ToolRuntime 应持久化原始 provider 参数。MCP 专属的 T1/T2 测试可以收缩为一个 SQLite replay 测试，同时证明：

• 原始 provider 参数能够正确 replay；
• opaque binding 永远不会进入 durable state。

P1 — 为 Runtime Host 保留权威的 callable snapshot revision

新 manager 删除了 toolSnapshotRevision()，但当前 Desktop Runtime Host 仍依赖这个 revision 发布 MCP capability 变化。

最干净的方案不是在 snapshot 之外恢复一个独立计数器，而是让 revision 成为不可变 snapshot 自身的一部分，并且只在 callable binding 集合真正变化时递增。Runtime Host 直接消费这份权威 revision。

P3 — 覆盖真实的 notifications/tools/list_changed 路径

当前测试都是直接调用 refreshTools()。建议增加一个小型集成测试，让 fixture 发出 notifications/tools/list_changed，然后验证：

• refresh 成功时原子发布新 snapshot；
• refresh 失败时继续保留上一份可调用 snapshot。

测试清理

mcp-tools.test.ts 中 persisted-argument/T1/T2 用例目前断言的是已经过时的包装参数结构。执行顺序和 binding 不进入持久化状态的边界仍然有价值，但不必保留为多组重复测试。

建议替换为一个 focused SQLite replay 测试，证明模型原始参数能够持久化，而 opaque binding 只存在于执行期间。Manager 的分页、refresh race、stale binding、校验和诊断测试仍然有明确价值，应当保留。

总体上，这个 PR 不需要再次推翻架构。请 rebase 到当前 main，保留 bound snapshot 设计，接回上述三条主线约束，删除已经过时的包装参数断言，并在解决冲突后的 merge head 上重新运行 CI。谢谢！

[me2seeks]

Thanks, this was very helpful. I rebased onto the current main and followed the suggested end state instead of adding a compatibility facade.

The manager now publishes one immutable { revision, tools } snapshot after complete discovery and validation. The revision changes only when the callable binding set changes, and Desktop and Runtime Host consume it directly.

I also kept the managed-network expansion check immediately before provider dispatch. ToolRuntime now persists and replays the model's raw arguments only; the opaque binding remains execution-only. I replaced the repeated MCP persistence cases with one real SQLite replay test covering both properties.

The SSE fixture now emits a real notifications/tools/list_changed notification. A valid refresh atomically publishes the replacement snapshot, while an invalid refresh leaves the previous snapshot callable.

The optional binding route, string-based call route, wrapped MCP arguments, and host-side snapshot reconstruction are gone. All checks are green on the current head 71907617. Ready for re-review — thanks again.

中文对照

谢谢，这些建议非常有帮助。我已经 rebase 到当前 main，并按建议完成了最终方案，没有引入兼容 facade。

现在 manager 会在完整发现和校验后发布一份不可变的 { revision, tools } snapshot。只有 callable binding 集合发生变化时 revision 才会更新，Desktop 和 Runtime Host 都直接消费这份 revision。

我也保留了 provider dispatch 之前的 managed-network expansion 检查。ToolRuntime 现在只持久化并 replay 模型的原始参数；opaque binding 只存在于执行期间。之前重复的 MCP 持久化测试已经收缩为一个真实的 SQLite replay 测试，同时覆盖这两个性质。

SSE fixture 现在会发送真实的 notifications/tools/list_changed 通知。有效 refresh 会原子发布新的 snapshot；无效 refresh 则继续保留上一份可调用 snapshot。

可选 binding 路径、基于字符串的调用路径、包装后的 MCP 参数以及 Host 侧 snapshot 重建都已经删除。当前 head 71907617 的所有检查均已通过，可以重新 review。再次感谢。

[Astro-Han]

Thanks — after looking at these findings together, I do not think the best fix is to patch schema validation and the initial-notification race independently. They have the same root cause: the manager owns the published snapshot, but it does not yet own the complete transition that makes a snapshot authoritative.

I suggest completing one generation-owned discovery transaction with these invariants:

1. Every entry in a published snapshot is fully validated and immediately callable.
2. A connection generation has exactly one discovery/refresh owner.
3. tools/list_changed is owned from the moment the connection is established, including during initial discovery.
4. A candidate observed to be stale before publication is discarded.
5. Binding validity is decided only by the manager's current index and snapshot state.

A minimal flow would be:

install generation-fenced notification handler
→ record the current change epoch
→ fetch all pages
→ validate and prepare every definition
→ if the epoch changed, discard and repeat
→ otherwise atomically publish

The notification handler only needs to increment the generation's change epoch and join the existing single-flight refresh. There should be no separate initial-refresh path or parallel notification queue.

This also places output-schema compilation at the correct boundary. callPreparation should be required on every published entry, not optional and initialized by the first call. A malformed refresh must fail before publication and preserve the previous snapshot, revision, and callable binding.

For bindings, please keep definition comparison as an internal concern, but simplify the externally exposed binding to a manager-minted opaque token. The digest and parser are not authorities: validity is already determined by bindingIndex, the current generation, and snapshot identity. Removing the encoded identity would let the format-only binding tests disappear while retaining the important reuse, rotation, reconnect, and stale-before-wire behavior tests.

For the Runtime SDK v1 dependency, please let ownership decide the result: remove the direct dependency from packages/runtime, regenerate the lockfile and notices, and rerun package/audit. If a real production packaging path requires it, that path should identify the dependency owner and have a focused package test. Keep the Hono override only if the audit still requires it.

Please rebase onto current main first so this transaction is implemented on top of the new stable-provider/reconnect ownership. Preserve main's identity and lease tests together with this PR's context assertions.

The target is not more defensive layers. It is one transaction owner, one published authority, and one binding authority.

[Astro-Han]

Thanks — this approval supersedes my previous blocking recommendation. I recalibrated the review after separating architectural importance from user-facing severity.

The central design is sound: the manager owns one immutable callable snapshot, Runtime preserves the network authority and raw durable arguments, and execution uses a binding from the same validated snapshot. I am approving this direction and the current migration.

The following are non-blocking follow-ups rather than P0/P1 regressions:

• Complete the generation-owned discovery transaction so output-schema preparation happens before publication and tools/list_changed is owned during initial discovery. The simplest model is one change epoch, one single-flight refresh owner, and publication only when the candidate is fully prepared and still current.
• Consider simplifying the external binding to a manager-minted opaque token. Definition comparison may remain internal; the digest and parser do not need to act as a second identity representation.
• Let package ownership determine whether packages/runtime needs the legacy MCP SDK directly. Remove it and rerun package/audit; retain it only if a concrete production packaging path demonstrates that requirement.

These recommendations should not lead to separate compatibility paths, notification queues, or additional authority layers. The desired end state remains one transaction owner, one published snapshot authority, and one binding authority.

Please still rebase onto current main and preserve both the stable-provider reconnect tests and this PR's invocation-context assertions when resolving the Runtime Host overlap.

[me2seeks]

Thanks — I rebased the approved head onto current main and preserved main's stable provider/reconnect ownership. The Runtime Host adapter remains keyed by providerId across reconnects, while the frozen snapshot binding still carries the full invocation context into Client Capability dispatch.

I kept the three non-blocking follow-ups separate from this migration: generation-owned initial discovery, opaque token simplification, and Runtime SDK dependency ownership. I did not add a compatibility facade or another authority path.

The new head is 8fc084ba. All 12 GitHub checks are green, including Runtime Host, workspace tests, E2E, dependency audit, Windows baseline, and Windows packaging. Ready for another look — thanks.

中文对照

谢谢。我已经将已批准的实现 rebase 到当前 main，并保留了主线稳定的 provider/reconnect 所有权。Runtime Host adapter 在重连前后仍由 providerId 标识，同时 frozen snapshot binding 继续将完整调用上下文传递到 Client Capability dispatch。

三个 non-blocking 后续方向没有混入本次迁移：generation-owned initial discovery、opaque token 简化，以及 Runtime SDK 依赖所有权。本次也没有增加兼容 facade 或另一条 authority 路径。

新的 head 是 8fc084ba。GitHub 的 12 项检查均已通过，包括 Runtime Host、workspace tests、E2E、dependency audit、Windows baseline 和 Windows packaging。可以再次查看了，谢谢。

[Astro-Han]

Thanks — I re-reviewed the current head 962878b6 after the final rebase and CLI snapshot adaptation.

The approved architecture still holds: the manager publishes one immutable callable snapshot; Runtime, Runtime Host, Desktop, and CLI consume bindings and revision from that same authority; managed-network approval remains immediately before provider dispatch; durable Tool state contains the model’s raw arguments; and the stable provider/reconnect ownership from main is preserved.

I found no P0–P2 issues. The current head is cleanly mergeable and all 13 GitHub checks are green.

One non-blocking cleanup remains:

• P3 — Remove the superseded revision field. packages/mcp/src/index.ts still declares toolSnapshotRevisionValue, but the old accessor and mutation path are gone and the field is never used. Removing it would complete the deletion of the replaced parallel revision state.

This does not block approval. Approved.

Disclosure: Codex assisted with read-only source tracing and evidence organization across this multi-round review. I verified the findings against the current head and own this approval decision.

中文对照

最终 rebase 和 CLI snapshot 适配完成后，我重新审查了当前 head 962878b6。

此前批准的架构仍然成立：manager 发布唯一一份不可变的 callable snapshot；Runtime、Runtime Host、Desktop 和 CLI 都从同一个权威来源消费 binding 与 revision；managed-network 授权仍紧邻 provider dispatch；持久化的 Tool 状态只包含模型的原始参数；同时保留了 main 中稳定的 provider/reconnect 所有权。

我没有发现 P0–P2 问题。当前 head 可以干净合并，GitHub 上的 13 项检查均已通过。

仍有一项非阻塞清理：

• P3 — 删除已被替代的 revision 字段。 packages/mcp/src/index.ts 仍声明了 toolSnapshotRevisionValue，但旧 accessor 和 mutation 路径均已删除，该字段也从未被使用。删除它可以彻底移除已被替代的平行 revision 状态。

这不影响批准。Approved。

披露：Codex 在这次多轮审查中协助进行了只读源码追踪和证据整理。我核对了当前 head 上的 findings，并对本次批准决定负责。