fix(web): stop login arrival effect from racing fresh form logins (#5009) (#5019)

The /login page's already-authenticated effect fired on any user change,
including the one verifyCode writes mid form-login while handleVerify is
still fetching the workspace list. It then read the cold list cache
(getQueryData ?? []) and raced handleSuccess with a replace to
/workspaces/new — users with existing workspaces could land on the
create-workspace page after login.

Two changes, both in the arrival effect:

- Ownership: latch once auth settles as logged-out on this page; any
  user appearing afterwards came from the login form, whose
  handleSuccess owns post-login navigation. The effect now only serves
  visitors who arrived authenticated. The desktop-handoff branch stays
  above the latch so form logins with platform=desktop still mint the
  deep-link token.
- Correct data: for genuine arrived-authenticated visitors, fetch the
  list via ensureQueryData instead of reading a possibly-cold cache,
  which misrouted workspace owners to /workspaces/new on fresh page
  loads.

Regression tests verified red against the previous implementation.

Fixes #5009

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

apps/web/app/(auth)/login/page.test.tsx

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, beforeEach } from "vitest";
-import { render, screen, waitFor } from "@testing-library/react";
+import { act, render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
 import { I18nProvider } from "@multica/core/i18n/react";
@@ -25,26 +25,35 @@ const {
   mockSendCode,
   mockVerifyCode,
   mockIssueCliToken,
+  mockListWorkspaces,
+  mockListMyInvitations,
+  mockPush,
+  mockReplace,
   searchParamsState,
   authStateRef,
 } = vi.hoisted(() => ({
   mockSendCode: vi.fn(),
   mockVerifyCode: vi.fn(),
   mockIssueCliToken: vi.fn(),
+  mockListWorkspaces: vi.fn(),
+  mockListMyInvitations: vi.fn(),
+  mockPush: vi.fn(),
+  mockReplace: vi.fn(),
   searchParamsState: { params: new URLSearchParams() },
   authStateRef: {
     state: {
       sendCode: vi.fn(),
       verifyCode: vi.fn(),
-      user: null as null | { id: string; email: string },
+      user: null as null | { id: string; email: string; onboarded_at?: string | null },
       isLoading: false,
     },
   },
 }));
 
-// Mock next/navigation
+// Mock next/navigation — router spies are hoisted so tests can assert
+// which navigation (if any) the page issued.
 vi.mock("next/navigation", () => ({
-  useRouter: () => ({ push: vi.fn(), replace: vi.fn() }),
+  useRouter: () => ({ push: mockPush, replace: mockReplace }),
   usePathname: () => "/login",
   useSearchParams: () => searchParamsState.params,
 }));
@@ -76,7 +85,8 @@ vi.mock("@/features/auth/auth-cookie", () => ({
 // Mock api
 vi.mock("@multica/core/api", () => ({
   api: {
-    listWorkspaces: vi.fn().mockResolvedValue([]),
+    listWorkspaces: mockListWorkspaces,
+    listMyInvitations: mockListMyInvitations,
     verifyCode: vi.fn(),
     setToken: vi.fn(),
     getMe: vi.fn(),
@@ -92,6 +102,8 @@ describe("LoginPage", () => {
     searchParamsState.params = new URLSearchParams();
     authStateRef.state.user = null;
     authStateRef.state.isLoading = false;
+    mockListWorkspaces.mockResolvedValue([]);
+    mockListMyInvitations.mockResolvedValue([]);
   });
 
   it("renders login form with email input and continue button", () => {
@@ -204,4 +216,63 @@ describe("LoginPage", () => {
       });
     }
   });
+
+  // Regression: #5009 — the "already authenticated on arrival" effect used to
+  // fire for fresh form logins too. verifyCode writes `user` while handleVerify
+  // is still fetching the workspace list, so the effect read an empty cache and
+  // raced handleSuccess with replace("/workspaces/new"); depending on the
+  // interleaving the user could end up stuck on the create-workspace page
+  // despite having workspaces.
+  describe("post-login redirect ownership (#5009)", () => {
+    const onboardedUser = {
+      id: "u1",
+      email: "test@multica.ai",
+      onboarded_at: "2026-01-01T00:00:00Z",
+    };
+
+    it("does not redirect from the arrival effect when the user logs in via the form", async () => {
+      // Auth settles as logged-out first — the page latches "any user from
+      // now on came from the form".
+      const wrapper = createWrapper();
+      const { rerender } = render(<LoginPage />, { wrapper });
+      // verifyCode set the user; the workspace list fetch is still in flight
+      // (cache cold). The arrival effect must stay silent — handleSuccess
+      // owns this navigation.
+      authStateRef.state.user = onboardedUser;
+      rerender(<LoginPage />);
+
+      await act(async () => {});
+      expect(mockReplace).not.toHaveBeenCalled();
+      expect(mockPush).not.toHaveBeenCalled();
+      expect(mockListWorkspaces).not.toHaveBeenCalled();
+    });
+
+    it("fetches the workspace list before redirecting a visitor who arrived authenticated", async () => {
+      // Cold Query cache on a fresh page load: reading it would say "no
+      // workspaces" and misroute to /workspaces/new. The effect must fetch.
+      authStateRef.state.user = onboardedUser;
+      mockListWorkspaces.mockResolvedValue([{ id: "ws-1", slug: "acme" }]);
+
+      render(<LoginPage />, { wrapper: createWrapper() });
+
+      await waitFor(() => {
+        expect(mockReplace).toHaveBeenCalledWith("/acme/issues");
+      });
+      expect(mockListWorkspaces).toHaveBeenCalledTimes(1);
+    });
+
+    it("still honors ?next= for a visitor who arrived authenticated", async () => {
+      searchParamsState.params = new URLSearchParams({
+        next: "/invite/abc",
+      });
+      authStateRef.state.user = onboardedUser;
+
+      render(<LoginPage />, { wrapper: createWrapper() });
+
+      await waitFor(() => {
+        expect(mockReplace).toHaveBeenCalledWith("/invite/abc");
+      });
+      expect(mockListWorkspaces).not.toHaveBeenCalled();
+    });
+  });
 });

apps/web/app/(auth)/login/page.tsx

@@ -1,11 +1,14 @@
 "use client";
 
-import { Suspense, useEffect, useState } from "react";
+import { Suspense, useEffect, useRef, useState } from "react";
 import { useSearchParams, useRouter } from "next/navigation";
 import { useQueryClient, type QueryClient } from "@tanstack/react-query";
 import { sanitizeNextUrl, useAuthStore } from "@multica/core/auth";
 import { useConfigStore } from "@multica/core/config";
-import { workspaceKeys } from "@multica/core/workspace/queries";
+import {
+  workspaceKeys,
+  workspaceListOptions,
+} from "@multica/core/workspace/queries";
 import {
   paths,
   resolvePostAuthDestination,
@@ -77,11 +80,21 @@ function LoginPageContent() {
   const [desktopError, setDesktopError] = useState("");
   const hasOnboarded = useHasOnboarded();
 
-  // Already authenticated — honor ?next= or fall back to first workspace
-  // (or /onboarding if the user has none). Skip this entire path when
-  // the user arrived to authorize the CLI.
+  // Latched once auth has been observed settled as logged-out on this page.
+  // Any `user` that appears afterwards came from the login form in this
+  // session — not from an existing session found on arrival.
+  const settledLoggedOutRef = useRef(false);
+
+  // Already authenticated ON ARRIVAL — honor ?next= or fall back to first
+  // workspace (or /onboarding if the user has none). Skip this entire path
+  // when the user arrived to authorize the CLI.
   useEffect(() => {
-    if (isLoading || !user || cliCallbackRaw) return;
+    if (isLoading) return;
+    if (!user) {
+      settledLoggedOutRef.current = true;
+      return;
+    }
+    if (cliCallbackRaw) return;
     if (isDesktopHandoff) {
       // Desktop opened the browser for login but the web session is already
       // authenticated — mint a bearer token from the cookie session and hand
@@ -101,14 +114,26 @@ function LoginPageContent() {
         });
       return;
     }
+    // Fresh form login (issue #5009): `user` was written by verifyCode while
+    // handleVerify was still fetching the workspace list, so this effect used
+    // to read the not-yet-seeded list cache and race handleSuccess with a
+    // replace to /workspaces/new. handleSuccess owns post-login navigation;
+    // this effect only serves visitors who arrived already authenticated.
+    if (settledLoggedOutRef.current) return;
     if (nextUrl) {
       router.replace(nextUrl);
       return;
     }
-    const list = qc.getQueryData<Workspace[]>(workspaceKeys.list()) ?? [];
-    void resolveLoggedInDestination(qc, hasOnboarded, list).then((dest) =>
-      router.replace(dest),
-    );
+    // Fetch instead of reading the cache: on a fresh page load the cache is
+    // cold, and `getQueryData() ?? []` would misroute a user who does have
+    // workspaces to /workspaces/new. On fetch failure fall back to [] —
+    // same destination the cold-cache read produced, rather than trapping
+    // the user on the login page.
+    void qc
+      .ensureQueryData(workspaceListOptions())
+      .catch(() => [] as Workspace[])
+      .then((list) => resolveLoggedInDestination(qc, hasOnboarded, list))
+      .then((dest) => router.replace(dest));
   }, [isLoading, user, router, nextUrl, cliCallbackRaw, isDesktopHandoff, hasOnboarded, qc]);
 
   const handleSuccess = async () => {