fix(desktop): isolate invalid optional MCP tools - #3366
Conversation
Keep the Desktop Runtime Host candidate usable when one optional MCP tool publishes an invalid schema, while retaining fail-closed behavior for required native capabilities. Cover AI SDK JSON Schema invocation and rate-limit terminal admission recovery. Generated-by: Codex
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
深入 review 后需要修改再合入。
两个阻断问题：
- optional MCP tool 的过滤只验证单个 tool，无法保证最终 capability manifest 仍满足单 offer 64 tools、总 manifest 56 KiB 等协议约束，candidate 仍会因完整注册失败。
- AI SDK
jsonSchema()默认没有validate，当前 fallback 会直接放行参数；这使 Desktop 与 CodeMode 的 JSON Schema 执行契约不一致，非法参数会在 admission 后发往 MCP server。
建议以完整 manifest 作为唯一验证单位，确定性地加入 optional tool；并抽取 Runtime 共用的 JSON Schema validation adapter。另请将无 production change 对应的 rate-limit test 拆出本 PR。
| try { | ||
| decodeClientCapabilityReplaceInput({ | ||
| registrationId: "desktop_capability_validation", | ||
| offers: [capabilityOffer({ ...group, tools: [tool] }, hostPathAccess)], |
There was a problem hiding this comment.
这里每次只对一个 tool 构造 offer 并 decode，但最终会把整个 MCP group 一次性注册。协议的 CLIENT_CAPABILITY_MAX_TOOLS_PER_OFFER（64）和 56 KiB manifest 限制都是跨 tool 的：65 个各自合法的 MCP tool，或 3 个各约 20 KiB 的 schema，都会分别在最终注册时失败。请按最终完整 manifest 做增量校验并确定性地省略不能加入的 optional tool，否则 candidate 仍会被 optional MCP 工具拖垮。
| typeof parameters.jsonSchema === "object" && | ||
| !Array.isArray(parameters.jsonSchema) | ||
| ) { | ||
| return value; |
There was a problem hiding this comment.
buildMcpTools() 用的是 AI SDK jsonSchema(descriptor.inputSchema)；AI SDK 将 validate 定义为 optional，默认正是 undefined。因此这个分支会把例如缺少 required 字段的参数直接放行，并在 options.accept() 后调用 MCP。CodeMode 已对同一类 schema 用 Ajv fallback 验证。请复用/抽取那条 Runtime 级 validation path，而不是在 Desktop 绕过 schema 的执行契约。
| }); | ||
| }); | ||
|
|
||
| test('a rate-limited root Turn releases admission before regenerate', async () => { |
There was a problem hiding this comment.
本 PR 没有对应的 Runtime Host production change；该 test 在 main 上也应成立，且与 optional MCP schema isolation 无关。请从此 PR 移除，若确有独立 rate-limit regression，再用 focused PR 提交，避免继续扩张较重的 execution-host suite。
|
Addressed the requested changes in 2bdd7f9. Optional MCP tools are now admitted deterministically only when the complete manifest decodes successfully, covering the 64-tool and 56 KiB limits. Desktop now uses the Runtime Ajv JSON Schema adapter before accept/MCP dispatch, with a regression test proving invalid arguments are neither accepted nor invoked. The unrelated rate-limit Runtime Host test has been removed. |
Validate optional MCP tools against the complete capability manifest, reuse Runtime JSON Schema validation before admission, and remove unrelated Runtime Host coverage. Generated-by: Codex
d593184 to
2bdd7f9
Compare
2 participants
Summary
Refs #3302
An invalid optional MCP tool schema previously caused the entire Desktop Runtime Host capability candidate to fail. This PR now:
This PR intentionally scopes itself to the MCP capability-isolation portion of #3302.
Verification
npm --workspace @maka/runtime run build- passednpm --workspace @maka/desktop run build:main- passednpm --workspace @maka/runtime run typecheck- passednpm --workspace @maka/runtime-host run typecheck- passednpm run lint- passednpm run format:check- passednpm run typecheck/ Desktop typecheck currently fail on unrelated UI API mismatches (settledText,conversationKey,unlockAutoFollow, andtrailingAction).npx knip --workspace apps/desktopcurrently reports three existing unused PiP exports and onetaskkillconfiguration hint.AI use
Tool(s) and scope: Codex contributed to issue analysis, implementation, tests, code review, and PR wording. I reviewed the changes and remain the human contributor of record.
Checklist
Does this PR entail a change in behavior?