Skip to content

Commit 6fdbc11

Browse files
committed
fix: resolve $ref siblings defensively, preserve property names definitions/$defs
Address review feedback from PR #1563: 1. Defensive: resolve sibling values through resolve() before merging onto the resolved $ref schema. No known generator triggers this (Zod/ArkType/Valibot only produce metadata siblings), but it makes the sibling-merge and object-traversal paths consistent. 2. Bug fix: only strip $defs/definitions keys at the root schema level. Previously the filter fired at every depth, silently dropping any property named 'definitions' or '$defs' from nested objects. Tests added for both fixes.
1 parent 62b23cb commit 6fdbc11

2 files changed

Lines changed: 54 additions & 2 deletions

File tree

packages/core/src/util/schema.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,15 @@ export function dereferenceLocalRefs(schema: Record<string, unknown>): Record<st
102102

103103
// Merge sibling keywords onto the resolved schema
104104
if (hasSiblings && resolved !== null && typeof resolved === 'object' && !Array.isArray(resolved)) {
105-
return { ...(resolved as Record<string, unknown>), ...siblings };
105+
const resolvedSiblings = Object.fromEntries(Object.entries(siblings).map(([k, v]) => [k, resolve(v, stack)]));
106+
return { ...(resolved as Record<string, unknown>), ...resolvedSiblings };
106107
}
107108
return resolved;
108109
}
109110

110111
const result: Record<string, unknown> = {};
111112
for (const [key, value] of Object.entries(obj)) {
112-
if (key === '$defs' || key === 'definitions') continue;
113+
if (obj === schema && (key === '$defs' || key === 'definitions')) continue;
113114
result[key] = resolve(value, stack);
114115
}
115116
return result;

packages/core/test/schema.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,4 +196,55 @@ describe('dereferenceLocalRefs', () => {
196196
expect(hq['type']).toBe('object');
197197
expect(hq['properties']).toEqual({ street: { type: 'string' } });
198198
});
199+
200+
// Defensive: hand-crafted synthetic schema — no known schema generator (Zod v4,
201+
// ArkType, Valibot) produces $ref with a sibling containing nested $ref.
202+
// Generators put $ref inside allOf/anyOf/oneOf array elements (resolved by
203+
// array recursion), not as siblings of $ref on the same node.
204+
// See: https://github.com/modelcontextprotocol/typescript-sdk/pull/1563#discussion_r3022304127
205+
test('$ref siblings containing nested $ref are resolved (defensive)', () => {
206+
const schema = {
207+
type: 'object',
208+
properties: { x: { $ref: '#/$defs/Outer' } },
209+
$defs: {
210+
Outer: { $ref: '#/$defs/Inner', allOf: [{ $ref: '#/$defs/Mixin' }] },
211+
Inner: { type: 'object' },
212+
Mixin: { title: 'mixin' }
213+
}
214+
};
215+
const result = dereferenceLocalRefs(schema);
216+
expect(JSON.stringify(result)).not.toContain('$ref');
217+
expect(JSON.stringify(result)).not.toContain('$defs');
218+
const x = (result['properties'] as Record<string, Record<string, unknown>>)['x']!;
219+
expect(x['type']).toBe('object');
220+
expect(x['allOf']).toEqual([{ title: 'mixin' }]);
221+
});
222+
223+
test('property named "definitions" is not stripped from nested objects', () => {
224+
const schema = {
225+
type: 'object',
226+
properties: {
227+
items: { type: 'array', items: { type: 'string' } },
228+
definitions: { type: 'array', items: { type: 'string' } }
229+
},
230+
required: ['items', 'definitions']
231+
};
232+
const result = dereferenceLocalRefs(schema);
233+
const props = result['properties'] as Record<string, unknown>;
234+
expect(props['definitions']).toEqual({ type: 'array', items: { type: 'string' } });
235+
expect(props['items']).toEqual({ type: 'array', items: { type: 'string' } });
236+
});
237+
238+
test('property named "$defs" is not stripped from nested objects', () => {
239+
const schema = {
240+
type: 'object',
241+
properties: {
242+
name: { type: 'string' },
243+
$defs: { type: 'object', properties: { x: { type: 'number' } } }
244+
}
245+
};
246+
const result = dereferenceLocalRefs(schema);
247+
const props = result['properties'] as Record<string, unknown>;
248+
expect(props['$defs']).toEqual({ type: 'object', properties: { x: { type: 'number' } } });
249+
});
199250
});

0 commit comments

Comments
 (0)