mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-10 01:39:25 +00:00
refactor(agent-core): align malformed tool args with schema validation (#1209)
Some checks are pending
CI / test-windows (push) Waiting to run
CI / lint (push) Waiting to run
CI / build (push) Waiting to run
CI / test (push) Waiting to run
CI / typecheck (push) Waiting to run
Nix Build / Check flake.nix workspace sync (push) Waiting to run
Nix Build / nix build .#kimi-code (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
Some checks are pending
CI / test-windows (push) Waiting to run
CI / lint (push) Waiting to run
CI / build (push) Waiting to run
CI / test (push) Waiting to run
CI / typecheck (push) Waiting to run
Nix Build / Check flake.nix workspace sync (push) Waiting to run
Nix Build / nix build .#kimi-code (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
* 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. * chore(nix): update pnpm deps hash Update the fixed-output pnpmDeps hash after adding jsonrepair so the Nix build can fetch dependencies. * 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. * chore: add changeset for tool args validation
This commit is contained in:
parent
53e79e4bae
commit
063538744f
5 changed files with 124 additions and 30 deletions
6
.changeset/tool-args-schema-validation.md
Normal file
6
.changeset/tool-args-schema-validation.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
"@moonshot-ai/agent-core": patch
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Align malformed tool call argument handling with schema validation fallback.
|
||||
25
packages/agent-core/src/loop/tool-args-parse.ts
Normal file
25
packages/agent-core/src/loop/tool-args-parse.ts
Normal file
|
|
@ -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),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
@ -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<ToolCallBatchResult> {
|
||||
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<PendingToolResult>();
|
||||
const pendingResults: Array<Promise<PendingToolResult>> = [];
|
||||
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<ToolCallStepContext, 'tools' | 'log'>,
|
||||
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) {
|
||||
|
|
|
|||
45
packages/agent-core/test/loop/tool-args-parse.test.ts
Normal file
45
packages/agent-core/test/loop/tool-args-parse.test.ts
Normal file
|
|
@ -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),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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 () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue