From 3e42ae7607ba03e4e54ff95f24c7e7708536e817 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Tue, 30 Jun 2026 13:28:02 +0800 Subject: [PATCH] fix(agent-core): repair misplaced tool results in projected context Long tool-use sessions could get stuck with repeated 400 errors when a tool result was not adjacent to its tool_use (for example a notification flushed between step.begin and the first tool.call, or an interrupted or nested step delaying the result). Micro compaction exposed this latent misordering by busting the prompt cache and forcing a full revalidation. Repair the adjacency at projection time so every tool_use is immediately followed by its tool_result(s), and default the experimental micro compaction flag off. --- .changeset/fix-tool-use-ordering.md | 5 + .../agent-core/src/agent/context/projector.ts | 53 ++- packages/agent-core/src/flags/registry.ts | 2 +- packages/agent-core/test/agent/basic.test.ts | 2 +- .../test/agent/compaction/micro.test.ts | 4 +- .../agent-core/test/agent/context.test.ts | 88 ++++ .../test/agent/context/projector.test.ts | 394 ++++++++++++++++++ 7 files changed, 543 insertions(+), 5 deletions(-) create mode 100644 .changeset/fix-tool-use-ordering.md create mode 100644 packages/agent-core/test/agent/context/projector.test.ts diff --git a/.changeset/fix-tool-use-ordering.md b/.changeset/fix-tool-use-ordering.md new file mode 100644 index 000000000..c140628c4 --- /dev/null +++ b/.changeset/fix-tool-use-ordering.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix long tool-use conversations getting stuck with repeated request errors. diff --git a/packages/agent-core/src/agent/context/projector.ts b/packages/agent-core/src/agent/context/projector.ts index 02e574c3d..5e2d8e99d 100644 --- a/packages/agent-core/src/agent/context/projector.ts +++ b/packages/agent-core/src/agent/context/projector.ts @@ -4,7 +4,58 @@ import { ErrorCodes, KimiError } from '../../errors'; import type { ContextMessage } from './types'; export function project(history: readonly ContextMessage[]): Message[] { - return mergeAdjacentUserMessages(history); + return repairToolExchangeAdjacency(mergeAdjacentUserMessages(history)); +} + +// Strict providers (Anthropic) require every assistant `tool_use` to be answered +// by a matching `tool_result` in the immediately following message(s). A +// misordered history — where a `tool_result` is not adjacent to its `tool_use`, +// e.g. because a user message (background-task notification, flushed steer) +// landed in between, or because an interrupted / nested step delayed the result +// — is rejected with HTTP 400 ("`tool_use` without `tool_result` immediately +// after"). Micro compaction only exposed this latent misordering by busting the +// prompt cache and forcing a full revalidation. +// +// Repair the adjacency so every assistant `tool_use` is immediately followed by +// its matching `tool_result` message(s). Matching results are moved up from +// wherever they appear later in the history; any intervening messages keep their +// relative order and simply follow the repaired exchange. A tool call with no +// recorded result anywhere later in the history is left untouched — it is still +// in-flight (pending) rather than orphaned, and the trailing-open-exchange trim +// plus the interrupted-result synthesis during replay own those cases. This is +// purely a projection-time fix: the underlying history is left untouched, so +// replay and transcripts keep their original order, while the model always sees +// a well-formed tool exchange. +function repairToolExchangeAdjacency(messages: readonly Message[]): Message[] { + const out: Message[] = []; + const consumed = new Set(); + for (let i = 0; i < messages.length; i++) { + if (consumed.has(i)) continue; + const message = messages[i]!; + if (message.role !== 'assistant' || message.toolCalls.length === 0) { + out.push(message); + continue; + } + + out.push(message); + const pending = new Set(message.toolCalls.map((toolCall) => toolCall.id)); + for (let j = i + 1; j < messages.length && pending.size > 0; j++) { + if (consumed.has(j)) continue; + const next = messages[j]!; + const toolCallId = next.toolCallId; + if (next.role === 'tool' && toolCallId !== undefined && pending.has(toolCallId)) { + out.push(next); + consumed.add(j); + pending.delete(toolCallId); + } + } + // If a tool call has no recorded result anywhere later in the history, it is + // still in-flight (pending) rather than orphaned, so leave it untouched — the + // trailing-open-exchange trim and the interrupted-result synthesis during + // replay own those cases, and synthesizing here would wrongly close a call + // that is simply still running. + } + return out; } function mergeAdjacentUserMessages(history: readonly ContextMessage[]): Message[] { diff --git a/packages/agent-core/src/flags/registry.ts b/packages/agent-core/src/flags/registry.ts index 16f88d592..fcce75ece 100644 --- a/packages/agent-core/src/flags/registry.ts +++ b/packages/agent-core/src/flags/registry.ts @@ -17,7 +17,7 @@ export const FLAG_DEFINITIONS = [ title: 'Micro compaction', description: 'Trim older large tool results from context while keeping recent conversation intact.', env: 'KIMI_CODE_EXPERIMENTAL_MICRO_COMPACTION', - default: true, + default: false, surface: 'core', }, ] as const satisfies readonly FlagDefinitionInput[]; diff --git a/packages/agent-core/test/agent/basic.test.ts b/packages/agent-core/test/agent/basic.test.ts index 1c9bfec61..2ecaf615e 100644 --- a/packages/agent-core/test/agent/basic.test.ts +++ b/packages/agent-core/test/agent/basic.test.ts @@ -9,7 +9,7 @@ it('creates an independent agent with a scoped experimental flag resolver', () = experimentalFlags: new FlagResolver({}, FLAG_DEFINITIONS), }); - expect(ctx.agent.experimentalFlags.enabled('micro_compaction')).toBe(true); + expect(ctx.agent.experimentalFlags.enabled('micro_compaction')).toBe(false); }); it('runs a text-only agent turn from prompt to completion', async () => { diff --git a/packages/agent-core/test/agent/compaction/micro.test.ts b/packages/agent-core/test/agent/compaction/micro.test.ts index edc931aaa..7fe14e880 100644 --- a/packages/agent-core/test/agent/compaction/micro.test.ts +++ b/packages/agent-core/test/agent/compaction/micro.test.ts @@ -35,8 +35,8 @@ describe('MicroCompaction', () => { vi.stubEnv(MICRO_COMPACTION_FLAG_ENV, '1'); }); - it('defaults the micro_compaction flag on', () => { - expect(new FlagResolver({}, FLAG_DEFINITIONS).enabled('micro_compaction')).toBe(true); + it('defaults the micro_compaction flag off', () => { + expect(new FlagResolver({}, FLAG_DEFINITIONS).enabled('micro_compaction')).toBe(false); }); it('truncates old tool results after cache miss', () => { diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 580bda69c..f17b64ad1 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -563,6 +563,94 @@ describe('Agent context', () => { await ctx.expectResumeMatches(); }); + // Regression: a user message injected after `step.begin` but before the first + // `tool.call` (e.g. a background-task notification flushed mid-step) lands + // between the assistant `tool_use` and its `tool_result` in history, which + // strict providers (Anthropic) reject with HTTP 400. The projector must repair + // the adjacency so the `tool_result` immediately follows the `tool_use`. Micro + // compaction exposed this latent misordering by busting the prompt cache. + it('repairs a tool_use/tool_result adjacency broken by an injected user message', async () => { + const ctx = testAgent(); + ctx.configure(); + const stepUuid = 'mid-step-notify-step'; + + ctx.agent.context.appendUserMessage([{ type: 'text', text: 'drive the tank' }]); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { type: 'step.begin', uuid: stepUuid, turnId: '0', step: 1 }, + }); + + // Notification arrives in the gap between step.begin and tool.call, when no + // tool result is yet pending, so it is pushed directly into history. + ctx.agent.context.appendUserMessage([{ type: 'text', text: 'bg done' }], { + kind: 'background_task', + taskId: 'task-1', + status: 'completed', + notificationId: 'task:task-1:completed', + }); + + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.call', + uuid: 'call_drive', + turnId: '0', + step: 1, + stepUuid, + toolCallId: 'call_drive', + name: 'Drive', + args: {}, + }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'step.end', + uuid: stepUuid, + turnId: '0', + step: 1, + finishReason: 'tool_use', + }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.result', + parentUuid: 'call_drive', + toolCallId: 'call_drive', + result: { output: 'drove forward' }, + }, + }); + + // History preserves the original (misordered) sequence: the notification sits + // between the assistant tool_use and its tool_result. + expect(ctx.agent.context.history.map((message) => message.role)).toEqual([ + 'user', + 'assistant', + 'user', + 'tool', + ]); + + // Projection repairs the adjacency: the tool_result immediately follows the + // assistant tool_use, and the sandwiched notification is moved after it. + const projected = ctx.agent.context.messages; + expect(projected.map((message) => message.role)).toEqual(['user', 'assistant', 'tool', 'user']); + const assistantIndex = projected.findIndex( + (message) => message.role === 'assistant' && message.toolCalls.length > 0, + ); + expect(projected[assistantIndex]?.toolCalls.map((toolCall) => toolCall.id)).toEqual([ + 'call_drive', + ]); + expect(projected[assistantIndex + 1]).toMatchObject({ + role: 'tool', + toolCallId: 'call_drive', + }); + expect(projected[assistantIndex + 2]?.content).toEqual([ + { type: 'text', text: 'bg done' }, + ]); + await ctx.expectResumeMatches(); + }); + it('preserves deferred reminders when compaction keeps a pending tool exchange', async () => { const ctx = testAgent(); ctx.configure(); diff --git a/packages/agent-core/test/agent/context/projector.test.ts b/packages/agent-core/test/agent/context/projector.test.ts new file mode 100644 index 000000000..838520e7e --- /dev/null +++ b/packages/agent-core/test/agent/context/projector.test.ts @@ -0,0 +1,394 @@ +import type { ContentPart, Message, ToolCall } from '@moonshot-ai/kosong'; +import { describe, expect, it } from 'vitest'; + +import { project } from '../../../src/agent/context/projector'; +import type { ContextMessage } from '../../../src/agent/context/types'; + +// --------------------------------------------------------------------------- +// Invariant under test +// --------------------------------------------------------------------------- +// +// Strict providers (Anthropic) reject a request with HTTP 400 when an assistant +// `tool_use` is not immediately followed by its matching `tool_result`. The +// projector must therefore guarantee that, for every assistant tool call whose +// result exists anywhere in the projected history, that result sits in the +// consecutive tool messages immediately following the assistant message. +// +// A tool call with no recorded result anywhere is considered still in-flight +// (pending) and is intentionally left untouched — it is not an orphan. + +interface MisplacedToolUse { + readonly assistantIndex: number; + readonly toolCallId: string; +} + +/** + * Return tool calls whose result exists somewhere in `messages` but is not + * adjacent to the assistant `tool_use`. An empty result means the invariant + * holds and the history is safe to send to a strict provider. + */ +function findMisplacedToolUses(messages: readonly Message[]): MisplacedToolUse[] { + // Index every recorded tool result by its toolCallId. + const resultIndexById = new Map(); + messages.forEach((message, index) => { + if (message.role === 'tool' && message.toolCallId !== undefined) { + resultIndexById.set(message.toolCallId, index); + } + }); + + const violations: MisplacedToolUse[] = []; + for (let i = 0; i < messages.length; i++) { + const message = messages[i]!; + if (message.role !== 'assistant' || message.toolCalls.length === 0) continue; + + // Collect the toolCallIds answered in the consecutive tool messages that + // immediately follow this assistant message. + const adjacentResultIds = new Set(); + let j = i + 1; + while (j < messages.length && messages[j]!.role === 'tool') { + const id = messages[j]!.toolCallId; + if (id !== undefined) adjacentResultIds.add(id); + j++; + } + + for (const toolCall of message.toolCalls) { + // Only flag tool calls whose result was actually recorded; a missing + // result means the call is still in-flight, not misplaced. + if (!resultIndexById.has(toolCall.id)) continue; + if (!adjacentResultIds.has(toolCall.id)) { + violations.push({ assistantIndex: i, toolCallId: toolCall.id }); + } + } + } + return violations; +} + +// --------------------------------------------------------------------------- +// Builders +// --------------------------------------------------------------------------- + +function textPart(text: string): ContentPart { + return { type: 'text', text }; +} + +function user(text: string): ContextMessage { + return { role: 'user', content: [textPart(text)], toolCalls: [] }; +} + +function notification(text: string): ContextMessage { + return { + role: 'user', + content: [textPart(text)], + toolCalls: [], + origin: { + kind: 'background_task', + taskId: 'task', + status: 'completed', + notificationId: 'task:task:completed', + }, + }; +} + +function assistant(toolCallIds: readonly string[], text = ''): ContextMessage { + return { + role: 'assistant', + content: text.length > 0 ? [textPart(text)] : [], + toolCalls: toolCallIds.map( + (id): ToolCall => ({ type: 'function', id, name: 'Run', arguments: '{}' }), + ), + }; +} + +function emptyAssistant(): ContextMessage { + return { role: 'assistant', content: [], toolCalls: [] }; +} + +function tool(toolCallId: string, text = 'ok'): ContextMessage { + return { role: 'tool', content: [textPart(text)], toolCalls: [], toolCallId }; +} + +function compactionSummary(text = 'summary'): ContextMessage { + return { + role: 'assistant', + content: [textPart(text)], + toolCalls: [], + origin: { kind: 'compaction_summary' }, + }; +} + +// --------------------------------------------------------------------------- +// Targeted regression tests +// --------------------------------------------------------------------------- + +describe('project tool_use/tool_result adjacency', () => { + it('leaves an already well-formed history unchanged (idempotent)', () => { + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + tool('a'), + user('u2'), + assistant(['b', 'c']), + tool('b'), + tool('c'), + user('u3'), + ]; + const projected = project(history); + expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([ + ['user', undefined], + ['assistant', undefined], + ['tool', 'a'], + ['user', undefined], + ['assistant', undefined], + ['tool', 'b'], + ['tool', 'c'], + ['user', undefined], + ]); + expect(findMisplacedToolUses(projected)).toEqual([]); + }); + + it('moves a user message sandwiched between tool_use and tool_result to after the result', () => { + const history: ContextMessage[] = [user('u1'), assistant(['a']), notification('ping'), tool('a')]; + const projected = project(history); + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant', 'tool', 'user']); + expect(projected[1]?.toolCalls.map((tc) => tc.id)).toEqual(['a']); + expect(projected[2]).toMatchObject({ role: 'tool', toolCallId: 'a' }); + expect(findMisplacedToolUses(projected)).toEqual([]); + }); + + it('pulls a distant tool result back up across intervening exchanges', () => { + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + user('middle'), + assistant(['b']), + tool('b'), + user('later'), + tool('a'), + ]; + const projected = project(history); + expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([ + ['user', undefined], + ['assistant', undefined], + ['tool', 'a'], + ['user', undefined], + ['assistant', undefined], + ['tool', 'b'], + ['user', undefined], + ]); + expect(findMisplacedToolUses(projected)).toEqual([]); + }); + + it('reorders parallel tool results that arrive out of order', () => { + const history: ContextMessage[] = [ + user('u1'), + assistant(['a', 'b', 'c']), + tool('c'), + tool('a'), + tool('b'), + ]; + const projected = project(history); + // All three results must be adjacent to the assistant, regardless of order. + expect(projected.map((m) => m.role)).toEqual(['user', 'assistant', 'tool', 'tool', 'tool']); + const resultIds = projected.slice(2).map((m) => m.toolCallId); + expect(resultIds).toEqual(expect.arrayContaining(['a', 'b', 'c'])); + expect(findMisplacedToolUses(projected)).toEqual([]); + }); + + it('repairs multiple misplaced exchanges in a single history', () => { + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + user('sandwich-a'), + assistant(['b']), + tool('b'), + user('sandwich-b'), + tool('a'), + ]; + const projected = project(history); + expect(findMisplacedToolUses(projected)).toEqual([]); + // a's result must immediately follow a's assistant. + const aIndex = projected.findIndex((m) => m.toolCalls.some((tc) => tc.id === 'a')); + expect(projected[aIndex + 1]).toMatchObject({ role: 'tool', toolCallId: 'a' }); + const bIndex = projected.findIndex((m) => m.toolCalls.some((tc) => tc.id === 'b')); + expect(projected[bIndex + 1]).toMatchObject({ role: 'tool', toolCallId: 'b' }); + }); + + it('leaves a pending (in-flight) tool call without a recorded result untouched', () => { + const history: ContextMessage[] = [user('u1'), assistant(['a', 'b']), tool('a')]; + // b has no recorded result — it is still pending, not orphaned. + const projected = project(history); + expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([ + ['user', undefined], + ['assistant', undefined], + ['tool', 'a'], + ]); + // No new (synthetic) tool result for b was introduced. + expect(projected.some((m) => m.toolCallId === 'b')).toBe(false); + }); + + it('does not move a tool result whose toolCallId matches no assistant tool_use', () => { + const history: ContextMessage[] = [ + user('u1'), + assistant(['a']), + tool('a'), + tool('orphan-result'), + user('u2'), + ]; + const projected = project(history); + // The stray result stays where it was; nothing references it. + expect(projected.map((m) => [m.role, m.toolCallId])).toEqual([ + ['user', undefined], + ['assistant', undefined], + ['tool', 'a'], + ['tool', 'orphan-result'], + ['user', undefined], + ]); + }); + + it('does not crash when a tool result appears before its tool_use', () => { + const history: ContextMessage[] = [tool('a'), user('u1'), assistant(['a'])]; + // Forward scan cannot find the result (it is behind the assistant), so the + // exchange is left as-is rather than throwing. + expect(() => project(history)).not.toThrow(); + }); + + it('preserves compaction summaries and empty assistants while repairing', () => { + const history: ContextMessage[] = [ + compactionSummary(), + user('u1'), + assistant(['a']), + notification('ping'), + emptyAssistant(), + tool('a'), + ]; + const projected = project(history); + expect(findMisplacedToolUses(projected)).toEqual([]); + const aIndex = projected.findIndex((m) => m.toolCalls.some((tc) => tc.id === 'a')); + expect(projected[aIndex + 1]).toMatchObject({ role: 'tool', toolCallId: 'a' }); + }); +}); + +// --------------------------------------------------------------------------- +// Property-based fuzz test +// --------------------------------------------------------------------------- +// +// Generate a large number of histories with randomized, worst-case misordering +// (sandwiched user messages, distant results, parallel calls, pending calls, +// empty assistants, compaction summaries) and assert the projector ALWAYS +// produces a history that satisfies the adjacency invariant. This is the guard +// that catches regressions which would otherwise strand the user with HTTP 400. + +describe('project adjacency invariant (fuzz)', () => { + it('holds for thousands of randomized histories', () => { + const rng = mulberry32(0x5eed_c0de); + const iterations = 4000; + for (let n = 0; n < iterations; n++) { + const history = generateHistory(rng, n); + const projected = project(history); + const violations = findMisplacedToolUses(projected); + expect( + violations, + `adjacency invariant violated at iteration ${n}\n` + + `history: ${JSON.stringify(history.map(label))}\n` + + `projected: ${JSON.stringify(projected.map(label))}`, + ).toEqual([]); + } + }); +}); + +// --------------------------------------------------------------------------- +// Fuzz generator +// --------------------------------------------------------------------------- + +type Rng = () => number; + +function label(message: Message): string { + const id = message.toolCallId ?? message.toolCalls.map((toolCall) => toolCall.id).join(','); + return `${message.role}:${id}`; +} + +// mulberry32 requires unsigned 32-bit wrapping arithmetic (`>>> 0`), which +// `Math.trunc` does not provide, so the prefer-math-trunc lint is a false +// positive here. +/* eslint-disable unicorn/prefer-math-trunc */ +function mulberry32(seed: number): Rng { + let state = seed >>> 0; + return () => { + state = (state + 0x6d2b79f5) >>> 0; + let t = state; + t = Math.imul(t ^ (t >>> 15), t | 1); + t ^= t + Math.imul(t ^ (t >>> 7), t | 61); + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} +/* eslint-enable unicorn/prefer-math-trunc */ + +function pick(rng: Rng, items: readonly T[]): T { + return items[Math.floor(rng() * items.length)]!; +} + +function generateHistory(rng: Rng, seed: number): ContextMessage[] { + const messages: ContextMessage[] = []; + let nextId = 1; + const blockCount = 2 + Math.floor(rng() * 8); + + for (let b = 0; b < blockCount; b++) { + const kind = pick(rng, ['user', 'exchange', 'notification', 'empty', 'compaction'] as const); + switch (kind) { + case 'user': + messages.push(user(`u-${seed}-${b}`)); + break; + case 'notification': + messages.push(notification(`n-${seed}-${b}`)); + break; + case 'empty': + messages.push(emptyAssistant()); + break; + case 'compaction': + messages.push(compactionSummary()); + break; + case 'exchange': { + const arity = 1 + Math.floor(rng() * 3); + const ids: string[] = []; + for (let k = 0; k < arity; k++) { + ids.push(`c${nextId++}`); + } + messages.push(assistant(ids)); + // Decide which results are recorded (some may be pending). + const recorded = ids.filter(() => rng() > 0.25); + // Randomize result order to simulate parallel calls completing out of order. + shuffle(recorded, rng); + // Randomly inject a sandwiched user/notification before the results. + if (rng() > 0.5) { + messages.push(pick(rng, [user(`sandwich-${seed}-${b}`), notification(`sandwich-n-${seed}-${b}`)])); + } + // Randomly delay one recorded result past a following exchange. + let delayed: ContextMessage | undefined; + if (recorded.length > 0 && rng() > 0.6) { + delayed = tool(recorded.pop()!); + } + for (const id of recorded) { + messages.push(tool(id)); + } + // Possibly emit a full extra exchange before the delayed result lands. + if (delayed !== undefined) { + if (rng() > 0.4) { + const laterIds = [`c${nextId++}`]; + messages.push(assistant(laterIds)); + messages.push(tool(laterIds[0]!)); + } + messages.push(delayed); + } + break; + } + } + } + return messages; +} + +function shuffle(items: T[], rng: Rng): void { + for (let i = items.length - 1; i > 0; i--) { + const j = Math.floor(rng() * (i + 1)); + [items[i], items[j]] = [items[j]!, items[i]!]; + } +}