-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: inline local $ref in tool inputSchema for LLM consumption #1563
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 all commits
45d2ba1
d70736c
de2e1d6
96c99f0
8cc0f47
4fd247b
84a7676
290ebc6
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,7 @@ | ||
| --- | ||
| '@modelcontextprotocol/core-internal': patch | ||
| '@modelcontextprotocol/client': patch | ||
| '@modelcontextprotocol/server': patch | ||
| --- | ||
|
|
||
| Inline local `$ref` pointers in tool `inputSchema` so schemas are self-contained and LLM-consumable. LLMs cannot resolve JSON Schema `$ref` — they serialize referenced parameters as strings instead of objects. Recursive schemas are handled gracefully — cyclic `$ref` pointers are left in place with only their `$defs` entries preserved, while all non-cyclic refs are fully inlined. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,122 @@ export type AnyObjectSchema = z.core.$ZodObject; | |
| */ | ||
| export type SchemaOutput<T extends AnySchema> = z.output<T>; | ||
|
|
||
| /** | ||
| * Resolves all local `$ref` pointers in a JSON Schema by inlining the | ||
| * referenced definitions. | ||
| * | ||
| * - Caches resolved defs to avoid redundant work with diamond references | ||
| * (A→B→D, A→C→D — D is resolved once and reused). | ||
| * - Gracefully handles cycles — cyclic `$ref` are left in place with their | ||
| * `$defs` entries preserved. Non-cyclic refs in the same schema are still | ||
| * fully inlined. This avoids breaking existing servers that have recursive | ||
| * schemas which work (degraded) today. | ||
| * - Preserves sibling keywords alongside `$ref` per JSON Schema 2020-12 | ||
| * (e.g. `{ "$ref": "...", "description": "override" }`). | ||
| * | ||
| * @internal Exported for testing only. | ||
| */ | ||
| export function dereferenceLocalRefs(schema: Record<string, unknown>): Record<string, unknown> { | ||
| // "$defs" is the standard keyword since JSON Schema 2019-09. | ||
| // See: https://json-schema.org/draft/2020-12/json-schema-core#section-8.2.4 | ||
| // "definitions" is the legacy equivalent from drafts 04–07. | ||
| // See: https://json-schema.org/draft-07/json-schema-validation#section-9 | ||
| // If both exist (malformed schema), "$defs" takes precedence. | ||
| const defsKey = '$defs' in schema ? '$defs' : 'definitions' in schema ? 'definitions' : undefined; | ||
| const defs: Record<string, unknown> = defsKey ? (schema[defsKey] as Record<string, unknown>) : {}; | ||
|
|
||
| // No definitions container — nothing to inline. | ||
| // Note: $ref: "#" (root self-reference) is intentionally not handled — no schema | ||
| // library produces it, no other MCP SDK handles it, and it's always cyclic. | ||
| if (!defsKey) return schema; | ||
|
|
||
| // Cache resolved defs to avoid redundant traversal on diamond references | ||
| // (A→B→D, A→C→D — D is resolved once and reused). Cached values are shared | ||
| // by reference, which is safe because schemas are immutable after generation. | ||
| const resolvedDefs = new Map<string, unknown>(); | ||
| // Def names where a cycle was detected — these $ref are left in place | ||
| // and their $defs entries must be preserved in the output. | ||
| const cyclicDefs = new Set<string>(); | ||
|
|
||
| /** | ||
| * Recursively inlines `$ref` pointers in a JSON Schema node by replacing | ||
| * them with the referenced definition content. | ||
| * | ||
| * @param node - The current schema node being traversed. | ||
| * @param stack - Def names currently being inlined (ancestor chain). If a | ||
| * def is encountered while already on the stack, it's a cycle — the | ||
| * `$ref` is left in place and the def name is added to `cyclicDefs`. | ||
| */ | ||
| function inlineRefs(node: unknown, stack: Set<string>): unknown { | ||
| if (node === null || typeof node !== 'object') return node; | ||
| if (Array.isArray(node)) return node.map(item => inlineRefs(item, stack)); | ||
|
|
||
| const obj = node as Record<string, unknown>; | ||
|
|
||
| // JSON Schema 2020-12 allows keywords alongside $ref (e.g. description, default). | ||
| // Destructure to get the ref target and any sibling keywords to merge later. | ||
| const { $ref: ref, ...siblings } = obj; | ||
| if (typeof ref === 'string') { | ||
| const hasSiblings = Object.keys(siblings).length > 0; | ||
|
|
||
| let resolved: unknown; | ||
|
|
||
| // Local definition reference: #/$defs/Name or #/definitions/Name | ||
| const prefix = `#/${defsKey}/`; | ||
| if (!ref.startsWith(prefix)) return obj; // Non-local $ref (external URL, etc.) — leave as-is | ||
|
|
||
| const defName = ref.slice(prefix.length); | ||
| const def = defs[defName]; | ||
| if (def === undefined) return obj; // Unknown def — leave as-is | ||
| if (stack.has(defName)) { | ||
| cyclicDefs.add(defName); | ||
| return obj; // Cycle — leave $ref in place | ||
| } | ||
|
|
||
| if (resolvedDefs.has(defName)) { | ||
| resolved = resolvedDefs.get(defName); | ||
| } else { | ||
| stack.add(defName); | ||
| resolved = inlineRefs(def, stack); | ||
| stack.delete(defName); | ||
| resolvedDefs.set(defName, resolved); | ||
|
Comment on lines
+95
to
+101
Contributor
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. 🟡 (optional) Servers whose Zod schemas use z.globalRegistry/.meta({id}) types that reference each other (OpenAPI-derived graphs) now ship a tools/list payload that grows with the product of fan-outs (N^D) instead of one Extended reasoning...Trigger: a tool registered with a Zod inputSchema built from registered types that fan out, e.g. Verification: nit — conflicts with stated purpose: a size bound would leave some |
||
| } | ||
|
|
||
| // Merge sibling keywords onto the resolved definition. | ||
| // Note: boolean JSON Schemas (true/false) skip this merge — siblings are dropped. | ||
| // This is acceptable: the SDK's JsonSchemaType excludes boolean schemas by design, | ||
| // and no schema library (Zod v4, ArkType, Valibot) produces boolean $defs entries. | ||
| if (hasSiblings && resolved !== null && typeof resolved === 'object' && !Array.isArray(resolved)) { | ||
| const resolvedSiblings = Object.fromEntries(Object.entries(siblings).map(([k, v]) => [k, inlineRefs(v, stack)])); | ||
| return { ...(resolved as Record<string, unknown>), ...resolvedSiblings }; | ||
|
Comment on lines
+108
to
+110
Contributor
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. 🟡 (optional) The sibling merge at schema.ts:110 spreads the Extended reasoning...JSON Schema 2020-12 defines Verification: nit — triggers when a non-'mcp' vendor schema (a custom Standard JSON Schema adapter, or a JSON-Schema-native library) yields a |
||
| } | ||
| return resolved; | ||
| } | ||
|
|
||
| // Regular object — recurse into values, skipping root-level $defs container | ||
| const result: Record<string, unknown> = {}; | ||
| for (const [key, value] of Object.entries(obj)) { | ||
| if (obj === schema && (key === '$defs' || key === 'definitions')) continue; | ||
|
Comment on lines
+89
to
+118
Contributor
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. 🟡 (optional) A local Extended reasoning...Condition: a non- Verification: nit — triggers only when a non-'mcp' StandardJSONSchemaV1 converter emits a local |
||
| result[key] = inlineRefs(value, stack); | ||
| } | ||
| return result; | ||
|
claude[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| const resolved = inlineRefs(schema, new Set()) as Record<string, unknown>; | ||
|
|
||
| // Re-attach $defs only for cyclic definitions, using their resolved/cached | ||
| // versions so that any non-cyclic refs inside them are already inlined. | ||
| if (defsKey && cyclicDefs.size > 0) { | ||
| const prunedDefs: Record<string, unknown> = {}; | ||
| for (const name of cyclicDefs) { | ||
| prunedDefs[name] = resolvedDefs.get(name) ?? defs[name]; | ||
| } | ||
| resolved[defsKey] = prunedDefs; | ||
| } | ||
|
|
||
| return resolved; | ||
| } | ||
|
|
||
| /** | ||
| * Parses data against a Zod schema (synchronous). | ||
| * Returns a discriminated union with success/error. | ||
|
|
||
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.
🟡 (optional) If any non-'mcp' Standard Schema converter returns
$defs(ordefinitions) asnull/a non-object, schema.ts:44-45 selects it as the defs container without a type check and the first local$refdereferencesdefs[defName]onnullat :88, so tools/list (mcp.ts:189, uncached, every request) throws a TypeError and returns a JSON-RPC error for the whole tool list, where base passed the schema through untouched. Fix: only treat$defs/definitionsas a container when it is a non-null, non-array object (fall back to returningschemaunchanged otherwise), so malformed converter output degrades to the base behaviour instead of a 500.Extended reasoning...
Path: standardSchemaToJsonSchema :242 -> dereferenceLocalRefs. :44
defsKey = '$defs' in schema ? '$defs' : ...is a key-presence test, so{$defs: null, properties:{a:{$ref:'#/$defs/A'}}}selects '$defs'; :45 castsnullto Record without checking; :50 does not return because defsKey is set; walk reaches{$ref}at :78 -> prefix matches -> :88defs['A']-> TypeError: Cannot read properties of null. Consumer: mcp.ts:189 runs this inside the tools/list handler on every request (no memo) so the exception becomes a JSON-RPC internal error for tools/list; registerTool's eager conversion at :831 swallows it per its warn-never-throw contract, so registration appears to succeed and the failure only surfaces at list time. Base:{ type:'object', ...result }returned as-is, list succeeds. Population (external trigger, conditional): any StandardJSONSchemaV1 vendor other than 'mcp' whose~standard.jsonSchema.input()emits a null/empty-literal defs slot — themcpguard at standardSchema.ts:239 does not protect ArkType/Valibot/third-party converters, and five finders independently…Verification: nit — triggers only when a non-'mcp' StandardJSONSchemaV1 converter returns
$defs(ordefinitions) asnull/undefinedwhile also emitting a#/$defs/Xlocal$ref(a self-contradictory schema no known library — Zod v4, ArkType, Valibot — produces; realistically only a custom or buggy third-party~standard.jsonSchemaimplementation). Mechanism verified in… | nit. Trigger: a non-'mcp'…