Skip to content

fix(desktop): publish MCP tools through native capabilities - #4490

Merged
me2seeks merged 14 commits into
apache:mainfrom
me2seeks:fix/desktop-mcp-capability-schema
Sep 4, 2026
Merged

fix(desktop): publish MCP tools through native capabilities#4490
me2seeks merged 14 commits into
apache:mainfrom
me2seeks:fix/desktop-mcp-capability-schema

Conversation

@me2seeks

@me2seeks me2seeks commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Desktop can connect to an MCP server while still failing to publish its tools to Runtime Host: MCP tools carry an AI SDK JSON Schema wrapper, but the native-capability adapter previously accepted Zod only. The connection then appears healthy while the model cannot see or call the tool.

This PR covers the full Desktop MCP capability path:

  • Publication: the single Desktop native-capability adapter publishes declared JSON Schema (Zod-backed Desktop tools retain the existing path). Tools whose schema uses keywords outside the Client Capability allowlist (Desktop: widen Client Capability tool-schema allowlist so MCP tools using prefixItems/not/patternProperties/contentEncoding/if-then-else/deprecated are offered #4614, e.g. prefixItems produced by a pydantic tuple[...]), whose description exceeds 8192 chars, or whose schema exceeds 32 KiB are skipped with a per-tool diagnostic — never offered.
  • Admission (feat(desktop): authorize client capabilities in managed sessions #4143 follow-up): desktop_mcp tools go through Host managed admission. Managed ask mode produces the existing desktop_mcp / mcp_tool Session Grant (enhancement(runtime): allow Auto mode to use tools currently restricted to Full Access on macOS #4012); Host-level regressions cover accept → approval → admit → execute, per-tool grant reuse, and the deny path.
  • Manifest budgets: dynamic groups are chunked past the 64-tool single-offer limit and shed trailing tools under the total-tools / offer-count / manifest-byte budgets with a visible diagnostic, so an oversized MCP set can no longer fail the whole Desktop registration (Browser / Settings / Rive stay intact and fail loudly if they ever overflow).
  • Identity: one dynamic offer per MCP server; descriptors carry the normalized real serverId and bare tool name, so model-facing names stay mcp__<server>__<tool> and approval prompts ("Allow create_pull_request from github") and Session Grant scopes keep their designed per-server granularity. A server change re-prompts only that server's tools.

Closes #4591, closes #4652.

Relationship to #4592: this PR is the superset (it also lands #4652 and the #4143 admission follow-up); #4592's two useful ideas — the per-tool omission warning and dropping the dead validate branch — are folded in here.

Verification

  • npm run build
  • node --test on the affected suites: packages/runtime-host client-capability coordinator / protocol / admission-integration / invocation-broker / channel (47 passed, including the new Host-level ask regressions), packages/runtime mcp-tools (10), packages/cli capability provider command + remote publication (19), apps/desktop native-capabilities / mcp-runtime-e2e / desktop-candidate / browser-tools / session-execution-ipc (83)
  • Protocol epoch guard simulated locally against the merge result: declared compatible extension at the current epoch

The end-to-end test connects the real stdio MCP fixture, builds its tools, publishes them through the Desktop capability provider under their real server identity, validates the protocol payload, and invokes the offered tool. No screenshot applies: this repairs capability publication behind the existing MCP settings UI.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: AI coding assistants assisted investigation, implementation, tests, and review; the commits include the required trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — connected Desktop MCP tools are now published to and invocable through Runtime Host, and managed ask mode requires a per-tool Session Grant for them.
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 1, 2026
@me2seeks
me2seeks force-pushed the fix/desktop-mcp-capability-schema branch 4 times, most recently from 37d94f6 to 0c2aae4 Compare September 3, 2026 10:56

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

English

I found two issues that should be resolved or explicitly scoped before this is treated as a complete Runtime Host fix.

  1. prepareCapabilityGroups() validates MCP tools individually, but publishes every surviving MCP tool in one desktop_mcp offer. The protocol caps an offer at 64 tools, the full manifest at 256 tools and 56 KiB. With 65 otherwise valid small tools, the complete capability replacement is rejected with Invalid Client Capability offer tools, so MCP can also prevent Browser, Settings, Rive, and the other Desktop capabilities from registering.

    The existing CLI MCP provider already sorts and chunks tools before validating the complete replacement. Desktop needs equivalent aggregate handling, including a defined policy for the total-count/byte ceiling that preserves the fixed Desktop capabilities and reports why MCP publication was omitted.

  2. The new E2E test calls provider.call() directly, so it does not cross Runtime Host admission. In the default managed ask mode, managedClientCapabilityGrantTarget() only recognizes Desktop Settings and Browser; a published desktop_mcp call still fails with Client Capability has no managed admission policy.

    This does not require moving MCP ownership into Host. If this PR is intentionally limited to publication and Full Access invocation, say that explicitly and narrow the behavior claim/test accordingly. If it is meant to make Desktop MCP invocable on the default path, wire the existing desktop_mcp / mcp_tool grant into the coordinator and add a Host-level invocation test. #4012 tracks that work, and #4143 explicitly left Desktop MCP admission to a follow-up.

I did not find a useful simplification in the adapter itself: the prepared tool/descriptor pair, per-group omission policy, and shared descriptor decoder each prevent a real consistency problem.

中文

这次改动的方向没问题,但现在还不能算“Desktop MCP 已经完整走通 Runtime Host”,有两个坑需要处理清楚。

  1. 现在只检查了每个 MCP 工具自己是否合法,最后却把剩下的工具一股脑塞进同一个 desktop_mcp offer。协议规定单个 offer 最多 64 个工具,整份清单最多 256 个工具、56 KiB。我用 65 个很小的合法工具复现后,整次 capability replacement 会直接报 Invalid Client Capability offer tools。倒霉的不只是 MCP:Browser、Settings、Rive 等同一批注册的 Desktop 能力也会一起发布失败。

    CLI 那套 MCP provider 已经会先稳定排序、分组,再校验完整清单。Desktop 至少也得把总量限制管起来;超过总工具数或字节上限时,要保证固定的 Desktop 能力还能正常注册,同时明确告诉用户为什么这次没有发布 MCP,不能让一组动态工具把整包能力拖死。

  2. 新的 E2E 实际上是直接调用 provider.call(),没有经过 Runtime Host 的准入逻辑。默认 managed ask 模式里,Host 目前只给 Desktop Settings 和 Browser 配了策略;desktop_mcp 就算成功出现在工具列表里,调用时仍然会报 Client Capability has no managed admission policy

    这里不需要把 MCP 的执行权搬到 Host。如果这个 PR 只打算修“能发布、Full Access 下能调用”,就把 PR 的行为说明和测试范围写准确;如果目标是默认模式也能用,那就把现成的 desktop_mcp / mcp_tool grant 接到 coordinator,并补一条真正穿过 Host 的调用测试。#4012 正在跟踪这件事,#4143 也明确说 Desktop MCP admission 留到后续。

简化审计没有发现值得删的结构:prepared tool/descriptor、按 group 决定是否跳过不兼容工具,以及共享 decoder,都在避免真实的一致性问题。

@me2seeks
me2seeks force-pushed the fix/desktop-mcp-capability-schema branch from 0c2aae4 to d5f1fb8 Compare September 3, 2026 14:19
…ssion Grant

managedClientCapabilityGrantTarget() only recognized Desktop Settings and
Browser, so a managed ask-mode call to a desktop_mcp tool failed before
admission with 'no managed admission policy'. Recognize the Desktop-owned
desktop_mcp server identity and produce the existing desktop_mcp/mcp_tool
grant target, reusing the generic approval, grant persistence, and prompt
plumbing. The policy keys on the descriptor serverId so chunked offers stay
admissible. Adds Host-level ask regressions covering
accept-approval-admit-execute, per-tool Session Grant reuse, and the deny
path (apache#4490).
A desktop_mcp group beyond the 64-tool single-offer limit failed the full
manifest decode, taking Browser, Settings, and Rive registration down with
it. Chunk dynamic groups into stable per-offer ids while keeping the group's
server identity, shed trailing dynamic tools when the total tool, offer, or
byte budgets are exceeded, keep fixed groups loud when they overflow, decode
the manifest authoritatively before it is sent, and report omissions through
a visible diagnostic (apache#4490).

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The direction is right and the wiring is correct: this attaches to the existing seams (the shared protocol decoder, the already-defined desktop_mcp capability and mcp_tool grant scope, which core, storage and the UI have supported since #4143 but nothing published) rather than adding a Desktop-specific MCP path. The two points from the earlier review are addressed at this head, so I did not re-litigate them. CI is green. Three P2s below, none of which block the design.

P2 (cross-cutting): the model-facing tool name gets double-prefixed. buildMcpTools already produces mcp__<server>__<tool>, the Desktop group pins every descriptor's serverId to the constant desktop_mcp, and the coordinator prefixes again at client-capability-coordinator.ts:1441/:619. Running the real mcpProxyToolName algorithm: github/create_pull_request becomes mcp__desktop_mcp__mcp__github__create_pull_request, and playwright/browser_generate_playwright_test overflows 64 chars and becomes mcp__desktop_mcp__mcp__playwright__browser_generate___9cccd640a3. The approval prompt has the same collapse: conversation-copy.ts:674 renders Allow ${toolName} from ${serverId}, and serverId is always desktop_mcp, so the real MCP server only survives inside the tool name. The mcp_tool scope was shaped as {serverId, toolName} for per-server granularity, and it now carries a constant. This was unreachable on main because no MCP tool was ever published, so this PR is what exposes it. Giving each MCP server its own offerId and serverId in the Desktop group (publishing the bare MCP tool name) would fix the naming and restore the scope's designed granularity. If the single desktop_mcp identity is deliberate because the chunking logic depends on it, please say so in the description and I will treat the naming as accepted. The same point applies to #4592.

Housekeeping: the description links no issue. This closes #4591 and #4652, and it should say so. It would also help to state that tools hitting #4614's allowlist gap are skipped here, not offered.

Overlap with #4592, for ordering: #4592 (head 42b7433) touches three files that are all a subset of this PR's Desktop files, rewrites the same capabilityOffer / toolInputSchema / requireZodSchema combination, and solves the same #4591. They conflict textually as well: both change the same assertion in runtime-host-desktop-candidate.test.ts, to incompatible values, because #4592 deliberately stops treating an unrepresentable schema as a construction failure while this PR keeps that for fixed groups. They are alternatives, not complements. This PR is the superset (it also lands #4652 and the #4143 admission follow-up), so taking it as the base and folding in #4592's two useful ideas (the per-tool warning and dropping the dead validate branch) is the cheaper ordering. Merging #4592 first would leave this PR needing a full rewrite of the same function, not a rebase.

I found no P0 or P1. managedClientCapabilityGrantTarget still gates on registration.trustedProvider, the desktop_mcp branch's lack of a tool allowlist is appropriate for a dynamic tool set, and every tool still needs its own user approval and Session Grant outside bypass.

Comment thread apps/desktop/src/main/runtime-host-native-capabilities.ts Outdated
Comment thread apps/desktop/src/main/runtime-host-native-capabilities.ts
Comment thread apps/desktop/src/main/runtime-host-native-capabilities.ts Outdated
Comment thread apps/desktop/src/main/runtime-host-native-capabilities.ts Outdated
Review apache#4490: report every tool the decoder rejects through the shared
diagnostic channel instead of dropping it silently; drop the unreachable
validate branch in parseToolArguments (JSON-Schema tools have no client-side
validator); narrow the chunking comment to the scope and published names the
grant key actually pins; rename omitUnsupportedTools to dynamic with a
comment covering all three behaviors it gates.
Review apache#4490: the shared desktop_mcp server identity double-prefixed every
model-facing tool name and collapsed the mcp_tool grant scope's per-server
granularity to a constant. Publish one dynamic offer per MCP server with the
descriptor carrying the normalized real serverId and bare tool name, so the
model sees mcp__<server>__<tool> and approval prompts and Session Grants are
scoped per server. buildMcpToolsWithIdentities exports the source identity
from a single snapshot read; clientCapabilityEntityId moves to the protocol
package and is shared with the CLI provider; Host admission matches the
desktop_mcp offer prefix.
…ility-schema

Resolve packages/runtime/src/mcp-tools.ts: keep buildMcpToolsWithIdentities
and fold in main's requestInteraction plumbing (apache#4379).

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed current head 04fc935f7030604238b5e29efad12e942a780456 (OPEN, hosted test green). No P0–P3; nothing here blocks merging on technical grounds.

What was checked

MCP native-publish schema/descriptor boundaries, per-server offers against the 64/256/32/56 KiB budgets, dynamic-degradation diagnostics, host-managed admission with server/tool scopes, refuse/offline/unknown-result paths, and replace-vs-concurrent paths. Local verification: runtime-host coordinator 32/32, runtime MCP 10/10, desktop native-capabilities 21/21, MCP end-to-end 1/1, protocol tests 76/76, @maka/desktop build passes, git diff --check clean.

What I could not judge

The root-workspace full npm run build is still blocked by pre-existing UI type errors this PR does not touch (streaming/autoScroll/trailingAction and similar) — recorded here so it is not misattributed to this change either way. Remaining merge requirements are the normal independent-review/branch-protection gates.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @未开智选手 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移、以及 exact-head 的 CI 状态。当前 head 是 04fc935,test 通过。技术上无阻断问题,根目录全量构建的既有 UI 类型错误与本 PR 无关。合并仍需走正常独立复核与分支保护门禁。

@github-actions github-actions Bot added effort/XL Under 2500 readable lines and removed effort/M Under 500 readable lines labels Sep 3, 2026

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed exact head 04fc935f7030604238b5e29efad12e942a780456.

The two previously reported failures are closed. The default managed ask path now admits trusted Desktop MCP tools through a tool-scoped Session Grant, and large MCP catalogs are decoded per tool, split within the protocol's 64-tool offer limit, and deterministically degraded without removing healthy fixed capabilities. The published descriptor and dispatch path retain the real MCP server/tool identity from one immutable snapshot.

I found no remaining P0–P3 issue in this change. The exact-head hosted check is green. Focused exact-head tests passed 102/102 across Desktop, Runtime, Runtime Host, and the CLI provider. I also built and tested the current-main merge result because mcp-tools.ts changed on both sides; its relevant builds passed and the combined focused set passed 98/98.


Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.

@me2seeks
me2seeks merged commit 9e1ad2e into apache:main Sep 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

3 participants