diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 3282eb90d1..c3aa7e8a34 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -304,6 +304,7 @@ export async function presentAssistantMessage(cline: Task) { // Track if we've already pushed a tool result for this tool call (native protocol only) let hasToolResult = false + let lastToolResponse: string | undefined // Determine protocol by checking if this tool call has an ID. // Native protocol tool calls ALWAYS have an ID (set when parsed from tool_call chunks). @@ -339,6 +340,11 @@ export async function presentAssistantMessage(cline: Task) { "(tool did not return anything)" } + // Track the response for progress detection + lastToolResponse = resultContent + // Update the detector with this response for future checks + cline.toolRepetitionDetector.updateLastResponse(resultContent) + // Add tool_result with text content only cline.userMessageContent.push({ type: "tool_result", @@ -357,11 +363,26 @@ export async function presentAssistantMessage(cline: Task) { cline.userMessageContent.push({ type: "text", text: `${toolDescription()} Result:` }) if (typeof content === "string") { + // Track the response for progress detection + lastToolResponse = content || "(tool did not return anything)" + // Update the detector with this response for future checks + cline.toolRepetitionDetector.updateLastResponse(lastToolResponse) + cline.userMessageContent.push({ type: "text", text: content || "(tool did not return anything)", }) } else { + // For array responses, track text content + const textContent = content + .filter((item) => item.type === "text") + .map((item) => (item as Anthropic.TextBlockParam).text) + .join("\n") + if (textContent) { + lastToolResponse = textContent + // Update the detector with this response for future checks + cline.toolRepetitionDetector.updateLastResponse(textContent) + } cline.userMessageContent.push(...content) } } @@ -514,7 +535,8 @@ export async function presentAssistantMessage(cline: Task) { // Check for identical consecutive tool calls. if (!block.partial) { // Use the detector to check for repetition, passing the ToolUse - // block directly. + // block directly and the last tool response for progress detection. + // The lastToolResponse from previous execution is already tracked in the detector const repetitionCheck = cline.toolRepetitionDetector.check(block) // If execution is not allowed, notify user and break. diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index 9e70bb41a0..24fd2716d6 100644 --- a/src/core/tools/ToolRepetitionDetector.ts +++ b/src/core/tools/ToolRepetitionDetector.ts @@ -2,21 +2,50 @@ import stringify from "safe-stable-stringify" import { ToolUse } from "../../shared/tools" import { t } from "../../i18n" +/** + * Configuration for tool-specific repetition limits + */ +interface ToolSpecificConfig { + /** Tool names that should be excluded from repetition detection */ + excludedTools?: string[] + /** Custom limits for specific tools (key is tool name, value is limit) */ + toolLimits?: Record +} + /** * Class for detecting consecutive identical tool calls * to prevent the AI from getting stuck in a loop. + * + * Enhanced to detect actual progress by tracking tool responses + * and providing special handling for tools that legitimately need + * multiple consecutive calls (e.g., MCP tools reading streaming data). */ export class ToolRepetitionDetector { private previousToolCallJson: string | null = null private consecutiveIdenticalToolCallCount: number = 0 private readonly consecutiveIdenticalToolCallLimit: number + private lastToolResponse: string | null = null + private responseHistory: string[] = [] + private readonly maxResponseHistorySize = 10 + private readonly toolSpecificConfig: ToolSpecificConfig /** * Creates a new ToolRepetitionDetector * @param limit The maximum number of identical consecutive tool calls allowed + * @param toolSpecificConfig Configuration for tool-specific behavior */ - constructor(limit: number = 3) { + constructor(limit: number = 3, toolSpecificConfig: ToolSpecificConfig = {}) { this.consecutiveIdenticalToolCallLimit = limit + this.toolSpecificConfig = { + excludedTools: toolSpecificConfig.excludedTools || [], + toolLimits: { + // MCP tools often need more consecutive calls for legitimate streaming/chunked data + use_mcp_tool: 50, + access_mcp_resource: 50, + // Override with any user-provided limits + ...(toolSpecificConfig.toolLimits || {}), + }, + } } /** @@ -24,15 +53,24 @@ export class ToolRepetitionDetector { * and determines if execution should be allowed * * @param currentToolCallBlock ToolUse object representing the current tool call + * @param toolResponse Optional tool response from the last execution to track progress * @returns Object indicating if execution is allowed and a message to show if not */ - public check(currentToolCallBlock: ToolUse): { + public check( + currentToolCallBlock: ToolUse, + toolResponse?: string, + ): { allowExecution: boolean askUser?: { messageKey: string messageDetail: string } } { + // Check if this tool is excluded from repetition detection + if (this.toolSpecificConfig.excludedTools?.includes(currentToolCallBlock.name)) { + return { allowExecution: true } + } + // Browser scroll actions should not be subject to repetition detection // as they are frequently needed for navigating through web pages if (this.isBrowserScrollAction(currentToolCallBlock)) { @@ -40,6 +78,16 @@ export class ToolRepetitionDetector { return { allowExecution: true } } + // Special handling for MCP tools that may be reading streaming data + if (this.isMcpStreamingTool(currentToolCallBlock)) { + // Check if we're making progress by comparing responses + if (this.isShowingProgress(toolResponse)) { + // Reset the counter since we're making progress + this.consecutiveIdenticalToolCallCount = 0 + return { allowExecution: true } + } + } + // Serialize the block to a canonical JSON string for comparison const currentToolCallJson = this.serializeToolUse(currentToolCallBlock) @@ -49,23 +97,45 @@ export class ToolRepetitionDetector { } else { this.consecutiveIdenticalToolCallCount = 0 // Reset to 0 for a new tool this.previousToolCallJson = currentToolCallJson + // Clear response history when switching to a different tool + this.responseHistory = [] + this.lastToolResponse = null } + // Update response tracking + if (toolResponse) { + this.responseHistory.push(toolResponse) + if (this.responseHistory.length > this.maxResponseHistorySize) { + this.responseHistory.shift() + } + this.lastToolResponse = toolResponse + } + + // Get the appropriate limit for this tool + const toolLimit = this.getToolLimit(currentToolCallBlock.name) + // Check if limit is reached (0 means unlimited) - if ( - this.consecutiveIdenticalToolCallLimit > 0 && - this.consecutiveIdenticalToolCallCount >= this.consecutiveIdenticalToolCallLimit - ) { + if (toolLimit > 0 && this.consecutiveIdenticalToolCallCount >= toolLimit) { + // For MCP tools, provide a more helpful message + const isMcpTool = this.isMcpStreamingTool(currentToolCallBlock) + const messageDetail = isMcpTool + ? t("tools:toolRepetitionLimitReached", { toolName: currentToolCallBlock.name }) + + " This may be a false positive if the tool is legitimately reading streaming data. " + + "Consider increasing the repetition limit for MCP tools in settings." + : t("tools:toolRepetitionLimitReached", { toolName: currentToolCallBlock.name }) + // Reset counters to allow recovery if user guides the AI past this point this.consecutiveIdenticalToolCallCount = 0 this.previousToolCallJson = null + this.responseHistory = [] + this.lastToolResponse = null // Return result indicating execution should not be allowed return { allowExecution: false, askUser: { messageKey: "mistake_limit_reached", - messageDetail: t("tools:toolRepetitionLimitReached", { toolName: currentToolCallBlock.name }), + messageDetail, }, } } @@ -74,6 +144,67 @@ export class ToolRepetitionDetector { return { allowExecution: true } } + /** + * Updates the last tool response for progress tracking + * @param response The response from the last tool execution + */ + public updateLastResponse(response: string): void { + this.lastToolResponse = response + this.responseHistory.push(response) + if (this.responseHistory.length > this.maxResponseHistorySize) { + this.responseHistory.shift() + } + } + + /** + * Checks if the tool responses show progress is being made + * @param currentResponse The current tool response to check + * @returns true if progress is detected + */ + private isShowingProgress(currentResponse?: string): boolean { + if (!currentResponse || !this.lastToolResponse) { + return false + } + + // Check if the response is different from the last one + if (currentResponse !== this.lastToolResponse) { + return true + } + + // Check if we have varying responses in history (not all the same) + if (this.responseHistory.length > 1) { + const uniqueResponses = new Set(this.responseHistory) + // If we have more than one unique response, we're likely making progress + return uniqueResponses.size > 1 + } + + return false + } + + /** + * Checks if a tool is an MCP streaming tool that may need many consecutive calls + * @param toolUse The tool use to check + * @returns true if it's an MCP tool that might stream data + */ + private isMcpStreamingTool(toolUse: ToolUse): boolean { + // MCP tools that commonly need to read streaming/chunked data + return toolUse.name === "use_mcp_tool" || toolUse.name === "access_mcp_resource" + } + + /** + * Gets the repetition limit for a specific tool + * @param toolName The name of the tool + * @returns The repetition limit for the tool + */ + private getToolLimit(toolName: string): number { + // Check for tool-specific limit + if (this.toolSpecificConfig.toolLimits && toolName in this.toolSpecificConfig.toolLimits) { + return this.toolSpecificConfig.toolLimits[toolName] + } + // Fall back to default limit + return this.consecutiveIdenticalToolCallLimit + } + /** * Checks if a tool use is a browser scroll action * diff --git a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts index 3e156dd7c4..f58bbdac3b 100644 --- a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts +++ b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts @@ -697,4 +697,186 @@ describe("ToolRepetitionDetector", () => { expect(result.askUser).toBeDefined() }) }) + + describe("Progress Detection for MCP Tools", () => { + it("should reset counter when MCP tool shows progress through different responses", () => { + const detector = new ToolRepetitionDetector(3) + + const mcpTool = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test-server", + tool_name: "read_output", + }, + } as ToolUse + + // First call + expect(detector.check(mcpTool).allowExecution).toBe(true) + detector.updateLastResponse("Output chunk 1: data...") + + // Second call with different response - should reset counter + detector.updateLastResponse("Output chunk 2: more data...") + expect(detector.check(mcpTool).allowExecution).toBe(true) + + // Third call with another different response - should still allow + detector.updateLastResponse("Output chunk 3: even more data...") + expect(detector.check(mcpTool).allowExecution).toBe(true) + + // Fourth call with yet another different response - should still allow + detector.updateLastResponse("Output chunk 4: final data...") + expect(detector.check(mcpTool).allowExecution).toBe(true) + }) + + it("should allow higher limits for MCP tools by default", () => { + const detector = new ToolRepetitionDetector(3) // Default limit is 3 + + const mcpTool = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test-server", + tool_name: "read_output", + }, + } as ToolUse + + // MCP tools should have a much higher limit (50 by default) + // Let's test up to 10 calls to verify it's not using the default 3 + for (let i = 1; i <= 10; i++) { + const result = detector.check(mcpTool) + expect(result.allowExecution).toBe(true) // Should allow all 10 calls + } + }) + + it("should block MCP tools when reaching their higher limit without progress", () => { + const detector = new ToolRepetitionDetector(3, { + toolLimits: { + use_mcp_tool: 5, // Override default to a lower number for testing + }, + }) + + const mcpTool = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test-server", + tool_name: "read_output", + }, + } as ToolUse + + // First 5 calls should be allowed + for (let i = 1; i <= 5; i++) { + expect(detector.check(mcpTool).allowExecution).toBe(true) + } + + // 6th call should be blocked + const result = detector.check(mcpTool) + expect(result.allowExecution).toBe(false) + expect(result.askUser?.messageDetail).toContain("use_mcp_tool") + expect(result.askUser?.messageDetail).toContain("false positive") + }) + + it("should exclude tools from repetition detection when configured", () => { + const detector = new ToolRepetitionDetector(2, { + excludedTools: ["execute_command"], + }) + + const excludedTool = { + type: "tool_use", + name: "execute_command", + params: { + command: "ls -la", + }, + } as ToolUse + + // Should always allow execution for excluded tools + for (let i = 1; i <= 10; i++) { + expect(detector.check(excludedTool).allowExecution).toBe(true) + } + }) + + it("should track response history and detect progress patterns", () => { + const detector = new ToolRepetitionDetector(3) + + const tool = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test-server", + }, + } as ToolUse + + // Same response multiple times - no progress + detector.updateLastResponse("Same output") + expect(detector.check(tool).allowExecution).toBe(true) + + detector.updateLastResponse("Same output") + expect(detector.check(tool).allowExecution).toBe(true) + + detector.updateLastResponse("Same output") + expect(detector.check(tool).allowExecution).toBe(true) + + // Now a different response - shows progress + detector.updateLastResponse("Different output") + expect(detector.check(tool).allowExecution).toBe(true) // Counter should reset + }) + + it("should use custom tool limits when provided", () => { + const detector = new ToolRepetitionDetector(3, { + toolLimits: { + read_file: 10, + write_to_file: 1, + }, + }) + + const readTool = { + type: "tool_use", + name: "read_file", + params: { path: "test.txt" }, + } as ToolUse + + const writeTool = { + type: "tool_use", + name: "write_to_file", + params: { path: "test.txt", content: "data" }, + } as ToolUse + + // read_file should allow up to 10 calls + for (let i = 1; i <= 10; i++) { + expect(detector.check(readTool).allowExecution).toBe(true) + } + expect(detector.check(readTool).allowExecution).toBe(false) // 11th call blocked + + // write_to_file should allow only 1 call + expect(detector.check(writeTool).allowExecution).toBe(true) + expect(detector.check(writeTool).allowExecution).toBe(false) // 2nd call blocked + }) + + it("should clear response history when switching tools", () => { + const detector = new ToolRepetitionDetector(3) + + const tool1 = { + type: "tool_use", + name: "use_mcp_tool", + params: { server_name: "server1" }, + } as ToolUse + + const tool2 = { + type: "tool_use", + name: "list_files", + params: { path: "/" }, + } as ToolUse + + // Set up response history for tool1 + detector.updateLastResponse("Response 1") + detector.updateLastResponse("Response 2") + expect(detector.check(tool1).allowExecution).toBe(true) + + // Switch to tool2 - should clear response history + expect(detector.check(tool2).allowExecution).toBe(true) + + // Back to tool1 - should start fresh without previous response history + expect(detector.check(tool1).allowExecution).toBe(true) + }) + }) })