fix(agent-core/mcp): sanitize MCP JSON Schemas for Moonshot MFJS vali… - #2242
fix(agent-core/mcp): sanitize MCP JSON Schemas for Moonshot MFJS vali…#2242creatiVision wants to merge 1 commit into
Conversation
…dator Resolve local $ref (with RFC 6901 pointer unescaping and circular-safe inlining), fill missing property types, normalize tuple items arrays, and split mixed-type enums into typed anyOf branches. Also map common config aliases (disabled→enabled, max_tokens→maxOutputSize).
🦋 Changeset detectedLatest commit: a0fc9fe The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0fc9fe2a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (Array.isArray(items)) { | ||
| for (const value of items) normalizeProperty(value); | ||
| record['items'] = { anyOf: items }; |
There was a problem hiding this comment.
Preserve positional semantics when normalizing tuple items
When an MCP server advertises a draft-07 tuple such as items: [{ type: 'string' }, { type: 'number' }], replacing it with items: { anyOf: ... } changes the contract from positional validation to allowing either type at every position. Since the resulting Tool.parameters is also compiled by validateExecutableToolArgs, malformed calls such as [1, 'x'] now pass local validation and reach the MCP server. Preserve the original tuple semantics rather than converting it to a homogeneous union.
Useful? React with 👍 / 👎.
| if (typeof current !== 'object' || current === null || Array.isArray(current)) { | ||
| throw new Error(`Unable to resolve reference path: ${pointer}`); |
There was a problem hiding this comment.
Resolve array segments in local JSON Pointers
For a valid local reference such as #/$defs/Choice/anyOf/0, traversal reaches the anyOf array before processing segment 0, and this condition throws instead of resolving the array index. The exception propagates through connectAndDiscoverTools, causing connectOne to mark the entire MCP server failed, so one such schema prevents all of that server's tools from loading. Handle numeric RFC 6901 array segments during pointer resolution.
Useful? React with 👍 / 👎.
| const { $ref: _, ...rest } = record; | ||
| return { ...(target as JsonRecord), ...rest }; |
There was a problem hiding this comment.
Dereference schema content in
$ref siblings
When a draft 2020-12 $ref node has sibling schema content containing another local reference, the sibling values in rest are merged without passing through traverse. The root definition buckets are subsequently deleted, leaving that nested reference dangling; Ajv then fails to compile the tool schema and every invocation is rejected. Traverse the sibling values before merging them into the resolved target.
Useful? React with 👍 / 👎.
…dator
Resolve local $ref (with RFC 6901 pointer unescaping and circular-safe inlining), fill missing property types, normalize tuple items arrays, and split mixed-type enums into typed anyOf branches. Also map common config aliases (disabled→enabled, max_tokens→maxOutputSize).
Related Issue
Resolve #(issue_number)
Problem
What changed
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.