diff --git a/.changeset/tool-args-schema-validation.md b/.changeset/tool-args-schema-validation.md new file mode 100644 index 000000000..87b2a6074 --- /dev/null +++ b/.changeset/tool-args-schema-validation.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +--- + +Align malformed tool call argument handling with schema validation fallback. diff --git a/packages/agent-core/src/loop/tool-args-parse.ts b/packages/agent-core/src/loop/tool-args-parse.ts new file mode 100644 index 000000000..4b8755c37 --- /dev/null +++ b/packages/agent-core/src/loop/tool-args-parse.ts @@ -0,0 +1,25 @@ +import { errorMessage } from './errors'; + +export type ParseToolArgsResult = { + readonly success: true; + readonly data: unknown; + readonly parseFailed: boolean; + readonly error?: string; +}; + +export function parseToolCallArguments(raw: string | null): ParseToolArgsResult { + if (raw === null || raw.length === 0) { + return { success: true, data: {}, parseFailed: false }; + } + + try { + return { success: true, data: JSON.parse(raw) as unknown, parseFailed: false }; + } catch (error) { + return { + success: true, + data: {}, + parseFailed: true, + error: errorMessage(error), + }; + } +} diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index 7379406ce..801609df9 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -27,6 +27,7 @@ import { PathSecurityError } from '../tools/policies/path-access'; import { isUserCancellation } from '../utils/abort'; import { errorMessage, isAbortError } from './errors'; import type { LoopEventDispatcher, LoopToolCallEvent } from './events'; +import { parseToolCallArguments } from './tool-args-parse'; import type { LLM, LLMChatResponse } from './llm'; import { ToolAccesses } from './tool-access'; import { ToolScheduler, type ToolCallTask } from './tool-scheduler'; @@ -125,7 +126,7 @@ export async function runToolCallBatch( ): Promise { if (response.toolCalls.length === 0) return { stopTurn: false }; const batchStep: ToolCallBatchContext = { ...step, toolCalls: response.toolCalls }; - const calls = response.toolCalls.map((toolCall) => preflightToolCall(step.tools, toolCall)); + const calls = response.toolCalls.map((toolCall) => preflightToolCall(step, toolCall)); const scheduler = new ToolScheduler(); const pendingResults: Array> = []; let stopTurn = false; @@ -173,31 +174,31 @@ export async function runToolCallBatch( * events. Validator compilation may populate the local cache. */ function preflightToolCall( - tools: readonly ExecutableTool[] | undefined, + step: Pick, toolCall: ToolCall, ): PreflightedToolCall { const toolName = toolCall.name; const parsedArgs = parseToolCallArguments(toolCall.arguments); - const args = parsedArgs.success ? parsedArgs.data : {}; - const tool = tools?.find((candidate) => candidate.name === toolName); + const tool = step.tools?.find((candidate) => candidate.name === toolName); if (tool === undefined) { return { kind: 'rejected', toolCall, toolName, - args, + args: parsedArgs.data, output: `Tool "${toolName}" not found`, }; } - if (!parsedArgs.success) { - return { - kind: 'rejected', - toolCall, + + if (parsedArgs.parseFailed) { + step.log?.debug('tool args JSON parse failed', { toolName, - args, - output: `Invalid args for tool "${toolName}": malformed JSON in arguments: ${parsedArgs.error}`, - }; + toolCallId: toolCall.id, + rawLength: toolCall.arguments?.length ?? 0, + error: parsedArgs.error, + }); } + const validationError = validateExecutableToolArgs(tool, parsedArgs.data); if (validationError !== null) { return { @@ -211,21 +212,6 @@ function preflightToolCall( return { kind: 'runnable', toolCall, toolName, tool, args: parsedArgs.data }; } -function parseToolCallArguments( - raw: string | null, -): - | { readonly success: true; readonly data: unknown } - | { readonly success: false; readonly error: string } { - if (raw === null || raw.length === 0) { - return { success: true, data: {} }; - } - try { - return { success: true, data: JSON.parse(raw) as unknown }; - } catch (error) { - return { success: false, error: errorMessage(error) }; - } -} - function validateExecutableToolArgs(tool: ExecutableTool, args: unknown): string | null { let validator = validators.get(tool); if (validator === undefined) { diff --git a/packages/agent-core/test/loop/tool-args-parse.test.ts b/packages/agent-core/test/loop/tool-args-parse.test.ts new file mode 100644 index 000000000..92a90f8b5 --- /dev/null +++ b/packages/agent-core/test/loop/tool-args-parse.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest'; + +import { parseToolCallArguments } from '../../src/loop/tool-args-parse'; + +describe('parseToolCallArguments', () => { + it('treats null or empty arguments as an empty object', () => { + expect(parseToolCallArguments(null)).toEqual({ + success: true, + data: {}, + parseFailed: false, + }); + expect(parseToolCallArguments('')).toEqual({ + success: true, + data: {}, + parseFailed: false, + }); + }); + + it('parses valid JSON', () => { + expect(parseToolCallArguments('{"text":"hi"}')).toEqual({ + success: true, + data: { text: 'hi' }, + parseFailed: false, + }); + }); + + it('falls back to an empty object when JSON is malformed', () => { + expect(parseToolCallArguments('{"text":"hi",}')).toEqual({ + success: true, + data: {}, + parseFailed: true, + error: expect.any(String), + }); + }); + + it('falls back to an empty object for unrecoverable JSON', () => { + const result = parseToolCallArguments('{}{'); + expect(result).toEqual({ + success: true, + data: {}, + parseFailed: true, + error: expect.any(String), + }); + }); +}); diff --git a/packages/agent-core/test/loop/tool-call.e2e.test.ts b/packages/agent-core/test/loop/tool-call.e2e.test.ts index 32dfe34d6..654fabe26 100644 --- a/packages/agent-core/test/loop/tool-call.e2e.test.ts +++ b/packages/agent-core/test/loop/tool-call.e2e.test.ts @@ -170,6 +170,7 @@ describe('runTurn — tool-call behaviour', () => { const tcRow = context.toolCalls(); const trRow = context.toolResults(); expect(tcRow.length).toBe(1); + expect(tcRow[0]?.args).toEqual({ x: 1 }); expect(trRow.length).toBe(1); expect(trRow[0]?.result.isError).toBe(true); }); @@ -191,7 +192,7 @@ describe('runTurn — tool-call behaviour', () => { expect(expectTextOutput(results[0]?.result.output).toLowerCase()).toContain('invalid args'); }); - it('records an error tool.result when LLM-side args parsing already failed', async () => { + it('falls back to schema validation when LLM-side args parsing fails', async () => { const echo = new EchoTool(); const { sink } = await runTurn({ tools: [echo], @@ -201,7 +202,7 @@ describe('runTurn — tool-call behaviour', () => { type: 'function', id: 'tc-1', name: 'echo', - arguments: '{', + arguments: '{}{', }, ]), makeEndTurnResponse('done'), @@ -212,7 +213,38 @@ describe('runTurn — tool-call behaviour', () => { const results = sink.byType('tool.result'); expect(results.length).toBe(1); expect(results[0]?.result.isError).toBe(true); - expect(results[0]?.result.output).toContain('malformed JSON in arguments'); + const output = expectTextOutput(results[0]?.result.output); + expect(output).toContain('Invalid args'); + expect(output).toContain("must have required property 'text'"); + expect(output).not.toContain('malformed JSON in arguments'); + expect(output).not.toContain('Expected arguments schema:'); + }); + + it('does not repair malformed tool args JSON', async () => { + const echo = new EchoTool(); + const { sink } = await runTurn({ + tools: [echo], + responses: [ + makeToolUseResponse([ + { + type: 'function', + id: 'tc-1', + name: 'echo', + arguments: '{"text":"hi",}', + }, + ]), + makeEndTurnResponse('done'), + ], + }); + + expect(echo.calls).toHaveLength(0); + + const results = sink.byType('tool.result'); + expect(results.length).toBe(1); + expect(results[0]?.result.isError).toBe(true); + const output = expectTextOutput(results[0]?.result.output); + expect(output).toContain('Invalid args'); + expect(output).toContain("must have required property 'text'"); }); it('captures tool execution failures as error results', async () => {