From b08fde8827476f0bc66fd655a9be120b589ab04b Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Tue, 16 Jun 2026 16:31:23 -0700 Subject: [PATCH] Reuse parsed plugin skill roots --- codex-rs/core-plugins/src/loader.rs | 10 +- codex-rs/core-plugins/src/loader_tests.rs | 3 + codex-rs/core-plugins/src/manager.rs | 12 ++ codex-rs/core-plugins/src/manager_tests.rs | 5 + codex-rs/core-skills/src/lib.rs | 2 + codex-rs/core-skills/src/loader.rs | 106 ++++------ codex-rs/core-skills/src/root_loader.rs | 188 ++++++++++++++++++ codex-rs/core-skills/src/root_loader_tests.rs | 55 +++++ codex-rs/core-skills/src/service.rs | 13 +- codex-rs/core/src/thread_manager.rs | 26 ++- 10 files changed, 339 insertions(+), 81 deletions(-) create mode 100644 codex-rs/core-skills/src/root_loader.rs create mode 100644 codex-rs/core-skills/src/root_loader_tests.rs diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 029be960b6..9b21ed1656 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -20,11 +20,11 @@ use codex_config::types::McpServerConfig; use codex_config::types::PluginConfig; use codex_config::types::PluginMcpServerConfig; use codex_core_skills::SkillMetadata; +use codex_core_skills::SkillRootLoader; 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::SkillRoot; -use codex_core_skills::loader::load_skills_from_roots; use codex_exec_server::LOCAL_FS; use codex_mcp::PluginMcpServerPlacement; use codex_mcp::parse_plugin_mcp_config; @@ -72,6 +72,7 @@ enum PluginLoadScope<'a> { AllCapabilities { restriction_product: Option, skill_config_rules: &'a SkillConfigRules, + skill_root_loader: &'a SkillRootLoader, }, HooksOnly, } @@ -113,6 +114,7 @@ pub(crate) async fn load_plugins_from_layer_stack( config_layer_stack: &ConfigLayerStack, extra_plugins: HashMap, store: &PluginStore, + skill_root_loader: &SkillRootLoader, restriction_product: Option, prefer_remote_curated_conflicts: bool, ) -> Vec> { @@ -125,6 +127,7 @@ pub(crate) async fn load_plugins_from_layer_stack( PluginLoadScope::AllCapabilities { restriction_product, skill_config_rules: &skill_config_rules, + skill_root_loader, }, ) .await @@ -713,6 +716,7 @@ async fn load_plugin( PluginLoadScope::AllCapabilities { restriction_product, skill_config_rules, + skill_root_loader, } => { loaded_plugin.manifest_name = Some(manifest.display_name().to_string()); loaded_plugin.manifest_description = manifest.description.clone(); @@ -723,6 +727,7 @@ async fn load_plugin( &manifest, *restriction_product, skill_config_rules, + skill_root_loader, ) .await; let has_enabled_skills = resolved_skills.has_enabled_skills(); @@ -791,6 +796,7 @@ pub async fn load_plugin_skills( manifest: &PluginManifest, restriction_product: Option, skill_config_rules: &SkillConfigRules, + skill_root_loader: &SkillRootLoader, ) -> ResolvedPluginSkills { let roots = plugin_skill_roots(plugin_root, &manifest.paths) .into_iter() @@ -803,7 +809,7 @@ pub async fn load_plugin_skills( plugin_root: Some(plugin_root.clone()), }) .collect::>(); - let outcome = load_skills_from_roots(roots).await; + let outcome = skill_root_loader.load_skills_from_roots(roots).await; let had_errors = !outcome.errors.is_empty(); let skills = outcome .skills diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 133103b78a..cdec96128a 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -5,6 +5,7 @@ use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerSource; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; +use codex_core_skills::SkillRootLoader; use codex_plugin::PluginId; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -155,11 +156,13 @@ enabled = true ) .expect("valid config layer stack"); let store = PluginStore::new(temp_dir.path().to_path_buf()); + let skill_root_loader = SkillRootLoader::default(); let full = load_plugins_from_layer_stack( &stack, HashMap::new(), &store, + &skill_root_loader, Some(Product::Codex), /*prefer_remote_curated_conflicts*/ false, ) diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index b1bb508eeb..e081a26136 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -60,6 +60,7 @@ use codex_config::types::PluginConfig; use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; use codex_core_skills::SkillMetadata; +use codex_core_skills::SkillRootLoader; use codex_core_skills::config_rules::SkillConfigRules; use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_hooks::plugin_hook_declarations; @@ -354,6 +355,7 @@ pub struct PluginsManager { // Keep the cache auth-independent so auth changes only need to resolve capabilities again. loaded_plugins_cache: RwLock, loaded_plugins_load_semaphore: Semaphore, + skill_root_loader: Arc, remote_installed_plugins_cache: RwLock>>, remote_installed_plugins_cache_refresh_state: RwLock, global_remote_catalog_cache_refresh_state: RwLock, @@ -410,6 +412,7 @@ impl PluginsManager { non_curated_cache_refresh_state: RwLock::new(NonCuratedCacheRefreshState::default()), loaded_plugins_cache: RwLock::new(LoadedPluginsCache::default()), loaded_plugins_load_semaphore: Semaphore::new(/*permits*/ 1), + skill_root_loader: Arc::new(SkillRootLoader::default()), remote_installed_plugins_cache: RwLock::new(None), remote_installed_plugins_cache_refresh_state: RwLock::new( RemoteInstalledPluginsCacheRefreshState::default(), @@ -435,6 +438,11 @@ impl PluginsManager { true } + /// Returns the loader whose plugin-root snapshots are populated while loading plugins. + pub fn skill_root_loader(&self) -> Arc { + Arc::clone(&self.skill_root_loader) + } + pub fn auth_mode(&self) -> Option { match self.auth_mode.read() { Ok(auth_mode_guard) => *auth_mode_guard, @@ -505,6 +513,7 @@ impl PluginsManager { &config.config_layer_stack, self.remote_installed_plugin_configs(), &self.store, + self.skill_root_loader.as_ref(), self.restriction_product, config.remote_plugin_enabled, ) @@ -551,6 +560,7 @@ impl PluginsManager { } fn clear_loaded_plugins_cache(&self) { + self.skill_root_loader.clear_cache(); let mut cache = match self.loaded_plugins_cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -572,6 +582,7 @@ impl PluginsManager { config_layer_stack, self.remote_installed_plugin_configs(), &self.store, + self.skill_root_loader.as_ref(), self.restriction_product, config.remote_plugin_enabled, ) @@ -1596,6 +1607,7 @@ impl PluginsManager { &codex_core_skills::config_rules::skill_config_rules_from_stack( &config.config_layer_stack, ), + self.skill_root_loader.as_ref(), ) .await; let plugin_data_root = self.store.plugin_data_root(&plugin_id); diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index f5073f68b0..1c639a5dc3 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -34,6 +34,7 @@ use codex_config::McpServerConfig; use codex_config::McpServerOAuthConfig; use codex_config::McpServerToolConfig; use codex_config::types::McpServerTransportConfig; +use codex_core_skills::SkillRootLoader; use codex_core_skills::config_rules::SkillConfigRules; use codex_login::CodexAuth; use codex_plugin::AppDeclaration; @@ -1466,6 +1467,7 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() { interface: None, }; let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse"); + let skill_root_loader = SkillRootLoader::default(); let resolved = load_plugin_skills( &plugin_root, @@ -1473,6 +1475,7 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() { &manifest, /*restriction_product*/ None, &SkillConfigRules::default(), + &skill_root_loader, ) .await; @@ -4820,10 +4823,12 @@ async fn load_plugins_ignores_project_config_files() { ) .expect("config layer stack should build"); + let skill_root_loader = SkillRootLoader::default(); let plugins = load_plugins_from_layer_stack( &stack, std::collections::HashMap::new(), &PluginStore::new(codex_home.path().to_path_buf()), + &skill_root_loader, Some(Product::Codex), /*prefer_remote_curated_conflicts*/ false, ) diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 31e5a7c531..8ee6d39f86 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -6,6 +6,7 @@ mod mention_counts; pub mod model; pub mod remote; pub mod render; +mod root_loader; pub mod service; mod skill_instructions; pub mod system; @@ -29,6 +30,7 @@ pub use render::SkillRenderReport; pub use render::build_available_skills; pub use render::default_skill_metadata_budget; pub use render::render_available_skills_body; +pub use root_loader::SkillRootLoader; pub use service::SkillsLoadInput; pub use service::SkillsService; pub use skill_instructions::SkillInstructions; diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 64861c45eb..346eebbec9 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -1,11 +1,11 @@ use crate::model::SkillDependencies; use crate::model::SkillError; -use crate::model::SkillFileSystemsByPath; use crate::model::SkillInterface; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; use crate::model::SkillPolicy; use crate::model::SkillToolDependency; +use crate::root_loader::SkillRootLoader; use crate::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; use codex_config::ConfigLayerStack; @@ -24,7 +24,6 @@ use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::plugin_namespace_for_skill_path; use dirs::home_dir; use serde::Deserialize; -use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; use std::error::Error; @@ -165,73 +164,46 @@ pub async fn load_skills_from_roots(roots: I) -> SkillLoadOutcome where I: IntoIterator, { + SkillRootLoader::default() + .load_skills_from_roots(roots) + .await +} + +#[derive(Clone)] +pub(crate) struct SkillRootSnapshot { + pub(crate) root: AbsolutePathBuf, + pub(crate) skills: Vec, + pub(crate) errors: Vec, + pub(crate) file_system: Arc, +} + +pub(crate) async fn load_skill_root(root: SkillRoot) -> SkillRootSnapshot { + let SkillRoot { + path, + scope, + file_system, + plugin_id, + plugin_namespace, + plugin_root, + } = root; + let root = canonicalize_for_skill_identity(file_system.as_ref(), &path).await; let mut outcome = SkillLoadOutcome::default(); - let mut skill_roots: Vec = Vec::new(); - let mut skill_root_by_path: HashMap = HashMap::new(); - let mut file_systems_by_skill_path: HashMap> = - HashMap::new(); - for root in roots { - let fs = root.file_system; - let root_path = canonicalize_for_skill_identity(fs.as_ref(), &root.path).await; - let skills_before_root = outcome.skills.len(); - discover_skills_under_root( - fs.as_ref(), - &root_path, - root.scope, - root.plugin_id.as_deref(), - root.plugin_namespace.as_deref(), - root.plugin_root.as_ref(), - &mut outcome, - ) - .await; - for skill in &outcome.skills[skills_before_root..] { - if !skill_roots.contains(&root_path) { - skill_roots.push(root_path.clone()); - } - skill_root_by_path - .entry(skill.path_to_skills_md.clone()) - .or_insert_with(|| root_path.clone()); - file_systems_by_skill_path - .entry(skill.path_to_skills_md.clone()) - .or_insert_with(|| Arc::clone(&fs)); - } + discover_skills_under_root( + file_system.as_ref(), + &root, + scope, + plugin_id.as_deref(), + plugin_namespace.as_deref(), + plugin_root.as_ref(), + &mut outcome, + ) + .await; + SkillRootSnapshot { + root, + skills: outcome.skills, + errors: outcome.errors, + file_system, } - - let mut seen: HashSet = HashSet::new(); - outcome - .skills - .retain(|skill| seen.insert(skill.path_to_skills_md.clone())); - let retained_skill_paths: HashSet = outcome - .skills - .iter() - .map(|skill| skill.path_to_skills_md.clone()) - .collect(); - skill_root_by_path.retain(|path, _| retained_skill_paths.contains(path)); - let used_roots: HashSet = skill_root_by_path.values().cloned().collect(); - skill_roots.retain(|root| used_roots.contains(root)); - file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path)); - outcome.skill_roots = skill_roots; - outcome.skill_root_by_path = Arc::new(skill_root_by_path); - outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path); - - fn scope_rank(scope: SkillScope) -> u8 { - // Higher-priority scopes first (matches root scan order for dedupe). - match scope { - SkillScope::Repo => 0, - SkillScope::User => 1, - SkillScope::System => 2, - SkillScope::Admin => 3, - } - } - - outcome.skills.sort_by(|a, b| { - scope_rank(a.scope) - .cmp(&scope_rank(b.scope)) - .then_with(|| a.name.cmp(&b.name)) - .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) - }); - - outcome } pub(crate) async fn skill_roots( diff --git a/codex-rs/core-skills/src/root_loader.rs b/codex-rs/core-skills/src/root_loader.rs new file mode 100644 index 0000000000..42a2a69087 --- /dev/null +++ b/codex-rs/core-skills/src/root_loader.rs @@ -0,0 +1,188 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::hash::Hash; +use std::hash::Hasher; +use std::sync::Arc; +use std::sync::RwLock; + +use codex_exec_server::ExecutorFileSystem; +use codex_utils_absolute_path::AbsolutePathBuf; + +use crate::SkillLoadOutcome; +use crate::loader::SkillRoot; +use crate::loader::SkillRootSnapshot; +use crate::loader::load_skill_root; +use crate::model::SkillFileSystemsByPath; + +const MAX_CACHED_PLUGIN_SKILL_ROOTS: usize = 256; + +#[derive(Clone)] +struct FileSystemIdentity(Arc); + +impl PartialEq for FileSystemIdentity { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) + } +} + +impl Eq for FileSystemIdentity {} + +impl Hash for FileSystemIdentity { + fn hash(&self, state: &mut H) { + (Arc::as_ptr(&self.0) as *const ()).hash(state); + } +} + +#[derive(Clone, PartialEq, Eq, Hash)] +struct SkillRootCacheKey { + path: AbsolutePathBuf, + scope_rank: u8, + file_system: FileSystemIdentity, + plugin_id: String, + plugin_namespace: String, + plugin_root: AbsolutePathBuf, +} + +impl SkillRootCacheKey { + fn from_root(root: &SkillRoot) -> Option { + Some(Self { + path: root.path.clone(), + scope_rank: scope_rank(root.scope), + file_system: FileSystemIdentity(Arc::clone(&root.file_system)), + plugin_id: root.plugin_id.clone()?, + plugin_namespace: root.plugin_namespace.clone()?, + plugin_root: root.plugin_root.clone()?, + }) + } +} + +/// Loads skill roots and reuses parsed plugin-root snapshots until explicitly invalidated. +/// +/// Non-plugin roots are always loaded directly because their filesystem lifecycle is owned by the +/// environment or config layer that supplied them. +#[derive(Default)] +pub struct SkillRootLoader { + plugin_root_cache: RwLock>, +} + +impl SkillRootLoader { + /// Loads and merges roots, reusing snapshots for roots owned by a plugin. + pub async fn load_skills_from_roots(&self, roots: I) -> SkillLoadOutcome + where + I: IntoIterator, + { + let mut snapshots = Vec::new(); + for root in roots { + let cache_key = SkillRootCacheKey::from_root(&root); + let snapshot = match cache_key.as_ref().and_then(|key| self.cached_snapshot(key)) { + Some(snapshot) => snapshot, + None => { + let snapshot = load_skill_root(root).await; + if let Some(cache_key) = cache_key { + self.cache_snapshot(cache_key, snapshot.clone()); + } + snapshot + } + }; + snapshots.push(snapshot); + } + + merge_skill_root_snapshots(snapshots) + } + + /// Invalidates every cached plugin-root snapshot. + pub fn clear_cache(&self) { + let mut cache = self + .plugin_root_cache + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + cache.clear(); + } + + fn cached_snapshot(&self, key: &SkillRootCacheKey) -> Option { + match self.plugin_root_cache.read() { + Ok(cache) => cache.get(key).cloned(), + Err(err) => err.into_inner().get(key).cloned(), + } + } + + fn cache_snapshot(&self, key: SkillRootCacheKey, snapshot: SkillRootSnapshot) { + let mut cache = self + .plugin_root_cache + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if cache.len() < MAX_CACHED_PLUGIN_SKILL_ROOTS || cache.contains_key(&key) { + cache.insert(key, snapshot); + } + } +} + +fn merge_skill_root_snapshots(snapshots: Vec) -> SkillLoadOutcome { + let mut outcome = SkillLoadOutcome::default(); + let mut skill_roots = Vec::new(); + let mut skill_root_by_path = HashMap::new(); + let mut file_systems_by_skill_path = HashMap::new(); + + for snapshot in snapshots { + let SkillRootSnapshot { + root, + skills, + errors, + file_system, + } = snapshot; + if !skills.is_empty() && !skill_roots.contains(&root) { + skill_roots.push(root.clone()); + } + for skill in &skills { + skill_root_by_path + .entry(skill.path_to_skills_md.clone()) + .or_insert_with(|| root.clone()); + file_systems_by_skill_path + .entry(skill.path_to_skills_md.clone()) + .or_insert_with(|| Arc::clone(&file_system)); + } + outcome.skills.extend(skills); + outcome.errors.extend(errors); + } + + let mut seen = HashSet::new(); + outcome + .skills + .retain(|skill| seen.insert(skill.path_to_skills_md.clone())); + let retained_skill_paths = outcome + .skills + .iter() + .map(|skill| skill.path_to_skills_md.clone()) + .collect::>(); + skill_root_by_path.retain(|path, _| retained_skill_paths.contains(path)); + let used_roots = skill_root_by_path.values().cloned().collect::>(); + skill_roots.retain(|root| used_roots.contains(root)); + file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path)); + outcome.skill_roots = skill_roots; + outcome.skill_root_by_path = Arc::new(skill_root_by_path); + outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path); + + outcome.skills.sort_by(|a, b| { + scope_rank(a.scope) + .cmp(&scope_rank(b.scope)) + .then_with(|| a.name.cmp(&b.name)) + .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) + }); + + outcome +} + +fn scope_rank(scope: codex_protocol::protocol::SkillScope) -> u8 { + use codex_protocol::protocol::SkillScope; + + match scope { + SkillScope::Repo => 0, + SkillScope::User => 1, + SkillScope::System => 2, + SkillScope::Admin => 3, + } +} + +#[cfg(test)] +#[path = "root_loader_tests.rs"] +mod tests; diff --git a/codex-rs/core-skills/src/root_loader_tests.rs b/codex-rs/core-skills/src/root_loader_tests.rs new file mode 100644 index 0000000000..f2af87bd24 --- /dev/null +++ b/codex-rs/core-skills/src/root_loader_tests.rs @@ -0,0 +1,55 @@ +use std::fs; +use std::sync::Arc; + +use codex_exec_server::LOCAL_FS; +use codex_protocol::protocol::SkillScope; +use codex_utils_absolute_path::test_support::PathExt; + +use super::SkillRootLoader; +use crate::loader::SkillRoot; + +fn plugin_root(path: &std::path::Path) -> SkillRoot { + SkillRoot { + path: path.join("skills").abs(), + scope: SkillScope::User, + file_system: Arc::clone(&LOCAL_FS), + plugin_id: Some("sample@test".to_string()), + plugin_namespace: Some("sample".to_string()), + plugin_root: Some(path.abs()), + } +} + +fn write_skill(path: &std::path::Path, description: &str) { + let skill_dir = path.join("skills/search"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: search\ndescription: {description}\n---\n"), + ) + .expect("write skill"); +} + +#[tokio::test] +async fn reuses_plugin_root_snapshot_until_cache_is_cleared() { + let tempdir = tempfile::tempdir().expect("tempdir"); + let loader = SkillRootLoader::default(); + + write_skill(tempdir.path(), "first"); + let first = loader + .load_skills_from_roots([plugin_root(tempdir.path())]) + .await; + + write_skill(tempdir.path(), "second"); + let cached = loader + .load_skills_from_roots([plugin_root(tempdir.path())]) + .await; + assert_eq!(cached.skills, first.skills); + + loader.clear_cache(); + let refreshed = loader + .load_skills_from_roots([plugin_root(tempdir.path())]) + .await; + let mut expected = first.skills; + expected[0].description = "second".to_string(); + assert_eq!(refreshed.skills, expected); +} diff --git a/codex-rs/core-skills/src/service.rs b/codex-rs/core-skills/src/service.rs index a6a1e30f57..1187565d0a 100644 --- a/codex-rs/core-skills/src/service.rs +++ b/codex-rs/core-skills/src/service.rs @@ -15,12 +15,12 @@ use tracing::warn; use crate::HostSkillsSnapshot; use crate::SkillLoadOutcome; +use crate::SkillRootLoader; use crate::build_implicit_skill_path_indexes; use crate::config_rules::SkillConfigRules; use crate::config_rules::resolve_disabled_skill_paths; use crate::config_rules::skill_config_rules_from_stack; use crate::loader::SkillRoot; -use crate::loader::load_skills_from_roots; use crate::loader::skill_roots; use crate::system::install_system_skills; use crate::system::uninstall_system_skills; @@ -56,6 +56,7 @@ impl SkillsLoadInput { pub struct SkillsService { codex_home: AbsolutePathBuf, restriction_product: Option, + skill_root_loader: Arc, extra_roots: RwLock>, cache_by_cwd: RwLock>, cache_by_config: RwLock>, @@ -74,6 +75,7 @@ impl SkillsService { let service = Self { codex_home, restriction_product, + skill_root_loader: Arc::new(SkillRootLoader::default()), extra_roots: RwLock::new(Vec::new()), cache_by_cwd: RwLock::new(HashMap::new()), cache_by_config: RwLock::new(HashMap::new()), @@ -88,6 +90,12 @@ impl SkillsService { service } + /// Reuses plugin-root snapshots populated by another owner of the loader. + pub fn with_skill_root_loader(mut self, skill_root_loader: Arc) -> Self { + self.skill_root_loader = skill_root_loader; + self + } + pub fn set_extra_roots(&self, extra_roots: Vec) { { let mut roots = self @@ -199,7 +207,7 @@ impl SkillsService { skill_config_rules: &SkillConfigRules, ) -> SkillLoadOutcome { let outcome = crate::filter_skill_load_outcome_for_product( - load_skills_from_roots(roots).await, + self.skill_root_loader.load_skills_from_roots(roots).await, self.restriction_product, ); let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules); @@ -207,6 +215,7 @@ impl SkillsService { } pub fn clear_cache(&self) { + self.skill_root_loader.clear_cache(); let cleared_cwd = { let mut cache = self .cache_by_cwd diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 63335c0129..6ed81cb059 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -281,11 +281,14 @@ impl ThreadManager { Arc::clone(&plugins_manager), Arc::clone(&extensions), )); - let skills_service = Arc::new(SkillsService::new_with_restriction_product( - codex_home, - config.bundled_skills_enabled(), - restriction_product, - )); + let skills_service = Arc::new( + SkillsService::new_with_restriction_product( + codex_home, + config.bundled_skills_enabled(), + restriction_product, + ) + .with_skill_root_loader(plugins_manager.skill_root_loader()), + ); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -373,11 +376,14 @@ impl ThreadManager { auth_manager.get_api_auth_mode(), )); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_service = Arc::new(SkillsService::new_with_restriction_product( - skills_codex_home, - /*bundled_skills_enabled*/ true, - restriction_product, - )); + let skills_service = Arc::new( + SkillsService::new_with_restriction_product( + skills_codex_home, + /*bundled_skills_enabled*/ true, + restriction_product, + ) + .with_skill_root_loader(plugins_manager.skill_root_loader()), + ); // 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(