-
Notifications
You must be signed in to change notification settings - Fork 0
feat(version): add CLI version check on extension startup #9
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ import com.intellij.openapi.vfs.VirtualFile | |
| import xyz.shelltime.jetbrains.heartbeat.HeartbeatCollector | ||
| import xyz.shelltime.jetbrains.heartbeat.HeartbeatSender | ||
| import xyz.shelltime.jetbrains.heartbeat.HeartbeatSenderCallback | ||
| import xyz.shelltime.jetbrains.version.VersionChecker | ||
| import kotlinx.coroutines.* | ||
|
|
||
| /** | ||
| * Project-level service for ShellTime | ||
|
|
@@ -25,7 +27,9 @@ class ShellTimeProjectService(private val project: Project) : Disposable { | |
|
|
||
| private lateinit var collector: HeartbeatCollector | ||
| private lateinit var sender: HeartbeatSender | ||
| private var versionChecker: VersionChecker? = null | ||
| private var statusCallback: HeartbeatSenderCallback? = null | ||
| private val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) | ||
|
|
||
| private val editorFactoryListener = object : EditorFactoryListener { | ||
| override fun editorCreated(event: EditorFactoryEvent) { | ||
|
|
@@ -72,6 +76,37 @@ class ShellTimeProjectService(private val project: Project) : Disposable { | |
|
|
||
| // Start the sender | ||
| sender.start() | ||
|
|
||
| // Check CLI version in background (non-blocking) | ||
| checkCliVersion() | ||
| } | ||
|
|
||
| /** | ||
| * Check CLI version against server | ||
| */ | ||
| private fun checkCliVersion() { | ||
| val apiEndpoint = settings.apiEndpoint | ||
| val webEndpoint = settings.webEndpoint | ||
|
|
||
| if (apiEndpoint.isNullOrEmpty() || webEndpoint.isNullOrEmpty()) { | ||
| return | ||
| } | ||
|
|
||
| versionChecker = VersionChecker(apiEndpoint, webEndpoint, settings.debug) | ||
|
|
||
| scope.launch { | ||
| try { | ||
| val socketClient = appService.getSocketClient() | ||
| val status = socketClient.getStatus() | ||
| val daemonVersion = status?.version | ||
|
|
||
| if (daemonVersion != null) { | ||
| versionChecker?.checkVersion(daemonVersion, project) | ||
| } | ||
| } catch (e: Exception) { | ||
| // Silently ignore - version check is optional | ||
| } | ||
|
Comment on lines
+106
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -147,5 +182,7 @@ class ShellTimeProjectService(private val project: Project) : Disposable { | |
| if (::collector.isInitialized) { | ||
| collector.dispose() | ||
| } | ||
| versionChecker?.dispose() | ||
| scope.cancel() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,121 @@ | ||||||||
| package xyz.shelltime.jetbrains.version | ||||||||
|
|
||||||||
| import com.intellij.ide.BrowserUtil | ||||||||
| import com.intellij.notification.NotificationGroupManager | ||||||||
| import com.intellij.notification.NotificationType | ||||||||
| import com.intellij.openapi.project.Project | ||||||||
| import kotlinx.coroutines.* | ||||||||
| import kotlinx.serialization.json.Json | ||||||||
| import xyz.shelltime.jetbrains.config.Constants | ||||||||
| import xyz.shelltime.jetbrains.heartbeat.VersionCheckResponse | ||||||||
| import xyz.shelltime.jetbrains.utils.Logger | ||||||||
| import java.net.HttpURLConnection | ||||||||
| import java.net.URL | ||||||||
| import java.net.URLEncoder | ||||||||
|
|
||||||||
| /** | ||||||||
| * Checks CLI version against the server and notifies user if update is available | ||||||||
| */ | ||||||||
| class VersionChecker( | ||||||||
| private val apiEndpoint: String, | ||||||||
| private val webEndpoint: String, | ||||||||
| debug: Boolean = false | ||||||||
| ) { | ||||||||
| private val logger = Logger("VersionChecker", debug) | ||||||||
| private val json = Json { ignoreUnknownKeys = true } | ||||||||
| private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) | ||||||||
|
|
||||||||
| @Volatile | ||||||||
| private var hasShownWarning = false | ||||||||
|
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment "Version warning already shown this session" implies this flag should be shared across all Furthermore, the current implementation has a race condition. If To fix both issues, you can move
|
||||||||
|
|
||||||||
| /** | ||||||||
| * Check CLI version asynchronously | ||||||||
| */ | ||||||||
| fun checkVersion(currentVersion: String, project: Project?) { | ||||||||
| if (hasShownWarning) { | ||||||||
| logger.log("Version warning already shown this session, skipping") | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| scope.launch { | ||||||||
| try { | ||||||||
| val result = fetchVersionCheck(currentVersion) | ||||||||
| if (result != null && !result.isLatest) { | ||||||||
| hasShownWarning = true | ||||||||
| withContext(Dispatchers.Main) { | ||||||||
| showUpdateWarning(currentVersion, result.latestVersion, project) | ||||||||
| } | ||||||||
| } else if (result != null) { | ||||||||
| logger.log("CLI version $currentVersion is up to date") | ||||||||
| } | ||||||||
| } catch (e: Exception) { | ||||||||
| logger.log("Version check failed: ${e.message}") | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private fun fetchVersionCheck(version: String): VersionCheckResponse? { | ||||||||
| val encodedVersion = URLEncoder.encode(version, "UTF-8") | ||||||||
| val url = URL("$apiEndpoint${Constants.VERSION_CHECK_ENDPOINT}?version=$encodedVersion") | ||||||||
|
|
||||||||
| logger.log("Checking version at: $url") | ||||||||
|
|
||||||||
| val connection = url.openConnection() as HttpURLConnection | ||||||||
| connection.requestMethod = "GET" | ||||||||
| connection.connectTimeout = Constants.VERSION_CHECK_TIMEOUT_MS.toInt() | ||||||||
| connection.readTimeout = Constants.VERSION_CHECK_TIMEOUT_MS.toInt() | ||||||||
| connection.setRequestProperty("Accept", "application/json") | ||||||||
|
|
||||||||
| return try { | ||||||||
| if (connection.responseCode == HttpURLConnection.HTTP_OK) { | ||||||||
| val response = connection.inputStream.bufferedReader().use { it.readText() } | ||||||||
| json.decodeFromString<VersionCheckResponse>(response) | ||||||||
| } else { | ||||||||
| logger.log("Version check HTTP error: ${connection.responseCode}") | ||||||||
| null | ||||||||
| } | ||||||||
| } finally { | ||||||||
| connection.disconnect() | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private fun showUpdateWarning(currentVersion: String, latestVersion: String, project: Project?) { | ||||||||
| val message = "ShellTime CLI update available: $currentVersion -> $latestVersion" | ||||||||
| val updateCommand = "curl -sSL $webEndpoint/i | bash" | ||||||||
|
|
||||||||
| NotificationGroupManager.getInstance() | ||||||||
| .getNotificationGroup(Constants.NOTIFICATION_GROUP_ID) | ||||||||
| .createNotification( | ||||||||
| "ShellTime Update Available", | ||||||||
| "$message<br><br>Run: <code>$updateCommand</code>", | ||||||||
| NotificationType.WARNING | ||||||||
| ) | ||||||||
| .addAction(object : com.intellij.notification.NotificationAction("Copy Update Command") { | ||||||||
| override fun actionPerformed( | ||||||||
| e: com.intellij.notification.AnActionEvent, | ||||||||
| notification: com.intellij.notification.Notification | ||||||||
| ) { | ||||||||
| val clipboard = java.awt.Toolkit.getDefaultToolkit().systemClipboard | ||||||||
| clipboard.setContents(java.awt.datatransfer.StringSelection(updateCommand), null) | ||||||||
|
Comment on lines
+98
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to use the IntelliJ Platform's You'll need to add this import:
Suggested change
|
||||||||
| notification.expire() | ||||||||
|
|
||||||||
| NotificationGroupManager.getInstance() | ||||||||
| .getNotificationGroup(Constants.NOTIFICATION_GROUP_ID) | ||||||||
| .createNotification( | ||||||||
| "Update command copied to clipboard", | ||||||||
| NotificationType.INFORMATION | ||||||||
| ) | ||||||||
| .notify(project) | ||||||||
| } | ||||||||
| }) | ||||||||
| .notify(project) | ||||||||
| } | ||||||||
|
|
||||||||
| fun setDebug(enabled: Boolean) { | ||||||||
| logger.setDebug(enabled) | ||||||||
| } | ||||||||
|
|
||||||||
| fun dispose() { | ||||||||
| scope.cancel() | ||||||||
| } | ||||||||
| } | ||||||||
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.
This new function
getFileConfigappears to be unused within the scope of this pull request. Furthermore, it bypasses the caching mechanism implemented ingetConfig(), leading to inefficient file I/O and parsing on every call. The properties it seems intended to expose (apiEndpointandwebEndpoint) are already merged intoShellTimeConfigand available via the cachedgetConfig()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.