Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrubens merged 1 commit into from
Jan 9, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 73 additions & 10 deletions src/core/assistant-message/presentAssistantMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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)
}
}
}

Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/core/task/validateToolResultIds.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
Loading