diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 28cb038d3e..77f5b600cb 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -145,7 +145,10 @@ export async function presentAssistantMessage(cline: Task) { const toolCallId = mcpBlock.id const toolProtocol = TOOL_PROTOCOL.NATIVE // MCP tools in native mode always use native protocol - const pushToolResult = (content: ToolResponse) => { + // Store approval feedback to merge into tool result (GitHub #10465) + let approvalFeedback: { text: string; images?: string[] } | undefined + + const pushToolResult = (content: ToolResponse, feedbackImages?: string[]) => { if (hasToolResult) { console.warn( `[presentAssistantMessage] Skipping duplicate tool_result for mcp_tool_use: ${toolCallId}`, @@ -166,6 +169,18 @@ export async function presentAssistantMessage(cline: Task) { "(tool did not return anything)" } + // Merge approval feedback into tool result (GitHub #10465) + if (approvalFeedback) { + const feedbackText = formatResponse.toolApprovedWithFeedback(approvalFeedback.text, toolProtocol) + resultContent = `${feedbackText}\n\n${resultContent}` + + // Add feedback images to the image blocks + if (approvalFeedback.images) { + const feedbackImageBlocks = formatResponse.imageBlocks(approvalFeedback.images) + imageBlocks = [...feedbackImageBlocks, ...imageBlocks] + } + } + if (toolCallId) { cline.pushToolResultToUserContent({ type: "tool_result", @@ -214,11 +229,12 @@ export async function presentAssistantMessage(cline: Task) { return false } + // Store approval feedback to be merged into tool result (GitHub #10465) + // Don't push it as a separate tool_result here - that would create duplicates. + // The tool will call pushToolResult, which will merge the feedback into the actual result. if (text) { await cline.say("user_feedback", text, images) - pushToolResult( - formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text, toolProtocol), images), - ) + approvalFeedback = { text, images } } return true @@ -501,6 +517,9 @@ export async function presentAssistantMessage(cline: Task) { // Previously resolved from experiments.isEnabled(..., EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS) const isMultipleNativeToolCallsEnabled = false + // Store approval feedback to merge into tool result (GitHub #10465) + let approvalFeedback: { text: string; images?: string[] } | undefined + const pushToolResult = (content: ToolResponse) => { if (toolProtocol === TOOL_PROTOCOL.NATIVE) { // For native protocol, only allow ONE tool_result per tool call @@ -529,6 +548,21 @@ export async function presentAssistantMessage(cline: Task) { "(tool did not return anything)" } + // Merge approval feedback into tool result (GitHub #10465) + if (approvalFeedback) { + const feedbackText = formatResponse.toolApprovedWithFeedback( + approvalFeedback.text, + toolProtocol, + ) + resultContent = `${feedbackText}\n\n${resultContent}` + + // Add feedback images to the image blocks + if (approvalFeedback.images) { + const feedbackImageBlocks = formatResponse.imageBlocks(approvalFeedback.images) + imageBlocks = [...feedbackImageBlocks, ...imageBlocks] + } + } + // Add tool_result with text content only cline.pushToolResultToUserContent({ type: "tool_result", @@ -544,15 +578,44 @@ export async function presentAssistantMessage(cline: Task) { hasToolResult = true } else { // For XML protocol, add as text blocks (legacy behavior) + let resultContent: string + + if (typeof content === "string") { + resultContent = content || "(tool did not return anything)" + } else { + const textBlocks = content.filter((item) => item.type === "text") + resultContent = + textBlocks.map((item) => (item as Anthropic.TextBlockParam).text).join("\n") || + "(tool did not return anything)" + } + + // Merge approval feedback into tool result (GitHub #10465) + if (approvalFeedback) { + const feedbackText = formatResponse.toolApprovedWithFeedback( + approvalFeedback.text, + toolProtocol, + ) + resultContent = `${feedbackText}\n\n${resultContent}` + } + cline.userMessageContent.push({ type: "text", text: `${toolDescription()} Result:` }) if (typeof content === "string") { cline.userMessageContent.push({ type: "text", - text: content || "(tool did not return anything)", + text: resultContent, }) } else { - cline.userMessageContent.push(...content) + // Add text content with merged feedback + cline.userMessageContent.push({ + type: "text", + text: resultContent, + }) + // Add any images from the tool result + const imageBlocks = content.filter((item) => item.type === "image") + if (imageBlocks.length > 0) { + cline.userMessageContent.push(...imageBlocks) + } } } @@ -603,12 +666,12 @@ export async function presentAssistantMessage(cline: Task) { return false } - // Handle yesButtonClicked with text. + // Store approval feedback to be merged into tool result (GitHub #10465) + // Don't push it as a separate tool_result here - that would create duplicates. + // The tool will call pushToolResult, which will merge the feedback into the actual result. if (text) { await cline.say("user_feedback", text, images) - pushToolResult( - formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text, toolProtocol), images), - ) + approvalFeedback = { text, images } } return true diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index 9dd73723a3..a966d429ed 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -83,8 +83,10 @@ export function validateAndFixToolResultIds( ) // Deduplicate tool_result blocks to prevent API protocol violations (GitHub #10465) - // Terminal fallback race conditions can generate duplicate tool_results with the same tool_use_id. - // Filter out duplicates before validation since Set-based checks below would miss them. + // This serves as a safety net for any potential race conditions that could generate + // duplicate tool_results with the same tool_use_id. The root cause (approval feedback + // creating duplicate results) has been fixed in presentAssistantMessage.ts, but this + // deduplication remains as a defensive measure for unknown edge cases. const seenToolResultIds = new Set() const deduplicatedContent = userMessage.content.filter((block) => { if (block.type !== "tool_result") {