fix(acp): Initialize config (#18897)#18898
fix(acp): Initialize config (#18897)#18898jackwotherspoon merged 2 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello @Mervap, 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 critical bug within the ACP integration where the GeminiAgent.config instance was not being initialized. By adding an explicit initialization call, the change prevents inconsistent behavior and ensures the configuration is always in a valid state when accessed by various integration methods. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request correctly identifies and fixes an issue where the GeminiAgent's configuration was not being initialized. By adding await this.config.initialize() in the initialize method, you ensure that the agent's configuration is properly set up before it's used. However, this change reveals a potential race condition in Config.initialize() related to how it handles interactive mode, which could lead to the agent operating with an incomplete set of tools. This race condition aligns with the principles discussed in our rules regarding robust tool synchronization. I've added a comment with a more detailed explanation.
|
config is initialized here: https://github.com/google-gemini/gemini-cli/blob/main/packages/cli/src/zed-integration/zedIntegration.ts#L305 What is broken? |
|
This is a different config, the session config |
|
The config is usually initialized here – gemini-cli/packages/cli/src/gemini.tsx Line 712 in bb7bb11 But for ACP the early return happens here gemini-cli/packages/cli/src/gemini.tsx Line 667 in bb7bb11 |
|
Seems I have no permissions to merge, even when everything is approved. @sripasg, could you please merge this |
|
I got you covered @Mervap , in the queue now 👍 |
Summary
Fix non-initialized Config in ACP integration
Details
There is a GeminiAgent.config instance that is used across different ACP integration methods. However, this config is never initialized, which leads to inconsistent behavior
Related Issues
Fixes #18897
Pre-Merge Checklist