diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 3c3600008..985160985 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -379,7 +379,7 @@ impl MessageProcessor { thread_manager .plugins_manager() .set_analytics_events_client(analytics_events_client.clone()); - let skills_watcher = SkillsWatcher::new(thread_manager.skills_manager(), outgoing.clone()); + let skills_watcher = SkillsWatcher::new(thread_manager.skills_service(), outgoing.clone()); let pending_thread_unloads = Arc::new(Mutex::new(HashSet::new())); let thread_watch_manager = diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 1fa0961dd..5d234f700 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -219,7 +219,7 @@ impl AccountRequestProcessor { ) { tokio::spawn(async move { thread_manager.plugins_manager().clear_cache(); - thread_manager.skills_manager().clear_cache(); + thread_manager.skills_service().clear_cache(); if thread_manager.list_thread_ids().await.is_empty() { return; } 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 9c0cc1c8b..f61d0ed4b 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -511,7 +511,7 @@ impl CatalogRequestProcessor { let workspace_codex_plugins_enabled = self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await; - let skills_manager = self.thread_manager.skills_manager(); + let skills_service = self.thread_manager.skills_service(); let plugins_manager = self.thread_manager.plugins_manager(); let fs = self .thread_manager @@ -523,7 +523,7 @@ impl CatalogRequestProcessor { let config = &config; let fs = fs.clone(); let plugins_manager = &plugins_manager; - let skills_manager = &skills_manager; + let skills_service = &skills_service; async move { let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await { Ok(resolved) => resolved, @@ -559,9 +559,10 @@ impl CatalogRequestProcessor { config_layer_stack, config.bundled_skills_enabled(), ); - let outcome = skills_manager - .skills_for_cwd(&skills_input, force_reload, fs) + let snapshot = skills_service + .snapshot_for_cwd(&skills_input, force_reload, fs) .await; + let outcome = snapshot.outcome(); let errors = errors_to_info(&outcome.errors); let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths); ( @@ -590,7 +591,7 @@ impl CatalogRequestProcessor { self.skills_watcher .register_runtime_extra_roots(&extra_roots); self.thread_manager - .skills_manager() + .skills_service() .set_extra_roots(extra_roots); self.outgoing .send_server_notification(ServerNotification::SkillsChanged( @@ -703,7 +704,7 @@ impl CatalogRequestProcessor { .await .map(|()| { self.thread_manager.plugins_manager().clear_cache(); - self.thread_manager.skills_manager().clear_cache(); + self.thread_manager.skills_service().clear_cache(); SkillsConfigWriteResponse { effective_enabled: enabled, } diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index 837018107..15c3b7c86 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -169,7 +169,7 @@ impl ConfigRequestProcessor { pub(crate) async fn handle_config_mutation(&self) { self.thread_manager.plugins_manager().clear_cache(); - self.thread_manager.skills_manager().clear_cache(); + self.thread_manager.skills_service().clear_cache(); } async fn handle_config_mutation_result( diff --git a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs b/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs index 1a9002dd1..b3cdb0e21 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs +++ b/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs @@ -319,7 +319,7 @@ impl ExternalAgentConfigRequestProcessor { completed_item_results.extend(background_item_results); if has_plugin_imports { thread_manager.plugins_manager().clear_cache(); - thread_manager.skills_manager().clear_cache(); + thread_manager.skills_service().clear_cache(); } send_completed_import_notification( &outgoing, diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 609d89459..d2cd08b63 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -474,7 +474,7 @@ impl PluginRequestProcessor { ) { tokio::spawn(async move { thread_manager.plugins_manager().clear_cache(); - thread_manager.skills_manager().clear_cache(); + thread_manager.skills_service().clear_cache(); if thread_manager.list_thread_ids().await.is_empty() { return; } @@ -484,7 +484,7 @@ impl PluginRequestProcessor { fn clear_plugin_related_caches(&self) { self.thread_manager.plugins_manager().clear_cache(); - self.thread_manager.skills_manager().clear_cache(); + self.thread_manager.skills_service().clear_cache(); } async fn load_latest_config( diff --git a/codex-rs/app-server/src/skills_watcher.rs b/codex-rs/app-server/src/skills_watcher.rs index 57edb89c7..086e076d4 100644 --- a/codex-rs/app-server/src/skills_watcher.rs +++ b/codex-rs/app-server/src/skills_watcher.rs @@ -8,7 +8,7 @@ use codex_app_server_protocol::SkillsChangedNotification; use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::skills::SkillsLoadInput; -use codex_core::skills::SkillsManager; +use codex_core::skills::SkillsService; use codex_file_watcher::FileWatcher; use codex_file_watcher::FileWatcherSubscriber; use codex_file_watcher::Receiver; @@ -35,7 +35,7 @@ pub(crate) struct SkillsWatcher { impl SkillsWatcher { pub(crate) fn new( - skills_manager: Arc, + skills_service: Arc, outgoing: Arc, ) -> Arc { let file_watcher = match FileWatcher::new() { @@ -48,7 +48,7 @@ impl SkillsWatcher { let (subscriber, rx) = file_watcher.add_subscriber(); let shutdown_token = CancellationToken::new(); let shutdown_drop_guard = shutdown_token.clone().drop_guard(); - Self::spawn_event_loop(rx, skills_manager, outgoing, shutdown_token.child_token()); + Self::spawn_event_loop(rx, skills_service, outgoing, shutdown_token.child_token()); Arc::new(Self { subscriber, runtime_extra_roots_registration: Mutex::new(WatchRegistration::default()), @@ -110,7 +110,7 @@ impl SkillsWatcher { config.bundled_skills_enabled(), ); let roots = thread_manager - .skills_manager() + .skills_service() .skill_roots_for_config(&skills_input, Some(environment.get_filesystem())) .await .into_iter() @@ -124,7 +124,7 @@ impl SkillsWatcher { fn spawn_event_loop( rx: Receiver, - skills_manager: Arc, + skills_service: Arc, outgoing: Arc, shutdown_token: CancellationToken, ) { @@ -142,7 +142,7 @@ impl SkillsWatcher { if event.is_none() { break; } - skills_manager.clear_cache(); + skills_service.clear_cache(); outgoing .send_server_notification(ServerNotification::SkillsChanged( SkillsChangedNotification {}, diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index df4e6ea8a..aac0e2168 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -45,7 +45,7 @@ pub use codex_core::config::ThreadStoreConfig; pub use codex_core::config::find_codex_home; pub use codex_core::init_state_db; pub use codex_core::resolve_installation_id; -pub use codex_core::skills::SkillsManager; +pub use codex_core::skills::SkillsService; pub use codex_core::thread_store_from_config; pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::ExecServerError; diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 0390302af..31e5a7c53 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -2,20 +2,18 @@ pub mod config_rules; pub mod injection; pub(crate) mod invocation_utils; pub mod loader; -pub mod manager; mod mention_counts; pub mod model; pub mod remote; pub mod render; +pub mod service; mod skill_instructions; pub mod system; pub(crate) use invocation_utils::build_implicit_skill_path_indexes; pub use invocation_utils::detect_implicit_skill_invocation_for_command; -pub use manager::SkillsLoadInput; -pub use manager::SkillsManager; pub use mention_counts::build_skill_name_counts; -pub use model::HostLoadedSkills; +pub use model::HostSkillsSnapshot; pub use model::SkillError; pub use model::SkillLoadOutcome; pub use model::SkillMetadata; @@ -31,4 +29,6 @@ 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 service::SkillsLoadInput; +pub use service::SkillsService; pub use skill_instructions::SkillInstructions; diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index 3abb30c96..6afbde6cc 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -132,14 +132,14 @@ impl SkillLoadOutcome { } } -/// Host-loaded skills for one turn, including the filesystem mapping needed to -/// read skill bodies through the environment that loaded them. +/// Immutable snapshot of host-owned skills and the filesystem mapping needed +/// to read each skill through the environment that discovered it. #[derive(Debug, Clone)] -pub struct HostLoadedSkills { +pub struct HostSkillsSnapshot { outcome: Arc, } -impl HostLoadedSkills { +impl HostSkillsSnapshot { pub fn new(outcome: Arc) -> Self { Self { outcome } } diff --git a/codex-rs/core-skills/src/manager.rs b/codex-rs/core-skills/src/service.rs similarity index 85% rename from codex-rs/core-skills/src/manager.rs rename to codex-rs/core-skills/src/service.rs index f8c5bfd04..29d34c380 100644 --- a/codex-rs/core-skills/src/manager.rs +++ b/codex-rs/core-skills/src/service.rs @@ -13,6 +13,7 @@ use tracing::info; use tracing::instrument; use tracing::warn; +use crate::HostSkillsSnapshot; use crate::SkillLoadOutcome; use crate::build_implicit_skill_path_indexes; use crate::config_rules::SkillConfigRules; @@ -49,15 +50,18 @@ impl SkillsLoadInput { } } -pub struct SkillsManager { +/// Owns host skill discovery, immutable snapshots, cache invalidation, and extra roots. +/// +/// Source-specific model exposure remains the responsibility of the skills extension. +pub struct SkillsService { codex_home: AbsolutePathBuf, restriction_product: Option, extra_roots: RwLock>, - cache_by_cwd: RwLock>, - cache_by_config: RwLock>, + cache_by_cwd: RwLock>, + cache_by_config: RwLock>, } -impl SkillsManager { +impl SkillsService { pub fn new(codex_home: AbsolutePathBuf, bundled_skills_enabled: bool) -> Self { Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex)) } @@ -67,7 +71,7 @@ impl SkillsManager { bundled_skills_enabled: bool, restriction_product: Option, ) -> Self { - let manager = Self { + let service = Self { codex_home, restriction_product, extra_roots: RwLock::new(Vec::new()), @@ -77,11 +81,11 @@ impl SkillsManager { if !bundled_skills_enabled { // The loader caches bundled skills under `skills/.system`. Clearing that directory is // best-effort cleanup; root selection still enforces the config even if removal fails. - uninstall_system_skills(&manager.codex_home); - } else if let Err(err) = install_system_skills(&manager.codex_home) { + uninstall_system_skills(&service.codex_home); + } else if let Err(err) = install_system_skills(&service.codex_home) { tracing::error!("failed to install system skills: {err}"); } - manager + service } pub fn set_extra_roots(&self, extra_roots: Vec) { @@ -102,25 +106,27 @@ impl SkillsManager { /// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen /// to share a directory. #[instrument(level = "trace", skip_all)] - pub async fn skills_for_config( + pub async fn snapshot_for_config( &self, input: &SkillsLoadInput, fs: Option>, - ) -> SkillLoadOutcome { + ) -> HostSkillsSnapshot { let roots = self.skill_roots_for_config(input, fs).await; let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let cache_key = config_skills_cache_key(&roots, &skill_config_rules); - if let Some(outcome) = self.cached_outcome_for_config(&cache_key) { - return outcome; + if let Some(snapshot) = self.cached_snapshot_for_config(&cache_key) { + return snapshot; } - let outcome = self.build_skill_outcome(roots, &skill_config_rules).await; + let snapshot = HostSkillsSnapshot::new(Arc::new( + self.build_skill_outcome(roots, &skill_config_rules).await, + )); let mut cache = self .cache_by_config .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.insert(cache_key, outcome.clone()); - outcome + cache.insert(cache_key, snapshot.clone()); + snapshot } pub async fn skill_roots_for_config( @@ -142,18 +148,18 @@ impl SkillsManager { roots } - pub async fn skills_for_cwd( + pub async fn snapshot_for_cwd( &self, input: &SkillsLoadInput, force_reload: bool, fs: Option>, - ) -> SkillLoadOutcome { + ) -> HostSkillsSnapshot { let use_cwd_cache = fs.is_some(); if use_cwd_cache && !force_reload - && let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd) + && let Some(snapshot) = self.cached_snapshot_for_cwd(&input.cwd) { - return outcome; + return snapshot; } let mut roots = skill_roots( @@ -168,15 +174,17 @@ impl SkillsManager { roots.retain(|root| root.scope != SkillScope::System); } let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); - let outcome = self.build_skill_outcome(roots, &skill_config_rules).await; + let snapshot = HostSkillsSnapshot::new(Arc::new( + self.build_skill_outcome(roots, &skill_config_rules).await, + )); if use_cwd_cache { let mut cache = self .cache_by_cwd .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.insert(input.cwd.clone(), outcome.clone()); + cache.insert(input.cwd.clone(), snapshot.clone()); } - outcome + snapshot } #[instrument(level = "trace", skip_all)] @@ -216,17 +224,17 @@ impl SkillsManager { info!("skills cache cleared ({cleared} entries)"); } - fn cached_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option { + fn cached_snapshot_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option { match self.cache_by_cwd.read() { Ok(cache) => cache.get(cwd).cloned(), Err(err) => err.into_inner().get(cwd).cloned(), } } - fn cached_outcome_for_config( + fn cached_snapshot_for_config( &self, cache_key: &ConfigSkillsCacheKey, - ) -> Option { + ) -> Option { match self.cache_by_config.read() { Ok(cache) => cache.get(cache_key).cloned(), Err(err) => err.into_inner().get(cache_key).cloned(), @@ -303,5 +311,5 @@ fn finalize_skill_outcome( } #[cfg(test)] -#[path = "manager_tests.rs"] +#[path = "service_tests.rs"] mod tests; diff --git a/codex-rs/core-skills/src/manager_tests.rs b/codex-rs/core-skills/src/service_tests.rs similarity index 91% rename from codex-rs/core-skills/src/manager_tests.rs rename to codex-rs/core-skills/src/service_tests.rs index 713b94d7b..bc05ebf35 100644 --- a/codex-rs/core-skills/src/manager_tests.rs +++ b/codex-rs/core-skills/src/service_tests.rs @@ -159,7 +159,7 @@ enabled = {enabled} } async fn skills_for_config_with_stack( - skills_manager: &SkillsManager, + skills_service: &SkillsService, cwd: &TempDir, config_layer_stack: &ConfigLayerStack, effective_skill_roots: &[PluginSkillRoot], @@ -170,9 +170,11 @@ async fn skills_for_config_with_stack( config_layer_stack.clone(), bundled_skills_enabled_from_stack(config_layer_stack), ); - skills_manager - .skills_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS))) + skills_service + .snapshot_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS))) .await + .outcome() + .clone() } #[test] @@ -183,7 +185,7 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() { fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n") .expect("write stale system skill"); - let _skills_manager = SkillsManager::new( + let _skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ false, ); @@ -199,14 +201,14 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() { let codex_home = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir"); let config_layer_stack = config_stack(&codex_home, ""); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); write_user_skill(&codex_home, "a", "skill-a", "from a"); let outcome1 = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert!( outcome1.skills.iter().any(|s| s.name == "skill-a"), "expected skill-a to be discovered" @@ -216,7 +218,7 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() { // entry because the effective skill config is unchanged. write_user_skill(&codex_home, "b", "skill-b", "from b"); let outcome2 = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert_eq!(outcome2.errors, outcome1.errors); assert_eq!(outcome2.skills, outcome1.skills); } @@ -227,7 +229,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { let cwd = tempfile::tempdir().expect("tempdir"); let extra_root = tempfile::tempdir().expect("tempdir"); let config_layer_stack = config_stack(&codex_home, ""); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -238,13 +240,14 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let empty_outcome = skills_manager - .skills_for_cwd( + let empty_snapshot = skills_service + .snapshot_for_cwd( &skills_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), ) .await; + let empty_outcome = empty_snapshot.outcome(); assert!( empty_outcome .skills @@ -260,15 +263,16 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { "---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n", ) .expect("write skill"); - skills_manager.set_extra_roots(vec![extra_skills_root.abs()]); + skills_service.set_extra_roots(vec![extra_skills_root.abs()]); - let runtime_outcome = skills_manager - .skills_for_cwd( + let runtime_snapshot = skills_service + .snapshot_for_cwd( &skills_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), ) .await; + let runtime_outcome = runtime_snapshot.outcome(); assert!( runtime_outcome .skills @@ -276,14 +280,15 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { .any(|skill| skill.name == "runtime-skill") ); - skills_manager.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]); - let replaced_outcome = skills_manager - .skills_for_cwd( + skills_service.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]); + let replaced_snapshot = skills_service + .snapshot_for_cwd( &skills_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), ) .await; + let replaced_outcome = replaced_snapshot.outcome(); assert_eq!(replaced_outcome.errors, Vec::new()); assert!( replaced_outcome @@ -299,13 +304,13 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() { let cwd = tempfile::tempdir().expect("tempdir"); let extra_root = tempfile::tempdir().expect("tempdir"); let config_layer_stack = config_stack(&codex_home, ""); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let empty_outcome = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert!( empty_outcome .skills @@ -321,10 +326,10 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() { "---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n", ) .expect("write skill"); - skills_manager.set_extra_roots(vec![extra_skills_root.abs()]); + skills_service.set_extra_roots(vec![extra_skills_root.abs()]); let runtime_outcome = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert!( runtime_outcome .skills @@ -332,9 +337,9 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() { .any(|skill| skill.name == "runtime-skill") ); - skills_manager.set_extra_roots(Vec::new()); + skills_service.set_extra_roots(Vec::new()); let cleared_outcome = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert!( cleared_outcome .skills @@ -360,13 +365,13 @@ async fn skills_for_config_disables_plugin_skills_by_name() { &name_toggle_config("sample:sample-search", /*enabled*/ false), ); let plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test"); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let outcome = skills_for_config_with_stack( - &skills_manager, + &skills_service, &cwd, &config_layer_stack, &[plugin_skill_root], @@ -427,18 +432,19 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() { config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let outcome = skills_manager - .skills_for_cwd( + let snapshot = skills_service + .snapshot_for_cwd( &skills_input, /*force_reload*/ true, Some(Arc::clone(&LOCAL_FS)), ) .await; + let outcome = snapshot.outcome(); assert!( outcome.errors.is_empty(), @@ -490,14 +496,15 @@ async fn skills_for_cwd_without_fs_skips_repo_roots() { config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let outcome = skills_manager - .skills_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None) + let snapshot = skills_service + .snapshot_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None) .await; + let outcome = snapshot.outcome(); assert!( outcome.errors.is_empty(), @@ -525,7 +532,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() { ) .expect("write bundled skill"); let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n"); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ false, ); @@ -540,7 +547,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() { .expect("rewrite bundled skill"); let outcome = - skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; assert!( outcome .skills @@ -560,24 +567,25 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { let codex_home = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir"); let config_layer_stack = config_stack(&codex_home, ""); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; + let _ = skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; let base_input = SkillsLoadInput::new( cwd.path().abs(), Vec::new(), config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let outcome_a = skills_manager - .skills_for_cwd( + let snapshot_a = skills_service + .snapshot_for_cwd( &base_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), ) .await; + let outcome_a = snapshot_a.outcome(); assert!( outcome_a .skills @@ -587,13 +595,14 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { write_user_skill(&codex_home, "late", "late-skill", "added after cache"); - let outcome_b = skills_manager - .skills_for_cwd( + let snapshot_b = skills_service + .snapshot_for_cwd( &base_input, /*force_reload*/ false, Some(Arc::clone(&LOCAL_FS)), ) .await; + let outcome_b = snapshot_b.outcome(); assert!( outcome_b .skills @@ -601,13 +610,14 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { .all(|skill| skill.name != "late-skill") ); - let outcome_reloaded = skills_manager - .skills_for_cwd( + let snapshot_reloaded = skills_service + .snapshot_for_cwd( &base_input, /*force_reload*/ true, Some(Arc::clone(&LOCAL_FS)), ) .await; + let outcome_reloaded = snapshot_reloaded.outcome(); assert!( outcome_reloaded .skills @@ -775,7 +785,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() let parent_stack = config_stack(&codex_home, &disabled_skill_config); let child_stack = config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config); - let skills_manager = SkillsManager::new( + let skills_service = SkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -786,13 +796,14 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() bundled_skills_enabled_from_stack(&parent_stack), ); - let parent_outcome = skills_manager - .skills_for_cwd( + let parent_snapshot = skills_service + .snapshot_for_cwd( &parent_input, /*force_reload*/ true, Some(Arc::clone(&LOCAL_FS)), ) .await; + let parent_outcome = parent_snapshot.outcome(); let parent_skill = parent_outcome .skills .iter() @@ -801,7 +812,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false); let child_outcome = - skills_for_config_with_stack(&skills_manager, &cwd, &child_stack, &[]).await; + skills_for_config_with_stack(&skills_service, &cwd, &child_stack, &[]).await; let child_skill = child_outcome .skills .iter() diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 5461323a3..e258c0ad9 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::SkillsManager; +use crate::SkillsService; use crate::config::ConfigBuilder; use crate::skills_load_input_from_config; use codex_config::ConfigLayerStackOrdering; @@ -416,18 +416,19 @@ enabled = false .expect("custom role should apply"); let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf())); - let skills_manager = - SkillsManager::new(home.path().abs(), /*bundled_skills_enabled*/ true); + let skills_service = + SkillsService::new(home.path().abs(), /*bundled_skills_enabled*/ true); let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(&config, effective_skill_roots); - let outcome = skills_manager - .skills_for_config( + let snapshot = skills_service + .snapshot_for_config( &skills_input, Some(Arc::clone(&codex_exec_server::LOCAL_FS)), ) .await; + let outcome = snapshot.outcome(); let skill = outcome .skills .iter() diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index d31dc138b..16398a2f2 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -94,7 +94,7 @@ pub(crate) async fn run_codex_thread_interactive( .services .turn_environments .environment_manager(), - skills_manager: Arc::clone(&parent_session.services.skills_manager), + skills_service: Arc::clone(&parent_session.services.skills_service), plugins_manager: Arc::clone(&parent_session.services.plugins_manager), mcp_manager: Arc::clone(&parent_session.services.mcp_manager), extensions: Arc::clone(&parent_session.services.extensions), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3a6120b9f..72d3d45c9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -3874,7 +3874,7 @@ impl Config { } pub fn bundled_skills_enabled(&self) -> bool { - crate::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack) + crate::skills::service::bundled_skills_enabled_from_stack(&self.config_layer_stack) } } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index fefe15cb7..b19cd9d35 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -1678,7 +1678,7 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot() "---\nname: {GUARDIAN_SKILL_NAME}\ndescription: Guardian skill injection probe.\n---\n\n{GUARDIAN_SKILL_BODY_PROBE}\n" ), )?; - session.services.skills_manager.clear_cache(); + session.services.skills_service.clear_cache(); turn.config = Arc::clone(&config); turn.provider = create_model_provider(config.model_provider.clone(), turn.auth_manager.clone()); let session = Arc::new(session); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 073c06ced..0641d7226 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -87,16 +87,14 @@ mod session_prefix; mod session_startup_prewarm; pub mod skills; pub(crate) use skills::SkillInjections; -pub(crate) use skills::SkillLoadOutcome; pub(crate) use skills::SkillMetadata; -pub(crate) use skills::SkillsManager; +pub(crate) use skills::SkillsService; pub(crate) use skills::build_available_skills; pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_explicit_skill_mentions; pub(crate) use skills::default_skill_metadata_budget; pub(crate) use skills::injection; -pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; pub(crate) use skills::skills_load_input_from_config; mod stream_events_utils; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index d61de71bc..01d6246ce 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -291,11 +291,9 @@ pub(crate) struct PreviousTurnSettings { pub(crate) realtime_active: Option, } -#[cfg(test)] -use crate::SkillLoadOutcome; #[cfg(test)] use crate::SkillMetadata; -use crate::SkillsManager; +use crate::SkillsService; use crate::agents_md::load_project_instructions; use crate::exec_policy::ExecPolicyUpdateError; use crate::guardian::GuardianReviewSessionManager; @@ -304,6 +302,8 @@ use crate::network_policy_decision::execpolicy_network_rule_amendment; use crate::rollout::map_session_init_error; use crate::session_startup_prewarm::SessionStartupPrewarmHandle; use crate::shell; +#[cfg(test)] +use crate::skills::SkillLoadOutcome; use crate::state::AutoCompactWindowSnapshot; use crate::state::PendingRequestPermissions; use crate::state::SessionServices; @@ -409,7 +409,7 @@ pub(crate) struct CodexSpawnArgs { pub(crate) auth_manager: Arc, pub(crate) models_manager: SharedModelsManager, pub(crate) environment_manager: Arc, - pub(crate) skills_manager: Arc, + pub(crate) skills_service: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, pub(crate) extensions: Arc>, @@ -495,7 +495,7 @@ impl Codex { auth_manager, models_manager, environment_manager, - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions, @@ -651,7 +651,7 @@ impl Codex { agent_status_tx.clone(), conversation_history, session_source_clone, - skills_manager, + skills_service, plugins_manager, mcp_manager.clone(), extensions, @@ -1507,7 +1507,7 @@ impl Session { (previous_config, new_config, config) }; self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref()); - self.services.skills_manager.clear_cache(); + self.services.skills_service.clear_cache(); self.services.plugins_manager.clear_cache(); let environments = self.services.turn_environments.snapshot().await; let hooks = build_hooks_for_config( @@ -2969,7 +2969,7 @@ impl Session { } if turn_context.config.include_skill_instructions { let available_skills = build_available_skills( - &turn_context.turn_skills.outcome, + turn_context.turn_skills.snapshot.outcome(), default_skill_metadata_budget(turn_context.model_info.context_window), SkillRenderSideEffects::ThreadStart { session_telemetry: &self.services.session_telemetry, diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 0296d9e44..bbdfc3f98 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -1,5 +1,4 @@ use super::*; -use codex_core_skills::HostLoadedSkills; use std::sync::atomic::AtomicBool; /// Spawn a review thread using the given prompt. @@ -100,9 +99,7 @@ pub(super) async fn spawn_review_thread( let extension_data = Arc::new(codex_extension_api::ExtensionData::new( review_turn_id.clone(), )); - extension_data.insert(HostLoadedSkills::new( - parent_turn_context.turn_skills.outcome.clone(), - )); + extension_data.insert(parent_turn_context.turn_skills.snapshot.clone()); let review_turn_context = TurnContext { sub_id: review_turn_id.clone(), @@ -138,7 +135,7 @@ pub(super) async fn spawn_review_thread( dynamic_tools: parent_turn_context.dynamic_tools.clone(), turn_metadata_state, extension_data, - turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()), + turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.snapshot.clone()), turn_timing_state: Arc::new(TurnTimingState::default()), terminal_error: Arc::new(Mutex::new(None)), server_model_warning_emitted: AtomicBool::new(false), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 578152814..c4bc064a4 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -438,7 +438,7 @@ pub(crate) struct AppServerClientMetadata { async fn warm_plugins_and_skills_for_session_init( config: Arc, plugins_manager: Arc, - skills_manager: Arc, + skills_service: Arc, turn_environments: &TurnEnvironmentSnapshot, ) -> Vec { let fs = turn_environments.primary_filesystem(); @@ -446,10 +446,12 @@ async fn warm_plugins_and_skills_for_session_init( let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots); - skills_manager - .skills_for_config(&skills_input, fs) + skills_service + .snapshot_for_config(&skills_input, fs) .await + .outcome() .errors + .clone() } impl Session { @@ -477,7 +479,7 @@ impl Session { agent_status: watch::Sender, initial_history: InitialHistory, session_source: SessionSource, - skills_manager: Arc, + skills_service: Arc, plugins_manager: Arc, mcp_manager: Arc, extensions: Arc>, @@ -828,7 +830,7 @@ impl Session { let plugin_skill_errors = warm_plugins_and_skills_for_session_init( Arc::clone(&config), Arc::clone(&plugins_manager), - Arc::clone(&skills_manager), + Arc::clone(&skills_service), &resolved_environments, ) .instrument(info_span!( @@ -999,7 +1001,7 @@ impl Session { guardian_rejections: Mutex::new(HashMap::new()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), - skills_manager, + skills_service, plugins_manager: Arc::clone(&plugins_manager), mcp_manager: Arc::clone(&mcp_manager), extensions, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index a26fb6599..84c4a3888 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -24,6 +24,7 @@ use codex_config::RequirementSource; use codex_config::Sourced; use codex_config::loader::project_trust_key; use codex_config::types::ToolSuggestDisabledTool; +use codex_core_skills::HostSkillsSnapshot; use core_test_support::test_codex::local_selections; use codex_features::Feature; @@ -4434,15 +4435,16 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { .default_environment() .map(|environment| environment.get_filesystem()) .unwrap_or_else(|| std::sync::Arc::clone(&codex_exec_server::LOCAL_FS)); - let parent_outcome = session + let parent_snapshot = session .services - .skills_manager - .skills_for_cwd( + .skills_service + .snapshot_for_cwd( &crate::skills_load_input_from_config(&parent_config, Vec::new()), /*force_reload*/ true, Some(Arc::clone(&skill_fs)), ) .await; + let parent_outcome = parent_snapshot.outcome(); let parent_skill = parent_outcome .skills .iter() @@ -4488,13 +4490,18 @@ enabled = false .await; let child_skill = child_turn .turn_skills - .outcome + .snapshot + .outcome() .skills .iter() .find(|skill| skill.name == "demo-skill") .expect("demo skill should be discovered"); assert_eq!( - child_turn.turn_skills.outcome.is_skill_enabled(child_skill), + child_turn + .turn_skills + .snapshot + .outcome() + .is_skill_enabled(child_skill), false ); } @@ -4825,7 +4832,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -4842,7 +4849,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { agent_status_tx, InitialHistory::New, SessionSource::Exec, - skills_manager, + skills_service, plugins_manager, mcp_manager, Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), @@ -4961,7 +4968,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { ); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -5000,7 +5007,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { guardian_rejections: Mutex::new(std::collections::HashMap::new()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), @@ -5045,12 +5052,10 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { let skills_input = crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skill_fs = environment.get_filesystem(); - let skills_outcome = Arc::new( - services - .skills_manager - .skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) - .await, - ); + let skills_snapshot = services + .skills_service + .snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs))) + .await; let turn_context = Session::make_turn_context( thread_id, SessionId::from(thread_id), @@ -5069,7 +5074,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { resolved_turn_environments, session_configuration.cwd().clone(), "turn_id".to_string(), - skills_outcome, + skills_snapshot, ); let session = Session { @@ -5173,7 +5178,7 @@ async fn make_session_with_config_and_rx( let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -5191,7 +5196,7 @@ async fn make_session_with_config_and_rx( agent_status_tx, InitialHistory::New, SessionSource::Exec, - skills_manager, + skills_service, plugins_manager, mcp_manager, Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), @@ -5277,7 +5282,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -5295,7 +5300,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( agent_status_tx, initial_history, session_source, - skills_manager, + skills_service, plugins_manager, mcp_manager, Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), @@ -7006,7 +7011,7 @@ where ); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -7045,7 +7050,7 @@ where guardian_rejections: Mutex::new(std::collections::HashMap::new()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), @@ -7090,12 +7095,10 @@ where let skills_input = crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skill_fs = environment.get_filesystem(); - let skills_outcome = Arc::new( - services - .skills_manager - .skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) - .await, - ); + let skills_snapshot = services + .skills_service + .snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs))) + .await; let turn_context = Arc::new(Session::make_turn_context( thread_id, SessionId::from(thread_id), @@ -7114,7 +7117,7 @@ where resolved_turn_environments, session_configuration.cwd().clone(), "turn_id".to_string(), - skills_outcome, + skills_snapshot, )); let session = Arc::new(Session { @@ -7781,7 +7784,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() }, ]; turn_context.model_info.context_window = Some(100); - turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome))); let initial_context = session.build_initial_context(&turn_context).await; let developer_texts = developer_input_texts(&initial_context); @@ -7932,7 +7935,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil }, ]; turn_context.model_info.context_window = Some(100); - turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome))); let _ = session.build_initial_context(&turn_context).await; let warning_event = timeout(Duration::from_secs(1), rx.recv()) diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 43cd91a99..b9727a040 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -694,7 +694,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { config.model_provider.clone(), ); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); - let skills_manager = Arc::new(SkillsManager::new( + let skills_service = Arc::new(SkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -711,7 +711,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { auth_manager, models_manager, environment_manager: Arc::new(EnvironmentManager::default_for_tests()), - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions: codex_extension_api::empty_extension_registry(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 7da157cf7..4beef792b 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -528,7 +528,7 @@ async fn build_skills_and_plugins( } else { Vec::new() }; - let skills_outcome = turn_context.turn_skills.outcome.as_ref(); + let skills_outcome = turn_context.turn_skills.snapshot.outcome(); let connector_slug_counts = build_connector_slug_counts(&available_connectors); let extension_injection_items = build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token) diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index b76c9384f..cd01f92b9 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -1,9 +1,8 @@ use super::*; -use crate::SkillLoadOutcome; use crate::agents_md::LoadedAgentsMd; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::shell_snapshot::ShellSnapshotFile; -use codex_core_skills::HostLoadedSkills; +use codex_core_skills::HostSkillsSnapshot; use codex_file_system::FileSystemSandboxContext; use codex_model_provider::SharedModelProvider; use codex_model_provider::create_model_provider; @@ -26,14 +25,14 @@ use tracing::instrument; #[derive(Clone, Debug)] pub(crate) struct TurnSkillsContext { - pub(crate) outcome: Arc, + pub(crate) snapshot: HostSkillsSnapshot, pub(crate) implicit_invocation_seen_skills: Arc>>, } impl TurnSkillsContext { - pub(crate) fn new(outcome: Arc) -> Self { + pub(crate) fn new(snapshot: HostSkillsSnapshot) -> Self { Self { - outcome, + snapshot, implicit_invocation_seen_skills: Arc::new(Mutex::new(HashSet::new())), } } @@ -488,7 +487,7 @@ impl Session { environments: TurnEnvironmentSnapshot, cwd: AbsolutePathBuf, sub_id: String, - skills_outcome: Arc, + skills_snapshot: HostSkillsSnapshot, ) -> TurnContext { let reasoning_effort = session_configuration.collaboration_mode.reasoning_effort(); let reasoning_summary = session_configuration @@ -531,7 +530,7 @@ impl Session { )); let (current_date, timezone) = local_time_context(); let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone())); - extension_data.insert(HostLoadedSkills::new(Arc::clone(&skills_outcome))); + extension_data.insert(skills_snapshot.clone()); TurnContext { sub_id, trace_id: current_span_trace_id(), @@ -569,7 +568,7 @@ impl Session { dynamic_tools: session_configuration.dynamic_tools.clone(), turn_metadata_state, extension_data, - turn_skills: TurnSkillsContext::new(skills_outcome), + turn_skills: TurnSkillsContext::new(skills_snapshot), turn_timing_state: Arc::new(TurnTimingState::default()), terminal_error: Arc::new(Mutex::new(None)), server_model_warning_emitted: AtomicBool::new(false), @@ -726,12 +725,11 @@ impl Session { let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); let fs = primary_turn_environment .map(|turn_environment| turn_environment.environment.get_filesystem()); - let skills_outcome = Arc::new( - self.services - .skills_manager - .skills_for_config(&skills_input, fs) - .await, - ); + let skills_snapshot = self + .services + .skills_service + .snapshot_for_config(&skills_input, fs) + .await; let mut turn_context: TurnContext = Self::make_turn_context( self.thread_id(), self.session_id(), @@ -759,7 +757,7 @@ impl Session { turn_environments, cwd, sub_id, - skills_outcome, + skills_snapshot, ); turn_context.realtime_active = self.conversation.running_state().await.is_some(); diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index b11f79260..2c1ba8245 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -14,7 +14,7 @@ pub use codex_core_skills::SkillMetadata; pub use codex_core_skills::SkillPolicy; pub use codex_core_skills::SkillRenderReport; pub use codex_core_skills::SkillsLoadInput; -pub use codex_core_skills::SkillsManager; +pub use codex_core_skills::SkillsService; pub use codex_core_skills::build_available_skills; pub use codex_core_skills::build_skill_name_counts; pub use codex_core_skills::config_rules; @@ -26,11 +26,11 @@ pub use codex_core_skills::injection::SkillInjections; pub use codex_core_skills::injection::build_skill_injections; pub use codex_core_skills::injection::collect_explicit_skill_mentions; pub use codex_core_skills::loader; -pub use codex_core_skills::manager; pub use codex_core_skills::model; pub use codex_core_skills::remote; pub use codex_core_skills::render; pub use codex_core_skills::render::SkillRenderSideEffects; +pub use codex_core_skills::service; pub use codex_core_skills::system; pub(crate) fn skills_load_input_from_config( @@ -52,7 +52,7 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation( workdir: &AbsolutePathBuf, ) { let Some(candidate) = detect_implicit_skill_invocation_for_command( - turn_context.turn_skills.outcome.as_ref(), + turn_context.turn_skills.snapshot.outcome(), command, workdir, ) else { diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 00380c6d6..9724d8b3a 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::sync::Arc; -use crate::SkillsManager; +use crate::SkillsService; use crate::agent::AgentControl; use crate::attestation::AttestationProvider; use crate::client::ModelClient; @@ -61,7 +61,7 @@ pub(crate) struct SessionServices { pub(crate) guardian_rejections: Mutex>, pub(crate) guardian_rejection_circuit_breaker: Mutex, pub(crate) runtime_handle: Handle, - pub(crate) skills_manager: Arc, + pub(crate) skills_service: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, pub(crate) extensions: Arc>, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 2adb713a0..63335c012 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1,4 +1,4 @@ -use crate::SkillsManager; +use crate::SkillsService; use crate::agent::AgentControl; use crate::attestation::AttestationProvider; use crate::codex_thread::CodexThread; @@ -207,7 +207,7 @@ pub(crate) struct ThreadManagerState { auth_manager: Arc, models_manager: SharedModelsManager, environment_manager: Arc, - skills_manager: Arc, + skills_service: Arc, plugins_manager: Arc, mcp_manager: Arc, extensions: Arc>, @@ -281,7 +281,7 @@ impl ThreadManager { Arc::clone(&plugins_manager), Arc::clone(&extensions), )); - let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( + let skills_service = Arc::new(SkillsService::new_with_restriction_product( codex_home, config.bundled_skills_enabled(), restriction_product, @@ -292,7 +292,7 @@ impl ThreadManager { thread_created_tx, models_manager: build_models_manager(config, auth_manager.clone()), environment_manager, - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions, @@ -373,7 +373,7 @@ impl ThreadManager { auth_manager.get_api_auth_mode(), )); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( + let skills_service = Arc::new(SkillsService::new_with_restriction_product( skills_codex_home, /*bundled_skills_enabled*/ true, restriction_product, @@ -395,7 +395,7 @@ impl ThreadManager { models_manager: create_model_provider(provider, Some(auth_manager.clone())) .models_manager(codex_home, /*config_model_catalog*/ None), environment_manager, - skills_manager, + skills_service, plugins_manager, mcp_manager, extensions: empty_extension_registry(), @@ -424,8 +424,8 @@ impl ThreadManager { self.state.auth_manager.clone() } - pub fn skills_manager(&self) -> Arc { - self.state.skills_manager.clone() + pub fn skills_service(&self) -> Arc { + self.state.skills_service.clone() } pub fn plugins_manager(&self) -> Arc { @@ -1406,7 +1406,7 @@ impl ThreadManagerState { auth_manager, models_manager: Arc::clone(&self.models_manager), environment_manager: Arc::clone(&self.environment_manager), - skills_manager: Arc::clone(&self.skills_manager), + skills_service: Arc::clone(&self.skills_service), plugins_manager: Arc::clone(&self.plugins_manager), mcp_manager: Arc::clone(&self.mcp_manager), extensions: Arc::clone(&self.extensions), diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index bf4d86d36..c82ce6642 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use codex_core_skills::HostLoadedSkills; +use codex_core_skills::HostSkillsSnapshot; use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_extension_api::ConfigContributor; @@ -121,7 +121,7 @@ where SkillListQuery { turn_id: thread_store.level_id().to_string(), executor_roots: thread_state.selected_roots().to_vec(), - host: None, + host_snapshot: None, include_host_skills: false, include_bundled_skills: config.bundled_skills_enabled, include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), @@ -184,11 +184,11 @@ where }; let config = thread_state.config(); - let host_loaded_skills = turn_store.get::(); + let host_snapshot = turn_store.get::(); let query = SkillListQuery { turn_id: input.turn_id.clone(), executor_roots: thread_state.selected_roots().to_vec(), - host: host_loaded_skills.clone(), + host_snapshot: host_snapshot.clone(), include_host_skills: true, include_bundled_skills: config.bundled_skills_enabled, include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), @@ -217,12 +217,7 @@ where let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default(); for entry in &selected_entries { match self - .read_main_prompt( - entry, - host_loaded_skills.clone(), - session_store, - &thread_state, - ) + .read_main_prompt(entry, host_snapshot.clone(), session_store, &thread_state) .await { Ok(read_result) => { @@ -259,12 +254,12 @@ where } } - if let Some(host_loaded_skills) = &host_loaded_skills { + if let Some(host_snapshot) = &host_snapshot { for entry in selected_entries .iter() .filter(|entry| entry.authority.kind != SkillSourceKind::Host) { - for host_skill in host_loaded_skills + for host_skill in host_snapshot .outcome() .skills .iter() @@ -319,7 +314,7 @@ impl SkillsExtension { async fn read_main_prompt( &self, entry: &SkillCatalogEntry, - host_loaded_skills: Option>, + host_snapshot: Option>, session_store: &ExtensionData, thread_state: &SkillsThreadState, ) -> Result { @@ -330,7 +325,7 @@ impl SkillsExtension { authority: entry.authority.clone(), package: entry.id.clone(), resource: entry.main_prompt.clone(), - host: host_loaded_skills, + host_snapshot, mcp_resources: session_store.get::(), }, ) diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs index 329c0c581..405592e93 100644 --- a/codex-rs/ext/skills/src/provider.rs +++ b/codex-rs/ext/skills/src/provider.rs @@ -6,7 +6,7 @@ mod executor; mod host; mod orchestrator; -use codex_core_skills::HostLoadedSkills; +use codex_core_skills::HostSkillsSnapshot; use codex_mcp::McpResourceClient; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -26,7 +26,7 @@ pub use orchestrator::OrchestratorSkillProvider; pub struct SkillListQuery { pub turn_id: String, pub executor_roots: Vec, - pub host: Option>, + pub host_snapshot: Option>, pub include_host_skills: bool, pub include_bundled_skills: bool, pub include_orchestrator_skills: bool, @@ -38,7 +38,7 @@ pub struct SkillReadRequest { pub authority: SkillAuthority, pub package: SkillPackageId, pub resource: SkillResourceId, - pub host: Option>, + pub host_snapshot: Option>, pub mcp_resources: Option>, } diff --git a/codex-rs/ext/skills/src/provider/host.rs b/codex-rs/ext/skills/src/provider/host.rs index 63d0d4588..2cee69421 100644 --- a/codex-rs/ext/skills/src/provider/host.rs +++ b/codex-rs/ext/skills/src/provider/host.rs @@ -18,12 +18,10 @@ use crate::provider::SkillSearchRequest; const HOST_AUTHORITY_ID: &str = "host"; -/// Host-owned skill provider backed by the already-loaded turn skills. +/// Host-owned skill provider backed by an immutable service snapshot. /// -/// The provider intentionally does not reload or cache host skills. Core owns -/// skill loading, including plugin roots, runtime extra roots, and the primary -/// environment filesystem. This adapter only maps that loaded outcome into the -/// skills-extension catalog/read contract. +/// Discovery and caching belong to `SkillsService`; this provider only maps a +/// snapshot into the authority-aware catalog/read contract. #[derive(Clone, Default)] pub struct HostSkillProvider; @@ -36,24 +34,24 @@ impl HostSkillProvider { impl SkillProvider for HostSkillProvider { fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { Box::pin(async move { - let Some(host_loaded_skills) = query.host else { + let Some(host_snapshot) = query.host_snapshot else { return Err(SkillProviderError::new( - "host skill provider requires loaded host skills", + "host skill provider requires a host skills snapshot", )); }; - Ok(catalog_from_outcome(host_loaded_skills.outcome())) + Ok(catalog_from_outcome(host_snapshot.outcome())) }) } fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { Box::pin(async move { - let Some(host_loaded_skills) = request.host else { + let Some(host_snapshot) = request.host_snapshot else { return Err(SkillProviderError::new( - "host skill provider requires loaded host skills", + "host skill provider requires a host skills snapshot", )); }; - let Some(skill) = host_loaded_skills.outcome().skills.iter().find(|skill| { + let Some(skill) = host_snapshot.outcome().skills.iter().find(|skill| { let skill_path = skill.path_to_skills_md.to_string_lossy(); skill_path == request.resource.as_str() || skill_path.replace('\\', "/") == request.resource.as_str() @@ -64,15 +62,12 @@ impl SkillProvider for HostSkillProvider { ))); }; - let contents = host_loaded_skills - .read_skill_text(skill) - .await - .map_err(|err| { - SkillProviderError::new(format!( - "failed to read host skill resource {}: {err}", - request.resource.as_str() - )) - })?; + let contents = host_snapshot.read_skill_text(skill).await.map_err(|err| { + SkillProviderError::new(format!( + "failed to read host skill resource {}: {err}", + request.resource.as_str() + )) + })?; Ok(SkillReadResult { resource: request.resource, diff --git a/codex-rs/ext/skills/src/tools/mod.rs b/codex-rs/ext/skills/src/tools/mod.rs index a1898b1a2..0d05064af 100644 --- a/codex-rs/ext/skills/src/tools/mod.rs +++ b/codex-rs/ext/skills/src/tools/mod.rs @@ -68,7 +68,7 @@ impl SkillToolContext { self.providers.list_orchestrator_for_turn(SkillListQuery { turn_id: turn_id.to_string(), executor_roots: Vec::new(), - host: None, + host_snapshot: None, include_host_skills: false, include_bundled_skills: false, include_orchestrator_skills: true, diff --git a/codex-rs/ext/skills/src/tools/read.rs b/codex-rs/ext/skills/src/tools/read.rs index 358842aa1..64587cab9 100644 --- a/codex-rs/ext/skills/src/tools/read.rs +++ b/codex-rs/ext/skills/src/tools/read.rs @@ -82,7 +82,7 @@ impl ToolExecutor for ReadTool { authority, package: SkillPackageId(args.package), resource: requested_resource.clone(), - host: None, + host_snapshot: None, mcp_resources: self.context.mcp_resources.clone(), }, ) diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index f3d17e352..1eff53c3b 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -use codex_core_skills::HostLoadedSkills; +use codex_core_skills::HostSkillsSnapshot; use codex_core_skills::loader::SkillRoot; use codex_core_skills::loader::load_skills_from_roots; use codex_exec_server::CopyOptions; @@ -208,7 +208,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { skill.path_to_skills_md, canonical_root.join("skill/SKILL.md") ); - let loaded = HostLoadedSkills::new(Arc::new(outcome)); + let loaded = HostSkillsSnapshot::new(Arc::new(outcome)); assert_eq!( loaded.read_skill_text(&skill).await.expect("skill body"), SKILL_CONTENTS @@ -242,7 +242,7 @@ async fn selected_root_id_distinguishes_identical_executor_paths() { }, }) .collect(), - host: None, + host_snapshot: None, include_host_skills: false, include_bundled_skills: true, include_orchestrator_skills: false, diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index bcf058d6b..cd6f46d9c 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -4,7 +4,7 @@ use std::sync::Mutex; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; -use codex_core_skills::HostLoadedSkills; +use codex_core_skills::HostSkillsSnapshot; use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; @@ -52,7 +52,7 @@ const DEMO_SKILL_CONTENTS: &str = "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n"; #[tokio::test] -async fn installed_extension_uses_host_loaded_skills() -> TestResult { +async fn installed_extension_uses_host_service_snapshot() -> TestResult { let codex_home = test_codex_home(); let skill_path = codex_home.join("skills").join("demo").join("SKILL.md"); std::fs::create_dir_all( @@ -97,7 +97,7 @@ async fn installed_extension_uses_host_loaded_skills() -> TestResult { let loaded_skills = Arc::new(outcome); let skill_prompt_path = skill_path_string.replace('\\', "/"); let turn_store = ExtensionData::new("turn-1"); - turn_store.insert(HostLoadedSkills::new(Arc::clone(&loaded_skills))); + turn_store.insert(HostSkillsSnapshot::new(Arc::clone(&loaded_skills))); let fragments = registry.turn_input_contributors()[0] .contribute(