From a4f780feb963b2149f9405b2d7ff782737f187e3 Mon Sep 17 00:00:00 2001 From: Aishanee Shah Date: Fri, 6 Feb 2026 19:05:30 +0000 Subject: [PATCH] test: add tests for model-dependent tool definitions --- .../__snapshots__/read-file.test.ts.snap | 5 +++ .../tools/__snapshots__/shell.test.ts.snap | 26 ++++++++++++ .../core/src/tools/definitions/coreTools.ts | 19 ++++----- .../src/tools/definitions/resolver.test.ts | 40 +++++++++++++++++++ packages/core/src/tools/read-file.test.ts | 15 +++++++ packages/core/src/tools/read-file.ts | 10 ++--- packages/core/src/tools/shell.test.ts | 15 +++++++ packages/core/src/tools/shell.ts | 18 ++------- packages/core/src/tools/tool-registry.test.ts | 11 +++++ packages/core/src/tools/tools.ts | 12 +++++- 10 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 packages/core/src/tools/__snapshots__/read-file.test.ts.snap create mode 100644 packages/core/src/tools/definitions/resolver.test.ts diff --git a/packages/core/src/tools/__snapshots__/read-file.test.ts.snap b/packages/core/src/tools/__snapshots__/read-file.test.ts.snap new file mode 100644 index 0000000000..c6adf2819d --- /dev/null +++ b/packages/core/src/tools/__snapshots__/read-file.test.ts.snap @@ -0,0 +1,5 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ReadFileTool > getSchema > should return the base schema when no modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`; + +exports[`ReadFileTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = `"Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges."`; diff --git a/packages/core/src/tools/__snapshots__/shell.test.ts.snap b/packages/core/src/tools/__snapshots__/shell.test.ts.snap index 6592993160..3cfef4d8ca 100644 --- a/packages/core/src/tools/__snapshots__/shell.test.ts.snap +++ b/packages/core/src/tools/__snapshots__/shell.test.ts.snap @@ -25,3 +25,29 @@ exports[`ShellTool > getDescription > should return the windows description when Background PIDs: Only included if background processes were started. Process Group PGID: Only included if available." `; + +exports[`ShellTool > getSchema > should return the base schema when no modelId is provided 1`] = ` +"This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. + + The following information is returned: + + Output: Combined stdout/stderr. Can be \`(empty)\` or partial on error and for any unwaited background processes. + Exit Code: Only included if non-zero (command failed). + Error: Only included if a process-level error occurred (e.g., spawn failure). + Signal: Only included if process was terminated by a signal. + Background PIDs: Only included if background processes were started. + Process Group PGID: Only included if available." +`; + +exports[`ShellTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = ` +"This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. + + The following information is returned: + + Output: Combined stdout/stderr. Can be \`(empty)\` or partial on error and for any unwaited background processes. + Exit Code: Only included if non-zero (command failed). + Error: Only included if a process-level error occurred (e.g., spawn failure). + Signal: Only included if process was terminated by a signal. + Background PIDs: Only included if background processes were started. + Process Group PGID: Only included if available." +`; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index 8470db41dc..9678ab81d3 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { Type } from '@google/genai'; import type { ToolDefinition } from './types.js'; import { READ_FILE_TOOL_NAME, SHELL_TOOL_NAME } from '../tool-names.js'; import { getCommandDescription } from '../shell.js'; @@ -13,21 +14,21 @@ export const READ_FILE_DEFINITION: ToolDefinition = { name: READ_FILE_TOOL_NAME, description: `Reads and returns the content of a specified file. If the file is large, the content will be truncated. The tool's response will clearly indicate if truncation has occurred and will provide details on how to read more of the file using the 'offset' and 'limit' parameters. Handles text, images (PNG, JPG, GIF, WEBP, SVG, BMP), audio files (MP3, WAV, AIFF, AAC, OGG, FLAC), and PDF files. For text files, it can read specific line ranges.`, parameters: { - type: 'object', + type: Type.OBJECT, properties: { file_path: { description: 'The path to the file to read.', - type: 'string', + type: Type.STRING, }, offset: { description: "Optional: For text files, the 0-based line number to start reading from. Requires 'limit' to be set. Use for paginating through large files.", - type: 'number', + type: Type.NUMBER, }, limit: { description: "Optional: For text files, maximum number of lines to read. Use with 'offset' to paginate through large files. If omitted, reads the entire file (if feasible, up to a default limit).", - type: 'number', + type: Type.NUMBER, }, }, required: ['file_path'], @@ -40,24 +41,24 @@ export const SHELL_DEFINITION: ToolDefinition = { name: SHELL_TOOL_NAME, description: 'Executes a shell command.', parameters: { - type: 'object', + type: Type.OBJECT, properties: { command: { - type: 'string', + type: Type.STRING, description: getCommandDescription(), }, description: { - type: 'string', + type: Type.STRING, description: 'Brief description of the command for the user. Be specific and concise. Ideally a single sentence. Can be up to 3 sentences for clarity. No line breaks.', }, dir_path: { - type: 'string', + type: Type.STRING, description: '(OPTIONAL) The path of the directory to run the command in. If not provided, the project root directory is used. Must be a directory within the workspace and must already exist.', }, is_background: { - type: 'boolean', + type: Type.BOOLEAN, description: 'Set to true if this command should be run in the background (e.g. for long-running servers or watchers). The command will be started, allowed to run for a brief moment to check for immediate errors, and then moved to the background.', }, diff --git a/packages/core/src/tools/definitions/resolver.test.ts b/packages/core/src/tools/definitions/resolver.test.ts new file mode 100644 index 0000000000..a765608ac7 --- /dev/null +++ b/packages/core/src/tools/definitions/resolver.test.ts @@ -0,0 +1,40 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { Type } from '@google/genai'; +import { resolveToolDeclaration } from './resolver.js'; +import type { ToolDefinition } from './types.js'; + +describe('resolveToolDeclaration', () => { + const mockDefinition: ToolDefinition = { + base: { + name: 'test_tool', + description: 'A test tool description', + parameters: { + type: Type.OBJECT, + properties: { + param1: { type: Type.STRING }, + }, + }, + }, + }; + + it('should return the base definition when no modelId is provided', () => { + const result = resolveToolDeclaration(mockDefinition); + expect(result).toEqual(mockDefinition.base); + }); + + it('should return the base definition when a modelId is provided (current implementation)', () => { + const result = resolveToolDeclaration(mockDefinition, 'gemini-1.5-pro'); + expect(result).toEqual(mockDefinition.base); + }); + + it('should return the same object reference as base (current implementation)', () => { + const result = resolveToolDeclaration(mockDefinition); + expect(result).toBe(mockDefinition.base); + }); +}); diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 15071f2620..494b007dec 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -563,4 +563,19 @@ describe('ReadFileTool', () => { }); }); }); + + describe('getSchema', () => { + it('should return the base schema when no modelId is provided', () => { + const schema = tool.getSchema(); + expect(schema.name).toBe(ReadFileTool.Name); + expect(schema.description).toMatchSnapshot(); + }); + + it('should return the schema from the resolver when modelId is provided', () => { + const modelId = 'gemini-2.0-flash'; + const schema = tool.getSchema(modelId); + expect(schema.name).toBe(ReadFileTool.Name); + expect(schema.description).toMatchSnapshot(); + }); + }); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 27b13f1f9a..dbed3b7220 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -24,7 +24,6 @@ import { FileOperationEvent } from '../telemetry/types.js'; import { READ_FILE_TOOL_NAME } from './tool-names.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { READ_FILE_DEFINITION } from './definitions/coreTools.js'; -import { resolveToolDeclaration } from './definitions/resolver.js'; /** * Parameters for the ReadFile tool @@ -236,10 +235,9 @@ export class ReadFileTool extends BaseDeclarativeTool< ); } - override getSchema(modelId?: string) { - if (!modelId) { - return super.getSchema(); - } - return resolveToolDeclaration(READ_FILE_DEFINITION, modelId); + override getSchema(_modelId?: string) { + // Pure refactor: maintain existing behavior. + // getSchema(modelId) is now available for future model-specific overrides. + return super.getSchema(); } } diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index b851ee99d4..01c4f4a8f1 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -815,4 +815,19 @@ describe('ShellTool', () => { } }); }); + + describe('getSchema', () => { + it('should return the base schema when no modelId is provided', () => { + const schema = shellTool.getSchema(); + expect(schema.name).toBe(SHELL_TOOL_NAME); + expect(schema.description).toMatchSnapshot(); + }); + + it('should return the schema from the resolver when modelId is provided', () => { + const modelId = 'gemini-2.0-flash'; + const schema = shellTool.getSchema(modelId); + expect(schema.name).toBe(SHELL_TOOL_NAME); + expect(schema.description).toMatchSnapshot(); + }); + }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 0d4fdcb368..acda68b7c0 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -44,7 +44,6 @@ import { import { SHELL_TOOL_NAME } from './tool-names.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { SHELL_DEFINITION } from './definitions/coreTools.js'; -import { resolveToolDeclaration } from './definitions/resolver.js'; export const OUTPUT_UPDATE_INTERVAL_MS = 1000; @@ -545,18 +544,9 @@ export class ShellTool extends BaseDeclarativeTool< ); } - override getSchema(modelId?: string) { - const declaration = modelId - ? resolveToolDeclaration(SHELL_DEFINITION, modelId) - : super.getSchema(); - - // Append platform-specific info which is currently not in the static definition - const platformInfo = getShellToolDescription( - this.config.getEnableInteractiveShell(), - ); - return { - ...declaration, - description: `${declaration.description}\n\n${platformInfo}`, - }; + override getSchema(_modelId?: string) { + // Pure refactor: maintain existing behavior. + // getSchema(modelId) is now available for future model-specific overrides. + return super.getSchema(); } } diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index c26349f50f..963830200d 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -261,6 +261,17 @@ describe('ToolRegistry', () => { toolRegistry.registerTool(tool); expect(toolRegistry.getTool('mock-tool')).toBe(tool); }); + + it('should pass modelId to getSchema when getting function declarations', () => { + const tool = new MockTool({ name: 'mock-tool' }); + const getSchemaSpy = vi.spyOn(tool, 'getSchema'); + toolRegistry.registerTool(tool); + + const modelId = 'test-model-id'; + toolRegistry.getFunctionDeclarations(modelId); + + expect(getSchemaSpy).toHaveBeenCalledWith(modelId); + }); }); describe('excluded tools', () => { diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index ec4e308c9a..2811653b20 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -316,6 +316,12 @@ export interface ToolBuilder< */ getSchema(modelId?: string): FunctionDeclaration; + /** + * Function declaration schema for the default model. + * @deprecated Use getSchema(modelId) for model-specific schemas. + */ + readonly schema: FunctionDeclaration; + /** * Whether the tool's output should be rendered as markdown. */ @@ -364,6 +370,10 @@ export abstract class DeclarativeTool< }; } + get schema(): FunctionDeclaration { + return this.getSchema(); + } + /** * Validates the raw tool parameters. * Subclasses should override this to add custom validation logic @@ -487,7 +497,7 @@ export abstract class BaseDeclarativeTool< override validateToolParams(params: TParams): string | null { const errors = SchemaValidator.validate( - this.getSchema().parametersJsonSchema, + this.schema.parametersJsonSchema, params, );