From 19dc40825e9ec1a9bc9d5eed3adb185a49addf76 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Fri, 6 Feb 2026 16:49:25 -0800 Subject: [PATCH] fix(core): expand excludeTools with legacy aliases for renamed tools (#18498) --- packages/core/src/tools/tool-registry.test.ts | 39 +++++++++++++++++-- packages/core/src/tools/tool-registry.ts | 34 ++++++++++++++-- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 73bb351f7a..c26349f50f 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -84,11 +84,24 @@ vi.mock('@google/genai', async () => { // Mock tool-names to provide a consistent alias for testing vi.mock('./tool-names.js', async (importOriginal) => { const actual = await importOriginal(); + const mockedAliases: Record = { + ...actual.TOOL_LEGACY_ALIASES, + legacy_test_tool: 'current_test_tool', + }; return { ...actual, - TOOL_LEGACY_ALIASES: { - ...actual.TOOL_LEGACY_ALIASES, - legacy_test_tool: 'current_test_tool', + TOOL_LEGACY_ALIASES: mockedAliases, + // Override getToolAliases to use the mocked aliases map + getToolAliases: (name: string): string[] => { + const aliases = new Set([name]); + const canonicalName = mockedAliases[name] ?? name; + aliases.add(canonicalName); + for (const [legacyName, currentName] of Object.entries(mockedAliases)) { + if (currentName === canonicalName) { + aliases.add(legacyName); + } + } + return Array.from(aliases); }, }; }); @@ -290,6 +303,26 @@ describe('ToolRegistry', () => { tools: [excludedTool], excludedTools: ['ExcludedMockTool'], }, + { + name: 'should exclude a tool when its legacy alias is in excludeTools', + tools: [ + new MockTool({ + name: 'current_test_tool', + displayName: 'Current Test Tool', + }), + ], + excludedTools: ['legacy_test_tool'], + }, + { + name: 'should exclude a tool when its current name is in excludeTools and tool is registered under current name', + tools: [ + new MockTool({ + name: 'current_test_tool', + displayName: 'Current Test Tool', + }), + ], + excludedTools: ['current_test_tool'], + }, ])('$name', ({ tools, excludedTools }) => { toolRegistry.registerTool(allowedTool); for (const tool of tools) { diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 9da0932cde..ae4278986b 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -21,7 +21,11 @@ import { safeJsonStringify } from '../utils/safeJsonStringify.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents } from '../utils/events.js'; -import { DISCOVERED_TOOL_PREFIX, TOOL_LEGACY_ALIASES } from './tool-names.js'; +import { + DISCOVERED_TOOL_PREFIX, + TOOL_LEGACY_ALIASES, + getToolAliases, +} from './tool-names.js'; type ToolParams = Record; @@ -431,7 +435,9 @@ export class ToolRegistry { * @returns All the tools that are not excluded. */ private getActiveTools(): AnyDeclarativeTool[] { - const excludedTools = this.config.getExcludeTools() ?? new Set([]); + const excludedTools = + this.expandExcludeToolsWithAliases(this.config.getExcludeTools()) ?? + new Set([]); const activeTools: AnyDeclarativeTool[] = []; for (const tool of this.allKnownTools.values()) { if (this.isActiveTool(tool, excludedTools)) { @@ -441,6 +447,26 @@ export class ToolRegistry { return activeTools; } + /** + * Expands an excludeTools set to include all legacy aliases. + * For example, if 'search_file_content' is excluded and it's an alias for + * 'grep_search', both names will be in the returned set. + */ + private expandExcludeToolsWithAliases( + excludeTools: Set | undefined, + ): Set | undefined { + if (!excludeTools || excludeTools.size === 0) { + return excludeTools; + } + const expanded = new Set(); + for (const name of excludeTools) { + for (const alias of getToolAliases(name)) { + expanded.add(alias); + } + } + return expanded; + } + /** * @param tool * @param excludeTools (optional, helps performance for repeated calls) @@ -450,7 +476,9 @@ export class ToolRegistry { tool: AnyDeclarativeTool, excludeTools?: Set, ): boolean { - excludeTools ??= this.config.getExcludeTools() ?? new Set([]); + excludeTools ??= + this.expandExcludeToolsWithAliases(this.config.getExcludeTools()) ?? + new Set([]); const normalizedClassName = tool.constructor.name.replace(/^_+/, ''); const possibleNames = [tool.name, normalizedClassName]; if (tool instanceof DiscoveredMCPTool) {