From c7aaadc76f6ce240a48c85d24386fbf57982fe9f Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Mon, 18 May 2026 00:11:05 -0700 Subject: [PATCH] simplify dependency --- codex-rs/Cargo.lock | 1 - codex-rs/app-server/src/extensions.rs | 7 +- codex-rs/app-server/src/mcp_refresh.rs | 6 +- codex-rs/app-server/src/message_processor.rs | 6 +- .../tools/handlers/request_plugin_install.rs | 1 - codex-rs/core/src/tools/spec_plan.rs | 12 +-- codex-rs/core/src/tools/spec_plan_tests.rs | 16 ++- codex-rs/core/tests/common/test_codex.rs | 2 +- codex-rs/ext/plugins/Cargo.toml | 1 - codex-rs/ext/plugins/src/lib.rs | 99 ++++++------------- codex-rs/tools/src/tool_config.rs | 4 - 11 files changed, 63 insertions(+), 92 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 113c2d5a26..9f6ef752db 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3402,7 +3402,6 @@ dependencies = [ "codex-extension-api", "codex-tools", "pretty_assertions", - "serde", "serde_json", "tokio", ] diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index ea33737571..5bb8d414f4 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -12,14 +12,17 @@ use codex_extension_api::ExtensionRegistryBuilder; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; -pub(crate) fn thread_extensions(guardian_agent_spawner: S) -> Arc> +pub(crate) fn thread_extensions( + guardian_agent_spawner: S, + plugin_install_list_tool_enabled: bool, +) -> Arc> where S: AgentSpawner + 'static, { let mut builder = ExtensionRegistryBuilder::::new(); codex_guardian::install(&mut builder, guardian_agent_spawner); codex_memories_extension::install(&mut builder); - codex_plugins_extension::install(&mut builder); + codex_plugins_extension::install(&mut builder, plugin_install_list_tool_enabled); Arc::new(builder.build()) } diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index f7d32b2ea8..0ea6c8769e 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -2,6 +2,7 @@ use crate::config_manager::ConfigManager; use codex_core::CodexThread; use codex_core::ThreadManager; use codex_core::config::Config; +use codex_features::Feature; use codex_protocol::ThreadId; use codex_protocol::protocol::McpServerRefreshConfig; use codex_protocol::protocol::Op; @@ -186,7 +187,10 @@ mod tests { auth_manager, SessionSource::Exec, Arc::new(EnvironmentManager::default_for_tests()), - thread_extensions(guardian_agent_spawner(thread_manager.clone())), + thread_extensions( + guardian_agent_spawner(thread_manager.clone()), + good_config.features.enabled(Feature::PluginInstallListTool), + ), /*analytics_events_client*/ None, thread_store, Some(state_db.clone()), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index fc835b82ec..e2a95d8cec 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -68,6 +68,7 @@ use codex_chatgpt::workspace_settings; use codex_core::ThreadManager; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; +use codex_features::Feature; use codex_feedback::CodexFeedback; use codex_login::AuthManager; use codex_login::auth::ExternalAuth; @@ -307,7 +308,10 @@ impl MessageProcessor { auth_manager.clone(), session_source, environment_manager, - thread_extensions(guardian_agent_spawner(thread_manager.clone())), + thread_extensions( + guardian_agent_spawner(thread_manager.clone()), + config.features.enabled(Feature::PluginInstallListTool), + ), Some(analytics_events_client.clone()), Arc::clone(&thread_store), state_db.clone(), 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 6e474788fe..f6dad793d6 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -40,7 +40,6 @@ use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_i use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -#[derive(Default)] pub struct RequestPluginInstallHandler { discoverable_tools: Vec, plugin_install_list_tool: bool, diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 0809f4b73d..fd704a26e7 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -85,6 +85,7 @@ struct ToolRegistryBuildParams<'a> { deferred_mcp_tools: Option<&'a [ToolInfo]>, discoverable_tools: Option<&'a [DiscoverableTool]>, extension_tool_executors: &'a [Arc>], + list_installable_plugins_registered: bool, dynamic_tools: &'a [DynamicToolSpec], default_agent_type_description: &'a str, wait_agent_timeouts: WaitAgentTimeoutOptions, @@ -106,6 +107,9 @@ fn build_tool_specs_and_registry( extension_tool_executors, dynamic_tools, } = params; + let list_installable_plugins_registered = extension_tool_executors.iter().any(|executor| { + executor.tool_name() == ToolName::plain(LIST_INSTALLABLE_PLUGINS_TOOL_NAME) + }); let default_agent_type_description = crate::agent::role::spawn_tool_spec::build(&std::collections::BTreeMap::new()); let mut executors = collect_tool_executors( @@ -115,6 +119,7 @@ fn build_tool_specs_and_registry( deferred_mcp_tools: deferred_mcp_tools.as_deref(), discoverable_tools: discoverable_tools.as_deref(), extension_tool_executors: &extension_tool_executors, + list_installable_plugins_registered, dynamic_tools, default_agent_type_description: &default_agent_type_description, wait_agent_timeouts: wait_agent_timeout_options(config), @@ -419,7 +424,7 @@ fn collect_tool_executors( { executors.push(Arc::new(RequestPluginInstallHandler::new( discoverable_tools, - config.plugin_install_list_tool, + params.list_installable_plugins_registered, ))); } @@ -614,11 +619,6 @@ fn append_extension_tool_executors( for executor in executors.iter().cloned() { let tool_name = executor.tool_name(); - if tool_name == ToolName::plain(LIST_INSTALLABLE_PLUGINS_TOOL_NAME) - && !config.plugin_install_list_tool - { - continue; - } if !reserved_tool_names.insert(tool_name.clone()) { warn!("Skipping extension tool `{tool_name}`: tool already registered"); continue; diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 327742b0bf..f53279b755 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -1970,7 +1970,10 @@ fn request_plugin_install_is_not_registered_without_feature_flag() { "Google Calendar", "Plan events and schedules.", )]), - /*extension_tool_executors*/ &[], + &[extension_tool_executor( + LIST_INSTALLABLE_PLUGINS_TOOL_NAME, + "List installable plugins.", + )], &[], ); @@ -2011,7 +2014,10 @@ fn request_plugin_install_can_be_registered_without_search_tool() { "Google Calendar", "Plan events and schedules.", )]), - /*extension_tool_executors*/ &[], + &[extension_tool_executor( + LIST_INSTALLABLE_PLUGINS_TOOL_NAME, + "List installable plugins.", + )], &[], ); @@ -2039,14 +2045,13 @@ fn request_plugin_install_can_be_registered_without_search_tool() { } #[test] -fn request_plugin_install_description_lists_discoverable_tools() { +fn request_plugin_install_description_lists_discoverable_tools_without_list_tool() { let model_info = search_capable_model_info(); let mut features = Features::with_defaults(); features.enable(Feature::Apps); features.enable(Feature::Plugins); features.enable(Feature::ToolSearch); features.enable(Feature::ToolSuggest); - features.disable(Feature::PluginInstallListTool); let available_models = Vec::new(); let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, @@ -2509,6 +2514,9 @@ fn build_specs_with_inputs_for_test( deferred_mcp_tools: deferred_mcp_tools.as_deref(), discoverable_tools: discoverable_tools.as_deref(), extension_tool_executors, + list_installable_plugins_registered: extension_tool_executors.iter().any(|executor| { + executor.tool_name() == ToolName::plain(LIST_INSTALLABLE_PLUGINS_TOOL_NAME) + }), dynamic_tools, default_agent_type_description: DEFAULT_AGENT_TYPE_DESCRIPTION, wait_agent_timeouts: wait_agent_timeout_options(), diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 5c3727b551..97b77e143f 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -477,7 +477,7 @@ impl TestCodexBuilder { let installation_id = resolve_installation_id(&config.codex_home).await?; let extensions = if self.plugins_extension_enabled { let mut builder = ExtensionRegistryBuilder::new(); - codex_plugins_extension::install(&mut builder); + codex_plugins_extension::install(&mut builder, true); Arc::new(builder.build()) } else { empty_extension_registry() diff --git a/codex-rs/ext/plugins/Cargo.toml b/codex-rs/ext/plugins/Cargo.toml index bc9aeb966d..f4542ea4dd 100644 --- a/codex-rs/ext/plugins/Cargo.toml +++ b/codex-rs/ext/plugins/Cargo.toml @@ -16,7 +16,6 @@ workspace = true async-trait = { workspace = true } codex-extension-api = { workspace = true } codex-tools = { workspace = true } -serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } [dev-dependencies] diff --git a/codex-rs/ext/plugins/src/lib.rs b/codex-rs/ext/plugins/src/lib.rs index 09f1875209..85f168c5b6 100644 --- a/codex-rs/ext/plugins/src/lib.rs +++ b/codex-rs/ext/plugins/src/lib.rs @@ -17,66 +17,34 @@ use codex_extension_api::ToolSpec; use codex_tools::JsonSchema; use codex_tools::LIST_INSTALLABLE_PLUGINS_TOOL_NAME; use codex_tools::RequestPluginInstallEntry; -use serde::Serialize; use serde_json::json; -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug)] struct PluginsExtension; -#[async_trait::async_trait] -pub trait InstallablePluginsProvider: Send + Sync { - async fn list_installable_plugins(&self) -> Result, String>; -} - -#[derive(Clone)] -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 - } +#[derive(Clone)] +pub struct InstallablePluginsProviderHandle { + list_installable_plugins: Arc InstallablePluginsFuture + Send + Sync + 'static>, } 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, - })) + Self { + list_installable_plugins: Arc::new(list_installable_plugins), + } } async fn list_installable_plugins(&self) -> Result, String> { - self.provider.list_installable_plugins().await + (self.list_installable_plugins)().await } } -#[derive(Clone, Debug, Serialize, PartialEq, Eq)] -struct ListInstallablePluginsResponse { - entries: Vec, -} - impl ToolContributor for PluginsExtension { fn tools( &self, @@ -128,15 +96,18 @@ impl ToolExecutor for ListInstallablePluginsTool { .then_with(|| left.id.cmp(&right.id)) }); - Ok(Box::new(JsonToolOutput::new(json!( - ListInstallablePluginsResponse { entries } - )))) + Ok(Box::new(JsonToolOutput::new(json!({ "entries": entries })))) } } /// Installs plugins extension contributors into the supplied extension registry. -pub fn install(registry: &mut ExtensionRegistryBuilder) { - registry.tool_contributor(Arc::new(PluginsExtension)); +pub fn install( + registry: &mut ExtensionRegistryBuilder, + list_installable_plugins_enabled: bool, +) { + if list_installable_plugins_enabled { + registry.tool_contributor(Arc::new(PluginsExtension)); + } } #[cfg(test)] @@ -145,18 +116,6 @@ mod tests { use codex_tools::DiscoverableToolType; use pretty_assertions::assert_eq; - #[derive(Clone)] - struct StaticInstallablePluginsProvider { - entries: Vec, - } - - #[async_trait::async_trait] - impl InstallablePluginsProvider for StaticInstallablePluginsProvider { - async fn list_installable_plugins(&self) -> Result, String> { - Ok(self.entries.clone()) - } - } - #[test] fn tools_are_not_contributed_without_provider() { let extension = PluginsExtension; @@ -175,19 +134,19 @@ mod tests { async fn list_tool_returns_provider_installable_entries() { let extension = PluginsExtension; let session_store = ExtensionData::new("session"); - session_store.insert(InstallablePluginsProviderHandle::new(Arc::new( - StaticInstallablePluginsProvider { - entries: vec![RequestPluginInstallEntry { - id: "sample@openai-curated".to_string(), - name: "Sample Plugin".to_string(), - description: Some("Adds sample capabilities.".to_string()), - tool_type: DiscoverableToolType::Plugin, - has_skills: true, - mcp_server_names: vec!["sample-docs".to_string()], - app_connector_ids: vec!["connector_sample".to_string()], - }], - }, - ))); + let entries = vec![RequestPluginInstallEntry { + id: "sample@openai-curated".to_string(), + name: "Sample Plugin".to_string(), + description: Some("Adds sample capabilities.".to_string()), + tool_type: DiscoverableToolType::Plugin, + has_skills: true, + mcp_server_names: vec!["sample-docs".to_string()], + app_connector_ids: vec!["connector_sample".to_string()], + }]; + session_store.insert(InstallablePluginsProviderHandle::from_fn(move || { + let entries = entries.clone(); + Box::pin(async move { Ok(entries) }) + })); let tools = extension.tools(&session_store, &ExtensionData::new("thread")); assert_eq!(tools.len(), 1); diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index c978cf8a73..5c1681d164 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -109,7 +109,6 @@ pub struct ToolsConfig { pub search_tool: bool, pub namespace_tools: bool, pub tool_suggest: bool, - pub plugin_install_list_tool: bool, pub exec_permission_approvals_enabled: bool, pub request_permissions_tool_enabled: bool, pub code_mode_enabled: bool, @@ -188,8 +187,6 @@ impl ToolsConfig { let include_tool_suggest = features.enabled(Feature::ToolSuggest) && features.enabled(Feature::Apps) && features.enabled(Feature::Plugins); - let include_plugin_install_list_tool = - include_tool_suggest && features.enabled(Feature::PluginInstallListTool); let include_original_image_detail = can_request_original_image_detail(model_info); // API-key auth bypasses Codex backend entitlement/tool normalization, so // callers must confirm ChatGPT auth before exposing the built-in tool. @@ -252,7 +249,6 @@ impl ToolsConfig { search_tool: include_search_tool, namespace_tools: true, tool_suggest: include_tool_suggest, - plugin_install_list_tool: include_plugin_install_list_tool, exec_permission_approvals_enabled, request_permissions_tool_enabled, code_mode_enabled: include_code_mode,