mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(desktop): surface a missing DATABRICKS_HOST instead of loading models forever (#8504)
## 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 | |---|---| |  |  | ### 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>
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user