From 8d3fec08e59b100da036b685d20b080203ba3a4c Mon Sep 17 00:00:00 2001 From: moon jooho Date: Fri, 4 Jul 2025 09:13:02 +0900 Subject: [PATCH] Add and improve JSDoc comments for core tool methods (#3128) --- CONTRIBUTING.md | 2 +- .../cli/src/ui/hooks/useToolScheduler.test.ts | 4 ++-- packages/core/src/tools/glob.ts | 6 +++++- packages/core/src/tools/mcp-client.ts | 17 +++++++++++++++++ packages/core/src/tools/shell.ts | 16 ++++++++++++++++ packages/core/src/tools/web-search.ts | 9 +++++++-- packages/core/src/tools/write-file.ts | 7 +++++++ 7 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 332c6128..2a5e70a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -134,7 +134,7 @@ To start the Gemini CLI from the source code (after building), run the following npm start ``` -If you’d like to run the source build outside of the gemini-cli folder you can utilize `npm link path/to/gemini-cli/packages/cli` (see: [docs](https://docs.npmjs.com/cli/v9/commands/npm-link)) or `alias gemini="node path/to/gemini-cli/packages/cli"` to run with `gemini` +If you'd like to run the source build outside of the gemini-cli folder you can utilize `npm link path/to/gemini-cli/packages/cli` (see: [docs](https://docs.npmjs.com/cli/v9/commands/npm-link)) or `alias gemini="node path/to/gemini-cli/packages/cli"` to run with `gemini` ### Running Tests diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index 4b88adaa..d448f0e2 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -560,8 +560,8 @@ describe('useReactToolScheduler', () => { (mockToolWithLiveOutput.execute as Mock).mockImplementation( async ( - _args: any, - _signal: any, + _args: Record, + _signal: AbortSignal, updateFn: ((output: string) => void) | undefined, ) => { liveUpdateFn = updateFn; diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 0dd2f411..22dacc83 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -122,7 +122,11 @@ export class GlobTool extends BaseTool { } /** - * Checks if a path is within the root directory. + * Checks if a given path is within the root directory bounds. + * This security check prevents accessing files outside the designated root directory. + * + * @param pathToCheck The absolute path to validate + * @returns True if the path is within the root directory, false otherwise */ private isWithinRoot(pathToCheck: string): boolean { const absolutePathToCheck = path.resolve(pathToCheck); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 52196b80..359ce30a 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -162,6 +162,16 @@ export async function discoverMcpTools( } } +/** + * Connects to an MCP server and discovers available tools, registering them with the tool registry. + * This function handles the complete lifecycle of connecting to a server, discovering tools, + * and cleaning up resources if no tools are found. + * + * @param mcpServerName The name identifier for this MCP server + * @param mcpServerConfig Configuration object containing connection details + * @param toolRegistry The registry to register discovered tools with + * @returns Promise that resolves when discovery is complete + */ async function connectAndDiscover( mcpServerName: string, mcpServerConfig: MCPServerConfig, @@ -375,6 +385,13 @@ async function connectAndDiscover( } } +/** + * Sanitizes a JSON schema object to ensure compatibility with Vertex AI. + * This function recursively processes the schema to remove problematic properties + * that can cause issues with the Gemini API. + * + * @param schema The JSON schema object to sanitize (modified in-place) + */ export function sanitizeParameters(schema?: Schema) { if (!schema) { return; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index bbb998d5..f0a30a9c 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -89,6 +89,15 @@ Process Group PGID: Process group started or \`(none)\``, return description; } + /** + * Extracts the root command from a given shell command string. + * This is used to identify the base command for permission checks. + * + * @param command The shell command string to parse + * @returns The root command name, or undefined if it cannot be determined + * @example getCommandRoot("ls -la /tmp") returns "ls" + * @example getCommandRoot("git status && npm test") returns "git" + */ getCommandRoot(command: string): string | undefined { return command .trim() // remove leading and trailing whitespace @@ -98,6 +107,13 @@ Process Group PGID: Process group started or \`(none)\``, .pop(); // take last part and return command root (or undefined if previous line was empty) } + /** + * Determines whether a given shell command is allowed to execute based on + * the tool's configuration including allowlists and blocklists. + * + * @param command The shell command string to validate + * @returns True if the command is allowed to execute, false otherwise + */ isCommandAllowed(command: string): boolean { // 0. Disallow command substitution if (command.includes('$(') || command.includes('`')) { diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index c4dcc54a..5ee8e85c 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -81,7 +81,12 @@ export class WebSearchTool extends BaseTool< ); } - validateParams(params: WebSearchToolParams): string | null { + /** + * Validates the parameters for the WebSearchTool. + * @param params The parameters to validate + * @returns An error message string if validation fails, null if valid + */ + validateToolParams(params: WebSearchToolParams): string | null { if ( this.schema.parameters && !SchemaValidator.validate( @@ -105,7 +110,7 @@ export class WebSearchTool extends BaseTool< params: WebSearchToolParams, signal: AbortSignal, ): Promise { - const validationError = this.validateParams(params); + const validationError = this.validateToolParams(params); if (validationError) { return { llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`, diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 2d5e85be..c343cab8 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -96,6 +96,13 @@ export class WriteFileTool this.client = this.config.getGeminiClient(); } + /** + * Checks if a given path is within the root directory bounds. + * This security check prevents writing files outside the designated root directory. + * + * @param pathToCheck The absolute path to validate + * @returns True if the path is within the root directory, false otherwise + */ private isWithinRoot(pathToCheck: string): boolean { const normalizedPath = path.normalize(pathToCheck); const normalizedRoot = path.normalize(this.config.getTargetDir());