diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index bf5bae6b9f..47dc3d3b4b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -318,6 +318,17 @@ Plugin activation and MCP settings use the existing merged configuration, includ system settings and trusted project overrides. `skills/list` resolves plugin skills independently for each requested working directory. +Sites migration persists an account/backend-scoped list of excluded bundled plugin IDs, not +remote installed metadata. Once remote Sites is installed and locally loadable, the shared +marketplace and runtime loaders exclude `sites@openai-bundled`. The exclusion survives restarts; +normal remote refresh remains authoritative and clears it when remote Sites is unavailable. +The bundled files and preference remain available for account changes or a missing replacement. +Direct local reads and installs of excluded bundled Sites return the existing plugin-not-found error. +Plugin Service implicitly installs eligible Sites; migration does not call install, ensure, enable, +or disable. Remote enablement stays authoritative, including when bundled preferences differ. +A successful check that remote Sites is unavailable is throttled for 60 seconds. Catalog requests +then skip blocking bundle synchronization; normal background synchronization continues unchanged. + For local `plugin/list` and `plugin/installed` results, each requested cwd supplies its effective plugin state and plugin feature flag. When a plugin appears in multiple contexts, the first source wins and installed/enabled state is merged across contexts. diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 1e0e09fafe..1e3e41d954 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1486,7 +1486,7 @@ impl PluginRequestProcessor { }; let result = match plugins_manager - .install_plugin(&config.config_layer_stack, request) + .install_plugin(&config.plugins_config_input(), request) .await { Ok(result) => result, diff --git a/codex-rs/app-server/src/request_processors/plugins/local.rs b/codex-rs/app-server/src/request_processors/plugins/local.rs index 67cbd5c4bb..79b236b1e0 100644 --- a/codex-rs/app-server/src/request_processors/plugins/local.rs +++ b/codex-rs/app-server/src/request_processors/plugins/local.rs @@ -12,14 +12,30 @@ impl PluginRequestProcessor { &self, cwds: &[AbsolutePathBuf], ) -> Result { - if cwds.is_empty() { + let config = if cwds.is_empty() { self.config_manager .load_non_project_config() .await .map_err(|err| internal_error(format!("failed to reload config: {err}"))) } else { self.load_latest_config(/*fallback_cwd*/ None).await + }?; + // TODO(sites-migration): Remove this initial migration wait after bundled Sites is retired. + let auth = self.auth_manager.auth().await; + match Box::pin( + self.thread_manager + .plugins_manager() + .ensure_sites_migration_ready(&config.plugins_config_input(), auth.as_ref()), + ) + .await + { + Ok(Some(change)) => (self.effective_plugins_changed_callback())(change), + Ok(None) => {} + Err(err) => { + warn!(error = %err, "Sites migration refresh failed; preserving local plugins") + } } + Ok(config) } pub(super) async fn load_marketplace_context( 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 3f839369bb..69d19f3bd0 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -394,6 +394,72 @@ enabled = true Ok(()) } +#[tokio::test] +async fn plugin_installed_hides_bundled_sites_when_remote_sites_is_effective() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + write_curated_marketplace( + codex_home.path(), + "bundled_marketplace.json", + "openai-bundled", + Some("OpenAI Bundled"), + &["sites"], + )?; + write_installed_plugin(&codex_home, "openai-bundled", "sites")?; + write_installed_plugin(&codex_home, "openai-curated-remote", "sites")?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#"chatgpt_base_url = "{}/backend-api/" + +[features] +plugins = true +plugin_sharing = false + +[plugins."sites@openai-bundled"] +enabled = false +"#, + server.uri() + ), + )?; + write_remote_plugin_test_auth(codex_home.path())?; + + let mut remote_sites: serde_json::Value = serde_json::from_str(&remote_installed_plugin_body( + "", "1.2.3", /*enabled*/ false, + ))?; + remote_sites["plugins"][0]["id"] = + serde_json::json!("plugins~plugin_connector_1p_689987207de08191979cf68eca2941c6"); + remote_sites["plugins"][0]["name"] = serde_json::json!("sites"); + remote_sites["plugins"][0]["release"]["display_name"] = serde_json::json!("Sites"); + mount_remote_installed_plugins(&server, "GLOBAL", &serde_json::to_string(&remote_sites)?).await; + mount_remote_installed_plugins(&server, "WORKSPACE", empty_remote_installed_plugins_body()) + .await; + mount_empty_user_installed_plugins(&server).await; + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + let request_id = app_server + .send_plugin_installed_request(PluginInstalledParams { + cwds: None, + install_suggestion_plugin_names: None, + }) + .await?; + let response: PluginInstalledResponse = + timeout(DEFAULT_TIMEOUT, app_server.read_response(request_id)).await??; + + assert_eq!( + response + .marketplaces + .iter() + .flat_map(|marketplace| &marketplace.plugins) + .map(|plugin| (plugin.id.as_str(), plugin.enabled)) + .collect::>(), + vec![("sites@openai-curated-remote", false)] + ); + Ok(()) +} + #[tokio::test] async fn plugin_installed_prefers_api_curated_conflicts_after_switching_to_api_auth() -> Result<()> { @@ -1263,8 +1329,13 @@ async fn plugin_catalogs_skip_invalid_project_config_and_report_cwd_error() -> R )?; write_installed_plugin(&codex_home, "valid-marketplace", "sample")?; + let home = codex_home.path().to_string_lossy().into_owned(); let mut server = TestAppServer::builder() .with_codex_home(codex_home.path()) + .with_env_overrides(&[ + ("HOME", Some(home.as_str())), + ("USERPROFILE", Some(home.as_str())), + ]) .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; let invalid_cwd = AbsolutePathBuf::try_from(invalid_repo.as_path())?; @@ -4781,8 +4852,13 @@ remote_plugin = true let repo_cwd = AbsolutePathBuf::try_from(repo.path())?; let cwds = project_enables_plugins.then(|| vec![repo_cwd]); + let home = codex_home.path().to_string_lossy().into_owned(); let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) + .with_env_overrides(&[ + ("HOME", Some(home.as_str())), + ("USERPROFILE", Some(home.as_str())), + ]) .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; @@ -4866,8 +4942,13 @@ async fn plugin_list_omits_featured_plugin_ids_without_chatgpt_auth() -> Result< .mount(&server) .await; + let home = codex_home.path().to_string_lossy().into_owned(); let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) + .with_env_overrides(&[ + ("HOME", Some(home.as_str())), + ("USERPROFILE", Some(home.as_str())), + ]) .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/plugin_search.rs b/codex-rs/app-server/tests/suite/v2/plugin_search.rs index 1ca2dde2f2..aaa34607c9 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_search.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_search.rs @@ -421,6 +421,102 @@ async fn plugin_search_stitches_local_results_into_the_first_remote_page() -> Re Ok(()) } +#[tokio::test] +async fn plugin_search_hides_bundled_sites_when_remote_sites_is_effective() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let curated_root = codex_home.path().join(".tmp/plugins"); + write_local_marketplace( + &curated_root, + "openai-bundled", + "bundled_marketplace.json", + &[LocalPluginFixture { + name: "sites", + display_name: "Sites", + keywords: &[], + description: "Build and deploy websites", + }], + )?; + write_installed_plugin(codex_home.path(), "openai-bundled", "sites")?; + write_installed_plugin(codex_home.path(), "openai-curated-remote", "sites")?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#"chatgpt_base_url = "{}/backend-api/" + +[features] +plugins = true +remote_plugin = true + +[plugins."sites@openai-bundled"] +enabled = false +"#, + server.uri() + ), + )?; + write_chatgpt_search_auth(codex_home.path())?; + + let remote_sites_backend_id = "plugins~plugin_connector_1p_689987207de08191979cf68eca2941c6"; + let mut remote_sites = remote_plugin_json( + remote_sites_backend_id, + "sites", + "GLOBAL", + /*discoverability*/ None, + ); + remote_sites["release"]["version"] = json!("1.2.3"); + remote_sites["enabled"] = json!(true); + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [remote_sites], + "pagination": {"next_page_token": null}, + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/search")) + .and(query_param("q", "sites")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [remote_plugin_json( + remote_sites_backend_id, + "sites", + "GLOBAL", + /*discoverability*/ None, + )], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + let request_id = app_server + .send_plugin_search_request(PluginSearchParams { + search_term: "sites".to_string(), + scope: None, + cwds: None, + cursor: None, + limit: None, + }) + .await?; + let response: PluginSearchResponse = + timeout(DEFAULT_TIMEOUT, app_server.read_response(request_id)).await??; + + assert_eq!( + response + .data + .iter() + .map(|result| result.plugin.id.as_str()) + .collect::>(), + vec!["sites@openai-curated-remote"] + ); + Ok(()) +} + #[tokio::test] async fn plugin_search_returns_local_matches_for_api_key_auth() -> Result<()> { for remote_plugin_enabled in [false, true] { diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 73f8a1db8d..44b3a11f01 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -150,7 +150,7 @@ pub async fn run_plugin_add( )?; let outcome = manager .install_plugin( - &plugins_input.config_layer_stack, + &plugins_input, PluginInstallRequest { plugin_name, marketplace_path: marketplace.path, diff --git a/codex-rs/core-plugins/src/bundled_plugin_exclusions.rs b/codex-rs/core-plugins/src/bundled_plugin_exclusions.rs new file mode 100644 index 0000000000..3ad3b449d1 --- /dev/null +++ b/codex-rs/core-plugins/src/bundled_plugin_exclusions.rs @@ -0,0 +1,201 @@ +//! Account/backend-scoped exclusions for bundled plugins replaced by remote plugins. +//! Only plugin IDs are persisted; remote installation and enablement remain server-owned. +// TODO(sites-migration): Remove this module when bundled Sites is retired and rollback support ends. + +use super::*; +use serde::Deserialize; +use serde::Serialize; +use sha2::Digest; +use sha2::Sha256; + +const BUNDLED_SITES_PLUGIN_ID: &str = "sites@openai-bundled"; +const REMOTE_SITES_PLUGIN_ID: &str = "sites@openai-curated-remote"; + +fn is_remote_sites(plugin: &RemoteInstalledPlugin) -> bool { + plugin.name == "sites" + && plugin.marketplace_name == REMOTE_GLOBAL_MARKETPLACE_NAME + && plugin.id.strip_prefix("plugins~").unwrap_or(&plugin.id) + == "plugin_connector_1p_689987207de08191979cf68eca2941c6" +} + +#[derive(Default, Deserialize, Serialize)] +struct BundledPluginExclusions { + #[serde(rename = "disabled-bundled-plugin-ids")] + ids: BTreeSet, +} + +fn read_exclusions(path: &Path) -> BundledPluginExclusions { + std::fs::read(path) + .ok() + .and_then(|bytes| serde_json::from_slice(&bytes).ok()) + .unwrap_or_default() +} + +impl PluginsManager { + // Temporary direct-access guard using the same exclusion state as catalog/runtime loading. + pub(super) fn bundled_sites_is_hidden( + &self, + config: &PluginsConfigInput, + plugin_id: &str, + ) -> bool { + plugin_id == BUNDLED_SITES_PLUGIN_ID + && self.excluded_bundled_plugin_ids(config).contains(plugin_id) + } + + fn bundled_plugin_exclusion_path( + &self, + base_url: &str, + auth: Option<&CodexAuth>, + ) -> Option { + if self.restriction_product != Some(Product::Codex) { + return None; + } + let auth = auth.filter(|auth| auth.uses_codex_backend())?; + let account_id = auth.get_account_id().filter(|id| !id.is_empty())?; + let key = serde_json::to_vec(&( + base_url, + account_id, + auth.get_chatgpt_user_id(), + auth.is_workspace_account(), + )) + .ok()?; + let digest = Sha256::digest(key); + Some( + self.codex_home + .join("cache/bundled_plugin_exclusions") + .join(format!("{digest:x}.json")), + ) + } + + fn remote_sites_bundle_is_loadable(&self) -> bool { + PluginId::parse(REMOTE_SITES_PLUGIN_ID) + .ok() + .is_some_and(|id| self.store.active_plugin_root(&id).is_some()) + } + + pub(super) fn excluded_bundled_plugin_ids( + &self, + config: &PluginsConfigInput, + ) -> BTreeSet { + if !config.plugins_enabled || !self.remote_global_catalog_active(config) { + return BTreeSet::new(); + } + let auth = self.auth_manager.auth_cached(); + let Some(path) = + self.bundled_plugin_exclusion_path(&config.chatgpt_base_url, auth.as_ref()) + else { + return BTreeSet::new(); + }; + let mut exclusions = read_exclusions(&path).ids; + // Preserve the bundled fallback if the replacement's local files are missing. + if !self.remote_sites_bundle_is_loadable() { + exclusions.remove(BUNDLED_SITES_PLUGIN_ID); + } + exclusions + } + + // Called only after a current, successful installed-snapshot fetch. Failed requests never + // erase the last known exclusion; successful absence restores the bundled fallback. + pub(super) fn update_sites_exclusion( + &self, + base_url: &str, + auth: Option<&CodexAuth>, + plugins: &[RemoteInstalledPlugin], + ) -> bool { + let Some(path) = self.bundled_plugin_exclusion_path(base_url, auth) else { + return false; + }; + let mut exclusions = read_exclusions(&path); + let remote_sites_ready = + plugins.iter().any(is_remote_sites) && self.remote_sites_bundle_is_loadable(); + let changed = if remote_sites_ready { + exclusions.ids.insert(BUNDLED_SITES_PLUGIN_ID.to_string()) + } else { + exclusions.ids.remove(BUNDLED_SITES_PLUGIN_ID) + }; + if changed { + let result = serde_json::to_string(&exclusions) + .map_err(std::io::Error::other) + .and_then(|contents| codex_utils_path::write_atomically(&path, &contents)); + if let Err(err) = result { + warn!(error = %err, "failed to persist bundled Sites exclusion"); + } + } + changed + } + + fn sites_migration_needed( + &self, + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + ) -> bool { + if !config.plugins_enabled + || !self.remote_global_catalog_active(config) + || self + .excluded_bundled_plugin_ids(config) + .contains(BUNDLED_SITES_PLUGIN_ID) + { + return false; + } + let Some(path) = self.bundled_plugin_exclusion_path(&config.chatgpt_base_url, auth) else { + return false; + }; + !self + .sites_migration_checked_at + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_ref() + .is_some_and(|(checked_path, checked_at)| { + checked_path == &path && checked_at.elapsed() < Duration::from_secs(60) + }) + } + + /// Coordinates the first Sites migration at the shared inventory boundary. A persisted + /// exclusion skips migration on restart without restoring any remote installed metadata. + pub async fn ensure_sites_migration_ready( + &self, + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + ) -> Result, RemoteInstalledPluginBundleSyncError> { + if !self.sites_migration_needed(config, auth) { + return Ok(None); + } + // Sites is implicitly installed by Plugin Service. Never create an explicit install or + // overwrite account-wide preferences from bundled config; /installed owns both decisions. + let plugins = crate::remote::fetch_remote_installed_plugins( + &remote_plugin_service_config(config), + auth, + ) + .await?; + let remote_sites_available = plugins.iter().any(is_remote_sites); + // A rollout-off check must not wait behind unrelated startup bundle downloads. + let _guard = if remote_sites_available { + let guard = self.acquire_remote_installed_plugin_sync_guard().await?; + // Another catalog request or background sync may have completed while we waited. + if !self.sites_migration_needed(config, auth) { + return Ok(None); + } + Some(guard) + } else { + None + }; + *self + .sites_migration_checked_at + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = self + .bundled_plugin_exclusion_path(&config.chatgpt_base_url, auth) + .map(|path| (path, Instant::now())); + if !remote_sites_available { + return Ok(None); + } + let (outcome, changed) = + Box::pin(self.reconcile_remote_installed_plugins_after_acquiring_gate(config, auth)) + .await?; + // Preserve materialization details so the existing callback can trust eligible hooks. + Ok( + (changed || !outcome.changed_plugins.is_empty()).then_some(EffectivePluginsChange { + materialized_remote_plugins: outcome.materialized_remote_plugins, + }), + ) + } +} diff --git a/codex-rs/core-plugins/src/discoverable_tests.rs b/codex-rs/core-plugins/src/discoverable_tests.rs index 63538ce9ba..53c3d5dbf6 100644 --- a/codex-rs/core-plugins/src/discoverable_tests.rs +++ b/codex-rs/core-plugins/src/discoverable_tests.rs @@ -1081,7 +1081,7 @@ async fn install_marketplace_plugin(codex_home: &Path, marketplace_root: &Path, let config = load_plugins_config(codex_home, marketplace_root).await; test_plugins_manager(codex_home.to_path_buf()) .install_plugin( - &config.config_layer_stack, + &config, PluginInstallRequest { plugin_name: plugin_name.to_string(), marketplace_path: AbsolutePathBuf::try_from( diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 752795e668..1ccd60066d 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -53,6 +53,7 @@ use codex_utils_plugins::SkillDiscoveryMode; use codex_utils_plugins::find_plugin_manifest_path; use codex_utils_plugins::migrated_command_skills_root; use serde_json::Value as JsonValue; +use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; use std::fs; @@ -128,6 +129,8 @@ pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin]) } /// Load configured plugins without applying auth-dependent runtime policies. +// TODO(sites-migration): Remove the exclusion parameter and lint allowance when bundled Sites is retired. +#[allow(clippy::too_many_arguments)] #[instrument(level = "trace", skip_all)] pub(crate) async fn load_plugins_from_layer_stack( config_layer_stack: &ConfigLayerStack, @@ -137,6 +140,7 @@ pub(crate) async fn load_plugins_from_layer_stack( restriction_product: Option, remote_global_catalog_active: bool, skill_root_loader: &dyn SkillRootLoader, + excluded_plugin_ids: &BTreeSet, ) -> Vec> { let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); let RemoteInstalledPluginsSnapshot { @@ -146,6 +150,7 @@ pub(crate) async fn load_plugins_from_layer_stack( load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, + excluded_plugin_ids, store, remote_global_catalog_active, PluginLoadScope::AllCapabilities { @@ -162,16 +167,18 @@ pub(crate) async fn load_plugins_from_layer_stack( async fn load_plugins_from_layer_stack_with_scope( config_layer_stack: &ConfigLayerStack, extra_plugins: HashMap, + excluded_plugin_ids: &BTreeSet, store: &PluginStore, remote_global_catalog_active: bool, scope: PluginLoadScope<'_>, ) -> Vec> { - let configured_plugins = merge_configured_plugins_with_remote_installed( + let mut configured_plugins = merge_configured_plugins_with_remote_installed( configured_plugins_from_stack(config_layer_stack, store.codex_home().as_path()), extra_plugins, store, remote_global_catalog_active, ); + configured_plugins.retain(|id, _| !excluded_plugin_ids.contains(id)); let mut configured_plugins: Vec<_> = configured_plugins.into_iter().collect(); configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); @@ -201,6 +208,7 @@ async fn load_plugins_from_layer_stack_with_scope( pub async fn load_plugin_hooks_from_layer_stack( config_layer_stack: &ConfigLayerStack, extra_plugins: HashMap, + excluded_plugin_ids: &BTreeSet, store: &PluginStore, target_curated_marketplace: TargetCuratedMarketplace, remote_global_catalog_active: bool, @@ -208,6 +216,7 @@ pub async fn load_plugin_hooks_from_layer_stack( let mut plugins = load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, + excluded_plugin_ids, store, remote_global_catalog_active, PluginLoadScope::HooksOnly, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 11a80bd927..34f75548ea 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -284,6 +284,7 @@ async fn installed_agent_plugin_uses_isolated_data_root_for_stdio_mcp() { Some(Product::Codex), /*remote_global_catalog_active*/ false, test_skill_root_loader().as_ref(), + &BTreeSet::new(), ) .await; @@ -494,11 +495,13 @@ enabled = true Some(Product::Codex), /*remote_global_catalog_active*/ false, test_skill_root_loader().as_ref(), + &BTreeSet::new(), ) .await; let hooks_only = load_plugins_from_layer_stack_with_scope( &stack, HashMap::new(), + &BTreeSet::new(), &store, /*remote_global_catalog_active*/ false, PluginLoadScope::HooksOnly, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 108afa006e..4b2d0ff69f 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1,3 +1,5 @@ +#[path = "bundled_plugin_exclusions.rs"] +mod bundled_plugin_exclusions; #[path = "marketplace_context.rs"] mod marketplace_context; pub use marketplace_context::PluginMarketplaceContext; @@ -528,6 +530,8 @@ pub struct PluginsManager { skill_root_loader: Arc>, tool_suggest_metadata_cache: ToolSuggestMetadataCache, remote_installed_plugins_cache: RwLock, + // TODO(sites-migration): Remove this throttle together with the bundled Sites migration. + sites_migration_checked_at: Mutex>, remote_installed_plugin_bundle_sync_gate: Arc, remote_installed_plugins_cache_refresh_state: RwLock, remote_catalog_cache_refresh_state: RwLock, @@ -566,6 +570,8 @@ struct PluginLoadCacheKey { skill_config_rules: SkillConfigRules, remote_global_catalog_active: bool, auth_identity: Option, + // TODO(sites-migration): Remove once bundled Sites no longer needs scoped exclusions. + excluded_plugin_ids: BTreeSet, } impl PluginLoadCacheKey { @@ -574,6 +580,7 @@ impl PluginLoadCacheKey { codex_home: &Path, remote_global_catalog_active: bool, auth_identity: RemoteInstalledPluginsAuthIdentity, + excluded_plugin_ids: BTreeSet, ) -> Self { Self { configured_plugins: configured_plugins_from_stack( @@ -584,6 +591,7 @@ impl PluginLoadCacheKey { remote_global_catalog_active, // Local curated loads are auth-independent; only remote snapshots vary by account. auth_identity: remote_global_catalog_active.then_some(auth_identity), + excluded_plugin_ids, } } } @@ -645,6 +653,7 @@ impl PluginsManager { skill_root_loader, tool_suggest_metadata_cache: ToolSuggestMetadataCache::new(), remote_installed_plugins_cache: RwLock::new(RemoteInstalledPluginsCache::default()), + sites_migration_checked_at: Mutex::new(None), remote_installed_plugin_bundle_sync_gate, remote_installed_plugins_cache_refresh_state: RwLock::new( RemoteInstalledPluginsCacheRefreshState::default(), @@ -723,6 +732,7 @@ impl PluginsManager { self.codex_home.as_path(), self.remote_global_catalog_active(config), RemoteInstalledPluginsAuthIdentity::from_auth(auth.as_ref()), + self.excluded_bundled_plugin_ids(config), ); self.loaded_plugins_cache .lock() @@ -758,11 +768,12 @@ impl PluginsManager { let auth = self.auth_manager.auth_cached(); let auth_mode = auth.as_ref().map(CodexAuth::api_auth_mode); let auth_identity = RemoteInstalledPluginsAuthIdentity::from_auth(auth.as_ref()); - let cache_key = PluginLoadCacheKey::from_config( + let mut cache_key = PluginLoadCacheKey::from_config( config, self.codex_home.as_path(), remote_global_catalog_active, auth_identity.clone(), + self.excluded_bundled_plugin_ids(config), ); if let Some(plugins) = self.cached_loaded_plugins(&cache_key) { let outcome = self.resolve_loaded_plugins_for_auth(plugins, auth_mode); @@ -795,6 +806,8 @@ impl PluginsManager { } continue; } + // Migration may have changed the effective exclusion while this load was queued. + cache_key.excluded_plugin_ids = self.excluded_bundled_plugin_ids(config); if let Some(plugins) = self.cached_loaded_plugins(&cache_key) { let outcome = self.resolve_loaded_plugins_for_auth(plugins, auth_mode); if *auth_change_receiver.borrow() == auth_revision { @@ -818,6 +831,7 @@ impl PluginsManager { self.restriction_product, remote_global_catalog_active, self.skill_root_loader.as_ref(), + &cache_key.excluded_plugin_ids, ) .await; loaded_cache_metrics::record_duration( @@ -934,6 +948,7 @@ impl PluginsManager { load_plugin_hooks_from_layer_stack( config_layer_stack, self.remote_installed_plugin_configs(), + &self.excluded_bundled_plugin_ids(config), &self.store, target_curated_marketplace, self.remote_global_catalog_active(config), @@ -1212,6 +1227,7 @@ impl PluginsManager { generation, plugins, auth, + &config.chatgpt_base_url, RemoteInstalledPluginsCachePublication::Refresh, ) else { let auth_identity = RemoteInstalledPluginsAuthIdentity::from_auth(auth); @@ -1315,6 +1331,7 @@ impl PluginsManager { generation: u64, plugins: Vec, auth: Option<&CodexAuth>, + service_base_url: &str, publication: RemoteInstalledPluginsCachePublication, ) -> Option { let auth_identity = RemoteInstalledPluginsAuthIdentity::from_auth(auth); @@ -1347,8 +1364,11 @@ impl PluginsManager { cache.reconciliation_generation = None; cache.generation = cache.generation.wrapping_add(1); } - let needs_effective_plugins_refresh = - is_reconcile && std::mem::take(&mut cache.needs_effective_plugins_refresh); + // TODO(sites-migration): Remove exclusion publication once bundled Sites is retired. + let exclusion_changed = self.update_sites_exclusion(service_base_url, auth, &plugins); + let needs_effective_plugins_refresh = (is_reconcile + && std::mem::take(&mut cache.needs_effective_plugins_refresh)) + || exclusion_changed; if cache.plugins.as_ref() == Some(&plugins) { drop(cache); if needs_effective_plugins_refresh { @@ -1372,6 +1392,7 @@ impl PluginsManager { generation, plugins, auth.as_ref(), + "", RemoteInstalledPluginsCachePublication::Refresh, ) .expect("test cache generation should remain current") @@ -1656,6 +1677,7 @@ impl PluginsManager { generation, result.installed_plugins, auth, + &config.chatgpt_base_url, RemoteInstalledPluginsCachePublication::Reconcile, ) else { return Err(RemoteInstalledPluginBundleSyncError::Superseded); @@ -1956,10 +1978,10 @@ impl PluginsManager { pub async fn install_plugin( &self, - config_layer_stack: &ConfigLayerStack, + config: &PluginsConfigInput, request: PluginInstallRequest, ) -> Result { - let resolved = self.resolve_installable_plugin(config_layer_stack, &request)?; + let resolved = self.resolve_installable_plugin(config, &request)?; let plugin_id = resolved.plugin_id.clone(); match self.install_resolved_plugin(resolved).await { Ok(outcome) => Ok(outcome), @@ -1977,7 +1999,7 @@ impl PluginsManager { fn resolve_installable_plugin( &self, - config_layer_stack: &ConfigLayerStack, + config: &PluginsConfigInput, request: &PluginInstallRequest, ) -> Result { let resolved = match find_installable_marketplace_plugin( @@ -1991,6 +2013,15 @@ impl PluginsManager { return Err(err.into()); } }; + if self.bundled_sites_is_hidden(config, &resolved.plugin_id.as_key()) { + let err = MarketplaceError::PluginNotFound { + plugin_name: resolved.plugin_id.plugin_name, + marketplace_name: resolved.plugin_id.marketplace_name, + }; + self.track_plugin_install_resolution_failed(&err); + return Err(err.into()); + } + let config_layer_stack = &config.config_layer_stack; if let Err(message) = MarketplacePolicy::from_requirements(config_layer_stack.requirements()) .validate_install( @@ -2016,7 +2047,7 @@ impl PluginsManager { auth: Option<&CodexAuth>, request: PluginInstallRequest, ) -> Result { - let resolved = self.resolve_installable_plugin(&config.config_layer_stack, &request)?; + let resolved = self.resolve_installable_plugin(config, &request)?; let plugin_id = resolved.plugin_id.as_key(); // This only forwards the backend mutation before the local install flow. if let Err(err) = crate::remote_legacy::enable_remote_plugin( @@ -2303,6 +2334,7 @@ impl PluginsManager { include_openai_curated: bool, plugin_states: &ConfiguredPluginStates, ) -> Result { + let excluded_plugin_ids = self.excluded_bundled_plugin_ids(config); let marketplace_roots = self.marketplace_roots(config, additional_roots, include_openai_curated); let marketplace_outcome = self.list_marketplaces_with_policy(config, &marketplace_roots)?; @@ -2317,6 +2349,9 @@ impl PluginsManager { .into_iter() .filter_map(|plugin| { let plugin_key = format!("{}@{marketplace_name}", plugin.name); + if excluded_plugin_ids.contains(&plugin_key) { + return None; + } if !seen_plugin_keys.insert(plugin_key.clone()) { return None; } @@ -2430,6 +2465,12 @@ impl PluginsManager { } let plugin = find_marketplace_plugin(&request.marketplace_path, &request.plugin_name)?; + if self.bundled_sites_is_hidden(config, &plugin.plugin_id.as_key()) { + return Err(MarketplaceError::PluginNotFound { + plugin_name: plugin.plugin_id.plugin_name, + marketplace_name: plugin.plugin_id.marketplace_name, + }); + } MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()) .validate_install( &config.config_layer_stack, @@ -3239,6 +3280,7 @@ impl PluginsManager { request.generation, plugins, request.auth.as_ref(), + &request.service_config.chatgpt_base_url, RemoteInstalledPluginsCachePublication::Refresh, ) else { continue; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 70594e8704..767d7a1d97 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -72,6 +72,7 @@ use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; +use wiremock::matchers::query_param_is_missing; const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; @@ -82,6 +83,17 @@ fn unrestricted_config_layer_stack() -> ConfigLayerStack { ConfigLayerStack::default() } +fn unrestricted_plugins_config_input() -> PluginsConfigInput { + PluginsConfigInput::new( + unrestricted_config_layer_stack(), + String::new(), + /*plugins_enabled*/ true, + /*remote_plugin_enabled*/ false, + String::new(), + test_http_client_factory(), + ) +} + fn config_layer_stack_with_requirements( codex_home: &Path, user_config: &str, @@ -3105,6 +3117,7 @@ fn loaded_plugins_cache_evicts_least_recently_used_configuration() { skill_config_rules: SkillConfigRules::default(), remote_global_catalog_active: false, auth_identity: None, + excluded_plugin_ids: BTreeSet::new(), }) .collect::>(); let generation = manager.loaded_plugins_cache_generation(); @@ -3149,6 +3162,7 @@ fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() { skill_config_rules: SkillConfigRules::default(), remote_global_catalog_active: false, auth_identity: None, + excluded_plugin_ids: BTreeSet::new(), }; let stale_generation = manager.loaded_plugins_cache_generation(); @@ -3287,7 +3301,7 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() { let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "sample-plugin".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -3345,7 +3359,7 @@ source = "local" path = {marketplace_root:?} "# ); - let config = config_layer_stack_with_requirements(codex_home.path(), "", &requirements); + let config = plugins_config_input_with_requirements(codex_home.path(), "", &requirements); let marketplace_path = AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) .expect("absolute marketplace path"); @@ -3377,7 +3391,7 @@ source = {marketplace_root:?} ); write_file(&codex_home.path().join(CONFIG_TOML_FILE), &user_config); let config = - config_layer_stack_with_requirements(codex_home.path(), &user_config, &requirements); + plugins_config_input_with_requirements(codex_home.path(), &user_config, &requirements); let outcome = manager .install_plugin( &config, @@ -3403,7 +3417,7 @@ async fn install_openai_curated_plugin_uses_short_sha_cache_version() { let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "slack".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -3464,7 +3478,7 @@ async fn install_plugin_uses_manifest_version_for_non_curated_plugins() { let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "sample-plugin".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -3532,7 +3546,7 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "quality-review".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -3632,7 +3646,7 @@ async fn install_plugin_supports_git_subdir_marketplace_sources() { let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "toolkit".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -3686,7 +3700,7 @@ async fn install_plugin_supports_relative_git_subdir_marketplace_sources() { let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( - &unrestricted_config_layer_stack(), + &unrestricted_plugins_config_input(), PluginInstallRequest { plugin_name: "toolkit".to_string(), marketplace_path: AbsolutePathBuf::try_from( @@ -5095,6 +5109,13 @@ plugins = true marketplaces .iter() .map(|marketplace| marketplace.name.as_str()) + // Personal marketplaces do not affect which curated catalog auth selects. + .filter(|name| { + matches!( + *name, + OPENAI_CURATED_MARKETPLACE_NAME | OPENAI_API_CURATED_MARKETPLACE_NAME + ) + }) .collect::>(), vec![expected_marketplace], "unexpected curated catalog for provider `{resolved_provider}` with auth {auth_mode:?}" @@ -6923,6 +6944,7 @@ async fn load_plugins_uses_project_config_files() { Some(Product::Codex), /*remote_global_catalog_active*/ false, test_skill_root_loader().as_ref(), + &BTreeSet::new(), ) .await; @@ -7210,6 +7232,255 @@ remote_plugin = true server.verify().await; } +#[tokio::test] +async fn sites_migration_throttles_absence_without_blocking_normal_sync() { + let home = TempDir::new().unwrap(); + write_file( + &home.path().join(CONFIG_TOML_FILE), + "[features]\nplugins = true\nremote_plugin = true\n", + ); + let server = MockServer::start().await; + let mut config = load_config(home.path(), home.path()).await; + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); + let manager = test_plugins_manager_with_options( + home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + let auth = manager.auth_manager.auth_cached().unwrap(); + let installed_path = "/backend-api/ps/plugins/installed"; + let failed_check = Mock::given(method("GET")) + .and(path(installed_path)) + .respond_with(ResponseTemplate::new(503)) + .mount_as_scoped(&server) + .await; + assert!( + manager + .ensure_sites_migration_ready(&config, Some(&auth)) + .await + .is_err() + ); + drop(failed_check); + Mock::given(method("GET")) + .and(path(installed_path)) + .and(query_param_is_missing("includeDownloadUrls")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [], "pagination": {"next_page_token": null} + }))) + .expect(1) + .mount(&server) + .await; + // An unrelated startup download must not block a successful Sites-absence check. + let unrelated_sync = manager + .acquire_remote_installed_plugin_sync_guard() + .await + .unwrap(); + for _ in 0..2 { + assert!( + tokio::time::timeout( + Duration::from_secs(1), + manager.ensure_sites_migration_ready(&config, Some(&auth)), + ) + .await + .expect("Sites absence must not wait for unrelated bundle downloads") + .unwrap() + .is_none() + ); + } + drop(unrelated_sync); + Mock::given(method("GET")) + .and(path(installed_path)) + .and(query_param("includeDownloadUrls", "true")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [], "pagination": {"next_page_token": null} + }))) + .expect(1) + .mount(&server) + .await; + manager + .reconcile_remote_installed_plugins(&config, Some(&auth)) + .await + .unwrap(); +} + +#[tokio::test] +async fn sites_migration_persists_only_exclusion_without_repeating_rollout() { + let home = TempDir::new().unwrap(); + let marketplace_root = home.path().join(".tmp/bundled-marketplaces/openai-bundled"); + write_file( + &home.path().join(CONFIG_TOML_FILE), + &format!( + "[features]\nplugins = true\nremote_plugin = true\n[plugins.\"sites@openai-bundled\"]\nenabled = true\n[marketplaces.openai-bundled]\nsource_type = \"local\"\nsource = {marketplace_root:?}\n" + ), + ); + write_cached_plugin(home.path(), "openai-bundled", "sites"); + write_plugin(&marketplace_root, "sites", "sites"); + write_file( + &marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{"name":"openai-bundled","plugins":[{"name":"sites","source":{"source":"local","path":"./sites"}}]}"#, + ); + let server = MockServer::start().await; + let mut config = load_config(home.path(), home.path()).await; + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); + let auth_manager = test_auth_manager(Some(AuthMode::Chatgpt)); + let auth = auth_manager.auth_cached().unwrap(); + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [{ + "id": "plugins~plugin_connector_1p_689987207de08191979cf68eca2941c6", + "name": "sites", "scope": "GLOBAL", "enabled": false, + "installation_policy": "AVAILABLE", "authentication_policy": "ON_USE", + "release": {"version": "local", "display_name": "Sites", "description": "Sites", "interface": {}} + }], + "pagination": {"next_page_token": null} + }))) + .mount(&server).await; + let manager = test_plugins_manager_with_auth_manager( + home.path().to_path_buf(), + Some(Product::Codex), + auth_manager.clone(), + ); + let has_bundled_sites = |manager: &PluginsManager| { + manager + .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ false) + .unwrap() + .marketplaces + .into_iter() + .flat_map(|marketplace| marketplace.plugins) + .any(|plugin| plugin.id == "sites@openai-bundled") + }; + // An installed remote entry without local files must preserve the bundled runtime fallback. + manager + .reconcile_remote_installed_plugins(&config, Some(&auth)) + .await + .unwrap(); + assert!(has_bundled_sites(&manager)); + assert_eq!( + loaded_plugin_names(&manager, &config).await, + vec!["sites@openai-bundled"] + ); + write_cached_plugin(home.path(), REMOTE_GLOBAL_MARKETPLACE_NAME, "sites"); + let read_request = PluginReadRequest { + plugin_name: "sites".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + marketplace_root.join(".agents/plugins/marketplace.json"), + ) + .unwrap(), + }; + let install_request = PluginInstallRequest { + plugin_name: read_request.plugin_name.clone(), + marketplace_path: read_request.marketplace_path.clone(), + }; + let (first, concurrent) = tokio::join!( + manager.ensure_sites_migration_ready(&config, Some(&auth)), + manager.ensure_sites_migration_ready(&config, Some(&auth)), + ); + assert!(first.unwrap().is_some() ^ concurrent.unwrap().is_some()); + assert!(!manager.remote_installed_plugin_configs()["sites@openai-curated-remote"].enabled); + assert!( + server + .received_requests() + .await + .unwrap() + .iter() + .all(|request| request.method == "GET") + ); + let config_before_rejected_install = fs::read(home.path().join(CONFIG_TOML_FILE)).unwrap(); + assert!(matches!( + manager.read_plugin_for_config(&config, &read_request).await, + Err(MarketplaceError::PluginNotFound { plugin_name, marketplace_name }) + if plugin_name == "sites" && marketplace_name == "openai-bundled" + )); + assert!(matches!( + manager.install_plugin(&config, install_request.clone()).await, + Err(PluginInstallError::Marketplace(MarketplaceError::PluginNotFound { plugin_name, marketplace_name })) + if plugin_name == "sites" && marketplace_name == "openai-bundled" + )); + assert_eq!( + fs::read(home.path().join(CONFIG_TOML_FILE)).unwrap(), + config_before_rejected_install + ); + let exclusion_file = std::fs::read_dir(home.path().join("cache/bundled_plugin_exclusions")) + .unwrap() + .next() + .unwrap() + .unwrap() + .path(); + assert_eq!( + serde_json::from_slice::(&std::fs::read(exclusion_file).unwrap()) + .unwrap(), + serde_json::json!({"disabled-bundled-plugin-ids": ["sites@openai-bundled"]}) + ); + let requests = server.received_requests().await.unwrap().len(); + let restarted = test_plugins_manager_with_auth_manager( + home.path().to_path_buf(), + Some(Product::Codex), + auth_manager.clone(), + ); + assert!( + restarted + .ensure_sites_migration_ready(&config, Some(&auth)) + .await + .unwrap() + .is_none() + ); + assert_eq!(server.received_requests().await.unwrap().len(), requests); + assert!(!has_bundled_sites(&restarted)); + assert!(restarted.remote_installed_plugin_configs().is_empty()); + assert!( + !loaded_plugin_names(&restarted, &config) + .await + .contains(&"sites@openai-bundled".to_string()) + ); + + // A warm runtime/skill cache must not carry this backend's exclusion into another backend. + let mut other_backend = config.clone(); + other_backend.chatgpt_base_url = format!("{}/other-backend", server.uri()); + assert!( + restarted + .excluded_bundled_plugin_ids(&other_backend) + .is_empty() + ); + assert!( + restarted + .plugin_skill_snapshots_for_config(&config) + .is_some() + ); + assert!( + restarted + .plugin_skill_snapshots_for_config(&other_backend) + .is_none() + ); + assert_eq!( + loaded_plugin_names(&restarted, &other_backend).await, + vec!["sites@openai-bundled"] + ); + assert!(loaded_plugin_names(&restarted, &config).await.is_empty()); + + server.reset().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [], "pagination": {"next_page_token": null} + }))) + .mount(&server) + .await; + restarted + .reconcile_remote_installed_plugins(&config, Some(&auth)) + .await + .unwrap(); + assert!(has_bundled_sites(&restarted)); + restarted + .read_plugin_for_config(&config, &read_request) + .await + .unwrap(); + restarted + .install_plugin(&config, install_request) + .await + .unwrap(); +} + #[tokio::test] async fn reconcile_remote_installed_plugins_reports_cached_state_changes() { let codex_home = TempDir::new().unwrap(); @@ -7469,6 +7740,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { stale_refresh_generation, Vec::new(), /*auth*/ None, + "", RemoteInstalledPluginsCachePublication::Refresh, ), None @@ -7478,6 +7750,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { reconcile_generation, vec![remote_installed_linear_plugin()], /*auth*/ None, + "", RemoteInstalledPluginsCachePublication::Reconcile, ), Some(true) @@ -7501,6 +7774,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { refresh_generation, vec![remote_installed_plugin("beta")], /*auth*/ None, + "", RemoteInstalledPluginsCachePublication::Refresh, ), Some(true) @@ -7513,6 +7787,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { retry_generation, vec![remote_installed_plugin("beta")], /*auth*/ None, + "", RemoteInstalledPluginsCachePublication::Reconcile, ), Some(true) diff --git a/codex-rs/core-plugins/src/marketplace_policy/curated_loading_tests.rs b/codex-rs/core-plugins/src/marketplace_policy/curated_loading_tests.rs index 841ebf1d87..64186f0845 100644 --- a/codex-rs/core-plugins/src/marketplace_policy/curated_loading_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_policy/curated_loading_tests.rs @@ -77,7 +77,7 @@ host_pattern = '^github[.]com$' assert_eq!( manager .install_plugin( - &config.config_layer_stack, + config, PluginInstallRequest { plugin_name: "sample".to_string(), marketplace_path: AbsolutePathBuf::try_from(manifest.clone()).unwrap(), diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs index 4a31375583..631d0755bf 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs @@ -52,7 +52,7 @@ async fn verified_plugin_install_completed_requires_installed_plugin() { plugins_manager .install_plugin( - &config.config_layer_stack, + &config.plugins_config_input(), PluginInstallRequest { plugin_name: "sample".to_string(), marketplace_path: AbsolutePathBuf::try_from( diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index f07245e329..d549c62a49 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -63,7 +63,11 @@ use core_test_support::zsh_fork::zsh_fork_test_builder; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; +use wiremock::Mock; use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; const SAMPLE_PLUGIN_CONFIG_NAME: &str = "sample@test"; const SAMPLE_REMOTE_PLUGIN_CONFIG_NAME: &str = "sample@openai-curated-remote"; @@ -629,10 +633,8 @@ async fn plugin_skill_product_policy_and_migrated_command_precedence_reach_agent }) .await; - let developer_text = response - .single_request() - .message_input_texts("developer") - .join("\n"); + let requests = response.requests(); + let developer_text = requests[0].message_input_texts("developer").join("\n"); assert_eq!( ( developer_text.contains("sample:source-command-review: native review skill"), @@ -688,6 +690,107 @@ async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sites_migration_agent_turn_prefers_loadable_remote_sites() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + Mock::given(method("GET")) + .and(path("/ps/plugins/installed")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [{ + "id": "plugins~plugin_connector_1p_689987207de08191979cf68eca2941c6", + "name": "sites", + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "release": { + "version": "local", + "display_name": "Sites", + "description": "Sites", + "interface": {}, + }, + "enabled": true, + }], + "pagination": {"next_page_token": null}, + }))) + .mount(&server) + .await; + + let codex_home = Arc::new(TempDir::new()?); + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true +remote_plugin = true + +[plugins."sites@openai-bundled"] +enabled = true +"#, + )?; + for (marketplace, skill) in [ + ("openai-bundled", "bundled-sites"), + ("openai-curated-remote", "remote-sites"), + ] { + let root = codex_home + .path() + .join(format!("plugins/cache/{marketplace}/sites/local")); + std::fs::create_dir_all(root.join(".codex-plugin"))?; + std::fs::write( + root.join(".codex-plugin/plugin.json"), + r#"{"name":"sites"}"#, + )?; + let skill_dir = root.join("skills").join(skill); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {skill}\ndescription: test skill\n---\n\n# body\n"), + )?; + } + + let chatgpt_base_url = server.uri(); + let mut builder = test_codex() + .with_home(codex_home) + .with_extensions(skills_extensions()) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| config.chatgpt_base_url = chatgpt_base_url); + let test = builder.build_with_auto_env(&server).await?; + + let auth = test.thread_manager.auth_manager().auth().await; + test.thread_manager + .plugins_manager() + .reconcile_remote_installed_plugins(&test.config.plugins_config_input(), auth.as_ref()) + .await?; + + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Inspect the available Sites skills.".to_string(), + text_elements: Vec::new(), + }])) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let requests = response.requests(); + let developer_text = requests[0].message_input_texts("developer").join("\n"); + assert_eq!( + ( + developer_text.contains("sites:remote-sites"), + developer_text.contains("sites:bundled-sites"), + ), + (true, false), + "unexpected Sites skills in developer prompt: {developer_text:?}" + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths_and_codex_env_overlay() -> Result<()> { diff --git a/codex-rs/external-agent-migration/src/plugins.rs b/codex-rs/external-agent-migration/src/plugins.rs index 26932c4278..f61c071d4d 100644 --- a/codex-rs/external-agent-migration/src/plugins.rs +++ b/codex-rs/external-agent-migration/src/plugins.rs @@ -223,7 +223,7 @@ impl ExternalAgentConfigService { for plugin_name in plugin_names { match plugins_manager .install_plugin( - &install_config.config_layer_stack, + &install_config.plugins_config_input(), PluginInstallRequest { plugin_name: plugin_name.clone(), marketplace_path: marketplace_path.clone(),