Skip to content

Commit 25c6f46

Browse files
authored
fix: prevent message loss during queue drain race condition (#8955)
1 parent 31de103 commit 25c6f46

File tree

2 files changed

+226
-6
lines changed

2 files changed

+226
-6
lines changed

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
596596
text = text.trim()
597597

598598
if (text || images.length > 0) {
599-
if (sendingDisabled) {
599+
// Queue message if:
600+
// - Task is busy (sendingDisabled)
601+
// - API request in progress (isStreaming)
602+
// - Queue has items (preserve message order during drain)
603+
if (sendingDisabled || isStreaming || messageQueue.length > 0) {
600604
try {
601605
console.log("queueMessage", text, images)
602606
vscode.postMessage({ type: "queueMessage", text, images })
@@ -652,7 +656,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
652656
handleChatReset()
653657
}
654658
},
655-
[handleChatReset, markFollowUpAsAnswered, sendingDisabled], // messagesRef and clineAskRef are stable
659+
[handleChatReset, markFollowUpAsAnswered, sendingDisabled, isStreaming, messageQueue.length], // messagesRef and clineAskRef are stable
656660
)
657661

658662
const handleSetChatBoxMessage = useCallback(

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

Lines changed: 220 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// npx vitest run src/components/chat/__tests__/ChatView.spec.tsx
22

33
import React from "react"
4-
import { render, waitFor, act } from "@/utils/test-utils"
4+
import { render, waitFor, act, fireEvent } from "@/utils/test-utils"
55
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
66

77
import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContext"
@@ -158,8 +158,9 @@ vi.mock("react-i18next", () => ({
158158
}))
159159

160160
interface ChatTextAreaProps {
161-
onSend: (value: string) => void
161+
onSend: () => void
162162
inputValue?: string
163+
setInputValue?: (value: string) => void
163164
sendingDisabled?: boolean
164165
placeholderText?: string
165166
selectedImages?: string[]
@@ -187,9 +188,19 @@ vi.mock("../ChatTextArea", () => {
187188
<input
188189
ref={mockInputRef}
189190
type="text"
191+
value={props.inputValue || ""}
190192
onChange={(e) => {
191-
// With message queueing, onSend is always called (it handles queueing internally)
192-
props.onSend(e.target.value)
193+
// Use parent's setInputValue if available
194+
if (props.setInputValue) {
195+
props.setInputValue(e.target.value)
196+
}
197+
}}
198+
onKeyDown={(e) => {
199+
// Only call onSend when Enter is pressed (simulating real behavior)
200+
if (e.key === "Enter" && !e.shiftKey) {
201+
e.preventDefault()
202+
props.onSend()
203+
}
193204
}}
194205
data-sending-disabled={props.sendingDisabled}
195206
/>
@@ -1487,4 +1498,209 @@ describe("ChatView - Message Queueing Tests", () => {
14871498
const input = chatTextArea.querySelector("input")!
14881499
expect(input.getAttribute("data-sending-disabled")).toBe("false")
14891500
})
1501+
1502+
it("queues messages when API request is in progress (spinner visible)", async () => {
1503+
const { getByTestId } = renderChatView()
1504+
1505+
// First hydrate state with initial task
1506+
mockPostMessage({
1507+
clineMessages: [
1508+
{
1509+
type: "say",
1510+
say: "task",
1511+
ts: Date.now() - 2000,
1512+
text: "Initial task",
1513+
},
1514+
],
1515+
})
1516+
1517+
// Clear any initial calls
1518+
vi.mocked(vscode.postMessage).mockClear()
1519+
1520+
// Add api_req_started without cost (spinner state - API request in progress)
1521+
mockPostMessage({
1522+
clineMessages: [
1523+
{
1524+
type: "say",
1525+
say: "task",
1526+
ts: Date.now() - 2000,
1527+
text: "Initial task",
1528+
},
1529+
{
1530+
type: "say",
1531+
say: "api_req_started",
1532+
ts: Date.now(),
1533+
text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost = still streaming
1534+
},
1535+
],
1536+
})
1537+
1538+
// Wait for state to be updated
1539+
await waitFor(() => {
1540+
expect(getByTestId("chat-textarea")).toBeInTheDocument()
1541+
})
1542+
1543+
// Clear message calls before simulating user input
1544+
vi.mocked(vscode.postMessage).mockClear()
1545+
1546+
// Simulate user typing and sending a message during the spinner
1547+
const chatTextArea = getByTestId("chat-textarea")
1548+
const input = chatTextArea.querySelector("input")! as HTMLInputElement
1549+
1550+
// Trigger message send by simulating typing and Enter key press
1551+
await act(async () => {
1552+
// Use fireEvent to properly trigger React's onChange handler
1553+
fireEvent.change(input, { target: { value: "follow-up question during spinner" } })
1554+
1555+
// Simulate pressing Enter to send
1556+
fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
1557+
})
1558+
1559+
// Verify that the message was queued, not sent as askResponse
1560+
await waitFor(() => {
1561+
expect(vscode.postMessage).toHaveBeenCalledWith({
1562+
type: "queueMessage",
1563+
text: "follow-up question during spinner",
1564+
images: [],
1565+
})
1566+
})
1567+
1568+
// Verify it was NOT sent as a direct askResponse (which would get lost)
1569+
expect(vscode.postMessage).not.toHaveBeenCalledWith(
1570+
expect.objectContaining({
1571+
type: "askResponse",
1572+
askResponse: "messageResponse",
1573+
}),
1574+
)
1575+
})
1576+
1577+
it("sends messages normally when API request is complete (cost present)", async () => {
1578+
const { getByTestId } = renderChatView()
1579+
1580+
// Hydrate state with completed API request (cost present)
1581+
mockPostMessage({
1582+
clineMessages: [
1583+
{
1584+
type: "say",
1585+
say: "task",
1586+
ts: Date.now() - 2000,
1587+
text: "Initial task",
1588+
},
1589+
{
1590+
type: "say",
1591+
say: "api_req_started",
1592+
ts: Date.now(),
1593+
text: JSON.stringify({
1594+
apiProtocol: "anthropic",
1595+
cost: 0.05, // Cost present = streaming complete
1596+
tokensIn: 100,
1597+
tokensOut: 50,
1598+
}),
1599+
},
1600+
{
1601+
type: "say",
1602+
say: "text",
1603+
ts: Date.now(),
1604+
text: "Response from API",
1605+
},
1606+
],
1607+
})
1608+
1609+
// Wait for state to be updated
1610+
await waitFor(() => {
1611+
expect(getByTestId("chat-textarea")).toBeInTheDocument()
1612+
})
1613+
1614+
// Clear message calls before simulating user input
1615+
vi.mocked(vscode.postMessage).mockClear()
1616+
1617+
// Simulate user sending a message when API is done
1618+
const chatTextArea = getByTestId("chat-textarea")
1619+
const input = chatTextArea.querySelector("input")! as HTMLInputElement
1620+
1621+
await act(async () => {
1622+
// Use fireEvent to properly trigger React's onChange handler
1623+
fireEvent.change(input, { target: { value: "follow-up after completion" } })
1624+
1625+
// Simulate pressing Enter to send
1626+
fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
1627+
})
1628+
1629+
// Verify that the message was sent as askResponse, not queued
1630+
await waitFor(() => {
1631+
expect(vscode.postMessage).toHaveBeenCalledWith({
1632+
type: "askResponse",
1633+
askResponse: "messageResponse",
1634+
text: "follow-up after completion",
1635+
images: [],
1636+
})
1637+
})
1638+
1639+
// Verify it was NOT queued
1640+
expect(vscode.postMessage).not.toHaveBeenCalledWith(
1641+
expect.objectContaining({
1642+
type: "queueMessage",
1643+
}),
1644+
)
1645+
})
1646+
1647+
it("preserves message order when messages sent during queue drain", async () => {
1648+
const { getByTestId } = renderChatView()
1649+
1650+
// Hydrate state with API request in progress and existing queue
1651+
mockPostMessage({
1652+
clineMessages: [
1653+
{
1654+
type: "say",
1655+
say: "task",
1656+
ts: Date.now() - 2000,
1657+
text: "Initial task",
1658+
},
1659+
{
1660+
type: "say",
1661+
say: "api_req_started",
1662+
ts: Date.now(),
1663+
text: JSON.stringify({ apiProtocol: "anthropic" }), // No cost = still streaming
1664+
},
1665+
],
1666+
messageQueue: [
1667+
{ id: "msg1", text: "queued message 1", images: [] },
1668+
{ id: "msg2", text: "queued message 2", images: [] },
1669+
],
1670+
})
1671+
1672+
// Wait for state to be updated
1673+
await waitFor(() => {
1674+
expect(getByTestId("chat-textarea")).toBeInTheDocument()
1675+
})
1676+
1677+
// Clear message calls before simulating user input
1678+
vi.mocked(vscode.postMessage).mockClear()
1679+
1680+
// Simulate user sending a new message while queue has items
1681+
const chatTextArea = getByTestId("chat-textarea")
1682+
const input = chatTextArea.querySelector("input")! as HTMLInputElement
1683+
1684+
await act(async () => {
1685+
fireEvent.change(input, { target: { value: "message during queue drain" } })
1686+
fireEvent.keyDown(input, { key: "Enter", code: "Enter" })
1687+
})
1688+
1689+
// Verify that the new message was queued (not sent directly) to preserve order
1690+
await waitFor(() => {
1691+
expect(vscode.postMessage).toHaveBeenCalledWith({
1692+
type: "queueMessage",
1693+
text: "message during queue drain",
1694+
images: [],
1695+
})
1696+
})
1697+
1698+
// Verify it was NOT sent as askResponse (which would break ordering)
1699+
expect(vscode.postMessage).not.toHaveBeenCalledWith(
1700+
expect.objectContaining({
1701+
type: "askResponse",
1702+
askResponse: "messageResponse",
1703+
}),
1704+
)
1705+
})
14901706
})

0 commit comments

Comments
 (0)