Repository navigation
fix(agent-core/mcp): sanitize MCP JSON Schemas for Moonshot MFJS validator #830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
b759414
c285628
57754ea
c561f26
8b01448
ae291f4
2006db8
e79be78
31066ad
7ac49de
7228e16
3374898
a24e2b6
177cd93
ea6d861
552724e
3c2bfa4
9bca8dc
d1b440d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@moonshot-ai/agent-core-v2": patch | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Sanitize circular and non-standard MCP JSON schemas and map disabled field to enabled in MCP config. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { SseMcpClient } from './client-sse'; | |
| import type { UnexpectedCloseReason } from './client-shared'; | ||
| import { StdioMcpClient } from './client-stdio'; | ||
| import type { McpOAuthService } from '#/mcpCore/oauth/service'; | ||
| import { sanitizeMcpSchema } from './schema-sanitize'; | ||
| import { assertMcpInputSchema, type MCPClient, type MCPToolDefinition } from './types'; | ||
|
|
||
| export type McpServerStatus = 'pending' | 'connected' | 'failed' | 'disabled' | 'needs-auth' | 'removed'; | ||
|
|
@@ -451,7 +452,7 @@ export class McpConnectionManager implements McpConnectionView { | |
| tools: mcpTools.map((mcpTool) => ({ | ||
| name: mcpTool.name, | ||
| description: mcpTool.description, | ||
| parameters: assertMcpInputSchema(mcpTool.name, mcpTool.inputSchema), | ||
| parameters: sanitizeMcpSchema(assertMcpInputSchema(mcpTool.name, mcpTool.inputSchema)), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence beyond the earlier reference-resolution comments is the filter ordering: when AGENTS.md reference: AGENTS.md:L110-L112 Useful? React with 👍 / 👎. |
||
| })), | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,237 @@ | ||
| type Json = string | number | boolean | null | Json[] | { [key: string]: Json }; | ||
| type JsonRecord = Record<string, Json>; | ||
|
|
||
| const COMBINATOR_KEYS = [ | ||
| 'anyOf', | ||
| 'oneOf', | ||
| 'allOf', | ||
| 'not', | ||
| 'if', | ||
| 'then', | ||
| 'else', | ||
| '$ref', | ||
| ] as const; | ||
|
|
||
| const OBJECT_KEYWORDS = [ | ||
| 'properties', | ||
| 'additionalProperties', | ||
| 'patternProperties', | ||
| 'propertyNames', | ||
| 'required', | ||
| 'minProperties', | ||
| 'maxProperties', | ||
| ] as const; | ||
|
|
||
| const ARRAY_KEYWORDS = [ | ||
| 'items', | ||
| 'prefixItems', | ||
| 'minItems', | ||
| 'maxItems', | ||
| 'uniqueItems', | ||
| 'contains', | ||
| ] as const; | ||
|
|
||
| const STRING_KEYWORDS = ['minLength', 'maxLength', 'pattern', 'format'] as const; | ||
|
|
||
| const NUMERIC_KEYWORDS = [ | ||
| 'minimum', | ||
| 'maximum', | ||
| 'multipleOf', | ||
| 'exclusiveMinimum', | ||
| 'exclusiveMaximum', | ||
| ] as const; | ||
|
|
||
| function decodePointerSegment(segment: string): string { | ||
| return segment.replace(/~1/g, '/').replace(/~0/g, '~'); | ||
| } | ||
|
|
||
| function jsonTypeOf(value: Json): string { | ||
| if (typeof value === 'boolean') return 'boolean'; | ||
| if (typeof value === 'number') return Number.isInteger(value) ? 'integer' : 'number'; | ||
| if (typeof value === 'string') return 'string'; | ||
| if (value === null) return 'null'; | ||
| if (Array.isArray(value)) return 'array'; | ||
| if (typeof value === 'object') return 'object'; | ||
| return 'string'; | ||
| } | ||
|
|
||
| function derefJsonSchema(schema: JsonRecord): JsonRecord { | ||
| const root = structuredClone(schema); | ||
|
|
||
| function resolvePointer(pointer: string): Json { | ||
| const pathStr = pointer.replace(/^#\/?/, ''); | ||
| if (pathStr === '') { | ||
| return root; | ||
| } | ||
| const parts = pathStr.split('/').map(decodePointerSegment); | ||
| let current: Json = root; | ||
| for (const part of parts) { | ||
| if (Array.isArray(current)) { | ||
| if (!/^(0|[1-9][0-9]*)$/.test(part)) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); | ||
| } | ||
| const index = Number(part); | ||
| if (index < 0 || index >= current.length) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); | ||
| } | ||
| current = current[index] as Json; | ||
| continue; | ||
| } | ||
| if (typeof current !== 'object' || current === null) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); | ||
| } | ||
| if (!(part in (current as JsonRecord))) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); | ||
| } | ||
| current = (current as JsonRecord)[part] as Json; | ||
| } | ||
| return current; | ||
| } | ||
|
|
||
| function traverse(node: Json, activeRefs: Set<string> = new Set()): Json { | ||
| if (Array.isArray(node)) { | ||
| return node.map((item) => traverse(item, activeRefs)); | ||
| } | ||
| if (typeof node !== 'object' || node === null) { | ||
| return node; | ||
| } | ||
| const record = node as JsonRecord; | ||
| if (typeof record['$ref'] === 'string') { | ||
| const ref = record['$ref']; | ||
| if (ref.startsWith('#')) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a valid modern JSON Schema uses Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
| if (activeRefs.has(ref)) { | ||
| return { type: 'object', description: 'Circular reference' }; | ||
| } | ||
| const nextActive = new Set(activeRefs); | ||
| nextActive.add(ref); | ||
| const target = traverse(resolvePointer(ref), nextActive); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an advertised schema contains a non-cyclic fan-out graph—for example, each Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
| if (typeof target !== 'object' || target === null || Array.isArray(target)) { | ||
| throw new Error('Local $ref must resolve to a JSON object'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a valid local Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit |
||
| } | ||
| const { $ref: _, ...rest } = record; | ||
| const traversedRest: JsonRecord = {}; | ||
| for (const [key, value] of Object.entries(rest)) { | ||
| traversedRest[key] = traverse(value, nextActive); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a schema with a local AGENTS.md reference: AGENTS.md:L98-L100 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit |
||
| } | ||
| return { ...(target as JsonRecord), ...traversedRest }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a draft-2020 AGENTS.md reference: AGENTS.md:L98-L100 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. Sibling keywords alongside |
||
| } | ||
| return record; | ||
| } | ||
| const result: JsonRecord = {}; | ||
| for (const [key, value] of Object.entries(record)) { | ||
| result[key] = traverse(value, activeRefs); | ||
|
creatiVision marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an MCP input schema defines a property literally named AGENTS.md reference: AGENTS.md:L130-L132 Useful? React with 👍 / 👎. |
||
| } | ||
| return result; | ||
| } | ||
|
|
||
| const resolved = traverse(root) as JsonRecord; | ||
| delete resolved['$defs']; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a draft-2020 schema declares AGENTS.md reference: AGENTS.md:L110-L112 Useful? React with 👍 / 👎. |
||
| delete resolved['definitions']; | ||
| return resolved; | ||
| } | ||
|
|
||
| function splitMixedEnum(values: Json[]): JsonRecord[] { | ||
| const buckets = new Map<string, Json[]>(); | ||
| for (const value of values) { | ||
| const t = jsonTypeOf(value); | ||
| const list = buckets.get(t) ?? []; | ||
| list.push(value); | ||
| buckets.set(t, list); | ||
| } | ||
| if (buckets.has('integer') && buckets.has('number')) { | ||
| const merged = [...(buckets.get('integer') ?? []), ...(buckets.get('number') ?? [])]; | ||
| buckets.delete('integer'); | ||
| buckets.set('number', merged); | ||
| } | ||
| return [...buckets.entries()].map(([type, enumValues]) => ({ | ||
| type, | ||
| enum: enumValues, | ||
| })); | ||
| } | ||
|
|
||
| function inferTypeFromStructure(node: JsonRecord): string { | ||
| if (OBJECT_KEYWORDS.some((k) => k in node)) return 'object'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a typeless MCP property uses AGENTS.md reference: AGENTS.md:L98-L100 Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit |
||
| if (ARRAY_KEYWORDS.some((k) => k in node)) return 'array'; | ||
| if (STRING_KEYWORDS.some((k) => k in node)) return 'string'; | ||
| if (NUMERIC_KEYWORDS.some((k) => k in node)) return 'number'; | ||
| return 'string'; | ||
| } | ||
|
|
||
| function normalizeProperty(node: Json): void { | ||
| if (typeof node !== 'object' || node === null || Array.isArray(node)) return; | ||
| const record = node as JsonRecord; | ||
|
|
||
| if (!('type' in record) && !COMBINATOR_KEYS.some((key) => key in record)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an MCP property declares Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a 2020-12 schema puts AGENTS.md reference: AGENTS.md:L100-L100 Useful? React with 👍 / 👎. |
||
| const enumValues = record['enum']; | ||
| if (Array.isArray(enumValues) && enumValues.length > 0) { | ||
| const types = new Set(enumValues.map((v) => jsonTypeOf(v))); | ||
| if (types.size === 1) { | ||
| record['type'] = [...types][0]!; | ||
| } else if (types.size === 2 && types.has('integer') && types.has('number')) { | ||
| record['type'] = 'number'; | ||
| } else { | ||
| record['anyOf'] = splitMixedEnum(enumValues); | ||
| delete record['enum']; | ||
| } | ||
| } else if ('const' in record) { | ||
| record['type'] = jsonTypeOf(record['const'] as Json); | ||
| } else { | ||
| record['type'] = inferTypeFromStructure(record); | ||
| } | ||
| } | ||
|
|
||
| recurseSchema(record); | ||
| } | ||
|
|
||
| function recurseSchema(node: Json): void { | ||
| if (typeof node !== 'object' || node === null || Array.isArray(node)) return; | ||
| const record = node as JsonRecord; | ||
|
|
||
| const props = record['properties']; | ||
| if (typeof props === 'object' && props !== null && !Array.isArray(props)) { | ||
| for (const value of Object.values(props as JsonRecord)) { | ||
| normalizeProperty(value); | ||
| } | ||
| } | ||
|
|
||
| const items = record['items']; | ||
| if (Array.isArray(items)) { | ||
| for (const value of items) normalizeProperty(value); | ||
| if (items.length === 0) { | ||
| delete record['items']; | ||
| } else if (items.length === 1) { | ||
| record['items'] = items[0] as Json; | ||
| } else { | ||
| record['items'] = { anyOf: items }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a draft-07 tuple such as Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
| } | ||
| } else if (typeof items === 'object' && items !== null) { | ||
| normalizeProperty(items); | ||
| } | ||
|
|
||
| const prefixItems = record['prefixItems']; | ||
| if (Array.isArray(prefixItems)) { | ||
| for (const value of prefixItems) normalizeProperty(value); | ||
| } | ||
|
|
||
| const additional = record['additionalProperties']; | ||
| if (typeof additional === 'object' && additional !== null && !Array.isArray(additional)) { | ||
| normalizeProperty(additional); | ||
| } | ||
|
|
||
| for (const key of ['anyOf', 'oneOf', 'allOf'] as const) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When tuple-style Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit |
||
| const branches = record[key]; | ||
| if (Array.isArray(branches)) { | ||
| for (const value of branches) normalizeProperty(value); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function sanitizeMcpSchema(schema: unknown): Record<string, unknown> { | ||
| if (typeof schema !== 'object' || schema === null || Array.isArray(schema)) { | ||
| return schema as Record<string, unknown>; | ||
| } | ||
| const dereffed = derefJsonSchema(schema as JsonRecord); | ||
| const cloned = structuredClone(dereffed); | ||
| recurseSchema(cloned); | ||
| return cloned; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using Anthropic, OpenAI, or Google with an MCP tool, this stores the Kimi-specific rewrite in the shared
Tool.parameters; those request formatters and the tool-argument validator subsequently consume the rewritten schema unchanged. Valid schemas such as a typeless arbitrary-value property are therefore narrowed tostring, and tuple schemas are altered even though those providers can accept the original schema, causing models to generate or local validation to reject otherwise valid arguments. Preserve the MCP schema here and apply this compatibility normalization only in the Kimi request path.Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in
packages/agent-core-v2/src/mcpCore/connection-manager.ts. The unmodified original tool definitions from MCP discovery are preserved inrawTools. Sanitizingtools.parametersduring discovery ensures that schemas with local$ref, missing property types, and non-conforming tuple items are normalized upfront into clean standard JSON Schema, avoiding repeated per-turn dereferencing overhead across providers while preventing immediate HTTP 400 rejection from Moonshot MFJS.