diff --git a/codex-rs/app-server/src/request_processors/catalog_processor.rs b/codex-rs/app-server/src/request_processors/catalog_processor.rs index 38fd521e44..14a60482a7 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -514,17 +514,30 @@ impl CatalogRequestProcessor { plugins_manager.clear_cache(); skills_service.clear_cache(); } + // Plugin configuration is user-scoped; workspace skill rules are applied below. + let (effective_skill_roots, plugin_skill_snapshots) = if workspace_codex_plugins_enabled { + let plugins_input = config.plugins_config_input(); + let plugins = plugins_manager.plugins_for_config(&plugins_input).await; + ( + plugins.effective_plugin_skill_roots(), + plugins_manager.plugin_skill_snapshots_for_config(&plugins_input), + ) + } else { + (Vec::new(), None) + }; let fs = self .thread_manager .environment_manager() .default_environment() .map(|environment| environment.get_filesystem()); + let skills_request = skills_service.for_request(); let mut data = futures::stream::iter(cwds.into_iter().enumerate()) .map(|(index, cwd)| { let config = &config; let fs = fs.clone(); - let plugins_manager = &plugins_manager; - let skills_service = &skills_service; + let skills_request = &skills_request; + let effective_skill_roots = effective_skill_roots.clone(); + let plugin_skill_snapshots = plugin_skill_snapshots.clone(); async move { let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await { Ok(resolved) => resolved, @@ -543,31 +556,6 @@ impl CatalogRequestProcessor { ); } }; - let (effective_skill_roots, plugin_skill_snapshots) = - if workspace_codex_plugins_enabled { - let plugins_input = config.plugins_config_input(); - if config_layer_stack == plugins_input.config_layer_stack { - let plugins = - plugins_manager.plugins_for_config(&plugins_input).await; - ( - plugins.effective_plugin_skill_roots(), - plugins_manager - .plugin_skill_snapshots_for_config(&plugins_input), - ) - } else { - ( - plugins_manager - .effective_skill_roots_for_layer_stack( - &config_layer_stack, - &plugins_input, - ) - .await, - None, - ) - } - } else { - (Vec::new(), None) - }; let skills_input = codex_core::skills::HostSkillsLoadInput::new( cwd_abs.clone(), effective_skill_roots, @@ -575,7 +563,7 @@ impl CatalogRequestProcessor { config.bundled_skills_enabled(), ) .with_plugin_skill_snapshots(plugin_skill_snapshots); - let snapshot = skills_service + let snapshot = skills_request .snapshot_for_cwd(&skills_input, force_reload, fs) .await; let outcome = snapshot.outcome(); diff --git a/codex-rs/app-server/tests/suite/v2/skills_list.rs b/codex-rs/app-server/tests/suite/v2/skills_list.rs index 16079ddfcf..2685182748 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -24,9 +24,14 @@ use codex_app_server_protocol::SkillsListResponse; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_config::types::AuthCredentialsStoreMode; +use codex_core::config::set_project_trust_level; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; +use codex_exec_server::CreateDirectoryOptions; +use codex_protocol::config_types::TrustLevel; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::skip_if_remote; +use core_test_support::skip_if_wine_exec; use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::time::timeout; @@ -777,37 +782,194 @@ async fn skills_list_accepts_relative_cwds() -> Result<()> { #[tokio::test] async fn skills_list_preserves_requested_cwd_order() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "skills/list currently requires host-native cwd paths for workspace config" + ); let codex_home = TempDir::new()?; let first_cwd = TempDir::new()?; let second_cwd = TempDir::new()?; + write_skill(&codex_home, "shared-skill")?; + write_cached_local_curated_plugin_with_skill(codex_home.path())?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true + +[plugins."google-calendar@openai-curated"] +enabled = true +"#, + )?; + + for (cwd, plugin_enabled) in [(first_cwd.path(), true), (second_cwd.path(), false)] { + std::fs::create_dir_all(cwd.join(".git"))?; + std::fs::create_dir_all(cwd.join(".codex"))?; + std::fs::write( + cwd.join(".codex/config.toml"), + format!("[plugins.\"google-calendar@openai-curated\"]\nenabled = {plugin_enabled}\n"), + )?; + set_project_trust_level(codex_home.path(), cwd, TrustLevel::Trusted)?; + } let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) - .without_auto_env() .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; - let request_id = mcp - .send_skills_list_request(SkillsListParams { - cwds: vec![ - first_cwd.path().to_path_buf(), - second_cwd.path().to_path_buf(), - ], - force_reload: true, - }) + let file_system = mcp.auto_env()?.environment().get_filesystem(); + for (cwd, name) in [ + (first_cwd.path(), "first-project-skill"), + (second_cwd.path(), "second-project-skill"), + ] { + let cwd = AbsolutePathBuf::try_from(cwd)?; + let git_dir = PathUri::from_abs_path(&cwd.join(".git")); + let skill_dir = PathUri::from_abs_path(&cwd.join(".agents/skills").join(name)); + for directory in [&git_dir, &skill_dir] { + file_system + .create_directory( + directory, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + } + file_system + .write_file( + &skill_dir.join("SKILL.md")?, + format!("---\nname: {name}\ndescription: {name}\n---\n").into_bytes(), + /*sandbox*/ None, + ) + .await?; + } + + for (request_index, force_reload) in [false, false, true].into_iter().enumerate() { + if request_index == 1 { + write_skill(&codex_home, "new-skill")?; + } + + let request_id = mcp + .send_skills_list_request(SkillsListParams { + cwds: vec![ + first_cwd.path().to_path_buf(), + second_cwd.path().to_path_buf(), + ], + force_reload, + }) + .await?; + let SkillsListResponse { data } = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + assert_eq!(data.len(), 2); + assert_eq!(data[0].cwd, first_cwd.path()); + assert_eq!(data[1].cwd, second_cwd.path()); + for (entry, project_skill) in data + .iter() + .zip(["first-project-skill", "second-project-skill"]) + { + assert_eq!(entry.errors, Vec::new()); + assert!( + entry + .skills + .iter() + .any(|skill| skill.name == "shared-skill") + ); + assert_eq!( + entry + .skills + .iter() + .filter(|skill| { + skill.name.ends_with("-project-skill") + || skill.name.starts_with("google-calendar:") + }) + .map(|skill| skill.name.as_str()) + .collect::>(), + vec![project_skill, "google-calendar:meeting-prep"] + ); + assert_eq!( + entry.skills.iter().any(|skill| skill.name == "new-skill"), + force_reload + ); + } + } + + Ok(()) +} + +#[tokio::test] +async fn skills_list_force_reload_refreshes_cached_plugin_roots() -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "skills/list currently requires host-native cwd paths for workspace config" + ); + let codex_home = TempDir::new()?; + let first_cwd = TempDir::new()?; + let second_cwd = TempDir::new()?; + write_cached_local_curated_plugin_with_skill(codex_home.path())?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true + +[plugins."google-calendar@openai-curated"] +enabled = true +"#, + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) .await?; - let SkillsListResponse { data } = - timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; - assert_eq!( - data.iter() - .map(|entry| entry.cwd.clone()) - .collect::>(), - vec![ - first_cwd.path().to_path_buf(), - second_cwd.path().to_path_buf(), - ] - ); + let file_system = mcp.auto_env()?.environment().get_filesystem(); + for cwd in [first_cwd.path(), second_cwd.path()] { + let cwd = PathUri::from_abs_path(&AbsolutePathBuf::try_from(cwd)?); + file_system + .create_directory( + &cwd.join(".git")?, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + } + + for (cwd, force_reload, expected_skill) in [ + (first_cwd.path(), false, "google-calendar:meeting-prep"), + (first_cwd.path(), true, "google-calendar:refreshed-skill"), + (second_cwd.path(), false, "google-calendar:refreshed-skill"), + ] { + if force_reload { + let plugin_root = codex_home + .path() + .join("plugins/cache/openai-curated/google-calendar/local"); + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"google-calendar","skills":"./replacement-skills"}"#, + )?; + let skill_dir = plugin_root.join("replacement-skills/refreshed-skill"); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: refreshed-skill\ndescription: refreshed skill\n---\n", + )?; + } + + let request_id = mcp + .send_skills_list_request(SkillsListParams { + cwds: vec![cwd.to_path_buf()], + force_reload, + }) + .await?; + let SkillsListResponse { data } = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + assert_eq!( + data[0] + .skills + .iter() + .filter(|skill| skill.name.starts_with("google-calendar:")) + .map(|skill| skill.name.as_str()) + .collect::>(), + vec![expected_skill] + ); + } Ok(()) } diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 48cb599929..8ad903dc86 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -787,28 +787,6 @@ impl PluginsManager { } } - /// Load plugins for a config layer stack without touching the plugins cache. - pub async fn plugins_for_layer_stack( - &self, - config_layer_stack: &ConfigLayerStack, - config: &PluginsConfigInput, - ) -> PluginLoadOutcome { - if !config.plugins_enabled { - return PluginLoadOutcome::default(); - } - let plugins = load_plugins_from_layer_stack( - config_layer_stack, - self.remote_installed_plugins_snapshot(), - &self.store, - /*plugin_skill_snapshots*/ None, - self.restriction_product, - self.remote_global_catalog_active(config), - self.skill_root_loader.as_ref(), - ) - .await; - self.resolve_loaded_plugins_for_auth(plugins, &config.model_provider_id) - } - /// Resolve plugin hooks for a config layer stack without loading other plugin capabilities. pub async fn plugin_hooks_for_layer_stack( &self, @@ -830,17 +808,6 @@ impl PluginsManager { .await } - /// Resolve plugin skill roots for a config layer stack without touching the plugins cache. - pub async fn effective_skill_roots_for_layer_stack( - &self, - config_layer_stack: &ConfigLayerStack, - config: &PluginsConfigInput, - ) -> Vec { - self.plugins_for_layer_stack(config_layer_stack, config) - .await - .effective_plugin_skill_roots() - } - fn cached_loaded_plugins(&self, key: &PluginLoadCacheKey) -> Option> { self.loaded_plugins_cache .read() diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 877ba93dce..581529461e 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5183,6 +5183,7 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { let parent_snapshot = session .services .skills_service + .for_request() .snapshot_for_cwd( &crate::skills_load_input_from_config(&parent_config, Vec::new()), /*force_reload*/ true, diff --git a/codex-rs/ext/skills/src/host_service.rs b/codex-rs/ext/skills/src/host_service.rs index 199c6a0184..3c08db87da 100644 --- a/codex-rs/ext/skills/src/host_service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -33,8 +33,10 @@ use crate::HostSkillsSnapshot; use crate::SkillLoadOutcome; use crate::host_roots::resolve_skill_roots; use crate::loader::HostSkillRoot; +use crate::loader::HostSkillRootSnapshot; use crate::loader::MAX_CONCURRENT_ROOT_SCANS; use crate::loader::load_and_merge_host_skill_roots; +use crate::loader::load_and_merge_host_skill_roots_with_request_snapshots; #[derive(Debug, Clone)] pub struct HostSkillsLoadInput { @@ -84,6 +86,33 @@ pub struct HostSkillsService { root_scan_slots: Arc, } +pub(crate) type RequestSkillRootSnapshots = + RwLock>>>; + +/// Shares host-root scans between workspaces for the lifetime of one request. +pub struct HostSkillsRequest<'a> { + service: &'a HostSkillsService, + root_snapshots: RequestSkillRootSnapshots, +} + +impl HostSkillsRequest<'_> { + pub async fn snapshot_for_cwd( + &self, + input: &HostSkillsLoadInput, + force_reload: bool, + fs: Option>, + ) -> HostSkillsSnapshot { + self.service + .snapshot_for_cwd_with_root_snapshots( + input, + force_reload, + fs, + Some(&self.root_snapshots), + ) + .await + } +} + impl HostSkillsService { pub fn new(codex_home: AbsolutePathBuf, bundled_skills_enabled: bool) -> Self { Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex)) @@ -110,6 +139,14 @@ impl HostSkillsService { service } + /// Creates a request-local view without changing persistent plugin snapshots. + pub fn for_request(&self) -> HostSkillsRequest<'_> { + HostSkillsRequest { + service: self, + root_snapshots: RwLock::new(HashMap::new()), + } + } + pub fn set_extra_roots(&self, extra_roots: Vec) { { let mut roots = self @@ -155,6 +192,7 @@ impl HostSkillsService { &skill_config_rules, cache_key, /*force_reload*/ false, + /*request_root_snapshots*/ None, ) .await } @@ -181,11 +219,12 @@ impl HostSkillsService { roots } - pub async fn snapshot_for_cwd( + async fn snapshot_for_cwd_with_root_snapshots( &self, input: &HostSkillsLoadInput, force_reload: bool, fs: Option>, + request_root_snapshots: Option<&RequestSkillRootSnapshots>, ) -> HostSkillsSnapshot { let bundled_skills_enabled = bundled_skills_enabled_from_stack(&input.config_layer_stack); if bundled_skills_enabled { @@ -224,11 +263,12 @@ impl HostSkillsService { &skill_config_rules, cache_key, force_reload, + request_root_snapshots, ) .await } else { HostSkillsSnapshot::new(Arc::new( - self.build_skill_outcome(input, roots, &skill_config_rules) + self.build_skill_outcome(input, roots, &skill_config_rules, request_root_snapshots) .await, )) }; @@ -249,6 +289,7 @@ impl HostSkillsService { skill_config_rules: &SkillConfigRules, cache_key: ConfigSkillsCacheKey, force_reload: bool, + request_root_snapshots: Option<&RequestSkillRootSnapshots>, ) -> HostSkillsSnapshot { let snapshot_cell = { let mut cache = self @@ -271,8 +312,13 @@ impl HostSkillsService { snapshot_cell .get_or_init(|| async { HostSkillsSnapshot::new(Arc::new( - self.build_skill_outcome(input, roots, skill_config_rules) - .await, + self.build_skill_outcome( + input, + roots, + skill_config_rules, + request_root_snapshots, + ) + .await, )) }) .await @@ -285,12 +331,14 @@ impl HostSkillsService { input: &HostSkillsLoadInput, roots: Vec, skill_config_rules: &SkillConfigRules, + request_root_snapshots: Option<&RequestSkillRootSnapshots>, ) -> SkillLoadOutcome { - let outcome = load_and_merge_host_skill_roots( + let outcome = load_and_merge_host_skill_roots_with_request_snapshots( roots, &self.root_scan_slots, self.restriction_product, input.plugin_skill_snapshots.as_ref(), + request_root_snapshots, ) .await; let disabled_paths = skill_config_rules.resolve_disabled_paths( @@ -397,7 +445,7 @@ struct ConfigSkillsCacheKey { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -struct ConfigSkillRootCacheKey { +pub(crate) struct ConfigSkillRootCacheKey { path: AbsolutePathBuf, scope_rank: u8, plugin_identity: Option, @@ -450,24 +498,7 @@ fn config_skills_cache_key( plugin_skill_snapshots: Option<&SkillRootSnapshots>, ) -> ConfigSkillsCacheKey { ConfigSkillsCacheKey { - roots: roots - .iter() - .map(|root| { - let scope_rank = match root.scope { - SkillScope::Repo => 0, - SkillScope::User => 1, - SkillScope::System => 2, - SkillScope::Admin => 3, - }; - ConfigSkillRootCacheKey { - path: root.path.clone(), - scope_rank, - plugin_identity: root.plugin_identity().cloned(), - plugin_namespace: root.plugin_namespace().map(str::to_string), - file_system: FileSystemIdentity(Arc::downgrade(&root.file_system)), - } - }) - .collect(), + roots: roots.iter().map(config_skill_root_cache_key).collect(), skill_config_rules: skill_config_rules.clone(), plugin_skill_snapshots: plugin_skill_snapshots .filter(|_| roots.iter().any(|root| root.plugin_identity().is_some())) @@ -475,6 +506,22 @@ fn config_skills_cache_key( } } +pub(crate) fn config_skill_root_cache_key(root: &HostSkillRoot) -> ConfigSkillRootCacheKey { + let scope_rank = match root.scope { + SkillScope::Repo => 0, + SkillScope::User => 1, + SkillScope::System => 2, + SkillScope::Admin => 3, + }; + ConfigSkillRootCacheKey { + path: root.path.clone(), + scope_rank, + plugin_identity: root.plugin_identity().cloned(), + plugin_namespace: root.plugin_namespace().map(str::to_string), + file_system: FileSystemIdentity(Arc::downgrade(&root.file_system)), + } +} + #[cfg(test)] #[path = "host_service_tests.rs"] mod tests; diff --git a/codex-rs/ext/skills/src/host_service_tests.rs b/codex-rs/ext/skills/src/host_service_tests.rs index e650b27b34..d5217b3be2 100644 --- a/codex-rs/ext/skills/src/host_service_tests.rs +++ b/codex-rs/ext/skills/src/host_service_tests.rs @@ -305,6 +305,76 @@ async fn snapshot_for_config_preserves_host_precedence_for_symlinked_plugin_root ); } +#[tokio::test] +async fn skills_list_snapshots_share_host_roots_only_within_one_request() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let first_cwd = tempfile::tempdir().expect("tempdir"); + let second_cwd = tempfile::tempdir().expect("tempdir"); + let config_layer_stack = config_stack(&codex_home, ""); + let disabled_config_layer_stack = config_stack( + &codex_home, + &name_toggle_config("first-skill", /*enabled*/ false), + ); + let skills_service = HostSkillsService::new( + codex_home.path().abs(), + /*bundled_skills_enabled*/ true, + ); + let request = skills_service.for_request(); + let input = |cwd: &TempDir, config_layer_stack| { + HostSkillsLoadInput::new( + cwd.path().abs(), + Vec::new(), + config_layer_stack, + /*bundled_skills_enabled*/ true, + ) + }; + + write_user_skill(&codex_home, "first", "first-skill", "first skill"); + let first = request + .snapshot_for_cwd( + &input(&first_cwd, config_layer_stack.clone()), + /*force_reload*/ false, + Some(Arc::clone(&LOCAL_FS)), + ) + .await; + + write_user_skill(&codex_home, "second", "second-skill", "second skill"); + let second = request + .snapshot_for_cwd( + &input(&second_cwd, disabled_config_layer_stack), + /*force_reload*/ false, + Some(Arc::clone(&LOCAL_FS)), + ) + .await; + assert_eq!(second.outcome().skills, first.outcome().skills); + let first_skill_path = first + .outcome() + .skills + .iter() + .find(|skill| skill.name == "first-skill") + .expect("first skill should be discovered") + .path_to_skills_md + .clone(); + assert!(!first.outcome().disabled_paths.contains(&first_skill_path)); + assert!(second.outcome().disabled_paths.contains(&first_skill_path)); + + let third = skills_service + .for_request() + .snapshot_for_cwd( + &input(&first_cwd, config_layer_stack), + /*force_reload*/ true, + Some(Arc::clone(&LOCAL_FS)), + ) + .await; + assert!( + third + .outcome() + .skills + .iter() + .any(|skill| skill.name == "second-skill") + ); +} + #[tokio::test] async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -344,6 +414,7 @@ async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() { .clone() .with_plugin_skill_snapshots(/*plugin_skill_snapshots*/ None); let listing_snapshot = skills_service + .for_request() .snapshot_for_cwd( &listing_input, /*force_reload*/ false, @@ -405,6 +476,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { bundled_skills_enabled_from_stack(&config_layer_stack), ); let empty_snapshot = skills_service + .for_request() .snapshot_for_cwd( &skills_input, /*force_reload*/ false, @@ -430,6 +502,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { skills_service.set_extra_roots(vec![extra_skills_root.abs()]); let runtime_snapshot = skills_service + .for_request() .snapshot_for_cwd( &skills_input, /*force_reload*/ false, @@ -446,6 +519,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { skills_service.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]); let replaced_snapshot = skills_service + .for_request() .snapshot_for_cwd( &skills_input, /*force_reload*/ false, @@ -603,6 +677,7 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() { ); let snapshot = skills_service + .for_request() .snapshot_for_cwd( &skills_input, /*force_reload*/ true, @@ -673,6 +748,7 @@ async fn skills_for_cwd_without_fs_skips_repo_roots() { ); let snapshot = skills_service + .for_request() .snapshot_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None) .await; let outcome = snapshot.outcome(); @@ -742,9 +818,10 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { let config_input = base_input .clone() .with_plugin_skill_snapshots(Some(test_plugin_skill_snapshots())); + let request = skills_service.for_request(); let (config_snapshot, snapshot_a) = tokio::join!( skills_service.snapshot_for_config(&config_input, Some(Arc::clone(&LOCAL_FS))), - skills_service.snapshot_for_cwd( + request.snapshot_for_cwd( &base_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), @@ -765,6 +842,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { write_user_skill(&codex_home, "late", "late-skill", "added after cache"); let snapshot_b = skills_service + .for_request() .snapshot_for_cwd( &base_input, /*force_reload*/ false, @@ -780,6 +858,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { ); let snapshot_reloaded = skills_service + .for_request() .snapshot_for_cwd( &base_input, /*force_reload*/ true, @@ -825,6 +904,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() ); let parent_snapshot = skills_service + .for_request() .snapshot_for_cwd( &parent_input, /*force_reload*/ true, diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 6c770c28f3..5cf48c811c 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -32,6 +32,7 @@ pub use host_outcome::SkillLoadOutcome; pub use host_prompt::HostSkillPrompts; pub use host_prompt::InjectedHostSkillPrompts; pub use host_service::HostSkillsLoadInput; +pub use host_service::HostSkillsRequest; pub use host_service::HostSkillsService; pub use host_service::bundled_skills_enabled_from_stack; pub use host_snapshot::HostSkillsSnapshot; diff --git a/codex-rs/ext/skills/src/loader/host_merge.rs b/codex-rs/ext/skills/src/loader/host_merge.rs index 9db69bb8a7..ac309afe85 100644 --- a/codex-rs/ext/skills/src/loader/host_merge.rs +++ b/codex-rs/ext/skills/src/loader/host_merge.rs @@ -11,6 +11,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::migrated_command_skills_root; +use futures::FutureExt; use futures::StreamExt; use tokio::sync::OnceCell; use tokio::sync::Semaphore; @@ -20,12 +21,31 @@ use super::MAX_CONCURRENT_ROOT_SCANS; use super::host::HostSkillRootSnapshot; use super::load_host_skill_root; use crate::SkillLoadOutcome; +use crate::host_service::RequestSkillRootSnapshots; +use crate::host_service::config_skill_root_cache_key; pub(crate) async fn load_and_merge_host_skill_roots( roots: Vec, root_scan_slots: &Semaphore, restriction_product: Option, plugin_skill_snapshots: Option<&SkillRootSnapshots>, +) -> SkillLoadOutcome { + load_and_merge_host_skill_roots_with_request_snapshots( + roots, + root_scan_slots, + restriction_product, + plugin_skill_snapshots, + /*request_root_snapshots*/ None, + ) + .await +} + +pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots( + roots: Vec, + root_scan_slots: &Semaphore, + restriction_product: Option, + plugin_skill_snapshots: Option<&SkillRootSnapshots>, + request_root_snapshots: Option<&RequestSkillRootSnapshots>, ) -> SkillLoadOutcome { let migrated_command_roots_by_plugin = roots .iter() @@ -42,42 +62,63 @@ pub(crate) async fn load_and_merge_host_skill_roots( .cloned(); let plugin_skill_root = root.plugin_skill_root(); let file_system = Arc::clone(&root.file_system); - let _root_scan_slot = root_scan_slots - .acquire() - .await - .unwrap_or_else(|_| unreachable!()); - let cached_snapshot = plugin_skill_root - .as_ref() - .and_then(|plugin_skill_root| plugin_skill_snapshots?.get(plugin_skill_root)); - let snapshot = match cached_snapshot { - Some(snapshot) => HostSkillRootSnapshot { - root: snapshot.root, - skills: snapshot.skills, - skill_discovery_path_by_path: snapshot.skill_discovery_path_by_path, - errors: snapshot.errors, - file_system: Arc::clone(&file_system), - is_agent_plugin: snapshot.is_agent_plugin, - }, - None => { - let snapshot = load_host_skill_root(root).await; - if let Some(plugin_skill_snapshots) = plugin_skill_snapshots - && let Some(plugin_skill_root) = plugin_skill_root - { - plugin_skill_snapshots.insert( - plugin_skill_root, - LoadedSkillRoot { - root: snapshot.root.clone(), - skills: snapshot.skills.clone(), - skill_discovery_path_by_path: Arc::clone( - &snapshot.skill_discovery_path_by_path, - ), - errors: snapshot.errors.clone(), - is_agent_plugin: snapshot.is_agent_plugin, - }, - ); + let request_snapshot = if plugin_skill_root.is_none() { + request_root_snapshots.map(|snapshots| { + Arc::clone( + snapshots + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(config_skill_root_cache_key(&root)) + .or_insert_with(|| Arc::new(OnceCell::new())), + ) + }) + } else { + None + }; + let load = async { + let _root_scan_slot = root_scan_slots + .acquire() + .await + .unwrap_or_else(|_| unreachable!()); + let cached_snapshot = plugin_skill_root + .as_ref() + .and_then(|plugin_skill_root| plugin_skill_snapshots?.get(plugin_skill_root)); + match cached_snapshot { + Some(snapshot) => HostSkillRootSnapshot { + root: snapshot.root, + skills: snapshot.skills, + skill_discovery_path_by_path: snapshot.skill_discovery_path_by_path, + errors: snapshot.errors, + file_system: Arc::clone(&file_system), + is_agent_plugin: snapshot.is_agent_plugin, + }, + None => { + let snapshot = load_host_skill_root(root).await; + if let Some(plugin_skill_snapshots) = plugin_skill_snapshots + && let Some(plugin_skill_root) = plugin_skill_root + { + plugin_skill_snapshots.insert( + plugin_skill_root, + LoadedSkillRoot { + root: snapshot.root.clone(), + skills: snapshot.skills.clone(), + skill_discovery_path_by_path: Arc::clone( + &snapshot.skill_discovery_path_by_path, + ), + errors: snapshot.errors.clone(), + is_agent_plugin: snapshot.is_agent_plugin, + }, + ); + } + snapshot } - snapshot } + } + .boxed(); + let snapshot = if let Some(snapshot) = request_snapshot { + snapshot.get_or_init(|| load).await.clone() + } else { + load.await }; let migrated_command_root = match (plugin_root.as_ref(), migrated_command_root) { (Some(plugin_root), Some(migrated_command_root)) => Some( diff --git a/codex-rs/ext/skills/src/loader/mod.rs b/codex-rs/ext/skills/src/loader/mod.rs index a94cc6a0c3..5d7476bfe3 100644 --- a/codex-rs/ext/skills/src/loader/mod.rs +++ b/codex-rs/ext/skills/src/loader/mod.rs @@ -10,8 +10,10 @@ mod namespace; pub(crate) use environment::load_environment_skills_from_discovery; pub(crate) use environment::load_environment_skills_from_root; pub use host::HostSkillRoot; +pub(crate) use host::HostSkillRootSnapshot; pub(crate) use host::load_host_skill_root; pub(crate) use host_merge::load_and_merge_host_skill_roots; +pub(crate) use host_merge::load_and_merge_host_skill_roots_with_request_snapshots; pub(crate) const MAX_CONCURRENT_ROOT_SCANS: usize = 8; pub(super) const SKILLS_FILENAME: &str = "SKILL.md";