diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 851df91e6c5e..b96fd90e0374 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -862,8 +862,50 @@ export class Task extends EventEmitter implements TaskLike { } } + // Set up queue listener to actively monitor for newly queued messages while waiting + let queueListener: (() => void) | undefined + if (isBlocking) { + queueListener = () => { + // Check if a message was queued while we're waiting + if ( + !this.messageQueueService.isEmpty() && + this.askResponse === undefined && + this.lastMessageTs === askTs + ) { + console.log("Task#ask detected queued message while waiting") + const message = this.messageQueueService.dequeueMessage() + + if (message) { + // Check if this is a tool approval ask that needs to be handled + if ( + type === "tool" || + type === "command" || + type === "browser_action_launch" || + type === "use_mcp_server" + ) { + // For tool approvals, we need to approve first, then send the message if there's text/images + this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images) + } else { + // For other ask types (like followup), fulfill the ask directly + this.setMessageResponse(message.text, message.images) + } + } + } + } + + // Attach the listener + this.messageQueueService.on("stateChanged", queueListener) + } + + try { // Wait for askResponse to be set. await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 }) + } finally { + // Clean up queue listener + if (queueListener) { + this.messageQueueService.removeListener("stateChanged", queueListener) + } + } if (this.lastMessageTs !== askTs) { // Could happen if we send multiple asks in a row i.e. with @@ -2275,6 +2317,14 @@ export class Task extends EventEmitter implements TaskLike { // Reset parser after each complete conversation round this.assistantMessageParser.reset() + // Drain any queued messages after API turn completes + // This ensures that user messages sent during LLM processing are promptly handled + // between API steps rather than being stuck in queue + while (!this.messageQueueService.isEmpty()) { + console.log("[Task] Draining message queue after API turn") + this.processQueuedMessages() + } + // Now add to apiConversationHistory. // Need to save assistant responses to file before proceeding to // tool use since user can exit at any moment and we wouldn't be diff --git a/src/core/webview/__tests__/webviewMessageHandler.race-condition.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.race-condition.spec.ts new file mode 100644 index 000000000000..137d721d84ff --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.race-condition.spec.ts @@ -0,0 +1,279 @@ +// npx vitest core/webview/__tests__/webviewMessageHandler.race-condition.spec.ts + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import * as vscode from "vscode" +import { webviewMessageHandler } from "../webviewMessageHandler" +import { ClineProvider } from "../ClineProvider" +import { Task } from "../../task/Task" +import { WebviewMessage } from "../../../shared/WebviewMessage" + +vi.mock("vscode") + +describe("webviewMessageHandler - Race Condition Prevention", () => { + let provider: ClineProvider + let task: any + let messageQueueService: any + let postStateToWebviewSpy: any + let addMessageSpy: any + + beforeEach(() => { + // Create mock message queue service + messageQueueService = { + addMessage: vi.fn().mockReturnValue(true), + getMessage: vi.fn(), + getAllMessages: vi.fn().mockReturnValue([]), + clearMessages: vi.fn(), + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + } + + // Create mock task - use 'any' type to allow modifiable taskAsk + task = { + id: "test-task-123", + taskAsk: undefined, // Initially no pending ask + messageQueueService, + handleWebviewAskResponse: vi.fn(), + } + + // Create mock provider + provider = { + context: { + globalState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + }, + workspaceState: { + get: vi.fn().mockReturnValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + }, + extensionPath: "/test/path", + }, + contextProxy: { + getValue: vi.fn(), + setValue: vi.fn().mockResolvedValue(undefined), + globalStorageUri: { fsPath: "/test/storage" }, + }, + cwd: "/test/workspace", + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + getCurrentTask: vi.fn().mockReturnValue(task), + log: vi.fn(), + } as any + + // Setup spies - use 'any' type to avoid type issues + postStateToWebviewSpy = vi.spyOn(provider, "postStateToWebview") + addMessageSpy = vi.spyOn(messageQueueService, "addMessage") + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe("Defensive routing for free user messages", () => { + it('should route message to queue when askResponse is "messageResponse" and no taskAsk exists', async () => { + // Arrange: No pending ask + task.taskAsk = undefined + + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "User message during race condition", + images: ["image1.png"], + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert: Message should be queued instead of processed as askResponse + expect(addMessageSpy).toHaveBeenCalledWith("User message during race condition", ["image1.png"]) + expect(task.handleWebviewAskResponse).not.toHaveBeenCalled() + expect(postStateToWebviewSpy).toHaveBeenCalled() + }) + + it("should process askResponse normally when taskAsk exists", async () => { + // Arrange: Pending ask exists + task.taskAsk = { + type: "followup", + text: "Need user input", + } as any + + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "User response to ask", + images: [], + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert: Message should be processed as askResponse + expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "User response to ask", []) + expect(addMessageSpy).not.toHaveBeenCalled() + }) + + it("should not affect other askResponse types", async () => { + // Arrange: No pending ask but different response type + task.taskAsk = undefined + + const message: WebviewMessage = { + type: "askResponse", + askResponse: "yesButtonClicked", // Different response type + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert: Should process normally (not queue) + expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("yesButtonClicked", undefined, undefined) + expect(addMessageSpy).not.toHaveBeenCalled() + }) + + it("should handle case when no current task exists", async () => { + // Arrange: No current task + provider.getCurrentTask = vi.fn().mockReturnValue(undefined) + + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "Message with no task", + images: [], + } + + // Act & Assert: Should not throw error + await expect(webviewMessageHandler(provider, message)).resolves.not.toThrow() + expect(addMessageSpy).not.toHaveBeenCalled() + }) + }) + + describe("Queue message handling", () => { + it("should add message to queue when queueMessage type is received", async () => { + // Arrange + const message: WebviewMessage = { + type: "queueMessage", + text: "Queued message", + images: ["image.jpg"], + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert + expect(addMessageSpy).toHaveBeenCalledWith("Queued message", ["image.jpg"]) + // Note: postStateToWebview is not called for queueMessage in actual implementation + }) + + it("should handle queue full scenario gracefully", async () => { + // Arrange: Queue is full + addMessageSpy.mockReturnValue(false) + + const message: WebviewMessage = { + type: "queueMessage", + text: "Message when queue is full", + images: [], + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert: Should attempt to add to queue even if full + expect(addMessageSpy).toHaveBeenCalledWith("Message when queue is full", []) + // The actual implementation uses message.text ?? "" so it passes the actual text + }) + }) + + describe("Race condition window coverage", () => { + it("should prevent message loss during rapid state transitions", async () => { + // Simulate rapid fire messages during state transition + const messages: WebviewMessage[] = [ + { type: "askResponse", askResponse: "messageResponse", text: "Message 1", images: [] }, + { type: "askResponse", askResponse: "messageResponse", text: "Message 2", images: [] }, + { type: "askResponse", askResponse: "messageResponse", text: "Message 3", images: [] }, + ] + + // Initially no ask + task.taskAsk = undefined + + // Process all messages + for (const msg of messages) { + await webviewMessageHandler(provider, msg) + } + + // All messages should be queued + expect(addMessageSpy).toHaveBeenCalledTimes(3) + expect(addMessageSpy).toHaveBeenNthCalledWith(1, "Message 1", []) + expect(addMessageSpy).toHaveBeenNthCalledWith(2, "Message 2", []) + expect(addMessageSpy).toHaveBeenNthCalledWith(3, "Message 3", []) + + // None should be processed as askResponse + expect(task.handleWebviewAskResponse).not.toHaveBeenCalled() + }) + + it("should handle mixed message scenarios correctly", async () => { + // Start with no ask + task.taskAsk = undefined + + // Message 1: Should be queued + await webviewMessageHandler(provider, { + type: "askResponse", + askResponse: "messageResponse", + text: "Message during no ask", + images: [], + }) + + expect(addMessageSpy).toHaveBeenCalledWith("Message during no ask", []) + expect(addMessageSpy).toHaveBeenCalledTimes(1) + + // Now set taskAsk + task.taskAsk = { type: "followup" } as any + + // Message 2: Should be processed + await webviewMessageHandler(provider, { + type: "askResponse", + askResponse: "messageResponse", + text: "Message during ask", + images: [], + }) + + expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Message during ask", []) + + // Remove taskAsk again + task.taskAsk = undefined + + // Message 3: Should be queued again + await webviewMessageHandler(provider, { + type: "askResponse", + askResponse: "messageResponse", + text: "Another message during no ask", + images: [], + }) + + expect(addMessageSpy).toHaveBeenCalledTimes(2) + expect(addMessageSpy).toHaveBeenNthCalledWith(2, "Another message during no ask", []) + }) + }) + + describe("Console logging behavior", () => { + it("should log when routing message to queue due to no pending ask", async () => { + // Arrange + const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + task.taskAsk = undefined + + const message: WebviewMessage = { + type: "askResponse", + askResponse: "messageResponse", + text: "Test message", + images: [], + } + + // Act + await webviewMessageHandler(provider, message) + + // Assert + expect(consoleSpy).toHaveBeenCalledWith("[webviewMessageHandler] No pending ask, routing message to queue") + + consoleSpy.mockRestore() + }) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index af5f9925c353..57ad3b7abd77 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -577,9 +577,25 @@ export const webviewMessageHandler = async ( await updateGlobalState("alwaysAllowUpdateTodoList", message.bool) await provider.postStateToWebview() break - case "askResponse": - provider.getCurrentTask()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) + case "askResponse": { + // Defensive routing: Check if there's an active ask before processing the response + // This prevents messages from vanishing when sent during the race condition window + const task = provider.getCurrentTask() + if (task) { + // If this is a "messageResponse" and there's no pending ask, queue it instead + if (message.askResponse === "messageResponse" && !task.taskAsk) { + // No ask is pending, route to queue to ensure message is captured + console.log("[webviewMessageHandler] No pending ask, routing message to queue") + task.messageQueueService.addMessage(message.text || "", message.images) + // Post state to update UI with queued message + await provider.postStateToWebview() + } else { + // Normal flow: there's a pending ask or it's not a messageResponse + task.handleWebviewAskResponse(message.askResponse!, message.text, message.images) + } + } break + } case "autoCondenseContext": await updateGlobalState("autoCondenseContext", message.bool) await provider.postStateToWebview() diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index d358c68f1cff..5a222dc688a2 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -176,6 +176,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction(null) const [sendingDisabled, setSendingDisabled] = useState(false) const [selectedImages, setSelectedImages] = useState([]) + // Keep a stable ref for selected images to preserve user draft while queue drains + const selectedImagesRef = useRef(selectedImages) // we need to hold on to the ask because useEffect > lastMessage will always let us know when an ask comes in and handle it, but by the time handleMessage is called, the last message might not be the ask anymore (it could be a say that followed) const [clineAsk, setClineAsk] = useState(undefined) @@ -224,6 +226,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + selectedImagesRef.current = selectedImages + }, [selectedImages]) + useEffect(() => { isMountedRef.current = true return () => { @@ -594,9 +601,18 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0) { - if (sendingDisabled) { + // UI hardening: Prefer queuing when no active ask during model flight or transitions + // This helps narrow the race condition window where messages could vanish + const shouldQueue = + sendingDisabled || (messagesRef.current.length > 0 && !clineAskRef.current && isStreaming) + + if (shouldQueue) { try { - console.log("queueMessage", text, images) + console.log("queueMessage", text, images, { + sendingDisabled, + noActiveAsk: !clineAskRef.current, + isStreaming, + }) vscode.postMessage({ type: "queueMessage", text, images }) setInputValue("") setSelectedImages([]) @@ -650,7 +666,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction