-
Notifications
You must be signed in to change notification settings - Fork 8k
[ide-mode] Write port to file in ide-server #5811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @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!
I've implemented a change to address a race condition that can occur during the startup or installation of the Gemini CLI, where the CLI might attempt to load before the IDE server's port is fully established. My solution involves persisting the IDE server's port number to a dedicated file within the workspace. This allows the CLI to reliably retrieve the port, ensuring a smoother and more robust initialization process for users. Additionally, I've included proper cleanup to remove this temporary file when the server is no longer active.
Highlights
- Port Persistence: I've introduced a mechanism to write the IDE server's dynamically assigned port to a file (
.gemini/ide-server-port
) within the user's workspace when the server starts. - Race Condition Mitigation: To prevent potential race conditions during startup or installation, the Gemini CLI can now read the port from this file, ensuring it always has the correct port information.
- Resource Cleanup: I've also added cleanup logic to ensure that the port file is automatically removed when the IDE server is disposed of, maintaining a clean workspace.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a mechanism to write the IDE server port to a file to resolve a race condition with the Gemini CLI. My review focuses on improving the error handling for these new file operations. I've identified two high-severity issues where failures are handled silently, which could lead to a poor user experience and difficult debugging. My suggestions ensure that errors are appropriately reported to the user or logged for diagnostics.
also can we update the notification we show at startup to just say the extension installed successfully: https://github.com/google-gemini/gemini-cli/blob/main/packages/vscode-ide-companion/src/extension.ts#L88C65-L90C8 |
Thanks for the suggestion, @skeshive! The change you're requesting for the startup notification is in It would be best to address the notification message update in a separate pull request. This helps keep PRs focused and makes reviews easier. |
actually nvm, we should remove the notification when we make the corresponding CLI changes to read the file. |
Thanks for the update, @skeshive! That sounds like a good plan to remove the notification once the CLI changes are in place to read the file. Appreciate you keeping this PR focused. |
/gemini review |
Code Coverage Summary
CLI Package - Full Text Report
Core Package - Full Text Report
For detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a mechanism to write the IDE server port to a file, which resolves a race condition during startup. The client is updated to first try reading the port from this file before falling back to the environment variable, ensuring backward compatibility. The changes look good and address the described problem effectively. I've added a couple of suggestions to improve the implementation's robustness and performance: one to use asynchronous file I/O instead of synchronous to avoid blocking the event loop, and another to replace an unbounded while
loop with a bounded for
loop to prevent potential hangs.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…nto cb/write-to-file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll approve once this code has tests. Likely an integration test is the right way to test this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
TLDR
This will allow us to dynamically read the port for the IDE server.
Currently there is a race condition where, on startup, if gemini CLI loads before the port is written to, the user needs to open a new terminal. This is similarly an issue on install.
This implementation is backwards-compatible because it writes the port to both the tmpdir file and to the env variable.
The IDE client will first try to fetch + connect to a port from the file (if one exists). If that fails, it will fall back on the environment variable.
NOTE: should we want to remove the environment variable, we will need to update the sandboxing approach. I don't see any immediate need to do that, however.
The tmpdir file is tied to the vs code process ID via the name of the file. This will get deleted when the server stops running. This ensures that gemini CLI can read the associated VSCode window's process ID from the integrated terminal, separating the process for the current window from other vs code windows.
Dive Deeper
portfile.mov
Reviewer Test Plan
Run the extension locally, then run gemini CLI in the extension host. Confirm it's able to connect.
Testing Matrix
Linked issues / bugs