No matches
+ {filteredOptions.length === 0 ? ( ++ {showSearch && query.trim().length > 0 + ? "No matches" + : emptyOptionsLabel} +
) : null} {filteredOptions.map((option) => { const selected = option.value === value; @@ -470,6 +480,7 @@ export function AgentModelField({ ariaRequired={isRequired} className={selectClassName} disabled={selectDisabled} + emptyOptionsLabel="Couldn't load models" id={id} onValueChange={handleModelSelectChange} options={modelOptions} diff --git a/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs b/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs index 8933ffb258..2435130f6d 100644 --- a/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs +++ b/desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs @@ -21,6 +21,7 @@ import test from "node:test"; import { CANONICAL_CONFIG_BEHAVIORS, resolveDisclosure, + shouldRenderModelControl, shouldShowModelStatusMessage, } from "./AgentConfigFields.tsx"; @@ -83,3 +84,94 @@ test("shouldShowModelStatusMessage_onboardingPreset_warningStatus_showsMessage", }; assert.equal(shouldShowModelStatusMessage(showDescriptions, warning), true); }); + +// ── shouldRenderModelControl ────────────────────────────────────────────────── +// Optional-model harnesses omit the control only after a confirmed successful +// empty catalog. Failures keep the control so #2246 status UI can render. + +test("optional model control hides while loading", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: true, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + false, + "optional + loading must hide the control", + ); +}); + +test("optional model control hides after successful empty discovery", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: true, + showCustomModelOption: false, + }), + false, + "optional + successful empty + no custom must hide the control", + ); +}); + +test("optional model control stays visible on discovery failure", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + true, + "optional + failed/unavailable discovery must keep the control", + ); +}); + +test("optional model control shows when explicit models are available", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: [ + { id: "", label: "Default model" }, + { id: "claude-sonnet-4", label: "Claude Sonnet 4" }, + ], + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: false, + modelIsOptional: true, + showCustomModelOption: false, + }), + true, + "explicit models must show the control", + ); +}); + +test("full disclosure keeps Custom escape hatch when discovery is empty", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: true, + showCustomModelOption: true, + }), + true, + "full disclosure keeps Custom escape hatch when discovery is empty", + ); +}); + +test("required-model harnesses always keep the control", () => { + assert.equal( + shouldRenderModelControl({ + discoveredModelOptions: null, + modelDiscoveryLoading: false, + modelDiscoverySuccessfulEmpty: true, + modelIsOptional: false, + showCustomModelOption: false, + }), + true, + "required-model harnesses always keep the control", + ); +}); diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs index 0246fe6ac8..ecb36a6fc5 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs @@ -5,6 +5,7 @@ import { deriveModelDiscoveryPending, getDiscoveredPersonaModelOptions, isCacheableDiscoveryResponse, + isSuccessfulEmptyDiscovery, synthesizeEmptyDiscoveryStatus, } from "./usePersonaModelDiscovery.ts"; @@ -258,3 +259,56 @@ test("deriveModelDiscoveryPending_noKey_isNotPending", () => { false, ); }); + +// ── isSuccessfulEmptyDiscovery ──────────────────────────────────────────────── + +test("isSuccessfulEmptyDiscovery_resolvedEmptyResponse_isTrue", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: response({ models: [] }), + discoveredModelOptions: null, + modelDiscoveryPending: false, + }), + true, + ); +}); + +test("isSuccessfulEmptyDiscovery_thrownFailure_isFalse", () => { + // Failure path leaves data null — must not be treated as successful empty. + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: null, + discoveredModelOptions: null, + modelDiscoveryPending: false, + }), + false, + ); +}); + +test("isSuccessfulEmptyDiscovery_withUsableModels_isFalse", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: response({ + models: [ + { id: "claude-sonnet-5", name: "Claude Sonnet 5", description: null }, + ], + }), + discoveredModelOptions: [ + { id: "claude-sonnet-5", label: "Claude Sonnet 5" }, + ], + modelDiscoveryPending: false, + }), + false, + ); +}); + +test("isSuccessfulEmptyDiscovery_stillPending_isFalse", () => { + assert.equal( + isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData: null, + discoveredModelOptions: null, + modelDiscoveryPending: true, + }), + false, + ); +}); diff --git a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts index 2bbed69c28..b0ab0f8379 100644 --- a/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts +++ b/desktop/src/features/agents/ui/usePersonaModelDiscovery.ts @@ -139,6 +139,28 @@ export function deriveModelDiscoveryPending({ ); } +/** + * True when discovery IPC resolved with a response that yielded no usable + * model options. Distinct from a thrown/unavailable failure (data stays null). + * Callers that omit the Model control or heal persisted values must gate on + * this — not on `discoveredModelOptions === null` alone. + */ +export function isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData, + discoveredModelOptions, + modelDiscoveryPending, +}: { + activeModelDiscoveryData: AgentModelsResponse | null; + discoveredModelOptions: readonly PersonaModelOption[] | null; + modelDiscoveryPending: boolean; +}): boolean { + return ( + !modelDiscoveryPending && + activeModelDiscoveryData !== null && + discoveredModelOptions === null + ); +} + export function usePersonaModelDiscovery({ envVars, isCustomProviderEditing, @@ -358,6 +380,11 @@ export function usePersonaModelDiscovery({ activeModelDiscoveryData, activeModelDiscoveryStatus, }); + const modelDiscoverySuccessfulEmpty = isSuccessfulEmptyDiscovery({ + activeModelDiscoveryData, + discoveredModelOptions, + modelDiscoveryPending, + }); return { discoveredModelOptions, @@ -366,5 +393,6 @@ export function usePersonaModelDiscovery({ modelDiscoveryPending || discoveredModelOptions !== null ? null : activeModelDiscoveryStatus, + modelDiscoverySuccessfulEmpty, }; } diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 1ea8998313..97e162aeef 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -384,6 +384,25 @@ type E2eConfig = { * spec can interleave edits and exercise the mid-save race handling. */ globalConfigSaveDelayMs?: number; + /** + * Override the `discover_agent_models` mock response. When set, returns + * this catalog instead of the default per-harness model list. + */ + discoverAgentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; + /** + * When set, `discover_agent_models` throws with this message instead of + * returning a catalog. + */ + discoverAgentModelsError?: string; }; relayHttpUrl?: string; relayWsUrl?: string; @@ -10005,6 +10024,25 @@ export function maybeInstallE2eTauriMocks() { supportsSwitching: false, }; case "discover_agent_models": { + const discoverError = activeConfig?.mock?.discoverAgentModelsError; + if (discoverError) { + throw new Error(discoverError); + } + const discoverOverride = activeConfig?.mock?.discoverAgentModels; + if (discoverOverride) { + return { + agentName: "mock-agent", + agentVersion: "0.0.0", + models: discoverOverride.models.map((model) => ({ + id: model.id, + name: model.name, + description: model.description ?? null, + })), + agentDefaultModel: discoverOverride.agentDefaultModel ?? null, + selectedModel: discoverOverride.selectedModel ?? null, + supportsSwitching: discoverOverride.supportsSwitching, + }; + } const input = ( payload as { input?: { agentCommand?: string; provider?: string }; diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 40ddc64ef6..f8e6322ed3 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -431,6 +431,76 @@ test("defaults renders only fields supported by the selected harness", async ({ ).toHaveCount(0); }); +test("defaults hides model when optional harness has empty discovery", async ({ + page, +}) => { + await installMockBridge( + page, + { + acpRuntimesCatalog: [ + runtime("claude", "available", { status: "logged_in" }), + ], + discoverAgentModels: { + models: [], + supportsSwitching: false, + }, + globalAgentConfig: { + env_vars: {}, + provider: null, + model: null, + preferred_runtime: null, + }, + }, + { skipCommunitySeed: true, skipOnboardingSeed: true }, + ); + await page.goto("/"); + await navigateToSetupPage(page); + await page.getByTestId("onboarding-setup-next").click(); + + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + await expect(page.getByTestId("global-agent-default-harness")).toHaveText( + "Claude Code", + ); + // Confirmed successful empty catalog — omit the Model control; harness + // default applies and Finish stays available. + await expect(page.getByTestId("global-agent-model")).toHaveCount(0); + await expect(page.getByTestId("onboarding-finish")).toBeEnabled(); +}); + +test("defaults keeps model control when optional harness discovery fails", async ({ + page, +}) => { + await installMockBridge( + page, + { + acpRuntimesCatalog: [ + runtime("claude", "available", { status: "logged_in" }), + ], + discoverAgentModelsError: "CLI discovery timed out", + globalAgentConfig: { + env_vars: {}, + provider: null, + model: null, + preferred_runtime: null, + }, + }, + { skipCommunitySeed: true, skipOnboardingSeed: true }, + ); + await page.goto("/"); + await navigateToSetupPage(page); + await page.getByTestId("onboarding-setup-next").click(); + + await expect(page.getByTestId("onboarding-page-config")).toBeVisible(); + await expect(page.getByTestId("global-agent-default-harness")).toHaveText( + "Claude Code", + ); + // Failed discovery must not look like successful empty: keep the control + // and surface #2246 failure UI (status line bypasses onboarding-essential). + await expect(page.getByTestId("global-agent-model")).toBeVisible(); + await expect(page.getByText(/Could not load live models/i)).toBeVisible(); + await expect(page.getByTestId("onboarding-finish")).toBeEnabled(); +}); + test("defaults Back returns to harness setup", async ({ page }) => { await installMockBridge( page, diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 8273e697a4..e0c228525f 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -419,6 +419,27 @@ type MockBridgeOptions = { * test can interleave edits and exercise the mid-save race handling. */ globalConfigSaveDelayMs?: number; + /** + * Override the `discover_agent_models` mock response. When set, the bridge + * returns this catalog instead of the default per-harness model list. + * Use `{ models: [], supportsSwitching: false }` to exercise empty discovery + * (e.g. optional harnesses that omit the Model control). + */ + discoverAgentModels?: { + models: Array<{ + id: string; + name: string | null; + description?: string | null; + }>; + supportsSwitching: boolean; + agentDefaultModel?: string | null; + selectedModel?: string | null; + }; + /** + * When set, `discover_agent_models` throws with this message instead of + * returning a catalog. Exercises the discovery-failure UI path. + */ + discoverAgentModelsError?: string; }; type BridgeOptions = {