ui: auto update card upon config (#1610)

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

@@ -11,9 +11,9 @@ use http::{HeaderMap, StatusCode};
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::collections::HashMap;
-use std::env;
 use utoipa::ToSchema;
 
+use crate::routes::utils::check_provider_configured;
 use crate::state::AppState;
 
 fn verify_secret_key(headers: &HeaderMap, state: &AppState) -> Result<StatusCode, StatusCode> {
@@ -123,7 +123,7 @@ pub async fn remove_config(
 }
 
 #[utoipa::path(
-    post, // Change from get to post
+    post,
     path = "/config/read",
     request_body = ConfigKeyQuery, // Switch back to request_body
     responses(
@@ -335,31 +335,6 @@ pub async fn providers(
     Ok(Json(providers_response))
 }
 
-fn check_provider_configured(metadata: &ProviderMetadata) -> bool {
-    let config = Config::global();
-
-    // Check all required keys for the provider
-    for key in &metadata.config_keys {
-        if key.required {
-            let key_name = &key.name;
-
-            // First, check if the key is set in the environment
-            let is_set_in_env = env::var(key_name).is_ok();
-
-            // If not set in environment, check the config file based on whether it's a secret or not
-            let is_set_in_config = config.get(key_name, key.secret).is_ok();
-
-            // If the key is neither in the environment nor in the config, the provider is not configured
-            if !is_set_in_env && !is_set_in_config {
-                return false;
-            }
-        }
-    }
-
-    // If all required keys are accounted for, the provider is considered configured
-    true
-}
-
 pub fn routes(state: AppState) -> Router {
     Router::new()
         .route("/config", get(read_all_config))

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

@@ -6,7 +6,7 @@ pub mod extension;
 pub mod health;
 pub mod reply;
 pub mod session;
-
+pub mod utils;
 use axum::Router;
 
 // Function to configure all routes

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

@@ -1,13 +1,15 @@
-use serde::{Deserialize, Serialize};
-use std::error::Error;
 use goose::config::Config;
+use goose::providers::base::{ConfigKey, ProviderMetadata};
+use serde::{Deserialize, Serialize};
+use std::env;
+use std::error::Error;
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub enum KeyLocation {
     Environment,
     ConfigFile,
     Keychain,
-    NotFound
+    NotFound,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -20,10 +22,8 @@ pub struct KeyInfo {
 }
 
 /// Inspects a configuration key to determine if it's set, its location, and value (for non-secret keys)
-pub fn inspect_key(
-    key_name: &str,
-    is_secret: bool,
-) -> Result<KeyInfo, Box<dyn Error>> {
+#[allow(dead_code)]
+pub fn inspect_key(key_name: &str, is_secret: bool) -> Result<KeyInfo, Box<dyn Error>> {
     let config = Config::global();
 
     // Check environment variable first
@@ -44,7 +44,7 @@ pub fn inspect_key(
     let config_result = if is_secret {
         config.get_secret(key_name).map(|v| (v, true))
     } else {
-        config.get(key_name).map(|v| (v, false))
+        config.get_param(key_name).map(|v| (v, false))
     };
 
     match config_result {
@@ -64,20 +64,19 @@ pub fn inspect_key(
                 // Only include value for non-secret keys
                 value: if !is_secret_actual { Some(value) } else { None },
             })
-        },
-        Err(_) => {
-            Ok(KeyInfo {
-                name: key_name.to_string(),
-                is_set: false,
-                location: KeyLocation::NotFound,
-                is_secret,
-                value: None,
-            })
         }
+        Err(_) => Ok(KeyInfo {
+            name: key_name.to_string(),
+            is_set: false,
+            location: KeyLocation::NotFound,
+            is_secret,
+            value: None,
+        }),
     }
 }
 
 /// Inspects multiple keys at once
+#[allow(dead_code)]
 pub fn inspect_keys(
     keys: &[(String, bool)], // (name, is_secret) pairs
 ) -> Result<Vec<KeyInfo>, Box<dyn Error>> {
@@ -89,4 +88,53 @@ pub fn inspect_keys(
     }
 
     Ok(results)
-}
+}
+
+pub fn check_provider_configured(metadata: &ProviderMetadata) -> bool {
+    let config = Config::global();
+
+    // Get all required keys
+    let required_keys: Vec<&ConfigKey> = metadata
+        .config_keys
+        .iter()
+        .filter(|key| key.required)
+        .collect();
+
+    // Special case: If a provider has exactly one required key and that key
+    // has a default value, check if it's explicitly set
+    if required_keys.len() == 1 && required_keys[0].default.is_some() {
+        let key = &required_keys[0];
+
+        // Check if the key is explicitly set (either in env or config)
+        let is_set_in_env = env::var(&key.name).is_ok();
+        let is_set_in_config = config.get(&key.name, key.secret).is_ok();
+
+        return is_set_in_env || is_set_in_config;
+    }
+
+    // For providers with multiple keys or keys without defaults:
+    // Find required keys that don't have default values
+    let required_non_default_keys: Vec<&ConfigKey> = required_keys
+        .iter()
+        .filter(|key| key.default.is_none())
+        .cloned()
+        .collect();
+
+    // If there are no non-default keys, this provider needs at least one key explicitly set
+    if required_non_default_keys.is_empty() {
+        return required_keys.iter().any(|key| {
+            let is_set_in_env = env::var(&key.name).is_ok();
+            let is_set_in_config = config.get(&key.name, key.secret).is_ok();
+
+            is_set_in_env || is_set_in_config
+        });
+    }
+
+    // Otherwise, all non-default keys must be set
+    required_non_default_keys.iter().all(|key| {
+        let is_set_in_env = env::var(&key.name).is_ok();
+        let is_set_in_config = config.get(&key.name, key.secret).is_ok();
+
+        is_set_in_env || is_set_in_config
+    })
+}

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

@@ -17,9 +17,11 @@ const GridLayout = memo(function GridLayout({ children }: { children: React.Reac
 const ProviderCards = memo(function ProviderCards({
   providers,
   isOnboarding,
+  refreshProviders,
 }: {
   providers: ProviderDetails[];
   isOnboarding: boolean;
+  refreshProviders?: () => void;
 }) {
   const { openModal } = useProviderModal();
 
@@ -27,13 +29,16 @@ const ProviderCards = memo(function ProviderCards({
   const configureProviderViaModal = useCallback(
     (provider: ProviderDetails) => {
       openModal(provider, {
-        onSubmit: (values: any) => {
-          // Your logic to save the configuration
+        onSubmit: () => {
+          // Only refresh if the function is provided
+          if (refreshProviders) {
+            refreshProviders();
+          }
         },
         formProps: {},
       });
     },
-    [openModal]
+    [openModal, refreshProviders]
   );
 
   const handleLaunch = useCallback(() => {
@@ -56,48 +61,28 @@ const ProviderCards = memo(function ProviderCards({
   return <>{providerCards}</>;
 });
 
-// Fix the ProviderModalProvider
-export const OptimizedProviderModalProvider = memo(function OptimizedProviderModalProvider({
-  children,
-}: {
-  children: React.ReactNode;
-}) {
-  const contextValue = useMemo(
-    () => ({
-      isOpen: false,
-      currentProvider: null,
-      modalProps: {},
-      openModal: (provider, additionalProps = {}) => {
-        // Implementation
-      },
-      closeModal: () => {
-        // Implementation
-      },
-    }),
-    []
-  );
-
-  return <ProviderModalProvider>{children}</ProviderModalProvider>;
-});
-
 export default memo(function ProviderGrid({
   providers,
   isOnboarding,
+  refreshProviders,
 }: {
   providers: ProviderDetails[];
   isOnboarding: boolean;
+  refreshProviders?: () => void;
 }) {
-  // Remove the console.log
-  console.log('provider grid');
   // Memoize the modal provider and its children to avoid recreating on every render
   const modalProviderContent = useMemo(
     () => (
       <ProviderModalProvider>
-        <ProviderCards providers={providers} isOnboarding={isOnboarding} />
+        <ProviderCards
+          providers={providers}
+          isOnboarding={isOnboarding}
+          refreshProviders={refreshProviders}
+        />
         <ProviderConfigurationModal />
       </ProviderModalProvider>
     ),
-    [providers, isOnboarding]
+    [providers, isOnboarding, refreshProviders]
   );
 
   return <GridLayout>{modalProviderContent}</GridLayout>;

ui/desktop/src/components/settings_v2/providers/ProviderSettingsPage.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useState, useCallback, useRef } from 'react';
 import { ScrollArea } from '../../ui/scroll-area';
 import BackButton from '../../ui/BackButton';
 import ProviderGrid from './ProviderGrid';
@@ -9,37 +9,40 @@ export default function ProviderSettings({ onClose }: { onClose: () => void }) {
   const { getProviders } = useConfig();
   const [loading, setLoading] = useState(true);
   const [providers, setProviders] = useState<ProviderDetails[]>([]);
+  const initialLoadDone = useRef(false);
+
+  // Create a function to load providers that can be called multiple times
+  const loadProviders = useCallback(async () => {
+    setLoading(true);
+    try {
+      // Only force refresh when explicitly requested, not on initial load
+      const result = await getProviders(!initialLoadDone.current);
+      if (result) {
+        setProviders(result);
+        initialLoadDone.current = true;
+      }
+    } catch (error) {
+      console.error('Failed to load providers:', error);
+    } finally {
+      setLoading(false);
+    }
+  }, [getProviders]);
 
   // Load providers only once when component mounts
   useEffect(() => {
-    let isMounted = true;
-
-    const loadProviders = async () => {
-      try {
-        // Force refresh to ensure we have the latest data
-        const result = await getProviders(true);
-        // Only update state if component is still mounted
-        if (isMounted && result) {
-          setProviders(result);
-        }
-      } catch (error) {
-        console.error('Failed to load providers:', error);
-      } finally {
-        if (isMounted) {
-          setLoading(false);
-        }
-      }
-    };
-
     loadProviders();
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []); // Intentionally not including loadProviders in deps to prevent reloading
 
-    // Cleanup function to prevent state updates on unmounted component
-    return () => {
-      isMounted = false;
-    };
-  }, []); // Empty dependency array ensures this only runs once
+  // This function will be passed to ProviderGrid for manual refreshes after config changes
+  const refreshProviders = useCallback(() => {
+    if (initialLoadDone.current) {
+      getProviders(true).then((result) => {
+        if (result) setProviders(result);
+      });
+    }
+  }, [getProviders]);
 
-  console.log(providers);
   return (
     <div className="h-screen w-full">
       <div className="relative flex items-center h-[36px] w-full bg-bgSubtle"></div>
@@ -61,7 +64,11 @@ export default function ProviderSettings({ onClose }: { onClose: () => void }) {
               {loading ? (
                 <div>Loading providers...</div>
               ) : (
-                <ProviderGrid providers={providers} isOnboarding={false} />
+                <ProviderGrid
+                  providers={providers}
+                  isOnboarding={false}
+                  refreshProviders={refreshProviders}
+                />
               )}
             </div>
           </div>

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

@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useState } from 'react';
 import Modal from '../../../../components/Modal';
 import ProviderSetupHeader from './subcomponents/ProviderSetupHeader';
 import DefaultProviderSetupForm from './subcomponents/forms/DefaultProviderSetupForm';
@@ -20,7 +20,7 @@ const customFormsMap = {
 };
 
 export default function ProviderConfigurationModal() {
-  const { upsert } = useConfig();
+  const { upsert, getProviders } = useConfig();
   const { isOpen, currentProvider, modalProps, closeModal } = useProviderModal();
   const [configValues, setConfigValues] = useState({});
 
@@ -32,15 +32,25 @@ export default function ProviderConfigurationModal() {
   const SubmitHandler = customSubmitHandlerMap[currentProvider.name] || DefaultSubmitHandler;
   const FormComponent = customFormsMap[currentProvider.name] || DefaultProviderSetupForm;
 
-  const handleSubmitForm = (e) => {
+  const handleSubmitForm = async (e) => {
     e.preventDefault();
     console.log('Form submitted for:', currentProvider.name);
 
-    SubmitHandler(upsert, currentProvider, configValues);
+    try {
+      // Wait for the submission to complete
+      await SubmitHandler(upsert, currentProvider, configValues);
 
-    // Close the modal unless the custom handler explicitly returns false
-    // This gives custom handlers the ability to keep the modal open if needed
-    closeModal();
+      // 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);
+      }
+    } catch (error) {
+      console.error('Failed to save configuration:', error);
+      // Keep modal open if there's an error
+    }
   };
 
   const handleCancel = () => {

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

@@ -1,60 +1,56 @@
-import React, { createContext, useContext, useState, useMemo, useCallback } from 'react';
-import { ProviderDetails } from '../../../../api';
+import React, { createContext, useContext, useState } from 'react';
+import { ProviderDetails } from '../../../../api/types.gen';
+
+interface ModalProps {
+  onSubmit?: (values: any) => void;
+  onCancel?: () => void;
+  formProps?: any;
+}
 
 interface ProviderModalContextType {
   isOpen: boolean;
   currentProvider: ProviderDetails | null;
-  modalProps: any;
-  openModal: (provider: ProviderDetails, additionalProps: any) => void;
+  modalProps: ModalProps;
+  openModal: (provider: ProviderDetails, additionalProps?: ModalProps) => void;
   closeModal: () => void;
 }
 
-const defaultContext: ProviderModalContextType = {
-  isOpen: false,
-  currentProvider: null,
-  modalProps: {},
-  openModal: () => {},
-  closeModal: () => {},
-};
-
-const ProviderModalContext = createContext<ProviderModalContextType>(defaultContext);
-
-export const useProviderModal = () => useContext<ProviderModalContextType>(ProviderModalContext);
+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({});
+  const [modalProps, setModalProps] = useState<ModalProps>({});
 
-  // Use useCallback to prevent function recreation on each render
-  const openModal = useCallback((provider: ProviderDetails, additionalProps = {}) => {
+  const openModal = (provider: ProviderDetails, additionalProps: ModalProps = {}) => {
     setCurrentProvider(provider);
     setModalProps(additionalProps);
     setIsOpen(true);
-  }, []);
+  };
 
-  const closeModal = useCallback(() => {
+  const closeModal = () => {
     setIsOpen(false);
-    // Use a small timeout to prevent UI flicker
-    setTimeout(() => {
-      setCurrentProvider(null);
-      setModalProps({});
-    }, 200);
-  }, []);
-
-  // Memoize the context value to prevent unnecessary re-renders
-  const contextValue = useMemo(
-    () => ({
-      isOpen,
-      currentProvider,
-      modalProps,
-      openModal,
-      closeModal,
-    }),
-    [isOpen, currentProvider, modalProps, openModal, closeModal]
-  );
+  };
 
   return (
-    <ProviderModalContext.Provider value={contextValue}>{children}</ProviderModalContext.Provider>
+    <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_v2/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx

@@ -25,12 +25,11 @@ export default function DefaultProviderSetupForm({
 
       // Try to load actual values from config for each parameter that is not secret
       for (const parameter of parameters) {
-        if (parameter.required && !parameter.secret) {
+        if (parameter.required) {
           try {
             // Check if there's a stored value in the config system
             const configKey = `${parameter.name}`;
             const configResponse = await read(configKey, parameter.secret || false);
-            console.log('configResponse', configResponse);
 
             if (configResponse) {
               // Use the value from the config provider

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

@@ -101,8 +101,7 @@ export const DefaultSubmitHandler = async (upsertFn, provider, configValues) =>
     // Create the provider-specific config key
     const configKey = `${parameter.name}`;
 
-    // Explicitly define is_secret as a boolean (true/false) or null
-    // This is critical for Rust's Option<bool> type
+    // Explicitly define is_secret as a boolean (true/false)
     const isSecret = parameter.secret === true;
 
     // Pass the is_secret flag from the parameter definition