Fix workspace recovery for desktop and web (MUL-2894) (#3436)

* 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 <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
marovole
2026-06-02 14:04:27 +08:00
committed by GitHub
parent 03961206ff
commit baf8b215cb
4 changed files with 105 additions and 11 deletions

View File

@@ -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");

View File

@@ -557,6 +557,24 @@ export const useTabStore = create<TabStore>()(
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 });
},

View File

@@ -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<typeof import("@multica/core/paths")>(
"@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 (
<I18nProvider locale="en" resources={TEST_RESOURCES}>
@@ -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(<NoAccessPage />);
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.

View File

@@ -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
// `/<lastSlug>/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.
// `/<lastSlug>/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 `/<bad-slug>` 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 (
<div className="flex min-h-svh flex-col">
<DragStrip />
@@ -44,7 +59,7 @@ export function NoAccessPage() {
</p>
</div>
<div className="flex flex-col gap-2 sm:flex-row">
<Button onClick={() => nav.push(paths.root())}>
<Button onClick={recover}>
{t(($) => $.no_access.go_to_workspaces)}
</Button>
<Button variant="outline" onClick={logout}>