diff --git a/AGENTS.md b/AGENTS.md index 342c63556..f32254633 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -584,7 +584,7 @@ When adding code that needs a new string, decide up front which rule it falls un - Changes tab / `FileDiffViewer` deleted-file note: the agent-server's `/api/git/diff` endpoint calls `path.exists()` first (see `openhands-sdk/openhands/sdk/git/git_diff.py` → `get_git_diff`), so requesting a diff for a `D` (deleted) file returns `GitPathError` → HTTP 400 and trips the global QueryCache error toast. `useUnifiedGitDiff` disables the query when `type === "D"` and `FileDiffViewer` renders a localized "file deleted" placeholder (`DIFF_VIEWER$FILE_DELETED`, `data-testid="file-deleted-message"`) instead of the view-mode toolbar / Monaco editor for that case. -- Onboarding modal: `src/components/features/onboarding/onboarding-modal.tsx` is rendered by `` and gated by the `openhands-onboarded` localStorage flag. It tracks logical phases (`backend`, `agent`, `setup`, `hello`) rather than fixed numeric steps; the backend phase may be omitted for an already configured backend. Agent choices are derived from `ACP_PROVIDERS` plus OpenHands. The setup phase renders `SetupLlmStep` for OpenHands or `SetupAcpSecretsStep` for ACP providers. Keep the phase-based navigation so adding or removing the backend slide cannot move users to the wrong step. +- Onboarding modal: `src/components/features/onboarding/onboarding-modal.tsx` is rendered by `` and normally gated by the `openhands-onboarded` localStorage flag. `OnboardingHost` also suppresses the modal, without writing that flag, when any active Cloud backend's settings report a usable LLM (non-empty model plus API-key or subscription auth); this readiness exception must not apply to Local backends. It tracks logical phases (`backend`, `agent`, `setup`, `hello`) rather than fixed numeric steps; the backend phase may be omitted for an already configured backend. Agent choices are derived from `ACP_PROVIDERS` plus OpenHands. The setup phase renders `SetupLlmStep` for OpenHands or `SetupAcpSecretsStep` for ACP providers. Keep the phase-based navigation so adding or removing the backend slide cannot move users to the wrong step. - Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached _either_ a repo (`conversation.selected_repository`) _or_ a local workspace (`getStoredConversationMetadata(id).selected_workspace`, persisted by `createConversation` when `workingDirOverride` is supplied). The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace). diff --git a/__tests__/components/onboarding/onboarding-host.test.tsx b/__tests__/components/onboarding/onboarding-host.test.tsx index 790f89205..42860c44b 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 } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; import { MemoryRouter } from "react-router"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -108,43 +108,80 @@ describe("OnboardingHost", () => { ).toBeInTheDocument(); }); - it("shows the modal for a fresh Cloud user even when the backend has a configured LLM", async () => { + it("skips the modal when the active Cloud backend has a configured LLM", async () => { seedCloudBackend(); - vi.spyOn(SettingsService, "getSettings").mockResolvedValue({ - ...DEFAULT_SETTINGS, - llm_api_key_set: true, - agent_settings: { - ...DEFAULT_SETTINGS.agent_settings, - llm: { model: "anthropic/claude-sonnet-4-5", api_key: "stored" }, - }, - }); + const getSettings = vi + .spyOn(SettingsService, "getSettings") + .mockResolvedValue({ + ...DEFAULT_SETTINGS, + llm_api_key_set: true, + agent_settings: { + ...DEFAULT_SETTINGS.agent_settings, + llm: { model: "anthropic/claude-sonnet-4-5", api_key: "stored" }, + }, + }); renderHost(); - expect( - await screen.findByTestId("onboarding-modal-stub"), - ).toBeInTheDocument(); + await waitFor(() => { + expect(getSettings).toHaveBeenCalledOnce(); + expect( + screen.queryByTestId("onboarding-modal-stub"), + ).not.toBeInTheDocument(); + }); expect( window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), ).toBeNull(); }); - it("shows the modal for a fresh Cloud user even when the active LLM uses subscription auth", async () => { + it("skips the modal for a configured Cloud backend that does not match the locked host", async () => { + vi.stubEnv("VITE_LOCK_TO_CLOUD", "https://other-cloud.example.com"); seedCloudBackend(); - vi.spyOn(SettingsService, "getSettings").mockResolvedValue({ - ...DEFAULT_SETTINGS, - llm_api_key_set: false, - agent_settings: { - ...DEFAULT_SETTINGS.agent_settings, - llm: { model: "openai/gpt-5.5", auth_type: "subscription" }, - }, - }); + const getSettings = vi + .spyOn(SettingsService, "getSettings") + .mockResolvedValue({ + ...DEFAULT_SETTINGS, + llm_api_key_set: true, + agent_settings: { + ...DEFAULT_SETTINGS.agent_settings, + llm: { model: "anthropic/claude-sonnet-4-5", api_key: "stored" }, + }, + }); renderHost(); + await waitFor(() => { + expect(getSettings).toHaveBeenCalledOnce(); + expect( + screen.queryByTestId("onboarding-modal-stub"), + ).not.toBeInTheDocument(); + }); expect( - await screen.findByTestId("onboarding-modal-stub"), - ).toBeInTheDocument(); + window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), + ).toBeNull(); + }); + + it("skips the modal when the active Cloud backend uses subscription auth", async () => { + seedCloudBackend(); + const getSettings = vi + .spyOn(SettingsService, "getSettings") + .mockResolvedValue({ + ...DEFAULT_SETTINGS, + llm_api_key_set: false, + agent_settings: { + ...DEFAULT_SETTINGS.agent_settings, + llm: { model: "openai/gpt-5.5", auth_type: "subscription" }, + }, + }); + + renderHost(); + + await waitFor(() => { + expect(getSettings).toHaveBeenCalledOnce(); + expect( + screen.queryByTestId("onboarding-modal-stub"), + ).not.toBeInTheDocument(); + }); expect( window.localStorage.getItem(ONBOARDING_COMPLETED_STORAGE_KEY), ).toBeNull(); diff --git a/src/components/features/onboarding/onboarding-host.tsx b/src/components/features/onboarding/onboarding-host.tsx index d40f7ce42..18d765891 100644 --- a/src/components/features/onboarding/onboarding-host.tsx +++ b/src/components/features/onboarding/onboarding-host.tsx @@ -1,4 +1,8 @@ import { useLocation } from "react-router"; +import { useActiveBackend } from "#/contexts/active-backend-context"; +import { useSettings } from "#/hooks/query/use-settings"; +import { isSubscriptionLlmConfig } from "#/constants/llm-subscription"; +import type { Settings } from "#/types/settings"; import { OnboardingModal } from "./onboarding-modal"; import { isOnboardingPreviewActive, @@ -6,15 +10,27 @@ import { } from "./onboarding-preview"; import { useOnboardingCompletion } from "./use-onboarding-completion"; +function hasUsableCloudLlm(settings: Settings | undefined): boolean { + const llm = settings?.agent_settings?.llm as + | Record + | undefined; + const hasModel = + typeof llm?.model === "string" && llm.model.trim().length > 0; + const hasAuth = + settings?.llm_api_key_set === true || isSubscriptionLlmConfig(llm); + return hasModel && hasAuth; +} + /** * Mounts the onboarding modal automatically the first time the user * lands on a host route (i.e. when the localStorage onboarding flag * isn't set yet). Closing or completing the flow marks it done so the * modal won't re-appear on subsequent visits. * - * 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. + * An already-authenticated Cloud backend may provide everything onboarding + * would collect. Once the active Cloud backend reports a usable LLM, suppress + * the redundant modal without writing a fake completion marker. Local backends + * remain browser-onboarding-driven. * * With `?previewOnboardingStep=<0-3>` the modal opens on that slide for * design review without persisting completion (works on any route when @@ -25,9 +41,16 @@ export function OnboardingHost() { const previewStep = readOnboardingPreviewStep(location.search); const isPreview = isOnboardingPreviewActive(location.search); const { isCompleted, markCompleted } = useOnboardingCompletion(); + const { backend } = useActiveBackend(); + const settings = useSettings(); + const isCloudBackend = backend.kind === "cloud"; if (!isPreview) { if (isCompleted) return null; + if (isCloudBackend && settings.isLoading) return null; + if (isCloudBackend && hasUsableCloudLlm(settings.data)) { + return null; + } } const handleClose = () => {