fix: respect disabled builtin extensions (developer) at session start (#10223)

Co-authored-by: Michael Neale <14976+michaelneale@users.noreply.github.com>
Co-authored-by: Michael Neale <mneale@squareup.com>
Co-authored-by: Lifei Zhou <lifei@squareup.com>

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",

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()

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 }

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<String>,
+    pub explicit: Vec<String>,
+}
+
 pub struct GooseAcpAgentOptions {
     pub provider_factory: AcpProviderFactory,
-    pub builtins: Vec<String>,
+    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<Mutex<HashSet<String>>>,
     agent_manager: Arc<AgentManager>,
     provider_factory: AcpProviderFactory,
-    builtins: Vec<String>,
+    builtin_selection: AcpBuiltinSelection,
     client_fs_capabilities: OnceCell<FileSystemCapabilities>,
     client_terminal: OnceCell<bool>,
     client_mcp_host_info: OnceCell<GooseMcpHostInfo>,
@@ -390,6 +396,25 @@ fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result<ExtensionConf
     }
 }
 
+fn selected_builtin_extensions(
+    config: &Config,
+    builtin_selection: &AcpBuiltinSelection,
+) -> Vec<ExtensionConfig> {
+    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<ExtensionConfig>, 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<Vec<GooseExtension>>,
         recipe_extensions: Option<&[ExtensionConfig]>,
     ) -> Result<Vec<ExtensionConfig>, 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<String>, 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")

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,

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<String>,
+    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,

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<bool> {
+    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<ExtensionConfig> {
     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));
+    }
 }

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,

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<HeaderValue>,
 ) -> 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,