-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix: Improve message queue reliability when AI is working #8034
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 VS Code notifications for queue success/failure - Implement error recovery to restore user input when queueing fails - Enhance error handling with proper user feedback - Prevent data loss by preserving user messages on queue failures Fixes #8033
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.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| error: "Message cannot be empty", | ||
| text: message.text, | ||
| images: message.images, | ||
| }) |
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.
When queueing fails due to empty text/images, we're only logging but not showing a VS Code error notification (unlike the "No active task" case at line 3031). Consider adding vscode.window.showErrorMessage() here for consistency in user feedback.
| messageId: queuedMessage.id, | ||
| }) | ||
| // Show VS Code notification for success | ||
| vscode.window.showInformationMessage("Message queued. It will be sent when the AI is ready.") |
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.
Is the VS Code notification at line 3010 necessary given we're already sending a messageQueued response to the webview? We might be duplicating the success feedback - the webview could handle showing the notification to avoid redundancy.
| | "insertTextIntoTextarea" | ||
| | "dismissedUpsells" | ||
| | "messageQueued" | ||
| | "showNotification" |
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.
Could we improve type safety here? The message field (line 208) is currently string | undefined, but we could create more specific types for different notification levels to catch potential issues at compile time.
| console.error("Message queueing failed:", message.error) | ||
| // Restore the user's input since queueing failed | ||
| if (message.text || message.images?.length) { | ||
| setInputValue(message.text || "") |
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 error recovery correctly restores the input, but users only see console logs. Consider adding a toast notification or inline error message for better UX - users might not realize their message failed to queue without checking the console.
|
This PR seems to be doing way more than we need, I have noticed that the message to be queued is lost when Roo is in certain states like transitioning between one request and the next, notifying the users won't help solving that issue. |
Fixes #8033
Problem
When the AI is working and a user wants to queue a prompt, sometimes the message doesn't get queued and the user's input is lost. This has been happening since the feature was first introduced.
Solution
This PR implements a comprehensive fix for the message queue reliability issue:
Backend Changes (
webviewMessageHandler.ts):Frontend Changes (
ChatView.tsx):Key Improvements
Testing
This implementation addresses the root cause identified in the code review - the lack of proper error recovery when message queueing fails.
Important
Improve message queue reliability by adding user feedback and input recovery mechanisms in
webviewMessageHandler.tsandChatView.tsx.webviewMessageHandler.ts):ChatView.tsx):messageQueuedandshowNotificationtypes toExtensionMessageinExtensionMessage.ts.This description was created by
for 4fd1d51. You can customize this summary. It will automatically update as commits are pushed.