From ead613625ccc658b82ee839090b670395b6c079f Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 27 Aug 2026 23:05:14 +0300 Subject: [PATCH 1/4] chore: upgrade TBX API version to latest 1.13.87111 This drops support for Toolbox versions older than 3.7.2 but instead provides new APIs that can give better control and insight to the Coder plugin. --- CHANGELOG.md | 4 ++++ gradle/libs.versions.toml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c280f0e..2962f02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Changed + +- upgraded the Toolbox plugin API, dropping support for Toolbox versions older than 3.7.2 + ## 0.9.4 - 2026-08-26 ### Added diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c8e197f..3328177 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,5 +1,5 @@ [versions] -toolbox-plugin-api = "1.10.76281" +toolbox-plugin-api = "1.13.87111" kotlin = "2.3.10" coroutines = "1.10.2" serialization = "1.9.0" From a2d960db3e691e94da2cb9cc38450474c96f0ae5 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 27 Aug 2026 23:39:55 +0300 Subject: [PATCH 2/4] Add Toolbox session ID registry Keep one generated session ID for each workspace and agent pair so SSH reconnects share the same correlation value. Remove the entry only when Toolbox disposes the environment, allowing a later environment to begin a new session. --- .../toolbox/session/SessionIdRegistry.kt | 74 ++++++++++++++++ .../toolbox/session/SessionIdRegistryTest.kt | 88 +++++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt create mode 100644 src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt diff --git a/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt b/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt new file mode 100644 index 0000000..789faa6 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt @@ -0,0 +1,74 @@ +package com.coder.toolbox.session + +import com.coder.toolbox.util.toHex +import java.security.SecureRandom +import java.util.concurrent.ConcurrentHashMap + +private const val SESSION_ID_BYTE_LENGTH = 16 +private val SESSION_ID_PATTERN = Regex("^[0-9a-f]{32}$") + +/** + * Identifies one client-managed connection session. + * + * Session IDs are 16-byte values encoded as 32 lowercase hexadecimal characters. + */ +@JvmInline +value class SessionId private constructor(val value: String) { + init { + require(SESSION_ID_PATTERN.matches(value)) { "Session ID must be a 32-character lowercase hexadecimal string" } + } + + override fun toString(): String = value + + companion object { + internal fun generate(): SessionId { + val bytes = ByteArray(SESSION_ID_BYTE_LENGTH) + SecureRandomHolder.instance.nextBytes(bytes) + return SessionId(bytes.toHex()) + } + } +} + +private object SecureRandomHolder { + val instance = SecureRandom() +} + +private data class SessionKey( + val workspaceName: String, + val agentName: String, +) + +/** + * Process-local registry of active connection sessions. + * + * A session is keyed only by workspace and agent names. Call [startSession] from the initial SSH + * connection path; all other code should use [findSession] so observing a session cannot create one. + * Entries intentionally remain across SSH disconnects and reconnects. Call [removeSession] only + * when the Toolbox environment that owns the session is disposed. + */ +object SessionIdRegistry { + private val sessionIds = ConcurrentHashMap() + + /** + * Returns the active session ID for this workspace and agent, creating it when absent. + * + * Reusing an existing ID allows transient reconnects to remain part of the same session. + */ + fun startSession(workspaceName: String, agentName: String): SessionId = + sessionIds.computeIfAbsent(SessionKey(workspaceName, agentName)) { SessionId.generate() } + + /** Returns the active session ID without creating a session. */ + fun findSession(workspaceName: String, agentName: String): SessionId? = + sessionIds[SessionKey(workspaceName, agentName)] + + /** + * Removes the session when its owning Toolbox environment is disposed. + * + * This must only be called from the environment disposal lifecycle, such as + * `RemoteEnvironment.dispose()`, when Toolbox removes or destroys that environment. It must + * not be called when an IDE closes, the SSH transport disconnects, or the SSH transport reconnects; + * those events remain part of the same Toolbox session. + */ + fun removeSession(workspaceName: String, agentName: String): SessionId? = + sessionIds.remove(SessionKey(workspaceName, agentName)) +} diff --git a/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt new file mode 100644 index 0000000..924322c --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt @@ -0,0 +1,88 @@ +package com.coder.toolbox.session + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.test.runTest +import java.util.UUID +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class SessionIdRegistryTest { + @Test + fun `start session creates a correctly encoded id`() { + val key = uniqueKey() + val sessionId = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + + assertTrue(sessionId.value.matches(Regex("^[0-9a-f]{32}$"))) + } + + @Test + fun `start session reuses the active id for the same workspace and agent`() { + val key = uniqueKey() + val first = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + val second = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + + assertEquals(first, second) + assertEquals(first, SessionIdRegistry.findSession(key.workspaceName, key.agentName)) + } + + @Test + fun `workspace and agent names both participate in the key`() { + val suffix = UUID.randomUUID().toString() + val workspaceOne = "workspace-one-$suffix" + val workspaceTwo = "workspace-two-$suffix" + val agentOne = "agent-one-$suffix" + val agentTwo = "agent-two-$suffix" + val first = SessionIdRegistry.startSession(workspaceOne, agentOne) + val differentWorkspace = SessionIdRegistry.startSession(workspaceTwo, agentOne) + val differentAgent = SessionIdRegistry.startSession(workspaceOne, agentTwo) + + assertNotEquals(first, differentWorkspace) + assertNotEquals(first, differentAgent) + } + + @Test + fun `finding a missing session does not create one`() { + val key = uniqueKey() + + assertNull(SessionIdRegistry.findSession(key.workspaceName, key.agentName)) + } + + @Test + fun `disposing an environment removes its session`() { + val key = uniqueKey() + val disposedSession = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + + assertEquals(disposedSession, SessionIdRegistry.removeSession(key.workspaceName, key.agentName)) + assertNull(SessionIdRegistry.findSession(key.workspaceName, key.agentName)) + + val replacementSession = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + assertNotEquals(disposedSession, replacementSession) + } + + @Test + fun `concurrent starts create only one session`() = runTest { + val key = uniqueKey() + val sessions = List(100) { + async(Dispatchers.Default) { + SessionIdRegistry.startSession(key.workspaceName, key.agentName) + } + }.awaitAll() + + assertEquals(1, sessions.toSet().size) + } + + private fun uniqueKey(): TestSessionKey { + val suffix = UUID.randomUUID().toString() + return TestSessionKey("workspace-$suffix", "agent-$suffix") + } + + private data class TestSessionKey( + val workspaceName: String, + val agentName: String, + ) +} From 684c1b409c09ce5ee22e37f0a355bef4819bd6ec Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 27 Aug 2026 23:57:21 +0300 Subject: [PATCH 3/4] Add session-aware Toolbox logging Add one logger wrapper that preserves existing logging calls and lets callers attach a connection session ID when a message belongs to a workspace session. Keep the existing log-and-show behavior in the same wrapper so messages are logged before they are displayed to the user. --- .../coder/toolbox/diagnostics/CoderLogger.kt | 61 +++++++++++++++++++ .../toolbox/diagnostics/CoderLoggerTest.kt | 57 +++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt create mode 100644 src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt new file mode 100644 index 0000000..8bbbdf6 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt @@ -0,0 +1,61 @@ +package com.coder.toolbox.diagnostics + +import com.coder.toolbox.session.SessionId +import com.jetbrains.toolbox.api.core.diagnostics.Logger + +private const val CLIENT_SESSION_ID_LOG_KEY = "client_session_id" + +/** + * The plugin's single logging entry point. + * + * Calls without a [SessionId] are delegated unchanged. Calls with a session ID add the correlation + * field to the log message. + */ +class CoderLogger( + private val delegate: Logger, + private val showInfoPopup: (title: String, text: String) -> Unit, +) : Logger by delegate { + fun error(sessionId: SessionId, message: String) { + delegate.error(withSessionId(sessionId, message)) + } + + fun warn(sessionId: SessionId, message: String) { + delegate.warn(withSessionId(sessionId, message)) + } + + fun debug(sessionId: SessionId, message: String) { + delegate.debug(withSessionId(sessionId, message)) + } + + fun info(sessionId: SessionId, message: String) { + delegate.info(withSessionId(sessionId, message)) + } + + fun logAndShowError(title: String, error: String) { + error(error) + showInfoPopup(title, error) + } + + fun logAndShowError(title: String, error: String, exception: Throwable) { + error(exception, error) + showInfoPopup(title, error) + } + + fun logAndShowWarning(title: String, warning: String) { + warn(warning) + showInfoPopup(title, warning) + } + + fun logAndShowWarning(title: String, warning: String, exception: Throwable) { + warn(exception, warning) + showInfoPopup(title, warning) + } + + fun logAndShowInfo(title: String, info: String) { + info(info) + showInfoPopup(title, info) + } + + private fun withSessionId(sessionId: SessionId, message: String): String = + "$CLIENT_SESSION_ID_LOG_KEY=$sessionId $message" +} diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt new file mode 100644 index 0000000..753231f --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -0,0 +1,57 @@ +package com.coder.toolbox.diagnostics + +import com.coder.toolbox.session.SessionId +import com.jetbrains.toolbox.api.core.diagnostics.Logger +import io.mockk.mockk +import io.mockk.verify +import kotlin.test.Test + +class CoderLoggerTest { + private val delegate = mockk(relaxed = true) + private val showInfoPopup = mockk<(String, String) -> Unit>(relaxed = true) + private val logger = CoderLogger(delegate, showInfoPopup) + private val sessionId = SessionId.generate() + private val prefix = "client_session_id=$sessionId" + + @Test + fun `sessionless logs are delegated unchanged`() { + val exception = IllegalStateException("failed") + + logger.info("connected") + logger.error(exception, "connection failed") + + verify(exactly = 1) { delegate.info("connected") } + verify(exactly = 1) { delegate.error(exception, "connection failed") } + } + + @Test + fun `session-aware logs include the client session id`() { + logger.error(sessionId, "error") + logger.warn(sessionId, "warning") + logger.debug(sessionId, "debug") + logger.info(sessionId, "info") + + verify(exactly = 1) { delegate.error("$prefix error") } + verify(exactly = 1) { delegate.warn("$prefix warning") } + verify(exactly = 1) { delegate.debug("$prefix debug") } + verify(exactly = 1) { delegate.info("$prefix info") } + } + + @Test + fun `log and show logs and displays the same user message`() { + logger.logAndShowInfo("Connection ready", "Connected to the workspace") + + verify(exactly = 1) { delegate.info("Connected to the workspace") } + verify(exactly = 1) { showInfoPopup("Connection ready", "Connected to the workspace") } + } + + @Test + fun `sessionless log and show remains unchanged`() { + val exception = IllegalStateException("failed") + + logger.logAndShowError("Connection failed", "Could not connect", exception) + + verify(exactly = 1) { delegate.error(exception, "Could not connect") } + verify(exactly = 1) { showInfoPopup("Connection failed", "Could not connect") } + } +} From fef060b91201ea31f918148ad5f9424ba78d8a4f Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 28 Aug 2026 00:11:58 +0300 Subject: [PATCH 4/4] Route plugin logging through CoderLogger Expose the Coder logger from the shared plugin context and use it for existing log-and-show calls. Keep popup creation and error handling inside the logger so callers use one place for logging and user notifications. --- .../com/coder/toolbox/CoderRemoteProvider.kt | 32 +++++----- .../com/coder/toolbox/CoderToolboxContext.kt | 62 +------------------ .../coder/toolbox/diagnostics/CoderLogger.kt | 42 ++++++++++++- .../com/coder/toolbox/sdk/CoderRestClient.kt | 2 +- .../toolbox/util/CoderProtocolHandler.kt | 34 +++++----- .../util/ConnectionMonitoringService.kt | 2 +- .../com/coder/toolbox/views/CoderPage.kt | 2 +- .../com/coder/toolbox/views/ConnectStep.kt | 4 +- .../coder/toolbox/CoderRemoteProviderTest.kt | 8 ++- .../toolbox/diagnostics/CoderLoggerTest.kt | 29 +++++++-- .../toolbox/feed/IdeFeedManagerOfflineTest.kt | 4 +- .../coder/toolbox/feed/IdeFeedManagerTest.kt | 4 +- .../util/ConnectionMonitoringServiceTest.kt | 25 +++++--- 13 files changed, 131 insertions(+), 119 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index 056f2c2..5e0c228 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -171,7 +171,7 @@ class CoderRemoteProvider( if ((ex is APIResponseException && ex.isTokenExpired) || ex is OAuthTokenResponseException) { close() context.envPageManager.showPluginEnvironmentsPage(false) - context.logAndShowError( + context.logger.logAndShowError( "Error encountered while setting up Coder", "Your Coder session has expired. Please re-authenticate and try again.", ex @@ -242,7 +242,7 @@ class CoderRemoteProvider( if (!isSshConfigurationWarningShown) { isSshConfigurationWarningShown = true val reason = ex.message?.takeIf { it.isNotBlank() } ?: ex.javaClass.simpleName - context.logAndShowWarning( + context.logger.logAndShowWarning( SSH_CONFIGURATION_WARNING_TITLE, "Workspaces remain available, but SSH connections are unavailable: $reason. " + "Update ${context.settingsStore.sshConfigPath} and try again.", @@ -428,7 +428,7 @@ class CoderRemoteProvider( val params = uri.toQueryParameters() if (params.isEmpty()) { // probably a plugin installation scenario - context.logAndShowInfo("URI will not be handled", "No query parameters were provided") + context.logger.logAndShowInfo("URI will not be handled", "No query parameters were provided") return } context.logger.info("Handling $uri...") @@ -469,7 +469,7 @@ class CoderRemoteProvider( ex.reason } else ex.message } else ex.message - context.logAndShowError( + context.logger.logAndShowError( "Error encountered while handling Coder URI", textError ?: "" ) @@ -487,35 +487,35 @@ class CoderRemoteProvider( val error = params["error"] if (error != null) { val description = params["error_description"]?.let { " - $it" } ?: "" - return context.logAndShowError( + return context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "OAuth2 authorization error: $error$description" ) } if (!router.hasActiveWizard) { - return context.logAndShowError( + return context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "OAuth2 callback arrived but the setup wizard is no longer active" ) } - val pendingOAuthConnection = router.pendingOAuthConnection ?: return context.logAndShowError( + val pendingOAuthConnection = router.pendingOAuthConnection ?: return context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "OAuth2 callback arrived but no OAuth session was started" ) params["state"]?.takeIf { it == pendingOAuthConnection.session.state } - ?: return context.logAndShowError( + ?: return context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "Server responded back with an invalid state that does not match the initial authorization state sent to the server" ) - val code = params["code"] ?: return context.logAndShowError( + val code = params["code"] ?: return context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "OAuth2 server did not respond back with an access token" ) // before going forward we check to make sure OAuth is not disabled in the meantime if (!context.settingsStore.preferOAuth2IfAvailable) { - context.logAndShowError( + context.logger.logAndShowError( FAILED_TO_HANDLE_OAUTH2_TITLE, "OAuth based authentication is not enabled for Coder plugin in Toolbox. Please enable it in plugin settings or use the API token instead." ) @@ -545,19 +545,19 @@ class CoderRemoteProvider( context.envPageManager.showPluginEnvironmentsPage(false) context.ui.showUiPage(wizard) } catch (e: Exception) { - context.logAndShowError("OAuth Error", "Exception during token exchange: ${e.message}", e) + context.logger.logAndShowError("OAuth Error", "Exception during token exchange: ${e.message}", e) } } private suspend fun resolveDeploymentUrl(params: Map): String? { val deploymentURL = params.url() ?: askUrl() if (deploymentURL.isNullOrBlank()) { - context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"${URL}\" is missing from URI") + context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"${URL}\" is missing from URI") return null } val validationResult = deploymentURL.validateStrictWebUrl() if (validationResult is Invalid) { - context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "\"$URL\" is invalid: ${validationResult.reason}") + context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "\"$URL\" is invalid: ${validationResult.reason}") return null } return deploymentURL @@ -566,7 +566,7 @@ class CoderRemoteProvider( private suspend fun resolveToken(params: Map): String? { val token = params.token() if (token.isNullOrBlank()) { - context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"$TOKEN\" is missing from URI") + context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"$TOKEN\" is missing from URI") return null } return token @@ -647,7 +647,7 @@ class CoderRemoteProvider( onTokenRefreshed = ::onTokenRefreshed, ) } catch (ex: Exception) { - context.logAndShowError( + context.logger.logAndShowError( "Error encountered while setting up Coder", "Failed to set up Coder: ${ex.message}", ex @@ -756,7 +756,7 @@ class CoderRemoteProvider( try { handleLink(params, deploymentUrl, client, cli) } catch (ex: Exception) { - context.logAndShowError( + context.logger.logAndShowError( "Error handling deferred link", ex.message ?: "" ) diff --git a/src/main/kotlin/com/coder/toolbox/CoderToolboxContext.kt b/src/main/kotlin/com/coder/toolbox/CoderToolboxContext.kt index c64823e..2aeda7a 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderToolboxContext.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderToolboxContext.kt @@ -1,5 +1,6 @@ package com.coder.toolbox +import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.store.CoderSecretsStore import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.util.ConnectionMonitoringService @@ -14,10 +15,7 @@ import com.jetbrains.toolbox.api.remoteDev.states.EnvironmentStateColorPalette import com.jetbrains.toolbox.api.remoteDev.ui.EnvironmentUiPageManager import com.jetbrains.toolbox.api.ui.ToolboxUi import com.jetbrains.toolbox.api.ui.components.UiComponents -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch import java.net.URL @Suppress("UnstableApiUsage") @@ -30,12 +28,13 @@ data class CoderToolboxContext( val jbClientOrchestrator: ClientHelper, val desktop: LocalDesktopManager, val cs: CoroutineScope, - val logger: Logger, + private val underlyingLogger: Logger, val i18n: LocalizableStringFactory, val settingsStore: CoderSettingsStore, val secrets: CoderSecretsStore, val proxySettings: ToolboxProxySettings, ) { + val logger: CoderLogger = CoderLogger(underlyingLogger, ui, cs, i18n) val connectionMonitoringService: ConnectionMonitoringService = ConnectionMonitoringService(this) /** @@ -54,61 +53,6 @@ data class CoderToolboxContext( ?: settingsStore.defaultURL.toURL() } - fun logAndShowError(title: String, error: String) { - logger.error(error) - showInfoPopup(title, error) - } - - fun logAndShowError(title: String, error: String, exception: Exception) { - logger.error(exception, error) - showInfoPopup(title, error) - } - - fun logAndShowWarning(title: String, warning: String) { - logger.warn(warning) - showInfoPopup(title, warning) - } - - fun logAndShowWarning(title: String, warning: String, exception: Exception) { - logger.warn(exception, warning) - showInfoPopup(title, warning) - } - - fun logAndShowInfo(title: String, info: String) { - logger.info(info) - showInfoPopup(title, info) - } - - /** - * Displays an informational popup on a child of the plugin coroutine scope rather than on - * the caller's coroutine, without waiting for it. - * - * Unlike [ToolboxUi.showSnackbar], a popup is backed by a persistent dialog state: it is - * still rendered once the window becomes visible even if it was requested while the window - * was hidden, it is not silently dropped when several are requested, and dismissing it - * resumes the [ToolboxUi.showInfoPopup] coroutine normally instead of cancelling it. - * - * It is launched fire-and-forget so the caller is not suspended until the user closes the - * popup - the caller (e.g. the URI handler) can run any follow-up code, such as resetting - * the busy state, immediately. The popups are serialized via [popupMutex] so they are - * shown one after another rather than overwriting each other. - */ - fun showInfoPopup(title: String, text: String) { - cs.launch(CoroutineName("popup")) { - try { - ui.showInfoPopup( - i18n.pnotr(title), - i18n.pnotr(text), - i18n.ptrl("OK") - ) - } catch (_: CancellationException) { - // Expected when the plugin scope shuts down while the popup is open. - } catch (ex: Exception) { - logger.error(ex, "Failed to display popup with title '$title'") - } - } - } - fun popupPluginMainPage() { this.ui.showWindow() this.envPageManager.showPluginEnvironmentsPage(false) diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt index 8bbbdf6..8a95293 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt @@ -2,9 +2,18 @@ package com.coder.toolbox.diagnostics import com.coder.toolbox.session.SessionId import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineName +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch private const val CLIENT_SESSION_ID_LOG_KEY = "client_session_id" +private fun withSessionId(sessionId: SessionId, message: String): String = + "$CLIENT_SESSION_ID_LOG_KEY=$sessionId $message" + /** * The plugin's single logging entry point. * @@ -13,7 +22,9 @@ private const val CLIENT_SESSION_ID_LOG_KEY = "client_session_id" */ class CoderLogger( private val delegate: Logger, - private val showInfoPopup: (title: String, text: String) -> Unit, + private val ui: ToolboxUi, + private val cs: CoroutineScope, + private val i18n: LocalizableStringFactory, ) : Logger by delegate { fun error(sessionId: SessionId, message: String) { delegate.error(withSessionId(sessionId, message)) @@ -56,6 +67,31 @@ class CoderLogger( showInfoPopup(title, info) } - private fun withSessionId(sessionId: SessionId, message: String): String = - "$CLIENT_SESSION_ID_LOG_KEY=$sessionId $message" + /** + * Displays an informational popup on a child of the plugin coroutine scope rather than on + * the caller's coroutine, without waiting for it. + * + * Unlike [ToolboxUi.showSnackbar], a popup is backed by a persistent dialog state: it is + * still rendered once the window becomes visible even if it was requested while the window + * was hidden, it is not silently dropped when several are requested, and dismissing it + * resumes the [ToolboxUi.showInfoPopup] coroutine normally instead of cancelling it. + * + * It is launched fire-and-forget so the caller is not suspended until the user closes the + * popup. The caller can run any follow-up work immediately. + */ + private fun showInfoPopup(title: String, text: String) { + cs.launch(CoroutineName("popup")) { + try { + ui.showInfoPopup( + i18n.pnotr(title), + i18n.pnotr(text), + i18n.ptrl("OK") + ) + } catch (_: CancellationException) { + // Expected when the plugin scope shuts down while the popup is open. + } catch (ex: Exception) { + error(ex, "Failed to display popup with title '$title'") + } + } + } } diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index 234aa17..eeb2503 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -401,7 +401,7 @@ open class CoderRestClient( } isInvalidDeploymentDataWarningShown = true - context.logAndShowWarning( + context.logger.logAndShowWarning( INVALID_DEPLOYMENT_DATA_WARNING_TITLE, INVALID_DEPLOYMENT_DATA_WARNING, ex, diff --git a/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt b/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt index b83bc5e..ebb6bde 100644 --- a/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt +++ b/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt @@ -75,7 +75,7 @@ open class CoderProtocolHandler( // poller and wait for the environment to show up before using its id. workspaceRefreshTrigger.trySend(true) if (!waitForEnvironment(environmentId)) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "The environment $environmentId did not become available in time" ) @@ -96,7 +96,7 @@ open class CoderProtocolHandler( private fun resolveWorkspaceName(params: Map): String? { val workspace = params.workspace() if (workspace.isNullOrBlank()) { - context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"$WORKSPACE\" is missing from URI") + context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"$WORKSPACE\" is missing from URI") return null } return workspace @@ -117,7 +117,7 @@ open class CoderProtocolHandler( } if (workspace == null) { val workspaceLabel = if (ownerName == null) workspaceName else "$ownerName/$workspaceName" - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "There is no workspace with name $workspaceLabel on $deploymentURL" ) @@ -135,7 +135,7 @@ open class CoderProtocolHandler( when (workspace.latestBuild.status) { WorkspaceStatus.PENDING, WorkspaceStatus.STARTING -> if (!restClient.waitForReady(workspace)) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "${workspace.name} from $url could not be ready in time" ) @@ -145,7 +145,7 @@ open class CoderProtocolHandler( WorkspaceStatus.STOPPING, WorkspaceStatus.STOPPED, WorkspaceStatus.CANCELING, WorkspaceStatus.CANCELED -> { if (settings.disableAutostart) { - context.logAndShowWarning( + context.logger.logAndShowWarning( CAN_T_HANDLE_URI_TITLE, "${workspace.name} from $url is not running and autostart is disabled" ) @@ -159,7 +159,7 @@ open class CoderProtocolHandler( cli.startWorkspace(WorkspaceAddress.from(workspace)) } } catch (e: Exception) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "${workspace.name} from $url could not be started", e @@ -168,7 +168,7 @@ open class CoderProtocolHandler( } if (!restClient.waitForReady(workspace)) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "${workspace.name} from $url could not be started in time", ) @@ -177,7 +177,7 @@ open class CoderProtocolHandler( } WorkspaceStatus.FAILED, WorkspaceStatus.DELETING, WorkspaceStatus.DELETED -> { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Unable to connect to ${workspace.name} from $url" ) @@ -196,7 +196,7 @@ open class CoderProtocolHandler( try { return getMatchingAgent(params, workspace) } catch (e: IllegalArgumentException) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Can't resolve an agent for workspace ${workspace.name}", e @@ -219,7 +219,7 @@ open class CoderProtocolHandler( .flatten() if (agents.isEmpty()) { - context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "The workspace \"${workspace.name}\" has no agents") + context.logger.logAndShowError(CAN_T_HANDLE_URI_TITLE, "The workspace \"${workspace.name}\" has no agents") return null } @@ -234,13 +234,13 @@ open class CoderProtocolHandler( if (agent == null) { if (!parameters.agentName().isNullOrBlank()) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "The workspace \"${workspace.name}\" does not have an agent with name \"${parameters.agentName()}\"" ) return null } else { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Unable to determine which agent to connect to; \"$AGENT_NAME\" must be set because the workspace \"${workspace.name}\" has more than one agent" ) @@ -257,7 +257,7 @@ open class CoderProtocolHandler( val status = WorkspaceAndAgentStatus.from(workspace, agent) if (!status.ready()) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Agent ${agent.name} for workspace ${workspace.name} is not ready" ) @@ -344,7 +344,7 @@ open class CoderProtocolHandler( bestEap.build } else { if (availableBuilds.isEmpty()) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Can't launch EAP for $productCode because no version is available on $environmentId" ) @@ -368,7 +368,7 @@ open class CoderProtocolHandler( bestRelease.build } else { if (availableBuilds.isEmpty()) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Can't launch Release for $productCode because no version is available on $environmentId" ) @@ -384,7 +384,7 @@ open class CoderProtocolHandler( if (installed.isNotEmpty()) { installed.maxByOrNull { it } } else if (availableBuilds.isEmpty()) { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Can't launch latest installed version for $productCode because there is no version installed nor available for install on $environmentId" ) @@ -408,7 +408,7 @@ open class CoderProtocolHandler( if (availableMatch != null) { availableMatch } else { - context.logAndShowError( + context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "Can't launch $productCode-$buildNumberHint because there is no matching version installed nor available for install on $environmentId" ) diff --git a/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt b/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt index dd24342..4e29954 100644 --- a/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt +++ b/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt @@ -26,7 +26,7 @@ class ConnectionMonitoringService( when { isWorkspaceRunning && isAgentReady && hasConnectionIssue -> { - context.logAndShowWarning( + context.logger.logAndShowWarning( "Unstable connection detected", "Unstable connection between Coder server and workspace detected. Your active sessions may disconnect" ) diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt index b8fb045..8504b16 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt @@ -84,7 +84,7 @@ class Action( ex.reason } else ex.message } else ex.message - context.logAndShowError("Error while running `$description`", textError ?: "", ex) + context.logger.logAndShowError("Error while running `$description`", textError ?: "", ex) } } } diff --git a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt index 9f21a99..4adc782 100644 --- a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt @@ -144,7 +144,7 @@ class ConnectStep( // dispose() must cancel without navigating. Treat these control-flow // cancellations separately so we do not run navigateBack() twice. if (ex.message != USER_HIT_THE_BACK_BUTTON && ex.message != WIZARD_WAS_DISPOSED) { - context.logAndShowError( + context.logger.logAndShowError( "Error encountered while setting up Coder", "Failed to configure $hostName. ${ex.message}", ex @@ -152,7 +152,7 @@ class ConnectStep( navigateBack() } } catch (ex: Exception) { - context.logAndShowError( + context.logger.logAndShowError( "Error encountered while setting up Coder", "Failed to configure $hostName. ${ex.message}", ex diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt index 84560ac..1ba8437 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt @@ -1,6 +1,7 @@ package com.coder.toolbox import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.oauth.TokenEndpointAuthMethod import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.sdk.v2.models.InvalidCoderIdentifierException @@ -46,6 +47,7 @@ class CoderRemoteProviderTest { private lateinit var mockClient: CoderRestClient private lateinit var mockCli: CoderCLIManager private lateinit var mockContext: CoderToolboxContext + private lateinit var mockLogger: CoderLogger private lateinit var remoteProvider: CoderRemoteProvider @BeforeTest @@ -53,8 +55,10 @@ class CoderRemoteProviderTest { mockClient = mockk(relaxed = true) mockCli = mockk(relaxed = true) mockContext = mockk(relaxed = true) + mockLogger = mockk(relaxed = true) val settingsStore = mockk(relaxed = true) every { mockContext.settingsStore } returns settingsStore + every { mockContext.logger } returns mockLogger every { mockClient.url } returns URI("https://coder.example.com").toURL() remoteProvider = CoderRemoteProvider(mockContext) } @@ -97,7 +101,7 @@ class CoderRemoteProviderTest { } val warningText = slot() verify(exactly = 1) { - mockContext.logAndShowWarning( + mockLogger.logAndShowWarning( "SSH configuration could not be updated", capture(warningText), any(), @@ -123,7 +127,7 @@ class CoderRemoteProviderTest { assertTrue(remoteProvider.environments.value is LoadableState.Loading) verify(exactly = 0) { - mockContext.logAndShowWarning( + mockLogger.logAndShowWarning( "SSH configuration could not be updated", any(), any(), diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt index 753231f..555d8fb 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -2,14 +2,21 @@ package com.coder.toolbox.diagnostics import com.coder.toolbox.session.SessionId import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.jetbrains.toolbox.api.localization.LocalizableString +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.ui.ToolboxUi +import io.mockk.coVerify import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlin.test.Test class CoderLoggerTest { private val delegate = mockk(relaxed = true) - private val showInfoPopup = mockk<(String, String) -> Unit>(relaxed = true) - private val logger = CoderLogger(delegate, showInfoPopup) + private val ui = mockk(relaxed = true) + private val i18n = mockk(relaxed = true) + private val logger = CoderLogger(delegate, ui, CoroutineScope(Dispatchers.Unconfined), i18n) private val sessionId = SessionId.generate() private val prefix = "client_session_id=$sessionId" @@ -42,7 +49,15 @@ class CoderLoggerTest { logger.logAndShowInfo("Connection ready", "Connected to the workspace") verify(exactly = 1) { delegate.info("Connected to the workspace") } - verify(exactly = 1) { showInfoPopup("Connection ready", "Connected to the workspace") } + verify(exactly = 1) { i18n.pnotr("Connection ready") } + verify(exactly = 1) { i18n.pnotr("Connected to the workspace") } + coVerify(exactly = 1) { + ui.showInfoPopup( + any(), + any(), + any(), + ) + } } @Test @@ -52,6 +67,12 @@ class CoderLoggerTest { logger.logAndShowError("Connection failed", "Could not connect", exception) verify(exactly = 1) { delegate.error(exception, "Could not connect") } - verify(exactly = 1) { showInfoPopup("Connection failed", "Could not connect") } + coVerify(exactly = 1) { + ui.showInfoPopup( + any(), + any(), + any(), + ) + } } } diff --git a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt index e6eed89..bd13b0d 100644 --- a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt +++ b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerOfflineTest.kt @@ -1,8 +1,8 @@ package com.coder.toolbox.feed import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.store.CoderSettingsStore -import com.jetbrains.toolbox.api.core.diagnostics.Logger import com.squareup.moshi.Moshi import com.squareup.moshi.Types import io.mockk.every @@ -21,7 +21,7 @@ import kotlin.io.path.writeText class IdeFeedManagerOfflineTest { private lateinit var context: CoderToolboxContext private lateinit var settingsStore: CoderSettingsStore - private lateinit var logger: Logger + private lateinit var logger: CoderLogger private lateinit var ideFeedManager: IdeFeedManager private val moshi = Moshi.Builder() diff --git a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt index 20f319a..94fe5a1 100644 --- a/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/feed/IdeFeedManagerTest.kt @@ -1,7 +1,7 @@ package com.coder.toolbox.feed import com.coder.toolbox.CoderToolboxContext -import com.jetbrains.toolbox.api.core.diagnostics.Logger +import com.coder.toolbox.diagnostics.CoderLogger import io.mockk.coEvery import io.mockk.every import io.mockk.mockk @@ -17,7 +17,7 @@ import java.nio.file.Path class IdeFeedManagerTest { private lateinit var context: CoderToolboxContext - private lateinit var logger: Logger + private lateinit var logger: CoderLogger private lateinit var feedService: JetBrainsFeedService private lateinit var ideFeedManager: IdeFeedManager diff --git a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt index 4b65105..4baae3a 100644 --- a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt @@ -1,6 +1,7 @@ package com.coder.toolbox.util import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.sdk.v2.models.WorkspaceAgentLifecycleState @@ -8,6 +9,7 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceAgentStatus import com.coder.toolbox.sdk.v2.models.WorkspaceBuild import com.coder.toolbox.sdk.v2.models.WorkspaceStatus import io.mockk.clearMocks +import io.mockk.every import io.mockk.mockk import io.mockk.verify import java.util.UUID @@ -16,6 +18,11 @@ import kotlin.test.Test class ConnectionMonitoringServiceTest { private val context = mockk(relaxed = true) + private val logger = mockk(relaxed = true) + + init { + every { context.logger } returns logger + } @Test fun `given a running workspace with a timed out agent and a ready lifecycle then expect a connection unstable notification`() { @@ -25,7 +32,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { context.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } } @Test @@ -36,7 +43,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { context.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } } @Test @@ -47,7 +54,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { context.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } } @Test @@ -58,7 +65,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { context.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } } @Test @@ -71,12 +78,12 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) // Reset mocks to verify subsequent calls - clearMocks(context, answers = false) + clearMocks(context, logger, answers = false) // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { context.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } } @Test @@ -91,7 +98,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { context.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } } @Test @@ -108,7 +115,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { context.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } } @Test @@ -125,7 +132,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { context.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } }