feat(version): add CLI version check on extension startup#9
Conversation
Check daemon CLI version against server API when extension starts.
If a newer version is available, show warning notification with
update command: curl -sSL {webEndpoint}/i | bash
- Add VersionChecker class with HTTP client and notifications
- Read apiEndpoint and webEndpoint from config file
- Show warning only once per session
- Add "Copy Update Command" action to notification
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, 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 introduces a new feature that automatically checks the installed ShellTime CLI daemon's version when the JetBrains extension starts. It proactively informs users if their CLI is outdated and provides an easy way to update it, enhancing the user experience by ensuring compatibility and access to the latest features. The implementation also allows for custom API and web endpoints to be specified in the configuration. 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 introduces a CLI version check on extension startup, which is a great enhancement for keeping users on the latest version. The implementation is mostly solid, using coroutines for background work and showing a non-intrusive notification.
My review includes a few suggestions:
- A fix for a race condition in the version check logic to prevent multiple notifications from appearing when multiple projects are open.
- A recommendation to use the IDE's
CopyPasteManagerfor better integration when copying text to the clipboard. - A suggestion to improve error handling by logging exceptions instead of silently ignoring them, which will help with future debugging.
- A point about a potentially unused and inefficient function in the config loader.
Overall, these are good changes that improve the user experience.
| @Volatile | ||
| private var hasShownWarning = false |
There was a problem hiding this comment.
The comment "Version warning already shown this session" implies this flag should be shared across all VersionChecker instances to prevent multiple notifications when multiple projects are open. Currently, it's an instance variable.
Furthermore, the current implementation has a race condition. If checkVersion is called concurrently (e.g., from multiple projects opening), multiple threads could pass the if (hasShownWarning) check before one of them sets it to true, leading to multiple notifications. @Volatile ensures visibility but not atomicity for the check-then-set operation.
To fix both issues, you can move hasShownWarning to a companion object and use java.util.concurrent.atomic.AtomicBoolean to ensure atomicity.
- Change the declaration to be in a companion object:
companion object { private val hasShownWarning = java.util.concurrent.atomic.AtomicBoolean(false) }
- Update the check in
checkVersion:if (hasShownWarning.get()) { ... }
- Update the set operation inside the coroutine to be atomic:
if (result != null && !result.isLatest) { if (hasShownWarning.compareAndSet(false, true)) { withContext(Dispatchers.Main) { showUpdateWarning(currentVersion, result.latestVersion, project) } } }
| fun getFileConfig(): ShellTimeFileConfig? { | ||
| val configFile = findConfigFile() ?: return null | ||
| return loadConfigFile(configFile) | ||
| } |
There was a problem hiding this comment.
This new function getFileConfig appears to be unused within the scope of this pull request. Furthermore, it bypasses the caching mechanism implemented in getConfig(), leading to inefficient file I/O and parsing on every call. The properties it seems intended to expose (apiEndpoint and webEndpoint) are already merged into ShellTimeConfig and available via the cached getConfig() method. If this function is not needed, it should be removed. If it is needed for some other purpose, it should be refactored to leverage the existing caching logic.
| } catch (e: Exception) { | ||
| // Silently ignore - version check is optional | ||
| } |
There was a problem hiding this comment.
While the version check is an optional feature, completely swallowing exceptions without any logging can make it very difficult to diagnose potential issues. For example, problems with socketClient.getStatus() would go unnoticed. It's better to log the exception, at least at a debug level, to aid in future debugging. You can use the Logger utility class from xyz.shelltime.jetbrains.utils.Logger for this.
| val clipboard = java.awt.Toolkit.getDefaultToolkit().systemClipboard | ||
| clipboard.setContents(java.awt.datatransfer.StringSelection(updateCommand), null) |
There was a problem hiding this comment.
It's better to use the IntelliJ Platform's CopyPasteManager for clipboard operations. It's more integrated with the IDE's clipboard handling and is the recommended approach for plugins.
You'll need to add this import:
import com.intellij.openapi.ide.CopyPasteManager
| val clipboard = java.awt.Toolkit.getDefaultToolkit().systemClipboard | |
| clipboard.setContents(java.awt.datatransfer.StringSelection(updateCommand), null) | |
| com.intellij.openapi.ide.CopyPasteManager.getInstance().setContents(java.awt.datatransfer.StringSelection(updateCommand)) |
Code Review: CLI Version Check FeatureThank you for this implementation! The feature adds valuable functionality to notify users about available CLI updates. Below is my detailed review: ✅ Strengths
🐛 Potential Bugs & Issues1. Critical: Config Merging Issue in
|
1 participant
Summary
apiEndpointandwebEndpointfrom config fileTest plan
apiEndpointandwebEndpointin~/.shelltime/config.toml🤖 Generated with Claude Code