mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): stop spamming chats list endpoint on diff_status_change events (#23167)
## 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.
This commit is contained in:
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user