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

Co-authored-by: Jacob Richman <[email protected]>

3 files changed, 249 insertions, 0 deletions

diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts
index bd81400f..c0e41b5e 100644
--- a/packages/core/src/core/geminiChat.ts
+++ b/packages/core/src/core/geminiChat.ts
@@ -32,6 +32,8 @@ import {
   ApiResponseEvent,
 } from '../telemetry/types.js';
 import { DEFAULT_GEMINI_FLASH_MODEL } from '../config/models.js';
+import { hasCycleInSchema } from '../tools/tools.js';
+import { isStructuredError } from '../utils/quotaErrorDetection.js';
 
 /**
  * Returns true if the response is valid, false otherwise.
@@ -299,6 +301,10 @@ export class GeminiChat {
 
       response = await retryWithBackoff(apiCall, {
         shouldRetry: (error: Error) => {
+          // Check for likely cyclic schema errors, don't retry those.
+          if (error.message.includes('maximum schema depth exceeded'))
+            return false;
+          // Check error messages for status codes, or specific error names if known
           if (error && error.message) {
             if (error.message.includes('429')) return true;
             if (error.message.match(/5\d{2}/)) return true;
@@ -345,6 +351,7 @@ export class GeminiChat {
     } catch (error) {
       const durationMs = Date.now() - startTime;
       this._logApiError(durationMs, error, prompt_id);
+      await this.maybeIncludeSchemaDepthContext(error);
       this.sendPromise = Promise.resolve();
       throw error;
     }
@@ -413,6 +420,9 @@ export class GeminiChat {
       // If errors occur mid-stream, this setup won't resume the stream; it will restart it.
       const streamResponse = await retryWithBackoff(apiCall, {
         shouldRetry: (error: Error) => {
+          // Check for likely cyclic schema errors, don't retry those.
+          if (error.message.includes('maximum schema depth exceeded'))
+            return false;
           // Check error messages for status codes, or specific error names if known
           if (error && error.message) {
             if (error.message.includes('429')) return true;
@@ -443,6 +453,7 @@ export class GeminiChat {
       const durationMs = Date.now() - startTime;
       this._logApiError(durationMs, error, prompt_id);
       this.sendPromise = Promise.resolve();
+      await this.maybeIncludeSchemaDepthContext(error);
       throw error;
     }
   }
@@ -674,4 +685,32 @@ export class GeminiChat {
       content.parts[0].thought === true
     );
   }
+
+  private async maybeIncludeSchemaDepthContext(error: unknown): Promise<void> {
+    // Check for potentially problematic cyclic tools with cyclic schemas
+    // and include a recommendation to remove potentially problematic tools.
+    if (
+      isStructuredError(error) &&
+      error.message.includes('maximum schema depth exceeded')
+    ) {
+      const tools = (await this.config.getToolRegistry()).getAllTools();
+      const cyclicSchemaTools: string[] = [];
+      for (const tool of tools) {
+        if (
+          (tool.schema.parametersJsonSchema &&
+            hasCycleInSchema(tool.schema.parametersJsonSchema)) ||
+          (tool.schema.parameters && hasCycleInSchema(tool.schema.parameters))
+        ) {
+          cyclicSchemaTools.push(tool.displayName);
+        }
+      }
+      if (cyclicSchemaTools.length > 0) {
+        const extraDetails =
+          `\n\nThis error was probably caused by cyclic schema references in one of the following tools, try disabling them:\n\n - ` +
+          cyclicSchemaTools.join(`\n - `) +
+          `\n`;
+        error.message += extraDetails;
+      }
+    }
+  }
 }
diff --git a/packages/core/src/tools/tools.test.ts b/packages/core/src/tools/tools.test.ts
new file mode 100644
index 00000000..9942d3a9
--- /dev/null
+++ b/packages/core/src/tools/tools.test.ts
@@ -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);
+  });
+});
diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts
index 0e3ffabf..5d9d9253 100644
--- a/packages/core/src/tools/tools.ts
+++ b/packages/core/src/tools/tools.ts
@@ -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 {