From db31f6966d1eb625a3ace5b8ad8be1c697d36acd Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 29 Oct 2025 20:52:46 +0000 Subject: [PATCH] chore: config editor (#5878) The goal is to have a single place where we actually write files In a follow-up PR, will move everything config related in a dedicated module and move the helpers in a dedicated file --- .../app-server/src/codex_message_processor.rs | 23 +- codex-rs/cli/src/mcp_cmd.rs | 12 +- codex-rs/cli/tests/mcp_list.rs | 10 +- codex-rs/core/src/config.rs | 504 ++---- codex-rs/core/src/config_edit.rs | 1460 ++++++++++------- codex-rs/core/src/config_types.rs | 2 +- codex-rs/tui/src/app.rs | 14 +- codex-rs/tui/src/onboarding/windows.rs | 7 +- 8 files changed, 976 insertions(+), 1056 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index fabdf03c..eb895820 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -74,9 +74,7 @@ use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; use codex_core::config::load_config_as_toml; -use codex_core::config_edit::CONFIG_KEY_EFFORT; -use codex_core::config_edit::CONFIG_KEY_MODEL; -use codex_core::config_edit::persist_overrides_and_clear_if_none; +use codex_core::config_edit::ConfigEditsBuilder; use codex_core::default_client::get_codex_user_agent; use codex_core::exec::ExecParams; use codex_core::exec_env::create_env; @@ -689,19 +687,12 @@ impl CodexMessageProcessor { model, reasoning_effort, } = params; - let effort_str = reasoning_effort.map(|effort| effort.to_string()); - let overrides: [(&[&str], Option<&str>); 2] = [ - (&[CONFIG_KEY_MODEL], model.as_deref()), - (&[CONFIG_KEY_EFFORT], effort_str.as_deref()), - ]; - - match persist_overrides_and_clear_if_none( - &self.config.codex_home, - self.config.active_profile.as_deref(), - &overrides, - ) - .await + match ConfigEditsBuilder::new(&self.config.codex_home) + .with_profile(self.config.active_profile.as_deref()) + .set_model(model.as_deref(), reasoning_effort) + .apply() + .await { Ok(()) => { let response = SetDefaultModelResponse {}; @@ -710,7 +701,7 @@ impl CodexMessageProcessor { Err(err) => { let error = JSONRPCErrorError { code: INTERNAL_ERROR_CODE, - message: format!("failed to persist overrides: {err}"), + message: format!("failed to persist model selection: {err}"), data: None, }; self.outgoing.send_error(request_id, error).await; diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 21676dec..96a0759e 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -11,7 +11,7 @@ use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::find_codex_home; use codex_core::config::load_global_mcp_servers; -use codex_core::config::write_global_mcp_servers; +use codex_core::config_edit::ConfigEditsBuilder; use codex_core::config_types::McpServerConfig; use codex_core::config_types::McpServerTransportConfig; use codex_core::features::Feature; @@ -263,7 +263,10 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re servers.insert(name.clone(), new_entry); - write_global_mcp_servers(&codex_home, &servers) + ConfigEditsBuilder::new(&codex_home) + .replace_mcp_servers(&servers) + .apply() + .await .with_context(|| format!("failed to write MCP servers to {}", codex_home.display()))?; println!("Added global MCP server '{name}'."); @@ -321,7 +324,10 @@ async fn run_remove(config_overrides: &CliConfigOverrides, remove_args: RemoveAr let removed = servers.remove(&name).is_some(); if removed { - write_global_mcp_servers(&codex_home, &servers) + ConfigEditsBuilder::new(&codex_home) + .replace_mcp_servers(&servers) + .apply() + .await .with_context(|| format!("failed to write MCP servers to {}", codex_home.display()))?; } diff --git a/codex-rs/cli/tests/mcp_list.rs b/codex-rs/cli/tests/mcp_list.rs index 553c7315..2293e701 100644 --- a/codex-rs/cli/tests/mcp_list.rs +++ b/codex-rs/cli/tests/mcp_list.rs @@ -2,7 +2,7 @@ use std::path::Path; use anyhow::Result; use codex_core::config::load_global_mcp_servers; -use codex_core::config::write_global_mcp_servers; +use codex_core::config_edit::ConfigEditsBuilder; use codex_core::config_types::McpServerTransportConfig; use predicates::prelude::PredicateBooleanExt; use predicates::str::contains; @@ -59,7 +59,9 @@ async fn list_and_get_render_expected_output() -> Result<()> { } other => panic!("unexpected transport: {other:?}"), } - write_global_mcp_servers(codex_home.path(), &servers)?; + ConfigEditsBuilder::new(codex_home.path()) + .replace_mcp_servers(&servers) + .apply_blocking()?; let mut list_cmd = codex_command(codex_home.path())?; let list_output = list_cmd.args(["mcp", "list"]).output()?; @@ -149,7 +151,9 @@ async fn get_disabled_server_shows_single_line() -> Result<()> { .get_mut("docs") .expect("docs server should exist after add"); docs.enabled = false; - write_global_mcp_servers(codex_home.path(), &servers)?; + ConfigEditsBuilder::new(codex_home.path()) + .replace_mcp_servers(&servers) + .apply_blocking()?; let mut get_cmd = codex_command(codex_home.path())?; let get_output = get_cmd.args(["mcp", "get", "docs"]).output()?; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index f671e8dd..c2510dc7 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -7,7 +7,6 @@ use crate::config_profile::ConfigProfile; use crate::config_types::DEFAULT_OTEL_ENVIRONMENT; use crate::config_types::History; use crate::config_types::McpServerConfig; -use crate::config_types::McpServerTransportConfig; use crate::config_types::Notice; use crate::config_types::Notifications; use crate::config_types::OtelConfig; @@ -34,7 +33,6 @@ use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; -use anyhow::Context; use codex_app_server_protocol::Tools; use codex_app_server_protocol::UserSavedConfig; use codex_protocol::config_types::ForcedLoginMethod; @@ -53,12 +51,8 @@ use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; -use tempfile::NamedTempFile; use toml::Value as TomlValue; -use toml_edit::Array as TomlArray; use toml_edit::DocumentMut; -use toml_edit::Item as TomlItem; -use toml_edit::Table as TomlTable; #[cfg(target_os = "windows")] pub const OPENAI_DEFAULT_MODEL: &str = "gpt-5"; @@ -383,141 +377,10 @@ fn ensure_no_inline_bearer_tokens(value: &TomlValue) -> std::io::Result<()> { Ok(()) } -pub fn write_global_mcp_servers( - codex_home: &Path, - servers: &BTreeMap, -) -> std::io::Result<()> { - let config_path = codex_home.join(CONFIG_TOML_FILE); - let mut doc = match std::fs::read_to_string(&config_path) { - Ok(contents) => contents - .parse::() - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => DocumentMut::new(), - Err(e) => return Err(e), - }; - - doc.as_table_mut().remove("mcp_servers"); - - if !servers.is_empty() { - let mut table = TomlTable::new(); - table.set_implicit(true); - doc["mcp_servers"] = TomlItem::Table(table); - - for (name, config) in servers { - let mut entry = TomlTable::new(); - entry.set_implicit(false); - match &config.transport { - McpServerTransportConfig::Stdio { - command, - args, - env, - env_vars, - cwd, - } => { - entry["command"] = toml_edit::value(command.clone()); - - if !args.is_empty() { - let mut args_array = TomlArray::new(); - for arg in args { - args_array.push(arg.clone()); - } - entry["args"] = TomlItem::Value(args_array.into()); - } - - if let Some(env) = env - && !env.is_empty() - { - let mut env_table = TomlTable::new(); - env_table.set_implicit(false); - let mut pairs: Vec<_> = env.iter().collect(); - pairs.sort_by(|(a, _), (b, _)| a.cmp(b)); - for (key, value) in pairs { - env_table.insert(key, toml_edit::value(value.clone())); - } - entry["env"] = TomlItem::Table(env_table); - } - - if !env_vars.is_empty() { - entry["env_vars"] = - TomlItem::Value(env_vars.iter().collect::().into()); - } - - if let Some(cwd) = cwd { - entry["cwd"] = toml_edit::value(cwd.to_string_lossy().to_string()); - } - } - McpServerTransportConfig::StreamableHttp { - url, - bearer_token_env_var, - http_headers, - env_http_headers, - } => { - entry["url"] = toml_edit::value(url.clone()); - if let Some(env_var) = bearer_token_env_var { - entry["bearer_token_env_var"] = toml_edit::value(env_var.clone()); - } - if let Some(headers) = http_headers - && !headers.is_empty() - { - let mut table = TomlTable::new(); - table.set_implicit(false); - let mut pairs: Vec<_> = headers.iter().collect(); - pairs.sort_by(|(a, _), (b, _)| a.cmp(b)); - for (key, value) in pairs { - table.insert(key, toml_edit::value(value.clone())); - } - entry["http_headers"] = TomlItem::Table(table); - } - if let Some(headers) = env_http_headers - && !headers.is_empty() - { - let mut table = TomlTable::new(); - table.set_implicit(false); - let mut pairs: Vec<_> = headers.iter().collect(); - pairs.sort_by(|(a, _), (b, _)| a.cmp(b)); - for (key, value) in pairs { - table.insert(key, toml_edit::value(value.clone())); - } - entry["env_http_headers"] = TomlItem::Table(table); - } - } - } - - if !config.enabled { - entry["enabled"] = toml_edit::value(false); - } - - if let Some(timeout) = config.startup_timeout_sec { - entry["startup_timeout_sec"] = toml_edit::value(timeout.as_secs_f64()); - } - - if let Some(timeout) = config.tool_timeout_sec { - entry["tool_timeout_sec"] = toml_edit::value(timeout.as_secs_f64()); - } - - if let Some(enabled_tools) = &config.enabled_tools { - entry["enabled_tools"] = - TomlItem::Value(enabled_tools.iter().collect::().into()); - } - - if let Some(disabled_tools) = &config.disabled_tools { - entry["disabled_tools"] = - TomlItem::Value(disabled_tools.iter().collect::().into()); - } - - doc["mcp_servers"][name.as_str()] = TomlItem::Table(entry); - } - } - - std::fs::create_dir_all(codex_home)?; - let tmp_file = NamedTempFile::new_in(codex_home)?; - std::fs::write(tmp_file.path(), doc.to_string())?; - tmp_file.persist(config_path).map_err(|err| err.error)?; - - Ok(()) -} - -fn set_project_trusted_inner(doc: &mut DocumentMut, project_path: &Path) -> anyhow::Result<()> { +pub(crate) fn set_project_trusted_inner( + doc: &mut DocumentMut, + project_path: &Path, +) -> anyhow::Result<()> { // Ensure we render a human-friendly structure: // // [projects] @@ -585,209 +448,11 @@ fn set_project_trusted_inner(doc: &mut DocumentMut, project_path: &Path) -> anyh /// Patch `CODEX_HOME/config.toml` project state. /// Use with caution. pub fn set_project_trusted(codex_home: &Path, project_path: &Path) -> anyhow::Result<()> { - let config_path = codex_home.join(CONFIG_TOML_FILE); - // Parse existing config if present; otherwise start a new document. - let mut doc = match std::fs::read_to_string(config_path.clone()) { - Ok(s) => s.parse::()?, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => DocumentMut::new(), - Err(e) => return Err(e.into()), - }; + use crate::config_edit::ConfigEditsBuilder; - set_project_trusted_inner(&mut doc, project_path)?; - - // ensure codex_home exists - std::fs::create_dir_all(codex_home)?; - - // create a tmp_file - let tmp_file = NamedTempFile::new_in(codex_home)?; - std::fs::write(tmp_file.path(), doc.to_string())?; - - // atomically move the tmp file into config.toml - tmp_file.persist(config_path)?; - - Ok(()) -} - -/// Persist the acknowledgement flag for the Windows onboarding screen. -pub fn set_windows_wsl_setup_acknowledged( - codex_home: &Path, - acknowledged: bool, -) -> anyhow::Result<()> { - let config_path = codex_home.join(CONFIG_TOML_FILE); - let mut doc = match std::fs::read_to_string(config_path.clone()) { - Ok(s) => s.parse::()?, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => DocumentMut::new(), - Err(e) => return Err(e.into()), - }; - - doc["windows_wsl_setup_acknowledged"] = toml_edit::value(acknowledged); - - std::fs::create_dir_all(codex_home)?; - - let tmp_file = NamedTempFile::new_in(codex_home)?; - std::fs::write(tmp_file.path(), doc.to_string())?; - tmp_file.persist(config_path)?; - - Ok(()) -} - -/// Persist the acknowledgement flag for the full access warning prompt. -pub fn set_hide_full_access_warning(codex_home: &Path, acknowledged: bool) -> anyhow::Result<()> { - let config_path = codex_home.join(CONFIG_TOML_FILE); - let mut doc = match std::fs::read_to_string(config_path.clone()) { - Ok(s) => s.parse::()?, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => DocumentMut::new(), - Err(e) => return Err(e.into()), - }; - - let notices_table = load_or_create_top_level_table(&mut doc, Notice::TABLE_KEY)?; - - notices_table["hide_full_access_warning"] = toml_edit::value(acknowledged); - - std::fs::create_dir_all(codex_home)?; - let tmp_file = NamedTempFile::new_in(codex_home)?; - std::fs::write(tmp_file.path(), doc.to_string())?; - tmp_file.persist(config_path)?; - - Ok(()) -} - -fn load_or_create_top_level_table<'a>( - doc: &'a mut DocumentMut, - key: &str, -) -> anyhow::Result<&'a mut toml_edit::Table> { - let mut created_table = false; - - let root = doc.as_table_mut(); - let needs_table = - !root.contains_key(key) || root.get(key).and_then(|item| item.as_table()).is_none(); - if needs_table { - root.insert(key, toml_edit::table()); - created_table = true; - } - - let Some(table) = doc[key].as_table_mut() else { - return Err(anyhow::anyhow!(format!( - "table [{key}] missing after initialization" - ))); - }; - - if created_table { - table.set_implicit(true); - } - - Ok(table) -} - -fn ensure_profile_table<'a>( - doc: &'a mut DocumentMut, - profile_name: &str, -) -> anyhow::Result<&'a mut toml_edit::Table> { - let mut created_profiles_table = false; - { - let root = doc.as_table_mut(); - let needs_table = !root.contains_key("profiles") - || root - .get("profiles") - .and_then(|item| item.as_table()) - .is_none(); - if needs_table { - root.insert("profiles", toml_edit::table()); - created_profiles_table = true; - } - } - - let Some(profiles_table) = doc["profiles"].as_table_mut() else { - return Err(anyhow::anyhow!( - "profiles table missing after initialization" - )); - }; - - if created_profiles_table { - profiles_table.set_implicit(true); - } - - let needs_profile_table = !profiles_table.contains_key(profile_name) - || profiles_table - .get(profile_name) - .and_then(|item| item.as_table()) - .is_none(); - if needs_profile_table { - profiles_table.insert(profile_name, toml_edit::table()); - } - - let Some(profile_table) = profiles_table - .get_mut(profile_name) - .and_then(|item| item.as_table_mut()) - else { - return Err(anyhow::anyhow!(format!( - "profile table missing for {profile_name}" - ))); - }; - - profile_table.set_implicit(false); - Ok(profile_table) -} - -// TODO(jif) refactor config persistence. -pub async fn persist_model_selection( - codex_home: &Path, - active_profile: Option<&str>, - model: &str, - effort: Option, -) -> anyhow::Result<()> { - let config_path = codex_home.join(CONFIG_TOML_FILE); - let serialized = match tokio::fs::read_to_string(&config_path).await { - Ok(contents) => contents, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), - Err(err) => return Err(err.into()), - }; - - let mut doc = if serialized.is_empty() { - DocumentMut::new() - } else { - serialized.parse::()? - }; - - if let Some(profile_name) = active_profile { - let profile_table = ensure_profile_table(&mut doc, profile_name)?; - profile_table["model"] = toml_edit::value(model); - match effort { - Some(effort) => { - profile_table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); - } - None => { - profile_table.remove("model_reasoning_effort"); - } - } - } else { - let table = doc.as_table_mut(); - table["model"] = toml_edit::value(model); - match effort { - Some(effort) => { - table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); - } - None => { - table.remove("model_reasoning_effort"); - } - } - } - - // TODO(jif) refactor the home creation - tokio::fs::create_dir_all(codex_home) - .await - .with_context(|| { - format!( - "failed to create Codex home directory at {}", - codex_home.display() - ) - })?; - - tokio::fs::write(&config_path, doc.to_string()) - .await - .with_context(|| format!("failed to persist config.toml at {}", config_path.display()))?; - - Ok(()) + ConfigEditsBuilder::new(codex_home) + .set_project_trusted(project_path) + .apply_blocking() } /// Apply a single dotted-path override onto a TOML value. @@ -1579,7 +1244,11 @@ pub fn log_dir(cfg: &Config) -> std::io::Result { #[cfg(test)] mod tests { + use crate::config_edit::ConfigEdit; + use crate::config_edit::ConfigEditsBuilder; + use crate::config_edit::apply_blocking; use crate::config_types::HistoryPersistence; + use crate::config_types::McpServerTransportConfig; use crate::config_types::Notifications; use crate::features::Feature; @@ -2107,7 +1776,7 @@ trust_level = "trusted" } #[tokio::test] - async fn write_global_mcp_servers_round_trips_entries() -> anyhow::Result<()> { + async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let mut servers = BTreeMap::new(); @@ -2129,7 +1798,11 @@ trust_level = "trusted" }, ); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let loaded = load_global_mcp_servers(codex_home.path()).await?; assert_eq!(loaded.len(), 1); @@ -2155,7 +1828,11 @@ trust_level = "trusted" assert!(docs.enabled); let empty = BTreeMap::new(); - write_global_mcp_servers(codex_home.path(), &empty)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(empty.clone())], + )?; let loaded = load_global_mcp_servers(codex_home.path()).await?; assert!(loaded.is_empty()); @@ -2243,7 +1920,7 @@ bearer_token = "secret" } #[tokio::test] - async fn write_global_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { + async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2267,7 +1944,11 @@ bearer_token = "secret" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2310,7 +1991,7 @@ ZIG_VAR = "3" } #[tokio::test] - async fn write_global_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { + async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2331,7 +2012,11 @@ ZIG_VAR = "3" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2353,7 +2038,7 @@ ZIG_VAR = "3" } #[tokio::test] - async fn write_global_mcp_servers_serializes_cwd() -> anyhow::Result<()> { + async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let cwd_path = PathBuf::from("/tmp/codex-mcp"); @@ -2375,7 +2060,11 @@ ZIG_VAR = "3" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2397,8 +2086,7 @@ ZIG_VAR = "3" } #[tokio::test] - async fn write_global_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow::Result<()> - { + async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2418,7 +2106,11 @@ ZIG_VAR = "3" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2453,8 +2145,7 @@ startup_timeout_sec = 2.0 } #[tokio::test] - async fn write_global_mcp_servers_streamable_http_serializes_custom_headers() - -> anyhow::Result<()> { + async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2476,7 +2167,11 @@ startup_timeout_sec = 2.0 disabled_tools: None, }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2522,8 +2217,7 @@ X-Auth = "DOCS_AUTH" } #[tokio::test] - async fn write_global_mcp_servers_streamable_http_removes_optional_sections() - -> anyhow::Result<()> { + async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); @@ -2548,7 +2242,11 @@ X-Auth = "DOCS_AUTH" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let serialized_with_optional = std::fs::read_to_string(&config_path)?; assert!(serialized_with_optional.contains("bearer_token_env_var = \"MCP_TOKEN\"")); assert!(serialized_with_optional.contains("[mcp_servers.docs.http_headers]")); @@ -2570,7 +2268,11 @@ X-Auth = "DOCS_AUTH" disabled_tools: None, }, ); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let serialized = std::fs::read_to_string(&config_path)?; assert_eq!( @@ -2603,7 +2305,7 @@ url = "https://example.com/mcp" } #[tokio::test] - async fn write_global_mcp_servers_streamable_http_isolates_headers_between_servers() + async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); @@ -2650,7 +2352,11 @@ url = "https://example.com/mcp" ), ]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let serialized = std::fs::read_to_string(&config_path)?; assert!( @@ -2704,7 +2410,7 @@ url = "https://example.com/mcp" } #[tokio::test] - async fn write_global_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { + async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2725,7 +2431,11 @@ url = "https://example.com/mcp" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2742,7 +2452,7 @@ url = "https://example.com/mcp" } #[tokio::test] - async fn write_global_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { + async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let servers = BTreeMap::from([( @@ -2763,7 +2473,11 @@ url = "https://example.com/mcp" }, )]); - write_global_mcp_servers(codex_home.path(), &servers)?; + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); let serialized = std::fs::read_to_string(&config_path)?; @@ -2785,16 +2499,13 @@ url = "https://example.com/mcp" } #[tokio::test] - async fn persist_model_selection_updates_defaults() -> anyhow::Result<()> { + async fn set_model_updates_defaults() -> anyhow::Result<()> { let codex_home = TempDir::new()?; - persist_model_selection( - codex_home.path(), - None, - "gpt-5-codex", - Some(ReasoningEffort::High), - ) - .await?; + ConfigEditsBuilder::new(codex_home.path()) + .set_model(Some("gpt-5-codex"), Some(ReasoningEffort::High)) + .apply() + .await?; let serialized = tokio::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).await?; @@ -2807,7 +2518,7 @@ url = "https://example.com/mcp" } #[tokio::test] - async fn persist_model_selection_overwrites_existing_model() -> anyhow::Result<()> { + async fn set_model_overwrites_existing_model() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); @@ -2823,13 +2534,10 @@ model = "gpt-4.1" ) .await?; - persist_model_selection( - codex_home.path(), - None, - "o4-mini", - Some(ReasoningEffort::High), - ) - .await?; + ConfigEditsBuilder::new(codex_home.path()) + .set_model(Some("o4-mini"), Some(ReasoningEffort::High)) + .apply() + .await?; let serialized = tokio::fs::read_to_string(config_path).await?; let parsed: ConfigToml = toml::from_str(&serialized)?; @@ -2848,16 +2556,14 @@ model = "gpt-4.1" } #[tokio::test] - async fn persist_model_selection_updates_profile() -> anyhow::Result<()> { + async fn set_model_updates_profile() -> anyhow::Result<()> { let codex_home = TempDir::new()?; - persist_model_selection( - codex_home.path(), - Some("dev"), - "gpt-5-codex", - Some(ReasoningEffort::Medium), - ) - .await?; + ConfigEditsBuilder::new(codex_home.path()) + .with_profile(Some("dev")) + .set_model(Some("gpt-5-codex"), Some(ReasoningEffort::Medium)) + .apply() + .await?; let serialized = tokio::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).await?; @@ -2877,7 +2583,7 @@ model = "gpt-4.1" } #[tokio::test] - async fn persist_model_selection_updates_existing_profile() -> anyhow::Result<()> { + async fn set_model_updates_existing_profile() -> anyhow::Result<()> { let codex_home = TempDir::new()?; let config_path = codex_home.path().join(CONFIG_TOML_FILE); @@ -2894,13 +2600,11 @@ model = "gpt-5-codex" ) .await?; - persist_model_selection( - codex_home.path(), - Some("dev"), - "o4-high", - Some(ReasoningEffort::Medium), - ) - .await?; + ConfigEditsBuilder::new(codex_home.path()) + .with_profile(Some("dev")) + .set_model(Some("o4-high"), Some(ReasoningEffort::Medium)) + .apply() + .await?; let serialized = tokio::fs::read_to_string(config_path).await?; let parsed: ConfigToml = toml::from_str(&serialized)?; diff --git a/codex-rs/core/src/config_edit.rs b/codex-rs/core/src/config_edit.rs index 6e68c08c..38419fb5 100644 --- a/codex-rs/core/src/config_edit.rs +++ b/codex-rs/core/src/config_edit.rs @@ -1,748 +1,954 @@ use crate::config::CONFIG_TOML_FILE; -use anyhow::Result; +use crate::config_types::McpServerConfig; +use crate::config_types::Notice; +use anyhow::Context; +use codex_protocol::config_types::ReasoningEffort; +use std::collections::BTreeMap; use std::path::Path; +use std::path::PathBuf; use tempfile::NamedTempFile; +use tokio::task; use toml_edit::DocumentMut; +use toml_edit::Item as TomlItem; +use toml_edit::Table as TomlTable; +use toml_edit::value; -pub const CONFIG_KEY_MODEL: &str = "model"; -pub const CONFIG_KEY_EFFORT: &str = "model_reasoning_effort"; +/// Discrete config mutations supported by the persistence engine. +#[derive(Clone, Debug)] +pub enum ConfigEdit { + /// Update the active (or default) model selection and optional reasoning effort. + SetModel { + model: Option, + effort: Option, + }, + /// Toggle the acknowledgement flag under `[notice]`. + SetNoticeHideFullAccessWarning(bool), + /// Toggle the Windows onboarding acknowledgement flag. + SetWindowsWslSetupAcknowledged(bool), + /// Replace the entire `[mcp_servers]` table. + ReplaceMcpServers(BTreeMap), + /// Set trust_level = "trusted" under `[projects.""]`, + /// migrating inline tables to explicit tables. + SetProjectTrusted(PathBuf), + /// Set the value stored at the exact dotted path. + SetPath { + segments: Vec, + value: TomlItem, + }, + /// Remove the value stored at the exact dotted path. + ClearPath { segments: Vec }, +} + +// TODO(jif) move to a dedicated file +mod document_helpers { + use crate::config_types::McpServerConfig; + use crate::config_types::McpServerTransportConfig; + use toml_edit::Array as TomlArray; + use toml_edit::InlineTable; + use toml_edit::Item as TomlItem; + use toml_edit::Table as TomlTable; + use toml_edit::value; + + pub(super) fn ensure_table_for_write(item: &mut TomlItem) -> Option<&mut TomlTable> { + match item { + TomlItem::Table(table) => Some(table), + TomlItem::Value(value) => { + if let Some(inline) = value.as_inline_table() { + *item = TomlItem::Table(table_from_inline(inline)); + item.as_table_mut() + } else { + *item = TomlItem::Table(new_implicit_table()); + item.as_table_mut() + } + } + TomlItem::None => { + *item = TomlItem::Table(new_implicit_table()); + item.as_table_mut() + } + _ => None, + } + } + + pub(super) fn ensure_table_for_read(item: &mut TomlItem) -> Option<&mut TomlTable> { + match item { + TomlItem::Table(table) => Some(table), + TomlItem::Value(value) => { + let inline = value.as_inline_table()?; + *item = TomlItem::Table(table_from_inline(inline)); + item.as_table_mut() + } + _ => None, + } + } + + pub(super) fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { + let mut entry = TomlTable::new(); + entry.set_implicit(false); + + match &config.transport { + McpServerTransportConfig::Stdio { + command, + args, + env, + env_vars, + cwd, + } => { + entry["command"] = value(command.clone()); + if !args.is_empty() { + entry["args"] = array_from_iter(args.iter().cloned()); + } + if let Some(env) = env + && !env.is_empty() + { + entry["env"] = table_from_pairs(env.iter()); + } + if !env_vars.is_empty() { + entry["env_vars"] = array_from_iter(env_vars.iter().cloned()); + } + if let Some(cwd) = cwd { + entry["cwd"] = value(cwd.to_string_lossy().to_string()); + } + } + McpServerTransportConfig::StreamableHttp { + url, + bearer_token_env_var, + http_headers, + env_http_headers, + } => { + entry["url"] = value(url.clone()); + if let Some(env_var) = bearer_token_env_var { + entry["bearer_token_env_var"] = value(env_var.clone()); + } + if let Some(headers) = http_headers + && !headers.is_empty() + { + entry["http_headers"] = table_from_pairs(headers.iter()); + } + if let Some(headers) = env_http_headers + && !headers.is_empty() + { + entry["env_http_headers"] = table_from_pairs(headers.iter()); + } + } + } + + if !config.enabled { + entry["enabled"] = value(false); + } + if let Some(timeout) = config.startup_timeout_sec { + entry["startup_timeout_sec"] = value(timeout.as_secs_f64()); + } + if let Some(timeout) = config.tool_timeout_sec { + entry["tool_timeout_sec"] = value(timeout.as_secs_f64()); + } + if let Some(enabled_tools) = &config.enabled_tools + && !enabled_tools.is_empty() + { + entry["enabled_tools"] = array_from_iter(enabled_tools.iter().cloned()); + } + if let Some(disabled_tools) = &config.disabled_tools + && !disabled_tools.is_empty() + { + entry["disabled_tools"] = array_from_iter(disabled_tools.iter().cloned()); + } + + TomlItem::Table(entry) + } + + fn table_from_inline(inline: &InlineTable) -> TomlTable { + let mut table = new_implicit_table(); + for (key, value) in inline.iter() { + let mut value = value.clone(); + let decor = value.decor_mut(); + decor.set_suffix(""); + table.insert(key, TomlItem::Value(value)); + } + table + } + + pub(super) fn new_implicit_table() -> TomlTable { + let mut table = TomlTable::new(); + table.set_implicit(true); + table + } + + fn array_from_iter(iter: I) -> TomlItem + where + I: Iterator, + { + let mut array = TomlArray::new(); + for value in iter { + array.push(value); + } + TomlItem::Value(array.into()) + } + + fn table_from_pairs<'a, I>(pairs: I) -> TomlItem + where + I: IntoIterator, + { + let mut entries: Vec<_> = pairs.into_iter().collect(); + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + let mut table = TomlTable::new(); + table.set_implicit(false); + for (key, val) in entries { + table.insert(key, value(val.clone())); + } + TomlItem::Table(table) + } +} + +struct ConfigDocument { + doc: DocumentMut, + profile: Option, +} #[derive(Copy, Clone)] -enum NoneBehavior { - Skip, - Remove, +enum Scope { + Global, + Profile, } -/// Persist overrides into `config.toml` using explicit key segments per -/// override. This avoids ambiguity with keys that contain dots or spaces. -pub async fn persist_overrides( - codex_home: &Path, - profile: Option<&str>, - overrides: &[(&[&str], &str)], -) -> Result<()> { - let with_options: Vec<(&[&str], Option<&str>)> = overrides - .iter() - .map(|(segments, value)| (*segments, Some(*value))) - .collect(); - - persist_overrides_with_behavior(codex_home, profile, &with_options, NoneBehavior::Skip).await +#[derive(Copy, Clone)] +enum TraversalMode { + Create, + Existing, } -/// Persist overrides where values may be optional. Any entries with `None` -/// values are skipped. If all values are `None`, this becomes a no-op and -/// returns `Ok(())` without touching the file. -pub async fn persist_non_null_overrides( - codex_home: &Path, - profile: Option<&str>, - overrides: &[(&[&str], Option<&str>)], -) -> Result<()> { - persist_overrides_with_behavior(codex_home, profile, overrides, NoneBehavior::Skip).await -} - -/// Persist overrides where `None` values clear any existing values from the -/// configuration file. -pub async fn persist_overrides_and_clear_if_none( - codex_home: &Path, - profile: Option<&str>, - overrides: &[(&[&str], Option<&str>)], -) -> Result<()> { - persist_overrides_with_behavior(codex_home, profile, overrides, NoneBehavior::Remove).await -} - -/// Apply a single override onto a `toml_edit` document while preserving -/// existing formatting/comments. -/// The key is expressed as explicit segments to correctly handle keys that -/// contain dots or spaces. -fn apply_toml_edit_override_segments( - doc: &mut DocumentMut, - segments: &[&str], - value: toml_edit::Item, -) { - use toml_edit::Item; - - if segments.is_empty() { - return; +impl ConfigDocument { + fn new(doc: DocumentMut, profile: Option) -> Self { + Self { doc, profile } } - let mut current = doc.as_table_mut(); - for seg in &segments[..segments.len() - 1] { - if !current.contains_key(seg) { - current[*seg] = Item::Table(toml_edit::Table::new()); - if let Some(t) = current[*seg].as_table_mut() { - t.set_implicit(true); + fn apply(&mut self, edit: &ConfigEdit) -> anyhow::Result { + match edit { + ConfigEdit::SetModel { model, effort } => Ok({ + let mut mutated = false; + mutated |= self.write_profile_value( + &["model"], + model.as_ref().map(|model_value| value(model_value.clone())), + ); + mutated |= self.write_profile_value( + &["model_reasoning_effort"], + effort.map(|effort| value(effort.to_string())), + ); + mutated + }), + ConfigEdit::SetNoticeHideFullAccessWarning(acknowledged) => Ok(self.write_value( + Scope::Global, + &[Notice::TABLE_KEY, "hide_full_access_warning"], + value(*acknowledged), + )), + ConfigEdit::SetWindowsWslSetupAcknowledged(acknowledged) => Ok(self.write_value( + Scope::Global, + &["windows_wsl_setup_acknowledged"], + value(*acknowledged), + )), + ConfigEdit::ReplaceMcpServers(servers) => Ok(self.replace_mcp_servers(servers)), + ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())), + ConfigEdit::ClearPath { segments } => Ok(self.clear_owned(segments)), + ConfigEdit::SetProjectTrusted(project_path) => { + // Delegate to the existing, tested logic in config.rs to + // ensure tables are explicit and migration is preserved. + crate::config::set_project_trusted_inner(&mut self.doc, project_path.as_path())?; + Ok(true) } } + } - let maybe_item = current.get_mut(seg); - let Some(item) = maybe_item else { return }; + fn write_profile_value(&mut self, segments: &[&str], value: Option) -> bool { + match value { + Some(item) => self.write_value(Scope::Profile, segments, item), + None => self.clear(Scope::Profile, segments), + } + } - if !item.is_table() { - *item = Item::Table(toml_edit::Table::new()); - if let Some(t) = item.as_table_mut() { - t.set_implicit(true); - } + fn write_value(&mut self, scope: Scope, segments: &[&str], value: TomlItem) -> bool { + let resolved = self.scoped_segments(scope, segments); + self.insert(&resolved, value) + } + + fn clear(&mut self, scope: Scope, segments: &[&str]) -> bool { + let resolved = self.scoped_segments(scope, segments); + self.remove(&resolved) + } + + fn clear_owned(&mut self, segments: &[String]) -> bool { + self.remove(segments) + } + + fn replace_mcp_servers(&mut self, servers: &BTreeMap) -> bool { + if servers.is_empty() { + return self.clear(Scope::Global, &["mcp_servers"]); } - let Some(tbl) = item.as_table_mut() else { - return; + let mut table = TomlTable::new(); + table.set_implicit(true); + + for (name, config) in servers { + table.insert(name, document_helpers::serialize_mcp_server(config)); + } + + let item = TomlItem::Table(table); + self.write_value(Scope::Global, &["mcp_servers"], item) + } + + fn scoped_segments(&self, scope: Scope, segments: &[&str]) -> Vec { + let resolved: Vec = segments + .iter() + .map(|segment| (*segment).to_string()) + .collect(); + + if matches!(scope, Scope::Profile) + && resolved.first().is_none_or(|segment| segment != "profiles") + && let Some(profile) = self.profile.as_deref() + { + let mut scoped = Vec::with_capacity(resolved.len() + 2); + scoped.push("profiles".to_string()); + scoped.push(profile.to_string()); + scoped.extend(resolved); + return scoped; + } + + resolved + } + + fn insert(&mut self, segments: &[String], value: TomlItem) -> bool { + let Some((last, parents)) = segments.split_last() else { + return false; }; - current = tbl; + + let Some(parent) = self.descend(parents, TraversalMode::Create) else { + return false; + }; + + parent[last] = value; + true } - let last = segments[segments.len() - 1]; - current[last] = value; + fn remove(&mut self, segments: &[String]) -> bool { + let Some((last, parents)) = segments.split_last() else { + return false; + }; + + let Some(parent) = self.descend(parents, TraversalMode::Existing) else { + return false; + }; + + parent.remove(last).is_some() + } + + fn descend(&mut self, segments: &[String], mode: TraversalMode) -> Option<&mut TomlTable> { + let mut current = self.doc.as_table_mut(); + + for segment in segments { + match mode { + TraversalMode::Create => { + if !current.contains_key(segment.as_str()) { + current.insert( + segment.as_str(), + TomlItem::Table(document_helpers::new_implicit_table()), + ); + } + + let item = current.get_mut(segment.as_str())?; + current = document_helpers::ensure_table_for_write(item)?; + } + TraversalMode::Existing => { + let item = current.get_mut(segment.as_str())?; + current = document_helpers::ensure_table_for_read(item)?; + } + } + } + + Some(current) + } } -async fn persist_overrides_with_behavior( +/// Persist edits using a blocking strategy. +pub fn apply_blocking( codex_home: &Path, profile: Option<&str>, - overrides: &[(&[&str], Option<&str>)], - none_behavior: NoneBehavior, -) -> Result<()> { - if overrides.is_empty() { - return Ok(()); - } - - let should_skip = match none_behavior { - NoneBehavior::Skip => overrides.iter().all(|(_, value)| value.is_none()), - NoneBehavior::Remove => false, - }; - - if should_skip { + edits: &[ConfigEdit], +) -> anyhow::Result<()> { + if edits.is_empty() { return Ok(()); } let config_path = codex_home.join(CONFIG_TOML_FILE); - - let read_result = tokio::fs::read_to_string(&config_path).await; - let mut doc = match read_result { - Ok(contents) => contents.parse::()?, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - if overrides - .iter() - .all(|(_, value)| value.is_none() && matches!(none_behavior, NoneBehavior::Remove)) - { - return Ok(()); - } - - tokio::fs::create_dir_all(codex_home).await?; - DocumentMut::new() - } - Err(e) => return Err(e.into()), + let serialized = match std::fs::read_to_string(&config_path) { + Ok(contents) => contents, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(err) => return Err(err.into()), }; - let effective_profile = if let Some(p) = profile { - Some(p.to_owned()) + let doc = if serialized.is_empty() { + DocumentMut::new() } else { - doc.get("profile") - .and_then(|i| i.as_str()) - .map(str::to_string) + serialized.parse::()? }; + let profile = profile.map(ToOwned::to_owned).or_else(|| { + doc.get("profile") + .and_then(|item| item.as_str()) + .map(ToOwned::to_owned) + }); + + let mut document = ConfigDocument::new(doc, profile); let mut mutated = false; - for (segments, value) in overrides.iter().copied() { - let mut seg_buf: Vec<&str> = Vec::new(); - let segments_to_apply: &[&str]; - - if let Some(ref name) = effective_profile { - if segments.first().copied() == Some("profiles") { - segments_to_apply = segments; - } else { - seg_buf.reserve(2 + segments.len()); - seg_buf.push("profiles"); - seg_buf.push(name.as_str()); - seg_buf.extend_from_slice(segments); - segments_to_apply = seg_buf.as_slice(); - } - } else { - segments_to_apply = segments; - } - - match value { - Some(v) => { - let item_value = toml_edit::value(v); - apply_toml_edit_override_segments(&mut doc, segments_to_apply, item_value); - mutated = true; - } - None => { - if matches!(none_behavior, NoneBehavior::Remove) - && remove_toml_edit_segments(&mut doc, segments_to_apply) - { - mutated = true; - } - } - } + for edit in edits { + mutated |= document.apply(edit)?; } if !mutated { return Ok(()); } - let tmp_file = NamedTempFile::new_in(codex_home)?; - tokio::fs::write(tmp_file.path(), doc.to_string()).await?; - tmp_file.persist(config_path)?; + std::fs::create_dir_all(codex_home).with_context(|| { + format!( + "failed to create Codex home directory at {}", + codex_home.display() + ) + })?; + + let tmp = NamedTempFile::new_in(codex_home)?; + std::fs::write(tmp.path(), document.doc.to_string()).with_context(|| { + format!( + "failed to write temporary config file at {}", + tmp.path().display() + ) + })?; + tmp.persist(config_path)?; Ok(()) } -fn remove_toml_edit_segments(doc: &mut DocumentMut, segments: &[&str]) -> bool { - use toml_edit::Item; +/// Persist edits asynchronously by offloading the blocking writer. +pub async fn apply( + codex_home: &Path, + profile: Option<&str>, + edits: Vec, +) -> anyhow::Result<()> { + let codex_home = codex_home.to_path_buf(); + let profile = profile.map(ToOwned::to_owned); + task::spawn_blocking(move || apply_blocking(&codex_home, profile.as_deref(), &edits)) + .await + .context("config persistence task panicked")? +} - if segments.is_empty() { - return false; - } +/// Fluent builder to batch config edits and apply them atomically. +#[derive(Default)] +pub struct ConfigEditsBuilder { + codex_home: PathBuf, + profile: Option, + edits: Vec, +} - let mut current = doc.as_table_mut(); - for seg in &segments[..segments.len() - 1] { - let Some(item) = current.get_mut(seg) else { - return false; - }; - - match item { - Item::Table(table) => { - current = table; - } - _ => { - return false; - } +impl ConfigEditsBuilder { + pub fn new(codex_home: &Path) -> Self { + Self { + codex_home: codex_home.to_path_buf(), + profile: None, + edits: Vec::new(), } } - current.remove(segments[segments.len() - 1]).is_some() + pub fn with_profile(mut self, profile: Option<&str>) -> Self { + self.profile = profile.map(ToOwned::to_owned); + self + } + + pub fn set_model(mut self, model: Option<&str>, effort: Option) -> Self { + self.edits.push(ConfigEdit::SetModel { + model: model.map(ToOwned::to_owned), + effort, + }); + self + } + + pub fn set_hide_full_access_warning(mut self, acknowledged: bool) -> Self { + self.edits + .push(ConfigEdit::SetNoticeHideFullAccessWarning(acknowledged)); + self + } + + pub fn set_windows_wsl_setup_acknowledged(mut self, acknowledged: bool) -> Self { + self.edits + .push(ConfigEdit::SetWindowsWslSetupAcknowledged(acknowledged)); + self + } + + pub fn replace_mcp_servers(mut self, servers: &BTreeMap) -> Self { + self.edits + .push(ConfigEdit::ReplaceMcpServers(servers.clone())); + self + } + + pub fn set_project_trusted>(mut self, project_path: P) -> Self { + self.edits + .push(ConfigEdit::SetProjectTrusted(project_path.into())); + self + } + + /// Apply edits on a blocking thread. + pub fn apply_blocking(self) -> anyhow::Result<()> { + apply_blocking(&self.codex_home, self.profile.as_deref(), &self.edits) + } + + /// Apply edits asynchronously via a blocking offload. + pub async fn apply(self) -> anyhow::Result<()> { + task::spawn_blocking(move || { + apply_blocking(&self.codex_home, self.profile.as_deref(), &self.edits) + }) + .await + .context("config persistence task panicked")? + } } #[cfg(test)] mod tests { use super::*; + use crate::config_types::McpServerTransportConfig; + use codex_protocol::config_types::ReasoningEffort; use pretty_assertions::assert_eq; use tempfile::tempdir; + use tokio::runtime::Builder; + use toml::Value as TomlValue; - /// Verifies model and effort are written at top-level when no profile is set. - #[tokio::test] - async fn set_default_model_and_effort_top_level_when_no_profile() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_set_model_top_level() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); - persist_overrides( + apply_blocking( codex_home, None, - &[ - (&[CONFIG_KEY_MODEL], "gpt-5-codex"), - (&[CONFIG_KEY_EFFORT], "high"), - ], + &[ConfigEdit::SetModel { + model: Some("gpt-5-codex".to_string()), + effort: Some(ReasoningEffort::High), + }], ) - .await .expect("persist"); - let contents = read_config(codex_home).await; + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); let expected = r#"model = "gpt-5-codex" model_reasoning_effort = "high" "#; assert_eq!(contents, expected); } - /// Verifies values are written under the active profile when `profile` is set. - #[tokio::test] - async fn set_defaults_update_profile_when_profile_set() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_set_model_preserves_inline_table_contents() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); - // Seed config with a profile selection but without profiles table - let seed = "profile = \"o3\"\n"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); + // Seed with inline tables for profiles to simulate common user config. + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"profile = "fast" - persist_overrides( +profiles = { fast = { model = "gpt-4o", sandbox_mode = "strict" } } +"#, + ) + .expect("seed"); + + apply_blocking( codex_home, None, - &[ - (&[CONFIG_KEY_MODEL], "o3"), - (&[CONFIG_KEY_EFFORT], "minimal"), - ], + &[ConfigEdit::SetModel { + model: Some("o4-mini".to_string()), + effort: None, + }], ) - .await .expect("persist"); - let contents = read_config(codex_home).await; - let expected = r#"profile = "o3" + let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let value: TomlValue = toml::from_str(&raw).expect("parse config"); -[profiles.o3] -model = "o3" -model_reasoning_effort = "minimal" -"#; - assert_eq!(contents, expected); + // Ensure sandbox_mode is preserved under profiles.fast and model updated. + let profiles_tbl = value + .get("profiles") + .and_then(|v| v.as_table()) + .expect("profiles table"); + let fast_tbl = profiles_tbl + .get("fast") + .and_then(|v| v.as_table()) + .expect("fast table"); + assert_eq!( + fast_tbl.get("sandbox_mode").and_then(|v| v.as_str()), + Some("strict") + ); + assert_eq!( + fast_tbl.get("model").and_then(|v| v.as_str()), + Some("o4-mini") + ); } - /// Verifies profile names with dots/spaces are preserved via explicit segments. - #[tokio::test] - async fn set_defaults_update_profile_with_dot_and_space() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_clear_model_removes_inline_table_entry() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); - // Seed config with a profile name that contains a dot and a space - let seed = "profile = \"my.team name\"\n"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"profile = "fast" - persist_overrides( +profiles = { fast = { model = "gpt-4o", sandbox_mode = "strict" } } +"#, + ) + .expect("seed"); + + apply_blocking( codex_home, None, - &[ - (&[CONFIG_KEY_MODEL], "o3"), - (&[CONFIG_KEY_EFFORT], "minimal"), - ], + &[ConfigEdit::SetModel { + model: None, + effort: Some(ReasoningEffort::High), + }], ) - .await .expect("persist"); - let contents = read_config(codex_home).await; - let expected = r#"profile = "my.team name" + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"profile = "fast" -[profiles."my.team name"] -model = "o3" -model_reasoning_effort = "minimal" -"#; - assert_eq!(contents, expected); - } - - /// Verifies explicit profile override writes under that profile even without active profile. - #[tokio::test] - async fn set_defaults_update_when_profile_override_supplied() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // No profile key in config.toml - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), "") - .await - .expect("seed write"); - - // Persist with an explicit profile override - persist_overrides( - codex_home, - Some("o3"), - &[(&[CONFIG_KEY_MODEL], "o3"), (&[CONFIG_KEY_EFFORT], "high")], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"[profiles.o3] -model = "o3" +[profiles.fast] +sandbox_mode = "strict" model_reasoning_effort = "high" "#; assert_eq!(contents, expected); } - /// Verifies nested tables are created as needed when applying overrides. - #[tokio::test] - async fn persist_overrides_creates_nested_tables() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_set_model_scopes_to_active_profile() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"profile = "team" - persist_overrides( - codex_home, - None, - &[ - (&["a", "b", "c"], "v"), - (&["x"], "y"), - (&["profiles", "p1", CONFIG_KEY_MODEL], "gpt-5-codex"), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"x = "y" - -[a.b] -c = "v" - -[profiles.p1] -model = "gpt-5-codex" -"#; - assert_eq!(contents, expected); - } - - /// Verifies a scalar key becomes a table when nested keys are written. - #[tokio::test] - async fn persist_overrides_replaces_scalar_with_table() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - let seed = "foo = \"bar\"\n"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - persist_overrides(codex_home, None, &[(&["foo", "bar", "baz"], "ok")]) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"[foo.bar] -baz = "ok" -"#; - assert_eq!(contents, expected); - } - - /// Verifies comments and spacing are preserved when writing under active profile. - #[tokio::test] - async fn set_defaults_preserve_comments() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Seed a config with comments and spacing we expect to preserve - let seed = r#"# Global comment -# Another line - -profile = "o3" - -# Profile settings -[profiles.o3] -# keep me -existing = "keep" -"#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - // Apply defaults; since profile is set, it should write under [profiles.o3] - persist_overrides( - codex_home, - None, - &[(&[CONFIG_KEY_MODEL], "o3"), (&[CONFIG_KEY_EFFORT], "high")], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"# Global comment -# Another line - -profile = "o3" - -# Profile settings -[profiles.o3] -# keep me -existing = "keep" -model = "o3" -model_reasoning_effort = "high" -"#; - assert_eq!(contents, expected); - } - - /// Verifies comments and spacing are preserved when writing at top level. - #[tokio::test] - async fn set_defaults_preserve_global_comments() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Seed a config WITHOUT a profile, containing comments and spacing - let seed = r#"# Top-level comments -# should be preserved - -existing = "keep" -"#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - // Since there is no profile, the defaults should be written at top-level - persist_overrides( - codex_home, - None, - &[ - (&[CONFIG_KEY_MODEL], "gpt-5-codex"), - (&[CONFIG_KEY_EFFORT], "minimal"), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"# Top-level comments -# should be preserved - -existing = "keep" -model = "gpt-5-codex" -model_reasoning_effort = "minimal" -"#; - assert_eq!(contents, expected); - } - - /// Verifies errors on invalid TOML propagate and file is not clobbered. - #[tokio::test] - async fn persist_overrides_errors_on_parse_failure() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Write an intentionally invalid TOML file - let invalid = "invalid = [unclosed"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), invalid) - .await - .expect("seed write"); - - // Attempting to persist should return an error and must not clobber the file. - let res = persist_overrides(codex_home, None, &[(&["x"], "y")]).await; - assert!(res.is_err(), "expected parse error to propagate"); - - // File should be unchanged - let contents = read_config(codex_home).await; - assert_eq!(contents, invalid); - } - - /// Verifies changing model only preserves existing effort at top-level. - #[tokio::test] - async fn changing_only_model_preserves_existing_effort_top_level() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Seed with an effort value only - let seed = "model_reasoning_effort = \"minimal\"\n"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - // Change only the model - persist_overrides(codex_home, None, &[(&[CONFIG_KEY_MODEL], "o3")]) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"model_reasoning_effort = "minimal" -model = "o3" -"#; - assert_eq!(contents, expected); - } - - /// Verifies changing effort only preserves existing model at top-level. - #[tokio::test] - async fn changing_only_effort_preserves_existing_model_top_level() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Seed with a model value only - let seed = "model = \"gpt-5-codex\"\n"; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - // Change only the effort - persist_overrides(codex_home, None, &[(&[CONFIG_KEY_EFFORT], "high")]) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"model = "gpt-5-codex" -model_reasoning_effort = "high" -"#; - assert_eq!(contents, expected); - } - - /// Verifies changing model only preserves existing effort in active profile. - #[tokio::test] - async fn changing_only_model_preserves_effort_in_active_profile() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - // Seed with an active profile and an existing effort under that profile - let seed = r#"profile = "p1" - -[profiles.p1] +[profiles.team] model_reasoning_effort = "low" +"#, + ) + .expect("seed"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetModel { + model: Some("o5-preview".to_string()), + effort: Some(ReasoningEffort::Minimal), + }], + ) + .expect("persist"); + + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"profile = "team" + +[profiles.team] +model_reasoning_effort = "minimal" +model = "o5-preview" "#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); + assert_eq!(contents, expected); + } - persist_overrides(codex_home, None, &[(&[CONFIG_KEY_MODEL], "o4-mini")]) - .await - .expect("persist"); + #[test] + fn blocking_set_model_with_explicit_profile() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"[profiles."team a"] +model = "gpt-5-codex" +"#, + ) + .expect("seed"); - let contents = read_config(codex_home).await; - let expected = r#"profile = "p1" + apply_blocking( + codex_home, + Some("team a"), + &[ConfigEdit::SetModel { + model: Some("o4-mini".to_string()), + effort: None, + }], + ) + .expect("persist"); -[profiles.p1] -model_reasoning_effort = "low" + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[profiles."team a"] model = "o4-mini" "#; assert_eq!(contents, expected); } - /// Verifies changing effort only preserves existing model in a profile override. - #[tokio::test] - async fn changing_only_effort_preserves_model_in_profile_override() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_set_hide_full_access_warning_preserves_table() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"# Global comment - // No active profile key; we'll target an explicit override - let seed = r#"[profiles.team] -model = "gpt-5-codex" +[notice] +# keep me +existing = "value" +"#, + ) + .expect("seed"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetNoticeHideFullAccessWarning(true)], + ) + .expect("persist"); + + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"# Global comment + +[notice] +# keep me +existing = "value" +hide_full_access_warning = true "#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) + assert_eq!(contents, expected); + } + + #[test] + fn blocking_replace_mcp_servers_round_trips() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + let mut servers = BTreeMap::new(); + servers.insert( + "stdio".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "cmd".to_string(), + args: vec!["--flag".to_string()], + env: Some( + [ + ("B".to_string(), "2".to_string()), + ("A".to_string(), "1".to_string()), + ] + .into_iter() + .collect(), + ), + env_vars: vec!["FOO".to_string()], + cwd: None, + }, + enabled: true, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), + disabled_tools: None, + }, + ); + + servers.insert( + "http".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://example.com".to_string(), + bearer_token_env_var: Some("TOKEN".to_string()), + http_headers: Some( + [("Z-Header".to_string(), "z".to_string())] + .into_iter() + .collect(), + ), + env_http_headers: None, + }, + enabled: false, + startup_timeout_sec: Some(std::time::Duration::from_secs(5)), + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: Some(vec!["forbidden".to_string()]), + }, + ); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + ) + .expect("persist"); + + let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = "\ +[mcp_servers.http] +url = \"https://example.com\" +bearer_token_env_var = \"TOKEN\" +enabled = false +startup_timeout_sec = 5.0 +disabled_tools = [\"forbidden\"] + +[mcp_servers.http.http_headers] +Z-Header = \"z\" + +[mcp_servers.stdio] +command = \"cmd\" +args = [\"--flag\"] +env_vars = [\"FOO\"] +enabled_tools = [\"one\", \"two\"] + +[mcp_servers.stdio.env] +A = \"1\" +B = \"2\" +"; + assert_eq!(raw, expected); + } + + #[test] + fn blocking_clear_path_noop_when_missing() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::ClearPath { + segments: vec!["missing".to_string()], + }], + ) + .expect("apply"); + + assert!( + !codex_home.join(CONFIG_TOML_FILE).exists(), + "config.toml should not be created on noop" + ); + } + + #[test] + fn blocking_set_path_updates_notifications() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + let item = value(false); + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetPath { + segments: vec!["tui".to_string(), "notifications".to_string()], + value: item, + }], + ) + .expect("apply"); + + let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let config: TomlValue = toml::from_str(&raw).expect("parse config"); + let notifications = config + .get("tui") + .and_then(|item| item.as_table()) + .and_then(|tbl| tbl.get("notifications")) + .and_then(toml::Value::as_bool); + assert_eq!(notifications, Some(false)); + } + + #[tokio::test] + async fn async_builder_set_model_persists() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path().to_path_buf(); + + ConfigEditsBuilder::new(&codex_home) + .set_model(Some("gpt-5-codex"), Some(ReasoningEffort::High)) + .apply() .await - .expect("seed write"); + .expect("persist"); - persist_overrides( - codex_home, - Some("team"), - &[(&[CONFIG_KEY_EFFORT], "minimal")], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"[profiles.team] -model = "gpt-5-codex" -model_reasoning_effort = "minimal" -"#; - assert_eq!(contents, expected); - } - - /// Verifies `persist_non_null_overrides` skips `None` entries and writes only present values at top-level. - #[tokio::test] - async fn persist_non_null_skips_none_top_level() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - persist_non_null_overrides( - codex_home, - None, - &[ - (&[CONFIG_KEY_MODEL], Some("gpt-5-codex")), - (&[CONFIG_KEY_EFFORT], None), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = "model = \"gpt-5-codex\"\n"; - assert_eq!(contents, expected); - } - - /// Verifies no-op behavior when all provided overrides are `None` (no file created/modified). - #[tokio::test] - async fn persist_non_null_noop_when_all_none() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - persist_non_null_overrides( - codex_home, - None, - &[(&["a"], None), (&["profiles", "p", "x"], None)], - ) - .await - .expect("persist"); - - // Should not create config.toml on a pure no-op - assert!(!codex_home.join(CONFIG_TOML_FILE).exists()); - } - - /// Verifies entries are written under the specified profile and `None` entries are skipped. - #[tokio::test] - async fn persist_non_null_respects_profile_override() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - persist_non_null_overrides( - codex_home, - Some("team"), - &[ - (&[CONFIG_KEY_MODEL], Some("o3")), - (&[CONFIG_KEY_EFFORT], None), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"[profiles.team] -model = "o3" -"#; - assert_eq!(contents, expected); - } - - #[tokio::test] - async fn persist_clear_none_removes_top_level_value() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - let seed = r#"model = "gpt-5-codex" -model_reasoning_effort = "medium" -"#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - persist_overrides_and_clear_if_none( - codex_home, - None, - &[ - (&[CONFIG_KEY_MODEL], None), - (&[CONFIG_KEY_EFFORT], Some("high")), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = "model_reasoning_effort = \"high\"\n"; - assert_eq!(contents, expected); - } - - #[tokio::test] - async fn persist_clear_none_respects_active_profile() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); - - let seed = r#"profile = "team" - -[profiles.team] -model = "gpt-4" -model_reasoning_effort = "minimal" -"#; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) - .await - .expect("seed write"); - - persist_overrides_and_clear_if_none( - codex_home, - None, - &[ - (&[CONFIG_KEY_MODEL], None), - (&[CONFIG_KEY_EFFORT], Some("high")), - ], - ) - .await - .expect("persist"); - - let contents = read_config(codex_home).await; - let expected = r#"profile = "team" - -[profiles.team] + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"model = "gpt-5-codex" model_reasoning_effort = "high" "#; assert_eq!(contents, expected); } - #[tokio::test] - async fn persist_clear_none_noop_when_file_missing() { - let tmpdir = tempdir().expect("tmp"); - let codex_home = tmpdir.path(); + #[test] + fn blocking_builder_set_model_round_trips_back_and_forth() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); - persist_overrides_and_clear_if_none(codex_home, None, &[(&[CONFIG_KEY_MODEL], None)]) - .await - .expect("persist"); + let initial_expected = r#"model = "o4-mini" +model_reasoning_effort = "low" +"#; + ConfigEditsBuilder::new(codex_home) + .set_model(Some("o4-mini"), Some(ReasoningEffort::Low)) + .apply_blocking() + .expect("persist initial"); + let mut contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, initial_expected); - assert!(!codex_home.join(CONFIG_TOML_FILE).exists()); + let updated_expected = r#"model = "gpt-5-codex" +model_reasoning_effort = "high" +"#; + ConfigEditsBuilder::new(codex_home) + .set_model(Some("gpt-5-codex"), Some(ReasoningEffort::High)) + .apply_blocking() + .expect("persist update"); + contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, updated_expected); + + ConfigEditsBuilder::new(codex_home) + .set_model(Some("o4-mini"), Some(ReasoningEffort::Low)) + .apply_blocking() + .expect("persist revert"); + contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert_eq!(contents, initial_expected); } - // Test helper moved to bottom per review guidance. - async fn read_config(codex_home: &Path) -> String { - let p = codex_home.join(CONFIG_TOML_FILE); - tokio::fs::read_to_string(p).await.unwrap_or_default() + #[test] + fn blocking_set_asynchronous_helpers_available() { + let rt = Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime"); + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path().to_path_buf(); + + rt.block_on(async { + ConfigEditsBuilder::new(&codex_home) + .set_hide_full_access_warning(true) + .apply() + .await + .expect("persist"); + }); + + let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let notice = toml::from_str::(&raw) + .expect("parse config") + .get("notice") + .and_then(|item| item.as_table()) + .and_then(|tbl| tbl.get("hide_full_access_warning")) + .and_then(toml::Value::as_bool); + assert_eq!(notice, Some(true)); + } + + #[test] + fn replace_mcp_servers_blocking_clears_table_when_empty() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + "[mcp_servers]\nfoo = { command = \"cmd\" }\n", + ) + .expect("seed"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::ReplaceMcpServers(BTreeMap::new())], + ) + .expect("persist"); + + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + assert!(!contents.contains("mcp_servers")); } } diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs index 2f4be2b6..fd93df8f 100644 --- a/codex-rs/core/src/config_types.rs +++ b/codex-rs/core/src/config_types.rs @@ -361,7 +361,7 @@ pub struct Notice { } impl Notice { - /// used by set_hide_full_access_warning until we refactor config updates + /// referenced by config_edit helpers when writing notice flags pub(crate) const TABLE_KEY: &'static str = "notice"; } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 8fc18a1d..9af54af7 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -17,8 +17,7 @@ use codex_ansi_escape::ansi_escape_line; use codex_core::AuthManager; use codex_core::ConversationManager; use codex_core::config::Config; -use codex_core::config::persist_model_selection; -use codex_core::config::set_hide_full_access_warning; +use codex_core::config_edit::ConfigEditsBuilder; use codex_core::model_family::find_family_for_model; use codex_core::protocol::SessionSource; use codex_core::protocol::TokenUsage; @@ -374,7 +373,10 @@ impl App { } AppEvent::PersistModelSelection { model, effort } => { let profile = self.active_profile.as_deref(); - match persist_model_selection(&self.config.codex_home, profile, &model, effort) + match ConfigEditsBuilder::new(&self.config.codex_home) + .with_profile(profile) + .set_model(Some(model.as_str()), effort) + .apply() .await { Ok(()) => { @@ -421,7 +423,11 @@ impl App { self.chat_widget.set_full_access_warning_acknowledged(ack); } AppEvent::PersistFullAccessWarningAcknowledged => { - if let Err(err) = set_hide_full_access_warning(&self.config.codex_home, true) { + if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) + .set_hide_full_access_warning(true) + .apply() + .await + { tracing::error!( error = %err, "failed to persist full access warning acknowledgement" diff --git a/codex-rs/tui/src/onboarding/windows.rs b/codex-rs/tui/src/onboarding/windows.rs index fb57b7af..431f2a1a 100644 --- a/codex-rs/tui/src/onboarding/windows.rs +++ b/codex-rs/tui/src/onboarding/windows.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use codex_core::config::set_windows_wsl_setup_acknowledged; +use codex_core::config_edit::ConfigEditsBuilder; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -66,7 +66,10 @@ impl WindowsSetupWidget { fn handle_continue(&mut self) { self.highlighted = WindowsSetupSelection::Continue; - match set_windows_wsl_setup_acknowledged(&self.codex_home, true) { + match ConfigEditsBuilder::new(&self.codex_home) + .set_windows_wsl_setup_acknowledged(true) + .apply_blocking() + { Ok(()) => { self.selection = Some(WindowsSetupSelection::Continue); self.exit_requested = false;