diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 923e132167..c0c7c0b4e6 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/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. diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index 6ec28c30c3..3072f94e97 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/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. diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 9c9a71ab06..ab1be92ce1 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/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} diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index d69028cfd1..8a2e449485 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/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 diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 8be5f4719b..5de76f254a 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/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 ? (
) : null} -

- {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."} -

+ {modelDiscoveryLoadingMessage || + modelDiscoveryStatus !== null ? ( + + ) : modelDiscoveryLoading ? null : ( +

+ {discoveredModelOptions !== null + ? "Saved changes take effect on the next start." + : "Select a provider above to see available models."} +

+ )} 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 ( +

+ {text} +

+ ); + } + + if (status === null) { + return null; + } + + return ( +
+

+ {status.message} +

+ {status.retryable && onRetry ? ( + + ) : null} +
+ ); +} diff --git a/desktop/src/features/agents/ui/PersonaModelField.tsx b/desktop/src/features/agents/ui/PersonaModelField.tsx index bfbdb2c83d..743b63ec70 100644 --- a/desktop/src/features/agents/ui/PersonaModelField.tsx +++ b/desktop/src/features/agents/ui/PersonaModelField.tsx @@ -3,6 +3,7 @@ import { motion } from "motion/react"; import { cn } from "@/shared/lib/cn"; import { Input } from "@/shared/ui/input"; +import { ModelDiscoveryStatusLine } from "./ModelDiscoveryStatusLine"; import { PersonaModelCombobox } from "./PersonaModelCombobox"; import type { PersonaModelDiscoveryStatus } from "./personaModelDiscoveryStatus"; import { @@ -16,12 +17,18 @@ type PersonaModelFieldProps = { disabled: boolean; isExplicitModelRequired: boolean; model: string; + /** True while discovery is in flight (#2261). */ + modelDiscoveryLoading?: boolean; + /** Progressive status-line copy only (not the control). */ + modelDiscoveryLoadingMessage?: string | null; modelDiscoveryStatus: PersonaModelDiscoveryStatus | null; modelDropdownOptions: readonly PersonaDropdownOption[]; showSharedComputeAutoHint: boolean; modelSelectValue: string; onCustomModelChange: (value: string) => void; onModelValueChange: (value: string) => void; + /** Re-run discovery after timeout / empty / path failure (#2261). */ + onRetryModelDiscovery?: () => void; showCustomModelInput: boolean; transition: React.ComponentProps["transition"]; }; @@ -30,12 +37,15 @@ export function PersonaModelField({ disabled, isExplicitModelRequired, model, + modelDiscoveryLoading = false, + modelDiscoveryLoadingMessage = null, modelDiscoveryStatus, modelDropdownOptions, modelSelectValue, showSharedComputeAutoHint, onCustomModelChange, onModelValueChange, + onRetryModelDiscovery, showCustomModelInput, transition, }: PersonaModelFieldProps) { @@ -95,19 +105,15 @@ export function PersonaModelField({ Buzz will choose an available shared model when the agent starts.

) : null} - {modelDiscoveryStatus ? ( -

- {modelDiscoveryStatus.message} -

+ {modelDiscoveryLoadingMessage || modelDiscoveryStatus ? ( + ) : null} diff --git a/desktop/src/features/agents/ui/agentConfigControls.tsx b/desktop/src/features/agents/ui/agentConfigControls.tsx index df0321fa7b..180297e7d5 100644 --- a/desktop/src/features/agents/ui/agentConfigControls.tsx +++ b/desktop/src/features/agents/ui/agentConfigControls.tsx @@ -24,7 +24,11 @@ import { type PersonaModelOption, } from "./agentConfigOptions"; import { MODEL_DISCOVERY_LOADING_VALUE } from "./usePersonaModelDiscovery"; -import type { PersonaModelDiscoveryStatus } from "./personaModelDiscoveryStatus"; +import { + MODEL_DISCOVERY_LOADING_SHORT, + type PersonaModelDiscoveryStatus, +} from "./personaModelDiscoveryStatus"; +import { ModelDiscoveryStatusLine } from "./ModelDiscoveryStatusLine"; export const MODEL_NO_MODELS_VALUE = "__no_models__"; @@ -283,9 +287,11 @@ export function AgentModelField({ isRequired, model, modelDiscoveryLoading, + modelDiscoveryLoadingMessage = null, modelDiscoveryStatus, onIsCustomModelEditingChange, onModelChange, + onRetryModelDiscovery, placeholder = "Select model", placeholderClassName, provider, @@ -318,9 +324,13 @@ export function AgentModelField({ isRequired: boolean; model: string; modelDiscoveryLoading: boolean; + /** Progressive loading copy from {@link usePersonaModelDiscovery}. */ + modelDiscoveryLoadingMessage?: string | null; modelDiscoveryStatus: PersonaModelDiscoveryStatus | null; onIsCustomModelEditingChange: (value: boolean) => void; onModelChange: (value: string) => void; + /** Re-run discovery after a timeout / empty / path failure. */ + onRetryModelDiscovery?: () => void; /** Trigger placeholder shown when there is no selected model option. */ placeholder?: string; /** Optional class override for placeholder text. */ @@ -425,16 +435,22 @@ export function AgentModelField({ onModelChange(nextValue); }; + // Discovery in flight with no catalog yet: force SHORT label on the control + // (trigger + option). The long progressive sentence must never enter the + // pill — it truncates and duplicated the under-field note (#2261 screenshot). + const controlShowsLoading = + modelDiscoveryLoading && discoveredModelOptions === null; + const modelOptions: AgentDropdownOption[] = [ ...effectiveModelOptions.map((option) => ({ label: option.label, value: option.id || AUTO_MODEL_DROPDOWN_VALUE, })), - ...(modelDiscoveryLoading && discoveredModelOptions === null + ...(controlShowsLoading ? [ { disabled: true, - label: "Loading models...", + label: MODEL_DISCOVERY_LOADING_SHORT, value: MODEL_DISCOVERY_LOADING_VALUE, }, ] @@ -454,16 +470,15 @@ export function AgentModelField({ trimmedModel.length > 0 ? trimmedModel : undefined; - // While discovery is in flight with nothing selected, the closed field - // reads "Loading models…" instead of a select-prompt — the field isn't - // waiting on the user, it's waiting on the harness. - const restingPlaceholder = - modelDiscoveryLoading && - discoveredModelOptions === null && - trimmedModel.length === 0 && - !isCustomModelEditing - ? "Loading models..." - : placeholder; + + // Closed trigger: while loading, always SHORT — do not fall through to + // selectedOption.label / selectedLabel, which could surface longer copy. + const controlSelectedLabel = controlShowsLoading + ? MODEL_DISCOVERY_LOADING_SHORT + : stableSelectedModelLabel; + const controlPlaceholder = controlShowsLoading + ? MODEL_DISCOVERY_LOADING_SHORT + : placeholder; const modelSelect = useCustomSelect ? ( ) : (