mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
fix(agents): remove optimistic message rendering and fix auto-promote delivery (#22588)
## Problem Two bugs in the agents chat flow: 1. **Optimistic rendering glitch**: When sending a message while the agent is busy, a fake message with a negative ID appears in the timeline, then gets rolled back to the queued state. This causes a jarring flash. 2. **Auto-promoted messages not appearing**: When the server auto-promotes a queued message after finishing a task, the promoted user message doesn't show up in the timeline until the LLM finishes its response. ## Root Causes **Bug 1**: The optimistic rendering system injected placeholder messages with `id: -Date.now()` into the store. When the server responded with `queued: true`, the optimistic message was rolled back — but the user had already seen it flash in the timeline. **Bug 2**: In `processChat`'s deferred cleanup, the auto-promoted message was published via `publishEvent()`, which only delivers to local in-process stream subscribers. The SSE subscriber goroutine only forwards `message_part` events from the local channel — it ignores `message` events. Durable events reach the SSE client via pubsub → DB read, but `publishEvent` doesn't trigger a pubsub notification. The explicit `PromoteQueued` endpoint correctly used `publishMessage()` (which does both), but the auto-promote path did not. ## Changes ### Frontend (`site/`) - **AgentDetail.tsx**: Remove optimistic message injection from send and edit flows. Instead, use the `CreateChatMessageResponse.message` from the POST response to insert the real server message into the store immediately. - **ChatContext.ts**: Remove the negative-ID cleanup logic from `upsertDurableMessage` that stripped optimistic placeholders when real messages arrived. - **chatStore.test.ts**: Remove 2 tests for negative-ID optimistic message behavior. ### Backend (`coderd/chatd/`) - **chatd.go**: In `processChat` cleanup, replace `publishEvent()` with `publishMessage()` for auto-promoted messages. This ensures the pubsub notification (`AfterMessageID`) is sent, so SSE subscribers read the new message from the DB immediately.
This commit is contained in:
@@ -1766,11 +1766,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) {
|
||||
} else {
|
||||
status = database.ChatStatusPending
|
||||
|
||||
sdkMsg := db2sdk.ChatMessage(msg)
|
||||
p.publishEvent(chat.ID, codersdk.ChatStreamEvent{
|
||||
Type: codersdk.ChatStreamEventTypeMessage,
|
||||
Message: &sdkMsg,
|
||||
})
|
||||
p.publishMessage(chat.ID, msg)
|
||||
|
||||
remaining, qErr := tx.GetChatQueuedMessages(cleanupCtx, chat.ID)
|
||||
if qErr == nil {
|
||||
|
||||
@@ -18,15 +18,15 @@ export const chat = (chatId: string) => ({
|
||||
|
||||
export const createChat = (queryClient: QueryClient) => ({
|
||||
mutationFn: (req: TypesGen.CreateChatRequest) => API.createChat(req),
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
},
|
||||
});
|
||||
|
||||
export const archiveChat = (queryClient: QueryClient) => ({
|
||||
mutationFn: (chatId: string) => API.archiveChat(chatId),
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
},
|
||||
});
|
||||
|
||||
@@ -36,8 +36,8 @@ export const createChatMessage = (
|
||||
) => ({
|
||||
mutationFn: (req: TypesGen.CreateChatMessageRequest) =>
|
||||
API.createChatMessage(chatId, req),
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
},
|
||||
});
|
||||
|
||||
@@ -49,18 +49,16 @@ type EditChatMessageMutationArgs = {
|
||||
export const editChatMessage = (queryClient: QueryClient, chatId: string) => ({
|
||||
mutationFn: ({ messageId, req }: EditChatMessageMutationArgs) =>
|
||||
API.editChatMessage(chatId, messageId, req),
|
||||
onSuccess: async () => {
|
||||
await Promise.all([
|
||||
queryClient.invalidateQueries({ queryKey: chatsKey }),
|
||||
queryClient.invalidateQueries({ queryKey: chatKey(chatId) }),
|
||||
]);
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
void queryClient.invalidateQueries({ queryKey: chatKey(chatId) });
|
||||
},
|
||||
});
|
||||
|
||||
export const interruptChat = (queryClient: QueryClient, chatId: string) => ({
|
||||
mutationFn: () => API.interruptChat(chatId),
|
||||
onSuccess: async () => {
|
||||
await queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
},
|
||||
});
|
||||
|
||||
@@ -81,11 +79,9 @@ export const promoteChatQueuedMessage = (
|
||||
) => ({
|
||||
mutationFn: (queuedMessageId: number) =>
|
||||
API.promoteChatQueuedMessage(chatId, queuedMessageId),
|
||||
onSuccess: async () => {
|
||||
await Promise.all([
|
||||
queryClient.invalidateQueries({ queryKey: chatsKey }),
|
||||
queryClient.invalidateQueries({ queryKey: chatKey(chatId) }),
|
||||
]);
|
||||
onSuccess: () => {
|
||||
void queryClient.invalidateQueries({ queryKey: chatsKey });
|
||||
void queryClient.invalidateQueries({ queryKey: chatKey(chatId) });
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -292,7 +292,6 @@ export const AgentChatInput = memo<AgentChatInputProps>(
|
||||
}
|
||||
|
||||
onSend(text);
|
||||
internalRef.current?.clear();
|
||||
internalRef.current?.focus();
|
||||
}, [
|
||||
isDisabled,
|
||||
@@ -392,7 +391,7 @@ export const AgentChatInput = memo<AgentChatInputProps>(
|
||||
initialValue={initialValue}
|
||||
onChange={handleContentChange}
|
||||
onEnter={handleSubmit}
|
||||
disabled={isDisabled}
|
||||
disabled={isDisabled || isLoading}
|
||||
rows={4}
|
||||
autoFocus
|
||||
/>
|
||||
|
||||
@@ -88,23 +88,6 @@ const isChatMessage = (
|
||||
message: TypesGen.ChatMessage | undefined,
|
||||
): message is TypesGen.ChatMessage => Boolean(message);
|
||||
|
||||
const toOptimisticMessageParts = (
|
||||
inputParts: readonly TypesGen.ChatInputPart[],
|
||||
): readonly TypesGen.ChatMessagePart[] =>
|
||||
inputParts.map((part) => ({
|
||||
type: "text",
|
||||
...(part.text !== undefined ? { text: part.text } : {}),
|
||||
}));
|
||||
|
||||
const getOrderedMessagesFromStore = (
|
||||
store: ChatStoreHandle,
|
||||
): readonly TypesGen.ChatMessage[] => {
|
||||
const snapshot = store.getSnapshot();
|
||||
return snapshot.orderedMessageIDs
|
||||
.map((messageID) => snapshot.messagesByID.get(messageID))
|
||||
.filter(isChatMessage);
|
||||
};
|
||||
|
||||
interface AgentDetailTimelineProps {
|
||||
store: ChatStoreHandle;
|
||||
chatID: string;
|
||||
@@ -371,29 +354,21 @@ function useConversationEditingState(deps: {
|
||||
editingMessageId !== null ? editingMessageId : undefined;
|
||||
const queueEditID = editingQueuedMessageID;
|
||||
|
||||
// Clear input and editing state optimistically.
|
||||
setEditorInitialValue("");
|
||||
inputValueRef.current = "";
|
||||
if (editingMessageId !== null) {
|
||||
setEditingMessageId(null);
|
||||
setDraftBeforeHistoryEdit(null);
|
||||
}
|
||||
if (queueEditID !== null) {
|
||||
setEditingQueuedMessageID(null);
|
||||
setDraftBeforeQueueEdit(null);
|
||||
}
|
||||
|
||||
void onSend(message, editedMessageID)
|
||||
.then(() => {
|
||||
if (queueEditID !== null) {
|
||||
void onDeleteQueuedMessage(queueEditID);
|
||||
}
|
||||
})
|
||||
.catch(() => {
|
||||
// Restore input so the user can retry.
|
||||
setEditorInitialValue(message);
|
||||
inputValueRef.current = message;
|
||||
});
|
||||
void onSend(message, editedMessageID).then(() => {
|
||||
// Clear input and editing state on success.
|
||||
chatInputRef.current?.clear();
|
||||
chatInputRef.current?.focus();
|
||||
inputValueRef.current = "";
|
||||
if (editingMessageId !== null) {
|
||||
setEditingMessageId(null);
|
||||
setDraftBeforeHistoryEdit(null);
|
||||
}
|
||||
if (queueEditID !== null) {
|
||||
setEditingQueuedMessageID(null);
|
||||
setDraftBeforeQueueEdit(null);
|
||||
void onDeleteQueuedMessage(queueEditID);
|
||||
}
|
||||
});
|
||||
},
|
||||
[editingMessageId, editingQueuedMessageID, onDeleteQueuedMessage, onSend],
|
||||
);
|
||||
@@ -603,32 +578,12 @@ const AgentDetail: FC = () => {
|
||||
if (scrollContainerRef.current) {
|
||||
scrollContainerRef.current.scrollTop = 0;
|
||||
}
|
||||
const previousChatStatus = store.getSnapshot().chatStatus;
|
||||
const previousMessages = getOrderedMessagesFromStore(store);
|
||||
const messageIndex = previousMessages.findIndex(
|
||||
(msg) => msg.id === editedMessageID,
|
||||
);
|
||||
if (messageIndex !== -1) {
|
||||
const optimisticEditedMessage: TypesGen.ChatMessage = {
|
||||
...previousMessages[messageIndex],
|
||||
content: toOptimisticMessageParts(request.content),
|
||||
};
|
||||
store.replaceMessages([
|
||||
...previousMessages.slice(0, messageIndex),
|
||||
optimisticEditedMessage,
|
||||
]);
|
||||
}
|
||||
store.clearStreamState();
|
||||
store.setChatStatus("pending");
|
||||
try {
|
||||
await editMutation.mutateAsync({
|
||||
messageId: editedMessageID,
|
||||
req: request,
|
||||
});
|
||||
} catch (error) {
|
||||
store.replaceMessages(previousMessages);
|
||||
store.setChatStatus(previousChatStatus);
|
||||
throw error;
|
||||
} finally {
|
||||
setPendingEditMessageId(null);
|
||||
}
|
||||
@@ -646,38 +601,16 @@ const AgentDetail: FC = () => {
|
||||
scrollContainerRef.current.scrollTop = 0;
|
||||
}
|
||||
|
||||
// Inject an optimistic user message so the bubble appears in
|
||||
// the timeline immediately, without waiting for the server.
|
||||
const previousMessages = getOrderedMessagesFromStore(store);
|
||||
const previousChatStatus = store.getSnapshot().chatStatus;
|
||||
const optimisticMessage: TypesGen.ChatMessage = {
|
||||
id: -Date.now(),
|
||||
chat_id: agentId,
|
||||
created_at: new Date().toISOString(),
|
||||
role: "user",
|
||||
content: toOptimisticMessageParts(content),
|
||||
};
|
||||
store.upsertDurableMessage(optimisticMessage);
|
||||
// No optimistic rendering — the message will appear in the
|
||||
// timeline when the server confirms via the POST response or
|
||||
// via the SSE stream.
|
||||
store.clearStreamState();
|
||||
store.setChatStatus("pending");
|
||||
|
||||
try {
|
||||
const response = await sendMutation.mutateAsync(request);
|
||||
if (response.queued) {
|
||||
// The server queued the message instead of processing
|
||||
// it immediately (the agent is already busy). Roll back
|
||||
// the optimistic timeline message so it doesn't appear
|
||||
// as a sent message. The queue_update SSE event will
|
||||
// add it to the queued messages list.
|
||||
store.replaceMessages(previousMessages);
|
||||
store.setChatStatus(previousChatStatus);
|
||||
}
|
||||
} catch (error) {
|
||||
// Roll back the optimistic message so the timeline
|
||||
// returns to its previous state.
|
||||
store.replaceMessages(previousMessages);
|
||||
store.setChatStatus(previousChatStatus);
|
||||
throw error;
|
||||
const response = await sendMutation.mutateAsync(request);
|
||||
// When the server accepts the message immediately (not
|
||||
// queued), insert it into the store so it appears in the
|
||||
// timeline without waiting for the SSE stream.
|
||||
if (!response.queued && response.message) {
|
||||
store.upsertDurableMessage(response.message);
|
||||
}
|
||||
if (typeof window !== "undefined") {
|
||||
if (selectedModelConfigID) {
|
||||
|
||||
@@ -237,17 +237,6 @@ export const createChatStore = (): ChatStore => {
|
||||
const nextMessagesByID = new Map(state.messagesByID);
|
||||
nextMessagesByID.set(message.id, message);
|
||||
|
||||
// When a real server message (positive ID) arrives, remove any
|
||||
// optimistic placeholder (negative ID) for the same role so the
|
||||
// user doesn't momentarily see the message twice.
|
||||
if (message.id > 0) {
|
||||
for (const [id, existing] of nextMessagesByID) {
|
||||
if (id < 0 && existing.role === message.role) {
|
||||
nextMessagesByID.delete(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const needsReorder =
|
||||
!isDuplicate || nextMessagesByID.size !== state.messagesByID.size;
|
||||
const nextOrderedMessageIDs = needsReorder
|
||||
|
||||
@@ -139,33 +139,6 @@ describe("upsertDurableMessage", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("removes optimistic (negative-ID) messages when a real message arrives", () => {
|
||||
const store = createChatStore();
|
||||
const optimistic = makeMessage(-1, "user", "typing...");
|
||||
store.replaceMessages([optimistic]);
|
||||
expect(store.getSnapshot().messagesByID.has(-1)).toBe(true);
|
||||
|
||||
const real = makeMessage(5, "user", "typed!");
|
||||
store.upsertDurableMessage(real);
|
||||
|
||||
expect(store.getSnapshot().messagesByID.has(-1)).toBe(false);
|
||||
expect(store.getSnapshot().messagesByID.has(5)).toBe(true);
|
||||
});
|
||||
|
||||
it("only removes optimistic messages with the same role", () => {
|
||||
const store = createChatStore();
|
||||
const optimisticUser = makeMessage(-1, "user", "my prompt");
|
||||
const optimisticAssistant = makeMessage(-2, "assistant", "placeholder");
|
||||
store.replaceMessages([optimisticUser, optimisticAssistant]);
|
||||
|
||||
// A real "user" message arrives — only the user optimistic should
|
||||
// be removed, not the assistant one.
|
||||
store.upsertDurableMessage(makeMessage(5, "user", "real prompt"));
|
||||
|
||||
expect(store.getSnapshot().messagesByID.has(-1)).toBe(false);
|
||||
expect(store.getSnapshot().messagesByID.has(-2)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not reorder when updating an existing message in place", () => {
|
||||
const store = createChatStore();
|
||||
store.upsertDurableMessage(makeMessage(1, "user", "first"));
|
||||
|
||||
Reference in New Issue
Block a user