From 09d56a330bc85a6437c87383cfd36a87faa08529 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Mon, 29 Jun 2026 06:01:05 -0400 Subject: [PATCH] [codex] Always show first-run onboarding (#1528) * Always show first-run onboarding * Fix first-run onboarding CI regressions * Fix root onboarding launch navigation --------- Co-authored-by: Codex --- .../onboarding/onboarding-host.test.tsx | 50 +++---- __tests__/root.test.tsx | 130 ++++++++++++++---- .../recommended-automations-launcher.tsx | 2 +- .../onboarding/is-backend-llm-ready.ts | 49 ------- .../features/onboarding/onboarding-host.tsx | 30 +--- .../onboarding/steps/say-hello-step.tsx | 2 +- src/root.tsx | 113 +++++---------- .../backends/mock-llm-auth-modes.spec.ts | 1 + .../backends/mock-llm-cross-connect.spec.ts | 69 ++++------ 9 files changed, 186 insertions(+), 260 deletions(-) delete mode 100644 src/components/features/onboarding/is-backend-llm-ready.ts diff --git a/__tests__/components/onboarding/onboarding-host.test.tsx b/__tests__/components/onboarding/onboarding-host.test.tsx index 90627ac267..790f892052 100644 --- a/__tests__/components/onboarding/onboarding-host.test.tsx +++ b/__tests__/components/onboarding/onboarding-host.test.tsx @@ -1,6 +1,6 @@ import React from "react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; import { MemoryRouter } from "react-router"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -108,7 +108,7 @@ describe("OnboardingHost", () => { ).toBeInTheDocument(); }); - it("skips the modal and marks completion for a returning Cloud user with a configured LLM", async () => { + it("shows the modal for a fresh Cloud user even when the backend has a configured LLM", async () => { seedCloudBackend(); vi.spyOn(SettingsService, "getSettings").mockResolvedValue({ ...DEFAULT_SETTINGS, @@ -121,15 +121,15 @@ describe("OnboardingHost", () => { renderHost(); - await waitFor(() => { - expect( - window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), - ).not.toBeNull(); - }); - expect(screen.queryByTestId("onboarding-modal-stub")).toBeNull(); + expect( + await screen.findByTestId("onboarding-modal-stub"), + ).toBeInTheDocument(); + expect( + window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), + ).toBeNull(); }); - it("skips the modal when the active Cloud LLM uses subscription auth (no API key)", async () => { + it("shows the modal for a fresh Cloud user even when the active LLM uses subscription auth", async () => { seedCloudBackend(); vi.spyOn(SettingsService, "getSettings").mockResolvedValue({ ...DEFAULT_SETTINGS, @@ -142,12 +142,12 @@ describe("OnboardingHost", () => { renderHost(); - await waitFor(() => { - expect( - window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), - ).not.toBeNull(); - }); - expect(screen.queryByTestId("onboarding-modal-stub")).toBeNull(); + expect( + await screen.findByTestId("onboarding-modal-stub"), + ).toBeInTheDocument(); + expect( + window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), + ).toBeNull(); }); it("still shows the modal for a Cloud user when an API key is set but no model is configured", async () => { @@ -171,13 +171,7 @@ describe("OnboardingHost", () => { ).toBeNull(); }); - it("skips the modal for a user-added Local backend that already has an LLM configured (llm_api_key_is_set)", async () => { - // When the user connects via Add Backend to an existing - // agent-server that already has an LLM saved, walking them - // through "Set up your LLM" would just overwrite the stored - // value with a copy. Skip and persist completion. This only - // applies to Local backends the user explicitly added (not the - // launcher-seeded default-local — see the next test). + it("shows the modal for a user-added Local backend that already has an LLM configured", async () => { seedUserAddedLocalBackend(); vi.spyOn(SettingsService, "getSettings").mockResolvedValue({ ...DEFAULT_SETTINGS, @@ -190,12 +184,12 @@ describe("OnboardingHost", () => { renderHost(); - await waitFor(() => { - expect( - window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), - ).not.toBeNull(); - }); - expect(screen.queryByTestId("onboarding-modal-stub")).toBeNull(); + expect( + await screen.findByTestId("onboarding-modal-stub"), + ).toBeInTheDocument(); + expect( + window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), + ).toBeNull(); }); it("still shows the modal for a launcher-seeded default-local backend even when the agent-server reports a configured LLM", async () => { diff --git a/__tests__/root.test.tsx b/__tests__/root.test.tsx index f44ba0bc32..ebe178ae10 100644 --- a/__tests__/root.test.tsx +++ b/__tests__/root.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { createRoutesStub } from "react-router"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -34,13 +34,35 @@ vi.mock("react-i18next", () => ({ }), })); -vi.mock("#/components/features/onboarding/onboarding-modal", () => ({ - OnboardingModal: () => ( -
-
-
- ), -})); +vi.mock("#/components/features/onboarding/onboarding-modal", async () => { + const React = await import("react"); + const { useNavigation } = await import("#/context/navigation-context"); + + return { + OnboardingModal: ({ onClose }: { onClose: () => void }) => { + const { navigate } = useNavigation(); + return React.createElement( + "div", + { "data-testid": "onboarding-modal" }, + React.createElement("div", { + "data-testid": "onboarding-step-check-backend", + }), + React.createElement( + "button", + { + type: "button", + "data-testid": "mock-onboarding-launch", + onClick: () => { + navigate("/conversations/mock-conversation"); + onClose(); + }, + }, + "Launch conversation", + ), + ); + }, + }; +}); const RouterStub = createRoutesStub([ { @@ -51,6 +73,12 @@ const RouterStub = createRoutesStub([ Component: () =>
app outlet
, path: "/", }, + { + Component: () => ( +
conversation outlet
+ ), + path: "/conversations/:conversationId", + }, ], }, ]); @@ -108,6 +136,51 @@ describe("App root agent-server availability guard", () => { ).not.toBeInTheDocument(); }); + it("shows first-run onboarding before the recovery modal when no backend is configured", async () => { + vi.stubEnv("VITE_SESSION_API_KEY", ""); + delete (window as unknown as Record) + .__AGENT_CANVAS_SESSION_API_KEY__; + window.localStorage.clear(); + __resetActiveStoreForTests(); + + renderApp(["/"]); + + await waitFor(() => { + expect( + screen.getByTestId("first-run-onboarding-screen"), + ).toBeInTheDocument(); + }); + expect(await screen.findByTestId("onboarding-modal")).toBeInTheDocument(); + expect( + screen.queryByTestId("agent-server-onboarding-screen"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("manage-backends-modal"), + ).not.toBeInTheDocument(); + }); + + it("lets root-level onboarding navigate to the launched conversation before closing", async () => { + server.use( + http.get("*/server_info", () => + HttpResponse.json({ uptime: 0, idle_time: 0, version: "1.28.1" }), + ), + ); + + renderApp(["/"]); + + fireEvent.click(await screen.findByTestId("mock-onboarding-launch")); + + await waitFor(() => { + expect(screen.getByTestId("conversation-outlet")).toBeInTheDocument(); + }); + expect(window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY)).toBe( + "1", + ); + expect( + screen.queryByTestId("first-run-onboarding-screen"), + ).not.toBeInTheDocument(); + }); + it("shows first-run onboarding before the recovery modal when locked to Cloud with no backend", async () => { vi.stubEnv("VITE_LOCK_TO_CLOUD", "https://app.all-hands.dev"); vi.stubEnv("VITE_SESSION_API_KEY", ""); @@ -204,13 +277,9 @@ describe("App root agent-server availability guard", () => { it("forces first-run onboarding in locked mode even when a stale Local backend reports a configured LLM", async () => { // Critical regression for PR #1389 review: in locked-to-Cloud mode the - // ready-backend fast-path must NOT skip onboarding for a reachable - // stale Local backend that happens to report a configured LLM. The - // fast-path may only fire when the active backend IS the locked Cloud - // host; otherwise the user must be routed through the Cloud login / - // replacement flow. Here the agent-server (stale Local backend) reports - // `llm_api_key_is_set: true` and a configured model, which previously - // made `isBackendLlmReady` true and bypassed onboarding entirely. + // stale Local backend must not bypass onboarding, even when it happens + // to report a configured LLM. The user must be routed through the Cloud + // login / replacement flow instead. vi.stubEnv("VITE_LOCK_TO_CLOUD", "https://app.all-hands.dev"); vi.stubEnv("VITE_SESSION_API_KEY", ""); delete (window as unknown as Record) @@ -257,7 +326,7 @@ describe("App root agent-server availability guard", () => { expect( screen.queryByTestId("manage-backends-modal"), ).not.toBeInTheDocument(); - // The ready-backend fast-path must NOT have persisted completion. + // Backend readiness must NOT persist onboarding completion. expect( window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), ).toBeNull(); @@ -367,6 +436,7 @@ describe("App root agent-server availability guard", () => { }); it("shows the manage-backends modal when the connected server reports an old version", async () => { + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); server.use( http.get("/server_info", () => HttpResponse.json({ uptime: 0, idle_time: 0, version: "1.27.1" }), @@ -388,6 +458,7 @@ describe("App root agent-server availability guard", () => { }); it("shows the manage-backends modal when the server omits a version field", async () => { + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); server.use( http.get("/server_info", () => HttpResponse.json({ uptime: 0, idle_time: 0 }), @@ -406,6 +477,7 @@ describe("App root agent-server availability guard", () => { it("shows the manage-backends modal when the backend is unreachable", async () => { let serverInfoRequests = 0; + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); // Use "*" prefix to match both relative paths and absolute URLs (e.g., // http://127.0.0.1:8000/server_info) when VITE_BACKEND_BASE_URL is configured. @@ -456,6 +528,7 @@ describe("App root agent-server availability guard", () => { "openhands-active-backend", JSON.stringify({ backendId: cloudBackend.id, orgId: null }), ); + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); __resetActiveStoreForTests(); server.use( http.get("https://app.all-hands.dev/api/keys/current", () => @@ -479,6 +552,8 @@ describe("App root agent-server availability guard", () => { }); it("renders the routed page when the agent server is reachable", async () => { + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); + renderApp(["/"]); await waitFor(() => { @@ -490,7 +565,7 @@ describe("App root agent-server availability guard", () => { ).not.toBeInTheDocument(); }); - it("does not mark onboarding complete for the launcher-seeded default-local backend even when the agent-server reports a configured LLM", async () => { + it("shows first-run onboarding for the launcher-seeded default-local backend even when the agent-server reports a configured LLM", async () => { // Regression for mock-llm-onboarding-regressions.spec.ts:16 // ("keeps the modal open on backdrop click and Escape") and // mock-llm-auth-modes.spec.ts:57 ("reaches the onboarding modal @@ -498,9 +573,7 @@ describe("App root agent-server availability guard", () => { // agent-server retains a previously-configured LLM across browser // sessions, so a genuinely fresh browser install (launcher-seeded // default-local backend, no `openhands-onboarded` flag) must NOT - // have onboarding auto-marked complete by the returning-user - // fast-path. The settings-based LLM-ready signal is unreliable for - // the launcher-seeded default backend and must be suppressed there. + // have onboarding auto-marked complete by backend readiness. vi.stubEnv("VITE_BACKEND_BASE_URL", "http://127.0.0.1:8000"); vi.stubEnv("VITE_SESSION_API_KEY", "test-session-key"); // The launcher-seeded default-local backend (id @@ -524,10 +597,12 @@ describe("App root agent-server availability guard", () => { renderApp(["/"]); await waitFor(() => { - expect(screen.getByTestId("app-outlet")).toBeInTheDocument(); + expect( + screen.getByTestId("first-run-onboarding-screen"), + ).toBeInTheDocument(); }); + expect(screen.queryByTestId("app-outlet")).not.toBeInTheDocument(); - // The returning-user fast-path must NOT have persisted completion. expect( window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), ).toBeNull(); @@ -544,8 +619,8 @@ describe("App root agent-server availability guard", () => { // post-login state (active locked Cloud backend + completion flag // set by the modal's onClose) with the Cloud settings probe // reporting NO configured LLM, which is exactly the window where - // the old gate (`!isActiveLockedCloudBackend || !backendLlmReady`) - // kept the first-run screen mounted and caused the reopen. + // the old LLM-readiness gate kept the first-run screen mounted and + // caused the reopen. vi.stubEnv("VITE_LOCK_TO_CLOUD", "https://app.all-hands.dev"); vi.stubEnv("VITE_SESSION_API_KEY", ""); delete (window as unknown as Record) @@ -570,10 +645,9 @@ describe("App root agent-server availability guard", () => { // resolves. Seed it to reproduce the post-login moment. window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); __resetActiveStoreForTests(); - // Cloud settings probe reports no configured LLM. This is the - // critical condition: the old gate treated `!backendLlmReady` as - // "keep showing first-run onboarding" even though the user had - // just finished Cloud login, so the modal reappeared. + // Cloud settings probe reports no configured LLM. The completed + // onboarding flag should still hide first-run onboarding once the + // locked Cloud backend is active. server.use( http.get("https://app.all-hands.dev/api/v1/settings", () => HttpResponse.json({ llm_api_key_set: false }), diff --git a/src/components/features/automations/recommended-automations-launcher.tsx b/src/components/features/automations/recommended-automations-launcher.tsx index dda3fee479..6c4e9361c6 100644 --- a/src/components/features/automations/recommended-automations-launcher.tsx +++ b/src/components/features/automations/recommended-automations-launcher.tsx @@ -110,8 +110,8 @@ export function RecommendedAutomationsLauncher({ draftMessage: prompt, }); } - onLaunched?.(); navigate?.(`/conversations/${conversation.conversation_id}`); + onLaunched?.(); window.setTimeout(() => setMessageToSend(prompt), 0); }, onError: () => { diff --git a/src/components/features/onboarding/is-backend-llm-ready.ts b/src/components/features/onboarding/is-backend-llm-ready.ts deleted file mode 100644 index 50b7594368..0000000000 --- a/src/components/features/onboarding/is-backend-llm-ready.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { SEEDED_DEFAULT_BACKEND_ID } from "#/api/backend-registry/default-backend"; -import type { Backend } from "#/api/backend-registry/types"; -import type { Settings } from "#/types/settings"; - -/** - * `true` when the active backend already has a ready-to-use LLM: - * * `agent_settings.llm.model` is a non-empty string, AND - * * the backend reports an API key on file OR the model uses - * subscription auth (no key required). - * - * Cloud surfaces this via `llm_api_key_set`; the agent-server - * surfaces it via `llm_api_key_is_set` — we accept either, so the - * same skip rule applies in both modes. A truly fresh agent-server - * with no key configured reports both flags as `false` and the - * modal continues to show. - * - * For Local backends the skip is intentionally suppressed for the - * launcher-seeded default backend (`SEEDED_DEFAULT_BACKEND_ID`). The - * agent-server can be started with an env-injected LLM key, and in - * shared-server deployments (e.g. the mock-LLM E2E stack) a - * previously-configured LLM persists across browser sessions — so - * keying the first-run onboarding modal off the server's LLM state - * would suppress onboarding for a genuinely fresh browser install. - * The skip still fires for Local backends the user explicitly added - * via "Add Backend" (which carry a different id), preserving the - * "don't walk a pre-configured server through Set Up LLM" behavior. - * - * Extracted into its own module (no UI imports) so `root.tsx` can - * reuse the exact same rule for its first-run gate without pulling - * the onboarding modal graph into the root's eager bundle. - */ -export function isBackendLlmReady( - backend: Backend, - settings: Settings | undefined, -): boolean { - const llm = settings?.agent_settings?.llm as - | { model?: unknown; auth_type?: unknown } - | undefined; - const hasModel = typeof llm?.model === "string" && llm.model.length > 0; - const isAuthed = - settings?.llm_api_key_set === true || - settings?.llm_api_key_is_set === true || - llm?.auth_type === "subscription"; - if (!hasModel || !isAuthed) return false; - if (backend.kind === "local" && backend.id === SEEDED_DEFAULT_BACKEND_ID) { - return false; - } - return true; -} diff --git a/src/components/features/onboarding/onboarding-host.tsx b/src/components/features/onboarding/onboarding-host.tsx index 3cf147c81f..d40f7ce42a 100644 --- a/src/components/features/onboarding/onboarding-host.tsx +++ b/src/components/features/onboarding/onboarding-host.tsx @@ -1,4 +1,3 @@ -import React from "react"; import { useLocation } from "react-router"; import { OnboardingModal } from "./onboarding-modal"; import { @@ -6,9 +5,6 @@ import { readOnboardingPreviewStep, } from "./onboarding-preview"; import { useOnboardingCompletion } from "./use-onboarding-completion"; -import { useSettings } from "#/hooks/query/use-settings"; -import { useActiveBackend } from "#/contexts/active-backend-context"; -import { isBackendLlmReady } from "./is-backend-llm-ready"; /** * Mounts the onboarding modal automatically the first time the user @@ -16,19 +12,9 @@ import { isBackendLlmReady } from "./is-backend-llm-ready"; * isn't set yet). Closing or completing the flow marks it done so the * modal won't re-appear on subsequent visits. * - * Returning Cloud users are detected via the live settings query: if - * Cloud already reports a configured LLM (api key + model, or a - * subscription model with no key), onboarding is skipped and the - * completion flag is persisted so the same user keeps skipping across - * tabs and devices. - * - * Local backends fall through to the existing localStorage-based - * gating — env-injected keys make the settings-based signal unreliable - * there. - * - * Stale or unreachable backends fall through to the modal so the - * existing backend-check / manage-backends recovery path still kicks - * in for those users. + * Backend readiness is intentionally not treated as onboarding completion: + * a fresh browser/origin should see onboarding once even when it connects + * to an existing backend that already has an LLM configured. * * With `?previewOnboardingStep=<0-3>` the modal opens on that slide for * design review without persisting completion (works on any route when @@ -39,19 +25,9 @@ export function OnboardingHost() { const previewStep = readOnboardingPreviewStep(location.search); const isPreview = isOnboardingPreviewActive(location.search); const { isCompleted, markCompleted } = useOnboardingCompletion(); - const { backend } = useActiveBackend(); - const { data: settings } = useSettings(); - const skipForReadyBackend = isBackendLlmReady(backend, settings); - - React.useEffect(() => { - if (!isPreview && !isCompleted && skipForReadyBackend) { - markCompleted(); - } - }, [isPreview, isCompleted, skipForReadyBackend, markCompleted]); if (!isPreview) { if (isCompleted) return null; - if (skipForReadyBackend) return null; } const handleClose = () => { diff --git a/src/components/features/onboarding/steps/say-hello-step.tsx b/src/components/features/onboarding/steps/say-hello-step.tsx index 63674fea3f..4fa45f2e0e 100644 --- a/src/components/features/onboarding/steps/say-hello-step.tsx +++ b/src/components/features/onboarding/steps/say-hello-step.tsx @@ -57,8 +57,8 @@ export function SayHelloStep({ { query: message.trim() }, { onSuccess: (data) => { - onLaunched(); navigate(`/conversations/${data.conversation_id}`); + onLaunched(); }, onError: () => { launchInFlightRef.current = false; diff --git a/src/root.tsx b/src/root.tsx index ab1055a73a..d6c2129404 100644 --- a/src/root.tsx +++ b/src/root.tsx @@ -6,6 +6,9 @@ import { Outlet, Scripts, ScrollRestoration, + useLocation, + useNavigate, + useNavigation as useRouterNavigation, } from "react-router"; import "./tailwind.css"; import "./index.css"; @@ -32,11 +35,10 @@ import { TOAST_OPTIONS } from "#/utils/custom-toast-handlers"; import { TelemetryConsentBanner } from "#/components/features/analytics/telemetry-consent-banner"; import { LoadingSpinner } from "#/components/shared/loading-spinner"; import { useConfig } from "#/hooks/query/use-config"; -import { useSettings } from "#/hooks/query/use-settings"; import { QUERY_KEYS } from "#/hooks/query/query-keys"; import { AgentServerUIRoot } from "#/components/providers"; import { useOnboardingCompletion } from "#/components/features/onboarding/use-onboarding-completion"; -import { isBackendLlmReady } from "#/components/features/onboarding/is-backend-llm-ready"; +import { NavigationProvider } from "#/context/navigation-context"; import { applyColorTheme, readPersistedColorTheme, @@ -141,14 +143,32 @@ function MissingAgentServerScreen() { ); } function FirstRunOnboardingScreen({ onClose }: { onClose: () => void }) { + const location = useLocation(); + const navigate = useNavigate(); + const routerNavigation = useRouterNavigation(); + const conversationId = + location.pathname.match(/^\/conversations\/([^/]+)/)?.[1] ?? null; + const navigationValue = React.useMemo( + () => ({ + currentPath: location.pathname, + conversationId, + isNavigating: Boolean(routerNavigation.location), + navigate: (to: string, options?: { replace?: boolean }) => + navigate(to, options), + }), + [conversationId, location.pathname, navigate, routerNavigation.location], + ); + return (
- }> - - + + }> + + +
); } @@ -198,32 +218,6 @@ export default function App() { const { isCompleted: onboardingCompleted, markCompleted } = useOnboardingCompletion(); - // Returning-user fast-path: when the active backend already reports a - // ready-to-use LLM (model + key, or subscription auth), skip first-run - // onboarding entirely. Covers two cases: - // * Cloud (settings.llm_api_key_set) — same Cloud account on a new - // origin/browser would otherwise re-trigger the modal because the - // `openhands-onboarded` flag is origin-scoped and starts empty. - // * Local (settings.llm_api_key_is_set) — when the user connects via - // Add Backend to an existing agent-server that already has an LLM - // configured, walking them through Set Up LLM is redundant. - // A truly fresh agent-server (no env-injected key, no saved settings) - // reports both flags as false and the modal still shows normally. - // - // The skip is intentionally suppressed for the launcher-seeded default - // Local backend (`SEEDED_DEFAULT_BACKEND_ID`): the agent-server can be - // started with an env-injected LLM key, and shared-server deployments - // (e.g. the mock-LLM E2E stack) retain configured LLMs across browser - // sessions, so keying first-run onboarding off the server's LLM state - // would suppress the modal (and persist `openhands-onboarded`) for a - // genuinely fresh browser install. The shared helper is the same one - // `OnboardingHost` uses, so the two gates stay in sync. - const { data: activeBackendSettings } = useSettings(); - const backendLlmReady = isBackendLlmReady( - active.backend, - activeBackendSettings, - ); - // In locked-to-Cloud mode the `openhands-onboarded` localStorage flag is // not trustworthy: it may have been set during a previous non-locked // session on the same origin, and origin-scoped localStorage cannot tell @@ -233,61 +227,16 @@ export default function App() { // the user on the Manage Backends recovery modal ("Add Backend") in locked // mode. // - // The ready-backend fast-path is additionally restricted in locked mode: - // it may only skip onboarding when the active backend IS the locked Cloud - // host. A reachable stale Local backend (or a Cloud backend on a different - // host) that happens to report a configured LLM must NOT bypass the Cloud - // login/replacement flow — otherwise the user continues as Local despite - // `VITE_LOCK_TO_CLOUD`. - // // Once the active backend IS the locked Cloud host, a Cloud login that // just succeeded (markCompleted fired via the onboarding modal's onClose) - // must hide first-run onboarding immediately — without waiting for the - // Cloud settings probe to confirm a configured LLM. Waiting caused the - // PR #1389 flicker: the modal advanced to Choose Agent, then the root - // gate tore it down, then OnboardingHost remounted it. Treating + // must hide first-run onboarding immediately. Treating // `onboardingCompleted` as authoritative once the locked Cloud backend is - // active suppresses the reopen. (The flag is only honored when the active - // backend really is the locked Cloud host, so the stale-flag bypass + // active suppresses reopen flicker. (The flag is only honored when the + // active backend really is the locked Cloud host, so the stale-flag bypass // concerns above don't apply here.) - const shouldShowFirstRunOnboarding = isLockedToCloud - ? !isActiveLockedCloudBackend || (!backendLlmReady && !onboardingCompleted) - : authMissing && !onboardingCompleted && !backendLlmReady; - const [showFirstRunOnboarding, setShowFirstRunOnboarding] = React.useState( - () => shouldShowFirstRunOnboarding, - ); - - React.useEffect(() => { - if (shouldShowFirstRunOnboarding) { - setShowFirstRunOnboarding(true); - return; - } - - if (onboardingCompleted || backendLlmReady) { - setShowFirstRunOnboarding(false); - } - }, [onboardingCompleted, shouldShowFirstRunOnboarding, backendLlmReady]); - - // Persist completion once we observe a returning user with a ready LLM, - // so future first renders short-circuit immediately (before settings - // load) and the modal never flashes on a reload. - // - // In locked-to-Cloud mode this must only fire for the legitimate locked - // Cloud host: a stale Local backend (or a Cloud backend on a different - // host) that happens to report a configured LLM must NOT be treated as - // "onboarding complete" — the user is being routed through the Cloud - // login/replacement flow, not skipped past it. - React.useEffect(() => { - if (!backendLlmReady || onboardingCompleted) return; - if (isLockedToCloud && !isActiveLockedCloudBackend) return; - markCompleted(); - }, [ - backendLlmReady, - onboardingCompleted, - markCompleted, - isLockedToCloud, - isActiveLockedCloudBackend, - ]); + const showFirstRunOnboarding = isLockedToCloud + ? !isActiveLockedCloudBackend || !onboardingCompleted + : !onboardingCompleted; // Skip the /server_info probe entirely when we already know auth is // required and missing — it would just 401 and waste time. Also keep the diff --git a/tests/e2e/mock-llm/backends/mock-llm-auth-modes.spec.ts b/tests/e2e/mock-llm/backends/mock-llm-auth-modes.spec.ts index f857877f99..7d08d9f113 100644 --- a/tests/e2e/mock-llm/backends/mock-llm-auth-modes.spec.ts +++ b/tests/e2e/mock-llm/backends/mock-llm-auth-modes.spec.ts @@ -343,6 +343,7 @@ test.describe("auth mode: public gate", () => { }, ]), ); + window.localStorage.setItem("openhands-onboarded", "1"); }, { staleKey: STALE_KEY, host: PUBLIC_MODE_URL }, ); diff --git a/tests/e2e/mock-llm/backends/mock-llm-cross-connect.spec.ts b/tests/e2e/mock-llm/backends/mock-llm-cross-connect.spec.ts index f14305c43a..401f2cb8f8 100644 --- a/tests/e2e/mock-llm/backends/mock-llm-cross-connect.spec.ts +++ b/tests/e2e/mock-llm/backends/mock-llm-cross-connect.spec.ts @@ -151,43 +151,29 @@ async function suppressAnalytics(page: Page) { }); } -/** - * Add a backend through the Manage Backends modal. - * - * Assumes the modal is already visible (shown automatically when no - * backend is available). Clicks "+ Add Backend", fills in the form, - * and submits. - */ -async function addBackendViaModal( +async function addBackendViaOnboarding( page: Page, opts: { name: string; host: string; apiKey: string }, ) { - // Click "+ Add Backend" button inside the manage-backends modal - await page.getByTestId("manage-backends-add").click(); - - // The AddBackendFormModal should appear - await expect(page.getByTestId("add-backend-modal")).toBeVisible({ - timeout: 5_000, + await expect(page.getByTestId("onboarding-step-check-backend")).toBeVisible({ + timeout: 15_000, }); - // Fill in the backend details - const nameInput = page.getByTestId("add-backend-name"); + const nameInput = page.getByTestId("onboarding-backend-name"); await nameInput.click(); await nameInput.fill(opts.name); - const hostInput = page.getByTestId("add-backend-host"); + const hostInput = page.getByTestId("onboarding-backend-host"); await hostInput.click(); await hostInput.fill(opts.host); - const keyInput = page.getByTestId("add-backend-api-key"); + const keyInput = page.getByTestId("onboarding-backend-api-key"); await keyInput.click(); await keyInput.fill(opts.apiKey); - // Submit — validates connection then saves - await page.getByTestId("add-backend-submit").click(); + await page.getByTestId("onboarding-backend-next").click(); - // Wait for the add-backend modal to close (means connection succeeded) - await expect(page.getByTestId("add-backend-modal")).not.toBeVisible({ + await expect(page.getByTestId("onboarding-step-choose-agent")).toBeVisible({ timeout: 15_000, }); } @@ -270,25 +256,24 @@ test.describe("cross-connect: frontend-only → backend-only", () => { await suppressAnalytics(page); await page.goto(feUrl, { waitUntil: "domcontentloaded" }); - // The app detects no reachable backend and shows the Manage Backends - // modal (MissingAgentServerScreen path). - await expect(page.getByTestId("manage-backends-modal")).toBeVisible({ - timeout: 15_000, + // First-run onboarding owns the initial backend collection before + // the Manage Backends recovery screen is allowed to appear. + await expect(page.getByTestId("manage-backends-modal")).not.toBeVisible({ + timeout: 1_000, }); - // ── 5. Add the backend-only instance via the modal ──────────────── - await addBackendViaModal(page, { + // ── 5. Add the backend-only instance via onboarding ─────────────── + await addBackendViaOnboarding(page, { name: "Remote Backend", host: beUrl, apiKey: beEnv.sessionKey, }); + await page.getByTestId("onboarding-skip").click(); // ── 6. Reload to pick up the new backend ────────────────────────── - // After adding a backend through the MissingAgentServerScreen modal, - // root.tsx's useConfig still holds the cached AgentServerUnavailable - // error (retries are disabled for that error class). A page reload - // re-evaluates everything from scratch: getEffectiveLocalBackend() - // now returns the newly added backend, and useConfig probes it. + // After adding a backend while the root first-run gate is active, + // reload once to re-evaluate the root bootstrap against the newly + // persisted active backend. await page.reload({ waitUntil: "domcontentloaded" }); await dismissAnalyticsModal(page); @@ -411,24 +396,20 @@ test.describe("cross-connect: frontend-only → multiple backends", () => { await suppressAnalytics(page); await page.goto(feUrl, { waitUntil: "domcontentloaded" }); - // The manage-backends modal should appear (no backend configured). - await expect(page.getByTestId("manage-backends-modal")).toBeVisible({ - timeout: 15_000, + // The first-run onboarding backend step should appear before the + // Manage Backends recovery modal. + await expect(page.getByTestId("manage-backends-modal")).not.toBeVisible({ + timeout: 1_000, }); - // ── 5. Add Backend A ────────────────────────────────────────────── - await addBackendViaModal(page, { + // ── 5. Add Backend A through onboarding ────────────────────────── + await addBackendViaOnboarding(page, { name: "Backend A", host: beUrlA, apiKey: beEnvA.sessionKey, }); - // Reload to pick up the new backend (same reason as single-backend - // test: useConfig caches the AgentServerUnavailableError). - // Also mark onboarding as done so we land on the home page. - await page.evaluate(() => { - window.localStorage.setItem("openhands-onboarded", "1"); - }); + await page.getByTestId("onboarding-skip").click(); await page.reload({ waitUntil: "domcontentloaded" }); await dismissAnalyticsModal(page); await expect(page.getByTestId("home-chat-launcher")).toBeVisible({