From 427488e2fcb6ef9c5eb3d73e8801f8ba1ab2ad85 Mon Sep 17 00:00:00 2001 From: Diem Nguyen Date: Tue, 20 Oct 2026 18:03:48 -0700 Subject: [PATCH] fix(desktop): surface a missing DATABRICKS_HOST instead of loading models forever (#8504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary If the `databricks` / `databricks_v2` provider is selected but `DATABRICKS_HOST` isn't set, the Model field stays on "Loading models..." forever and never shows an error. How it happens: 1. Desktop discovers Databricks models in-process only when `DATABRICKS_HOST` is set. Without it, discovery falls back to `buzz-acp models`. buzz-agent then fails at config time with `DATABRICKS_MODEL required` (model is checked first) or `DATABRICKS_HOST required`. 2. `formatModelDiscoveryErrorStatus` maps those errors, and `BUZZ_AGENT_PROVIDER is required`, to a `null` status. The idea was that the field's Required row would explain the problem. 3. `deriveModelDiscoveryPending` treats "no data and no status" as still pending. So a failed discovery with a `null` status looks like it is still loading. The fix: - `formatModelDiscoveryErrorStatus` now always returns a status, and its return type is no longer nullable. A failed discovery therefore always settles. - A missing Databricks host shows: "Set DATABRICKS_HOST to your Databricks workspace URL under Advanced to load Databricks models." - A missing provider shows a muted "Choose a provider to load models." Builds that bake `DATABRICKS_HOST` in never reach this path. ### Screenshots Settings → Agents → Agent defaults, with Buzz Agent + Databricks v2 selected, no `DATABRICKS_HOST`, and model discovery failing with `DATABRICKS_MODEL required`. Captured from the desktop e2e build with the mock bridge. The only difference between the two builds is `personaModelDiscoveryStatus.ts`. | Before | After | |---|---| | ![Before: Model field stuck on Loading models](https://raw.githubusercontent.com/block/buzz/bbdfc1f8eb91d8f0f5dc6fd98567bc6af62d8117/pr-8504--before.png) | ![After: Model field asks for DATABRICKS_HOST](https://raw.githubusercontent.com/block/buzz/bbdfc1f8eb91d8f0f5dc6fd98567bc6af62d8117/pr-8504--after.png) | ### Related issue None found. Searched open PRs that touch `personaModelDiscoveryStatus` / `usePersonaModelDiscovery`, and issues mentioning "loading models". ### Testing - New jsdom test drives the real `usePersonaModelDiscovery` hook with a rejected `DATABRICKS_MODEL required` discovery. It asserts that loading settles and that the status names `DATABRICKS_HOST`. On `main` the test fails (`modelDiscoveryLoading` stays `true`), and it passes with this change. - Updated unit tests in `personaModelDiscoveryStatus.test.mjs`. - `pnpm test` in `desktop/` passes (6834 + 161 tests). `tsc --noEmit` and biome are clean. - Exercised in the desktop e2e build with the mock bridge (screenshots above). Not yet exercised in the real Tauri app. Signed-off-by: Fizz <6be7be23ec5bba868f3e1db5ec86013d76bd1a293dce5b435a192158fec13641@buzz.block.builderlab.xyz> Co-authored-by: Fizz <6be7be23ec5bba868f3e1db5ec86013d76bd1a293dce5b435a192158fec13641@buzz.block.builderlab.xyz> --- .../ui/personaModelDiscoveryStatus.test.mjs | 25 ++++++++-- .../agents/ui/personaModelDiscoveryStatus.ts | 17 +++++-- .../usePersonaModelDiscovery.jsdom-test.mjs | 50 ++++++++++++++++--- 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs index dfa086738f..2c652c1816 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.test.mjs @@ -60,13 +60,30 @@ test("Buzz shared compute names a missing relay member roster", () => { assert.doesNotMatch(status?.message ?? "", /relay connection/); }); -test("model discovery status stays quiet for missing Databricks defaults", () => { +for (const error of [ + "config: DATABRICKS_HOST required", + "buzz-acp models failed (exit 1): config: DATABRICKS_MODEL required", +]) { + test(`missing Databricks host is named, not swallowed: ${error}`, () => { + const status = formatModelDiscoveryErrorStatus( + new Error(error), + "databricks_v2", + ); + + assert.equal(status.tone, "warning"); + assert.match(status.message, /DATABRICKS_HOST/); + assert.match(status.message, /Advanced/); + }); +} + +test("missing provider asks the user to choose one", () => { const status = formatModelDiscoveryErrorStatus( - new Error("config: DATABRICKS_HOST required"), - "databricks", + new Error("config: BUZZ_AGENT_PROVIDER is required"), + "", ); - assert.equal(status, null); + assert.equal(status.tone, "muted"); + assert.match(status.message, /Choose a provider/); }); test("Databricks sign-in-required is a muted note pointing at the picker and CLI", () => { diff --git a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts index 0d89526425..2ae6fe4ed8 100644 --- a/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts +++ b/desktop/src/features/agents/ui/personaModelDiscoveryStatus.ts @@ -45,7 +45,7 @@ export function formatModelDiscoveryErrorStatus( error: unknown, provider: string, agentLabel?: string, -): PersonaModelDiscoveryStatus | null { +): PersonaModelDiscoveryStatus { const message = errorMessage(error); if (provider.trim() === "relay-mesh") { @@ -116,12 +116,21 @@ export function formatModelDiscoveryErrorStatus( }; } + // Desktop discovers Databricks models in-process whenever DATABRICKS_HOST + // is set, so either error from the harness fallback means the host is missing. if ( message.includes("DATABRICKS_HOST required") || - message.includes("DATABRICKS_MODEL required") || - message.includes("BUZZ_AGENT_PROVIDER is required") + message.includes("DATABRICKS_MODEL required") ) { - return null; + return { + message: + "Set DATABRICKS_HOST to your Databricks workspace URL under Advanced to load Databricks models.", + tone: "warning", + }; + } + + if (message.includes("BUZZ_AGENT_PROVIDER is required")) { + return { message: "Choose a provider to load models.", tone: "muted" }; } // Databricks transparent auth (agent_models_databricks.rs). The backend diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.jsdom-test.mjs b/desktop/src/features/agents/ui/usePersonaModelDiscovery.jsdom-test.mjs index adbc72dc47..0fc6ab77e8 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.jsdom-test.mjs +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.jsdom-test.mjs @@ -1,7 +1,6 @@ /** - * The real discovery hook passes the selected runtime to the label code: - * switching Claude Code -> Codex -> Claude Code relabels the same cached - * discovery response. + * Exercises the real discovery hook: runtime switches relabel the same cached + * response, and a failed discovery always settles instead of loading forever. */ import assert from "node:assert/strict"; import { afterEach, test } from "node:test"; @@ -15,10 +14,11 @@ const discovered = { models: [{ id: "claude-sonnet-5", name: "Sonnet", description: null }], }; const calls = []; +let discover = () => Promise.resolve(discovered); const tauriMock = { invoke(command) { calls.push(command); - if (command === "discover_agent_models") return Promise.resolve(discovered); + if (command === "discover_agent_models") return discover(); return Promise.reject(new Error(`unmocked Tauri command: ${command}`)); }, transformCallback() { @@ -48,8 +48,15 @@ let container; afterEach(() => { act(() => root?.unmount()); container?.remove(); + discover = () => Promise.resolve(discovered); }); +function mountRoot() { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +} + let labels = null; function Probe({ selectedRuntime }) { const { discoveredModelOptions } = usePersonaModelDiscovery({ @@ -71,9 +78,7 @@ async function render(id) { } test("hook relabels discovered models when the runtime switches", async () => { - container = document.createElement("div"); - document.body.appendChild(container); - root = createRoot(container); + mountRoot(); await render("claude"); assert.deepEqual(labels, [ @@ -92,3 +97,34 @@ test("hook relabels discovered models when the runtime switches", async () => { 1, ); }); + +test("a missing Databricks host settles discovery instead of loading forever", async () => { + discover = () => + Promise.reject( + "buzz-acp models failed (exit 1): config: DATABRICKS_MODEL required", + ); + let state = null; + function DatabricksProbe() { + state = usePersonaModelDiscovery({ + envVars, + isCustomProviderEditing: false, + modelFieldVisible: true, + open: true, + provider: "databricks_v2", + selectedRuntime: runtime("buzz-agent"), + }); + return null; + } + mountRoot(); + + await act(async () => { + root.render(React.createElement(DatabricksProbe)); + }); + // databricks_v2 discovery is debounced before the IPC call. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 400)); + }); + + assert.equal(state.modelDiscoveryLoading, false); + assert.match(state.modelDiscoveryStatus?.message ?? "", /DATABRICKS_HOST/); +});