diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 69e92a8557..2d9d305cf9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -386,11 +386,6 @@ pub struct ConfigBuilder { loader_overrides: Option, } -#[derive(Debug, Clone, PartialEq, Eq)] -struct ConfigWarning { - pub message: String, -} - impl ConfigBuilder { pub fn codex_home(mut self, codex_home: PathBuf) -> Self { self.codex_home = Some(codex_home); @@ -413,11 +408,6 @@ impl ConfigBuilder { } pub async fn build(self) -> std::io::Result { - let (config, _) = self.build_with_warnings().await?; - Ok(config) - } - - async fn build_with_warnings(self) -> std::io::Result<(Config, Vec)> { let Self { codex_home, cli_overrides, @@ -436,7 +426,7 @@ impl ConfigBuilder { load_config_layers_state(&codex_home, Some(cwd), &cli_overrides, loader_overrides) .await?; let merged_toml = config_layer_stack.effective_config(); - let (merged_toml, warnings) = sanitize_config_toml(merged_toml); + let merged_toml = sanitize_config_toml(merged_toml); // Note that each layer in ConfigLayerStack should have resolved // relative paths to absolute paths based on the parent folder of the @@ -445,80 +435,44 @@ impl ConfigBuilder { let config_toml: ConfigToml = merged_toml .try_into() .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - let config = Config::load_config_with_layer_stack( + Config::load_config_with_layer_stack( config_toml, harness_overrides, codex_home, config_layer_stack, - )?; - Ok((config, warnings)) + ) } } -fn sanitize_config_toml(mut merged_toml: TomlValue) -> (TomlValue, Vec) { - let mut warnings = Vec::new(); - let allowed = [ - ReasoningEffort::None, - ReasoningEffort::Minimal, - ReasoningEffort::Low, - ReasoningEffort::Medium, - ReasoningEffort::High, - ReasoningEffort::XHigh, - ] - .iter() - .map(ReasoningEffort::to_string) - .collect::>(); - +fn sanitize_config_toml(mut merged_toml: TomlValue) -> TomlValue { if let TomlValue::Table(table) = &mut merged_toml { - sanitize_reasoning_effort_table(table, "model_reasoning_effort", &allowed, &mut warnings); + sanitize_reasoning_effort_table(table); if let Some(TomlValue::Table(profiles)) = table.get_mut("profiles") { - for (profile_name, profile_value) in profiles.iter_mut() { + for (_profile_name, profile_value) in profiles.iter_mut() { if let TomlValue::Table(profile_table) = profile_value { - let path = format!("profiles.{profile_name}.model_reasoning_effort"); - sanitize_reasoning_effort_table(profile_table, &path, &allowed, &mut warnings); + sanitize_reasoning_effort_table(profile_table); } } } } - (merged_toml, warnings) + merged_toml } -fn sanitize_reasoning_effort_table( - table: &mut toml::map::Map, - path: &str, - allowed: &[String], - warnings: &mut Vec, -) { +fn sanitize_reasoning_effort_table(table: &mut toml::map::Map) { let Some(value) = table.get("model_reasoning_effort") else { return; }; - let raw = match value.as_str() { - Some(raw) => raw.to_string(), - None => value.to_string(), - }; - let normalized = raw.trim().to_ascii_lowercase(); - - if allowed.iter().any(|effort| effort == &normalized) { - if normalized != raw { - table.insert( - "model_reasoning_effort".to_string(), - TomlValue::String(normalized), - ); - } + let Some(raw) = value.as_str() else { + table.remove("model_reasoning_effort"); return; + }; + + if serde_json::from_str::(&format!("\"{raw}\"")).is_err() { + table.remove("model_reasoning_effort"); } - - table.remove("model_reasoning_effort"); - - let allowed_text = allowed.join(", "); - warnings.push(ConfigWarning { - message: format!( - "Invalid config value for {path}: \"{raw}\". Allowed values: {allowed_text}. Falling back to default." - ), - }); } impl Config { @@ -568,7 +522,7 @@ pub async fn load_config_as_toml_with_cli_overrides( .await?; let merged_toml = config_layer_stack.effective_config(); - let (merged_toml, _) = sanitize_config_toml(merged_toml); + let merged_toml = sanitize_config_toml(merged_toml); let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| { tracing::error!("Failed to deserialize overridden config: {e}"); e @@ -1720,7 +1674,7 @@ persistence = "none" } #[tokio::test] - async fn invalid_reasoning_effort_emits_warning_and_uses_default() -> anyhow::Result<()> { + async fn invalid_reasoning_effort_is_ignored() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); @@ -1732,15 +1686,12 @@ model_reasoning_effort = "super-high" ) .await?; - let (config, warnings) = ConfigBuilder::default() + let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) - .build_with_warnings() + .build() .await?; assert_eq!(config.model_reasoning_effort, None); - assert_eq!(warnings.len(), 1); - assert!(warnings[0].message.contains("model_reasoning_effort")); - assert!(warnings[0].message.contains("Allowed values")); Ok(()) }