diff --git a/ui/src/adapters/use-disabled-adapters.ts b/ui/src/adapters/use-disabled-adapters.ts index d3140fed6d..c1f9467d26 100644 --- a/ui/src/adapters/use-disabled-adapters.ts +++ b/ui/src/adapters/use-disabled-adapters.ts @@ -54,3 +54,34 @@ export function useDisabledAdaptersSync(options: { enabled?: boolean } = {}): Se [adapters], ); } + +/** + * Whether the adapter list has arrived, so callers can tell "this instance + * does not offer that adapter" from "the registry has not loaded yet". + * + * External adapter types are registered into the UI registry by + * {@link useDisabledAdaptersSync} only once the query resolves. Until then + * `listUIAdapters()` returns the built-ins alone, so an external adapter looks + * exactly like one the deployer has disabled. + * + * Deliberately reports arrival rather than settlement, unlike the fail-open + * gates elsewhere in onboarding. The directions of harm are opposite here. A + * caller that acts on an unloaded registry replaces the customer's chosen + * adapter with a built-in and persists that choice; a caller that waits + * forever simply leaves the selection alone, which is the behaviour that + * existed before any of this. Silently changing a saved answer is the error + * worth refusing to make. + * + * Reads the same query key as {@link useDisabledAdaptersSync}, so it shares + * that cache entry rather than adding a request. + */ +export function useAdapterRegistryLoaded(options: { enabled?: boolean } = {}): boolean { + const enabled = options.enabled ?? true; + const { data: adapters } = useQuery({ + queryKey: queryKeys.adapters.all, + queryFn: () => adaptersApi.list(), + enabled, + staleTime: 5 * 60 * 1000, + }); + return adapters !== undefined; +} diff --git a/ui/src/components/OnboardingWizard.adapters.test.tsx b/ui/src/components/OnboardingWizard.adapters.test.tsx new file mode 100644 index 0000000000..5796793f25 --- /dev/null +++ b/ui/src/components/OnboardingWizard.adapters.test.tsx @@ -0,0 +1,191 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// --- Mocks (hoisted so vi.mock factories can close over them) ---------------- + +const ONBOARDING_STORAGE_KEY = "paperclip-onboarding-state"; + +const mockDialog = vi.hoisted(() => ({ + onboardingOpen: true, + onboardingOptions: {} as { initialStep?: number; companyId?: string }, + closeOnboarding: vi.fn(), + onboardingRouteDismissed: false, + setOnboardingRouteDismissed: vi.fn(), +})); + +const mockCompany = vi.hoisted(() => ({ + companies: [] as Array<{ id: string; name: string; issuePrefix: string }>, + setSelectedCompanyId: vi.fn(), + loading: false, +})); + +// The real adapter registry eagerly imports every adapter package. The +// model/harness picker internals are out of scope here, so stub the adapter +// layer entirely and drive the grid through these two knobs. +const mockAdapterRegistry = vi.hoisted(() => ({ + list: [] as Array<{ type: string }>, + disabled: new Set(), + loaded: true, +})); + +vi.mock("@/lib/router", () => ({ + useLocation: () => ({ pathname: "/", search: "", hash: "", state: null }), + useNavigate: () => vi.fn(), + useParams: () => ({}), +})); +vi.mock("../context/DialogContext", () => ({ + useDialog: () => mockDialog, +})); +vi.mock("../context/CompanyContext", () => ({ + useCompany: () => mockCompany, +})); +vi.mock("../adapters", () => ({ + listUIAdapters: () => mockAdapterRegistry.list, + getUIAdapter: () => ({ buildAdapterConfig: () => ({}) }), +})); +vi.mock("../adapters/metadata", () => ({ isVisualAdapterChoice: () => true })); +vi.mock("../adapters/adapter-display-registry", () => ({ + getAdapterDisplay: (type: string) => ({ + type, + recommended: false, + label: type, + description: "", + icon: () => null, + }), +})); +vi.mock("../adapters/use-disabled-adapters", () => ({ + useDisabledAdaptersSync: () => mockAdapterRegistry.disabled, + useAdapterRegistryLoaded: () => mockAdapterRegistry.loaded, +})); +vi.mock("../adapters/use-adapter-capabilities", () => ({ + useAdapterCapabilities: () => () => ({ + supportsInstructionsBundle: false, + supportsSkills: false, + supportsLocalAgentJwt: false, + requiresMaterializedRuntimeSkills: false, + supportsModelProfiles: false, + }), +})); +// Animation / canvas-ish children that add nothing to the logic under test. +vi.mock("./AsciiArtAnimation", () => ({ AsciiArtAnimation: () => null })); +vi.mock("./FrontDoor", () => ({ FrontDoor: () => null })); +vi.mock("./AgentCapsule", () => ({ AgentCapsule: () => null })); + +import { OnboardingWizard } from "./OnboardingWizard"; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +async function flushReact() { + await act(async () => { + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + }); +} + +async function mount() { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + return { container, root }; +} + +describe("OnboardingWizard adapter selection", () => { + beforeEach(() => { + window.localStorage.clear(); + mockDialog.onboardingOpen = true; + mockDialog.onboardingOptions = {}; + mockCompany.companies = []; + mockAdapterRegistry.list = []; + mockAdapterRegistry.disabled = new Set(); + mockAdapterRegistry.loaded = true; + }); + + afterEach(() => { + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + it("snaps a disabled default adapterType to the first enabled adapter", async () => { + // A deployment whose adapter registry omits claude_local disables it, so + // the wizard's claude_local default must not survive as an invisible + // selection (the created agent could never acquire a lease). + mockAdapterRegistry.list = [ + { type: "claude_local" }, + { type: "codex_local" }, + { type: "opencode_local" }, + ]; + mockAdapterRegistry.disabled = new Set(["claude_local"]); + + const { root } = await mount(); + + const saved = JSON.parse( + window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}", + ); + expect(saved.adapterType).toBe("codex_local"); + + await act(async () => { + root.unmount(); + }); + }); + + it("keeps an enabled saved adapterType untouched", async () => { + mockAdapterRegistry.list = [ + { type: "claude_local" }, + { type: "codex_local" }, + ]; + window.localStorage.setItem( + ONBOARDING_STORAGE_KEY, + JSON.stringify({ step: 0, adapterType: "claude_local" }), + ); + + const { root } = await mount(); + + const saved = JSON.parse( + window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}", + ); + expect(saved.adapterType).toBe("claude_local"); + + await act(async () => { + root.unmount(); + }); + }); + it("does not replace a saved adapter before the registry has loaded", async () => { + // External adapter types are only registered once the adapters query + // resolves. Until then `listUIAdapters()` returns the built-ins alone, so + // a saved external adapter looks exactly like a disabled one — and + // snapping would swap the customer's choice for a built-in and persist it. + window.localStorage.setItem( + ONBOARDING_STORAGE_KEY, + JSON.stringify({ step: 0, adapterType: "acme_external" }), + ); + mockAdapterRegistry.loaded = false; + mockAdapterRegistry.list = [{ type: "codex_local" }]; + + const { root } = await mount(); + + const saved = JSON.parse( + window.localStorage.getItem(ONBOARDING_STORAGE_KEY) ?? "{}", + ); + expect(saved.adapterType).toBe("acme_external"); + + await act(async () => { + root.unmount(); + }); + }); +}); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index bdf0bd3c70..3028262ac5 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -26,7 +26,7 @@ import { import { getUIAdapter } from "../adapters"; import { listUIAdapters } from "../adapters"; import { isVisualAdapterChoice } from "../adapters/metadata"; -import { useDisabledAdaptersSync } from "../adapters/use-disabled-adapters"; +import { useDisabledAdaptersSync, useAdapterRegistryLoaded } from "../adapters/use-disabled-adapters"; import { useAdapterCapabilities } from "../adapters/use-adapter-capabilities"; import { getAdapterDisplay } from "../adapters/adapter-display-registry"; import { defaultCreateValues } from "./agent-config-defaults"; @@ -181,6 +181,7 @@ export function OnboardingWizard() { // mounted globally, including on /auth, where protected adapter routes are // expected to reject signed-out browsers. const disabledTypes = useDisabledAdaptersSync({ enabled: effectiveOnboardingOpen }); + const adapterRegistryLoaded = useAdapterRegistryLoaded({ enabled: effectiveOnboardingOpen }); const initialStep = effectiveOnboardingOptions.initialStep ?? 0; const existingCompanyId = effectiveOnboardingOptions.companyId; @@ -452,6 +453,39 @@ export function OnboardingWizard() { }; }, [disabledTypes]); + // The default (or a saved) adapterType can name an adapter the server has + // since disabled — e.g. a cloud sandbox registry without claude_local. The + // grid hides it, so without this snap the wizard would silently keep an + // invisible selection and create an agent that can never acquire a lease. + useEffect(() => { + // Not until the registry has loaded. External adapter types are only + // registered once the adapters query resolves, so before that a saved + // external adapter is indistinguishable from a disabled one - and snapping + // would replace the customer's choice with a built-in and persist it. + if (!adapterRegistryLoaded) return; + const visible = [...recommendedAdapters, ...moreAdapters].filter( + (a) => !a.comingSoon, + ); + if (visible.length === 0) return; + if (visible.some((a) => a.type === adapterType)) return; + const next = visible[0].type as AdapterType; + setAdapterType(next); + if (next === "codex_local") return; + if (next === "opencode_local") { + setModel(DEFAULT_OPENCODE_LOCAL_MODEL); + return; + } + if (next === "gemini_local") { + setModel(DEFAULT_GEMINI_LOCAL_MODEL); + return; + } + if (next === "cursor") { + setModel(DEFAULT_CURSOR_LOCAL_MODEL); + return; + } + setModel(""); + }, [adapterRegistryLoaded, recommendedAdapters, moreAdapters, adapterType]); + const COMMAND_PLACEHOLDERS: Record = { claude_local: "claude", codex_local: "codex",