From efec029e862cf63dc910d29d9faac24b80a83b43 Mon Sep 17 00:00:00 2001 From: Adam Dalloul <47503782+Adam-Dalloul@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:41:28 -0700 Subject: [PATCH 1/2] fix(tabs): keep the transcript when a tab drag reparents its view --- .../conversation-detail-panel.tsx | 22 ++++- .../group-shell-reconciliation.test.tsx | 72 ++++++++++++++++- src/contexts/tab-context.test.tsx | 81 +++++++++++++++++++ src/stores/tab-store.ts | 67 +++++++++++---- 4 files changed, 222 insertions(+), 20 deletions(-) diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index 2c18e3ed28..c9101c36ec 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -950,6 +950,20 @@ const ConversationTabView = memo(function ConversationTabView({ return () => { mountedRef.current = false syncCancelRef.current?.() + if (isReparentUnmount(useTabStore.getState(), tabId, groupId)) { + // Dragging the tab into another group reparents this view: React + // remounts it under that group's shell while the tab stays open. The + // connection is already held across that unmount (see + // `isTransientUnmount` above) and the runtime session has to be held + // with it — it holds the transcript. Dropping it emptied the message + // list and nothing brought it back: the remounted view re-registers + // its live-message sink on the connection it just kept, which recreates + // the session with a `liveMessage` and no `detail`, and `fetchDetail` + // skips a session that already has live data. Header, title and + // composer all read the tab row, so they looked untouched while the + // transcript stayed blank until the tab was closed and reopened. + return + } if (connStatusRef.current === "prompting" && !isViewerRef.current) { // Owner, agent still responding — keep the session for deferred cleanup // (the background turn_complete handler removes it once done). @@ -963,7 +977,13 @@ const ConversationTabView = memo(function ConversationTabView({ removeConversation(effectiveConversationId) } } - }, [effectiveConversationId, removeConversation, setPendingCleanup]) + }, [ + effectiveConversationId, + groupId, + removeConversation, + setPendingCleanup, + tabId, + ]) const handleSend = useCallback( ( diff --git a/src/components/conversations/group-shell-reconciliation.test.tsx b/src/components/conversations/group-shell-reconciliation.test.tsx index a964fa09f4..42c1a52517 100644 --- a/src/components/conversations/group-shell-reconciliation.test.tsx +++ b/src/components/conversations/group-shell-reconciliation.test.tsx @@ -1,7 +1,19 @@ import { readFileSync } from "node:fs" import { resolve } from "node:path" -import { describe, it, expect } from "vitest" -import { render } from "@testing-library/react" +import { describe, it, expect, vi } from "vitest" +import { act, render } from "@testing-library/react" + +import { + getTimelineTurns, + resetConversationRuntimeStore, + useConversationRuntimeStore, +} from "@/stores/conversation-runtime-store" + +vi.mock("@/lib/api", () => ({ + getFolderConversation: vi.fn(), +})) + +const { getFolderConversation } = await import("@/lib/api") const source = readFileSync( resolve( @@ -155,3 +167,59 @@ describe("split group shell source shape", () => { expect(shellBody.slice(stripIdx, contentIdx)).not.toContain("<>") }) }) + +/** + * The reparents the shells above cannot absorb. + * + * A tab dragged into another group DOES change React parents, so its view is + * remounted by design. The connection is deliberately carried across that + * unmount (`isTransientUnmount`), and the runtime session — which holds the + * transcript — has to be carried with it. Dropping the session there left the + * message list empty for as long as the tab stayed open: the remounted view + * re-registers its live-message sink on the connection it just kept, that + * recreates the session with live data and no detail, and `fetchDetail` skips + * a session that already has live data. Nothing refetches after that. + */ +describe("a reparented conversation view keeps its runtime session", () => { + it("consults the reparent classifier before either destructive branch", () => { + const cleanupStart = source.indexOf( + "// Cleanup runtime data on unmount (tab close)" + ) + expect(cleanupStart).toBeGreaterThan(-1) + const cleanup = source.slice(cleanupStart, cleanupStart + 2000) + const guardIdx = cleanup.indexOf("isReparentUnmount(useTabStore.getState()") + const deferIdx = cleanup.indexOf("setPendingCleanup(") + const removeIdx = cleanup.indexOf("removeConversation(") + expect(guardIdx).toBeGreaterThan(-1) + // Both ways of ending a session sit behind the classifier. + expect(deferIdx).toBeGreaterThan(guardIdx) + expect(removeIdx).toBeGreaterThan(guardIdx) + // Same inputs the connection's own guard uses, so the two agree on what a + // reparent is. + expect(cleanup.slice(guardIdx, deferIdx)).toContain("tabId, groupId") + }) + + it("cannot reload the transcript once a live sink has recreated the session", async () => { + resetConversationRuntimeStore() + const { actions } = useConversationRuntimeStore.getState() + + // What the remounted view does first: re-register its live-message sink on + // the connection it kept. The session comes back empty, but live. + act(() => { + actions.setLiveMessage( + 7, + { id: "lm-1", role: "assistant", content: [], startedAt: 0 }, + true + ) + }) + expect(getTimelineTurns(7)).toHaveLength(0) + + act(() => { + actions.fetchDetail(7) + }) + await act(async () => {}) + + expect(getFolderConversation).not.toHaveBeenCalled() + expect(getTimelineTurns(7)).toHaveLength(0) + }) +}) diff --git a/src/contexts/tab-context.test.tsx b/src/contexts/tab-context.test.tsx index d0587b4512..1e2ff72891 100644 --- a/src/contexts/tab-context.test.tsx +++ b/src/contexts/tab-context.test.tsx @@ -2215,6 +2215,87 @@ describe("TabProvider tab groups", () => { expect(store().rawTabs).toBe(before) }) + /** + * The unsplit strip's reorder, which has to take the same care its + * split-group sibling above already takes. + * + * `Reorder.Group` emits the order of the items it has measured since its own + * last render, filtered by REFERENCE against its current `values`. The filter + * can only remove, and the tab objects it holds are the ones the strip + * rendered with — so the list it hands back is a request to MOVE tabs, not a + * new tab set. Adopting it wholesale closed whichever tab was missing from it + * and rolled the rest back to whatever the strip last rendered. + */ + describe("reorderTabs", () => { + it("keeps a tab the reorder callback left out", async () => { + await renderWithTabs([tabItem(1, 1, true), tabItem(1, 2), tabItem(1, 3)]) + const rendered = store().tabs + + act(() => { + store().reorderTabs([rendered[2], rendered[1]]) + }) + + // The omitted tab is still open — and still the one being looked at. + expect(store().rawTabs.map((t) => t.id)).toContain("conv-1-codex-1") + expect(store().activeTabId).toBe("conv-1-codex-1") + expect(store().rawTabs).toHaveLength(3) + }) + + it("does not roll a tab back to what the strip rendered with", async () => { + await renderWithTabs([tabItem(1, 1, true)]) + act(() => { + store().openNewConversationTab(1, "/repo") + }) + const draftId = store().rawTabs.find((t) => t.conversationId == null)!.id + // What the strip was holding when the drag began. + const rendered = store().tabs + + // The draft's first message lands mid-drag: it binds to a real + // conversation, and its transcript now lives under a virtual runtime id. + act(() => { + store().bindConversationTab(draftId, 7, "codex", "sent", -42) + }) + act(() => { + store().reorderTabs([rendered[1], rendered[0]]) + }) + + expect(store().rawTabs.map((t) => t.id)).toEqual([ + draftId, + "conv-1-codex-1", + ]) + const moved = store().rawTabs.find((t) => t.id === draftId)! + expect(moved.conversationId).toBe(7) + expect(moved.runtimeConversationId).toBe(-42) + }) + + it("still moves the tab on a complete permutation", async () => { + await renderWithTabs([tabItem(1, 1, true), tabItem(1, 2), tabItem(1, 3)]) + const rendered = store().tabs + + act(() => { + store().reorderTabs([rendered[1], rendered[2], rendered[0]]) + }) + + expect(store().rawTabs.map((t) => t.id)).toEqual([ + "conv-1-codex-2", + "conv-1-codex-3", + "conv-1-codex-1", + ]) + }) + + it("refuses a list that names one tab twice", async () => { + await renderWithTabs([tabItem(1, 1, true), tabItem(1, 2)]) + const rendered = store().tabs + const before = store().rawTabs + + act(() => { + store().reorderTabs([rendered[1], rendered[1]]) + }) + + expect(store().rawTabs).toBe(before) + }) + }) + it("per-group draft singleton: each group reuses its own draft", async () => { await renderWithTabs([tabItem(1, 1, true)]) const home = leaves()[0] diff --git a/src/stores/tab-store.ts b/src/stores/tab-store.ts index 0d76967df9..9f119f22a0 100644 --- a/src/stores/tab-store.ts +++ b/src/stores/tab-store.ts @@ -502,6 +502,44 @@ function moveTabToSlot( return insertTab(without, tabs[from], index) } +/** + * `raw` with the tabs sitting at `slots` rearranged into the order a reorder + * callback asked for, or `null` when that list is not a permutation of exactly + * those slots (and so must be ignored). + * + * A reorder callback is a request to MOVE tabs, never a new tab set. The + * distinction matters because `Reorder.Group` emits the order it has measured + * since its own last render and then drops, by reference, whatever is missing + * from its current `values` — so mid-drag it can legitimately hand back a list + * that is short, repeats an id, or carries a tab object from an earlier derive. + * Resolving every entry back to the live `rawTabs` item by id keeps a drag + * unable to close a tab, resurrect a closed one, or write a stale copy of a + * tab's fields over the current one (a draft that bound to a conversation + * mid-drag would otherwise go back to being an unbound draft). + */ +function permuteSlots( + raw: TabItemInternal[], + slots: number[], + orderedTabs: TabItem[] +): TabItemInternal[] | null { + if (orderedTabs.length !== slots.length) return null + const slotIds = new Set(slots.map((i) => raw[i].id)) + const seen = new Set() + const ordered: TabItemInternal[] = [] + for (const tab of orderedTabs) { + if (!slotIds.has(tab.id) || seen.has(tab.id)) return null + seen.add(tab.id) + const item = raw.find((t) => t.id === tab.id) + if (!item) return null + ordered.push(item) + } + const next = [...raw] + slots.forEach((slot, k) => { + next[slot] = ordered[k] + }) + return next.every((tab, i) => tab === raw[i]) ? null : next +} + /** Field-wise equality for derived tab items. Backs the cross-derive reuse in * the `tabs` derivation: an item whose every field matches the previous derive * keeps its old reference, so downstream `Object.is` gates (consumers' memos) @@ -1628,24 +1666,10 @@ export const useTabStore = create()((set, get) => ({ slots.push(i) } }) - if (orderedTabs.length !== slots.length) return - const slotIds = new Set(slots.map((i) => raw[i].id)) - const seen = new Set() - const ordered: TabItemInternal[] = [] - for (const tab of orderedTabs) { - if (!slotIds.has(tab.id) || seen.has(tab.id)) return - seen.add(tab.id) - const item = raw.find((t) => t.id === tab.id) - if (!item) return - ordered.push(item) - } // Partition permutation: only this group's slots move, so the other // groups' persisted positions stay byte-stable. - const next = [...raw] - slots.forEach((slot, k) => { - next[slot] = ordered[k] - }) - if (next.every((tab, i) => tab === raw[i])) return + const next = permuteSlots(raw, slots, orderedTabs) + if (!next) return set({ rawTabs: next }) recomputeTabs() }, @@ -1664,7 +1688,16 @@ export const useTabStore = create()((set, get) => ({ }, reorderTabs: (reorderedTabs) => { - set({ rawTabs: reorderedTabs }) + // The unsplit strip shows every tab, so its slots are the whole array — + // otherwise identical to a group reorder, guards included. + const raw = get().rawTabs + const next = permuteSlots( + raw, + raw.map((_, i) => i), + reorderedTabs + ) + if (!next) return + set({ rawTabs: next }) recomputeTabs() }, From 9b7adad1edb036bf24feeaca5eb00525ca07f365 Mon Sep 17 00:00:00 2001 From: xintaofei Date: Thu, 24 Sep 2026 09:32:33 +0800 Subject: [PATCH 2/2] fix(tabs): carry the runtime session and metadata sync across a reparent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reparent keeps the conversation's runtime session, but the view it remounts could still lose it. The remounted view keyed itself by the row id, so a tab that started as a draft (its transcript filed under a virtual key) reloaded the row into a second session while the kept one lingered, and the context-usage, session-details and Diff readers stayed on a copy nothing updated. Each view now registers the group and key it mounted with, and an arriving view inherits its predecessor's key when `isReparentUnmount` says that predecessor is being reparented — the question its cleanup asks. An unmount also cannot tell that a view is about to mount straight back onto its session: StrictMode replays every fresh mount's effects in development, and the desktop/mobile layout swap remounts every view in one commit. Neither is a reparent, so the session was removed on the spot and the returning view came up empty. Views now release their session for removal at the end of the task, and a view mounting on the same key claims it back. The unmount also cancelled the post-turn metadata sync before consulting the classifier, so a drag within its retry window of a reply finishing lost that reply's usage, model and fork-point name. --- .../conversation-detail-panel.tsx | 60 ++-- .../group-shell-reconciliation.test.tsx | 270 +++++++++++++++++- src/stores/conversation-runtime-store.ts | 40 +++ src/stores/tab-store.ts | 51 ++++ 4 files changed, 398 insertions(+), 23 deletions(-) diff --git a/src/components/conversations/conversation-detail-panel.tsx b/src/components/conversations/conversation-detail-panel.tsx index c9101c36ec..6fa296b5db 100644 --- a/src/components/conversations/conversation-detail-panel.tsx +++ b/src/components/conversations/conversation-detail-panel.tsx @@ -27,7 +27,12 @@ import { useAcpAgents } from "@/hooks/use-acp-agents" import { useActiveFolder } from "@/contexts/active-folder-context" import { useAppWorkspaceStore } from "@/stores/app-workspace-store" import { useTabActions, useTabStore } from "@/contexts/tab-context" -import { groupOfTab, isReparentUnmount } from "@/stores/tab-store" +import { + groupOfTab, + isReparentUnmount, + reparentedViewRuntimeConversationId, + trackConversationView, +} from "@/stores/tab-store" import { computeRects, leafIds } from "@/lib/tab-group-layout" import { useTaskContext } from "@/contexts/task-context" import { cn, copyTextToClipboard, randomUUID } from "@/lib/utils" @@ -91,9 +96,11 @@ import { } from "@/lib/queue-flush" import { TurnBusyError } from "@/lib/turn-busy" import { + claimRuntimeSession, getConversationIdByExternalIdFromStore, getRuntimeSession, getTimelineTurns, + releaseRuntimeSession, useConversationRuntimeActions, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" @@ -286,7 +293,6 @@ const ConversationTabView = memo(function ConversationTabView({ completeTurn, refetchDetail, syncTurnMetadata, - removeConversation, setAcpLoadError, setDbConversationId, setExternalId, @@ -298,9 +304,19 @@ const ConversationTabView = memo(function ConversationTabView({ // Stable runtime session key — set once at mount, never changes. // For new conversations this is a virtual (negative) ID; for existing - // conversations opened from the sidebar it equals the real DB ID. + // conversations opened from the sidebar it equals the real DB ID. A view + // remounted by a reparent carries on with its predecessor's key, which for a + // tab that started as a draft is still the virtual one (see + // `reparentedViewRuntimeConversationId`). const [effectiveConversationId] = useState( - () => conversationId ?? buildVirtualConversationId(`draft-${tabId}`) + () => + reparentedViewRuntimeConversationId(useTabStore.getState(), tabId) ?? + conversationId ?? + buildVirtualConversationId(`draft-${tabId}`) + ) + useEffect( + () => trackConversationView(tabId, groupId, effectiveConversationId), + [tabId, groupId, effectiveConversationId] ) const [createdConversationId, setCreatedConversationId] = useState< number | null @@ -361,8 +377,10 @@ const ConversationTabView = memo(function ConversationTabView({ setTabRuntimeConversationId, ]) - // Clear pendingCleanup when tab is (re)opened + // Clear pendingCleanup when tab is (re)opened, and take the session back from + // a release the view just before this one scheduled on it. useEffect(() => { + claimRuntimeSession(effectiveConversationId) setPendingCleanup(effectiveConversationId, false) }, [effectiveConversationId, setPendingCleanup]) @@ -850,7 +868,7 @@ const ConversationTabView = memo(function ConversationTabView({ // rekey path: close+reopen mid-turn, where detail.turns may already hold user // turns that would otherwise drop the live assistant stream). Turn-end clearing // is owned by COMPLETE_TURN (nulls liveMessage); unmount clearing by - // removeConversation. `tabId` is the connection contextKey. + // releaseRuntimeSession. `tabId` is the connection contextKey. useEffect(() => { return acpActions.registerLiveMessageSink(tabId, (liveMessage, isLive) => setLiveMessage(effectiveConversationId, liveMessage, isLive) @@ -949,7 +967,6 @@ const ConversationTabView = memo(function ConversationTabView({ mountedRef.current = true return () => { mountedRef.current = false - syncCancelRef.current?.() if (isReparentUnmount(useTabStore.getState(), tabId, groupId)) { // Dragging the tab into another group reparents this view: React // remounts it under that group's shell while the tab stays open. The @@ -962,28 +979,31 @@ const ConversationTabView = memo(function ConversationTabView({ // skips a session that already has live data. Header, title and // composer all read the tab row, so they looked untouched while the // transcript stayed blank until the tab was closed and reopened. + // + // The post-turn metadata sync is left running for the same reason: it + // patches the session, not this view, and short of reopening the + // conversation it is the only thing that lands a live reply's usage, + // model and fork-point name. Cancelling it here lost them whenever the + // drag came within its retry window of a reply finishing. return } + syncCancelRef.current?.() if (connStatusRef.current === "prompting" && !isViewerRef.current) { // Owner, agent still responding — keep the session for deferred cleanup // (the background turn_complete handler removes it once done). setPendingCleanup(effectiveConversationId, true) } else { - // Idle owner, or a VIEWER (any status): remove immediately. A viewer's - // unmount detaches its attach subscription, so no turn_complete will - // arrive to resolve a deferred cleanup — deferring would leak the - // runtime session (especially in web mode, which has no event firehose - // after detach). - removeConversation(effectiveConversationId) + // Idle owner, or a VIEWER (any status): remove now rather than on a + // turn_complete. A viewer's unmount detaches its attach subscription, + // so no turn_complete will arrive to resolve a deferred cleanup — + // waiting for one would leak the runtime session (especially in web + // mode, which has no event firehose after detach). "Now" is the end of + // this task, so a view mounting straight back onto the session can + // still claim it (see `releaseRuntimeSession`). + releaseRuntimeSession(effectiveConversationId) } } - }, [ - effectiveConversationId, - groupId, - removeConversation, - setPendingCleanup, - tabId, - ]) + }, [effectiveConversationId, groupId, setPendingCleanup, tabId]) const handleSend = useCallback( ( diff --git a/src/components/conversations/group-shell-reconciliation.test.tsx b/src/components/conversations/group-shell-reconciliation.test.tsx index 42c1a52517..9d0a7b5615 100644 --- a/src/components/conversations/group-shell-reconciliation.test.tsx +++ b/src/components/conversations/group-shell-reconciliation.test.tsx @@ -1,13 +1,24 @@ import { readFileSync } from "node:fs" import { resolve } from "node:path" +import { StrictMode, useEffect, useState, type ReactNode } from "react" import { describe, it, expect, vi } from "vitest" import { act, render } from "@testing-library/react" import { + claimRuntimeSession, + getRuntimeSession, getTimelineTurns, + releaseRuntimeSession, resetConversationRuntimeStore, useConversationRuntimeStore, } from "@/stores/conversation-runtime-store" +import { + isReparentUnmount, + reparentedViewRuntimeConversationId, + trackConversationView, + type TabItemInternal, +} from "@/stores/tab-store" +import { singleGroupLayout, splitGroup } from "@/lib/tab-group-layout" vi.mock("@/lib/api", () => ({ getFolderConversation: vi.fn(), @@ -181,19 +192,29 @@ describe("split group shell source shape", () => { * a session that already has live data. Nothing refetches after that. */ describe("a reparented conversation view keeps its runtime session", () => { - it("consults the reparent classifier before either destructive branch", () => { + it("consults the reparent classifier before anything that ends the session's work", () => { const cleanupStart = source.indexOf( "// Cleanup runtime data on unmount (tab close)" ) + const cleanupEnd = source.indexOf( + "const handleSend = useCallback(", + cleanupStart + ) expect(cleanupStart).toBeGreaterThan(-1) - const cleanup = source.slice(cleanupStart, cleanupStart + 2000) + expect(cleanupEnd).toBeGreaterThan(cleanupStart) + const cleanup = source.slice(cleanupStart, cleanupEnd) const guardIdx = cleanup.indexOf("isReparentUnmount(useTabStore.getState()") + const cancelSyncIdx = cleanup.indexOf("syncCancelRef.current?.()") const deferIdx = cleanup.indexOf("setPendingCleanup(") - const removeIdx = cleanup.indexOf("removeConversation(") + const removeIdx = cleanup.indexOf("releaseRuntimeSession(") expect(guardIdx).toBeGreaterThan(-1) // Both ways of ending a session sit behind the classifier. expect(deferIdx).toBeGreaterThan(guardIdx) expect(removeIdx).toBeGreaterThan(guardIdx) + // So does cancelling its post-turn metadata sync: the sync patches the + // session the reparent keeps, and a reply whose sync was cancelled never + // gets its usage, model or fork-point name while the tab stays open. + expect(cancelSyncIdx).toBeGreaterThan(guardIdx) // Same inputs the connection's own guard uses, so the two agree on what a // reparent is. expect(cleanup.slice(guardIdx, deferIdx)).toContain("tabId, groupId") @@ -223,3 +244,246 @@ describe("a reparented conversation view keeps its runtime session", () => { expect(getTimelineTurns(7)).toHaveLength(0) }) }) + +/** + * The other half of a reparent: the view that REMOUNTS has to key itself on the + * session its predecessor kept. + * + * A conversation started as a draft streams under a virtual runtime id and + * keeps it after the first send binds the tab to a row. A remount that keyed + * itself by the row id instead reloaded the row into a second session beside + * the kept one, which then sat in the store for good — with the usage / + * session-details / Diff readers that follow `runtimeConversationId ?? + * conversationId` still reading it, though nothing updates it anymore. + * + * The inheritance has to fire for a reparent and ONLY for one. The + * desktop/mobile layout swap also remounts every view in one commit, but moves + * none of them, so the predecessor's cleanup releases its session: a successor + * that inherited that key would sit on nothing, and a virtual key never fetches. + * And because an unmount cannot tell that a view is coming straight back — + * that swap, or StrictMode replaying a fresh mount's effects in development — + * a released session survives the task, and a view mounting on it claims it. + */ +describe("a reparented view inherits its predecessor's runtime key", () => { + const TAB = "conv-1-codex-7" + /** What the tab mounted on while it was still a draft. */ + const DRAFT_KEY = -7 + /** What a fresh mount keys on once the tab is bound to its row. */ + const ROW = 7 + const LAYOUT = splitGroup(singleGroupLayout("a"), "a", "right", "b") + + type TabState = Parameters[0] + + /** The tab store as the views read it: the tab open, assigned to `group`. */ + function tabIn(group: string): TabState { + return { + rawTabs: [{ id: TAB } as TabItemInternal], + groupOf: { [TAB]: group }, + groupLayout: LAYOUT, + } + } + + /** A runtime session with a transcript in it, like the one a bound draft's + * view has been streaming into. */ + function seedSession(key: number) { + resetConversationRuntimeStore() + const { actions } = useConversationRuntimeStore.getState() + actions.setDbConversationId(key, ROW) + } + + /** Let the task a release waits out end. */ + async function endTask() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + } + + interface ViewProps { + groupId: string + freshKey: number + state: () => TabState + onKey: (key: number) => void + onCleanup: (verdict: "keep" | "release") => void + } + + /** `ConversationTabView` reduced to its session hand-off, wired the same + * way: pick the key at first render, register it, claim the session, and + * let the unmount ask the reparent classifier whether to release it. */ + function View({ groupId, freshKey, state, onKey, onCleanup }: ViewProps) { + const [key] = useState( + () => reparentedViewRuntimeConversationId(state(), TAB) ?? freshKey + ) + useEffect(() => trackConversationView(TAB, groupId, key), [groupId, key]) + useEffect(() => { + claimRuntimeSession(key) + }, [key]) + useEffect(() => { + onKey(key) + return () => { + if (isReparentUnmount(state(), TAB, groupId)) { + onCleanup("keep") + return + } + onCleanup("release") + releaseRuntimeSession(key) + } + }, [groupId, key, onCleanup, onKey, state]) + return null + } + + function Desktop({ children }: { children: ReactNode }) { + return
{children}
+ } + + function Mobile({ children }: { children: ReactNode }) { + return
{children}
+ } + + /** Keyed group shells, as `renderGroupShell` lays them out, under the layout + * shell — which, like the workspace layout's, is a different component on + * mobile and so remounts everything below it when the breakpoint flips. */ + function Workspace({ + mobile, + group, + ...view + }: { mobile: boolean; group: string } & Omit) { + const shells = ["a", "b"].map((groupId) => ( +
+ {groupId === group && } +
+ )) + return mobile ? {shells} : {shells} + } + + function harness({ strict = false } = {}) { + let current = tabIn("a") + const keys: number[] = [] + const cleanups: string[] = [] + const view = { + state: () => current, + onKey: (key: number) => keys.push(key), + onCleanup: (verdict: string) => cleanups.push(verdict), + } + return { + keys, + cleanups, + moveTo: (group: string) => { + current = tabIn(group) + }, + ui: (mobile: boolean, group: string, freshKey: number) => { + const workspace = ( + + ) + return strict ? {workspace} : workspace + }, + } + } + + it("carries the kept key and session when a move reparents the view", async () => { + seedSession(DRAFT_KEY) + const h = harness() + const { rerender, unmount } = render(h.ui(false, "a", DRAFT_KEY)) + + // The first send has bound the tab (a fresh mount would now key on the + // row), and then the tab is dragged into the other group. + h.moveTo("b") + rerender(h.ui(false, "b", ROW)) + await endTask() + + expect(h.cleanups).toEqual(["keep"]) + expect(h.keys).toEqual([DRAFT_KEY, DRAFT_KEY]) + expect(getRuntimeSession(DRAFT_KEY)).not.toBeNull() + + // Closing it for real still gives the session up. + unmount() + await endTask() + expect(getRuntimeSession(DRAFT_KEY)).toBeNull() + expect(reparentedViewRuntimeConversationId(tabIn("a"), TAB)).toBeNull() + }) + + it("starts fresh when a layout swap remounts the view without moving it", async () => { + seedSession(DRAFT_KEY) + const h = harness() + const { rerender, unmount } = render(h.ui(false, "a", DRAFT_KEY)) + + rerender(h.ui(true, "a", ROW)) + await endTask() + + expect(h.cleanups).toEqual(["release"]) + expect(h.keys).toEqual([DRAFT_KEY, ROW]) + // Nothing mounted on the draft key again, so its session went. + expect(getRuntimeSession(DRAFT_KEY)).toBeNull() + unmount() + }) + + it("keeps the session a layout swap remounts a view straight back onto", async () => { + seedSession(ROW) + const h = harness() + const { rerender, unmount } = render(h.ui(false, "a", ROW)) + + rerender(h.ui(true, "a", ROW)) + await endTask() + + expect(h.cleanups).toEqual(["release"]) + expect(getRuntimeSession(ROW)).not.toBeNull() + unmount() + }) + + it("survives StrictMode replaying the arriving view's effects", async () => { + seedSession(DRAFT_KEY) + const h = harness({ strict: true }) + const { rerender, unmount } = render(h.ui(false, "a", DRAFT_KEY)) + + h.moveTo("b") + rerender(h.ui(false, "b", ROW)) + await endTask() + + // The replay's unmount is not a reparent (the view is already in "b"), so + // it releases — and the replayed mount claims the session straight back. + expect(h.cleanups).toContain("release") + expect(h.keys[h.keys.length - 1]).toBe(DRAFT_KEY) + expect(getRuntimeSession(DRAFT_KEY)).not.toBeNull() + unmount() + await endTask() + expect(getRuntimeSession(DRAFT_KEY)).toBeNull() + }) + + it("lets a view's release drop only its own registration", () => { + const releasePredecessor = trackConversationView(TAB, "a", -1) + const releaseSuccessor = trackConversationView(TAB, "b", -2) + + releasePredecessor() + // The successor is still registered: moving the tab out of its group again + // hands ITS key on. + expect(reparentedViewRuntimeConversationId(tabIn("a"), TAB)).toBe(-2) + + releaseSuccessor() + expect(reparentedViewRuntimeConversationId(tabIn("a"), TAB)).toBeNull() + }) + + it("is how the real view picks, registers and claims its key", () => { + const initStart = source.indexOf( + "const [effectiveConversationId] = useState(" + ) + const initEnd = source.indexOf("const [createdConversationId", initStart) + expect(initStart).toBeGreaterThan(-1) + expect(initEnd).toBeGreaterThan(initStart) + const init = source.slice(initStart, initEnd) + expect(init).toMatch( + /reparentedViewRuntimeConversationId\(\s*useTabStore\.getState\(\),\s*tabId\s*\)\s*\?\?\s*conversationId\s*\?\?/ + ) + expect(init).toMatch( + /trackConversationView\(\s*tabId,\s*groupId,\s*effectiveConversationId\s*\)/ + ) + // The mount effect that clears a deferred cleanup also takes the session + // back from a release the previous view scheduled. + expect(source).toMatch( + /claimRuntimeSession\(effectiveConversationId\)\s*setPendingCleanup\(effectiveConversationId, false\)/ + ) + }) +}) diff --git a/src/stores/conversation-runtime-store.ts b/src/stores/conversation-runtime-store.ts index 64fa185355..b4ad594415 100644 --- a/src/stores/conversation-runtime-store.ts +++ b/src/stores/conversation-runtime-store.ts @@ -4009,6 +4009,44 @@ export function getRuntimeSession( ) } +// Sessions a view released on unmount, each waiting one task for its removal. +const pendingSessionReleases = new Map>() + +/** + * Remove a runtime session once the current task is over, unless a view claims + * it first (`claimRuntimeSession`). + * + * This is how a conversation view gives up its session on unmount, because an + * unmount cannot tell that a view is about to mount straight back onto the + * same session: React StrictMode replays the effects of every fresh mount in + * development, and the desktop/mobile layout swap remounts every view in one + * commit. Neither moves the tab, so neither is a reparent, and removing the + * session on the spot emptied the transcript under the view that came back — + * its live-message sink recreates the session with live data and no detail, + * and `fetchDetail` skips a session that already has live data. + */ +export function releaseRuntimeSession(conversationId: number): void { + const pending = pendingSessionReleases.get(conversationId) + if (pending != null) clearTimeout(pending) + pendingSessionReleases.set( + conversationId, + setTimeout(() => { + pendingSessionReleases.delete(conversationId) + useConversationRuntimeStore + .getState() + .actions.removeConversation(conversationId) + }, 0) + ) +} + +/** Keep a session a view is mounting on: cancels its pending release. */ +export function claimRuntimeSession(conversationId: number): void { + const pending = pendingSessionReleases.get(conversationId) + if (pending == null) return + clearTimeout(pending) + pendingSessionReleases.delete(conversationId) +} + /** Resolve a runtime conversation id from an agent's external session id. */ export function getConversationIdByExternalIdFromStore( externalId: string @@ -4062,6 +4100,8 @@ export function resetConversationRuntimeStore(): void { fetchGeneration.clear() for (const cancel of viewerDetailSyncCancels.values()) cancel() viewerDetailSyncCancels.clear() + for (const pending of pendingSessionReleases.values()) clearTimeout(pending) + pendingSessionReleases.clear() timelineCache = new WeakMap() timelinePrefixCache = new WeakMap() useConversationRuntimeStore.setState({ diff --git a/src/stores/tab-store.ts b/src/stores/tab-store.ts index 9f119f22a0..d6f6a691eb 100644 --- a/src/stores/tab-store.ts +++ b/src/stores/tab-store.ts @@ -623,6 +623,57 @@ export function isReparentUnmount( return groupOfTab(state.groupOf, state.groupLayout, tabId) !== renderedGroupId } +/** The conversation view each tab has mounted: the group it rendered under and + * the runtime session key it mounted on. Module scope rather than store state + * because it tracks React mounts, which nothing renders from. */ +const mountedConversationViews = new Map< + string, + { groupId: string; runtimeConversationId: number } +>() + +/** Record the conversation view `tabId` just mounted. The returned release + * drops only its own entry, so it can never unregister a successor. */ +export function trackConversationView( + tabId: string, + groupId: string, + runtimeConversationId: number +): () => void { + const entry = { groupId, runtimeConversationId } + mountedConversationViews.set(tabId, entry) + return () => { + if (mountedConversationViews.get(tabId) === entry) { + mountedConversationViews.delete(tabId) + } + } +} + +/** + * Mount-side counterpart of `isReparentUnmount`: the runtime session key a + * conversation view arriving for `tabId` must inherit from the view it + * replaces, or null when a fresh key is right. + * + * React renders the arriving view BEFORE the departing one's cleanup runs, so + * at this view's first render its predecessor is still registered, and that + * cleanup is about to ask `isReparentUnmount` about its own group. This asks the + * same question first. Yes means the session is kept, and the arriving view has + * to carry on with it rather than key itself by the tab's row id — a tab that + * started as a draft keeps its transcript under a virtual key for good. No + * means the session is about to be removed (the desktop/mobile layout swap + * remounts every view in one commit without moving any), so inheriting it + * would strand the view on a key with nothing behind it; a virtual key never + * fetches. + */ +export function reparentedViewRuntimeConversationId( + state: Pick, + tabId: string +): number | null { + const predecessor = mountedConversationViews.get(tabId) + if (!predecessor) return null + return isReparentUnmount(state, tabId, predecessor.groupId) + ? predecessor.runtimeConversationId + : null +} + /** Where a new tab should land: the explicit target when it's a live group, * else the focused (active tab's) group. */ function resolveTargetGroup(