diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index e28dc129e4..62c2fde879 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -189,6 +189,17 @@ interface ConversationTabViewProps { * reparent (the tab moved to another group — remount, keep the connection) * apart from a real teardown (pane switch / route change — disconnect). */ groupId: string + /** Whether this view is actually on screen: the active tab, or every member + * of a tiled group. A mounted-but-hidden tab keeps its runtime session + * alive, but it must NOT auto-fetch its own detail. Restoring a workspace + * with N open tabs used to issue N concurrent `get_folder_conversation` + * calls — each one a tail window of up to `TAIL_TURNS_DEFAULT` (120) turns, + * so tens of MB across a large tab set — before the user had looked at a + * single one of them. A hidden tab now fetches on the first frame it + * becomes visible, so the workspace opens fetching only the tabs on screen + * (one per split group, or every member of a tiled one) instead of all N. + * Only that first load waits: see the fetch gate in the view. */ + isVisible: boolean } function buildOptimisticUserTurnFromDraft( @@ -255,6 +266,7 @@ const ConversationTabView = memo(function ConversationTabView({ showActiveFlow, reloadSignal, groupId, + isVisible, }: ConversationTabViewProps) { const t = useTranslations("Folder.conversation") // Composer-namespace copy for the queue row's click-to-insert outcomes @@ -492,12 +504,40 @@ const ConversationTabView = memo(function ConversationTabView({ setAgentConnectError(null) }, [agentType, conversationId]) + // Gate the auto-fetch on VISIBILITY, not on mount. Every open tab stays + // mounted (that is what keeps a background session's stream and scroll state + // alive), so an ungated hook here would fire one detail fetch per open tab + // the moment the workspace restores — the N-concurrent-requests / + // tens-of-MB first paint described on `isVisible`. A hidden tab's effect + // re-runs when `isVisible` flips true (tab switch, group selection, tiling), + // which is also the moment its detail is first needed; until then the panel + // renders an empty transcript behind `invisible`, costing nothing. The + // render that first shows it already reports `detailLoading` (the hook + // counts a fetch it is about to start as loading) — that is what keeps + // `awaitingHistoricalSessionId` below closed on that render. `isActive` is in + // the gate as well, so that holds without leaning on the tab store always + // keeping the active tab its group's selected one. + // + // Only the FIRST load waits. Once this view has held a detail it fetches the + // way every view did before the gate, on screen or not. A hidden view can + // lose its session (closing the sub-agent viewer drops the session it shares + // with an open tab), and if the tab holds a live connection, the next + // streamed batch recreates that session with live data and no detail, which + // `fetchDetail` then never fills in: the tab came back without its history. + // Refetching straight away, as before, normally starts ahead of that batch. + // Latched on a held detail rather than on having been shown, so a view + // remounted while hidden onto a session that is already loaded (a move to + // another group) keeps it loaded too. + const [heldDetail, setHeldDetail] = useState(false) const { detail, loading: detailLoading, error: detailError, acpLoadError, - } = useConversationDetail(effectiveConversationId) + } = useConversationDetail(effectiveConversationId, { + enabled: isVisible || isActive || heldDetail, + }) + if (detail != null && !heldDetail) setHeldDetail(true) // Subscribe to only the fields this panel actually reads from its runtime // session — NOT the whole session object. The live-message sink rewrites the @@ -557,6 +597,12 @@ const ConversationTabView = memo(function ConversationTabView({ // the backend falls back to session/new, orphaning the historical // context. cline doesn't support session resume, so it connects // immediately regardless. + // + // `detailLoading` must already be true on the render a tab turns active, + // since the auto-connect effect reads this gate from that very render: a + // restored tab that was hidden has a runtime session but no detail, and its + // fetch starts only in that render's effects. `useConversationDetail` + // reports the fetch it is about to start as loading for exactly this. const awaitingHistoricalSessionId = hasPersistedConversation && selectedAgent !== "cline" && detailLoading // Install status of the currently selected agent. An agent can be enabled and @@ -2906,6 +2952,7 @@ export function ConversationDetailPanel() { showActiveFlow={(isSplit || canTileG) && active} reloadSignal={reloadByTabId[tab.id] ?? 0} groupId={groupId} + isVisible={visible} /> ) return ( diff --git a/src/components/conversations/tab-first-show-auto-connect.test.tsx b/src/components/conversations/tab-first-show-auto-connect.test.tsx new file mode 100644 index 0000000000..5415aee01a --- /dev/null +++ b/src/components/conversations/tab-first-show-auto-connect.test.tsx @@ -0,0 +1,304 @@ +/** + * A tab restored behind another one stays mounted but hidden, and its detail + * fetch waits until it is shown. It is not session-less meanwhile: its mount + * effects create its runtime session. So the render that shows it — which is + * also the render that makes it the active tab — sees a session with no detail + * and no fetch in flight, and the auto-connect effect takes its gate from that + * very render. If the gate reads "not loading" there, the connect goes out with + * `sessionId: undefined`: the backend takes `session/new`, and the next prompt + * re-points the conversation at that empty session. + * + * Only that first load waits. A view that has held its detail keeps fetching + * it while hidden, as every view did before the visibility gate, so a session + * dropped under a hidden tab is loaded again before the tab's live connection + * can refill it with nothing but the turn in progress. + * + * `ConversationTabView` is too heavy to render here, so `TabViewGate` repeats + * its wiring of the pieces involved — the mount-time session claim, the fetch + * gate on `useConversationDetail`, and the persisted-conversation gate in front + * of the REAL `useConnectionLifecycle` — and the source checks at the bottom + * keep that wiring pinned to the panel. + */ +import { readFileSync } from "node:fs" +import { resolve } from "node:path" +import { act, cleanup, render } from "@testing-library/react" +import { useEffect, useState } from "react" +import { afterEach, describe, expect, it, vi } from "vitest" +import type { LiveMessage } from "@/contexts/acp-connections-context" +import { useConnectionLifecycle } from "@/hooks/use-connection-lifecycle" +import { useConversationDetail } from "@/hooks/use-conversation-detail" +import type { DbConversationDetail } from "@/lib/types" +import { + claimRuntimeSession, + resetConversationRuntimeStore, + useConversationRuntimeActions, + useConversationRuntimeStore, +} from "@/stores/conversation-runtime-store" + +vi.mock("@/lib/api", () => ({ + getFolderConversation: vi.fn(), + getFolderConversationTurns: vi.fn(), +})) + +// Stable across renders: the lifecycle hook's unmount-cleanup effect depends +// on these, and fresh identities would tear the "connection" down every render. +const stubs = vi.hoisted(() => { + const connect = vi.fn(() => Promise.resolve()) + return { + connect, + conn: { + status: null, + selectorsReady: false, + connect, + disconnect: vi.fn(() => Promise.resolve()), + sendPrompt: vi.fn(), + setMode: vi.fn(), + setConfigOption: vi.fn(), + cancel: vi.fn(), + respondPermission: vi.fn(), + modes: null, + configOptions: null, + hasCachedSelectors: false, + isViewer: false, + backgroundOutstanding: 0, + sessionId: null, + }, + acp: { setActiveKey: vi.fn(), touchActivity: vi.fn() }, + tasks: { addTask: vi.fn(), updateTask: vi.fn(), removeTask: vi.fn() }, + t: Object.assign((key: string) => key, { rich: (key: string) => key }), + } +}) +vi.mock("@/hooks/use-connection", () => ({ useConnection: () => stubs.conn })) +vi.mock("@/contexts/acp-connections-context", () => ({ + useAcpActions: () => stubs.acp, +})) +vi.mock("@/contexts/task-context", () => ({ + useTaskContext: () => stubs.tasks, +})) +vi.mock("next-intl", () => ({ useTranslations: () => stubs.t })) + +const { getFolderConversation } = await import("@/lib/api") +const mockGet = vi.mocked(getFolderConversation) + +const CID = 42 +const STORED_SESSION = "sess-stored" + +const storedDetail = (): DbConversationDetail => + ({ + summary: { id: CID, external_id: STORED_SESSION }, + turns: [], + }) as unknown as DbConversationDetail + +const liveMsg: LiveMessage = { + id: "live-1", + role: "assistant", + content: [], + startedAt: 0, +} + +function TabViewGate({ + visible, + active, +}: { + visible: boolean + active: boolean +}) { + // The panel's mount effect: claim the session and clear pendingCleanup — + // which is what materializes a hidden tab's runtime session. + const { setPendingCleanup } = useConversationRuntimeActions() + useEffect(() => { + claimRuntimeSession(CID) + setPendingCleanup(CID, false) + }, [setPendingCleanup]) + // The panel's fetch gate: the first load waits for the tab to be shown or + // made active; once a detail has been held, the view keeps fetching. + const [heldDetail, setHeldDetail] = useState(false) + const { detail, loading: detailLoading } = useConversationDetail(CID, { + enabled: visible || active || heldDetail, + }) + if (detail != null && !heldDetail) setHeldDetail(true) + const runtimeExternalId = useConversationRuntimeStore( + (s) => s.byConversationId.get(CID)?.externalId ?? null + ) + const externalId = + runtimeExternalId ?? detail?.summary.external_id ?? undefined + // A persisted, non-cline conversation: the gate is `detailLoading`. + const awaitingHistoricalSessionId = detailLoading + useConnectionLifecycle({ + contextKey: "tab-1", + agentType: "claude_code", + isActive: active && !awaitingHistoricalSessionId, + workingDir: "/repo", + sessionId: externalId, + conversationId: CID, + preparing: active && awaitingHistoricalSessionId, + }) + return null +} + +function storedSession() { + return useConversationRuntimeStore.getState().byConversationId.get(CID) +} + +function resetAll() { + cleanup() + act(() => resetConversationRuntimeStore()) + mockGet.mockReset() + stubs.connect.mockClear() +} + +describe("a hidden tab's first show", () => { + afterEach(resetAll) + + it("auto-connects only once its stored session id has arrived", async () => { + let land!: (detail: DbConversationDetail) => void + mockGet.mockImplementation( + () => + new Promise((resolveFetch) => { + land = resolveFetch + }) + ) + + const { rerender } = render() + await act(async () => {}) + expect(mockGet).not.toHaveBeenCalled() + expect(stubs.connect).not.toHaveBeenCalled() + + await act(async () => { + rerender() + }) + expect(mockGet).toHaveBeenCalledTimes(1) + expect(stubs.connect).not.toHaveBeenCalled() + + await act(async () => { + land(storedDetail()) + }) + expect(stubs.connect).toHaveBeenCalledTimes(1) + expect(stubs.connect).toHaveBeenCalledWith( + "claude_code", + "/repo", + STORED_SESSION, + CID + ) + }) + + // Today the tab store makes a tab its group's selected one in the same write + // that makes it active, so an active tab is always visible. The gate does not + // lean on that: an active tab fetches, and holds its connect, regardless. + it("holds the connect for a tab made active without being shown", async () => { + let land!: (detail: DbConversationDetail) => void + mockGet.mockImplementation( + () => + new Promise((resolveFetch) => { + land = resolveFetch + }) + ) + + const { rerender } = render() + await act(async () => {}) + await act(async () => { + rerender() + }) + expect(mockGet).toHaveBeenCalledTimes(1) + expect(stubs.connect).not.toHaveBeenCalled() + + await act(async () => { + land(storedDetail()) + }) + expect(stubs.connect).toHaveBeenCalledTimes(1) + expect(stubs.connect).toHaveBeenCalledWith( + "claude_code", + "/repo", + STORED_SESSION, + CID + ) + }) +}) + +// Closing the sub-agent viewer drops the runtime session it shares with an +// open tab. If that tab is hidden and holds a live connection, the +// connection's next streamed batch recreates the session with live data and no +// detail, and `fetchDetail` never fetches a session holding a turn in progress. +describe("a hidden tab whose session is dropped", () => { + afterEach(resetAll) + + it("loads its history again if it had loaded it before", async () => { + let landAgain!: (detail: DbConversationDetail) => void + mockGet.mockResolvedValueOnce(storedDetail()).mockImplementationOnce( + () => + new Promise((resolveFetch) => { + landAgain = resolveFetch + }) + ) + + const { rerender } = render() + await act(async () => {}) + expect(storedSession()?.detail).not.toBeNull() + + // Switched away from: hidden, and no longer the active tab. + await act(async () => { + rerender() + }) + act(() => { + useConversationRuntimeStore.getState().actions.removeConversation(CID) + }) + act(() => { + useConversationRuntimeStore + .getState() + .actions.setLiveMessage(CID, liveMsg, true) + }) + expect(mockGet).toHaveBeenCalledTimes(2) + + const again = storedDetail() + await act(async () => { + landAgain(again) + }) + await act(async () => { + rerender() + }) + expect(storedSession()?.detail).toBe(again) + }) + + it("stays lazy if it never loaded", async () => { + render() + await act(async () => {}) + act(() => { + useConversationRuntimeStore.getState().actions.removeConversation(CID) + }) + await act(async () => {}) + + expect(mockGet).not.toHaveBeenCalled() + }) +}) + +describe("TabViewGate mirrors ConversationTabView", () => { + const panel = readFileSync( + resolve( + process.cwd(), + "src/components/conversations/conversation-detail-panel.tsx" + ), + "utf8" + ) + + it("creates the session on mount and gates the first fetch on being shown", () => { + expect(panel).toContain("setPendingCleanup(effectiveConversationId, false)") + expect(panel).toContain( + "const [heldDetail, setHeldDetail] = useState(false)" + ) + expect(panel).toMatch( + /useConversationDetail\(effectiveConversationId, \{\s+enabled: isVisible \|\| isActive \|\| heldDetail,\s+\}\)/ + ) + expect(panel).toContain( + "if (detail != null && !heldDetail) setHeldDetail(true)" + ) + expect(panel).toContain("isVisible={visible}") + }) + + it("holds the auto-connect on detailLoading", () => { + expect(panel).toMatch( + /const awaitingHistoricalSessionId =\s+hasPersistedConversation && selectedAgent !== "cline" && detailLoading/ + ) + expect(panel).toContain("!awaitingHistoricalSessionId &&") + expect(panel).toContain("isActive: isActive && canAutoConnect,") + }) +}) diff --git a/src/contexts/tab-context.test.tsx b/src/contexts/tab-context.test.tsx index 1e2ff72891..0f4f254744 100644 --- a/src/contexts/tab-context.test.tsx +++ b/src/contexts/tab-context.test.tsx @@ -1798,7 +1798,9 @@ describe("TabProvider sub-session tabs", () => { latestContext?.openTab(1, 99, "codex", true) }) await act(async () => {}) - expect(getFolderConversationMock).toHaveBeenCalledWith(99) + // Only the summary is consumed, so the seed asks for the smallest window + // rather than transferring the child's full transcript. + expect(getFolderConversationMock).toHaveBeenCalledWith(99, { tailTurns: 1 }) expect( latestContext?.tabs.find((tab) => tab.conversationId === 99)?.title ).toBe("Review the auth module") diff --git a/src/hooks/use-conversation-detail.test.tsx b/src/hooks/use-conversation-detail.test.tsx index a2bb2ccd57..74ebc63e58 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -1,16 +1,34 @@ -import { act, renderHook } from "@testing-library/react" -import { afterEach, describe, expect, it } from "vitest" +import { act, cleanup, renderHook } from "@testing-library/react" +import { afterEach, describe, expect, it, vi } from "vitest" import type { LiveMessage } from "@/contexts/acp-connections-context" -import type { DbConversationDetail } from "@/lib/types" +import type { DbConversationDetail, MessageTurn } from "@/lib/types" import { + type ConversationRuntimeSession, resetConversationRuntimeStore, + TAIL_TURNS_DEFAULT, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" -import { useConversationDetail } from "./use-conversation-detail" +import { + DETAIL_RETRY_DELAYS_MS, + useConversationDetail, +} from "./use-conversation-detail" + +// The runtime store calls the transport directly; the visibility-gating tests +// below assert on the call itself, so the transport is stubbed out. +vi.mock("@/lib/api", () => ({ + getFolderConversation: vi.fn(), + getFolderConversationTurns: vi.fn(), +})) + +const { getFolderConversation } = await import("@/lib/api") +const mockGet = vi.mocked(getFolderConversation) const CID = 77 -function seedSession(detail: DbConversationDetail | null) { +function seedSession( + detail: DbConversationDetail | null, + overrides: Partial = {} +) { useConversationRuntimeStore.setState({ byConversationId: new Map([ [ @@ -41,6 +59,7 @@ function seedSession(detail: DbConversationDetail | null) { olderTurnsPrependEpoch: 0, pendingOutOfTurnContent: false, pendingCleanup: false, + ...overrides, }, ], ]), @@ -57,6 +76,14 @@ const liveMsg = (id: string): LiveMessage => ({ startedAt: 0, }) +const turn = (role: "user" | "assistant"): MessageTurn => + ({ + id: `${role}-1`, + role, + blocks: [], + timestamp: "2026-10-10T00:00:00Z", + }) as unknown as MessageTurn + // `useConversationDetail` is one of the two runtime-store subscriptions the // keep-alive conversation panel (`ConversationTabView`) makes for its own // session. The live-message sink replaces the session object on every streaming @@ -111,3 +138,253 @@ describe("useConversationDetail streaming decoupling", () => { expect(result.current.detail).toBe(nextDetail) }) }) + +// The workspace keeps every open tab's view MOUNTED (that is what preserves a +// background session's stream and scroll state), so the auto-fetch has to be +// gated on visibility rather than on mount. Restoring a workspace with N open +// tabs used to issue N concurrent detail fetches — each a tail window of up to +// TAIL_TURNS_DEFAULT turns, i.e. tens of MB across a large tab set — before the +// user had looked at a single one of them. A hidden view must therefore hold +// its fetch, and fire it on the render that flips it visible. +describe("useConversationDetail visibility gating", () => { + afterEach(() => { + // Unmount BEFORE the reset: a still-mounted enabled hook answers the reset + // by fetching again, and that in-flight session would leak into the next + // test (RTL's own cleanup runs after this hook). + cleanup() + act(() => resetConversationRuntimeStore()) + mockGet.mockReset() + }) + + it("holds the fetch while the view is off screen", () => { + const { result } = renderHook(() => + useConversationDetail(CID, { enabled: false }) + ) + + expect(mockGet).not.toHaveBeenCalled() + expect(result.current.detail).toBeNull() + }) + + it("fetches the default tail window once the view becomes visible", () => { + // Never resolves on purpose: this test is about WHEN the request is + // issued, and a pending promise keeps the resolution-driven store write + // (which would need its own act scope) out of the picture. + mockGet.mockReturnValue(new Promise(() => {})) + + const { rerender } = renderHook( + ({ visible }: { visible: boolean }) => + useConversationDetail(CID, { enabled: visible }), + { initialProps: { visible: false } } + ) + expect(mockGet).not.toHaveBeenCalled() + + act(() => { + rerender({ visible: true }) + }) + + expect(mockGet).toHaveBeenCalledTimes(1) + expect(mockGet).toHaveBeenCalledWith(CID, { tailTurns: TAIL_TURNS_DEFAULT }) + }) + + // A hidden tab is not session-less: the panel's mount effects (the runtime + // claim + `setPendingCleanup`) materialize its runtime session while the + // fetch waits. The render that shows it therefore reads a session with no + // detail and nothing in flight — and that is the render the panel's + // auto-connect gate is taken from. Reporting it as settled let the connect + // out with `sessionId: undefined`; it has to read as loading already. + it("reports loading on the very render that first shows the view", () => { + mockGet.mockReturnValue(new Promise(() => {})) + act(() => { + useConversationRuntimeStore + .getState() + .actions.setPendingCleanup(CID, false) + }) + const seen: Array<{ visible: boolean; loading: boolean; detail: boolean }> = + [] + const { rerender } = renderHook( + ({ visible }: { visible: boolean }) => { + const view = useConversationDetail(CID, { enabled: visible }) + seen.push({ + visible, + loading: view.loading, + detail: view.detail != null, + }) + return view + }, + { initialProps: { visible: false } } + ) + expect(mockGet).not.toHaveBeenCalled() + + act(() => { + rerender({ visible: true }) + }) + + const shown = seen.filter((render) => render.visible) + expect(shown.length).toBeGreaterThan(0) + expect(shown).toEqual( + shown.map(() => ({ visible: true, loading: true, detail: false })) + ) + expect(mockGet).toHaveBeenCalledTimes(1) + }) + + // The pending-fetch half of `loading` must follow `fetchDetail`'s own skip + // rule exactly: a session an ongoing turn holds is never fetched, so + // reporting it as loading would hold the panel's connect gate shut forever. + it.each<[string, Partial]>([ + ["a live stream", { liveMessage: liveMsg("m1") }], + ["an optimistic prompt", { optimisticTurns: [turn("user")] }], + ["promoted local turns", { localTurns: [turn("assistant")] }], + ])( + "does not report a fetch it will not make as loading (%s)", + (_holder, holds) => { + act(() => seedSession(null, holds)) + + const { result } = renderHook(() => useConversationDetail(CID)) + + expect(result.current.loading).toBe(false) + expect(result.current.detail).toBeNull() + expect(mockGet).not.toHaveBeenCalled() + } + ) +}) + +// A failed fetch leaves no detail and nothing in flight — exactly what the +// auto-fetch keys on — so it used to be re-requested on the very next render, +// as fast as the transport could fail, with the error flickering in and out. +describe("useConversationDetail after a failed fetch", () => { + // Rejects every call up to a cap, then never settles: a regression back to + // the hot loop then fails the call-count assertions instead of spinning the + // worker out of memory. + function failEveryFetch(message: string) { + let calls = 0 + mockGet.mockImplementation(() => { + calls += 1 + return calls <= 50 + ? Promise.reject(new Error(message)) + : new Promise(() => {}) + }) + } + + afterEach(() => { + cleanup() + vi.useRealTimers() + act(() => resetConversationRuntimeStore()) + mockGet.mockReset() + }) + + it("leaves the failure on screen instead of re-requesting it at once", async () => { + failEveryFetch("unreadable transcript") + + const { result } = renderHook(() => useConversationDetail(CID)) + for (let round = 0; round < 5; round++) { + await act(async () => {}) + } + + expect(mockGet).toHaveBeenCalledTimes(1) + expect(result.current.error).toBe("unreadable transcript") + expect(result.current.loading).toBe(false) + }) + + it("retries on a growing delay, then keeps retrying at the last one", async () => { + vi.useFakeTimers() + failEveryFetch("server down") + + renderHook(() => useConversationDetail(CID)) + await act(async () => {}) + expect(mockGet).toHaveBeenCalledTimes(1) + + const last = DETAIL_RETRY_DELAYS_MS[DETAIL_RETRY_DELAYS_MS.length - 1] + const schedule = [...DETAIL_RETRY_DELAYS_MS, last, last] + for (const [step, delay] of schedule.entries()) { + await act(async () => { + vi.advanceTimersByTime(delay - 1) + }) + expect(mockGet).toHaveBeenCalledTimes(step + 1) + await act(async () => { + vi.advanceTimersByTime(1) + }) + expect(mockGet).toHaveBeenCalledTimes(step + 2) + } + }) + + it("starts the schedule over once a detail has loaded", async () => { + vi.useFakeTimers() + mockGet + .mockRejectedValueOnce(new Error("blip")) + .mockRejectedValueOnce(new Error("blip")) + .mockResolvedValueOnce(makeDetail()) + .mockRejectedValueOnce(new Error("blip again")) + .mockReturnValue(new Promise(() => {})) + + renderHook(() => useConversationDetail(CID)) + await act(async () => {}) + await act(async () => { + vi.advanceTimersByTime(DETAIL_RETRY_DELAYS_MS[0]) + }) + await act(async () => { + vi.advanceTimersByTime(DETAIL_RETRY_DELAYS_MS[1]) + }) + expect(mockGet).toHaveBeenCalledTimes(3) + + // The session goes away under the mounted view (released and recreated), + // so the view fetches afresh — and that fetch fails again. + act(() => { + useConversationRuntimeStore.getState().actions.removeConversation(CID) + }) + await act(async () => {}) + expect(mockGet).toHaveBeenCalledTimes(4) + + await act(async () => { + vi.advanceTimersByTime(DETAIL_RETRY_DELAYS_MS[0]) + }) + expect(mockGet).toHaveBeenCalledTimes(5) + }) + + it("lands the detail when a retry succeeds", async () => { + vi.useFakeTimers() + const detail = makeDetail() + mockGet + .mockRejectedValueOnce(new Error("blip")) + .mockResolvedValueOnce(detail) + + const { result } = renderHook(() => useConversationDetail(CID)) + await act(async () => {}) + expect(result.current.error).toBe("blip") + + await act(async () => { + vi.advanceTimersByTime(DETAIL_RETRY_DELAYS_MS[0]) + }) + + expect(mockGet).toHaveBeenCalledTimes(2) + expect(result.current.detail).toBe(detail) + expect(result.current.error).toBeNull() + expect(result.current.loading).toBe(false) + }) + + it("holds a pending retry while the view is hidden", async () => { + vi.useFakeTimers() + failEveryFetch("server down") + + const { rerender } = renderHook( + ({ visible }: { visible: boolean }) => + useConversationDetail(CID, { enabled: visible }), + { initialProps: { visible: true } } + ) + await act(async () => {}) + act(() => { + rerender({ visible: false }) + }) + await act(async () => { + vi.advanceTimersByTime(60_000) + }) + expect(mockGet).toHaveBeenCalledTimes(1) + + act(() => { + rerender({ visible: true }) + }) + await act(async () => { + vi.advanceTimersByTime(DETAIL_RETRY_DELAYS_MS[0]) + }) + expect(mockGet).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/hooks/use-conversation-detail.ts b/src/hooks/use-conversation-detail.ts index 40245329e4..5017e0a33d 100644 --- a/src/hooks/use-conversation-detail.ts +++ b/src/hooks/use-conversation-detail.ts @@ -1,8 +1,9 @@ "use client" -import { useEffect } from "react" +import { useEffect, useRef } from "react" import { useShallow } from "zustand/react/shallow" import { + sessionHoldsActiveTurns, useConversationRuntimeActions, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" @@ -12,6 +13,22 @@ function isVirtualConversationId(conversationId: number): boolean { return !Number.isFinite(conversationId) || conversationId <= 0 } +/** + * Delays before each automatic retry of a failed detail fetch; the last one + * repeats for as long as the failure lasts. A failure leaves no detail and + * nothing in flight, which is exactly what the auto-fetch keys on, so it used + * to re-request on the very next render, as fast as the transport could fail: + * a transcript that will not parse was re-parsed in a hot loop, a server that + * was down was hammered until it came back, and the error flickered in and out + * the whole time. Spaced retries keep what that loop did right — a view picks + * its conversation back up once the server is reachable again, here within + * half a minute — and the error stays readable, next to its Reload action, in + * between. + */ +export const DETAIL_RETRY_DELAYS_MS: readonly number[] = [ + 1_000, 2_000, 4_000, 8_000, 16_000, 30_000, +] + export function useConversationDetail( conversationId: number, options?: { @@ -22,11 +39,32 @@ export function useConversationDetail( * the child's persisted detail while it is mid-stream (the parser surfaces * the in-progress turn as a normal turn, which would then duplicate the * live stream). + * + * Also pass `false` for a mounted-but-OFF-SCREEN view. The workspace keeps + * every open tab mounted (that is what preserves a background session's + * stream and scroll state), so an ungated hook fires one detail fetch per + * open tab as soon as the tab set is restored — N concurrent + * `get_folder_conversation` calls carrying tens of MB for a large tab set, + * before the user has looked at any of them. Flipping `enabled` to `true` + * (which is what a tab switch / group selection / tiling does) re-runs the + * effect and fetches then. */ enabled?: boolean } ): { detail: DbConversationDetail | null + /** + * True while the detail is being fetched — and ALSO on the render that is + * about to start that fetch. The fetch is dispatched from an effect, i.e. + * only after the render that decided it has committed, so without the second + * half that render reads as "settled, nothing persisted": no detail, not + * loading. A view kept mounted while hidden already has a runtime session + * (its mount effects create one), so the render that first shows it is + * exactly such a render — and the conversation panel's auto-connect gate, + * which waits on `loading` for the stored session id, would let the connect + * through with `sessionId: undefined` (backend `session/new`, history + * orphaned on the next prompt). + */ loading: boolean error: string | null acpLoadError: string | null @@ -40,32 +78,74 @@ export function useConversationDetail( // mid-stream, so `useShallow` keeps the slice reference-stable across batches // and consumers re-render only on a real detail transition. (`hasSession` // preserves the "session exists yet?" signal the loading state depends on.) - const { detail, detailLoading, detailError, acpLoadError, hasSession } = - useConversationRuntimeStore( - useShallow((s) => { - const session = s.byConversationId.get(conversationId) - return { - detail: session?.detail ?? null, - detailLoading: session?.detailLoading ?? false, - detailError: session?.detailError ?? null, - acpLoadError: session?.acpLoadError ?? null, - hasSession: session != null, - } - }) - ) + const { + detail, + detailLoading, + detailError, + acpLoadError, + hasSession, + needsFetch, + retryDue, + } = useConversationRuntimeStore( + useShallow((s) => { + const session = s.byConversationId.get(conversationId) + // `fetchDetail`'s own admission rule: nothing loaded, nothing in flight, + // no ongoing turn holding the session. Exposed only as booleans: a + // streaming batch can't flip them — once a stream is under way (or a + // detail exists) they are already false — so the slice stays stable. + const admissible = + session == null || + (session.detail == null && + !session.detailLoading && + !sessionHoldsActiveTurns(session)) + return { + detail: session?.detail ?? null, + detailLoading: session?.detailLoading ?? false, + detailError: session?.detailError ?? null, + acpLoadError: session?.acpLoadError ?? null, + hasSession: session != null, + // Fetch right away — unless the last fetch failed, which waits for + // the retry schedule instead. + needsFetch: admissible && session?.detailError == null, + retryDue: admissible && session?.detailError != null, + } + }) + ) const { fetchDetail } = useConversationRuntimeActions() const isVirtual = isVirtualConversationId(conversationId) + const fetchPending = enabled && !isVirtual && needsFetch + const retryPending = enabled && !isVirtual && retryDue useEffect(() => { - if (!enabled) return - if (isVirtual) return - if (detail || detailLoading) return + if (!fetchPending) return fetchDetail(conversationId) - }, [enabled, conversationId, isVirtual, detail, detailLoading, fetchDetail]) + }, [fetchPending, conversationId, fetchDetail]) + + // Automatic retries spent on the current run of failures. A loaded detail or + // another conversation starts a new run. A hidden view's pending retry is + // dropped and rescheduled, at the same step, when the view is shown again. + const retriesUsedRef = useRef(0) + useEffect(() => { + retriesUsedRef.current = 0 + }, [conversationId]) + useEffect(() => { + if (detail) retriesUsedRef.current = 0 + }, [detail]) + useEffect(() => { + if (!retryPending) return + const used = retriesUsedRef.current + const delay = + DETAIL_RETRY_DELAYS_MS[Math.min(used, DETAIL_RETRY_DELAYS_MS.length - 1)] + const timer = setTimeout(() => { + retriesUsedRef.current = used + 1 + fetchDetail(conversationId) + }, delay) + return () => clearTimeout(timer) + }, [retryPending, conversationId, fetchDetail]) return { detail, - loading: hasSession ? detailLoading : !isVirtual, + loading: hasSession ? detailLoading || fetchPending : !isVirtual, error: detailError, acpLoadError, } diff --git a/src/stores/conversation-runtime-store.ts b/src/stores/conversation-runtime-store.ts index 542217250c..af91516d64 100644 --- a/src/stores/conversation-runtime-store.ts +++ b/src/stores/conversation-runtime-store.ts @@ -2902,6 +2902,11 @@ function isLatestGeneration( // trailing USER turn (Claude/Codex append the assistant reply to the JSONL only // on completion, so a trailing user turn means the reply is still mid-flush). const VIEWER_DETAIL_SYNC_DELAYS_MS = [0, 300, 700, 1500, 2500] as const +// A poll that finds a detail load the view started still in flight waits for +// it in steps of this length, up to the cap, before it reads (see +// `syncViewerDetail`). +const VIEWER_DETAIL_SYNC_LOAD_WAIT_STEP_MS = 300 +const VIEWER_DETAIL_SYNC_LOAD_WAIT_CAP_MS = 30_000 // ─── Post-turn metadata reparse ────────────────────────────────────────── // Backoff for `syncTurnMetadata`, which re-reads the agent's transcript after @@ -2992,6 +2997,23 @@ function isPureViewerSession(session: ConversationRuntimeSession): boolean { ) } +/** + * Whether a session already holds turns of an ongoing conversation (an + * optimistic prompt, a live stream, or promoted local turns). `fetchDetail` + * skips such a session, and `useConversationDetail` asks the same question to + * tell whether its auto-fetch is about to run — one predicate, so the hook can + * never report a fetch as pending that `fetchDetail` would then decline. + */ +export function sessionHoldsActiveTurns( + session: ConversationRuntimeSession +): boolean { + return ( + session.optimisticTurns.length > 0 || + session.liveMessage !== null || + session.localTurns.length > 0 + ) +} + /** * Build the render timeline for a conversation from its runtime session, * memoized per session object via `timelineCache`. Verbatim port of the former @@ -3743,14 +3765,7 @@ export const useConversationRuntimeStore = create()(( if (session?.detail || session?.detailLoading) return // Skip fetch if session has active data (ongoing conversation) - if ( - session && - (session.optimisticTurns.length > 0 || - session.liveMessage !== null || - session.localTurns.length > 0) - ) { - return - } + if (session && sessionHoldsActiveTurns(session)) return const generation = bumpFetchGeneration(conversationId) dispatch({ type: "FETCH_DETAIL_START", conversationId }) @@ -3810,9 +3825,16 @@ export const useConversationRuntimeStore = create()(( * Load one page of older history above the current window and prepend it. * No-op unless the detail is windowed with `turns_offset > 0`, and single- * flight per session. Participates in the SAME fetch-generation total order - * as every other detail fetch: issuing a page invalidates any in-flight - * window refresh (whose response predates the page and would clobber it), + * as every other detail fetch: issuing a page invalidates an in-flight + * viewer-sync read (whose response predates the page and would clobber it), * and any fetch issued after the page invalidates the page. + * + * Never issued while a detail load is in flight (`detailLoading`: a reload, + * an overlay fold). Only that load's own result clears the flag, so a page + * invalidating it left the session loading for good: auto-connect held shut, + * overlay folds stopped. The load lands its window instead; the near-top + * trigger fires again the next time the list is scrolled up into the top, + * and the loader row pages on click. */ const loadOlderTurns = (conversationId: number): void => { const session = get().byConversationId.get(conversationId) @@ -3820,6 +3842,7 @@ export const useConversationRuntimeStore = create()(( if (!session || !detail || !isWindowedDetail(detail)) return const beforeIndex = detail.turns_offset if (beforeIndex <= 0 || session.loadingOlderTurns) return + if (session.detailLoading) return const expectedSeamHash = detail.prefix_hash const fetchId = session.dbConversationId ?? conversationId const generation = bumpFetchGeneration(conversationId) @@ -3860,7 +3883,8 @@ export const useConversationRuntimeStore = create()(( // rather than refetch once. No-op (returns immediately) unless the session is // open AND a pure viewer, so the owner's in-flight/just-completed reply is // never touched. Never sets `detailLoading` — a passive background sync must - // not flash a spinner over the content the viewer is already reading. + // not flash a spinner over the content the viewer is already reading — and + // never supersedes a load that did set it: it waits for that load instead. const syncViewerDetail = (nudgedConversationId: number): void => { // The nudge carries a positive DB id; map it to the runtime session key, // which may be a virtual negative id for a draft-originated tab (issue: the @@ -3887,6 +3911,7 @@ export const useConversationRuntimeStore = create()(( } viewerDetailSyncCancels.set(conversationId, cancel) + let loadWaitedMs = 0 const attempt = (n: number): void => { if (cancelled) return const cur = get().byConversationId.get(conversationId) @@ -3897,6 +3922,25 @@ export const useConversationRuntimeStore = create()(( cancel() return } + // A detail load the view started (its first fetch, a reload) is in + // flight. Reading now would bump the fetch generation and drop that + // load's result as stale, and only that result clears `detailLoading`: + // a poll that then failed or stopped left the view loading for good. The + // load is a fresh read in its own right, so wait for it to land and poll + // on from there, without spending an attempt. Past the cap, give up + // rather than tick forever behind a load that never settles. + if (cur.detailLoading) { + if (loadWaitedMs >= VIEWER_DETAIL_SYNC_LOAD_WAIT_CAP_MS) { + cancel() + return + } + loadWaitedMs += VIEWER_DETAIL_SYNC_LOAD_WAIT_STEP_MS + timer = setTimeout( + () => attempt(n), + VIEWER_DETAIL_SYNC_LOAD_WAIT_STEP_MS + ) + return + } // Read the DB fetch id fresh each tick: a just-bound draft resolves its // `dbConversationId` asynchronously, and the runtime key alone is not // always fetchable (a virtual negative id). Falls back to the key. diff --git a/src/stores/conversation-window-loading.test.ts b/src/stores/conversation-window-loading.test.ts index bf179267e3..3899432112 100644 --- a/src/stores/conversation-window-loading.test.ts +++ b/src/stores/conversation-window-loading.test.ts @@ -313,6 +313,30 @@ describe("loadOlderTurns", () => { expect(mockGetTurns).not.toHaveBeenCalled() }) + it("waits out a detail load in flight instead of discarding it", async () => { + // A refetch (a reload, an overlay fold) sets `detailLoading`, and only its + // own result clears it. A page issued under it bumped the fetch generation, + // so that result was dropped as stale and the session stayed loading for + // good: auto-connect held shut, overlay folds stopped. + seed({ detail: windowedDetail(4) }) + let land: (d: DbConversationDetail) => void = () => {} + mockGet.mockImplementation( + () => + new Promise((r) => { + land = r + }) + ) + mockGetTurns.mockResolvedValue(page(2, 4)) + actions().refetchDetail(CID) + actions().loadOlderTurns(CID) + expect(mockGetTurns).not.toHaveBeenCalled() + + land(windowedDetail(4)) + await flush() + expect(session()?.detailLoading).toBe(false) + expect(session()?.loadingOlderTurns).toBe(false) + }) + it("rejects a page whose seam proof mismatches and resets via refetch", async () => { seed({ detail: windowedDetail(4) }) mockGetTurns.mockResolvedValue( diff --git a/src/stores/tab-store-child-summary-window.test.ts b/src/stores/tab-store-child-summary-window.test.ts new file mode 100644 index 0000000000..440acffb90 --- /dev/null +++ b/src/stores/tab-store-child-summary-window.test.ts @@ -0,0 +1,95 @@ +/** + * `reconcileChildSummaries` seeds a child tab's label and status from + * `detail.summary` — nothing else in the response is read. It must therefore + * ask for the SMALLEST window the backend accepts instead of the legacy full + * detail: without a window the response carries every turn of the child + * session, so a long child costs megabytes of transcript transferred and + * re-parsed just to read a title. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +import type { DbConversationDetail, DbConversationSummary } from "@/lib/types" +import { + resetAppWorkspaceStore, + useAppWorkspaceStore, +} from "./app-workspace-store" +import { resetTabStore, useTabStore } from "./tab-store" + +vi.mock("@/lib/api", () => ({ + listOpenedTabs: vi.fn(), + saveOpenedTabs: vi.fn(), + getFolderConversation: vi.fn(), +})) + +vi.mock("@/lib/platform", () => ({ + subscribe: vi.fn(), + onTransportReconnect: vi.fn(), +})) + +const { getFolderConversation } = await import("@/lib/api") +const mockGet = vi.mocked(getFolderConversation) + +const CHILD_ID = 7 + +const childSummary = { + id: CHILD_ID, + folder_id: 1, + title: "child session", + status: "completed", +} as unknown as DbConversationSummary + +const summaryOnlyDetail = { + summary: childSummary, + turns: [], +} as unknown as DbConversationDetail + +beforeEach(() => { + resetTabStore() + resetAppWorkspaceStore() + mockGet.mockReset() +}) + +afterEach(() => { + resetTabStore() + resetAppWorkspaceStore() +}) + +describe("child-tab summary seeding", () => { + it("requests the smallest window instead of the full transcript", async () => { + // A child tab is one that carries a conversationId but is absent from the + // workspace's conversation list — that absence is what marks it a child. + useTabStore.setState({ + rawTabs: [ + { + id: "child-1", + kind: "conversation", + folderId: 1, + conversationId: CHILD_ID, + agentType: "claude_code", + title: "loading", + isPinned: false, + }, + ], + activeTabId: "child-1", + }) + useAppWorkspaceStore.setState({ + conversations: [], + conversationsLoading: false, + }) + mockGet.mockResolvedValue(summaryOnlyDetail) + + useTabStore.getState().reconcileChildSummaries() + + await vi.waitFor(() => expect(mockGet).toHaveBeenCalledTimes(1)) + expect(mockGet).toHaveBeenCalledWith(CHILD_ID, { tailTurns: 1 }) + // …and the windowed response still seeds the tab from its summary. + await vi.waitFor(() => + expect(useTabStore.getState().childSummaries.get(CHILD_ID)).toBe( + childSummary + ) + ) + expect( + useTabStore.getState().tabs.find((tab) => tab.id === "child-1")?.title + ).toBe("child session") + }) +}) diff --git a/src/stores/tab-store.ts b/src/stores/tab-store.ts index d6f6a691eb..ba806fe86b 100644 --- a/src/stores/tab-store.ts +++ b/src/stores/tab-store.ts @@ -2377,7 +2377,15 @@ export const useTabStore = create()((set, get) => ({ if (childSummaryInFlight.has(id)) continue childSummaryInFlight.add(id) const epoch = seedEpoch - void getFolderConversation(id) + // Only the SUMMARY is used below, so ask for the smallest window the + // backend accepts instead of the legacy full detail. Without a window the + // response carries every turn of the child session — for a long child + // that is megabytes of transcript transferred and re-parsed purely to + // read `detail.summary`. `tailTurns: 1` is the smallest window the + // backend accepts (it still widens back to the last user turn). The + // summary describes the whole transcript whatever the window, and it is + // all the tab label and status read. + void getFolderConversation(id, { tailTurns: 1 }) .then((detail) => { if (seedEpoch !== epoch) return const buffered = childSeedBuffer.get(id) diff --git a/src/stores/viewer-detail-sync.test.ts b/src/stores/viewer-detail-sync.test.ts index 7a36cabcf2..fa18b8d9e8 100644 --- a/src/stores/viewer-detail-sync.test.ts +++ b/src/stores/viewer-detail-sync.test.ts @@ -482,6 +482,88 @@ describe("syncViewerDetail — pure viewer refetch", () => { }) }) +// A view's own detail load (a tab's first fetch, a reload) sets +// `detailLoading`, and only its own result clears it. A poll read issued under +// it bumped the fetch generation, so that result was dropped as stale: if the +// poll then never committed (every read failing), the view was left loading +// for good. The overlap is common: the load's own read can backfill the title, +// and the upsert that broadcasts is a nudge. +describe("syncViewerDetail — a detail load in flight", () => { + it("lets the load land, then polls on from there", async () => { + vi.useFakeTimers() + seed({}) + let landLoad: (d: DbConversationDetail) => void = () => {} + mockGet + .mockImplementationOnce( + () => + new Promise((r) => { + landLoad = r + }) + ) + .mockResolvedValue( + detail([userTurn("u", "hi"), assistantTurn("a", "Hi! …")], 42) + ) + useConversationRuntimeStore.getState().actions.fetchDetail(CID) + + sync() + await vi.advanceTimersByTimeAsync(0) + // The poll holds its read while the load is in flight… + expect(mockGet).toHaveBeenCalledTimes(1) + + // …the load lands, from before the reply reached disk… + landLoad(detail([userTurn("u", "hi")], 10)) + await vi.advanceTimersByTimeAsync(0) + expect(session()?.detailLoading).toBe(false) + expect(session()?.detail?.transcript_watermark).toBe(10) + + // …and the poll goes on to pick the reply up. + await vi.advanceTimersByTimeAsync(300) + expect(mockGet).toHaveBeenCalledTimes(2) + expect((session()?.detail?.turns ?? []).map((t) => t.role)).toEqual([ + "user", + "assistant", + ]) + }) + + it("never leaves the view loading when every poll read fails", async () => { + vi.useFakeTimers() + seed({}) + let landLoad: (d: DbConversationDetail) => void = () => {} + mockGet + .mockImplementationOnce( + () => + new Promise((r) => { + landLoad = r + }) + ) + .mockRejectedValue(new Error("server down")) + useConversationRuntimeStore.getState().actions.fetchDetail(CID) + + sync() + await vi.advanceTimersByTimeAsync(0) + landLoad(detail([userTurn("u", "hi"), assistantTurn("a", "Hi! …")], 42)) + await vi.advanceTimersByTimeAsync(10_000) + + expect(session()?.detailLoading).toBe(false) + expect(session()?.detail?.transcript_watermark).toBe(42) + }) + + it("stops waiting behind a load that never settles", async () => { + vi.useFakeTimers() + seed({}) + mockGet.mockImplementation( + () => new Promise(() => {}) + ) + useConversationRuntimeStore.getState().actions.fetchDetail(CID) + + sync() + await vi.advanceTimersByTimeAsync(60_000) + + expect(mockGet).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + }) +}) + describe("syncViewerDetail — cancellation", () => { it("removeConversation cancels a pending poll (no further fetch)", async () => { vi.useFakeTimers()