From cee231b136ca55a0ab102d7f8c1e410d26d4b8c7 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Wed, 24 Jun 2026 14:43:07 +0300 Subject: [PATCH 1/2] fix(ai-chat): reset chat on New chat during the first turn's stream (#161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pressing "New chat" while a brand-new chat's first turn was still streaming only removed the role badge — the thread, session and stream were kept. The thread remount is driven by a render-phase reconciler in useChatSession that fires only when activeChatId !== thread.chatId; on a not-yet-adopted new chat both are null, so startNewChat's setActiveChatId(null) was a no-op and nothing remounted. - useChatSession: add startFreshThread(), which unconditionally dispatches a reconcile to a fresh mount key so ChatThread remounts even when activeChatId stays null. startNewChat now calls it. - Guard against the abandoned thread's late finish: ai@6's useChat does not abort on unmount and proxies its callbacks, so onFinish/onError of the chat the user just left still fire and would adopt it back. onTurnFinished now takes the finishing thread's key and, when it no longer matches the mounted thread, only refreshes the chat list — no adoption, no fallback arming, no per-chat message invalidation. ChatThread forwards its threadKey in both onFinish and onError. An omitted key (legacy callers/tests) counts as the current thread, preserving the #137 adoption behavior. Tests: 3 new useChatSession cases (fresh-thread remount with activeChatId null, abandoned-thread finish rejected, current-thread finish still adopts). Full ai-chat suite green (183). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/components/ai-chat-window.tsx | 25 +++++++- .../ai-chat/components/chat-thread.tsx | 12 +++- .../ai-chat/hooks/use-chat-session.test.tsx | 44 ++++++++++++- .../ai-chat/hooks/use-chat-session.ts | 62 ++++++++++++++++++- 4 files changed, 133 insertions(+), 10 deletions(-) diff --git a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx index 5f6b1dde..99da5a9f 100644 --- a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx +++ b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx @@ -185,8 +185,13 @@ export default function AiChatWindow() { // The invalidate closures are passed inline: `onTurnFinished` is read live by // useChat's onFinish (never in an effect dep array), so their identity does not // matter — no memoization ceremony needed. - const { threadKey, waitingForHistory, onTurnFinished, cancelPendingAdoption } = - useChatSession({ + const { + threadKey, + waitingForHistory, + onTurnFinished, + startFreshThread, + cancelPendingAdoption, + } = useChatSession({ activeChatId, setActiveChatId, chats, @@ -205,12 +210,25 @@ export default function AiChatWindow() { // just-failed chat after they chose a fresh one. const startNewChat = useCallback((): void => { cancelPendingAdoption(); + // Force a fresh thread UNCONDITIONALLY. If the user is still on a brand-new, + // not-yet-adopted chat (activeChatId === null) while its first turn streams, + // setActiveChatId(null) is a no-op and the render-phase reconciler would not + // remount — leaving the streaming thread in place (#161). startFreshThread + // guarantees a clean remount; the abandoned thread's late finish is rejected + // by the threadKey guard in the session hook. + startFreshThread(); setActiveChatId(null); setHistoryOpen(false); setDraft(""); // Default the picker back to "Universal assistant" for the fresh chat. setSelectedRoleId(null); - }, [cancelPendingAdoption, setActiveChatId, setDraft, setSelectedRoleId]); + }, [ + cancelPendingAdoption, + startFreshThread, + setActiveChatId, + setDraft, + setSelectedRoleId, + ]); const selectChat = useCallback( (chatId: string): void => { @@ -621,6 +639,7 @@ export default function AiChatWindow() { onRolePicked={(role) => setSelectedRoleId(role.id)} assistantName={currentRole?.name} onTurnFinished={onTurnFinished} + threadKey={threadKey} liveStateRef={liveThreadRef} onLiveTurnTokens={setLiveTurnTokens} /> diff --git a/apps/client/src/features/ai-chat/components/chat-thread.tsx b/apps/client/src/features/ai-chat/components/chat-thread.tsx index 3898136e..4e681e2f 100644 --- a/apps/client/src/features/ai-chat/components/chat-thread.tsx +++ b/apps/client/src/features/ai-chat/components/chat-thread.tsx @@ -67,7 +67,12 @@ interface ChatThreadProps { * new chat, adopts the freshly created chat id. `serverChatId` is the * authoritative id the server streamed on the assistant message metadata, or * undefined on a failed turn — see adopt-chat-id.ts for the full #137 design. */ - onTurnFinished: (serverChatId?: string) => void; + onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void; + /** This thread's mount key (the same value the parent uses as React `key`). + * Forwarded back through onTurnFinished so the session hook can tell a finish + * from THIS still-mounted thread from a late finish of an abandoned thread the + * user already left via New chat / switch (#161). */ + threadKey?: string; /** 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 @@ -123,6 +128,7 @@ export default function ChatThread({ onRolePicked, assistantName, onTurnFinished, + threadKey, liveStateRef, onLiveTurnTokens, }: ChatThreadProps) { @@ -265,7 +271,7 @@ export default function ChatThread({ // Forward the authoritative server chatId (streamed on the assistant // message metadata) so the parent adopts the REAL created chat id for a new // chat — see adopt-chat-id.ts for the full #137 design. - onTurnFinished(extractServerChatId(message)); + onTurnFinished(extractServerChatId(message), threadKey); // 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); @@ -286,7 +292,7 @@ export default function ChatThread({ // Surface the raw failure in the browser console (devtools) for debugging; // the UI separately shows a friendly classified banner (see errorView). console.error("AI chat stream error:", streamError); - onTurnFinished(); + onTurnFinished(undefined, threadKey); }, }); diff --git a/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx b/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx index 8104d1e6..7df952d6 100644 --- a/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx +++ b/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { renderHook } from "@testing-library/react"; +import { renderHook, act } from "@testing-library/react"; import { useChatSession } from "./use-chat-session"; import type { UseChatSessionOptions } from "./use-chat-session"; @@ -187,6 +187,48 @@ describe("useChatSession", () => { expect(result.current.threadKey).toBe("C"); }); + it("#161: startFreshThread remounts even when activeChatId stays null (New chat mid-stream)", () => { + // The bug: pressing New chat while still on a brand-new, not-yet-adopted chat + // leaves activeChatId === null, so the render-phase reconciler never fires. + // startFreshThread must remount UNCONDITIONALLY (a new mount key). + const { result } = setup({ activeChatId: null, chats: { items: [] } }); + const keyBefore = result.current.threadKey; + act(() => result.current.startFreshThread()); + expect(result.current.threadKey).not.toBe(keyBefore); + }); + + it("#161: a late finish from an ABANDONED thread does not adopt or invalidate messages", () => { + const { + result, + setActiveChatId, + onInvalidateChatList, + onInvalidateChatMessages, + } = setup({ activeChatId: null, chats: { items: [{ id: "x" }] } }); + const abandonedKey = result.current.threadKey; + // User pressed New chat mid-stream → fresh thread (new mount key). + act(() => result.current.startFreshThread()); + expect(result.current.threadKey).not.toBe(abandonedKey); + // The left-behind thread's onFinish fires late, carrying ITS (now stale) key + // and the server id of the chat the user just left. It must NOT be adopted. + act(() => result.current.onTurnFinished("A", abandonedKey)); + expect(setActiveChatId).not.toHaveBeenCalled(); + expect(onInvalidateChatMessages).not.toHaveBeenCalled(); + // The abandoned chat should still surface in the history list. + expect(onInvalidateChatList).toHaveBeenCalled(); + }); + + it("#161: a finish from the CURRENT thread (matching key) still adopts", () => { + const { result, setActiveChatId } = setup({ + activeChatId: null, + chats: { items: [{ id: "x" }] }, + }); + // Same thread that is mounted reports its finish with the matching key. + act(() => + result.current.onTurnFinished("A", result.current.threadKey), + ); + expect(setActiveChatId).toHaveBeenCalledWith("A"); + }); + it("waitingForHistory gates the loader only while opening an unloaded existing chat", () => { // Open an existing chat whose history is still loading => loader on. const { result, rerender } = setup({ diff --git a/apps/client/src/features/ai-chat/hooks/use-chat-session.ts b/apps/client/src/features/ai-chat/hooks/use-chat-session.ts index 998f2631..f55da7a0 100644 --- a/apps/client/src/features/ai-chat/hooks/use-chat-session.ts +++ b/apps/client/src/features/ai-chat/hooks/use-chat-session.ts @@ -32,8 +32,20 @@ export interface UseChatSessionResult { /** Show the history loader instead of the live thread. */ waitingForHistory: boolean; /** Call when a turn finishes; `serverChatId` is the authoritative streamed id - * (undefined on a failed turn). Handles new-chat id adoption + invalidations. */ - onTurnFinished: (serverChatId?: string) => void; + * (undefined on a failed turn). `finishingThreadKey` is the mount key of the + * thread that produced this callback — when it no longer matches the mounted + * thread (the user pressed New chat / switched mid-stream), the call is from an + * abandoned thread and must NOT adopt or re-arm the fallback. Omitting it (old + * callers / tests) treats the call as belonging to the current thread. Handles + * new-chat id adoption + invalidations. */ + onTurnFinished: (serverChatId?: string, finishingThreadKey?: string) => void; + /** Force a brand-new, empty thread (new mount key, no chat id) UNCONDITIONALLY. + * The render-phase reconciler only remounts when `activeChatId` actually + * changes; pressing "New chat" while already in a not-yet-adopted new chat + * leaves `activeChatId === null` (a no-op for the atom), so the reconciler + * never fires and the stale streaming thread stays mounted (#161). The window + * calls this from startNewChat to guarantee a fresh thread regardless. */ + startFreshThread: () => void; /** Disarm any pending error-path new-chat fallback. The window calls this from * startNewChat/selectChat so a late refetch can't yank the user back into a * just-failed chat after they explicitly moved on. */ @@ -78,6 +90,14 @@ export function useChatSession( const activeChatIdRef = useRef(activeChatId); activeChatIdRef.current = activeChatId; + // Live mirror of the mounted thread's key, read by onTurnFinished to tell a + // current-thread finish from an ABANDONED one. ai@6's useChat does not abort + // its request on unmount, and its callbacks are proxied so onFinish/onError of + // a thread the user already left (via New chat / switch) still fire AFTER that + // thread unmounts. By then this ref holds the NEW thread's key, so comparing it + // to the key the finishing thread reports rejects the abandoned turn (#161). + const threadKeyRef = useRef(""); + // The mounted thread's identity: ONE atomic value tying ChatThread's mount key // (`thread.key`) to the chat id that mounted thread holds (`thread.chatId`). // Consolidating these makes the "key vs chat id diverged" state unrepresentable @@ -94,6 +114,10 @@ export function useChatSession( : switchThread(activeChatId), ); + // Keep the live mirror pointed at the currently-mounted thread's key so a late + // onTurnFinished can be matched against it (see threadKeyRef above). + threadKeyRef.current = thread.key; + // Error-path fallback for new-chat id adoption. When a brand-new chat's first // turn errors BEFORE the server's `start` chunk, no authoritative chatId ever // reaches the client, so the primary metadata adoption cannot run. We then ARM @@ -111,7 +135,21 @@ export function useChatSession( // yet) we adopt the server's AUTHORITATIVE streamed id (never the newest in the // list, which races a second tab — #137; see adopt-chat-id.ts). const onTurnFinished = useCallback( - (serverChatId?: string) => { + (serverChatId?: string, finishingThreadKey?: string) => { + // Reject a finish from an ABANDONED thread. After the user pressed New chat + // (or switched chats) mid-stream, the left-behind thread's onFinish/onError + // still fire (ai@6 does not abort on unmount). Adopting/arming off that late + // callback would yank the user back into the chat they just left (#161). + // `undefined` (legacy callers/tests) is treated as the current thread. + const isCurrentThread = + finishingThreadKey === undefined || + finishingThreadKey === threadKeyRef.current; + if (!isCurrentThread) { + // Still surface the abandoned chat in the history list, but do NOT adopt, + // arm the fallback, or invalidate per-chat messages (no thread shows it). + onInvalidateChatList(); + return; + } // Read the live id from the ref, not the closure: on a failed turn this can // run twice in one turn (onFinish + onError) before any re-render, and the // primary branch below updates the ref so the second call sees the adopted id. @@ -229,10 +267,28 @@ export function useChatSession( pendingNewChatRef.current = null; }, []); + // Force a fresh, empty thread regardless of the current `activeChatId`. The + // render-phase reconciler only remounts on an activeChatId CHANGE, so "New chat" + // pressed while already in a not-yet-adopted new chat (activeChatId stays null) + // would otherwise leave the in-flight streaming thread mounted (#161). Dispatch + // `reconcile` to chatId:null with a brand-new key so React remounts ChatThread + // (a fresh useChat store). Disarm any armed fallback too. After this dispatch + // thread.chatId is null; the window also sets activeChatId to null, so the + // render-phase reconciler then finds them equal and does not double-remount. + const startFreshThread = useCallback(() => { + pendingNewChatRef.current = null; + dispatch({ + type: "reconcile", + chatId: null, + newKey: `new-${generateId()}`, + }); + }, []); + return { threadKey: thread.key, waitingForHistory, onTurnFinished, + startFreshThread, cancelPendingAdoption, }; } -- 2.49.1 From 8f9d7fe0bd3ab1bd1be56d83ec7de1ba40dfdd99 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 16:59:09 +0300 Subject: [PATCH 2/2] test(ai-chat): cover the #161 New-chat-during-stream reset + changelog Address PR #162 review must-fixes: - CHANGELOG: add an [Unreleased] > Fixed bullet for #161 (New chat during the first turn's stream now resets the thread; a late refetch/onFinish from the abandoned thread no longer pulls the user back in). - Re-anchor the stale startNewChat/cancelPendingAdoption test to selectChat: startNewChat now calls startFreshThread, but cancelPendingAdoption still backs selectChat, so the disarm guard is decoupled and kept valid. - Add a test pinning that startFreshThread() disarms the armed error-path fallback (the justification for dropping cancelPendingAdoption from startNewChat). - Add a ChatThread render test (mocked useChat) asserting the onError branch forwards onTurnFinished(undefined, threadKey). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 ++ .../ai-chat/components/chat-thread.test.tsx | 94 +++++++++++++++++++ .../ai-chat/hooks/use-chat-session.test.tsx | 34 ++++++- 3 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 apps/client/src/features/ai-chat/components/chat-thread.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c2aa9c9..a301635e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 no longer froze on the previous step's authoritative usage; the current step's estimate is combined per-component with `max`, so the count rises smoothly and never jumps backwards. (#163) +- **AI chat: "New chat" pressed during the first turn's stream now resets the + thread instead of leaving the old turn streaming.** While a brand-new, + not-yet-adopted chat streamed its first turn, hitting "New chat" left + `activeChatId === null` (a no-op for the atom), so the reconciler never + remounted and the in-flight thread kept streaming behind the fresh one — and a + late refetch / late `onFinish` from that abandoned thread could yank the user + back into the chat they just left. "New chat" now forces a fresh empty thread + unconditionally and the finished thread's mount key is checked so a late + callback from an abandoned thread no longer adopts or re-arms the fallback. + (#161) ## [0.93.0] - 2026-06-21 diff --git a/apps/client/src/features/ai-chat/components/chat-thread.test.tsx b/apps/client/src/features/ai-chat/components/chat-thread.test.tsx new file mode 100644 index 00000000..d1c71bc3 --- /dev/null +++ b/apps/client/src/features/ai-chat/components/chat-thread.test.tsx @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, act } from "@testing-library/react"; +import { MantineProvider } from "@mantine/core"; + +// Capture the options ChatThread passes to useChat so the test can drive the +// hook's terminal callbacks (here: onError) directly, without a real stream. The +// box is created via vi.hoisted so the hoisted vi.mock factory below can close +// over it. +const { useChatBox } = vi.hoisted(() => ({ + useChatBox: { options: null as unknown as Record | null }, +})); + +// Mock the AI SDK hook: record the options and return an inert, ready store so +// ChatThread renders without any network/streaming machinery. +vi.mock("@ai-sdk/react", () => ({ + useChat: (options: Record) => { + useChatBox.options = options; + return { + messages: [], + sendMessage: vi.fn(), + status: "ready", + stop: vi.fn(), + error: null, + }; + }, +})); + +// Stub react-i18next so `t` returns the key (other component tests use the same +// pattern); ChatThread's rendered chrome is irrelevant to this wiring test. +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +// Mock the heavy presentational children to trivial stubs — this test only +// exercises the onError → onTurnFinished wiring, not their rendering. +vi.mock("@/features/ai-chat/components/message-list.tsx", () => ({ + default: () => null, +})); +vi.mock("@/features/ai-chat/components/chat-input.tsx", () => ({ + default: () => null, +})); +vi.mock("@/features/ai-chat/components/role-cards.tsx", () => ({ + default: () => null, +})); +vi.mock("@/features/ai-chat/components/chat-error-alert.tsx", () => ({ + default: () => null, +})); +vi.mock("@/features/ai-chat/components/chat-stopped-notice.tsx", () => ({ + default: () => null, +})); + +import ChatThread from "./chat-thread"; + +// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts. + +describe("ChatThread onError wiring (#161)", () => { + beforeEach(() => { + useChatBox.options = null; + vi.clearAllMocks(); + }); + + it("onError calls onTurnFinished with (undefined, threadKey) so a late error from an abandoned thread is rejected", () => { + const onTurnFinished = vi.fn(); + // Silence the deliberate console.error ChatThread logs for devtools. + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + + render( + + + , + ); + + const options = useChatBox.options; + expect(options).not.toBeNull(); + expect(typeof options?.onError).toBe("function"); + + // Drive the captured onError exactly as the AI SDK would on a stream error. + act(() => { + (options!.onError as (e: Error) => void)(new Error("stream blew up")); + }); + + // The thread's own mount key must be forwarded with NO server id, so the + // session hook can reject this finish if the thread has been abandoned. + expect(onTurnFinished).toHaveBeenCalledWith(undefined, "thread-key-1"); + + consoleError.mockRestore(); + }); +}); diff --git a/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx b/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx index 7588b1e4..164f3976 100644 --- a/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx +++ b/apps/client/src/features/ai-chat/hooks/use-chat-session.test.tsx @@ -120,16 +120,40 @@ describe("useChatSession", () => { expect(setActiveChatId).not.toHaveBeenCalledWith("new"); }); - it("startNewChat while already in a new chat: cancelPendingAdoption stops a late refetch adopting the failed chat", () => { - // The Warning path the render-phase reconciler can't catch: pressing "New - // chat" while already in a new chat keeps activeChatId === null (a no-op for - // the atom), so only the explicit cancelPendingAdoption() disarms. + it("cancelPendingAdoption (selectChat) disarms a late refetch from adopting the just-failed chat", () => { + // cancelPendingAdoption is the explicit disarm the window calls from + // selectChat: switching to a chat whose id == null is a no-op for the atom, so + // the render-phase reconciler never fires and only this call disarms an armed + // error-path fallback. (startNewChat no longer routes through here — it calls + // startFreshThread, covered by the next test — but cancelPendingAdoption still + // backs selectChat, so this guard must hold.) const { result, rerender, setActiveChatId } = setup({ activeChatId: null, chats: { items: [{ id: "x" }] }, }); result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"]) - result.current.cancelPendingAdoption(); // window calls this from startNewChat + result.current.cancelPendingAdoption(); // window calls this from selectChat + // The just-failed row lands in a late refetch; it must NOT be adopted. + rerender({ + activeChatId: null, + chats: { items: [{ id: "x" }, { id: "failed" }] }, + }); + expect(setActiveChatId).not.toHaveBeenCalledWith("failed"); + }); + + it("#161: startFreshThread disarms the armed error-path fallback (New chat during the first turn)", () => { + // Pressing "New chat" while already in a not-yet-adopted new chat keeps + // activeChatId === null, so the render-phase reconciler never fires. The + // window now calls startFreshThread() (NOT cancelPendingAdoption) to force a + // fresh thread; this test pins the load-bearing fact that startFreshThread + // ALSO nulls pendingNewChatRef, so a late refetch of the just-failed row can't + // yank the user back into the abandoned chat. + const { result, rerender, setActiveChatId } = setup({ + activeChatId: null, + chats: { items: [{ id: "x" }] }, + }); + result.current.onTurnFinished(undefined); // first turn failed → arm (before=["x"]) + act(() => result.current.startFreshThread()); // "New chat" → fresh thread + disarm // The just-failed row lands in a late refetch; it must NOT be adopted. rerender({ activeChatId: null, -- 2.49.1