diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts index 6ad8c58282c..e90646fd9a4 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts @@ -77,6 +77,18 @@ describe("presentAssistantMessage - Custom Tool Recording", () => { say: vi.fn().mockResolvedValue(undefined), ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), } + + // Add pushToolResultToUserContent method after mockTask is created so it can reference mockTask + mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: any) => { + const existingResult = mockTask.userMessageContent.find( + (block: any) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }) }) describe("Custom tool usage recording", () => { diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts index 39d71bc88b7..72ee4306097 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts @@ -60,6 +60,18 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calls", () => say: vi.fn().mockResolvedValue(undefined), ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), } + + // Add pushToolResultToUserContent method after mockTask is created so it can reference mockTask + mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: any) => { + const existingResult = mockTask.userMessageContent.find( + (block: any) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }) }) it("should preserve images in tool_result for native protocol", async () => { diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts index 2c71dc78113..d4ae2764a05 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts @@ -59,6 +59,18 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { say: vi.fn().mockResolvedValue(undefined), ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), } + + // Add pushToolResultToUserContent method after mockTask is created so 'this' binds correctly + mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: any) => { + const existingResult = mockTask.userMessageContent.find( + (block: any) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }) }) it("should return error for unknown tool in native protocol", async () => { diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 9b1a5fcffb1..28cb038d3ee 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -115,12 +115,12 @@ export async function presentAssistantMessage(cline: Task) { : `MCP tool ${mcpBlock.name} was interrupted and not executed due to user rejecting a previous tool.` if (toolCallId) { - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: errorMessage, is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } break } @@ -130,12 +130,12 @@ export async function presentAssistantMessage(cline: Task) { const errorMessage = `MCP tool [${mcpBlock.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message.` if (toolCallId) { - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: errorMessage, is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } break } @@ -167,11 +167,11 @@ export async function presentAssistantMessage(cline: Task) { } if (toolCallId) { - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: resultContent, - } as Anthropic.ToolResultBlockParam) + }) if (imageBlocks.length > 0) { cline.userMessageContent.push(...imageBlocks) @@ -446,12 +446,12 @@ export async function presentAssistantMessage(cline: Task) { if (toolCallId) { // Native protocol: MUST send tool_result for every tool_use - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: errorMessage, is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } else { // XML protocol: send as text cline.userMessageContent.push({ @@ -471,12 +471,12 @@ export async function presentAssistantMessage(cline: Task) { if (toolCallId) { // Native protocol: MUST send tool_result for every tool_use - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: errorMessage, is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } else { // XML protocol: send as text cline.userMessageContent.push({ @@ -530,11 +530,11 @@ export async function presentAssistantMessage(cline: Task) { } // Add tool_result with text content only - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: resultContent, - } as Anthropic.ToolResultBlockParam) + }) // Add image blocks separately after tool_result if (imageBlocks.length > 0) { @@ -735,12 +735,12 @@ export async function presentAssistantMessage(cline: Task) { if (toolProtocol === TOOL_PROTOCOL.NATIVE && toolCallId) { // For native protocol, push tool_result directly without setting didAlreadyUseTool - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: typeof errorContent === "string" ? errorContent : "(validation error)", is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } else { // For XML protocol, use the standard pushToolResult pushToolResult(errorContent) @@ -1110,12 +1110,12 @@ export async function presentAssistantMessage(cline: Task) { // Push tool_result directly for native protocol WITHOUT setting didAlreadyUseTool // This prevents the stream from being interrupted with "Response interrupted by tool use result" if (toolProtocol === TOOL_PROTOCOL.NATIVE && toolCallId) { - cline.userMessageContent.push({ + cline.pushToolResultToUserContent({ type: "tool_result", tool_use_id: toolCallId, content: formatResponse.toolError(errorMessage, toolProtocol), is_error: true, - } as Anthropic.ToolResultBlockParam) + }) } else { pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 33e8245ccc6..3340ae5d6ca 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -337,6 +337,28 @@ export class Task extends EventEmitter implements TaskLike { presentAssistantMessageHasPendingUpdates = false userMessageContent: (Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolResultBlockParam)[] = [] userMessageContentReady = false + + /** + * Push a tool_result block to userMessageContent, preventing duplicates. + * This is critical for native tool protocol where duplicate tool_use_ids cause API errors. + * + * @param toolResult - The tool_result block to add + * @returns true if added, false if duplicate was skipped + */ + public pushToolResultToUserContent(toolResult: Anthropic.ToolResultBlockParam): boolean { + const existingResult = this.userMessageContent.find( + (block): block is Anthropic.ToolResultBlockParam => + block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + console.warn( + `[Task#pushToolResultToUserContent] Skipping duplicate tool_result for tool_use_id: ${toolResult.tool_use_id}`, + ) + return false + } + this.userMessageContent.push(toolResult) + return true + } didRejectTool = false didAlreadyUseTool = false didToolFailInCurrentTurn = false diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index c3a88b3e298..5b7346d49da 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1977,3 +1977,205 @@ describe("Queued message processing after condense", () => { expect(taskB.messageQueueService.isEmpty()).toBe(true) }) }) + +describe("pushToolResultToUserContent", () => { + let mockProvider: any + let mockApiConfig: ProviderSettings + + beforeEach(() => { + mockApiConfig = { + apiProvider: "anthropic", + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + + const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") } + const mockExtensionContext = { + globalState: { + get: vi.fn().mockImplementation((_key: keyof GlobalState) => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + globalStorageUri: storageUri, + workspaceState: { + get: vi.fn().mockImplementation((_key) => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }, + extensionUri: { fsPath: "/mock/extension/path" }, + extension: { packageJSON: { version: "1.0.0" } }, + } as unknown as vscode.ExtensionContext + + const mockOutputChannel = { + name: "test-output", + appendLine: vi.fn(), + append: vi.fn(), + replace: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + } + + mockProvider = new ClineProvider( + mockExtensionContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockExtensionContext), + ) as any + + mockProvider.postMessageToWebview = vi.fn().mockResolvedValue(undefined) + mockProvider.postStateToWebview = vi.fn().mockResolvedValue(undefined) + }) + + it("should add tool_result when not a duplicate", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const toolResult: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "test-id-1", + content: "Test result", + } + + const added = task.pushToolResultToUserContent(toolResult) + + expect(added).toBe(true) + expect(task.userMessageContent).toHaveLength(1) + expect(task.userMessageContent[0]).toEqual(toolResult) + }) + + it("should prevent duplicate tool_result with same tool_use_id", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const toolResult1: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "duplicate-id", + content: "First result", + } + + const toolResult2: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "duplicate-id", + content: "Second result (should be skipped)", + } + + // Spy on console.warn to verify warning is logged + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + // Add first result - should succeed + const added1 = task.pushToolResultToUserContent(toolResult1) + expect(added1).toBe(true) + expect(task.userMessageContent).toHaveLength(1) + + // Add second result with same ID - should be skipped + const added2 = task.pushToolResultToUserContent(toolResult2) + expect(added2).toBe(false) + expect(task.userMessageContent).toHaveLength(1) + + // Verify only the first result is in the array + expect(task.userMessageContent[0]).toEqual(toolResult1) + + // Verify warning was logged + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("Skipping duplicate tool_result for tool_use_id: duplicate-id"), + ) + + warnSpy.mockRestore() + }) + + it("should allow different tool_use_ids to be added", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const toolResult1: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "id-1", + content: "Result 1", + } + + const toolResult2: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "id-2", + content: "Result 2", + } + + const added1 = task.pushToolResultToUserContent(toolResult1) + const added2 = task.pushToolResultToUserContent(toolResult2) + + expect(added1).toBe(true) + expect(added2).toBe(true) + expect(task.userMessageContent).toHaveLength(2) + expect(task.userMessageContent[0]).toEqual(toolResult1) + expect(task.userMessageContent[1]).toEqual(toolResult2) + }) + + it("should handle tool_result with is_error flag", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + const errorResult: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "error-id", + content: "Error message", + is_error: true, + } + + const added = task.pushToolResultToUserContent(errorResult) + + expect(added).toBe(true) + expect(task.userMessageContent).toHaveLength(1) + expect(task.userMessageContent[0]).toEqual(errorResult) + }) + + it("should not interfere with other content types in userMessageContent", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Add text and image blocks manually + task.userMessageContent.push( + { type: "text", text: "Some text" }, + { type: "image", source: { type: "base64", media_type: "image/png", data: "base64data" } }, + ) + + const toolResult: Anthropic.ToolResultBlockParam = { + type: "tool_result", + tool_use_id: "test-id", + content: "Result", + } + + const added = task.pushToolResultToUserContent(toolResult) + + expect(added).toBe(true) + expect(task.userMessageContent).toHaveLength(3) + expect(task.userMessageContent[0].type).toBe("text") + expect(task.userMessageContent[1].type).toBe("image") + expect(task.userMessageContent[2]).toEqual(toolResult) + }) +})