mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Tried another approach — a lenient decoder
This commit is contained in:
2
codex-rs/Cargo.lock
generated
2
codex-rs/Cargo.lock
generated
@@ -1323,7 +1323,7 @@ dependencies = [
|
||||
"seccompiler",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_with",
|
||||
"serde_path_to_error",
|
||||
"serde_yaml",
|
||||
"serial_test",
|
||||
"sha1",
|
||||
|
||||
@@ -187,6 +187,7 @@ seccompiler = "0.5.0"
|
||||
sentry = "0.46.0"
|
||||
serde = "1"
|
||||
serde_json = "1"
|
||||
serde_path_to_error = "0.1"
|
||||
serde_with = "3.16"
|
||||
serde_yaml = "0.9"
|
||||
serial_test = "3.2.0"
|
||||
|
||||
@@ -57,7 +57,7 @@ regex-lite = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_with = { workspace = true }
|
||||
serde_path_to_error = { workspace = true }
|
||||
serde_yaml = { workspace = true }
|
||||
sha1 = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
|
||||
@@ -45,8 +45,6 @@ use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use dirs::home_dir;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_with::DefaultOnError;
|
||||
use serde_with::serde_as;
|
||||
use similar::DiffableStr;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
@@ -433,9 +431,7 @@ 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 config_toml: ConfigToml = merged_toml
|
||||
.try_into()
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
let config_toml = deserialize_config_toml_lenient(merged_toml, None)?;
|
||||
Config::load_config_with_layer_stack(
|
||||
config_toml,
|
||||
harness_overrides,
|
||||
@@ -492,7 +488,7 @@ pub async fn load_config_as_toml_with_cli_overrides(
|
||||
.await?;
|
||||
|
||||
let merged_toml = config_layer_stack.effective_config();
|
||||
let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| {
|
||||
let cfg = deserialize_config_toml_lenient(merged_toml, Some(codex_home)).map_err(|e| {
|
||||
tracing::error!("Failed to deserialize overridden config: {e}");
|
||||
e
|
||||
})?;
|
||||
@@ -500,6 +496,7 @@ pub async fn load_config_as_toml_with_cli_overrides(
|
||||
Ok(cfg)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn deserialize_config_toml_with_base(
|
||||
root_value: TomlValue,
|
||||
config_base_dir: &Path,
|
||||
@@ -512,6 +509,97 @@ fn deserialize_config_toml_with_base(
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
|
||||
}
|
||||
|
||||
fn deserialize_config_toml_lenient(
|
||||
mut root_value: TomlValue,
|
||||
config_base_dir: Option<&Path>,
|
||||
) -> std::io::Result<ConfigToml> {
|
||||
loop {
|
||||
let attempt = match config_base_dir {
|
||||
Some(base_dir) => {
|
||||
let _guard = AbsolutePathBufGuard::new(base_dir);
|
||||
deserialize_config_toml_with_path(&root_value)
|
||||
}
|
||||
None => deserialize_config_toml_with_path(&root_value),
|
||||
};
|
||||
|
||||
match attempt {
|
||||
Ok(config) => return Ok(config),
|
||||
Err(err) => {
|
||||
let path = err.path().clone();
|
||||
let inner = err.into_inner();
|
||||
if !is_unknown_enum_error(&inner) {
|
||||
return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, inner));
|
||||
}
|
||||
if path.to_string().is_empty() || !remove_toml_path(&mut root_value, &path) {
|
||||
return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, inner));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn deserialize_config_toml_with_path(
|
||||
value: &TomlValue,
|
||||
) -> Result<ConfigToml, serde_path_to_error::Error<toml::de::Error>> {
|
||||
serde_path_to_error::deserialize(value.clone())
|
||||
}
|
||||
|
||||
fn is_unknown_enum_error(error: &toml::de::Error) -> bool {
|
||||
error.to_string().contains("unknown variant")
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum TomlPathSegment {
|
||||
Key(String),
|
||||
Index(usize),
|
||||
}
|
||||
|
||||
fn remove_toml_path(root: &mut TomlValue, path: &serde_path_to_error::Path) -> bool {
|
||||
let segments: Vec<TomlPathSegment> = path
|
||||
.iter()
|
||||
.filter_map(|segment| match segment {
|
||||
serde_path_to_error::Segment::Seq { index } => Some(TomlPathSegment::Index(*index)),
|
||||
serde_path_to_error::Segment::Map { key } => Some(TomlPathSegment::Key(key.clone())),
|
||||
serde_path_to_error::Segment::Enum { .. } => None,
|
||||
serde_path_to_error::Segment::Unknown => None,
|
||||
})
|
||||
.collect();
|
||||
|
||||
if segments.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let mut current = root;
|
||||
for segment in &segments[..segments.len() - 1] {
|
||||
current = match (current, segment) {
|
||||
(TomlValue::Table(table), TomlPathSegment::Key(key)) => match table.get_mut(key) {
|
||||
Some(value) => value,
|
||||
None => return false,
|
||||
},
|
||||
(TomlValue::Array(items), TomlPathSegment::Index(index)) => {
|
||||
match items.get_mut(*index) {
|
||||
Some(value) => value,
|
||||
None => return false,
|
||||
}
|
||||
}
|
||||
_ => return false,
|
||||
};
|
||||
}
|
||||
|
||||
match (current, segments.last()) {
|
||||
(TomlValue::Table(table), Some(TomlPathSegment::Key(key))) => table.remove(key).is_some(),
|
||||
(TomlValue::Array(items), Some(TomlPathSegment::Index(index))) => {
|
||||
if *index < items.len() {
|
||||
items.remove(*index);
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn load_global_mcp_servers(
|
||||
codex_home: &Path,
|
||||
) -> std::io::Result<BTreeMap<String, McpServerConfig>> {
|
||||
@@ -689,7 +777,6 @@ pub fn set_default_oss_provider(codex_home: &Path, provider: &str) -> std::io::R
|
||||
}
|
||||
|
||||
/// Base config deserialized from ~/.codex/config.toml.
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq)]
|
||||
pub struct ConfigToml {
|
||||
/// Optional override of model selection.
|
||||
@@ -708,7 +795,6 @@ pub struct ConfigToml {
|
||||
|
||||
/// Default approval policy for executing commands.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
|
||||
#[serde(default)]
|
||||
@@ -716,7 +802,6 @@ pub struct ConfigToml {
|
||||
|
||||
/// Sandbox mode to use.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
|
||||
/// Sandbox configuration to apply if `sandbox` is `WorkspaceWrite`.
|
||||
@@ -742,7 +827,6 @@ pub struct ConfigToml {
|
||||
|
||||
/// When set, restricts the login mechanism users may use.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub forced_login_method: Option<ForcedLoginMethod>,
|
||||
|
||||
/// Preferred backend for storing CLI auth credentials.
|
||||
@@ -750,7 +834,6 @@ pub struct ConfigToml {
|
||||
/// keyring: Use an OS-specific keyring service.
|
||||
/// auto: Use the keyring if available, otherwise use a file.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub cli_auth_credentials_store: Option<AuthCredentialsStoreMode>,
|
||||
|
||||
/// Definition for MCP servers that Codex can reach out to for tool calls.
|
||||
@@ -763,7 +846,6 @@ 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)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub mcp_oauth_credentials_store: Option<OAuthCredentialsStoreMode>,
|
||||
|
||||
/// Optional fixed port for the local HTTP callback server used during MCP OAuth login.
|
||||
@@ -797,7 +879,6 @@ 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.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub file_opener: Option<UriBasedFileOpener>,
|
||||
|
||||
/// Collection of settings that are specific to the TUI.
|
||||
@@ -812,14 +893,11 @@ pub struct ConfigToml {
|
||||
pub show_raw_agent_reasoning: Option<bool>,
|
||||
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_reasoning_effort: Option<ReasoningEffort>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_reasoning_summary: Option<ReasoningSummary>,
|
||||
/// Optional verbosity control for GPT-5 models (Responses API `text.verbosity`).
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_verbosity: Option<Verbosity>,
|
||||
|
||||
/// Override to force-enable reasoning summaries for the configured model.
|
||||
@@ -908,11 +986,9 @@ impl From<ConfigToml> for UserSavedConfig {
|
||||
}
|
||||
}
|
||||
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ProjectConfig {
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub trust_level: Option<TrustLevel>,
|
||||
}
|
||||
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_with::DefaultOnError;
|
||||
use serde_with::serde_as;
|
||||
|
||||
use crate::protocol::AskForApproval;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
@@ -12,7 +10,6 @@ use codex_protocol::openai_models::ReasoningEffort;
|
||||
|
||||
/// Collection of common configuration options that a user can define as a unit
|
||||
/// in `config.toml`.
|
||||
#[serde_as]
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
pub struct ConfigProfile {
|
||||
pub model: Option<String>,
|
||||
@@ -20,19 +17,14 @@ pub struct ConfigProfile {
|
||||
/// [`ModelProviderInfo`] to use.
|
||||
pub model_provider: Option<String>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_reasoning_effort: Option<ReasoningEffort>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_reasoning_summary: Option<ReasoningSummary>,
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub model_verbosity: Option<Verbosity>,
|
||||
pub chatgpt_base_url: Option<String>,
|
||||
pub experimental_instructions_file: Option<AbsolutePathBuf>,
|
||||
|
||||
@@ -941,7 +941,7 @@ remote_compaction = true
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_user_value_overridden_by_managed_is_accepted() {
|
||||
async fn invalid_user_value_rejected_even_if_overridden_by_managed() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"").unwrap();
|
||||
|
||||
@@ -959,7 +959,7 @@ remote_compaction = true
|
||||
},
|
||||
);
|
||||
|
||||
let response = service
|
||||
let error = service
|
||||
.write_value(ConfigValueWriteParams {
|
||||
file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()),
|
||||
key_path: "approval_policy".to_string(),
|
||||
@@ -968,13 +968,16 @@ remote_compaction = true
|
||||
expected_version: None,
|
||||
})
|
||||
.await
|
||||
.expect("write succeeds");
|
||||
.expect_err("should fail validation");
|
||||
|
||||
assert_eq!(response.status, WriteStatus::OkOverridden);
|
||||
assert_eq!(
|
||||
error.write_error_code(),
|
||||
Some(ConfigWriteErrorCode::ConfigValidationError)
|
||||
);
|
||||
|
||||
let contents =
|
||||
std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config");
|
||||
assert!(contents.contains("approval_policy = \"bogus\""));
|
||||
assert_eq!(contents.trim(), "model = \"user\"");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -15,8 +15,6 @@ use serde::Deserialize;
|
||||
use serde::Deserializer;
|
||||
use serde::Serialize;
|
||||
use serde::de::Error as SerdeError;
|
||||
use serde_with::DefaultOnError;
|
||||
use serde_with::serde_as;
|
||||
|
||||
pub const DEFAULT_OTEL_ENVIRONMENT: &str = "dev";
|
||||
|
||||
@@ -256,11 +254,9 @@ impl UriBasedFileOpener {
|
||||
}
|
||||
|
||||
/// Settings that govern if and what will be written to `~/.codex/history.jsonl`.
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)]
|
||||
pub struct History {
|
||||
/// If true, history entries will not be written to disk.
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub persistence: HistoryPersistence,
|
||||
|
||||
/// If set, the maximum size of the history file in bytes. The oldest entries
|
||||
@@ -336,7 +332,6 @@ pub enum OtelExporterKind {
|
||||
}
|
||||
|
||||
/// OTEL settings loaded from config.toml. Fields are optional so we can apply defaults.
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)]
|
||||
pub struct OtelConfigToml {
|
||||
/// Log user prompt in traces
|
||||
@@ -347,12 +342,10 @@ pub struct OtelConfigToml {
|
||||
|
||||
/// Optional log exporter
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub exporter: Option<OtelExporterKind>,
|
||||
|
||||
/// Optional trace exporter
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub trace_exporter: Option<OtelExporterKind>,
|
||||
}
|
||||
|
||||
@@ -414,13 +407,11 @@ impl Default for ScrollInputMode {
|
||||
}
|
||||
|
||||
/// Collection of settings that are specific to the TUI.
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)]
|
||||
pub struct Tui {
|
||||
/// Enable desktop notifications from the TUI when the terminal is unfocused.
|
||||
/// Defaults to `true`.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub notifications: Notifications,
|
||||
|
||||
/// Enable animations (welcome screen, shimmer effects, spinners).
|
||||
@@ -510,7 +501,6 @@ pub struct Tui {
|
||||
/// - `wheel`: always use wheel behavior (fixed lines per wheel notch).
|
||||
/// - `trackpad`: always use trackpad behavior (fractional accumulation; wheel may feel slow).
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub scroll_mode: ScrollInputMode,
|
||||
|
||||
/// Auto-mode threshold: maximum time (ms) for the first tick-worth of events to arrive.
|
||||
@@ -546,7 +536,6 @@ pub struct Tui {
|
||||
/// Using alternate screen provides a cleaner fullscreen experience but prevents
|
||||
/// scrollback in terminal multiplexers like Zellij that follow the xterm spec.
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub alternate_screen: AltScreenMode,
|
||||
}
|
||||
|
||||
@@ -620,11 +609,9 @@ pub enum ShellEnvironmentPolicyInherit {
|
||||
|
||||
/// Policy for building the `env` when spawning a process via either the
|
||||
/// `shell` or `local_shell` tool.
|
||||
#[serde_as]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)]
|
||||
pub struct ShellEnvironmentPolicyToml {
|
||||
#[serde(default)]
|
||||
#[serde_as(as = "DefaultOnError")]
|
||||
pub inherit: Option<ShellEnvironmentPolicyInherit>,
|
||||
|
||||
pub ignore_default_excludes: Option<bool>,
|
||||
|
||||
Reference in New Issue
Block a user