mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-17 15:19:00 +02:00
80a24bf627
* refactor(desktop): tabs are per-workspace, not cross-workspace Tabs are now grouped by workspace in the store; the TabBar shows only the active workspace's tabs, and switching workspace swaps the visible group. Before this change tabs were a flat list that spanned workspaces, which produced a confusing experience: working in acme with three tabs, then switching to butter and back, still showed whatever tabs you happened to open while you were in butter alongside your acme work. The bug had the same shape as the pre-workspace-overlay bug we fixed in #1237 — a concept ("workspace") was encoded in data (tab paths) but ignored by the UI that displayed it (TabBar). The fix is structural: make the data model match the concept. Key changes: - **Schema**: `{ activeWorkspaceSlug, byWorkspace: {slug: {tabs, activeTabId}} }`. The invariant "every tab belongs to a workspace group" is enforced at sanitize time and at migration time; there is no longer a root `/` sentinel. - **NavigationAdapter** detects cross-workspace pushes and delegates to `switchWorkspace(slug, path)` instead of navigating the active tab's router. All existing call sites in shared code (sidebar dropdown, settings post-delete redirect, invite-accept, cmd+k) keep calling `push(paths.workspace(x).issues())` unchanged. - **TabContent** renders only the active workspace's tabs under Activity. Cross-workspace state preservation is an explicit non-goal — switching workspaces should feel like switching. - **WorkspaceRouteLayout** auto-heal no longer navigates the tab router to `/`. Stale-slug cleanup is a store-level op (`validateWorkspaceSlugs`) that drops the whole stale group in one go. - **App.tsx** bootstrap seeds `activeWorkspaceSlug` when null and the user has workspaces; the new-workspace overlay opens/closes based on workspace count independently of any route. - **Persistence migration** (v1 → v2) groups old flat tabs by extracted slug, drops root / transition / reserved-slug tabs, and picks an active workspace from the old active tab's owning group. No data loss for existing users with workspace-scoped tabs. Web is unchanged — tabs are a desktop-only concept. `packages/views`, `packages/core`, `apps/web` are all untouched. `setCurrentWorkspace` in core remains the single source of truth for the API client's workspace header, driven by `WorkspaceRouteLayout` as before. Tests: 19 tab-store tests (sanitize, migration, switchWorkspace, validate, close-last-reseeds, reset). 38 desktop tests total pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: stable selectors + defensive guards on tab-store Addresses self-review findings on #1239. **C1 — perf cliff from unstable selector returns.** The previous `useActiveTab()` selector used `.find()` inside, so every router tick on the active tab (which replaces the Tab object via immutable spread in updateTab / updateTabHistory) forced every subscriber to re-render. Replaced with finer-grained selectors: - `useActiveTabIdentity()` — { slug, tabId } primitives (stable across unrelated updates). - `useActiveTabRouter()` — stable object reference for a tab's lifetime. - `useActiveTabHistory()` — { historyIndex, historyLength } numbers. `useTabHistory` and `DesktopNavigationProvider` now consume the primitive selectors, so back/forward buttons don't churn on every path change. A non-hook `getActiveTab(state)` helper covers the event-handler case. **I1 — `switchWorkspace` no-ops on empty slug.** Defensive guard in case a malformed path ever reaches the adapter's detector. **I2 — merge warns on path/slug mismatch.** Previously silent drop; now `console.warn` makes the condition visible during debugging. **Misc — TabRouterInner takes `tab` prop directly.** Passing the Tab object eliminates a redundant store read per rendered tab. Known follow-up (not this PR): `packages/core/realtime/use-realtime-sync.ts` still uses `window.location.assign` for workspace-deleted eviction — that's a full renderer reload on desktop, which post-refactor wastes the careful in-memory tab state we just set up. Fixing cleanly requires a navigation-callback injection pattern through CoreProvider, which is cross-cutting and deserves its own PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workspace): navigate away BEFORE leave/delete mutation to avoid CancelledError Symptom: deleting the current workspace logged "current workspace deleted, switching" from the realtime handler and surfaced an "Uncaught (in promise) CancelledError" from TanStack Query's refetchQueries batch. Root cause: a three-way race between the mutation's own invalidateQueries(workspaceKeys.list()), the settings page's navigateAwayFromCurrentWorkspace() fetchQuery, and the realtime workspace:deleted handler's relocateAfterWorkspaceLoss fetchQuery. All three refetched the same query concurrently; TanStack Query cancelled the in-flight loser(s), and the rejection bubbled out of invalidateQueries as an unhandled promise rejection. Fix: invert the order. Compute the destination from the current cached workspace list, navigate immediately, *then* fire the mutation. By the time the backend fires workspace:deleted, the active workspace is already something else — the realtime handler's "current === deleted" check fails and its relocate branch no-ops. Only one refetch happens (the mutation's onSettled), no race, no cancellation. navigateAwayFromCurrentWorkspace no longer needs async/fetchQuery since it reads from cache and returns before the mutation fires. Applies to both Leave and Delete flows. Both web and desktop benefit since the code is in packages/views. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(desktop): clear workspace singleton + flex drag strip + defer seeding Three issues that the last round of delete-workspace fixes missed. **1. `setCurrentWorkspace` singleton leaks after delete.** Navigating before the mutation (prior fix) changed the URL but nothing cleared the core platform's currentSlug/currentWsId singleton. Three downstream consumers still believed the deleted workspace was active: - `useRealtimeSync`'s `workspace:deleted` handler: its `getCurrentWsId() === deleted` check fired, triggering a parallel relocate that raced the mutation's invalidate and the settings page's navigate — CancelledError + `window.location.assign` (white screen reload). - Chrome gating: `{slug && <AppSidebar />}` stayed truthy, the sidebar mounted, and `useWorkspaceId` inside it threw because the workspace was gone from the list cache. - API client's `X-Workspace-Slug` header: stale on the next call. Fix: `navigateAwayFromCurrentWorkspace` now calls `setCurrentWorkspace(null, null)` before pushing. The next workspace's `WorkspaceRouteLayout` re-sets the singleton when it mounts; for the last-workspace case, null is the correct state (overlay has no workspace context). Same family as the previous logout bug: persist only writes to storage, reset on logout must also wipe in-memory state. Here the singleton is another in-memory bit that survives a URL change if we don't explicitly clear it. **2. "Cannot update a component while rendering" warning.** The per-workspace-tabs refactor kept the validate+seed call in render phase (matching the pre-refactor pattern). It worked before because `validateWorkspaceSlugs` is idempotent; the new `switchWorkspace` seed is not, and triggers a TabBar re-render during AppContent's render. Moved to `useLayoutEffect` — synchronously after render, before paint, no flicker. **3. Welcome-screen drag region didn't work on desktop.** The absolute-positioned `h-10 z-10` drag strip relied on z-index stacking to beat the content wrapper's no-drag for hit-testing, which wasn't reliable for `-webkit-app-region` on the overlay. Replaced with a flex child (`h-12 shrink-0` at top of the overlay's flex-col), so the drag region owns its own layout space — any pixel in the top 48 is unambiguously drag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(CLAUDE): desktop-specific rules — routing, singleton, drag, UX split Codifies the lessons from the recent desktop refactor series (#1237, #1238, #1239) so future work doesn't re-derive them from bugs. Covers: - **Route categories** (session / transition / error) — explains why `/workspaces/new` and `/invite/:id` are overlay state, not routes, on desktop; stale slugs auto-heal instead of rendering error pages. - **`setCurrentWorkspace` singleton hygiene** — unmount doesn't clear it; any code leaving workspace context must call `setCurrentWorkspace(null, null)` explicitly. - **Workspace destructive operations ordering** — navigate first, mutate after, to avoid the three-way refetch race that surfaces as CancelledError + full-page reload. - **Tab isolation** — tabs are grouped per workspace; cross-workspace push is intercepted by the navigation adapter and translated into switchWorkspace. - **Drag region pattern** — flex child at top, not absolute overlay; `-webkit-app-region` hit-testing is unreliable with z-index stacking. - **UX vs platform chrome split** — UX affordances (Back, Log out, welcome copy) in packages/views/; platform chrome (drag, immersive mode, tab system) in desktop-only code. Also patches the Cross-Platform Development Rules' rule #2 which previously said "add a route in both apps" unconditionally — added the exception for pre-workspace transition flows pointing at the new Desktop-specific Rules section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>