diff --git a/codex-rs/app-server/src/request_processors/catalog_processor.rs b/codex-rs/app-server/src/request_processors/catalog_processor.rs index 99aa8b6e726..a4ef8316a9b 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -513,15 +513,19 @@ impl CatalogRequestProcessor { .await; let skills_manager = self.thread_manager.skills_manager(); let plugins_manager = self.thread_manager.plugins_manager(); - let fs = self + let local_environment = self .thread_manager .environment_manager() - .default_environment() - .map(|environment| environment.get_filesystem()); + .try_local_environment(); + // TODO: Reintroduce env-scoped `skills/list` only after cwd/config-layer discovery can run + // against the selected environment and the design decides which non-repo roots stay local + // versus follow that environment. + let local_file_system = Some(Arc::clone(&codex_exec_server::LOCAL_FS)); let mut data = futures::stream::iter(cwds.into_iter().enumerate()) .map(|(index, cwd)| { let config = &config; - let fs = fs.clone(); + let local_environment = local_environment.clone(); + let local_file_system = local_file_system.clone(); let plugins_manager = &plugins_manager; let skills_manager = &skills_manager; async move { @@ -554,13 +558,19 @@ impl CatalogRequestProcessor { Vec::new() }; let skills_input = codex_core::skills::SkillsLoadInput::new( - cwd_abs.clone(), + local_environment.as_ref().map(|environment| { + codex_exec_server::EnvironmentPathRef::new( + environment.get_filesystem(), + cwd_abs.clone(), + ) + }), + local_file_system, effective_skill_roots, config_layer_stack, config.bundled_skills_enabled(), ); let outcome = skills_manager - .skills_for_cwd(&skills_input, force_reload, fs) + .skills_for_cwd(&skills_input, force_reload) .await; let errors = errors_to_info(&outcome.errors); let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths); diff --git a/codex-rs/app-server/src/skills_watcher.rs b/codex-rs/app-server/src/skills_watcher.rs index e43237ba446..433ee18c10a 100644 --- a/codex-rs/app-server/src/skills_watcher.rs +++ b/codex-rs/app-server/src/skills_watcher.rs @@ -9,12 +9,14 @@ use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::skills::SkillsLoadInput; use codex_core::skills::SkillsManager; +use codex_exec_server::EnvironmentPathRef; use codex_file_watcher::FileWatcher; use codex_file_watcher::FileWatcherSubscriber; use codex_file_watcher::Receiver; use codex_file_watcher::ThrottledWatchReceiver; use codex_file_watcher::WatchPath; use codex_file_watcher::WatchRegistration; +use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_absolute_path::AbsolutePathBuf; use tokio_util::sync::CancellationToken; @@ -96,24 +98,26 @@ impl SkillsWatcher { ); return WatchRegistration::default(); }; - if environment.is_remote() { - return WatchRegistration::default(); - } + let root_path_ref = (!environment.is_remote()) + .then(|| EnvironmentPathRef::new(environment.get_filesystem(), config.cwd.clone())); + let local_file_system = Some(Arc::clone(&codex_exec_server::LOCAL_FS)); let plugins_input = config.plugins_config_input(); let plugins_manager = thread_manager.plugins_manager(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let skills_input = SkillsLoadInput::new( - config.cwd.clone(), + root_path_ref, + local_file_system, plugin_outcome.effective_plugin_skill_roots(), config.config_layer_stack.clone(), config.bundled_skills_enabled(), ); let roots = thread_manager .skills_manager() - .skill_roots_for_config(&skills_input, Some(environment.get_filesystem())) + .skill_roots_for_config(&skills_input) .await .into_iter() + .filter(|root| !environment.is_remote() || root.scope != SkillScope::Repo) .map(|root| WatchPath { path: root.path.path().to_path_buf(), recursive: true, diff --git a/codex-rs/app-server/tests/suite/v2/skills_list.rs b/codex-rs/app-server/tests/suite/v2/skills_list.rs index 2ad0b2006a5..dd25ba7a51c 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -450,6 +450,56 @@ async fn skills_list_skips_cwd_roots_when_environment_disabled() -> Result<()> { Ok(()) } +#[tokio::test] +async fn skills_list_skips_cwd_roots_when_only_remote_environment_is_configured() -> Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + write_skill(&codex_home, "home-skill")?; + let repo_skill_dir = cwd.path().join(".codex/skills/repo-skill"); + std::fs::create_dir_all(&repo_skill_dir)?; + std::fs::write( + repo_skill_dir.join("SKILL.md"), + "---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n", + )?; + + let mut mcp = McpProcess::new_with_env( + codex_home.path(), + &[(CODEX_EXEC_SERVER_URL_ENV_VAR, Some("ws://127.0.0.1:8765"))], + ) + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_skills_list_request(SkillsListParams { + cwds: vec![cwd.path().to_path_buf()], + force_reload: true, + }) + .await?; + + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let SkillsListResponse { data } = to_response(response)?; + assert_eq!(data.len(), 1); + assert_eq!(data[0].cwd, cwd.path().to_path_buf()); + assert_eq!(data[0].errors, Vec::new()); + assert!( + data[0] + .skills + .iter() + .any(|skill| skill.name == "home-skill") + ); + assert!( + data[0] + .skills + .iter() + .all(|skill| skill.name != "repo-skill") + ); + Ok(()) +} + #[tokio::test] async fn skills_list_accepts_relative_cwds() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core-skills/src/manager.rs b/codex-rs/core-skills/src/manager.rs index a92d582f480..1e0c958e27d 100644 --- a/codex-rs/core-skills/src/manager.rs +++ b/codex-rs/core-skills/src/manager.rs @@ -24,12 +24,12 @@ use crate::system::uninstall_system_skills; use codex_config::SkillsConfig; use codex_exec_server::EnvironmentPathRef; use codex_exec_server::ExecutorFileSystem; -use codex_exec_server::LOCAL_FS; #[derive(Clone)] pub struct SkillsLoadInput { - pub cwd: AbsolutePathBuf, - local_file_system: Option>, + pub env_path: Option, + /// Local user/system/plugin skill roots are read from this filesystem for now. + pub local_file_system: Option>, pub effective_skill_roots: Vec, pub config_layer_stack: ConfigLayerStack, pub bundled_skills_enabled: bool, @@ -38,7 +38,7 @@ pub struct SkillsLoadInput { impl std::fmt::Debug for SkillsLoadInput { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SkillsLoadInput") - .field("cwd", &self.cwd) + .field("env_path", &self.env_path) .field("has_local_file_system", &self.local_file_system.is_some()) .field("effective_skill_roots", &self.effective_skill_roots) .field("config_layer_stack", &self.config_layer_stack) @@ -49,41 +49,20 @@ impl std::fmt::Debug for SkillsLoadInput { impl SkillsLoadInput { pub fn new( - cwd: AbsolutePathBuf, + env_path: Option, + local_file_system: Option>, effective_skill_roots: Vec, config_layer_stack: ConfigLayerStack, bundled_skills_enabled: bool, ) -> Self { Self { - cwd, - local_file_system: Some(Arc::clone(&LOCAL_FS)), + env_path, + local_file_system, effective_skill_roots, config_layer_stack, bundled_skills_enabled, } } - - #[cfg(test)] - fn with_local_file_system( - mut self, - local_file_system: Option>, - ) -> Self { - self.local_file_system = local_file_system; - self - } - - fn authorities( - &self, - fs: Option>, - ) -> ( - Option, - Option>, - ) { - ( - fs.map(|fs| EnvironmentPathRef::new(fs, self.cwd.clone())), - self.local_file_system.clone(), - ) - } } pub struct SkillsManager { @@ -138,12 +117,8 @@ impl SkillsManager { /// This path uses a cache keyed by the effective skill-relevant config state rather than just /// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen /// to share a directory. - pub async fn skills_for_config( - &self, - input: &SkillsLoadInput, - fs: Option>, - ) -> SkillLoadOutcome { - let roots = self.skill_roots_for_config(input, fs).await; + pub async fn skills_for_config(&self, input: &SkillsLoadInput) -> SkillLoadOutcome { + let roots = self.skill_roots_for_config(input).await; let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let cache_key = config_skills_cache_key(&roots, &skill_config_rules); if let Some(outcome) = self.cached_outcome_for_config(&cache_key) { @@ -159,15 +134,10 @@ impl SkillsManager { outcome } - pub async fn skill_roots_for_config( - &self, - input: &SkillsLoadInput, - fs: Option>, - ) -> Vec { - let (env_path, local_file_system) = input.authorities(fs); + pub async fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec { let mut roots = skill_roots( - env_path.as_ref(), - local_file_system.as_ref(), + input.env_path.as_ref(), + input.local_file_system.as_ref(), &input.config_layer_stack, input.effective_skill_roots.clone(), self.extra_roots(), @@ -183,12 +153,13 @@ impl SkillsManager { &self, input: &SkillsLoadInput, force_reload: bool, - fs: Option>, ) -> SkillLoadOutcome { - let (env_path, local_file_system) = input.authorities(fs); - let path_ref_cache_key = env_path.as_ref().map(|_| SkillsPathCacheKey { - env_path: env_path.clone(), - local_file_system: local_file_system.as_ref().map(ExecutorFileSystemRef::new), + let path_ref_cache_key = input.env_path.as_ref().map(|_| SkillsPathCacheKey { + env_path: input.env_path.clone(), + local_file_system: input + .local_file_system + .as_ref() + .map(ExecutorFileSystemRef::new), }); if !force_reload && let Some(path_ref_cache_key) = path_ref_cache_key.as_ref() @@ -198,8 +169,8 @@ impl SkillsManager { } let mut roots = skill_roots( - env_path.as_ref(), - local_file_system.as_ref(), + input.env_path.as_ref(), + input.local_file_system.as_ref(), &input.config_layer_stack, input.effective_skill_roots.clone(), self.extra_roots(), diff --git a/codex-rs/core-skills/src/manager_tests.rs b/codex-rs/core-skills/src/manager_tests.rs index 0545fc54e49..a68a94f5e4d 100644 --- a/codex-rs/core-skills/src/manager_tests.rs +++ b/codex-rs/core-skills/src/manager_tests.rs @@ -92,23 +92,25 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata { } } +fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef { + EnvironmentPathRef::local(path) +} + fn local_skills_input( cwd: AbsolutePathBuf, effective_skill_roots: Vec, config_layer_stack: ConfigLayerStack, ) -> SkillsLoadInput { + let path_ref = skill_root_path_ref(cwd); SkillsLoadInput::new( - cwd, + Some(path_ref), + Some(Arc::clone(&LOCAL_FS)), effective_skill_roots, config_layer_stack.clone(), bundled_skills_enabled_from_stack(&config_layer_stack), ) } -fn local_file_system() -> Option> { - Some(Arc::clone(&LOCAL_FS)) -} - fn write_demo_skill(tempdir: &TempDir) -> PathBuf { let skill_path = tempdir.path().join("skills").join("demo").join("SKILL.md"); fs::create_dir_all(skill_path.parent().expect("skill path should have parent")) @@ -191,9 +193,7 @@ async fn skills_for_config_with_stack( effective_skill_roots.to_vec(), config_layer_stack.clone(), ); - skills_manager - .skills_for_config(&skills_input, local_file_system()) - .await + skills_manager.skills_for_config(&skills_input).await } #[test] @@ -254,25 +254,32 @@ async fn skills_for_config_keeps_cache_entries_bound_to_file_system() { "---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n", ) .expect("write repo skill"); - let skills_input = SkillsLoadInput::new( - cwd.path().abs(), - Vec::new(), - config_layer_stack.clone(), - bundled_skills_enabled_from_stack(&config_layer_stack), - ); let skills_manager = SkillsManager::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let first_file_system: Arc = Arc::new(LocalFileSystem::unsandboxed()); let second_file_system: Arc = Arc::new(LocalFileSystem::unsandboxed()); + let first_input = SkillsLoadInput::new( + Some(EnvironmentPathRef::new(first_file_system, cwd.path().abs())), + Some(Arc::clone(&LOCAL_FS)), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); + let second_input = SkillsLoadInput::new( + Some(EnvironmentPathRef::new( + second_file_system, + cwd.path().abs(), + )), + Some(Arc::clone(&LOCAL_FS)), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); - let first = skills_manager - .skills_for_config(&skills_input, Some(first_file_system)) - .await; - let second = skills_manager - .skills_for_config(&skills_input, Some(second_file_system)) - .await; + let first = skills_manager.skills_for_config(&first_input).await; + let second = skills_manager.skills_for_config(&second_input).await; let first_skill = first .skills .iter() @@ -300,11 +307,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone()); let empty_outcome = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ false, - local_file_system(), - ) + .skills_for_cwd(&skills_input, /*force_reload*/ false) .await; assert!( empty_outcome @@ -324,11 +327,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { skills_manager.set_extra_roots(vec![extra_skills_root.abs()]); let runtime_outcome = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ false, - local_file_system(), - ) + .skills_for_cwd(&skills_input, /*force_reload*/ false) .await; assert!( runtime_outcome @@ -339,11 +338,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() { skills_manager.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]); let replaced_outcome = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ false, - local_file_system(), - ) + .skills_for_cwd(&skills_input, /*force_reload*/ false) .await; assert_eq!(replaced_outcome.errors, Vec::new()); assert!( @@ -489,11 +484,7 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() { ); let outcome = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ true, - local_file_system(), - ) + .skills_for_cwd(&skills_input, /*force_reload*/ true) .await; assert!( @@ -540,19 +531,20 @@ async fn skills_for_cwd_without_local_fs_skips_local_roots() { ConfigRequirementsToml::default(), ) .expect("valid config layer stack"); - let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone()) - .with_local_file_system(/*local_file_system*/ None); + let skills_input = SkillsLoadInput::new( + Some(skill_root_path_ref(cwd.path().abs())), + /*local_file_system*/ None, + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); let skills_manager = SkillsManager::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let outcome = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ true, - local_file_system(), - ) + .skills_for_cwd(&skills_input, /*force_reload*/ true) .await; assert!( @@ -599,14 +591,20 @@ async fn skills_for_cwd_without_env_path_still_loads_local_roots() { ConfigRequirementsToml::default(), ) .expect("valid config layer stack"); - let skills_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack); + let skills_input = SkillsLoadInput::new( + /*env_path*/ None, + Some(Arc::clone(&LOCAL_FS)), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); let skills_manager = SkillsManager::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let outcome = skills_manager - .skills_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None) + .skills_for_cwd(&skills_input, /*force_reload*/ true) .await; assert!( @@ -677,11 +675,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; let base_input = local_skills_input(cwd.path().abs(), Vec::new(), config_layer_stack.clone()); let outcome_a = skills_manager - .skills_for_cwd( - &base_input, - /*force_reload*/ false, - local_file_system(), - ) + .skills_for_cwd(&base_input, /*force_reload*/ false) .await; assert!( outcome_a @@ -693,11 +687,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { write_user_skill(&codex_home, "late", "late-skill", "added after cache"); let outcome_b = skills_manager - .skills_for_cwd( - &base_input, - /*force_reload*/ false, - local_file_system(), - ) + .skills_for_cwd(&base_input, /*force_reload*/ false) .await; assert!( outcome_b @@ -707,7 +697,7 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() { ); let outcome_reloaded = skills_manager - .skills_for_cwd(&base_input, /*force_reload*/ true, local_file_system()) + .skills_for_cwd(&base_input, /*force_reload*/ true) .await; assert!( outcome_reloaded @@ -729,32 +719,35 @@ async fn skills_for_cwd_keeps_cache_entries_bound_to_file_system() { "---\nname: repo-skill\ndescription: from repo root\n---\n\n# Body\n", ) .expect("write repo skill"); - let skills_input = SkillsLoadInput::new( - cwd.path().abs(), - Vec::new(), - config_layer_stack.clone(), - bundled_skills_enabled_from_stack(&config_layer_stack), - ); let skills_manager = SkillsManager::new( codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); let first_file_system: Arc = Arc::new(LocalFileSystem::unsandboxed()); let second_file_system: Arc = Arc::new(LocalFileSystem::unsandboxed()); + let first_input = SkillsLoadInput::new( + Some(EnvironmentPathRef::new(first_file_system, cwd.path().abs())), + Some(Arc::clone(&LOCAL_FS)), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); + let second_input = SkillsLoadInput::new( + Some(EnvironmentPathRef::new( + second_file_system, + cwd.path().abs(), + )), + Some(Arc::clone(&LOCAL_FS)), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); let first = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ false, - Some(first_file_system), - ) + .skills_for_cwd(&first_input, /*force_reload*/ false) .await; let second = skills_manager - .skills_for_cwd( - &skills_input, - /*force_reload*/ false, - Some(second_file_system), - ) + .skills_for_cwd(&second_input, /*force_reload*/ false) .await; let first_skill = first .skills @@ -775,30 +768,32 @@ async fn skills_for_cwd_skips_cache_without_environment_path() { let manager_codex_home = tempfile::tempdir().expect("tempdir"); let first_codex_home = tempfile::tempdir().expect("tempdir"); let second_codex_home = tempfile::tempdir().expect("tempdir"); - let first_cwd = tempfile::tempdir().expect("tempdir"); - let second_cwd = tempfile::tempdir().expect("tempdir"); write_user_skill(&first_codex_home, "first", "first-skill", "first"); write_user_skill(&second_codex_home, "second", "second-skill", "second"); let skills_manager = SkillsManager::new( manager_codex_home.path().abs(), /*bundled_skills_enabled*/ true, ); - let first_input = local_skills_input( - first_cwd.path().abs(), + let first_input = SkillsLoadInput::new( + /*env_path*/ None, + Some(Arc::clone(&LOCAL_FS)), Vec::new(), config_stack(&first_codex_home, ""), + /*bundled_skills_enabled*/ true, ); - let second_input = local_skills_input( - second_cwd.path().abs(), + let second_input = SkillsLoadInput::new( + /*env_path*/ None, + Some(Arc::clone(&LOCAL_FS)), Vec::new(), config_stack(&second_codex_home, ""), + /*bundled_skills_enabled*/ true, ); let first = skills_manager - .skills_for_cwd(&first_input, /*force_reload*/ false, /*fs*/ None) + .skills_for_cwd(&first_input, /*force_reload*/ false) .await; let second = skills_manager - .skills_for_cwd(&second_input, /*force_reload*/ false, /*fs*/ None) + .skills_for_cwd(&second_input, /*force_reload*/ false) .await; assert!(first.skills.iter().any(|skill| skill.name == "first-skill")); @@ -1094,11 +1089,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() let parent_input = local_skills_input(cwd.path().abs(), Vec::new(), parent_stack.clone()); let parent_outcome = skills_manager - .skills_for_cwd( - &parent_input, - /*force_reload*/ true, - local_file_system(), - ) + .skills_for_cwd(&parent_input, /*force_reload*/ true) .await; let parent_skill = parent_outcome .skills diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 5461323a366..4935515c48b 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -421,13 +421,17 @@ enabled = false let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let skills_input = skills_load_input_from_config(&config, effective_skill_roots); - let outcome = skills_manager - .skills_for_config( - &skills_input, - Some(Arc::clone(&codex_exec_server::LOCAL_FS)), - ) - .await; + let cwd = crate::skills::EnvironmentPathRef::new( + Arc::clone(&codex_exec_server::LOCAL_FS), + config.cwd.clone(), + ); + let skills_input = skills_load_input_from_config( + &config, + Some(cwd), + Some(Arc::clone(&codex_exec_server::LOCAL_FS)), + effective_skill_roots, + ); + let outcome = skills_manager.skills_for_config(&skills_input).await; let skill = outcome .skills .iter() diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 749212f70af..0922dd95a0c 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -2,7 +2,6 @@ use std::collections::HashSet; use std::sync::Arc; use codex_exec_server::EnvironmentManager; -use codex_exec_server::ExecutorFileSystem; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; use codex_protocol::protocol::TurnEnvironmentSelection; @@ -45,11 +44,6 @@ impl ResolvedTurnEnvironments { self.primary() .map(|environment| Arc::clone(&environment.environment)) } - - pub(crate) fn primary_filesystem(&self) -> Option> { - self.primary() - .map(|environment| environment.environment.get_filesystem()) - } } pub(crate) fn resolve_environment_selections( diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index c9e1807fe6f..f301bcb962a 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -447,20 +447,34 @@ async fn warm_plugins_and_skills_for_session_init( skills_manager: Arc, environments: Vec, ) -> Vec { - let fs = crate::environment_selection::resolve_environment_selections( + let resolved_environments = crate::environment_selection::resolve_environment_selections( environment_manager.as_ref(), &environments, ) - .ok() - .and_then(|resolved| resolved.primary_filesystem()); + .ok(); + let path_ref = resolved_environments + .as_ref() + .and_then(crate::environment_selection::ResolvedTurnEnvironments::primary) + .map(|environment| { + crate::skills::EnvironmentPathRef::new( + environment.environment.get_filesystem(), + environment.cwd.clone(), + ) + }); + let local_file_system = environment_manager + .try_local_environment() + .map(|environment| environment.get_filesystem()) + .or_else(|| Some(Arc::clone(&codex_exec_server::LOCAL_FS))); let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots); - skills_manager - .skills_for_config(&skills_input, fs) - .await - .errors + let skills_input = skills_load_input_from_config( + config.as_ref(), + path_ref, + local_file_system, + effective_skill_roots, + ); + skills_manager.skills_for_config(&skills_input).await.errors } impl Session { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 45355779ef7..4b82006ee60 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4127,9 +4127,20 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { .services .skills_manager .skills_for_cwd( - &crate::skills_load_input_from_config(&parent_config, Vec::new()), + &crate::skills_load_input_from_config( + &parent_config, + Some(crate::skills::EnvironmentPathRef::new( + Arc::clone(&skill_fs), + parent_config.cwd.clone(), + )), + session + .services + .environment_manager + .try_local_environment() + .map(|environment| environment.get_filesystem()), + Vec::new(), + ), /*force_reload*/ true, - Some(Arc::clone(&skill_fs)), ) .await; let parent_skill = parent_outcome @@ -4692,13 +4703,23 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { .plugins_for_config(&per_turn_config.plugins_config_input()) .await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let skills_input = - crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); - let skill_fs = environment.get_filesystem(); + let cwd = crate::skills::EnvironmentPathRef::new( + environment.get_filesystem(), + per_turn_config.cwd.clone(), + ); + let skills_input = crate::skills_load_input_from_config( + &per_turn_config, + Some(cwd), + services + .environment_manager + .try_local_environment() + .map(|environment| environment.get_filesystem()), + effective_skill_roots, + ); let skills_outcome = Arc::new( services .skills_manager - .skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) + .skills_for_config(&skills_input) .await, ); let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd); @@ -6535,13 +6556,23 @@ where .plugins_for_config(&per_turn_config.plugins_config_input()) .await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let skills_input = - crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); - let skill_fs = environment.get_filesystem(); + let cwd = crate::skills::EnvironmentPathRef::new( + environment.get_filesystem(), + per_turn_config.cwd.clone(), + ); + let skills_input = crate::skills_load_input_from_config( + &per_turn_config, + Some(cwd), + services + .environment_manager + .try_local_environment() + .map(|environment| environment.get_filesystem()), + effective_skill_roots, + ); let skills_outcome = Arc::new( services .skills_manager - .skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) + .skills_for_config(&skills_input) .await, ); let turn_environments = turn_environments_for_tests(&environment, &session_configuration.cwd); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 895ab1d8933..36d316b963c 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -702,19 +702,38 @@ impl Session { &per_turn_config.to_models_manager_config(), ) .await; + let path_ref = primary_turn_environment.map(|turn_environment| { + crate::skills::EnvironmentPathRef::new( + turn_environment.environment.get_filesystem(), + turn_environment.cwd.clone(), + ) + }); + // Local turns use their local environment filesystem for both cwd and host-local roots so + // implicit command detection can match either kind of loaded skill. Remote turns still + // keep host-local user/system/plugin roots on the process-local filesystem. + let local_file_system = self + .services + .environment_manager + .try_local_environment() + .map(|environment| environment.get_filesystem()) + .or_else(|| Some(Arc::clone(&codex_exec_server::LOCAL_FS))); + let plugins_input = per_turn_config.plugins_config_input(); let plugin_outcome = self .services .plugins_manager - .plugins_for_config(&per_turn_config.plugins_config_input()) + .plugins_for_config(&plugins_input) .await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); - let fs = primary_turn_environment - .map(|turn_environment| turn_environment.environment.get_filesystem()); + let skills_input = skills_load_input_from_config( + &per_turn_config, + path_ref, + local_file_system, + effective_skill_roots, + ); let skills_outcome = Arc::new( self.services .skills_manager - .skills_for_config(&skills_input, fs) + .skills_for_config(&skills_input) .await, ); let goal_tools_supported = !per_turn_config.ephemeral && self.state_db().is_some(); diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 526c425d62f..2aa6d330f09 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -5,7 +5,9 @@ use codex_analytics::InvocationType; use codex_analytics::SkillInvocation; use codex_analytics::build_track_events_context; pub use codex_exec_server::EnvironmentPathRef; +use codex_exec_server::ExecutorFileSystem; use codex_utils_plugins::PluginSkillRoot; +use std::sync::Arc; pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; @@ -34,10 +36,13 @@ pub use codex_core_skills::system; pub(crate) fn skills_load_input_from_config( config: &Config, + env_path: Option, + local_file_system: Option>, effective_skill_roots: Vec, ) -> SkillsLoadInput { SkillsLoadInput::new( - config.cwd.clone(), + env_path, + local_file_system, effective_skill_roots, config.config_layer_stack.clone(), config.bundled_skills_enabled(),