From baf8b215cb4da5b5f8e48065b7658596a0ce2d4c Mon Sep 17 00:00:00 2001 From: marovole Date: Tue, 2 Jun 2026 14:04:27 +0800 Subject: [PATCH] Fix workspace recovery for desktop and web (MUL-2894) (#3436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(workspace): recover from stale workspace state * fix(workspace): apply review nits for recovery flow - no-access-page: navigate via nav.replace so a browser Back doesn't land the user back on NoAccessPage with the dead slug - no-access-page: refresh the stale cookie-clear comment — the recovery button no longer routes through `/`; the clear now guards other `/` entry points (manual nav, Back into `/`, fresh page load) - tab-store: drop the redundant `as string | undefined` cast (the Set value is already string | undefined under TS 5.9) - tab-store.test: cover the route-layout heal path (all stale groups dropped, then seed a fresh tab for a valid slug) and assert the dropped group's router is disposed Co-authored-by: multica-agent --------- Co-authored-by: J Co-authored-by: multica-agent --- .../src/renderer/src/stores/tab-store.test.ts | 41 +++++++++++++++++++ .../src/renderer/src/stores/tab-store.ts | 18 ++++++++ .../views/workspace/no-access-page.test.tsx | 26 ++++++++++-- packages/views/workspace/no-access-page.tsx | 31 ++++++++++---- 4 files changed, 105 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/renderer/src/stores/tab-store.test.ts b/apps/desktop/src/renderer/src/stores/tab-store.test.ts index 3460c9636..276637f45 100644 --- a/apps/desktop/src/renderer/src/stores/tab-store.test.ts +++ b/apps/desktop/src/renderer/src/stores/tab-store.test.ts @@ -259,6 +259,47 @@ describe("useTabStore actions", () => { expect(s.activeWorkspaceSlug).toBeNull(); }); + it("validateWorkspaceSlugs seeds the first valid workspace when no group exists", () => { + const store = useTabStore.getState(); + store.validateWorkspaceSlugs(new Set(["acme", "butter"])); + const s = useTabStore.getState(); + expect(s.activeWorkspaceSlug).toBe("acme"); + expect(s.byWorkspace.acme.tabs).toHaveLength(1); + expect(s.byWorkspace.acme.tabs[0].path).toBe("/acme/issues"); + }); + + it("validateWorkspaceSlugs reactivates an existing valid group before seeding", () => { + const store = useTabStore.getState(); + store.switchWorkspace("acme"); + const existingTabId = useTabStore.getState().byWorkspace.acme.tabs[0].id; + + useTabStore.setState({ activeWorkspaceSlug: null }); + store.validateWorkspaceSlugs(new Set(["acme"])); + + const s = useTabStore.getState(); + expect(s.activeWorkspaceSlug).toBe("acme"); + expect(s.byWorkspace.acme.tabs).toHaveLength(1); + expect(s.byWorkspace.acme.tabs[0].id).toBe(existingTabId); + }); + + it("validateWorkspaceSlugs seeds a fresh tab for a valid slug after dropping all stale groups", () => { + const store = useTabStore.getState(); + // The only persisted group points at a workspace the user has lost access + // to — the stale-tab heal path WorkspaceRouteLayout drives. + store.switchWorkspace("stale"); + const staleRouter = useTabStore.getState().byWorkspace.stale.tabs[0].router; + + store.validateWorkspaceSlugs(new Set(["acme"])); + + const s = useTabStore.getState(); + expect(Object.keys(s.byWorkspace)).toEqual(["acme"]); + expect(s.activeWorkspaceSlug).toBe("acme"); + expect(s.byWorkspace.acme.tabs).toHaveLength(1); + expect(s.byWorkspace.acme.tabs[0].path).toBe("/acme/issues"); + // The dropped stale group's router must be disposed, not leaked. + expect(staleRouter.dispose).toHaveBeenCalled(); + }); + it("reset wipes the whole store", () => { const store = useTabStore.getState(); store.switchWorkspace("acme"); diff --git a/apps/desktop/src/renderer/src/stores/tab-store.ts b/apps/desktop/src/renderer/src/stores/tab-store.ts index 98c23aa5c..07b9dd708 100644 --- a/apps/desktop/src/renderer/src/stores/tab-store.ts +++ b/apps/desktop/src/renderer/src/stores/tab-store.ts @@ -557,6 +557,24 @@ export const useTabStore = create()( changed = true; } + if (!nextActive) { + nextActive = Object.keys(nextByWorkspace)[0] ?? null; + if (nextActive) changed = true; + } + + if (!nextActive) { + const fallbackSlug = validSlugs.values().next().value; + if (fallbackSlug) { + const fresh = defaultTabFor(fallbackSlug); + nextByWorkspace[fallbackSlug] = { + tabs: [fresh], + activeTabId: fresh.id, + }; + nextActive = fallbackSlug; + changed = true; + } + } + if (!changed) return; set({ byWorkspace: nextByWorkspace, activeWorkspaceSlug: nextActive }); }, diff --git a/packages/views/workspace/no-access-page.test.tsx b/packages/views/workspace/no-access-page.test.tsx index 2c2e379f2..c1f286d23 100644 --- a/packages/views/workspace/no-access-page.test.tsx +++ b/packages/views/workspace/no-access-page.test.tsx @@ -12,6 +12,7 @@ const TEST_RESOURCES = { const navigate = vi.fn(); const logout = vi.fn(); +const mockWorkspaces = vi.hoisted(() => [{ slug: "valid-team" }]); vi.mock("../navigation", () => ({ useNavigation: () => ({ push: navigate, replace: navigate }), @@ -21,6 +22,25 @@ vi.mock("../auth", () => ({ useLogout: () => logout, })); +vi.mock("@multica/core/paths", async () => { + const actual = + await vi.importActual( + "@multica/core/paths", + ); + return { + ...actual, + useHasOnboarded: () => true, + }; +}); + +vi.mock("@tanstack/react-query", () => ({ + useQuery: () => ({ data: mockWorkspaces }), +})); + +vi.mock("@multica/core/workspace/queries", () => ({ + workspaceListOptions: () => ({ queryKey: ["workspaces", "list"] }), +})); + function I18nWrapper({ children }: { children: ReactNode }) { return ( @@ -46,15 +66,15 @@ describe("NoAccessPage", () => { ).toBeInTheDocument(); }); - it("navigates to root on 'Go to my workspaces'", () => { + it("navigates to the first accessible workspace on 'Go to my workspaces'", () => { renderPage(); fireEvent.click(screen.getByRole("button", { name: /go to my workspaces/i })); - expect(navigate).toHaveBeenCalledWith("/"); + expect(navigate).toHaveBeenCalledWith("/valid-team/issues"); }); it("clears last_workspace_slug cookie on mount so the proxy stops looping us back", () => { document.cookie = "last_workspace_slug=stale; path=/"; - render(); + renderPage(); // Assert empty value, not just absence of "stale" — the proxy reads any // truthy value as a redirect target, so a buggy clear that left e.g. // `last_workspace_slug=other` would still trap users. diff --git a/packages/views/workspace/no-access-page.tsx b/packages/views/workspace/no-access-page.tsx index 0dfba07de..f636f11e1 100644 --- a/packages/views/workspace/no-access-page.tsx +++ b/packages/views/workspace/no-access-page.tsx @@ -1,8 +1,13 @@ "use client"; import { useEffect } from "react"; +import { useQuery } from "@tanstack/react-query"; import { Button } from "@multica/ui/components/ui/button"; -import { paths } from "@multica/core/paths"; +import { + resolvePostAuthDestination, + useHasOnboarded, +} from "@multica/core/paths"; +import { workspaceListOptions } from "@multica/core/workspace/queries"; import { useNavigation } from "../navigation"; import { useLogout } from "../auth"; import { DragStrip } from "../platform"; @@ -18,19 +23,29 @@ export function NoAccessPage() { const { t } = useT("workspace"); const nav = useNavigation(); const logout = useLogout(); + const hasOnboarded = useHasOnboarded(); + const { data: workspaces = [] } = useQuery(workspaceListOptions()); // Clear stale `last_workspace_slug` cookie. The web proxy redirects `/` to - // `//issues` based on this cookie alone (no access check). When - // the cookie points at a workspace the user has just lost access to, the - // user gets trapped in a loop: NoAccessPage → click "Go to my workspaces" - // → `/` → proxy redirects back to the same bad slug → NoAccessPage. - // Clearing the cookie here lets the proxy fall through to the landing page, - // which then resolves the correct destination via the workspace list. + // `//issues` based on this cookie alone (no access check). When the + // cookie points at a workspace the user has just lost access to, any hit on + // `/` — manual navigation, a browser Back into `/`, or a fresh page load — + // bounces the user straight back to the bad slug and re-traps them on + // NoAccessPage. The recovery button no longer routes through `/` (recover() + // resolves a concrete destination directly), but clearing the cookie here + // keeps those other `/` entry points from re-triggering the loop. // No-op outside the browser (desktop renderer also has document, harmless). useEffect(() => { if (typeof document === "undefined") return; document.cookie = "last_workspace_slug=; path=/; max-age=0; SameSite=Lax"; }, []); + + // replace, not push: the failed `/` URL must not stay in history, + // or a browser Back would land the user right back on this NoAccessPage. + const recover = () => { + nav.replace(resolvePostAuthDestination(workspaces, hasOnboarded)); + }; + return (
@@ -44,7 +59,7 @@ export function NoAccessPage() {

-