diff --git a/Cargo.lock b/Cargo.lock index 716dad989..2b6aabcf8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7026,9 +7026,9 @@ dependencies = [ [[package]] name = "nostr" -version = "0.44.6" +version = "0.44.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e826dd648489de2c5b293920e20b92932ef820302007c1987c758d4d06eeb2cf" +checksum = "c7d3d987ea7078dc36947cde532637c472a229426702e4331dd7667325378bd9" dependencies = [ "base64 0.22.1", "bech32", @@ -7070,9 +7070,9 @@ dependencies = [ [[package]] name = "nostr-relay-pool" -version = "0.44.1" +version = "0.44.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91b2c039df4f96c4bf7dae52a74fd5516ad6dda83a11c0c69dea91b5255a4f37" +checksum = "c85c54d6ca9aae4ae2bf19a7663ba9db5f45f783f1d24aff55f006386b8b99a1" dependencies = [ "async-utility", "async-wsocket", diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index f999940ea..0ac0c4feb 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -1399,6 +1399,7 @@ struct ServeCommandArgs { async fn handle_serve_command(args: ServeCommandArgs) -> Result<()> { use axum::http::HeaderValue; + use goose::acp::server::AcpBuiltinSelection; use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; use goose::acp::transport::create_router; use goose::config::paths::Paths; @@ -1420,9 +1421,15 @@ async fn handle_serve_command(args: ServeCommandArgs) -> Result<()> { } = args; let builtins = if builtins.is_empty() { - vec!["developer".to_string()] + AcpBuiltinSelection { + defaults: vec!["developer".to_string()], + explicit: Vec::new(), + } } else { - builtins + AcpBuiltinSelection { + defaults: Vec::new(), + explicit: builtins, + } }; let additional_source_roots = Config::global() diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index 04603cab4..ae7a2bf70 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -208,7 +208,7 @@ goose-acp-macros = { path = "../goose-acp-macros", default-features = false } tower-http = { workspace = true, features = ["cors"] } tracing-appender = { workspace = true } process-wrap = { version = "9", default-features = false, features = ["std"] } -nostr = { version = "0.44", default-features = false, features = ["nip44", "std"], optional = true } +nostr = { version = "0.44.7", default-features = false, features = ["nip44", "std"], optional = true } nostr-sdk = { version = "0.44", default-features = false, features = ["nip44"], optional = true } rustls = { workspace = true, optional = true } rcgen = { version = "0.14", default-features = false, features = ["crypto", "pem"], optional = true } diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 82a249c0e..9a0f69356 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -15,7 +15,7 @@ use crate::agents::{ Agent, AgentConfig, ExtensionConfig, ExtensionLoadResult, GoosePlatform, SessionConfig, }; use crate::config::base::CONFIG_YAML_NAME; -use crate::config::extensions::get_enabled_extensions_with_config; +use crate::config::extensions::{configured_enabled_state, get_enabled_extensions_with_config}; use crate::config::paths::Paths; use crate::config::permission::PermissionManager; use crate::config::{Config, GooseMode}; @@ -177,9 +177,15 @@ struct ActivePromptRun { cancel_token: CancellationToken, } +#[derive(Clone, Debug, Default)] +pub struct AcpBuiltinSelection { + pub defaults: Vec, + pub explicit: Vec, +} + pub struct GooseAcpAgentOptions { pub provider_factory: AcpProviderFactory, - pub builtins: Vec, + pub builtin_selection: AcpBuiltinSelection, pub data_dir: std::path::PathBuf, pub config_dir: std::path::PathBuf, pub disable_session_naming: bool, @@ -194,7 +200,7 @@ pub struct GooseAcpAgent { closed_session_ids: Arc>>, agent_manager: Arc, provider_factory: AcpProviderFactory, - builtins: Vec, + builtin_selection: AcpBuiltinSelection, client_fs_capabilities: OnceCell, client_terminal: OnceCell, client_mcp_host_info: OnceCell, @@ -390,6 +396,25 @@ fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result Vec { + let mut extensions = Vec::new(); + + for builtin in &builtin_selection.defaults { + if configured_enabled_state(config, builtin) != Some(false) { + push_or_replace_extension(&mut extensions, builtin_to_extension_config(builtin)); + } + } + + for builtin in &builtin_selection.explicit { + push_or_replace_extension(&mut extensions, builtin_to_extension_config(builtin)); + } + + extensions +} + fn push_or_replace_extension(extensions: &mut Vec, extension: ExtensionConfig) { let name = extension.name().to_string(); if let Some(index) = extensions @@ -617,7 +642,7 @@ impl GooseAcpAgent { closed_session_ids: Arc::new(Mutex::new(HashSet::new())), agent_manager, provider_factory: options.provider_factory, - builtins: options.builtins, + builtin_selection: options.builtin_selection, client_fs_capabilities: OnceCell::new(), client_terminal: OnceCell::new(), client_mcp_host_info: OnceCell::new(), @@ -712,10 +737,7 @@ impl GooseAcpAgent { goose_extensions: Option>, recipe_extensions: Option<&[ExtensionConfig]>, ) -> Result, agent_client_protocol::Error> { - let mut extensions = Vec::new(); - for builtin in &self.builtins { - push_or_replace_extension(&mut extensions, builtin_to_extension_config(builtin)); - } + let mut extensions = selected_builtin_extensions(config, &self.builtin_selection); if let Some(recipe_extensions) = recipe_extensions { for extension in recipe_extensions { @@ -2293,7 +2315,10 @@ pub async fn run(builtins: Vec, enable_scheduler: bool) -> Result<()> { let server = crate::acp::server_factory::AcpServer::new( crate::acp::server_factory::AcpServerFactoryConfig { - builtins, + builtins: AcpBuiltinSelection { + explicit: builtins, + ..Default::default() + }, data_dir: Paths::data_dir(), config_dir: Paths::config_dir(), goose_platform: GoosePlatform::GooseCli, @@ -2319,6 +2344,101 @@ mod tests { use tempfile::NamedTempFile; use test_case::test_case; + fn config_with_yaml(yaml: &str) -> (Config, NamedTempFile, NamedTempFile) { + let config_file = NamedTempFile::new().unwrap(); + let secrets_file = NamedTempFile::new().unwrap(); + std::fs::write(config_file.path(), yaml).unwrap(); + let config = + Config::new_with_file_secrets(config_file.path(), secrets_file.path()).unwrap(); + (config, config_file, secrets_file) + } + + fn has_developer(extensions: &[ExtensionConfig]) -> bool { + extensions.iter().any(|ext| ext.name() == "developer") + } + + fn default_builtin(name: &str) -> AcpBuiltinSelection { + AcpBuiltinSelection { + defaults: vec![name.to_string()], + ..Default::default() + } + } + + fn explicit_builtin(name: &str) -> AcpBuiltinSelection { + AcpBuiltinSelection { + explicit: vec![name.to_string()], + ..Default::default() + } + } + + #[test] + fn default_builtin_developer_loads_when_config_is_empty() { + let (config, _c, _s) = config_with_yaml(""); + let selected = selected_builtin_extensions(&config, &default_builtin("developer")); + assert!( + has_developer(&selected), + "developer should load by default on a fresh config" + ); + } + + #[test] + fn default_builtin_developer_loads_when_enabled() { + let (config, _c, _s) = config_with_yaml( + r#" +extensions: + developer: + enabled: true + type: builtin + name: developer +"#, + ); + let selected = selected_builtin_extensions(&config, &default_builtin("developer")); + assert!(has_developer(&selected)); + } + + #[test] + fn default_builtin_developer_skipped_when_disabled() { + let (config, _c, _s) = config_with_yaml( + r#" +extensions: + developer: + enabled: false + type: builtin + name: developer +"#, + ); + let selected = selected_builtin_extensions(&config, &default_builtin("developer")); + assert!( + !has_developer(&selected), + "developer must NOT load when the user disabled it (issue #10221)" + ); + } + + #[test] + fn explicit_builtin_developer_loads_when_disabled() { + let (config, _c, _s) = config_with_yaml( + r#" +extensions: + developer: + enabled: false + type: builtin + name: developer +"#, + ); + let selected = selected_builtin_extensions(&config, &explicit_builtin("developer")); + assert!(has_developer(&selected)); + } + + #[test] + fn default_off_builtin_loads_when_explicitly_requested() { + let (config, _c, _s) = config_with_yaml(""); + let selected = selected_builtin_extensions(&config, &explicit_builtin("chatrecall")); + assert!( + selected.iter().any(|ext| ext.name() == "chatrecall"), + "default-off builtins must load when explicitly requested via builtins" + ); + } + #[test_case( McpServer::Stdio( McpServerStdio::new("github", "/path/to/github-mcp-server") diff --git a/crates/goose/src/acp/server/schedule.rs b/crates/goose/src/acp/server/schedule.rs index 8e22fc850..676773c36 100644 --- a/crates/goose/src/acp/server/schedule.rs +++ b/crates/goose/src/acp/server/schedule.rs @@ -344,6 +344,7 @@ impl GooseAcpAgent { #[cfg(test)] mod tests { use super::*; + use crate::acp::server::AcpBuiltinSelection; use crate::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; use crate::agents::GoosePlatform; use goose_sdk_types::custom_requests::{ListRecipesRequest, ScheduleRecipeRequest}; @@ -369,7 +370,7 @@ mod tests { ("GOOSE_PATH_ROOT", root.path().to_str()), ]); let server = AcpServer::new(AcpServerFactoryConfig { - builtins: Vec::new(), + builtins: AcpBuiltinSelection::default(), data_dir: root.path().join("data"), config_dir: root.path().join("config"), goose_platform: GoosePlatform::GooseCli, diff --git a/crates/goose/src/acp/server_factory.rs b/crates/goose/src/acp/server_factory.rs index 4f33ddafc..752400b12 100644 --- a/crates/goose/src/acp/server_factory.rs +++ b/crates/goose/src/acp/server_factory.rs @@ -1,4 +1,6 @@ -use crate::acp::server::{AcpProviderFactory, GooseAcpAgent, GooseAcpAgentOptions}; +use crate::acp::server::{ + AcpBuiltinSelection, AcpProviderFactory, GooseAcpAgent, GooseAcpAgentOptions, +}; use crate::agents::GoosePlatform; use crate::scheduler_trait::SchedulerTrait; use crate::session::SessionManager; @@ -9,7 +11,7 @@ use tokio::sync::OnceCell; use tracing::info; pub struct AcpServerFactoryConfig { - pub builtins: Vec, + pub builtins: AcpBuiltinSelection, pub data_dir: std::path::PathBuf, pub config_dir: std::path::PathBuf, pub goose_platform: GoosePlatform, @@ -75,7 +77,7 @@ impl AcpServer { let agent = GooseAcpAgent::new(GooseAcpAgentOptions { provider_factory, - builtins: self.config.builtins.clone(), + builtin_selection: self.config.builtins.clone(), data_dir: self.config.data_dir.clone(), config_dir: self.config.config_dir.clone(), disable_session_naming, @@ -96,7 +98,7 @@ mod tests { fn server(data_dir: std::path::PathBuf, enable_scheduler: bool) -> AcpServer { AcpServer::new(AcpServerFactoryConfig { - builtins: Vec::new(), + builtins: AcpBuiltinSelection::default(), config_dir: data_dir.clone(), data_dir, goose_platform: GoosePlatform::GooseCli, diff --git a/crates/goose/src/config/extensions.rs b/crates/goose/src/config/extensions.rs index 94ad96220..bbf87bf22 100644 --- a/crates/goose/src/config/extensions.rs +++ b/crates/goose/src/config/extensions.rs @@ -198,6 +198,17 @@ pub fn is_extension_enabled(key: &str) -> bool { extensions.get(key).map(|e| e.enabled).unwrap_or(false) } +/// Returns the configured enabled state for an extension, or `None` when it has no entry. +pub fn configured_enabled_state(config: &Config, name: &str) -> Option { + let extensions = get_extensions_map_with_config(config); + let key = name_to_key(name); + extensions + .values() + .find(|entry| entry.config.name() == name) + .or_else(|| extensions.get(&key)) + .map(|entry| entry.enabled) +} + pub fn get_enabled_extensions() -> Vec { get_all_extensions() .into_iter() @@ -663,4 +674,39 @@ extensions: other_keys ); } + + #[test] + fn test_configured_enabled_state_unknown_extension_is_none() { + let (config, _config_file, _secrets_file) = test_config(""); + + assert_eq!( + configured_enabled_state(&config, "not_a_real_extension"), + None + ); + } + + #[test] + fn test_default_on_extension_enabled_when_config_empty() { + let (config, _config_file, _secrets_file) = test_config(""); + + assert_eq!(configured_enabled_state(&config, "developer"), Some(true)); + } + + #[test] + fn test_configured_enabled_state_tracks_changes() { + let (config, _config_file, _secrets_file) = test_config(""); + set_extension_with_config(&config, builtin_entry("developer", false)); + + assert_eq!(configured_enabled_state(&config, "developer"), Some(false)); + + set_extension_enabled_with_config(&config, "developer", true); + assert_eq!(configured_enabled_state(&config, "developer"), Some(true)); + } + + #[test] + fn test_default_off_extension_disabled_when_config_empty() { + let (config, _config_file, _secrets_file) = test_config(""); + + assert_eq!(configured_enabled_state(&config, "chatrecall"), Some(false)); + } } diff --git a/crates/goose/tests/acp_fixtures/mod.rs b/crates/goose/tests/acp_fixtures/mod.rs index 20f41638c..ade7cfef9 100644 --- a/crates/goose/tests/acp_fixtures/mod.rs +++ b/crates/goose/tests/acp_fixtures/mod.rs @@ -372,7 +372,10 @@ pub async fn spawn_acp_server_in_process( let agent = GooseAcpAgent::new(GooseAcpAgentOptions { provider_factory, - builtins: builtins.to_vec(), + builtin_selection: goose::acp::server::AcpBuiltinSelection { + explicit: builtins.to_vec(), + ..Default::default() + }, data_dir: data_root.to_path_buf(), config_dir: data_root.to_path_buf(), disable_session_naming, diff --git a/crates/goose/tests/acp_transport_auth_test.rs b/crates/goose/tests/acp_transport_auth_test.rs index 0ad1ecbb5..8acd6be9d 100644 --- a/crates/goose/tests/acp_transport_auth_test.rs +++ b/crates/goose/tests/acp_transport_auth_test.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use axum::body::Body; use axum::http::{HeaderValue, Method, Request, Response, StatusCode}; use axum::Router; +use goose::acp::server::AcpBuiltinSelection; use goose::acp::server_factory::{AcpServer, AcpServerFactoryConfig}; use goose::acp::transport::{create_acp_router, create_router}; use goose::agents::GoosePlatform; @@ -16,7 +17,7 @@ fn test_router(require_token: bool, dir: &tempfile::TempDir) -> Router { fn test_acp_router(dir: &tempfile::TempDir) -> Router { let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { - builtins: vec![], + builtins: AcpBuiltinSelection::default(), data_dir: dir.path().join("data"), config_dir: dir.path().join("config"), goose_platform: GoosePlatform::GooseCli, @@ -28,7 +29,7 @@ fn test_acp_router(dir: &tempfile::TempDir) -> Router { fn test_authenticated_acp_router(dir: &tempfile::TempDir) -> Router { let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { - builtins: vec![], + builtins: AcpBuiltinSelection::default(), data_dir: dir.path().join("data"), config_dir: dir.path().join("config"), goose_platform: GoosePlatform::GooseCli, @@ -44,7 +45,7 @@ fn test_router_with_origins( additional_allowed_origins: Vec, ) -> Router { let server = Arc::new(AcpServer::new(AcpServerFactoryConfig { - builtins: vec![], + builtins: AcpBuiltinSelection::default(), data_dir: dir.path().join("data"), config_dir: dir.path().join("config"), goose_platform: GoosePlatform::GooseCli,