From d90859dcb375309e901267c9467e228c157c8028 Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Wed, 17 Jun 2026 20:59:47 +0000 Subject: [PATCH] Preserve curated plugin root selection --- codex-rs/core-plugins/src/loader.rs | 35 +++----------- codex-rs/core-plugins/src/loader_tests.rs | 55 ++++++++++++---------- codex-rs/core-plugins/src/manager_tests.rs | 5 +- 3 files changed, 37 insertions(+), 58 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index cbd9f55627..bf52d3c88b 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,7 +1,6 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; use crate::is_openai_curated_marketplace_name; -use crate::manifest::PluginManifest; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; @@ -657,7 +656,7 @@ async fn load_plugin( let active_plugin_root = plugin_id .as_ref() .ok() - .and_then(|plugin_id| active_plugin_root(plugin_id, store)); + .and_then(|plugin_id| store.active_plugin_root(plugin_id)); let root = active_plugin_root .clone() .unwrap_or_else(|| match &plugin_id { @@ -709,7 +708,7 @@ async fn load_plugin( return loaded_plugin; }; - loaded_plugin.is_first_party = is_openai_curated_plugin(&loaded_plugin_id, &manifest, store); + loaded_plugin.is_first_party = is_openai_curated_plugin(&loaded_plugin_id, &plugin_root, store); let manifest_paths = &manifest.paths; match scope { PluginLoadScope::AllCapabilities { @@ -751,31 +750,12 @@ async fn load_plugin( loaded_plugin } -fn active_plugin_root(plugin_id: &PluginId, store: &PluginStore) -> Option { - if is_openai_curated_marketplace_name(&plugin_id.marketplace_name) - && let Some(curated_sha) = read_curated_plugins_sha(store.codex_home()) - { - let expected_version = curated_plugin_cache_version(&curated_sha); - let expected_root = store.plugin_root(plugin_id, &expected_version); - if expected_root.as_path().is_dir() { - return Some(expected_root); - } - } - store.active_plugin_root(plugin_id) -} - fn is_openai_curated_plugin( plugin_id: &PluginId, - manifest: &PluginManifest, + loaded_root: &AbsolutePathBuf, store: &PluginStore, ) -> bool { - if !is_openai_curated_marketplace_name(&plugin_id.marketplace_name) - || manifest - .interface - .as_ref() - .and_then(|interface| interface.developer_name.as_deref()) - != Some("OpenAI") - { + if !is_openai_curated_marketplace_name(&plugin_id.marketplace_name) { return false; } @@ -783,11 +763,8 @@ fn is_openai_curated_plugin( return false; }; let expected_version = curated_plugin_cache_version(&curated_sha); - if !store - .plugin_root(plugin_id, &expected_version) - .as_path() - .is_dir() - { + let expected_root = store.plugin_root(plugin_id, &expected_version); + if loaded_root != &expected_root || !expected_root.as_path().is_dir() { return false; } diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 48b988bf41..fba34ebd35 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -248,21 +248,14 @@ enabled = true .expect("valid curated plugin config") } -fn write_curated_cached_plugin_with_developer( - codex_home: &Path, - plugin_name: &str, - version: &str, - developer_name: &str, -) { +fn write_curated_cached_plugin(codex_home: &Path, plugin_name: &str, version: &str) { let plugin_root = codex_home .join("plugins/cache/openai-curated") .join(plugin_name) .join(version); write_file( &plugin_root.join(".codex-plugin/plugin.json"), - &format!( - r#"{{"name":"{plugin_name}","interface":{{"developerName":"{developer_name}"}}}}"#, - ), + &format!(r#"{{"name":"{plugin_name}"}}"#), ); write_file(&plugin_root.join("skills/SKILL.md"), "skill"); } @@ -281,18 +274,12 @@ async fn load_curated_plugin(temp_dir: &TempDir, plugin_name: &str) -> PluginLoa } #[tokio::test] -async fn load_curated_plugin_prefers_synced_sha_root_over_stale_local_cache() { +async fn load_curated_plugin_uses_synced_sha_root_when_it_is_only_installed_root() { let temp_dir = TempDir::new().expect("tempdir"); let curated_root = curated_plugins_repo_path(temp_dir.path()); write_openai_curated_marketplace(&curated_root, &["slack"]); write_curated_plugin_sha_with(temp_dir.path(), TEST_CURATED_PLUGIN_SHA); - write_curated_cached_plugin_with_developer(temp_dir.path(), "slack", "local", "Third Party"); - write_curated_cached_plugin_with_developer( - temp_dir.path(), - "slack", - TEST_CURATED_PLUGIN_CACHE_VERSION, - "OpenAI", - ); + write_curated_cached_plugin(temp_dir.path(), "slack", TEST_CURATED_PLUGIN_CACHE_VERSION); let outcome = load_curated_plugin(&temp_dir, "slack").await; let plugin = outcome.plugins().first().expect("configured plugin"); @@ -313,12 +300,12 @@ async fn load_curated_plugin_prefers_synced_sha_root_over_stale_local_cache() { } #[tokio::test] -async fn load_curated_plugin_with_openai_metadata_without_synced_cache_is_not_first_party() { +async fn load_curated_plugin_without_synced_cache_root_is_not_first_party() { let temp_dir = TempDir::new().expect("tempdir"); let curated_root = curated_plugins_repo_path(temp_dir.path()); write_openai_curated_marketplace(&curated_root, &["slack"]); write_curated_plugin_sha_with(temp_dir.path(), TEST_CURATED_PLUGIN_SHA); - write_curated_cached_plugin_with_developer(temp_dir.path(), "slack", "local", "OpenAI"); + write_curated_cached_plugin(temp_dir.path(), "slack", "local"); let outcome = load_curated_plugin(&temp_dir, "slack").await; @@ -332,18 +319,36 @@ async fn load_curated_plugin_with_openai_metadata_without_synced_cache_is_not_fi assert_eq!(outcome.effective_first_party_plugin_roots(), Vec::new()); } +#[tokio::test] +async fn load_curated_plugin_keeps_local_root_priority_over_synced_sha_root() { + let temp_dir = TempDir::new().expect("tempdir"); + let curated_root = curated_plugins_repo_path(temp_dir.path()); + write_openai_curated_marketplace(&curated_root, &["slack"]); + write_curated_plugin_sha_with(temp_dir.path(), TEST_CURATED_PLUGIN_SHA); + write_curated_cached_plugin(temp_dir.path(), "slack", "local"); + write_curated_cached_plugin(temp_dir.path(), "slack", TEST_CURATED_PLUGIN_CACHE_VERSION); + + let outcome = load_curated_plugin(&temp_dir, "slack").await; + let plugin = outcome.plugins().first().expect("configured plugin"); + let local_root = AbsolutePathBuf::try_from( + temp_dir + .path() + .join("plugins/cache/openai-curated/slack/local"), + ) + .expect("local plugin root"); + + assert_eq!(plugin.root, local_root); + assert!(!plugin.is_first_party); + assert_eq!(outcome.effective_first_party_plugin_roots(), Vec::new()); +} + #[tokio::test] async fn load_curated_plugin_missing_from_synced_marketplace_is_not_first_party() { let temp_dir = TempDir::new().expect("tempdir"); let curated_root = curated_plugins_repo_path(temp_dir.path()); write_openai_curated_marketplace(&curated_root, &["github"]); write_curated_plugin_sha_with(temp_dir.path(), TEST_CURATED_PLUGIN_SHA); - write_curated_cached_plugin_with_developer( - temp_dir.path(), - "slack", - TEST_CURATED_PLUGIN_CACHE_VERSION, - "OpenAI", - ); + write_curated_cached_plugin(temp_dir.path(), "slack", TEST_CURATED_PLUGIN_CACHE_VERSION); let outcome = load_curated_plugin(&temp_dir, "slack").await; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 5ce9770e04..bcc46bbe2c 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -574,10 +574,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { &plugin_root.join(".codex-plugin/plugin.json"), r#"{ "name": "sample", - "description": "Plugin that includes the sample MCP server and Skills", - "interface": { - "developerName": "OpenAI" - } + "description": "Plugin that includes the sample MCP server and Skills" }"#, ); write_file(