diff --git a/apps/app/src/components/sidebar/ThreadRow.test.tsx b/apps/app/src/components/sidebar/ThreadRow.test.tsx index 977573937..68c2d6b90 100644 --- a/apps/app/src/components/sidebar/ThreadRow.test.tsx +++ b/apps/app/src/components/sidebar/ThreadRow.test.tsx @@ -224,7 +224,9 @@ function renderSplitThreadRow({ afterEach(() => { cleanup(); resetPluginThreadRowStatusesForTest(); + // The layout is tab-scoped, so it lands in both stores (createTabScopedStorage). window.localStorage.removeItem(SPLIT_LAYOUT_STORAGE_KEY); + window.sessionStorage.removeItem(SPLIT_LAYOUT_STORAGE_KEY); }); describe("ThreadRow", () => { diff --git a/apps/app/src/lib/browser-storage.ts b/apps/app/src/lib/browser-storage.ts index 5c25e3a1d..2ce0d3417 100644 --- a/apps/app/src/lib/browser-storage.ts +++ b/apps/app/src/lib/browser-storage.ts @@ -24,7 +24,7 @@ interface SyncStringStorage { ) => (() => void) | undefined; } -interface LocalStorageValueCodec { +interface StoredValueCodec { parse: (storedValue: string | null, initialValue: T) => T; serialize: (value: T) => string; } @@ -36,6 +36,13 @@ function getLocalStorage(): Storage | null { return window.localStorage; } +function getSessionStorage(): Storage | null { + if (typeof window === "undefined") { + return null; + } + return window.sessionStorage; +} + function subscribeToLocalStorageKey( key: string, callback: StoredValueListener, @@ -107,8 +114,42 @@ export function createBooleanPreferenceAtom( ); } +/** + * Storage for state that belongs to one tab rather than to the user, such as + * the split workspace layout. + * + * Reads prefer `sessionStorage` (per tab, survives that tab's reload) and fall + * back to `localStorage` so a newly opened tab still starts from the most + * recent arrangement. Writes go to both. There is deliberately no `storage` + * subscription: that event fires in *other* tabs, so subscribing would make one + * tab adopt another tab's value mid-session — the cross-tab thread bleed in + * issue #873. + */ +export function createTabScopedStorage( + codec: StoredValueCodec, +): SyncStorage { + return { + getItem: (key: string, initialValue: T) => { + // An empty-but-present tab value is a real value (a cleared layout + // serializes to ""), so only a missing key falls back to the seed. + const tabValue = getSessionStorage()?.getItem(key) ?? null; + const storedValue = tabValue ?? getLocalStorage()?.getItem(key) ?? null; + return codec.parse(storedValue, initialValue); + }, + setItem: (key: string, value: T) => { + const serialized = codec.serialize(value); + getSessionStorage()?.setItem(key, serialized); + getLocalStorage()?.setItem(key, serialized); + }, + removeItem: (key: string) => { + getSessionStorage()?.removeItem(key); + getLocalStorage()?.removeItem(key); + }, + }; +} + export function createLocalStorageSyncStorage( - codec: LocalStorageValueCodec, + codec: StoredValueCodec, ): SyncStorage { return { getItem: (key: string, initialValue: T) => diff --git a/apps/app/src/lib/split-layout/atoms.test.ts b/apps/app/src/lib/split-layout/atoms.test.ts index 6412bafbd..19117fa69 100644 --- a/apps/app/src/lib/split-layout/atoms.test.ts +++ b/apps/app/src/lib/split-layout/atoms.test.ts @@ -9,6 +9,7 @@ import { splitLayoutAtom, } from "./atoms"; import { countPanes, findPaneByThread, splitPane } from "./ops"; +import { serializeSplitLayout, SPLIT_LAYOUT_STORAGE_KEY } from "./persistence"; import type { SplitLayout } from "./types"; function singlePane(threadId: string): SplitLayout { @@ -33,6 +34,82 @@ function twoPanes(): SplitLayout { afterEach(() => { window.localStorage.clear(); + window.sessionStorage.clear(); +}); + +/** A write by another tab: same origin storage, then the cross-tab event. */ +function writeFromOtherTab(key: string, value: string): void { + const previous = window.localStorage.getItem(key); + window.localStorage.setItem(key, value); + window.dispatchEvent( + new StorageEvent("storage", { + key, + newValue: value, + oldValue: previous, + storageArea: window.localStorage, + }), + ); +} + +/** + * A fresh page load: atomWithStorage only re-reads storage when the atom is + * mounted, so subscribe first, exactly like a rendered component does. + */ +function hydrateLayoutOnLoad(): SplitLayout | null { + const store = createStore(); + const unsubscribe = store.sub(splitLayoutAtom, () => {}); + const layout = store.get(splitLayoutAtom); + unsubscribe(); + return layout; +} + +describe("tab-scoped workspace state", () => { + it("keeps this tab's panes when another tab opens a different thread", () => { + const store = createStore(); + // Mounting is what would install a cross-tab subscription. + const unsubscribe = store.sub(splitLayoutAtom, () => {}); + store.set(splitLayoutAtom, singlePane("thread-1")); + + writeFromOtherTab( + SPLIT_LAYOUT_STORAGE_KEY, + serializeSplitLayout(singlePane("thread-2")), + ); + + expect(store.get(splitLayoutAtom)).toEqual(singlePane("thread-1")); + unsubscribe(); + }); + + it("keeps this tab's maximized pane when another tab maximizes a different one", () => { + const store = createStore(); + const unsubscribe = store.sub(maximizedPaneIdAtom, () => {}); + store.set(splitLayoutAtom, twoPanes()); + store.set(maximizedPaneIdAtom, "pane-1"); + + writeFromOtherTab(MAXIMIZED_PANE_STORAGE_KEY, "pane-2"); + + expect(store.get(maximizedPaneIdAtom)).toBe("pane-1"); + unsubscribe(); + }); + + it("seeds a new tab from the last arrangement, then reloads its own", () => { + // A tab that has never had a layout starts from the shared seed. + window.localStorage.setItem( + SPLIT_LAYOUT_STORAGE_KEY, + serializeSplitLayout(twoPanes()), + ); + expect(countPanes(hydrateLayoutOnLoad()!.root)).toBe(2); + + // Once this tab owns a layout, its reload restores that one even though + // another tab wrote the shared key afterwards. + const store = createStore(); + store.set(splitLayoutAtom, singlePane("thread-3")); + window.localStorage.setItem( + SPLIT_LAYOUT_STORAGE_KEY, + serializeSplitLayout(singlePane("thread-9")), + ); + + expect(hydrateLayoutOnLoad()).toEqual(singlePane("thread-3")); + }); }); describe("closePanesForThreadsAtom", () => { diff --git a/apps/app/src/lib/split-layout/atoms.ts b/apps/app/src/lib/split-layout/atoms.ts index b11c761c3..37b279795 100644 --- a/apps/app/src/lib/split-layout/atoms.ts +++ b/apps/app/src/lib/split-layout/atoms.ts @@ -1,10 +1,6 @@ import { atom } from "jotai"; import { atomWithStorage } from "jotai/utils"; -import { - createNullableLocalStorageEnumStorage, - createLocalStorageSyncStorage, - type SyncStorage, -} from "@/lib/browser-storage"; +import { createTabScopedStorage, type SyncStorage } from "@/lib/browser-storage"; import type { ThreadRoutePathArgs } from "@/lib/route-paths"; import { findPane, listPanes, removePane } from "./ops"; import { @@ -15,7 +11,7 @@ import { import type { SplitLayout } from "./types"; function createSplitLayoutStorage(): SyncStorage { - return createLocalStorageSyncStorage({ + return createTabScopedStorage({ // A malformed or stale value deserializes to null, which the split area // reads as "seed a single pane from the current route". parse: (storedValue) => deserializeSplitLayout(storedValue), @@ -25,10 +21,15 @@ function createSplitLayoutStorage(): SyncStorage { /** * Global split layout, shared across projects like {@link - * sidebarCollapsedAtoms}. Null until the first thread view seeds a single pane - * from the route; persisted through the versioned split-layout codec so reload - * restores the arrangement (the URL's thread claims focus). The focused pane is - * carried inside {@link SplitLayout.focusedPaneId}, not a separate atom. + * sidebarCollapsedAtoms} but scoped to one tab. Null until the first thread + * view seeds a single pane from the route; persisted through the versioned + * split-layout codec so reload restores the arrangement (the URL's thread + * claims focus). The focused pane is carried inside {@link + * SplitLayout.focusedPaneId}, not a separate atom. + * + * Tab-scoped rather than cross-tab synced: which thread sits in which pane is a + * property of the window you are looking at, so a second tab must never adopt + * the first tab's panes (issue #873). See {@link createTabScopedStorage}. */ export const splitLayoutAtom = atomWithStorage( SPLIT_LAYOUT_STORAGE_KEY, @@ -39,20 +40,25 @@ export const splitLayoutAtom = atomWithStorage( export const MAXIMIZED_PANE_STORAGE_KEY = "bb.splitLayout.maximizedPaneId"; -function isPersistedPaneId(value: string): value is string { - return value.length > 0; -} - /** * The pane temporarily filling the split workspace. This is persisted beside, * rather than inside, the versioned split tree so maximizing never rewrites or * migrates the exact arrangement and sizes it will restore. SplitThreadArea * validates the id against the hydrated layout and clears stale values. + * + * Tab-scoped for the same reason as {@link splitLayoutAtom}: it names a pane in + * that tab's layout. */ export const maximizedPaneIdAtom = atomWithStorage( MAXIMIZED_PANE_STORAGE_KEY, null, - createNullableLocalStorageEnumStorage(isPersistedPaneId), + createTabScopedStorage({ + parse: (storedValue, initialValue) => + storedValue !== null && storedValue.length > 0 + ? storedValue + : initialValue, + serialize: (value) => value ?? "", + }), { getOnInit: true }, ); diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx index 137180b2b..02306e5ac 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx @@ -450,6 +450,7 @@ afterEach(() => { resetPluginSlotStoreForTest(); delete window.bbDesktop; window.localStorage.clear(); + window.sessionStorage.clear(); }); describe("SplitThreadArea", () => { @@ -1378,6 +1379,37 @@ describe("SplitThreadArea", () => { ).not.toBe(0); }); + it("ignores a layout written by another tab (issue #873)", async () => { + renderSplitArea({ path: threadPath("thr-a") }); + expect(await screen.findByTestId("pane-thr-a")).toBeTruthy(); + + // Another tab selects thr-b: same-origin localStorage write plus the + // `storage` event the browser delivers to every other tab. + const otherTabLayout = serializeSplitLayout({ + root: { type: "pane", paneId: "pane-1", content: threadContent("thr-b") }, + focusedPaneId: "pane-1", + }); + window.localStorage.setItem(SPLIT_LAYOUT_STORAGE_KEY, otherTabLayout); + fireEvent( + window, + new StorageEvent("storage", { + key: SPLIT_LAYOUT_STORAGE_KEY, + newValue: otherTabLayout, + storageArea: window.localStorage, + }), + ); + + // This tab keeps its own thread and URL; nothing bleeds across tabs. + await waitFor(() => { + expect(screen.getByTestId("location").textContent).toBe( + threadPath("thr-a"), + ); + }); + expect(screen.queryAllByTestId(/^pane-/)).toHaveLength(1); + expect(screen.getByTestId("pane-thr-a")).toBeTruthy(); + expect(screen.queryByTestId("pane-thr-b")).toBeNull(); + }); + it("falls back to a single pane from the route when persisted state is malformed", async () => { window.localStorage.setItem(SPLIT_LAYOUT_STORAGE_KEY, "not json"); renderSplitArea({ path: threadPath("thr-a") });