Detect and warn about cyclic tool refs when schema depth errors are encountered (#5609)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Jacob MacDonald
2025-08-05 18:48:00 -07:00
committed by GitHub
parent 9db5aab498
commit 7e5a5e2da7
3 changed files with 249 additions and 0 deletions

View File

@@ -0,0 +1,125 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { hasCycleInSchema } from './tools.js'; // Added getStringifiedResultForDisplay
describe('hasCycleInSchema', () => {
it('should detect a simple direct cycle', () => {
const schema = {
properties: {
data: {
$ref: '#/properties/data',
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle from object properties referencing parent properties', () => {
const schema = {
type: 'object',
properties: {
data: {
type: 'object',
properties: {
child: { $ref: '#/properties/data' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle from array items referencing parent properties', () => {
const schema = {
type: 'object',
properties: {
data: {
type: 'array',
items: {
type: 'object',
properties: {
child: { $ref: '#/properties/data/items' },
},
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should detect a cycle between sibling properties', () => {
const schema = {
type: 'object',
properties: {
a: {
type: 'object',
properties: {
child: { $ref: '#/properties/b' },
},
},
b: {
type: 'object',
properties: {
child: { $ref: '#/properties/a' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(true);
});
it('should not detect a cycle in a valid schema', () => {
const schema = {
type: 'object',
properties: {
name: { type: 'string' },
address: { $ref: '#/definitions/address' },
},
definitions: {
address: {
type: 'object',
properties: {
street: { type: 'string' },
city: { type: 'string' },
},
},
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should handle non-cyclic sibling refs', () => {
const schema = {
properties: {
a: { $ref: '#/definitions/stringDef' },
b: { $ref: '#/definitions/stringDef' },
},
definitions: {
stringDef: { type: 'string' },
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should handle nested but not cyclic refs', () => {
const schema = {
properties: {
a: { $ref: '#/definitions/defA' },
},
definitions: {
defA: { properties: { b: { $ref: '#/definitions/defB' } } },
defB: { type: 'string' },
},
};
expect(hasCycleInSchema(schema)).toBe(false);
});
it('should return false for an empty schema', () => {
expect(hasCycleInSchema({})).toBe(false);
});
});

View File

@@ -228,6 +228,91 @@ export interface ToolResult {
};
}
/**
* Detects cycles in a JSON schemas due to `$ref`s.
* @param schema The root of the JSON schema.
* @returns `true` if a cycle is detected, `false` otherwise.
*/
export function hasCycleInSchema(schema: object): boolean {
function resolveRef(ref: string): object | null {
if (!ref.startsWith('#/')) {
return null;
}
const path = ref.substring(2).split('/');
let current: unknown = schema;
for (const segment of path) {
if (
typeof current !== 'object' ||
current === null ||
!Object.prototype.hasOwnProperty.call(current, segment)
) {
return null;
}
current = (current as Record<string, unknown>)[segment];
}
return current as object;
}
function traverse(
node: unknown,
visitedRefs: Set<string>,
pathRefs: Set<string>,
): boolean {
if (typeof node !== 'object' || node === null) {
return false;
}
if (Array.isArray(node)) {
for (const item of node) {
if (traverse(item, visitedRefs, pathRefs)) {
return true;
}
}
return false;
}
if ('$ref' in node && typeof node.$ref === 'string') {
const ref = node.$ref;
if (ref === '#/' || pathRefs.has(ref)) {
// A ref to just '#/' is always a cycle.
return true; // Cycle detected!
}
if (visitedRefs.has(ref)) {
return false; // Bail early, we have checked this ref before.
}
const resolvedNode = resolveRef(ref);
if (resolvedNode) {
// Add it to both visited and the current path
visitedRefs.add(ref);
pathRefs.add(ref);
const hasCycle = traverse(resolvedNode, visitedRefs, pathRefs);
pathRefs.delete(ref); // Backtrack, leaving it in visited
return hasCycle;
}
}
// Crawl all the properties of node
for (const key in node) {
if (Object.prototype.hasOwnProperty.call(node, key)) {
if (
traverse(
(node as Record<string, unknown>)[key],
visitedRefs,
pathRefs,
)
) {
return true;
}
}
}
return false;
}
return traverse(schema, new Set<string>(), new Set<string>());
}
export type ToolResultDisplay = string | FileDiff;
export interface FileDiff {