From 02bc1dd796e367619b44fe62825d9f118470ad6f Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Tue, 4 Aug 2026 16:23:04 +0000 Subject: [PATCH] Move the host skills service into the skills extension (#36921) ## What changed - Move host skill discovery, snapshot caching, and configuration handling from `codex-core-skills` to `codex-skills-extension`. - Rename `SkillsService` and `SkillsLoadInput` to `HostSkillsService` and `HostSkillsLoadInput` to distinguish the host implementation from other skill providers. - Keep shared loading primitives and skill outcome modeling in `codex-core-skills`. GitOrigin-RevId: d81a21791d2ef8c066e157e7e538b8cb7ee4c24b --- codex-rs/Cargo.lock | 4 + .../request_processors/catalog_processor.rs | 2 +- codex-rs/app-server/src/skills_watcher.rs | 10 +-- codex-rs/core-api/src/lib.rs | 2 +- codex-rs/core-plugins/Cargo.toml | 1 + codex-rs/core-plugins/src/manager_tests.rs | 8 +- codex-rs/core-skills/src/lib.rs | 3 - codex-rs/core-skills/src/loader.rs | 2 +- codex-rs/core-skills/src/model.rs | 14 ++++ codex-rs/core-skills/src/system.rs | 7 -- codex-rs/core/Cargo.toml | 2 +- codex-rs/core/src/agent/role_tests.rs | 4 +- codex-rs/core/src/config/mod.rs | 2 +- codex-rs/core/src/lib.rs | 2 +- codex-rs/core/src/session/mod.rs | 4 +- codex-rs/core/src/session/session.rs | 4 +- codex-rs/core/src/session/tests.rs | 10 +-- .../core/src/session/tests/guardian_tests.rs | 2 +- codex-rs/core/src/skills.rs | 10 +-- codex-rs/core/src/state/service.rs | 4 +- codex-rs/core/src/thread_manager.rs | 10 +-- codex-rs/ext/skills/Cargo.toml | 3 + .../skills/src/host_service.rs} | 74 +++++++------------ .../skills/src/host_service_tests.rs} | 36 ++++----- codex-rs/ext/skills/src/lib.rs | 4 + codex-rs/ext/skills/src/provider/host.rs | 2 +- 26 files changed, 111 insertions(+), 115 deletions(-) rename codex-rs/{core-skills/src/service.rs => ext/skills/src/host_service.rs} (85%) rename codex-rs/{core-skills/src/service_tests.rs => ext/skills/src/host_service_tests.rs} (96%) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 07961e4361..f5ec6bc45b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2873,6 +2873,7 @@ dependencies = [ "codex-protocol", "codex-shell-command", "codex-skills", + "codex-skills-extension", "codex-tools", "codex-utils-absolute-path", "codex-utils-path", @@ -4099,6 +4100,7 @@ dependencies = [ name = "codex-skills-extension" version = "0.0.0" dependencies = [ + "codex-config", "codex-core-skills", "codex-exec-server", "codex-extension-api", @@ -4113,6 +4115,7 @@ dependencies = [ "codex-utils-path-uri", "codex-utils-plugins", "codex-utils-string", + "dunce", "futures", "insta", "opentelemetry_sdk", @@ -4123,6 +4126,7 @@ dependencies = [ "serde_yaml", "tempfile", "tokio", + "toml 0.9.11+spec-1.1.0", "tracing", "url", ] 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 add2648806..4070254f1a 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -546,7 +546,7 @@ impl CatalogRequestProcessor { } else { Vec::new() }; - let skills_input = codex_core::skills::SkillsLoadInput::new( + let skills_input = codex_core::skills::HostSkillsLoadInput::new( cwd_abs.clone(), effective_skill_roots, config_layer_stack, diff --git a/codex-rs/app-server/src/skills_watcher.rs b/codex-rs/app-server/src/skills_watcher.rs index 87661b904b..49a83297ab 100644 --- a/codex-rs/app-server/src/skills_watcher.rs +++ b/codex-rs/app-server/src/skills_watcher.rs @@ -7,8 +7,8 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::SkillsChangedNotification; use codex_core::ThreadManager; use codex_core::config::Config; -use codex_core::skills::SkillsLoadInput; -use codex_core::skills::SkillsService; +use codex_core::skills::HostSkillsLoadInput; +use codex_core::skills::HostSkillsService; use codex_file_watcher::FileWatcher; use codex_file_watcher::FileWatcherSubscriber; use codex_file_watcher::Receiver; @@ -37,7 +37,7 @@ pub(crate) struct SkillsWatcher { impl SkillsWatcher { pub(crate) fn new( - skills_service: Arc, + skills_service: Arc, codex_home: &AbsolutePathBuf, outgoing: Arc, ) -> Arc { @@ -113,7 +113,7 @@ impl SkillsWatcher { let plugins_input = config.plugins_config_input(); let plugins_manager = thread_manager.plugins_manager(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( config.cwd.clone(), plugin_outcome.effective_plugin_skill_roots(), config.config_layer_stack.clone(), @@ -137,7 +137,7 @@ impl SkillsWatcher { fn spawn_event_loop( rx: Receiver, - skills_service: Arc, + skills_service: Arc, system_skills_root: AbsolutePathBuf, outgoing: Arc, shutdown_token: CancellationToken, diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index c2bd8c93cd..bc22a12c40 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -50,7 +50,7 @@ pub use codex_core::config::find_codex_home; pub use codex_core::init_state_db; pub use codex_core::local_agent_graph_store_from_state_db; pub use codex_core::resolve_installation_id; -pub use codex_core::skills::SkillsService; +pub use codex_core::skills::HostSkillsService; pub use codex_core::thread_store_from_config; pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::EnvironmentRegistryConnectRequest; diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 3c3526f061..c4f52ab010 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -60,6 +60,7 @@ which = { workspace = true } [dev-dependencies] codex-exec-server-test-support = { workspace = true } +codex-skills-extension = { workspace = true } libc = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index c2a90decbd..b48c30bfd7 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -41,8 +41,6 @@ use codex_config::RequirementsLayerEntry; use codex_config::compose_requirements; use codex_config::types::McpServerTransportConfig; use codex_core_skills::PluginSkillSnapshots; -use codex_core_skills::SkillsLoadInput; -use codex_core_skills::SkillsService; use codex_login::CodexAuth; use codex_plugin::AppDeclaration; use codex_plugin::PluginId; @@ -50,6 +48,8 @@ use codex_protocol::auth::AuthMode; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; use codex_skills::SkillConfigRules; +use codex_skills_extension::HostSkillsLoadInput; +use codex_skills_extension::HostSkillsService; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use pretty_assertions::assert_eq; @@ -2917,7 +2917,7 @@ enabled = true ); write_file(&skill_path, "---\nname: search\ndescription: second\n---\n"); - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( codex_home_abs.clone(), plugin_outcome.effective_plugin_skill_roots(), config.config_layer_stack.clone(), @@ -2925,7 +2925,7 @@ enabled = true ) .with_plugin_skill_snapshots(manager.plugin_skill_snapshots_for_config(&config)); let skills_service = - SkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false); + HostSkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false); let snapshot = skills_service .snapshot_for_config(&skills_input, /*fs*/ None) .await; diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index ce7211b9fb..8b70ad57ae 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -6,7 +6,6 @@ mod mention_counts; pub mod model; pub mod remote; mod root_loader; -pub mod service; mod skill_instructions; pub mod system; @@ -20,6 +19,4 @@ pub use model::SkillMetadata; pub use model::SkillPolicy; pub use model::filter_skill_load_outcome_for_product; pub use root_loader::PluginSkillSnapshots; -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 ac5d090e67..f8e866a23a 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -197,7 +197,7 @@ pub(crate) async fn load_skill_root(root: SkillRoot) -> SkillRootSnapshot { } } -pub(crate) async fn skill_roots( +pub async fn skill_roots( fs: Option>, config_layer_stack: &ConfigLayerStack, cwd: &AbsolutePathBuf, diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index c9d04dd9e5..f01d972788 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -56,6 +56,20 @@ impl SkillLoadOutcome { .map(|skill| (skill, self.is_skill_enabled(skill))) } + pub fn with_disabled_paths(mut self, disabled_paths: HashSet) -> Self { + self.disabled_paths = disabled_paths; + let (by_scripts_dir, by_doc_path) = crate::build_implicit_skill_path_indexes( + self.skills + .iter() + .filter(|skill| self.is_skill_enabled(skill)) + .cloned() + .collect(), + ); + self.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); + self.implicit_skills_by_doc_path = Arc::new(by_doc_path); + self + } + /// Returns the discovery root that supplied a loaded skill path. pub fn skill_root_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> { self.skill_root_by_path.get(path) diff --git a/codex-rs/core-skills/src/system.rs b/codex-rs/core-skills/src/system.rs index 5eec94c729..3031246190 100644 --- a/codex-rs/core-skills/src/system.rs +++ b/codex-rs/core-skills/src/system.rs @@ -1,8 +1 @@ -pub(crate) use codex_skills::install_system_skills; pub(crate) use codex_skills::system_cache_root_dir; - -use codex_utils_absolute_path::AbsolutePathBuf; - -pub(crate) fn uninstall_system_skills(codex_home: &AbsolutePathBuf) { - let _ = std::fs::remove_dir_all(system_cache_root_dir(codex_home)); -} diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 3c8ad1b9fb..0fc5d1a865 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -64,6 +64,7 @@ codex-rollout-trace = { workspace = true } codex-rmcp-client = { workspace = true } codex-sandboxing = { workspace = true } codex-skills = { workspace = true } +codex-skills-extension = { workspace = true } codex-state = { workspace = true } codex-terminal-detection = { workspace = true } codex-thread-store = { workspace = true } @@ -141,7 +142,6 @@ codex-exec-server-test-support = { workspace = true } codex-image-generation-extension = { workspace = true } codex-home = { workspace = true } codex-otel = { workspace = true } -codex-skills-extension = { workspace = true } codex-test-binary-support = { workspace = true } codex-utils-cargo-bin = { workspace = true } codex-web-search-extension = { workspace = true } diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 72e4486c40..ad64052fb2 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::SkillsService; +use crate::HostSkillsService; use crate::config::ConfigBuilder; use crate::skills_load_input_from_config; use codex_core_plugins::PluginsManager; @@ -398,7 +398,7 @@ enabled = false let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf())); let skills_service = - SkillsService::new(home.path().abs(), /*bundled_skills_enabled*/ true); + HostSkillsService::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(); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 0c7ef1d40b..a46090ef30 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -4355,7 +4355,7 @@ impl Config { } pub fn bundled_skills_enabled(&self) -> bool { - crate::skills::service::bundled_skills_enabled_from_stack(&self.config_layer_stack) + crate::skills::bundled_skills_enabled_from_stack(&self.config_layer_stack) } /// Returns whether effective requirements allow selecting a concrete profile. diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 862d98e9ca..612879cedc 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -87,9 +87,9 @@ pub mod sandboxing; mod session_prefix; mod session_startup_prewarm; pub mod skills; +pub(crate) use skills::HostSkillsService; pub(crate) use skills::SkillInjections; pub(crate) use skills::SkillMetadata; -pub(crate) use skills::SkillsService; pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_explicit_skill_mentions; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 13bd3a0208..f6af2f206a 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -297,7 +297,7 @@ pub(crate) struct PreviousTurnSettings { pub(crate) realtime_active: Option, } -use crate::SkillsService; +use crate::HostSkillsService; use crate::exec_policy::ExecPolicyUpdateError; use crate::guardian::GuardianReviewOptions; use crate::guardian::GuardianReviewSessionManager; @@ -422,7 +422,7 @@ pub(crate) struct SessionSpawnArgs { pub(crate) auth_manager: Arc, pub(crate) models_manager: SharedModelsManager, pub(crate) environment_manager: Arc, - pub(crate) skills_service: Arc, + pub(crate) skills_service: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, pub(crate) code_mode_session_provider: Arc, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index bb1154713b..7445888964 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -461,7 +461,7 @@ pub(crate) struct AppServerClientMetadata { async fn warm_plugins_and_skills_for_session_init( config: Arc, plugins_manager: Arc, - skills_service: Arc, + skills_service: Arc, turn_environments: &TurnEnvironmentSnapshot, ) -> Vec { let fs = turn_environments.primary_filesystem(); @@ -510,7 +510,7 @@ impl Session { mut initial_history: InitialHistory, fork_persistence: ForkPersistence, session_source: SessionSource, - skills_service: Arc, + skills_service: Arc, plugins_manager: Arc, mcp_manager: Arc, code_mode_session_provider: Arc, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6180b49d14..486ccbac91 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5522,7 +5522,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -5689,7 +5689,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -5931,7 +5931,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -6041,7 +6041,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); @@ -7887,7 +7887,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 8e9ab645a2..132260023f 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -727,7 +727,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_service = Arc::new(SkillsService::new( + let skills_service = Arc::new(HostSkillsService::new( config.codex_home.clone(), /*bundled_skills_enabled*/ true, )); diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index f3a9cfd710..fb53a1ea8b 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -15,8 +15,6 @@ use tokio::sync::Mutex; pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; -pub use codex_core_skills::SkillsLoadInput; -pub use codex_core_skills::SkillsService; pub use codex_core_skills::build_skill_name_counts; pub use codex_core_skills::config_rules; pub use codex_core_skills::detect_implicit_skill_invocation_for_command; @@ -28,10 +26,12 @@ pub use codex_core_skills::injection::collect_explicit_skill_mentions; pub use codex_core_skills::loader; pub use codex_core_skills::model; pub use codex_core_skills::remote; -pub use codex_core_skills::service; pub use codex_core_skills::system; pub use codex_skills::SkillMetadata; pub use codex_skills::SkillPolicy; +pub use codex_skills_extension::HostSkillsLoadInput; +pub use codex_skills_extension::HostSkillsService; +pub use codex_skills_extension::bundled_skills_enabled_from_stack; #[derive(Debug, Default)] struct ImplicitSkillInvocations(Mutex>); @@ -39,8 +39,8 @@ struct ImplicitSkillInvocations(Mutex>); pub(crate) fn skills_load_input_from_config( config: &Config, effective_skill_roots: Vec, -) -> SkillsLoadInput { - SkillsLoadInput::new( +) -> HostSkillsLoadInput { + HostSkillsLoadInput::new( config.cwd.clone(), effective_skill_roots, config.config_layer_stack.clone(), diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index f62b77b32d..dd17a076c1 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::sync::Arc; -use crate::SkillsService; +use crate::HostSkillsService; use crate::agent::AgentControl; use crate::agents_md_manager::AgentsMdManager; use crate::attestation::AttestationProvider; @@ -65,7 +65,7 @@ pub(crate) struct SessionServices { pub(crate) tool_approvals: Mutex, pub(crate) guardian_rejection_circuit_breaker: Mutex, pub(crate) runtime_handle: Handle, - pub(crate) skills_service: Arc, + pub(crate) skills_service: Arc, pub(crate) agents_md_manager: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index dffdb605bd..95795c28d6 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1,5 +1,5 @@ use crate::CodexAppsToolsCache; -use crate::SkillsService; +use crate::HostSkillsService; use crate::agent::AgentControl; use crate::attestation::AttestationProvider; use crate::codex_thread::CodexThread; @@ -315,7 +315,7 @@ pub(crate) struct ThreadManagerState { models_manager: SharedModelsManager, environment_manager: Arc, starting_mcp_runtimes: std::sync::Mutex>>, - skills_service: Arc, + skills_service: Arc, plugins_manager: Arc, mcp_manager: Arc, code_mode_session_provider: Arc, @@ -404,7 +404,7 @@ impl ThreadManager { Arc::clone(&extensions), codex_apps_tools_cache, )); - let skills_service = Arc::new(SkillsService::new_with_restriction_product( + let skills_service = Arc::new(HostSkillsService::new_with_restriction_product( codex_home, config.bundled_skills_enabled(), restriction_product, @@ -533,7 +533,7 @@ 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( + let skills_service = Arc::new(HostSkillsService::new_with_restriction_product( absolute_codex_home.clone(), /*bundled_skills_enabled*/ true, restriction_product, @@ -588,7 +588,7 @@ impl ThreadManager { self.state.auth_manager.clone() } - pub fn skills_service(&self) -> Arc { + pub fn skills_service(&self) -> Arc { self.state.skills_service.clone() } diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index f074d45c88..a79e1188fc 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -13,6 +13,7 @@ doctest = false workspace = true [dependencies] +codex-config = { workspace = true } codex-core-skills = { workspace = true } codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } @@ -37,8 +38,10 @@ url = { workspace = true } [dev-dependencies] codex-models-manager = { workspace = true } codex-utils-cargo-bin = { workspace = true } +dunce = { workspace = true } insta = { workspace = true } opentelemetry_sdk = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +toml = { workspace = true } diff --git a/codex-rs/core-skills/src/service.rs b/codex-rs/ext/skills/src/host_service.rs similarity index 85% rename from codex-rs/core-skills/src/service.rs rename to codex-rs/ext/skills/src/host_service.rs index 09f87fd9f8..6edb174d2a 100644 --- a/codex-rs/core-skills/src/service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::collections::HashSet; use std::sync::Arc; use std::sync::RwLock; @@ -15,23 +14,22 @@ use tracing::info; use tracing::instrument; use tracing::warn; -use crate::HostSkillsSnapshot; -use crate::PluginSkillSnapshots; -use crate::SkillLoadOutcome; -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::MAX_CONCURRENT_ROOT_SCANS; -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; use codex_config::SkillsConfig; +use codex_core_skills::HostSkillsSnapshot; +use codex_core_skills::PluginSkillSnapshots; +use codex_core_skills::SkillLoadOutcome; +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::MAX_CONCURRENT_ROOT_SCANS; +use codex_core_skills::loader::SkillRoot; +use codex_core_skills::loader::load_skills_from_roots; +use codex_core_skills::loader::skill_roots; +use codex_skills::install_system_skills; +use codex_skills::system_cache_root_dir; #[derive(Debug, Clone)] -pub struct SkillsLoadInput { +pub struct HostSkillsLoadInput { pub cwd: AbsolutePathBuf, pub effective_skill_roots: Vec, pub config_layer_stack: ConfigLayerStack, @@ -39,7 +37,7 @@ pub struct SkillsLoadInput { plugin_skill_snapshots: Option, } -impl SkillsLoadInput { +impl HostSkillsLoadInput { pub fn new( cwd: AbsolutePathBuf, effective_skill_roots: Vec, @@ -68,7 +66,7 @@ impl SkillsLoadInput { /// 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 { +pub struct HostSkillsService { codex_home: AbsolutePathBuf, restriction_product: Option, extra_roots: RwLock>, @@ -78,7 +76,7 @@ pub struct SkillsService { root_scan_slots: Arc, } -impl SkillsService { +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)) } @@ -99,7 +97,7 @@ impl SkillsService { 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(&service.codex_home); + let _ = std::fs::remove_dir_all(system_cache_root_dir(&service.codex_home)); } else if let Err(err) = install_system_skills(&service.codex_home) { tracing::error!("failed to install system skills: {err}"); } @@ -131,7 +129,7 @@ impl SkillsService { )] pub async fn snapshot_for_config( &self, - input: &SkillsLoadInput, + input: &HostSkillsLoadInput, fs: Option>, ) -> HostSkillsSnapshot { let roots = self.skill_roots_for_config(input, fs).await; @@ -155,7 +153,7 @@ impl SkillsService { pub async fn skill_roots_for_config( &self, - input: &SkillsLoadInput, + input: &HostSkillsLoadInput, fs: Option>, ) -> Vec { let mut roots = skill_roots( @@ -174,7 +172,7 @@ impl SkillsService { pub async fn snapshot_for_cwd( &self, - input: &SkillsLoadInput, + input: &HostSkillsLoadInput, force_reload: bool, fs: Option>, ) -> HostSkillsSnapshot { @@ -215,7 +213,7 @@ impl SkillsService { #[instrument(level = "trace", skip_all)] async fn build_skill_outcome( &self, - input: &SkillsLoadInput, + input: &HostSkillsLoadInput, roots: Vec, skill_config_rules: &SkillConfigRules, ) -> SkillLoadOutcome { @@ -225,10 +223,12 @@ impl SkillsService { Arc::clone(&self.root_scan_slots), ) .await; - let outcome = - crate::filter_skill_load_outcome_for_product(outcome, self.restriction_product); + let outcome = codex_core_skills::filter_skill_load_outcome_for_product( + outcome, + self.restriction_product, + ); let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules); - finalize_skill_outcome(outcome, disabled_paths) + outcome.with_disabled_paths(disabled_paths) } pub fn clear_cache(&self) { @@ -341,26 +341,6 @@ fn config_skills_cache_key( } } -fn finalize_skill_outcome( - mut outcome: SkillLoadOutcome, - disabled_paths: HashSet, -) -> SkillLoadOutcome { - outcome.disabled_paths = disabled_paths; - // Usage-event detection should see any enabled skill file/script read, even when the - // skill is not model-routable through implicit invocation. - let (by_scripts_dir, by_doc_path) = build_implicit_skill_path_indexes( - outcome - .skills - .iter() - .filter(|skill| outcome.is_skill_enabled(skill)) - .cloned() - .collect(), - ); - outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); - outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path); - outcome -} - #[cfg(test)] -#[path = "service_tests.rs"] +#[path = "host_service_tests.rs"] mod tests; diff --git a/codex-rs/core-skills/src/service_tests.rs b/codex-rs/ext/skills/src/host_service_tests.rs similarity index 96% rename from codex-rs/core-skills/src/service_tests.rs rename to codex-rs/ext/skills/src/host_service_tests.rs index 3f89ce517a..0b355d2c1f 100644 --- a/codex-rs/core-skills/src/service_tests.rs +++ b/codex-rs/ext/skills/src/host_service_tests.rs @@ -138,12 +138,12 @@ enabled = {enabled} } async fn skills_for_config_with_stack( - skills_service: &SkillsService, + skills_service: &HostSkillsService, cwd: &TempDir, config_layer_stack: &ConfigLayerStack, effective_skill_roots: &[PluginSkillRoot], ) -> SkillLoadOutcome { - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( cwd.path().abs(), effective_skill_roots.to_vec(), config_layer_stack.clone(), @@ -164,7 +164,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_service = SkillsService::new( + let _skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ false, ); @@ -180,7 +180,7 @@ 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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -217,7 +217,7 @@ async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() { let config_layer_stack = config_stack(&codex_home, ""); let mut plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "sample@test", "sample"); - let skills_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -255,12 +255,12 @@ 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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( cwd.path().abs(), Vec::new(), config_layer_stack.clone(), @@ -330,7 +330,7 @@ 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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -392,7 +392,7 @@ async fn skills_for_config_disables_plugin_skills_by_name() { ); let plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test", "sample"); - let skills_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -453,13 +453,13 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() { ConfigRequirementsToml::default(), ) .expect("valid config layer stack"); - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( cwd.path().abs(), Vec::new(), config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let skills_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -517,13 +517,13 @@ async fn skills_for_cwd_without_fs_skips_repo_roots() { ConfigRequirementsToml::default(), ) .expect("valid config layer stack"); - let skills_input = SkillsLoadInput::new( + let skills_input = HostSkillsLoadInput::new( cwd.path().abs(), Vec::new(), config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ); - let skills_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); @@ -559,7 +559,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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ false, ); @@ -594,12 +594,12 @@ 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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let _ = skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await; - let base_input = SkillsLoadInput::new( + let base_input = HostSkillsLoadInput::new( cwd.path().abs(), Vec::new(), config_layer_stack.clone(), @@ -671,11 +671,11 @@ 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_service = SkillsService::new( + let skills_service = HostSkillsService::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let parent_input = SkillsLoadInput::new( + let parent_input = HostSkillsLoadInput::new( cwd.path().abs(), Vec::new(), parent_stack.clone(), diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 39b3611ff5..2901d2b1a7 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -4,6 +4,7 @@ mod config; mod dynamic_skill_selector; mod extension; mod fragments; +mod host_service; // Host loading is staged before its crate-internal runtime caller in this PR stack. #[allow(dead_code)] mod loader; @@ -22,6 +23,9 @@ pub use config::SkillsExtensionConfig; pub use extension::install; pub use extension::install_with_providers; pub use extension::install_with_providers_and_metrics; +pub use host_service::HostSkillsLoadInput; +pub use host_service::HostSkillsService; +pub use host_service::bundled_skills_enabled_from_stack; pub use provider::ExecutorSkillProvider; pub use provider::HostSkillProvider; pub use provider::OrchestratorSkillProvider; diff --git a/codex-rs/ext/skills/src/provider/host.rs b/codex-rs/ext/skills/src/provider/host.rs index 53ccb885aa..7e391e090b 100644 --- a/codex-rs/ext/skills/src/provider/host.rs +++ b/codex-rs/ext/skills/src/provider/host.rs @@ -22,7 +22,7 @@ const HOST_AUTHORITY_ID: &str = "host"; /// Host-owned skill provider backed by an immutable service snapshot. /// -/// Discovery and caching belong to `SkillsService`; this provider only maps a +/// Discovery and caching belong to `HostSkillsService`; this provider only maps a /// snapshot into the authority-aware catalog/read contract. #[derive(Clone, Default)] pub struct HostSkillProvider;