From 62aee001a6f627e24ba642c50d36084a01734821 Mon Sep 17 00:00:00 2001 From: Flyneen <48707833+Flyneen@users.noreply.github.com> Date: Sat, 10 Oct 2026 10:59:15 +0800 Subject: [PATCH 1/8] perf(conversations): fetch a tab's detail only when it is visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every open tab keeps its ConversationTabView mounted (that is what preserves a background session's stream and scroll state), so the ungated useConversationDetail() call there fired one get_folder_conversation per open tab the moment the workspace restored its tab set — each one a tail window of up to TAIL_TURNS_DEFAULT (120) turns. Measured on a workspace with 49 open tabs: 49 concurrent requests in a single 100 ms burst, 25.4 MB decoded (7.2 MB gzipped), issued before the active conversation had painted. The cost grows with the number of open tabs AND with the length of each conversation, which is why reopening the workspace gets slower the longer it is used. Gate the auto-fetch on visibility instead of on mount. A hidden tab keeps its session, its scroll state and its position in the strip; it fetches on the first frame it becomes visible (tab switch, group selection, tiling), which is also the first moment its detail is actually needed. While here, stop seeding a child tab's summary from the full transcript: reconcileChildSummaries only reads detail.summary off the response, but it asked for the legacy unwindowed detail, so a long child session transferred and re-parsed megabytes of turns to read a title and a status. It now asks for tailTurns: 1. --- .../conversation-detail-panel.tsx | 22 ++++- src/contexts/tab-context.test.tsx | 4 +- src/hooks/use-conversation-detail.test.tsx | 57 +++++++++++- src/hooks/use-conversation-detail.ts | 9 ++ .../tab-store-child-summary-window.test.ts | 86 +++++++++++++++++++ src/stores/tab-store.ts | 9 +- 6 files changed, 183 insertions(+), 4 deletions(-) create mode 100644 src/stores/tab-store-child-summary-window.test.ts diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index e28dc129e4..656945d761 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -189,6 +189,16 @@ 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 with one conversation in flight + * instead of N. */ + isVisible: boolean } function buildOptimisticUserTurnFromDraft( @@ -255,6 +265,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 +503,20 @@ 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 its loading state behind `invisible`, costing nothing. const { detail, loading: detailLoading, error: detailError, acpLoadError, - } = useConversationDetail(effectiveConversationId) + } = useConversationDetail(effectiveConversationId, { enabled: isVisible }) // Subscribe to only the fields this panel actually reads from its runtime // session — NOT the whole session object. The live-message sink rewrites the @@ -2906,6 +2925,7 @@ export function ConversationDetailPanel() { showActiveFlow={(isSplit || canTileG) && active} reloadSignal={reloadByTabId[tab.id] ?? 0} groupId={groupId} + isVisible={visible} /> ) return ( 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..b56b76da6d 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -1,13 +1,24 @@ import { act, renderHook } from "@testing-library/react" -import { afterEach, describe, expect, it } from "vitest" +import { afterEach, describe, expect, it, vi } from "vitest" import type { LiveMessage } from "@/contexts/acp-connections-context" import type { DbConversationDetail } from "@/lib/types" import { resetConversationRuntimeStore, + TAIL_TURNS_DEFAULT, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" import { 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) { @@ -111,3 +122,47 @@ 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(() => { + 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 }) + }) +}) diff --git a/src/hooks/use-conversation-detail.ts b/src/hooks/use-conversation-detail.ts index 40245329e4..f2737c5094 100644 --- a/src/hooks/use-conversation-detail.ts +++ b/src/hooks/use-conversation-detail.ts @@ -22,6 +22,15 @@ 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 } 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..12a29365f7 --- /dev/null +++ b/src/stores/tab-store-child-summary-window.test.ts @@ -0,0 +1,86 @@ +/** + * `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 }) + }) +}) diff --git a/src/stores/tab-store.ts b/src/stores/tab-store.ts index d6f6a691eb..4dd2cf991a 100644 --- a/src/stores/tab-store.ts +++ b/src/stores/tab-store.ts @@ -2377,7 +2377,14 @@ 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` still returns the summary and + // round-aligns to the last user round, which is what the tab label and + // status need. + void getFolderConversation(id, { tailTurns: 1 }) .then((detail) => { if (seedEpoch !== epoch) return const buffered = childSeedBuffer.get(id) From 6ac1ed2a21fd2b6c09aa1601815df1771b163b02 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 14:54:28 +0800 Subject: [PATCH 2/8] fix(conversations): keep a newly shown tab from connecting before its detail loads A tab restored behind another one is mounted hidden with a runtime session (its mount effects create one) but no detail, because its fetch now waits until it is shown. The render that shows it reported that session as settled, with no detail and nothing loading. It is also the render that makes the tab active, so the auto-connect gate, which waits on detailLoading for the stored session id, let the connect out with no session id. Depending on which finished first, that either spawned an agent on session/new only to kill it once the detail landed, or kept the tab on that new session, so the next prompt re-pointed the conversation away from its history. useConversationDetail now counts a fetch it is about to start as loading. It uses the same admission rule as fetchDetail, shared as sessionHoldsActiveTurns, so the gate stays closed from the first render until the stored session id arrives. --- .../conversation-detail-panel.tsx | 15 +- .../tab-first-show-auto-connect.test.tsx | 178 ++++++++++++++++++ src/hooks/use-conversation-detail.test.tsx | 64 ++++++- src/hooks/use-conversation-detail.ts | 63 +++++-- src/stores/conversation-runtime-store.ts | 26 ++- 5 files changed, 316 insertions(+), 30 deletions(-) create mode 100644 src/components/conversations/tab-first-show-auto-connect.test.tsx diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index 656945d761..20f6f39a50 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -196,8 +196,8 @@ interface ConversationTabViewProps { * 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 with one conversation in flight - * instead of N. */ + * 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. */ isVisible: boolean } @@ -510,7 +510,10 @@ const ConversationTabView = memo(function ConversationTabView({ // 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 its loading state behind `invisible`, costing nothing. + // 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. const { detail, loading: detailLoading, @@ -576,6 +579,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 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..aadf3004ef --- /dev/null +++ b/src/components/conversations/tab-first-show-auto-connect.test.tsx @@ -0,0 +1,178 @@ +/** + * 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. + * + * `ConversationTabView` is too heavy to render here, so `TabViewGate` repeats + * its wiring of the pieces involved — the mount-time session claim, the + * visibility-gated `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 } from "react" +import { afterEach, describe, expect, it, vi } from "vitest" +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" + +function TabViewGate({ shown }: { shown: 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]) + const { detail, loading: detailLoading } = useConversationDetail(CID, { + enabled: shown, + }) + 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: shown && !awaitingHistoricalSessionId, + workingDir: "/repo", + sessionId: externalId, + conversationId: CID, + preparing: shown && awaitingHistoricalSessionId, + }) + return null +} + +describe("a hidden tab's first show", () => { + afterEach(() => { + cleanup() + act(() => resetConversationRuntimeStore()) + mockGet.mockReset() + stubs.connect.mockClear() + }) + + 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({ + summary: { id: CID, external_id: STORED_SESSION }, + turns: [], + } as unknown as DbConversationDetail) + }) + expect(stubs.connect).toHaveBeenCalledTimes(1) + expect(stubs.connect).toHaveBeenCalledWith( + "claude_code", + "/repo", + STORED_SESSION, + CID + ) + }) +}) + +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 fetch on visibility", () => { + expect(panel).toContain("setPendingCleanup(effectiveConversationId, false)") + expect(panel).toContain( + "useConversationDetail(effectiveConversationId, { enabled: isVisible })" + ) + 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/hooks/use-conversation-detail.test.tsx b/src/hooks/use-conversation-detail.test.tsx index b56b76da6d..dec0631bad 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -1,4 +1,4 @@ -import { act, renderHook } from "@testing-library/react" +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" @@ -132,6 +132,10 @@ describe("useConversationDetail streaming decoupling", () => { // 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() }) @@ -165,4 +169,62 @@ describe("useConversationDetail visibility gating", () => { 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("does not report a fetch it will not make as loading", () => { + act(() => { + useConversationRuntimeStore + .getState() + .actions.setLiveMessage(CID, liveMsg("m1"), true) + }) + + const { result } = renderHook(() => useConversationDetail(CID)) + + expect(result.current.loading).toBe(false) + expect(result.current.detail).toBeNull() + expect(mockGet).not.toHaveBeenCalled() + }) }) diff --git a/src/hooks/use-conversation-detail.ts b/src/hooks/use-conversation-detail.ts index f2737c5094..f8ca913867 100644 --- a/src/hooks/use-conversation-detail.ts +++ b/src/hooks/use-conversation-detail.ts @@ -3,6 +3,7 @@ import { useEffect } from "react" import { useShallow } from "zustand/react/shallow" import { + sessionHoldsActiveTurns, useConversationRuntimeActions, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" @@ -36,6 +37,18 @@ export function useConversationDetail( } ): { 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 @@ -49,32 +62,46 @@ 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, + } = 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, + // `fetchDetail`'s own admission rule, folded to one boolean: nothing + // loaded, nothing in flight, no ongoing turn holding the session. A + // streaming batch can't flip it — once a stream is under way (or a + // detail exists) it is already false — so the slice stays stable. + needsFetch: + session == null || + (session.detail == null && + !session.detailLoading && + !sessionHoldsActiveTurns(session)), + } + }) + ) const { fetchDetail } = useConversationRuntimeActions() const isVirtual = isVirtualConversationId(conversationId) + const fetchPending = enabled && !isVirtual && needsFetch 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]) 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..cb134e0156 100644 --- a/src/stores/conversation-runtime-store.ts +++ b/src/stores/conversation-runtime-store.ts @@ -2992,6 +2992,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 +3760,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 }) From 3f64610afee6b9711678f8a7310e494026ee5cd4 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 15:00:35 +0800 Subject: [PATCH 3/8] fix(conversations): stop re-requesting a conversation's detail as fast as it fails A failed detail fetch leaves no detail and nothing in flight, which is exactly what useConversationDetail's auto-fetch keys on, so the hook re-requested it on the very next render. 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 instead of staying next to its Reload action. A failure now waits for a retry schedule instead: 1, 2, 4, 8 and 16 seconds, then the error stays on screen until the user reloads. A loaded detail or a different conversation starts the schedule over, and a hidden view holds its pending retry until it is shown again. --- src/hooks/use-conversation-detail.test.tsx | 116 ++++++++++++++++++++- src/hooks/use-conversation-detail.ts | 61 +++++++++-- 2 files changed, 166 insertions(+), 11 deletions(-) diff --git a/src/hooks/use-conversation-detail.test.tsx b/src/hooks/use-conversation-detail.test.tsx index dec0631bad..676eed439c 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -7,7 +7,10 @@ import { 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. @@ -228,3 +231,114 @@ describe("useConversationDetail visibility gating", () => { 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 stops", async () => { + vi.useFakeTimers() + failEveryFetch("server down") + + renderHook(() => useConversationDetail(CID)) + await act(async () => {}) + expect(mockGet).toHaveBeenCalledTimes(1) + + for (const [step, delay] of DETAIL_RETRY_DELAYS_MS.entries()) { + await act(async () => { + vi.advanceTimersByTime(delay - 1) + }) + expect(mockGet).toHaveBeenCalledTimes(step + 1) + await act(async () => { + vi.advanceTimersByTime(1) + }) + expect(mockGet).toHaveBeenCalledTimes(step + 2) + } + + await act(async () => { + vi.advanceTimersByTime(10 * 60_000) + }) + expect(mockGet).toHaveBeenCalledTimes(DETAIL_RETRY_DELAYS_MS.length + 1) + }) + + 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 f8ca913867..56ede08595 100644 --- a/src/hooks/use-conversation-detail.ts +++ b/src/hooks/use-conversation-detail.ts @@ -1,6 +1,6 @@ "use client" -import { useEffect } from "react" +import { useEffect, useRef } from "react" import { useShallow } from "zustand/react/shallow" import { sessionHoldsActiveTurns, @@ -13,6 +13,20 @@ function isVirtualConversationId(conversationId: number): boolean { return !Number.isFinite(conversationId) || conversationId <= 0 } +/** + * Delays before each automatic retry of a failed detail fetch. 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. A few spaced retries still ride + * out a blip or a restart; after the last one the error stays on screen next + * to its Reload action. + */ +export const DETAIL_RETRY_DELAYS_MS: readonly number[] = [ + 1_000, 2_000, 4_000, 8_000, 16_000, +] + export function useConversationDetail( conversationId: number, options?: { @@ -69,36 +83,63 @@ export function useConversationDetail( 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, - // `fetchDetail`'s own admission rule, folded to one boolean: nothing - // loaded, nothing in flight, no ongoing turn holding the session. A - // streaming batch can't flip it — once a stream is under way (or a - // detail exists) it is already false — so the slice stays stable. - needsFetch: - session == null || - (session.detail == null && - !session.detailLoading && - !sessionHoldsActiveTurns(session)), + // 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 (!fetchPending) return fetchDetail(conversationId) }, [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 + if (used >= DETAIL_RETRY_DELAYS_MS.length) return + const timer = setTimeout(() => { + retriesUsedRef.current = used + 1 + fetchDetail(conversationId) + }, DETAIL_RETRY_DELAYS_MS[used]) + return () => clearTimeout(timer) + }, [retryPending, conversationId, fetchDetail]) + return { detail, loading: hasSession ? detailLoading || fetchPending : !isVirtual, From a67909737d282f1de598df72cdd256c75580421c Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 15:02:23 +0800 Subject: [PATCH 4/8] docs(tabs): say what the child-summary seed's one-turn window returns --- src/stores/tab-store.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/stores/tab-store.ts b/src/stores/tab-store.ts index 4dd2cf991a..ba806fe86b 100644 --- a/src/stores/tab-store.ts +++ b/src/stores/tab-store.ts @@ -2381,9 +2381,10 @@ export const useTabStore = create()((set, get) => ({ // 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` still returns the summary and - // round-aligns to the last user round, which is what the tab label and - // status need. + // 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 From 7fdc20770b7700d1e7ee0ca029b92d87c051d13b Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 15:14:34 +0800 Subject: [PATCH 5/8] fix(conversations): keep retrying a failed detail fetch every 30 seconds The retry schedule for a failed detail fetch used to end after its fifth step, leaving the error up until the user pressed Reload. The loop it replaced did recover on its own once the server was reachable again, so an outage longer than half a minute now needed a manual Reload where it once did not. The last step, 30 seconds, now repeats for as long as the failure lasts and the view stays on screen. A view picks its conversation back up within half a minute of the server returning, and the error stays readable in between. --- src/hooks/use-conversation-detail.test.tsx | 38 +++++++++++++++++++--- src/hooks/use-conversation-detail.ts | 25 +++++++------- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/hooks/use-conversation-detail.test.tsx b/src/hooks/use-conversation-detail.test.tsx index 676eed439c..1b3e3cd8dd 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -269,7 +269,7 @@ describe("useConversationDetail after a failed fetch", () => { expect(result.current.loading).toBe(false) }) - it("retries on a growing delay, then stops", async () => { + it("retries on a growing delay, then keeps retrying at the last one", async () => { vi.useFakeTimers() failEveryFetch("server down") @@ -277,7 +277,9 @@ describe("useConversationDetail after a failed fetch", () => { await act(async () => {}) expect(mockGet).toHaveBeenCalledTimes(1) - for (const [step, delay] of DETAIL_RETRY_DELAYS_MS.entries()) { + 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) }) @@ -287,11 +289,39 @@ describe("useConversationDetail after a failed fetch", () => { }) 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(10 * 60_000) + 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(DETAIL_RETRY_DELAYS_MS.length + 1) + expect(mockGet).toHaveBeenCalledTimes(5) }) it("lands the detail when a retry succeeds", async () => { diff --git a/src/hooks/use-conversation-detail.ts b/src/hooks/use-conversation-detail.ts index 56ede08595..5017e0a33d 100644 --- a/src/hooks/use-conversation-detail.ts +++ b/src/hooks/use-conversation-detail.ts @@ -14,17 +14,19 @@ function isVirtualConversationId(conversationId: number): boolean { } /** - * Delays before each automatic retry of a failed detail fetch. 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. A few spaced retries still ride - * out a blip or a restart; after the last one the error stays on screen next - * to its Reload action. + * 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, + 1_000, 2_000, 4_000, 8_000, 16_000, 30_000, ] export function useConversationDetail( @@ -132,11 +134,12 @@ export function useConversationDetail( useEffect(() => { if (!retryPending) return const used = retriesUsedRef.current - if (used >= DETAIL_RETRY_DELAYS_MS.length) return + const delay = + DETAIL_RETRY_DELAYS_MS[Math.min(used, DETAIL_RETRY_DELAYS_MS.length - 1)] const timer = setTimeout(() => { retriesUsedRef.current = used + 1 fetchDetail(conversationId) - }, DETAIL_RETRY_DELAYS_MS[used]) + }, delay) return () => clearTimeout(timer) }, [retryPending, conversationId, fetchDetail]) From a184bc089988c131ba8f296e9a2c8c0b8027cb17 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 15:14:34 +0800 Subject: [PATCH 6/8] test(conversations): cover every turn that skips a fetch, and the child-summary seed --- src/hooks/use-conversation-detail.test.tsx | 46 +++++++++++++------ .../tab-store-child-summary-window.test.ts | 9 ++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/hooks/use-conversation-detail.test.tsx b/src/hooks/use-conversation-detail.test.tsx index 1b3e3cd8dd..74ebc63e58 100644 --- a/src/hooks/use-conversation-detail.test.tsx +++ b/src/hooks/use-conversation-detail.test.tsx @@ -1,8 +1,9 @@ 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, @@ -24,7 +25,10 @@ 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([ [ @@ -55,6 +59,7 @@ function seedSession(detail: DbConversationDetail | null) { olderTurnsPrependEpoch: 0, pendingOutOfTurnContent: false, pendingCleanup: false, + ...overrides, }, ], ]), @@ -71,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 @@ -217,19 +230,22 @@ describe("useConversationDetail visibility gating", () => { // 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("does not report a fetch it will not make as loading", () => { - act(() => { - useConversationRuntimeStore - .getState() - .actions.setLiveMessage(CID, liveMsg("m1"), true) - }) - - const { result } = renderHook(() => useConversationDetail(CID)) - - expect(result.current.loading).toBe(false) - expect(result.current.detail).toBeNull() - expect(mockGet).not.toHaveBeenCalled() - }) + 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 diff --git a/src/stores/tab-store-child-summary-window.test.ts b/src/stores/tab-store-child-summary-window.test.ts index 12a29365f7..440acffb90 100644 --- a/src/stores/tab-store-child-summary-window.test.ts +++ b/src/stores/tab-store-child-summary-window.test.ts @@ -82,5 +82,14 @@ describe("child-tab summary seeding", () => { 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") }) }) From c515056c0bea9f76093a6138155fea3e8baa5b65 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 15:59:33 +0800 Subject: [PATCH 7/8] fix(conversations): keep a loaded hidden tab refetching its detail Gating the auto-fetch on visibility also stopped a hidden tab from refetching after its runtime session was dropped. Closing the sub-agent viewer drops the session it shares with an open tab; if that tab is hidden and holds a live connection, the next streamed batch recreates the session with live data and no detail, which fetchDetail never fills in, so the tab came back without its history. Only the first load waits for the tab to be shown now: once a view has held a detail it fetches the way every view did before. The gate also takes isActive, so the loading signal the auto-connect gate reads no longer leans on the tab store keeping the active tab its group's selected one. --- .../conversation-detail-panel.tsx | 24 ++- .../tab-first-show-auto-connect.test.tsx | 172 +++++++++++++++--- 2 files changed, 170 insertions(+), 26 deletions(-) diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index 20f6f39a50..62c2fde879 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -197,7 +197,8 @@ interface ConversationTabViewProps { * 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. */ + * (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 } @@ -513,13 +514,30 @@ const ConversationTabView = memo(function ConversationTabView({ // 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. + // `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, { enabled: isVisible }) + } = 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 diff --git a/src/components/conversations/tab-first-show-auto-connect.test.tsx b/src/components/conversations/tab-first-show-auto-connect.test.tsx index aadf3004ef..5415aee01a 100644 --- a/src/components/conversations/tab-first-show-auto-connect.test.tsx +++ b/src/components/conversations/tab-first-show-auto-connect.test.tsx @@ -8,17 +8,23 @@ * `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 - * visibility-gated `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. + * 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 } from "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" @@ -77,7 +83,26 @@ const mockGet = vi.mocked(getFolderConversation) const CID = 42 const STORED_SESSION = "sess-stored" -function TabViewGate({ shown }: { shown: boolean }) { +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() @@ -85,9 +110,13 @@ function TabViewGate({ shown }: { shown: boolean }) { 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: shown, + enabled: visible || active || heldDetail, }) + if (detail != null && !heldDetail) setHeldDetail(true) const runtimeExternalId = useConversationRuntimeStore( (s) => s.byConversationId.get(CID)?.externalId ?? null ) @@ -98,22 +127,28 @@ function TabViewGate({ shown }: { shown: boolean }) { useConnectionLifecycle({ contextKey: "tab-1", agentType: "claude_code", - isActive: shown && !awaitingHistoricalSessionId, + isActive: active && !awaitingHistoricalSessionId, workingDir: "/repo", sessionId: externalId, conversationId: CID, - preparing: shown && awaitingHistoricalSessionId, + 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(() => { - cleanup() - act(() => resetConversationRuntimeStore()) - mockGet.mockReset() - stubs.connect.mockClear() - }) + afterEach(resetAll) it("auto-connects only once its stored session id has arrived", async () => { let land!: (detail: DbConversationDetail) => void @@ -124,22 +159,19 @@ describe("a hidden tab's first show", () => { }) ) - const { rerender } = render() + const { rerender } = render() await act(async () => {}) expect(mockGet).not.toHaveBeenCalled() expect(stubs.connect).not.toHaveBeenCalled() await act(async () => { - rerender() + rerender() }) expect(mockGet).toHaveBeenCalledTimes(1) expect(stubs.connect).not.toHaveBeenCalled() await act(async () => { - land({ - summary: { id: CID, external_id: STORED_SESSION }, - turns: [], - } as unknown as DbConversationDetail) + land(storedDetail()) }) expect(stubs.connect).toHaveBeenCalledTimes(1) expect(stubs.connect).toHaveBeenCalledWith( @@ -149,6 +181,94 @@ describe("a hidden tab's first show", () => { 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", () => { @@ -160,10 +280,16 @@ describe("TabViewGate mirrors ConversationTabView", () => { "utf8" ) - it("creates the session on mount and gates the fetch on visibility", () => { + it("creates the session on mount and gates the first fetch on being shown", () => { expect(panel).toContain("setPendingCleanup(effectiveConversationId, false)") expect(panel).toContain( - "useConversationDetail(effectiveConversationId, { enabled: isVisible })" + "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}") }) From afecd5f8e6f8b0f878579f849a9e0eb42e53b3c3 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Sat, 10 Oct 2026 16:19:45 +0800 Subject: [PATCH 8/8] fix(conversations): never leave a detail load stranded by a background read Only a detail load's own result clears detailLoading, and two readers that never set the flag could invalidate that load by bumping the fetch generation: the cross-client viewer sync poll and the older-turns page. The load's result was then dropped as stale, and if the poll never committed (every read failing) or a page landed instead, the session stayed loading for good: spinner, auto-connect held shut, overlay folds stopped. The poll overlap is common, since a load's own read can backfill the title and that upsert is a nudge. The poll now waits for a load in flight to land, without spending an attempt, and polls on from there, giving up after 30 seconds behind a load that never settles. An older-turns page is not issued while a load is in flight; the near-top trigger fires again the next time the list is scrolled up into the top, and the loader row pages on click. --- src/stores/conversation-runtime-store.ts | 40 ++++++++- .../conversation-window-loading.test.ts | 24 ++++++ src/stores/viewer-detail-sync.test.ts | 82 +++++++++++++++++++ 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/src/stores/conversation-runtime-store.ts b/src/stores/conversation-runtime-store.ts index cb134e0156..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 @@ -3820,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) @@ -3830,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) @@ -3870,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 @@ -3897,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) @@ -3907,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/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()