Improve Function Call argument validation and typing (#2881)

Co-authored-by: N. Taylor Mullen <ntaylormullen@google.com>
This commit is contained in:
Tommaso Sciortino 2025-07-07 23:48:44 -07:00 committed by GitHub
parent 137ffec3f6
commit 4dab31f1c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 239 additions and 246 deletions

48
package-lock.json generated
View File

@ -5205,6 +5205,22 @@
"dev": true, "dev": true,
"license": "MIT" "license": "MIT"
}, },
"node_modules/fast-uri": {
"version": "3.0.6",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.6.tgz",
"integrity": "sha512-Atfo14OibSv5wAp4VWNsFYE1AchQRTv9cBGWET4pZWHzYshFSS9NQI6I57rdKn9croWVMbYFbLhJ+yJvmZIIHw==",
"funding": [
{
"type": "github",
"url": "https://github.com/sponsors/fastify"
},
{
"type": "opencollective",
"url": "https://opencollective.com/fastify"
}
],
"license": "BSD-3-Clause"
},
"node_modules/fastq": { "node_modules/fastq": {
"version": "1.19.1", "version": "1.19.1",
"resolved": "https://registry.npmjs.org/fastq/-/fastq-1.19.1.tgz", "resolved": "https://registry.npmjs.org/fastq/-/fastq-1.19.1.tgz",
@ -8683,6 +8699,15 @@
"node": ">=0.10.0" "node": ">=0.10.0"
} }
}, },
"node_modules/require-from-string": {
"version": "2.0.2",
"resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz",
"integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==",
"license": "MIT",
"engines": {
"node": ">=0.10.0"
}
},
"node_modules/require-in-the-middle": { "node_modules/require-in-the-middle": {
"version": "7.5.2", "version": "7.5.2",
"resolved": "https://registry.npmjs.org/require-in-the-middle/-/require-in-the-middle-7.5.2.tgz", "resolved": "https://registry.npmjs.org/require-in-the-middle/-/require-in-the-middle-7.5.2.tgz",
@ -11375,6 +11400,7 @@
"@opentelemetry/sdk-node": "^0.52.0", "@opentelemetry/sdk-node": "^0.52.0",
"@types/glob": "^8.1.0", "@types/glob": "^8.1.0",
"@types/html-to-text": "^9.0.4", "@types/html-to-text": "^9.0.4",
"ajv": "^8.17.1",
"diff": "^7.0.0", "diff": "^7.0.0",
"dotenv": "^16.6.1", "dotenv": "^16.6.1",
"gaxios": "^6.1.1", "gaxios": "^6.1.1",
@ -11403,6 +11429,22 @@
"node": ">=20" "node": ">=20"
} }
}, },
"packages/core/node_modules/ajv": {
"version": "8.17.1",
"resolved": "https://registry.npmjs.org/ajv/-/ajv-8.17.1.tgz",
"integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==",
"license": "MIT",
"dependencies": {
"fast-deep-equal": "^3.1.3",
"fast-uri": "^3.0.1",
"json-schema-traverse": "^1.0.0",
"require-from-string": "^2.0.2"
},
"funding": {
"type": "github",
"url": "https://github.com/sponsors/epoberezkin"
}
},
"packages/core/node_modules/ignore": { "packages/core/node_modules/ignore": {
"version": "7.0.5", "version": "7.0.5",
"license": "MIT", "license": "MIT",
@ -11410,6 +11452,12 @@
"node": ">= 4" "node": ">= 4"
} }
}, },
"packages/core/node_modules/json-schema-traverse": {
"version": "1.0.0",
"resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz",
"integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==",
"license": "MIT"
},
"packages/core/node_modules/ws": { "packages/core/node_modules/ws": {
"version": "8.18.2", "version": "8.18.2",
"license": "MIT", "license": "MIT",

View File

@ -30,6 +30,7 @@
"@opentelemetry/sdk-node": "^0.52.0", "@opentelemetry/sdk-node": "^0.52.0",
"@types/glob": "^8.1.0", "@types/glob": "^8.1.0",
"@types/html-to-text": "^9.0.4", "@types/html-to-text": "^9.0.4",
"ajv": "^8.17.1",
"diff": "^7.0.0", "diff": "^7.0.0",
"dotenv": "^16.6.1", "dotenv": "^16.6.1",
"gaxios": "^6.1.1", "gaxios": "^6.1.1",

View File

@ -15,6 +15,7 @@ import {
ToolResult, ToolResult,
ToolResultDisplay, ToolResultDisplay,
} from './tools.js'; } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js'; import { makeRelative, shortenPath } from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js'; import { isNodeError } from '../utils/errors.js';
@ -97,27 +98,27 @@ Expectation for required parameters:
file_path: { file_path: {
description: description:
"The absolute path to the file to modify. Must start with '/'.", "The absolute path to the file to modify. Must start with '/'.",
type: 'string', type: Type.STRING,
}, },
old_string: { old_string: {
description: description:
'The exact literal text to replace, preferably unescaped. For single replacements (default), include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. For multiple replacements, specify expected_replacements parameter. If this string is not the exact literal text (i.e. you escaped it) or does not match exactly, the tool will fail.', 'The exact literal text to replace, preferably unescaped. For single replacements (default), include at least 3 lines of context BEFORE and AFTER the target text, matching whitespace and indentation precisely. For multiple replacements, specify expected_replacements parameter. If this string is not the exact literal text (i.e. you escaped it) or does not match exactly, the tool will fail.',
type: 'string', type: Type.STRING,
}, },
new_string: { new_string: {
description: description:
'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.', 'The exact literal text to replace `old_string` with, preferably unescaped. Provide the EXACT text. Ensure the resulting code is correct and idiomatic.',
type: 'string', type: Type.STRING,
}, },
expected_replacements: { expected_replacements: {
type: 'number', type: Type.NUMBER,
description: description:
'Number of replacements expected. Defaults to 1 if not specified. Use when you want to replace multiple occurrences.', 'Number of replacements expected. Defaults to 1 if not specified. Use when you want to replace multiple occurrences.',
minimum: 1, minimum: 1,
}, },
}, },
required: ['file_path', 'old_string', 'new_string'], required: ['file_path', 'old_string', 'new_string'],
type: 'object', type: Type.OBJECT,
}, },
); );
this.rootDirectory = path.resolve(this.config.getTargetDir()); this.rootDirectory = path.resolve(this.config.getTargetDir());
@ -146,14 +147,9 @@ Expectation for required parameters:
* @returns Error message string or null if valid * @returns Error message string or null if valid
*/ */
validateToolParams(params: EditToolParams): string | null { validateToolParams(params: EditToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
if (!path.isAbsolute(params.file_path)) { if (!path.isAbsolute(params.file_path)) {

View File

@ -181,8 +181,8 @@ describe('GlobTool', () => {
// Need to correctly define this as an object without pattern // Need to correctly define this as an object without pattern
const params = { path: '.' }; const params = { path: '.' };
// @ts-expect-error - We're intentionally creating invalid params for testing // @ts-expect-error - We're intentionally creating invalid params for testing
expect(globTool.validateToolParams(params)).toContain( expect(globTool.validateToolParams(params)).toBe(
'Parameters failed schema validation', `params must have required property 'pattern'`,
); );
}); });
@ -206,8 +206,8 @@ describe('GlobTool', () => {
path: 123, path: 123,
}; };
// @ts-expect-error - We're intentionally creating invalid params for testing // @ts-expect-error - We're intentionally creating invalid params for testing
expect(globTool.validateToolParams(params)).toContain( expect(globTool.validateToolParams(params)).toBe(
'Parameters failed schema validation', 'params/path must be string',
); );
}); });
@ -217,8 +217,8 @@ describe('GlobTool', () => {
case_sensitive: 'true', case_sensitive: 'true',
}; };
// @ts-expect-error - We're intentionally creating invalid params for testing // @ts-expect-error - We're intentionally creating invalid params for testing
expect(globTool.validateToolParams(params)).toContain( expect(globTool.validateToolParams(params)).toBe(
'Parameters failed schema validation', 'params/case_sensitive must be boolean',
); );
}); });

View File

@ -9,6 +9,7 @@ import path from 'path';
import { glob } from 'glob'; import { glob } from 'glob';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { shortenPath, makeRelative } from '../utils/paths.js'; import { shortenPath, makeRelative } from '../utils/paths.js';
import { Config } from '../config/config.js'; import { Config } from '../config/config.js';
@ -95,26 +96,26 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
pattern: { pattern: {
description: description:
"The glob pattern to match against (e.g., '**/*.py', 'docs/*.md').", "The glob pattern to match against (e.g., '**/*.py', 'docs/*.md').",
type: 'string', type: Type.STRING,
}, },
path: { path: {
description: description:
'Optional: The absolute path to the directory to search within. If omitted, searches the root directory.', 'Optional: The absolute path to the directory to search within. If omitted, searches the root directory.',
type: 'string', type: Type.STRING,
}, },
case_sensitive: { case_sensitive: {
description: description:
'Optional: Whether the search should be case-sensitive. Defaults to false.', 'Optional: Whether the search should be case-sensitive. Defaults to false.',
type: 'boolean', type: Type.BOOLEAN,
}, },
respect_git_ignore: { respect_git_ignore: {
description: description:
'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.', 'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.',
type: 'boolean', type: Type.BOOLEAN,
}, },
}, },
required: ['pattern'], required: ['pattern'],
type: 'object', type: Type.OBJECT,
}, },
); );
@ -145,14 +146,9 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> {
* Validates the parameters for the tool. * Validates the parameters for the tool.
*/ */
validateToolParams(params: GlobToolParams): string | null { validateToolParams(params: GlobToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return "Parameters failed schema validation. Ensure 'pattern' is a string, 'path' (if provided) is a string, and 'case_sensitive' (if provided) is a boolean.";
} }
const searchDirAbsolute = path.resolve( const searchDirAbsolute = path.resolve(

View File

@ -80,8 +80,8 @@ describe('GrepTool', () => {
it('should return error if pattern is missing', () => { it('should return error if pattern is missing', () => {
const params = { path: '.' } as unknown as GrepToolParams; const params = { path: '.' } as unknown as GrepToolParams;
expect(grepTool.validateToolParams(params)).toContain( expect(grepTool.validateToolParams(params)).toBe(
'Parameters failed schema validation', `params must have required property 'pattern'`,
); );
}); });
@ -204,11 +204,11 @@ describe('GrepTool', () => {
it('should return an error if params are invalid', async () => { it('should return an error if params are invalid', async () => {
const params = { path: '.' } as unknown as GrepToolParams; // Invalid: pattern missing const params = { path: '.' } as unknown as GrepToolParams; // Invalid: pattern missing
const result = await grepTool.execute(params, abortSignal); const result = await grepTool.execute(params, abortSignal);
expect(result.llmContent).toContain( expect(result.llmContent).toBe(
'Error: Invalid parameters provided. Reason: Parameters failed schema validation', "Error: Invalid parameters provided. Reason: params must have required property 'pattern'",
); );
expect(result.returnDisplay).toContain( expect(result.returnDisplay).toBe(
'Model provided invalid parameters. Error: Parameters failed schema validation', "Model provided invalid parameters. Error: params must have required property 'pattern'",
); );
}); });
}); });

View File

@ -11,6 +11,7 @@ import { EOL } from 'os';
import { spawn } from 'child_process'; import { spawn } from 'child_process';
import { globStream } from 'glob'; import { globStream } from 'glob';
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js'; import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js'; import { getErrorMessage, isNodeError } from '../utils/errors.js';
@ -69,21 +70,21 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
pattern: { pattern: {
description: description:
"The regular expression (regex) pattern to search for within file contents (e.g., 'function\\s+myFunction', 'import\\s+\\{.*\\}\\s+from\\s+.*').", "The regular expression (regex) pattern to search for within file contents (e.g., 'function\\s+myFunction', 'import\\s+\\{.*\\}\\s+from\\s+.*').",
type: 'string', type: Type.STRING,
}, },
path: { path: {
description: description:
'Optional: The absolute path to the directory to search within. If omitted, searches the current working directory.', 'Optional: The absolute path to the directory to search within. If omitted, searches the current working directory.',
type: 'string', type: Type.STRING,
}, },
include: { include: {
description: description:
"Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).", "Optional: A glob pattern to filter which files are searched (e.g., '*.js', '*.{ts,tsx}', 'src/**'). If omitted, searches all files (respecting potential global ignores).",
type: 'string', type: Type.STRING,
}, },
}, },
required: ['pattern'], required: ['pattern'],
type: 'object', type: Type.OBJECT,
}, },
); );
// Ensure rootDirectory is absolute and normalized // Ensure rootDirectory is absolute and normalized
@ -135,14 +136,9 @@ export class GrepTool extends BaseTool<GrepToolParams, ToolResult> {
* @returns An error message string if invalid, null otherwise * @returns An error message string if invalid, null otherwise
*/ */
validateToolParams(params: GrepToolParams): string | null { validateToolParams(params: GrepToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
try { try {

View File

@ -7,6 +7,7 @@
import fs from 'fs'; import fs from 'fs';
import path from 'path'; import path from 'path';
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js'; import { makeRelative, shortenPath } from '../utils/paths.js';
import { Config } from '../config/config.js'; import { Config } from '../config/config.js';
@ -84,23 +85,23 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> {
path: { path: {
description: description:
'The absolute path to the directory to list (must be absolute, not relative)', 'The absolute path to the directory to list (must be absolute, not relative)',
type: 'string', type: Type.STRING,
}, },
ignore: { ignore: {
description: 'List of glob patterns to ignore', description: 'List of glob patterns to ignore',
items: { items: {
type: 'string', type: Type.STRING,
}, },
type: 'array', type: Type.ARRAY,
}, },
respect_git_ignore: { respect_git_ignore: {
description: description:
'Optional: Whether to respect .gitignore patterns when listing files. Only available in git repositories. Defaults to true.', 'Optional: Whether to respect .gitignore patterns when listing files. Only available in git repositories. Defaults to true.',
type: 'boolean', type: Type.BOOLEAN,
}, },
}, },
required: ['path'], required: ['path'],
type: 'object', type: Type.OBJECT,
}, },
); );
@ -132,14 +133,9 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> {
* @returns An error message string if invalid, null otherwise * @returns An error message string if invalid, null otherwise
*/ */
validateToolParams(params: LSToolParams): string | null { validateToolParams(params: LSToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
if (!path.isAbsolute(params.path)) { if (!path.isAbsolute(params.path)) {
return `Path must be absolute: ${params.path}`; return `Path must be absolute: ${params.path}`;

View File

@ -14,7 +14,7 @@ import {
import { parse } from 'shell-quote'; import { parse } from 'shell-quote';
import { MCPServerConfig } from '../config/config.js'; import { MCPServerConfig } from '../config/config.js';
import { DiscoveredMCPTool } from './mcp-tool.js'; import { DiscoveredMCPTool } from './mcp-tool.js';
import { CallableTool, FunctionDeclaration, mcpToTool } from '@google/genai'; import { Type, mcpToTool } from '@google/genai';
import { sanitizeParameters, ToolRegistry } from './tool-registry.js'; import { sanitizeParameters, ToolRegistry } from './tool-registry.js';
export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes
@ -275,13 +275,10 @@ async function connectAndDiscover(
} }
try { try {
const mcpCallableTool: CallableTool = mcpToTool(mcpClient); const mcpCallableTool = mcpToTool(mcpClient);
const discoveredToolFunctions = await mcpCallableTool.tool(); const tool = await mcpCallableTool.tool();
if ( if (!tool || !Array.isArray(tool.functionDeclarations)) {
!discoveredToolFunctions ||
!Array.isArray(discoveredToolFunctions.functionDeclarations)
) {
console.error( console.error(
`MCP server '${mcpServerName}' did not return valid tool function declarations. Skipping.`, `MCP server '${mcpServerName}' did not return valid tool function declarations. Skipping.`,
); );
@ -297,7 +294,7 @@ async function connectAndDiscover(
return; return;
} }
for (const funcDecl of discoveredToolFunctions.functionDeclarations) { for (const funcDecl of tool.functionDeclarations) {
if (!funcDecl.name) { if (!funcDecl.name) {
console.warn( console.warn(
`Discovered a function declaration without a name from MCP server '${mcpServerName}'. Skipping.`, `Discovered a function declaration without a name from MCP server '${mcpServerName}'. Skipping.`,
@ -344,19 +341,13 @@ async function connectAndDiscover(
sanitizeParameters(funcDecl.parameters); sanitizeParameters(funcDecl.parameters);
// Ensure parameters is a valid JSON schema object, default to empty if not.
const parameterSchema: Record<string, unknown> =
funcDecl.parameters && typeof funcDecl.parameters === 'object'
? { ...(funcDecl.parameters as FunctionDeclaration) }
: { type: 'object', properties: {} };
toolRegistry.registerTool( toolRegistry.registerTool(
new DiscoveredMCPTool( new DiscoveredMCPTool(
mcpCallableTool, mcpCallableTool,
mcpServerName, mcpServerName,
toolNameForModel, toolNameForModel,
funcDecl.description ?? '', funcDecl.description ?? '',
parameterSchema, funcDecl.parameters ?? { type: Type.OBJECT, properties: {} },
funcDecl.name, funcDecl.name,
mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
mcpServerConfig.trust, mcpServerConfig.trust,

View File

@ -11,7 +11,7 @@ import {
ToolConfirmationOutcome, ToolConfirmationOutcome,
ToolMcpConfirmationDetails, ToolMcpConfirmationDetails,
} from './tools.js'; } from './tools.js';
import { CallableTool, Part, FunctionCall } from '@google/genai'; import { CallableTool, Part, FunctionCall, Schema } from '@google/genai';
type ToolParams = Record<string, unknown>; type ToolParams = Record<string, unknown>;
@ -23,7 +23,7 @@ export class DiscoveredMCPTool extends BaseTool<ToolParams, ToolResult> {
readonly serverName: string, readonly serverName: string,
readonly name: string, readonly name: string,
readonly description: string, readonly description: string,
readonly parameterSchema: Record<string, unknown>, readonly parameterSchema: Schema,
readonly serverToolName: string, readonly serverToolName: string,
readonly timeout?: number, readonly timeout?: number,
readonly trust?: boolean, readonly trust?: boolean,

View File

@ -5,19 +5,20 @@
*/ */
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { FunctionDeclaration, Type } from '@google/genai';
import * as fs from 'fs/promises'; import * as fs from 'fs/promises';
import * as path from 'path'; import * as path from 'path';
import { homedir } from 'os'; import { homedir } from 'os';
const memoryToolSchemaData = { const memoryToolSchemaData: FunctionDeclaration = {
name: 'save_memory', name: 'save_memory',
description: description:
'Saves a specific piece of information or fact to your long-term memory. Use this when the user explicitly asks you to remember something, or when they state a clear, concise fact that seems important to retain for future interactions.', 'Saves a specific piece of information or fact to your long-term memory. Use this when the user explicitly asks you to remember something, or when they state a clear, concise fact that seems important to retain for future interactions.',
parameters: { parameters: {
type: 'object', type: Type.OBJECT,
properties: { properties: {
fact: { fact: {
type: 'string', type: Type.STRING,
description: description:
'The specific fact or piece of information to remember. Should be a clear, self-contained statement.', 'The specific fact or piece of information to remember. Should be a clear, self-contained statement.',
}, },
@ -98,7 +99,7 @@ function ensureNewlineSeparation(currentContent: string): string {
} }
export class MemoryTool extends BaseTool<SaveMemoryParams, ToolResult> { export class MemoryTool extends BaseTool<SaveMemoryParams, ToolResult> {
static readonly Name: string = memoryToolSchemaData.name; static readonly Name: string = memoryToolSchemaData.name!;
constructor() { constructor() {
super( super(
MemoryTool.Name, MemoryTool.Name,

View File

@ -73,8 +73,8 @@ describe('ReadFileTool', () => {
it('should return error for relative path', () => { it('should return error for relative path', () => {
const params: ReadFileToolParams = { absolute_path: 'test.txt' }; const params: ReadFileToolParams = { absolute_path: 'test.txt' };
expect(tool.validateToolParams(params)).toMatch( expect(tool.validateToolParams(params)).toBe(
/File path must be absolute/, `params/absolute_path must match pattern "^/"`,
); );
}); });
@ -119,7 +119,7 @@ describe('ReadFileTool', () => {
it('should return error for schema validation failure (e.g. missing path)', () => { it('should return error for schema validation failure (e.g. missing path)', () => {
const params = { offset: 0 } as unknown as ReadFileToolParams; const params = { offset: 0 } as unknown as ReadFileToolParams;
expect(tool.validateToolParams(params)).toBe( expect(tool.validateToolParams(params)).toBe(
'Parameters failed schema validation.', `params must have required property 'absolute_path'`,
); );
}); });
}); });
@ -143,8 +143,12 @@ describe('ReadFileTool', () => {
it('should return validation error if params are invalid', async () => { it('should return validation error if params are invalid', async () => {
const params: ReadFileToolParams = { absolute_path: 'relative/path.txt' }; const params: ReadFileToolParams = { absolute_path: 'relative/path.txt' };
const result = await tool.execute(params, abortSignal); const result = await tool.execute(params, abortSignal);
expect(result.llmContent).toMatch(/Error: Invalid parameters provided/); expect(result.llmContent).toBe(
expect(result.returnDisplay).toMatch(/File path must be absolute/); 'Error: Invalid parameters provided. Reason: params/absolute_path must match pattern "^/"',
);
expect(result.returnDisplay).toBe(
'params/absolute_path must match pattern "^/"',
);
}); });
it('should return error from processSingleFileContent if it fails', async () => { it('should return error from processSingleFileContent if it fails', async () => {

View File

@ -8,6 +8,7 @@ import path from 'path';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js'; import { makeRelative, shortenPath } from '../utils/paths.js';
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { import {
isWithinRoot, isWithinRoot,
processSingleFileContent, processSingleFileContent,
@ -58,37 +59,33 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> {
absolute_path: { absolute_path: {
description: description:
"The absolute path to the file to read (e.g., '/home/user/project/file.txt'). Relative paths are not supported. You must provide an absolute path.", "The absolute path to the file to read (e.g., '/home/user/project/file.txt'). Relative paths are not supported. You must provide an absolute path.",
type: 'string', type: Type.STRING,
pattern: '^/', pattern: '^/',
}, },
offset: { offset: {
description: 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.", "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: { limit: {
description: 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).", "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: ['absolute_path'], required: ['absolute_path'],
type: 'object', type: Type.OBJECT,
}, },
); );
this.rootDirectory = path.resolve(rootDirectory); this.rootDirectory = path.resolve(rootDirectory);
} }
validateToolParams(params: ReadFileToolParams): string | null { validateToolParams(params: ReadFileToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
const filePath = params.absolute_path; const filePath = params.absolute_path;
if (!path.isAbsolute(filePath)) { if (!path.isAbsolute(filePath)) {
return `File path must be absolute, but was relative: ${filePath}. You must provide an absolute path.`; return `File path must be absolute, but was relative: ${filePath}. You must provide an absolute path.`;

View File

@ -138,7 +138,7 @@ describe('ReadManyFilesTool', () => {
it('should return error if paths array is empty', () => { it('should return error if paths array is empty', () => {
const params = { paths: [] }; const params = { paths: [] };
expect(tool.validateParams(params)).toBe( expect(tool.validateParams(params)).toBe(
'The "paths" parameter is required and must be a non-empty array of strings/glob patterns.', 'params/paths must NOT have fewer than 1 items',
); );
}); });
@ -154,7 +154,7 @@ describe('ReadManyFilesTool', () => {
it('should return error if paths array contains an empty string', () => { it('should return error if paths array contains an empty string', () => {
const params = { paths: ['file1.txt', ''] }; const params = { paths: ['file1.txt', ''] };
expect(tool.validateParams(params)).toBe( expect(tool.validateParams(params)).toBe(
'Each item in "paths" must be a non-empty string/glob pattern.', 'params/paths/1 must NOT have fewer than 1 characters',
); );
}); });
@ -164,7 +164,7 @@ describe('ReadManyFilesTool', () => {
include: ['*.ts', 123] as string[], include: ['*.ts', 123] as string[],
}; };
expect(tool.validateParams(params)).toBe( expect(tool.validateParams(params)).toBe(
'If provided, "include" must be an array of strings/glob patterns.', 'params/include/1 must be string',
); );
}); });
@ -174,7 +174,7 @@ describe('ReadManyFilesTool', () => {
exclude: ['*.log', {}] as string[], exclude: ['*.log', {}] as string[],
}; };
expect(tool.validateParams(params)).toBe( expect(tool.validateParams(params)).toBe(
'If provided, "exclude" must be an array of strings/glob patterns.', 'params/exclude/1 must be string',
); );
}); });
}); });

View File

@ -16,7 +16,7 @@ import {
DEFAULT_ENCODING, DEFAULT_ENCODING,
getSpecificMimeType, getSpecificMimeType,
} from '../utils/fileUtils.js'; } from '../utils/fileUtils.js';
import { PartListUnion } from '@google/genai'; import { PartListUnion, Schema, Type } from '@google/genai';
import { Config } from '../config/config.js'; import { Config } from '../config/config.js';
import { import {
recordFileOperationMetric, recordFileOperationMetric,
@ -135,43 +135,53 @@ export class ReadManyFilesTool extends BaseTool<
readonly targetDir: string, readonly targetDir: string,
private config: Config, private config: Config,
) { ) {
const parameterSchema: Record<string, unknown> = { const parameterSchema: Schema = {
type: 'object', type: Type.OBJECT,
properties: { properties: {
paths: { paths: {
type: 'array', type: Type.ARRAY,
items: { type: 'string' }, items: {
type: Type.STRING,
minLength: '1',
},
minItems: '1',
description: description:
"Required. An array of glob patterns or paths relative to the tool's target directory. Examples: ['src/**/*.ts'], ['README.md', 'docs/']", "Required. An array of glob patterns or paths relative to the tool's target directory. Examples: ['src/**/*.ts'], ['README.md', 'docs/']",
}, },
include: { include: {
type: 'array', type: Type.ARRAY,
items: { type: 'string' }, items: {
type: Type.STRING,
minLength: '1',
},
description: description:
'Optional. Additional glob patterns to include. These are merged with `paths`. Example: ["*.test.ts"] to specifically add test files if they were broadly excluded.', 'Optional. Additional glob patterns to include. These are merged with `paths`. Example: ["*.test.ts"] to specifically add test files if they were broadly excluded.',
default: [], default: [],
}, },
exclude: { exclude: {
type: 'array', type: Type.ARRAY,
items: { type: 'string' }, items: {
type: Type.STRING,
minLength: '1',
},
description: description:
'Optional. Glob patterns for files/directories to exclude. Added to default excludes if useDefaultExcludes is true. Example: ["**/*.log", "temp/"]', 'Optional. Glob patterns for files/directories to exclude. Added to default excludes if useDefaultExcludes is true. Example: ["**/*.log", "temp/"]',
default: [], default: [],
}, },
recursive: { recursive: {
type: 'boolean', type: Type.BOOLEAN,
description: description:
'Optional. Whether to search recursively (primarily controlled by `**` in glob patterns). Defaults to true.', 'Optional. Whether to search recursively (primarily controlled by `**` in glob patterns). Defaults to true.',
default: true, default: true,
}, },
useDefaultExcludes: { useDefaultExcludes: {
type: 'boolean', type: Type.BOOLEAN,
description: description:
'Optional. Whether to apply a list of default exclusion patterns (e.g., node_modules, .git, binary files). Defaults to true.', 'Optional. Whether to apply a list of default exclusion patterns (e.g., node_modules, .git, binary files). Defaults to true.',
default: true, default: true,
}, },
respect_git_ignore: { respect_git_ignore: {
type: 'boolean', type: Type.BOOLEAN,
description: description:
'Optional. Whether to respect .gitignore patterns when discovering files. Only available in git repositories. Defaults to true.', 'Optional. Whether to respect .gitignore patterns when discovering files. Only available in git repositories. Defaults to true.',
default: true, default: true,
@ -202,47 +212,9 @@ Use this tool when the user's query implies needing the content of several files
} }
validateParams(params: ReadManyFilesParams): string | null { validateParams(params: ReadManyFilesParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
!params.paths || if (errors) {
!Array.isArray(params.paths) || return errors;
params.paths.length === 0
) {
return 'The "paths" parameter is required and must be a non-empty array of strings/glob patterns.';
}
if (
this.schema.parameters &&
!SchemaValidator.validate(
this.schema.parameters as Record<string, unknown>,
params,
)
) {
if (
!params.paths ||
!Array.isArray(params.paths) ||
params.paths.length === 0
) {
return 'The "paths" parameter is required and must be a non-empty array of strings/glob patterns.';
}
return 'Parameters failed schema validation. Ensure "paths" is a non-empty array and other parameters match their expected types.';
}
for (const p of params.paths) {
if (typeof p !== 'string' || p.trim() === '') {
return 'Each item in "paths" must be a non-empty string/glob pattern.';
}
}
if (
params.include &&
(!Array.isArray(params.include) ||
!params.include.every((item) => typeof item === 'string'))
) {
return 'If provided, "include" must be an array of strings/glob patterns.';
}
if (
params.exclude &&
(!Array.isArray(params.exclude) ||
!params.exclude.every((item) => typeof item === 'string'))
) {
return 'If provided, "exclude" must be an array of strings/glob patterns.';
} }
return null; return null;
} }

View File

@ -16,6 +16,7 @@ import {
ToolExecuteConfirmationDetails, ToolExecuteConfirmationDetails,
ToolConfirmationOutcome, ToolConfirmationOutcome,
} from './tools.js'; } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { getErrorMessage } from '../utils/errors.js'; import { getErrorMessage } from '../utils/errors.js';
import stripAnsi from 'strip-ansi'; import stripAnsi from 'strip-ansi';
@ -51,19 +52,19 @@ Signal: Signal number or \`(none)\` if no signal was received.
Background PIDs: List of background processes started or \`(none)\`. Background PIDs: List of background processes started or \`(none)\`.
Process Group PGID: Process group started or \`(none)\``, Process Group PGID: Process group started or \`(none)\``,
{ {
type: 'object', type: Type.OBJECT,
properties: { properties: {
command: { command: {
type: 'string', type: Type.STRING,
description: 'Exact bash command to execute as `bash -c <command>`', description: 'Exact bash command to execute as `bash -c <command>`',
}, },
description: { description: {
type: 'string', type: Type.STRING,
description: 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.', '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.',
}, },
directory: { directory: {
type: 'string', type: Type.STRING,
description: description:
'(OPTIONAL) Directory to run the command in, if not the project root directory. Must be relative to the project root directory and must already exist.', '(OPTIONAL) Directory to run the command in, if not the project root directory. Must be relative to the project root directory and must already exist.',
}, },
@ -223,13 +224,9 @@ Process Group PGID: Process group started or \`(none)\``,
} }
return commandCheck.reason; return commandCheck.reason;
} }
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
!SchemaValidator.validate( if (errors) {
this.parameterSchema as Record<string, unknown>, return errors;
params,
)
) {
return `Parameters failed schema validation.`;
} }
if (!params.command.trim()) { if (!params.command.trim()) {
return 'Command cannot be empty.'; return 'Command cannot be empty.';

View File

@ -106,9 +106,9 @@ const createMockCallableTool = (
class MockTool extends BaseTool<{ param: string }, ToolResult> { class MockTool extends BaseTool<{ param: string }, ToolResult> {
constructor(name = 'mock-tool', description = 'A mock tool') { constructor(name = 'mock-tool', description = 'A mock tool') {
super(name, name, description, { super(name, name, description, {
type: 'object', type: Type.OBJECT,
properties: { properties: {
param: { type: 'string' }, param: { type: Type.STRING },
}, },
required: ['param'], required: ['param'],
}); });

View File

@ -103,7 +103,7 @@ export abstract class BaseTool<
readonly name: string, readonly name: string,
readonly displayName: string, readonly displayName: string,
readonly description: string, readonly description: string,
readonly parameterSchema: Record<string, unknown>, readonly parameterSchema: Schema,
readonly isOutputMarkdown: boolean = true, readonly isOutputMarkdown: boolean = true,
readonly canUpdateOutput: boolean = false, readonly canUpdateOutput: boolean = false,
) {} ) {}
@ -115,7 +115,7 @@ export abstract class BaseTool<
return { return {
name: this.name, name: this.name,
description: this.description, description: this.description,
parameters: this.parameterSchema as Schema, parameters: this.parameterSchema,
}; };
} }

View File

@ -11,6 +11,7 @@ import {
ToolCallConfirmationDetails, ToolCallConfirmationDetails,
ToolConfirmationOutcome, ToolConfirmationOutcome,
} from './tools.js'; } from './tools.js';
import { Type } from '@google/genai';
import { getErrorMessage } from '../utils/errors.js'; import { getErrorMessage } from '../utils/errors.js';
import { Config, ApprovalMode } from '../config/config.js'; import { Config, ApprovalMode } from '../config/config.js';
import { getResponseText } from '../utils/generateContentResponseUtilities.js'; import { getResponseText } from '../utils/generateContentResponseUtilities.js';
@ -73,11 +74,11 @@ export class WebFetchTool extends BaseTool<WebFetchToolParams, ToolResult> {
prompt: { prompt: {
description: description:
'A comprehensive prompt that includes the URL(s) (up to 20) to fetch and specific instructions on how to process their content (e.g., "Summarize https://example.com/article and extract key points from https://another.com/data"). Must contain as least one URL starting with http:// or https://.', 'A comprehensive prompt that includes the URL(s) (up to 20) to fetch and specific instructions on how to process their content (e.g., "Summarize https://example.com/article and extract key points from https://another.com/data"). Must contain as least one URL starting with http:// or https://.',
type: 'string', type: Type.STRING,
}, },
}, },
required: ['prompt'], required: ['prompt'],
type: 'object', type: Type.OBJECT,
}, },
); );
} }
@ -148,14 +149,9 @@ ${textContent}
} }
validateParams(params: WebFetchToolParams): string | null { validateParams(params: WebFetchToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
if (!params.prompt || params.prompt.trim() === '') { if (!params.prompt || params.prompt.trim() === '') {
return "The 'prompt' parameter cannot be empty and must contain URL(s) and instructions."; return "The 'prompt' parameter cannot be empty and must contain URL(s) and instructions.";

View File

@ -6,6 +6,7 @@
import { GroundingMetadata } from '@google/genai'; import { GroundingMetadata } from '@google/genai';
import { BaseTool, ToolResult } from './tools.js'; import { BaseTool, ToolResult } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { getErrorMessage } from '../utils/errors.js'; import { getErrorMessage } from '../utils/errors.js';
@ -69,10 +70,10 @@ export class WebSearchTool extends BaseTool<
'GoogleSearch', 'GoogleSearch',
'Performs a web search using Google Search (via the Gemini API) and returns the results. This tool is useful for finding information on the internet based on a query.', 'Performs a web search using Google Search (via the Gemini API) and returns the results. This tool is useful for finding information on the internet based on a query.',
{ {
type: 'object', type: Type.OBJECT,
properties: { properties: {
query: { query: {
type: 'string', type: Type.STRING,
description: 'The search query to find information on the web.', description: 'The search query to find information on the web.',
}, },
}, },
@ -86,16 +87,12 @@ export class WebSearchTool extends BaseTool<
* @param params The parameters to validate * @param params The parameters to validate
* @returns An error message string if validation fails, null if valid * @returns An error message string if validation fails, null if valid
*/ */
validateToolParams(params: WebSearchToolParams): string | null { validateParams(params: WebSearchToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return "Parameters failed schema validation. Ensure 'query' is a string.";
} }
if (!params.query || params.query.trim() === '') { if (!params.query || params.query.trim() === '') {
return "The 'query' parameter cannot be empty."; return "The 'query' parameter cannot be empty.";
} }

View File

@ -16,6 +16,7 @@ import {
ToolConfirmationOutcome, ToolConfirmationOutcome,
ToolCallConfirmationDetails, ToolCallConfirmationDetails,
} from './tools.js'; } from './tools.js';
import { Type } from '@google/genai';
import { SchemaValidator } from '../utils/schemaValidator.js'; import { SchemaValidator } from '../utils/schemaValidator.js';
import { makeRelative, shortenPath } from '../utils/paths.js'; import { makeRelative, shortenPath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js'; import { getErrorMessage, isNodeError } from '../utils/errors.js';
@ -79,15 +80,15 @@ export class WriteFileTool
file_path: { file_path: {
description: description:
"The absolute path to the file to write to (e.g., '/home/user/project/file.txt'). Relative paths are not supported.", "The absolute path to the file to write to (e.g., '/home/user/project/file.txt'). Relative paths are not supported.",
type: 'string', type: Type.STRING,
}, },
content: { content: {
description: 'The content to write to the file.', description: 'The content to write to the file.',
type: 'string', type: Type.STRING,
}, },
}, },
required: ['file_path', 'content'], required: ['file_path', 'content'],
type: 'object', type: Type.OBJECT,
}, },
); );
} }
@ -112,15 +113,11 @@ export class WriteFileTool
} }
validateToolParams(params: WriteFileToolParams): string | null { validateToolParams(params: WriteFileToolParams): string | null {
if ( const errors = SchemaValidator.validate(this.schema.parameters, params);
this.schema.parameters && if (errors) {
!SchemaValidator.validate( return errors;
this.schema.parameters as Record<string, unknown>,
params,
)
) {
return 'Parameters failed schema validation.';
} }
const filePath = params.file_path; const filePath = params.file_path;
if (!path.isAbsolute(filePath)) { if (!path.isAbsolute(filePath)) {
return `File path must be absolute: ${filePath}`; return `File path must be absolute: ${filePath}`;

View File

@ -4,55 +4,63 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import { Schema } from '@google/genai';
import * as ajv from 'ajv';
const ajValidator = new ajv.Ajv();
/** /**
* Simple utility to validate objects against JSON Schemas * Simple utility to validate objects against JSON Schemas
*/ */
export class SchemaValidator { export class SchemaValidator {
/** /**
* Validates data against a JSON schema * Returns null if the data confroms to the schema described by schema (or if schema
* @param schema JSON Schema to validate against * is null). Otherwise, returns a string describing the error.
* @param data Data to validate
* @returns True if valid, false otherwise
*/ */
static validate(schema: Record<string, unknown>, data: unknown): boolean { static validate(schema: Schema | undefined, data: unknown): string | null {
// This is a simplified implementation if (!schema) {
// In a real application, you would use a library like Ajv for proper validation return null;
// Check for required fields
if (schema.required && Array.isArray(schema.required)) {
const required = schema.required as string[];
const dataObj = data as Record<string, unknown>;
for (const field of required) {
if (dataObj[field] === undefined) {
console.error(`Missing required field: ${field}`);
return false;
}
}
} }
if (typeof data !== 'object' || data === null) {
// Check property types if properties are defined return 'Value of params must be an object';
if (schema.properties && typeof schema.properties === 'object') {
const properties = schema.properties as Record<string, { type?: string }>;
const dataObj = data as Record<string, unknown>;
for (const [key, prop] of Object.entries(properties)) {
if (dataObj[key] !== undefined && prop.type) {
const expectedType = prop.type;
const actualType = Array.isArray(dataObj[key])
? 'array'
: typeof dataObj[key];
if (expectedType !== actualType) {
console.error(
`Type mismatch for property "${key}": expected ${expectedType}, got ${actualType}`,
);
return false;
}
}
}
} }
const validate = ajValidator.compile(this.toObjectSchema(schema));
const valid = validate(data);
if (!valid && validate.errors) {
return ajValidator.errorsText(validate.errors, { dataVar: 'params' });
}
return null;
}
return true; /**
* Converts @google/genai's Schema to an object compatible with avj.
* This is necessry because it represents Types as an Enum (with
* UPPERCASE values) and minItems and minLength as strings, when they should be numbers.
*/
private static toObjectSchema(schema: Schema): object {
const newSchema: Record<string, unknown> = { ...schema };
if (newSchema.anyOf && Array.isArray(newSchema.anyOf)) {
newSchema.anyOf = newSchema.anyOf.map((v) => this.toObjectSchema(v));
}
if (newSchema.items) {
newSchema.items = this.toObjectSchema(newSchema.items);
}
if (newSchema.properties && typeof newSchema.properties === 'object') {
const newProperties: Record<string, unknown> = {};
for (const [key, value] of Object.entries(newSchema.properties)) {
newProperties[key] = this.toObjectSchema(value as Schema);
}
newSchema.properties = newProperties;
}
if (newSchema.type) {
newSchema.type = String(newSchema.type).toLowerCase();
}
if (newSchema.minItems) {
newSchema.minItems = Number(newSchema.minItems);
}
if (newSchema.minLength) {
newSchema.minLength = Number(newSchema.minLength);
}
return newSchema;
} }
} }