feat!: add ability to cancel pending workspace build #18713

kacpersaw · 2025-07-02T12:51:13Z

Closes #17791

This PR adds ability to cancel workspace builds that are in "pending" status.

Breaking changes:

CancelWorkspaceBuild method in codersdk now accepts an optional request parameter

API:

Added expect_status query parameter to the cancel workspace build endpoint
This parameter ensures the job hasn't changed state before canceling
API returns 412 Precondition Failed if the job is not in the expected status
Valid values: running or pending
Wrapped the entire cancel method in a database transaction

UI:

Added confirmation dialog to the Cancel button, since it's a destructive operation
Enabled cancel action for pending workspaces (expect_status=pending is sent if workspace is in pending status)

…ocking)

…atus for pending builds

ethanndickson

Looking good, just one comment w/ the transaction

coderd/workspacebuilds.go

coderd/workspacebuilds_test.go

deansheather

BE only review

deansheather · 2025-07-03T14:05:30Z

coderd/workspacebuilds.go

+	code := http.StatusOK
+	resp := codersdk.Response{}
+	err = api.Database.InTx(func(store database.Store) error {
+		valid, err := verifyUserCanCancelWorkspaceBuilds(ctx, store, httpmw.APIKey(r).UserID, workspace.TemplateID)


We often just use the variable name db in these transaction closures

deansheather · 2025-07-03T14:12:56Z

coderd/workspacebuilds.go

+		if !valid {
+			code = http.StatusForbidden
+			resp.Message = "User is not allowed to cancel workspace builds. Owner role is required."
+
+			return xerrors.Errorf("user is not allowed to cancel workspace builds")
+		}


I think we should still allow for cancellations even if the template forbids them as long as the user specified expect_status=pending.

deansheather · 2025-07-03T14:17:27Z

coderd/workspacebuilds.go

+			return xerrors.Errorf("user is not allowed to cancel workspace builds")
+		}
+
+		job, err := store.GetProvisionerJobByID(ctx, workspaceBuild.JobID)


This should probably be a clone of this query with a SELECT ... FOR UPDATE query instead, otherwise there's nothing preventing this job from being picked up during the cancel operation.

Good catch, I forgot to change that query 🙅

deansheather · 2025-07-03T14:17:39Z

coderd/workspacebuilds.go

+			code = http.StatusBadRequest
+			resp.Message = "Job has already completed!"
+
+			return xerrors.Errorf("job has already completed")


Suggested change

return xerrors.Errorf("job has already completed")

return xerrors.New("job has already completed")

deansheather · 2025-07-03T14:17:46Z

coderd/workspacebuilds.go

+			code = http.StatusBadRequest
+			resp.Message = "Job has already been marked as canceled!"
+
+			return xerrors.Errorf("job has already been marked as canceled")


Suggested change

return xerrors.Errorf("job has already been marked as canceled")

return xerrors.New("job has already been marked as canceled")

deansheather · 2025-07-03T14:18:17Z

coderd/workspacebuilds.go

+				code = http.StatusBadRequest
+				resp.Message = "Invalid expect_status. Only 'running' or 'pending' are allowed."
+
+				return xerrors.Errorf("invalid expect_status")


Suggested change

return xerrors.Errorf("invalid expect_status")

return xerrors.Errorf("invalid expect_status %q", expectStatus)

deansheather · 2025-07-03T14:18:30Z

coderd/workspacebuilds.go

+		if expectStatus != "" {
+			if expectStatus != "running" && expectStatus != "pending" {
+				code = http.StatusBadRequest
+				resp.Message = "Invalid expect_status. Only 'running' or 'pending' are allowed."


Suggested change

resp.Message = "Invalid expect_status. Only 'running' or 'pending' are allowed."

resp.Message = fmt.Sprintf("Invalid expect_status %q. Only 'running' or 'pending' are allowed.", expectStatus)

deansheather · 2025-07-03T14:18:58Z

coderd/workspacebuilds.go

+				code = http.StatusPreconditionFailed
+				resp.Message = "Job is not in the expected state."
+
+				return xerrors.Errorf("job is not in the expected state")


Suggested change

return xerrors.Errorf("job is not in the expected state")

return xerrors.Errorf("job is not in the expected state: expected %q, got %q", expectStatus, job.JobStatus)

deansheather · 2025-07-03T14:25:37Z

codersdk/workspacebuilds.go

+	CancelWorkspaceBuildStatusPending CancelWorkspaceBuildStatus = "pending"
+)
+
+type CancelWorkspaceBuildRequest struct {


Maybe call this Params instead of Request since it's not the request body

BrunoQuaresma

I left a few comments related to the FE.

Note: When submitting FE related code, it is always nice to include a screenshot or a short video demo showing off the changes. It makes easier for us to visualize things and make faster reviews.

BrunoQuaresma · 2025-07-03T17:48:11Z

site/src/api/api.ts

@@ -1277,9 +1277,14 @@ class ApiMethods {

 	cancelWorkspaceBuild = async (
 		workspaceBuildId: TypesGen.WorkspaceBuild["id"],
+		request?: TypesGen.CancelWorkspaceBuildParams,


Since the type has the "Params" sufix, I would name this variable as params instead of request.

BrunoQuaresma · 2025-07-03T17:48:43Z

site/src/api/api.ts

 	): Promise<TypesGen.Response> => {
+		const params = request?.expect_status


You can do:

const params = new URLSearchParams(params)

BrunoQuaresma · 2025-07-03T17:49:43Z

site/src/api/api.ts

 		const response = await this.axios.patch(
-			`/api/v2/workspacebuilds/${workspaceBuildId}/cancel`,
+			`/api/v2/workspacebuilds/${workspaceBuildId}/cancel${params ? `?${params}` : ""}`,


And just use it directly:

`/api/v2/workspacebuilds/${workspaceBuildId}/cancel?${params}`

BrunoQuaresma · 2025-07-03T17:51:27Z

site/src/api/queries/workspaces.ts

@@ -266,6 +266,12 @@ export const startWorkspace = (
 export const cancelBuild = (workspace: Workspace, queryClient: QueryClient) => {
 	return {
 		mutationFn: () => {
+			// If workspace status is pending, include expect_status parameter


The comment is just saying what the code is doing below. I would rather not having this comment at all, since I can see it in the code, or explaining the why we need to set the expect_status to pending.

BrunoQuaresma · 2025-07-03T17:52:19Z

site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx

@@ -352,6 +354,21 @@ export const WorkspaceReadyPage: FC<WorkspaceReadyPageProps> = ({
 				}
 			/>

+			{/* Cancel confirmation dialog */}


I don't think we need this comment. The code below is pretty easy to understand.

BrunoQuaresma · 2025-07-03T17:52:51Z

site/src/pages/WorkspacesPage/WorkspacesTable.tsx

@@ -497,6 +497,8 @@ const WorkspaceActionsCell: FC<WorkspaceActionsCellProps> = ({

 	// State for stop confirmation dialog
 	const [isStopConfirmOpen, setIsStopConfirmOpen] = useState(false);
+	// State for cancel confirmation dialog


The same here, even seeing we do this for the state above.

BrunoQuaresma · 2025-07-03T17:56:58Z

site/src/api/typesGenerated.ts

@@ -292,6 +292,19 @@ export const BypassRatelimitHeader = "X-Coder-Bypass-Ratelimit";
 // From codersdk/client.go
 export const CLITelemetryHeader = "Coder-CLI-Telemetry";

+// From codersdk/workspacebuilds.go
+export interface CancelWorkspaceBuildParams {
+	readonly expect_status?: CancelWorkspaceBuildStatus;


I can be wrong, but I think when we add a comment to the struct, or its props in codersdk package, it gets included in the TS types as well. I think lefting a comment in the CancelWorkspaceBuildParams.ExpectStatus could be helpful to understand when and why use it.

kacpersaw added 4 commits July 1, 2025 12:31

add support for canceling workspace builds with expect_state param (l…

ba41ae8

…ocking)

Use single transaction for canceling workspace build

c4ee5b6

Fix lint problem in ut

c49c33e

add cancel confirmation dialog for workspace builds and add expect_st…

ba1dbf3

…atus for pending builds

kacpersaw requested review from deansheather and ethanndickson July 2, 2025 12:51

github-actions bot assigned kacpersaw Jul 2, 2025

kacpersaw added 2 commits July 2, 2025 12:55

Fix lint

acffda6

Merge branch 'main' into kacpersaw/cancel-pending-provisioner-jobs

b672d76

kacpersaw marked this pull request as ready for review July 2, 2025 13:52

github-actions bot added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Jul 2, 2025

ethanndickson reviewed Jul 3, 2025

View reviewed changes

coderd/workspacebuilds.go Outdated Show resolved Hide resolved

coderd/workspacebuilds.go Outdated Show resolved Hide resolved

coderd/workspacebuilds_test.go Outdated Show resolved Hide resolved

Apply review suggestions

86a34df

ethanndickson approved these changes Jul 3, 2025

View reviewed changes

Fix unit test

42170ab

kacpersaw force-pushed the kacpersaw/cancel-pending-provisioner-jobs branch from 8081f33 to 42170ab Compare July 3, 2025 10:51

kacpersaw requested a review from BrunoQuaresma July 3, 2025 11:42

deansheather reviewed Jul 3, 2025

View reviewed changes

kacpersaw added 5 commits July 3, 2025 15:10

Apply review suggestions

5db9d71

Fix typo

1ede20c

Regenerate api types

2597615

Fix typo

6c2d0cf

Merge branch 'main' into kacpersaw/cancel-pending-provisioner-jobs

c800494

BrunoQuaresma reviewed Jul 3, 2025

View reviewed changes

	return xerrors.Errorf("job has already completed")
	return xerrors.New("job has already completed")

	return xerrors.Errorf("job has already been marked as canceled")
	return xerrors.New("job has already been marked as canceled")

	return xerrors.Errorf("invalid expect_status")
	return xerrors.Errorf("invalid expect_status %q", expectStatus)

	resp.Message = "Invalid expect_status. Only 'running' or 'pending' are allowed."
	resp.Message = fmt.Sprintf("Invalid expect_status %q. Only 'running' or 'pending' are allowed.", expectStatus)

	return xerrors.Errorf("job is not in the expected state")
	return xerrors.Errorf("job is not in the expected state: expected %q, got %q", expectStatus, job.JobStatus)

		): Promise<TypesGen.Response> => {
		const params = request?.expect_status

feat!: add ability to cancel pending workspace build #18713

Are you sure you want to change the base?

feat!: add ability to cancel pending workspace build #18713

Uh oh!

Conversation

kacpersaw commented Jul 2, 2025

Uh oh!

ethanndickson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deansheather left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrunoQuaresma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TMZ Celebrity News – Breaking Stories, Videos & Gossip

🎥 Watch TMZ Live