Skip to content

Commit 0b26088

Browse files
committed
fix: clone visited-refs set to prevent false circular-ref detection across sibling branches
The shared visitedRefs Set was mutated in place, so when two sibling properties referenced the same $def (e.g. billing and shipping both referencing Address), the second resolution was incorrectly flagged as circular. Clone the set before recursing so each branch has its own ancestry path. Added regression test that fails without this fix.
1 parent 18552b5 commit 0b26088

2 files changed

Lines changed: 90 additions & 2 deletions

File tree

packages/shared/src/utils/__tests__/json-schema.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,92 @@ describe("convertJsonSchemaToZodSchema", () => {
279279
warnSpy.mockRestore();
280280
});
281281

282+
it("should resolve the same $ref used by multiple sibling properties", () => {
283+
// Two properties reference the same $def — the visited set must NOT
284+
// mark the second usage as circular.
285+
const jsonSchema = {
286+
type: "object",
287+
properties: {
288+
billing: { $ref: "#/$defs/Address" },
289+
shipping: { $ref: "#/$defs/Address" },
290+
},
291+
required: ["billing", "shipping"],
292+
$defs: {
293+
Address: {
294+
type: "object",
295+
properties: {
296+
street: { type: "string", description: "Street" },
297+
city: { type: "string", description: "City" },
298+
},
299+
required: ["street", "city"],
300+
description: "An address",
301+
},
302+
},
303+
};
304+
305+
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
306+
307+
const result = convertJsonSchemaToZodSchema(jsonSchema, true);
308+
const shape = (result as z.ZodObject<any>).shape;
309+
310+
// Both should be fully resolved objects, NOT z.any()
311+
expect(shape.billing._def.typeName).toBe("ZodObject");
312+
expect(shape.shipping._def.typeName).toBe("ZodObject");
313+
314+
// No circular-ref warning should have been emitted
315+
expect(warnSpy).not.toHaveBeenCalled();
316+
317+
warnSpy.mockRestore();
318+
});
319+
320+
it("should resolve a shared $ref used in different branches of a $ref chain", () => {
321+
// Wrapper -> Container (via $ref) which has two children both using $ref to Leaf.
322+
// Without set cloning, the second Leaf ref in Container would be wrongly flagged
323+
// as circular because the first Leaf resolution already added it to the set.
324+
const jsonSchema = {
325+
type: "object",
326+
properties: {
327+
wrapper: { $ref: "#/$defs/Container" },
328+
},
329+
required: ["wrapper"],
330+
$defs: {
331+
Container: {
332+
type: "object",
333+
properties: {
334+
first: { $ref: "#/$defs/Leaf" },
335+
second: { $ref: "#/$defs/Leaf" },
336+
},
337+
required: ["first", "second"],
338+
description: "A container with two leaves",
339+
},
340+
Leaf: {
341+
type: "object",
342+
properties: {
343+
label: { type: "string", description: "Leaf label" },
344+
},
345+
required: ["label"],
346+
description: "A leaf node",
347+
},
348+
},
349+
};
350+
351+
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
352+
353+
const result = convertJsonSchemaToZodSchema(jsonSchema, true);
354+
const wrapperShape = (
355+
(result as z.ZodObject<any>).shape.wrapper as z.ZodObject<any>
356+
).shape;
357+
358+
// Both first and second should be fully resolved Leaf objects
359+
expect(wrapperShape.first._def.typeName).toBe("ZodObject");
360+
expect(wrapperShape.second._def.typeName).toBe("ZodObject");
361+
362+
// No circular-ref warning should have been emitted
363+
expect(warnSpy).not.toHaveBeenCalled();
364+
365+
warnSpy.mockRestore();
366+
});
367+
282368
it("should handle anyOf with $ref variants", () => {
283369
const jsonSchema = {
284370
type: "object",

packages/shared/src/utils/json-schema.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,12 +266,14 @@ export function convertJsonSchemaToZodSchema(
266266

267267
const resolved = definitions[refPath];
268268
if (resolved) {
269-
refs.add(refPath);
269+
// Clone the set so sibling branches don't see each other's visited refs
270+
const nextRefs = new Set(refs);
271+
nextRefs.add(refPath);
270272
return convertJsonSchemaToZodSchema(
271273
resolved,
272274
required,
273275
definitions,
274-
refs,
276+
nextRefs,
275277
);
276278
}
277279
}

0 commit comments

Comments
 (0)