diff --git a/codex-rs/app-server/tests/suite/v2/skills_list.rs b/codex-rs/app-server/tests/suite/v2/skills_list.rs index ef7ee6c1d..075cc007a 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::time::Duration; use anyhow::Context; @@ -8,6 +9,8 @@ use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url; +use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams; +use codex_app_server_protocol::ExperimentalFeatureEnablementSetResponse; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::PluginListParams; use codex_app_server_protocol::PluginListResponse; @@ -154,6 +157,108 @@ fn write_cached_remote_plugin_with_skill( Ok(skill_path) } +fn write_cached_local_curated_plugin_with_skill(codex_home: &std::path::Path) -> Result<()> { + let plugin_root = codex_home.join("plugins/cache/openai-curated/google-calendar/local"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"google-calendar"}"#, + )?; + + let skill_dir = plugin_root.join("skills/meeting-prep"); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: meeting-prep\ndescription: Prepare for meetings\n---\n\n# Body\n", + )?; + Ok(()) +} + +#[tokio::test] +async fn runtime_remote_plugin_enablement_excludes_local_curated_plugin_skills() -> Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let server = MockServer::start().await; + write_cached_local_curated_plugin_with_skill(codex_home.path())?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#"chatgpt_base_url = "{}/backend-api/" + +[features] +plugins = true + +[plugins."google-calendar@openai-curated"] +enabled = true +"#, + 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 = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let initial_skills_list_request_id = mcp + .send_skills_list_request(SkillsListParams { + cwds: vec![cwd.path().to_path_buf()], + force_reload: true, + }) + .await?; + let initial_skills_list_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(initial_skills_list_request_id)), + ) + .await??; + let SkillsListResponse { data } = to_response(initial_skills_list_response)?; + assert!(data.iter().any(|entry| { + entry + .skills + .iter() + .any(|skill| skill.name == "google-calendar:meeting-prep") + })); + + let enablement_request_id = mcp + .send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams { + enablement: BTreeMap::from([("remote_plugin".to_string(), true)]), + }) + .await?; + let enablement_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(enablement_request_id)), + ) + .await??; + let _: ExperimentalFeatureEnablementSetResponse = to_response(enablement_response)?; + + let skills_list_request_id = mcp + .send_skills_list_request(SkillsListParams { + cwds: vec![cwd.path().to_path_buf()], + force_reload: true, + }) + .await?; + let skills_list_response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(skills_list_request_id)), + ) + .await??; + let SkillsListResponse { data } = to_response(skills_list_response)?; + + assert!(data.iter().all(|entry| { + entry + .skills + .iter() + .all(|skill| skill.name != "google-calendar:meeting-prep") + })); + Ok(()) +} + #[tokio::test] async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 2a8d44fc9..7244b65ce 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -118,14 +118,14 @@ pub(crate) async fn load_plugins_from_layer_stack( store: &PluginStore, plugin_skill_snapshots: Option<&PluginSkillSnapshots>, restriction_product: Option, - prefer_remote_curated_conflicts: bool, + remote_global_catalog_active: bool, ) -> Vec> { let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, store, - prefer_remote_curated_conflicts, + remote_global_catalog_active, PluginLoadScope::AllCapabilities { restriction_product, skill_config_rules: &skill_config_rules, @@ -139,14 +139,14 @@ async fn load_plugins_from_layer_stack_with_scope( config_layer_stack: &ConfigLayerStack, extra_plugins: HashMap, store: &PluginStore, - prefer_remote_curated_conflicts: bool, + remote_global_catalog_active: bool, scope: PluginLoadScope<'_>, ) -> Vec> { let configured_plugins = merge_configured_plugins_with_remote_installed( configured_plugins_from_stack(config_layer_stack), extra_plugins, store, - prefer_remote_curated_conflicts, + remote_global_catalog_active, ); let mut configured_plugins: Vec<_> = configured_plugins.into_iter().collect(); configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); @@ -178,13 +178,13 @@ pub async fn load_plugin_hooks_from_layer_stack( config_layer_stack: &ConfigLayerStack, extra_plugins: HashMap, store: &PluginStore, - prefer_remote_curated_conflicts: bool, + remote_global_catalog_active: bool, ) -> PluginHookLoadOutcome { let plugins = load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, store, - prefer_remote_curated_conflicts, + remote_global_catalog_active, PluginLoadScope::HooksOnly, ) .await; @@ -206,8 +206,17 @@ fn merge_configured_plugins_with_remote_installed( mut configured_plugins: HashMap, extra_plugins: HashMap, store: &PluginStore, - prefer_remote_curated_conflicts: bool, + remote_global_catalog_active: bool, ) -> HashMap { + if remote_global_catalog_active { + configured_plugins.retain(|plugin_key, _| match PluginId::parse(plugin_key) { + Ok(plugin_id) => plugin_id.marketplace_name != crate::OPENAI_CURATED_MARKETPLACE_NAME, + Err(_) => true, + }); + configured_plugins.extend(extra_plugins); + return configured_plugins; + } + let mut local_curated_installed_plugin_keys = HashMap::>::new(); for plugin_key in configured_plugins.keys() { let Ok(plugin_id) = PluginId::parse(plugin_key) else { @@ -234,14 +243,8 @@ fn merge_configured_plugins_with_remote_installed( .as_ref() .and_then(|plugin_name| local_curated_installed_plugin_keys.get(plugin_name)); - if let Some(local_curated_plugin_keys) = local_curated_plugin_keys { - if prefer_remote_curated_conflicts { - for local_curated_plugin_key in local_curated_plugin_keys { - configured_plugins.remove(local_curated_plugin_key); - } - } else { - continue; - } + if local_curated_plugin_keys.is_some() { + continue; } configured_plugins.insert(plugin_key, plugin_config); diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index ee22256fd..d4faaa8e7 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -162,14 +162,14 @@ enabled = true &store, /*plugin_skill_snapshots*/ None, Some(Product::Codex), - /*prefer_remote_curated_conflicts*/ false, + /*remote_global_catalog_active*/ false, ) .await; let hooks_only = load_plugins_from_layer_stack_with_scope( &stack, HashMap::new(), &store, - /*prefer_remote_curated_conflicts*/ false, + /*remote_global_catalog_active*/ false, PluginLoadScope::HooksOnly, ) .await; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index e60ce5034..9b84094ba 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -384,15 +384,15 @@ struct LoadedPluginsCache { struct PluginLoadCacheKey { configured_plugins: HashMap, skill_config_rules: SkillConfigRules, - remote_plugin_enabled: bool, + remote_global_catalog_active: bool, } impl PluginLoadCacheKey { - fn from_config(config: &PluginsConfigInput) -> Self { + fn from_config(config: &PluginsConfigInput, remote_global_catalog_active: bool) -> Self { Self { configured_plugins: configured_plugins_from_stack(&config.config_layer_stack), skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack), - remote_plugin_enabled: config.remote_plugin_enabled, + remote_global_catalog_active, } } } @@ -459,6 +459,10 @@ impl PluginsManager { } } + fn remote_global_catalog_active(&self, config: &PluginsConfigInput) -> bool { + config.remote_plugin_enabled && self.auth_mode().is_some_and(AuthMode::uses_codex_backend) + } + pub fn set_analytics_events_client(&self, analytics_events_client: AnalyticsEventsClient) { let mut stored_client = match self.analytics_events_client.write() { Ok(client_guard) => client_guard, @@ -490,7 +494,8 @@ impl PluginsManager { if !config.plugins_enabled { return None; } - let key = PluginLoadCacheKey::from_config(config); + let key = + PluginLoadCacheKey::from_config(config, self.remote_global_catalog_active(config)); self.loaded_plugins_cache .read() .unwrap_or_else(std::sync::PoisonError::into_inner) @@ -519,7 +524,8 @@ impl PluginsManager { return PluginLoadOutcome::default(); } - let cache_key = PluginLoadCacheKey::from_config(config); + let remote_global_catalog_active = self.remote_global_catalog_active(config); + let cache_key = PluginLoadCacheKey::from_config(config, remote_global_catalog_active); if !force_reload && let Some(plugins) = self.cached_loaded_plugins(&cache_key) { return self.resolve_loaded_plugins_for_auth(plugins); } @@ -539,7 +545,7 @@ impl PluginsManager { &self.store, Some(&plugin_skill_snapshots), self.restriction_product, - config.remote_plugin_enabled, + remote_global_catalog_active, ) .await; log_plugin_load_errors(&plugins); @@ -624,7 +630,7 @@ impl PluginsManager { &self.store, /*plugin_skill_snapshots*/ None, self.restriction_product, - config.remote_plugin_enabled, + self.remote_global_catalog_active(config), ) .await; self.resolve_loaded_plugins_for_auth(plugins) @@ -643,7 +649,7 @@ impl PluginsManager { config_layer_stack, self.remote_installed_plugin_configs(), &self.store, - config.remote_plugin_enabled, + self.remote_global_catalog_active(config), ) .await } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 3dfcc2aa8..1d3363fff 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -1061,7 +1061,7 @@ enabled = true } #[tokio::test] -async fn remote_installed_cache_prefers_remote_curated_conflicts_when_remote_plugin_enabled() { +async fn remote_global_catalog_ignores_local_curated_plugins() { let codex_home = TempDir::new().unwrap(); write_file( &codex_home.path().join(CONFIG_TOML_FILE), @@ -1086,7 +1086,11 @@ enabled = true 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()); + let manager = PluginsManager::new_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); manager.write_remote_installed_plugins_cache(vec![ remote_installed_plugin("linear"), remote_installed_plugin("remote-only"), @@ -1100,13 +1104,54 @@ enabled = true .map(|plugin| plugin.config_name.clone()) .collect::>(), vec![ - "calendar@openai-curated".to_string(), + "linear@openai-api-curated".to_string(), "linear@openai-curated-remote".to_string(), "remote-only@openai-curated-remote".to_string(), ] ); } +#[tokio::test] +async fn remote_plugin_feature_keeps_local_curated_without_codex_backend() { + let codex_home = TempDir::new().unwrap(); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true + +[plugins."linear@openai-curated"] +enabled = true + +[plugins."linear@openai-api-curated"] +enabled = true +"#, + ); + write_cached_plugin(codex_home.path(), "openai-curated", "linear"); + write_cached_plugin(codex_home.path(), "openai-api-curated", "linear"); + + let config = load_config(codex_home.path(), codex_home.path()).await; + let manager = PluginsManager::new_with_options( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::ApiKey), + ); + + let outcome = manager.plugins_for_config(&config).await; + + assert_eq!( + outcome + .plugins() + .iter() + .map(|plugin| plugin.config_name.clone()) + .collect::>(), + vec![ + "linear@openai-api-curated".to_string(), + "linear@openai-curated".to_string(), + ] + ); +} + #[tokio::test] async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metadata() { let codex_home = TempDir::new().unwrap(); @@ -2273,7 +2318,7 @@ fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() { let cache_key = PluginLoadCacheKey { configured_plugins: HashMap::new(), skill_config_rules: SkillConfigRules::default(), - remote_plugin_enabled: false, + remote_global_catalog_active: false, }; let stale_generation = manager.loaded_plugins_cache_generation(); @@ -5398,7 +5443,7 @@ async fn load_plugins_ignores_project_config_files() { &PluginStore::new(codex_home.path().to_path_buf()), /*plugin_skill_snapshots*/ None, Some(Product::Codex), - /*prefer_remote_curated_conflicts*/ false, + /*remote_global_catalog_active*/ false, ) .await;