From 39fa686c27ffa8b21f0e1b2fef310fdcf9e0518e Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Sun, 12 Jul 2026 17:49:57 -0400 Subject: [PATCH 1/2] fix(desktop): correct provider and model handling in agent config dialogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OSS builds lost the Databricks v1 option when PR #1686 removed it unconditionally; it should only be hidden on internal Block builds where a boot migration rewrites v1→v2 (BLOCK_BUILD_HIDDEN_PROVIDER_IDS is now exported and gated on the baked BUZZ_AGENT_PROVIDER key). Builtin persona edit (e.g. Fizz) never seeded a default runtime in edit mode so model discovery never ran and the model dropdown stayed empty. The global provider leaked into codex and claude model discovery, showing Anthropic models for codex agents. The global effort dropdown showed bare "Inherit" on OSS builds where nothing is baked and the provider has a known default. The start path used runtimes[0] (catalog order puts goose first), so an installed goose beat the bundled buzz-agent sidecar for runtime-less personas — both the start path and team-deploy dialog now use the buzz-agent-first preference matching the create-dialog default. Review-round hardening: editing a null-runtime builtin silently discarded the model/provider choice on submit (the auto-seeded runtime failed the editable-without-runtime gate and fell through to preserving the null initial values) — isAutoSeededRuntimeForBuiltinEdit now gates the submit payload, built by a pure buildRuntimeModelProviderPayload helper with unit tests. hasSeededForOpenRef stops the seeding effect re-firing after the user clears runtime to "No preference". usePersonaModelDiscovery's effect deps are narrowed to the consumed fields. E2E specs cover the OSS provider dropdown (Databricks v1 present), the effort select showing "Default (medium)" when nothing is baked, and edit dialogs populating the model combobox via auto-seeding; the mock bridge gains get_baked_build_env / get_baked_build_env_keys handlers. --- desktop/playwright.config.ts | 1 + desktop/scripts/check-file-sizes.mjs | 9 +- desktop/src/features/agents/hooks.ts | 38 ---- .../lib/instanceInputForDefinition.test.mjs | 45 ++++ .../agents/lib/instanceInputForDefinition.ts | 6 +- .../agents/lib/resolvePersonaRuntime.ts | 26 ++- .../agents/ui/AddTeamToChannelDialog.tsx | 13 +- .../agents/ui/AgentDefinitionDialog.tsx | 92 ++++++--- .../agents/ui/AgentInstanceEditDialog.tsx | 17 +- .../ui/agentDefinitionSubmitPayload.test.mjs | 88 ++++++++ .../agents/ui/agentDefinitionSubmitPayload.ts | 75 +++++++ .../agents/ui/buzzAgentModelTuningFields.tsx | 8 +- .../ui/editAgentProviderDiscovery.test.mjs | 52 +++-- .../agents/ui/personaDialogPickers.test.mjs | 187 +++++++++++++++++ .../agents/ui/personaDialogPickers.tsx | 51 +++-- .../agents/ui/usePersonaModelDiscovery.ts | 26 ++- .../ui/GlobalAgentConfigSettingsCard.tsx | 22 ++ desktop/src/testing/e2eBridge.ts | 6 + .../e2e/agent-provider-dropdowns.spec.ts | 194 ++++++++++++++++++ 19 files changed, 850 insertions(+), 106 deletions(-) create mode 100644 desktop/src/features/agents/ui/agentDefinitionSubmitPayload.test.mjs create mode 100644 desktop/src/features/agents/ui/agentDefinitionSubmitPayload.ts create mode 100644 desktop/src/features/agents/ui/personaDialogPickers.test.mjs create mode 100644 desktop/tests/e2e/agent-provider-dropdowns.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 441c2b6505..3570864a60 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -93,6 +93,7 @@ export default defineConfig({ "**/global-agent-config-screenshots.spec.ts", "**/doctor-states.spec.ts", "**/agent-lifecycle-feedback.spec.ts", + "**/agent-provider-dropdowns.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 184caab6b1..40a9cc4e07 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -403,7 +403,14 @@ const overrides = new Map([ // +23 rebase onto #1667: behavioral quad fields (respond_to/parallelism/toolsets) // plumbed through AgentInstanceEditDialog from PersonaAdvancedFields. // +2 provider-aware effort: model/provider props threaded to BuzzAgentModelTuningFields. - ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1165], + // +15 provider/model dropdown fixes: useBakedBuildEnvKeysQuery + hideProviderIds + // for Databricks v1 gate; prospectiveRuntimeId default fallback for builtins. + ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1180], + // AgentDefinitionDialog grew past 1000 with the following load-bearing fixes: + // isRuntimeAutoSeededRef tracking for edit-mode seeding (Fizz shows models); + // runtimeSupportsLlmProviderSelection guard on discovery provider (codex fix); + // hideProviderIds computation for Databricks v1 gate. Queued to split. + ["src/features/agents/ui/AgentDefinitionDialog.tsx", 1035], ]); await runFileSizeCheck({ diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index b89841ffd2..9423bfb458 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -57,7 +57,6 @@ import type { } from "@/shared/api/types"; import type { AttachManagedAgentToChannelInput, - AttachManagedAgentToChannelResult, CreateChannelManagedAgentInput, CreateChannelManagedAgentsResult, CreateChannelManagedAgentResult, @@ -84,10 +83,6 @@ export const acpRuntimesQueryKey = ["acp-runtimes"] as const; export const managedAgentPrereqsQueryKey = ["managed-agent-prereqs"] as const; export const backendProvidersQueryKey = ["backend-providers"] as const; -export type EnsureGooseInChannelResult = AttachManagedAgentToChannelResult & { - created: boolean; -}; - async function invalidateAgentQueries( queryClient: ReturnType, channelId: string | null, @@ -590,39 +585,6 @@ export function useCreateChannelManagedAgentsMutation( }); } -export function useEnsureGooseInChannelMutation(channelId: string | null) { - const queryClient = useQueryClient(); - - return useMutation({ - mutationFn: async (): Promise => { - if (!channelId) { - throw new Error("No channel selected."); - } - - const attached = await ensureChannelAgentPresetInChannel(channelId, { - runtime: { - id: "goose", - label: "Goose", - command: "goose", - defaultArgs: ["acp"], - mcpCommand: null, - }, - role: "bot", - }); - - return { - agent: attached.agent, - membershipAdded: attached.membershipAdded, - started: attached.started, - created: attached.created, - }; - }, - onSettled: () => { - invalidateAgentQueriesInBackground(queryClient, channelId); - }, - }); -} - export function useExportPersonaJsonMutation() { return useMutation({ mutationFn: (id: string) => exportPersonaToJson(id), diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs index dfd287ad23..92b3dd467b 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs @@ -42,6 +42,14 @@ const claudeRuntime = { mcpCommand: null, }; +const buzzAgentRuntime = { + ...gooseRuntime, + id: "buzz-agent", + label: "Buzz Agent", + command: "buzz-agent-cmd", + mcpCommand: null, +}; + function persona(overrides = {}) { return { id: "p-1", @@ -346,3 +354,40 @@ test("row 6: unfetched query refetches instead of resolving empty", async () => "an unfetched query must fetch, not spuriously report no runtimes", ); }); + +// ── item-13 regression: buzz-agent-first default runtime ───────────────────── +// +// Before this fix, resolveStartRuntimeForDefinition used runtimes[0] (catalog +// order: goose, claude, codex, buzz-agent), so an installed goose would beat +// the bundled buzz-agent sidecar as the default for runtime-less personas. +// The fix applies the preference order: buzz-agent → goose → first available. + +test("item-13: goose+buzz-agent both available — persona with no runtime resolves buzz-agent", () => { + const { runtime, warnings } = resolveStartRuntimeForDefinition( + persona({ runtime: undefined }), + [gooseRuntime, claudeRuntime, buzzAgentRuntime], + ); + assert.equal( + runtime.id, + "buzz-agent", + "buzz-agent must win over catalog-first goose for runtime-less personas", + ); + assert.deepEqual(warnings, []); +}); + +test("item-13: goose-only available — persona with no runtime resolves goose", () => { + const { runtime, warnings } = resolveStartRuntimeForDefinition( + persona({ runtime: undefined }), + [gooseRuntime, claudeRuntime], + ); + assert.equal(runtime.id, "goose"); + assert.deepEqual(warnings, []); +}); + +test("item-13: no runtimes available — refuses with actionable error", () => { + assert.throws( + () => resolveStartRuntimeForDefinition(persona({ runtime: undefined }), []), + /No available runtime/, + "empty runtime list must throw, not silently return null", + ); +}); diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.ts b/desktop/src/features/agents/lib/instanceInputForDefinition.ts index 3faa5318ff..408ea5ca11 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.ts +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.ts @@ -7,6 +7,7 @@ import type { import type { MeshAgentPresetPatch } from "@/features/mesh-compute/applyMeshAgentPreset"; import type { MeshServeTarget } from "@/shared/api/tauriMesh"; import { + getDefaultPersonaRuntime, resolvePersonaRuntime, type ResolvePersonaRuntimeResult, } from "./resolvePersonaRuntime"; @@ -47,7 +48,10 @@ export function resolveStartRuntimeForDefinition( persona: AgentPersona, runtimes: readonly AcpRuntime[], ): { runtime: AcpRuntime; warnings: string[] } { - const defaultRuntime = runtimes[0] ?? null; + // Use the buzz-agent-first preference (buzz-agent → goose → first available) + // so a freshly installed goose never beats the bundled buzz-agent sidecar + // for runtime-less personas (item 13 regression guard). + const defaultRuntime = getDefaultPersonaRuntime(runtimes); const { runtime, warnings, isOverridden }: ResolvePersonaRuntimeResult = resolvePersonaRuntime(persona.runtime, runtimes, defaultRuntime); diff --git a/desktop/src/features/agents/lib/resolvePersonaRuntime.ts b/desktop/src/features/agents/lib/resolvePersonaRuntime.ts index f59f963abb..e425ecb755 100644 --- a/desktop/src/features/agents/lib/resolvePersonaRuntime.ts +++ b/desktop/src/features/agents/lib/resolvePersonaRuntime.ts @@ -1,4 +1,28 @@ -import type { AcpRuntime } from "@/shared/api/types"; +import type { AcpRuntime, AcpRuntimeCatalogEntry } from "@/shared/api/types"; + +/** + * Select the best default runtime from a catalog, using the same preference + * order as the UI picker: buzz-agent first (bundled sidecar), then goose, + * then the first available entry, then null when nothing is available. + * + * Generic so that passing AcpRuntime[] (the already-filtered start-path + * list) returns AcpRuntime | null while passing AcpRuntimeCatalogEntry[] + * (the full catalog) returns AcpRuntimeCatalogEntry | null. Both call sites + * share one preference-order implementation. + */ +export function getDefaultPersonaRuntime( + runtimes: readonly T[], +): T | null { + const available = runtimes.filter( + (runtime) => runtime.availability === "available", + ); + return ( + available.find((runtime) => runtime.id === "buzz-agent") ?? + available.find((runtime) => runtime.id === "goose") ?? + available[0] ?? + null + ); +} /** * Result of resolving a persona's preferred runtime against the set of diff --git a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx index 2f9c13eb00..454df09e5d 100644 --- a/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx +++ b/desktop/src/features/agents/ui/AddTeamToChannelDialog.tsx @@ -12,6 +12,7 @@ import { } from "@/features/agents/lib/teamPersonas"; import { collectRuntimeWarnings, + getDefaultPersonaRuntime, resolvePersonaRuntime, } from "@/features/agents/lib/resolvePersonaRuntime"; import { useChannelsQuery } from "@/features/channels/hooks"; @@ -65,8 +66,10 @@ export function AddTeamToChannelDialog({ [channelsQuery.data], ); - const providers = providersQuery.data ?? []; - const defaultProvider = providers[0] ?? null; + const runtimes = providersQuery.data ?? []; + // Use the buzz-agent-first preference so the team-deploy fallback mirrors the + // single-agent start path (buzz-agent → goose → first available). + const defaultProvider = getDefaultPersonaRuntime(runtimes); const teamPersonaResolution = React.useMemo( () => @@ -80,8 +83,8 @@ export function AddTeamToChannelDialog({ // This dialog has no runtime selector, so the fallback is always // `defaultProvider` (the first available runtime). const runtimeWarnings = React.useMemo( - () => collectRuntimeWarnings(resolved, providers, defaultProvider), - [resolved, providers, defaultProvider], + () => collectRuntimeWarnings(resolved, runtimes, defaultProvider), + [resolved, runtimes, defaultProvider], ); function reset() { @@ -122,7 +125,7 @@ export function AddTeamToChannelDialog({ const inputs = resolved.map((persona) => { const { runtime: personaRuntime } = resolvePersonaRuntime( persona.runtime, - providers, + runtimes, defaultProvider, ); const runtimeToUse = personaRuntime ?? defaultProvider; diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index e4f1dfcac9..a2fdde4829 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -40,6 +40,7 @@ import { } from "./personaBehaviorDraft"; import { AUTO_PROVIDER_DROPDOWN_VALUE, + BLOCK_BUILD_HIDDEN_PROVIDER_IDS, buildTemplateModelDropdownOptions, CUSTOM_MODEL_DROPDOWN_VALUE, CUSTOM_PROVIDER_DROPDOWN_VALUE, @@ -76,6 +77,7 @@ import { import { useBakedBuildEnvKeysQuery, useRuntimeFileConfigQuery } from "../hooks"; import { useGlobalAgentConfig } from "../useGlobalAgentConfig"; import { isBuzzAgentRuntime } from "./buzzAgentConfig"; +import { buildRuntimeModelProviderPayload } from "./agentDefinitionSubmitPayload"; type AgentDefinitionDialogProps = { open: boolean; @@ -154,6 +156,16 @@ export function AgentDefinitionDialog({ // The seed the draft is diffed against at submit: an untouched quad // submits no behavior group, keeping unrelated edits hash-quiet. const behaviorSeedRef = React.useRef(emptyPersonaBehaviorDraft); + // Tracks when the runtime was auto-seeded by the default-runtime effect in + // edit mode (i.e. the user never explicitly chose a runtime). Used to omit + // the seeded runtime from the submit payload for builtin definitions whose + // canonical runtime is null — the sync would revert it anyway. + const isRuntimeAutoSeededRef = React.useRef(false); + // Guards the seeding effect so it fires at most once per dialog-open. + // Without this, clearing runtime back to "" via "No preference" would re- + // trigger the effect (the `runtime` dep would pass the length guard) and + // snap the dropdown back to the default — an edit-mode regression. + const hasSeededForOpenRef = React.useRef(false); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); @@ -214,22 +226,32 @@ export function AgentDefinitionDialog({ setIsAvatarUploadPending(false); setImportErrorMessage(null); setIsImportingUpdate(false); + isRuntimeAutoSeededRef.current = false; + hasSeededForOpenRef.current = false; }, [initialValues, open]); React.useEffect(() => { if ( !open || !initialValues || - "id" in initialValues || initialValues.runtime?.trim() || runtimesLoading || runtime.trim().length > 0 || - defaultRuntime === null + defaultRuntime === null || + hasSeededForOpenRef.current ) { return; } setRuntime(defaultRuntime.id); + hasSeededForOpenRef.current = true; + if ("id" in initialValues) { + // Edit mode: record that this runtime was auto-seeded so the submit path + // can omit it from the payload for builtin definitions (canonical runtime + // null; sync would revert the value anyway). Explicit user changes via + // the dropdown clear this flag. + isRuntimeAutoSeededRef.current = true; + } }, [defaultRuntime, initialValues, open, runtime, runtimesLoading]); const isWindowFileDragOver = useWindowFileDragOver( @@ -299,6 +321,8 @@ export function AgentDefinitionDialog({ setIsAvatarUploadPending(false); setImportErrorMessage(null); setIsImportingUpdate(false); + // isRuntimeAutoSeededRef and hasSeededForOpenRef are NOT reset here — the + // [initialValues, open] effect resets both when the dialog re-opens. } onOpenChange(next); @@ -313,19 +337,21 @@ export function AgentDefinitionDialog({ return; } - const trimmedRuntime = runtime.trim(); - const previousRuntime = initialValues.runtime?.trim() ?? ""; - const modelProviderEditableWithoutRuntime = - initialModelProviderEditableWithoutRuntime && trimmedRuntime.length === 0; - const llmProviderVisibleForSubmit = - (trimmedRuntime.length > 0 && - runtimeSupportsLlmProviderSelection(trimmedRuntime)) || - modelProviderEditableWithoutRuntime; - const shouldPreserveHiddenModelProvider = - "id" in initialValues && - previousRuntime.length === 0 && - trimmedRuntime.length === 0 && - !modelProviderEditableWithoutRuntime; + const { + runtime: runtimeForSubmit, + model: modelForSubmit, + provider: providerForSubmit, + } = buildRuntimeModelProviderPayload({ + runtime, + model, + provider, + isEditMode: "id" in initialValues, + isAutoSeeded: isRuntimeAutoSeededRef.current, + initialPreviousRuntime: initialValues.runtime?.trim() ?? "", + initialModel: initialValues.model, + initialProvider: initialValues.provider, + initialModelProviderEditableWithoutRuntime, + }); const namePool = parsePersonaNamePoolText(namePoolText); const namePoolInput = namePool.length > 0 @@ -337,18 +363,9 @@ export function AgentDefinitionDialog({ displayName: displayName.trim(), avatarUrl: avatarUrl.trim() || undefined, systemPrompt: systemPrompt, - runtime: trimmedRuntime || undefined, - model: - trimmedRuntime || modelProviderEditableWithoutRuntime - ? model.trim() || undefined - : shouldPreserveHiddenModelProvider - ? initialValues.model - : undefined, - provider: llmProviderVisibleForSubmit - ? provider.trim() || undefined - : shouldPreserveHiddenModelProvider - ? initialValues.provider - : undefined, + runtime: runtimeForSubmit, + model: modelForSubmit, + provider: providerForSubmit, namePool: namePoolInput, envVars, behavior: behaviorForSubmit( @@ -515,7 +532,12 @@ export function AgentDefinitionDialog({ isCustomProviderEditing, modelFieldVisible, open, - provider: effectiveProvider, + // Gate provider by runtime: runtimes that don't support LLM provider + // selection (codex, claude) must not inherit the global provider — doing + // so causes them to discover models from the wrong provider. + provider: runtimeSupportsLlmProviderSelection(runtime) + ? effectiveProvider + : "", selectedRuntime, }); const staticModelOptions = getPersonaModelOptions(runtime, effectiveProvider); @@ -532,10 +554,22 @@ export function AgentDefinitionDialog({ }); const showCustomModelInput = modelFieldVisible && (isCustomModelEditing || isModelCustom); + // On internal Block builds, BUZZ_AGENT_PROVIDER is baked in and a boot + // migration rewrites any persisted Databricks v1 values → v2. Hide the v1 + // option there so it is not offered for new selections. OSS builds have no + // baked provider, so v1 remains visible. + const hideProviderIds = React.useMemo( + () => + (bakedEnvKeys ?? []).includes("BUZZ_AGENT_PROVIDER") + ? BLOCK_BUILD_HIDDEN_PROVIDER_IDS + : new Set(), + [bakedEnvKeys], + ); const providerOptions = getPersonaProviderOptions( trimmedProvider, runtime, globalConfig.provider ?? "", + hideProviderIds, ); const defaultLlmProviderLabel = getDefaultLlmProviderLabel( runtime, @@ -668,6 +702,8 @@ export function AgentDefinitionDialog({ function handleRuntimeDropdownChange(nextValue: string) { const nextRuntime = nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue; + // The user made an explicit choice — no longer auto-seeded. + isRuntimeAutoSeededRef.current = false; setRuntime(nextRuntime); applySelection( selectionOnRuntimeChange(selection, { diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 7ba254ff75..dbf226bb71 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -7,6 +7,7 @@ import { toast } from "sonner"; import { useAcpRuntimesQuery, useAgentConfigSurface, + useBakedBuildEnvKeysQuery, usePersonasQuery, useStartManagedAgentMutation, useUpdateManagedAgentMutation, @@ -28,10 +29,12 @@ import { EditAgentAdvancedFields } from "./EditAgentAdvancedFields"; import { AUTO_MODEL_DROPDOWN_VALUE, AUTO_PROVIDER_DROPDOWN_VALUE, + BLOCK_BUILD_HIDDEN_PROVIDER_IDS, CUSTOM_MODEL_DROPDOWN_VALUE, CUSTOM_PROVIDER_DROPDOWN_VALUE, formatRuntimeOptionLabel, getDefaultLlmModelLabel, + getDefaultPersonaRuntime, getModelSelectValue, getPersonaProviderOptions, hasPersonaModelOption, @@ -328,6 +331,9 @@ export function AgentInstanceEditDialog({ runtimes.find((r) => r.command?.trim() === agent.agentCommand.trim()) ?.id ?? runtimes.find((r) => r.id === agent.agentCommand.trim())?.id ?? + // Fall back to the app default runtime so discovery can run for agents + // whose persona has no runtime set (e.g. freshly-added catalog builtins). + getDefaultPersonaRuntime(runtimes)?.id ?? "" ); }, [ @@ -387,6 +393,8 @@ export function AgentInstanceEditDialog({ setShowAdvancedFields, }); + const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); + // Merge global env as the base layer so credential keys satisfied via global // config (e.g. ANTHROPIC_API_KEY) are available to model discovery. Use // `inheritedSubmission.envVars` (the same snapshot the credential gate @@ -773,10 +781,18 @@ export function AgentInstanceEditDialog({ // Provider field derived state const trimmedProvider = provider.trim(); + const hideProviderIds = React.useMemo( + () => + (bakedEnvKeys ?? []).includes("BUZZ_AGENT_PROVIDER") + ? BLOCK_BUILD_HIDDEN_PROVIDER_IDS + : new Set(), + [bakedEnvKeys], + ); const providerOptions = getPersonaProviderOptions( trimmedProvider, selectedRuntime?.id ?? "", globalConfig.provider ?? "", + hideProviderIds, ); const providerSelectValue = isCustomProviderEditing ? CUSTOM_PROVIDER_DROPDOWN_VALUE @@ -923,7 +939,6 @@ export function AgentInstanceEditDialog({

) : null} - {/* LLM provider */} {llmProviderFieldVisible ? (
diff --git a/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.test.mjs b/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.test.mjs new file mode 100644 index 0000000000..9cf4654650 --- /dev/null +++ b/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.test.mjs @@ -0,0 +1,88 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { buildRuntimeModelProviderPayload } from "./agentDefinitionSubmitPayload.ts"; + +// Shared fixture for a builtin edit: previous runtime null, no saved model/provider. +const BUILTIN_EDIT_BASE = { + isEditMode: true, + initialPreviousRuntime: "", + initialModel: null, + initialProvider: null, + initialModelProviderEditableWithoutRuntime: false, +}; + +// ── edit-untouched ───────────────────────────────────────────────────────────── +// +// User opens a null-runtime builtin, doesn't change model or provider, submits. +// Runtime was auto-seeded (isAutoSeeded=true), model/provider still empty strings. +// Expected: runtime and model and provider all omitted (undefined). + +test("edit-untouched: model and provider omitted when user changes nothing on auto-seeded builtin", () => { + const result = buildRuntimeModelProviderPayload({ + ...BUILTIN_EDIT_BASE, + runtime: "", + model: "", + provider: "", + isAutoSeeded: true, + }); + assert.equal(result.runtime, undefined, "runtime must be omitted"); + assert.equal(result.model, undefined, "model must be omitted"); + assert.equal(result.provider, undefined, "provider must be omitted"); +}); + +// ── edit-model-only ──────────────────────────────────────────────────────────── +// +// User opens a null-runtime builtin with auto-seed and picks a model. +// Expected: model persisted, runtime omitted (auto-seeded, not explicit). + +test("edit-model-only: chosen model persists, runtime omitted on auto-seeded builtin", () => { + const result = buildRuntimeModelProviderPayload({ + ...BUILTIN_EDIT_BASE, + runtime: "", + model: "claude-opus-4-8", + provider: "", + isAutoSeeded: true, + }); + assert.equal(result.runtime, undefined, "runtime must be omitted"); + assert.equal(result.model, "claude-opus-4-8", "model must be persisted"); + assert.equal(result.provider, undefined, "provider must be omitted"); +}); + +// ── edit-provider-only ───────────────────────────────────────────────────────── +// +// User opens a null-runtime builtin with auto-seed and picks a provider. +// Expected: provider persisted, model and runtime omitted. + +test("edit-provider-only: chosen provider persists, runtime omitted on auto-seeded builtin", () => { + const result = buildRuntimeModelProviderPayload({ + ...BUILTIN_EDIT_BASE, + runtime: "", + model: "", + provider: "anthropic", + isAutoSeeded: true, + }); + assert.equal(result.runtime, undefined, "runtime must be omitted"); + assert.equal(result.model, undefined, "model must be omitted"); + assert.equal(result.provider, "anthropic", "provider must be persisted"); +}); + +// ── explicit-runtime-chosen ──────────────────────────────────────────────────── +// +// User opens a null-runtime builtin, the seeded default is shown, then the user +// explicitly re-selects the same (or a different) runtime via the dropdown. +// handleRuntimeDropdownChange clears isAutoSeeded=false so the runtime is no +// longer treated as auto-seeded and MUST appear in the payload. + +test("explicit-runtime-chosen: runtime and model both persisted when user explicitly selects runtime", () => { + const result = buildRuntimeModelProviderPayload({ + ...BUILTIN_EDIT_BASE, + runtime: "buzz-agent", + model: "claude-opus-4-8", + provider: "", + isAutoSeeded: false, // user made an explicit choice + }); + assert.equal(result.runtime, "buzz-agent", "runtime must be persisted"); + assert.equal(result.model, "claude-opus-4-8", "model must be persisted"); + assert.equal(result.provider, undefined, "empty provider must be omitted"); +}); diff --git a/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.ts b/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.ts new file mode 100644 index 0000000000..56f623ecbc --- /dev/null +++ b/desktop/src/features/agents/ui/agentDefinitionSubmitPayload.ts @@ -0,0 +1,75 @@ +import { runtimeSupportsLlmProviderSelection } from "./personaDialogPickers"; + +/** + * Pure helper extracted from the `handleSubmit` path of `AgentDefinitionDialog` + * so the payload logic can be unit-tested without rendering the component. + * + * Computes the `runtime`, `model`, and `provider` fields for the definition + * submit payload, resolving auto-seeded builtin-edit semantics: when the + * runtime was auto-seeded (the user never explicitly chose one), it is omitted + * from the payload, and model/provider edits are still persisted via the + * `modelProviderEditableWithoutRuntime` path. + */ +export function buildRuntimeModelProviderPayload({ + runtime, + model, + provider, + isEditMode, + isAutoSeeded, + initialPreviousRuntime, + initialModel, + initialProvider, + initialModelProviderEditableWithoutRuntime, +}: { + runtime: string; + model: string; + provider: string; + isEditMode: boolean; + isAutoSeeded: boolean; + initialPreviousRuntime: string; + initialModel: string | null | undefined; + initialProvider: string | null | undefined; + initialModelProviderEditableWithoutRuntime: boolean; +}): { + runtime: string | undefined; + model: string | undefined; + provider: string | undefined; +} { + const trimmedRuntime = runtime.trim(); + const previousRuntime = initialPreviousRuntime; + const isAutoSeededRuntimeForBuiltinEdit = + isEditMode && previousRuntime.length === 0 && isAutoSeeded; + const runtimeForSubmit = isAutoSeededRuntimeForBuiltinEdit + ? "" + : trimmedRuntime; + // An auto-seeded builtin edit is treated the same as an existing builtin with + // a saved model/provider: the field is editable without a runtime, and the + // user's model/provider choice is persisted in the payload. + const modelProviderEditableWithoutRuntime = + (initialModelProviderEditableWithoutRuntime || + isAutoSeededRuntimeForBuiltinEdit) && + runtimeForSubmit.length === 0; + const llmProviderVisibleForSubmit = + (runtimeForSubmit.length > 0 && + runtimeSupportsLlmProviderSelection(runtimeForSubmit)) || + modelProviderEditableWithoutRuntime; + const shouldPreserveHiddenModelProvider = + isEditMode && + previousRuntime.length === 0 && + runtimeForSubmit.length === 0 && + !modelProviderEditableWithoutRuntime; + return { + runtime: runtimeForSubmit || undefined, + model: + runtimeForSubmit || modelProviderEditableWithoutRuntime + ? model.trim() || undefined + : shouldPreserveHiddenModelProvider + ? (initialModel ?? undefined) + : undefined, + provider: llmProviderVisibleForSubmit + ? provider.trim() || undefined + : shouldPreserveHiddenModelProvider + ? (initialProvider ?? undefined) + : undefined, + }; +} diff --git a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx index 98c6faebb1..dc8d90c1c0 100644 --- a/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx +++ b/desktop/src/features/agents/ui/buzzAgentModelTuningFields.tsx @@ -55,8 +55,14 @@ export function EffortSelectField({ * Label for the "Inherit" option when no inherited effort is set and the model * has a semantic default (i.e. `effortDefault !== null`). * - * Defaults to `"Inherit"`. Per-agent callers may pass `"Inherit (agent default)"` + * Defaults to `"Inherit"`. + * + * Per-agent callers (BuzzAgentModelTuningFields) pass `"Inherit (agent default)"` * to preserve the label that appeared before this component was extracted. + * + * The global-defaults card (GlobalAgentConfigSettingsCard) passes + * `"Default ()"` when a semantic default exists and nothing is baked + * in — so OSS users see "Default (medium)" rather than bare "Inherit". */ inheritFallbackLabel?: string; /** Label text for the dropdown. */ diff --git a/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs b/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs index 16278a49ce..759e9a605d 100644 --- a/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs +++ b/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs @@ -56,30 +56,54 @@ test("editAgent_providerFieldHidden_forBlankRuntime", () => { // ── Provider dropdown options for EditAgentProviderField ──────────────────── // // The provider dropdown contains the well-known providers -// (databricks_v2, anthropic, openai, openai-compat) plus a default-provider -// fallback entry so users can clear a saved provider. -// Note: bare "databricks" (V1 / Model Serving) is no longer offered as a -// fresh choice in the default picker — Block builds migrate it to V2 at boot -// and the picker would silently undo any intentional V1 selection. - -test("editAgent_providerOptions_includesDatabricksV2Provider", () => { +// (databricks, databricks_v2, anthropic, openai, openai-compat) plus a +// default-provider fallback entry so users can clear a saved provider. +// +// On OSS builds, Databricks v1 ("databricks") is shown alongside v2 so OSS +// users who were previously on v1 can select it. On internal Block builds, +// callers pass hideProviderIds=new Set(["databricks"]) to suppress v1 — the +// boot migration rewrites any persisted v1 value to v2, so offering v1 there +// would silently undo the migration. + +test("editAgent_providerOptions_includesDatabricksV2AndV1OnOSS", () => { + // OSS builds: no hideProviderIds → both v1 and v2 appear. const options = getPersonaProviderOptions("", "buzz-agent"); const ids = options.map((o) => o.id); + assert.ok(ids.includes("databricks_v2"), "databricks_v2 must be present"); + assert.ok( + ids.includes("databricks"), + "bare databricks (V1) must be present on OSS builds", + ); +}); + +test("editAgent_providerOptions_hidesDatabricksV1OnInternalBuild", () => { + // Internal Block builds: pass hideProviderIds to suppress v1. + const options = getPersonaProviderOptions( + "", + "buzz-agent", + "", + new Set(["databricks"]), + ); + const ids = options.map((o) => o.id); assert.ok( ids.includes("databricks_v2"), - "databricks_v2 must be a provider option", + "databricks_v2 must still be present", ); assert.ok( !ids.includes("databricks"), - "bare databricks (V1) must NOT be in the default provider list", + "bare databricks (V1) must be hidden on internal builds", ); }); -test("editAgent_providerOptions_includesDatabricksV1AsCurrentIfSaved", () => { - // A record that already has provider="databricks" (OSS / pre-migration) - // must still show it in the dropdown as the current selection so it - // remains visible without offering it as a fresh default choice. - const options = getPersonaProviderOptions("databricks", "buzz-agent"); +test("editAgent_providerOptions_includesDatabricksV1AsCurrentEvenWhenHidden", () => { + // A record already persisted with provider="databricks" must show it as the + // current value even when hideProviderIds hides it from fresh selection. + const options = getPersonaProviderOptions( + "databricks", + "buzz-agent", + "", + new Set(["databricks"]), + ); const ids = options.map((o) => o.id); assert.ok( ids.includes("databricks"), diff --git a/desktop/src/features/agents/ui/personaDialogPickers.test.mjs b/desktop/src/features/agents/ui/personaDialogPickers.test.mjs new file mode 100644 index 0000000000..5cc49a9896 --- /dev/null +++ b/desktop/src/features/agents/ui/personaDialogPickers.test.mjs @@ -0,0 +1,187 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + getDefaultPersonaRuntime, + getPersonaModelOptions, + getPersonaProviderOptions, + runtimeSupportsLlmProviderSelection, +} from "./personaDialogPickers.tsx"; +import { formatModelDiscoveryErrorStatus } from "./personaModelDiscoveryStatus.ts"; + +// ── helpers ────────────────────────────────────────────────────────────────── + +function makeRuntime(id, availability = "available") { + return { + id, + label: id, + command: id, + defaultArgs: [], + mcpCommand: null, + availability, + }; +} + +// ── getPersonaProviderOptions — hideProviderIds ─────────────────────────────── + +test("getPersonaProviderOptions returns databricks v1 and v2 when hideProviderIds is empty", () => { + const options = getPersonaProviderOptions("", "buzz-agent", "", new Set()); + const ids = options.map((o) => o.id); + assert.ok(ids.includes("databricks"), "databricks v1 present"); + assert.ok(ids.includes("databricks_v2"), "databricks v2 present"); +}); + +test("getPersonaProviderOptions hides databricks v1 when it is in hideProviderIds", () => { + const options = getPersonaProviderOptions( + "", + "buzz-agent", + "", + new Set(["databricks"]), + ); + const ids = options.map((o) => o.id); + assert.ok(!ids.includes("databricks"), "databricks v1 hidden"); + assert.ok(ids.includes("databricks_v2"), "databricks v2 still present"); +}); + +test("getPersonaProviderOptions appends (current) tail for a saved databricks v1 value even when hidden", () => { + // An agent already persisted with v1 must still render its saved value. + const options = getPersonaProviderOptions( + "databricks", + "buzz-agent", + "", + new Set(["databricks"]), + ); + const tail = options.at(-1); + assert.equal(tail?.id, "databricks"); + assert.equal(tail?.label, "databricks (current)"); +}); + +test("getPersonaProviderOptions with no hideProviderIds omits the tail for a known provider", () => { + const options = getPersonaProviderOptions("anthropic", "buzz-agent"); + const tail = options.at(-1); + // "anthropic" is a known id — no (current) tail appended + assert.ok( + tail?.id !== "anthropic" || tail?.label === "Anthropic", + "no duplicate tail for known provider", + ); +}); + +test("getPersonaProviderOptions appends (current) tail for an unknown saved provider", () => { + const options = getPersonaProviderOptions("my-custom-llm", "buzz-agent"); + const tail = options.at(-1); + assert.equal(tail?.id, "my-custom-llm"); + assert.equal(tail?.label, "my-custom-llm (current)"); +}); + +// ── getDefaultPersonaRuntime — buzz-agent first ─────────────────────────────── + +test("getDefaultPersonaRuntime returns buzz-agent over goose when both are available", () => { + const runtimes = [ + makeRuntime("goose"), + makeRuntime("buzz-agent"), + makeRuntime("claude"), + ]; + const result = getDefaultPersonaRuntime(runtimes); + assert.equal(result?.id, "buzz-agent"); +}); + +test("getDefaultPersonaRuntime falls back to goose when buzz-agent is unavailable", () => { + const runtimes = [ + makeRuntime("buzz-agent", "not_installed"), + makeRuntime("goose"), + ]; + const result = getDefaultPersonaRuntime(runtimes); + assert.equal(result?.id, "goose"); +}); + +test("getDefaultPersonaRuntime returns first available when neither buzz-agent nor goose is available", () => { + const runtimes = [ + makeRuntime("buzz-agent", "adapter_missing"), + makeRuntime("goose", "cli_missing"), + makeRuntime("claude"), + ]; + const result = getDefaultPersonaRuntime(runtimes); + assert.equal(result?.id, "claude"); +}); + +test("getDefaultPersonaRuntime returns null for an empty list", () => { + assert.equal(getDefaultPersonaRuntime([]), null); +}); + +test("getDefaultPersonaRuntime returns null when no runtime is available", () => { + const runtimes = [ + makeRuntime("buzz-agent", "not_installed"), + makeRuntime("goose", "cli_missing"), + ]; + assert.equal(getDefaultPersonaRuntime(runtimes), null); +}); + +// ── runtimeSupportsLlmProviderSelection — provider gating ──────────────────── + +test("runtimeSupportsLlmProviderSelection is true for buzz-agent and goose", () => { + assert.equal(runtimeSupportsLlmProviderSelection("buzz-agent"), true); + assert.equal(runtimeSupportsLlmProviderSelection("goose"), true); +}); + +test("runtimeSupportsLlmProviderSelection is false for codex and claude", () => { + assert.equal(runtimeSupportsLlmProviderSelection("codex"), false); + assert.equal(runtimeSupportsLlmProviderSelection("claude"), false); +}); + +// ── getPersonaModelOptions — codex/claude do not use global provider ────────── +// +// The discovery call in AgentDefinitionDialog passes +// `runtimeSupportsLlmProviderSelection(runtime) ? effectiveProvider : ""` +// so codex/claude never receive the global provider. These tests verify that +// the static model options also stay provider-agnostic for those runtimes. + +test("getPersonaModelOptions for codex returns only default model regardless of provider", () => { + const withProvider = getPersonaModelOptions("codex", "anthropic"); + const withoutProvider = getPersonaModelOptions("codex", ""); + assert.deepEqual(withProvider, withoutProvider); + assert.equal(withProvider.length, 1); + assert.equal(withProvider[0]?.id, ""); +}); + +test("getPersonaModelOptions for buzz-agent with anthropic filters out zero-value default", () => { + // anthropic requires explicit model — zero-value option is filtered out + const options = getPersonaModelOptions("buzz-agent", "anthropic"); + const zeroValue = options.find((o) => o.id === ""); + assert.equal( + zeroValue, + undefined, + "explicit-model provider must not allow zero-value selection", + ); +}); + +test("getPersonaModelOptions for buzz-agent with no provider returns default model", () => { + const options = getPersonaModelOptions("buzz-agent", ""); + assert.equal(options.length, 1); + assert.equal(options[0]?.id, ""); +}); + +// ── formatModelDiscoveryErrorStatus — runtime unavailable ──────────────────── +// +// When selectedRuntime.availability !== "available", AgentDefinitionDialog and +// usePersonaModelDiscovery now call formatModelDiscoveryErrorStatus with a +// synthetic "Runtime not available: " error. Verify the status +// is non-null (so the UI surfaces the reason) for each unavailability reason. + +test("formatModelDiscoveryErrorStatus returns a non-null status for runtime unavailable errors", () => { + for (const availability of [ + "adapter_missing", + "cli_missing", + "not_installed", + ]) { + const status = formatModelDiscoveryErrorStatus( + new Error(`Runtime not available: ${availability}`), + "anthropic", + ); + assert.ok( + status !== null, + `should return a status for availability=${availability}`, + ); + assert.ok(typeof status?.message === "string", "status has a message"); + assert.ok(typeof status?.tone === "string", "status has a tone"); + } +}); diff --git a/desktop/src/features/agents/ui/personaDialogPickers.tsx b/desktop/src/features/agents/ui/personaDialogPickers.tsx index f62866b37f..93e61ea813 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.tsx +++ b/desktop/src/features/agents/ui/personaDialogPickers.tsx @@ -1,5 +1,23 @@ import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; import type { RuntimeFileConfigSubset } from "@/shared/api/tauri"; +// Dialogs import getDefaultPersonaRuntime via this re-export; lib code imports +// directly from lib/resolvePersonaRuntime. +export { getDefaultPersonaRuntime } from "../lib/resolvePersonaRuntime"; + +/** + * Provider ids suppressed from the selection list on internal Block builds. + * Databricks v1 (`"databricks"`) is boot-migrated to v2 on those builds, so + * offering it for new selections would create a regression path. + * OSS builds pass an empty `Set` so v1 remains visible. + * + * All three dialog sites that show a provider picker import this constant — + * `AgentDefinitionDialog`, `AgentInstanceEditDialog`, and + * `GlobalAgentConfigSettingsCard` — making it the single source of truth for + * which provider ids to suppress on Block builds. + */ +export const BLOCK_BUILD_HIDDEN_PROVIDER_IDS: ReadonlySet = new Set([ + "databricks", +]); export const PERSONA_FIELD_SHELL_CLASS = "rounded-xl border border-input bg-muted/40 transition-colors duration-150 ease-out hover:border-muted-foreground/40 focus-within:border-muted-foreground/50"; @@ -16,6 +34,7 @@ export const NO_RUNTIME_DROPDOWN_VALUE = "__no_runtime__"; const KNOWN_LLM_PROVIDER_IDS = [ "anthropic", + "databricks", "databricks_v2", "openai", "openai-compat", @@ -97,6 +116,7 @@ const PERSONA_LLM_PROVIDER_OPTIONS: readonly PersonaModelOption[] = [ { id: "anthropic", label: "Anthropic" }, { id: "openai", label: "OpenAI" }, { id: "openai-compat", label: "OpenAI-compatible" }, + { id: "databricks", label: "Databricks" }, { id: "databricks_v2", label: "Databricks v2 (AI Gateway)" }, ]; @@ -294,16 +314,33 @@ export function buildTemplateModelDropdownOptions( })); } +/** + * Build the provider dropdown options for a persona/instance dialog. + * + * `hideProviderIds` suppresses specific provider ids from the base list while + * still preserving the `(current)` tail-append for saved values that are in + * the hidden set — so an agent already persisted with a hidden provider + * continues to render its current value, while the hidden option is not + * offered for new selections. + * + * Internal Block builds pass `BLOCK_BUILD_HIDDEN_PROVIDER_IDS` to hide the + * legacy Databricks v1 option (the boot migration rewrites v1→v2 on those + * builds). OSS builds pass an empty Set so v1 remains visible. + */ export function getPersonaProviderOptions( currentProvider: string, runtimeId: string, globalProvider?: string, + hideProviderIds?: ReadonlySet, ): readonly PersonaModelOption[] { const trimmedProvider = currentProvider.trim(); const defaultProviderOptions = [ { id: "", label: getDefaultLlmProviderLabel(runtimeId, globalProvider) }, ]; - const options = [...defaultProviderOptions, ...PERSONA_LLM_PROVIDER_OPTIONS]; + const filteredOptions = hideProviderIds?.size + ? PERSONA_LLM_PROVIDER_OPTIONS.filter((o) => !hideProviderIds.has(o.id)) + : PERSONA_LLM_PROVIDER_OPTIONS; + const options = [...defaultProviderOptions, ...filteredOptions]; if ( trimmedProvider.length === 0 || options.some((option) => option.id === trimmedProvider) @@ -408,18 +445,6 @@ export function sortPersonaRuntimes( }); } -export function getDefaultPersonaRuntime(runtimes: AcpRuntimeCatalogEntry[]) { - const available = runtimes.filter( - (runtime) => runtime.availability === "available", - ); - return ( - available.find((runtime) => runtime.id === "buzz-agent") ?? - available.find((runtime) => runtime.id === "goose") ?? - available[0] ?? - null - ); -} - /** * Returns true when `key` is satisfied at the global layer AND the agent-local * `envVars` does NOT explicitly shadow it with an empty string. diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts index e0d4cda986..626c1ef9f0 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts @@ -85,6 +85,11 @@ export function usePersonaModelDiscovery({ const discoveryAgentCommand = selectedRuntime?.command?.trim() ? selectedRuntime.command : null; + // Narrow to the individual fields the effect consumes so a new object + // reference from a React Query refetch (same data, unstable ref) does not + // abandon and re-issue an in-flight discovery IPC call. + const selectedRuntimeAvailability = selectedRuntime?.availability; + const selectedRuntimeDefaultArgs = selectedRuntime?.defaultArgs; const canDiscoverModelOptions = open && modelFieldVisible && @@ -121,7 +126,21 @@ export function usePersonaModelDiscovery({ if (modelDiscoveryKey === null || discoveryAgentCommand === null) { modelDiscoveryRequestRef.current += 1; setModelDiscoveryData(null); - setModelDiscoveryStatus(null); + // When the runtime exists but is not available, surface a status message + // so the model dropdown explains why no live models can be loaded. + if ( + selectedRuntimeAvailability != null && + selectedRuntimeAvailability !== "available" + ) { + setModelDiscoveryStatus( + formatModelDiscoveryErrorStatus( + new Error(`Runtime not available: ${selectedRuntimeAvailability}`), + trimmedProvider, + ), + ); + } else { + setModelDiscoveryStatus(null); + } setModelDiscoveryLoading(false); return; } @@ -144,7 +163,7 @@ export function usePersonaModelDiscovery({ function runModelDiscovery() { void discoverAgentModels({ agentCommand: activeAgentCommand, - agentArgs: selectedRuntime?.defaultArgs ?? [], + agentArgs: selectedRuntimeDefaultArgs ?? [], provider: trimmedProvider || undefined, envVars, }) @@ -193,7 +212,8 @@ export function usePersonaModelDiscovery({ discoveryAgentCommand, envVars, modelDiscoveryKey, - selectedRuntime?.defaultArgs, + selectedRuntimeAvailability, + selectedRuntimeDefaultArgs, shouldDebounceModelDiscovery, trimmedProvider, ]); diff --git a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx index b28e4e8f16..b0f79e2550 100644 --- a/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx +++ b/desktop/src/features/settings/ui/GlobalAgentConfigSettingsCard.tsx @@ -25,6 +25,7 @@ import type { InheritedEnvRow } from "@/features/agents/ui/EnvVarsEditor"; import { getBakedProviderInheritLabel } from "@/features/agents/ui/bakedEnvHelpers"; import { AUTO_PROVIDER_DROPDOWN_VALUE, + BLOCK_BUILD_HIDDEN_PROVIDER_IDS, CUSTOM_PROVIDER_DROPDOWN_VALUE, getPersonaProviderOptions, } from "@/features/agents/ui/personaDialogPickers"; @@ -136,6 +137,10 @@ export function GlobalAgentConfigSettingsCard() { () => bakedEnv.filter((e) => !BAKED_STRUCTURED_KEYS.has(e.key)), [bakedEnv], ); + const bakedEnvKeys = React.useMemo( + () => bakedEnv.map((e) => e.key), + [bakedEnv], + ); // Resolve the buzz-agent runtime catalog entry for model discovery. // The card is always visible (open=true), so the query is always enabled. @@ -257,9 +262,21 @@ export function GlobalAgentConfigSettingsCard() { } } + // On internal Block builds, BUZZ_AGENT_PROVIDER is baked in and a boot + // migration rewrites v1→v2. Hide the legacy v1 option so it is not offered + // for new selections; OSS builds show it. + const hideProviderIds = React.useMemo( + () => + bakedEnvKeys.includes("BUZZ_AGENT_PROVIDER") + ? BLOCK_BUILD_HIDDEN_PROVIDER_IDS + : new Set(), + [bakedEnvKeys], + ); const providerOptions = getPersonaProviderOptions( providerValue, "buzz-agent", + undefined, + hideProviderIds, ); const providerSelectValue = isCustomProvider ? CUSTOM_PROVIDER_DROPDOWN_VALUE @@ -371,6 +388,11 @@ export function GlobalAgentConfigSettingsCard() { effortValid={effortValid} htmlFor="global-agent-thinking-effort" inheritedEffort={bakedEffort ?? undefined} + inheritFallbackLabel={ + effortDefault !== null + ? `Default (${effortDefault})` + : undefined + } label="Default thinking / effort" onChange={(value) => { setConfig((prev) => { diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 196368ff45..77d1ff6ff0 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -8808,6 +8808,12 @@ export function maybeInstallE2eTauriMocks() { config?.mock?.globalConfigFailedRestartCount ?? 0, }; } + case "get_baked_build_env": + // Mock always returns an empty baked env (OSS build simulation). + return []; + case "get_baked_build_env_keys": + // Mock always returns no baked env key names (OSS build simulation). + return []; case "update_managed_agent": return handleUpdateManagedAgent( payload as Parameters[0], diff --git a/desktop/tests/e2e/agent-provider-dropdowns.spec.ts b/desktop/tests/e2e/agent-provider-dropdowns.spec.ts new file mode 100644 index 0000000000..db69d8901b --- /dev/null +++ b/desktop/tests/e2e/agent-provider-dropdowns.spec.ts @@ -0,0 +1,194 @@ +/** + * Screenshot-regression spec for PR #1764: provider/model dropdown fixes. + * + * Three states that previously regressed on fresh OSS installs: + * + * 01 – Global agent config provider select renders both Databricks v1 and v2 + * options (they are always shown on OSS builds; the mock bridge returns an + * empty baked env, simulating an OSS install with no BUZZ_AGENT_PROVIDER). + * + * 02 – Effort dropdown shows "Default (medium)" instead of bare "Inherit" when + * no effort is baked and the provider uses a known default effort level. + * (provider unset → effortDefault = "medium" → inheritFallbackLabel fires) + * + * 03 – Edit dialog for a definition with runtime null auto-seeds the app-default + * runtime (buzz-agent) and model discovery runs → model combobox is non-empty. + * Previously, the seeding effect bailed in edit mode, leaving the runtime + * empty and the model dropdown silently blank. + */ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; +import { waitForAnimations } from "../helpers/animations"; + +const SHOTS = "test-results/screenshots-dialogs"; + +/** + * Navigate to the agents view and wait for the global agent config card to + * finish its async load (spinner gone, card content visible). + */ +async function openAgentsView(page: import("@playwright/test").Page) { + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("settings-global-agent-config")).toBeVisible({ + timeout: 10_000, + }); + // The card shows a spinner during the async load effect; wait for it to clear. + await expect(page.locator(".animate-spin").first()).not.toBeVisible({ + timeout: 5_000, + }); +} + +test.describe("agent provider dropdown screenshots", () => { + test.use({ viewport: { width: 1280, height: 900 } }); + + test.beforeEach(async ({ page }) => { + page.on("pageerror", (err) => { + console.error( + "PAGE ERROR:", + err.message, + err.stack?.split("\n").slice(0, 5).join("\n"), + ); + }); + }); + + // Shot 01: OSS provider dropdown includes both Databricks v1 and v2. + // + // The mock bridge returns get_baked_build_env_keys = [] (OSS), so no + // BUZZ_AGENT_PROVIDER is baked and hideProviderIds is empty → v1 appears. + test("01-provider-dropdown-oss", async ({ page }) => { + await installMockBridge(page); + await openAgentsView(page); + + const providerSelect = page.locator("#global-agent-provider"); + await expect(providerSelect).toBeVisible({ timeout: 5_000 }); + + // Regression: both v1 and v2 must be present on OSS. + const optionTexts = await providerSelect.evaluate((el: HTMLSelectElement) => + Array.from(el.options).map((o) => o.text.trim()), + ); + expect(optionTexts).toContain("Databricks"); + expect(optionTexts).toContain("Databricks v2 (AI Gateway)"); + + await waitForAnimations(page); + + // Screenshot the card — provider select is closed but the assertion above + // proves both Databricks options are present in the option list. + await page + .getByTestId("settings-global-agent-config") + .screenshot({ path: `${SHOTS}/01-provider-dropdown-oss.png` }); + }); + + // Shot 02: Effort dropdown shows "Default (medium)" (no baked effort, + // provider=databricks_v2 with no model → effortDefault = "medium"). + // + // getProviderEffortConfig("databricks_v2", "") hits the blank-model branch: + // → { defaultValue: "medium" } + // inheritFallbackLabel = "Default (medium)", bakedEffort = null (OSS) + // → the zero-value option reads "Default (medium)" instead of bare "Inherit". + // + // Using databricks_v2 here (rather than no provider) makes this card + // visually distinct from shot 01 — the provider select shows "Databricks v2 + // (AI Gateway)" as the selected value — so the two PNG hashes differ. + test("02-effort-default-label", async ({ page }) => { + await installMockBridge(page, { + globalAgentConfig: { + provider: "databricks_v2", + model: null, + env_vars: {}, + }, + }); + await openAgentsView(page); + + const effortSelect = page.locator("#global-agent-thinking-effort"); + await expect(effortSelect).toBeVisible({ timeout: 5_000 }); + + // Regression: the zero-value option must show the provider's default + // effort rather than a bare "Inherit". + const zeroOptionText = await effortSelect.evaluate( + (el: HTMLSelectElement) => + Array.from(el.options) + .find((o) => o.value === "") + ?.text.trim() ?? "", + ); + expect(zeroOptionText).toBe("Default (medium)"); + + await waitForAnimations(page); + + await page + .getByTestId("settings-global-agent-config") + .screenshot({ path: `${SHOTS}/02-effort-default-label.png` }); + }); + + // Shot 03: Edit dialog for a definition with null runtime auto-seeds the + // default runtime (buzz-agent via getDefaultPersonaRuntime) and model + // discovery runs, producing a non-empty model combobox. + // + // Previously the seeding effect bailed in edit mode ("id" in initialValues), + // leaving runtime = "" → modelFieldVisible = false → discovery never ran → + // the model dropdown was silently empty. + test("03-builtin-edit-runtime-seeded", async ({ page }) => { + await installMockBridge(page, { + personas: [ + { + displayName: "Null Runtime Agent", + systemPrompt: "An agent with no runtime configured.", + // runtime/provider/model not set → all null in the mock, so the + // edit dialog must auto-seed the app default runtime. + }, + ], + }); + + await page.goto("/"); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("agents-library-personas")).toBeVisible({ + timeout: 10_000, + }); + + // Open the persona's actions menu (visible for non-builtin personas). + const actionsBtn = page.getByRole("button", { + name: "Open actions for Null Runtime Agent", + }); + await expect(actionsBtn).toBeVisible({ timeout: 8_000 }); + await actionsBtn.click(); + + await page.getByRole("menuitem", { name: "Edit" }).click(); + + const dialog = page.getByTestId("persona-dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + + // Regression: the runtime trigger must not be empty — the auto-seed effect + // must have run and selected the app default (buzz-agent in the mock catalog). + const runtimeTrigger = dialog.locator("#persona-runtime"); + await expect(runtimeTrigger).toBeVisible({ timeout: 8_000 }); + await expect(runtimeTrigger).not.toContainText("No preference", { + timeout: 8_000, + }); + + // Regression: the model combobox must appear (modelFieldVisible = true once + // runtime is non-empty) and model discovery must have run, populating it. + const modelCombobox = dialog.getByRole("combobox", { name: /model/i }); + await expect(modelCombobox).toBeVisible({ timeout: 8_000 }); + + // Open the picker and assert that a known model from the mock discovery + // response is listed. The mock returns "Claude Opus 4.6" for all providers + // (see discover_agent_models in e2eBridge.ts). A zero-model discovery + // regression would leave the list empty and this assertion would fail. + // The picker is a searchable command popover portaled outside the dialog + // whose items render as buttons — query at page level. + await modelCombobox.click(); + await expect( + page.getByRole("button", { name: /Claude Opus 4\.6/i }), + ).toBeVisible({ timeout: 5_000 }); + // Close the popover so the screenshot captures the dialog's resting state. + await page.keyboard.press("Escape"); + await expect( + page.getByRole("button", { name: /Claude Opus 4\.6/i }), + ).toBeHidden(); + + await waitForAnimations(page); + await dialog.screenshot({ + path: `${SHOTS}/03-builtin-edit-runtime-seeded.png`, + }); + }); +}); From 348a684af0c34d89e4a5c800e14476e991289209 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Sun, 12 Jul 2026 18:16:43 -0400 Subject: [PATCH 2/2] fix(desktop): drop "(AI Gateway)" qualifier from Databricks v2 provider label --- desktop/src/features/agents/ui/EnvVarsEditor.test.mjs | 4 ++-- desktop/src/features/agents/ui/personaDialogPickers.tsx | 2 +- desktop/tests/e2e/agent-provider-dropdowns.spec.ts | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs b/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs index 5671950268..318247e1aa 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs +++ b/desktop/src/features/agents/ui/EnvVarsEditor.test.mjs @@ -483,13 +483,13 @@ test("inheritedRows_structured_keys_excluded_from_generic_rows", () => { test("getBakedProviderInheritLabel_known_provider_returns_friendly_name", () => { const options = [ { id: "anthropic", label: "Anthropic" }, - { id: "databricks_v2", label: "Databricks v2 (AI Gateway)" }, + { id: "databricks_v2", label: "Databricks v2" }, { id: "openai", label: "OpenAI" }, ]; const label = getBakedProviderInheritLabel("databricks_v2", options); assert.equal( label, - "Databricks v2 (AI Gateway) (inherited from build)", + "Databricks v2 (inherited from build)", "known provider id must resolve to friendly label", ); }); diff --git a/desktop/src/features/agents/ui/personaDialogPickers.tsx b/desktop/src/features/agents/ui/personaDialogPickers.tsx index 93e61ea813..f7d53213a7 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.tsx +++ b/desktop/src/features/agents/ui/personaDialogPickers.tsx @@ -117,7 +117,7 @@ const PERSONA_LLM_PROVIDER_OPTIONS: readonly PersonaModelOption[] = [ { id: "openai", label: "OpenAI" }, { id: "openai-compat", label: "OpenAI-compatible" }, { id: "databricks", label: "Databricks" }, - { id: "databricks_v2", label: "Databricks v2 (AI Gateway)" }, + { id: "databricks_v2", label: "Databricks v2" }, ]; const PERSONA_MODEL_OPTIONS_BY_RUNTIME: Record< diff --git a/desktop/tests/e2e/agent-provider-dropdowns.spec.ts b/desktop/tests/e2e/agent-provider-dropdowns.spec.ts index db69d8901b..68cc6b8d9e 100644 --- a/desktop/tests/e2e/agent-provider-dropdowns.spec.ts +++ b/desktop/tests/e2e/agent-provider-dropdowns.spec.ts @@ -68,7 +68,7 @@ test.describe("agent provider dropdown screenshots", () => { Array.from(el.options).map((o) => o.text.trim()), ); expect(optionTexts).toContain("Databricks"); - expect(optionTexts).toContain("Databricks v2 (AI Gateway)"); + expect(optionTexts).toContain("Databricks v2"); await waitForAnimations(page); @@ -88,8 +88,8 @@ test.describe("agent provider dropdown screenshots", () => { // → the zero-value option reads "Default (medium)" instead of bare "Inherit". // // Using databricks_v2 here (rather than no provider) makes this card - // visually distinct from shot 01 — the provider select shows "Databricks v2 - // (AI Gateway)" as the selected value — so the two PNG hashes differ. + // visually distinct from shot 01 — the provider select shows "Databricks v2" + // as the selected value — so the two PNG hashes differ. test("02-effort-default-label", async ({ page }) => { await installMockBridge(page, { globalAgentConfig: {