From 37a85338de32aa659c4030d977b2d7443c49de41 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 5 Jan 2026 14:55:39 -0800 Subject: [PATCH] fix: remove CODEX_MANAGED_CONFIG_PATH environment variable --- .../app-server/src/codex_message_processor.rs | 2 +- codex-rs/app-server/src/config_api.rs | 9 ++++- codex-rs/app-server/src/lib.rs | 12 +++++- codex-rs/app-server/src/main.rs | 34 ++++++++++++++++- codex-rs/app-server/src/message_processor.rs | 4 +- .../app-server/tests/suite/v2/config_rpc.rs | 5 ++- codex-rs/cli/src/main.rs | 7 +++- codex-rs/core/src/config/service.rs | 37 +++++++++---------- codex-rs/core/src/config_loader/layer_io.rs | 6 +-- 9 files changed, 83 insertions(+), 33 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 5930fe43559..c1166d95088 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1103,7 +1103,7 @@ impl CodexMessageProcessor { } async fn get_user_saved_config(&self, request_id: RequestId) { - let service = ConfigService::new(self.config.codex_home.clone(), Vec::new()); + let service = ConfigService::new_with_defaults(self.config.codex_home.clone()); let user_saved_config: UserSavedConfig = match service.load_user_saved_config().await { Ok(config) => config, Err(err) => { diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 98e0f108e93..b06de3f1e45 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -9,6 +9,7 @@ use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::JSONRPCErrorError; use codex_core::config::ConfigService; use codex_core::config::ConfigServiceError; +use codex_core::config_loader::LoaderOverrides; use serde_json::json; use std::path::PathBuf; use toml::Value as TomlValue; @@ -19,9 +20,13 @@ pub(crate) struct ConfigApi { } impl ConfigApi { - pub(crate) fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { + pub(crate) fn new( + codex_home: PathBuf, + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, + ) -> Self { Self { - service: ConfigService::new(codex_home, cli_overrides), + service: ConfigService::new(codex_home, cli_overrides, loader_overrides), } } diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 224e0da10be..68663a991db 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -1,7 +1,8 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] use codex_common::CliConfigOverrides; -use codex_core::config::Config; +use codex_core::config::ConfigBuilder; +use codex_core::config_loader::LoaderOverrides; use std::io::ErrorKind; use std::io::Result as IoResult; use std::path::PathBuf; @@ -42,6 +43,7 @@ const CHANNEL_CAPACITY: usize = 128; pub async fn run_main( codex_linux_sandbox_exe: Option, cli_config_overrides: CliConfigOverrides, + loader_overrides: LoaderOverrides, ) -> IoResult<()> { // Set up channels. let (incoming_tx, mut incoming_rx) = mpsc::channel::(CHANNEL_CAPACITY); @@ -78,7 +80,11 @@ pub async fn run_main( format!("error parsing -c overrides: {e}"), ) })?; - let config = Config::load_with_cli_overrides(cli_kv_overrides.clone()) + let loader_overrides_for_config_api = loader_overrides.clone(); + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides.clone()) + .loader_overrides(loader_overrides) + .build() .await .map_err(|e| { std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}")) @@ -120,11 +126,13 @@ pub async fn run_main( let processor_handle = tokio::spawn({ let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let cli_overrides: Vec<(String, TomlValue)> = cli_kv_overrides.clone(); + let loader_overrides = loader_overrides_for_config_api; let mut processor = MessageProcessor::new( outgoing_message_sender, codex_linux_sandbox_exe, std::sync::Arc::new(config), cli_overrides, + loader_overrides, feedback.clone(), ); async move { diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index 689ec0877a7..be57311e83d 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -1,10 +1,42 @@ use codex_app_server::run_main; use codex_arg0::arg0_dispatch_or_else; use codex_common::CliConfigOverrides; +use codex_core::config_loader::LoaderOverrides; +use std::path::PathBuf; + +// Debug-only test hook: lets integration tests point the server at a temporary +// managed config file without writing to /etc. +const MANAGED_CONFIG_PATH_ENV_VAR: &str = "CODEX_APP_SERVER_MANAGED_CONFIG_PATH"; fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { - run_main(codex_linux_sandbox_exe, CliConfigOverrides::default()).await?; + let managed_config_path = managed_config_path_from_debug_env(); + let loader_overrides = LoaderOverrides { + managed_config_path, + ..Default::default() + }; + + run_main( + codex_linux_sandbox_exe, + CliConfigOverrides::default(), + loader_overrides, + ) + .await?; Ok(()) }) } + +fn managed_config_path_from_debug_env() -> Option { + #[cfg(debug_assertions)] + { + if let Ok(value) = std::env::var(MANAGED_CONFIG_PATH_ENV_VAR) { + return if value.is_empty() { + None + } else { + Some(PathBuf::from(value)) + }; + } + } + + None +} diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 6a6cf5edb25..be57ad39700 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -20,6 +20,7 @@ use codex_app_server_protocol::RequestId; use codex_core::AuthManager; use codex_core::ConversationManager; use codex_core::config::Config; +use codex_core::config_loader::LoaderOverrides; use codex_core::default_client::USER_AGENT_SUFFIX; use codex_core::default_client::get_codex_user_agent; use codex_feedback::CodexFeedback; @@ -41,6 +42,7 @@ impl MessageProcessor { codex_linux_sandbox_exe: Option, config: Arc, cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, feedback: CodexFeedback, ) -> Self { let outgoing = Arc::new(outgoing); @@ -62,7 +64,7 @@ impl MessageProcessor { cli_overrides.clone(), feedback, ); - let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides); + let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides, loader_overrides); Self { outgoing, diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index c0be58f50c7..18311d324b8 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -184,7 +184,10 @@ writable_roots = [{}] let mut mcp = McpProcess::new_with_env( codex_home.path(), - &[("CODEX_MANAGED_CONFIG_PATH", Some(&managed_path_str))], + &[( + "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", + Some(&managed_path_str), + )], ) .await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index ae6dabe6729..5cf4678bfc3 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -480,7 +480,12 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() } Some(Subcommand::AppServer(app_server_cli)) => match app_server_cli.subcommand { None => { - codex_app_server::run_main(codex_linux_sandbox_exe, root_config_overrides).await?; + codex_app_server::run_main( + codex_linux_sandbox_exe, + root_config_overrides, + codex_core::config_loader::LoaderOverrides::default(), + ) + .await?; } Some(AppServerSubcommand::GenerateTs(gen_cli)) => { codex_app_server_protocol::generate_ts( diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 211a12fa037..d65ed10a050 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -106,16 +106,7 @@ pub struct ConfigService { } impl ConfigService { - pub fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { - Self { - codex_home, - cli_overrides, - loader_overrides: LoaderOverrides::default(), - } - } - - #[cfg(test)] - fn with_overrides( + pub fn new( codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>, loader_overrides: LoaderOverrides, @@ -127,6 +118,14 @@ impl ConfigService { } } + pub fn new_with_defaults(codex_home: PathBuf) -> Self { + Self { + codex_home, + cli_overrides: Vec::new(), + loader_overrides: LoaderOverrides::default(), + } + } + pub async fn read( &self, params: ConfigReadParams, @@ -707,7 +706,7 @@ unified_exec = true "#; std::fs::write(tmp.path().join(CONFIG_TOML_FILE), original)?; - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -748,7 +747,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -829,7 +828,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -881,7 +880,7 @@ remote_compaction = true let user_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&user_path, "model = \"user\"").unwrap(); - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); let error = service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -904,7 +903,7 @@ remote_compaction = true let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: None, @@ -932,7 +931,7 @@ remote_compaction = true let managed_path = tmp.path().join("managed_config.toml"); std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -980,7 +979,7 @@ remote_compaction = true TomlValue::String("session".to_string()), )]; - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), cli_overrides, LoaderOverrides { @@ -1026,7 +1025,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -1085,7 +1084,7 @@ alpha = "a" std::fs::write(&path, base)?; - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(path.display().to_string()), diff --git a/codex-rs/core/src/config_loader/layer_io.rs b/codex-rs/core/src/config_loader/layer_io.rs index 84a29a6119c..0ece69b4710 100644 --- a/codex-rs/core/src/config_loader/layer_io.rs +++ b/codex-rs/core/src/config_loader/layer_io.rs @@ -93,12 +93,8 @@ pub(super) async fn read_config_from_path( } } -/// Return the default managed config path (honoring `CODEX_MANAGED_CONFIG_PATH`). +/// Return the default managed config path. pub(super) fn managed_config_default_path(codex_home: &Path) -> PathBuf { - if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") { - return PathBuf::from(path); - } - #[cfg(unix)] { let _ = codex_home;