diff --git a/extensions/cli/src/stream/handleToolCalls.test.ts b/extensions/cli/src/stream/handleToolCalls.test.ts index a6587e7bd0a..756a86d79ec 100644 --- a/extensions/cli/src/stream/handleToolCalls.test.ts +++ b/extensions/cli/src/stream/handleToolCalls.test.ts @@ -63,6 +63,46 @@ describe("handleToolCalls - duplicate tool_result prevention", () => { vi.mocked(services.chatHistory.isReady).mockReturnValue(true); }); + it("records repeated tool calls as errors without executing them", async () => { + const toolCalls = [ + { + id: "repeated-tool-1", + name: "read_file", + arguments: { filepath: "/tmp/example" }, + argumentsStr: '{"filepath":"/tmp/example"}', + startNotified: false, + }, + ]; + + mockPreprocess.mockResolvedValue({ + preprocessedCalls: [], + errorChatEntries: [], + }); + mockExecute.mockResolvedValue({ + hasRejection: false, + chatHistoryEntries: [], + }); + + const shouldReturn = await handleToolCalls({ + toolCalls, + chatHistory, + content: "", + callbacks: undefined, + isHeadless: false, + blockedToolCallIds: new Set(["repeated-tool-1"]), + }); + + expect(shouldReturn).toBe(true); + expect(services.chatHistory.addAssistantMessage).toHaveBeenCalledTimes(1); + expect(services.chatHistory.addToolResult).toHaveBeenCalledWith( + "repeated-tool-1", + "Repeated identical tool call blocked to prevent an unbounded agent loop", + "errored", + ); + expect(mockPreprocess).toHaveBeenCalledWith(false, [], undefined); + expect(mockExecute).toHaveBeenCalledWith([], undefined, false); + }); + /** * This test verifies that preprocessing errors are stored in toolCallStates * (on the assistant message) rather than as separate tool history items. diff --git a/extensions/cli/src/stream/handleToolCalls.ts b/extensions/cli/src/stream/handleToolCalls.ts index c58206b56f8..76ddcba1c00 100644 --- a/extensions/cli/src/stream/handleToolCalls.ts +++ b/extensions/cli/src/stream/handleToolCalls.ts @@ -31,13 +31,55 @@ interface HandleToolCallsOptions { callbacks: StreamCallbacks | undefined; isHeadless: boolean; usage?: any; + blockedToolCallIds?: ReadonlySet; +} + +const REPEATED_TOOL_CALL_ERROR = + "Repeated identical tool call blocked to prevent an unbounded agent loop"; + +function recordToolResult( + chatHistory: ChatHistoryItem[], + useService: boolean, + toolCallId: string, + content: string, + status: ToolStatus, +): void { + if (useService) { + services.chatHistory.addToolResult(toolCallId, content, status); + return; + } + + const lastAssistantIndex = chatHistory.findLastIndex( + (item) => item.message.role === "assistant" && item.toolCallStates, + ); + const toolCallStates = chatHistory[lastAssistantIndex]?.toolCallStates; + const toolState = toolCallStates?.find( + (state) => state.toolCallId === toolCallId, + ); + if (toolState) { + toolState.status = status; + toolState.output = [ + { + content, + name: "Tool Result", + description: "Tool execution result", + }, + ]; + } } export async function handleToolCalls( options: HandleToolCallsOptions, ): Promise { - const { toolCalls, chatHistory, content, callbacks, isHeadless, usage } = - options; + const { + toolCalls, + chatHistory, + content, + callbacks, + isHeadless, + usage, + blockedToolCallIds, + } = options; const chatHistorySvc = services.chatHistory; const useService = typeof chatHistorySvc?.isReady === "function" && chatHistorySvc.isReady(); @@ -61,6 +103,13 @@ export async function handleToolCalls( return false; } + const blockedToolCalls = toolCalls.filter((toolCall) => + blockedToolCallIds?.has(toolCall.id), + ); + const executableToolCalls = toolCalls.filter( + (toolCall) => !blockedToolCallIds?.has(toolCall.id), + ); + // Create tool call states for the ChatHistoryItem const toolCallStates = toolCalls.map((tc) => ({ toolCallId: tc.id, @@ -106,44 +155,37 @@ export async function handleToolCalls( chatHistory.push(createHistoryItem(messageWithUsage, [], toolCallStates)); } + for (const blockedToolCall of blockedToolCalls) { + callbacks?.onToolStart?.(blockedToolCall.name, blockedToolCall.arguments); + recordToolResult( + chatHistory, + useService, + blockedToolCall.id, + REPEATED_TOOL_CALL_ERROR, + "errored", + ); + callbacks?.onToolError?.(REPEATED_TOOL_CALL_ERROR, blockedToolCall.name); + } + // First preprocess the tool calls const { preprocessedCalls, errorChatEntries } = - await preprocessStreamedToolCalls(isHeadless, toolCalls, callbacks); + await preprocessStreamedToolCalls( + isHeadless, + executableToolCalls, + callbacks, + ); // Add any preprocessing errors to the toolCallStates on the assistant message // (NOT as separate history items, which would cause duplicate tool_result messages) errorChatEntries.forEach((errorEntry) => { const errorContent = stripImages(errorEntry.content) || ""; - if (useService) { - chatHistorySvc.addToolResult( - errorEntry.tool_call_id, - errorContent, - "errored", - ); - } else { - // Fallback only when service is unavailable: update local tool state - const lastAssistantIndex = chatHistory.findLastIndex( - (item) => item.message.role === "assistant" && item.toolCallStates, - ); - if ( - lastAssistantIndex >= 0 && - chatHistory[lastAssistantIndex].toolCallStates - ) { - const toolState = chatHistory[lastAssistantIndex].toolCallStates.find( - (ts) => ts.toolCallId === errorEntry.tool_call_id, - ); - if (toolState) { - toolState.status = "errored"; - toolState.output = [ - { - content: errorContent, - name: `Tool Result`, - description: "Tool execution result", - }, - ]; - } - } - } + recordToolResult( + chatHistory, + useService, + errorEntry.tool_call_id, + errorContent, + "errored", + ); }); // Execute the valid preprocessed tool calls @@ -166,7 +208,7 @@ export async function handleToolCalls( // via services.chatHistory.addToolResult() - no need to add them again here. // Adding them again would be redundant (and previously caused duplicate tool_result messages // when combined with separate tool history items). - return false; + return blockedToolCalls.length > 0; } export async function getRequestTools(isHeadless: boolean) { diff --git a/extensions/cli/src/stream/streamChatResponse.autoContinuation.test.ts b/extensions/cli/src/stream/streamChatResponse.autoContinuation.test.ts index c507a99183e..96cd6ca592f 100644 --- a/extensions/cli/src/stream/streamChatResponse.autoContinuation.test.ts +++ b/extensions/cli/src/stream/streamChatResponse.autoContinuation.test.ts @@ -197,6 +197,62 @@ describe("streamChatResponse - auto-continuation after compaction", () => { expect(callCount).toBeGreaterThan(1); }); + it("should block a third consecutive identical tool-call batch", async () => { + const { handleToolCalls } = await import("./handleToolCalls.js"); + + const blockedCallIds: string[][] = []; + vi.mocked(handleToolCalls).mockImplementation(async (options: any) => { + blockedCallIds.push( + options.blockedToolCallIds + ? Array.from(options.blockedToolCallIds) + : [], + ); + return Boolean(options.blockedToolCallIds?.size); + }); + + let callCount = 0; + mockLlmApi.chatCompletionStream = vi + .fn() + .mockImplementation(async function* () { + callCount++; + yield { + id: "test", + object: "chat.completion.chunk", + created: Date.now(), + model: "test-model", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: `call_${callCount}`, + type: "function", + function: { + name: "read_file", + arguments: '{"filepath":"/tmp/repeat"}', + }, + }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }; + }) as any; + + await streamChatResponse( + chatHistory, + mockModel, + mockLlmApi, + mockAbortController, + ); + + expect(callCount).toBe(3); + expect(blockedCallIds).toEqual([[], [], ["call_3"]]); + }); + it("should not auto-continue if compaction occurs with tool calls pending", async () => { const { services } = await import("../services/index.js"); const { handleNormalAutoCompaction } = await import( diff --git a/extensions/cli/src/stream/streamChatResponse.ts b/extensions/cli/src/stream/streamChatResponse.ts index 20156663381..96fe1126ba8 100644 --- a/extensions/cli/src/stream/streamChatResponse.ts +++ b/extensions/cli/src/stream/streamChatResponse.ts @@ -37,6 +37,7 @@ import { getDefaultCompletionOptions, StreamCallbacks, } from "./streamChatResponse.types.js"; +import { ToolCallLoopGuard } from "./toolCallLoopGuard.js"; dotenv.config(); @@ -439,6 +440,7 @@ export async function streamChatResponse( let fullResponse = ""; let finalResponse = ""; let compactionOccurredThisTurn = false; // Track if compaction happened during this conversation turn + const toolCallLoopGuard = new ToolCallLoopGuard(); while (true) { // If ChatHistoryService is available, refresh local chatHistory view @@ -504,6 +506,17 @@ export async function streamChatResponse( finalResponse, ); + const loopGuardResult = toolCallLoopGuard.observe(toolCalls); + const blockedToolCallIds = loopGuardResult.blocked + ? new Set(toolCalls.map((toolCall) => toolCall.id)) + : undefined; + if (loopGuardResult.blocked) { + logger.warn("Blocked repeated tool-call batch", { + count: loopGuardResult.count, + toolCalls: toolCalls.map((toolCall) => toolCall.name), + }); + } + // Handle content display handleContentDisplay(content, callbacks, isHeadless); @@ -515,6 +528,7 @@ export async function streamChatResponse( callbacks, isHeadless, usage, + blockedToolCallIds, }); if (shouldReturn) { diff --git a/extensions/cli/src/stream/toolCallLoopGuard.test.ts b/extensions/cli/src/stream/toolCallLoopGuard.test.ts new file mode 100644 index 00000000000..7f40153a414 --- /dev/null +++ b/extensions/cli/src/stream/toolCallLoopGuard.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; + +import { + DEFAULT_MAX_IDENTICAL_TOOL_CALLS, + getToolCallFingerprint, + ToolCallLoopGuard, +} from "./toolCallLoopGuard.js"; + +const toolCall = (id: string, args: Record) => ({ + id, + name: "read_file", + arguments: args, + argumentsStr: JSON.stringify(args), + startNotified: true, +}); + +describe("ToolCallLoopGuard", () => { + it("ignores provider-generated IDs and object key order", () => { + expect( + getToolCallFingerprint([toolCall("first", { path: "a", line: 1 })]), + ).toBe( + getToolCallFingerprint([toolCall("second", { line: 1, path: "a" })]), + ); + }); + + it("blocks only after the configured consecutive limit", () => { + const guard = new ToolCallLoopGuard(); + const calls = [toolCall("call-1", { path: "a" })]; + + expect(guard.observe(calls)).toMatchObject({ + count: 1, + blocked: false, + }); + expect(guard.observe([toolCall("call-2", { path: "a" })])).toMatchObject({ + count: DEFAULT_MAX_IDENTICAL_TOOL_CALLS, + blocked: false, + }); + expect(guard.observe([toolCall("call-3", { path: "a" })])).toMatchObject({ + count: DEFAULT_MAX_IDENTICAL_TOOL_CALLS + 1, + blocked: true, + }); + }); + + it("resets when the agent changes the tool call or returns no tools", () => { + const guard = new ToolCallLoopGuard(2); + + guard.observe([toolCall("call-1", { path: "a" })]); + guard.observe([toolCall("call-2", { path: "a" })]); + expect(guard.observe([toolCall("call-3", { path: "b" })])).toMatchObject({ + count: 1, + blocked: false, + }); + expect(guard.observe([])).toMatchObject({ + count: 0, + blocked: false, + }); + expect(guard.observe([toolCall("call-4", { path: "a" })])).toMatchObject({ + count: 1, + blocked: false, + }); + }); +}); diff --git a/extensions/cli/src/stream/toolCallLoopGuard.ts b/extensions/cli/src/stream/toolCallLoopGuard.ts new file mode 100644 index 00000000000..5a2e1c91d77 --- /dev/null +++ b/extensions/cli/src/stream/toolCallLoopGuard.ts @@ -0,0 +1,76 @@ +import type { ToolCall } from "../tools/types.js"; + +/** The number of times an identical tool-call batch may run in one turn. */ +export const DEFAULT_MAX_IDENTICAL_TOOL_CALLS = 2; + +function canonicalize(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(canonicalize); + } + + if (value && typeof value === "object") { + return Object.fromEntries( + Object.entries(value) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([key, entry]) => [key, canonicalize(entry)]), + ); + } + + return value; +} + +/** + * Builds a stable fingerprint for a tool-call batch. + * Call IDs are intentionally excluded because providers may generate a new ID + * when they repeat the same operation. + */ +export function getToolCallFingerprint(toolCalls: readonly ToolCall[]): string { + return JSON.stringify( + toolCalls.map((toolCall) => [ + toolCall.name, + canonicalize(toolCall.arguments), + ]), + ); +} + +export interface ToolCallLoopGuardResult { + count: number; + fingerprint: string; + blocked: boolean; +} + +/** Tracks consecutive identical tool-call batches within one agent turn. */ +export class ToolCallLoopGuard { + private lastFingerprint: string | undefined; + private identicalCount = 0; + + constructor( + private readonly maxIdenticalToolCalls = DEFAULT_MAX_IDENTICAL_TOOL_CALLS, + ) { + if (!Number.isInteger(maxIdenticalToolCalls) || maxIdenticalToolCalls < 1) { + throw new Error("maxIdenticalToolCalls must be a positive integer"); + } + } + + observe(toolCalls: readonly ToolCall[]): ToolCallLoopGuardResult { + if (toolCalls.length === 0) { + this.lastFingerprint = undefined; + this.identicalCount = 0; + return { count: 0, fingerprint: "", blocked: false }; + } + + const fingerprint = getToolCallFingerprint(toolCalls); + if (fingerprint === this.lastFingerprint) { + this.identicalCount += 1; + } else { + this.lastFingerprint = fingerprint; + this.identicalCount = 1; + } + + return { + count: this.identicalCount, + fingerprint, + blocked: this.identicalCount > this.maxIdenticalToolCalls, + }; + } +}