diff --git a/CHANGELOG.md b/CHANGELOG.md index 476d1e42e..55c28b8ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ New features, integrations, and notable improvements to Open-Inspect — newest first. +## September 2, 2026 + +**Opening a session marks it read.** A session's latest reply is read as soon as its page is +visible, without scrolling to the end of the timeline. The sidebar reflects the change on every row +at once, including sessions loaded through "Load more", and Needs attention lists only sessions with +an unread reply. + ## September 1, 2026 **Workspace audit log.** Owners, Administrators, and authorized custom roles can review paginated diff --git a/packages/control-plane/src/session/alarm/scheduler.ts b/packages/control-plane/src/session/alarm/scheduler.ts index 52f25e4e0..9fee13960 100644 --- a/packages/control-plane/src/session/alarm/scheduler.ts +++ b/packages/control-plane/src/session/alarm/scheduler.ts @@ -171,7 +171,19 @@ export function createEarliestAlarmScheduler( }; } -/** Track delivery separately so retries cannot acknowledge a replacement deadline. */ +/** + * Runs one alarm delivery against the runtime's single alarm slot. + * + * `beginDelivery` moves the pending deadline into flight and clears it, so + * while `handle` runs the store holds no future deadline. (Only a retry of + * a delivery already in flight keeps its replacement pending deadline.) + * Every step of the handler that still needs a wake-up must schedule it + * again from its own persisted state (a deferred projection, a + * stop-confirmation deadline, an execution timeout); otherwise `rearm` finds + * nothing and that deadline is lost until the next rehydration. Delivery is + * tracked separately from the pending deadline so a retry cannot acknowledge + * a replacement deadline. + */ export async function handleAlarmDelivery( deadlines: AlarmDeadlineStore, handle: () => Promise, diff --git a/packages/control-plane/src/session/message-queue.ts b/packages/control-plane/src/session/message-queue.ts index c4f3af8a0..1e8c8825c 100644 --- a/packages/control-plane/src/session/message-queue.ts +++ b/packages/control-plane/src/session/message-queue.ts @@ -649,14 +649,7 @@ export class SessionMessageQueue { completion.messageId, completion.messageCreatedAt, completion.completedAt - ) - .catch((projectionError) => { - this.log.error("terminal_message.projection_failed", { - message_id: message.id, - error: projectionError, - }); - }) - .then(() => this.messenger.broadcast({ type: "sandbox_event", event })), + ).then(() => this.messenger.broadcast({ type: "sandbox_event", event })), { name: "terminal_message.project", context: { message_id: message.id }, diff --git a/packages/web/src/components/session-list-item.test.tsx b/packages/web/src/components/session-list-item.test.tsx index 13ff16f40..0b70faa67 100644 --- a/packages/web/src/components/session-list-item.test.tsx +++ b/packages/web/src/components/session-list-item.test.tsx @@ -11,6 +11,7 @@ expect.extend(matchers); const mocks = vi.hoisted(() => ({ allowedPermissions: new Set(), + renameOptions: undefined as Record | undefined, })); vi.mock("next/link", () => ({ @@ -18,7 +19,10 @@ vi.mock("next/link", () => ({ })); vi.mock("@/hooks/use-session-rename", () => ({ - useSessionRename: () => ({ optimisticTitle: null, renameSession: vi.fn() }), + useSessionRename: (options: Record) => { + mocks.renameOptions = options; + return { optimisticTitle: null, renameSession: vi.fn() }; + }, })); vi.mock("@/hooks/use-current-user-authorization", () => ({ @@ -62,6 +66,18 @@ function renderItem(unread = false) { ); } +it("keeps a confirmed rename until the row's fetched title catches up", () => { + renderItem(); + + // Rows from loaded pages never refetch, so the rename overlay must wait for + // this row's own title rather than clear on request success. + expect(mocks.renameOptions).toMatchObject({ + sessionId: "session-1", + authoritativeTitle: "Session one", + awaitAuthoritativeTitle: true, + }); +}); + it("fails closed when sessions.lifecycle is denied", () => { renderItem(); diff --git a/packages/web/src/components/session-list-item.tsx b/packages/web/src/components/session-list-item.tsx index cace41b11..cd63bfc3d 100644 --- a/packages/web/src/components/session-list-item.tsx +++ b/packages/web/src/components/session-list-item.tsx @@ -53,9 +53,13 @@ export function SessionListItem({ session.repositories ); const prDisplay = pullRequestSummaryDisplay(session.pullRequestSummary); + // A row from a loaded page never refetches, so a confirmed rename stays + // on screen until the fetched title catches up. const { optimisticTitle, renameSession } = useSessionRename({ sessionId: session.id, currentTitle: session.title, + authoritativeTitle: session.title, + awaitAuthoritativeTitle: true, }); const displayTitle = optimisticTitle ?? session.title ?? repoInfo; // Orphan child (parent filtered out) — show a subtle badge diff --git a/packages/web/src/hooks/use-mark-session-read.test.tsx b/packages/web/src/hooks/use-mark-session-read.test.tsx index ec1815990..cfaf98ef0 100644 --- a/packages/web/src/hooks/use-mark-session-read.test.tsx +++ b/packages/web/src/hooks/use-mark-session-read.test.tsx @@ -4,16 +4,28 @@ import { act, cleanup, renderHook } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { SessionReadResult } from "@open-inspect/shared/types/sessions"; import { useMarkSessionRead } from "./use-mark-session-read"; -import { SessionReadRequestError } from "@/lib/session-read-state"; +import { + getSessionReadOverlay, + resetSessionReadOverlay, + SessionReadRequestError, +} from "@/lib/session-read-state"; const markMessageRead = vi.fn<(sessionId: string, messageId: string) => Promise>(); -const reconcileSessionReadState = vi.fn(async (_result: SessionReadResult) => {}); + +const mutate = vi.fn(async (_key: unknown) => []); +let viewerId: string | null = "viewer-a"; vi.mock("@/lib/session-read-state", async (importOriginal) => ({ ...(await importOriginal>()), markMessageRead: (sessionId: string, messageId: string) => markMessageRead(sessionId, messageId), - reconcileSessionReadState: (result: SessionReadResult) => reconcileSessionReadState(result), +})); +vi.mock("swr", async (importOriginal) => ({ + ...(await importOriginal>()), + useSWRConfig: () => ({ mutate }), +})); +vi.mock("@/lib/auth-session", () => ({ + useAuthSession: () => ({ data: viewerId ? { user: { id: viewerId } } : undefined }), })); function result( @@ -36,8 +48,11 @@ function setVisibility(value: "visible" | "hidden") { beforeEach(() => { setVisibility("visible"); + resetSessionReadOverlay(); + viewerId = "viewer-a"; markMessageRead.mockReset(); - reconcileSessionReadState.mockClear(); + mutate.mockReset(); + mutate.mockImplementation(async () => []); vi.spyOn(console, "error").mockImplementation(() => {}); }); afterEach(() => { @@ -56,7 +71,29 @@ describe("useMarkSessionRead", () => { }); expect(markMessageRead).toHaveBeenCalledExactlyOnceWith("session-1", "message-1"); - expect(reconcileSessionReadState).toHaveBeenCalledExactlyOnceWith(result("marked_read")); + expect(getSessionReadOverlay("viewer-a").get("session-1")).toEqual({ + latestMessageId: "message-1", + unread: false, + version: 1, + }); + }); + + it("does not ask again about a message this page already read", async () => { + markMessageRead.mockResolvedValue(result("marked_read")); + const first = await act(async () => + renderHook(() => useMarkSessionRead("session-1", "message-1")) + ); + first.unmount(); + + await act(async () => { + renderHook(() => useMarkSessionRead("session-1", "message-1")); + }); + expect(markMessageRead).toHaveBeenCalledTimes(1); + + await act(async () => { + renderHook(() => useMarkSessionRead("session-1", "message-2")); + }); + expect(markMessageRead).toHaveBeenCalledTimes(2); }); it("acknowledges each message once and follows the message ID", async () => { @@ -74,6 +111,54 @@ describe("useMarkSessionRead", () => { expect(markMessageRead).toHaveBeenLastCalledWith("session-1", "message-2"); }); + it("refetches the inbox once after a new read and does not resend the read if that fails", async () => { + vi.useFakeTimers(); + mutate.mockRejectedValueOnce(new Error("offline")); + markMessageRead.mockResolvedValue(result("marked_read")); + + await act(async () => { + renderHook(() => useMarkSessionRead("session-1", "message-1")); + }); + await act(async () => vi.advanceTimersByTimeAsync(60_000)); + + expect(markMessageRead).toHaveBeenCalledTimes(1); + expect(mutate).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalled(); + }); + + it("records a late response under the viewer who sent it", async () => { + let resolveFirst!: (value: SessionReadResult) => void; + markMessageRead + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }) + ) + .mockResolvedValue(result("marked_read")); + const { rerender } = await act(async () => + renderHook(() => useMarkSessionRead("session-1", "message-1")) + ); + expect(markMessageRead).toHaveBeenCalledTimes(1); + + viewerId = "viewer-b"; + await act(async () => rerender()); + expect(markMessageRead).toHaveBeenCalledTimes(2); + + await act(async () => resolveFirst(result("not_latest", true))); + + expect(getSessionReadOverlay("viewer-a").get("session-1")).toEqual({ + latestMessageId: "message-1", + unread: true, + version: 1, + }); + expect(getSessionReadOverlay("viewer-b").get("session-1")).toEqual({ + latestMessageId: "message-1", + unread: false, + version: 1, + }); + }); + it("does nothing until the session has a terminal message", async () => { await act(async () => { renderHook(() => useMarkSessionRead("session-1", null)); diff --git a/packages/web/src/hooks/use-mark-session-read.ts b/packages/web/src/hooks/use-mark-session-read.ts index 1dfc572d0..3e83a38a0 100644 --- a/packages/web/src/hooks/use-mark-session-read.ts +++ b/packages/web/src/hooks/use-mark-session-read.ts @@ -1,50 +1,61 @@ "use client"; import { useEffect } from "react"; +import { useSWRConfig } from "swr"; +import { useAuthSession } from "@/lib/auth-session"; import { - classifySessionReadAttempt, + applySessionReadResult, + isSessionMessageRead, markMessageRead, - reconcileSessionReadState, SessionReadRequestError, - type SessionReadAttemptDisposition, } from "@/lib/session-read-state"; const SESSION_READ_RETRY_MS = 2_000; const SESSION_READ_MAX_ATTEMPTS = 4; const PERMANENT_FAILURE_STATUSES = new Set([400, 401, 403, 404, 405]); -async function attemptMarkMessageRead( - sessionId: string, - messageId: string -): Promise { - try { - const result = await markMessageRead(sessionId, messageId); - await reconcileSessionReadState(result); - return classifySessionReadAttempt(result); - } catch (error) { - if (error instanceof SessionReadRequestError && PERMANENT_FAILURE_STATUSES.has(error.status)) { - return "permanent_failure"; - } - console.error("Failed to mark session message read", error); - return "retry"; - } -} - /** * Opening a session reads its latest terminal message. Each message ID is * acknowledged once while the document is visible; a hidden tab waits for * visibility. Focus is not required, since the terminal pane holds it for - * much of a working session. + * much of a working session. A message this viewer already read in this + * page is not asked about again. + * + * Only a missing projection is retried: the message exists on the client, + * so the server row will catch up. A `not_latest` result means a newer + * message is on its way to the client, which acknowledges that one instead. */ export function useMarkSessionRead(sessionId: string, messageId: string | null): void { + const { mutate } = useSWRConfig(); + const { data: authSession } = useAuthSession(); + const viewerId = authSession?.user.id ?? null; + useEffect(() => { - if (!messageId) return; + if (!messageId || !viewerId) return; + if (isSessionMessageRead(viewerId, sessionId, messageId)) return; let cancelled = false; let settled = false; let inFlight = false; let attempts = 0; let retryTimer: ReturnType | null = null; + const attemptOnce = async (): Promise<"settled" | "retry"> => { + try { + const result = await markMessageRead(sessionId, messageId); + applySessionReadResult(result, mutate, viewerId); + return result.outcome === "no_terminal_message" ? "retry" : "settled"; + } catch (error) { + if ( + error instanceof SessionReadRequestError && + PERMANENT_FAILURE_STATUSES.has(error.status) + ) { + return "settled"; + } + console.error("Failed to mark session message read", error); + return "retry"; + } + }; + const attempt = async () => { if ( cancelled || @@ -57,10 +68,10 @@ export function useMarkSessionRead(sessionId: string, messageId: string | null): } inFlight = true; attempts += 1; - const disposition = await attemptMarkMessageRead(sessionId, messageId); + const disposition = await attemptOnce(); inFlight = false; if (cancelled) return; - if (disposition !== "retry") { + if (disposition === "settled") { settled = true; return; } @@ -88,5 +99,5 @@ export function useMarkSessionRead(sessionId: string, messageId: string | null): document.removeEventListener("visibilitychange", onVisibilityChange); if (retryTimer) clearTimeout(retryTimer); }; - }, [messageId, sessionId]); + }, [messageId, mutate, sessionId, viewerId]); } diff --git a/packages/web/src/hooks/use-session-rename.ts b/packages/web/src/hooks/use-session-rename.ts index b23a6dcc0..79bac04a5 100644 --- a/packages/web/src/hooks/use-session-rename.ts +++ b/packages/web/src/hooks/use-session-rename.ts @@ -7,7 +7,6 @@ import { applyTitleUpdate, isSessionListKey, type SessionListResponse } from "@/ import { applySessionInboxTitleUpdate, isSessionInboxKey, - type SessionInboxPage, type SessionInboxSnapshot, } from "@/lib/session-inbox-api"; @@ -15,7 +14,7 @@ type SessionCacheMutator = ReturnType["mutate"]; /** * A session's title is cached in two payload families: session-list responses - * and inbox snapshots/pages. Every optimistic update, settlement, and rollback + * and inbox snapshots. Every optimistic update, settlement, and rollback * must touch both, or the sidebar inbox briefly reverts to the stale title * once the optimistic overlay clears. */ @@ -30,7 +29,7 @@ function applyTitleToSessionCaches( (current) => applyTitleUpdate(current, sessionId, title), { populateCache: true, revalidate: false } ), - mutate( + mutate( isSessionInboxKey, (current) => applySessionInboxTitleUpdate(current, sessionId, title), { populateCache: true, revalidate: false } diff --git a/packages/web/src/hooks/use-sidebar-sessions.test.tsx b/packages/web/src/hooks/use-sidebar-sessions.test.tsx index bafd47021..23c41f340 100644 --- a/packages/web/src/hooks/use-sidebar-sessions.test.tsx +++ b/packages/web/src/hooks/use-sidebar-sessions.test.tsx @@ -9,28 +9,37 @@ import type { SessionInboxPage, SessionInboxSnapshot, } from "@open-inspect/shared/types/session-inbox"; +import type { SessionReadResult } from "@open-inspect/shared/types/sessions"; import { useSidebarSessions } from "./use-sidebar-sessions"; -import { reconcileSessionReadState } from "@/lib/session-read-state"; - +import { + applySessionReadResult, + isSessionMessageRead, + resetSessionReadOverlay, +} from "@/lib/session-read-state"; + +const defaultUser = { id: "github:123", name: "Test User" }; +let authUser: typeof defaultUser | null = defaultUser; vi.mock("@/lib/auth-session", () => ({ - useAuthSession: () => ({ data: { user: { id: "github:123", name: "Test User" } } }), + useAuthSession: () => ({ data: authUser ? { user: authUser } : undefined }), })); -vi.mock("@/lib/session-read-state", async (importOriginal) => { - const actual = await importOriginal>(); - return { - ...actual, - markLatestMessageRead: async (sessionId: string) => ({ - sessionId, - outcome: "marked_read" as const, - unread: false, - latestMessageId: "msg-1", - version: 1, - }), - }; -}); +function readResult( + sessionId: string, + outcome: SessionReadResult["outcome"] = "marked_read" +): SessionReadResult { + return outcome === "no_terminal_message" + ? { sessionId, outcome, unread: false, latestMessageId: null, version: 0 } + : { sessionId, outcome, unread: false, latestMessageId: "msg-1", version: 1 }; +} +const markLatestMessageRead = vi.fn(async (sessionId: string) => readResult(sessionId)); +vi.mock("@/lib/session-read-state", async (importOriginal) => ({ + ...(await importOriginal>()), + markLatestMessageRead: (sessionId: string) => markLatestMessageRead(sessionId), +})); -function item(id: string) { +// Rows are unread by default so they qualify for attention; read state is +// what these tests change, so it is explicit where it matters. +function item(id: string): SessionInboxItem { return { rootSession: { id, @@ -38,29 +47,35 @@ function item(id: string) { repoOwner: null, repoName: null, baseBranch: null, - status: "active" as const, + status: "active", parentSessionId: null, - spawnSource: "user" as const, + spawnSource: "user", environmentId: null, createdAt: 1, updatedAt: 2, - readState: { latestMessageId: null, version: 0, unread: false as const }, + readState: { latestMessageId: "msg-1", version: 1, unread: true }, }, descendantSessions: [], }; } -function unreadItem(id: string): SessionInboxItem { +function readItem(id: string): SessionInboxItem { const base = item(id); return { ...base, rootSession: { ...base.rootSession, - readState: { latestMessageId: "msg-1", version: 1, unread: true }, + readState: { latestMessageId: "msg-1", version: 1, unread: false }, }, }; } +const noRevalidate = async () => []; +/** A read the session page recorded for the signed-in viewer. */ +function recordPageRead(result: SessionReadResult) { + applySessionReadResult(result, noRevalidate, defaultUser.id); +} + function page(ids: string[], nextCursor: string | null = null): SessionInboxPage { return { items: ids.map(item), hasMore: nextCursor !== null, nextCursor }; } @@ -115,6 +130,10 @@ afterEach(() => { setVisibility("visible"); vi.restoreAllMocks(); vi.useRealTimers(); + resetSessionReadOverlay(); + authUser = defaultUser; + markLatestMessageRead.mockReset(); + markLatestMessageRead.mockImplementation(async (sessionId: string) => readResult(sessionId)); }); describe("useSidebarSessions", () => { @@ -192,6 +211,27 @@ describe("useSidebarSessions", () => { ); }); + it("sends one request when Load more is clicked twice before it renders as loading", async () => { + let paginationRequests = 0; + const fetcher = vi.fn(async (key: string) => { + if (key.includes("category=")) { + paginationRequests += 1; + return page(["page-2"]); + } + return snapshot({ needs_attention: page(["attention"], "next") }); + }); + const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => { + result.current.sectionPagination.needsAttention.loadMore(); + result.current.sectionPagination.needsAttention.loadMore(); + }); + await waitFor(() => expect(result.current.needsAttention).toHaveLength(2)); + + expect(paginationRequests).toBe(1); + }); + it("keeps additional pages across unchanged and changed coherent head refreshes", async () => { let snapshotRequest = 0; const fetcher = vi.fn(async (key: string) => { @@ -422,34 +462,73 @@ describe("useSidebarSessions", () => { expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention", "tail-b"]); }); - it("reconciles read state on retained pages when a session is marked read", async () => { - const fetcher = vi.fn(async (key: string) => - key.includes("category=") - ? { - items: [unreadItem("tail-unread"), unreadItem("tail-other")], - hasMore: false, - nextCursor: null, - } - : snapshot({ needs_attention: page(["attention"], "next") }) - ); + it("hides a fully read hierarchy from attention and lets the refetched snapshot place it", async () => { + const refetchedSnapshot = deferred(); + let headRequests = 0; + const fetcher = vi.fn(async (key: string) => { + if (key.includes("category=")) + return { items: [item("tail-read"), item("tail-other")], hasMore: false, nextCursor: null }; + headRequests += 1; + if (headRequests === 1) return snapshot({ needs_attention: page(["attention"], "next") }); + return refetchedSnapshot.promise; + }); const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); await waitFor(() => expect(result.current.loading).toBe(false)); act(() => result.current.sectionPagination.needsAttention.loadMore()); await waitFor(() => expect(result.current.needsAttention).toHaveLength(3)); - await act(async () => result.current.handleMarkLatestMessageRead("tail-unread")); + const marked = act(async () => result.current.handleMarkLatestMessageRead("tail-read")); + await waitFor(() => + expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention", "tail-other"]) + ); + // Nothing on the client moved the session: it is absent until the server places it. + expect(result.current.inProgress.map(({ id }) => id)).toEqual(["running"]); + expect( + result.current.needsAttention.find(({ id }) => id === "tail-other")?.readState.unread + ).toBe(true); + expect(headRequests).toBe(2); - // The freshly read hierarchy leaves the retained attention page; the still - // unread one stays with its read state intact. + refetchedSnapshot.resolve( + snapshot({ + needs_attention: page(["attention"], "next"), + in_progress: { + items: [readItem("tail-read"), item("running")], + hasMore: false, + nextCursor: null, + }, + }) + ); + await marked; + await waitFor(() => + expect(result.current.inProgress.map(({ id }) => id)).toEqual(["tail-read", "running"]) + ); expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention", "tail-other"]); - const remainingTail = result.current.needsAttention.find(({ id }) => id === "tail-other"); - expect(remainingTail?.readState.unread).toBe(true); }); - it("keeps retained pages when an already-read session is acknowledged", async () => { + it("hides a read head row from attention until the server places it", async () => { + const fetcher = vi.fn(async () => snapshot({ needs_attention: page(["attention", "other"]) })); + const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => recordPageRead(readResult("attention"))); + + expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["other"]); + expect(result.current.inProgress.map(({ id }) => id)).toEqual(["running"]); + }); + + it("does not refetch when the server reports the session was already read", async () => { + markLatestMessageRead.mockImplementation(async (sessionId: string) => + readResult(sessionId, "already_read") + ); let snapshotFetches = 0; const fetcher = vi.fn(async (key: string) => { - if (key.includes("category=finished")) return page(["finished-tail-a", "finished-tail-b"]); + if (key.includes("category=finished")) { + return { + items: [readItem("finished-tail-a"), readItem("finished-tail-b")], + hasMore: false, + nextCursor: null, + }; + } if (key.includes("category=in_progress")) return page(["progress-tail"]); snapshotFetches += 1; return snapshot({ @@ -467,15 +546,7 @@ describe("useSidebarSessions", () => { // Opening a session acknowledges its terminal message even when it is // already read. Nothing changed, so nothing in the sidebar should move. - await act(() => - reconcileSessionReadState({ - sessionId: "finished-tail-b", - outcome: "already_read", - latestMessageId: "msg-1", - version: 1, - unread: false, - }) - ); + await act(async () => result.current.handleMarkLatestMessageRead("finished-tail-b")); await act(() => new Promise((resolve) => setTimeout(resolve, 20))); expect(result.current.finished.map(({ id }) => id)).toEqual([ @@ -487,34 +558,41 @@ describe("useSidebarSessions", () => { expect(snapshotFetches).toBe(fetchesBefore); }); - it("keeps retained pages for a not_latest result", async () => { - const fetcher = vi.fn(async (key: string) => - key.includes("category=") - ? page(["finished-tail"]) - : snapshot({ finished: page(["finished"], "finished-next") }) - ); + it("shows a newer unread message from a not_latest result and refetches the snapshot", async () => { + markLatestMessageRead.mockImplementation(async (sessionId: string) => ({ + sessionId, + outcome: "not_latest", + latestMessageId: "msg-9", + version: 9, + unread: true, + })); + let snapshotFetches = 0; + const fetcher = vi.fn(async (key: string) => { + if (key.includes("category=")) { + return { items: [readItem("finished-tail")], hasMore: false, nextCursor: null }; + } + snapshotFetches += 1; + return snapshot({ finished: page(["finished"], "finished-next") }); + }); const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); await waitFor(() => expect(result.current.loading).toBe(false)); act(() => result.current.sectionPagination.finished.loadMore()); await waitFor(() => expect(result.current.finished).toHaveLength(2)); + const fetchesBefore = snapshotFetches; - await act(() => - reconcileSessionReadState({ - sessionId: "finished-tail", - outcome: "not_latest", - latestMessageId: "msg-9", - version: 9, - unread: true, - }) - ); + await act(async () => result.current.handleMarkLatestMessageRead("finished-tail")); - expect(result.current.finished.map(({ id }) => id)).toEqual(["finished", "finished-tail"]); + const tail = result.current.finished.find(({ id }) => id === "finished-tail"); + expect(tail?.readState).toEqual({ latestMessageId: "msg-9", version: 9, unread: true }); + // The server owns placement: a newer unread message means a refetch, so + // the row can move to attention where the inbox query puts it. + await waitFor(() => expect(snapshotFetches).toBe(fetchesBefore + 1)); }); - it("updates a read tail row in place instead of dropping it", async () => { + it("shows a read on a loaded page row without dropping it", async () => { const fetcher = vi.fn(async (key: string) => key.includes("category=") - ? { items: [unreadItem("finished-tail")], hasMore: false, nextCursor: null } + ? { items: [item("finished-tail")], hasMore: false, nextCursor: null } : snapshot({ finished: page(["finished"], "finished-next") }) ); const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); @@ -522,98 +600,105 @@ describe("useSidebarSessions", () => { act(() => result.current.sectionPagination.finished.loadMore()); await waitFor(() => expect(result.current.finished).toHaveLength(2)); - await act(() => - reconcileSessionReadState({ - sessionId: "finished-tail", - outcome: "marked_read", - latestMessageId: "msg-1", - version: 1, - unread: false, - }) - ); + await act(async () => result.current.handleMarkLatestMessageRead("finished-tail")); const tail = result.current.finished.find(({ id }) => id === "finished-tail"); expect(tail?.readState).toEqual({ latestMessageId: "msg-1", version: 1, unread: false }); + expect(result.current.finished.map(({ id }) => id)).toEqual(["finished", "finished-tail"]); }); - it("resets only the destination chain when an attention tail leaves attention", async () => { - const fetcher = vi.fn(async (key: string) => { - if (key.includes("category=needs_attention")) { - return { items: [unreadItem("moving")], hasMore: false, nextCursor: null }; - } - if (key.includes("category=in_progress")) return page(["progress-tail"]); - if (key.includes("category=finished")) return page(["finished-tail"]); - return snapshot({ - needs_attention: page(["attention"], "attention-next"), - in_progress: page(["running"], "progress-next"), - finished: page(["finished"], "finished-next"), - }); - }); + it("does not let a read recorded earlier hide a newer fetched message", async () => { + const fetcher = vi.fn(async () => + snapshot({ + needs_attention: { + items: [ + { + ...item("attention"), + rootSession: { + ...item("attention").rootSession, + readState: { latestMessageId: "msg-2", version: 2, unread: true }, + }, + }, + ], + hasMore: false, + nextCursor: null, + }, + }) + ); const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); await waitFor(() => expect(result.current.loading).toBe(false)); - act(() => result.current.sectionPagination.needsAttention.loadMore()); - act(() => result.current.sectionPagination.inProgress.loadMore()); - act(() => result.current.sectionPagination.finished.loadMore()); - await waitFor(() => expect(result.current.needsAttention).toHaveLength(2)); - await waitFor(() => expect(result.current.inProgress).toHaveLength(2)); - await waitFor(() => expect(result.current.finished).toHaveLength(2)); - await act(() => - reconcileSessionReadState({ - sessionId: "moving", - outcome: "marked_read", - latestMessageId: "msg-1", - version: 1, - unread: false, - }) - ); + act(() => recordPageRead(readResult("attention"))); - // "moving" is active, so it heads for in_progress: that chain restarts from - // the head page. The finished chain is unaffected and keeps its tail. expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention"]); - expect(result.current.inProgress.map(({ id }) => id)).toEqual(["running"]); - expect(result.current.finished.map(({ id }) => id)).toEqual(["finished", "finished-tail"]); + expect(result.current.needsAttention[0]?.readState.unread).toBe(true); }); - it("reconciles retained pages when read state changes outside the sidebar", async () => { - const fetcher = vi.fn(async (key: string) => - key.includes("category=") - ? { items: [unreadItem("tail-unread")], hasMore: false, nextCursor: null } - : snapshot({ needs_attention: page(["attention"], "next") }) + it("keeps a recorded read once the fetched row catches up, so reopening need not ask", async () => { + let sessionRead = false; + const fetcher = vi.fn(async () => + sessionRead + ? snapshot({ + needs_attention: page([]), + in_progress: { + items: [readItem("target"), item("running")], + hasMore: false, + nextCursor: null, + }, + }) + : snapshot({ needs_attention: page(["target"]) }) ); const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); await waitFor(() => expect(result.current.loading).toBe(false)); - act(() => result.current.sectionPagination.needsAttention.loadMore()); - await waitFor(() => expect(result.current.needsAttention).toHaveLength(2)); - await act(() => - reconcileSessionReadState({ - sessionId: "tail-unread", - outcome: "marked_read", - latestMessageId: "msg-1", - version: 1, - unread: false, - }) + act(() => recordPageRead(readResult("target"))); + expect(result.current.needsAttention).toEqual([]); + + sessionRead = true; + await act(async () => result.current.refreshSnapshot()); + + await waitFor(() => + expect(result.current.inProgress.map(({ id }) => id)).toEqual(["target", "running"]) ); + expect(result.current.inProgress[0]?.readState.unread).toBe(false); + expect(isSessionMessageRead(defaultUser.id, "target", "msg-1")).toBe(true); + }); - expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention"]); + it("does not render another viewer's reads", async () => { + const fetcher = vi.fn(async () => snapshot()); + const { result, rerender } = renderHook(() => useSidebarSessions(), { + wrapper: wrapper(fetcher), + }); + await waitFor(() => expect(result.current.loading).toBe(false)); + act(() => recordPageRead(readResult("attention"))); + expect(result.current.needsAttention).toEqual([]); + + authUser = { id: "github:456", name: "Other User" }; + rerender(); + + await waitFor(() => + expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention"]) + ); + expect(result.current.needsAttention[0]?.readState.unread).toBe(true); }); - it("resets destination pagination when an attention tail changes category", async () => { + it("discards loaded pages when the head boundary moves", async () => { let sessionRead = false; const fetcher = vi.fn(async (key: string) => { - if (key.includes("category=needs_attention")) { - return { items: [unreadItem("moving")], hasMore: false, nextCursor: null }; - } if (key.includes("category=in_progress")) { return page([ key.includes("new-progress-next") ? "new-progress-tail" : "old-progress-tail", ]); } + if (key.includes("category=")) return page(["attention-tail"]); return sessionRead ? snapshot({ - needs_attention: page(["attention"]), - in_progress: page(["moving", "running"], "new-progress-next"), + needs_attention: page(["attention"], "attention-next"), + in_progress: { + items: [readItem("moving"), item("running")], + hasMore: true, + nextCursor: "new-progress-next", + }, }) : snapshot({ needs_attention: page(["attention"], "attention-next"), @@ -628,20 +713,19 @@ describe("useSidebarSessions", () => { await waitFor(() => expect(result.current.inProgress).toHaveLength(2)); sessionRead = true; - await act(() => - reconcileSessionReadState({ - sessionId: "moving", - outcome: "marked_read", - latestMessageId: "msg-2", - version: 2, - unread: false, - }) - ); + await act(async () => result.current.refreshSnapshot()); + // The in-progress head gained a row, so its old tail could hide the rows + // now below the new boundary; it is dropped. The attention chain, whose + // boundary did not move, keeps its tail. await waitFor(() => expect(result.current.inProgress.map(({ id }) => id)).toEqual(["moving", "running"]) ); - expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["attention"]); + expect(result.current.sectionPagination.inProgress.hasMore).toBe(true); + expect(result.current.needsAttention.map(({ id }) => id)).toEqual([ + "attention", + "attention-tail", + ]); act(() => result.current.sectionPagination.inProgress.loadMore()); await waitFor(() => @@ -651,12 +735,46 @@ describe("useSidebarSessions", () => { "new-progress-tail", ]) ); - expect(fetcher).toHaveBeenCalledWith( - "/api/sessions/inbox?category=in_progress&cursor=new-progress-next" + }); + + it("drops a response from an earlier chain even when the filter returns to the same identity", async () => { + const pendingPage = deferred(); + let paginationRequests = 0; + const fetcher = vi.fn(async (key: string) => { + if (key.includes("category=")) { + paginationRequests += 1; + return paginationRequests === 1 ? pendingPage.promise : page(["fresh-page-2"]); + } + return snapshot({ + needs_attention: page([key.includes("mine=true") ? "mine-first" : "all-first"], "next"), + }); + }); + const { result } = renderHook(() => useSidebarSessions(), { wrapper: wrapper(fetcher) }); + await waitFor(() => expect(result.current.loading).toBe(false)); + act(() => result.current.sectionPagination.needsAttention.loadMore()); + await waitFor(() => + expect(result.current.sectionPagination.needsAttention.loadingMore).toBe(true) + ); + + act(() => result.current.setSessionCreatorFilter("mine")); + await waitFor(() => expect(result.current.needsAttention[0]?.id).toBe("mine-first")); + act(() => result.current.setSessionCreatorFilter("all")); + await waitFor(() => expect(result.current.needsAttention[0]?.id).toBe("all-first")); + expect(result.current.sectionPagination.needsAttention.loadingMore).toBe(false); + + await act(async () => pendingPage.resolve(page(["stale-page-2"]))); + expect(result.current.needsAttention.map(({ id }) => id)).toEqual(["all-first"]); + + act(() => result.current.sectionPagination.needsAttention.loadMore()); + await waitFor(() => + expect(result.current.needsAttention.map(({ id }) => id)).toEqual([ + "all-first", + "fresh-page-2", + ]) ); }); - it("invalidates cached pagination before a remount can restore stale unread state", async () => { + it("starts pagination from the head after a remount", async () => { let sessionRead = false; let paginationRequests = 0; const fetcher = vi.fn(async (key: string) => { @@ -666,7 +784,7 @@ describe("useSidebarSessions", () => { paginationRequests += 1; return sessionRead ? { items: [], hasMore: false, nextCursor: null } - : { items: [unreadItem("tail-unread")], hasMore: false, nextCursor: null }; + : { items: [item("tail-unread")], hasMore: false, nextCursor: null }; }); const cache = new Map(); const TestWrapper = wrapper(fetcher, cache); @@ -676,19 +794,12 @@ describe("useSidebarSessions", () => { await waitFor(() => expect(first.result.current.needsAttention).toHaveLength(2)); sessionRead = true; - await act(() => - reconcileSessionReadState({ - sessionId: "tail-unread", - outcome: "marked_read", - latestMessageId: "msg-1", - version: 1, - unread: false, - }) - ); + await act(async () => first.result.current.handleMarkLatestMessageRead("tail-unread")); first.unmount(); const second = renderHook(() => useSidebarSessions(), { wrapper: TestWrapper }); await waitFor(() => expect(second.result.current.loading).toBe(false)); + expect([...cache.keys()].every((key) => typeof key === "string")).toBe(true); act(() => second.result.current.sectionPagination.needsAttention.loadMore()); await waitFor(() => expect(paginationRequests).toBe(2)); diff --git a/packages/web/src/hooks/use-sidebar-sessions.ts b/packages/web/src/hooks/use-sidebar-sessions.ts index 58c5f5a6d..83a0e695f 100644 --- a/packages/web/src/hooks/use-sidebar-sessions.ts +++ b/packages/web/src/hooks/use-sidebar-sessions.ts @@ -1,31 +1,27 @@ "use client"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from "react"; import type { MutableRefObject } from "react"; import { useAuthSession } from "@/lib/auth-session"; import useSWR, { mutate, useSWRConfig } from "swr"; import type { SessionInboxCategory, - SessionInboxItem, SessionInboxPage, SessionInboxSnapshot, SessionListItem, } from "@open-inspect/shared/types/session-inbox"; import { - applySessionInboxItemReadState, - applySessionInboxReadStateUpdate, buildSessionInboxKey, buildSessionInboxSnapshotKey, isSessionInboxItemFullyRead, isSessionInboxKey, - isSessionInboxPaginationKey, - sessionInboxDestinationCategory, } from "@/lib/session-inbox-api"; import { + applySessionReadOverlay, + applySessionReadResult, + getSessionReadOverlay, markLatestMessageRead, - reconcileSessionReadState, - subscribeSessionReadStateReconciliation, - type SessionReadStateReconciledDetail, + subscribeSessionReadOverlay, } from "@/lib/session-read-state"; const VISIBLE_INBOX_POLL_MS = 30_000; @@ -33,16 +29,40 @@ const SESSION_CREATOR_FILTER_STORAGE_KEY = "open-inspect-sidebar-session-creator export type SessionItem = SessionListItem; type SessionCreatorFilter = "all" | "mine"; -type PaginationKey = ReturnType; -interface AdditionalPagesState { - filterIdentity: string; - pages: Array<{ page: SessionInboxPage; sequence: number }>; +interface LoadedPage { + page: SessionInboxPage; + sequence: number; } -interface PaginationRequest { - filterIdentity: string; - key: PaginationKey; +/** + * Pages loaded through "Load more" have exactly one owner: this state. They + * are fetched with a plain request and never stored under a cache key, so a + * remount starts again from the head and nothing can restore a page the + * server has since changed. + * + * A chain of loaded pages continues the head page from its cursor, so it is + * only coherent with the head it was loaded after. When the head's boundary + * moves (a row entered or left the first page) the chain is discarded along + * with any response still in flight; keeping it would hide the rows between + * the new boundary and the old tail. `generation` counts those resets so a + * response for an earlier chain can never land in a later one, even one + * with the same identity. + */ +interface LoadedPagesState { + identity: string; + generation: number; + pages: LoadedPage[]; + loading: boolean; + error: unknown; +} + +function emptyPages(identity: string, generation: number): LoadedPagesState { + return { identity, generation, pages: [], loading: false, error: undefined }; +} + +function withoutRoots(page: SessionInboxPage, rootIds: Set): SessionInboxPage { + return { ...page, items: page.items.filter((item) => !rootIds.has(item.rootSession.id)) }; } function useCategoryPagination( @@ -55,128 +75,125 @@ function useCategoryPagination( nextPageSequence: MutableRefObject ) { const { fetcher } = useSWRConfig(); - const [additionalPagesState, setAdditionalPagesState] = useState({ - filterIdentity, - pages: [], - }); - const [paginationRequest, setPaginationRequest] = useState(null); + // The generation with a request in flight, so a second click before the + // loading state renders cannot send the same cursor twice. + const inFlightGeneration = useRef(null); const firstPage = snapshot?.categories[category]; - const additionalPages = - additionalPagesState.filterIdentity === filterIdentity ? additionalPagesState.pages : []; + const chainIdentity = JSON.stringify([filterIdentity, firstPage?.nextCursor ?? null]); + const [state, setState] = useState(() => emptyPages(chainIdentity, 0)); + const current = + state.identity === chainIdentity ? state : emptyPages(chainIdentity, state.generation); + const lastPage = current.pages.at(-1)?.page ?? firstPage; useEffect(() => { - setAdditionalPagesState({ filterIdentity, pages: [] }); - setPaginationRequest(null); - }, [filterIdentity]); + setState((previous) => + previous.identity === chainIdentity + ? previous + : emptyPages(chainIdentity, previous.generation + 1) + ); + }, [chainIdentity]); + // Once the head snapshot carries a root, the loaded copy is stale for good: + // the session may since have been archived or ranked past every loaded page. useEffect(() => { - setAdditionalPagesState((state) => { - if (state.filterIdentity !== filterIdentity) return state; - const pages = state.pages.map(({ page, sequence }) => ({ - sequence, - page: { - ...page, - items: page.items.filter((item) => !canonicalRootIds.has(item.rootSession.id)), - }, - })); - return { filterIdentity, pages }; + setState((previous) => { + let changed = false; + const pages = previous.pages.map(({ page, sequence }) => { + const filtered = withoutRoots(page, canonicalRootIds); + if (filtered.items.length === page.items.length) return { page, sequence }; + changed = true; + return { page: filtered, sequence }; + }); + return changed ? { ...previous, pages } : previous; }); - }, [canonicalRootIds, filterIdentity]); + }, [canonicalRootIds]); - const { - data: loadedPage, - error, - isLoading: loadingMore, - mutate: retryPage, - } = useSWR( - paginationRequest ? [paginationRequest.key, paginationRequest.filterIdentity] : null, - paginationRequest - ? () => { - if (!fetcher) throw new Error("Missing SWR fetcher"); - return fetcher(paginationRequest.key) as Promise; - } - : null, - { shouldRetryOnError: false } - ); - - useEffect(() => { - if (!loadedPage || !paginationRequest) return; - if (paginationRequest.filterIdentity === filterIdentity) { - const sequence = nextPageSequence.current++; - const page = { - ...loadedPage, - items: loadedPage.items.filter((item) => !canonicalRootIds.has(item.rootSession.id)), - }; - setAdditionalPagesState((state) => ({ - filterIdentity, - pages: [ - ...(state.filterIdentity === filterIdentity ? state.pages : []), - { page, sequence }, - ], - })); + const requestPage = useCallback(async () => { + const cursor = lastPage?.nextCursor; + const generation = current.generation; + if (!snapshot || !cursor || inFlightGeneration.current === generation) return; + inFlightGeneration.current = generation; + const key = buildSessionInboxKey({ category, cursor, mine }); + const sequence = nextPageSequence.current++; + setState((previous) => + previous.generation === generation + ? { ...previous, loading: true, error: undefined } + : previous + ); + try { + if (!fetcher) throw new Error("Missing SWR fetcher"); + const page = withoutRoots((await fetcher(key)) as SessionInboxPage, canonicalRootIds); + setState((previous) => + previous.generation === generation + ? { ...previous, loading: false, pages: [...previous.pages, { page, sequence }] } + : previous + ); + } catch (error) { + setState((previous) => + previous.generation === generation ? { ...previous, loading: false, error } : previous + ); + } finally { + if (inFlightGeneration.current === generation) inFlightGeneration.current = null; } - setPaginationRequest(null); - }, [canonicalRootIds, filterIdentity, loadedPage, nextPageSequence, paginationRequest]); + }, [ + canonicalRootIds, + category, + current.generation, + fetcher, + lastPage, + mine, + nextPageSequence, + snapshot, + ]); - const lastPage = additionalPages.at(-1)?.page ?? firstPage; - const hasMore = lastPage?.hasMore ?? false; const loadMore = useCallback(() => { - if (!snapshot || !lastPage?.nextCursor || paginationRequest) return; - setPaginationRequest({ - filterIdentity, - key: buildSessionInboxKey({ - category, - cursor: lastPage.nextCursor, - mine, - }), - }); - }, [category, filterIdentity, lastPage, mine, paginationRequest, snapshot]); - + void requestPage(); + }, [requestPage]); const retry = useCallback( - () => (error && paginationRequest ? retryPage() : refreshSnapshot()), - [error, paginationRequest, refreshSnapshot, retryPage] + () => (current.error ? requestPage() : refreshSnapshot()), + [current.error, refreshSnapshot, requestPage] ); - - // Mutations revalidate the head snapshot, but pages loaded through `Load - // more` live only in this retained state. Reconcile them in place or they - // keep rendering the pre-mutation rows. - const updateRetainedItems = useCallback( - (update: (item: SessionInboxItem) => SessionInboxItem | null) => { - setAdditionalPagesState((state) => ({ - ...state, - pages: state.pages.map(({ page, sequence }) => ({ - sequence, - page: { - ...page, - items: page.items.flatMap((item) => { - const updated = update(item); - return updated ? [updated] : []; - }), - }, - })), - })); - }, - [] - ); - const resetRetainedPages = useCallback(() => { - setAdditionalPagesState({ filterIdentity, pages: [] }); - setPaginationRequest(null); - }, [filterIdentity]); + const removeSession = useCallback((sessionId: string) => { + setState((previous) => ({ + ...previous, + pages: previous.pages.map(({ page, sequence }) => ({ + sequence, + page: { + ...page, + items: page.items.flatMap((item) => { + if (item.rootSession.id === sessionId) return []; + return [ + { + ...item, + descendantSessions: item.descendantSessions.filter( + (session) => session.id !== sessionId + ), + }, + ]; + }), + }, + })), + })); + }, []); return { firstPageItems: firstPage?.items ?? [], - additionalPages, - error, + loadedPages: current.pages, + error: current.error, isLoading: firstPage === undefined, - hasMore, - loadingMore, + hasMore: lastPage?.hasMore ?? false, + loadingMore: current.loading, loadMore, retry, - updateRetainedItems, - resetRetainedPages, + removeSession, }; } +function useSessionReadOverlay(viewerId: string | null) { + const getSnapshot = useCallback(() => getSessionReadOverlay(viewerId), [viewerId]); + return useSyncExternalStore(subscribeSessionReadOverlay, getSnapshot, getSnapshot); +} + export function useSidebarSessions() { const { data: authSession } = useAuthSession(); const { mutate: mutateCache } = useSWRConfig(); @@ -265,24 +282,17 @@ export function useSidebarSessions() { ); const categoryResults = [attention, inProgress, finished]; - const categoryItems = useMemo(() => { + // Rows as the server sent them. A root that appears in several loaded + // pages renders only in the newest of them. + const fetchedCategoryItems = useMemo(() => { const results = [ - { - firstPageItems: attention.firstPageItems, - additionalPages: attention.additionalPages, - }, - { - firstPageItems: inProgress.firstPageItems, - additionalPages: inProgress.additionalPages, - }, - { - firstPageItems: finished.firstPageItems, - additionalPages: finished.additionalPages, - }, + { firstPageItems: attention.firstPageItems, loadedPages: attention.loadedPages }, + { firstPageItems: inProgress.firstPageItems, loadedPages: inProgress.loadedPages }, + { firstPageItems: finished.firstPageItems, loadedPages: finished.loadedPages }, ]; const latestTailSequence = new Map(); for (const result of results) { - for (const { page, sequence } of result.additionalPages) { + for (const { page, sequence } of result.loadedPages) { for (const item of page.items) { const id = item.rootSession.id; if (!canonicalRootIds.has(id) && sequence > (latestTailSequence.get(id) ?? -1)) { @@ -296,7 +306,7 @@ export function useSidebarSessions() { const renderedIds = new Set(result.firstPageItems.map((item) => item.rootSession.id)); return [ ...result.firstPageItems, - ...result.additionalPages.flatMap(({ page, sequence }) => + ...result.loadedPages.flatMap(({ page, sequence }) => page.items.filter((item) => { const id = item.rootSession.id; if (renderedIds.has(id) || latestTailSequence.get(id) !== sequence) return false; @@ -307,15 +317,30 @@ export function useSidebarSessions() { ]; }); }, [ - attention.additionalPages, attention.firstPageItems, + attention.loadedPages, canonicalRootIds, - finished.additionalPages, finished.firstPageItems, - inProgress.additionalPages, + finished.loadedPages, inProgress.firstPageItems, + inProgress.loadedPages, ]); - const [attentionItems, inProgressItems, finishedItems] = categoryItems; + + const overlay = useSessionReadOverlay(userId); + + // Reads this page established are merged over the fetched rows at render. + // The server places sessions; the client only stops showing a hierarchy in + // attention once the viewer has read all of it. + const [attentionItems, inProgressItems, finishedItems] = useMemo(() => { + const [attentionRows, inProgressRows, finishedRows] = fetchedCategoryItems.map((items) => + items.map((item) => applySessionReadOverlay(item, overlay)) + ); + return [ + attentionRows.filter((item) => !isSessionInboxItemFullyRead(item)), + inProgressRows, + finishedRows, + ]; + }, [fetchedCategoryItems, overlay]); const inboxItems = useMemo( () => [...attentionItems, ...inProgressItems, ...finishedItems], @@ -337,107 +362,29 @@ export function useSidebarSessions() { return result; }, [inboxItems]); - const updateAttentionRetained = attention.updateRetainedItems; - const updateInProgressRetained = inProgress.updateRetainedItems; - const updateFinishedRetained = finished.updateRetainedItems; - const resetInProgressRetained = inProgress.resetRetainedPages; - const resetFinishedRetained = finished.resetRetainedPages; - const updateAllRetainedItems = useCallback( - (update: (item: SessionInboxItem) => SessionInboxItem | null) => { - updateAttentionRetained(update); - updateInProgressRetained(update); - updateFinishedRetained(update); - }, - [updateAttentionRetained, updateFinishedRetained, updateInProgressRetained] - ); - - // Every session open acknowledges its terminal message, read or not, so - // this runs far more often than read state actually changes. Retained pages - // are updated in place; only a hierarchy leaving attention restarts a chain. - const reconcileSidebarReadState = useCallback( - ({ sessionId, outcome, readState }: SessionReadStateReconciledDetail) => { - const applyReadState = (item: SessionInboxItem) => - applySessionInboxItemReadState(item, sessionId, readState); - updateAttentionRetained((item) => { - const updated = applyReadState(item); - return isSessionInboxItemFullyRead(updated) ? null : updated; - }); - updateInProgressRetained(applyReadState); - updateFinishedRetained(applyReadState); - - // The destination chain was paged without the arriving hierarchy, so a - // rank between its head and retained tail would never render. Restart - // that one chain from the head. - const attentionItem = attentionItems.find( - (item) => - item.rootSession.id === sessionId || - item.descendantSessions.some((session) => session.id === sessionId) - ); - if ( - attentionItem && - !readState.unread && - isSessionInboxItemFullyRead(applyReadState(attentionItem)) - ) { - if (sessionInboxDestinationCategory(attentionItem) === "in_progress") { - resetInProgressRetained(); - } else { - resetFinishedRetained(); - } - } - - return Promise.all([ - mutateCache( - isSessionInboxKey, - (current) => applySessionInboxReadStateUpdate(current, sessionId, readState), - // `already_read` confirms the cached state; any other outcome may - // carry a change the snapshot has not seen yet. - { populateCache: true, revalidate: outcome !== "already_read" } - ), - // Cached cursor pages would restore pre-acknowledgement rows on remount. - mutateCache(isSessionInboxPaginationKey, () => undefined, { - populateCache: true, - revalidate: false, - }), - ]); - }, - [ - attentionItems, - mutateCache, - resetFinishedRetained, - resetInProgressRetained, - updateAttentionRetained, - updateFinishedRetained, - updateInProgressRetained, - ] - ); - - useEffect(() => { - return subscribeSessionReadStateReconciliation(reconcileSidebarReadState); - }, [reconcileSidebarReadState]); - + const removeFromAttention = attention.removeSession; + const removeFromInProgress = inProgress.removeSession; + const removeFromFinished = finished.removeSession; const handleSessionArchived = useCallback( async (sessionId: string) => { - updateAllRetainedItems((item) => - item.rootSession.id === sessionId - ? null - : { - ...item, - descendantSessions: item.descendantSessions.filter( - (session) => session.id !== sessionId - ), - } - ); + removeFromAttention(sessionId); + removeFromInProgress(sessionId); + removeFromFinished(sessionId); void refreshInbox().catch((error) => { console.error("Failed to refresh session inbox after archive", error); }); }, - [refreshInbox, updateAllRetainedItems] + [refreshInbox, removeFromAttention, removeFromFinished, removeFromInProgress] ); - const handleMarkLatestMessageRead = useCallback(async (sessionId: string) => { - const result = await markLatestMessageRead(sessionId); - await reconcileSessionReadState(result); - }, []); + const handleMarkLatestMessageRead = useCallback( + async (sessionId: string) => { + if (!userId) return; + const result = await markLatestMessageRead(sessionId); + applySessionReadResult(result, mutateCache, userId); + }, + [mutateCache, userId] + ); return { needsAttention: attentionItems.map((item) => item.rootSession), diff --git a/packages/web/src/lib/session-inbox-api.test.ts b/packages/web/src/lib/session-inbox-api.test.ts index 6cc788e6e..eea0a89b3 100644 --- a/packages/web/src/lib/session-inbox-api.test.ts +++ b/packages/web/src/lib/session-inbox-api.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from "vitest"; import type { SessionListItem } from "@open-inspect/shared/types/session-inbox"; import { - applySessionInboxReadStateUpdate, + applySessionInboxTitleUpdate, buildSessionInboxKey, + isSessionInboxItemFullyRead, isSessionInboxKey, - isSessionInboxPaginationKey, type SessionInboxPage, type SessionInboxSnapshot, } from "./session-inbox-api"; @@ -70,121 +70,44 @@ describe("session inbox API keys", () => { ])("does not match unrelated key %s", (key) => { expect(isSessionInboxKey(key)).toBe(false); }); - - it("matches cached pagination tuple keys", () => { - expect( - isSessionInboxPaginationKey([ - "/api/sessions/inbox?category=needs_attention&cursor=next", - '["user-1",false]', - ]) - ).toBe(true); - expect(isSessionInboxPaginationKey(["/api/sessions?status=active", "filter"])).toBe(false); - }); }); -describe("applySessionInboxReadStateUpdate", () => { - const readState = { latestMessageId: "old-message", version: 1, unread: false } as const; - - it("updates a matching root session in a page without disturbing unrelated sessions", () => { - const data: SessionInboxPage = { - ...page("target", ["target-child"]), - items: [...page("target", ["target-child"]).items, ...page("unrelated").items], - }; - - const result = applySessionInboxReadStateUpdate(data, "target", readState); - - expect(result?.items[0].rootSession.readState).toEqual(readState); - expect(result?.items[0].descendantSessions[0].readState).toEqual({ - latestMessageId: "old-message", - version: 1, - unread: true, - }); - expect(result?.items[1].rootSession.readState).toEqual({ - latestMessageId: "old-message", - version: 1, - unread: true, - }); - expect(data.items[0].rootSession.readState.unread).toBe(true); - }); - - it("updates a matching descendant in a snapshot without disturbing other sessions", () => { - const data: SessionInboxSnapshot = { - categories: { - needs_attention: page("attention-root", ["target-child", "sibling-child"]), - in_progress: page("progress-root", ["progress-child"]), - finished: page("finished-root"), - }, - }; +describe("isSessionInboxItemFullyRead", () => { + const read = { latestMessageId: "old-message", version: 1, unread: false } as const; - const result = applySessionInboxReadStateUpdate(data, "target-child", readState); + it("keeps a hierarchy in attention while any session in it is unread", () => { + const unread = page("root", ["child"]).items[0]; + expect(isSessionInboxItemFullyRead(unread)).toBe(false); - expect(result?.categories.needs_attention.items[0].descendantSessions[0].readState).toEqual( - readState - ); - expect(result?.categories.needs_attention.items[0].rootSession.readState.unread).toBe(true); - expect(result?.categories.needs_attention.items[0].descendantSessions[1].readState.unread).toBe( - true - ); - expect(result?.categories.in_progress.items[0].rootSession.readState.unread).toBe(true); - expect(data.categories.needs_attention.items[0].descendantSessions[0].readState.unread).toBe( - true - ); - }); + const rootRead = { ...unread, rootSession: { ...unread.rootSession, readState: read } }; + expect(isSessionInboxItemFullyRead(rootRead)).toBe(false); - it("moves a fully read active hierarchy from attention to in progress", () => { - const data: SessionInboxSnapshot = { - categories: { - needs_attention: page("target"), - in_progress: page("progress-root"), - finished: page("finished-root"), - }, + const allRead = { + ...rootRead, + descendantSessions: rootRead.descendantSessions.map((child) => ({ + ...child, + readState: read, + })), }; - - const result = applySessionInboxReadStateUpdate(data, "target", readState); - - expect(result?.categories.needs_attention.items).toEqual([]); - expect(result?.categories.in_progress.items.map((item) => item.rootSession.id)).toEqual([ - "target", - "progress-root", - ]); + expect(isSessionInboxItemFullyRead(allRead)).toBe(true); }); +}); - it("moves a fully read finished descendant hierarchy from attention to finished", () => { - const attentionPage = page("target-root", ["target-child"]); - attentionPage.items[0].rootSession.status = "completed"; - attentionPage.items[0].rootSession.readState.unread = false; - attentionPage.items[0].descendantSessions[0].status = "completed"; +describe("applySessionInboxTitleUpdate", () => { + it("renames a session in every category without disturbing other rows", () => { const data: SessionInboxSnapshot = { categories: { - needs_attention: attentionPage, + needs_attention: page("attention-root", ["target"]), in_progress: page("progress-root"), finished: page("finished-root"), }, }; - const result = applySessionInboxReadStateUpdate(data, "target-child", readState); - - expect(result?.categories.needs_attention.items).toEqual([]); - expect(result?.categories.finished.items.map((item) => item.rootSession.id)).toEqual([ - "target-root", - "finished-root", - ]); - }); - - it("does not let an older result overwrite a newer cached terminal message", () => { - const data = page("target"); - data.items[0].rootSession.readState = { - latestMessageId: "newer-message", - version: 2, - unread: true, - }; - - const result = applySessionInboxReadStateUpdate(data, "target", readState); + const result = applySessionInboxTitleUpdate(data, "target", "Renamed"); - expect(result?.items[0].rootSession.readState).toEqual({ - latestMessageId: "newer-message", - version: 2, - unread: true, - }); + expect(result?.categories.needs_attention.items[0].descendantSessions[0].title).toBe("Renamed"); + expect(result?.categories.needs_attention.items[0].rootSession.title).toBe("attention-root"); + expect(result?.categories.in_progress).toEqual(data.categories.in_progress); + expect(applySessionInboxTitleUpdate(undefined, "target", "Renamed")).toBeUndefined(); }); }); diff --git a/packages/web/src/lib/session-inbox-api.ts b/packages/web/src/lib/session-inbox-api.ts index 490b475c8..431def5b2 100644 --- a/packages/web/src/lib/session-inbox-api.ts +++ b/packages/web/src/lib/session-inbox-api.ts @@ -5,9 +5,7 @@ import type { SessionInboxSnapshot, SessionListItem, } from "@open-inspect/shared/types/session-inbox"; -import type { SessionReadState } from "@open-inspect/shared/types/sessions"; import type { BrowserApiPath } from "./browser-api-fetch"; -import { applySessionReadStateToItem } from "./session-read-state"; const SESSION_INBOX_API_PATH = "/api/sessions/inbox"; @@ -35,10 +33,6 @@ export function isSessionInboxKey(key: unknown): key is string { ); } -export function isSessionInboxPaginationKey(key: unknown): boolean { - return Array.isArray(key) && isSessionInboxKey(key[0]); -} - function applyTitleToSession(session: SessionListItem, sessionId: string, title: string | null) { return session.id === sessionId ? { ...session, title } : session; } @@ -59,25 +53,11 @@ function applyTitleToPage( }; } -function applyReadStateToPage( - page: SessionInboxPage, - sessionId: string, - readState: SessionReadState -): SessionInboxPage { - return { - ...page, - items: page.items.map((item) => applySessionInboxItemReadState(item, sessionId, readState)), - }; -} - -function latestHierarchyUpdate(item: SessionInboxItem): number { - return Math.max( - item.rootSession.updatedAt, - ...item.descendantSessions.map(({ updatedAt }) => updatedAt) - ); -} - -/** Attention membership is unread-driven: a hierarchy stays while any session is unread. */ +/** + * Attention membership is unread-driven, as in the inbox query's category + * rule (`MAX(unread) = 1`): a hierarchy stays while any session in it is + * unread. This is the only category rule the client evaluates. + */ export function isSessionInboxItemFullyRead(item: SessionInboxItem): boolean { return ( !item.rootSession.readState.unread && @@ -85,96 +65,22 @@ export function isSessionInboxItemFullyRead(item: SessionInboxItem): boolean { ); } -/** Where a fully read hierarchy lands; mirrors the category rule in the inbox query. */ -export function sessionInboxDestinationCategory( - item: SessionInboxItem -): Exclude { - return item.rootSession.status === "active" || - item.descendantSessions.some(({ status }) => status === "active") - ? "in_progress" - : "finished"; -} - -export function applySessionInboxItemReadState( - item: SessionInboxItem, - sessionId: string, - readState: SessionReadState -): SessionInboxItem { - return { - rootSession: applySessionReadStateToItem(item.rootSession, sessionId, readState), - descendantSessions: item.descendantSessions.map((session) => - applySessionReadStateToItem(session, sessionId, readState) - ), - }; -} - -/** - * Applies a rename to a cached inbox payload. Inbox keys cache two shapes — - * the category snapshot and a single paginated page — so the transform - * dispatches on the presence of `categories`. - */ -export function applySessionInboxTitleUpdate( - data: T | undefined, +/** Applies a rename to the cached inbox snapshot. Loaded pages are React state, not cache. */ +export function applySessionInboxTitleUpdate( + data: SessionInboxSnapshot | undefined, sessionId: string, title: string | null -): T | undefined { +): SessionInboxSnapshot | undefined { if (!data) return data; - if ("categories" in data) { - return { - ...data, - categories: Object.fromEntries( - Object.entries(data.categories).map(([category, page]) => [ - category, - applyTitleToPage(page, sessionId, title), - ]) - ) as Record, - }; - } - return applyTitleToPage(data, sessionId, title) as T; -} - -export function applySessionInboxReadStateUpdate( - data: T | undefined, - sessionId: string, - readState: SessionReadState -): T | undefined { - if (!data) return data; - if ("categories" in data) { - const categories = Object.fromEntries( + return { + ...data, + categories: Object.fromEntries( Object.entries(data.categories).map(([category, page]) => [ category, - applyReadStateToPage(page, sessionId, readState), + applyTitleToPage(page, sessionId, title), ]) - ) as Record; - const attentionItem = categories.needs_attention.items.find( - (item) => - item.rootSession.id === sessionId || - item.descendantSessions.some((session) => session.id === sessionId) - ); - if (attentionItem && isSessionInboxItemFullyRead(attentionItem)) { - categories.needs_attention = { - ...categories.needs_attention, - items: categories.needs_attention.items.filter( - (item) => item.rootSession.id !== attentionItem.rootSession.id - ), - }; - const destination = sessionInboxDestinationCategory(attentionItem); - categories[destination] = { - ...categories[destination], - items: [ - attentionItem, - ...categories[destination].items.filter( - (item) => item.rootSession.id !== attentionItem.rootSession.id - ), - ].sort((a, b) => latestHierarchyUpdate(b) - latestHierarchyUpdate(a)), - }; - } - return { - ...data, - categories, - } as T; - } - return applyReadStateToPage(data, sessionId, readState) as T; + ) as Record, + }; } export type { SessionInboxItem, SessionInboxPage, SessionInboxSnapshot }; diff --git a/packages/web/src/lib/session-list.ts b/packages/web/src/lib/session-list.ts index d66f5ea62..9445d4128 100644 --- a/packages/web/src/lib/session-list.ts +++ b/packages/web/src/lib/session-list.ts @@ -42,20 +42,6 @@ const sessionListItemSchema = z.object({ }) ) .optional(), - readState: z - .union([ - z.object({ - latestMessageId: z.null(), - unread: z.literal(false), - version: z.number().default(0), - }), - z.object({ - latestMessageId: z.string(), - unread: z.boolean(), - version: z.number().default(0), - }), - ]) - .optional(), }); export type SessionListItem = z.infer; diff --git a/packages/web/src/lib/session-read-state.test.ts b/packages/web/src/lib/session-read-state.test.ts index 6e5f7806d..71b66d2e8 100644 --- a/packages/web/src/lib/session-read-state.test.ts +++ b/packages/web/src/lib/session-read-state.test.ts @@ -1,14 +1,49 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { SandboxEvent } from "@/types/session"; +import type { SessionInboxItem, SessionListItem } from "@open-inspect/shared/types/session-inbox"; +import type { SessionReadState } from "@open-inspect/shared/types/sessions"; import { - applySessionReadStateToItem, - classifySessionReadAttempt, + applySessionReadOverlay, + applySessionReadResult, findLatestTerminalMessageId, + getSessionReadOverlay, + isSessionMessageRead, readStateSupersedes, - reconcileSessionReadState, - subscribeSessionReadStateReconciliation, + resetSessionReadOverlay, + subscribeSessionReadOverlay, } from "./session-read-state"; +function session(id: string, readState: SessionReadState): SessionListItem { + return { + id, + title: id, + repoOwner: null, + repoName: null, + baseBranch: null, + status: "active", + parentSessionId: null, + spawnSource: "user", + environmentId: null, + createdAt: 1, + updatedAt: 2, + readState, + }; +} + +const unreadFirst: SessionReadState = { latestMessageId: "message-1", unread: true, version: 1 }; +const readFirst: SessionReadState = { latestMessageId: "message-1", unread: false, version: 1 }; +const unreadSecond: SessionReadState = { latestMessageId: "message-2", unread: true, version: 2 }; + +const noRevalidate = vi.fn(async (_key: unknown) => []); + +const VIEWER = "viewer-a"; + +afterEach(() => { + resetSessionReadOverlay(); + noRevalidate.mockClear(); + vi.restoreAllMocks(); +}); + describe("findLatestTerminalMessageId", () => { it("returns the last completed message", () => { const events: SandboxEvent[] = [ @@ -33,35 +68,6 @@ describe("findLatestTerminalMessageId", () => { }); }); -describe("classifySessionReadAttempt", () => { - it.each(["marked_read", "already_read", "not_latest"] as const)( - "completes after a %s result", - (outcome) => { - expect( - classifySessionReadAttempt({ - sessionId: "session-1", - outcome, - unread: false, - latestMessageId: "message-1", - version: 1, - }) - ).toBe("complete"); - } - ); - - it("retries while the terminal message projection is missing", () => { - expect( - classifySessionReadAttempt({ - sessionId: "session-1", - outcome: "no_terminal_message", - unread: false, - latestMessageId: null, - version: 0, - }) - ).toBe("retry"); - }); -}); - describe("readStateSupersedes", () => { it("orders by version and keeps read final within a version", () => { const olderUnread = { latestMessageId: "message-1", unread: true, version: 1 } as const; @@ -84,132 +90,189 @@ describe("readStateSupersedes", () => { }); }); -describe("applySessionReadStateToItem", () => { - const cached = { - id: "session-1", - readState: { latestMessageId: "message-2", unread: true, version: 2 } as const, - }; +describe("applySessionReadResult", () => { + it("records the server's decision and refetches the inbox when placement can change", async () => { + const listener = vi.fn(); + const unsubscribe = subscribeSessionReadOverlay(listener); - it("does not let a same-version older message hide a newer unread one", () => { - const newerUnread = { - id: "session-1", - readState: { latestMessageId: "message-b", unread: true, version: 5 } as const, - }; - expect( - applySessionReadStateToItem(newerUnread, "session-1", { - latestMessageId: "message-a", + applySessionReadResult( + { + sessionId: "session-1", + outcome: "already_read", unread: false, - version: 5, - }) - ).toBe(newerUnread); - }); - - it("does not let an older result overwrite a newer terminal message", () => { - expect( - applySessionReadStateToItem(cached, "session-1", { latestMessageId: "message-1", + version: 1, + }, + noRevalidate, + VIEWER + ); + expect(getSessionReadOverlay(VIEWER).get("session-1")).toEqual(readFirst); + expect(noRevalidate).not.toHaveBeenCalled(); + expect(listener).toHaveBeenCalledTimes(1); + + applySessionReadResult( + { + sessionId: "session-2", + outcome: "marked_read", unread: false, + latestMessageId: "message-1", version: 1, - }) - ).toBe(cached); - }); + }, + noRevalidate, + VIEWER + ); + expect(noRevalidate).toHaveBeenCalledTimes(1); + expect(typeof noRevalidate.mock.calls[0]?.[0]).toBe("function"); + + applySessionReadResult( + { sessionId: "session-3", outcome: "not_latest", ...unreadSecond }, + noRevalidate, + VIEWER + ); + expect(noRevalidate).toHaveBeenCalledTimes(2); - it("applies a read result for the cached version", () => { - expect( - applySessionReadStateToItem(cached, "session-1", { - latestMessageId: "message-2", + applySessionReadResult( + { + sessionId: "session-4", + outcome: "no_terminal_message", unread: false, - version: 2, - }).readState - ).toEqual({ latestMessageId: "message-2", unread: false, version: 2 }); + latestMessageId: null, + version: 0, + }, + noRevalidate, + VIEWER + ); + expect(noRevalidate).toHaveBeenCalledTimes(2); + unsubscribe(); }); - it("accepts the first terminal message when the cache had none", () => { - const empty = { - id: "session-1", - readState: { latestMessageId: null, unread: false, version: 0 } as const, - }; - expect( - applySessionReadStateToItem(empty, "session-1", { - latestMessageId: "message-1", - unread: true, - version: 1, - }).readState - ).toEqual({ latestMessageId: "message-1", unread: true, version: 1 }); + it("keeps each viewer's reads apart", () => { + applySessionReadResult( + { sessionId: "session-1", outcome: "marked_read", ...readFirst }, + noRevalidate, + "viewer-b" + ); + + expect(getSessionReadOverlay(VIEWER).size).toBe(0); + expect(getSessionReadOverlay("viewer-b").get("session-1")).toEqual(readFirst); + expect(getSessionReadOverlay(null).size).toBe(0); + expect(isSessionMessageRead(VIEWER, "session-1", "message-1")).toBe(false); + expect(isSessionMessageRead("viewer-b", "session-1", "message-1")).toBe(true); }); - it("does not restore unread state for the same terminal message", () => { - const read = { - id: "session-1", - readState: { latestMessageId: "message-1", unread: false, version: 1 } as const, - }; - expect( - applySessionReadStateToItem(read, "session-1", { - latestMessageId: "message-1", - unread: true, - version: 1, - }) - ).toBe(read); + it("settles the read even when the inbox refresh fails", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const failingRevalidate = vi.fn(async (_key: unknown) => { + throw new Error("offline"); + }); + + applySessionReadResult( + { sessionId: "session-1", outcome: "marked_read", ...readFirst }, + failingRevalidate, + VIEWER + ); + await vi.waitFor(() => expect(error).toHaveBeenCalledOnce()); + expect(getSessionReadOverlay(VIEWER).get("session-1")).toEqual(readFirst); }); - it("leaves other sessions and missing results alone", () => { - expect( - applySessionReadStateToItem(cached, "session-2", { - latestMessageId: "message-9", - unread: false, - version: 9, - }) - ).toBe(cached); - expect(applySessionReadStateToItem(cached, "session-1", undefined)).toBe(cached); + it("keeps the newest state per session and ignores repeats", async () => { + const listener = vi.fn(); + const unsubscribe = subscribeSessionReadOverlay(listener); + + applySessionReadResult( + { sessionId: "session-1", outcome: "not_latest", ...unreadSecond }, + noRevalidate, + VIEWER + ); + applySessionReadResult( + { sessionId: "session-1", outcome: "marked_read", ...readFirst }, + noRevalidate, + VIEWER + ); + expect(getSessionReadOverlay(VIEWER).get("session-1")).toEqual(unreadSecond); + + applySessionReadResult( + { sessionId: "session-1", outcome: "not_latest", ...unreadSecond }, + noRevalidate, + VIEWER + ); + expect(listener).toHaveBeenCalledTimes(1); + unsubscribe(); + }); + + it("answers whether this page already read a message", async () => { + expect(isSessionMessageRead(VIEWER, "session-1", "message-1")).toBe(false); + applySessionReadResult( + { sessionId: "session-1", outcome: "marked_read", ...readFirst }, + noRevalidate, + VIEWER + ); + expect(isSessionMessageRead(VIEWER, "session-1", "message-1")).toBe(true); + expect(isSessionMessageRead(VIEWER, "session-1", "message-2")).toBe(false); + + applySessionReadResult( + { sessionId: "session-1", outcome: "not_latest", ...unreadSecond }, + noRevalidate, + VIEWER + ); + expect(isSessionMessageRead(VIEWER, "session-1", "message-1")).toBe(false); + }); + + it("keeps a read after the fetched row catches up, so reopening need not ask", () => { + applySessionReadResult( + { sessionId: "session-1", outcome: "marked_read", ...readFirst }, + noRevalidate, + VIEWER + ); + + const merged = applySessionReadOverlay( + { rootSession: session("session-1", readFirst), descendantSessions: [] }, + getSessionReadOverlay(VIEWER) + ); + + expect(merged.rootSession.readState).toEqual(readFirst); + expect(isSessionMessageRead(VIEWER, "session-1", "message-1")).toBe(true); }); }); -describe("reconcileSessionReadState", () => { - it("tells reconcilers what the server decided", async () => { - const reconcile = vi.fn(); - const unsubscribe = subscribeSessionReadStateReconciliation(reconcile); - - await reconcileSessionReadState({ - sessionId: "session-1", - outcome: "already_read", - unread: false, - latestMessageId: "message-1", - version: 1, - }); - unsubscribe(); +describe("applySessionReadOverlay", () => { + const item: SessionInboxItem = { + rootSession: session("root", unreadFirst), + descendantSessions: [session("child", unreadFirst), session("other", unreadFirst)], + }; - expect(reconcile).toHaveBeenCalledWith({ - sessionId: "session-1", - outcome: "already_read", - readState: { unread: false, latestMessageId: "message-1", version: 1 }, - }); + it("merges a superseding entry into root and descendant rows", () => { + const overlay = new Map([ + ["root", readFirst], + ["child", unreadSecond], + ]); + + const merged = applySessionReadOverlay(item, overlay); + + expect(merged.rootSession.readState).toEqual(readFirst); + expect(merged.descendantSessions[0]?.readState).toEqual(unreadSecond); + expect(merged.descendantSessions[1]).toBe(item.descendantSessions[1]); + expect(item.rootSession.readState).toEqual(unreadFirst); }); - it("waits for registered cache reconcilers", async () => { - let finishReconciliation!: () => void; - const pendingReconciliation = new Promise((resolve) => { - finishReconciliation = resolve; - }); - const reconcile = vi.fn(() => pendingReconciliation); - const unsubscribe = subscribeSessionReadStateReconciliation(reconcile); - - const result = reconcileSessionReadState({ - sessionId: "session-1", - outcome: "marked_read", - unread: false, - latestMessageId: "message-1", - version: 1, - }); - await vi.waitFor(() => expect(reconcile).toHaveBeenCalledOnce()); - let settled = false; - void result.then(() => { - settled = true; - }); - await Promise.resolve(); - expect(settled).toBe(false); + it("does not let an older entry hide a newer fetched message", () => { + const fetched: SessionInboxItem = { ...item, rootSession: session("root", unreadSecond) }; - finishReconciliation(); - await result; - unsubscribe(); + const merged = applySessionReadOverlay(fetched, new Map([["root", readFirst]])); + + expect(merged.rootSession.readState).toEqual(unreadSecond); + }); + + it("returns the item untouched when the overlay is empty", () => { + expect(applySessionReadOverlay(item, new Map())).toBe(item); + }); + + it("returns the item untouched when no row in it has a superseding entry", () => { + const overlay = new Map([ + ["elsewhere", readFirst], + ["root", { latestMessageId: "message-0", unread: false, version: 0 }], + ]); + + expect(applySessionReadOverlay(item, overlay)).toBe(item); }); }); diff --git a/packages/web/src/lib/session-read-state.ts b/packages/web/src/lib/session-read-state.ts index 8ec860d7e..f83046566 100644 --- a/packages/web/src/lib/session-read-state.ts +++ b/packages/web/src/lib/session-read-state.ts @@ -1,5 +1,8 @@ +import type { ScopedMutator } from "swr"; import { browserApiFetch } from "./browser-api-fetch"; +import { isSessionInboxKey } from "./session-inbox-api"; import type { SandboxEvent } from "@/types/session"; +import type { SessionInboxItem } from "@open-inspect/shared/types/session-inbox"; import { sessionReadResultSchema, type SessionReadAction, @@ -7,24 +10,6 @@ import { type SessionReadState, } from "@open-inspect/shared/types/sessions"; -export type SessionReadAttemptDisposition = "complete" | "retry" | "permanent_failure"; -export interface SessionReadStateReconciledDetail { - sessionId: string; - outcome: SessionReadResult["outcome"]; - readState: SessionReadState; -} -type SessionReadStateReconciler = ( - detail: SessionReadStateReconciledDetail -) => Promise | unknown; -const readStateReconcilers = new Set(); - -export function subscribeSessionReadStateReconciliation( - reconcile: SessionReadStateReconciler -): () => void { - readStateReconcilers.add(reconcile); - return () => readStateReconcilers.delete(reconcile); -} - export class SessionReadRequestError extends Error { constructor(readonly status: number) { super(`Failed to update session read state: ${status}`); @@ -54,17 +39,6 @@ export function findLatestTerminalMessageId(events: SandboxEvent[]): string | nu return null; } -/** - * Only a missing projection is worth retrying: the message exists on the - * client, so the server row will catch up. A `not_latest` result means a newer - * message is on its way to the client, which acknowledges that one instead. - */ -export function classifySessionReadAttempt( - result: SessionReadResult -): SessionReadAttemptDisposition { - return result.outcome === "no_terminal_message" ? "retry" : "complete"; -} - export function markMessageRead(sessionId: string, messageId: string): Promise { return patchSessionReadState(sessionId, { action: "mark_message_read", @@ -104,21 +78,113 @@ export function readStateSupersedes(next: SessionReadState, current: SessionRead return !(current.unread === false && next.unread); } -export function applySessionReadStateToItem( - session: T, +/** + * Read state the viewer established in this page, keyed by session ID. + * + * Fetched inbox rows are never edited. Each row is merged with its overlay + * entry at render and the higher version wins, so a read shows on every row + * at once, including pages loaded through "Load more". Entries are written + * only from read-state responses, under the viewer who sent the request, so + * one viewer's reads never render for another. A fetched row that catches up + * with an entry simply wins at render; nothing is retired. + */ +export type SessionReadOverlay = ReadonlyMap; + +const EMPTY_OVERLAY: SessionReadOverlay = new Map(); +let overlays: ReadonlyMap = new Map(); +const overlayListeners = new Set<() => void>(); + +function replaceOverlays(next: ReadonlyMap): void { + overlays = next; + for (const listener of overlayListeners) listener(); +} + +export function getSessionReadOverlay(viewerId: string | null): SessionReadOverlay { + return (viewerId !== null && overlays.get(viewerId)) || EMPTY_OVERLAY; +} + +export function subscribeSessionReadOverlay(listener: () => void): () => void { + overlayListeners.add(listener); + return () => overlayListeners.delete(listener); +} + +/** Forgets every viewer's reads; tests start from a clean page. */ +export function resetSessionReadOverlay(): void { + if (overlays.size > 0) replaceOverlays(new Map()); +} + +/** Whether this page already read `messageId` for this viewer, so opening it again need not ask. */ +export function isSessionMessageRead( + viewerId: string, sessionId: string, - readState: SessionReadState | undefined + messageId: string +): boolean { + const entry = getSessionReadOverlay(viewerId).get(sessionId); + return entry?.latestMessageId === messageId && !entry.unread; +} + +function recordReadState(viewerId: string, sessionId: string, readState: SessionReadState): void { + const entries = getSessionReadOverlay(viewerId); + const current = entries.get(sessionId); + if (current && !readStateSupersedes(readState, current)) return; + if ( + current && + current.version === readState.version && + current.latestMessageId === readState.latestMessageId && + current.unread === readState.unread + ) { + return; + } + const next = new Map(entries); + next.set(sessionId, readState); + const nextOverlays = new Map(overlays); + nextOverlays.set(viewerId, next); + replaceOverlays(nextOverlays); +} + +/** + * Settles a read-state response for the viewer who sent the request. + * + * The overlay shows the result immediately. A result that can change where + * the server places the session (`marked_read`, or `not_latest` carrying a + * newer unread message) refetches the inbox; the client never moves a + * session between categories itself. The refetch is independent of the + * acknowledgement: a failed refresh is SWR's to retry, not a reason to send + * the read again. + */ +export function applySessionReadResult( + result: SessionReadResult, + mutate: ScopedMutator, + viewerId: string +): void { + recordReadState(viewerId, result.sessionId, readStateFromResult(result)); + if (result.outcome === "marked_read" || result.outcome === "not_latest") { + void mutate(isSessionInboxKey).catch((error: unknown) => { + console.error("Failed to refresh session inbox after read", error); + }); + } +} + +function mergeReadState( + session: T, + entries: SessionReadOverlay ): T { - if (session.id !== sessionId || !readState) return session; - if (session.readState && !readStateSupersedes(readState, session.readState)) return session; - return { ...session, readState }; -} - -export async function reconcileSessionReadState(result: SessionReadResult): Promise { - const readState = readStateFromResult(result); - await Promise.all( - [...readStateReconcilers].map((reconcile) => - reconcile({ sessionId: result.sessionId, outcome: result.outcome, readState }) - ) - ); + const entry = entries.get(session.id); + if (!entry || !readStateSupersedes(entry, session.readState)) return session; + return { ...session, readState: entry }; +} + +export function applySessionReadOverlay( + item: SessionInboxItem, + entries: SessionReadOverlay +): SessionInboxItem { + if (entries.size === 0) return item; + const rootSession = mergeReadState(item.rootSession, entries); + let changed = rootSession !== item.rootSession; + const descendantSessions = item.descendantSessions.map((session) => { + const merged = mergeReadState(session, entries); + if (merged !== session) changed = true; + return merged; + }); + return changed ? { rootSession, descendantSessions } : item; }