diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 8f87e219d2..8f8980c65c 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -1516,6 +1516,85 @@ func nullInt64Ptr(v sql.NullInt64) *int64 { return &value } +// 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. +func Chat(c database.Chat, diffStatus *database.ChatDiffStatus) codersdk.Chat { + mcpServerIDs := c.MCPServerIDs + if mcpServerIDs == nil { + mcpServerIDs = []uuid.UUID{} + } + labels := map[string]string(c.Labels) + if labels == nil { + labels = map[string]string{} + } + chat := codersdk.Chat{ + ID: c.ID, + OwnerID: c.OwnerID, + LastModelConfigID: c.LastModelConfigID, + Title: c.Title, + Status: codersdk.ChatStatus(c.Status), + Archived: c.Archived, + CreatedAt: c.CreatedAt, + UpdatedAt: c.UpdatedAt, + MCPServerIDs: mcpServerIDs, + Labels: labels, + } + if c.LastError.Valid { + chat.LastError = &c.LastError.String + } + if c.ParentChatID.Valid { + parentChatID := c.ParentChatID.UUID + chat.ParentChatID = &parentChatID + } + switch { + case c.RootChatID.Valid: + rootChatID := c.RootChatID.UUID + chat.RootChatID = &rootChatID + case c.ParentChatID.Valid: + rootChatID := c.ParentChatID.UUID + chat.RootChatID = &rootChatID + default: + rootChatID := c.ID + chat.RootChatID = &rootChatID + } + if c.WorkspaceID.Valid { + chat.WorkspaceID = &c.WorkspaceID.UUID + } + if c.BuildID.Valid { + chat.BuildID = &c.BuildID.UUID + } + if c.AgentID.Valid { + chat.AgentID = &c.AgentID.UUID + } + if diffStatus != nil { + convertedDiffStatus := ChatDiffStatus(c.ID, diffStatus) + chat.DiffStatus = &convertedDiffStatus + } + return chat +} + +// Chats converts a slice of database.Chat to codersdk.Chat, looking +// up diff statuses from the provided map. When diffStatusesByChatID +// is non-nil, chats without an entry receive an empty DiffStatus. +func Chats(chats []database.Chat, diffStatusesByChatID map[uuid.UUID]database.ChatDiffStatus) []codersdk.Chat { + result := make([]codersdk.Chat, len(chats)) + for i, c := range chats { + diffStatus, ok := diffStatusesByChatID[c.ID] + if ok { + result[i] = Chat(c, &diffStatus) + continue + } + + result[i] = Chat(c, nil) + if diffStatusesByChatID != nil { + emptyDiffStatus := ChatDiffStatus(c.ID, nil) + result[i].DiffStatus = &emptyDiffStatus + } + } + return result +} + // ChatDiffStatus converts a database.ChatDiffStatus to a // codersdk.ChatDiffStatus. When status is nil an empty value // containing only the chatID is returned. diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index 3b98e185ff..9c1b4f109d 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -5,6 +5,7 @@ import ( "database/sql" "encoding/json" "fmt" + "reflect" "testing" "time" @@ -513,6 +514,54 @@ func TestChatQueuedMessage_ParsesUserContentParts(t *testing.T) { require.Equal(t, "queued text", queued.Content[0].Text) } +func TestChat_AllFieldsPopulated(t *testing.T) { + t.Parallel() + + // Every field of database.Chat is set to a non-zero value so + // that the reflection check below catches any field that + // db2sdk.Chat forgets to populate. When someone adds a new + // field to codersdk.Chat, this test will fail until the + // converter is updated. + now := dbtime.Now() + input := database.Chat{ + ID: uuid.New(), + OwnerID: uuid.New(), + WorkspaceID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + BuildID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + AgentID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + ParentChatID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + RootChatID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + LastModelConfigID: uuid.New(), + Title: "all-fields-test", + Status: database.ChatStatusRunning, + LastError: sql.NullString{String: "boom", Valid: true}, + CreatedAt: now, + UpdatedAt: now, + Archived: true, + MCPServerIDs: []uuid.UUID{uuid.New()}, + Labels: database.StringMap{"env": "prod"}, + } + // Only ChatID is needed here. This test checks that + // Chat.DiffStatus is non-nil, not that every DiffStatus + // field is populated — that would be a separate test for + // the ChatDiffStatus converter. + diffStatus := &database.ChatDiffStatus{ + ChatID: input.ID, + } + + got := db2sdk.Chat(input, diffStatus) + + v := reflect.ValueOf(got) + typ := v.Type() + for i := range typ.NumField() { + field := typ.Field(i) + require.False(t, v.Field(i).IsZero(), + "codersdk.Chat field %q is zero-valued — db2sdk.Chat may not be populating it", + field.Name, + ) + } +} + func TestChatQueuedMessage_MalformedContent(t *testing.T) { t.Parallel() diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 4b208c500c..573a565fa9 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -354,7 +354,7 @@ func (api *API) listChats(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, convertChats(chats, diffStatusesByChatID)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Chats(chats, diffStatusesByChatID)) } func (api *API) getChatDiffStatusesByChatID( @@ -499,7 +499,7 @@ func (api *API) postChats(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusCreated, convertChat(chat, nil)) + httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Chat(chat, nil)) } // EXPERIMENTAL: this endpoint is experimental and is subject to change. @@ -1224,7 +1224,7 @@ func (api *API) getChat(rw http.ResponseWriter, r *http.Request) { slog.Error(err), ) } - httpapi.Write(ctx, rw, http.StatusOK, convertChat(chat, diffStatus)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Chat(chat, diffStatus)) } // EXPERIMENTAL: this endpoint is experimental and is subject to change. @@ -2054,7 +2054,7 @@ func (api *API) interruptChat(rw http.ResponseWriter, r *http.Request) { chat = updatedChat } - httpapi.Write(ctx, rw, http.StatusOK, convertChat(chat, nil)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Chat(chat, nil)) } // EXPERIMENTAL: this endpoint is experimental and is subject to change. @@ -3696,79 +3696,6 @@ func truncateRunes(value string, maxLen int) string { return string(runes[:maxLen]) } -func convertChat(c database.Chat, diffStatus *database.ChatDiffStatus) codersdk.Chat { - mcpServerIDs := c.MCPServerIDs - if mcpServerIDs == nil { - mcpServerIDs = []uuid.UUID{} - } - labels := map[string]string(c.Labels) - if labels == nil { - labels = map[string]string{} - } - chat := codersdk.Chat{ - ID: c.ID, - OwnerID: c.OwnerID, - LastModelConfigID: c.LastModelConfigID, - Title: c.Title, - Status: codersdk.ChatStatus(c.Status), - Archived: c.Archived, - CreatedAt: c.CreatedAt, - UpdatedAt: c.UpdatedAt, - MCPServerIDs: mcpServerIDs, - Labels: labels, - } - if c.LastError.Valid { - chat.LastError = &c.LastError.String - } - if c.ParentChatID.Valid { - parentChatID := c.ParentChatID.UUID - chat.ParentChatID = &parentChatID - } - switch { - case c.RootChatID.Valid: - rootChatID := c.RootChatID.UUID - chat.RootChatID = &rootChatID - case c.ParentChatID.Valid: - rootChatID := c.ParentChatID.UUID - chat.RootChatID = &rootChatID - default: - rootChatID := c.ID - chat.RootChatID = &rootChatID - } - if c.WorkspaceID.Valid { - chat.WorkspaceID = &c.WorkspaceID.UUID - } - if c.BuildID.Valid { - chat.BuildID = &c.BuildID.UUID - } - if c.AgentID.Valid { - chat.AgentID = &c.AgentID.UUID - } - if diffStatus != nil { - convertedDiffStatus := db2sdk.ChatDiffStatus(c.ID, diffStatus) - chat.DiffStatus = &convertedDiffStatus - } - return chat -} - -func convertChats(chats []database.Chat, diffStatusesByChatID map[uuid.UUID]database.ChatDiffStatus) []codersdk.Chat { - result := make([]codersdk.Chat, len(chats)) - for i, c := range chats { - diffStatus, ok := diffStatusesByChatID[c.ID] - if ok { - result[i] = convertChat(c, &diffStatus) - continue - } - - result[i] = convertChat(c, nil) - if diffStatusesByChatID != nil { - emptyDiffStatus := db2sdk.ChatDiffStatus(c.ID, nil) - result[i].DiffStatus = &emptyDiffStatus - } - } - return result -} - func convertChatCostModelBreakdown(model database.GetChatCostPerModelRow) codersdk.ChatCostModelBreakdown { displayName := strings.TrimSpace(model.DisplayName) if displayName == "" { diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index fd0247001b..59cb7ced1b 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -775,6 +775,79 @@ func TestWatchChats(t *testing.T) { } }) + t.Run("CreatedEventIncludesAllChatFields", func(t *testing.T) { + t.Parallel() + + // This test verifies that the pubsub "created" event + // carries a fully-populated codersdk.Chat. Exhaustive + // field-level coverage of the converter is handled by + // TestChat_AllFieldsPopulated (db2sdk) and + // TestChat_JSONRoundTrip (codersdk). This integration + // test only checks that key fields survive the full + // API → pubsub → websocket pipeline. + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + modelConfig := createChatModelConfig(t, client) + + conn, err := client.Dial(ctx, "/api/experimental/chats/watch", nil) + require.NoError(t, err) + defer conn.Close(websocket.StatusNormalClosure, "done") + + type watchEvent struct { + Type codersdk.ServerSentEventType `json:"type"` + Data json.RawMessage `json:"data,omitempty"` + } + + // Skip the initial ping. + var event watchEvent + err = wsjson.Read(ctx, conn, &event) + require.NoError(t, err) + require.Equal(t, codersdk.ServerSentEventTypePing, event.Type) + require.True(t, len(event.Data) == 0 || string(event.Data) == "null") + + createdChat, err := client.CreateChat(ctx, codersdk.CreateChatRequest{ + Content: []codersdk.ChatInputPart{ + { + Type: codersdk.ChatInputPartTypeText, + Text: "watch route fields completeness test", + }, + }, + }) + require.NoError(t, err) + + var got codersdk.Chat + testutil.Eventually(ctx, t, func(_ context.Context) bool { + var update watchEvent + if readErr := wsjson.Read(ctx, conn, &update); readErr != nil { + return false + } + if update.Type != codersdk.ServerSentEventTypeData { + return false + } + var payload coderdpubsub.ChatEvent + if unmarshalErr := json.Unmarshal(update.Data, &payload); unmarshalErr != nil { + return false + } + if payload.Kind == coderdpubsub.ChatEventKindCreated && + payload.Chat.ID == createdChat.ID { + got = payload.Chat + return true + } + return false + }, testutil.IntervalFast, "expected a created event for chat %s", createdChat.ID) + + require.Equal(t, createdChat.ID, got.ID) + require.Equal(t, createdChat.OwnerID, got.OwnerID) + require.Equal(t, modelConfig.ID, got.LastModelConfigID) + require.Equal(t, createdChat.Title, got.Title) + require.Equal(t, codersdk.ChatStatusPending, got.Status) + require.NotNil(t, got.RootChatID) + require.Equal(t, createdChat.ID, *got.RootChatID) + require.NotZero(t, got.CreatedAt) + require.NotZero(t, got.UpdatedAt) + }) + t.Run("DiffStatusChangeIncludesDiffStatus", func(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 48b2fd5032..1669e8536f 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -2479,28 +2479,7 @@ func (p *Server) publishChatPubsubEvent(chat database.Chat, kind coderdpubsub.Ch if p.pubsub == nil { return } - sdkChat := codersdk.Chat{ - ID: chat.ID, - OwnerID: chat.OwnerID, - Title: chat.Title, - Status: codersdk.ChatStatus(chat.Status), - CreatedAt: chat.CreatedAt, - UpdatedAt: chat.UpdatedAt, - } - if chat.ParentChatID.Valid { - parentChatID := chat.ParentChatID.UUID - sdkChat.ParentChatID = &parentChatID - } - if chat.RootChatID.Valid { - rootChatID := chat.RootChatID.UUID - sdkChat.RootChatID = &rootChatID - } else if !chat.ParentChatID.Valid { - rootChatID := chat.ID - sdkChat.RootChatID = &rootChatID - } - if chat.WorkspaceID.Valid { - sdkChat.WorkspaceID = &chat.WorkspaceID.UUID - } + sdkChat := db2sdk.Chat(chat, nil) // we have diffStatus already converted if diffStatus != nil { sdkChat.DiffStatus = diffStatus } diff --git a/codersdk/chats_test.go b/codersdk/chats_test.go index 9978828227..7066a52e2a 100644 --- a/codersdk/chats_test.go +++ b/codersdk/chats_test.go @@ -387,6 +387,84 @@ func TestChatCostSummary_JSONRoundTrip(t *testing.T) { require.Equal(t, original.TotalCostMicros, decoded.TotalCostMicros) } +// TestChat_JSONRoundTrip verifies that every field of codersdk.Chat +// survives a JSON marshal/unmarshal cycle. This catches omitempty +// silently eating zero-ish values, struct tag typos, and similar +// serialization bugs in the pubsub path. +func TestChat_JSONRoundTrip(t *testing.T) { + t.Parallel() + + now := time.Now().UTC().Truncate(time.Microsecond) + prState := "open" + prTitle := "test PR" + authorLogin := "testuser" + avatarURL := "https://example.com/avatar.png" + baseBranch := "main" + headBranch := "feature/test" + prNumber := int32(42) + commits := int32(3) + approved := true + reviewerCount := int32(2) + refreshedAt := now + staleAt := now.Add(time.Hour) + lastError := "boom" + prURL := "https://github.com/coder/coder/pull/42" + workspaceID := uuid.New() + buildID := uuid.New() + agentID := uuid.New() + parentChatID := uuid.New() + rootChatID := uuid.New() + + original := codersdk.Chat{ + ID: uuid.New(), + OwnerID: uuid.New(), + WorkspaceID: &workspaceID, + BuildID: &buildID, + AgentID: &agentID, + ParentChatID: &parentChatID, + RootChatID: &rootChatID, + LastModelConfigID: uuid.New(), + Title: "round-trip-test", + Status: codersdk.ChatStatusRunning, + LastError: &lastError, + CreatedAt: now, + UpdatedAt: now, + Archived: true, + MCPServerIDs: []uuid.UUID{uuid.New()}, + Labels: map[string]string{"env": "prod"}, + DiffStatus: &codersdk.ChatDiffStatus{ + ChatID: uuid.New(), + URL: &prURL, + PullRequestState: &prState, + PullRequestTitle: prTitle, + PullRequestDraft: true, + ChangesRequested: true, + Additions: 10, + Deletions: 5, + ChangedFiles: 3, + AuthorLogin: &authorLogin, + AuthorAvatarURL: &avatarURL, + BaseBranch: &baseBranch, + HeadBranch: &headBranch, + PRNumber: &prNumber, + Commits: &commits, + Approved: &approved, + ReviewerCount: &reviewerCount, + RefreshedAt: &refreshedAt, + StaleAt: &staleAt, + }, + } + + data, err := json.Marshal(original) + require.NoError(t, err) + + var decoded codersdk.Chat + err = json.Unmarshal(data, &decoded) + require.NoError(t, err) + + require.Equal(t, original, decoded) +} + //nolint:tparallel,paralleltest func TestParseChatWorkspaceTTL(t *testing.T) { t.Parallel() diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx index 0130b46b6e..f14a08e5e3 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.test.tsx @@ -411,4 +411,63 @@ describe("AgentsSidebar model display names", () => { expect(getByText("GPT-4o (Quality)")).toBeInTheDocument(); }); + + it("shows Default model when last_model_config_id is a nil UUID", () => { + const { getByText } = render( + + + , + ); + + // A nil UUID means LastModelConfigID was left at its zero value, + // so the sidebar cannot resolve the model and falls back. + expect(getByText("Default model")).toBeInTheDocument(); + }); + + it("shows model name when last_model_config_id matches a config", () => { + const { getByText, queryByText } = render( + + + , + ); + + // Regression guard: a valid last_model_config_id must resolve + // to the actual model display name, not "Default model". + expect(getByText("GPT-4o")).toBeInTheDocument(); + expect(queryByText("Default model")).not.toBeInTheDocument(); + }); });