From a57e59bde589cd3526a08fd7ebf2f181dda82060 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 20 Nov 2025 21:36:26 -0500 Subject: [PATCH] fix: send tool_result blocks for skipped tools in native protocol When using native tool calling, if multiple tools are invoked in a single message but only one can be executed, the skipped tools were sending error messages as 'text' blocks instead of 'tool_result' blocks. This violated the native protocol requirement that every tool_use must have a corresponding tool_result with the same tool_use_id, causing the API to throw 400 errors. Changes: - Modified presentAssistantMessage.ts to detect protocol by checking for tool ID - For native protocol: send proper tool_result blocks with is_error: true - For XML protocol: preserve existing text block behavior - Added comprehensive test coverage for multiple tool scenarios Fixes the 400 error when LLM invokes multiple tools in native protocol. --- .../presentAssistantMessage-images.spec.ts | 155 ++++++++++++++++++ .../presentAssistantMessage.ts | 44 +++-- 2 files changed, 189 insertions(+), 10 deletions(-) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts index b78f184251f..39d71bc88b7 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts @@ -202,4 +202,159 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calls", () => // Should have fallback text expect(toolResult.content).toBeTruthy() }) + + describe("Multiple tool calls handling", () => { + it("should send tool_result with is_error for skipped tools in native protocol when didRejectTool is true", async () => { + // Simulate multiple tool calls with native protocol (all have IDs) + const toolCallId1 = "tool_call_001" + const toolCallId2 = "tool_call_002" + + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId1, + name: "read_file", + params: { path: "test.txt" }, + }, + { + type: "tool_use", + id: toolCallId2, + name: "write_to_file", + params: { path: "output.txt", content: "test" }, + }, + ] + + // First tool is rejected + mockTask.didRejectTool = true + + // Process the second tool (should be skipped) + mockTask.currentStreamingContentIndex = 1 + await presentAssistantMessage(mockTask) + + // Find the tool_result for the second tool + const toolResult = mockTask.userMessageContent.find( + (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId2, + ) + + // Verify that a tool_result block was created (not a text block) + expect(toolResult).toBeDefined() + expect(toolResult.tool_use_id).toBe(toolCallId2) + expect(toolResult.is_error).toBe(true) + expect(toolResult.content).toContain("due to user rejecting a previous tool") + + // Ensure no text blocks were added for this rejection + const textBlocks = mockTask.userMessageContent.filter( + (item: any) => item.type === "text" && item.text.includes("due to user rejecting"), + ) + expect(textBlocks.length).toBe(0) + }) + + it("should send tool_result with is_error for skipped tools in native protocol when didAlreadyUseTool is true", async () => { + // Simulate multiple tool calls with native protocol + const toolCallId1 = "tool_call_003" + const toolCallId2 = "tool_call_004" + + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId1, + name: "read_file", + params: { path: "test.txt" }, + }, + { + type: "tool_use", + id: toolCallId2, + name: "write_to_file", + params: { path: "output.txt", content: "test" }, + }, + ] + + // First tool was already used + mockTask.didAlreadyUseTool = true + + // Process the second tool (should be skipped) + mockTask.currentStreamingContentIndex = 1 + await presentAssistantMessage(mockTask) + + // Find the tool_result for the second tool + const toolResult = mockTask.userMessageContent.find( + (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId2, + ) + + // Verify that a tool_result block was created (not a text block) + expect(toolResult).toBeDefined() + expect(toolResult.tool_use_id).toBe(toolCallId2) + expect(toolResult.is_error).toBe(true) + expect(toolResult.content).toContain("was not executed because a tool has already been used") + + // Ensure no text blocks were added for this rejection + const textBlocks = mockTask.userMessageContent.filter( + (item: any) => item.type === "text" && item.text.includes("was not executed because"), + ) + expect(textBlocks.length).toBe(0) + }) + + it("should send text blocks for skipped tools in XML protocol (no tool IDs)", async () => { + // Simulate multiple tool calls with XML protocol (no IDs) + mockTask.assistantMessageContent = [ + { + type: "tool_use", + // No ID = XML protocol + name: "read_file", + params: { path: "test.txt" }, + }, + { + type: "tool_use", + // No ID = XML protocol + name: "write_to_file", + params: { path: "output.txt", content: "test" }, + }, + ] + + // First tool is rejected + mockTask.didRejectTool = true + + // Process the second tool (should be skipped) + mockTask.currentStreamingContentIndex = 1 + await presentAssistantMessage(mockTask) + + // For XML protocol, should add text block (not tool_result) + const textBlocks = mockTask.userMessageContent.filter( + (item: any) => item.type === "text" && item.text.includes("due to user rejecting"), + ) + expect(textBlocks.length).toBeGreaterThan(0) + + // Ensure no tool_result blocks were added + const toolResults = mockTask.userMessageContent.filter((item: any) => item.type === "tool_result") + expect(toolResults.length).toBe(0) + }) + + it("should handle partial tool blocks when didRejectTool is true in native protocol", async () => { + const toolCallId = "tool_call_005" + + mockTask.assistantMessageContent = [ + { + type: "tool_use", + id: toolCallId, + name: "write_to_file", + params: { path: "output.txt", content: "test" }, + partial: true, // Partial tool block + }, + ] + + mockTask.didRejectTool = true + + await presentAssistantMessage(mockTask) + + // Find the tool_result + const toolResult = mockTask.userMessageContent.find( + (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId, + ) + + // Verify tool_result was created for partial block + expect(toolResult).toBeDefined() + expect(toolResult.is_error).toBe(true) + expect(toolResult.content).toContain("was interrupted and not executed") + }) + }) }) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 0955d5d111f..72716e1ef10 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -252,16 +252,25 @@ export async function presentAssistantMessage(cline: Task) { if (cline.didRejectTool) { // Ignore any tool content after user has rejected tool once. - if (!block.partial) { + // For native protocol, we must send a tool_result for every tool_use to avoid API errors + const toolCallId = block.id + const errorMessage = !block.partial + ? `Skipping tool ${toolDescription()} due to user rejecting a previous tool.` + : `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.` + + if (toolCallId) { + // Native protocol: MUST send tool_result for every tool_use cline.userMessageContent.push({ - type: "text", - text: `Skipping tool ${toolDescription()} due to user rejecting a previous tool.`, - }) + type: "tool_result", + tool_use_id: toolCallId, + content: errorMessage, + is_error: true, + } as Anthropic.ToolResultBlockParam) } else { - // Partial tool after user rejected a previous tool. + // XML protocol: send as text cline.userMessageContent.push({ type: "text", - text: `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.`, + text: errorMessage, }) } @@ -270,10 +279,25 @@ export async function presentAssistantMessage(cline: Task) { if (cline.didAlreadyUseTool) { // Ignore any content after a tool has already been used. - cline.userMessageContent.push({ - type: "text", - text: `Tool [${block.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message. You must assess the first tool's result before proceeding to use the next tool.`, - }) + // For native protocol, we must send a tool_result for every tool_use to avoid API errors + const toolCallId = block.id + const errorMessage = `Tool [${block.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message. You must assess the first tool's result before proceeding to use the next tool.` + + if (toolCallId) { + // Native protocol: MUST send tool_result for every tool_use + cline.userMessageContent.push({ + type: "tool_result", + tool_use_id: toolCallId, + content: errorMessage, + is_error: true, + } as Anthropic.ToolResultBlockParam) + } else { + // XML protocol: send as text + cline.userMessageContent.push({ + type: "text", + text: errorMessage, + }) + } break }