mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Collect config enum warnings while unwrapping
Remove the explicit invalid_enum_warnings tree walk and have Lenient::into_valid append warning messages when invalid values are consumed. Keep higher-level resolvers returning values instead of accepting active profile and warning sink plumbing. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -6,7 +6,6 @@ use std::path::Path;
|
||||
|
||||
use crate::HooksToml;
|
||||
use crate::Lenient;
|
||||
use crate::invalid_config_warnings;
|
||||
use crate::permissions_toml::PermissionsToml;
|
||||
use crate::profile_toml::ConfigProfile;
|
||||
use crate::types::AnalyticsConfigToml;
|
||||
@@ -511,21 +510,27 @@ impl From<ConfigToml> 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
|
||||
.and_then(|value| value.into_valid("approval_policy", None)),
|
||||
sandbox_mode: config_toml
|
||||
.sandbox_mode
|
||||
.and_then(|value| value.into_valid("sandbox_mode", None)),
|
||||
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),
|
||||
.and_then(|value| value.into_valid("forced_login_method", None)),
|
||||
model: config_toml.model,
|
||||
model_reasoning_effort: config_toml
|
||||
.model_reasoning_effort
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
model_reasoning_summary: config_toml
|
||||
.model_reasoning_summary
|
||||
.and_then(Lenient::into_valid),
|
||||
model_verbosity: config_toml.model_verbosity.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_summary", None)),
|
||||
model_verbosity: config_toml
|
||||
.model_verbosity
|
||||
.and_then(|value| value.into_valid("model_verbosity", None)),
|
||||
tools: config_toml.tools.map(From::from),
|
||||
profile: config_toml.profile,
|
||||
profiles,
|
||||
@@ -719,15 +724,11 @@ impl ConfigToml {
|
||||
&self,
|
||||
sandbox_mode_override: Option<SandboxMode>,
|
||||
profile_sandbox_mode: Option<SandboxMode>,
|
||||
config_sandbox_mode: Option<SandboxMode>,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
active_project: Option<&ProjectConfig>,
|
||||
permission_profile_constraint: Option<&crate::Constrained<PermissionProfile>>,
|
||||
) -> PermissionProfile {
|
||||
let config_sandbox_mode = self
|
||||
.sandbox_mode
|
||||
.as_ref()
|
||||
.and_then(Lenient::as_valid)
|
||||
.copied();
|
||||
let sandbox_mode_was_explicit = sandbox_mode_override.is_some()
|
||||
|| profile_sandbox_mode.is_some()
|
||||
|| config_sandbox_mode.is_some();
|
||||
@@ -853,69 +854,6 @@ impl ConfigToml {
|
||||
None => Ok(ConfigProfile::default()),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn invalid_enum_warnings(&self) -> Vec<String> {
|
||||
let mut warnings = Vec::new();
|
||||
push_invalid(&mut warnings, "approval_policy", &self.approval_policy);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"approvals_reviewer",
|
||||
&self.approvals_reviewer,
|
||||
);
|
||||
push_invalid(&mut warnings, "sandbox_mode", &self.sandbox_mode);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"forced_login_method",
|
||||
&self.forced_login_method,
|
||||
);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"cli_auth_credentials_store",
|
||||
&self.cli_auth_credentials_store,
|
||||
);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"mcp_oauth_credentials_store",
|
||||
&self.mcp_oauth_credentials_store,
|
||||
);
|
||||
push_invalid(&mut warnings, "file_opener", &self.file_opener);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"model_reasoning_effort",
|
||||
&self.model_reasoning_effort,
|
||||
);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"plan_mode_reasoning_effort",
|
||||
&self.plan_mode_reasoning_effort,
|
||||
);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"model_reasoning_summary",
|
||||
&self.model_reasoning_summary,
|
||||
);
|
||||
push_invalid(&mut warnings, "model_verbosity", &self.model_verbosity);
|
||||
push_invalid(&mut warnings, "personality", &self.personality);
|
||||
push_invalid(&mut warnings, "service_tier", &self.service_tier);
|
||||
push_invalid(
|
||||
&mut warnings,
|
||||
"experimental_thread_store",
|
||||
&self.experimental_thread_store,
|
||||
);
|
||||
push_invalid(&mut warnings, "web_search", &self.web_search);
|
||||
let mut profiles: Vec<_> = self.profiles.iter().collect();
|
||||
profiles.sort_by_key(|(name, _)| *name);
|
||||
for (name, profile) in profiles {
|
||||
profile.push_invalid_enum_warnings(&mut warnings, &format!("profiles.{name}"));
|
||||
}
|
||||
warnings
|
||||
}
|
||||
}
|
||||
|
||||
fn push_invalid<T>(warnings: &mut Vec<String>, path: &str, value: &Option<Lenient<T>>) {
|
||||
if let Some(warning) = invalid_config_warnings(path, value) {
|
||||
warnings.push(warning);
|
||||
}
|
||||
}
|
||||
|
||||
/// Canonicalize the path and convert it to a string to be used as a key in the
|
||||
|
||||
@@ -28,10 +28,17 @@ impl<T> Lenient<T> {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn into_valid(self) -> Option<T> {
|
||||
pub fn into_valid(self, field_path: &str, warnings: Option<&mut Vec<String>>) -> Option<T> {
|
||||
match self {
|
||||
Self::Valid(value) => Some(value),
|
||||
Self::Invalid(_) => None,
|
||||
Self::Invalid(value) => {
|
||||
if let Some(warnings) = warnings {
|
||||
warnings.push(format!(
|
||||
"Ignoring invalid config value at {field_path}: {value}"
|
||||
));
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -91,11 +98,3 @@ where
|
||||
T::json_schema(generator)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn invalid_config_warnings<T>(field_path: &str, value: &Option<Lenient<T>>) -> Option<String> {
|
||||
value.as_ref().and_then(|value| {
|
||||
value
|
||||
.invalid_value()
|
||||
.map(|invalid| format!("Ignoring invalid config value at {field_path}: {invalid}"))
|
||||
})
|
||||
}
|
||||
|
||||
@@ -82,7 +82,6 @@ pub use hook_config::ManagedHooksRequirementsToml;
|
||||
pub use hook_config::MatcherGroup;
|
||||
pub use host_name::host_name;
|
||||
pub use lenient::Lenient;
|
||||
pub use lenient::invalid_config_warnings;
|
||||
pub use marketplace_edit::MarketplaceConfigUpdate;
|
||||
pub use marketplace_edit::RemoveMarketplaceConfigOutcome;
|
||||
pub use marketplace_edit::record_user_marketplace;
|
||||
|
||||
@@ -5,7 +5,6 @@ use serde::Serialize;
|
||||
|
||||
use crate::Lenient;
|
||||
use crate::config_toml::ToolsToml;
|
||||
use crate::invalid_config_warnings;
|
||||
use crate::types::AnalyticsConfigToml;
|
||||
use crate::types::ApprovalsReviewer;
|
||||
use crate::types::Personality;
|
||||
@@ -80,72 +79,19 @@ impl From<ConfigProfile> 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),
|
||||
approval_policy: config_profile
|
||||
.approval_policy
|
||||
.and_then(|value| value.into_valid("approval_policy", None)),
|
||||
model_reasoning_effort: config_profile
|
||||
.model_reasoning_effort
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
model_reasoning_summary: config_profile
|
||||
.model_reasoning_summary
|
||||
.and_then(Lenient::into_valid),
|
||||
model_verbosity: config_profile.model_verbosity.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_summary", None)),
|
||||
model_verbosity: config_profile
|
||||
.model_verbosity
|
||||
.and_then(|value| value.into_valid("model_verbosity", None)),
|
||||
chatgpt_base_url: config_profile.chatgpt_base_url,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigProfile {
|
||||
pub fn push_invalid_enum_warnings(&self, warnings: &mut Vec<String>, prefix: &str) {
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.service_tier"),
|
||||
&self.service_tier,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.approval_policy"),
|
||||
&self.approval_policy,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.approvals_reviewer"),
|
||||
&self.approvals_reviewer,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.sandbox_mode"),
|
||||
&self.sandbox_mode,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.model_reasoning_effort"),
|
||||
&self.model_reasoning_effort,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.plan_mode_reasoning_effort"),
|
||||
&self.plan_mode_reasoning_effort,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.model_reasoning_summary"),
|
||||
&self.model_reasoning_summary,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.model_verbosity"),
|
||||
&self.model_verbosity,
|
||||
);
|
||||
push_invalid(
|
||||
warnings,
|
||||
&format!("{prefix}.personality"),
|
||||
&self.personality,
|
||||
);
|
||||
push_invalid(warnings, &format!("{prefix}.web_search"), &self.web_search);
|
||||
}
|
||||
}
|
||||
|
||||
fn push_invalid<T>(warnings: &mut Vec<String>, path: &str, value: &Option<Lenient<T>>) {
|
||||
if let Some(warning) = invalid_config_warnings(path, value) {
|
||||
warnings.push(warning);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -145,7 +145,10 @@ sandbox_mode = "make-it-so"
|
||||
.clone()
|
||||
.try_into()
|
||||
.map_err(std::io::Error::other)?;
|
||||
let enum_warnings = config_toml.invalid_enum_warnings();
|
||||
let mut enum_warnings = Vec::new();
|
||||
let sandbox_mode = config_toml
|
||||
.sandbox_mode
|
||||
.and_then(|value| value.into_valid("sandbox_mode", Some(&mut enum_warnings)));
|
||||
let expected_config = toml::from_str::<TomlValue>(
|
||||
r#"
|
||||
model = "gpt-5-codex"
|
||||
@@ -157,8 +160,8 @@ sandbox_mode = "make-it-so"
|
||||
vec!["Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string()];
|
||||
|
||||
assert_eq!(
|
||||
(effective_config, enum_warnings),
|
||||
(expected_config, expected_startup_warnings)
|
||||
(effective_config, sandbox_mode, enum_warnings),
|
||||
(expected_config, None, expected_startup_warnings)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -173,6 +176,7 @@ async fn invalid_enum_values_emit_startup_warnings_when_loading_config() -> std:
|
||||
r#"
|
||||
model = "gpt-5-codex"
|
||||
sandbox_mode = "make-it-so"
|
||||
profile = "dev"
|
||||
|
||||
[profiles.dev]
|
||||
model_reasoning_effort = "much"
|
||||
|
||||
@@ -152,6 +152,10 @@ 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,
|
||||
@@ -3105,41 +3109,29 @@ 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(&cfg, &profile, &features), None);
|
||||
assert_eq!(resolve_web_search_mode(None, &features), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn web_search_mode_prefers_profile_over_legacy_flags() {
|
||||
let cfg = ConfigToml::default();
|
||||
let profile = ConfigProfile {
|
||||
web_search: Some(Lenient::from(WebSearchMode::Live)),
|
||||
..Default::default()
|
||||
};
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::WebSearchCached);
|
||||
|
||||
assert_eq!(
|
||||
resolve_web_search_mode(&cfg, &profile, &features),
|
||||
resolve_web_search_mode(Some(WebSearchMode::Live), &features),
|
||||
Some(WebSearchMode::Live)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn web_search_mode_disabled_overrides_legacy_request() {
|
||||
let cfg = ConfigToml {
|
||||
web_search: Some(Lenient::from(WebSearchMode::Disabled)),
|
||||
..Default::default()
|
||||
};
|
||||
let profile = ConfigProfile::default();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::WebSearchRequest);
|
||||
|
||||
assert_eq!(
|
||||
resolve_web_search_mode(&cfg, &profile, &features),
|
||||
resolve_web_search_mode(Some(WebSearchMode::Disabled), &features),
|
||||
Some(WebSearchMode::Disabled)
|
||||
);
|
||||
}
|
||||
@@ -3451,7 +3443,7 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
|
||||
assert_eq!(
|
||||
cfg.mcp_oauth_credentials_store
|
||||
.clone()
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("mcp_oauth_credentials_store", None)),
|
||||
Some(OAuthCredentialsStoreMode::Keyring),
|
||||
);
|
||||
|
||||
@@ -4576,7 +4568,9 @@ async fn set_model_updates_defaults() -> anyhow::Result<()> {
|
||||
|
||||
assert_eq!(parsed.model.as_deref(), Some("gpt-5.4"));
|
||||
assert_eq!(
|
||||
parsed.model_reasoning_effort.and_then(Lenient::into_valid),
|
||||
parsed
|
||||
.model_reasoning_effort
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
Some(ReasoningEffort::High)
|
||||
);
|
||||
|
||||
@@ -4610,7 +4604,9 @@ model = "gpt-4.1"
|
||||
|
||||
assert_eq!(parsed.model.as_deref(), Some("o4-mini"));
|
||||
assert_eq!(
|
||||
parsed.model_reasoning_effort.and_then(Lenient::into_valid),
|
||||
parsed
|
||||
.model_reasoning_effort
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
Some(ReasoningEffort::High)
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -4646,7 +4642,7 @@ async fn set_model_updates_profile() -> anyhow::Result<()> {
|
||||
profile
|
||||
.model_reasoning_effort
|
||||
.clone()
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
Some(ReasoningEffort::Medium)
|
||||
);
|
||||
|
||||
@@ -4689,7 +4685,7 @@ model = "gpt-5.4"
|
||||
dev_profile
|
||||
.model_reasoning_effort
|
||||
.clone()
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
Some(ReasoningEffort::Medium)
|
||||
);
|
||||
|
||||
|
||||
+221
-58
@@ -987,8 +987,6 @@ impl ConfigBuilder {
|
||||
return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, err));
|
||||
}
|
||||
};
|
||||
let enum_warnings = config_toml.invalid_enum_warnings();
|
||||
let config_layer_stack = config_layer_stack.with_additional_startup_warnings(enum_warnings);
|
||||
let config_lock_settings = config_toml
|
||||
.debug
|
||||
.as_ref()
|
||||
@@ -1675,14 +1673,35 @@ fn thread_store_config(
|
||||
}
|
||||
}
|
||||
|
||||
fn valid_lenient<T>(value: Option<Lenient<T>>) -> Option<T> {
|
||||
value.and_then(Lenient::into_valid)
|
||||
fn valid_lenient<T>(
|
||||
value: Option<Lenient<T>>,
|
||||
field_path: &str,
|
||||
warnings: &mut Vec<String>,
|
||||
) -> Option<T> {
|
||||
value.and_then(|value| value.into_valid(field_path, Some(warnings)))
|
||||
}
|
||||
|
||||
fn valid_lenient_ref<T: Clone>(value: &Option<Lenient<T>>) -> Option<T> {
|
||||
fn valid_lenient_ref<T: Clone>(
|
||||
value: &Option<Lenient<T>>,
|
||||
field_path: &str,
|
||||
warnings: &mut Vec<String>,
|
||||
) -> Option<T> {
|
||||
value
|
||||
.clone()
|
||||
.and_then(|value| value.into_valid(field_path, Some(warnings)))
|
||||
}
|
||||
|
||||
fn valid_lenient_ref_silent<T: Clone>(value: &Option<Lenient<T>>) -> Option<T> {
|
||||
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,
|
||||
@@ -1847,13 +1866,10 @@ pub fn resolve_oss_provider(
|
||||
|
||||
/// Resolve the web search mode from explicit config and feature flags.
|
||||
fn resolve_web_search_mode(
|
||||
config_toml: &ConfigToml,
|
||||
config_profile: &ConfigProfile,
|
||||
configured_mode: Option<WebSearchMode>,
|
||||
features: &Features,
|
||||
) -> Option<WebSearchMode> {
|
||||
if let Some(mode) = valid_lenient_ref(&config_profile.web_search)
|
||||
.or_else(|| valid_lenient_ref(&config_toml.web_search))
|
||||
{
|
||||
if let Some(mode) = configured_mode {
|
||||
return Some(mode);
|
||||
}
|
||||
if features.enabled(Feature::WebSearchCached) {
|
||||
@@ -2179,6 +2195,12 @@ 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,
|
||||
@@ -2244,7 +2266,7 @@ impl Config {
|
||||
&config_layer_stack,
|
||||
&cfg,
|
||||
sandbox_mode,
|
||||
valid_lenient_ref(&config_profile.sandbox_mode),
|
||||
valid_lenient_ref_silent(&config_profile.sandbox_mode),
|
||||
);
|
||||
let has_permission_profiles = cfg
|
||||
.permissions
|
||||
@@ -2447,10 +2469,17 @@ 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 =
|
||||
valid_lenient_ref(&cfg.sandbox_mode, "sandbox_mode", &mut startup_warnings);
|
||||
let mut permission_profile = cfg
|
||||
.derive_permission_profile(
|
||||
sandbox_mode,
|
||||
valid_lenient_ref(&config_profile.sandbox_mode),
|
||||
valid_lenient_ref(
|
||||
&config_profile.sandbox_mode,
|
||||
&profile_sandbox_mode_path,
|
||||
&mut startup_warnings,
|
||||
),
|
||||
config_sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
Some(&active_project),
|
||||
Some(&constrained_permission_profile),
|
||||
@@ -2504,12 +2533,23 @@ impl Config {
|
||||
)
|
||||
};
|
||||
let approval_policy_was_explicit = approval_policy_override.is_some()
|
||||
|| valid_lenient_ref(&config_profile.approval_policy).is_some()
|
||||
|| valid_lenient_ref(&cfg.approval_policy).is_some();
|
||||
let mut approval_policy = approval_policy_override
|
||||
.or_else(|| valid_lenient_ref(&config_profile.approval_policy))
|
||||
.or_else(|| valid_lenient_ref(&cfg.approval_policy))
|
||||
.unwrap_or_else(|| {
|
||||
|| 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) = valid_lenient_ref(
|
||||
&config_profile.approval_policy,
|
||||
&profile_approval_policy_path,
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
approval_policy
|
||||
} else if let Some(approval_policy) = valid_lenient_ref(
|
||||
&cfg.approval_policy,
|
||||
"approval_policy",
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
approval_policy
|
||||
} else {
|
||||
if active_project.is_trusted() {
|
||||
AskForApproval::OnRequest
|
||||
} else if active_project.is_untrusted() {
|
||||
@@ -2517,7 +2557,7 @@ impl Config {
|
||||
} else {
|
||||
AskForApproval::default()
|
||||
}
|
||||
});
|
||||
};
|
||||
if !approval_policy_was_explicit
|
||||
&& let Err(err) = constrained_approval_policy.can_set(&approval_policy)
|
||||
{
|
||||
@@ -2528,12 +2568,21 @@ impl Config {
|
||||
approval_policy = constrained_approval_policy.value();
|
||||
}
|
||||
let approvals_reviewer_was_explicit = approvals_reviewer_override.is_some()
|
||||
|| valid_lenient_ref(&config_profile.approvals_reviewer).is_some()
|
||||
|| valid_lenient_ref(&cfg.approvals_reviewer).is_some();
|
||||
let mut approvals_reviewer = approvals_reviewer_override
|
||||
.or_else(|| valid_lenient_ref(&config_profile.approvals_reviewer))
|
||||
.or_else(|| valid_lenient_ref(&cfg.approvals_reviewer))
|
||||
.unwrap_or(ApprovalsReviewer::User);
|
||||
|| 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) = valid_lenient_ref(
|
||||
&config_profile.approvals_reviewer,
|
||||
&profile_approvals_reviewer_path,
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
approvals_reviewer
|
||||
} else {
|
||||
valid_lenient_ref(&cfg.approvals_reviewer, "approvals_reviewer", &mut startup_warnings)
|
||||
.unwrap_or(ApprovalsReviewer::User)
|
||||
};
|
||||
if !approvals_reviewer_was_explicit
|
||||
&& let Err(err) = constrained_approvals_reviewer.can_set(&approvals_reviewer)
|
||||
{
|
||||
@@ -2543,7 +2592,17 @@ impl Config {
|
||||
);
|
||||
approvals_reviewer = constrained_approvals_reviewer.value();
|
||||
}
|
||||
let web_search_mode = resolve_web_search_mode(&cfg, &config_profile, &features)
|
||||
let profile_web_search_path =
|
||||
active_profile_field_path(active_profile_name.as_deref(), "web_search");
|
||||
let configured_web_search_mode = match valid_lenient_ref(
|
||||
&config_profile.web_search,
|
||||
&profile_web_search_path,
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
Some(mode) => Some(mode),
|
||||
None => valid_lenient_ref(&cfg.web_search, "web_search", &mut startup_warnings),
|
||||
};
|
||||
let web_search_mode = resolve_web_search_mode(configured_web_search_mode, &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);
|
||||
@@ -2714,7 +2773,11 @@ impl Config {
|
||||
}
|
||||
});
|
||||
|
||||
let forced_login_method = valid_lenient(cfg.forced_login_method.clone());
|
||||
let forced_login_method = valid_lenient(
|
||||
cfg.forced_login_method.clone(),
|
||||
"forced_login_method",
|
||||
&mut startup_warnings,
|
||||
);
|
||||
|
||||
let model = model.or(config_profile.model).or(cfg.model);
|
||||
let mut notices = cfg.notice.unwrap_or_default();
|
||||
@@ -2726,8 +2789,20 @@ impl Config {
|
||||
notices.fast_default_opt_out = Some(true);
|
||||
None
|
||||
}
|
||||
None => valid_lenient_ref(&config_profile.service_tier)
|
||||
.or_else(|| valid_lenient_ref(&cfg.service_tier)),
|
||||
None => {
|
||||
let profile_service_tier_path =
|
||||
active_profile_field_path(active_profile_name.as_deref(), "service_tier");
|
||||
match valid_lenient_ref(
|
||||
&config_profile.service_tier,
|
||||
&profile_service_tier_path,
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
Some(service_tier) => Some(service_tier),
|
||||
None => {
|
||||
valid_lenient_ref(&cfg.service_tier, "service_tier", &mut startup_warnings)
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
let service_tier = match service_tier {
|
||||
Some(ServiceTier::Fast) if features.enabled(Feature::FastMode) => {
|
||||
@@ -2792,14 +2867,25 @@ impl Config {
|
||||
auto_review.policy.as_deref(),
|
||||
))
|
||||
});
|
||||
let personality = personality
|
||||
.or_else(|| valid_lenient_ref(&config_profile.personality))
|
||||
.or_else(|| valid_lenient_ref(&cfg.personality))
|
||||
.or_else(|| {
|
||||
features
|
||||
.enabled(Feature::Personality)
|
||||
.then_some(Personality::Pragmatic)
|
||||
});
|
||||
let personality = if personality.is_some() {
|
||||
personality
|
||||
} else {
|
||||
let profile_personality_path =
|
||||
active_profile_field_path(active_profile_name.as_deref(), "personality");
|
||||
match valid_lenient_ref(
|
||||
&config_profile.personality,
|
||||
&profile_personality_path,
|
||||
&mut startup_warnings,
|
||||
) {
|
||||
Some(personality) => Some(personality),
|
||||
None => valid_lenient_ref(&cfg.personality, "personality", &mut startup_warnings)
|
||||
.or_else(|| {
|
||||
features
|
||||
.enabled(Feature::Personality)
|
||||
.then_some(Personality::Pragmatic)
|
||||
}),
|
||||
}
|
||||
};
|
||||
|
||||
let experimental_compact_prompt_path = config_profile
|
||||
.experimental_compact_prompt_file
|
||||
@@ -2963,6 +3049,96 @@ 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(
|
||||
valid_lenient(
|
||||
cfg.cli_auth_credentials_store.clone(),
|
||||
"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(
|
||||
valid_lenient(
|
||||
cfg.mcp_oauth_credentials_store.clone(),
|
||||
"mcp_oauth_credentials_store",
|
||||
&mut startup_warnings,
|
||||
)
|
||||
.unwrap_or_default(),
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
);
|
||||
let file_opener = valid_lenient(
|
||||
cfg.file_opener,
|
||||
"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 = valid_lenient(
|
||||
config_profile.model_reasoning_effort,
|
||||
&model_reasoning_effort_path,
|
||||
&mut startup_warnings,
|
||||
)
|
||||
.or_else(|| {
|
||||
valid_lenient(
|
||||
cfg.model_reasoning_effort,
|
||||
"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 = valid_lenient(
|
||||
config_profile.plan_mode_reasoning_effort,
|
||||
&plan_mode_reasoning_effort_path,
|
||||
&mut startup_warnings,
|
||||
)
|
||||
.or_else(|| {
|
||||
valid_lenient(
|
||||
cfg.plan_mode_reasoning_effort,
|
||||
"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 = valid_lenient(
|
||||
config_profile.model_reasoning_summary,
|
||||
&model_reasoning_summary_path,
|
||||
&mut startup_warnings,
|
||||
)
|
||||
.or_else(|| {
|
||||
valid_lenient(
|
||||
cfg.model_reasoning_summary,
|
||||
"model_reasoning_summary",
|
||||
&mut startup_warnings,
|
||||
)
|
||||
});
|
||||
let model_verbosity_path =
|
||||
active_profile_field_path(active_profile_name.as_deref(), "model_verbosity");
|
||||
let model_verbosity = valid_lenient(
|
||||
config_profile.model_verbosity,
|
||||
&model_verbosity_path,
|
||||
&mut startup_warnings,
|
||||
)
|
||||
.or_else(|| {
|
||||
valid_lenient(
|
||||
cfg.model_verbosity,
|
||||
"model_verbosity",
|
||||
&mut startup_warnings,
|
||||
)
|
||||
});
|
||||
let experimental_thread_store = thread_store_config(
|
||||
valid_lenient(
|
||||
cfg.experimental_thread_store,
|
||||
"experimental_thread_store",
|
||||
&mut startup_warnings,
|
||||
),
|
||||
cfg.experimental_thread_store_endpoint,
|
||||
);
|
||||
let config = Self {
|
||||
model,
|
||||
service_tier,
|
||||
@@ -2998,17 +3174,11 @@ 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: resolve_cli_auth_credentials_store_mode(
|
||||
valid_lenient(cfg.cli_auth_credentials_store.clone()).unwrap_or_default(),
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
cli_auth_credentials_store_mode,
|
||||
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: resolve_mcp_oauth_credentials_store_mode(
|
||||
valid_lenient(cfg.mcp_oauth_credentials_store.clone()).unwrap_or_default(),
|
||||
env!("CARGO_PKG_VERSION"),
|
||||
),
|
||||
mcp_oauth_credentials_store_mode,
|
||||
mcp_oauth_callback_port: cfg.mcp_oauth_callback_port,
|
||||
mcp_oauth_callback_url: cfg.mcp_oauth_callback_url.clone(),
|
||||
model_providers,
|
||||
@@ -3057,7 +3227,7 @@ impl Config {
|
||||
config_layer_stack,
|
||||
history,
|
||||
ephemeral: ephemeral.unwrap_or_default(),
|
||||
file_opener: valid_lenient(cfg.file_opener).unwrap_or(UriBasedFileOpener::VsCode),
|
||||
file_opener,
|
||||
codex_self_exe,
|
||||
codex_linux_sandbox_exe,
|
||||
main_execve_wrapper_exe,
|
||||
@@ -3069,16 +3239,12 @@ impl Config {
|
||||
.or(show_raw_agent_reasoning)
|
||||
.unwrap_or(false),
|
||||
guardian_policy_config,
|
||||
model_reasoning_effort: valid_lenient(config_profile.model_reasoning_effort)
|
||||
.or_else(|| valid_lenient(cfg.model_reasoning_effort)),
|
||||
plan_mode_reasoning_effort: valid_lenient(config_profile.plan_mode_reasoning_effort)
|
||||
.or_else(|| valid_lenient(cfg.plan_mode_reasoning_effort)),
|
||||
model_reasoning_summary: valid_lenient(config_profile.model_reasoning_summary)
|
||||
.or_else(|| valid_lenient(cfg.model_reasoning_summary)),
|
||||
model_reasoning_effort,
|
||||
plan_mode_reasoning_effort,
|
||||
model_reasoning_summary,
|
||||
model_supports_reasoning_summaries: cfg.model_supports_reasoning_summaries,
|
||||
model_catalog,
|
||||
model_verbosity: valid_lenient(config_profile.model_verbosity)
|
||||
.or_else(|| valid_lenient(cfg.model_verbosity)),
|
||||
model_verbosity,
|
||||
chatgpt_base_url: config_profile
|
||||
.chatgpt_base_url
|
||||
.or(cfg.chatgpt_base_url)
|
||||
@@ -3107,10 +3273,7 @@ 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: thread_store_config(
|
||||
valid_lenient(cfg.experimental_thread_store),
|
||||
cfg.experimental_thread_store_endpoint,
|
||||
),
|
||||
experimental_thread_store,
|
||||
forced_chatgpt_workspace_id,
|
||||
forced_login_method,
|
||||
include_apply_patch_tool: include_apply_patch_tool_flag,
|
||||
|
||||
@@ -17,7 +17,7 @@ use tokio::io::AsyncWriteExt;
|
||||
const TEST_TIMESTAMP: &str = "2025-01-01T00-00-00";
|
||||
|
||||
fn valid_lenient<T>(value: Option<Lenient<T>>) -> Option<T> {
|
||||
value.and_then(Lenient::into_valid)
|
||||
value.and_then(|value| value.into_valid("personality", None))
|
||||
}
|
||||
|
||||
async fn read_config_toml(codex_home: &Path) -> io::Result<ConfigToml> {
|
||||
|
||||
@@ -149,7 +149,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
lock.model_reasoning_effort
|
||||
.clone()
|
||||
.and_then(Lenient::into_valid),
|
||||
.and_then(|value| value.into_valid("model_reasoning_effort", None)),
|
||||
sc.collaboration_mode.reasoning_effort()
|
||||
);
|
||||
assert_eq!(lock.profile, None);
|
||||
|
||||
@@ -23,7 +23,7 @@ use tokio::io::AsyncWriteExt;
|
||||
const TEST_TIMESTAMP: &str = "2025-01-01T00-00-00";
|
||||
|
||||
fn valid_lenient<T>(value: Option<Lenient<T>>) -> Option<T> {
|
||||
value.and_then(Lenient::into_valid)
|
||||
value.and_then(|value| value.into_valid("personality", None))
|
||||
}
|
||||
|
||||
async fn read_config_toml(codex_home: &Path) -> io::Result<ConfigToml> {
|
||||
|
||||
Reference in New Issue
Block a user