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,