openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-08-23 13:09:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(skills): resolve plugin namespaces per root (#31348)

Browse Source 
## Why

Loading skills from a remote executor can add a lot to thread start time
when there are many skills. Previous changes added some concurrency for
the file reads themselves, but we're still bottlenecked on the initial
root path discovery.

Using the benchmark from
[#31295](https://github.com/openai/codex/pull/31295), this change
reduces the measured mean of loading 66 skills about 71%. Behavioral
coverage lands separately in
[#31369](https://github.com/openai/codex/pull/31369).

## What

- resolve the scanned root's inherited namespace once
- resolve discovered nested plugin roots once, retaining
nearest-valid-ancestor behavior
- pass an explicit resolved Plain / Plugin namespace into skill parsing
instead of probing per skill
- preserve explicitly provided plugin namespaces as the highest-priority
source
- reuse the same resolver for environment skills, deleting its duplicate
root-probe and ancestor-selection path

## Validation

- just test -p codex-core-skills namespace: 12 passed on both the parent
and optimized branches
This commit is contained in:
Adam Perry @ OpenAI
2026-07-07 16:00:15 -07:00
committed by GitHub
parent 8139255b46
commit 58b66d39e4
3 changed files with 173 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,4 +1,5 @@
mod environment;
mod namespace;
pub use environment::EnvironmentSkillLoadOutcome;
pub use environment::EnvironmentSkillMetadata;
@@ -27,9 +28,10 @@ use codex_utils_absolute_path::AbsolutePathBufGuard;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS;
use codex_utils_plugins::PluginSkillRoot;
use codex_utils_plugins::plugin_namespace_for_skill_path;
use dirs::home_dir;
use futures::future::join_all;
use namespace::ResolvedSkillNamespace;
use namespace::SkillNamespaceResolver;
use serde::Deserialize;
use std::collections::HashSet;
use std::collections::VecDeque;
@@ -680,12 +682,27 @@ async fn load_skills_under_root(
};
let SkillFileDiscovery {
skill_files,
plugin_roots,
namespace_roots,
warnings,
..
} = discover_skills_under_root(fs, &PathUri::from_abs_path(root), symlink_policy).await;
for warning in warnings {
error!("{warning}");
}
let root_uri = PathUri::from_abs_path(root);
let namespace_resolver = match plugin_namespace {
Some(namespace) => SkillNamespaceResolver::with_provided_namespace(namespace),
None => {
SkillNamespaceResolver::discover(
fs,
&root_uri,
&skill_files,
plugin_roots,
namespace_roots,
)
.await
}
};
for path_uri in skill_files {
let path = match path_uri.to_abs_path() {
Ok(path) => path,
@@ -699,7 +716,7 @@ async fn load_skills_under_root(
&path,
scope,
plugin_id,
plugin_namespace,
namespace_resolver.for_skill(&root_uri, &path_uri),
plugin_root.as_ref(),
)
.await
@@ -719,7 +736,7 @@ async fn parse_skill_file(
path: &AbsolutePathBuf,
scope: SkillScope,
plugin_id: Option<&str>,
plugin_namespace: Option<&str>,
namespace: &ResolvedSkillNamespace,
plugin_root: Option<&AbsolutePathBuf>,
) -> Result<SkillMetadata, SkillParseError> {
let path_uri = PathUri::from_abs_path(path);
@@ -732,7 +749,7 @@ async fn parse_skill_file(
description,
short_description,
} = parse_skill_frontmatter_metadata_inner(&contents, || default_skill_name(path))?;
let name = namespaced_skill_name(fs, path, &base_name, plugin_namespace).await;
let name = namespace.qualify(&base_name);
let LoadedSkillMetadata {
interface,
dependencies,
@@ -818,21 +835,6 @@ fn default_skill_name(path: &AbsolutePathBuf) -> String {
.unwrap_or_else(|| "skill".to_string())
}
async fn namespaced_skill_name(
fs: &dyn ExecutorFileSystem,
path: &AbsolutePathBuf,
base_name: &str,
plugin_namespace: Option<&str>,
) -> String {
if let Some(plugin_namespace) = plugin_namespace {
return format!("{plugin_namespace}:{base_name}");
}
plugin_namespace_for_skill_path(fs, path)
.await
.map(|namespace| format!("{namespace}:{base_name}"))
.unwrap_or_else(|| base_name.to_string())
}
async fn load_skill_metadata(
fs: &dyn ExecutorFileSystem,
skill_path: &AbsolutePathBuf,

74
codex-rs/core-skills/src/loader/environment.rs
View File

@@ -1,4 +1,3 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::io;
@@ -8,10 +7,7 @@ use codex_exec_server::WalkOptions;
use codex_protocol::protocol::Product;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS;
use codex_utils_plugins::plugin_namespace_for_root_uri;
use codex_utils_plugins::plugin_namespace_for_skill_uri;
use futures::StreamExt;
use futures::future::join_all;
use crate::model::SkillDependencies;
use crate::model::SkillPolicy;
@@ -24,6 +20,7 @@ use super::SKILLS_FILENAME;
use super::SKILLS_METADATA_DIR;
use super::SKILLS_METADATA_FILENAME;
use super::SkillMetadataFile;
use super::namespace::SkillNamespaceResolver;
use super::parse_skill_frontmatter_metadata_inner;
use super::resolve_dependencies;
use super::resolve_policy;
@@ -247,45 +244,18 @@ pub async fn load_environment_skills_from_root(
return outcome;
}
let mut skill_ancestors = HashSet::new();
for skill in &discovery.skills {
let mut ancestor = skill.path.parent();
while let Some(path) = ancestor {
skill_ancestors.insert(path.clone());
ancestor = path.parent();
}
}
let namespace_roots = discovery.namespace_roots;
let plugin_namespaces = async {
let namespace_lookups = join_all(namespace_roots.iter().map(|namespace_root| async {
(
namespace_root.clone(),
plugin_namespace_for_skill_uri(file_system, namespace_root).await,
)
}));
let plugin_lookups = join_all(
discovery
.plugin_roots
.iter()
.filter(|plugin_root| skill_ancestors.contains(*plugin_root))
.filter(|plugin_root| !namespace_roots.contains(*plugin_root))
.map(|plugin_root| async {
(
plugin_root.clone(),
plugin_namespace_for_root_uri(file_system, plugin_root).await,
)
}),
);
let (namespace_lookups, plugin_lookups) = tokio::join!(namespace_lookups, plugin_lookups);
namespace_lookups
.into_iter()
.chain(plugin_lookups)
.filter_map(|(plugin_root, namespace)| {
namespace.map(|namespace| (plugin_root, namespace))
})
.collect::<HashMap<_, _>>()
};
let skill_paths = discovery
.skills
.iter()
.map(|skill| skill.path.clone())
.collect::<Vec<_>>();
let namespace_resolver = SkillNamespaceResolver::discover(
file_system,
root,
&skill_paths,
discovery.plugin_roots,
discovery.namespace_roots,
);
// Remote executors can multiplex these independent per-skill reads, so polling a bounded
// number together allows the I/O for each skill and its metadata to happen concurrently.
@@ -301,23 +271,13 @@ pub async fn load_environment_skills_from_root(
})
.buffered(MAX_CONCURRENT_SKILL_LOADS)
.collect::<Vec<_>>();
let (plugin_namespaces, skill_results) = tokio::join!(plugin_namespaces, skill_results);
let (namespace_resolver, skill_results) = tokio::join!(namespace_resolver, skill_results);
for (path, result) in skill_results {
let result = result.and_then(|skill| {
let mut ancestor = skill.path_to_skills_md.parent();
let plugin_namespace = loop {
let Some(current) = ancestor else {
break None;
};
if let Some(namespace) = plugin_namespaces.get(&current) {
break Some(namespace.as_str());
}
ancestor = current.parent();
};
let name = plugin_namespace
.map(|namespace| format!("{namespace}:{}", skill.base_name))
.unwrap_or(skill.base_name);
let name = namespace_resolver
.for_skill(root, &skill.path_to_skills_md)
.qualify(&skill.base_name);
validate_len(&name, MAX_QUALIFIED_NAME_LEN, "qualified name")
.map_err(|err| err.to_string())?;

134
codex-rs/core-skills/src/loader/namespace.rs Normal file
View File

@@ -0,0 +1,134 @@
use codex_exec_server::ExecutorFileSystem;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::plugin_namespace_for_root_uri;
use codex_utils_plugins::plugin_namespace_for_skill_uri;
use futures::future::join_all;
use std::collections::HashSet;
/// Resolves the namespace prefix applied to skill names during one skills scan.
///
/// A plugin namespace is the plugin name from the nearest valid plugin manifest
/// above a skill path. For example, a skill named `search` beneath a plugin named
/// `sample` is exposed as `sample:search`.
///
/// Resolving the namespace separately for every `SKILL.md` repeats the same
/// ancestor manifest probes for sibling skills. This resolver resolves relevant
/// roots once per scan, then selects the nearest matching root for each skill.
///
/// Namespace precedence is:
///
/// 1. an explicitly provided plugin namespace;
/// 2. the deepest matching canonical symlink root or nested plugin root;
/// 3. the namespace inherited from the scanned skills root.
pub(crate) struct SkillNamespaceResolver {
inherited_namespace: ResolvedSkillNamespace,
nested_namespaces: Vec<(PathUri, ResolvedSkillNamespace)>,
}
impl SkillNamespaceResolver {
/// Builds a resolver whose explicit plugin-owned namespace overrides discovery.
pub(crate) fn with_provided_namespace(namespace: &str) -> Self {
Self {
inherited_namespace: ResolvedSkillNamespace::Plugin(namespace.to_string()),
nested_namespaces: Vec::new(),
}
}
pub(crate) async fn discover(
fs: &dyn ExecutorFileSystem,
root: &PathUri,
skill_paths: &[PathUri],
plugin_roots: HashSet<PathUri>,
namespace_roots: HashSet<PathUri>,
) -> Self {
// Only probe plugin roots above loaded skills; unused siblings cannot affect names.
let mut skill_ancestors = HashSet::new();
for skill_path in skill_paths {
let mut ancestor = skill_path.parent();
while let Some(path) = ancestor {
skill_ancestors.insert(path.clone());
ancestor = path.parent();
}
}
let plugin_roots = plugin_roots
.into_iter()
.filter(|plugin_root| skill_ancestors.contains(plugin_root))
.collect::<HashSet<_>>();
// Ordinary descendants fall back to the nearest valid manifest at or above the scan root.
let inherited_namespace = plugin_namespace_for_skill_uri(fs, root)
.await
.map(ResolvedSkillNamespace::Plugin)
.unwrap_or(ResolvedSkillNamespace::Plain);
// The scan root is already the fallback above if nothing else matches, exclude from the search.
let namespace_roots = namespace_roots
.into_iter()
.filter(|namespace_root| namespace_root != root)
.collect::<Vec<_>>();
let namespace_root_set = namespace_roots.iter().cloned().collect::<HashSet<_>>();
// Keep independent probes concurrent for remote executor latency.
let namespace_lookups = join_all(namespace_roots.into_iter().map(|namespace_root| async {
let namespace = plugin_namespace_for_skill_uri(fs, &namespace_root)
.await
.map(ResolvedSkillNamespace::Plugin)
.unwrap_or(ResolvedSkillNamespace::Plain);
(namespace_root, namespace)
}));
// Invalid nested manifests are omitted, so the deepest remaining match wins.
let plugin_lookups = join_all(
plugin_roots
.into_iter()
.filter(|plugin_root| {
plugin_root != root && !namespace_root_set.contains(plugin_root)
})
.map(|plugin_root| async move {
plugin_namespace_for_root_uri(fs, &plugin_root)
.await
.map(|namespace| (plugin_root, ResolvedSkillNamespace::Plugin(namespace)))
}),
);
let (namespace_lookups, plugin_lookups) = tokio::join!(namespace_lookups, plugin_lookups);
let nested_namespaces = namespace_lookups
.into_iter()
.chain(plugin_lookups.into_iter().flatten())
.collect();
Self {
inherited_namespace,
nested_namespaces,
}
}
pub(crate) fn for_skill(&self, root: &PathUri, path: &PathUri) -> &ResolvedSkillNamespace {
// Ancestor symlink targets cannot override skills still owned by the scan root.
let path_is_under_root = path.starts_with(root);
// The deepest matching path prefix is the nearest applicable namespace.
self.nested_namespaces
.iter()
.filter(|(namespace_root, _)| {
path.starts_with(namespace_root)
&& (!path_is_under_root || !root.starts_with(namespace_root))
})
.max_by_key(|(namespace_root, _)| namespace_root.ancestors().count())
.map(|(_, namespace)| namespace)
.unwrap_or(&self.inherited_namespace)
}
}
/// The completed namespace resolution for a skill root.
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) enum ResolvedSkillNamespace {
/// No plugin namespace applies to matching skills.
Plain,
/// Qualify matching skill names with this plugin namespace.
Plugin(String),
}
impl ResolvedSkillNamespace {
pub(crate) fn qualify(&self, base_name: &str) -> String {
match self {
Self::Plain => base_name.to_string(),
Self::Plugin(namespace) => format!("{namespace}:{base_name}"),
}
}
}