diff --git a/codex-rs/config/src/key_aliases.rs b/codex-rs/config/src/key_aliases.rs index c0f2dea391..f2967f87b6 100644 --- a/codex-rs/config/src/key_aliases.rs +++ b/codex-rs/config/src/key_aliases.rs @@ -1,3 +1,5 @@ +//! Normalize config aliases before merging layers and reporting origins. + use toml::Value as TomlValue; use toml::map::Map as TomlMap; @@ -21,7 +23,7 @@ const CONFIG_KEY_ALIASES: &[ConfigKeyAlias] = &[ }, ]; -pub(crate) fn normalize_key_aliases(path: &[String], table: &mut TomlMap) { +fn normalize_table_key_aliases(path: &[String], table: &mut TomlMap) { for alias in CONFIG_KEY_ALIASES { if path .iter() @@ -36,24 +38,61 @@ pub(crate) fn normalize_key_aliases(path: &[String], table: &mut TomlMap TomlValue { +pub(crate) fn normalize_key_aliases(value: &TomlValue) -> TomlValue { + normalize_key_aliases_at_path(value, &[]) +} + +fn normalize_key_aliases_at_path(value: &TomlValue, path: &[String]) -> TomlValue { match value { TomlValue::Table(table) => { let mut normalized = TomlMap::new(); for (key, child) in table { let mut child_path = path.to_vec(); child_path.push(key.clone()); - normalized.insert(key.clone(), normalized_with_key_aliases(child, &child_path)); + normalized.insert( + key.clone(), + normalize_key_aliases_at_path(child, &child_path), + ); } - normalize_key_aliases(path, &mut normalized); + normalize_table_key_aliases(path, &mut normalized); TomlValue::Table(normalized) } TomlValue::Array(items) => TomlValue::Array( items .iter() - .map(|item| normalized_with_key_aliases(item, path)) + .map(|item| normalize_key_aliases_at_path(item, path)) .collect(), ), _ => value.clone(), } } + +/// Normalize only base tables reached by the overlay, leaving unrelated raw config intact. +/// +/// Config-edit table Upserts start from the raw user layer, not the normalized effective config. +/// They use a sparse overlay and persist only the edited subtree, but compute the returned +/// version from the whole in-memory config. Normalizing unrelated base tables would make +/// that version disagree with the file on disk. +/// +/// For example, given `[memories] no_memories_if_mcp_or_web_search = false` and +/// `[agents] max_depth = 1`, upserting `agents` with `{ "max_depth": 2 }` must leave +/// the memories alias untouched. Renaming it only in memory would cause the next write +/// using the returned version to fail with `ConfigVersionConflict`, despite no intervening edit. +pub(crate) fn normalize_base_key_aliases( + base: &mut TomlValue, + overlay: &TomlValue, + path: &mut Vec, +) { + if let TomlValue::Table(base) = base + && let TomlValue::Table(overlay) = overlay + { + normalize_table_key_aliases(path, base); + for (key, value) in overlay { + if let Some(existing) = base.get_mut(key) { + path.push(key.clone()); + normalize_base_key_aliases(existing, value, path); + path.pop(); + } + } + } +} diff --git a/codex-rs/config/src/merge.rs b/codex-rs/config/src/merge.rs index 0ad63aecc8..ef67c26f72 100644 --- a/codex-rs/config/src/merge.rs +++ b/codex-rs/config/src/merge.rs @@ -1,5 +1,5 @@ +use crate::key_aliases::normalize_base_key_aliases; use crate::key_aliases::normalize_key_aliases; -use crate::key_aliases::normalized_with_key_aliases; use codex_network_proxy::normalize_host; use toml::Value as TomlValue; @@ -55,7 +55,9 @@ impl ShellEnvironmentPolicyFilterRepresentation { /// Merge config `overlay` into `base`, giving `overlay` precedence. pub fn merge_toml_values(base: &mut TomlValue, overlay: &TomlValue) { - merge_toml_values_at_path(base, overlay.clone(), &mut Vec::new()); + let overlay = normalize_key_aliases(overlay); + normalize_base_key_aliases(base, &overlay, &mut Vec::new()); + merge_toml_values_at_path(base, overlay, &mut Vec::new()); } pub fn is_structured_feature_path>(path: &[S]) -> bool { @@ -94,8 +96,6 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: TomlValue, path: &mu if let TomlValue::Table(base_table) = &mut *base && let TomlValue::Table(mut overlay_table) = overlay { - normalize_key_aliases(path, base_table); - normalize_key_aliases(path, &mut overlay_table); if is_network_domains_path(path) { normalize_network_domain_keys(base_table); normalize_network_domain_keys(&mut overlay_table); @@ -141,12 +141,12 @@ fn merge_toml_values_at_path(base: &mut TomlValue, overlay: TomlValue, path: &mu if let Some(existing) = base_table.get_mut(&key) { merge_toml_values_at_path(existing, value, path); } else { - base_table.insert(key, normalized_with_key_aliases(&value, path)); + base_table.insert(key, value); } path.pop(); } } else { - *base = normalized_with_key_aliases(&overlay, path); + *base = overlay; } } diff --git a/codex-rs/config/src/merge_tests.rs b/codex-rs/config/src/merge_tests.rs index 1c16565c49..2c09762751 100644 --- a/codex-rs/config/src/merge_tests.rs +++ b/codex-rs/config/src/merge_tests.rs @@ -729,3 +729,20 @@ exclude = [] assert_eq!(base, overlay); } } + +#[test] +fn sparse_overlay_preserves_unvisited_base_aliases() { + let mut base = parse_toml( + "[memories]\nno_memories_if_mcp_or_web_search = false\n[agents]\nmax_threads = 2", + ); + let overlay = parse_toml("[agents]\nmax_depth = 3"); + + merge_toml_values(&mut base, &overlay); + + assert_eq!( + base, + parse_toml( + "[memories]\nno_memories_if_mcp_or_web_search = false\n[agents]\nmax_concurrent_threads_per_session = 2\nmax_depth = 3", + ), + ); +} diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index 35b9f783c5..ec271fd820 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -5,7 +5,7 @@ use crate::format_config_layer_source; use super::fingerprint::record_origins; use super::fingerprint::version_for_toml; -use super::key_aliases::normalized_with_key_aliases; +use super::key_aliases::normalize_key_aliases; use super::merge::merge_toml_values; use crate::CloudConfigBundleLoader; use crate::ConfigLayer; @@ -506,7 +506,7 @@ impl ConfigLayerStack { let mut provider_paths = vec!["features.network_proxy.credentials.".to_string()]; for layer in self.layers_low_to_high() { - let config = normalized_with_key_aliases(&layer.config, &[]); + let config = normalize_key_aliases(&layer.config); if let Some(profiles) = config.get("profiles").and_then(TomlValue::as_table) { provider_paths.extend( profiles