diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index 7ba89e83..0bc6b873 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -853,8 +853,7 @@ pub async fn configure_provider_dialog() -> anyhow::Result { match test_provider_configuration(provider_name, &model, toolshim_enabled, toolshim_model).await { Ok(()) => { - config.set_goose_provider(provider_name)?; - config.set_goose_model(&model)?; + goose::config::set_active_provider(config, provider_name, &model)?; print_config_file_saved()?; Ok(true) } diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index e5203fe4..82202f61 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -69,6 +69,8 @@ pub struct ProviderDetails { pub metadata: ProviderMetadata, pub is_configured: bool, pub provider_type: ProviderType, + #[serde(skip_serializing_if = "Option::is_none")] + pub saved_model: Option, } #[derive(Serialize, ToSchema)] @@ -171,6 +173,29 @@ pub async fn upsert_config( Json(query): Json, ) -> Result, ErrorResponse> { let config = Config::global(); + + // Intercept legacy keys to write structured provider config + if query.key == "GOOSE_PROVIDER" { + if let Some(name) = query.value.as_str() { + // Preserve the target provider's saved model rather than copying + // the current active provider's model into the new entry. + let model = goose::config::get_provider_entry(config, name) + .map(|e| e.model) + .or_else(|| config.get_goose_model().ok()) + .unwrap_or_default(); + goose::config::set_active_provider(config, name, &model)?; + return Ok(Json(Value::String(format!("Upserted key {}", query.key)))); + } + } + if query.key == "GOOSE_MODEL" { + if let Some(model) = query.value.as_str() { + if let Ok(provider) = config.get_goose_provider() { + goose::config::set_active_provider(config, &provider, model)?; + return Ok(Json(Value::String(format!("Upserted key {}", query.key)))); + } + } + } + config.set(&query.key, &query.value, query.is_secret)?; Ok(Json(Value::String(format!("Upserted key {}", query.key)))) } @@ -192,6 +217,14 @@ pub async fn remove_config( if query.is_secret { config.delete_secret(&query.key)?; + } else if query.key == "GOOSE_PROVIDER" || query.key == "active_provider" { + config.delete("active_provider")?; + config.delete("GOOSE_PROVIDER")?; + } else if query.key == "GOOSE_MODEL" { + if let Ok(provider) = config.get_goose_provider() { + goose::config::set_active_provider(config, &provider, "")?; + } + config.delete("GOOSE_MODEL")?; } else { config.delete(&query.key)?; } @@ -236,6 +269,20 @@ pub async fn read_config( ) -> Result, ErrorResponse> { let config = Config::global(); + // Intercept legacy keys to return structured provider config + if query.key == "GOOSE_PROVIDER" || query.key == "active_provider" { + if let Ok(val) = config.get_goose_provider() { + return Ok(Json(ConfigValueResponse::Value(Value::String(val)))); + } + return Ok(Json(ConfigValueResponse::Value(Value::Null))); + } + if query.key == "GOOSE_MODEL" { + if let Ok(val) = config.get_goose_model() { + return Ok(Json(ConfigValueResponse::Value(Value::String(val)))); + } + return Ok(Json(ConfigValueResponse::Value(Value::Null))); + } + let response_value = match config.get(&query.key, query.is_secret) { Ok(value) => { if query.is_secret { @@ -341,17 +388,22 @@ pub async fn read_all_config() -> Result, ErrorResponse> { ) )] pub async fn providers() -> Result>, ErrorResponse> { + let config = Config::global(); let providers = get_providers().await; let providers_response: Vec = providers .into_iter() .map(|(metadata, provider_type)| { let is_configured = check_provider_configured(&metadata, provider_type); + let saved_model = goose::config::get_provider_entry(config, &metadata.name) + .map(|e| e.model) + .filter(|m| !m.is_empty()); ProviderDetails { name: metadata.name.clone(), metadata, is_configured, provider_type, + saved_model, } }) .collect(); @@ -729,9 +781,7 @@ pub async fn set_config_provider( .await .and_then(|_| { let config = Config::global(); - config - .set_goose_provider(provider.clone()) - .and_then(|_| config.set_goose_model(model.clone())) + goose::config::set_active_provider(config, &provider, &model) .map_err(|e| anyhow::anyhow!(e)) }) .map_err(|err| { @@ -839,9 +889,28 @@ pub async fn configure_provider_oauth( })?; // Mark the provider as configured after successful OAuth - let configured_marker = format!("{}_configured", provider_name); let config = goose::config::Config::global(); - config.set_param(&configured_marker, true)?; + if let Some(mut entry) = goose::config::get_provider_entry(config, &provider_name) { + entry.configured = true; + goose::config::set_provider_entry(config, &provider_name, &entry)?; + } else { + let model = if goose::config::get_active_provider(config).as_deref() + == Some(provider_name.as_str()) + { + config.get_goose_model().unwrap_or_default() + } else { + String::new() + }; + goose::config::set_provider_entry( + config, + &provider_name, + &goose::config::ProviderEntry { + enabled: true, + model, + configured: true, + }, + )?; + } Ok(Json("OAuth configuration completed".to_string())) } diff --git a/crates/goose-server/src/routes/utils.rs b/crates/goose-server/src/routes/utils.rs index 1e97a027..2c4a7d2a 100644 --- a/crates/goose-server/src/routes/utils.rs +++ b/crates/goose-server/src/routes/utils.rs @@ -118,20 +118,37 @@ pub fn check_provider_configured(metadata: &ProviderMetadata, provider_type: Pro } } - // Special case: OAuth providers - check for configured marker + // OAuth providers: trust the structured configured flag or legacy marker let has_oauth_key = metadata.config_keys.iter().any(|key| key.oauth_flow); if has_oauth_key { + if let Some(entry) = goose::config::get_provider_entry(config, &metadata.name) { + if entry.configured { + return true; + } + } let configured_marker = format!("{}_configured", metadata.name); if matches!(config.get_param::(&configured_marker), Ok(true)) { return true; } } - // Special case: Zero-config providers (no config keys) + // Zero-config providers (no config keys): trust structured flag or active status if metadata.config_keys.is_empty() { - // Check if the provider has been explicitly configured via the UI + if let Some(entry) = goose::config::get_provider_entry(config, &metadata.name) { + if entry.configured { + return true; + } + } let configured_marker = format!("{}_configured", metadata.name); - return config.get_param::(&configured_marker).is_ok(); + if config.get_param::(&configured_marker).is_ok() { + return true; + } + if let Ok(current) = config.get_goose_provider() { + if current == metadata.name { + return true; + } + } + return false; } // Get all required keys diff --git a/crates/goose/src/acp/server/config.rs b/crates/goose/src/acp/server/config.rs index 15cdffc7..41a9b96d 100644 --- a/crates/goose/src/acp/server/config.rs +++ b/crates/goose/src/acp/server/config.rs @@ -63,8 +63,8 @@ impl GooseAcpAgent { ) -> Result { let config = self.config()?; Ok(DefaultsReadResponse { - provider_id: optional_config_string(&config, "GOOSE_PROVIDER")?, - model_id: optional_config_string(&config, "GOOSE_MODEL")?, + provider_id: config.get_goose_provider().ok(), + model_id: config.get_goose_model().ok(), }) } @@ -113,21 +113,13 @@ impl GooseAcpAgent { } let config = self.config()?; - config - .set_param_values(&[( - "GOOSE_PROVIDER".to_string(), - serde_json::Value::String(provider_id.clone()), - )]) + let model = model_id.clone().unwrap_or_else(|| { + crate::config::get_provider_entry(&config, &provider_id) + .map(|e| e.model) + .unwrap_or_default() + }); + crate::config::set_active_provider(&config, &provider_id, &model) .internal_err_ctx("Failed to save default provider")?; - if let Some(model_id) = model_id.as_deref() { - config - .set_param("GOOSE_MODEL", model_id) - .internal_err_ctx("Failed to save default model")?; - } else { - config - .delete("GOOSE_MODEL") - .internal_err_ctx("Failed to clear default model")?; - } Ok(DefaultsReadResponse { provider_id: Some(provider_id), @@ -245,14 +237,3 @@ fn is_supported_voice_dictation_provider(value: &str) -> bool { } } } - -fn optional_config_string( - config: &Config, - key: &str, -) -> Result, agent_client_protocol::Error> { - match config.get_param::(key) { - Ok(value) => Ok(Some(value)), - Err(crate::config::ConfigError::NotFound(_)) => Ok(None), - Err(e) => Err(agent_client_protocol::Error::internal_error().data(e.to_string())), - } -} diff --git a/crates/goose/src/acp/server/onboarding.rs b/crates/goose/src/acp/server/onboarding.rs index 8e175f7d..d0ff12a1 100644 --- a/crates/goose/src/acp/server/onboarding.rs +++ b/crates/goose/src/acp/server/onboarding.rs @@ -324,20 +324,15 @@ fn apply_goose_config_candidate( let provider = yaml_string(&source, "GOOSE_PROVIDER"); let model = yaml_string(&source, "GOOSE_MODEL"); - if provider.is_some() || model.is_some() { - let mut updates = Vec::new(); - if let Some(provider) = provider.clone() { - updates.push(( - "GOOSE_PROVIDER".to_string(), - serde_json::Value::String(provider), - )); - } - if let Some(model) = model.clone() { - updates.push(("GOOSE_MODEL".to_string(), serde_json::Value::String(model))); - } - target_config.set_param_values(&updates)?; + if let Some(ref p) = provider { + let m = model.clone().unwrap_or_else(|| { + crate::config::get_provider_entry(target_config, p) + .map(|e| e.model) + .unwrap_or_default() + }); + crate::config::set_active_provider(target_config, p, &m)?; result.provider_defaults = DefaultsReadResponse { - provider_id: provider, + provider_id: provider.clone(), model_id: model, }; result.imported.providers = 1; @@ -669,13 +664,30 @@ extensions: assert_eq!(result.imported.providers, 1); assert_eq!(result.imported.extensions, 1); assert_eq!(result.imported.skills, 1); - assert_eq!( - target_config.get_param::("GOOSE_PROVIDER").unwrap(), - "openai" - ); + assert_eq!(target_config.get_goose_provider().unwrap(), "openai"); assert!(target.path().join("skills").join("reviewer").exists()); } + #[test] + fn apply_goose_config_model_only_skips_provider_activation() { + let source = TempDir::new().unwrap(); + let target = TempDir::new().unwrap(); + let source_config = source.path().join(CONFIG_YAML_NAME); + fs::write(&source_config, "GOOSE_MODEL: gpt-5.1\n").unwrap(); + + let target_config = Config::new_with_file_secrets( + target.path().join(CONFIG_YAML_NAME), + target.path().join("secrets.yaml"), + ) + .unwrap(); + + let result = + apply_goose_config_candidate(&target_config, target.path(), &source_config).unwrap(); + + assert_eq!(result.imported.providers, 0); + assert!(target_config.get_goose_provider().is_err()); + } + #[cfg(unix)] #[test] fn import_skill_dirs_skips_symlink_cycles() { diff --git a/crates/goose/src/acp/server/providers.rs b/crates/goose/src/acp/server/providers.rs index 964b16d4..6a1870be 100644 --- a/crates/goose/src/acp/server/providers.rs +++ b/crates/goose/src/acp/server/providers.rs @@ -613,12 +613,7 @@ impl GooseAcpAgent { .data(format!("Provider is not editable: {}", req.provider_id))); } - if Config::global() - .get_param::("GOOSE_PROVIDER") - .ok() - .as_deref() - == Some(req.provider_id.as_str()) - { + if Config::global().get_goose_provider().ok().as_deref() == Some(req.provider_id.as_str()) { return Err(agent_client_protocol::Error::invalid_params().data(format!( "Cannot delete active provider: {}", req.provider_id diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 20c5dda0..79fcfebd 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -281,9 +281,11 @@ fn keyring_disabled_value(value: &serde_yaml::Value) -> bool { } const EXTENSIONS_KEY: &str = "extensions"; +const PROVIDERS_KEY: &str = "providers"; pub fn merge_config_values(base: &mut Mapping, overlay: Mapping) { let extensions_key = serde_yaml::Value::String(EXTENSIONS_KEY.to_string()); + let providers_key = serde_yaml::Value::String(PROVIDERS_KEY.to_string()); for (key, overlay_value) in overlay { if key == extensions_key { @@ -293,7 +295,18 @@ pub fn merge_config_values(base: &mut Mapping, overlay: Mapping) { if let (Some(base_map), Some(overlay_map)) = (base_ext.as_mapping_mut(), overlay_value.as_mapping()) { - merge_extensions(base_map, overlay_map); + merge_nested_entries(base_map, overlay_map); + } else { + base.insert(key, overlay_value); + } + } else if key == providers_key { + let base_prov = base + .entry(key.clone()) + .or_insert_with(|| serde_yaml::Value::Mapping(Mapping::new())); + if let (Some(base_map), Some(overlay_map)) = + (base_prov.as_mapping_mut(), overlay_value.as_mapping()) + { + merge_nested_entries(base_map, overlay_map); } else { base.insert(key, overlay_value); } @@ -303,7 +316,7 @@ pub fn merge_config_values(base: &mut Mapping, overlay: Mapping) { } } -fn merge_extensions(base: &mut Mapping, overlay: &Mapping) { +fn merge_nested_entries(base: &mut Mapping, overlay: &Mapping) { for (ext_key, overlay_ext) in overlay { match base.get_mut(ext_key) { Some(base_ext) => { @@ -471,20 +484,27 @@ impl Config { } } - crate::config::migrations::run_migrations(&mut merged); + crate::config::migrations::run_read_migrations(&mut merged); Ok(merged) } pub fn all_values(&self) -> Result, ConfigError> { let config_values = self.load()?; - Ok(HashMap::from_iter(config_values.into_iter().filter_map( - |(k, v)| { - k.as_str() - .map(|k| k.to_string()) - .zip(serde_json::to_value(v).ok()) - }, - ))) + let mut map = HashMap::from_iter(config_values.into_iter().filter_map(|(k, v)| { + k.as_str() + .map(|k| k.to_string()) + .zip(serde_json::to_value(v).ok()) + })); + + if let Ok(provider) = self.get_goose_provider() { + map.insert("GOOSE_PROVIDER".to_string(), Value::String(provider)); + } + if let Ok(model) = self.get_goose_model() { + map.insert("GOOSE_MODEL".to_string(), Value::String(model)); + } + + Ok(map) } fn config_write_target_path(&self) -> Result { @@ -703,6 +723,24 @@ impl Config { } } + /// Read-modify-write a configuration value atomically through the write path. + pub fn update_param(&self, key: &str, f: F) -> Result<(), ConfigError> + where + T: for<'de> Deserialize<'de> + Default, + V: Serialize, + F: FnOnce(T) -> V, + { + let _guard = self.guard.lock().unwrap(); + let mut values = self.load_write_config()?; + let current: T = values + .get(key) + .and_then(|v| serde_yaml::from_value(v.clone()).ok()) + .unwrap_or_default(); + let updated = f(current); + values.insert(serde_yaml::to_value(key)?, serde_yaml::to_value(updated)?); + self.save_values(&values) + } + /// Set a configuration value in the config file (non-secret). /// /// This will immediately write the value to the config file. The value @@ -1031,8 +1069,33 @@ config_value!(CHATGPT_CODEX_REASONING_EFFORT, String, "medium"); config_value!(GOOSE_SEARCH_PATHS, Vec); config_value!(GOOSE_MODE, GooseMode); -config_value!(GOOSE_PROVIDER, String); -config_value!(GOOSE_MODEL, String); +// GOOSE_PROVIDER and GOOSE_MODEL are handled by crate::config::providers +// which checks the structured `providers:` block first and falls back to +// the legacy flat keys. The accessors below delegate to that module. +impl Config { + pub fn get_goose_provider(&self) -> Result { + crate::config::providers::get_active_provider(self) + .ok_or_else(|| ConfigError::NotFound("GOOSE_PROVIDER".to_string())) + } + pub fn set_goose_provider(&self, v: impl Into) -> Result<(), ConfigError> { + let name = v.into(); + let model = crate::config::providers::get_provider_entry(self, &name) + .map(|e| e.model) + .unwrap_or_default(); + crate::config::providers::set_active_provider(self, &name, &model) + } + pub fn get_goose_model(&self) -> Result { + crate::config::providers::get_active_model(self) + .ok_or_else(|| ConfigError::NotFound("GOOSE_MODEL".to_string())) + } + pub fn set_goose_model(&self, v: impl Into) -> Result<(), ConfigError> { + let model = v.into(); + if let Some(provider) = crate::config::providers::get_active_provider(self) { + crate::config::providers::set_active_provider(self, &provider, &model)?; + } + Ok(()) + } +} config_value!(GOOSE_PROMPT_EDITOR, Option); config_value!(GOOSE_PROMPT_EDITOR_ALWAYS, Option); config_value!(GOOSE_MAX_ACTIVE_AGENTS, usize); @@ -2145,4 +2208,123 @@ extensions: Ok(()) } + + #[test] + fn test_merge_providers_append_new() { + let mut base = Mapping::new(); + let mut base_prov = Mapping::new(); + let mut prov_a = Mapping::new(); + prov_a.insert( + serde_yaml::Value::String("enabled".into()), + serde_yaml::Value::Bool(true), + ); + prov_a.insert( + serde_yaml::Value::String("model".into()), + serde_yaml::Value::String("gpt-4o".into()), + ); + prov_a.insert( + serde_yaml::Value::String("configured".into()), + serde_yaml::Value::Bool(true), + ); + base_prov.insert( + serde_yaml::Value::String("openai".into()), + serde_yaml::Value::Mapping(prov_a), + ); + base.insert( + serde_yaml::Value::String("providers".into()), + serde_yaml::Value::Mapping(base_prov), + ); + + let mut overlay = Mapping::new(); + let mut overlay_prov = Mapping::new(); + let mut prov_b = Mapping::new(); + prov_b.insert( + serde_yaml::Value::String("enabled".into()), + serde_yaml::Value::Bool(true), + ); + prov_b.insert( + serde_yaml::Value::String("model".into()), + serde_yaml::Value::String("claude-3-opus".into()), + ); + prov_b.insert( + serde_yaml::Value::String("configured".into()), + serde_yaml::Value::Bool(true), + ); + overlay_prov.insert( + serde_yaml::Value::String("anthropic".into()), + serde_yaml::Value::Mapping(prov_b), + ); + overlay.insert( + serde_yaml::Value::String("providers".into()), + serde_yaml::Value::Mapping(overlay_prov), + ); + + merge_config_values(&mut base, overlay); + + let providers = base.get("providers").unwrap().as_mapping().unwrap(); + assert!(providers.contains_key("openai")); + assert!(providers.contains_key("anthropic")); + // openai should be unchanged + let a = providers.get("openai").unwrap().as_mapping().unwrap(); + assert!(a.get("enabled").unwrap().as_bool().unwrap()); + assert_eq!(a.get("model").unwrap().as_str().unwrap(), "gpt-4o"); + } + + #[test] + fn test_merge_providers_partial_override() { + let mut base = Mapping::new(); + let mut base_prov = Mapping::new(); + let mut prov = Mapping::new(); + prov.insert( + serde_yaml::Value::String("enabled".into()), + serde_yaml::Value::Bool(true), + ); + prov.insert( + serde_yaml::Value::String("model".into()), + serde_yaml::Value::String("gpt-4o".into()), + ); + prov.insert( + serde_yaml::Value::String("configured".into()), + serde_yaml::Value::Bool(true), + ); + base_prov.insert( + serde_yaml::Value::String("openai".into()), + serde_yaml::Value::Mapping(prov), + ); + base.insert( + serde_yaml::Value::String("providers".into()), + serde_yaml::Value::Mapping(base_prov), + ); + + // Overlay just changes the model + let mut overlay = Mapping::new(); + let mut overlay_prov = Mapping::new(); + let mut prov_override = Mapping::new(); + prov_override.insert( + serde_yaml::Value::String("model".into()), + serde_yaml::Value::String("gpt-4o-mini".into()), + ); + overlay_prov.insert( + serde_yaml::Value::String("openai".into()), + serde_yaml::Value::Mapping(prov_override), + ); + overlay.insert( + serde_yaml::Value::String("providers".into()), + serde_yaml::Value::Mapping(overlay_prov), + ); + + merge_config_values(&mut base, overlay); + + let providers = base.get("providers").unwrap().as_mapping().unwrap(); + let openai = providers.get("openai").unwrap().as_mapping().unwrap(); + + // model should be overridden + assert_eq!( + openai.get("model").unwrap().as_str().unwrap(), + "gpt-4o-mini" + ); + // Other fields should be preserved + assert!(openai.get("enabled").unwrap().as_bool().unwrap()); + assert!(openai.get("configured").unwrap().as_bool().unwrap()); + } } diff --git a/crates/goose/src/config/migrations.rs b/crates/goose/src/config/migrations.rs index d6f20e91..33be9406 100644 --- a/crates/goose/src/config/migrations.rs +++ b/crates/goose/src/config/migrations.rs @@ -1,16 +1,27 @@ use crate::agents::extension::PLATFORM_EXTENSIONS; use crate::agents::ExtensionConfig; use crate::config::extensions::ExtensionEntry; +use crate::config::providers::ProviderEntry; use serde_yaml::Mapping; const EXTENSIONS_CONFIG_KEY: &str = "extensions"; +const PROVIDERS_CONFIG_KEY: &str = "providers"; +const ACTIVE_PROVIDER_KEY: &str = "active_provider"; pub fn run_migrations(config: &mut Mapping) -> bool { let mut changed = false; changed |= migrate_platform_extensions(config); + changed |= migrate_provider_config(config); changed } +/// Run only non-destructive migrations suitable for in-memory read paths. +/// Provider migration is excluded because it removes flat keys that +/// `get_param()` callers may still look up directly. +pub fn run_read_migrations(config: &mut Mapping) { + migrate_platform_extensions(config); +} + fn migrate_platform_extensions(config: &mut Mapping) -> bool { let extensions_key = serde_yaml::Value::String(EXTENSIONS_CONFIG_KEY.to_string()); @@ -102,6 +113,153 @@ fn migrate_platform_extensions(config: &mut Mapping) -> bool { needs_save } +/// Remove leftover legacy flat keys when `providers:` block already exists. +fn cleanup_legacy_provider_keys(config: &mut Mapping) -> bool { + let configured_suffix = "_configured"; + let mut changed = false; + + let stale_keys: Vec = config + .keys() + .filter(|k| { + k.as_str() + .map(|s| { + s == "GOOSE_PROVIDER" || s == "GOOSE_MODEL" || s.ends_with(configured_suffix) + }) + .unwrap_or(false) + }) + .cloned() + .collect(); + + for key in stale_keys { + config.shift_remove(&key); + changed = true; + } + + changed +} + +/// Migrate flat provider keys to the structured `providers:` block. +/// +/// Old layout (flat keys): +/// ```yaml +/// GOOSE_PROVIDER: claude-acp +/// GOOSE_MODEL: current +/// claude-acp_configured: true +/// lmstudio_configured: true +/// ``` +/// +/// New layout: +/// ```yaml +/// active_provider: claude-acp +/// providers: +/// claude-acp: +/// enabled: true +/// model: current +/// configured: true +/// lmstudio: +/// enabled: true +/// model: "" +/// configured: true +/// ``` +/// +fn migrate_provider_config(config: &mut Mapping) -> bool { + let providers_key = serde_yaml::Value::String(PROVIDERS_CONFIG_KEY.to_string()); + + // If providers block already exists, backfill active_provider from the + // legacy flat key when missing, then clean up leftover flat keys. + if config.contains_key(&providers_key) { + let ap_key = serde_yaml::Value::String(ACTIVE_PROVIDER_KEY.to_string()); + if !config.contains_key(&ap_key) { + if let Some(legacy) = config + .get(serde_yaml::Value::String("GOOSE_PROVIDER".to_string())) + .and_then(|v| v.as_str()) + { + config.insert(ap_key, serde_yaml::Value::String(legacy.to_string())); + } + } + return cleanup_legacy_provider_keys(config); + } + + // Read the old flat keys, if present. + let active_provider = config + .get(serde_yaml::Value::String("GOOSE_PROVIDER".to_string())) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()); + + let active_model = config + .get(serde_yaml::Value::String("GOOSE_MODEL".to_string())) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .unwrap_or_default(); + + // Scan for `*_configured` keys to discover all previously-used providers. + let configured_suffix = "_configured"; + let mut discovered_providers: Vec = config + .keys() + .filter_map(|k| { + k.as_str().and_then(|s| { + if s.ends_with(configured_suffix) { + Some(s.trim_end_matches(configured_suffix).to_string()) + } else { + None + } + }) + }) + .collect(); + + // Ensure the active provider is in the list even if no `*_configured` + // marker exists for it yet. + if let Some(ref ap) = active_provider { + if !discovered_providers.contains(ap) { + discovered_providers.push(ap.clone()); + } + } + + // If there is nothing to migrate, bail out. + if discovered_providers.is_empty() && active_provider.is_none() { + return false; + } + + // Build the providers mapping. + let mut providers_map = Mapping::new(); + for name in &discovered_providers { + let is_active = active_provider.as_deref() == Some(name.as_str()); + let model = if is_active { + active_model.clone() + } else { + String::new() + }; + let entry = ProviderEntry { + enabled: true, + model, + configured: true, + }; + if let Ok(value) = serde_yaml::to_value(&entry) { + providers_map.insert(serde_yaml::Value::String(name.clone()), value); + } + } + + config.insert(providers_key, serde_yaml::Value::Mapping(providers_map)); + + // Write `active_provider` top-level key. + if let Some(ref ap) = active_provider { + config.insert( + serde_yaml::Value::String(ACTIVE_PROVIDER_KEY.to_string()), + serde_yaml::Value::String(ap.clone()), + ); + } + + // Remove old flat keys. + config.shift_remove(serde_yaml::Value::String("GOOSE_PROVIDER".to_string())); + config.shift_remove(serde_yaml::Value::String("GOOSE_MODEL".to_string())); + for name in &discovered_providers { + let marker_key = serde_yaml::Value::String(format!("{}{}", name, configured_suffix)); + config.shift_remove(&marker_key); + } + + true +} + #[cfg(test)] mod tests { use super::*; @@ -159,4 +317,223 @@ mod tests { let changed = run_migrations(&mut config); assert!(!changed); } + + // ----------------------------------------------------------------------- + // Provider migration tests + // ----------------------------------------------------------------------- + + #[test] + fn test_migrate_provider_config_basic() { + let mut config = Mapping::new(); + config.insert( + serde_yaml::Value::String("GOOSE_PROVIDER".to_string()), + serde_yaml::Value::String("claude-acp".to_string()), + ); + config.insert( + serde_yaml::Value::String("GOOSE_MODEL".to_string()), + serde_yaml::Value::String("current".to_string()), + ); + config.insert( + serde_yaml::Value::String("claude-acp_configured".to_string()), + serde_yaml::Value::Bool(true), + ); + + let changed = migrate_provider_config(&mut config); + assert!(changed); + + // active_provider should be set + let active = config + .get(serde_yaml::Value::String("active_provider".to_string())) + .unwrap() + .as_str() + .unwrap(); + assert_eq!(active, "claude-acp"); + + // providers block should exist with the entry + let providers = config + .get(serde_yaml::Value::String("providers".to_string())) + .unwrap() + .as_mapping() + .unwrap(); + let entry: ProviderEntry = serde_yaml::from_value( + providers + .get(serde_yaml::Value::String("claude-acp".to_string())) + .unwrap() + .clone(), + ) + .unwrap(); + assert!(entry.enabled); + assert!(entry.configured); + assert_eq!(entry.model, "current"); + + // Old flat keys should be removed + assert!(!config.contains_key(serde_yaml::Value::String("GOOSE_PROVIDER".to_string()))); + assert!(!config.contains_key(serde_yaml::Value::String("GOOSE_MODEL".to_string()))); + assert!(!config.contains_key(serde_yaml::Value::String( + "claude-acp_configured".to_string() + ))); + } + + #[test] + fn test_migrate_provider_config_multiple_configured() { + let mut config = Mapping::new(); + config.insert( + serde_yaml::Value::String("GOOSE_PROVIDER".to_string()), + serde_yaml::Value::String("claude-acp".to_string()), + ); + config.insert( + serde_yaml::Value::String("GOOSE_MODEL".to_string()), + serde_yaml::Value::String("current".to_string()), + ); + config.insert( + serde_yaml::Value::String("claude-acp_configured".to_string()), + serde_yaml::Value::Bool(true), + ); + config.insert( + serde_yaml::Value::String("lmstudio_configured".to_string()), + serde_yaml::Value::Bool(true), + ); + + let changed = migrate_provider_config(&mut config); + assert!(changed); + + let providers = config + .get(serde_yaml::Value::String("providers".to_string())) + .unwrap() + .as_mapping() + .unwrap(); + + // Both providers should exist + let claude: ProviderEntry = serde_yaml::from_value( + providers + .get(serde_yaml::Value::String("claude-acp".to_string())) + .unwrap() + .clone(), + ) + .unwrap(); + assert_eq!(claude.model, "current"); + assert!(claude.configured); + + let lmstudio: ProviderEntry = serde_yaml::from_value( + providers + .get(serde_yaml::Value::String("lmstudio".to_string())) + .unwrap() + .clone(), + ) + .unwrap(); + // lmstudio was not the active provider, so model should be empty + assert_eq!(lmstudio.model, ""); + assert!(lmstudio.configured); + + // Old markers removed + assert!(!config.contains_key(serde_yaml::Value::String( + "claude-acp_configured".to_string() + ))); + assert!(!config.contains_key(serde_yaml::Value::String("lmstudio_configured".to_string()))); + } + + #[test] + fn test_migrate_provider_config_idempotent() { + let mut config = Mapping::new(); + config.insert( + serde_yaml::Value::String("GOOSE_PROVIDER".to_string()), + serde_yaml::Value::String("openai".to_string()), + ); + config.insert( + serde_yaml::Value::String("GOOSE_MODEL".to_string()), + serde_yaml::Value::String("gpt-4o".to_string()), + ); + + let changed_first = migrate_provider_config(&mut config); + assert!(changed_first); + + let changed_second = migrate_provider_config(&mut config); + assert!(!changed_second, "Second migration run should be a no-op"); + } + + #[test] + fn test_migrate_provider_config_empty_config() { + let mut config = Mapping::new(); + + let changed = migrate_provider_config(&mut config); + assert!(!changed, "Empty config should not trigger migration"); + } + + #[test] + fn test_migrate_provider_config_no_model() { + let mut config = Mapping::new(); + config.insert( + serde_yaml::Value::String("GOOSE_PROVIDER".to_string()), + serde_yaml::Value::String("anthropic".to_string()), + ); + // No GOOSE_MODEL key + + let changed = migrate_provider_config(&mut config); + assert!(changed); + + let providers = config + .get(serde_yaml::Value::String("providers".to_string())) + .unwrap() + .as_mapping() + .unwrap(); + let entry: ProviderEntry = serde_yaml::from_value( + providers + .get(serde_yaml::Value::String("anthropic".to_string())) + .unwrap() + .clone(), + ) + .unwrap(); + assert_eq!(entry.model, ""); + } + + #[test] + fn test_cleanup_legacy_keys_when_providers_exists() { + let mut config = Mapping::new(); + // Simulate state: providers block exists but stale flat keys remain + let mut providers_map = Mapping::new(); + if let Ok(value) = serde_yaml::to_value(&ProviderEntry { + enabled: true, + model: "current".to_string(), + configured: true, + }) { + providers_map.insert(serde_yaml::Value::String("claude-acp".to_string()), value); + } + config.insert( + serde_yaml::Value::String("providers".to_string()), + serde_yaml::Value::Mapping(providers_map), + ); + config.insert( + serde_yaml::Value::String("GOOSE_PROVIDER".to_string()), + serde_yaml::Value::String("lmstudio".to_string()), + ); + config.insert( + serde_yaml::Value::String("GOOSE_MODEL".to_string()), + serde_yaml::Value::String("some-model".to_string()), + ); + config.insert( + serde_yaml::Value::String("claude-acp_configured".to_string()), + serde_yaml::Value::Bool(true), + ); + + let changed = migrate_provider_config(&mut config); + assert!(changed); + + // Legacy keys should be gone + assert!(!config.contains_key(serde_yaml::Value::String("GOOSE_PROVIDER".to_string()))); + assert!(!config.contains_key(serde_yaml::Value::String("GOOSE_MODEL".to_string()))); + assert!(!config.contains_key(serde_yaml::Value::String( + "claude-acp_configured".to_string() + ))); + + // Providers block should be untouched + assert!(config.contains_key(serde_yaml::Value::String("providers".to_string()))); + + // active_provider should be backfilled from legacy GOOSE_PROVIDER + assert_eq!( + config + .get(serde_yaml::Value::String("active_provider".to_string())) + .and_then(|v| v.as_str()), + Some("lmstudio") + ); + } } diff --git a/crates/goose/src/config/mod.rs b/crates/goose/src/config/mod.rs index cd731c2a..402c835f 100644 --- a/crates/goose/src/config/mod.rs +++ b/crates/goose/src/config/mod.rs @@ -6,6 +6,7 @@ pub mod goose_mode; mod migrations; pub mod paths; pub mod permission; +pub mod providers; pub mod search_path; pub mod signup_nanogpt; pub mod signup_openrouter; @@ -30,3 +31,7 @@ pub use extensions::DEFAULT_DISPLAY_NAME; pub use extensions::DEFAULT_EXTENSION; pub use extensions::DEFAULT_EXTENSION_DESCRIPTION; pub use extensions::DEFAULT_EXTENSION_TIMEOUT; +pub use providers::{ + get_active_model, get_active_provider, get_provider_entry, set_active_provider, + set_provider_entry, ProviderEntry, +}; diff --git a/crates/goose/src/config/providers.rs b/crates/goose/src/config/providers.rs new file mode 100644 index 00000000..fd7dd14f --- /dev/null +++ b/crates/goose/src/config/providers.rs @@ -0,0 +1,259 @@ +use super::base::{Config, ConfigError}; +use indexmap::IndexMap; +use serde::{Deserialize, Serialize}; +use serde_yaml::Mapping; +use std::env; +use tracing::warn; + +const PROVIDERS_CONFIG_KEY: &str = "providers"; +const ACTIVE_PROVIDER_KEY: &str = "active_provider"; + +/// A single provider's persisted configuration within the `providers:` block. +/// +/// The `providers` block in config.yaml is the authoritative source for +/// per-provider settings, replacing the old flat-key scheme where switching +/// providers destructively overwrote `GOOSE_PROVIDER` / `GOOSE_MODEL`. +#[derive(Debug, Deserialize, Serialize, Clone)] +pub struct ProviderEntry { + #[serde(default)] + pub enabled: bool, + #[serde(default)] + pub model: String, + #[serde(default)] + pub configured: bool, +} + +// --------------------------------------------------------------------------- +// Read helpers +// --------------------------------------------------------------------------- + +fn parse_providers_map(raw: Mapping) -> IndexMap { + let mut map = IndexMap::with_capacity(raw.len()); + for (k, v) in raw { + match (k, serde_yaml::from_value::(v)) { + (serde_yaml::Value::String(key), Ok(entry)) => { + map.insert(key, entry); + } + (k, v) => { + warn!( + key = ?k, + value = ?v, + "Skipping malformed provider config entry" + ); + } + } + } + map +} + +fn get_providers_map(config: &Config) -> IndexMap { + let raw: Mapping = config + .get_param(PROVIDERS_CONFIG_KEY) + .unwrap_or_else(|_| Default::default()); + parse_providers_map(raw) +} + +/// Retrieve the [`ProviderEntry`] for a named provider, if it exists. +pub fn get_provider_entry(config: &Config, name: &str) -> Option { + get_providers_map(config).get(name).cloned() +} + +// --------------------------------------------------------------------------- +// Write helpers +// --------------------------------------------------------------------------- + +/// Persist a [`ProviderEntry`] under `providers.{name}`. +pub fn set_provider_entry( + config: &Config, + name: &str, + entry: &ProviderEntry, +) -> Result<(), ConfigError> { + let name = name.to_string(); + let entry = entry.clone(); + config.update_param::(PROVIDERS_CONFIG_KEY, |raw| { + let mut map = parse_providers_map(raw); + map.insert(name, entry); + map + }) +} + +// --------------------------------------------------------------------------- +// Active-provider accessors +// --------------------------------------------------------------------------- + +/// Return the currently active provider name. +/// +/// Resolution order: +/// 1. `GOOSE_PROVIDER` environment variable (uppercase check performed by +/// `get_param`) +/// 2. `active_provider` key in config.yaml +/// 3. Legacy flat `GOOSE_PROVIDER` key in config.yaml (backward compat) +pub fn get_active_provider(config: &Config) -> Option { + // Env var takes precedence (get_param checks env automatically) + if let Ok(val) = env::var("GOOSE_PROVIDER") { + return Some(val); + } + + // New structured key + if let Ok(val) = config.get_param::(ACTIVE_PROVIDER_KEY) { + return Some(val); + } + + // Legacy flat key fallback + config.get_param::("GOOSE_PROVIDER").ok() +} + +/// Return the model for the currently active provider. +/// +/// Resolution order: +/// 1. `GOOSE_MODEL` environment variable +/// 2. Model recorded in the active provider's entry (`providers.{name}.model`) +/// 3. Legacy flat `GOOSE_MODEL` key in config.yaml +pub fn get_active_model(config: &Config) -> Option { + // Env var takes precedence + if let Ok(val) = env::var("GOOSE_MODEL") { + return Some(val); + } + + // Try provider entry model + if let Some(provider_name) = get_active_provider(config) { + if let Some(entry) = get_provider_entry(config, &provider_name) { + if !entry.model.is_empty() { + return Some(entry.model); + } + } + } + + // Legacy flat key fallback + config.get_param::("GOOSE_MODEL").ok() +} + +/// Set the active provider and update its entry in the `providers` block. +/// +/// This writes: +/// - `active_provider: {name}` at the top level +/// - `providers.{name}` with `configured: true`, `enabled: true`, and the +/// supplied model. +pub fn set_active_provider(config: &Config, name: &str, model: &str) -> Result<(), ConfigError> { + config.set_param(ACTIVE_PROVIDER_KEY, name)?; + let entry = ProviderEntry { + enabled: true, + model: model.to_string(), + configured: true, + }; + set_provider_entry(config, name, &entry) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::NamedTempFile; + + fn new_test_config() -> Config { + let config_file = NamedTempFile::new().unwrap(); + let secrets_file = NamedTempFile::new().unwrap(); + Config::new_with_file_secrets(config_file.path(), secrets_file.path()).unwrap() + } + + #[test] + fn test_set_and_get_provider_entry() { + let config = new_test_config(); + let entry = ProviderEntry { + enabled: true, + model: "gpt-4o".to_string(), + configured: true, + }; + set_provider_entry(&config, "openai", &entry).unwrap(); + + let loaded = get_provider_entry(&config, "openai").unwrap(); + assert!(loaded.enabled); + assert_eq!(loaded.model, "gpt-4o"); + assert!(loaded.configured); + } + + #[test] + fn test_get_provider_entry_missing() { + let config = new_test_config(); + assert!(get_provider_entry(&config, "nonexistent").is_none()); + } + + #[test] + fn test_set_active_provider_writes_structured_keys() { + let config = new_test_config(); + set_active_provider(&config, "claude-acp", "current").unwrap(); + + let active: String = config.get_param(ACTIVE_PROVIDER_KEY).unwrap(); + assert_eq!(active, "claude-acp"); + + let entry = get_provider_entry(&config, "claude-acp").unwrap(); + assert!(entry.enabled); + assert!(entry.configured); + assert_eq!(entry.model, "current"); + } + + #[test] + fn test_get_active_provider_from_new_key() { + let config = new_test_config(); + config.set_param(ACTIVE_PROVIDER_KEY, "openai").unwrap(); + + let result = get_active_provider(&config); + assert_eq!(result, Some("openai".to_string())); + } + + #[test] + fn test_get_active_provider_falls_back_to_legacy() { + let config = new_test_config(); + config.set_param("GOOSE_PROVIDER", "anthropic").unwrap(); + + let result = get_active_provider(&config); + assert_eq!(result, Some("anthropic".to_string())); + } + + #[test] + fn test_get_active_provider_none_when_empty() { + let config = new_test_config(); + let result = get_active_provider(&config); + assert_eq!(result, None); + } + + #[test] + fn test_get_active_model_from_provider_entry() { + let config = new_test_config(); + set_active_provider(&config, "openai", "gpt-4o").unwrap(); + + let result = get_active_model(&config); + assert_eq!(result, Some("gpt-4o".to_string())); + } + + #[test] + fn test_get_active_model_falls_back_to_legacy() { + let config = new_test_config(); + // Only set the legacy key, no providers block + config.set_param("GOOSE_MODEL", "gpt-3.5-turbo").unwrap(); + + let result = get_active_model(&config); + assert_eq!(result, Some("gpt-3.5-turbo".to_string())); + } + + #[test] + fn test_multiple_providers_preserved() { + let config = new_test_config(); + + // Set up two providers + set_active_provider(&config, "openai", "gpt-4o").unwrap(); + set_active_provider(&config, "anthropic", "claude-3-opus").unwrap(); + + // Both entries should exist + let openai = get_provider_entry(&config, "openai").unwrap(); + assert_eq!(openai.model, "gpt-4o"); + assert!(openai.configured); + + let anthropic = get_provider_entry(&config, "anthropic").unwrap(); + assert_eq!(anthropic.model, "claude-3-opus"); + assert!(anthropic.configured); + + // Active provider should be the last one set + let active = get_active_provider(&config); + assert_eq!(active, Some("anthropic".to_string())); + } +} diff --git a/crates/goose/src/config/signup_nanogpt/mod.rs b/crates/goose/src/config/signup_nanogpt/mod.rs index ecdc76cf..45699f6c 100644 --- a/crates/goose/src/config/signup_nanogpt/mod.rs +++ b/crates/goose/src/config/signup_nanogpt/mod.rs @@ -113,6 +113,10 @@ pub async fn complete_nanogpt_auth() -> Result { pub fn configure_nanogpt(config: &Config, api_key: String) -> Result<()> { config.set_secret("NANOGPT_API_KEY", &api_key)?; - config.set_goose_provider("nano-gpt")?; + crate::config::set_active_provider( + config, + crate::providers::nanogpt::NANOGPT_PROVIDER_NAME, + crate::providers::nanogpt::NANOGPT_DEFAULT_MODEL, + )?; Ok(()) } diff --git a/crates/goose/src/config/signup_openrouter/mod.rs b/crates/goose/src/config/signup_openrouter/mod.rs index 04237270..26b2655a 100644 --- a/crates/goose/src/config/signup_openrouter/mod.rs +++ b/crates/goose/src/config/signup_openrouter/mod.rs @@ -165,7 +165,10 @@ use crate::config::Config; pub fn configure_openrouter(config: &Config, api_key: String) -> Result<()> { config.set_secret("OPENROUTER_API_KEY", &api_key)?; - config.set_goose_provider("openrouter")?; - config.set_goose_model(OPENROUTER_DEFAULT_MODEL)?; + crate::config::set_active_provider( + config, + crate::providers::openrouter::OPENROUTER_PROVIDER_NAME, + OPENROUTER_DEFAULT_MODEL, + )?; Ok(()) } diff --git a/crates/goose/src/config/signup_tetrate/mod.rs b/crates/goose/src/config/signup_tetrate/mod.rs index 0d4398e6..c8adc9e9 100644 --- a/crates/goose/src/config/signup_tetrate/mod.rs +++ b/crates/goose/src/config/signup_tetrate/mod.rs @@ -162,7 +162,10 @@ use crate::config::Config; pub fn configure_tetrate(config: &Config, api_key: String) -> Result<()> { config.set_secret("TETRATE_API_KEY", &api_key)?; - config.set_goose_provider("tetrate")?; - config.set_goose_model(TETRATE_DEFAULT_MODEL)?; + crate::config::set_active_provider( + config, + crate::providers::tetrate::TETRATE_PROVIDER_NAME, + TETRATE_DEFAULT_MODEL, + )?; Ok(()) } diff --git a/crates/goose/src/doctor.rs b/crates/goose/src/doctor.rs index 313ff267..2da8cadf 100644 --- a/crates/goose/src/doctor.rs +++ b/crates/goose/src/doctor.rs @@ -141,10 +141,11 @@ async fn save_and_set( provider: Arc, ) -> anyhow::Result<()> { let config = Config::global(); - config.set_goose_provider(provider.get_name()).ok(); - config - .set_goose_model(&provider.get_model_config().model_name) - .ok(); + crate::config::set_active_provider( + config, + provider.get_name(), + &provider.get_model_config().model_name, + )?; agent.update_provider(provider, session_id).await } diff --git a/crates/goose/src/posthog.rs b/crates/goose/src/posthog.rs index 6f54c26e..66a34b61 100644 --- a/crates/goose/src/posthog.rs +++ b/crates/goose/src/posthog.rs @@ -355,10 +355,10 @@ async fn send_error_event( } let config = Config::global(); - if let Ok(provider) = config.get_param::("GOOSE_PROVIDER") { + if let Ok(provider) = config.get_goose_provider() { insert(&mut props, "provider", provider); } - if let Ok(model) = config.get_param::("GOOSE_MODEL") { + if let Ok(model) = config.get_goose_model() { insert(&mut props, "model", model); } @@ -407,10 +407,10 @@ async fn send_session_event(installation: &InstallationData) -> Result<(), Strin insert(&mut props, "days_since_install", days_since_install); let config = Config::global(); - if let Ok(provider) = config.get_param::("GOOSE_PROVIDER") { + if let Ok(provider) = config.get_goose_provider() { insert(&mut props, "provider", provider); } - if let Ok(model) = config.get_param::("GOOSE_MODEL") { + if let Ok(model) = config.get_goose_model() { insert(&mut props, "model", model); } diff --git a/crates/goose/src/providers/amp_acp.rs b/crates/goose/src/providers/amp_acp.rs index 8640c446..96d4f3c4 100644 --- a/crates/goose/src/providers/amp_acp.rs +++ b/crates/goose/src/providers/amp_acp.rs @@ -36,7 +36,7 @@ impl ProviderDef for AmpAcpProvider { "Install the Amp CLI: `curl -fsSL https://ampcode.com/install.sh | bash`", "Install the ACP adapter: `npm install -g amp-acp`", "Ensure your Amp CLI is authenticated (run `amp` to verify)", - "Set in your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: amp-acp\n GOOSE_MODEL: current", + "Add to your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: amp-acp\n GOOSE_MODEL: current\n amp-acp_configured: true", "Restart goose for changes to take effect", ]) .with_model_selection_hint("Use the Amp CLI to configure models") diff --git a/crates/goose/src/providers/claude_acp.rs b/crates/goose/src/providers/claude_acp.rs index 415889af..93f31976 100644 --- a/crates/goose/src/providers/claude_acp.rs +++ b/crates/goose/src/providers/claude_acp.rs @@ -35,7 +35,7 @@ impl ProviderDef for ClaudeAcpProvider { .with_setup_steps(vec![ "Install the ACP adapter: `npm install -g @agentclientprotocol/claude-agent-acp`", "Ensure your Claude CLI is authenticated (run `claude` to verify)", - "Set in your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: claude-acp\n GOOSE_MODEL: current", + "Add to your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: claude-acp\n GOOSE_MODEL: current\n claude-acp_configured: true", "Restart goose for changes to take effect", ]) } diff --git a/crates/goose/src/providers/codex_acp.rs b/crates/goose/src/providers/codex_acp.rs index 5c3f46b3..9447e67b 100644 --- a/crates/goose/src/providers/codex_acp.rs +++ b/crates/goose/src/providers/codex_acp.rs @@ -34,7 +34,7 @@ impl ProviderDef for CodexAcpProvider { .with_setup_steps(vec![ "Install the ACP adapter: `npm install -g @zed-industries/codex-acp`", "Run `codex` once to authenticate with your OpenAI account", - "Set in your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: codex-acp\n GOOSE_MODEL: current", + "Add to your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: codex-acp\n GOOSE_MODEL: current\n codex-acp_configured: true", "Restart goose for changes to take effect", ]) } diff --git a/crates/goose/src/providers/copilot_acp.rs b/crates/goose/src/providers/copilot_acp.rs index 7f02c7b8..1c0d05a2 100644 --- a/crates/goose/src/providers/copilot_acp.rs +++ b/crates/goose/src/providers/copilot_acp.rs @@ -38,7 +38,7 @@ impl ProviderDef for CopilotAcpProvider { .with_setup_steps(vec![ "Install the Copilot CLI: `npm install -g @github/copilot`", "Run `copilot login` to authenticate with your GitHub account", - "Set in your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: copilot-acp\n GOOSE_MODEL: current", + "Add to your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: copilot-acp\n GOOSE_MODEL: current\n copilot-acp_configured: true", "Restart goose for changes to take effect", ]) } diff --git a/crates/goose/src/providers/nanogpt.rs b/crates/goose/src/providers/nanogpt.rs index fa2bb386..6c28d3b2 100644 --- a/crates/goose/src/providers/nanogpt.rs +++ b/crates/goose/src/providers/nanogpt.rs @@ -12,7 +12,7 @@ use async_trait::async_trait; use futures::future::BoxFuture; use rmcp::model::Tool; -const NANOGPT_PROVIDER_NAME: &str = "nano-gpt"; +pub const NANOGPT_PROVIDER_NAME: &str = "nano-gpt"; pub const NANOGPT_API_HOST: &str = "https://nano-gpt.com/api/v1"; pub const NANOGPT_SUBSCRIPTION_HOST: &str = "https://nano-gpt.com/api/subscription/v1"; pub const NANOGPT_DEFAULT_MODEL: &str = "anthropic/claude-sonnet-4.6"; diff --git a/crates/goose/src/providers/openrouter.rs b/crates/goose/src/providers/openrouter.rs index 08b8689b..5e1cf5bf 100644 --- a/crates/goose/src/providers/openrouter.rs +++ b/crates/goose/src/providers/openrouter.rs @@ -15,7 +15,7 @@ use crate::providers::formats::openai::create_request; use crate::providers::formats::openrouter as openrouter_format; use rmcp::model::Tool; -const OPENROUTER_PROVIDER_NAME: &str = "openrouter"; +pub const OPENROUTER_PROVIDER_NAME: &str = "openrouter"; pub const OPENROUTER_DEFAULT_MODEL: &str = "anthropic/claude-sonnet-4"; pub const OPENROUTER_DEFAULT_FAST_MODEL: &str = "google/gemini-2.5-flash"; pub const OPENROUTER_MODEL_PREFIX_ANTHROPIC: &str = "anthropic"; diff --git a/crates/goose/src/providers/pi_acp.rs b/crates/goose/src/providers/pi_acp.rs index 52215d56..5e36292a 100644 --- a/crates/goose/src/providers/pi_acp.rs +++ b/crates/goose/src/providers/pi_acp.rs @@ -35,7 +35,7 @@ impl ProviderDef for PiAcpProvider { .with_setup_steps(vec![ "Install the Pi CLI and the pi-acp adapter", "Ensure your Pi CLI is authenticated (run `pi` to verify)", - "Set in your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: pi-acp\n GOOSE_MODEL: current", + "Add to your goose config file (`~/.config/goose/config.yaml` on macOS/Linux):\n GOOSE_PROVIDER: pi-acp\n GOOSE_MODEL: current\n pi-acp_configured: true", "Restart goose for changes to take effect", ]) .with_model_selection_hint("Use the Pi CLI to configure models") diff --git a/crates/goose/src/providers/tetrate.rs b/crates/goose/src/providers/tetrate.rs index 81024616..834a2061 100644 --- a/crates/goose/src/providers/tetrate.rs +++ b/crates/goose/src/providers/tetrate.rs @@ -18,7 +18,7 @@ use crate::providers::formats::openai::create_request; use rmcp::model::Tool; use serde_json::Value; -const TETRATE_PROVIDER_NAME: &str = "tetrate"; +pub const TETRATE_PROVIDER_NAME: &str = "tetrate"; pub const TETRATE_DOC_URL: &str = "https://router.tetrate.ai"; pub const TETRATE_BILLING_URL: &str = "https://router.tetrate.ai/billing"; diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index e0b73df4..460ea464 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -6897,6 +6897,10 @@ }, "provider_type": { "$ref": "#/components/schemas/ProviderType" + }, + "saved_model": { + "type": "string", + "nullable": true } } }, diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index da88661b..e1e5c83e 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -953,6 +953,7 @@ export type ProviderDetails = { metadata: ProviderMetadata; name: string; provider_type: ProviderType; + saved_model?: string | null; }; export type ProviderEngine = 'openai' | 'ollama' | 'anthropic'; diff --git a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx index 0bbda0cf..c1f0fb33 100644 --- a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx +++ b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx @@ -594,6 +594,13 @@ export const SwitchModelModal = ({ // Don't auto-select if user explicitly cleared the model if (!provider || loadingModels || model || isCustomModel || userClearedModel) return; + // Use saved model from provider config if available + const providerInfo = activeProvidersList.find((p) => p.name === provider); + if (providerInfo?.saved_model) { + setModel(providerInfo.saved_model); + return; + } + const providerModels = modelOptions .filter((group) => group.options[0]?.provider === provider) .flatMap((group) => group.options); @@ -604,7 +611,7 @@ export const SwitchModelModal = ({ setModel(preferredModel); } } - }, [provider, modelOptions, loadingModels, model, isCustomModel, userClearedModel]); + }, [provider, modelOptions, loadingModels, model, isCustomModel, userClearedModel, activeProvidersList]); // Handle model selection change const handleModelChange = (newValue: unknown) => {