diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index d9a4bba30f..6406f9e1ff 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -5,7 +5,6 @@ use std::collections::HashMap; use std::path::Path; use crate::HooksToml; -use crate::Lenient; use crate::permissions_toml::PermissionsToml; use crate::profile_toml::ConfigProfile; use crate::types::AnalyticsConfigToml; @@ -107,12 +106,12 @@ pub struct ConfigToml { pub model_auto_compact_token_limit: Option, /// Default approval policy for executing commands. - pub approval_policy: Option>, + pub approval_policy: Option, /// Configures who approval requests are routed to for review once they have /// been escalated. This does not disable separate safety checks such as /// ARC. - pub approvals_reviewer: Option>, + pub approvals_reviewer: Option, /// Optional policy instructions for the guardian auto-reviewer. #[serde(default)] @@ -133,7 +132,7 @@ pub struct ConfigToml { pub allow_login_shell: Option, /// Sandbox mode to use. - pub sandbox_mode: Option>, + pub sandbox_mode: Option, /// Sandbox configuration to apply if `sandbox` is `WorkspaceWrite`. pub sandbox_workspace_write: Option, @@ -187,14 +186,14 @@ pub struct ConfigToml { /// When set, restricts the login mechanism users may use. #[serde(default)] - pub forced_login_method: Option>, + pub forced_login_method: Option, /// Preferred backend for storing CLI auth credentials. /// file (default): Use a file in the Codex home directory. /// keyring: Use an OS-specific keyring service. /// auto: Use the keyring if available, otherwise use a file. #[serde(default)] - pub cli_auth_credentials_store: Option>, + pub cli_auth_credentials_store: Option, /// Definition for MCP servers that Codex can reach out to for tool calls. #[serde(default)] @@ -208,7 +207,7 @@ pub struct ConfigToml { /// file: Use a file in the Codex home directory. /// auto (default): Use the OS-specific keyring service if available, otherwise use a file. #[serde(default)] - pub mcp_oauth_credentials_store: Option>, + pub mcp_oauth_credentials_store: Option, /// Optional fixed port for the local HTTP callback server used during MCP OAuth login. /// When unset, Codex will bind to an ephemeral port chosen by the OS. @@ -275,7 +274,7 @@ pub struct ConfigToml { /// Optional URI-based file opener. If set, citations to files in the model /// output will be hyperlinked using the specified URI scheme. - pub file_opener: Option>, + pub file_opener: Option, /// Collection of settings that are specific to the TUI. pub tui: Option, @@ -289,11 +288,11 @@ pub struct ConfigToml { /// Defaults to `false`. pub show_raw_agent_reasoning: Option, - pub model_reasoning_effort: Option>, - pub plan_mode_reasoning_effort: Option>, - pub model_reasoning_summary: Option>, + pub model_reasoning_effort: Option, + pub plan_mode_reasoning_effort: Option, + pub model_reasoning_summary: Option, /// Optional verbosity control for GPT-5 models (Responses API `text.verbosity`). - pub model_verbosity: Option>, + pub model_verbosity: Option, /// Override to force-enable reasoning summaries for the configured model. pub model_supports_reasoning_summaries: Option, @@ -303,10 +302,10 @@ pub struct ConfigToml { pub model_catalog_json: Option, /// Optionally specify a personality for the model - pub personality: Option>, + pub personality: Option, /// Optional explicit service tier preference for new turns (`fast` or `flex`). - pub service_tier: Option>, + pub service_tier: Option, /// Base URL for requests to ChatGPT (as opposed to the OpenAI API). pub chatgpt_base_url: Option, @@ -352,11 +351,11 @@ pub struct ConfigToml { pub experimental_thread_config_endpoint: Option, /// Experimental / do not use. Selects the thread store implementation. - pub experimental_thread_store: Option>, + pub experimental_thread_store: Option, pub projects: Option>, /// Controls the web search tool mode: disabled, cached, or live. - pub web_search: Option>, + pub web_search: Option, /// Nested tools section for feature toggles pub tools: Option, @@ -510,21 +509,15 @@ impl From for UserSavedConfig { .collect(); Self { - approval_policy: config_toml.approval_policy.and_then(Lenient::into_valid), - sandbox_mode: config_toml.sandbox_mode.and_then(Lenient::into_valid), + approval_policy: config_toml.approval_policy, + sandbox_mode: config_toml.sandbox_mode, sandbox_settings: config_toml.sandbox_workspace_write.map(From::from), forced_chatgpt_workspace_id: config_toml.forced_chatgpt_workspace_id, - forced_login_method: config_toml - .forced_login_method - .and_then(Lenient::into_valid), + forced_login_method: config_toml.forced_login_method, model: config_toml.model, - model_reasoning_effort: config_toml - .model_reasoning_effort - .and_then(Lenient::into_valid), - model_reasoning_summary: config_toml - .model_reasoning_summary - .and_then(Lenient::into_valid), - model_verbosity: config_toml.model_verbosity.and_then(Lenient::into_valid), + model_reasoning_effort: config_toml.model_reasoning_effort, + model_reasoning_summary: config_toml.model_reasoning_summary, + model_verbosity: config_toml.model_verbosity, tools: config_toml.tools.map(From::from), profile: config_toml.profile, profiles, @@ -718,17 +711,16 @@ impl ConfigToml { &self, sandbox_mode_override: Option, profile_sandbox_mode: Option, - config_sandbox_mode: Option, windows_sandbox_level: WindowsSandboxLevel, active_project: Option<&ProjectConfig>, permission_profile_constraint: Option<&crate::Constrained>, ) -> PermissionProfile { let sandbox_mode_was_explicit = sandbox_mode_override.is_some() || profile_sandbox_mode.is_some() - || config_sandbox_mode.is_some(); + || self.sandbox_mode.is_some(); let resolved_sandbox_mode = sandbox_mode_override .or(profile_sandbox_mode) - .or(config_sandbox_mode) + .or(self.sandbox_mode) .or(if sandbox_mode_was_explicit { None } else { diff --git a/codex-rs/config/src/lenient.rs b/codex-rs/config/src/lenient.rs index 12159742ab..45256ec3c4 100644 --- a/codex-rs/config/src/lenient.rs +++ b/codex-rs/config/src/lenient.rs @@ -1,109 +1,49 @@ -use schemars::JsonSchema; -use schemars::r#gen::SchemaGenerator; -use schemars::schema::Schema; -use serde::Deserialize; -use serde::Deserializer; -use serde::Serialize; -use serde::Serializer; +use serde::de::DeserializeOwned; use toml::Value as TomlValue; -#[derive(Debug, Clone, PartialEq)] -pub enum Lenient { - Valid(T), - Invalid(TomlValue), -} +pub(crate) fn deserialize_with_enum_warnings( + mut value: TomlValue, +) -> Result<(TomlValue, T, Vec), toml::de::Error> +where + T: DeserializeOwned, +{ + let mut warnings = Vec::new(); -#[derive(Deserialize)] -#[serde(untagged)] -enum LenientInput { - Valid(T), - Invalid(TomlValue), -} + loop { + match serde_path_to_error::deserialize(value.clone()) { + Ok(parsed) => return Ok((value, parsed, warnings)), + Err(err) => { + let path = err.path().to_string(); + let toml_error = err.into_inner(); + if !is_unknown_variant_error(&toml_error) { + return Err(toml_error); + } -impl Lenient { - pub fn as_valid(&self) -> Option<&T> { - match self { - Self::Valid(value) => Some(value), - Self::Invalid(_) => None, - } - } - - pub fn into_valid(self) -> Option { - match self { - Self::Valid(value) => Some(value), - Self::Invalid(_) => None, - } - } - - pub fn into_valid_with_warning( - self, - field_path: &str, - warnings: &mut Vec, - ) -> Option { - match self { - Self::Valid(value) => Some(value), - Self::Invalid(value) => { + let Some(invalid_value) = remove_value_at_path(&mut value, &path) else { + return Err(toml_error); + }; warnings.push(format!( - "Ignoring invalid config value at {field_path}: {value}" + "Ignoring invalid config value at {path}: {invalid_value}" )); - None } } } +} - pub fn invalid_value(&self) -> Option<&TomlValue> { - match self { - Self::Valid(_) => None, - Self::Invalid(value) => Some(value), +fn is_unknown_variant_error(err: &toml::de::Error) -> bool { + err.message().contains("unknown variant") +} + +fn remove_value_at_path(value: &mut TomlValue, path: &str) -> Option { + let mut parts = path.split('.').peekable(); + let mut current = value; + + while let Some(part) = parts.next() { + if parts.peek().is_none() { + return current.as_table_mut()?.remove(part); } - } -} - -impl From for Lenient { - fn from(value: T) -> Self { - Self::Valid(value) - } -} - -impl<'de, T> Deserialize<'de> for Lenient -where - T: Deserialize<'de>, -{ - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Ok(match LenientInput::::deserialize(deserializer)? { - LenientInput::Valid(value) => Self::Valid(value), - LenientInput::Invalid(value) => Self::Invalid(value), - }) - } -} - -impl Serialize for Lenient -where - T: Serialize, -{ - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - match self { - Self::Valid(value) => value.serialize(serializer), - Self::Invalid(value) => value.serialize(serializer), - } - } -} - -impl JsonSchema for Lenient -where - T: JsonSchema, -{ - fn schema_name() -> String { - T::schema_name() + current = current.as_table_mut()?.get_mut(part)?; } - fn json_schema(generator: &mut SchemaGenerator) -> Schema { - T::json_schema(generator) - } + None } diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index a920f75e42..7cc97a7063 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -81,7 +81,6 @@ pub use hook_config::HooksToml; pub use hook_config::ManagedHooksRequirementsToml; pub use hook_config::MatcherGroup; pub use host_name::host_name; -pub use lenient::Lenient; pub use marketplace_edit::MarketplaceConfigUpdate; pub use marketplace_edit::RemoveMarketplaceConfigOutcome; pub use marketplace_edit::record_user_marketplace; diff --git a/codex-rs/config/src/profile_toml.rs b/codex-rs/config/src/profile_toml.rs index 5359c20c2a..f6f63191b5 100644 --- a/codex-rs/config/src/profile_toml.rs +++ b/codex-rs/config/src/profile_toml.rs @@ -3,7 +3,6 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use crate::Lenient; use crate::config_toml::ToolsToml; use crate::types::AnalyticsConfigToml; use crate::types::ApprovalsReviewer; @@ -25,20 +24,20 @@ use codex_protocol::protocol::AskForApproval; pub struct ConfigProfile { pub model: Option, /// Optional explicit service tier preference for new turns (`fast` or `flex`). - pub service_tier: Option>, + pub service_tier: Option, /// The key in the `model_providers` map identifying the /// [`ModelProviderInfo`] to use. pub model_provider: Option, - pub approval_policy: Option>, - pub approvals_reviewer: Option>, - pub sandbox_mode: Option>, - pub model_reasoning_effort: Option>, - pub plan_mode_reasoning_effort: Option>, - pub model_reasoning_summary: Option>, - pub model_verbosity: Option>, + pub approval_policy: Option, + pub approvals_reviewer: Option, + pub sandbox_mode: Option, + pub model_reasoning_effort: Option, + pub plan_mode_reasoning_effort: Option, + pub model_reasoning_summary: Option, + pub model_verbosity: Option, /// Optional path to a JSON model catalog (applied on startup only). pub model_catalog_json: Option, - pub personality: Option>, + pub personality: Option, pub chatgpt_base_url: Option, /// Optional path to a file containing model instructions. pub model_instructions_file: Option, @@ -62,7 +61,7 @@ pub struct ConfigProfile { pub experimental_use_freeform_apply_patch: Option, pub tools_view_image: Option, pub tools: Option, - pub web_search: Option>, + pub web_search: Option, pub analytics: Option, #[serde(default)] pub windows: Option, @@ -79,14 +78,10 @@ impl From for codex_app_server_protocol::Profile { Self { model: config_profile.model, model_provider: config_profile.model_provider, - approval_policy: config_profile.approval_policy.and_then(Lenient::into_valid), - model_reasoning_effort: config_profile - .model_reasoning_effort - .and_then(Lenient::into_valid), - model_reasoning_summary: config_profile - .model_reasoning_summary - .and_then(Lenient::into_valid), - model_verbosity: config_profile.model_verbosity.and_then(Lenient::into_valid), + approval_policy: config_profile.approval_policy, + model_reasoning_effort: config_profile.model_reasoning_effort, + model_reasoning_summary: config_profile.model_reasoning_summary, + model_verbosity: config_profile.model_verbosity, chatgpt_base_url: config_profile.chatgpt_base_url, } } diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index efecb77e46..e313757159 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -4,6 +4,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::lenient::deserialize_with_enum_warnings; use super::merge::merge_toml_values; use codex_app_server_protocol::ConfigLayer; use codex_app_server_protocol::ConfigLayerMetadata; @@ -310,6 +311,15 @@ impl ConfigLayerStack { merged } + pub fn deserialize_effective_config_with_warnings( + &self, + ) -> Result<(TomlValue, T, Vec), toml::de::Error> + where + T: serde::de::DeserializeOwned, + { + deserialize_with_enum_warnings(self.effective_config()) + } + /// Returns field origins for the merged config-layer view. /// /// Requirement sources are tracked separately and are not included here. diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 7392d29d16..a843dbbbbe 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -124,6 +124,9 @@ async fn invalid_enum_values_emit_warnings_without_poisoning_config() -> std::io let contents = r#" model = "gpt-5-codex" sandbox_mode = "make-it-so" + +[tui] +notification_method = "loudly" "#; let config_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&config_path, contents).expect("write config"); @@ -140,70 +143,28 @@ sandbox_mode = "make-it-so" ) .await?; - let effective_config = layers.effective_config(); - let config_toml: ConfigToml = effective_config - .clone() - .try_into() - .map_err(std::io::Error::other)?; - let mut enum_warnings = Vec::new(); - let sandbox_mode = config_toml - .sandbox_mode - .and_then(|value| value.into_valid_with_warning("sandbox_mode", &mut enum_warnings)); + let (effective_config, _config_toml, enum_warnings): (TomlValue, ConfigToml, Vec) = + layers.deserialize_effective_config_with_warnings()?; let expected_config = toml::from_str::( r#" model = "gpt-5-codex" -sandbox_mode = "make-it-so" + +[tui] "#, ) .expect("expected config should parse"); - let expected_startup_warnings = - vec!["Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string()]; + let expected_startup_warnings = vec![ + "Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string(), + "Ignoring invalid config value at tui.notification_method: \"loudly\"".to_string(), + ]; assert_eq!( - (effective_config, sandbox_mode, enum_warnings), - (expected_config, None, expected_startup_warnings) + (effective_config, enum_warnings), + (expected_config, expected_startup_warnings) ); Ok(()) } -#[tokio::test] -async fn invalid_enum_values_emit_startup_warnings_when_loading_config() -> std::io::Result<()> { - let tmp = tempdir().expect("tempdir"); - let codex_home = tmp.path().join("home"); - tokio::fs::create_dir_all(&codex_home).await?; - std::fs::write( - codex_home.join(CONFIG_TOML_FILE), - r#" -model = "gpt-5-codex" -sandbox_mode = "make-it-so" -profile = "dev" - -[profiles.dev] -model_reasoning_effort = "much" -"#, - ) - .expect("write config"); - - let config = ConfigBuilder::without_managed_config_for_tests() - .codex_home(codex_home) - .build() - .await?; - - assert_eq!( - (config.model.as_deref(), config.startup_warnings), - ( - Some("gpt-5-codex"), - vec![ - "Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string(), - "Ignoring invalid config value at profiles.dev.model_reasoning_effort: \"much\"" - .to_string(), - ], - ) - ); - - Ok(()) -} - #[tokio::test] async fn ignore_user_config_keeps_empty_user_layer() -> std::io::Result<()> { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index f07082d224..b48cf020f2 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -6,7 +6,6 @@ use crate::config::edit::ConfigEditsBuilder; use crate::config::edit::apply_blocking; use assert_matches::assert_matches; use codex_config::CONFIG_TOML_FILE; -use codex_config::Lenient; use codex_config::RequirementSource; use codex_config::config_toml::AgentRoleToml; use codex_config::config_toml::AgentsToml; @@ -152,10 +151,6 @@ async fn derive_legacy_sandbox_policy_for_test( .derive_permission_profile( sandbox_mode_override, profile_sandbox_mode, - cfg.sandbox_mode - .as_ref() - .and_then(Lenient::as_valid) - .copied(), windows_sandbox_level, active_project, permission_profile_constraint, @@ -2991,7 +2986,7 @@ async fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> { async fn config_resolves_explicit_keyring_auth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cfg = ConfigToml { - cli_auth_credentials_store: Some(Lenient::from(AuthCredentialsStoreMode::Keyring)), + cli_auth_credentials_store: Some(AuthCredentialsStoreMode::Keyring), ..Default::default() }; @@ -3109,32 +3104,41 @@ async fn feedback_enabled_defaults_to_true() -> std::io::Result<()> { #[test] fn web_search_mode_defaults_to_none_if_unset() { + let cfg = ConfigToml::default(); + let profile = ConfigProfile::default(); let features = Features::with_defaults(); - assert_eq!( - resolve_web_search_mode(/*configured_mode*/ None, &features), - None - ); + assert_eq!(resolve_web_search_mode(&cfg, &profile, &features), None); } #[test] fn web_search_mode_prefers_profile_over_legacy_flags() { + let cfg = ConfigToml::default(); + let profile = ConfigProfile { + web_search: Some(WebSearchMode::Live), + ..Default::default() + }; let mut features = Features::with_defaults(); features.enable(Feature::WebSearchCached); assert_eq!( - resolve_web_search_mode(Some(WebSearchMode::Live), &features), + resolve_web_search_mode(&cfg, &profile, &features), Some(WebSearchMode::Live) ); } #[test] fn web_search_mode_disabled_overrides_legacy_request() { + let cfg = ConfigToml { + web_search: Some(WebSearchMode::Disabled), + ..Default::default() + }; + let profile = ConfigProfile::default(); let mut features = Features::with_defaults(); features.enable(Feature::WebSearchRequest); assert_eq!( - resolve_web_search_mode(Some(WebSearchMode::Disabled), &features), + resolve_web_search_mode(&cfg, &profile, &features), Some(WebSearchMode::Disabled) ); } @@ -3253,14 +3257,14 @@ async fn profile_sandbox_mode_overrides_base() -> std::io::Result<()> { profiles.insert( "work".to_string(), ConfigProfile { - sandbox_mode: Some(Lenient::from(SandboxMode::DangerFullAccess)), + sandbox_mode: Some(SandboxMode::DangerFullAccess), ..Default::default() }, ); let cfg = ConfigToml { profiles, profile: Some("work".to_string()), - sandbox_mode: Some(Lenient::from(SandboxMode::ReadOnly)), + sandbox_mode: Some(SandboxMode::ReadOnly), ..Default::default() }; @@ -3286,7 +3290,7 @@ async fn cli_override_takes_precedence_over_profile_sandbox_mode() -> std::io::R profiles.insert( "work".to_string(), ConfigProfile { - sandbox_mode: Some(Lenient::from(SandboxMode::DangerFullAccess)), + sandbox_mode: Some(SandboxMode::DangerFullAccess), ..Default::default() }, ); @@ -3396,7 +3400,7 @@ async fn responses_websocket_features_do_not_change_wire_api() -> std::io::Resul async fn config_honors_explicit_file_oauth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cfg = ConfigToml { - mcp_oauth_credentials_store: Some(Lenient::from(OAuthCredentialsStoreMode::File)), + mcp_oauth_credentials_store: Some(OAuthCredentialsStoreMode::File), ..Default::default() }; @@ -3444,9 +3448,7 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> { e })?; assert_eq!( - cfg.mcp_oauth_credentials_store - .clone() - .and_then(codex_config::Lenient::into_valid), + cfg.mcp_oauth_credentials_store, Some(OAuthCredentialsStoreMode::Keyring), ); @@ -4570,12 +4572,7 @@ async fn set_model_updates_defaults() -> anyhow::Result<()> { let parsed: ConfigToml = toml::from_str(&serialized)?; assert_eq!(parsed.model.as_deref(), Some("gpt-5.4")); - assert_eq!( - parsed - .model_reasoning_effort - .and_then(codex_config::Lenient::into_valid), - Some(ReasoningEffort::High) - ); + assert_eq!(parsed.model_reasoning_effort, Some(ReasoningEffort::High)); Ok(()) } @@ -4606,12 +4603,7 @@ model = "gpt-4.1" let parsed: ConfigToml = toml::from_str(&serialized)?; assert_eq!(parsed.model.as_deref(), Some("o4-mini")); - assert_eq!( - parsed - .model_reasoning_effort - .and_then(codex_config::Lenient::into_valid), - Some(ReasoningEffort::High) - ); + assert_eq!(parsed.model_reasoning_effort, Some(ReasoningEffort::High)); assert_eq!( parsed .profiles @@ -4642,10 +4634,7 @@ async fn set_model_updates_profile() -> anyhow::Result<()> { assert_eq!(profile.model.as_deref(), Some("gpt-5.4")); assert_eq!( - profile - .model_reasoning_effort - .clone() - .and_then(codex_config::Lenient::into_valid), + profile.model_reasoning_effort, Some(ReasoningEffort::Medium) ); @@ -4685,10 +4674,7 @@ model = "gpt-5.4" .expect("dev profile should survive updates"); assert_eq!(dev_profile.model.as_deref(), Some("o4-high")); assert_eq!( - dev_profile - .model_reasoning_effort - .clone() - .and_then(codex_config::Lenient::into_valid), + dev_profile.model_reasoning_effort, Some(ReasoningEffort::Medium) ); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 90c465aed0..524da299c6 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -15,7 +15,6 @@ use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::ConstrainedWithSource; use codex_config::FeatureRequirementsToml; -use codex_config::Lenient; use codex_config::LoaderOverrides; use codex_config::McpServerIdentity; use codex_config::McpServerRequirement; @@ -968,9 +967,10 @@ impl ConfigBuilder { // relative paths to absolute paths based on the parent folder of the // respective config file, so we should be safe to deserialize without // AbsolutePathBufGuard here. - let merged_toml = config_layer_stack.effective_config(); - let config_toml: ConfigToml = match merged_toml.try_into() { - Ok(config_toml) => config_toml, + let (_merged_toml, config_toml, enum_warnings) = match config_layer_stack + .deserialize_effective_config_with_warnings::() + { + Ok(result) => result, Err(err) => { if let Some(config_error) = codex_config::first_layer_config_error::( &config_layer_stack, @@ -987,6 +987,7 @@ impl ConfigBuilder { return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, err)); } }; + let config_layer_stack = config_layer_stack.with_additional_startup_warnings(enum_warnings); let config_lock_settings = config_toml .debug .as_ref() @@ -1225,11 +1226,14 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides( ) .await?; - 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}"); - e - })?; + let _guard = AbsolutePathBufGuard::new(codex_home); + let (_merged_toml, cfg, _enum_warnings) = config_layer_stack + .deserialize_effective_config_with_warnings::() + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) + .map_err(|e| { + tracing::error!("Failed to deserialize overridden config: {e}"); + e + })?; Ok(cfg) } @@ -1673,17 +1677,6 @@ fn thread_store_config( } } -fn valid_lenient_ref_silent(value: &Option>) -> Option { - value.as_ref().and_then(Lenient::as_valid).cloned() -} - -fn active_profile_field_path(active_profile_name: Option<&str>, field: &str) -> String { - active_profile_name.map_or_else( - || field.to_string(), - |name| format!("profiles.{name}.{field}"), - ) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum PermissionConfigSyntax { Legacy, @@ -1848,10 +1841,11 @@ pub fn resolve_oss_provider( /// Resolve the web search mode from explicit config and feature flags. fn resolve_web_search_mode( - configured_mode: Option, + config_toml: &ConfigToml, + config_profile: &ConfigProfile, features: &Features, ) -> Option { - if let Some(mode) = configured_mode { + if let Some(mode) = config_profile.web_search.or(config_toml.web_search) { return Some(mode); } if features.enabled(Feature::WebSearchCached) { @@ -2177,12 +2171,6 @@ impl Config { .clone(), None => ConfigProfile::default(), }; - let profile_sandbox_mode_path = - active_profile_field_path(active_profile_name.as_deref(), "sandbox_mode"); - let profile_approval_policy_path = - active_profile_field_path(active_profile_name.as_deref(), "approval_policy"); - let profile_approvals_reviewer_path = - active_profile_field_path(active_profile_name.as_deref(), "approvals_reviewer"); let tool_suggest = resolve_tool_suggest_config(&cfg, &config_layer_stack); let feature_overrides = FeatureOverrides { include_apply_patch_tool: include_apply_patch_tool_override, @@ -2248,7 +2236,7 @@ impl Config { &config_layer_stack, &cfg, sandbox_mode, - valid_lenient_ref_silent(&config_profile.sandbox_mode), + config_profile.sandbox_mode, ); let has_permission_profiles = cfg .permissions @@ -2451,20 +2439,10 @@ impl Config { // Derive the old `sandbox_mode` defaults as a profile first, then // keep a legacy-compatible projection only for the remaining code // paths that still speak `SandboxPolicy`. - let config_sandbox_mode = cfg - .sandbox_mode - .clone() - .and_then(|value| value.into_valid_with_warning("sandbox_mode", &mut startup_warnings)); let mut permission_profile = cfg .derive_permission_profile( sandbox_mode, - config_profile.sandbox_mode.clone().and_then(|value| { - value.into_valid_with_warning( - &profile_sandbox_mode_path, - &mut startup_warnings, - ) - }), - config_sandbox_mode, + config_profile.sandbox_mode, windows_sandbox_level, Some(&active_project), Some(&constrained_permission_profile), @@ -2518,27 +2496,20 @@ impl Config { ) }; let approval_policy_was_explicit = approval_policy_override.is_some() - || valid_lenient_ref_silent(&config_profile.approval_policy).is_some() - || valid_lenient_ref_silent(&cfg.approval_policy).is_some(); - let mut approval_policy = if let Some(approval_policy) = approval_policy_override { - approval_policy - } else if let Some(approval_policy) = - config_profile.approval_policy.clone().and_then(|value| { - value.into_valid_with_warning(&profile_approval_policy_path, &mut startup_warnings) - }) - { - approval_policy - } else if let Some(approval_policy) = cfg.approval_policy.clone().and_then(|value| { - value.into_valid_with_warning("approval_policy", &mut startup_warnings) - }) { - approval_policy - } else if active_project.is_trusted() { - AskForApproval::OnRequest - } else if active_project.is_untrusted() { - AskForApproval::UnlessTrusted - } else { - AskForApproval::default() - }; + || config_profile.approval_policy.is_some() + || cfg.approval_policy.is_some(); + let mut approval_policy = approval_policy_override + .or(config_profile.approval_policy) + .or(cfg.approval_policy) + .unwrap_or_else(|| { + if active_project.is_trusted() { + AskForApproval::OnRequest + } else if active_project.is_untrusted() { + AskForApproval::UnlessTrusted + } else { + AskForApproval::default() + } + }); if !approval_policy_was_explicit && let Err(err) = constrained_approval_policy.can_set(&approval_policy) { @@ -2549,28 +2520,12 @@ impl Config { approval_policy = constrained_approval_policy.value(); } let approvals_reviewer_was_explicit = approvals_reviewer_override.is_some() - || valid_lenient_ref_silent(&config_profile.approvals_reviewer) - .is_some() - || valid_lenient_ref_silent(&cfg.approvals_reviewer).is_some(); - let mut approvals_reviewer = if let Some(approvals_reviewer) = approvals_reviewer_override { - approvals_reviewer - } else if let Some(approvals_reviewer) = - config_profile.approvals_reviewer.clone().and_then(|value| { - value.into_valid_with_warning( - &profile_approvals_reviewer_path, - &mut startup_warnings, - ) - }) - { - approvals_reviewer - } else { - cfg.approvals_reviewer - .clone() - .and_then(|value| { - value.into_valid_with_warning("approvals_reviewer", &mut startup_warnings) - }) - .unwrap_or(ApprovalsReviewer::User) - }; + || config_profile.approvals_reviewer.is_some() + || cfg.approvals_reviewer.is_some(); + let mut approvals_reviewer = approvals_reviewer_override + .or(config_profile.approvals_reviewer) + .or(cfg.approvals_reviewer) + .unwrap_or(ApprovalsReviewer::User); if !approvals_reviewer_was_explicit && let Err(err) = constrained_approvals_reviewer.can_set(&approvals_reviewer) { @@ -2580,17 +2535,7 @@ impl Config { ); approvals_reviewer = constrained_approvals_reviewer.value(); } - let profile_web_search_path = - active_profile_field_path(active_profile_name.as_deref(), "web_search"); - let configured_web_search_mode = match config_profile.web_search.clone().and_then(|value| { - value.into_valid_with_warning(&profile_web_search_path, &mut startup_warnings) - }) { - Some(mode) => Some(mode), - None => cfg.web_search.clone().and_then(|value| { - value.into_valid_with_warning("web_search", &mut startup_warnings) - }), - }; - let web_search_mode = resolve_web_search_mode(configured_web_search_mode, &features) + let web_search_mode = resolve_web_search_mode(&cfg, &config_profile, &features) .unwrap_or(WebSearchMode::Cached); let web_search_config = resolve_web_search_config(&cfg, &config_profile); let multi_agent_v2 = resolve_multi_agent_v2_config(&cfg, &config_profile); @@ -2761,9 +2706,7 @@ impl Config { } }); - let forced_login_method = cfg.forced_login_method.clone().and_then(|value| { - value.into_valid_with_warning("forced_login_method", &mut startup_warnings) - }); + let forced_login_method = cfg.forced_login_method; let model = model.or(config_profile.model).or(cfg.model); let mut notices = cfg.notice.unwrap_or_default(); @@ -2775,21 +2718,7 @@ impl Config { notices.fast_default_opt_out = Some(true); None } - None => { - let profile_service_tier_path = - active_profile_field_path(active_profile_name.as_deref(), "service_tier"); - match config_profile.service_tier.clone().and_then(|value| { - value.into_valid_with_warning( - &profile_service_tier_path, - &mut startup_warnings, - ) - }) { - Some(service_tier) => Some(service_tier), - None => cfg.service_tier.clone().and_then(|value| { - value.into_valid_with_warning("service_tier", &mut startup_warnings) - }), - } - } + None => config_profile.service_tier.or(cfg.service_tier), }; let service_tier = match service_tier { Some(ServiceTier::Fast) if features.enabled(Feature::FastMode) => { @@ -2854,28 +2783,14 @@ impl Config { auto_review.policy.as_deref(), )) }); - let personality = if personality.is_some() { - personality - } else { - let profile_personality_path = - active_profile_field_path(active_profile_name.as_deref(), "personality"); - match config_profile.personality.clone().and_then(|value| { - value.into_valid_with_warning(&profile_personality_path, &mut startup_warnings) - }) { - Some(personality) => Some(personality), - None => cfg - .personality - .clone() - .and_then(|value| { - value.into_valid_with_warning("personality", &mut startup_warnings) - }) - .or_else(|| { - features - .enabled(Feature::Personality) - .then_some(Personality::Pragmatic) - }), - } - }; + let personality = personality + .or(config_profile.personality) + .or(cfg.personality) + .or_else(|| { + features + .enabled(Feature::Personality) + .then_some(Personality::Pragmatic) + }); let experimental_compact_prompt_path = config_profile .experimental_compact_prompt_file @@ -3039,92 +2954,6 @@ impl Config { .value .set(effective_permission_profile) .map_err(std::io::Error::from)?; - let cli_auth_credentials_store_mode = resolve_cli_auth_credentials_store_mode( - cfg.cli_auth_credentials_store - .clone() - .and_then(|value| { - value.into_valid_with_warning( - "cli_auth_credentials_store", - &mut startup_warnings, - ) - }) - .unwrap_or_default(), - env!("CARGO_PKG_VERSION"), - ); - let mcp_oauth_credentials_store_mode = resolve_mcp_oauth_credentials_store_mode( - cfg.mcp_oauth_credentials_store - .clone() - .and_then(|value| { - value.into_valid_with_warning( - "mcp_oauth_credentials_store", - &mut startup_warnings, - ) - }) - .unwrap_or_default(), - env!("CARGO_PKG_VERSION"), - ); - let file_opener = cfg - .file_opener - .and_then(|value| { - value.into_valid_with_warning("file_opener", &mut startup_warnings) - }) - .unwrap_or(UriBasedFileOpener::VsCode); - let model_reasoning_effort_path = - active_profile_field_path(active_profile_name.as_deref(), "model_reasoning_effort"); - let model_reasoning_effort = match config_profile.model_reasoning_effort.and_then(|value| { - value.into_valid_with_warning(&model_reasoning_effort_path, &mut startup_warnings) - }) { - Some(effort) => Some(effort), - None => cfg.model_reasoning_effort.and_then(|value| { - value.into_valid_with_warning("model_reasoning_effort", &mut startup_warnings) - }), - }; - let plan_mode_reasoning_effort_path = active_profile_field_path( - active_profile_name.as_deref(), - "plan_mode_reasoning_effort", - ); - let plan_mode_reasoning_effort = - match config_profile.plan_mode_reasoning_effort.and_then(|value| { - value.into_valid_with_warning( - &plan_mode_reasoning_effort_path, - &mut startup_warnings, - ) - }) { - Some(effort) => Some(effort), - None => cfg.plan_mode_reasoning_effort.and_then(|value| { - value.into_valid_with_warning( - "plan_mode_reasoning_effort", - &mut startup_warnings, - ) - }), - }; - let model_reasoning_summary_path = - active_profile_field_path(active_profile_name.as_deref(), "model_reasoning_summary"); - let model_reasoning_summary = - match config_profile.model_reasoning_summary.and_then(|value| { - value.into_valid_with_warning(&model_reasoning_summary_path, &mut startup_warnings) - }) { - Some(summary) => Some(summary), - None => cfg.model_reasoning_summary.and_then(|value| { - value.into_valid_with_warning("model_reasoning_summary", &mut startup_warnings) - }), - }; - let model_verbosity_path = - active_profile_field_path(active_profile_name.as_deref(), "model_verbosity"); - let model_verbosity = match config_profile.model_verbosity.and_then(|value| { - value.into_valid_with_warning(&model_verbosity_path, &mut startup_warnings) - }) { - Some(verbosity) => Some(verbosity), - None => cfg.model_verbosity.and_then(|value| { - value.into_valid_with_warning("model_verbosity", &mut startup_warnings) - }), - }; - let experimental_thread_store = thread_store_config( - cfg.experimental_thread_store.and_then(|value| { - value.into_valid_with_warning("experimental_thread_store", &mut startup_warnings) - }), - cfg.experimental_thread_store_endpoint, - ); let config = Self { model, service_tier, @@ -3160,11 +2989,17 @@ impl Config { include_environment_context, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. - cli_auth_credentials_store_mode, + cli_auth_credentials_store_mode: resolve_cli_auth_credentials_store_mode( + cfg.cli_auth_credentials_store.unwrap_or_default(), + env!("CARGO_PKG_VERSION"), + ), mcp_servers, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. - mcp_oauth_credentials_store_mode, + mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( + cfg.mcp_oauth_credentials_store.unwrap_or_default(), + env!("CARGO_PKG_VERSION"), + ), mcp_oauth_callback_port: cfg.mcp_oauth_callback_port, mcp_oauth_callback_url: cfg.mcp_oauth_callback_url.clone(), model_providers, @@ -3213,7 +3048,7 @@ impl Config { config_layer_stack, history, ephemeral: ephemeral.unwrap_or_default(), - file_opener, + file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), codex_self_exe, codex_linux_sandbox_exe, main_execve_wrapper_exe, @@ -3225,12 +3060,18 @@ impl Config { .or(show_raw_agent_reasoning) .unwrap_or(false), guardian_policy_config, - model_reasoning_effort, - plan_mode_reasoning_effort, - model_reasoning_summary, + model_reasoning_effort: config_profile + .model_reasoning_effort + .or(cfg.model_reasoning_effort), + plan_mode_reasoning_effort: config_profile + .plan_mode_reasoning_effort + .or(cfg.plan_mode_reasoning_effort), + model_reasoning_summary: config_profile + .model_reasoning_summary + .or(cfg.model_reasoning_summary), model_supports_reasoning_summaries: cfg.model_supports_reasoning_summaries, model_catalog, - model_verbosity, + model_verbosity: config_profile.model_verbosity.or(cfg.model_verbosity), chatgpt_base_url: config_profile .chatgpt_base_url .or(cfg.chatgpt_base_url) @@ -3259,7 +3100,10 @@ impl Config { experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context, experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions, experimental_thread_config_endpoint: cfg.experimental_thread_config_endpoint, - experimental_thread_store, + experimental_thread_store: thread_store_config( + cfg.experimental_thread_store, + cfg.experimental_thread_store_endpoint, + ), forced_chatgpt_workspace_id, forced_login_method, include_apply_patch_tool: include_apply_patch_tool_flag, diff --git a/codex-rs/core/src/config/template_interpolation.rs b/codex-rs/core/src/config/template_interpolation.rs index 5812ab186d..7e38fed86f 100644 --- a/codex-rs/core/src/config/template_interpolation.rs +++ b/codex-rs/core/src/config/template_interpolation.rs @@ -1,6 +1,5 @@ use anyhow::Context; use anyhow::bail; -use codex_config::Lenient; use codex_config::config_toml::ConfigToml; use codex_config::types::MemoriesToml; use codex_features::AppsMcpPathOverrideConfigToml; @@ -66,10 +65,10 @@ pub(crate) fn apply_resolved_config_fields( config: &Config, config_toml: &mut ConfigToml, ) -> anyhow::Result<()> { - config_toml.web_search = Some(Lenient::from(config.web_search_mode.value())); + config_toml.web_search = Some(config.web_search_mode.value()); config_toml.model_provider = Some(config.model_provider_id.clone()); - config_toml.plan_mode_reasoning_effort = config.plan_mode_reasoning_effort.map(Lenient::from); - config_toml.model_verbosity = config.model_verbosity.map(Lenient::from); + config_toml.plan_mode_reasoning_effort = config.plan_mode_reasoning_effort; + config_toml.model_verbosity = config.model_verbosity; config_toml.include_permissions_instructions = Some(config.include_permissions_instructions); config_toml.include_apps_instructions = Some(config.include_apps_instructions); config_toml.include_environment_context = Some(config.include_environment_context); diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index 40efe1104e..28d34bc3c9 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -1,5 +1,4 @@ use super::*; -use codex_config::Lenient; use codex_protocol::ThreadId; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; @@ -16,10 +15,6 @@ use tokio::io::AsyncWriteExt; const TEST_TIMESTAMP: &str = "2025-01-01T00-00-00"; -fn valid_lenient(value: Option>) -> Option { - value.and_then(Lenient::into_valid) -} - async fn read_config_toml(codex_home: &Path) -> io::Result { let contents = tokio::fs::read_to_string(codex_home.join("config.toml")).await?; toml::from_str(&contents).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err)) @@ -98,10 +93,7 @@ async fn applies_when_sessions_exist_and_no_personality() -> io::Result<()> { assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists()); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) } @@ -117,10 +109,7 @@ async fn applies_when_only_archived_sessions_exist_and_no_personality() -> io::R assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists()); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) } @@ -156,10 +145,7 @@ async fn skips_when_personality_explicit() -> io::Result<()> { assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists()); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Friendly) - ); + assert_eq!(persisted.personality, Some(Personality::Friendly)); Ok(()) } diff --git a/codex-rs/core/src/session/config_lock.rs b/codex-rs/core/src/session/config_lock.rs index 62da0b3eae..f0a8d209d3 100644 --- a/codex-rs/core/src/session/config_lock.rs +++ b/codex-rs/core/src/session/config_lock.rs @@ -1,5 +1,4 @@ use anyhow::Context; -use codex_config::Lenient; use codex_config::config_toml::ConfigLockfileToml; use codex_config::config_toml::ConfigToml; use codex_protocol::ThreadId; @@ -93,16 +92,15 @@ fn session_configuration_to_lock_config_toml( /// from the live session when the lockfile should be fully self-contained. fn save_session_resolved_fields(sc: &SessionConfiguration, lock_config: &mut ConfigToml) { lock_config.model = Some(sc.collaboration_mode.model().to_string()); - lock_config.model_reasoning_effort = - sc.collaboration_mode.reasoning_effort().map(Lenient::from); - lock_config.model_reasoning_summary = sc.model_reasoning_summary.map(Lenient::from); - lock_config.service_tier = sc.service_tier.map(Lenient::from); + lock_config.model_reasoning_effort = sc.collaboration_mode.reasoning_effort(); + lock_config.model_reasoning_summary = sc.model_reasoning_summary; + lock_config.service_tier = sc.service_tier; lock_config.instructions = Some(sc.base_instructions.clone()); lock_config.developer_instructions = sc.developer_instructions.clone(); lock_config.compact_prompt = sc.compact_prompt.clone(); - lock_config.personality = sc.personality.map(Lenient::from); - lock_config.approval_policy = Some(Lenient::from(sc.approval_policy.value())); - lock_config.approvals_reviewer = Some(Lenient::from(sc.approvals_reviewer)); + lock_config.personality = sc.personality; + lock_config.approval_policy = Some(sc.approval_policy.value()); + lock_config.approvals_reviewer = Some(sc.approvals_reviewer); } fn drop_lockfile_inputs(lock_config: &mut ConfigToml) { @@ -147,9 +145,7 @@ mod tests { assert_eq!(lock.compact_prompt, sc.compact_prompt); assert_eq!(lock.model, Some(sc.collaboration_mode.model().to_string())); assert_eq!( - lock.model_reasoning_effort - .clone() - .and_then(codex_config::Lenient::into_valid), + lock.model_reasoning_effort, sc.collaboration_mode.reasoning_effort() ); assert_eq!(lock.profile, None); diff --git a/codex-rs/core/tests/suite/personality_migration.rs b/codex-rs/core/tests/suite/personality_migration.rs index 76d2200aab..68121d2e12 100644 --- a/codex-rs/core/tests/suite/personality_migration.rs +++ b/codex-rs/core/tests/suite/personality_migration.rs @@ -1,4 +1,3 @@ -use codex_config::Lenient; use codex_config::config_toml::ConfigToml; use codex_core::ARCHIVED_SESSIONS_SUBDIR; use codex_core::SESSIONS_SUBDIR; @@ -22,10 +21,6 @@ use tokio::io::AsyncWriteExt; const TEST_TIMESTAMP: &str = "2025-01-01T00-00-00"; -fn valid_lenient(value: Option>) -> Option { - value.and_then(Lenient::into_valid) -} - async fn read_config_toml(codex_home: &Path) -> io::Result { let contents = tokio::fs::read_to_string(codex_home.join("config.toml")).await?; toml::from_str(&contents).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err)) @@ -191,10 +186,7 @@ async fn no_marker_sessions_sets_personality() -> io::Result<()> { ); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) } @@ -210,10 +202,7 @@ async fn no_marker_sessions_preserves_existing_config_fields() -> io::Result<()> assert_eq!(status, PersonalityMigrationStatus::Applied); let persisted = read_config_toml(temp.path()).await?; assert_eq!(persisted.model, Some("gpt-5.4".to_string())); - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) } @@ -332,10 +321,7 @@ async fn applied_migration_is_idempotent_on_second_run() -> io::Result<()> { assert_eq!(first_status, PersonalityMigrationStatus::Applied); assert_eq!(second_status, PersonalityMigrationStatus::SkippedMarker); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) } @@ -354,9 +340,6 @@ async fn no_marker_archived_sessions_sets_personality() -> io::Result<()> { ); let persisted = read_config_toml(temp.path()).await?; - assert_eq!( - valid_lenient(persisted.personality), - Some(Personality::Pragmatic) - ); + assert_eq!(persisted.personality, Some(Personality::Pragmatic)); Ok(()) }