From 35589570bef9565b918f9c2d0894ba3e33ccdf80 Mon Sep 17 00:00:00 2001 From: gemini-cli-robot Date: Tue, 28 Apr 2026 00:17:23 +0000 Subject: [PATCH] # Fix 400 error when more than 128 tools are enabled ## What the change is - Implemented `smartLimitTools` in `ToolRegistry` to ensure that `getFunctionDeclarations` returns at most 128 tools (the Gemini API limit). - The "smart" limiting strategy: 1. Always prioritizes built-in tools. 2. Prioritizes non-MCP discovered tools. 3. Fairly distributes the remaining slots among all available MCP servers (round-robin style). - Added a hard limit of 128 tools in `GeminiChat` as a final safety measure before calling the API. - Added warning logs when tools are truncated to inform the user. ## Why it is recommended The Gemini API has a hard limit of 128 tools per request. If more than 128 tools are enabled (e.g., multiple MCP servers with many tools), the CLI currently sends all of them, resulting in a 400 Bad Request error from the API. This change prevents those errors while ensuring that a representative and critical set of tools remains available to the model. ## Expected impact - Fixes the 400 error reported in #24246. - Improves reliability when many MCP servers are connected. - No negative impact on existing functionality, as most users have fewer than 128 tools, and for those who have more, it provides a stable fallback instead of a crash. --- packages/core/src/core/geminiChat.test.ts | 48 ++++++++++ packages/core/src/core/geminiChat.ts | 44 +++++++++ packages/core/src/tools/tool-registry.test.ts | 57 ++++++++++++ packages/core/src/tools/tool-registry.ts | 89 +++++++++++++++++-- 4 files changed, 230 insertions(+), 8 deletions(-) diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 1a190fde2d..afa3a80969 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -1150,6 +1150,54 @@ describe('GeminiChat', () => { }); }); + describe('tool limiting safety net', () => { + it('should truncate tools to 128 as a safety net in generateContent', async () => { + const tools = Array.from({ length: 150 }, (_, i) => ({ + functionDeclarations: [{ name: `tool_${i}`, description: `tool ${i}` }], + })); + + chat.setTools(tools); + + // We need to mock the generator response to avoid actual API calls + vi.mocked(mockContentGenerator.generateContentStream).mockResolvedValue( + (async function* () { + yield { + candidates: [ + { + content: { parts: [{ text: 'Response' }] }, + finishReason: 'STOP', + }, + ], + } as any; + })(), + ); + + const stream = await chat.sendMessageStream( + { model: 'test-model' }, + 'hello', + 'test-id', + new AbortController().signal, + LlmRole.MAIN, + ); + for await (const _ of stream) { + // consume stream + } + + const lastCall = + vi.mocked(mockContentGenerator.generateContentStream).mock.calls[0]; + const callConfig = lastCall?.[0]?.config; + + let totalFunctions = 0; + for (const t of callConfig?.tools ?? []) { + if ('functionDeclarations' in t && t.functionDeclarations) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + totalFunctions += (t.functionDeclarations as any).length; + } + } + expect(totalFunctions).toBe(128); + }); + }); + describe('sendMessageStream with retries', () => { it('should yield a RETRY event when an invalid stream is encountered', async () => { // ARRANGE: Mock the stream to fail once, then succeed. diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index f6ae67e725..708b5e434c 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -50,6 +50,7 @@ import { handleFallback } from '../fallback/handler.js'; import { isFunctionResponse } from '../utils/messageInspectors.js'; import { partListUnionToString } from './geminiRequest.js'; import type { ModelConfigKey } from '../services/modelConfigService.js'; +import { debugLogger } from '../utils/debugLogger.js'; import { estimateTokenCountSync } from '../utils/tokenCalculation.js'; import { applyModelSelection, @@ -651,6 +652,49 @@ export class GeminiChat { } } + // Enforce 128 tool limit for Gemini API. + const MAX_TOOLS = 128; + if (config.tools && config.tools.length > 0) { + let totalTools = 0; + for (const tool of config.tools) { + if ( + 'functionDeclarations' in tool && + Array.isArray(tool.functionDeclarations) + ) { + totalTools += tool.functionDeclarations.length; + } + } + + if (totalTools > MAX_TOOLS) { + debugLogger.warn( + `Total tools exceed Gemini API limit of ${MAX_TOOLS} (found ${totalTools}). Truncating.`, + ); + // Truncate function declarations to fit within the limit. + let remainingSlots = MAX_TOOLS; + const limitedTools: Tool[] = []; + for (const tool of config.tools) { + if ( + 'functionDeclarations' in tool && + Array.isArray(tool.functionDeclarations) + ) { + const declarations = tool.functionDeclarations.slice( + 0, + remainingSlots, + ); + if (declarations.length > 0) { + limitedTools.push({ functionDeclarations: declarations }); + } + remainingSlots -= declarations.length; + } else { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + limitedTools.push(tool as Tool); + } + if (remainingSlots <= 0) break; + } + config.tools = limitedTools; + } + } + if (this.onModelChanged) { this.tools = await this.onModelChanged(modelToUse); } diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 0f1e79ca25..60b13c9a34 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -889,6 +889,63 @@ describe('ToolRegistry', () => { expect(description).toBe(JSON.stringify(params)); }); }); + + describe('tool limiting', () => { + it('should limit tools to 128 and prioritize built-in tools', async () => { + const config = new Config({ targetDir: '/tmp' } as any); + const registry = new ToolRegistry(config, mockMessageBusForHelper, true); + + // Add 150 tools: 30 built-in, 120 MCP tools across 2 servers + for (let i = 0; i < 30; i++) { + const tool = new MockTool({ + name: `builtin_${i}`, + description: `Built-in tool ${i}`, + }); + registry.registerTool(tool); + } + + for (let i = 0; i < 60; i++) { + const tool = createMCPTool( + 'server1', + `mcp1_${i}`, + `MCP 1 tool ${i}`, + createMockCallableTool([{ name: `mcp1_${i}`, description: '' }]), + ); + registry.registerTool(tool); + } + + for (let i = 0; i < 60; i++) { + const tool = createMCPTool( + 'server2', + `mcp2_${i}`, + `MCP 2 tool ${i}`, + createMockCallableTool([{ name: `mcp2_${i}`, description: '' }]), + ); + registry.registerTool(tool); + } + + const declarations = registry.getFunctionDeclarations(); + expect(declarations.length).toBe(128); + + // Verify built-in tools are present + for (let i = 0; i < 30; i++) { + expect( + declarations.some((d) => d.name && d.name === `builtin_${i}`), + ).toBe(true); + } + + // Verify MCP tools are fairly distributed (approx 49 each from server1 and server2) + // 128 total - 30 built-in = 98 MCP slots. 98 / 2 = 49 per server. + const s1Count = declarations.filter( + (d) => d.name && d.name.includes('mcp1_'), + ).length; + const s2Count = declarations.filter( + (d) => d.name && d.name.includes('mcp2_'), + ).length; + expect(s1Count).toBe(49); + expect(s2Count).toBe(49); + }); + }); }); /** diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index ea21a5dc3e..c0b2f1bf12 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -648,29 +648,46 @@ export class ToolRegistry { const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN; const plansDir = this.config.storage.getPlansDir(); - const declarations: FunctionDeclaration[] = []; - const seenNames = new Set(); - const mainAgentTools = this.isMainRegistry ? this.config.getMainAgentTools() : undefined; - this.getActiveTools().forEach((tool) => { + const activeTools = this.getActiveTools().filter((tool) => { const toolName = tool instanceof DiscoveredMCPTool ? tool.getFullyQualifiedName() : tool.name; - if (seenNames.has(toolName)) { - return; - } - if ( mainAgentTools && !mainAgentTools.includes(toolName) && !mainAgentTools.includes(tool.constructor.name) && !mainAgentTools.some((t) => t.startsWith(`${tool.constructor.name}(`)) ) { + return false; + } + return true; + }); + + const MAX_TOOLS = 128; + let toolsToInclude = activeTools; + if (activeTools.length > MAX_TOOLS) { + toolsToInclude = this.smartLimitTools(activeTools, MAX_TOOLS); + debugLogger.warn( + `More than ${MAX_TOOLS} tools available (${activeTools.length}). Smart-limited to ${MAX_TOOLS}.`, + ); + } + + const declarations: FunctionDeclaration[] = []; + const seenNames = new Set(); + + toolsToInclude.forEach((tool) => { + const toolName = + tool instanceof DiscoveredMCPTool + ? tool.getFullyQualifiedName() + : tool.name; + + if (seenNames.has(toolName)) { return; } @@ -700,6 +717,62 @@ export class ToolRegistry { return declarations; } + /** + * Smartly limits the number of tools to fit within the Gemini API limit. + * Priority: + * 1. Built-in tools. + * 2. Discovered tools (non-MCP). + * 3. MCP tools (fairly distributed among servers). + */ + private smartLimitTools( + tools: AnyDeclarativeTool[], + limit: number, + ): AnyDeclarativeTool[] { + const builtIn = tools.filter( + (t) => !(t instanceof DiscoveredTool) && !(t instanceof DiscoveredMCPTool), + ); + const discovered = tools.filter((t) => t instanceof DiscoveredTool); + const mcp = tools.filter( + (t) => t instanceof DiscoveredMCPTool, + ) as DiscoveredMCPTool[]; + + const result: AnyDeclarativeTool[] = [...builtIn, ...discovered]; + if (result.length >= limit) { + return result.slice(0, limit); + } + + const remaining = limit - result.length; + // Distribute remaining slots among MCP servers + const mcpByServer = new Map(); + for (const t of mcp) { + const list = mcpByServer.get(t.serverName) ?? []; + list.push(t); + mcpByServer.set(t.serverName, list); + } + + const serverNames = Array.from(mcpByServer.keys()); + if (serverNames.length === 0) return result; + + let added = 0; + let index = 0; + while (added < remaining) { + let anyAddedInThisRound = false; + for (const serverName of serverNames) { + const serverTools = mcpByServer.get(serverName)!; + if (index < serverTools.length) { + result.push(serverTools[index]); + added++; + anyAddedInThisRound = true; + if (added >= remaining) break; + } + } + if (!anyAddedInThisRound) break; + index++; + } + + return result; + } + /** * Retrieves a filtered list of tool schemas based on a list of tool names. * @param toolNames - An array of tool names to include.