diff --git a/src/api/transform/__tests__/vscode-lm-format.spec.ts b/src/api/transform/__tests__/vscode-lm-format.spec.ts index 1f53cc5751..e60860b549 100644 --- a/src/api/transform/__tests__/vscode-lm-format.spec.ts +++ b/src/api/transform/__tests__/vscode-lm-format.spec.ts @@ -143,9 +143,11 @@ describe("convertToVsCodeLmMessages", () => { expect(result).toHaveLength(1) expect(result[0].role).toBe("assistant") expect(result[0].content).toHaveLength(2) - const [toolCall, textContent] = result[0].content as [MockLanguageModelToolCallPart, MockLanguageModelTextPart] - expect(toolCall.type).toBe("tool_call") + // Text must come before tool calls so that tool calls are at the end, + // properly followed by user message with tool results + const [textContent, toolCall] = result[0].content as [MockLanguageModelTextPart, MockLanguageModelToolCallPart] expect(textContent.type).toBe("text") + expect(toolCall.type).toBe("tool_call") }) it("should handle image blocks with appropriate placeholders", () => { diff --git a/src/api/transform/vscode-lm-format.ts b/src/api/transform/vscode-lm-format.ts index 8e2e955e5c..ee0ef254ad 100644 --- a/src/api/transform/vscode-lm-format.ts +++ b/src/api/transform/vscode-lm-format.ts @@ -114,9 +114,18 @@ export function convertToVsCodeLmMessages( { nonToolMessages: [], toolMessages: [] }, ) - // Process tool messages first then non-tool messages + // Process non-tool messages first, then tool messages + // Tool calls must come at the end so they are properly followed by user message with tool results const contentParts = [ - // Convert tool messages to ToolCallParts first + // Convert non-tool messages to TextParts first + ...nonToolMessages.map((part) => { + if (part.type === "image") { + return new vscode.LanguageModelTextPart("[Image generation not supported by VSCode LM API]") + } + return new vscode.LanguageModelTextPart(part.text) + }), + + // Convert tool messages to ToolCallParts after text ...toolMessages.map( (toolMessage) => new vscode.LanguageModelToolCallPart( @@ -125,14 +134,6 @@ export function convertToVsCodeLmMessages( asObjectSafe(toolMessage.input), ), ), - - // Convert non-tool messages to TextParts after tool messages - ...nonToolMessages.map((part) => { - if (part.type === "image") { - return new vscode.LanguageModelTextPart("[Image generation not supported by VSCode LM API]") - } - return new vscode.LanguageModelTextPart(part.text) - }), ] // Add the assistant message to the list of messages diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index bb64aedb67..d09c8835ad 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -151,7 +151,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}`, @@ -179,6 +182,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", @@ -230,11 +245,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 @@ -519,6 +535,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) => { const editTools = [ "write_to_file", @@ -555,6 +574,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", @@ -573,15 +607,44 @@ export async function presentAssistantMessage(cline: Task) { } } 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) + } } if (editTools.includes(block.name) && block.partial === false) { updateCospecMetadata(cline, block?.params?.path) @@ -635,12 +698,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/prompts/responses.ts b/src/core/prompts/responses.ts index ab5dfd75de..aeb629ac57 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -62,29 +62,18 @@ export const formatResponse = { return `Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.` }, - noToolsUsed: (protocol?: ToolProtocol, preUserContent?: string, preAssistantMessage?: string) => { - return `SYSTEM NOTICE (MUST COMPLY): - -In the previous turn, no tool was called. -This violates a system rule: EVERY assistant turn MUST include at least one tool call. - -In this turn: -- You MUST call one appropriate tool. -- Do NOT explain or justify the previous response. -- Do NOT repeat previous content. -- Do NOT respond conversationally. -- If you have completed the user's task, use the attempt_completion tool.` - // const instructions = getToolInstructionsReminder(protocol) + noToolsUsed: (protocol?: ToolProtocol) => { + const instructions = getToolInstructionsReminder(protocol) - // return `[ERROR] You did not use a tool in your previous response! Please retry with a tool use. - // ${instructions} + return `[ERROR] You did not use a tool in your previous response! Please retry with a tool use. +${instructions} - // # Next Steps +# Next Steps - // If you have completed the user's task, use the attempt_completion tool. - // If you require additional information from the user, use the ask_followup_question tool. - // Otherwise, if you have not completed the task and do not need additional information, then proceed with the next step of the task. - // (This is an automated message, so do not respond to it conversationally.)` +If you have completed the user's task, use the attempt_completion tool. +If you require additional information from the user, use the ask_followup_question tool. +Otherwise, if you have not completed the task and do not need additional information, then proceed with the next step of the task. +(This is an automated message, so do not respond to it conversationally.)` }, tooManyMistakes: (feedback?: string, protocol?: ToolProtocol) => { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 78f7f31117..444b0354c2 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2553,20 +2553,15 @@ export class Task extends EventEmitter implements TaskLike { // the user hits max requests and denies resetting the count. break } else { - const _nextUserContent = findLast( - nextUserContent, - (block) => block.type === "text" && typeof block.text === "string", - ) as { type: string; text: string } - // Use the task's locked protocol, NOT the current settings (fallback to xml if not set) const content = { type: "text", - text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml", _nextUserContent?.text), + text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"), } as Anthropic.Messages.ContentBlockParam & { __isNoToolsUsed?: boolean } Object.defineProperty(content, "__isNoToolsUsed", { value: true, - enumerable: false, // 不可枚举，JSON 序列化时会被忽略 + enumerable: false, writable: false, configurable: false, }) @@ -3436,6 +3431,9 @@ export class Task extends EventEmitter implements TaskLike { : await this.convertErrorMessage(error, () => { shouldStop = true }) + if (streamingFailedMessage) { + streamingFailedMessage = `${t("common:interruption.streamTerminatedByProvider")}: ${streamingFailedMessage}` + } // Clean up partial state await abortStream(cancelReason, streamingFailedMessage) @@ -3693,12 +3691,7 @@ export class Task extends EventEmitter implements TaskLike { const didToolUse = this.assistantMessageContent.some( (block) => block.type === "tool_use" || block.type === "mcp_tool_use", ) - const preAssistantMessage = - this.assistantMessageContent[0] && - ["text", "reasoning"].includes(this.assistantMessageContent[0].type) - ? (this.assistantMessageContent[0] as any).content || - (this.assistantMessageContent[0] as any).text - : undefined + if (!didToolUse) { // Increment consecutive no-tool-use counter this.consecutiveNoToolUseCount++ @@ -3713,18 +3706,14 @@ export class Task extends EventEmitter implements TaskLike { // Use the task's locked protocol for consistent behavior const _content = { type: "text", - text: formatResponse.noToolsUsed( - this._taskToolProtocol ?? "xml", - undefined, - preAssistantMessage, - ), + text: formatResponse.noToolsUsed(this._taskToolProtocol ?? "xml"), } as (Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolResultBlockParam) & { __isNoToolsUsed?: boolean } Object.defineProperty(_content, "__isNoToolsUsed", { value: true, - enumerable: false, // 不可枚举，JSON 序列化时会被忽略 + enumerable: false, writable: false, configurable: false, }) @@ -5242,7 +5231,7 @@ export class Task extends EventEmitter implements TaskLike { pauseHandler?.() } } else { - errorMsg = error.message + errorMsg = error.message ?? JSON.stringify(serializeError(error), null, 2) } return errorMsg 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") { diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index bd69961944..6b670e51ff 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -187,7 +187,8 @@ }, "interruption": { "responseInterruptedByUser": "Response interrupted by user", - "responseInterruptedByApiError": "Response interrupted by API error" + "responseInterruptedByApiError": "Response interrupted by API error", + "streamTerminatedByProvider": "Provider ended the request" }, "storage": { "prompt_custom_path": "Enter custom conversation history storage path, leave empty to use default location", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index b2c024db83..8ce36c4279 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -192,7 +192,8 @@ }, "interruption": { "responseInterruptedByUser": "响应被用户中断", - "responseInterruptedByApiError": "响应被 API 错误中断" + "responseInterruptedByApiError": "响应被 API 错误中断", + "streamTerminatedByProvider": "提供方终止了请求" }, "storage": { "prompt_custom_path": "输入自定义会话历史存储路径，留空以使用默认位置", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 5474f8fa9b..deb96c0a26 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -187,7 +187,8 @@ }, "interruption": { "responseInterruptedByUser": "回應被使用者中斷", - "responseInterruptedByApiError": "回應被 API 錯誤中斷" + "responseInterruptedByApiError": "回應被 API 錯誤中斷", + "streamTerminatedByProvider": "提供方終止了請求" }, "storage": { "prompt_custom_path": "輸入自訂會話歷史儲存路徑，留空以使用預設位置",