Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/main/kotlin/xyz/shelltime/jetbrains/config/ConfigLoader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,17 @@ class ConfigLoader {
socketPath = fileConfig.socketPath ?: Constants.DEFAULT_SOCKET_PATH,
heartbeatInterval = Constants.DEFAULT_FLUSH_INTERVAL_MS,
debug = false,
exclude = fileConfig.exclude ?: emptyList()
exclude = fileConfig.exclude ?: emptyList(),
apiEndpoint = fileConfig.apiEndpoint,
webEndpoint = fileConfig.webEndpoint
)
}

/**
* Get raw file config for accessing apiEndpoint/webEndpoint
*/
fun getFileConfig(): ShellTimeFileConfig? {
val configFile = findConfigFile() ?: return null
return loadConfigFile(configFile)
}
Comment on lines +177 to +180
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

}
6 changes: 6 additions & 0 deletions src/main/kotlin/xyz/shelltime/jetbrains/config/Constants.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,10 @@ object Constants {

/** Notification group ID */
const val NOTIFICATION_GROUP_ID = "ShellTime"

/** Version check API endpoint path */
const val VERSION_CHECK_ENDPOINT = "/api/v1/cli/version-check"

/** Version check timeout in milliseconds */
const val VERSION_CHECK_TIMEOUT_MS = 5_000L
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,11 @@ data class ShellTimeConfig(
val debug: Boolean = false,

/** Patterns to exclude from tracking */
val exclude: List<String> = emptyList()
val exclude: List<String> = emptyList(),

/** API endpoint for version check */
val apiEndpoint: String? = null,

/** Web endpoint for update command */
val webEndpoint: String? = null
)
10 changes: 10 additions & 0 deletions src/main/kotlin/xyz/shelltime/jetbrains/heartbeat/HeartbeatData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,13 @@ data class StatusResponse(
val platform: String? = null,
val goVersion: String? = null
)

/**
* Response from version check API
*/
@Serializable
data class VersionCheckResponse(
val isLatest: Boolean,
val latestVersion: String,
val version: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

}
}

/**
Expand Down Expand Up @@ -147,5 +182,7 @@ class ShellTimeProjectService(private val project: Project) : Disposable {
if (::collector.isInitialized) {
collector.dispose()
}
versionChecker?.dispose()
scope.cancel()
}
}
121 changes: 121 additions & 0 deletions src/main/kotlin/xyz/shelltime/jetbrains/version/VersionChecker.kt
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.

  1. Change the declaration to be in a companion object:
    companion object {
        private val hasShownWarning = java.util.concurrent.atomic.AtomicBoolean(false)
    }
  2. Update the check in checkVersion:
    if (hasShownWarning.get()) { ... }
  3. 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)
            }
        }
    }


/**
* 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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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

Suggested change
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))

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()
}
}
Loading