From e2fe62a5eff124b816a0f656355fc5bea85e3893 Mon Sep 17 00:00:00 2001 From: Kai Date: Fri, 3 Jul 2026 14:26:57 +0800 Subject: [PATCH] fix(agent-core): harden tool_use/tool_result exchange integrity (#1340) * 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 --- .changeset/close-abandoned-tool-exchanges.md | 5 + .changeset/dedupe-duplicate-tool-call-ids.md | 5 + .../agent-core/src/agent/context/index.ts | 65 +++++++++-- .../agent-core/src/agent/context/projector.ts | 72 ++++++++++++- .../agent-core/src/agent/permission/index.ts | 3 + packages/agent-core/src/agent/turn/index.ts | 42 ++++++++ .../agent-core/src/profile/default/system.md | 2 + .../agent-core/test/agent/context.test.ts | 60 +++++++++++ .../test/agent/context/projector.test.ts | 101 ++++++++++++++++++ .../agent-core/test/agent/permission.test.ts | 14 +-- packages/agent-core/test/agent/turn.test.ts | 68 +++++++++++- packages/kosong/src/errors.ts | 5 + packages/kosong/test/errors.test.ts | 8 ++ 13 files changed, 427 insertions(+), 23 deletions(-) create mode 100644 .changeset/close-abandoned-tool-exchanges.md create mode 100644 .changeset/dedupe-duplicate-tool-call-ids.md diff --git a/.changeset/close-abandoned-tool-exchanges.md b/.changeset/close-abandoned-tool-exchanges.md new file mode 100644 index 000000000..94f397435 --- /dev/null +++ b/.changeset/close-abandoned-tool-exchanges.md @@ -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. diff --git a/.changeset/dedupe-duplicate-tool-call-ids.md b/.changeset/dedupe-duplicate-tool-call-ids.md new file mode 100644 index 000000000..ab358a693 --- /dev/null +++ b/.changeset/dedupe-duplicate-tool-call-ids.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix requests being rejected by strict providers when the model emits duplicate tool call ids. diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 3b3580187..5c93ebf45 100644 --- a/packages/agent-core/src/agent/context/index.ts +++ b/packages/agent-core/src/agent/context/index.ts @@ -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: [], diff --git a/packages/agent-core/src/agent/context/projector.ts b/packages/agent-core/src/agent/context/projector.ts index ebf08dc1b..75ee0fbcb 100644 --- a/packages/agent-core/src/agent/context/projector.ts +++ b/packages/agent-core/src/agent/context/projector.ts @@ -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(); + const seenToolResultIds = new Set(); + 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 diff --git a/packages/agent-core/src/agent/permission/index.ts b/packages/agent-core/src/agent/permission/index.ts index 4df71cc90..3d901becf 100644 --- a/packages/agent-core/src/agent/permission/index.ts +++ b/packages/agent-core/src/agent/permission/index.ts @@ -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}`; } diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index 847794aaa..b79b6f83b 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -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.`; +} diff --git a/packages/agent-core/src/profile/default/system.md b/packages/agent-core/src/profile/default/system.md index 022e0699a..0671cd108 100644 --- a/packages/agent-core/src/profile/default/system.md +++ b/packages/agent-core/src/profile/default/system.md @@ -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 `` 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 `` tags. Unlike `` 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). diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 2d3d15e88..4b613dde0 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -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'); + }); +}); diff --git a/packages/agent-core/test/agent/context/projector.test.ts b/packages/agent-core/test/agent/context/projector.test.ts index f3cfbcf51..3742880e7 100644 --- a/packages/agent-core/test/agent/context/projector.test.ts +++ b/packages/agent-core/test/agent/context/projector.test.ts @@ -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 // --------------------------------------------------------------------------- diff --git a/packages/agent-core/test/agent/permission.test.ts b/packages/agent-core/test/agent/permission.test.ts index 3e3a75d6e..5321b26ba 100644 --- a/packages/agent-core/test/agent/permission.test.ts +++ b/packages/agent-core/test/agent/permission.test.ts @@ -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": "