mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-09 17:29:12 +00:00
fix(agent-core): harden tool_use/tool_result exchange integrity (#1340)
Some checks are pending
CI / lint (push) Waiting to run
CI / test (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / build (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 / 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 / lint (push) Waiting to run
CI / test (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / build (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 / 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
* feat(agent-core): guide the model away from repeating denied or failed tool calls - system.md: add a diagnose-before-retrying paragraph next to the existing permission-denial guidance, covering failed tool calls - permission: when the user rejects an approval on the main agent, tell the model not to re-attempt the exact same call (sub agents already had an equivalent hint) * fix(agent-core): close abandoned tool exchanges and dedupe duplicate tool_use ids A turn that dies between a recorded tool.call and its paired tool.result (e.g. a transcript write failure mid-batch) used to leave pendingToolResultIds open forever: every later message was stranded in deferredMessages and user input was silently swallowed. - runOneTurn now defensively closes any dangling tool calls when a turn ends (completed, cancelled, or failed), synthesizing an error result that names the cause, with a warn log and a tool_exchange_abandoned telemetry event - the projector drops assistant tool calls whose id already appeared earlier (first occurrence wins): a duplicate id is wire-invalid on strict providers and not repairable by the strict resend; reported via the existing projection-repair log and telemetry - resume-side closePendingToolResults now logs what it closes (warn for a mid-history gap, info for the routine trailing interruption) * chore: add changesets for tool exchange fixes * fix(agent-core): scope duplicate tool_use id dedup to the strict resend Unconditional dedup regressed providers that emit per-response counter ids (e.g. call_0 in every step) and accept their own duplicates: later tool exchanges silently vanished from the projected history, and a duplicate call's own recorded result was left dangling. - the dedupe pass is now opt-in via dedupeDuplicateToolCalls and enabled only in strictMessages, so the normal projection keeps the history the provider produced - the pass also drops every tool result after the first for an id, so no dangling tool message survives; when the kept call has no result of its own, the surviving one is reattached by the adjacency repair - kosong now classifies the Anthropic "tool_use ids must be unique" 400 as a recoverable request-structure error so it triggers the strict resend
This commit is contained in:
parent
9091627257
commit
e2fe62a5ef
13 changed files with 427 additions and 23 deletions
5
.changeset/close-abandoned-tool-exchanges.md
Normal file
5
.changeset/close-abandoned-tool-exchanges.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Fix sessions silently dropping later user messages after a turn was interrupted between a tool call and its result.
|
||||
5
.changeset/dedupe-duplicate-tool-call-ids.md
Normal file
5
.changeset/dedupe-duplicate-tool-call-ids.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Fix requests being rejected by strict providers when the model emits duplicate tool call ids.
|
||||
|
|
@ -397,6 +397,8 @@ export class ContextMemory {
|
|||
let reordered = 0;
|
||||
let synthesized = 0;
|
||||
let droppedOrphan = 0;
|
||||
let duplicateCallsDropped = 0;
|
||||
let duplicateResultsDropped = 0;
|
||||
let leadingDropped = 0;
|
||||
let assistantsMerged = 0;
|
||||
let whitespaceDropped = 0;
|
||||
|
|
@ -404,6 +406,8 @@ export class ContextMemory {
|
|||
if (anomaly.kind === 'tool_result_reordered') reordered += 1;
|
||||
else if (anomaly.kind === 'tool_result_synthesized') synthesized += 1;
|
||||
else if (anomaly.kind === 'orphan_tool_result_dropped') droppedOrphan += 1;
|
||||
else if (anomaly.kind === 'duplicate_tool_call_dropped') duplicateCallsDropped += 1;
|
||||
else if (anomaly.kind === 'duplicate_tool_result_dropped') duplicateResultsDropped += 1;
|
||||
else if (anomaly.kind === 'leading_non_user_dropped') leadingDropped += 1;
|
||||
else if (anomaly.kind === 'consecutive_assistants_merged') assistantsMerged += 1;
|
||||
else whitespaceDropped += 1;
|
||||
|
|
@ -417,6 +421,8 @@ export class ContextMemory {
|
|||
reordered,
|
||||
synthesized,
|
||||
droppedOrphan,
|
||||
duplicateCallsDropped,
|
||||
duplicateResultsDropped,
|
||||
leadingDropped,
|
||||
assistantsMerged,
|
||||
whitespaceDropped,
|
||||
|
|
@ -426,6 +432,8 @@ export class ContextMemory {
|
|||
reordered,
|
||||
synthesized,
|
||||
dropped_orphan: droppedOrphan,
|
||||
duplicate_calls_dropped: duplicateCallsDropped,
|
||||
duplicate_results_dropped: duplicateResultsDropped,
|
||||
leading_dropped: leadingDropped,
|
||||
assistants_merged: assistantsMerged,
|
||||
whitespace_dropped: whitespaceDropped,
|
||||
|
|
@ -443,15 +451,17 @@ export class ContextMemory {
|
|||
}
|
||||
|
||||
// Last-resort projection for the post-400 strict resend: close every open tool
|
||||
// call (including a trailing in-flight one), drop stray tool results, drop a
|
||||
// leading non-user message, and merge consecutive assistant turns, so the
|
||||
// request is wire-compliant for strict providers no matter how the history was
|
||||
// mangled. Only used when the provider has already rejected the normal
|
||||
// projection — see the adjacency fallback in `turn-step`.
|
||||
// call (including a trailing in-flight one), drop stray tool results, dedupe
|
||||
// duplicate tool call ids (with their extra results), drop a leading non-user
|
||||
// message, and merge consecutive assistant turns, so the request is
|
||||
// wire-compliant for strict providers no matter how the history was mangled.
|
||||
// Only used when the provider has already rejected the normal projection —
|
||||
// see the adjacency fallback in `turn-step`.
|
||||
get strictMessages(): Message[] {
|
||||
return this.project(this.history, {
|
||||
synthesizeMissing: true,
|
||||
dropOrphanResults: true,
|
||||
dedupeDuplicateToolCalls: true,
|
||||
dropLeadingNonUser: true,
|
||||
mergeConsecutiveAssistants: true,
|
||||
});
|
||||
|
|
@ -464,7 +474,15 @@ export class ContextMemory {
|
|||
|
||||
finishResume(): void {
|
||||
this.openSteps.clear();
|
||||
this.closePendingToolResults();
|
||||
const closed = this.closePendingToolResults();
|
||||
if (closed.length > 0) {
|
||||
// Routine end-of-resume close of a genuinely interrupted trailing call
|
||||
// (e.g. the process died mid-tool), logged for traceability.
|
||||
this.agent.log.info('closed interrupted tool calls at end of resume', {
|
||||
closed: closed.length,
|
||||
toolCallIds: closed.slice(0, 5),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Synthesize interrupted tool results for any still-open tool calls, closing
|
||||
|
|
@ -473,9 +491,11 @@ export class ContextMemory {
|
|||
// exactly where it occurred — otherwise it would keep `hasOpenToolExchange`
|
||||
// true and strand every later message in `deferredMessages`, so only the
|
||||
// trailing exchange ends up aligned. `finishResume` runs the same routine once
|
||||
// more to close a genuine trailing interruption at end of resume.
|
||||
private closePendingToolResults(): void {
|
||||
if (this.pendingToolResultIds.size === 0) return;
|
||||
// more to close a genuine trailing interruption at end of resume, and
|
||||
// `closeAbandonedToolExchange` reuses it (with a live-turn message) as the
|
||||
// turn-end teardown. Returns the ids it closed; callers own the logging.
|
||||
private closePendingToolResults(output: string = TOOL_INTERRUPTED_ON_RESUME_OUTPUT): string[] {
|
||||
if (this.pendingToolResultIds.size === 0) return [];
|
||||
const interruptedToolCallIds = [...this.pendingToolResultIds];
|
||||
for (const toolCallId of interruptedToolCallIds) {
|
||||
this.appendLoopEvent({
|
||||
|
|
@ -483,11 +503,25 @@ export class ContextMemory {
|
|||
parentUuid: toolCallId,
|
||||
toolCallId,
|
||||
result: {
|
||||
output: TOOL_INTERRUPTED_ON_RESUME_OUTPUT,
|
||||
output,
|
||||
isError: true,
|
||||
},
|
||||
});
|
||||
}
|
||||
return interruptedToolCallIds;
|
||||
}
|
||||
|
||||
/**
|
||||
* Defensive teardown for a live turn that ended — normally, cancelled, or
|
||||
* failed — while recorded tool calls were still awaiting results (e.g. the
|
||||
* batch's result dispatch died after a `tool.call` was already recorded).
|
||||
* Synthesizes an error result for each dangling call so the exchange closes:
|
||||
* left open, it would keep `hasOpenToolExchange` true and strand every later
|
||||
* message in `deferredMessages`, silently swallowing user input. No-op when
|
||||
* the exchange is already closed. Returns the number of calls it closed.
|
||||
*/
|
||||
closeAbandonedToolExchange(output: string): number {
|
||||
return this.closePendingToolResults(output).length;
|
||||
}
|
||||
|
||||
appendLoopEvent(event: LoopRecordedEvent): void {
|
||||
|
|
@ -501,7 +535,16 @@ export class ContextMemory {
|
|||
// earlier step were interrupted (the invariant guarantees this never
|
||||
// happens live, so this is a no-op outside replay). Close them in place
|
||||
// before opening the new step so mid-history gaps stay aligned.
|
||||
this.closePendingToolResults();
|
||||
const closed = this.closePendingToolResults();
|
||||
if (closed.length > 0) {
|
||||
// A mid-history gap means results were lost before this boundary —
|
||||
// a genuine defect worth investigating, unlike the expected trailing
|
||||
// interruption `finishResume` closes.
|
||||
this.agent.log.warn('closed unresolved tool calls at a step boundary', {
|
||||
closed: closed.length,
|
||||
toolCallIds: closed.slice(0, 5),
|
||||
});
|
||||
}
|
||||
const message: ContextMessage = {
|
||||
role: 'assistant',
|
||||
content: [],
|
||||
|
|
|
|||
|
|
@ -44,6 +44,18 @@ export interface ProjectOptions {
|
|||
* consecutive assistant turns do not arise in well-formed transcripts.
|
||||
*/
|
||||
readonly mergeConsecutiveAssistants?: boolean;
|
||||
/**
|
||||
* When `true`, drop assistant tool calls whose id already appeared earlier
|
||||
* (first occurrence wins; a message left with no content and no calls is
|
||||
* dropped), and drop every tool result after the first for a given id so the
|
||||
* kept call keeps exactly one answer. Duplicate ids are wire-invalid on
|
||||
* strict providers ("`tool_use` ids must be unique") and no other pass can
|
||||
* repair them. Strict-resend only: a provider that accepted the duplicates
|
||||
* when it produced them (e.g. per-response counter ids like `call_0`) must
|
||||
* keep seeing the history it generated — deduping the normal path would
|
||||
* silently erase its later tool exchanges.
|
||||
*/
|
||||
readonly dedupeDuplicateToolCalls?: boolean;
|
||||
/**
|
||||
* Optional sink invoked for every repair the projector applies to keep the
|
||||
* outgoing wire valid: a displaced result moved back next to its call, a
|
||||
|
|
@ -73,6 +85,10 @@ export type ProjectionAnomaly =
|
|||
| { readonly kind: 'tool_result_synthesized'; readonly toolCallId: string; readonly trailing: boolean }
|
||||
/** A result with no matching call anywhere was dropped (wire exits only). */
|
||||
| { readonly kind: 'orphan_tool_result_dropped'; readonly toolCallId: string }
|
||||
/** A tool call whose id already appeared earlier was dropped (strict-resend only). */
|
||||
| { readonly kind: 'duplicate_tool_call_dropped'; readonly toolCallId: string }
|
||||
/** A second result for an already-answered id was dropped (strict-resend only). */
|
||||
| { readonly kind: 'duplicate_tool_result_dropped'; readonly toolCallId: string }
|
||||
/** A leading non-user message was dropped so the first turn is user (strict). */
|
||||
| { readonly kind: 'leading_non_user_dropped'; readonly role: string }
|
||||
/** Two adjacent assistant turns were merged into one (strict). */
|
||||
|
|
@ -81,10 +97,11 @@ export type ProjectionAnomaly =
|
|||
| { readonly kind: 'whitespace_text_dropped'; readonly role: string };
|
||||
|
||||
export function project(history: readonly ContextMessage[], options?: ProjectOptions): Message[] {
|
||||
let result = repairToolExchangeAdjacency(
|
||||
mergeAdjacentUserMessages(history, options?.onAnomaly),
|
||||
options,
|
||||
);
|
||||
let result = mergeAdjacentUserMessages(history, options?.onAnomaly);
|
||||
if (options?.dedupeDuplicateToolCalls === true) {
|
||||
result = dedupeDuplicateToolCalls(result, options.onAnomaly);
|
||||
}
|
||||
result = repairToolExchangeAdjacency(result, options);
|
||||
if (options?.mergeConsecutiveAssistants === true) {
|
||||
result = mergeConsecutiveAssistantMessages(result, options.onAnomaly);
|
||||
}
|
||||
|
|
@ -189,6 +206,53 @@ function repairToolExchangeAdjacency(
|
|||
return out;
|
||||
}
|
||||
|
||||
// Strict providers reject a request whose assistant messages carry two
|
||||
// `tool_use` blocks with the same id ("tool_use ids must be unique"). Keep the
|
||||
// first occurrence of each call id, drop the rest, and drop an assistant
|
||||
// message entirely when duplicates were all it carried. Every result after the
|
||||
// first for a given id is dropped with its call, so no dangling tool message
|
||||
// survives the dedupe; when the kept call has no result of its own, the later
|
||||
// duplicate's surviving result is reattached by the adjacency repair. Runs
|
||||
// before the adjacency repair so pending-result matching never sees the
|
||||
// duplicate. Strict-resend only (see `ProjectOptions.dedupeDuplicateToolCalls`):
|
||||
// the normal projection keeps duplicates verbatim for the lax provider that
|
||||
// produced and accepts them.
|
||||
function dedupeDuplicateToolCalls(
|
||||
messages: readonly Message[],
|
||||
onAnomaly?: (anomaly: ProjectionAnomaly) => void,
|
||||
): Message[] {
|
||||
const seenToolCallIds = new Set<string>();
|
||||
const seenToolResultIds = new Set<string>();
|
||||
const out: Message[] = [];
|
||||
for (const message of messages) {
|
||||
if (message.role === 'assistant' && message.toolCalls.length > 0) {
|
||||
const kept = message.toolCalls.filter((toolCall) => {
|
||||
if (seenToolCallIds.has(toolCall.id)) {
|
||||
onAnomaly?.({ kind: 'duplicate_tool_call_dropped', toolCallId: toolCall.id });
|
||||
return false;
|
||||
}
|
||||
seenToolCallIds.add(toolCall.id);
|
||||
return true;
|
||||
});
|
||||
if (kept.length === message.toolCalls.length) {
|
||||
out.push(message);
|
||||
} else if (kept.length > 0 || message.content.length > 0) {
|
||||
out.push({ ...message, toolCalls: kept });
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (message.role === 'tool' && message.toolCallId !== undefined) {
|
||||
if (seenToolResultIds.has(message.toolCallId)) {
|
||||
onAnomaly?.({ kind: 'duplicate_tool_result_dropped', toolCallId: message.toolCallId });
|
||||
continue;
|
||||
}
|
||||
seenToolResultIds.add(message.toolCallId);
|
||||
}
|
||||
out.push(message);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// Remove any `tool_result` whose `toolCallId` matches no assistant `tool_use`
|
||||
// anywhere in the projected messages. Strict providers reject such a stray
|
||||
// result, and it is useless to the model regardless (it has no record of the
|
||||
|
|
|
|||
|
|
@ -302,6 +302,9 @@ export class PermissionManager {
|
|||
if (this.agent.type === 'sub') {
|
||||
return `${prefix}${suffix} Try a different approach — don't retry the same call, don't attempt to bypass the restriction.`;
|
||||
}
|
||||
if (result.decision === 'rejected') {
|
||||
return `${prefix}${suffix} Do not re-attempt the exact same call — think about why it was rejected, then adjust your approach or ask the user what they would prefer.`;
|
||||
}
|
||||
return `${prefix}${suffix}`;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -552,6 +552,11 @@ export class TurnFlow {
|
|||
}
|
||||
}
|
||||
}
|
||||
// A live turn must never end with recorded tool calls still awaiting
|
||||
// results; if one does (a dispatch failure mid-batch broke the "every
|
||||
// recorded call gets a result" invariant), close the exchange now so the
|
||||
// context state machine cannot strand later messages in deferredMessages.
|
||||
this.closeAbandonedToolExchange(ended);
|
||||
// Emit the terminal turn.ended and (for a standalone turn) release the active
|
||||
// turn in the SAME synchronous frame, so the session is observably idle the
|
||||
// instant turn.ended fires. A goal drive keeps the active turn across its
|
||||
|
|
@ -840,6 +845,29 @@ export class TurnFlow {
|
|||
}
|
||||
}
|
||||
|
||||
// Guarded so this repair can never turn a finished turn into a crash: a
|
||||
// failure to close (e.g. record persistence still broken) is logged and the
|
||||
// projection-level safeguards remain the last line of defense.
|
||||
private closeAbandonedToolExchange(ended: TurnEndedEvent): void {
|
||||
try {
|
||||
const closed = this.agent.context.closeAbandonedToolExchange(
|
||||
abandonedToolResultOutput(ended),
|
||||
);
|
||||
if (closed === 0) return;
|
||||
this.agent.log.warn('closed abandoned tool exchange at turn end', {
|
||||
turnId: ended.turnId,
|
||||
reason: ended.reason,
|
||||
closed,
|
||||
});
|
||||
this.agent.telemetry.track('tool_exchange_abandoned', {
|
||||
reason: ended.reason,
|
||||
closed,
|
||||
});
|
||||
} catch (error) {
|
||||
this.agent.log.warn('failed to close abandoned tool exchange', { error });
|
||||
}
|
||||
}
|
||||
|
||||
private buildDispatchEvent(turnId: number) {
|
||||
return createLoopEventDispatcher({
|
||||
appendTranscriptRecord: async (event: LoopRecordedEvent) => {
|
||||
|
|
@ -1265,3 +1293,17 @@ function telemetryToolErrorType(result: ToolTelemetryResult): string {
|
|||
function toolResultText(result: ToolTelemetryResult): string {
|
||||
return toolOutputText(result.output);
|
||||
}
|
||||
|
||||
// Output for a tool call abandoned by its turn (see closeAbandonedToolExchange):
|
||||
// name the cause so the model treats the gap as an interruption to reason about,
|
||||
// not a tool outcome. Mirrors the phrasing of the resume-time synthesis in
|
||||
// `ContextMemory`.
|
||||
function abandonedToolResultOutput(ended: TurnEndedEvent): string {
|
||||
const cause =
|
||||
ended.reason === 'cancelled'
|
||||
? 'the turn was cancelled'
|
||||
: ended.reason === 'failed'
|
||||
? `the turn failed${ended.error !== undefined ? ` (${ended.error.message})` : ''}`
|
||||
: 'the turn ended';
|
||||
return `Tool call did not complete: ${cause} before its result was recorded. Do not assume the tool completed successfully.`;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,8 @@ The results of the tool calls will be returned to you in a tool message. You mus
|
|||
|
||||
Tool calls run behind the user's permission settings. A rejected or denied call means the user or their policy declined that specific action — adjust your approach, or ask what they would prefer instead. Do not retry the same call unchanged, and do not route around the denial by doing the same thing through a different tool or shell command.
|
||||
|
||||
When a tool call fails, diagnose why before acting again: read the error, check your assumptions, and make a focused adjustment. Do not retry the identical call blindly, but do not abandon a viable approach after a single failure either — if you are still stuck after investigating, ask the user.
|
||||
|
||||
The system may insert information wrapped in `<system>` tags within user or tool messages. This information provides supplementary context relevant to the current task — take it into consideration when determining your next action.
|
||||
|
||||
Tool results and user messages may also include `<system-reminder>` tags. Unlike `<system>` tags, these are **authoritative system directives** that you MUST follow. They bear no direct relation to the specific tool results or user messages in which they appear. Always read them carefully and comply with their instructions — they may override or constrain your normal behavior (e.g., restricting you to read-only actions during plan mode).
|
||||
|
|
|
|||
|
|
@ -1254,3 +1254,63 @@ function textOf(message: Message): string {
|
|||
.map((part) => part.text)
|
||||
.join('');
|
||||
}
|
||||
|
||||
describe('strictMessages duplicate tool call ids', () => {
|
||||
it('keeps duplicates on the normal projection but dedupes them in the strict one', () => {
|
||||
const ctx = testAgent();
|
||||
ctx.configure();
|
||||
ctx.agent.context.appendUserMessage([{ type: 'text', text: 'run the tool twice' }]);
|
||||
// A provider with per-response counter ids reuses `call_dup` in two steps;
|
||||
// both exchanges record their own result.
|
||||
for (const step of [1, 2]) {
|
||||
const stepUuid = `dup-step-${String(step)}`;
|
||||
ctx.dispatch({
|
||||
type: 'context.append_loop_event',
|
||||
event: { type: 'step.begin', uuid: stepUuid, turnId: '', step },
|
||||
});
|
||||
ctx.dispatch({
|
||||
type: 'context.append_loop_event',
|
||||
event: {
|
||||
type: 'tool.call',
|
||||
uuid: `dup-call-${String(step)}`,
|
||||
turnId: '',
|
||||
step,
|
||||
stepUuid,
|
||||
toolCallId: 'call_dup',
|
||||
name: 'Run',
|
||||
args: { attempt: step },
|
||||
},
|
||||
});
|
||||
ctx.dispatch({
|
||||
type: 'context.append_loop_event',
|
||||
event: { type: 'step.end', uuid: stepUuid, turnId: '', step },
|
||||
});
|
||||
ctx.dispatch({
|
||||
type: 'context.append_loop_event',
|
||||
event: {
|
||||
type: 'tool.result',
|
||||
parentUuid: `dup-call-${String(step)}`,
|
||||
toolCallId: 'call_dup',
|
||||
result: { output: `result ${String(step)}` },
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
// Normal projection: the lax provider that produced the duplicate ids
|
||||
// accepts them, so nothing is dropped.
|
||||
const normal = ctx.agent.context.messages;
|
||||
expect(
|
||||
normal.filter((message) => message.role === 'assistant').flatMap((m) => m.toolCalls),
|
||||
).toHaveLength(2);
|
||||
expect(normal.filter((message) => message.role === 'tool')).toHaveLength(2);
|
||||
|
||||
// Strict resend projection: one call, one result.
|
||||
const strict = ctx.agent.context.strictMessages;
|
||||
expect(
|
||||
strict.filter((message) => message.role === 'assistant').flatMap((m) => m.toolCalls),
|
||||
).toHaveLength(1);
|
||||
const strictResults = strict.filter((message) => message.role === 'tool');
|
||||
expect(strictResults).toHaveLength(1);
|
||||
expect(textOf(strictResults[0]!)).toBe('result 1');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -543,6 +543,107 @@ describe('project strict-provider sanitizers', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('project duplicate tool_use ids', () => {
|
||||
// A provider that (buggily, or via per-response counter ids like `call_0`)
|
||||
// emits two tool_use blocks with the same id produces a request strict
|
||||
// providers reject ("`tool_use` ids must be unique"). The normal projection
|
||||
// must leave the duplicates untouched — the lax provider that produced them
|
||||
// accepts them, and deduping would silently erase its later tool exchanges.
|
||||
// Only the strict resend (after a provider already rejected the request)
|
||||
// dedupes, dropping later duplicate calls together with their recorded
|
||||
// results so no dangling tool message survives.
|
||||
const duplicateAcrossSteps: ContextMessage[] = [
|
||||
user('u1'),
|
||||
assistant(['call_a'], 'first'),
|
||||
tool('call_a', 'first result'),
|
||||
assistant(['call_a', 'call_b'], 'second'),
|
||||
tool('call_a', 'second result'),
|
||||
tool('call_b'),
|
||||
user('u2'),
|
||||
];
|
||||
|
||||
it('leaves duplicate ids and their results untouched on the normal path', () => {
|
||||
const projected = project(duplicateAcrossSteps, { dropOrphanResults: true });
|
||||
const assistants = projected.filter(
|
||||
(message) => message.role === 'assistant' && message.toolCalls.length > 0,
|
||||
);
|
||||
expect(assistants[0]?.toolCalls.map((toolCall) => toolCall.id)).toEqual(['call_a']);
|
||||
expect(assistants[1]?.toolCalls.map((toolCall) => toolCall.id)).toEqual(['call_a', 'call_b']);
|
||||
expect(projected.filter((message) => message.role === 'tool')).toHaveLength(3);
|
||||
});
|
||||
|
||||
it('under the strict flag, drops later duplicate calls together with their results', () => {
|
||||
const anomalies: ProjectionAnomaly[] = [];
|
||||
const projected = project(duplicateAcrossSteps, {
|
||||
dedupeDuplicateToolCalls: true,
|
||||
dropOrphanResults: true,
|
||||
onAnomaly: (anomaly) => anomalies.push(anomaly),
|
||||
});
|
||||
const assistants = projected.filter(
|
||||
(message) => message.role === 'assistant' && message.toolCalls.length > 0,
|
||||
);
|
||||
expect(assistants[0]?.toolCalls.map((toolCall) => toolCall.id)).toEqual(['call_a']);
|
||||
expect(assistants[1]?.toolCalls.map((toolCall) => toolCall.id)).toEqual(['call_b']);
|
||||
const toolMessages = projected.filter((message) => message.role === 'tool');
|
||||
expect(toolMessages.map((message) => message.toolCallId)).toEqual(['call_a', 'call_b']);
|
||||
expect(textOf(toolMessages[0])).toBe('first result');
|
||||
expect(anomalies).toContainEqual({
|
||||
kind: 'duplicate_tool_call_dropped',
|
||||
toolCallId: 'call_a',
|
||||
});
|
||||
expect(anomalies).toContainEqual({
|
||||
kind: 'duplicate_tool_result_dropped',
|
||||
toolCallId: 'call_a',
|
||||
});
|
||||
expect(everyToolUseImmediatelyAnswered(projected)).toBe(true);
|
||||
});
|
||||
|
||||
it('under the strict flag, drops a duplicate call id within one assistant message', () => {
|
||||
const projected = project(
|
||||
[
|
||||
user('u1'),
|
||||
assistant(['call_dup', 'call_dup'], 'calling twice'),
|
||||
tool('call_dup', 'result'),
|
||||
user('u2'),
|
||||
],
|
||||
{ dedupeDuplicateToolCalls: true, dropOrphanResults: true },
|
||||
);
|
||||
const assistantMessage = projected.find((message) => message.role === 'assistant');
|
||||
expect(assistantMessage?.toolCalls.map((toolCall) => toolCall.id)).toEqual(['call_dup']);
|
||||
expect(everyToolUseImmediatelyAnswered(projected)).toBe(true);
|
||||
});
|
||||
|
||||
it("under the strict flag, reattaches a later duplicate's result when the first call has none", () => {
|
||||
const projected = project(
|
||||
[
|
||||
user('u1'),
|
||||
assistant(['call_a'], 'first attempt'),
|
||||
assistant(['call_a'], 'second attempt'),
|
||||
tool('call_a', 'late result'),
|
||||
user('u2'),
|
||||
],
|
||||
{ dedupeDuplicateToolCalls: true, dropOrphanResults: true },
|
||||
);
|
||||
expect(projected.map((message) => message.role)).toEqual([
|
||||
'user',
|
||||
'assistant',
|
||||
'tool',
|
||||
'assistant',
|
||||
'user',
|
||||
]);
|
||||
expect(textOf(projected[2])).toBe('late result');
|
||||
expect(everyToolUseImmediatelyAnswered(projected)).toBe(true);
|
||||
});
|
||||
|
||||
it('under the strict flag, drops an assistant message left empty after removing duplicates', () => {
|
||||
const projected = project(
|
||||
[user('u1'), assistant(['call_a'], 'first'), tool('call_a'), assistant(['call_a']), user('u2')],
|
||||
{ dedupeDuplicateToolCalls: true, dropOrphanResults: true },
|
||||
);
|
||||
expect(projected.map((message) => message.role)).toEqual(['user', 'assistant', 'tool', 'user']);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Property-based fuzz test
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -232,8 +232,8 @@ describe('Agent permission', () => {
|
|||
[wire] permission.record_approval_result { "turnId": 0, "toolCallId": "call_bash", "toolName": "Bash", "action": "Running: printf should-not-run", "result": { "decision": "rejected", "selectedLabel": "reject" }, "time": "<time>" }
|
||||
[wire] context.append_loop_event { "event": { "type": "tool.call", "uuid": "call_bash", "turnId": "0", "step": 1, "stepUuid": "<uuid-1>", "toolCallId": "call_bash", "name": "Bash", "args": { "command": "printf should-not-run", "timeout": 60 }, "description": "Running: printf should-not-run", "display": { "kind": "command", "command": "printf should-not-run", "cwd": "<cwd>", "language": "bash" } }, "time": "<time>" }
|
||||
[emit] tool.call.started { "turnId": 0, "toolCallId": "call_bash", "name": "Bash", "args": { "command": "printf should-not-run", "timeout": 60 }, "description": "Running: printf should-not-run", "display": { "kind": "command", "command": "printf should-not-run", "cwd": "<cwd>", "language": "bash" } }
|
||||
[wire] context.append_loop_event { "event": { "type": "tool.result", "parentUuid": "call_bash", "toolCallId": "call_bash", "result": { "output": "Tool \\"Bash\\" was not run because the user rejected the approval request.", "isError": true } }, "time": "<time>" }
|
||||
[emit] tool.result { "turnId": 0, "toolCallId": "call_bash", "output": "Tool \\"Bash\\" was not run because the user rejected the approval request.", "isError": true }
|
||||
[wire] context.append_loop_event { "event": { "type": "tool.result", "parentUuid": "call_bash", "toolCallId": "call_bash", "result": { "output": "Tool \\"Bash\\" was not run because the user rejected the approval request. Do not re-attempt the exact same call — think about why it was rejected, then adjust your approach or ask the user what they would prefer.", "isError": true } }, "time": "<time>" }
|
||||
[emit] tool.result { "turnId": 0, "toolCallId": "call_bash", "output": "Tool \\"Bash\\" was not run because the user rejected the approval request. Do not re-attempt the exact same call — think about why it was rejected, then adjust your approach or ask the user what they would prefer.", "isError": true }
|
||||
[wire] context.append_loop_event { "event": { "type": "step.end", "uuid": "<uuid-1>", "turnId": "0", "step": 1, "usage": { "inputOther": 5, "output": 22, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "tool_use" }, "time": "<time>" }
|
||||
[emit] turn.step.completed { "turnId": 0, "step": 1, "stepId": "<uuid-1>", "usage": { "inputOther": 5, "output": 22, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "tool_use" }
|
||||
[wire] usage.record { "model": "mock-model", "usage": { "inputOther": 5, "output": 22, "inputCacheRead": 0, "inputCacheCreation": 0 }, "usageScope": "turn", "time": "<time>" }
|
||||
|
|
@ -242,10 +242,10 @@ describe('Agent permission', () => {
|
|||
[emit] turn.step.started { "turnId": 0, "step": 2, "stepId": "<uuid-3>" }
|
||||
[emit] assistant.delta { "turnId": 0, "delta": "I will not run the command." }
|
||||
[wire] context.append_loop_event { "event": { "type": "content.part", "uuid": "<uuid-4>", "turnId": "0", "step": 2, "stepUuid": "<uuid-3>", "part": { "type": "text", "text": "I will not run the command." } }, "time": "<time>" }
|
||||
[wire] context.append_loop_event { "event": { "type": "step.end", "uuid": "<uuid-3>", "turnId": "0", "step": 2, "usage": { "inputOther": 58, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "end_turn" }, "time": "<time>" }
|
||||
[emit] turn.step.completed { "turnId": 0, "step": 2, "stepId": "<uuid-3>", "usage": { "inputOther": 58, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "end_turn" }
|
||||
[wire] usage.record { "model": "mock-model", "usage": { "inputOther": 58, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "usageScope": "turn", "time": "<time>" }
|
||||
[emit] agent.status.updated { "model": "mock-model", "contextTokens": 68, "maxContextTokens": 1000000, "contextUsage": 0.000068, "planMode": false, "swarmMode": false, "permission": "manual", "usage": { "byModel": { "mock-model": { "inputOther": 63, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 } }, "total": { "inputOther": 63, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 }, "currentTurn": { "inputOther": 63, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 } } }
|
||||
[wire] context.append_loop_event { "event": { "type": "step.end", "uuid": "<uuid-3>", "turnId": "0", "step": 2, "usage": { "inputOther": 93, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "end_turn" }, "time": "<time>" }
|
||||
[emit] turn.step.completed { "turnId": 0, "step": 2, "stepId": "<uuid-3>", "usage": { "inputOther": 93, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "finishReason": "end_turn" }
|
||||
[wire] usage.record { "model": "mock-model", "usage": { "inputOther": 93, "output": 10, "inputCacheRead": 0, "inputCacheCreation": 0 }, "usageScope": "turn", "time": "<time>" }
|
||||
[emit] agent.status.updated { "model": "mock-model", "contextTokens": 103, "maxContextTokens": 1000000, "contextUsage": 0.000103, "planMode": false, "swarmMode": false, "permission": "manual", "usage": { "byModel": { "mock-model": { "inputOther": 98, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 } }, "total": { "inputOther": 98, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 }, "currentTurn": { "inputOther": 98, "output": 32, "inputCacheRead": 0, "inputCacheCreation": 0 } } }
|
||||
[emit] turn.ended { "turnId": 0, "reason": "completed" }
|
||||
`);
|
||||
expect(execWithEnv).not.toHaveBeenCalled();
|
||||
|
|
@ -253,7 +253,7 @@ describe('Agent permission', () => {
|
|||
messages:
|
||||
<last>
|
||||
assistant: text "I will try Bash." calls call_bash:Bash { "command": "printf should-not-run", "timeout": 60 }
|
||||
tool[call_bash]: text "<system>ERROR: Tool execution failed.</system>\\nTool \\"Bash\\" was not run because the user rejected the approval request."
|
||||
tool[call_bash]: text "<system>ERROR: Tool execution failed.</system>\\nTool \\"Bash\\" was not run because the user rejected the approval request. Do not re-attempt the exact same call — think about why it was rejected, then adjust your approach or ask the user what they would prefer."
|
||||
`);
|
||||
await ctx.expectResumeMatches();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -17,7 +17,8 @@ import { describe, expect, it, vi } from 'vitest';
|
|||
|
||||
import { HookEngine } from '../../src/session/hooks';
|
||||
import { abortError } from '../../src/utils/abort';
|
||||
import type { AgentOptions } from '../../src/agent';
|
||||
import type { AgentOptions, AgentRecord, AgentRecordPersistence } from '../../src/agent';
|
||||
import { InMemoryAgentRecordPersistence } from '../../src/agent/records';
|
||||
import { ErrorCodes, KimiError } from '../../src/errors';
|
||||
import type { Logger, LogPayload } from '../../src/logging';
|
||||
import type {
|
||||
|
|
@ -1928,3 +1929,68 @@ function textResult(text: string): Awaited<ReturnType<GenerateFn>> {
|
|||
rawFinishReason: 'stop',
|
||||
};
|
||||
}
|
||||
|
||||
describe('abandoned tool exchange teardown', () => {
|
||||
it('closes dangling tool calls when a turn dies mid-batch so follow-up messages are not swallowed', async () => {
|
||||
// A transcript write failure between a recorded tool.call and its paired
|
||||
// tool.result breaks the batch's "every recorded call gets a result"
|
||||
// invariant: the result-dispatch loop dies, the turn fails, and
|
||||
// pendingToolResultIds stays open — stranding every later message in
|
||||
// deferredMessages.
|
||||
const base = new InMemoryAgentRecordPersistence();
|
||||
let failedOnce = false;
|
||||
const persistence: AgentRecordPersistence = {
|
||||
read: () => base.read(),
|
||||
append: (record: AgentRecord) => {
|
||||
if (
|
||||
!failedOnce &&
|
||||
record.type === 'context.append_loop_event' &&
|
||||
record.event.type === 'tool.result'
|
||||
) {
|
||||
failedOnce = true;
|
||||
throw new Error('transcript write failed');
|
||||
}
|
||||
base.append(record);
|
||||
},
|
||||
rewrite: (records) => {
|
||||
base.rewrite(records);
|
||||
},
|
||||
flush: () => base.flush(),
|
||||
close: () => base.close(),
|
||||
};
|
||||
const ctx = testAgent({ kaos: createCommandKaos('ok'), persistence });
|
||||
ctx.configure({ tools: ['Bash'] });
|
||||
await ctx.rpc.setPermission({ mode: 'auto' });
|
||||
|
||||
ctx.mockNextResponse(
|
||||
{ type: 'text', text: 'I will run both commands.' },
|
||||
bashCallWithId('call_one', 'echo one'),
|
||||
bashCallWithId('call_two', 'echo two'),
|
||||
);
|
||||
await ctx.rpc.prompt({ input: [{ type: 'text', text: 'run both' }] });
|
||||
const events = await ctx.untilTurnEnd();
|
||||
expect(events).toContainEqual(
|
||||
expect.objectContaining({
|
||||
event: 'turn.ended',
|
||||
args: expect.objectContaining({ reason: 'failed' }),
|
||||
}),
|
||||
);
|
||||
|
||||
// Every recorded tool.call must still get a result: the turn teardown
|
||||
// synthesizes an error result for each dangling call.
|
||||
const toolMessages = ctx.agent.context.history.filter((message) => message.role === 'tool');
|
||||
expect(toolMessages.map((message) => message.toolCallId)).toEqual(['call_one', 'call_two']);
|
||||
for (const message of toolMessages) {
|
||||
expect(message.isError).toBe(true);
|
||||
}
|
||||
|
||||
// With the exchange closed, a follow-up message reaches the history instead
|
||||
// of being stranded in deferredMessages forever.
|
||||
ctx.agent.context.appendMessage({
|
||||
role: 'user',
|
||||
content: [{ type: 'text', text: 'follow-up after failure' }],
|
||||
toolCalls: [],
|
||||
});
|
||||
expect(JSON.stringify(ctx.agent.context.history)).toContain('follow-up after failure');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -237,6 +237,11 @@ const STRUCTURAL_REQUEST_MESSAGE_PATTERNS = [
|
|||
/first message must use the .*user.* role/,
|
||||
/roles must alternate/,
|
||||
/multiple .*(?:user|assistant).* roles in a row/,
|
||||
// Anthropic rejects a request whose assistant messages carry two `tool_use`
|
||||
// blocks with the same id: "messages: `tool_use` ids must be unique". Seen
|
||||
// when a provider reused a call id (e.g. per-response counter ids) earlier
|
||||
// in the session; the strict resend dedupes the ids.
|
||||
/tool_use[\s\S]*ids must be unique/,
|
||||
] as const;
|
||||
|
||||
export function isRecoverableRequestStructureError(error: unknown): boolean {
|
||||
|
|
|
|||
|
|
@ -378,6 +378,14 @@ describe('isRecoverableRequestStructureError', () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it('matches the Anthropic duplicate tool_use id rejection', () => {
|
||||
expect(
|
||||
isRecoverableRequestStructureError(
|
||||
new APIStatusError(400, 'messages: `tool_use` ids must be unique'),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('matches empty / whitespace-only text content rejections', () => {
|
||||
expect(
|
||||
isRecoverableRequestStructureError(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue