diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 7a360cee2..85ce73ab3 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -227,6 +227,8 @@ pub enum ConfigLayerSource { #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] System { + /// This is the path to the system config.toml file, though it is not + /// guaranteed to exist. file: AbsolutePathBuf, }, diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 805601d59..c0be58f50 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -18,6 +18,7 @@ use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SandboxMode; use codex_app_server_protocol::ToolsV2; use codex_app_server_protocol::WriteStatus; +use codex_core::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::json; @@ -73,8 +74,7 @@ sandbox_mode = "workspace-write" } ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 1); - assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); + assert_layers_user_then_optional_system(&layers, user_file)?; Ok(()) } @@ -136,8 +136,7 @@ view_image = false ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 1); - assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); + assert_layers_user_then_optional_system(&layers, user_file)?; Ok(()) } @@ -257,12 +256,7 @@ writable_roots = [{}] ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 2); - assert_eq!( - layers[0].name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } - ); - assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + assert_layers_managed_user_then_optional_system(&layers, managed_file, user_file)?; Ok(()) } @@ -433,3 +427,50 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { Ok(()) } + +fn assert_layers_user_then_optional_system( + layers: &[codex_app_server_protocol::ConfigLayer], + user_file: AbsolutePathBuf, +) -> Result<()> { + if cfg!(unix) { + let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?; + assert_eq!(layers.len(), 2); + assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); + assert_eq!( + layers[1].name, + ConfigLayerSource::System { file: system_file } + ); + } else { + assert_eq!(layers.len(), 1); + assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); + } + Ok(()) +} + +fn assert_layers_managed_user_then_optional_system( + layers: &[codex_app_server_protocol::ConfigLayer], + managed_file: AbsolutePathBuf, + user_file: AbsolutePathBuf, +) -> Result<()> { + if cfg!(unix) { + let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?; + assert_eq!(layers.len(), 3); + assert_eq!( + layers[0].name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } + ); + assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + assert_eq!( + layers[2].name, + ConfigLayerSource::System { file: system_file } + ); + } else { + assert_eq!(layers.len(), 2); + assert_eq!( + layers[0].name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } + ); + assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + } + Ok(()) +} diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index b82809568..bc6d96bcb 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -778,15 +778,41 @@ remote_compaction = true }, ); let layers = response.layers.expect("layers present"); - assert_eq!(layers.len(), 2, "expected two layers"); - assert_eq!( - layers.first().unwrap().name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } - ); - assert_eq!( - layers.get(1).unwrap().name, - ConfigLayerSource::User { file: user_file } - ); + if cfg!(unix) { + let system_file = AbsolutePathBuf::from_absolute_path( + crate::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX, + ) + .expect("system file"); + assert_eq!(layers.len(), 3, "expected three layers on unix"); + assert_eq!( + layers.first().unwrap().name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + } + ); + assert_eq!( + layers.get(1).unwrap().name, + ConfigLayerSource::User { + file: user_file.clone() + } + ); + assert_eq!( + layers.get(2).unwrap().name, + ConfigLayerSource::System { file: system_file } + ); + } else { + assert_eq!(layers.len(), 2, "expected two layers"); + assert_eq!( + layers.first().unwrap().name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + } + ); + assert_eq!( + layers.get(1).unwrap().name, + ConfigLayerSource::User { file: user_file } + ); + } } #[tokio::test] diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index be336bd05..73624c83c 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -33,6 +33,12 @@ pub use state::LoaderOverrides; /// On Unix systems, load requirements from this file path, if present. const DEFAULT_REQUIREMENTS_TOML_FILE_UNIX: &str = "/etc/codex/requirements.toml"; + +/// On Unix systems, load default settings from this file path, if present. +/// Note that /etc/codex/ is treated as a "config folder," so subfolders such +/// as skills/ and rules/ will also be honored. +pub const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml"; + const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; /// To build up the set of admin-enforced constraints, we build up from multiple @@ -72,7 +78,7 @@ pub async fn load_config_layers_state( ) -> io::Result { let mut config_requirements_toml = ConfigRequirementsToml::default(); - // TODO(mbolin): Support an entry in MDM for config requirements and use it + // TODO(gt): Support an entry in MDM for config requirements and use it // with `config_requirements_toml.merge_unset_fields(...)`, if present. // Honor /etc/codex/requirements.toml. @@ -95,57 +101,45 @@ pub async fn load_config_layers_state( let mut layers = Vec::::new(); - // TODO(mbolin): Honor managed preferences (macOS only). - // TODO(mbolin): Honor /etc/codex/config.toml. + // TODO(gt): Honor managed preferences (macOS only). + + // Include an entry for the "system" config folder, loading its config.toml, + // if it exists. + let system_config_toml_file = if cfg!(unix) { + Some(AbsolutePathBuf::from_absolute_path( + SYSTEM_CONFIG_TOML_FILE_UNIX, + )?) + } else { + // TODO(gt): Determine the path to load on Windows. + None + }; + if let Some(system_config_toml_file) = system_config_toml_file { + let system_layer = + load_config_toml_for_required_layer(&system_config_toml_file, |config_toml| { + ConfigLayerEntry::new( + ConfigLayerSource::System { + file: system_config_toml_file.clone(), + }, + config_toml, + ) + }) + .await?; + layers.push(system_layer); + } // Add a layer for $CODEX_HOME/config.toml if it exists. Note if the file // exists, but is malformed, then this error should be propagated to the // user. let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home)?; - let user_layer = match tokio::fs::read_to_string(&user_file).await { - Ok(contents) => { - let user_config: TomlValue = toml::from_str(&contents).map_err(|e| { - io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Error parsing user config file {}: {e}", - user_file.as_path().display(), - ), - ) - })?; - let user_config_parent = user_file.as_path().parent().ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidData, - format!( - "User config file {} has no parent directory", - user_file.as_path().display() - ), - ) - })?; - let user_config = - resolve_relative_paths_in_config_toml(user_config, user_config_parent)?; - ConfigLayerEntry::new(ConfigLayerSource::User { file: user_file }, user_config) - } - Err(e) => { - if e.kind() == io::ErrorKind::NotFound { - // If there is no config.toml file, record an empty entry - // for this user layer, as this may still have subfolders - // that are significant in the overall ConfigLayerStack. - ConfigLayerEntry::new( - ConfigLayerSource::User { file: user_file }, - TomlValue::Table(toml::map::Map::new()), - ) - } else { - return Err(io::Error::new( - e.kind(), - format!( - "Failed to read user config file {}: {e}", - user_file.as_path().display(), - ), - )); - } - } - }; + let user_layer = load_config_toml_for_required_layer(&user_file, |config_toml| { + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: user_file.clone(), + }, + config_toml, + ) + }) + .await?; layers.push(user_layer); if let Some(cwd) = cwd { @@ -206,6 +200,52 @@ pub async fn load_config_layers_state( ConfigLayerStack::new(layers, config_requirements_toml.try_into()?) } +/// Attempts to load a config.toml file from `config_toml`. +/// - If the file exists and is valid TOML, passes the parsed `toml::Value` to +/// `create_entry` and returns the resulting layer entry. +/// - If the file does not exist, uses an empty `Table` with `create_entry` and +/// returns the resulting layer entry. +/// - If there is an error reading the file or parsing the TOML, returns an +/// error. +async fn load_config_toml_for_required_layer( + config_toml: impl AsRef, + create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, +) -> io::Result { + let toml_file = config_toml.as_ref(); + let toml_value = match tokio::fs::read_to_string(toml_file).await { + Ok(contents) => { + let config: TomlValue = toml::from_str(&contents).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Error parsing config file {}: {e}", toml_file.display()), + ) + })?; + let config_parent = toml_file.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!( + "Config file {} has no parent directory", + toml_file.display() + ), + ) + })?; + resolve_relative_paths_in_config_toml(config, config_parent) + } + Err(e) => { + if e.kind() == io::ErrorKind::NotFound { + Ok(TomlValue::Table(toml::map::Map::new())) + } else { + Err(io::Error::new( + e.kind(), + format!("Failed to read config file {}: {e}", toml_file.display()), + )) + } + } + }?; + + Ok(create_entry(toml_value)) +} + /// If available, apply requirements from `/etc/codex/requirements.toml` to /// `config_requirements_toml` by filling in any unset fields. async fn load_requirements_toml( diff --git a/codex-rs/core/src/config_loader/state.rs b/codex-rs/core/src/config_loader/state.rs index bd00e7724..efb33dfac 100644 --- a/codex-rs/core/src/config_loader/state.rs +++ b/codex-rs/core/src/config_loader/state.rs @@ -55,7 +55,7 @@ impl ConfigLayerEntry { pub fn config_folder(&self) -> Option { match &self.name { ConfigLayerSource::Mdm { .. } => None, - ConfigLayerSource::System { .. } => None, + ConfigLayerSource::System { file } => file.parent(), ConfigLayerSource::User { file } => file.parent(), ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()), ConfigLayerSource::SessionFlags => None, diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 4efeeaa98..bb8898129 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -119,9 +119,10 @@ async fn returns_empty_when_all_layers_missing() { .iter() .filter(|layer| matches!(layer.name, super::ConfigLayerSource::System { .. })) .count(); + let expected_system_layers = if cfg!(unix) { 1 } else { 0 }; assert_eq!( - num_system_layers, 0, - "managed config layer should be absent when file missing" + num_system_layers, expected_system_layers, + "system layer should be present only on unix" ); #[cfg(not(target_os = "macos"))]