clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-29 23:40:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use environment path refs for skill roots

Browse Source
This commit is contained in:
starr-openai
2026-05-28 16:27:09 -07:00
parent dfa0274dd8
commit 8e20d8ea5f
6 changed files with 29 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
codex-rs/core-plugins/src/loader.rs
View File

@@ -14,7 +14,6 @@ use codex_config::types::McpServerConfig;
use codex_config::types::PluginConfig;
use codex_config::types::PluginMcpServerConfig;
use codex_core_skills::SkillMetadata;
use codex_core_skills::SkillRootPathRef;
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;
@@ -661,7 +660,7 @@ pub async fn load_plugin_skills(
let roots = plugin_skill_roots(plugin_root.path(), manifest_paths)
.into_iter()
.map(|path| SkillRoot {
path: SkillRootPathRef::new(plugin_root.with_path(path)),
path: plugin_root.with_path(path),
scope: SkillScope::User,
plugin_id: Some(plugin_id.as_key()),
plugin_root: Some(plugin_root.clone()),

1
codex-rs/core-skills/src/lib.rs
View File

@@ -18,7 +18,6 @@ pub use model::SkillError;
pub use model::SkillLoadOutcome;
pub use model::SkillMetadata;
pub use model::SkillPolicy;
pub use model::SkillRootPathRef;
pub use model::filter_skill_load_outcome_for_product;
pub use render::AvailableSkills;
pub use render::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;

40
codex-rs/core-skills/src/loader.rs
View File

@@ -5,7 +5,6 @@ use crate::model::SkillInterface;
use crate::model::SkillLoadOutcome;
use crate::model::SkillMetadata;
use crate::model::SkillPolicy;
use crate::model::SkillRootPathRef;
use crate::model::SkillToolDependency;
use crate::system::system_cache_root_dir;
use codex_app_server_protocol::ConfigLayerSource;
@@ -152,7 +151,7 @@ impl fmt::Display for SkillParseError {
impl Error for SkillParseError {}
pub struct SkillRoot {
pub path: SkillRootPathRef,
pub path: EnvironmentPathRef,
pub scope: SkillScope,
pub plugin_id: Option<String>,
pub plugin_root: Option<EnvironmentPathRef>,
@@ -169,8 +168,7 @@ where
HashMap::new();
for root in roots {
let root_path = canonicalize_for_skill_identity(root.path.path());
let root_path_ref =
SkillRootPathRef::new(root.path.path_ref().with_path(root_path.clone()));
let root_path_ref = root.path.with_path(root_path.clone());
let fs = root_path_ref.file_system();
let skills_before_root = outcome.skills.len();
discover_skills_under_root(
@@ -264,7 +262,7 @@ async fn skill_roots_with_home_dir(
);
if let Some(skill_root_path_ref) = skill_root_path_ref {
roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref.with_path(root.path)),
path: skill_root_path_ref.with_path(root.path),
scope: SkillScope::User,
plugin_id: Some(root.plugin_id),
plugin_root: Some(skill_root_path_ref.with_path(root.plugin_root)),
@@ -295,9 +293,7 @@ fn skill_roots_from_layer_stack_inner(
ConfigLayerSource::Project { .. } => {
if let Some(env_path_ref) = env_path_ref {
roots.push(SkillRoot {
path: SkillRootPathRef::new(
env_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
),
path: env_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::Repo,
plugin_id: None,
plugin_root: None,
@@ -311,9 +307,7 @@ fn skill_roots_from_layer_stack_inner(
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
// compatibility.
roots.push(SkillRoot {
path: SkillRootPathRef::new(
skill_root_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
),
path: skill_root_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::User,
plugin_id: None,
plugin_root: None,
@@ -322,10 +316,8 @@ fn skill_roots_from_layer_stack_inner(
// `$HOME/.agents/skills` (user-installed skills).
if let Some(home_dir) = home_dir {
roots.push(SkillRoot {
path: SkillRootPathRef::new(
skill_root_path_ref
.with_path(home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)),
),
path: skill_root_path_ref
.with_path(home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)),
scope: SkillScope::User,
plugin_id: None,
plugin_root: None,
@@ -335,9 +327,7 @@ fn skill_roots_from_layer_stack_inner(
// Embedded system skills are cached under `$CODEX_HOME/skills/.system` and are a
// special case (not a config layer).
roots.push(SkillRoot {
path: SkillRootPathRef::new(
skill_root_path_ref.with_path(system_cache_root_dir(&config_folder)),
),
path: skill_root_path_ref.with_path(system_cache_root_dir(&config_folder)),
scope: SkillScope::System,
plugin_id: None,
plugin_root: None,
@@ -350,9 +340,7 @@ fn skill_roots_from_layer_stack_inner(
// The system config layer lives under `/etc/codex/` on Unix, so treat
// `/etc/codex/skills` as admin-scoped skills.
roots.push(SkillRoot {
path: SkillRootPathRef::new(
skill_root_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
),
path: skill_root_path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::Admin,
plugin_id: None,
plugin_root: None,
@@ -383,7 +371,7 @@ async fn repo_agents_skill_roots(
let agents_skills = env_path_ref.with_path(dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME));
match agents_skills.metadata().await {
Ok(metadata) if metadata.is_directory => roots.push(SkillRoot {
path: SkillRootPathRef::new(agents_skills),
path: agents_skills,
scope: SkillScope::Repo,
plugin_id: None,
plugin_root: None,
@@ -473,7 +461,7 @@ fn dirs_between_project_root_and_cwd(
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
let mut seen: HashSet<EnvironmentPathRef> = HashSet::new();
roots.retain(|root| seen.insert(root.path.path_ref().clone()));
roots.retain(|root| seen.insert(root.path.clone()));
}
fn canonicalize_for_skill_identity(path: &AbsolutePathBuf) -> AbsolutePathBuf {
@@ -481,15 +469,13 @@ fn canonicalize_for_skill_identity(path: &AbsolutePathBuf) -> AbsolutePathBuf {
}
async fn discover_skills_under_root(
root: &SkillRootPathRef,
root: &EnvironmentPathRef,
scope: SkillScope,
plugin_id: Option<&str>,
plugin_root: Option<&EnvironmentPathRef>,
outcome: &mut SkillLoadOutcome,
) {
let root = root
.path_ref()
.with_path(canonicalize_for_skill_identity(root.path()));
let root = root.with_path(canonicalize_for_skill_identity(root.path()));
let plugin_root = plugin_root.map(|plugin_root| {
plugin_root.with_path(canonicalize_for_skill_identity(plugin_root.path()))
});

28
codex-rs/core-skills/src/loader_tests.rs
View File

@@ -145,7 +145,7 @@ fn normalized(path: &Path) -> AbsolutePathBuf {
.abs()
}
fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
fn env_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
EnvironmentPathRef::new(
LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&LOCAL_FS),
@@ -155,7 +155,7 @@ fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
fn local_skill_root(path: AbsolutePathBuf, scope: SkillScope) -> SkillRoot {
SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(path)),
path: env_path_ref(path),
scope,
plugin_id: None,
plugin_root: None,
@@ -864,10 +864,10 @@ interface:
let plugin_root_abs = plugin_root.abs();
let outcome = load_skills_from_roots([SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(plugin_root.join("skills").abs())),
path: env_path_ref(plugin_root.join("skills").abs()),
scope: SkillScope::User,
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_root: Some(skill_root_path_ref(plugin_root_abs.clone())),
plugin_root: Some(env_path_ref(plugin_root_abs.clone())),
}])
.await;
@@ -920,10 +920,10 @@ interface:
);
let outcome = load_skills_from_roots([SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(plugin_root.join("skills").abs())),
path: env_path_ref(plugin_root.join("skills").abs()),
scope: SkillScope::User,
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_root: Some(skill_root_path_ref(plugin_root.abs())),
plugin_root: Some(env_path_ref(plugin_root.abs())),
}])
.await;
@@ -1271,10 +1271,10 @@ async fn namespaces_plugin_skills_using_plugin_name() {
.unwrap();
let outcome = load_skills_from_roots([SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(plugin_root.join("skills").abs())),
path: env_path_ref(plugin_root.join("skills").abs()),
scope: SkillScope::User,
plugin_id: Some("sample@test".to_string()),
plugin_root: Some(skill_root_path_ref(plugin_root.abs())),
plugin_root: Some(env_path_ref(plugin_root.abs())),
}])
.await;
@@ -1593,13 +1593,13 @@ async fn deduplicates_by_path_preferring_first_root() {
let outcome = load_skills_from_roots([
SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(root.path().abs())),
path: env_path_ref(root.path().abs()),
scope: SkillScope::Repo,
plugin_id: None,
plugin_root: None,
},
SkillRoot {
path: SkillRootPathRef::new(skill_root_path_ref(root.path().abs())),
path: env_path_ref(root.path().abs()),
scope: SkillScope::User,
plugin_id: None,
plugin_root: None,
@@ -1891,7 +1891,7 @@ async fn skill_roots_include_admin_with_lowest_priority() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cfg = make_config(&codex_home).await;
let path_ref = skill_root_path_ref(cfg.cwd.clone());
let path_ref = env_path_ref(cfg.cwd.clone());
let scopes: Vec<SkillScope> = super::skill_roots(
Some(&path_ref),
Some(&path_ref),
@@ -1911,13 +1911,13 @@ async fn skill_roots_include_admin_with_lowest_priority() {
}
#[tokio::test]
async fn skill_roots_skip_local_roots_without_skill_root_path_ref() {
async fn skill_roots_skip_local_roots_without_local_env_path_ref() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cfg = make_config(&codex_home).await;
let env_path_ref = skill_root_path_ref(cfg.cwd.clone());
let local_env_path_ref = env_path_ref(cfg.cwd.clone());
let scopes: Vec<SkillScope> = super::skill_roots(
Some(&env_path_ref),
Some(&local_env_path_ref),
/*skill_root_path_ref*/ None,
&cfg.config_layer_stack,
Vec::new(),

2
codex-rs/core-skills/src/manager.rs
View File

@@ -253,7 +253,7 @@ fn config_skills_cache_key(
SkillScope::Admin => 3,
};
(
root.path.path_ref().clone(),
root.path.clone(),
scope_rank,
root.plugin_id.clone(),
)

24
codex-rs/core-skills/src/model.rs
View File

@@ -1,10 +1,7 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::sync::Arc;
use codex_exec_server::EnvironmentPathRef;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -47,27 +44,6 @@ impl SkillMetadata {
}
}
#[derive(Clone, Debug)]
pub struct SkillRootPathRef(EnvironmentPathRef);
impl SkillRootPathRef {
pub fn new(path_ref: EnvironmentPathRef) -> Self {
Self(path_ref)
}
pub fn path_ref(&self) -> &EnvironmentPathRef {
&self.0
}
pub fn path(&self) -> &AbsolutePathBuf {
self.0.path()
}
pub(crate) fn file_system(&self) -> Arc<dyn ExecutorFileSystem> {
self.0.file_system()
}
}
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct SkillPolicy {
pub allow_implicit_invocation: Option<bool>,