Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,8 +862,50 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
}
}

// Set up queue listener to actively monitor for newly queued messages while waiting
let queueListener: (() => void) | undefined
if (isBlocking) {
queueListener = () => {
// Check if a message was queued while we're waiting
if (
!this.messageQueueService.isEmpty() &&
this.askResponse === undefined &&
this.lastMessageTs === askTs
) {
console.log("Task#ask detected queued message while waiting")
const message = this.messageQueueService.dequeueMessage()

if (message) {
// Check if this is a tool approval ask that needs to be handled
if (
type === "tool" ||
type === "command" ||
type === "browser_action_launch" ||
type === "use_mcp_server"
) {
// For tool approvals, we need to approve first, then send the message if there's text/images
this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images)
} else {
// For other ask types (like followup), fulfill the ask directly
this.setMessageResponse(message.text, message.images)
}
}
}
}

// Attach the listener
this.messageQueueService.on("stateChanged", queueListener)
}

try {
// Wait for askResponse to be set.
await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 })
} finally {
// Clean up queue listener
if (queueListener) {
this.messageQueueService.removeListener("stateChanged", queueListener)
}
}

if (this.lastMessageTs !== askTs) {
// Could happen if we send multiple asks in a row i.e. with
Expand Down Expand Up @@ -2275,6 +2317,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Reset parser after each complete conversation round
this.assistantMessageParser.reset()

// Drain any queued messages after API turn completes
// This ensures that user messages sent during LLM processing are promptly handled
// between API steps rather than being stuck in queue
while (!this.messageQueueService.isEmpty()) {
console.log("[Task] Draining message queue after API turn")
this.processQueuedMessages()
}

// Now add to apiConversationHistory.
// Need to save assistant responses to file before proceeding to
// tool use since user can exit at any moment and we wouldn't be
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,279 @@
// npx vitest core/webview/__tests__/webviewMessageHandler.race-condition.spec.ts

import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import * as vscode from "vscode"
import { webviewMessageHandler } from "../webviewMessageHandler"
import { ClineProvider } from "../ClineProvider"
import { Task } from "../../task/Task"
import { WebviewMessage } from "../../../shared/WebviewMessage"

vi.mock("vscode")

describe("webviewMessageHandler - Race Condition Prevention", () => {
let provider: ClineProvider
let task: any
let messageQueueService: any
let postStateToWebviewSpy: any
let addMessageSpy: any

beforeEach(() => {
// Create mock message queue service
messageQueueService = {
addMessage: vi.fn().mockReturnValue(true),
getMessage: vi.fn(),
getAllMessages: vi.fn().mockReturnValue([]),
clearMessages: vi.fn(),
on: vi.fn(),
off: vi.fn(),
emit: vi.fn(),
}

// Create mock task - use 'any' type to allow modifiable taskAsk
task = {
id: "test-task-123",
taskAsk: undefined, // Initially no pending ask
messageQueueService,
handleWebviewAskResponse: vi.fn(),
}

// Create mock provider
provider = {
context: {
globalState: {
get: vi.fn().mockReturnValue(undefined),
update: vi.fn().mockResolvedValue(undefined),
},
workspaceState: {
get: vi.fn().mockReturnValue(undefined),
update: vi.fn().mockResolvedValue(undefined),
},
extensionPath: "/test/path",
},
contextProxy: {
getValue: vi.fn(),
setValue: vi.fn().mockResolvedValue(undefined),
globalStorageUri: { fsPath: "/test/storage" },
},
cwd: "/test/workspace",
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
getCurrentTask: vi.fn().mockReturnValue(task),
log: vi.fn(),
} as any

// Setup spies - use 'any' type to avoid type issues
postStateToWebviewSpy = vi.spyOn(provider, "postStateToWebview")
addMessageSpy = vi.spyOn(messageQueueService, "addMessage")
})

afterEach(() => {
vi.clearAllMocks()
})

describe("Defensive routing for free user messages", () => {
it('should route message to queue when askResponse is "messageResponse" and no taskAsk exists', async () => {
// Arrange: No pending ask
task.taskAsk = undefined

const message: WebviewMessage = {
type: "askResponse",
askResponse: "messageResponse",
text: "User message during race condition",
images: ["image1.png"],
}

// Act
await webviewMessageHandler(provider, message)

// Assert: Message should be queued instead of processed as askResponse
expect(addMessageSpy).toHaveBeenCalledWith("User message during race condition", ["image1.png"])
expect(task.handleWebviewAskResponse).not.toHaveBeenCalled()
expect(postStateToWebviewSpy).toHaveBeenCalled()
})

it("should process askResponse normally when taskAsk exists", async () => {
// Arrange: Pending ask exists
task.taskAsk = {
type: "followup",
text: "Need user input",
} as any

const message: WebviewMessage = {
type: "askResponse",
askResponse: "messageResponse",
text: "User response to ask",
images: [],
}

// Act
await webviewMessageHandler(provider, message)

// Assert: Message should be processed as askResponse
expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "User response to ask", [])
expect(addMessageSpy).not.toHaveBeenCalled()
})

it("should not affect other askResponse types", async () => {
// Arrange: No pending ask but different response type
task.taskAsk = undefined

const message: WebviewMessage = {
type: "askResponse",
askResponse: "yesButtonClicked", // Different response type
}

// Act
await webviewMessageHandler(provider, message)

// Assert: Should process normally (not queue)
expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("yesButtonClicked", undefined, undefined)
expect(addMessageSpy).not.toHaveBeenCalled()
})

it("should handle case when no current task exists", async () => {
// Arrange: No current task
provider.getCurrentTask = vi.fn().mockReturnValue(undefined)

const message: WebviewMessage = {
type: "askResponse",
askResponse: "messageResponse",
text: "Message with no task",
images: [],
}

// Act & Assert: Should not throw error
await expect(webviewMessageHandler(provider, message)).resolves.not.toThrow()
expect(addMessageSpy).not.toHaveBeenCalled()
})
})

describe("Queue message handling", () => {
it("should add message to queue when queueMessage type is received", async () => {
// Arrange
const message: WebviewMessage = {
type: "queueMessage",
text: "Queued message",
images: ["image.jpg"],
}

// Act
await webviewMessageHandler(provider, message)

// Assert
expect(addMessageSpy).toHaveBeenCalledWith("Queued message", ["image.jpg"])
// Note: postStateToWebview is not called for queueMessage in actual implementation
})

it("should handle queue full scenario gracefully", async () => {
// Arrange: Queue is full
addMessageSpy.mockReturnValue(false)

const message: WebviewMessage = {
type: "queueMessage",
text: "Message when queue is full",
images: [],
}

// Act
await webviewMessageHandler(provider, message)

// Assert: Should attempt to add to queue even if full
expect(addMessageSpy).toHaveBeenCalledWith("Message when queue is full", [])
// The actual implementation uses message.text ?? "" so it passes the actual text
})
})

describe("Race condition window coverage", () => {
it("should prevent message loss during rapid state transitions", async () => {
// Simulate rapid fire messages during state transition
const messages: WebviewMessage[] = [
{ type: "askResponse", askResponse: "messageResponse", text: "Message 1", images: [] },
{ type: "askResponse", askResponse: "messageResponse", text: "Message 2", images: [] },
{ type: "askResponse", askResponse: "messageResponse", text: "Message 3", images: [] },
]

// Initially no ask
task.taskAsk = undefined

// Process all messages
for (const msg of messages) {
await webviewMessageHandler(provider, msg)
}

// All messages should be queued
expect(addMessageSpy).toHaveBeenCalledTimes(3)
expect(addMessageSpy).toHaveBeenNthCalledWith(1, "Message 1", [])
expect(addMessageSpy).toHaveBeenNthCalledWith(2, "Message 2", [])
expect(addMessageSpy).toHaveBeenNthCalledWith(3, "Message 3", [])

// None should be processed as askResponse
expect(task.handleWebviewAskResponse).not.toHaveBeenCalled()
})

it("should handle mixed message scenarios correctly", async () => {
// Start with no ask
task.taskAsk = undefined

// Message 1: Should be queued
await webviewMessageHandler(provider, {
type: "askResponse",
askResponse: "messageResponse",
text: "Message during no ask",
images: [],
})

expect(addMessageSpy).toHaveBeenCalledWith("Message during no ask", [])
expect(addMessageSpy).toHaveBeenCalledTimes(1)

// Now set taskAsk
task.taskAsk = { type: "followup" } as any

// Message 2: Should be processed
await webviewMessageHandler(provider, {
type: "askResponse",
askResponse: "messageResponse",
text: "Message during ask",
images: [],
})

expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Message during ask", [])

// Remove taskAsk again
task.taskAsk = undefined

// Message 3: Should be queued again
await webviewMessageHandler(provider, {
type: "askResponse",
askResponse: "messageResponse",
text: "Another message during no ask",
images: [],
})

expect(addMessageSpy).toHaveBeenCalledTimes(2)
expect(addMessageSpy).toHaveBeenNthCalledWith(2, "Another message during no ask", [])
})
})

describe("Console logging behavior", () => {
it("should log when routing message to queue due to no pending ask", async () => {
// Arrange
const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {})
task.taskAsk = undefined

const message: WebviewMessage = {
type: "askResponse",
askResponse: "messageResponse",
text: "Test message",
images: [],
}

// Act
await webviewMessageHandler(provider, message)

// Assert
expect(consoleSpy).toHaveBeenCalledWith("[webviewMessageHandler] No pending ask, routing message to queue")

consoleSpy.mockRestore()
})
})
})
20 changes: 18 additions & 2 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,9 +577,25 @@ export const webviewMessageHandler = async (
await updateGlobalState("alwaysAllowUpdateTodoList", message.bool)
await provider.postStateToWebview()
break
case "askResponse":
provider.getCurrentTask()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images)
case "askResponse": {
// Defensive routing: Check if there's an active ask before processing the response
// This prevents messages from vanishing when sent during the race condition window
const task = provider.getCurrentTask()
if (task) {
// If this is a "messageResponse" and there's no pending ask, queue it instead
if (message.askResponse === "messageResponse" && !task.taskAsk) {
// No ask is pending, route to queue to ensure message is captured
console.log("[webviewMessageHandler] No pending ask, routing message to queue")
task.messageQueueService.addMessage(message.text || "", message.images)
// Post state to update UI with queued message
await provider.postStateToWebview()
} else {
// Normal flow: there's a pending ask or it's not a messageResponse
task.handleWebviewAskResponse(message.askResponse!, message.text, message.images)
}
}
break
}
case "autoCondenseContext":
await updateGlobalState("autoCondenseContext", message.bool)
await provider.postStateToWebview()
Expand Down
Loading