From 0d64ab158c1e0a995d8a4abe26213b865650c9d0 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 21 Jul 2026 11:10:37 -0700 Subject: [PATCH] fix(desktop): keep model settings populated when onboarding reopens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Going Back from the first-community screen reopens machine onboarding directly on the "Configure your default model settings" page, but that remount reset the harness selection made on the setup step — it lived only in MachineOnboardingFlow state. DefaultConfigStep filtered the runtime catalog against the now-empty selection, leaving the default harness dropdown with no value and no options, and the model fields empty. Persist the setup step's harness selection to localStorage whenever it is saved, restore it when the flow reopens on the config page, and fall back to listing every installed harness (instead of an empty dropdown) for installs that completed onboarding before the selection was persisted. The config surface now also shows its error state whenever no runtime resolves after loading, rather than only when a selection was present. Covered by unit tests for the new storage helpers and by extending the back-navigation Playwright spec to assert the dropdowns stay populated (plus a new spec for the no-stored-selection fallback). Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../onboarding/ui/DefaultConfigStep.tsx | 27 ++++-- .../onboarding/ui/MachineOnboardingFlow.tsx | 9 +- .../src/features/onboarding/ui/SetupStep.tsx | 5 +- .../ui/onboardingRuntimeSelection.test.mjs | 85 +++++++++++++++++++ .../ui/onboardingRuntimeSelection.ts | 42 +++++++++ .../e2e/onboarding-agent-defaults.spec.ts | 58 +++++++++++++ 6 files changed, 212 insertions(+), 14 deletions(-) diff --git a/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx b/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx index 68a01f053d..ca5defd625 100644 --- a/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx +++ b/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx @@ -25,7 +25,10 @@ import { type OnboardingTransitionDirection, OnboardingSlideTransition, } from "./OnboardingSlideTransition"; -import { ONBOARDING_RUNTIME_ORDER } from "./onboardingRuntimeSelection"; +import { + ONBOARDING_RUNTIME_ORDER, + runtimeIsInstalled, +} from "./onboardingRuntimeSelection"; import type { DefaultConfigStepActions } from "./types"; type DefaultConfigStepProps = { @@ -115,10 +118,19 @@ function AgentDefaultsSection({ }; }, []); - const selectedRuntimes = React.useMemo( - () => sortSelectedRuntimes(runtimesQuery.data ?? [], selectedRuntimeIds), - [runtimesQuery.data, selectedRuntimeIds], - ); + const selectedRuntimes = React.useMemo(() => { + const catalog = runtimesQuery.data ?? []; + if (selectedRuntimeIds.length > 0) { + return sortSelectedRuntimes(catalog, selectedRuntimeIds); + } + // Reopening onboarding on this page can land here with no recorded + // harness selection (installs that predate selection persistence). Fall + // back to every installed harness rather than an empty dropdown. + return sortSelectedRuntimes( + catalog, + catalog.filter(runtimeIsInstalled).map((runtime) => runtime.id), + ); + }, [runtimesQuery.data, selectedRuntimeIds]); const selectedRuntime = React.useMemo(() => { const preferredRuntime = selectedRuntimes.find( (runtime) => runtime.id === config.preferred_runtime, @@ -129,10 +141,7 @@ function AgentDefaultsSection({ selectedRuntime?.id ?? config.preferred_runtime ?? ""; const configSurfaceLoading = isLoading || runtimesQuery.isLoading; const configSurfaceError = - runtimesQuery.isError || - (!configSurfaceLoading && - selectedRuntimeIds.length > 0 && - !selectedRuntime); + runtimesQuery.isError || (!configSurfaceLoading && !selectedRuntime); const harnessOptions = React.useMemo( () => selectedRuntimes.map((runtime) => ({ diff --git a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx index dfd4e03045..854d1ab4e0 100644 --- a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx +++ b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx @@ -25,7 +25,9 @@ import { import { OnboardingFooterProvider } from "./OnboardingFooter"; import { getPreferredRuntimeIdForSelection, + loadStoredOnboardingRuntimeSelection, runtimeSelectionNeedsDefaultsStep, + storeOnboardingRuntimeSelection, } from "./onboardingRuntimeSelection"; import { OnboardingSlideTransition } from "./OnboardingSlideTransition"; import { SetupStep } from "./SetupStep"; @@ -60,7 +62,11 @@ export function MachineOnboardingFlow({ null, ); const [selectedRuntimeIds, setSelectedRuntimeIds] = React.useState( - [], + // Reopening onboarding directly on the config page (Back from the + // first-community screen) skips the setup step that normally populates + // the selection — restore the one the setup step last saved. + () => + initialPage === "config" ? loadStoredOnboardingRuntimeSelection() : [], ); const [isRuntimeSelectionSaving, setIsRuntimeSelectionSaving] = React.useState(false); @@ -78,6 +84,7 @@ export function MachineOnboardingFlow({ const sequence = runtimeSaveSequence.current + 1; runtimeSaveSequence.current = sequence; setSelectedRuntimeIds(nextRuntimeIds); + storeOnboardingRuntimeSelection(nextRuntimeIds); setRuntimeSelectionError(null); setIsRuntimeSelectionSaving(true); diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 9307e4dc99..98a3d29449 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -22,6 +22,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; import { runtimeCanAdvanceOnboarding, runtimeCanBeSelected, + runtimeIsInstalled, } from "./onboardingRuntimeSelection"; import { ONBOARDING_PRIMARY_CTA_CLASS } from "./OnboardingChrome"; import { RuntimeErrorTooltip } from "./RuntimeErrorTooltip"; @@ -104,10 +105,6 @@ function RuntimeSelectionIndicator({ ); } -function runtimeIsInstalled(runtime: AcpRuntimeCatalogEntry) { - return runtimeCanBeSelected(runtime) && runtimeCanAdvanceOnboarding(runtime); -} - function useSetupFlashState(setupFlashToken: number) { const [isFlashing, setIsFlashing] = React.useState(false); diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs index 046cca93a0..6d342bf07d 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs @@ -4,16 +4,33 @@ import test from "node:test"; import { getDefaultModelConfigRuntimeId, getPreferredRuntimeIdForSelection, + loadStoredOnboardingRuntimeSelection, runtimeCanAdvanceOnboarding, runtimeCanBeSelected, + runtimeIsInstalled, runtimeSelectionNeedsDefaultModelConfig, runtimeSelectionNeedsDefaultsStep, + storeOnboardingRuntimeSelection, } from "./onboardingRuntimeSelection.ts"; function runtime(id, availability, status) { return { id, availability, authStatus: { status } }; } +function createMemoryStorage(initial = {}) { + const values = new Map(Object.entries(initial)); + return { + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => values.set(key, String(value)), + removeItem: (key) => values.delete(key), + clear: () => values.clear(), + key: (index) => Array.from(values.keys())[index] ?? null, + get length() { + return values.size; + }, + }; +} + test("known onboarding harnesses can be selected regardless of setup state", () => { for (const id of ["claude", "codex"]) { assert.equal( @@ -133,3 +150,71 @@ test("any harness selection drives the defaults step", () => { assert.equal(runtimeSelectionNeedsDefaultsStep(["claude", "codex"]), true); assert.equal(runtimeSelectionNeedsDefaultsStep(["goose"]), true); }); + +test("installed means selectable and set up", () => { + assert.equal( + runtimeIsInstalled(runtime("claude", "available", "logged_in")), + true, + ); + assert.equal( + runtimeIsInstalled(runtime("claude", "available", "logged_out")), + false, + ); + assert.equal( + runtimeIsInstalled(runtime("custom", "available", "logged_in")), + false, + ); +}); + +test("runtime selection round-trips through storage", () => { + const storage = createMemoryStorage(); + storeOnboardingRuntimeSelection(["claude", "buzz-agent"], storage); + assert.deepEqual(loadStoredOnboardingRuntimeSelection(storage), [ + "claude", + "buzz-agent", + ]); +}); + +test("storing overwrites the previous runtime selection", () => { + const storage = createMemoryStorage(); + storeOnboardingRuntimeSelection(["claude", "codex"], storage); + storeOnboardingRuntimeSelection(["goose"], storage); + assert.deepEqual(loadStoredOnboardingRuntimeSelection(storage), ["goose"]); +}); + +test("loading with nothing stored returns an empty selection", () => { + assert.deepEqual( + loadStoredOnboardingRuntimeSelection(createMemoryStorage()), + [], + ); +}); + +test("corrupt or non-array stored selections are treated as empty", () => { + assert.deepEqual( + loadStoredOnboardingRuntimeSelection( + createMemoryStorage({ + "buzz-machine-onboarding-runtime-selection.v1": "{not json", + }), + ), + [], + ); + assert.deepEqual( + loadStoredOnboardingRuntimeSelection( + createMemoryStorage({ + "buzz-machine-onboarding-runtime-selection.v1": '{"claude":true}', + }), + ), + [], + ); +}); + +test("non-string entries are dropped from a stored selection", () => { + const storage = createMemoryStorage({ + "buzz-machine-onboarding-runtime-selection.v1": + '["claude", 7, null, "goose"]', + }); + assert.deepEqual(loadStoredOnboardingRuntimeSelection(storage), [ + "claude", + "goose", + ]); +}); diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts index 3ff78e9b5f..7ea8e2ce6a 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts @@ -1,3 +1,4 @@ +import { setLocalStorageItemWithRecovery } from "@/shared/lib/localStorageQuota"; import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; export const ONBOARDING_RUNTIME_ORDER = [ @@ -7,6 +8,43 @@ export const ONBOARDING_RUNTIME_ORDER = [ "buzz-agent", ]; +const RUNTIME_SELECTION_STORAGE_KEY = + "buzz-machine-onboarding-runtime-selection.v1"; + +/** + * Restores the harness selection saved by the setup step. Machine onboarding + * can be reopened directly on the config page (Back from the first-community + * screen), which skips the setup step that normally populates the selection. + */ +export function loadStoredOnboardingRuntimeSelection( + storage: Storage = localStorage, +): string[] { + try { + const raw = storage.getItem(RUNTIME_SELECTION_STORAGE_KEY); + if (!raw) return []; + const parsed: unknown = JSON.parse(raw); + if (!Array.isArray(parsed)) return []; + return parsed.filter( + (runtimeId): runtimeId is string => typeof runtimeId === "string", + ); + } catch { + return []; + } +} + +/** Persists the setup step's harness selection for config-page reopens. */ +export function storeOnboardingRuntimeSelection( + runtimeIds: readonly string[], + storage: Storage = localStorage, +): void { + const serialized = JSON.stringify(runtimeIds); + if (typeof localStorage !== "undefined" && storage === localStorage) { + setLocalStorageItemWithRecovery(RUNTIME_SELECTION_STORAGE_KEY, serialized); + } else { + storage.setItem(RUNTIME_SELECTION_STORAGE_KEY, serialized); + } +} + const KNOWN_ONBOARDING_RUNTIME_IDS = new Set(ONBOARDING_RUNTIME_ORDER); export function runtimeUsesDefaultModelConfig(runtimeId: string) { @@ -66,3 +104,7 @@ export function runtimeCanAdvanceOnboarding(runtime: AcpRuntimeCatalogEntry) { runtime.authStatus.status === "not_applicable") ); } + +export function runtimeIsInstalled(runtime: AcpRuntimeCatalogEntry) { + return runtimeCanBeSelected(runtime) && runtimeCanAdvanceOnboarding(runtime); +} diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 4d1a293ecf..6abc9724c4 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -1289,6 +1289,64 @@ test("community setup back button returns to agent defaults", async ({ await page.getByTestId("welcome-setup-back").click(); await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + + // Regression: reopening remounts the onboarding flow, which used to lose + // the setup step's harness selection — both dropdowns came back empty. + const harnessSelect = page.getByTestId("global-agent-default-harness"); + await expect(harnessSelect).toHaveText("Buzz"); + await harnessSelect.click(); + await expect( + page.getByTestId("global-agent-default-harness-option-buzz-agent"), + ).toBeVisible(); + // The restored selection is exact — installed-but-unselected harnesses + // (the fallback list) must not appear. + await expect( + page.getByTestId("global-agent-default-harness-option-goose"), + ).toHaveCount(0); + await page.keyboard.press("Escape"); + await expect(page.locator("#global-agent-provider")).toBeVisible(); + await expect(page.locator("#global-agent-model")).toBeVisible(); +}); + +test("returning to agent defaults without a stored selection lists installed harnesses", async ({ + page, +}) => { + await installMockBridge(page, undefined, { + skipCommunitySeed: true, + skipOnboardingSeed: true, + }); + await page.goto("/"); + + await navigateToConfigPage(page); + await page.getByTestId("onboarding-finish").click(); + await expect(page.getByText("Join or create a community")).toBeVisible(); + + // Installs that completed onboarding before selection persistence shipped + // have no stored selection to restore. + await page.evaluate(() => + window.localStorage.removeItem( + "buzz-machine-onboarding-runtime-selection.v1", + ), + ); + + await page.getByTestId("welcome-setup-back").click(); + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + + // Falls back to every installed harness instead of an empty dropdown; the + // persisted preferred runtime stays selected. + const harnessSelect = page.getByTestId("global-agent-default-harness"); + await expect(harnessSelect).toHaveText("Buzz"); + await harnessSelect.click(); + await expect( + page.getByTestId("global-agent-default-harness-option-buzz-agent"), + ).toBeVisible(); + await expect( + page.getByTestId("global-agent-default-harness-option-goose"), + ).toBeVisible(); + await expect( + page.getByTestId("global-agent-default-harness-option-claude"), + ).toHaveCount(0); + await page.keyboard.press("Escape"); }); // ---------------------------------------------------------------------------