diff --git a/package-lock.json b/package-lock.json index 0cef0ecf..475ebad9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3195,6 +3195,13 @@ "@types/send": "*" } }, + "node_modules/@types/fast-levenshtein": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/@types/fast-levenshtein/-/fast-levenshtein-0.0.4.tgz", + "integrity": "sha512-tkDveuitddQCxut1Db8eEFfMahTjOumTJGPHmT9E7KUH+DkVq9WTpVvlfenf3S+uCBeu8j5FP2xik/KfxOEjeA==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/fs-extra": { "version": "11.0.4", "resolved": "https://registry.npmjs.org/@types/fs-extra/-/fs-extra-11.0.4.tgz", @@ -6885,7 +6892,6 @@ "version": "2.0.6", "resolved": "https://registry.npmjs.org/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz", "integrity": "sha512-DCXu6Ifhqcks7TZKY3Hxp3y6qphY5SJZmrWMDrKcERSOXWQdMhU9Ig/PYrzyw/ul9jOIyh0N4M0tbC5hodg8dw==", - "dev": true, "license": "MIT" }, "node_modules/fast-safe-stringify": { @@ -14679,6 +14685,7 @@ "chardet": "^2.1.0", "diff": "^7.0.0", "dotenv": "^17.1.0", + "fast-levenshtein": "^2.0.6", "fast-uri": "^3.0.6", "fdir": "^6.4.6", "fzf": "^0.5.2", @@ -14701,6 +14708,7 @@ "@google/gemini-cli-test-utils": "file:../test-utils", "@types/diff": "^7.0.2", "@types/dotenv": "^6.1.1", + "@types/fast-levenshtein": "^0.0.4", "@types/minimatch": "^5.1.2", "@types/picomatch": "^4.0.1", "@types/ws": "^8.5.10", diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index 3a06dd27..0100b551 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -48,6 +48,7 @@ vi.mock('@google/gemini-cli-core', async () => { const mockToolRegistry = { getTool: vi.fn(), + getAllToolNames: vi.fn(() => ['mockTool', 'anotherTool']), }; const mockConfig = { @@ -427,11 +428,17 @@ describe('useReactToolScheduler', () => { request, response: expect.objectContaining({ error: expect.objectContaining({ - message: 'Tool "nonexistentTool" not found in registry.', + message: expect.stringMatching( + /Tool "nonexistentTool" not found in registry/, + ), }), }), }), ]); + const errorMessage = onComplete.mock.calls[0][0][0].response.error.message; + expect(errorMessage).toContain('Did you mean one of:'); + expect(errorMessage).toContain('"mockTool"'); + expect(errorMessage).toContain('"anotherTool"'); expect(result.current[0]).toEqual([]); }); diff --git a/packages/core/package.json b/packages/core/package.json index bcf49806..a444eba1 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -33,12 +33,14 @@ "@opentelemetry/sdk-node": "^0.203.0", "@types/glob": "^8.1.0", "@types/html-to-text": "^9.0.4", + "@xterm/headless": "5.5.0", "ajv": "^8.17.1", - "fast-uri": "^3.0.6", "ajv-formats": "^3.0.0", "chardet": "^2.1.0", "diff": "^7.0.0", "dotenv": "^17.1.0", + "fast-levenshtein": "^2.0.6", + "fast-uri": "^3.0.6", "fdir": "^6.4.6", "fzf": "^0.5.2", "glob": "^10.4.5", @@ -54,22 +56,22 @@ "simple-git": "^3.28.0", "strip-ansi": "^7.1.0", "undici": "^7.10.0", - "ws": "^8.18.0", - "@xterm/headless": "5.5.0" + "ws": "^8.18.0" }, "optionalDependencies": { "@lydell/node-pty": "1.1.0", - "node-pty": "^1.0.0", "@lydell/node-pty-darwin-arm64": "1.1.0", "@lydell/node-pty-darwin-x64": "1.1.0", "@lydell/node-pty-linux-x64": "1.1.0", "@lydell/node-pty-win32-arm64": "1.1.0", - "@lydell/node-pty-win32-x64": "1.1.0" + "@lydell/node-pty-win32-x64": "1.1.0", + "node-pty": "^1.0.0" }, "devDependencies": { "@google/gemini-cli-test-utils": "file:../test-utils", "@types/diff": "^7.0.2", "@types/dotenv": "^6.1.1", + "@types/fast-levenshtein": "^0.0.4", "@types/minimatch": "^5.1.2", "@types/picomatch": "^4.0.1", "@types/ws": "^8.5.10", diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index e4b4ac5b..e4ef0dc8 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -195,6 +195,42 @@ describe('CoreToolScheduler', () => { .calls[0][0] as ToolCall[]; expect(completedCalls[0].status).toBe('cancelled'); }); + + describe('getToolSuggestion', () => { + it('should suggest the top N closest tool names for a typo', () => { + // Create mocked tool registry + const mockConfig = { + getToolRegistry: () => mockToolRegistry, + } as unknown as Config; + const mockToolRegistry = { + getAllToolNames: () => ['list_files', 'read_file', 'write_file'], + } as unknown as ToolRegistry; + + // Create scheduler + const scheduler = new CoreToolScheduler({ + config: mockConfig, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + // Test that the right tool is selected, with only 1 result, for typos + // @ts-expect-error accessing private method + const misspelledTool = scheduler.getToolSuggestion('list_fils', 1); + expect(misspelledTool).toBe(' Did you mean "list_files"?'); + + // Test that the right tool is selected, with only 1 result, for prefixes + // @ts-expect-error accessing private method + const prefixedTool = scheduler.getToolSuggestion('github.list_files', 1); + expect(prefixedTool).toBe(' Did you mean "list_files"?'); + + // Test that the right tool is first + // @ts-expect-error accessing private method + const suggestionMultiple = scheduler.getToolSuggestion('list_fils'); + expect(suggestionMultiple).toBe( + ' Did you mean one of: "list_files", "read_file", "write_file"?', + ); + }); + }); }); describe('CoreToolScheduler with payload', () => { diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 8a4a7a1c..9af10a82 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -33,6 +33,7 @@ import { } from '../tools/modifiable-tool.js'; import * as Diff from 'diff'; import { doesToolInvocationMatch } from '../utils/tool-utils.js'; +import levenshtein from 'fast-levenshtein'; export type ValidatingToolCall = { status: 'validating'; @@ -504,6 +505,40 @@ export class CoreToolScheduler { } } + /** + * Generates a suggestion string for a tool name that was not found in the registry. + * It finds the closest matches based on Levenshtein distance. + * @param unknownToolName The tool name that was not found. + * @param topN The number of suggestions to return. Defaults to 3. + * @returns A suggestion string like " Did you mean 'tool'?" or " Did you mean one of: 'tool1', 'tool2'?", or an empty string if no suggestions are found. + */ + private getToolSuggestion(unknownToolName: string, topN = 3): string { + const allToolNames = this.toolRegistry.getAllToolNames(); + + const matches = allToolNames.map((toolName) => ({ + name: toolName, + distance: levenshtein.get(unknownToolName, toolName), + })); + + matches.sort((a, b) => a.distance - b.distance); + + const topNResults = matches.slice(0, topN); + + if (topNResults.length === 0) { + return ''; + } + + const suggestedNames = topNResults + .map((match) => `"${match.name}"`) + .join(', '); + + if (topNResults.length > 1) { + return ` Did you mean one of: ${suggestedNames}?`; + } else { + return ` Did you mean ${suggestedNames}?`; + } + } + schedule( request: ToolCallRequestInfo | ToolCallRequestInfo[], signal: AbortSignal, @@ -557,12 +592,14 @@ export class CoreToolScheduler { (reqInfo): ToolCall => { const toolInstance = this.toolRegistry.getTool(reqInfo.name); if (!toolInstance) { + const suggestion = this.getToolSuggestion(reqInfo.name); + const errorMessage = `Tool "${reqInfo.name}" not found in registry. Tools must use the exact names that are registered.${suggestion}`; return { status: 'error', request: reqInfo, response: createErrorResponse( reqInfo, - new Error(`Tool "${reqInfo.name}" not found in registry.`), + new Error(errorMessage), ToolErrorType.TOOL_NOT_REGISTERED, ), durationMs: 0, diff --git a/packages/core/src/core/nonInteractiveToolExecutor.test.ts b/packages/core/src/core/nonInteractiveToolExecutor.test.ts index c46328bf..99cd5ab7 100644 --- a/packages/core/src/core/nonInteractiveToolExecutor.test.ts +++ b/packages/core/src/core/nonInteractiveToolExecutor.test.ts @@ -27,6 +27,7 @@ describe('executeToolCall', () => { mockToolRegistry = { getTool: vi.fn(), + getAllToolNames: vi.fn(), } as unknown as ToolRegistry; mockConfig = { @@ -94,6 +95,10 @@ describe('executeToolCall', () => { prompt_id: 'prompt-id-2', }; vi.mocked(mockToolRegistry.getTool).mockReturnValue(undefined); + vi.mocked(mockToolRegistry.getAllToolNames).mockReturnValue([ + 'testTool', + 'anotherTool', + ]); const response = await executeToolCall( mockConfig, @@ -101,18 +106,20 @@ describe('executeToolCall', () => { abortController.signal, ); + const expectedErrorMessage = + 'Tool "nonexistentTool" not found in registry. Tools must use the exact names that are registered. Did you mean one of: "testTool", "anotherTool"?'; expect(response).toStrictEqual({ callId: 'call2', - error: new Error('Tool "nonexistentTool" not found in registry.'), + error: new Error(expectedErrorMessage), errorType: ToolErrorType.TOOL_NOT_REGISTERED, - resultDisplay: 'Tool "nonexistentTool" not found in registry.', + resultDisplay: expectedErrorMessage, responseParts: [ { functionResponse: { name: 'nonexistentTool', id: 'call2', response: { - error: 'Tool "nonexistentTool" not found in registry.', + error: expectedErrorMessage, }, }, }, diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 1bcded76..f4210687 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -174,6 +174,24 @@ describe('ToolRegistry', () => { }); }); + describe('getAllToolNames', () => { + it('should return all registered tool names', () => { + // Register tools with displayNames in non-alphabetical order + const toolC = new MockTool('c-tool', 'Tool C'); + const toolA = new MockTool('a-tool', 'Tool A'); + const toolB = new MockTool('b-tool', 'Tool B'); + + toolRegistry.registerTool(toolC); + toolRegistry.registerTool(toolA); + toolRegistry.registerTool(toolB); + + const toolNames = toolRegistry.getAllToolNames(); + + // Assert that the returned array contains all tool names + expect(toolNames).toEqual(['c-tool', 'a-tool', 'b-tool']); + }); + }); + describe('getToolsByServer', () => { it('should return an empty array if no tools match the server name', () => { toolRegistry.registerTool(new MockTool()); diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index ff1c8498..07f92309 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -437,6 +437,13 @@ export class ToolRegistry { return declarations; } + /** + * Returns an array of all registered and discovered tool names. + */ + getAllToolNames(): string[] { + return Array.from(this.tools.keys()); + } + /** * Returns an array of all registered and discovered tool instances. */