feat(extensions): show package.json version alongside tag in extension info#23105
feat(extensions): show package.json version alongside tag in extension info#23105Br1an67 wants to merge 1 commit intogoogle-gemini:mainfrom
Conversation
…n info When extensions use non-descriptive version strings like 'latest' in their gemini-extension.json, the list and update commands now also read the package.json version and display both values, e.g. 'latest (0.20.2)'. This enriches version display in: - /extensions list output - /extensions update success messages - CLI 'gemini extensions list' and 'gemini extensions update' commands Closes google-gemini#22932
Summary of ChangesHello, 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 significantly improves the clarity of extension version reporting within the CLI. By integrating 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. Footnotes
|
gemini-cli
bot
added
the
area/extensions
![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 introduces a valuable feature to display the package.json version alongside the configured extension version, enhancing user information. A security audit found no exploitable high or critical vulnerabilities, with functions like getPackageVersion and formatVersion handling data safely. However, there's a potential efficiency concern due to synchronous file system access within the getPackageVersion function, which could lead to performance degradation when listing or updating multiple extensions, as highlighted by the comments referencing the rule against blocking I/O.
| .map((extension) => `${extension.name} (${extension.version})`) | ||
| .map( | ||
| (extension) => | ||
| `${extension.name} (${formatVersion(extension.version, getPackageVersion(extension.path))})`, |
There was a problem hiding this comment.
The call to getPackageVersion(extension.path) within this .map function performs a synchronous file read for each installed extension. Performing synchronous I/O operations in a loop can block the Node.js event loop and lead to performance degradation, especially when a large number of extensions are installed.
Suggestion:
Consider optimizing the retrieval of package.json versions. The most robust solution would be to load and store the packageVersion directly on the GeminiCLIExtension object during its initial loading (e.g., in _buildExtension in extension-manager.ts). If modifying the GeminiCLIExtension interface is not feasible within this change, an alternative could be to make getPackageVersion asynchronous and use Promise.all to fetch all versions concurrently, or implement a caching mechanism for getPackageVersion results if the same extension.path might be queried multiple times.
References
- Use asynchronous file system operations (e.g.,
fs.promises.readFile) instead of synchronous ones (e.g.,fs.readFileSync) to avoid blocking the event loop.
|
|
||
| const status = workspaceEnabled ? chalk.green('✓') : chalk.red('✗'); | ||
| let output = `${status} ${extension.name} (${extension.version})`; | ||
| let output = `${status} ${extension.name} (${formatVersion(extension.version, getPackageVersion(extension.path))})`; |
There was a problem hiding this comment.
Similar to the previous comment, the toOutputString method calls getPackageVersion(extension.path) for each extension. This also involves synchronous file system access in a potentially iterated context (e.g., when listing all extensions). This can contribute to performance issues due to blocking I/O, particularly if many extensions are displayed.
Suggestion:
Refer to the suggestion for packages/cli/src/commands/extensions/update.ts. Ideally, the packageVersion should be loaded once and stored on the GeminiCLIExtension object. If that's not possible, explore asynchronous file reading with Promise.all or a caching strategy for getPackageVersion to mitigate the impact of synchronous I/O.
References
- Use asynchronous file system operations (e.g.,
fs.promises.readFile) instead of synchronous ones (e.g.,fs.readFileSync) to avoid blocking the event loop.
| const originalVersion = extension.version; | ||
| const originalVersion = formatVersion( | ||
| extension.version, | ||
| getPackageVersion(extension.path), |
There was a problem hiding this comment.
The updateExtension function calls getPackageVersion(extension.path) twice: once for originalVersion and once for updatedVersion. While this function is typically called for a single extension at a time, the synchronous nature of fs.readFileSync within getPackageVersion still introduces blocking I/O. If updateExtension were to be called in a batch process (e.g., for multiple extensions in parallel), this could compound the performance impact.
Suggestion:
While the impact here might be less severe than in listing scenarios, it's still a synchronous file read. The long-term solution remains to store the packageVersion on the GeminiCLIExtension object. For immediate improvement, if getPackageVersion were made asynchronous, these calls would need to be awaited.
References
- Use asynchronous file system operations (e.g.,
fs.promises.readFile) instead of synchronous ones (e.g.,fs.readFileSync) to avoid blocking the event loop.
| const updatedVersion = updatedExtension.version; | ||
| const updatedVersion = formatVersion( | ||
| updatedExtension.version, | ||
| getPackageVersion(updatedExtension.path), |
There was a problem hiding this comment.
This is another instance where getPackageVersion(updatedExtension.path) is called synchronously. As noted in previous comments, repeated synchronous file I/O can lead to performance bottlenecks and is generally an anti-pattern in Node.js for operations that might involve multiple items.
Suggestion:
Refer to the suggestion for packages/cli/src/commands/extensions/update.ts. The long-term solution remains to store the packageVersion on the GeminiCLIExtension object. For immediate improvement, if getPackageVersion were made asynchronous, these calls would need to be awaited.
References
- Use asynchronous file system operations (e.g.,
fs.promises.readFile) instead of synchronous ones (e.g.,fs.readFileSync) to avoid blocking the event loop.
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383). We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'. This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community! |
|
@Br1an67, apologies for the bot closing this PR! We have reopened it. Please sync your branch to the latest |
gemini-cli
bot
added
the
help wanted
|
@Br1an67 - I would greatly appreciate your completing this PR, if you can spare the time. Thanks, Lester |
3 participants
Summary
When extensions use non-descriptive version strings like
"latest"in theirgemini-extension.json, the/extensions listand/extensions updatecommands now also display the concrete version frompackage.json. For example:latest (0.20.2)instead of justlatest.Details
Added two utility functions in
extension.ts:getPackageVersion(extensionDir)— reads theversionfield from an extension'spackage.json, returningundefinedif unavailable.formatVersion(configVersion, packageVersion?)— returns just the config version when it matches the package version (or when no package version exists), otherwise combines them:configVersion (packageVersion).These are applied in:
extension-manager.ts—toOutputString()for/extensions listconfig/extensions/update.ts—originalVersion/updatedVersioninExtensionUpdateInfocommands/extensions/update.ts— "extension not found" error message listingWhen config version equals package version (i.e., the author already uses proper semver), the display is unchanged.
Related Issues
Closes #22932
How to Validate
"latest"as its version ingemini-extension.json(e.g.chrome-devtools-mcp)./extensions list— the output should now showlatest (x.y.z)instead of justlatest./extensions update <name>— the success message should show meaningful version transitions likelatest (0.20.0) → latest (0.20.2)instead oflatest → latest.1.0.0) — verify the display remains unchanged (no duplicate version).Pre-Merge Checklist