diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 4b914b313e..5f1603dfe6 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -1276,6 +1276,14 @@ config_value!(CHATGPT_CODEX_REASONING_EFFORT, String, "medium"); config_value!(GOOSE_SEARCH_PATHS, Vec); config_value!(GOOSE_MODE, GooseMode); impl Config { + pub(crate) fn resolve_goose_mode(&self) -> Result { + match self.get_goose_mode_strict() { + Ok(mode) => Ok(mode), + Err(ConfigError::NotFound(_)) => Ok(GooseMode::Auto), + Err(error) => Err(error), + } + } + pub(crate) fn get_goose_mode_strict(&self) -> Result { match env::var("GOOSE_MODE") { Ok(value) => { @@ -1456,6 +1464,43 @@ mod tests { use super::*; use serial_test::serial; use tempfile::{NamedTempFile, TempDir}; + + #[test] + fn resolve_goose_mode_defaults_only_when_missing() { + let _guard = env_lock::lock_env([("GOOSE_MODE", None::<&str>)]); + let config_file = NamedTempFile::new().unwrap(); + let secrets_file = NamedTempFile::new().unwrap(); + let config = + Config::new_with_file_secrets(config_file.path(), secrets_file.path()).unwrap(); + + assert_eq!(config.resolve_goose_mode().unwrap(), GooseMode::Auto); + for mode in [ + GooseMode::Auto, + GooseMode::SmartApprove, + GooseMode::Approve, + GooseMode::Chat, + ] { + config.set_goose_mode(mode).unwrap(); + assert_eq!(config.resolve_goose_mode().unwrap(), mode); + } + + std::fs::write(config_file.path(), "GOOSE_MODE: invalid\n").unwrap(); + assert!(matches!( + config.resolve_goose_mode(), + Err(ConfigError::DeserializeError(_)) + )); + } + + #[test] + fn resolve_goose_mode_rejects_invalid_environment_value() { + let _guard = env_lock::lock_env([("GOOSE_MODE", Some("invalid"))]); + let config = new_test_config(); + assert!(matches!( + config.resolve_goose_mode(), + Err(ConfigError::DeserializeError(_)) + )); + } + #[test] fn test_basic_config() -> Result<(), ConfigError> { let config = new_test_config(); diff --git a/crates/goose/src/providers/amp_acp.rs b/crates/goose/src/providers/amp_acp.rs index 63cecf57db..f8d1cadc4b 100644 --- a/crates/goose/src/providers/amp_acp.rs +++ b/crates/goose/src/providers/amp_acp.rs @@ -64,7 +64,7 @@ impl ProviderDef for AmpAcpProvider { Box::pin(async move { let config = Config::global(); let resolved_command = SearchPaths::builder().with_npm().resolve(AMP_ACP_BINARY)?; - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = config.resolve_goose_mode()?; let mode_mapping = HashMap::from([ // "bypass" skips confirmations, closest to autonomous mode. diff --git a/crates/goose/src/providers/claude_acp.rs b/crates/goose/src/providers/claude_acp.rs index 589cab85c4..7271c5f114 100644 --- a/crates/goose/src/providers/claude_acp.rs +++ b/crates/goose/src/providers/claude_acp.rs @@ -69,7 +69,7 @@ impl ProviderDef for ClaudeAcpProvider { let resolved_command = SearchPaths::builder() .with_npm() .resolve(CLAUDE_ACP_BINARY)?; - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = config.resolve_goose_mode()?; let mode_mapping = HashMap::from([ // Closest to "autonomous": bypassPermissions skips confirmations. diff --git a/crates/goose/src/providers/claude_code.rs b/crates/goose/src/providers/claude_code.rs index 1ea4a3cd8c..8d523d899e 100644 --- a/crates/goose/src/providers/claude_code.rs +++ b/crates/goose/src/providers/claude_code.rs @@ -349,7 +349,9 @@ impl ClaudeCodeProvider { /// Returns true when the control protocol is enabled. fn apply_permission_flags(cmd: &mut Command) -> Result { let config = Config::global(); - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = config + .resolve_goose_mode() + .map_err(|error| ProviderError::RequestFailed(error.to_string()))?; match goose_mode { GooseMode::Auto => { diff --git a/crates/goose/src/providers/codex_acp.rs b/crates/goose/src/providers/codex_acp.rs index aa4d06e39a..ef94161c3c 100644 --- a/crates/goose/src/providers/codex_acp.rs +++ b/crates/goose/src/providers/codex_acp.rs @@ -7,7 +7,7 @@ use crate::acp::{ extension_configs_to_mcp_servers, AcpProvider, AcpProviderConfig, ACP_CURRENT_MODEL, }; use crate::config::search_path::SearchPaths; -use crate::config::{Config, ConfigError, GooseMode}; +use crate::config::{Config, GooseMode}; use crate::providers::base::{ current_working_dir, ProviderDef, ProviderDescriptor, ProviderMetadata, }; @@ -18,16 +18,6 @@ const CODEX_ACP_DOC_URL: &str = "https://github.com/agentclientprotocol/codex-ac pub struct CodexAcpProvider; -fn resolve_goose_mode( - configured_mode: Result, -) -> Result { - match configured_mode { - Ok(mode) => Ok(mode), - Err(ConfigError::NotFound(_)) => Ok(GooseMode::Auto), - Err(error) => Err(error), - } -} - impl goose_providers::base::ProviderDescriptor for CodexAcpProvider { fn metadata() -> ProviderMetadata { ProviderMetadata::new( @@ -77,7 +67,7 @@ impl ProviderDef for CodexAcpProvider { let resolved_command = SearchPaths::builder() .with_npm() .resolve(CODEX_ACP_PROVIDER_NAME)?; - let goose_mode = resolve_goose_mode(config.get_goose_mode_strict())?; + let goose_mode = config.resolve_goose_mode()?; let mcp_servers = extension_configs_to_mcp_servers(&extensions); let mode_mapping = HashMap::from([ @@ -110,6 +100,7 @@ impl ProviderDef for CodexAcpProvider { #[cfg(test)] mod tests { use super::*; + use crate::config::ConfigError; #[cfg(unix)] use std::fs; #[cfg(unix)] @@ -132,40 +123,6 @@ mod tests { DanglingParentSymlink, } - #[test] - fn missing_goose_mode_defaults_to_auto() { - assert_eq!( - resolve_goose_mode(Err(ConfigError::NotFound("GOOSE_MODE".to_string()))).unwrap(), - GooseMode::Auto - ); - } - - #[test] - fn configured_goose_modes_are_preserved() { - for mode in [ - GooseMode::Auto, - GooseMode::SmartApprove, - GooseMode::Approve, - GooseMode::Chat, - ] { - assert_eq!(resolve_goose_mode(Ok(mode)).unwrap(), mode); - } - } - - #[test] - fn invalid_goose_mode_errors_are_preserved() { - assert!(matches!( - resolve_goose_mode(Err(ConfigError::DeserializeError("invalid".to_string()))), - Err(ConfigError::DeserializeError(_)) - )); - assert!(matches!( - resolve_goose_mode(Err(ConfigError::FileError(std::io::Error::other( - "unreadable" - )))), - Err(ConfigError::FileError(_)) - )); - } - #[cfg(unix)] #[tokio::test] async fn goose_mode_validation_precedes_codex_acp_launch() { diff --git a/crates/goose/src/providers/copilot_acp.rs b/crates/goose/src/providers/copilot_acp.rs index f78e782b14..d280553dec 100644 --- a/crates/goose/src/providers/copilot_acp.rs +++ b/crates/goose/src/providers/copilot_acp.rs @@ -62,7 +62,7 @@ impl CopilotAcpProvider { let resolved_command = SearchPaths::builder() .with_npm() .resolve(COPILOT_ACP_BINARY)?; - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = config.resolve_goose_mode()?; let model = if use_default_model { ACP_CURRENT_MODEL.to_string() } else { diff --git a/crates/goose/src/providers/pi_acp.rs b/crates/goose/src/providers/pi_acp.rs index 797890e930..14b4b27587 100644 --- a/crates/goose/src/providers/pi_acp.rs +++ b/crates/goose/src/providers/pi_acp.rs @@ -8,7 +8,7 @@ use crate::acp::{ AcpProviderConfig, ACP_CURRENT_MODEL, }; use crate::config::search_path::SearchPaths; -use crate::config::{Config, GooseMode}; +use crate::config::Config; use crate::providers::base::{ current_working_dir, ProviderDef, ProviderDescriptor, ProviderMetadata, }; @@ -53,7 +53,7 @@ impl PiAcpProvider { Box::pin(async move { let config = Config::global(); let resolved_command = SearchPaths::builder().with_npm().resolve(PI_ACP_BINARY)?; - let goose_mode = config.get_goose_mode().unwrap_or(GooseMode::Auto); + let goose_mode = config.resolve_goose_mode()?; let model = if use_default_model { ACP_CURRENT_MODEL.to_string() } else {