You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2026/09/02 21:47:53 UTC
[PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
liuxiaocs7 opened a new pull request, #4592:
URL: https://github.com/apache/maka/pull/4592
## Summary
MCP tools configured on the Desktop app (including servers added in the TUI, which share the workspace's `mcp.json`) never load: the Runtime Host logs `MCP startup failed: ... Desktop native capability tool has an invalid schema` and `MCP capability refresh failed` on every change, and no MCP tool is offered to sessions.
`buildMcpTools()` sets each MCP tool's `parameters` to `jsonSchema(descriptor.inputSchema)` — an AI SDK `Schema`, not a Zod type. The Desktop native-capability provider offered and validated tools through `requireZodSchema` (`tool.parameters instanceof z.ZodType`), so it threw on the first MCP tool. The protocol's receiving side (`client-capability-coordinator`) already rebuilds the same tools with `buildMcpTools` and accepts `jsonSchema()`, so the two halves of the boundary disagreed on the allowed `parameters` shape; this aligns the offering side.
- `toolInputSchema` now reads the JSON Schema directly from an AI SDK `Schema` when `parameters` is not Zod, cloning it before deleting `$schema`/freezing so the MCP descriptor's shared `inputSchema` is never mutated.
- Call-time validation (`invokeNativeTool`) goes through a new `parseToolArguments` helper that mirrors the runtime's `validateDeclaredToolArgs` precedence: Zod `parseAsync`, AI SDK `validate`, or pass-through for JSON-schema-only MCP tools (which carry no client-side validator). Zod-based tools (browser, computer-use, settings, rive) are unchanged.
- Offers are built synchronously and frozen, so only synchronously-available schemas are supported at offer time (all current tools qualify: Zod, or MCP's plain-object `jsonSchema()`); an async/thenable schema throws a clear error rather than failing silently.
Fixes #4591
## Verification
- `npm --workspace @maka/desktop run build:main` (tsc) — passes, no type errors.
- `node --test apps/desktop/dist/main/__tests__/runtime-host-native-capabilities.test.js` — **15/15 pass**, including the new test `offers and dispatches MCP tools whose parameters are JSON Schema, not Zod`, which runs real `buildMcpTools()` output through the provider and fails without this change (it throws the "invalid schema" error).
- `biome format .` — 1867 files, no fixes needed; `biome lint` on the changed files — clean.
Not run: the full Electron `npm run dev` end-to-end launch (headless environment). The added test exercises the same offer + call code paths that fail in the reported log, including protocol encoding of the offer via `decodeClientCapabilityReplaceInput`.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Claude Code diagnosed the root cause, implemented the fix and the added test, and drafted this PR. The authoring commit carries a `Generated-by: Claude Code` trailer.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5528787948
Severities for the review above, which I should have included with it.
**P1 — the aggregate limits still take every Desktop capability down.** Normal supported operation, and a common configuration rather than an edge: `CLIENT_CAPABILITY_MAX_TOOLS_PER_OFFER` is 64 while `runtime-host-boot.ts:995-1003` puts every MCP tool from every server into one `desktop_mcp` offer with no cap, so 65 perfectly representable tools fail the whole frame in `replace()` and Browser, Computer Use, Client settings and Rive go with them, leaving one console line. Same at 256 total and at a 56 KiB manifest. The defect is pre-existing, and per-tool probing cannot see an aggregate by construction, so this is not a hole in your isolation. What it does mean is that `Fixes #4591` closes an issue still one MCP server away from reproducing. Either decode the assembled frame once and shed or chunk on failure, as `packages/cli/src/mcp-capability-provider.ts:38-99` already does, or say `Refs #4591` and leave the issue open.
**P2 — `publishes every production Desktop-owned tool schema through the protocol` no longer proves anything.** Normal path, developer-facing: it asserts only `doesNotThrow`, and now that an unrepresentable tool is dropped rather than thrown, it fails only if every offer disappears. A future Desktop-owned tool with a bad schema vanishes with green CI. Asserting the expected tool names per offer is four lines.
**P2 — a skipped tool is invisible to the user and contradicted by the UI.** Normal path: `mcp-page.tsx:917-924` renders `status.tools` from the MCP manager, which knows nothing about the probe, so Settings lists a tool as live while the model does not have it, and the only diagnostic is a main-process console line. Still a much better trade than today's total outage, so a follow-up issue is a fine resolution; `McpServerStatus` already carries `error` and `stderrTail`.
**P3 — `parseToolArguments`' comment claims a validator that does not exist.** No observable consequence, but it makes an unvalidated path read as validated.
**P3 — `capabilityMetadataValid` and its constants, about 45 lines, buy log wording only**, and its test asserts on `console.warn` text. Both go under the whole-frame check.
**P3 — the body still says 16/16**; head has 18.
**No finding** on the four points from the earlier round, all of which are genuinely fixed, nor on binding and offer consistency, nor on the per-tool probe as a second authority (it borrows the Host's own decoder, which is the right call), nor on its cost (256 probes measure under 7 ms).
The P1 is a judgement call rather than a defect you introduced: decide whether this PR carries the aggregate case or whether the issue stays open. Everything else is small.
<details><summary>简体中文</summary>
上面那份 review 的分级，应该跟着一起给的。
**P1 —— 聚合上限仍然会把桌面端所有能力一起带走。** 正常支持路径，而且是常见配置而不是边角：`CLIENT_CAPABILITY_MAX_TOOLS_PER_OFFER` 是 64，而 `runtime-host-boot.ts:995-1003` 把所有 server 的所有 MCP 工具塞进单个 `desktop_mcp` offer 且没有上限，所以 65 个完全可表达的工具照样在 `replace()` 里整帧被拒，Browser、Computer Use、Client settings、Rive 一起消失，只留一行 console。总数 256 和 56 KiB manifest 同理。缺陷本身先于这个 PR，而逐工具探针在结构上就看不见聚合量，所以这不是你隔离做得不够。它的实际含义是 `Fixes #4591` 会关掉一个再接一个 MCP server 就能复现的 issue。要么对组装好的帧解码一次、失败就分块或丢弃（`packages/cli/src/mcp-capability-provider.ts:38-99` 已经是这个写法），要么改成 `Refs #4591` 把 issue 留着。
**P2 —— `publishes every production Desktop-owned tool schema through the protocol` 已经证明不了任何事。** 正常路径，面向开发者：它只断言不抛，而现在不可表达的工具是被丢掉而不是抛出，所以它只在所有 offer 都消失时才红。将来某个 Desktop 自有工具 schema 坏掉，会在 CI 全绿的情况下消失。逐 offer 断言期望的工具名，四行。
**P2 —— 被跳过的工具对用户是隐形的，而且和界面自相矛盾。** 正常路径：`mcp-page.tsx:917-924` 直接渲染 MCP manager 的 `status.tools`，它不知道探针的存在，所以设置里把一个工具列为在线，模型手上却没有，唯一的诊断是一行主进程日志。这仍然远好于现在的整体停摆，所以开个 follow-up issue 就够；`McpServerStatus` 已经带了 `error` 和 `stderrTail`。
**P3 —— `parseToolArguments` 的注释声称了一个不存在的校验器。** 没有可观察后果，但它让一条未校验的路径读起来像已校验。
**P3 —— `capabilityMetadataValid` 和那几个常量，约 45 行，只买到日志措辞**，对应测试断言的是 `console.warn` 的字符串。整帧校验一落地两者一起走。
**P3 —— 正文还写着 16/16**，head 上是 18。
**无发现**：上一轮那四点确实都真修好了；bindings 与 offers 的一致性；逐工具探针是不是第二个校验权威（它借用的是 Host 自己的解码器，这个选择是对的）；以及它的成本（256 次探针实测不到 7 ms）。
那条 P1 与其说是你引入的缺陷，不如说是一个取舍：这个 PR 要不要把聚合这一半也扛下来，还是让 issue 留着。其余都很小。
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5535062267
Closing — superseded by #4490 (merged 2026-09-04), which its author scoped as an explicit superset of this PR: the same `jsonSchema()` publication fix, plus the #4652 aggregate-limit handling (offer chunking + manifest-budget shedding) and the #4143 managed-admission follow-up, with this PR's two useful ideas — the per-tool omission diagnostic and dropping the dead `validate` branch — folded in. #4591 and #4652 are both closed there, and main already carries the complete fix.
Thanks @Astro-Han for the thorough multi-round review; the per-tool isolation and the Refs-vs-Fixes analysis were the right calls and match where #4490 drew the boundary. Nothing left to land here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 closed pull request #4592: fix(desktop): accept jsonSchema() tool params in native capability offers
URL: https://github.com/apache/maka/pull/4592
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 closed pull request #4592: fix(desktop): accept jsonSchema() tool params in native capability offers
URL: https://github.com/apache/maka/pull/4592
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5531685716
Pushed e4ab066a — took the P1 as a boundary decision and the rest as fixes.
**P1 — `Refs #4591`, issue stays open.** Kept the per-tool boundary rather than pulling the aggregate case into this PR. The body now reads `Refs #4591` and states plainly that >64 tools/offer, 256 total and the 56 KiB manifest can still fail a large config's whole registration, with that half tracked in #4652. So it no longer claims to close the issue.
**P2 — the schema test proves something again.** `publishes every production Desktop-owned tool schema through the protocol` now asserts the offered tool names per offer (`desktop_settings` → the two client-settings tools, `desktop_rive` → `RiveWorkflow`), so a future Desktop-owned tool with an unrepresentable schema fails the assertion instead of vanishing green. Kept the `doesNotThrow` alongside it.
**P3 — dropped `capabilityMetadataValid`.** Removed it, the `OfferMetadata` type and the two probe constants (~45 lines). Each tool is now probed inside its group's real offer metadata, as the first round did, so a group whose metadata is invalid still drops all its tools and is shed whole — N warnings instead of one, on a path that cannot reach production (every production `offerId` is a literal). Its test now asserts the bad-metadata group is shed while the rest register, instead of asserting `console.warn` text.
**P3 — `parseToolArguments` comment corrected.** It no longer claims the Host validates MCP arguments; it says the owning MCP server is the validator (the Host rebuilds these with `buildMcpTools`, no client-side validator), as for the TUI.
**P3 — body test count** now reads 18/18.
**P2 — Settings visibility of a skipped tool** is real: `mcp-page.tsx` renders the manager's `status.tools`, which doesn't know about the probe, so a skipped tool reads as live. That's a separate UI seam (`McpServerStatus` already carries `error`/`stderrTail`); it deserves a dedicated follow-up rather than widening this PR — happy to open one.
Local: 18/18 native-capability tests pass; `build:main` (tsc) and Biome lint on the two changed files are clean.
<details><summary>简体中文</summary>
已推送 e4ab066a —— P1 当作边界取舍，其余当作修复处理。
**P1 —— 改为 `Refs #4591`，issue 保留。** 保持逐工具的边界，没有把聚合那半塞进本 PR。正文已改为 `Refs #4591`，并明确写出 >64 工具/offer、总数 256、56 KiB manifest 仍会让大配置整帧失败，这半由 #4652 跟踪，所以不再声称关闭该 issue。
**P2 —— 那条测试重新有了意义。** `publishes every production Desktop-owned tool schema through the protocol` 现在逐 offer 断言工具名（`desktop_settings` → 两个 client-settings 工具，`desktop_rive` → `RiveWorkflow`），将来某个 Desktop 自有工具 schema 不可表达会让断言失败，而不是 CI 全绿地消失；`doesNotThrow` 也保留。
**P3 —— 删掉了 `capabilityMetadataValid`。** 连同 `OfferMetadata` 类型和两个探针常量一起删（约 45 行）。每个工具现在在其分组的真实 offer 元数据里探针，跟第一轮一样；元数据非法的分组仍会把所有工具丢掉、整组不 advertise —— N 条 warning 而不是一条，而这条路径生产不可达（生产 `offerId` 都是字面量）。对应测试改为断言坏元数据分组被丢、其余照常注册，不再断言 `console.warn` 文本。
**P3 —— `parseToolArguments` 注释已更正。** 不再声称 Host 校验 MCP 参数；改为说由拥有该工具的 MCP server 校验（Host 用 `buildMcpTools` 重建，没有客户端校验器），和 TUI 一致。
**P3 —— 正文测试数**已改为 18/18。
**P2 —— 被跳过工具在设置里的可见性**确实存在：`mcp-page.tsx` 渲染 manager 的 `status.tools`，它不知道探针，所以被跳过的工具读起来是在线的。这是另一处 UI 接缝（`McpServerStatus` 已带 `error`/`stderrTail`），更适合单开 follow-up 而不是扩大本 PR —— 我可以开一个。
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5531895512
CI (`test`) failed on a single unrelated e2e flake — `e2e/partial-history-notice.spec.ts` › *quiet reading-column control with neutral rail ticks*, a `toBeVisible()` timeout waiting for `[data-turn-id="turn-partial-history-8"]` (104/109 passed, 4 skipped). That spec is renderer-only; this commit (`e4ab066a`) changes only `apps/desktop/src/main/runtime-host-native-capabilities.ts` and its test — main-process native-capability offering, nothing on the partial-history path — and the parent commit `42b7433` was green. Re-triggering CI via reopen (no admin rights to re-run the job directly).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5531900162
<!-- maka-lifecycle:reopened -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5520689495
Thanks for the careful review — reproducing the whole-frame blast radius and running real `buildMcpTools()` output through the allowlist was exactly the right thing to check. Pushed 8cb1f53f6 addressing all of it.
**Isolation (main comment + inline on `toolInputSchema`).** `capabilityOffer` now builds each tool's descriptor on its own and validates it through the same `decodeClientCapabilityReplaceInput` the Runtime Host runs, inside a per-tool try/catch. An unrepresentable tool is logged and skipped; the rest of that capability and every other capability still register. This deliberately covers the schema-keyword rejections too, not just the Zod throw: an MCP tool using `prefixItems` (or `not`, `patternProperties`, `contentEncoding`, `if`/`then`/`else`, `deprecated`) now costs one tool instead of the whole app. New test `drops one unrepresentable tool instead of failing every Desktop capability` pins this with a real `buildMcpTools()` `prefixItems` schema — it fails without the isolation (the bad tool survives and the frame throws).
**Widening the allowlist** is left as a separate change, as you suggested — it touches the protocol package and wants its own review. Happy to open the follow-up.
**`parseToolArguments` `validate` branch.** Removed — you're right there's no producer (`buildMcpTools` calls `jsonSchema()` with one argument, so `validate` is always undefined). Dropped the branch, the thenable guard in `aiSchemaJson`, and the `NativeToolValidation` type; call-time parsing is now Zod `parseAsync`, else JSON-schema pass-through, else throw. The thenable case falls through to `toolInputSchema`'s object-shape check as you noted.
**`cloneNativeToolJsonSchema` comment + shared error string.** Corrected the comment: MCP `inputSchema` is already `$schema`-stripped and deep-frozen upstream (`packages/mcp`), so the copy is defensive, not load-bearing. The call path now throws a distinct message (`... cannot parse call arguments`) so its logs stay distinguishable from the offer path.
**PR body** narrowed to say what's actually true now: both causes are fixed, unrepresentable-schema tools are skipped rather than fatal, and allowlist-widening is called out as follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4592:
URL: https://github.com/apache/maka/pull/4592#issuecomment-5524649847
Follow-up review surfaced two more issues in the isolation I added; pushed 42b743358:
- **Bindings tracked the raw groups, not the advertised offers.** A tool dropped from its offer (unrepresentable schema) was still dispatchable via `provider.call()` even though it was never advertised. Bindings are now built from the resolved offers, so the provider only dispatches a tool it advertised. Test added: a dropped tool now rejects with `not offered`.
- **The per-tool probe also validated group metadata**, so a misconfigured group (bad `offerId`/label) was reported as every tool having an unrepresentable schema. Offer-level metadata is now validated once, separately, and each tool is probed against constant known-valid metadata, so group-level and tool-level failures are diagnosed distinctly. Test added.
One thing this PR still does **not** cover: the protocol's *aggregate* limits (>64 tools/offer, 256 total, 56 KiB manifest). Per-tool validation can't see them, so a large MCP config (e.g. a server exposing 65 valid tools — MCP discovery allows up to 1000) still fails the whole registration in `replace()` and drops every capability. Splitting/trimming the `desktop_mcp` offer needs a bit of design (offer-splitting + deterministic budget trimming + a final full-manifest validation), so I've filed it as a dedicated follow-up: #4652. Happy to fold it in here instead if you'd prefer it in one PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4592:
URL: https://github.com/apache/maka/pull/4592#discussion_r3920743027
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -448,13 +447,17 @@ function capabilityOffer(
}
function toolInputSchema(tool: MakaTool): Record<string, unknown> {
- const schema = toJSONSchema(requireZodSchema(tool), {
- io: "input",
- target: "draft-07",
- unrepresentable: "any",
- cycles: "ref",
- reused: "inline",
- });
+ const parameters = tool.parameters;
+ const schema: Record<string, unknown> =
+ parameters instanceof z.ZodType
Review Comment:
This builds the whole group in one pass, and `replace()` sends the frame as a unit, so any tool that throws here removes Browser, Computer Use, Client settings and Rive as well. A try/catch per tool that skips and warns keeps the blast radius at one tool, and it is what makes the schema-keyword rejections in the main comment survivable rather than another total outage.
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -464,13 +467,75 @@ function toolInputSchema(tool: MakaTool): Record<string, unknown> {
return Object.freeze(schema);
}
-function requireZodSchema(tool: MakaTool): z.ZodType {
- if (!(tool.parameters instanceof z.ZodType)) {
+function cloneNativeToolJsonSchema(tool: MakaTool): Record<string, unknown> {
Review Comment:
The comment claims more than the clone does: `packages/mcp/src/index.ts:2301` already strips `$schema` and `$comment` at every level and `:2076` deep-freezes the bound tool, so for the MCP path the delete is a no-op and the freeze re-freezes. The clone is cheap and harmless, but the reason should match. Separately, offer-time and call-time now throw the same string, which is the one quoted in #4591; a different message on the call path would keep the logs distinguishable.
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -464,13 +467,75 @@ function toolInputSchema(tool: MakaTool): Record<string, unknown> {
return Object.freeze(schema);
}
-function requireZodSchema(tool: MakaTool): z.ZodType {
- if (!(tool.parameters instanceof z.ZodType)) {
+function cloneNativeToolJsonSchema(tool: MakaTool): Record<string, unknown> {
+ const json = aiSchemaJson(tool.parameters);
+ if (!json) {
throw new Error(
`Desktop native capability tool has an invalid schema: ${tool.name}`,
);
}
- return tool.parameters;
+ // Shallow clone so deleting $schema / freezing the offer never mutates the
+ // MCP descriptor's shared inputSchema object.
+ return { ...json };
+}
+
+/**
+ * Read the JSON Schema from an AI SDK `Schema` (e.g. an MCP tool built through
+ * `jsonSchema()`), when it is synchronously available. Desktop offers are built
+ * eagerly and frozen, so an async (thenable) schema cannot be resolved here.
+ */
+function aiSchemaJson(
+ parameters: unknown,
+): Record<string, unknown> | undefined {
+ if (!parameters || typeof parameters !== "object") return undefined;
+ const json = (parameters as { jsonSchema?: unknown }).jsonSchema;
+ if (
+ json &&
+ typeof json === "object" &&
+ typeof (json as { then?: unknown }).then !== "function"
+ ) {
+ return json as Record<string, unknown>;
+ }
+ return undefined;
+}
Review Comment:
No producer reaches this branch. The only non-Zod source is `mcp-tools.ts:117`, which calls `jsonSchema()` with one argument, so `validate` is always undefined. Without it and the thenable guard this reduces to Zod `parseAsync`, else pass through when we have a JSON Schema, else throw, and `NativeToolValidation` goes too. The thenable case is already caught downstream by the `type !== 'object'` check, with a different message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): accept jsonSchema() tool params in native capability offers [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4592:
URL: https://github.com/apache/maka/pull/4592#discussion_r3926552666
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -114,10 +141,11 @@ export function createDesktopNativeCapabilityProvider(
): DesktopNativeCapabilityProvider {
const groups = capabilityGroups(input);
const hostPathAccess = providerOptions.hostPathAccess ?? "cwd";
- const offers = Object.freeze(
- groups.map((group) => capabilityOffer(group, hostPathAccess)),
- );
- const bindings = indexBindings(groups);
+ const resolved = groups
+ .map((group) => capabilityOffer(group, hostPathAccess))
+ .filter((entry): entry is ResolvedCapability => entry !== undefined);
+ const offers = Object.freeze(resolved.map((entry) => entry.offer));
Review Comment:
**P1, and the one thing that decides whether `Fixes #4591` is honest.** Per-tool probing cannot see the decoder's aggregate checks, and this is where they would be visible. `CLIENT_CAPABILITY_MAX_TOOLS_PER_OFFER` is 64 (`client-capability.ts:79`), `CLIENT_CAPABILITY_MAX_TOOLS` is 256, and the manifest cap is 56 KiB, all enforced only when `replace()` decodes the whole frame. `runtime-host-boot.ts:995-1003` puts every MCP tool from every server into the single `desktop_mcp` offer with no cap, so 65 tools whose schemas are all perfectly representable still fail the whole frame and take Browser, Computer Use, Client settings and Rive with them, leaving one console line. GitHub's own MCP server exposes around 70 tools by itself and discovery allows up to 1000, so this is a common configuration rather than an edge, and every tool passes its own probe so the new machinery reports nothing.
The defect predates this PR, so this is not a hole in your isolation. It does mean merging as written closes an issue that is still one MCP server away from reproducing.
The repo already has the end state: `packages/cli/src/mcp-capability-provider.ts:38-99` sorts tools by `(serverId, name)`, chunks into offers of at most `CLIENT_CAPABILITY_MAX_TOOLS_PER_OFFER`, and validates the whole assembled frame once before serving it. Desktop cannot reuse it verbatim, since it dispatches through `tool.impl` and the CLI through `manager.callTool`, but the shape transfers: assemble, decode the frame once here, chunk or shed on failure, serve the canonical result. That covers the aggregates and deletes `capabilityMetadataValid` on the way through.
If you would rather keep the boundary where it is, that is defensible, but then this should say `Refs #4591` and the issue should stay open.
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -464,29 +590,73 @@ function toolInputSchema(tool: MakaTool): Record<string, unknown> {
return Object.freeze(schema);
}
-function requireZodSchema(tool: MakaTool): z.ZodType {
- if (!(tool.parameters instanceof z.ZodType)) {
+function cloneNativeToolJsonSchema(tool: MakaTool): Record<string, unknown> {
+ const json = aiSchemaJson(tool.parameters);
+ if (!json) {
throw new Error(
`Desktop native capability tool has an invalid schema: ${tool.name}`,
);
}
- return tool.parameters;
+ // Copy defensively before the offer deletes `$schema` and freezes the result,
+ // so we never mutate the source object. Today the only non-Zod source is an
+ // MCP tool, whose `inputSchema` is already `$schema`-stripped and deep-frozen
+ // upstream (`packages/mcp/src/index.ts`), so this copy is belt-and-suspenders
+ // rather than load-bearing — but Desktop must not depend on that invariant.
+ return { ...json };
+}
+
+/**
+ * Read the JSON Schema from an AI SDK `Schema` (e.g. an MCP tool built through
+ * `jsonSchema()`), when it is synchronously available. Desktop offers are built
+ * eagerly and frozen; an async (thenable) schema is not resolved here and is
+ * rejected downstream by `toolInputSchema`'s object-shape check.
+ */
+function aiSchemaJson(
+ parameters: unknown,
+): Record<string, unknown> | undefined {
+ if (!parameters || typeof parameters !== "object") return undefined;
+ const json = (parameters as { jsonSchema?: unknown }).jsonSchema;
+ if (json && typeof json === "object") {
+ return json as Record<string, unknown>;
+ }
+ return undefined;
+}
+
+/**
+ * Coerce/validate incoming call arguments against a tool's declared parameters,
+ * mirroring the runtime's `validateDeclaredToolArgs` precedence. Zod schemas
+ * parse (applying defaults/transforms); JSON-schema-only tools (MCP) carry no
Review Comment:
**P3** This comment says JSON-schema-only arguments "are validated by the receiving Runtime Host against the same schema". They are not: the Host rebuilds these with `buildMcpTools` (`client-capability-coordinator.ts:600`), which calls `jsonSchema(descriptor.inputSchema)` with no `validate`, and `validateDeclaredToolArgs` (`tool-runtime.ts:3021-3065`) returns silently when there is no `safeParse`, `validate` or `~standard`.
No observable consequence, the MCP server validates its own input and this matches the TUI, but the comment makes an unvalidated path read as a validated one, which is the kind of claim a later reader builds on. Saying the MCP server is the validator would be true.
##########
apps/desktop/src/main/runtime-host-native-capabilities.ts:
##########
@@ -422,39 +449,138 @@ function abortInvocations(
function capabilityOffer(
group: DesktopCapabilityGroup,
hostPathAccess: ClientCapabilityHostPathAccess,
-): ClientCapabilityOffer {
- return Object.freeze({
+): ResolvedCapability | undefined {
+ const base: OfferMetadata = {
offerId: group.offerId,
version: CAPABILITY_VERSION,
affinity: "session",
hostPathAccess,
label: group.label,
description: group.description,
- tools: Object.freeze(
- group.tools.map((tool) =>
- Object.freeze({
- serverId: group.offerId,
- name: tool.name,
- description: tool.description,
- inputSchema: toolInputSchema(tool),
- ...(tool.activityKind ? { activityKind: tool.activityKind } : {}),
- ...(tool.displayName
- ? { annotations: Object.freeze({ title: tool.displayName }) }
- : {}),
- }),
- ),
- ),
+ };
+ // Validate the offer-level metadata once, apart from any tool, so a
+ // misconfigured group (a caller bug — bad offerId, over-long label, …) is
+ // reported as such instead of blaming every tool's schema.
+ if (!capabilityMetadataValid(group, base)) return undefined;
+
+ const surviving: {
+ readonly descriptor: ClientCapabilityToolDescriptor;
+ readonly tool: MakaTool;
+ }[] = [];
+ for (const tool of group.tools) {
+ const descriptor = offerableToolDescriptor(group, tool);
+ if (descriptor) surviving.push({ descriptor, tool });
+ }
+ if (surviving.length === 0) {
+ console.warn(
+ `[capabilities] Desktop capability ${group.offerId} has no representable tools; not advertising it`,
+ );
+ return undefined;
+ }
+ const offer = Object.freeze({
+ ...base,
+ tools: Object.freeze(surviving.map((entry) => entry.descriptor)),
});
+ return { offer, tools: surviving.map((entry) => entry.tool) };
+}
+
+/**
+ * Validate a capability's offer-level metadata (offerId, label, version, Host
+ * path access) in isolation, using one synthetic always-valid tool. Keeps a
+ * misconfigured group from being misreported as an unrepresentable tool schema
+ * on every tool it holds.
+ */
+function capabilityMetadataValid(
+ group: DesktopCapabilityGroup,
+ base: OfferMetadata,
+): boolean {
+ try {
+ decodeClientCapabilityReplaceInput({
+ registrationId: CAPABILITY_PROBE_REGISTRATION_ID,
+ offers: [
Review Comment:
**P3** This, `OfferMetadata` and the three probe constants are about 45 lines that buy log wording only. All four production `offerId`s are literals in `runtime-host-boot.ts`, so a bad `offerId` is a Desktop coding error that cannot reach production, and the test covering it asserts on `console.warn` text. Drop it and let the per-tool probe carry the real group metadata, as it did in the first round: a bad group is still dropped whole, you just get N warnings instead of one. Under the whole-frame check above, both this and its test go anyway.
##########
apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts:
##########
@@ -133,6 +135,287 @@ test('publishes the real Computer Use schema through the Client Capability proto
assert.equal(Array.isArray(coordinateSchema?.coordinate?.items), true);
});
+test('offers and dispatches MCP tools whose parameters are JSON Schema, not Zod', async () => {
+ let receivedArgs: unknown;
+ const mcpProvider: McpToolProvider = {
+ toolSnapshot: () => ({
+ revision: 1,
+ tools: [
+ {
+ descriptor: {
+ serverId: 'filesystem',
+ name: 'read_file',
+ description: 'Read a file',
+ inputSchema: {
+ type: 'object',
+ properties: { value: { type: 'string' } },
+ },
+ },
+ binding: 'binding-1' as McpToolBinding,
+ },
+ ],
+ }),
+ callTool: async (_binding, args) => {
+ receivedArgs = args;
+ return { content: [{ type: 'text', text: JSON.stringify(args) }] };
+ },
+ };
+ // Real production projection: parameters become jsonSchema(...), not Zod.
+ const mcpTools = buildMcpTools(mcpProvider);
+ const mcpTool = mcpTools[0];
+ assert.ok(mcpTool, 'expected buildMcpTools to project one tool');
+
+ const provider = createDesktopNativeCapabilityProvider({
+ browserTools: [],
+ resolveBrowserUrl: () => 'https://example.com/',
+ releaseBrowserSession() {},
+ computerUseTools: computerTools(),
+ releaseComputerUseSession() {},
+ additionalGroups: () => [
+ {
+ offerId: 'desktop_mcp',
+ label: 'MCP',
+ description: 'MCP tools',
+ tools: mcpTools,
+ },
+ ],
+ });
+
+ // Offer path: the JSON Schema is advertised verbatim and survives protocol encoding.
+ const offer = provider.offers().find((entry) => entry.offerId === 'desktop_mcp');
+ assert.ok(offer, 'expected a desktop_mcp offer');
+ const inputSchema = offer.tools[0]?.inputSchema;
+ assert.equal(inputSchema?.type, 'object');
+ assert.deepEqual(
+ Object.keys((inputSchema?.properties as object | undefined) ?? {}),
+ ['value'],
+ );
+ assert.doesNotThrow(() =>
+ decodeClientCapabilityReplaceInput({
+ registrationId: 'registration-1',
+ offers: provider.offers(),
+ }),
+ );
+
+ // Call path: args flow through to callTool without a Zod parser, and the MCP
+ // result is projected back over the protocol.
+ const result = await call(
+ provider,
+ capabilityFrame({
+ offerId: 'desktop_mcp',
+ serverId: 'desktop_mcp',
+ toolName: mcpTool.name,
+ arguments: { value: 'hi' },
+ }),
+ );
+ assert.deepEqual(receivedArgs, { value: 'hi' });
+ assert.deepEqual(result, {
+ content: [{ type: 'text', text: JSON.stringify({ value: 'hi' }) }],
+ });
+});
+
+test('drops one unrepresentable tool instead of failing every Desktop capability', () => {
+ const mcpProvider: McpToolProvider = {
+ toolSnapshot: () => ({
+ revision: 1,
+ tools: [
+ {
+ descriptor: {
+ serverId: 'filesystem',
+ name: 'read_file',
+ description: 'Read a file',
+ inputSchema: {
+ type: 'object',
+ properties: { value: { type: 'string' } },
+ },
+ },
+ binding: 'binding-read' as McpToolBinding,
+ },
+ {
+ descriptor: {
+ serverId: 'filesystem',
+ name: 'read_tuple',
+ // `prefixItems` (a pydantic `tuple[...]` produces it) is outside the
+ // Client Capability schema allowlist, so this tool cannot be offered.
+ description: 'Uses a JSON Schema keyword the protocol rejects',
+ inputSchema: {
+ type: 'object',
+ properties: {
+ pair: {
+ type: 'array',
+ prefixItems: [{ type: 'string' }, { type: 'number' }],
+ },
+ },
+ },
+ },
+ binding: 'binding-tuple' as McpToolBinding,
+ },
+ ],
+ }),
+ callTool: async () => ({ content: [] }),
+ };
+ const mcpTools = buildMcpTools(mcpProvider);
+ const survivingToolName = mcpTools[0]?.name;
+ const droppedToolName = mcpTools[1]?.name;
+ assert.ok(survivingToolName && droppedToolName);
+
+ const warnings: string[] = [];
+ const originalWarn = console.warn;
+ console.warn = (...args: unknown[]) => {
+ warnings.push(String(args[0]));
+ };
+ let provider: ReturnType<typeof createDesktopNativeCapabilityProvider>;
+ try {
+ provider = createDesktopNativeCapabilityProvider({
+ browserTools: [tool('browser_snapshot', z.object({}), async () => 'ok')],
+ resolveBrowserUrl: () => 'https://example.com/',
+ releaseBrowserSession() {},
+ computerUseTools: computerTools(),
+ releaseComputerUseSession() {},
+ additionalGroups: () => [
+ { offerId: 'desktop_mcp', label: 'MCP', description: 'MCP tools', tools: mcpTools },
+ ],
+ });
+ } finally {
+ console.warn = originalWarn;
+ }
+
+ // The unrepresentable tool is dropped, yet its server and every other Desktop
+ // capability still register — the #4591 outage took all of them down at once.
+ assert.deepEqual(
+ provider.offers().map((offer) => offer.offerId),
+ ['desktop_browser', 'desktop_mcp'],
+ );
+ const mcpOffer = provider.offers().find((offer) => offer.offerId === 'desktop_mcp');
+ assert.deepEqual(
+ mcpOffer?.tools.map((descriptor) => descriptor.name),
+ [survivingToolName],
+ );
+ assert.equal(warnings.some((line) => line.includes(droppedToolName)), true);
+
+ // The surviving frame still encodes cleanly over the protocol.
+ assert.doesNotThrow(() =>
+ decodeClientCapabilityReplaceInput({
+ registrationId: 'registration-1',
+ offers: provider.offers(),
+ }),
+ );
+});
+
+test('does not dispatch a tool that was dropped from its offer', async () => {
+ const mcpProvider: McpToolProvider = {
+ toolSnapshot: () => ({
+ revision: 1,
+ tools: [
+ {
+ descriptor: {
+ serverId: 'filesystem',
+ name: 'read_file',
+ description: 'Read a file',
+ inputSchema: { type: 'object', properties: { value: { type: 'string' } } },
+ },
+ binding: 'binding-ok' as McpToolBinding,
+ },
+ {
+ descriptor: {
+ serverId: 'filesystem',
+ name: 'read_tuple',
+ description: 'Uses a JSON Schema keyword the protocol rejects',
+ inputSchema: {
+ type: 'object',
+ properties: { pair: { type: 'array', prefixItems: [{ type: 'string' }] } },
+ },
+ },
+ binding: 'binding-bad' as McpToolBinding,
+ },
+ ],
+ }),
+ callTool: async () => ({ content: [{ type: 'text', text: 'should not run' }] }),
+ };
+ const mcpTools = buildMcpTools(mcpProvider);
+ const droppedName = mcpTools[1]?.name;
+ assert.ok(droppedName);
+
+ const originalWarn = console.warn;
+ console.warn = () => {};
+ let provider: ReturnType<typeof createDesktopNativeCapabilityProvider>;
+ try {
+ provider = createDesktopNativeCapabilityProvider({
+ browserTools: [],
+ resolveBrowserUrl: () => 'https://example.com/',
+ releaseBrowserSession() {},
+ computerUseTools: computerTools(),
+ releaseComputerUseSession() {},
+ additionalGroups: () => [
+ { offerId: 'desktop_mcp', label: 'MCP', description: 'MCP tools', tools: mcpTools },
+ ],
+ });
+ } finally {
+ console.warn = originalWarn;
+ }
+
+ const mcpOffer = provider.offers().find((offer) => offer.offerId === 'desktop_mcp');
+ assert.equal(mcpOffer?.tools.some((descriptor) => descriptor.name === droppedName), false);
+ // A tool that was never advertised must not be dispatchable, even though the
+ // MakaTool still exists in the source group — bindings track the advertised
+ // snapshot, not the raw group.
+ await assert.rejects(
+ () =>
+ call(
+ provider,
+ capabilityFrame({
+ offerId: 'desktop_mcp',
+ serverId: 'desktop_mcp',
+ toolName: droppedName,
+ arguments: { pair: ['x'] },
+ }),
+ ),
+ /not offered/u,
+ );
+});
+
+test('skips a capability with invalid metadata without blaming its tool schemas', () => {
+ const warnings: string[] = [];
+ const originalWarn = console.warn;
+ console.warn = (...args: unknown[]) => {
+ warnings.push(String(args[0]));
+ };
+ let provider: ReturnType<typeof createDesktopNativeCapabilityProvider>;
+ try {
+ provider = createDesktopNativeCapabilityProvider({
+ browserTools: [tool('browser_snapshot', z.object({}), async () => 'ok')],
+ resolveBrowserUrl: () => 'https://example.com/',
+ releaseBrowserSession() {},
+ computerUseTools: computerTools(),
+ releaseComputerUseSession() {},
+ additionalGroups: () => [
+ // Invalid offerId — a caller misconfiguration, not a tool-schema
+ // problem. Its one tool has a perfectly valid schema.
+ {
+ offerId: 'bad offer id!',
+ label: 'Bad',
+ description: 'bad',
+ tools: [tool('fine_tool', z.object({}), async () => 'ok')],
+ },
+ ],
+ });
+ } finally {
+ console.warn = originalWarn;
+ }
+
+ // The misconfigured capability is dropped; Browser still registers.
+ assert.deepEqual(
+ provider.offers().map((offer) => offer.offerId),
+ ['desktop_browser'],
+ );
+ // The diagnostic blames the capability's metadata, not the tool's schema.
+ assert.equal(
+ warnings.some((line) => line.includes('bad offer id!') && line.includes('offer metadata')),
+ true,
+ );
+ assert.equal(warnings.some((line) => line.includes('fine_tool')), false);
+});
+
test('publishes every production Desktop-owned tool schema through the protocol', () => {
const settingsTools = buildClientSettingsTools({
Review Comment:
**P2, and worth doing even if nothing else changes.** This test's text did not change but its meaning did. It asserts only `doesNotThrow` on `provider.offers()`. Before this PR that proved every Client-settings and Rive tool was representable, because an unrepresentable one threw at construction. Now a dropped tool simply vanishes from `offers()` and the remainder still decodes, so it passes; it fails only if every offer is dropped. A future Desktop-owned tool with an unrepresentable Zod schema will disappear silently with green CI, which is the exact failure mode this PR exists to make visible.
Asserting the expected tool names per offer is about four lines, for example `deepEqual(offer.tools.map(t => t.name), settingsTools.map(t => t.name))` for `desktop_settings` and `['rive_workflow']` for `desktop_rive`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org