diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 4d97f992d0..09c7447382 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -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(); diff --git a/codex-rs/core/src/plugins/skill_snapshot_tests.rs b/codex-rs/core/src/plugins/skill_snapshot_tests.rs index 6540d55d17..175c2b8a8e 100644 --- a/codex-rs/core/src/plugins/skill_snapshot_tests.rs +++ b/codex-rs/core/src/plugins/skill_snapshot_tests.rs @@ -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!( diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 5cc388cd40..c11bb7b582 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -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 diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 3704a9079b..c8494dd0b0 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -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, diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 637904c2bb..0bbbd5294e 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -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( diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index b179f9b1ad..47fd6f4fbc 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -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() diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index 458f4c59d0..20fbe6d4de 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -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| { diff --git a/codex-rs/ext/skills/src/host_roots.rs b/codex-rs/ext/skills/src/host_roots.rs index 2559b4bc7f..1bfee96524 100644 --- a/codex-rs/ext/skills/src/host_roots.rs +++ b/codex-rs/ext/skills/src/host_roots.rs @@ -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, + fs: EnvironmentAccessKey, cwd: AbsolutePathBuf, markers: Vec, watch: EmptyRootsWatch, @@ -53,18 +53,20 @@ impl EmptyRemoteRootsCache { } } -pub(crate) async fn resolve_skill_roots( - repository_file_system: Option>, +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, extra_skill_roots: Vec, empty_remote_roots: Option<&Mutex>, -) -> Vec { +) -> Vec> { 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>, +#[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, extra_skill_roots: Vec, empty_remote_roots: Option<&Mutex>, -) -> Vec { +) -> Vec> { 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>, -) -> Vec { + repository_file_system: Option<&'a dyn EnvironmentAccess>, + host: &'a dyn EnvironmentAccess, +) -> Vec> { 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>, +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>, -) -> Vec { +) -> Vec> { 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, 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) }) diff --git a/codex-rs/ext/skills/src/host_roots_tests.rs b/codex-rs/ext/skills/src/host_roots_tests.rs index e7b46f71d9..94ca74df6a 100644 --- a/codex-rs/ext/skills/src/host_roots_tests.rs +++ b/codex-rs/ext/skills/src/host_roots_tests.rs @@ -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::>(); + let roots = roots_from_layer_stack(&config_stack, Some(&home_folder), Some(&access), &access) + .into_iter() + .map(|root| (root.scope, root.path)) + .collect::>(); 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), diff --git a/codex-rs/ext/skills/src/host_service.rs b/codex-rs/ext/skills/src/host_service.rs index d5f4790f3b..be9f20b016 100644 --- a/codex-rs/ext/skills/src/host_service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -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>, ) -> 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>, + 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, ) -> Vec { - 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>, + repository: Option<&'a dyn EnvironmentAccess>, + host: &'a dyn EnvironmentAccess, empty_remote_roots: Option<&Mutex>, - ) -> Vec { + ) -> Vec> { 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>, + 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, + roots: Vec>, 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, + roots: Vec>, skill_config_rules: &SkillConfigRules, request_root_snapshots: Option<&RequestSkillRootSnapshots>, ) -> SkillLoadOutcome { @@ -441,10 +457,11 @@ impl SkillRootLoader for HostSkillsService { request: SkillRootLoadRequest, ) -> 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, plugin_namespace: Option, - file_system: FileSystemIdentity, -} - -#[derive(Debug, Clone)] -struct FileSystemIdentity(Weak); - -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(&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>, ) -> 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(), } } diff --git a/codex-rs/ext/skills/src/host_service_tests.rs b/codex-rs/ext/skills/src/host_service_tests.rs index fd8d450a5a..0d5e22c819 100644 --- a/codex-rs/ext/skills/src/host_service_tests.rs +++ b/codex-rs/ext/skills/src/host_service_tests.rs @@ -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 = 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, diff --git a/codex-rs/ext/skills/src/loader/discovery.rs b/codex-rs/ext/skills/src/loader/discovery.rs index ad68d3ba02..c7e04136f9 100644 --- a/codex-rs/ext/skills/src/loader/discovery.rs +++ b/codex-rs/ext/skills/src/loader/discovery.rs @@ -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, diff --git a/codex-rs/ext/skills/src/loader/host.rs b/codex-rs/ext/skills/src/loader/host.rs index e89c5de1b5..eb142b092d 100644 --- a/codex-rs/ext/skills/src/loader/host.rs +++ b/codex-rs/ext/skills/src/loader/host.rs @@ -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, + pub(crate) file_system: &'a dyn EnvironmentAccess, plugin: Option, } @@ -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, + file_system: &'a dyn EnvironmentAccess, ) -> Self { Self { path, @@ -66,7 +65,7 @@ impl HostSkillRoot { } } - pub(crate) fn plugin(root: PluginSkillRoot, file_system: Arc) -> 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, Arc>, Vec, ) { - 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()) diff --git a/codex-rs/ext/skills/src/loader/host_io_tests.rs b/codex-rs/ext/skills/src/loader/host_io_tests.rs index 36bd34046e..9caf67e17e 100644 --- a/codex-rs/ext/skills/src/loader/host_io_tests.rs +++ b/codex-rs/ext/skills/src/loader/host_io_tests.rs @@ -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 = 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, diff --git a/codex-rs/ext/skills/src/loader/host_merge.rs b/codex-rs/ext/skills/src/loader/host_merge.rs index 56ad696324..cd686965f0 100644 --- a/codex-rs/ext/skills/src/loader/host_merge.rs +++ b/codex-rs/ext/skills/src/loader/host_merge.rs @@ -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, + roots: Vec>, root_scan_slots: &Semaphore, restriction_product: Option, plugin_skill_snapshots: Option<&SkillRootSnapshots>, @@ -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, + roots: Vec>, root_scan_slots: &Semaphore, restriction_product: Option, plugin_skill_snapshots: Option<&SkillRootSnapshots>, @@ -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::>(); 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::>() .await; diff --git a/codex-rs/ext/skills/src/loader/host_merge_tests.rs b/codex-rs/ext/skills/src/loader/host_merge_tests.rs index d3eb50ba1f..3653457241 100644 --- a/codex-rs/ext/skills/src/loader/host_merge_tests.rs +++ b/codex-rs/ext/skills/src/loader/host_merge_tests.rs @@ -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 { + fn roots(&self) -> Vec> { [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 = recording.clone(); - let host_roots = roots - .iter() - .map(|root| HostSkillRoot::host(root.clone().abs(), SkillScope::User, file_system.clone())) - .collect::>(); 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::>(); 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, diff --git a/codex-rs/ext/skills/src/loader/host_tests.rs b/codex-rs/ext/skills/src/loader/host_tests.rs index 4894c9dd10..ad05449540 100644 --- a/codex-rs/ext/skills/src/loader/host_tests.rs +++ b/codex-rs/ext/skills/src/loader/host_tests.rs @@ -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()); } diff --git a/codex-rs/ext/skills/src/loader/metadata.rs b/codex-rs/ext/skills/src/loader/metadata.rs index 7c0299d443..9fc743e206 100644 --- a/codex-rs/ext/skills/src/loader/metadata.rs +++ b/codex-rs/ext/skills/src/loader/metadata.rs @@ -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, diff --git a/codex-rs/ext/skills/src/loader/mod.rs b/codex-rs/ext/skills/src/loader/mod.rs index 55df37677b..53da2d5dcd 100644 --- a/codex-rs/ext/skills/src/loader/mod.rs +++ b/codex-rs/ext/skills/src/loader/mod.rs @@ -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; diff --git a/codex-rs/ext/skills/src/loader/namespace_tests.rs b/codex-rs/ext/skills/src/loader/namespace_tests.rs index 96f0408a20..d5e172c923 100644 --- a/codex-rs/ext/skills/src/loader/namespace_tests.rs +++ b/codex-rs/ext/skills/src/loader/namespace_tests.rs @@ -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; diff --git a/codex-rs/ext/skills/src/provider/host_tests.rs b/codex-rs/ext/skills/src/provider/host_tests.rs index 818dbfc3cc..80731e24ac 100644 --- a/codex-rs/ext/skills/src/provider/host_tests.rs +++ b/codex-rs/ext/skills/src/provider/host_tests.rs @@ -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> { + 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 Result<(), Box Result<(), Box> { + 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> { + 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, diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index 7813f5c0a1..eca0a1492b 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -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> { + 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 Result<(), Box> { + 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)? diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 2265ed1519..f27e0ce57f 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -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(); diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 90f2dae8cd..25090eaef2 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -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()); diff --git a/codex-rs/file-system/src/environment_accessor.rs b/codex-rs/file-system/src/environment_accessor.rs index caed0c180c..ed16a9f077 100644 --- a/codex-rs/file-system/src/environment_accessor.rs +++ b/codex-rs/file-system/src/environment_accessor.rs @@ -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, + _present: Option, + ) -> ExecutorFileSystemFuture<'static, Option>> { + 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, + present: Option, + ) -> ExecutorFileSystemFuture<'static, Option>> { + // 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() }