fix: Tool call was added to history without being executed when hitting SubAgent max turn#14703
fix: Tool call was added to history without being executed when hitting SubAgent max turn#14703Amanda111 wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
…ausing tool calls to be added to history without being executed
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @Amanda111, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where tool calls were incorrectly added to the agent's history without being executed when the SubAgent reached its maximum turn limit. The core fix involves reordering the termination check to happen after a turn's execution and introducing a mechanism to append a warning message and allow a final turn for the agent to complete its task with full context, thereby preventing incomplete state issues and improving the agent's ability to resolve tasks under turn constraints. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes an important issue where tool calls on the final turn of a sub-agent were not executed before the max_turns termination. The new approach of checking isLastTurn and allowing one final turn with a warning is a good solution. However, I've identified a critical issue where the new logic for handling max_turns conflicts with the existing generic recovery mechanism, leading to a redundant recovery attempt. This should be addressed before merging. Additionally, the related unit tests need to be updated to reflect these significant logic changes, as the current tests do not catch this new issue.
| if (isLastTurn && turnResult.status === 'continue') { | ||
| // Append warning message to the tool results | ||
| const warningText = this.getFinalWarningMessage( | ||
| AgentTerminateMode.MAX_TURNS, | ||
| ); | ||
| const nextMessageParts = turnResult.nextMessage.parts || []; | ||
| nextMessageParts.push({ text: warningText }); | ||
| currentMessage = { | ||
| role: 'user', | ||
| parts: nextMessageParts, | ||
| }; | ||
|
|
||
| // Execute one more turn with the warning included | ||
| const finalTurnResult = await this.executeTurn( | ||
| chat, | ||
| currentMessage, | ||
| turnCounter++, | ||
| combinedSignal, | ||
| timeoutController.signal, | ||
| ); | ||
|
|
||
| if ( | ||
| finalTurnResult.status === 'stop' && | ||
| finalTurnResult.terminateReason === AgentTerminateMode.GOAL | ||
| ) { | ||
| terminateReason = AgentTerminateMode.GOAL; | ||
| finalResult = finalTurnResult.finalResult; | ||
| } else { | ||
| terminateReason = AgentTerminateMode.MAX_TURNS; | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
This new logic block is a great improvement for handling the max_turns limit, as it ensures the final turn's tool calls are executed. However, it introduces a side effect by conflicting with the existing generic recovery mechanism.
When this block sets terminateReason = AgentTerminateMode.MAX_TURNS and breaks the loop, the code proceeds to the unified recovery block at line 472. This causes executeFinalWarningTurn to run again, resulting in a redundant recovery attempt and an extra, unexpected turn for the agent.
To fix this, the unified recovery block should be modified to exclude the MAX_TURNS case, since it's now handled here. Please update the condition at line 472 to prevent this redundant execution:
// In file packages/core/src/agents/executor.ts at line 472
if (
terminateReason !== AgentTerminateMode.ERROR &&
terminateReason !== AgentTerminateMode.ABORTED &&
terminateReason !== AgentTerminateMode.GOAL &&
terminateReason !== AgentTerminateMode.MAX_TURNS
) {
Amanda111
changed the title
gemini-cli
bot
added
status/need-issue
|
Hi @Amanda111, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
|
Hi there! Thank you for your contribution to Gemini CLI. To improve our contribution process and better track changes, we now require all pull requests to be associated with an existing issue, as announced in our recent discussion and as detailed in our CONTRIBUTING.md. This pull request is being closed because it is not currently linked to an issue. You can easily reopen this PR once you have linked it to an issue. How to link an issue: Thank you for your understanding and for being a part of our community! |
3 participants
Summary
The issue occurred because the termination check happened before executing the last turn, causing tool calls to be added to history without being executed. Now the warning message is appended after the last turn's tools execute, giving the model a final chance to complete with full context.
Details
Changes:
Related Issues
#14088
How to Validate
Hitting SubAgent max turn
Pre-Merge Checklist