diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 15ca924a8b42..0d2772fdb71b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1499,11 +1499,13 @@ dependencies = [ "sha2", "shlex", "tempfile", + "thiserror 2.0.18", "time", "tokio", "tokio-tungstenite", "tokio-util", "toml 0.9.11+spec-1.1.0", + "toml_edit 0.24.0+spec-1.1.0", "tracing", "tracing-opentelemetry", "tracing-subscriber", diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index f35453964041..ea49058e6cf5 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -72,8 +72,10 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha2 = { workspace = true } tempfile = { workspace = true } +thiserror = { workspace = true } time = { workspace = true } toml = { workspace = true } +toml_edit = { workspace = true } tokio = { workspace = true, features = [ "io-std", "macros", diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 7a2ada0765f6..873dc5a5a1ad 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2,7 +2,7 @@ use crate::bespoke_event_handling::apply_bespoke_event_handling; use crate::bespoke_event_handling::maybe_emit_hook_prompt_item_completed; use crate::command_exec::CommandExecManager; use crate::command_exec::StartCommandExecParams; -use crate::config_api::apply_runtime_feature_enablement; +use crate::config_manager::ConfigManager; use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE; use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_PARAMS_ERROR_CODE; @@ -212,8 +212,6 @@ use codex_arg0::Arg0DispatchPaths; use codex_backend_client::AddCreditsNudgeCreditType as BackendAddCreditsNudgeCreditType; use codex_backend_client::Client as BackendClient; use codex_chatgpt::connectors; -use codex_cloud_requirements::cloud_requirements_loader; -use codex_config::ThreadConfigLoader; use codex_config::types::McpServerTransportConfig; use codex_core::CodexThread; use codex_core::ForkSnapshot; @@ -231,9 +229,6 @@ use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config_loader::CloudRequirementsLoadError; use codex_core::config_loader::CloudRequirementsLoadErrorCode; -use codex_core::config_loader::CloudRequirementsLoader; -use codex_core::config_loader::LoaderOverrides; -use codex_core::config_loader::load_config_layers_state; use codex_core::config_loader::project_trust_key; use codex_core::exec::ExecCapturePolicy; use codex_core::exec::ExecExpiration; @@ -280,7 +275,6 @@ use codex_login::ServerOptions as LoginServerOptions; use codex_login::ShutdownHandle; use codex_login::auth::login_with_chatgpt_auth_tokens; use codex_login::complete_device_code_login; -use codex_login::default_client::set_default_client_residency_requirement; use codex_login::login_with_api_key; use codex_login::request_device_code; use codex_login::run_login_server; @@ -348,16 +342,13 @@ use codex_thread_store::ThreadStore; use codex_thread_store::ThreadStoreError; use codex_thread_store::UpdateThreadMetadataParams as StoreUpdateThreadMetadataParams; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_json_to_toml::json_to_toml; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; -use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::io::Error as IoError; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use std::sync::RwLock; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Duration; @@ -478,10 +469,7 @@ pub(crate) struct CodexMessageProcessor { arg0_paths: Arg0DispatchPaths, config: Arc, thread_store: Arc, - cli_overrides: Arc>>, - runtime_feature_enablement: Arc>>, - cloud_requirements: Arc>, - thread_config_loader: Arc, + config_manager: ConfigManager, active_login: Arc>>, pending_thread_unloads: Arc>>, thread_state_manager: ThreadStateManager, @@ -638,11 +626,10 @@ pub(crate) struct CodexMessageProcessorArgs { pub(crate) outgoing: Arc, pub(crate) analytics_events_client: AnalyticsEventsClient, pub(crate) arg0_paths: Arg0DispatchPaths, + /// Startup config used as the process baseline. Fresh effective config loads + /// go through `config_manager`. pub(crate) config: Arc, - pub(crate) cli_overrides: Arc>>, - pub(crate) runtime_feature_enablement: Arc>>, - pub(crate) cloud_requirements: Arc>, - pub(crate) thread_config_loader: Arc, + pub(crate) config_manager: ConfigManager, pub(crate) feedback: CodexFeedback, pub(crate) log_db: Option, } @@ -727,10 +714,7 @@ impl CodexMessageProcessor { analytics_events_client, arg0_paths, config, - cli_overrides, - runtime_feature_enablement, - cloud_requirements, - thread_config_loader, + config_manager, feedback, log_db, } = args; @@ -742,10 +726,7 @@ impl CodexMessageProcessor { arg0_paths, thread_store: configured_thread_store(&config), config, - cli_overrides, - runtime_feature_enablement, - cloud_requirements, - thread_config_loader, + config_manager, active_login: Arc::new(Mutex::new(None)), pending_thread_unloads: Arc::new(Mutex::new(HashSet::new())), thread_state_manager: ThreadStateManager::new(), @@ -763,44 +744,14 @@ impl CodexMessageProcessor { &self, fallback_cwd: Option, ) -> Result { - let cloud_requirements = self.current_cloud_requirements(); - let mut config = codex_core::config::ConfigBuilder::default() - .cli_overrides(self.current_cli_overrides()) - .fallback_cwd(fallback_cwd) - .cloud_requirements(cloud_requirements) - .build() + self.config_manager + .load_latest_config(fallback_cwd) .await .map_err(|err| JSONRPCErrorError { code: INTERNAL_ERROR_CODE, message: format!("failed to reload config: {err}"), data: None, - })?; - apply_runtime_feature_enablement(&mut config, &self.current_runtime_feature_enablement()); - config.codex_self_exe = self.arg0_paths.codex_self_exe.clone(); - config.codex_linux_sandbox_exe = self.arg0_paths.codex_linux_sandbox_exe.clone(); - config.main_execve_wrapper_exe = self.arg0_paths.main_execve_wrapper_exe.clone(); - Ok(config) - } - - fn current_cloud_requirements(&self) -> CloudRequirementsLoader { - self.cloud_requirements - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() - } - - fn current_cli_overrides(&self) -> Vec<(String, TomlValue)> { - self.cli_overrides - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() - } - - fn current_runtime_feature_enablement(&self) -> BTreeMap { - self.runtime_feature_enablement - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() + }) } /// If a client sends `developer_instructions: null` during a mode switch, @@ -1419,10 +1370,8 @@ impl CodexMessageProcessor { let outgoing_clone = self.outgoing.clone(); let active_login = self.active_login.clone(); let auth_manager = self.auth_manager.clone(); - let cloud_requirements = self.cloud_requirements.clone(); + let config_manager = self.config_manager.clone(); let chatgpt_base_url = self.config.chatgpt_base_url.clone(); - let codex_home = self.config.codex_home.to_path_buf(); - let cli_overrides = self.current_cli_overrides(); let auth_url = server.auth_url.clone(); tokio::spawn(async move { let (success, error_msg) = match tokio::time::timeout( @@ -1452,17 +1401,13 @@ impl CodexMessageProcessor { if success { auth_manager.reload(); - replace_cloud_requirements_loader( - cloud_requirements.as_ref(), + config_manager.replace_cloud_requirements_loader( auth_manager.clone(), chatgpt_base_url, - codex_home, ); - sync_default_client_residency_requirement( - &cli_overrides, - cloud_requirements.as_ref(), - ) - .await; + config_manager + .sync_default_client_residency_requirement() + .await; // Notify clients with the actual current auth mode. let auth = auth_manager.auth_cached(); @@ -1536,10 +1481,8 @@ impl CodexMessageProcessor { let outgoing_clone = self.outgoing.clone(); let active_login = self.active_login.clone(); let auth_manager = self.auth_manager.clone(); - let cloud_requirements = self.cloud_requirements.clone(); + let config_manager = self.config_manager.clone(); let chatgpt_base_url = self.config.chatgpt_base_url.clone(); - let codex_home = self.config.codex_home.to_path_buf(); - let cli_overrides = self.current_cli_overrides(); tokio::spawn(async move { let (success, error_msg) = tokio::select! { _ = cancel.cancelled() => { @@ -1566,17 +1509,13 @@ impl CodexMessageProcessor { if success { auth_manager.reload(); - replace_cloud_requirements_loader( - cloud_requirements.as_ref(), + config_manager.replace_cloud_requirements_loader( auth_manager.clone(), chatgpt_base_url, - codex_home, ); - sync_default_client_residency_requirement( - &cli_overrides, - cloud_requirements.as_ref(), - ) - .await; + config_manager + .sync_default_client_residency_requirement() + .await; let auth = auth_manager.auth_cached(); let payload_v2 = AccountUpdatedNotification { @@ -1706,14 +1645,12 @@ impl CodexMessageProcessor { return; } self.auth_manager.reload(); - replace_cloud_requirements_loader( - self.cloud_requirements.as_ref(), + self.config_manager.replace_cloud_requirements_loader( self.auth_manager.clone(), self.config.chatgpt_base_url.clone(), - self.config.codex_home.to_path_buf(), ); - let cli_overrides = self.current_cli_overrides(); - sync_default_client_residency_requirement(&cli_overrides, self.cloud_requirements.as_ref()) + self.config_manager + .sync_default_client_residency_requirement() .await; self.outgoing @@ -2424,8 +2361,6 @@ impl CodexMessageProcessor { personality, ); typesafe_overrides.ephemeral = ephemeral; - let cloud_requirements = self.current_cloud_requirements(); - let cli_overrides = self.current_cli_overrides(); let listener_task_context = ListenerTaskContext { thread_manager: Arc::clone(&self.thread_manager), thread_state_manager: self.thread_state_manager.clone(), @@ -2438,15 +2373,11 @@ impl CodexMessageProcessor { codex_home: self.config.codex_home.to_path_buf(), }; let request_trace = request_context.request_trace(); - let runtime_feature_enablement = self.current_runtime_feature_enablement(); - let thread_config_loader = Arc::clone(&self.thread_config_loader); + let config_manager = self.config_manager.clone(); let thread_start_task = async move { Self::thread_start_task( listener_task_context, - cli_overrides, - runtime_feature_enablement, - cloud_requirements, - thread_config_loader, + config_manager, request_id, app_server_client_name, app_server_client_version, @@ -2520,10 +2451,7 @@ impl CodexMessageProcessor { #[allow(clippy::too_many_arguments)] async fn thread_start_task( listener_task_context: ListenerTaskContext, - cli_overrides: Vec<(String, TomlValue)>, - runtime_feature_enablement: BTreeMap, - cloud_requirements: CloudRequirementsLoader, - thread_config_loader: Arc, + config_manager: ConfigManager, request_id: ConnectionRequestId, app_server_client_name: Option, app_server_client_version: Option, @@ -2537,16 +2465,9 @@ impl CodexMessageProcessor { request_trace: Option, ) { let requested_cwd = typesafe_overrides.cwd.clone(); - let mut config = match derive_config_from_params( - &cli_overrides, - config_overrides.clone(), - typesafe_overrides.clone(), - Arc::clone(&thread_config_loader), - &cloud_requirements, - &listener_task_context.codex_home, - &runtime_feature_enablement, - ) - .await + let mut config = match config_manager + .load_with_overrides(config_overrides.clone(), typesafe_overrides.clone()) + .await { Ok(config) => config, Err(err) => { @@ -2585,6 +2506,7 @@ impl CodexMessageProcessor { let trust_target = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &config.cwd) .await .unwrap_or_else(|| config.cwd.clone()); + let current_cli_overrides = config_manager.current_cli_overrides(); let cli_overrides_with_trust; let cli_overrides_for_reload = if let Err(err) = codex_core::config::set_project_trust_level( @@ -2606,7 +2528,7 @@ impl CodexMessageProcessor { project_trust_key(trust_target.as_path()), TomlValue::Table(project), ); - cli_overrides_with_trust = cli_overrides + cli_overrides_with_trust = current_cli_overrides .iter() .cloned() .chain(std::iter::once(( @@ -2616,19 +2538,17 @@ impl CodexMessageProcessor { .collect::>(); cli_overrides_with_trust.as_slice() } else { - &cli_overrides + current_cli_overrides.as_slice() }; - config = match derive_config_from_params( - cli_overrides_for_reload, - config_overrides, - typesafe_overrides, - thread_config_loader, - &cloud_requirements, - &listener_task_context.codex_home, - &runtime_feature_enablement, - ) - .await + config = match config_manager + .load_with_cli_overrides( + cli_overrides_for_reload, + config_overrides, + typesafe_overrides, + /*fallback_cwd*/ None, + ) + .await { Ok(config) => config, Err(err) => { @@ -4412,19 +4332,10 @@ impl CodexMessageProcessor { .await; // Derive a Config using the same logic as new conversation, honoring overrides if provided. - let cloud_requirements = self.current_cloud_requirements(); - let cli_overrides = self.current_cli_overrides(); - let runtime_feature_enablement = self.current_runtime_feature_enablement(); - let config = match derive_config_for_cwd( - &cli_overrides, - request_overrides, - typesafe_overrides, - history_cwd, - &cloud_requirements, - &self.config.codex_home, - &runtime_feature_enablement, - ) - .await + let config = match self + .config_manager + .load_for_cwd(request_overrides, typesafe_overrides, history_cwd) + .await { Ok(config) => config, Err(err) => { @@ -4992,19 +4903,10 @@ impl CodexMessageProcessor { ); typesafe_overrides.ephemeral = ephemeral.then_some(true); // Derive a Config using the same logic as new conversation, honoring overrides if provided. - let cloud_requirements = self.current_cloud_requirements(); - let cli_overrides = self.current_cli_overrides(); - let runtime_feature_enablement = self.current_runtime_feature_enablement(); - let config = match derive_config_for_cwd( - &cli_overrides, - request_overrides, - typesafe_overrides, - history_cwd, - &cloud_requirements, - &self.config.codex_home, - &runtime_feature_enablement, - ) - .await + let config = match self + .config_manager + .load_for_cwd(request_overrides, typesafe_overrides, history_cwd) + .await { Ok(config) => config, Err(err) => { @@ -6577,8 +6479,6 @@ impl CodexMessageProcessor { return; } }; - let cli_overrides = self.current_cli_overrides(); - let host_name = codex_config::host_name(); let mut data = Vec::new(); for cwd in cwds { let extra_roots = extra_roots_by_cwd @@ -6599,17 +6499,10 @@ impl CodexMessageProcessor { continue; } }; - let config_layer_stack = match load_config_layers_state( - LOCAL_FS.as_ref(), - &self.config.codex_home, - Some(cwd_abs.clone()), - &cli_overrides, - LoaderOverrides::default(), - CloudRequirementsLoader::default(), - self.thread_config_loader.as_ref(), - host_name.as_deref(), - ) - .await + let config_layer_stack = match self + .config_manager + .load_config_layers_for_cwd(cwd_abs.clone()) + .await { Ok(config_layer_stack) => config_layer_stack, Err(err) => { @@ -8765,30 +8658,25 @@ impl CodexMessageProcessor { WindowsSandboxSetupMode::Unelevated => CoreWindowsSandboxSetupMode::Unelevated, }; let config = Arc::clone(&self.config); - let cloud_requirements = self.current_cloud_requirements(); + let config_manager = self.config_manager.clone(); let command_cwd = params .cwd .map(PathBuf::from) .unwrap_or_else(|| config.cwd.to_path_buf()); - let cli_overrides = self.current_cli_overrides(); - let runtime_feature_enablement = self.current_runtime_feature_enablement(); let outgoing = Arc::clone(&self.outgoing); let connection_id = request_id.connection_id; tokio::spawn(async move { - let derived_config = derive_config_for_cwd( - &cli_overrides, - /*request_overrides*/ None, - ConfigOverrides { - cwd: Some(command_cwd.clone()), - ..Default::default() - }, - Some(command_cwd.clone()), - &cloud_requirements, - &config.codex_home, - &runtime_feature_enablement, - ) - .await; + let derived_config = config_manager + .load_for_cwd( + /*request_overrides*/ None, + ConfigOverrides { + cwd: Some(command_cwd.clone()), + ..Default::default() + }, + Some(command_cwd.clone()), + ) + .await; let setup_result = match derived_config { Ok(config) => { let setup_request = WindowsSandboxSetupRequest { @@ -9518,116 +9406,6 @@ fn validate_dynamic_tools(tools: &[ApiDynamicToolSpec]) -> Result<(), String> { Ok(()) } -fn replace_cloud_requirements_loader( - cloud_requirements: &RwLock, - auth_manager: Arc, - chatgpt_base_url: String, - codex_home: PathBuf, -) { - let loader = cloud_requirements_loader(auth_manager, chatgpt_base_url, codex_home); - if let Ok(mut guard) = cloud_requirements.write() { - *guard = loader; - } else { - warn!("failed to update cloud requirements loader"); - } -} - -async fn sync_default_client_residency_requirement( - cli_overrides: &[(String, TomlValue)], - cloud_requirements: &RwLock, -) { - let loader = cloud_requirements - .read() - .map(|guard| guard.clone()) - .unwrap_or_default(); - match codex_core::config::ConfigBuilder::default() - .cli_overrides(cli_overrides.to_vec()) - .cloud_requirements(loader) - .build() - .await - { - Ok(config) => set_default_client_residency_requirement(config.enforce_residency.value()), - Err(err) => warn!( - error = %err, - "failed to sync default client residency requirement after auth refresh" - ), - } -} - -/// Derive the effective [`Config`] by layering three override sources. -/// -/// Precedence (lowest to highest): -/// - `cli_overrides`: process-wide startup `--config` flags. -/// - `request_overrides`: per-request dotted-path overrides (`params.config`), converted JSON->TOML. -/// - `typesafe_overrides`: Request objects such as `NewThreadParams` and -/// `ThreadStartParams` support a limited set of _explicit_ config overrides, so -/// `typesafe_overrides` is a `ConfigOverrides` derived from the respective request object. -/// Because the overrides are defined explicitly in the `*Params`, this takes priority over -/// the more general "bag of config options" provided by `cli_overrides` and `request_overrides`. -async fn derive_config_from_params( - cli_overrides: &[(String, TomlValue)], - request_overrides: Option>, - typesafe_overrides: ConfigOverrides, - thread_config_loader: Arc, - cloud_requirements: &CloudRequirementsLoader, - codex_home: &Path, - runtime_feature_enablement: &BTreeMap, -) -> std::io::Result { - let merged_cli_overrides = cli_overrides - .iter() - .cloned() - .chain( - request_overrides - .unwrap_or_default() - .into_iter() - .map(|(k, v)| (k, json_to_toml(v))), - ) - .collect::>(); - - let mut config = codex_core::config::ConfigBuilder::default() - .codex_home(codex_home.to_path_buf()) - .cli_overrides(merged_cli_overrides) - .harness_overrides(typesafe_overrides) - .cloud_requirements(cloud_requirements.clone()) - .thread_config_loader(thread_config_loader) - .build() - .await?; - apply_runtime_feature_enablement(&mut config, runtime_feature_enablement); - Ok(config) -} - -async fn derive_config_for_cwd( - cli_overrides: &[(String, TomlValue)], - request_overrides: Option>, - typesafe_overrides: ConfigOverrides, - cwd: Option, - cloud_requirements: &CloudRequirementsLoader, - codex_home: &Path, - runtime_feature_enablement: &BTreeMap, -) -> std::io::Result { - let merged_cli_overrides = cli_overrides - .iter() - .cloned() - .chain( - request_overrides - .unwrap_or_default() - .into_iter() - .map(|(k, v)| (k, json_to_toml(v))), - ) - .collect::>(); - - let mut config = codex_core::config::ConfigBuilder::default() - .codex_home(codex_home.to_path_buf()) - .cli_overrides(merged_cli_overrides) - .harness_overrides(typesafe_overrides) - .fallback_cwd(cwd) - .cloud_requirements(cloud_requirements.clone()) - .build() - .await?; - apply_runtime_feature_enablement(&mut config, runtime_feature_enablement); - Ok(config) -} - async fn read_history_cwd_from_state_db( config: &Config, thread_id: Option, @@ -10523,6 +10301,8 @@ mod tests { use codex_config::SessionThreadConfig; use codex_config::StaticThreadConfigLoader; use codex_config::ThreadConfigSource; + use codex_core::config_loader::CloudRequirementsLoader; + use codex_core::config_loader::LoaderOverrides; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::WireApi; use codex_protocol::ThreadId; @@ -10536,8 +10316,10 @@ mod tests { use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; use serde_json::json; + use std::collections::BTreeMap; use std::path::PathBuf; use std::sync::Arc; + use std::sync::RwLock; use tempfile::TempDir; #[test] @@ -10811,21 +10593,13 @@ mod tests { requires_openai_auth: false, supports_websockets: true, }; - let config = derive_config_from_params( - &[], - Some(HashMap::from([ - ("model_provider".to_string(), json!("request")), - ("features.plugins".to_string(), json!(true)), - ( - "model_providers.session".to_string(), - json!({ - "name": "request", - "base_url": "http://127.0.0.1:9999/api/codex", - "wire_api": "responses", - }), - ), - ])), - ConfigOverrides::default(), + let config_manager = ConfigManager::new( + temp_dir.path().to_path_buf(), + Arc::new(RwLock::new(Vec::new())), + Arc::new(RwLock::new(BTreeMap::new())), + LoaderOverrides::default(), + Arc::new(RwLock::new(CloudRequirementsLoader::default())), + Arg0DispatchPaths::default(), Arc::new(StaticThreadConfigLoader::new(vec![ ThreadConfigSource::Session(SessionThreadConfig { model_provider: Some("session".to_string()), @@ -10836,11 +10610,24 @@ mod tests { features: BTreeMap::from([("plugins".to_string(), false)]), }), ])), - &CloudRequirementsLoader::default(), - temp_dir.path(), - &BTreeMap::new(), - ) - .await?; + ); + let config = config_manager + .load_with_overrides( + Some(HashMap::from([ + ("model_provider".to_string(), json!("request")), + ("features.plugins".to_string(), json!(true)), + ( + "model_providers.session".to_string(), + json!({ + "name": "request", + "base_url": "http://127.0.0.1:9999/api/codex", + "wire_api": "responses", + }), + ), + ])), + ConfigOverrides::default(), + ) + .await?; assert_eq!(config.model_provider_id, "session"); assert_eq!(config.model_provider, session_provider); diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index c57d3893c12c..ffccd6b1b242 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -1,3 +1,5 @@ +use crate::config_manager::ConfigManager; +use crate::config_manager_service::ConfigManagerError; use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_REQUEST_ERROR_CODE; use async_trait::async_trait; @@ -19,11 +21,7 @@ use codex_app_server_protocol::NetworkUnixSocketPermission; use codex_app_server_protocol::SandboxMode; use codex_core::ThreadManager; use codex_core::config::Config; -use codex_core::config::ConfigService; -use codex_core::config::ConfigServiceError; -use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::ConfigRequirementsToml; -use codex_core::config_loader::LoaderOverrides; use codex_core::config_loader::ResidencyRequirement as CoreResidencyRequirement; use codex_core::config_loader::SandboxModeRequirement as CoreSandboxModeRequirement; use codex_core::plugins::PluginId; @@ -34,12 +32,8 @@ use codex_features::feature_for_key; use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::Op; use serde_json::json; -use std::collections::BTreeMap; -use std::collections::BTreeSet; use std::path::PathBuf; use std::sync::Arc; -use std::sync::RwLock; -use toml::Value as TomlValue; use tracing::warn; const SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT: &[&str] = &[ @@ -72,86 +66,36 @@ impl UserConfigReloader for ThreadManager { #[derive(Clone)] pub(crate) struct ConfigApi { - codex_home: PathBuf, - cli_overrides: Arc>>, - runtime_feature_enablement: Arc>>, - loader_overrides: LoaderOverrides, - cloud_requirements: Arc>, + config_manager: ConfigManager, user_config_reloader: Arc, analytics_events_client: AnalyticsEventsClient, } impl ConfigApi { pub(crate) fn new( - codex_home: PathBuf, - cli_overrides: Arc>>, - runtime_feature_enablement: Arc>>, - loader_overrides: LoaderOverrides, - cloud_requirements: Arc>, + config_manager: ConfigManager, user_config_reloader: Arc, analytics_events_client: AnalyticsEventsClient, ) -> Self { Self { - codex_home, - cli_overrides, - runtime_feature_enablement, - loader_overrides, - cloud_requirements, + config_manager, user_config_reloader, analytics_events_client, } } - fn config_service(&self) -> ConfigService { - ConfigService::new( - self.codex_home.clone(), - self.current_cli_overrides(), - self.loader_overrides.clone(), - self.current_cloud_requirements(), - codex_config::host_name(), - ) - } - - fn current_cli_overrides(&self) -> Vec<(String, TomlValue)> { - self.cli_overrides - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() - } - - fn current_runtime_feature_enablement(&self) -> BTreeMap { - self.runtime_feature_enablement - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() - } - - fn current_cloud_requirements(&self) -> CloudRequirementsLoader { - self.cloud_requirements - .read() - .map(|guard| guard.clone()) - .unwrap_or_default() - } - pub(crate) async fn load_latest_config( &self, fallback_cwd: Option, ) -> Result { - let mut config = codex_core::config::ConfigBuilder::default() - .codex_home(self.codex_home.clone()) - .cli_overrides(self.current_cli_overrides()) - .loader_overrides(self.loader_overrides.clone()) - .fallback_cwd(fallback_cwd) - .cloud_requirements(self.current_cloud_requirements()) - .build() + self.config_manager + .load_latest_config(fallback_cwd) .await .map_err(|err| JSONRPCErrorError { code: INTERNAL_ERROR_CODE, message: format!("failed to resolve feature override precedence: {err}"), data: None, - })?; - apply_runtime_feature_enablement(&mut config, &self.current_runtime_feature_enablement()); - Ok(config) + }) } pub(crate) async fn read( @@ -159,11 +103,7 @@ impl ConfigApi { params: ConfigReadParams, ) -> Result { let fallback_cwd = params.cwd.as_ref().map(PathBuf::from); - let mut response = self - .config_service() - .read(params) - .await - .map_err(map_error)?; + let mut response = self.config_manager.read(params).await.map_err(map_error)?; let config = self.load_latest_config(fallback_cwd).await?; for feature_key in SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT { let Some(feature) = feature_for_key(feature_key) else { @@ -191,7 +131,7 @@ impl ConfigApi { &self, ) -> Result { let requirements = self - .config_service() + .config_manager .read_requirements() .await .map_err(map_error)? @@ -207,7 +147,7 @@ impl ConfigApi { let pending_changes = collect_plugin_enabled_candidates([(¶ms.key_path, ¶ms.value)].into_iter()); let response = self - .config_service() + .config_manager .write_value(params) .await .map_err(map_error)?; @@ -227,7 +167,7 @@ impl ConfigApi { .map(|edit| (&edit.key_path, &edit.value)), ); let response = self - .config_service() + .config_manager .batch_write(params) .await .map_err(map_error)?; @@ -278,21 +218,17 @@ impl ConfigApi { return Ok(ExperimentalFeatureEnablementSetResponse { enablement }); } - { - let mut runtime_feature_enablement = - self.runtime_feature_enablement - .write() - .map_err(|_| JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: "failed to update feature enablement".to_string(), - data: None, - })?; - runtime_feature_enablement.extend( + self.config_manager + .extend_runtime_feature_enablement( enablement .iter() .map(|(name, enabled)| (name.clone(), *enabled)), - ); - } + ) + .map_err(|_| JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "failed to update feature enablement".to_string(), + data: None, + })?; self.load_latest_config(/*fallback_cwd*/ None).await?; self.user_config_reloader.reload_user_config().await; @@ -309,7 +245,8 @@ impl ConfigApi { continue; }; let metadata = - installed_plugin_telemetry_metadata(self.codex_home.as_path(), &plugin_id).await; + installed_plugin_telemetry_metadata(self.config_manager.codex_home(), &plugin_id) + .await; if enabled { self.analytics_events_client.track_plugin_enabled(metadata); } else { @@ -319,49 +256,6 @@ impl ConfigApi { } } -pub(crate) fn protected_feature_keys( - config_layer_stack: &codex_core::config_loader::ConfigLayerStack, -) -> BTreeSet { - let mut protected_features = config_layer_stack - .effective_config() - .get("features") - .and_then(toml::Value::as_table) - .map(|features| features.keys().cloned().collect::>()) - .unwrap_or_default(); - - if let Some(feature_requirements) = config_layer_stack - .requirements_toml() - .feature_requirements - .as_ref() - { - protected_features.extend(feature_requirements.entries.keys().cloned()); - } - - protected_features -} - -pub(crate) fn apply_runtime_feature_enablement( - config: &mut Config, - runtime_feature_enablement: &BTreeMap, -) { - let protected_features = protected_feature_keys(&config.config_layer_stack); - for (name, enabled) in runtime_feature_enablement { - if protected_features.contains(name) { - continue; - } - let Some(feature) = feature_for_key(name) else { - continue; - }; - if let Err(err) = config.features.set_enabled(feature, *enabled) { - warn!( - feature = name, - error = %err, - "failed to apply runtime feature enablement" - ); - } - } -} - fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigRequirements { ConfigRequirements { allowed_approval_policies: requirements.allowed_approval_policies.map(|policies| { @@ -495,7 +389,7 @@ fn map_network_unix_socket_permission_to_api( } } -fn map_error(err: ConfigServiceError) -> JSONRPCErrorError { +fn map_error(err: ConfigManagerError) -> JSONRPCErrorError { if let Some(code) = err.write_error_code() { return config_write_error(code, err.to_string()); } @@ -520,7 +414,11 @@ fn config_write_error(code: ConfigWriteErrorCode, message: impl Into) -> #[cfg(test)] mod tests { use super::*; + use crate::config_manager::apply_runtime_feature_enablement; use codex_analytics::AnalyticsEventsClient; + use codex_arg0::Arg0DispatchPaths; + use codex_core::config_loader::CloudRequirementsLoader; + use codex_core::config_loader::LoaderOverrides; use codex_core::config_loader::NetworkDomainPermissionToml as CoreNetworkDomainPermissionToml; use codex_core::config_loader::NetworkDomainPermissionsToml as CoreNetworkDomainPermissionsToml; use codex_core::config_loader::NetworkRequirementsToml as CoreNetworkRequirementsToml; @@ -533,9 +431,12 @@ mod tests { use codex_protocol::protocol::AskForApproval as CoreAskForApproval; use pretty_assertions::assert_eq; use serde_json::json; + use std::collections::BTreeMap; + use std::sync::RwLock; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use tempfile::TempDir; + use toml::Value as TomlValue; #[derive(Default)] struct RecordingUserConfigReloader { @@ -830,11 +731,15 @@ mod tests { ); let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test")); let config_api = ConfigApi::new( - codex_home.path().to_path_buf(), - Arc::new(RwLock::new(Vec::new())), - Arc::new(RwLock::new(BTreeMap::new())), - LoaderOverrides::default(), - Arc::new(RwLock::new(CloudRequirementsLoader::default())), + ConfigManager::new( + codex_home.path().to_path_buf(), + Arc::new(RwLock::new(Vec::new())), + Arc::new(RwLock::new(BTreeMap::new())), + LoaderOverrides::default(), + Arc::new(RwLock::new(CloudRequirementsLoader::default())), + Arg0DispatchPaths::default(), + Arc::new(codex_config::NoopThreadConfigLoader), + ), reloader.clone(), AnalyticsEventsClient::new( auth_manager, diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs new file mode 100644 index 000000000000..67e0072f471e --- /dev/null +++ b/codex-rs/app-server/src/config_manager.rs @@ -0,0 +1,326 @@ +use codex_arg0::Arg0DispatchPaths; +use codex_cloud_requirements::cloud_requirements_loader; +use codex_config::ThreadConfigLoader; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::ConfigLayerStack; +use codex_core::config_loader::LoaderOverrides; +use codex_core::config_loader::load_config_layers_state; +use codex_exec_server::LOCAL_FS; +use codex_features::feature_for_key; +use codex_login::AuthManager; +use codex_login::default_client::set_default_client_residency_requirement; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_json_to_toml::json_to_toml; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; +use std::sync::RwLock; +use toml::Value as TomlValue; +use tracing::warn; + +/// Shared app-server entry point for loading effective Codex configuration. +#[derive(Clone)] +pub(crate) struct ConfigManager { + codex_home: PathBuf, + cli_overrides: Arc>>, + runtime_feature_enablement: Arc>>, + loader_overrides: LoaderOverrides, + cloud_requirements: Arc>, + arg0_paths: Arg0DispatchPaths, + thread_config_loader: Arc, + host_name: Option, +} + +impl ConfigManager { + pub(crate) fn new( + codex_home: PathBuf, + cli_overrides: Arc>>, + runtime_feature_enablement: Arc>>, + loader_overrides: LoaderOverrides, + cloud_requirements: Arc>, + arg0_paths: Arg0DispatchPaths, + thread_config_loader: Arc, + ) -> Self { + Self::new_with_host_name( + codex_home, + cli_overrides, + runtime_feature_enablement, + loader_overrides, + cloud_requirements, + arg0_paths, + thread_config_loader, + codex_config::host_name(), + ) + } + + #[allow(clippy::too_many_arguments)] + fn new_with_host_name( + codex_home: PathBuf, + cli_overrides: Arc>>, + runtime_feature_enablement: Arc>>, + loader_overrides: LoaderOverrides, + cloud_requirements: Arc>, + arg0_paths: Arg0DispatchPaths, + thread_config_loader: Arc, + host_name: Option, + ) -> Self { + Self { + codex_home, + cli_overrides, + runtime_feature_enablement, + loader_overrides, + cloud_requirements, + arg0_paths, + thread_config_loader, + host_name, + } + } + + pub(crate) fn codex_home(&self) -> &Path { + self.codex_home.as_path() + } + + pub(crate) fn current_cli_overrides(&self) -> Vec<(String, TomlValue)> { + self.cli_overrides + .read() + .map(|guard| guard.clone()) + .unwrap_or_default() + } + + pub(crate) fn current_cloud_requirements(&self) -> CloudRequirementsLoader { + self.cloud_requirements + .read() + .map(|guard| guard.clone()) + .unwrap_or_default() + } + + pub(crate) fn extend_runtime_feature_enablement(&self, enablement: I) -> Result<(), ()> + where + I: IntoIterator, + { + let mut runtime_feature_enablement = + self.runtime_feature_enablement.write().map_err(|_| ())?; + runtime_feature_enablement.extend(enablement); + Ok(()) + } + + pub(crate) fn replace_cloud_requirements_loader( + &self, + auth_manager: Arc, + chatgpt_base_url: String, + ) { + let loader = + cloud_requirements_loader(auth_manager, chatgpt_base_url, self.codex_home.clone()); + if let Ok(mut guard) = self.cloud_requirements.write() { + *guard = loader; + } else { + warn!("failed to update cloud requirements loader"); + } + } + + pub(crate) async fn sync_default_client_residency_requirement(&self) { + match self.load_latest_config(/*fallback_cwd*/ None).await { + Ok(config) => { + set_default_client_residency_requirement(config.enforce_residency.value()); + } + Err(err) => warn!( + error = %err, + "failed to sync default client residency requirement after auth refresh" + ), + } + } + + pub(crate) async fn load_latest_config( + &self, + fallback_cwd: Option, + ) -> std::io::Result { + self.load_with_cli_overrides( + &self.current_cli_overrides(), + /*request_overrides*/ None, + ConfigOverrides::default(), + fallback_cwd, + ) + .await + } + + pub(crate) async fn load_with_overrides( + &self, + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + ) -> std::io::Result { + self.load_with_cli_overrides( + &self.current_cli_overrides(), + request_overrides, + typesafe_overrides, + /*fallback_cwd*/ None, + ) + .await + } + + pub(crate) async fn load_for_cwd( + &self, + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + cwd: Option, + ) -> std::io::Result { + self.load_with_cli_overrides( + &self.current_cli_overrides(), + request_overrides, + typesafe_overrides, + cwd, + ) + .await + } + + pub(crate) async fn load_with_cli_overrides( + &self, + cli_overrides: &[(String, TomlValue)], + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + fallback_cwd: Option, + ) -> std::io::Result { + let merged_cli_overrides = cli_overrides + .iter() + .cloned() + .chain( + request_overrides + .unwrap_or_default() + .into_iter() + .map(|(key, value)| (key, json_to_toml(value))), + ) + .collect::>(); + + let mut config = codex_core::config::ConfigBuilder::default() + .codex_home(self.codex_home.clone()) + .cli_overrides(merged_cli_overrides) + .loader_overrides(self.loader_overrides.clone()) + .harness_overrides(typesafe_overrides) + .fallback_cwd(fallback_cwd) + .cloud_requirements(self.current_cloud_requirements()) + .thread_config_loader(Arc::clone(&self.thread_config_loader)) + .host_name(self.host_name.clone()) + .build() + .await?; + self.apply_runtime_feature_enablement(&mut config); + self.apply_arg0_paths(&mut config); + Ok(config) + } + + pub(crate) async fn load_config_layers_for_cwd( + &self, + cwd: AbsolutePathBuf, + ) -> std::io::Result { + self.load_config_layers(Some(cwd)).await + } + + pub(crate) async fn load_config_layers( + &self, + cwd: Option, + ) -> std::io::Result { + load_config_layers_state( + LOCAL_FS.as_ref(), + &self.codex_home, + cwd, + &self.current_cli_overrides(), + self.loader_overrides.clone(), + self.current_cloud_requirements(), + self.thread_config_loader.as_ref(), + self.host_name.as_deref(), + ) + .await + } + + fn apply_runtime_feature_enablement(&self, config: &mut Config) { + apply_runtime_feature_enablement(config, &self.current_runtime_feature_enablement()); + } + + fn current_runtime_feature_enablement(&self) -> BTreeMap { + self.runtime_feature_enablement + .read() + .map(|guard| guard.clone()) + .unwrap_or_default() + } + + fn apply_arg0_paths(&self, config: &mut Config) { + config.codex_self_exe = self.arg0_paths.codex_self_exe.clone(); + config.codex_linux_sandbox_exe = self.arg0_paths.codex_linux_sandbox_exe.clone(); + config.main_execve_wrapper_exe = self.arg0_paths.main_execve_wrapper_exe.clone(); + } + + #[cfg(test)] + pub(crate) fn new_for_tests( + codex_home: PathBuf, + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, + cloud_requirements: CloudRequirementsLoader, + host_name: Option, + ) -> Self { + Self::new_with_host_name( + codex_home, + Arc::new(RwLock::new(cli_overrides)), + Arc::new(RwLock::new(BTreeMap::new())), + loader_overrides, + Arc::new(RwLock::new(cloud_requirements)), + Arg0DispatchPaths::default(), + Arc::new(codex_config::NoopThreadConfigLoader), + host_name, + ) + } + + #[cfg(test)] + pub(crate) fn without_managed_config_for_tests(codex_home: PathBuf) -> Self { + Self::new_for_tests( + codex_home, + Vec::new(), + LoaderOverrides::without_managed_config_for_tests(), + CloudRequirementsLoader::default(), + /*host_name*/ None, + ) + } +} + +pub(crate) fn protected_feature_keys(config_layer_stack: &ConfigLayerStack) -> BTreeSet { + let mut protected_features = config_layer_stack + .effective_config() + .get("features") + .and_then(toml::Value::as_table) + .map(|features| features.keys().cloned().collect::>()) + .unwrap_or_default(); + + if let Some(feature_requirements) = config_layer_stack + .requirements_toml() + .feature_requirements + .as_ref() + { + protected_features.extend(feature_requirements.entries.keys().cloned()); + } + + protected_features +} + +pub(crate) fn apply_runtime_feature_enablement( + config: &mut Config, + runtime_feature_enablement: &BTreeMap, +) { + let protected_features = protected_feature_keys(&config.config_layer_stack); + for (name, enabled) in runtime_feature_enablement { + if protected_features.contains(name) { + continue; + } + let Some(feature) = feature_for_key(name) else { + continue; + }; + if let Err(err) = config.features.set_enabled(feature, *enabled) { + warn!( + feature = name, + error = %err, + "failed to apply runtime feature enablement" + ); + } + } +} diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/app-server/src/config_manager_service.rs similarity index 74% rename from codex-rs/core/src/config/service.rs rename to codex-rs/app-server/src/config_manager_service.rs index e9546ef4db16..0104429a4b5f 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -1,20 +1,4 @@ -use super::deserialize_config_toml_with_base; -use crate::config::edit::ConfigEdit; -use crate::config::edit::ConfigEditsBuilder; -use crate::config::managed_features::validate_explicit_feature_settings_in_config_toml; -use crate::config::managed_features::validate_feature_requirements_in_config_toml; -use crate::config_loader::CloudRequirementsLoader; -use crate::config_loader::ConfigLayerEntry; -use crate::config_loader::ConfigLayerStack; -use crate::config_loader::ConfigLayerStackOrdering; -use crate::config_loader::ConfigRequirementsToml; -use crate::config_loader::LoaderOverrides; -use crate::config_loader::load_config_layers_state; -use crate::config_loader::merge_toml_values; -use crate::path_utils; -use crate::path_utils::SymlinkWritePaths; -use crate::path_utils::resolve_symlink_write_paths; -use crate::path_utils::write_atomically; +use crate::config_manager::ConfigManager; use codex_app_server_protocol::Config as ApiConfig; use codex_app_server_protocol::ConfigBatchWriteParams; use codex_app_server_protocol::ConfigLayerMetadata; @@ -29,7 +13,19 @@ use codex_app_server_protocol::OverriddenMetadata; use codex_app_server_protocol::WriteStatus; use codex_config::CONFIG_TOML_FILE; use codex_config::config_toml::ConfigToml; -use codex_exec_server::LOCAL_FS; +use codex_core::config::deserialize_config_toml_with_base; +use codex_core::config::edit::ConfigEdit; +use codex_core::config::edit::ConfigEditsBuilder; +use codex_core::config::validate_feature_requirements_for_config_toml; +use codex_core::config_loader::ConfigLayerEntry; +use codex_core::config_loader::ConfigLayerStack; +use codex_core::config_loader::ConfigLayerStackOrdering; +use codex_core::config_loader::ConfigRequirementsToml; +use codex_core::config_loader::merge_toml_values; +use codex_core::path_utils; +use codex_core::path_utils::SymlinkWritePaths; +use codex_core::path_utils::resolve_symlink_write_paths; +use codex_core::path_utils::write_atomically; use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value as JsonValue; use std::borrow::Cow; @@ -41,7 +37,7 @@ use toml::Value as TomlValue; use toml_edit::Item as TomlItem; #[derive(Debug, Error)] -pub enum ConfigServiceError { +pub(crate) enum ConfigManagerError { #[error("{message}")] Write { code: ConfigWriteErrorCode, @@ -77,7 +73,7 @@ pub enum ConfigServiceError { }, } -impl ConfigServiceError { +impl ConfigManagerError { fn write(code: ConfigWriteErrorCode, message: impl Into) -> Self { Self::Write { code, @@ -101,7 +97,7 @@ impl ConfigServiceError { Self::Anyhow { context, source } } - pub fn write_error_code(&self) -> Option { + pub(crate) fn write_error_code(&self) -> Option { match self { Self::Write { code, .. } => Some(code.clone()), _ => None, @@ -109,78 +105,22 @@ impl ConfigServiceError { } } -#[derive(Clone)] -pub struct ConfigService { - codex_home: PathBuf, - cli_overrides: Vec<(String, TomlValue)>, - loader_overrides: LoaderOverrides, - cloud_requirements: CloudRequirementsLoader, - host_name: Option, -} - -impl ConfigService { - pub fn new( - codex_home: PathBuf, - cli_overrides: Vec<(String, TomlValue)>, - loader_overrides: LoaderOverrides, - cloud_requirements: CloudRequirementsLoader, - host_name: Option, - ) -> Self { - Self { - codex_home, - cli_overrides, - loader_overrides, - cloud_requirements, - host_name, - } - } - - pub fn new_with_defaults(codex_home: PathBuf) -> Self { - Self { - codex_home, - cli_overrides: Vec::new(), - loader_overrides: LoaderOverrides::default(), - cloud_requirements: CloudRequirementsLoader::default(), - host_name: codex_config::host_name(), - } - } - - #[cfg(test)] - pub(crate) fn without_managed_config_for_tests(codex_home: PathBuf) -> Self { - Self::new( - codex_home, - Vec::new(), - LoaderOverrides::without_managed_config_for_tests(), - CloudRequirementsLoader::default(), - /*host_name*/ None, - ) - } - - pub async fn read( +impl ConfigManager { + pub(crate) async fn read( &self, params: ConfigReadParams, - ) -> Result { + ) -> Result { let layers = match params.cwd.as_deref() { Some(cwd) => { let cwd = AbsolutePathBuf::try_from(PathBuf::from(cwd)).map_err(|err| { - ConfigServiceError::io("failed to resolve config cwd to an absolute path", err) + ConfigManagerError::io("failed to resolve config cwd to an absolute path", err) })?; - crate::config::ConfigBuilder::default() - .codex_home(self.codex_home.clone()) - .cli_overrides(self.cli_overrides.clone()) - .loader_overrides(self.loader_overrides.clone()) - .fallback_cwd(Some(cwd.to_path_buf())) - .cloud_requirements(self.cloud_requirements.clone()) - .host_name(self.host_name.clone()) - .build() - .await - .map_err(|err| { - ConfigServiceError::io("failed to read configuration layers", err) - })? - .config_layer_stack + self.load_config_layers(Some(cwd)).await.map_err(|err| { + ConfigManagerError::io("failed to read configuration layers", err) + })? } None => self.load_thread_agnostic_config().await.map_err(|err| { - ConfigServiceError::io("failed to read configuration layers", err) + ConfigManagerError::io("failed to read configuration layers", err) })?, }; @@ -188,12 +128,12 @@ impl ConfigService { let effective_config_toml: ConfigToml = effective .try_into() - .map_err(|err| ConfigServiceError::toml("invalid configuration", err))?; + .map_err(|err| ConfigManagerError::toml("invalid configuration", err))?; let json_value = serde_json::to_value(&effective_config_toml) - .map_err(|err| ConfigServiceError::json("failed to serialize configuration", err))?; + .map_err(|err| ConfigManagerError::json("failed to serialize configuration", err))?; let config: ApiConfig = serde_json::from_value(json_value) - .map_err(|err| ConfigServiceError::json("failed to deserialize configuration", err))?; + .map_err(|err| ConfigManagerError::json("failed to deserialize configuration", err))?; Ok(ConfigReadResponse { config, @@ -211,13 +151,13 @@ impl ConfigService { }) } - pub async fn read_requirements( + pub(crate) async fn read_requirements( &self, - ) -> Result, ConfigServiceError> { + ) -> Result, ConfigManagerError> { let layers = self .load_thread_agnostic_config() .await - .map_err(|err| ConfigServiceError::io("failed to read configuration layers", err))?; + .map_err(|err| ConfigManagerError::io("failed to read configuration layers", err))?; let requirements = layers.requirements_toml().clone(); if requirements.is_empty() { @@ -227,19 +167,19 @@ impl ConfigService { } } - pub async fn write_value( + pub(crate) async fn write_value( &self, params: ConfigValueWriteParams, - ) -> Result { + ) -> Result { let edits = vec![(params.key_path, params.value, params.merge_strategy)]; self.apply_edits(params.file_path, params.expected_version, edits) .await } - pub async fn batch_write( + pub(crate) async fn batch_write( &self, params: ConfigBatchWriteParams, - ) -> Result { + ) -> Result { let edits = params .edits .into_iter() @@ -250,37 +190,22 @@ impl ConfigService { .await } - pub async fn load_user_saved_config( - &self, - ) -> Result { - let layers = self - .load_thread_agnostic_config() - .await - .map_err(|err| ConfigServiceError::io("failed to load configuration", err))?; - - let toml_value = layers.effective_config(); - let cfg: ConfigToml = toml_value - .try_into() - .map_err(|err| ConfigServiceError::toml("failed to parse config.toml", err))?; - Ok(cfg.into()) - } - async fn apply_edits( &self, file_path: Option, expected_version: Option, edits: Vec<(String, JsonValue, MergeStrategy)>, - ) -> Result { + ) -> Result { let allowed_path = - AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, &self.codex_home); + AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, self.codex_home()); let provided_path = match file_path { Some(path) => AbsolutePathBuf::from_absolute_path(PathBuf::from(path)) - .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?, + .map_err(|err| ConfigManagerError::io("failed to resolve user config path", err))?, None => allowed_path.clone(), }; if !paths_match(&allowed_path, &provided_path) { - return Err(ConfigServiceError::write( + return Err(ConfigManagerError::write( ConfigWriteErrorCode::ConfigLayerReadonly, "Only writes to the user config are allowed", )); @@ -289,7 +214,7 @@ impl ConfigService { let layers = self .load_thread_agnostic_config() .await - .map_err(|err| ConfigServiceError::io("failed to load configuration", err))?; + .map_err(|err| ConfigManagerError::io("failed to load configuration", err))?; let user_layer = match layers.get_user_layer() { Some(layer) => Cow::Borrowed(layer), None => Cow::Owned(create_empty_user_layer(&allowed_path).await?), @@ -298,7 +223,7 @@ impl ConfigService { if let Some(expected) = expected_version.as_deref() && expected != user_layer.version { - return Err(ConfigServiceError::write( + return Err(ConfigManagerError::write( ConfigWriteErrorCode::ConfigVersionConflict, "Configuration was modified since last read. Fetch latest version and retry.", )); @@ -310,20 +235,20 @@ impl ConfigService { for (key_path, value, strategy) in edits.into_iter() { let segments = parse_key_path(&key_path).map_err(|message| { - ConfigServiceError::write(ConfigWriteErrorCode::ConfigValidationError, message) + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) })?; let original_value = value_at_path(&user_config, &segments).cloned(); let parsed_value = parse_value(value).map_err(|message| { - ConfigServiceError::write(ConfigWriteErrorCode::ConfigValidationError, message) + ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) })?; apply_merge(&mut user_config, &segments, parsed_value.as_ref(), strategy).map_err( |err| match err { - MergeError::PathNotFound => ConfigServiceError::write( + MergeError::PathNotFound => ConfigManagerError::write( ConfigWriteErrorCode::ConfigPathNotFound, "Path not found", ), - MergeError::Validation(message) => ConfigServiceError::write( + MergeError::Validation(message) => ConfigManagerError::write( ConfigWriteErrorCode::ConfigValidationError, message, ), @@ -336,7 +261,7 @@ impl ConfigService { Some(value) => ConfigEdit::SetPath { segments: segments.clone(), value: toml_value_to_item(&value).map_err(|err| { - ConfigServiceError::anyhow("failed to build config edits", err) + ConfigManagerError::anyhow("failed to build config edits", err) })?, }, None => ConfigEdit::ClearPath { @@ -350,56 +275,45 @@ impl ConfigService { } validate_config(&user_config).map_err(|err| { - ConfigServiceError::write( + ConfigManagerError::write( ConfigWriteErrorCode::ConfigValidationError, format!("Invalid configuration: {err}"), ) })?; let user_config_toml = - deserialize_config_toml_with_base(user_config.clone(), &self.codex_home).map_err( + deserialize_config_toml_with_base(user_config.clone(), self.codex_home()).map_err( |err| { - ConfigServiceError::write( + ConfigManagerError::write( ConfigWriteErrorCode::ConfigValidationError, format!("Invalid configuration: {err}"), ) }, )?; - validate_explicit_feature_settings_in_config_toml( + validate_feature_requirements_for_config_toml( &user_config_toml, layers.requirements().feature_requirements.as_ref(), ) .map_err(|err| { - ConfigServiceError::write( + ConfigManagerError::write( ConfigWriteErrorCode::ConfigValidationError, format!("Invalid configuration: {err}"), ) })?; - validate_feature_requirements_in_config_toml( - &user_config_toml, - layers.requirements().feature_requirements.as_ref(), - ) - .map_err(|err| { - ConfigServiceError::write( - ConfigWriteErrorCode::ConfigValidationError, - format!("Invalid configuration: {err}"), - ) - })?; - let updated_layers = layers.with_user_config(&provided_path, user_config.clone()); let effective = updated_layers.effective_config(); validate_config(&effective).map_err(|err| { - ConfigServiceError::write( + ConfigManagerError::write( ConfigWriteErrorCode::ConfigValidationError, format!("Invalid configuration: {err}"), ) })?; if !config_edits.is_empty() { - ConfigEditsBuilder::new(&self.codex_home) + ConfigEditsBuilder::new(self.codex_home()) .with_edits(config_edits) .apply() .await - .map_err(|err| ConfigServiceError::anyhow("failed to persist config.toml", err))?; + .map_err(|err| ConfigManagerError::anyhow("failed to persist config.toml", err))?; } let overridden = first_overridden_edit(&updated_layers, &effective, &parsed_segments); @@ -413,7 +327,7 @@ impl ConfigService { version: updated_layers .get_user_layer() .ok_or_else(|| { - ConfigServiceError::write( + ConfigManagerError::write( ConfigWriteErrorCode::UserLayerNotFound, "user layer not found in updated layers", ) @@ -429,40 +343,29 @@ impl ConfigService { /// include any in-repo .codex/ folders because there is no cwd/project root /// associated with this query. async fn load_thread_agnostic_config(&self) -> std::io::Result { - let cwd: Option = None; - load_config_layers_state( - LOCAL_FS.as_ref(), - &self.codex_home, - cwd, - &self.cli_overrides, - self.loader_overrides.clone(), - self.cloud_requirements.clone(), - &codex_config::NoopThreadConfigLoader, - self.host_name.as_deref(), - ) - .await + self.load_config_layers(/*cwd*/ None).await } } async fn create_empty_user_layer( config_toml: &AbsolutePathBuf, -) -> Result { +) -> Result { let SymlinkWritePaths { read_path, write_path, } = resolve_symlink_write_paths(config_toml.as_path()) - .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?; + .map_err(|err| ConfigManagerError::io("failed to resolve user config path", err))?; let toml_value = match read_path { Some(path) => match tokio::fs::read_to_string(&path).await { Ok(contents) => toml::from_str(&contents).map_err(|e| { - ConfigServiceError::toml("failed to parse existing user config.toml", e) + ConfigManagerError::toml("failed to parse existing user config.toml", e) })?, Err(err) if err.kind() == std::io::ErrorKind::NotFound => { write_empty_user_config(write_path.clone()).await?; TomlValue::Table(toml::map::Map::new()) } Err(err) => { - return Err(ConfigServiceError::io( + return Err(ConfigManagerError::io( "failed to read user config.toml", err, )); @@ -481,11 +384,11 @@ async fn create_empty_user_layer( )) } -async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigServiceError> { +async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigManagerError> { task::spawn_blocking(move || write_atomically(&write_path, "")) .await - .map_err(|err| ConfigServiceError::anyhow("config persistence task panicked", err.into()))? - .map_err(|err| ConfigServiceError::io("failed to create empty user config.toml", err)) + .map_err(|err| ConfigManagerError::anyhow("config persistence task panicked", err.into()))? + .map_err(|err| ConfigManagerError::io("failed to create empty user config.toml", err)) } fn parse_value(value: JsonValue) -> Result, String> { @@ -746,5 +649,5 @@ fn find_effective_layer( } #[cfg(test)] -#[path = "service_tests.rs"] +#[path = "config_manager_service_tests.rs"] mod tests; diff --git a/codex-rs/core/src/config/service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs similarity index 95% rename from codex-rs/core/src/config/service_tests.rs rename to codex-rs/app-server/src/config_manager_service_tests.rs index ceb801c456eb..a871d8e43f0b 100644 --- a/codex-rs/core/src/config/service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -4,6 +4,9 @@ use codex_app_server_protocol::AppConfig; use codex_app_server_protocol::AppToolApproval; use codex_app_server_protocol::AppsConfig; use codex_app_server_protocol::AskForApproval; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::FeatureRequirementsToml; +use codex_core::config_loader::LoaderOverrides; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::BTreeMap; @@ -74,7 +77,7 @@ unified_exec = true "#; std::fs::write(tmp.path().join(CONFIG_TOML_FILE), original)?; - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -108,7 +111,7 @@ async fn write_value_supports_nested_app_paths() -> Result<()> { let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "")?; - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -172,7 +175,7 @@ async fn write_value_supports_custom_mcp_server_default_tool_approval_mode() -> "[mcp_servers.docs]\ncommand = \"docs-server\"\n", )?; - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -218,7 +221,7 @@ async fn read_includes_origins_and_layers() { std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()), @@ -297,7 +300,7 @@ writable_roots = ["~/code"] ), ); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], loader_overrides, @@ -338,7 +341,7 @@ async fn write_value_reports_override() { std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()), @@ -388,7 +391,7 @@ async fn version_conflict_rejected() { let user_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&user_path, "model = \"user\"").unwrap(); - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); let error = service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -411,7 +414,7 @@ async fn write_value_defaults_to_user_config_path() { let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: None, @@ -438,7 +441,7 @@ async fn invalid_user_value_rejected_even_if_overridden_by_managed() { let managed_path = tmp.path().join("managed_config.toml"); std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()), @@ -471,7 +474,7 @@ async fn reserved_builtin_provider_override_rejected() { let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "model = \"user\"\n").unwrap(); - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); let error = service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -499,13 +502,13 @@ async fn write_value_rejects_feature_requirement_conflict() { let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::without_managed_config_for_tests(), CloudRequirementsLoader::new(async { Ok(Some(ConfigRequirementsToml { - feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + feature_requirements: Some(FeatureRequirementsToml { entries: BTreeMap::from([("personality".to_string(), true)]), }), ..Default::default() @@ -546,13 +549,13 @@ async fn write_value_rejects_profile_feature_requirement_conflict() { let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::without_managed_config_for_tests(), CloudRequirementsLoader::new(async { Ok(Some(ConfigRequirementsToml { - feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + feature_requirements: Some(FeatureRequirementsToml { entries: BTreeMap::from([("personality".to_string(), true)]), }), ..Default::default() @@ -604,7 +607,7 @@ async fn read_reports_managed_overrides_user_and_session_flags() { TomlValue::String("session".to_string()), )]; - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), cli_overrides, LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()), @@ -658,7 +661,7 @@ async fn write_value_reports_managed_override() { std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::new( + let service = ConfigManager::new_for_tests( tmp.path().to_path_buf(), vec![], LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()), @@ -714,7 +717,7 @@ alpha = "a" std::fs::write(&path, base)?; - let service = ConfigService::without_managed_config_for_tests(tmp.path().to_path_buf()); + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(path.display().to_string()), diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index d9c108033f28..f037ca5e65d3 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -71,6 +71,8 @@ mod bespoke_event_handling; mod codex_message_processor; mod command_exec; mod config_api; +mod config_manager; +mod config_manager_service; mod dynamic_tools; mod error_code; mod external_agent_config_api; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 2032eb61cd90..53d9f2df4ce3 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -10,6 +10,7 @@ use std::sync::atomic::Ordering; use crate::codex_message_processor::CodexMessageProcessor; use crate::codex_message_processor::CodexMessageProcessorArgs; use crate::config_api::ConfigApi; +use crate::config_manager::ConfigManager; use crate::error_code::INVALID_REQUEST_ERROR_CODE; use crate::external_agent_config_api::ExternalAgentConfigApi; use crate::fs_api::FsApi; @@ -294,6 +295,15 @@ impl MessageProcessor { let cli_overrides = Arc::new(RwLock::new(cli_overrides)); let runtime_feature_enablement = Arc::new(RwLock::new(BTreeMap::new())); let cloud_requirements = Arc::new(RwLock::new(cloud_requirements)); + let config_manager = ConfigManager::new( + config.codex_home.to_path_buf(), + cli_overrides, + runtime_feature_enablement, + loader_overrides, + cloud_requirements, + arg0_paths.clone(), + thread_config_loader, + ); let codex_message_processor = CodexMessageProcessor::new(CodexMessageProcessorArgs { auth_manager: auth_manager.clone(), thread_manager: Arc::clone(&thread_manager), @@ -301,10 +311,7 @@ impl MessageProcessor { analytics_events_client: analytics_events_client.clone(), arg0_paths, config: Arc::clone(&config), - cli_overrides: cli_overrides.clone(), - runtime_feature_enablement: runtime_feature_enablement.clone(), - cloud_requirements: cloud_requirements.clone(), - thread_config_loader, + config_manager: config_manager.clone(), feedback, log_db, }); @@ -314,11 +321,7 @@ impl MessageProcessor { .plugins_manager() .maybe_start_plugin_startup_tasks_for_config(&config, auth_manager.clone()); let config_api = ConfigApi::new( - config.codex_home.to_path_buf(), - cli_overrides, - runtime_feature_enablement, - loader_overrides, - cloud_requirements, + config_manager, thread_manager.clone(), analytics_events_client.clone(), ); diff --git a/codex-rs/app-server/tests/suite/v2/experimental_feature_list.rs b/codex-rs/app-server/tests/suite/v2/experimental_feature_list.rs index 07bdd0bde175..3f7e6989f8f5 100644 --- a/codex-rs/app-server/tests/suite/v2/experimental_feature_list.rs +++ b/codex-rs/app-server/tests/suite/v2/experimental_feature_list.rs @@ -15,6 +15,7 @@ use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_core::config::ConfigBuilder; +use codex_core::config_loader::LoaderOverrides; use codex_features::FEATURES; use codex_features::Stage; use pretty_assertions::assert_eq; @@ -32,6 +33,9 @@ async fn experimental_feature_list_returns_feature_metadata_with_stage() -> Resu let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .fallback_cwd(Some(codex_home.path().to_path_buf())) + .loader_overrides(LoaderOverrides::with_managed_config_path_for_tests( + codex_home.path().join("managed_config.toml"), + )) .build() .await?; let mut mcp = McpProcess::new(codex_home.path()).await?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index a6240fdcb3e9..7ae710f83cbc 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -7,6 +7,7 @@ use crate::config_loader::ConfigLayerStackOrdering; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; use crate::config_loader::ConstrainedWithSource; +use crate::config_loader::FeatureRequirementsToml; use crate::config_loader::LoaderOverrides; use crate::config_loader::McpServerIdentity; use crate::config_loader::McpServerRequirement; @@ -108,7 +109,6 @@ mod network_proxy_spec; mod permissions; #[cfg(test)] mod schema; -pub(crate) mod service; pub use codex_config::Constrained; pub use codex_config::ConstraintError; pub use codex_config::ConstraintResult; @@ -118,8 +118,6 @@ pub use managed_features::ManagedFeatures; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub(crate) use permissions::resolve_permission_profile; -pub use service::ConfigService; -pub use service::ConfigServiceError; pub use codex_git_utils::GhostSnapshotConfig; @@ -947,7 +945,7 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides( Ok(cfg) } -pub(crate) fn deserialize_config_toml_with_base( +pub fn deserialize_config_toml_with_base( root_value: TomlValue, config_base_dir: &Path, ) -> std::io::Result { @@ -959,6 +957,15 @@ pub(crate) fn deserialize_config_toml_with_base( .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) } +/// Validate user-visible feature settings against managed feature requirements. +pub fn validate_feature_requirements_for_config_toml( + cfg: &ConfigToml, + feature_requirements: Option<&Sourced>, +) -> std::io::Result<()> { + managed_features::validate_explicit_feature_settings_in_config_toml(cfg, feature_requirements)?; + managed_features::validate_feature_requirements_in_config_toml(cfg, feature_requirements) +} + fn load_catalog_json(path: &AbsolutePathBuf) -> std::io::Result { let file_contents = std::fs::read_to_string(path)?; let catalog = serde_json::from_str::(&file_contents).map_err(|err| { diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 3698b4b19183..efa91edc7ce5 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -6,14 +6,10 @@ use super::startup_sync::start_startup_remote_plugin_sync_once; use super::sync_openai_plugins_repo; use crate::SkillMetadata; use crate::config::Config; -use crate::config::ConfigService; -use crate::config::ConfigServiceError; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config_loader::ConfigLayerStack; use codex_analytics::AnalyticsEventsClient; -use codex_app_server_protocol::ConfigValueWriteParams; -use codex_app_server_protocol::MergeStrategy; use codex_config::types::PluginConfig; use codex_core_plugins::loader::configured_curated_plugin_ids_from_codex_home; use codex_core_plugins::loader::installed_plugin_telemetry_metadata; @@ -61,7 +57,6 @@ use codex_plugin::PluginIdError; use codex_plugin::prompt_safe_plugin_description; use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; -use serde_json::json; use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; @@ -609,18 +604,17 @@ impl PluginsManager { .await .map_err(PluginInstallError::join)??; - ConfigService::new_with_defaults(self.codex_home.clone()) - .write_value(ConfigValueWriteParams { - key_path: format!("plugins.{}", result.plugin_id.as_key()), - value: json!({ - "enabled": true, - }), - merge_strategy: MergeStrategy::Replace, - file_path: None, - expected_version: None, - }) + ConfigEditsBuilder::new(&self.codex_home) + .with_edits([ConfigEdit::SetPath { + segments: vec![ + "plugins".to_string(), + result.plugin_id.as_key(), + "enabled".to_string(), + ], + value: value(true), + }]) + .apply() .await - .map(|_| ()) .map_err(PluginInstallError::from)?; let analytics_events_client = match self.analytics_events_client.read() { @@ -1538,7 +1532,7 @@ pub enum PluginInstallError { Store(#[from] PluginStoreError), #[error("{0}")] - Config(#[from] ConfigServiceError), + Config(#[from] anyhow::Error), #[error("failed to join plugin install task: {0}")] Join(#[from] tokio::task::JoinError),