fix: merge approval feedback into tool result instead of pushing duplicate (ROO-410)

When users approve tool execution with feedback text, the feedback was being
pushed as a separate tool_result before the actual tool executed and pushed
its own result. This created duplicate tool_results with the same tool_use_id,
violating the Anthropic API protocol (GitHub #10465).

Changes:
- Add approvalFeedback storage to hold feedback until tool completes
- Modify pushToolResult to merge stored feedback into the tool's actual result
- Update askApproval to store feedback instead of pushing as separate result
- Apply fix to both MCP tools and regular tools, native and XML protocols
- Update validateToolResultIds comments to clarify deduplication is now safety net

Author: daniel-lxs

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

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<string>()
 	const deduplicatedContent = userMessage.content.filter((block) => {
 		if (block.type !== "tool_result") {