fix(ai-chat): adopt the server-returned chat id, not the newest in the list
A brand-new chat (activeChatId === null) had no way to learn the id of the row the server created: the SSE stream never returned it, so the client adopted the NEWEST chat in the per-user list (chats.items[0]). With two tabs open, a second tab creating a chat at ~the same time made its row the newest, so the first tab adopted the wrong id — its later turns persisted into the other chat and the agent rebuilt history from it (commands leaked between chats), while the live UI still showed the original conversation. (#137) The server now attaches the authoritative chatId to the streamed assistant message via the AI SDK messageMetadata on the 'start' part, so it reaches the client on the first chunk. The client reads message.metadata.chatId in useChat's onFinish and adopts that id in place (no remount, so the live turn and the thread's chatIdRef follow the real id and the next turn targets the right chat). The chats.items[0] guess and the adoptNewChat ref are removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -101,7 +101,8 @@ function clampGeom(g: { left: number; top: number; width: number; height: number
|
||||
/**
|
||||
* Floating, draggable, resizable, minimizable AI chat window. Replaces the
|
||||
* former right-aside `AiChatPanel`: it owns ALL chat orchestration (active
|
||||
* chat, new chat, adopt-new-chat, open-page context, token sum) and wraps the
|
||||
* chat, new chat, in-place id adoption from streamed metadata, open-page
|
||||
* context, token sum) and wraps the
|
||||
* reused inner components (ConversationList + ChatThread) in window chrome
|
||||
* ported from the GitmostAgent.jsx design.
|
||||
*/
|
||||
@@ -132,10 +133,6 @@ export default function AiChatWindow() {
|
||||
// left partly off-screen).
|
||||
const [geom, setGeom] = useAtom(aiChatWindowGeomAtom);
|
||||
|
||||
// Track whether we are awaiting the id of a just-created (new) chat, so we
|
||||
// can adopt it once the chat list refreshes after the first turn finishes.
|
||||
const adoptNewChat = useRef(false);
|
||||
|
||||
// Latch: the chat id whose full persisted history has finished loading while
|
||||
// its thread is mounted. Used so a later BACKGROUND refetch (the post-turn
|
||||
// messages invalidation) never tears the live thread back down to the loader.
|
||||
@@ -197,9 +194,6 @@ export default function AiChatWindow() {
|
||||
: null;
|
||||
|
||||
const startNewChat = useCallback((): void => {
|
||||
// Cancel any pending adoption so a just-finished new chat can't yank the user
|
||||
// back here after they explicitly started a fresh one.
|
||||
adoptNewChat.current = false;
|
||||
setActiveChatId(null);
|
||||
setHistoryOpen(false);
|
||||
setDraft("");
|
||||
@@ -209,8 +203,6 @@ export default function AiChatWindow() {
|
||||
|
||||
const selectChat = useCallback(
|
||||
(chatId: string): void => {
|
||||
// Cancel any pending adoption so it can't override an explicit selection.
|
||||
adoptNewChat.current = false;
|
||||
setActiveChatId(chatId);
|
||||
setHistoryOpen(false);
|
||||
setDraft("");
|
||||
@@ -222,25 +214,42 @@ export default function AiChatWindow() {
|
||||
);
|
||||
|
||||
// After a turn finishes, refresh the chat list. For a brand-new chat (no id
|
||||
// yet), the server has just created the row; adopt the newest chat id so the
|
||||
// thread switches from "new" to the persisted chat (and loads its history on
|
||||
// later opens).
|
||||
const onTurnFinished = useCallback(() => {
|
||||
if (activeChatId === null) adoptNewChat.current = true;
|
||||
queryClient.invalidateQueries({ queryKey: AI_CHATS_RQ_KEY });
|
||||
// Re-sync the persisted message rows for the active chat so the Markdown
|
||||
// export and the token counters reflect the turn that just finished. The
|
||||
// live thread renders from its own useChat store (stable threadKey / store
|
||||
// id), so refetching these rows never re-seeds or tears down the open
|
||||
// thread. For a brand-new chat activeChatId is still null here; that chat's
|
||||
// first row load happens right after id adoption, and every later turn hits
|
||||
// this invalidation with the adopted id.
|
||||
if (activeChatId) {
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: AI_CHAT_MESSAGES_RQ_KEY(activeChatId),
|
||||
});
|
||||
}
|
||||
}, [activeChatId, queryClient]);
|
||||
// yet), the server has just created the row and reported its AUTHORITATIVE id
|
||||
// via the streamed assistant message metadata (`serverChatId`); we adopt THAT
|
||||
// exact id so the thread switches from "new" to its OWN persisted chat — never
|
||||
// guessing the newest chat in the list, which would adopt a SIBLING chat that
|
||||
// a second tab created in the same moment and leak this tab's later turns into
|
||||
// it (the two-tab adoption race, #137).
|
||||
const onTurnFinished = useCallback(
|
||||
(serverChatId?: string) => {
|
||||
if (activeChatId === null && serverChatId) {
|
||||
// In-place adoption: move the active chat AND the live-thread marker to
|
||||
// the real id together, so the render-phase switch check below sees no
|
||||
// "switch" and keeps the SAME mounted thread (its useChat already holds
|
||||
// the just-finished turn, and its chatIdRef now mirrors the real id so
|
||||
// the SECOND turn sends the correct chatId) instead of remounting and
|
||||
// re-seeding from not-yet-persisted history. React's automatic batching
|
||||
// inside this callback lands both updates in one render, so the guard
|
||||
// never observes the new activeChatId with a stale liveThreadChatId.
|
||||
setLiveThreadChatId(serverChatId);
|
||||
setActiveChatId(serverChatId);
|
||||
}
|
||||
queryClient.invalidateQueries({ queryKey: AI_CHATS_RQ_KEY });
|
||||
// Re-sync the persisted message rows for the active chat so the Markdown
|
||||
// export and the token counters reflect the turn that just finished. The
|
||||
// live thread renders from its own useChat store (stable threadKey / store
|
||||
// id), so refetching these rows never re-seeds or tears down the open
|
||||
// thread. For a brand-new chat activeChatId is still null here; that chat's
|
||||
// first row load happens right after id adoption, and every later turn hits
|
||||
// this invalidation with the adopted id.
|
||||
if (activeChatId) {
|
||||
queryClient.invalidateQueries({
|
||||
queryKey: AI_CHAT_MESSAGES_RQ_KEY(activeChatId),
|
||||
});
|
||||
}
|
||||
},
|
||||
[activeChatId, queryClient, setActiveChatId],
|
||||
);
|
||||
|
||||
// The active chat object (for its title) and an export gate: only enable the
|
||||
// export button when an existing chat with loaded persisted rows is active.
|
||||
@@ -294,26 +303,11 @@ export default function AiChatWindow() {
|
||||
notifications.show({ message: t("Copied") });
|
||||
}, [activeChatId, messageRows, activeChat, clipboard, t]);
|
||||
|
||||
// When awaiting a new chat's id, adopt the most-recent chat (the list is
|
||||
// ordered newest-first) once it appears.
|
||||
useEffect(() => {
|
||||
if (!adoptNewChat.current) return;
|
||||
const newest = chats?.items?.[0];
|
||||
if (newest) {
|
||||
adoptNewChat.current = false;
|
||||
// In-place adoption: move the active chat AND the live-thread marker to the
|
||||
// new id together, so the threadKey derivation below sees no "switch" and
|
||||
// keeps the SAME mounted thread (its useChat already holds the finished
|
||||
// turn) instead of remounting and re-seeding from not-yet-persisted history.
|
||||
// ASSUMPTION: these two updates (jotai atom + useState) must land in ONE
|
||||
// render so the render-phase guard never observes the new activeChatId with
|
||||
// a stale liveThreadChatId (which would wrongly remount). React 18 automatic
|
||||
// batching inside this effect callback guarantees that; if the store/atom
|
||||
// mechanism ever changes, gate adoption on an explicit flag instead.
|
||||
setLiveThreadChatId(newest.id);
|
||||
setActiveChatId(newest.id);
|
||||
}
|
||||
}, [chats, setActiveChatId]);
|
||||
// NOTE: new-chat id adoption is no longer a heuristic effect. The server now
|
||||
// reports the authoritative created chat id on the streamed assistant message
|
||||
// metadata, and `onTurnFinished(serverChatId)` adopts THAT id in place — see
|
||||
// above. The old "newest chat in the list" guess was removed because it raced
|
||||
// a second tab's simultaneous new chat (#137).
|
||||
|
||||
// Adjust the derived thread state during render when the active chat genuinely
|
||||
// changes — the React-sanctioned alternative to an effect (it re-renders before
|
||||
|
||||
@@ -58,8 +58,12 @@ interface ChatThreadProps {
|
||||
* forwarded to MessageList. Absent => the generic "AI agent". */
|
||||
assistantName?: string;
|
||||
/** Called when a turn finishes; the parent refreshes the chat list and, for
|
||||
* a new chat, adopts the freshly created chat id. */
|
||||
onTurnFinished: () => void;
|
||||
* a new chat, adopts the freshly created chat id. `serverChatId` is the
|
||||
* authoritative id the server attached to the streamed assistant message
|
||||
* metadata (see the server's `messageMetadata`); the parent adopts THIS for a
|
||||
* new chat instead of guessing the newest chat in the list (fixes the two-tab
|
||||
* adoption race, #137). Undefined on a failed turn that produced no metadata. */
|
||||
onTurnFinished: (serverChatId?: string) => void;
|
||||
/** Parent-owned ref that this thread keeps updated with its live useChat
|
||||
* snapshot (full message list + streaming flag), so the header's
|
||||
* "Copy chat" export can include the in-progress, not-yet-persisted
|
||||
@@ -246,8 +250,14 @@ export default function ChatThread({
|
||||
// sending after the user hit Stop — or blindly retrying after a failure —
|
||||
// would be wrong, so on Stop/disconnect/error the queue is left intact for
|
||||
// the user to decide.
|
||||
onFinish: ({ isAbort, isDisconnect, isError }) => {
|
||||
onTurnFinished();
|
||||
onFinish: ({ message, isAbort, isDisconnect, isError }) => {
|
||||
// The server attaches the authoritative chatId to the streamed assistant
|
||||
// message metadata (see `messageMetadata` in ai-chat.service.ts). Forward it
|
||||
// so the parent adopts the REAL created chat id for a new chat, rather than
|
||||
// guessing the newest chat in the list (which races a second tab — #137).
|
||||
const serverChatId = (message?.metadata as { chatId?: string } | undefined)
|
||||
?.chatId;
|
||||
onTurnFinished(serverChatId);
|
||||
// Show a neutral "stopped" marker for an aborted turn; the red error banner
|
||||
// (via `error`) already covers isError, and a clean finish clears any marker.
|
||||
if (isError) setStopNotice(null);
|
||||
|
||||
@@ -504,6 +504,16 @@ export class AiChatService {
|
||||
// does not buffer responses by default.
|
||||
result.pipeUIMessageStreamToResponse(res.raw, {
|
||||
headers: { 'X-Accel-Buffering': 'no' },
|
||||
// Surface the authoritative chatId on the streamed assistant UI message so
|
||||
// the client adopts the REAL id of the row we created, instead of guessing
|
||||
// the newest chat in its list. `messageMetadata` is invoked by the AI SDK
|
||||
// on the `start` and `finish` stream parts (ai@6); we attach `chatId` on the
|
||||
// `start` part so it reaches the client (as message.metadata.chatId) at the
|
||||
// very first chunk — before any second tab can race a newer chat into the
|
||||
// list. This fixes the two-tab "adoption race" (#137) where a new chat in
|
||||
// tab A could adopt tab B's id and leak its turns into the wrong row.
|
||||
messageMetadata: ({ part }) =>
|
||||
part.type === 'start' ? { chatId } : undefined,
|
||||
onError: (error: unknown) => {
|
||||
// Reuse the shared formatter so provider error formatting stays
|
||||
// unified between the log line and the streamed error message.
|
||||
|
||||
Reference in New Issue
Block a user