From 3248c5df37e306b04837affea03f71a35a557706 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 7 May 2026 18:41:20 +0000 Subject: [PATCH 1/3] fix: update workspaces via API defaults --- src/api/workspace.ts | 172 +++++++- src/remote/remote.ts | 2 +- src/remote/workspaceStateMachine.ts | 30 +- test/unit/api/workspace.test.ts | 387 +++++++++++++++++- .../unit/remote/workspaceStateMachine.test.ts | 13 + 5 files changed, 586 insertions(+), 18 deletions(-) diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 3c23e28f..5f15458f 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -1,8 +1,11 @@ import { type Api } from "coder/site/src/api/api"; import { - type WorkspaceAgentLog, + type CreateWorkspaceBuildRequest, type ProvisionerJobLog, + type TemplateVersionParameter, type Workspace, + type WorkspaceAgentLog, + type WorkspaceBuildParameter, } from "coder/site/src/api/typesGenerated"; import { spawn } from "node:child_process"; import * as vscode from "vscode"; @@ -113,21 +116,40 @@ export async function startWorkspace(ctx: CliContext): Promise { } /** - * Update a workspace to the latest template version. + * Update a workspace to the latest template version using the API. * - * Uses `coder update` when the CLI supports it (>= 2.24). - * Falls back to the REST API: stop, wait, then updateWorkspaceVersion. + * Parameter prompts cannot be answered from the read-only output channel, so + * update builds are created with existing mutable values where valid and + * template defaults for values that would otherwise require prompting. */ export async function updateWorkspace(ctx: CliContext): Promise { - if (ctx.featureSet.cliUpdate) { - await runCliCommand(ctx, ["update"]); - return ctx.restClient.getWorkspace(ctx.workspace.id); + const workspace = await ctx.restClient.getWorkspace(ctx.workspace.id); + if (!workspace.outdated) { + ctx.write("Workspace is up-to-date.\r\n"); + return workspace; } - // REST API fallback for older CLIs. - if (ctx.workspace.latest_build.status === "running") { + const targetVersionId = workspace.template_active_version_id; + const oldBuildParameters = await ctx.restClient.getWorkspaceBuildParameters( + workspace.latest_build.id, + ); + const templateParameters = await getUpdateTemplateParameters( + ctx, + workspace, + oldBuildParameters, + targetVersionId, + ); + const buildParameters = resolveUpdateParametersWithDefaults( + oldBuildParameters, + templateParameters, + ); + + if (workspace.latest_build.transition === "start") { ctx.write("Stopping workspace for update...\r\n"); - const stopBuild = await ctx.restClient.stopWorkspace(ctx.workspace.id); + const stopBuild = await ctx.restClient.postWorkspaceBuild( + workspace.id, + withBuildReason(ctx, { transition: "stop" }), + ); const stoppedJob = await ctx.restClient.waitForBuild(stopBuild); if (stoppedJob?.status === "canceled") { throw new Error("Workspace update canceled during stop"); @@ -135,8 +157,134 @@ export async function updateWorkspace(ctx: CliContext): Promise { } ctx.write("Starting workspace with updated template...\r\n"); - await ctx.restClient.updateWorkspaceVersion(ctx.workspace); - return ctx.restClient.getWorkspace(ctx.workspace.id); + const startBuild = await ctx.restClient.postWorkspaceBuild( + workspace.id, + withBuildReason(ctx, { + transition: "start", + template_version_id: targetVersionId, + rich_parameter_values: buildParameters, + }), + ); + const startedJob = await ctx.restClient.waitForBuild(startBuild); + if (startedJob?.status === "canceled") { + throw new Error("Workspace update canceled during start"); + } + + return ctx.restClient.getWorkspace(workspace.id); +} + +async function getUpdateTemplateParameters( + ctx: CliContext, + workspace: Workspace, + oldBuildParameters: WorkspaceBuildParameter[], + targetVersionId: string, +): Promise { + if (workspace.template_use_classic_parameter_flow) { + return ctx.restClient.getTemplateVersionRichParameters(targetVersionId); + } + + return ctx.restClient.getDynamicParameters( + targetVersionId, + workspace.owner_id, + oldBuildParameters, + ); +} + +function withBuildReason( + ctx: CliContext, + request: CreateWorkspaceBuildRequest, +): CreateWorkspaceBuildRequest { + if (!ctx.featureSet.buildReason) { + return request; + } + return { ...request, reason: "vscode_connection" }; +} + +function resolveUpdateParametersWithDefaults( + oldBuildParameters: WorkspaceBuildParameter[], + templateParameters: TemplateVersionParameter[], +): WorkspaceBuildParameter[] { + const oldBuildParametersByName = new Map( + oldBuildParameters.map((parameter) => [parameter.name, parameter]), + ); + const resolvedParameters: WorkspaceBuildParameter[] = []; + + for (const parameter of templateParameters) { + if (parameter.ephemeral) { + continue; + } + + const oldParameter = oldBuildParametersByName.get(parameter.name); + const oldParameterIsValid = oldParameter + ? isValidParameterValue(oldParameter.value, parameter) + : false; + + if (oldParameter && parameter.mutable && oldParameterIsValid) { + resolvedParameters.push(oldParameter); + continue; + } + + if (!oldParameter || !oldParameterIsValid || parameter.mutable) { + if (!hasUsableDefault(parameter)) { + throw new Error(missingDefaultMessage(parameter)); + } + resolvedParameters.push({ + name: parameter.name, + value: parameter.default_value, + }); + } + } + + return resolvedParameters; +} + +function hasUsableDefault(parameter: TemplateVersionParameter): boolean { + return !parameter.required; +} + +function missingDefaultMessage(parameter: TemplateVersionParameter): string { + const name = parameter.display_name + ? `${parameter.display_name} (${parameter.name})` + : parameter.name; + return `Workspace update requires a value for parameter "${name}", but no default is available. Open Coder in your browser to set this parameter, then try updating again.`; +} + +function isValidParameterValue( + value: string, + parameter: TemplateVersionParameter, +): boolean { + if (parameter.options.length === 0) { + return true; + } + + const allowedValues = new Set( + parameter.options.map((option) => option.value), + ); + if (parameter.type === "list(string)") { + return isValidListParameterValue(value, allowedValues); + } + + return allowedValues.has(value); +} + +function isValidListParameterValue( + value: string, + allowedValues: ReadonlySet, +): boolean { + let values: unknown; + try { + values = JSON.parse(value); + } catch { + return false; + } + + return ( + Array.isArray(values) && + values.every( + (parameterValue) => + typeof parameterValue === "string" && allowedValues.has(parameterValue), + ) + ); } /** diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 0f8714d5..898191f5 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -406,7 +406,7 @@ export class Remote { ); if (isReady) { subscription.dispose(); - resolve(w); + resolve(stateMachine.getWorkspace() ?? w); return; } } catch (error: unknown) { diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index b533ffb8..24653981 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -35,6 +35,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly agentLogStream = new LazyStream(); private agent: { id: string; name: string } | undefined; + private workspace: Workspace | undefined; constructor( private readonly parts: AuthorityParts, @@ -56,13 +57,19 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspace: Workspace, progress: vscode.Progress<{ message?: string }>, ): Promise { + this.workspace = workspace; const workspaceName = createWorkspaceIdentifier(workspace); switch (workspace.latest_build.status) { case "running": this.buildLogStream.close(); if (this.startupMode === "update") { - await this.triggerUpdate(workspace, workspaceName, progress); + workspace = await this.triggerUpdate( + workspace, + workspaceName, + progress, + ); + this.workspace = workspace; // Agent IDs may have changed after an update. this.agent = undefined; } @@ -84,7 +91,15 @@ export class WorkspaceStateMachine implements vscode.Disposable { } if (this.startupMode === "update") { - await this.triggerUpdate(workspace, workspaceName, progress); + workspace = await this.triggerUpdate( + workspace, + workspaceName, + progress, + ); + this.workspace = workspace; + if (workspace.latest_build.status === "running") { + break; + } } else { await this.triggerStart(workspace, workspaceName, progress); } @@ -252,16 +267,19 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspace: Workspace, workspaceName: string, progress: vscode.Progress<{ message?: string }>, - ): Promise { + ): Promise { progress.report({ message: `updating ${workspaceName}...` }); this.logger.info(`Updating ${workspaceName}`, { mode: this.startupMode, status: workspace.latest_build.status, }); - await updateWorkspace(this.buildCliContext(workspace)); + const updatedWorkspace = await updateWorkspace( + this.buildCliContext(workspace), + ); // Downgrade so subsequent transitions don't re-trigger the update. this.startupMode = "start"; this.logger.info(`${workspaceName} update initiated`); + return updatedWorkspace; } private async confirmStartOrUpdate( @@ -286,6 +304,10 @@ export class WorkspaceStateMachine implements vscode.Disposable { return this.agent?.id; } + public getWorkspace(): Workspace | undefined { + return this.workspace; + } + dispose(): void { this.buildLogStream.close(); this.agentLogStream.close(); diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 182ee390..be8d85c8 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -1,8 +1,54 @@ import { describe, expect, it, vi } from "vitest"; -import { LazyStream } from "@/api/workspace"; +import { LazyStream, updateWorkspace } from "@/api/workspace"; +import { type FeatureSet } from "@/featureSet"; import { type UnidirectionalStream } from "@/websocket/eventStreamConnection"; +import { workspace as createWorkspace } from "../../mocks/workspace"; + +import type { Api } from "coder/site/src/api/api"; +import type { + CreateWorkspaceBuildRequest, + ProvisionerJob, + TemplateVersionParameter, + TemplateVersionParameterOption, + Workspace, + WorkspaceBuild, + WorkspaceBuildParameter, +} from "coder/site/src/api/typesGenerated"; + +type UpdateWorkspaceContext = Parameters[0]; +interface UpdateRestClient { + getWorkspace: (workspaceId: string) => Promise; + getWorkspaceBuildParameters: ( + workspaceBuildId: string, + ) => Promise; + getTemplateVersionRichParameters: ( + versionId: string, + ) => Promise; + getDynamicParameters: ( + templateVersionId: string, + ownerId: string, + oldBuildParameters: WorkspaceBuildParameter[], + ) => Promise; + postWorkspaceBuild: ( + workspaceId: string, + data: CreateWorkspaceBuildRequest, + ) => Promise; + waitForBuild: (build: WorkspaceBuild) => Promise; +} + +const featureSet: FeatureSet = { + vscodessh: true, + proxyLogDirectory: true, + wildcardSSH: true, + buildReason: true, + cliUpdate: true, + keyringAuth: true, + keyringTokenRead: true, + supportBundle: true, +}; + function mockStream(): UnidirectionalStream { return { url: "ws://test", @@ -28,6 +74,132 @@ function deferredFactory() { }; } +function templateParameter( + overrides: Partial = {}, +): TemplateVersionParameter { + return { + name: "parameter", + description: "", + description_plaintext: "", + type: "string", + form_type: "", + mutable: true, + default_value: "default", + icon: "", + options: [], + required: false, + ephemeral: false, + ...overrides, + }; +} + +function parameterOption( + value: string, + overrides: Partial = {}, +): TemplateVersionParameterOption { + return { + name: value, + description: "", + value, + icon: "", + ...overrides, + }; +} + +function createBuild( + workspace: Workspace, + overrides: Partial = {}, +): WorkspaceBuild { + return { + ...workspace.latest_build, + ...overrides, + }; +} + +function succeededJob(workspace: Workspace): ProvisionerJob { + return { ...workspace.latest_build.job, status: "succeeded" }; +} + +function setupUpdateWorkspace({ + workspace = createWorkspace({ + outdated: true, + template_use_classic_parameter_flow: true, + latest_build: { status: "stopped", transition: "stop" }, + }), + finalWorkspace = createWorkspace({ + outdated: false, + template_use_classic_parameter_flow: true, + latest_build: { status: "running" }, + }), + oldBuildParameters = [], + templateParameters = [], + featureSetOverrides = {}, +}: { + workspace?: Workspace; + finalWorkspace?: Workspace; + oldBuildParameters?: WorkspaceBuildParameter[]; + templateParameters?: TemplateVersionParameter[]; + featureSetOverrides?: Partial; +} = {}) { + const stopBuild = createBuild(workspace, { + id: "stop-build", + build_number: 2, + transition: "stop", + status: "stopped", + }); + const startBuild = createBuild(workspace, { + id: "start-build", + build_number: 3, + transition: "start", + status: "running", + }); + const postWorkspaceBuild = vi + .fn< + ( + workspaceId: string, + data: CreateWorkspaceBuildRequest, + ) => Promise + >() + .mockResolvedValueOnce(stopBuild) + .mockResolvedValue(startBuild); + const waitForBuild = vi + .fn<(build: WorkspaceBuild) => Promise>() + .mockResolvedValue(succeededJob(workspace)); + const restClient: UpdateRestClient = { + getWorkspace: vi + .fn<(workspaceId: string) => Promise>() + .mockResolvedValueOnce(workspace) + .mockResolvedValue(finalWorkspace), + getWorkspaceBuildParameters: vi + .fn<(workspaceBuildId: string) => Promise>() + .mockResolvedValue(oldBuildParameters), + getTemplateVersionRichParameters: vi + .fn<(versionId: string) => Promise>() + .mockResolvedValue(templateParameters), + getDynamicParameters: vi + .fn< + ( + templateVersionId: string, + ownerId: string, + oldBuildParameters: WorkspaceBuildParameter[], + ) => Promise + >() + .mockResolvedValue(templateParameters), + postWorkspaceBuild, + waitForBuild, + }; + const write = vi.fn<(data: string) => void>(); + const ctx: UpdateWorkspaceContext = { + restClient: restClient as Api, + auth: { mode: "url", url: "https://test.coder.com" }, + binPath: "/usr/bin/coder", + workspace, + write, + featureSet: { ...featureSet, ...featureSetOverrides }, + }; + return { ctx, restClient, startBuild, stopBuild, write }; +} + describe("LazyStream", () => { it("opens once and ignores subsequent calls", async () => { const factory: StreamFactory = vi.fn().mockResolvedValue(mockStream()); @@ -86,3 +258,216 @@ describe("LazyStream", () => { expect(factory2).toHaveBeenCalledOnce(); }); }); + +describe("updateWorkspace", () => { + it("returns the fresh workspace without building when already up to date", async () => { + const workspace = createWorkspace({ outdated: false }); + const { ctx, restClient, write } = setupUpdateWorkspace({ workspace }); + + await expect(updateWorkspace(ctx)).resolves.toBe(workspace); + + expect(write).toHaveBeenCalledWith("Workspace is up-to-date.\r\n"); + expect(restClient.getWorkspaceBuildParameters).not.toHaveBeenCalled(); + expect(restClient.postWorkspaceBuild).not.toHaveBeenCalled(); + expect(restClient.waitForBuild).not.toHaveBeenCalled(); + }); + + it("stops a started workspace before starting the update build", async () => { + const workspace = createWorkspace({ + outdated: true, + template_active_version_id: "active-version", + template_use_classic_parameter_flow: true, + latest_build: { status: "running", transition: "start" }, + }); + const finalWorkspace = createWorkspace({ + outdated: false, + template_active_version_id: "active-version", + latest_build: { + status: "running", + template_version_id: "active-version", + }, + }); + const oldBuildParameters = [{ name: "region", value: "us" }]; + const templateParameters = [ + templateParameter({ name: "region", default_value: "eu" }), + ]; + const { ctx, restClient, startBuild, stopBuild } = setupUpdateWorkspace({ + workspace, + finalWorkspace, + oldBuildParameters, + templateParameters, + }); + + await expect(updateWorkspace(ctx)).resolves.toBe(finalWorkspace); + + expect(restClient.getWorkspace).toHaveBeenNthCalledWith(1, workspace.id); + expect(restClient.getWorkspaceBuildParameters).toHaveBeenCalledWith( + workspace.latest_build.id, + ); + expect(restClient.getTemplateVersionRichParameters).toHaveBeenCalledWith( + "active-version", + ); + expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( + 1, + workspace.id, + { transition: "stop", reason: "vscode_connection" }, + ); + expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( + 2, + workspace.id, + { + transition: "start", + template_version_id: "active-version", + rich_parameter_values: oldBuildParameters, + reason: "vscode_connection", + }, + ); + expect(restClient.waitForBuild).toHaveBeenNthCalledWith(1, stopBuild); + expect(restClient.waitForBuild).toHaveBeenNthCalledWith(2, startBuild); + }); + + it("evaluates dynamic parameters when the template uses dynamic parameter flow", async () => { + const workspace = createWorkspace({ + outdated: true, + template_active_version_id: "active-version", + template_use_classic_parameter_flow: false, + latest_build: { status: "stopped", transition: "stop" }, + }); + const oldBuildParameters = [{ name: "region", value: "us" }]; + const { ctx, restClient } = setupUpdateWorkspace({ + workspace, + oldBuildParameters, + templateParameters: [templateParameter({ name: "region" })], + }); + + await updateWorkspace(ctx); + + expect(restClient.getDynamicParameters).toHaveBeenCalledWith( + "active-version", + workspace.owner_id, + oldBuildParameters, + ); + expect(restClient.getTemplateVersionRichParameters).not.toHaveBeenCalled(); + }); + + it("uses template defaults for missing optional parameters", async () => { + const { ctx, restClient } = setupUpdateWorkspace({ + templateParameters: [ + templateParameter({ name: "editor", default_value: "vim" }), + ], + }); + + await updateWorkspace(ctx); + + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + expect.objectContaining({ + rich_parameter_values: [{ name: "editor", value: "vim" }], + }), + ); + }); + + it("falls back to the template default when an old option value is invalid", async () => { + const { ctx, restClient } = setupUpdateWorkspace({ + oldBuildParameters: [{ name: "color", value: "blue" }], + templateParameters: [ + templateParameter({ + name: "color", + default_value: "red", + options: [parameterOption("red"), parameterOption("green")], + }), + ], + }); + + await updateWorkspace(ctx); + + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + expect.objectContaining({ + rich_parameter_values: [{ name: "color", value: "red" }], + }), + ); + }); + + it("preserves valid multi-select values", async () => { + const oldBuildParameters = [ + { name: "tools", value: JSON.stringify(["vim", "emacs"]) }, + ]; + const { ctx, restClient } = setupUpdateWorkspace({ + oldBuildParameters, + templateParameters: [ + templateParameter({ + name: "tools", + type: "list(string)", + default_value: JSON.stringify(["vim"]), + options: [parameterOption("vim"), parameterOption("emacs")], + }), + ], + }); + + await updateWorkspace(ctx); + + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + expect.objectContaining({ rich_parameter_values: oldBuildParameters }), + ); + }); + + it("omits ephemeral and previously-set immutable parameters", async () => { + const { ctx, restClient } = setupUpdateWorkspace({ + oldBuildParameters: [ + { name: "token", value: "secret" }, + { name: "size", value: "large" }, + ], + templateParameters: [ + templateParameter({ name: "token", ephemeral: true }), + templateParameter({ + name: "size", + mutable: false, + default_value: "small", + }), + ], + }); + + await updateWorkspace(ctx); + + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + expect.objectContaining({ rich_parameter_values: [] }), + ); + }); + + it("throws before stopping when a required parameter has no default", async () => { + const workspace = createWorkspace({ + outdated: true, + template_use_classic_parameter_flow: true, + latest_build: { status: "running", transition: "start" }, + }); + const { ctx, restClient } = setupUpdateWorkspace({ + workspace, + templateParameters: [ + templateParameter({ + name: "project", + required: true, + default_value: "", + }), + ], + }); + + await expect(updateWorkspace(ctx)).rejects.toThrow('parameter "project"'); + expect(restClient.postWorkspaceBuild).not.toHaveBeenCalled(); + expect(restClient.waitForBuild).not.toHaveBeenCalled(); + }); + + it("omits build reasons when unsupported by the server", async () => { + const { ctx, restClient } = setupUpdateWorkspace({ + templateParameters: [templateParameter({ name: "region" })], + featureSetOverrides: { buildReason: false }, + }); + + await updateWorkspace(ctx); + + const request = vi.mocked(restClient.postWorkspaceBuild).mock.calls[0][1]; + expect(request).not.toHaveProperty("reason"); + }); +}); diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index 24840124..8b7b390b 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -93,6 +93,9 @@ describe("WorkspaceStateMachine", () => { beforeEach(() => { vi.clearAllMocks(); 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), ); @@ -181,6 +184,16 @@ describe("WorkspaceStateMachine", () => { expect(updateWorkspace).toHaveBeenCalledOnce(); }); + it("falls through to the agent check after an update completes", async () => { + vi.mocked(updateWorkspace).mockResolvedValueOnce(runningWorkspace()); + const { sm, progress } = setup("update"); + const ws = createWorkspace({ latest_build: { status: "stopped" } }); + + expect(await sm.processWorkspace(ws, progress)).toBe(true); + expect(updateWorkspace).toHaveBeenCalledOnce(); + expect(sm.getWorkspace()?.latest_build.status).toBe("running"); + }); + it("prompts user when mode is 'none' and user picks 'Start'", async () => { const { sm, progress, userInteraction } = setup("none"); userInteraction.setResponse(CONFIRM_MESSAGE, "Start"); From 1ced05a4a47115714baf20b439e58c67507d8abe Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 7 May 2026 19:04:56 +0000 Subject: [PATCH 2/3] fix: prompt for workspace update parameters --- src/api/workspace.ts | 279 +++++++++++++++++++++++++------- test/unit/api/workspace.test.ts | 114 +++++++++++-- 2 files changed, 321 insertions(+), 72 deletions(-) diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 5f15458f..5637a346 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -118,9 +118,8 @@ export async function startWorkspace(ctx: CliContext): Promise { /** * Update a workspace to the latest template version using the API. * - * Parameter prompts cannot be answered from the read-only output channel, so - * update builds are created with existing mutable values where valid and - * template defaults for values that would otherwise require prompting. + * Parameter values that need user input are collected with VS Code prompts + * before the workspace is stopped. */ export async function updateWorkspace(ctx: CliContext): Promise { const workspace = await ctx.restClient.getWorkspace(ctx.workspace.id); @@ -139,7 +138,7 @@ export async function updateWorkspace(ctx: CliContext): Promise { oldBuildParameters, targetVersionId, ); - const buildParameters = resolveUpdateParametersWithDefaults( + const buildParameters = await promptForUpdateParameters( oldBuildParameters, templateParameters, ); @@ -200,93 +199,261 @@ function withBuildReason( return { ...request, reason: "vscode_connection" }; } -function resolveUpdateParametersWithDefaults( +async function promptForUpdateParameters( oldBuildParameters: WorkspaceBuildParameter[], templateParameters: TemplateVersionParameter[], -): WorkspaceBuildParameter[] { - const oldBuildParametersByName = new Map( - oldBuildParameters.map((parameter) => [parameter.name, parameter]), +): Promise { + const missingParameters = getMissingParameters( + oldBuildParameters, + [], + templateParameters, + ); + const buildParameters: WorkspaceBuildParameter[] = []; + + for (const parameter of missingParameters) { + const value = await promptForParameter(parameter); + if (value === undefined) { + throw new Error("Workspace update canceled while configuring parameters"); + } + buildParameters.push({ name: parameter.name, value }); + } + + return buildParameters; +} + +function getMissingParameters( + oldBuildParameters: WorkspaceBuildParameter[], + buildParameters: WorkspaceBuildParameter[], + templateParameters: TemplateVersionParameter[], +): TemplateVersionParameter[] { + const missingParameters: TemplateVersionParameter[] = []; + const requiredParameters = templateParameters.filter( + (parameter) => + (parameter.mutable && parameter.required) || !parameter.mutable, ); - const resolvedParameters: WorkspaceBuildParameter[] = []; + + for (const parameter of requiredParameters) { + const buildParameter = findBuildParameter( + parameter, + oldBuildParameters, + buildParameters, + ); + if (!buildParameter) { + missingParameters.push(parameter); + } + } for (const parameter of templateParameters) { - if (parameter.ephemeral) { + if ( + parameter.options.length === 0 || + parameter.form_type === "multi-select" + ) { continue; } - const oldParameter = oldBuildParametersByName.get(parameter.name); - const oldParameterIsValid = oldParameter - ? isValidParameterValue(oldParameter.value, parameter) - : false; - - if (oldParameter && parameter.mutable && oldParameterIsValid) { - resolvedParameters.push(oldParameter); + const buildParameter = findBuildParameter( + parameter, + oldBuildParameters, + buildParameters, + ); + if (!buildParameter) { continue; } - if (!oldParameter || !oldParameterIsValid || parameter.mutable) { - if (!hasUsableDefault(parameter)) { - throw new Error(missingDefaultMessage(parameter)); - } - resolvedParameters.push({ - name: parameter.name, - value: parameter.default_value, - }); + const matchingOption = parameter.options.find( + (option) => option.value === buildParameter.value, + ); + if (!matchingOption && !missingParameters.includes(parameter)) { + missingParameters.push(parameter); } } - return resolvedParameters; + return missingParameters; } -function hasUsableDefault(parameter: TemplateVersionParameter): boolean { - return !parameter.required; +function findBuildParameter( + parameter: TemplateVersionParameter, + oldBuildParameters: WorkspaceBuildParameter[], + buildParameters: WorkspaceBuildParameter[], +): WorkspaceBuildParameter | undefined { + return ( + buildParameters.find((p) => p.name === parameter.name) ?? + oldBuildParameters.find((p) => p.name === parameter.name) + ); } -function missingDefaultMessage(parameter: TemplateVersionParameter): string { - const name = parameter.display_name - ? `${parameter.display_name} (${parameter.name})` - : parameter.name; - return `Workspace update requires a value for parameter "${name}", but no default is available. Open Coder in your browser to set this parameter, then try updating again.`; +async function promptForParameter( + parameter: TemplateVersionParameter, +): Promise { + if (parameter.options.length > 0) { + if ( + parameter.form_type === "multi-select" || + parameter.type === "list(string)" + ) { + return promptForMultiSelectParameter(parameter); + } + return promptForSelectParameter(parameter); + } + + if (parameter.type === "bool" || parameter.form_type === "checkbox") { + return promptForBooleanParameter(parameter); + } + + return promptForTextParameter(parameter); } -function isValidParameterValue( - value: string, +type ParameterQuickPickItem = vscode.QuickPickItem & { + value: string; +}; + +async function promptForSelectParameter( parameter: TemplateVersionParameter, -): boolean { - if (parameter.options.length === 0) { - return true; - } +): Promise { + const items = parameter.options.map((option): ParameterQuickPickItem => { + const details = [option.description]; + if (option.value === parameter.default_value) { + details.push("Default"); + } + return { + label: option.name || option.value, + description: option.value, + detail: details.filter(Boolean).join(" • "), + value: option.value, + }; + }); + const choice = await vscode.window.showQuickPick(items, { + title: parameterTitle(parameter), + placeHolder: parameterPlaceHolder(parameter), + ignoreFocusOut: true, + }); + return choice?.value; +} - const allowedValues = new Set( - parameter.options.map((option) => option.value), +async function promptForMultiSelectParameter( + parameter: TemplateVersionParameter, +): Promise { + const defaultValues = parseListParameterValue(parameter.default_value); + const items = parameter.options.map( + (option): ParameterQuickPickItem => ({ + label: option.name || option.value, + description: option.value, + detail: option.description, + picked: defaultValues.includes(option.value), + value: option.value, + }), ); - if (parameter.type === "list(string)") { - return isValidListParameterValue(value, allowedValues); - } + const choices = await vscode.window.showQuickPick(items, { + title: parameterTitle(parameter), + placeHolder: parameterPlaceHolder(parameter), + canPickMany: true, + ignoreFocusOut: true, + }); + return choices + ? JSON.stringify(choices.map((choice) => choice.value)) + : undefined; +} - return allowedValues.has(value); +async function promptForBooleanParameter( + parameter: TemplateVersionParameter, +): Promise { + const items: ParameterQuickPickItem[] = [ + { label: "Yes", value: "true" }, + { label: "No", value: "false" }, + ].map((item) => ({ + ...item, + detail: item.value === parameter.default_value ? "Default" : undefined, + })); + const choice = await vscode.window.showQuickPick(items, { + title: parameterTitle(parameter), + placeHolder: parameterPlaceHolder(parameter), + ignoreFocusOut: true, + }); + return choice?.value; } -function isValidListParameterValue( +async function promptForTextParameter( + parameter: TemplateVersionParameter, +): Promise { + return vscode.window.showInputBox({ + title: parameterTitle(parameter), + prompt: parameterPlaceHolder(parameter), + value: parameter.default_value, + password: parameter.form_type === "password", + ignoreFocusOut: true, + validateInput: (value) => validateParameterInput(parameter, value), + }); +} + +function validateParameterInput( + parameter: TemplateVersionParameter, value: string, - allowedValues: ReadonlySet, -): boolean { - let values: unknown; +): string | undefined { + if (parameter.required && value === "") { + return "A value is required."; + } + + if (parameter.type === "number") { + const numberValue = Number(value); + if (!Number.isFinite(numberValue)) { + return "Enter a number."; + } + if ( + parameter.validation_min !== undefined && + numberValue < parameter.validation_min + ) { + return `Enter a number greater than or equal to ${parameter.validation_min}.`; + } + if ( + parameter.validation_max !== undefined && + numberValue > parameter.validation_max + ) { + return `Enter a number less than or equal to ${parameter.validation_max}.`; + } + } + + if (parameter.type === "list(string)" && parameter.options.length === 0) { + const values = parseListParameterValue(value); + if (values.length === 0 && value !== "[]") { + return "Enter a JSON array of strings."; + } + } + + if (parameter.validation_regex) { + const regex = new RegExp(parameter.validation_regex); + if (!regex.test(value)) { + return parameter.validation_error || "Enter a valid value."; + } + } + + return undefined; +} + +function parseListParameterValue(value: string): string[] { try { - values = JSON.parse(value); + const parsed: unknown = JSON.parse(value); + return Array.isArray(parsed) && + parsed.every((item): item is string => typeof item === "string") + ? parsed + : []; } catch { - return false; + return []; } +} +function parameterTitle(parameter: TemplateVersionParameter): string { + return `Workspace parameter: ${parameterDisplayName(parameter)}`; +} + +function parameterPlaceHolder(parameter: TemplateVersionParameter): string { return ( - Array.isArray(values) && - values.every( - (parameterValue) => - typeof parameterValue === "string" && allowedValues.has(parameterValue), - ) + parameter.description_plaintext || parameter.description || parameter.name ); } +function parameterDisplayName(parameter: TemplateVersionParameter): string { + return parameter.display_name || parameter.name; +} + /** * Streams build logs in real-time via a callback. * Returns the websocket for lifecycle management. diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index be8d85c8..f7eea842 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; import { LazyStream, updateWorkspace } from "@/api/workspace"; import { type FeatureSet } from "@/featureSet"; @@ -106,6 +107,13 @@ function parameterOption( }; } +function chooseQuickPickValue(value: string): void { + vi.mocked(vscode.window.showQuickPick).mockResolvedValueOnce({ + label: value, + value, + } as never); +} + function createBuild( workspace: Workspace, overrides: Partial = {}, @@ -141,6 +149,9 @@ function setupUpdateWorkspace({ templateParameters?: TemplateVersionParameter[]; featureSetOverrides?: Partial; } = {}) { + vi.mocked(vscode.window.showInputBox).mockReset(); + vi.mocked(vscode.window.showQuickPick).mockReset(); + const stopBuild = createBuild(workspace, { id: "stop-build", build_number: 2, @@ -160,8 +171,9 @@ function setupUpdateWorkspace({ data: CreateWorkspaceBuildRequest, ) => Promise >() - .mockResolvedValueOnce(stopBuild) - .mockResolvedValue(startBuild); + .mockImplementation((_workspaceId, data) => + Promise.resolve(data.transition === "stop" ? stopBuild : startBuild), + ); const waitForBuild = vi .fn<(build: WorkspaceBuild) => Promise>() .mockResolvedValue(succeededJob(workspace)); @@ -318,7 +330,7 @@ describe("updateWorkspace", () => { { transition: "start", template_version_id: "active-version", - rich_parameter_values: oldBuildParameters, + rich_parameter_values: [], reason: "vscode_connection", }, ); @@ -350,7 +362,7 @@ describe("updateWorkspace", () => { expect(restClient.getTemplateVersionRichParameters).not.toHaveBeenCalled(); }); - it("uses template defaults for missing optional parameters", async () => { + it("omits missing optional mutable parameters", async () => { const { ctx, restClient } = setupUpdateWorkspace({ templateParameters: [ templateParameter({ name: "editor", default_value: "vim" }), @@ -359,15 +371,74 @@ describe("updateWorkspace", () => { await updateWorkspace(ctx); + expect(vscode.window.showInputBox).not.toHaveBeenCalled(); + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( + ctx.workspace.id, + expect.objectContaining({ rich_parameter_values: [] }), + ); + }); + + it("prompts for missing required parameters before stopping", async () => { + const workspace = createWorkspace({ + outdated: true, + template_use_classic_parameter_flow: true, + latest_build: { status: "running", transition: "start" }, + }); + const { ctx, restClient } = setupUpdateWorkspace({ + workspace, + templateParameters: [ + templateParameter({ + name: "project", + required: true, + default_value: "", + }), + ], + }); + vi.mocked(vscode.window.showInputBox).mockResolvedValueOnce("project-a"); + + await updateWorkspace(ctx); + + expect(vscode.window.showInputBox).toHaveBeenCalledWith( + expect.objectContaining({ title: "Workspace parameter: project" }), + ); + expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( + 2, + workspace.id, + expect.objectContaining({ + rich_parameter_values: [{ name: "project", value: "project-a" }], + }), + ); + expect( + vi.mocked(vscode.window.showInputBox).mock.invocationCallOrder[0], + ).toBeLessThan( + vi.mocked(restClient.postWorkspaceBuild).mock.invocationCallOrder[0], + ); + }); + + it("prompts for first-time immutable parameters", async () => { + const { ctx, restClient } = setupUpdateWorkspace({ + templateParameters: [ + templateParameter({ + name: "image", + mutable: false, + required: false, + default_value: "ubuntu", + }), + ], + }); + vi.mocked(vscode.window.showInputBox).mockResolvedValueOnce("debian"); + + await updateWorkspace(ctx); + expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( ctx.workspace.id, expect.objectContaining({ - rich_parameter_values: [{ name: "editor", value: "vim" }], + rich_parameter_values: [{ name: "image", value: "debian" }], }), ); }); - it("falls back to the template default when an old option value is invalid", async () => { + it("prompts when an old scalar option value is invalid", async () => { const { ctx, restClient } = setupUpdateWorkspace({ oldBuildParameters: [{ name: "color", value: "blue" }], templateParameters: [ @@ -378,38 +449,47 @@ describe("updateWorkspace", () => { }), ], }); + chooseQuickPickValue("green"); await updateWorkspace(ctx); + expect(vscode.window.showQuickPick).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ label: "red", value: "red" }), + expect.objectContaining({ label: "green", value: "green" }), + ]), + expect.objectContaining({ title: "Workspace parameter: color" }), + ); expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( ctx.workspace.id, expect.objectContaining({ - rich_parameter_values: [{ name: "color", value: "red" }], + rich_parameter_values: [{ name: "color", value: "green" }], }), ); }); - it("preserves valid multi-select values", async () => { - const oldBuildParameters = [ - { name: "tools", value: JSON.stringify(["vim", "emacs"]) }, - ]; + it("does not prompt for invalid multi-select values", async () => { const { ctx, restClient } = setupUpdateWorkspace({ - oldBuildParameters, + oldBuildParameters: [ + { name: "tools", value: JSON.stringify(["vim", "emacs"]) }, + ], templateParameters: [ templateParameter({ name: "tools", type: "list(string)", + form_type: "multi-select", default_value: JSON.stringify(["vim"]), - options: [parameterOption("vim"), parameterOption("emacs")], + options: [parameterOption("vim")], }), ], }); await updateWorkspace(ctx); + expect(vscode.window.showQuickPick).not.toHaveBeenCalled(); expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( ctx.workspace.id, - expect.objectContaining({ rich_parameter_values: oldBuildParameters }), + expect.objectContaining({ rich_parameter_values: [] }), ); }); @@ -437,7 +517,7 @@ describe("updateWorkspace", () => { ); }); - it("throws before stopping when a required parameter has no default", async () => { + it("cancels before stopping when a parameter prompt is dismissed", async () => { const workspace = createWorkspace({ outdated: true, template_use_classic_parameter_flow: true, @@ -454,7 +534,9 @@ describe("updateWorkspace", () => { ], }); - await expect(updateWorkspace(ctx)).rejects.toThrow('parameter "project"'); + await expect(updateWorkspace(ctx)).rejects.toThrow( + "Workspace update canceled while configuring parameters", + ); expect(restClient.postWorkspaceBuild).not.toHaveBeenCalled(); expect(restClient.waitForBuild).not.toHaveBeenCalled(); }); From ef744a2a4d11616343670a119ce3a976a1ec118b Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Fri, 8 May 2026 13:57:31 +0000 Subject: [PATCH 3/3] fix: run workspace updates in interactive task --- src/api/workspace.ts | 380 +++++----------------------- test/mocks/vscode.runtime.ts | 35 +++ test/unit/api/workspace.test.ts | 433 +++++++------------------------- 3 files changed, 190 insertions(+), 658 deletions(-) diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 5637a346..b1bf7831 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -1,11 +1,8 @@ import { type Api } from "coder/site/src/api/api"; import { - type CreateWorkspaceBuildRequest, type ProvisionerJobLog, - type TemplateVersionParameter, type Workspace, type WorkspaceAgentLog, - type WorkspaceBuildParameter, } from "coder/site/src/api/typesGenerated"; import { spawn } from "node:child_process"; import * as vscode from "vscode"; @@ -116,39 +113,23 @@ export async function startWorkspace(ctx: CliContext): Promise { } /** - * Update a workspace to the latest template version using the API. + * Update a workspace to the latest template version. * - * Parameter values that need user input are collected with VS Code prompts - * before the workspace is stopped. + * Uses an interactive VS Code task so the CLI can handle parameter prompts. */ export async function updateWorkspace(ctx: CliContext): Promise { - const workspace = await ctx.restClient.getWorkspace(ctx.workspace.id); - if (!workspace.outdated) { - ctx.write("Workspace is up-to-date.\r\n"); - return workspace; + if (!ctx.featureSet.cliUpdate) { + return updateWorkspaceVersion(ctx); } - const targetVersionId = workspace.template_active_version_id; - const oldBuildParameters = await ctx.restClient.getWorkspaceBuildParameters( - workspace.latest_build.id, - ); - const templateParameters = await getUpdateTemplateParameters( - ctx, - workspace, - oldBuildParameters, - targetVersionId, - ); - const buildParameters = await promptForUpdateParameters( - oldBuildParameters, - templateParameters, - ); - - if (workspace.latest_build.transition === "start") { + await runCliCommandInTask(ctx, ["update"]); + return ctx.restClient.getWorkspace(ctx.workspace.id); +} + +async function updateWorkspaceVersion(ctx: CliContext): Promise { + if (ctx.workspace.latest_build.status === "running") { ctx.write("Stopping workspace for update...\r\n"); - const stopBuild = await ctx.restClient.postWorkspaceBuild( - workspace.id, - withBuildReason(ctx, { transition: "stop" }), - ); + 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 canceled during stop"); @@ -156,302 +137,73 @@ export async function updateWorkspace(ctx: CliContext): Promise { } ctx.write("Starting workspace with updated template...\r\n"); - const startBuild = await ctx.restClient.postWorkspaceBuild( - workspace.id, - withBuildReason(ctx, { - transition: "start", - template_version_id: targetVersionId, - rich_parameter_values: buildParameters, - }), - ); - const startedJob = await ctx.restClient.waitForBuild(startBuild); - if (startedJob?.status === "canceled") { - throw new Error("Workspace update canceled during start"); - } - - return ctx.restClient.getWorkspace(workspace.id); -} - -async function getUpdateTemplateParameters( - ctx: CliContext, - workspace: Workspace, - oldBuildParameters: WorkspaceBuildParameter[], - targetVersionId: string, -): Promise { - if (workspace.template_use_classic_parameter_flow) { - return ctx.restClient.getTemplateVersionRichParameters(targetVersionId); - } - - return ctx.restClient.getDynamicParameters( - targetVersionId, - workspace.owner_id, - oldBuildParameters, - ); -} - -function withBuildReason( - ctx: CliContext, - request: CreateWorkspaceBuildRequest, -): CreateWorkspaceBuildRequest { - if (!ctx.featureSet.buildReason) { - return request; - } - return { ...request, reason: "vscode_connection" }; -} - -async function promptForUpdateParameters( - oldBuildParameters: WorkspaceBuildParameter[], - templateParameters: TemplateVersionParameter[], -): Promise { - const missingParameters = getMissingParameters( - oldBuildParameters, - [], - templateParameters, - ); - const buildParameters: WorkspaceBuildParameter[] = []; - - for (const parameter of missingParameters) { - const value = await promptForParameter(parameter); - if (value === undefined) { - throw new Error("Workspace update canceled while configuring parameters"); - } - buildParameters.push({ name: parameter.name, value }); - } - - return buildParameters; + await ctx.restClient.updateWorkspaceVersion(ctx.workspace); + return ctx.restClient.getWorkspace(ctx.workspace.id); } -function getMissingParameters( - oldBuildParameters: WorkspaceBuildParameter[], - buildParameters: WorkspaceBuildParameter[], - templateParameters: TemplateVersionParameter[], -): TemplateVersionParameter[] { - const missingParameters: TemplateVersionParameter[] = []; - const requiredParameters = templateParameters.filter( - (parameter) => - (parameter.mutable && parameter.required) || !parameter.mutable, - ); - - for (const parameter of requiredParameters) { - const buildParameter = findBuildParameter( - parameter, - oldBuildParameters, - buildParameters, - ); - if (!buildParameter) { - missingParameters.push(parameter); - } - } - - for (const parameter of templateParameters) { - if ( - parameter.options.length === 0 || - parameter.form_type === "multi-select" - ) { - continue; - } - - const buildParameter = findBuildParameter( - parameter, - oldBuildParameters, - buildParameters, - ); - if (!buildParameter) { - continue; - } - - const matchingOption = parameter.options.find( - (option) => option.value === buildParameter.value, +function runCliCommandInTask(ctx: CliContext, args: string[]): Promise { + return new Promise((resolve, reject) => { + const workspaceIdentifier = createWorkspaceIdentifier(ctx.workspace); + const fullArgs = [ + ...getGlobalShellFlags(vscode.workspace.getConfiguration(), ctx.auth), + ...args, + workspaceIdentifier, + ]; + const cmd = `${escapeCommandArg(ctx.binPath)} ${fullArgs.join(" ")}`; + const task = new vscode.Task( + { type: "coder", command: args[0] }, + vscode.TaskScope.Workspace, + `Coder: ${args[0]} ${workspaceIdentifier}`, + "coder", + new vscode.ShellExecution(cmd), + [], ); - if (!matchingOption && !missingParameters.includes(parameter)) { - missingParameters.push(parameter); - } - } - - return missingParameters; -} - -function findBuildParameter( - parameter: TemplateVersionParameter, - oldBuildParameters: WorkspaceBuildParameter[], - buildParameters: WorkspaceBuildParameter[], -): WorkspaceBuildParameter | undefined { - return ( - buildParameters.find((p) => p.name === parameter.name) ?? - oldBuildParameters.find((p) => p.name === parameter.name) - ); -} - -async function promptForParameter( - parameter: TemplateVersionParameter, -): Promise { - if (parameter.options.length > 0) { - if ( - parameter.form_type === "multi-select" || - parameter.type === "list(string)" - ) { - return promptForMultiSelectParameter(parameter); - } - return promptForSelectParameter(parameter); - } - - if (parameter.type === "bool" || parameter.form_type === "checkbox") { - return promptForBooleanParameter(parameter); - } - - return promptForTextParameter(parameter); -} - -type ParameterQuickPickItem = vscode.QuickPickItem & { - value: string; -}; - -async function promptForSelectParameter( - parameter: TemplateVersionParameter, -): Promise { - const items = parameter.options.map((option): ParameterQuickPickItem => { - const details = [option.description]; - if (option.value === parameter.default_value) { - details.push("Default"); - } - return { - label: option.name || option.value, - description: option.value, - detail: details.filter(Boolean).join(" • "), - value: option.value, + task.presentationOptions = { + reveal: vscode.TaskRevealKind.Always, + panel: vscode.TaskPanelKind.Dedicated, + focus: true, + clear: true, }; - }); - const choice = await vscode.window.showQuickPick(items, { - title: parameterTitle(parameter), - placeHolder: parameterPlaceHolder(parameter), - ignoreFocusOut: true, - }); - return choice?.value; -} -async function promptForMultiSelectParameter( - parameter: TemplateVersionParameter, -): Promise { - const defaultValues = parseListParameterValue(parameter.default_value); - const items = parameter.options.map( - (option): ParameterQuickPickItem => ({ - label: option.name || option.value, - description: option.value, - detail: option.description, - picked: defaultValues.includes(option.value), - value: option.value, - }), - ); - const choices = await vscode.window.showQuickPick(items, { - title: parameterTitle(parameter), - placeHolder: parameterPlaceHolder(parameter), - canPickMany: true, - ignoreFocusOut: true, - }); - return choices - ? JSON.stringify(choices.map((choice) => choice.value)) - : undefined; -} - -async function promptForBooleanParameter( - parameter: TemplateVersionParameter, -): Promise { - const items: ParameterQuickPickItem[] = [ - { label: "Yes", value: "true" }, - { label: "No", value: "false" }, - ].map((item) => ({ - ...item, - detail: item.value === parameter.default_value ? "Default" : undefined, - })); - const choice = await vscode.window.showQuickPick(items, { - title: parameterTitle(parameter), - placeHolder: parameterPlaceHolder(parameter), - ignoreFocusOut: true, - }); - return choice?.value; -} - -async function promptForTextParameter( - parameter: TemplateVersionParameter, -): Promise { - return vscode.window.showInputBox({ - title: parameterTitle(parameter), - prompt: parameterPlaceHolder(parameter), - value: parameter.default_value, - password: parameter.form_type === "password", - ignoreFocusOut: true, - validateInput: (value) => validateParameterInput(parameter, value), - }); -} + let execution: vscode.TaskExecution | undefined; + let settled = false; -function validateParameterInput( - parameter: TemplateVersionParameter, - value: string, -): string | undefined { - if (parameter.required && value === "") { - return "A value is required."; - } - - if (parameter.type === "number") { - const numberValue = Number(value); - if (!Number.isFinite(numberValue)) { - return "Enter a number."; - } - if ( - parameter.validation_min !== undefined && - numberValue < parameter.validation_min - ) { - return `Enter a number greater than or equal to ${parameter.validation_min}.`; - } - if ( - parameter.validation_max !== undefined && - numberValue > parameter.validation_max - ) { - return `Enter a number less than or equal to ${parameter.validation_max}.`; - } - } - - if (parameter.type === "list(string)" && parameter.options.length === 0) { - const values = parseListParameterValue(value); - if (values.length === 0 && value !== "[]") { - return "Enter a JSON array of strings."; + function finish(exitCode: number | undefined) { + if (settled) return; + settled = true; + disposable.dispose(); + if (exitCode === 0) { + resolve(); + } else { + reject( + new Error(`"${fullArgs.join(" ")}" exited with code ${exitCode}`), + ); + } } - } - if (parameter.validation_regex) { - const regex = new RegExp(parameter.validation_regex); - if (!regex.test(value)) { - return parameter.validation_error || "Enter a valid value."; + function fail(error: unknown) { + if (settled) return; + settled = true; + disposable.dispose(); + reject(error instanceof Error ? error : new Error(String(error))); } - } - - return undefined; -} -function parseListParameterValue(value: string): string[] { - try { - const parsed: unknown = JSON.parse(value); - return Array.isArray(parsed) && - parsed.every((item): item is string => typeof item === "string") - ? parsed - : []; - } catch { - return []; - } -} - -function parameterTitle(parameter: TemplateVersionParameter): string { - return `Workspace parameter: ${parameterDisplayName(parameter)}`; -} + const disposable = vscode.tasks.onDidEndTaskProcess((event) => { + if (execution) { + if (event.execution !== execution) { + return; + } + } else if (event.execution.task !== task) { + return; + } -function parameterPlaceHolder(parameter: TemplateVersionParameter): string { - return ( - parameter.description_plaintext || parameter.description || parameter.name - ); -} + finish(event.exitCode); + }); -function parameterDisplayName(parameter: TemplateVersionParameter): string { - return parameter.display_name || parameter.name; + vscode.tasks.executeTask(task).then((taskExecution) => { + execution = taskExecution; + }, fail); + }); } /** diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index 8def001e..88f349fb 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -47,6 +47,9 @@ export const InputBoxValidationSeverity = E({ Warning: 2, Error: 3, }); +export const TaskScope = E({ Global: 1, Workspace: 2 }); +export const TaskRevealKind = E({ Always: 1, Silent: 2, Never: 3 }); +export const TaskPanelKind = E({ Shared: 1, Dedicated: 2, New: 3 }); export class MarkdownString { value: string; @@ -65,6 +68,25 @@ export class ThemeColor { } } +export class ShellExecution { + commandLine: string | undefined; + constructor(commandLine: string) { + this.commandLine = commandLine; + } +} + +export class Task { + presentationOptions: unknown; + constructor( + public definition: unknown, + public scope: unknown, + public name: string, + public source: string, + public execution?: unknown, + public problemMatchers: string[] = [], + ) {} +} + export class Uri { constructor( public scheme: string, @@ -114,6 +136,7 @@ export class EventEmitter { const onDidChangeConfiguration = new EventEmitter(); const onDidChangeWorkspaceFolders = new EventEmitter(); const onDidChangeActiveColorTheme = new EventEmitter(); +const onDidEndTaskProcess = new EventEmitter(); export const window = { activeColorTheme: { kind: ColorThemeKind.Dark }, @@ -148,6 +171,12 @@ export const commands = { executeCommand: vi.fn(), }; +export const tasks = { + executeTask: vi.fn(), + onDidEndTaskProcess: onDidEndTaskProcess.event, + __fireDidEndTaskProcess: (e: unknown) => onDidEndTaskProcess.fire(e), +}; + export const workspace = { getConfiguration: vi.fn(), // your helpers override this workspaceFolders: [] as unknown[], @@ -197,12 +226,18 @@ const vscode = { ExtensionMode, UIKind, InputBoxValidationSeverity, + TaskScope, + TaskRevealKind, + TaskPanelKind, + ShellExecution, + Task, Uri, EventEmitter, MarkdownString, ThemeColor, window, commands, + tasks, workspace, env, extensions, diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index f7eea842..0419ba94 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -9,33 +9,16 @@ import { workspace as createWorkspace } from "../../mocks/workspace"; import type { Api } from "coder/site/src/api/api"; import type { - CreateWorkspaceBuildRequest, ProvisionerJob, - TemplateVersionParameter, - TemplateVersionParameterOption, Workspace, WorkspaceBuild, - WorkspaceBuildParameter, } from "coder/site/src/api/typesGenerated"; type UpdateWorkspaceContext = Parameters[0]; interface UpdateRestClient { getWorkspace: (workspaceId: string) => Promise; - getWorkspaceBuildParameters: ( - workspaceBuildId: string, - ) => Promise; - getTemplateVersionRichParameters: ( - versionId: string, - ) => Promise; - getDynamicParameters: ( - templateVersionId: string, - ownerId: string, - oldBuildParameters: WorkspaceBuildParameter[], - ) => Promise; - postWorkspaceBuild: ( - workspaceId: string, - data: CreateWorkspaceBuildRequest, - ) => Promise; + stopWorkspace: (workspaceId: string) => Promise; + updateWorkspaceVersion: (workspace: Workspace) => Promise; waitForBuild: (build: WorkspaceBuild) => Promise; } @@ -75,45 +58,6 @@ function deferredFactory() { }; } -function templateParameter( - overrides: Partial = {}, -): TemplateVersionParameter { - return { - name: "parameter", - description: "", - description_plaintext: "", - type: "string", - form_type: "", - mutable: true, - default_value: "default", - icon: "", - options: [], - required: false, - ephemeral: false, - ...overrides, - }; -} - -function parameterOption( - value: string, - overrides: Partial = {}, -): TemplateVersionParameterOption { - return { - name: value, - description: "", - value, - icon: "", - ...overrides, - }; -} - -function chooseQuickPickValue(value: string): void { - vi.mocked(vscode.window.showQuickPick).mockResolvedValueOnce({ - label: value, - value, - } as never); -} - function createBuild( workspace: Workspace, overrides: Partial = {}, @@ -131,26 +75,22 @@ function succeededJob(workspace: Workspace): ProvisionerJob { function setupUpdateWorkspace({ workspace = createWorkspace({ outdated: true, - template_use_classic_parameter_flow: true, - latest_build: { status: "stopped", transition: "stop" }, + latest_build: { status: "running", transition: "start" }, }), finalWorkspace = createWorkspace({ outdated: false, - template_use_classic_parameter_flow: true, latest_build: { status: "running" }, }), - oldBuildParameters = [], - templateParameters = [], featureSetOverrides = {}, }: { workspace?: Workspace; finalWorkspace?: Workspace; - oldBuildParameters?: WorkspaceBuildParameter[]; - templateParameters?: TemplateVersionParameter[]; featureSetOverrides?: Partial; } = {}) { - vi.mocked(vscode.window.showInputBox).mockReset(); - vi.mocked(vscode.window.showQuickPick).mockReset(); + vi.mocked(vscode.tasks.executeTask).mockReset(); + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn((_key: string, defaultValue: unknown) => defaultValue), + } as never); const stopBuild = createBuild(workspace, { id: "stop-build", @@ -164,41 +104,19 @@ function setupUpdateWorkspace({ transition: "start", status: "running", }); - const postWorkspaceBuild = vi - .fn< - ( - workspaceId: string, - data: CreateWorkspaceBuildRequest, - ) => Promise - >() - .mockImplementation((_workspaceId, data) => - Promise.resolve(data.transition === "stop" ? stopBuild : startBuild), - ); - const waitForBuild = vi - .fn<(build: WorkspaceBuild) => Promise>() - .mockResolvedValue(succeededJob(workspace)); const restClient: UpdateRestClient = { getWorkspace: vi .fn<(workspaceId: string) => Promise>() - .mockResolvedValueOnce(workspace) .mockResolvedValue(finalWorkspace), - getWorkspaceBuildParameters: vi - .fn<(workspaceBuildId: string) => Promise>() - .mockResolvedValue(oldBuildParameters), - getTemplateVersionRichParameters: vi - .fn<(versionId: string) => Promise>() - .mockResolvedValue(templateParameters), - getDynamicParameters: vi - .fn< - ( - templateVersionId: string, - ownerId: string, - oldBuildParameters: WorkspaceBuildParameter[], - ) => Promise - >() - .mockResolvedValue(templateParameters), - postWorkspaceBuild, - waitForBuild, + stopWorkspace: vi + .fn<(workspaceId: string) => Promise>() + .mockResolvedValue(stopBuild), + updateWorkspaceVersion: vi + .fn<(workspace: Workspace) => Promise>() + .mockResolvedValue(startBuild), + waitForBuild: vi + .fn<(build: WorkspaceBuild) => Promise>() + .mockResolvedValue(succeededJob(workspace)), }; const write = vi.fn<(data: string) => void>(); const ctx: UpdateWorkspaceContext = { @@ -209,7 +127,25 @@ function setupUpdateWorkspace({ write, featureSet: { ...featureSet, ...featureSetOverrides }, }; - return { ctx, restClient, startBuild, stopBuild, write }; + return { ctx, restClient, startBuild, stopBuild, write, finalWorkspace }; +} + +function setupTaskExecution(exitCode = 0) { + const mockedTasks = vscode.tasks as typeof vscode.tasks & { + __fireDidEndTaskProcess: (event: unknown) => void; + }; + const execution = { + task: undefined as unknown as vscode.Task, + terminate: vi.fn(), + }; + vi.mocked(vscode.tasks.executeTask).mockImplementation((task) => { + execution.task = task; + queueMicrotask(() => { + mockedTasks.__fireDidEndTaskProcess({ execution, exitCode }); + }); + return Promise.resolve(execution); + }); + return execution; } describe("LazyStream", () => { @@ -272,284 +208,93 @@ describe("LazyStream", () => { }); describe("updateWorkspace", () => { - it("returns the fresh workspace without building when already up to date", async () => { - const workspace = createWorkspace({ outdated: false }); - const { ctx, restClient, write } = setupUpdateWorkspace({ workspace }); - - await expect(updateWorkspace(ctx)).resolves.toBe(workspace); - - expect(write).toHaveBeenCalledWith("Workspace is up-to-date.\r\n"); - expect(restClient.getWorkspaceBuildParameters).not.toHaveBeenCalled(); - expect(restClient.postWorkspaceBuild).not.toHaveBeenCalled(); - expect(restClient.waitForBuild).not.toHaveBeenCalled(); - }); - - it("stops a started workspace before starting the update build", async () => { - const workspace = createWorkspace({ - outdated: true, - template_active_version_id: "active-version", - template_use_classic_parameter_flow: true, - latest_build: { status: "running", transition: "start" }, - }); - const finalWorkspace = createWorkspace({ - outdated: false, - template_active_version_id: "active-version", - latest_build: { - status: "running", - template_version_id: "active-version", - }, - }); - const oldBuildParameters = [{ name: "region", value: "us" }]; - const templateParameters = [ - templateParameter({ name: "region", default_value: "eu" }), - ]; - const { ctx, restClient, startBuild, stopBuild } = setupUpdateWorkspace({ - workspace, - finalWorkspace, - oldBuildParameters, - templateParameters, - }); + it("runs coder update in an interactive task", async () => { + const { ctx, restClient, finalWorkspace } = setupUpdateWorkspace(); + setupTaskExecution(); await expect(updateWorkspace(ctx)).resolves.toBe(finalWorkspace); - expect(restClient.getWorkspace).toHaveBeenNthCalledWith(1, workspace.id); - expect(restClient.getWorkspaceBuildParameters).toHaveBeenCalledWith( - workspace.latest_build.id, - ); - expect(restClient.getTemplateVersionRichParameters).toHaveBeenCalledWith( - "active-version", - ); - expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( - 1, - workspace.id, - { transition: "stop", reason: "vscode_connection" }, - ); - expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( - 2, - workspace.id, - { - transition: "start", - template_version_id: "active-version", - rich_parameter_values: [], - reason: "vscode_connection", - }, - ); - expect(restClient.waitForBuild).toHaveBeenNthCalledWith(1, stopBuild); - expect(restClient.waitForBuild).toHaveBeenNthCalledWith(2, startBuild); - }); - - it("evaluates dynamic parameters when the template uses dynamic parameter flow", async () => { - const workspace = createWorkspace({ - outdated: true, - template_active_version_id: "active-version", - template_use_classic_parameter_flow: false, - latest_build: { status: "stopped", transition: "stop" }, + expect(vscode.tasks.executeTask).toHaveBeenCalledOnce(); + const task = vi.mocked(vscode.tasks.executeTask).mock.calls[0][0]; + expect(task.name).toBe("Coder: update testuser/test-workspace"); + expect(task.presentationOptions).toMatchObject({ + reveal: vscode.TaskRevealKind.Always, + panel: vscode.TaskPanelKind.Dedicated, + focus: true, + clear: true, }); - const oldBuildParameters = [{ name: "region", value: "us" }]; - const { ctx, restClient } = setupUpdateWorkspace({ - workspace, - oldBuildParameters, - templateParameters: [templateParameter({ name: "region" })], + expect(task.execution).toMatchObject({ + commandLine: + '"/usr/bin/coder" --url "https://test.coder.com" update testuser/test-workspace', }); - - await updateWorkspace(ctx); - - expect(restClient.getDynamicParameters).toHaveBeenCalledWith( - "active-version", - workspace.owner_id, - oldBuildParameters, - ); - expect(restClient.getTemplateVersionRichParameters).not.toHaveBeenCalled(); + expect(restClient.stopWorkspace).not.toHaveBeenCalled(); + expect(restClient.updateWorkspaceVersion).not.toHaveBeenCalled(); + expect(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id); }); - it("omits missing optional mutable parameters", async () => { - const { ctx, restClient } = setupUpdateWorkspace({ - templateParameters: [ - templateParameter({ name: "editor", default_value: "vim" }), - ], - }); + it("rejects when the update task exits non-zero", async () => { + const { ctx, restClient } = setupUpdateWorkspace(); + setupTaskExecution(1); - await updateWorkspace(ctx); + await expect(updateWorkspace(ctx)).rejects.toThrow("exited with code 1"); - expect(vscode.window.showInputBox).not.toHaveBeenCalled(); - expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( - ctx.workspace.id, - expect.objectContaining({ rich_parameter_values: [] }), - ); + expect(restClient.getWorkspace).not.toHaveBeenCalled(); }); - it("prompts for missing required parameters before stopping", async () => { + it("falls back to the API update path when coder update is unsupported", async () => { const workspace = createWorkspace({ outdated: true, - template_use_classic_parameter_flow: true, latest_build: { status: "running", transition: "start" }, }); - const { ctx, restClient } = setupUpdateWorkspace({ - workspace, - templateParameters: [ - templateParameter({ - name: "project", - required: true, - default_value: "", - }), - ], - }); - vi.mocked(vscode.window.showInputBox).mockResolvedValueOnce("project-a"); + const { ctx, restClient, finalWorkspace, startBuild, stopBuild, write } = + setupUpdateWorkspace({ + workspace, + featureSetOverrides: { cliUpdate: false }, + }); - await updateWorkspace(ctx); - - expect(vscode.window.showInputBox).toHaveBeenCalledWith( - expect.objectContaining({ title: "Workspace parameter: project" }), - ); - expect(restClient.postWorkspaceBuild).toHaveBeenNthCalledWith( - 2, - workspace.id, - expect.objectContaining({ - rich_parameter_values: [{ name: "project", value: "project-a" }], - }), - ); - expect( - vi.mocked(vscode.window.showInputBox).mock.invocationCallOrder[0], - ).toBeLessThan( - vi.mocked(restClient.postWorkspaceBuild).mock.invocationCallOrder[0], - ); - }); - - it("prompts for first-time immutable parameters", async () => { - const { ctx, restClient } = setupUpdateWorkspace({ - templateParameters: [ - templateParameter({ - name: "image", - mutable: false, - required: false, - default_value: "ubuntu", - }), - ], - }); - vi.mocked(vscode.window.showInputBox).mockResolvedValueOnce("debian"); - - await updateWorkspace(ctx); + await expect(updateWorkspace(ctx)).resolves.toBe(finalWorkspace); - expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( - ctx.workspace.id, - expect.objectContaining({ - rich_parameter_values: [{ name: "image", value: "debian" }], - }), + expect(vscode.tasks.executeTask).not.toHaveBeenCalled(); + expect(write).toHaveBeenCalledWith("Stopping workspace for update...\r\n"); + expect(restClient.stopWorkspace).toHaveBeenCalledWith(workspace.id); + expect(restClient.waitForBuild).toHaveBeenCalledWith(stopBuild); + expect(write).toHaveBeenCalledWith( + "Starting workspace with updated template...\r\n", ); + expect(restClient.updateWorkspaceVersion).toHaveBeenCalledWith(workspace); + expect(restClient.getWorkspace).toHaveBeenCalledWith(workspace.id); + expect(startBuild.transition).toBe("start"); }); - it("prompts when an old scalar option value is invalid", async () => { - const { ctx, restClient } = setupUpdateWorkspace({ - oldBuildParameters: [{ name: "color", value: "blue" }], - templateParameters: [ - templateParameter({ - name: "color", - default_value: "red", - options: [parameterOption("red"), parameterOption("green")], - }), - ], + it("does not stop before API fallback update when the workspace is not running", async () => { + const workspace = createWorkspace({ + outdated: true, + latest_build: { status: "stopped", transition: "stop" }, }); - chooseQuickPickValue("green"); - - await updateWorkspace(ctx); - - expect(vscode.window.showQuickPick).toHaveBeenCalledWith( - expect.arrayContaining([ - expect.objectContaining({ label: "red", value: "red" }), - expect.objectContaining({ label: "green", value: "green" }), - ]), - expect.objectContaining({ title: "Workspace parameter: color" }), - ); - expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( - ctx.workspace.id, - expect.objectContaining({ - rich_parameter_values: [{ name: "color", value: "green" }], - }), - ); - }); - - it("does not prompt for invalid multi-select values", async () => { const { ctx, restClient } = setupUpdateWorkspace({ - oldBuildParameters: [ - { name: "tools", value: JSON.stringify(["vim", "emacs"]) }, - ], - templateParameters: [ - templateParameter({ - name: "tools", - type: "list(string)", - form_type: "multi-select", - default_value: JSON.stringify(["vim"]), - options: [parameterOption("vim")], - }), - ], + workspace, + featureSetOverrides: { cliUpdate: false }, }); await updateWorkspace(ctx); - expect(vscode.window.showQuickPick).not.toHaveBeenCalled(); - expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( - ctx.workspace.id, - expect.objectContaining({ rich_parameter_values: [] }), - ); + expect(restClient.stopWorkspace).not.toHaveBeenCalled(); + expect(restClient.waitForBuild).not.toHaveBeenCalled(); + expect(restClient.updateWorkspaceVersion).toHaveBeenCalledWith(workspace); }); - it("omits ephemeral and previously-set immutable parameters", async () => { + it("throws before update when the API fallback stop is canceled", async () => { const { ctx, restClient } = setupUpdateWorkspace({ - oldBuildParameters: [ - { name: "token", value: "secret" }, - { name: "size", value: "large" }, - ], - templateParameters: [ - templateParameter({ name: "token", ephemeral: true }), - templateParameter({ - name: "size", - mutable: false, - default_value: "small", - }), - ], + featureSetOverrides: { cliUpdate: false }, }); - - await updateWorkspace(ctx); - - expect(restClient.postWorkspaceBuild).toHaveBeenCalledWith( - ctx.workspace.id, - expect.objectContaining({ rich_parameter_values: [] }), - ); - }); - - it("cancels before stopping when a parameter prompt is dismissed", async () => { - const workspace = createWorkspace({ - outdated: true, - template_use_classic_parameter_flow: true, - latest_build: { status: "running", transition: "start" }, - }); - const { ctx, restClient } = setupUpdateWorkspace({ - workspace, - templateParameters: [ - templateParameter({ - name: "project", - required: true, - default_value: "", - }), - ], + vi.mocked(restClient.waitForBuild).mockResolvedValueOnce({ + ...ctx.workspace.latest_build.job, + status: "canceled", }); await expect(updateWorkspace(ctx)).rejects.toThrow( - "Workspace update canceled while configuring parameters", + "Workspace update canceled during stop", ); - expect(restClient.postWorkspaceBuild).not.toHaveBeenCalled(); - expect(restClient.waitForBuild).not.toHaveBeenCalled(); - }); - - it("omits build reasons when unsupported by the server", async () => { - const { ctx, restClient } = setupUpdateWorkspace({ - templateParameters: [templateParameter({ name: "region" })], - featureSetOverrides: { buildReason: false }, - }); - - await updateWorkspace(ctx); - - const request = vi.mocked(restClient.postWorkspaceBuild).mock.calls[0][1]; - expect(request).not.toHaveProperty("reason"); + expect(restClient.updateWorkspaceVersion).not.toHaveBeenCalled(); }); });