mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
codex: preserve local skill root authority
This commit is contained in:
@@ -531,9 +531,13 @@ impl CatalogRequestProcessor {
|
||||
}
|
||||
None => environment_manager.try_local_environment(),
|
||||
};
|
||||
let local_file_system = 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 local_file_system = local_file_system.clone();
|
||||
let selected_environment = selected_environment.clone();
|
||||
let plugins_manager = &plugins_manager;
|
||||
let skills_manager = &skills_manager;
|
||||
@@ -555,22 +559,6 @@ impl CatalogRequestProcessor {
|
||||
);
|
||||
}
|
||||
};
|
||||
let Some(environment) = selected_environment.as_ref() else {
|
||||
// Legacy callers without `environmentId` remain local-only; if local is
|
||||
// unavailable, do not fabricate an unbound skill path.
|
||||
return (
|
||||
index,
|
||||
codex_app_server_protocol::SkillsListEntry {
|
||||
cwd,
|
||||
skills: Vec::new(),
|
||||
errors: Vec::new(),
|
||||
},
|
||||
);
|
||||
};
|
||||
let path_ref = codex_core::skills::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
cwd_abs.clone(),
|
||||
);
|
||||
let effective_skill_roots = if workspace_codex_plugins_enabled {
|
||||
let plugins_input = config.plugins_config_input();
|
||||
plugins_manager
|
||||
@@ -582,8 +570,25 @@ impl CatalogRequestProcessor {
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let Some(environment) = selected_environment.as_ref() else {
|
||||
// Omitted `environmentId` uses the legacy implicit local target. When
|
||||
// local exec is disabled there is no valid implicit target, so return an
|
||||
// empty catalog rather than silently switching to the default environment.
|
||||
return (
|
||||
index,
|
||||
codex_app_server_protocol::SkillsListEntry {
|
||||
cwd,
|
||||
skills: Vec::new(),
|
||||
errors: Vec::new(),
|
||||
},
|
||||
);
|
||||
};
|
||||
let skills_input = codex_core::skills::SkillsLoadInput::new(
|
||||
path_ref,
|
||||
codex_core::skills::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
cwd_abs.clone(),
|
||||
),
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
config.bundled_skills_enabled(),
|
||||
|
||||
@@ -103,11 +103,16 @@ impl SkillsWatcher {
|
||||
|
||||
let root_path_ref =
|
||||
EnvironmentPathRef::new(environment.get_filesystem(), config.cwd.clone());
|
||||
let local_file_system = thread_manager
|
||||
.environment_manager()
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
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(
|
||||
root_path_ref,
|
||||
local_file_system,
|
||||
plugin_outcome.effective_plugin_skill_roots(),
|
||||
config.config_layer_stack.clone(),
|
||||
config.bundled_skills_enabled(),
|
||||
|
||||
@@ -230,7 +230,8 @@ where
|
||||
}
|
||||
|
||||
pub(crate) async fn skill_roots(
|
||||
path_ref: &EnvironmentPathRef,
|
||||
env_path: &EnvironmentPathRef,
|
||||
local_file_system: Option<&Arc<dyn ExecutorFileSystem>>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
extra_skill_roots: Vec<AbsolutePathBuf>,
|
||||
@@ -238,7 +239,8 @@ pub(crate) async fn skill_roots(
|
||||
let home_dir =
|
||||
home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
|
||||
skill_roots_with_home_dir(
|
||||
path_ref,
|
||||
env_path,
|
||||
local_file_system,
|
||||
config_layer_stack,
|
||||
home_dir.as_ref(),
|
||||
plugin_skill_roots,
|
||||
@@ -248,35 +250,38 @@ pub(crate) async fn skill_roots(
|
||||
}
|
||||
|
||||
async fn skill_roots_with_home_dir(
|
||||
path_ref: &EnvironmentPathRef,
|
||||
env_path: &EnvironmentPathRef,
|
||||
local_file_system: Option<&Arc<dyn ExecutorFileSystem>>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
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, path_ref);
|
||||
roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot {
|
||||
path: path_ref.with_path(root.path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some(root.plugin_id),
|
||||
plugin_root: Some(path_ref.with_path(root.plugin_root)),
|
||||
}));
|
||||
roots.extend(extra_skill_roots.into_iter().map(|path| SkillRoot {
|
||||
path: path_ref.with_path(path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
}));
|
||||
roots.extend(repo_agents_skill_roots(path_ref, config_layer_stack).await);
|
||||
let mut roots = repo_config_skill_roots(env_path, config_layer_stack);
|
||||
roots.extend(local_skill_roots(
|
||||
local_file_system,
|
||||
config_layer_stack,
|
||||
home_dir,
|
||||
plugin_skill_roots,
|
||||
extra_skill_roots,
|
||||
));
|
||||
roots.extend(repo_agents_skill_roots(env_path, config_layer_stack).await);
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
roots
|
||||
}
|
||||
|
||||
fn skill_roots_from_layer_stack_inner(
|
||||
fn local_skill_roots(
|
||||
local_file_system: Option<&Arc<dyn ExecutorFileSystem>>,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
home_dir: Option<&AbsolutePathBuf>,
|
||||
path_ref: &EnvironmentPathRef,
|
||||
plugin_skill_roots: Vec<PluginSkillRoot>,
|
||||
extra_skill_roots: Vec<AbsolutePathBuf>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let Some(local_file_system) = local_file_system else {
|
||||
// Local exec can be disabled. In that case, keep repo/env skill loading intact but skip
|
||||
// local absolute roots instead of falling back to the process-global local filesystem.
|
||||
return Vec::new();
|
||||
};
|
||||
let mut roots = Vec::new();
|
||||
|
||||
for layer in config_layer_stack.get_layers(
|
||||
@@ -288,19 +293,13 @@ fn skill_roots_from_layer_stack_inner(
|
||||
};
|
||||
|
||||
match &layer.name {
|
||||
ConfigLayerSource::Project { .. } => {
|
||||
roots.push(SkillRoot {
|
||||
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
});
|
||||
}
|
||||
ConfigLayerSource::Project { .. } => {}
|
||||
ConfigLayerSource::User { .. } => {
|
||||
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
|
||||
// compatibility.
|
||||
// compatibility. These are client-local absolute paths even when the active repo
|
||||
// environment is remote, so bind them to the available local filesystem only.
|
||||
roots.push(SkillRoot {
|
||||
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
|
||||
path: local_path_ref(local_file_system, config_folder.join(SKILLS_DIR_NAME)),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
@@ -309,8 +308,10 @@ fn skill_roots_from_layer_stack_inner(
|
||||
// `$HOME/.agents/skills` (user-installed skills).
|
||||
if let Some(home_dir) = home_dir {
|
||||
roots.push(SkillRoot {
|
||||
path: path_ref
|
||||
.with_path(home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)),
|
||||
path: local_path_ref(
|
||||
local_file_system,
|
||||
home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
|
||||
),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
@@ -320,7 +321,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
// Embedded system skills are cached under `$CODEX_HOME/skills/.system` and are a
|
||||
// special case (not a config layer).
|
||||
roots.push(SkillRoot {
|
||||
path: path_ref.with_path(system_cache_root_dir(&config_folder)),
|
||||
path: local_path_ref(local_file_system, system_cache_root_dir(&config_folder)),
|
||||
scope: SkillScope::System,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
@@ -330,7 +331,7 @@ fn skill_roots_from_layer_stack_inner(
|
||||
// The system config layer lives under `/etc/codex/` on Unix, so treat
|
||||
// `/etc/codex/skills` as admin-scoped skills.
|
||||
roots.push(SkillRoot {
|
||||
path: path_ref.with_path(config_folder.join(SKILLS_DIR_NAME)),
|
||||
path: local_path_ref(local_file_system, config_folder.join(SKILLS_DIR_NAME)),
|
||||
scope: SkillScope::Admin,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
@@ -343,19 +344,64 @@ fn skill_roots_from_layer_stack_inner(
|
||||
}
|
||||
}
|
||||
|
||||
roots.extend(plugin_skill_roots.into_iter().map(|root| SkillRoot {
|
||||
// Plugin discovery is currently local-only; multi-env plugin skill loading should be an
|
||||
// explicit future change rather than inheriting the selected repo environment here.
|
||||
path: local_path_ref(local_file_system, root.path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some(root.plugin_id),
|
||||
plugin_root: Some(local_path_ref(local_file_system, root.plugin_root)),
|
||||
}));
|
||||
roots.extend(extra_skill_roots.into_iter().map(|path| SkillRoot {
|
||||
path: local_path_ref(local_file_system, path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
}));
|
||||
|
||||
roots
|
||||
}
|
||||
|
||||
fn repo_config_skill_roots(
|
||||
env_path: &EnvironmentPathRef,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> Vec<SkillRoot> {
|
||||
config_layer_stack
|
||||
.get_layers(
|
||||
ConfigLayerStackOrdering::HighestPrecedenceFirst,
|
||||
/*include_disabled*/ true,
|
||||
)
|
||||
.filter_map(|layer| match &layer.name {
|
||||
ConfigLayerSource::Project { .. } => layer.config_folder().map(|config_folder| {
|
||||
SkillRoot {
|
||||
// Project and repo `.agents` roots belong to the selected environment because
|
||||
// their absolute paths are only meaningful within that environment's cwd/repo.
|
||||
path: env_path.with_path(config_folder.join(SKILLS_DIR_NAME)),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
}
|
||||
}),
|
||||
ConfigLayerSource::User { .. }
|
||||
| ConfigLayerSource::System { .. }
|
||||
| ConfigLayerSource::Mdm { .. }
|
||||
| ConfigLayerSource::SessionFlags
|
||||
| ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. }
|
||||
| ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
async fn repo_agents_skill_roots(
|
||||
path_ref: &EnvironmentPathRef,
|
||||
env_path: &EnvironmentPathRef,
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
) -> Vec<SkillRoot> {
|
||||
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
|
||||
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 project_root = find_project_root(env_path, &project_root_markers).await;
|
||||
let dirs = dirs_between_project_root_and_cwd(env_path.path(), project_root.path());
|
||||
let mut roots = Vec::new();
|
||||
for dir in dirs {
|
||||
let agents_skills = path_ref.with_path(dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME));
|
||||
let agents_skills = env_path.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,
|
||||
@@ -376,6 +422,13 @@ async fn repo_agents_skill_roots(
|
||||
roots
|
||||
}
|
||||
|
||||
fn local_path_ref(
|
||||
file_system: &Arc<dyn ExecutorFileSystem>,
|
||||
path: AbsolutePathBuf,
|
||||
) -> EnvironmentPathRef {
|
||||
EnvironmentPathRef::new(Arc::clone(file_system), path)
|
||||
}
|
||||
|
||||
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(
|
||||
@@ -1061,8 +1114,16 @@ pub(crate) async fn skill_roots_from_layer_stack(
|
||||
home_dir: Option<&AbsolutePathBuf>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let path_ref = EnvironmentPathRef::new(fs, cwd.clone());
|
||||
skill_roots_with_home_dir(&path_ref, config_layer_stack, home_dir, Vec::new(), Vec::new())
|
||||
.await
|
||||
let local_file_system = path_ref.file_system();
|
||||
skill_roots_with_home_dir(
|
||||
&path_ref,
|
||||
Some(&local_file_system),
|
||||
config_layer_stack,
|
||||
home_dir,
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -4,7 +4,9 @@ use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_exec_server::LocalFileSystem;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -291,6 +293,55 @@ async fn skill_roots_from_layer_stack_includes_disabled_project_layers() -> anyh
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skill_roots_bind_repo_and_local_roots_to_their_own_file_systems() -> anyhow::Result<()> {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let project_root = codex_home.path().join("workspace");
|
||||
fs::create_dir_all(project_root.join(".git"))?;
|
||||
fs::create_dir_all(project_root.join(REPO_ROOT_CONFIG_DIR_NAME))?;
|
||||
let cfg = make_config_for_cwd(&codex_home, project_root.clone()).await;
|
||||
|
||||
let env_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let local_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let env_path = EnvironmentPathRef::new(env_file_system.clone(), cfg.cwd.clone());
|
||||
let roots = super::skill_roots(
|
||||
&env_path,
|
||||
Some(&local_file_system),
|
||||
&cfg.config_layer_stack,
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let repo_root = roots
|
||||
.iter()
|
||||
.find(|root| root.scope == SkillScope::Repo)
|
||||
.expect("repo root");
|
||||
let user_root = roots
|
||||
.iter()
|
||||
.find(|root| root.scope == SkillScope::User)
|
||||
.expect("user root");
|
||||
assert!(Arc::ptr_eq(&repo_root.path.file_system(), &env_file_system));
|
||||
assert!(Arc::ptr_eq(
|
||||
&user_root.path.file_system(),
|
||||
&local_file_system
|
||||
));
|
||||
|
||||
let roots_without_local = super::skill_roots(
|
||||
&env_path,
|
||||
/*local_file_system*/ None,
|
||||
&cfg.config_layer_stack,
|
||||
Vec::new(),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
roots_without_local
|
||||
.iter()
|
||||
.all(|root| root.scope == SkillScope::Repo)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<()> {
|
||||
let tmp = tempfile::tempdir()?;
|
||||
@@ -1887,12 +1938,18 @@ async fn skill_roots_include_admin_with_lowest_priority() {
|
||||
let cfg = make_config(&codex_home).await;
|
||||
|
||||
let cwd = local_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 local_file_system = cwd.file_system();
|
||||
let scopes: Vec<SkillScope> = super::skill_roots(
|
||||
&cwd,
|
||||
Some(&local_file_system),
|
||||
&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);
|
||||
|
||||
@@ -23,10 +23,15 @@ use crate::system::install_system_skills;
|
||||
use crate::system::uninstall_system_skills;
|
||||
use codex_config::SkillsConfig;
|
||||
use codex_exec_server::EnvironmentPathRef;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct SkillsLoadInput {
|
||||
pub path_ref: EnvironmentPathRef,
|
||||
pub env_path: EnvironmentPathRef,
|
||||
/// Local absolute skill roots use this filesystem when local exec is available.
|
||||
/// Repo-relative roots still use `env_path`, so remote turns do not accidentally
|
||||
/// read local user/system/plugin skill paths from the remote filesystem.
|
||||
pub local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
pub effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
pub config_layer_stack: ConfigLayerStack,
|
||||
pub bundled_skills_enabled: bool,
|
||||
@@ -34,13 +39,15 @@ pub struct SkillsLoadInput {
|
||||
|
||||
impl SkillsLoadInput {
|
||||
pub fn new(
|
||||
path_ref: EnvironmentPathRef,
|
||||
env_path: EnvironmentPathRef,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
bundled_skills_enabled: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
path_ref,
|
||||
env_path,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
bundled_skills_enabled,
|
||||
@@ -52,7 +59,7 @@ pub struct SkillsManager {
|
||||
codex_home: AbsolutePathBuf,
|
||||
restriction_product: Option<Product>,
|
||||
extra_roots: RwLock<Vec<AbsolutePathBuf>>,
|
||||
cache_by_path_ref: RwLock<HashMap<EnvironmentPathRef, SkillLoadOutcome>>,
|
||||
cache_by_path_ref: RwLock<HashMap<SkillsPathCacheKey, SkillLoadOutcome>>,
|
||||
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
|
||||
}
|
||||
|
||||
@@ -119,7 +126,8 @@ impl SkillsManager {
|
||||
|
||||
pub async fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots(
|
||||
&input.path_ref,
|
||||
&input.env_path,
|
||||
input.local_file_system.as_ref(),
|
||||
&input.config_layer_stack,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
@@ -136,7 +144,13 @@ impl SkillsManager {
|
||||
input: &SkillsLoadInput,
|
||||
force_reload: bool,
|
||||
) -> SkillLoadOutcome {
|
||||
let path_ref_cache_key = input.path_ref.clone();
|
||||
let path_ref_cache_key = SkillsPathCacheKey {
|
||||
env_path: input.env_path.clone(),
|
||||
local_file_system: input
|
||||
.local_file_system
|
||||
.as_ref()
|
||||
.map(ExecutorFileSystemRef::new),
|
||||
};
|
||||
if !force_reload
|
||||
&& let Some(outcome) = self.cached_outcome_for_path_ref(&path_ref_cache_key)
|
||||
{
|
||||
@@ -144,7 +158,8 @@ impl SkillsManager {
|
||||
}
|
||||
|
||||
let mut roots = skill_roots(
|
||||
&input.path_ref,
|
||||
&input.env_path,
|
||||
input.local_file_system.as_ref(),
|
||||
&input.config_layer_stack,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
@@ -201,11 +216,11 @@ impl SkillsManager {
|
||||
|
||||
fn cached_outcome_for_path_ref(
|
||||
&self,
|
||||
path_ref: &EnvironmentPathRef,
|
||||
cache_key: &SkillsPathCacheKey,
|
||||
) -> 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(),
|
||||
Ok(cache) => cache.get(cache_key).cloned(),
|
||||
Err(err) => err.into_inner().get(cache_key).cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -227,6 +242,41 @@ impl SkillsManager {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
|
||||
struct SkillsPathCacheKey {
|
||||
env_path: EnvironmentPathRef,
|
||||
local_file_system: Option<ExecutorFileSystemRef>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct ExecutorFileSystemRef(Arc<dyn ExecutorFileSystem>);
|
||||
|
||||
impl ExecutorFileSystemRef {
|
||||
fn new(file_system: &Arc<dyn ExecutorFileSystem>) -> Self {
|
||||
Self(Arc::clone(file_system))
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for ExecutorFileSystemRef {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
Arc::ptr_eq(&self.0, &other.0)
|
||||
}
|
||||
}
|
||||
|
||||
impl Eq for ExecutorFileSystemRef {}
|
||||
|
||||
impl std::hash::Hash for ExecutorFileSystemRef {
|
||||
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
|
||||
std::hash::Hash::hash(&(Arc::as_ptr(&self.0) as *const () as usize), state);
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for ExecutorFileSystemRef {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("ExecutorFileSystemRef").finish()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
struct ConfigSkillsCacheKey {
|
||||
roots: Vec<(EnvironmentPathRef, u8, Option<String>)>,
|
||||
|
||||
@@ -97,6 +97,7 @@ fn local_skills_input(
|
||||
let path_ref = skill_root_path_ref(cwd);
|
||||
SkillsLoadInput::new(
|
||||
path_ref,
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
effective_skill_roots,
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
|
||||
@@ -425,7 +425,12 @@ enabled = false
|
||||
Arc::clone(&codex_exec_server::LOCAL_FS),
|
||||
config.cwd.clone(),
|
||||
);
|
||||
let skills_input = skills_load_input_from_config(&config, cwd, effective_skill_roots);
|
||||
let skills_input = skills_load_input_from_config(
|
||||
&config,
|
||||
cwd,
|
||||
Some(Arc::clone(&codex_exec_server::LOCAL_FS)),
|
||||
effective_skill_roots,
|
||||
);
|
||||
let outcome = skills_manager.skills_for_config(&skills_input).await;
|
||||
let skill = outcome
|
||||
.skills
|
||||
|
||||
@@ -464,11 +464,18 @@ async fn warm_plugins_and_skills_for_session_init(
|
||||
let Some(path_ref) = path_ref else {
|
||||
return Vec::new();
|
||||
};
|
||||
let local_file_system = environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
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(), path_ref, effective_skill_roots);
|
||||
let skills_input = skills_load_input_from_config(
|
||||
config.as_ref(),
|
||||
path_ref,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
);
|
||||
skills_manager.skills_for_config(&skills_input).await.errors
|
||||
}
|
||||
|
||||
|
||||
@@ -4133,6 +4133,11 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
|
||||
Arc::clone(&skill_fs),
|
||||
parent_config.cwd.clone(),
|
||||
),
|
||||
session
|
||||
.services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem()),
|
||||
Vec::new(),
|
||||
),
|
||||
/*force_reload*/ true,
|
||||
@@ -4702,8 +4707,15 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
environment.get_filesystem(),
|
||||
per_turn_config.cwd.clone(),
|
||||
);
|
||||
let skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, cwd, effective_skill_roots);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
cwd,
|
||||
services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem()),
|
||||
effective_skill_roots,
|
||||
);
|
||||
let skills_outcome = Arc::new(
|
||||
services
|
||||
.skills_manager
|
||||
@@ -6548,8 +6560,15 @@ where
|
||||
environment.get_filesystem(),
|
||||
per_turn_config.cwd.clone(),
|
||||
);
|
||||
let skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, cwd, effective_skill_roots);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
cwd,
|
||||
services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem()),
|
||||
effective_skill_roots,
|
||||
);
|
||||
let skills_outcome = Arc::new(
|
||||
services
|
||||
.skills_manager
|
||||
|
||||
@@ -685,6 +685,11 @@ impl Session {
|
||||
)
|
||||
});
|
||||
let skills_outcome = if let Some(path_ref) = path_ref {
|
||||
let local_file_system = self
|
||||
.services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
let plugins_input = per_turn_config.plugins_config_input();
|
||||
let plugin_outcome = self
|
||||
.services
|
||||
@@ -692,8 +697,12 @@ impl Session {
|
||||
.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);
|
||||
let skills_input = skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
path_ref,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
);
|
||||
Arc::new(
|
||||
self.services
|
||||
.skills_manager
|
||||
|
||||
@@ -5,9 +5,11 @@ 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;
|
||||
@@ -36,11 +38,13 @@ pub use codex_core_skills::system;
|
||||
|
||||
pub(crate) fn skills_load_input_from_config(
|
||||
config: &Config,
|
||||
path_ref: EnvironmentPathRef,
|
||||
env_path: EnvironmentPathRef,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> SkillsLoadInput {
|
||||
SkillsLoadInput::new(
|
||||
path_ref,
|
||||
env_path,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config.config_layer_stack.clone(),
|
||||
config.bundled_skills_enabled(),
|
||||
|
||||
Reference in New Issue
Block a user