openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve filesystem authority when loading skills (#59279)

Browse Source 
## Why

Skill discovery and metadata loading used raw filesystem handles with unrestricted reads, and cache keys distinguished filesystems without accounting for captured permissions.

## What changed

- Pass borrowed `EnvironmentAccess` accessors through skill root resolution, discovery, canonicalization, and metadata reads.
- Include captured permissions in skill snapshot and empty remote root cache keys.
- Add fixed-path watches to `EnvironmentAccess`, falling back to live discovery when read restrictions prevent using unsandboxed executor watches.
- Keep user and plugin roots under host filesystem authority independently of the selected repository executor.

## Testing

Extend remote root cache coverage to verify that changing captured permissions triggers a fresh scan and that restricted accessors decline unsandboxed watches.

GitOrigin-RevId: 97d4bfdfcba78a93fc8845773c6397d4364494e3
This commit is contained in:
Dmitriy Kovalenko
2026-11-04 01:13:54 +00:00
committed by copyberry
parent afefd0ba95
commit 526f6099f6
25 changed files with 403 additions and 273 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/agent/role_tests.rs
+5 -1
View File
@@ -684,7 +684,11 @@ enabled = false
let snapshot = skills_service
.snapshot_for_config(
&skills_input,
Some(Arc::clone(&codex_exec_server::LOCAL_FS)),
Some(
&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(
&codex_exec_server::LOCAL_FS,
),
),
)
.await;
let outcome = snapshot.outcome();
codex-rs/core/src/plugins/skill_snapshot_tests.rs
+1 -1
View File
@@ -77,7 +77,7 @@ enabled = false
plugins_manager.plugin_skill_snapshots_for_config(&plugins_input),
);
let snapshot = skills_service
.snapshot_for_config(&skills_input, /*fs*/ None)
.snapshot_for_config(&skills_input, /*repository*/ None)
.await;
assert_eq!(
codex-rs/core/src/session/session.rs
+9 -1
View File
@@ -689,8 +689,16 @@ async fn warm_plugins_and_skills_for_session_init(
effective_skill_roots,
turn_environments,
);
let access = fs
.as_ref()
.map(codex_exec_server::FileSystemEnvironmentAccessor::unrestricted);
skills_service
.snapshot_for_config(&skills_input, fs)
.snapshot_for_config(
&skills_input,
access
.as_ref()
.map(|access| access as &dyn codex_file_system::EnvironmentAccess),
)
.await
.outcome()
.errors
codex-rs/core/src/session/tests.rs
+8 -2
View File
@@ -7196,7 +7196,10 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
let skills_snapshot = session
.services
.skills_service
.snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.snapshot_for_config(
&skills_input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&skill_fs)),
)
.await;
let turn_context = Session::make_turn_context(
thread_id,
@@ -9531,7 +9534,10 @@ where
let skills_snapshot = session
.services
.skills_service
.snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.snapshot_for_config(
&skills_input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&skill_fs)),
)
.await;
let turn_context = Arc::new(Session::make_turn_context(
thread_id,
codex-rs/core/src/session/turn_context.rs
+9 -1
View File
@@ -1364,9 +1364,17 @@ impl Session {
effective_skill_roots,
&turn_environments,
);
let access = fs
.as_ref()
.map(codex_exec_server::FileSystemEnvironmentAccessor::unrestricted);
self.services
.skills_service
.snapshot_for_config(&skills_input, fs)
.snapshot_for_config(
&skills_input,
access
.as_ref()
.map(|access| access as &dyn EnvironmentAccess),
)
.await
};
let step_settings = Arc::new(ResolvedStepSettings::new(
codex-rs/core/tests/suite/plugins.rs
+1 -1
View File
@@ -1598,7 +1598,7 @@ async fn curated_plugin_skills_follow_auth_switch() -> Result<()> {
let skills_snapshot = test_codex
.thread_manager
.skills_service()
.snapshot_for_config(&skills_input, /*fs*/ None)
.snapshot_for_config(&skills_input, /*repository*/ None)
.await;
skills_snapshot
.outcome()
codex-rs/core/tests/suite/skills_extension.rs
+1 -1
View File
@@ -922,7 +922,7 @@ async fn repository_skill_cache_cannot_read_inaccessible_files(
Vec::new(),
test.config.config_layer_stack.clone(),
),
Some(Arc::clone(&file_system)),
Some(&open),
)
.await;
let [revoked_skill, gated_skill] = ["repo-revoked", "repo-gated"].map(|name| {
codex-rs/ext/skills/src/host_roots.rs
+55 -59
View File
@@ -1,8 +1,6 @@
use std::collections::HashSet;
use std::io;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::Weak;
use tokio::sync::oneshot;
use codex_config::ConfigLayerSource;
@@ -10,7 +8,9 @@ use codex_config::ConfigLayerStack;
use codex_config::default_project_root_markers;
use codex_config::merge_toml_values;
use codex_config::project_root_markers_from_config;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessKey;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::GetMetadataOptions;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::SkillScope;
@@ -35,7 +35,7 @@ pub(crate) struct EmptyRemoteRootsCache {
}
struct EmptyRemoteRoots {
fs: Weak<dyn ExecutorFileSystem>,
fs: EnvironmentAccessKey,
cwd: AbsolutePathBuf,
markers: Vec<String>,
watch: EmptyRootsWatch,
@@ -53,18 +53,20 @@ impl EmptyRemoteRootsCache {
}
}
pub(crate) async fn resolve_skill_roots(
repository_file_system: Option<Arc<dyn ExecutorFileSystem>>,
pub(crate) async fn resolve_skill_roots<'a>(
repository_file_system: Option<&'a dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot> {
) -> Vec<HostSkillRoot<'a>> {
let home_dir =
home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
resolve_skill_roots_with_home_dir(
repository_file_system,
host,
config_layer_stack,
cwd,
home_dir.as_ref(),
@@ -75,26 +77,31 @@ pub(crate) async fn resolve_skill_roots(
.await
}
async fn resolve_skill_roots_with_home_dir(
repository_file_system: Option<Arc<dyn ExecutorFileSystem>>,
#[expect(
clippy::too_many_arguments,
reason = "explicit filesystem authorities and a home override keep root-order tests deterministic"
)]
async fn resolve_skill_roots_with_home_dir<'a>(
repository_file_system: Option<&'a dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
home_dir: Option<&AbsolutePathBuf>,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot> {
) -> Vec<HostSkillRoot<'a>> {
let mut roots =
roots_from_layer_stack(config_layer_stack, home_dir, repository_file_system.clone());
roots_from_layer_stack(config_layer_stack, home_dir, repository_file_system, host);
roots.extend(
plugin_skill_roots
.into_iter()
.map(|root| HostSkillRoot::plugin(root, Arc::clone(&LOCAL_FS))),
.map(|root| HostSkillRoot::plugin(root, host)),
);
roots.extend(
extra_skill_roots
.into_iter()
.map(|path| local_root(path, SkillScope::User)),
.map(|path| HostSkillRoot::host(path, SkillScope::User, host)),
);
roots.extend(
repo_agents_skill_roots(
@@ -109,11 +116,12 @@ async fn resolve_skill_roots_with_home_dir(
roots
}
fn roots_from_layer_stack(
fn roots_from_layer_stack<'a>(
config_layer_stack: &ConfigLayerStack,
home_dir: Option<&AbsolutePathBuf>,
repository_file_system: Option<Arc<dyn ExecutorFileSystem>>,
) -> Vec<HostSkillRoot> {
repository_file_system: Option<&'a dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
) -> Vec<HostSkillRoot<'a>> {
let mut roots = Vec::new();
for layer in config_layer_stack.all_layers_high_to_low() {
@@ -123,38 +131,42 @@ fn roots_from_layer_stack(
match &layer.name {
ConfigLayerSource::Project { .. } => {
if let Some(repository_file_system) = &repository_file_system {
if let Some(repository_file_system) = repository_file_system {
roots.push(HostSkillRoot::host(
config_folder.join(SKILLS_DIR_NAME),
SkillScope::Repo,
Arc::clone(repository_file_system),
repository_file_system,
));
}
}
ConfigLayerSource::User { .. } => {
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
// compatibility.
roots.push(local_root(
roots.push(HostSkillRoot::host(
config_folder.join(SKILLS_DIR_NAME),
SkillScope::User,
host,
));
if let Some(home_dir) = home_dir {
roots.push(local_root(
roots.push(HostSkillRoot::host(
home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
SkillScope::User,
host,
));
}
roots.push(local_root(
roots.push(HostSkillRoot::host(
system_cache_root_dir(&config_folder),
SkillScope::System,
host,
));
}
ConfigLayerSource::System { .. } => {
roots.push(local_root(
roots.push(HostSkillRoot::host(
config_folder.join(SKILLS_DIR_NAME),
SkillScope::Admin,
host,
));
}
ConfigLayerSource::PackagedDefaults { .. }
@@ -169,31 +181,26 @@ fn roots_from_layer_stack(
roots
}
fn local_root(path: AbsolutePathBuf, scope: SkillScope) -> HostSkillRoot {
HostSkillRoot::host(path, scope, Arc::clone(&LOCAL_FS))
}
async fn repo_agents_skill_roots(
repository_file_system: Option<Arc<dyn ExecutorFileSystem>>,
async fn repo_agents_skill_roots<'a>(
repository_file_system: Option<&'a dyn EnvironmentAccess>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot> {
) -> Vec<HostSkillRoot<'a>> {
let Some(repository_file_system) = repository_file_system else {
return Vec::new();
};
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
let cache = empty_remote_roots.filter(|_| !Arc::ptr_eq(&repository_file_system, &LOCAL_FS));
let file_system = Arc::downgrade(&repository_file_system);
let file_system = repository_file_system.cache_key();
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let cache = empty_remote_roots.filter(|_| !file_system.same_file_system(&host.cache_key()));
let generation = if let Some(cache) = cache {
let mut cache = cache
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let mut register = true;
if let Some(entry) = cache.entry.as_mut().filter(|entry| {
entry.fs.ptr_eq(&file_system)
&& entry.cwd == *cwd
&& entry.markers == project_root_markers
entry.fs == file_system && entry.cwd == *cwd && entry.markers == project_root_markers
}) {
if let EmptyRootsWatch::Registering(registered) = &mut entry.watch {
match registered.try_recv() {
@@ -214,25 +221,18 @@ async fn repo_agents_skill_roots(
None
};
let (project_root, marker, mut complete) =
find_project_root(repository_file_system.as_ref(), cwd, &project_root_markers).await;
find_project_root(repository_file_system, cwd, &project_root_markers).await;
let directories = dirs_between_project_root_and_cwd(cwd, &project_root);
let mut roots = Vec::new();
let mut missing = Vec::new();
let mut results = futures::stream::iter(directories)
.map(|directory| {
let repository_file_system = Arc::clone(&repository_file_system);
async move {
let agents_skills = directory.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
let agents_skills_uri = PathUri::from_abs_path(&agents_skills);
let result = repository_file_system
.get_metadata(
&agents_skills_uri,
GetMetadataOptions::default(),
/*sandbox*/ None,
)
.await;
(agents_skills, result)
}
.map(|directory| async move {
let agents_skills = directory.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
let agents_skills_uri = PathUri::from_abs_path(&agents_skills);
let result = repository_file_system
.get_metadata(&agents_skills_uri, GetMetadataOptions::default())
.await;
(agents_skills, result)
})
.buffered(MAX_CONCURRENT_ANCESTOR_PROBES);
while let Some((agents_skills, result)) = results.next().await {
@@ -240,7 +240,7 @@ async fn repo_agents_skill_roots(
Ok(metadata) if metadata.is_directory => roots.push(HostSkillRoot::host(
agents_skills,
SkillScope::Repo,
Arc::clone(&repository_file_system),
repository_file_system,
)),
Ok(_) => complete = false,
Err(error) if error.kind() == io::ErrorKind::NotFound => {
@@ -285,13 +285,13 @@ async fn repo_agents_skill_roots(
});
// A cold question never waits for an extra round trip. Until the executor
// acknowledges coverage, other questions take the ordinary live scan.
let repository_file_system = Arc::clone(&repository_file_system);
let present = marker.as_ref().map(PathUri::from_abs_path);
let watch = repository_file_system.watch_missing(missing, present);
tokio::spawn(async move {
let present = marker.as_ref().map(PathUri::from_abs_path);
tokio::select! {
biased;
_ = ready.closed() => {}
watch = repository_file_system.watch_missing(missing, present) => match watch {
watch = watch => match watch {
Ok(Some(invalidated)) => {
tracing::debug!("watch established for empty remote repository skills");
let _ = ready.send(invalidated);
@@ -326,7 +326,7 @@ fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec
}
async fn find_project_root(
repository_file_system: &dyn ExecutorFileSystem,
repository_file_system: &dyn EnvironmentAccess,
cwd: &AbsolutePathBuf,
project_root_markers: &[String],
) -> (AbsolutePathBuf, Option<AbsolutePathBuf>, bool) {
@@ -345,11 +345,7 @@ async fn find_project_root(
.map(|(ancestor, marker_path)| async move {
let marker_path_uri = PathUri::from_abs_path(&marker_path);
let result = repository_file_system
.get_metadata(
&marker_path_uri,
GetMetadataOptions::default(),
/*sandbox*/ None,
)
.get_metadata(&marker_path_uri, GetMetadataOptions::default())
.await;
(ancestor, marker_path, result)
})
codex-rs/ext/skills/src/host_roots_tests.rs
+81 -22
View File
@@ -9,9 +9,11 @@ use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirementsToml;
use codex_exec_server::CopyOptions;
use codex_exec_server::CreateDirectoryOptions;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::ExecutorFileSystemFuture;
use codex_exec_server::FileMetadata;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::FileSystemReadStream;
use codex_exec_server::FileSystemSandboxContext;
use codex_exec_server::GetMetadataOptions;
@@ -22,6 +24,10 @@ use codex_exec_server::RemoveOptions;
use codex_exec_server::WalkOptions;
use codex_exec_server::WalkOutcome;
use codex_exec_server::WriteFileOptions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SkillScope;
use codex_skills::SkillMetadata;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -280,6 +286,7 @@ fn expected_skill(path: AbsolutePathBuf, name: &str, scope: SkillScope) -> Skill
#[test]
fn layer_roots_preserve_scope_precedence_and_disabled_projects() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let system_folder = absolute(temp_dir.path().join("etc/codex"));
let home_folder = absolute(temp_dir.path().join("home"));
@@ -304,14 +311,10 @@ fn layer_roots_preserve_scope_precedence_and_disabled_projects() {
project_layer(&nested_project_folder),
]);
let roots = roots_from_layer_stack(
&config_stack,
Some(&home_folder),
Some(Arc::clone(&LOCAL_FS)),
)
.into_iter()
.map(|root| (root.scope, root.path))
.collect::<Vec<_>>();
let roots = roots_from_layer_stack(&config_stack, Some(&home_folder), Some(&access), &access)
.into_iter()
.map(|root| (root.scope, root.path))
.collect::<Vec<_>>();
assert_eq!(
roots,
@@ -328,6 +331,7 @@ fn layer_roots_preserve_scope_precedence_and_disabled_projects() {
#[tokio::test]
async fn plugin_roots_preserve_plugin_resolution_metadata() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let cwd = absolute(temp_dir.path().join("workspace"));
let plugin_root = absolute(temp_dir.path().join("plugins/example"));
@@ -340,6 +344,7 @@ async fn plugin_roots_preserve_plugin_resolution_metadata() {
let roots = resolve_skill_roots_with_home_dir(
/*repository_file_system*/ None,
&access,
&stack(Vec::new()),
&cwd,
/*home_dir*/ None,
@@ -375,11 +380,12 @@ async fn plugin_roots_preserve_plugin_resolution_metadata() {
SkillDiscoveryMode::DirectChildren,
)
);
assert!(Arc::ptr_eq(&root.file_system, &LOCAL_FS));
assert_eq!(root.file_system.cache_key(), access.cache_key());
}
#[tokio::test]
async fn unique_extra_root_loads_as_recursive_user_root() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let cwd = absolute(temp_dir.path().join("workspace"));
let extra_root = absolute(temp_dir.path().join("runtime-skills"));
@@ -387,6 +393,7 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
let roots = resolve_skill_roots_with_home_dir(
/*repository_file_system*/ None,
&access,
&stack(Vec::new()),
&cwd,
/*home_dir*/ None,
@@ -416,7 +423,7 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
SkillDiscoveryMode::Recursive,
)
);
assert!(Arc::ptr_eq(&root.file_system, &LOCAL_FS));
assert_eq!(root.file_system.cache_key(), access.cache_key());
let outcome = load_and_merge_host_skill_roots(
roots,
@@ -439,6 +446,7 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
#[tokio::test]
async fn repo_ancestry_without_project_marker_does_not_walk_parents() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let outer = absolute(temp_dir.path().join("outer"));
let cwd = outer.join("nested/inner");
@@ -452,7 +460,7 @@ async fn repo_ancestry_without_project_marker_does_not_walk_parents() {
toml::toml! { project_root_markers = [".missing-skill-test-project"] }.into(),
)]);
let roots = repo_agents_skill_roots(
Some(Arc::clone(&LOCAL_FS)),
Some(&access),
&config_stack,
&cwd,
/*empty_remote_roots*/ None,
@@ -483,13 +491,14 @@ async fn time_empty_remote_repo_skill_discovery() {
inner: Arc::clone(&LOCAL_FS),
calls: Arc::clone(&calls),
});
let access = FileSystemEnvironmentAccessor::unrestricted(&remote);
let config = stack(Vec::new());
let cache = Mutex::new(super::EmptyRemoteRootsCache::default());
for (label, cache) in [("baseline", None), ("warm", Some(&cache))] {
// Prewarm before measuring admission, as session initialization does.
let setup = std::time::Instant::now();
assert!(
repo_agents_skill_roots(Some(remote.clone()), &config, &cwd, cache)
repo_agents_skill_roots(Some(&access), &config, &cwd, cache)
.await
.is_empty()
);
@@ -503,7 +512,7 @@ async fn time_empty_remote_repo_skill_discovery() {
for _ in 0..20 {
let start = std::time::Instant::now();
assert!(
repo_agents_skill_roots(Some(remote.clone()), &config, &cwd, cache)
repo_agents_skill_roots(Some(&access), &config, &cwd, cache)
.await
.is_empty()
);
@@ -525,10 +534,10 @@ async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
inner: Arc::clone(&LOCAL_FS),
calls: Arc::clone(&calls),
});
let access = FileSystemEnvironmentAccessor::unrestricted(&remote);
let cache = Mutex::new(super::EmptyRemoteRootsCache::default());
let config = stack(Vec::new());
let discover =
|| repo_agents_skill_roots(Some(Arc::clone(&remote)), &config, &cwd, Some(&cache));
let discover = || repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache));
assert!(discover().await.is_empty());
calls.watch_ready.notified().await;
// No clock deadline or polling during an idle connection.
@@ -536,6 +545,47 @@ async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
assert!(discover().await.is_empty());
assert_eq!(calls.paths.lock().unwrap().len(), 1);
// The same executor with different captured permissions must scan again.
let read_only = FileSystemEnvironmentAccessor::new(
&remote,
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::new_read_only_policy(),
PathUri::from_abs_path(&cwd),
)
.unwrap(),
);
assert!(
repo_agents_skill_roots(Some(&read_only), &config, &cwd, Some(&cache))
.await
.is_empty()
);
assert_eq!(calls.paths.lock().unwrap().len(), 2);
calls.watch_ready.notified().await;
assert!(discover().await.is_empty());
calls.watch_ready.notified().await;
// Read restrictions cannot use the executor's unsandboxed watch operation.
let restricted = FileSystemEnvironmentAccessor::new(
&remote,
FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(Vec::new()),
NetworkSandboxPolicy::Restricted,
),
PathUri::from_abs_path(&cwd),
),
);
assert!(
restricted
.watch_missing(
vec![PathUri::from_abs_path(&cwd.join(".agents/skills"))],
Some(PathUri::from_abs_path(&cwd.join(".git"))),
)
.await
.unwrap()
.is_none()
);
let skills = cwd.join(".agents/skills");
fs::create_dir_all(&skills).unwrap();
calls.watch.lock().unwrap().take();
@@ -563,17 +613,18 @@ async fn newly_created_roots_are_visible_before_watch_ack_or_without_watch_suppo
inner: Arc::clone(&LOCAL_FS),
calls,
});
let access = FileSystemEnvironmentAccessor::unrestricted(&remote);
let cache = Mutex::new(super::EmptyRemoteRootsCache::default());
let config = stack(Vec::new());
assert!(
repo_agents_skill_roots(Some(remote.clone()), &config, &cwd, Some(&cache))
repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache))
.await
.is_empty()
);
let skills = cwd.join(".agents/skills");
fs::create_dir_all(&skills).unwrap();
assert_eq!(
repo_agents_skill_roots(Some(remote), &config, &cwd, Some(&cache)).await[0].path,
repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache)).await[0].path,
skills
);
}
@@ -581,6 +632,7 @@ async fn newly_created_roots_are_visible_before_watch_ack_or_without_watch_suppo
#[tokio::test]
async fn repo_ancestry_stops_at_project_root_and_preserves_root_to_cwd_order() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let outer = absolute(temp_dir.path().join("outer"));
let repository = outer.join("repo");
@@ -593,7 +645,7 @@ async fn repo_ancestry_stops_at_project_root_and_preserves_root_to_cwd_order() {
let config_stack = stack(Vec::new());
let roots = repo_agents_skill_roots(
Some(Arc::clone(&LOCAL_FS)),
Some(&access),
&config_stack,
&nested,
/*empty_remote_roots*/ None,
@@ -614,6 +666,7 @@ async fn repo_ancestry_stops_at_project_root_and_preserves_root_to_cwd_order() {
#[tokio::test]
async fn resolved_project_layer_loads_skill_without_git_marker() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let workspace = absolute(temp_dir.path().join("workspace"));
let dot_codex = workspace.join(".codex");
@@ -623,7 +676,8 @@ async fn resolved_project_layer_loads_skill_without_git_marker() {
let config_stack = stack(vec![project_layer(&dot_codex)]);
let roots = resolve_skill_roots_with_home_dir(
Some(Arc::clone(&LOCAL_FS)),
Some(&access),
&access,
&config_stack,
&workspace,
/*home_dir*/ None,
@@ -649,6 +703,7 @@ async fn resolved_project_layer_loads_skill_without_git_marker() {
#[tokio::test]
async fn resolved_project_layer_loads_skill_when_cwd_is_file() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let repository = absolute(temp_dir.path().join("repo"));
let dot_codex = repository.join(".codex");
@@ -661,7 +716,8 @@ async fn resolved_project_layer_loads_skill_when_cwd_is_file() {
let config_stack = stack(vec![project_layer(&dot_codex)]);
let roots = resolve_skill_roots_with_home_dir(
Some(Arc::clone(&LOCAL_FS)),
Some(&access),
&access,
&config_stack,
&cwd,
/*home_dir*/ None,
@@ -754,9 +810,10 @@ async fn repo_ancestry_limits_concurrent_probes_and_preserves_order() {
calls.release.add_permits(expected_probes.len());
};
let config_stack = stack(Vec::new());
let access = FileSystemEnvironmentAccessor::unrestricted(&file_system);
let (roots, ()) = tokio::join!(
repo_agents_skill_roots(
Some(file_system),
Some(&access),
&config_stack,
&cwd,
/*empty_remote_roots*/ None
@@ -772,6 +829,7 @@ async fn repo_ancestry_limits_concurrent_probes_and_preserves_order() {
#[tokio::test]
async fn resolved_config_and_repo_roots_preserve_order_and_dedupe_paths_not_names() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp_dir = TempDir::new().expect("temp dir");
let home_folder = absolute(temp_dir.path().join("home"));
let codex_home = home_folder.join("codex");
@@ -821,7 +879,8 @@ async fn resolved_config_and_repo_roots_preserve_order_and_dedupe_paths_not_name
]);
let roots = resolve_skill_roots_with_home_dir(
Some(Arc::clone(&LOCAL_FS)),
Some(&access),
&access,
&config_stack,
&cwd,
Some(&home_folder),
codex-rs/ext/skills/src/host_service.rs
+43 -43
View File
@@ -1,17 +1,17 @@
use std::collections::HashMap;
use std::collections::VecDeque;
use std::hash::Hash;
use std::hash::Hasher;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::RwLock;
use std::sync::Weak;
use codex_config::ConfigLayerStack;
use codex_config::SkillConfigRules;
use codex_config::bundled_skills_enabled_from_stack;
use codex_config::skill_config_rules_from_stack;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessKey;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -109,11 +109,14 @@ impl HostSkillsRequest<'_> {
force_reload: bool,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> HostSkillsSnapshot {
let repository = fs.as_ref().map(FileSystemEnvironmentAccessor::unrestricted);
self.service
.snapshot_for_cwd_with_root_snapshots(
input,
force_reload,
fs,
repository
.as_ref()
.map(|access| access as &dyn EnvironmentAccess),
Some(&self.root_snapshots),
)
.await
@@ -181,10 +184,12 @@ impl HostSkillsService {
pub async fn snapshot_for_config(
&self,
input: &HostSkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
repository: Option<&dyn EnvironmentAccess>,
) -> HostSkillsSnapshot {
// User and plugin roots retain host authority independently of the selected executor.
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let roots = self
.skill_roots_for_config(input, fs, Some(&self.empty_remote_roots))
.skill_roots_for_config(input, repository, &host, Some(&self.empty_remote_roots))
.await;
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let cache_key = config_skills_cache_key(
@@ -212,26 +217,35 @@ impl HostSkillsService {
input: &HostSkillsLoadInput,
fs: Arc<dyn ExecutorFileSystem>,
) -> Vec<AbsolutePathBuf> {
self.skill_roots_for_config(input, Some(fs), /*empty_remote_roots*/ None)
.await
.into_iter()
.filter(|root| root.plugin_identity().is_none() && root.scope != SkillScope::System)
.map(|root| root.path)
.collect()
let repository = FileSystemEnvironmentAccessor::unrestricted(&fs);
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
self.skill_roots_for_config(
input,
Some(&repository),
&host,
/*empty_remote_roots*/ None,
)
.await
.into_iter()
.filter(|root| root.plugin_identity().is_none() && root.scope != SkillScope::System)
.map(|root| root.path)
.collect()
}
async fn skill_roots_for_config(
async fn skill_roots_for_config<'a>(
&self,
input: &HostSkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>,
repository: Option<&'a dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot> {
) -> Vec<HostSkillRoot<'a>> {
let bundled_skills_enabled = bundled_skills_enabled_from_stack(&input.config_layer_stack);
if bundled_skills_enabled {
self.ensure_system_skills_installed();
}
let mut roots = resolve_skill_roots(
fs,
repository,
host,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
@@ -249,14 +263,15 @@ impl HostSkillsService {
&self,
input: &HostSkillsLoadInput,
force_reload: bool,
fs: Option<Arc<dyn ExecutorFileSystem>>,
repository: Option<&dyn EnvironmentAccess>,
request_root_snapshots: Option<&RequestSkillRootSnapshots>,
) -> HostSkillsSnapshot {
let bundled_skills_enabled = bundled_skills_enabled_from_stack(&input.config_layer_stack);
if bundled_skills_enabled {
self.ensure_system_skills_installed();
}
let use_cwd_cache = fs.is_some();
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let use_cwd_cache = repository.is_some();
let cache_snapshot_by_cwd = use_cwd_cache && input.effective_skill_roots.is_empty();
if cache_snapshot_by_cwd
&& !force_reload
@@ -272,7 +287,8 @@ impl HostSkillsService {
.clear();
}
let mut roots = resolve_skill_roots(
fs.clone(),
repository,
&host,
&input.config_layer_stack,
&input.cwd,
input.effective_skill_roots.clone(),
@@ -318,7 +334,7 @@ impl HostSkillsService {
async fn snapshot_for_skill_roots(
&self,
input: &HostSkillsLoadInput,
roots: Vec<HostSkillRoot>,
roots: Vec<HostSkillRoot<'_>>,
skill_config_rules: &SkillConfigRules,
cache_key: ConfigSkillsCacheKey,
force_reload: bool,
@@ -366,7 +382,7 @@ impl HostSkillsService {
async fn build_skill_outcome(
&self,
input: &HostSkillsLoadInput,
roots: Vec<HostSkillRoot>,
roots: Vec<HostSkillRoot<'_>>,
skill_config_rules: &SkillConfigRules,
request_root_snapshots: Option<&RequestSkillRootSnapshots>,
) -> SkillLoadOutcome {
@@ -441,10 +457,11 @@ impl SkillRootLoader<PluginSkillRoot> for HostSkillsService {
request: SkillRootLoadRequest<PluginSkillRoot>,
) -> SkillLoadFuture<'_, LoadedSkills> {
Box::pin(async move {
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let roots = request
.roots
.into_iter()
.map(|root| HostSkillRoot::plugin(root, Arc::clone(&LOCAL_FS)))
.map(|root| HostSkillRoot::plugin(root, &host))
.collect();
let outcome = load_and_merge_host_skill_roots(
roots,
@@ -474,28 +491,11 @@ pub(crate) struct ConfigSkillRootCacheKey {
scope_rank: u8,
plugin_identity: Option<PluginIdentity>,
plugin_namespace: Option<String>,
file_system: FileSystemIdentity,
}
#[derive(Debug, Clone)]
struct FileSystemIdentity(Weak<dyn ExecutorFileSystem>);
impl PartialEq for FileSystemIdentity {
fn eq(&self, other: &Self) -> bool {
Weak::ptr_eq(&self.0, &other.0)
}
}
impl Eq for FileSystemIdentity {}
impl Hash for FileSystemIdentity {
fn hash<H: Hasher>(&self, state: &mut H) {
(self.0.as_ptr() as *const ()).hash(state);
}
access: EnvironmentAccessKey,
}
fn config_skills_cache_key(
roots: &[HostSkillRoot],
roots: &[HostSkillRoot<'_>],
skill_config_rules: &SkillConfigRules,
plugin_skill_snapshots: Option<&SkillRootSnapshots<PluginSkillRoot>>,
) -> ConfigSkillsCacheKey {
@@ -508,7 +508,7 @@ fn config_skills_cache_key(
}
}
pub(crate) fn config_skill_root_cache_key(root: &HostSkillRoot) -> ConfigSkillRootCacheKey {
pub(crate) fn config_skill_root_cache_key(root: &HostSkillRoot<'_>) -> ConfigSkillRootCacheKey {
let scope_rank = match root.scope {
SkillScope::Repo => 0,
SkillScope::User => 1,
@@ -520,7 +520,7 @@ pub(crate) fn config_skill_root_cache_key(root: &HostSkillRoot) -> ConfigSkillRo
scope_rank,
plugin_identity: root.plugin_identity().cloned(),
plugin_namespace: root.plugin_namespace().map(str::to_string),
file_system: FileSystemIdentity(Arc::downgrade(&root.file_system)),
access: root.file_system.cache_key(),
}
}
codex-rs/ext/skills/src/host_service_tests.rs
+31 -7
View File
@@ -24,6 +24,7 @@ use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::Weak;
use tempfile::TempDir;
#[derive(Default)]
@@ -175,7 +176,10 @@ async fn skills_for_config_with_stack(
config_layer_stack.clone(),
);
skills_service
.snapshot_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS)))
.snapshot_for_config(
&skills_input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS)),
)
.await
.outcome()
.clone()
@@ -248,7 +252,10 @@ async fn skills_for_config_bounds_plugin_generations_and_preserves_live_snapshot
.clone()
.with_plugin_skill_snapshots(Some(SkillRootSnapshots::new(owner)));
let snapshot = skills_service
.snapshot_for_config(&input, Some(Arc::clone(&LOCAL_FS)))
.snapshot_for_config(
&input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS)),
)
.await;
if generation == 0 {
@@ -260,7 +267,12 @@ async fn skills_for_config_bounds_plugin_generations_and_preserves_live_snapshot
if generation == CONFIG_SKILLS_CACHE_CAPACITY - 1 {
let (input, snapshot) = recent.as_ref().expect("first generation");
let reused = skills_service
.snapshot_for_config(input, Some(Arc::clone(&LOCAL_FS)))
.snapshot_for_config(
input,
Some(
&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS),
),
)
.await;
assert!(std::ptr::eq(snapshot.outcome(), reused.outcome()));
}
@@ -350,7 +362,10 @@ async fn snapshot_for_config_merges_extension_host_and_legacy_plugin_roots() {
);
let snapshot = skills_service
.snapshot_for_config(&input, Some(Arc::clone(&LOCAL_FS)))
.snapshot_for_config(
&input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS)),
)
.await;
let skills = snapshot
.outcome()
@@ -514,7 +529,10 @@ async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() {
)
.with_plugin_skill_snapshots(Some(test_plugin_skill_snapshots()));
let plugin_snapshot = skills_service
.snapshot_for_config(&plugin_input, Some(Arc::clone(&LOCAL_FS)))
.snapshot_for_config(
&plugin_input,
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS)),
)
.await;
fs::write(
&skill_path,
@@ -799,7 +817,12 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
let other_file_system: Arc<dyn codex_exec_server::ExecutorFileSystem> =
Arc::new(codex_exec_server::LocalFileSystem::unsandboxed());
let other_snapshot = skills_service
.snapshot_for_config(&skills_input, Some(other_file_system))
.snapshot_for_config(
&skills_input,
Some(
&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&other_file_system),
),
)
.await;
assert!(!std::ptr::eq(snapshot.outcome(), other_snapshot.outcome()));
}
@@ -909,8 +932,9 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
.clone()
.with_plugin_skill_snapshots(Some(test_plugin_skill_snapshots()));
let request = skills_service.for_request();
let access = codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let (config_snapshot, snapshot_a) = tokio::join!(
skills_service.snapshot_for_config(&config_input, Some(Arc::clone(&LOCAL_FS))),
skills_service.snapshot_for_config(&config_input, Some(&access)),
request.snapshot_for_cwd(
&base_input,
/*force_reload*/ false,
codex-rs/ext/skills/src/loader/discovery.rs
+1 -1
View File
@@ -15,7 +15,7 @@ use super::SKILLS_METADATA_DIR;
use super::SKILLS_METADATA_FILENAME;
const MAX_SKILLS_ENTRIES_PER_ROOT: usize = 20_000;
pub(super) const MAX_CONCURRENT_SKILL_LOADS: usize = 64;
pub(crate) const MAX_CONCURRENT_SKILL_LOADS: usize = 64;
pub(super) enum DirectorySymlinkPolicy {
Follow,
codex-rs/ext/skills/src/loader/host.rs
+20 -32
View File
@@ -2,9 +2,8 @@ use std::collections::HashMap;
use std::sync::Arc;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessExt;
use codex_exec_server::EnvironmentAccessKey;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::GetMetadataOptions;
use codex_exec_server::ReadFileOptions;
use codex_protocol::protocol::SkillScope;
@@ -38,10 +37,10 @@ use super::metadata::validate_len;
use super::namespace::SkillNamespaceResolver;
/// A resolved host skill root ready for filesystem discovery.
pub(crate) struct HostSkillRoot {
pub(crate) struct HostSkillRoot<'a> {
pub(crate) path: AbsolutePathBuf,
pub(crate) scope: SkillScope,
pub(crate) file_system: Arc<dyn ExecutorFileSystem>,
pub(crate) file_system: &'a dyn EnvironmentAccess,
plugin: Option<PluginSkillRootContext>,
}
@@ -52,11 +51,11 @@ struct PluginSkillRootContext {
discovery_mode: SkillDiscoveryMode,
}
impl HostSkillRoot {
impl<'a> HostSkillRoot<'a> {
pub(crate) fn host(
path: AbsolutePathBuf,
scope: SkillScope,
file_system: Arc<dyn ExecutorFileSystem>,
file_system: &'a dyn EnvironmentAccess,
) -> Self {
Self {
path,
@@ -66,7 +65,7 @@ impl HostSkillRoot {
}
}
pub(crate) fn plugin(root: PluginSkillRoot, file_system: Arc<dyn ExecutorFileSystem>) -> Self {
pub(crate) fn plugin(root: PluginSkillRoot, file_system: &'a dyn EnvironmentAccess) -> Self {
Self {
path: root.path,
scope: SkillScope::User,
@@ -129,10 +128,9 @@ struct ResolvedDiscoveredSkill {
path_uri: PathUri,
}
pub(crate) async fn load_host_skill_root(root: HostSkillRoot) -> HostSkillRootSnapshot {
pub(crate) async fn load_host_skill_root(root: HostSkillRoot<'_>) -> HostSkillRootSnapshot {
let is_agent_plugin = root.discovery_mode() == SkillDiscoveryMode::DirectChildren;
let canonical_root =
canonicalize_for_skill_identity(root.file_system.as_ref(), &root.path).await;
let canonical_root = canonicalize_for_skill_identity(root.file_system, &root.path).await;
let (skills, skill_discovery_path_by_path, errors) =
load_skills_under_root(&root, &canonical_root).await;
HostSkillRootSnapshot {
@@ -140,23 +138,20 @@ pub(crate) async fn load_host_skill_root(root: HostSkillRoot) -> HostSkillRootSn
skills,
skill_discovery_path_by_path,
errors,
file_system: FileSystemEnvironmentAccessor::unrestricted(&root.file_system).cache_key(),
file_system: root.file_system.cache_key(),
is_agent_plugin,
}
}
async fn load_skills_under_root(
skill_root: &HostSkillRoot,
skill_root: &HostSkillRoot<'_>,
root: &AbsolutePathBuf,
) -> (
Vec<SkillMetadata>,
Arc<HashMap<PathUri, PathUri>>,
Vec<SkillError>,
) {
let file_system = skill_root.file_system.as_ref();
// TODO(anp): Bind discovery to turn permissions when host skill roots accept an accessor;
// until then, keep using the same unrestricted filesystem that supplied the root.
let discovery_access = FileSystemEnvironmentAccessor::unrestricted(&skill_root.file_system);
let file_system = skill_root.file_system;
let plugin_identity = skill_root.plugin_identity();
let plugin_root = match skill_root.plugin_root() {
Some(plugin_root) => Some(canonicalize_for_skill_identity(file_system, plugin_root).await),
@@ -173,7 +168,7 @@ async fn load_skills_under_root(
mut namespace_roots,
warnings,
} = discover_skills(
&discovery_access,
file_system,
&PathUri::from_abs_path(root),
SkillDiscoveryOptions {
directory_symlinks,
@@ -193,10 +188,7 @@ async fn load_skills_under_root(
let resolved_plugin_root = plugin_root.as_ref();
let resolved_skills = futures::stream::iter(skills)
.map(|skill| async move {
let path_uri = match file_system
.canonicalize(&skill.path, /*sandbox*/ None)
.await
{
let path_uri = match file_system.canonicalize(&skill.path).await {
Ok(path) => path,
Err(error) if discovery_mode == SkillDiscoveryMode::DirectChildren => {
error!(
@@ -228,11 +220,7 @@ async fn load_skills_under_root(
return None;
}
match file_system
.get_metadata(
&path_uri,
GetMetadataOptions::default(),
/*sandbox*/ None,
)
.get_metadata(&path_uri, GetMetadataOptions::default())
.await
{
Ok(metadata) if metadata.is_file => {}
@@ -276,7 +264,7 @@ async fn load_skills_under_root(
Some(namespace) => SkillNamespaceResolver::with_provided_namespace(namespace),
None => {
SkillNamespaceResolver::discover(
&discovery_access,
file_system,
&root_uri,
&skill_paths,
plugin_roots,
@@ -346,7 +334,7 @@ async fn load_skills_under_root(
}
async fn parse_skill_file(
file_system: &dyn ExecutorFileSystem,
file_system: &dyn EnvironmentAccess,
skill: &DiscoveredSkill,
path: &AbsolutePathBuf,
path_uri: &PathUri,
@@ -365,7 +353,7 @@ async fn parse_skill_file(
}
.unwrap_or(SkillMetadataDiscovery::Absent);
let (contents, loaded_metadata) = tokio::join!(
file_system.read_file_text(path_uri, ReadFileOptions::default(), /*sandbox*/ None,),
file_system.read_file_text(path_uri, ReadFileOptions::default(),),
load_host_skill_metadata(file_system, path, &metadata, plugin_root),
);
let contents = contents.map_err(|error| format!("failed to read file: {error}"))?;
@@ -407,13 +395,13 @@ fn default_skill_name(path: &AbsolutePathBuf) -> String {
.unwrap_or_else(|| "skill".to_string())
}
async fn canonicalize_for_skill_identity(
file_system: &dyn ExecutorFileSystem,
pub(crate) async fn canonicalize_for_skill_identity(
file_system: &dyn EnvironmentAccess,
path: &AbsolutePathBuf,
) -> AbsolutePathBuf {
let path_uri = PathUri::from_abs_path(path);
file_system
.canonicalize(&path_uri, /*sandbox*/ None)
.canonicalize(&path_uri)
.await
.and_then(|path| path.to_abs_path())
.unwrap_or_else(|_| path.clone())
codex-rs/ext/skills/src/loader/host_io_tests.rs
+3 -1
View File
@@ -3,6 +3,7 @@ use std::os::unix::fs::symlink;
use std::sync::Arc;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::SkillScope;
use codex_skills::SkillMetadata;
@@ -55,12 +56,13 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
ManifestMetadataBehavior::Immediate,
));
let file_system: Arc<dyn ExecutorFileSystem> = recording.clone();
let access = FileSystemEnvironmentAccessor::unrestricted(&file_system);
let root_scan_slots = Semaphore::new(MAX_CONCURRENT_ROOT_SCANS);
let future = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
host_root.abs(),
SkillScope::User,
file_system,
&access,
)],
&root_scan_slots,
/*restriction_product*/ None,
codex-rs/ext/skills/src/loader/host_merge.rs
+16 -19
View File
@@ -2,9 +2,7 @@ use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessKey;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_skills::LoadedSkillRoot;
@@ -19,6 +17,7 @@ use tokio::sync::Semaphore;
use super::HostSkillRoot;
use super::MAX_CONCURRENT_ROOT_SCANS;
use super::canonicalize_for_skill_identity;
use super::host::HostSkillRootSnapshot;
use super::load_host_skill_root;
use crate::SkillLoadOutcome;
@@ -26,7 +25,7 @@ use crate::host_service::RequestSkillRootSnapshots;
use crate::host_service::config_skill_root_cache_key;
pub(crate) async fn load_and_merge_host_skill_roots(
roots: Vec<HostSkillRoot>,
roots: Vec<HostSkillRoot<'_>>,
root_scan_slots: &Semaphore,
restriction_product: Option<Product>,
plugin_skill_snapshots: Option<&SkillRootSnapshots<PluginSkillRoot>>,
@@ -42,7 +41,7 @@ pub(crate) async fn load_and_merge_host_skill_roots(
}
pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
roots: Vec<HostSkillRoot>,
roots: Vec<HostSkillRoot<'_>>,
root_scan_slots: &Semaphore,
restriction_product: Option<Product>,
plugin_skill_snapshots: Option<&SkillRootSnapshots<PluginSkillRoot>>,
@@ -54,15 +53,16 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
.map(|plugin_root| (plugin_root.clone(), Arc::new(OnceCell::new())))
.collect::<HashMap<_, _>>();
let migrated_command_roots = &migrated_command_roots_by_plugin;
let mut indexed_snapshots = futures::stream::iter(roots.into_iter().enumerate())
.map(|(root_index, root)| async move {
let mut loads = Vec::with_capacity(roots.len());
for (root_index, root) in roots.into_iter().enumerate() {
loads.push(async move {
let plugin_root = root.plugin_root().cloned();
let migrated_command_root = plugin_root
.as_ref()
.and_then(|plugin_root| migrated_command_roots.get(plugin_root))
.cloned();
let plugin_skill_root = root.plugin_skill_root();
let file_system = Arc::clone(&root.file_system);
let file_system = root.file_system;
let request_snapshot = if plugin_skill_root.is_none() {
request_root_snapshots.map(|snapshots| {
Arc::clone(
@@ -90,8 +90,7 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
skills: snapshot.skills,
skill_discovery_path_by_path: snapshot.skill_discovery_path_by_path,
errors: snapshot.errors,
file_system: FileSystemEnvironmentAccessor::unrestricted(&file_system)
.cache_key(),
file_system: file_system.cache_key(),
is_agent_plugin: snapshot.is_agent_plugin,
},
None => {
@@ -126,15 +125,11 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
(Some(plugin_root), Some(migrated_command_root)) => Some(
migrated_command_root
.get_or_init(|| async {
let migrated_root = migrated_command_skills_root(plugin_root);
file_system
.canonicalize(
&PathUri::from_abs_path(&migrated_root),
/*sandbox*/ None,
)
.await
.and_then(|path| path.to_abs_path())
.unwrap_or(migrated_root)
canonicalize_for_skill_identity(
file_system,
&migrated_command_skills_root(plugin_root),
)
.await
})
.await
.clone(),
@@ -142,7 +137,9 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
_ => None,
};
(root_index, migrated_command_root, snapshot)
})
});
}
let mut indexed_snapshots = futures::stream::iter(loads)
.buffer_unordered(MAX_CONCURRENT_ROOT_SCANS)
.collect::<Vec<_>>()
.await;
codex-rs/ext/skills/src/loader/host_merge_tests.rs
+21 -21
View File
@@ -5,6 +5,7 @@ use std::sync::Mutex;
use std::sync::atomic::Ordering;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -61,6 +62,7 @@ struct PluginSkillsFixture {
native_root: AbsolutePathBuf,
migrated_root: AbsolutePathBuf,
identity: PluginIdentity,
access: FileSystemEnvironmentAccessor<'static>,
}
impl PluginSkillsFixture {
@@ -88,6 +90,7 @@ impl PluginSkillsFixture {
plugin_path,
native_root,
migrated_root,
access: FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS),
identity: PluginIdentity {
plugin_id: "sample@test".to_string(),
remote_plugin_id: Some("remote-sample".to_string()),
@@ -95,7 +98,7 @@ impl PluginSkillsFixture {
}
}
fn root(&self, path: AbsolutePathBuf) -> HostSkillRoot {
fn root(&self, path: AbsolutePathBuf) -> HostSkillRoot<'_> {
HostSkillRoot::plugin(
PluginSkillRoot {
path,
@@ -104,11 +107,11 @@ impl PluginSkillsFixture {
plugin_root: self.plugin_path.clone(),
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&self.access,
)
}
fn roots(&self) -> Vec<HostSkillRoot> {
fn roots(&self) -> Vec<HostSkillRoot<'_>> {
[self.migrated_root.clone(), self.native_root.clone()]
.into_iter()
.map(|path| self.root(path))
@@ -324,6 +327,7 @@ async fn recognizes_symlinked_migrated_command_roots() {
#[tokio::test]
async fn host_root_scans_wait_for_shared_capacity() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp = TempDir::new().expect("temp dir");
let root = AbsolutePathBuf::from_absolute_path(temp.path()).expect("absolute skill root");
let root_scan_slots = Semaphore::new(/*permits*/ 1);
@@ -331,11 +335,7 @@ async fn host_root_scans_wait_for_shared_capacity() {
.try_acquire()
.expect("root scan slot should be available");
let load = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
root,
SkillScope::Repo,
Arc::clone(&LOCAL_FS),
)],
vec![HostSkillRoot::host(root, SkillScope::Repo, &access)],
&root_scan_slots,
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
@@ -352,14 +352,15 @@ async fn host_root_scans_wait_for_shared_capacity() {
#[tokio::test]
async fn duplicate_host_root_paths_preserve_first_root_scope() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp = TempDir::new().expect("temp dir");
let root = AbsolutePathBuf::from_absolute_path(temp.path()).expect("absolute skill root");
write_skill(&root, "demo", "demo", "Shared skill");
let outcome = load_and_merge_host_skill_roots(
vec![
HostSkillRoot::host(root.clone(), SkillScope::Repo, Arc::clone(&LOCAL_FS)),
HostSkillRoot::host(root.clone(), SkillScope::User, Arc::clone(&LOCAL_FS)),
HostSkillRoot::host(root.clone(), SkillScope::Repo, &access),
HostSkillRoot::host(root.clone(), SkillScope::User, &access),
],
&Semaphore::new(/*permits*/ 2),
/*restriction_product*/ None,
@@ -383,6 +384,7 @@ async fn duplicate_host_root_paths_preserve_first_root_scope() {
#[cfg(unix)]
#[tokio::test]
async fn deduplicated_symlinked_skill_preserves_first_discovery_path() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let source = TempDir::new().expect("source temp dir");
let first = TempDir::new().expect("first root temp dir");
let second = TempDir::new().expect("second root temp dir");
@@ -403,8 +405,8 @@ async fn deduplicated_symlinked_skill_preserves_first_discovery_path() {
let outcome = load_and_merge_host_skill_roots(
vec![
HostSkillRoot::host(first_root.clone(), SkillScope::Repo, Arc::clone(&LOCAL_FS)),
HostSkillRoot::host(second_root, SkillScope::User, Arc::clone(&LOCAL_FS)),
HostSkillRoot::host(first_root.clone(), SkillScope::Repo, &access),
HostSkillRoot::host(second_root, SkillScope::User, &access),
],
&Semaphore::new(/*permits*/ 2),
/*restriction_product*/ None,
@@ -461,11 +463,12 @@ async fn merges_host_root_results_in_input_order_when_scans_finish_out_of_order(
));
let recording = Arc::new(recording);
let file_system: Arc<dyn ExecutorFileSystem> = recording.clone();
let host_roots = roots
.iter()
.map(|root| HostSkillRoot::host(root.clone().abs(), SkillScope::User, file_system.clone()))
.collect::<Vec<_>>();
let load = tokio::spawn(async move {
let access = FileSystemEnvironmentAccessor::unrestricted(&file_system);
let host_roots = roots
.iter()
.map(|root| HostSkillRoot::host(root.clone().abs(), SkillScope::User, &access))
.collect::<Vec<_>>();
load_and_merge_host_skill_roots(
host_roots,
&Semaphore::new(MAX_CONCURRENT_ROOT_SCANS),
@@ -512,6 +515,7 @@ async fn merges_host_root_results_in_input_order_when_scans_finish_out_of_order(
/// Equal-name skills retain native path order so URI escaping does not reorder the catalog.
#[tokio::test]
async fn same_name_skills_preserve_native_path_order() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let temp = TempDir::new().expect("temp dir");
let root = dunce::canonicalize(temp.path())
.expect("canonical skill root")
@@ -520,11 +524,7 @@ async fn same_name_skills_preserve_native_path_order() {
write_skill(&root, "a", "demo", "ASCII path");
let outcome = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
root.clone(),
SkillScope::User,
Arc::clone(&LOCAL_FS),
)],
vec![HostSkillRoot::host(root.clone(), SkillScope::User, &access)],
&Semaphore::new(/*permits*/ 1),
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
codex-rs/ext/skills/src/loader/host_tests.rs
+39 -19
View File
@@ -1,6 +1,7 @@
use std::fs;
use std::sync::Arc;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -41,11 +42,15 @@ fn write_metadata(root: &TempDir, directory: &str, contents: &str) {
fs::write(metadata_dir.join("openai.yaml"), contents).expect("write metadata");
}
fn root_for(temp_dir: &TempDir, scope: SkillScope) -> HostSkillRoot {
fn root_for<'a>(
temp_dir: &TempDir,
scope: SkillScope,
access: &'a dyn EnvironmentAccess,
) -> HostSkillRoot<'a> {
HostSkillRoot::host(
AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute root"),
scope,
Arc::clone(&LOCAL_FS),
access,
)
}
@@ -53,6 +58,7 @@ struct PluginSkillFixture {
root: TempDir,
plugin_root: AbsolutePathBuf,
skill_path: AbsolutePathBuf,
access: FileSystemEnvironmentAccessor<'static>,
}
impl PluginSkillFixture {
@@ -71,6 +77,7 @@ impl PluginSkillFixture {
root,
plugin_root,
skill_path,
access: FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS),
}
}
@@ -85,7 +92,7 @@ impl PluginSkillFixture {
write_metadata(&self.root, "skills/send-message", contents);
}
fn host_root(&self) -> HostSkillRoot {
fn host_root(&self) -> HostSkillRoot<'_> {
HostSkillRoot::plugin(
PluginSkillRoot {
path: self.plugin_root.join("skills"),
@@ -97,13 +104,14 @@ impl PluginSkillFixture {
plugin_root: self.plugin_root.clone(),
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&self.access,
)
}
}
#[tokio::test]
async fn loads_host_frontmatter_dependencies_and_policy() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_path = write_skill(
&root,
@@ -126,7 +134,7 @@ policy:
"##,
);
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(
@@ -161,11 +169,12 @@ policy:
#[tokio::test]
async fn invalid_optional_metadata_fails_open() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_path = write_skill(&root, "demo", "name: demo\ndescription: Demo skill");
write_metadata(&root, "demo", "interface: [not-an-interface]");
let snapshot = load_host_skill_root(root_for(&root, SkillScope::Repo)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::Repo, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(
@@ -187,6 +196,7 @@ async fn invalid_optional_metadata_fails_open() {
#[tokio::test]
async fn loads_host_interface_metadata_and_local_asset_paths() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_path = write_skill(&root, "demo", "name: demo\ndescription: Demo skill");
fs::create_dir_all(root.path().join("demo/assets")).expect("create assets");
@@ -203,7 +213,7 @@ async fn loads_host_interface_metadata_and_local_asset_paths() {
"##,
);
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User, &access)).await;
let skill_dir = skill_path.parent().expect("skill parent");
assert_eq!(snapshot.errors, Vec::new());
@@ -314,6 +324,7 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() {
#[tokio::test]
async fn rejects_interface_fields_that_escape_or_fail_validation() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_path = write_skill(&root, "demo", "name: demo\ndescription: Demo skill");
write_metadata(
@@ -322,7 +333,7 @@ async fn rejects_interface_fields_that_escape_or_fail_validation() {
"interface:\n icon_small: ../outside.svg\n brand_color: blue\n",
);
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(
@@ -344,6 +355,7 @@ async fn rejects_interface_fields_that_escape_or_fail_validation() {
#[tokio::test]
async fn skips_hidden_host_skills() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let visible_path = write_skill(
&root,
@@ -352,7 +364,7 @@ async fn skips_hidden_host_skills() {
);
write_skill(&root, ".hidden", "name: hidden\ndescription: Hidden skill");
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(snapshot.skills.len(), 1);
@@ -364,6 +376,7 @@ async fn skips_hidden_host_skills() {
#[tokio::test]
async fn discovers_nested_plugin_namespace_without_plugin_identity() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let plugin_manifest = root.path().join("nested/.codex-plugin/plugin.json");
fs::create_dir_all(plugin_manifest.parent().expect("plugin manifest parent"))
@@ -375,7 +388,7 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() {
"name: demo\ndescription: Demo skill",
);
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User)).await;
let snapshot = load_host_skill_root(root_for(&root, SkillScope::User, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(
@@ -397,6 +410,7 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() {
#[tokio::test]
async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let plugin_namespace = "p".repeat(MAX_NAME_LEN);
let skill_name = "s".repeat(MAX_NAME_LEN);
@@ -418,7 +432,7 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
plugin_root,
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -442,6 +456,7 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
#[tokio::test]
async fn plugin_root_rejects_overlong_qualified_skill_name() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_name = "s".repeat(MAX_NAME_LEN);
write_skill(
@@ -462,7 +477,7 @@ async fn plugin_root_rejects_overlong_qualified_skill_name() {
plugin_root,
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -477,6 +492,7 @@ async fn plugin_root_rejects_overlong_qualified_skill_name() {
#[tokio::test]
async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let skill_path = write_skill(
&root,
@@ -512,7 +528,7 @@ async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy
plugin_root,
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -549,6 +565,7 @@ async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy
#[tokio::test]
async fn direct_child_plugin_root_ignores_nested_skills() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let direct_path = write_skill(
&root,
@@ -573,7 +590,7 @@ async fn direct_child_plugin_root_ignores_nested_skills() {
plugin_root,
discovery_mode: SkillDiscoveryMode::DirectChildren,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -598,6 +615,7 @@ async fn direct_child_plugin_root_ignores_nested_skills() {
#[cfg(unix)]
#[tokio::test]
async fn direct_child_plugin_root_skips_skills_resolving_outside_plugin_root() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let outside_root = TempDir::new().expect("outside temp dir");
write_skill(
@@ -624,7 +642,7 @@ async fn direct_child_plugin_root_skips_skills_resolving_outside_plugin_root() {
plugin_root,
discovery_mode: SkillDiscoveryMode::DirectChildren,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -635,6 +653,7 @@ async fn direct_child_plugin_root_skips_skills_resolving_outside_plugin_root() {
#[cfg(unix)]
#[tokio::test]
async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("temp dir");
let target = TempDir::new().expect("target temp dir");
let target_skill = write_skill(&target, "demo", "name: demo\ndescription: Symlinked skill");
@@ -654,7 +673,7 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
plugin_root,
discovery_mode: SkillDiscoveryMode::Recursive,
},
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -687,6 +706,7 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
#[cfg(unix)]
#[tokio::test]
async fn follows_directory_symlinks_except_for_system_scope() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
use std::os::unix::fs::symlink;
let root = TempDir::new().expect("temp dir");
@@ -695,7 +715,7 @@ async fn follows_directory_symlinks_except_for_system_scope() {
symlink(target.path().join("demo"), root.path().join("alias")).expect("create symlink");
for scope in [SkillScope::User, SkillScope::Repo, SkillScope::Admin] {
let snapshot = load_host_skill_root(root_for(&root, scope)).await;
let snapshot = load_host_skill_root(root_for(&root, scope, &access)).await;
assert_eq!(snapshot.errors, Vec::new());
assert_eq!(snapshot.skills.len(), 1);
@@ -716,7 +736,7 @@ async fn follows_directory_symlinks_except_for_system_scope() {
);
}
let system_snapshot = load_host_skill_root(root_for(&root, SkillScope::System)).await;
let system_snapshot = load_host_skill_root(root_for(&root, SkillScope::System, &access)).await;
assert_eq!(system_snapshot.errors, Vec::new());
assert_eq!(system_snapshot.skills, Vec::new());
}
codex-rs/ext/skills/src/loader/metadata.rs
+5 -8
View File
@@ -1,6 +1,7 @@
use std::io;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessExt;
use codex_exec_server::GetMetadataOptions;
use codex_exec_server::ReadFileOptions;
use codex_skills::ParsedSkillMetadata;
@@ -24,7 +25,7 @@ pub(super) struct LoadedSkillMetadata {
}
pub(super) async fn load_host_skill_metadata(
file_system: &dyn ExecutorFileSystem,
file_system: &dyn EnvironmentAccess,
skill_path: &AbsolutePathBuf,
metadata: &SkillMetadataDiscovery,
plugin_root: Option<&AbsolutePathBuf>,
@@ -38,7 +39,7 @@ pub(super) async fn load_host_skill_metadata(
SkillMetadataDiscovery::Absent => return LoadedSkillMetadata::default(),
SkillMetadataDiscovery::Probe(path) => {
match file_system
.get_metadata(path, GetMetadataOptions::default(), /*sandbox*/ None)
.get_metadata(path, GetMetadataOptions::default())
.await
{
Ok(metadata) if metadata.is_file => {}
@@ -59,11 +60,7 @@ pub(super) async fn load_host_skill_metadata(
};
let contents = match file_system
.read_file_text(
metadata_path,
ReadFileOptions::default(),
/*sandbox*/ None,
)
.read_file_text(metadata_path, ReadFileOptions::default())
.await
{
Ok(contents) => contents,
codex-rs/ext/skills/src/loader/mod.rs
+1
View File
@@ -11,6 +11,7 @@ pub(crate) use environment::load_environment_skills_from_discovery;
pub(crate) use environment::load_environment_skills_from_root;
pub(crate) use host::HostSkillRoot;
pub(crate) use host::HostSkillRootSnapshot;
pub(crate) use host::canonicalize_for_skill_identity;
pub(crate) use host::load_host_skill_root;
pub(crate) use host_merge::load_and_merge_host_skill_roots;
pub(crate) use host_merge::load_and_merge_host_skill_roots_with_request_snapshots;
codex-rs/ext/skills/src/loader/namespace_tests.rs
+7 -8
View File
@@ -1,9 +1,3 @@
use std::collections::HashSet;
use std::fs;
use std::path::Path;
#[cfg(unix)]
use std::sync::Arc;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
#[cfg(unix)]
@@ -12,6 +6,9 @@ use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::fs;
use std::path::Path;
use tempfile::TempDir;
#[cfg(unix)]
@@ -115,6 +112,7 @@ async fn nearest_valid_manifest_overrides_or_falls_back_to_outer_namespace() {
#[cfg(unix)]
#[tokio::test]
async fn symlinked_external_directories_do_not_inherit_plugin_namespace() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("root temp dir");
let external = TempDir::new().expect("external temp dir");
let plugin_root = root.path().join("outer");
@@ -128,7 +126,7 @@ async fn symlinked_external_directories_do_not_inherit_plugin_namespace() {
let snapshot = load_host_skill_root(HostSkillRoot::host(
AbsolutePathBuf::from_absolute_path(&skills_root).expect("absolute skills root"),
SkillScope::User,
Arc::clone(&LOCAL_FS),
&access,
))
.await;
@@ -146,6 +144,7 @@ async fn symlinked_external_directories_do_not_inherit_plugin_namespace() {
#[cfg(unix)]
#[tokio::test]
async fn ancestor_symlink_roots_preserve_inherited_plugin_namespace() {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = TempDir::new().expect("root temp dir");
let plugin_root = root.path().join("a/b/c/d/e/f/outer");
let skills_root = plugin_root.join("skills");
@@ -157,7 +156,7 @@ async fn ancestor_symlink_roots_preserve_inherited_plugin_namespace() {
let snapshot = load_host_skill_root(HostSkillRoot::host(
AbsolutePathBuf::from_absolute_path(&skills_root).expect("absolute skills root"),
SkillScope::User,
Arc::clone(&LOCAL_FS),
&access,
))
.await;
codex-rs/ext/skills/src/provider/host_tests.rs
+7 -15
View File
@@ -1,3 +1,4 @@
use codex_exec_server::FileSystemEnvironmentAccessor;
use std::sync::Arc;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
@@ -22,6 +23,7 @@ use crate::provider::SkillReadRequest;
#[tokio::test]
async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dyn std::error::Error>>
{
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
let root = std::env::temp_dir().join(format!(
"codex-skills-extension-host-provider-{}-{unique}",
@@ -39,11 +41,7 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dy
)?;
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?;
let outcome = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
root.clone(),
SkillScope::User,
Arc::clone(&LOCAL_FS),
)],
vec![HostSkillRoot::host(root.clone(), SkillScope::User, &access)],
&Semaphore::new(/*permits*/ 1),
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
@@ -73,6 +71,7 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dy
#[tokio::test]
async fn host_catalog_resource_spellings_read_the_discovered_skill()
-> Result<(), Box<dyn std::error::Error>> {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = tempfile::tempdir()?;
let skill_path = root.path().join("demo skill%23").join("SKILL.md");
std::fs::create_dir_all(
@@ -84,11 +83,7 @@ async fn host_catalog_resource_spellings_read_the_discovered_skill()
std::fs::write(&skill_path, contents)?;
let root_path = AbsolutePathBuf::try_from(std::fs::canonicalize(root.path())?)?;
let outcome = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
root_path,
SkillScope::User,
Arc::clone(&LOCAL_FS),
)],
vec![HostSkillRoot::host(root_path, SkillScope::User, &access)],
&Semaphore::new(/*permits*/ 1),
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
@@ -130,6 +125,7 @@ async fn host_catalog_resource_spellings_read_the_discovered_skill()
#[tokio::test]
async fn host_catalog_preserves_symlinked_skill_discovery_paths()
-> Result<(), Box<dyn std::error::Error>> {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let root = tempfile::tempdir()?;
let source = tempfile::tempdir()?;
let source_skill_dir = source.path().join("linked-skill");
@@ -142,11 +138,7 @@ async fn host_catalog_preserves_symlinked_skill_discovery_paths()
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root.path())?)?;
let outcome = load_and_merge_host_skill_roots(
vec![HostSkillRoot::host(
root.clone(),
SkillScope::User,
Arc::clone(&LOCAL_FS),
)],
vec![HostSkillRoot::host(root.clone(), SkillScope::User, &access)],
&Semaphore::new(/*permits*/ 1),
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
codex-rs/ext/skills/src/render_tests.rs
+6 -7
View File
@@ -6,6 +6,7 @@ use crate::catalog::SkillResourceId;
use crate::provider::HostSkillProvider;
use crate::provider::SkillListQuery;
use crate::provider::SkillProvider;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_FS;
use codex_extension_api::ContextualUserFragment;
use codex_protocol::protocol::SkillScope;
@@ -600,6 +601,7 @@ fn executor_aliases_preserve_literal_backslashes_in_package_ids() {
#[tokio::test]
async fn host_alias_roots_follow_core_discovery_order() -> Result<(), Box<dyn std::error::Error>> {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
let parent = std::env::temp_dir().join(format!(
"codex-skills-extension-alias-order-{}-{unique}",
@@ -624,12 +626,8 @@ async fn host_alias_roots_follow_core_discovery_order() -> Result<(), Box<dyn st
let system_root = AbsolutePathBuf::try_from(std::fs::canonicalize(&system_root_path)?)?;
let outcome = load_and_merge_host_skill_roots(
vec![
HostSkillRoot::host(user_root.clone(), SkillScope::User, Arc::clone(&LOCAL_FS)),
HostSkillRoot::host(
system_root.clone(),
SkillScope::System,
Arc::clone(&LOCAL_FS),
),
HostSkillRoot::host(user_root.clone(), SkillScope::User, &access),
HostSkillRoot::host(system_root.clone(), SkillScope::System, &access),
],
&Semaphore::new(/*permits*/ 2),
/*restriction_product*/ None,
@@ -981,6 +979,7 @@ fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() {
#[tokio::test]
async fn singleton_plugin_versions_share_the_marketplace_alias_root()
-> Result<(), Box<dyn std::error::Error>> {
let access = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
let parent = std::env::temp_dir().join(format!(
"codex-skills-extension-marketplace-alias-{}-{unique}",
@@ -999,7 +998,7 @@ async fn singleton_plugin_versions_share_the_marketplace_alias_root()
roots.push(HostSkillRoot::host(
AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?,
SkillScope::User,
Arc::clone(&LOCAL_FS),
&access,
));
}
let marketplace_root = dunce::canonicalize(&marketplace_path)?
codex-rs/ext/skills/tests/executor_file_system_authority.rs
+1 -1
View File
@@ -296,7 +296,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
let snapshot = service
.snapshot_for_config(
&HostSkillsLoadInput::new(cwd, Vec::new(), config_layer_stack),
Some(Arc::clone(&file_system)),
Some(&access),
)
.await;
let outcome = snapshot.outcome();
codex-rs/ext/skills/tests/skills_extension.rs
+1 -1
View File
@@ -3271,7 +3271,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul
let snapshot = service
.snapshot_for_config(
&HostSkillsLoadInput::new(codex_home, Vec::new(), config_layer_stack),
Some(Arc::clone(&LOCAL_FS)),
Some(&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS)),
)
.await;
assert_eq!(snapshot.outcome().errors, Vec::new());
codex-rs/file-system/src/environment_accessor.rs
+31 -1
View File
@@ -27,11 +27,21 @@ use std::sync::Weak;
/// Filesystem operations authorized by the environment that supplied this borrowed accessor.
///
/// Callers cannot extract the underlying filesystem or choose another sandbox. They may retain
/// returned data, cache keys, and already-opened read streams after the accessor is gone.
/// returned data, cache keys, opened read streams, and fixed-path watches after the accessor is gone.
pub trait EnvironmentAccess: Send + Sync {
/// Identifies the filesystem and captured permissions without granting access to either.
fn cache_key(&self) -> EnvironmentAccessKey;
/// Registers an owned watch for these fixed paths, or returns `None` when unsupported.
/// Implementations must preserve captured read permissions when checking the observation.
fn watch_missing(
&self,
_missing: Vec<PathUri>,
_present: Option<PathUri>,
) -> ExecutorFileSystemFuture<'static, Option<tokio::sync::oneshot::Receiver<()>>> {
Box::pin(async { Ok(None) })
}
fn canonicalize<'a>(&'a self, path: &'a PathUri) -> ExecutorFileSystemFuture<'a, PathUri>;
fn read_file<'a>(
@@ -201,6 +211,26 @@ impl<'environment> FileSystemEnvironmentAccessor<'environment> {
}
impl EnvironmentAccess for FileSystemEnvironmentAccessor<'_> {
fn watch_missing(
&self,
missing: Vec<PathUri>,
present: Option<PathUri>,
) -> ExecutorFileSystemFuture<'static, Option<tokio::sync::oneshot::Receiver<()>>> {
// Executor watches make unsandboxed observations, so restricted reads use live discovery.
if self
.key
.sandbox
.as_ref()
.is_some_and(|sandbox| sandbox.should_read_from_sandbox())
{
return Box::pin(async { Ok(None) });
}
let Some(file_system) = self.key.file_system.upgrade() else {
return Box::pin(async { Ok(None) });
};
Box::pin(async move { file_system.watch_missing(missing, present).await })
}
fn cache_key(&self) -> EnvironmentAccessKey {
self.key.clone()
}