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/6] 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/6] 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/6] 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/6] 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()) } } From 0d7a35b950a68939b653ba1d2da7cec545a3ab07 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Thu, 3 Sep 2026 23:43:21 +0300 Subject: [PATCH 5/6] Correlate Toolbox logs with SSH sessions Connection diagnostics require one 16-byte, 32-character lowercase hexadecimal session ID for each logical Toolbox SSH connection. Logs that affect the connection and the coder ssh subprocess must carry the same ID. In Toolbox, the session belongs to the workspace and agent SSH transport: it may start without an IDE, outlive an IDE, and serve multiple JetBrains IDE launches. Unlike VS Code's one-to-one IDE and connection lifecycle, Toolbox has a shared polling loop that tracks additions, removals, and status changes across multiple workspaces. A single session ID cannot represent every active connection in that loop. We agreed on a hybrid direction: monitor status per workspace through WebSockets so each change can use the correct session (will be done in a separate PR), retain shared polling for additions and removals with logs multiplexed for every affected session. It remains an open question how shared polling requests should carry multiple IDs through baggage or another header. A future mode where the plugin runs inside the IDE will be closer to VS Code, so both lifecycle models will eventually need support. An additional problem is that the current Toolbox callbacks also do not fully describe the ssh connection state. `beforeConnection` reports an attempt to establish the ssh connection but not its reason or its outcome, while `afterDisconnect` only tells us whether the user explicitly disconnected. We therefore create an ID on the first connection attempt, retain and reuse it across automatic retries and every non-manual disconnect, remove it after a manual disconnect or environment disposal, and create a new ID when the user connects again. Since Toolbox cannot distinguish a temporary transport loss from a terminal non-manual failure, both remain part of the existing session. In terms of implementation, this PR implements a process-local workspace and agent registry, session-aware logging, propagation through CODER_TRACE_SESSION_ID, dynamic session lookup in connection and IDE paths, and multi-session fan-out for provider and SSH configuration work. It also records status and disconnect context and covers creation, reuse, manual removal, automatic retries, and environment disposal. In parallel with this PR work we asked JetBrains to add callbacks for successful connections and failed attempts, together with reasons that distinguish explicit connect, disconnect, and reconnect actions from automatic startup and retry, plugin-requested disconnects, transport loss, remote exit, and environment removal. Those signals would let the plugin track connection state and correlation IDs directly instead of inferring lifecycle from the limited callbacks and workspace status. - resolves https://linear.app/codercom/issue/DEVEX-667 --- .../coder/toolbox/CoderRemoteEnvironment.kt | 223 +++++++---- .../com/coder/toolbox/CoderRemoteProvider.kt | 102 +++-- .../com/coder/toolbox/cli/CoderCLIManager.kt | 23 +- .../toolbox/cli/SshCommandProcessHandle.kt | 9 +- .../coder/toolbox/diagnostics/CoderLogger.kt | 88 ++++- .../toolbox/session/SessionIdRegistry.kt | 40 +- .../toolbox/util/CoderProtocolHandler.kt | 111 ++++-- .../util/ConnectionMonitoringService.kt | 2 + .../com/coder/toolbox/views/CoderPage.kt | 17 +- .../coder/toolbox/views/EnvironmentView.kt | 9 + .../toolbox/CoderRemoteEnvironmentTest.kt | 373 ++++++++++++++++++ .../coder/toolbox/CoderRemoteProviderTest.kt | 107 ++++- .../coder/toolbox/cli/CoderCLIManagerTest.kt | 99 ++++- .../toolbox/diagnostics/CoderLoggerTest.kt | 99 +++++ .../toolbox/session/SessionIdRegistryTest.kt | 49 ++- .../toolbox/util/CoderProtocolHandlerTest.kt | 124 +++++- .../util/ConnectionMonitoringServiceTest.kt | 39 +- .../com/coder/toolbox/views/ActionTest.kt | 68 ++++ .../toolbox/views/EnvironmentViewTest.kt | 46 +++ 19 files changed, 1417 insertions(+), 211 deletions(-) create mode 100644 src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt create mode 100644 src/test/kotlin/com/coder/toolbox/views/ActionTest.kt diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt index e43ff0b..6df71fc 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt @@ -10,6 +10,8 @@ import com.coder.toolbox.sdk.ex.APIResponseException import com.coder.toolbox.sdk.v2.models.NetworkMetrics import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.session.SessionId +import com.coder.toolbox.session.SessionIdRegistry import com.coder.toolbox.util.OS import com.coder.toolbox.util.waitForFalseWithTimeout import com.coder.toolbox.util.withPath @@ -88,7 +90,7 @@ class CoderRemoteEnvironment( init { if (context.settingsStore.shouldAutoConnect(id)) { - context.logger.info("Last session to $id was still active, trying to establish SSH connection") + context.logger.info("Auto-connect is enabled for $id, trying to establish SSH connection") startSshConnection() } refreshAvailableActions() @@ -96,6 +98,9 @@ class CoderRemoteEnvironment( internal fun toWorkspaceAddressOrNull(): WorkspaceAddress? = agent?.let { WorkspaceAddress.from(workspace, it) } + internal fun currentSessionId(): SessionId? = + agent?.let { SessionIdRegistry.findSession(workspace.name, it.name) } + private fun refreshAvailableActions() { val actions = mutableListOf() context.logger.debug("Refreshing available actions for workspace $id with status: $environmentStatus") @@ -158,47 +163,47 @@ class CoderRemoteEnvironment( } if (environmentStatus.canStop()) { if (workspace.outdated) { - actions.add(Action(context, "Update and restart") { - context.logger.debug("Updating and re-starting $id...") - val build = client.updateWorkspace(workspace) - update(workspace.copy(latestBuild = build), agent) - workspaceRefreshTrigger.trySend(true) - } + actions.add( + Action(context, "Update and restart") { + context.logger.debug(currentSessionId(), "Updating and re-starting $id...") + val build = client.updateWorkspace(workspace) + update(workspace.copy(latestBuild = build), agent) + workspaceRefreshTrigger.trySend(true) + }.withCurrentSessionId(::currentSessionId) ) } - actions.add(Action(context, "Stop") { - tryStopSshConnection() - context.logger.debug("Stoping $id...") - val build = client.stopWorkspace(workspace) - update(workspace.copy(latestBuild = build), agent) - } + actions.add( + Action(context, "Stop") { + tryStopSshConnection() + context.logger.debug(currentSessionId(), "Stopping $id...") + val build = client.stopWorkspace(workspace) + update(workspace.copy(latestBuild = build), agent) + }.withCurrentSessionId(::currentSessionId) ) } actions.add(CoderDelimiter(context.i18n.pnotr(""))) - actions.add(Action(context, "Delete workspace", highlightInRed = true) { - context.cs.launch(CoroutineName("Delete Workspace Action")) { + actions.add( + Action(context, "Delete workspace", highlightInRed = true) { var dialogText = if (environmentStatus.canStop()) "This will close the workspace and remove all its information, including files, unsaved changes, history, and usage data." else "This will remove all information from the workspace, including files, unsaved changes, history, and usage data." dialogText += "\n\nType \"${workspace.name}\" below to confirm:" val confirmation = context.ui.showTextInputPopup( - if (environmentStatus.canStop()) context.i18n.ptrl("Delete running workspace?") else context.i18n.ptrl( - "Delete workspace?" - ), + if (environmentStatus.canStop()) context.i18n.ptrl("Delete running workspace?") + else context.i18n.ptrl("Delete workspace?"), context.i18n.pnotr(dialogText), context.i18n.ptrl("Workspace name"), TextType.General, context.i18n.ptrl("OK"), context.i18n.ptrl("Cancel") ) - if (confirmation != workspace.name) { - return@launch + if (confirmation == workspace.name) { + context.logger.debug(currentSessionId(), "Deleting $id...") + deleteWorkspace() } - context.logger.debug("Deleting $id...") - deleteWorkspace() - } - }) + }.withCurrentSessionId(::currentSessionId) + ) actionsList.update { actions @@ -212,7 +217,10 @@ class CoderRemoteEnvironment( } if (isConnected.waitForFalseWithTimeout(10.seconds) == null) { - context.logger.warn("The SSH connection to workspace $name could not be dropped in time, going to stop the workspace while the SSH connection is live") + val message = + "The SSH connection to workspace $name could not be dropped in time, " + + "going to stop the workspace while the SSH connection is live" + context.logger.warn(currentSessionId(), message) } } } @@ -222,49 +230,55 @@ class CoderRemoteEnvironment( override fun getAfterDisconnectHooks(): List = listOf(this) override fun beforeConnection() { - if (agent == null) { - return - } + val currentAgent = agent ?: return + val sessionId = SessionIdRegistry.startSession(context, workspace.name, currentAgent.name) context.logger.info( - "Launching SSH connection to $id on a ${agent?.operatingSystem.displayName()} machine" + sessionId, + "Launching SSH connection to $id on a ${currentAgent.operatingSystem.displayName()} machine" ) isConnected.update { true } context.settingsStore.updateAutoConnect(this.id, true) + // Toolbox can invoke this hook again while retrying without first reporting a disconnect. + // Replace the previous poller so one environment never leaves multiple pollers behind. + pollJob?.cancel() pollJob = pollNetworkMetrics() } - private fun pollNetworkMetrics(): Job = context.cs.launch(CoroutineName("Network Metrics Poller")) { - context.logger.info("Starting the network metrics poll job for $id") - while (isActive) { - val currentAgent = agent ?: break - context.logger.debug("Searching SSH command's PID for workspace $id...") - val pid = proxyCommandHandle.findByWorkspaceAndAgent(workspace, currentAgent) - if (pid == null) { - context.logger.debug("No SSH command PID was found for workspace $id") - delay(POLL_INTERVAL) - continue - } + private fun pollNetworkMetrics(): Job = + context.cs.launch(CoroutineName("Network Metrics Poller")) { + context.logger.info(currentSessionId(), "Starting the network metrics poll job for $id") + while (isActive) { + val currentAgent = agent ?: break + val sessionId = currentSessionId() + context.logger.debug(sessionId, "Searching SSH command's PID for workspace $id...") + val pid = proxyCommandHandle.findByWorkspaceAndAgent(workspace, currentAgent) + if (pid == null) { + context.logger.debug(sessionId, "No SSH command PID was found for workspace $id") + delay(POLL_INTERVAL) + continue + } - val metricsFile = Path.of(context.settingsStore.networkInfoDir, "$pid.json").toFile() - if (metricsFile.doesNotExists()) { - context.logger.debug("No metrics file found at ${metricsFile.absolutePath} for $id") + val metricsFile = Path.of(context.settingsStore.networkInfoDir, "$pid.json").toFile() + if (metricsFile.doesNotExists()) { + context.logger.debug(sessionId, "No metrics file found at ${metricsFile.absolutePath} for $id") + delay(POLL_INTERVAL) + continue + } + context.logger.debug(sessionId, "Loading metrics from ${metricsFile.absolutePath} for $id") + try { + val metrics = networkMetricsMarshaller.fromJson(metricsFile.readText()) ?: return@launch + context.logger.debug(sessionId, "$id metrics: $metrics") + additionalEnvironmentInformation[context.i18n.ptrl("Network Status")] = metrics.toPretty() + } catch (e: Exception) { + context.logger.error( + sessionId, + e, + "Error encountered while trying to load network metrics from ${metricsFile.absolutePath} for $id" + ) + } delay(POLL_INTERVAL) - continue - } - context.logger.debug("Loading metrics from ${metricsFile.absolutePath} for $id") - try { - val metrics = networkMetricsMarshaller.fromJson(metricsFile.readText()) ?: return@launch - context.logger.debug("$id metrics: $metrics") - additionalEnvironmentInformation[context.i18n.ptrl("Network Status")] = metrics.toPretty() - } catch (e: Exception) { - context.logger.error( - e, - "Error encountered while trying to load network metrics from ${metricsFile.absolutePath} for $id" - ) } - delay(POLL_INTERVAL) } - } private fun File.doesNotExists(): Boolean = !this.exists() @@ -275,41 +289,73 @@ class CoderRemoteEnvironment( * * Toolbox reacts to the removal by closing its environment wrapper, which only * cancels the wrapper's own coroutine scope and never invokes the - * [AfterDisconnectHook], so the network metrics poller must be stopped here. + * [AfterDisconnectHook], so the network metrics poller must be stopped and the session + * registry entry must be removed here. */ fun dispose() { pollJob?.cancel() + pollJob = null isConnected.update { false } + val removedSessionId = agent?.let { + SessionIdRegistry.removeSession(workspace.name, it.name) + } + removedSessionId?.let { sessionId -> + context.logger.info(sessionId, "Removed Toolbox SSH session for $id") + } } override fun afterDisconnect(isManual: Boolean) { - context.logger.info("Stopping the network metrics poll job for $id") + val sessionId = currentSessionId() + // A false value also covers Toolbox's Reconnect action. The current Coder state is useful + // context, but it cannot establish the disconnect cause on its own. + val disconnectKind = + if (isManual) "after an explicit user disconnect" + else "without an explicit user disconnect" + val stateInference = + if (!isManual && !environmentStatus.ready()) { + " The latest state may indicate a workspace or agent change." + } else { + "" + } + val latestCoderState = + "environment=${environmentStatus.label}, " + + "workspace=${workspace.latestBuild.status}, " + + "agent=${agent?.status ?: "unavailable"}, " + + "agentLifecycle=${agent?.lifecycleState ?: "unavailable"}" + context.logger.info( + sessionId, + "Toolbox is disconnecting SSH from $id $disconnectKind.$stateInference " + + "Latest known Coder state: $latestCoderState", + ) + context.logger.info(sessionId, "Stopping the network metrics poll job for $id") pollJob?.cancel() - this.connectionRequest.update { false } + pollJob = null + connectionRequest.update { false } isConnected.update { false } if (isManual) { // if the user manually disconnects the ssh connection we should not connect automatically context.settingsStore.updateAutoConnect(this.id, false) + agent?.let { + SessionIdRegistry.removeSession(workspace.name, it.name) + }?.let { + context.logger.info(it, "Removed Toolbox SSH session for $id after manual disconnect") + } } - context.logger.info("Disconnected from $id") } /** * Update the workspace/agent status to the listeners, if it has changed. */ - fun update(workspace: Workspace, agent: WorkspaceAgent?) { - if (this.workspace.latestBuild == workspace.latestBuild) { + fun update(newWorkspace: Workspace, newAgent: WorkspaceAgent?) { + if (workspace.latestBuild == newWorkspace.latestBuild) { return } - this.workspace = workspace - this.agent = agent - name = environmentId(workspace, agent) // workspace&agent status can be different from "environment status" // which is forced to queued state when a workspace is scheduled to start - updateStatus(WorkspaceAndAgentStatus.from(workspace, agent)) - if (agent != null) { - context.connectionMonitoringService.checkConnectionStatus(workspace, agent) + updateStatus(WorkspaceAndAgentStatus.from(newWorkspace, newAgent), newAgent) + if (newAgent != null) { + context.connectionMonitoringService.checkConnectionStatus(newWorkspace, newAgent) } // we have to regenerate the action list in order to force a redraw @@ -318,18 +364,30 @@ class CoderRemoteEnvironment( } - private fun updateStatus(status: WorkspaceAndAgentStatus) { - environmentStatus = status + private fun updateStatus( + newState: WorkspaceAndAgentStatus, + newAgent: WorkspaceAgent? = agent, + ) { + val previousEnvironmentStatus = environmentStatus + val previousWorkspace = workspace + val previousAgent = agent + val sessionId = currentSessionId() + + environmentStatus = newState + workspace = newState.workspace + agent = newAgent + name = environmentId(workspace, agent) state.update { environmentStatus.toRemoteEnvironmentState(context) } - context.logger.info( - "Overall status for workspace $id is $environmentStatus. " + - "Workspace status: ${workspace.latestBuild.status}, " + - "agent status: ${agent?.status}, " + - "agent lifecycle state: ${agent?.lifecycleState}, " + - "login before ready: ${agent?.loginBeforeReady}" - ) + val message = + "Overall status for workspace $id changed from ${previousEnvironmentStatus.label} " + + "to ${environmentStatus.label}. " + + "Workspace status: ${previousWorkspace.latestBuild.status} -> ${workspace.latestBuild.status}, " + + "agent status: ${previousAgent?.status} -> ${agent?.status}, " + + "agent lifecycle state: ${previousAgent?.lifecycleState} -> ${agent?.lifecycleState}, " + + "login before ready: ${previousAgent?.loginBeforeReady} -> ${agent?.loginBeforeReady}" + context.logger.info(sessionId, message) } /** @@ -347,7 +405,7 @@ class CoderRemoteEnvironment( client.url, cli, workspace, - envAgent + envAgent, ) } @@ -362,9 +420,14 @@ class CoderRemoteEnvironment( } /** - * Schedules the SSH connection to start as soon as possible if the workspace is ready and there is no connection already established. + * Schedules the SSH connection to start as soon as possible if the workspace is ready and + * there is no connection already established. + * + * The session is created or reactivated by [beforeConnection], when Toolbox confirms that it + * is starting the connection. */ fun startSshConnection() { + if (agent == null) return if (environmentStatus.ready() && !isConnected.value) { connectionRequest.update { true diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index 5e0c228..056e4e7 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -10,6 +10,7 @@ import com.coder.toolbox.sdk.ex.APIResponseException import com.coder.toolbox.sdk.ex.OAuthTokenResponseException import com.coder.toolbox.sdk.v2.models.InvalidCoderIdentifierException import com.coder.toolbox.sdk.v2.models.WorkspaceStatus +import com.coder.toolbox.session.SessionId import com.coder.toolbox.util.CoderProtocolHandler import com.coder.toolbox.util.DialogUi import com.coder.toolbox.util.TOKEN @@ -66,6 +67,9 @@ private const val CAN_T_HANDLE_URI_TITLE = "Can't handle URI" private const val FAILED_TO_HANDLE_OAUTH2_TITLE = "Failed to handle OAuth2 request" private const val SSH_CONFIGURATION_WARNING_TITLE = "SSH configuration could not be updated" +private fun List.currentSessionIds(): Set = + mapNotNull(CoderRemoteEnvironment::currentSessionId).toSet() + @OptIn(ExperimentalCoroutinesApi::class) class CoderRemoteProvider( private val context: CoderToolboxContext, @@ -128,8 +132,9 @@ class CoderRemoteProvider( context.cs.launch(CoroutineName("Workspace Poller")) { var lastPollTime = TimeSource.Monotonic.markNow() while (isActive) { + var sessionIds = lastEnvironments.currentSessionIds() try { - context.logger.debug("Fetching workspace agents from ${client.url}") + context.logger.debug(sessionIds, "Fetching workspace agents from ${client.url}") val resolvedEnvironments = resolveWorkspaceEnvironments(client, cli) // In case we logged out while running the query. @@ -137,16 +142,26 @@ class CoderRemoteProvider( return@launch } - // Toolbox closes removed environments without firing their - // disconnect hooks, so stop their background work before dropping them. - lastEnvironments.filter { it !in resolvedEnvironments }.forEach { it.dispose() } + // Sessions can start while the workspace request is in flight. Refresh the + // snapshot before disposal so it includes those sessions and retains the IDs + // of environments removed by this result. + sessionIds = lastEnvironments.currentSessionIds() + + val removedEnvironments = lastEnvironments.filter { it !in resolvedEnvironments } // Reconfigure if environments changed. if (lastEnvironments.size != resolvedEnvironments.size || lastEnvironments != resolvedEnvironments) { - context.logger.info("Workspaces have changed, reconfiguring CLI: $resolvedEnvironments") + context.logger.info( + sessionIds, + "Workspaces have changed, reconfiguring CLI: $resolvedEnvironments", + ) configureSsh(cli, resolvedEnvironments) } + // Toolbox closes removed environments without firing their disconnect hooks. + // Dispose them after SSH configuration has captured their session IDs. + removedEnvironments.forEach { it.dispose() } + environments.update { LoadableState.Value(resolvedEnvironments) } @@ -161,43 +176,61 @@ class CoderRemoteProvider( addAll(resolvedEnvironments) } } catch (_: CancellationException) { - context.logger.debug("${client.url} polling loop canceled") + context.logger.debug(sessionIds, "${client.url} polling loop canceled") break } catch (ex: Exception) { val elapsed = lastPollTime.elapsedNow() if (elapsed > POLL_INTERVAL * 2) { - context.logger.info("wake-up from an OS sleep was detected") + context.logger.info(sessionIds, "wake-up from an OS sleep was detected") } else { if ((ex is APIResponseException && ex.isTokenExpired) || ex is OAuthTokenResponseException) { close() context.envPageManager.showPluginEnvironmentsPage(false) context.logger.logAndShowError( + sessionIds, "Error encountered while setting up Coder", "Your Coder session has expired. Please re-authenticate and try again.", ex ) break } - context.logger.error(ex, "workspace polling error encountered") + context.logger.error(sessionIds, ex, "workspace polling error encountered") } } select { onTimeout(POLL_INTERVAL) { - context.logger.debug("workspace poller waked up by the $POLL_INTERVAL timeout") + context.logger.debug( + lastEnvironments.currentSessionIds(), + "workspace poller waked up by the $POLL_INTERVAL timeout", + ) } sshConfigTrigger.onReceive { staleSshConfigPath -> - context.logger.debug("workspace poller waked up because it should reconfigure the ssh configurations") - configureSsh(cli, lastEnvironments, staleSshConfigPath) + val currentSessionIds = lastEnvironments.currentSessionIds() + context.logger.debug( + currentSessionIds, + "workspace poller waked up because it should reconfigure the ssh configurations", + ) + configureSsh( + cli, + lastEnvironments, + staleSshConfigPath, + ) } workspaceRefreshTrigger.onReceive { shouldTrigger -> if (shouldTrigger) { - context.logger.debug("workspace poller waked up to fetch workspaces from the latest header settings") + context.logger.debug( + lastEnvironments.currentSessionIds(), + "workspace poller waked up to fetch workspaces from the latest header settings", + ) } } providerVisibleTrigger.onReceive { isCoderProviderVisible -> if (isCoderProviderVisible) { - context.logger.debug("workspace poller waked up by Coder Toolbox which is currently visible, fetching latest workspace statuses") + context.logger.debug( + lastEnvironments.currentSessionIds(), + "workspace poller waked up by Coder Toolbox which is currently visible, fetching latest workspace statuses", + ) } } } @@ -209,15 +242,21 @@ class CoderRemoteProvider( * Keep SSH configuration failures separate from workspace discovery. SSH * configuration is necessary to connect, but a read-only or malformed SSH * config must not prevent Toolbox from showing the workspaces it resolved. + * + * The affected sessions are captured from [lastEnvironments] before removed environments are + * disposed. They cannot be reconstructed from [resolvedEnvironments], which no longer contains + * those environments. */ private fun configureSsh( cli: CoderCLIManager, resolvedEnvironments: List, staleSshConfigPath: String? = null, ) { + val sessionIds = lastEnvironments.currentSessionIds() try { cli.configSsh( resolvedEnvironments.mapNotNull { it.toWorkspaceAddressOrNull() }.toSet(), + sessionIds = sessionIds, sshConfigPath = context.settingsStore.sshConfigPath, ) isSshConfigurationWarningShown = false @@ -226,9 +265,14 @@ class CoderRemoteProvider( // otherwise create a stray file there just to hold an empty managed block. if (staleSshConfigPath != null && Path.of(staleSshConfigPath).toFile().exists()) { runCatching { - cli.configSsh(emptySet(), sshConfigPath = staleSshConfigPath) + cli.configSsh( + emptySet(), + sessionIds = sessionIds, + sshConfigPath = staleSshConfigPath, + ) }.onFailure { ex -> context.logger.warn( + sessionIds, ex, "Failed to remove the managed SSH config block from the previous location: $staleSshConfigPath" ) @@ -243,13 +287,18 @@ class CoderRemoteProvider( isSshConfigurationWarningShown = true val reason = ex.message?.takeIf { it.isNotBlank() } ?: ex.javaClass.simpleName context.logger.logAndShowWarning( + sessionIds, SSH_CONFIGURATION_WARNING_TITLE, "Workspaces remain available, but SSH connections are unavailable: $reason. " + "Update ${context.settingsStore.sshConfigPath} and try again.", ex, ) } else { - context.logger.warn(ex, "Failed to update SSH configuration at ${context.settingsStore.sshConfigPath}") + context.logger.warn( + sessionIds, + ex, + "Failed to update SSH configuration at ${context.settingsStore.sshConfigPath}" + ) } } } @@ -306,9 +355,10 @@ class CoderRemoteProvider( * first page. */ private fun logout() { - context.logger.info("Logging out ${client?.me?.username}...") + val sessionIds = lastEnvironments.currentSessionIds() + context.logger.info(sessionIds, "Logging out ${client?.me?.username}...") close() - context.logger.info("User ${client?.me?.username} logged out successfully") + context.logger.info(sessionIds, "User ${client?.me?.username} logged out successfully") } /** @@ -340,6 +390,7 @@ class CoderRemoteProvider( * Also called as part of our own logout. */ override fun close() { + val sessionIds = lastEnvironments.currentSessionIds() softClose() client = null cli = null @@ -350,18 +401,19 @@ class CoderRemoteProvider( isInitialized.update { false } accountDropdownField.visibility.update { false } router.clear() - context.logger.info("Coder plugin is now closed") + context.logger.info(sessionIds, "Coder plugin is now closed") } private fun softClose() { + val sessionIds = lastEnvironments.currentSessionIds() pollJob?.let { it.cancel() - context.logger.info("Cancelled workspace poll job ${pollJob.toString()}") + context.logger.info(sessionIds, "Cancelled workspace poll job ${pollJob.toString()}") } pollJob = null client?.let { it.close() - context.logger.info("REST API client closed and resources released") + context.logger.info(sessionIds, "REST API client closed and resources released") } } @@ -575,7 +627,10 @@ class CoderRemoteProvider( private fun sameUrl(first: URL, second: URL?): Boolean = first.toURI().normalize() == second?.toURI()?.normalize() private suspend fun refreshSession(url: URL, token: String): Pair { - context.logger.info("Stopping workspace polling and re-initializing the http client and cli with a new token") + context.logger.info( + lastEnvironments.currentSessionIds(), + "Stopping workspace polling and re-initializing the http client and cli with a new token", + ) softClose() val newRestClient = CoderRestClient( context, @@ -595,7 +650,10 @@ class CoderRemoteProvider( coderHeaderPage.resetFilter() context.cs.launch(CoroutineName("Load Templates")) { coderHeaderPage.reloadTemplates() } pollJob = poll(newRestClient, newCli) - context.logger.info("Workspace poll job with name ${pollJob.toString()} was created while handling URI") + context.logger.info( + lastEnvironments.currentSessionIds(), + "Workspace poll job with name ${pollJob.toString()} was created while handling URI", + ) return newRestClient to newCli } diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index ce9fcd9..9dca745 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -13,6 +13,7 @@ import com.coder.toolbox.cli.gpg.VerificationResult import com.coder.toolbox.cli.gpg.VerificationResult.Failed import com.coder.toolbox.cli.gpg.VerificationResult.Invalid import com.coder.toolbox.sdk.CoderHttpClientBuilder +import com.coder.toolbox.session.SessionId import com.coder.toolbox.settings.SignatureFallbackStrategy.ALLOW import com.coder.toolbox.util.InvalidVersionException import com.coder.toolbox.util.SemVer @@ -293,16 +294,23 @@ class CoderCLIManager( /** * Configure SSH to use this binary. * + * The caller supplies the affected [sessionIds] because [workspaceAddresses] may be empty + * while cleaning up configuration or may no longer contain a removed environment. + * * This can take supported features for testing purposes only. */ internal fun configSsh( workspaceAddresses: Set, + sessionIds: Set = emptySet(), feats: Features = features, sshConfigPath: String = context.settingsStore.sshConfigPath, ) { - context.logger.info("Configuring SSH config at $sshConfigPath") - writeSSHConfig(modifySSHConfig(readSSHConfig(sshConfigPath), workspaceAddresses, feats), sshConfigPath) - context.logger.info("Finished configuring SSH config") + context.logger.info(sessionIds, "Configuring SSH config at $sshConfigPath") + writeSSHConfig( + modifySSHConfig(workspaceAddresses, sessionIds, readSSHConfig(sshConfigPath), feats), + sshConfigPath, + ) + context.logger.info(sessionIds, "Finished configuring SSH config") } /** @@ -323,8 +331,9 @@ class CoderCLIManager( * version. */ private fun modifySSHConfig( - contents: String?, workspaceAddresses: Set, + sessionIds: Set, + contents: String?, feats: Features, ): String? { val host = deploymentURL.safeHost() @@ -425,7 +434,7 @@ class CoderCLIManager( } if (managedBlock == null) { - context.logger.info("Appending config block") + context.logger.info(sessionIds, "Appending config block") val toAppend = if (contents.isEmpty()) { blockContent @@ -441,7 +450,7 @@ class CoderCLIManager( val (start, end) = managedBlock if (isRemoving) { - context.logger.info("No workspaces; removing config block") + context.logger.info(sessionIds, "No workspaces; removing config block") return listOf( contents.substring(0, start.range.first), // Need to keep the trailing newline(s) if we are not at the @@ -452,7 +461,7 @@ class CoderCLIManager( ).joinToString("") } - context.logger.info("Replacing existing config block") + context.logger.info(sessionIds, "Replacing existing config block") return listOf( contents.substring(0, start.range.first), start.groupValues[1], // Leading newline(s). diff --git a/src/main/kotlin/com/coder/toolbox/cli/SshCommandProcessHandle.kt b/src/main/kotlin/com/coder/toolbox/cli/SshCommandProcessHandle.kt index a31b116..1b62b4f 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/SshCommandProcessHandle.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/SshCommandProcessHandle.kt @@ -3,6 +3,7 @@ package com.coder.toolbox.cli import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.session.SessionIdRegistry import kotlin.jvm.optionals.getOrNull /** @@ -20,13 +21,17 @@ class SshCommandProcessHandle(private val ctx: CoderToolboxContext) { * as a separate command which in turns spawns another child for the proxy command. */ fun findByWorkspaceAndAgent(ws: Workspace, agent: WorkspaceAgent): Long? { + val sessionId = SessionIdRegistry.findSession(ws.name, agent.name) val stack = ArrayDeque(ProcessHandle.current().children().toList()) while (stack.isNotEmpty()) { val processHandle = stack.removeLast() val cmdLine = processHandle.info().commandLine().getOrNull() - ctx.logger.debug("SSH command PID: ${processHandle.pid()} Command: $cmdLine") + ctx.logger.debug(sessionId, "SSH command PID: ${processHandle.pid()} Command: $cmdLine") if (cmdLine != null && cmdLine.isSshCommandFor(ws, agent)) { - ctx.logger.debug("SSH command with PID: ${processHandle.pid()} and Command: $cmdLine matches ${ws.name}.${agent.name}") + ctx.logger.debug( + sessionId, + "SSH command with PID: ${processHandle.pid()} and Command: $cmdLine matches ${ws.name}.${agent.name}" + ) return processHandle.pid() } else { stack.addAll(processHandle.children().toList()) diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt index 8a95293..1569b35 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt @@ -11,14 +11,14 @@ 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" +private fun withSessionId(sessionId: SessionId?, message: String): String = + sessionId?.let { "$CLIENT_SESSION_ID_LOG_KEY=$it $message" } ?: message /** * 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. + * A null [SessionId] leaves the message unchanged. A non-null ID adds the correlation field, while + * a set of IDs emits one log for each session (or one unchanged log when the set is empty). */ class CoderLogger( private val delegate: Logger, @@ -26,37 +26,101 @@ class CoderLogger( private val cs: CoroutineScope, private val i18n: LocalizableStringFactory, ) : Logger by delegate { - fun error(sessionId: SessionId, message: String) { + fun error(sessionId: SessionId? = null, message: String) { delegate.error(withSessionId(sessionId, message)) } - fun warn(sessionId: SessionId, message: String) { + fun error(sessionId: SessionId? = null, exception: Throwable, message: String) { + delegate.error(exception, withSessionId(sessionId, message)) + } + + fun error(sessionIds: Set, exception: Throwable, message: String) { + sessionIds.onceOrForEach { error(it, exception, message) } + } + + fun warn(sessionId: SessionId? = null, message: String) { delegate.warn(withSessionId(sessionId, message)) } - fun debug(sessionId: SessionId, message: String) { + fun warn(sessionId: SessionId? = null, exception: Throwable, message: String) { + delegate.warn(exception, withSessionId(sessionId, message)) + } + + fun warn(sessionIds: Set, exception: Throwable, message: String) { + sessionIds.onceOrForEach { warn(it, exception, message) } + } + + fun debug(sessionId: SessionId? = null, message: String) { delegate.debug(withSessionId(sessionId, message)) } - fun info(sessionId: SessionId, message: String) { + fun debug(sessionIds: Set, message: String) { + sessionIds.onceOrForEach { debug(it, message) } + } + + fun info(sessionId: SessionId? = null, message: String) { delegate.info(withSessionId(sessionId, message)) } + fun info(sessionIds: Set, message: String) { + sessionIds.onceOrForEach { info(it, message) } + } + fun logAndShowError(title: String, error: String) { error(error) showInfoPopup(title, error) } + fun logAndShowError(sessionId: SessionId? = null, title: String, error: String) { + error(sessionId, error) + showInfoPopup(title, error) + } + fun logAndShowError(title: String, error: String, exception: Throwable) { error(exception, error) showInfoPopup(title, error) } + fun logAndShowError(sessionId: SessionId? = null, title: String, error: String, exception: Throwable) { + error(sessionId, exception, error) + showInfoPopup(title, error) + } + + fun logAndShowError( + sessionIds: Set, + title: String, + error: String, + exception: Throwable, + ) { + sessionIds.onceOrForEach { this.error(it, exception, error) } + showInfoPopup(title, error) + } + fun logAndShowWarning(title: String, warning: String) { warn(warning) showInfoPopup(title, warning) } + fun logAndShowWarning(sessionId: SessionId? = null, title: String, warning: String) { + warn(sessionId, warning) + showInfoPopup(title, warning) + } + + fun logAndShowWarning(sessionId: SessionId? = null, title: String, warning: String, exception: Throwable) { + warn(sessionId, exception, warning) + showInfoPopup(title, warning) + } + + fun logAndShowWarning( + sessionIds: Set, + title: String, + warning: String, + exception: Throwable, + ) { + sessionIds.onceOrForEach { warn(it, exception, warning) } + showInfoPopup(title, warning) + } + fun logAndShowWarning(title: String, warning: String, exception: Throwable) { warn(exception, warning) showInfoPopup(title, warning) @@ -94,4 +158,12 @@ class CoderLogger( } } } + + private inline fun Set.onceOrForEach(action: (SessionId?) -> Unit) { + if (isEmpty()) { + action(null) + } else { + forEach(action) + } + } } diff --git a/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt b/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt index 789faa6..0b79b07 100644 --- a/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt +++ b/src/main/kotlin/com/coder/toolbox/session/SessionIdRegistry.kt @@ -1,5 +1,6 @@ package com.coder.toolbox.session +import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.util.toHex import java.security.SecureRandom import java.util.concurrent.ConcurrentHashMap @@ -39,35 +40,44 @@ private data class SessionKey( ) /** - * Process-local registry of active connection sessions. + * Process-local registry of 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. + * A session is keyed only by workspace and agent names. Call [startSession] from the SSH + * before-connection callback; all other code should use [findSession] so observing a session cannot + * create one. Non-user disconnects and retries remain part of the same logical session. A manual + * disconnect ends the session, as does disposal of the Toolbox environment that owns it. */ object SessionIdRegistry { private val sessionIds = ConcurrentHashMap() /** - * Returns the active session ID for this workspace and agent, creating it when absent. + * Returns the 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. + * Reusing an existing ID allows transient reconnects to remain part of the same session. A + * newly created session is logged here so callers do not need to distinguish creation from reuse. */ - fun startSession(workspaceName: String, agentName: String): SessionId = - sessionIds.computeIfAbsent(SessionKey(workspaceName, agentName)) { SessionId.generate() } + fun startSession(context: CoderToolboxContext, workspaceName: String, agentName: String): SessionId { + var created = false + val sessionId = sessionIds.computeIfAbsent(SessionKey(workspaceName, agentName)) { + created = true + SessionId.generate() + } + if (created) { + context.logger.info(sessionId, "Created Toolbox SSH session for $workspaceName.$agentName") + } + return sessionId + } - /** Returns the active session ID without creating a session. */ + /** Returns the current logical session ID without creating one. */ fun findSession(workspaceName: String, agentName: String): SessionId? = sessionIds[SessionKey(workspaceName, agentName)] /** - * Removes the session when its owning Toolbox environment is disposed. + * Removes a session that has reached the end of its lifetime. * - * 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. + * Call this when the user deliberately disconnects or when Toolbox disposes the environment. + * Do not call it for a transient transport disconnect or an IDE closing; 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/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt b/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt index ebb6bde..f5c2c3d 100644 --- a/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt +++ b/src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt @@ -11,6 +11,7 @@ import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.sdk.v2.models.WorkspaceStatus +import com.coder.toolbox.session.SessionIdRegistry import com.jetbrains.toolbox.api.core.util.LoadableState import com.jetbrains.toolbox.api.remoteDev.connection.RemoteToolsHelper import kotlinx.coroutines.CoroutineName @@ -74,7 +75,8 @@ open class CoderProtocolHandler( // after the workspace poller observes the running workspace. Nudge the // poller and wait for the environment to show up before using its id. workspaceRefreshTrigger.trySend(true) - if (!waitForEnvironment(environmentId)) { + val environment = waitForEnvironment(environmentId) + if (environment == null) { context.logger.logAndShowError( CAN_T_HANDLE_URI_TITLE, "The environment $environmentId did not become available in time" @@ -82,13 +84,15 @@ open class CoderProtocolHandler( return } context.showEnvironmentPage(environmentId) + // send a signal to start the ssh connection if it is not already started + environment.startSshConnection() val productCode = params.ideProductCode() val buildNumber = params.ideBuildNumber() val projectFolder = params.projectFolder() if (!productCode.isNullOrBlank() && !buildNumber.isNullOrBlank()) { - launchIde(environmentId, productCode, buildNumber, projectFolder) + launchIde(environment, productCode, buildNumber, projectFolder) } } } @@ -258,6 +262,7 @@ open class CoderProtocolHandler( if (!status.ready()) { context.logger.logAndShowError( + SessionIdRegistry.findSession(workspace.name, agent.name), CAN_T_HANDLE_URI_TITLE, "Agent ${agent.name} for workspace ${workspace.name} is not ready" ) @@ -267,42 +272,55 @@ open class CoderProtocolHandler( } private fun launchIde( - environmentId: String, + environment: CoderRemoteEnvironment, productCode: String, buildNumberHint: String, projectFolder: String? ) { context.cs.launch(CoroutineName("Launch Remote IDE")) { - val selectedIde = selectAndInstallRemoteIde(productCode, buildNumberHint, environmentId) ?: return@launch - context.logger.info("Selected IDE $selectedIde for $productCode with hint $buildNumberHint") + val selectedIde = + selectAndInstallRemoteIde(environment, productCode, buildNumberHint) + ?: return@launch + context.logger.info( + environment.currentSessionId(), + "Selected IDE $selectedIde for $productCode with hint $buildNumberHint", + ) // Ensure JBClient is prepared (installed/downloaded locally) - installJBClient(selectedIde, environmentId).join() + installJBClient(environment, selectedIde).join() // Launch - launchJBClient(selectedIde, environmentId, projectFolder) + launchJBClient(environment, selectedIde, projectFolder) } } private suspend fun selectAndInstallRemoteIde( + environment: CoderRemoteEnvironment, productCode: String, - buildNumberHint: String, - environmentId: String + buildNumberHint: String ): String? { - val selectedIde = resolveIdeIdentifier(environmentId, productCode, buildNumberHint) ?: return null + val selectedIde = + resolveIdeIdentifier(environment, productCode, buildNumberHint) ?: return null + val environmentId = environment.id val installedIdeVersions = context.remoteIdeOrchestrator.getInstalledRemoteTools(environmentId, productCode) - context.logger.info("Selected IDE $installedIdeVersions for $productCode for $environmentId") + context.logger.info( + environment.currentSessionId(), + "Selected IDE $installedIdeVersions for $productCode for $environmentId", + ) if (installedIdeVersions.contains(selectedIde)) { - context.logger.info("$selectedIde is already installed on $environmentId") + context.logger.info(environment.currentSessionId(), "$selectedIde is already installed on $environmentId") return selectedIde } - context.logger.info("Installing $selectedIde on $environmentId...") + context.logger.info(environment.currentSessionId(), "Installing $selectedIde on $environmentId...") context.remoteIdeOrchestrator.installRemoteTool(environmentId, selectedIde) if (context.remoteIdeOrchestrator.waitForIdeToBeInstalled(environmentId, selectedIde)) { - context.logger.info("Successfully installed $selectedIde on $environmentId.") + context.logger.info( + environment.currentSessionId(), + "Successfully installed $selectedIde on $environmentId." + ) return selectedIde } else { context.ui.showInfoPopup( @@ -319,17 +337,18 @@ open class CoderProtocolHandler( * Supports: latest_eap, latest_release, latest_installed, or specific build number. */ internal suspend fun resolveIdeIdentifier( - environmentId: String, + environment: CoderRemoteEnvironment, productCode: String, - buildNumberHint: String + buildNumberHint: String, ): String? { + val environmentId = environment.id val availableBuilds = context.remoteIdeOrchestrator.getAvailableRemoteTools(environmentId, productCode) .map { it.substringAfter("$productCode-") }.apply { - context.logger.info("Available $productCode IDEs: $this") + context.logger.info(environment.currentSessionId(), "Available $productCode IDEs: $this") } val installed = context.remoteIdeOrchestrator.getInstalledRemoteTools(environmentId, productCode) .map { it.substringAfter("$productCode-") }.apply { - context.logger.info("Installed $productCode IDEs: $this") + context.logger.info(environment.currentSessionId(), "Installed $productCode IDEs: $this") } val resolvedBuildNumber = when (buildNumberHint) { @@ -345,6 +364,7 @@ open class CoderProtocolHandler( } else { if (availableBuilds.isEmpty()) { context.logger.logAndShowError( + environment.currentSessionId(), CAN_T_HANDLE_URI_TITLE, "Can't launch EAP for $productCode because no version is available on $environmentId" ) @@ -352,7 +372,10 @@ open class CoderProtocolHandler( } // Fallback to max available val fallback = availableBuilds.maxByOrNull { it } - context.logger.info("No EAP found for $productCode, falling back to latest available: $fallback") + context.logger.info( + environment.currentSessionId(), + "No EAP found for $productCode, falling back to latest available: $fallback", + ) fallback } } @@ -369,13 +392,17 @@ open class CoderProtocolHandler( } else { if (availableBuilds.isEmpty()) { context.logger.logAndShowError( + environment.currentSessionId(), CAN_T_HANDLE_URI_TITLE, "Can't launch Release for $productCode because no version is available on $environmentId" ) return null } val fallback = availableBuilds.maxByOrNull { it } - context.logger.info("No Release found for $productCode, falling back to latest available: $fallback") + context.logger.info( + environment.currentSessionId(), + "No Release found for $productCode, falling back to latest available: $fallback" + ) fallback } } @@ -385,6 +412,7 @@ open class CoderProtocolHandler( installed.maxByOrNull { it } } else if (availableBuilds.isEmpty()) { context.logger.logAndShowError( + environment.currentSessionId(), 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" ) @@ -392,7 +420,10 @@ open class CoderProtocolHandler( } else { // Fallback to latest available if valid val fallback = availableBuilds.maxByOrNull { it } - context.logger.info("No installed IDE found, falling back to latest available: $fallback") + context.logger.info( + environment.currentSessionId(), + "No installed IDE found, falling back to latest available: $fallback", + ) fallback } } @@ -409,6 +440,7 @@ open class CoderProtocolHandler( availableMatch } else { context.logger.logAndShowError( + environment.currentSessionId(), CAN_T_HANDLE_URI_TITLE, "Can't launch $productCode-$buildNumberHint because there is no matching version installed nor available for install on $environmentId" ) @@ -420,15 +452,25 @@ open class CoderProtocolHandler( return resolvedBuildNumber?.let { "$productCode-$it" } } - private fun installJBClient(selectedIde: String, environmentId: String): Job = + private fun installJBClient( + environment: CoderRemoteEnvironment, + selectedIde: String, + ): Job = context.cs.launch(CoroutineName("JBClient Installer")) { - context.logger.info("Downloading and installing JBClient counterpart to $selectedIde locally") - context.jbClientOrchestrator.prepareClient(environmentId, selectedIde) + context.logger.info( + environment.currentSessionId(), + "Downloading and installing JBClient counterpart to $selectedIde locally", + ) + context.jbClientOrchestrator.prepareClient(environment.id, selectedIde) } - private fun launchJBClient(selectedIde: String, environmentId: String, projectFolder: String?) { - context.logger.info("Launching $selectedIde on $environmentId") - context.jbClientOrchestrator.connectToIde(environmentId, selectedIde, projectFolder) + private fun launchJBClient( + environment: CoderRemoteEnvironment, + selectedIde: String, + projectFolder: String?, + ) { + context.logger.info(environment.currentSessionId(), "Launching $selectedIde on ${environment.id}") + context.jbClientOrchestrator.connectToIde(environment.id, selectedIde, projectFolder) } /** @@ -436,15 +478,22 @@ open class CoderProtocolHandler( * environment list, i.e. the workspace poller resolved the agent and wrote * the SSH configuration for it. */ - private suspend fun waitForEnvironment(environmentId: String, waitTime: Duration = 1.minutes): Boolean = try { + private suspend fun waitForEnvironment( + environmentId: String, + waitTime: Duration = 1.minutes, + ): CoderRemoteEnvironment? = try { withTimeout(waitTime.toJavaDuration()) { - environments.first { state -> + val state = environments.first { state -> state is LoadableState.Value && state.value.any { it.id == environmentId } } + if (state is LoadableState.Value) { + state.value.firstOrNull { it.id == environmentId } + } else { + null + } } - true } catch (_: TimeoutCancellationException) { - false + null } private suspend fun CoderRestClient.waitForReady(workspace: Workspace): Boolean { diff --git a/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt b/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt index 4e29954..e54b9df 100644 --- a/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt +++ b/src/main/kotlin/com/coder/toolbox/util/ConnectionMonitoringService.kt @@ -6,6 +6,7 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceAgent import com.coder.toolbox.sdk.v2.models.WorkspaceAgentLifecycleState import com.coder.toolbox.sdk.v2.models.WorkspaceAgentStatus import com.coder.toolbox.sdk.v2.models.WorkspaceStatus +import com.coder.toolbox.session.SessionIdRegistry class ConnectionMonitoringService( private val context: CoderToolboxContext @@ -27,6 +28,7 @@ class ConnectionMonitoringService( when { isWorkspaceRunning && isAgentReady && hasConnectionIssue -> { context.logger.logAndShowWarning( + SessionIdRegistry.findSession(ws.name, agent.name), "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 8504b16..68e5517 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt @@ -2,6 +2,7 @@ package com.coder.toolbox.views import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.sdk.ex.APIResponseException +import com.coder.toolbox.session.SessionId import com.jetbrains.toolbox.api.core.ui.icons.SvgIcon import com.jetbrains.toolbox.api.core.ui.icons.SvgIcon.IconType import com.jetbrains.toolbox.api.localization.LocalizableString @@ -63,6 +64,8 @@ class Action( private val validateBlock: () -> Boolean = { true }, private val actionBlock: suspend () -> Unit, ) : RunnableActionDescription { + private var currentSessionId: () -> SessionId? = { null } + override val label: LocalizableString = context.i18n.ptrl(description) override val shouldClosePage: Boolean = closesPage override val isEnabled: Boolean = enabled() @@ -74,6 +77,11 @@ class Action( */ override fun validate(): Boolean = validateBlock() + /** Associates failures from this action with its current Toolbox SSH session, when one exists. */ + fun withCurrentSessionId(currentSessionId: () -> SessionId?): Action = apply { + this.currentSessionId = currentSessionId + } + override fun run() { context.cs.launch(CoroutineName("$description Action")) { try { @@ -84,10 +92,15 @@ class Action( ex.reason } else ex.message } else ex.message - context.logger.logAndShowError("Error while running `$description`", textError ?: "", ex) + context.logger.logAndShowError( + currentSessionId(), + "Error while running `$description`", + textError ?: "", + ex, + ) } } } } -class CoderDelimiter(override val label: LocalizableString) : ActionDelimiter \ No newline at end of file +class CoderDelimiter(override val label: LocalizableString) : ActionDelimiter diff --git a/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt b/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt index 0e0d3e5..f6db8ad 100644 --- a/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt +++ b/src/main/kotlin/com/coder/toolbox/views/EnvironmentView.kt @@ -5,6 +5,7 @@ import com.coder.toolbox.cli.CoderCLIManager import com.coder.toolbox.cli.WorkspaceAddress import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.session.SessionIdRegistry import com.coder.toolbox.util.OS import com.jetbrains.toolbox.api.remoteDev.deploy.DeploymentSettings import com.jetbrains.toolbox.api.remoteDev.deploy.DeploymentTarget @@ -13,6 +14,8 @@ import com.jetbrains.toolbox.api.remoteDev.ssh.SshConnectionInfo import java.net.URL import kotlin.time.Duration.Companion.seconds +private const val CODER_TRACE_SESSION_ID = "CODER_TRACE_SESSION_ID" + private fun OS?.toDeploymentTarget(): DeploymentTarget = when (this) { OS.LINUX -> DeploymentTarget.LINUX OS.MAC -> DeploymentTarget.MACOS @@ -59,6 +62,10 @@ private class WorkspaceSshConnectionInfo( */ override val host: String = cli.getHostname(url, WorkspaceAddress.from(workspace, agent)) + /** Makes the Toolbox session authoritative for the coder ssh child process. */ + override val environment: Map? = SessionIdRegistry.findSession(workspace.name, agent.name) + ?.let { mapOf(CODER_TRACE_SESSION_ID to it.value) } + /** * The port is ignored by the Coder proxy command. */ @@ -89,6 +96,7 @@ private class WorkspaceSshConnectionInfo( if (agent.name != other.agent.name) return false if (host != other.host) return false if (sshConfigPath != other.sshConfigPath) return false + if (environment != other.environment) return false return true } @@ -99,6 +107,7 @@ private class WorkspaceSshConnectionInfo( result = 31 * result + agent.name.hashCode() result = 31 * result + host.hashCode() result = 31 * result + sshConfigPath.hashCode() + result = 31 * result + environment.hashCode() return result } diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt new file mode 100644 index 0000000..c44693d --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteEnvironmentTest.kt @@ -0,0 +1,373 @@ +package com.coder.toolbox + +import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.diagnostics.CoderLogger +import com.coder.toolbox.sdk.CoderRestClient +import com.coder.toolbox.sdk.DataGen +import com.coder.toolbox.sdk.v2.models.Workspace +import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.sdk.v2.models.WorkspaceAgentLifecycleState +import com.coder.toolbox.sdk.v2.models.WorkspaceAgentStatus +import com.coder.toolbox.sdk.v2.models.WorkspaceStatus +import com.coder.toolbox.session.SessionId +import com.coder.toolbox.session.SessionIdRegistry +import com.coder.toolbox.store.CoderSettingsStore +import com.jetbrains.toolbox.api.localization.LocalizableStringFactory +import com.jetbrains.toolbox.api.remoteDev.environments.SshEnvironmentContentsView +import com.jetbrains.toolbox.api.remoteDev.states.EnvironmentStateColorPalette +import io.mockk.Called +import io.mockk.clearMocks +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import java.util.UUID +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class CoderRemoteEnvironmentTest { + @Test + fun `auto-connect requests SSH while the environment is initialized`() = runTest { + val fixture = fixture(backgroundScope, autoConnect = true) + + try { + assertTrue(fixture.environment.connectionRequest.value) + assertNull(fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info( + "Auto-connect is enabled for ${fixture.environment.id}, trying to establish SSH connection" + ) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `requesting an SSH connection does not create its session`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.startSshConnection() + + assertTrue(fixture.environment.connectionRequest.value) + assertNull(fixture.currentSessionId()) + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `SSH connection info exports the session activated by the connection callback`() = runTest { + val fixture = fixture(backgroundScope) + + try { + val contentsView = fixture.environment.getContentsView() as SshEnvironmentContentsView + assertNull(contentsView.getConnectionInfo().environment) + assertNull(fixture.currentSessionId()) + + fixture.environment.beforeConnection() + val sessionId = assertNotNull(fixture.currentSessionId()) + val connectionInfo = contentsView.getConnectionInfo() + + assertEquals( + mapOf("CODER_TRACE_SESSION_ID" to sessionId.value), + connectionInfo.environment, + ) + verify(exactly = 1) { + fixture.logger.info(sessionId, match(::isSessionStartedMessage)) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `non-manual disconnect retains the session for reconnect`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + val firstSessionId = assertNotNull(fixture.currentSessionId()) + + verify(exactly = 1) { + fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + } + + fixture.environment.afterDisconnect(isManual = false) + assertEquals(firstSessionId, fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info( + firstSessionId, + match { + it.contains("without an explicit user disconnect") && + it.contains("environment=Ready") && + it.contains("workspace=RUNNING") && + it.contains("agent=CONNECTED") && + it.contains("agentLifecycle=READY") && + !it.contains("may indicate a workspace or agent change") + }, + ) + } + + fixture.environment.beforeConnection() + assertEquals(firstSessionId, fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `repeated connection callbacks replace the network metrics poller`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + fixture.environment.beforeConnection() + runCurrent() + + val sessionId = assertNotNull(fixture.currentSessionId()) + assertEquals( + 1, + backgroundScope.coroutineContext[Job]?.children?.count { it.isActive }, + ) + verify(exactly = 1) { + fixture.logger.info( + sessionId, + "Starting the network metrics poll job for ${fixture.environment.id}", + ) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `reconnecting after a manual disconnect creates a new session`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + val firstSessionId = assertNotNull(fixture.currentSessionId()) + + fixture.environment.afterDisconnect(isManual = true) + assertNull(fixture.currentSessionId()) + + fixture.environment.beforeConnection() + val secondSessionId = assertNotNull(fixture.currentSessionId()) + + assertNotEquals(firstSessionId, secondSessionId) + verify(exactly = 1) { + fixture.settingsStore.updateAutoConnect(fixture.environment.id, false) + } + verify(exactly = 1) { + fixture.logger.info( + firstSessionId, + "Removed Toolbox SSH session for ${fixture.environment.id} after manual disconnect", + ) + } + verify(exactly = 1) { + fixture.logger.info( + firstSessionId, + match { + it.contains("after an explicit user disconnect") && + it.contains("Latest known Coder state") + }, + ) + } + verify(exactly = 1) { + fixture.logger.info(firstSessionId, match(::isSessionStartedMessage)) + } + verify(exactly = 1) { + fixture.logger.info(secondSessionId, match(::isSessionStartedMessage)) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `non-manual disconnect logs a possible workspace state cause`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + val sessionId = assertNotNull(fixture.currentSessionId()) + val updatedAgent = fixture.agent.copy( + status = WorkspaceAgentStatus.DISCONNECTED, + lifecycleState = WorkspaceAgentLifecycleState.SHUTTING_DOWN, + ) + val updatedWorkspace = fixture.workspace.copy( + latestBuild = fixture.workspace.latestBuild.copy(status = WorkspaceStatus.STOPPING), + ) + fixture.environment.update(updatedWorkspace, updatedAgent) + + fixture.environment.afterDisconnect(isManual = false) + + assertEquals(sessionId, fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info( + sessionId, + match { + it.contains("without an explicit user disconnect") && + it.contains("may indicate a workspace or agent change") && + it.contains("environment=Stopping") && + it.contains("workspace=STOPPING") && + it.contains("agent=DISCONNECTED") && + it.contains("agentLifecycle=SHUTTING_DOWN") + }, + ) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `disposing an environment removes and logs its SSH session once`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + val sessionId = assertNotNull(fixture.currentSessionId()) + + fixture.environment.dispose() + + assertNull(fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info(sessionId, match(::isSessionDisposedMessage)) + } + + fixture.environment.dispose() + assertNull(fixture.currentSessionId()) + verify(exactly = 1) { + fixture.logger.info(sessionId, match(::isSessionDisposedMessage)) + } + } finally { + fixture.removeSession() + } + } + + @Test + fun `workspace and agent status logs include the old and new values`() = runTest { + val fixture = fixture(backgroundScope) + + try { + fixture.environment.beforeConnection() + val sessionId = assertNotNull(fixture.currentSessionId()) + val updatedAgent = fixture.agent.copy( + status = WorkspaceAgentStatus.DISCONNECTED, + lifecycleState = WorkspaceAgentLifecycleState.SHUTTING_DOWN, + ) + val updatedWorkspace = fixture.workspace.copy( + latestBuild = fixture.workspace.latestBuild.copy(status = WorkspaceStatus.STOPPING), + ) + + fixture.environment.update(updatedWorkspace, updatedAgent) + + verify(exactly = 1) { + fixture.logger.info( + sessionId, + match { + it.contains("changed from Ready to Stopping") && + it.contains("Workspace status: RUNNING -> STOPPING") && + it.contains("agent status: CONNECTED -> DISCONNECTED") && + it.contains("agent lifecycle state: READY -> SHUTTING_DOWN") + }, + ) + } + } finally { + fixture.environment.dispose() + fixture.removeSession() + } + } + + @Test + fun `disposing an environment without a session is a no-op`() = runTest { + val fixture = fixture(backgroundScope) + clearMocks(fixture.logger, answers = false, recordedCalls = true) + + fixture.environment.dispose() + fixture.environment.dispose() + + assertNull(fixture.currentSessionId()) + verify { fixture.logger wasNot Called } + } + + private fun fixture(scope: CoroutineScope, autoConnect: Boolean = false): Fixture { + val suffix = UUID.randomUUID().toString().take(8) + val workspaceName = "workspace-$suffix" + val agentName = "agent-$suffix" + val workspace = DataGen.workspace( + name = workspaceName, + agents = mapOf(agentName to UUID.randomUUID().toString()), + ) + val agent = requireNotNull(workspace.latestBuild.resources.single().agents).single() + val context = mockk(relaxed = true) + val logger = mockk(relaxed = true) + val settingsStore = mockk(relaxed = true) + + every { context.cs } returns scope + every { context.logger } returns logger + every { context.settingsStore } returns settingsStore + every { context.i18n } returns mockk(relaxed = true) + every { context.envStateColorPalette } returns mockk(relaxed = true) + every { settingsStore.shouldAutoConnect(any()) } returns autoConnect + + val environment = CoderRemoteEnvironment( + context = context, + client = mockk(relaxed = true), + cli = mockk(relaxed = true), + workspaceRefreshTrigger = Channel(Channel.CONFLATED), + workspace = workspace, + agent = agent, + ) + return Fixture(environment, logger, settingsStore, workspace, agent, workspaceName, agentName) + } + + private fun isSessionStartedMessage(message: String): Boolean = + message.contains("session", ignoreCase = true) && + (message.contains("start", ignoreCase = true) || message.contains("creat", ignoreCase = true)) + + private fun isSessionDisposedMessage(message: String): Boolean = + message.contains("session", ignoreCase = true) && + (message.contains("dispos", ignoreCase = true) || + message.contains("remov", ignoreCase = true) || + message.contains("end", ignoreCase = true)) + + private data class Fixture( + val environment: CoderRemoteEnvironment, + val logger: CoderLogger, + val settingsStore: CoderSettingsStore, + val workspace: Workspace, + val agent: WorkspaceAgent, + val workspaceName: String, + val agentName: String, + ) { + fun currentSessionId(): SessionId? = SessionIdRegistry.findSession(workspaceName, agentName) + + fun removeSession() { + SessionIdRegistry.removeSession(workspaceName, agentName) + } + } +} diff --git a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt index 1ba8437..a0bc662 100644 --- a/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt +++ b/src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt @@ -12,6 +12,7 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceAgentStatus import com.coder.toolbox.sdk.v2.models.WorkspaceBuild import com.coder.toolbox.sdk.v2.models.WorkspaceResource import com.coder.toolbox.sdk.v2.models.WorkspaceStatus +import com.coder.toolbox.session.SessionId import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.views.CoderSetupWizardPage import com.coder.toolbox.views.state.StoredOAuthSession @@ -87,7 +88,7 @@ class CoderRemoteProviderTest { val agent = mockAgent("agent1") val workspace = mockWorkspace("ws1", WorkspaceStatus.RUNNING, listOf(mockResource(listOf(agent)))) coEvery { mockClient.workspaces(any()) } returns listOf(workspace) - every { mockCli.configSsh(any(), any(), any()) } throws FileNotFoundException("Permission denied") + every { mockCli.configSsh(any(), any(), any(), any()) } throws FileNotFoundException("Permission denied") // when val pollJob = remoteProvider.poll(mockClient, mockCli) @@ -102,6 +103,7 @@ class CoderRemoteProviderTest { val warningText = slot() verify(exactly = 1) { mockLogger.logAndShowWarning( + emptySet(), "SSH configuration could not be updated", capture(warningText), any(), @@ -112,6 +114,107 @@ class CoderRemoteProviderTest { pollJob.cancel() } + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `workspace change refreshes sessions after the workspace request`() = runTest { + every { mockContext.cs } returns CoroutineScope(StandardTestDispatcher(testScheduler)) + val firstSessionId = SessionId.generate() + val secondSessionId = SessionId.generate() + var sessionsStarted = false + val firstEnvironment = mockk(relaxed = true) { + every { id } returns "ws1.agent1" + every { currentSessionId() } answers { firstSessionId.takeIf { sessionsStarted } } + } + val secondEnvironment = mockk(relaxed = true) { + every { id } returns "ws1.agent2" + every { currentSessionId() } answers { secondSessionId.takeIf { sessionsStarted } } + } + remoteProvider.lastEnvironments.addAll(listOf(secondEnvironment, firstEnvironment)) + val workspace = mockWorkspace( + "ws1", + WorkspaceStatus.RUNNING, + listOf(mockResource(listOf(mockAgent("agent1"), mockAgent("agent2")))), + ) + coEvery { mockClient.workspaces(any()) } answers { + sessionsStarted = true + listOf(workspace) + } + + val pollJob = remoteProvider.poll(mockClient, mockCli) + runCurrent() + + verify(exactly = 1) { + mockLogger.info( + setOf(firstSessionId, secondSessionId), + match { it.startsWith("Workspaces have changed, reconfiguring CLI:") }, + ) + } + + pollJob.cancel() + } + + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `workspace poll failure is emitted for every current Toolbox SSH session`() = runTest { + every { mockContext.cs } returns CoroutineScope(StandardTestDispatcher(testScheduler)) + val firstSessionId = SessionId.generate() + val secondSessionId = SessionId.generate() + remoteProvider.lastEnvironments.addAll( + listOf( + mockk(relaxed = true) { + every { currentSessionId() } returns firstSessionId + }, + mockk(relaxed = true) { + every { currentSessionId() } returns secondSessionId + }, + ) + ) + val failure = IllegalStateException("poll failed") + coEvery { mockClient.workspaces(any()) } throws failure + + val pollJob = remoteProvider.poll(mockClient, mockCli) + runCurrent() + + verify(exactly = 1) { + mockLogger.error( + setOf(firstSessionId, secondSessionId), + failure, + "workspace polling error encountered", + ) + } + + pollJob.cancel() + } + + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `removed environment session is retained for its final shared configuration logs`() = runTest { + every { mockContext.cs } returns CoroutineScope(StandardTestDispatcher(testScheduler)) + val sessionId = SessionId.generate() + var disposed = false + val existingEnvironment = mockk(relaxed = true) { + every { id } returns "ws1.agent1" + every { currentSessionId() } answers { sessionId.takeUnless { disposed } } + every { dispose() } answers { disposed = true } + } + remoteProvider.lastEnvironments.add(existingEnvironment) + val stoppedWorkspace = mockWorkspace("ws1", WorkspaceStatus.STOPPED, emptyList()) + coEvery { mockClient.workspaces(any()) } returns listOf(stoppedWorkspace) + + val pollJob = remoteProvider.poll(mockClient, mockCli) + runCurrent() + + verify(exactly = 1) { existingEnvironment.dispose() } + verify(exactly = 1) { + mockLogger.info(setOf(sessionId), match { it.startsWith("Workspaces have changed, reconfiguring CLI:") }) + } + verify(exactly = 1) { + mockCli.configSsh(any(), setOf(sessionId), any(), any()) + } + + pollJob.cancel() + } + @Test @OptIn(ExperimentalCoroutinesApi::class) fun `identifier failures from SSH rendering are not treated as writable config errors`() = runTest { @@ -119,7 +222,7 @@ class CoderRemoteProviderTest { val agent = mockAgent("agent1") val workspace = mockWorkspace("ws1", WorkspaceStatus.RUNNING, listOf(mockResource(listOf(agent)))) coEvery { mockClient.workspaces(any()) } returns listOf(workspace) - every { mockCli.configSsh(any(), any(), any()) } throws + every { mockCli.configSsh(any(), any(), any(), any()) } throws InvalidCoderIdentifierException("The deployment returned an invalid workspace name") val pollJob = remoteProvider.poll(mockClient, mockCli) diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index 1b69775..fbbaf6a 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -8,6 +8,7 @@ import com.coder.toolbox.sdk.DataGen.Companion.workspace import com.coder.toolbox.sdk.v2.models.InvalidCoderIdentifierException import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.session.SessionId import com.coder.toolbox.settings.Environment import com.coder.toolbox.store.BINARY_DESTINATION import com.coder.toolbox.store.BINARY_DIRECTORY @@ -47,6 +48,7 @@ import com.sun.net.httpserver.HttpServer import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.BeforeAll @@ -76,6 +78,7 @@ private val noOpTextProgress: (String) -> Unit = { _ -> } internal class CoderCLIManagerTest { private val ui = mockk(relaxed = true) + private val underlyingLogger = mockk(relaxed = true) private val context = CoderToolboxContext( ui, mockk(relaxed = true), @@ -85,7 +88,7 @@ internal class CoderCLIManagerTest { mockk(), mockk(), mockk(), - mockk(relaxed = true), + underlyingLogger, mockk(relaxed = true), CoderSettingsStore( pluginTestSettingsStore(), @@ -706,7 +709,7 @@ internal class CoderCLIManagerTest { WorkspaceAddress.from(ws, a) } }.toSet(), - it.features, + feats = it.features, ) assertEquals(expectedConf, sshConfigPath.toFile().readText()) @@ -717,7 +720,7 @@ internal class CoderCLIManagerTest { } // Remove configuration. - ccm.configSsh(emptySet(), it.features) + ccm.configSsh(emptySet(), feats = it.features) // Remove is the configuration we expect after removing. assertEquals( @@ -728,6 +731,92 @@ internal class CoderCLIManagerTest { } } + @Test + fun `SSH setup keeps feature detection sessionless and correlates configuration logs`() { + val testDirectory = tmpdir.resolve("session-correlated-config-${UUID.randomUUID()}") + val binaryPath = if (getOS() == OS.WINDOWS) { + testDirectory.resolve("coder.bat") + } else { + testDirectory.resolve("coder") + } + binaryPath.parent.toFile().mkdirs() + binaryPath.toFile().writeText(mkbinVersion("2.25.0")) + if (getOS() != OS.WINDOWS) { + binaryPath.toFile().setExecutable(true) + } + val sshConfigPath = testDirectory.resolve("ssh.conf") + val settings = CoderSettingsStore( + pluginTestSettingsStore( + BINARY_DESTINATION to binaryPath.toString(), + ENABLE_DOWNLOADS to "false", + SSH_CONFIG_PATH to sshConfigPath.toString(), + ), + Environment(), + context.logger, + ) + val ccm = CoderCLIManager( + context.copy(settingsStore = settings), + URI("https://test.coder.invalid").toURL(), + ) + val sessionId = SessionId.generate() + val sessionPrefix = "client_session_id=$sessionId" + val workspace = workspace("foo", agents = mapOf("agent" to UUID.randomUUID().toString())) + val agent = workspace.latestBuild.resources.single().agents!!.single() + + ccm.configSsh( + setOf(WorkspaceAddress.from(workspace, agent)), + sessionIds = setOf(sessionId), + sshConfigPath = sshConfigPath.toString(), + ) + ccm.getHostname( + URI("https://test.coder.invalid").toURL(), + WorkspaceAddress.from(workspace, agent), + ) + + verify(exactly = 1) { + underlyingLogger.info("$sessionPrefix Configuring SSH config at $sshConfigPath") + } + verify(atLeast = 1) { + underlyingLogger.info(match { + it.startsWith("`${ccm.localBinaryPath} version --output json`:") && + it.contains("\"version\": \"2.25.0\"") + }) + } + verify(exactly = 1) { + underlyingLogger.info("No existing SSH config to modify") + } + verify(exactly = 0) { + underlyingLogger.info("$sessionPrefix No existing SSH config to modify") + } + verify(exactly = 1) { + underlyingLogger.info("$sessionPrefix Finished configuring SSH config") + } + verify(exactly = 0) { + underlyingLogger.info(match { + it.startsWith("$sessionPrefix `${ccm.localBinaryPath} version --output json`:") + }) + } + } + + @Test + fun `no-op SSH config removal is logged without a session`() { + val sshConfigPath = tmpdir.resolve("unmanaged-ssh-config-${UUID.randomUUID()}.conf") + sshConfigPath.toFile().writeText("Host unrelated" + System.lineSeparator()) + val ccm = CoderCLIManager(context, URI("https://test.coder.invalid").toURL()) + val sessionId = SessionId.generate() + val message = "No workspaces and no existing config blocks to remove" + + ccm.configSsh( + workspaceAddresses = emptySet(), + sessionIds = setOf(sessionId), + feats = Features(), + sshConfigPath = sshConfigPath.toString(), + ) + + verify(exactly = 1) { underlyingLogger.info(message) } + verify(exactly = 0) { underlyingLogger.info("client_session_id=$sessionId $message") } + } + @Test fun testMalformedConfig() { val tests = @@ -817,7 +906,7 @@ internal class CoderCLIManagerTest { } assertFailsWith { - ccm.configSsh(setOf(WorkspaceAddress.from(workspace, agent)), Features()) + ccm.configSsh(setOf(WorkspaceAddress.from(workspace, agent)), feats = Features()) } assertFalse(sshConfigPath.toFile().exists()) @@ -857,7 +946,7 @@ internal class CoderCLIManagerTest { val workspace = workspace("safe", agents = mapOf("agent" to UUID.randomUUID().toString())) val withAgent = workspace.latestBuild.resources.single().agents!!.single() - ccm.configSsh(setOf(WorkspaceAddress.from(workspace, withAgent)), Features()) + ccm.configSsh(setOf(WorkspaceAddress.from(workspace, withAgent)), feats = Features()) val updatedConfig = sshConfigPath.toFile().readText() assertFalse(updatedConfig.contains("unsafe-command")) diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt index 555d8fb..5aaac19 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -31,19 +31,66 @@ class CoderLoggerTest { verify(exactly = 1) { delegate.error(exception, "connection failed") } } + @Test + fun `nullable session logs are delegated unchanged when the id is null`() { + val exception = IllegalStateException("failed") + + logger.error(null, "error") + logger.error(null, exception, "exception") + logger.warn(null, "warning") + logger.warn(null, exception, "warning exception") + logger.debug(null, "debug") + logger.info(null, "info") + logger.info(message = "named info") + logger.error(exception = exception, message = "named exception") + + verify(exactly = 1) { delegate.error("error") } + verify(exactly = 1) { delegate.error(exception, "exception") } + verify(exactly = 1) { delegate.warn("warning") } + verify(exactly = 1) { delegate.warn(exception, "warning exception") } + verify(exactly = 1) { delegate.debug("debug") } + verify(exactly = 1) { delegate.info("info") } + verify(exactly = 1) { delegate.info("named info") } + verify(exactly = 1) { delegate.error(exception, "named exception") } + } + @Test fun `session-aware logs include the client session id`() { + val exception = IllegalStateException("failed") + logger.error(sessionId, "error") + logger.error(sessionId, exception, "exception") logger.warn(sessionId, "warning") + logger.warn(sessionId, exception, "warning exception") logger.debug(sessionId, "debug") logger.info(sessionId, "info") verify(exactly = 1) { delegate.error("$prefix error") } + verify(exactly = 1) { delegate.error(exception, "$prefix exception") } verify(exactly = 1) { delegate.warn("$prefix warning") } + verify(exactly = 1) { delegate.warn(exception, "$prefix warning exception") } verify(exactly = 1) { delegate.debug("$prefix debug") } verify(exactly = 1) { delegate.info("$prefix info") } } + @Test + fun `session sets fan out or emit one sessionless log when empty`() { + val secondSessionId = SessionId.generate() + val exception = IllegalStateException("failed") + + logger.info(setOf(sessionId, secondSessionId), "shared info") + logger.debug(setOf(sessionId, secondSessionId), "shared debug") + logger.error(emptySet(), exception, "shared error") + + verify(exactly = 1) { delegate.info("$prefix shared info") } + verify(exactly = 1) { delegate.info("client_session_id=$secondSessionId shared info") } + verify(exactly = 0) { delegate.info("shared info") } + verify(exactly = 1) { delegate.debug("$prefix shared debug") } + verify(exactly = 1) { delegate.debug("client_session_id=$secondSessionId shared debug") } + verify(exactly = 0) { delegate.debug("shared debug") } + verify(exactly = 1) { delegate.error(exception, "shared error") } + } + @Test fun `log and show logs and displays the same user message`() { logger.logAndShowInfo("Connection ready", "Connected to the workspace") @@ -75,4 +122,56 @@ class CoderLoggerTest { ) } } + + @Test + fun `session-aware log and show correlates logs without changing popup text`() { + val exception = IllegalStateException("failed") + + logger.logAndShowError(sessionId, "Connection failed", "Could not connect") + logger.logAndShowError(sessionId, "Connection crashed", "The connection crashed", exception) + logger.logAndShowWarning(sessionId, "Connection unstable", "The connection is unstable") + logger.logAndShowWarning(sessionId, "Connection failed", "The connection failed", exception) + + verify(exactly = 1) { delegate.error("$prefix Could not connect") } + verify(exactly = 1) { delegate.error(exception, "$prefix The connection crashed") } + verify(exactly = 1) { delegate.warn("$prefix The connection is unstable") } + verify(exactly = 1) { delegate.warn(exception, "$prefix The connection failed") } + verify(exactly = 1) { i18n.pnotr("Could not connect") } + verify(exactly = 1) { i18n.pnotr("The connection crashed") } + verify(exactly = 1) { i18n.pnotr("The connection is unstable") } + verify(exactly = 1) { i18n.pnotr("The connection failed") } + coVerify(exactly = 4) { + ui.showInfoPopup( + any(), + any(), + any(), + ) + } + } + + @Test + fun `session set log and show logs every session but displays one popup`() { + val secondSessionId = SessionId.generate() + val exception = IllegalStateException("failed") + + logger.logAndShowWarning( + setOf(sessionId, secondSessionId), + "Connection unstable", + "The connection is unstable", + exception, + ) + + verify(exactly = 1) { delegate.warn(exception, "$prefix The connection is unstable") } + verify(exactly = 1) { + delegate.warn(exception, "client_session_id=$secondSessionId The connection is unstable") + } + verify(exactly = 0) { delegate.warn(exception, "The connection is unstable") } + coVerify(exactly = 1) { + ui.showInfoPopup( + any(), + any(), + any(), + ) + } + } } diff --git a/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt index 924322c..970bb1c 100644 --- a/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt +++ b/src/test/kotlin/com/coder/toolbox/session/SessionIdRegistryTest.kt @@ -1,5 +1,10 @@ package com.coder.toolbox.session +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.diagnostics.CoderLogger +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll @@ -12,22 +17,32 @@ import kotlin.test.assertNull import kotlin.test.assertTrue class SessionIdRegistryTest { + private val logger = mockk(relaxed = true) + private val context = mockk(relaxed = true) + + init { + every { context.logger } returns logger + } + @Test fun `start session creates a correctly encoded id`() { val key = uniqueKey() - val sessionId = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + val sessionId = SessionIdRegistry.startSession(context, 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`() { + fun `start session reuses the 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) + val first = SessionIdRegistry.startSession(context, key.workspaceName, key.agentName) + val second = SessionIdRegistry.startSession(context, key.workspaceName, key.agentName) assertEquals(first, second) assertEquals(first, SessionIdRegistry.findSession(key.workspaceName, key.agentName)) + verify(exactly = 1) { + logger.info(first, "Created Toolbox SSH session for ${key.workspaceName}.${key.agentName}") + } } @Test @@ -37,9 +52,9 @@ class SessionIdRegistryTest { 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) + val first = SessionIdRegistry.startSession(context, workspaceOne, agentOne) + val differentWorkspace = SessionIdRegistry.startSession(context, workspaceTwo, agentOne) + val differentAgent = SessionIdRegistry.startSession(context, workspaceOne, agentTwo) assertNotEquals(first, differentWorkspace) assertNotEquals(first, differentAgent) @@ -53,27 +68,29 @@ class SessionIdRegistryTest { } @Test - fun `disposing an environment removes its session`() { + fun `removing a session gives the next connection a new id`() { val key = uniqueKey() - val disposedSession = SessionIdRegistry.startSession(key.workspaceName, key.agentName) + val removedSession = SessionIdRegistry.startSession(context, key.workspaceName, key.agentName) - assertEquals(disposedSession, SessionIdRegistry.removeSession(key.workspaceName, key.agentName)) - assertNull(SessionIdRegistry.findSession(key.workspaceName, key.agentName)) + assertEquals(removedSession, SessionIdRegistry.removeSession(key.workspaceName, key.agentName)) - val replacementSession = SessionIdRegistry.startSession(key.workspaceName, key.agentName) - assertNotEquals(disposedSession, replacementSession) + val replacementSession = SessionIdRegistry.startSession(context, key.workspaceName, key.agentName) + assertNotEquals(removedSession, replacementSession) } @Test fun `concurrent starts create only one session`() = runTest { val key = uniqueKey() - val sessions = List(100) { + val results = List(100) { async(Dispatchers.Default) { - SessionIdRegistry.startSession(key.workspaceName, key.agentName) + SessionIdRegistry.startSession(context, key.workspaceName, key.agentName) } }.awaitAll() - assertEquals(1, sessions.toSet().size) + assertEquals(1, results.toSet().size) + verify(exactly = 1) { + logger.info(results.first(), "Created Toolbox SSH session for ${key.workspaceName}.${key.agentName}") + } } private fun uniqueKey(): TestSessionKey { diff --git a/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt b/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt index 594bb5f..3a2d57e 100644 --- a/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/CoderProtocolHandlerTest.kt @@ -1,24 +1,31 @@ package com.coder.toolbox.util +import com.coder.toolbox.CoderRemoteEnvironment import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.CoderCLIManager +import com.coder.toolbox.diagnostics.CoderLogger import com.coder.toolbox.feed.Ide import com.coder.toolbox.feed.IdeFeedManager import com.coder.toolbox.feed.IdeType import com.coder.toolbox.feed.JetBrainsFeedService import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.sdk.DataGen +import com.coder.toolbox.session.SessionId +import com.coder.toolbox.session.SessionIdRegistry import com.jetbrains.toolbox.api.core.util.LoadableState import com.jetbrains.toolbox.api.remoteDev.connection.RemoteToolsHelper import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.verify import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNull @@ -34,6 +41,8 @@ class CoderProtocolHandlerTest { private lateinit var ideFeedManager: IdeFeedManager private lateinit var handler: CoderProtocolHandler private lateinit var remoteToolsHelper: RemoteToolsHelper + private lateinit var logger: CoderLogger + private lateinit var environment: CoderRemoteEnvironment // Test Coroutine Scope private val dispatcher = StandardTestDispatcher() @@ -59,9 +68,15 @@ class CoderProtocolHandlerTest { feedService = mockk(relaxed = true) ideFeedManager = IdeFeedManager(context, feedService) remoteToolsHelper = mockk(relaxed = true) + logger = mockk(relaxed = true) + environment = mockk { + every { id } returns "env-1" + every { currentSessionId() } returns null + } every { context.cs } returns CoroutineScope(dispatcher) every { context.remoteIdeOrchestrator } returns remoteToolsHelper + every { context.logger } returns logger handler = CoderProtocolHandler( context, @@ -287,7 +302,7 @@ class CoderProtocolHandlerTest { fun `given empty available tools when resolving latest eap then returns null`() = runTest(dispatcher) { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns emptyList() - assertNull(handler.resolveIdeIdentifier("env-1", "RR", "latest_eap")) + assertNull(handler.resolveIdeIdentifier(environment, "RR", "latest_eap")) } @Test @@ -297,7 +312,7 @@ class CoderProtocolHandlerTest { // Feed returns empty or irrelevant EAPs coEvery { feedService.fetchEapFeed() } returns emptyList() - assertEquals("RR-241.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_eap")) + assertEquals("RR-241.1", handler.resolveIdeIdentifier(environment, "RR", "latest_eap")) } @Test @@ -310,14 +325,14 @@ class CoderProtocolHandlerTest { Ide("RR", "243.1", "2024.3", IdeType.EAP) ) - assertEquals("RR-243.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_eap")) + assertEquals("RR-243.1", handler.resolveIdeIdentifier(environment, "RR", "latest_eap")) } @Test fun `given empty available tools when resolving latest release then returns null`() = runTest(dispatcher) { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns emptyList() - assertNull(handler.resolveIdeIdentifier("env-1", "RR", "latest_release")) + assertNull(handler.resolveIdeIdentifier(environment, "RR", "latest_release")) } @Test @@ -326,7 +341,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-243.1", "RR-242.1") coEvery { feedService.fetchReleaseFeed() } returns emptyList() - assertEquals("RR-243.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_release")) + assertEquals("RR-243.1", handler.resolveIdeIdentifier(environment, "RR", "latest_release")) } @Test @@ -339,7 +354,7 @@ class CoderProtocolHandlerTest { Ide("RR", "242.1", "2024.2", IdeType.RELEASE) ) - assertEquals("RR-242.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_release")) + assertEquals("RR-242.1", handler.resolveIdeIdentifier(environment, "RR", "latest_release")) } @Test @@ -347,7 +362,7 @@ class CoderProtocolHandlerTest { runTest(dispatcher) { coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns listOf("RR-240.1", "RR-241.1") - assertEquals("RR-241.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_installed")) + assertEquals("RR-241.1", handler.resolveIdeIdentifier(environment, "RR", "latest_installed")) } @Test @@ -356,7 +371,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns emptyList() coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-243.1", "RR-242.1") - assertEquals("RR-243.1", handler.resolveIdeIdentifier("env-1", "RR", "latest_installed")) + assertEquals("RR-243.1", handler.resolveIdeIdentifier(environment, "RR", "latest_installed")) } @Test @@ -365,7 +380,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns emptyList() coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns emptyList() - assertNull(handler.resolveIdeIdentifier("env-1", "RR", "latest_installed")) + assertNull(handler.resolveIdeIdentifier(environment, "RR", "latest_installed")) } @Test @@ -374,7 +389,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-241.1", "RR-242.1") coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns listOf("RR-251.1", "RR-252.1") - assertEquals("RR-251.1", handler.resolveIdeIdentifier("env-1", "RR", "251.1")) + assertEquals("RR-251.1", handler.resolveIdeIdentifier(environment, "RR", "251.1")) } @Test @@ -383,7 +398,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-241.1", "RR-242.1") coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns listOf("RR-251.1", "RR-252.1") - assertEquals("RR-241.1", handler.resolveIdeIdentifier("env-1", "RR", "241.1")) + assertEquals("RR-241.1", handler.resolveIdeIdentifier(environment, "RR", "241.1")) } @Test @@ -392,7 +407,7 @@ class CoderProtocolHandlerTest { coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-241.1", "RR-242.1") coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns listOf("RR-251.1", "RR-252.1") - assertNull(handler.resolveIdeIdentifier("env-1", "RR", "221.1")) + assertNull(handler.resolveIdeIdentifier(environment, "RR", "221.1")) } @Test @@ -407,8 +422,91 @@ class CoderProtocolHandlerTest { "RR-252.1" ) - assertEquals("RR-241.1.2", handler.resolveIdeIdentifier("env-1", "RR", "241.1")) + assertEquals("RR-241.1.2", handler.resolveIdeIdentifier(environment, "RR", "241.1")) + } + + @Test + fun `IDE resolution logs use the current environment session`() = runTest(dispatcher) { + val sessionId = SessionId.generate() + val environment = mockk { + every { id } returns "env-1" + every { currentSessionId() } returns sessionId + } + coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns listOf("RR-241.1") + coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns emptyList() + + assertEquals("RR-241.1", handler.resolveIdeIdentifier(environment, "RR", "241.1")) + + verify(exactly = 1) { logger.info(sessionId, "Available RR IDEs: [241.1]") } + verify(exactly = 1) { logger.info(sessionId, "Installed RR IDEs: []") } + verify(exactly = 0) { logger.info("Available RR IDEs: [241.1]") } + verify(exactly = 0) { logger.info("Installed RR IDEs: []") } + } + + @Test + fun `IDE resolution errors use the current environment session`() = runTest(dispatcher) { + val sessionId = SessionId.generate() + val environment = mockk { + every { id } returns "env-1" + every { currentSessionId() } returns sessionId + } + coEvery { remoteToolsHelper.getAvailableRemoteTools("env-1", "RR") } returns emptyList() + coEvery { remoteToolsHelper.getInstalledRemoteTools("env-1", "RR") } returns emptyList() + val message = "Can't launch EAP for RR because no version is available on env-1" + + assertNull(handler.resolveIdeIdentifier(environment, "RR", "latest_eap")) + + verify(exactly = 1) { logger.logAndShowError(sessionId, "Can't handle URI", message) } + verify(exactly = 0) { logger.logAndShowError("Can't handle URI", message) } + } + + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `URI IDE launch observes the session activated after requesting SSH`() = runTest(dispatcher) { + val workspace = DataGen.workspace("delta-eridani", agents = SINGLE_AGENT) + val environmentId = "${workspace.name}.${AGENT_BOB.name}" + val environment = mockk(relaxed = true) + every { environment.id } returns environmentId + every { environment.currentSessionId() } answers { + SessionIdRegistry.findSession(workspace.name, AGENT_BOB.name) + } + coEvery { remoteToolsHelper.getAvailableRemoteTools(environmentId, "RR") } returns emptyList() + coEvery { remoteToolsHelper.getInstalledRemoteTools(environmentId, "RR") } returns listOf("RR-241.1") + val correlatedHandler = CoderProtocolHandler( + context, + ideFeedManager, + Channel(Channel.CONFLATED), + MutableStateFlow(LoadableState.Value(listOf(environment))), + ) + val restClient = mockk(relaxed = true) + val cli = mockk(relaxed = true) + coEvery { restClient.workspaces(null) } returns listOf(workspace) + coEvery { restClient.workspace(workspace.id) } returns workspace + + correlatedHandler.handle( + mapOf( + "workspace" to workspace.name, + "ide_product_code" to "RR", + "ide_build_number" to "241.1", + ), + URI("https://coder.example.com").toURL(), + restClient, + cli, + ) + val sessionId = SessionIdRegistry.startSession(context, workspace.name, AGENT_BOB.name) + try { + advanceUntilIdle() + + verify(exactly = 1) { environment.startSshConnection() } + verify(exactly = 1) { + logger.info(sessionId, "Selected IDE RR-241.1 for RR with hint 241.1") + } + verify(exactly = 1) { logger.info(sessionId, "Launching RR-241.1 on $environmentId") } + verify(exactly = 0) { logger.info("Launching RR-241.1 on $environmentId") } + } finally { + SessionIdRegistry.removeSession(workspace.name, AGENT_BOB.name) } + } internal data class AgentTestData(val name: String, val id: String) { val uuid: UUID get() = UUID.fromString(id) diff --git a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt index 4baae3a..0222e42 100644 --- a/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt +++ b/src/test/kotlin/com/coder/toolbox/util/ConnectionMonitoringServiceTest.kt @@ -8,6 +8,7 @@ import com.coder.toolbox.sdk.v2.models.WorkspaceAgentLifecycleState 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 com.coder.toolbox.session.SessionIdRegistry import io.mockk.clearMocks import io.mockk.every import io.mockk.mockk @@ -32,7 +33,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -43,7 +44,29 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } + } + + @Test + fun `connection warning uses the current workspace and agent session`() { + val service = ConnectionMonitoringService(context) + val workspace = createWorkspace(WorkspaceStatus.RUNNING) + val agent = createAgent(WorkspaceAgentStatus.DISCONNECTED, WorkspaceAgentLifecycleState.READY) + val sessionId = SessionIdRegistry.startSession(context, workspace.name, agent.name) + + try { + service.checkConnectionStatus(workspace, agent) + + verify(exactly = 1) { + logger.logAndShowWarning( + sessionId, + "Unstable connection detected", + any(), + ) + } + } finally { + SessionIdRegistry.removeSession(workspace.name, agent.name) + } } @Test @@ -54,7 +77,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -65,7 +88,7 @@ class ConnectionMonitoringServiceTest { service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -83,7 +106,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 0) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 0) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -98,7 +121,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(workspace, agent) - verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -115,7 +138,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } } @Test @@ -132,7 +155,7 @@ class ConnectionMonitoringServiceTest { // Second call should not trigger notification service.checkConnectionStatus(ws2, agent2) - verify(exactly = 1) { logger.logAndShowWarning(any(), any()) } + verify(exactly = 1) { logger.logAndShowWarning(null, any(), any()) } } diff --git a/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt b/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt new file mode 100644 index 0000000..41eff3e --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/views/ActionTest.kt @@ -0,0 +1,68 @@ +package com.coder.toolbox.views + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.diagnostics.CoderLogger +import com.coder.toolbox.session.SessionId +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlin.test.Test + +class ActionTest { + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `action failure uses the current session`() = runTest { + val context = mockk(relaxed = true) + val logger = mockk(relaxed = true) + val sessionId = SessionId.generate() + val testScope = this + every { context.cs } returns testScope + every { context.logger } returns logger + val action = Action(context, "Stop workspace") { + error("stop failed") + }.withCurrentSessionId { sessionId } + + action.run() + advanceUntilIdle() + + verify(exactly = 1) { + logger.logAndShowError( + sessionId, + "Error while running `Stop workspace`", + "stop failed", + any(), + ) + } + } + + @Test + @OptIn(ExperimentalCoroutinesApi::class) + fun `action failure resolves the session when the error is logged`() = runTest { + val context = mockk(relaxed = true) + val logger = mockk(relaxed = true) + val sessionId = SessionId.generate() + var currentSessionId: SessionId? = sessionId + val testScope = this + every { context.cs } returns testScope + every { context.logger } returns logger + val action = Action(context, "Stop workspace") { + currentSessionId = null + error("stop failed") + }.withCurrentSessionId { currentSessionId } + + action.run() + advanceUntilIdle() + + verify(exactly = 1) { + logger.logAndShowError( + null, + "Error while running `Stop workspace`", + "stop failed", + any(), + ) + } + } +} diff --git a/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt b/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt index 9ead00b..58c207b 100644 --- a/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt +++ b/src/test/kotlin/com/coder/toolbox/views/EnvironmentViewTest.kt @@ -5,6 +5,7 @@ import com.coder.toolbox.cli.CoderCLIManager import com.coder.toolbox.cli.WorkspaceAddress import com.coder.toolbox.sdk.v2.models.Workspace import com.coder.toolbox.sdk.v2.models.WorkspaceAgent +import com.coder.toolbox.session.SessionIdRegistry import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.util.OS import com.jetbrains.toolbox.api.remoteDev.deploy.DeploymentTarget @@ -14,6 +15,7 @@ import kotlinx.coroutines.runBlocking import java.net.URL import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotEquals class EnvironmentViewTest { @Test @@ -63,4 +65,48 @@ class EnvironmentViewTest { assertEquals(configuredPath, connectionInfo.sshConfigPath) } + + @Test + fun `connection info resolves the current Toolbox session for the SSH process`() = runBlocking { + val context = mockk(relaxed = true) + val cli = mockk() + val workspace = mockk { + every { name } returns "workspace" + every { ownerName } returns "owner" + } + val agent = mockk { + every { name } returns "agent" + } + val url = URL("https://coder.example.com") + every { cli.getHostname(url, any()) } returns "coder.example.com--workspace.agent" + + val view = EnvironmentView(context, url, cli, workspace, agent) + val connectionInfoWithoutSession = view.getConnectionInfo() + assertEquals(null, connectionInfoWithoutSession.environment) + + val firstSessionId = SessionIdRegistry.startSession(context, workspace.name, agent.name) + try { + val firstConnectionInfo = view.getConnectionInfo() + assertEquals( + mapOf("CODER_TRACE_SESSION_ID" to firstSessionId.value), + firstConnectionInfo.environment, + ) + + SessionIdRegistry.removeSession(workspace.name, agent.name) + val secondSessionId = SessionIdRegistry.startSession(context, workspace.name, agent.name) + val secondConnectionInfo = view.getConnectionInfo() + + assertNotEquals(firstSessionId, secondSessionId) + assertEquals( + mapOf("CODER_TRACE_SESSION_ID" to firstSessionId.value), + firstConnectionInfo.environment, + ) + assertEquals( + mapOf("CODER_TRACE_SESSION_ID" to secondSessionId.value), + secondConnectionInfo.environment, + ) + } finally { + SessionIdRegistry.removeSession(workspace.name, agent.name) + } + } } From 62f6d795dc04bbea6182e915773b4a2bd6eca234 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 4 Sep 2026 00:32:43 +0300 Subject: [PATCH 6/6] Clean up CoderLogger overloads Remove unused session-aware overloads and default arguments, then update the focused tests to match the smaller logging API. --- .../coder/toolbox/diagnostics/CoderLogger.kt | 33 ++++++------------- .../toolbox/diagnostics/CoderLoggerTest.kt | 13 +------- 2 files changed, 11 insertions(+), 35 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt index 1569b35..618e888 100644 --- a/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt +++ b/src/main/kotlin/com/coder/toolbox/diagnostics/CoderLogger.kt @@ -26,11 +26,7 @@ class CoderLogger( private val cs: CoroutineScope, private val i18n: LocalizableStringFactory, ) : Logger by delegate { - fun error(sessionId: SessionId? = null, message: String) { - delegate.error(withSessionId(sessionId, message)) - } - - fun error(sessionId: SessionId? = null, exception: Throwable, message: String) { + fun error(sessionId: SessionId?, exception: Throwable, message: String) { delegate.error(exception, withSessionId(sessionId, message)) } @@ -38,19 +34,15 @@ class CoderLogger( sessionIds.onceOrForEach { error(it, exception, message) } } - fun warn(sessionId: SessionId? = null, message: String) { + fun warn(sessionId: SessionId?, message: String) { delegate.warn(withSessionId(sessionId, message)) } - fun warn(sessionId: SessionId? = null, exception: Throwable, message: String) { - delegate.warn(exception, withSessionId(sessionId, message)) - } - fun warn(sessionIds: Set, exception: Throwable, message: String) { - sessionIds.onceOrForEach { warn(it, exception, message) } + sessionIds.onceOrForEach { delegate.warn(exception, withSessionId(it, message)) } } - fun debug(sessionId: SessionId? = null, message: String) { + fun debug(sessionId: SessionId?, message: String) { delegate.debug(withSessionId(sessionId, message)) } @@ -58,7 +50,7 @@ class CoderLogger( sessionIds.onceOrForEach { debug(it, message) } } - fun info(sessionId: SessionId? = null, message: String) { + fun info(sessionId: SessionId?, message: String) { delegate.info(withSessionId(sessionId, message)) } @@ -71,8 +63,8 @@ class CoderLogger( showInfoPopup(title, error) } - fun logAndShowError(sessionId: SessionId? = null, title: String, error: String) { - error(sessionId, error) + fun logAndShowError(sessionId: SessionId?, title: String, error: String) { + delegate.error(withSessionId(sessionId, error)) showInfoPopup(title, error) } @@ -81,7 +73,7 @@ class CoderLogger( showInfoPopup(title, error) } - fun logAndShowError(sessionId: SessionId? = null, title: String, error: String, exception: Throwable) { + fun logAndShowError(sessionId: SessionId?, title: String, error: String, exception: Throwable) { error(sessionId, exception, error) showInfoPopup(title, error) } @@ -101,23 +93,18 @@ class CoderLogger( showInfoPopup(title, warning) } - fun logAndShowWarning(sessionId: SessionId? = null, title: String, warning: String) { + fun logAndShowWarning(sessionId: SessionId?, title: String, warning: String) { warn(sessionId, warning) showInfoPopup(title, warning) } - fun logAndShowWarning(sessionId: SessionId? = null, title: String, warning: String, exception: Throwable) { - warn(sessionId, exception, warning) - showInfoPopup(title, warning) - } - fun logAndShowWarning( sessionIds: Set, title: String, warning: String, exception: Throwable, ) { - sessionIds.onceOrForEach { warn(it, exception, warning) } + sessionIds.onceOrForEach { delegate.warn(exception, withSessionId(it, warning)) } showInfoPopup(title, warning) } diff --git a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt index 5aaac19..af155ef 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -35,19 +35,15 @@ class CoderLoggerTest { fun `nullable session logs are delegated unchanged when the id is null`() { val exception = IllegalStateException("failed") - logger.error(null, "error") logger.error(null, exception, "exception") logger.warn(null, "warning") - logger.warn(null, exception, "warning exception") logger.debug(null, "debug") logger.info(null, "info") logger.info(message = "named info") logger.error(exception = exception, message = "named exception") - verify(exactly = 1) { delegate.error("error") } verify(exactly = 1) { delegate.error(exception, "exception") } verify(exactly = 1) { delegate.warn("warning") } - verify(exactly = 1) { delegate.warn(exception, "warning exception") } verify(exactly = 1) { delegate.debug("debug") } verify(exactly = 1) { delegate.info("info") } verify(exactly = 1) { delegate.info("named info") } @@ -58,17 +54,13 @@ class CoderLoggerTest { fun `session-aware logs include the client session id`() { val exception = IllegalStateException("failed") - logger.error(sessionId, "error") logger.error(sessionId, exception, "exception") logger.warn(sessionId, "warning") - logger.warn(sessionId, exception, "warning exception") logger.debug(sessionId, "debug") logger.info(sessionId, "info") - verify(exactly = 1) { delegate.error("$prefix error") } verify(exactly = 1) { delegate.error(exception, "$prefix exception") } verify(exactly = 1) { delegate.warn("$prefix warning") } - verify(exactly = 1) { delegate.warn(exception, "$prefix warning exception") } verify(exactly = 1) { delegate.debug("$prefix debug") } verify(exactly = 1) { delegate.info("$prefix info") } } @@ -130,17 +122,14 @@ class CoderLoggerTest { logger.logAndShowError(sessionId, "Connection failed", "Could not connect") logger.logAndShowError(sessionId, "Connection crashed", "The connection crashed", exception) logger.logAndShowWarning(sessionId, "Connection unstable", "The connection is unstable") - logger.logAndShowWarning(sessionId, "Connection failed", "The connection failed", exception) verify(exactly = 1) { delegate.error("$prefix Could not connect") } verify(exactly = 1) { delegate.error(exception, "$prefix The connection crashed") } verify(exactly = 1) { delegate.warn("$prefix The connection is unstable") } - verify(exactly = 1) { delegate.warn(exception, "$prefix The connection failed") } verify(exactly = 1) { i18n.pnotr("Could not connect") } verify(exactly = 1) { i18n.pnotr("The connection crashed") } verify(exactly = 1) { i18n.pnotr("The connection is unstable") } - verify(exactly = 1) { i18n.pnotr("The connection failed") } - coVerify(exactly = 4) { + coVerify(exactly = 3) { ui.showInfoPopup( any(), any(),