diff --git a/crates/goose-server/src/routes/utils.rs b/crates/goose-server/src/routes/utils.rs index a6659e5e4f46..0c9f47e978a5 100644 --- a/crates/goose-server/src/routes/utils.rs +++ b/crates/goose-server/src/routes/utils.rs @@ -116,6 +116,11 @@ pub fn check_provider_configured(metadata: &ProviderMetadata) -> bool { .filter(|key| key.required) .collect(); + // If there are no required keys at all, the provider is considered configured + if required_keys.is_empty() { + return true; + } + // 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() { @@ -154,3 +159,108 @@ pub fn check_provider_configured(metadata: &ProviderMetadata) -> bool { is_set_in_env || is_set_in_config }) } + +#[cfg(test)] +mod tests { + use super::*; + use goose::providers::base::{ConfigKey, Provider, ProviderMetadata}; + + #[test] + fn test_check_provider_configured_no_required_keys() { + // Test provider with no required keys (like gemini-cli) + let metadata = ProviderMetadata::new( + "test-provider", + "Test Provider", + "A test provider with no required keys", + "default", + vec!["default"], + "https://example.com", + vec![], // No config keys at all + ); + + assert!(check_provider_configured(&metadata)); + } + + #[test] + fn test_check_provider_configured_optional_key_with_default() { + // Test provider with one optional key that has a default (like claude-code) + let metadata = ProviderMetadata::new( + "test-provider", + "Test Provider", + "A test provider with optional key with default", + "default", + vec!["default"], + "https://example.com", + vec![ConfigKey::new( + "TEST_COMMAND", + false, + false, + Some("default_cmd"), + )], + ); + + // Should be considered configured because it has no required keys + assert!(check_provider_configured(&metadata)); + } + + #[test] + fn test_check_provider_configured_required_key_with_default_not_set() { + // Test provider with one required key that has a default but is not set + let metadata = ProviderMetadata::new( + "test-provider", + "Test Provider", + "A test provider with required key with default", + "default", + vec!["default"], + "https://example.com", + vec![ConfigKey::new( + "TEST_COMMAND", + true, + false, + Some("default_cmd"), + )], // required=true, secret=false + ); + + // Should NOT be considered configured because the required key is not explicitly set + // even though it has a default value + assert!(!check_provider_configured(&metadata)); + } + + #[test] + fn test_check_provider_configured_required_key_without_default() { + // Test provider with required key without default + let metadata = ProviderMetadata::new( + "test-provider", + "Test Provider", + "A test provider with required key without default", + "default", + vec!["default"], + "https://example.com", + vec![ConfigKey::new("TEST_API_KEY", true, true, None)], // required=true, secret=true + ); + + // Should NOT be considered configured + assert!(!check_provider_configured(&metadata)); + } + + #[test] + fn test_check_provider_configured_real_gemini_cli() { + // Test the actual gemini-cli provider metadata + use goose::providers::gemini_cli::GeminiCliProvider; + let metadata = GeminiCliProvider::metadata(); + + // Should be considered configured because it has no required keys + assert!(check_provider_configured(&metadata)); + } + + #[test] + fn test_check_provider_configured_real_claude_code() { + // Test the actual claude-code provider metadata + use goose::providers::claude_code::ClaudeCodeProvider; + let metadata = ClaudeCodeProvider::metadata(); + + // Should be considered configured because it has no required keys + // (CLAUDE_CODE_COMMAND is optional with a default) + assert!(check_provider_configured(&metadata)); + } +} diff --git a/ui/desktop/src/components/settings/providers/ProviderGrid.tsx b/ui/desktop/src/components/settings/providers/ProviderGrid.tsx index 504f4d70e815..1adbcbeac0f9 100644 --- a/ui/desktop/src/components/settings/providers/ProviderGrid.tsx +++ b/ui/desktop/src/components/settings/providers/ProviderGrid.tsx @@ -36,10 +36,13 @@ const ProviderCards = memo(function ProviderCards({ refreshProviders(); } }, - formProps: {}, + formProps: { + isOnboarding, + onProviderLaunch, + }, }); }, - [openModal, refreshProviders] + [openModal, refreshProviders, isOnboarding, onProviderLaunch] ); const deleteProviderConfigViaModal = useCallback( diff --git a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx index 36e4245cf0ad..ffccc058c849 100644 --- a/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx +++ b/ui/desktop/src/components/settings/providers/modal/ProviderConfiguationModal.tsx @@ -98,6 +98,17 @@ export default function ProviderConfigurationModal() { // Wait for the submission to complete await SubmitHandler(upsert, currentProvider, configValues); + // Check if this provider has no required configuration and we're in onboarding mode + const hasNoRequiredConfig = parameters.filter(p => p.required).length === 0; + const isOnboardingMode = modalProps.formProps?.isOnboarding; + const onProviderLaunch = modalProps.formProps?.onProviderLaunch; + + // If it's onboarding mode, has no required config, and we have a launch function, launch the provider + if (isOnboardingMode && hasNoRequiredConfig && onProviderLaunch) { + console.log('Launching provider after configuration:', currentProvider.name); + await onProviderLaunch(currentProvider); + } + // Close the modal before triggering refreshes to avoid UI issues closeModal(); diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx index f66d14d412b7..a5dbe0481252 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx @@ -12,6 +12,8 @@ interface DefaultProviderSetupFormProps { setConfigValues: React.Dispatch>>; provider: ProviderDetails; validationErrors: ValidationErrors; + isOnboarding?: boolean; + onProviderLaunch?: (provider: ProviderDetails) => void; } export default function DefaultProviderSetupForm({ @@ -19,6 +21,8 @@ export default function DefaultProviderSetupForm({ setConfigValues, provider, validationErrors = {}, + isOnboarding, + onProviderLaunch, }: DefaultProviderSetupFormProps) { const parameters = useMemo( () => provider.metadata.config_keys || [], diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx index 933b1e23c934..1aa2fb3c5e88 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/handlers/DefaultSubmitHandler.tsx @@ -47,6 +47,18 @@ export const DefaultSubmitHandler = async ( } ); + // For providers with no required configuration, save optional parameters with defaults + // This ensures something gets saved to mark the provider as configured + if (parameters.length > 0 && parameters.every(p => !p.required)) { + parameters.forEach((parameter) => { + if (parameter.default !== undefined && parameter.default !== null) { + const configKey = `${parameter.name}`; + const isSecret = parameter.secret === true; + upsertPromises.push(upsertFn(configKey, parameter.default, isSecret)); + } + }); + } + // Wait for all upsert operations to complete return Promise.all(upsertPromises); };