refactor: use Config::global() instead of per-instance config in SACP server

crates/goose/src/acp/server_factory.rs

@@ -21,11 +21,7 @@ impl AcpServer {
     }
 
     pub async fn create_agent(&self) -> Result<Arc<GooseAcpAgent>> {
-        let config_path = self
-            .config
-            .config_dir
-            .join(crate::config::base::CONFIG_YAML_NAME);
-        let config = crate::config::Config::new(&config_path, "goose")?;
+        let config = crate::config::Config::for_config_dir(self.config.config_dir.clone())?;
 
         let goose_mode = config
             .get_goose_mode()

crates/goose/src/config/base.rs

@@ -2,7 +2,7 @@ use crate::config::paths::Paths;
 use crate::config::GooseMode;
 use fs2::FileExt;
 use keyring::Entry;
-use once_cell::sync::OnceCell;
+use once_cell::sync::{Lazy, OnceCell};
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use serde_yaml::Mapping;
@@ -130,13 +130,38 @@ pub struct Config {
     secrets_cache: Arc<Mutex<Option<HashMap<String, Value>>>>,
 }
 
+#[derive(Clone)]
+pub enum ConfigHandle {
+    Global,
+    Cached(Arc<Config>),
+}
+
+impl AsRef<Config> for ConfigHandle {
+    fn as_ref(&self) -> &Config {
+        match self {
+            Self::Global => Config::global(),
+            Self::Cached(config) => config.as_ref(),
+        }
+    }
+}
+
+impl std::ops::Deref for ConfigHandle {
+    type Target = Config;
+
+    fn deref(&self) -> &Self::Target {
+        self.as_ref()
+    }
+}
+
 enum SecretStorage {
     Keyring { service: String },
     File { path: PathBuf },
 }
 
 // Global instance
 static GLOBAL_CONFIG: OnceCell<Config> = OnceCell::new();
+static CONFIG_CACHE: Lazy<Mutex<HashMap<PathBuf, Arc<Config>>>> =
+    Lazy::new(|| Mutex::new(HashMap::new()));
 
 fn system_config_path() -> PathBuf {
     #[cfg(unix)]
@@ -163,43 +188,7 @@ fn bundled_defaults_path() -> Option<PathBuf> {
 
 impl Default for Config {
     fn default() -> Self {
-        let config_dir = Paths::config_dir();
-        let user_config_path = config_dir.join(CONFIG_YAML_NAME);
-
-        let mut config_paths = vec![system_config_path()];
-        if let Some(defaults) = bundled_defaults_path() {
-            config_paths.insert(0, defaults);
-        }
-        config_paths.push(user_config_path.clone());
-
-        let no_secrets_config = Self {
-            config_paths: config_paths.clone(),
-            secrets: SecretStorage::File {
-                path: Default::default(),
-            },
-            guard: Mutex::new(()),
-            secrets_cache: Arc::new(Mutex::new(None)),
-        };
-
-        let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
-            || no_secrets_config
-                .get_param::<serde_yaml::Value>("GOOSE_DISABLE_KEYRING")
-                .is_ok_and(|v| keyring_disabled_value(&v))
-        {
-            SecretStorage::File {
-                path: config_dir.join("secrets.yaml"),
-            }
-        } else {
-            SecretStorage::Keyring {
-                service: KEYRING_SERVICE.to_string(),
-            }
-        };
-        Self {
-            config_paths,
-            secrets,
-            guard: Mutex::new(()),
-            secrets_cache: Arc::new(Mutex::new(None)),
-        }
+        Self::with_config_dir(Paths::config_dir())
     }
 }
 
@@ -343,6 +332,45 @@ fn keyring_disabled_in_config(config_path: &Path) -> bool {
         .unwrap_or(false)
 }
 
+fn normalize_writable_config_path(path: &Path) -> Result<PathBuf, ConfigError> {
+    if path.exists() {
+        return Ok(path.canonicalize()?);
+    }
+
+    if let Some(parent) = path
+        .parent()
+        .filter(|parent| !parent.as_os_str().is_empty())
+    {
+        if parent.exists() {
+            let file_name = path
+                .file_name()
+                .map(PathBuf::from)
+                .unwrap_or_else(|| PathBuf::from(CONFIG_YAML_NAME));
+            return Ok(parent.canonicalize()?.join(file_name));
+        }
+    }
+
+    let absolute = if path.is_absolute() {
+        path.to_path_buf()
+    } else {
+        env::current_dir()?.join(path)
+    };
+
+    let mut normalized = PathBuf::new();
+    for component in absolute.components() {
+        match component {
+            std::path::Component::CurDir => {}
+            std::path::Component::ParentDir => {
+                normalized.pop();
+            }
+            std::path::Component::Normal(_)
+            | std::path::Component::RootDir
+            | std::path::Component::Prefix(_) => normalized.push(component.as_os_str()),
+        }
+    }
+    Ok(normalized)
+}
+
 impl Config {
     /// Get the global configuration instance.
     ///
@@ -352,6 +380,69 @@ impl Config {
         GLOBAL_CONFIG.get_or_init(Config::default)
     }
 
+    pub fn for_config_dir(config_dir: PathBuf) -> Result<ConfigHandle, ConfigError> {
+        let config_path = config_dir.join(CONFIG_YAML_NAME);
+        let cache_key = normalize_writable_config_path(&config_path)?;
+        let default_key =
+            normalize_writable_config_path(&Paths::config_dir().join(CONFIG_YAML_NAME))?;
+
+        if cache_key == default_key {
+            return Ok(ConfigHandle::Global);
+        }
+
+        let mut cache = CONFIG_CACHE.lock().unwrap();
+        if let Some(config) = cache.get(&cache_key) {
+            return Ok(ConfigHandle::Cached(Arc::clone(config)));
+        }
+
+        let config = Arc::new(Self::new(config_path, KEYRING_SERVICE)?);
+        cache.insert(cache_key, Arc::clone(&config));
+        Ok(ConfigHandle::Cached(config))
+    }
+
+    fn config_paths_for_dir(config_dir: &Path) -> Vec<PathBuf> {
+        let mut config_paths = vec![system_config_path()];
+        if let Some(defaults) = bundled_defaults_path() {
+            config_paths.insert(0, defaults);
+        }
+        config_paths.push(config_dir.join(CONFIG_YAML_NAME));
+        config_paths
+    }
+
+    fn with_config_dir(config_dir: PathBuf) -> Self {
+        let config_paths = Self::config_paths_for_dir(&config_dir);
+
+        let no_secrets_config = Self {
+            config_paths: config_paths.clone(),
+            secrets: SecretStorage::File {
+                path: Default::default(),
+            },
+            guard: Mutex::new(()),
+            secrets_cache: Arc::new(Mutex::new(None)),
+        };
+
+        let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
+            || no_secrets_config
+                .get_param::<serde_yaml::Value>("GOOSE_DISABLE_KEYRING")
+                .is_ok_and(|v| keyring_disabled_value(&v))
+        {
+            SecretStorage::File {
+                path: config_dir.join("secrets.yaml"),
+            }
+        } else {
+            SecretStorage::Keyring {
+                service: KEYRING_SERVICE.to_string(),
+            }
+        };
+
+        Self {
+            config_paths,
+            secrets,
+            guard: Mutex::new(()),
+            secrets_cache: Arc::new(Mutex::new(None)),
+        }
+    }
+
     /// Create a new configuration instance with custom paths
     ///
     /// This is primarily useful for testing or for applications that need

crates/goose/src/config/mod.rs

@@ -12,7 +12,7 @@ pub mod signup_openrouter;
 pub mod signup_tetrate;
 
 pub use crate::agents::ExtensionConfig;
-pub use base::{merge_config_values, Config, ConfigError};
+pub use base::{merge_config_values, Config, ConfigError, ConfigHandle};
 pub use declarative_providers::DeclarativeProviderConfig;
 pub use experiments::ExperimentManager;
 pub use extensions::{

crates/goose/tests/acp_common_tests/mod.rs

@@ -948,18 +948,19 @@ pub async fn run_permission_persistence<C: Connection>() {
     };
 
     let mut conn = C::new(config, openai).await;
+    let permission_path = conn.permission_config_path();
     let SessionData { mut session, .. } = conn.new_session().await.unwrap();
     expected_session_id.set(&session.session_id().0);
 
     for (decision, expected_status, expected_yaml) in cases {
         conn.reset_openai();
         conn.reset_permissions();
-        let _ = fs::remove_file(temp_dir.path().join("permission.yaml"));
+        let _ = fs::remove_file(&permission_path);
         let output = session.prompt(prompt, decision).await.unwrap();
 
         assert_eq!(output.tool_status.unwrap(), expected_status);
         assert_eq!(
-            fs::read_to_string(temp_dir.path().join("permission.yaml")).unwrap_or_default(),
+            fs::read_to_string(&permission_path).unwrap_or_default(),
             expected_yaml,
         );
     }

crates/goose/tests/acp_custom_requests_test.rs

@@ -15,6 +15,7 @@ use goose_test_support::{EnforceSessionId, IgnoreSessionId};
 use std::sync::{Arc, Mutex};
 
 use common_tests::fixtures::OpenAiFixture;
+use goose::config::Config;
 
 struct MockProvider {
     name: String,
@@ -142,8 +143,16 @@ fn test_custom_provider_inventory_includes_metadata() {
 #[test]
 fn test_custom_config_crud() {
     run_test(async {
+        let temp_dir = tempfile::tempdir().unwrap();
         let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
-        let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
+        let conn = AcpServerConnection::new(
+            TestConnectionConfig {
+                data_root: temp_dir.path().to_path_buf(),
+                ..Default::default()
+            },
+            openai,
+        )
+        .await;
 
         send_custom(
             conn.cx(),
@@ -156,6 +165,12 @@ fn test_custom_config_crud() {
         .await
         .expect("config upsert should succeed");
 
+        let cached_config = Config::for_config_dir(temp_dir.path().to_path_buf()).unwrap();
+        assert_eq!(
+            cached_config.get_param::<String>("GOOSE_PROVIDER").unwrap(),
+            "anthropic"
+        );
+
         let response = send_custom(
             conn.cx(),
             "_goose/config/read",
@@ -190,6 +205,44 @@ fn test_custom_config_crud() {
     });
 }
 
+#[test]
+fn test_concurrent_custom_config_upserts_preserve_all_keys() {
+    run_test(async {
+        let temp_dir = tempfile::tempdir().unwrap();
+        let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
+        let conn = AcpServerConnection::new(
+            TestConnectionConfig {
+                data_root: temp_dir.path().to_path_buf(),
+                ..Default::default()
+            },
+            openai,
+        )
+        .await;
+
+        let requests = (0..24).map(|i| {
+            let key = format!("CONCURRENT_CONFIG_KEY_{i}");
+            send_custom(
+                conn.cx(),
+                "_goose/config/upsert",
+                serde_json::json!({
+                    "key": key,
+                    "value": i,
+                }),
+            )
+        });
+
+        for result in futures::future::join_all(requests).await {
+            result.expect("config upsert should succeed");
+        }
+
+        let cached_config = Config::for_config_dir(temp_dir.path().to_path_buf()).unwrap();
+        for i in 0..24 {
+            let key = format!("CONCURRENT_CONFIG_KEY_{i}");
+            assert_eq!(cached_config.get_param::<i64>(&key).unwrap(), i);
+        }
+    });
+}
+
 #[test]
 fn test_provider_switching_updates_session_state() {
     run_test(async {