Files
codex/codex-rs/core-skills/src/manager.rs
xli-oai 314229fd72 Remove skills list extra roots (#21485)
## Summary
- Remove `perCwdExtraUserRoots` / `SkillsListExtraRootsForCwd` from the
`skills/list` app-server API.
- Drop Rust app-server and `codex-core-skills` extra-root plumbing so
skill scans are keyed by the normal cwd/user/plugin roots only.
- Regenerate app-server schemas and update docs/tests that only existed
for the removed extra-roots behavior.

## Validation
- `just write-app-server-schema`
- `just fmt`
- `cargo test -p codex-app-server-protocol`
- `cargo test -p codex-core-skills`
- `just fix -p codex-app-server-protocol`
- `just fix -p codex-core-skills`
- `just fix -p codex-app-server`
- `just fix -p codex-tui`

## Notes
- `cargo test -p codex-app-server --test all skills_list` ran the edited
skills-list cases, but the full filtered run ended on existing
`skills_changed_notification_is_emitted_after_skill_change` timeout
after a websocket `401`.
- `cargo test -p codex-tui --lib` compiled the changed TUI callers, then
failed two unrelated status permission tests because local
`/etc/codex/requirements.toml` forbids `DangerFullAccess`.
- Source-truth check found the OpenAI monorepo still has
generated/app-server-kit mirror references to the removed field; those
should be cleaned up when generated app-server types are synced or in a
companion OpenAI cleanup.
2026-05-07 20:56:42 -07:00

283 lines
9.1 KiB
Rust

use std::collections::HashMap;
use std::collections::HashSet;
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::PluginSkillRoot;
use tracing::info;
use tracing::warn;
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::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;
#[derive(Debug, Clone)]
pub struct SkillsLoadInput {
pub cwd: AbsolutePathBuf,
pub effective_skill_roots: Vec<PluginSkillRoot>,
pub config_layer_stack: ConfigLayerStack,
pub bundled_skills_enabled: bool,
}
impl SkillsLoadInput {
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,
}
}
}
pub struct SkillsManager {
codex_home: AbsolutePathBuf,
restriction_product: Option<Product>,
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>,
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
}
impl SkillsManager {
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 manager = Self {
codex_home,
restriction_product,
cache_by_cwd: RwLock::new(HashMap::new()),
cache_by_config: RwLock::new(HashMap::new()),
};
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) {
tracing::error!("failed to install system skills: {err}");
}
manager
}
/// 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.
pub async fn skills_for_config(
&self,
input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome {
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;
}
let outcome = 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
}
pub async fn skill_roots_for_config(
&self,
input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> Vec<SkillRoot> {
let mut roots = skill_roots(
fs,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
)
.await;
if !input.bundled_skills_enabled {
roots.retain(|root| root.scope != SkillScope::System);
}
roots
}
pub async fn skills_for_cwd(
&self,
input: &SkillsLoadInput,
force_reload: bool,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome {
let use_cwd_cache = fs.is_some();
if use_cwd_cache
&& !force_reload
&& let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd)
{
return outcome;
}
let mut roots = skill_roots(
fs.clone(),
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
)
.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 outcome = 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());
}
outcome
}
async fn build_skill_outcome(
&self,
roots: Vec<SkillRoot>,
skill_config_rules: &SkillConfigRules,
) -> SkillLoadOutcome {
let outcome = crate::filter_skill_load_outcome_for_product(
load_skills_from_roots(roots).await,
self.restriction_product,
);
let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules);
finalize_skill_outcome(outcome, 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_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<SkillLoadOutcome> {
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(
&self,
cache_key: &ConfigSkillsCacheKey,
) -> Option<SkillLoadOutcome> {
match self.cache_by_config.read() {
Ok(cache) => cache.get(cache_key).cloned(),
Err(err) => err.into_inner().get(cache_key).cloned(),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<(AbsolutePathBuf, u8, Option<String>)>,
skill_config_rules: SkillConfigRules,
}
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,
};
(root.path.clone(), scope_rank, root.plugin_id.clone())
})
.collect(),
skill_config_rules: skill_config_rules.clone(),
}
}
fn finalize_skill_outcome(
mut outcome: SkillLoadOutcome,
disabled_paths: HashSet<AbsolutePathBuf>,
) -> SkillLoadOutcome {
outcome.disabled_paths = disabled_paths;
let (by_scripts_dir, by_doc_path) =
build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation());
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 = "manager_tests.rs"]
mod tests;