From 07fd04abb199fa4c3a1530873ec69e938061a615 Mon Sep 17 00:00:00 2001 From: jameswt-oai Date: Fri, 24 Jul 2026 23:31:40 +0000 Subject: [PATCH] Propagate remote plugin IDs to skill metadata (#35261) ## What changed - Carry a plugin's local and remote identities together from plugin loading into `SkillMetadata`. - Resolve remote IDs from the installed-plugin snapshot when available, falling back to persisted install metadata only when no snapshot exists. - Include plugin identity in skill cache keys so identity changes refresh cached skill metadata. ## Testing - Cover snapshot and persisted identity resolution, local marketplace isolation, cached skill refreshes, and propagation through plugin skill snapshots. GitOrigin-RevId: aabeeb631a43361fe817358ace7f1ea8ba5db708 --- codex-rs/app-server/src/skills_watcher.rs | 2 +- codex-rs/core-plugins/src/lib.rs | 1 + codex-rs/core-plugins/src/loader.rs | 77 +++++++-- codex-rs/core-plugins/src/loader_tests.rs | 2 +- codex-rs/core-plugins/src/manager.rs | 70 ++++---- codex-rs/core-plugins/src/manager_tests.rs | 159 +++++++++++++----- codex-rs/core-plugins/src/remote.rs | 2 +- .../src/remote_plugin_id_resolver.rs | 65 +++++++ .../src/script_attribution_tests.rs | 1 + codex-rs/core-plugins/src/store.rs | 88 ++++++---- .../core-plugins/src/tool_suggest_metadata.rs | 7 +- codex-rs/core-skills/src/injection_tests.rs | 1 + .../core-skills/src/invocation_utils_tests.rs | 1 + codex-rs/core-skills/src/loader.rs | 27 +-- codex-rs/core-skills/src/loader_tests.rs | 88 ++++++++-- codex-rs/core-skills/src/render.rs | 1 + codex-rs/core-skills/src/root_loader.rs | 6 +- codex-rs/core-skills/src/service.rs | 21 ++- codex-rs/core-skills/src/service_tests.rs | 54 +++++- .../core-skills/tests/environment_loader.rs | 4 +- codex-rs/core/src/session/tests.rs | 7 + .../ext/skills/src/provider/host_tests.rs | 2 +- .../tests/executor_file_system_authority.rs | 2 +- codex-rs/ext/skills/tests/skills_extension.rs | 3 +- codex-rs/plugin/src/load_outcome.rs | 17 +- codex-rs/plugin/src/plugin_id.rs | 2 +- codex-rs/skills/src/model.rs | 1 + codex-rs/utils/plugins/src/lib.rs | 9 +- 28 files changed, 550 insertions(+), 170 deletions(-) create mode 100644 codex-rs/core-plugins/src/remote_plugin_id_resolver.rs diff --git a/codex-rs/app-server/src/skills_watcher.rs b/codex-rs/app-server/src/skills_watcher.rs index 637fe963ea98..4852394fef0b 100644 --- a/codex-rs/app-server/src/skills_watcher.rs +++ b/codex-rs/app-server/src/skills_watcher.rs @@ -115,7 +115,7 @@ impl SkillsWatcher { .await .into_iter() // Plugin roots are invalidated by plugin lifecycle operations. - .filter(|root| root.plugin_id.is_none()) + .filter(|root| root.plugin_identity.is_none()) .map(|root| WatchPath { path: root.path.into_path_buf(), recursive: true, diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index e7d33ea212d1..5ab8f7f71ede 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -17,6 +17,7 @@ mod provider; pub mod remote; pub mod remote_bundle; pub mod remote_legacy; +mod remote_plugin_id_resolver; mod script_attribution; pub mod startup_sync; pub mod store; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 727c8cb4d5f2..f47e4d602336 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -15,6 +15,8 @@ use crate::marketplace_policy::configured_plugins_from_stack; use crate::npm_source::materialize_npm_plugin_source; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemoteInstalledPlugin; +use crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot; +use crate::remote_plugin_id_resolver::RemotePluginIdResolver; use crate::store::PluginStore; use crate::store::plugin_version_for_source; use crate::store::plugin_version_for_source_with_fallback_manifest; @@ -45,6 +47,7 @@ use codex_protocol::protocol::SkillScope; use codex_skills::SkillConfigRules; use codex_skills::SkillMetadata; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::SkillDiscoveryMode; use codex_utils_plugins::find_plugin_manifest_path; use serde_json::Value as JsonValue; @@ -78,6 +81,7 @@ enum PluginLoadScope<'a> { restriction_product: Option, skill_config_rules: &'a SkillConfigRules, plugin_skill_snapshots: Option<&'a PluginSkillSnapshots>, + remote_plugin_id_resolver: &'a RemotePluginIdResolver, root_scan_slots: Arc, }, HooksOnly, @@ -117,7 +121,7 @@ pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin]) #[instrument(level = "trace", skip_all)] pub(crate) async fn load_plugins_from_layer_stack( config_layer_stack: &ConfigLayerStack, - extra_plugins: HashMap, + remote_installed_plugins_snapshot: RemoteInstalledPluginsSnapshot, store: &PluginStore, plugin_skill_snapshots: Option<&PluginSkillSnapshots>, restriction_product: Option, @@ -125,6 +129,10 @@ pub(crate) async fn load_plugins_from_layer_stack( root_scan_slots: Arc, ) -> Vec> { let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); + let RemoteInstalledPluginsSnapshot { + configs: extra_plugins, + remote_plugin_id_resolver, + } = remote_installed_plugins_snapshot; load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, @@ -134,6 +142,7 @@ pub(crate) async fn load_plugins_from_layer_stack( restriction_product, skill_config_rules: &skill_config_rules, plugin_skill_snapshots, + remote_plugin_id_resolver: &remote_plugin_id_resolver, root_scan_slots, }, ) @@ -753,18 +762,20 @@ async fn load_plugin( scope: &PluginLoadScope<'_>, ) -> LoadedPlugin { let plugin_id = PluginId::parse(&config_name); - let active_plugin_root = plugin_id + let active_plugin_installation = plugin_id .as_ref() .ok() - .and_then(|plugin_id| store.active_plugin_root(plugin_id)); - let root = active_plugin_root - .clone() + .and_then(|plugin_id| store.active_plugin_installation(plugin_id)); + let root = active_plugin_installation + .as_ref() + .map(|installation| installation.root.clone()) .unwrap_or_else(|| match &plugin_id { Ok(plugin_id) => store.plugin_base_root(plugin_id), Err(_) => store.root().clone(), }); let mut loaded_plugin = LoadedPlugin { config_name, + remote_plugin_id: None, manifest_name: None, plugin_namespace: None, manifest_description: None, @@ -784,13 +795,13 @@ async fn load_plugin( return loaded_plugin; } - let (loaded_plugin_id, plugin_root) = match plugin_id { + let (loaded_plugin_id, installation) = match plugin_id { Ok(plugin_id) => { - let Some(plugin_root) = active_plugin_root else { + let Some(installation) = active_plugin_installation else { loaded_plugin.error = Some("plugin is not installed".to_string()); return loaded_plugin; }; - (plugin_id, plugin_root) + (plugin_id, installation) } Err(err) => { loaded_plugin.error = Some(err.to_string()); @@ -798,6 +809,16 @@ async fn load_plugin( } }; + loaded_plugin.remote_plugin_id = match scope { + PluginLoadScope::AllCapabilities { + remote_plugin_id_resolver, + .. + } => remote_plugin_id_resolver.remote_plugin_id_for_installation(&installation), + PluginLoadScope::HooksOnly => None, + }; + + let plugin_root = installation.root; + if !plugin_root.as_path().is_dir() { loaded_plugin.error = Some("path does not exist or is not a directory".to_string()); return loaded_plugin; @@ -815,14 +836,19 @@ async fn load_plugin( restriction_product, skill_config_rules, plugin_skill_snapshots, + remote_plugin_id_resolver: _, root_scan_slots, } => { loaded_plugin.manifest_name = Some(manifest.display_name().to_string()); loaded_plugin.manifest_description = manifest.description.clone(); loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths); - let resolved_skills = load_plugin_skills( + let plugin_identity = PluginIdentity { + plugin_id: loaded_plugin_id.as_key(), + remote_plugin_id: loaded_plugin.remote_plugin_id.clone(), + }; + let resolved_skills = load_plugin_skills_with_identity( &plugin_root, - &loaded_plugin_id, + &plugin_identity, &manifest, *restriction_product, skill_config_rules, @@ -926,10 +952,35 @@ pub async fn load_plugin_skills( skill_config_rules: &SkillConfigRules, plugin_skill_snapshots: Option<&PluginSkillSnapshots>, root_scan_slots: Arc, +) -> ResolvedPluginSkills { + let plugin_identity = PluginIdentity { + plugin_id: plugin_id.as_key(), + remote_plugin_id: None, + }; + load_plugin_skills_with_identity( + plugin_root, + &plugin_identity, + manifest, + restriction_product, + skill_config_rules, + plugin_skill_snapshots, + root_scan_slots, + ) + .await +} + +pub(crate) async fn load_plugin_skills_with_identity( + plugin_root: &AbsolutePathBuf, + plugin_identity: &PluginIdentity, + manifest: &PluginManifest, + restriction_product: Option, + skill_config_rules: &SkillConfigRules, + plugin_skill_snapshots: Option<&PluginSkillSnapshots>, + root_scan_slots: Arc, ) -> ResolvedPluginSkills { load_plugin_skill_inventory( plugin_root, - plugin_id, + plugin_identity, manifest, restriction_product, plugin_skill_snapshots, @@ -941,7 +992,7 @@ pub async fn load_plugin_skills( pub(crate) async fn load_plugin_skill_inventory( plugin_root: &AbsolutePathBuf, - plugin_id: &PluginId, + plugin_identity: &PluginIdentity, manifest: &PluginManifest, restriction_product: Option, plugin_skill_snapshots: Option<&PluginSkillSnapshots>, @@ -953,7 +1004,7 @@ pub(crate) async fn load_plugin_skill_inventory( path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some(plugin_id.as_key()), + plugin_identity: Some(plugin_identity.clone()), plugin_namespace: Some(manifest.name.clone()), plugin_root: Some(plugin_root.clone()), discovery_mode: SkillDiscoveryMode::Recursive, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 022c34855993..cfc5c892daac 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -159,7 +159,7 @@ enabled = true let full = load_plugins_from_layer_stack( &stack, - HashMap::new(), + RemoteInstalledPluginsSnapshot::default(), &store, /*plugin_skill_snapshots*/ None, Some(Product::Codex), diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 8893aa2a4521..69f2490d20ad 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -10,7 +10,7 @@ use crate::loader::load_plugin_apps_from_manifest; use crate::loader::load_plugin_hooks; use crate::loader::load_plugin_hooks_from_layer_stack; use crate::loader::load_plugin_mcp_servers_from_manifest; -use crate::loader::load_plugin_skills; +use crate::loader::load_plugin_skills_with_identity; use crate::loader::load_plugins_from_layer_stack; use crate::loader::log_plugin_load_errors; use crate::loader::materialize_marketplace_plugin_source; @@ -51,6 +51,9 @@ use crate::remote::RemotePluginScope; use crate::remote::RemotePluginServiceConfig; use crate::remote_legacy::RemotePluginFetchError; 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::startup_sync::curated_plugins_api_marketplace_path; use crate::startup_sync::curated_plugins_repo_path; use crate::startup_sync::read_curated_plugins_sha; @@ -91,6 +94,7 @@ use codex_tools::DiscoverablePluginInfo; use codex_tools::DiscoverableTool; use codex_tools::filter_request_plugin_install_discoverable_tools_for_client; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use std::collections::BTreeSet; use std::collections::HashMap; @@ -643,7 +647,7 @@ impl PluginsManager { let plugin_skill_snapshots = PluginSkillSnapshots::for_plugin_load(); let plugins = load_plugins_from_layer_stack( &config.config_layer_stack, - self.remote_installed_plugin_configs(), + self.remote_installed_plugins_snapshot(), &self.store, Some(&plugin_skill_snapshots), self.restriction_product, @@ -729,7 +733,7 @@ impl PluginsManager { } let plugins = load_plugins_from_layer_stack( config_layer_stack, - self.remote_installed_plugin_configs(), + self.remote_installed_plugins_snapshot(), &self.store, /*plugin_skill_snapshots*/ None, self.restriction_product, @@ -818,35 +822,37 @@ impl PluginsManager { remote_installed_plugins_to_config(plugins, &self.store) } - fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option { - let cached_remote_plugin_id = { - let cache = match self.remote_installed_plugins_cache.read() { - Ok(cache) => cache, - Err(err) => err.into_inner(), - }; - cache.as_ref().and_then(|plugins| { - plugins.iter().find_map(|plugin| { - (plugin.name == plugin_id.plugin_name - && plugin.marketplace_name == plugin_id.marketplace_name) - .then(|| plugin.id.clone()) - }) - }) + fn remote_installed_plugins_snapshot(&self) -> RemoteInstalledPluginsSnapshot { + let cache = match self.remote_installed_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), }; - if cached_remote_plugin_id.is_some() { - return cached_remote_plugin_id; + let Some(plugins) = cache.as_ref() else { + return RemoteInstalledPluginsSnapshot::default(); + }; + + RemoteInstalledPluginsSnapshot { + configs: remote_installed_plugins_to_config(plugins, &self.store), + remote_plugin_id_resolver: RemotePluginIdResolver::new(plugins), } + } - match self.store.remote_plugin_id(plugin_id) { - Ok(remote_plugin_id) => remote_plugin_id, - Err(err) => { - tracing::warn!( - plugin_id = %plugin_id.as_key(), - error = %err, - "failed to read persisted remote plugin identity" - ); - None - } + fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option { + let cache = match self.remote_installed_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + if let Some(plugins) = cache.as_ref() { + return plugins.iter().find_map(|plugin| { + (plugin.name == plugin_id.plugin_name + && plugin.marketplace_name == plugin_id.marketplace_name) + .then(|| plugin.id.clone()) + }); } + drop(cache); + + let installation = self.store.active_plugin_installation(plugin_id)?; + persisted_remote_plugin_id_for_installation(&installation) } pub async fn telemetry_metadata_for_installed_plugin( @@ -1958,9 +1964,13 @@ impl PluginsManager { manifest.interface.clone(), marketplace_category, ); - let resolved_skills = load_plugin_skills( + let plugin_identity = PluginIdentity { + plugin_id: plugin_id.as_key(), + remote_plugin_id: self.remote_plugin_id_for(&plugin_id), + }; + let resolved_skills = load_plugin_skills_with_identity( &source_path, - &plugin_id, + &plugin_identity, &manifest, self.restriction_product, &codex_core_skills::config_rules::skill_config_rules_from_stack( diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 2928e72caebb..30a938ccda9b 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -861,6 +861,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { outcome.plugins(), vec![LoadedPlugin { config_name: "sample@test".to_string(), + remote_plugin_id: None, manifest_name: Some("sample".to_string()), plugin_namespace: Some("sample".to_string()), manifest_description: Some( @@ -1139,6 +1140,26 @@ async fn installed_plugin_telemetry_metadata_resolves_persisted_remote_identity( ); } +#[test] +fn plugin_telemetry_ignores_local_marketplace_sidecars() { + let codex_home = TempDir::new().unwrap(); + write_cached_plugin(codex_home.path(), "test", "sample"); + let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse"); + 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()); + + assert_eq!( + manager.telemetry_metadata_for_plugin_id(&plugin_id), + PluginTelemetryMetadata { + plugin_id: Some(plugin_id), + remote_plugin_id: None, + capability_summary: None, + } + ); +} + #[tokio::test] async fn installed_plugin_telemetry_metadata_prefers_remote_snapshot_identity() { let codex_home = TempDir::new().unwrap(); @@ -2292,6 +2313,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions outcome.plugins(), vec![LoadedPlugin { config_name: "sample@test".to_string(), + remote_plugin_id: None, manifest_name: None, plugin_namespace: None, manifest_description: None, @@ -2462,6 +2484,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { }; let plugin = |config_name: &str, dir_name: &str, manifest_name: &str| LoadedPlugin { config_name: config_name.to_string(), + remote_plugin_id: None, manifest_name: Some(manifest_name.to_string()), plugin_namespace: Some( config_name @@ -2648,52 +2671,98 @@ async fn plugin_cache_ignores_unrelated_session_overrides() { } #[tokio::test] -async fn skills_service_reuses_skills_parsed_during_plugin_load() { - let codex_home = TempDir::new().unwrap(); - let codex_home_abs = codex_home.path().to_path_buf().abs(); - let plugin_root = codex_home - .path() - .join("plugins/cache") - .join("test/sample/local"); - write_plugin( - codex_home.path().join("plugins/cache/test").as_path(), - "sample/local", - "sample", - ); - let skill_path = plugin_root.join("skills/SKILL.md"); - write_file(&skill_path, "---\nname: search\ndescription: first\n---\n"); - write_file( - &codex_home.path().join(CONFIG_TOML_FILE), - &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), - ); +async fn skill_snapshots_resolve_remote_plugin_identity_from_authoritative_source() { + let mut duplicate_plugin = remote_installed_plugin("sample"); + duplicate_plugin.id = "plugins~Plugin_duplicate".to_string(); - let config = load_config(codex_home.path(), codex_home.path()).await; - let manager = PluginsManager::new(codex_home.path().to_path_buf()); - let plugin_outcome = manager.plugins_for_config(&config).await; - let plugin_skill_snapshots = manager.plugin_skill_snapshots_for_config(&config); - write_file(&skill_path, "---\nname: search\ndescription: second\n---\n"); - - let skills_input = SkillsLoadInput::new( - codex_home_abs.clone(), - plugin_outcome.effective_plugin_skill_roots(), - config.config_layer_stack.clone(), - /*bundled_skills_enabled*/ false, - ) - .with_plugin_skill_snapshots(plugin_skill_snapshots); - let skills_service = SkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false); - let cached = skills_service - .snapshot_for_config(&skills_input, /*fs*/ None) - .await; + for (installed_plugins, expected_remote_plugin_id) in [ + (None, Some("plugins~Plugin_persisted")), + (Some(Vec::new()), None), + ( + Some(vec![remote_installed_plugin("sample"), duplicate_plugin]), + Some("plugins~Plugin_sample"), + ), + ] { + let codex_home = TempDir::new().unwrap(); + let codex_home_abs = codex_home.path().to_path_buf().abs(); + let plugin_root = codex_home + .path() + .join("plugins/cache/openai-curated-remote/sample/local"); + write_plugin( + codex_home + .path() + .join("plugins/cache/openai-curated-remote") + .as_path(), + "sample/local", + "sample", + ); + let skill_path = plugin_root.join("skills/SKILL.md"); + write_file(&skill_path, "---\nname: search\ndescription: first\n---\n"); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true - assert_eq!( - cached - .outcome() - .skills - .iter() - .map(|skill| skill.description.as_str()) - .collect::>(), - vec!["first"] - ); +[plugins."sample@openai-curated-remote"] +enabled = true +"#, + ); + + let plugin_id = + PluginId::parse("sample@openai-curated-remote").expect("remote plugin id should parse"); + PluginStore::new(codex_home.path().to_path_buf()) + .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(codex_home.path().to_path_buf()); + if let Some(installed_plugins) = installed_plugins { + manager.write_remote_installed_plugins_cache(installed_plugins); + } + + let plugin_outcome = manager.plugins_for_config(&config).await; + assert_eq!( + manager + .telemetry_metadata_for_plugin_id(&plugin_id) + .remote_plugin_id + .as_deref(), + expected_remote_plugin_id + ); + write_file(&skill_path, "---\nname: search\ndescription: second\n---\n"); + + let skills_input = SkillsLoadInput::new( + codex_home_abs.clone(), + plugin_outcome.effective_plugin_skill_roots(), + config.config_layer_stack.clone(), + /*bundled_skills_enabled*/ false, + ) + .with_plugin_skill_snapshots(manager.plugin_skill_snapshots_for_config(&config)); + let skills_service = + SkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false); + let snapshot = skills_service + .snapshot_for_config(&skills_input, /*fs*/ None) + .await; + + assert_eq!( + snapshot + .outcome() + .skills + .iter() + .map(|skill| { + ( + skill.description.as_str(), + skill.plugin_id.as_deref(), + skill.remote_plugin_id.as_deref(), + ) + }) + .collect::>(), + vec![( + "first", + Some("sample@openai-curated-remote"), + expected_remote_plugin_id, + )] + ); + } } #[test] @@ -6030,7 +6099,7 @@ async fn load_plugins_ignores_project_config_files() { let plugins = load_plugins_from_layer_stack( &stack, - std::collections::HashMap::new(), + crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot::default(), &PluginStore::new(codex_home.path().to_path_buf()), /*plugin_skill_snapshots*/ None, Some(Product::Codex), diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 1348a47ce65c..f913e0586b99 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -482,7 +482,7 @@ impl RemotePluginScope { } } - fn from_marketplace_name(name: &str) -> Option { + pub(crate) fn from_marketplace_name(name: &str) -> Option { match name { REMOTE_GLOBAL_MARKETPLACE_NAME => Some(Self::Global), REMOTE_CREATED_BY_ME_MARKETPLACE_NAME => Some(Self::User), diff --git a/codex-rs/core-plugins/src/remote_plugin_id_resolver.rs b/codex-rs/core-plugins/src/remote_plugin_id_resolver.rs new file mode 100644 index 000000000000..fdc63bef1df2 --- /dev/null +++ b/codex-rs/core-plugins/src/remote_plugin_id_resolver.rs @@ -0,0 +1,65 @@ +use crate::remote::RemoteInstalledPlugin; +use crate::remote::RemotePluginScope; +use crate::store::ActivePluginInstallation; +use codex_config::types::PluginConfig; +use codex_plugin::PluginId; +use std::collections::HashMap; +use tracing::warn; + +#[derive(Default)] +pub(crate) struct RemoteInstalledPluginsSnapshot { + pub(crate) configs: HashMap, + pub(crate) remote_plugin_id_resolver: RemotePluginIdResolver, +} + +#[derive(Default)] +pub(crate) struct RemotePluginIdResolver { + snapshot_ids: Option>, +} + +impl RemotePluginIdResolver { + pub(crate) fn new(plugins: &[RemoteInstalledPlugin]) -> Self { + let mut snapshot_ids = HashMap::with_capacity(plugins.len()); + for plugin in plugins { + let Ok(plugin_id) = PluginId::new(plugin.name.clone(), plugin.marketplace_name.clone()) + else { + continue; + }; + snapshot_ids + .entry(plugin_id) + .or_insert_with(|| plugin.id.clone()); + } + Self { + snapshot_ids: Some(snapshot_ids), + } + } + + pub(crate) fn remote_plugin_id_for_installation( + &self, + installation: &ActivePluginInstallation, + ) -> Option { + if let Some(snapshot_ids) = &self.snapshot_ids { + return snapshot_ids.get(&installation.plugin_id).cloned(); + } + + persisted_remote_plugin_id_for_installation(installation) + } +} + +pub(crate) fn persisted_remote_plugin_id_for_installation( + installation: &ActivePluginInstallation, +) -> Option { + RemotePluginScope::from_marketplace_name(&installation.plugin_id.marketplace_name)?; + + match installation.persisted_remote_plugin_id() { + Ok(remote_plugin_id) => remote_plugin_id, + Err(err) => { + warn!( + plugin_id = %installation.plugin_id.as_key(), + error = %err, + "failed to read persisted remote plugin identity" + ); + None + } + } +} diff --git a/codex-rs/core-plugins/src/script_attribution_tests.rs b/codex-rs/core-plugins/src/script_attribution_tests.rs index 192b15ef82b8..0455002d25c1 100644 --- a/codex-rs/core-plugins/src/script_attribution_tests.rs +++ b/codex-rs/core-plugins/src/script_attribution_tests.rs @@ -23,6 +23,7 @@ fn path(path: &Path) -> AbsolutePathBuf { fn loaded_plugin(config_name: &str, root: &Path, enabled: bool) -> LoadedPlugin { LoadedPlugin { config_name: config_name.to_string(), + remote_plugin_id: None, manifest_name: None, plugin_namespace: None, manifest_description: None, diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 92c531938a27..aa3abdbbdcce 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -43,6 +43,48 @@ pub struct PluginStore { data_root: AbsolutePathBuf, } +pub(crate) struct ActivePluginInstallation { + pub(crate) plugin_id: PluginId, + pub(crate) root: AbsolutePathBuf, + remote_plugin_install_metadata_path: AbsolutePathBuf, +} + +impl ActivePluginInstallation { + pub(crate) fn persisted_remote_plugin_id(&self) -> Result, PluginStoreError> { + let contents = match fs::read_to_string(self.remote_plugin_install_metadata_path.as_path()) + { + Ok(contents) => contents, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(PluginStoreError::io( + "failed to read remote plugin install metadata", + err, + )); + } + }; + let metadata: RemotePluginInstallMetadata = + serde_json::from_str(&contents).map_err(|err| { + PluginStoreError::Invalid(format!( + "failed to parse remote plugin install metadata: {err}" + )) + })?; + if metadata.schema_version != REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION { + return Err(PluginStoreError::Invalid(format!( + "unsupported remote plugin install metadata schema version: {}", + metadata.schema_version + ))); + } + let remote_plugin_id = metadata.remote_plugin_id.trim(); + if remote_plugin_id.is_empty() { + return Err(PluginStoreError::Invalid( + "invalid remote plugin install metadata: remote plugin id must not be blank" + .to_string(), + )); + } + Ok(Some(remote_plugin_id.to_string())) + } +} + #[derive(Clone, Copy)] enum InstallManifest<'a> { OnDisk, @@ -124,6 +166,18 @@ impl PluginStore { .map(|plugin_version| self.plugin_root(plugin_id, &plugin_version)) } + pub(crate) fn active_plugin_installation( + &self, + plugin_id: &PluginId, + ) -> Option { + Some(ActivePluginInstallation { + plugin_id: plugin_id.clone(), + root: self.active_plugin_root(plugin_id)?, + remote_plugin_install_metadata_path: self + .remote_plugin_install_metadata_path(plugin_id), + }) + } + pub fn is_installed(&self, plugin_id: &PluginId) -> bool { self.active_plugin_version(plugin_id).is_some() } @@ -132,40 +186,10 @@ impl PluginStore { &self, plugin_id: &PluginId, ) -> Result, PluginStoreError> { - if !self.is_installed(plugin_id) { + let Some(installation) = self.active_plugin_installation(plugin_id) else { return Ok(None); - } - let path = self.remote_plugin_install_metadata_path(plugin_id); - let contents = match fs::read_to_string(path.as_path()) { - Ok(contents) => contents, - Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None), - Err(err) => { - return Err(PluginStoreError::io( - "failed to read remote plugin install metadata", - err, - )); - } }; - let metadata: RemotePluginInstallMetadata = - serde_json::from_str(&contents).map_err(|err| { - PluginStoreError::Invalid(format!( - "failed to parse remote plugin install metadata: {err}" - )) - })?; - if metadata.schema_version != REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION { - return Err(PluginStoreError::Invalid(format!( - "unsupported remote plugin install metadata schema version: {}", - metadata.schema_version - ))); - } - let remote_plugin_id = metadata.remote_plugin_id.trim(); - if remote_plugin_id.is_empty() { - return Err(PluginStoreError::Invalid( - "invalid remote plugin install metadata: remote plugin id must not be blank" - .to_string(), - )); - } - Ok(Some(remote_plugin_id.to_string())) + installation.persisted_remote_plugin_id() } pub fn write_remote_plugin_id( diff --git a/codex-rs/core-plugins/src/tool_suggest_metadata.rs b/codex-rs/core-plugins/src/tool_suggest_metadata.rs index 2f9102a3daef..0871a7436c68 100644 --- a/codex-rs/core-plugins/src/tool_suggest_metadata.rs +++ b/codex-rs/core-plugins/src/tool_suggest_metadata.rs @@ -11,6 +11,7 @@ use codex_plugin::prompt_safe_plugin_description; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::Product; use codex_skills::SkillConfigRules; +use codex_utils_plugins::PluginIdentity; use tokio::sync::Semaphore; use crate::app_mcp_routing::apply_app_mcp_routing_policy; @@ -218,9 +219,13 @@ async fn load_plugin_metadata( } let manifest = load_plugin_manifest(plugin_root.as_path()) .ok_or_else(|| "missing or invalid plugin.json".to_string())?; + let plugin_identity = PluginIdentity { + plugin_id: plugin_id.as_key(), + remote_plugin_id: None, + }; let skill_inventory = load_plugin_skill_inventory( plugin_root, - &plugin_id, + &plugin_identity, &manifest, restriction_product, /*plugin_skill_snapshots*/ None, diff --git a/codex-rs/core-skills/src/injection_tests.rs b/codex-rs/core-skills/src/injection_tests.rs index 78aa19589527..1b6e14dac3ed 100644 --- a/codex-rs/core-skills/src/injection_tests.rs +++ b/codex-rs/core-skills/src/injection_tests.rs @@ -17,6 +17,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata { path_to_skills_md: test_path_buf(path).abs(), scope: codex_protocol::protocol::SkillScope::User, plugin_id: None, + remote_plugin_id: None, } } diff --git a/codex-rs/core-skills/src/invocation_utils_tests.rs b/codex-rs/core-skills/src/invocation_utils_tests.rs index 30f978118bdf..cbbf4c52aff9 100644 --- a/codex-rs/core-skills/src/invocation_utils_tests.rs +++ b/codex-rs/core-skills/src/invocation_utils_tests.rs @@ -22,6 +22,7 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata { path_to_skills_md: skill_doc_path, scope: codex_protocol::protocol::SkillScope::User, plugin_id: None, + remote_plugin_id: None, } } diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index e5b0ac4970f6..bb043ab97177 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -30,6 +30,7 @@ use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; use codex_utils_path_uri::PathUri; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; use dirs::home_dir; @@ -195,7 +196,7 @@ pub struct SkillRoot { pub path: AbsolutePathBuf, pub scope: SkillScope, pub file_system: Arc, - pub plugin_id: Option, + pub plugin_identity: Option, pub plugin_namespace: Option, pub plugin_root: Option, pub discovery_mode: SkillDiscoveryMode, @@ -269,7 +270,7 @@ async fn skill_roots_with_home_dir( path: root.path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some(root.plugin_id), + plugin_identity: Some(root.plugin_identity), plugin_namespace: Some(root.plugin_namespace), plugin_root: Some(root.plugin_root), discovery_mode: root.discovery_mode, @@ -278,7 +279,7 @@ async fn skill_roots_with_home_dir( path, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -310,7 +311,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::Repo, file_system: Arc::clone(repo_fs), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -324,7 +325,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -336,7 +337,7 @@ fn skill_roots_from_layer_stack_inner( path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -349,7 +350,7 @@ fn skill_roots_from_layer_stack_inner( path: system_cache_root_dir(&config_folder), scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -362,7 +363,7 @@ fn skill_roots_from_layer_stack_inner( path: config_folder.join(SKILLS_DIR_NAME), scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -408,7 +409,7 @@ async fn repo_agents_skill_roots( path: agents_skills, scope: SkillScope::Repo, file_system: Arc::clone(&fs), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -530,6 +531,7 @@ async fn load_skills_under_root( outcome: &mut SkillLoadOutcome, ) { let fs = skill_root.file_system.as_ref(); + let plugin_identity = skill_root.plugin_identity.as_ref(); let plugin_root = match skill_root.plugin_root.as_ref() { Some(plugin_root) => Some(canonicalize_for_skill_identity(fs, plugin_root).await), None => None, @@ -659,7 +661,7 @@ async fn load_skills_under_root( &skill.path, &skill.path_uri, skill_root.scope, - skill_root.plugin_id.as_deref(), + plugin_identity, plugin_root, ) .await @@ -696,7 +698,7 @@ async fn parse_skill_file( path: &AbsolutePathBuf, path_uri: &PathUri, scope: SkillScope, - plugin_id: Option<&str>, + plugin_identity: Option<&PluginIdentity>, plugin_root: Option<&AbsolutePathBuf>, ) -> Result { let metadata_path = path_uri @@ -734,7 +736,8 @@ async fn parse_skill_file( policy, path_to_skills_md: path.clone(), scope, - plugin_id: plugin_id.map(str::to_string), + plugin_id: plugin_identity.map(|identity| identity.plugin_id.clone()), + remote_plugin_id: plugin_identity.and_then(|identity| identity.remote_plugin_id.clone()), }) } diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index 58a4a5c74f86..61d5e62cdb07 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -505,6 +505,7 @@ async fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<() path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); @@ -571,7 +572,7 @@ async fn load_user_skills_root(root: &Path) -> SkillLoadOutcome { path: root.abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -593,6 +594,7 @@ fn expected_user_skill(path: &Path, name: &str, description: &str) -> SkillMetad path_to_skills_md: normalized(path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, } } @@ -680,6 +682,7 @@ async fn loads_skill_dependencies_metadata_from_yaml() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -736,6 +739,7 @@ interface: path_to_skills_md: normalized(skill_path.as_path()), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -890,6 +894,7 @@ async fn accepts_icon_paths_under_assets_dir() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -931,6 +936,7 @@ async fn ignores_invalid_brand_color() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -985,6 +991,7 @@ async fn ignores_default_prompt_over_max_length() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1027,6 +1034,7 @@ async fn drops_interface_when_icons_are_invalid() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1059,7 +1067,10 @@ interface: path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("twilio-developer-kit@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "twilio-developer-kit@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: None, plugin_root: Some(plugin_root_abs.clone()), discovery_mode: SkillDiscoveryMode::Recursive, @@ -1094,6 +1105,7 @@ interface: path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: Some("twilio-developer-kit@test".to_string()), + remote_plugin_id: None, }] ); } @@ -1122,7 +1134,10 @@ interface: path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("twilio-developer-kit@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "twilio-developer-kit@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: None, plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::Recursive, @@ -1149,6 +1164,7 @@ interface: path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: Some("twilio-developer-kit@test".to_string()), + remote_plugin_id: None, }] ); } @@ -1194,6 +1210,7 @@ async fn loads_skills_via_symlinked_subdir_for_user_scope() { path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1280,6 +1297,7 @@ async fn does_not_loop_on_symlink_cycle_for_user_scope() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1300,7 +1318,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() { path: admin_root.path().abs(), scope: SkillScope::Admin, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -1327,6 +1345,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() { path_to_skills_md: normalized(&shared_skill_path), scope: SkillScope::Admin, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1367,6 +1386,7 @@ async fn loads_skills_via_symlinked_subdir_for_repo_scope() { path_to_skills_md: normalized(&linked_skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1388,7 +1408,7 @@ async fn system_scope_ignores_symlinked_subdir() { path: system_root.abs(), scope: SkillScope::System, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -1428,7 +1448,7 @@ async fn respects_max_scan_depth_for_user_scope() { path: skills_root.abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -1455,6 +1475,7 @@ async fn respects_max_scan_depth_for_user_scope() { path_to_skills_md: normalized(&within_depth_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1483,6 +1504,7 @@ async fn loads_valid_skill() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1516,6 +1538,7 @@ async fn falls_back_to_directory_name_when_skill_name_is_missing() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1541,7 +1564,10 @@ async fn namespaces_plugin_skills_using_provided_namespace() { path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("sample@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "sample@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: Some("sample".to_string()), plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::Recursive, @@ -1568,6 +1594,7 @@ async fn namespaces_plugin_skills_using_provided_namespace() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: Some("sample@test".to_string()), + remote_plugin_id: None, }] ); } @@ -1790,7 +1817,10 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() { path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("sample@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "sample@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: Some(plugin_name.clone()), plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::Recursive, @@ -1817,6 +1847,7 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: Some("sample@test".to_string()), + remote_plugin_id: None, }] ); } @@ -1841,7 +1872,10 @@ async fn plugin_skill_name_length_limit_rejects_overlong_qualified_name() { path: plugin_root.join("skills").abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("sample@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "sample@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: Some(plugin_name.clone()), plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::Recursive, @@ -1873,7 +1907,10 @@ async fn direct_child_discovery_ignores_nested_skills() { path: skills_root.abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("plugin@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "plugin@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: Some("plugin".to_string()), plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::DirectChildren, @@ -1896,6 +1933,7 @@ async fn direct_child_discovery_ignores_nested_skills() { path_to_skills_md: normalized(&direct), scope: SkillScope::User, plugin_id: Some("plugin@test".to_string()), + remote_plugin_id: None, }] ); } @@ -1917,7 +1955,10 @@ async fn direct_child_discovery_skips_skills_resolving_outside_plugin_root() { path: skills_root.abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: Some("plugin@test".to_string()), + plugin_identity: Some(PluginIdentity { + plugin_id: "plugin@test".to_string(), + remote_plugin_id: None, + }), plugin_namespace: Some("plugin".to_string()), plugin_root: Some(plugin_root.abs()), discovery_mode: SkillDiscoveryMode::DirectChildren, @@ -1958,6 +1999,7 @@ async fn loads_short_description_from_metadata() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -1990,6 +2032,7 @@ async fn loads_unquoted_description_containing_colon_space() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2022,6 +2065,7 @@ async fn loads_unquoted_short_description_containing_colon_space_and_apostrophe( path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2054,6 +2098,7 @@ async fn loads_unrecognized_frontmatter_fields_that_need_quotes() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2086,6 +2131,7 @@ async fn preserves_block_scalar_body_while_repairing_other_fields() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2203,6 +2249,7 @@ async fn loads_skills_from_repo_root() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2239,6 +2286,7 @@ async fn loads_skills_from_agents_dir_without_codex_dir() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2293,6 +2341,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() { path_to_skills_md: normalized(&nested_skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "root-skill".to_string(), @@ -2304,6 +2353,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() { path_to_skills_md: normalized(&root_skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, ] ); @@ -2461,7 +2511,7 @@ async fn merges_root_results_in_input_order_when_scans_finish_out_of_order() { SkillScope::User }, file_system: Arc::clone(&root_file_system), - plugin_id: None, + plugin_identity: None, plugin_namespace: Some("test".to_string()), plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -2527,7 +2577,7 @@ async fn skill_root_scans_wait_for_shared_capacity() { path: root.abs(), scope: SkillScope::Repo, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: Some("test".to_string()), plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -2580,6 +2630,7 @@ async fn loads_skills_from_codex_dir_when_not_git_repo() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2596,7 +2647,7 @@ async fn deduplicates_by_path_preferring_first_root() { path: root.path().abs(), scope: SkillScope::Repo, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -2605,7 +2656,7 @@ async fn deduplicates_by_path_preferring_first_root() { path: root.path().abs(), scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: SkillDiscoveryMode::Recursive, @@ -2633,6 +2684,7 @@ async fn deduplicates_by_path_preferring_first_root() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2675,6 +2727,7 @@ async fn keeps_duplicate_names_from_repo_and_user() { path_to_skills_md: normalized(&repo_skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "dupe-skill".to_string(), @@ -2686,6 +2739,7 @@ async fn keeps_duplicate_names_from_repo_and_user() { path_to_skills_md: normalized(&user_skill_path), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }, ] ); @@ -2749,6 +2803,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() { path_to_skills_md: first_path, scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "dupe-skill".to_string(), @@ -2760,6 +2815,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() { path_to_skills_md: second_path, scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, ] ); @@ -2832,6 +2888,7 @@ async fn loads_skills_when_cwd_is_file_in_repo() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }] ); } @@ -2891,6 +2948,7 @@ async fn loads_skills_from_system_cache_when_present() { path_to_skills_md: normalized(&skill_path), scope: SkillScope::System, plugin_id: None, + remote_plugin_id: None, }] ); } diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 584b4fb607c4..73a37e502bcb 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -939,6 +939,7 @@ mod tests { path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), scope, plugin_id: None, + remote_plugin_id: None, } } diff --git a/codex-rs/core-skills/src/root_loader.rs b/codex-rs/core-skills/src/root_loader.rs index a8d30efbe5da..f18f96e85d1f 100644 --- a/codex-rs/core-skills/src/root_loader.rs +++ b/codex-rs/core-skills/src/root_loader.rs @@ -56,14 +56,14 @@ where .await .unwrap_or_else(|_| unreachable!()); let cache_key = match ( - root.plugin_id.clone(), + root.plugin_identity.clone(), root.plugin_namespace.clone(), root.plugin_root.clone(), ) { - (Some(plugin_id), Some(plugin_namespace), Some(plugin_root)) => { + (Some(plugin_identity), Some(plugin_namespace), Some(plugin_root)) => { Some(PluginSkillRoot { path: root.path.clone(), - plugin_id, + plugin_identity, plugin_namespace, plugin_root, discovery_mode: root.discovery_mode, diff --git a/codex-rs/core-skills/src/service.rs b/codex-rs/core-skills/src/service.rs index 60c8c8748584..09f87fd9f82f 100644 --- a/codex-rs/core-skills/src/service.rs +++ b/codex-rs/core-skills/src/service.rs @@ -8,6 +8,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use tokio::sync::Semaphore; use tracing::info; @@ -280,10 +281,18 @@ impl SkillsService { #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct ConfigSkillsCacheKey { - roots: Vec<(AbsolutePathBuf, u8, Option, Option)>, + roots: Vec, skill_config_rules: SkillConfigRules, } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +struct ConfigSkillRootCacheKey { + path: AbsolutePathBuf, + scope_rank: u8, + plugin_identity: Option, + plugin_namespace: Option, +} + pub fn bundled_skills_enabled_from_stack( config_layer_stack: &codex_config::ConfigLayerStack, ) -> bool { @@ -320,12 +329,12 @@ fn config_skills_cache_key( SkillScope::System => 2, SkillScope::Admin => 3, }; - ( - root.path.clone(), + ConfigSkillRootCacheKey { + path: root.path.clone(), scope_rank, - root.plugin_id.clone(), - root.plugin_namespace.clone(), - ) + plugin_identity: root.plugin_identity.clone(), + plugin_namespace: root.plugin_namespace.clone(), + } }) .collect(), skill_config_rules: skill_config_rules.clone(), diff --git a/codex-rs/core-skills/src/service_tests.rs b/codex-rs/core-skills/src/service_tests.rs index 0d9f4e73b17d..1a27e8034ccc 100644 --- a/codex-rs/core-skills/src/service_tests.rs +++ b/codex-rs/core-skills/src/service_tests.rs @@ -11,6 +11,7 @@ use codex_exec_server::LOCAL_FS; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::PathExt; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; use pretty_assertions::assert_eq; @@ -70,7 +71,10 @@ fn plugin_skill_root_for_skill_path( .expect("plugin skills root should live under a plugin root"); PluginSkillRoot { path: skills_root.abs(), - plugin_id: plugin_id.to_string(), + plugin_identity: PluginIdentity { + plugin_id: plugin_id.to_string(), + remote_plugin_id: None, + }, plugin_namespace: plugin_namespace.to_string(), plugin_root: plugin_root.abs(), discovery_mode: SkillDiscoveryMode::Recursive, @@ -91,6 +95,7 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata { .expect("skill path should canonicalize"), scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, } } @@ -230,6 +235,53 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() { assert_eq!(outcome2.skills, outcome1.skills); } +#[tokio::test] +async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let cwd = tempfile::tempdir().expect("tempdir"); + let skill_path = write_plugin_skill( + &codex_home, + "test", + "sample", + "sample-search", + "sample-search", + "search sample data", + ); + let config_layer_stack = config_stack(&codex_home, ""); + let mut plugin_skill_root = + plugin_skill_root_for_skill_path(&skill_path, "sample@test", "sample"); + let skills_service = SkillsService::new( + codex_home.path().abs(), + /*bundled_skills_enabled*/ true, + ); + + skills_for_config_with_stack( + &skills_service, + &cwd, + &config_layer_stack, + &[plugin_skill_root.clone()], + ) + .await; + + plugin_skill_root.plugin_identity.remote_plugin_id = Some("plugins~Plugin_sample".to_string()); + let refreshed = skills_for_config_with_stack( + &skills_service, + &cwd, + &config_layer_stack, + &[plugin_skill_root], + ) + .await; + + assert_eq!( + refreshed + .skills + .iter() + .find(|skill| skill.name == "sample:sample-search") + .and_then(|skill| skill.remote_plugin_id.as_deref()), + Some("plugins~Plugin_sample") + ); +} + #[tokio::test] async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { let codex_home = tempfile::tempdir().expect("tempdir"); diff --git a/codex-rs/core-skills/tests/environment_loader.rs b/codex-rs/core-skills/tests/environment_loader.rs index 7c35ef88806b..e33b010b4db3 100644 --- a/codex-rs/core-skills/tests/environment_loader.rs +++ b/codex-rs/core-skills/tests/environment_loader.rs @@ -469,7 +469,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() { path: host_root.abs(), scope: SkillScope::User, file_system, - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: codex_utils_plugins::SkillDiscoveryMode::Recursive, @@ -504,6 +504,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() { path_to_skills_md: first_skill_path, scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "linked:second".to_string(), @@ -515,6 +516,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() { path_to_skills_md: second_skill_path, scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }, ] ); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 515d189e9162..e8bdeb72b91a 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8921,6 +8921,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "repo-skill".to_string(), @@ -8932,6 +8933,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, ]; turn_context.model_info.context_window = Some(100); @@ -8969,6 +8971,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }]; let rendered = build_available_skills( &outcome, @@ -9014,6 +9017,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o path_to_skills_md: test_path_buf("/tmp/alpha-skill/SKILL.md").abs(), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }; let beta = SkillMetadata { name: "beta-skill".to_string(), @@ -9025,6 +9029,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o path_to_skills_md: test_path_buf("/tmp/beta-skill/SKILL.md").abs(), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }; let minimum_skill_line_cost = |skill: &SkillMetadata| { let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); @@ -9073,6 +9078,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, plugin_id: None, + remote_plugin_id: None, }, SkillMetadata { name: "repo-skill".to_string(), @@ -9084,6 +9090,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, plugin_id: None, + remote_plugin_id: None, }, ]; turn_context.model_info.context_window = Some(100); diff --git a/codex-rs/ext/skills/src/provider/host_tests.rs b/codex-rs/ext/skills/src/provider/host_tests.rs index 39243cbf6097..302cf68de7d1 100644 --- a/codex-rs/ext/skills/src/provider/host_tests.rs +++ b/codex-rs/ext/skills/src/provider/host_tests.rs @@ -36,7 +36,7 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box TestResult { path_to_skills_md: skill_path, scope: SkillScope::User, plugin_id: None, + remote_plugin_id: None, }); let loaded_skills = Arc::new(outcome); let skill_prompt_path = skill_path_string.replace('\\', "/"); @@ -1069,7 +1070,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul path: root, scope: SkillScope::User, file_system: Arc::clone(&LOCAL_FS), - plugin_id: None, + plugin_identity: None, plugin_namespace: None, plugin_root: None, discovery_mode: Default::default(), diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 51d59d3d21c9..5fd1320c0993 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::collections::HashSet; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::SkillDiscoveryMode; @@ -17,6 +18,7 @@ const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; #[derive(Debug, Clone, PartialEq)] pub struct LoadedPlugin { pub config_name: String, + pub remote_plugin_id: Option, pub manifest_name: Option, pub plugin_namespace: Option, pub manifest_description: Option, @@ -135,7 +137,10 @@ impl PluginLoadOutcome { if seen_paths.insert(path.clone()) { skill_roots.push(PluginSkillRoot { path: path.clone(), - plugin_id: plugin.config_name.clone(), + plugin_identity: PluginIdentity { + plugin_id: plugin.config_name.clone(), + remote_plugin_id: plugin.remote_plugin_id.clone(), + }, plugin_namespace: plugin_namespace.clone(), plugin_root: plugin.root.clone(), discovery_mode: SkillDiscoveryMode::Recursive, @@ -224,6 +229,7 @@ mod tests { fn loaded_plugin(config_name: &str, skill_roots: Vec) -> LoadedPlugin<()> { LoadedPlugin { config_name: config_name.to_string(), + remote_plugin_id: None, manifest_name: None, plugin_namespace: Some( config_name @@ -248,8 +254,10 @@ mod tests { #[test] fn effective_plugin_skill_roots_preserves_first_plugin_for_shared_root() { let shared_root = test_path("shared-skills"); + let mut first_plugin = loaded_plugin("zeta@test", vec![shared_root.clone()]); + first_plugin.remote_plugin_id = Some("plugins~Plugin_zeta".to_string()); let outcome = PluginLoadOutcome::from_plugins(vec![ - loaded_plugin("zeta@test", vec![shared_root.clone()]), + first_plugin, loaded_plugin("alpha@test", vec![shared_root.clone()]), ]); @@ -257,7 +265,10 @@ mod tests { outcome.effective_plugin_skill_roots(), vec![PluginSkillRoot { path: shared_root, - plugin_id: "zeta@test".to_string(), + plugin_identity: PluginIdentity { + plugin_id: "zeta@test".to_string(), + remote_plugin_id: Some("plugins~Plugin_zeta".to_string()), + }, plugin_namespace: "zeta".to_string(), plugin_root: test_path("zeta@test"), discovery_mode: SkillDiscoveryMode::Recursive, diff --git a/codex-rs/plugin/src/plugin_id.rs b/codex-rs/plugin/src/plugin_id.rs index 075116322bb7..0b7b21f14ed0 100644 --- a/codex-rs/plugin/src/plugin_id.rs +++ b/codex-rs/plugin/src/plugin_id.rs @@ -6,7 +6,7 @@ pub enum PluginIdError { Invalid(String), } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PluginId { pub plugin_name: String, pub marketplace_name: String, diff --git a/codex-rs/skills/src/model.rs b/codex-rs/skills/src/model.rs index c388718ad66a..977decdf759a 100644 --- a/codex-rs/skills/src/model.rs +++ b/codex-rs/skills/src/model.rs @@ -16,6 +16,7 @@ pub struct SkillMetadata { pub path_to_skills_md: AbsolutePathBuf, pub scope: SkillScope, pub plugin_id: Option, + pub remote_plugin_id: Option, } impl SkillMetadata { diff --git a/codex-rs/utils/plugins/src/lib.rs b/codex-rs/utils/plugins/src/lib.rs index eb3e21131fe9..1af5b6d9c860 100644 --- a/codex-rs/utils/plugins/src/lib.rs +++ b/codex-rs/utils/plugins/src/lib.rs @@ -26,10 +26,17 @@ pub enum SkillDiscoveryMode { DirectChildren, } +/// The local identifier and optional remote identifier for a plugin. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct PluginIdentity { + pub plugin_id: String, + pub remote_plugin_id: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PluginSkillRoot { pub path: AbsolutePathBuf, - pub plugin_id: String, + pub plugin_identity: PluginIdentity, pub plugin_namespace: String, pub plugin_root: AbsolutePathBuf, pub discovery_mode: SkillDiscoveryMode,