mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Load remote repo skills through session environment
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -277,6 +277,7 @@ use crate::skills::SkillLoadOutcome;
|
||||
use crate::skills::SkillMetadata;
|
||||
use crate::skills::SkillsManager;
|
||||
use crate::skills::build_skill_injections;
|
||||
use crate::skills::build_skill_injections_with_environment;
|
||||
use crate::skills::collect_env_var_dependencies;
|
||||
use crate::skills::collect_explicit_skill_mentions;
|
||||
use crate::skills::injection::ToolMentionKind;
|
||||
@@ -2385,7 +2386,8 @@ impl Session {
|
||||
let skills_outcome = Arc::new(
|
||||
self.services
|
||||
.skills_manager
|
||||
.skills_for_config(&per_turn_config),
|
||||
.skills_for_config_with_environment(&per_turn_config, self.services.environment.as_ref())
|
||||
.await,
|
||||
);
|
||||
let mut turn_context: TurnContext = Self::make_turn_context(
|
||||
Some(Arc::clone(&self.services.auth_manager)),
|
||||
@@ -5441,8 +5443,9 @@ pub(crate) async fn run_turn(
|
||||
let SkillInjections {
|
||||
items: skill_items,
|
||||
warnings: skill_warnings,
|
||||
} = build_skill_injections(
|
||||
} = build_skill_injections_with_environment(
|
||||
&mentioned_skills,
|
||||
Some(turn_context.environment.as_ref()),
|
||||
Some(&session_telemetry),
|
||||
&sess.services.analytics_events_client,
|
||||
tracking.clone(),
|
||||
|
||||
@@ -10,9 +10,11 @@ use crate::instructions::SkillInstructions;
|
||||
use crate::mention_syntax::TOOL_MENTION_SIGIL;
|
||||
use crate::mentions::build_skill_name_counts;
|
||||
use crate::skills::SkillMetadata;
|
||||
use codex_environment::Environment;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use tokio::fs;
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
@@ -26,6 +28,23 @@ pub(crate) async fn build_skill_injections(
|
||||
otel: Option<&SessionTelemetry>,
|
||||
analytics_client: &AnalyticsEventsClient,
|
||||
tracking: TrackEventsContext,
|
||||
) -> SkillInjections {
|
||||
build_skill_injections_with_environment(
|
||||
mentioned_skills,
|
||||
None,
|
||||
otel,
|
||||
analytics_client,
|
||||
tracking,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn build_skill_injections_with_environment(
|
||||
mentioned_skills: &[SkillMetadata],
|
||||
environment: Option<&Environment>,
|
||||
otel: Option<&SessionTelemetry>,
|
||||
analytics_client: &AnalyticsEventsClient,
|
||||
tracking: TrackEventsContext,
|
||||
) -> SkillInjections {
|
||||
if mentioned_skills.is_empty() {
|
||||
return SkillInjections::default();
|
||||
@@ -38,7 +57,7 @@ pub(crate) async fn build_skill_injections(
|
||||
let mut invocations = Vec::new();
|
||||
|
||||
for skill in mentioned_skills {
|
||||
match fs::read_to_string(&skill.path_to_skills_md).await {
|
||||
match read_skill_contents(skill, environment).await {
|
||||
Ok(contents) => {
|
||||
emit_skill_injected_metric(otel, skill, "ok");
|
||||
invocations.push(SkillInvocation {
|
||||
@@ -70,6 +89,22 @@ pub(crate) async fn build_skill_injections(
|
||||
result
|
||||
}
|
||||
|
||||
async fn read_skill_contents(
|
||||
skill: &SkillMetadata,
|
||||
environment: Option<&Environment>,
|
||||
) -> std::io::Result<String> {
|
||||
if skill.scope == codex_protocol::protocol::SkillScope::Repo
|
||||
&& let Some(environment) = environment
|
||||
{
|
||||
let abs_path = AbsolutePathBuf::try_from(skill.path_to_skills_md.clone())
|
||||
.map_err(std::io::Error::other)?;
|
||||
let bytes = environment.get_filesystem().read_file(&abs_path).await?;
|
||||
return Ok(String::from_utf8_lossy(&bytes).to_string());
|
||||
}
|
||||
|
||||
fs::read_to_string(&skill.path_to_skills_md).await
|
||||
}
|
||||
|
||||
fn emit_skill_injected_metric(
|
||||
otel: Option<&SessionTelemetry>,
|
||||
skill: &SkillMetadata,
|
||||
|
||||
@@ -14,12 +14,14 @@ use crate::skills::model::SkillPolicy;
|
||||
use crate::skills::model::SkillToolDependency;
|
||||
use crate::skills::system::system_cache_root_dir;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_environment::Environment;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use dirs::home_dir;
|
||||
use dunce::canonicalize as canonicalize_path;
|
||||
@@ -190,28 +192,24 @@ where
|
||||
discover_skills_under_root(&root.path, root.scope, &mut outcome);
|
||||
}
|
||||
|
||||
let mut seen: HashSet<PathBuf> = HashSet::new();
|
||||
finalize_loaded_skills(&mut outcome);
|
||||
outcome
|
||||
.skills
|
||||
.retain(|skill| seen.insert(skill.path_to_skills_md.clone()));
|
||||
}
|
||||
|
||||
fn scope_rank(scope: SkillScope) -> u8 {
|
||||
// Higher-priority scopes first (matches root scan order for dedupe).
|
||||
match scope {
|
||||
SkillScope::Repo => 0,
|
||||
SkillScope::User => 1,
|
||||
SkillScope::System => 2,
|
||||
SkillScope::Admin => 3,
|
||||
}
|
||||
pub(crate) async fn load_skills_from_roots_with_environment<I>(
|
||||
roots: I,
|
||||
environment: &Environment,
|
||||
) -> SkillLoadOutcome
|
||||
where
|
||||
I: IntoIterator<Item = SkillRoot>,
|
||||
{
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
for root in roots {
|
||||
discover_skills_under_root_with_environment(&root.path, root.scope, environment, &mut outcome)
|
||||
.await;
|
||||
}
|
||||
|
||||
outcome.skills.sort_by(|a, b| {
|
||||
scope_rank(a.scope)
|
||||
.cmp(&scope_rank(b.scope))
|
||||
.then_with(|| a.name.cmp(&b.name))
|
||||
.then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md))
|
||||
});
|
||||
|
||||
finalize_loaded_skills(&mut outcome);
|
||||
outcome
|
||||
}
|
||||
|
||||
@@ -323,6 +321,40 @@ fn repo_agents_skill_roots(config_layer_stack: &ConfigLayerStack, cwd: &Path) ->
|
||||
roots
|
||||
}
|
||||
|
||||
pub(crate) async fn repo_agents_skill_roots_with_environment(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &Path,
|
||||
environment: &Environment,
|
||||
) -> Vec<SkillRoot> {
|
||||
let Ok(cwd_abs) = AbsolutePathBuf::try_from(cwd.to_path_buf()) else {
|
||||
return Vec::new();
|
||||
};
|
||||
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
|
||||
let project_root =
|
||||
find_project_root_with_environment(cwd_abs.as_path(), &project_root_markers, environment)
|
||||
.await;
|
||||
let dirs = dirs_between_project_root_and_cwd(cwd_abs.as_path(), project_root.as_path());
|
||||
let file_system = environment.get_filesystem();
|
||||
let mut roots = Vec::new();
|
||||
for dir in dirs {
|
||||
let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
|
||||
let Ok(agents_skills_abs) = AbsolutePathBuf::try_from(agents_skills.clone()) else {
|
||||
continue;
|
||||
};
|
||||
let is_directory = match file_system.get_metadata(&agents_skills_abs).await {
|
||||
Ok(metadata) => metadata.is_directory,
|
||||
Err(_) => false,
|
||||
};
|
||||
if is_directory {
|
||||
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(
|
||||
@@ -380,11 +412,63 @@ fn dirs_between_project_root_and_cwd(cwd: &Path, project_root: &Path) -> Vec<Pat
|
||||
dirs
|
||||
}
|
||||
|
||||
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
|
||||
pub(crate) 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()));
|
||||
}
|
||||
|
||||
async fn find_project_root_with_environment(
|
||||
cwd: &Path,
|
||||
project_root_markers: &[String],
|
||||
environment: &Environment,
|
||||
) -> PathBuf {
|
||||
if project_root_markers.is_empty() {
|
||||
return cwd.to_path_buf();
|
||||
}
|
||||
|
||||
let file_system = environment.get_filesystem();
|
||||
for ancestor in cwd.ancestors() {
|
||||
let Ok(ancestor_abs) = AbsolutePathBuf::try_from(ancestor.to_path_buf()) else {
|
||||
continue;
|
||||
};
|
||||
for marker in project_root_markers {
|
||||
let Ok(marker_path) = AbsolutePathBuf::try_from(ancestor_abs.as_path().join(marker)) else {
|
||||
continue;
|
||||
};
|
||||
if let Ok(metadata) = file_system.get_metadata(&marker_path).await
|
||||
&& (metadata.is_directory || metadata.is_file)
|
||||
{
|
||||
return ancestor.to_path_buf();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
cwd.to_path_buf()
|
||||
}
|
||||
|
||||
fn finalize_loaded_skills(outcome: &mut SkillLoadOutcome) {
|
||||
let mut seen: HashSet<PathBuf> = HashSet::new();
|
||||
outcome
|
||||
.skills
|
||||
.retain(|skill| seen.insert(skill.path_to_skills_md.clone()));
|
||||
|
||||
fn scope_rank(scope: SkillScope) -> u8 {
|
||||
match scope {
|
||||
SkillScope::Repo => 0,
|
||||
SkillScope::User => 1,
|
||||
SkillScope::System => 2,
|
||||
SkillScope::Admin => 3,
|
||||
}
|
||||
}
|
||||
|
||||
outcome.skills.sort_by(|a, b| {
|
||||
scope_rank(a.scope)
|
||||
.cmp(&scope_rank(b.scope))
|
||||
.then_with(|| a.name.cmp(&b.name))
|
||||
.then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md))
|
||||
});
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) {
|
||||
let Ok(root) = canonicalize_path(root) else {
|
||||
return;
|
||||
@@ -429,7 +513,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
let entries = match fs::read_dir(&dir) {
|
||||
Ok(entries) => entries,
|
||||
Err(e) => {
|
||||
error!("failed to read skills dir {}: {e:#}", dir.display());
|
||||
error!("failed to read skills dir {}: {e:#}", dir.as_path().display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
@@ -519,16 +603,152 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
tracing::warn!(
|
||||
"skills scan truncated after {} directories (root: {})",
|
||||
MAX_SKILLS_DIRS_PER_ROOT,
|
||||
root.display()
|
||||
root.as_path().display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async fn discover_skills_under_root_with_environment(
|
||||
root: &Path,
|
||||
scope: SkillScope,
|
||||
environment: &Environment,
|
||||
outcome: &mut SkillLoadOutcome,
|
||||
) {
|
||||
let Ok(root) = AbsolutePathBuf::try_from(root.to_path_buf()) else {
|
||||
return;
|
||||
};
|
||||
let file_system = environment.get_filesystem();
|
||||
let root_metadata = match file_system.get_metadata(&root).await {
|
||||
Ok(metadata) => metadata,
|
||||
Err(_) => return,
|
||||
};
|
||||
if !root_metadata.is_directory {
|
||||
return;
|
||||
}
|
||||
|
||||
fn enqueue_dir(
|
||||
queue: &mut VecDeque<(AbsolutePathBuf, usize)>,
|
||||
visited_dirs: &mut HashSet<PathBuf>,
|
||||
truncated_by_dir_limit: &mut bool,
|
||||
path: AbsolutePathBuf,
|
||||
depth: usize,
|
||||
) {
|
||||
if depth > MAX_SCAN_DEPTH {
|
||||
return;
|
||||
}
|
||||
if visited_dirs.len() >= MAX_SKILLS_DIRS_PER_ROOT {
|
||||
*truncated_by_dir_limit = true;
|
||||
return;
|
||||
}
|
||||
if visited_dirs.insert(path.as_path().to_path_buf()) {
|
||||
queue.push_back((path, depth));
|
||||
}
|
||||
}
|
||||
|
||||
let mut visited_dirs: HashSet<PathBuf> = HashSet::new();
|
||||
visited_dirs.insert(root.as_path().to_path_buf());
|
||||
let mut queue: VecDeque<(AbsolutePathBuf, usize)> = VecDeque::from([(root.clone(), 0)]);
|
||||
let mut truncated_by_dir_limit = false;
|
||||
|
||||
while let Some((dir, depth)) = queue.pop_front() {
|
||||
let entries = match file_system.read_directory(&dir).await {
|
||||
Ok(entries) => entries,
|
||||
Err(e) => {
|
||||
error!("failed to read skills dir {}: {e:#}", dir.as_path().display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
for entry in entries {
|
||||
if entry.file_name.starts_with('.') {
|
||||
continue;
|
||||
}
|
||||
|
||||
let Ok(path) = AbsolutePathBuf::try_from(dir.as_path().join(&entry.file_name)) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if entry.is_directory {
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
path,
|
||||
depth + 1,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if entry.is_symlink {
|
||||
let metadata = match file_system.get_metadata(&path).await {
|
||||
Ok(metadata) => metadata,
|
||||
Err(_) => continue,
|
||||
};
|
||||
if metadata.is_directory {
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
path,
|
||||
depth + 1,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if entry.is_file && entry.file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file_with_environment(&path, scope, environment).await {
|
||||
Ok(skill) => outcome.skills.push(skill),
|
||||
Err(err) => {
|
||||
if scope != SkillScope::System {
|
||||
outcome.errors.push(SkillError {
|
||||
path: path.as_path().to_path_buf(),
|
||||
message: err.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if truncated_by_dir_limit {
|
||||
tracing::warn!(
|
||||
"skills scan truncated after {} directories (root: {})",
|
||||
MAX_SKILLS_DIRS_PER_ROOT,
|
||||
root.as_path().display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, SkillParseError> {
|
||||
let contents = fs::read_to_string(path).map_err(SkillParseError::Read)?;
|
||||
let loaded_metadata = load_skill_metadata(path);
|
||||
parse_skill_contents(path, scope, &contents, loaded_metadata)
|
||||
}
|
||||
|
||||
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
|
||||
async fn parse_skill_file_with_environment(
|
||||
path: &AbsolutePathBuf,
|
||||
scope: SkillScope,
|
||||
environment: &Environment,
|
||||
) -> Result<SkillMetadata, SkillParseError> {
|
||||
let contents = environment
|
||||
.get_filesystem()
|
||||
.read_file(path)
|
||||
.await
|
||||
.map_err(SkillParseError::Read)?;
|
||||
let contents = String::from_utf8_lossy(&contents).to_string();
|
||||
let loaded_metadata = load_skill_metadata_with_environment(path, environment).await;
|
||||
parse_skill_contents(path.as_path(), scope, &contents, loaded_metadata)
|
||||
}
|
||||
|
||||
fn parse_skill_contents(
|
||||
path: &Path,
|
||||
scope: SkillScope,
|
||||
contents: &str,
|
||||
loaded_metadata: LoadedSkillMetadata,
|
||||
) -> Result<SkillMetadata, SkillParseError> {
|
||||
let frontmatter = extract_frontmatter(contents).ok_or(SkillParseError::MissingFrontmatter)?;
|
||||
let parsed: SkillFrontmatter =
|
||||
serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?;
|
||||
|
||||
@@ -556,7 +776,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
policy,
|
||||
permission_profile,
|
||||
managed_network_override,
|
||||
} = load_skill_metadata(path);
|
||||
} = loaded_metadata;
|
||||
|
||||
validate_len(&name, MAX_NAME_LEN, "name")?;
|
||||
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
|
||||
@@ -601,7 +821,7 @@ fn namespaced_skill_name(path: &Path, base_name: &str) -> String {
|
||||
|
||||
fn load_skill_metadata(skill_path: &Path) -> 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.as_path().parent() else {
|
||||
return LoadedSkillMetadata::default();
|
||||
};
|
||||
let metadata_path = skill_dir
|
||||
@@ -654,6 +874,56 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata {
|
||||
}
|
||||
}
|
||||
|
||||
async fn load_skill_metadata_with_environment(
|
||||
skill_path: &AbsolutePathBuf,
|
||||
environment: &Environment,
|
||||
) -> LoadedSkillMetadata {
|
||||
let Some(skill_dir) = skill_path.parent() else {
|
||||
return LoadedSkillMetadata::default();
|
||||
};
|
||||
let metadata_path = skill_dir
|
||||
.join(SKILLS_METADATA_DIR)
|
||||
.join(SKILLS_METADATA_FILENAME);
|
||||
let Ok(metadata_path_abs) = AbsolutePathBuf::try_from(metadata_path.clone()) else {
|
||||
return LoadedSkillMetadata::default();
|
||||
};
|
||||
|
||||
let contents = match environment.get_filesystem().read_file(&metadata_path_abs).await {
|
||||
Ok(contents) => String::from_utf8_lossy(&contents).to_string(),
|
||||
Err(_) => return LoadedSkillMetadata::default(),
|
||||
};
|
||||
|
||||
let parsed: SkillMetadataFile = {
|
||||
let _guard = AbsolutePathBufGuard::new(skill_dir);
|
||||
match serde_yaml::from_str(&contents) {
|
||||
Ok(parsed) => parsed,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
"ignoring {path}: invalid {label}: {error}",
|
||||
path = metadata_path.display(),
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return LoadedSkillMetadata::default();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let SkillMetadataFile {
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
permissions,
|
||||
} = parsed;
|
||||
let (permission_profile, managed_network_override) = normalize_permissions(permissions);
|
||||
LoadedSkillMetadata {
|
||||
interface: resolve_interface(interface, skill_dir),
|
||||
dependencies: resolve_dependencies(dependencies),
|
||||
policy: resolve_policy(policy),
|
||||
permission_profile,
|
||||
managed_network_override,
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_permissions(
|
||||
permissions: Option<SkillPermissionProfile>,
|
||||
) -> (
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_environment::Environment;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
@@ -21,8 +22,11 @@ use crate::config_loader::load_config_layers_state;
|
||||
use crate::plugins::PluginsManager;
|
||||
use crate::skills::SkillLoadOutcome;
|
||||
use crate::skills::build_implicit_skill_path_indexes;
|
||||
use crate::skills::loader::dedupe_skill_roots_by_path;
|
||||
use crate::skills::loader::SkillRoot;
|
||||
use crate::skills::loader::load_skills_from_roots;
|
||||
use crate::skills::loader::load_skills_from_roots_with_environment;
|
||||
use crate::skills::loader::repo_agents_skill_roots_with_environment;
|
||||
use crate::skills::loader::skill_roots;
|
||||
use crate::skills::system::install_system_skills;
|
||||
use crate::skills::system::uninstall_system_skills;
|
||||
@@ -79,6 +83,43 @@ impl SkillsManager {
|
||||
outcome
|
||||
}
|
||||
|
||||
pub async fn skills_for_config_with_environment(
|
||||
&self,
|
||||
config: &Config,
|
||||
environment: &Environment,
|
||||
) -> SkillLoadOutcome {
|
||||
let loaded_plugins = self.plugins_manager.plugins_for_config(config);
|
||||
let mut roots = skill_roots(
|
||||
&config.config_layer_stack,
|
||||
&config.cwd,
|
||||
loaded_plugins.effective_skill_roots(),
|
||||
);
|
||||
roots.extend(
|
||||
repo_agents_skill_roots_with_environment(&config.config_layer_stack, &config.cwd, environment)
|
||||
.await,
|
||||
);
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
if !config.bundled_skills_enabled() {
|
||||
roots.retain(|root| root.scope != SkillScope::System);
|
||||
}
|
||||
|
||||
let mut local_roots = Vec::new();
|
||||
let mut repo_roots = Vec::new();
|
||||
for root in roots {
|
||||
if root.scope == SkillScope::Repo {
|
||||
repo_roots.push(root);
|
||||
} else {
|
||||
local_roots.push(root);
|
||||
}
|
||||
}
|
||||
|
||||
let mut outcome = load_skills_from_roots(local_roots);
|
||||
let remote_repo_outcome = load_skills_from_roots_with_environment(repo_roots, environment).await;
|
||||
outcome.skills.extend(remote_repo_outcome.skills);
|
||||
outcome.errors.extend(remote_repo_outcome.errors);
|
||||
finalize_skill_outcome(outcome, &config.config_layer_stack)
|
||||
}
|
||||
|
||||
pub(crate) fn skill_roots_for_config(&self, config: &Config) -> Vec<SkillRoot> {
|
||||
let loaded_plugins = self.plugins_manager.plugins_for_config(config);
|
||||
let mut roots = skill_roots(
|
||||
|
||||
@@ -5,9 +5,20 @@ use crate::config_loader::ConfigLayerEntry;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use crate::plugins::PluginsManager;
|
||||
use async_trait::async_trait;
|
||||
use codex_environment::CopyOptions;
|
||||
use codex_environment::CreateDirectoryOptions;
|
||||
use codex_environment::Environment;
|
||||
use codex_environment::ExecutorFileSystem;
|
||||
use codex_environment::FileMetadata;
|
||||
use codex_environment::FileSystemResult;
|
||||
use codex_environment::ReadDirectoryEntry;
|
||||
use codex_environment::RemoveOptions;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) {
|
||||
@@ -17,6 +28,113 @@ fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &s
|
||||
fs::write(skill_dir.join("SKILL.md"), content).unwrap();
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct RemappedFileSystem {
|
||||
local_root: AbsolutePathBuf,
|
||||
remote_root: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl RemappedFileSystem {
|
||||
fn new(local_root: &std::path::Path, remote_root: &std::path::Path) -> Self {
|
||||
Self {
|
||||
local_root: AbsolutePathBuf::try_from(local_root.to_path_buf()).unwrap(),
|
||||
remote_root: AbsolutePathBuf::try_from(remote_root.to_path_buf()).unwrap(),
|
||||
}
|
||||
}
|
||||
|
||||
fn remap(&self, path: &AbsolutePathBuf) -> AbsolutePathBuf {
|
||||
let relative = path
|
||||
.as_path()
|
||||
.strip_prefix(self.local_root.as_path())
|
||||
.expect("path should stay under the local test root");
|
||||
AbsolutePathBuf::try_from(self.remote_root.as_path().join(relative)).unwrap()
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for RemappedFileSystem {
|
||||
async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>> {
|
||||
tokio::fs::read(self.remap(path).as_path()).await
|
||||
}
|
||||
|
||||
async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()> {
|
||||
tokio::fs::write(self.remap(path).as_path(), contents).await
|
||||
}
|
||||
|
||||
async fn create_directory(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
options: CreateDirectoryOptions,
|
||||
) -> FileSystemResult<()> {
|
||||
if options.recursive {
|
||||
tokio::fs::create_dir_all(self.remap(path).as_path()).await
|
||||
} else {
|
||||
tokio::fs::create_dir(self.remap(path).as_path()).await
|
||||
}
|
||||
}
|
||||
|
||||
async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata> {
|
||||
let metadata = tokio::fs::metadata(self.remap(path).as_path()).await?;
|
||||
Ok(FileMetadata {
|
||||
is_directory: metadata.is_dir(),
|
||||
is_file: metadata.is_file(),
|
||||
created_at_ms: 0,
|
||||
modified_at_ms: 0,
|
||||
})
|
||||
}
|
||||
|
||||
async fn read_directory(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
|
||||
let mut entries = Vec::new();
|
||||
let mut read_dir = tokio::fs::read_dir(self.remap(path).as_path()).await?;
|
||||
while let Some(entry) = read_dir.next_entry().await? {
|
||||
let metadata = tokio::fs::symlink_metadata(entry.path()).await?;
|
||||
entries.push(ReadDirectoryEntry {
|
||||
file_name: entry.file_name().to_string_lossy().into_owned(),
|
||||
is_directory: metadata.is_dir(),
|
||||
is_file: metadata.is_file(),
|
||||
is_symlink: metadata.file_type().is_symlink(),
|
||||
});
|
||||
}
|
||||
Ok(entries)
|
||||
}
|
||||
|
||||
async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> {
|
||||
let remapped = self.remap(path);
|
||||
match tokio::fs::symlink_metadata(remapped.as_path()).await {
|
||||
Ok(metadata) => {
|
||||
if metadata.is_dir() {
|
||||
if options.recursive {
|
||||
tokio::fs::remove_dir_all(remapped.as_path()).await
|
||||
} else {
|
||||
tokio::fs::remove_dir(remapped.as_path()).await
|
||||
}
|
||||
} else {
|
||||
tokio::fs::remove_file(remapped.as_path()).await
|
||||
}
|
||||
}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound && options.force => Ok(()),
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
async fn copy(
|
||||
&self,
|
||||
source_path: &AbsolutePathBuf,
|
||||
destination_path: &AbsolutePathBuf,
|
||||
_options: CopyOptions,
|
||||
) -> FileSystemResult<()> {
|
||||
tokio::fs::copy(
|
||||
self.remap(source_path).as_path(),
|
||||
self.remap(destination_path).as_path(),
|
||||
)
|
||||
.await
|
||||
.map(|_| ())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
@@ -68,6 +186,55 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
|
||||
assert_eq!(outcome2.skills, outcome1.skills);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_config_with_environment_reads_repo_skills_from_remote_workspace() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let local_root = tempfile::tempdir().expect("tempdir");
|
||||
let remote_root = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
fs::create_dir_all(local_root.path().join("repo/subdir")).unwrap();
|
||||
fs::create_dir_all(remote_root.path().join("repo/subdir")).unwrap();
|
||||
fs::create_dir_all(local_root.path().join(".git")).unwrap();
|
||||
fs::create_dir_all(remote_root.path().join(".git")).unwrap();
|
||||
fs::create_dir_all(local_root.path().join(".agents/skills/demo")).unwrap();
|
||||
fs::create_dir_all(remote_root.path().join(".agents/skills/demo")).unwrap();
|
||||
|
||||
fs::write(
|
||||
local_root.path().join(".agents/skills/demo/SKILL.md"),
|
||||
"---\nname: local-skill\ndescription: local\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
fs::write(
|
||||
remote_root.path().join(".agents/skills/demo/SKILL.md"),
|
||||
"---\nname: remote-skill\ndescription: remote\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
cwd: Some(local_root.path().join("repo/subdir")),
|
||||
..Default::default()
|
||||
})
|
||||
.build()
|
||||
.await
|
||||
.expect("config");
|
||||
|
||||
let environment = Environment::new(Arc::new(RemappedFileSystem::new(
|
||||
local_root.path(),
|
||||
remote_root.path(),
|
||||
)));
|
||||
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
|
||||
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
|
||||
|
||||
let outcome = skills_manager
|
||||
.skills_for_config_with_environment(&config, &environment)
|
||||
.await;
|
||||
|
||||
assert!(outcome.skills.iter().any(|skill| skill.name == "remote-skill"));
|
||||
assert!(outcome.skills.iter().all(|skill| skill.name != "local-skill"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -12,6 +12,7 @@ pub(crate) use env_var_dependencies::collect_env_var_dependencies;
|
||||
pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn;
|
||||
pub(crate) use injection::SkillInjections;
|
||||
pub(crate) use injection::build_skill_injections;
|
||||
pub(crate) use injection::build_skill_injections_with_environment;
|
||||
pub(crate) use injection::collect_explicit_skill_mentions;
|
||||
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
|
||||
pub(crate) use invocation_utils::maybe_emit_implicit_skill_invocation;
|
||||
|
||||
Reference in New Issue
Block a user