Skip to content

Fix message queueing blocking diff save feedback #6512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
roomote wants to merge 1 commit into from
Closed

Fix message queueing blocking diff save feedback #6512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 25 additions & 6 deletions webview-ui/src/components/chat/ChatView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,12 +556,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
* @param fromQueue - Internal flag indicating if this message is being sent from the queue (prevents re-queueing)
*/
const handleSendMessage = useCallback(
(text: string, images: string[], fromQueue = false) => {
(text: string, images: string[], fromQueue = false, bypassQueue = false) => {
Copy link
Contributor Author

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc comment should be updated to document the new bypassQueue parameter. Consider adding documentation for this parameter to clarify its purpose:

@param bypassQueue - When true, sends the message immediately even if sendingDisabled is true (used for tool operations)

try {
text = text.trim()

if (text || images.length > 0) {
if (sendingDisabled && !fromQueue) {
if (sendingDisabled && !fromQueue && !bypassQueue) {
// Generate a more unique ID using timestamp + random component
const messageId = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`
setMessageQueue((prev) => [...prev, { id: messageId, text, images }])
Expand Down Expand Up @@ -718,7 +718,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
switch (clineAsk) {
case "api_req_failed":
case "command":
case "tool":
case "browser_action_launch":
case "use_mcp_server":
case "resume_task":
Expand All @@ -738,6 +737,16 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" })
}
break
case "tool":
// For tool operations (including diff saves), send message with bypass queue flag
if (trimmedInput || (images && images.length > 0)) {
// Use handleSendMessage with bypassQueue=true to ensure message is sent immediately
// even if sendingDisabled is true (which happens during diff saves)
handleSendMessage(trimmedInput || "", images || [], false, true)
Copy link
Contributor Author

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice there's identical logic for handling tool operations in both button click handlers (here and around line 805). Would it make sense to extract this into a shared helper function to reduce duplication? This would make the code more maintainable and ensure consistent behavior.

} else {
vscode.postMessage({ type: "askResponse", askResponse: "yesButtonClicked" })
}
break
case "completion_result":
case "resume_completed_task":
// Waiting for feedback, but we can just present a new task button
Expand All @@ -752,7 +761,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
setClineAsk(undefined)
setEnableButtons(false)
},
[clineAsk, startNewTask],
[clineAsk, startNewTask, handleSendMessage],
)

const handleSecondaryButtonClick = useCallback(
Expand All @@ -775,7 +784,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
startNewTask()
break
case "command":
case "tool":
case "browser_action_launch":
case "use_mcp_server":
// Only send text/images if they exist
Expand All @@ -794,6 +802,17 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
vscode.postMessage({ type: "askResponse", askResponse: "noButtonClicked" })
}
break
case "tool":
// For tool operations (including diff saves), send message with bypass queue flag
if (trimmedInput || (images && images.length > 0)) {
// Use handleSendMessage with bypassQueue=true to ensure message is sent immediately
// even if sendingDisabled is true (which happens during diff saves)
handleSendMessage(trimmedInput || "", images || [], false, true)
Copy link
Contributor Author

@roomote roomote bot Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same duplication here as mentioned above. This identical logic could use the same shared helper function to avoid code duplication.

} else {
// Responds to the API with a "This operation failed" and lets it try again
vscode.postMessage({ type: "askResponse", askResponse: "noButtonClicked" })
}
break
case "command_output":
vscode.postMessage({ type: "terminalOperation", terminalOperation: "abort" })
break
Expand All @@ -802,7 +821,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
setClineAsk(undefined)
setEnableButtons(false)
},
[clineAsk, startNewTask, isStreaming],
[clineAsk, startNewTask, isStreaming, handleSendMessage],
)

const handleTaskCloseButtonClick = useCallback(() => startNewTask(), [startNewTask])
Expand Down