-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
Conversation
- Add bypassQueue parameter to handleSendMessage function - Detect tool operations (diff saves) in button click handlers - Allow messages to be sent immediately during tool operations - Fixes issue where user feedback during diff saves was being queued - Resolves #6509
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and the fix correctly addresses the issue. The implementation allows messages to bypass the queue during tool operations (including diff saves), ensuring user feedback is sent immediately. I have some suggestions for improvement below.
| */ | ||
| const handleSendMessage = useCallback( | ||
| (text: string, images: string[], fromQueue = false) => { | ||
| (text: string, images: string[], fromQueue = false, bypassQueue = false) => { |
There was a problem hiding this comment.
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)
| 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) |
There was a problem hiding this comment.
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.
| 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) |
There was a problem hiding this comment.
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.
|
Fixed on #6487 |
Fixes issue where user feedback messages during diff save operations were being queued instead of sent immediately with the diff save.
Problem
Issue #6509 reported that Roo was no longer sending messages with diff saves. When users would type a message to provide steering/feedback while saving a diff, the message would no longer be included in the context as it had been previously. This was a regression caused by the message queueing functionality.
Root Cause
The message queueing system was intercepting user messages during diff save operations when
sendingDisabledwas true, causing them to be queued instead of sent immediately with the diff save.Solution
bypassQueueparameter to thehandleSendMessagefunctionChanges
bypassQueueparameter tohandleSendMessagehandlePrimaryButtonClickandhandleSecondaryButtonClickto detect tool operationsbypassQueueis true, even ifsendingDisabledis trueTesting
Resolves #6509
Important
Fixes message queueing issue during diff saves by adding
bypassQueuetohandleSendMessageinChatView.tsx.bypassQueueparameter tohandleSendMessageto allow immediate message sending during tool operations.handlePrimaryButtonClickandhandleSecondaryButtonClickto usebypassQueuefor tool operations.ChatView.tsx, modifieshandleSendMessageto includebypassQueuelogic.bypassQueue.This description was created by
for d437638. You can customize this summary. It will automatically update as commits are pushed.