openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-03 19:53:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move skills path refs into exec server

Browse Source
This commit is contained in:
starr-openai
2026-05-28 22:08:58 -07:00
parent 56958f2512
commit be7728da60
18 changed files with 731 additions and 379 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/catalog_processor.rs
+23 -7
View File
@@ -513,15 +513,14 @@ impl CatalogRequestProcessor {
.await;
let skills_manager = self.thread_manager.skills_manager();
let plugins_manager = self.thread_manager.plugins_manager();
let fs = self
.thread_manager
.environment_manager()
.default_environment()
let environment_manager = self.thread_manager.environment_manager();
let local_fs = environment_manager
.try_local_environment()
.map(|environment| environment.get_filesystem());
let mut data = futures::stream::iter(cwds.into_iter().enumerate())
.map(|(index, cwd)| {
let config = &config;
let fs = fs.clone();
let local_fs = local_fs.clone();
let plugins_manager = &plugins_manager;
let skills_manager = &skills_manager;
async move {
@@ -542,6 +541,23 @@ impl CatalogRequestProcessor {
);
}
};
let local_path_ref = local_fs.clone().map(|file_system| {
codex_core::skills::EnvironmentPathRef::new(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
file_system,
cwd_abs.clone(),
)
});
let Some(path_ref) = local_path_ref else {
return (
index,
codex_app_server_protocol::SkillsListEntry {
cwd,
skills: Vec::new(),
errors: Vec::new(),
},
);
};
let effective_skill_roots = if workspace_codex_plugins_enabled {
let plugins_input = config.plugins_config_input();
plugins_manager
@@ -554,13 +570,13 @@ impl CatalogRequestProcessor {
Vec::new()
};
let skills_input = codex_core::skills::SkillsLoadInput::new(
cwd_abs.clone(),
path_ref,
effective_skill_roots,
config_layer_stack,
config.bundled_skills_enabled(),
);
let outcome = skills_manager
.skills_for_cwd(&skills_input, force_reload, fs)
.skills_for_cwd(&skills_input, force_reload)
.await;
let errors = errors_to_info(&outcome.errors);
let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths);
codex-rs/app-server/src/skills_watcher.rs
+9 -3
View File
@@ -7,6 +7,7 @@ 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::EnvironmentPathRef;
use codex_core::skills::SkillsLoadInput;
use codex_core::skills::SkillsManager;
use codex_file_watcher::FileWatcher;
@@ -100,22 +101,27 @@ impl SkillsWatcher {
return WatchRegistration::default();
}
let root_path_ref = EnvironmentPathRef::new(
environment_selection.environment_id.clone(),
environment.get_filesystem(),
config.cwd.clone(),
);
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(
config.cwd.clone(),
root_path_ref,
plugin_outcome.effective_plugin_skill_roots(),
config.config_layer_stack.clone(),
config.bundled_skills_enabled(),
);
let roots = thread_manager
.skills_manager()
.skill_roots_for_config(&skills_input, Some(environment.get_filesystem()))
.skill_roots_for_config(&skills_input)
.await
.into_iter()
.map(|root| WatchPath {
path: root.path.into_path_buf(),
path: root.path.path().to_path_buf(),
recursive: true,
})
.collect();
codex-rs/core-plugins/src/loader.rs
+5 -7
View File
@@ -19,7 +19,7 @@ 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::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::LOCAL_FS;
use codex_exec_server::EnvironmentPathRef;
use codex_plugin::AppConnectorId;
use codex_plugin::LoadedPlugin;
use codex_plugin::PluginCapabilitySummary;
@@ -40,7 +40,6 @@ use std::collections::HashSet;
use std::fs;
use std::path::Path;
use std::process::Command;
use std::sync::Arc;
use tempfile::TempDir;
use tracing::warn;
@@ -565,7 +564,7 @@ async fn load_plugin(
loaded_plugin.manifest_description = manifest.description.clone();
loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths);
let resolved_skills = load_plugin_skills(
&plugin_root,
EnvironmentPathRef::local(plugin_root.clone()),
&loaded_plugin_id,
manifest_paths,
restriction_product,
@@ -642,18 +641,17 @@ impl ResolvedPluginSkills {
}
pub async fn load_plugin_skills(
plugin_root: &AbsolutePathBuf,
plugin_root: EnvironmentPathRef,
plugin_id: &PluginId,
manifest_paths: &PluginManifestPaths,
restriction_product: Option<Product>,
skill_config_rules: &SkillConfigRules,
) -> ResolvedPluginSkills {
let roots = plugin_skill_roots(plugin_root, manifest_paths)
let roots = plugin_skill_roots(plugin_root.path(), manifest_paths)
.into_iter()
.map(|path| SkillRoot {
path,
path: plugin_root.with_path(path),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some(plugin_id.as_key()),
plugin_root: Some(plugin_root.clone()),
})
codex-rs/core-plugins/src/manager.rs
+1 -1
View File
@@ -1420,7 +1420,7 @@ impl PluginsManager {
marketplace_category,
);
let resolved_skills = load_plugin_skills(
&source_path,
codex_exec_server::EnvironmentPathRef::local(source_path.clone()),
&plugin_id,
&manifest.paths,
self.restriction_product,
codex-rs/core-skills/src/loader.rs
+110 -113
View File
@@ -13,8 +13,8 @@ use codex_config::ConfigLayerStackOrdering;
use codex_config::default_project_root_markers;
use codex_config::merge_toml_values;
use codex_config::project_root_markers_from_config;
use codex_exec_server::EnvironmentPathRef;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -151,11 +151,10 @@ impl fmt::Display for SkillParseError {
impl Error for SkillParseError {}
pub struct SkillRoot {
pub path: AbsolutePathBuf,
pub path: EnvironmentPathRef,
pub scope: SkillScope,
pub file_system: Arc<dyn ExecutorFileSystem>,
pub plugin_id: Option<String>,
pub plugin_root: Option<AbsolutePathBuf>,
pub plugin_root: Option<EnvironmentPathRef>,
}
pub async fn load_skills_from_roots<I>(roots: I) -> SkillLoadOutcome
@@ -168,12 +167,12 @@ where
let mut file_systems_by_skill_path: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>> =
HashMap::new();
for root in roots {
let root_path = canonicalize_for_skill_identity(&root.path);
let fs = root.file_system;
let root_path = canonicalize_for_skill_identity(root.path.path());
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(
fs.as_ref(),
&root_path,
&root_path_ref,
root.scope,
root.plugin_id.as_deref(),
root.plugin_root.as_ref(),
@@ -231,18 +230,16 @@ where
}
pub(crate) async fn skill_roots(
fs: Option<Arc<dyn ExecutorFileSystem>>,
path_ref: &EnvironmentPathRef,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
) -> Vec<SkillRoot> {
let home_dir =
home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
skill_roots_with_home_dir(
fs,
path_ref,
config_layer_stack,
cwd,
home_dir.as_ref(),
plugin_skill_roots,
extra_skill_roots,
@@ -251,29 +248,26 @@ pub(crate) async fn skill_roots(
}
async fn skill_roots_with_home_dir(
fs: Option<Arc<dyn ExecutorFileSystem>>,
path_ref: &EnvironmentPathRef,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
home_dir: Option<&AbsolutePathBuf>,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
) -> Vec<SkillRoot> {
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir, fs.clone());
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir, path_ref);
roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot {
path: root.path,
path: path_ref.with_path(root.path),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some(root.plugin_id),
plugin_root: Some(root.plugin_root),
plugin_root: Some(path_ref.with_path(root.plugin_root)),
}));
roots.extend(extra_skill_roots.into_iter().map(|path| SkillRoot {
path,
path: path_ref.with_path(path),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}));
roots.extend(repo_agents_skill_roots(fs, config_layer_stack, cwd).await);
roots.extend(repo_agents_skill_roots(path_ref, config_layer_stack).await);
dedupe_skill_roots_by_path(&mut roots);
roots
}
@@ -281,7 +275,7 @@ async fn skill_roots_with_home_dir(
fn skill_roots_from_layer_stack_inner(
config_layer_stack: &ConfigLayerStack,
home_dir: Option<&AbsolutePathBuf>,
repo_fs: Option<Arc<dyn ExecutorFileSystem>>,
path_ref: &EnvironmentPathRef,
) -> Vec<SkillRoot> {
let mut roots = Vec::new();
@@ -295,23 +289,19 @@ fn skill_roots_from_layer_stack_inner(
match &layer.name {
ConfigLayerSource::Project { .. } => {
if let Some(repo_fs) = &repo_fs {
roots.push(SkillRoot {
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::Repo,
file_system: Arc::clone(repo_fs),
plugin_id: None,
plugin_root: None,
});
}
roots.push(SkillRoot {
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::Repo,
plugin_id: None,
plugin_root: None,
});
}
ConfigLayerSource::User { .. } => {
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
// compatibility.
roots.push(SkillRoot {
path: config_folder.join(SKILLS_DIR_NAME),
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
});
@@ -319,9 +309,9 @@ fn skill_roots_from_layer_stack_inner(
// `$HOME/.agents/skills` (user-installed skills).
if let Some(home_dir) = home_dir {
roots.push(SkillRoot {
path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
path: path_ref
.with_path(home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
});
@@ -330,9 +320,8 @@ 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: system_cache_root_dir(&config_folder),
path: path_ref.with_path(system_cache_root_dir(&config_folder)),
scope: SkillScope::System,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
});
@@ -341,9 +330,8 @@ 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: config_folder.join(SKILLS_DIR_NAME),
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
scope: SkillScope::Admin,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
});
@@ -359,24 +347,19 @@ fn skill_roots_from_layer_stack_inner(
}
async fn repo_agents_skill_roots(
fs: Option<Arc<dyn ExecutorFileSystem>>,
path_ref: &EnvironmentPathRef,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
) -> Vec<SkillRoot> {
let Some(fs) = fs else {
return Vec::new();
};
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
let project_root = find_project_root(fs.as_ref(), cwd, &project_root_markers).await;
let dirs = dirs_between_project_root_and_cwd(cwd, &project_root);
let project_root = find_project_root(path_ref, &project_root_markers).await;
let dirs = dirs_between_project_root_and_cwd(path_ref.path(), project_root.path());
let mut roots = Vec::new();
for dir in dirs {
let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
match fs.get_metadata(&agents_skills, /*sandbox*/ None).await {
let agents_skills = path_ref.with_path(dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME));
match agents_skills.metadata(/*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => roots.push(SkillRoot {
path: agents_skills,
scope: SkillScope::Repo,
file_system: Arc::clone(&fs),
plugin_id: None,
plugin_root: None,
}),
@@ -385,7 +368,7 @@ async fn repo_agents_skill_roots(
Err(err) => {
tracing::warn!(
"failed to stat repo skills root {}: {err:#}",
agents_skills.display()
agents_skills.path().display()
);
}
}
@@ -416,24 +399,23 @@ fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec
}
async fn find_project_root(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
cwd: &EnvironmentPathRef,
project_root_markers: &[String],
) -> AbsolutePathBuf {
) -> EnvironmentPathRef {
if project_root_markers.is_empty() {
return cwd.clone();
}
for ancestor in cwd.ancestors() {
for ancestor in cwd.path().ancestors() {
for marker in project_root_markers {
let marker_path = ancestor.join(marker);
match fs.get_metadata(&marker_path, /*sandbox*/ None).await {
Ok(_) => return ancestor,
let marker_path = cwd.with_path(ancestor.join(marker));
match marker_path.metadata(/*sandbox*/ None).await {
Ok(_) => return cwd.with_path(ancestor),
Err(err) if err.kind() == io::ErrorKind::NotFound => {}
Err(err) => {
tracing::warn!(
"failed to stat project root marker {}: {err:#}",
marker_path.display()
marker_path.path().display()
);
}
}
@@ -465,7 +447,7 @@ fn dirs_between_project_root_and_cwd(
}
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
let mut seen: HashSet<AbsolutePathBuf> = HashSet::new();
let mut seen: HashSet<EnvironmentPathRef> = HashSet::new();
roots.retain(|root| seen.insert(root.path.clone()));
}
@@ -474,31 +456,35 @@ fn canonicalize_for_skill_identity(path: &AbsolutePathBuf) -> AbsolutePathBuf {
}
async fn discover_skills_under_root(
fs: &dyn ExecutorFileSystem,
root: &AbsolutePathBuf,
root: &EnvironmentPathRef,
scope: SkillScope,
plugin_id: Option<&str>,
plugin_root: Option<&AbsolutePathBuf>,
plugin_root: Option<&EnvironmentPathRef>,
outcome: &mut SkillLoadOutcome,
) {
let root = canonicalize_for_skill_identity(root);
let plugin_root = plugin_root.map(canonicalize_for_skill_identity);
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()))
});
match fs.get_metadata(&root, /*sandbox*/ None).await {
match root.metadata(/*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => {}
Ok(_) => return,
Err(err) if err.kind() == io::ErrorKind::NotFound => return,
Err(err) => {
error!("failed to stat skills root {}: {err:#}", root.display());
error!(
"failed to stat skills root {}: {err:#}",
root.path().display()
);
return;
}
}
fn enqueue_dir(
queue: &mut VecDeque<(AbsolutePathBuf, usize)>,
queue: &mut VecDeque<(EnvironmentPathRef, usize)>,
visited_dirs: &mut HashSet<AbsolutePathBuf>,
truncated_by_dir_limit: &mut bool,
path: AbsolutePathBuf,
path: EnvironmentPathRef,
depth: usize,
) {
if depth > MAX_SCAN_DEPTH {
@@ -508,7 +494,7 @@ async fn discover_skills_under_root(
*truncated_by_dir_limit = true;
return;
}
if visited_dirs.insert(path.clone()) {
if visited_dirs.insert(path.path().clone()) {
queue.push_back((path, depth));
}
}
@@ -520,16 +506,16 @@ async fn discover_skills_under_root(
);
let mut visited_dirs: HashSet<AbsolutePathBuf> = HashSet::new();
visited_dirs.insert(root.clone());
visited_dirs.insert(root.path().clone());
let mut queue: VecDeque<(AbsolutePathBuf, usize)> = VecDeque::from([(root.clone(), 0)]);
let mut queue: VecDeque<(EnvironmentPathRef, usize)> = VecDeque::from([(root.clone(), 0)]);
let mut truncated_by_dir_limit = false;
while let Some((dir, depth)) = queue.pop_front() {
let entries = match fs.read_directory(&dir, /*sandbox*/ None).await {
let entries = match dir.read_directory(/*sandbox*/ None).await {
Ok(entries) => entries,
Err(e) => {
error!("failed to read skills dir {}: {e:#}", dir.display());
error!("failed to read skills dir {}: {e:#}", dir.path().display());
continue;
}
};
@@ -540,11 +526,16 @@ async fn discover_skills_under_root(
continue;
}
let path = dir.join(&file_name);
let metadata = match fs.get_metadata(&path, /*sandbox*/ None).await {
let Some(path) = dir.join_relative(Path::new(&file_name)) else {
continue;
};
let metadata = match path.metadata(/*sandbox*/ None).await {
Ok(metadata) => metadata,
Err(e) => {
error!("failed to stat skills path {}: {e:#}", path.display());
error!(
"failed to stat skills path {}: {e:#}",
path.path().display()
);
continue;
}
};
@@ -553,9 +544,10 @@ async fn discover_skills_under_root(
if !follow_symlinks {
continue;
}
match fs.read_directory(&path, /*sandbox*/ None).await {
match path.read_directory(/*sandbox*/ None).await {
Ok(_) => {
let resolved_dir = canonicalize_for_skill_identity(&path);
let resolved_dir =
path.with_path(canonicalize_for_skill_identity(path.path()));
enqueue_dir(
&mut queue,
&mut visited_dirs,
@@ -572,7 +564,7 @@ async fn discover_skills_under_root(
Err(err) => {
error!(
"failed to read skills symlink dir {}: {err:#}",
path.display()
path.path().display()
);
}
}
@@ -580,7 +572,7 @@ async fn discover_skills_under_root(
}
if metadata.is_directory {
let resolved_dir = canonicalize_for_skill_identity(&path);
let resolved_dir = path.with_path(canonicalize_for_skill_identity(path.path()));
enqueue_dir(
&mut queue,
&mut visited_dirs,
@@ -592,14 +584,14 @@ async fn discover_skills_under_root(
}
if metadata.is_file && file_name == SKILLS_FILENAME {
match parse_skill_file(fs, &path, scope, plugin_id, plugin_root.as_ref()).await {
match parse_skill_file(&path, scope, plugin_id, plugin_root.as_ref()).await {
Ok(skill) => {
outcome.skills.push(skill);
}
Err(err) => {
if scope != SkillScope::System {
outcome.errors.push(SkillError {
path: path.clone(),
path: path.path().clone(),
message: err.to_string(),
});
}
@@ -613,22 +605,22 @@ async fn discover_skills_under_root(
tracing::warn!(
"skills scan truncated after {} directories (root: {})",
MAX_SKILLS_DIRS_PER_ROOT,
root.display()
root.path().display()
);
}
}
async fn parse_skill_file(
fs: &dyn ExecutorFileSystem,
path: &AbsolutePathBuf,
skill_path: &EnvironmentPathRef,
scope: SkillScope,
plugin_id: Option<&str>,
plugin_root: Option<&AbsolutePathBuf>,
plugin_root: Option<&EnvironmentPathRef>,
) -> Result<SkillMetadata, SkillParseError> {
let contents = fs
.read_file_text(path, /*sandbox*/ None)
let contents = skill_path
.read_to_string(/*sandbox*/ None)
.await
.map_err(SkillParseError::Read)?;
let path = skill_path.path();
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
@@ -641,7 +633,7 @@ async fn parse_skill_file(
.map(sanitize_single_line)
.filter(|value| !value.is_empty())
.unwrap_or_else(|| default_skill_name(path));
let name = namespaced_skill_name(fs, path, &base_name).await;
let name = namespaced_skill_name(skill_path.file_system().as_ref(), path, &base_name).await;
let description = parsed
.description
.as_deref()
@@ -657,7 +649,7 @@ async fn parse_skill_file(
interface,
dependencies,
policy,
} = load_skill_metadata(fs, path, plugin_root).await;
} = load_skill_metadata(skill_path, plugin_root).await;
validate_len(&name, MAX_NAME_LEN, "name")?;
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
@@ -708,18 +700,21 @@ async fn namespaced_skill_name(
}
async fn load_skill_metadata(
fs: &dyn ExecutorFileSystem,
skill_path: &AbsolutePathBuf,
plugin_root: Option<&AbsolutePathBuf>,
skill_path: &EnvironmentPathRef,
plugin_root: Option<&EnvironmentPathRef>,
) -> LoadedSkillMetadata {
// Fail open: optional metadata should not block loading SKILL.md.
let Some(skill_dir) = skill_path.parent() else {
let Some(skill_dir) = skill_path.parent_dir() else {
return LoadedSkillMetadata::default();
};
let metadata_path = skill_dir
.join(SKILLS_METADATA_DIR)
.join(SKILLS_METADATA_FILENAME);
match fs.get_metadata(&metadata_path, /*sandbox*/ None).await {
let Some(metadata_path) = skill_dir.join_relative(
Path::new(SKILLS_METADATA_DIR)
.join(SKILLS_METADATA_FILENAME)
.as_path(),
) else {
return LoadedSkillMetadata::default();
};
match metadata_path.metadata(/*sandbox*/ None).await {
Ok(metadata) if metadata.is_file => {}
Ok(_) => return LoadedSkillMetadata::default(),
Err(error) if error.kind() == io::ErrorKind::NotFound => {
@@ -728,19 +723,19 @@ async fn load_skill_metadata(
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to stat {label}: {error}",
path = metadata_path.display(),
path = metadata_path.path().display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
}
}
let contents = match fs.read_file_text(&metadata_path, /*sandbox*/ None).await {
let contents = match metadata_path.read_to_string(/*sandbox*/ None).await {
Ok(contents) => contents,
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to read {label}: {error}",
path = metadata_path.display(),
path = metadata_path.path().display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
@@ -748,13 +743,13 @@ async fn load_skill_metadata(
};
let parsed: SkillMetadataFile = {
let _guard = AbsolutePathBufGuard::new(skill_dir.as_path());
let _guard = AbsolutePathBufGuard::new(skill_dir.path().as_path());
match serde_yaml::from_str(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"ignoring {path}: invalid {label}: {error}",
path = metadata_path.display(),
path = metadata_path.path().display(),
label = SKILLS_METADATA_FILENAME
);
return LoadedSkillMetadata::default();
@@ -768,7 +763,11 @@ async fn load_skill_metadata(
policy,
} = parsed;
LoadedSkillMetadata {
interface: resolve_interface(interface, &skill_dir, plugin_root),
interface: resolve_interface(
interface,
skill_dir.path(),
plugin_root.map(EnvironmentPathRef::path),
),
dependencies: resolve_dependencies(dependencies),
policy: resolve_policy(policy),
}
@@ -1061,15 +1060,13 @@ pub(crate) async fn skill_roots_from_layer_stack(
cwd: &AbsolutePathBuf,
home_dir: Option<&AbsolutePathBuf>,
) -> Vec<SkillRoot> {
skill_roots_with_home_dir(
Some(fs),
config_layer_stack,
cwd,
home_dir,
Vec::new(),
Vec::new(),
)
.await
let path_ref = EnvironmentPathRef::new(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
fs,
cwd.clone(),
);
skill_roots_with_home_dir(&path_ref, config_layer_stack, home_dir, Vec::new(), Vec::new())
.await
}
#[cfg(test)]
codex-rs/core-skills/src/loader_tests.rs
+42 -50
View File
@@ -4,6 +4,7 @@ use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirements;
use codex_config::ConfigRequirementsToml;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -144,6 +145,23 @@ fn normalized(path: &Path) -> AbsolutePathBuf {
.abs()
}
fn env_path(path: AbsolutePathBuf) -> EnvironmentPathRef {
EnvironmentPathRef::new(
LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&LOCAL_FS),
path,
)
}
fn local_skill_root(path: AbsolutePathBuf, scope: SkillScope) -> SkillRoot {
SkillRoot {
path: env_path(path),
scope,
plugin_id: None,
plugin_root: None,
}
}
#[tokio::test]
async fn skill_roots_from_layer_stack_maps_user_to_user_and_system_cache_and_system_to_admin()
-> anyhow::Result<()> {
@@ -187,7 +205,7 @@ async fn skill_roots_from_layer_stack_maps_user_to_user_and_system_cache_and_sys
)
.await
.into_iter()
.map(|root| (root.scope, root.path.to_path_buf()))
.map(|root| (root.scope, root.path.path().to_path_buf()))
.collect::<Vec<_>>();
assert_eq!(
@@ -256,7 +274,7 @@ async fn skill_roots_from_layer_stack_includes_disabled_project_layers() -> anyh
)
.await
.into_iter()
.map(|root| (root.scope, root.path.to_path_buf()))
.map(|root| (root.scope, root.path.path().to_path_buf()))
.collect::<Vec<_>>();
assert_eq!(
@@ -846,11 +864,10 @@ interface:
let plugin_root_abs = plugin_root.abs();
let outcome = load_skills_from_roots([SkillRoot {
path: plugin_root.join("skills").abs(),
path: env_path(plugin_root.join("skills").abs()),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_root: Some(plugin_root_abs.clone()),
plugin_root: Some(env_path(plugin_root_abs.clone())),
}])
.await;
@@ -903,11 +920,10 @@ interface:
);
let outcome = load_skills_from_roots([SkillRoot {
path: plugin_root.join("skills").abs(),
path: env_path(plugin_root.join("skills").abs()),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_root: Some(plugin_root.abs()),
plugin_root: Some(env_path(plugin_root.abs())),
}])
.await;
@@ -1048,14 +1064,9 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() {
fs::create_dir_all(admin_root.path()).unwrap();
symlink_dir(shared.path(), &admin_root.path().join("shared"));
let outcome = load_skills_from_roots([SkillRoot {
path: admin_root.path().abs(),
scope: SkillScope::Admin,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}])
.await;
let outcome =
load_skills_from_roots([local_skill_root(admin_root.path().abs(), SkillScope::Admin)])
.await;
assert!(
outcome.errors.is_empty(),
@@ -1130,14 +1141,8 @@ async fn system_scope_ignores_symlinked_subdir() {
fs::create_dir_all(&system_root).unwrap();
symlink_dir(shared.path(), &system_root.join("shared"));
let outcome = load_skills_from_roots([SkillRoot {
path: system_root.abs(),
scope: SkillScope::System,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}])
.await;
let outcome =
load_skills_from_roots([local_skill_root(system_root.abs(), SkillScope::System)]).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
@@ -1164,14 +1169,8 @@ async fn respects_max_scan_depth_for_user_scope() {
);
let skills_root = codex_home.path().join("skills");
let outcome = load_skills_from_roots([SkillRoot {
path: skills_root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
}])
.await;
let outcome =
load_skills_from_roots([local_skill_root(skills_root.abs(), SkillScope::User)]).await;
assert!(
outcome.errors.is_empty(),
@@ -1272,11 +1271,10 @@ async fn namespaces_plugin_skills_using_plugin_name() {
.unwrap();
let outcome = load_skills_from_roots([SkillRoot {
path: plugin_root.join("skills").abs(),
path: env_path(plugin_root.join("skills").abs()),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_root: Some(plugin_root.abs()),
plugin_root: Some(env_path(plugin_root.abs())),
}])
.await;
@@ -1595,16 +1593,14 @@ async fn deduplicates_by_path_preferring_first_root() {
let outcome = load_skills_from_roots([
SkillRoot {
path: root.path().abs(),
path: env_path(root.path().abs()),
scope: SkillScope::Repo,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
},
SkillRoot {
path: root.path().abs(),
path: env_path(root.path().abs()),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_root: None,
},
@@ -1895,17 +1891,13 @@ async fn skill_roots_include_admin_with_lowest_priority() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cfg = make_config(&codex_home).await;
let scopes: Vec<SkillScope> = super::skill_roots(
Some(Arc::clone(&LOCAL_FS)),
&cfg.config_layer_stack,
&cfg.cwd,
Vec::new(),
Vec::new(),
)
.await
.into_iter()
.map(|root| root.scope)
.collect();
let cwd = env_path(cfg.cwd.clone());
let scopes: Vec<SkillScope> =
super::skill_roots(&cwd, &cfg.config_layer_stack, Vec::new(), Vec::new())
.await
.into_iter()
.map(|root| root.scope)
.collect();
let mut expected = vec![SkillScope::User, SkillScope::System];
if home_dir().is_some() {
expected.insert(1, SkillScope::User);
codex-rs/core-skills/src/manager.rs
+30 -41
View File
@@ -4,7 +4,6 @@ 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;
@@ -23,10 +22,11 @@ use crate::loader::skill_roots;
use crate::system::install_system_skills;
use crate::system::uninstall_system_skills;
use codex_config::SkillsConfig;
use codex_exec_server::EnvironmentPathRef;
#[derive(Debug, Clone)]
pub struct SkillsLoadInput {
pub cwd: AbsolutePathBuf,
pub path_ref: EnvironmentPathRef,
pub effective_skill_roots: Vec<PluginSkillRoot>,
pub config_layer_stack: ConfigLayerStack,
pub bundled_skills_enabled: bool,
@@ -34,13 +34,13 @@ pub struct SkillsLoadInput {
impl SkillsLoadInput {
pub fn new(
cwd: AbsolutePathBuf,
path_ref: EnvironmentPathRef,
effective_skill_roots: Vec<PluginSkillRoot>,
config_layer_stack: ConfigLayerStack,
bundled_skills_enabled: bool,
) -> Self {
Self {
cwd,
path_ref,
effective_skill_roots,
config_layer_stack,
bundled_skills_enabled,
@@ -52,7 +52,7 @@ pub struct SkillsManager {
codex_home: AbsolutePathBuf,
restriction_product: Option<Product>,
extra_roots: RwLock<Vec<AbsolutePathBuf>>,
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>,
cache_by_path_ref: RwLock<HashMap<EnvironmentPathRef, SkillLoadOutcome>>,
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
}
@@ -70,7 +70,7 @@ impl SkillsManager {
codex_home,
restriction_product,
extra_roots: RwLock::new(Vec::new()),
cache_by_cwd: RwLock::new(HashMap::new()),
cache_by_path_ref: RwLock::new(HashMap::new()),
cache_by_config: RwLock::new(HashMap::new()),
};
if !bundled_skills_enabled {
@@ -100,12 +100,8 @@ impl SkillsManager {
/// 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;
pub async fn skills_for_config(&self, input: &SkillsLoadInput) -> SkillLoadOutcome {
let roots = self.skill_roots_for_config(input).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) {
@@ -121,15 +117,10 @@ impl SkillsManager {
outcome
}
pub async fn skill_roots_for_config(
&self,
input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> Vec<SkillRoot> {
pub async fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec<SkillRoot> {
let mut roots = skill_roots(
fs,
&input.path_ref,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
self.extra_roots(),
)
@@ -144,20 +135,17 @@ impl SkillsManager {
&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)
let path_ref_cache_key = input.path_ref.clone();
if !force_reload
&& let Some(outcome) = self.cached_outcome_for_path_ref(&path_ref_cache_key)
{
return outcome;
}
let mut roots = skill_roots(
fs.clone(),
&input.path_ref,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
self.extra_roots(),
)
@@ -167,13 +155,11 @@ impl SkillsManager {
}
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());
}
let mut cache = self
.cache_by_path_ref
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(path_ref_cache_key, outcome.clone());
outcome
}
@@ -191,9 +177,9 @@ impl SkillsManager {
}
pub fn clear_cache(&self) {
let cleared_cwd = {
let cleared_path_refs = {
let mut cache = self
.cache_by_cwd
.cache_by_path_ref
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let cleared = cache.len();
@@ -209,14 +195,17 @@ impl SkillsManager {
cache.clear();
cleared
};
let cleared = cleared_cwd + cleared_config;
let cleared = cleared_path_refs + 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_path_ref(
&self,
path_ref: &EnvironmentPathRef,
) -> Option<SkillLoadOutcome> {
match self.cache_by_path_ref.read() {
Ok(cache) => cache.get(path_ref).cloned(),
Err(err) => err.into_inner().get(path_ref).cloned(),
}
}
@@ -240,7 +229,7 @@ impl SkillsManager {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<(AbsolutePathBuf, u8, Option<String>)>,
roots: Vec<(EnvironmentPathRef, u8, Option<String>)>,
skill_config_rules: SkillConfigRules,
}
codex-rs/core-skills/src/manager_tests.rs
+33 -107
View File
@@ -7,6 +7,7 @@ use codex_config::CONFIG_TOML_FILE;
use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirementsToml;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
@@ -87,6 +88,28 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
}
}
fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
EnvironmentPathRef::new(
LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&LOCAL_FS),
path,
)
}
fn local_skills_input(
cwd: AbsolutePathBuf,
effective_skill_roots: Vec<PluginSkillRoot>,
config_layer_stack: ConfigLayerStack,
) -> SkillsLoadInput {
let path_ref = skill_root_path_ref(cwd);
SkillsLoadInput::new(
path_ref,
effective_skill_roots,
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
)
}
fn write_demo_skill(tempdir: &TempDir) -> PathBuf {
let skill_path = tempdir.path().join("skills").join("demo").join("SKILL.md");
fs::create_dir_all(skill_path.parent().expect("skill path should have parent"))
@@ -164,15 +187,12 @@ async fn skills_for_config_with_stack(
config_layer_stack: &ConfigLayerStack,
effective_skill_roots: &[PluginSkillRoot],
) -> SkillLoadOutcome {
let skills_input = SkillsLoadInput::new(
let skills_input = local_skills_input(
cwd.path().abs(),
effective_skill_roots.to_vec(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(config_layer_stack),
);
skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS)))
.await
skills_manager.skills_for_config(&skills_input).await
}
#[test]
@@ -421,23 +441,14 @@ 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(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone());
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let outcome = skills_manager
.skills_for_cwd(
&skills_input,
/*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)),
)
.skills_for_cwd(&skills_input, /*force_reload*/ true)
.await;
assert!(
@@ -454,65 +465,6 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
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 = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let skills_manager = SkillsManager::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let outcome = skills_manager
.skills_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None)
.await;
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");
@@ -565,18 +517,9 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
/*bundled_skills_enabled*/ true,
);
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
let base_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let base_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone());
let outcome_a = skills_manager
.skills_for_cwd(
&base_input,
/*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)),
)
.skills_for_cwd(&base_input, /*force_reload*/ false)
.await;
assert!(
outcome_a
@@ -588,11 +531,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
write_user_skill(&codex_home, "late", "late-skill", "added after cache");
let outcome_b = skills_manager
.skills_for_cwd(
&base_input,
/*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)),
)
.skills_for_cwd(&base_input, /*force_reload*/ false)
.await;
assert!(
outcome_b
@@ -602,11 +541,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
);
let outcome_reloaded = skills_manager
.skills_for_cwd(
&base_input,
/*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)),
)
.skills_for_cwd(&base_input, /*force_reload*/ true)
.await;
assert!(
outcome_reloaded
@@ -779,19 +714,10 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
let parent_input = SkillsLoadInput::new(
cwd.path().abs(),
Vec::new(),
parent_stack.clone(),
bundled_skills_enabled_from_stack(&parent_stack),
);
let parent_input = local_skills_input(cwd.path().abs(), Vec::new(), parent_stack.clone());
let parent_outcome = skills_manager
.skills_for_cwd(
&parent_input,
/*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)),
)
.skills_for_cwd(&parent_input, /*force_reload*/ true)
.await;
let parent_skill = parent_outcome
.skills
codex-rs/core-skills/src/model.rs
+31 -5
View File
@@ -1,12 +1,11 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::sync::Arc;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::sync::Arc;
#[derive(Debug, Clone, PartialEq)]
pub struct SkillMetadata {
@@ -210,3 +209,30 @@ pub fn filter_skill_load_outcome_for_product(
);
outcome
}
#[cfg(test)]
mod tests {
use std::path::Path;
use codex_exec_server::EnvironmentPathRef;
use codex_utils_absolute_path::test_support::PathBufExt;
use pretty_assertions::assert_eq;
#[test]
fn environment_path_ref_joins_only_relative_paths() {
let root_path = std::env::temp_dir().join("skills");
let path_ref = EnvironmentPathRef::local(root_path.abs());
assert_eq!(
path_ref
.join_relative(Path::new("demo/SKILL.md"))
.map(|path_ref| path_ref.path().clone()),
Some(root_path.join("demo/SKILL.md").abs())
);
assert!(
path_ref
.join_relative(std::env::temp_dir().join("SKILL.md").as_path())
.is_none()
);
}
}
codex-rs/core/src/agent/role_tests.rs
+7 -7
View File
@@ -421,13 +421,13 @@ enabled = false
let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
let outcome = skills_manager
.skills_for_config(
&skills_input,
Some(Arc::clone(&codex_exec_server::LOCAL_FS)),
)
.await;
let cwd = crate::environment_path(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&codex_exec_server::LOCAL_FS),
config.cwd.clone(),
);
let skills_input = skills_load_input_from_config(&config, cwd, effective_skill_roots);
let outcome = skills_manager.skills_for_config(&skills_input).await;
let skill = outcome
.skills
.iter()
codex-rs/core/src/environment_selection.rs
-6
View File
@@ -2,7 +2,6 @@ use std::collections::HashSet;
use std::sync::Arc;
use codex_exec_server::EnvironmentManager;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::error::CodexErr;
use codex_protocol::error::Result as CodexResult;
use codex_protocol::protocol::TurnEnvironmentSelection;
@@ -45,11 +44,6 @@ impl ResolvedTurnEnvironments {
self.primary()
.map(|environment| Arc::clone(&environment.environment))
}
pub(crate) fn primary_filesystem(&self) -> Option<Arc<dyn ExecutorFileSystem>> {
self.primary()
.map(|environment| environment.environment.get_filesystem())
}
}
pub(crate) fn resolve_environment_selections(
codex-rs/core/src/lib.rs
+1
View File
@@ -92,6 +92,7 @@ pub(crate) use skills::build_skill_injections;
pub(crate) use skills::build_skill_name_counts;
pub(crate) use skills::collect_explicit_skill_mentions;
pub(crate) use skills::default_skill_metadata_budget;
pub(crate) use skills::environment_path_ref;
pub(crate) use skills::injection;
pub(crate) use skills::manager;
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
codex-rs/core/src/session/session.rs
+19 -8
View File
@@ -1,6 +1,7 @@
use super::input_queue::InputQueue;
use super::*;
use crate::config::ConstraintError;
use crate::environment_path;
use crate::goals::GoalRuntimeState;
use crate::skills::SkillError;
use crate::state::ActiveTurn;
@@ -447,20 +448,30 @@ async fn warm_plugins_and_skills_for_session_init(
skills_manager: Arc<SkillsManager>,
environments: Vec<TurnEnvironmentSelection>,
) -> Vec<SkillError> {
let fs = crate::environment_selection::resolve_environment_selections(
let resolved_environments = crate::environment_selection::resolve_environment_selections(
environment_manager.as_ref(),
&environments,
)
.ok()
.and_then(|resolved| resolved.primary_filesystem());
.ok();
let path_ref = resolved_environments
.as_ref()
.and_then(crate::environment_selection::ResolvedTurnEnvironments::primary)
.map(|environment| {
environment_path(
environment.environment_id.clone(),
environment.environment.get_filesystem(),
environment.cwd.clone(),
)
});
let Some(path_ref) = path_ref else {
return Vec::new();
};
let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots);
skills_manager
.skills_for_config(&skills_input, fs)
.await
.errors
let skills_input =
skills_load_input_from_config(config.as_ref(), path_ref, effective_skill_roots);
skills_manager.skills_for_config(&skills_input).await.errors
}
impl Session {
codex-rs/core/src/session/tests.rs
+23 -8
View File
@@ -4127,9 +4127,16 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
.services
.skills_manager
.skills_for_cwd(
&crate::skills_load_input_from_config(&parent_config, Vec::new()),
&crate::skills_load_input_from_config(
&parent_config,
crate::environment_path(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&skill_fs),
parent_config.cwd.clone(),
),
Vec::new(),
),
/*force_reload*/ true,
Some(Arc::clone(&skill_fs)),
)
.await;
let parent_skill = parent_outcome
@@ -4692,13 +4699,17 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
.plugins_for_config(&per_turn_config.plugins_config_input())
.await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let cwd = crate::environment_path(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
environment.get_filesystem(),
per_turn_config.cwd.clone(),
);
let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skill_fs = environment.get_filesystem();
crate::skills_load_input_from_config(&per_turn_config, cwd, effective_skill_roots);
let skills_outcome = Arc::new(
services
.skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.skills_for_config(&skills_input)
.await,
);
let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd);
@@ -6535,13 +6546,17 @@ where
.plugins_for_config(&per_turn_config.plugins_config_input())
.await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let cwd = crate::environment_path(
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
environment.get_filesystem(),
per_turn_config.cwd.clone(),
);
let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skill_fs = environment.get_filesystem();
crate::skills_load_input_from_config(&per_turn_config, cwd, effective_skill_roots);
let skills_outcome = Arc::new(
services
.skills_manager
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.skills_for_config(&skills_input)
.await,
);
let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd);
codex-rs/core/src/session/turn_context.rs
+27 -15
View File
@@ -1,6 +1,7 @@
use super::*;
use crate::SkillLoadOutcome;
use crate::config::GhostSnapshotConfig;
use crate::environment_path;
use crate::environment_selection::ResolvedTurnEnvironments;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
@@ -678,21 +679,32 @@ impl Session {
&per_turn_config.to_models_manager_config(),
)
.await;
let plugin_outcome = self
.services
.plugins_manager
.plugins_for_config(&per_turn_config.plugins_config_input())
.await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let fs = primary_turn_environment
.map(|turn_environment| turn_environment.environment.get_filesystem());
let skills_outcome = Arc::new(
self.services
.skills_manager
.skills_for_config(&skills_input, fs)
.await,
);
let path_ref = primary_turn_environment.map(|turn_environment| {
environment_path(
turn_environment.environment_id.clone(),
turn_environment.environment.get_filesystem(),
turn_environment.cwd.clone(),
)
});
let skills_outcome = if let Some(path_ref) = path_ref {
let plugins_input = per_turn_config.plugins_config_input();
let plugin_outcome = self
.services
.plugins_manager
.plugins_for_config(&plugins_input)
.await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input =
skills_load_input_from_config(&per_turn_config, path_ref, effective_skill_roots);
Arc::new(
self.services
.skills_manager
.skills_for_config(&skills_input)
.await,
)
} else {
Arc::new(SkillLoadOutcome::default())
};
let goal_tools_supported = !per_turn_config.ephemeral && self.state_db().is_some();
let mut turn_context: TurnContext = Self::make_turn_context(
self.thread_id(),
codex-rs/core/src/skills.rs
+13 -1
View File
@@ -4,9 +4,12 @@ use crate::session::turn_context::TurnContext;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::build_track_events_context;
pub use codex_exec_server::EnvironmentPathRef;
use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginSkillRoot;
use std::sync::Arc;
pub use codex_core_skills::SkillError;
pub use codex_core_skills::SkillLoadOutcome;
@@ -35,16 +38,25 @@ pub use codex_core_skills::system;
pub(crate) fn skills_load_input_from_config(
config: &Config,
path_ref: EnvironmentPathRef,
effective_skill_roots: Vec<PluginSkillRoot>,
) -> SkillsLoadInput {
SkillsLoadInput::new(
config.cwd.clone(),
path_ref,
effective_skill_roots,
config.config_layer_stack.clone(),
config.bundled_skills_enabled(),
)
}
pub(crate) fn environment_path(
environment_id: String,
file_system: Arc<dyn ExecutorFileSystem>,
path: AbsolutePathBuf,
) -> EnvironmentPathRef {
EnvironmentPathRef::new(environment_id, file_system, path)
}
pub(crate) async fn maybe_emit_implicit_skill_invocation(
sess: &Session,
turn_context: &TurnContext,
codex-rs/exec-server/src/environment_path.rs
+355
View File
@@ -0,0 +1,355 @@
use std::fmt;
use std::hash::Hash;
use std::hash::Hasher;
use std::io;
use std::path::Path;
use std::sync::Arc;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::ExecutorFileSystem;
use crate::FileMetadata;
use crate::FileSystemSandboxContext;
use crate::LOCAL_ENVIRONMENT_ID;
use crate::LOCAL_FS;
use crate::ReadDirectoryEntry;
/// Binds an absolute path to the executor filesystem and environment that owns it.
#[derive(Clone)]
pub struct EnvironmentPathRef {
environment_id: String,
file_system: Arc<dyn ExecutorFileSystem>,
path: AbsolutePathBuf,
}
impl EnvironmentPathRef {
pub fn new(
environment_id: String,
file_system: Arc<dyn ExecutorFileSystem>,
path: AbsolutePathBuf,
) -> Self {
Self {
environment_id,
file_system,
path,
}
}
pub fn local(path: AbsolutePathBuf) -> Self {
Self::new(
LOCAL_ENVIRONMENT_ID.to_string(),
Arc::clone(&LOCAL_FS),
path,
)
}
pub fn path(&self) -> &AbsolutePathBuf {
&self.path
}
pub fn environment_id(&self) -> &str {
&self.environment_id
}
pub fn file_system(&self) -> Arc<dyn ExecutorFileSystem> {
Arc::clone(&self.file_system)
}
pub async fn read_to_string(
&self,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<String> {
self.file_system.read_file_text(&self.path, sandbox).await
}
pub async fn metadata(
&self,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<FileMetadata> {
self.file_system.get_metadata(&self.path, sandbox).await
}
pub async fn read_directory(
&self,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<Vec<ReadDirectoryEntry>> {
self.file_system.read_directory(&self.path, sandbox).await
}
pub fn join_relative(&self, relative: &Path) -> Option<Self> {
relative
.is_relative()
.then(|| self.with_path(self.path.join(relative)))
}
pub fn parent_dir(&self) -> Option<Self> {
self.path.parent().map(|path| self.with_path(path))
}
pub fn with_path(&self, path: AbsolutePathBuf) -> Self {
Self::new(
self.environment_id.clone(),
Arc::clone(&self.file_system),
path,
)
}
}
impl PartialEq for EnvironmentPathRef {
fn eq(&self, other: &Self) -> bool {
self.environment_id == other.environment_id
&& Arc::ptr_eq(&self.file_system, &other.file_system)
&& self.path == other.path
}
}
impl Eq for EnvironmentPathRef {}
impl Hash for EnvironmentPathRef {
fn hash<H: Hasher>(&self, state: &mut H) {
self.environment_id.hash(state);
(Arc::as_ptr(&self.file_system) as *const () as usize).hash(state);
self.path.hash(state);
}
}
impl fmt::Debug for EnvironmentPathRef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("EnvironmentPathRef")
.field("environment_id", &self.environment_id)
.field("path", &self.path)
.finish()
}
}
#[cfg(test)]
mod tests {
use super::*;
use async_trait::async_trait;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_utils_absolute_path::test_support::PathBufExt;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::sync::Mutex;
#[derive(Clone, Debug, Eq, PartialEq)]
enum RecordedMethod {
ReadFileText,
Metadata,
ReadDirectory,
}
#[derive(Clone, Debug, Eq, PartialEq)]
struct RecordedCall {
method: RecordedMethod,
path: AbsolutePathBuf,
sandbox: Option<FileSystemSandboxContext>,
}
#[derive(Default)]
struct RecordingFileSystem {
calls: Mutex<Vec<RecordedCall>>,
}
impl RecordingFileSystem {
fn recorded_calls(&self) -> Vec<RecordedCall> {
match self.calls.lock() {
Ok(calls) => calls.clone(),
Err(err) => err.into_inner().clone(),
}
}
fn push_call(&self, call: RecordedCall) {
match self.calls.lock() {
Ok(mut calls) => calls.push(call),
Err(err) => err.into_inner().push(call),
}
}
}
#[async_trait]
impl ExecutorFileSystem for RecordingFileSystem {
async fn read_file(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<Vec<u8>> {
self.push_call(RecordedCall {
method: RecordedMethod::ReadFileText,
path: path.clone(),
sandbox: sandbox.cloned(),
});
Ok(b"skill contents".to_vec())
}
async fn write_file(
&self,
_path: &AbsolutePathBuf,
_contents: Vec<u8>,
_sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<()> {
unreachable!("write_file should not be called")
}
async fn create_directory(
&self,
_path: &AbsolutePathBuf,
_create_directory_options: crate::CreateDirectoryOptions,
_sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<()> {
unreachable!("create_directory should not be called")
}
async fn get_metadata(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<FileMetadata> {
self.push_call(RecordedCall {
method: RecordedMethod::Metadata,
path: path.clone(),
sandbox: sandbox.cloned(),
});
Ok(FileMetadata {
is_directory: true,
is_file: false,
is_symlink: false,
created_at_ms: 1,
modified_at_ms: 2,
})
}
async fn read_directory(
&self,
path: &AbsolutePathBuf,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<Vec<ReadDirectoryEntry>> {
self.push_call(RecordedCall {
method: RecordedMethod::ReadDirectory,
path: path.clone(),
sandbox: sandbox.cloned(),
});
Ok(vec![ReadDirectoryEntry {
file_name: "SKILL.md".to_string(),
is_directory: false,
is_file: true,
}])
}
async fn remove(
&self,
_path: &AbsolutePathBuf,
_remove_options: crate::RemoveOptions,
_sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<()> {
unreachable!("remove should not be called")
}
async fn copy(
&self,
_source_path: &AbsolutePathBuf,
_destination_path: &AbsolutePathBuf,
_copy_options: crate::CopyOptions,
_sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<()> {
unreachable!("copy should not be called")
}
}
fn restricted_sandbox() -> FileSystemSandboxContext {
FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(Vec::new()),
NetworkSandboxPolicy::Restricted,
),
)
}
#[tokio::test]
async fn environment_path_ref_forwards_sandbox_to_file_system_methods() {
let path = std::env::temp_dir().join("skills/demo").abs();
let file_system = Arc::new(RecordingFileSystem::default());
let path_ref =
EnvironmentPathRef::new("local".to_string(), file_system.clone(), path.clone());
let sandbox = restricted_sandbox();
assert_eq!(
path_ref
.read_to_string(Some(&sandbox))
.await
.expect("read skill contents"),
"skill contents".to_string()
);
assert_eq!(
path_ref
.metadata(Some(&sandbox))
.await
.expect("read metadata"),
FileMetadata {
is_directory: true,
is_file: false,
is_symlink: false,
created_at_ms: 1,
modified_at_ms: 2,
}
);
assert_eq!(
path_ref
.read_directory(Some(&sandbox))
.await
.expect("read directory"),
vec![ReadDirectoryEntry {
file_name: "SKILL.md".to_string(),
is_directory: false,
is_file: true,
}]
);
assert_eq!(
file_system.recorded_calls(),
vec![
RecordedCall {
method: RecordedMethod::ReadFileText,
path: path.clone(),
sandbox: Some(sandbox.clone()),
},
RecordedCall {
method: RecordedMethod::Metadata,
path: path.clone(),
sandbox: Some(sandbox.clone()),
},
RecordedCall {
method: RecordedMethod::ReadDirectory,
path,
sandbox: Some(sandbox),
},
]
);
}
#[test]
fn environment_path_ref_equality_and_hash_include_environment_id_and_file_system_identity() {
let path = std::env::temp_dir().join("skills/demo").abs();
let file_system = Arc::new(RecordingFileSystem::default());
let same_file_system: Arc<dyn ExecutorFileSystem> = file_system.clone();
let different_file_system: Arc<dyn ExecutorFileSystem> =
Arc::new(RecordingFileSystem::default());
let left =
EnvironmentPathRef::new("local".to_string(), same_file_system.clone(), path.clone());
let same = EnvironmentPathRef::new("local".to_string(), same_file_system, path.clone());
let different_env =
EnvironmentPathRef::new("remote".to_string(), file_system.clone(), path.clone());
let different_fs =
EnvironmentPathRef::new("local".to_string(), different_file_system, path);
assert_eq!(left, same);
assert_ne!(left, different_env);
assert_ne!(left, different_fs);
let set = HashSet::from([left, same, different_env, different_fs]);
assert_eq!(set.len(), 3);
}
}
codex-rs/exec-server/src/lib.rs
+2
View File
@@ -3,6 +3,7 @@ mod client_api;
mod client_transport;
mod connection;
mod environment;
mod environment_path;
mod environment_provider;
mod environment_toml;
mod fs_helper;
@@ -43,6 +44,7 @@ pub use environment::Environment;
pub use environment::EnvironmentManager;
pub use environment::LOCAL_ENVIRONMENT_ID;
pub use environment::REMOTE_ENVIRONMENT_ID;
pub use environment_path::EnvironmentPathRef;
pub use environment_provider::DefaultEnvironmentProvider;
pub use environment_provider::EnvironmentProvider;
pub use fs_helper::CODEX_FS_HELPER_ARG1;