From 9de4930b1dbda1f3e75341fbfc5391f25302bfd2 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 22 Apr 2026 03:41:25 +0000 Subject: [PATCH 1/8] fix: persist structured chat errors --- coderd/database/db2sdk/db2sdk.go | 31 +++++- coderd/database/db2sdk/db2sdk_test.go | 57 ++++++++++- coderd/database/dump.sql | 2 +- .../000474_chat_last_error_jsonb.down.sql | 3 + .../000474_chat_last_error_jsonb.up.sql | 9 ++ .../fixtures/000424_chat_last_error.up.sql | 6 ++ .../000474_chat_last_error_jsonb.up.sql | 20 ++++ coderd/database/models.go | 2 +- coderd/database/queries.sql.go | 32 +++---- coderd/database/queries/chats.sql | 4 +- coderd/exp_chats.go | 2 +- coderd/exp_chats_test.go | 21 ++-- coderd/x/chatd/chatd.go | 80 +++++++++++----- coderd/x/chatd/chatd_internal_test.go | 6 +- coderd/x/chatd/chatd_test.go | 96 +++++++++++++------ coderd/x/chatd/chaterror/classify_test.go | 14 +-- coderd/x/chatd/chaterror/message.go | 34 ++----- coderd/x/chatd/chaterror/payload.go | 14 +++ coderd/x/chatd/chaterror/payload_test.go | 22 ++++- coderd/x/chatd/chatloop/chatloop_test.go | 2 +- coderd/x/chatd/subagent_internal_test.go | 8 +- codersdk/chats.go | 67 +++++++++---- codersdk/chats_test.go | 25 +++++ enterprise/coderd/x/chatd/chatd_test.go | 16 +++- site/src/api/typesGenerated.ts | 44 +++++++++ .../AgentsPage/AgentChatPage.stories.tsx | 36 +++++++ site/src/pages/AgentsPage/AgentChatPage.tsx | 13 +++ .../AgentsPage/AgentChatPageView.stories.tsx | 5 +- .../ChatConversation/ChatStatusCallout.tsx | 3 + .../LiveStreamTail.stories.tsx | 10 +- 30 files changed, 525 insertions(+), 159 deletions(-) create mode 100644 coderd/database/migrations/000474_chat_last_error_jsonb.down.sql create mode 100644 coderd/database/migrations/000474_chat_last_error_jsonb.up.sql create mode 100644 coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql create mode 100644 coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 77b6bd0867b4e..b9374f2ff3df2 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -1607,6 +1607,31 @@ func nullTimePtr(v sql.NullTime) *time.Time { return &value } +const fallbackChatLastErrorMessage = "The chat request failed unexpectedly." + +func decodeChatLastError(raw pqtype.NullRawMessage) (*codersdk.ChatLastError, *string) { + if !raw.Valid { + return nil, nil + } + + var payload codersdk.ChatLastError + if err := json.Unmarshal(raw.RawMessage, &payload); err != nil { + return nil, ptr.Ref(fallbackChatLastErrorMessage) + } + + payload.Message = strings.TrimSpace(payload.Message) + payload.Detail = strings.TrimSpace(payload.Detail) + payload.Kind = strings.TrimSpace(payload.Kind) + payload.Provider = strings.TrimSpace(payload.Provider) + if payload.Message == "" { + return nil, ptr.Ref(fallbackChatLastErrorMessage) + } + if payload.Kind == "" { + payload.Kind = "generic" + } + return &payload, ptr.Ref(payload.Message) +} + // Chat converts a database.Chat to a codersdk.Chat. It coalesces // nil slices and maps to empty values for JSON serialization and // derives RootChatID from the parent chain when not explicitly set. @@ -1622,6 +1647,7 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database if labels == nil { labels = map[string]string{} } + lastErrorPayload, lastError := decodeChatLastError(c.LastError) chat := codersdk.Chat{ ID: c.ID, OrganizationID: c.OrganizationID, @@ -1636,9 +1662,8 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database MCPServerIDs: mcpServerIDs, Labels: labels, ClientType: codersdk.ChatClientType(c.ClientType), - } - if c.LastError.Valid { - chat.LastError = &c.LastError.String + LastError: lastError, + LastErrorPayload: lastErrorPayload, } if c.PlanMode.Valid { chat.PlanMode = codersdk.ChatPlanMode(c.PlanMode.ChatPlanMode) diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index acdb43b03e3e9..1c2426cc78af2 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -916,6 +916,13 @@ func TestChat_AllFieldsPopulated(t *testing.T) { // field to codersdk.Chat, this test will fail until the // converter is updated. now := dbtime.Now() + lastErrorPayload := codersdk.ChatLastError{ + Message: "boom", + Kind: "generic", + } + lastErrorRaw, err := json.Marshal(lastErrorPayload) + require.NoError(t, err) + input := database.Chat{ ID: uuid.New(), OwnerID: uuid.New(), @@ -929,7 +936,7 @@ func TestChat_AllFieldsPopulated(t *testing.T) { Title: "all-fields-test", Status: database.ChatStatusRunning, ClientType: database.ChatClientTypeUi, - LastError: sql.NullString{String: "boom", Valid: true}, + LastError: pqtype.NullRawMessage{RawMessage: lastErrorRaw, Valid: true}, CreatedAt: now, UpdatedAt: now, Archived: true, @@ -970,6 +977,10 @@ func TestChat_AllFieldsPopulated(t *testing.T) { got := db2sdk.Chat(input, diffStatus, fileRows) + require.NotNil(t, got.LastError) + require.Equal(t, lastErrorPayload.Message, *got.LastError) + require.Equal(t, &lastErrorPayload, got.LastErrorPayload) + v := reflect.ValueOf(got) typ := v.Type() // HasUnread is populated by ChatRowsWithChildren (which joins the @@ -1053,6 +1064,50 @@ func TestChat_NilFilesOmitted(t *testing.T) { require.Empty(t, result.Files) } +func TestChat_LastErrorPayloadFallback(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw json.RawMessage + }{ + { + name: "MessageMissing", + raw: json.RawMessage(`{"kind":"generic"}`), + }, + { + name: "MalformedJSON", + raw: json.RawMessage(`{`), + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + chat := database.Chat{ + ID: uuid.New(), + OwnerID: uuid.New(), + LastModelConfigID: uuid.New(), + Title: "fallback payload", + Status: database.ChatStatusError, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + LastError: pqtype.NullRawMessage{ + RawMessage: tc.raw, + Valid: true, + }, + } + + result := db2sdk.Chat(chat, nil, nil) + require.NotNil(t, result.LastError) + require.Equal(t, "The chat request failed unexpectedly.", *result.LastError) + require.Nil(t, result.LastErrorPayload) + }) + } +} + func TestChat_MultipleFiles(t *testing.T) { t.Parallel() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 99109c94868ca..8b5162ba43255 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1438,7 +1438,7 @@ CREATE TABLE chats ( root_chat_id uuid, last_model_config_id uuid NOT NULL, archived boolean DEFAULT false NOT NULL, - last_error text, + last_error jsonb, mode chat_mode, mcp_server_ids uuid[] DEFAULT '{}'::uuid[] NOT NULL, labels jsonb DEFAULT '{}'::jsonb NOT NULL, diff --git a/coderd/database/migrations/000474_chat_last_error_jsonb.down.sql b/coderd/database/migrations/000474_chat_last_error_jsonb.down.sql new file mode 100644 index 0000000000000..f3a565a331b77 --- /dev/null +++ b/coderd/database/migrations/000474_chat_last_error_jsonb.down.sql @@ -0,0 +1,3 @@ +ALTER TABLE chats + ALTER COLUMN last_error TYPE text + USING last_error ->> 'message'; diff --git a/coderd/database/migrations/000474_chat_last_error_jsonb.up.sql b/coderd/database/migrations/000474_chat_last_error_jsonb.up.sql new file mode 100644 index 0000000000000..7ab895c8b7174 --- /dev/null +++ b/coderd/database/migrations/000474_chat_last_error_jsonb.up.sql @@ -0,0 +1,9 @@ +ALTER TABLE chats + ALTER COLUMN last_error TYPE jsonb + USING CASE + WHEN last_error IS NULL THEN NULL + ELSE jsonb_build_object( + 'message', last_error, + 'kind', 'generic' + ) + END; diff --git a/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql new file mode 100644 index 0000000000000..6237891a44cf5 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql @@ -0,0 +1,6 @@ +-- Migration 424 adds chats.last_error as text. Seed the existing fixture +-- chat with a legacy plain-text error so migration 474 has a non-null row +-- to backfill. +UPDATE chats +SET last_error = 'Legacy provider failure' +WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96'; diff --git a/coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql b/coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql new file mode 100644 index 0000000000000..d757432932d3d --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql @@ -0,0 +1,20 @@ +-- Migration 474 retypes chats.last_error to jsonb and backfills legacy +-- text rows into the structured persisted payload shape. +DO $$ +DECLARE + payload jsonb; +BEGIN + SELECT last_error INTO STRICT payload + FROM chats + WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96'; + + IF payload ->> 'message' <> 'Legacy provider failure' THEN + RAISE EXCEPTION 'expected migrated last_error message, got %', + payload ->> 'message'; + END IF; + + IF payload ->> 'kind' <> 'generic' THEN + RAISE EXCEPTION 'expected migrated last_error kind, got %', + payload ->> 'kind'; + END IF; +END $$; diff --git a/coderd/database/models.go b/coderd/database/models.go index 65d3cfb2b4c26..65e6d5a1420eb 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -4367,7 +4367,7 @@ type Chat struct { RootChatID uuid.NullUUID `db:"root_chat_id" json:"root_chat_id"` LastModelConfigID uuid.UUID `db:"last_model_config_id" json:"last_model_config_id"` Archived bool `db:"archived" json:"archived"` - LastError sql.NullString `db:"last_error" json:"last_error"` + LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"` Mode NullChatMode `db:"mode" json:"mode"` MCPServerIDs []uuid.UUID `db:"mcp_server_ids" json:"mcp_server_ids"` Labels StringMap `db:"labels" json:"labels"` diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d4f2feb71a695..120976f3c86f2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5400,7 +5400,7 @@ type AutoArchiveInactiveChatsRow struct { RootChatID uuid.NullUUID `db:"root_chat_id" json:"root_chat_id"` LastModelConfigID uuid.UUID `db:"last_model_config_id" json:"last_model_config_id"` Archived bool `db:"archived" json:"archived"` - LastError sql.NullString `db:"last_error" json:"last_error"` + LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"` Mode NullChatMode `db:"mode" json:"mode"` MCPServerIDs []uuid.UUID `db:"mcp_server_ids" json:"mcp_server_ids"` Labels json.RawMessage `db:"labels" json:"labels"` @@ -8701,7 +8701,7 @@ SET worker_id = $2::uuid, started_at = $3::timestamptz, heartbeat_at = $4::timestamptz, - last_error = $5::text, + last_error = $5::jsonb, updated_at = NOW() WHERE id = $6::uuid @@ -8710,12 +8710,12 @@ RETURNING ` type UpdateChatStatusParams struct { - Status ChatStatus `db:"status" json:"status"` - WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` - StartedAt sql.NullTime `db:"started_at" json:"started_at"` - HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"` - LastError sql.NullString `db:"last_error" json:"last_error"` - ID uuid.UUID `db:"id" json:"id"` + Status ChatStatus `db:"status" json:"status"` + WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` + StartedAt sql.NullTime `db:"started_at" json:"started_at"` + HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"` + LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"` + ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateChatStatus(ctx context.Context, arg UpdateChatStatusParams) (Chat, error) { @@ -8768,7 +8768,7 @@ SET worker_id = $2::uuid, started_at = $3::timestamptz, heartbeat_at = $4::timestamptz, - last_error = $5::text, + last_error = $5::jsonb, updated_at = $6::timestamptz WHERE id = $7::uuid @@ -8777,13 +8777,13 @@ RETURNING ` type UpdateChatStatusPreserveUpdatedAtParams struct { - Status ChatStatus `db:"status" json:"status"` - WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` - StartedAt sql.NullTime `db:"started_at" json:"started_at"` - HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"` - LastError sql.NullString `db:"last_error" json:"last_error"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - ID uuid.UUID `db:"id" json:"id"` + Status ChatStatus `db:"status" json:"status"` + WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` + StartedAt sql.NullTime `db:"started_at" json:"started_at"` + HeartbeatAt sql.NullTime `db:"heartbeat_at" json:"heartbeat_at"` + LastError pqtype.NullRawMessage `db:"last_error" json:"last_error"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateChatStatusPreserveUpdatedAt(ctx context.Context, arg UpdateChatStatusPreserveUpdatedAtParams) (Chat, error) { diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index 8efde5ae7acae..4f3e6935ada5e 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -718,7 +718,7 @@ SET worker_id = sqlc.narg('worker_id')::uuid, started_at = sqlc.narg('started_at')::timestamptz, heartbeat_at = sqlc.narg('heartbeat_at')::timestamptz, - last_error = sqlc.narg('last_error')::text, + last_error = sqlc.narg('last_error')::jsonb, updated_at = NOW() WHERE id = @id::uuid @@ -733,7 +733,7 @@ SET worker_id = sqlc.narg('worker_id')::uuid, started_at = sqlc.narg('started_at')::timestamptz, heartbeat_at = sqlc.narg('heartbeat_at')::timestamptz, - last_error = sqlc.narg('last_error')::text, + last_error = sqlc.narg('last_error')::jsonb, updated_at = @updated_at::timestamptz WHERE id = @id::uuid diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 88b93338a95b0..2069081bd1420 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -3252,7 +3252,7 @@ func (api *API) interruptChat(rw http.ResponseWriter, r *http.Request) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) if updateErr != nil { logger.Error(ctx, "failed to mark chat as waiting", slog.Error(updateErr)) diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index 68ea5f58abdad..ee9b878fbda0d 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -4767,6 +4767,7 @@ func TestPatchChat(t *testing.T) { _ = createChatModelConfig(t, client) chat := createChat(ctx, t, client, firstUser.OrganizationID, "boundary baseline") + waitChatSettled(ctx, t, client, chat.ID) err := client.UpdateChat(ctx, chat.ID, codersdk.UpdateChatRequest{ @@ -5882,7 +5883,7 @@ func TestSendMessageQueuesEffectiveModelConfigID(t *testing.T) { WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, HeartbeatAt: sql.NullTime{Time: time.Now(), Valid: true}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -5933,7 +5934,7 @@ func TestQueuedMessageWithoutOverrideCapturesEnqueueTimeModel(t *testing.T) { WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, HeartbeatAt: sql.NullTime{Time: time.Now(), Valid: true}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -6059,7 +6060,7 @@ func TestWatchChatsStatusChangeCarriesUpdatedLastModelConfigID(t *testing.T) { WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, HeartbeatAt: sql.NullTime{Time: time.Now(), Valid: true}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -6081,7 +6082,7 @@ func TestWatchChatsStatusChangeCarriesUpdatedLastModelConfigID(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -7564,7 +7565,7 @@ func TestRegenerateChatTitle(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -7604,8 +7605,7 @@ func TestRegenerateChatTitle(t *testing.T) { WorkerID: uuid.NullUUID{UUID: uuid.MustParse("00000000-0000-0000-0000-000000000001"), Valid: true}, StartedAt: sql.NullTime{Time: time.Now(), Valid: true}, HeartbeatAt: sql.NullTime{Time: time.Now(), Valid: true}, - - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -7646,8 +7646,7 @@ func TestRegenerateChatTitle(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -7711,7 +7710,7 @@ func TestRegenerateChatTitle(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -8237,7 +8236,7 @@ func TestPromoteChatQueuedMessage(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index e3788f265cbf2..5d2e54fe16e89 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -1761,7 +1761,7 @@ func (p *Server) EditMessage( WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) if err != nil { return xerrors.Errorf("set chat pending: %w", err) @@ -1849,7 +1849,7 @@ func (p *Server) ArchiveChat(ctx context.Context, chat database.Chat) error { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) if updateErr != nil { return xerrors.Errorf("set archived chat waiting before cleanup: %w", updateErr) @@ -2365,7 +2365,7 @@ func (p *Server) SubmitToolResults( WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }); updateErr != nil { return xerrors.Errorf("update chat status: %w", updateErr) } @@ -3369,7 +3369,7 @@ func (p *Server) setChatWaiting(ctx context.Context, chatID uuid.UUID) (database WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) return updateErr }, nil) @@ -3569,7 +3569,7 @@ func insertUserMessageAndSetPending( WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) if err != nil { return database.ChatMessage{}, database.Chat{}, xerrors.Errorf("set chat pending: %w", err) @@ -3846,7 +3846,7 @@ func (p *Server) processOnce(ctx context.Context) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) if updateErr != nil { p.logger.Error(ctx, "failed to release chat acquired during shutdown", @@ -4882,6 +4882,17 @@ func processingFailure(err error) (chaterror.ClassifiedError, bool) { return classified, true } +func encodeChatLastErrorPayload(payload *codersdk.ChatLastError) (pqtype.NullRawMessage, error) { + if payload == nil { + return pqtype.NullRawMessage{}, nil + } + encoded, err := json.Marshal(payload) + if err != nil { + return pqtype.NullRawMessage{}, err + } + return pqtype.NullRawMessage{RawMessage: encoded, Valid: true}, nil +} + func panicFailureReason(recovered any) string { var reason string switch typed := recovered.(type) { @@ -5156,7 +5167,7 @@ func (p *Server) finishActiveChat( logger slog.Logger, chat database.Chat, status database.ChatStatus, - lastError string, + lastError pqtype.NullRawMessage, ) (finishActiveChatResult, error) { result := finishActiveChatResult{} @@ -5204,7 +5215,7 @@ func (p *Server) finishActiveChat( WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{String: lastError, Valid: lastError != ""}, + LastError: lastError, }) return updateErr }, nil) @@ -5330,10 +5341,10 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { // interrupt processing. close(controlArmed) - // Determine the final status and last error to set when we're done. + // Determine the final status and last error payload to set when we're done. status := database.ChatStatusWaiting wasInterrupted := false - lastError := "" + var lastErrorPayload *codersdk.ChatLastError generatedTitle := &generatedChatTitle{} runResult := runChatResult{} remainingQueuedMessages := []database.ChatQueuedMessage{} @@ -5349,20 +5360,30 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { // Handle panics gracefully. if r := recover(); r != nil { logger.Error(cleanupCtx, "panic during chat processing", slog.F("panic", r)) - lastError = panicFailureReason(r) - p.publishError(chat.ID, chaterror.ClassifiedError{ - Message: lastError, + classified := chaterror.ClassifiedError{ + Message: panicFailureReason(r), Kind: chaterror.KindGeneric, - }) + } + lastErrorPayload = chaterror.LastErrorPayload(classified) + p.publishError(chat.ID, classified) status = database.ChatStatusError } + encodedLastError, err := encodeChatLastErrorPayload(lastErrorPayload) + if err != nil { + logger.Warn(cleanupCtx, "failed to marshal chat last error payload", + slog.Error(err), + ) + lastErrorPayload = nil + encodedLastError = pqtype.NullRawMessage{} + } + // Check for queued messages and auto-promote the next one. // This must be done atomically with the status update to avoid // races with the promote endpoint (which also sets status to // pending). We use a transaction with FOR UPDATE to ensure we // don't overwrite a status change made by another caller. - finishResult, err := p.finishActiveChat(cleanupCtx, logger, chat, status, lastError) + finishResult, err := p.finishActiveChat(cleanupCtx, logger, chat, status, encodedLastError) if errors.Is(err, errChatTakenByOtherWorker) { // Another worker owns this chat now — skip all // post-TX side effects (status publish, pubsub, @@ -5425,7 +5446,11 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { p.publishChatActionRequired(finishResult.updatedChat, runResult.PendingDynamicToolCalls) } if !wasInterrupted { - p.maybeSendPushNotification(cleanupCtx, finishResult.updatedChat, status, lastError, runResult, logger) + lastErrorMessage := "" + if lastErrorPayload != nil { + lastErrorMessage = lastErrorPayload.Message + } + p.maybeSendPushNotification(cleanupCtx, finishResult.updatedChat, status, lastErrorMessage, runResult, logger) } }() @@ -5440,18 +5465,19 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { if errors.Is(err, chatloop.ErrInterrupted) || errors.Is(context.Cause(chatCtx), chatloop.ErrInterrupted) { logger.Info(ctx, "chat interrupted") status = database.ChatStatusWaiting + lastErrorPayload = nil wasInterrupted = true return } if isShutdownCancellation(ctx, chatCtx, err) { logger.Info(ctx, "chat canceled during shutdown; returning to pending") status = database.ChatStatusPending - lastError = "" + lastErrorPayload = nil return } logger.Error(ctx, "failed to process chat", slog.Error(err)) if classified, ok := processingFailure(err); ok { - lastError = classified.Message + lastErrorPayload = chaterror.LastErrorPayload(classified) p.publishError(chat.ID, classified) } status = database.ChatStatusError @@ -5476,7 +5502,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { if ctx.Err() != nil { logger.Info(ctx, "chat completed during shutdown; returning to pending") status = database.ChatStatusPending - lastError = "" + lastErrorPayload = nil return } } @@ -7983,11 +8009,19 @@ func (p *Server) recoverStaleChats(ctx context.Context) { return nil } - lastError := sql.NullString{} + lastError := pqtype.NullRawMessage{} if locked.Status == database.ChatStatusRequiresAction { - lastError = sql.NullString{ - String: "Dynamic tool execution timed out", - Valid: true, + lastErrorPayload, marshalErr := encodeChatLastErrorPayload(&codersdk.ChatLastError{ + Message: "Dynamic tool execution timed out", + Kind: chaterror.KindGeneric, + }) + if marshalErr != nil { + p.logger.Warn(ctx, "failed to marshal stale recovery last error payload", + slog.F("chat_id", chat.ID), + slog.Error(marshalErr), + ) + } else { + lastError = lastErrorPayload } } diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index 5a351f337194d..5d43b3350ea59 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -2322,7 +2322,7 @@ func TestSubscribeDoesNotReplayRetryAfterTerminalError(t *testing.T) { server.publishRetry(chatID, newTestRetryPayload()) server.publishError(chatID, chaterror.ClassifiedError{ - Message: "OpenAI is rate limiting requests (HTTP 429).", + Message: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", Retryable: true, @@ -2398,7 +2398,7 @@ func TestSubscribePrefersStructuredErrorPayloadViaPubsub(t *testing.T) { defer cancel() classified := chaterror.ClassifiedError{ - Message: "OpenAI is rate limiting requests (HTTP 429).", + Message: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", Retryable: true, @@ -2450,7 +2450,7 @@ func newTestRetryPayload() *codersdk.ChatStreamRetry { return &codersdk.ChatStreamRetry{ Attempt: 1, DelayMs: (1500 * time.Millisecond).Milliseconds(), - Error: "OpenAI is rate limiting requests (HTTP 429).", + Error: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", StatusCode: 429, diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 46e99fbf8915f..fba8ce137adc7 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -70,6 +70,35 @@ func openAIToolName(tool chattest.OpenAITool) string { return cmp.Or(tool.Function.Name, tool.Name, tool.Type) } +func mustChatLastErrorRawMessage(t testing.TB, payload codersdk.ChatLastError) pqtype.NullRawMessage { + t.Helper() + + encoded, err := json.Marshal(payload) + require.NoError(t, err) + return pqtype.NullRawMessage{RawMessage: encoded, Valid: true} +} + +func requireChatLastErrorPayload(t testing.TB, raw pqtype.NullRawMessage) codersdk.ChatLastError { + t.Helper() + require.True(t, raw.Valid, "last error should be set") + + var payload codersdk.ChatLastError + require.NoError(t, json.Unmarshal(raw.RawMessage, &payload)) + return payload +} + +func chatLastErrorMessage(raw pqtype.NullRawMessage) string { + if !raw.Valid { + return "" + } + + var payload codersdk.ChatLastError + if err := json.Unmarshal(raw.RawMessage, &payload); err == nil && payload.Message != "" { + return payload.Message + } + return string(raw.RawMessage) +} + func recordOpenAIRequest(req *chattest.OpenAIRequest) recordedOpenAIRequest { messages := append([]chattest.OpenAIMessage(nil), req.Messages...) tools := make([]string, 0, len(req.Tools)) @@ -1481,7 +1510,7 @@ func TestArchiveChatMovesPendingChatToWaiting(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -1953,7 +1982,7 @@ func TestSendMessageQueuesWhenWaitingWithQueuedBacklog(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -2418,7 +2447,7 @@ func TestPromoteQueuedAllowsAlreadyQueuedMessageWhenUsageLimitReached(t *testing WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -3657,7 +3686,11 @@ func TestRecoverStaleRequiresActionChat(t *testing.T) { return chatResult.Status == database.ChatStatusError }, testutil.WaitMedium, testutil.IntervalFast) - require.Contains(t, chatResult.LastError.String, "Dynamic tool execution timed out") + persistedError := requireChatLastErrorPayload(t, chatResult.LastError) + require.Equal(t, codersdk.ChatLastError{ + Message: "Dynamic tool execution timed out", + Kind: "generic", + }, persistedError) require.False(t, chatResult.WorkerID.Valid) } @@ -3764,25 +3797,30 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { LastModelConfigID: model.ID, }) - // Simulate a chat that failed with an error. + // Simulate a migrated legacy row that now stores a minimal + // structured payload in last_error. errorMessage := "stream response: status 500: internal server error" + legacyPayload := codersdk.ChatLastError{ + Message: errorMessage, + Kind: "generic", + } chat, err := db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{ ID: chat.ID, Status: database.ChatStatusError, WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{String: errorMessage, Valid: true}, + LastError: mustChatLastErrorRawMessage(t, legacyPayload), }) require.NoError(t, err) require.Equal(t, database.ChatStatusError, chat.Status) - require.Equal(t, sql.NullString{String: errorMessage, Valid: true}, chat.LastError) + require.Equal(t, legacyPayload, requireChatLastErrorPayload(t, chat.LastError)) // Verify the error is persisted when re-read from the database. fromDB, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) require.Equal(t, database.ChatStatusError, fromDB.Status) - require.Equal(t, sql.NullString{String: errorMessage, Valid: true}, fromDB.LastError) + require.Equal(t, legacyPayload, requireChatLastErrorPayload(t, fromDB.LastError)) // Verify the error is cleared when the chat transitions to a // non-error status (e.g. pending after a retry). @@ -3792,7 +3830,7 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) require.Equal(t, database.ChatStatusPending, chat.Status) @@ -3949,7 +3987,7 @@ func TestPersistToolResultWithBinaryData(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat run failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat run failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } var toolMessage *database.ChatMessage @@ -4100,7 +4138,7 @@ func TestDynamicToolCallPausesAndResumes(t *testing.T) { require.Equal(t, database.ChatStatusRequiresAction, chatResult.Status, "expected requires_action, got %s (last_error=%q)", - chatResult.Status, chatResult.LastError.String) + chatResult.Status, chatLastErrorMessage(chatResult.LastError)) // 2. Read the assistant message to find the tool-call ID. var toolCallID string @@ -4160,7 +4198,7 @@ func TestDynamicToolCallPausesAndResumes(t *testing.T) { // 5. Verify the chat completed successfully. if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat run failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat run failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // 6. Verify the mock received exactly 2 streaming calls. @@ -4264,7 +4302,7 @@ func TestDynamicToolNamedProposePlanRemainsAvailableOutsidePlanMode(t *testing.T }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat run failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat run failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } streamedCallsMu.Lock() @@ -4381,7 +4419,7 @@ func TestDynamicToolCallMixedWithBuiltIn(t *testing.T) { require.Equal(t, database.ChatStatusRequiresAction, chatResult.Status, "expected requires_action, got %s (last_error=%q)", - chatResult.Status, chatResult.LastError.String) + chatResult.Status, chatLastErrorMessage(chatResult.LastError)) // 2. Verify the built-in tool (read_file) was already // executed by checking that a tool result message @@ -4443,7 +4481,7 @@ func TestDynamicToolCallMixedWithBuiltIn(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat run failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat run failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // 5. Verify the LLM received exactly 2 streaming calls. @@ -4519,7 +4557,7 @@ func TestSubmitToolResultsConcurrency(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) require.Equal(t, database.ChatStatusRequiresAction, chatResult.Status, "expected requires_action, got %s (last_error=%q)", - chatResult.Status, chatResult.LastError.String) + chatResult.Status, chatLastErrorMessage(chatResult.LastError)) // Find the tool call ID from the assistant message. var toolCallID string @@ -5138,7 +5176,7 @@ func TestStoppedWorkspaceWithPersistedAgentBindingDoesNotBlockChat(t *testing.T) WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: sql.NullString{}, + LastError: pqtype.NullRawMessage{}, }) require.NoError(t, err) @@ -5177,7 +5215,7 @@ func TestStoppedWorkspaceWithPersistedAgentBindingDoesNotBlockChat(t *testing.T) }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } require.EqualValues(t, 1, dialCalls.Load()) @@ -7035,9 +7073,10 @@ func TestProcessChat_UserProviderKey_MissingKeyError(t *testing.T) { chatResult := waitForTerminalChat(ctx, t, db, chat.ID) require.Equal(t, database.ChatStatusError, chatResult.Status) - require.True(t, chatResult.LastError.Valid, "LastError should be set") - require.NotEmpty(t, chatResult.LastError.String) - require.NotContains(t, chatResult.LastError.String, "panicked") + persistedError := requireChatLastErrorPayload(t, chatResult.LastError) + require.NotEmpty(t, persistedError.Message) + require.NotContains(t, persistedError.Message, "panicked") + require.Equal(t, "generic", persistedError.Kind) require.NotEqual(t, database.ChatStatusRunning, chatResult.Status) require.Zero(t, llmCalls.Load(), "missing user key should fail before any LLM request") } @@ -7099,9 +7138,10 @@ func TestProcessChatPanicRecovery(t *testing.T) { return got.Status == database.ChatStatusError }, testutil.WaitLong, testutil.IntervalFast) - require.True(t, chatResult.LastError.Valid, "LastError should be set") - require.Contains(t, chatResult.LastError.String, "chat processing panicked") - require.Contains(t, chatResult.LastError.String, "intentional test panic") + persistedError := requireChatLastErrorPayload(t, chatResult.LastError) + require.Contains(t, persistedError.Message, "chat processing panicked") + require.Contains(t, persistedError.Message, "intentional test panic") + require.Equal(t, "generic", persistedError.Kind) } // panicOnInTxDB wraps a database.Store and panics on the first InTx @@ -7265,7 +7305,7 @@ func TestMCPServerToolInvocation(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // The MCP tool (test-mcp__echo) should appear in the tool @@ -7765,7 +7805,7 @@ func TestMCPServerOAuth2TokenRefresh(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // The token should have been refreshed. @@ -7873,7 +7913,7 @@ func TestMCPServerOAuth2TokenRefreshFailureGraceful(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat should not fail", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat should not fail", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // The LLM should have been called at least once. @@ -7996,7 +8036,7 @@ func TestChatTemplateAllowlistEnforcement(t *testing.T) { }, testutil.WaitLong, testutil.IntervalFast) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "chat run failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "chat run failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } // Collect all tool results keyed by tool name. Each tool may diff --git a/coderd/x/chatd/chaterror/classify_test.go b/coderd/x/chatd/chaterror/classify_test.go index 4d3f654d3797b..353c758134718 100644 --- a/coderd/x/chatd/chaterror/classify_test.go +++ b/coderd/x/chatd/chaterror/classify_test.go @@ -26,7 +26,7 @@ func TestClassify(t *testing.T) { name: "AmbiguousOverloadKeepsProviderUnknown", err: xerrors.New("status 529 from upstream"), want: chaterror.ClassifiedError{ - Message: "The AI provider is temporarily overloaded (HTTP 529).", + Message: "The AI provider is temporarily overloaded.", Kind: chaterror.KindOverloaded, Provider: "", Retryable: true, @@ -114,7 +114,7 @@ func TestClassify(t *testing.T) { name: "ExplicitStatus429ClassifiesAsRateLimit", err: xerrors.New("status 429 from upstream"), want: chaterror.ClassifiedError{ - Message: "The AI provider is rate limiting requests (HTTP 429).", + Message: "The AI provider is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "", Retryable: true, @@ -561,7 +561,7 @@ func TestWithProviderUsesExplicitHint(t *testing.T) { enriched := classified.WithProvider("azure openai") require.Equal(t, chaterror.ClassifiedError{ - Message: "Azure OpenAI is rate limiting requests (HTTP 429).", + Message: "Azure OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "azure", Retryable: true, @@ -577,7 +577,7 @@ func TestWithProviderAddsProviderWhenUnknown(t *testing.T) { enriched := classified.WithProvider("openai") require.Equal(t, chaterror.ClassifiedError{ - Message: "OpenAI is rate limiting requests (HTTP 429).", + Message: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", Retryable: true, @@ -595,7 +595,7 @@ func TestClassify_UsesStructuredProviderStatusAndRetryAfter(t *testing.T) { )) require.Equal(t, chaterror.ClassifiedError{ - Message: "The AI provider is rate limiting requests (HTTP 429).", + Message: "The AI provider is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "", Retryable: true, @@ -659,7 +659,7 @@ func TestWithProviderPreservesRetryAfter(t *testing.T) { enriched := classified.WithProvider("openai") require.Equal(t, 30*time.Second, enriched.RetryAfter) require.Equal(t, chaterror.ClassifiedError{ - Message: "OpenAI is rate limiting requests (HTTP 429).", + Message: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", Retryable: true, @@ -679,7 +679,7 @@ func TestClassify_UsesStructuredProviderDetailFromResponseDump(t *testing.T) { )) require.Equal(t, chaterror.ClassifiedError{ - Message: "The AI provider returned an unexpected error (HTTP 400).", + Message: "The AI provider returned an unexpected error.", Detail: "Image exceeds 5 MB maximum.", Kind: chaterror.KindGeneric, Provider: "", diff --git a/coderd/x/chatd/chaterror/message.go b/coderd/x/chatd/chaterror/message.go index 850c7ab4612c1..a3e361773adf6 100644 --- a/coderd/x/chatd/chaterror/message.go +++ b/coderd/x/chatd/chaterror/message.go @@ -6,37 +6,21 @@ import ( ) // terminalMessage produces the user-facing error description shown -// when retries are exhausted. It includes HTTP status codes and -// actionable remediation guidance. +// when retries are exhausted. HTTP status codes are carried in the +// classified payload's StatusCode field and rendered as a separate +// footer chip by the UI, so they are intentionally omitted here to +// avoid duplicating the same information in two places. func terminalMessage(classified ClassifiedError) string { subject := providerSubject(classified.Provider) switch classified.Kind { case KindOverloaded: - if classified.StatusCode > 0 { - return fmt.Sprintf( - "%s is temporarily overloaded (HTTP %d).", - subject, classified.StatusCode, - ) - } return fmt.Sprintf("%s is temporarily overloaded.", subject) case KindRateLimit: - if classified.StatusCode > 0 { - return fmt.Sprintf( - "%s is rate limiting requests (HTTP %d).", - subject, classified.StatusCode, - ) - } return fmt.Sprintf("%s is rate limiting requests.", subject) case KindTimeout: - if classified.StatusCode > 0 { - return fmt.Sprintf( - "%s is temporarily unavailable (HTTP %d).", - subject, classified.StatusCode, - ) - } - if !classified.Retryable { + if !classified.Retryable && classified.StatusCode == 0 { return "The request timed out before it completed." } return fmt.Sprintf("%s is temporarily unavailable.", subject) @@ -65,13 +49,7 @@ func terminalMessage(classified ClassifiedError) string { ) default: - if classified.StatusCode > 0 { - return fmt.Sprintf( - "%s returned an unexpected error (HTTP %d).", - subject, classified.StatusCode, - ) - } - if !classified.Retryable { + if !classified.Retryable && classified.StatusCode == 0 { return "The chat request failed unexpectedly." } return fmt.Sprintf("%s returned an unexpected error.", subject) diff --git a/coderd/x/chatd/chaterror/payload.go b/coderd/x/chatd/chaterror/payload.go index ca7dc214f4520..ac1086c31fc20 100644 --- a/coderd/x/chatd/chaterror/payload.go +++ b/coderd/x/chatd/chaterror/payload.go @@ -6,6 +6,20 @@ import ( "github.com/coder/coder/v2/codersdk" ) +func LastErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { + if classified.Message == "" { + return nil + } + return &codersdk.ChatLastError{ + Message: classified.Message, + Detail: classified.Detail, + Kind: classified.Kind, + Provider: classified.Provider, + Retryable: classified.Retryable, + StatusCode: classified.StatusCode, + } +} + func StreamErrorPayload(classified ClassifiedError) *codersdk.ChatStreamError { if classified.Message == "" { return nil diff --git a/coderd/x/chatd/chaterror/payload_test.go b/coderd/x/chatd/chaterror/payload_test.go index c41bf7cd0da26..f0eb9ced3a1d3 100644 --- a/coderd/x/chatd/chaterror/payload_test.go +++ b/coderd/x/chatd/chaterror/payload_test.go @@ -20,7 +20,24 @@ func TestStreamErrorPayloadUsesNormalizedClassification(t *testing.T) { payload := chaterror.StreamErrorPayload(classified) require.Equal(t, &codersdk.ChatStreamError{ - Message: "Azure OpenAI is rate limiting requests (HTTP 429).", + Message: "Azure OpenAI is rate limiting requests.", + Kind: chaterror.KindRateLimit, + Provider: "azure", + Retryable: true, + StatusCode: 429, + }, payload) +} + +func TestLastErrorPayloadUsesNormalizedClassification(t *testing.T) { + t.Parallel() + + classified := chaterror.Classify( + xerrors.New("azure openai received status 429 from upstream"), + ) + payload := chaterror.LastErrorPayload(classified) + + require.Equal(t, &codersdk.ChatLastError{ + Message: "Azure OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "azure", Retryable: true, @@ -44,6 +61,7 @@ func TestStreamErrorPayloadIncludesProviderDetail(t *testing.T) { func TestStreamErrorPayloadNilForEmptyClassification(t *testing.T) { t.Parallel() + require.Nil(t, chaterror.LastErrorPayload(chaterror.ClassifiedError{})) require.Nil(t, chaterror.StreamErrorPayload(chaterror.ClassifiedError{})) } @@ -53,7 +71,7 @@ func TestStreamRetryPayloadUsesNormalizedClassification(t *testing.T) { delay := 3 * time.Second startedAt := time.Now() payload := chaterror.StreamRetryPayload(2, delay, chaterror.ClassifiedError{ - Message: "OpenAI returned an unexpected error (HTTP 503).", + Message: "OpenAI returned an unexpected error.", Kind: chaterror.KindGeneric, Provider: "openai", Retryable: true, diff --git a/coderd/x/chatd/chatloop/chatloop_test.go b/coderd/x/chatd/chatloop/chatloop_test.go index 89f5996e1a9fc..862aa9ed0575d 100644 --- a/coderd/x/chatd/chatloop/chatloop_test.go +++ b/coderd/x/chatd/chatloop/chatloop_test.go @@ -576,7 +576,7 @@ func TestRun_OnRetryEnrichesProvider(t *testing.T) { require.Equal(t, 429, records[0].classified.StatusCode) require.Equal( t, - "OpenAI is rate limiting requests (HTTP 429).", + "OpenAI is rate limiting requests.", records[0].classified.Message, ) } diff --git a/coderd/x/chatd/subagent_internal_test.go b/coderd/x/chatd/subagent_internal_test.go index e46e3189ce2ee..480454fe29639 100644 --- a/coderd/x/chatd/subagent_internal_test.go +++ b/coderd/x/chatd/subagent_internal_test.go @@ -2,7 +2,6 @@ package chatd import ( "context" - "database/sql" "encoding/json" "sync" "testing" @@ -2717,7 +2716,12 @@ func setChatStatus( Status: status, } if lastError != "" { - params.LastError = sql.NullString{String: lastError, Valid: true} + encodedLastError, err := json.Marshal(codersdk.ChatLastError{ + Message: lastError, + Kind: "generic", + }) + require.NoError(t, err) + params.LastError = pqtype.NullRawMessage{RawMessage: encodedLastError, Valid: true} } _, err := db.UpdateChatStatus(ctx, params) require.NoError(t, err) diff --git a/codersdk/chats.go b/codersdk/chats.go index 7f829332b7e73..28935ffa67792 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -97,27 +97,32 @@ const ( // Chat represents a chat session with an AI agent. type Chat struct { - ID uuid.UUID `json:"id" format:"uuid"` - OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` - OwnerID uuid.UUID `json:"owner_id" format:"uuid"` - WorkspaceID *uuid.UUID `json:"workspace_id,omitempty" format:"uuid"` - BuildID *uuid.UUID `json:"build_id,omitempty" format:"uuid"` - AgentID *uuid.UUID `json:"agent_id,omitempty" format:"uuid"` - ParentChatID *uuid.UUID `json:"parent_chat_id,omitempty" format:"uuid"` - RootChatID *uuid.UUID `json:"root_chat_id,omitempty" format:"uuid"` - LastModelConfigID uuid.UUID `json:"last_model_config_id" format:"uuid"` - Title string `json:"title"` - Status ChatStatus `json:"status"` - PlanMode ChatPlanMode `json:"plan_mode,omitempty"` - LastError *string `json:"last_error"` - DiffStatus *ChatDiffStatus `json:"diff_status,omitempty"` - CreatedAt time.Time `json:"created_at" format:"date-time"` - UpdatedAt time.Time `json:"updated_at" format:"date-time"` - Archived bool `json:"archived"` - PinOrder int32 `json:"pin_order"` - MCPServerIDs []uuid.UUID `json:"mcp_server_ids" format:"uuid"` - Labels map[string]string `json:"labels"` - Files []ChatFileMetadata `json:"files,omitempty"` + ID uuid.UUID `json:"id" format:"uuid"` + OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` + OwnerID uuid.UUID `json:"owner_id" format:"uuid"` + WorkspaceID *uuid.UUID `json:"workspace_id,omitempty" format:"uuid"` + BuildID *uuid.UUID `json:"build_id,omitempty" format:"uuid"` + AgentID *uuid.UUID `json:"agent_id,omitempty" format:"uuid"` + ParentChatID *uuid.UUID `json:"parent_chat_id,omitempty" format:"uuid"` + RootChatID *uuid.UUID `json:"root_chat_id,omitempty" format:"uuid"` + LastModelConfigID uuid.UUID `json:"last_model_config_id" format:"uuid"` + Title string `json:"title"` + Status ChatStatus `json:"status"` + PlanMode ChatPlanMode `json:"plan_mode,omitempty"` + // LastError is a backward-compatible one-line headline derived from + // LastErrorPayload.Message when a persisted chat error exists. + LastError *string `json:"last_error"` + // LastErrorPayload is the structured persisted error state used to + // rehydrate failed chat callouts after refresh. + LastErrorPayload *ChatLastError `json:"last_error_payload,omitempty"` + DiffStatus *ChatDiffStatus `json:"diff_status,omitempty"` + CreatedAt time.Time `json:"created_at" format:"date-time"` + UpdatedAt time.Time `json:"updated_at" format:"date-time"` + Archived bool `json:"archived"` + PinOrder int32 `json:"pin_order"` + MCPServerIDs []uuid.UUID `json:"mcp_server_ids" format:"uuid"` + Labels map[string]string `json:"labels"` + Files []ChatFileMetadata `json:"files,omitempty"` // HasUnread is true when assistant messages exist beyond // the owner's read cursor, which updates on stream // connect and disconnect. @@ -1425,6 +1430,26 @@ type ChatStreamStatus struct { Status ChatStatus `json:"status"` } +// ChatLastError represents a persisted terminal chat error. +// +// Keep this aligned with ChatStreamError so persisted and live failures +// expose the same field set to clients. +type ChatLastError struct { + // Message is the normalized, user-facing error message. + Message string `json:"message"` + // Detail is optional provider-specific context shown alongside the + // normalized error message when available. + Detail string `json:"detail,omitempty"` + // Kind classifies the error for consistent client rendering. + Kind string `json:"kind,omitempty"` + // Provider identifies the upstream model provider when known. + Provider string `json:"provider,omitempty"` + // Retryable reports whether the underlying error is transient. + Retryable bool `json:"retryable"` + // StatusCode is the best-effort upstream HTTP status code. + StatusCode int `json:"status_code,omitempty"` +} + // ChatStreamError represents an error event in the stream. type ChatStreamError struct { // Message is the normalized, user-facing error message. diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 6d0fb732b7469..c994a33540780 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -448,6 +448,14 @@ func TestChat_JSONRoundTrip(t *testing.T) { refreshedAt := now staleAt := now.Add(time.Hour) lastError := "boom" + lastErrorPayload := &codersdk.ChatLastError{ + Message: lastError, + Detail: "provider detail", + Kind: "generic", + Provider: "openai", + Retryable: true, + StatusCode: 503, + } prURL := "https://github.com/coder/coder/pull/42" workspaceID := uuid.New() buildID := uuid.New() @@ -467,6 +475,7 @@ func TestChat_JSONRoundTrip(t *testing.T) { Title: "round-trip-test", Status: codersdk.ChatStatusRunning, LastError: &lastError, + LastErrorPayload: lastErrorPayload, CreatedAt: now, UpdatedAt: now, Archived: true, @@ -505,6 +514,22 @@ func TestChat_JSONRoundTrip(t *testing.T) { require.Equal(t, original, decoded) } +func TestChatLastErrorMatchesChatStreamError(t *testing.T) { + t.Parallel() + + lastErrorType := reflect.TypeFor[codersdk.ChatLastError]() + streamErrorType := reflect.TypeFor[codersdk.ChatStreamError]() + require.Equal(t, streamErrorType.NumField(), lastErrorType.NumField()) + + for i := range lastErrorType.NumField() { + lastErrorField := lastErrorType.Field(i) + streamErrorField := streamErrorType.Field(i) + require.Equal(t, streamErrorField.Name, lastErrorField.Name) + require.Equal(t, streamErrorField.Type, lastErrorField.Type) + require.Equal(t, streamErrorField.Tag, lastErrorField.Tag) + } +} + func TestNewDynamicTool(t *testing.T) { t.Parallel() diff --git a/enterprise/coderd/x/chatd/chatd_test.go b/enterprise/coderd/x/chatd/chatd_test.go index 37d30e23c2304..dc455f5580163 100644 --- a/enterprise/coderd/x/chatd/chatd_test.go +++ b/enterprise/coderd/x/chatd/chatd_test.go @@ -30,6 +30,18 @@ import ( "github.com/coder/quartz" ) +func chatLastErrorMessage(raw database.Chat) string { + if !raw.LastError.Valid { + return "" + } + + var payload codersdk.ChatLastError + if err := json.Unmarshal(raw.LastError.RawMessage, &payload); err == nil && payload.Message != "" { + return payload.Message + } + return string(raw.LastError.RawMessage) +} + func newTestServer( t *testing.T, db database.Store, @@ -1712,14 +1724,14 @@ waitForStream: currentChat, dbErr := db.GetChatByID(ctx, chat.ID) if dbErr == nil && currentChat.Status == database.ChatStatusError { t.Fatalf("worker failed to process chat: status=%s last_error=%s", - currentChat.Status, currentChat.LastError.String) + currentChat.Status, chatLastErrorMessage(currentChat)) } case <-ctx.Done(): // Dump the final chat status for debugging. currentChat, dbErr := db.GetChatByID(context.Background(), chat.ID) if dbErr == nil { t.Fatalf("timed out waiting for worker to start streaming (chat status=%s, last_error=%q)", - currentChat.Status, currentChat.LastError.String) + currentChat.Status, chatLastErrorMessage(currentChat)) } t.Fatal("timed out waiting for worker to start streaming") } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 39c5d2f56c3ba..273c35e66bdb8 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1286,7 +1286,16 @@ export interface Chat { readonly title: string; readonly status: ChatStatus; readonly plan_mode?: ChatPlanMode; + /** + * LastError is a backward-compatible one-line headline derived from + * LastErrorPayload.Message when a persisted chat error exists. + */ readonly last_error: string | null; + /** + * LastErrorPayload is the structured persisted error state used to + * rehydrate failed chat callouts after refresh. + */ + readonly last_error_payload?: ChatLastError; readonly diff_status?: ChatDiffStatus; readonly created_at: string; readonly updated_at: string; @@ -1811,6 +1820,41 @@ export const ChatInputPartTypes: ChatInputPartType[] = [ "text", ]; +// From codersdk/chats.go +/** + * ChatLastError represents a persisted terminal chat error. + * + * Keep this aligned with ChatStreamError so persisted and live failures + * expose the same field set to clients. + */ +export interface ChatLastError { + /** + * Message is the normalized, user-facing error message. + */ + readonly message: string; + /** + * Detail is optional provider-specific context shown alongside the + * normalized error message when available. + */ + readonly detail?: string; + /** + * Kind classifies the error for consistent client rendering. + */ + readonly kind?: string; + /** + * Provider identifies the upstream model provider when known. + */ + readonly provider?: string; + /** + * Retryable reports whether the underlying error is transient. + */ + readonly retryable: boolean; + /** + * StatusCode is the best-effort upstream HTTP status code. + */ + readonly status_code?: number; +} + // From codersdk/chats.go /** * ChatMessage represents a single message in a chat. diff --git a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx index f51acb871279a..db2bd948332ce 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx @@ -1188,6 +1188,42 @@ export const ArchivedOtherUserChat: Story = { }, }; +/** Persisted structured errors rehydrate the failed callout after refresh. */ +export const PersistedStructuredError: Story = { + parameters: { + queries: buildQueries( + { + id: CHAT_ID, + ...baseChatFields, + title: "Persisted provider error", + status: "error", + last_error: { + message: "Anthropic returned an unexpected error.", + detail: + "messages.0.content.1.image.source.base64: image exceeds 5 MB maximum.", + kind: "generic", + provider: "anthropic", + retryable: false, + status_code: 400, + }, + }, + { messages: [], queued_messages: [], has_more: false }, + { diffUrl: undefined }, + ), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByRole("heading", { name: /request failed/i }), + ).toBeVisible(); + expect( + canvas.getByText(/anthropic returned an unexpected error\./i), + ).toBeVisible(); + expect(canvas.getByText(/^HTTP 400$/)).toBeVisible(); + expect(canvas.getByText(/image exceeds 5 mb maximum/i)).toBeVisible(); + }, +}; + export const PlanModeFromChatState: Story = { parameters: { queries: buildQueries( diff --git a/site/src/pages/AgentsPage/AgentChatPage.tsx b/site/src/pages/AgentsPage/AgentChatPage.tsx index 68a25564a4b0f..cc39c1c51c8c3 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.tsx @@ -551,6 +551,19 @@ const getPersistedDetailError = ({ if (cachedError) { return cachedError; } + const lastErrorPayload = chatRecord?.last_error_payload; + const lastErrorMessage = lastErrorPayload?.message?.trim(); + const lastErrorDetail = lastErrorPayload?.detail?.trim(); + if (lastErrorPayload && lastErrorMessage) { + return { + message: lastErrorMessage, + kind: lastErrorPayload.kind?.trim() || "generic", + provider: lastErrorPayload.provider?.trim() || undefined, + retryable: lastErrorPayload.retryable, + statusCode: lastErrorPayload.status_code, + ...(lastErrorDetail ? { detail: lastErrorDetail } : {}), + }; + } const lastError = chatRecord?.last_error?.trim(); if (lastError) { return { kind: "generic", message: lastError }; diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index f5500bb4fc11d..2b6c9f3d4a44d 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -295,7 +295,7 @@ export const WithError: Story = { = ({ status }) => { if (status.phase === "retrying") { metadataItems.push(Attempt {status.attempt}); } + if (status.phase === "failed" && status.statusCode !== undefined) { + metadataItems.push(HTTP {status.statusCode}); + } return ( Date: Mon, 4 May 2026 06:20:34 +0000 Subject: [PATCH 2/8] refactor: collapse persisted chat error onto canonical structured schema Drop the legacy `last_error` headline string and rename `last_error_payload` to `last_error` so chats expose a single structured error object. Make persisted decode lossless, share one frontend normalizer between live and persisted paths, guard the HTTP chip on positive status codes, and align tests, stories, fixtures, and docs. --- cli/agents_list.go | 13 +++- coderd/database/db2sdk/db2sdk.go | 20 +++--- coderd/database/db2sdk/db2sdk_test.go | 66 +++++++++++++---- ... => 000485_chat_last_error_jsonb.down.sql} | 0 ...ql => 000485_chat_last_error_jsonb.up.sql} | 0 .../fixtures/000424_chat_last_error.up.sql | 25 ++++++- ...ql => 000485_chat_last_error_jsonb.up.sql} | 8 +++ coderd/exp_chats_test.go | 2 - coderd/x/chatd/chatd.go | 14 ++-- coderd/x/chatd/chatd_internal_test.go | 13 ++-- coderd/x/chatd/chatd_test.go | 20 +++--- coderd/x/chatd/chaterror/payload.go | 14 +--- coderd/x/chatd/chaterror/payload_test.go | 25 ++----- coderd/x/chatd/integration_responses_test.go | 2 +- codersdk/chats.go | 72 +++++++------------ codersdk/chats_test.go | 24 +------ docs/ai-coder/agents/chats-api.md | 1 - enterprise/coderd/x/chatd/chatd_test.go | 13 ++-- site/src/api/queries/chats.test.ts | 1 - site/src/api/typesGenerated.ts | 16 +---- .../AgentsPage/AgentChatPage.stories.tsx | 1 - site/src/pages/AgentsPage/AgentChatPage.tsx | 29 ++------ .../AgentsPage/AgentChatPageView.stories.tsx | 1 - .../AgentsPage/AgentsPageView.stories.tsx | 7 +- .../ChatConversation/ChatStatusCallout.tsx | 6 +- .../components/ChatConversation/chatError.ts | 32 +++++++++ .../ChatConversation/chatStore.test.tsx | 1 - .../ChatConversation/useChatStore.ts | 19 ++--- .../components/ChatTopBar.stories.tsx | 1 - .../Sidebar/AgentsSidebar.stories.tsx | 1 - .../components/Sidebar/AgentsSidebar.test.tsx | 1 - .../components/Sidebar/AgentsSidebar.tsx | 2 +- 32 files changed, 229 insertions(+), 221 deletions(-) rename coderd/database/migrations/{000474_chat_last_error_jsonb.down.sql => 000485_chat_last_error_jsonb.down.sql} (100%) rename coderd/database/migrations/{000474_chat_last_error_jsonb.up.sql => 000485_chat_last_error_jsonb.up.sql} (100%) rename coderd/database/migrations/testdata/fixtures/{000474_chat_last_error_jsonb.up.sql => 000485_chat_last_error_jsonb.up.sql} (73%) create mode 100644 site/src/pages/AgentsPage/components/ChatConversation/chatError.ts diff --git a/cli/agents_list.go b/cli/agents_list.go index d6ea6a1d5f8f6..fdaa05ab6e885 100644 --- a/cli/agents_list.go +++ b/cli/agents_list.go @@ -66,6 +66,13 @@ func (m chatListModel) searchQuery() string { return strings.TrimSpace(strings.ToLower(m.search.Value())) } +func chatLastErrorMessage(chat codersdk.Chat) string { + if chat.LastError == nil { + return "" + } + return chat.LastError.Message +} + func (m chatListModel) filteredChats() []codersdk.Chat { query := m.searchQuery() if query == "" { @@ -78,7 +85,7 @@ func (m chatListModel) filteredChats() []codersdk.Chat { filtered = append(filtered, chat) continue } - if chat.LastError != nil && strings.Contains(strings.ToLower(*chat.LastError), query) { + if lastError := chatLastErrorMessage(chat); strings.Contains(strings.ToLower(lastError), query) { filtered = append(filtered, chat) } } @@ -445,13 +452,13 @@ func (m chatListModel) View() string { rowText := fmt.Sprintf("%s%s %s %s%s", rowPrefix, rowStyle.Render(title), status, m.styles.dimmedText.Render(timeAgo(row.chat.UpdatedAt)), extra) lines = append(lines, rowText) - if row.chat.Status == codersdk.ChatStatusError && row.chat.LastError != nil { + if lastError := chatLastErrorMessage(row.chat); row.chat.Status == codersdk.ChatStatusError && lastError != "" { errWidth := max(m.width-4, 20) errPrefix := " " if row.depth > 0 { errPrefix += strings.Repeat(" ", row.depth) } - lines = append(lines, errPrefix+m.styles.dimmedText.Render(m.styles.truncate(sanitizeTerminalRenderableText(*row.chat.LastError), errWidth))) + lines = append(lines, errPrefix+m.styles.dimmedText.Render(m.styles.truncate(sanitizeTerminalRenderableText(lastError), errWidth))) } } diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index b9374f2ff3df2..2f1294d8f02cb 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -1609,27 +1609,30 @@ func nullTimePtr(v sql.NullTime) *time.Time { const fallbackChatLastErrorMessage = "The chat request failed unexpectedly." -func decodeChatLastError(raw pqtype.NullRawMessage) (*codersdk.ChatLastError, *string) { +func decodeChatLastError(raw pqtype.NullRawMessage) *codersdk.ChatLastError { if !raw.Valid { - return nil, nil + return nil } var payload codersdk.ChatLastError if err := json.Unmarshal(raw.RawMessage, &payload); err != nil { - return nil, ptr.Ref(fallbackChatLastErrorMessage) + return &codersdk.ChatLastError{ + Message: fallbackChatLastErrorMessage, + Kind: "generic", + } } payload.Message = strings.TrimSpace(payload.Message) payload.Detail = strings.TrimSpace(payload.Detail) payload.Kind = strings.TrimSpace(payload.Kind) payload.Provider = strings.TrimSpace(payload.Provider) - if payload.Message == "" { - return nil, ptr.Ref(fallbackChatLastErrorMessage) - } if payload.Kind == "" { payload.Kind = "generic" } - return &payload, ptr.Ref(payload.Message) + if payload.Message == "" { + payload.Message = fallbackChatLastErrorMessage + } + return &payload } // Chat converts a database.Chat to a codersdk.Chat. It coalesces @@ -1647,7 +1650,7 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database if labels == nil { labels = map[string]string{} } - lastErrorPayload, lastError := decodeChatLastError(c.LastError) + lastError := decodeChatLastError(c.LastError) chat := codersdk.Chat{ ID: c.ID, OrganizationID: c.OrganizationID, @@ -1663,7 +1666,6 @@ func Chat(c database.Chat, diffStatus *database.ChatDiffStatus, files []database Labels: labels, ClientType: codersdk.ChatClientType(c.ClientType), LastError: lastError, - LastErrorPayload: lastErrorPayload, } if c.PlanMode.Valid { chat.PlanMode = codersdk.ChatPlanMode(c.PlanMode.ChatPlanMode) diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index 1c2426cc78af2..1a85b148a9f40 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -917,8 +917,12 @@ func TestChat_AllFieldsPopulated(t *testing.T) { // converter is updated. now := dbtime.Now() lastErrorPayload := codersdk.ChatLastError{ - Message: "boom", - Kind: "generic", + Message: "boom", + Detail: "provider detail", + Kind: "generic", + Provider: "openai", + Retryable: true, + StatusCode: 503, } lastErrorRaw, err := json.Marshal(lastErrorPayload) require.NoError(t, err) @@ -977,9 +981,7 @@ func TestChat_AllFieldsPopulated(t *testing.T) { got := db2sdk.Chat(input, diffStatus, fileRows) - require.NotNil(t, got.LastError) - require.Equal(t, lastErrorPayload.Message, *got.LastError) - require.Equal(t, &lastErrorPayload, got.LastErrorPayload) + require.Equal(t, &lastErrorPayload, got.LastError) v := reflect.ValueOf(got) typ := v.Type() @@ -1064,20 +1066,56 @@ func TestChat_NilFilesOmitted(t *testing.T) { require.Empty(t, result.Files) } -func TestChat_LastErrorPayloadFallback(t *testing.T) { +func TestChat_LastErrorFallback(t *testing.T) { t.Parallel() + const fallbackMessage = "The chat request failed unexpectedly." + tests := []struct { - name string - raw json.RawMessage + name string + raw json.RawMessage + expectPayload *codersdk.ChatLastError }{ - { - name: "MessageMissing", - raw: json.RawMessage(`{"kind":"generic"}`), - }, { name: "MalformedJSON", raw: json.RawMessage(`{`), + expectPayload: &codersdk.ChatLastError{ + Message: fallbackMessage, + Kind: "generic", + Retryable: false, + }, + }, + { + name: "MessageMissingPreservesMetadata", + raw: json.RawMessage(`{"kind":"timeout","provider":"openai","status_code":504}`), + expectPayload: &codersdk.ChatLastError{ + Message: fallbackMessage, + Kind: "timeout", + Provider: "openai", + Retryable: false, + StatusCode: 504, + }, + }, + { + name: "WhitespaceMessageDefaultsKind", + raw: json.RawMessage(`{"message":" ","provider":"openai"}`), + expectPayload: &codersdk.ChatLastError{ + Message: fallbackMessage, + Kind: "generic", + Provider: "openai", + Retryable: false, + }, + }, + { + name: "KindMissingDefaultsGeneric", + raw: json.RawMessage(`{"message":"OpenAI returned an unexpected error.","provider":"openai","status_code":502}`), + expectPayload: &codersdk.ChatLastError{ + Message: "OpenAI returned an unexpected error.", + Kind: "generic", + Provider: "openai", + Retryable: false, + StatusCode: 502, + }, }, } @@ -1101,9 +1139,7 @@ func TestChat_LastErrorPayloadFallback(t *testing.T) { } result := db2sdk.Chat(chat, nil, nil) - require.NotNil(t, result.LastError) - require.Equal(t, "The chat request failed unexpectedly.", *result.LastError) - require.Nil(t, result.LastErrorPayload) + require.Equal(t, tc.expectPayload, result.LastError) }) } } diff --git a/coderd/database/migrations/000474_chat_last_error_jsonb.down.sql b/coderd/database/migrations/000485_chat_last_error_jsonb.down.sql similarity index 100% rename from coderd/database/migrations/000474_chat_last_error_jsonb.down.sql rename to coderd/database/migrations/000485_chat_last_error_jsonb.down.sql diff --git a/coderd/database/migrations/000474_chat_last_error_jsonb.up.sql b/coderd/database/migrations/000485_chat_last_error_jsonb.up.sql similarity index 100% rename from coderd/database/migrations/000474_chat_last_error_jsonb.up.sql rename to coderd/database/migrations/000485_chat_last_error_jsonb.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql index 6237891a44cf5..09284437740b7 100644 --- a/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql @@ -1,6 +1,27 @@ --- Migration 424 adds chats.last_error as text. Seed the existing fixture +-- Migration 424 adds chats.last_error as text. Seed one existing fixture -- chat with a legacy plain-text error so migration 474 has a non-null row --- to backfill. +-- to backfill, and add a second chat that leaves last_error NULL so the +-- migration fixture can assert both branches of the CASE expression. UPDATE chats SET last_error = 'Legacy provider failure' WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96'; + +INSERT INTO chats ( + id, + owner_id, + last_model_config_id, + title, + status, + created_at, + updated_at +) +SELECT + '5a4ac6a3-9dc5-440f-ae6b-5805e477bc59', + owner_id, + last_model_config_id, + 'Fixture Chat With Null Error', + 'waiting', + '2024-01-01 00:00:00+00', + '2024-01-01 00:00:00+00' +FROM chats +WHERE id = '72c0438a-18eb-4688-ab80-e4c6a126ef96'; diff --git a/coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql b/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql similarity index 73% rename from coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql rename to coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql index d757432932d3d..08c416a9edd2d 100644 --- a/coderd/database/migrations/testdata/fixtures/000474_chat_last_error_jsonb.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql @@ -17,4 +17,12 @@ BEGIN RAISE EXCEPTION 'expected migrated last_error kind, got %', payload ->> 'kind'; END IF; + + PERFORM 1 + FROM chats + WHERE id = '5a4ac6a3-9dc5-440f-ae6b-5805e477bc59' + AND last_error IS NULL; + IF NOT FOUND THEN + RAISE EXCEPTION 'expected null last_error row to remain NULL after migration'; + END IF; END $$; diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index ee9b878fbda0d..e3de80e28e7bd 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -4765,9 +4765,7 @@ func TestPatchChat(t *testing.T) { client := newChatClient(t) firstUser := coderdtest.CreateFirstUser(t, client.Client) _ = createChatModelConfig(t, client) - chat := createChat(ctx, t, client, firstUser.OrganizationID, "boundary baseline") - waitChatSettled(ctx, t, client, chat.ID) err := client.UpdateChat(ctx, chat.ID, codersdk.UpdateChatRequest{ diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 5d2e54fe16e89..66977b16854bb 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -5364,7 +5364,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { Message: panicFailureReason(r), Kind: chaterror.KindGeneric, } - lastErrorPayload = chaterror.LastErrorPayload(classified) + lastErrorPayload = chaterror.TerminalErrorPayload(classified) p.publishError(chat.ID, classified) status = database.ChatStatusError } @@ -5477,7 +5477,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { } logger.Error(ctx, "failed to process chat", slog.Error(err)) if classified, ok := processingFailure(err); ok { - lastErrorPayload = chaterror.LastErrorPayload(classified) + lastErrorPayload = chaterror.TerminalErrorPayload(classified) p.publishError(chat.ID, classified) } status = database.ChatStatusError @@ -8011,10 +8011,12 @@ func (p *Server) recoverStaleChats(ctx context.Context) { lastError := pqtype.NullRawMessage{} if locked.Status == database.ChatStatusRequiresAction { - lastErrorPayload, marshalErr := encodeChatLastErrorPayload(&codersdk.ChatLastError{ - Message: "Dynamic tool execution timed out", - Kind: chaterror.KindGeneric, - }) + lastErrorPayload, marshalErr := encodeChatLastErrorPayload( + chaterror.TerminalErrorPayload(chaterror.ClassifiedError{ + Message: "Dynamic tool execution timed out", + Kind: chaterror.KindGeneric, + }), + ) if marshalErr != nil { p.logger.Warn(ctx, "failed to marshal stale recovery last error payload", slog.F("chat_id", chat.ID), diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index 5d43b3350ea59..e38d5a138bbfa 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -2447,15 +2447,18 @@ func TestSubscribeFallsBackToLegacyErrorStringViaPubsub(t *testing.T) { } func newTestRetryPayload() *codersdk.ChatStreamRetry { - return &codersdk.ChatStreamRetry{ - Attempt: 1, - DelayMs: (1500 * time.Millisecond).Milliseconds(), - Error: "OpenAI is rate limiting requests.", + payload := chaterror.StreamRetryPayload(1, 1500*time.Millisecond, chaterror.ClassifiedError{ + Message: "OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "openai", + Retryable: true, StatusCode: 429, - RetryingAt: time.Unix(1_700_000_000, 0).UTC(), + }) + if payload == nil { + panic("expected retry payload") } + payload.RetryingAt = time.Unix(1_700_000_000, 0).UTC() + return payload } func newSubscribeTestServer(t *testing.T, db database.Store) *Server { diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index fba8ce137adc7..28b97aab75ab2 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -896,7 +896,7 @@ func TestExploreChatUsesPersistedMCPSnapshot(t *testing.T) { chatResult := waitForTerminalChat(ctx, t, db, exploreChat.ID) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "explore chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "explore chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } requestsMu.Lock() @@ -989,7 +989,7 @@ func TestRootExploreChatStaysBuiltinOnlyAtRuntime(t *testing.T) { storedChat, err := db.GetChatByID(ctx, exploreChat.ID) require.NoError(t, err) if storedChat.Status == database.ChatStatusError { - require.FailNowf(t, "explore chat failed", "last_error=%q", storedChat.LastError.String) + require.FailNowf(t, "explore chat failed", "last_error=%q", chatLastErrorMessage(storedChat.LastError)) } require.Equal(t, database.ChatStatusWaiting, storedChat.Status) require.ElementsMatch(t, []uuid.UUID{mcpConfig.ID}, storedChat.MCPServerIDs) @@ -1073,7 +1073,7 @@ func TestRootExploreChatExcludesWebSearchProviderToolAtRuntime(t *testing.T) { storedChat, err := db.GetChatByID(ctx, exploreChat.ID) require.NoError(t, err) if storedChat.Status == database.ChatStatusError { - require.FailNowf(t, "explore chat failed", "last_error=%q", storedChat.LastError.String) + require.FailNowf(t, "explore chat failed", "last_error=%q", chatLastErrorMessage(storedChat.LastError)) } require.Equal(t, database.ChatStatusWaiting, storedChat.Status) @@ -1208,7 +1208,7 @@ func TestExploreChatSendMessageCannotMutateMCPSnapshot(t *testing.T) { chatResult := waitForTerminalChat(ctx, t, db, exploreChat.ID) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "explore chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "explore chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } exploreChat, err = db.GetChatByID(ctx, exploreChat.ID) @@ -1237,7 +1237,7 @@ func TestExploreChatSendMessageCannotMutateMCPSnapshot(t *testing.T) { chatResult = waitForTerminalChat(ctx, t, db, exploreChat.ID) if chatResult.Status == database.ChatStatusError { - require.FailNowf(t, "explore chat failed", "last_error=%q", chatResult.LastError.String) + require.FailNowf(t, "explore chat failed", "last_error=%q", chatLastErrorMessage(chatResult.LastError)) } recordedChildRequests := childRequests() @@ -3797,8 +3797,8 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { LastModelConfigID: model.ID, }) - // Simulate a migrated legacy row that now stores a minimal - // structured payload in last_error. + // Write a minimal structured last_error payload through the + // query layer, then verify it round-trips through storage. errorMessage := "stream response: status 500: internal server error" legacyPayload := codersdk.ChatLastError{ Message: errorMessage, @@ -4880,7 +4880,7 @@ func TestCreateWorkspaceTool_EndToEnd(t *testing.T) { if chatResult.Status == codersdk.ChatStatusError { lastError := "" if chatResult.LastError != nil { - lastError = *chatResult.LastError + lastError = chatResult.LastError.Message } require.FailNowf(t, "chat run failed", "last_error=%q", lastError) } @@ -5052,7 +5052,7 @@ func TestStartWorkspaceTool_EndToEnd(t *testing.T) { if chatResult.Status == codersdk.ChatStatusError { lastError := "" if chatResult.LastError != nil { - lastError = *chatResult.LastError + lastError = chatResult.LastError.Message } require.FailNowf(t, "chat run failed", "last_error=%q", lastError) } @@ -9325,7 +9325,7 @@ func TestAdvisorChainMode_SnapshotKeepsFullHistory(t *testing.T) { turn1Chat, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) require.Equal(t, database.ChatStatusWaiting, turn1Chat.Status, - "turn 1 must complete before turn 2 can be sent; last_error=%q", turn1Chat.LastError.String) + "turn 1 must complete before turn 2 can be sent; last_error=%q", chatLastErrorMessage(turn1Chat.LastError)) _, err = server.SendMessage(ctx, chatd.SendMessageOptions{ ChatID: chat.ID, diff --git a/coderd/x/chatd/chaterror/payload.go b/coderd/x/chatd/chaterror/payload.go index ac1086c31fc20..0e1e918799fc1 100644 --- a/coderd/x/chatd/chaterror/payload.go +++ b/coderd/x/chatd/chaterror/payload.go @@ -6,7 +6,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func LastErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { +func TerminalErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { if classified.Message == "" { return nil } @@ -21,17 +21,7 @@ func LastErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { } func StreamErrorPayload(classified ClassifiedError) *codersdk.ChatStreamError { - if classified.Message == "" { - return nil - } - return &codersdk.ChatStreamError{ - Message: classified.Message, - Detail: classified.Detail, - Kind: classified.Kind, - Provider: classified.Provider, - Retryable: classified.Retryable, - StatusCode: classified.StatusCode, - } + return TerminalErrorPayload(classified) } func StreamRetryPayload( diff --git a/coderd/x/chatd/chaterror/payload_test.go b/coderd/x/chatd/chaterror/payload_test.go index f0eb9ced3a1d3..101d585c5515b 100644 --- a/coderd/x/chatd/chaterror/payload_test.go +++ b/coderd/x/chatd/chaterror/payload_test.go @@ -11,30 +11,13 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func TestStreamErrorPayloadUsesNormalizedClassification(t *testing.T) { +func TestTerminalErrorPayloadUsesNormalizedClassification(t *testing.T) { t.Parallel() classified := chaterror.Classify( xerrors.New("azure openai received status 429 from upstream"), ) - payload := chaterror.StreamErrorPayload(classified) - - require.Equal(t, &codersdk.ChatStreamError{ - Message: "Azure OpenAI is rate limiting requests.", - Kind: chaterror.KindRateLimit, - Provider: "azure", - Retryable: true, - StatusCode: 429, - }, payload) -} - -func TestLastErrorPayloadUsesNormalizedClassification(t *testing.T) { - t.Parallel() - - classified := chaterror.Classify( - xerrors.New("azure openai received status 429 from upstream"), - ) - payload := chaterror.LastErrorPayload(classified) + payload := chaterror.TerminalErrorPayload(classified) require.Equal(t, &codersdk.ChatLastError{ Message: "Azure OpenAI is rate limiting requests.", @@ -58,10 +41,10 @@ func TestStreamErrorPayloadIncludesProviderDetail(t *testing.T) { require.Equal(t, "Image exceeds 5 MB maximum.", payload.Detail) } -func TestStreamErrorPayloadNilForEmptyClassification(t *testing.T) { +func TestTerminalErrorPayloadNilForEmptyClassification(t *testing.T) { t.Parallel() - require.Nil(t, chaterror.LastErrorPayload(chaterror.ClassifiedError{})) + require.Nil(t, chaterror.TerminalErrorPayload(chaterror.ClassifiedError{})) require.Nil(t, chaterror.StreamErrorPayload(chaterror.ClassifiedError{})) } diff --git a/coderd/x/chatd/integration_responses_test.go b/coderd/x/chatd/integration_responses_test.go index adb4c1f7088b0..ecb99539ba54b 100644 --- a/coderd/x/chatd/integration_responses_test.go +++ b/coderd/x/chatd/integration_responses_test.go @@ -493,7 +493,7 @@ func requireResponsesChatWaiting( chat, err := db.GetChatByID(ctx, chatID) require.NoError(t, err) if chat.Status == database.ChatStatusError { - require.FailNowf(t, "chat failed", "last_error=%q", chat.LastError.String) + require.FailNowf(t, "chat failed", "last_error=%q", chatLastErrorMessage(chat.LastError)) } require.Equal(t, database.ChatStatusWaiting, chat.Status) } diff --git a/codersdk/chats.go b/codersdk/chats.go index 28935ffa67792..74b87093bab92 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -97,32 +97,27 @@ const ( // Chat represents a chat session with an AI agent. type Chat struct { - ID uuid.UUID `json:"id" format:"uuid"` - OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` - OwnerID uuid.UUID `json:"owner_id" format:"uuid"` - WorkspaceID *uuid.UUID `json:"workspace_id,omitempty" format:"uuid"` - BuildID *uuid.UUID `json:"build_id,omitempty" format:"uuid"` - AgentID *uuid.UUID `json:"agent_id,omitempty" format:"uuid"` - ParentChatID *uuid.UUID `json:"parent_chat_id,omitempty" format:"uuid"` - RootChatID *uuid.UUID `json:"root_chat_id,omitempty" format:"uuid"` - LastModelConfigID uuid.UUID `json:"last_model_config_id" format:"uuid"` - Title string `json:"title"` - Status ChatStatus `json:"status"` - PlanMode ChatPlanMode `json:"plan_mode,omitempty"` - // LastError is a backward-compatible one-line headline derived from - // LastErrorPayload.Message when a persisted chat error exists. - LastError *string `json:"last_error"` - // LastErrorPayload is the structured persisted error state used to - // rehydrate failed chat callouts after refresh. - LastErrorPayload *ChatLastError `json:"last_error_payload,omitempty"` - DiffStatus *ChatDiffStatus `json:"diff_status,omitempty"` - CreatedAt time.Time `json:"created_at" format:"date-time"` - UpdatedAt time.Time `json:"updated_at" format:"date-time"` - Archived bool `json:"archived"` - PinOrder int32 `json:"pin_order"` - MCPServerIDs []uuid.UUID `json:"mcp_server_ids" format:"uuid"` - Labels map[string]string `json:"labels"` - Files []ChatFileMetadata `json:"files,omitempty"` + ID uuid.UUID `json:"id" format:"uuid"` + OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` + OwnerID uuid.UUID `json:"owner_id" format:"uuid"` + WorkspaceID *uuid.UUID `json:"workspace_id,omitempty" format:"uuid"` + BuildID *uuid.UUID `json:"build_id,omitempty" format:"uuid"` + AgentID *uuid.UUID `json:"agent_id,omitempty" format:"uuid"` + ParentChatID *uuid.UUID `json:"parent_chat_id,omitempty" format:"uuid"` + RootChatID *uuid.UUID `json:"root_chat_id,omitempty" format:"uuid"` + LastModelConfigID uuid.UUID `json:"last_model_config_id" format:"uuid"` + Title string `json:"title"` + Status ChatStatus `json:"status"` + PlanMode ChatPlanMode `json:"plan_mode,omitempty"` + LastError *ChatLastError `json:"last_error,omitempty"` + DiffStatus *ChatDiffStatus `json:"diff_status,omitempty"` + CreatedAt time.Time `json:"created_at" format:"date-time"` + UpdatedAt time.Time `json:"updated_at" format:"date-time"` + Archived bool `json:"archived"` + PinOrder int32 `json:"pin_order"` + MCPServerIDs []uuid.UUID `json:"mcp_server_ids" format:"uuid"` + Labels map[string]string `json:"labels"` + Files []ChatFileMetadata `json:"files,omitempty"` // HasUnread is true when assistant messages exist beyond // the owner's read cursor, which updates on stream // connect and disconnect. @@ -1430,26 +1425,6 @@ type ChatStreamStatus struct { Status ChatStatus `json:"status"` } -// ChatLastError represents a persisted terminal chat error. -// -// Keep this aligned with ChatStreamError so persisted and live failures -// expose the same field set to clients. -type ChatLastError struct { - // Message is the normalized, user-facing error message. - Message string `json:"message"` - // Detail is optional provider-specific context shown alongside the - // normalized error message when available. - Detail string `json:"detail,omitempty"` - // Kind classifies the error for consistent client rendering. - Kind string `json:"kind,omitempty"` - // Provider identifies the upstream model provider when known. - Provider string `json:"provider,omitempty"` - // Retryable reports whether the underlying error is transient. - Retryable bool `json:"retryable"` - // StatusCode is the best-effort upstream HTTP status code. - StatusCode int `json:"status_code,omitempty"` -} - // ChatStreamError represents an error event in the stream. type ChatStreamError struct { // Message is the normalized, user-facing error message. @@ -1467,6 +1442,11 @@ type ChatStreamError struct { StatusCode int `json:"status_code,omitempty"` } +// ChatLastError represents a persisted terminal chat error. +// It is the same schema as ChatStreamError, surfaced from persisted chat +// state instead of the live stream. +type ChatLastError = ChatStreamError + // ChatStreamRetry represents an auto-retry status event in the stream. // Published when the server automatically retries a failed LLM call. type ChatStreamRetry struct { diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index c994a33540780..ba6f4d6469c3f 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -447,9 +447,8 @@ func TestChat_JSONRoundTrip(t *testing.T) { reviewerCount := int32(2) refreshedAt := now staleAt := now.Add(time.Hour) - lastError := "boom" - lastErrorPayload := &codersdk.ChatLastError{ - Message: lastError, + lastError := &codersdk.ChatLastError{ + Message: "boom", Detail: "provider detail", Kind: "generic", Provider: "openai", @@ -474,8 +473,7 @@ func TestChat_JSONRoundTrip(t *testing.T) { LastModelConfigID: uuid.New(), Title: "round-trip-test", Status: codersdk.ChatStatusRunning, - LastError: &lastError, - LastErrorPayload: lastErrorPayload, + LastError: lastError, CreatedAt: now, UpdatedAt: now, Archived: true, @@ -514,22 +512,6 @@ func TestChat_JSONRoundTrip(t *testing.T) { require.Equal(t, original, decoded) } -func TestChatLastErrorMatchesChatStreamError(t *testing.T) { - t.Parallel() - - lastErrorType := reflect.TypeFor[codersdk.ChatLastError]() - streamErrorType := reflect.TypeFor[codersdk.ChatStreamError]() - require.Equal(t, streamErrorType.NumField(), lastErrorType.NumField()) - - for i := range lastErrorType.NumField() { - lastErrorField := lastErrorType.Field(i) - streamErrorField := streamErrorType.Field(i) - require.Equal(t, streamErrorField.Name, lastErrorField.Name) - require.Equal(t, streamErrorField.Type, lastErrorField.Type) - require.Equal(t, streamErrorField.Tag, lastErrorField.Tag) - } -} - func TestNewDynamicTool(t *testing.T) { t.Parallel() diff --git a/docs/ai-coder/agents/chats-api.md b/docs/ai-coder/agents/chats-api.md index a2b523516a94a..4347524abb613 100644 --- a/docs/ai-coder/agents/chats-api.md +++ b/docs/ai-coder/agents/chats-api.md @@ -48,7 +48,6 @@ The response is the newly created `Chat` object: "last_model_config_id": "...", "title": "hello world", "status": "waiting", - "last_error": null, "diff_status": null, "created_at": "2025-07-17T00:00:00Z", "updated_at": "2025-07-17T00:00:00Z", diff --git a/enterprise/coderd/x/chatd/chatd_test.go b/enterprise/coderd/x/chatd/chatd_test.go index dc455f5580163..c5ef89bcd357c 100644 --- a/enterprise/coderd/x/chatd/chatd_test.go +++ b/enterprise/coderd/x/chatd/chatd_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/google/uuid" + "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" "golang.org/x/xerrors" @@ -30,16 +31,16 @@ import ( "github.com/coder/quartz" ) -func chatLastErrorMessage(raw database.Chat) string { - if !raw.LastError.Valid { +func chatLastErrorMessage(raw pqtype.NullRawMessage) string { + if !raw.Valid { return "" } var payload codersdk.ChatLastError - if err := json.Unmarshal(raw.LastError.RawMessage, &payload); err == nil && payload.Message != "" { + if err := json.Unmarshal(raw.RawMessage, &payload); err == nil && payload.Message != "" { return payload.Message } - return string(raw.LastError.RawMessage) + return string(raw.RawMessage) } func newTestServer( @@ -1724,14 +1725,14 @@ waitForStream: currentChat, dbErr := db.GetChatByID(ctx, chat.ID) if dbErr == nil && currentChat.Status == database.ChatStatusError { t.Fatalf("worker failed to process chat: status=%s last_error=%s", - currentChat.Status, chatLastErrorMessage(currentChat)) + currentChat.Status, chatLastErrorMessage(currentChat.LastError)) } case <-ctx.Done(): // Dump the final chat status for debugging. currentChat, dbErr := db.GetChatByID(context.Background(), chat.ID) if dbErr == nil { t.Fatalf("timed out waiting for worker to start streaming (chat status=%s, last_error=%q)", - currentChat.Status, chatLastErrorMessage(currentChat)) + currentChat.Status, chatLastErrorMessage(currentChat.LastError)) } t.Fatal("timed out waiting for worker to start streaming") } diff --git a/site/src/api/queries/chats.test.ts b/site/src/api/queries/chats.test.ts index eb0fb9c2bb838..5ba7549c31768 100644 --- a/site/src/api/queries/chats.test.ts +++ b/site/src/api/queries/chats.test.ts @@ -112,7 +112,6 @@ const makeChat = ( pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], ...overrides, }); diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 273c35e66bdb8..69fac0ececee2 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1286,16 +1286,7 @@ export interface Chat { readonly title: string; readonly status: ChatStatus; readonly plan_mode?: ChatPlanMode; - /** - * LastError is a backward-compatible one-line headline derived from - * LastErrorPayload.Message when a persisted chat error exists. - */ - readonly last_error: string | null; - /** - * LastErrorPayload is the structured persisted error state used to - * rehydrate failed chat callouts after refresh. - */ - readonly last_error_payload?: ChatLastError; + readonly last_error?: ChatStreamError; readonly diff_status?: ChatDiffStatus; readonly created_at: string; readonly updated_at: string; @@ -1823,9 +1814,8 @@ export const ChatInputPartTypes: ChatInputPartType[] = [ // From codersdk/chats.go /** * ChatLastError represents a persisted terminal chat error. - * - * Keep this aligned with ChatStreamError so persisted and live failures - * expose the same field set to clients. + * It is the same schema as ChatStreamError, surfaced from persisted chat + * state instead of the live stream. */ export interface ChatLastError { /** diff --git a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx index db2bd948332ce..7dc7d12d790f7 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx @@ -137,7 +137,6 @@ const baseChatFields = { pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], } as const; diff --git a/site/src/pages/AgentsPage/AgentChatPage.tsx b/site/src/pages/AgentsPage/AgentChatPage.tsx index cc39c1c51c8c3..2d7b20132bcd4 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.tsx @@ -57,6 +57,7 @@ import { } from "./AgentChatPageView"; import type { AgentsOutletContext } from "./AgentsPage"; import type { ChatMessageInputRef } from "./components/AgentChatInput"; +import { normalizeChatErrorPayload } from "./components/ChatConversation/chatError"; import { getParentChatID, getWorkspaceAgent, @@ -547,29 +548,13 @@ const getPersistedDetailError = ({ if (cachedError?.kind === "usage_limit") { return cachedError; } - if (chatStatus === "error") { - if (cachedError) { - return cachedError; - } - const lastErrorPayload = chatRecord?.last_error_payload; - const lastErrorMessage = lastErrorPayload?.message?.trim(); - const lastErrorDetail = lastErrorPayload?.detail?.trim(); - if (lastErrorPayload && lastErrorMessage) { - return { - message: lastErrorMessage, - kind: lastErrorPayload.kind?.trim() || "generic", - provider: lastErrorPayload.provider?.trim() || undefined, - retryable: lastErrorPayload.retryable, - statusCode: lastErrorPayload.status_code, - ...(lastErrorDetail ? { detail: lastErrorDetail } : {}), - }; - } - const lastError = chatRecord?.last_error?.trim(); - if (lastError) { - return { kind: "generic", message: lastError }; - } + if (chatStatus !== "error") { + return undefined; + } + if (cachedError) { + return cachedError; } - return undefined; + return normalizeChatErrorPayload(chatRecord?.last_error); }; /** diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index 2b6c9f3d4a44d..ce06574414567 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -62,7 +62,6 @@ const buildChat = (overrides: Partial = {}): TypesGen.Chat => ({ pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], ...overrides, }); diff --git a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx index 6f5756849676d..c7aca855f1db8 100644 --- a/site/src/pages/AgentsPage/AgentsPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentsPageView.stories.tsx @@ -145,7 +145,6 @@ const buildChat = (overrides: Partial = {}): Chat => ({ pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], ...overrides, }); @@ -426,7 +425,11 @@ export const WithChatList: Story = { id: "chat-3", title: "Fix database migration issue", status: "error", - last_error: "Connection timeout", + last_error: { + message: "Connection timeout", + kind: "generic", + retryable: false, + }, updated_at: todayTimestamp, }), buildChat({ diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx index 91657568020c4..36eed9310eb1f 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx @@ -133,7 +133,11 @@ const StatusAlert: FC<{ status: RetryOrFailedStatus }> = ({ status }) => { if (status.phase === "retrying") { metadataItems.push(Attempt {status.attempt}); } - if (status.phase === "failed" && status.statusCode !== undefined) { + if ( + status.phase === "failed" && + status.statusCode != null && + status.statusCode > 0 + ) { metadataItems.push(HTTP {status.statusCode}); } diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts new file mode 100644 index 0000000000000..2880efdf80b9e --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts @@ -0,0 +1,32 @@ +import type * as TypesGen from "#/api/typesGenerated"; +import type { ChatDetailError } from "../../utils/usageLimitMessage"; + +type StructuredChatError = TypesGen.ChatLastError | TypesGen.ChatStreamError; + +interface NormalizeChatErrorOptions { + fallbackMessage?: string; +} + +export const normalizeChatErrorPayload = ( + error: StructuredChatError | undefined, + options?: NormalizeChatErrorOptions, +): ChatDetailError | undefined => { + const fallbackMessage = options?.fallbackMessage?.trim(); + const message = error?.message?.trim() || fallbackMessage; + if (!message) { + return undefined; + } + const detail = error?.detail?.trim(); + const statusCode = + typeof error?.status_code === "number" && error.status_code > 0 + ? error.status_code + : undefined; + return { + message, + kind: error?.kind?.trim() || "generic", + provider: error?.provider?.trim() || undefined, + retryable: error?.retryable, + statusCode, + ...(detail ? { detail } : {}), + }; +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx index 9f9de1138856f..819533da2c548 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx @@ -218,7 +218,6 @@ const makeChat = (chatID: string): TypesGen.Chat => ({ pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], }); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts index 0e6e18a9172e6..01f355473a3ca 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts @@ -12,6 +12,7 @@ import type * as TypesGen from "#/api/typesGenerated"; import type { OneWayMessageEvent } from "#/utils/OneWayWebSocket"; import { createReconnectingWebSocket } from "#/utils/reconnectingWebSocket"; import type { ChatDetailError } from "../../utils/usageLimitMessage"; +import { normalizeChatErrorPayload } from "./chatError"; import { type ChatStore, type ChatStoreState, @@ -22,20 +23,6 @@ import { } from "./chatStore"; import type { RetryState } from "./types"; -const normalizeChatDetailError = ( - error: TypesGen.ChatStreamError | undefined, -): ChatDetailError => { - const detail = error?.detail?.trim(); - return { - message: error?.message.trim() || "Chat processing failed.", - kind: error?.kind?.trim() || "generic", - provider: error?.provider?.trim() || undefined, - retryable: error?.retryable, - statusCode: error?.status_code, - ...(detail ? { detail } : {}), - }; -}; - const normalizeRetryState = (retry: TypesGen.ChatStreamRetry): RetryState => ({ attempt: Math.max(1, retry.attempt), error: retry.error.trim() || "Retrying request shortly.", @@ -527,7 +514,9 @@ export const useChatStore = ( if (streamEvent.chat_id && streamEvent.chat_id !== chatID) { continue; } - const reason = normalizeChatDetailError(streamEvent.error); + const reason = normalizeChatErrorPayload(streamEvent.error, { + fallbackMessage: "Chat processing failed.", + }) ?? { kind: "generic", message: "Chat processing failed." }; store.setChatStatus("error"); store.setStreamError(reason); store.clearRetryState(); diff --git a/site/src/pages/AgentsPage/components/ChatTopBar.stories.tsx b/site/src/pages/AgentsPage/components/ChatTopBar.stories.tsx index 37d88aa1f2fe0..38dc30283d87b 100644 --- a/site/src/pages/AgentsPage/components/ChatTopBar.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatTopBar.stories.tsx @@ -65,7 +65,6 @@ export const WithParentChat: Story = { labels: {}, title: "Set up CI/CD pipeline", status: "completed", - last_error: null, created_at: "2026-02-18T00:00:00.000Z", updated_at: "2026-02-18T00:00:00.000Z", archived: false, diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.stories.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.stories.tsx index ed9aa34eb075c..11b5ba4fd3f84 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.stories.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.stories.tsx @@ -69,7 +69,6 @@ const buildChat = (overrides: Partial = {}): Chat => ({ pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, children: [], ...overrides, }); diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx index ca42fa0084a54..946dca27482a7 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx @@ -66,7 +66,6 @@ const buildChat = (overrides: Partial = {}): Chat => ({ pin_order: 0, has_unread: false, client_type: "ui", - last_error: null, mcp_server_ids: [], labels: {}, children: [], diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx index 78faff52d2f66..c13065ab16362 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx @@ -503,7 +503,7 @@ const ChatTreeNode: FC = ({ chat, isChildNode }) => { ); const errorReason = chat.status === "error" - ? chatErrorReasons[chat.id] || chat.last_error || undefined + ? chatErrorReasons[chat.id] || chat.last_error?.message || undefined : undefined; const subtitle = errorReason || modelName; const diffStatus = getChatDiffStatus(chat); From 5cb15ea0539aecba88e597cf064725c5f9189845 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 07:10:21 +0000 Subject: [PATCH 3/8] refactor: drop vestigial chat last-error scaffolding Removes leftover compatibility scaffolding from the chat last-error redesign: - normalizeChatErrorPayload no longer takes a fallbackMessage option; the live-stream "Chat processing failed." default lives at the only caller that needs it, and the redundant statusCode > 0 check in ChatStatusCallout is gone now that the normalizer filters bad codes. - Deletes chaterror.StreamErrorPayload, which was a thin alias for TerminalErrorPayload, and inlines the chatLastErrorMessage helper in cli/agents_list.go to match the file's existing nil-check style. - Renames legacyPayload to wantPayload in chatd_test.go and corrects stale "migration 474" comments in the chat last-error fixtures. - Adds a structured last_error example to docs/ai-coder/agents/chats-api.md. decodeChatLastError, the cached-error precedence in AgentChatPage, the encodeChatLastErrorPayload helper, and the LastError clears on chat status transitions are intentionally unchanged. --- cli/agents_list.go | 12 +++------ .../fixtures/000424_chat_last_error.up.sql | 2 +- .../000485_chat_last_error_jsonb.up.sql | 2 +- coderd/x/chatd/chatd.go | 2 +- coderd/x/chatd/chatd_internal_test.go | 2 +- coderd/x/chatd/chatd_test.go | 8 +++--- coderd/x/chatd/chaterror/payload.go | 4 --- coderd/x/chatd/chaterror/payload_test.go | 5 ++-- docs/ai-coder/agents/chats-api.md | 27 +++++++++++++++++++ .../ChatConversation/ChatStatusCallout.tsx | 6 +---- .../components/ChatConversation/chatError.ts | 8 +----- .../ChatConversation/useChatStore.ts | 7 ++--- 12 files changed, 46 insertions(+), 39 deletions(-) diff --git a/cli/agents_list.go b/cli/agents_list.go index fdaa05ab6e885..1d9912365c264 100644 --- a/cli/agents_list.go +++ b/cli/agents_list.go @@ -66,13 +66,6 @@ func (m chatListModel) searchQuery() string { return strings.TrimSpace(strings.ToLower(m.search.Value())) } -func chatLastErrorMessage(chat codersdk.Chat) string { - if chat.LastError == nil { - return "" - } - return chat.LastError.Message -} - func (m chatListModel) filteredChats() []codersdk.Chat { query := m.searchQuery() if query == "" { @@ -85,7 +78,7 @@ func (m chatListModel) filteredChats() []codersdk.Chat { filtered = append(filtered, chat) continue } - if lastError := chatLastErrorMessage(chat); strings.Contains(strings.ToLower(lastError), query) { + if chat.LastError != nil && strings.Contains(strings.ToLower(chat.LastError.Message), query) { filtered = append(filtered, chat) } } @@ -452,7 +445,8 @@ func (m chatListModel) View() string { rowText := fmt.Sprintf("%s%s %s %s%s", rowPrefix, rowStyle.Render(title), status, m.styles.dimmedText.Render(timeAgo(row.chat.UpdatedAt)), extra) lines = append(lines, rowText) - if lastError := chatLastErrorMessage(row.chat); row.chat.Status == codersdk.ChatStatusError && lastError != "" { + if row.chat.Status == codersdk.ChatStatusError && row.chat.LastError != nil && row.chat.LastError.Message != "" { + lastError := row.chat.LastError.Message errWidth := max(m.width-4, 20) errPrefix := " " if row.depth > 0 { diff --git a/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql index 09284437740b7..1feeacebc7678 100644 --- a/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000424_chat_last_error.up.sql @@ -1,5 +1,5 @@ -- Migration 424 adds chats.last_error as text. Seed one existing fixture --- chat with a legacy plain-text error so migration 474 has a non-null row +-- chat with a legacy plain-text error so migration 485 has a non-null row -- to backfill, and add a second chat that leaves last_error NULL so the -- migration fixture can assert both branches of the CASE expression. UPDATE chats diff --git a/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql b/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql index 08c416a9edd2d..d7d86cf17c4a9 100644 --- a/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000485_chat_last_error_jsonb.up.sql @@ -1,4 +1,4 @@ --- Migration 474 retypes chats.last_error to jsonb and backfills legacy +-- Migration 485 retypes chats.last_error to jsonb and backfills legacy -- text rows into the structured persisted payload shape. DO $$ DECLARE diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 66977b16854bb..5f08d68b67a0e 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -4856,7 +4856,7 @@ func (p *Server) publishRetry(chatID uuid.UUID, payload *codersdk.ChatStreamRetr } func (p *Server) publishError(chatID uuid.UUID, classified chaterror.ClassifiedError) { - payload := chaterror.StreamErrorPayload(classified) + payload := chaterror.TerminalErrorPayload(classified) if payload == nil { return } diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index e38d5a138bbfa..98c170def169c 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -2407,7 +2407,7 @@ func TestSubscribePrefersStructuredErrorPayloadViaPubsub(t *testing.T) { server.publishError(chatID, classified) event := requireStreamErrorEvent(t, events) - require.Equal(t, chaterror.StreamErrorPayload(classified), event.Error) + require.Equal(t, chaterror.TerminalErrorPayload(classified), event.Error) requireNoStreamEvent(t, events, 200*time.Millisecond) } diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 28b97aab75ab2..0a01140088653 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -3800,7 +3800,7 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { // Write a minimal structured last_error payload through the // query layer, then verify it round-trips through storage. errorMessage := "stream response: status 500: internal server error" - legacyPayload := codersdk.ChatLastError{ + wantPayload := codersdk.ChatLastError{ Message: errorMessage, Kind: "generic", } @@ -3810,17 +3810,17 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { WorkerID: uuid.NullUUID{}, StartedAt: sql.NullTime{}, HeartbeatAt: sql.NullTime{}, - LastError: mustChatLastErrorRawMessage(t, legacyPayload), + LastError: mustChatLastErrorRawMessage(t, wantPayload), }) require.NoError(t, err) require.Equal(t, database.ChatStatusError, chat.Status) - require.Equal(t, legacyPayload, requireChatLastErrorPayload(t, chat.LastError)) + require.Equal(t, wantPayload, requireChatLastErrorPayload(t, chat.LastError)) // Verify the error is persisted when re-read from the database. fromDB, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) require.Equal(t, database.ChatStatusError, fromDB.Status) - require.Equal(t, legacyPayload, requireChatLastErrorPayload(t, fromDB.LastError)) + require.Equal(t, wantPayload, requireChatLastErrorPayload(t, fromDB.LastError)) // Verify the error is cleared when the chat transitions to a // non-error status (e.g. pending after a retry). diff --git a/coderd/x/chatd/chaterror/payload.go b/coderd/x/chatd/chaterror/payload.go index 0e1e918799fc1..11b39a951bceb 100644 --- a/coderd/x/chatd/chaterror/payload.go +++ b/coderd/x/chatd/chaterror/payload.go @@ -20,10 +20,6 @@ func TerminalErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { } } -func StreamErrorPayload(classified ClassifiedError) *codersdk.ChatStreamError { - return TerminalErrorPayload(classified) -} - func StreamRetryPayload( attempt int, delay time.Duration, diff --git a/coderd/x/chatd/chaterror/payload_test.go b/coderd/x/chatd/chaterror/payload_test.go index 101d585c5515b..d0edd41bd2b53 100644 --- a/coderd/x/chatd/chaterror/payload_test.go +++ b/coderd/x/chatd/chaterror/payload_test.go @@ -28,10 +28,10 @@ func TestTerminalErrorPayloadUsesNormalizedClassification(t *testing.T) { }, payload) } -func TestStreamErrorPayloadIncludesProviderDetail(t *testing.T) { +func TestTerminalErrorPayloadIncludesProviderDetail(t *testing.T) { t.Parallel() - payload := chaterror.StreamErrorPayload(chaterror.Classify(testProviderError( + payload := chaterror.TerminalErrorPayload(chaterror.Classify(testProviderError( "", 400, nil, @@ -45,7 +45,6 @@ func TestTerminalErrorPayloadNilForEmptyClassification(t *testing.T) { t.Parallel() require.Nil(t, chaterror.TerminalErrorPayload(chaterror.ClassifiedError{})) - require.Nil(t, chaterror.StreamErrorPayload(chaterror.ClassifiedError{})) } func TestStreamRetryPayloadUsesNormalizedClassification(t *testing.T) { diff --git a/docs/ai-coder/agents/chats-api.md b/docs/ai-coder/agents/chats-api.md index 4347524abb613..568b04a7d695a 100644 --- a/docs/ai-coder/agents/chats-api.md +++ b/docs/ai-coder/agents/chats-api.md @@ -60,6 +60,33 @@ The response is the newly created `Chat` object: } ``` +If a chat later ends in error, the same `Chat` shape includes a structured +`last_error` object. For brevity, unchanged nullable IDs are omitted here: + +```json +{ + "id": "a1b2c3d4-...", + "title": "hello world", + "status": "error", + "last_error": { + "message": "Azure OpenAI is rate limiting requests.", + "kind": "rate_limit", + "provider": "azure", + "retryable": true, + "status_code": 429, + "detail": "Retry after 30 seconds." + }, + "created_at": "2025-07-17T00:00:00Z", + "updated_at": "2025-07-17T00:00:30Z", + "archived": false, + "pin_order": 0, + "mcp_server_ids": [], + "labels": {}, + "has_unread": false, + "client_type": "api" +} +``` + The agent begins processing the prompt asynchronously. Use the [stream endpoint](#stream-updates) to follow its progress. diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx index 36eed9310eb1f..38c29641281b3 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ChatStatusCallout.tsx @@ -133,11 +133,7 @@ const StatusAlert: FC<{ status: RetryOrFailedStatus }> = ({ status }) => { if (status.phase === "retrying") { metadataItems.push(Attempt {status.attempt}); } - if ( - status.phase === "failed" && - status.statusCode != null && - status.statusCode > 0 - ) { + if (status.phase === "failed" && status.statusCode != null) { metadataItems.push(HTTP {status.statusCode}); } diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts index 2880efdf80b9e..6c9790355a1fe 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts @@ -3,16 +3,10 @@ import type { ChatDetailError } from "../../utils/usageLimitMessage"; type StructuredChatError = TypesGen.ChatLastError | TypesGen.ChatStreamError; -interface NormalizeChatErrorOptions { - fallbackMessage?: string; -} - export const normalizeChatErrorPayload = ( error: StructuredChatError | undefined, - options?: NormalizeChatErrorOptions, ): ChatDetailError | undefined => { - const fallbackMessage = options?.fallbackMessage?.trim(); - const message = error?.message?.trim() || fallbackMessage; + const message = error?.message?.trim(); if (!message) { return undefined; } diff --git a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts index 01f355473a3ca..7fbaf98c4cca3 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts @@ -514,9 +514,10 @@ export const useChatStore = ( if (streamEvent.chat_id && streamEvent.chat_id !== chatID) { continue; } - const reason = normalizeChatErrorPayload(streamEvent.error, { - fallbackMessage: "Chat processing failed.", - }) ?? { kind: "generic", message: "Chat processing failed." }; + const reason = normalizeChatErrorPayload(streamEvent.error) ?? { + kind: "generic", + message: "Chat processing failed.", + }; store.setChatStatus("error"); store.setStreamError(reason); store.clearRetryState(); From fcf4faa99d053c57aa6d34f1cf2f1a855ba8481e Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 07:36:49 +0000 Subject: [PATCH 4/8] test(site): fix chat store fallback expectation --- .../AgentsPage/components/ChatConversation/chatStore.test.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx index 819533da2c548..baf68d6c23c72 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx @@ -1804,9 +1804,6 @@ describe("useChatStore", () => { expect(result.current.streamError).toEqual({ kind: "generic", message: "Chat processing failed.", - provider: undefined, - retryable: false, - statusCode: undefined, }); }); }); From d342d1bfa40a1ba9059106ab470ee82c5341e461 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 13:40:53 +0000 Subject: [PATCH 5/8] refactor: unify chat terminal error payload --- cli/agents_chat.go | 2 +- cli/agents_stream_test.go | 2 +- cli/agents_test.go | 2 +- coderd/database/db2sdk/db2sdk.go | 11 +- coderd/database/db2sdk/db2sdk_test.go | 12 +- coderd/pubsub/chatstreamnotify.go | 2 +- coderd/x/chatd/chatd.go | 14 +-- coderd/x/chatd/chatd_internal_test.go | 2 +- coderd/x/chatd/chatd_test.go | 12 +- coderd/x/chatd/chaterror/payload.go | 4 +- coderd/x/chatd/chaterror/payload_test.go | 2 +- coderd/x/chatd/subagent_internal_test.go | 2 +- codersdk/chats.go | 16 +-- codersdk/chats_test.go | 2 +- enterprise/coderd/x/chatd/chatd.go | 2 +- enterprise/coderd/x/chatd/chatd_test.go | 2 +- site/src/api/typesGenerated.ts | 103 ++++++------------ .../components/ChatConversation/chatError.ts | 4 +- 18 files changed, 79 insertions(+), 117 deletions(-) diff --git a/cli/agents_chat.go b/cli/agents_chat.go index f3bde23257591..82116590036a5 100644 --- a/cli/agents_chat.go +++ b/cli/agents_chat.go @@ -1287,7 +1287,7 @@ func (m chatViewModel) handleStreamEvent(event codersdk.ChatStreamEvent) (chatVi chatID: m.activeChatID, event: codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, - Error: &codersdk.ChatStreamError{ + Error: &codersdk.ChatError{ Message: fmt.Sprintf( "failed to parse ask_user_question: %v", err, diff --git a/cli/agents_stream_test.go b/cli/agents_stream_test.go index 95826db47499f..169e5118a0860 100644 --- a/cli/agents_stream_test.go +++ b/cli/agents_stream_test.go @@ -116,7 +116,7 @@ func TestConsumeChatStreamText(t *testing.T) { {Type: codersdk.ChatStreamEventTypeMessage, Message: &codersdk.ChatMessage{ID: 1, ChatID: uuid.New(), Role: codersdk.ChatMessageRoleAssistant, Content: []codersdk.ChatMessagePart{{Type: codersdk.ChatMessagePartTypeText, Text: "Hello world"}}}}, {Type: codersdk.ChatStreamEventTypeStatus, Status: &codersdk.ChatStreamStatus{Status: codersdk.ChatStatusRunning}}, {Type: codersdk.ChatStreamEventTypeRetry, Retry: &codersdk.ChatStreamRetry{Attempt: 2, Error: "rate limited"}}, - {Type: codersdk.ChatStreamEventTypeError, Error: &codersdk.ChatStreamError{Message: "boom"}}, + {Type: codersdk.ChatStreamEventTypeError, Error: &codersdk.ChatError{Message: "boom"}}, } { events <- event } diff --git a/cli/agents_test.go b/cli/agents_test.go index cc77da20a0a1c..8d08145f2ceb9 100644 --- a/cli/agents_test.go +++ b/cli/agents_test.go @@ -1070,7 +1070,7 @@ func TestAgents(t *testing.T) { t.Parallel() updated, cmd := applyStream(newTestChatViewModel(nil), codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, - Error: &codersdk.ChatStreamError{Message: "stream blew up"}, + Error: &codersdk.ChatError{Message: "stream blew up"}, }) require.Nil(t, cmd) require.Equal(t, "stream error: stream blew up", updated.err.Error()) diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 2f1294d8f02cb..94eae63b6927f 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -28,6 +28,7 @@ import ( "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/coderd/workspaceapps/appurl" + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -1609,16 +1610,16 @@ func nullTimePtr(v sql.NullTime) *time.Time { const fallbackChatLastErrorMessage = "The chat request failed unexpectedly." -func decodeChatLastError(raw pqtype.NullRawMessage) *codersdk.ChatLastError { +func decodeChatLastError(raw pqtype.NullRawMessage) *codersdk.ChatError { if !raw.Valid { return nil } - var payload codersdk.ChatLastError + var payload codersdk.ChatError if err := json.Unmarshal(raw.RawMessage, &payload); err != nil { - return &codersdk.ChatLastError{ + return &codersdk.ChatError{ Message: fallbackChatLastErrorMessage, - Kind: "generic", + Kind: chaterror.KindGeneric, } } @@ -1627,7 +1628,7 @@ func decodeChatLastError(raw pqtype.NullRawMessage) *codersdk.ChatLastError { payload.Kind = strings.TrimSpace(payload.Kind) payload.Provider = strings.TrimSpace(payload.Provider) if payload.Kind == "" { - payload.Kind = "generic" + payload.Kind = chaterror.KindGeneric } if payload.Message == "" { payload.Message = fallbackChatLastErrorMessage diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index 1a85b148a9f40..57394b03931e9 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -916,7 +916,7 @@ func TestChat_AllFieldsPopulated(t *testing.T) { // field to codersdk.Chat, this test will fail until the // converter is updated. now := dbtime.Now() - lastErrorPayload := codersdk.ChatLastError{ + lastErrorPayload := codersdk.ChatError{ Message: "boom", Detail: "provider detail", Kind: "generic", @@ -1074,12 +1074,12 @@ func TestChat_LastErrorFallback(t *testing.T) { tests := []struct { name string raw json.RawMessage - expectPayload *codersdk.ChatLastError + expectPayload *codersdk.ChatError }{ { name: "MalformedJSON", raw: json.RawMessage(`{`), - expectPayload: &codersdk.ChatLastError{ + expectPayload: &codersdk.ChatError{ Message: fallbackMessage, Kind: "generic", Retryable: false, @@ -1088,7 +1088,7 @@ func TestChat_LastErrorFallback(t *testing.T) { { name: "MessageMissingPreservesMetadata", raw: json.RawMessage(`{"kind":"timeout","provider":"openai","status_code":504}`), - expectPayload: &codersdk.ChatLastError{ + expectPayload: &codersdk.ChatError{ Message: fallbackMessage, Kind: "timeout", Provider: "openai", @@ -1099,7 +1099,7 @@ func TestChat_LastErrorFallback(t *testing.T) { { name: "WhitespaceMessageDefaultsKind", raw: json.RawMessage(`{"message":" ","provider":"openai"}`), - expectPayload: &codersdk.ChatLastError{ + expectPayload: &codersdk.ChatError{ Message: fallbackMessage, Kind: "generic", Provider: "openai", @@ -1109,7 +1109,7 @@ func TestChat_LastErrorFallback(t *testing.T) { { name: "KindMissingDefaultsGeneric", raw: json.RawMessage(`{"message":"OpenAI returned an unexpected error.","provider":"openai","status_code":502}`), - expectPayload: &codersdk.ChatLastError{ + expectPayload: &codersdk.ChatError{ Message: "OpenAI returned an unexpected error.", Kind: "generic", Provider: "openai", diff --git a/coderd/pubsub/chatstreamnotify.go b/coderd/pubsub/chatstreamnotify.go index b39ca2cda70e9..d53605d29c07b 100644 --- a/coderd/pubsub/chatstreamnotify.go +++ b/coderd/pubsub/chatstreamnotify.go @@ -40,7 +40,7 @@ type ChatStreamNotifyMessage struct { // ErrorPayload carries a structured error event for cross-replica // live delivery. Keep Error for backward compatibility with older // replicas during rolling deploys. - ErrorPayload *codersdk.ChatStreamError `json:"error_payload,omitempty"` + ErrorPayload *codersdk.ChatError `json:"error_payload,omitempty"` // Error is the legacy string-only error payload kept for mixed- // version compatibility during rollout. diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 5f08d68b67a0e..adebbc834b977 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -4364,7 +4364,7 @@ func (p *Server) Subscribe( initialSnapshot = append(initialSnapshot, codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{Message: "failed to load initial snapshot"}, + Error: &codersdk.ChatError{Message: "failed to load initial snapshot"}, }) } else { for _, msg := range messages { @@ -4387,7 +4387,7 @@ func (p *Server) Subscribe( initialSnapshot = append(initialSnapshot, codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{Message: "failed to load initial snapshot"}, + Error: &codersdk.ChatError{Message: "failed to load initial snapshot"}, }) } else if len(queued) > 0 { initialSnapshot = append(initialSnapshot, codersdk.ChatStreamEvent{ @@ -4412,7 +4412,7 @@ func (p *Server) Subscribe( initialSnapshot = append(initialSnapshot, codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{Message: "failed to load initial snapshot"}, + Error: &codersdk.ChatError{Message: "failed to load initial snapshot"}, }) } else { statusEvent := codersdk.ChatStreamEvent{ @@ -4490,7 +4490,7 @@ func (p *Server) Subscribe( case mergedEvents <- codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{ + Error: &codersdk.ChatError{ Message: psErr.Error(), }, }: @@ -4593,7 +4593,7 @@ func (p *Server) Subscribe( case mergedEvents <- codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{ + Error: &codersdk.ChatError{ Message: notify.Error, }, }: @@ -4882,7 +4882,7 @@ func processingFailure(err error) (chaterror.ClassifiedError, bool) { return classified, true } -func encodeChatLastErrorPayload(payload *codersdk.ChatLastError) (pqtype.NullRawMessage, error) { +func encodeChatLastErrorPayload(payload *codersdk.ChatError) (pqtype.NullRawMessage, error) { if payload == nil { return pqtype.NullRawMessage{}, nil } @@ -5344,7 +5344,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { // Determine the final status and last error payload to set when we're done. status := database.ChatStatusWaiting wasInterrupted := false - var lastErrorPayload *codersdk.ChatLastError + var lastErrorPayload *codersdk.ChatError generatedTitle := &generatedChatTitle{} runResult := runChatResult{} remainingQueuedMessages := []database.ChatQueuedMessage{} diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index 98c170def169c..dd3180eae2071 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -2442,7 +2442,7 @@ func TestSubscribeFallsBackToLegacyErrorStringViaPubsub(t *testing.T) { }) event := requireStreamErrorEvent(t, events) - require.Equal(t, &codersdk.ChatStreamError{Message: "legacy error only"}, event.Error) + require.Equal(t, &codersdk.ChatError{Message: "legacy error only"}, event.Error) requireNoStreamEvent(t, events, 200*time.Millisecond) } diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 0a01140088653..40d011ffab231 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -70,7 +70,7 @@ func openAIToolName(tool chattest.OpenAITool) string { return cmp.Or(tool.Function.Name, tool.Name, tool.Type) } -func mustChatLastErrorRawMessage(t testing.TB, payload codersdk.ChatLastError) pqtype.NullRawMessage { +func mustChatLastErrorRawMessage(t testing.TB, payload codersdk.ChatError) pqtype.NullRawMessage { t.Helper() encoded, err := json.Marshal(payload) @@ -78,11 +78,11 @@ func mustChatLastErrorRawMessage(t testing.TB, payload codersdk.ChatLastError) p return pqtype.NullRawMessage{RawMessage: encoded, Valid: true} } -func requireChatLastErrorPayload(t testing.TB, raw pqtype.NullRawMessage) codersdk.ChatLastError { +func requireChatLastErrorPayload(t testing.TB, raw pqtype.NullRawMessage) codersdk.ChatError { t.Helper() require.True(t, raw.Valid, "last error should be set") - var payload codersdk.ChatLastError + var payload codersdk.ChatError require.NoError(t, json.Unmarshal(raw.RawMessage, &payload)) return payload } @@ -92,7 +92,7 @@ func chatLastErrorMessage(raw pqtype.NullRawMessage) string { return "" } - var payload codersdk.ChatLastError + var payload codersdk.ChatError if err := json.Unmarshal(raw.RawMessage, &payload); err == nil && payload.Message != "" { return payload.Message } @@ -3687,7 +3687,7 @@ func TestRecoverStaleRequiresActionChat(t *testing.T) { }, testutil.WaitMedium, testutil.IntervalFast) persistedError := requireChatLastErrorPayload(t, chatResult.LastError) - require.Equal(t, codersdk.ChatLastError{ + require.Equal(t, codersdk.ChatError{ Message: "Dynamic tool execution timed out", Kind: "generic", }, persistedError) @@ -3800,7 +3800,7 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { // Write a minimal structured last_error payload through the // query layer, then verify it round-trips through storage. errorMessage := "stream response: status 500: internal server error" - wantPayload := codersdk.ChatLastError{ + wantPayload := codersdk.ChatError{ Message: errorMessage, Kind: "generic", } diff --git a/coderd/x/chatd/chaterror/payload.go b/coderd/x/chatd/chaterror/payload.go index 11b39a951bceb..6262384525c45 100644 --- a/coderd/x/chatd/chaterror/payload.go +++ b/coderd/x/chatd/chaterror/payload.go @@ -6,11 +6,11 @@ import ( "github.com/coder/coder/v2/codersdk" ) -func TerminalErrorPayload(classified ClassifiedError) *codersdk.ChatLastError { +func TerminalErrorPayload(classified ClassifiedError) *codersdk.ChatError { if classified.Message == "" { return nil } - return &codersdk.ChatLastError{ + return &codersdk.ChatError{ Message: classified.Message, Detail: classified.Detail, Kind: classified.Kind, diff --git a/coderd/x/chatd/chaterror/payload_test.go b/coderd/x/chatd/chaterror/payload_test.go index d0edd41bd2b53..7aa21e6500c54 100644 --- a/coderd/x/chatd/chaterror/payload_test.go +++ b/coderd/x/chatd/chaterror/payload_test.go @@ -19,7 +19,7 @@ func TestTerminalErrorPayloadUsesNormalizedClassification(t *testing.T) { ) payload := chaterror.TerminalErrorPayload(classified) - require.Equal(t, &codersdk.ChatLastError{ + require.Equal(t, &codersdk.ChatError{ Message: "Azure OpenAI is rate limiting requests.", Kind: chaterror.KindRateLimit, Provider: "azure", diff --git a/coderd/x/chatd/subagent_internal_test.go b/coderd/x/chatd/subagent_internal_test.go index 480454fe29639..404951dba8865 100644 --- a/coderd/x/chatd/subagent_internal_test.go +++ b/coderd/x/chatd/subagent_internal_test.go @@ -2716,7 +2716,7 @@ func setChatStatus( Status: status, } if lastError != "" { - encodedLastError, err := json.Marshal(codersdk.ChatLastError{ + encodedLastError, err := json.Marshal(codersdk.ChatError{ Message: lastError, Kind: "generic", }) diff --git a/codersdk/chats.go b/codersdk/chats.go index 74b87093bab92..cc372f72fdfdb 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -109,7 +109,7 @@ type Chat struct { Title string `json:"title"` Status ChatStatus `json:"status"` PlanMode ChatPlanMode `json:"plan_mode,omitempty"` - LastError *ChatLastError `json:"last_error,omitempty"` + LastError *ChatError `json:"last_error,omitempty"` DiffStatus *ChatDiffStatus `json:"diff_status,omitempty"` CreatedAt time.Time `json:"created_at" format:"date-time"` UpdatedAt time.Time `json:"updated_at" format:"date-time"` @@ -1425,8 +1425,9 @@ type ChatStreamStatus struct { Status ChatStatus `json:"status"` } -// ChatStreamError represents an error event in the stream. -type ChatStreamError struct { +// ChatError represents a terminal chat error in persisted chat state or the +// live stream. +type ChatError struct { // Message is the normalized, user-facing error message. Message string `json:"message"` // Detail is optional provider-specific context shown alongside the @@ -1442,11 +1443,6 @@ type ChatStreamError struct { StatusCode int `json:"status_code,omitempty"` } -// ChatLastError represents a persisted terminal chat error. -// It is the same schema as ChatStreamError, surfaced from persisted chat -// state instead of the live stream. -type ChatLastError = ChatStreamError - // ChatStreamRetry represents an auto-retry status event in the stream. // Published when the server automatically retries a failed LLM call. type ChatStreamRetry struct { @@ -1579,7 +1575,7 @@ type ChatStreamEvent struct { Message *ChatMessage `json:"message,omitempty"` MessagePart *ChatStreamMessagePart `json:"message_part,omitempty"` Status *ChatStreamStatus `json:"status,omitempty"` - Error *ChatStreamError `json:"error,omitempty"` + Error *ChatError `json:"error,omitempty"` Retry *ChatStreamRetry `json:"retry,omitempty"` QueuedMessages []ChatQueuedMessage `json:"queued_messages,omitempty"` ActionRequired *ChatStreamActionRequired `json:"action_required,omitempty"` @@ -2656,7 +2652,7 @@ func (c *ExperimentalClient) StreamChat(ctx context.Context, chatID uuid.UUID, o } _ = send(ChatStreamEvent{ Type: ChatStreamEventTypeError, - Error: &ChatStreamError{ + Error: &ChatError{ Message: fmt.Sprintf("read chat stream: %v", err), }, }) diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index ba6f4d6469c3f..68d6e0ecce8af 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -447,7 +447,7 @@ func TestChat_JSONRoundTrip(t *testing.T) { reviewerCount := int32(2) refreshedAt := now staleAt := now.Add(time.Hour) - lastError := &codersdk.ChatLastError{ + lastError := &codersdk.ChatError{ Message: "boom", Detail: "provider detail", Kind: "generic", diff --git a/enterprise/coderd/x/chatd/chatd.go b/enterprise/coderd/x/chatd/chatd.go index ce1a002723bec..e407e0e23dc66 100644 --- a/enterprise/coderd/x/chatd/chatd.go +++ b/enterprise/coderd/x/chatd/chatd.go @@ -394,7 +394,7 @@ func NewMultiReplicaSubscribeFn( case mergedEvents <- codersdk.ChatStreamEvent{ Type: codersdk.ChatStreamEventTypeError, ChatID: chatID, - Error: &codersdk.ChatStreamError{Message: msg}, + Error: &codersdk.ChatError{Message: msg}, }: case <-ctx.Done(): } diff --git a/enterprise/coderd/x/chatd/chatd_test.go b/enterprise/coderd/x/chatd/chatd_test.go index c5ef89bcd357c..3345819696006 100644 --- a/enterprise/coderd/x/chatd/chatd_test.go +++ b/enterprise/coderd/x/chatd/chatd_test.go @@ -36,7 +36,7 @@ func chatLastErrorMessage(raw pqtype.NullRawMessage) string { return "" } - var payload codersdk.ChatLastError + var payload codersdk.ChatError if err := json.Unmarshal(raw.RawMessage, &payload); err == nil && payload.Message != "" { return payload.Message } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 69fac0ececee2..00f9617ab0aaa 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1286,7 +1286,7 @@ export interface Chat { readonly title: string; readonly status: ChatStatus; readonly plan_mode?: ChatPlanMode; - readonly last_error?: ChatStreamError; + readonly last_error?: ChatError; readonly diff_status?: ChatDiffStatus; readonly created_at: string; readonly updated_at: string; @@ -1679,6 +1679,39 @@ export interface ChatDiffStatus { readonly stale_at?: string; } +// From codersdk/chats.go +/** + * ChatError represents a terminal chat error in persisted chat state or the + * live stream. + */ +export interface ChatError { + /** + * Message is the normalized, user-facing error message. + */ + readonly message: string; + /** + * Detail is optional provider-specific context shown alongside the + * normalized error message when available. + */ + readonly detail?: string; + /** + * Kind classifies the error for consistent client rendering. + */ + readonly kind?: string; + /** + * Provider identifies the upstream model provider when known. + */ + readonly provider?: string; + /** + * Retryable reports whether the underlying error is transient. + */ + readonly retryable: boolean; + /** + * StatusCode is the best-effort upstream HTTP status code. + */ + readonly status_code?: number; +} + // From codersdk/chats.go /** * ChatFileMetadata contains lightweight metadata about a file @@ -1811,40 +1844,6 @@ export const ChatInputPartTypes: ChatInputPartType[] = [ "text", ]; -// From codersdk/chats.go -/** - * ChatLastError represents a persisted terminal chat error. - * It is the same schema as ChatStreamError, surfaced from persisted chat - * state instead of the live stream. - */ -export interface ChatLastError { - /** - * Message is the normalized, user-facing error message. - */ - readonly message: string; - /** - * Detail is optional provider-specific context shown alongside the - * normalized error message when available. - */ - readonly detail?: string; - /** - * Kind classifies the error for consistent client rendering. - */ - readonly kind?: string; - /** - * Provider identifies the upstream model provider when known. - */ - readonly provider?: string; - /** - * Retryable reports whether the underlying error is transient. - */ - readonly retryable: boolean; - /** - * StatusCode is the best-effort upstream HTTP status code. - */ - readonly status_code?: number; -} - // From codersdk/chats.go /** * ChatMessage represents a single message in a chat. @@ -2415,38 +2414,6 @@ export interface ChatStreamActionRequired { readonly tool_calls: readonly ChatStreamToolCall[]; } -// From codersdk/chats.go -/** - * ChatStreamError represents an error event in the stream. - */ -export interface ChatStreamError { - /** - * Message is the normalized, user-facing error message. - */ - readonly message: string; - /** - * Detail is optional provider-specific context shown alongside the - * normalized error message when available. - */ - readonly detail?: string; - /** - * Kind classifies the error for consistent client rendering. - */ - readonly kind?: string; - /** - * Provider identifies the upstream model provider when known. - */ - readonly provider?: string; - /** - * Retryable reports whether the underlying error is transient. - */ - readonly retryable: boolean; - /** - * StatusCode is the best-effort upstream HTTP status code. - */ - readonly status_code?: number; -} - // From codersdk/chats.go /** * ChatStreamEvent represents a real-time update for chat streaming. @@ -2457,7 +2424,7 @@ export interface ChatStreamEvent { readonly message?: ChatMessage; readonly message_part?: ChatStreamMessagePart; readonly status?: ChatStreamStatus; - readonly error?: ChatStreamError; + readonly error?: ChatError; readonly retry?: ChatStreamRetry; readonly queued_messages?: readonly ChatQueuedMessage[]; readonly action_required?: ChatStreamActionRequired; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts index 6c9790355a1fe..e8c396e5abfea 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/chatError.ts @@ -1,10 +1,8 @@ import type * as TypesGen from "#/api/typesGenerated"; import type { ChatDetailError } from "../../utils/usageLimitMessage"; -type StructuredChatError = TypesGen.ChatLastError | TypesGen.ChatStreamError; - export const normalizeChatErrorPayload = ( - error: StructuredChatError | undefined, + error: TypesGen.ChatError | undefined, ): ChatDetailError | undefined => { const message = error?.message?.trim(); if (!message) { From fe127f75a114f35996e11d2e6db418cfcb8708a2 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 13:49:16 +0000 Subject: [PATCH 6/8] test: use chat generic kind constant --- coderd/database/db2sdk/db2sdk_test.go | 9 +++++---- coderd/x/chatd/chatd_test.go | 9 +++++---- coderd/x/chatd/subagent_internal_test.go | 3 ++- codersdk/chats_test.go | 3 ++- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index 57394b03931e9..41a0d0e3d57ee 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk/proto" ) @@ -919,7 +920,7 @@ func TestChat_AllFieldsPopulated(t *testing.T) { lastErrorPayload := codersdk.ChatError{ Message: "boom", Detail: "provider detail", - Kind: "generic", + Kind: chaterror.KindGeneric, Provider: "openai", Retryable: true, StatusCode: 503, @@ -1081,7 +1082,7 @@ func TestChat_LastErrorFallback(t *testing.T) { raw: json.RawMessage(`{`), expectPayload: &codersdk.ChatError{ Message: fallbackMessage, - Kind: "generic", + Kind: chaterror.KindGeneric, Retryable: false, }, }, @@ -1101,7 +1102,7 @@ func TestChat_LastErrorFallback(t *testing.T) { raw: json.RawMessage(`{"message":" ","provider":"openai"}`), expectPayload: &codersdk.ChatError{ Message: fallbackMessage, - Kind: "generic", + Kind: chaterror.KindGeneric, Provider: "openai", Retryable: false, }, @@ -1111,7 +1112,7 @@ func TestChat_LastErrorFallback(t *testing.T) { raw: json.RawMessage(`{"message":"OpenAI returned an unexpected error.","provider":"openai","status_code":502}`), expectPayload: &codersdk.ChatError{ Message: "OpenAI returned an unexpected error.", - Kind: "generic", + Kind: chaterror.KindGeneric, Provider: "openai", Retryable: false, StatusCode: 502, diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 40d011ffab231..f64a013fa3e66 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -46,6 +46,7 @@ import ( "github.com/coder/coder/v2/coderd/workspacestats" "github.com/coder/coder/v2/coderd/x/chatd" "github.com/coder/coder/v2/coderd/x/chatd/chatadvisor" + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/coderd/x/chatd/chattest" "github.com/coder/coder/v2/coderd/x/chatd/chattool" @@ -3689,7 +3690,7 @@ func TestRecoverStaleRequiresActionChat(t *testing.T) { persistedError := requireChatLastErrorPayload(t, chatResult.LastError) require.Equal(t, codersdk.ChatError{ Message: "Dynamic tool execution timed out", - Kind: "generic", + Kind: chaterror.KindGeneric, }, persistedError) require.False(t, chatResult.WorkerID.Valid) } @@ -3802,7 +3803,7 @@ func TestUpdateChatStatusPersistsLastError(t *testing.T) { errorMessage := "stream response: status 500: internal server error" wantPayload := codersdk.ChatError{ Message: errorMessage, - Kind: "generic", + Kind: chaterror.KindGeneric, } chat, err := db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{ ID: chat.ID, @@ -7076,7 +7077,7 @@ func TestProcessChat_UserProviderKey_MissingKeyError(t *testing.T) { persistedError := requireChatLastErrorPayload(t, chatResult.LastError) require.NotEmpty(t, persistedError.Message) require.NotContains(t, persistedError.Message, "panicked") - require.Equal(t, "generic", persistedError.Kind) + require.Equal(t, chaterror.KindGeneric, persistedError.Kind) require.NotEqual(t, database.ChatStatusRunning, chatResult.Status) require.Zero(t, llmCalls.Load(), "missing user key should fail before any LLM request") } @@ -7141,7 +7142,7 @@ func TestProcessChatPanicRecovery(t *testing.T) { persistedError := requireChatLastErrorPayload(t, chatResult.LastError) require.Contains(t, persistedError.Message, "chat processing panicked") require.Contains(t, persistedError.Message, "intentional test panic") - require.Equal(t, "generic", persistedError.Kind) + require.Equal(t, chaterror.KindGeneric, persistedError.Kind) } // panicOnInTxDB wraps a database.Store and panics on the first InTx diff --git a/coderd/x/chatd/subagent_internal_test.go b/coderd/x/chatd/subagent_internal_test.go index 404951dba8865..f536f44d33ed9 100644 --- a/coderd/x/chatd/subagent_internal_test.go +++ b/coderd/x/chatd/subagent_internal_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/pubsub" coderdpubsub "github.com/coder/coder/v2/coderd/pubsub" + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/coderd/x/chatd/chatloop" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" @@ -2718,7 +2719,7 @@ func setChatStatus( if lastError != "" { encodedLastError, err := json.Marshal(codersdk.ChatError{ Message: lastError, - Kind: "generic", + Kind: chaterror.KindGeneric, }) require.NoError(t, err) params.LastError = pqtype.NullRawMessage{RawMessage: encodedLastError, Valid: true} diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 68d6e0ecce8af..307d18a8e6449 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/codersdk" ) @@ -450,7 +451,7 @@ func TestChat_JSONRoundTrip(t *testing.T) { lastError := &codersdk.ChatError{ Message: "boom", Detail: "provider detail", - Kind: "generic", + Kind: chaterror.KindGeneric, Provider: "openai", Retryable: true, StatusCode: 503, From 624130bac3b4686d00700dc8ab03cf4df20d80c6 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 14:07:37 +0000 Subject: [PATCH 7/8] fix(codersdk/chats_test.go): keep generic kind inline to satisfy import lint --- codersdk/chats_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 307d18a8e6449..9cd3ece0ea9aa 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -16,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/codersdk" ) @@ -449,9 +448,14 @@ func TestChat_JSONRoundTrip(t *testing.T) { refreshedAt := now staleAt := now.Add(time.Hour) lastError := &codersdk.ChatError{ - Message: "boom", - Detail: "provider detail", - Kind: chaterror.KindGeneric, + Message: "boom", + Detail: "provider detail", + // Kind is the inline literal "generic" rather than + // chaterror.KindGeneric because codersdk (including its + // test imports) cannot reach packages that depend on + // modules replaced in go.mod, and chaterror imports + // charm.land/fantasy. + Kind: "generic", Provider: "openai", Retryable: true, StatusCode: 503, From 0b82b792804c5a7a101ac747a98882633bf6615d Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 4 May 2026 14:10:42 +0000 Subject: [PATCH 8/8] test(codersdk/chats_test.go): drop redundant inline-kind comment --- codersdk/chats_test.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 9cd3ece0ea9aa..68d6e0ecce8af 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -448,13 +448,8 @@ func TestChat_JSONRoundTrip(t *testing.T) { refreshedAt := now staleAt := now.Add(time.Hour) lastError := &codersdk.ChatError{ - Message: "boom", - Detail: "provider detail", - // Kind is the inline literal "generic" rather than - // chaterror.KindGeneric because codersdk (including its - // test imports) cannot reach packages that depend on - // modules replaced in go.mod, and chaterror imports - // charm.land/fantasy. + Message: "boom", + Detail: "provider detail", Kind: "generic", Provider: "openai", Retryable: true,