mirror of
https://github.com/openai/codex.git
synced 2026-02-01 14:44:17 +00:00
feat: Support loading skills from .agents/skills (#10317)
This PR adds support for loading [skills](https://developers.openai.com/codex/skills) from `.agents/skills/`. - Issue: https://github.com/agentskills/agentskills/issues/15 - Motivation: When skills live on the filesystem, sharing them across agents is awkward and often ends up requiring symlinks/duplication. A single location under `.agents/` makes it easier to share skills. - Loading from `.codex/skills/` will remain but will be deprecated soon. The change only applies to the [REPO scope](https://developers.openai.com/codex/skills#where-to-save-skills). - Documentation will be updated before this change is live. Testing with skills in two locations of this repo: <img width="960" height="152" alt="image" src="https://github.com/user-attachments/assets/28975ff9-7363-46dd-ad40-f4c7bfdb8234" /> When starting Codex with CWD in `$repo_root` (should only pick up at root): <img width="513" height="143" alt="image" src="https://github.com/user-attachments/assets/389e1ea7-020c-481e-bda0-ce58562db59f" /> When starting Codex with CWD in `$repo_root/codex-rs` (should pick up at cwd and crawl up to root): <img width="552" height="177" alt="image" src="https://github.com/user-attachments/assets/a5beb8de-11b4-45ed-8660-80707c77006a" />
This commit is contained in:
@@ -425,7 +425,9 @@ async fn load_requirements_from_legacy_scheme(
|
||||
/// empty array, which indicates that root detection should be disabled).
|
||||
/// - Returns an error if `project_root_markers` is specified but is not an
|
||||
/// array of strings.
|
||||
fn project_root_markers_from_config(config: &TomlValue) -> io::Result<Option<Vec<String>>> {
|
||||
pub(crate) fn project_root_markers_from_config(
|
||||
config: &TomlValue,
|
||||
) -> io::Result<Option<Vec<String>>> {
|
||||
let Some(table) = config.as_table() else {
|
||||
return Ok(None);
|
||||
};
|
||||
@@ -454,7 +456,7 @@ fn project_root_markers_from_config(config: &TomlValue) -> io::Result<Option<Vec
|
||||
Ok(Some(markers))
|
||||
}
|
||||
|
||||
fn default_project_root_markers() -> Vec<String> {
|
||||
pub(crate) fn default_project_root_markers() -> Vec<String> {
|
||||
DEFAULT_PROJECT_ROOT_MARKERS
|
||||
.iter()
|
||||
.map(ToString::to_string)
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
use crate::config::Config;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigLayerStackOrdering;
|
||||
use crate::config_loader::default_project_root_markers;
|
||||
use crate::config_loader::merge_toml_values;
|
||||
use crate::config_loader::project_root_markers_from_config;
|
||||
use crate::skills::model::SkillDependencies;
|
||||
use crate::skills::model::SkillError;
|
||||
use crate::skills::model::SkillInterface;
|
||||
@@ -20,6 +23,7 @@ use std::fs;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
use tracing::error;
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
@@ -72,6 +76,7 @@ struct DependencyTool {
|
||||
}
|
||||
|
||||
const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
const AGENTS_DIR_NAME: &str = ".agents";
|
||||
const SKILLS_METADATA_DIR: &str = "agents";
|
||||
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
|
||||
const SKILLS_DIR_NAME: &str = "skills";
|
||||
@@ -209,15 +214,104 @@ fn skill_roots_from_layer_stack_inner(config_layer_stack: &ConfigLayerStack) ->
|
||||
}
|
||||
|
||||
fn skill_roots(config: &Config) -> Vec<SkillRoot> {
|
||||
skill_roots_from_layer_stack_inner(&config.config_layer_stack)
|
||||
skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn skill_roots_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> Vec<SkillRoot> {
|
||||
skill_roots_from_layer_stack_inner(config_layer_stack)
|
||||
}
|
||||
|
||||
pub(crate) fn skill_roots_from_layer_stack_with_agents(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &Path,
|
||||
) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack);
|
||||
roots.extend(repo_agents_skill_roots(config_layer_stack, cwd));
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
roots
|
||||
}
|
||||
|
||||
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
|
||||
let mut seen: HashSet<PathBuf> = HashSet::new();
|
||||
roots.retain(|root| seen.insert(root.path.clone()));
|
||||
}
|
||||
|
||||
fn repo_agents_skill_roots(config_layer_stack: &ConfigLayerStack, cwd: &Path) -> Vec<SkillRoot> {
|
||||
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
|
||||
let project_root = find_project_root(cwd, &project_root_markers);
|
||||
let dirs = dirs_between_project_root_and_cwd(cwd, &project_root);
|
||||
let mut roots = Vec::new();
|
||||
for dir in dirs {
|
||||
let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
|
||||
if agents_skills.is_dir() {
|
||||
roots.push(SkillRoot {
|
||||
path: agents_skills,
|
||||
scope: SkillScope::Repo,
|
||||
});
|
||||
}
|
||||
}
|
||||
roots
|
||||
}
|
||||
|
||||
fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec<String> {
|
||||
let mut merged = TomlValue::Table(toml::map::Map::new());
|
||||
for layer in
|
||||
config_layer_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false)
|
||||
{
|
||||
if matches!(layer.name, ConfigLayerSource::Project { .. }) {
|
||||
continue;
|
||||
}
|
||||
merge_toml_values(&mut merged, &layer.config);
|
||||
}
|
||||
|
||||
match project_root_markers_from_config(&merged) {
|
||||
Ok(Some(markers)) => markers,
|
||||
Ok(None) => default_project_root_markers(),
|
||||
Err(err) => {
|
||||
tracing::warn!("invalid project_root_markers: {err}");
|
||||
default_project_root_markers()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn find_project_root(cwd: &Path, project_root_markers: &[String]) -> PathBuf {
|
||||
if project_root_markers.is_empty() {
|
||||
return cwd.to_path_buf();
|
||||
}
|
||||
|
||||
for ancestor in cwd.ancestors() {
|
||||
for marker in project_root_markers {
|
||||
let marker_path = ancestor.join(marker);
|
||||
if marker_path.exists() {
|
||||
return ancestor.to_path_buf();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
cwd.to_path_buf()
|
||||
}
|
||||
|
||||
fn dirs_between_project_root_and_cwd(cwd: &Path, project_root: &Path) -> Vec<PathBuf> {
|
||||
let mut dirs = cwd
|
||||
.ancestors()
|
||||
.scan(false, |done, a| {
|
||||
if *done {
|
||||
None
|
||||
} else {
|
||||
if a == project_root {
|
||||
*done = true;
|
||||
}
|
||||
Some(a.to_path_buf())
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
dirs.reverse();
|
||||
dirs
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) {
|
||||
let Ok(root) = canonicalize_path(root) else {
|
||||
return;
|
||||
@@ -1615,6 +1709,40 @@ interface:
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_skills_from_agents_dir_without_codex_dir() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let repo_dir = tempfile::tempdir().expect("tempdir");
|
||||
mark_as_git_repo(repo_dir.path());
|
||||
|
||||
let skill_path = write_skill_at(
|
||||
&repo_dir.path().join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
|
||||
"agents",
|
||||
"agents-skill",
|
||||
"from agents",
|
||||
);
|
||||
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
|
||||
|
||||
let outcome = load_skills(&cfg);
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "agents-skill".to_string(),
|
||||
description: "from agents".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_skills_from_all_codex_dirs_under_project_root() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -15,7 +15,7 @@ use crate::config_loader::LoaderOverrides;
|
||||
use crate::config_loader::load_config_layers_state;
|
||||
use crate::skills::SkillLoadOutcome;
|
||||
use crate::skills::loader::load_skills_from_roots;
|
||||
use crate::skills::loader::skill_roots_from_layer_stack;
|
||||
use crate::skills::loader::skill_roots_from_layer_stack_with_agents;
|
||||
use crate::skills::system::install_system_skills;
|
||||
|
||||
pub struct SkillsManager {
|
||||
@@ -47,7 +47,8 @@ impl SkillsManager {
|
||||
return outcome;
|
||||
}
|
||||
|
||||
let roots = skill_roots_from_layer_stack(&config.config_layer_stack);
|
||||
let roots =
|
||||
skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd);
|
||||
let mut outcome = load_skills_from_roots(roots);
|
||||
outcome.disabled_paths = disabled_paths_from_stack(&config.config_layer_stack);
|
||||
match self.cache_by_cwd.write() {
|
||||
@@ -105,7 +106,7 @@ impl SkillsManager {
|
||||
}
|
||||
};
|
||||
|
||||
let roots = skill_roots_from_layer_stack(&config_layer_stack);
|
||||
let roots = skill_roots_from_layer_stack_with_agents(&config_layer_stack, cwd);
|
||||
let mut outcome = load_skills_from_roots(roots);
|
||||
outcome.disabled_paths = disabled_paths_from_stack(&config_layer_stack);
|
||||
match self.cache_by_cwd.write() {
|
||||
|
||||
Reference in New Issue
Block a user