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..618e888 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,88 @@ class CoderLogger( private val cs: CoroutineScope, private val i18n: LocalizableStringFactory, ) : Logger by delegate { - fun error(sessionId: SessionId, message: String) { - delegate.error(withSessionId(sessionId, message)) + fun error(sessionId: SessionId?, exception: Throwable, message: String) { + delegate.error(exception, withSessionId(sessionId, message)) } - fun warn(sessionId: SessionId, message: String) { + fun error(sessionIds: Set, exception: Throwable, message: String) { + sessionIds.onceOrForEach { error(it, exception, message) } + } + + fun warn(sessionId: SessionId?, message: String) { delegate.warn(withSessionId(sessionId, message)) } - fun debug(sessionId: SessionId, message: String) { + fun warn(sessionIds: Set, exception: Throwable, message: String) { + sessionIds.onceOrForEach { delegate.warn(exception, withSessionId(it, message)) } + } + + fun debug(sessionId: SessionId?, 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?, 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?, title: String, error: String) { + delegate.error(withSessionId(sessionId, error)) + showInfoPopup(title, error) + } + fun logAndShowError(title: String, error: String, exception: Throwable) { error(exception, error) showInfoPopup(title, error) } + fun logAndShowError(sessionId: SessionId?, 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?, title: String, warning: String) { + warn(sessionId, warning) + showInfoPopup(title, warning) + } + + fun logAndShowWarning( + sessionIds: Set, + title: String, + warning: String, + exception: Throwable, + ) { + sessionIds.onceOrForEach { delegate.warn(exception, withSessionId(it, warning)) } + showInfoPopup(title, warning) + } + fun logAndShowWarning(title: String, warning: String, exception: Throwable) { warn(exception, warning) showInfoPopup(title, warning) @@ -94,4 +145,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..af155ef 100644 --- a/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/diagnostics/CoderLoggerTest.kt @@ -31,19 +31,58 @@ 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, exception, "exception") + logger.warn(null, "warning") + 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(exception, "exception") } + verify(exactly = 1) { delegate.warn("warning") } + 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`() { - logger.error(sessionId, "error") + val exception = IllegalStateException("failed") + + logger.error(sessionId, exception, "exception") logger.warn(sessionId, "warning") 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.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 +114,53 @@ 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") + + 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) { i18n.pnotr("Could not connect") } + verify(exactly = 1) { i18n.pnotr("The connection crashed") } + verify(exactly = 1) { i18n.pnotr("The connection is unstable") } + coVerify(exactly = 3) { + 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) + } + } }