From 5e96373e6b4a66ce4d2351eab70991e2d190996b Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Mon, 2 Feb 2026 16:34:14 -0800 Subject: [PATCH] feat(core): implement tool name aliasing for backward compatibility (#17974) --- .../core/src/policy/policy-engine.test.ts | 83 +++++++++++++++++++ packages/core/src/policy/policy-engine.ts | 19 +++-- packages/core/src/tools/tool-names.test.ts | 60 +++++++++++++- packages/core/src/tools/tool-names.ts | 38 ++++++++- packages/core/src/tools/tool-registry.test.ts | 29 +++++++ packages/core/src/tools/tool-registry.ts | 14 +++- 6 files changed, 234 insertions(+), 9 deletions(-) diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 495ca5d145..dba06550d2 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -43,6 +43,43 @@ vi.mock('../utils/shell-utils.js', async (importOriginal) => { }; }); +// Mock tool-names to provide a consistent alias for testing + +vi.mock('../tools/tool-names.js', async (importOriginal) => { + const actual = + await importOriginal(); + + const mockedAliases: Record = { + ...actual.TOOL_LEGACY_ALIASES, + + legacy_test_tool: 'current_test_tool', + + another_legacy_test_tool: 'current_test_tool', + }; + + return { + ...actual, + + TOOL_LEGACY_ALIASES: mockedAliases, + + getToolAliases: vi.fn().mockImplementation((name: 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); + }), + }; +}); + describe('PolicyEngine', () => { let engine: PolicyEngine; let mockCheckerRunner: CheckerRunner; @@ -187,6 +224,52 @@ describe('PolicyEngine', () => { ); }); + it('should match current tool call against legacy tool name rules', async () => { + const legacyName = 'legacy_test_tool'; + const currentName = 'current_test_tool'; + + const rules: PolicyRule[] = [ + { toolName: legacyName, decision: PolicyDecision.DENY }, + ]; + + engine = new PolicyEngine({ rules }); + + // Call using the CURRENT name, should be denied because of legacy rule + const { decision } = await engine.check({ name: currentName }, undefined); + expect(decision).toBe(PolicyDecision.DENY); + }); + + it('should match legacy tool call against current tool name rules (for skills support)', async () => { + const legacyName = 'legacy_test_tool'; + const currentName = 'current_test_tool'; + + const rules: PolicyRule[] = [ + { toolName: currentName, decision: PolicyDecision.ALLOW }, + ]; + + engine = new PolicyEngine({ rules }); + + // Call using the LEGACY name (from a skill), should be allowed because of current rule + const { decision } = await engine.check({ name: legacyName }, undefined); + expect(decision).toBe(PolicyDecision.ALLOW); + }); + + it('should match tool call using one legacy name against policy for another legacy name (same canonical tool)', async () => { + const legacyName1 = 'legacy_test_tool'; + const legacyName2 = 'another_legacy_test_tool'; + + const rules: PolicyRule[] = [ + { toolName: legacyName2, decision: PolicyDecision.DENY }, + ]; + + engine = new PolicyEngine({ rules }); + + // Call using legacyName1, should be denied because legacyName2 has a deny rule + // and they both point to the same canonical tool. + const { decision } = await engine.check({ name: legacyName1 }, undefined); + expect(decision).toBe(PolicyDecision.DENY); + }); + it('should apply wildcard rules (no toolName)', async () => { const rules: PolicyRule[] = [ { decision: PolicyDecision.DENY }, // Applies to all tools diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index d617d3d75c..c0baf3e5c7 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -24,6 +24,7 @@ import { splitCommands, hasRedirection, } from '../utils/shell-utils.js'; +import { getToolAliases } from '../tools/tool-names.js'; function ruleMatches( rule: PolicyRule | SafetyCheckerRule, @@ -322,12 +323,18 @@ export class PolicyEngine { // For tools with a server name, we want to try matching both the // original name and the fully qualified name (server__tool). - const toolCallsToTry: FunctionCall[] = [toolCall]; - if (serverName && toolCall.name && !toolCall.name.includes('__')) { - toolCallsToTry.push({ - ...toolCall, - name: `${serverName}__${toolCall.name}`, - }); + // We also want to check legacy aliases for the tool name. + const toolNamesToTry = toolCall.name ? getToolAliases(toolCall.name) : []; + + const toolCallsToTry: FunctionCall[] = []; + for (const name of toolNamesToTry) { + toolCallsToTry.push({ ...toolCall, name }); + if (serverName && !name.includes('__')) { + toolCallsToTry.push({ + ...toolCall, + name: `${serverName}__${name}`, + }); + } } for (const rule of this.rules) { diff --git a/packages/core/src/tools/tool-names.test.ts b/packages/core/src/tools/tool-names.test.ts index 805727c283..344ff48376 100644 --- a/packages/core/src/tools/tool-names.test.ts +++ b/packages/core/src/tools/tool-names.test.ts @@ -4,14 +4,44 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { isValidToolName, + getToolAliases, ALL_BUILTIN_TOOL_NAMES, DISCOVERED_TOOL_PREFIX, LS_TOOL_NAME, } from './tool-names.js'; +// 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', + another_legacy_test_tool: 'current_test_tool', + }; + return { + ...actual, + TOOL_LEGACY_ALIASES: mockedAliases, + isValidToolName: vi.fn().mockImplementation((name: string, options) => { + if (mockedAliases[name]) return true; + return actual.isValidToolName(name, options); + }), + getToolAliases: vi.fn().mockImplementation((name: 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); + }), + }; +}); + describe('tool-names', () => { describe('isValidToolName', () => { it('should validate built-in tool names', () => { @@ -30,6 +60,13 @@ describe('tool-names', () => { expect(isValidToolName('my-server__my-tool')).toBe(true); }); + it('should validate legacy tool aliases', async () => { + const { TOOL_LEGACY_ALIASES } = await import('./tool-names.js'); + for (const legacyName of Object.keys(TOOL_LEGACY_ALIASES)) { + expect(isValidToolName(legacyName)).toBe(true); + } + }); + it('should reject invalid tool names', () => { expect(isValidToolName('')).toBe(false); expect(isValidToolName('invalid-name')).toBe(false); @@ -54,4 +91,25 @@ describe('tool-names', () => { ); }); }); + + describe('getToolAliases', () => { + it('should return all associated names for a current tool', () => { + const aliases = getToolAliases('current_test_tool'); + expect(aliases).toContain('current_test_tool'); + expect(aliases).toContain('legacy_test_tool'); + expect(aliases).toContain('another_legacy_test_tool'); + }); + + it('should return all associated names for a legacy tool', () => { + const aliases = getToolAliases('legacy_test_tool'); + expect(aliases).toContain('current_test_tool'); + expect(aliases).toContain('legacy_test_tool'); + expect(aliases).toContain('another_legacy_test_tool'); + }); + + it('should return only the name itself if no aliases exist', () => { + const aliases = getToolAliases('unknown_tool'); + expect(aliases).toEqual(['unknown_tool']); + }); + }); }); diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index ee3eb8f930..3ea35054d4 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -26,7 +26,38 @@ export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); export const ASK_USER_TOOL_NAME = 'ask_user'; export const ASK_USER_DISPLAY_NAME = 'Ask User'; -/** Prefix used for tools discovered via the toolDiscoveryCommand. */ +/** + * Mapping of legacy tool names to their current names. + * This ensures backward compatibility for user-defined policies, skills, and hooks. + */ +export const TOOL_LEGACY_ALIASES: Record = { + // Add future renames here, e.g.: + // 'search_file_content': GREP_TOOL_NAME, +}; + +/** + * Returns all associated names for a tool (including legacy aliases and current name). + * This ensures that if multiple legacy names point to the same tool, we consider all of them + * for policy application. + */ +export function getToolAliases(name: string): string[] { + const aliases = new Set([name]); + + // Determine the canonical (current) name + const canonicalName = TOOL_LEGACY_ALIASES[name] ?? name; + aliases.add(canonicalName); + + // Find all other legacy aliases that point to the same canonical name + for (const [legacyName, currentName] of Object.entries(TOOL_LEGACY_ALIASES)) { + if (currentName === canonicalName) { + aliases.add(legacyName); + } + } + + return Array.from(aliases); +} + +/** Prefix used for tools discovered via the tool DiscoveryCommand. */ export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_'; /** @@ -76,6 +107,11 @@ export function isValidToolName( return true; } + // Legacy aliases + if (TOOL_LEGACY_ALIASES[name]) { + return true; + } + // Discovered tools if (name.startsWith(DISCOVERED_TOOL_PREFIX)) { return true; diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index df4984595a..73bb351f7a 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -81,6 +81,18 @@ 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(); + return { + ...actual, + TOOL_LEGACY_ALIASES: { + ...actual.TOOL_LEGACY_ALIASES, + legacy_test_tool: 'current_test_tool', + }, + }; +}); + // Helper to create a mock CallableTool for specific test needs const createMockCallableTool = ( toolDeclarations: FunctionDeclaration[], @@ -584,6 +596,23 @@ describe('ToolRegistry', () => { expect(declarations).toHaveLength(1); expect(declarations[0].name).toBe(toolName); }); + + it('should retrieve a tool using its legacy alias', async () => { + const legacyName = 'legacy_test_tool'; + const currentName = 'current_test_tool'; + + const mockTool = new MockTool({ + name: currentName, + description: 'Test Tool', + messageBus: mockMessageBus, + }); + + toolRegistry.registerTool(mockTool); + + const retrievedTool = toolRegistry.getTool(legacyName); + expect(retrievedTool).toBeDefined(); + expect(retrievedTool?.name).toBe(currentName); + }); }); describe('DiscoveredToolInvocation', () => { diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index f9e5f8fa8d..9da0932cde 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -21,7 +21,7 @@ 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 } from './tool-names.js'; +import { DISCOVERED_TOOL_PREFIX, TOOL_LEGACY_ALIASES } from './tool-names.js'; type ToolParams = Record; @@ -531,6 +531,18 @@ export class ToolRegistry { */ getTool(name: string): AnyDeclarativeTool | undefined { let tool = this.allKnownTools.get(name); + + // If not found, check legacy aliases + if (!tool && TOOL_LEGACY_ALIASES[name]) { + const currentName = TOOL_LEGACY_ALIASES[name]; + tool = this.allKnownTools.get(currentName); + if (tool) { + debugLogger.debug( + `Resolved legacy tool name "${name}" to current name "${currentName}"`, + ); + } + } + if (!tool && name.includes('__')) { for (const t of this.allKnownTools.values()) { if (t instanceof DiscoveredMCPTool) {