mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Move config key alias normalization ahead of merging (#47903)
## Why Sparse config edits must preserve aliases in unrelated base tables so the returned config version remains consistent with the file on disk. ## What changed Normalize the full overlay before merging and normalize base aliases only in tables reached by the overlay. Reuse the root-level normalization helper when reporting config origins. ## Testing Add a regression test showing that an `agents` overlay normalizes `max_threads` while leaving the unrelated `memories` alias untouched. GitOrigin-RevId: 25cc95b0969979c49c43b766824fa0bd019465a4
This commit is contained in:
@@ -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<String, TomlValue>) {
|
||||
fn normalize_table_key_aliases(path: &[String], table: &mut TomlMap<String, TomlValue>) {
|
||||
for alias in CONFIG_KEY_ALIASES {
|
||||
if path
|
||||
.iter()
|
||||
@@ -36,24 +38,61 @@ pub(crate) fn normalize_key_aliases(path: &[String], table: &mut TomlMap<String,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn normalized_with_key_aliases(value: &TomlValue, path: &[String]) -> 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<String>,
|
||||
) {
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<S: AsRef<str>>(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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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",
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user