From ba80e90638702dd18fa9069c214930ae3a9d8204 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 15 Aug 2025 03:28:48 +0000 Subject: [PATCH] fix: prevent message loss during attempt_completion streaming - Keep sendingDisabled as true during completion_result to queue messages - Add isStreaming check to message queue processing to prevent premature processing - Process queued messages after user starts new task from completion_result - Add comprehensive tests for message queueing behavior Fixes issue where messages sent during attempt_completion streaming were lost --- webview-ui/src/components/chat/ChatView.tsx | 29 +++- .../chat/__tests__/ChatView.spec.tsx | 144 +++++++++++++++++- 2 files changed, 162 insertions(+), 11 deletions(-) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 3b77bbdb7d4..0c9c48fe044 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -368,7 +368,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction { // Early return if conditions aren't met - // Also don't process queue if there's an API error (clineAsk === "api_req_failed") - if ( - sendingDisabled || + // Don't process queue if: + // - sending is disabled (except for completion_result which is a special case) + // - there's an API error + // - we're in the middle of streaming (except completion_result) + const isCompletionResult = clineAsk === "completion_result" + const shouldBlockQueue = + (sendingDisabled && !isCompletionResult) || messageQueue.length === 0 || isProcessingQueueRef.current || - clineAsk === "api_req_failed" - ) { + clineAsk === "api_req_failed" || + (isStreaming && !isCompletionResult) + + if (shouldBlockQueue) { return } @@ -685,7 +693,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { isProcessingQueueRef.current = false } - }, [sendingDisabled, messageQueue, handleSendMessage, clineAsk]) + }, [sendingDisabled, messageQueue, handleSendMessage, clineAsk, isStreaming]) const handleSetChatBoxMessage = useCallback( (text: string, images: string[]) => { @@ -753,6 +761,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + setSendingDisabled(false) + }, 100) break case "command_output": vscode.postMessage({ type: "terminalOperation", terminalOperation: "continue" }) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 19538ef9323..f1d34e55744 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -1469,13 +1469,81 @@ describe("ChatView - Message Queueing Tests", () => { it("shows sending is enabled when no task is active", async () => { const { getByTestId } = renderChatView() - // Hydrate state with completed task + // Hydrate state with no active task (empty messages) + mockPostMessage({ + clineMessages: [], + }) + + // Wait for state to be updated + await waitFor(() => { + expect(getByTestId("chat-textarea")).toBeInTheDocument() + }) + + // Check that sending is enabled + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! + expect(input.getAttribute("data-sending-disabled")).toBe("false") + }) + + it("keeps sending disabled during completion_result streaming to prevent message loss", async () => { + const { getByTestId } = renderChatView() + + // Hydrate state with initial task + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + ], + }) + + // Add completion_result that is not partial (streaming finished) mockPostMessage({ clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, { type: "ask", ask: "completion_result", ts: Date.now(), + text: "Task completed successfully", + partial: false, // Not partial, streaming is done + }, + ], + }) + + // Wait for state to be updated + await waitFor(() => { + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! + // Should remain disabled during completion_result to prevent message loss + expect(input.getAttribute("data-sending-disabled")).toBe("true") + }) + }) + + it("keeps sending disabled during completion_result to queue messages properly", async () => { + const { getByTestId } = renderChatView() + + // Hydrate state with completion_result + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + { + type: "ask", + ask: "completion_result", + ts: Date.now() - 1000, text: "Task completed", partial: false, }, @@ -1484,12 +1552,80 @@ describe("ChatView - Message Queueing Tests", () => { // Wait for state to be updated await waitFor(() => { - expect(getByTestId("chat-textarea")).toBeInTheDocument() + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! + // Verify that sending is disabled during completion_result + // This ensures messages will be queued instead of lost + expect(input.getAttribute("data-sending-disabled")).toBe("true") }) - // Check that sending is enabled + // Clear any initial calls + vi.mocked(vscode.postMessage).mockClear() + + // Simulate user typing a message while completion_result is active + // This would queue the message since sending is disabled const chatTextArea = getByTestId("chat-textarea") const input = chatTextArea.querySelector("input")! - expect(input.getAttribute("data-sending-disabled")).toBe("false") + + // Trigger the onChange handler which calls onSend + act(() => { + const event = new Event("change", { bubbles: true }) + Object.defineProperty(event, "target", { value: { value: "New task message" }, writable: false }) + input.dispatchEvent(event) + }) + + // The message should be queued (not sent immediately) because sending is disabled + expect(vscode.postMessage).not.toHaveBeenCalledWith({ + type: "newTask", + text: "New task message", + images: [], + }) + }) + + it("does not process queue during API errors", async () => { + const { getByTestId } = renderChatView() + + // Hydrate state with API error + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + { + type: "ask", + ask: "api_req_failed", + ts: Date.now(), + text: "API request failed", + partial: false, + }, + ], + }) + + // Clear any initial calls + vi.mocked(vscode.postMessage).mockClear() + + // Try to send a message - it should be queued but not processed + const chatTextArea = getByTestId("chat-textarea") + const input = chatTextArea.querySelector("input")! + + act(() => { + const event = new Event("change", { bubbles: true }) + Object.defineProperty(event, "target", { value: { value: "Message during API error" }, writable: false }) + input.dispatchEvent(event) + }) + + // Wait a bit to ensure no processing happens + await new Promise((resolve) => setTimeout(resolve, 200)) + + // Message should not be sent during API error + expect(vscode.postMessage).not.toHaveBeenCalledWith({ + type: "askResponse", + askResponse: "messageResponse", + text: "Message during API error", + images: [], + }) }) })