Fix race condition by awaiting scheduleToolCalls#16759
Conversation
Summary of ChangesHello @chrstnb, 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 and resolves a race condition related to the scheduling of tool calls. By transitioning the 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
|
|
Hi @chrstnb, 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! |
|
Size Change: +12 B (0%) Total Size: 23.1 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a race condition by making scheduleToolCalls asynchronous and awaiting its promise. The changes are consistently applied across the affected files, including type definitions and test updates. My review includes one high-severity comment regarding a potential unhandled promise rejection that could occur if scheduleToolCalls rejects, which has been kept as it aligns with good error handling practices.
cda9b9b to
b7f26a5
Compare
|
/patch |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
❌ Patch Release Failed! 📋 Details:
🔍 Next Steps:
🔗 Troubleshooting: |
|
❌ Patch Release Failed! 📋 Details:
🔍 Next Steps:
🔗 Troubleshooting: |
1 similar comment
|
❌ Patch Release Failed! 📋 Details:
🔍 Next Steps:
🔗 Troubleshooting: |
|
❌ Patch Release Failed! 📋 Details:
🔍 Next Steps:
🔗 Troubleshooting: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
|
Thanks for catching this race condition with We discovered a related race condition in the same area and opened #16146 to fix it. Our fix awaits Your fix (awaiting Let's coordinate on merge order to avoid conflicts. Happy to rebase on top of yours once it lands, or we can combine both fixes into one PR if you prefer. |
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
Summary
We've been seeing a lot of errors like this: #16415
This was caused by sending requests that looked something like this:
Note that we are providing the function response to the first
write_filecall twice.My theory is that this is happening a) only in yolo mode and b) when a user queues a message via the input prompt while the tool calls are still running.
This meant the main chat loop thought it was "done" (Idle) for a split second while the tool was still getting ready to run. Because the system briefly reported "Idle", the queued message was immediately grabbed and sent. This started a new conversation turn while the old tool turn was still happening, causing the model to get confused and the history to be fragmented.
I changed the code to await the tool scheduling. Now, the system explicitly says "I am busy (Responding)" until the tool has been fully scheduled.
Details
Related Issues
How to Validate
In order to validate, prompt the CLI with this prompt:
Then, while it's running tool calls, enqueue a message.
Pre-Merge Checklist