diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index afecf79fcf..b264e5a16e 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -122,6 +122,80 @@ fn share_context_for_source( } } +fn configured_marketplace_to_info( + marketplace: codex_core_plugins::ConfiguredMarketplace, + shared_plugin_ids_by_local_path: &std::collections::BTreeMap, +) -> PluginMarketplaceEntry { + PluginMarketplaceEntry { + name: marketplace.name, + path: Some(marketplace.path), + interface: marketplace.interface.map(|interface| MarketplaceInterface { + display_name: interface.display_name, + }), + plugins: marketplace + .plugins + .into_iter() + .map(|plugin| { + configured_marketplace_plugin_to_info(plugin, shared_plugin_ids_by_local_path) + }) + .collect(), + } +} + +fn configured_marketplace_plugin_to_info( + plugin: codex_core_plugins::ConfiguredMarketplacePlugin, + shared_plugin_ids_by_local_path: &std::collections::BTreeMap, +) -> PluginSummary { + let share_context = share_context_for_source(&plugin.source, shared_plugin_ids_by_local_path); + PluginSummary { + id: plugin.id, + remote_plugin_id: None, + local_version: plugin.local_version, + installed: plugin.installed, + enabled: plugin.enabled, + name: plugin.name, + share_context, + source: marketplace_plugin_source_to_info(plugin.source), + install_policy: plugin.policy.installation.into(), + auth_policy: plugin.policy.authentication.into(), + availability: PluginAvailability::Available, + interface: plugin.interface.map(local_plugin_interface_to_info), + keywords: plugin.keywords, + } +} + +fn merge_plugin_marketplace_entry( + data: &mut Vec, + incoming: PluginMarketplaceEntry, +) { + let Some(existing) = data + .iter_mut() + .find(|marketplace| marketplace.name == incoming.name) + else { + data.push(incoming); + return; + }; + + if existing.interface.is_none() { + existing.interface = incoming.interface; + } + if incoming.path.is_some() { + existing.path = incoming.path.clone(); + } + + let mut seen_plugin_ids = existing + .plugins + .iter() + .map(|plugin| plugin.id.clone()) + .collect::>(); + existing.plugins.extend( + incoming + .plugins + .into_iter() + .filter(|plugin| seen_plugin_ids.insert(plugin.id.clone())), + ); +} + fn remote_plugin_share_discoverability( discoverability: PluginShareDiscoverability, ) -> codex_core_plugins::remote::RemotePluginShareDiscoverability { @@ -674,12 +748,12 @@ impl PluginRequestProcessor { let plugins_input = config.plugins_config_input(); - let config_for_marketplace_listing = plugins_input.clone(); - let plugins_manager_for_marketplace_listing = plugins_manager.clone(); + let config_for_installed_listing = plugins_input.clone(); + let plugins_manager_for_installed_listing = plugins_manager.clone(); let shared_plugin_ids_by_local_path = load_shared_plugin_ids_by_local_path(&config)?; - let (mut data, marketplace_load_errors) = match tokio::task::spawn_blocking(move || { - let outcome = plugins_manager_for_marketplace_listing - .list_marketplaces_for_config(&config_for_marketplace_listing, &roots)?; + let (mut data, mut marketplace_load_errors) = match tokio::task::spawn_blocking(move || { + let outcome = plugins_manager_for_installed_listing + .list_installed_plugins_for_config(&config_for_installed_listing); Ok::< ( Vec, @@ -690,47 +764,11 @@ impl PluginRequestProcessor { outcome .marketplaces .into_iter() - .filter_map(|marketplace| { - let plugins = marketplace - .plugins - .into_iter() - .filter(|plugin| { - plugin.installed - || install_suggestion_plugin_names.contains(&plugin.name) - }) - .map(|plugin| { - let share_context = share_context_for_source( - &plugin.source, - &shared_plugin_ids_by_local_path, - ); - PluginSummary { - id: plugin.id, - remote_plugin_id: None, - local_version: plugin.local_version, - installed: plugin.installed, - enabled: plugin.enabled, - name: plugin.name, - share_context, - source: marketplace_plugin_source_to_info(plugin.source), - install_policy: plugin.policy.installation.into(), - auth_policy: plugin.policy.authentication.into(), - availability: PluginAvailability::Available, - interface: plugin.interface.map(local_plugin_interface_to_info), - keywords: plugin.keywords, - } - }) - .collect::>(); - - (!plugins.is_empty()).then_some(PluginMarketplaceEntry { - name: marketplace.name, - path: Some(marketplace.path), - interface: marketplace.interface.map(|interface| { - MarketplaceInterface { - display_name: interface.display_name, - } - }), - plugins, - }) + .map(|marketplace| { + configured_marketplace_to_info( + marketplace, + &shared_plugin_ids_by_local_path, + ) }) .collect(), outcome @@ -754,11 +792,91 @@ impl PluginRequestProcessor { } Err(err) => { return Err(internal_error(format!( - "failed to list installed marketplace plugins: {err}" + "failed to list installed plugins: {err}" ))); } }; + if !install_suggestion_plugin_names.is_empty() { + let config_for_marketplace_listing = plugins_input.clone(); + let plugins_manager_for_marketplace_listing = plugins_manager.clone(); + let shared_plugin_ids_by_local_path_for_suggestions = + load_shared_plugin_ids_by_local_path(&config)?; + let (suggestion_data, suggestion_errors) = + match tokio::task::spawn_blocking(move || { + let outcome = plugins_manager_for_marketplace_listing + .list_marketplaces_for_config(&config_for_marketplace_listing, &roots)?; + Ok::< + ( + Vec, + Vec, + ), + MarketplaceError, + >(( + outcome + .marketplaces + .into_iter() + .filter_map(|marketplace| { + let plugins = marketplace + .plugins + .into_iter() + .filter(|plugin| { + !plugin.installed + && install_suggestion_plugin_names + .contains(&plugin.name) + }) + .map(|plugin| { + configured_marketplace_plugin_to_info( + plugin, + &shared_plugin_ids_by_local_path_for_suggestions, + ) + }) + .collect::>(); + + (!plugins.is_empty()).then_some(PluginMarketplaceEntry { + name: marketplace.name, + path: Some(marketplace.path), + interface: marketplace.interface.map(|interface| { + MarketplaceInterface { + display_name: interface.display_name, + } + }), + plugins, + }) + }) + .collect(), + outcome + .errors + .into_iter() + .map(|err| codex_app_server_protocol::MarketplaceLoadErrorInfo { + marketplace_path: err.path, + message: err.message, + }) + .collect(), + )) + }) + .await + { + Ok(Ok(outcome)) => outcome, + Ok(Err(err)) => { + return Err(Self::marketplace_error( + err, + "list plugin install suggestions", + )); + } + Err(err) => { + return Err(internal_error(format!( + "failed to list plugin install suggestions: {err}" + ))); + } + }; + + for marketplace in suggestion_data { + merge_plugin_marketplace_entry(&mut data, marketplace); + } + marketplace_load_errors.extend(suggestion_errors); + } + if config.features.enabled(Feature::RemotePlugin) { let remote_marketplaces = if let Some(remote_marketplaces) = plugins_manager.cached_remote_installed_marketplaces() @@ -768,11 +886,12 @@ impl PluginRequestProcessor { let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), }; - codex_core_plugins::remote::fetch_remote_installed_marketplaces( - &remote_plugin_service_config, - auth.as_ref(), - ) - .await + plugins_manager + .fetch_cached_remote_installed_marketplaces( + &remote_plugin_service_config, + auth.as_ref(), + ) + .await }; match remote_marketplaces { @@ -781,14 +900,7 @@ impl PluginRequestProcessor { .into_iter() .map(remote_marketplace_to_info) { - if let Some(existing) = data - .iter_mut() - .find(|marketplace| marketplace.name == remote_marketplace.name) - { - *existing = remote_marketplace; - } else { - data.push(remote_marketplace); - } + merge_plugin_marketplace_entry(&mut data, remote_marketplace); } } Err( diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index 0339ae7563..d8a4db3cfa 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -180,6 +180,51 @@ enabled = true Ok(()) } +#[tokio::test] +async fn plugin_installed_reads_local_cache_without_catalog() -> Result<()> { + let codex_home = TempDir::new()?; + write_installed_plugin(&codex_home, "openai-curated", "linear")?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true + +[plugins."linear@openai-curated"] +enabled = true +"#, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_plugin_installed_request(PluginInstalledParams { + cwds: None, + install_suggestion_plugin_names: None, + }) + .await?; + + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstalledResponse = to_response(response)?; + + assert_eq!(response.marketplaces.len(), 1); + assert_eq!(response.marketplaces[0].name, "openai-curated"); + assert_eq!( + response.marketplaces[0] + .plugins + .iter() + .map(|plugin| (plugin.id.clone(), plugin.installed, plugin.enabled)) + .collect::>(), + vec![("linear@openai-curated".to_string(), true, true)] + ); + assert_eq!(response.marketplace_load_errors, Vec::new()); + Ok(()) +} + #[tokio::test] async fn plugin_list_rejects_relative_cwds() -> Result<()> { let codex_home = TempDir::new()?; @@ -1766,8 +1811,7 @@ async fn plugin_list_does_not_append_global_remote_when_marketplace_kinds_are_ex } #[tokio::test] -async fn plugin_installed_fetches_remote_installed_rows_without_remote_catalog_list() -> Result<()> -{ +async fn plugin_installed_skips_remote_installed_rows_without_cached_bundle() -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; write_remote_plugin_catalog_config( @@ -1810,28 +1854,7 @@ async fn plugin_installed_fetches_remote_installed_rows_without_remote_catalog_l .await??; let response: PluginInstalledResponse = to_response(response)?; - assert_eq!(response.marketplaces.len(), 1); - assert_eq!(response.marketplaces[0].name, "chatgpt-global"); - assert_eq!( - response.marketplaces[0] - .plugins - .iter() - .map(|plugin| { - ( - plugin.id.clone(), - plugin.remote_plugin_id.clone(), - plugin.installed, - plugin.enabled, - ) - }) - .collect::>(), - vec![( - "linear@chatgpt-global".to_string(), - Some("plugins~Plugin_00000000000000000000000000000000".to_string()), - true, - true, - )] - ); + assert_eq!(response.marketplaces, Vec::new()); wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?; Ok(()) } diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index f82caa8edf..6c9589daaf 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -23,6 +23,7 @@ use crate::marketplace::MarketplaceError; use crate::marketplace::MarketplaceInterface; use crate::marketplace::MarketplaceListError; use crate::marketplace::MarketplacePluginAuthPolicy; +use crate::marketplace::MarketplacePluginInstallPolicy; use crate::marketplace::MarketplacePluginPolicy; use crate::marketplace::MarketplacePluginSource; use crate::marketplace::ResolvedMarketplacePlugin; @@ -67,6 +68,7 @@ use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginSkillRoot; +use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; @@ -611,6 +613,18 @@ impl PluginsManager { )) } + pub async fn fetch_cached_remote_installed_marketplaces( + &self, + config: &RemotePluginServiceConfig, + auth: Option<&CodexAuth>, + ) -> Result, RemotePluginCatalogError> { + let plugins = crate::remote::fetch_remote_installed_plugins(config, auth).await?; + Ok(crate::remote::cached_remote_installed_marketplaces( + &plugins, + &self.store, + )) + } + fn write_remote_installed_plugins_cache(&self, plugins: Vec) -> bool { let mut cache = match self.remote_installed_plugins_cache.write() { Ok(cache) => cache, @@ -1258,6 +1272,68 @@ impl PluginsManager { }) } + pub fn list_installed_plugins_for_config( + &self, + config: &PluginsConfigInput, + ) -> ConfiguredMarketplaceListOutcome { + if !config.plugins_enabled { + return ConfiguredMarketplaceListOutcome::default(); + } + + let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let mut plugins_by_marketplace = + BTreeMap::>::new(); + + for (plugin_key, plugin_config) in configured_plugins { + let Ok(plugin_id) = PluginId::parse(&plugin_key) else { + continue; + }; + let Some(plugin_root) = self.store.active_plugin_root(&plugin_id) else { + continue; + }; + let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { + continue; + }; + + plugins_by_marketplace + .entry(plugin_id.marketplace_name.clone()) + .or_default() + .push(ConfiguredMarketplacePlugin { + id: plugin_id.as_key(), + name: plugin_id.plugin_name, + local_version: manifest.version, + source: MarketplacePluginSource::Local { path: plugin_root }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: manifest.interface, + keywords: manifest.keywords, + installed: true, + enabled: plugin_config.enabled, + }); + } + + let marketplaces = plugins_by_marketplace + .into_iter() + .map(|(marketplace_name, mut plugins)| { + plugins.sort_by(|left, right| left.id.cmp(&right.id)); + ConfiguredMarketplace { + path: self.store.root().join(&marketplace_name), + name: marketplace_name, + interface: None, + plugins, + } + }) + .collect(); + + ConfiguredMarketplaceListOutcome { + marketplaces, + errors: Vec::new(), + } + } + pub async fn read_plugin_for_config( &self, config: &PluginsConfigInput,