fix(agent-core): harden tool_use/tool_result exchange integrity

.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.

.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.

packages/agent-core/src/agent/context/index.ts

@@ -397,13 +397,17 @@ 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;
     for (const anomaly of notable) {
       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,21 +491,37 @@ 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({
         type: 'tool.result',
         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: [],

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<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

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}`;
   }
 

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.`;
+}

packages/agent-core/src/profile/default/system.md

@@ -20,6 +20,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).

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');
+  });
+});