From 9797296564fdeea774e4f5745dbeed6876654419 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 13 May 2026 20:38:30 -0700 Subject: [PATCH] Relax remote plugin sync gate (#22594) ## Summary - Allow remote installed-plugin cache refresh to start whenever plugins are enabled. - Allow remote installed-plugin bundle sync to start whenever plugins are enabled. - Remove the extra local `remote_plugin_enabled` guard from those background sync paths. ## Context Server-side installed plugin state and optional bundle URL behavior are owned by plugin-service `/public/plugins/installed`, so these local sync paths only need the overall plugin enablement gate. ## Test plan - `just fmt` - `cargo test -p codex-core-plugins` --- .../src/request_processors/plugins.rs | 8 ++- .../app-server/tests/suite/v2/plugin_list.rs | 9 +++- .../app-server/tests/suite/v2/plugin_share.rs | 51 ++++++++++++++++++ codex-rs/core-plugins/src/manager.rs | 53 ++++++++----------- codex-rs/core-plugins/src/manager_tests.rs | 3 +- 5 files changed, 88 insertions(+), 36 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 6980945ae..ea1ff3ead 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -450,14 +450,15 @@ impl PluginRequestProcessor { return Ok(empty_response()); } let plugins_input = config.plugins_config_input(); - let (mut data, marketplace_load_errors) = if include_local { + if include_local || marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe) { plugins_manager.maybe_start_plugin_list_background_tasks_for_config( &plugins_input, auth.clone(), &roots, Some(self.effective_plugins_changed_callback()), ); - + } + let (mut data, marketplace_load_errors) = if include_local { let config_for_marketplace_listing = plugins_input.clone(); let plugins_manager_for_marketplace_listing = plugins_manager.clone(); let shared_plugin_ids_by_local_path = load_shared_plugin_ids_by_local_path(&config)?; @@ -916,6 +917,9 @@ impl PluginRequestProcessor { params: PluginShareUpdateTargetsParams, ) -> Result { let (config, auth) = self.load_plugin_share_config_and_auth().await?; + if !config.features.enabled(Feature::PluginSharing) { + return Err(invalid_request("plugin sharing is disabled")); + } let PluginShareUpdateTargetsParams { remote_plugin_id, discoverability, 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 6fccdca9d..4ffa3893d 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -1360,7 +1360,7 @@ async fn app_server_startup_remote_plugin_sync_runs_once() -> Result<()> { async fn app_server_startup_sync_downloads_remote_installed_plugin_bundles() -> Result<()> { let codex_home = TempDir::new()?; let server = MockServer::start().await; - write_remote_plugin_catalog_config( + write_plugins_enabled_config_with_base_url( codex_home.path(), &format!("{}/backend-api/", server.uri()), )?; @@ -1858,6 +1858,7 @@ 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 = McpProcess::new(codex_home.path()).await?; @@ -1986,6 +1987,12 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> { share_context.discoverability, Some(PluginShareDiscoverability::Unlisted) ); + wait_for_remote_plugin_request_count( + &server, + "/ps/plugins/installed", + /*expected_count*/ 5, + ) + .await?; wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?; Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_share.rs b/codex-rs/app-server/tests/suite/v2/plugin_share.rs index 43f652c1e..69f93f0b7 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_share.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_share.rs @@ -1038,6 +1038,57 @@ async fn plugin_share_update_targets_updates_share_targets() -> Result<()> { Ok(()) } +#[tokio::test] +async fn plugin_share_update_targets_rejects_when_plugin_sharing_disabled() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#" +chatgpt_base_url = "{}/backend-api" + +[features] +plugins = true +remote_plugin = true +plugin_sharing = false +"#, + server.uri() + ), + )?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + let request_id = mcp + .send_raw_request( + "plugin/share/updateTargets", + Some(json!({ + "remotePluginId": "plugins_123", + "discoverability": "UNLISTED", + "shareTargets": [], + })), + ) + .await?; + + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(error.error.code, -32600); + assert_eq!(error.error.message, "plugin sharing is disabled"); + Ok(()) +} + #[tokio::test] async fn plugin_share_delete_removes_created_workspace_plugin() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 2df35b401..717aff24f 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -494,7 +494,7 @@ impl PluginsManager { let outcome = load_plugins_from_layer_stack( &config.config_layer_stack, - self.remote_installed_plugin_configs(config), + self.remote_installed_plugin_configs(), &self.store, self.restriction_product, plugin_hooks_enabled, @@ -542,7 +542,7 @@ impl PluginsManager { } load_plugins_from_layer_stack( config_layer_stack, - self.remote_installed_plugin_configs(config), + self.remote_installed_plugin_configs(), &self.store, self.restriction_product, plugin_hooks_feature_enabled, @@ -585,14 +585,7 @@ impl PluginsManager { } } - fn remote_installed_plugin_configs( - &self, - config: &PluginsConfigInput, - ) -> HashMap { - if !config.remote_plugin_enabled { - return HashMap::new(); - } - + fn remote_installed_plugin_configs(&self) -> HashMap { let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -667,7 +660,7 @@ impl PluginsManager { notify: RemoteInstalledPluginsCacheRefreshNotify, on_effective_plugins_changed: Option>, ) { - if !config.plugins_enabled || !config.remote_plugin_enabled { + if !config.plugins_enabled { return; } @@ -687,7 +680,7 @@ impl PluginsManager { auth: Option, on_effective_plugins_changed: Option>, ) { - if !config.plugins_enabled || !config.remote_plugin_enabled { + if !config.plugins_enabled { return; } @@ -1504,25 +1497,23 @@ impl PluginsManager { auth_manager.clone(), ); - if config.remote_plugin_enabled { - let config = config.clone(); - let manager = Arc::clone(self); - let auth_manager = auth_manager.clone(); - let on_effective_plugins_changed = on_effective_plugins_changed.clone(); - tokio::spawn(async move { - let auth = auth_manager.auth().await; - manager.maybe_start_remote_installed_plugins_cache_refresh( - &config, - auth.clone(), - on_effective_plugins_changed.clone(), - ); - manager.maybe_start_remote_installed_plugin_bundle_sync( - &config, - auth, - on_effective_plugins_changed, - ); - }); - } + let config_for_remote_sync = config.clone(); + let manager = Arc::clone(self); + let auth_manager_for_remote_sync = auth_manager.clone(); + let on_effective_plugins_changed = on_effective_plugins_changed.clone(); + tokio::spawn(async move { + let auth = auth_manager_for_remote_sync.auth().await; + manager.maybe_start_remote_installed_plugins_cache_refresh( + &config_for_remote_sync, + auth.clone(), + on_effective_plugins_changed.clone(), + ); + manager.maybe_start_remote_installed_plugin_bundle_sync( + &config_for_remote_sync, + auth, + on_effective_plugins_changed, + ); + }); let config = config.clone(); let manager = Arc::clone(self); diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index d04852ea1..e0b22c004 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -331,7 +331,7 @@ approval_mode = "approve" } #[tokio::test] -async fn remote_installed_cache_adds_plugin_skill_roots_without_marketplace_config() { +async fn remote_installed_cache_adds_plugin_skill_roots_without_remote_plugin_flag() { let codex_home = TempDir::new().unwrap(); let plugin_base = codex_home .path() @@ -341,7 +341,6 @@ async fn remote_installed_cache_adds_plugin_skill_roots_without_marketplace_conf &codex_home.path().join(CONFIG_TOML_FILE), r#"[features] plugins = true -remote_plugin = true "#, );