From dd72c7cb18c9d7e577f154ab510889b5f6a64ace Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Sun, 17 May 2026 22:55:16 -0700 Subject: [PATCH] simplify --- codex-rs/Cargo.lock | 2 - codex-rs/core-api/Cargo.toml | 1 - codex-rs/core-api/src/lib.rs | 7 -- codex-rs/core/src/prompt_debug.rs | 11 +-- codex-rs/core/src/session/session.rs | 31 ++++++-- codex-rs/core/src/tools/handlers/mod.rs | 1 + .../tools/handlers/request_plugin_install.rs | 37 ++++++++- .../core/src/tools/installable_plugins.rs | 75 ------------------- codex-rs/core/src/tools/mod.rs | 1 - codex-rs/ext/plugins/src/lib.rs | 31 ++++++++ codex-rs/mcp-server/Cargo.toml | 1 - codex-rs/mcp-server/src/message_processor.rs | 11 +-- codex-rs/thread-manager-sample/src/main.rs | 4 +- 13 files changed, 100 insertions(+), 113 deletions(-) delete mode 100644 codex-rs/core/src/tools/installable_plugins.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d0f14800bd..113c2d5a26 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2609,7 +2609,6 @@ dependencies = [ "codex-login", "codex-model-provider-info", "codex-models-manager", - "codex-plugins-extension", "codex-protocol", "codex-utils-absolute-path", ] @@ -3120,7 +3119,6 @@ dependencies = [ "codex-exec-server", "codex-extension-api", "codex-login", - "codex-plugins-extension", "codex-protocol", "codex-shell-command", "codex-utils-absolute-path", diff --git a/codex-rs/core-api/Cargo.toml b/codex-rs/core-api/Cargo.toml index 4dae10e385..998da39ce7 100644 --- a/codex-rs/core-api/Cargo.toml +++ b/codex-rs/core-api/Cargo.toml @@ -25,6 +25,5 @@ codex-features = { workspace = true } codex-login = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } -codex-plugins-extension = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index e120f619e4..04ebaf8e7e 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -76,10 +76,3 @@ pub use codex_protocol::protocol::TurnEnvironmentSelection; pub use codex_protocol::protocol::W3cTraceContext; pub use codex_protocol::user_input::UserInput; pub use codex_utils_absolute_path::AbsolutePathBuf; - -pub fn plugins_extension_registry() -> std::sync::Arc> -{ - let mut builder = codex_extension_api::ExtensionRegistryBuilder::new(); - codex_plugins_extension::install(&mut builder); - std::sync::Arc::new(builder.build()) -} diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index d8e3cfa015..72fa5b4e44 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -20,8 +20,7 @@ use crate::session::turn::built_tools; use crate::state_db_bridge::StateDbHandle; use crate::thread_manager::ThreadManager; use crate::thread_manager::thread_store_from_config; -use codex_extension_api::ExtensionRegistry; -use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::empty_extension_registry; /// Build the model-visible `input` list for a single debug turn. #[doc(hidden)] @@ -51,7 +50,7 @@ pub async fn build_prompt_input( .await .map_err(|err| CodexErr::Fatal(err.to_string()))?, ), - plugins_extension_registry(), + empty_extension_registry(), /*analytics_events_client*/ None, thread_store, state_db.clone(), @@ -106,9 +105,3 @@ pub(crate) async fn build_prompt_input_from_session( Ok(prompt.get_formatted_input()) } - -fn plugins_extension_registry() -> Arc> { - let mut builder = ExtensionRegistryBuilder::new(); - codex_plugins_extension::install(&mut builder); - Arc::new(builder.build()) -} diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 34fd5f2b3e..8251a4ee48 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -971,12 +971,33 @@ impl Session { services, next_internal_sub_id: AtomicU64::new(0), }); + let session = Arc::downgrade(&sess); sess.services.session_extension_data.insert( - codex_plugins_extension::InstallablePluginsProviderHandle::new(Arc::new( - crate::tools::installable_plugins::SessionInstallablePluginsProvider::new( - Arc::downgrade(&sess), - ), - )), + codex_plugins_extension::InstallablePluginsProviderHandle::from_fn(move || { + let session = session.clone(); + Box::pin(async move { + let session = session.upgrade().ok_or_else(|| { + "plugin install requests are unavailable right now".to_string() + })?; + let config = session.get_config().await; + let app_server_client_metadata = + session.app_server_client_metadata().await; + let discoverable_tools = + crate::tools::handlers::discoverable_request_plugin_install_tools( + session.as_ref(), + config.as_ref(), + app_server_client_metadata.client_name.as_deref(), + ) + .await + .map_err(|err| { + format!("plugin install requests are unavailable right now: {err}") + })?; + + Ok(codex_tools::collect_request_plugin_install_entries( + &discoverable_tools, + )) + }) + }), ); if let Some(network_policy_decider_session) = network_policy_decider_session { let mut guard = network_policy_decider_session.write().await; diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 69281acc7d..f574c1da0b 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -61,6 +61,7 @@ pub use mcp_resource::ReadMcpResourceHandler; pub use plan::PlanHandler; pub use request_permissions::RequestPermissionsHandler; pub use request_plugin_install::RequestPluginInstallHandler; +pub(crate) use request_plugin_install::discoverable_request_plugin_install_tools; pub use request_user_input::RequestUserInputHandler; pub use shell::ShellCommandHandler; pub(crate) use shell::ShellCommandHandlerOptions; diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index 8b2c246a2a..6e474788fe 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -19,22 +19,24 @@ use codex_tools::ToolSpec; use codex_tools::all_requested_connectors_picked_up; use codex_tools::build_request_plugin_install_elicitation_request; use codex_tools::collect_request_plugin_install_entries; +use codex_tools::filter_request_plugin_install_discoverable_tools_for_client; use codex_tools::verified_connector_install_completed; use rmcp::model::RequestId; use serde_json::Value; use tracing::warn; +use crate::config::Config; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::connectors; use crate::function_tool::FunctionCallError; +use crate::session::session::Session; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_install_tool; -use crate::tools::installable_plugins::discoverable_request_plugin_install_tools; use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; @@ -44,6 +46,39 @@ pub struct RequestPluginInstallHandler { plugin_install_list_tool: bool, } +#[expect( + clippy::await_holding_invalid_type, + reason = "plugin install discovery reads through the session-owned manager guard" +)] +pub(crate) async fn discoverable_request_plugin_install_tools( + session: &Session, + config: &Config, + app_server_client_name: Option<&str>, +) -> anyhow::Result> { + let auth = session.services.auth_manager.auth().await; + let manager = session.services.mcp_connection_manager.read().await; + let mcp_tools = manager.list_all_tools().await; + drop(manager); + + let accessible_connectors = connectors::with_app_enabled_state( + connectors::accessible_connectors_from_mcp_tools(&mcp_tools), + config, + ); + connectors::list_tool_suggest_discoverable_tools_with_auth( + config, + auth.as_ref(), + &accessible_connectors, + session.services.plugins_manager.as_ref(), + ) + .await + .map(|discoverable_tools| { + filter_request_plugin_install_discoverable_tools_for_client( + discoverable_tools, + app_server_client_name, + ) + }) +} + impl RequestPluginInstallHandler { pub(crate) fn new( discoverable_tools: &[DiscoverableTool], diff --git a/codex-rs/core/src/tools/installable_plugins.rs b/codex-rs/core/src/tools/installable_plugins.rs deleted file mode 100644 index f7be1aeb2b..0000000000 --- a/codex-rs/core/src/tools/installable_plugins.rs +++ /dev/null @@ -1,75 +0,0 @@ -use std::sync::Weak; - -use codex_plugins_extension::InstallablePluginsProvider; -use codex_tools::DiscoverableTool; -use codex_tools::RequestPluginInstallEntry; -use codex_tools::collect_request_plugin_install_entries; -use codex_tools::filter_request_plugin_install_discoverable_tools_for_client; - -use crate::config::Config; -use crate::connectors; -use crate::session::session::Session; - -pub(crate) struct SessionInstallablePluginsProvider { - session: Weak, -} - -impl SessionInstallablePluginsProvider { - pub(crate) fn new(session: Weak) -> Self { - Self { session } - } -} - -#[async_trait::async_trait] -impl InstallablePluginsProvider for SessionInstallablePluginsProvider { - async fn list_installable_plugins(&self) -> Result, String> { - let session = self - .session - .upgrade() - .ok_or_else(|| "plugin install requests are unavailable right now".to_string())?; - let config = session.get_config().await; - let app_server_client_metadata = session.app_server_client_metadata().await; - let discoverable_tools = discoverable_request_plugin_install_tools( - session.as_ref(), - config.as_ref(), - app_server_client_metadata.client_name.as_deref(), - ) - .await - .map_err(|err| format!("plugin install requests are unavailable right now: {err}"))?; - - Ok(collect_request_plugin_install_entries(&discoverable_tools)) - } -} - -#[expect( - clippy::await_holding_invalid_type, - reason = "plugin install discovery reads through the session-owned manager guard" -)] -pub(crate) async fn discoverable_request_plugin_install_tools( - session: &Session, - config: &Config, - app_server_client_name: Option<&str>, -) -> anyhow::Result> { - let auth = session.services.auth_manager.auth().await; - let manager = session.services.mcp_connection_manager.read().await; - let mcp_tools = manager.list_all_tools().await; - drop(manager); - - let accessible_connectors = connectors::with_app_enabled_state( - connectors::accessible_connectors_from_mcp_tools(&mcp_tools), - config, - ); - connectors::list_tool_suggest_discoverable_tools_with_auth( - config, - auth.as_ref(), - &accessible_connectors, - session.services.plugins_manager.as_ref(), - ) - .await - .map(|discoverable_tools| { - filter_request_plugin_install_discoverable_tools_for_client( - discoverable_tools, - app_server_client_name, - ) - }) -} diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index f6fae2672a..5cd943e199 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -4,7 +4,6 @@ pub(crate) mod events; pub(crate) mod handlers; pub(crate) mod hook_names; pub(crate) mod hosted_spec; -pub(crate) mod installable_plugins; pub(crate) mod network_approval; pub(crate) mod orchestrator; pub(crate) mod parallel; diff --git a/codex-rs/ext/plugins/src/lib.rs b/codex-rs/ext/plugins/src/lib.rs index 41d1702bfe..09f1875209 100644 --- a/codex-rs/ext/plugins/src/lib.rs +++ b/codex-rs/ext/plugins/src/lib.rs @@ -1,4 +1,6 @@ use std::collections::BTreeMap; +use std::future::Future; +use std::pin::Pin; use std::sync::Arc; use codex_extension_api::ExtensionData; @@ -31,11 +33,40 @@ pub struct InstallablePluginsProviderHandle { provider: Arc, } +pub type InstallablePluginsFuture = + Pin, String>> + Send>>; + +struct FnInstallablePluginsProvider +where + F: Fn() -> InstallablePluginsFuture + Send + Sync, +{ + list_installable_plugins: F, +} + +#[async_trait::async_trait] +impl InstallablePluginsProvider for FnInstallablePluginsProvider +where + F: Fn() -> InstallablePluginsFuture + Send + Sync, +{ + async fn list_installable_plugins(&self) -> Result, String> { + (self.list_installable_plugins)().await + } +} + impl InstallablePluginsProviderHandle { pub fn new(provider: Arc) -> Self { Self { provider } } + pub fn from_fn(list_installable_plugins: F) -> Self + where + F: Fn() -> InstallablePluginsFuture + Send + Sync + 'static, + { + Self::new(Arc::new(FnInstallablePluginsProvider { + list_installable_plugins, + })) + } + async fn list_installable_plugins(&self) -> Result, String> { self.provider.list_installable_plugins().await } diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 6892676b7e..29b2d6c7d7 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -24,7 +24,6 @@ codex-core = { workspace = true } codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } codex-login = { workspace = true } -codex-plugins-extension = { workspace = true } codex-protocol = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-json-to-toml = { workspace = true } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 19d59b0d8d..9e536d930c 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -6,8 +6,7 @@ use codex_core::StateDbHandle; use codex_core::ThreadManager; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; -use codex_extension_api::ExtensionRegistry; -use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::empty_extension_registry; use codex_login::AuthManager; use codex_login::default_client::USER_AGENT_SUFFIX; use codex_login::default_client::get_codex_user_agent; @@ -69,7 +68,7 @@ impl MessageProcessor { auth_manager, SessionSource::Mcp, environment_manager, - plugins_extension_registry(), + empty_extension_registry(), /*analytics_events_client*/ None, codex_core::thread_store_from_config(config.as_ref(), state_db.clone()), state_db.clone(), @@ -609,9 +608,3 @@ impl MessageProcessor { tracing::info!("notifications/initialized"); } } - -fn plugins_extension_registry() -> Arc> { - let mut builder = ExtensionRegistryBuilder::new(); - codex_plugins_extension::install(&mut builder); - Arc::new(builder.build()) -} diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 70fb77ec55..8a168bad13 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -53,10 +53,10 @@ use codex_core_api::UserInput; use codex_core_api::WebSearchMode; use codex_core_api::arg0_dispatch_or_else; use codex_core_api::built_in_model_providers; +use codex_core_api::empty_extension_registry; use codex_core_api::find_codex_home; use codex_core_api::init_state_db; use codex_core_api::item_event_to_server_notification; -use codex_core_api::plugins_extension_registry; use codex_core_api::resolve_installation_id; use codex_core_api::set_default_originator; use codex_core_api::thread_store_from_config; @@ -123,7 +123,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { auth_manager, SessionSource::Exec, environment_manager, - plugins_extension_registry(), + empty_extension_registry(), /*analytics_events_client*/ None, Arc::clone(&thread_store), state_db,