From cfc23eee3df8ab73fa48fd70138bbad818ecd984 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 17 Apr 2026 18:26:09 +0100 Subject: [PATCH] feat: config aliases (#18140) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename `no_memories_if_mcp_or_web_search` → `disable_on_external_context` with backward compatibility While doing so, we add a key alias system on our layer merging system. What we try to avoid is a case where a company managed config use an old name while the user has a new name in it's local config (which would make the deserialization fail) --- codex-rs/config/src/key_aliases.rs | 52 +++++++++++ codex-rs/config/src/lib.rs | 1 + codex-rs/config/src/merge.rs | 24 +++++- codex-rs/config/src/merge_tests.rs | 100 ++++++++++++++++++++++ codex-rs/config/src/state.rs | 8 +- codex-rs/config/src/state_tests.rs | 34 ++++++++ codex-rs/config/src/types.rs | 15 ++-- codex-rs/core/config.schema.json | 8 +- codex-rs/core/src/config/config_tests.rs | 18 +++- codex-rs/core/src/mcp_tool_call.rs | 6 +- codex-rs/core/src/stream_events_utils.rs | 5 +- codex-rs/core/tests/suite/memories.rs | 4 +- codex-rs/core/tests/suite/sqlite_state.rs | 4 +- codex-rs/tui/src/app.rs | 5 +- 14 files changed, 250 insertions(+), 34 deletions(-) create mode 100644 codex-rs/config/src/key_aliases.rs create mode 100644 codex-rs/config/src/merge_tests.rs create mode 100644 codex-rs/config/src/state_tests.rs diff --git a/codex-rs/config/src/key_aliases.rs b/codex-rs/config/src/key_aliases.rs new file mode 100644 index 000000000..07cb44fa6 --- /dev/null +++ b/codex-rs/config/src/key_aliases.rs @@ -0,0 +1,52 @@ +use toml::Value as TomlValue; +use toml::map::Map as TomlMap; + +#[derive(Debug, Clone, Copy)] +struct ConfigKeyAlias { + table_path: &'static [&'static str], + legacy_key: &'static str, + canonical_key: &'static str, +} + +const CONFIG_KEY_ALIASES: &[ConfigKeyAlias] = &[ConfigKeyAlias { + table_path: &["memories"], + legacy_key: "no_memories_if_mcp_or_web_search", + canonical_key: "disable_on_external_context", +}]; + +pub(crate) fn normalize_key_aliases(path: &[String], table: &mut TomlMap) { + for alias in CONFIG_KEY_ALIASES { + if path + .iter() + .map(String::as_str) + .eq(alias.table_path.iter().copied()) + && let Some(value) = table.remove(alias.legacy_key) + { + table + .entry(alias.canonical_key.to_string()) + .or_insert(value); + } + } +} + +pub(crate) fn normalized_with_key_aliases(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)); + } + normalize_key_aliases(path, &mut normalized); + TomlValue::Table(normalized) + } + TomlValue::Array(items) => TomlValue::Array( + items + .iter() + .map(|item| normalized_with_key_aliases(item, path)) + .collect(), + ), + _ => value.clone(), + } +} diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index f86369aee..18779f980 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -4,6 +4,7 @@ pub mod config_toml; mod constraint; mod diagnostics; mod fingerprint; +mod key_aliases; mod marketplace_edit; mod mcp_edit; mod mcp_types; diff --git a/codex-rs/config/src/merge.rs b/codex-rs/config/src/merge.rs index eacbbcb78..020adb031 100644 --- a/codex-rs/config/src/merge.rs +++ b/codex-rs/config/src/merge.rs @@ -1,18 +1,34 @@ +use crate::key_aliases::normalize_key_aliases; +use crate::key_aliases::normalized_with_key_aliases; use toml::Value as TomlValue; /// 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, &mut Vec::new()); +} + +fn merge_toml_values_at_path(base: &mut TomlValue, overlay: &TomlValue, path: &mut Vec) { if let TomlValue::Table(overlay_table) = overlay && let TomlValue::Table(base_table) = base { + normalize_key_aliases(path, base_table); + let mut overlay_table = overlay_table.clone(); + normalize_key_aliases(path, &mut overlay_table); + for (key, value) in overlay_table { - if let Some(existing) = base_table.get_mut(key) { - merge_toml_values(existing, value); + path.push(key.clone()); + if let Some(existing) = base_table.get_mut(&key) { + merge_toml_values_at_path(existing, &value, path); } else { - base_table.insert(key.clone(), value.clone()); + base_table.insert(key, normalized_with_key_aliases(&value, path)); } + path.pop(); } } else { - *base = overlay.clone(); + *base = normalized_with_key_aliases(overlay, path); } } + +#[cfg(test)] +#[path = "merge_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/merge_tests.rs b/codex-rs/config/src/merge_tests.rs new file mode 100644 index 000000000..0bb92c001 --- /dev/null +++ b/codex-rs/config/src/merge_tests.rs @@ -0,0 +1,100 @@ +use super::*; +use crate::config_toml::ConfigToml; +use crate::types::MemoriesToml; +use pretty_assertions::assert_eq; + +fn parse_toml(value: &str) -> TomlValue { + toml::from_str(value).expect("TOML should parse") +} + +#[test] +fn merge_toml_values_normalizes_legacy_key_from_base_layer() { + let mut base = parse_toml( + r#" +[memories] +no_memories_if_mcp_or_web_search = false +"#, + ); + let overlay = parse_toml( + r#" +[memories] +disable_on_external_context = true +"#, + ); + + merge_toml_values(&mut base, &overlay); + + let expected = parse_toml( + r#" +[memories] +disable_on_external_context = true +"#, + ); + assert_eq!(base, expected); + + let config: ConfigToml = base.try_into().expect("merged config should deserialize"); + assert_eq!( + config.memories, + Some(MemoriesToml { + disable_on_external_context: Some(true), + ..Default::default() + }) + ); +} + +#[test] +fn merge_toml_values_normalizes_legacy_key_from_overlay_layer() { + let mut base = parse_toml( + r#" +[memories] +disable_on_external_context = false +"#, + ); + let overlay = parse_toml( + r#" +[memories] +no_memories_if_mcp_or_web_search = true +"#, + ); + + merge_toml_values(&mut base, &overlay); + + let expected = parse_toml( + r#" +[memories] +disable_on_external_context = true +"#, + ); + assert_eq!(base, expected); + + let config: ConfigToml = base.try_into().expect("merged config should deserialize"); + assert_eq!( + config.memories, + Some(MemoriesToml { + disable_on_external_context: Some(true), + ..Default::default() + }) + ); +} + +#[test] +fn merge_toml_values_prefers_canonical_key_when_one_layer_has_both_names() { + let mut base = TomlValue::Table(toml::map::Map::new()); + let overlay = parse_toml( + r#" +[memories] +disable_on_external_context = true +no_memories_if_mcp_or_web_search = false +"#, + ); + + merge_toml_values(&mut base, &overlay); + + let expected = parse_toml( + r#" +[memories] +disable_on_external_context = true +"#, + ); + assert_eq!(base, expected); +} diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index a34ccc6cd..f4f23880e 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -3,6 +3,7 @@ use crate::config_requirements::ConfigRequirementsToml; use super::fingerprint::record_origins; use super::fingerprint::version_for_toml; +use super::key_aliases::normalized_with_key_aliases; use super::merge::merge_toml_values; use codex_app_server_protocol::ConfigLayer; use codex_app_server_protocol::ConfigLayerMetadata; @@ -262,7 +263,8 @@ impl ConfigLayerStack { ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ false, ) { - record_origins(&layer.config, &layer.metadata(), &mut path, &mut origins); + let config = normalized_with_key_aliases(&layer.config, &[]); + record_origins(&config, &layer.metadata(), &mut path, &mut origins); } origins @@ -354,3 +356,7 @@ fn verify_layer_ordering(layers: &[ConfigLayerEntry]) -> std::io::Result, + /// When `true`, external context sources mark the thread `memory_mode` as `"polluted"`. + #[serde(alias = "no_memories_if_mcp_or_web_search")] + pub disable_on_external_context: Option, /// When `false`, newly created threads are stored with `memory_mode = "disabled"` in the state DB. pub generate_memories: Option, /// When `false`, skip injecting memory usage instructions into developer prompts. @@ -209,7 +210,7 @@ pub struct MemoriesToml { /// Effective memories settings after defaults are applied. #[derive(Debug, Clone, PartialEq, Eq)] pub struct MemoriesConfig { - pub no_memories_if_mcp_or_web_search: bool, + pub disable_on_external_context: bool, pub generate_memories: bool, pub use_memories: bool, pub max_raw_memories_for_consolidation: usize, @@ -224,7 +225,7 @@ pub struct MemoriesConfig { impl Default for MemoriesConfig { fn default() -> Self { Self { - no_memories_if_mcp_or_web_search: false, + disable_on_external_context: false, generate_memories: true, use_memories: true, max_raw_memories_for_consolidation: DEFAULT_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION, @@ -242,9 +243,9 @@ impl From for MemoriesConfig { fn from(toml: MemoriesToml) -> Self { let defaults = Self::default(); Self { - no_memories_if_mcp_or_web_search: toml - .no_memories_if_mcp_or_web_search - .unwrap_or(defaults.no_memories_if_mcp_or_web_search), + disable_on_external_context: toml + .disable_on_external_context + .unwrap_or(defaults.disable_on_external_context), generate_memories: toml.generate_memories.unwrap_or(defaults.generate_memories), use_memories: toml.use_memories.unwrap_or(defaults.use_memories), max_raw_memories_for_consolidation: toml diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 2eb8cf727..f0fbe8994 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -863,6 +863,10 @@ "description": "Model used for memory consolidation.", "type": "string" }, + "disable_on_external_context": { + "description": "When `true`, external context sources mark the thread `memory_mode` as `\"polluted\"`.", + "type": "boolean" + }, "extract_model": { "description": "Model used for thread summarisation.", "type": "string" @@ -900,10 +904,6 @@ "format": "int64", "type": "integer" }, - "no_memories_if_mcp_or_web_search": { - "description": "When `true`, web searches and MCP tool calls mark the thread `memory_mode` as `\"polluted\"`.", - "type": "boolean" - }, "use_memories": { "description": "When `false`, skip injecting memory usage instructions into developer prompts.", "type": "boolean" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index c40d49fb8..6e721b123 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -221,7 +221,7 @@ persistence = "none" let memories = r#" [memories] -no_memories_if_mcp_or_web_search = true +disable_on_external_context = true generate_memories = false use_memories = false max_raw_memories_for_consolidation = 512 @@ -236,7 +236,7 @@ consolidation_model = "gpt-5" toml::from_str::(memories).expect("TOML deserialization should succeed"); assert_eq!( Some(MemoriesToml { - no_memories_if_mcp_or_web_search: Some(true), + disable_on_external_context: Some(true), generate_memories: Some(false), use_memories: Some(false), max_raw_memories_for_consolidation: Some(512), @@ -260,7 +260,7 @@ consolidation_model = "gpt-5" assert_eq!( config.memories, MemoriesConfig { - no_memories_if_mcp_or_web_search: true, + disable_on_external_context: true, generate_memories: false, use_memories: false, max_raw_memories_for_consolidation: 512, @@ -272,6 +272,18 @@ consolidation_model = "gpt-5" consolidation_model: Some("gpt-5".to_string()), } ); + + let legacy_memories_cfg = + toml::from_str::("[memories]\nno_memories_if_mcp_or_web_search = true\n") + .expect("legacy memories TOML should deserialize"); + assert!( + MemoriesConfig::from( + legacy_memories_cfg + .memories + .expect("legacy memories config") + ) + .disable_on_external_context + ); } #[test] diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 87e574910..32c7cf879 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -541,11 +541,7 @@ async fn augment_mcp_tool_request_meta_with_sandbox_state( } async fn maybe_mark_thread_memory_mode_polluted(sess: &Session, turn_context: &TurnContext) { - if !turn_context - .config - .memories - .no_memories_if_mcp_or_web_search - { + if !turn_context.config.memories.disable_on_external_context { return; } state_db::mark_thread_memory_mode_polluted( diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index c6cc79bc4..a3534fcc3 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -154,10 +154,7 @@ pub(crate) async fn mark_thread_memory_mode_polluted_if_external_context( turn_context: &TurnContext, item: &ResponseItem, ) { - if !turn_context - .config - .memories - .no_memories_if_mcp_or_web_search + if !turn_context.config.memories.disable_on_external_context || !response_item_may_include_external_context(item) { return; diff --git a/codex-rs/core/tests/suite/memories.rs b/codex-rs/core/tests/suite/memories.rs index 60685592f..554efe63c 100644 --- a/codex-rs/core/tests/suite/memories.rs +++ b/codex-rs/core/tests/suite/memories.rs @@ -314,7 +314,7 @@ async fn web_search_pollution_moves_selected_thread_into_removed_phase2_inputs() .enable(Feature::MemoryTool) .expect("test config should allow feature update"); config.memories.max_raw_memories_for_consolidation = 1; - config.memories.no_memories_if_mcp_or_web_search = true; + config.memories.disable_on_external_context = true; }); let initial = initial_builder.build(&server).await?; mount_sse_once( @@ -386,7 +386,7 @@ async fn web_search_pollution_moves_selected_thread_into_removed_phase2_inputs() .enable(Feature::MemoryTool) .expect("test config should allow feature update"); config.memories.max_raw_memories_for_consolidation = 1; - config.memories.no_memories_if_mcp_or_web_search = true; + config.memories.disable_on_external_context = true; }); let resumed = resumed_builder .resume(&server, home.clone(), rollout_path.clone()) diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 271f7bfc9..e7125210b 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -297,7 +297,7 @@ async fn web_search_marks_thread_memory_mode_polluted_when_configured() -> Resul .features .enable(Feature::Sqlite) .expect("test config should allow feature update"); - config.memories.no_memories_if_mcp_or_web_search = true; + config.memories.disable_on_external_context = true; }); let test = builder.build(&server).await?; let db = test.codex.state_db().expect("state db enabled"); @@ -355,7 +355,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< .features .enable(Feature::Sqlite) .expect("test config should allow feature update"); - config.memories.no_memories_if_mcp_or_web_search = true; + config.memories.disable_on_external_context = true; let mut servers = config.mcp_servers.get().clone(); servers.insert( diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4e2b61929..13438f8d5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -8274,8 +8274,9 @@ mod tests { Some(&TomlValue::Boolean(false)) ); assert!( - !memories.contains_key("no_memories_if_mcp_or_web_search"), - "the TUI menu should not write the MCP pollution setting" + !memories.contains_key("disable_on_external_context") + && !memories.contains_key("no_memories_if_mcp_or_web_search"), + "the TUI menu should not write the external-context memory setting" ); app_server.shutdown().await?; Ok(())