mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
@@ -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()))
|
||||
|
||||
@@ -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<Product>,
|
||||
root_scan_slots: Arc<Semaphore>,
|
||||
skill_root_loader: &dyn SkillRootLoader<PluginSkillRoot>,
|
||||
) -> Result<Arc<ToolSuggestMetadataFragment>, 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<Product>,
|
||||
root_scan_slots: Arc<Semaphore>,
|
||||
skill_root_loader: &dyn SkillRootLoader<PluginSkillRoot>,
|
||||
) -> 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 =
|
||||
|
||||
@@ -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(()));
|
||||
|
||||
Reference in New Issue
Block a user