From 5646d47358a2bd172671b92c03de30e9c8bb454c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 21 Jul 2026 11:40:34 -0700 Subject: [PATCH] fix(desktop): stop onboarding fallback from rewriting saved model config Resolve both review findings on the config-page fallback added in 0d64ab15: The installed-only fallback could silently rewrite the user's saved model settings. With no stored harness selection, a persisted preferred runtime that is no longer installed (e.g. Claude logged out since onboarding) was excluded from the fallback list, so the page resolved a different harness and the reconcile effect persisted a reset config on open, with no user interaction. The fallback now also includes the persisted preferred runtime, matching the Settings defaults editor which lists harnesses regardless of install state, so the reconcile effect resolves it as selected and stays a no-op. The fallback also only triggered when the stored selection was empty, so a stored selection whose ids had all dropped out of the runtime catalog landed on the error surface instead. The fallback now triggers whenever the catalog intersection is empty, reserving the error state for query failures or zero usable harnesses. Both cases are covered by new Playwright specs (verified to fail against the previous component): one asserts a logged-out preferred harness stays listed and selected with the saved config untouched, the other asserts a stale stored selection falls back to installed harnesses instead of erroring. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../onboarding/ui/DefaultConfigStep.tsx | 28 +++-- .../e2e/onboarding-agent-defaults.spec.ts | 111 ++++++++++++++++++ 2 files changed, 129 insertions(+), 10 deletions(-) diff --git a/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx b/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx index ca5defd62..ca08e61ec 100644 --- a/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx +++ b/desktop/src/features/onboarding/ui/DefaultConfigStep.tsx @@ -120,17 +120,25 @@ function AgentDefaultsSection({ const selectedRuntimes = React.useMemo(() => { const catalog = runtimesQuery.data ?? []; - if (selectedRuntimeIds.length > 0) { - return sortSelectedRuntimes(catalog, selectedRuntimeIds); + const storedSelection = sortSelectedRuntimes(catalog, selectedRuntimeIds); + if (storedSelection.length > 0) { + return storedSelection; } - // 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]); + // Reopening onboarding on this page can land here with no usable harness + // selection — none was recorded (installs that predate selection + // persistence), or every recorded id has since left the catalog. Fall + // back to every installed harness rather than an empty dropdown, keeping + // the persisted preferred runtime listed even when it is no longer + // installed (e.g. logged out since onboarding) so the reconcile effect + // below resolves it as selected instead of rewriting the saved config. + const fallbackRuntimeIds = catalog + .filter(runtimeIsInstalled) + .map((runtime) => runtime.id); + if (config.preferred_runtime) { + fallbackRuntimeIds.push(config.preferred_runtime); + } + return sortSelectedRuntimes(catalog, fallbackRuntimeIds); + }, [config.preferred_runtime, runtimesQuery.data, selectedRuntimeIds]); const selectedRuntime = React.useMemo(() => { const preferredRuntime = selectedRuntimes.find( (runtime) => runtime.id === config.preferred_runtime, diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 6abc9724c..020efeda0 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -1349,6 +1349,117 @@ test("returning to agent defaults without a stored selection lists installed har await page.keyboard.press("Escape"); }); +test("fallback keeps a no-longer-installed preferred harness without rewriting the saved config", async ({ + page, +}) => { + await installMockBridge( + page, + { + acpRuntimesCatalog: [ + availableRuntime("claude", { status: "logged_out" }), + availableRuntime("goose", { status: "not_applicable" }), + availableRuntime("buzz-agent", { status: "not_applicable" }), + ], + }, + { 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(); + + // Simulate an install that predates selection persistence and whose saved + // preference is a harness that is no longer installed (Claude logged out + // since onboarding). + await page.evaluate(async () => { + window.localStorage.removeItem( + "buzz-machine-onboarding-runtime-selection.v1", + ); + await ( + window as Window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: ( + command: string, + payload: unknown, + ) => Promise; + } + ).__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.("set_global_agent_config", { + config: { + env_vars: {}, + provider: null, + // A model the mock's Claude catalog knows, so onboarding's + // stale-model healing (a separate, intended cleanup) stays inert and + // any config write observed below is the reconcile-effect rewrite. + model: "claude-opus-4-20250514", + preferred_runtime: "claude", + }, + }); + }); + + await page.getByTestId("welcome-setup-back").click(); + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + + // The persisted preferred harness stays listed and selected even though it + // is no longer installed — resolving to another harness here would make the + // reconcile effect silently rewrite the saved config on open. + const harnessSelect = page.getByTestId("global-agent-default-harness"); + await expect(harnessSelect).toHaveText("Claude"); + await harnessSelect.click(); + await expect( + page.getByTestId("global-agent-default-harness-option-claude"), + ).toBeVisible(); + await expect( + page.getByTestId("global-agent-default-harness-option-buzz-agent"), + ).toBeVisible(); + await page.keyboard.press("Escape"); + + const savedConfig = await readSavedConfig(page); + expect(savedConfig?.preferred_runtime).toBe("claude"); + expect(savedConfig?.model).toBe("claude-opus-4-20250514"); +}); + +test("stored selection whose harnesses left the catalog falls back instead of erroring", 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(); + + // A stored selection can go stale — every recorded harness id has since + // dropped out of the runtime catalog (e.g. uninstalled between sessions). + await page.evaluate(() => + window.localStorage.setItem( + "buzz-machine-onboarding-runtime-selection.v1", + JSON.stringify(["retired-harness"]), + ), + ); + + await page.getByTestId("welcome-setup-back").click(); + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + + // The stale selection behaves like a missing one: installed harnesses are + // listed instead of the error surface. + await expect( + page.getByText("Couldn't load harness settings. Go back and try again."), + ).toHaveCount(0); + 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 page.keyboard.press("Escape"); +}); + // --------------------------------------------------------------------------- // B1 regression: rapid consecutive edits must not lose the later change // ---------------------------------------------------------------------------