-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2995,17 +2995,59 @@ export const webviewMessageHandler = async ( | |
| */ | ||
|
|
||
| case "queueMessage": { | ||
| provider.getCurrentTask()?.messageQueueService.addMessage(message.text ?? "", message.images) | ||
| const currentTask = provider.getCurrentTask() | ||
| if (currentTask && currentTask.messageQueueService) { | ||
| const queuedMessage = currentTask.messageQueueService.addMessage(message.text ?? "", message.images) | ||
| if (queuedMessage) { | ||
| provider.log(`Message queued successfully: ${queuedMessage.id}`) | ||
| // Send confirmation back to webview | ||
| provider.postMessageToWebview({ | ||
| type: "messageQueued", | ||
| success: true, | ||
| messageId: queuedMessage.id, | ||
| }) | ||
| // Show VS Code notification for success | ||
| vscode.window.showInformationMessage("Message queued. It will be sent when the AI is ready.") | ||
| } else { | ||
| provider.log(`Failed to queue message: empty text and no images`) | ||
| provider.postMessageToWebview({ | ||
| type: "messageQueued", | ||
| success: false, | ||
| error: "Message cannot be empty", | ||
| text: message.text, | ||
| images: message.images, | ||
| }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
| } else { | ||
| provider.log(`Failed to queue message: No active task or message queue service`) | ||
| provider.postMessageToWebview({ | ||
| type: "messageQueued", | ||
| success: false, | ||
| error: "No active task to queue message", | ||
| text: message.text, | ||
| images: message.images, | ||
| }) | ||
| // Show VS Code notification for failure | ||
| vscode.window.showErrorMessage("Failed to queue message: No active task. Please try again.") | ||
| } | ||
| break | ||
| } | ||
| case "removeQueuedMessage": { | ||
| provider.getCurrentTask()?.messageQueueService.removeMessage(message.text ?? "") | ||
| const currentTask = provider.getCurrentTask() | ||
| if (currentTask && currentTask.messageQueueService) { | ||
| const success = currentTask.messageQueueService.removeMessage(message.text ?? "") | ||
| provider.log(`Message removal ${success ? "successful" : "failed"}: ${message.text}`) | ||
| } | ||
| break | ||
| } | ||
| case "editQueuedMessage": { | ||
| if (message.payload) { | ||
| const { id, text, images } = message.payload as EditQueuedMessagePayload | ||
| provider.getCurrentTask()?.messageQueueService.updateMessage(id, text, images) | ||
| const currentTask = provider.getCurrentTask() | ||
| if (currentTask && currentTask.messageQueueService) { | ||
| const success = currentTask.messageQueueService.updateMessage(id, text, images) | ||
| provider.log(`Message edit ${success ? "successful" : "failed"}: ${id}`) | ||
| } | ||
| } | ||
|
|
||
| break | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,6 +124,8 @@ export interface ExtensionMessage { | |
| | "commands" | ||
| | "insertTextIntoTextarea" | ||
| | "dismissedUpsells" | ||
| | "messageQueued" | ||
| | "showNotification" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we improve type safety here? The |
||
| text?: string | ||
| payload?: any // Add a generic payload for now, can refine later | ||
| action?: | ||
|
|
@@ -201,6 +203,9 @@ export interface ExtensionMessage { | |
| commands?: Command[] | ||
| queuedMessages?: QueuedMessage[] | ||
| list?: string[] // For dismissedUpsells | ||
| messageId?: string // For messageQueued response | ||
| level?: "error" | "warning" | "info" // For showNotification | ||
| message?: string // For showNotification | ||
| } | ||
|
|
||
| export type ExtensionState = Pick< | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -595,15 +595,22 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |
|
|
||
| if (text || images.length > 0) { | ||
| if (sendingDisabled) { | ||
| // Queue the message when sending is disabled | ||
| try { | ||
| console.log("queueMessage", text, images) | ||
| vscode.postMessage({ type: "queueMessage", text, images }) | ||
|
|
||
| // Clear the input immediately for better UX | ||
| // The backend will confirm if queueing was successful | ||
| setInputValue("") | ||
| setSelectedImages([]) | ||
| } catch (error) { | ||
| console.error( | ||
| `Failed to queue message: ${error instanceof Error ? error.message : String(error)}`, | ||
| ) | ||
| // Keep the input so user doesn't lose their message | ||
| // In webview context, we can't show VS Code notifications directly | ||
| // The error will be logged to console for debugging | ||
| } | ||
|
|
||
| return | ||
|
|
@@ -781,6 +788,30 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro | |
| const message: ExtensionMessage = e.data | ||
|
|
||
| switch (message.type) { | ||
| case "messageQueued": | ||
| // Handle message queue confirmation from backend | ||
| if (!message.success) { | ||
| // If queueing failed, show error and restore the input | ||
| console.error("Message queueing failed:", message.error) | ||
| // Restore the user's input since queueing failed | ||
| if (message.text || message.images?.length) { | ||
| setInputValue(message.text || "") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| setSelectedImages(message.images || []) | ||
| } | ||
| } else { | ||
| console.log("Message queued successfully:", message.messageId) | ||
| } | ||
| break | ||
| case "showNotification": | ||
| // Handle notification messages from backend | ||
| if (message.level === "error") { | ||
| console.error(message.message) | ||
| } else if (message.level === "warning") { | ||
| console.warn(message.message) | ||
| } else { | ||
| console.log(message.message) | ||
| } | ||
| break | ||
| case "action": | ||
| switch (message.action!) { | ||
| case "didBecomeVisible": | ||
|
|
||
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
messageQueuedresponse to the webview? We might be duplicating the success feedback - the webview could handle showing the notification to avoid redundancy.