From 559cfce5a537cfc04d85e719cb8fef456bd9492f Mon Sep 17 00:00:00 2001 From: xli-oai Date: Tue, 28 Apr 2026 21:16:52 -0700 Subject: [PATCH] Route remote plugin config toggles outside ConfigApi --- .../src/codex_message_processor/plugins.rs | 10 +- codex-rs/app-server/src/config_api.rs | 125 ------------- .../app-server/src/config_write_router.rs | 165 ++++++++++++++++++ codex-rs/app-server/src/lib.rs | 1 + codex-rs/app-server/src/message_processor.rs | 15 +- codex-rs/core-plugins/src/remote.rs | 31 +--- 6 files changed, 183 insertions(+), 164 deletions(-) create mode 100644 codex-rs/app-server/src/config_write_router.rs diff --git a/codex-rs/app-server/src/codex_message_processor/plugins.rs b/codex-rs/app-server/src/codex_message_processor/plugins.rs index 61237e7e1f..4772f98746 100644 --- a/codex-rs/app-server/src/codex_message_processor/plugins.rs +++ b/codex-rs/app-server/src/codex_message_processor/plugins.rs @@ -1,13 +1,13 @@ use super::*; -use crate::config_api::RemotePluginConfigWriter; +use crate::config_write_router::RemotePluginEnablementWriter; use crate::error_code::internal_error; use crate::error_code::invalid_request; use async_trait::async_trait; use codex_app_server_protocol::PluginInstallPolicy; #[async_trait] -impl RemotePluginConfigWriter for CodexMessageProcessor { - async fn write_remote_plugin_enabled_config( +impl RemotePluginEnablementWriter for CodexMessageProcessor { + async fn set_remote_plugin_enabled( &self, plugin_id: String, enabled: bool, @@ -626,13 +626,13 @@ impl CodexMessageProcessor { ) -> Result { let PluginUninstallParams { plugin_id } = params; if codex_core::plugins::PluginId::parse(&plugin_id).is_err() - && !codex_core_plugins::remote::is_remote_plugin_config_id(&plugin_id) + && !codex_core_plugins::remote::is_supported_remote_plugin_id(&plugin_id) { return Err(invalid_request( "invalid plugin id: expected a local plugin id in the form `plugin@marketplace` or a remote plugin id starting with `plugins~`, `app_`, `asdk_app_`, or `connector_`", )); } - if codex_core_plugins::remote::is_remote_plugin_config_id(&plugin_id) { + if codex_core_plugins::remote::is_supported_remote_plugin_id(&plugin_id) { return self.remote_plugin_uninstall_response(plugin_id).await; } let plugins_manager = self.thread_manager.plugins_manager(); diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index ec8b78a6e5..ac7c1510c8 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -6,7 +6,6 @@ use crate::error_code::invalid_request; use async_trait::async_trait; use codex_analytics::AnalyticsEventsClient; use codex_app_server_protocol::ConfigBatchWriteParams; -use codex_app_server_protocol::ConfigEdit; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigRequirements; @@ -35,14 +34,12 @@ use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::plugins::PluginId; use codex_core_plugins::loader::installed_plugin_telemetry_metadata; -use codex_core_plugins::remote::remote_plugin_enabled_config_edit; use codex_core_plugins::toggles::collect_plugin_enabled_candidates; use codex_features::canonical_feature_for_key; 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::path::PathBuf; use std::sync::Arc; use tracing::warn; @@ -62,17 +59,6 @@ pub(crate) trait UserConfigReloader: Send + Sync { async fn reload_user_config(&self); } -/// Handles config-write compatibility cases whose source of truth is outside -/// `config.toml`. -#[async_trait] -pub(crate) trait RemotePluginConfigWriter: Send + Sync { - async fn write_remote_plugin_enabled_config( - &self, - plugin_id: String, - enabled: bool, - ) -> Result<(), JSONRPCErrorError>; -} - #[async_trait] impl UserConfigReloader for ThreadManager { async fn reload_user_config(&self) { @@ -92,7 +78,6 @@ impl UserConfigReloader for ThreadManager { pub(crate) struct ConfigApi { config_manager: ConfigManager, user_config_reloader: Arc, - remote_plugin_config_writer: Option>, analytics_events_client: AnalyticsEventsClient, } @@ -105,19 +90,10 @@ impl ConfigApi { Self { config_manager, user_config_reloader, - remote_plugin_config_writer: None, analytics_events_client, } } - pub(crate) fn with_remote_plugin_config_writer( - mut self, - remote_plugin_config_writer: Arc, - ) -> Self { - self.remote_plugin_config_writer = Some(remote_plugin_config_writer); - self - } - pub(crate) async fn load_latest_config( &self, fallback_cwd: Option, @@ -177,34 +153,6 @@ impl ConfigApi { pub(crate) async fn write_value( &self, params: ConfigValueWriteParams, - ) -> Result { - if let Some(remote_plugin_edit) = - remote_plugin_enabled_config_edit(¶ms.key_path, ¶ms.value) - && let Some(remote_plugin_config_writer) = &self.remote_plugin_config_writer - { - let response = self - .batch_write_to_config(ConfigBatchWriteParams { - edits: Vec::new(), - file_path: params.file_path, - expected_version: params.expected_version, - reload_user_config: false, - }) - .await?; - remote_plugin_config_writer - .write_remote_plugin_enabled_config( - remote_plugin_edit.plugin_id, - remote_plugin_edit.enabled, - ) - .await?; - return Ok(response); - } - - self.write_value_to_config(params).await - } - - async fn write_value_to_config( - &self, - params: ConfigValueWriteParams, ) -> Result { let pending_changes = collect_plugin_enabled_candidates([(¶ms.key_path, ¶ms.value)].into_iter()); @@ -220,79 +168,6 @@ impl ConfigApi { pub(crate) async fn batch_write( &self, params: ConfigBatchWriteParams, - ) -> Result { - let Some(remote_plugin_config_writer) = &self.remote_plugin_config_writer else { - return self.batch_write_to_config(params).await; - }; - - let ConfigBatchWriteParams { - edits, - file_path, - expected_version, - reload_user_config, - } = params; - let mut local_edits = Vec::::new(); - let mut remote_plugin_toggles = BTreeMap::::new(); - - for edit in edits { - if let Some(remote_plugin_edit) = - remote_plugin_enabled_config_edit(&edit.key_path, &edit.value) - { - remote_plugin_toggles - .insert(remote_plugin_edit.plugin_id, remote_plugin_edit.enabled); - } else { - local_edits.push(edit); - } - } - - if !remote_plugin_toggles.is_empty() && !local_edits.is_empty() { - return Err(invalid_request( - "remote plugin enablement edits cannot be batched with local config edits", - )); - } - - if remote_plugin_toggles.is_empty() { - return self - .batch_write_to_config(ConfigBatchWriteParams { - edits: local_edits, - file_path, - expected_version, - reload_user_config, - }) - .await; - } - - let response = self - .batch_write_to_config(ConfigBatchWriteParams { - edits: Vec::new(), - file_path: file_path.clone(), - expected_version, - reload_user_config: false, - }) - .await?; - - for (plugin_id, enabled) in remote_plugin_toggles { - remote_plugin_config_writer - .write_remote_plugin_enabled_config(plugin_id, enabled) - .await?; - } - - if reload_user_config { - self.batch_write_to_config(ConfigBatchWriteParams { - edits: Vec::new(), - file_path, - expected_version: None, - reload_user_config: true, - }) - .await?; - } - - Ok(response) - } - - async fn batch_write_to_config( - &self, - params: ConfigBatchWriteParams, ) -> Result { let reload_user_config = params.reload_user_config; let pending_changes = collect_plugin_enabled_candidates( diff --git a/codex-rs/app-server/src/config_write_router.rs b/codex-rs/app-server/src/config_write_router.rs new file mode 100644 index 0000000000..7ee86f23ec --- /dev/null +++ b/codex-rs/app-server/src/config_write_router.rs @@ -0,0 +1,165 @@ +use crate::config_api::ConfigApi; +use crate::error_code::invalid_request; +use async_trait::async_trait; +use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigEdit; +use codex_app_server_protocol::ConfigValueWriteParams; +use codex_app_server_protocol::ConfigWriteResponse; +use codex_app_server_protocol::JSONRPCErrorError; +use serde_json::Value as JsonValue; +use std::collections::BTreeMap; +use std::sync::Arc; + +/// Applies remote plugin enablement changes whose current UI entry point is a +/// config-shaped RPC. +#[async_trait] +pub(crate) trait RemotePluginEnablementWriter: Send + Sync { + async fn set_remote_plugin_enabled( + &self, + plugin_id: String, + enabled: bool, + ) -> Result<(), JSONRPCErrorError>; +} + +#[derive(Clone)] +pub(crate) struct ConfigWriteRouter { + config_api: ConfigApi, + remote_plugin_enablement_writer: Arc, +} + +impl ConfigWriteRouter { + pub(crate) fn new( + config_api: ConfigApi, + remote_plugin_enablement_writer: Arc, + ) -> Self { + Self { + config_api, + remote_plugin_enablement_writer, + } + } + + pub(crate) async fn write_value( + &self, + params: ConfigValueWriteParams, + ) -> Result { + if let Some(remote_plugin_edit) = + remote_plugin_enabled_config_edit(¶ms.key_path, ¶ms.value) + { + let response = self + .config_api + .batch_write(ConfigBatchWriteParams { + edits: Vec::new(), + file_path: params.file_path, + expected_version: params.expected_version, + reload_user_config: false, + }) + .await?; + self.remote_plugin_enablement_writer + .set_remote_plugin_enabled(remote_plugin_edit.plugin_id, remote_plugin_edit.enabled) + .await?; + return Ok(response); + } + + self.config_api.write_value(params).await + } + + pub(crate) async fn batch_write( + &self, + params: ConfigBatchWriteParams, + ) -> Result { + let ConfigBatchWriteParams { + edits, + file_path, + expected_version, + reload_user_config, + } = params; + let mut local_edits = Vec::::new(); + let mut remote_plugin_toggles = BTreeMap::::new(); + + for edit in edits { + if let Some(remote_plugin_edit) = + remote_plugin_enabled_config_edit(&edit.key_path, &edit.value) + { + remote_plugin_toggles + .insert(remote_plugin_edit.plugin_id, remote_plugin_edit.enabled); + } else { + local_edits.push(edit); + } + } + + if !remote_plugin_toggles.is_empty() && !local_edits.is_empty() { + return Err(invalid_request( + "remote plugin enablement edits cannot be batched with local config edits", + )); + } + + if remote_plugin_toggles.is_empty() { + return self + .config_api + .batch_write(ConfigBatchWriteParams { + edits: local_edits, + file_path, + expected_version, + reload_user_config, + }) + .await; + } + + let response = self + .config_api + .batch_write(ConfigBatchWriteParams { + edits: Vec::new(), + file_path: file_path.clone(), + expected_version, + reload_user_config: false, + }) + .await?; + + for (plugin_id, enabled) in remote_plugin_toggles { + self.remote_plugin_enablement_writer + .set_remote_plugin_enabled(plugin_id, enabled) + .await?; + } + + if reload_user_config { + self.config_api + .batch_write(ConfigBatchWriteParams { + edits: Vec::new(), + file_path, + expected_version: None, + reload_user_config: true, + }) + .await?; + } + + Ok(response) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct RemotePluginEnabledConfigEdit { + plugin_id: String, + enabled: bool, +} + +fn remote_plugin_enabled_config_edit( + key_path: &str, + value: &JsonValue, +) -> Option { + let enabled = value.as_bool()?; + let mut segments = key_path.split('.'); + let table = segments.next()?; + let plugin_id = segments.next()?; + let field = segments.next()?; + if table == "plugins" + && field == "enabled" + && segments.next().is_none() + && codex_core_plugins::remote::is_supported_remote_plugin_id(plugin_id) + { + return Some(RemotePluginEnabledConfigEdit { + plugin_id: plugin_id.to_string(), + enabled, + }); + } + None +} diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index b80510b0ff..e183132724 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -77,6 +77,7 @@ mod config; mod config_api; mod config_manager; mod config_manager_service; +mod config_write_router; mod connection_rpc_gate; mod device_key_api; mod dynamic_tools; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 5bc534286b..c8886dc711 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -9,6 +9,8 @@ use crate::codex_message_processor::CodexMessageProcessor; use crate::codex_message_processor::CodexMessageProcessorArgs; use crate::config_api::ConfigApi; use crate::config_manager::ConfigManager; +use crate::config_write_router::ConfigWriteRouter; +use crate::config_write_router::RemotePluginEnablementWriter; use crate::connection_rpc_gate::ConnectionRpcGate; use crate::device_key_api::DeviceKeyApi; use crate::error_code::invalid_request; @@ -164,6 +166,7 @@ pub(crate) struct MessageProcessor { codex_message_processor: Arc, thread_manager: Arc, config_api: ConfigApi, + config_write_router: ConfigWriteRouter, device_key_api: DeviceKeyApi, external_agent_config_api: ExternalAgentConfigApi, fs_api: FsApi, @@ -327,8 +330,11 @@ impl MessageProcessor { config_manager, thread_manager.clone(), analytics_events_client.clone(), - ) - .with_remote_plugin_config_writer(codex_message_processor.clone()); + ); + let remote_plugin_enablement_writer: Arc = + codex_message_processor.clone(); + let config_write_router = + ConfigWriteRouter::new(config_api.clone(), remote_plugin_enablement_writer); let device_key_api = DeviceKeyApi::new(config.sqlite_home.clone(), config.model_provider_id.clone()); let external_agent_config_api = @@ -346,6 +352,7 @@ impl MessageProcessor { codex_message_processor, thread_manager: Arc::clone(&thread_manager), config_api, + config_write_router, device_key_api, external_agent_config_api, fs_api, @@ -993,7 +1000,7 @@ impl MessageProcessor { request_id: ConnectionRequestId, params: ConfigValueWriteParams, ) { - let result = self.config_api.write_value(params).await; + let result = self.config_write_router.write_value(params).await; self.handle_config_mutation_result(request_id, result).await } @@ -1002,7 +1009,7 @@ impl MessageProcessor { request_id: ConnectionRequestId, params: ConfigBatchWriteParams, ) { - let result = self.config_api.batch_write(params).await; + let result = self.config_write_router.batch_write(params).await; self.handle_config_mutation_result(request_id, result).await; } diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 097e168d6a..f500edd56c 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -9,7 +9,6 @@ use codex_login::default_client::build_reqwest_client; use codex_plugin::PluginId; use reqwest::RequestBuilder; use serde::Deserialize; -use serde_json::Value as JsonValue; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashSet; @@ -33,12 +32,6 @@ pub struct RemotePluginServiceConfig { pub chatgpt_base_url: String, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct RemotePluginEnabledConfigEdit { - pub plugin_id: String, - pub enabled: bool, -} - #[derive(Debug, Clone, PartialEq)] pub struct RemoteMarketplace { pub name: String, @@ -553,35 +546,13 @@ pub async fn set_remote_plugin_enabled( Ok(()) } -pub fn remote_plugin_enabled_config_edit( - key_path: &str, - value: &JsonValue, -) -> Option { - let enabled = value.as_bool()?; - let mut segments = key_path.split('.'); - let table = segments.next()?; - let plugin_id = segments.next()?; - let field = segments.next()?; - if table == "plugins" - && field == "enabled" - && segments.next().is_none() - && is_remote_plugin_config_id(plugin_id) - { - return Some(RemotePluginEnabledConfigEdit { - plugin_id: plugin_id.to_string(), - enabled, - }); - } - None -} - pub fn is_valid_remote_plugin_id(plugin_id: &str) -> bool { plugin_id .chars() .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~') } -pub fn is_remote_plugin_config_id(plugin_id: &str) -> bool { +pub fn is_supported_remote_plugin_id(plugin_id: &str) -> bool { !plugin_id.is_empty() && is_valid_remote_plugin_id(plugin_id) && (plugin_id.starts_with("plugins~")