Skip to content

Commit ba80e90

Browse files
committed
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
1 parent 0d0bba2 commit ba80e90

File tree

2 files changed

+162
-11
lines changed

2 files changed

+162
-11
lines changed

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
368368
if (!isPartial) {
369369
playSound("celebration")
370370
}
371-
setSendingDisabled(isPartial)
371+
// Keep sendingDisabled as true during completion_result to prevent message loss
372+
// Messages will be queued and processed after the user starts a new task
373+
setSendingDisabled(true)
372374
setClineAsk("completion_result")
373375
setEnableButtons(!isPartial)
374376
setPrimaryButtonText(t("chat:startNewTask.title"))
@@ -635,13 +637,19 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
635637

636638
useEffect(() => {
637639
// Early return if conditions aren't met
638-
// Also don't process queue if there's an API error (clineAsk === "api_req_failed")
639-
if (
640-
sendingDisabled ||
640+
// Don't process queue if:
641+
// - sending is disabled (except for completion_result which is a special case)
642+
// - there's an API error
643+
// - we're in the middle of streaming (except completion_result)
644+
const isCompletionResult = clineAsk === "completion_result"
645+
const shouldBlockQueue =
646+
(sendingDisabled && !isCompletionResult) ||
641647
messageQueue.length === 0 ||
642648
isProcessingQueueRef.current ||
643-
clineAsk === "api_req_failed"
644-
) {
649+
clineAsk === "api_req_failed" ||
650+
(isStreaming && !isCompletionResult)
651+
652+
if (shouldBlockQueue) {
645653
return
646654
}
647655

@@ -685,7 +693,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
685693
return () => {
686694
isProcessingQueueRef.current = false
687695
}
688-
}, [sendingDisabled, messageQueue, handleSendMessage, clineAsk])
696+
}, [sendingDisabled, messageQueue, handleSendMessage, clineAsk, isStreaming])
689697

690698
const handleSetChatBoxMessage = useCallback(
691699
(text: string, images: string[]) => {
@@ -753,6 +761,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
753761
case "resume_completed_task":
754762
// Waiting for feedback, but we can just present a new task button
755763
startNewTask()
764+
// Process any queued messages after starting new task
765+
// We don't need to check messageQueue.length here as it will be
766+
// processed automatically once sendingDisabled is set to false
767+
// Give the new task a moment to initialize, then enable sending
768+
setTimeout(() => {
769+
setSendingDisabled(false)
770+
}, 100)
756771
break
757772
case "command_output":
758773
vscode.postMessage({ type: "terminalOperation", terminalOperation: "continue" })

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

Lines changed: 140 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,13 +1469,81 @@ describe("ChatView - Message Queueing Tests", () => {
14691469
it("shows sending is enabled when no task is active", async () => {
14701470
const { getByTestId } = renderChatView()
14711471

1472-
// Hydrate state with completed task
1472+
// Hydrate state with no active task (empty messages)
1473+
mockPostMessage({
1474+
clineMessages: [],
1475+
})
1476+
1477+
// Wait for state to be updated
1478+
await waitFor(() => {
1479+
expect(getByTestId("chat-textarea")).toBeInTheDocument()
1480+
})
1481+
1482+
// Check that sending is enabled
1483+
const chatTextArea = getByTestId("chat-textarea")
1484+
const input = chatTextArea.querySelector("input")!
1485+
expect(input.getAttribute("data-sending-disabled")).toBe("false")
1486+
})
1487+
1488+
it("keeps sending disabled during completion_result streaming to prevent message loss", async () => {
1489+
const { getByTestId } = renderChatView()
1490+
1491+
// Hydrate state with initial task
1492+
mockPostMessage({
1493+
clineMessages: [
1494+
{
1495+
type: "say",
1496+
say: "task",
1497+
ts: Date.now() - 2000,
1498+
text: "Initial task",
1499+
},
1500+
],
1501+
})
1502+
1503+
// Add completion_result that is not partial (streaming finished)
14731504
mockPostMessage({
14741505
clineMessages: [
1506+
{
1507+
type: "say",
1508+
say: "task",
1509+
ts: Date.now() - 2000,
1510+
text: "Initial task",
1511+
},
14751512
{
14761513
type: "ask",
14771514
ask: "completion_result",
14781515
ts: Date.now(),
1516+
text: "Task completed successfully",
1517+
partial: false, // Not partial, streaming is done
1518+
},
1519+
],
1520+
})
1521+
1522+
// Wait for state to be updated
1523+
await waitFor(() => {
1524+
const chatTextArea = getByTestId("chat-textarea")
1525+
const input = chatTextArea.querySelector("input")!
1526+
// Should remain disabled during completion_result to prevent message loss
1527+
expect(input.getAttribute("data-sending-disabled")).toBe("true")
1528+
})
1529+
})
1530+
1531+
it("keeps sending disabled during completion_result to queue messages properly", async () => {
1532+
const { getByTestId } = renderChatView()
1533+
1534+
// Hydrate state with completion_result
1535+
mockPostMessage({
1536+
clineMessages: [
1537+
{
1538+
type: "say",
1539+
say: "task",
1540+
ts: Date.now() - 2000,
1541+
text: "Initial task",
1542+
},
1543+
{
1544+
type: "ask",
1545+
ask: "completion_result",
1546+
ts: Date.now() - 1000,
14791547
text: "Task completed",
14801548
partial: false,
14811549
},
@@ -1484,12 +1552,80 @@ describe("ChatView - Message Queueing Tests", () => {
14841552

14851553
// Wait for state to be updated
14861554
await waitFor(() => {
1487-
expect(getByTestId("chat-textarea")).toBeInTheDocument()
1555+
const chatTextArea = getByTestId("chat-textarea")
1556+
const input = chatTextArea.querySelector("input")!
1557+
// Verify that sending is disabled during completion_result
1558+
// This ensures messages will be queued instead of lost
1559+
expect(input.getAttribute("data-sending-disabled")).toBe("true")
14881560
})
14891561

1490-
// Check that sending is enabled
1562+
// Clear any initial calls
1563+
vi.mocked(vscode.postMessage).mockClear()
1564+
1565+
// Simulate user typing a message while completion_result is active
1566+
// This would queue the message since sending is disabled
14911567
const chatTextArea = getByTestId("chat-textarea")
14921568
const input = chatTextArea.querySelector("input")!
1493-
expect(input.getAttribute("data-sending-disabled")).toBe("false")
1569+
1570+
// Trigger the onChange handler which calls onSend
1571+
act(() => {
1572+
const event = new Event("change", { bubbles: true })
1573+
Object.defineProperty(event, "target", { value: { value: "New task message" }, writable: false })
1574+
input.dispatchEvent(event)
1575+
})
1576+
1577+
// The message should be queued (not sent immediately) because sending is disabled
1578+
expect(vscode.postMessage).not.toHaveBeenCalledWith({
1579+
type: "newTask",
1580+
text: "New task message",
1581+
images: [],
1582+
})
1583+
})
1584+
1585+
it("does not process queue during API errors", async () => {
1586+
const { getByTestId } = renderChatView()
1587+
1588+
// Hydrate state with API error
1589+
mockPostMessage({
1590+
clineMessages: [
1591+
{
1592+
type: "say",
1593+
say: "task",
1594+
ts: Date.now() - 2000,
1595+
text: "Initial task",
1596+
},
1597+
{
1598+
type: "ask",
1599+
ask: "api_req_failed",
1600+
ts: Date.now(),
1601+
text: "API request failed",
1602+
partial: false,
1603+
},
1604+
],
1605+
})
1606+
1607+
// Clear any initial calls
1608+
vi.mocked(vscode.postMessage).mockClear()
1609+
1610+
// Try to send a message - it should be queued but not processed
1611+
const chatTextArea = getByTestId("chat-textarea")
1612+
const input = chatTextArea.querySelector("input")!
1613+
1614+
act(() => {
1615+
const event = new Event("change", { bubbles: true })
1616+
Object.defineProperty(event, "target", { value: { value: "Message during API error" }, writable: false })
1617+
input.dispatchEvent(event)
1618+
})
1619+
1620+
// Wait a bit to ensure no processing happens
1621+
await new Promise((resolve) => setTimeout(resolve, 200))
1622+
1623+
// Message should not be sent during API error
1624+
expect(vscode.postMessage).not.toHaveBeenCalledWith({
1625+
type: "askResponse",
1626+
askResponse: "messageResponse",
1627+
text: "Message during API error",
1628+
images: [],
1629+
})
14941630
})
14951631
})

0 commit comments

Comments
 (0)