From 146135a7dd8d8f7f6d4bed088c456cdb86d59bfb Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 14 May 2026 14:53:36 +0100 Subject: [PATCH 1/2] Reject legacy profiles with profile v2 --- codex-rs/config/src/loader/mod.rs | 38 +++++--- codex-rs/config/src/loader/tests.rs | 134 ++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 codex-rs/config/src/loader/tests.rs diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 37cb41b3e175..bb2381430976 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -1,6 +1,8 @@ mod layer_io; #[cfg(target_os = "macos")] mod macos; +#[cfg(test)] +mod tests; use self::layer_io::LoadedConfigLayers; use crate::CONFIG_TOML_FILE; @@ -211,19 +213,33 @@ pub async fn load_config_layers_state( // Add the base user config layer. When profile-v2 is selected, add the // profile config as a second user layer on top so the profile only needs to // contain overrides. + let active_user_file = overrides.user_config_path(codex_home)?; let base_user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home); - layers.push( - load_user_config_layer( - fs, - &base_user_file, - /*profile*/ None, - ignore_user_config, - strict_config, - ) - .await?, - ); + let base_user_layer = load_user_config_layer( + fs, + &base_user_file, + /*profile*/ None, + ignore_user_config, + strict_config, + ) + .await?; + if active_user_profile.is_some() + && base_user_layer + .config + .as_table() + .is_some_and(|config| config.contains_key("profiles")) + { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "--profile-v2 cannot be used while {} contains legacy `[profiles]` config; move those profile settings into a profile-v2 file such as {} or remove `[profiles]`", + base_user_file.as_path().display(), + active_user_file.as_path().display() + ), + )); + } + layers.push(base_user_layer); - let active_user_file = overrides.user_config_path(codex_home)?; if active_user_file != base_user_file { layers.push( load_user_config_layer( diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs new file mode 100644 index 000000000000..13f65607e780 --- /dev/null +++ b/codex-rs/config/src/loader/tests.rs @@ -0,0 +1,134 @@ +use super::*; +use async_trait::async_trait; +use codex_file_system::CopyOptions; +use codex_file_system::CreateDirectoryOptions; +use codex_file_system::FileMetadata; +use codex_file_system::FileSystemResult; +use codex_file_system::FileSystemSandboxContext; +use codex_file_system::ReadDirectoryEntry; +use codex_file_system::RemoveOptions; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +struct TestFileSystem; + +#[async_trait] +impl ExecutorFileSystem for TestFileSystem { + async fn read_file( + &self, + path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + tokio::fs::read(path.as_path()).await + } + + async fn write_file( + &self, + _path: &AbsolutePathBuf, + _contents: Vec, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn create_directory( + &self, + _path: &AbsolutePathBuf, + _create_directory_options: CreateDirectoryOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn get_metadata( + &self, + _path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult { + unimplemented!("test filesystem only supports reads") + } + + async fn read_directory( + &self, + _path: &AbsolutePathBuf, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult> { + unimplemented!("test filesystem only supports reads") + } + + async fn remove( + &self, + _path: &AbsolutePathBuf, + _remove_options: RemoveOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } + + async fn copy( + &self, + _source_path: &AbsolutePathBuf, + _destination_path: &AbsolutePathBuf, + _copy_options: CopyOptions, + _sandbox: Option<&FileSystemSandboxContext>, + ) -> FileSystemResult<()> { + unimplemented!("test filesystem only supports reads") + } +} + +#[tokio::test] +async fn profile_v2_rejects_legacy_profiles_in_base_user_config() { + let tmp = tempdir().expect("tempdir"); + let selected_config = tmp.path().join("work.config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#" +model = "gpt-main" + +[profiles.work] +model = "gpt-work" +"#, + ) + .expect("write default user config"); + std::fs::write(&selected_config, r#"model = "gpt-work-v2""#) + .expect("write selected user config"); + + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base( + "work.config.toml", + tmp.path(), + )); + overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); + + let err = load_config_layers_state( + &TestFileSystem, + tmp.path(), + /*cwd*/ None, + &[], + overrides, + CloudRequirementsLoader::default(), + &crate::NoopThreadConfigLoader, + ) + .await + .expect_err("profile-v2 should reject legacy profiles in base user config"); + + assert_eq!( + err.kind(), + io::ErrorKind::InvalidData, + "legacy profiles should be a hard config error" + ); + let message = err.to_string(); + assert!( + message.contains("--profile-v2 cannot be used"), + "unexpected error message: {message}" + ); + assert!( + message.contains("config.toml"), + "unexpected error message: {message}" + ); + assert!( + message.contains("[profiles]"), + "unexpected error message: {message}" + ); +} From 1c2a61033e69314f06f10beb6d625e43c419bad0 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 15 May 2026 11:23:40 +0200 Subject: [PATCH 2/2] make it smoother --- codex-rs/config/src/loader/mod.rs | 14 +++++---- codex-rs/config/src/loader/tests.rs | 48 ++++++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index bb2381430976..89a35afebea8 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -223,16 +223,18 @@ pub async fn load_config_layers_state( strict_config, ) .await?; - if active_user_profile.is_some() - && base_user_layer - .config - .as_table() - .is_some_and(|config| config.contains_key("profiles")) + if let Some(active_user_profile) = active_user_profile.as_ref() + && base_user_layer.config.as_table().is_some_and(|config| { + config + .get("profiles") + .and_then(TomlValue::as_table) + .is_some_and(|profiles| profiles.contains_key(active_user_profile.as_str())) + }) { return Err(io::Error::new( io::ErrorKind::InvalidData, format!( - "--profile-v2 cannot be used while {} contains legacy `[profiles]` config; move those profile settings into a profile-v2 file such as {} or remove `[profiles]`", + "--profile-v2 `{active_user_profile}` cannot be used while {} contains legacy `[profiles.{active_user_profile}]` config; move those settings into {} or remove `[profiles.{active_user_profile}]`", base_user_file.as_path().display(), active_user_file.as_path().display() ), diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 13f65607e780..2d9462bfa781 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -77,7 +77,7 @@ impl ExecutorFileSystem for TestFileSystem { } #[tokio::test] -async fn profile_v2_rejects_legacy_profiles_in_base_user_config() { +async fn profile_v2_rejects_matching_legacy_profile_in_base_user_config() { let tmp = tempdir().expect("tempdir"); let selected_config = tmp.path().join("work.config.toml"); @@ -111,16 +111,16 @@ model = "gpt-work" &crate::NoopThreadConfigLoader, ) .await - .expect_err("profile-v2 should reject legacy profiles in base user config"); + .expect_err("profile-v2 should reject a matching legacy profile in base user config"); assert_eq!( err.kind(), io::ErrorKind::InvalidData, - "legacy profiles should be a hard config error" + "a matching legacy profile should be a hard config error" ); let message = err.to_string(); assert!( - message.contains("--profile-v2 cannot be used"), + message.contains("--profile-v2 `work` cannot be used"), "unexpected error message: {message}" ); assert!( @@ -128,7 +128,45 @@ model = "gpt-work" "unexpected error message: {message}" ); assert!( - message.contains("[profiles]"), + message.contains("[profiles.work]"), "unexpected error message: {message}" ); } + +#[tokio::test] +async fn profile_v2_allows_unrelated_legacy_profiles_in_base_user_config() { + let tmp = tempdir().expect("tempdir"); + let selected_config = tmp.path().join("work.config.toml"); + + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#" +model = "gpt-main" + +[profiles.dev] +model = "gpt-dev" +"#, + ) + .expect("write default user config"); + std::fs::write(&selected_config, r#"model = "gpt-work-v2""#) + .expect("write selected user config"); + + let mut overrides = LoaderOverrides::without_managed_config_for_tests(); + overrides.user_config_path = Some(AbsolutePathBuf::resolve_path_against_base( + "work.config.toml", + tmp.path(), + )); + overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); + + load_config_layers_state( + &TestFileSystem, + tmp.path(), + /*cwd*/ None, + &[], + overrides, + CloudRequirementsLoader::default(), + &crate::NoopThreadConfigLoader, + ) + .await + .expect("profile-v2 should allow unrelated legacy profiles in base user config"); +}