From ffd4bf861360f17556042ebe3f2c3731ff1d5a84 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:41:01 +0800 Subject: [PATCH] fix(chat): stop auto-mark-read from clearing a transiently-active session on mount (MUL-4360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chat page persists `activeSessionId`, so on a bare `/chat` navigation it restores the last-open session as active for one frame before its URL→store effect (which runs AFTER useChatController's effects, since the hook is called first) clears it back to null. The auto-mark-read effect fired in that gap and marked the restored-but-never-opened session read — its unread badge vanished though the right pane still showed "select a chat" and the user never opened it. This is why the sidebar count dropped (e.g. 2 → 1) just by entering the tab. Defer the read by a tick and cancel it on cleanup: a session that is only momentarily active (restored on mount, then cleared) has its pending read cancelled when activeSessionId changes; only a session that stays active past the tick — a real select, deep link, or refresh — is marked read. A live-store re-check in the timer is a belt-and-suspenders guard. Adds the previously-missing auto-mark-read coverage: a stable-active session is read after the tick; a momentarily-active one is not. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/use-chat-controller.test.tsx | 57 ++++++++++++++++++- .../chat/components/use-chat-controller.ts | 22 ++++++- 2 files changed, 75 insertions(+), 4 deletions(-) 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]);