From fd273db5f10844f2a5d8bbfe5c0ba655d9b579e4 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 29 Jun 2026 21:58:26 +0800 Subject: [PATCH 1/4] feat(agent-core): repair malformed tool args JSON Attempt jsonrepair when tool call arguments fail JSON.parse, then continue schema validation. Return malformed JSON errors with a concise expected schema hint so the model can retry with corrected arguments. --- packages/agent-core/package.json | 1 + .../agent-core/src/loop/tool-args-repair.ts | 99 +++++++++++++++++++ packages/agent-core/src/loop/tool-call.ts | 48 ++++----- .../test/loop/tool-args-repair.test.ts | 69 +++++++++++++ .../test/loop/tool-call.e2e.test.ts | 33 ++++++- pnpm-lock.yaml | 11 ++- 6 files changed, 235 insertions(+), 26 deletions(-) create mode 100644 packages/agent-core/src/loop/tool-args-repair.ts create mode 100644 packages/agent-core/test/loop/tool-args-repair.test.ts diff --git a/packages/agent-core/package.json b/packages/agent-core/package.json index 8d3871282..e3dc9f579 100644 --- a/packages/agent-core/package.json +++ b/packages/agent-core/package.json @@ -70,6 +70,7 @@ "chokidar": "^4.0.3", "ignore": "^5.3.2", "js-yaml": "^4.1.1", + "jsonrepair": "^3.14.1", "linkedom": "^0.18.12", "node-pty": "^1.1.0", "nunjucks": "^3.2.4", diff --git a/packages/agent-core/src/loop/tool-args-repair.ts b/packages/agent-core/src/loop/tool-args-repair.ts new file mode 100644 index 000000000..d875415fe --- /dev/null +++ b/packages/agent-core/src/loop/tool-args-repair.ts @@ -0,0 +1,99 @@ +import { jsonrepair } from 'jsonrepair'; + +import { errorMessage } from './errors'; + +const MAX_SCHEMA_HINT_CHARS = 1200; + +export type ParseToolArgsResult = + | { + readonly success: true; + readonly data: unknown; + readonly repaired: boolean; + readonly originalError?: string; + } + | { + readonly success: false; + readonly error: string; + }; + +export function parseOrRepairToolCallArguments(raw: string | null): ParseToolArgsResult { + if (raw === null || raw.length === 0) { + return { success: true, data: {}, repaired: false }; + } + + try { + return { success: true, data: JSON.parse(raw) as unknown, repaired: false }; + } catch (error) { + const originalError = errorMessage(error); + + try { + const repairedText = jsonrepair(raw); + const data = JSON.parse(repairedText) as unknown; + return { success: true, data, repaired: true, originalError }; + } catch { + return { success: false, error: originalError }; + } + } +} + +export function buildToolArgsSchemaHint(parameters: Record): string { + const propertiesValue = parameters['properties']; + if (!isRecord(propertiesValue)) { + return ''; + } + + const propertyEntries = Object.entries(propertiesValue); + if (propertyEntries.length === 0) { + return ''; + } + + const required = readRequiredNames(parameters['required']); + const requiredNames = [...required]; + const requiredLine = + requiredNames.length > 0 ? `- required: ${requiredNames.join(', ')}` : '- required: (none)'; + + const propertiesLine = propertyEntries + .map(([name, schema]) => { + const optionalMarker = required.has(name) ? '' : '?'; + return `${name} (${schemaTypeLabel(schema)}${optionalMarker})`; + }) + .join(', '); + + const hint = `Expected arguments schema:\n${requiredLine}\n- properties: ${propertiesLine}`; + return hint.length > MAX_SCHEMA_HINT_CHARS ? `${hint.slice(0, MAX_SCHEMA_HINT_CHARS)}…` : hint; +} + +function readRequiredNames(value: unknown): ReadonlySet { + if (!Array.isArray(value)) { + return new Set(); + } + return new Set(value.filter((item): item is string => typeof item === 'string')); +} + +function schemaTypeLabel(schema: unknown): string { + if (!isRecord(schema)) { + return 'unknown'; + } + + const type = schema['type']; + if (typeof type === 'string') { + return type; + } + if (Array.isArray(type)) { + return type.filter((item): item is string => typeof item === 'string').join(' | ') || 'unknown'; + } + if (isRecord(schema['properties'])) { + return 'object'; + } + if (schema['enum'] !== undefined) { + return 'enum'; + } + if (schema['anyOf'] !== undefined || schema['oneOf'] !== undefined || schema['allOf'] !== undefined) { + return 'object'; + } + return 'unknown'; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index 7379406ce..fb40750c1 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -27,6 +27,10 @@ import { PathSecurityError } from '../tools/policies/path-access'; import { isUserCancellation } from '../utils/abort'; import { errorMessage, isAbortError } from './errors'; import type { LoopEventDispatcher, LoopToolCallEvent } from './events'; +import { + buildToolArgsSchemaHint, + parseOrRepairToolCallArguments, +} from './tool-args-repair'; import type { LLM, LLMChatResponse } from './llm'; import { ToolAccesses } from './tool-access'; import { ToolScheduler, type ToolCallTask } from './tool-scheduler'; @@ -125,7 +129,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 +177,44 @@ 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: {}, output: `Tool "${toolName}" not found`, }; } + + const parsedArgs = parseOrRepairToolCallArguments(toolCall.arguments); + if (!parsedArgs.success) { + const schemaHint = buildToolArgsSchemaHint(tool.parameters); + const schemaSuffix = schemaHint.length > 0 ? `\n\n${schemaHint}` : ''; return { kind: 'rejected', toolCall, toolName, - args, - output: `Invalid args for tool "${toolName}": malformed JSON in arguments: ${parsedArgs.error}`, + args: {}, + output: `Invalid args for tool "${toolName}": malformed JSON in arguments: ${parsedArgs.error}${schemaSuffix}`, }; } + + if (parsedArgs.repaired) { + step.log?.debug('tool args JSON repaired', { + toolName, + toolCallId: toolCall.id, + rawLength: toolCall.arguments?.length ?? 0, + originalError: parsedArgs.originalError, + }); + } + const validationError = validateExecutableToolArgs(tool, parsedArgs.data); if (validationError !== null) { return { @@ -211,21 +228,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-repair.test.ts b/packages/agent-core/test/loop/tool-args-repair.test.ts new file mode 100644 index 000000000..ff87ade98 --- /dev/null +++ b/packages/agent-core/test/loop/tool-args-repair.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildToolArgsSchemaHint, + parseOrRepairToolCallArguments, +} from '../../src/loop/tool-args-repair'; + +describe('parseOrRepairToolCallArguments', () => { + it('treats null or empty arguments as an empty object', () => { + expect(parseOrRepairToolCallArguments(null)).toEqual({ + success: true, + data: {}, + repaired: false, + }); + expect(parseOrRepairToolCallArguments('')).toEqual({ + success: true, + data: {}, + repaired: false, + }); + }); + + it('parses valid JSON without repair', () => { + expect(parseOrRepairToolCallArguments('{"text":"hi"}')).toEqual({ + success: true, + data: { text: 'hi' }, + repaired: false, + }); + }); + + it('repairs trailing commas after JSON.parse fails', () => { + expect(parseOrRepairToolCallArguments('{"text":"hi",}')).toEqual({ + success: true, + data: { text: 'hi' }, + repaired: true, + originalError: expect.any(String), + }); + }); + + it('returns the original parse error when repair cannot help', () => { + const result = parseOrRepairToolCallArguments('{}{'); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.length).toBeGreaterThan(0); + } + }); +}); + +describe('buildToolArgsSchemaHint', () => { + it('summarizes required and optional properties', () => { + const hint = buildToolArgsSchemaHint({ + type: 'object', + properties: { + text: { type: 'string' }, + limit: { type: 'number' }, + }, + required: ['text'], + additionalProperties: false, + }); + + expect(hint).toContain('Expected arguments schema:'); + expect(hint).toContain('- required: text'); + expect(hint).toContain('text (string)'); + expect(hint).toContain('limit (number?)'); + }); + + it('returns an empty string when there are no object properties', () => { + expect(buildToolArgsSchemaHint({ type: 'object', additionalProperties: true })).toBe(''); + }); +}); 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..53104f388 100644 --- a/packages/agent-core/test/loop/tool-call.e2e.test.ts +++ b/packages/agent-core/test/loop/tool-call.e2e.test.ts @@ -201,7 +201,7 @@ describe('runTurn — tool-call behaviour', () => { type: 'function', id: 'tc-1', name: 'echo', - arguments: '{', + arguments: '{}{', }, ]), makeEndTurnResponse('done'), @@ -212,7 +212,36 @@ 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('malformed JSON in arguments'); + expect(output).toContain('Expected arguments schema:'); + expect(output).toContain('text (string)'); + }); + + it('repairs malformed tool args JSON before rejecting the tool call', 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(1); + expect(echo.calls[0]?.args).toEqual({ text: 'hi' }); + + const results = sink.byType('tool.result'); + expect(results.length).toBe(1); + expect(results[0]?.result.isError).toBeUndefined(); + expect(results[0]?.result.output).toBe('hi'); }); it('captures tool execution failures as error results', async () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bd0208433..4732d5f09 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -347,6 +347,9 @@ importers: js-yaml: specifier: ^4.1.1 version: 4.1.1 + jsonrepair: + specifier: ^3.14.1 + version: 3.14.1 linkedom: specifier: ^0.18.12 version: 0.18.12 @@ -4362,6 +4365,10 @@ packages: jsonfile@6.2.1: resolution: {integrity: sha512-zwOTdL3rFQ/lRdBnntKVOX6k5cKJwEc1HdilT71BWEu7J41gXIB2MRp+vxduPSwZJPWBxEzv4yH1wYLJGUHX4Q==} + jsonrepair@3.14.1: + resolution: {integrity: sha512-NpGgMhmzG/fajkBEFlS9jZvMSGDvc2xN/9wNCHZ+Nx32GZfLRELU6UE6dQkebvrQUct9S+7bvnpX29NB36Qbdw==} + hasBin: true + jwa@2.0.1: resolution: {integrity: sha512-hRF04fqJIP8Abbkq5NKGN0Bbr3JxlQ+qhZufXVr0DvujKy93ZCbXZMHDL4EOtodSbCWxOqR8MS1tXA5hwqCXDg==} @@ -8295,7 +8302,7 @@ snapshots: obug: 2.1.1 std-env: 4.0.0 tinyrainbow: 3.1.0 - vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@6.4.2(@types/node@22.19.17)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)(yaml@2.8.3)) + vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@8.0.8(@types/node@22.19.17)(esbuild@0.27.7)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3)) '@vitest/expect@4.1.4': dependencies: @@ -10074,6 +10081,8 @@ snapshots: optionalDependencies: graceful-fs: 4.2.11 + jsonrepair@3.14.1: {} + jwa@2.0.1: dependencies: buffer-equal-constant-time: 1.0.1 From 800ff549a84cdb31c4e68e17672b7b78b1ebc8f0 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 29 Jun 2026 22:08:12 +0800 Subject: [PATCH 2/4] chore(nix): update pnpm deps hash Update the fixed-output pnpmDeps hash after adding jsonrepair so the Nix build can fetch dependencies. --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 037283001..3cb6dc355 100644 --- a/flake.nix +++ b/flake.nix @@ -150,7 +150,7 @@ inherit (finalAttrs) pname version src pnpmWorkspaces; inherit pnpm; fetcherVersion = 3; - hash = "sha256-O9xDt/5bCakst2mKTyki3oyUph1g+CuH/BIqA/4fgYE="; + hash = "sha256-qSJmNAzgPQj5RkZ2VLxE6IhCEhZyl5oMLBFtJt9NbIY="; }; nativeBuildInputs = [ From c6ab18fb24e5ea8d9c703b026628c233eaff5f34 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 29 Jun 2026 22:44:26 +0800 Subject: [PATCH 3/4] refactor(agent-core): drop tool args JSON repair Stop repairing malformed tool call arguments. Fall back to an empty object on JSON parse failure and let schema validation produce the retry error, while preserving valid arguments for unknown tools in the transcript. --- flake.nix | 2 +- packages/agent-core/package.json | 1 - .../agent-core/src/loop/tool-args-parse.ts | 25 +++++ .../agent-core/src/loop/tool-args-repair.ts | 99 ------------------- packages/agent-core/src/loop/tool-call.ts | 28 ++---- .../test/loop/tool-args-parse.test.ts | 45 +++++++++ .../test/loop/tool-args-repair.test.ts | 69 ------------- .../test/loop/tool-call.e2e.test.ts | 21 ++-- pnpm-lock.yaml | 11 +-- 9 files changed, 90 insertions(+), 211 deletions(-) create mode 100644 packages/agent-core/src/loop/tool-args-parse.ts delete mode 100644 packages/agent-core/src/loop/tool-args-repair.ts create mode 100644 packages/agent-core/test/loop/tool-args-parse.test.ts delete mode 100644 packages/agent-core/test/loop/tool-args-repair.test.ts diff --git a/flake.nix b/flake.nix index 3cb6dc355..8583c6d8f 100644 --- a/flake.nix +++ b/flake.nix @@ -150,7 +150,7 @@ inherit (finalAttrs) pname version src pnpmWorkspaces; inherit pnpm; fetcherVersion = 3; - hash = "sha256-qSJmNAzgPQj5RkZ2VLxE6IhCEhZyl5oMLBFtJt9NbIY="; + hash = "sha256-w/mEQrb5gNn4S5jZ95vO9uy4u/JB3wFbUfIZDcWqTXU="; }; nativeBuildInputs = [ diff --git a/packages/agent-core/package.json b/packages/agent-core/package.json index 653bc88d7..2b8d30ec0 100644 --- a/packages/agent-core/package.json +++ b/packages/agent-core/package.json @@ -70,7 +70,6 @@ "chokidar": "^4.0.3", "ignore": "^5.3.2", "js-yaml": "^4.1.1", - "jsonrepair": "^3.14.1", "linkedom": "^0.18.12", "node-pty": "^1.1.0", "nunjucks": "^3.2.4", 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-args-repair.ts b/packages/agent-core/src/loop/tool-args-repair.ts deleted file mode 100644 index d875415fe..000000000 --- a/packages/agent-core/src/loop/tool-args-repair.ts +++ /dev/null @@ -1,99 +0,0 @@ -import { jsonrepair } from 'jsonrepair'; - -import { errorMessage } from './errors'; - -const MAX_SCHEMA_HINT_CHARS = 1200; - -export type ParseToolArgsResult = - | { - readonly success: true; - readonly data: unknown; - readonly repaired: boolean; - readonly originalError?: string; - } - | { - readonly success: false; - readonly error: string; - }; - -export function parseOrRepairToolCallArguments(raw: string | null): ParseToolArgsResult { - if (raw === null || raw.length === 0) { - return { success: true, data: {}, repaired: false }; - } - - try { - return { success: true, data: JSON.parse(raw) as unknown, repaired: false }; - } catch (error) { - const originalError = errorMessage(error); - - try { - const repairedText = jsonrepair(raw); - const data = JSON.parse(repairedText) as unknown; - return { success: true, data, repaired: true, originalError }; - } catch { - return { success: false, error: originalError }; - } - } -} - -export function buildToolArgsSchemaHint(parameters: Record): string { - const propertiesValue = parameters['properties']; - if (!isRecord(propertiesValue)) { - return ''; - } - - const propertyEntries = Object.entries(propertiesValue); - if (propertyEntries.length === 0) { - return ''; - } - - const required = readRequiredNames(parameters['required']); - const requiredNames = [...required]; - const requiredLine = - requiredNames.length > 0 ? `- required: ${requiredNames.join(', ')}` : '- required: (none)'; - - const propertiesLine = propertyEntries - .map(([name, schema]) => { - const optionalMarker = required.has(name) ? '' : '?'; - return `${name} (${schemaTypeLabel(schema)}${optionalMarker})`; - }) - .join(', '); - - const hint = `Expected arguments schema:\n${requiredLine}\n- properties: ${propertiesLine}`; - return hint.length > MAX_SCHEMA_HINT_CHARS ? `${hint.slice(0, MAX_SCHEMA_HINT_CHARS)}…` : hint; -} - -function readRequiredNames(value: unknown): ReadonlySet { - if (!Array.isArray(value)) { - return new Set(); - } - return new Set(value.filter((item): item is string => typeof item === 'string')); -} - -function schemaTypeLabel(schema: unknown): string { - if (!isRecord(schema)) { - return 'unknown'; - } - - const type = schema['type']; - if (typeof type === 'string') { - return type; - } - if (Array.isArray(type)) { - return type.filter((item): item is string => typeof item === 'string').join(' | ') || 'unknown'; - } - if (isRecord(schema['properties'])) { - return 'object'; - } - if (schema['enum'] !== undefined) { - return 'enum'; - } - if (schema['anyOf'] !== undefined || schema['oneOf'] !== undefined || schema['allOf'] !== undefined) { - return 'object'; - } - return 'unknown'; -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index fb40750c1..801609df9 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -27,10 +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 { - buildToolArgsSchemaHint, - parseOrRepairToolCallArguments, -} from './tool-args-repair'; +import { parseToolCallArguments } from './tool-args-parse'; import type { LLM, LLMChatResponse } from './llm'; import { ToolAccesses } from './tool-access'; import { ToolScheduler, type ToolCallTask } from './tool-scheduler'; @@ -181,37 +178,24 @@ function preflightToolCall( toolCall: ToolCall, ): PreflightedToolCall { const toolName = toolCall.name; + const parsedArgs = parseToolCallArguments(toolCall.arguments); 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`, }; } - const parsedArgs = parseOrRepairToolCallArguments(toolCall.arguments); - - if (!parsedArgs.success) { - const schemaHint = buildToolArgsSchemaHint(tool.parameters); - const schemaSuffix = schemaHint.length > 0 ? `\n\n${schemaHint}` : ''; - return { - kind: 'rejected', - toolCall, - toolName, - args: {}, - output: `Invalid args for tool "${toolName}": malformed JSON in arguments: ${parsedArgs.error}${schemaSuffix}`, - }; - } - - if (parsedArgs.repaired) { - step.log?.debug('tool args JSON repaired', { + if (parsedArgs.parseFailed) { + step.log?.debug('tool args JSON parse failed', { toolName, toolCallId: toolCall.id, rawLength: toolCall.arguments?.length ?? 0, - originalError: parsedArgs.originalError, + error: parsedArgs.error, }); } 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-args-repair.test.ts b/packages/agent-core/test/loop/tool-args-repair.test.ts deleted file mode 100644 index ff87ade98..000000000 --- a/packages/agent-core/test/loop/tool-args-repair.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -import { - buildToolArgsSchemaHint, - parseOrRepairToolCallArguments, -} from '../../src/loop/tool-args-repair'; - -describe('parseOrRepairToolCallArguments', () => { - it('treats null or empty arguments as an empty object', () => { - expect(parseOrRepairToolCallArguments(null)).toEqual({ - success: true, - data: {}, - repaired: false, - }); - expect(parseOrRepairToolCallArguments('')).toEqual({ - success: true, - data: {}, - repaired: false, - }); - }); - - it('parses valid JSON without repair', () => { - expect(parseOrRepairToolCallArguments('{"text":"hi"}')).toEqual({ - success: true, - data: { text: 'hi' }, - repaired: false, - }); - }); - - it('repairs trailing commas after JSON.parse fails', () => { - expect(parseOrRepairToolCallArguments('{"text":"hi",}')).toEqual({ - success: true, - data: { text: 'hi' }, - repaired: true, - originalError: expect.any(String), - }); - }); - - it('returns the original parse error when repair cannot help', () => { - const result = parseOrRepairToolCallArguments('{}{'); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.length).toBeGreaterThan(0); - } - }); -}); - -describe('buildToolArgsSchemaHint', () => { - it('summarizes required and optional properties', () => { - const hint = buildToolArgsSchemaHint({ - type: 'object', - properties: { - text: { type: 'string' }, - limit: { type: 'number' }, - }, - required: ['text'], - additionalProperties: false, - }); - - expect(hint).toContain('Expected arguments schema:'); - expect(hint).toContain('- required: text'); - expect(hint).toContain('text (string)'); - expect(hint).toContain('limit (number?)'); - }); - - it('returns an empty string when there are no object properties', () => { - expect(buildToolArgsSchemaHint({ type: 'object', additionalProperties: true })).toBe(''); - }); -}); 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 53104f388..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], @@ -213,12 +214,13 @@ describe('runTurn — tool-call behaviour', () => { expect(results.length).toBe(1); expect(results[0]?.result.isError).toBe(true); const output = expectTextOutput(results[0]?.result.output); - expect(output).toContain('malformed JSON in arguments'); - expect(output).toContain('Expected arguments schema:'); - expect(output).toContain('text (string)'); + 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('repairs malformed tool args JSON before rejecting the tool call', async () => { + it('does not repair malformed tool args JSON', async () => { const echo = new EchoTool(); const { sink } = await runTurn({ tools: [echo], @@ -235,13 +237,14 @@ describe('runTurn — tool-call behaviour', () => { ], }); - expect(echo.calls).toHaveLength(1); - expect(echo.calls[0]?.args).toEqual({ text: 'hi' }); + expect(echo.calls).toHaveLength(0); const results = sink.byType('tool.result'); expect(results.length).toBe(1); - expect(results[0]?.result.isError).toBeUndefined(); - expect(results[0]?.result.output).toBe('hi'); + 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 () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9fe079af3..ddb67e8cc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -347,9 +347,6 @@ importers: js-yaml: specifier: ^4.1.1 version: 4.1.1 - jsonrepair: - specifier: ^3.14.1 - version: 3.14.1 linkedom: specifier: ^0.18.12 version: 0.18.12 @@ -4365,10 +4362,6 @@ packages: jsonfile@6.2.1: resolution: {integrity: sha512-zwOTdL3rFQ/lRdBnntKVOX6k5cKJwEc1HdilT71BWEu7J41gXIB2MRp+vxduPSwZJPWBxEzv4yH1wYLJGUHX4Q==} - jsonrepair@3.14.1: - resolution: {integrity: sha512-NpGgMhmzG/fajkBEFlS9jZvMSGDvc2xN/9wNCHZ+Nx32GZfLRELU6UE6dQkebvrQUct9S+7bvnpX29NB36Qbdw==} - hasBin: true - jwa@2.0.1: resolution: {integrity: sha512-hRF04fqJIP8Abbkq5NKGN0Bbr3JxlQ+qhZufXVr0DvujKy93ZCbXZMHDL4EOtodSbCWxOqR8MS1tXA5hwqCXDg==} @@ -8306,7 +8299,7 @@ snapshots: obug: 2.1.1 std-env: 4.0.0 tinyrainbow: 3.1.0 - vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@8.0.8(@types/node@22.19.17)(esbuild@0.27.7)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3)) + vitest: 4.1.4(@types/node@22.19.17)(@vitest/coverage-v8@4.1.4)(jsdom@25.0.1)(vite@6.4.2(@types/node@22.19.17)(jiti@2.6.1)(lightningcss@1.32.0)(tsx@4.21.0)(yaml@2.8.3)) '@vitest/expect@4.1.4': dependencies: @@ -10085,8 +10078,6 @@ snapshots: optionalDependencies: graceful-fs: 4.2.11 - jsonrepair@3.14.1: {} - jwa@2.0.1: dependencies: buffer-equal-constant-time: 1.0.1 From 3037783b5eb02a00e2bbe780470e72fa49ee75fd Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 29 Jun 2026 23:13:16 +0800 Subject: [PATCH 4/4] chore: add changeset for tool args validation --- .changeset/tool-args-schema-validation.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/tool-args-schema-validation.md 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.