From 26fadfdff3ac77c84b3af057036ff6fc8c8511d3 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 4 Jun 2026 22:34:21 -0700 Subject: [PATCH] Respect remote plugin visibility scopes Fixes #26275 --- .../src/request_processors/plugins.rs | 13 +- .../app-server/tests/suite/v2/plugin_list.rs | 8 -- codex-rs/core-plugins/src/loader.rs | 4 + codex-rs/core-plugins/src/manager.rs | 103 +++++++++++++--- codex-rs/core-plugins/src/manager_tests.rs | 113 +++++++++++++++--- codex-rs/core-plugins/src/remote.rs | 35 +++--- .../remote/remote_installed_plugin_sync.rs | 106 ++++++++-------- codex-rs/core-plugins/src/test_support.rs | 5 + codex-rs/core/src/config/mod.rs | 1 + .../core/src/plugins/discoverable_tests.rs | 26 ++-- 10 files changed, 275 insertions(+), 139 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 79791c142a..6cdd8f7170 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -148,17 +148,6 @@ fn convert_configured_marketplace_plugin_to_plugin_summary( } } -fn remote_installed_plugin_visible_scopes(config: &Config) -> Vec { - let mut scopes = Vec::new(); - if config.features.enabled(Feature::RemotePlugin) { - scopes.push(RemotePluginScope::Global); - } - if config.features.enabled(Feature::PluginSharing) { - scopes.push(RemotePluginScope::Workspace); - } - scopes -} - fn filter_openai_curated_installed_conflicts( marketplaces: &mut Vec, prefer_remote_curated_conflicts: bool, @@ -764,7 +753,7 @@ impl PluginRequestProcessor { let plugins_input = config.plugins_config_input(); let remote_installed_plugin_visible_scopes = - remote_installed_plugin_visible_scopes(&config); + plugins_input.remote_installed_plugin_visible_scopes(); plugins_manager.maybe_start_remote_installed_plugin_bundle_sync( &plugins_input, auth.clone(), 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 ae622db8a8..94a3e4a36a 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -2305,8 +2305,6 @@ plugin_sharing = true .expect("installed plugins should be an array") .push(unlisted_installed_body["plugins"][0].clone()); let workspace_installed_body = serde_json::to_string(&workspace_installed_body)?; - let global_installed_body = remote_installed_plugin_body("", "1.2.3", /*enabled*/ true); - mount_remote_installed_plugins(&server, "GLOBAL", &global_installed_body).await; mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await; let mut mcp = TestAppServer::new(codex_home.path()).await?; @@ -2356,7 +2354,6 @@ plugin_sharing = true ] ); wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; - wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; Ok(()) } @@ -2395,8 +2392,6 @@ plugin_sharing = false let global_installed_body = remote_installed_plugin_body(&bundle_url, "1.2.3", /*enabled*/ true); mount_remote_installed_plugins(&server, "GLOBAL", &global_installed_body).await; - mount_remote_installed_plugins(&server, "WORKSPACE", empty_remote_installed_plugins_body()) - .await; let mut mcp = TestAppServer::new_with_env( codex_home.path(), @@ -2434,7 +2429,6 @@ plugin_sharing = false .join("plugins/cache/openai-curated-remote/linear/1.2.3/.codex-plugin/plugin.json"); wait_for_path_exists(&installed_path).await?; wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; - wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; Ok(()) } @@ -2586,7 +2580,6 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> { .push(unlisted_installed_body["plugins"][0].clone()); let workspace_installed_body = serde_json::to_string(&workspace_installed_body)?; mount_shared_workspace_plugins(&server, &shared_plugin_body).await; - mount_remote_installed_plugins(&server, "GLOBAL", empty_remote_installed_plugins_body()).await; mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await; let mut mcp = TestAppServer::new(codex_home.path()).await?; @@ -2716,7 +2709,6 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> { Some(PluginShareDiscoverability::Unlisted) ); wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; - wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?; Ok(()) } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index d6414be6ff..e2b1680fe1 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -223,6 +223,10 @@ fn merge_configured_plugins_with_remote_installed( .collect::>(); for (plugin_key, plugin_config) in extra_plugins { + if configured_plugins.contains_key(&plugin_key) { + continue; + } + let remote_curated_plugin_name = installed_plugin_name_for_marketplace( &plugin_key, REMOTE_GLOBAL_MARKETPLACE_NAME, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 08f668d130..a8d047fe11 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -92,6 +92,7 @@ pub struct PluginsConfigInput { pub config_layer_stack: ConfigLayerStack, pub plugins_enabled: bool, pub remote_plugin_enabled: bool, + pub plugin_sharing_enabled: bool, pub chatgpt_base_url: String, } @@ -100,15 +101,28 @@ impl PluginsConfigInput { config_layer_stack: ConfigLayerStack, plugins_enabled: bool, remote_plugin_enabled: bool, + plugin_sharing_enabled: bool, chatgpt_base_url: String, ) -> Self { Self { config_layer_stack, plugins_enabled, remote_plugin_enabled, + plugin_sharing_enabled, chatgpt_base_url, } } + + pub fn remote_installed_plugin_visible_scopes(&self) -> Vec { + let mut scopes = Vec::new(); + if self.remote_plugin_enabled { + scopes.push(RemotePluginScope::Global); + } + if self.plugin_sharing_enabled { + scopes.push(RemotePluginScope::Workspace); + } + scopes + } } #[derive(Clone, PartialEq, Eq)] @@ -129,6 +143,7 @@ struct CachedFeaturedPluginIds { struct RemoteInstalledPluginsCacheRefreshRequest { service_config: RemotePluginServiceConfig, auth: Option, + scopes: Vec, notify: RemoteInstalledPluginsCacheRefreshNotify, // App-server attaches side effects such as skills metadata invalidation and MCP refreshes when // remote installed state changes. @@ -404,7 +419,7 @@ pub struct PluginsManager { configured_marketplace_upgrade_state: RwLock, non_curated_cache_refresh_state: RwLock, cached_enabled_outcome: RwLock>, - remote_installed_plugins_cache: RwLock>>, + remote_installed_plugins_cache: RwLock>, remote_installed_plugins_cache_refresh_state: RwLock, remote_sync_lock: Semaphore, restriction_product: Option, @@ -417,6 +432,12 @@ struct CachedPluginLoadOutcome { outcome: PluginLoadOutcome, } +#[derive(PartialEq)] +struct CachedRemoteInstalledPlugins { + scopes: Vec, + plugins: Vec, +} + impl PluginsManager { pub fn new(codex_home: PathBuf) -> Self { Self::new_with_restriction_product(codex_home, Some(Product::Codex)) @@ -491,7 +512,7 @@ impl PluginsManager { let outcome = load_plugins_from_layer_stack( &config.config_layer_stack, - self.remote_installed_plugin_configs(), + self.remote_installed_plugin_configs(config), &self.store, self.restriction_product, config.remote_plugin_enabled, @@ -537,7 +558,7 @@ impl PluginsManager { } load_plugins_from_layer_stack( config_layer_stack, - self.remote_installed_plugin_configs(), + self.remote_installed_plugin_configs(config), &self.store, self.restriction_product, config.remote_plugin_enabled, @@ -556,7 +577,7 @@ impl PluginsManager { } load_plugin_hooks_from_layer_stack( config_layer_stack, - self.remote_installed_plugin_configs(), + self.remote_installed_plugin_configs(config), &self.store, config.remote_plugin_enabled, ) @@ -588,16 +609,32 @@ impl PluginsManager { } } - fn remote_installed_plugin_configs(&self) -> HashMap { + fn remote_installed_plugin_configs( + &self, + config: &PluginsConfigInput, + ) -> HashMap { + let visible_scopes = config.remote_installed_plugin_visible_scopes(); + if visible_scopes.is_empty() { + return HashMap::new(); + } let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - let Some(plugins) = cache.as_ref() else { + let Some(cache) = cache.as_ref() else { return HashMap::new(); }; - remote_installed_plugins_to_config(plugins, &self.store) + let plugins = cache + .plugins + .iter() + .filter(|plugin| { + RemotePluginScope::from_marketplace_name(&plugin.marketplace_name) + .is_some_and(|scope| visible_scopes.contains(&scope)) + }) + .cloned() + .collect::>(); + remote_installed_plugins_to_config(&plugins, &self.store) } pub fn build_remote_installed_plugin_marketplaces_from_cache( @@ -608,8 +645,19 @@ impl PluginsManager { Ok(cache) => cache, Err(err) => err.into_inner(), }; - let plugins = cache.as_ref()?; - Some(crate::remote::group_remote_installed_plugins_by_marketplaces(plugins, visible_scopes)) + let cache = cache.as_ref()?; + if !visible_scopes + .iter() + .all(|scope| cache.scopes.contains(scope)) + { + return None; + } + Some( + crate::remote::group_remote_installed_plugins_by_marketplaces( + &cache.plugins, + visible_scopes, + ), + ) } pub fn cached_global_remote_discoverable_plugins_for_config( @@ -644,29 +692,42 @@ impl PluginsManager { visible_scopes: &[RemotePluginScope], on_effective_plugins_changed: Option>, ) -> Result, RemotePluginCatalogError> { - let plugins = crate::remote::fetch_remote_installed_plugins( + if visible_scopes.is_empty() { + return Ok(Vec::new()); + } + + let plugins = crate::remote::fetch_remote_installed_plugins_for_scopes( &remote_plugin_service_config(config), auth, + visible_scopes, ) .await?; let marketplaces = crate::remote::group_remote_installed_plugins_by_marketplaces(&plugins, visible_scopes); - let changed = self.write_remote_installed_plugins_cache(plugins); + let changed = self.write_remote_installed_plugins_cache(visible_scopes, plugins); if changed && let Some(on_effective_plugins_changed) = on_effective_plugins_changed { on_effective_plugins_changed(); } Ok(marketplaces) } - fn write_remote_installed_plugins_cache(&self, plugins: Vec) -> bool { + fn write_remote_installed_plugins_cache( + &self, + scopes: &[RemotePluginScope], + plugins: Vec, + ) -> bool { + let mut scopes = scopes.to_vec(); + scopes.sort_unstable(); + scopes.dedup(); + let cached_plugins = CachedRemoteInstalledPlugins { scopes, plugins }; let mut cache = match self.remote_installed_plugins_cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - if cache.as_ref().is_some_and(|cache| cache.eq(&plugins)) { + if cache.as_ref().is_some_and(|cache| cache == &cached_plugins) { return false; } - *cache = Some(plugins); + *cache = Some(cached_plugins); drop(cache); self.clear_enabled_outcome_cache(); true @@ -721,7 +782,8 @@ impl PluginsManager { notify: RemoteInstalledPluginsCacheRefreshNotify, on_effective_plugins_changed: Option>, ) { - if !config.plugins_enabled { + let scopes = config.remote_installed_plugin_visible_scopes(); + if !config.plugins_enabled || scopes.is_empty() { return; } @@ -729,6 +791,7 @@ impl PluginsManager { RemoteInstalledPluginsCacheRefreshRequest { service_config: remote_plugin_service_config(config), auth, + scopes, notify, on_effective_plugins_changed, }, @@ -741,7 +804,8 @@ impl PluginsManager { auth: Option, on_effective_plugins_changed: Option>, ) { - if !config.plugins_enabled { + let scopes = config.remote_installed_plugin_visible_scopes(); + if !config.plugins_enabled || scopes.is_empty() { return; } @@ -760,6 +824,7 @@ impl PluginsManager { self.codex_home.clone(), remote_plugin_service_config(config), auth, + scopes, Some(on_local_cache_changed), ); } @@ -1856,16 +1921,18 @@ impl PluginsManager { } }; - let installed_plugins = crate::remote::fetch_remote_installed_plugins( + let installed_plugins = crate::remote::fetch_remote_installed_plugins_for_scopes( &request.service_config, request.auth.as_ref(), + &request.scopes, ) .await; match installed_plugins { Ok(installed_plugins) => { // TODO(remote plugins): reconcile missing or stale local bundles before // publishing remote installed state as effective local plugin config. - let changed = self.write_remote_installed_plugins_cache(installed_plugins); + let changed = self + .write_remote_installed_plugins_cache(&request.scopes, installed_plugins); let should_notify = changed || matches!( request.notify, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index d6c13b1fe0..711ad498be 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -137,8 +137,15 @@ fn remote_installed_linear_plugin() -> RemoteInstalledPlugin { } fn remote_installed_plugin(name: &str) -> RemoteInstalledPlugin { + remote_installed_plugin_in_marketplace(name, "openai-curated-remote") +} + +fn remote_installed_plugin_in_marketplace( + name: &str, + marketplace_name: &str, +) -> RemoteInstalledPlugin { RemoteInstalledPlugin { - marketplace_name: "openai-curated-remote".to_string(), + marketplace_name: marketplace_name.to_string(), id: format!("plugins~Plugin_{name}"), name: name.to_string(), enabled: true, @@ -362,14 +369,17 @@ remote_plugin = true let config = load_config(codex_home.path(), codex_home.path()).await; let manager = PluginsManager::new(codex_home.path().to_path_buf()); - manager.write_remote_installed_plugins_cache(vec![remote_installed_linear_plugin()]); + manager.write_remote_installed_plugins_cache( + &[RemotePluginScope::Global], + vec![remote_installed_linear_plugin()], + ); let outcome = manager.plugins_for_config(&config).await; assert_eq!(outcome, PluginLoadOutcome::default()); } #[tokio::test] -async fn remote_installed_cache_prefers_local_curated_conflicts_when_remote_plugin_disabled() { +async fn remote_installed_cache_omits_global_remote_plugins_when_remote_plugin_disabled() { let codex_home = TempDir::new().unwrap(); write_file( &codex_home.path().join(CONFIG_TOML_FILE), @@ -391,10 +401,13 @@ enabled = true let config = load_config(codex_home.path(), codex_home.path()).await; let manager = PluginsManager::new(codex_home.path().to_path_buf()); - manager.write_remote_installed_plugins_cache(vec![ - remote_installed_plugin("linear"), - remote_installed_plugin("remote-only"), - ]); + manager.write_remote_installed_plugins_cache( + &[RemotePluginScope::Global], + vec![ + remote_installed_plugin("linear"), + remote_installed_plugin("remote-only"), + ], + ); let outcome = manager.plugins_for_config(&config).await; assert_eq!( @@ -406,11 +419,44 @@ enabled = true vec![ "calendar@openai-curated".to_string(), "linear@openai-curated".to_string(), - "remote-only@openai-curated-remote".to_string(), ] ); } +#[tokio::test] +async fn remote_installed_cache_includes_workspace_plugins_when_plugin_sharing_enabled() { + let codex_home = TempDir::new().unwrap(); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = false +plugin_sharing = true +"#, + ); + write_cached_plugin(codex_home.path(), "workspace-directory", "workspace-only"); + + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + manager.write_remote_installed_plugins_cache( + &[RemotePluginScope::Workspace], + vec![remote_installed_plugin_in_marketplace( + "workspace-only", + "workspace-directory", + )], + ); + + let outcome = manager.plugins_for_config(&config).await; + assert_eq!( + outcome + .plugins() + .iter() + .map(|plugin| plugin.config_name.clone()) + .collect::>(), + vec!["workspace-only@workspace-directory".to_string()] + ); +} + #[tokio::test] async fn remote_installed_cache_prefers_remote_curated_conflicts_when_remote_plugin_enabled() { let codex_home = TempDir::new().unwrap(); @@ -434,10 +480,13 @@ enabled = true let config = load_config(codex_home.path(), codex_home.path()).await; let manager = PluginsManager::new(codex_home.path().to_path_buf()); - manager.write_remote_installed_plugins_cache(vec![ - remote_installed_plugin("linear"), - remote_installed_plugin("remote-only"), - ]); + manager.write_remote_installed_plugins_cache( + &[RemotePluginScope::Global], + vec![ + remote_installed_plugin("linear"), + remote_installed_plugin("remote-only"), + ], + ); let outcome = manager.plugins_for_config(&config).await; assert_eq!( @@ -454,6 +503,39 @@ enabled = true ); } +#[tokio::test] +async fn remote_installed_cache_respects_local_disabled_remote_plugin_config() { + let codex_home = TempDir::new().unwrap(); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true + +[plugins."remote-only@openai-curated-remote"] +enabled = false +"#, + ); + write_cached_plugin(codex_home.path(), "openai-curated-remote", "remote-only"); + + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + manager.write_remote_installed_plugins_cache( + &[RemotePluginScope::Global], + vec![remote_installed_plugin("remote-only")], + ); + + let outcome = manager.plugins_for_config(&config).await; + assert_eq!( + outcome + .plugins() + .iter() + .map(|plugin| (plugin.config_name.clone(), plugin.enabled)) + .collect::>(), + vec![("remote-only@openai-curated-remote".to_string(), false)] + ); +} + #[tokio::test] async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metadata() { let codex_home = TempDir::new().unwrap(); @@ -481,7 +563,7 @@ async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metad screenshot_urls: Vec::new(), }); plugin.keywords = vec!["issues".to_string()]; - manager.write_remote_installed_plugins_cache(vec![plugin]); + manager.write_remote_installed_plugins_cache(&[RemotePluginScope::Global], vec![plugin]); let marketplaces = manager .build_remote_installed_plugin_marketplaces_from_cache(&[RemotePluginScope::Global]) @@ -521,9 +603,8 @@ async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metad ); assert_eq!( manager - .build_remote_installed_plugin_marketplaces_from_cache(&[RemotePluginScope::Workspace]) - .expect("remote installed cache should be present"), - Vec::new() + .build_remote_installed_plugin_marketplaces_from_cache(&[RemotePluginScope::Workspace]), + None ); } diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 7daf1da31f..4456e67932 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -360,7 +360,7 @@ impl RemotePluginScope { } } - fn from_marketplace_name(name: &str) -> Option { + pub(crate) fn from_marketplace_name(name: &str) -> Option { match name { REMOTE_GLOBAL_MARKETPLACE_NAME => Some(Self::Global), REMOTE_WORKSPACE_MARKETPLACE_NAME @@ -781,34 +781,29 @@ fn build_remote_marketplace( })) } -pub(crate) async fn fetch_remote_installed_plugins( +pub(crate) async fn fetch_remote_installed_plugins_for_scopes( config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, + scopes: &[RemotePluginScope], ) -> Result, RemotePluginCatalogError> { let auth = ensure_chatgpt_auth(auth)?; - let global = async { - let scope = RemotePluginScope::Global; - let installed_plugins = fetch_installed_plugins_for_scope(config, auth, scope).await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let workspace = async { - let scope = RemotePluginScope::Workspace; - let installed_plugins = fetch_installed_plugins_for_scope(config, auth, scope).await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; + let mut scopes = scopes.to_vec(); + scopes.sort_unstable(); + scopes.dedup(); - let (global, workspace) = tokio::try_join!(global, workspace)?; - let mut installed_plugins = [global, workspace] - .into_iter() - .flat_map(|(_scope, plugins)| plugins) - .map(|plugin| remote_installed_plugin_to_cache_entry(&plugin)) - .collect::, _>>()?; - installed_plugins.sort_by(|left, right| { + let mut remote_installed_plugins = Vec::new(); + for scope in scopes { + let installed_items = fetch_installed_plugins_for_scope(config, auth, scope).await?; + for plugin in installed_items { + remote_installed_plugins.push(remote_installed_plugin_to_cache_entry(&plugin)?); + } + } + remote_installed_plugins.sort_by(|left, right| { left.marketplace_name .cmp(&right.marketplace_name) .then_with(|| left.id.cmp(&right.id)) }); - Ok(installed_plugins) + Ok(remote_installed_plugins) } pub fn group_remote_installed_plugins_by_marketplaces( diff --git a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs index fe3a597b0f..1ff02dc9f5 100644 --- a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs +++ b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs @@ -82,11 +82,15 @@ pub(crate) fn maybe_start_remote_installed_plugin_bundle_sync( codex_home: PathBuf, config: RemotePluginServiceConfig, auth: Option, + scopes: Vec, on_local_cache_changed: Option>, ) { let Some(auth) = auth else { return; }; + if scopes.is_empty() { + return; + } let key = RemoteInstalledPluginBundleSyncKey { plugin_cache_root: remote_plugin_cache_root(&codex_home), }; @@ -96,7 +100,8 @@ pub(crate) fn maybe_start_remote_installed_plugin_bundle_sync( tokio::spawn(async move { let result = - sync_remote_installed_plugin_bundles_once(codex_home, &config, Some(&auth)).await; + sync_remote_installed_plugin_bundles_once(codex_home, &config, Some(&auth), &scopes) + .await; match result { Ok(outcome) => { if outcome.changed_local_cache() @@ -126,51 +131,32 @@ pub async fn sync_remote_installed_plugin_bundles_once( codex_home: PathBuf, config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, + scopes: &[RemotePluginScope], ) -> Result { + if scopes.is_empty() { + return Ok(RemoteInstalledPluginBundleSyncOutcome::default()); + } let auth = ensure_chatgpt_auth(auth)?; - let global = async { - let scope = RemotePluginScope::Global; - let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ true, - ) - .await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let workspace = async { - let scope = RemotePluginScope::Workspace; - let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ true, - ) - .await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - - let (global, workspace) = tokio::try_join!(global, workspace)?; - let store = PluginStore::try_new(codex_home.clone())?; + let mut scopes = scopes.to_vec(); + scopes.sort_unstable(); + scopes.dedup(); let mut installed_plugin_names_by_marketplace = - BTreeMap::>::from_iter([ - (REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), BTreeSet::new()), - ( - REMOTE_WORKSPACE_MARKETPLACE_NAME.to_string(), - BTreeSet::new(), - ), - ( - REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME.to_string(), - BTreeSet::new(), - ), - ( - REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME.to_string(), - BTreeSet::new(), - ), - ( - REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME.to_string(), - BTreeSet::new(), - ), - ]); + installed_plugin_names_by_marketplace_for_scopes(scopes.iter().copied()); + + let mut installed_plugins_by_scope = Vec::new(); + for scope in scopes { + let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( + config, auth, scope, /*include_download_urls*/ true, + ) + .await?; + installed_plugins_by_scope.push(installed_plugins); + } + + let store = PluginStore::try_new(codex_home.clone())?; let mut installed_plugin_ids = BTreeSet::new(); let mut failed_remote_plugin_ids = BTreeSet::new(); - for (_scope, installed_plugins) in [global, workspace] { + for installed_plugins in installed_plugins_by_scope { for installed_plugin in installed_plugins { let plugin = installed_plugin.plugin; let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string(); @@ -306,21 +292,11 @@ fn remove_stale_remote_plugin_caches( installed_plugin_names_by_marketplace: &BTreeMap>, ) -> Result, String> { let mut removed_cache_plugin_ids = Vec::new(); - for marketplace_name in [ - REMOTE_GLOBAL_MARKETPLACE_NAME, - REMOTE_WORKSPACE_MARKETPLACE_NAME, - REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME, - REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME, - REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME, - ] { + for (marketplace_name, installed_plugin_names) in installed_plugin_names_by_marketplace { let marketplace_root = codex_home.join(PLUGINS_CACHE_DIR).join(marketplace_name); if !marketplace_root.exists() { continue; } - let installed_plugin_names = installed_plugin_names_by_marketplace - .get(marketplace_name) - .cloned() - .unwrap_or_default(); for entry in fs::read_dir(&marketplace_root).map_err(|err| { format!( "failed to read remote plugin cache directory {}: {err}", @@ -375,6 +351,34 @@ fn remove_stale_remote_plugin_caches( Ok(removed_cache_plugin_ids) } +fn installed_plugin_names_by_marketplace_for_scopes( + scopes: impl IntoIterator, +) -> BTreeMap> { + let mut installed_plugin_names_by_marketplace = BTreeMap::>::new(); + for scope in scopes { + match scope { + RemotePluginScope::Global => { + installed_plugin_names_by_marketplace + .entry(REMOTE_GLOBAL_MARKETPLACE_NAME.to_string()) + .or_default(); + } + RemotePluginScope::Workspace => { + for marketplace_name in [ + REMOTE_WORKSPACE_MARKETPLACE_NAME, + REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME, + REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME, + REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME, + ] { + installed_plugin_names_by_marketplace + .entry(marketplace_name.to_string()) + .or_default(); + } + } + } + } + installed_plugin_names_by_marketplace +} + fn remote_plugin_cache_root(codex_home: &Path) -> PathBuf { codex_home.join(PLUGINS_CACHE_DIR) } diff --git a/codex-rs/core-plugins/src/test_support.rs b/codex-rs/core-plugins/src/test_support.rs index 07f1fd4f26..3bc943d1b2 100644 --- a/codex-rs/core-plugins/src/test_support.rs +++ b/codex-rs/core-plugins/src/test_support.rs @@ -118,6 +118,11 @@ pub(crate) async fn load_plugins_config(codex_home: &Path, cwd: &Path) -> Plugin "remote_plugin", /*default_enabled*/ false, ), + feature_enabled( + &effective_config, + "plugin_sharing", + /*default_enabled*/ true, + ), "https://chatgpt.com/backend-api/".to_string(), ) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index b74f2feefe..523dbaed43 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1368,6 +1368,7 @@ impl Config { self.config_layer_stack.clone(), self.features.enabled(Feature::Plugins), self.features.enabled(Feature::RemotePlugin), + self.features.enabled(Feature::PluginSharing), self.chatgpt_base_url.clone(), ) } diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 992bf93f45..0eb76d7d87 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -344,20 +344,18 @@ remote_plugin = true .all(|plugin| plugin.id != "github@openai-curated-remote") ); - for scope in ["GLOBAL", "WORKSPACE"] { - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", scope)) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [], - "pagination": { - "next_page_token": null - } - }))) - .expect(1) - .mount(&server) - .await; - } + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [], + "pagination": { + "next_page_token": null + } + }))) + .expect(1) + .mount(&server) + .await; plugins_manager .build_and_cache_remote_installed_plugin_marketplaces( &config.plugins_config_input(),