diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index e88c736db0..5fc172b877 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -24,6 +24,7 @@ mod state; mod thread_config; mod tui_keymap; pub mod types; +mod unknown_enum_values; pub const CONFIG_TOML_FILE: &str = "config.toml"; @@ -126,3 +127,4 @@ pub use thread_config::ThreadConfigLoader; pub use thread_config::ThreadConfigSource; pub use thread_config::UserThreadConfig; pub use toml::Value as TomlValue; +pub use unknown_enum_values::sanitize_unknown_enum_values; diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index fc5ec79971..d17407d6af 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -1,5 +1,6 @@ use crate::config_requirements::ConfigRequirements; use crate::config_requirements::ConfigRequirementsToml; +use crate::unknown_enum_values::sanitize_unknown_enum_values; use super::fingerprint::record_origins; use super::fingerprint::version_for_toml; @@ -216,6 +217,18 @@ impl ConfigLayerStack { self.startup_warnings.as_deref() } + pub fn sanitize_unknown_enum_values(&mut self) -> Vec { + let mut warnings = Vec::new(); + for layer in &mut self.layers { + let layer_warnings = sanitize_unknown_enum_values(&mut layer.config); + if !layer_warnings.is_empty() { + layer.version = version_for_toml(&layer.config); + warnings.extend(layer_warnings); + } + } + warnings + } + /// Returns the raw user config layer, if any. /// /// This does not merge other config layers or apply any requirements. diff --git a/codex-rs/config/src/unknown_enum_values.rs b/codex-rs/config/src/unknown_enum_values.rs new file mode 100644 index 0000000000..75c6bda6bb --- /dev/null +++ b/codex-rs/config/src/unknown_enum_values.rs @@ -0,0 +1,353 @@ +use crate::config_toml::RealtimeTransport; +use crate::config_toml::RealtimeVoice; +use crate::config_toml::RealtimeWsMode; +use crate::config_toml::RealtimeWsVersion; +use crate::config_toml::ThreadStoreToml; +use crate::types::ApprovalsReviewer; +use crate::types::AuthCredentialsStoreMode; +use crate::types::HistoryPersistence; +use crate::types::OAuthCredentialsStoreMode; +use crate::types::Personality; +use crate::types::ServiceTier; +use crate::types::UriBasedFileOpener; +use crate::types::WebSearchMode; +use crate::types::WindowsSandboxModeToml; +use codex_protocol::config_types::ForcedLoginMethod; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::SandboxMode; +use codex_protocol::config_types::TrustLevel; +use codex_protocol::config_types::Verbosity; +use codex_protocol::config_types::WebSearchContextSize; +use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::protocol::AskForApproval; +use serde::Deserialize; +use toml::Value as TomlValue; + +/// Removes unrecognized string values from enum-typed config fields. +/// +/// This keeps older clients from failing to load a config written by a newer +/// client that knows about a newly added enum variant. The field is treated as +/// unset, so the normal default/resolution path applies. Non-string shape +/// errors are left intact and still fail during typed deserialization. +pub fn sanitize_unknown_enum_values(root: &mut TomlValue) -> Vec { + let mut warnings = Vec::new(); + + sanitize_enum::(root, &["approval_policy"], &mut warnings); + sanitize_enum::(root, &["approvals_reviewer"], &mut warnings); + sanitize_enum::(root, &["sandbox_mode"], &mut warnings); + sanitize_enum::(root, &["forced_login_method"], &mut warnings); + sanitize_enum::(root, &["cli_auth_credentials_store"], &mut warnings); + sanitize_enum::( + root, + &["mcp_oauth_credentials_store"], + &mut warnings, + ); + sanitize_enum::(root, &["file_opener"], &mut warnings); + sanitize_enum::(root, &["model_reasoning_effort"], &mut warnings); + sanitize_enum::(root, &["plan_mode_reasoning_effort"], &mut warnings); + sanitize_enum::(root, &["model_reasoning_summary"], &mut warnings); + sanitize_enum::(root, &["model_verbosity"], &mut warnings); + sanitize_enum::(root, &["personality"], &mut warnings); + sanitize_enum::(root, &["service_tier"], &mut warnings); + sanitize_enum::(root, &["web_search"], &mut warnings); + sanitize_enum::(root, &["history", "persistence"], &mut warnings); + sanitize_enum::(root, &["windows", "sandbox"], &mut warnings); + sanitize_enum::(root, &["realtime", "version"], &mut warnings); + sanitize_enum::(root, &["realtime", "type"], &mut warnings); + sanitize_enum::(root, &["realtime", "transport"], &mut warnings); + sanitize_enum::(root, &["realtime", "voice"], &mut warnings); + sanitize_tagged_enum::( + root, + &["experimental_thread_store"], + "type", + &mut warnings, + ); + sanitize_enum::( + root, + &["tools", "web_search", "context_size"], + &mut warnings, + ); + + sanitize_table_entries( + root, + &["profiles"], + |value, prefix, warnings| { + sanitize_enum_with_prefix::(value, prefix, &["service_tier"], warnings); + sanitize_enum_with_prefix::( + value, + prefix, + &["approval_policy"], + warnings, + ); + sanitize_enum_with_prefix::( + value, + prefix, + &["approvals_reviewer"], + warnings, + ); + sanitize_enum_with_prefix::(value, prefix, &["sandbox_mode"], warnings); + sanitize_enum_with_prefix::( + value, + prefix, + &["model_reasoning_effort"], + warnings, + ); + sanitize_enum_with_prefix::( + value, + prefix, + &["plan_mode_reasoning_effort"], + warnings, + ); + sanitize_enum_with_prefix::( + value, + prefix, + &["model_reasoning_summary"], + warnings, + ); + sanitize_enum_with_prefix::(value, prefix, &["model_verbosity"], warnings); + sanitize_enum_with_prefix::(value, prefix, &["personality"], warnings); + sanitize_enum_with_prefix::(value, prefix, &["web_search"], warnings); + sanitize_enum_with_prefix::( + value, + prefix, + &["windows", "sandbox"], + warnings, + ); + sanitize_enum_with_prefix::( + value, + prefix, + &["tools", "web_search", "context_size"], + warnings, + ); + }, + &mut warnings, + ); + + sanitize_table_entries( + root, + &["projects"], + |value, prefix, warnings| { + sanitize_enum_with_prefix::(value, prefix, &["trust_level"], warnings); + }, + &mut warnings, + ); + + warnings +} + +fn sanitize_enum(root: &mut TomlValue, path: &[&str], warnings: &mut Vec) +where + T: for<'de> Deserialize<'de>, +{ + sanitize_enum_with_prefix::(root, &[], path, warnings); +} + +fn sanitize_enum_with_prefix( + root: &mut TomlValue, + prefix: &[String], + path: &[&str], + warnings: &mut Vec, +) where + T: for<'de> Deserialize<'de>, +{ + let Some(value) = value_at_path(root, path) else { + return; + }; + let Some(raw_value) = value.as_str() else { + return; + }; + let parsed: Result = value.clone().try_into(); + if parsed.is_ok() { + return; + } + + let field_path = display_path(prefix, path); + warnings.push(format!( + "Ignoring unrecognized config value `{raw_value}` for `{field_path}`; using the default for this setting." + )); + tracing::warn!( + field = field_path, + value = raw_value, + "ignoring unrecognized config enum value" + ); + remove_value_at_path(root, path); +} + +fn sanitize_tagged_enum( + root: &mut TomlValue, + path: &[&str], + tag: &str, + warnings: &mut Vec, +) where + T: for<'de> Deserialize<'de>, +{ + let Some(value) = value_at_path(root, path) else { + return; + }; + let Some(table) = value.as_table() else { + return; + }; + let Some(raw_value) = table.get(tag).and_then(TomlValue::as_str) else { + return; + }; + let parsed: Result = value.clone().try_into(); + if parsed.is_ok() { + return; + } + + let field_path = display_path(&[], path); + warnings.push(format!( + "Ignoring unrecognized config value `{raw_value}` for `{field_path}.{tag}`; using the default for this setting." + )); + tracing::warn!( + field = format!("{field_path}.{tag}"), + value = raw_value, + "ignoring unrecognized config enum value" + ); + remove_value_at_path(root, path); +} + +fn sanitize_table_entries( + root: &mut TomlValue, + path: &[&str], + mut sanitize_entry: impl FnMut(&mut TomlValue, &[String], &mut Vec), + warnings: &mut Vec, +) { + let Some(TomlValue::Table(table)) = value_at_path_mut(root, path) else { + return; + }; + + for (key, value) in table { + let mut prefix = path + .iter() + .map(|part| (*part).to_string()) + .collect::>(); + prefix.push(key.clone()); + sanitize_entry(value, &prefix, warnings); + } +} + +fn value_at_path_mut<'a>(root: &'a mut TomlValue, path: &[&str]) -> Option<&'a mut TomlValue> { + let mut value = root; + for part in path { + value = value.as_table_mut()?.get_mut(*part)?; + } + Some(value) +} + +fn value_at_path<'a>(root: &'a TomlValue, path: &[&str]) -> Option<&'a TomlValue> { + let mut value = root; + for part in path { + value = value.as_table()?.get(*part)?; + } + Some(value) +} + +fn remove_value_at_path(root: &mut TomlValue, path: &[&str]) { + let Some((last, parent_path)) = path.split_last() else { + return; + }; + let Some(parent) = value_at_path_mut(root, parent_path).and_then(TomlValue::as_table_mut) + else { + return; + }; + parent.remove(*last); +} + +fn display_path(prefix: &[String], path: &[&str]) -> String { + prefix + .iter() + .map(String::as_str) + .chain(path.iter().copied()) + .collect::>() + .join(".") +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config_toml::ConfigToml; + use pretty_assertions::assert_eq; + + #[test] + fn unknown_config_enum_values_are_removed_with_warnings() { + let mut value: TomlValue = toml::from_str( + r#" +service_tier = "ultrafast" +sandbox_mode = "workspace-write" + +[profiles.future] +model_reasoning_effort = "huge" +model_verbosity = "high" + +[projects."/tmp/project"] +trust_level = "very-trusted" + +[tools.web_search] +context_size = "massive" +"#, + ) + .expect("config should parse as TOML"); + + let warnings = sanitize_unknown_enum_values(&mut value); + let expected: TomlValue = toml::from_str( + r#" +sandbox_mode = "workspace-write" + +[profiles.future] +model_verbosity = "high" + +[projects."/tmp/project"] + +[tools.web_search] +"#, + ) + .expect("expected TOML should parse"); + + assert_eq!( + ( + expected, + vec![ + "Ignoring unrecognized config value `ultrafast` for `service_tier`; using the default for this setting.".to_string(), + "Ignoring unrecognized config value `huge` for `profiles.future.model_reasoning_effort`; using the default for this setting.".to_string(), + "Ignoring unrecognized config value `very-trusted` for `projects./tmp/project.trust_level`; using the default for this setting.".to_string(), + "Ignoring unrecognized config value `massive` for `tools.web_search.context_size`; using the default for this setting.".to_string(), + ], + ), + (value, warnings) + ); + } + + #[test] + fn unknown_config_enum_values_allow_config_toml_deserialization() { + let mut value: TomlValue = toml::from_str( + r#" +service_tier = "ultrafast" +model_reasoning_summary = "verbose" +approval_policy = "on-request" +"#, + ) + .expect("config should parse as TOML"); + + let warnings = sanitize_unknown_enum_values(&mut value); + let config: ConfigToml = value.try_into().expect("config should deserialize"); + + assert_eq!( + ( + None, + None, + Some(AskForApproval::OnRequest), + vec![ + "Ignoring unrecognized config value `ultrafast` for `service_tier`; using the default for this setting.".to_string(), + "Ignoring unrecognized config value `verbose` for `model_reasoning_summary`; using the default for this setting.".to_string(), + ], + ), + ( + config.service_tier, + config.model_reasoning_summary, + config.approval_policy, + warnings, + ) + ); + } +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 2154bb0170..2dd1c9b427 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -35,6 +35,7 @@ use codex_config::loader::load_config_layers_state; use codex_config::loader::project_trust_key; use codex_config::profile_toml::ConfigProfile; use codex_config::sandbox_mode_requirement_for_permission_profile; +use codex_config::sanitize_unknown_enum_values; use codex_config::types::ApprovalsReviewer; use codex_config::types::AuthCredentialsStoreMode; use codex_config::types::DEFAULT_OTEL_ENVIRONMENT; @@ -951,7 +952,7 @@ impl ConfigBuilder { None => AbsolutePathBuf::current_dir()?, }; harness_overrides.cwd = Some(cwd.to_path_buf()); - let config_layer_stack = load_config_layers_state( + let mut config_layer_stack = load_config_layers_state( LOCAL_FS.as_ref(), &codex_home, Some(cwd), @@ -963,6 +964,7 @@ impl ConfigBuilder { .unwrap_or(&codex_config::NoopThreadConfigLoader), ) .await?; + let unknown_enum_warnings = config_layer_stack.sanitize_unknown_enum_values(); let merged_toml = config_layer_stack.effective_config(); // Note that each layer in ConfigLayerStack should have resolved @@ -1017,20 +1019,25 @@ impl ConfigBuilder { lock_config_layer_stack, ) .await?; + config + .startup_warnings + .splice(0..0, unknown_enum_warnings.clone()); config.config_lock_toml = Some(Arc::new(expected_lock_config)); config.config_lock_allow_codex_version_mismatch = allow_codex_version_mismatch; config.config_lock_save_fields_resolved_from_model_catalog = save_fields_resolved_from_model_catalog; return Ok(config); } - Config::load_config_with_layer_stack( + let mut config = Config::load_config_with_layer_stack( LOCAL_FS.as_ref(), config_toml, harness_overrides, codex_home, config_layer_stack, ) - .await + .await?; + config.startup_warnings.splice(0..0, unknown_enum_warnings); + Ok(config) } #[cfg(test)] @@ -1159,6 +1166,7 @@ impl Config { })?; let cli_layer = codex_config::build_cli_overrides_layer(&cli_overrides); codex_config::merge_toml_values(&mut merged, &cli_layer); + sanitize_unknown_enum_values(&mut merged); let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home)?; let config_toml = deserialize_config_toml_with_base(merged, &codex_home)?; Self::load_config_with_layer_stack( @@ -1225,6 +1233,8 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides( ) .await?; + let mut config_layer_stack = config_layer_stack; + config_layer_stack.sanitize_unknown_enum_values(); let merged_toml = config_layer_stack.effective_config(); let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| { tracing::error!("Failed to deserialize overridden config: {e}"); @@ -1241,6 +1251,8 @@ pub fn deserialize_config_toml_with_base( // This guard ensures that any relative paths that is deserialized into an // [AbsolutePathBuf] is resolved against `config_base_dir`. let _guard = AbsolutePathBufGuard::new(config_base_dir); + let mut root_value = root_value; + sanitize_unknown_enum_values(&mut root_value); root_value .try_into() .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) diff --git a/codex-rs/core/tests/suite/config_unknown_enum_values.rs b/codex-rs/core/tests/suite/config_unknown_enum_values.rs new file mode 100644 index 0000000000..2068229767 --- /dev/null +++ b/codex-rs/core/tests/suite/config_unknown_enum_values.rs @@ -0,0 +1,35 @@ +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::WarningEvent; +use core_test_support::responses::start_mock_server; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; + +const CONFIG_TOML: &str = "config.toml"; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unknown_config_enum_value_emits_startup_warning_and_uses_default() { + let server = start_mock_server().await; + let mut builder = test_codex().with_pre_build_hook(|home| { + std::fs::write(home.join(CONFIG_TOML), "service_tier = \"ultrafast\"\n") + .expect("seed config.toml"); + }); + + let test = builder.build(&server).await.expect("create conversation"); + let warning = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::Warning(WarningEvent { message }) + if message.contains("service_tier") && message.contains("ultrafast") + ) + }) + .await; + + assert_eq!(None, test.config.service_tier); + assert_eq!( + EventMsg::Warning(WarningEvent { + message: "Ignoring unrecognized config value `ultrafast` for `service_tier`; using the default for this setting.".to_string(), + }), + warning + ); +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index ad3280ebf0..4ac82d131c 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -44,6 +44,7 @@ mod collaboration_instructions; mod compact; mod compact_remote; mod compact_resume_fork; +mod config_unknown_enum_values; mod deprecation_notice; mod exec; mod exec_policy;