diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index c016c5f06c..b4f8633a65 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -3059,8 +3059,14 @@ func (api *API) chatStartWorkspace( ) if err != nil { if updatedToActiveVersion && isChatStartWorkspaceManualUpdateRequiredError(err) { + const retryInstructions = "The workspace needs the template's active version before it can start. Use read_template with this workspace's template_id to inspect the active version's required parameters, then retry start_workspace with a parameters object that supplies any missing or changed values. If the correct value for a parameter is not obvious from its description or defaults, ask the user rather than guessing." + if responder, ok := httperror.IsResponder(err); ok { + status, resp := responder.Response() + resp = rewriteChatStartWorkspaceManualUpdateResponse(resp, err.Error(), retryInstructions) + return codersdk.WorkspaceBuild{}, httperror.NewResponseError(status, resp) + } return codersdk.WorkspaceBuild{}, httperror.NewResponseError(http.StatusBadRequest, codersdk.Response{ - Message: "The workspace needs to be updated before it can start because the template requires the active version, and the newer version has parameter changes that must be chosen manually. Please update and start the workspace from the UI.", + Message: retryInstructions, Detail: err.Error(), }) } @@ -3070,6 +3076,21 @@ func (api *API) chatStartWorkspace( return apiBuild, nil } +func rewriteChatStartWorkspaceManualUpdateResponse(resp codersdk.Response, fallbackDetail string, retryInstructions string) codersdk.Response { + originalMessage := resp.Message + resp.Message = retryInstructions + if len(resp.Validations) == 0 && originalMessage != "" { + if resp.Detail == "" { + resp.Detail = originalMessage + } else { + resp.Detail = originalMessage + ": " + resp.Detail + } + } else if resp.Detail == "" { + resp.Detail = fallbackDetail + } + return resp +} + func isChatStartWorkspaceManualUpdateRequiredError(err error) bool { var diagnosticErr *dynamicparameters.DiagnosticError if errors.As(err, &diagnosticErr) { diff --git a/coderd/exp_chats_internal_test.go b/coderd/exp_chats_internal_test.go new file mode 100644 index 0000000000..17c93182e7 --- /dev/null +++ b/coderd/exp_chats_internal_test.go @@ -0,0 +1,76 @@ +package coderd + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/codersdk" +) + +func TestRewriteChatStartWorkspaceManualUpdateResponse(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + resp codersdk.Response + fallbackDetail string + wantDetail string + }{ + { + name: "NoValidationsAndEmptyDetail", + resp: codersdk.Response{ + Message: "missing required parameter", + }, + fallbackDetail: "wrapped missing required parameter", + wantDetail: "missing required parameter", + }, + { + name: "NoValidationsAndExistingDetail", + resp: codersdk.Response{ + Message: "missing required parameter", + Detail: "region must be set before the workspace can start", + }, + fallbackDetail: "wrapped missing required parameter", + wantDetail: "missing required parameter: region must be set before the workspace can start", + }, + { + name: "ValidationsAndEmptyDetail", + resp: codersdk.Response{ + Message: "missing required parameter", + Validations: []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, + }, + fallbackDetail: "wrapped missing required parameter", + wantDetail: "wrapped missing required parameter", + }, + { + name: "ValidationsAndExistingDetail", + resp: codersdk.Response{ + Message: "missing required parameter", + Detail: "region must be set before the workspace can start", + Validations: []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, + }, + fallbackDetail: "wrapped missing required parameter", + wantDetail: "region must be set before the workspace can start", + }, + } + + const retryInstructions = "Use read_template before retrying start_workspace." + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := rewriteChatStartWorkspaceManualUpdateResponse(tt.resp, tt.fallbackDetail, retryInstructions) + require.Equal(t, retryInstructions, got.Message) + require.Equal(t, tt.wantDetail, got.Detail) + require.Equal(t, tt.resp.Validations, got.Validations) + }) + } +} diff --git a/coderd/x/chatd/chattool/chattool.go b/coderd/x/chatd/chattool/chattool.go index 5a6a0b9872..b4eef8a50c 100644 --- a/coderd/x/chatd/chattool/chattool.go +++ b/coderd/x/chatd/chattool/chattool.go @@ -6,6 +6,8 @@ import ( "charm.land/fantasy" "github.com/google/uuid" + + "github.com/coder/coder/v2/codersdk" ) // toolResponse builds a fantasy.ToolResponse from a JSON-serializable @@ -30,6 +32,28 @@ func buildToolResponse(r buildErrorResult) fantasy.ToolResponse { return fantasy.NewTextResponse(string(data)) } +// responseErrorResult converts a codersdk.Response into a structured +// tool result. We return these via toolResponse rather than +// NewTextErrorResponse because the fantasy/chatprompt pipeline flattens +// IsError content into a single string and drops validation details. +func responseErrorResult(resp codersdk.Response) map[string]any { + message := resp.Message + if message == "" { + message = "request failed" + } + + result := map[string]any{ + "error": message, + } + if resp.Detail != "" { + result["detail"] = resp.Detail + } + if len(resp.Validations) > 0 { + result["validations"] = resp.Validations + } + return result +} + func truncateRunes(value string, maxLen int) string { if maxLen <= 0 || value == "" { return "" diff --git a/coderd/x/chatd/chattool/createworkspace.go b/coderd/x/chatd/chattool/createworkspace.go index be04be61e4..2d40608dc6 100644 --- a/coderd/x/chatd/chattool/createworkspace.go +++ b/coderd/x/chatd/chattool/createworkspace.go @@ -15,6 +15,7 @@ import ( "cdr.dev/slog/v3" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/httpapi/httperror" "github.com/coder/coder/v2/coderd/util/namesgenerator" "github.com/coder/coder/v2/coderd/x/chatd/internal/agentselect" "github.com/coder/coder/v2/codersdk" @@ -201,6 +202,10 @@ func CreateWorkspace(organizationID uuid.UUID, db database.Store, options Create workspace, err := options.CreateFn(ctx, ownerID, createReq) if err != nil { + if responseErr, ok := httperror.IsResponder(err); ok { + _, resp := responseErr.Response() + return toolResponse(responseErrorResult(resp)), nil + } return fantasy.NewTextErrorResponse(err.Error()), nil } diff --git a/coderd/x/chatd/chattool/createworkspace_test.go b/coderd/x/chatd/chattool/createworkspace_test.go index 94ee88d700..86f83d0e40 100644 --- a/coderd/x/chatd/chattool/createworkspace_test.go +++ b/coderd/x/chatd/chattool/createworkspace_test.go @@ -18,6 +18,7 @@ import ( "cdr.dev/slog/v3/sloggers/slogtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmock" + "github.com/coder/coder/v2/coderd/httpapi/httperror" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -411,6 +412,75 @@ func TestCreateWorkspace_PostCreationBuildFailure(t *testing.T) { "buildToolResponse must not set IsError; chatprompt strips structured fields from error responses") } +func TestCreateWorkspace_ResponderErrorPreservesStructuredFields(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + db := dbmock.NewMockStore(ctrl) + + ownerID := uuid.New() + orgID := uuid.New() + templateID := uuid.New() + + db.EXPECT(). + GetAuthorizationUserRoles(gomock.Any(), ownerID). + Return(database.GetAuthorizationUserRolesRow{ + ID: ownerID, + Roles: []string{}, + Groups: []string{}, + Status: database.UserStatusActive, + }, nil) + + db.EXPECT(). + GetTemplateByID(gomock.Any(), templateID). + Return(database.Template{ + ID: templateID, + OrganizationID: orgID, + }, nil) + + db.EXPECT(). + GetChatWorkspaceTTL(gomock.Any()). + Return("0s", nil) + + tool := CreateWorkspace(orgID, db, CreateWorkspaceOptions{ + OwnerID: ownerID, + CreateFn: func(context.Context, uuid.UUID, codersdk.CreateWorkspaceRequest) (codersdk.Workspace, error) { + return codersdk.Workspace{}, httperror.NewResponseError(400, codersdk.Response{ + Message: "missing required parameter", + Detail: "region must be set before the workspace can start", + Validations: []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, + }) + }, + WorkspaceMu: &sync.Mutex{}, + Logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}), + }) + + input := fmt.Sprintf(`{"template_id":%q,"name":"test-structured-error"}`, templateID.String()) + resp, err := tool.Run(context.Background(), fantasy.ToolCall{ + ID: "call-1", + Name: "create_workspace", + Input: input, + }) + require.NoError(t, err) + require.False(t, resp.IsError) + + var result struct { + Error string `json:"error"` + Detail string `json:"detail"` + Validations []codersdk.ValidationError `json:"validations"` + } + require.NoError(t, json.Unmarshal([]byte(resp.Content), &result)) + require.Equal(t, "missing required parameter", result.Error) + require.Equal(t, "region must be set before the workspace can start", result.Detail) + require.Equal(t, []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, result.Validations) +} + func TestCreateWorkspace_GlobalTTL(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/chattool/startworkspace.go b/coderd/x/chatd/chattool/startworkspace.go index 7acf3da124..473be1c273 100644 --- a/coderd/x/chatd/chattool/startworkspace.go +++ b/coderd/x/chatd/chattool/startworkspace.go @@ -36,6 +36,10 @@ type StartWorkspaceOptions struct { Logger slog.Logger } +type startWorkspaceArgs struct { + Parameters map[string]string `json:"parameters,omitempty"` +} + // StartWorkspace returns a tool that starts a stopped workspace // associated with the current chat. The tool is idempotent: if the // workspace is already running or building, it returns immediately. @@ -45,8 +49,10 @@ func StartWorkspace(options StartWorkspaceOptions) fantasy.AgentTool { "Start the chat's workspace if it is currently stopped. "+ "This tool is idempotent — if the workspace is already "+ "running, it returns immediately. Use create_workspace "+ - "first if no workspace exists yet.", - func(ctx context.Context, _ struct{}, _ fantasy.ToolCall) (fantasy.ToolResponse, error) { + "first if no workspace exists yet. Provide parameter "+ + "values (from read_template) only if necessary or "+ + "explicitly requested by the user.", + func(ctx context.Context, args startWorkspaceArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) { if options.StartFn == nil { return fantasy.NewTextErrorResponse("workspace starter is not configured"), nil } @@ -165,15 +171,24 @@ func StartWorkspace(options StartWorkspaceOptions) fantasy.AgentTool { return fantasy.NewTextErrorResponse(ownerErr.Error()), nil } - startBuild, err := options.StartFn(ownerCtx, options.OwnerID, ws.ID, codersdk.CreateWorkspaceBuildRequest{ + startReq := codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, - }) + } + for k, v := range args.Parameters { + startReq.RichParameterValues = append( + startReq.RichParameterValues, + codersdk.WorkspaceBuildParameter{Name: k, Value: v}, + ) + } + startBuild, err := options.StartFn(ownerCtx, options.OwnerID, ws.ID, startReq) if err != nil { if responseErr, ok := httperror.IsResponder(err); ok { _, resp := responseErr.Response() - if resp.Message != "" { - return fantasy.NewTextErrorResponse(resp.Message), nil + result := responseErrorResult(resp) + if len(resp.Validations) > 0 && ws.TemplateID != uuid.Nil { + result["template_id"] = ws.TemplateID.String() } + return toolResponse(result), nil } return fantasy.NewTextErrorResponse( xerrors.Errorf("start workspace: %w", err).Error(), diff --git a/coderd/x/chatd/chattool/startworkspace_test.go b/coderd/x/chatd/chattool/startworkspace_test.go index 57fb969ee8..4c073e82ec 100644 --- a/coderd/x/chatd/chattool/startworkspace_test.go +++ b/coderd/x/chatd/chattool/startworkspace_test.go @@ -373,6 +373,7 @@ func TestStartWorkspace(t *testing.T) { startCalled = true require.Equal(t, codersdk.WorkspaceTransitionStart, req.Transition) require.Equal(t, ws.ID, wsID) + require.Empty(t, req.RichParameterValues, "no parameters should be forwarded for bare start") // Simulate start by inserting a new completed "start" build. buildResp := dbfake.WorkspaceBuild(t, db, ws).Seed(database.WorkspaceBuild{ Transition: database.WorkspaceTransitionStart, @@ -473,6 +474,71 @@ func TestStartWorkspace(t *testing.T) { require.Contains(t, result["message"], "updated to the active template version") }) + t.Run("PassesParameters", 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, + }) + wsResp := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + OrganizationID: org.ID, + }).Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionStop, + }).Do() + ws := wsResp.Workspace + + chat, err := db.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: org.ID, + Status: database.ChatStatusWaiting, + OwnerID: user.ID, + WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true}, + LastModelConfigID: modelCfg.ID, + Title: "test-start-workspace-passes-parameters", + ClientType: database.ChatClientTypeUi, + }) + require.NoError(t, err) + + expectedParams := []codersdk.WorkspaceBuildParameter{ + {Name: "region", Value: "us-east-1"}, + {Name: "size", Value: "large"}, + } + startFn := func(_ context.Context, _ uuid.UUID, wsID uuid.UUID, req codersdk.CreateWorkspaceBuildRequest) (codersdk.WorkspaceBuild, error) { + require.Equal(t, codersdk.WorkspaceTransitionStart, req.Transition) + require.Equal(t, ws.ID, wsID) + require.ElementsMatch(t, expectedParams, req.RichParameterValues) + buildResp := dbfake.WorkspaceBuild(t, db, ws).Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionStart, + BuildNumber: 2, + }).Do() + return codersdk.WorkspaceBuild{ID: buildResp.Build.ID}, nil + } + + tool := chattool.StartWorkspace(chattool.StartWorkspaceOptions{ + DB: db, + OwnerID: user.ID, + ChatID: chat.ID, + StartFn: startFn, + AgentConnFn: func(_ context.Context, _ uuid.UUID) (workspacesdk.AgentConn, func(), error) { + return nil, func() {}, nil + }, + WorkspaceMu: &sync.Mutex{}, + }) + + resp, err := tool.Run(ctx, fantasy.ToolCall{ID: "call-1", Name: "start_workspace", Input: `{"parameters":{"region":"us-east-1","size":"large"}}`}) + require.NoError(t, err) + + var result map[string]any + require.NoError(t, json.Unmarshal([]byte(resp.Content), &result)) + require.Equal(t, true, result["started"]) + }) + t.Run("ManualUpdateRequired", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) @@ -509,17 +575,94 @@ func TestStartWorkspace(t *testing.T) { OwnerID: user.ID, ChatID: chat.ID, StartFn: func(_ context.Context, _ uuid.UUID, _ uuid.UUID, _ codersdk.CreateWorkspaceBuildRequest) (codersdk.WorkspaceBuild, error) { - return codersdk.WorkspaceBuild{}, httperror.NewResponseError(400, codersdk.Response{Message: "The workspace needs to be updated before it can start because the template requires the active version, and the newer version has parameter changes that must be chosen manually. Please update and start the workspace from the UI."}) + return codersdk.WorkspaceBuild{}, httperror.NewResponseError(400, codersdk.Response{ + Message: "The workspace needs the template's active version before it can start. Use read_template with this workspace's template_id to inspect the active version's required parameters, then retry start_workspace with a parameters object that supplies any missing or changed values.", + Detail: "region must be set before the workspace can start", + Validations: []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, + }) }, WorkspaceMu: &sync.Mutex{}, }) resp, err := tool.Run(ctx, fantasy.ToolCall{ID: "call-1", Name: "start_workspace", Input: "{}"}) require.NoError(t, err) - require.True(t, resp.IsError) - require.Contains(t, resp.Content, "template requires the active version") - require.Contains(t, resp.Content, "must be chosen manually") + require.False(t, resp.IsError) require.NotContains(t, resp.Content, "start workspace:") + + var result struct { + Error string `json:"error"` + Detail string `json:"detail"` + TemplateID string `json:"template_id"` + Validations []codersdk.ValidationError `json:"validations"` + } + require.NoError(t, json.Unmarshal([]byte(resp.Content), &result)) + require.Contains(t, result.Error, "read_template") + require.Contains(t, result.Error, "retry start_workspace") + require.Equal(t, ws.TemplateID.String(), result.TemplateID) + require.Equal(t, "region must be set before the workspace can start", result.Detail) + require.Equal(t, []codersdk.ValidationError{{ + Field: "region", + Detail: "region must be set before the workspace can start", + }}, result.Validations) + }) + + t.Run("ResponderErrorWithoutValidationsOmitsTemplateID", 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, + }) + wsResp := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + OrganizationID: org.ID, + }).Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionStop, + }).Do() + ws := wsResp.Workspace + + chat, err := db.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: org.ID, + Status: database.ChatStatusWaiting, + OwnerID: user.ID, + WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true}, + LastModelConfigID: modelCfg.ID, + Title: "test-start-workspace-responder-error-without-validations", + ClientType: database.ChatClientTypeUi, + }) + require.NoError(t, err) + + tool := chattool.StartWorkspace(chattool.StartWorkspaceOptions{ + DB: db, + OwnerID: user.ID, + ChatID: chat.ID, + StartFn: func(_ context.Context, _ uuid.UUID, _ uuid.UUID, _ codersdk.CreateWorkspaceBuildRequest) (codersdk.WorkspaceBuild, error) { + return codersdk.WorkspaceBuild{}, httperror.NewResponseError(502, codersdk.Response{ + Message: "workspace start failed", + Detail: "temporary provisioner outage", + }) + }, + WorkspaceMu: &sync.Mutex{}, + }) + + resp, err := tool.Run(ctx, fantasy.ToolCall{ID: "call-1", Name: "start_workspace", Input: "{}"}) + require.NoError(t, err) + require.False(t, resp.IsError) + + var result map[string]any + require.NoError(t, json.Unmarshal([]byte(resp.Content), &result)) + require.Equal(t, "workspace start failed", result["error"]) + require.Equal(t, "temporary provisioner outage", result["detail"]) + _, hasTemplateID := result["template_id"] + require.False(t, hasTemplateID) }) t.Run("InProgressBuild", func(t *testing.T) {