fix: lazy provider creation for goose acp (#7026)

crates/goose-acp/src/bin/server.rs

@@ -1,5 +1,6 @@
 use anyhow::Result;
 use clap::Parser;
+use goose::config::paths::Paths;
 use goose_acp::server_factory::{AcpServer, AcpServerFactoryConfig};
 use std::net::SocketAddr;
 use std::sync::Arc;
@@ -36,12 +37,11 @@ async fn main() -> Result<()> {
         cli.builtins
     };
 
-    let config = AcpServerFactoryConfig {
+    let server = Arc::new(AcpServer::new(AcpServerFactoryConfig {
         builtins,
-        ..Default::default()
-    };
-
-    let server = Arc::new(AcpServer::new(config));
+        data_dir: Paths::data_dir(),
+        config_dir: Paths::config_dir(),
+    }));
     let router = goose_acp::transport::create_router(server);
 
     let addr: SocketAddr = format!("{}:{}", cli.host, cli.port).parse()?;

crates/goose-acp/src/server.rs

@@ -13,12 +13,12 @@ use goose::conversation::Conversation;
 use goose::mcp_utils::ToolResult;
 use goose::permission::permission_confirmation::PrincipalType;
 use goose::permission::{Permission, PermissionConfirmation};
-use goose::providers::create;
+use goose::providers::provider_registry::ProviderConstructor;
 use goose::session::session_manager::SessionType;
 use goose::session::{Session, SessionManager};
 use rmcp::model::{CallToolResult, RawContent, ResourceContents, Role};
 use sacp::schema::{
-    AgentCapabilities, AuthenticateRequest, AuthenticateResponse, BlobResourceContents,
+    AgentCapabilities, AuthMethod, AuthenticateRequest, AuthenticateResponse, BlobResourceContents,
     CancelNotification, Content, ContentBlock, ContentChunk, EmbeddedResource,
     EmbeddedResourceResource, ImageContent, InitializeRequest, InitializeResponse,
     LoadSessionRequest, LoadSessionResponse, McpCapabilities, McpServer, NewSessionRequest,
@@ -46,16 +46,9 @@ struct GooseAcpSession {
 pub struct GooseAcpAgent {
     sessions: Arc<Mutex<HashMap<String, GooseAcpSession>>>,
     agent: Arc<Agent>,
-    provider: Arc<dyn goose::providers::base::Provider>,
-}
-
-pub struct AcpServerConfig {
-    pub provider: Arc<dyn goose::providers::base::Provider>,
-    pub builtins: Vec<String>,
-    pub data_dir: std::path::PathBuf,
-    pub config_dir: std::path::PathBuf,
-    pub goose_mode: goose::config::GooseMode,
-    pub disable_session_naming: bool,
+    provider_factory: ProviderConstructor,
+    config_dir: std::path::PathBuf,
+    provider_initialized: tokio::sync::OnceCell<String>,
 }
 
 fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result<ExtensionConfig, String> {
@@ -278,54 +271,23 @@ impl GooseAcpAgent {
         Arc::clone(&self.agent.config.permission_manager)
     }
 
-    pub async fn new(builtins: Vec<String>) -> Result<Self> {
-        let config = Config::global();
-
-        let provider_name: String = config
-            .get_goose_provider()
-            .map_err(|e| anyhow::anyhow!("No provider configured: {}", e))?;
-
-        let model_name: String = config
-            .get_goose_model()
-            .map_err(|e| anyhow::anyhow!("No model configured: {}", e))?;
-
-        let model_config = goose::model::ModelConfig {
-            model_name: model_name.clone(),
-            context_limit: None,
-            temperature: None,
-            max_tokens: None,
-            toolshim: false,
-            toolshim_model: None,
-            fast_model: None,
-            request_params: None,
-        };
-        let provider = create(&provider_name, model_config).await?;
-        let goose_mode = config
-            .get_goose_mode()
-            .unwrap_or(goose::config::GooseMode::Auto);
-
-        Self::with_config(AcpServerConfig {
-            provider,
-            builtins,
-            data_dir: Paths::data_dir(),
-            config_dir: Paths::config_dir(),
-            goose_mode,
-            disable_session_naming: config.get_goose_disable_session_naming().unwrap_or(false),
-        })
-        .await
-    }
-
-    pub async fn with_config(config: AcpServerConfig) -> Result<Self> {
-        let session_manager = Arc::new(SessionManager::new(config.data_dir));
-        let config_dir = config.config_dir.clone();
-        let permission_manager = Arc::new(PermissionManager::new(config.config_dir));
+    pub async fn new(
+        provider_factory: ProviderConstructor,
+        builtins: Vec<String>,
+        data_dir: std::path::PathBuf,
+        config_dir: std::path::PathBuf,
+        goose_mode: goose::config::GooseMode,
+        disable_session_naming: bool,
+    ) -> Result<Self> {
+        let session_manager = Arc::new(SessionManager::new(data_dir));
+        let permission_manager = Arc::new(PermissionManager::new(config_dir.clone()));
 
         let agent = Agent::with_config(AgentConfig::new(
             Arc::clone(&session_manager),
             permission_manager,
             None,
-            config.goose_mode,
-            config.disable_session_naming,
+            goose_mode,
+            disable_session_naming,
         ));
 
         let agent_ptr = Arc::new(agent);
@@ -334,13 +296,15 @@ impl GooseAcpAgent {
         let config_file = Config::new(&config_path, "goose")?;
         let extensions = get_enabled_extensions_with_config(&config_file);
 
-        add_builtins(&agent_ptr, config.builtins).await;
+        add_builtins(&agent_ptr, builtins).await;
         add_extensions(&agent_ptr, extensions).await;
 
         Ok(Self {
-            provider: config.provider.clone(),
             sessions: Arc::new(Mutex::new(HashMap::new())),
             agent: agent_ptr,
+            provider_factory,
+            config_dir,
+            provider_initialized: tokio::sync::OnceCell::new(),
         })
     }
 
@@ -354,9 +318,7 @@ impl GooseAcpAgent {
             )
             .await?;
 
-        self.agent
-            .update_provider(self.provider.clone(), &goose_session.id)
-            .await?;
+        self.ensure_provider(&goose_session).await?;
 
         let session = GooseAcpSession {
             messages: Conversation::new_unvalidated(Vec::new()),
@@ -692,7 +654,15 @@ impl GooseAcpAgent {
                     .embedded_context(true),
             )
             .mcp_capabilities(McpCapabilities::new().http(true));
-        Ok(InitializeResponse::new(args.protocol_version).agent_capabilities(capabilities))
+        Ok(InitializeResponse::new(args.protocol_version)
+            .agent_capabilities(capabilities)
+            .auth_methods(vec![AuthMethod::new(
+                "goose-provider",
+                "Configure Provider",
+            )
+            .description(
+                "Run `goose configure` to set up your AI provider and API key",
+            )]))
     }
 
     async fn on_new_session(
@@ -712,7 +682,9 @@ impl GooseAcpAgent {
             .map_err(|e| {
                 sacp::Error::internal_error().data(format!("Failed to create session: {}", e))
             })?;
-        self.update_session_with_provider(&goose_session).await?;
+        self.ensure_provider(&goose_session).await.map_err(|e| {
+            sacp::Error::internal_error().data(format!("Failed to set provider: {}", e))
+        })?;
 
         for mcp_server in args.mcp_servers {
             let config = match mcp_server_to_extension_config(mcp_server) {
@@ -746,16 +718,21 @@ impl GooseAcpAgent {
         Ok(NewSessionResponse::new(SessionId::new(goose_session.id)))
     }
 
-    async fn update_session_with_provider(
-        &self,
-        goose_session: &Session,
-    ) -> Result<(), sacp::Error> {
-        self.agent
-            .update_provider(self.provider.clone(), &goose_session.id)
-            .await
-            .map_err(|e| {
-                sacp::Error::internal_error().data(format!("Failed to set provider: {}", e))
-            })?;
+    // Called at most once via OnceCell; returns the model_id used.
+    async fn create_provider(&self, session: &Session) -> Result<String> {
+        let config_path = self.config_dir.join(CONFIG_YAML_NAME);
+        let config = Config::new(&config_path, "goose")?;
+        let model_id = config.get_goose_model()?;
+        let model_config = goose::model::ModelConfig::new(&model_id)?;
+        let provider = (self.provider_factory)(model_config).await?;
+        self.agent.update_provider(provider, &session.id).await?;
+        Ok(model_id)
+    }
+
+    async fn ensure_provider(&self, session: &Session) -> Result<()> {
+        self.provider_initialized
+            .get_or_try_init(|| self.create_provider(session))
+            .await?;
         Ok(())
     }
 
@@ -773,7 +750,9 @@ impl GooseAcpAgent {
             sacp::Error::invalid_params()
                 .data(format!("Failed to load session {}: {}", session_id, e))
         })?;
-        self.update_session_with_provider(&goose_session).await?;
+        self.ensure_provider(&goose_session).await.map_err(|e| {
+            sacp::Error::internal_error().data(format!("Failed to set provider: {}", e))
+        })?;
 
         let conversation = goose_session.conversation.ok_or_else(|| {
             sacp::Error::internal_error()
@@ -1045,7 +1024,13 @@ pub async fn run(builtins: Vec<String>) -> Result<()> {
     let outgoing = tokio::io::stdout().compat_write();
     let incoming = tokio::io::stdin().compat();
 
-    let agent = Arc::new(GooseAcpAgent::new(builtins).await?);
+    let server =
+        crate::server_factory::AcpServer::new(crate::server_factory::AcpServerFactoryConfig {
+            builtins,
+            data_dir: Paths::data_dir(),
+            config_dir: Paths::config_dir(),
+        });
+    let agent = server.create_agent().await?;
     serve(agent, incoming, outgoing).await
 }
 

crates/goose-acp/src/server_factory.rs

@@ -1,29 +1,16 @@
 use anyhow::Result;
-use goose::config::paths::Paths;
-use goose::config::Config;
-use goose::model::ModelConfig;
-use goose::providers::create;
+use goose::providers::provider_registry::ProviderConstructor;
 use std::sync::Arc;
 use tracing::info;
 
-use crate::server::{AcpServerConfig, GooseAcpAgent};
+use crate::server::GooseAcpAgent;
 
 pub struct AcpServerFactoryConfig {
     pub builtins: Vec<String>,
     pub data_dir: std::path::PathBuf,
     pub config_dir: std::path::PathBuf,
 }
 
-impl Default for AcpServerFactoryConfig {
-    fn default() -> Self {
-        Self {
-            builtins: vec!["developer".to_string()],
-            data_dir: Paths::data_dir(),
-            config_dir: Paths::config_dir(),
-        }
-    }
-}
-
 pub struct AcpServer {
     config: AcpServerFactoryConfig,
 }
@@ -34,44 +21,39 @@ impl AcpServer {
     }
 
     pub async fn create_agent(&self) -> Result<Arc<GooseAcpAgent>> {
-        let global_config = Config::global();
-
-        let provider_name: String = global_config
-            .get_goose_provider()
-            .map_err(|e| anyhow::anyhow!("No provider configured: {}", e))?;
+        let config_path = self
+            .config
+            .config_dir
+            .join(goose::config::base::CONFIG_YAML_NAME);
+        let config = goose::config::Config::new(&config_path, "goose")?;
 
-        let model_name: String = global_config
-            .get_goose_model()
-            .map_err(|e| anyhow::anyhow!("No model configured: {}", e))?;
-
-        let model_config = ModelConfig {
-            request_params: None,
-            model_name: model_name.clone(),
-            context_limit: None,
-            temperature: None,
-            max_tokens: None,
-            toolshim: false,
-            toolshim_model: None,
-            fast_model: None,
-        };
-
-        let provider = create(&provider_name, model_config).await?;
-        let goose_mode = global_config
+        let goose_mode = config
             .get_goose_mode()
             .unwrap_or(goose::config::GooseMode::Auto);
-
-        let acp_config = AcpServerConfig {
-            provider,
-            builtins: self.config.builtins.clone(),
-            data_dir: self.config.data_dir.clone(),
-            config_dir: self.config.config_dir.clone(),
+        let disable_session_naming = config.get_goose_disable_session_naming().unwrap_or(false);
+
+        let config_dir = self.config.config_dir.clone();
+        let provider_factory: ProviderConstructor = Arc::new(move |model_config| {
+            let config_dir = config_dir.clone();
+            Box::pin(async move {
+                let config_path = config_dir.join(goose::config::base::CONFIG_YAML_NAME);
+                let config = goose::config::Config::new(&config_path, "goose")?;
+                let provider_name = config
+                    .get_goose_provider()
+                    .map_err(|_| anyhow::anyhow!("No provider configured"))?;
+                goose::providers::create(&provider_name, model_config).await
+            })
+        });
+
+        let agent = GooseAcpAgent::new(
+            provider_factory,
+            self.config.builtins.clone(),
+            self.config.data_dir.clone(),
+            self.config.config_dir.clone(),
             goose_mode,
-            disable_session_naming: global_config
-                .get_goose_disable_session_naming()
-                .unwrap_or(false),
-        };
-
-        let agent = GooseAcpAgent::with_config(acp_config).await?;
+            disable_session_naming,
+        )
+        .await?;
         info!("Created new ACP agent");
 
         Ok(Arc::new(agent))

crates/goose-acp/tests/common_tests/mod.rs

@@ -189,7 +189,7 @@ pub async fn run_configured_extension<S: Session>() {
     let mcp = McpFixture::new(expected_session_id.clone()).await;
 
     let config_yaml = format!(
-        "extensions:\n  lookup:\n    enabled: true\n    type: streamable_http\n    name: lookup\n    description: Lookup server\n    uri: \"{}\"\n",
+        "GOOSE_MODEL: gpt-5-nano\nextensions:\n  lookup:\n    enabled: true\n    type: streamable_http\n    name: lookup\n    description: Lookup server\n    uri: \"{}\"\n",
         mcp.url
     );
     fs::write(temp_dir.path().join(CONFIG_YAML_NAME), config_yaml).unwrap();