fix(desktop): new tab inherits current workspace + guard against malformed tab paths (#1198)

* fix(desktop): new tab inherits current workspace + guard against malformed tab paths

Three layered fixes for the same root cause: tab URLs were being
constructed without a workspace slug in some code paths, triggering
NoAccessPage whenever the router interpreted the first segment as a
(non-existent) workspace slug.

## Layer 1 — tab-bar "+" button now inherits current workspace

The handler had a hardcoded `path = "/issues"` left over from before
the slug URL refactor. Without a workspace prefix, the router saw
`workspaceSlug = "issues"` and rendered NoAccessPage. Read
`getCurrentSlug()` and build `/{slug}/issues` instead. Falls back to
"/" (→ IndexRedirect) when there is no current workspace.

This matches terminal/IDE new-tab semantics: new tab opens in the
same workspace as the active tab, not in `wsList[0]`.

## Layer 2 — validateWorkspaceSlugs runs synchronously

PR #1178 added startup validation of persisted tab slugs against the
current workspace list, but ran it in a useEffect. useEffect fires
AFTER commit, so the initial render would briefly show NoAccessPage
on a stale slug before the effect reset the tab path. Moving the call
into render phase eliminates that flash; zustand supports setState in
render, and the validator is idempotent (early-returns if nothing
changed) so this doesn't loop.

## Layer 3 — tab store rejects malformed paths at construction

Any path whose first segment is a reserved slug (e.g. "/issues",
"/login") clearly lacks a workspace prefix and is a caller bug.
sanitizeTabPath catches these at makeTab time, rewrites to "/", and
logs a console.warn naming the offending path so the bug can be fixed
at source. Any future new-tab entry point that forgets the slug will
not reach NoAccessPage.

Net effect: NoAccessPage is reserved for its legitimate purpose —
users navigating to URLs they genuinely don't have access to — and
can no longer be triggered by system bugs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* review: read new-tab workspace from active tab + unify sanitize + add tests

Three follow-ups from self-review of PR #1198:

1. Resolve the current workspace from the active tab's path instead of
   from getCurrentSlug(). With N tabs mounted under <Activity>, every
   WorkspaceRouteLayout calls setCurrentWorkspace() in render — the
   singleton ends up holding "whichever tab rendered last", which is
   non-deterministic. activeTabId is the unambiguous source of truth
   for "which workspace is the user actually looking at right now".

2. Unify the persist merge's stale-path detection with sanitizeTabPath.
   The merge previously checked ROUTE_ICONS (dashboard segments only);
   sanitizeTabPath uses isReservedSlug (dashboard + auth + platform +
   RFC 2142 + hostname confusables). Same code path now, wider
   coverage, and one source of truth.

3. Add unit tests for sanitizeTabPath: root pass-through, global paths,
   valid workspace-scoped paths, malformed paths (reserved first
   segment) rejected with console.warn, and user slugs that happen to
   look path-like but aren't reserved.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: NevilleQingNY

apps/desktop/src/renderer/src/App.tsx

@@ -87,14 +87,18 @@ function AppContent() {
   // Tabs survive across app restarts and account switches (persisted to
   // localStorage `multica_tabs`), so a tab path like `/naiyuan/issues` may
   // reference a workspace the current user can't access — showing
-  // NoAccessPage every time they open the app. Reset any such tab to `/`
-  // so IndexRedirect picks a valid workspace. Runs on every workspace list
-  // change (login, refetch, realtime workspace:deleted); idempotent.
-  useEffect(() => {
-    if (!workspaces) return;
+  // NoAccessPage every time they open the app.
+  //
+  // Run synchronously in render phase rather than in useEffect so the first
+  // render already sees validated tabs. useEffect runs AFTER commit, which
+  // means the initial render would briefly show NoAccessPage before the
+  // effect resets the tab. Zustand supports render-phase setState; the
+  // validator is idempotent (exits early if nothing changed) so this
+  // doesn't loop.
+  if (workspaces) {
     const validSlugs = new Set(workspaces.map((w) => w.slug));
     useTabStore.getState().validateWorkspaceSlugs(validSlugs);
-  }, [workspaces]);
+  }
   // null = undecided (pre-login or list hasn't settled yet)
   // true  = session started with zero workspaces; next transition to >=1 triggers restart
   // false = session started with >=1 workspace, OR we've already restarted; skip

apps/desktop/src/renderer/src/components/tab-bar.tsx

@@ -30,6 +30,7 @@ import {
 import { CSS } from "@dnd-kit/utilities";
 import { cn } from "@multica/ui/lib/utils";
 import { useTabStore, resolveRouteIcon, type Tab } from "@/stores/tab-store";
+import { isGlobalPath, paths } from "@multica/core/paths";
 
 const TAB_ICONS: Record<string, LucideIcon> = {
   Inbox,
@@ -124,10 +125,22 @@ function NewTabButton() {
   const setActiveTab = useTabStore((s) => s.setActiveTab);
 
   const handleClick = () => {
-    const path = "/issues";
+    // Inherit the active tab's workspace. Terminal/IDE convention: new tab
+    // opens in the same context as the active one. Read the slug from the
+    // active tab's path directly rather than from getCurrentSlug(), because
+    // that singleton is "last tab to render" (non-deterministic with N tabs
+    // mounted under <Activity>), while activeTabId is the unambiguous truth.
+    // Falls back to "/" (→ IndexRedirect → first workspace) when the active
+    // tab is on a global route (e.g. /workspaces/new, /login).
+    const { tabs, activeTabId } = useTabStore.getState();
+    const activePath = tabs.find((t) => t.id === activeTabId)?.path ?? "/";
+    let slug: string | null = null;
+    if (activePath !== "/" && !isGlobalPath(activePath)) {
+      slug = activePath.split("/").filter(Boolean)[0] ?? null;
+    }
+    const path = slug ? paths.workspace(slug).issues() : "/";
     const tabId = addTab(path, "Issues", resolveRouteIcon(path));
     setActiveTab(tabId);
-    // No navigate() — new tab's router starts at /issues automatically
   };
 
   return (

apps/desktop/src/renderer/src/stores/tab-store.test.ts

@@ -0,0 +1,45 @@
+import { describe, expect, it, vi } from "vitest";
+
+// createTabRouter transitively pulls in route modules that expect a browser
+// router context. For pure-function tests we stub it out.
+vi.mock("../routes", () => ({
+  createTabRouter: vi.fn(() => ({ dispose: vi.fn() })),
+}));
+
+import { sanitizeTabPath } from "./tab-store";
+
+describe("sanitizeTabPath", () => {
+  it("passes through root sentinel", () => {
+    expect(sanitizeTabPath("/")).toBe("/");
+  });
+
+  it("passes through global paths", () => {
+    expect(sanitizeTabPath("/login")).toBe("/login");
+    expect(sanitizeTabPath("/workspaces/new")).toBe("/workspaces/new");
+    expect(sanitizeTabPath("/invite/abc")).toBe("/invite/abc");
+    expect(sanitizeTabPath("/auth/callback")).toBe("/auth/callback");
+  });
+
+  it("passes through valid workspace-scoped paths", () => {
+    expect(sanitizeTabPath("/acme/issues")).toBe("/acme/issues");
+    expect(sanitizeTabPath("/my-team/projects/abc")).toBe("/my-team/projects/abc");
+  });
+
+  it("rejects paths whose first segment is a reserved slug", () => {
+    // A stray "/issues" (pre-refactor leftover, missing workspace prefix)
+    // would be interpreted as workspaceSlug="issues" → NoAccessPage.
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    expect(sanitizeTabPath("/issues")).toBe("/");
+    expect(sanitizeTabPath("/issues/abc-123")).toBe("/");
+    expect(sanitizeTabPath("/settings")).toBe("/");
+    expect(warn).toHaveBeenCalled();
+    warn.mockRestore();
+  });
+
+  it("passes through user slugs that happen to look path-like but aren't reserved", () => {
+    // A workspace owner could legitimately pick "acme-issues" or
+    // "project-x" as their slug — sanitize must not touch these.
+    expect(sanitizeTabPath("/acme-issues/issues")).toBe("/acme-issues/issues");
+    expect(sanitizeTabPath("/project-x/inbox")).toBe("/project-x/inbox");
+  });
+});

apps/desktop/src/renderer/src/stores/tab-store.ts

@@ -3,7 +3,7 @@ import { createJSONStorage, persist } from "zustand/middleware";
 import { arrayMove } from "@dnd-kit/sortable";
 import { createPersistStorage, defaultStorage } from "@multica/core/platform";
 import { createSafeId } from "@multica/core/utils";
-import { isGlobalPath } from "@multica/core/paths";
+import { isGlobalPath, isReservedSlug } from "@multica/core/paths";
 import type { DataRouter } from "react-router-dom";
 import { createTabRouter } from "../routes";
 
@@ -104,13 +104,44 @@ function createId(): string {
   return createSafeId();
 }
 
+/**
+ * Defensive: catch tab paths that were constructed without a workspace slug
+ * (e.g. a hardcoded "/issues" leftover from before the URL refactor). Such
+ * paths would get matched as `workspaceSlug="issues"` by the router and
+ * render NoAccessPage. Sanitize by falling back to "/" (IndexRedirect picks
+ * a valid workspace).
+ *
+ * Passes through:
+ *  - "/" and global paths (/login, /workspaces/new, /invite/..., /auth/...)
+ *  - workspace-scoped paths whose first segment is not a reserved word
+ *
+ * Rejects (and rewrites to "/"):
+ *  - Paths whose first segment is a reserved slug (=/=workspace slug), which
+ *    means the caller forgot to prefix the workspace. Logs a warning so the
+ *    buggy call site is easy to find.
+ */
+export function sanitizeTabPath(path: string): string {
+  if (path === DEFAULT_PATH || isGlobalPath(path)) return path;
+  const firstSegment = path.split("/").filter(Boolean)[0] ?? "";
+  if (isReservedSlug(firstSegment)) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `[tab-store] tab path "${path}" starts with reserved slug "${firstSegment}" — ` +
+        `caller likely forgot the workspace prefix. Falling back to "/".`,
+    );
+    return DEFAULT_PATH;
+  }
+  return path;
+}
+
 function makeTab(path: string, title: string, icon: string): Tab {
+  const safePath = sanitizeTabPath(path);
   return {
     id: createId(),
-    path,
+    path: safePath,
     title,
     icon,
-    router: createTabRouter(path),
+    router: createTabRouter(safePath),
     historyIndex: 0,
     historyLength: 1,
   };
@@ -239,19 +270,13 @@ export const useTabStore = create<TabStore>()(
         if (!persisted?.tabs?.length) return currentState;
 
         const tabs: Tab[] = persisted.tabs.map((tab) => {
-          // Migration: pre-refactor tab paths like "/issues/abc" lack a
-          // workspace slug prefix. These would 404 in the new router.
-          // Reset to "/" so IndexRedirect picks the right workspace.
-          let path = tab.path;
-          if (path !== "/" && !isGlobalPath(path)) {
-            const segments = path.split("/").filter(Boolean);
-            const firstSegment = segments[0] ?? "";
-            // If the first segment IS a known route name (e.g. "issues",
-            // "projects"), it's an old-format path missing the slug prefix.
-            if (ROUTE_ICONS[firstSegment]) {
-              path = "/";
-            }
-          }
+          // Sanitize persisted paths against reserved-slug rules. Catches
+          // both pre-refactor paths like "/issues/abc" (missing workspace
+          // slug) and any other malformed paths that slipped past the
+          // write-time guard. The defense across makeTab + merge + runtime
+          // validate ensures stale or malformed paths never reach the
+          // router.
+          const path = sanitizeTabPath(tab.path);
           return {
             ...tab,
             path,