mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
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
This commit is contained in:
1 parent
d75f94a94d
commit
02bc1dd796
26 files changed
+111
-115
No files matched your search
@@ -0,0 +1,346 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use tokio::sync::Semaphore;
|
||||
use tracing::info;
|
||||
use tracing::instrument;
|
||||
use tracing::warn;
|
||||
|
||||
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 HostSkillsLoadInput {
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
pub config_layer_stack: ConfigLayerStack,
|
||||
pub bundled_skills_enabled: bool,
|
||||
plugin_skill_snapshots: Option<PluginSkillSnapshots>,
|
||||
}
|
||||
|
||||
impl HostSkillsLoadInput {
|
||||
pub fn new(
|
||||
cwd: AbsolutePathBuf,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
bundled_skills_enabled: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
cwd,
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
bundled_skills_enabled,
|
||||
plugin_skill_snapshots: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Attaches plugin skill snapshots parsed during plugin loading, when available.
|
||||
pub fn with_plugin_skill_snapshots(
|
||||
mut self,
|
||||
plugin_skill_snapshots: Option<PluginSkillSnapshots>,
|
||||
) -> Self {
|
||||
self.plugin_skill_snapshots = plugin_skill_snapshots;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
/// Owns host skill discovery, immutable snapshots, cache invalidation, and extra roots.
|
||||
///
|
||||
/// Source-specific model exposure remains the responsibility of the skills extension.
|
||||
pub struct HostSkillsService {
|
||||
codex_home: AbsolutePathBuf,
|
||||
restriction_product: Option<Product>,
|
||||
extra_roots: RwLock<Vec<AbsolutePathBuf>>,
|
||||
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, HostSkillsSnapshot>>,
|
||||
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, HostSkillsSnapshot>>,
|
||||
// Shared across cwds so root scheduling cannot multiply per-root I/O fanout.
|
||||
root_scan_slots: Arc<Semaphore>,
|
||||
}
|
||||
|
||||
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))
|
||||
}
|
||||
|
||||
pub fn new_with_restriction_product(
|
||||
codex_home: AbsolutePathBuf,
|
||||
bundled_skills_enabled: bool,
|
||||
restriction_product: Option<Product>,
|
||||
) -> Self {
|
||||
let service = Self {
|
||||
codex_home,
|
||||
restriction_product,
|
||||
extra_roots: RwLock::new(Vec::new()),
|
||||
cache_by_cwd: RwLock::new(HashMap::new()),
|
||||
cache_by_config: RwLock::new(HashMap::new()),
|
||||
root_scan_slots: Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
};
|
||||
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.
|
||||
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}");
|
||||
}
|
||||
service
|
||||
}
|
||||
|
||||
pub fn set_extra_roots(&self, extra_roots: Vec<AbsolutePathBuf>) {
|
||||
{
|
||||
let mut roots = self
|
||||
.extra_roots
|
||||
.write()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
*roots = extra_roots;
|
||||
}
|
||||
self.clear_cache();
|
||||
}
|
||||
|
||||
/// Load skills for an already-constructed [`Config`], avoiding any additional config-layer
|
||||
/// loading.
|
||||
///
|
||||
/// This path uses a cache keyed by the effective skill-relevant config state rather than just
|
||||
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
|
||||
/// to share a directory.
|
||||
#[instrument(
|
||||
name = "skills_for_config",
|
||||
level = "info",
|
||||
skip_all,
|
||||
fields(otel.name = "skills_for_config")
|
||||
)]
|
||||
pub async fn snapshot_for_config(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> 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(snapshot) = self.cached_snapshot_for_config(&cache_key) {
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
let snapshot = HostSkillsSnapshot::new(Arc::new(
|
||||
self.build_skill_outcome(input, 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, snapshot.clone());
|
||||
snapshot
|
||||
}
|
||||
|
||||
pub async fn skill_roots_for_config(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots(
|
||||
fs,
|
||||
&input.config_layer_stack,
|
||||
&input.cwd,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
)
|
||||
.await;
|
||||
if !input.bundled_skills_enabled {
|
||||
roots.retain(|root| root.scope != SkillScope::System);
|
||||
}
|
||||
roots
|
||||
}
|
||||
|
||||
pub async fn snapshot_for_cwd(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
force_reload: bool,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> HostSkillsSnapshot {
|
||||
let use_cwd_cache = fs.is_some();
|
||||
if use_cwd_cache
|
||||
&& !force_reload
|
||||
&& let Some(snapshot) = self.cached_snapshot_for_cwd(&input.cwd)
|
||||
{
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
let mut roots = skill_roots(
|
||||
fs.clone(),
|
||||
&input.config_layer_stack,
|
||||
&input.cwd,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
)
|
||||
.await;
|
||||
if !bundled_skills_enabled_from_stack(&input.config_layer_stack) {
|
||||
roots.retain(|root| root.scope != SkillScope::System);
|
||||
}
|
||||
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
|
||||
let snapshot = HostSkillsSnapshot::new(Arc::new(
|
||||
self.build_skill_outcome(input, 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(), snapshot.clone());
|
||||
}
|
||||
snapshot
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
async fn build_skill_outcome(
|
||||
&self,
|
||||
input: &HostSkillsLoadInput,
|
||||
roots: Vec<SkillRoot>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> SkillLoadOutcome {
|
||||
let outcome = load_skills_from_roots(
|
||||
roots,
|
||||
input.plugin_skill_snapshots.as_ref(),
|
||||
Arc::clone(&self.root_scan_slots),
|
||||
)
|
||||
.await;
|
||||
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);
|
||||
outcome.with_disabled_paths(disabled_paths)
|
||||
}
|
||||
|
||||
pub fn clear_cache(&self) {
|
||||
let cleared_cwd = {
|
||||
let mut cache = self
|
||||
.cache_by_cwd
|
||||
.write()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
let cleared = cache.len();
|
||||
cache.clear();
|
||||
cleared
|
||||
};
|
||||
let cleared_config = {
|
||||
let mut cache = self
|
||||
.cache_by_config
|
||||
.write()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
let cleared = cache.len();
|
||||
cache.clear();
|
||||
cleared
|
||||
};
|
||||
let cleared = cleared_cwd + cleared_config;
|
||||
info!("skills cache cleared ({cleared} entries)");
|
||||
}
|
||||
|
||||
fn cached_snapshot_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<HostSkillsSnapshot> {
|
||||
match self.cache_by_cwd.read() {
|
||||
Ok(cache) => cache.get(cwd).cloned(),
|
||||
Err(err) => err.into_inner().get(cwd).cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
fn cached_snapshot_for_config(
|
||||
&self,
|
||||
cache_key: &ConfigSkillsCacheKey,
|
||||
) -> Option<HostSkillsSnapshot> {
|
||||
match self.cache_by_config.read() {
|
||||
Ok(cache) => cache.get(cache_key).cloned(),
|
||||
Err(err) => err.into_inner().get(cache_key).cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
fn extra_roots(&self) -> Vec<AbsolutePathBuf> {
|
||||
match self.extra_roots.read() {
|
||||
Ok(roots) => roots.clone(),
|
||||
Err(err) => err.into_inner().clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
struct ConfigSkillsCacheKey {
|
||||
roots: Vec<ConfigSkillRootCacheKey>,
|
||||
skill_config_rules: SkillConfigRules,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
struct ConfigSkillRootCacheKey {
|
||||
path: AbsolutePathBuf,
|
||||
scope_rank: u8,
|
||||
plugin_identity: Option<PluginIdentity>,
|
||||
plugin_namespace: Option<String>,
|
||||
}
|
||||
|
||||
pub fn bundled_skills_enabled_from_stack(
|
||||
config_layer_stack: &codex_config::ConfigLayerStack,
|
||||
) -> bool {
|
||||
let effective_config = config_layer_stack.effective_config();
|
||||
let Some(skills_value) = effective_config
|
||||
.as_table()
|
||||
.and_then(|table| table.get("skills"))
|
||||
else {
|
||||
return true;
|
||||
};
|
||||
|
||||
let skills: SkillsConfig = match skills_value.clone().try_into() {
|
||||
Ok(skills) => skills,
|
||||
Err(err) => {
|
||||
warn!("invalid skills config: {err}");
|
||||
return true;
|
||||
}
|
||||
};
|
||||
|
||||
skills.bundled.unwrap_or_default().enabled
|
||||
}
|
||||
|
||||
fn config_skills_cache_key(
|
||||
roots: &[SkillRoot],
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> ConfigSkillsCacheKey {
|
||||
ConfigSkillsCacheKey {
|
||||
roots: roots
|
||||
.iter()
|
||||
.map(|root| {
|
||||
let scope_rank = match root.scope {
|
||||
SkillScope::Repo => 0,
|
||||
SkillScope::User => 1,
|
||||
SkillScope::System => 2,
|
||||
SkillScope::Admin => 3,
|
||||
};
|
||||
ConfigSkillRootCacheKey {
|
||||
path: root.path.clone(),
|
||||
scope_rank,
|
||||
plugin_identity: root.plugin_identity.clone(),
|
||||
plugin_namespace: root.plugin_namespace.clone(),
|
||||
}
|
||||
})
|
||||
.collect(),
|
||||
skill_config_rules: skill_config_rules.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "host_service_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,708 @@
|
||||
use super::*;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::PathExt;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashSet;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) {
|
||||
let skill_dir = codex_home.path().join("skills").join(dir);
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
|
||||
fs::write(skill_dir.join("SKILL.md"), content).unwrap();
|
||||
}
|
||||
|
||||
fn write_plugin_skill(
|
||||
codex_home: &TempDir,
|
||||
marketplace: &str,
|
||||
plugin_name: &str,
|
||||
dir: &str,
|
||||
name: &str,
|
||||
description: &str,
|
||||
) -> PathBuf {
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join(marketplace)
|
||||
.join(plugin_name)
|
||||
.join("local");
|
||||
let skill_dir = plugin_root.join("skills").join(dir);
|
||||
fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap();
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
format!(r#"{{"name":"{plugin_name}"}}"#),
|
||||
)
|
||||
.unwrap();
|
||||
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
|
||||
let skill_path = skill_dir.join("SKILL.md");
|
||||
fs::write(&skill_path, content).unwrap();
|
||||
skill_path
|
||||
}
|
||||
|
||||
fn plugin_skill_root_for_skill_path(
|
||||
skill_path: &Path,
|
||||
plugin_id: &str,
|
||||
plugin_namespace: &str,
|
||||
) -> PluginSkillRoot {
|
||||
let skills_root = skill_path
|
||||
.parent()
|
||||
.and_then(Path::parent)
|
||||
.expect("plugin skill should live under a skills root");
|
||||
let plugin_root = skills_root
|
||||
.parent()
|
||||
.expect("plugin skills root should live under a plugin root");
|
||||
PluginSkillRoot {
|
||||
path: skills_root.abs(),
|
||||
plugin_identity: PluginIdentity {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
remote_plugin_id: None,
|
||||
},
|
||||
plugin_namespace: plugin_namespace.to_string(),
|
||||
plugin_root: plugin_root.abs(),
|
||||
discovery_mode: SkillDiscoveryMode::Recursive,
|
||||
}
|
||||
}
|
||||
|
||||
fn user_config_layer(codex_home: &TempDir, config_toml: &str) -> ConfigLayerEntry {
|
||||
let config_path = AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE))
|
||||
.expect("user config path should be absolute");
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: config_path,
|
||||
profile: None,
|
||||
},
|
||||
toml::from_str(config_toml).expect("user layer toml"),
|
||||
)
|
||||
}
|
||||
|
||||
fn config_stack(codex_home: &TempDir, user_config_toml: &str) -> ConfigLayerStack {
|
||||
ConfigLayerStack::new(
|
||||
vec![user_config_layer(codex_home, user_config_toml)],
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack")
|
||||
}
|
||||
|
||||
fn config_stack_with_session_flags(
|
||||
codex_home: &TempDir,
|
||||
user_config_toml: &str,
|
||||
session_flags_toml: &str,
|
||||
) -> ConfigLayerStack {
|
||||
ConfigLayerStack::new(
|
||||
vec![
|
||||
user_config_layer(codex_home, user_config_toml),
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::SessionFlags,
|
||||
toml::from_str(session_flags_toml).expect("session layer toml"),
|
||||
),
|
||||
],
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack")
|
||||
}
|
||||
|
||||
fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String {
|
||||
format!(
|
||||
r#"[[skills.config]]
|
||||
path = "{}"
|
||||
enabled = {enabled}
|
||||
"#,
|
||||
path.display()
|
||||
)
|
||||
}
|
||||
|
||||
fn name_toggle_config(name: &str, enabled: bool) -> String {
|
||||
format!(
|
||||
r#"[[skills.config]]
|
||||
name = "{name}"
|
||||
enabled = {enabled}
|
||||
"#
|
||||
)
|
||||
}
|
||||
|
||||
async fn skills_for_config_with_stack(
|
||||
skills_service: &HostSkillsService,
|
||||
cwd: &TempDir,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
effective_skill_roots: &[PluginSkillRoot],
|
||||
) -> SkillLoadOutcome {
|
||||
let skills_input = HostSkillsLoadInput::new(
|
||||
cwd.path().abs(),
|
||||
effective_skill_roots.to_vec(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(config_layer_stack),
|
||||
);
|
||||
skills_service
|
||||
.snapshot_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS)))
|
||||
.await
|
||||
.outcome()
|
||||
.clone()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let stale_system_skill_dir = codex_home.path().join("skills/.system/stale-skill");
|
||||
fs::create_dir_all(&stale_system_skill_dir).expect("create stale system skill dir");
|
||||
fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n")
|
||||
.expect("write stale system skill");
|
||||
|
||||
let _skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ false,
|
||||
);
|
||||
|
||||
assert!(
|
||||
!codex_home.path().join("skills/.system").exists(),
|
||||
"expected disabling system skills to remove stale cached bundled skills"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
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 = HostSkillsService::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_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert!(
|
||||
outcome1.skills.iter().any(|s| s.name == "skill-a"),
|
||||
"expected skill-a to be discovered"
|
||||
);
|
||||
|
||||
// Write a new skill after the first call; the second call should reuse the config-aware cache
|
||||
// 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_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert_eq!(outcome2.errors, outcome1.errors);
|
||||
assert_eq!(outcome2.skills, outcome1.skills);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_plugin_skill(
|
||||
&codex_home,
|
||||
"test",
|
||||
"sample",
|
||||
"sample-search",
|
||||
"sample-search",
|
||||
"search sample data",
|
||||
);
|
||||
let config_layer_stack = config_stack(&codex_home, "");
|
||||
let mut plugin_skill_root =
|
||||
plugin_skill_root_for_skill_path(&skill_path, "sample@test", "sample");
|
||||
let skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
skills_for_config_with_stack(
|
||||
&skills_service,
|
||||
&cwd,
|
||||
&config_layer_stack,
|
||||
&[plugin_skill_root.clone()],
|
||||
)
|
||||
.await;
|
||||
|
||||
plugin_skill_root.plugin_identity.remote_plugin_id = Some("plugins~Plugin_sample".to_string());
|
||||
let refreshed = skills_for_config_with_stack(
|
||||
&skills_service,
|
||||
&cwd,
|
||||
&config_layer_stack,
|
||||
&[plugin_skill_root],
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
refreshed
|
||||
.skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "sample:sample-search")
|
||||
.and_then(|skill| skill.remote_plugin_id.as_deref()),
|
||||
Some("plugins~Plugin_sample")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let extra_root = tempfile::tempdir().expect("tempdir");
|
||||
let config_layer_stack = config_stack(&codex_home, "");
|
||||
let skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
let skills_input = HostSkillsLoadInput::new(
|
||||
cwd.path().abs(),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
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
|
||||
.iter()
|
||||
.all(|skill| skill.name != "runtime-skill")
|
||||
);
|
||||
|
||||
let extra_skills_root = extra_root.path().join("skills");
|
||||
let skill_dir = extra_skills_root.join("runtime-skill");
|
||||
fs::create_dir_all(&skill_dir).expect("create skill dir");
|
||||
fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write skill");
|
||||
skills_service.set_extra_roots(vec![extra_skills_root.abs()]);
|
||||
|
||||
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
|
||||
.iter()
|
||||
.any(|skill| skill.name == "runtime-skill")
|
||||
);
|
||||
|
||||
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
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "runtime-skill")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_extra_roots_applies_to_config_loads_and_empty_clears() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let extra_root = tempfile::tempdir().expect("tempdir");
|
||||
let config_layer_stack = config_stack(&codex_home, "");
|
||||
let skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
let empty_outcome =
|
||||
skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert!(
|
||||
empty_outcome
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "runtime-skill")
|
||||
);
|
||||
|
||||
let extra_skills_root = extra_root.path().join("skills");
|
||||
let skill_dir = extra_skills_root.join("runtime-skill");
|
||||
fs::create_dir_all(&skill_dir).expect("create skill dir");
|
||||
fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write skill");
|
||||
skills_service.set_extra_roots(vec![extra_skills_root.abs()]);
|
||||
|
||||
let runtime_outcome =
|
||||
skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert!(
|
||||
runtime_outcome
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| skill.name == "runtime-skill")
|
||||
);
|
||||
|
||||
skills_service.set_extra_roots(Vec::new());
|
||||
let cleared_outcome =
|
||||
skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert!(
|
||||
cleared_outcome
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "runtime-skill")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_config_disables_plugin_skills_by_name() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_plugin_skill(
|
||||
&codex_home,
|
||||
"test",
|
||||
"sample",
|
||||
"sample-search",
|
||||
"sample-search",
|
||||
"search sample data",
|
||||
);
|
||||
let config_layer_stack = config_stack(
|
||||
&codex_home,
|
||||
&name_toggle_config("sample:sample-search", /*enabled*/ false),
|
||||
);
|
||||
let plugin_skill_root =
|
||||
plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test", "sample");
|
||||
let skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
let outcome = skills_for_config_with_stack(
|
||||
&skills_service,
|
||||
&cwd,
|
||||
&config_layer_stack,
|
||||
&[plugin_skill_root],
|
||||
)
|
||||
.await;
|
||||
let skill = outcome
|
||||
.skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "sample:sample-search")
|
||||
.expect("plugin skill should load");
|
||||
let skill_path = dunce::canonicalize(skill_path)
|
||||
.expect("skill path should canonicalize")
|
||||
.abs();
|
||||
|
||||
assert_eq!(skill.path_to_skills_md, skill_path);
|
||||
assert!(outcome.disabled_paths.contains(&skill.path_to_skills_md));
|
||||
assert!(
|
||||
!outcome
|
||||
.allowed_skills_for_implicit_invocation()
|
||||
.iter()
|
||||
.any(|allowed_skill| allowed_skill.path_to_skills_md == skill.path_to_skills_md)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let repo_dot_codex = cwd.path().join(".codex");
|
||||
fs::create_dir_all(&repo_dot_codex).expect("create repo config dir");
|
||||
|
||||
write_user_skill(&codex_home, "user", "user-skill", "from local user root");
|
||||
let repo_skill_dir = repo_dot_codex.join("skills/repo");
|
||||
fs::create_dir_all(&repo_skill_dir).expect("create repo skill dir");
|
||||
fs::write(
|
||||
repo_skill_dir.join("SKILL.md"),
|
||||
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write repo skill");
|
||||
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
vec![
|
||||
user_config_layer(&codex_home, ""),
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::Project {
|
||||
dot_codex_folder: repo_dot_codex.abs(),
|
||||
},
|
||||
toml::Value::Table(toml::map::Map::new()),
|
||||
),
|
||||
],
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack");
|
||||
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 = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
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(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
let loaded_names = outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.name.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
assert!(loaded_names.contains("user-skill"));
|
||||
assert!(loaded_names.contains("repo-skill"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_cwd_without_fs_skips_repo_roots() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let repo_dot_codex = cwd.path().join(".codex");
|
||||
fs::create_dir_all(&repo_dot_codex).expect("create repo config dir");
|
||||
|
||||
write_user_skill(&codex_home, "user", "user-skill", "from local user root");
|
||||
let repo_skill_dir = repo_dot_codex.join("skills/repo");
|
||||
fs::create_dir_all(&repo_skill_dir).expect("create repo skill dir");
|
||||
fs::write(
|
||||
repo_skill_dir.join("SKILL.md"),
|
||||
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write repo skill");
|
||||
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
vec![
|
||||
user_config_layer(&codex_home, ""),
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::Project {
|
||||
dot_codex_folder: repo_dot_codex.abs(),
|
||||
},
|
||||
toml::Value::Table(toml::map::Map::new()),
|
||||
),
|
||||
],
|
||||
Default::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack");
|
||||
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 = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
let snapshot = skills_service
|
||||
.snapshot_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None)
|
||||
.await;
|
||||
let outcome = snapshot.outcome();
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
let loaded_names = outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.name.as_str())
|
||||
.collect::<HashSet<_>>();
|
||||
assert!(loaded_names.contains("user-skill"));
|
||||
assert!(!loaded_names.contains("repo-skill"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let bundled_skill_dir = codex_home.path().join("skills/.system/bundled-skill");
|
||||
fs::create_dir_all(&bundled_skill_dir).expect("create bundled skill dir");
|
||||
fs::write(
|
||||
bundled_skill_dir.join("SKILL.md"),
|
||||
"---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write bundled skill");
|
||||
let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n");
|
||||
let skills_service = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ false,
|
||||
);
|
||||
|
||||
// Recreate the cached bundled skill after startup cleanup so this assertion exercises
|
||||
// root selection rather than relying on directory removal succeeding.
|
||||
fs::create_dir_all(&bundled_skill_dir).expect("recreate bundled skill dir");
|
||||
fs::write(
|
||||
bundled_skill_dir.join("SKILL.md"),
|
||||
"---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("rewrite bundled skill");
|
||||
|
||||
let outcome =
|
||||
skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
|
||||
assert!(
|
||||
outcome
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "bundled-skill")
|
||||
);
|
||||
assert!(
|
||||
outcome
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.scope != SkillScope::System)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
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 = 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 = HostSkillsLoadInput::new(
|
||||
cwd.path().abs(),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
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
|
||||
.iter()
|
||||
.all(|skill| skill.name != "late-skill")
|
||||
);
|
||||
|
||||
write_user_skill(&codex_home, "late", "late-skill", "added after cache");
|
||||
|
||||
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
|
||||
.iter()
|
||||
.all(|skill| skill.name != "late-skill")
|
||||
);
|
||||
|
||||
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
|
||||
.iter()
|
||||
.any(|skill| skill.name == "late-skill")
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg_attr(windows, ignore)]
|
||||
#[tokio::test]
|
||||
async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let cwd = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = codex_home.path().join("skills").join("demo");
|
||||
fs::create_dir_all(&skill_dir).expect("create skill dir");
|
||||
let skill_path = skill_dir.join("SKILL.md");
|
||||
fs::write(
|
||||
&skill_path,
|
||||
"---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write skill");
|
||||
let disabled_skill_config = path_toggle_config(&skill_path, /*enabled*/ false);
|
||||
let enabled_skill_config = path_toggle_config(&skill_path, /*enabled*/ true);
|
||||
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 = HostSkillsService::new(
|
||||
codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
let parent_input = HostSkillsLoadInput::new(
|
||||
cwd.path().abs(),
|
||||
Vec::new(),
|
||||
parent_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&parent_stack),
|
||||
);
|
||||
|
||||
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()
|
||||
.find(|skill| skill.name == "demo-skill")
|
||||
.expect("demo skill should be discovered");
|
||||
assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false);
|
||||
|
||||
let child_outcome =
|
||||
skills_for_config_with_stack(&skills_service, &cwd, &child_stack, &[]).await;
|
||||
let child_skill = child_outcome
|
||||
.skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "demo-skill")
|
||||
.expect("demo skill should be discovered");
|
||||
assert_eq!(child_outcome.is_skill_enabled(child_skill), true);
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user