mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
core/app-server: route skill loading through bound path refs
This commit is contained in:
@@ -513,15 +513,19 @@ impl CatalogRequestProcessor {
|
||||
.await;
|
||||
let skills_manager = self.thread_manager.skills_manager();
|
||||
let plugins_manager = self.thread_manager.plugins_manager();
|
||||
let fs = self
|
||||
let local_environment = self
|
||||
.thread_manager
|
||||
.environment_manager()
|
||||
.default_environment()
|
||||
.map(|environment| environment.get_filesystem());
|
||||
.try_local_environment();
|
||||
// TODO: Reintroduce env-scoped `skills/list` only after cwd/config-layer discovery can run
|
||||
// against the selected environment and the design decides which non-repo roots stay local
|
||||
// versus follow that environment.
|
||||
let local_file_system = Some(Arc::clone(&codex_exec_server::LOCAL_FS));
|
||||
let mut data = futures::stream::iter(cwds.into_iter().enumerate())
|
||||
.map(|(index, cwd)| {
|
||||
let config = &config;
|
||||
let fs = fs.clone();
|
||||
let local_environment = local_environment.clone();
|
||||
let local_file_system = local_file_system.clone();
|
||||
let plugins_manager = &plugins_manager;
|
||||
let skills_manager = &skills_manager;
|
||||
async move {
|
||||
@@ -554,13 +558,19 @@ impl CatalogRequestProcessor {
|
||||
Vec::new()
|
||||
};
|
||||
let skills_input = codex_core::skills::SkillsLoadInput::new(
|
||||
cwd_abs.clone(),
|
||||
local_environment.as_ref().map(|environment| {
|
||||
codex_exec_server::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
cwd_abs.clone(),
|
||||
)
|
||||
}),
|
||||
local_file_system,
|
||||
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);
|
||||
|
||||
@@ -9,12 +9,14 @@ use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::skills::SkillsLoadInput;
|
||||
use codex_core::skills::SkillsManager;
|
||||
use codex_exec_server::EnvironmentPathRef;
|
||||
use codex_file_watcher::FileWatcher;
|
||||
use codex_file_watcher::FileWatcherSubscriber;
|
||||
use codex_file_watcher::Receiver;
|
||||
use codex_file_watcher::ThrottledWatchReceiver;
|
||||
use codex_file_watcher::WatchPath;
|
||||
use codex_file_watcher::WatchRegistration;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
@@ -96,24 +98,26 @@ impl SkillsWatcher {
|
||||
);
|
||||
return WatchRegistration::default();
|
||||
};
|
||||
if environment.is_remote() {
|
||||
return WatchRegistration::default();
|
||||
}
|
||||
|
||||
let root_path_ref = (!environment.is_remote())
|
||||
.then(|| EnvironmentPathRef::new(environment.get_filesystem(), config.cwd.clone()));
|
||||
let local_file_system = Some(Arc::clone(&codex_exec_server::LOCAL_FS));
|
||||
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,
|
||||
local_file_system,
|
||||
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()
|
||||
.filter(|root| !environment.is_remote() || root.scope != SkillScope::Repo)
|
||||
.map(|root| WatchPath {
|
||||
path: root.path.path().to_path_buf(),
|
||||
recursive: true,
|
||||
|
||||
@@ -450,6 +450,56 @@ async fn skills_list_skips_cwd_roots_when_environment_disabled() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_list_skips_cwd_roots_when_only_remote_environment_is_configured() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
write_skill(&codex_home, "home-skill")?;
|
||||
let repo_skill_dir = cwd.path().join(".codex/skills/repo-skill");
|
||||
std::fs::create_dir_all(&repo_skill_dir)?;
|
||||
std::fs::write(
|
||||
repo_skill_dir.join("SKILL.md"),
|
||||
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new_with_env(
|
||||
codex_home.path(),
|
||||
&[(CODEX_EXEC_SERVER_URL_ENV_VAR, Some("ws://127.0.0.1:8765"))],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_skills_list_request(SkillsListParams {
|
||||
cwds: vec![cwd.path().to_path_buf()],
|
||||
force_reload: true,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let SkillsListResponse { data } = to_response(response)?;
|
||||
assert_eq!(data.len(), 1);
|
||||
assert_eq!(data[0].cwd, cwd.path().to_path_buf());
|
||||
assert_eq!(data[0].errors, Vec::new());
|
||||
assert!(
|
||||
data[0]
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| skill.name == "home-skill")
|
||||
);
|
||||
assert!(
|
||||
data[0]
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "repo-skill")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_list_accepts_relative_cwds() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -24,12 +24,12 @@ use crate::system::uninstall_system_skills;
|
||||
use codex_config::SkillsConfig;
|
||||
use codex_exec_server::EnvironmentPathRef;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct SkillsLoadInput {
|
||||
pub cwd: AbsolutePathBuf,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
pub env_path: Option<EnvironmentPathRef>,
|
||||
/// Local user/system/plugin skill roots are read from this filesystem for now.
|
||||
pub local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
pub effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
pub config_layer_stack: ConfigLayerStack,
|
||||
pub bundled_skills_enabled: bool,
|
||||
@@ -38,7 +38,7 @@ pub struct SkillsLoadInput {
|
||||
impl std::fmt::Debug for SkillsLoadInput {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("SkillsLoadInput")
|
||||
.field("cwd", &self.cwd)
|
||||
.field("env_path", &self.env_path)
|
||||
.field("has_local_file_system", &self.local_file_system.is_some())
|
||||
.field("effective_skill_roots", &self.effective_skill_roots)
|
||||
.field("config_layer_stack", &self.config_layer_stack)
|
||||
@@ -49,41 +49,20 @@ impl std::fmt::Debug for SkillsLoadInput {
|
||||
|
||||
impl SkillsLoadInput {
|
||||
pub fn new(
|
||||
cwd: AbsolutePathBuf,
|
||||
env_path: Option<EnvironmentPathRef>,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
config_layer_stack: ConfigLayerStack,
|
||||
bundled_skills_enabled: bool,
|
||||
) -> Self {
|
||||
Self {
|
||||
cwd,
|
||||
local_file_system: Some(Arc::clone(&LOCAL_FS)),
|
||||
env_path,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
bundled_skills_enabled,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn with_local_file_system(
|
||||
mut self,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> Self {
|
||||
self.local_file_system = local_file_system;
|
||||
self
|
||||
}
|
||||
|
||||
fn authorities(
|
||||
&self,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> (
|
||||
Option<EnvironmentPathRef>,
|
||||
Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) {
|
||||
(
|
||||
fs.map(|fs| EnvironmentPathRef::new(fs, self.cwd.clone())),
|
||||
self.local_file_system.clone(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
pub struct SkillsManager {
|
||||
@@ -138,12 +117,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) {
|
||||
@@ -159,15 +134,10 @@ impl SkillsManager {
|
||||
outcome
|
||||
}
|
||||
|
||||
pub async fn skill_roots_for_config(
|
||||
&self,
|
||||
input: &SkillsLoadInput,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let (env_path, local_file_system) = input.authorities(fs);
|
||||
pub async fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots(
|
||||
env_path.as_ref(),
|
||||
local_file_system.as_ref(),
|
||||
input.env_path.as_ref(),
|
||||
input.local_file_system.as_ref(),
|
||||
&input.config_layer_stack,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
@@ -183,12 +153,13 @@ impl SkillsManager {
|
||||
&self,
|
||||
input: &SkillsLoadInput,
|
||||
force_reload: bool,
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> SkillLoadOutcome {
|
||||
let (env_path, local_file_system) = input.authorities(fs);
|
||||
let path_ref_cache_key = env_path.as_ref().map(|_| SkillsPathCacheKey {
|
||||
env_path: env_path.clone(),
|
||||
local_file_system: local_file_system.as_ref().map(ExecutorFileSystemRef::new),
|
||||
let path_ref_cache_key = input.env_path.as_ref().map(|_| SkillsPathCacheKey {
|
||||
env_path: input.env_path.clone(),
|
||||
local_file_system: input
|
||||
.local_file_system
|
||||
.as_ref()
|
||||
.map(ExecutorFileSystemRef::new),
|
||||
});
|
||||
if !force_reload
|
||||
&& let Some(path_ref_cache_key) = path_ref_cache_key.as_ref()
|
||||
@@ -198,8 +169,8 @@ impl SkillsManager {
|
||||
}
|
||||
|
||||
let mut roots = skill_roots(
|
||||
env_path.as_ref(),
|
||||
local_file_system.as_ref(),
|
||||
input.env_path.as_ref(),
|
||||
input.local_file_system.as_ref(),
|
||||
&input.config_layer_stack,
|
||||
input.effective_skill_roots.clone(),
|
||||
self.extra_roots(),
|
||||
|
||||
@@ -92,23 +92,25 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
|
||||
}
|
||||
}
|
||||
|
||||
fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
|
||||
EnvironmentPathRef::local(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(
|
||||
cwd,
|
||||
Some(path_ref),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
effective_skill_roots,
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
)
|
||||
}
|
||||
|
||||
fn local_file_system() -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
Some(Arc::clone(&LOCAL_FS))
|
||||
}
|
||||
|
||||
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"))
|
||||
@@ -191,9 +193,7 @@ async fn skills_for_config_with_stack(
|
||||
effective_skill_roots.to_vec(),
|
||||
config_layer_stack.clone(),
|
||||
);
|
||||
skills_manager
|
||||
.skills_for_config(&skills_input, local_file_system())
|
||||
.await
|
||||
skills_manager.skills_for_config(&skills_input).await
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -254,25 +254,32 @@ async fn skills_for_config_keeps_cache_entries_bound_to_file_system() {
|
||||
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write repo skill");
|
||||
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 first_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let second_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let first_input = SkillsLoadInput::new(
|
||||
Some(EnvironmentPathRef::new(first_file_system, cwd.path().abs())),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
let second_input = SkillsLoadInput::new(
|
||||
Some(EnvironmentPathRef::new(
|
||||
second_file_system,
|
||||
cwd.path().abs(),
|
||||
)),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
|
||||
let first = skills_manager
|
||||
.skills_for_config(&skills_input, Some(first_file_system))
|
||||
.await;
|
||||
let second = skills_manager
|
||||
.skills_for_config(&skills_input, Some(second_file_system))
|
||||
.await;
|
||||
let first = skills_manager.skills_for_config(&first_input).await;
|
||||
let second = skills_manager.skills_for_config(&second_input).await;
|
||||
let first_skill = first
|
||||
.skills
|
||||
.iter()
|
||||
@@ -300,11 +307,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
|
||||
|
||||
let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone());
|
||||
let empty_outcome = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ false,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ false)
|
||||
.await;
|
||||
assert!(
|
||||
empty_outcome
|
||||
@@ -324,11 +327,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
|
||||
skills_manager.set_extra_roots(vec![extra_skills_root.abs()]);
|
||||
|
||||
let runtime_outcome = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ false,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ false)
|
||||
.await;
|
||||
assert!(
|
||||
runtime_outcome
|
||||
@@ -339,11 +338,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
|
||||
|
||||
skills_manager.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]);
|
||||
let replaced_outcome = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ false,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ false)
|
||||
.await;
|
||||
assert_eq!(replaced_outcome.errors, Vec::new());
|
||||
assert!(
|
||||
@@ -489,11 +484,7 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
|
||||
);
|
||||
|
||||
let outcome = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ true,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ true)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
@@ -540,19 +531,20 @@ async fn skills_for_cwd_without_local_fs_skips_local_roots() {
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack");
|
||||
let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone())
|
||||
.with_local_file_system(/*local_file_system*/ None);
|
||||
let skills_input = SkillsLoadInput::new(
|
||||
Some(skill_root_path_ref(cwd.path().abs())),
|
||||
/*local_file_system*/ None,
|
||||
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,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ true)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
@@ -599,14 +591,20 @@ async fn skills_for_cwd_without_env_path_still_loads_local_roots() {
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("valid config layer stack");
|
||||
let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack);
|
||||
let skills_input = SkillsLoadInput::new(
|
||||
/*env_path*/ None,
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
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)
|
||||
.skills_for_cwd(&skills_input, /*force_reload*/ true)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
@@ -677,11 +675,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
|
||||
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await;
|
||||
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,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&base_input, /*force_reload*/ false)
|
||||
.await;
|
||||
assert!(
|
||||
outcome_a
|
||||
@@ -693,11 +687,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,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&base_input, /*force_reload*/ false)
|
||||
.await;
|
||||
assert!(
|
||||
outcome_b
|
||||
@@ -707,7 +697,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, local_file_system())
|
||||
.skills_for_cwd(&base_input, /*force_reload*/ true)
|
||||
.await;
|
||||
assert!(
|
||||
outcome_reloaded
|
||||
@@ -729,32 +719,35 @@ async fn skills_for_cwd_keeps_cache_entries_bound_to_file_system() {
|
||||
"---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n",
|
||||
)
|
||||
.expect("write repo skill");
|
||||
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 first_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let second_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let first_input = SkillsLoadInput::new(
|
||||
Some(EnvironmentPathRef::new(first_file_system, cwd.path().abs())),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
let second_input = SkillsLoadInput::new(
|
||||
Some(EnvironmentPathRef::new(
|
||||
second_file_system,
|
||||
cwd.path().abs(),
|
||||
)),
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_layer_stack.clone(),
|
||||
bundled_skills_enabled_from_stack(&config_layer_stack),
|
||||
);
|
||||
|
||||
let first = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ false,
|
||||
Some(first_file_system),
|
||||
)
|
||||
.skills_for_cwd(&first_input, /*force_reload*/ false)
|
||||
.await;
|
||||
let second = skills_manager
|
||||
.skills_for_cwd(
|
||||
&skills_input,
|
||||
/*force_reload*/ false,
|
||||
Some(second_file_system),
|
||||
)
|
||||
.skills_for_cwd(&second_input, /*force_reload*/ false)
|
||||
.await;
|
||||
let first_skill = first
|
||||
.skills
|
||||
@@ -775,30 +768,32 @@ async fn skills_for_cwd_skips_cache_without_environment_path() {
|
||||
let manager_codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let first_codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let second_codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let first_cwd = tempfile::tempdir().expect("tempdir");
|
||||
let second_cwd = tempfile::tempdir().expect("tempdir");
|
||||
write_user_skill(&first_codex_home, "first", "first-skill", "first");
|
||||
write_user_skill(&second_codex_home, "second", "second-skill", "second");
|
||||
let skills_manager = SkillsManager::new(
|
||||
manager_codex_home.path().abs(),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
let first_input = local_skills_input(
|
||||
first_cwd.path().abs(),
|
||||
let first_input = SkillsLoadInput::new(
|
||||
/*env_path*/ None,
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_stack(&first_codex_home, ""),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
let second_input = local_skills_input(
|
||||
second_cwd.path().abs(),
|
||||
let second_input = SkillsLoadInput::new(
|
||||
/*env_path*/ None,
|
||||
Some(Arc::clone(&LOCAL_FS)),
|
||||
Vec::new(),
|
||||
config_stack(&second_codex_home, ""),
|
||||
/*bundled_skills_enabled*/ true,
|
||||
);
|
||||
|
||||
let first = skills_manager
|
||||
.skills_for_cwd(&first_input, /*force_reload*/ false, /*fs*/ None)
|
||||
.skills_for_cwd(&first_input, /*force_reload*/ false)
|
||||
.await;
|
||||
let second = skills_manager
|
||||
.skills_for_cwd(&second_input, /*force_reload*/ false, /*fs*/ None)
|
||||
.skills_for_cwd(&second_input, /*force_reload*/ false)
|
||||
.await;
|
||||
|
||||
assert!(first.skills.iter().any(|skill| skill.name == "first-skill"));
|
||||
@@ -1094,11 +1089,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
|
||||
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,
|
||||
local_file_system(),
|
||||
)
|
||||
.skills_for_cwd(&parent_input, /*force_reload*/ true)
|
||||
.await;
|
||||
let parent_skill = parent_outcome
|
||||
.skills
|
||||
|
||||
@@ -421,13 +421,17 @@ 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::skills::EnvironmentPathRef::new(
|
||||
Arc::clone(&codex_exec_server::LOCAL_FS),
|
||||
config.cwd.clone(),
|
||||
);
|
||||
let skills_input = skills_load_input_from_config(
|
||||
&config,
|
||||
Some(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
|
||||
.iter()
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -447,20 +447,34 @@ 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| {
|
||||
crate::skills::EnvironmentPathRef::new(
|
||||
environment.environment.get_filesystem(),
|
||||
environment.cwd.clone(),
|
||||
)
|
||||
});
|
||||
let local_file_system = environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem())
|
||||
.or_else(|| Some(Arc::clone(&codex_exec_server::LOCAL_FS)));
|
||||
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,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
);
|
||||
skills_manager.skills_for_config(&skills_input).await.errors
|
||||
}
|
||||
|
||||
impl Session {
|
||||
|
||||
@@ -4127,9 +4127,20 @@ 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,
|
||||
Some(crate::skills::EnvironmentPathRef::new(
|
||||
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,
|
||||
Some(Arc::clone(&skill_fs)),
|
||||
)
|
||||
.await;
|
||||
let parent_skill = parent_outcome
|
||||
@@ -4692,13 +4703,23 @@ 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 skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let skill_fs = environment.get_filesystem();
|
||||
let cwd = crate::skills::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
per_turn_config.cwd.clone(),
|
||||
);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
Some(cwd),
|
||||
services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem()),
|
||||
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 +6556,23 @@ where
|
||||
.plugins_for_config(&per_turn_config.plugins_config_input())
|
||||
.await;
|
||||
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
|
||||
let skills_input =
|
||||
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
|
||||
let skill_fs = environment.get_filesystem();
|
||||
let cwd = crate::skills::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
per_turn_config.cwd.clone(),
|
||||
);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
Some(cwd),
|
||||
services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem()),
|
||||
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);
|
||||
|
||||
@@ -702,19 +702,38 @@ impl Session {
|
||||
&per_turn_config.to_models_manager_config(),
|
||||
)
|
||||
.await;
|
||||
let path_ref = primary_turn_environment.map(|turn_environment| {
|
||||
crate::skills::EnvironmentPathRef::new(
|
||||
turn_environment.environment.get_filesystem(),
|
||||
turn_environment.cwd.clone(),
|
||||
)
|
||||
});
|
||||
// Local turns use their local environment filesystem for both cwd and host-local roots so
|
||||
// implicit command detection can match either kind of loaded skill. Remote turns still
|
||||
// keep host-local user/system/plugin roots on the process-local filesystem.
|
||||
let local_file_system = self
|
||||
.services
|
||||
.environment_manager
|
||||
.try_local_environment()
|
||||
.map(|environment| environment.get_filesystem())
|
||||
.or_else(|| Some(Arc::clone(&codex_exec_server::LOCAL_FS)));
|
||||
let plugins_input = per_turn_config.plugins_config_input();
|
||||
let plugin_outcome = self
|
||||
.services
|
||||
.plugins_manager
|
||||
.plugins_for_config(&per_turn_config.plugins_config_input())
|
||||
.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, effective_skill_roots);
|
||||
let fs = primary_turn_environment
|
||||
.map(|turn_environment| turn_environment.environment.get_filesystem());
|
||||
let skills_input = skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
path_ref,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
);
|
||||
let skills_outcome = Arc::new(
|
||||
self.services
|
||||
.skills_manager
|
||||
.skills_for_config(&skills_input, fs)
|
||||
.skills_for_config(&skills_input)
|
||||
.await,
|
||||
);
|
||||
let goal_tools_supported = !per_turn_config.ephemeral && self.state_db().is_some();
|
||||
|
||||
@@ -5,7 +5,9 @@ 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_utils_plugins::PluginSkillRoot;
|
||||
use std::sync::Arc;
|
||||
|
||||
pub use codex_core_skills::SkillError;
|
||||
pub use codex_core_skills::SkillLoadOutcome;
|
||||
@@ -34,10 +36,13 @@ pub use codex_core_skills::system;
|
||||
|
||||
pub(crate) fn skills_load_input_from_config(
|
||||
config: &Config,
|
||||
env_path: Option<EnvironmentPathRef>,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> SkillsLoadInput {
|
||||
SkillsLoadInput::new(
|
||||
config.cwd.clone(),
|
||||
env_path,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config.config_layer_stack.clone(),
|
||||
config.bundled_skills_enabled(),
|
||||
|
||||
Reference in New Issue
Block a user