mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 13:18:19 +08:00
[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 <codex@openai.com>
This commit is contained in:
committed by
GitHub
co-authored by
Codex
parent
65040758ab
commit
09d56a330b
@@ -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 () => {
|
||||
|
||||
+102
-28
@@ -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: () => (
|
||||
<div data-testid="onboarding-modal">
|
||||
<div data-testid="onboarding-step-check-backend" />
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
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: () => <div data-testid="app-outlet">app outlet</div>,
|
||||
path: "/",
|
||||
},
|
||||
{
|
||||
Component: () => (
|
||||
<div data-testid="conversation-outlet">conversation outlet</div>
|
||||
),
|
||||
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<string, unknown>)
|
||||
.__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<string, unknown>)
|
||||
@@ -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<string, unknown>)
|
||||
@@ -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 }),
|
||||
|
||||
@@ -110,8 +110,8 @@ export function RecommendedAutomationsLauncher({
|
||||
draftMessage: prompt,
|
||||
});
|
||||
}
|
||||
onLaunched?.();
|
||||
navigate?.(`/conversations/${conversation.conversation_id}`);
|
||||
onLaunched?.();
|
||||
window.setTimeout(() => setMessageToSend(prompt), 0);
|
||||
},
|
||||
onError: () => {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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 = () => {
|
||||
|
||||
@@ -57,8 +57,8 @@ export function SayHelloStep({
|
||||
{ query: message.trim() },
|
||||
{
|
||||
onSuccess: (data) => {
|
||||
onLaunched();
|
||||
navigate(`/conversations/${data.conversation_id}`);
|
||||
onLaunched();
|
||||
},
|
||||
onError: () => {
|
||||
launchInFlightRef.current = false;
|
||||
|
||||
+31
-82
@@ -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 (
|
||||
<main
|
||||
data-testid="first-run-onboarding-screen"
|
||||
className="min-h-screen bg-base"
|
||||
>
|
||||
<React.Suspense fallback={<AgentServerBootstrapLoading />}>
|
||||
<OnboardingModal onClose={onClose} />
|
||||
</React.Suspense>
|
||||
<NavigationProvider value={navigationValue}>
|
||||
<React.Suspense fallback={<AgentServerBootstrapLoading />}>
|
||||
<OnboardingModal onClose={onClose} />
|
||||
</React.Suspense>
|
||||
</NavigationProvider>
|
||||
</main>
|
||||
);
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -343,6 +343,7 @@ test.describe("auth mode: public gate", () => {
|
||||
},
|
||||
]),
|
||||
);
|
||||
window.localStorage.setItem("openhands-onboarded", "1");
|
||||
},
|
||||
{ staleKey: STALE_KEY, host: PUBLIC_MODE_URL },
|
||||
);
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in New Issue
Block a user