Show provider failures (#6744)

Co-authored-by: Douwe Osinga <douwe@squareup.com>

crates/goose-server/src/routes/config_management.rs

@@ -17,7 +17,6 @@ use goose::providers::canonical::maybe_get_canonical_model;
 use goose::providers::create_with_default_model;
 use goose::providers::errors::ProviderError;
 use goose::providers::providers as get_providers;
-use goose::providers::{retry_operation, RetryConfig};
 use goose::{
     agents::execute_commands, agents::ExtensionConfig, config::permission::PermissionLevel,
     slash_commands,
@@ -408,10 +407,7 @@ pub async fn get_provider_models(
         .await
         .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
 
-    let models_result = retry_operation(&RetryConfig::default(), || async {
+    let models_result = provider.fetch_recommended_models().await;
-        provider.fetch_recommended_models().await
-    })
-    .await;
 
     match models_result {
         Ok(Some(models)) => Ok(Json(models)),

ui/desktop/src/components/ConfigContext.tsx

@@ -8,7 +8,6 @@ import {
   addExtension as apiAddExtension,
   removeExtension as apiRemoveExtension,
   providers,
-  getProviderModels as apiGetProviderModels,
 } from '../api';
 import { syncBundledExtensions } from './settings/extensions';
 import type {
@@ -41,7 +40,6 @@ interface ConfigContextType {
   removeExtension: (name: string) => Promise<void>;
   getProviders: (b: boolean) => Promise<ProviderDetails[]>;
   getExtensions: (b: boolean) => Promise<FixedExtensionEntry[]>;
-  getProviderModels: (providerName: string) => Promise<string[]>;
   disableAllExtensions: () => Promise<void>;
   enableBotExtensions: (extensions: ExtensionConfig[]) => Promise<void>;
 }
@@ -187,19 +185,6 @@ export const ConfigProvider: React.FC<ConfigProviderProps> = ({ children }) => {
     return providersListRef.current;
   }, []);
 
-  const getProviderModels = useCallback(async (providerName: string): Promise<string[]> => {
-    try {
-      const response = await apiGetProviderModels({
-        path: { name: providerName },
-        throwOnError: true,
-      });
-      return response.data || [];
-    } catch (error) {
-      console.error(`Failed to fetch models for provider ${providerName}:`, error);
-      return [];
-    }
-  }, []);
-
   useEffect(() => {
     // Load all configuration data and providers on mount
     (async () => {
@@ -284,7 +269,6 @@ export const ConfigProvider: React.FC<ConfigProviderProps> = ({ children }) => {
       toggleExtension,
       getProviders,
       getExtensions,
-      getProviderModels,
       disableAllExtensions,
       enableBotExtensions,
     };
@@ -301,7 +285,6 @@ export const ConfigProvider: React.FC<ConfigProviderProps> = ({ children }) => {
     toggleExtension,
     getProviders,
     getExtensions,
-    getProviderModels,
     reloadConfig,
   ]);
 

ui/desktop/src/components/settings/models/modelInterface.ts

@@ -1,4 +1,4 @@
-import { ProviderDetails } from '../../../api';
+import { ProviderDetails, getProviderModels } from '../../../api';
 
 export default interface Model {
   id?: number; // Make `id` optional to allow user-defined models
@@ -48,24 +48,19 @@ export interface ProviderModelsResult {
   error: string | null;
 }
 
-/**
- * Fetches recommended models for all active providers in parallel.
- * Falls back to known_models if fetching fails or returns no models.
- */
 export async function fetchModelsForProviders(
-  activeProviders: ProviderDetails[],
+  activeProviders: ProviderDetails[]
-  getProviderModelsFunc: (providerName: string) => Promise<string[]>
 ): Promise<ProviderModelsResult[]> {
   const modelPromises = activeProviders.map(async (p) => {
-    const providerName = p.name;
     try {
-      let models = await getProviderModelsFunc(providerName);
+      const response = await getProviderModels({
-      if ((!models || models.length === 0) && p.metadata.known_models?.length) {
+        path: { name: p.name },
-        models = p.metadata.known_models.map((m) => m.name);
+        throwOnError: true,
-      }
+      });
+      const models = response.data || [];
       return { provider: p, models, error: null };
     } catch (e: unknown) {
-      const errorMessage = `Failed to fetch models for ${providerName}${e instanceof Error ? `: ${e.message}` : ''}`;
+      const errorMessage = `Failed to fetch models for ${p.name}${e instanceof Error ? `: ${e.message}` : ''}`;
       return {
         provider: p,
         models: null,

ui/desktop/src/components/settings/models/subcomponents/LeadWorkerSettings.tsx

@@ -5,8 +5,8 @@ import { Button } from '../../../ui/button';
 import { Select } from '../../../ui/Select';
 import { Input } from '../../../ui/input';
 import { getPredefinedModelsFromEnv, shouldShowPredefinedModels } from '../predefinedModelsUtils';
-import { fetchModelsForProviders } from '../modelInterface';
 import { Dialog, DialogContent, DialogHeader, DialogTitle } from '../../../ui/dialog';
+import { fetchModelsForProviders } from '../modelInterface';
 
 interface LeadWorkerSettingsProps {
   isOpen: boolean;
@@ -14,7 +14,7 @@ interface LeadWorkerSettingsProps {
 }
 
 export function LeadWorkerSettings({ isOpen, onClose }: LeadWorkerSettingsProps) {
-  const { read, upsert, getProviders, getProviderModels, remove } = useConfig();
+  const { read, upsert, getProviders, remove } = useConfig();
   const { currentModel } = useModelAndProvider();
   const [leadModel, setLeadModel] = useState<string>('');
   const [workerModel, setWorkerModel] = useState<string>('');
@@ -104,7 +104,8 @@ export function LeadWorkerSettings({ isOpen, onClose }: LeadWorkerSettingsProps)
           const providers = await getProviders(false);
           const activeProviders = providers.filter((p) => p.is_configured);
 
-          const results = await fetchModelsForProviders(activeProviders, getProviderModels);
+          const results = await fetchModelsForProviders(activeProviders);
+
           results.forEach(({ provider: p, models, error }) => {
             if (error) {
               console.error(error);
@@ -139,7 +140,7 @@ export function LeadWorkerSettings({ isOpen, onClose }: LeadWorkerSettingsProps)
     };
 
     loadConfig();
-  }, [read, getProviders, getProviderModels, currentModel, isOpen]);
+  }, [read, getProviders, currentModel, isOpen]);
 
   // If current models are not in the list (e.g., previously set to custom), switch to custom mode
   useEffect(() => {

ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx

@@ -75,7 +75,7 @@ export const SwitchModelModal = ({
   initialProvider,
   titleOverride,
 }: SwitchModelModalProps) => {
-  const { getProviders, getProviderModels, read } = useConfig();
+  const { getProviders, read } = useConfig();
   const { changeModel, currentModel, currentProvider } = useModelAndProvider();
   const [providerOptions, setProviderOptions] = useState<{ value: string; label: string }[]>([]);
   type ModelOption = { value: string; label: string; provider: string; isDisabled?: boolean };
@@ -96,6 +96,7 @@ export const SwitchModelModal = ({
   const [predefinedModels, setPredefinedModels] = useState<Model[]>([]);
   const [loadingModels, setLoadingModels] = useState<boolean>(false);
   const [userClearedModel, setUserClearedModel] = useState(false);
+  const [providerErrors, setProviderErrors] = useState<Record<string, string>>({});
 
   // Validate form data
   const validateForm = useCallback(() => {
@@ -204,24 +205,22 @@ export const SwitchModelModal = ({
 
         setLoadingModels(true);
 
-        // Fetching models for all providers (always recommended)
+        const results = await fetchModelsForProviders(activeProviders);
-        const results = await fetchModelsForProviders(activeProviders, getProviderModels);
 
         // Process results and build grouped options
         const groupedOptions: {
           options: { value: string; label: string; provider: string; providerType: ProviderType }[];
         }[] = [];
-        const errors: string[] = [];
+        const errorMap: Record<string, string> = {};
 
         results.forEach(({ provider: p, models, error }) => {
-          const modelList = error
-            ? p.metadata.known_models?.map(({ name }) => name) || []
-            : models || [];
-
           if (error) {
-            errors.push(error);
+            errorMap[p.name] = error;
+            return;
           }
 
+          const modelList = models || [];
+
           const options: {
             value: string;
             label: string;
@@ -248,10 +247,8 @@ export const SwitchModelModal = ({
           }
         });
 
-        // Log errors if any providers failed (don't show to user)
+        // Save provider errors to state
-        if (errors.length > 0) {
+        setProviderErrors(errorMap);
-          console.error('Provider model fetch errors:', errors);
-        }
 
         setModelOptions(groupedOptions);
         setOriginalModelOptions(groupedOptions);
@@ -261,7 +258,7 @@ export const SwitchModelModal = ({
         setLoadingModels(false);
       }
     })();
-  }, [getProviders, getProviderModels, usePredefinedModels, read, initialProvider]);
+  }, [getProviders, usePredefinedModels, read, initialProvider]);
 
   const filteredModelOptions = provider
     ? modelOptions.filter((group) => group.options[0]?.provider === provider)
@@ -454,7 +451,24 @@ export const SwitchModelModal = ({
 
               {provider && (
                 <>
-                  {!isCustomModel ? (
+                  {providerErrors[provider] ? (
+                    /* Show error message when provider failed to connect */
+                    <div className="rounded-md bg-red-50 dark:bg-red-900/20 border border-red-200 dark:border-red-800 p-3">
+                      <div className="flex items-start">
+                        <div className="flex-1">
+                          <h3 className="text-sm font-medium text-red-800 dark:text-red-200">
+                            Could not contact provider
+                          </h3>
+                          <div className="mt-1 text-sm text-red-700 dark:text-red-300">
+                            {providerErrors[provider]}
+                          </div>
+                          <div className="mt-2 text-xs text-red-600 dark:text-red-400">
+                            Check your provider configuration in Settings → Providers
+                          </div>
+                        </div>
+                      </div>
+                    </div>
+                  ) : !isCustomModel ? (
                     <div>
                       <Select
                         options={
@@ -466,10 +480,14 @@ export const SwitchModelModal = ({
                         }
                         onChange={handleModelChange}
                         onInputChange={handleInputChange}
-                        value={model ? { value: model, label: model } : null}
+                        value={
-                        placeholder={
+                          loadingModels
-                          loadingModels ? 'Loading models…' : 'Select a model, type to search'
+                            ? { value: '', label: 'Loading models…', isDisabled: true }
+                            : model
+                              ? { value: model, label: model }
+                              : null
                         }
+                        placeholder="Select a model, type to search"
                         isClearable
                         isDisabled={loadingModels}
                       />

ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx

@@ -1,4 +1,4 @@
-import { checkProvider } from '../../../../../../api';
+import { getProviderModels, readConfig } from '../../../../../../api';
 
 /**
  * Standalone function to submit provider configuration
@@ -21,6 +21,30 @@ export const providerConfigSubmitHandler = async (
 ) => {
   const parameters = provider.metadata.config_keys || [];
 
+  // Save current NON-SECRET config values for rollback on failure
+  // We skip secrets because readConfig returns masked values for secrets,
+  // and upserting those masked values would corrupt the actual secret
+  const previousConfigValues: Record<string, { value: unknown; isSecret: boolean }> = {};
+  const nonSecretParams = parameters.filter((param) => !param.secret);
+
+  await Promise.all(
+    nonSecretParams.map(async (param) => {
+      try {
+        const currentValue = await readConfig({
+          body: { key: param.name, is_secret: false },
+        });
+        if (currentValue.data) {
+          previousConfigValues[param.name] = {
+            value: currentValue.data,
+            isSecret: false,
+          };
+        }
+      } catch {
+        // No previous value exists, that's fine
+      }
+    })
+  );
+
   const requiredParams = parameters.filter((param) => param.required);
   if (requiredParams.length === 0 && parameters.length > 0) {
     const allOptionalWithDefaults = parameters.every(
@@ -70,8 +94,19 @@ export const providerConfigSubmitHandler = async (
   );
 
   await Promise.all(upsertPromises);
-  await checkProvider({
+
-    body: { provider: provider.name },
+  try {
-    throwOnError: true,
+    await getProviderModels({
-  });
+      path: { name: provider.name },
+      throwOnError: true,
+    });
+  } catch (error) {
+    const rollbackPromises: Promise<void>[] = [];
+    for (const [key, { value, isSecret }] of Object.entries(previousConfigValues)) {
+      rollbackPromises.push(upsertFn(key, value, isSecret));
+    }
+    await Promise.all(rollbackPromises);
+
+    throw error;
+  }
 };