Skip to content

Commit 2549fc7

Browse files
mafredrimafredri
authored
feat(coderd): return 409 Conflict for non-active task states (#21887)
Previously we returned 400 Bad Request for all non-active states. This was semantically incorrect for transitional and paused states where the request is valid but conflicts with current state. We now return 409 Conflict for pending/initializing/paused (resolvable by waiting or resuming) and 400 for error/unknown (actual problems). This enables client-side auto-resume orchestration per the task lifecycle RFC. Closes coder/internal#1265
1 parent c60c373 commit 2549fc7

2 files changed

Lines changed: 178 additions & 66 deletions

File tree

‎coderd/aitasks.go‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -977,10 +977,27 @@ func (api *API) authAndDoWithTaskAppClient(
977977
ctx := r.Context()
978978

979979
if task.Status != database.TaskStatusActive {
980-
return httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
981-
Message: "Task status must be active.",
982-
Detail: fmt.Sprintf("Task status is %q, it must be %q to interact with the task.", task.Status, codersdk.TaskStatusActive),
983-
})
980+
// Return 409 Conflict for valid requests blocked by current state
981+
// (pending/initializing are transitional, paused requires resume).
982+
// Return 400 Bad Request for error/unknown states.
983+
switch task.Status {
984+
case database.TaskStatusPending, database.TaskStatusInitializing:
985+
return httperror.NewResponseError(http.StatusConflict, codersdk.Response{
986+
Message: fmt.Sprintf("Task is %s.", task.Status),
987+
Detail: "The task is resuming. Wait for the task to become active before sending messages.",
988+
})
989+
case database.TaskStatusPaused:
990+
return httperror.NewResponseError(http.StatusConflict, codersdk.Response{
991+
Message: "Task is paused.",
992+
Detail: "Resume the task to send messages.",
993+
})
994+
default:
995+
// Default handler for error and unknown status.
996+
return httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
997+
Message: "Task must be active.",
998+
Detail: fmt.Sprintf("Task status is %q, it must be %q to interact with the task.", task.Status, codersdk.TaskStatusActive),
999+
})
1000+
}
9841001
}
9851002
if !task.WorkspaceID.Valid {
9861003
return httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{

‎coderd/aitasks_test.go‎

Lines changed: 157 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/coder/coder/v2/coderd/httpapi"
3131
"github.com/coder/coder/v2/coderd/notifications"
3232
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
33+
"github.com/coder/coder/v2/coderd/rbac"
3334
"github.com/coder/coder/v2/coderd/util/slice"
3435
"github.com/coder/coder/v2/codersdk"
3536
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -39,6 +40,66 @@ import (
3940
"github.com/coder/quartz"
4041
)
4142

43+
// createTaskInState is a helper to create a task in the desired state.
44+
// It returns a function that takes context, test, and status, and returns the task ID.
45+
// The caller is responsible for setting up the database, owner, and user.
46+
func createTaskInState(db database.Store, ownerSubject rbac.Subject, ownerOrgID, userID uuid.UUID) func(context.Context, *testing.T, database.TaskStatus) uuid.UUID {
47+
return func(ctx context.Context, t *testing.T, status database.TaskStatus) uuid.UUID {
48+
ctx = dbauthz.As(ctx, ownerSubject)
49+
50+
builder := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
51+
OrganizationID: ownerOrgID,
52+
OwnerID: userID,
53+
}).
54+
WithTask(database.TaskTable{
55+
OrganizationID: ownerOrgID,
56+
OwnerID: userID,
57+
}, nil)
58+
59+
switch status {
60+
case database.TaskStatusPending:
61+
builder = builder.Pending()
62+
case database.TaskStatusInitializing:
63+
builder = builder.Starting()
64+
case database.TaskStatusPaused:
65+
builder = builder.Seed(database.WorkspaceBuild{
66+
Transition: database.WorkspaceTransitionStop,
67+
})
68+
case database.TaskStatusError:
69+
// For error state, create a completed build then manipulate app health.
70+
default:
71+
require.Fail(t, "unsupported task status in test helper", "status: %s", status)
72+
}
73+
74+
resp := builder.Do()
75+
taskID := resp.Task.ID
76+
77+
// Post-process by manipulating agent and app state.
78+
if status == database.TaskStatusError {
79+
// First, set agent to ready state so agent_status returns 'active'.
80+
// This ensures the cascade reaches app_status.
81+
err := db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
82+
ID: resp.Agents[0].ID,
83+
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
84+
})
85+
require.NoError(t, err)
86+
87+
// Then set workspace app health to unhealthy to trigger error state.
88+
apps, err := db.GetWorkspaceAppsByAgentID(ctx, resp.Agents[0].ID)
89+
require.NoError(t, err)
90+
require.Len(t, apps, 1, "expected exactly one app for task")
91+
92+
err = db.UpdateWorkspaceAppHealthByID(ctx, database.UpdateWorkspaceAppHealthByIDParams{
93+
ID: apps[0].ID,
94+
Health: database.WorkspaceAppHealthUnhealthy,
95+
})
96+
require.NoError(t, err)
97+
}
98+
99+
return taskID
100+
}
101+
}
102+
42103
func TestTasks(t *testing.T) {
43104
t.Parallel()
44105

@@ -729,6 +790,94 @@ func TestTasks(t *testing.T) {
729790
require.ErrorAs(t, err, &sdkErr)
730791
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
731792
})
793+
794+
t.Run("SendToNonActiveStates", func(t *testing.T) {
795+
t.Parallel()
796+
797+
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
798+
owner := coderdtest.CreateFirstUser(t, client)
799+
ctx := testutil.Context(t, testutil.WaitMedium)
800+
801+
ownerUser, err := client.User(ctx, owner.UserID.String())
802+
require.NoError(t, err)
803+
ownerSubject := coderdtest.AuthzUserSubject(ownerUser)
804+
805+
// Create a regular user for task ownership.
806+
_, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
807+
808+
createTask := createTaskInState(db, ownerSubject, owner.OrganizationID, user.ID)
809+
810+
t.Run("Paused", func(t *testing.T) {
811+
t.Parallel()
812+
813+
ctx := testutil.Context(t, testutil.WaitMedium)
814+
taskID := createTask(ctx, t, database.TaskStatusPaused)
815+
816+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
817+
Input: "Hello",
818+
})
819+
820+
var sdkErr *codersdk.Error
821+
require.Error(t, err)
822+
require.ErrorAs(t, err, &sdkErr)
823+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
824+
require.Contains(t, sdkErr.Message, "paused")
825+
require.Contains(t, sdkErr.Detail, "Resume")
826+
})
827+
828+
t.Run("Initializing", func(t *testing.T) {
829+
t.Parallel()
830+
831+
ctx := testutil.Context(t, testutil.WaitMedium)
832+
taskID := createTask(ctx, t, database.TaskStatusInitializing)
833+
834+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
835+
Input: "Hello",
836+
})
837+
838+
var sdkErr *codersdk.Error
839+
require.Error(t, err)
840+
require.ErrorAs(t, err, &sdkErr)
841+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
842+
require.Contains(t, sdkErr.Message, "initializing")
843+
require.Contains(t, sdkErr.Detail, "resuming")
844+
})
845+
846+
t.Run("Pending", func(t *testing.T) {
847+
t.Parallel()
848+
849+
ctx := testutil.Context(t, testutil.WaitMedium)
850+
taskID := createTask(ctx, t, database.TaskStatusPending)
851+
852+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
853+
Input: "Hello",
854+
})
855+
856+
var sdkErr *codersdk.Error
857+
require.Error(t, err)
858+
require.ErrorAs(t, err, &sdkErr)
859+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
860+
require.Contains(t, sdkErr.Message, "pending")
861+
require.Contains(t, sdkErr.Detail, "resuming")
862+
})
863+
864+
t.Run("Error", func(t *testing.T) {
865+
t.Parallel()
866+
867+
ctx := testutil.Context(t, testutil.WaitMedium)
868+
taskID := createTask(ctx, t, database.TaskStatusError)
869+
870+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
871+
Input: "Hello",
872+
})
873+
874+
var sdkErr *codersdk.Error
875+
require.Error(t, err)
876+
require.ErrorAs(t, err, &sdkErr)
877+
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
878+
require.Contains(t, sdkErr.Message, "must be active")
879+
})
880+
})
732881
})
733882

734883
t.Run("Logs", func(t *testing.T) {
@@ -875,61 +1024,7 @@ func TestTasks(t *testing.T) {
8751024
// Create a regular user to test snapshot access.
8761025
client, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
8771026

878-
// Helper to create a task in the desired state.
879-
createTaskInState := func(ctx context.Context, t *testing.T, status database.TaskStatus) uuid.UUID {
880-
ctx = dbauthz.As(ctx, ownerSubject)
881-
882-
builder := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
883-
OrganizationID: owner.OrganizationID,
884-
OwnerID: user.ID,
885-
}).
886-
WithTask(database.TaskTable{
887-
OrganizationID: owner.OrganizationID,
888-
OwnerID: user.ID,
889-
}, nil)
890-
891-
switch status {
892-
case database.TaskStatusPending:
893-
builder = builder.Pending()
894-
case database.TaskStatusInitializing:
895-
builder = builder.Starting()
896-
case database.TaskStatusPaused:
897-
builder = builder.Seed(database.WorkspaceBuild{
898-
Transition: database.WorkspaceTransitionStop,
899-
})
900-
case database.TaskStatusError:
901-
// For error state, create a completed build then manipulate app health.
902-
default:
903-
require.Fail(t, "unsupported task status in test helper", "status: %s", status)
904-
}
905-
906-
resp := builder.Do()
907-
taskID := resp.Task.ID
908-
909-
// Post-process by manipulating agent and app state.
910-
if status == database.TaskStatusError {
911-
// First, set agent to ready state so agent_status returns 'active'.
912-
// This ensures the cascade reaches app_status.
913-
err := db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
914-
ID: resp.Agents[0].ID,
915-
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
916-
})
917-
require.NoError(t, err)
918-
919-
// Then set workspace app health to unhealthy to trigger error state.
920-
apps, err := db.GetWorkspaceAppsByAgentID(ctx, resp.Agents[0].ID)
921-
require.NoError(t, err)
922-
require.Len(t, apps, 1, "expected exactly one app for task")
923-
924-
err = db.UpdateWorkspaceAppHealthByID(ctx, database.UpdateWorkspaceAppHealthByIDParams{
925-
ID: apps[0].ID,
926-
Health: database.WorkspaceAppHealthUnhealthy,
927-
})
928-
require.NoError(t, err)
929-
}
930-
931-
return taskID
932-
}
1027+
createTask := createTaskInState(db, ownerSubject, owner.OrganizationID, user.ID)
9331028

9341029
// Prepare snapshot data used across tests.
9351030
snapshotMessages := []agentapisdk.Message{
@@ -991,7 +1086,7 @@ func TestTasks(t *testing.T) {
9911086
t.Parallel()
9921087

9931088
ctx := testutil.Context(t, testutil.WaitMedium)
994-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1089+
taskID := createTask(ctx, t, database.TaskStatusPending)
9951090

9961091
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
9971092
TaskID: taskID,
@@ -1009,7 +1104,7 @@ func TestTasks(t *testing.T) {
10091104
t.Parallel()
10101105

10111106
ctx := testutil.Context(t, testutil.WaitMedium)
1012-
taskID := createTaskInState(ctx, t, database.TaskStatusInitializing)
1107+
taskID := createTask(ctx, t, database.TaskStatusInitializing)
10131108

10141109
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
10151110
TaskID: taskID,
@@ -1027,7 +1122,7 @@ func TestTasks(t *testing.T) {
10271122
t.Parallel()
10281123

10291124
ctx := testutil.Context(t, testutil.WaitMedium)
1030-
taskID := createTaskInState(ctx, t, database.TaskStatusPaused)
1125+
taskID := createTask(ctx, t, database.TaskStatusPaused)
10311126

10321127
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
10331128
TaskID: taskID,
@@ -1045,7 +1140,7 @@ func TestTasks(t *testing.T) {
10451140
t.Parallel()
10461141

10471142
ctx := testutil.Context(t, testutil.WaitMedium)
1048-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1143+
taskID := createTask(ctx, t, database.TaskStatusPending)
10491144

10501145
logsResp, err := client.TaskLogs(ctx, "me", taskID)
10511146
require.NoError(t, err)
@@ -1059,7 +1154,7 @@ func TestTasks(t *testing.T) {
10591154
t.Parallel()
10601155

10611156
ctx := testutil.Context(t, testutil.WaitMedium)
1062-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1157+
taskID := createTask(ctx, t, database.TaskStatusPending)
10631158

10641159
invalidEnvelope := coderd.TaskLogSnapshotEnvelope{
10651160
Format: "unknown-format",
@@ -1088,7 +1183,7 @@ func TestTasks(t *testing.T) {
10881183
t.Parallel()
10891184

10901185
ctx := testutil.Context(t, testutil.WaitMedium)
1091-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1186+
taskID := createTask(ctx, t, database.TaskStatusPending)
10921187

10931188
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
10941189
TaskID: taskID,
@@ -1109,7 +1204,7 @@ func TestTasks(t *testing.T) {
11091204
t.Parallel()
11101205

11111206
ctx := testutil.Context(t, testutil.WaitMedium)
1112-
taskID := createTaskInState(ctx, t, database.TaskStatusError)
1207+
taskID := createTask(ctx, t, database.TaskStatusError)
11131208

11141209
_, err := client.TaskLogs(ctx, "me", taskID)
11151210
require.Error(t, err)

0 commit comments

Comments
 (0)