diff --git a/packages/views/chat/components/use-chat-controller.test.tsx b/packages/views/chat/components/use-chat-controller.test.tsx index 6c2861552e..2f7d3d771f 100644 --- a/packages/views/chat/components/use-chat-controller.test.tsx +++ b/packages/views/chat/components/use-chat-controller.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { act, renderHook } from "@testing-library/react"; import type { Agent, ChatSession } from "@multica/core/types"; @@ -17,6 +17,7 @@ const h = vi.hoisted(() => { return { store, archivedMutate: vi.fn(), + markReadMutate: vi.fn(), // useQuery reads these so each test can vary the loaded data. sessions: [] as ChatSession[], agents: [] as Agent[], @@ -45,7 +46,7 @@ vi.mock("@multica/core/hooks/use-file-upload", () => ({ })); vi.mock("@multica/core/chat/mutations", () => ({ useCreateChatSession: () => ({ mutateAsync: vi.fn() }), - useMarkChatSessionRead: () => ({ mutate: vi.fn() }), + useMarkChatSessionRead: () => ({ mutate: h.markReadMutate }), useSetChatSessionArchived: () => ({ mutate: h.archivedMutate }), })); vi.mock("@multica/core/chat", () => ({ @@ -193,3 +194,55 @@ describe("useChatController.archiveSession", () => { expect(h.archivedMutate).toHaveBeenCalledWith({ sessionId: "sA", archived: true }); }); }); + +// MUL-4360 mount race: `activeSessionId` is persisted, so on a bare `/chat` +// navigation the page restores the last session as active for one frame before +// its URL→store effect clears it back to null. The auto-mark-read must NOT fire +// for that transiently-active session — otherwise the badge vanishes though the +// user never opened it (the exact "no active session yet the red dot cleared" +// report). The read is deferred a tick and cancelled when activeSessionId moves. +describe("useChatController auto mark-read", () => { + const unread = makeSession({ + id: "sA", + agent_id: "agent-a", + has_unread: true, + unread_count: 2, + }); + + beforeEach(() => { + vi.useFakeTimers(); + h.store.activeSessionId = null; + h.store.selectedAgentId = null; + h.markReadMutate.mockClear(); + h.sessions = [unread]; + h.agents = [agentA]; + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("marks read a session that stays active past the tick", () => { + h.store.activeSessionId = "sA"; + renderHook(() => useChatController()); + + // Deferred, not synchronous — nothing fires on the mount frame. + expect(h.markReadMutate).not.toHaveBeenCalled(); + + act(() => vi.advanceTimersByTime(1)); + expect(h.markReadMutate).toHaveBeenCalledWith("sA"); + }); + + it("does NOT mark read a session that was only momentarily active on mount", () => { + // Mount restores sA as active (persisted, unread)... + h.store.activeSessionId = "sA"; + const { rerender } = renderHook(() => useChatController()); + + // ...then the page's URL→store effect clears it before the tick elapses. + h.store.activeSessionId = null; + rerender(); + + act(() => vi.advanceTimersByTime(1)); + expect(h.markReadMutate).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/views/chat/components/use-chat-controller.ts b/packages/views/chat/components/use-chat-controller.ts index 74aedea202..a181c99651 100644 --- a/packages/views/chat/components/use-chat-controller.ts +++ b/packages/views/chat/components/use-chat-controller.ts @@ -308,13 +308,31 @@ export function useChatController(opts?: { isActive?: boolean }) { // Auto mark-as-read whenever the user is actively looking at a session with // unread state. `isActive` lets the caller say "my surface is on screen": // the floating overlay passes `isOpen`, the tab passes `true`. + // + // The read is deferred by a tick and cancelled on cleanup, so a session that + // is only *momentarily* active never gets marked read. This is the fix for + // MUL-4360's mount race: `activeSessionId` is persisted, so on a bare `/chat` + // navigation the page restores the last session for one frame before its + // URL→store effect (which runs AFTER this hook's effects, since the hook is + // called first) clears it back to null. Without the defer, that restored-but- + // never-opened session was marked read in that gap — its badge vanished + // though the user never opened it (right pane still shows "select a chat"). + // Deferring lets the subsequent activeSessionId change cancel the pending + // read via cleanup; the store re-check is a belt-and-suspenders guard. Only a + // session that stays active past the tick — a real select, deep link, or + // refresh — is read. const currentHasUnread = sessions.find((s) => s.id === activeSessionId)?.has_unread ?? false; useEffect(() => { if (!isActive || !activeSessionId) return; if (!currentHasUnread) return; - uiLogger.info("auto markRead", { sessionId: activeSessionId }); - markRead.mutate(activeSessionId); + const sessionId = activeSessionId; + const timer = setTimeout(() => { + if (useChatStore.getState().activeSessionId !== sessionId) return; + uiLogger.info("auto markRead", { sessionId }); + markRead.mutate(sessionId); + }, 0); + return () => clearTimeout(timer); // eslint-disable-next-line react-hooks/exhaustive-deps -- markRead ref stable }, [isActive, activeSessionId, currentHasUnread]);