mirror of
https://github.com/block/goose.git
synced 2026-10-19 01:27:15 +00:00
fix(providers): reject malformed GOOSE_MODE across ACP providers (#12846)
This commit is contained in:
parent
29a1f73cdd
commit
3295f5e0f6
7 changed files with 56 additions and 52 deletions
|
|
@ -1276,6 +1276,14 @@ config_value!(CHATGPT_CODEX_REASONING_EFFORT, String, "medium");
|
|||
config_value!(GOOSE_SEARCH_PATHS, Vec<String>);
|
||||
config_value!(GOOSE_MODE, GooseMode);
|
||||
impl Config {
|
||||
pub(crate) fn resolve_goose_mode(&self) -> Result<GooseMode, ConfigError> {
|
||||
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<GooseMode, ConfigError> {
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -349,7 +349,9 @@ impl ClaudeCodeProvider {
|
|||
/// Returns true when the control protocol is enabled.
|
||||
fn apply_permission_flags(cmd: &mut Command) -> Result<bool, ProviderError> {
|
||||
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 => {
|
||||
|
|
|
|||
|
|
@ -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<GooseMode, ConfigError>,
|
||||
) -> Result<GooseMode, ConfigError> {
|
||||
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() {
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue