From ffb47cea19f6ff36760fcbe14ed79a700f9e95b8 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 6 Mar 2026 10:48:58 +0000 Subject: [PATCH] feat(chatd): add tag-based dedup to push notifications (#22669) --- coderd/chatd/chatd.go | 1 + coderd/chatd/chatd_test.go | 86 +++++++++++++++++++++++++++++----- codersdk/notifications.go | 1 + site/src/api/typesGenerated.ts | 1 + site/src/serviceWorker.ts | 1 + 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/coderd/chatd/chatd.go b/coderd/chatd/chatd.go index 6b6c4beffb..b03dbb48c2 100644 --- a/coderd/chatd/chatd.go +++ b/coderd/chatd/chatd.go @@ -1816,6 +1816,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { Body: "Agent has finished running.", Icon: "/favicon.ico", Data: map[string]string{"url": fmt.Sprintf("/agents/%s", chat.ID)}, + Tag: chat.ID.String(), } if status == database.ChatStatusError { pushMsg.Body = "Agent encountered an error." diff --git a/coderd/chatd/chatd_test.go b/coderd/chatd/chatd_test.go index 7f307b57b9..069f2ff7fa 100644 --- a/coderd/chatd/chatd_test.go +++ b/coderd/chatd/chatd_test.go @@ -1533,21 +1533,14 @@ func TestSuccessfulChatSendsWebPushWithNavigationData(t *testing.T) { }) require.NoError(t, err) - // Wait for the chat to complete and return to waiting status. - testutil.Eventually(ctx, t, func(ctx context.Context) bool { - fromDB, dbErr := db.GetChatByID(ctx, chat.ID) - if dbErr != nil { - return false - } - return fromDB.Status == database.ChatStatusWaiting && !fromDB.WorkerID.Valid - }, testutil.IntervalFast) - // Wait for a web push notification to be dispatched. The dispatch // happens asynchronously after the DB status is updated, so we need // to poll rather than assert immediately. - testutil.Eventually(ctx, t, func(ctx context.Context) bool { - return mockPush.dispatchCount.Load() == 1 - }, testutil.IntervalFast, + testutil.Eventually(ctx, t, func(_ context.Context) bool { + return mockPush.dispatchCount.Load() >= 1 + }, testutil.IntervalFast) + + require.Equal(t, int32(1), mockPush.dispatchCount.Load(), "expected exactly one web push dispatch for a completed chat") // Verify the notification was sent to the correct user. @@ -1565,6 +1558,75 @@ func TestSuccessfulChatSendsWebPushWithNavigationData(t *testing.T) { "web push Data should contain the chat navigation URL") } +func TestSuccessfulChatSendsWebPushWithTag(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitLong) + + // Set up a mock OpenAI that returns a simple streaming response. + openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { + if !req.Stream { + return chattest.OpenAINonStreamingResponse("title") + } + return chattest.OpenAIStreamingResponse(chattest.OpenAITextChunks("done")...) + }) + + // Mock webpush dispatcher that captures calls. + mockPush := &mockWebpushDispatcher{} + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) + server := chatd.New(chatd.Config{ + Logger: logger, + Database: db, + ReplicaID: uuid.New(), + Pubsub: ps, + PendingChatAcquireInterval: 10 * time.Millisecond, + InFlightChatStaleAfter: testutil.WaitSuperLong, + WebpushDispatcher: mockPush, + }) + t.Cleanup(func() { + require.NoError(t, server.Close()) + }) + + user, model := seedChatDependencies(ctx, t, db) + setOpenAIProviderBaseURL(ctx, t, db, openAIURL) + + chat, err := server.CreateChat(ctx, chatd.CreateOptions{ + OwnerID: user.ID, + Title: "push-tag-test", + ModelConfigID: model.ID, + InitialUserContent: []fantasy.Content{fantasy.TextContent{Text: "hello"}}, + }) + require.NoError(t, err) + + // Wait for the web push notification to be dispatched. + // We poll dispatchCount rather than DB status because the + // push fires after the status update, creating a small race + // window. + testutil.Eventually(ctx, t, func(_ context.Context) bool { + return mockPush.dispatchCount.Load() >= 1 + }, testutil.IntervalFast) + + require.Equal(t, int32(1), mockPush.dispatchCount.Load(), + "expected exactly one web push dispatch for a completed chat") + + // Verify the push notification tag is set to the chat ID for dedup. + mockPush.mu.Lock() + capturedMsg := mockPush.lastMessage + capturedUser := mockPush.lastUserID + mockPush.mu.Unlock() + + require.Equal(t, chat.ID.String(), capturedMsg.Tag, + "push notification tag should equal the chat ID for deduplication") + require.Equal(t, user.ID, capturedUser, + "push notification should be dispatched to the chat owner") + require.Equal(t, "push-tag-test", capturedMsg.Title, + "push notification title should match the chat title") + require.Equal(t, "Agent has finished running.", capturedMsg.Body, + "push notification body should indicate the agent finished") +} + func TestCloseDuringShutdownContextCanceledShouldRetryOnNewReplica(t *testing.T) { t.Parallel() diff --git a/codersdk/notifications.go b/codersdk/notifications.go index 76479cd15d..559a0116a3 100644 --- a/codersdk/notifications.go +++ b/codersdk/notifications.go @@ -224,6 +224,7 @@ type WebpushMessage struct { Icon string `json:"icon"` Title string `json:"title"` Body string `json:"body"` + Tag string `json:"tag,omitempty"` Actions []WebpushMessageAction `json:"actions"` Data map[string]string `json:"data,omitempty"` } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 18df4f456a..fb58a6a963 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -6798,6 +6798,7 @@ export interface WebpushMessage { readonly icon: string; readonly title: string; readonly body: string; + readonly tag?: string; readonly actions: readonly WebpushMessageAction[]; readonly data?: Record; } diff --git a/site/src/serviceWorker.ts b/site/src/serviceWorker.ts index 4c53bf2bed..5bd1e14539 100644 --- a/site/src/serviceWorker.ts +++ b/site/src/serviceWorker.ts @@ -46,6 +46,7 @@ self.addEventListener("push", (event) => { body: payload.body || "", icon: payload.icon || "/favicon.ico", data: payload.data, + tag: payload.tag, }); }), );