Răsfoiți Sursa

fix: prevent message loss during queue drain race condition (#8955)

Daniel 2 luni în urmă
părinte
comite
25c6f46e71

+ 6 - 2
webview-ui/src/components/chat/ChatView.tsx

@@ -596,7 +596,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 			text = text.trim()
 
 			if (text || images.length > 0) {
-				if (sendingDisabled) {
+				// Queue message if:
+				// - Task is busy (sendingDisabled)
+				// - API request in progress (isStreaming)
+				// - Queue has items (preserve message order during drain)
+				if (sendingDisabled || isStreaming || messageQueue.length > 0) {
 					try {
 						console.log("queueMessage", text, images)
 						vscode.postMessage({ type: "queueMessage", text, images })
@@ -652,7 +656,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 				handleChatReset()
 			}
 		},
-		[handleChatReset, markFollowUpAsAnswered, sendingDisabled], // messagesRef and clineAskRef are stable
+		[handleChatReset, markFollowUpAsAnswered, sendingDisabled, isStreaming, messageQueue.length], // messagesRef and clineAskRef are stable
 	)
 
 	const handleSetChatBoxMessage = useCallback(

+ 220 - 4
webview-ui/src/components/chat/__tests__/ChatView.spec.tsx

@@ -1,7 +1,7 @@
 // npx vitest run src/components/chat/__tests__/ChatView.spec.tsx
 
 import React from "react"
-import { render, waitFor, act } from "@/utils/test-utils"
+import { render, waitFor, act, fireEvent } from "@/utils/test-utils"
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
 
 import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContext"
@@ -158,8 +158,9 @@ vi.mock("react-i18next", () => ({
 }))
 
 interface ChatTextAreaProps {
-	onSend: (value: string) => void
+	onSend: () => void
 	inputValue?: string
+	setInputValue?: (value: string) => void
 	sendingDisabled?: boolean
 	placeholderText?: string
 	selectedImages?: string[]
@@ -187,9 +188,19 @@ vi.mock("../ChatTextArea", () => {
 				<input
 					ref={mockInputRef}
 					type="text"
+					value={props.inputValue || ""}
 					onChange={(e) => {
-						// With message queueing, onSend is always called (it handles queueing internally)
-						props.onSend(e.target.value)
+						// Use parent's setInputValue if available
+						if (props.setInputValue) {
+							props.setInputValue(e.target.value)
+						}
+					}}
+					onKeyDown={(e) => {
+						// Only call onSend when Enter is pressed (simulating real behavior)
+						if (e.key === "Enter" && !e.shiftKey) {
+							e.preventDefault()
+							props.onSend()
+						}
 					}}
 					data-sending-disabled={props.sendingDisabled}
 				/>
@@ -1487,4 +1498,209 @@ describe("ChatView - Message Queueing Tests", () => {
 		const input = chatTextArea.querySelector("input")!
 		expect(input.getAttribute("data-sending-disabled")).toBe("false")
 	})
+
+	it("queues messages when API request is in progress (spinner visible)", async () => {
+		const { getByTestId } = renderChatView()
+
+		// First hydrate state with initial task
+		mockPostMessage({
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+			],
+		})
+
+		// Clear any initial calls
+		vi.mocked(vscode.postMessage).mockClear()
+
+		// Add api_req_started without cost (spinner state - API request in progress)
+		mockPostMessage({
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "say",
+					say: "api_req_started",
+					ts: Date.now(),
+					text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost = still streaming
+				},
+			],
+		})
+
+		// Wait for state to be updated
+		await waitFor(() => {
+			expect(getByTestId("chat-textarea")).toBeInTheDocument()
+		})
+
+		// Clear message calls before simulating user input
+		vi.mocked(vscode.postMessage).mockClear()
+
+		// Simulate user typing and sending a message during the spinner
+		const chatTextArea = getByTestId("chat-textarea")
+		const input = chatTextArea.querySelector("input")! as HTMLInputElement
+
+		// Trigger message send by simulating typing and Enter key press
+		await act(async () => {
+			// Use fireEvent to properly trigger React's onChange handler
+			fireEvent.change(input, { target: { value: "follow-up question during spinner" } })
+
+			// Simulate pressing Enter to send
+			fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
+		})
+
+		// Verify that the message was queued, not sent as askResponse
+		await waitFor(() => {
+			expect(vscode.postMessage).toHaveBeenCalledWith({
+				type: "queueMessage",
+				text: "follow-up question during spinner",
+				images: [],
+			})
+		})
+
+		// Verify it was NOT sent as a direct askResponse (which would get lost)
+		expect(vscode.postMessage).not.toHaveBeenCalledWith(
+			expect.objectContaining({
+				type: "askResponse",
+				askResponse: "messageResponse",
+			}),
+		)
+	})
+
+	it("sends messages normally when API request is complete (cost present)", async () => {
+		const { getByTestId } = renderChatView()
+
+		// Hydrate state with completed API request (cost present)
+		mockPostMessage({
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "say",
+					say: "api_req_started",
+					ts: Date.now(),
+					text: JSON.stringify({
+						apiProtocol: "anthropic",
+						cost: 0.05, // Cost present = streaming complete
+						tokensIn: 100,
+						tokensOut: 50,
+					}),
+				},
+				{
+					type: "say",
+					say: "text",
+					ts: Date.now(),
+					text: "Response from API",
+				},
+			],
+		})
+
+		// Wait for state to be updated
+		await waitFor(() => {
+			expect(getByTestId("chat-textarea")).toBeInTheDocument()
+		})
+
+		// Clear message calls before simulating user input
+		vi.mocked(vscode.postMessage).mockClear()
+
+		// Simulate user sending a message when API is done
+		const chatTextArea = getByTestId("chat-textarea")
+		const input = chatTextArea.querySelector("input")! as HTMLInputElement
+
+		await act(async () => {
+			// Use fireEvent to properly trigger React's onChange handler
+			fireEvent.change(input, { target: { value: "follow-up after completion" } })
+
+			// Simulate pressing Enter to send
+			fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
+		})
+
+		// Verify that the message was sent as askResponse, not queued
+		await waitFor(() => {
+			expect(vscode.postMessage).toHaveBeenCalledWith({
+				type: "askResponse",
+				askResponse: "messageResponse",
+				text: "follow-up after completion",
+				images: [],
+			})
+		})
+
+		// Verify it was NOT queued
+		expect(vscode.postMessage).not.toHaveBeenCalledWith(
+			expect.objectContaining({
+				type: "queueMessage",
+			}),
+		)
+	})
+
+	it("preserves message order when messages sent during queue drain", async () => {
+		const { getByTestId } = renderChatView()
+
+		// Hydrate state with API request in progress and existing queue
+		mockPostMessage({
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: Date.now() - 2000,
+					text: "Initial task",
+				},
+				{
+					type: "say",
+					say: "api_req_started",
+					ts: Date.now(),
+					text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost = still streaming
+				},
+			],
+			messageQueue: [
+				{ id: "msg1", text: "queued message 1", images: [] },
+				{ id: "msg2", text: "queued message 2", images: [] },
+			],
+		})
+
+		// Wait for state to be updated
+		await waitFor(() => {
+			expect(getByTestId("chat-textarea")).toBeInTheDocument()
+		})
+
+		// Clear message calls before simulating user input
+		vi.mocked(vscode.postMessage).mockClear()
+
+		// Simulate user sending a new message while queue has items
+		const chatTextArea = getByTestId("chat-textarea")
+		const input = chatTextArea.querySelector("input")! as HTMLInputElement
+
+		await act(async () => {
+			fireEvent.change(input, { target: { value: "message during queue drain" } })
+			fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
+		})
+
+		// Verify that the new message was queued (not sent directly) to preserve order
+		await waitFor(() => {
+			expect(vscode.postMessage).toHaveBeenCalledWith({
+				type: "queueMessage",
+				text: "message during queue drain",
+				images: [],
+			})
+		})
+
+		// Verify it was NOT sent as askResponse (which would break ordering)
+		expect(vscode.postMessage).not.toHaveBeenCalledWith(
+			expect.objectContaining({
+				type: "askResponse",
+				askResponse: "messageResponse",
+			}),
+		)
+	})
 })