fix(chat): stop auto-mark-read from clearing a transiently-active session on mount (MUL-4360)

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) <noreply@anthropic.com>
This commit is contained in:
Naiyuan Qing
2026-07-13 13:41:01 +08:00
parent 98cadd2ae6
commit ffd4bf8613
2 changed files with 75 additions and 4 deletions

View File

@@ -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();
});
});

View File

@@ -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]);