From fc9e04da67b33258ffd8a7132145c2c4585c3753 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 12 Mar 2026 09:09:17 -0700 Subject: [PATCH] fix(chatd): handle soft-deleted workspaces in chattool start/create (#22997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Both `start_workspace` and `create_workspace` chattool tools failed to handle soft-deleted workspaces correctly. Coder uses soft-delete for workspaces (`deleted = true` on the row). Both tools called `GetWorkspaceByID`, which queries `workspaces_expanded` with **no** `deleted = false` filter — so it returns the workspace row even when soft-deleted. The only deletion check was for `sql.ErrNoRows`, which never fires because the row still exists. ### `start_workspace` behavior (before fix) 1. Loads the soft-deleted workspace successfully 2. Finds the latest build (a delete transition) 3. Falls through to attempt to **start** the deleted workspace 4. Produces a confusing downstream error ### `create_workspace` behavior (before fix) 1. `checkExistingWorkspace` loads the soft-deleted workspace 2. If a delete build is **in-progress**: waits for it, then falsely reports `already_exists` — blocks new workspace creation 3. If the delete build **succeeded**: accidentally allows creation (because no agents are found), but via fragile logic rather than an explicit check ## Fix Add `ws.Deleted` checks immediately after `GetWorkspaceByID` succeeds in both tools: - **`startworkspace.go`**: Returns `"workspace was deleted; use create_workspace to make a new one"` - **`createworkspace.go`** (`checkExistingWorkspace`): Returns `(nil, false, nil)` to allow new workspace creation ## Tests - `TestStartWorkspace/DeletedWorkspace` — verifies `start_workspace` returns deleted error and never calls `StartFn` - `TestCheckExistingWorkspace_DeletedWorkspace` — verifies `checkExistingWorkspace` allows creation for soft-deleted workspaces --- coderd/chatd/chattool/createworkspace.go | 10 ++--- coderd/chatd/chattool/createworkspace_test.go | 32 +++++++++++++ coderd/chatd/chattool/startworkspace.go | 11 +++-- coderd/chatd/chattool/startworkspace_test.go | 45 +++++++++++++++++++ 4 files changed, 86 insertions(+), 12 deletions(-) diff --git a/coderd/chatd/chattool/createworkspace.go b/coderd/chatd/chattool/createworkspace.go index 3f4c480b64..6d1f998808 100644 --- a/coderd/chatd/chattool/createworkspace.go +++ b/coderd/chatd/chattool/createworkspace.go @@ -2,7 +2,6 @@ package chattool import ( "context" - "database/sql" "errors" "fmt" "strings" @@ -241,15 +240,14 @@ func checkExistingWorkspace( return nil, false, nil } - // Check if workspace still exists. ws, err := db.GetWorkspaceByID(ctx, chat.WorkspaceID.UUID) if err != nil { - if xerrors.Is(err, sql.ErrNoRows) { - // Workspace was deleted — allow creation. - return nil, false, nil - } return nil, false, xerrors.Errorf("load workspace: %w", err) } + // Workspace was soft-deleted — allow creation. + if ws.Deleted { + return nil, false, nil + } // Check the latest build status. build, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, ws.ID) diff --git a/coderd/chatd/chattool/createworkspace_test.go b/coderd/chatd/chattool/createworkspace_test.go index 3b8c914eb4..d8c38c55bf 100644 --- a/coderd/chatd/chattool/createworkspace_test.go +++ b/coderd/chatd/chattool/createworkspace_test.go @@ -108,3 +108,35 @@ func TestWaitForAgentReady(t *testing.T) { require.Empty(t, result) }) } + +func TestCheckExistingWorkspace_DeletedWorkspace(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + db := dbmock.NewMockStore(ctrl) + + chatID := uuid.New() + workspaceID := uuid.New() + + // Mock GetChatByID returns a chat linked to a workspace. + db.EXPECT(). + GetChatByID(gomock.Any(), chatID). + Return(database.Chat{ + ID: chatID, + WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true}, + }, nil) + + // Mock GetWorkspaceByID returns a soft-deleted workspace. + db.EXPECT(). + GetWorkspaceByID(gomock.Any(), workspaceID). + Return(database.Workspace{ + ID: workspaceID, + Deleted: true, + }, nil) + + result, done, err := checkExistingWorkspace( + context.Background(), db, chatID, nil, + ) + require.NoError(t, err) + require.False(t, done, "should allow creation for deleted workspace") + require.Nil(t, result) +} diff --git a/coderd/chatd/chattool/startworkspace.go b/coderd/chatd/chattool/startworkspace.go index 9b8dee9d36..bc19a8cd77 100644 --- a/coderd/chatd/chattool/startworkspace.go +++ b/coderd/chatd/chattool/startworkspace.go @@ -2,7 +2,6 @@ package chattool import ( "context" - "database/sql" "sync" "charm.land/fantasy" @@ -71,15 +70,15 @@ func StartWorkspace(options StartWorkspaceOptions) fantasy.AgentTool { ws, err := options.DB.GetWorkspaceByID(ctx, chat.WorkspaceID.UUID) if err != nil { - if xerrors.Is(err, sql.ErrNoRows) { - return fantasy.NewTextErrorResponse( - "workspace was deleted; use create_workspace to make a new one", - ), nil - } return fantasy.NewTextErrorResponse( xerrors.Errorf("load workspace: %w", err).Error(), ), nil } + if ws.Deleted { + return fantasy.NewTextErrorResponse( + "workspace was deleted; use create_workspace to make a new one", + ), nil + } build, err := options.DB.GetLatestWorkspaceBuildByWorkspaceID(ctx, ws.ID) if err != nil { diff --git a/coderd/chatd/chattool/startworkspace_test.go b/coderd/chatd/chattool/startworkspace_test.go index 1b9023fa76..d8952346a5 100644 --- a/coderd/chatd/chattool/startworkspace_test.go +++ b/coderd/chatd/chattool/startworkspace_test.go @@ -174,6 +174,51 @@ func TestStartWorkspace(t *testing.T) { require.True(t, ok) require.True(t, started) }) + + t.Run("DeletedWorkspace", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + db, _ := dbtestutil.NewDB(t) + + user := dbgen.User(t, db, database.User{}) + modelCfg := seedModelConfig(ctx, t, db, user.ID) + org := dbgen.Organization(t, db, database.Organization{}) + _ = dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: org.ID, + }) + // Create a workspace that has been soft-deleted. + wsResp := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + OrganizationID: org.ID, + Deleted: true, + }).Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionDelete, + }).Do() + ws := wsResp.Workspace + + chat, err := db.InsertChat(ctx, database.InsertChatParams{ + OwnerID: user.ID, + WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true}, + LastModelConfigID: modelCfg.ID, + Title: "test-deleted-workspace", + }) + require.NoError(t, err) + + tool := chattool.StartWorkspace(chattool.StartWorkspaceOptions{ + DB: db, + ChatID: chat.ID, + StartFn: func(_ context.Context, _ uuid.UUID, _ uuid.UUID, _ codersdk.CreateWorkspaceBuildRequest) (codersdk.WorkspaceBuild, error) { + t.Fatal("StartFn should not be called for deleted workspace") + return codersdk.WorkspaceBuild{}, nil + }, + WorkspaceMu: &sync.Mutex{}, + }) + + resp, err := tool.Run(ctx, fantasy.ToolCall{ID: "call-1", Name: "start_workspace", Input: "{}"}) + require.NoError(t, err) + require.Contains(t, resp.Content, "workspace was deleted") + }) } // seedModelConfig inserts a provider and model config for testing.