From 10a8a4e84f47c24e07ffd24e7f2eee2464649b52 Mon Sep 17 00:00:00 2001 From: Noah MacCallum Date: Fri, 29 May 2026 01:31:17 -0700 Subject: [PATCH] Support plugin install suggestions from loaded plugin apps --- codex-rs/core/src/connectors.rs | 10 ++-- codex-rs/core/src/connectors_tests.rs | 2 +- codex-rs/core/src/plugins/discoverable.rs | 36 ++++--------- .../core/src/plugins/discoverable_tests.rs | 52 ++++++++++++++----- codex-rs/core/src/session/turn.rs | 6 +++ .../tools/handlers/request_plugin_install.rs | 42 +++++---------- codex-rs/core/src/tools/spec_plan.rs | 4 +- 7 files changed, 78 insertions(+), 74 deletions(-) diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 09717bb121..70ccbc83d9 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -113,6 +113,7 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( config: &Config, auth: Option<&CodexAuth>, accessible_connectors: &[AppInfo], + loaded_plugin_app_connector_ids: &[String], ) -> anyhow::Result> { let connector_ids = tool_suggest_connector_ids(config).await; let directory_connectors = codex_connectors::merge::merge_plugin_connectors( @@ -128,10 +129,11 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( ) .into_iter() .map(DiscoverableTool::from); - let discoverable_plugins = list_tool_suggest_discoverable_plugins(config) - .await? - .into_iter() - .map(DiscoverableTool::from); + let discoverable_plugins = + list_tool_suggest_discoverable_plugins(config, loaded_plugin_app_connector_ids) + .await? + .into_iter() + .map(DiscoverableTool::from); Ok(discoverable_connectors .chain(discoverable_plugins) .collect()) diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 3733d78c38..972743c6eb 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -1238,7 +1238,7 @@ discoverables = [ let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let discoverable_tools = - list_tool_suggest_discoverable_tools_with_auth(&config, Some(&auth), &[]) + list_tool_suggest_discoverable_tools_with_auth(&config, Some(&auth), &[], &[]) .await .expect("discoverable tools should load"); diff --git a/codex-rs/core/src/plugins/discoverable.rs b/codex-rs/core/src/plugins/discoverable.rs index 3ecd08b65d..4f0d88b512 100644 --- a/codex-rs/core/src/plugins/discoverable.rs +++ b/codex-rs/core/src/plugins/discoverable.rs @@ -20,6 +20,7 @@ const TOOL_SUGGEST_DISCOVERABLE_MARKETPLACE_ALLOWLIST: &[&str] = &[ pub(crate) async fn list_tool_suggest_discoverable_plugins( config: &Config, + loaded_plugin_app_connector_ids: &[String], ) -> anyhow::Result> { if !config.features.enabled(Feature::Plugins) { return Ok(Vec::new()); @@ -45,32 +46,15 @@ pub(crate) async fn list_tool_suggest_discoverable_plugins( .list_marketplaces_for_config(&plugins_input, &[]) .context("failed to list plugin marketplaces for tool suggestions")? .marketplaces; - let mut installed_app_connector_ids = HashSet::::new(); - for marketplace in &marketplaces { - for plugin in &marketplace.plugins { - if !plugin.installed { - continue; - } - - let plugin_id = plugin.id.clone(); - match plugins_manager - .read_plugin_detail_for_marketplace_plugin( - &plugins_input, - &marketplace.name, - plugin.clone(), - ) - .await - { - Ok(plugin) => { - installed_app_connector_ids - .extend(plugin.apps.into_iter().map(|connector_id| connector_id.0)); - } - Err(err) => warn!( - "failed to load installed plugin apps for tool suggestion {plugin_id}: {err:#}" - ), - } - } - } + let mut installed_app_connector_ids = plugins_manager + .plugins_for_config(&plugins_input) + .await + .capability_summaries() + .iter() + .flat_map(|plugin| plugin.app_connector_ids.iter()) + .map(|connector_id| connector_id.0.clone()) + .collect::>(); + installed_app_connector_ids.extend(loaded_plugin_app_connector_ids.iter().cloned()); let mut discoverable_plugins = Vec::::new(); for marketplace in marketplaces { diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 8e4eaf1bee..21203aea23 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -25,7 +25,7 @@ async fn list_tool_suggest_discoverable_plugins_returns_fallback_plugins_without write_plugins_feature_config(codex_home.path()); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -51,7 +51,7 @@ async fn list_tool_suggest_discoverable_plugins_filters_non_fallback_by_installe install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "slack").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -64,6 +64,32 @@ async fn list_tool_suggest_discoverable_plugins_filters_non_fallback_by_installe ); } +#[tokio::test] +async fn list_tool_suggest_discoverable_plugins_filters_by_loaded_plugin_apps() { + let hubspot_app_id = "asdk_app_697acb8e53d88191bf7a79e62012ae14"; + let granola_app_id = "asdk_app_697761cab6f48191b5ed345919a3ce8b"; + let codex_home = tempdir().expect("tempdir should succeed"); + let curated_root = curated_plugins_repo_path(codex_home.path()); + write_openai_curated_marketplace(&curated_root, &["hubspot", "granola"]); + write_plugin_app(&curated_root, "hubspot", "hubspot", hubspot_app_id); + write_plugin_app(&curated_root, "granola", "granola", granola_app_id); + write_plugins_feature_config(codex_home.path()); + + let config = load_plugins_config(codex_home.path()).await; + let discoverable_plugins = + list_tool_suggest_discoverable_plugins(&config, &[hubspot_app_id.to_string()]) + .await + .unwrap(); + + assert_eq!( + discoverable_plugins + .into_iter() + .map(|plugin| plugin.id) + .collect::>(), + vec!["hubspot@openai-curated".to_string()] + ); +} + #[tokio::test] async fn list_tool_suggest_discoverable_plugins_filters_microsoft_by_installed_apps() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -76,7 +102,7 @@ async fn list_tool_suggest_discoverable_plugins_filters_microsoft_by_installed_a install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "teams").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -153,7 +179,7 @@ source = "/tmp/{sales_marketplace_name}" install_marketplace_plugin(codex_home.path(), sales_marketplace_root.as_path(), "sales").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -208,7 +234,7 @@ discoverables = [{{ type = "plugin", id = "{plugin_id}" }}] ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -252,7 +278,7 @@ source = "/tmp/{marketplace_name}" install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -273,7 +299,7 @@ plugins = false ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -296,7 +322,7 @@ async fn list_tool_suggest_discoverable_plugins_normalizes_description() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -333,7 +359,7 @@ async fn list_tool_suggest_discoverable_plugins_omits_installed_curated_plugins( .expect("plugin should install"); let refreshed_config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&refreshed_config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&refreshed_config, &[]) .await .unwrap(); @@ -358,7 +384,7 @@ disabled_tools = [ ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -409,7 +435,7 @@ async fn list_tool_suggest_discoverable_plugins_omits_not_available_curated_plug install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -438,7 +464,7 @@ discoverables = [{ type = "plugin", id = "sample@openai-curated" }] ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); @@ -493,7 +519,7 @@ async fn list_tool_suggest_discoverable_plugins_does_not_reload_marketplace_per_ .finish(); let _guard = tracing::subscriber::set_default(subscriber); - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config) + let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) .await .unwrap(); diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 8219c19019..efa55c032a 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1075,12 +1075,18 @@ pub(crate) async fn built_tools( None }; let auth = sess.services.auth_manager.auth().await; + let loaded_plugin_app_connector_ids = loaded_plugins + .effective_apps() + .into_iter() + .map(|connector_id| connector_id.0) + .collect::>(); let discoverable_tools = if apps_enabled && tool_suggest_enabled(turn_context) { if let Some(accessible_connectors) = accessible_connectors_with_enabled_state.as_ref() { match connectors::list_tool_suggest_discoverable_tools_with_auth( &turn_context.config, auth.as_ref(), accessible_connectors.as_slice(), + &loaded_plugin_app_connector_ids, ) .await .map(|discoverable_tools| { 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 b73a81a940..c134eae079 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -37,7 +37,15 @@ use crate::tools::handlers::request_plugin_install_spec::create_request_plugin_i use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; -pub struct RequestPluginInstallHandler; +pub struct RequestPluginInstallHandler { + discoverable_tools: Vec, +} + +impl RequestPluginInstallHandler { + pub(crate) fn new(discoverable_tools: Vec) -> Self { + Self { discoverable_tools } + } +} #[async_trait::async_trait] impl ToolExecutor for RequestPluginInstallHandler { @@ -53,10 +61,6 @@ impl ToolExecutor for RequestPluginInstallHandler { true } - #[expect( - clippy::await_holding_invalid_type, - reason = "plugin install discovery reads through the session-owned manager guard" - )] async fn handle( &self, invocation: ToolInvocation, @@ -99,31 +103,10 @@ impl ToolExecutor for RequestPluginInstallHandler { )); } - 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), - &turn.config, + let discoverable_tools = filter_request_plugin_install_discoverable_tools_for_client( + self.discoverable_tools.clone(), + turn.app_server_client_name.as_deref(), ); - let discoverable_tools = connectors::list_tool_suggest_discoverable_tools_with_auth( - &turn.config, - auth.as_ref(), - &accessible_connectors, - ) - .await - .map(|discoverable_tools| { - filter_request_plugin_install_discoverable_tools_for_client( - discoverable_tools, - turn.app_server_client_name.as_deref(), - ) - }) - .map_err(|err| { - FunctionCallError::RespondToModel(format!( - "plugin install requests are unavailable right now: {err}" - )) - })?; let tool = discoverable_tools .into_iter() @@ -154,6 +137,7 @@ impl ToolExecutor for RequestPluginInstallHandler { .as_ref() .is_some_and(|response| response.action == ElicitationAction::Accept); + let auth = session.services.auth_manager.auth().await; let completed = if user_confirmed { verify_request_plugin_install_completed(&session, &turn, &tool, auth.as_ref()).await } else { diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index bb7baf4bd8..45327c3eb5 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -617,7 +617,9 @@ fn add_core_utility_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut planned_tools.add(ListAvailablePluginsToInstallHandler::new( collect_request_plugin_install_entries(discoverable_tools), )); - planned_tools.add(RequestPluginInstallHandler); + planned_tools.add(RequestPluginInstallHandler::new( + discoverable_tools.to_vec(), + )); } if environment_mode.has_environment() && turn_context.model_info.apply_patch_tool_type.is_some()