From e58d9ef447785d4e81718dc11c2bcce14782fa8a Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Fri, 7 Aug 2026 13:47:56 +0000 Subject: [PATCH] Unify plugin skill loading with the host skill service (#37444) ## What changed - Inject the host skill loader into `PluginsManager` so plugin discovery and agent turns use the same loading and product-policy behavior. - Share plugin skill snapshots across those paths, preserving a consistent view of skills across workspaces. - Apply migrated-command precedence after product filtering, allowing an eligible migrated command to replace a filtered native skill with the same name. ## Testing - Add coverage for product-restricted plugin skills, native-versus-migrated command precedence, and the skills exposed to agent turns. GitOrigin-RevId: f5ef0d0766ebeeb30d73ffaf044d003c2906ea4d --- codex-rs/cli/src/marketplace_cmd.rs | 7 +- codex-rs/cli/src/mcp_cmd.rs | 4 +- codex-rs/cli/src/plugin_cmd.rs | 3 +- .../core-plugins/src/discoverable_tests.rs | 39 ++-- codex-rs/core-plugins/src/lib.rs | 1 + codex-rs/core-plugins/src/loader.rs | 65 ++++-- codex-rs/core-plugins/src/loader_tests.rs | 6 +- codex-rs/core-plugins/src/manager.rs | 31 ++- codex-rs/core-plugins/src/manager_tests.rs | 160 +++++++-------- codex-rs/core-plugins/src/skill_snapshots.rs | 34 ++++ codex-rs/core-plugins/src/test_support.rs | 185 ++++++++++++++++++ codex-rs/core/src/agent/role_tests.rs | 4 +- codex-rs/core/src/config/config_tests.rs | 20 +- codex-rs/core/src/connectors.rs | 3 +- codex-rs/core/src/connectors_tests.rs | 5 +- codex-rs/core/src/lib.rs | 1 + .../core/src/plugins/discoverable_tests.rs | 4 +- codex-rs/core/src/plugins/mod.rs | 16 ++ .../core/src/plugins/skill_snapshot_tests.rs | 71 ++++--- codex-rs/core/src/session/tests.rs | 11 +- .../core/src/session/tests/guardian_tests.rs | 3 +- codex-rs/core/src/thread_manager.rs | 24 +-- .../handlers/request_plugin_install_tests.rs | 4 +- codex-rs/core/tests/suite/mcp_auth_refresh.rs | 3 +- codex-rs/core/tests/suite/plugins.rs | 105 ++++++++++ codex-rs/ext/mcp/tests/hosted_apps_mcp.rs | 14 +- codex-rs/ext/skills/src/host_service.rs | 95 ++++----- codex-rs/ext/skills/src/host_service_tests.rs | 28 ++- .../src/detect/mod.rs | 4 +- .../external-agent-migration/src/plugins.rs | 4 +- 30 files changed, 682 insertions(+), 272 deletions(-) create mode 100644 codex-rs/core-plugins/src/skill_snapshots.rs diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 16aeb092e9..4a9609649b 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -4,9 +4,9 @@ use anyhow::bail; use clap::Parser; use codex_core::config::Config; use codex_core::config::find_codex_home; +use codex_core::plugins_manager_for_config; use codex_core_plugins::PluginMarketplaceUpgradeOutcome; use codex_core_plugins::PluginsConfigInput; -use codex_core_plugins::PluginsManager; use codex_core_plugins::installed_marketplaces::marketplace_install_root; use codex_core_plugins::installed_marketplaces::resolve_configured_marketplace_root; use codex_core_plugins::marketplace::marketplace_root_dir; @@ -211,7 +211,7 @@ async fn run_list(overrides: Vec<(String, toml::Value)>, args: ListMarketplaceAr let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let manager = PluginsManager::new(config.codex_home.to_path_buf()); + let manager = plugins_manager_for_config(&config); manager.set_auth_mode(load_cli_auth_mode(&config).await); let plugins_input = config.plugins_config_input(); let marketplace_listing = manager @@ -378,8 +378,7 @@ async fn run_upgrade( let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; - let manager = PluginsManager::new(codex_home.to_path_buf()); + let manager = plugins_manager_for_config(&config); let plugins_input = config.plugins_config_input(); let outcome = manager .upgrade_configured_marketplaces_for_config(&plugins_input, marketplace_name.as_deref()) diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index ec51a8452e..7f1ab720d6 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -17,7 +17,7 @@ use codex_core::config::LoaderOverrides; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config::find_codex_home; use codex_core::config::load_global_mcp_servers; -use codex_core_plugins::PluginsManager; +use codex_core::plugins_manager_for_config; use codex_exec_server::EnvironmentManager; use codex_exec_server::HttpClient; use codex_exec_server::RouteAwareHttpClient; @@ -478,7 +478,7 @@ async fn run_remove(config_overrides: &CliConfigOverrides, remove_args: RemoveAr } async fn load_mcp_manager(config: &Config) -> McpManager { - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(config)); plugins_manager.set_auth_mode(load_cli_auth_mode(config).await); McpManager::new(plugins_manager) } diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 73c65d48fc..d8d5b509d6 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -4,6 +4,7 @@ use anyhow::bail; use clap::Parser; use codex_core::config::Config; use codex_core::config::find_codex_home; +use codex_core::plugins_manager_for_config; use codex_core_plugins::ConfiguredMarketplace; use codex_core_plugins::OPENAI_BUNDLED_MARKETPLACE_NAME; use codex_core_plugins::PluginInstallOutcome; @@ -589,7 +590,7 @@ async fn load_plugin_command_context( .await .context("failed to load configuration")?; let plugins_input = config.plugins_config_input(); - let manager = PluginsManager::new(codex_home.to_path_buf()); + let manager = plugins_manager_for_config(&config); manager.set_auth_mode(load_cli_auth_mode(&config).await); Ok(PluginCommandContext { codex_home: codex_home.to_path_buf(), diff --git a/codex-rs/core-plugins/src/discoverable_tests.rs b/codex-rs/core-plugins/src/discoverable_tests.rs index 672f53fce4..d95eaa6eaf 100644 --- a/codex-rs/core-plugins/src/discoverable_tests.rs +++ b/codex-rs/core-plugins/src/discoverable_tests.rs @@ -10,6 +10,7 @@ use crate::remote::fetch_and_cache_global_remote_plugin_catalog; use crate::startup_sync::curated_plugins_repo_path; use crate::test_support::TEST_CURATED_PLUGIN_SHA; use crate::test_support::load_plugins_config; +use crate::test_support::test_plugins_manager; use crate::test_support::write_curated_plugin; use crate::test_support::write_curated_plugin_sha_with; use crate::test_support::write_file; @@ -49,7 +50,7 @@ remote_plugin = false write_openai_curated_marketplace(&curated_root, &["sample", "slack", "openai-developers"]); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); plugins_manager.set_auth_mode(Some(AuthMode::Chatgpt)); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let discoverable_plugins = list_discoverable_plugins( @@ -78,7 +79,7 @@ async fn returns_api_curated_fallback_plugins_for_direct_provider_auth() { write_openai_api_curated_marketplace(&curated_root, &["sample", "slack", "openai-developers"]); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); plugins_manager.set_auth_mode(Some(AuthMode::ApiKey)); let auth = CodexAuth::from_api_key("test-api-key"); let discoverable_plugins = list_discoverable_plugins( @@ -111,7 +112,7 @@ async fn returns_microsoft_fallback_plugins() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "teams").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -169,7 +170,7 @@ source = "/tmp/{bundled_marketplace_name}" ); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); plugins_manager.set_auth_mode(Some(AuthMode::Chatgpt)); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let discoverable_plugins = list_discoverable_plugins( @@ -195,7 +196,7 @@ async fn includes_openai_curated_when_remote_enabled_without_auth() { write_openai_curated_marketplace(&curated_root, &["slack"]); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -253,7 +254,7 @@ source = "/tmp/{marketplace_name}" ), ); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); assert!(plugins_manager.set_auth_mode(Some(AuthMode::Chatgpt))); let chatgpt_projection = list_discoverable_plugins( &plugins_manager, @@ -298,7 +299,7 @@ async fn reprojects_cached_skill_availability_for_current_config() { write_openai_curated_marketplace(&curated_root, &["slack"]); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let expected = ToolSuggestDiscoverablePlugin { id: "slack@openai-curated".to_string(), remote_plugin_id: None, @@ -352,7 +353,7 @@ async fn does_not_advertise_skills_when_skill_loading_fails() { ); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -391,7 +392,7 @@ async fn clear_cache_invalidates_cached_tool_suggest_metadata() { ); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let input = discovery_input(plugins, &[], &[], &[]); let expected_cached = vec![ToolSuggestDiscoverablePlugin { id: "slack@openai-curated".to_string(), @@ -463,7 +464,7 @@ source = "/tmp/{marketplace_name}" install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -490,7 +491,7 @@ async fn normalizes_description() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -520,7 +521,7 @@ async fn omits_installed_curated_plugins() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "slack").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -574,7 +575,7 @@ async fn omits_not_available_curated_plugins() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -615,7 +616,7 @@ async fn does_not_reload_marketplace_per_plugin() { } let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let buffer: &'static std::sync::Mutex> = Box::leak(Box::new(std::sync::Mutex::new(Vec::new()))); let subscriber = tracing_subscriber::fmt() @@ -663,7 +664,7 @@ async fn does_not_expand_local_plugins_by_installed_apps() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "slack").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -689,7 +690,7 @@ async fn does_not_read_local_plugins_for_loaded_apps() { ); let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let buffer: &'static std::sync::Mutex> = Box::leak(Box::new(std::sync::Mutex::new(Vec::new()))); let subscriber = tracing_subscriber::fmt() @@ -775,7 +776,7 @@ source = "/tmp/{sales_marketplace_name}" install_marketplace_plugin(codex_home.path(), sales_marketplace_root.as_path(), "sales").await; let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); let discoverable_plugins = list_discoverable_plugins( &plugins_manager, discovery_input(plugins, &[], &[], &[]), @@ -894,7 +895,7 @@ plugins = true let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let mut plugins = load_plugins_config(codex_home.path(), codex_home.path()).await; plugins.chatgpt_base_url = format!("{}/backend-api", server.uri()); - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = test_plugins_manager(codex_home.path().to_path_buf()); fetch_and_cache_global_remote_plugin_catalog( codex_home.path(), &RemotePluginServiceConfig::new( @@ -1023,7 +1024,7 @@ fn string_set(values: &[&str]) -> HashSet { async fn install_marketplace_plugin(codex_home: &Path, marketplace_root: &Path, plugin_name: &str) { write_curated_plugin_sha_with(codex_home, TEST_CURATED_PLUGIN_SHA); let config = load_plugins_config(codex_home, marketplace_root).await; - PluginsManager::new(codex_home.to_path_buf()) + test_plugins_manager(codex_home.to_path_buf()) .install_plugin( &config.config_layer_stack, PluginInstallRequest { diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 5ab8f7f71e..a31a0f6619 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -19,6 +19,7 @@ pub mod remote_bundle; pub mod remote_legacy; mod remote_plugin_id_resolver; mod script_attribution; +mod skill_snapshots; pub mod startup_sync; pub mod store; #[cfg(test)] diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 042037e171..414c0791b6 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -28,7 +28,7 @@ use codex_config::types::PluginConfig; use codex_config::types::PluginMcpServerConfig; use codex_connectors::parse_plugin_app_config; use codex_connectors::parse_plugin_app_config_value; -use codex_core_skills::PluginSkillSnapshots; +use codex_core_skills::PluginSkillSnapshots as LegacyPluginSkillSnapshots; use codex_core_skills::config_rules::resolve_disabled_skill_paths; use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_core_skills::loader::SkillRoot; @@ -48,8 +48,12 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_skills::SkillConfigRules; use codex_skills::SkillMetadata; +use codex_skills::SkillRootLoadRequest; +use codex_skills::SkillRootLoader; +use codex_skills::SkillRootSnapshots; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginIdentity; +use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; use codex_utils_plugins::find_plugin_manifest_path; use codex_utils_plugins::migrated_command_skills_root; @@ -91,9 +95,9 @@ enum PluginLoadScope<'a> { AllCapabilities { restriction_product: Option, skill_config_rules: &'a SkillConfigRules, - plugin_skill_snapshots: Option<&'a PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&'a SkillRootSnapshots>, remote_plugin_id_resolver: &'a RemotePluginIdResolver, - root_scan_slots: Arc, + skill_root_loader: &'a dyn SkillRootLoader, }, HooksOnly, } @@ -134,10 +138,10 @@ pub(crate) async fn load_plugins_from_layer_stack( config_layer_stack: &ConfigLayerStack, remote_installed_plugins_snapshot: RemoteInstalledPluginsSnapshot, store: &PluginStore, - plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&SkillRootSnapshots>, restriction_product: Option, remote_global_catalog_active: bool, - root_scan_slots: Arc, + skill_root_loader: &dyn SkillRootLoader, ) -> Vec> { let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); let RemoteInstalledPluginsSnapshot { @@ -154,7 +158,7 @@ pub(crate) async fn load_plugins_from_layer_stack( skill_config_rules: &skill_config_rules, plugin_skill_snapshots, remote_plugin_id_resolver: &remote_plugin_id_resolver, - root_scan_slots, + skill_root_loader, }, ) .await @@ -897,7 +901,7 @@ async fn load_plugin( skill_config_rules, plugin_skill_snapshots, remote_plugin_id_resolver: _, - root_scan_slots, + skill_root_loader, } => { loaded_plugin.manifest_name = Some(manifest.display_name().to_string()); loaded_plugin.manifest_description = manifest.description.clone(); @@ -907,14 +911,14 @@ async fn load_plugin( plugin_id: loaded_plugin_id.as_key(), remote_plugin_id: loaded_plugin.remote_plugin_id.clone(), }; - let resolved_skills = load_plugin_skill_inventory( + let resolved_skills = load_plugin_skill_inventory_with_loader( &plugin_root, &plugin_identity, &manifest, loaded_manifest.format, *restriction_product, *plugin_skill_snapshots, - Arc::clone(root_scan_slots), + *skill_root_loader, ) .await .resolve(skill_config_rules); @@ -1021,7 +1025,7 @@ pub async fn load_plugin_skills( manifest: &PluginManifest, restriction_product: Option, skill_config_rules: &SkillConfigRules, - plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&LegacyPluginSkillSnapshots>, root_scan_slots: Arc, ) -> ResolvedPluginSkills { let plugin_identity = PluginIdentity { @@ -1046,7 +1050,7 @@ pub(crate) async fn load_plugin_skills_with_identity( manifest: &PluginManifest, restriction_product: Option, skill_config_rules: &SkillConfigRules, - plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&LegacyPluginSkillSnapshots>, root_scan_slots: Arc, ) -> ResolvedPluginSkills { load_plugin_skill_inventory( @@ -1068,7 +1072,7 @@ pub(crate) async fn load_plugin_skill_inventory( manifest: &PluginManifest, manifest_format: PluginManifestFormat, restriction_product: Option, - plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&LegacyPluginSkillSnapshots>, root_scan_slots: Arc, ) -> PluginSkillInventory { let discovery_mode = match manifest_format { @@ -1123,6 +1127,43 @@ pub(crate) async fn load_plugin_skill_inventory( PluginSkillInventory { skills, had_errors } } +pub(crate) async fn load_plugin_skill_inventory_with_loader( + plugin_root: &AbsolutePathBuf, + plugin_identity: &PluginIdentity, + manifest: &PluginManifest, + manifest_format: PluginManifestFormat, + restriction_product: Option, + plugin_skill_snapshots: Option<&SkillRootSnapshots>, + skill_root_loader: &dyn SkillRootLoader, +) -> PluginSkillInventory { + let discovery_mode = match manifest_format { + PluginManifestFormat::Legacy => SkillDiscoveryMode::Recursive, + PluginManifestFormat::AgentPlugin => SkillDiscoveryMode::DirectChildren, + }; + let roots = plugin_skill_roots(plugin_root, &manifest.paths, manifest_format) + .into_iter() + .map(|path| PluginSkillRoot { + path, + plugin_identity: plugin_identity.clone(), + plugin_namespace: manifest.name.clone(), + plugin_root: plugin_root.clone(), + discovery_mode, + }) + .collect(); + let outcome = skill_root_loader + .load_roots(SkillRootLoadRequest { + roots, + restriction_product, + snapshots: plugin_skill_snapshots.cloned(), + }) + .await; + + PluginSkillInventory { + skills: outcome.skills, + had_errors: !outcome.errors.is_empty(), + } +} + fn plugin_skill_roots( plugin_root: &AbsolutePathBuf, manifest_paths: &PluginManifestPaths, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 5150a6cec2..abbd3fcc24 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -1,12 +1,12 @@ use super::*; use crate::manifest::load_plugin_manifest; use crate::manifest::load_plugin_manifest_with_format; +use crate::test_support::test_skill_root_loader; use crate::test_support::write_file; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerSource; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; -use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS; use codex_plugin::PluginId; use codex_utils_plugins::AGENT_PLUGIN_SCHEMA_URI; use pretty_assertions::assert_eq; @@ -168,7 +168,7 @@ async fn installed_agent_plugin_uses_isolated_data_root_for_stdio_mcp() { /*plugin_skill_snapshots*/ None, Some(Product::Codex), /*remote_global_catalog_active*/ false, - Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)), + test_skill_root_loader().as_ref(), ) .await; @@ -337,7 +337,7 @@ enabled = true /*plugin_skill_snapshots*/ None, Some(Product::Codex), /*remote_global_catalog_active*/ false, - Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)), + test_skill_root_loader().as_ref(), ) .await; let hooks_only = load_plugins_from_layer_stack_with_scope( diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 04c8f1c6e5..098e416f3c 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -58,6 +58,7 @@ use crate::remote_legacy::RemotePluginMutationError; use crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot; use crate::remote_plugin_id_resolver::RemotePluginIdResolver; use crate::remote_plugin_id_resolver::persisted_remote_plugin_id_for_installation; +use crate::skill_snapshots::new_plugin_skill_snapshots; use crate::startup_sync::curated_plugins_api_marketplace_path; use crate::startup_sync::curated_plugins_repo_path; use crate::startup_sync::read_curated_plugins_sha; @@ -74,7 +75,6 @@ use codex_config::set_user_plugin_enabled; use codex_config::types::PluginConfig; use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; -use codex_core_skills::PluginSkillSnapshots; use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS; use codex_hooks::plugin_hook_declarations; @@ -94,6 +94,8 @@ use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; use codex_skills::SkillConfigRules; use codex_skills::SkillMetadata; +use codex_skills::SkillRootLoader; +use codex_skills::SkillRootSnapshots; use codex_tools::DiscoverablePluginInfo; use codex_tools::DiscoverableTool; use codex_tools::filter_request_plugin_install_discoverable_tools_for_client; @@ -442,6 +444,7 @@ pub struct PluginsManager { loaded_plugins_cache: RwLock, loaded_plugins_load_semaphore: Semaphore, skill_root_scan_slots: Arc, + skill_root_loader: Arc>, tool_suggest_metadata_cache: ToolSuggestMetadataCache, remote_installed_plugins_cache: RwLock>>, remote_installed_plugins_cache_refresh_state: RwLock, @@ -456,7 +459,7 @@ pub struct PluginsManager { struct LoadedPluginsCacheEntry { key: PluginLoadCacheKey, plugins: Vec, - plugin_skill_snapshots: PluginSkillSnapshots, + plugin_skill_snapshots: SkillRootSnapshots, } #[derive(Default)] @@ -510,14 +513,23 @@ fn target_curated_marketplace( } impl PluginsManager { - pub fn new(codex_home: PathBuf) -> Self { - Self::new_with_options(codex_home, Some(Product::Codex), /*auth_mode*/ None) + pub fn new( + codex_home: PathBuf, + skill_root_loader: Arc>, + ) -> Self { + Self::new_with_options( + codex_home, + Some(Product::Codex), + /*auth_mode*/ None, + skill_root_loader, + ) } pub fn new_with_options( codex_home: PathBuf, restriction_product: Option, auth_mode: Option, + skill_root_loader: Arc>, ) -> Self { // Product restrictions are enforced at marketplace admission time for a given CODEX_HOME: // listing, install, and curated refresh all consult this restriction context before new @@ -544,6 +556,7 @@ impl PluginsManager { loaded_plugins_cache: RwLock::new(LoadedPluginsCache::default()), loaded_plugins_load_semaphore: Semaphore::new(/*permits*/ 1), skill_root_scan_slots: Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)), + skill_root_loader, tool_suggest_metadata_cache: ToolSuggestMetadataCache::new(), remote_installed_plugins_cache: RwLock::new(None), remote_installed_plugins_cache_refresh_state: RwLock::new( @@ -628,7 +641,7 @@ impl PluginsManager { pub fn plugin_skill_snapshots_for_config( &self, config: &PluginsConfigInput, - ) -> Option { + ) -> Option> { if !config.plugins_enabled { return None; } @@ -683,7 +696,7 @@ impl PluginsManager { return self.resolve_loaded_plugins_for_auth(plugins, &config.model_provider_id); } let cache_generation = self.loaded_plugins_cache_generation(); - let plugin_skill_snapshots = PluginSkillSnapshots::for_plugin_load(); + let plugin_skill_snapshots = new_plugin_skill_snapshots(); let plugins = load_plugins_from_layer_stack( &config.config_layer_stack, self.remote_installed_plugins_snapshot(), @@ -691,7 +704,7 @@ impl PluginsManager { Some(&plugin_skill_snapshots), self.restriction_product, remote_global_catalog_active, - Arc::clone(&self.skill_root_scan_slots), + self.skill_root_loader.as_ref(), ) .await; log_plugin_load_errors(&plugins); @@ -792,7 +805,7 @@ impl PluginsManager { /*plugin_skill_snapshots*/ None, self.restriction_product, self.remote_global_catalog_active(config), - Arc::clone(&self.skill_root_scan_slots), + self.skill_root_loader.as_ref(), ) .await; self.resolve_loaded_plugins_for_auth(plugins, &config.model_provider_id) @@ -852,7 +865,7 @@ impl PluginsManager { generation: u64, key: PluginLoadCacheKey, plugins: Vec, - plugin_skill_snapshots: PluginSkillSnapshots, + plugin_skill_snapshots: SkillRootSnapshots, ) { let mut cache = match self.loaded_plugins_cache.write() { Ok(cache) => cache, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 9f7d211f20..4776447665 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -21,6 +21,9 @@ use crate::test_support::TEST_CURATED_PLUGIN_CACHE_VERSION; use crate::test_support::TEST_CURATED_PLUGIN_SHA; use crate::test_support::load_plugins_config as load_plugins_config_input; use crate::test_support::test_http_client_factory; +use crate::test_support::test_plugins_manager; +use crate::test_support::test_plugins_manager_with_options; +use crate::test_support::test_skill_root_loader; use crate::test_support::write_curated_plugin; use crate::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha; use crate::test_support::write_file; @@ -40,7 +43,6 @@ use codex_config::RequirementSource; use codex_config::RequirementsLayerEntry; use codex_config::compose_requirements; use codex_config::types::McpServerTransportConfig; -use codex_core_skills::PluginSkillSnapshots; use codex_login::CodexAuth; use codex_plugin::AppDeclaration; use codex_plugin::PluginId; @@ -118,7 +120,7 @@ fn plugins_config_input_with_requirements( #[test] fn plugins_manager_tracks_auth_mode() { let tmp = TempDir::new().unwrap(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); assert_eq!(manager.auth_mode(), None); assert!(manager.set_auth_mode(Some(AuthMode::ApiKey))); @@ -129,7 +131,7 @@ fn plugins_manager_tracks_auth_mode() { assert!(manager.set_auth_mode(/*auth_mode*/ None)); assert_eq!(manager.auth_mode(), None); - let manager_with_auth = PluginsManager::new_with_options( + let manager_with_auth = test_plugins_manager_with_options( tmp.path().join("auth"), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -149,7 +151,7 @@ fn curated_repo_sync_stays_deferred_for_remote_chatgpt_catalog() { "https://chatgpt.com".to_string(), test_http_client_factory(), ); - let manager = Arc::new(PluginsManager::new_with_options( + let manager = Arc::new(test_plugins_manager_with_options( tmp.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -165,7 +167,7 @@ fn curated_repo_sync_stays_deferred_for_remote_chatgpt_catalog() { #[test] fn marketplace_source_refresh_notifies_only_after_installed_cache_changes() { let tmp = TempDir::new().unwrap(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let callback_count = Arc::new(std::sync::atomic::AtomicUsize::new(0)); let callback_count_for_callback = Arc::clone(&callback_count); let callback: EffectivePluginsChangedCallback = Arc::new(move |_change| { @@ -225,7 +227,7 @@ source = "git" url = "https://github.com/example/other.git" "#, ); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let allowed_outcome = manager.plugins_for_config(&allowed).await; assert_eq!(allowed_outcome.plugins().len(), 1); @@ -264,7 +266,7 @@ restrict_to_allowed_sources = true AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) .expect("absolute marketplace path"); - let err = PluginsManager::new(codex_home.path().to_path_buf()) + let err = test_plugins_manager(codex_home.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -303,7 +305,7 @@ fn marketplace_policy_filters_discovered_marketplaces_by_configured_name() { let repo_root = AbsolutePathBuf::try_from(repo_root).expect("absolute repository root"); let subdirectory = AbsolutePathBuf::try_from(subdirectory).expect("absolute input subdirectory"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let user_config = format!( r#" [marketplaces.company] @@ -442,7 +444,7 @@ async fn plugin_auth_projection_hides_apps_without_chatgpt_auth() { write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); let config = auth_projection_config(codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::ApiKey), @@ -470,7 +472,7 @@ async fn plugin_auth_projection_hides_matching_mcp_with_chatgpt_apps_route() { write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); let config = auth_projection_config(codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -511,7 +513,7 @@ async fn plugin_auth_projection_hides_dual_surface_mcp_with_agent_identity_apps_ write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); let config = auth_projection_config(codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::AgentIdentity), @@ -536,7 +538,7 @@ async fn plugin_auth_projection_keeps_non_conflicting_mcp_with_chatgpt_apps_rout write_auth_projection_app(codex_home.path(), "sample", "sample_app"); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); let config = auth_projection_config(codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -619,7 +621,7 @@ enabled = true "#, ); let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -653,7 +655,7 @@ async fn plugin_auth_projection_reprojects_cached_plugins_when_auth_changes() { write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); write_auth_projection_plugin(codex_home.path(), "docs", /*include_app*/ false); let config = auth_projection_config(codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -811,7 +813,7 @@ async fn load_plugins_from_config( ) -> PluginLoadOutcome { write_file(&codex_home.join(CONFIG_TOML_FILE), config_toml); let config = load_config(codex_home, codex_home).await; - PluginsManager::new_with_options(codex_home.to_path_buf(), Some(Product::Codex), auth_mode) + test_plugins_manager_with_options(codex_home.to_path_buf(), Some(Product::Codex), auth_mode) .plugins_for_config(&config) .await } @@ -1172,7 +1174,7 @@ approval_mode = "approve" ); let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -1215,7 +1217,7 @@ plugins = true ); let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); manager.write_remote_installed_plugins_cache(vec![remote_installed_linear_plugin()]); let outcome = manager.plugins_for_config(&config).await; @@ -1226,7 +1228,7 @@ plugins = true async fn installed_plugin_telemetry_metadata_collects_capabilities() { let codex_home = TempDir::new().unwrap(); write_cached_plugin(codex_home.path(), "test", "sample"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse"); let metadata = manager @@ -1260,7 +1262,7 @@ async fn installed_plugin_telemetry_metadata_resolves_persisted_remote_identity( PluginStore::new(codex_home.path().to_path_buf()) .write_remote_plugin_id(&plugin_id, "plugins~Plugin_linear") .expect("persist remote plugin id"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let metadata = manager .telemetry_metadata_for_installed_plugin(&plugin_id) @@ -1292,7 +1294,7 @@ fn plugin_telemetry_ignores_local_marketplace_sidecars() { PluginStore::new(codex_home.path().to_path_buf()) .write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample") .expect("persist remote plugin id"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); assert_eq!( manager.telemetry_metadata_for_plugin_id(&plugin_id), @@ -1313,7 +1315,7 @@ async fn installed_plugin_telemetry_metadata_prefers_remote_snapshot_identity() PluginStore::new(codex_home.path().to_path_buf()) .write_remote_plugin_id(&plugin_id, "plugins~Plugin_stale") .expect("persist remote plugin id"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); manager.write_remote_installed_plugins_cache(vec![remote_installed_linear_plugin()]); let metadata = manager @@ -1341,7 +1343,7 @@ async fn installed_plugin_telemetry_metadata_prefers_remote_snapshot_identity() #[tokio::test] async fn installed_plugin_telemetry_metadata_accepts_authoritative_remote_identity() { let codex_home = TempDir::new().unwrap(); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let plugin_id = PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"); @@ -1362,7 +1364,7 @@ async fn installed_plugin_telemetry_metadata_accepts_authoritative_remote_identi #[test] fn capability_summary_telemetry_metadata_uses_local_identity() { let codex_home = TempDir::new().unwrap(); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let summary = PluginCapabilitySummary { config_name: "linear@openai-curated-remote".to_string(), display_name: "Linear".to_string(), @@ -1396,7 +1398,7 @@ fn capability_summary_telemetry_metadata_resolves_persisted_remote_identity() { PluginStore::new(codex_home.path().to_path_buf()) .write_remote_plugin_id(&plugin_id, "plugins~Plugin_linear") .expect("persist remote plugin id"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let summary = PluginCapabilitySummary { config_name: "linear@openai-curated-remote".to_string(), display_name: "Linear".to_string(), @@ -1445,7 +1447,7 @@ 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_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -1482,7 +1484,7 @@ enabled = true write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear"); let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -1520,7 +1522,7 @@ 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_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -1559,7 +1561,7 @@ enabled = true write_cached_plugin(codex_home.path(), "openai-curated-remote", "remote-only"); let mut config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -1602,7 +1604,7 @@ enabled = true #[tokio::test] async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metadata() { let codex_home = TempDir::new().unwrap(); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let mut plugin = remote_installed_linear_plugin(); plugin.install_policy = codex_app_server_protocol::PluginInstallPolicy::InstalledByDefault; plugin.auth_policy = codex_app_server_protocol::PluginAuthPolicy::OnInstall; @@ -1679,7 +1681,7 @@ async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metad #[tokio::test] async fn build_remote_installed_plugin_marketplaces_from_cache_filters_by_marketplace_name() { let codex_home = TempDir::new().unwrap(); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); manager.write_remote_installed_plugins_cache(vec![ remote_installed_plugin_in_marketplace( "workspace-linear", @@ -2798,7 +2800,7 @@ async fn load_plugins_returns_empty_when_feature_disabled() { ); let config = load_config(codex_home.path(), codex_home.path()).await; - let outcome = PluginsManager::new(codex_home.path().to_path_buf()) + let outcome = test_plugins_manager(codex_home.path().to_path_buf()) .plugins_for_config(&config) .await; @@ -2863,7 +2865,7 @@ async fn plugin_cache_ignores_unrelated_session_overrides() { test_http_client_factory(), ) }; - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let first = manager .plugins_for_config(&config(r#"model = "first""#)) @@ -2923,7 +2925,7 @@ enabled = true .write_remote_plugin_id(&plugin_id, "plugins~Plugin_persisted") .expect("persist remote plugin id"); let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -2962,7 +2964,7 @@ enabled = true #[test] fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() { let codex_home = TempDir::new().unwrap(); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let cache_key = PluginLoadCacheKey { configured_plugins: HashMap::new(), skill_config_rules: SkillConfigRules::default(), @@ -2975,7 +2977,7 @@ fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() { stale_generation, cache_key.clone(), Vec::new(), - PluginSkillSnapshots::for_plugin_load(), + crate::skill_snapshots::new_plugin_skill_snapshots(), ); assert_eq!(manager.cached_loaded_plugins(&cache_key), None); @@ -3049,7 +3051,7 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() { ) .unwrap(); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3113,7 +3115,7 @@ path = {marketplace_root:?} let marketplace_path = AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) .expect("absolute marketplace path"); - let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); let err = manager .install_plugin( @@ -3165,7 +3167,7 @@ async fn install_openai_curated_plugin_uses_short_sha_cache_version() { write_openai_curated_marketplace(&curated_root, &["slack"]); write_curated_plugin_sha(tmp.path(), TEST_CURATED_PLUGIN_SHA); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3226,7 +3228,7 @@ async fn install_plugin_uses_manifest_version_for_non_curated_plugins() { ) .unwrap(); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3294,7 +3296,7 @@ async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin ) .unwrap(); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3394,7 +3396,7 @@ async fn install_plugin_supports_git_subdir_marketplace_sources() { ) .unwrap(); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3448,7 +3450,7 @@ async fn install_plugin_supports_relative_git_subdir_marketplace_sources() { ) .unwrap(); - let result = PluginsManager::new(tmp.path().to_path_buf()) + let result = test_plugins_manager(tmp.path().to_path_buf()) .install_plugin( &unrestricted_config_layer_stack(), PluginInstallRequest { @@ -3493,7 +3495,7 @@ enabled = true "#, ); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); manager .uninstall_plugin("sample-plugin@debug".to_string()) .await @@ -3565,7 +3567,7 @@ enabled = false ); let config = load_config(tmp.path(), &repo_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[AbsolutePathBuf::try_from(repo_root).unwrap()], @@ -3673,7 +3675,7 @@ enabled = true ); let config = load_config(tmp.path(), &repo_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[AbsolutePathBuf::try_from(repo_root).unwrap()], @@ -3725,7 +3727,7 @@ plugins = true ); let config = load_config(tmp.path(), &repo_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[AbsolutePathBuf::try_from(repo_root).unwrap()], @@ -3803,7 +3805,7 @@ enabled = true ); let config = load_config(tmp.path(), &repo_root).await; - let err = PluginsManager::new(tmp.path().to_path_buf()) + let err = test_plugins_manager(tmp.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -3866,7 +3868,7 @@ plugins = true .unwrap(), }; - let chatgpt_outcome = PluginsManager::new_with_options( + let chatgpt_outcome = test_plugins_manager_with_options( tmp.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -3883,7 +3885,7 @@ plugins = true vec![AppConnectorId("connector_sample".to_string())] ); - let api_key_outcome = PluginsManager::new_with_options( + let api_key_outcome = test_plugins_manager_with_options( tmp.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::ApiKey), @@ -3935,7 +3937,7 @@ plugins = true ); let config = load_config(tmp.path(), &repo_root).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let outcome = manager .read_plugin_for_config( &config, @@ -4037,7 +4039,7 @@ async fn agent_plugin_read_and_tool_suggestions_use_portable_capabilities_only() ); let config = load_config(tmp.path(), &repo_root).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let plugin = manager .list_marketplaces_for_config( &config, @@ -4111,7 +4113,7 @@ plugins = true ); let config = load_config(tmp.path(), &repo_root).await; - let err = PluginsManager::new(tmp.path().to_path_buf()) + let err = test_plugins_manager(tmp.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -4176,7 +4178,7 @@ enabled = false ); let config = load_config(tmp.path(), &repo_root).await; - let outcome = PluginsManager::new(tmp.path().to_path_buf()) + let outcome = test_plugins_manager(tmp.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -4232,7 +4234,7 @@ plugins = true ); let config = load_config(tmp.path(), &repo_root).await; - let outcome = PluginsManager::new(tmp.path().to_path_buf()) + let outcome = test_plugins_manager(tmp.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -4375,7 +4377,7 @@ enabled = false ); let config = load_config(tmp.path(), &repo_root).await; - let outcome = PluginsManager::new(tmp.path().to_path_buf()) + let outcome = test_plugins_manager(tmp.path().to_path_buf()) .read_plugin_for_config( &config, &PluginReadRequest { @@ -4496,7 +4498,7 @@ enabled = true ); let config = load_config(tmp.path(), &repo_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], @@ -4602,7 +4604,7 @@ plugins = true .unwrap(); let config = load_config(tmp.path(), tmp.path()).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) .unwrap() .marketplaces; @@ -4659,7 +4661,7 @@ plugins = true ); let config = load_config(tmp.path(), tmp.path()).await; - let outcome = PluginsManager::new(tmp.path().to_path_buf()) + let outcome = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ false) .unwrap(); @@ -4719,7 +4721,7 @@ plugins = true ); let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); manager.set_auth_mode(Some(AuthMode::ApiKey)); let marketplaces = manager .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) @@ -4797,7 +4799,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.model_provider_id = resolved_provider.to_string(); - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) .unwrap() .marketplaces; @@ -4828,7 +4830,7 @@ plugins = true write_openai_api_curated_marketplace(&curated_root, &["api-plugin"]); let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); manager.set_auth_mode(Some(AuthMode::Chatgpt)); let marketplaces = manager .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) @@ -4864,7 +4866,7 @@ plugins = true ); let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); manager.set_auth_mode(Some(AuthMode::BedrockApiKey)); let outcome = manager .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) @@ -4921,7 +4923,7 @@ source = "/tmp/debug" ) .unwrap(); let config = load_config(tmp.path(), tmp.path()).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) .unwrap() .marketplaces; @@ -4993,7 +4995,7 @@ source = "{remote_repo_url}" ), ); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let config = load_config(tmp.path(), tmp.path()).await; let initial_upgrade = manager .upgrade_configured_marketplaces_for_config(&config, /*marketplace_name*/ None) @@ -5094,7 +5096,7 @@ source = "/tmp/debug" fs::write(registry_path, "{not valid json").unwrap(); let config = load_config(tmp.path(), tmp.path()).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) .unwrap() .marketplaces; @@ -5149,7 +5151,7 @@ plugins = true ) .unwrap(); let config = load_config(tmp.path(), tmp.path()).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[], /*include_openai_curated*/ true) .unwrap() .marketplaces; @@ -5228,7 +5230,7 @@ enabled = false ); let config = load_config(tmp.path(), &repo_a_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[ @@ -5347,7 +5349,7 @@ enabled = true ); let config = load_config(tmp.path(), &repo_root).await; - let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + let marketplaces = test_plugins_manager(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, &[AbsolutePathBuf::try_from(repo_root).unwrap()], @@ -5421,7 +5423,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = format!("{}/backend-api/", server.uri()); - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( tmp.path().to_path_buf(), Some(Product::Chatgpt), /*auth_mode*/ None, @@ -5458,7 +5460,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = format!("{}/backend-api/", server.uri()); - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( tmp.path().to_path_buf(), /*restriction_product*/ None, /*auth_mode*/ None, @@ -5496,7 +5498,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); - let manager = std::sync::Arc::new(PluginsManager::new(tmp.path().to_path_buf())); + let manager = std::sync::Arc::new(test_plugins_manager(tmp.path().to_path_buf())); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let cache_key = recommended_plugins_cache_key(&config); @@ -5577,7 +5579,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let expected = RecommendedPluginsMode::Endpoint { plugins: vec![ @@ -5647,7 +5649,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let mut installed_linear = remote_installed_plugin("linear"); installed_linear.id = "plugin_linear".to_string(); manager.write_remote_installed_plugins_cache(vec![installed_linear]); @@ -5704,7 +5706,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); assert_eq!( manager @@ -5740,7 +5742,7 @@ plugins = true let mut config = load_config(tmp.path(), tmp.path()).await; config.chatgpt_base_url = server.uri(); - let manager = PluginsManager::new(tmp.path().to_path_buf()); + let manager = test_plugins_manager(tmp.path().to_path_buf()); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); assert_eq!( manager @@ -6424,7 +6426,7 @@ async fn load_plugins_ignores_project_config_files() { /*plugin_skill_snapshots*/ None, Some(Product::Codex), /*remote_global_catalog_active*/ false, - Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)), + test_skill_root_loader().as_ref(), ) .await; @@ -6466,7 +6468,7 @@ async fn plugin_hooks_for_layer_stack_loads_configured_plugin_hooks() { ); let config = load_config(codex_home.path(), codex_home.path()).await; - let outcome = PluginsManager::new(codex_home.path().to_path_buf()) + let outcome = test_plugins_manager(codex_home.path().to_path_buf()) .plugin_hooks_for_layer_stack(&config.config_layer_stack, &config) .await; @@ -6524,7 +6526,7 @@ enabled = true ); } let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new_with_options( + let manager = test_plugins_manager_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), @@ -6572,7 +6574,7 @@ enabled = true #[test] fn remote_installed_plugins_cache_refresh_coalesces_materializations() { let tmp = TempDir::new().unwrap(); - let manager = std::sync::Arc::new(PluginsManager::new(tmp.path().to_path_buf())); + let manager = std::sync::Arc::new(test_plugins_manager(tmp.path().to_path_buf())); let materialization_callback_count = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); let unrelated_callback_count = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); diff --git a/codex-rs/core-plugins/src/skill_snapshots.rs b/codex-rs/core-plugins/src/skill_snapshots.rs new file mode 100644 index 0000000000..d236d691f6 --- /dev/null +++ b/codex-rs/core-plugins/src/skill_snapshots.rs @@ -0,0 +1,34 @@ +use std::collections::HashMap; +use std::sync::Arc; +use std::sync::Mutex; + +use codex_skills::LoadedSkillRoot; +use codex_skills::SkillRootSnapshotCache; +use codex_skills::SkillRootSnapshots; +use codex_utils_plugins::PluginSkillRoot; + +#[derive(Default)] +struct PluginSkillSnapshotCache { + snapshots_by_root: Mutex>, +} + +impl SkillRootSnapshotCache for PluginSkillSnapshotCache { + fn get(&self, root: &PluginSkillRoot) -> Option { + self.snapshots_by_root + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(root) + .cloned() + } + + fn insert(&self, root: PluginSkillRoot, snapshot: LoadedSkillRoot) { + self.snapshots_by_root + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(root, snapshot); + } +} + +pub(crate) fn new_plugin_skill_snapshots() -> SkillRootSnapshots { + SkillRootSnapshots::new(Arc::new(PluginSkillSnapshotCache::default())) +} diff --git a/codex-rs/core-plugins/src/test_support.rs b/codex-rs/core-plugins/src/test_support.rs index 962f66332c..c182bfb2d4 100644 --- a/codex-rs/core-plugins/src/test_support.rs +++ b/codex-rs/core-plugins/src/test_support.rs @@ -1,11 +1,15 @@ +use std::collections::HashMap; +use std::collections::HashSet; use std::fs; use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; use crate::OPENAI_API_CURATED_MARKETPLACE_NAME; use crate::OPENAI_CURATED_MARKETPLACE_NAME; use crate::PluginsConfigInput; +use crate::PluginsManager; use crate::http_client_selector::HttpClientSelector; use crate::remote::RemotePluginServiceConfig; use codex_config::LoaderOverrides; @@ -17,13 +21,194 @@ use codex_http_client::HttpClientFactory; use codex_http_client::OutboundProxyPolicy; use codex_http_client::RouteAwareClientPool; use codex_http_client::RouteAwareRequestBuilder; +use codex_protocol::auth::AuthMode; +use codex_protocol::protocol::Product; +use codex_protocol::protocol::SkillScope; +use codex_skills::LoadedSkillRoot; +use codex_skills::LoadedSkills; +use codex_skills::SkillError; +use codex_skills::SkillLoadFuture; +use codex_skills::SkillMetadata; +use codex_skills::SkillRootLoadRequest; +use codex_skills::SkillRootLoader; +use codex_skills::parse_skill_frontmatter_metadata; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginSkillRoot; +use codex_utils_plugins::SkillDiscoveryMode; +use codex_utils_plugins::migrated_command_skills_root; use http::Method; use toml::Value; pub(crate) const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; pub(crate) const TEST_CURATED_PLUGIN_CACHE_VERSION: &str = "01234567"; +pub(crate) fn test_plugins_manager(codex_home: PathBuf) -> PluginsManager { + PluginsManager::new(codex_home, test_skill_root_loader()) +} + +pub(crate) fn test_plugins_manager_with_options( + codex_home: PathBuf, + restriction_product: Option, + auth_mode: Option, +) -> PluginsManager { + PluginsManager::new_with_options( + codex_home, + restriction_product, + auth_mode, + test_skill_root_loader(), + ) +} + +pub(crate) fn test_skill_root_loader() -> Arc> { + Arc::new(TestSkillRootLoader) +} + +struct TestSkillRootLoader; + +impl SkillRootLoader for TestSkillRootLoader { + fn load_roots( + &self, + request: SkillRootLoadRequest, + ) -> SkillLoadFuture<'_, LoadedSkills> { + Box::pin(async move { + let mut loaded_roots = Vec::new(); + for root in request.roots { + let cached = request + .snapshots + .as_ref() + .and_then(|cache| cache.get(&root)); + let snapshot = match cached { + Some(snapshot) => snapshot, + None => { + let snapshot = load_test_skill_root(&root); + if let Some(snapshots) = &request.snapshots { + snapshots.insert(root.clone(), snapshot.clone()); + } + snapshot + } + }; + let migrated_root = migrated_command_skills_root(&root.plugin_root); + let canonical_migrated_root = fs::canonicalize(migrated_root.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok()) + .unwrap_or(migrated_root); + loaded_roots.push((snapshot.root == canonical_migrated_root, snapshot)); + } + + let native_names = loaded_roots + .iter() + .filter(|(migrated, _)| !migrated) + .flat_map(|(_, snapshot)| &snapshot.skills) + .map(|skill| (skill.plugin_id.clone(), skill.name.clone())) + .collect::>(); + let mut seen_paths = HashSet::new(); + let mut outcome = LoadedSkills::default(); + for (migrated, snapshot) in loaded_roots { + outcome + .skills + .extend(snapshot.skills.into_iter().filter(|skill| { + (!migrated + || !native_names + .contains(&(skill.plugin_id.clone(), skill.name.clone()))) + && skill.matches_product_restriction_for_product( + request.restriction_product, + ) + && seen_paths.insert(skill.path_to_skills_md.clone()) + })); + outcome.errors.extend(snapshot.errors); + } + outcome.skills.sort_by(|left, right| { + left.name + .cmp(&right.name) + .then_with(|| left.path_to_skills_md.cmp(&right.path_to_skills_md)) + }); + outcome + }) + } +} + +fn load_test_skill_root(root: &PluginSkillRoot) -> LoadedSkillRoot { + let canonical_root = fs::canonicalize(root.path.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok()) + .unwrap_or_else(|| root.path.clone()); + let mut skills = Vec::new(); + let mut errors = Vec::new(); + let mut discovery_paths = HashMap::new(); + let mut directories = vec![root.path.clone()]; + while let Some(directory) = directories.pop() { + let Ok(entries) = fs::read_dir(directory.as_path()) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + if (root.discovery_mode == SkillDiscoveryMode::Recursive || directory == root.path) + && let Ok(path) = AbsolutePathBuf::from_absolute_path_checked(path) + { + directories.push(path); + } + continue; + } + if path.file_name().is_none_or(|name| name != "SKILL.md") + || (root.discovery_mode == SkillDiscoveryMode::DirectChildren + && directory == root.path) + { + continue; + } + let Ok(path) = AbsolutePathBuf::from_absolute_path_checked(path) else { + continue; + }; + let canonical_path = fs::canonicalize(path.as_path()) + .ok() + .and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok()) + .unwrap_or_else(|| path.clone()); + let parsed = fs::read_to_string(path.as_path()) + .map_err(|error| error.to_string()) + .and_then(|contents| { + parse_skill_frontmatter_metadata(&contents, || { + directory + .as_path() + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or_default() + .to_string() + }) + .map_err(|error| error.to_string()) + }); + match parsed { + Ok(parsed) => { + discovery_paths.insert(canonical_path.clone(), path); + skills.push(SkillMetadata { + name: format!("{}:{}", root.plugin_namespace, parsed.name), + description: parsed.description, + short_description: parsed.short_description, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: canonical_path, + scope: SkillScope::User, + plugin_id: Some(root.plugin_identity.plugin_id.clone()), + remote_plugin_id: root.plugin_identity.remote_plugin_id.clone(), + }); + } + Err(message) => errors.push(SkillError { + path: canonical_path, + message, + }), + } + } + } + + LoadedSkillRoot { + root: canonical_root, + skills, + skill_discovery_path_by_path: Arc::new(discovery_paths), + errors, + is_agent_plugin: root.discovery_mode == SkillDiscoveryMode::DirectChildren, + } +} + pub(crate) fn test_http_client_factory() -> HttpClientFactory { HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault) } diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 002dd2ba86..ea0087171d 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -1,8 +1,8 @@ use super::*; use crate::HostSkillsService; use crate::config::ConfigBuilder; +use crate::plugins::plugins_manager_for_config; use crate::skills_load_input_from_config; -use codex_core_plugins::PluginsManager; use codex_protocol::config_types::ServiceTier; use codex_protocol::openai_models::ReasoningEffort; use codex_utils_absolute_path::test_support::PathExt; @@ -398,7 +398,7 @@ enabled = false .await .expect("custom role should apply"); - let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let skills_service = HostSkillsService::new(home.path().abs(), /*bundled_skills_enabled*/ true); let plugins_input = config.plugins_config_input(); diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index ffc67949b1..8e64146794 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1,6 +1,7 @@ use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config::edit::apply_blocking; +use crate::plugins::plugins_manager_for_config; use assert_matches::assert_matches; use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerEntry; @@ -67,7 +68,6 @@ use codex_config::types::TuiNotificationSettings; use codex_config::types::TuiPetAnchor; use codex_config::types::WindowsSandboxModeToml; use codex_config::types::WindowsToml; -use codex_core_plugins::PluginsManager; use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_features::FeaturesToml; @@ -499,7 +499,7 @@ async fn load_config_resolves_non_prefixed_mcp_tool_servers() -> std::io::Result assert_eq!(config.non_prefixed_mcp_tool_servers, expected_servers); assert_eq!(config.prefix_mcp_tool_names(), expected_prefix); - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; assert_eq!(mcp_config.prefix_mcp_tool_names, expected_prefix); assert_eq!( @@ -5214,7 +5214,7 @@ async fn rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config() let config = thread_config .rebuild_preserving_session_layers(&refreshed_config) .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; let configured_servers = mcp_config.mcp_server_catalog.configured_servers(); @@ -5276,7 +5276,7 @@ enabled = true .codex_home(codex_home.path().to_path_buf()) .build() .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; let configured_servers = mcp_config.mcp_server_catalog.configured_servers(); @@ -5344,7 +5344,7 @@ url = "https://sample.example/mcp" ) .build() .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; let configured_servers = mcp_config.mcp_server_catalog.configured_servers(); @@ -5446,7 +5446,7 @@ enabled = true ) .build() .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; let configured_servers = mcp_config.mcp_server_catalog.configured_servers(); @@ -6463,7 +6463,7 @@ async fn to_mcp_config_preserves_apps_feature_from_config() -> std::io::Result<( codex_home.abs(), ) .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); config.apps_mcp_product_sku = Some("tpp".to_string()); let mcp_config = config.to_mcp_config(&plugins_manager).await; @@ -6490,7 +6490,7 @@ async fn to_mcp_config_flows_mcp_tool_prefix_from_feature() -> std::io::Result<( codex_home.abs(), ) .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; assert!(mcp_config.prefix_mcp_tool_names); @@ -6526,7 +6526,7 @@ async fn to_mcp_config_flows_mcp_2026_feature_from_config() -> std::io::Result<( codex_home.abs(), ) .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; assert_eq!(mcp_config.protocol_mode, codex_mcp::McpProtocolMode::Legacy); @@ -6550,7 +6550,7 @@ async fn to_mcp_config_preserves_auth_elicitation_feature_from_config() -> std:: codex_home.abs(), ) .await?; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = config.to_mcp_config(&plugins_manager).await; assert_eq!( diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index af236b144e..d81b7f59cf 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -22,6 +22,7 @@ use tracing::warn; use crate::config::Config; use crate::mcp::McpManager; use crate::plugins::list_tool_suggest_discoverable_plugins; +use crate::plugins::plugins_manager_for_config; use crate::session::INITIAL_SUBMIT_ID; use codex_config::types::ApprovalsReviewer; use codex_config::types::ToolSuggestDiscoverableType; @@ -186,7 +187,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( force_refetch: bool, environment_manager: Arc, ) -> anyhow::Result { - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(config)); let mcp_manager = Arc::new(McpManager::new(plugins_manager)); list_accessible_connectors_from_mcp_tools_with_mcp_manager( config, diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index f481baf99b..b97a13b53f 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigBuilder; +use crate::plugins::plugins_manager_for_config; use codex_config::test_support::CloudConfigBundleFixture; use codex_config::types::ApprovalsReviewer; use codex_connectors::merge::plugin_connector_to_app_info; @@ -504,7 +505,7 @@ discoverables = [ .await .expect("config should load"); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let discoverable_tools = list_tool_suggest_discoverable_tools_with_auth( &config, @@ -542,7 +543,7 @@ apps = true .expect("config should load"); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let loaded_plugin_app_connector_ids = vec!["asdk_app_databricks_workspace".to_string()]; - let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let discoverable_tools = list_tool_suggest_discoverable_tools_with_auth( &config, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 4e030fe41e..9138a139c1 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -74,6 +74,7 @@ pub use mention_syntax::PLUGIN_TEXT_MENTION_SIGIL; pub use mention_syntax::TOOL_MENTION_SIGIL; pub use utils::path_utils; pub(crate) mod plugins; +pub use plugins::plugins_manager_for_config; #[doc(hidden)] pub(crate) mod prompt_debug; #[doc(hidden)] diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 12ca2fd3d4..0e11d7e832 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -1,7 +1,7 @@ +use crate::plugins::plugins_manager_for_config; use crate::plugins::test_support::load_plugins_config; use crate::plugins::test_support::write_file; use crate::plugins::test_support::write_openai_curated_marketplace; -use codex_core_plugins::PluginsManager; use codex_core_plugins::startup_sync::curated_plugins_repo_path; use codex_tools::DiscoverablePluginInfo; use pretty_assertions::assert_eq; @@ -11,7 +11,7 @@ async fn list_discoverable_plugins( config: &crate::config::Config, loaded_plugin_app_connector_ids: &[String], ) -> anyhow::Result> { - let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); + let plugins_manager = plugins_manager_for_config(config); super::list_tool_suggest_discoverable_plugins( config, &plugins_manager, diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index db3b651480..a2346abafe 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -8,6 +8,11 @@ mod skill_snapshot_tests; #[cfg(test)] pub(crate) mod test_support; +use crate::config::Config; +use codex_core_plugins::PluginsManager; +use codex_skills_extension::HostSkillsService; +use std::sync::Arc; + pub(crate) use codex_plugin::PluginCapabilitySummary; pub(crate) use discoverable::list_tool_suggest_discoverable_plugins; @@ -19,3 +24,14 @@ pub(crate) use mentions::build_skill_name_counts; pub(crate) use mentions::collect_explicit_app_ids; pub(crate) use mentions::collect_explicit_plugin_mentions; pub(crate) use mentions::collect_tool_mentions_from_messages; + +/// Constructs a standalone plugin manager with extension-owned plugin skill loading. +/// +/// Callers that already own a host skills service should inject that existing service instead. +pub fn plugins_manager_for_config(config: &Config) -> PluginsManager { + let skill_root_loader = Arc::new(HostSkillsService::new( + config.codex_home.clone(), + /*bundled_skills_enabled*/ false, + )); + PluginsManager::new(config.codex_home.to_path_buf(), skill_root_loader) +} diff --git a/codex-rs/core/src/plugins/skill_snapshot_tests.rs b/codex-rs/core/src/plugins/skill_snapshot_tests.rs index 2e3c8e3a0d..39e23930fe 100644 --- a/codex-rs/core/src/plugins/skill_snapshot_tests.rs +++ b/codex-rs/core/src/plugins/skill_snapshot_tests.rs @@ -5,7 +5,9 @@ use codex_protocol::auth::AuthMode; use codex_protocol::protocol::Product; use codex_skills_extension::HostSkillsLoadInput; use codex_skills_extension::HostSkillsService; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use std::sync::Arc; use super::test_support::load_plugins_config; use super::test_support::write_file; @@ -43,44 +45,53 @@ enabled = true .expect("persist remote plugin id"); let config = load_plugins_config(codex_home.path()).await; let plugins_input = config.plugins_config_input(); + let skills_service = Arc::new(HostSkillsService::new( + config.codex_home.clone(), + /*bundled_skills_enabled*/ false, + )); let plugins_manager = PluginsManager::new_with_options( codex_home.path().to_path_buf(), Some(Product::Codex), Some(AuthMode::Chatgpt), + skills_service.clone(), ); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; write_file(&skill_path, "---\nname: search\ndescription: second\n---\n"); - let skills_input = HostSkillsLoadInput::new( - config.cwd.clone(), - plugin_outcome.effective_plugin_skill_roots(), - config.config_layer_stack.clone(), - /*bundled_skills_enabled*/ false, - ) - .with_plugin_skill_snapshots(plugins_manager.plugin_skill_snapshots_for_config(&plugins_input)); - let skills_service = HostSkillsService::new( - config.codex_home.clone(), - /*bundled_skills_enabled*/ false, - ); - let snapshot = skills_service - .snapshot_for_config(&skills_input, /*fs*/ None) - .await; + let other_cwd = codex_home.path().join("other-workspace"); + std::fs::create_dir_all(&other_cwd).expect("create second workspace"); + let other_cwd = AbsolutePathBuf::from_absolute_path(other_cwd).expect("absolute workspace"); - assert_eq!( - snapshot - .outcome() - .skills - .iter() - .filter(|skill| skill.plugin_id.as_deref() == Some(PLUGIN_CONFIG_NAME)) - .map(|skill| { - ( - skill.description.as_str(), - skill.plugin_id.as_deref(), - skill.remote_plugin_id.as_deref(), - ) - }) - .collect::>(), - vec![("first", Some(PLUGIN_CONFIG_NAME), Some(REMOTE_PLUGIN_ID),)] - ); + for cwd in [config.cwd.clone(), other_cwd] { + let skills_input = HostSkillsLoadInput::new( + cwd, + plugin_outcome.effective_plugin_skill_roots(), + config.config_layer_stack.clone(), + /*bundled_skills_enabled*/ false, + ) + .with_plugin_skill_snapshots( + plugins_manager.plugin_skill_snapshots_for_config(&plugins_input), + ); + let snapshot = skills_service + .snapshot_for_config(&skills_input, /*fs*/ None) + .await; + + assert_eq!( + snapshot + .outcome() + .skills + .iter() + .filter(|skill| skill.plugin_id.as_deref() == Some(PLUGIN_CONFIG_NAME)) + .map(|skill| { + ( + skill.description.as_str(), + skill.plugin_id.as_deref(), + skill.remote_plugin_id.as_deref(), + ) + }) + .collect::>(), + vec![("first", Some(PLUGIN_CONFIG_NAME), Some(REMOTE_PLUGIN_ID),)] + ); + } } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 407db25c71..0dd5d28216 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -11,6 +11,7 @@ use crate::context::TurnAborted; use crate::environment_selection::ThreadEnvironments; use crate::environment_selection::TurnEnvironmentState; use crate::function_tool::FunctionCallError; +use crate::plugins::plugins_manager_for_config; use crate::session::step_context::StepContext; use crate::shell::default_user_shell; use crate::shell_snapshot::ShellSnapshot; @@ -5567,7 +5568,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { let (tx_event, _rx_event) = async_channel::unbounded(); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), @@ -5736,7 +5737,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { .expect("primary environment") .environment, ); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), @@ -5982,7 +5983,7 @@ async fn make_session_with_config_and_rx( let (tx_event, rx_event) = async_channel::unbounded(); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), @@ -6096,7 +6097,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( let (tx_event, rx_event) = async_channel::unbounded(); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), @@ -7955,7 +7956,7 @@ where .expect("primary environment") .environment, ); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 3daa3d08d5..8c3ce63d84 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -2,6 +2,7 @@ use super::*; use crate::compact::InitialContextInjection; use crate::exec_policy::ExecPolicyManager; use crate::guardian::GUARDIAN_REVIEWER_NAME; +use crate::plugins::plugins_manager_for_config; use crate::sandboxing::SandboxPermissions; use crate::session::step_context::StepContext; use crate::test_support::models_manager_with_provider; @@ -738,7 +739,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { auth_manager.clone(), config.model_provider.clone(), ); - let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); + let plugins_manager = Arc::new(plugins_manager_for_config(&config)); let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index f30f1f86b9..3c92f8a44f 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -416,21 +416,22 @@ impl ThreadManager { let codex_home = config.codex_home.clone(); let restriction_product = session_source.restriction_product(); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); + let skills_service = Arc::new(HostSkillsService::new_with_restriction_product( + codex_home.clone(), + config.bundled_skills_enabled(), + restriction_product, + )); let plugins_manager = Arc::new(PluginsManager::new_with_options( codex_home.to_path_buf(), restriction_product, auth_manager.get_api_auth_mode(), + skills_service.clone(), )); let mcp_manager = Arc::new(McpManager::new_with_extensions( Arc::clone(&plugins_manager), Arc::clone(&extensions), codex_apps_tools_cache, )); - let skills_service = Arc::new(HostSkillsService::new_with_restriction_product( - codex_home, - config.bundled_skills_enabled(), - restriction_product, - )); let code_mode_session_provider: Arc = if config.features.enabled(Feature::CodeModeHost) || config.code_mode.disable_in_process_fallback @@ -562,17 +563,18 @@ impl ThreadManager { }; let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); let restriction_product = SessionSource::Exec.restriction_product(); - let plugins_manager = Arc::new(PluginsManager::new_with_options( - codex_home.clone(), - restriction_product, - auth_manager.get_api_auth_mode(), - )); - let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_service = Arc::new(HostSkillsService::new_with_restriction_product( absolute_codex_home.clone(), /*bundled_skills_enabled*/ true, restriction_product, )); + let plugins_manager = Arc::new(PluginsManager::new_with_options( + codex_home.clone(), + restriction_product, + auth_manager.get_api_auth_mode(), + skills_service.clone(), + )); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); // This test constructor has no Config input. Tests that need a non-local // process store should construct ThreadManager::new with an explicit store. let thread_store: Arc = Arc::new(LocalThreadStore::new( 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 774ed49116..846ae38d13 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 @@ -1,4 +1,5 @@ use super::*; +use crate::plugins::plugins_manager_for_config; use crate::plugins::test_support::load_plugins_config; use crate::plugins::test_support::write_curated_plugin_sha; use crate::plugins::test_support::write_openai_curated_marketplace; @@ -10,7 +11,6 @@ use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverable; use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::PluginInstallRequest; -use codex_core_plugins::PluginsManager; use codex_core_plugins::startup_sync::curated_plugins_repo_path; use codex_rmcp_client::ElicitationResponse; use codex_tools::DiscoverablePluginInfo; @@ -40,7 +40,7 @@ async fn verified_plugin_install_completed_requires_installed_plugin() { write_plugins_feature_config(codex_home.path()); let config = load_plugins_config(codex_home.path()).await; - let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); assert!(!verified_plugin_install_completed( "sample@openai-curated", diff --git a/codex-rs/core/tests/suite/mcp_auth_refresh.rs b/codex-rs/core/tests/suite/mcp_auth_refresh.rs index 8929eca9f5..4fa1f437a5 100644 --- a/codex-rs/core/tests/suite/mcp_auth_refresh.rs +++ b/codex-rs/core/tests/suite/mcp_auth_refresh.rs @@ -4,6 +4,7 @@ use anyhow::Result; use codex_config::McpServerTransportConfig; use codex_core::config::ConfigBuilder; use codex_core::config::Constrained; +use codex_core::plugins_manager_for_config; use codex_exec_server_test_support::environment_manager_without_environments; use codex_login::AuthManager; use codex_login::CodexAuth; @@ -87,7 +88,7 @@ async fn hosted_plugin_runtime_ps_mcp_tool_calls_use_current_auth_manager_token( .build() .await?; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); - let plugins_manager = codex_core_plugins::PluginsManager::new(home.path().to_path_buf()); + let plugins_manager = plugins_manager_for_config(&config); let mcp_config = Arc::new(config.to_mcp_config(&plugins_manager).await); let runtime = McpRuntime::new(McpRuntimeInput { startup_policy: McpStartupPolicy::Eager, diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index 563441abe8..c50d8d5e0c 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -504,6 +504,111 @@ async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> Ok(()) } +#[test_case("CHATGPT", false, None; "product restricted skill is unavailable")] +#[test_case("CODEX", true, Some("native review skill"); "native skill wins over migrated command")] +#[test_case("CHATGPT", true, Some("migrated review command"); "migrated command replaces filtered native skill")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plugin_skill_product_policy_and_migrated_command_precedence_reach_agent_turns( + native_skill_product: &str, + include_migrated_command: bool, + expected_skill_description: Option<&str>, +) -> 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; + let codex_home = Arc::new(TempDir::new()?); + let plugin_root = write_sample_plugin_manifest_and_config(codex_home.as_ref()); + let native_skill_dir = plugin_root.join("skills/review"); + std::fs::create_dir_all(native_skill_dir.join("agents"))?; + std::fs::write( + native_skill_dir.join("SKILL.md"), + "---\nname: source-command-review\ndescription: native review skill\n---\n", + )?; + std::fs::write( + native_skill_dir.join("agents/openai.yaml"), + format!("policy:\n products: [{native_skill_product}]\n"), + )?; + if include_migrated_command { + let migrated_skill_dir = + plugin_root.join(".codex-plugin/migrated-command-skills/source-command-review"); + std::fs::create_dir_all(&migrated_skill_dir)?; + std::fs::write( + migrated_skill_dir.join("SKILL.md"), + "---\nname: source-command-review\ndescription: migrated review command\n---\n", + )?; + } + + let mut builder = test_codex() + .with_home(Arc::clone(&codex_home)) + .with_extensions(skills_extensions()); + let test = builder.build_with_auto_env(&server).await?; + let plugin_outcome = test + .thread_manager + .plugins_manager() + .plugins_for_config(&test.config.plugins_config_input()) + .await; + assert_eq!( + plugin_outcome + .plugins() + .iter() + .map(|plugin| (plugin.config_name.as_str(), plugin.has_enabled_skills)) + .collect::>(), + vec![( + SAMPLE_PLUGIN_CONFIG_NAME, + expected_skill_description.is_some() + )] + ); + assert_eq!( + plugin_outcome + .capability_summaries() + .iter() + .map(|plugin| (plugin.config_name.as_str(), plugin.has_skills)) + .collect::>(), + expected_skill_description + .map(|_| (SAMPLE_PLUGIN_CONFIG_NAME, true)) + .into_iter() + .collect::>() + ); + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "Inspect the available plugin skills.".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let developer_text = response + .single_request() + .message_input_texts("developer") + .join("\n"); + assert_eq!( + ( + developer_text.contains("sample:source-command-review: native review skill"), + developer_text.contains("sample:source-command-review: migrated review command"), + ), + ( + expected_skill_description == Some("native review skill"), + expected_skill_description == Some("migrated review command"), + ) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs b/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs index 80e0a313c8..6f15958f4d 100644 --- a/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs +++ b/codex-rs/ext/mcp/tests/hosted_apps_mcp.rs @@ -4,7 +4,7 @@ use codex_config::McpServerTransportConfig; use codex_core::McpManager; use codex_core::config::Config; use codex_core::config::ConfigBuilder; -use codex_core_plugins::PluginsManager; +use codex_core::plugins_manager_for_config; use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributionContext; @@ -87,9 +87,7 @@ async fn default_fallback_overwrites_reserved_config_without_an_extension() -> T .build() .await?; let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let manager = McpManager::new(Arc::new(PluginsManager::new( - config.codex_home.to_path_buf(), - ))); + let manager = McpManager::new(Arc::new(plugins_manager_for_config(&config))); let servers = manager.effective_servers(&config, Some(&auth)).await; let server = servers @@ -118,7 +116,7 @@ async fn later_extension_can_remove_same_name_registration() -> TestResult { codex_mcp_extension::install(&mut builder); builder.mcp_server_contributor(Arc::new(RemoveCodexApps)); let manager = McpManager::new_with_extensions( - Arc::new(PluginsManager::new(config.codex_home.to_path_buf())), + Arc::new(plugins_manager_for_config(&config)), Arc::new(builder.build()), codex_core::CodexAppsToolsCache::default(), ); @@ -164,9 +162,7 @@ async fn disabled_apps_remove_reserved_server_config_for_all_hosts() -> TestResu .await?; let managers = [ installed_manager(&config), - McpManager::new(Arc::new(PluginsManager::new( - config.codex_home.to_path_buf(), - ))), + McpManager::new(Arc::new(plugins_manager_for_config(&config))), ]; for manager in managers { let servers = manager.runtime_servers(&config).await; @@ -179,7 +175,7 @@ fn installed_manager(config: &Config) -> McpManager { let mut builder = ExtensionRegistryBuilder::new(); codex_mcp_extension::install(&mut builder); McpManager::new_with_extensions( - Arc::new(PluginsManager::new(config.codex_home.to_path_buf())), + Arc::new(plugins_manager_for_config(config)), Arc::new(builder.build()), codex_core::CodexAppsToolsCache::default(), ) diff --git a/codex-rs/ext/skills/src/host_service.rs b/codex-rs/ext/skills/src/host_service.rs index f7e5a1d27f..ab375a1ad3 100644 --- a/codex-rs/ext/skills/src/host_service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -13,8 +13,6 @@ use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; -use codex_utils_plugins::SkillDiscoveryMode; -use futures::StreamExt; use tokio::sync::OnceCell; use tokio::sync::Semaphore; use tracing::info; @@ -22,27 +20,23 @@ use tracing::instrument; use tracing::warn; use codex_config::SkillsConfig; -use codex_core_skills::PluginSkillSnapshots; -use codex_core_skills::SkillError; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::config_rules::SkillConfigRules; use codex_core_skills::config_rules::resolve_disabled_skill_paths; use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS; use codex_core_skills::loader::SkillRoot; -use codex_core_skills::loader::SkillRootSnapshot; -use codex_core_skills::loader::load_skill_root_snapshot; use codex_skills::LoadedSkills; use codex_skills::SkillLoadFuture; use codex_skills::SkillRootLoadRequest; use codex_skills::SkillRootLoader; +use codex_skills::SkillRootSnapshots; use codex_skills::install_system_skills; use crate::HostSkillsSnapshot; use crate::host_roots::resolve_skill_roots; use crate::loader::HostSkillRoot; use crate::loader::load_and_merge_host_skill_roots; -use crate::loader::load_host_skill_root; #[derive(Debug, Clone)] pub struct HostSkillsLoadInput { @@ -50,7 +44,7 @@ pub struct HostSkillsLoadInput { pub effective_skill_roots: Vec, pub config_layer_stack: ConfigLayerStack, pub bundled_skills_enabled: bool, - plugin_skill_snapshots: Option, + plugin_skill_snapshots: Option>, } impl HostSkillsLoadInput { @@ -72,7 +66,7 @@ impl HostSkillsLoadInput { /// Attaches plugin skill snapshots parsed during plugin loading, when available. pub fn with_plugin_skill_snapshots( mut self, - plugin_skill_snapshots: Option, + plugin_skill_snapshots: Option>, ) -> Self { self.plugin_skill_snapshots = plugin_skill_snapshots; self @@ -294,58 +288,37 @@ impl HostSkillsService { roots: Vec, skill_config_rules: &SkillConfigRules, ) -> SkillLoadOutcome { - let plugin_skill_snapshots = input.plugin_skill_snapshots.as_ref(); - let mut indexed_snapshots = futures::stream::iter(roots.into_iter().enumerate()) - .map(|(root_index, root)| async move { - let _root_scan_slot = self - .root_scan_slots - .acquire() - .await - .unwrap_or_else(|_| unreachable!()); - let use_legacy_loader = root.plugin_identity.is_some() - || root.plugin_namespace.is_some() - || root.plugin_root.is_some() - || root.discovery_mode != SkillDiscoveryMode::Recursive; - let snapshot = if use_legacy_loader { - load_skill_root_snapshot(root, plugin_skill_snapshots).await - } else { - let snapshot = load_host_skill_root(HostSkillRoot::host( - root.path, - root.scope, - root.file_system, - )) - .await; - SkillRootSnapshot::new( - snapshot.root, - snapshot.skills, - snapshot.skill_discovery_path_by_path, - snapshot - .errors - .into_iter() - .map(|error| SkillError { - path: error.path, - message: error.message, - }) - .collect(), - snapshot.file_system, - ) - }; - (root_index, snapshot) + let roots = roots + .into_iter() + .map(|root| { + match ( + root.plugin_identity, + root.plugin_namespace, + root.plugin_root, + ) { + (Some(plugin_identity), Some(plugin_namespace), Some(plugin_root)) => { + HostSkillRoot::plugin( + PluginSkillRoot { + path: root.path, + plugin_identity, + plugin_namespace, + plugin_root, + discovery_mode: root.discovery_mode, + }, + root.file_system, + ) + } + _ => HostSkillRoot::host(root.path, root.scope, root.file_system), + } }) - .buffer_unordered(MAX_CONCURRENT_ROOT_SCANS) - .collect::>() - .await; - indexed_snapshots.sort_unstable_by_key(|(root_index, _)| *root_index); - let outcome = SkillLoadOutcome::from_root_snapshots( - indexed_snapshots - .into_iter() - .map(|(_, snapshot)| snapshot) - .collect(), - ); - let outcome = codex_core_skills::filter_skill_load_outcome_for_product( - outcome, + .collect(); + let outcome = load_and_merge_host_skill_roots( + roots, + &self.root_scan_slots, self.restriction_product, - ); + input.plugin_skill_snapshots.as_ref(), + ) + .await; let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules); outcome.with_disabled_paths(disabled_paths) } @@ -441,7 +414,7 @@ impl SkillRootLoader for HostSkillsService { struct ConfigSkillsCacheKey { roots: Vec, skill_config_rules: SkillConfigRules, - plugin_skill_snapshots: Option, + plugin_skill_snapshots: Option>, } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -495,7 +468,7 @@ pub fn bundled_skills_enabled_from_stack( fn config_skills_cache_key( roots: &[SkillRoot], skill_config_rules: &SkillConfigRules, - plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + plugin_skill_snapshots: Option<&SkillRootSnapshots>, ) -> ConfigSkillsCacheKey { ConfigSkillsCacheKey { roots: roots diff --git a/codex-rs/ext/skills/src/host_service_tests.rs b/codex-rs/ext/skills/src/host_service_tests.rs index 2c5559d7ca..e650b27b34 100644 --- a/codex-rs/ext/skills/src/host_service_tests.rs +++ b/codex-rs/ext/skills/src/host_service_tests.rs @@ -5,6 +5,9 @@ use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirementsToml; use codex_exec_server::LOCAL_FS; +use codex_skills::LoadedSkillRoot; +use codex_skills::SkillRootSnapshotCache; +use codex_skills::SkillRootSnapshots; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::PathExt; @@ -12,13 +15,34 @@ use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; use pretty_assertions::assert_eq; +use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use std::sync::Mutex; use tempfile::TempDir; +#[derive(Default)] +struct TestPluginSkillSnapshotCache { + snapshots: Mutex>, +} + +impl SkillRootSnapshotCache for TestPluginSkillSnapshotCache { + fn get(&self, root: &PluginSkillRoot) -> Option { + self.snapshots.lock().unwrap().get(root).cloned() + } + + fn insert(&self, root: PluginSkillRoot, snapshot: LoadedSkillRoot) { + self.snapshots.lock().unwrap().insert(root, snapshot); + } +} + +fn test_plugin_skill_snapshots() -> SkillRootSnapshots { + SkillRootSnapshots::new(Arc::new(TestPluginSkillSnapshotCache::default())) +} + fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) { let skill_dir = codex_home.path().join("skills").join(dir); fs::create_dir_all(&skill_dir).unwrap(); @@ -307,7 +331,7 @@ async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() { config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ) - .with_plugin_skill_snapshots(Some(PluginSkillSnapshots::for_plugin_load())); + .with_plugin_skill_snapshots(Some(test_plugin_skill_snapshots())); let plugin_snapshot = skills_service .snapshot_for_config(&plugin_input, Some(Arc::clone(&LOCAL_FS))) .await; @@ -717,7 +741,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { ); let config_input = base_input .clone() - .with_plugin_skill_snapshots(Some(PluginSkillSnapshots::for_plugin_load())); + .with_plugin_skill_snapshots(Some(test_plugin_skill_snapshots())); let (config_snapshot, snapshot_a) = tokio::join!( skills_service.snapshot_for_config(&config_input, Some(Arc::clone(&LOCAL_FS))), skills_service.snapshot_for_cwd( diff --git a/codex-rs/external-agent-migration/src/detect/mod.rs b/codex-rs/external-agent-migration/src/detect/mod.rs index 009e7ef9ef..461e793eda 100644 --- a/codex-rs/external-agent-migration/src/detect/mod.rs +++ b/codex-rs/external-agent-migration/src/detect/mod.rs @@ -25,7 +25,7 @@ use crate::utils::invalid_data_error; use crate::utils::is_missing_or_empty_text_file; use codex_config::types::PluginConfig; use codex_core::config::ConfigBuilder; -use codex_core_plugins::PluginsManager; +use codex_core::plugins_manager_for_config; use std::collections::HashMap; use std::collections::HashSet; use std::fs; @@ -351,7 +351,7 @@ impl ExternalAgentConfigService { .unwrap_or_default(); let configured_marketplace_plugins = configured_marketplace_plugins( &config, - &PluginsManager::new(self.codex_home.clone()), + &plugins_manager_for_config(&config), )?; let source_root = repo_root.unwrap_or(self.external_agent_home.as_path()); if let Some(detected) = diff --git a/codex-rs/external-agent-migration/src/plugins.rs b/codex-rs/external-agent-migration/src/plugins.rs index 27eb5393df..46524e7482 100644 --- a/codex-rs/external-agent-migration/src/plugins.rs +++ b/codex-rs/external-agent-migration/src/plugins.rs @@ -1,8 +1,8 @@ use codex_analytics::PluginInstallSource; use codex_core::config::ConfigBuilder; +use codex_core::plugins_manager_for_config; use codex_core_plugins::PluginInstallError; use codex_core_plugins::PluginInstallRequest; -use codex_core_plugins::PluginsManager; use codex_core_plugins::marketplace::MarketplaceError; use codex_core_plugins::marketplace::find_marketplace_manifest_path; use codex_core_plugins::marketplace_add::MarketplaceAddRequest; @@ -101,7 +101,7 @@ impl ExternalAgentConfigService { .map_err(|err| io::Error::other(format!("failed to load config: {err}")))?; let requirements = config.config_layer_stack.requirements().clone(); let mut outcome = PluginImportOutcome::default(); - let plugins_manager = PluginsManager::new(self.codex_home.clone()) + let plugins_manager = plugins_manager_for_config(&config) .with_plugin_install_source(PluginInstallSource::ExternalAgentMigration); if let Some(analytics_events_client) = self.analytics_events_client.clone() { plugins_manager.set_analytics_events_client(analytics_events_client);