You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "sunheyi6 (via GitHub)" <gi...@apache.org> on 2026/08/30 02:31:11 UTC
[PR] fix(desktop): adapt MCP schemas for native capabilities [maka]
sunheyi6 opened a new pull request, #4235:
URL: https://github.com/apache/maka/pull/4235
## Summary
- convert MCP JSON Schema descriptors into Zod schemas at tool construction
- preserve Desktop native-capability schema publication and pre-admission validation
- add a composition regression covering MCP publication, invocation, and invalid arguments
## Testing
- `npm run build:test`
- `node --test packages/runtime/dist/__tests__/mcp-tools.test.js apps/desktop/dist/main/__tests__/runtime-host-native-capabilities.test.js apps/desktop/dist/main/__tests__/mcp-runtime-e2e.test.js` (22 passed)
- `npm run typecheck`
- `npm run check:asf-headers`
- `npm run lint`
Fixes #4209.
<details>
<summary>中文</summary>
## 摘要
- 在构造 MCP 工具时，将 MCP JSON Schema 描述转换为 Zod schema
- 保持 Desktop native capability 的 schema 发布与 admission 前参数校验
- 增加跨层组合回归测试，覆盖 MCP 工具发布、调用以及非法参数拒绝
## 验证
- `npm run build:test`
- `node --test packages/runtime/dist/__tests__/mcp-tools.test.js apps/desktop/dist/main/__tests__/runtime-host-native-capabilities.test.js apps/desktop/dist/main/__tests__/mcp-runtime-e2e.test.js`（22 项通过）
- `npm run typecheck`
- `npm run check:asf-headers`
- `npm run lint`
修复 #4209。
</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): adapt MCP schemas for native capabilities [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4235:
URL: https://github.com/apache/maka/pull/4235#discussion_r3890175054
##########
packages/runtime/src/tool-parameters.ts:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import Ajv, { type AnySchema, type ValidateFunction } from 'ajv';
+import Ajv2019 from 'ajv/dist/2019.js';
+import Ajv2020 from 'ajv/dist/2020.js';
+
+const jsonSchemaOptions = {
+ allErrors: true,
+ strict: false,
+ validateFormats: false,
+} as const;
+const draft7Validator = new Ajv(jsonSchemaOptions);
+const draft2019Validator = new Ajv2019(jsonSchemaOptions);
+const draft2020Validator = new Ajv2020(jsonSchemaOptions);
+const compiledSchemas = new WeakMap<object, ValidateFunction>();
+
+interface ToolParameterSchema {
+ safeParseAsync?: (
+ value: unknown,
+ ) => PromiseLike<{ success: true; data: unknown } | { success: false; error: unknown }>;
+ safeParse?: (
+ value: unknown,
+ ) => { success: true; data: unknown } | { success: false; error: unknown };
+ validate?: (
+ value: unknown,
+ ) =>
+ | { success: true; value: unknown }
+ | { success: false; error: unknown }
+ | PromiseLike<{ success: true; value: unknown } | { success: false; error: unknown }>;
+ jsonSchema?: unknown;
+ '~standard'?: {
+ validate?: (
+ value: unknown,
+ ) =>
+ | { value: unknown }
+ | { issues: readonly unknown[] }
+ | PromiseLike<{ value: unknown } | { issues: readonly unknown[] }>;
+ };
+}
+
+export class ToolParameterValidationError extends Error {
+ readonly issues: readonly unknown[];
+
+ constructor(issues: readonly unknown[]) {
+ super('Tool arguments failed declared schema validation', { cause: issues });
+ this.name = 'ToolParameterValidationError';
+ this.issues = issues;
+ }
+}
+
+export async function parseToolParameters(parameters: unknown, input: unknown): Promise<unknown> {
+ if (!parameters || (typeof parameters !== 'object' && typeof parameters !== 'function')) {
+ return input;
+ }
+ const schema = parameters as ToolParameterSchema;
+ if (typeof schema.safeParseAsync === 'function') {
+ const parsed = await schema.safeParseAsync(input);
+ if (parsed.success) return parsed.data;
+ throw parsed.error;
+ }
+ if (typeof schema.safeParse === 'function') {
+ const parsed = schema.safeParse(input);
+ if (parsed.success) return parsed.data;
+ throw parsed.error;
+ }
+ if (typeof schema.validate === 'function') {
+ const parsed = await schema.validate(input);
+ if (parsed.success) return parsed.value;
+ throw parsed.error;
+ }
+ if (typeof schema['~standard']?.validate === 'function') {
+ const parsed = await schema['~standard'].validate(input);
+ if ('value' in parsed) return parsed.value;
+ throw new ToolParameterValidationError(parsed.issues);
+ }
+
+ const declaredJsonSchema = await schema.jsonSchema;
+ const validator = compileJsonSchema(declaredJsonSchema ?? parameters);
+ if (!validator || (await validator(input))) return input;
+ throw new ToolParameterValidationError(validator.errors ?? []);
+}
+
+function compileJsonSchema(schema: unknown): ValidateFunction | undefined {
+ if (typeof schema === 'boolean') return draft2020Validator.compile(schema);
+ if (typeof schema !== 'object' || schema === null || Array.isArray(schema)) return undefined;
+ const cached = compiledSchemas.get(schema);
+ if (cached) return cached;
+ const declaredDialect = (schema as { readonly $schema?: unknown }).$schema;
Review Comment:
[P1] (category ① — normal supported MCP schema path)
Thanks for selecting the validator from the declared dialect. The production MCP manager currently strips `$schema` recursively in `normalizeToolInputSchema()`, so an explicit draft-07 schema reaches this branch with no dialect and is compiled as 2020-12. A valid draft-07 tuple (`items: [...]`, `additionalItems: false`) is then rejected by Ajv2020 even though the MCP server declared it correctly. The current native-capability tests hand-construct descriptors and bypass that normalization. Could the root dialect survive manager discovery into this validator (or be carried separately), with a real manager → `buildMcpTools` → native invocation draft-07 tuple regression? Please push back if the product intentionally canonicalizes every MCP schema to 2020-12 elsewhere.
##########
packages/runtime/src/tool-parameters.ts:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import Ajv, { type AnySchema, type ValidateFunction } from 'ajv';
+import Ajv2019 from 'ajv/dist/2019.js';
+import Ajv2020 from 'ajv/dist/2020.js';
+
+const jsonSchemaOptions = {
+ allErrors: true,
+ strict: false,
+ validateFormats: false,
+} as const;
+const draft7Validator = new Ajv(jsonSchemaOptions);
+const draft2019Validator = new Ajv2019(jsonSchemaOptions);
+const draft2020Validator = new Ajv2020(jsonSchemaOptions);
+const compiledSchemas = new WeakMap<object, ValidateFunction>();
+
+interface ToolParameterSchema {
+ safeParseAsync?: (
+ value: unknown,
+ ) => PromiseLike<{ success: true; data: unknown } | { success: false; error: unknown }>;
+ safeParse?: (
+ value: unknown,
+ ) => { success: true; data: unknown } | { success: false; error: unknown };
+ validate?: (
+ value: unknown,
+ ) =>
+ | { success: true; value: unknown }
+ | { success: false; error: unknown }
+ | PromiseLike<{ success: true; value: unknown } | { success: false; error: unknown }>;
+ jsonSchema?: unknown;
+ '~standard'?: {
+ validate?: (
+ value: unknown,
+ ) =>
+ | { value: unknown }
+ | { issues: readonly unknown[] }
+ | PromiseLike<{ value: unknown } | { issues: readonly unknown[] }>;
+ };
+}
+
+export class ToolParameterValidationError extends Error {
+ readonly issues: readonly unknown[];
+
+ constructor(issues: readonly unknown[]) {
+ super('Tool arguments failed declared schema validation', { cause: issues });
+ this.name = 'ToolParameterValidationError';
+ this.issues = issues;
+ }
+}
+
+export async function parseToolParameters(parameters: unknown, input: unknown): Promise<unknown> {
+ if (!parameters || (typeof parameters !== 'object' && typeof parameters !== 'function')) {
+ return input;
+ }
+ const schema = parameters as ToolParameterSchema;
+ if (typeof schema.safeParseAsync === 'function') {
+ const parsed = await schema.safeParseAsync(input);
+ if (parsed.success) return parsed.data;
+ throw parsed.error;
+ }
+ if (typeof schema.safeParse === 'function') {
+ const parsed = schema.safeParse(input);
+ if (parsed.success) return parsed.data;
+ throw parsed.error;
+ }
+ if (typeof schema.validate === 'function') {
+ const parsed = await schema.validate(input);
+ if (parsed.success) return parsed.value;
+ throw parsed.error;
+ }
+ if (typeof schema['~standard']?.validate === 'function') {
+ const parsed = await schema['~standard'].validate(input);
+ if ('value' in parsed) return parsed.value;
+ throw new ToolParameterValidationError(parsed.issues);
+ }
+
+ const declaredJsonSchema = await schema.jsonSchema;
+ const validator = compileJsonSchema(declaredJsonSchema ?? parameters);
+ if (!validator || (await validator(input))) return input;
+ throw new ToolParameterValidationError(validator.errors ?? []);
+}
+
+function compileJsonSchema(schema: unknown): ValidateFunction | undefined {
+ if (typeof schema === 'boolean') return draft2020Validator.compile(schema);
+ if (typeof schema !== 'object' || schema === null || Array.isArray(schema)) return undefined;
+ const cached = compiledSchemas.get(schema);
+ if (cached) return cached;
+ const declaredDialect = (schema as { readonly $schema?: unknown }).$schema;
+ const dialect = typeof declaredDialect === 'string' ? declaredDialect : '';
+ const validator = dialect.includes('draft-07')
+ ? draft7Validator
+ : dialect.includes('2019-09')
+ ? draft2019Validator
+ : draft2020Validator;
+ const schemaForCompile = dialect.startsWith('https://json-schema.org/draft-07/schema')
+ ? { ...schema, $schema: dialect.replace('https://', 'http://') }
+ : schema;
+ const compiled = validator.compile(schemaForCompile as AnySchema);
Review Comment:
[P1] (category ② — supported reconnect/recovery path)
Thanks for caching compiled schemas by object identity. The shared Ajv instance still registers each compiled schema’s `$id` globally. After a normal MCP reconnect/config refresh, the manager publishes a newly cloned descriptor object with the same `$id`, so the WeakMap misses and the second compile throws `schema with key or id ... already exists`; the tool then remains uncallable until process restart. The same happens on first use when two tools legitimately reuse one `$id`. Could independent tool schemas avoid the shared global ID registry (for example `addUsedSchema: false`, isolated validators, or canonical dialect+schema caching) and add a first call → reconnect/new clone → second call regression? Please feel free to push back if descriptors are guaranteed to preserve object identity across every reconnect.
--
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): adapt MCP schemas for native capabilities [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4235:
URL: https://github.com/apache/maka/pull/4235#discussion_r3889827386
##########
packages/runtime-host/src/protocol/client-capability.ts:
##########
@@ -687,39 +687,71 @@ const CLIENT_CAPABILITY_SCHEMA_TYPES = new Set([
'object',
'string',
]);
+const CLIENT_CAPABILITY_SCHEMA_DIALECTS = new Set([
+ 'http://json-schema.org/draft-07/schema',
+ 'http://json-schema.org/draft-07/schema#',
+ 'https://json-schema.org/draft-07/schema',
+ 'https://json-schema.org/draft-07/schema#',
+ 'https://json-schema.org/draft/2019-09/schema',
+ 'https://json-schema.org/draft/2020-12/schema',
+]);
const CLIENT_CAPABILITY_SCHEMA_KEYWORDS = new Set([
Review Comment:
[P1] Admit the MCP header annotation supported by this repository
This allowlist still rejects `x-mcp-header`, even though `packages/mcp/src/sep-2243.ts:120-184` validates that annotation and `packages/mcp/src/index.ts:2297-2346` deliberately preserves it in the discovered tool schema. Desktop then forwards that schema unchanged, so `decodeClientCapabilityReplaceInput()` rejects the entire capability replacement before the tool can be installed. I reproduced this on the exact head with a valid `{ type: 'integer', 'x-mcp-header': 'Shard' }` property through `buildMcpTools -> provider.offers -> decodeClientCapabilityReplaceInput`; it throws `Unsupported Client Capability tool schema keyword`. This recreates the original provider-registration failure for the product's supported SEP-2243 tools. Please either admit and type-check this annotation at the protocol boundary, or strip it only after the MCP layer has retained the header metadata, and add a production-path regression using a header-enabled tool.
--
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): adapt MCP schemas for native capabilities [maka]
Posted by "sunheyi6 (via GitHub)" <gi...@apache.org>.
sunheyi6 commented on PR #4235:
URL: https://github.com/apache/maka/pull/4235#issuecomment-5558340000
Closing this PR because the original MCP schema publication and invocation problem is covered by merged PRs #4490 and #4553. The upstream solution leaves complete JSON Schema validation to the MCP server; this PR's broader local validator was not merged. Any remaining validator proposal can be discussed separately.
<details>
<summary>中文翻译</summary>
关闭此 PR：原始 MCP Schema 发布和调用问题已由合并的 #4490 和 #4553 覆盖。上游方案将完整 JSON Schema 校验交给 MCP 服务端，本 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): adapt MCP schemas for native capabilities [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4235:
URL: https://github.com/apache/maka/pull/4235#discussion_r3889272817
##########
apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts:
##########
@@ -169,6 +172,108 @@ test('publishes every production Desktop-owned tool schema through the protocol'
);
});
+test('publishes and invokes MCP JSON Schema tools through Desktop native capabilities', async () => {
+ let receivedArguments: Record<string, unknown> | undefined;
+ const mcpProvider: McpToolProvider = {
+ toolSnapshot: () => ({
+ revision: 1,
+ tools: [
+ {
+ descriptor: {
+ serverId: 'fixture',
+ name: 'echo',
+ description: 'Echo one value',
+ inputSchema: {
+ $schema: 'https://json-schema.org/draft/2020-12/schema',
Review Comment:
**[P1] Exercise this schema through the actual Client Capability channel**
Thanks for preserving the complete MCP schema here. The direct provider test passes, but production does not call this provider directly: `replaceClientCapabilities()` first canonicalizes the offer through `decodeClientCapabilityReplaceInput()`.
That protocol validator still rejects `$schema`, `if`, `then`, `dependentRequired`, and `unevaluatedProperties` as unsupported keywords. Consequently, an MCP server advertising the exact valid schema in this test causes the complete native-capability registration to fail before Runtime can validate or invoke the tool.
Please make the protocol schema contract accept the same bounded JSON Schema dialect, then pass this fixture through `decodeClientCapabilityReplaceInput()` or a real replace/round-trip test. Keeping one shared schema boundary is preferable to adding another lossy projection.
<details><summary>中文对照</summary>
感谢这里保留完整的 MCP Schema。不过当前测试直接调用 provider，绕过了生产环境的 Client Capability 协议入口。协议校验器仍会拒绝 `$schema`、`if`、`then`、`dependentRequired` 和 `unevaluatedProperties`，所以包含这类合法 Schema 的 MCP Server 仍会导致整个能力注册失败。建议统一协议与 Runtime 支持的 Schema 边界，并补真实的 decode 或 replace round-trip 测试。
</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): adapt MCP schemas for native capabilities [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4235:
URL: https://github.com/apache/maka/pull/4235#discussion_r3888381825
##########
packages/runtime/src/mcp-tools.ts:
##########
@@ -96,7 +96,9 @@ export function buildMcpTools(
// ordinary MCP servers retain the side-effecting network default.
categoryHint: options.categoryHint ?? 'network_send',
...(options.recoveryMode ? { recoveryMode: options.recoveryMode } : {}),
- parameters: jsonSchema(descriptor.inputSchema),
+ // Client capabilities publish and validate Zod schemas before admission.
+ // Preserve that boundary when an MCP descriptor starts as JSON Schema.
+ parameters: z.fromJSONSchema(descriptor.inputSchema),
Review Comment:
[P1] Do not let valid MCP schemas crash tool construction
`z.fromJSONSchema` is a partial converter and throws synchronously for valid JSON Schema features such as `if`/`then`/`else`, `dependentSchemas`, `dependentRequired`, and `unevaluatedProperties`. The MCP manager deliberately preserves these keywords (`packages/mcp/src/index.ts:2313-2341`), and Desktop calls `buildMcpTools` while composing its native capability provider, so one MCP server advertising such a schema still takes down the whole capability setup. I reproduced `buildMcpTools` throwing `Conditional schemas (if/then/else) are not supported` with a valid object input schema. Please use a path that preserves and validates arbitrary MCP JSON Schema (or at minimum isolates unsupported schemas without crashing the provider), and add a regression with one of these accepted keywords.
--
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): adapt MCP schemas for native capabilities [maka]
Posted by "sunheyi6 (via GitHub)" <gi...@apache.org>.
sunheyi6 closed pull request #4235: fix(desktop): adapt MCP schemas for native capabilities
URL: https://github.com/apache/maka/pull/4235
--
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