fix(desktop,acp): Codex model discovery timeout UX + 45s probe budget

crates/buzz-acp/src/lib.rs

@@ -58,7 +58,12 @@ fn is_subcommand(name: &str) -> bool {
 }
 
 /// Timeout for lightweight helper subcommands (spawn + initialize + model/method probes).
-const MODELS_TIMEOUT: Duration = Duration::from_secs(10);
+///
+/// Codex ACP cold-starts the Codex App Server on every `models` probe; on
+/// Windows this commonly takes ~15–25s (see #2261). 10s was too short and
+/// produced empty model lists with opaque failures. Keep this well above the
+/// observed cold path so UI retry + progressive loading can succeed.
+const MODELS_TIMEOUT: Duration = Duration::from_secs(45);
 
 /// Timeout for `buzz-acp authenticate`. Browser-based vendor auth can require
 /// human interaction, so it must not share the short probe timeout.

desktop/src/features/agents/AGENTS.md

@@ -53,15 +53,25 @@ 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.  **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`).
+   boolean props.  **Exception (#2246, extended #2261):** `onboarding-essential`
+   hides happy-path helper copy (provider/model descriptions) but
+   `shouldShowModelStatusMessage(showDescriptions, status, loadingMessage)`
+   surfaces the status line when `status !== null` **or** the slow-phase
+   under-field note is non-null (`loadingMessage`). Early loading (control
+   only) does not force the under-field line. Quiet happy path stays hidden
+   in onboarding.
    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()`).
+   (`supportsSwitching:false` or empty model list) synthesizes a **retryable**
+   warning via `synthesizeEmptyDiscoveryStatus()` and is intentionally **not
+   cached** so Retry / close→reopen re-runs discovery after the user installs
+   or signs into the CLI (`isCacheableDiscoveryResponse()`). Timeouts and
+   path failures also set `retryable: true` and wire `retryModelDiscovery()`
+   (clears the in-memory cache key and re-probes). Loading UX: the control
+   always shows short `MODEL_DISCOVERY_LOADING_SHORT`. The long under-field
+   note (`formatModelDiscoveryLoadingMessage`) is **null until**
+   `MODEL_DISCOVERY_SLOW_MS` (10s), then appears only under the field via
+   `ModelDiscoveryStatusLine` (never in the pill). One-shot timeout, not a
+   polling ticker.
 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
@@ -74,11 +84,13 @@ 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 + `shouldShowModelStatusMessage` status-bypass rule. If this fails,
-  you probably reintroduced a per-surface flag or broke the status-bypass.
+  presets + `shouldShowModelStatusMessage` status/loading bypass. If this
+  fails, you probably reintroduced a per-surface flag or broke the bypass.
 - `ui/usePersonaModelDiscovery.test.mjs` — `synthesizeEmptyDiscoveryStatus`,
-  `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`. If the
-  "reopen to retry" copy becomes inert again, these tests will catch it.
+  `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`, retryable
+  empty status. If empty catalogs get cached or lose Retry, these fail.
+- `ui/personaModelDiscoveryStatus.test.mjs` — timeout / PATH / credential
+  status mapping + progressive loading copy.
 - `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

@@ -113,16 +113,23 @@ export function resolveDisclosure(disclosure: "full" | "onboarding-essential") {
 /**
  * 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.
+ * Discovery warnings and the **slow-phase** loading note bypass the
+ * `onboarding-essential` preset so first-run failures / long Codex cold
+ * starts are never invisible. Early loading (control shows "Loading
+ * models…") does not force the under-field line — only
+ * `loadingMessage !== null` or a non-null status does. Quiet happy path
+ * stays hidden in onboarding.
+ *
+ * @param loadingMessage - progressive under-field copy; null until slow phase
  */
 export function shouldShowModelStatusMessage(
   showDescriptions: boolean,
   status: { message: string; tone: string } | null,
+  loadingMessage: string | null = null,
 ): boolean {
-  return showDescriptions || status !== null;
+  return (
+    showDescriptions || status !== null || (loadingMessage?.trim() ?? "") !== ""
+  );
 }
 
 export type AgentConfigFieldsProps = {
@@ -276,7 +283,9 @@ export function AgentConfigFields({
   const {
     discoveredModelOptions,
     modelDiscoveryLoading,
+    modelDiscoveryLoadingMessage,
     modelDiscoveryStatus,
+    retryModelDiscovery,
   } = usePersonaModelDiscovery({
     envVars: config.env_vars,
     isCustomProviderEditing: isCustomProvider,
@@ -685,11 +694,17 @@ export function AgentConfigFields({
           modelDiscoveryLoading={
             dependentFieldsDisabled ? false : modelDiscoveryLoading
           }
+          modelDiscoveryLoadingMessage={
+            dependentFieldsDisabled ? null : modelDiscoveryLoadingMessage
+          }
           modelDiscoveryStatus={
             dependentFieldsDisabled ? null : modelDiscoveryStatus
           }
           onIsCustomModelEditingChange={onCustomModelEditingChange}
           onModelChange={handleModelChange}
+          onRetryModelDiscovery={
+            dependentFieldsDisabled ? undefined : retryModelDiscovery
+          }
           placeholderClassName={placeholderClassName}
           placeholder="Select a model"
           provider={providerForDiscovery}
@@ -700,6 +715,7 @@ export function AgentConfigFields({
           showStatusMessage={shouldShowModelStatusMessage(
             showDescriptions,
             modelDiscoveryStatus,
+            dependentFieldsDisabled ? null : modelDiscoveryLoadingMessage,
           )}
           testId="global-agent-model"
           useCustomSelect={useCustomSelect}

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

@@ -496,7 +496,9 @@ export function AgentDefinitionDialog({
   const {
     discoveredModelOptions,
     modelDiscoveryLoading,
+    modelDiscoveryLoadingMessage,
     modelDiscoveryStatus,
+    retryModelDiscovery,
   } = usePersonaModelDiscovery({
     envVars: envVarsForDiscovery,
     isCustomProviderEditing,
@@ -596,11 +598,13 @@ export function AgentDefinitionDialog({
       })),
     { label: "Custom provider...", value: CUSTOM_PROVIDER_DROPDOWN_VALUE },
   ];
+  const controlShowsModelLoading =
+    modelDiscoveryLoading && discoveredModelOptions === null;
   const modelDropdownOptions: PersonaDropdownOption[] =
     buildModelDropdownOptions({
       allowCustom: !isRelayMesh,
       globalModel: undefined,
-      loading: modelDiscoveryLoading && discoveredModelOptions === null,
+      loading: controlShowsModelLoading,
       loadingValue: MODEL_DISCOVERY_LOADING_VALUE,
       options: modelOptions,
     })
@@ -612,6 +616,11 @@ export function AgentDefinitionDialog({
           ? { ...option, label: "Automatic" }
           : option,
       );
+  // Force short loading sentinel on the control while probing (same as
+  // AgentModelField / instance edit — closed trigger must not look stuck).
+  const modelControlValue = controlShowsModelLoading
+    ? MODEL_DISCOVERY_LOADING_VALUE
+    : modelSelectValue;
   const previewLabel = displayName.trim() || "Agent name";
   const previewAvatarUrl = avatarUrl.trim() || null;
   const runtimeWarning =
@@ -937,10 +946,13 @@ export function AgentDefinitionDialog({
                   disabled={isPending}
                   isExplicitModelRequired={isExplicitModelRequired}
                   model={model}
+                  modelDiscoveryLoading={modelDiscoveryLoading}
+                  modelDiscoveryLoadingMessage={modelDiscoveryLoadingMessage}
                   modelDiscoveryStatus={modelDiscoveryStatus}
                   modelDropdownOptions={modelDropdownOptions}
-                  modelSelectValue={modelSelectValue}
+                  modelSelectValue={modelControlValue}
                   onCustomModelChange={setModel}
+                  onRetryModelDiscovery={retryModelDiscovery}
                   showSharedComputeAutoHint={
                     isRelayMesh &&
                     modelSelectValue === AUTO_MODEL_DROPDOWN_VALUE

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

@@ -80,6 +80,7 @@ import { getProviderApiKeyEnvVar } from "./agentConfigOptions";
 import { useAgentDialogDefaults } from "./useAgentDialogDefaults";
 import { AgentAiDefaultsNotice } from "./AgentAiDefaults";
 import { AgentDefaultsDialog } from "./AgentDefaultsDialog";
+import { ModelDiscoveryStatusLine } from "./ModelDiscoveryStatusLine";
 import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState";
 
 const ADVANCED_FIELDS_MOTION_TRANSITION = {
@@ -410,7 +411,9 @@ export function AgentInstanceEditDialog({
   const {
     discoveredModelOptions,
     modelDiscoveryLoading,
+    modelDiscoveryLoadingMessage,
     modelDiscoveryStatus,
+    retryModelDiscovery,
   } = usePersonaModelDiscovery({
     envVars: envVarsForDiscovery,
     isCustomProviderEditing,
@@ -785,14 +788,22 @@ export function AgentInstanceEditDialog({
     model,
     provider: providerForDiscovery,
   });
+  // Match AgentModelField: while probing with no catalog yet, force the
+  // control onto the short loading sentinel so the closed trigger does not
+  // keep showing a stale default/previous model with no loading cue (#2261).
+  const controlShowsModelLoading =
+    modelDiscoveryLoading && discoveredModelOptions === null;
   const modelDropdownOptions = buildModelDropdownOptions({
     allowCustom: !isRelayMesh,
     globalModel: isRelayMesh ? undefined : inheritedModelDefault.value,
     globalModelLabel: isRelayMesh ? undefined : inheritedModelLabel,
-    loading: modelDiscoveryLoading && discoveredModelOptions === null,
+    loading: controlShowsModelLoading,
     loadingValue: MODEL_DISCOVERY_LOADING_VALUE,
     options: effectiveModelOptions,
   });
+  const modelControlValue = controlShowsModelLoading
+    ? MODEL_DISCOVERY_LOADING_VALUE
+    : modelSelectValue;
 
   // Provider field derived state
   const trimmedProvider = provider.trim();
@@ -1055,7 +1066,7 @@ export function AgentInstanceEditDialog({
                 onValueChange={handleModelDropdownChange}
                 options={modelDropdownOptions}
                 placeholder="Default model"
-                value={modelSelectValue}
+                value={modelControlValue}
               />
               {showCustomModelInput ? (
                 <div
@@ -1079,15 +1090,22 @@ export function AgentInstanceEditDialog({
                   />
                 </div>
               ) : null}
-              <p className="text-xs text-muted-foreground">
-                {modelDiscoveryLoading
-                  ? "Loading models..."
-                  : modelDiscoveryStatus !== null
-                    ? modelDiscoveryStatus.message
-                    : discoveredModelOptions !== null
-                      ? "Saved changes take effect on the next start."
-                      : "Select a provider above to see available models."}
-              </p>
+              {modelDiscoveryLoadingMessage ||
+              modelDiscoveryStatus !== null ? (
+                <ModelDiscoveryStatusLine
+                  disabled={updateMutation.isPending}
+                  loading={modelDiscoveryLoading}
+                  loadingMessage={modelDiscoveryLoadingMessage}
+                  onRetry={retryModelDiscovery}
+                  status={modelDiscoveryStatus}
+                />
+              ) : modelDiscoveryLoading ? null : (
+                <p className="text-xs text-muted-foreground">
+                  {discoveredModelOptions !== null
+                    ? "Saved changes take effect on the next start."
+                    : "Select a provider above to see available models."}
+                </p>
+              )}
             </div>
 
             <AgentAiDefaultsNotice

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

@@ -0,0 +1,73 @@
+import { cn } from "@/shared/lib/cn";
+
+import type { PersonaModelDiscoveryStatus } from "./personaModelDiscoveryStatus";
+
+/**
+ * Status line under the Model control: progressive loading, failure copy,
+ * and optional Retry. Shared by onboarding (`AgentModelField`), create
+ * (`PersonaModelField`), and instance edit so surfaces do not fork markup.
+ */
+export function ModelDiscoveryStatusLine({
+  disabled = false,
+  loading,
+  loadingMessage,
+  onRetry,
+  status,
+  testId = "model-discovery-status",
+}: {
+  disabled?: boolean;
+  loading: boolean;
+  /** Full progressive copy for the status line (may be long). */
+  loadingMessage?: string | null;
+  onRetry?: () => void;
+  status: PersonaModelDiscoveryStatus | null;
+  testId?: string;
+}) {
+  if (loading) {
+    // Only render once progressive copy is ready (after MODEL_DISCOVERY_SLOW_MS).
+    // Early loading is communicated solely by the control's short label.
+    const text = loadingMessage?.trim();
+    if (!text) return null;
+    return (
+      <p
+        aria-live="polite"
+        className="text-xs text-muted-foreground"
+        data-testid={testId}
+      >
+        {text}
+      </p>
+    );
+  }
+
+  if (status === null) {
+    return null;
+  }
+
+  return (
+    <div
+      className="flex flex-wrap items-start gap-x-3 gap-y-1"
+      data-testid={testId}
+    >
+      <p
+        aria-live="polite"
+        className={cn(
+          "min-w-0 flex-1 text-xs",
+          status.tone === "warning" ? "text-warning" : "text-muted-foreground",
+        )}
+      >
+        {status.message}
+      </p>
+      {status.retryable && onRetry ? (
+        <button
+          className="shrink-0 text-xs font-medium text-foreground underline-offset-2 hover:underline disabled:opacity-50"
+          data-testid="model-discovery-retry"
+          disabled={disabled}
+          onClick={onRetry}
+          type="button"
+        >
+          Retry
+        </button>
+      ) : null}
+    </div>
+  );
+}