diff --git a/.changeset/record-unexecuted-tool-calls.md b/.changeset/record-unexecuted-tool-calls.md new file mode 100644 index 000000000..b23bf49d3 --- /dev/null +++ b/.changeset/record-unexecuted-tool-calls.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix repeated request rejections after an interrupted model response by recording tool calls that never ran and closing them with an interrupted result. diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index 069331b3f..a90fcdbbc 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -46,6 +46,18 @@ const GRACE_TIMEOUT_MS = 2_000; const TOOL_OUTPUT_EMPTY = 'Tool output is empty.'; const TOOL_OUTPUT_NON_TEXT = 'Tool returned non-text content.'; +/** + * Output for a tool call the step never executed: the provider stream broke + * off (paused / overloaded / token limit), so running the call — whose + * arguments may be truncated mid-stream — would be unsafe. The wording tells + * the model the call did not run and invites a clean re-issue instead of + * assumptions about the outcome. + */ +const UNEXECUTED_TOOL_CALL_OUTPUT = + 'This tool call was not executed: the model response ended before tool execution could start ' + + '(the provider stream was interrupted). Do not assume the tool ran — ' + + 're-issue the call if it is still needed.'; + const validators = new WeakMap(); /** @@ -173,6 +185,53 @@ export async function runToolCallBatch( return { stopTurn }; } +/** + * Record tool calls from a response the step will NOT execute: the provider + * stream broke off (paused / overloaded / token limit), so running the calls + * — whose arguments may be truncated mid-stream — would be unsafe. Dropping + * them silently is not an option either: it loses the model's intent and, + * when the response carried no other usable content, persists an assistant + * message strict providers reject as empty. Each call is recorded with + * sanitized arguments (unparseable JSON, e.g. truncated by an interrupted + * stream, becomes `{}`) and immediately closed with a synthetic error result, + * so the exchange stays wire-valid and the model learns the calls never ran. + */ +export async function recordUnexecutedToolCalls( + step: ToolCallStepContext, + response: LLMChatResponse, +): Promise { + for (const toolCall of response.toolCalls) { + const parsedArgs = parseToolCallArguments(toolCall.arguments); + if (parsedArgs.parseFailed) { + step.log?.debug('recording unexecuted tool call with unparseable arguments', { + toolName: toolCall.name, + toolCallId: toolCall.id, + rawLength: toolCall.arguments?.length ?? 0, + error: parsedArgs.error, + }); + } + await step.dispatchEvent({ + type: 'tool.call', + uuid: toolCall.id, + turnId: step.turnId, + step: step.currentStep, + stepUuid: step.stepUuid, + toolCallId: toolCall.id, + name: toolCall.name, + args: parsedArgs.data, + extras: toolCall.extras, + traceId: step.trace.traceId, + }); + await step.dispatchEvent({ + type: 'tool.result', + parentUuid: toolCall.id, + toolCallId: toolCall.id, + result: { output: UNEXECUTED_TOOL_CALL_OUTPUT, isError: true }, + traceId: step.trace.traceId, + }); + } +} + /** * Provider-order validation pass. It does not run hooks, spawn tools, or write * events. Validator compilation may populate the local cache. diff --git a/packages/agent-core/src/loop/turn-step.ts b/packages/agent-core/src/loop/turn-step.ts index 20edaba76..2890cab7e 100644 --- a/packages/agent-core/src/loop/turn-step.ts +++ b/packages/agent-core/src/loop/turn-step.ts @@ -27,7 +27,7 @@ import { type LLMRequestTrace, } from './llm'; import { chatWithRetry } from './retry'; -import { runToolCallBatch, type ToolCallStepContext } from './tool-call'; +import { recordUnexecutedToolCalls, runToolCallBatch, type ToolCallStepContext } from './tool-call'; import type { ExecutableTool, LoopHooks, @@ -403,6 +403,19 @@ export async function executeLoopStep(deps: ExecuteLoopStepDeps): Promise<{ if (effectiveStopReason === 'tool_use') { const toolBatch = await runToolCallBatch(step, response); if (toolBatch.stopTurn) effectiveStopReason = 'end_turn'; + } else if ( + (stopReason === 'paused' || stopReason === 'unknown' || stopReason === 'max_tokens') && + response.toolCalls.length > 0 + ) { + // The provider stream broke off (paused / overloaded / token limit) while + // the response still carries tool calls — possibly cut off mid-arguments. + // Record each call and close it with a synthetic interrupted result: + // dropping them would lose the model's intent and can persist an + // assistant message strict providers reject as empty. Filtered responses + // keep their existing behavior (calls vanish): persisting flagged content + // risks re-triggering the filter on every resend, and a well-formed + // tool_use response stopped by usage recording keeps its skip behavior. + await recordUnexecutedToolCalls(step, response); } // When a tool batch runs, it drains paired `tool.result` events even when 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 368d5b08d..2fe004d95 100644 --- a/packages/agent-core/test/loop/tool-call.e2e.test.ts +++ b/packages/agent-core/test/loop/tool-call.e2e.test.ts @@ -13,12 +13,13 @@ import { describe, expect, it } from 'vitest'; import { ToolAccesses } from '../../src/loop'; import type { Logger } from '../../src/logging'; -import type { ExecutableTool, ExecutableToolResult, LoopHooks, ToolExecution } from '../../src/loop'; +import type { ExecutableTool, ExecutableToolResult, LoopHooks, ToolCall, ToolExecution } from '../../src/loop'; import { PathSecurityError } from '../../src/tools/policies/path-access'; import { makeEndTurnResponse, makeResponse, makeTextParts, + makeThinkingParts, makeToolCall, makeToolUseResponse, } from './fixtures/fake-llm'; @@ -878,3 +879,117 @@ class StopSuccessTool implements ExecutableTool> { }; } } + + +describe('runTurn — unexecuted tool calls (abnormal step end)', () => { + it('records and closes a complete tool call when the step ends paused', async () => { + const echo = new EchoTool(); + const { result, context, sink } = await runTurn({ + tools: [echo], + responses: [ + makeResponse(makeThinkingParts('let me run a tool'), [makeToolCall('echo', { text: 'hi' }, 'tc-1')], 'paused'), + ], + }); + + // The call must not execute, but it is recorded and immediately closed + // with a synthetic interrupted result so the exchange stays wire-valid. + expect(echo.calls.length).toBe(0); + expect(result.stopReason).toBe('paused'); + expect(context.toolCalls().map((e) => [e.toolCallId, e.name, e.args])).toEqual([ + ['tc-1', 'echo', { text: 'hi' }], + ]); + const results = context.toolResults(); + expect(results.map((e) => e.toolCallId)).toEqual(['tc-1']); + expect(results[0]?.result.isError).toBe(true); + expect(expectTextOutput(results[0]?.result.output)).toContain('not executed'); + // Events pair up in the live stream too, and the step seals as paused. + expect(sink.byType('tool.call').map((e) => e.toolCallId)).toEqual(['tc-1']); + expect(sink.byType('tool.result').map((e) => e.toolCallId)).toEqual(['tc-1']); + expect(context.stepEnds()[0]?.finishReason).toBe('paused'); + }); + + it('sanitizes truncated (unparseable) arguments to {} when recording the call', async () => { + // The provider stream was cut mid-arguments: id and name arrived, the JSON + // did not. The recorded call must carry sanitized args — the raw fragment + // would crash the next request's wire conversion. + const partialCall: ToolCall = { + type: 'function', + id: 'tc-partial', + name: 'echo', + arguments: '{"text":"unterminated', + }; + const echo = new EchoTool(); + const { context } = await runTurn({ + tools: [echo], + responses: [ + // The realistic poison shape: an empty signed thinking block plus the + // partial call (what a pause_turn response carries). + makeResponse([{ type: 'think', think: '', encrypted: 'sig' }], [partialCall], 'paused'), + ], + }); + + expect(echo.calls.length).toBe(0); + expect(context.toolCalls().map((e) => [e.toolCallId, e.args])).toEqual([['tc-partial', {}]]); + const results = context.toolResults(); + expect(results.map((e) => e.toolCallId)).toEqual(['tc-partial']); + expect(results[0]?.result.isError).toBe(true); + }); + + it('records and closes tool calls when the stream fails overloaded (other finish)', async () => { + const echo = new EchoTool(); + const { result, context } = await runTurn({ + tools: [echo], + responses: [ + makeResponse(makeTextParts(''), [makeToolCall('echo', { text: 'hi' }, 'tc-ovl')], 'unknown'), + ], + }); + + expect(echo.calls.length).toBe(0); + expect(result.stopReason).toBe('unknown'); + expect(context.toolCalls().map((e) => e.toolCallId)).toEqual(['tc-ovl']); + expect(context.toolResults().map((e) => e.toolCallId)).toEqual(['tc-ovl']); + expect(context.toolResults()[0]?.result.isError).toBe(true); + }); + + it('records and closes a tool call cut off by max_tokens', async () => { + const echo = new EchoTool(); + const { result, context } = await runTurn({ + tools: [echo], + responses: [ + makeResponse(makeTextParts('partial answer'), [makeToolCall('echo', { text: 'hi' }, 'tc-max')], 'max_tokens'), + ], + }); + + expect(echo.calls.length).toBe(0); + expect(result.stopReason).toBe('max_tokens'); + expect(context.toolCalls().map((e) => e.toolCallId)).toEqual(['tc-max']); + expect(context.toolResults().map((e) => e.toolCallId)).toEqual(['tc-max']); + expect(context.toolResults()[0]?.result.isError).toBe(true); + }); + + it('closes every unexecuted call in provider order', async () => { + const echo = new EchoTool(); + const { context } = await runTurn({ + tools: [echo], + responses: [ + makeResponse( + makeThinkingParts('two calls'), + [makeToolCall('echo', { text: 'a' }, 'tc-a'), makeToolCall('echo', { text: 'b' }, 'tc-b')], + 'paused', + ), + ], + }); + + expect(echo.calls.length).toBe(0); + expect(context.toolCalls().map((e) => e.toolCallId)).toEqual(['tc-a', 'tc-b']); + expect(context.toolResults().map((e) => e.toolCallId)).toEqual(['tc-a', 'tc-b']); + }); + + it('does not record synthetic results for a normal end_turn without tool calls', async () => { + const { context } = await runTurn({ + responses: [makeEndTurnResponse('done')], + }); + expect(context.toolCalls()).toEqual([]); + expect(context.toolResults()).toEqual([]); + }); +});