From a9d59d8b8e4bcafc8425bee7c373cb8394a85e9b Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Fri, 7 Aug 2026 14:10:23 +0000 Subject: [PATCH] Respect plugin skill availability in tool suggestions (#37447) ## Why Plugin suggestions should report skills only when the plugin has an enabled skill. ## What changed - Load suggestion metadata through the shared plugin skill-root loader. - Apply skill configuration when setting `has_skills` on suggestion candidates. ## Testing Added coverage for local plugin suggestions with the plugin skill both enabled and disabled. GitOrigin-RevId: bceb14528c9042b474eaad6efaf919bef72e0ca4 --- codex-rs/core-plugins/src/manager.rs | 2 +- .../core-plugins/src/tool_suggest_metadata.rs | 14 ++- .../tests/suite/request_plugin_install.rs | 115 ++++++++++++++++++ 3 files changed, 124 insertions(+), 7 deletions(-) diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 098e416f3c..079514cc79 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1860,7 +1860,7 @@ impl PluginsManager { marketplace_name, plugin, self.restriction_product, - Arc::clone(&self.skill_root_scan_slots), + self.skill_root_loader.as_ref(), ) .await?; Ok(fragment.project(skill_config_rules, self.auth_mode())) diff --git a/codex-rs/core-plugins/src/tool_suggest_metadata.rs b/codex-rs/core-plugins/src/tool_suggest_metadata.rs index fbedcb5707..91bdcdb365 100644 --- a/codex-rs/core-plugins/src/tool_suggest_metadata.rs +++ b/codex-rs/core-plugins/src/tool_suggest_metadata.rs @@ -11,14 +11,16 @@ use codex_plugin::prompt_safe_plugin_description; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::Product; use codex_skills::SkillConfigRules; +use codex_skills::SkillRootLoader; use codex_utils_plugins::PluginIdentity; +use codex_utils_plugins::PluginSkillRoot; use tokio::sync::Semaphore; use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::loader::PluginSkillInventory; use crate::loader::load_plugin_apps; use crate::loader::load_plugin_mcp_servers; -use crate::loader::load_plugin_skill_inventory; +use crate::loader::load_plugin_skill_inventory_with_loader; use crate::manager::ConfiguredMarketplacePlugin; use crate::manager::remote_plugin_install_required_description; use crate::manifest::PluginManifestFormat; @@ -122,7 +124,7 @@ impl ToolSuggestMetadataCache { marketplace_name: &str, plugin: &ConfiguredMarketplacePlugin, restriction_product: Option, - root_scan_slots: Arc, + skill_root_loader: &dyn SkillRootLoader, ) -> Result, MarketplaceError> { let artifact = PluginArtifactIdentity { plugin_id: plugin.id.clone(), @@ -147,7 +149,7 @@ impl ToolSuggestMetadataCache { marketplace_name, plugin, restriction_product, - Arc::clone(&root_scan_slots), + skill_root_loader, ) .await; if self.cache_entry_if_current(generation, artifact.clone(), entry.clone()) { @@ -197,7 +199,7 @@ async fn load_plugin_metadata( marketplace_name: &str, plugin: &ConfiguredMarketplacePlugin, restriction_product: Option, - root_scan_slots: Arc, + skill_root_loader: &dyn SkillRootLoader, ) -> ToolSuggestMetadataEntry { let plugin_id = PluginId::new(plugin.name.clone(), marketplace_name.to_string()).map_err( |err| match err { @@ -228,14 +230,14 @@ async fn load_plugin_metadata( remote_plugin_id: None, }; let manifest = loaded_manifest.manifest; - let skill_inventory = load_plugin_skill_inventory( + let skill_inventory = load_plugin_skill_inventory_with_loader( plugin_root, &plugin_identity, &manifest, loaded_manifest.format, restriction_product, /*plugin_skill_snapshots*/ None, - root_scan_slots, + skill_root_loader, ) .await; let mut mcp_server_names = diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index 95eb37aafb..f833071bcd 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -7,6 +7,7 @@ use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverable; use codex_config::types::ToolSuggestDiscoverableType; use codex_core::config::Config; +use codex_core_plugins::startup_sync::curated_plugins_repo_path; use codex_features::Feature; use codex_login::CodexAuth; use codex_models_manager::bundled_models_response; @@ -46,6 +47,8 @@ use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Duration; use std::time::Instant; +use tempfile::TempDir; +use test_case::test_case; use wiremock::Mock; use wiremock::MockGuard; use wiremock::MockServer; @@ -548,6 +551,118 @@ async fn explicit_false_preserves_legacy_workflow() -> Result<()> { Ok(()) } +#[test_case(true; "enabled plugin skill")] +#[test_case(false; "disabled plugin skill")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn local_plugin_skill_availability_reaches_tool_suggestion_candidates( + skill_enabled: bool, +) -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let codex_home = Arc::new(TempDir::new()?); + let curated_root = curated_plugins_repo_path(codex_home.path()); + let plugin_root = curated_root.join("plugins/sample"); + std::fs::create_dir_all(curated_root.join(".agents/plugins"))?; + std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + std::fs::create_dir_all(plugin_root.join("skills/search"))?; + std::fs::write( + curated_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "openai-curated", + "plugins": [{ + "name": "sample", + "source": {"source": "local", "path": "./plugins/sample"} + }] +}"#, + )?; + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample","description":"Search sample data"}"#, + )?; + std::fs::write( + plugin_root.join("skills/search/SKILL.md"), + "---\nname: search\ndescription: Search sample data\n---\n", + )?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + "[features]\nplugins = true\n\n[[skills.config]]\nname = \"sample:search\"\nenabled = {skill_enabled}\n" + ), + )?; + + let call_id = "list-skill-backed-plugin-candidates"; + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME, "{}"), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + let mut builder = test_codex() + .with_home(codex_home) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config({ + let apps_base_url = apps_server.chatgpt_base_url.clone(); + move |config| { + config + .permissions + .set_permission_profile(PermissionProfile::Disabled) + .expect("test config should allow disabled permissions"); + configure_apps_without_search_tool(config, apps_base_url.as_str()); + config + .features + .disable(Feature::RemotePlugin) + .expect("test config should allow local plugin suggestions"); + config.tool_suggest.discoverables = vec![ToolSuggestDiscoverable { + kind: ToolSuggestDiscoverableType::Plugin, + id: "sample@openai-curated".to_string(), + }]; + } + }); + let test = builder.build_with_auto_env(&server).await?; + test.submit_turn_with_approval_and_permission_profile( + "list available plugins", + AskForApproval::Never, + PermissionProfile::Disabled, + ) + .await?; + + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + assert_legacy_tools(&requests[0].body_json()); + let output: Value = serde_json::from_str( + &requests[1] + .function_call_output_text(call_id) + .expect("list tool output"), + )?; + assert_eq!( + output, + json!({ + "tools": [{ + "id": "sample@openai-curated", + "name": "sample", + "description": "Search sample data", + "tool_type": "plugin", + "has_skills": skill_enabled, + "mcp_server_names": [], + "app_connector_ids": [] + }] + }) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn endpoint_mode_injects_candidates_hides_list_and_rejects_invented_ids() -> Result<()> { skip_if_no_network!(Ok(()));