fix(desktop): surface model discovery failures in agent config UI

desktop/src/features/agents/AGENTS.md

@@ -53,7 +53,15 @@ with a TypeScript lookup table or an id comparison in a component.
 6. **One canonical behavior, disclosure presets for visibility.** Behavior
    flags were deliberately killed in #2148 (`CANONICAL_CONFIG_BEHAVIORS`).
    Surface differences are expressed via the `disclosure` preset, not new
-   boolean props.
+   boolean props.  **Exception:** `onboarding-essential` hides happy-path
+   helper copy (provider/model descriptions) but a non-null model-discovery
+   status always bypasses the preset and renders the status line — enforced
+   via `shouldShowModelStatusMessage()` (`AgentConfigFields.tsx`).
+   Additionally, a successful discovery response that yields no usable options
+   (`supportsSwitching:false` or empty model list) synthesizes a warning status
+   via `synthesizeEmptyDiscoveryStatus()` and is intentionally **not cached**
+   so that closing → reopening the dialog re-runs discovery after the user
+   installs or signs into the CLI (`isCacheableDiscoveryResponse()`).
 7. **Onboarding setup detects readiness; it does not select defaults.** The
    setup page derives visible and ready harnesses from the runtime catalog and
    only offers install or sign-in actions. The following defaults page is the
@@ -66,7 +74,11 @@ with a TypeScript lookup table or an id comparison in a component.
 - `lib/agentConfigCore.test.mjs` — field model per harness × scope, clearing
   policy. Update when the capability model changes.
 - `ui/agentConfigFieldsContract.test.mjs` — canonical behaviors + disclosure
-  presets. If this fails, you probably reintroduced a per-surface flag.
+  presets + `shouldShowModelStatusMessage` status-bypass rule. If this fails,
+  you probably reintroduced a per-surface flag or broke the status-bypass.
+- `ui/usePersonaModelDiscovery.test.mjs` — `synthesizeEmptyDiscoveryStatus`,
+  `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`. If the
+  "reopen to retry" copy becomes inert again, these tests will catch it.
 - `desktop/tests/e2e/onboarding-agent-defaults.spec.ts` — onboarding behavior
   acceptance coverage for readiness, failure states, defaults, navigation, and
   persistence races.

desktop/src/features/agents/ui/AgentConfigFields.tsx

@@ -110,6 +110,21 @@ export function resolveDisclosure(disclosure: "full" | "onboarding-essential") {
   } as const;
 }
 
+/**
+ * Determines whether the status line beneath the Model field should render.
+ *
+ * Discovery warnings bypass the `onboarding-essential` preset so that a
+ * first-run failure is never silently invisible.  On the happy path
+ * (`status === null`) the status line stays hidden in onboarding, keeping
+ * the page clean.
+ */
+export function shouldShowModelStatusMessage(
+  showDescriptions: boolean,
+  status: { message: string; tone: string } | null,
+): boolean {
+  return showDescriptions || status !== null;
+}
+
 export type AgentConfigFieldsProps = {
   bakedEnv: BakedEnvEntry[];
   selectedRuntime: AcpRuntimeCatalogEntry | undefined;
@@ -682,7 +697,10 @@ export function AgentConfigFields({
           labelClassName={fieldLabelClassName}
           selectClassName={selectClassName}
           showCustomModelOption={showCustomModelOption}
-          showStatusMessage={showDescriptions}
+          showStatusMessage={shouldShowModelStatusMessage(
+            showDescriptions,
+            modelDiscoveryStatus,
+          )}
           testId="global-agent-model"
           useCustomSelect={useCustomSelect}
           useChevronIcon={useChevronSelectIcon}

desktop/src/features/agents/ui/agentConfigControls.test.mjs

@@ -1,7 +1,11 @@
 import assert from "node:assert/strict";
 import test from "node:test";
 
-import { resolveDefaultModelLabel } from "./agentConfigControls.tsx";
+import {
+  MODEL_NO_MODELS_VALUE,
+  appendNoModelsSentinel,
+  resolveDefaultModelLabel,
+} from "./agentConfigControls.tsx";
 
 test("uses the harness-discovered default model label for an unset model", () => {
   assert.equal(
@@ -38,3 +42,27 @@ test("an explicit inherited default label wins over harness discovery", () => {
     "Default model (team-model)",
   );
 });
+
+// ── appendNoModelsSentinel ─────────────────────────────────────────────────────
+
+test("appendNoModelsSentinel_emptyOptionsDiscoveryFinished_addsDisabledRow", () => {
+  const options = appendNoModelsSentinel([], false);
+  assert.equal(options.length, 1);
+  assert.equal(options[0].disabled, true);
+  assert.equal(options[0].label, "No models found");
+  assert.equal(options[0].value, MODEL_NO_MODELS_VALUE);
+});
+
+test("appendNoModelsSentinel_emptyOptionsDiscoveryLoading_doesNotAddRow", () => {
+  const options = appendNoModelsSentinel([], true);
+  assert.equal(options.length, 0);
+});
+
+test("appendNoModelsSentinel_nonEmptyOptionsDiscoveryFinished_doesNotAddRow", () => {
+  const options = appendNoModelsSentinel(
+    [{ label: "Default model", value: "" }],
+    false,
+  );
+  assert.equal(options.length, 1);
+  assert.equal(options[0].label, "Default model");
+});

desktop/src/features/agents/ui/agentConfigControls.tsx

@@ -26,12 +26,34 @@ import {
 import { MODEL_DISCOVERY_LOADING_VALUE } from "./usePersonaModelDiscovery";
 import type { PersonaModelDiscoveryStatus } from "./personaModelDiscoveryStatus";
 
+export const MODEL_NO_MODELS_VALUE = "__no_models__";
+
 export type AgentDropdownOption = {
   disabled?: boolean;
   label: React.ReactNode;
   value: string;
 };
 
+/**
+ * Ensures an opened model dropdown never renders as a blank white bar.
+ * When `options` is empty and discovery has finished, appends a single
+ * disabled "No models found" sentinel row in place.  Returns `options`
+ * for convenient chaining in tests.
+ */
+export function appendNoModelsSentinel(
+  options: AgentDropdownOption[],
+  loading: boolean,
+): AgentDropdownOption[] {
+  if (options.length === 0 && !loading) {
+    options.push({
+      disabled: true,
+      label: "No models found",
+      value: MODEL_NO_MODELS_VALUE,
+    });
+  }
+  return options;
+}
+
 function optionTestId(testId: string | undefined, value: string) {
   if (!testId) return undefined;
   return `${testId}-option-${value || "empty"}`;
@@ -422,6 +444,10 @@ export function AgentModelField({
       ? [{ label: "Custom model...", value: CUSTOM_MODEL_DROPDOWN_VALUE }]
       : []),
   ];
+  // An opened dropdown must never show a blank popover. When all the above
+  // yields an empty list and discovery has finished, add a disabled sentinel
+  // row so the user sees "No models found" instead of a bare white bar.
+  appendNoModelsSentinel(modelOptions, modelDiscoveryLoading);
   const stableSelectedModelLabel =
     keepSelectedModelValueLabel &&
     modelSelectValue === trimmedModel &&

desktop/src/features/agents/ui/agentConfigFieldsContract.test.mjs

@@ -21,6 +21,7 @@ import test from "node:test";
 import {
   CANONICAL_CONFIG_BEHAVIORS,
   resolveDisclosure,
+  shouldShowModelStatusMessage,
 } from "./AgentConfigFields.tsx";
 
 test("canonical behaviors: onboarding's values are the only behavior", () => {
@@ -57,3 +58,28 @@ test("onboarding-essential hides power tools but never the effort field", () =>
     showUnavailableEffortOptions: false,
   });
 });
+
+// ── shouldShowModelStatusMessage ──────────────────────────────────────────────
+// The onboarding-essential preset sets showDescriptions=false.  Discovery
+// warnings must bypass the preset so first-run failures are never invisible.
+
+test("shouldShowModelStatusMessage_fullDisclosure_nullStatus_showsMessage", () => {
+  // Full disclosure always shows the status line regardless of status.
+  assert.equal(shouldShowModelStatusMessage(true, null), true);
+});
+
+test("shouldShowModelStatusMessage_onboardingPreset_nullStatus_hidesMessage", () => {
+  // Happy path: no status → status line hidden in onboarding.
+  const { showDescriptions } = resolveDisclosure("onboarding-essential");
+  assert.equal(shouldShowModelStatusMessage(showDescriptions, null), false);
+});
+
+test("shouldShowModelStatusMessage_onboardingPreset_warningStatus_showsMessage", () => {
+  // Discovery failure → status line surfaces even in onboarding-essential.
+  const { showDescriptions } = resolveDisclosure("onboarding-essential");
+  const warning = {
+    message: "Claude Code reported no models.",
+    tone: "warning",
+  };
+  assert.equal(shouldShowModelStatusMessage(showDescriptions, warning), true);
+});

desktop/src/features/agents/ui/usePersonaModelDiscovery.test.mjs

@@ -1,7 +1,12 @@
 import assert from "node:assert/strict";
 import test from "node:test";
 
-import { getDiscoveredPersonaModelOptions } from "./usePersonaModelDiscovery.ts";
+import {
+  deriveModelDiscoveryPending,
+  getDiscoveredPersonaModelOptions,
+  isCacheableDiscoveryResponse,
+  synthesizeEmptyDiscoveryStatus,
+} from "./usePersonaModelDiscovery.ts";
 
 function response(overrides = {}) {
   return {
@@ -110,3 +115,146 @@ test("returns null when discovery is unsupported or empty", () => {
   );
   assert.equal(getDiscoveredPersonaModelOptions(null, ""), null);
 });
+
+// ── synthesizeEmptyDiscoveryStatus ────────────────────────────────────────────
+
+test("synthesizeEmptyDiscoveryStatus_emptyModels_producesWarningStatus", () => {
+  const status = synthesizeEmptyDiscoveryStatus(
+    response({ models: [], agentName: "Claude Code" }),
+    "",
+  );
+  assert.equal(status?.tone, "warning");
+  assert.match(status?.message ?? "", /Claude Code/);
+  assert.match(status?.message ?? "", /reported no models/);
+});
+
+test("synthesizeEmptyDiscoveryStatus_supportsSwitchingFalse_producesWarningStatus", () => {
+  const status = synthesizeEmptyDiscoveryStatus(
+    response({
+      supportsSwitching: false,
+      models: [{ id: "gpt-4", name: "GPT-4", description: null }],
+      agentName: "Codex",
+    }),
+    "",
+  );
+  assert.equal(status?.tone, "warning");
+  assert.match(status?.message ?? "", /Codex/);
+});
+
+test("synthesizeEmptyDiscoveryStatus_withUsableModels_returnsNull", () => {
+  assert.equal(
+    synthesizeEmptyDiscoveryStatus(
+      response({
+        models: [
+          { id: "claude-sonnet-5", name: "Claude Sonnet 5", description: null },
+        ],
+        agentName: "Claude Code",
+      }),
+      "",
+    ),
+    null,
+  );
+});
+
+test("synthesizeEmptyDiscoveryStatus_emptyAgentName_usesGenericFallback", () => {
+  const status = synthesizeEmptyDiscoveryStatus(
+    response({ models: [], agentName: "" }),
+    "",
+  );
+  assert.equal(status?.tone, "warning");
+  assert.match(status?.message ?? "", /This agent/);
+});
+
+// ── isCacheableDiscoveryResponse ──────────────────────────────────────────────
+
+test("isCacheableDiscoveryResponse_withUsableModels_returnsTrue", () => {
+  assert.equal(
+    isCacheableDiscoveryResponse(
+      response({
+        models: [
+          { id: "claude-sonnet-5", name: "Claude Sonnet 5", description: null },
+        ],
+      }),
+      "",
+    ),
+    true,
+  );
+});
+
+test("isCacheableDiscoveryResponse_emptyModels_returnsFalse", () => {
+  // An empty-result response must not be cached so close→reopen retries
+  // discovery after the user installs or signs into the CLI.
+  assert.equal(
+    isCacheableDiscoveryResponse(response({ models: [] }), ""),
+    false,
+  );
+});
+
+test("isCacheableDiscoveryResponse_supportsSwitchingFalse_returnsFalse", () => {
+  assert.equal(
+    isCacheableDiscoveryResponse(
+      response({
+        supportsSwitching: false,
+        models: [{ id: "gpt-4", name: "GPT-4", description: null }],
+      }),
+      "",
+    ),
+    false,
+  );
+});
+
+// ── deriveModelDiscoveryPending ────────────────────────────────────────────────
+
+test("deriveModelDiscoveryPending_stillLoading_isTrue", () => {
+  assert.equal(
+    deriveModelDiscoveryPending({
+      modelDiscoveryLoading: true,
+      modelDiscoveryKey: "key",
+      activeModelDiscoveryData: null,
+      activeModelDiscoveryStatus: null,
+    }),
+    true,
+  );
+});
+
+test("deriveModelDiscoveryPending_keySetDataNullStatusNull_isTrue", () => {
+  // A key is set but neither data nor status has arrived yet → still pending.
+  assert.equal(
+    deriveModelDiscoveryPending({
+      modelDiscoveryLoading: false,
+      modelDiscoveryKey: "key",
+      activeModelDiscoveryData: null,
+      activeModelDiscoveryStatus: null,
+    }),
+    true,
+  );
+});
+
+test("deriveModelDiscoveryPending_resolvedEmptyResponse_isNotPending", () => {
+  // A resolved-but-empty response sets data non-null and status to a warning.
+  // Neither condition for pending is met — the hook must not spin forever.
+  const emptyResponse = response({ models: [] });
+  const warningStatus = { message: "no models", tone: "warning" };
+  assert.equal(
+    deriveModelDiscoveryPending({
+      modelDiscoveryLoading: false,
+      modelDiscoveryKey: "key",
+      activeModelDiscoveryData: emptyResponse,
+      activeModelDiscoveryStatus: warningStatus,
+    }),
+    false,
+  );
+});
+
+test("deriveModelDiscoveryPending_noKey_isNotPending", () => {
+  // key=null means discovery is not expected (e.g. dialog closed).
+  assert.equal(
+    deriveModelDiscoveryPending({
+      modelDiscoveryLoading: false,
+      modelDiscoveryKey: null,
+      activeModelDiscoveryData: null,
+      activeModelDiscoveryStatus: null,
+    }),
+    false,
+  );
+});