From 522f9d420a40e1d85443d3ecc09d6197b8f5c6fe Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Fri, 18 Sep 2026 14:26:21 +0200 Subject: [PATCH 1/2] fix: queue workspace updates as one build `coder update` runs a stop build and then a start build. Between the two, the extension's own `coder ssh` reconnect autostarts the workspace on the old template version, so the update's start build is rejected with "A workspace build is already active" and the workspace comes back unchanged. On servers from 2.36, post a single build that carries the start in `on_success`, so nothing can take the build slot in between. Leave `template_version_id` unset on the follow-up build, because pinning it requires template update permission and the update targets the active version anyway. Servers before 2.36 keep the `coder update` path, and CLIs before 2.24 keep the REST path. While that build is queued, the state machine follows it instead of starting the workspace itself, and a failed update asks before connecting to the existing version instead of falling back silently. Split the version-keyed capabilities into `CliFeatureSet` and `ServerFeatureSet` so the new check reads the deployment version rather than the CLI's. The Tasks panel moves with it, bounded to the releases that serve `/api/v2/tasks`: 2.29 through 2.34. Fixes #1095 --- CHANGELOG.md | 7 + src/api/workspace.ts | 60 +++++++-- src/commands.ts | 6 +- src/core/cliCredentialManager.ts | 8 +- src/extension.ts | 4 +- src/featureSet.ts | 40 +++--- src/instrumentation/workspace.ts | 24 +++- src/remote/remote.ts | 55 +++++--- src/remote/workspaceStateMachine.ts | 99 ++++++++++---- src/settings/cli.ts | 4 +- test/unit/api/workspace.test.ts | 77 +++++++++-- test/unit/cliConfig.test.ts | 4 +- .../command/updateWorkspace.telemetry.test.ts | 99 ++++++-------- test/unit/featureSet.test.ts | 37 ++--- test/unit/instrumentation/workspace.test.ts | 24 ++++ test/unit/remote/remote.test.ts | 61 ++++++++- .../unit/remote/workspaceStateMachine.test.ts | 126 ++++++++++++++---- 17 files changed, 534 insertions(+), 201 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8ce8c5ff6..74efeeb6cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,13 @@ so connections stop failing with "Bad owner or permissions". Only you, SYSTEM, and Administrators keep access to them. The extension leaves your own SSH config untouched and needs no admin rights. +- Update a workspace in one build on Coder 2.36 and later. The connection's + own autostart could take the build slot between the update's stop and start, + so the workspace came back on the template version it already had. +- Ask whether to connect to the existing version when an update fails, instead + of connecting to it with only a warning. +- Hide the Tasks panel on deployments before 2.29, which do not serve + `/api/v2/tasks` and answered every poll with a 404. ## [v1.16.3](https://github.com/coder/vscode-coder/releases/tag/v1.16.3) 2026-09-14 diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 7ab6f28b2d..9824b81423 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -7,17 +7,20 @@ import { errToStr, createWorkspaceIdentifier } from "./api-helper"; import type { Api } from "coder/site/src/api/api"; import type { + CreateWorkspaceBuildOnSuccessRequest, ProvisionerJobLog, Workspace, WorkspaceAgentLog, WorkspaceBuildParameter, } from "coder/site/src/api/typesGenerated"; -import type { FeatureSet } from "../featureSet"; +import type { CliFeatureSet, ServerFeatureSet } from "../featureSet"; import type { UnidirectionalStream } from "../websocket/eventStreamConnection"; import type { CoderApi } from "./coderApi"; +const BUILD_REASON = "vscode_connection"; + /** Opens a stream once; subsequent open() calls are no-ops until closed. */ export class LazyStream { private stream: UnidirectionalStream | null = null; @@ -54,7 +57,8 @@ interface CliContext { binPath: string; workspace: Workspace; write: (data: string) => void; - featureSet: FeatureSet; + cliFeatures: CliFeatureSet; + serverFeatures: ServerFeatureSet; } /** Streams CLI output via `ctx.write`; rejects with stderr on non-zero exit. */ @@ -107,8 +111,8 @@ export async function startWorkspace(ctx: CliContext): Promise { } const args = ["start", "--yes"]; - if (ctx.featureSet.buildReason) { - args.push("--reason", "vscode_connection"); + if (ctx.cliFeatures.buildReason) { + args.push("--reason", BUILD_REASON); } await runCliCommand(ctx, args); @@ -118,14 +122,18 @@ export async function startWorkspace(ctx: CliContext): Promise { /** * Update a workspace to the latest template version. Callers must collect * any newly-required parameters via `collectUpdateParameters` first; this - * function does not prompt. Falls back to the REST API on CLIs older than - * 2.24. + * function does not prompt. On servers before 2.36, updating takes two + * builds: `coder update`, or the REST API on CLIs before 2.24. */ export async function updateWorkspace( ctx: CliContext, parameters: WorkspaceBuildParameter[], ): Promise { - if (!ctx.featureSet.cliUpdate) { + if (ctx.serverFeatures.onSuccessBuild) { + return updateWorkspaceInOneBuild(ctx, parameters); + } + + if (!ctx.cliFeatures.cliUpdate) { return updateWorkspaceViaApi(ctx, parameters); } @@ -137,6 +145,40 @@ export async function updateWorkspace( return ctx.restClient.getWorkspace(ctx.workspace.id); } +/** + * Stop and start in one build, so nothing can take the slot in between. The + * returned workspace carries the stop build; the server starts it after. + */ +async function updateWorkspaceInOneBuild( + ctx: CliContext, + parameters: WorkspaceBuildParameter[], +): Promise { + // The build may have changed while parameters were collected. + const workspace = await ctx.restClient.getWorkspace(ctx.workspace.id); + const start: CreateWorkspaceBuildOnSuccessRequest = { + transition: "start", + rich_parameter_values: parameters, + }; + const running = workspace.latest_build.status === "running"; + + ctx.write( + `${running ? "Restarting" : "Starting"} workspace with the updated template...\r\n`, + ); + const build = await ctx.restClient.postWorkspaceBuild( + workspace.id, + running + ? { transition: "stop", reason: BUILD_REASON, on_success: start } + : { + ...start, + reason: BUILD_REASON, + // Pinning a follow-up build needs template update permission, + // so only a lone start can name the version. + template_version_id: workspace.template_active_version_id, + }, + ); + return { ...workspace, latest_build: build }; +} + async function updateWorkspaceViaApi( ctx: CliContext, parameters: WorkspaceBuildParameter[], @@ -145,8 +187,8 @@ async function updateWorkspaceViaApi( ctx.write("Stopping workspace for update...\r\n"); const stopBuild = await ctx.restClient.stopWorkspace(ctx.workspace.id); const stoppedJob = await ctx.restClient.waitForBuild(stopBuild); - if (stoppedJob?.status === "canceled") { - throw new Error("Workspace update cancelled during stop"); + if (stoppedJob?.status !== "succeeded") { + throw new Error("Workspace update stop build did not succeed"); } } diff --git a/src/commands.ts b/src/commands.ts index 9a337f7eb3..8ed44367d0 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -13,7 +13,7 @@ import { runDiagnosticCli } from "./command/diagnosticFlow"; import * as cliExec from "./core/cliExec"; import { CertificateError } from "./error/certificateError"; import { raceWithAbort, toError } from "./error/errorUtils"; -import { type FeatureSet, featureSetForVersion } from "./featureSet"; +import { type CliFeatureSet, cliFeatureSet } from "./featureSet"; import { AuthTelemetry, type AuthLoginOutcome, @@ -1467,7 +1467,7 @@ export class Commands { /** Resolve a CliEnv, preferring a locally cached binary over a network fetch. */ private async resolveCliEnv( client: CoderApi, - ): Promise { + ): Promise { const baseUrl = client.getAxiosInstance().defaults.baseURL; if (!baseUrl) { throw new Error("You are not logged in"); @@ -1477,7 +1477,7 @@ export class Commands { (await this.cliManager.locateBinary(baseUrl)) ?? (await this.cliManager.fetchBinary(client)); const version = semver.parse(await cliExec.version(binary)); - const featureSet = featureSetForVersion(version); + const featureSet = cliFeatureSet(version); const configDir = this.pathResolver.getGlobalConfigDir(safeHost); const configs = vscode.workspace.getConfiguration(); const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir); diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index bfcdb0d13a..099eb1d6c7 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -4,7 +4,7 @@ import { promisify } from "node:util"; import * as semver from "semver"; import { isAbortError } from "../error/errorUtils"; -import { featureSetForVersion, type FeatureSet } from "../featureSet"; +import { cliFeatureSet, type CliFeatureSet } from "../featureSet"; import { categorizeCredentialError, CredentialCliError, @@ -36,7 +36,7 @@ const EXEC_LOG_INTERVAL_MS = 5_000; interface ResolvedCli { binPath: string; - featureSet: FeatureSet; + featureSet: CliFeatureSet; auth: CliAuth; flags: string[]; } @@ -218,9 +218,7 @@ export class CliCredentialManager { if (!binPath) { return undefined; } - const featureSet = featureSetForVersion( - semver.parse(await version(binPath)), - ); + const featureSet = cliFeatureSet(semver.parse(await version(binPath))); const configDir = this.pathResolver.getGlobalConfigDir(toSafeHost(url)); const auth = resolveCliAuth(configs, featureSet, url, configDir); return { binPath, featureSet, auth, flags: getGlobalFlags(configs, auth) }; diff --git a/src/extension.ts b/src/extension.ts index 8feb074615..bb32b36c65 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -16,7 +16,7 @@ import { ServiceContainer } from "./core/container"; import { DeploymentManager } from "./deployment/deploymentManager"; import { CertificateError } from "./error/certificateError"; import { getErrorDetail, toError } from "./error/errorUtils"; -import { tasksSupported } from "./featureSet"; +import { serverFeatureSet } from "./featureSet"; import { ActivationTelemetry, type ActivationTracer, @@ -209,7 +209,7 @@ async function doActivate( if (deploymentManager.session.current.kind === "signedIn") { try { const buildInfo = await client.getBuildInfo(); - supported = tasksSupported(semver.parse(buildInfo.version)); + supported = serverFeatureSet(semver.parse(buildInfo.version)).tasks; } catch (error) { output.warn( "Unable to fetch deployment version for Tasks panel", diff --git a/src/featureSet.ts b/src/featureSet.ts index 0b86f03149..29433e2fd0 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -1,6 +1,7 @@ import type * as semver from "semver"; -export interface FeatureSet { +/** Capabilities keyed to the Coder CLI version. */ +export interface CliFeatureSet { cliLogin: boolean; proxyLogDirectory: boolean; wildcardSSH: boolean; @@ -13,8 +14,14 @@ export interface FeatureSet { allowRedirects: boolean; } +/** Capabilities keyed to the Coder server (REST API) version. */ +export interface ServerFeatureSet { + tasks: boolean; + onSuccessBuild: boolean; +} + /** - * True when the CLI version is at least `minVersion`, or is a dev build. + * True when the version is at least `minVersion`, or is a dev build. * Returns false for null (unknown) versions. */ function versionAtLeast( @@ -27,20 +34,8 @@ function versionAtLeast( return version.compare(minVersion) >= 0 || version.prerelease[0] === "devel"; } -/** - * True when the deployment predates the June 2026 Tasks deprecation - * (2.34 and below). The deprecated Tasks panel is hidden everywhere else. - */ -export function tasksSupported(version: semver.SemVer | null): boolean { - return version !== null && !versionAtLeast(version, "2.35.0"); -} - -/** - * Builds and returns a FeatureSet object for a given coder version. - */ -export function featureSetForVersion( - version: semver.SemVer | null, -): FeatureSet { +/** Capabilities of the given CLI version. */ +export function cliFeatureSet(version: semver.SemVer | null): CliFeatureSet { return { // `coder login --use-token-as-session` to write a token (file or keyring). // The extension relies on this, so 0.25.0 is the minimum supported version. @@ -65,3 +60,16 @@ export function featureSetForVersion( allowRedirects: versionAtLeast(version, "2.38.0"), }; } + +/** Capabilities of the given deployment version. */ +export function serverFeatureSet( + version: semver.SemVer | null, +): ServerFeatureSet { + return { + // `/api/v2/tasks`, stable from 2.29 until the 2.35 deprecation + tasks: + versionAtLeast(version, "2.29.0") && !versionAtLeast(version, "2.35.0"), + // `on_success` on a stop build, which queues the start in one request + onSuccessBuild: versionAtLeast(version, "2.36.0"), + }; +} diff --git a/src/instrumentation/workspace.ts b/src/instrumentation/workspace.ts index d25783e9c2..e04cf8f3a4 100644 --- a/src/instrumentation/workspace.ts +++ b/src/instrumentation/workspace.ts @@ -11,7 +11,7 @@ import type { TelemetryReporter } from "../telemetry/reporter"; import type { Span } from "../telemetry/span"; export type WorkspacePromptAction = "start" | "update"; -export type WorkspaceUpdatePrompt = "parameters" | "confirmation"; +export type WorkspaceUpdatePrompt = "parameters" | "confirmation" | "failure"; /** * Emits `workspace.state_transitioned` for a detected workspace transition. @@ -141,13 +141,27 @@ export class WorkspaceOperationTelemetry { public traceConfirmationPrompt( fn: () => Promise, ): Promise { - return this.traceUpdatePrompt("confirmation", async (span) => { + return this.tracePromptChoice("confirmation", "update", fn); + } + + /** Records whether the user connects to the existing version anyway. */ + public traceFailurePrompt(fn: () => Promise): Promise { + return this.tracePromptChoice("failure", "connect", fn); + } + + /** Emits the prompt with `action`, or marks it aborted on a falsy answer. */ + private tracePromptChoice( + prompt: WorkspaceUpdatePrompt, + action: string, + fn: () => Promise, + ): Promise { + return this.traceUpdatePrompt(prompt, async (span) => { const value = await fn(); - if (value === undefined) { + if (!value) { span.markAborted(); - return undefined; + return value; } - span.setProperty("action", "update"); + span.setProperty("action", action); return value; }); } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index ce0321c1f1..49a2662c79 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -21,7 +21,12 @@ import { } from "../configWatcher"; import { version as cliVersion } from "../core/cliExec"; import { toError } from "../error/errorUtils"; -import { featureSetForVersion, type FeatureSet } from "../featureSet"; +import { + cliFeatureSet, + serverFeatureSet, + type CliFeatureSet, + type ServerFeatureSet, +} from "../featureSet"; import { Inbox } from "../inbox"; import { AuthTelemetry } from "../instrumentation/auth"; import { @@ -304,7 +309,7 @@ export class Remote { this.resolveRemoteBinary(workspaceClient), ); - const { featureSet, cliAuth } = await tracer.phase( + const { cliFeatures, serverFeatures, cliAuth } = await tracer.phase( "compatibility_check", () => this.checkCompatibility({ @@ -317,7 +322,7 @@ export class Remote { // Reject deployments below our minimum supported version (v0.25.0) // before configuring credentials, so they get a clear message. - if (!featureSet.cliLogin) { + if (!cliFeatures.cliLogin) { tracer.markAborted("incompatible_server"); await vscodeProposed.window.showErrorMessage( "Incompatible Server", @@ -391,7 +396,8 @@ export class Remote { workspaceClient, args.startupMode, binaryPath, - featureSet, + cliFeatures, + serverFeatures, cliAuth, this.serviceContainer, ); @@ -416,7 +422,7 @@ export class Remote { const inbox = await Inbox.create(workspace, workspaceClient, this.logger); disposables.push(inbox); - const logDir = this.getLogDir(featureSet); + const logDir = this.getLogDir(cliFeatures); const computedSshProperties = await tracer.phase("ssh_config_write", () => this.writeRemoteSshConfig( @@ -424,7 +430,7 @@ export class Remote { workspaceClient, binaryPath, logDir, - featureSet, + cliFeatures, cliAuth, ), ); @@ -518,11 +524,11 @@ export class Remote { getValue: () => vscode.workspace.getConfiguration().get(setting), })), ]; - if (featureSet.proxyLogDirectory) { + if (cliFeatures.proxyLogDirectory) { settingsToWatch.push({ setting: "coder.proxyLogDirectory", title: "Proxy Log Directory", - getValue: () => this.getLogDir(featureSet), + getValue: () => this.getLogDir(cliFeatures), }); } disposables.push(this.watchSettings(settingsToWatch)); @@ -686,7 +692,7 @@ export class Remote { workspaceClient: Api, binaryPath: string, logDir: string, - featureSet: FeatureSet, + cliFeatures: CliFeatureSet, cliAuth: CliAuth, ): Promise { try { @@ -696,7 +702,7 @@ export class Remote { context.parts, binaryPath, logDir, - featureSet, + cliFeatures, cliAuth, ); } catch (error) { @@ -760,7 +766,7 @@ export class Remote { } /** - * Resolve the feature set and CLI auth, falling back to the server version + * Resolve the feature sets and CLI auth, falling back to the server version * when the CLI version can't be read. */ private async checkCompatibility(options: { @@ -768,26 +774,35 @@ export class Remote { binaryPath: string; baseUrl: string; safeHostname: string; - }): Promise<{ featureSet: FeatureSet; cliAuth: CliAuth }> { + }): Promise<{ + cliFeatures: CliFeatureSet; + serverFeatures: ServerFeatureSet; + cliAuth: CliAuth; + }> { const { workspaceClient, binaryPath, baseUrl, safeHostname } = options; const buildInfo = await workspaceClient.getBuildInfo(); + const serverVersion = semver.parse(buildInfo.version); let version: semver.SemVer | null; try { version = semver.parse(await cliVersion(binaryPath)); } catch { - version = semver.parse(buildInfo.version); + version = serverVersion; } - const featureSet = featureSetForVersion(version); + const cliFeatures = cliFeatureSet(version); const configDir = this.pathResolver.getGlobalConfigDir(safeHostname); const cliAuth = resolveCliAuth( vscode.workspace.getConfiguration(), - featureSet, + cliFeatures, baseUrl, configDir, ); - return { featureSet, cliAuth }; + return { + cliFeatures, + serverFeatures: serverFeatureSet(serverVersion), + cliAuth, + }; } private watchRemoteSessionAuth( @@ -825,8 +840,8 @@ export class Remote { * * Value defined in the "coder.sshFlags" setting is not considered. */ - private getLogDir(featureSet: FeatureSet): string { - if (!featureSet.proxyLogDirectory) { + private getLogDir(cliFeatures: CliFeatureSet): string { + if (!cliFeatures.proxyLogDirectory) { return ""; } return this.pathResolver.getProxyLogPath(); @@ -918,7 +933,7 @@ export class Remote { parts: AuthorityParts, binaryPath: string, logDir: string, - featureSet: FeatureSet, + cliFeatures: CliFeatureSet, cliAuth: CliAuth, ): Promise { // Taken from the authority, so a legacy host keeps working. @@ -981,7 +996,7 @@ export class Remote { safeHostname, hostPrefix, logDir, - featureSet.wildcardSSH, + cliFeatures.wildcardSSH, cliAuth, ); diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index fc4242d967..1ed91335dc 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -1,5 +1,3 @@ -import * as vscode from "vscode"; - import { createWorkspaceIdentifier, errToStr, @@ -27,11 +25,12 @@ import type { Workspace, WorkspaceAgentLog, } from "coder/site/src/api/typesGenerated"; +import type * as vscode from "vscode"; import type { CoderApi } from "../api/coderApi"; import type { ServiceContainer } from "../core/container"; import type { StartupMode } from "../core/mementoManager"; -import type { FeatureSet } from "../featureSet"; +import type { CliFeatureSet, ServerFeatureSet } from "../featureSet"; import type { Logger } from "../logging/logger"; import type { CliAuth } from "../settings/cli"; import type { AuthorityParts } from "../util/authority"; @@ -48,6 +47,8 @@ export class WorkspaceStateMachine implements vscode.Disposable { private agent: { id: string; name: string } | undefined; private workspace: Workspace | undefined; + /** Stop build we posted, whose start the server queues behind it. */ + private queuedStopBuild: string | undefined; private readonly logger: Logger; @@ -56,7 +57,8 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly workspaceClient: CoderApi, private startupMode: StartupMode, private readonly binaryPath: string, - private readonly featureSet: FeatureSet, + private readonly cliFeatures: CliFeatureSet, + private readonly serverFeatures: ServerFeatureSet, private readonly cliAuth: CliAuth, container: ServiceContainer, ) { @@ -78,6 +80,15 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspace: Workspace, progress: vscode.Progress<{ message?: string }>, ): Promise { + const current = this.workspace?.latest_build; + // Snapshots taken before the build we posted are stale. + if (current && workspace.latest_build.build_number < current.build_number) { + return false; + } + if (workspace.latest_build.id !== current?.id) { + // Logs stream from one build, so a new build needs a new stream. + this.buildLogStream.close(); + } this.workspace = workspace; const workspaceName = createWorkspaceIdentifier(workspace); @@ -89,18 +100,28 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspaceName, progress, ); - if (updated) { - workspace = updated; - // Agent IDs may have changed after an update. - this.resetAgent(); - if (workspace.latest_build.status !== "running") return false; - } + if (!updated) break; + this.resetAgent(); + if (updated.latest_build.status !== "running") return false; + workspace = updated; break; } case "stopped": case "failed": { this.buildLogStream.close(); + if (workspace.latest_build.id === this.queuedStopBuild) { + if (workspace.latest_build.status === "failed") { + throw new Error( + `Update failed for ${workspaceName}. Check the workspace in the dashboard before retrying.`, + ); + } + // Starting it here would race the server's queued start. + progress.report({ + message: `waiting for the server to start ${workspaceName}...`, + }); + return false; + } if (this.startupMode === "none") { const choice = await this.confirmStartOrUpdate( @@ -118,16 +139,15 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspaceName, progress, ); - if (updated) { - workspace = updated; - // Agent IDs may have changed after an update. - this.resetAgent(); - if (workspace.latest_build.status !== "running") return false; - break; + if (!updated) { + // Start only when no update was requested. + await this.triggerStart(workspace, workspaceName, progress); + return false; } - // Either we weren't in update mode, or the update failed: start. - await this.triggerStart(workspace, workspaceName, progress); - return false; + this.resetAgent(); + if (updated.latest_build.status !== "running") return false; + workspace = updated; + break; } case "pending": @@ -269,7 +289,8 @@ export class WorkspaceStateMachine implements vscode.Disposable { binPath: this.binaryPath, workspace, write: (data: string) => this.terminal.write(data), - featureSet: this.featureSet, + cliFeatures: this.cliFeatures, + serverFeatures: this.serverFeatures, }; } @@ -289,7 +310,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { this.logger.info(`${workspaceName} start initiated`); } - /** No-op if not in update mode. Falls through to start on failure. */ + /** No-op outside update mode; asks before falling back to the old version. */ private async maybeUpdate( workspace: Workspace, workspaceName: string, @@ -306,11 +327,16 @@ export class WorkspaceStateMachine implements vscode.Disposable { const parameters = await this.operationTelemetry.traceParametersPrompt( () => collectUpdateParameters(this.workspaceClient, workspace), ); - this.workspace = await this.operationTelemetry.traceUpdate(() => + const updated = await this.operationTelemetry.traceUpdate(() => updateWorkspace(this.buildCliContext(workspace), parameters), ); + this.workspace = updated; + // Only the one-build update returns a stop; the server starts it after. + if (updated.latest_build.transition === "stop") { + this.queuedStopBuild = updated.latest_build.id; + } this.logger.info(`${workspaceName} update initiated`); - return this.workspace; + return updated; } catch (error) { if (error instanceof WorkspaceUpdateCancelledError) { this.logger.info( @@ -320,13 +346,35 @@ export class WorkspaceStateMachine implements vscode.Disposable { } const reason = errToStr(error); this.logger.warn(`Update failed for ${workspaceName}: ${reason}`); - vscode.window.showWarningMessage( - `Workspace update failed: ${reason}. Continuing with the existing version.`, + const connect = await this.operationTelemetry.traceFailurePrompt(() => + this.confirmConnectToExisting(workspaceName, reason), ); + if (!connect) { + throw error; + } + this.logger.info(`Connecting to the existing ${workspaceName} version`); return undefined; } } + /** Offers the existing version after a failed update. */ + private async confirmConnectToExisting( + workspaceName: string, + reason: string, + ): Promise { + const action = "Connect Anyway"; + const choice = await vscodeProposed.window.showWarningMessage( + `Failed to update ${workspaceName}`, + { + useCustom: true, + modal: true, + detail: reason, + }, + action, + ); + return choice === action; + } + private async confirmStartOrUpdate( workspaceName: string, outdated: boolean, @@ -357,6 +405,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { return this.workspace; } + /** Clears the agent; its ID can change across builds. */ private resetAgent(): void { this.agent = undefined; } diff --git a/src/settings/cli.ts b/src/settings/cli.ts index 36f867eb8b..f8810b839f 100644 --- a/src/settings/cli.ts +++ b/src/settings/cli.ts @@ -6,7 +6,7 @@ import { getHeaderArgs } from "./headers"; import type { WorkspaceConfiguration } from "vscode"; -import type { FeatureSet } from "../featureSet"; +import type { CliFeatureSet } from "../featureSet"; /** The CLI's own store (its config directory or the keyring, shared with the terminal), or a directory private to the extension. */ export type CliAuth = { @@ -127,7 +127,7 @@ export function mayUseCliStore( /** Uses the CLI's own store when the keyring is on or the user set a config directory. */ export function resolveCliAuth( configs: Pick, - featureSet: FeatureSet, + featureSet: CliFeatureSet, url: string, configDir: string, ): CliAuth { diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index c4204cb5b9..12515d4629 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -8,11 +8,12 @@ import { workspace as createWorkspace } from "@repo/mocks"; import type { Api } from "coder/site/src/api/api"; import type { + CreateWorkspaceBuildRequest, Workspace, WorkspaceBuild, } from "coder/site/src/api/typesGenerated"; -import type { FeatureSet } from "@/featureSet"; +import type { CliFeatureSet, ServerFeatureSet } from "@/featureSet"; import type { UnidirectionalStream } from "@/websocket/eventStreamConnection"; vi.mock(import("node:child_process"), async (importOriginal) => ({ @@ -21,7 +22,7 @@ vi.mock(import("node:child_process"), async (importOriginal) => ({ })); const { spawn } = await import("node:child_process"); -const featureSet: FeatureSet = { +const CLI_FEATURES: CliFeatureSet = { cliLogin: true, proxyLogDirectory: true, wildcardSSH: true, @@ -64,7 +65,8 @@ function createUpdateCtx( workspace?: Omit, "latest_build"> & { latest_build?: Partial; }; - featureSet?: Partial; + cliFeatures?: Partial; + serverFeatures?: Partial; } = {}, ) { vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ @@ -81,6 +83,7 @@ function createUpdateCtx( }); const restClient = { getWorkspace: vi.fn().mockResolvedValue(finalWorkspace), + postWorkspaceBuild: vi.fn(), stopWorkspace: vi .fn() .mockResolvedValue({ ...workspace.latest_build, status: "stopped" }), @@ -104,7 +107,12 @@ function createUpdateCtx( binPath: "/usr/bin/coder", workspace, write: vi.fn<(data: string) => void>(), - featureSet: { ...featureSet, ...overrides.featureSet }, + cliFeatures: { ...CLI_FEATURES, ...overrides.cliFeatures }, + serverFeatures: { + tasks: false, + onSuccessBuild: false, + ...overrides.serverFeatures, + }, }; return { ctx, restClient, finalWorkspace }; } @@ -206,7 +214,7 @@ describe("updateWorkspace", () => { vi.clearAllMocks(); }); - it("runs coder update and resolves with the refreshed workspace", async () => { + it("runs coder update on servers before 2.36", async () => { const { ctx, restClient, finalWorkspace } = createUpdateCtx(); const sp = controlSpawn(); @@ -283,9 +291,56 @@ describe("updateWorkspace", () => { await expect(result).rejects.toThrow(/signal SIGTERM/); }); + interface OneBuildCase { + name: string; + status: WorkspaceBuild["status"]; + request: CreateWorkspaceBuildRequest; + } + + it.each([ + { + name: "restarts a running workspace in one build", + status: "running", + request: { + transition: "stop", + reason: "vscode_connection", + on_success: { + transition: "start", + rich_parameter_values: [{ name: "region", value: "us-east" }], + }, + }, + }, + { + name: "starts a stopped workspace on the active version", + status: "stopped", + request: { + transition: "start", + reason: "vscode_connection", + template_version_id: "version-1", + rich_parameter_values: [{ name: "region", value: "us-east" }], + }, + }, + ])("$name from 2.36", async ({ status, request }) => { + const { ctx, restClient } = createUpdateCtx({ + workspace: { latest_build: { status } }, + serverFeatures: { onSuccessBuild: true }, + }); + const accepted = { ...ctx.workspace.latest_build, id: "accepted-build" }; + restClient.getWorkspace.mockResolvedValue(ctx.workspace); + restClient.postWorkspaceBuild.mockResolvedValue(accepted); + + await expect( + updateWorkspace(ctx, [{ name: "region", value: "us-east" }]), + ).resolves.toEqual({ ...ctx.workspace, latest_build: accepted }); + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + request, + ); + }); + it("falls back to the API update path when coder update is unsupported", async () => { const { ctx, restClient, finalWorkspace } = createUpdateCtx({ - featureSet: { cliUpdate: false }, + cliFeatures: { cliUpdate: false }, }); await expect(updateWorkspace(ctx, [])).resolves.toBe(finalWorkspace); @@ -302,7 +357,7 @@ describe("updateWorkspace", () => { it("passes collected parameters when using the API fallback", async () => { const { ctx, restClient } = createUpdateCtx({ - featureSet: { cliUpdate: false }, + cliFeatures: { cliUpdate: false }, }); const parameters = [{ name: "region", value: "us-east" }]; @@ -319,7 +374,7 @@ describe("updateWorkspace", () => { it("does not stop before API fallback update when the workspace is not running", async () => { const { ctx, restClient } = createUpdateCtx({ workspace: { latest_build: { status: "stopped", transition: "stop" } }, - featureSet: { cliUpdate: false }, + cliFeatures: { cliUpdate: false }, }); await updateWorkspace(ctx, []); @@ -335,7 +390,7 @@ describe("updateWorkspace", () => { it("throws before update when the API fallback stop is cancelled", async () => { const { ctx, restClient } = createUpdateCtx({ - featureSet: { cliUpdate: false }, + cliFeatures: { cliUpdate: false }, }); restClient.waitForBuild.mockResolvedValueOnce({ ...ctx.workspace.latest_build.job, @@ -343,7 +398,7 @@ describe("updateWorkspace", () => { }); await expect(updateWorkspace(ctx, [])).rejects.toThrow( - "Workspace update cancelled during stop", + "Workspace update stop build did not succeed", ); expect(restClient.startWorkspace).not.toHaveBeenCalled(); }); @@ -386,7 +441,7 @@ describe("startWorkspace", () => { it("omits --reason when buildReason feature is unavailable", async () => { const { ctx } = createUpdateCtx({ workspace: { latest_build: { status: "stopped", transition: "stop" } }, - featureSet: { buildReason: false }, + cliFeatures: { buildReason: false }, }); const sp = controlSpawn(); diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index feef6fb630..17c6163155 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -2,7 +2,7 @@ import * as os from "node:os"; import * as semver from "semver"; import { afterEach, beforeEach, it, expect, describe, vi } from "vitest"; -import { featureSetForVersion } from "@/featureSet"; +import { cliFeatureSet } from "@/featureSet"; import { type CliAuth, getExpandedUserGlobalFlags, @@ -318,7 +318,7 @@ describe("cliConfig", () => { describe("resolveCliAuth", () => { function resolve(config: MockConfigurationProvider, version: string) { - const featureSet = featureSetForVersion(semver.parse(version)); + const featureSet = cliFeatureSet(semver.parse(version)); return resolveCliAuth(config, featureSet, URL, EXT_DIR); } diff --git a/test/unit/command/updateWorkspace.telemetry.test.ts b/test/unit/command/updateWorkspace.telemetry.test.ts index 9747d86c65..b80b8a2e52 100644 --- a/test/unit/command/updateWorkspace.telemetry.test.ts +++ b/test/unit/command/updateWorkspace.telemetry.test.ts @@ -1,42 +1,20 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; -import { Commands } from "@/commands"; -import { MementoManager } from "@/core/mementoManager"; - import { workspace } from "@repo/mocks"; import { createTelemetryHarness } from "../../mocks/telemetry"; -import { createMockLogger, InMemoryMemento } from "../../mocks/testHelpers"; +import { createTestCommands } from "../../mocks/testHelpers"; import type { CoderApi } from "@/api/coderApi"; -import type { ServiceContainer } from "@/core/container"; -import type { DeploymentManager } from "@/deployment/deploymentManager"; const UPDATE_ACTION = "Update and Restart"; function setup() { const { sink, service } = createTelemetryHarness(); - const mementoManager = new MementoManager(new InMemoryMemento()); - const logger = createMockLogger(); - const container = { - getTelemetryService: () => service, - getLogger: () => logger, - getPathResolver: () => ({}), - getMementoManager: () => mementoManager, - getSecretsManager: () => ({}), - getCliManager: () => ({}), - getLoginCoordinator: () => ({}), - getDuplicateWorkspaceIpc: () => ({}), - getSpeedtestPanelFactory: () => ({}), - getNetcheckPanelFactory: () => ({}), - getConnectionLogBuffer: () => ({ flush: () => {} }), - } as unknown as ServiceContainer; - const commands = new Commands( - container, - {} as CoderApi, - {} as DeploymentManager, - ); + const commands = createTestCommands({ + services: { getTelemetryService: service }, + }); commands.workspace = workspace({ outdated: true }); commands.remoteWorkspaceClient = {} as CoderApi; return { commands, sink }; @@ -47,38 +25,39 @@ describe("Commands.updateWorkspace", () => { vi.resetAllMocks(); }); - it("records an aborted update confirmation when the prompt is dismissed", async () => { - const { commands, sink } = setup(); - vi.mocked(vscode.window.showWarningMessage).mockResolvedValue(undefined); - - await commands.updateWorkspace(); - - expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ - properties: { - prompt: "confirmation", - result: "aborted", - }, - }); - expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); - }); - - it("records success and reloads when the update confirmation is accepted", async () => { - const { commands, sink } = setup(); - vi.mocked(vscode.window.showWarningMessage).mockResolvedValue( - UPDATE_ACTION as never, - ); - - await commands.updateWorkspace(); - - expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ - properties: { - action: "update", - prompt: "confirmation", - result: "success", - }, - }); - expect(vscode.commands.executeCommand).toHaveBeenCalledWith( - "workbench.action.reloadWindow", - ); - }); + interface ConfirmationCase { + choice: string | undefined; + result: string; + properties: Record; + } + + it.each([ + { choice: undefined, result: "aborted", properties: {} }, + { + choice: UPDATE_ACTION, + result: "success", + properties: { action: "update" }, + }, + ])( + "records $result when confirmation returns $choice", + async ({ choice, result, properties }) => { + const { commands, sink } = setup(); + vi.mocked(vscode.window.showWarningMessage).mockResolvedValue( + choice as never, + ); + + await commands.updateWorkspace(); + + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { prompt: "confirmation", result, ...properties }, + }); + if (choice) { + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "workbench.action.reloadWindow", + ); + } else { + expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); + } + }, + ); }); diff --git a/test/unit/featureSet.test.ts b/test/unit/featureSet.test.ts index 0a97b3bf77..c17cd36dff 100644 --- a/test/unit/featureSet.test.ts +++ b/test/unit/featureSet.test.ts @@ -2,21 +2,21 @@ import * as semver from "semver"; import { describe, expect, it } from "vitest"; import { - type FeatureSet, - featureSetForVersion, - tasksSupported, + type CliFeatureSet, + cliFeatureSet, + serverFeatureSet, } from "@/featureSet"; function expectFlag( - flag: keyof FeatureSet, + flag: keyof CliFeatureSet, below: string[], atOrAbove: string[], ) { for (const v of below) { - expect(featureSetForVersion(semver.parse(v))[flag]).toBeFalsy(); + expect(cliFeatureSet(semver.parse(v))[flag]).toBeFalsy(); } for (const v of atOrAbove) { - expect(featureSetForVersion(semver.parse(v))[flag]).toBeTruthy(); + expect(cliFeatureSet(semver.parse(v))[flag]).toBeTruthy(); } } @@ -77,20 +77,27 @@ describe("check version support", () => { ["v2.36.0", "v2.36.1", "v2.37.0", "v3.0.0"], ); }); - it("tasks panel only on deployments before deprecation", () => { - for (const v of ["v2.34.0", "v2.34.7+e491217", "v2.0.0"]) { - expect(tasksSupported(semver.parse(v)), v).toBe(true); + it("tasks panel from the stable API until the deprecation", () => { + for (const v of ["v2.29.0", "v2.34.0", "v2.34.7+e491217"]) { + expect(serverFeatureSet(semver.parse(v)).tasks, v).toBe(true); } - for (const v of ["v2.35.0", "v2.36.1", "v3.0.0", "v2.36.0-devel+abc123"]) { - expect(tasksSupported(semver.parse(v)), v).toBe(false); + for (const v of ["v2.0.0", "v2.28.9", "v2.35.0", "v2.30.0-devel+abc123"]) { + expect(serverFeatureSet(semver.parse(v)).tasks, v).toBe(false); + } + expect(serverFeatureSet(null).tasks).toBe(false); + }); + + it("one-build restart from 2.36", () => { + for (const v of ["v2.35.9", "v2.0.0"]) { + expect(serverFeatureSet(semver.parse(v)).onSuccessBuild, v).toBe(false); + } + for (const v of ["v2.36.0", "v2.37.1", "v0.0.0-devel+abc123"]) { + expect(serverFeatureSet(semver.parse(v)).onSuccessBuild, v).toBe(true); } - expect(tasksSupported(null)).toBe(false); }); it("enables all features for development builds", () => { - const featureSet = featureSetForVersion( - semver.parse("v0.0.0-devel+abc123"), - ); + const featureSet = cliFeatureSet(semver.parse("v0.0.0-devel+abc123")); for (const [feature, enabled] of Object.entries(featureSet)) { expect(enabled, feature).toBe(true); diff --git a/test/unit/instrumentation/workspace.test.ts b/test/unit/instrumentation/workspace.test.ts index 87fee54b86..427e1ba7d3 100644 --- a/test/unit/instrumentation/workspace.test.ts +++ b/test/unit/instrumentation/workspace.test.ts @@ -179,6 +179,30 @@ describe("WorkspaceOperationTelemetry", () => { }); }); }); + + describe("traceFailurePrompt", () => { + interface FailurePromptCase { + connect: boolean; + properties: Record; + } + + it.each([ + { connect: true, properties: { action: "connect", result: "success" } }, + { connect: false, properties: { result: "aborted" } }, + ])( + "emits $properties.result when connect is $connect", + async ({ connect, properties }) => { + const { sink, instance: ops } = setup(newOps); + + await expect( + ops.traceFailurePrompt(() => Promise.resolve(connect)), + ).resolves.toBe(connect); + expect(sink.expectOne("workspace.update.prompted")).toMatchObject({ + properties: { prompt: "failure", ...properties }, + }); + }, + ); + }); }); describe("recordWorkspaceState", () => { diff --git a/test/unit/remote/remote.test.ts b/test/unit/remote/remote.test.ts index 50ec7c46c5..010229c356 100644 --- a/test/unit/remote/remote.test.ts +++ b/test/unit/remote/remote.test.ts @@ -22,6 +22,7 @@ import { import type { Commands } from "@/commands"; import type { CliManager } from "@/core/cliManager"; import type { Logger } from "@/logging/logger"; +import type { CliAuth } from "@/settings/cli"; const mockWorkspace = vscode.workspace as typeof vscode.workspace & { workspaceFile: vscode.Uri | undefined; @@ -41,9 +42,16 @@ const REMOTE_SSH_EXTENSION_ID = "anysphere.remote-ssh"; const MISMATCHED_URL = "https://cursor.example.com/private?token=sensitive-url-token"; const SESSION_TOKEN = "sensitive-session-token"; +const CLI_AUTH: CliAuth = { + store: "extension", + url: "https://coder.example.com", + configDir: "/mock/global", + useKeyring: undefined, + allowRedirects: false, +}; function createRemote(logger: Logger = createMockLogger()) { - new MockConfigurationProvider(); + const config = new MockConfigurationProvider(); const userInteraction = new MockUserInteraction(); const pathResolver = new PathResolver("/mock/global", "/mock/log"); vol.fromJSON({ @@ -80,6 +88,7 @@ function createRemote(logger: Logger = createMockLogger()) { {} as Commands, {} as vscode.ExtensionContext, ), + config, ensureLoggedInWithDialog, mementoManager, secretsManager, @@ -226,6 +235,56 @@ describe("Remote", () => { ], }); }); + + interface RemoteInternals { + buildProxyCommand: Remote["buildProxyCommand"]; + } + + interface SshFlagsCase { + name: string; + flags?: string[]; + expected: string; + } + + /** Drops the platform-specific network info path. */ + const elideNetworkInfoDir = (command: string) => + command.replace(/--network-info-dir \S+/, "--network-info-dir "); + + describe("ProxyCommand", () => { + it.each([ + { + name: "disables autostart by default", + expected: + "ssh --disable-autostart --stdio --usage-app=vscode --network-info-dir --ssh-host-prefix coder-vscode.coder.example.com-- %h", + }, + { + name: "passes the user's flags ahead of the managed ones", + flags: ["--wait=yes"], + expected: + "ssh --wait=yes --stdio --usage-app=vscode --network-info-dir --ssh-host-prefix coder-vscode.coder.example.com-- %h", + }, + ])("$name", async ({ flags, expected }) => { + const { remote, config } = createRemote(); + if (flags) { + config.set("coder.sshFlags", flags); + } + + const proxyCommand = await ( + remote as unknown as RemoteInternals + ).buildProxyCommand( + "/mock/coder", + SAFE_HOSTNAME, + "coder-vscode.coder.example.com--", + "", + true, + CLI_AUTH, + ); + + expect(elideNetworkInfoDir(proxyCommand)).toBe( + `/mock/coder --global-config /mock/global --url https://coder.example.com ${expected}`, + ); + }); + }); }); describe("workspaceLabelSuffix", () => { diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index cc1e970ecf..f8453087ed 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -16,7 +16,6 @@ import { WorkspaceStateMachine } from "@/remote/workspaceStateMachine"; import { agent as createAgent, - resource as createResource, workspace as createWorkspace, } from "@repo/mocks"; @@ -28,6 +27,7 @@ import { import { createMockLogger, createMockServiceContainer, + MockConfigurationProvider, MockProgress, MockTerminalOutputChannel, MockUserInteraction, @@ -40,18 +40,24 @@ import type { import type { CoderApi } from "@/api/coderApi"; import type { StartupMode } from "@/core/mementoManager"; -import type { FeatureSet } from "@/featureSet"; +import type { CliFeatureSet } from "@/featureSet"; import type { TelemetryService } from "@/telemetry/service"; import type { AuthorityParts } from "@/util/authority"; vi.mock("@/api/workspace", async (importActual) => { const { LazyStream } = await importActual(); + const { MockEventStream } = await import("../../mocks/testHelpers"); + const stream = () => Promise.resolve(new MockEventStream()); return { LazyStream, - startWorkspace: vi.fn().mockResolvedValue({}), - updateWorkspace: vi.fn().mockResolvedValue({}), - streamBuildLogs: vi.fn().mockResolvedValue({}), - streamAgentLogs: vi.fn().mockResolvedValue({}), + startWorkspace: vi.fn((ctx: { workspace: Workspace }) => + Promise.resolve(ctx.workspace), + ), + updateWorkspace: vi.fn((ctx: { workspace: Workspace }) => + Promise.resolve(ctx.workspace), + ), + streamBuildLogs: vi.fn(stream), + streamAgentLogs: vi.fn(stream), }; }); @@ -59,12 +65,14 @@ vi.mock("@/api/updateParameters", async (importActual) => { const actual = await importActual(); return { ...actual, - collectUpdateParameters: vi.fn().mockResolvedValue([]), + collectUpdateParameters: vi.fn(() => Promise.resolve([])), }; }); vi.mock("@/promptUtils", () => ({ - maybeAskAgent: vi.fn(), + maybeAskAgent: vi.fn((agents: WorkspaceAgent[]) => + Promise.resolve(agents.length > 0 ? agents[0] : undefined), + ), })); vi.mock("@/remote/terminalOutputChannel", async () => { @@ -84,15 +92,16 @@ const DEFAULT_PARTS: Readonly = { // The message shown by confirmStartOrUpdate for our test workspace. const CONFIRM_MESSAGE = "The workspace testuser/test-workspace is not running. How would you like to proceed?"; +// The message shown by confirmConnectToExisting. +const UPDATE_FAILED_MESSAGE = "Failed to update testuser/test-workspace"; function runningWorkspace( agentOverrides: Partial = {}, + buildOverrides: Partial = {}, ): Workspace { return createWorkspace({ - latest_build: { - status: "running", - resources: [createResource({ agents: [createAgent(agentOverrides)] })], - }, + agents: [createAgent(agentOverrides)], + latest_build: { status: "running", ...buildOverrides }, }); } @@ -108,7 +117,8 @@ function setup( {} as CoderApi, startupMode, "/usr/bin/coder", - {} as FeatureSet, + {} as CliFeatureSet, + { tasks: false, onSuccessBuild: true }, { store: "cli", url: "https://test.coder.com", @@ -120,16 +130,36 @@ function setup( return { sm, progress, userInteraction }; } +/** A workspace at the given build number, with the given build overrides. */ +function workspaceAtBuild( + buildNumber: number, + overrides: Partial, +): Workspace { + return runningWorkspace( + {}, + { id: `build-${buildNumber}`, build_number: buildNumber, ...overrides }, + ); +} + +/** Update mode; the update resolves with build 2, a queued stop. */ +function setupUpdate() { + vi.mocked(updateWorkspace).mockResolvedValueOnce( + workspaceAtBuild(2, { status: "stopping", transition: "stop" }), + ); + const { sm, progress } = setup("update"); + return { + progress, + process: (status: Workspace["latest_build"]["status"], number: number) => + sm.processWorkspace(workspaceAtBuild(number, { status }), progress), + }; +} + describe("WorkspaceStateMachine", () => { beforeEach(() => { - vi.clearAllMocks(); + // `vi.mock` factories hold the default implementations. + vi.resetAllMocks(); + new MockConfigurationProvider(); MockTerminalOutputChannel.lastInstance = undefined; - vi.mocked(updateWorkspace).mockImplementation((ctx) => - Promise.resolve(ctx.workspace), - ); - vi.mocked(maybeAskAgent).mockImplementation((agents) => - Promise.resolve(agents.length > 0 ? agents[0] : undefined), - ); }); describe("running workspace", () => { @@ -225,19 +255,32 @@ describe("WorkspaceStateMachine", () => { expect(sm.getWorkspace()?.latest_build.status).toBe("running"); }); - it("falls back to start and warns the user when the update fails", async () => { + it("starts the existing version when the update fails and the user accepts", async () => { vi.mocked(updateWorkspace).mockRejectedValueOnce( new Error("template not found"), ); - const { sm, progress } = setup("update"); + const { sm, progress, userInteraction } = setup("update"); + userInteraction.setResponse(UPDATE_FAILED_MESSAGE, "Connect Anyway"); const ws = createWorkspace({ latest_build: { status: "stopped" } }); expect(await sm.processWorkspace(ws, progress)).toBe(false); - expect(updateWorkspace).toHaveBeenCalledOnce(); expect(startWorkspace).toHaveBeenCalledOnce(); - expect(vscode.window.showWarningMessage).toHaveBeenCalledWith( - expect.stringMatching(/Workspace update failed:.*template not found/), + expect(userInteraction.getMessageCalls()[0].options).toMatchObject({ + detail: expect.stringContaining("template not found"), + }); + }); + + it("rethrows when the update fails and the user dismisses the prompt", async () => { + vi.mocked(updateWorkspace).mockRejectedValueOnce( + new Error("template not found"), + ); + const { sm, progress } = setup("update"); + const ws = createWorkspace({ latest_build: { status: "stopped" } }); + + await expect(sm.processWorkspace(ws, progress)).rejects.toThrow( + "template not found", ); + expect(startWorkspace).not.toHaveBeenCalled(); }); it("falls back to start silently when the user cancels the update", async () => { @@ -505,4 +548,37 @@ describe("WorkspaceStateMachine", () => { expect(() => sm.dispose()).not.toThrow(); }); }); + + describe("after an accepted update", () => { + it("waits for the server to run the queued start build", async () => { + const { progress, process } = setupUpdate(); + + // Queues the update. + expect(await process("running", 1)).toBe(false); + // A stale snapshot must not connect us. + expect(await process("running", 1)).toBe(false); + expect(maybeAskAgent).not.toHaveBeenCalled(); + expect(await process("stopping", 2)).toBe(false); + // The server starts it after the stop. + expect(await process("stopped", 2)).toBe(false); + expect(progress.report).toHaveBeenCalledWith({ + message: expect.stringContaining("waiting for the server"), + }); + expect(await process("starting", 3)).toBe(false); + // One log stream per build. + expect(streamBuildLogs).toHaveBeenCalledTimes(2); + + expect(await process("running", 3)).toBe(true); + expect(startWorkspace).not.toHaveBeenCalled(); + expect(updateWorkspace).toHaveBeenCalledOnce(); + }); + + it("throws instead of starting again when a build fails", async () => { + const { process } = setupUpdate(); + await process("running", 1); + + await expect(process("failed", 2)).rejects.toThrow("Update failed"); + expect(startWorkspace).not.toHaveBeenCalled(); + }); + }); }); From 94cfc4cf1707dfc46c680bf68eec7de5c9404969 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 22 Sep 2026 20:31:34 +0300 Subject: [PATCH 2/2] refactor: drop the `as unknown as` casts from the tests Narrow the build helpers' `restClient` to the API methods they call, so the mock in the test satisfies the type on its own. Move the ProxyCommand builder out of `Remote` and split it by CLI generation, so the test calls it directly instead of reaching into a private method through a cast. Each builder now takes only the options its branch uses, and the caller branches on `wildcardSSH`. --- src/api/workspace.ts | 12 ++- src/remote/remote.ts | 131 ++++++++++++++++---------------- test/unit/api/workspace.test.ts | 3 +- test/unit/remote/remote.test.ts | 34 ++++----- 4 files changed, 91 insertions(+), 89 deletions(-) diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 9824b81423..2ee12a6820 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -51,8 +51,18 @@ export class LazyStream { } } +type BuildApi = Pick< + Api, + | "getTemplate" + | "getWorkspace" + | "postWorkspaceBuild" + | "startWorkspace" + | "stopWorkspace" + | "waitForBuild" +>; + interface CliContext { - restClient: Api; + restClient: BuildApi; auth: CliAuth; binPath: string; workspace: Workspace; diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 49a2662c79..7bdb42cfa6 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -847,67 +847,6 @@ export class Remote { return this.pathResolver.getProxyLogPath(); } - /** - * Builds the ProxyCommand for SSH connections to Coder workspaces. - * Uses `coder ssh` for modern deployments with wildcard support, - * or falls back to `coder vscodessh` for older deployments. - */ - private async buildProxyCommand( - binaryPath: string, - label: string, - hostPrefix: string, - logDir: string, - useWildcardSSH: boolean, - cliAuth: CliAuth, - ): Promise { - const vscodeConfig = vscode.workspace.getConfiguration(); - - const escapedBinaryPath = escapeCommandArg(binaryPath); - const globalConfig = getGlobalShellFlags(vscodeConfig, cliAuth); - const logArgs = await this.getLogArgs(logDir); - - if (useWildcardSSH) { - // User SSH flags are included first; internally-managed flags - // are appended last so they take precedence. - const userSshFlags = getSshFlags(vscodeConfig); - // Make sure to update the `coder.sshFlags` description if we add more internal flags here! - const internalFlags = [ - "--stdio", - "--usage-app=vscode", - "--network-info-dir", - escapeCommandArg(this.pathResolver.getNetworkInfoPath()), - ...logArgs, - "--ssh-host-prefix", - hostPrefix, - "%h", - ]; - - const allFlags = [...userSshFlags, ...internalFlags]; - return `${escapedBinaryPath} ${globalConfig.join(" ")} ssh ${allFlags.join(" ")}`; - } else { - const networkInfoDir = escapeCommandArg( - this.pathResolver.getNetworkInfoPath(), - ); - const sessionTokenFile = escapeCommandArg( - this.pathResolver.getSessionTokenPath(label), - ); - const urlFile = escapeCommandArg(this.pathResolver.getUrlPath(label)); - - const sshFlags = [ - "--network-info-dir", - networkInfoDir, - ...logArgs, - "--session-token-file", - sessionTokenFile, - "--url-file", - urlFile, - "%h", - ]; - - return `${escapedBinaryPath} ${globalConfig.join(" ")} vscodessh ${sshFlags.join(" ")}`; - } - } - /** * Returns the --log-dir argument for the ProxyCommand after making sure it * has been created. @@ -991,14 +930,15 @@ export class Remote { userConfig, ); - const proxyCommand = await this.buildProxyCommand( + const proxyOptions = { + pathResolver: this.pathResolver, binaryPath, - safeHostname, - hostPrefix, - logDir, - cliFeatures.wildcardSSH, cliAuth, - ); + logArgs: await this.getLogArgs(logDir), + }; + const proxyCommand = cliFeatures.wildcardSSH + ? buildSshProxyCommand({ ...proxyOptions, hostPrefix }) + : buildVscodeSshProxyCommand({ ...proxyOptions, label: safeHostname }); const sshValues: SshValues = { Host: hostPrefix + `*`, @@ -1148,3 +1088,60 @@ export class Remote { }); } } + +interface ProxyCommandOptions { + pathResolver: PathResolver; + binaryPath: string; + cliAuth: CliAuth; + logArgs: string[]; +} + +function coderCommand( + options: ProxyCommandOptions, + subcommand: string, + flags: string[], +): string { + const globalFlags = getGlobalShellFlags( + vscode.workspace.getConfiguration(), + options.cliAuth, + ); + return `${escapeCommandArg(options.binaryPath)} ${globalFlags.join(" ")} ${subcommand} ${flags.join(" ")}`; +} + +/** ProxyCommand for CLIs that support wildcard hosts. */ +export function buildSshProxyCommand( + options: ProxyCommandOptions & { hostPrefix: string }, +): string { + // Make sure to update the `coder.sshFlags` description if we add more internal flags here! + const internalFlags = [ + "--stdio", + "--usage-app=vscode", + "--network-info-dir", + escapeCommandArg(options.pathResolver.getNetworkInfoPath()), + ...options.logArgs, + "--ssh-host-prefix", + options.hostPrefix, + "%h", + ]; + // User SSH flags are included first; internally-managed flags + // are appended last so they take precedence. + const userSshFlags = getSshFlags(vscode.workspace.getConfiguration()); + return coderCommand(options, "ssh", [...userSshFlags, ...internalFlags]); +} + +/** ProxyCommand for CLIs that predate wildcard hosts. */ +function buildVscodeSshProxyCommand( + options: ProxyCommandOptions & { label: string }, +): string { + const { pathResolver, label } = options; + return coderCommand(options, "vscodessh", [ + "--network-info-dir", + escapeCommandArg(pathResolver.getNetworkInfoPath()), + ...options.logArgs, + "--session-token-file", + escapeCommandArg(pathResolver.getSessionTokenPath(label)), + "--url-file", + escapeCommandArg(pathResolver.getUrlPath(label)), + "%h", + ]); +} diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 12515d4629..f7c7ac0bcd 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -6,7 +6,6 @@ import { LazyStream, startWorkspace, updateWorkspace } from "@/api/workspace"; import { workspace as createWorkspace } from "@repo/mocks"; -import type { Api } from "coder/site/src/api/api"; import type { CreateWorkspaceBuildRequest, Workspace, @@ -97,7 +96,7 @@ function createUpdateCtx( }), }; const ctx = { - restClient: restClient as unknown as Api, + restClient, auth: { store: "cli" as const, url: "https://test.coder.com", diff --git a/test/unit/remote/remote.test.ts b/test/unit/remote/remote.test.ts index 010229c356..8139c57266 100644 --- a/test/unit/remote/remote.test.ts +++ b/test/unit/remote/remote.test.ts @@ -5,7 +5,11 @@ import * as vscode from "vscode"; import { MementoManager } from "@/core/mementoManager"; import { PathResolver } from "@/core/pathResolver"; import { SecretsManager } from "@/core/secretsManager"; -import { Remote, workspaceLabelSuffix } from "@/remote/remote"; +import { + buildSshProxyCommand, + Remote, + workspaceLabelSuffix, +} from "@/remote/remote"; import { createTestTelemetryService } from "../../mocks/telemetry"; import { @@ -51,7 +55,7 @@ const CLI_AUTH: CliAuth = { }; function createRemote(logger: Logger = createMockLogger()) { - const config = new MockConfigurationProvider(); + new MockConfigurationProvider(); const userInteraction = new MockUserInteraction(); const pathResolver = new PathResolver("/mock/global", "/mock/log"); vol.fromJSON({ @@ -88,7 +92,6 @@ function createRemote(logger: Logger = createMockLogger()) { {} as Commands, {} as vscode.ExtensionContext, ), - config, ensureLoggedInWithDialog, mementoManager, secretsManager, @@ -236,10 +239,6 @@ describe("Remote", () => { }); }); - interface RemoteInternals { - buildProxyCommand: Remote["buildProxyCommand"]; - } - interface SshFlagsCase { name: string; flags?: string[]; @@ -263,22 +262,19 @@ describe("Remote", () => { expected: "ssh --wait=yes --stdio --usage-app=vscode --network-info-dir --ssh-host-prefix coder-vscode.coder.example.com-- %h", }, - ])("$name", async ({ flags, expected }) => { - const { remote, config } = createRemote(); + ])("$name", ({ flags, expected }) => { + const config = new MockConfigurationProvider(); if (flags) { config.set("coder.sshFlags", flags); } - const proxyCommand = await ( - remote as unknown as RemoteInternals - ).buildProxyCommand( - "/mock/coder", - SAFE_HOSTNAME, - "coder-vscode.coder.example.com--", - "", - true, - CLI_AUTH, - ); + const proxyCommand = buildSshProxyCommand({ + pathResolver: new PathResolver("/mock/global", "/mock/log"), + binaryPath: "/mock/coder", + cliAuth: CLI_AUTH, + logArgs: [], + hostPrefix: "coder-vscode.coder.example.com--", + }); expect(elideNetworkInfoDir(proxyCommand)).toBe( `/mock/coder --global-config /mock/global --url https://coder.example.com ${expected}`,