mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-27 10:04:58 +00:00
fix(agent-core): record and close tool calls interrupted mid-stream (#1790)
Some checks are pending
CI / build (push) Waiting to run
CI / VSIX package audit (darwin-arm64) (push) Waiting to run
CI / VSIX package audit (all) (push) Waiting to run
CI / typecheck (push) Waiting to run
CI / VSIX package audit (win32-x64) (push) Waiting to run
CI / test (1) (push) Waiting to run
CI / test (2) (push) Waiting to run
CI / test (3) (push) Waiting to run
CI / test (4) (push) Waiting to run
CI / test (5) (push) Waiting to run
CI / test-pi-tui (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / lint (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 / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Desktop release artifact (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
Some checks are pending
CI / build (push) Waiting to run
CI / VSIX package audit (darwin-arm64) (push) Waiting to run
CI / VSIX package audit (all) (push) Waiting to run
CI / typecheck (push) Waiting to run
CI / VSIX package audit (win32-x64) (push) Waiting to run
CI / test (1) (push) Waiting to run
CI / test (2) (push) Waiting to run
CI / test (3) (push) Waiting to run
CI / test (4) (push) Waiting to run
CI / test (5) (push) Waiting to run
CI / test-pi-tui (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / lint (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 / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Desktop release artifact (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
When the provider stream breaks off mid-tool-call (pause_turn, engine
overload, token limit), the step ended with the partial tool call dropped
silently: never executed, never recorded. If the response carried no other
usable content, the persisted assistant message was effectively empty and
every subsequent request — including compaction — was rejected with
"assistant must not be empty" (HTTP 400), wedging the session.
Record each unexecuted call instead (arguments sanitized to {} when the
truncated JSON is unparseable) and immediately close it with a synthetic
interrupted error result. The exchange stays wire-valid, the history stays
truthful, and the model learns the calls never ran so it can re-issue them.
This commit is contained in:
parent
319001ae5c
commit
373abb02f0
4 changed files with 194 additions and 2 deletions
5
.changeset/record-unexecuted-tool-calls.md
Normal file
5
.changeset/record-unexecuted-tool-calls.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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<ExecutableTool, ToolArgsValidator>();
|
||||
|
||||
/**
|
||||
|
|
@ -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<void> {
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<Record<string, unknown>> {
|
|||
};
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue