mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: update sidebar diff status on WebSocket events (#23116)
## 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.
This commit is contained in:
+19
-10
@@ -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)
|
||||
|
||||
@@ -111,7 +111,7 @@ func (p *Server) maybeGenerateChatTitle(
|
||||
return
|
||||
}
|
||||
chat.Title = title
|
||||
p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindTitleChange)
|
||||
p.publishChatPubsubEvent(chat, coderdpubsub.ChatEventKindTitleChange, nil)
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
+2
-82
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user