fix: prevent infinite loop when attempt_completion succeeds #9325
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
When attempt_completion was called and the user clicked 'Yes' to mark the task complete, pushToolResult("") was adding an empty tool_result to userMessageContent. This triggered Task.ts:2609 to create a NEW API request with the empty result, causing the task to continue indefinitely with additional tool calls.
Root Cause:
- pushToolResult("") adds content to userMessageContent[] via presentAssistantMessage.ts:323-327
- Task.ts:2609 checks 'if (this.userMessageContent.length > 0)' and pushes to stack
- This creates a new API request even though the task should end
The Fix:
Remove pushToolResult("") when task completes successfully. The tool_result is now only sent when the user provides feedback, which is the correct behavior for native protocol.
Before (buggy):
1. attempt_completion called
2. User clicks Yes
3. pushToolResult("") -> triggers new API request
4. Model continues -> infinite loop
After (fixed):
1. attempt_completion called
2. User clicks Yes
3. NO pushToolResult("") -> task stops (correct)
When user provides feedback:
1. attempt_completion called
2. User provides feedback
3. pushToolResult(feedback) -> new API request with feedback (correct)
This also fixes a related 400 error where duplicate tool results could occur when users sent messages after task completion.
The bug was introduced in commit 5e6e601 (native tool support PR) where the empty tool_result was meant as a 'stop signal' but was being treated as real content.
Contributor
Review completed. No issues found. This commit removes an outdated comment from the code - a purely cosmetic cleanup following the actual bug fix from the previous commit. Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
mrubens
approved these changes
Nov 17, 2025
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.
Problem
When
attempt_completionwas called and the user clicked 'Yes' to mark the task complete,pushToolResult("")was adding an empty tool_result touserMessageContent. This triggeredTask.ts:2609to create a NEW API request with the empty result, causing the task to continue indefinitely with additional tool calls.Root Cause
pushToolResult("")adds content touserMessageContent[]viapresentAssistantMessage.ts:323-327Task.ts:2609checksif (this.userMessageContent.length > 0)and pushes to stackThe Fix
Remove
pushToolResult("")when task completes successfully. The tool_result is now only sent when the user provides feedback, which is the correct behavior for native protocol.Before (buggy)
attempt_completioncalledpushToolResult("")triggers new API requestAfter (fixed)
attempt_completioncalledpushToolResult("")- task stops (correct)When user provides feedback
attempt_completioncalledpushToolResult(feedback)- new API request with feedback (correct)Additional Fix
This also fixes a related 400 error where duplicate tool results could occur when users sent messages after task completion.
Context
The bug was introduced in commit 5e6e601 (native tool support PR) where the empty tool_result was meant as a 'stop signal' but was being treated as real content.
Testing
All existing tests pass (317 test files, 4117 tests)
Important
Fixes infinite loop in
AttemptCompletionTool.tsby removing unnecessarypushToolResult("")call on task completion.pushToolResult("")inAttemptCompletionTool.tswhen task completes successfully to prevent infinite loop.pushToolResult(feedback)is only called when user provides feedback.AttemptCompletionTool.ts.tool_resultwas misused as a stop signal.This description was created by
for 364671d. You can customize this summary. It will automatically update as commits are pushed.