From 87654ec0b79b43a6ac5e54891eec0ea85e890e17 Mon Sep 17 00:00:00 2001 From: dedrisian-oai Date: Wed, 10 Sep 2025 13:53:46 -0700 Subject: [PATCH] Persist model & reasoning changes (#2799) Persists `/model` changes across both general and profile-specific sessions. --- codex-rs/core/src/codex.rs | 26 +- codex-rs/core/src/config.rs | 19 +- codex-rs/core/src/config_edit.rs | 582 +++++++++++++++++++++++++++++++ codex-rs/core/src/lib.rs | 1 + 4 files changed, 621 insertions(+), 7 deletions(-) create mode 100644 codex-rs/core/src/config_edit.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 53730f59..f3e9800c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -9,6 +9,9 @@ use std::sync::atomic::AtomicU64; use std::time::Duration; use crate::AuthManager; +use crate::config_edit::CONFIG_KEY_EFFORT; +use crate::config_edit::CONFIG_KEY_MODEL; +use crate::config_edit::persist_non_null_overrides; use crate::event_mapping::map_response_item_to_event_messages; use async_channel::Receiver; use async_channel::Sender; @@ -1100,10 +1103,10 @@ async fn submission_loop( let provider = prev.client.get_provider(); // Effective model + family - let (effective_model, effective_family) = if let Some(m) = model { + let (effective_model, effective_family) = if let Some(ref m) = model { let fam = - find_family_for_model(&m).unwrap_or_else(|| config.model_family.clone()); - (m, fam) + find_family_for_model(m).unwrap_or_else(|| config.model_family.clone()); + (m.clone(), fam) } else { (prev.client.get_model(), prev.client.get_model_family()) }; @@ -1161,6 +1164,23 @@ async fn submission_loop( // Install the new persistent context for subsequent tasks/turns. turn_context = Arc::new(new_turn_context); + + // Optionally persist changes to model / effort + let effort_str = effort.map(|_| effective_effort.to_string()); + + if let Err(e) = persist_non_null_overrides( + &config.codex_home, + config.active_profile.as_deref(), + &[ + (&[CONFIG_KEY_MODEL], model.as_deref()), + (&[CONFIG_KEY_EFFORT], effort_str.as_deref()), + ], + ) + .await + { + warn!("failed to persist overrides: {e:#}"); + } + if cwd.is_some() || approval_policy.is_some() || sandbox_policy.is_some() { sess.record_conversation_items(&[ResponseItem::from(EnvironmentContext::new( cwd, diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 338dd4a2..7ee71cb4 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -38,7 +38,7 @@ const OPENAI_DEFAULT_MODEL: &str = "gpt-5"; /// the context window. pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB -const CONFIG_TOML_FILE: &str = "config.toml"; +pub(crate) const CONFIG_TOML_FILE: &str = "config.toml"; /// Application configuration loaded from disk and merged with overrides. #[derive(Debug, Clone, PartialEq)] @@ -174,6 +174,10 @@ pub struct Config { /// Include the `view_image` tool that lets the agent attach a local image path to context. pub include_view_image_tool: bool, + + /// The active profile name used to derive this `Config` (if any). + pub active_profile: Option, + /// When true, disables burst-paste detection for typed input entirely. /// All characters are inserted as they are received, and no buffering /// or placeholder replacement will occur for fast keypress bursts. @@ -653,7 +657,11 @@ impl Config { tools_web_search_request: override_tools_web_search_request, } = overrides; - let config_profile = match config_profile_key.as_ref().or(cfg.profile.as_ref()) { + let active_profile_name = config_profile_key + .as_ref() + .or(cfg.profile.as_ref()) + .cloned(); + let config_profile = match active_profile_name.as_ref() { Some(key) => cfg .profiles .get(key) @@ -819,6 +827,7 @@ impl Config { .experimental_use_exec_command_tool .unwrap_or(false), include_view_image_tool, + active_profile: active_profile_name, disable_paste_burst: cfg.disable_paste_burst.unwrap_or(false), }; Ok(config) @@ -1193,6 +1202,7 @@ model_verbosity = "high" preferred_auth_method: AuthMode::ChatGPT, use_experimental_streamable_shell_tool: false, include_view_image_tool: true, + active_profile: Some("o3".to_string()), disable_paste_burst: false, }, o3_profile_config @@ -1249,6 +1259,7 @@ model_verbosity = "high" preferred_auth_method: AuthMode::ChatGPT, use_experimental_streamable_shell_tool: false, include_view_image_tool: true, + active_profile: Some("gpt3".to_string()), disable_paste_burst: false, }; @@ -1320,6 +1331,7 @@ model_verbosity = "high" preferred_auth_method: AuthMode::ChatGPT, use_experimental_streamable_shell_tool: false, include_view_image_tool: true, + active_profile: Some("zdr".to_string()), disable_paste_burst: false, }; @@ -1377,6 +1389,7 @@ model_verbosity = "high" preferred_auth_method: AuthMode::ChatGPT, use_experimental_streamable_shell_tool: false, include_view_image_tool: true, + active_profile: Some("gpt5".to_string()), disable_paste_burst: false, }; @@ -1452,6 +1465,4 @@ trust_level = "trusted" Ok(()) } - - // No test enforcing the presence of a standalone [projects] header. } diff --git a/codex-rs/core/src/config_edit.rs b/codex-rs/core/src/config_edit.rs new file mode 100644 index 00000000..ae41ebcb --- /dev/null +++ b/codex-rs/core/src/config_edit.rs @@ -0,0 +1,582 @@ +use crate::config::CONFIG_TOML_FILE; +use anyhow::Result; +use std::path::Path; +use tempfile::NamedTempFile; +use toml_edit::DocumentMut; + +pub const CONFIG_KEY_MODEL: &str = "model"; +pub const CONFIG_KEY_EFFORT: &str = "model_reasoning_effort"; + +/// 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 config_path = codex_home.join(CONFIG_TOML_FILE); + + let mut doc = match tokio::fs::read_to_string(&config_path).await { + Ok(s) => s.parse::()?, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + tokio::fs::create_dir_all(codex_home).await?; + DocumentMut::new() + } + Err(e) => return Err(e.into()), + }; + + let effective_profile = if let Some(p) = profile { + Some(p.to_owned()) + } else { + doc.get("profile") + .and_then(|i| i.as_str()) + .map(|s| s.to_string()) + }; + + for (segments, val) in overrides.iter().copied() { + let value = toml_edit::value(val); + if let Some(ref name) = effective_profile { + if segments.first().copied() == Some("profiles") { + apply_toml_edit_override_segments(&mut doc, segments, value); + } else { + let mut seg_buf: Vec<&str> = Vec::with_capacity(2 + segments.len()); + seg_buf.push("profiles"); + seg_buf.push(name.as_str()); + seg_buf.extend_from_slice(segments); + apply_toml_edit_override_segments(&mut doc, &seg_buf, value); + } + } else { + apply_toml_edit_override_segments(&mut doc, segments, value); + } + } + + let tmp_file = NamedTempFile::new_in(codex_home)?; + tokio::fs::write(tmp_file.path(), doc.to_string()).await?; + tmp_file.persist(config_path)?; + + Ok(()) +} + +/// 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<()> { + let filtered: Vec<(&[&str], &str)> = overrides + .iter() + .filter_map(|(k, v)| v.map(|vv| (*k, vv))) + .collect(); + + if filtered.is_empty() { + return Ok(()); + } + + persist_overrides(codex_home, profile, &filtered).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; + } + + 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); + } + } + + let maybe_item = current.get_mut(seg); + let Some(item) = maybe_item else { return }; + + if !item.is_table() { + *item = Item::Table(toml_edit::Table::new()); + if let Some(t) = item.as_table_mut() { + t.set_implicit(true); + } + } + + let Some(tbl) = item.as_table_mut() else { + return; + }; + current = tbl; + } + + let last = segments[segments.len() - 1]; + current[last] = value; +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::tempdir; + + /// 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(); + + persist_overrides( + codex_home, + None, + &[ + (&[CONFIG_KEY_MODEL], "gpt-5"), + (&[CONFIG_KEY_EFFORT], "high"), + ], + ) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = r#"model = "gpt-5" +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(); + + // 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"); + + persist_overrides( + codex_home, + None, + &[ + (&[CONFIG_KEY_MODEL], "o3"), + (&[CONFIG_KEY_EFFORT], "minimal"), + ], + ) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = r#"profile = "o3" + +[profiles.o3] +model = "o3" +model_reasoning_effort = "minimal" +"#; + assert_eq!(contents, expected); + } + + /// 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(); + + // 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"); + + persist_overrides( + codex_home, + None, + &[ + (&[CONFIG_KEY_MODEL], "o3"), + (&[CONFIG_KEY_EFFORT], "minimal"), + ], + ) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = r#"profile = "my.team name" + +[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" +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(); + + persist_overrides( + codex_home, + None, + &[ + (&["a", "b", "c"], "v"), + (&["x"], "y"), + (&["profiles", "p1", CONFIG_KEY_MODEL], "gpt-5"), + ], + ) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = r#"x = "y" + +[a.b] +c = "v" + +[profiles.p1] +model = "gpt-5" +"#; + 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"), + (&[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" +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\"\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" +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] +model_reasoning_effort = "low" +"#; + tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) + .await + .expect("seed write"); + + persist_overrides(codex_home, None, &[(&[CONFIG_KEY_MODEL], "o4-mini")]) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = r#"profile = "p1" + +[profiles.p1] +model_reasoning_effort = "low" +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(); + + // No active profile key; we'll target an explicit override + let seed = r#"[profiles.team] +model = "gpt-5" +"#; + tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), seed) + .await + .expect("seed write"); + + 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" +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")), + (&[CONFIG_KEY_EFFORT], None), + ], + ) + .await + .expect("persist"); + + let contents = read_config(codex_home).await; + let expected = "model = \"gpt-5\"\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); + } + + // 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() + } +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index c0531530..77b4037e 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -16,6 +16,7 @@ mod codex_conversation; pub mod token_data; pub use codex_conversation::CodexConversation; pub mod config; +pub mod config_edit; pub mod config_profile; pub mod config_types; mod conversation_history;