diff --git a/src/tool.ts b/src/tool.ts index ed0cab26..11f76202 100644 --- a/src/tool.ts +++ b/src/tool.ts @@ -134,14 +134,8 @@ export class Tools { isError: true, }; } - // // @ts-expect-error - // const result = validateToolParams(tool.parameters, args); - // if (!result.success) { - // return { - // llmContent: `Invalid tool parameters: ${result.error}`, - // isError: true, - // }; - // } + + // Parse JSON arguments let argsObj: any; try { argsObj = JSON.parse(args); @@ -151,6 +145,32 @@ export class Tools { isError: true, }; } + + // Validate parameters using Zod schema (skip for MCP tools) + const isMcpTool = toolName.startsWith('mcp__'); + if (!isMcpTool) { + const { validateToolParams, sanitizeToolParams } = await import( + './utils/validateToolParams' + ); + + // Try to sanitize parameters first (auto-fix common issues) + argsObj = sanitizeToolParams(tool.parameters, argsObj); + + // Validate after sanitization + const validationResult = validateToolParams(tool.parameters, argsObj); + if (!validationResult.success) { + // TODO: Add retry limit to prevent infinite loops when LLM repeatedly fails + // Current behavior: LLM will retry up to maxTurns (default 50) times + // Future improvement: Track consecutive identical parameter errors and fail fast after 3 attempts + // This should be implemented in loop.ts by detecting repeated tool+params failures + // Example: if (consecutiveSameErrors >= 3) { return 'repeated_tool_error' } + return { + llmContent: validationResult.error, + isError: true, + }; + } + } + return await tool.execute(argsObj); } diff --git a/src/tools/askUserQuestion.ts b/src/tools/askUserQuestion.ts index 0420d605..74013a06 100644 --- a/src/tools/askUserQuestion.ts +++ b/src/tools/askUserQuestion.ts @@ -58,6 +58,10 @@ const AskUserQuestionInputSchema = z }) .refine( (data) => { + // Defensive check: ensure questions is an array + if (!Array.isArray(data.questions)) { + return false; + } const questionTexts = data.questions.map((q) => q.question); return questionTexts.length === new Set(questionTexts).size; }, @@ -68,7 +72,14 @@ const AskUserQuestionInputSchema = z ) .refine( (data) => { + // Defensive check: ensure questions is an array + if (!Array.isArray(data.questions)) { + return false; + } for (const question of data.questions) { + if (!question.options || !Array.isArray(question.options)) { + return false; + } const labels = question.options.map((o) => o.label); if (labels.length !== new Set(labels).size) { return false; diff --git a/src/tools/bash.ts b/src/tools/bash.ts index cb38d9c3..bb0f928d 100644 --- a/src/tools/bash.ts +++ b/src/tools/bash.ts @@ -654,7 +654,7 @@ Usage: if (!params.task_id || typeof params.task_id !== 'string') { return 'Read background task output'; } - return `Read output from task: ${params.task_id}`; + return `Read output from task: ${String(params.task_id)}`; }, execute: async ({ task_id }) => { const task = backgroundTaskManager.getTask(task_id); @@ -713,7 +713,7 @@ Usage: if (!params.task_id || typeof params.task_id !== 'string') { return 'Terminate background task'; } - return `Terminate task: ${params.task_id}`; + return `Terminate task: ${String(params.task_id)}`; }, execute: async ({ task_id }) => { const task = backgroundTaskManager.getTask(task_id); @@ -830,8 +830,14 @@ cd /foo/bar && pytest tests if (!params.command || typeof params.command !== 'string') { return 'No command provided'; } - const command = params.command.trim(); - return command.length > 100 ? `${command.substring(0, 97)}...` : command; + try { + const command = String(params.command).trim(); + return command.length > 100 + ? `${command.substring(0, 97)}...` + : command; + } catch (error) { + return 'Invalid command'; + } }, execute: async ({ command, diff --git a/src/tools/edit.ts b/src/tools/edit.ts index eb5530ef..5e34eb71 100644 --- a/src/tools/edit.ts +++ b/src/tools/edit.ts @@ -34,7 +34,16 @@ Usage: if (!params.file_path || typeof params.file_path !== 'string') { return 'No file path provided'; } - return path.relative(cwd, params.file_path); + // Ensure cwd is a string to prevent .trim() errors in path.relative() + if (typeof cwd !== 'string') { + return params.file_path; + } + try { + return path.relative(cwd, params.file_path); + } catch (error) { + // Fallback if path.relative fails + return params.file_path; + } }, execute: async ({ file_path, old_string, new_string, replace_all }) => { try { diff --git a/src/tools/fetch.ts b/src/tools/fetch.ts index 0c878f82..b5fc89df 100644 --- a/src/tools/fetch.ts +++ b/src/tools/fetch.ts @@ -25,7 +25,7 @@ Remembers: if (!params.url || typeof params.url !== 'string') { return 'No URL provided'; } - return params.url; + return String(params.url); }, execute: async ({ url, prompt }) => { try { diff --git a/src/tools/glob.ts b/src/tools/glob.ts index bae21c30..c9d2886d 100644 --- a/src/tools/glob.ts +++ b/src/tools/glob.ts @@ -23,7 +23,7 @@ Glob if (!params.pattern || typeof params.pattern !== 'string') { return 'No pattern provided'; } - return params.pattern; + return String(params.pattern); }, execute: async ({ pattern, path }) => { try { diff --git a/src/tools/grep.ts b/src/tools/grep.ts index 39554d93..a2d86eaf 100644 --- a/src/tools/grep.ts +++ b/src/tools/grep.ts @@ -32,7 +32,7 @@ export function createGrepTool(opts: { cwd: string }) { if (!params.pattern || typeof params.pattern !== 'string') { return 'No pattern provided'; } - return params.pattern; + return String(params.pattern); }, execute: async ({ pattern, search_path, include, limit }) => { try { diff --git a/src/tools/ls.ts b/src/tools/ls.ts index fa6ae7ca..ce745f6e 100644 --- a/src/tools/ls.ts +++ b/src/tools/ls.ts @@ -16,11 +16,20 @@ export function createLSTool(opts: { cwd: string }) { parameters: z.object({ dir_path: z.string().describe('The path to the directory to list.'), }), - getDescription: ({ params }) => { + getDescription: ({ params, cwd }) => { if (!params.dir_path || typeof params.dir_path !== 'string') { return '.'; } - return path.relative(opts.cwd, params.dir_path); + // Ensure cwd is a string to prevent .trim() errors in path.relative() + if (typeof cwd !== 'string') { + return params.dir_path; + } + try { + return path.relative(cwd, params.dir_path); + } catch (error) { + // Fallback if path.relative fails + return params.dir_path; + } }, execute: async (params) => { const { dir_path } = params; diff --git a/src/tools/read.ts b/src/tools/read.ts index 55bb0c98..3e30014b 100644 --- a/src/tools/read.ts +++ b/src/tools/read.ts @@ -115,7 +115,16 @@ Usage: if (!params.file_path || typeof params.file_path !== 'string') { return 'No file path provided'; } - return path.relative(cwd, params.file_path); + // Ensure cwd is a string to prevent .trim() errors in path.relative() + if (typeof cwd !== 'string') { + return params.file_path; + } + try { + return path.relative(cwd, params.file_path); + } catch (error) { + // Fallback if path.relative fails + return params.file_path; + } }, execute: async ({ file_path, offset, limit }) => { try { diff --git a/src/tools/write.ts b/src/tools/write.ts index 8b634e35..7551c1dd 100644 --- a/src/tools/write.ts +++ b/src/tools/write.ts @@ -15,7 +15,16 @@ export function createWriteTool(opts: { cwd: string }) { if (!params.file_path || typeof params.file_path !== 'string') { return 'No file path provided'; } - return path.relative(cwd, params.file_path); + // Ensure cwd is a string to prevent .trim() errors in path.relative() + if (typeof cwd !== 'string') { + return params.file_path; + } + try { + return path.relative(cwd, params.file_path); + } catch (error) { + // Fallback if path.relative fails + return params.file_path; + } }, execute: async ({ file_path, content }) => { try { diff --git a/src/utils/validateToolParams.ts b/src/utils/validateToolParams.ts new file mode 100644 index 00000000..ba8a3448 --- /dev/null +++ b/src/utils/validateToolParams.ts @@ -0,0 +1,212 @@ +import type * as z from 'zod'; + +/** + * Check if a schema is a valid Zod object + */ +export function isZodObject(schema: any): schema is z.ZodTypeAny { + return schema && typeof schema.safeParse === 'function'; +} + +/** + * Validate tool parameters using Zod schema + * Returns validation result with helpful error messages for LLM to auto-fix + */ +export function validateToolParams( + schema: z.ZodTypeAny, + params: any, +): { success: true } | { success: false; error: string } { + if (!isZodObject(schema)) { + return { success: true }; // Skip validation for non-Zod schemas + } + + let result; + try { + result = schema.safeParse(params); + } catch (error) { + // Catch any errors during parsing itself + return { + success: false, + error: `Parameter validation error: ${error instanceof Error ? error.message : String(error)}`, + }; + } + + if (result.success) { + return { success: true }; + } + + // Generate helpful error messages for LLM to understand and fix + try { + // Safely handle issues array - add defensive check + const issues = Array.isArray(result.error.issues) + ? result.error.issues + : []; + + if (issues.length === 0) { + return { + success: false, + error: `Parameter validation failed: ${result.error.message || 'Unknown error'}`, + }; + } + + const errorMessages = issues.map((issue, index) => { + try { + const path = + issue.path && issue.path.length > 0 + ? issue.path.map(String).join('.') + : 'root'; + let message = `${index + 1}. Field '${path}': `; + + if (issue.code === 'invalid_type') { + const received = (issue as any).received; + const expected = (issue as any).expected; + + if (received === 'undefined') { + message += `Required field is missing`; + message += `\n Expected type: ${expected}`; + message += `\n Suggestion: Provide a valid ${expected} value`; + } else { + message += `Type mismatch`; + message += `\n Expected: ${expected}`; + message += `\n Received: ${received}`; + // Add specific suggestions for common type conversions + if (expected === 'string' && received === 'number') { + message += `\n Suggestion: Convert to string (e.g., "${received}" instead of ${received})`; + } else if (expected === 'number' && received === 'string') { + message += `\n Suggestion: Convert to number (remove quotes)`; + } else if (expected === 'boolean' && received === 'string') { + message += `\n Suggestion: Use boolean value (true or false without quotes)`; + } + } + } else if (issue.code === 'too_small') { + const minimum = (issue as any).minimum; + message += `Value is too small`; + message += `\n Minimum allowed: ${minimum}`; + message += `\n Suggestion: Provide a value >= ${minimum}`; + } else if (issue.code === 'too_big') { + const maximum = (issue as any).maximum; + message += `Value is too large`; + message += `\n Maximum allowed: ${maximum}`; + message += `\n Suggestion: Provide a value <= ${maximum}`; + } else { + // Handle other validation errors including enum values + message += issue.message || 'validation failed'; + // Add allowed values hint for enum-like errors if available + const options = (issue as any).options; + if (options && Array.isArray(options)) { + message += `\n Allowed values: ${options.join(', ')}`; + } + } + + return message; + } catch (err) { + // Fallback for individual issue formatting errors + return `${index + 1}. Validation error: ${issue.message || 'unknown'}`; + } + }); + + const errorMessage = `Parameter validation failed:\n\n${errorMessages.join('\n\n')}\n\nPlease correct these parameters according to the tool's schema and try again.`; + + return { + success: false, + error: errorMessage, + }; + } catch (error) { + // Fallback if error formatting itself fails + return { + success: false, + error: `Parameter validation failed: ${result.error.message || 'Unable to format error details'}`, + }; + } +} + +/** + * Sanitize parameters by providing default values for common issues + * This helps when LLM provides slightly incorrect parameters + */ +export function sanitizeToolParams(schema: z.ZodTypeAny, params: any): any { + if (!isZodObject(schema)) { + return params; + } + + // First try to parse as-is + let result; + try { + result = schema.safeParse(params); + if (result.success) { + return result.data; // Return parsed data with defaults applied + } + } catch (error) { + // If parsing throws, return original params + return params; + } + + // If validation fails, try to fix common issues + const sanitized = { ...params }; + + try { + // Safely handle issues array + const issues = Array.isArray(result.error.issues) + ? result.error.issues + : []; + + issues.forEach((issue) => { + try { + if (issue.code === 'invalid_type' && issue.path) { + const path = issue.path.map(String) as (string | number)[]; + const expected = (issue as any).expected; + const received = (issue as any).received; + + // Try to convert types + if (expected === 'string' && received === 'number') { + setNestedValue( + sanitized, + path, + String(getNestedValue(params, path)), + ); + } else if (expected === 'number' && received === 'string') { + const value = getNestedValue(params, path); + const parsed = Number(value); + if (!isNaN(parsed)) { + setNestedValue(sanitized, path, parsed); + } + } else if (expected === 'boolean' && received === 'string') { + const value = getNestedValue(params, path); + setNestedValue( + sanitized, + path, + value === 'true' || value === '1' || value === 'yes', + ); + } + } + } catch (err) { + // Skip this issue if we can't process it + } + }); + } catch (error) { + // If any error occurs during sanitization, return original params + return params; + } + + // Try again with sanitized values + try { + const retryResult = schema.safeParse(sanitized); + return retryResult.success ? retryResult.data : params; + } catch (error) { + return params; + } +} + +// Helper functions for nested object access +function getNestedValue(obj: any, path: (string | number)[]): any { + return path.reduce((current, key) => current?.[key], obj); +} + +function setNestedValue(obj: any, path: (string | number)[], value: any): void { + const lastKey = path[path.length - 1]; + const parent = path + .slice(0, -1) + .reduce((current, key) => current?.[key], obj); + if (parent) { + parent[lastKey] = value; + } +}