Skip to content

Commit 7bfdb51

Browse files
mafredrimafredri
committed
feat(coderd): return 409 Conflict for non-active task states
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 55e2f76 commit 7bfdb51

File tree

2 files changed

+177
-66
lines changed

2 files changed

+177
-66
lines changed

‎coderd/aitasks.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -977,10 +977,26 @@ 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: // error, unknown
995+
return httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{
996+
Message: "Task must be active.",
997+
Detail: fmt.Sprintf("Task status is %q, it must be %q to interact with the task.", task.Status, codersdk.TaskStatusActive),
998+
})
999+
}
9841000
}
9851001
if !task.WorkspaceID.Valid {
9861002
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

@@ -591,6 +652,94 @@ func TestTasks(t *testing.T) {
591652
require.ErrorAs(t, err, &sdkErr)
592653
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
593654
})
655+
656+
t.Run("SendToNonActiveStates", func(t *testing.T) {
657+
t.Parallel()
658+
659+
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
660+
owner := coderdtest.CreateFirstUser(t, client)
661+
ctx := testutil.Context(t, testutil.WaitMedium)
662+
663+
ownerUser, err := client.User(ctx, owner.UserID.String())
664+
require.NoError(t, err)
665+
ownerSubject := coderdtest.AuthzUserSubject(ownerUser)
666+
667+
// Create a regular user for task ownership.
668+
_, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
669+
670+
createTask := createTaskInState(db, ownerSubject, owner.OrganizationID, user.ID)
671+
672+
t.Run("Paused", func(t *testing.T) {
673+
t.Parallel()
674+
675+
ctx := testutil.Context(t, testutil.WaitMedium)
676+
taskID := createTask(ctx, t, database.TaskStatusPaused)
677+
678+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
679+
Input: "Hello",
680+
})
681+
682+
var sdkErr *codersdk.Error
683+
require.Error(t, err)
684+
require.ErrorAs(t, err, &sdkErr)
685+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
686+
require.Contains(t, sdkErr.Message, "paused")
687+
require.Contains(t, sdkErr.Detail, "Resume")
688+
})
689+
690+
t.Run("Initializing", func(t *testing.T) {
691+
t.Parallel()
692+
693+
ctx := testutil.Context(t, testutil.WaitMedium)
694+
taskID := createTask(ctx, t, database.TaskStatusInitializing)
695+
696+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
697+
Input: "Hello",
698+
})
699+
700+
var sdkErr *codersdk.Error
701+
require.Error(t, err)
702+
require.ErrorAs(t, err, &sdkErr)
703+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
704+
require.Contains(t, sdkErr.Message, "initializing")
705+
require.Contains(t, sdkErr.Detail, "resuming")
706+
})
707+
708+
t.Run("Pending", func(t *testing.T) {
709+
t.Parallel()
710+
711+
ctx := testutil.Context(t, testutil.WaitMedium)
712+
taskID := createTask(ctx, t, database.TaskStatusPending)
713+
714+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
715+
Input: "Hello",
716+
})
717+
718+
var sdkErr *codersdk.Error
719+
require.Error(t, err)
720+
require.ErrorAs(t, err, &sdkErr)
721+
require.Equal(t, http.StatusConflict, sdkErr.StatusCode())
722+
require.Contains(t, sdkErr.Message, "pending")
723+
require.Contains(t, sdkErr.Detail, "resuming")
724+
})
725+
726+
t.Run("Error", func(t *testing.T) {
727+
t.Parallel()
728+
729+
ctx := testutil.Context(t, testutil.WaitMedium)
730+
taskID := createTask(ctx, t, database.TaskStatusError)
731+
732+
err := client.TaskSend(ctx, "me", taskID, codersdk.TaskSendRequest{
733+
Input: "Hello",
734+
})
735+
736+
var sdkErr *codersdk.Error
737+
require.Error(t, err)
738+
require.ErrorAs(t, err, &sdkErr)
739+
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
740+
require.Contains(t, sdkErr.Message, "must be active")
741+
})
742+
})
594743
})
595744

596745
t.Run("Logs", func(t *testing.T) {
@@ -737,61 +886,7 @@ func TestTasks(t *testing.T) {
737886
// Create a regular user to test snapshot access.
738887
client, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
739888

740-
// Helper to create a task in the desired state.
741-
createTaskInState := func(ctx context.Context, t *testing.T, status database.TaskStatus) uuid.UUID {
742-
ctx = dbauthz.As(ctx, ownerSubject)
743-
744-
builder := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
745-
OrganizationID: owner.OrganizationID,
746-
OwnerID: user.ID,
747-
}).
748-
WithTask(database.TaskTable{
749-
OrganizationID: owner.OrganizationID,
750-
OwnerID: user.ID,
751-
}, nil)
752-
753-
switch status {
754-
case database.TaskStatusPending:
755-
builder = builder.Pending()
756-
case database.TaskStatusInitializing:
757-
builder = builder.Starting()
758-
case database.TaskStatusPaused:
759-
builder = builder.Seed(database.WorkspaceBuild{
760-
Transition: database.WorkspaceTransitionStop,
761-
})
762-
case database.TaskStatusError:
763-
// For error state, create a completed build then manipulate app health.
764-
default:
765-
require.Fail(t, "unsupported task status in test helper", "status: %s", status)
766-
}
767-
768-
resp := builder.Do()
769-
taskID := resp.Task.ID
770-
771-
// Post-process by manipulating agent and app state.
772-
if status == database.TaskStatusError {
773-
// First, set agent to ready state so agent_status returns 'active'.
774-
// This ensures the cascade reaches app_status.
775-
err := db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
776-
ID: resp.Agents[0].ID,
777-
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
778-
})
779-
require.NoError(t, err)
780-
781-
// Then set workspace app health to unhealthy to trigger error state.
782-
apps, err := db.GetWorkspaceAppsByAgentID(ctx, resp.Agents[0].ID)
783-
require.NoError(t, err)
784-
require.Len(t, apps, 1, "expected exactly one app for task")
785-
786-
err = db.UpdateWorkspaceAppHealthByID(ctx, database.UpdateWorkspaceAppHealthByIDParams{
787-
ID: apps[0].ID,
788-
Health: database.WorkspaceAppHealthUnhealthy,
789-
})
790-
require.NoError(t, err)
791-
}
792-
793-
return taskID
794-
}
889+
createTask := createTaskInState(db, ownerSubject, owner.OrganizationID, user.ID)
795890

796891
// Prepare snapshot data used across tests.
797892
snapshotMessages := []agentapisdk.Message{
@@ -853,7 +948,7 @@ func TestTasks(t *testing.T) {
853948
t.Parallel()
854949

855950
ctx := testutil.Context(t, testutil.WaitMedium)
856-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
951+
taskID := createTask(ctx, t, database.TaskStatusPending)
857952

858953
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
859954
TaskID: taskID,
@@ -871,7 +966,7 @@ func TestTasks(t *testing.T) {
871966
t.Parallel()
872967

873968
ctx := testutil.Context(t, testutil.WaitMedium)
874-
taskID := createTaskInState(ctx, t, database.TaskStatusInitializing)
969+
taskID := createTask(ctx, t, database.TaskStatusInitializing)
875970

876971
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
877972
TaskID: taskID,
@@ -889,7 +984,7 @@ func TestTasks(t *testing.T) {
889984
t.Parallel()
890985

891986
ctx := testutil.Context(t, testutil.WaitMedium)
892-
taskID := createTaskInState(ctx, t, database.TaskStatusPaused)
987+
taskID := createTask(ctx, t, database.TaskStatusPaused)
893988

894989
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
895990
TaskID: taskID,
@@ -907,7 +1002,7 @@ func TestTasks(t *testing.T) {
9071002
t.Parallel()
9081003

9091004
ctx := testutil.Context(t, testutil.WaitMedium)
910-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1005+
taskID := createTask(ctx, t, database.TaskStatusPending)
9111006

9121007
logsResp, err := client.TaskLogs(ctx, "me", taskID)
9131008
require.NoError(t, err)
@@ -921,7 +1016,7 @@ func TestTasks(t *testing.T) {
9211016
t.Parallel()
9221017

9231018
ctx := testutil.Context(t, testutil.WaitMedium)
924-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1019+
taskID := createTask(ctx, t, database.TaskStatusPending)
9251020

9261021
invalidEnvelope := coderd.TaskLogSnapshotEnvelope{
9271022
Format: "unknown-format",
@@ -950,7 +1045,7 @@ func TestTasks(t *testing.T) {
9501045
t.Parallel()
9511046

9521047
ctx := testutil.Context(t, testutil.WaitMedium)
953-
taskID := createTaskInState(ctx, t, database.TaskStatusPending)
1048+
taskID := createTask(ctx, t, database.TaskStatusPending)
9541049

9551050
err := db.UpsertTaskSnapshot(dbauthz.As(ctx, ownerSubject), database.UpsertTaskSnapshotParams{
9561051
TaskID: taskID,
@@ -971,7 +1066,7 @@ func TestTasks(t *testing.T) {
9711066
t.Parallel()
9721067

9731068
ctx := testutil.Context(t, testutil.WaitMedium)
974-
taskID := createTaskInState(ctx, t, database.TaskStatusError)
1069+
taskID := createTask(ctx, t, database.TaskStatusError)
9751070

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

0 commit comments

Comments
 (0)