diff --git a/.changeset/inline-ref-in-tool-schema.md b/.changeset/inline-ref-in-tool-schema.md new file mode 100644 index 000000000..56ed20b50 --- /dev/null +++ b/.changeset/inline-ref-in-tool-schema.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/core': 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 now throw at `tools/list` time instead of silently degrading. diff --git a/packages/core/src/util/schema.ts b/packages/core/src/util/schema.ts index 9676674b8..32bc6889d 100644 --- a/packages/core/src/util/schema.ts +++ b/packages/core/src/util/schema.ts @@ -20,6 +20,119 @@ export type AnyObjectSchema = z.core.$ZodObject; */ export type SchemaOutput = z.output; +/** + * 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): Record { + // "$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 = defsKey ? (schema[defsKey] as Record) : {}; + + // 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(); + // 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(); + + /** + * 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): 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; + + // 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); + } + + // Merge sibling keywords onto the resolved definition + 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), ...resolvedSiblings }; + } + return resolved; + } + + // Regular object — recurse into values, skipping root-level $defs container + const result: Record = {}; + for (const [key, value] of Object.entries(obj)) { + if (obj === schema && (key === '$defs' || key === 'definitions')) continue; + result[key] = inlineRefs(value, stack); + } + return result; + } + + const resolved = inlineRefs(schema, new Set()) as Record; + + // 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 = {}; + 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. diff --git a/packages/core/src/util/standardSchema.ts b/packages/core/src/util/standardSchema.ts index 9817dc39a..b6998ca6c 100644 --- a/packages/core/src/util/standardSchema.ts +++ b/packages/core/src/util/standardSchema.ts @@ -6,6 +6,8 @@ /* eslint-disable @typescript-eslint/no-namespace */ +import { dereferenceLocalRefs } from './schema.js'; + // Standard Schema interfaces — vendored from https://standardschema.dev (spec v1, Jan 2025) export interface StandardTypedV1 { @@ -156,7 +158,7 @@ export function standardSchemaToJsonSchema(schema: StandardJSONSchemaV1, io: 'in `Wrap your schema in z.object({...}) or equivalent.` ); } - return { type: 'object', ...result }; + return dereferenceLocalRefs({ type: 'object', ...result }); } // Validation diff --git a/packages/core/test/schema.test.ts b/packages/core/test/schema.test.ts new file mode 100644 index 000000000..c2c737578 --- /dev/null +++ b/packages/core/test/schema.test.ts @@ -0,0 +1,295 @@ +// Unit tests for dereferenceLocalRefs +// Tests raw JSON Schema edge cases independent of the server/client pipeline. +// See: https://github.com/anthropics/claude-code/issues/18260 + +import { describe, expect, test } from 'vitest'; + +import { dereferenceLocalRefs } from '../src/util/schema.js'; + +describe('dereferenceLocalRefs', () => { + test('schema with no $ref passes through unchanged', () => { + const schema = { + type: 'object', + properties: { name: { type: 'string' }, age: { type: 'number' } } + }; + const result = dereferenceLocalRefs(schema); + expect(result).toEqual(schema); + }); + + test('local $ref is inlined and $defs removed', () => { + const schema = { + type: 'object', + properties: { + primary: { $ref: '#/$defs/Tag' }, + secondary: { $ref: '#/$defs/Tag' } + }, + $defs: { + Tag: { type: 'object', properties: { label: { type: 'string' } } } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + primary: { type: 'object', properties: { label: { type: 'string' } } }, + secondary: { type: 'object', properties: { label: { type: 'string' } } } + } + }); + }); + + test('diamond references resolve correctly', () => { + const schema = { + type: 'object', + properties: { + b: { type: 'object', properties: { inner: { $ref: '#/$defs/Shared' } } }, + c: { type: 'object', properties: { inner: { $ref: '#/$defs/Shared' } } } + }, + $defs: { + Shared: { type: 'object', properties: { x: { type: 'number' } } } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + b: { type: 'object', properties: { inner: { type: 'object', properties: { x: { type: 'number' } } } } }, + c: { type: 'object', properties: { inner: { type: 'object', properties: { x: { type: 'number' } } } } } + } + }); + }); + + test('non-existent $def reference is left as-is', () => { + const schema = { + type: 'object', + properties: { broken: { $ref: '#/$defs/DoesNotExist' } }, + $defs: {} + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { broken: { $ref: '#/$defs/DoesNotExist' } } + }); + }); + + test('external $ref and root self-reference are left as-is', () => { + const schema = { + type: 'object', + properties: { + ext: { $ref: 'https://example.com/schemas/Foo.json' }, + self: { $ref: '#' } + } + }; + const result = dereferenceLocalRefs(schema); + expect(result).toEqual(schema); + }); + + test('sibling keywords alongside $ref are preserved', () => { + const schema = { + type: 'object', + properties: { + addr: { $ref: '#/$defs/Address', description: 'Home address', title: 'Home', default: { street: '123 Main' } } + }, + $defs: { + Address: { type: 'object', properties: { street: { type: 'string' } } } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + addr: { + type: 'object', + properties: { street: { type: 'string' } }, + description: 'Home address', + title: 'Home', + default: { street: '123 Main' } + } + } + }); + }); + + test('mixed cyclic and non-cyclic refs: non-cyclic inlined, cyclic preserved', () => { + const schema = { + type: 'object', + properties: { + tag: { $ref: '#/$defs/Tag' }, + tree: { $ref: '#/$defs/TreeNode' } + }, + $defs: { + Tag: { type: 'object', properties: { label: { type: 'string' } } }, + TreeNode: { + type: 'object', + properties: { + value: { type: 'string' }, + tag: { $ref: '#/$defs/Tag' }, + children: { type: 'array', items: { $ref: '#/$defs/TreeNode' } } + } + } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + // Tag fully inlined + tag: { type: 'object', properties: { label: { type: 'string' } } }, + // TreeNode resolved one level: Tag inlined, self-ref stays + tree: { + type: 'object', + properties: { + value: { type: 'string' }, + tag: { type: 'object', properties: { label: { type: 'string' } } }, + children: { type: 'array', items: { $ref: '#/$defs/TreeNode' } } + } + } + }, + // Only cyclic def preserved, with Tag inlined inside it + $defs: { + TreeNode: { + type: 'object', + properties: { + value: { type: 'string' }, + tag: { type: 'object', properties: { label: { type: 'string' } } }, + children: { type: 'array', items: { $ref: '#/$defs/TreeNode' } } + } + } + } + }); + }); + + test('$def referencing another $def (nested registered types)', () => { + const schema = { + type: 'object', + properties: { + employer: { $ref: '#/$defs/Company', description: 'The company' } + }, + $defs: { + Address: { type: 'object', properties: { street: { type: 'string' } } }, + Company: { + type: 'object', + properties: { + name: { type: 'string' }, + hq: { $ref: '#/$defs/Address' } + } + } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + employer: { + type: 'object', + properties: { + name: { type: 'string' }, + hq: { type: 'object', properties: { street: { type: 'string' } } } + }, + description: 'The company' + } + } + }); + }); + + // Defensive: hand-crafted synthetic schema — no known schema generator (Zod v4, + // ArkType, Valibot) produces $ref with a sibling containing nested $ref. + // See: https://github.com/modelcontextprotocol/typescript-sdk/pull/1563#discussion_r3022304127 + test('$ref siblings containing nested $ref are resolved (defensive)', () => { + const schema = { + type: 'object', + properties: { x: { $ref: '#/$defs/Outer' } }, + $defs: { + Outer: { $ref: '#/$defs/Inner', allOf: [{ $ref: '#/$defs/Mixin' }] }, + Inner: { type: 'object' }, + Mixin: { title: 'mixin' } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { x: { type: 'object', allOf: [{ title: 'mixin' }] } } + }); + }); + + test('multi-hop cycle (A → B → C → A) with non-cyclic sibling: cycle detected, all non-cyclic parts inlined', () => { + // Company → Employee → Department → Company (cycle) + // Department also → Location (non-cyclic sibling) + const schema = { + type: 'object', + properties: { company: { $ref: '#/$defs/Company' } }, + $defs: { + // Intentionally unordered — function follows $ref pointers, not declaration order + Location: { + type: 'object', + properties: { city: { type: 'string' } } + }, + Employee: { + type: 'object', + properties: { + name: { type: 'string' }, + department: { $ref: '#/$defs/Department' } + } + }, + Department: { + type: 'object', + properties: { + name: { type: 'string' }, + company: { $ref: '#/$defs/Company' }, + location: { $ref: '#/$defs/Location' } + } + }, + Company: { + type: 'object', + properties: { + name: { type: 'string' }, + employee: { $ref: '#/$defs/Employee' } + } + } + } + }; + const inlinedDepartment = { + type: 'object', + properties: { + name: { type: 'string' }, + company: { $ref: '#/$defs/Company' }, + location: { type: 'object', properties: { city: { type: 'string' } } } + } + }; + const inlinedEmployee = { + type: 'object', + properties: { + name: { type: 'string' }, + department: inlinedDepartment + } + }; + const inlinedCompany = { + type: 'object', + properties: { + name: { type: 'string' }, + employee: inlinedEmployee + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { company: inlinedCompany }, + // Only Company preserved — Location fully inlined everywhere including inside $defs + $defs: { Company: inlinedCompany } + }); + }); + + test('$ref inlined from real $defs while properties named "$defs" and "definitions" are preserved', () => { + const schema = { + type: 'object', + properties: { + definitions: { type: 'array', items: { type: 'string' } }, + $defs: { type: 'object', properties: { x: { type: 'number' } } }, + tag: { $ref: '#/$defs/Tag' } + }, + required: ['definitions', '$defs'], + $defs: { + Tag: { type: 'object', properties: { label: { type: 'string' } } } + } + }; + expect(dereferenceLocalRefs(schema)).toEqual({ + type: 'object', + properties: { + definitions: { type: 'array', items: { type: 'string' } }, + $defs: { type: 'object', properties: { x: { type: 'number' } } }, + tag: { type: 'object', properties: { label: { type: 'string' } } } + }, + required: ['definitions', '$defs'] + }); + }); +}); diff --git a/test/integration/test/server/mcp.test.ts b/test/integration/test/server/mcp.test.ts index 92af09744..447ec72e7 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -5169,6 +5169,222 @@ describe('Zod v4', () => { }); }); + // https://github.com/anthropics/claude-code/issues/18260 + // Tool inputSchema must not contain $ref — LLMs cannot resolve JSON Schema + // references and will stringify object parameters instead of passing objects. + describe('Tool inputSchema should not contain $ref', () => { + // Track schemas registered in globalRegistry so we can clean up + const registeredSchemas: z.core.$ZodType[] = []; + afterEach(() => { + for (const schema of registeredSchemas) { + z.globalRegistry.remove(schema); + } + registeredSchemas.length = 0; + }); + + function registerInGlobal(schema: T, meta: { id: string }): T { + z.globalRegistry.add(schema, meta); + registeredSchemas.push(schema); + return schema; + } + + test('registered types should be inlined in schema and callable', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + const Address = registerInGlobal(z.object({ street: z.string(), city: z.string() }), { id: 'Address' }); + + server.registerTool('update-address', { inputSchema: z.object({ home: Address, work: Address }) }, async args => ({ + content: [{ type: 'text' as const, text: `home: ${args.home.city}, work: ${args.work.city}` }] + })); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + + // Schema invariant: no $ref in the schema sent to clients + const { tools } = await client.request({ method: 'tools/list' }); + const schema = tools[0]!.inputSchema; + expect(JSON.stringify(schema)).not.toContain('$ref'); + expect(JSON.stringify(schema)).not.toContain('$defs'); + expect(schema.properties!['home']).toMatchObject({ + type: 'object', + properties: { street: { type: 'string' }, city: { type: 'string' } } + }); + + // Runtime invariant: callTool with object args should succeed + const result = await client.callTool({ + name: 'update-address', + arguments: { + home: { street: '123 Main St', city: 'Springfield' }, + work: { street: '456 Oak Ave', city: 'Shelbyville' } + } + }); + expect(result.content).toEqual([{ type: 'text', text: 'home: Springfield, work: Shelbyville' }]); + }); + + test('discriminatedUnion with registered types should be inlined and callable', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + const CreateOp = z.object({ type: z.literal('create'), file_text: z.string() }); + const AppendOp = z.object({ type: z.literal('append'), new_str: z.string() }); + registerInGlobal(CreateOp, { id: 'CreateOp' }); + registerInGlobal(AppendOp, { id: 'AppendOp' }); + + server.registerTool( + 'write-file', + { + inputSchema: z.object({ + path: z.string(), + operation: z.discriminatedUnion('type', [CreateOp, AppendOp]) + }) + }, + async args => ({ + content: [{ type: 'text' as const, text: `${args.operation.type}: ${args.path}` }] + }) + ); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + + // Schema invariant: oneOf variants should be inline objects, not $ref + const { tools } = await client.request({ method: 'tools/list' }); + const schema = tools[0]!.inputSchema; + expect(JSON.stringify(schema)).not.toContain('$ref'); + expect(JSON.stringify(schema)).not.toContain('$defs'); + const operation = schema.properties!['operation'] as Record; + const variants = (operation['oneOf'] ?? operation['anyOf']) as Array>; + expect(variants).toBeDefined(); + expect(variants.length).toBe(2); + expect(variants[0]).toHaveProperty('type', 'object'); + expect(variants[1]).toHaveProperty('type', 'object'); + + // Runtime invariant: callTool with object operation should succeed + // This is exactly the case that fails when LLMs stringify $ref params + const result = await client.callTool({ + name: 'write-file', + arguments: { + path: '/tmp/test.md', + operation: { type: 'create', file_text: 'hello world' } + } + }); + expect(result.content).toEqual([{ type: 'text', text: 'create: /tmp/test.md' }]); + }); + + test('mixed $ref and inline params in same tool should both work', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + // Reproduces the original Notion MCP bug: one param uses $ref, + // another uses inline type: object — only the $ref param gets stringified + const ParentRequest = z.object({ database_id: z.string() }); + registerInGlobal(ParentRequest, { id: 'ParentRequest' }); + + server.registerTool( + 'create-page', + { + inputSchema: z.object({ + parent: ParentRequest, + properties: z.object({ + title: z.string() + }) + }) + }, + async args => ({ + content: [{ type: 'text' as const, text: `db: ${args.parent.database_id}, title: ${args.properties.title}` }] + }) + ); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + + const { tools } = await client.request({ method: 'tools/list' }); + const schema = tools[0]!.inputSchema; + + // Both params should be inline objects + expect(JSON.stringify(schema)).not.toContain('$ref'); + expect(schema.properties!['parent']).toMatchObject({ type: 'object' }); + expect(schema.properties!['properties']).toMatchObject({ type: 'object' }); + + const result = await client.callTool({ + name: 'create-page', + arguments: { + parent: { database_id: '2275ad9e-1234' }, + properties: { title: 'Test Page' } + } + }); + expect(result.content).toEqual([{ type: 'text', text: 'db: 2275ad9e-1234, title: Test Page' }]); + }); + + test('$ref pointing to oneOf union should be inlined', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + // Notion-style: $defs contains a oneOf union referenced via $ref + const ParentRequest = z.union([z.object({ database_id: z.string() }), z.object({ page_id: z.string() })]); + registerInGlobal(ParentRequest, { id: 'ParentRequest' }); + + server.registerTool( + 'create-item', + { + inputSchema: z.object({ parent: ParentRequest }) + }, + async _args => ({ + content: [{ type: 'text' as const, text: 'created' }] + }) + ); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + + const { tools } = await client.request({ method: 'tools/list' }); + const schema = tools[0]!.inputSchema; + expect(JSON.stringify(schema)).not.toContain('$ref'); + expect(JSON.stringify(schema)).not.toContain('$defs'); + + // The inlined parent should have the union variants directly + const parent = schema.properties!['parent'] as Record; + const variants = (parent['oneOf'] ?? parent['anyOf']) as Array>; + expect(variants).toBeDefined(); + expect(variants.length).toBe(2); + + const result = await client.callTool({ + name: 'create-item', + arguments: { parent: { database_id: 'abc-123' } } + }); + expect(result.content).toEqual([{ type: 'text', text: 'created' }]); + }); + + test('recursive types should preserve $ref and $defs on tools/list', async () => { + const server = new McpServer({ name: 'test', version: '1.0.0' }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + + const TreeNode: z.ZodType = z.object({ + value: z.string(), + children: z.lazy(() => z.array(TreeNode)) + }); + + server.registerTool('process-tree', { inputSchema: z.object({ root: TreeNode }) }, async () => ({ + content: [{ type: 'text' as const, text: 'processed' }] + })); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + + // Recursive schemas should NOT throw — cyclic $ref stays in place + const { tools } = await client.request({ method: 'tools/list' }); + const schema = tools[0]!.inputSchema; + // Cyclic $ref is preserved (same as pre-PR behavior — no regression) + expect(JSON.stringify(schema)).toContain('$ref'); + expect(schema.$defs || schema.definitions).toBeDefined(); + }); + }); + describe('Tools with transformation schemas', () => { test('should support z.preprocess() schemas', async () => { const server = new McpServer({