Fix: Prevent auto-acceptance of attempt_completion when messages are queued #9316
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…queued - Check for queued messages before auto-accepting completion - Process queued messages as user feedback instead of accepting - Add tests for queued message handling during attempt_completion Fixes #9314
dosubot
bot
added
size:L
Contributor
Author
Review completed. Found 1 issue that needs attention:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
Comment on lines
+83
to
+102
| // Check if there are queued messages before auto-accepting | ||
| const hasQueuedMessages = !task.messageQueueService.isEmpty() | ||
|
|
||
| // If there are queued messages, don't auto-accept the completion | ||
| // Instead, process the queued messages first | ||
| if (hasQueuedMessages) { | ||
| // Get the first queued message | ||
| const queuedMessage = task.messageQueueService.dequeueMessage() | ||
| if (queuedMessage) { | ||
| // Add the queued message as user feedback | ||
| await task.say("user_feedback", queuedMessage.text ?? "", queuedMessage.images) | ||
|
|
||
| const feedbackText = `The user has provided feedback on the results. Consider their input to continue the task, and then attempt completion again.\n<feedback>\n${queuedMessage.text}\n</feedback>` | ||
| pushToolResult(formatResponse.toolResult(feedbackText, queuedMessage.images)) | ||
|
|
||
| // Process any remaining queued messages | ||
| task.processQueuedMessages() | ||
| return | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
The queued message check is placed after the subtask approval logic (lines 70-81), which means subtasks will bypass this check entirely. Since the issue mentions orchestrator flows (which use subtasks), queued messages during subtask completion will still be ignored. The check should be moved before the if (task.parentTask) block to ensure both regular tasks and subtasks handle queued messages.
Fix it with Roo Code or mention @roomote and request a fix.
hannesrudolph
added
the
Issue/PR - Triage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR attempts to address Issue #9314. When messages are queued during
attempt_completion, they are now processed as user feedback instead of being ignored and having the completion auto-accepted.Problem
Previously, if a user queued messages while a task was finishing (particularly in orchestrator flows), the
attempt_completionwould be automatically accepted without processing the queued messages. This caused issues where users had to go back to subtasks and re-send messages.Solution
AttemptCompletionToolto detect queued messages before auto-acceptingTesting
Fixes #9314
Feedback and guidance are welcome!
Important
This PR prevents auto-acceptance of task completion by processing queued messages as user feedback in
AttemptCompletionTool.ts.AttemptCompletionTool.ts, added check for queued messages before auto-accepting task completion.attemptCompletionTool.spec.tsto verify queued message handling.This description was created by
for 75bc9bb. You can customize this summary. It will automatically update as commits are pushed.