diff --git a/studio/frontend/src/app/routes/__root.tsx b/studio/frontend/src/app/routes/__root.tsx index 8c6ddd197a..6f68917224 100644 --- a/studio/frontend/src/app/routes/__root.tsx +++ b/studio/frontend/src/app/routes/__root.tsx @@ -44,6 +44,7 @@ declare module "@tanstack/react-router" { interface StaticDataRouteOption { title?: string; titleKey?: TranslationKey; + isAuthFlow?: boolean; } } @@ -103,6 +104,9 @@ function RootLayout() { const t = useT(); const pathname = useRouterState({ select: (s) => s.location.pathname }); const hideNavbar = HIDDEN_NAVBAR_ROUTES.includes(pathname); + const isAuthFlowRoute = useMatches({ + select: (matches) => matches.some((match) => match.staticData.isAuthFlow), + }); // Exact match: a prefix would treat /chatty as chat, hiding its not-found UI. const isChatRoute = pathname === "/chat"; const { pinned, setPinned, togglePinned } = useSidebarPin(); @@ -161,7 +165,8 @@ function RootLayout() { }); const settingsDialogOpen = useSettingsDialogStore((s) => s.open); - const documentTitle = settingsDialogOpen ? t("settings.title") : matchedTitle; + const documentTitle = + settingsDialogOpen && !isAuthFlowRoute ? t("settings.title") : matchedTitle; useLayoutEffect(() => { document.title = documentTitle @@ -170,9 +175,13 @@ function RootLayout() { }, [documentTitle]); useEffect(() => { + if (isAuthFlowRoute) { + useSettingsDialogStore.getState().closeDialog(); + } const handler = (e: KeyboardEvent) => { if (e.defaultPrevented) return; if ((e.metaKey || e.ctrlKey) && e.key === ",") { + if (isAuthFlowRoute) return; e.preventDefault(); useSettingsDialogStore.getState().openDialog(); return; @@ -196,7 +205,7 @@ function RootLayout() { }; window.addEventListener("keydown", handler); return () => window.removeEventListener("keydown", handler); - }, [navigate]); + }, [isAuthFlowRoute, navigate]); useEffect(() => { if (isChatRoute) return; @@ -219,7 +228,7 @@ function RootLayout() { return ( - + {!isAuthFlowRoute && } {hideNavbar ? (
diff --git a/studio/frontend/src/app/routes/change-password.tsx b/studio/frontend/src/app/routes/change-password.tsx index 55c8ceaa9c..beaf7ef60b 100644 --- a/studio/frontend/src/app/routes/change-password.tsx +++ b/studio/frontend/src/app/routes/change-password.tsx @@ -15,7 +15,7 @@ const ChangePasswordPage = lazy(() => export const Route = createRoute({ getParentRoute: () => rootRoute, path: "/change-password", - staticData: { title: "Change Password" }, + staticData: { title: "Change Password", isAuthFlow: true }, beforeLoad: () => requirePasswordChangeFlow(), component: ChangePasswordPage, }); diff --git a/studio/frontend/src/app/routes/login.tsx b/studio/frontend/src/app/routes/login.tsx index bfd1b82132..756e826a83 100644 --- a/studio/frontend/src/app/routes/login.tsx +++ b/studio/frontend/src/app/routes/login.tsx @@ -13,7 +13,7 @@ const LoginPage = lazy(() => export const Route = createRoute({ getParentRoute: () => rootRoute, path: "/login", - staticData: { title: "Login" }, + staticData: { title: "Login", isAuthFlow: true }, beforeLoad: () => requireGuest(), component: LoginPage, }); diff --git a/studio/frontend/src/app/routes/onboarding.tsx b/studio/frontend/src/app/routes/onboarding.tsx index 6c31d794ba..bdac4162b0 100644 --- a/studio/frontend/src/app/routes/onboarding.tsx +++ b/studio/frontend/src/app/routes/onboarding.tsx @@ -17,7 +17,7 @@ const WizardLayout = lazy(() => export const Route = createRoute({ getParentRoute: () => rootRoute, path: "/onboarding", - staticData: { title: "Onboarding" }, + staticData: { title: "Onboarding", isAuthFlow: true }, beforeLoad: () => requireAuth(), validateSearch: (search: Record): OnboardingSearch => ({ redirectTo: typeof search.redirectTo === "string" ? search.redirectTo : undefined, diff --git a/studio/frontend/src/features/auth/api.ts b/studio/frontend/src/features/auth/api.ts index 144e85d40a..38ab9aa5ea 100644 --- a/studio/frontend/src/features/auth/api.ts +++ b/studio/frontend/src/features/auth/api.ts @@ -82,6 +82,10 @@ async function redirectToAuth(): Promise { // Fall through to /login on error } + if (window.location.pathname === target) { + isRedirecting = false; + return; + } window.location.href = target; } diff --git a/tests/studio/playwright_chat_ui.py b/tests/studio/playwright_chat_ui.py index 6a88b98c19..065ba7a745 100644 --- a/tests/studio/playwright_chat_ui.py +++ b/tests/studio/playwright_chat_ui.py @@ -160,6 +160,15 @@ with sync_playwright() as p: # collapse run startViewTransition() which can leave intercepting # pointer events for a beat after each route swap. See _playwright_robust.py. install_view_transition_killer(ctx) + system_requests: list[str] = [] + ctx.on( + "request", + lambda request: ( + system_requests.append(request.url) + if request.url.split("?", 1)[0].endswith("/api/system") + else None + ), + ) page = ctx.new_page() # 60s default (was 30s): macos-14 under --single-process Chromium is # slow enough that renders/webfonts/lazy routes routinely crowd 30s. @@ -1259,11 +1268,10 @@ with sync_playwright() as p: ) # ───────────────────────────────────────────────────── - # 17. Shutdown via the account menu. The "Stop server" action - # POSTs /api/shutdown, swaps in the "Unsloth Studio has stopped" - # placeholder, and /api/health goes unreachable shortly after. + # 17. Persisted monitor auth boundary, then shutdown. A monitor left open + # must stay dormant on /login and resume after successful authentication. # ───────────────────────────────────────────────────── - step("Shutdown via account menu") + step("persisted monitor stays dormant on /login and resumes after auth") # Start fresh after the CLI rotation invalidates this browser session. # Stay in the SAME context: macOS Chromium runs --single-process, where # closing the last context kills the browser and a second context cannot @@ -1273,6 +1281,13 @@ with sync_playwright() as p: ctx.clear_cookies() except Exception as exc: info(f"WARN clearing stale session cookies failed: {exc!r}") + robust_evaluate( + page, + """() => localStorage.setItem( + "unsloth_monitor_overlay", + JSON.stringify({ state: { isOpen: true, isMinimized: false }, version: 0 }) + )""", + ) # Auth tokens live in localStorage, and /login's guest guard redirects on # their mere presence, so drop them before navigating. try: @@ -1291,6 +1306,7 @@ with sync_playwright() as p: except Exception: pass page = _fresh_page + login_system_request_count = len(system_requests) # Re-login with NEW2 for a valid /api/shutdown token. Route changes can # still abort or interrupt this navigation, so the field wait below is the @@ -1311,6 +1327,14 @@ with sync_playwright() as p: info(f"goto /login interrupted ({exc!r}); password-field wait will confirm /login") pw_field = page.locator("#password") pw_field.wait_for(state = "visible", timeout = 60_000) + page.keyboard.press("Control+,") + page.wait_for_timeout(5_500) + if len(system_requests) != login_system_request_count: + raise AssertionError( + "persisted monitor requested /api/system while /login was active" + ) + if "/login" not in page.url: + raise AssertionError(f"login route reloaded or redirected unexpectedly: {page.url}") pw_field.fill(NEW2) # Wait on the login POST so a transient 4xx/5xx is caught and retried # here, not swallowed until the out-of-loop composer wait. @@ -1384,8 +1408,17 @@ with sync_playwright() as p: # merely-slow composer look like a broken login. composer = page.locator('textarea[aria-label="Message input"]') composer.wait_for(state = "visible", timeout = 60_000) + monitor_deadline = time.time() + 10 + while len(system_requests) == login_system_request_count and time.time() < monitor_deadline: + page.wait_for_timeout(100) + if len(system_requests) == login_system_request_count: + fail("persisted monitor did not resume /api/system polling after login") + if page.get_by_role("dialog", name = re.compile(r"^Settings$")).count() != 0: + fail("settings shortcut on /login left the dialog open after authentication") + info("OK persisted monitor stayed dormant on /login and resumed after authentication") shoot("18-relogin-with-NEW2") + step("Shutdown via account menu") acct_btn = page.locator('button[aria-label$=" account menu"]').first if acct_btn.count() == 0: fail("account menu button missing -- can't reach Shutdown") diff --git a/tests/studio/test_auth_form_input_count.py b/tests/studio/test_auth_form_input_count.py index dd582d17d1..4d5d72d20b 100644 --- a/tests/studio/test_auth_form_input_count.py +++ b/tests/studio/test_auth_form_input_count.py @@ -1,22 +1,27 @@ # SPDX-License-Identifier: AGPL-3.0-only # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. -"""Pin the auth-form input-count contract on the change-password page. +"""Fast source and runtime contracts for Studio's frontend authentication flows. PR #5490 added a third "Current password" input, regressing first-boot UX to three inputs; PR #5545 restores two by rendering it only when BOOTSTRAP is absent. -These tests inspect the source directly (no Studio/browser/network); runtime is -covered by tests/studio/playwright_chat_ui.py.""" +Issue #7114 covers auth redirects and the persisted System monitor; its browser +lifecycle remains covered by tests/studio/playwright_chat_ui.py.""" from __future__ import annotations import re +import shutil +import subprocess +import textwrap from pathlib import Path -AUTH_FORM = ( - Path(__file__).resolve().parents[2] - / "studio/frontend/src/features/auth/components/auth-form.tsx" -) +import pytest + +REPO = Path(__file__).resolve().parents[2] +FRONTEND = REPO / "studio/frontend/src" +AUTH_FORM = FRONTEND / "features/auth/components/auth-form.tsx" +AUTH_API = FRONTEND / "features/auth/api.ts" CONDITIONAL_OPENER = "{!hasBootstrapPassword && (" @@ -161,3 +166,110 @@ def test_login_jsx_declares_exactly_one_password_input(): assert len(pw_ids) == 1, ( f"login JSX must declare exactly one password-typed input; " f"found {pw_ids!r}" ) + + +def test_auth_flow_routes_do_not_mount_global_settings(): + root = (FRONTEND / "app/routes/__root.tsx").read_text() + assert "{!isAuthFlowRoute && }" in root + assert "useSettingsDialogStore.getState().closeDialog();" in root + assert "if (isAuthFlowRoute) return;" in root + for route in ("login", "change-password", "onboarding"): + assert "isAuthFlow: true" in (FRONTEND / f"app/routes/{route}.tsx").read_text() + + +def test_auth_redirect_targets_are_idempotent_and_concurrent(tmp_path: Path): + if shutil.which("node") is None: + pytest.skip("node not available") + probe = subprocess.run( + ["node", "--experimental-strip-types", "--version"], + capture_output = True, + text = True, + timeout = 5, + ) + if probe.returncode != 0: + pytest.skip("node --experimental-strip-types not available") + + source = ( + AUTH_API.read_text() + .replace('from "@/lib/api-base"', 'from "./stubs.mjs"') + .replace('from "./session"', 'from "./stubs.mjs"') + ) + (tmp_path / "api.ts").write_text(source) + (tmp_path / "stubs.mjs").write_text( + textwrap.dedent(""" + let access = null, refresh = null, passwordChange = false; + export const apiUrl = (path) => path; + export const isTauri = false; + export const reset = (a = null, r = null) => { access = a; refresh = r; passwordChange = false; }; + export const clearAuthTokens = () => { access = null; refresh = null; }; + export const getAuthToken = () => access; + export const getRefreshToken = () => refresh; + export const mustChangePassword = () => passwordChange; + export const setMustChangePassword = (value) => { passwordChange = value; }; + export const storeAuthTokens = (a, r) => { access = a; refresh = r; }; + """) + ) + script = textwrap.dedent(""" + import assert from "node:assert/strict"; + import { reset } from "./stubs.mjs"; + const response = (status, value) => new Response( + value && JSON.stringify(value), { status } + ); + const settle = () => new Promise((resolve) => setImmediate(resolve)); + const load = (name) => import(`./api.ts?${name}`); + const locationAt = (pathname) => { + const assigned = []; + globalThis.window = { location: { pathname, + set href(value) { assigned.push(value); this.pathname = value; } + }}; + return assigned; + }; + async function redirectCase(path, requiresChange, name, repeats = 1) { + reset(); + const assigned = locationAt(path); + let statusCalls = 0; + globalThis.fetch = async (input) => { + if (input === "/api/auth/status") { + statusCalls += 1; + return response(200, { requires_password_change: requiresChange }); + } + return response(401); + }; + const { authFetch } = await load(name); + for (let i = 0; i < repeats; i += 1) { + await authFetch("/api/system"); + await settle(); + } + return { assigned, statusCalls }; + } + const login = await redirectCase("/login", false, "login", 2); + assert.deepEqual(login, { assigned: [], statusCalls: 2 }); + const change = await redirectCase("/chat", true, "change"); + assert.deepEqual(change.assigned, ["/change-password"]); + + reset("expired", "refresh"); + const assigned = locationAt("/chat"); + const calls = { refresh: 0, status: 0 }; + globalThis.fetch = async (input) => { + if (input === "/api/auth/refresh") calls.refresh += 1; + if (input === "/api/auth/status") { + calls.status += 1; + return response(200, { requires_password_change: false }); + } + return response(401); + }; + const { authFetch } = await load("concurrent"); + await Promise.all([authFetch("/api/system"), authFetch("/api/system")]); + await settle(); + assert.deepEqual(calls, { refresh: 1, status: 1 }); + assert.deepEqual(assigned, ["/login"]); + """) + result = subprocess.run( + ["node", "--experimental-strip-types", "--no-warnings", "--input-type=module"], + input = script, + cwd = tmp_path, + capture_output = True, + text = True, + timeout = 30, + ) + assert result.returncode == 0, f"stderr: {result.stderr}\nstdout: {result.stdout}"