From ee5327a9106817a57c0ee46a6a03ef46397f259e Mon Sep 17 00:00:00 2001 From: Thomas Zarebczan Date: Tue, 21 Jul 2026 15:39:01 -0400 Subject: [PATCH 1/2] fix(desktop,acp): Codex model discovery timeout UX + 45s probe budget Raise buzz-acp models probe timeout from 10s to 45s so Windows Codex cold starts can finish. Surface progressive under-field copy after 10s, clear timeout/PATH errors with Retry, and keep the model control on a short loading label so long status text is never truncated in the pill. Builds on #2246 (status bypass + empty non-cache). Fixes #2261. Signed-off-by: Thomas Zarebczan --- crates/buzz-acp/src/lib.rs | 7 +- desktop/src/features/agents/AGENTS.md | 36 +++++--- .../features/agents/ui/AgentConfigFields.tsx | 26 ++++-- .../agents/ui/AgentDefinitionDialog.tsx | 5 ++ .../agents/ui/AgentInstanceEditDialog.tsx | 28 +++++-- .../agents/ui/ModelDiscoveryStatusLine.tsx | 73 +++++++++++++++++ .../features/agents/ui/PersonaModelField.tsx | 32 +++++--- .../agents/ui/agentConfigControls.tsx | 81 ++++++++++++------ .../ui/agentConfigFieldsContract.test.mjs | 31 ++++++- .../ui/personaModelDiscoveryStatus.test.mjs | 68 ++++++++++++++- .../agents/ui/personaModelDiscoveryStatus.ts | 82 +++++++++++++++++++ .../ui/usePersonaModelDiscovery.test.mjs | 3 + .../agents/ui/usePersonaModelDiscovery.ts | 49 ++++++++++- 13 files changed, 448 insertions(+), 73 deletions(-) create mode 100644 desktop/src/features/agents/ui/ModelDiscoveryStatusLine.tsx 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..69e9c7e2a7 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, @@ -937,10 +939,13 @@ export function AgentDefinitionDialog({ disabled={isPending} isExplicitModelRequired={isExplicitModelRequired} model={model} + modelDiscoveryLoading={modelDiscoveryLoading} + modelDiscoveryLoadingMessage={modelDiscoveryLoadingMessage} modelDiscoveryStatus={modelDiscoveryStatus} modelDropdownOptions={modelDropdownOptions} modelSelectValue={modelSelectValue} 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..db0bc6813e 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, @@ -1079,15 +1082,22 @@ export function AgentInstanceEditDialog({ /> ) : 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 ? ( ) : (