diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index d74927c62e..70d634f9bd 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -111,14 +111,10 @@ impl ConfigLayerSource { ConfigLayerSource::Mdm { .. } => 0, ConfigLayerSource::System { .. } => 10, ConfigLayerSource::SystemOverride { .. } => 11, - ConfigLayerSource::User { profile, .. } => { - if profile.is_some() { - 21 - } else { - 20 - } - } - ConfigLayerSource::UserOverride { .. } => 20, + ConfigLayerSource::User { + profile: Some(_), .. + } => 21, + ConfigLayerSource::User { .. } | ConfigLayerSource::UserOverride { .. } => 20, ConfigLayerSource::Project { .. } | ConfigLayerSource::ProjectOverride { .. } => 25, ConfigLayerSource::SessionFlags => 30, ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => 40, diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 5347730b14..84cd3a35c2 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -1207,6 +1207,28 @@ struct LoadedProjectLayers { startup_warnings: Vec, } +#[derive(Clone, Copy)] +enum ProjectConfigFileKind { + Base, + Override, +} + +impl ProjectConfigFileKind { + fn file_name(self) -> &'static str { + match self { + Self::Base => CONFIG_TOML_FILE, + Self::Override => CONFIG_OVERRIDE_TOML_FILE, + } + } + + fn parse_error_label(self) -> &'static str { + match self { + Self::Base => "project config file", + Self::Override => "project override config file", + } + } +} + /// Return the appropriate list of layers (each with /// [ConfigLayerSource::Project] as the source) between `cwd` and /// `project_root`, inclusive. The list is ordered in _increasing_ precdence, @@ -1260,126 +1282,75 @@ async fn load_project_layers( if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized { continue; } - let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); + let config_file = dot_codex_abs.join(ProjectConfigFileKind::Base.file_name()); match fs.read_file_text(&config_file, /*sandbox*/ None).await { Ok(contents) => { - let config: TomlValue = match toml::from_str(&contents) { - Ok(config) => config, - Err(e) => { - if decision.is_trusted() { - let config_file_display = config_file.as_path().display(); - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Error parsing project config file {config_file_display}: {e}" - ), - )); - } - layers.push(project_layer_entry( - &dot_codex_abs, - TomlValue::Table(toml::map::Map::new()), - disabled_reason.clone(), - hooks_config_folder_override.clone(), - )); - continue; - } - }; - let config = prepare_project_layer_config( + let entry = load_project_layer_from_contents( fs, + ProjectConfigFileKind::Base, &config_file, - CONFIG_TOML_FILE, &contents, - config, &dot_codex_abs, - hooks_config_folder_override.as_ref(), - decision.is_trusted(), - strict_config, - disabled_reason.is_none(), - &mut startup_warnings, - ) - .await?; - let entry = project_layer_entry( - &dot_codex_abs, - config, disabled_reason.clone(), hooks_config_folder_override.clone(), - ); - layers.push(entry); - } - Err(err) => { - if err.kind() == io::ErrorKind::NotFound { - // If there is no config.toml file, record an empty entry - // for this project layer, as this may still have subfolders - // that are significant in the overall ConfigLayerStack. - let config = merge_root_checkout_project_hooks( - fs, - TomlValue::Table(toml::map::Map::new()), - hooks_config_folder_override.as_ref(), - decision.is_trusted(), - CONFIG_TOML_FILE, - ) - .await?; - layers.push(project_layer_entry( - &dot_codex_abs, - config, - disabled_reason.clone(), - hooks_config_folder_override.clone(), - )); - } else { - let config_file_display = config_file.as_path().display(); - return Err(io::Error::new( - err.kind(), - format!("Failed to read project config file {config_file_display}: {err}"), - )); - } - } - } - - let override_file = dot_codex_abs.join(CONFIG_OVERRIDE_TOML_FILE); - match fs.read_file_text(&override_file, /*sandbox*/ None).await { - Ok(contents) => { - let config: TomlValue = match toml::from_str(&contents) { - Ok(config) => config, - Err(err) => { - if decision.is_trusted() { - let override_file_display = override_file.as_path().display(); - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Error parsing project override config file {override_file_display}: {err}" - ), - )); - } - layers.push(project_override_layer_entry( - &dot_codex_abs, - TomlValue::Table(toml::map::Map::new()), - disabled_reason.clone(), - hooks_config_folder_override.clone(), - )); - continue; - } - }; - let config = prepare_project_layer_config( - fs, - &override_file, - CONFIG_OVERRIDE_TOML_FILE, - &contents, - config, - &dot_codex_abs, - hooks_config_folder_override.as_ref(), decision.is_trusted(), strict_config, disabled_reason.is_none(), &mut startup_warnings, ) .await?; - layers.push(project_override_layer_entry( + layers.push(entry); + } + Err(err) if err.kind() == io::ErrorKind::NotFound => { + // If there is no config.toml file, record an empty entry + // for this project layer, as this may still have subfolders + // that are significant in the overall ConfigLayerStack. + let config = merge_root_checkout_project_hooks( + fs, + TomlValue::Table(toml::map::Map::new()), + hooks_config_folder_override.as_ref(), + decision.is_trusted(), + ProjectConfigFileKind::Base.file_name(), + ) + .await?; + layers.push(project_layer_entry( &dot_codex_abs, config, disabled_reason.clone(), hooks_config_folder_override.clone(), )); } + Err(err) => { + let config_file_display = config_file.as_path().display(); + return Err(io::Error::new( + err.kind(), + format!( + "Failed to read {} {config_file_display}: {err}", + ProjectConfigFileKind::Base.parse_error_label() + ), + )); + } + } + + let override_file = dot_codex_abs.join(ProjectConfigFileKind::Override.file_name()); + match fs.read_file_text(&override_file, /*sandbox*/ None).await { + Ok(contents) => { + let entry = load_project_layer_from_contents( + fs, + ProjectConfigFileKind::Override, + &override_file, + &contents, + &dot_codex_abs, + disabled_reason.clone(), + hooks_config_folder_override.clone(), + decision.is_trusted(), + strict_config, + disabled_reason.is_none(), + &mut startup_warnings, + ) + .await?; + layers.push(entry); + } Err(err) if err.kind() == io::ErrorKind::NotFound => { if hooks_config_folder_override.is_some() { let config = merge_root_checkout_project_hooks( @@ -1387,7 +1358,7 @@ async fn load_project_layers( TomlValue::Table(toml::map::Map::new()), hooks_config_folder_override.as_ref(), decision.is_trusted(), - CONFIG_OVERRIDE_TOML_FILE, + ProjectConfigFileKind::Override.file_name(), ) .await?; layers.push(project_override_layer_entry( @@ -1403,7 +1374,8 @@ async fn load_project_layers( return Err(io::Error::new( err.kind(), format!( - "Failed to read project override config file {override_file_display}: {err}" + "Failed to read {} {override_file_display}: {err}", + ProjectConfigFileKind::Override.parse_error_label() ), )); } @@ -1416,6 +1388,88 @@ async fn load_project_layers( }) } +#[allow(clippy::too_many_arguments)] +async fn load_project_layer_from_contents( + fs: &dyn ExecutorFileSystem, + kind: ProjectConfigFileKind, + config_file: &AbsolutePathBuf, + contents: &str, + dot_codex_abs: &AbsolutePathBuf, + disabled_reason: Option, + hooks_config_folder_override: Option, + is_trusted: bool, + strict_config: bool, + is_enabled: bool, + startup_warnings: &mut Vec, +) -> io::Result { + let config: TomlValue = match toml::from_str(contents) { + Ok(config) => config, + Err(err) => { + if is_trusted { + let config_file_display = config_file.as_path().display(); + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "Error parsing {} {config_file_display}: {err}", + kind.parse_error_label() + ), + )); + } + return Ok(project_layer_entry_for_kind( + kind, + dot_codex_abs, + TomlValue::Table(toml::map::Map::new()), + disabled_reason, + hooks_config_folder_override, + )); + } + }; + let config = prepare_project_layer_config( + fs, + config_file, + kind.file_name(), + contents, + config, + dot_codex_abs, + hooks_config_folder_override.as_ref(), + is_trusted, + strict_config, + is_enabled, + startup_warnings, + ) + .await?; + Ok(project_layer_entry_for_kind( + kind, + dot_codex_abs, + config, + disabled_reason, + hooks_config_folder_override, + )) +} + +fn project_layer_entry_for_kind( + kind: ProjectConfigFileKind, + dot_codex_abs: &AbsolutePathBuf, + config: TomlValue, + disabled_reason: Option, + hooks_config_folder_override: Option, +) -> ConfigLayerEntry { + match kind { + ProjectConfigFileKind::Base => project_layer_entry( + dot_codex_abs, + config, + disabled_reason, + hooks_config_folder_override, + ), + ProjectConfigFileKind::Override => project_override_layer_entry( + dot_codex_abs, + config, + disabled_reason, + hooks_config_folder_override, + ), + } +} + #[allow(clippy::too_many_arguments)] async fn prepare_project_layer_config( fs: &dyn ExecutorFileSystem, diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index e8371300a3..0fe3c23417 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -409,19 +409,18 @@ impl ConfigLayerStack { /// Returns a new stack with the user layer copied from `other`, preserving /// every non-user layer already present in this stack. pub fn with_user_layer_from(&self, other: &Self) -> Self { - let user_layers = other - .layers - .iter() - .filter(|layer| is_user_config_layer(&layer.name)) - .cloned() - .collect::>(); let mut layers = self .layers .iter() .filter(|layer| !is_user_config_layer(&layer.name)) .cloned() .collect::>(); - for user_layer in user_layers { + for user_layer in other + .layers + .iter() + .filter(|layer| is_user_config_layer(&layer.name)) + .cloned() + { insert_user_layer(&mut layers, user_layer); } self.with_layers(layers) @@ -526,31 +525,29 @@ fn is_user_config_layer(layer: &ConfigLayerSource) -> bool { fn insert_user_layer(layers: &mut Vec, user_layer: ConfigLayerEntry) { let index = layers.iter().position(|layer| { + let sibling_override_should_insert_before = match (&user_layer.name, &layer.name) { + ( + ConfigLayerSource::User { + file, + profile: None, + }, + ConfigLayerSource::UserOverride { + file: override_file, + }, + ) => user_base_file_has_sibling_override(file, override_file), + ( + ConfigLayerSource::UserOverride { + file: override_file, + }, + ConfigLayerSource::User { + file, + profile: None, + }, + ) => !user_base_file_has_sibling_override(file, override_file), + _ => false, + }; layer.name.precedence() > user_layer.name.precedence() - || matches!( - (&user_layer.name, &layer.name), - ( - ConfigLayerSource::User { - file, - profile: None, - }, - ConfigLayerSource::UserOverride { - file: override_file, - } - ) if user_base_file_has_sibling_override(file, override_file) - ) - || matches!( - (&user_layer.name, &layer.name), - ( - ConfigLayerSource::UserOverride { - file: override_file, - }, - ConfigLayerSource::User { - file, - profile: None, - } - ) if !user_base_file_has_sibling_override(file, override_file) - ) + || sibling_override_should_insert_before }); match index { Some(index) => layers.insert(index, user_layer), @@ -569,9 +566,9 @@ fn user_base_file_has_sibling_override( } fn active_user_layer_index(layers: &[ConfigLayerEntry]) -> Option { - layers.iter().enumerate().rev().find_map(|(index, layer)| { - matches!(layer.name, ConfigLayerSource::User { .. }).then_some(index) - }) + layers + .iter() + .rposition(|layer| matches!(layer.name, ConfigLayerSource::User { .. })) } /// Ensures precedence ordering of config layers is correct. Returns the index diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 8c3ffc687a..f7e2c66ad2 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -593,10 +593,9 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result Vec> render_mdm_layer_details(layer) } ConfigLayerSource::System { .. } + | ConfigLayerSource::SystemOverride { .. } | ConfigLayerSource::User { .. } + | ConfigLayerSource::UserOverride { .. } | ConfigLayerSource::Project { .. } + | ConfigLayerSource::ProjectOverride { .. } | ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => Vec::new(), } } @@ -385,15 +388,27 @@ fn format_config_layer_source(source: &ConfigLayerSource) -> String { ConfigLayerSource::System { file } => { format!("system ({})", file.as_path().display()) } + ConfigLayerSource::SystemOverride { file } => { + format!("system override ({})", file.as_path().display()) + } ConfigLayerSource::User { file, .. } => { format!("user ({})", file.as_path().display()) } + ConfigLayerSource::UserOverride { file } => { + format!("user override ({})", file.as_path().display()) + } ConfigLayerSource::Project { dot_codex_folder } => { format!( "project ({}/config.toml)", dot_codex_folder.as_path().display() ) } + ConfigLayerSource::ProjectOverride { dot_codex_folder } => { + format!( + "project override ({}/config.override.toml)", + dot_codex_folder.as_path().display() + ) + } ConfigLayerSource::SessionFlags => "session-flags".to_string(), ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => { format!("legacy managed_config.toml ({})", file.as_path().display())