fix(tools): process queued messages when terminal command finishes

packages/types/src/task.ts

@@ -127,7 +127,7 @@ export interface TaskLike {
 
 	approveAsk(options?: { text?: string; images?: string[] }): void
 	denyAsk(options?: { text?: string; images?: string[] }): void
-	submitUserMessage(text: string, images?: string[], mode?: string, providerProfile?: string): Promise<void>
+	submitUserMessage(text: string, images?: string[], mode?: string, providerProfile?: string): Promise<boolean>
 	abortTask(): Promise<void>
 }
 

src/core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts

@@ -81,6 +81,7 @@ interface MockTask {
 		}
 	}
 	say: ReturnType<typeof vi.fn>
+	sayUserFeedbackAndAckQueued: ReturnType<typeof vi.fn>
 	ask: ReturnType<typeof vi.fn>
 	pushToolResultToUserContent: ReturnType<typeof vi.fn>
 	getTaskMode: ReturnType<typeof vi.fn>
@@ -119,6 +120,8 @@ function buildMockTask(): MockTask {
 			}),
 		},
 		say: vi.fn().mockResolvedValue(undefined),
+		// The merged askApproval routes feedback through the durable queued ack.
+		sayUserFeedbackAndAckQueued: vi.fn().mockResolvedValue(undefined),
 		ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
 		pushToolResultToUserContent: vi.fn(),
 		getTaskMode: vi.fn().mockResolvedValue("code"),

src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts

@@ -2,6 +2,7 @@
 
 import type { Anthropic } from "@anthropic-ai/sdk"
 import { describe, it, expect, beforeEach, vi, type Mock } from "vitest"
+import { providerIdentifiers } from "@roo-code/types"
 import { presentAssistantMessage } from "../presentAssistantMessage"
 import { validateToolUse } from "../../tools/validateToolUse"
 import { getModeBySlug } from "../../../shared/modes"
@@ -39,6 +40,7 @@ vi.mock("@roo-code/telemetry", () => ({
 		instance: {
 			captureToolUsage: vi.fn(),
 			captureConsecutiveMistakeError: vi.fn(),
+			captureException: vi.fn(),
 			captureEvent: vi.fn(),
 		},
 	},
@@ -64,6 +66,7 @@ interface MockTask {
 	api: { getModel: () => { id: string; info: Record<string, unknown> } }
 	recordToolUsage: ReturnType<typeof vi.fn>
 	recordToolError: ReturnType<typeof vi.fn>
+	apiConfiguration?: { apiProvider: string }
 	toolRepetitionDetector: { check: ReturnType<typeof vi.fn> }
 	providerRef: {
 		deref: () =>
@@ -74,6 +77,7 @@ interface MockTask {
 			| undefined
 	}
 	say: ReturnType<typeof vi.fn>
+	sayUserFeedbackAndAckQueued: ReturnType<typeof vi.fn>
 	ask: ReturnType<typeof vi.fn>
 	pushToolResultToUserContent: ReturnType<typeof vi.fn>
 }
@@ -83,6 +87,10 @@ describe("presentAssistantMessage - tool usage attribution", () => {
 
 	beforeEach(() => {
 		vi.clearAllMocks()
+		// clearAllMocks keeps queued one-shot implementations alive; tests that skip the
+		// validation arm would otherwise leak a mockImplementationOnce throw into a
+		// later test that runs the validated arm.
+		vi.mocked(validateToolUse).mockReset()
 		vi.mocked(validateToolUse).mockImplementation(() => undefined)
 
 		mockTask = {
@@ -117,6 +125,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
 				}),
 			},
 			say: vi.fn().mockResolvedValue(undefined),
+			sayUserFeedbackAndAckQueued: vi.fn().mockResolvedValue(undefined),
 			ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
 			pushToolResultToUserContent: vi.fn(),
 		}
@@ -651,6 +660,113 @@ describe("presentAssistantMessage - tool usage attribution", () => {
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
 			expect(mockTask.didAlreadyUseTool).toBe(false)
 		})
+
+		it("routes MCP approval feedback through the queued-ack wrapper", async () => {
+			mockTask.providerRef = {
+				deref: () => ({
+					getState: vi.fn().mockResolvedValue({
+						mode: "code",
+						customModes: [],
+					}),
+					getMcpHub: () => ({
+						findServerNameBySanitizedName: () => "my_server",
+						getAllServers: () => [
+							{
+								name: "my_server",
+								tools: [{ name: "do_thing", enabledForPrompt: true }],
+							},
+						],
+					}),
+				}),
+			}
+			mockTask.assistantMessageContent = [
+				{
+					type: "mcp_tool_use",
+					id: "call_native_mcp_feedback",
+					name: "mcp_my_server_do_thing",
+					serverName: "my_server",
+					toolName: "do_thing",
+					arguments: {},
+					partial: false,
+				},
+			]
+			mockTask.ask = vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: "Careful with this server" })
+
+			await presentAssistantMessage(mockTask as unknown as Task)
+
+			expect(mockTask.sayUserFeedbackAndAckQueued).toHaveBeenCalledExactlyOnceWith(
+				"Careful with this server",
+				undefined,
+				undefined,
+			)
+			expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything())
+		})
+
+		it("routes tool-repetition feedback through the queued-ack wrapper", async () => {
+			mockTask.toolRepetitionDetector.check = vi.fn().mockReturnValue({
+				allowExecution: false,
+				askUser: {
+					messageKey: "mistake_limit_reached",
+					messageDetail: "The tool {toolName} was called consecutively without progress.",
+				},
+			})
+			mockTask.apiConfiguration = { apiProvider: providerIdentifiers.anthropic }
+			mockTask.assistantMessageContent = [
+				{
+					type: "tool_use",
+					id: "call_repetition_feedback",
+					name: "read_file",
+					params: { path: "a.txt" },
+					nativeArgs: { path: "a.txt" },
+					partial: false,
+				},
+			]
+			mockTask.ask = vi.fn().mockResolvedValue({ response: "messageResponse", text: "Try another approach" })
+
+			await presentAssistantMessage(mockTask as unknown as Task)
+
+			expect(mockTask.sayUserFeedbackAndAckQueued).toHaveBeenCalledExactlyOnceWith(
+				"Try another approach",
+				undefined,
+				undefined,
+			)
+			expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything())
+			expect(mockTask.userMessageContent).toContainEqual(
+				expect.objectContaining({
+					type: "text",
+					text: expect.stringContaining("Try another approach"),
+				}),
+			)
+		})
+
+		it("acks tool-repetition feedback before it reaches the API turn", async () => {
+			mockTask.toolRepetitionDetector.check = vi.fn().mockReturnValue({
+				allowExecution: false,
+				askUser: {
+					messageKey: "mistake_limit_reached",
+					messageDetail: "The tool {toolName} was called consecutively without progress.",
+				},
+			})
+			mockTask.apiConfiguration = { apiProvider: providerIdentifiers.anthropic }
+			mockTask.assistantMessageContent = [
+				{
+					type: "tool_use",
+					id: "call_repetition_feedback",
+					name: "read_file",
+					params: { path: "a.txt" },
+					nativeArgs: { path: "a.txt" },
+					partial: false,
+				},
+			]
+			mockTask.ask = vi.fn().mockResolvedValue({ response: "messageResponse", text: "Try another approach" })
+			mockTask.sayUserFeedbackAndAckQueued = vi.fn().mockRejectedValue(new Error("persist failed"))
+
+			await expect(presentAssistantMessage(mockTask as unknown as Task)).rejects.toThrow("persist failed")
+
+			// The durable ack failed and the entry is re-queued for redelivery:
+			// the feedback must not already be part of the API turn.
+			expect(mockTask.userMessageContent).toHaveLength(0)
+		})
 	})
 
 	describe("undefined provider state", () => {

src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts

@@ -62,6 +62,7 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
 				}),
 			},
 			say: vi.fn().mockResolvedValue(undefined),
+			sayUserFeedbackAndAckQueued: vi.fn().mockResolvedValue(undefined),
 			ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
 		}
 

src/core/assistant-message/presentAssistantMessage.ts

@@ -218,7 +218,7 @@ export async function presentAssistantMessage(cline: Task) {
 				isProtected?: boolean,
 				autoApprovalContext?: AutoApprovalContext,
 			) => {
-				const { response, text, images, autoDenyDetail } = await cline.ask(
+				const { response, text, images, queuedMessageId, autoDenyDetail } = await cline.ask(
 					type,
 					partialMessage,
 					false,
@@ -250,8 +250,8 @@ export async function presentAssistantMessage(cline: Task) {
 						return false
 					}
 
+					await cline.sayUserFeedbackAndAckQueued(text, images, queuedMessageId)
 					if (text) {
-						await cline.say("user_feedback", text, images)
 						pushToolResult(formatResponse.toolResult(formatResponse.toolDeniedWithFeedback(text), images))
 					} else {
 						pushToolResult(formatResponse.toolDenied())
@@ -263,8 +263,8 @@ export async function presentAssistantMessage(cline: Task) {
 				// 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.
+				await cline.sayUserFeedbackAndAckQueued(text, images, queuedMessageId)
 				if (text) {
-					await cline.say("user_feedback", text, images)
 					approvalFeedback = { text, images }
 				}
 
@@ -787,12 +787,18 @@ export async function presentAssistantMessage(cline: Task) {
 				// If execution is not allowed, notify user and break.
 				if (!repetitionCheck.allowExecution && repetitionCheck.askUser) {
 					// Handle repetition similar to mistake_limit_reached pattern.
-					const { response, text, images } = await cline.ask(
+					const { response, text, images, queuedMessageId } = await cline.ask(
 						repetitionCheck.askUser.messageKey as ClineAsk,
 						repetitionCheck.askUser.messageDetail.replace("{toolName}", block.name),
 					)
 
 					if (response === "messageResponse") {
+						// Durable ack first: the feedback must reach history before
+						// the API turn. When the ack fails the entry is re-queued
+						// for redelivery, and the model must not already have seen
+						// the text (mirrors AttemptCompletionTool's ordering).
+						await cline.sayUserFeedbackAndAckQueued(text, images, queuedMessageId)
+
 						// Add user feedback to userContent.
 						cline.userMessageContent.push(
 							{
@@ -801,9 +807,6 @@ export async function presentAssistantMessage(cline: Task) {
 							},
 							...formatResponse.imageBlocks(images),
 						)
-
-						// Add user feedback to chat.
-						await cline.say("user_feedback", text, images)
 					}
 
 					// Track tool repetition in telemetry via PostHog exception tracking and event.

src/core/message-queue/MessageQueueService.ts

@@ -113,6 +113,23 @@ export class MessageQueueService extends EventEmitter<QueueEvents> {
 		return this._messages.length === 0
 	}
 
+	/**
+	 * Reserve a specific queued message by ID when it is still present and
+	 * unclaimed. Lets a consumer hold a consumed entry through its durable ack
+	 * so neither another ask nor a background drain can claim it again
+	 * mid-persistence.
+	 */
+	public claimMessage(id: string): boolean {
+		if (this.claimedMessageIds.has(id)) {
+			return false
+		}
+		if (!this._messages.some((message) => message.id === id)) {
+			return false
+		}
+		this.claimedMessageIds.add(id)
+		return true
+	}
+
 	/**
 	 * Whether at least one queued message is still available to be claimed.
 	 *