fix: prevent queue from interfering with manual approval workflow (#6996) #6997
Conversation
- Queue processing now pauses when approval buttons are showing (enableButtons is true) - Messages typed during approval requests are queued instead of being sent immediately - This prevents the unintended auto-rejection behavior when using QUEUE with manual approval Fixes #6996
dosubot
bot
added
size:S
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
|
|
||
| if (text || images.length > 0) { | ||
| if (sendingDisabled && !fromQueue) { | ||
| // Queue messages when: |
There was a problem hiding this comment.
The comment block here is helpful but could be more concise. Consider consolidating it to something like: 'Queue messages when approval buttons are showing or sending is disabled to prevent auto-rejection during manual approval'
| // 1. sendingDisabled is true (existing behavior) | ||
| // 2. OR when approval buttons are showing (enableButtons is true) to prevent auto-rejection | ||
| // This ensures that pressing Enter during manual approval doesn't immediately reject the request | ||
| if ((sendingDisabled || enableButtons) && !fromQueue) { |
There was a problem hiding this comment.
Consider extracting the condition (sendingDisabled || enableButtons) into a named variable or function like shouldQueueMessage for better readability and maintainability, since this logic appears in multiple places.
| isProcessingQueueRef.current || | ||
| clineAsk === "api_req_failed" | ||
| clineAsk === "api_req_failed" || | ||
| enableButtons // Don't process queue when approval buttons are shown |
There was a problem hiding this comment.
Is this intentional? The isProcessingQueueRef.current flag is set after the async check. If multiple effects trigger simultaneously, there could be a brief window where two queue processors start. Consider moving line 653 before the return statement to ensure the flag is set synchronously.
| isProcessingQueueRef.current = false | ||
| } | ||
| }, [sendingDisabled, messageQueue, handleSendMessage, clineAsk]) | ||
| }, [sendingDisabled, messageQueue, handleSendMessage, clineAsk, enableButtons]) |
There was a problem hiding this comment.
This fix correctly prevents queue processing during approval states, but it would benefit from test coverage. Consider adding tests for the queue/manual approval interaction to prevent regression of this critical workflow.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
|
Closing this PR. The fix creates a deadlock where queued messages can never be sent - they're blocked during approval (enableButtons=true) and after approval (sendingDisabled=true). The root issue is that #6996 lacks proper scoping. It needs clear requirements for:
The issue should be re-scoped before attempting another fix. |
|
Closing due to critical bug in implementation. Issue needs proper scoping before reattempting. |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
This PR fixes the issue where the QUEUE feature with manual approval enabled causes unintended auto-rejections when a follow-up message is placed in the queue.
Problem
When using QUEUE with manual approval:
Solution
The fix involves two key changes:
Changes Made
Testing
Related Issue
Fixes #6996
Important
Fixes QUEUE feature interference with manual approval by pausing queue processing when approval buttons are visible in
ChatView.tsx.enableButtonsis true inChatView.tsx, preventing interference with manual approval.handleSendMessagequeues messages ifenableButtonsis true, avoiding immediate sending during approval requests.ChatView.tsxto includeenableButtonsin conditions.handleSendMessageto checkenableButtonsbefore sending messages.enableButtonsto dependency arrays in relevantuseEffecthooks.This description was created by
for 4a96aa2. You can customize this summary. It will automatically update as commits are pushed.