diff --git a/common/src/tools/params/__tests__/coerce-to-array.test.ts b/common/src/tools/params/__tests__/coerce-to-array.test.ts index 64cba36a9..ece3e12c4 100644 --- a/common/src/tools/params/__tests__/coerce-to-array.test.ts +++ b/common/src/tools/params/__tests__/coerce-to-array.test.ts @@ -1,12 +1,14 @@ import { describe, expect, it } from 'bun:test' import z from 'zod/v4' -import { coerceToArray } from '../utils' +import { coerceToArray, normalizeReplacementAliases } from '../utils' describe('coerceToArray', () => { it('passes through arrays unchanged', () => { expect(coerceToArray(['a', 'b'])).toEqual(['a', 'b']) - expect(coerceToArray([{ old: 'x', new: 'y' }])).toEqual([{ old: 'x', new: 'y' }]) + expect(coerceToArray([{ old: 'x', new: 'y' }])).toEqual([ + { old: 'x', new: 'y' }, + ]) expect(coerceToArray([])).toEqual([]) }) @@ -15,7 +17,9 @@ describe('coerceToArray', () => { }) it('wraps a single object in an array', () => { - expect(coerceToArray({ old: 'x', new: 'y' })).toEqual([{ old: 'x', new: 'y' }]) + expect(coerceToArray({ old: 'x', new: 'y' })).toEqual([ + { old: 'x', new: 'y' }, + ]) }) it('wraps a single number in an array', () => { @@ -23,7 +27,10 @@ describe('coerceToArray', () => { }) it('parses a stringified JSON array', () => { - expect(coerceToArray('["file1.ts", "file2.ts"]')).toEqual(['file1.ts', 'file2.ts']) + expect(coerceToArray('["file1.ts", "file2.ts"]')).toEqual([ + 'file1.ts', + 'file2.ts', + ]) }) it('wraps a non-JSON string (does not parse as array)', () => { @@ -116,3 +123,51 @@ describe('coerceToArray with Zod schemas', () => { expect(coercedSchema).toEqual(plainSchema) }) }) + +describe('normalizeReplacementAliases', () => { + it('maps old_str and new_str onto the documented replacement keys', () => { + expect( + normalizeReplacementAliases({ + old_str: 'before', + new_str: 'after', + allowMultiple: true, + }), + ).toEqual({ + old_str: 'before', + new_str: 'after', + old: 'before', + new: 'after', + allowMultiple: true, + }) + }) + + it('maps old_string and new_string onto the documented replacement keys', () => { + expect( + normalizeReplacementAliases({ + old_string: 'before', + new_string: 'after', + }), + ).toEqual({ + old_string: 'before', + new_string: 'after', + old: 'before', + new: 'after', + }) + }) + + it('does not overwrite documented replacement keys', () => { + expect( + normalizeReplacementAliases({ + old: 'before', + new: 'after', + old_str: 'ignored', + new_str: 'ignored', + }), + ).toEqual({ + old: 'before', + new: 'after', + old_str: 'ignored', + new_str: 'ignored', + }) + }) +}) diff --git a/common/src/tools/params/tool/propose-str-replace.ts b/common/src/tools/params/tool/propose-str-replace.ts index 09223c9bb..d4d774747 100644 --- a/common/src/tools/params/tool/propose-str-replace.ts +++ b/common/src/tools/params/tool/propose-str-replace.ts @@ -1,6 +1,11 @@ import z from 'zod/v4' -import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils' +import { + $getNativeToolCallExampleString, + coerceToArray, + jsonToolResultSchema, + normalizeReplacementAliases, +} from '../utils' import type { $ToolParams } from '../../constants' @@ -30,33 +35,38 @@ const inputSchema = z z .array( z - .object({ - old: z - .string() - .min(1, 'Old cannot be empty') - .describe( - `The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`, - ), - new: z - .string() - .describe( - `The string to replace the corresponding old string with. Can be empty to delete.`, - ), - allowMultiple: z - .boolean() - .optional() - .default(false) - .describe( - 'Whether to allow multiple replacements of old string.', - ), - }) + .preprocess( + normalizeReplacementAliases, + z.object({ + old: z + .string() + .min(1, 'Old cannot be empty') + .describe( + `The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`, + ), + new: z + .string() + .describe( + `The string to replace the corresponding old string with. Can be empty to delete.`, + ), + allowMultiple: z + .boolean() + .optional() + .default(false) + .describe( + 'Whether to allow multiple replacements of old string.', + ), + }), + ) .describe('Pair of old and new strings.'), ) .min(1, 'Replacements cannot be empty'), ) .describe('Array of replacements to make.'), }) - .describe(`Propose string replacements in a file without actually applying them.`) + .describe( + `Propose string replacements in a file without actually applying them.`, + ) const description = ` Propose edits to a file without actually applying them. Use this tool when you want to draft changes that will be reviewed before being applied. diff --git a/common/src/tools/params/tool/str-replace.ts b/common/src/tools/params/tool/str-replace.ts index 1399564ae..60350a627 100644 --- a/common/src/tools/params/tool/str-replace.ts +++ b/common/src/tools/params/tool/str-replace.ts @@ -1,6 +1,11 @@ import z from 'zod/v4' -import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils' +import { + $getNativeToolCallExampleString, + coerceToArray, + jsonToolResultSchema, + normalizeReplacementAliases, +} from '../utils' import type { $ToolParams } from '../../constants' @@ -31,26 +36,29 @@ const inputSchema = z z .array( z - .object({ - old: z - .string() - .min(1, 'Old cannot be empty') - .describe( - `The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`, - ), - new: z - .string() - .describe( - `The string to replace the corresponding old string with. Can be empty to delete.`, - ), - allowMultiple: z - .boolean() - .optional() - .default(false) - .describe( - 'Whether to allow multiple replacements of old string.', - ), - }) + .preprocess( + normalizeReplacementAliases, + z.object({ + old: z + .string() + .min(1, 'Old cannot be empty') + .describe( + `The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`, + ), + new: z + .string() + .describe( + `The string to replace the corresponding old string with. Can be empty to delete.`, + ), + allowMultiple: z + .boolean() + .optional() + .default(false) + .describe( + 'Whether to allow multiple replacements of old string.', + ), + }), + ) .describe('Pair of old and new strings.'), ) .min(1, 'Replacements cannot be empty'), diff --git a/common/src/tools/params/utils.ts b/common/src/tools/params/utils.ts index ead011012..870d7c76c 100644 --- a/common/src/tools/params/utils.ts +++ b/common/src/tools/params/utils.ts @@ -32,6 +32,31 @@ export function coerceToArray(val: unknown): unknown { return val } +/** + * Handles common replacement-key aliases emitted by some models while keeping + * the documented schema stable. + */ +export function normalizeReplacementAliases(val: unknown): unknown { + if (val === null || typeof val !== 'object' || Array.isArray(val)) { + return val + } + + const replacement = { ...(val as Record) } + for (const [target, aliases] of [ + ['old', ['old_str', 'old_string']], + ['new', ['new_str', 'new_string']], + ] as const) { + if (replacement[target] !== undefined) { + continue + } + const alias = aliases.find((key) => typeof replacement[key] === 'string') + if (alias) { + replacement[target] = replacement[alias] + } + } + return replacement +} + /** Only used for generating tool call strings before all tools are defined. * * @param toolName - The name of the tool to call diff --git a/packages/agent-runtime/src/__tests__/tool-validation-error.test.ts b/packages/agent-runtime/src/__tests__/tool-validation-error.test.ts index eb982d368..50ef219ac 100644 --- a/packages/agent-runtime/src/__tests__/tool-validation-error.test.ts +++ b/packages/agent-runtime/src/__tests__/tool-validation-error.test.ts @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it } from 'bun:test' import { mockFileContext } from './test-utils' import { processStream } from '../tools/stream-parser' +import { parseRawToolCall } from '../tools/tool-executor' import type { AgentTemplate } from '../templates/types' import type { @@ -43,6 +44,136 @@ describe('tool validation error handling', () => { stepPrompt: 'Test step prompt', } + it('should parse repeatedly stringified native tool input before validation', () => { + const input = { + path: 'test.ts', + instructions: 'Writes a test file', + content: 'console.log("test")\n', + } + + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'write_file', + toolCallId: 'double-stringified-tool-call-id', + input: JSON.stringify(JSON.stringify(input)), + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input).toEqual(input) + } + }) + + it('should repair bare path values for list_directory string input', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'list_directory', + toolCallId: 'bare-path-tool-call-id', + input: '{"path": web/src/app/api/agents}', + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input).toEqual({ path: 'web/src/app/api/agents' }) + } + }) + + it('should repair bare pattern values for glob string input', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'glob', + toolCallId: 'bare-pattern-tool-call-id', + input: '{"pattern": backend/src/templates/agents/git-committer.ts}', + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input).toEqual({ + pattern: 'backend/src/templates/agents/git-committer.ts', + }) + } + }) + + it('should repair bare paths values for read_files string input', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'read_files', + toolCallId: 'bare-paths-tool-call-id', + input: '{"paths": sdk/src/client.ts}', + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input).toEqual({ paths: ['sdk/src/client.ts'] }) + } + }) + + it('should not repair bare path values for unrelated tools', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'write_file', + toolCallId: 'unrelated-bare-path-tool-call-id', + input: '{"path": web/src/app/api/agents}', + }, + }) + + expect('error' in result).toBe(true) + }) + + it('should accept old_str/new_str aliases for str_replace replacements', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'str_replace', + toolCallId: 'alias-tool-call-id', + input: { + path: 'test.ts', + replacements: [ + { + old_str: 'before', + new_str: 'after', + }, + ], + }, + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input.replacements).toEqual([ + { old: 'before', new: 'after', allowMultiple: false }, + ]) + } + }) + + it('should accept old_string/new_string aliases for str_replace replacements', () => { + const result = parseRawToolCall({ + rawToolCall: { + toolName: 'str_replace', + toolCallId: 'long-alias-tool-call-id', + input: { + path: 'test.ts', + replacements: [ + { + old_string: 'before', + new_string: 'after', + }, + ], + }, + }, + }) + + expect('error' in result).toBe(false) + if (!('error' in result)) { + expect(result.input.replacements).toEqual([ + { old: 'before', new: 'after', allowMultiple: false }, + ]) + } + }) + it('should emit error event instead of tool result when spawn_agents receives invalid parameters', async () => { // This simulates what happens when the LLM passes a string instead of an array to spawn_agents // The error from Anthropic was: "Invalid parameters for spawn_agents: expected array, received string" @@ -100,9 +231,13 @@ describe('tool validation error handling', () => { typeof chunk !== 'string' && chunk.type === 'error', ) expect(errorEvents.length).toBe(1) - expect(errorEvents[0].message).toContain('Invalid parameters for spawn_agents') + expect(errorEvents[0].message).toContain( + 'Invalid parameters for spawn_agents', + ) expect(errorEvents[0].message).toContain('Original tool call input:') - expect(errorEvents[0].message).toContain('this should be an array not a string') + expect(errorEvents[0].message).toContain( + 'this should be an array not a string', + ) // Verify hadToolCallError is true so the agent loop continues expect(result.hadToolCallError).toBe(true) @@ -128,8 +263,7 @@ describe('tool validation error handling', () => { ) const assistantToolCalls = agentState.messageHistory.filter( (m) => - m.role === 'assistant' && - m.content.some((c) => c.type === 'tool-call'), + m.role === 'assistant' && m.content.some((c) => c.type === 'tool-call'), ) // There should be no tool messages at all (the key fix!) @@ -144,8 +278,13 @@ describe('tool validation error handling', () => { const errorUserMessage = userMessages.find((m) => { const contentStr = Array.isArray(m.content) ? m.content.map((p) => ('text' in p ? p.text : '')).join('') - : typeof m.content === 'string' ? m.content : '' - return contentStr.includes('Error during tool call') && contentStr.includes('Invalid parameters for spawn_agents') + : typeof m.content === 'string' + ? m.content + : '' + return ( + contentStr.includes('Error during tool call') && + contentStr.includes('Invalid parameters for spawn_agents') + ) }) expect(errorUserMessage).toBeDefined() }) @@ -460,7 +599,9 @@ describe('tool validation error handling', () => { const assistantToolCallMessages = agentState.messageHistory.filter( (m): m is AssistantMessage => m.role === 'assistant' && - m.content.some((c) => c.type === 'tool-call' && c.toolName === toolName), + m.content.some( + (c) => c.type === 'tool-call' && c.toolName === toolName, + ), ) const toolMessages = agentState.messageHistory.filter( (m): m is ToolMessage => m.role === 'tool' && m.toolName === toolName, @@ -472,8 +613,10 @@ describe('tool validation error handling', () => { const assistantToolCallPart = assistantToolCallMessages[0].content.find( ( c, - ): c is Extract => - c.type === 'tool-call' && c.toolName === toolName, + ): c is Extract< + AssistantMessage['content'][number], + { type: 'tool-call' } + > => c.type === 'tool-call' && c.toolName === toolName, ) expect(assistantToolCallPart).toBeDefined() expect(toolMessages[0].toolCallId).toBe(assistantToolCallPart!.toolCallId) @@ -497,7 +640,8 @@ describe('tool validation error handling', () => { ) const orphanToolResults = agentState.messageHistory.filter( (message): message is ToolMessage => - message.role === 'tool' && !assistantToolCallIds.has(message.toolCallId), + message.role === 'tool' && + !assistantToolCallIds.has(message.toolCallId), ) expect(orphanToolResults.length).toBe(0) }) diff --git a/packages/agent-runtime/src/tools/tool-executor.ts b/packages/agent-runtime/src/tools/tool-executor.ts index fdcf0e709..a3f1a036b 100644 --- a/packages/agent-runtime/src/tools/tool-executor.ts +++ b/packages/agent-runtime/src/tools/tool-executor.ts @@ -48,30 +48,107 @@ export type CustomToolCall = { export type ToolCallError = { toolName?: string - input: Record + input: unknown error: string } & Pick +const bareStringFieldRepairAllowlist: Partial< + Record +> = { + code_search: ['pattern'], + find_files: ['prompt'], + glob: ['pattern'], + list_directory: ['path'], + lookup_agent_info: ['agentId'], + read_files: ['paths'], + read_subtree: ['paths'], + skill: ['name'], + web_search: ['query'], +} + +function repairBareStringFieldObject(input: string, toolName: string): unknown { + const allowedFields = bareStringFieldRepairAllowlist[toolName] + if (!allowedFields) { + return undefined + } + + const match = input + .trim() + .match( + /^\{\s*"([A-Za-z_][A-Za-z0-9_]*)"\s*:\s*([^"{}\[\],][^{}\[\],]*)\s*\}$/, + ) + if (!match) { + return undefined + } + + const [, field, rawValue] = match + if (!allowedFields.includes(field)) { + return undefined + } + + const value = rawValue.trim() + if (!value || value === 'null' || value === 'undefined') { + return undefined + } + + return { [field]: value } +} + +function parseStringifiedToolInput(input: unknown, toolName: string): unknown { + let parsed = input + + // Some providers/models double-encode tool arguments, for example an input + // value like "\"{\\\"path\\\":\\\"file.ts\\\"}\"". Repeated JSON.parse + // handles that before falling back to narrow, tool-specific repairs. + for (let i = 0; i < 3 && typeof parsed === 'string'; i++) { + const stringInput = parsed + try { + parsed = JSON.parse(stringInput) + } catch { + const repaired = repairBareStringFieldObject(stringInput, toolName) + if (repaired !== undefined) { + parsed = repaired + } + break + } + } + + return parsed +} + function stringInputError(toolName: string, toolCallId: string): ToolCallError { return { toolName, toolCallId, input: {}, - error: `Invalid parameters for ${toolName}: tool arguments were a string, not a JSON object. This usually means the model emitted malformed JSON (e.g. unescaped newlines or quotes inside a string value). Re-issue the tool call with properly escaped JSON.`, + error: `Invalid parameters for ${toolName}: tool arguments were a string, not a JSON object. The runtime tried to parse stringified JSON before validation, but the value was still not a JSON object. Re-issue the tool call as a JSON object with properly escaped string values.`, } } +function getToolValidationHint(toolName: string): string | undefined { + if (toolName === 'str_replace' || toolName === 'propose_str_replace') { + return 'Expected shape: { "path": string, "replacements": [{ "old": string, "new": string, "allowMultiple"?: boolean }] }.' + } + if (toolName === 'write_file' || toolName === 'propose_write_file') { + return 'Expected shape: { "path": string, "instructions": string, "content": string }. Quote string values and escape newlines/quotes inside content.' + } + return undefined +} + export function parseRawToolCall(params: { rawToolCall: { toolName: T toolCallId: string - input: Record + input: unknown } }): CodebuffToolCall | ToolCallError { const { rawToolCall } = params const toolName = rawToolCall.toolName - const processedParameters = rawToolCall.input + const processedParameters = parseStringifiedToolInput( + rawToolCall.input, + toolName, + ) const paramsSchema = toolParams[toolName].inputSchema if (typeof processedParameters === 'string') { @@ -81,6 +158,7 @@ export function parseRawToolCall(params: { const result = paramsSchema.safeParse(processedParameters) if (!result.success) { + const hint = getToolValidationHint(toolName) return { toolName, toolCallId: rawToolCall.toolCallId, @@ -89,7 +167,7 @@ export function parseRawToolCall(params: { result.error.issues, null, 2, - )}`, + )}${hint ? `\n\n${hint}` : ''}`, } } @@ -209,9 +287,9 @@ export async function executeToolCall( // TODO: Allow tools to provide a validation function, and move this logic into the spawn_agents validation function. // Pre-validate spawn_agents to filter out non-existent agents before streaming - let effectiveInput = input + let effectiveInput = toolCall.input as Record if (toolName === 'spawn_agents') { - const agents = (input as Record).agents + const agents = effectiveInput.agents if (Array.isArray(agents)) { const BASE_AGENTS = ['base', 'base-free', 'base-max', 'base-experimental'] const isBaseAgent = BASE_AGENTS.includes(agentTemplate.id) @@ -307,7 +385,7 @@ export async function executeToolCall( } const errorMsg = `Some agents could not be spawned: ${errors.join('; ')}. Proceeding with valid agents only.` onResponseChunk({ type: 'error', message: errorMsg }) - effectiveInput = { ...input, agents: validAgents } + effectiveInput = { ...effectiveInput, agents: validAgents } } } } @@ -397,7 +475,7 @@ export function parseRawCustomToolCall(params: { rawToolCall: { toolName: string toolCallId: string - input: Record + input: unknown } autoInsertEndStepParam?: boolean }): CustomToolCall | ToolCallError { @@ -416,12 +494,14 @@ export function parseRawCustomToolCall(params: { } } - if (typeof rawToolCall.input === 'string') { + const parsedInput = parseStringifiedToolInput(rawToolCall.input, toolName) + + if (typeof parsedInput === 'string') { return stringInputError(toolName, rawToolCall.toolCallId) } const processedParameters: Record = {} - for (const [param, val] of Object.entries(rawToolCall.input ?? {})) { + for (const [param, val] of Object.entries(parsedInput ?? {})) { processedParameters[param] = val } @@ -450,7 +530,7 @@ export function parseRawCustomToolCall(params: { } } - const input = JSON.parse(JSON.stringify(rawToolCall.input)) + const input = JSON.parse(JSON.stringify(parsedInput)) if (endsAgentStepParam in input) { delete input[endsAgentStepParam] }