From a40716b6fe00b719f558cba6af2c2cfd53607480 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 17 Mar 2026 10:51:01 -0400 Subject: [PATCH] fix(site): stop spamming chats list endpoint on diff_status_change events (#23167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem The WebSocket handler for `diff_status_change` events in `AgentsPage.tsx` was triggering a burst of redundant HTTP requests on every event: 1. **`invalidateChatListQueries(queryClient)`** — Full refetch of the chats list endpoint. Unnecessary because `updateInfiniteChatsCache` already writes `diff_status` into the sidebar cache optimistically on every event. 2. **`invalidateQueries({ queryKey: chatKey(id) })`** — Refetch of the individual chat. Also unnecessary — the SSE event carries `diff_status` in its payload and the optimistic updater writes it into the `chatKey` cache directly. Worse, this call was missing `exact: true`, so TanStack Query's prefix matching cascaded the invalidation to `chatMessagesKey`, `chatDiffContentsKey`, and every other query under `["chats", id]`. Since diff status changes fire frequently during active agent work, this spammed the chats list endpoint and caused redundant refetches of messages and diff contents on every single event. ## Fix Strip the handler down to the one invalidation that's actually needed — `chatDiffContentsKey` (the file-level diff contents aren't in the SSE payload): ```typescript if (chatEvent.kind === "diff_status_change") { void queryClient.invalidateQueries({ queryKey: chatDiffContentsKey(updatedChat.id), exact: true, }); } ``` ## Why tests didn't catch this The existing tests in `chats.test.ts` cover query utilities in isolation (e.g. `invalidateChatListQueries` scoping, mutation invalidation). The WebSocket event handler lives in the `AgentsPage` component — there was no test covering what the `diff_status_change` code path actually invalidates. Added regression tests verifying that `exact: true` prevents prefix-match cascade vs the old behavior. --- site/src/api/queries/chats.test.ts | 74 ++++++++++++++++++++++++ site/src/pages/AgentsPage/AgentsPage.tsx | 16 +++-- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/site/src/api/queries/chats.test.ts b/site/src/api/queries/chats.test.ts index c612a047a9..d51a2909d2 100644 --- a/site/src/api/queries/chats.test.ts +++ b/site/src/api/queries/chats.test.ts @@ -753,3 +753,77 @@ describe("infiniteChats", () => { }); }); }); + +describe("diff_status_change invalidation scope", () => { + // These tests verify the CORRECT invalidation pattern for + // diff_status_change WebSocket events. The handler should + // invalidate only the individual chat detail and diff-contents + // queries — NOT the chat list (sidebar) or messages. + + it("exact chatKey invalidation does not cascade to messages or diff-contents", async () => { + const queryClient = createTestQueryClient(); + const chatId = "chat-1"; + + // Seed all the queries that are active on the /agents/:id page. + queryClient.setQueryData(chatKey(chatId), makeChat(chatId)); + queryClient.setQueryData(chatMessagesKey(chatId), []); + queryClient.setQueryData(chatDiffContentsKey(chatId), { files: [] }); + queryClient.setQueryData(chatsKey, [makeChat(chatId)]); + + // This is what the fixed handler does — exact: true. + await queryClient.invalidateQueries({ + queryKey: chatKey(chatId), + exact: true, + }); + + // chatKey itself should be invalidated. + expect( + queryClient.getQueryState(chatKey(chatId))?.isInvalidated, + "chatKey should be invalidated", + ).toBe(true); + + // Messages should NOT be invalidated. + expect( + queryClient.getQueryState(chatMessagesKey(chatId))?.isInvalidated, + "chatMessagesKey should NOT be invalidated by exact chatKey", + ).not.toBe(true); + + // Diff-contents should NOT be invalidated. + expect( + queryClient.getQueryState(chatDiffContentsKey(chatId))?.isInvalidated, + "chatDiffContentsKey should NOT be invalidated by exact chatKey", + ).not.toBe(true); + + // Chat list should NOT be invalidated. + expect( + queryClient.getQueryState(chatsKey)?.isInvalidated, + "chatsKey should NOT be invalidated by exact chatKey", + ).not.toBe(true); + }); + + it("without exact: true, chatKey invalidation cascades to messages and diff-contents (the old bug)", async () => { + const queryClient = createTestQueryClient(); + const chatId = "chat-1"; + + queryClient.setQueryData(chatKey(chatId), makeChat(chatId)); + queryClient.setQueryData(chatMessagesKey(chatId), []); + queryClient.setQueryData(chatDiffContentsKey(chatId), { files: [] }); + + // This is what the OLD (broken) handler did — no exact: true. + await queryClient.invalidateQueries({ + queryKey: chatKey(chatId), + }); + + // Without exact: true, ALL queries starting with ["chats", chatId] + // get invalidated, including messages and diff-contents. + expect( + queryClient.getQueryState(chatMessagesKey(chatId))?.isInvalidated, + "chatMessagesKey IS invalidated without exact: true (old bug)", + ).toBe(true); + + expect( + queryClient.getQueryState(chatDiffContentsKey(chatId))?.isInvalidated, + "chatDiffContentsKey IS invalidated without exact: true (old bug)", + ).toBe(true); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentsPage.tsx b/site/src/pages/AgentsPage/AgentsPage.tsx index b2676eeabb..682ed6045c 100644 --- a/site/src/pages/AgentsPage/AgentsPage.tsx +++ b/site/src/pages/AgentsPage/AgentsPage.tsx @@ -395,15 +395,13 @@ const AgentsPage: FC = () => { } if (chatEvent.kind === "diff_status_change") { - void Promise.all([ - queryClient.invalidateQueries({ - queryKey: chatKey(updatedChat.id), - }), - queryClient.invalidateQueries({ - queryKey: chatDiffContentsKey(updatedChat.id), - }), - invalidateChatListQueries(queryClient), - ]); + // Only refetch the diff file contents — the chat's + // diff_status field is already written into the + // chatKey and infinite-list caches below. + void queryClient.invalidateQueries({ + queryKey: chatDiffContentsKey(updatedChat.id), + exact: true, + }); } // Scope field updates by event kind so that // status_change events (which may carry a stale title