mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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
This commit is contained in:
parent
0d0bba2eb7
commit
ba80e90638
2 changed files with 168 additions and 17 deletions
|
|
@ -368,7 +368,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
if (!isPartial) {
|
||||
playSound("celebration")
|
||||
}
|
||||
setSendingDisabled(isPartial)
|
||||
// Keep sendingDisabled as true during completion_result to prevent message loss
|
||||
// Messages will be queued and processed after the user starts a new task
|
||||
setSendingDisabled(true)
|
||||
setClineAsk("completion_result")
|
||||
setEnableButtons(!isPartial)
|
||||
setPrimaryButtonText(t("chat:startNewTask.title"))
|
||||
|
|
@ -635,13 +637,19 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
|
||||
useEffect(() => {
|
||||
// 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<ChatViewRef, ChatViewPro
|
|||
return () => {
|
||||
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<ChatViewRef, ChatViewPro
|
|||
case "resume_completed_task":
|
||||
// Waiting for feedback, but we can just present a new task button
|
||||
startNewTask()
|
||||
// Process any queued messages after starting new task
|
||||
// We don't need to check messageQueue.length here as it will be
|
||||
// processed automatically once sendingDisabled is set to false
|
||||
// Give the new task a moment to initialize, then enable sending
|
||||
setTimeout(() => {
|
||||
setSendingDisabled(false)
|
||||
}, 100)
|
||||
break
|
||||
case "command_output":
|
||||
vscode.postMessage({ type: "terminalOperation", terminalOperation: "continue" })
|
||||
|
|
|
|||
|
|
@ -1469,17 +1469,9 @@ 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: [
|
||||
{
|
||||
type: "ask",
|
||||
ask: "completion_result",
|
||||
ts: Date.now(),
|
||||
text: "Task completed",
|
||||
partial: false,
|
||||
},
|
||||
],
|
||||
clineMessages: [],
|
||||
})
|
||||
|
||||
// Wait for state to be updated
|
||||
|
|
@ -1492,4 +1484,148 @@ describe("ChatView - Message Queueing Tests", () => {
|
|||
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,
|
||||
},
|
||||
],
|
||||
})
|
||||
|
||||
// Wait for state to be updated
|
||||
await waitFor(() => {
|
||||
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")
|
||||
})
|
||||
|
||||
// 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")!
|
||||
|
||||
// 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: [],
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue