From 6f97539122e08dd1f9973a8836de824697878618 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 16 Mar 2026 08:41:32 -0700 Subject: [PATCH] fix: update sidebar diff status on WebSocket events (#23116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem The sidebar diff status (PR icon, +additions/-deletions, file count) was not updating in real-time. Users had to reload the page to see changes. Two root causes: 1. **Frontend**: The `diff_status_change` WebSocket handler in `AgentsPage.tsx` had an early `return` (line 398) that skipped `updateInfiniteChatsCache`, so the sidebar's cache was never updated. Even for other event types, the cache merge only spread `status` and `title` — never `diff_status`. 2. **Server**: `publishChatPubsubEvent` in `chatd.go` constructed a minimal `Chat` payload without `DiffStatus`, so even if the frontend consumed the event, `updatedChat.diff_status` would be `undefined`. ## Fix ### Server (`coderd/chatd/chatd.go`) - `publishChatPubsubEvent` now accepts an optional `*codersdk.ChatDiffStatus` parameter; when non-nil it's set on the outgoing `Chat` payload. - `PublishDiffStatusChange` fetches the diff status from the DB, converts it, and passes it through. - Added `convertDBChatDiffStatus` (mirrors `coderd/chats.go`'s converter to avoid circular import). - All other callers pass `nil`. ### Frontend (`site/src/pages/AgentsPage/AgentsPage.tsx`) - Removed the early `return` so `diff_status_change` events fall through to the cache update logic. - Added `isDiffStatusEvent` flag and spread `diff_status` into both the infinite chats cache (sidebar) and the individual chat cache. --- coderd/chatd/chatd.go | 29 ++++-- coderd/chatd/quickgen.go | 2 +- coderd/chats.go | 84 +--------------- coderd/chats_test.go | 122 +++++++++++++++++++++++ coderd/database/db2sdk/db2sdk.go | 84 ++++++++++++++++ site/src/pages/AgentsPage/AgentsPage.tsx | 8 +- 6 files changed, 235 insertions(+), 94 deletions(-) diff --git a/coderd/chatd/chatd.go b/coderd/chatd/chatd.go index 7e08845ea1..a00d4104af 100644 --- a/coderd/chatd/chatd.go +++ b/coderd/chatd/chatd.go @@ -347,7 +347,7 @@ func (p *Server) CreateChat(ctx context.Context, opts CreateOptions) (database.C return database.Chat{}, txErr } - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindCreated) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindCreated, nil) return chat, nil } @@ -488,7 +488,7 @@ func (p *Server) SendMessage( p.publishMessage(opts.ChatID, result.Message) p.publishStatus(opts.ChatID, result.Chat.Status, result.Chat.WorkerID) - p.publishChatPubsubEvent(result.Chat, coderdpubsub.ChatEventKindStatusChange) + p.publishChatPubsubEvent(result.Chat, coderdpubsub.ChatEventKindStatusChange, nil) return result, nil } @@ -585,7 +585,7 @@ func (p *Server) EditMessage( QueueUpdate: true, }) p.publishStatus(opts.ChatID, result.Chat.Status, result.Chat.WorkerID) - p.publishChatPubsubEvent(result.Chat, coderdpubsub.ChatEventKindStatusChange) + p.publishChatPubsubEvent(result.Chat, coderdpubsub.ChatEventKindStatusChange, nil) return result, nil } @@ -605,7 +605,7 @@ func (p *Server) ArchiveChat(ctx context.Context, chatID uuid.UUID) error { return xerrors.Errorf("archive chat: %w", err) } - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindDeleted) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindDeleted, nil) return nil } @@ -625,7 +625,7 @@ func (p *Server) UnarchiveChat(ctx context.Context, chatID uuid.UUID) error { return xerrors.Errorf("unarchive chat: %w", err) } - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindCreated) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindCreated, nil) return nil } @@ -780,7 +780,7 @@ func (p *Server) PromoteQueued( }) p.publishMessage(opts.ChatID, promoted) p.publishStatus(opts.ChatID, updatedChat.Status, updatedChat.WorkerID) - p.publishChatPubsubEvent(updatedChat, coderdpubsub.ChatEventKindStatusChange) + p.publishChatPubsubEvent(updatedChat, coderdpubsub.ChatEventKindStatusChange, nil) return result, nil } @@ -877,7 +877,7 @@ func (p *Server) setChatWaiting(ctx context.Context, chatID uuid.UUID) (database return database.Chat{}, err } p.publishStatus(chatID, updatedChat.Status, updatedChat.WorkerID) - p.publishChatPubsubEvent(updatedChat, coderdpubsub.ChatEventKindStatusChange) + p.publishChatPubsubEvent(updatedChat, coderdpubsub.ChatEventKindStatusChange, nil) return updatedChat, nil } @@ -1611,7 +1611,7 @@ func (p *Server) publishChatStreamNotify(chatID uuid.UUID, notify coderdpubsub.C // publishChatPubsubEvent broadcasts a chat lifecycle event via PostgreSQL // pubsub so that all replicas can push updates to watching clients. -func (p *Server) publishChatPubsubEvent(chat database.Chat, kind coderdpubsub.ChatEventKind) { +func (p *Server) publishChatPubsubEvent(chat database.Chat, kind coderdpubsub.ChatEventKind, diffStatus *codersdk.ChatDiffStatus) { if p.pubsub == nil { return } @@ -1637,6 +1637,9 @@ func (p *Server) publishChatPubsubEvent(chat database.Chat, kind coderdpubsub.Ch if chat.WorkspaceID.Valid { sdkChat.WorkspaceID = &chat.WorkspaceID.UUID } + if diffStatus != nil { + sdkChat.DiffStatus = diffStatus + } event := coderdpubsub.ChatEvent{ Kind: kind, Chat: sdkChat, @@ -1672,7 +1675,13 @@ func (p *Server) PublishDiffStatusChange(ctx context.Context, chatID uuid.UUID) return xerrors.Errorf("get chat: %w", err) } - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindDiffStatusChange) + dbStatus, err := p.db.GetChatDiffStatusByChatID(ctx, chatID) + if err != nil { + return xerrors.Errorf("get chat diff status: %w", err) + } + + sdkStatus := db2sdk.ChatDiffStatus(chatID, &dbStatus) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindDiffStatusChange, &sdkStatus) return nil } @@ -2041,7 +2050,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { slog.F("chat_id", chat.ID), slog.Error(readErr)) } chat.Status = status - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindStatusChange) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindStatusChange, nil) if !wasInterrupted { p.maybeSendPushNotification(cleanupCtx, chat, status, lastError, logger) diff --git a/coderd/chatd/quickgen.go b/coderd/chatd/quickgen.go index 4b9d3dc9b8..8d9e9d266b 100644 --- a/coderd/chatd/quickgen.go +++ b/coderd/chatd/quickgen.go @@ -111,7 +111,7 @@ func (p *Server) maybeGenerateChatTitle( return } chat.Title = title - p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindTitleChange) + p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindTitleChange, nil) return } diff --git a/coderd/chats.go b/coderd/chats.go index 080af99d67..befe951a58 100644 --- a/coderd/chats.go +++ b/coderd/chats.go @@ -2457,7 +2457,7 @@ func convertChat(c database.Chat, diffStatus *database.ChatDiffStatus) codersdk. chat.WorkspaceID = &c.WorkspaceID.UUID } if diffStatus != nil { - convertedDiffStatus := convertChatDiffStatus(c.ID, diffStatus) + convertedDiffStatus := db2sdk.ChatDiffStatus(c.ID, diffStatus) chat.DiffStatus = &convertedDiffStatus } return chat @@ -2474,7 +2474,7 @@ func convertChats(chats []database.Chat, diffStatusesByChatID map[uuid.UUID]data result[i] = convertChat(c, nil) if diffStatusesByChatID != nil { - emptyDiffStatus := convertChatDiffStatus(c.ID, nil) + emptyDiffStatus := db2sdk.ChatDiffStatus(c.ID, nil) result[i].DiffStatus = &emptyDiffStatus } } @@ -2558,86 +2558,6 @@ func convertChatMessages(messages []database.ChatMessage) []codersdk.ChatMessage return result } -func convertChatDiffStatus(chatID uuid.UUID, status *database.ChatDiffStatus) codersdk.ChatDiffStatus { - result := codersdk.ChatDiffStatus{ - ChatID: chatID, - } - if status == nil { - return result - } - - result.ChatID = status.ChatID - if status.Url.Valid { - u := strings.TrimSpace(status.Url.String) - if u != "" { - result.URL = &u - } - } - if result.URL == nil { - // Try to build a branch URL from the stored origin. - // Since convertChatDiffStatus does not have access to - // the API instance, we construct a GitHub provider - // directly as a best-effort fallback. - // TODO: This uses the default github.com API base URL, - // so branch URLs for GitHub Enterprise instances will - // be incorrect. To fix this, convertChatDiffStatus - // would need access to the external auth configs. - gp := gitprovider.New("github", "", nil) - if gp != nil { - if owner, repo, _, ok := gp.ParseRepositoryOrigin(status.GitRemoteOrigin); ok { - branchURL := gp.BuildBranchURL(owner, repo, status.GitBranch) - if branchURL != "" { - result.URL = &branchURL - } - } - } - } - if status.PullRequestState.Valid { - pullRequestState := strings.TrimSpace(status.PullRequestState.String) - if pullRequestState != "" { - result.PullRequestState = &pullRequestState - } - } - result.PullRequestTitle = status.PullRequestTitle - result.PullRequestDraft = status.PullRequestDraft - result.ChangesRequested = status.ChangesRequested - result.Additions = status.Additions - result.Deletions = status.Deletions - result.ChangedFiles = status.ChangedFiles - if status.AuthorLogin.Valid { - result.AuthorLogin = &status.AuthorLogin.String - } - if status.AuthorAvatarUrl.Valid { - result.AuthorAvatarURL = &status.AuthorAvatarUrl.String - } - if status.BaseBranch.Valid { - result.BaseBranch = &status.BaseBranch.String - } - if status.HeadBranch.Valid { - result.HeadBranch = &status.HeadBranch.String - } - if status.PrNumber.Valid { - result.PRNumber = &status.PrNumber.Int32 - } - if status.Commits.Valid { - result.Commits = &status.Commits.Int32 - } - if status.Approved.Valid { - result.Approved = &status.Approved.Bool - } - if status.ReviewerCount.Valid { - result.ReviewerCount = &status.ReviewerCount.Int32 - } - if status.RefreshedAt.Valid { - refreshedAt := status.RefreshedAt.Time - result.RefreshedAt = &refreshedAt - } - staleAt := status.StaleAt - result.StaleAt = &staleAt - - return result -} - func (api *API) listChatProviders(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() //nolint:gocritic // System context required to read enabled chat providers. diff --git a/coderd/chats_test.go b/coderd/chats_test.go index 4779954ded..0b7c3227f8 100644 --- a/coderd/chats_test.go +++ b/coderd/chats_test.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/coderdtest/oidctest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/externalauth" @@ -616,6 +617,127 @@ func TestWatchChats(t *testing.T) { } }) + t.Run("DiffStatusChangeIncludesDiffStatus", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + DeploymentValues: chatDeploymentValues(t), + }) + db := api.Database + user := coderdtest.CreateFirstUser(t, client) + modelConfig := createChatModelConfig(t, client) + + // Insert a chat and a diff status row. + chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ + OwnerID: user.UserID, + LastModelConfigID: modelConfig.ID, + Title: "diff status watch test", + }) + require.NoError(t, err) + + refreshedAt := time.Now().UTC().Truncate(time.Second) + staleAt := refreshedAt.Add(time.Hour) + _, err = db.UpsertChatDiffStatusReference( + dbauthz.AsSystemRestricted(ctx), + database.UpsertChatDiffStatusReferenceParams{ + ChatID: chat.ID, + Url: sql.NullString{String: "https://github.com/coder/coder/pull/99", Valid: true}, + GitBranch: "feature/test", + GitRemoteOrigin: "git@github.com:coder/coder.git", + StaleAt: staleAt, + }, + ) + require.NoError(t, err) + _, err = db.UpsertChatDiffStatus( + dbauthz.AsSystemRestricted(ctx), + database.UpsertChatDiffStatusParams{ + ChatID: chat.ID, + Url: sql.NullString{String: "https://github.com/coder/coder/pull/99", Valid: true}, + PullRequestState: sql.NullString{String: "open", Valid: true}, + Additions: 42, + Deletions: 7, + ChangedFiles: 5, + RefreshedAt: refreshedAt, + StaleAt: staleAt, + }, + ) + require.NoError(t, err) + + // Open the watch WebSocket. + 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"` + } + + // Read the initial ping. + var ping watchEvent + err = wsjson.Read(ctx, conn, &ping) + require.NoError(t, err) + require.Equal(t, codersdk.ServerSentEventTypePing, ping.Type) + + // Publish a diff_status_change event via pubsub, + // mimicking what PublishDiffStatusChange does after + // it reads the diff status from the DB. + dbStatus, err := db.GetChatDiffStatusByChatID(dbauthz.AsSystemRestricted(ctx), chat.ID) + require.NoError(t, err) + sdkDiffStatus := db2sdk.ChatDiffStatus(chat.ID, &dbStatus) + event := coderdpubsub.ChatEvent{ + Kind: coderdpubsub.ChatEventKindDiffStatusChange, + Chat: codersdk.Chat{ + ID: chat.ID, + OwnerID: chat.OwnerID, + Title: chat.Title, + Status: codersdk.ChatStatus(chat.Status), + CreatedAt: chat.CreatedAt, + UpdatedAt: chat.UpdatedAt, + DiffStatus: &sdkDiffStatus, + }, + } + payload, err := json.Marshal(event) + require.NoError(t, err) + err = api.Pubsub.Publish(coderdpubsub.ChatEventChannel(user.UserID), payload) + require.NoError(t, err) + + // Read events until we find the diff_status_change. + for { + var update watchEvent + err = wsjson.Read(ctx, conn, &update) + require.NoError(t, err) + + if update.Type == codersdk.ServerSentEventTypePing { + continue + } + require.Equal(t, codersdk.ServerSentEventTypeData, update.Type) + + var received coderdpubsub.ChatEvent + err = json.Unmarshal(update.Data, &received) + require.NoError(t, err) + + if received.Kind != coderdpubsub.ChatEventKindDiffStatusChange || + received.Chat.ID != chat.ID { + continue + } + + // Verify the event carries the full DiffStatus. + require.NotNil(t, received.Chat.DiffStatus, "diff_status_change event must include DiffStatus") + ds := received.Chat.DiffStatus + require.Equal(t, chat.ID, ds.ChatID) + require.NotNil(t, ds.URL) + require.Equal(t, "https://github.com/coder/coder/pull/99", *ds.URL) + require.NotNil(t, ds.PullRequestState) + require.Equal(t, "open", *ds.PullRequestState) + require.EqualValues(t, 42, ds.Additions) + require.EqualValues(t, 7, ds.Deletions) + require.EqualValues(t, 5, ds.ChangedFiles) + break + } + }) + t.Run("Unauthenticated", func(t *testing.T) { t.Parallel() diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 91c839bac6..b06defb119 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -21,6 +21,7 @@ import ( agentproto "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/chatd/chatprompt" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/externalauth/gitprovider" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/render" @@ -1164,3 +1165,86 @@ func nullInt64Ptr(v sql.NullInt64) *int64 { value := v.Int64 return &value } + +// ChatDiffStatus converts a database.ChatDiffStatus to a +// codersdk.ChatDiffStatus. When status is nil an empty value +// containing only the chatID is returned. +func ChatDiffStatus(chatID uuid.UUID, status *database.ChatDiffStatus) codersdk.ChatDiffStatus { + result := codersdk.ChatDiffStatus{ + ChatID: chatID, + } + if status == nil { + return result + } + + result.ChatID = status.ChatID + if status.Url.Valid { + u := strings.TrimSpace(status.Url.String) + if u != "" { + result.URL = &u + } + } + if result.URL == nil { + // Try to build a branch URL from the stored origin. + // Since this function does not have access to the API + // instance, we construct a GitHub provider directly as + // a best-effort fallback. + // TODO: This uses the default github.com API base URL, + // so branch URLs for GitHub Enterprise instances will + // be incorrect. To fix this, this function would need + // access to the external auth configs. + gp := gitprovider.New("github", "", nil) + if gp != nil { + if owner, repo, _, ok := gp.ParseRepositoryOrigin(status.GitRemoteOrigin); ok { + branchURL := gp.BuildBranchURL(owner, repo, status.GitBranch) + if branchURL != "" { + result.URL = &branchURL + } + } + } + } + if status.PullRequestState.Valid { + pullRequestState := strings.TrimSpace(status.PullRequestState.String) + if pullRequestState != "" { + result.PullRequestState = &pullRequestState + } + } + result.PullRequestTitle = status.PullRequestTitle + result.PullRequestDraft = status.PullRequestDraft + result.ChangesRequested = status.ChangesRequested + result.Additions = status.Additions + result.Deletions = status.Deletions + result.ChangedFiles = status.ChangedFiles + if status.AuthorLogin.Valid { + result.AuthorLogin = &status.AuthorLogin.String + } + if status.AuthorAvatarUrl.Valid { + result.AuthorAvatarURL = &status.AuthorAvatarUrl.String + } + if status.BaseBranch.Valid { + result.BaseBranch = &status.BaseBranch.String + } + if status.HeadBranch.Valid { + result.HeadBranch = &status.HeadBranch.String + } + if status.PrNumber.Valid { + result.PRNumber = &status.PrNumber.Int32 + } + if status.Commits.Valid { + result.Commits = &status.Commits.Int32 + } + if status.Approved.Valid { + result.Approved = &status.Approved.Bool + } + if status.ReviewerCount.Valid { + result.ReviewerCount = &status.ReviewerCount.Int32 + } + if status.RefreshedAt.Valid { + refreshedAt := status.RefreshedAt.Time + result.RefreshedAt = &refreshedAt + } + staleAt := status.StaleAt + result.StaleAt = &staleAt + + return result +} diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index fc088d64c3..b8d88849eb 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -394,7 +394,6 @@ const AgentsPage: FC = () => { queryKey: chatDiffContentsKey(updatedChat.id), }), ]); - return; } // Scope field updates by event kind so that // status_change events (which may carry a stale title @@ -403,6 +402,7 @@ const AgentsPage: FC = () => { // landed. const isTitleEvent = chatEvent.kind === "title_change"; const isStatusEvent = chatEvent.kind === "status_change"; + const isDiffStatusEvent = chatEvent.kind === "diff_status_change"; // For "created" events, use a cross-page existence // check and prepend only to the first page. @@ -419,6 +419,9 @@ const AgentsPage: FC = () => { ...c, ...(isStatusEvent && { status: updatedChat.status }), ...(isTitleEvent && { title: updatedChat.title }), + ...(isDiffStatusEvent && { + diff_status: updatedChat.diff_status, + }), updated_at: c.updated_at > updatedChat.updated_at ? c.updated_at @@ -437,6 +440,9 @@ const AgentsPage: FC = () => { ...previousChat, ...(isStatusEvent && { status: updatedChat.status }), ...(isTitleEvent && { title: updatedChat.title }), + ...(isDiffStatusEvent && { + diff_status: updatedChat.diff_status, + }), updated_at: previousChat.updated_at > updatedChat.updated_at ? previousChat.updated_at