From 7e782551338fdcd438f25c106751ac58ef0ab1c6 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Sun, 3 May 2026 15:29:59 -0700 Subject: [PATCH] Revert "feat(desktop): workspace pane store registry (v2 PR 3) (#3940)" This reverts commit f7db05ce8c33e5f9937326c3f1469914654a2aa8. --- .../addLaunchPanes.test.ts | 162 ------------ .../workspace-pane-registry/addLaunchPanes.ts | 111 --------- .../lib/workspace-pane-registry/index.ts | 7 - .../workspace-pane-registry.test.ts | 211 ---------------- .../workspace-pane-registry.ts | 234 ------------------ .../useV2WorkspacePaneLayout.ts | 87 ++++++- .../useDashboardSidebarState.ts | 2 - .../CollectionsProvider.tsx | 27 +- .../Workspace/components/Tab/Tab.tsx | 7 - plans/20260430-pane-store-registry-pr3.md | 149 ----------- 10 files changed, 85 insertions(+), 912 deletions(-) delete mode 100644 apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.test.ts delete mode 100644 apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.ts delete mode 100644 apps/desktop/src/renderer/lib/workspace-pane-registry/index.ts delete mode 100644 apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.test.ts delete mode 100644 apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.ts delete mode 100644 plans/20260430-pane-store-registry-pr3.md diff --git a/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.test.ts b/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.test.ts deleted file mode 100644 index 2e696711c2d..00000000000 --- a/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.test.ts +++ /dev/null @@ -1,162 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; -import { - createCollection, - localStorageCollectionOptions, -} from "@tanstack/react-db"; -import type { - ChatPaneData, - TerminalPaneData, -} from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types"; -import { - type WorkspaceLocalStateRow, - workspaceLocalStateSchema, -} from "renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema"; -import { addLaunchPanes } from "./addLaunchPanes"; -// Imported from the impl file rather than the barrel — the test-reset -// helper is intentionally not part of the public API surface. -import { - __resetWorkspacePaneRegistryForTests, - getOrCreateWorkspacePaneStore, - initWorkspacePaneRegistry, -} from "./workspace-pane-registry"; - -const PROJECT_ID = crypto.randomUUID(); -const WORKSPACE_ID = crypto.randomUUID(); - -let collection: ReturnType; - -function makeCollection() { - if (typeof globalThis.localStorage === "undefined") { - const store = new Map(); - (globalThis as { localStorage: Storage }).localStorage = { - getItem: (k) => store.get(k) ?? null, - setItem: (k, v) => { - store.set(k, v); - }, - removeItem: (k) => { - store.delete(k); - }, - clear: () => store.clear(), - key: () => null, - length: 0, - }; - } - return createCollection( - localStorageCollectionOptions({ - id: `test-v2-workspace-local-state-${Math.random()}`, - storageKey: `test-v2-workspace-local-state-${Math.random()}`, - schema: workspaceLocalStateSchema, - getKey: (item) => item.workspaceId, - }), - ); -} - -function seedRow( - workspaceId: string, - overrides: Partial = {}, -): void { - collection.insert({ - workspaceId, - createdAt: new Date(), - sidebarState: { - projectId: PROJECT_ID, - tabOrder: 0, - sectionId: null, - changesFilter: { kind: "all" }, - activeTab: "changes", - isHidden: false, - }, - paneLayout: { version: 1, tabs: [], activeTabId: null }, - viewedFiles: [], - recentlyViewedFiles: [], - ...overrides, - }); -} - -beforeEach(() => { - collection = makeCollection(); - initWorkspacePaneRegistry({ v2WorkspaceLocalState: collection }); - seedRow(WORKSPACE_ID); -}); - -afterEach(() => { - __resetWorkspacePaneRegistryForTests(); -}); - -describe("addLaunchPanes", () => { - it("is a no-op for an empty array", () => { - addLaunchPanes(WORKSPACE_ID, []); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(0); - }); - - it("adds a tab per terminal launch with no initialCommand", () => { - addLaunchPanes(WORKSPACE_ID, [ - { kind: "terminal", terminalId: "t-1", label: "Codex" }, - { kind: "terminal", terminalId: "t-2" }, - ]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(2); - const all = store.getState().tabs.flatMap((t) => Object.values(t.panes)); - expect(all).toHaveLength(2); - for (const pane of all) { - expect(pane.kind).toBe("terminal"); - const data = pane.data as TerminalPaneData; - expect(data.terminalId).toMatch(/^t-/); - expect(data.initialCommand).toBeUndefined(); - } - }); - - it("adds a tab per chat launch", () => { - addLaunchPanes(WORKSPACE_ID, [ - { kind: "chat", chatSessionId: "s-1", label: "Claude" }, - ]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - const tab = store.getState().tabs[0]; - expect(tab).toBeDefined(); - const pane = Object.values(tab?.panes)[0]; - expect(pane?.kind).toBe("chat"); - const data = pane?.data as ChatPaneData; - expect(data.sessionId).toBe("s-1"); - }); - - it("dedupes by terminalId on repeat calls", () => { - addLaunchPanes(WORKSPACE_ID, [{ kind: "terminal", terminalId: "t-1" }]); - addLaunchPanes(WORKSPACE_ID, [{ kind: "terminal", terminalId: "t-1" }]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(1); - }); - - it("dedupes by chatSessionId on repeat calls", () => { - addLaunchPanes(WORKSPACE_ID, [{ kind: "chat", chatSessionId: "s-1" }]); - addLaunchPanes(WORKSPACE_ID, [{ kind: "chat", chatSessionId: "s-1" }]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(1); - }); - - it("focuses the existing pane when called with a duplicate id", () => { - addLaunchPanes(WORKSPACE_ID, [ - { kind: "terminal", terminalId: "t-1" }, - { kind: "terminal", terminalId: "t-2" }, - ]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - const firstTabId = store.getState().tabs[0]?.id; - if (!firstTabId) throw new Error("expected at least one tab to be added"); - - addLaunchPanes(WORKSPACE_ID, [{ kind: "terminal", terminalId: "t-1" }]); - expect(store.getState().activeTabId).toBe(firstTabId); - }); - - it("handles a mixed terminal + chat batch", () => { - addLaunchPanes(WORKSPACE_ID, [ - { kind: "terminal", terminalId: "t-1" }, - { kind: "chat", chatSessionId: "s-1" }, - ]); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - const kinds = store - .getState() - .tabs.flatMap((t) => Object.values(t.panes).map((p) => p.kind)) - .sort(); - expect(kinds).toEqual(["chat", "terminal"]); - }); -}); diff --git a/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.ts b/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.ts deleted file mode 100644 index 40c4d3491a8..00000000000 --- a/apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.ts +++ /dev/null @@ -1,111 +0,0 @@ -import type { Pane } from "@superset/panes"; -import type { - ChatPaneData, - PaneViewerData, - TerminalPaneData, -} from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types"; -import { getOrCreateWorkspacePaneStore } from "./workspace-pane-registry"; - -export type LaunchPaneInput = - | { kind: "terminal"; terminalId: string; label?: string } - | { kind: "chat"; chatSessionId: string; label?: string }; - -interface PaneLocation { - tabId: string; - pane: Pane; -} - -/** - * Add panes to a workspace's pane store for sessions the host already - * started (e.g. the result of `workspace.create()`). Each launch - * becomes its own tab; calling twice with the same id dedupes and - * refocuses. Safe to call before the workspace route mounts — the - * registry takes care of persisting the in-memory tabs to - * `v2WorkspaceLocalState` once `ensureWorkspaceInSidebar` inserts the - * row. - * - * Attach-only: terminal launches do not carry `initialCommand`. The - * legacy renderer-mints-id + embedded-command flow used by - * `useV2PresetExecution` and the pending-row launch path is unchanged - * and uses `addTab` directly. - */ -export function addLaunchPanes( - workspaceId: string, - launches: LaunchPaneInput[], -): void { - if (launches.length === 0) return; - - const store = getOrCreateWorkspacePaneStore(workspaceId); - let lastFocusedLocation: PaneLocation | null = null; - - for (const launch of launches) { - const existing = findExistingPane(store.getState(), launch); - if (existing) { - lastFocusedLocation = existing; - continue; - } - - if (launch.kind === "terminal") { - const data: TerminalPaneData = { terminalId: launch.terminalId }; - store.getState().addTab({ - titleOverride: launch.label, - panes: [ - { - kind: "terminal", - titleOverride: launch.label, - data: data as PaneViewerData, - }, - ], - }); - } else { - const data: ChatPaneData = { sessionId: launch.chatSessionId }; - store.getState().addTab({ - titleOverride: launch.label, - panes: [ - { - kind: "chat", - titleOverride: launch.label, - data: data as PaneViewerData, - }, - ], - }); - } - - // addTab focuses the new tab itself (sets activeTabId). Capture the - // landing pane so we can re-focus on the dedupe-only path below if - // the dupe was the last entry. - const newLocation = findExistingPane(store.getState(), launch); - if (newLocation) lastFocusedLocation = newLocation; - } - - if (lastFocusedLocation) { - store.getState().setActivePane({ - tabId: lastFocusedLocation.tabId, - paneId: lastFocusedLocation.pane.id, - }); - } -} - -function findExistingPane( - state: ReturnType< - ReturnType["getState"] - >, - launch: LaunchPaneInput, -): PaneLocation | null { - for (const tab of state.tabs) { - for (const pane of Object.values(tab.panes)) { - if (launch.kind === "terminal" && pane.kind === "terminal") { - const data = pane.data as TerminalPaneData; - if (data.terminalId === launch.terminalId) { - return { tabId: tab.id, pane }; - } - } else if (launch.kind === "chat" && pane.kind === "chat") { - const data = pane.data as ChatPaneData; - if (data.sessionId === launch.chatSessionId) { - return { tabId: tab.id, pane }; - } - } - } - } - return null; -} diff --git a/apps/desktop/src/renderer/lib/workspace-pane-registry/index.ts b/apps/desktop/src/renderer/lib/workspace-pane-registry/index.ts deleted file mode 100644 index e71f6574130..00000000000 --- a/apps/desktop/src/renderer/lib/workspace-pane-registry/index.ts +++ /dev/null @@ -1,7 +0,0 @@ -export { addLaunchPanes, type LaunchPaneInput } from "./addLaunchPanes"; -export { - dropWorkspacePaneStore, - getOrCreateWorkspacePaneStore, - initWorkspacePaneRegistry, - type WorkspacePaneRegistryDeps, -} from "./workspace-pane-registry"; diff --git a/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.test.ts b/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.test.ts deleted file mode 100644 index 3006380cce6..00000000000 --- a/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.test.ts +++ /dev/null @@ -1,211 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; -import { - createCollection, - localStorageCollectionOptions, -} from "@tanstack/react-db"; -import { - type WorkspaceLocalStateRow, - workspaceLocalStateSchema, -} from "renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema"; -// Imported from the impl file rather than the barrel — the test-reset -// helper is intentionally not part of the public API surface. -import { - __resetWorkspacePaneRegistryForTests, - getOrCreateWorkspacePaneStore, - initWorkspacePaneRegistry, -} from "./workspace-pane-registry"; - -let collection: ReturnType; - -const PROJECT_ID = crypto.randomUUID(); -const WORKSPACE_ID = crypto.randomUUID(); -const OTHER_WORKSPACE_ID = crypto.randomUUID(); - -function makeCollection() { - // localStorage isn't available in bun:test by default; provide a shim. - if (typeof globalThis.localStorage === "undefined") { - const store = new Map(); - (globalThis as { localStorage: Storage }).localStorage = { - getItem: (k) => store.get(k) ?? null, - setItem: (k, v) => { - store.set(k, v); - }, - removeItem: (k) => { - store.delete(k); - }, - clear: () => store.clear(), - key: () => null, - length: 0, - }; - } - return createCollection( - localStorageCollectionOptions({ - id: `test-v2-workspace-local-state-${Math.random()}`, - storageKey: `test-v2-workspace-local-state-${Math.random()}`, - schema: workspaceLocalStateSchema, - getKey: (item) => item.workspaceId, - }), - ); -} - -function seedRow( - workspaceId: string, - overrides: Partial = {}, -): void { - collection.insert({ - workspaceId, - createdAt: new Date(), - sidebarState: { - projectId: PROJECT_ID, - tabOrder: 0, - sectionId: null, - changesFilter: { kind: "all" }, - activeTab: "changes", - isHidden: false, - }, - paneLayout: { version: 1, tabs: [], activeTabId: null }, - viewedFiles: [], - recentlyViewedFiles: [], - ...overrides, - }); -} - -beforeEach(() => { - collection = makeCollection(); - initWorkspacePaneRegistry({ v2WorkspaceLocalState: collection }); -}); - -afterEach(() => { - __resetWorkspacePaneRegistryForTests(); -}); - -describe("workspace-pane-registry", () => { - it("returns the same store for the same workspaceId", () => { - const a = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - const b = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(a).toBe(b); - }); - - it("returns different stores for different workspaceIds", () => { - const a = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - const b = getOrCreateWorkspacePaneStore(OTHER_WORKSPACE_ID); - expect(a).not.toBe(b); - }); - - it("seeds the store from a pre-existing row's paneLayout", () => { - seedRow(WORKSPACE_ID, { - paneLayout: { - version: 1, - tabs: [ - { - id: "tab-1", - titleOverride: undefined, - createdAt: 0, - activePaneId: "pane-1", - layout: { type: "pane", paneId: "pane-1" }, - panes: { - "pane-1": { - id: "pane-1", - kind: "terminal", - data: { terminalId: "t1" }, - }, - }, - }, - ], - activeTabId: "tab-1", - }, - }); - - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(1); - expect(store.getState().activeTabId).toBe("tab-1"); - }); - - it("writes store changes back to the row when the row exists", () => { - seedRow(WORKSPACE_ID); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - - store.getState().addTab({ - panes: [{ kind: "terminal", data: { terminalId: "t1" } }], - }); - - const row = collection.get(WORKSPACE_ID); - expect(row?.paneLayout.tabs).toHaveLength(1); - }); - - it("does not throw when writing back with no row present yet", () => { - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - // addTab fires the subscriber; should silently skip the write because - // no row exists. State is preserved in-memory. - expect(() => - store.getState().addTab({ - panes: [{ kind: "terminal", data: { terminalId: "t1" } }], - }), - ).not.toThrow(); - expect(store.getState().tabs).toHaveLength(1); - }); - - it("persists pre-mount panes when the row is later inserted with empty layout", async () => { - // Simulates the addLaunchPanes-before-route-mount flow: tabs are - // added to the store before the workspace row exists, then - // `ensureWorkspaceInSidebar` inserts the row with EMPTY paneLayout. - // The registry must (a) not wipe the in-memory tabs, and (b) push - // the store's state into the freshly-inserted row so the data - // survives an immediate app restart. - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - store.getState().addTab({ - panes: [{ kind: "terminal", data: { terminalId: "t1" } }], - }); - expect(store.getState().tabs).toHaveLength(1); - - seedRow(WORKSPACE_ID); // inserts with EMPTY paneLayout - await new Promise((r) => setTimeout(r, 0)); - - // Store still has the pre-mount tab. - expect(store.getState().tabs).toHaveLength(1); - // Row has been updated with the pre-mount tab too. - const row = collection.get(WORKSPACE_ID); - expect(row?.paneLayout.tabs).toHaveLength(1); - }); - - it("pushes external row updates into the store", async () => { - seedRow(WORKSPACE_ID); - const store = getOrCreateWorkspacePaneStore(WORKSPACE_ID); - expect(store.getState().tabs).toHaveLength(0); - - collection.update(WORKSPACE_ID, (draft) => { - draft.paneLayout = { - version: 1, - tabs: [ - { - id: "tab-2", - titleOverride: undefined, - createdAt: 0, - activePaneId: "pane-2", - layout: { type: "pane", paneId: "pane-2" }, - panes: { - "pane-2": { - id: "pane-2", - kind: "chat", - data: { sessionId: "s1" }, - }, - }, - }, - ], - activeTabId: "tab-2", - }; - }); - - // Allow the subscribe handler to flush. - await new Promise((r) => setTimeout(r, 0)); - expect(store.getState().tabs).toHaveLength(1); - expect(store.getState().activeTabId).toBe("tab-2"); - }); - - it("throws if used before init", () => { - __resetWorkspacePaneRegistryForTests(); - expect(() => getOrCreateWorkspacePaneStore(WORKSPACE_ID)).toThrow( - /not initialized/, - ); - }); -}); diff --git a/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.ts b/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.ts deleted file mode 100644 index 008fae38d13..00000000000 --- a/apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.ts +++ /dev/null @@ -1,234 +0,0 @@ -import { - createWorkspaceStore, - type WorkspaceState, - type WorkspaceStore, -} from "@superset/panes"; -import type { - Collection, - LocalStorageCollectionUtils, -} from "@tanstack/react-db"; -import type { PaneViewerData } from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types"; -import type { - WorkspaceLocalStateRow, - workspaceLocalStateSchema, -} from "renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema"; -import type { z } from "zod"; -import type { StoreApi } from "zustand/vanilla"; - -type V2WorkspaceLocalStateCollection = Collection< - WorkspaceLocalStateRow, - string, - LocalStorageCollectionUtils, - typeof workspaceLocalStateSchema, - z.input ->; - -export interface WorkspacePaneRegistryDeps { - v2WorkspaceLocalState: V2WorkspaceLocalStateCollection; -} - -interface RegistryEntry { - store: StoreApi>; - unsubscribeStore: () => void; - unsubscribeCollection: () => void; -} - -const EMPTY_STATE: WorkspaceState = { - version: 1, - tabs: [], - activeTabId: null, -}; - -let deps: WorkspacePaneRegistryDeps | null = null; -const registry = new Map(); - -/** - * Deterministic JSON serialization with deep key sorting. Used to - * compare store state against row state without false-positive - * mismatches when the two paths produce structurally equal objects - * but in different key orders (Immer `draft.paneLayout = ...` writes - * via the collection do not preserve insertion order). - */ -function getSnapshot(value: unknown): string { - return JSON.stringify(deepSortKeys(value)); -} - -function deepSortKeys(value: unknown): unknown { - if (Array.isArray(value)) return value.map(deepSortKeys); - if (value && typeof value === "object") { - const sorted: Record = {}; - for (const k of Object.keys(value as Record).sort()) { - sorted[k] = deepSortKeys((value as Record)[k]); - } - return sorted; - } - return value; -} - -/** - * Wire the registry to the active org's collections. Call once at app - * boot, after `CollectionsProvider` resolves an active organization. - * - * Calling this with a new `v2WorkspaceLocalState` collection instance - * (e.g. after switching organizations) drops every existing store and - * re-initializes against the new collection. Calling with the same - * collection instance is a no-op — important because the call site - * passes a fresh wrapper object each time, but the underlying - * collection is what determines org identity. Without the - * instance-level check, an accidental memo recomputation would silently - * drop every live store. - */ -export function initWorkspacePaneRegistry( - nextDeps: WorkspacePaneRegistryDeps, -): void { - if (deps && deps.v2WorkspaceLocalState !== nextDeps.v2WorkspaceLocalState) { - for (const entry of registry.values()) { - entry.unsubscribeStore(); - entry.unsubscribeCollection(); - } - registry.clear(); - } - deps = nextDeps; -} - -/** - * Get the pane store for `workspaceId`, creating + persisting-wiring it - * on first call. Subsequent calls return the same store instance. - * - * Persistence: on first creation, the store is seeded from the matching - * `v2WorkspaceLocalState` row (if present) and then bidirectionally - * synced — store changes write back to the row (when it exists), and - * external row changes push into the store. A snapshot guard prevents - * the two from echoing each other. - * - * If the row doesn't exist yet, the write-back is silently skipped - * (the row is normally inserted by `ensureWorkspaceInSidebar` on route - * mount). Once the row appears, the next store change propagates. - */ -export function getOrCreateWorkspacePaneStore( - workspaceId: string, -): StoreApi> { - if (!deps) { - throw new Error( - "workspace-pane-registry not initialized — call initWorkspacePaneRegistry first", - ); - } - const activeDeps = deps; - - const existing = registry.get(workspaceId); - if (existing) return existing.store; - - const initialRow = activeDeps.v2WorkspaceLocalState.get(workspaceId); - const initialPaneLayout = - (initialRow?.paneLayout as WorkspaceState | undefined) ?? - EMPTY_STATE; - - const store = createWorkspaceStore({ - initialState: initialPaneLayout, - }); - let lastSyncedSnapshot = getSnapshot(initialPaneLayout); - - const unsubscribeStore = store.subscribe((next) => { - const nextSnapshot = getSnapshot({ - version: next.version, - tabs: next.tabs, - activeTabId: next.activeTabId, - }); - if (nextSnapshot === lastSyncedSnapshot) return; - if (!activeDeps.v2WorkspaceLocalState.get(workspaceId)) { - // Row not present yet (pre-mount or pre-migration). The next - // change after the row is inserted will sync. - return; - } - activeDeps.v2WorkspaceLocalState.update(workspaceId, (draft) => { - draft.paneLayout = { - version: next.version, - tabs: next.tabs, - activeTabId: next.activeTabId, - }; - }); - lastSyncedSnapshot = nextSnapshot; - }); - - const subscription = activeDeps.v2WorkspaceLocalState.subscribeChanges( - (changes) => { - for (const change of changes) { - if (change.key !== workspaceId) continue; - if (change.type === "delete") continue; - const layout = change.value?.paneLayout as - | WorkspaceState - | undefined; - if (!layout) continue; - const incoming = getSnapshot(layout); - const currentState = store.getState(); - const storeSnapshot = getSnapshot({ - version: currentState.version, - tabs: currentState.tabs, - activeTabId: currentState.activeTabId, - }); - // Already in sync. (This branch also catches Tanstack DB's - // initial-state `insert` events whose value matches what we - // seeded from when the store was created.) - if (incoming === storeSnapshot) { - lastSyncedSnapshot = incoming; - continue; - } - // Three-way reconciliation: incoming (row), storeSnapshot - // (memory), and lastSyncedSnapshot (last seen agreement). - // - // If storeSnapshot !== lastSyncedSnapshot, the store has - // unsynced mutations the row hasn't seen yet — typically - // addLaunchPanes ran before the row existed, then - // ensureWorkspaceInSidebar inserted the row with EMPTY - // paneLayout. Push the store back so those panes persist. - // - // Otherwise, the row diverges from the store while the store - // is still in sync with what we last wrote — the row was - // modified externally (migration, future cross-tab sync). - // Pull the row into the store. - if (storeSnapshot !== lastSyncedSnapshot) { - activeDeps.v2WorkspaceLocalState.update(workspaceId, (draft) => { - draft.paneLayout = { - version: currentState.version, - tabs: currentState.tabs, - activeTabId: currentState.activeTabId, - }; - }); - lastSyncedSnapshot = storeSnapshot; - continue; - } - lastSyncedSnapshot = incoming; - store.getState().replaceState(layout); - } - }, - ); - - registry.set(workspaceId, { - store, - unsubscribeStore, - unsubscribeCollection: () => subscription.unsubscribe(), - }); - return store; -} - -/** - * Drop the store for `workspaceId` and unsubscribe its sync. Called - * when a workspace is removed; safe to call when no entry exists. - */ -export function dropWorkspacePaneStore(workspaceId: string): void { - const entry = registry.get(workspaceId); - if (!entry) return; - entry.unsubscribeStore(); - entry.unsubscribeCollection(); - registry.delete(workspaceId); -} - -/** Test-only: tear down all stores and clear the deps. */ -export function __resetWorkspacePaneRegistryForTests(): void { - for (const entry of registry.values()) { - entry.unsubscribeStore(); - entry.unsubscribeCollection(); - } - registry.clear(); - deps = null; -} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts index c7c6ae4b9f0..36da8ff40c6 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts @@ -1,6 +1,20 @@ -import { useEffect, useState } from "react"; -import { getOrCreateWorkspacePaneStore } from "renderer/lib/workspace-pane-registry"; +import { createWorkspaceStore, type WorkspaceState } from "@superset/panes"; +import { eq } from "@tanstack/db"; +import { useLiveQuery } from "@tanstack/react-db"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useDashboardSidebarState } from "renderer/routes/_authenticated/hooks/useDashboardSidebarState"; +import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider"; +import type { PaneViewerData } from "../../types"; + +const EMPTY_STATE: WorkspaceState = { + version: 1, + tabs: [], + activeTabId: null, +}; + +function getSnapshot(state: WorkspaceState): string { + return JSON.stringify(state); +} interface UseV2WorkspacePaneLayoutParams { projectId: string; @@ -11,15 +25,76 @@ export function useV2WorkspacePaneLayout({ projectId, workspaceId, }: UseV2WorkspacePaneLayoutParams) { + const collections = useCollections(); const { ensureWorkspaceInSidebar } = useDashboardSidebarState(); - // Pull from the registry — same instance the create flow / addLaunchPanes - // write to. Persistence wiring lives in the registry; this hook just - // surfaces the store to the route and keeps the sidebar entry alive. - const [store] = useState(() => getOrCreateWorkspacePaneStore(workspaceId)); + const [store] = useState(() => + createWorkspaceStore({ + initialState: EMPTY_STATE, + }), + ); + const lastSyncedSnapshotRef = useRef(getSnapshot(EMPTY_STATE)); + + const { data: localWorkspaceRows = [] } = useLiveQuery( + (query) => + query + .from({ v2WorkspaceLocalState: collections.v2WorkspaceLocalState }) + .where(({ v2WorkspaceLocalState }) => + eq(v2WorkspaceLocalState.workspaceId, workspaceId), + ), + [collections, workspaceId], + ); + const localWorkspaceState = localWorkspaceRows[0] ?? null; + const persistedPaneLayout = useMemo( + () => + (localWorkspaceState?.paneLayout as + | WorkspaceState + | undefined) ?? EMPTY_STATE, + [localWorkspaceState], + ); useEffect(() => { ensureWorkspaceInSidebar(workspaceId, projectId); }, [ensureWorkspaceInSidebar, projectId, workspaceId]); + useEffect(() => { + const nextSnapshot = getSnapshot(persistedPaneLayout); + if (nextSnapshot === lastSyncedSnapshotRef.current) { + return; + } + + lastSyncedSnapshotRef.current = nextSnapshot; + store.getState().replaceState(persistedPaneLayout); + }, [persistedPaneLayout, store]); + + useEffect(() => { + const unsubscribe = store.subscribe((nextStore) => { + const nextSnapshot = getSnapshot({ + version: nextStore.version, + tabs: nextStore.tabs, + activeTabId: nextStore.activeTabId, + }); + if (nextSnapshot === lastSyncedSnapshotRef.current) { + return; + } + + if (!collections.v2WorkspaceLocalState.get(workspaceId)) { + return; + } + + collections.v2WorkspaceLocalState.update(workspaceId, (draft) => { + draft.paneLayout = { + version: nextStore.version, + tabs: nextStore.tabs, + activeTabId: nextStore.activeTabId, + }; + }); + lastSyncedSnapshotRef.current = nextSnapshot; + }); + + return () => { + unsubscribe(); + }; + }, [collections, store, workspaceId]); + return { store }; } diff --git a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts index 792d84e6940..108ea0afb1f 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts @@ -1,7 +1,6 @@ import type { Pane, WorkspaceState } from "@superset/panes"; import { useCallback } from "react"; import { terminalRuntimeRegistry } from "renderer/lib/terminal/terminal-runtime-registry"; -import { dropWorkspacePaneStore } from "renderer/lib/workspace-pane-registry"; import { browserRuntimeRegistry } from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/browserRuntimeRegistry"; import { extractPaneIds, @@ -463,7 +462,6 @@ export function useDashboardSidebarState() { if (!workspace) return; cleanupWorkspacePaneRuntimes([workspace]); collections.v2WorkspaceLocalState.delete(workspaceId); - dropWorkspacePaneStore(workspaceId); }, [collections], ); diff --git a/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx b/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx index 4c24a9bad18..f5562295086 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/CollectionsProvider.tsx @@ -9,7 +9,6 @@ import { } from "react"; import { env } from "renderer/env.renderer"; import { authClient } from "renderer/lib/auth-client"; -import { initWorkspacePaneRegistry } from "renderer/lib/workspace-pane-registry"; import { MOCK_ORG_ID } from "shared/constants"; import { getCollections, preloadCollections } from "./collections"; @@ -57,28 +56,10 @@ export function CollectionsProvider({ children }: { children: ReactNode }) { preloadActiveOrganizationCollections(activeOrganizationId); }, [activeOrganizationId]); - // Wire (or rewire on org switch) the workspace pane registry against - // the active org's v2WorkspaceLocalState collection synchronously, so - // callers of getOrCreateWorkspacePaneStore — including the workspace - // route's `useState(() => ...)` initializer — see an initialized - // registry before they run. - // - // Side effects in `useMemo` are not React-blessed (the docs reserve - // the right to recompute or discard memo work). The synchrony - // requirement here can't be satisfied by `useEffect`, which runs - // after render commits. Mitigation: `initWorkspacePaneRegistry` - // keys teardown on the `v2WorkspaceLocalState` *instance*, so any - // recomputation that happens to receive the same collection is a - // no-op rather than a state-clearing event. The only real teardown - // is on org switch, when the underlying collection actually changes. - const collections = useMemo(() => { - if (!activeOrganizationId) return null; - const next = getCollections(activeOrganizationId); - initWorkspacePaneRegistry({ - v2WorkspaceLocalState: next.v2WorkspaceLocalState, - }); - return next; - }, [activeOrganizationId]); + const collections = useMemo( + () => (activeOrganizationId ? getCollections(activeOrganizationId) : null), + [activeOrganizationId], + ); const contextValue = useMemo( () => (collections ? { ...collections, switchOrganization } : null), diff --git a/packages/panes/src/react/components/Workspace/components/Tab/Tab.tsx b/packages/panes/src/react/components/Workspace/components/Tab/Tab.tsx index f4d26ca75c1..7a8065bd18e 100644 --- a/packages/panes/src/react/components/Workspace/components/Tab/Tab.tsx +++ b/packages/panes/src/react/components/Workspace/components/Tab/Tab.tsx @@ -148,15 +148,8 @@ function LayoutNodeView({ const pane = tab.panes[node.paneId]; if (!pane) return null; - // key={pane.id} so React reconciles Pane instances by identity rather - // than position. Without this, switching the active tab to one that - // has a Pane in the same layout slot reuses the previous Pane's - // component instance — its hooks (notably `useRef`s capturing initial - // pane data like initialCommand) keep stale values from the prior - // pane and renderers like TerminalPane never see the new data. return ( createWorkspaceStore({...}))`. That has three real consequences: - -1. **`workspace.create()` (PR 4) returns a list of already-started sessions** (`{ kind: "terminal"; terminalId }` / `{ kind: "chat"; chatSessionId }`). The renderer needs to write panes for those existing sessions, not generate new ones. Today there's no entry point that can reach the pane store before mount, so the only way to communicate launch info from creator to consumer is the `pendingWorkspaces.terminalLaunch` / `chatLaunch` side-channel. -2. **The pending-row side-channel exists only because of this constraint** — it's state that should be a function argument. -3. **Mount-as-trigger is race-prone.** `useConsumePendingLaunch` carries a `consumedRef` set keyed by `${pendingId}:terminal|chat` to dedupe across effect re-runs, which is the kind of thing you only need when you've conflated two responsibilities. - -After this PR, `addLaunchPanes(workspaceId, launches)` can be called from anywhere — modal, `workspace.create()`'s caller, automations, the CLI's renderer side. The route, when it mounts, just reads from the same store the registry is already holding. - -## Architecture - -### Registry - -A module-level singleton at `apps/desktop/src/renderer/lib/workspace-pane-registry/`. Same pattern as the existing `terminalRuntimeRegistry` in `apps/desktop/src/renderer/lib/terminal/`. - -```ts -// boot — wired into app startup once a collections instance exists -initWorkspacePaneRegistry(collections); - -// callers -const store = getOrCreateWorkspacePaneStore(workspaceId); -addLaunchPanes(workspaceId, launches); -``` - -`initWorkspacePaneRegistry(collections)` must be called at app boot before any caller. The registry holds the collections instance internally so `getOrCreateWorkspacePaneStore` doesn't need it as an arg. - -### Persistence - -The registry **owns the persistence sync**, not the route hook. When a store is created, the registry: - -- Reads the persisted `paneLayout` from `collections.v2WorkspaceLocalState` (or seeds with `EMPTY_STATE` if no row yet) and calls `store.replaceState(...)`. -- Subscribes to the store and writes back to `v2WorkspaceLocalState` on every change (with the same `getSnapshot` debounce currently in `useV2WorkspacePaneLayout`). -- Subscribes to `v2WorkspaceLocalState` row updates and pushes them into the store. - -Why owned by the registry, not the route hook: - -- We expect flows that **add panes without navigating** (background create, automations writing into a not-yet-visible workspace's pane state, pre-warming). If sync only happens while the route is mounted, those panes silently fail to persist — recoverable but bad UX. -- Single source of truth: store is always in sync regardless of caller. -- Costs (registry imports collections; novel pattern in this codebase compared to `terminalRuntimeRegistry`) are paid once at boot. - -The route hook becomes a thin wrapper: - -```ts -export function useV2WorkspacePaneLayout({ projectId, workspaceId }) { - const { ensureWorkspaceInSidebar } = useDashboardSidebarState(); - const store = getOrCreateWorkspacePaneStore(workspaceId); - useEffect(() => { - ensureWorkspaceInSidebar(workspaceId, projectId); - }, [ensureWorkspaceInSidebar, projectId, workspaceId]); - return { store }; -} -``` - -### `addLaunchPanes(workspaceId, launches)` - -```ts -type LaunchResult = - | { kind: "terminal"; terminalId: string; label?: string } - | { kind: "chat"; chatSessionId: string; label?: string }; - -addLaunchPanes(workspaceId: string, launches: LaunchResult[]): void; -``` - -Behavior: - -- Gets or creates the store for `workspaceId`. -- For each launch, dedupes by id against existing panes in the store. If a pane with the same `terminalId` (terminal) or `sessionId` (chat) already exists, focus it instead of adding a duplicate. -- For new launches, calls `store.getState().addTab(...)` with the right pane data. -- Focuses the last added (or matched) pane. - -`addLaunchPanes` is **attach-only**. It does not carry an `initialCommand`. The host has already started the underlying session; the pane just attaches. - -### `TerminalPaneData.initialCommand` becomes optional - -Today: `{ terminalId: string; initialCommand: string }`. -After: `{ terminalId: string; initialCommand?: string }`. - -- Existing callers (`useConsumePendingLaunch`, `useV2PresetExecution`) keep sending `initialCommand` and behavior stays identical. -- `addLaunchPanes` omits `initialCommand`. The terminal pane component, on connect, branches: if `initialCommand` is defined, write it + enter; otherwise, just attach. - -This is a one-line type change plus a small branch in `TerminalPane`. No other call sites need updating. - -## Files Changed - -New: - -- `apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.ts` -- `apps/desktop/src/renderer/lib/workspace-pane-registry/workspace-pane-registry.test.ts` -- `apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.ts` -- `apps/desktop/src/renderer/lib/workspace-pane-registry/addLaunchPanes.test.ts` -- `apps/desktop/src/renderer/lib/workspace-pane-registry/index.ts` - -Modified: - -- `apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2WorkspacePaneLayout/useV2WorkspacePaneLayout.ts` — drop the `useState` + persistence effects; read from registry. -- `apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types.ts` (or wherever `TerminalPaneData` is) — `initialCommand` becomes optional. -- `apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx` — branch on `initialCommand` defined vs not. -- App boot site (likely `apps/desktop/src/renderer/index.tsx` or the `CollectionsProvider`) — call `initWorkspacePaneRegistry(collections)` once collections are available. - -## Tests - -Registry: - -- `getOrCreateWorkspacePaneStore` returns the same instance for the same `workspaceId`, distinct instances for different ids. -- Persisted layout from a `v2WorkspaceLocalState` row is loaded into the store on first creation. -- Updates to the store write back to the row. -- Updates to the row push into the store. -- Init guard: `getOrCreateWorkspacePaneStore` throws if called before `initWorkspacePaneRegistry`. - -`addLaunchPanes`: - -- Adds terminal panes for `{ kind: "terminal"; terminalId; label? }` entries; pane data has the id but no `initialCommand`. -- Adds chat panes for `{ kind: "chat"; chatSessionId; label? }` entries. -- Dedupes by id: calling twice with the same id results in a single pane and focuses it. -- Mixed array of terminal + chat works. -- Empty array is a no-op. - -Route hook: - -- After the refactor, mounting `useV2WorkspacePaneLayout` for a `workspaceId` that already has panes added via `addLaunchPanes` shows those panes immediately (no flash of empty state). - -## Out of Scope - -- **Removing the pending-row side-channel.** `pendingWorkspaces.terminalLaunch` / `chatLaunch` and `useConsumePendingLaunch` keep working. PR 5 retires them when the new workspace modal moves onto `workspace.create()`. -- **`workspace.create()` itself.** Lives in PR 4. PR 3 just provides the surface PR 4 will call into. -- **The pending route's own logic.** Untouched. -- **Migrating terminal presets to host-driven start.** Today `useV2PresetExecution` mints terminal ids renderer-side and embeds `initialCommand`. That keeps working. A future PR could move presets onto the same attach-only flow `addLaunchPanes` uses, but that's not this PR. - -## Risks and rollout - -- **Registry boot ordering.** `initWorkspacePaneRegistry(collections)` must run before any caller. Unit-tested by the init guard. Real risk: a code path that calls `getOrCreateWorkspacePaneStore` from a top-level module load before app boot finishes. Mitigation: throw a clear error from the guard so the misuse is loud and fixable. -- **Persistence behavior change.** Previously, persistence only ran while the route was mounted. After PR 3, persistence runs whenever a store exists in the registry (i.e. for any workspace whose pane state has been touched this session). Net effect: stores stay in sync more, write more often. The existing snapshot-equality guard already avoids redundant writes. -- **Test isolation.** The registry holds module state; tests must reset it between cases. Plan: export a `__resetWorkspacePaneRegistryForTests()` like `terminalRuntimeRegistry` does. - -## Follow-Ups - -- PR 4: `workspace.create()` returns `launches` and the modal calls `addLaunchPanes(workspaceId, launches)` before navigating. -- PR 5: delete `pendingWorkspaces.terminalLaunch` / `chatLaunch` columns and `useConsumePendingLaunch` once the modal is migrated.