feat/fix: set optional config params, and don't overwrite unset secrets (#5325)

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

@@ -88,6 +88,19 @@ pub struct UpdateCustomProviderRequest {
     pub supports_streaming: Option<bool>,
 }
 
+#[derive(Serialize, ToSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct MaskedSecret {
+    pub masked_value: String,
+}
+
+#[derive(Serialize, ToSchema)]
+#[serde(untagged)]
+pub enum ConfigValueResponse {
+    Value(Value),
+    MaskedValue(MaskedSecret),
+}
+
 #[utoipa::path(
     post,
     path = "/config/upsert",
@@ -134,6 +147,22 @@ pub async fn remove_config(Json(query): Json<ConfigKeyQuery>) -> Result<Json<Str
     }
 }
 
+const SECRET_MASK_SHOW_LEN: usize = 8;
+
+fn mask_secret(secret: Value) -> String {
+    let as_string = match secret {
+        Value::String(s) => s,
+        _ => serde_json::to_string(&secret).unwrap_or_else(|_| secret.to_string()),
+    };
+
+    let chars: Vec<_> = as_string.chars().collect();
+    let show_len = std::cmp::min(chars.len() / 2, SECRET_MASK_SHOW_LEN);
+    let visible: String = chars.iter().take(show_len).collect();
+    let mask = "*".repeat(chars.len() - show_len);
+
+    format!("{}{}", visible, mask)
+}
+
 #[utoipa::path(
     post,
     path = "/config/read",
@@ -143,12 +172,14 @@ pub async fn remove_config(Json(query): Json<ConfigKeyQuery>) -> Result<Json<Str
         (status = 500, description = "Unable to get the configuration value"),
     )
 )]
-pub async fn read_config(Json(query): Json<ConfigKeyQuery>) -> Result<Json<Value>, StatusCode> {
+pub async fn read_config(
+    Json(query): Json<ConfigKeyQuery>,
+) -> Result<Json<ConfigValueResponse>, StatusCode> {
     if query.key == "model-limits" {
         let limits = ModelConfig::get_all_model_limits();
-        return Ok(Json(
+        return Ok(Json(ConfigValueResponse::Value(
             serde_json::to_value(limits).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
-        ));
+        )));
     }
 
     let config = Config::global();
@@ -156,18 +187,14 @@ pub async fn read_config(Json(query): Json<ConfigKeyQuery>) -> Result<Json<Value
     let response_value = match config.get(&query.key, query.is_secret) {
         Ok(value) => {
             if query.is_secret {
-                Value::Bool(true)
+                ConfigValueResponse::MaskedValue(MaskedSecret {
+                    masked_value: mask_secret(value),
+                })
             } else {
-                value
-            }
-        }
-        Err(ConfigError::NotFound(_)) => {
-            if query.is_secret {
-                Value::Bool(false)
-            } else {
-                Value::Null
+                ConfigValueResponse::Value(value)
             }
         }
+        Err(ConfigError::NotFound(_)) => ConfigValueResponse::Value(Value::Null),
         Err(_) => {
             return Err(StatusCode::INTERNAL_SERVER_ERROR);
         }
@@ -752,10 +779,12 @@ mod tests {
         .await;
 
         assert!(result.is_ok());
-        let response = result.unwrap();
+        let response = match result.unwrap().0 {
+            ConfigValueResponse::Value(value) => value,
+            ConfigValueResponse::MaskedValue(_) => panic!("unexpected secret"),
+        };
 
-        let limits: Vec<goose::model::ModelLimitConfig> =
-            serde_json::from_value(response.0).unwrap();
+        let limits: Vec<goose::model::ModelLimitConfig> = serde_json::from_value(response).unwrap();
         assert!(!limits.is_empty());
 
         let gpt4_limit = limits.iter().find(|l| l.pattern == "gpt-4o");

ui/desktop/src/components/settings/providers/ProviderGrid.tsx

@@ -1,7 +1,6 @@
 import React, { memo, useMemo, useCallback, useState } from 'react';
 import { ProviderCard } from './subcomponents/ProviderCard';
 import CardContainer from './subcomponents/CardContainer';
-import { ProviderModalProvider, useProviderModal } from './modal/ProviderModalProvider';
 import ProviderConfigurationModal from './modal/ProviderConfiguationModal';
 import {
   DeclarativeProviderConfig,
@@ -47,7 +46,7 @@ const CustomProviderCard = memo(function CustomProviderCard({ onClick }: { onCli
   );
 });
 
-const ProviderCards = memo(function ProviderCards({
+function ProviderCards({
   providers,
   isOnboarding,
   refreshProviders,
@@ -58,7 +57,7 @@ const ProviderCards = memo(function ProviderCards({
   refreshProviders?: () => void;
   onProviderLaunch: (provider: ProviderDetails) => void;
 }) {
-  const { openModal } = useProviderModal();
+  const [configuringProvider, setConfiguringProvider] = useState<ProviderDetails | null>(null);
   const [showCustomProviderModal, setShowCustomProviderModal] = useState(false);
   const [editingProvider, setEditingProvider] = useState<{
     id: string;
@@ -66,6 +65,11 @@ const ProviderCards = memo(function ProviderCards({
     isEditable: boolean;
   } | null>(null);
 
+  const openModal = useCallback(
+    (provider: ProviderDetails) => setConfiguringProvider(provider),
+    []
+  );
+
   const configureProviderViaModal = useCallback(
     async (provider: ProviderDetails) => {
       if (provider.provider_type === 'Custom' || provider.provider_type === 'Declarative') {
@@ -81,22 +85,10 @@ const ProviderCards = memo(function ProviderCards({
           setShowCustomProviderModal(true);
         }
       } else {
-        openModal(provider, {
-          onSubmit: () => {
-            if (refreshProviders) {
-              refreshProviders();
-            }
-          },
-          onDelete: (_values: unknown) => {
-            if (refreshProviders) {
-              refreshProviders();
-            }
-          },
-          formProps: {},
-        });
+        openModal(provider);
       }
     },
-    [openModal, refreshProviders]
+    [openModal]
   );
 
   const handleUpdateCustomProvider = useCallback(
@@ -123,20 +115,12 @@ const ProviderCards = memo(function ProviderCards({
     setEditingProvider(null);
   }, []);
 
-  const deleteProviderConfigViaModal = useCallback(
-    (provider: ProviderDetails) => {
-      openModal(provider, {
-        onDelete: (_values: unknown) => {
-          // Only refresh if the function is provided
-          if (refreshProviders) {
-            refreshProviders();
-          }
-        },
-        formProps: {},
-      });
-    },
-    [openModal, refreshProviders]
-  );
+  const onCloseProviderConfig = useCallback(() => {
+    setConfiguringProvider(null);
+    if (refreshProviders) {
+      refreshProviders();
+    }
+  }, [refreshProviders]);
 
   const handleCreateCustomProvider = useCallback(
     async (data: UpdateCustomProviderRequest) => {
@@ -160,7 +144,6 @@ const ProviderCards = memo(function ProviderCards({
         key={provider.name}
         provider={provider}
         onConfigure={() => configureProviderViaModal(provider)}
-        onDelete={() => deleteProviderConfigViaModal(provider)}
         onLaunch={() => onProviderLaunch(provider)}
         isOnboarding={isOnboarding}
       />
@@ -171,13 +154,7 @@ const ProviderCards = memo(function ProviderCards({
     );
 
     return cards;
-  }, [
-    providers,
-    isOnboarding,
-    configureProviderViaModal,
-    deleteProviderConfigViaModal,
-    onProviderLaunch,
-  ]);
+  }, [providers, isOnboarding, configureProviderViaModal, onProviderLaunch]);
 
   const initialData = editingProvider && {
     engine: editingProvider.config.engine.toLowerCase() + '_compatible',
@@ -206,11 +183,17 @@ const ProviderCards = memo(function ProviderCards({
           />
         </DialogContent>
       </Dialog>{' '}
+      {configuringProvider && (
+        <ProviderConfigurationModal
+          provider={configuringProvider}
+          onClose={onCloseProviderConfig}
+        />
+      )}
     </>
   );
-});
+}
 
-export default memo(function ProviderGrid({
+export default function ProviderGrid({
   providers,
   isOnboarding,
   refreshProviders,
@@ -221,20 +204,14 @@ export default memo(function ProviderGrid({
   refreshProviders?: () => void;
   onProviderLaunch?: (provider: ProviderDetails) => void;
 }) {
-  // Memoize the modal provider and its children to avoid recreating on every render
-  const modalProviderContent = useMemo(
-    () => (
-      <ProviderModalProvider>
-        <ProviderCards
-          providers={providers}
-          isOnboarding={isOnboarding}
-          refreshProviders={refreshProviders}
-          onProviderLaunch={onProviderLaunch || (() => {})}
-        />
-        <ProviderConfigurationModal />
-      </ProviderModalProvider>
-    ),
-    [providers, isOnboarding, refreshProviders, onProviderLaunch]
+  return (
+    <GridLayout>
+      <ProviderCards
+        providers={providers}
+        isOnboarding={isOnboarding}
+        refreshProviders={refreshProviders}
+        onProviderLaunch={onProviderLaunch || (() => {})}
+      />
+    </GridLayout>
   );
-  return <GridLayout>{modalProviderContent}</GridLayout>;
-});
+}

ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useState } from 'react';
 import {
   Dialog,
   DialogContent,
@@ -7,198 +7,123 @@ import {
   DialogHeader,
   DialogTitle,
 } from '../../../ui/dialog';
-import DefaultProviderSetupForm from './subcomponents/forms/DefaultProviderSetupForm';
+import DefaultProviderSetupForm, {
+  ConfigInput,
+} from './subcomponents/forms/DefaultProviderSetupForm';
 import ProviderSetupActions from './subcomponents/ProviderSetupActions';
 import ProviderLogo from './subcomponents/ProviderLogo';
-import { useProviderModal } from './ProviderModalProvider';
 import { SecureStorageNotice } from './subcomponents/SecureStorageNotice';
-import { DefaultSubmitHandler } from './subcomponents/handlers/DefaultSubmitHandler';
-import OllamaSubmitHandler from './subcomponents/handlers/OllamaSubmitHandler';
-import OllamaForm from './subcomponents/forms/OllamaForm';
+import { providerConfigSubmitHandler } from './subcomponents/handlers/DefaultSubmitHandler';
 import { useConfig } from '../../../ConfigContext';
 import { useModelAndProvider } from '../../../ModelAndProviderContext';
 import { AlertTriangle } from 'lucide-react';
-import { ConfigKey, removeCustomProvider } from '../../../../api';
+import { ProviderDetails, removeCustomProvider } from '../../../../api';
 
-interface FormValues {
-  [key: string]: string | number | boolean | null;
+interface ProviderConfigurationModalProps {
+  provider: ProviderDetails;
+  onClose: () => void;
 }
 
-const customSubmitHandlerMap: Record<string, unknown> = {
-  provider_name: OllamaSubmitHandler, // example
-};
-
-const customFormsMap: Record<string, unknown> = {
-  provider_name: OllamaForm, // example
-};
-
-export default function ProviderConfigurationModal() {
+export default function ProviderConfigurationModal({
+  provider,
+  onClose,
+}: ProviderConfigurationModalProps) {
   const [validationErrors, setValidationErrors] = useState<Record<string, string>>({});
   const { upsert, remove } = useConfig();
   const { getCurrentModelAndProvider } = useModelAndProvider();
-  const { isOpen, currentProvider, modalProps, closeModal } = useProviderModal();
-  const [configValues, setConfigValues] = useState<Record<string, string>>({});
+  const [configValues, setConfigValues] = useState<Record<string, ConfigInput>>({});
   const [showDeleteConfirmation, setShowDeleteConfirmation] = useState(false);
-  const [isActiveProvider, setIsActiveProvider] = useState(false); // New state for tracking active provider
-  const [requiredParameters, setRequiredParameters] = useState<ConfigKey[]>([]); // New state for tracking active provider
+  const [isActiveProvider, setIsActiveProvider] = useState(false);
 
-  useEffect(() => {
-    if (isOpen && currentProvider) {
-      // Reset form state when the modal opens with a new provider
-      const requiredParameters = currentProvider.metadata.config_keys.filter(
-        (param) => param.required === true
-      );
-      setRequiredParameters(requiredParameters);
-      setConfigValues({});
-      setValidationErrors({});
-      setShowDeleteConfirmation(false);
-      setIsActiveProvider(false); // Reset active provider state
-    }
-  }, [isOpen, currentProvider]);
+  const requiredParameters = provider.metadata.config_keys.filter(
+    (param) => param.required === true
+  );
 
-  if (!isOpen || !currentProvider) return null;
-
-  const isConfigured = currentProvider.is_configured;
+  const isConfigured = provider.is_configured;
   const headerText = showDeleteConfirmation
-    ? `Delete configuration for ${currentProvider.metadata.display_name}`
-    : `Configure ${currentProvider.metadata.display_name}`;
+    ? `Delete configuration for ${provider.metadata.display_name}`
+    : `Configure ${provider.metadata.display_name}`;
 
-  // Modify description text to show warning if it's the active provider
   const descriptionText = showDeleteConfirmation
     ? isActiveProvider
       ? `You cannot delete this provider while it's currently in use. Please switch to a different model first.`
       : 'This will permanently delete the current provider configuration.'
     : `Add your API key(s) for this provider to integrate into Goose`;
 
-  const SubmitHandler =
-    (customSubmitHandlerMap[currentProvider.name] as typeof DefaultSubmitHandler) ||
-    DefaultSubmitHandler;
-  const FormComponent =
-    (customFormsMap[currentProvider.name] as typeof DefaultProviderSetupForm) ||
-    DefaultProviderSetupForm;
-
   const handleSubmitForm = async (e: React.FormEvent) => {
     e.preventDefault();
-    console.log('Form submitted for:', currentProvider.name);
 
-    // Reset previous validation errors
     setValidationErrors({});
 
-    // Validation logic
-    const parameters = currentProvider.metadata.config_keys || [];
+    const parameters = provider.metadata.config_keys || [];
     const errors: Record<string, string> = {};
 
-    // Check required fields
     parameters.forEach((parameter) => {
       if (
         parameter.required &&
-        (configValues[parameter.name] === undefined ||
-          configValues[parameter.name] === null ||
-          configValues[parameter.name] === '')
+        !configValues[parameter.name]?.value &&
+        !configValues[parameter.name]?.serverValue
       ) {
         errors[parameter.name] = `${parameter.name} is required`;
       }
     });
 
-    // If there are validation errors, stop the submission
     if (Object.keys(errors).length > 0) {
       setValidationErrors(errors);
-      return; // Stop the submission process
+      return;
     }
 
-    try {
-      // Wait for the submission to complete
-      await SubmitHandler(upsert, currentProvider, configValues);
+    const toSubmit = Object.fromEntries(
+      Object.entries(configValues)
+        .filter(([_k, entry]) => !!entry.value)
+        .map(([k, entry]) => [k, entry.value || ''])
+    );
 
-      // Close the modal before triggering refreshes to avoid UI issues
-      closeModal();
-
-      // Call onSubmit callback if provided (from modal props)
-      if (modalProps.onSubmit) {
-        modalProps.onSubmit(configValues as FormValues);
-      }
-    } catch (error) {
-      console.error('Failed to save configuration:', error);
-      // Keep modal open if there's an error
-    }
+    await providerConfigSubmitHandler(upsert, provider, toSubmit);
+    onClose();
   };
 
   const handleCancel = () => {
-    // Reset delete confirmation state
-    setShowDeleteConfirmation(false);
-    setIsActiveProvider(false);
-
-    // Use custom cancel handler if provided
-    if (modalProps.onCancel) {
-      modalProps.onCancel();
-    }
-
-    closeModal();
+    onClose();
   };
 
   const handleDelete = async () => {
-    // Check if this is the currently active provider
     try {
       const providerModel = await getCurrentModelAndProvider();
-      if (currentProvider.name === providerModel.provider) {
-        // It's the active provider - set state and show warning
+      if (provider.name === providerModel.provider) {
         setIsActiveProvider(true);
         setShowDeleteConfirmation(true);
-        return; // Exit early - don't allow actual deletion
+        return;
       }
     } catch (error) {
       console.error('Failed to check current provider:', error);
     }
 
-    // If we get here, it's not the active provider
     setIsActiveProvider(false);
     setShowDeleteConfirmation(true);
   };
 
   const handleConfirmDelete = async () => {
-    // Don't proceed if this is the active provider
     if (isActiveProvider) {
       return;
     }
 
-    try {
-      const isCustomProvider = currentProvider.provider_type === 'Custom';
+    const isCustomProvider = provider.provider_type === 'Custom';
 
-      if (isCustomProvider) {
-        await removeCustomProvider({
-          path: { id: currentProvider.name },
-        });
-      } else {
-        // Remove the provider configuration
-        // get the keys
-        const params = currentProvider.metadata.config_keys;
-
-        // go through the keys are remove them
-        for (const param of params) {
-          await remove(param.name, param.secret);
-        }
+    if (isCustomProvider) {
+      await removeCustomProvider({
+        path: { id: provider.name },
+      });
+    } else {
+      const params = provider.metadata.config_keys;
+      for (const param of params) {
+        await remove(param.name, param.secret);
       }
-
-      // Call onDelete callback if provided
-      // This should trigger the refreshProviders function
-      if (modalProps.onDelete) {
-        modalProps.onDelete(currentProvider.name as unknown as FormValues);
-      }
-
-      // Reset the delete confirmation state before closing
-      setShowDeleteConfirmation(false);
-      setIsActiveProvider(false);
-
-      // Close the modal
-      // Close the modal after deletion and callback
-      closeModal();
-    } catch (error) {
-      console.error('Failed to delete provider:', error);
-      // Keep modal open if there's an error
     }
+
+    onClose();
   };
 
-  // Function to determine which icon to display
   const getModalIcon = () => {
     if (showDeleteConfirmation) {
       return (
@@ -208,11 +133,11 @@ export default function ProviderConfigurationModal() {
         />
       );
     }
-    return <ProviderLogo providerName={currentProvider.name} />;
+    return <ProviderLogo providerName={provider.name} />;
   };
 
   return (
-    <Dialog open={isOpen} onOpenChange={(open) => !open && closeModal()}>
+    <Dialog open onOpenChange={(open) => !open && onClose()}>
       <DialogContent className="sm:max-w-[600px] max-h-[90vh] overflow-y-auto">
         <DialogHeader>
           <DialogTitle className="flex items-center gap-2">
@@ -228,17 +153,16 @@ export default function ProviderConfigurationModal() {
           {!showDeleteConfirmation ? (
             <>
               {/* Contains information used to set up each provider */}
-              <FormComponent
+              <DefaultProviderSetupForm
                 configValues={configValues}
                 setConfigValues={setConfigValues}
-                provider={currentProvider}
+                provider={provider}
                 validationErrors={validationErrors}
-                {...(modalProps.formProps || {})} // Spread any custom form props
               />
 
               {requiredParameters.length > 0 &&
-                currentProvider.metadata.config_keys &&
-                currentProvider.metadata.config_keys.length > 0 && <SecureStorageNotice />}
+                provider.metadata.config_keys &&
+                provider.metadata.config_keys.length > 0 && <SecureStorageNotice />}
             </>
           ) : null}
         </div>
@@ -252,11 +176,11 @@ export default function ProviderConfigurationModal() {
             showDeleteConfirmation={showDeleteConfirmation}
             onConfirmDelete={handleConfirmDelete}
             onCancelDelete={() => {
-              setShowDeleteConfirmation(false);
               setIsActiveProvider(false);
+              setShowDeleteConfirmation(false);
             }}
             canDelete={isConfigured && !isActiveProvider}
-            providerName={currentProvider.metadata.display_name}
+            providerName={provider.metadata.display_name}
             isActiveProvider={isActiveProvider}
           />
         </DialogFooter>

ui/desktop/src/components/settings/providers/modal/ProviderModalProvider.tsx

@@ -1,65 +0,0 @@
-import React, { createContext, useContext, useState } from 'react';
-import { ProviderDetails } from '../../../../api/types.gen';
-
-interface FormValues {
-  [key: string]: string | number | boolean | null;
-}
-
-interface ModalProps {
-  onSubmit?: (values: FormValues) => void;
-  onCancel?: () => void;
-  onDelete?: (values: FormValues) => void;
-  formProps?: {
-    initialValues?: FormValues;
-    validationSchema?: object;
-    [key: string]: unknown;
-  };
-}
-
-interface ProviderModalContextType {
-  isOpen: boolean;
-  currentProvider: ProviderDetails | null;
-  modalProps: ModalProps;
-  openModal: (provider: ProviderDetails, additionalProps?: ModalProps) => void;
-  closeModal: () => void;
-}
-
-const ProviderModalContext = createContext<ProviderModalContextType | undefined>(undefined);
-
-export const ProviderModalProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => {
-  const [isOpen, setIsOpen] = useState(false);
-  const [currentProvider, setCurrentProvider] = useState<ProviderDetails | null>(null);
-  const [modalProps, setModalProps] = useState<ModalProps>({});
-
-  const openModal = (provider: ProviderDetails, additionalProps: ModalProps = {}) => {
-    setCurrentProvider(provider);
-    setModalProps(additionalProps);
-    setIsOpen(true);
-  };
-
-  const closeModal = () => {
-    setIsOpen(false);
-  };
-
-  return (
-    <ProviderModalContext.Provider
-      value={{
-        isOpen,
-        currentProvider,
-        modalProps,
-        openModal,
-        closeModal,
-      }}
-    >
-      {children}
-    </ProviderModalContext.Provider>
-  );
-};
-
-export const useProviderModal = () => {
-  const context = useContext(ProviderModalContext);
-  if (context === undefined) {
-    throw new Error('useProviderModal must be used within a ProviderModalProvider');
-  }
-  return context;
-};

ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx

@@ -1,17 +1,40 @@
 import React, { useEffect, useMemo, useState, useCallback } from 'react';
 import { Input } from '../../../../../ui/input';
-import { useConfig } from '../../../../../ConfigContext'; // Adjust this import path as needed
+import { useConfig } from '../../../../../ConfigContext';
 import { ProviderDetails, ConfigKey } from '../../../../../../api';
+import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '../../../../../ui/collapsible';
 
 type ValidationErrors = Record<string, string>;
 
+type ConfigValue = string | { maskedValue: string };
+export interface ConfigInput {
+  serverValue?: ConfigValue;
+  value?: string;
+}
+
 interface DefaultProviderSetupFormProps {
-  configValues: Record<string, string>;
-  setConfigValues: React.Dispatch<React.SetStateAction<Record<string, string>>>;
+  configValues: Record<string, ConfigInput>;
+  setConfigValues: React.Dispatch<React.SetStateAction<Record<string, ConfigInput>>>;
   provider: ProviderDetails;
   validationErrors: ValidationErrors;
 }
 
+const envToPrettyName = (envVar: string) => {
+  const wordReplacements: { [w: string]: string } = {
+    Api: 'API',
+    Aws: 'AWS',
+    Gcp: 'GCP',
+  };
+
+  return envVar
+    .toLowerCase()
+    .split('_')
+    .map((word) => word.charAt(0).toUpperCase() + word.slice(1))
+    .map((word) => wordReplacements[word] || word)
+    .join(' ')
+    .trim();
+};
+
 export default function DefaultProviderSetupForm({
   configValues,
   setConfigValues,
@@ -23,73 +46,49 @@ export default function DefaultProviderSetupForm({
     [provider.metadata.config_keys]
   );
   const [isLoading, setIsLoading] = useState(true);
+  const [optionalExpanded, setOptionalExpanded] = useState(false);
   const { read } = useConfig();
 
-  console.log('configValues default form', configValues);
-
-  // Initialize values when the component mounts or provider changes
   const loadConfigValues = useCallback(async () => {
     setIsLoading(true);
-    const newValues = { ...configValues };
+    try {
+      const values: { [k: string]: ConfigInput } = {};
 
-    // Try to load actual values from config for each parameter that is not secret
-    for (const parameter of parameters) {
-      try {
-        // Check if there's a stored value in the config system
+      for (const parameter of parameters) {
         const configKey = `${parameter.name}`;
-        const configResponse = await read(configKey, parameter.secret || false);
+        const configValue = (await read(configKey, parameter.secret || false)) as ConfigValue;
 
-        if (configResponse) {
-          newValues[parameter.name] = parameter.secret ? 'true' : String(configResponse);
-        } else if (
-          parameter.default !== undefined &&
-          parameter.default !== null &&
-          !configValues[parameter.name]
-        ) {
-          // Fall back to default value if no config value exists
-          newValues[parameter.name] = String(parameter.default);
-        }
-      } catch (error) {
-        console.error(`Failed to load config for ${parameter.name}:`, error);
-        // Fall back to default if read operation fails
-        if (
-          parameter.default !== undefined &&
-          parameter.default !== null &&
-          !configValues[parameter.name]
-        ) {
-          newValues[parameter.name] = String(parameter.default);
+        if (configValue) {
+          values[parameter.name] = { serverValue: configValue };
+        } else if (parameter.default !== undefined && parameter.default !== null) {
+          values[parameter.name] = { value: parameter.default };
         }
       }
-    }
 
-    // Update state with loaded values
-    setConfigValues((prev) => ({
-      ...prev,
-      ...newValues,
-    }));
-    setIsLoading(false);
-  }, [configValues, parameters, read, setConfigValues]);
+      setConfigValues((prev) => ({
+        ...prev,
+        ...values,
+      }));
+    } finally {
+      setIsLoading(false);
+    }
+  }, [parameters, read, setConfigValues]);
 
   useEffect(() => {
     loadConfigValues();
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  // Filter parameters to only show required ones
-  const requiredParameters = useMemo(() => {
-    return parameters.filter((param) => param.required === true);
-  }, [parameters]);
-
-  // TODO: show all params, not just required ones
-  // const allParameters = useMemo(() => {
-  //   return parameters;
-  // }, [parameters]);
-
-  // Helper function to generate appropriate placeholder text
   const getPlaceholder = (parameter: ConfigKey): string => {
-    // If default is defined and not null, show it
+    if (parameter.secret) {
+      const serverValue = configValues[parameter.name]?.serverValue;
+      if (typeof serverValue === 'object' && 'maskedValue' in serverValue) {
+        return serverValue.maskedValue;
+      }
+    }
+
     if (parameter.default !== undefined && parameter.default !== null) {
-      return `Default: ${parameter.default}`;
+      return parameter.default;
     }
 
     const name = parameter.name.toLowerCase();
@@ -99,66 +98,112 @@ export default function DefaultProviderSetupForm({
 
     return parameter.name
       .replace(/_/g, ' ')
-      .replace(/([A-Z])/g, ' $1')
       .replace(/^./, (str) => str.toUpperCase())
       .trim();
   };
 
-  // helper for custom labels
-  const getFieldLabel = (parameter: ConfigKey): string => {
+  const getFieldLabel = (parameter: ConfigKey) => {
     const name = parameter.name.toLowerCase();
     if (name.includes('api_key')) return 'API Key';
     if (name.includes('api_url') || name.includes('host')) return 'API Host';
     if (name.includes('models')) return 'Models';
 
-    return parameter.name
-      .replace(/_/g, ' ')
-      .replace(/([A-Z])/g, ' $1')
-      .replace(/^./, (str) => str.toUpperCase())
-      .trim();
+    let parameter_name = parameter.name.toUpperCase();
+    if (parameter_name.startsWith(provider.name.toUpperCase().replace('-', '_'))) {
+      parameter_name = parameter_name.slice(provider.name.length + 1);
+    }
+    let pretty = envToPrettyName(parameter_name);
+    return (
+      <span>
+        <span>{pretty}</span>
+        <span className="text-sm font-light ml-2">({parameter.name})</span>
+      </span>
+    );
   };
 
   if (isLoading) {
     return <div className="text-center py-4">Loading configuration values...</div>;
   }
 
-  console.log('required params', requiredParameters);
+  function getRenderValue(parameter: ConfigKey): string | undefined {
+    if (parameter.secret) {
+      return undefined;
+    }
+
+    const entry = configValues[parameter.name];
+    return entry?.value || (entry?.serverValue as string) || '';
+  }
+
+  const renderParametersList = (parameters: ConfigKey[]) => {
+    return parameters.map((parameter) => (
+      <div key={parameter.name}>
+        <label className="block text-sm font-medium text-textStandard mb-1">
+          {getFieldLabel(parameter)}
+          {parameter.required && <span className="text-red-500 ml-1">*</span>}
+        </label>
+        <Input
+          type="text"
+          value={getRenderValue(parameter)}
+          onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
+            setConfigValues((prev) => {
+              const newValue = { ...(prev[parameter.name] || {}), value: e.target.value };
+              return {
+                ...prev,
+                [parameter.name]: newValue,
+              };
+            });
+          }}
+          placeholder={getPlaceholder(parameter)}
+          className={`w-full h-14 px-4 font-regular rounded-lg shadow-none ${
+            validationErrors[parameter.name]
+              ? 'border-2 border-red-500'
+              : 'border border-borderSubtle hover:border-borderStandard'
+          } bg-background-default text-lg placeholder:text-textSubtle font-regular text-textStandard`}
+          required={parameter.required}
+        />
+        {validationErrors[parameter.name] && (
+          <p className="text-red-500 text-sm mt-1">{validationErrors[parameter.name]}</p>
+        )}
+      </div>
+    ));
+  };
+
+  let aboveFoldParameters = parameters.filter((p) => p.required);
+  let belowFoldParameters = parameters.filter((p) => !p.required);
+  if (aboveFoldParameters.length === 0) {
+    aboveFoldParameters = belowFoldParameters;
+    belowFoldParameters = [];
+  }
+
+  const expandCtaText = `${optionalExpanded ? 'Hide' : 'Show'} ${belowFoldParameters.length} options `;
+
   return (
     <div className="mt-4 space-y-4">
-      {requiredParameters.length === 0 ? (
+      {aboveFoldParameters.length === 0 && belowFoldParameters.length === 0 ? (
         <div className="text-center text-gray-500">
-          No required configuration for this provider.
+          No configuration parameters for this provider.
         </div>
       ) : (
-        requiredParameters.map((parameter) => (
-          <div key={parameter.name}>
-            <label className="block text-sm font-medium text-textStandard mb-1">
-              {getFieldLabel(parameter)}
-              {parameter.required && <span className="text-red-500 ml-1">*</span>}
-            </label>
-            <Input
-              type={parameter.secret ? 'password' : 'text'}
-              value={configValues[parameter.name] || ''}
-              onChange={(e: React.ChangeEvent<HTMLInputElement>) => {
-                console.log(`Setting ${parameter.name} to:`, e.target.value);
-                setConfigValues((prev) => ({
-                  ...prev,
-                  [parameter.name]: e.target.value,
-                }));
-              }}
-              placeholder={getPlaceholder(parameter)}
-              className={`w-full h-14 px-4 font-regular rounded-lg shadow-none ${
-                validationErrors[parameter.name]
-                  ? 'border-2 border-red-500'
-                  : 'border border-borderSubtle hover:border-borderStandard'
-              } bg-background-default text-lg placeholder:text-textSubtle font-regular text-textStandard`}
-              required={parameter.required}
-            />
-            {validationErrors[parameter.name] && (
-              <p className="text-red-500 text-sm mt-1">{validationErrors[parameter.name]}</p>
-            )}
-          </div>
-        ))
+        <div>
+          <div>{renderParametersList(aboveFoldParameters)}</div>
+          {belowFoldParameters.length > 0 && (
+            <Collapsible
+              open={optionalExpanded}
+              onOpenChange={setOptionalExpanded}
+              className="my-4 border-2 border-dashed border-secondary rounded-lg bg-secondary/10"
+            >
+              <CollapsibleTrigger className="m-3 w-full">
+                <div>
+                  <span className="text-sm">{expandCtaText}</span>
+                  <span className="text-sm">{optionalExpanded ? '↑' : '↓'}</span>
+                </div>
+              </CollapsibleTrigger>
+              <CollapsibleContent className="mx-3 mb-3">
+                {renderParametersList(belowFoldParameters)}
+              </CollapsibleContent>
+            </Collapsible>
+          )}
+        </div>
       )}
     </div>
   );

ui/desktop/src/components/settings/providers/modal/subcomponents/forms/OllamaForm.tsx

@@ -1,149 +0,0 @@
-import { PROVIDER_REGISTRY } from '../../../ProviderRegistry';
-import { Input } from '../../../../../ui/input';
-
-import React, { useState, useEffect, useCallback } from 'react';
-import { RefreshCw } from 'lucide-react';
-import CustomRadio from '../../../../../ui/CustomRadio';
-
-export default function OllamaForm({
-  configValues,
-  setConfigValues,
-  provider,
-}: {
-  configValues: Record<string, string>;
-  setConfigValues: React.Dispatch<React.SetStateAction<Record<string, string>>>;
-  provider: { name: string; [key: string]: unknown };
-}) {
-  const providerEntry = PROVIDER_REGISTRY.find((p) => p.name === provider.name);
-  const parameters = providerEntry?.details?.parameters || [];
-  const [isCheckingLocal, setIsCheckingLocal] = useState(false);
-  const [isLocalAvailable, setIsLocalAvailable] = useState(false);
-
-  const handleConnectionTypeChange = useCallback(
-    (value: string) => {
-      setConfigValues((prev) => ({
-        ...prev,
-        connection_type: value,
-      }));
-    },
-    [setConfigValues]
-  );
-
-  // Function to handle input changes and auto-select/deselect the host radio
-  const handleInputChange = (paramName: string, value: string) => {
-    // Update the parameter value
-    setConfigValues((prev) => ({
-      ...prev,
-      [paramName]: value,
-    }));
-
-    // If the user is typing, auto-select the host radio button
-    if (value && configValues.connection_type !== 'host') {
-      handleConnectionTypeChange('host');
-    }
-    // If the input becomes empty and the host radio is selected, switch to local if available
-    else if (!value && configValues.connection_type === 'host') {
-      if (isLocalAvailable) {
-        handleConnectionTypeChange('local');
-      }
-      // If local is not available, we keep the host selected but leave the input empty
-    }
-  };
-
-  const checkLocalAvailability = useCallback(async () => {
-    setIsCheckingLocal(true);
-
-    // Dummy implementation - simulates checking local availability
-    try {
-      console.log('Checking for local Ollama instance...');
-      // Simulate a network request with a delay
-      await new Promise((resolve) => setTimeout(resolve, 800));
-
-      // Randomly determine if Ollama is available (for demo purposes)
-      const isAvailable = Math.random() > 0.3;
-      setIsLocalAvailable(isAvailable);
-
-      if (isAvailable) {
-        console.log('Local Ollama instance found');
-        // Enable local radio button
-      } else {
-        console.log('No local Ollama instance found');
-        // If current selection is local, switch to host
-        if (configValues.connection_type === 'local') {
-          handleConnectionTypeChange('host');
-        }
-      }
-    } catch (error) {
-      console.error('Error checking for local Ollama:', error);
-      setIsLocalAvailable(false);
-    } finally {
-      setIsCheckingLocal(false);
-    }
-  }, [configValues.connection_type, handleConnectionTypeChange]);
-
-  // Check local availability on initial load
-  useEffect(() => {
-    checkLocalAvailability();
-  }, [checkLocalAvailability]);
-
-  return (
-    <div className="mt-4 space-y-4">
-      <div className="font-medium text-gray-900 dark:text-gray-100 mb-2">Connection</div>
-
-      {/* Local Option */}
-      <div className="flex items-center mb-3 justify-between">
-        <div className="flex items-center">
-          <span className="text-gray-700 dark:text-gray-300">Background App</span>
-          <button
-            type="button"
-            className="ml-2 p-1 rounded-full hover:bg-gray-100 dark:hover:bg-gray-800"
-            onClick={checkLocalAvailability}
-            disabled={isCheckingLocal}
-          >
-            <RefreshCw
-              className={`w-4 h-4 ${isCheckingLocal ? 'animate-spin' : ''} text-gray-600 dark:text-gray-400`}
-            />
-          </button>
-        </div>
-
-        <CustomRadio
-          id="connection-local"
-          name="connection_type"
-          value="local"
-          checked={configValues.connection_type === 'local'}
-          onChange={() => handleConnectionTypeChange('local')}
-          disabled={!isLocalAvailable}
-        />
-      </div>
-
-      {/* Other Parameters */}
-      {parameters
-        .filter((param) => param.name !== 'host_url') // Skip host_url as we handle it above
-        .map((parameter) => (
-          <div key={parameter.name} className="flex items-center mb-4">
-            <div className="flex-grow">
-              <Input
-                type={parameter.is_secret ? 'password' : 'text'}
-                value={configValues[parameter.name] || ''}
-                onChange={(e) => handleInputChange(parameter.name, e.target.value)}
-                placeholder={
-                  parameter.default ? parameter.default : parameter.name.replace(/_/g, ' ')
-                }
-                className="w-full h-14 px-4 font-regular rounded-lg border shadow-none border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-900 text-lg placeholder:text-gray-400 dark:placeholder:text-gray-500 font-regular text-gray-900 dark:text-gray-100"
-                required={parameter.default == null}
-              />
-            </div>
-            <div className="ml-4">
-              <CustomRadio
-                id={`connection-host-${parameter.name}`}
-                name="connection_type"
-                value="host"
-                checked={configValues.connection_type === 'host'}
-                onChange={() => handleConnectionTypeChange('host')}
-              />
-            </div>
-          </div>
-        ))}
-    </div>
-  );
-}

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

@@ -2,7 +2,7 @@
  * Standalone function to submit provider configuration
  * Useful for components that don't want to use the hook
  */
-export const DefaultSubmitHandler = async (
+export const providerConfigSubmitHandler = async (
   upsertFn: (key: string, value: unknown, isSecret: boolean) => Promise<void>,
   provider: {
     name: string;
@@ -15,7 +15,7 @@ export const DefaultSubmitHandler = async (
       }>;
     };
   },
-  configValues: Record<string, unknown>
+  configValues: Record<string, string>
 ) => {
   const parameters = provider.metadata.config_keys || [];
 

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

@@ -1,7 +0,0 @@
-export default function OllamaSubmitHandler(configValues: Record<string, unknown>) {
-  // Log each field value individually for clarity
-  console.log('Ollama field values:');
-  Object.entries(configValues).forEach(([key, value]) => {
-    console.log(`${key}: ${value}`);
-  });
-}

ui/desktop/src/components/settings/providers/subcomponents/ProviderCard.tsx

@@ -1,4 +1,4 @@
-import { memo, useMemo } from 'react';
+import { useMemo } from 'react';
 import CardContainer from './CardContainer';
 import CardHeader from './CardHeader';
 import CardBody from './CardBody';
@@ -9,11 +9,10 @@ type ProviderCardProps = {
   provider: ProviderDetails;
   onConfigure: () => void;
   onLaunch: () => void;
-  onDelete: () => void;
   isOnboarding: boolean;
 };
 
-export const ProviderCard = memo(function ProviderCard({
+export const ProviderCard = function ProviderCard({
   provider,
   onConfigure,
   onLaunch,
@@ -59,4 +58,4 @@ export const ProviderCard = memo(function ProviderCard({
       }
     />
   );
-});
+};

ui/desktop/src/components/ui/input.tsx

@@ -8,7 +8,7 @@ const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<'input'>>(
       <input
         type={type}
         className={cn(
-          'flex h-9 w-full rounded-md border focus:border-border-strong hover:border-border-strong bg-background-default px-3 py-1 text-base transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-textPlaceholder focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
+          'flex h-9 w-full rounded-md border focus:border-border-strong hover:border-border-strong bg-background-default px-3 py-1 text-base transition-colors file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-textPlaceholder placeholder:font-light focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm',
           className
         )}
         ref={ref}