diff --git a/src/components/layout/app-shell/AppShell.tsx b/src/components/layout/app-shell/AppShell.tsx index 33a2f3207..56f1c2c7b 100644 --- a/src/components/layout/app-shell/AppShell.tsx +++ b/src/components/layout/app-shell/AppShell.tsx @@ -391,8 +391,11 @@ function AppShellInner({ // viewport. Falls back to half the viewport when no wider width was ever // recorded (lastOpenWidth still at the MIN_WIDTH floor). const reopenWidth = () => { + // `>=`: lastOpenWidth === MIN_WIDTH is a real remembered width (user dragged + // to the floor), not "no saved width" — restore it instead of falling back + // to half-viewport. Only the seed default (0, below the floor) falls back. const remembered = - lastOpenWidth > MIN_WIDTH + lastOpenWidth >= MIN_WIDTH ? lastOpenWidth : (windowWidth || 1000) * 0.5; return Math.min(Math.max(remembered, MIN_WIDTH), maxWidthRef.current); @@ -433,7 +436,11 @@ function AppShellInner({ if (sidebarWidth <= MIN_WIDTH) { setSidebarWidth(reopenWidth()); } else { - setSidebarWidth(MIN_WIDTH); + // Collapse via seedWidth (NOT setSidebarWidth): now that a drag to exactly + // MIN_WIDTH is a remembered width, collapsing to the floor must NOT record + // it as the open width — otherwise expanding would have nothing else to + // restore. seedWidth shrinks the view without touching lastOpenWidth. + seedWidth(MIN_WIDTH); } }; diff --git a/src/context/sidebar/SidebarProvider.test.tsx b/src/context/sidebar/SidebarProvider.test.tsx index 39754b0b5..ce480ed90 100644 --- a/src/context/sidebar/SidebarProvider.test.tsx +++ b/src/context/sidebar/SidebarProvider.test.tsx @@ -13,14 +13,21 @@ afterEach(() => { }); function TestConsumer() { - const { sidebarWidth, setSidebarWidth, lastOpenWidth } = useSidebarWidth(); + const { sidebarWidth, setSidebarWidth, seedWidth, lastOpenWidth } = + useSidebarWidth(); return (
{sidebarWidth} {lastOpenWidth} - + {/* Collapse uses seedWidth (what AppShell's collapse button does) — sets + the rendered width to the floor WITHOUT recording it as the open + width, so reopen restores the prior size. */} + + {/* A user DRAG that lands exactly on the floor goes through + setSidebarWidth and IS a real chosen width (must be recorded). */} +
); } @@ -163,6 +170,35 @@ describe("SidebarProvider persistence", () => { expect(localStorage.getItem(SIDEBAR_WIDTH_STORAGE_KEY)).toBe("500"); }); + it("a DRAG to exactly minOpenWidth is recorded as the open width (regression: floor was dropped by a strict `>`)", () => { + // The bug: dragging the rail all the way to its minimum (width === + // minOpenWidth) was discarded — handleSetWidth/reopenWidth used a strict + // `>`, so lastOpenWidth kept the previous larger size and reopen restored + // THAT, not the dragged floor. A drag to the floor is a real chosen width + // and must stick. (Contrast the collapse test above, which seeds the floor + // and must NOT record.) + render( + + + , + ); + act(() => { + fireEvent.click(screen.getByText("Resize")); // open wide (500) first + }); + act(() => { + fireEvent.click(screen.getByText("DragFloor")); // drag down to the floor + }); + expect(screen.getByTestId("width").textContent).toBe("350"); + // The floor IS now the remembered open width — in memory AND storage. + expect(screen.getByTestId("last-open").textContent).toBe("350"); + expect(localStorage.getItem(SIDEBAR_WIDTH_STORAGE_KEY)).toBe("350"); + }); + it("rehydrates a valid stored width", () => { localStorage.setItem(SIDEBAR_WIDTH_STORAGE_KEY, "480"); render( diff --git a/src/context/sidebar/SidebarProvider.tsx b/src/context/sidebar/SidebarProvider.tsx index a148b3fe8..cf53f7db2 100644 --- a/src/context/sidebar/SidebarProvider.tsx +++ b/src/context/sidebar/SidebarProvider.tsx @@ -168,33 +168,21 @@ export function SidebarProvider({ // values (e.g. from a wider viewport, or a bad client) are ignored. if (stored === null) return; if (stored < min || stored > max) return; + // FIRST REAL WRITER WINS. Once a genuine user gesture (a drag through + // handleSetWidth) has recorded a width, widthHydratedRef is already true, + // and the late-arriving hydration read must NOT touch anything — not the + // rendered width AND not the remembered reopen width (lastOpenWidth). + // Previously `setLastOpenWidth(stored)` ran unconditionally here, so a GET + // that resolved just AFTER a drag overwrote the dragged size: the user + // dragged to B, the stale read stamped lastOpenWidth back to A, and the + // next reopen came back at A instead of B. A placeholder seed (seedWidth, + // used by AppShell's controlled-open effect on reload) does NOT set the + // flag, so the stored width still wins that race exactly once and reload + // restore keeps working. + if (widthHydratedRef.current) return; + widthHydratedRef.current = true; setLastOpenWidth(stored); - // Reconcile the fetched width into the RENDERED state, not just the - // reopen memory (`lastOpenWidth`). Without this the fetched width is - // stranded — it only surfaces on the NEXT open gesture (reopenWidth reads - // lastOpenWidth), which is why close-then-reopen restored it but a reload - // did not. - // - // The first time we apply, the fetched stored width overrides whatever is - // rendered — even a non-zero PLACEHOLDER seed. On reload AppShell's - // controlled-open effect synchronously seeds a half-viewport / floor width - // the instant the host's persisted open:true hydrates (lastOpenWidth is - // still 0 at that point, so reopenWidth() returns a placeholder, not the - // real saved width). That seed lands BEFORE this async GET resolves, so a - // plain `cur <= 0` guard would discard the real stored width. Overriding - // once on first hydrate makes the persisted width win that race. A genuine - // user drag landing before the fetch sets widthHydratedRef in - // handleSetWidth, so it is treated as already-hydrated and is NOT - // clobbered — first REAL writer wins. After the one-time hydrate, the - // functional updater seeds only from the default floor (`cur <= 0`). No - // render gate: width intentionally paints default-then-hydrate to avoid an - // SSR mismatch. - if (!widthHydratedRef.current) { - widthHydratedRef.current = true; - setSidebarWidth(stored); - } else { - setSidebarWidth((cur) => (cur <= 0 ? stored : cur)); - } + setSidebarWidth(stored); }; const persistence = persistenceRef.current; @@ -219,10 +207,11 @@ export function SidebarProvider({ apply(readPersistedWidth(persistKey, min, max)); }, []); - // Persist every meaningful open width; skip closed/collapsed-to-floor states - // so a close OR a collapse (width === minOpenWidth) doesn't wipe the - // remembered size. The injected server persistence wins; the legacy - // localStorage `persistKey` is written only when no persistence is wired. + // Persist a width. Called by handleSetWidth only for real open widths + // (>= minOpenWidth, including a drag to the floor); a close (0) is gated out + // upstream and a collapse never flows here (AppShell collapses via seedWidth). + // The injected persistence wins; the legacy localStorage `persistKey` is + // written only when no persistence is wired. const persist = useCallback( (width: number) => { const persistence = persistenceRef.current; @@ -252,16 +241,16 @@ export function SidebarProvider({ const handleSetWidth = useCallback( (width: number) => { setSidebarWidth(width); - // Strict `>`: a width of exactly `minOpenWidth` is the collapsed floor - // (AppShell collapses by setting the width to MIN_WIDTH), not a real open - // width — recording it would erase the user's chosen drag size and make - // reopen fall back to the default. Only widths strictly above the floor - // count as the remembered open width. - if (width > minOpenWidth) { - // A real user drag landing before the async fetch resolves is the - // authoritative width — mark hydrated so the late-arriving stored width - // does NOT overwrite it (first REAL writer wins). A placeholder seed at - // or below the floor never reaches here, so it can't claim hydration. + // `>=`, not `>`: minOpenWidth is the narrowest VALID OPEN width (the drag + // floor), so a user dragging all the way to it (width === minOpenWidth) is + // a deliberate choice and MUST be remembered. Only a CLOSE (width 0, below + // the floor) is excluded. The collapse button does NOT come through here — + // it uses `seedWidth`, so collapsing to the floor never overwrites the + // remembered open width. (The old strict `>` dropped a drag-to-minimum, + // leaving lastOpenWidth at the previous size, so reopen ignored it.) + if (width >= minOpenWidth) { + // A real user drag is the authoritative width — mark hydrated so a + // late-arriving stored width can't overwrite it (first REAL writer wins). widthHydratedRef.current = true; setLastOpenWidth(width); persist(width); diff --git a/src/hooks/agent-chat/useAgentTabs.ts b/src/hooks/agent-chat/useAgentTabs.ts index 09f92981c..b1b744c04 100644 --- a/src/hooks/agent-chat/useAgentTabs.ts +++ b/src/hooks/agent-chat/useAgentTabs.ts @@ -297,6 +297,15 @@ export interface UseAgentTabsResult { /** Sidebar open/closed — persisted, shared across hosts. */ open: boolean; setOpen: (open: boolean) => void; + /** Shared open-sidebar width (px) — the SAME persisted record as + * open/tabs/activeTabId. 0 = no saved width (host default). */ + width: number; + /** Persist a new open-sidebar width. Routes through the strip's single + * debounced writer so a width save can never race/clobber the open or tabs + * fields (and vice-versa) — there is exactly ONE writer for the record. The + * host wires this to the kit's width-persistence `set` so a drag-end folds + * width into the same PUT as the rest of the strip. */ + setWidth: (width: number) => void; /** Open tabs as conversation ids (conversation ids + local draft ids), * strip order. Map to titled ChatTabs with deriveTabTitles. */ tabs: string[]; @@ -329,9 +338,11 @@ export interface UseAgentTabsResult { * mutation is pending flush so it can't clobber newer local state. * * The persisted record (AgentSidebarState) also carries the shared - * open-sidebar `width`; the strip never MANAGES width (a host-side width - * persistence owns the read/write — see api-client-shared), but it preserves - * the hydrated width through every reducer so a tab PUT never drops it. + * open-sidebar `width`. The strip is the SINGLE writer of the whole record: + * `setWidth` folds a width change into the same debounced PUT as open/tabs, and + * every reducer preserves the hydrated width — so width and open/tabs can never + * race or clobber each other (the host's width-persistence `set` routes here + * instead of doing its own GET→merge→PUT). * * `persistence` and `resolveTabs` are read through refs — their identities * may change per render without retriggering fetches. @@ -546,6 +557,15 @@ export function useAgentTabs( (open: boolean) => mutate((s) => (s.open === open ? s : { ...s, open })), [mutate], ); + // Width is just another field of the one record — routing it through the same + // `mutate`/`schedulePut` makes the strip the SINGLE writer, so a width PUT + // carries the live open/tabs and an open PUT carries the live width. No second + // writer, no read-modify-write race (the old standalone width persister did a + // GET→merge→PUT that could clobber a freshly-written open). + const setWidth = useCallback( + (width: number) => mutate((s) => (s.width === width ? s : { ...s, width })), + [mutate], + ); const selectTab = useCallback((id: string) => mutate((s) => selectTabState(s, id)), [mutate]); const closeTab = useCallback( (id: string) => mutate((s) => closeTabState(s, id, freshDraft)), @@ -578,6 +598,8 @@ export function useAgentTabs( return { open: strip.open, setOpen, + width: strip.width, + setWidth, tabs, activeTabId: strip.activeTabId, hydrated,