From a4db9b34d479874a0841fa750b90eb7f28c2a489 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 28 May 2026 18:45:08 -0700 Subject: [PATCH] codex: simplify environment path refs --- .../src/protocol/common.rs | 1 + .../src/protocol/v2/plugin.rs | 4 ++ .../src/protocol/v2/tests.rs | 20 ++++++- codex-rs/app-server/README.md | 3 +- .../request_processors/catalog_processor.rs | 39 ++++++++----- codex-rs/app-server/src/skills_watcher.rs | 7 +-- .../app-server/tests/suite/v2/skills_list.rs | 12 ++++ codex-rs/core-skills/src/loader.rs | 6 +- codex-rs/core-skills/src/loader_tests.rs | 29 ++++------ codex-rs/core-skills/src/manager_tests.rs | 9 +-- codex-rs/core/src/agent/role_tests.rs | 3 +- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/session/session.rs | 4 +- codex-rs/core/src/session/tests.rs | 9 +-- codex-rs/core/src/session/turn_context.rs | 4 +- codex-rs/core/src/skills.rs | 10 ---- codex-rs/exec-server/src/environment_path.rs | 58 +++++-------------- codex-rs/tui/src/app/background_requests.rs | 1 + codex-rs/tui/src/app/thread_routing.rs | 1 + 19 files changed, 102 insertions(+), 119 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 71c89a185c..15262591b4 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1718,6 +1718,7 @@ mod tests { request_id: request_id(), params: v2::SkillsListParams { cwds: Vec::new(), + environment_id: None, force_reload: false, }, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 1b7e8ba8ff..82fb4e0389 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -23,6 +23,10 @@ pub struct SkillsListParams { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub cwds: Vec, + /// Environment whose filesystem should be used for all requested cwd values. + #[ts(optional = nullable)] + pub environment_id: Option, + /// When true, bypass the skills cache and re-scan skills from disk. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub force_reload: bool, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index a0be5cb15a..f230ea06a8 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2607,23 +2607,41 @@ fn skills_list_params_serialization_uses_force_reload() { assert_eq!( serde_json::to_value(SkillsListParams { cwds: Vec::new(), + environment_id: None, force_reload: false, }) .unwrap(), - json!({}), + json!({ + "environmentId": null, + }), ); assert_eq!( serde_json::to_value(SkillsListParams { cwds: vec![PathBuf::from("/repo")], + environment_id: None, force_reload: true, }) .unwrap(), json!({ "cwds": ["/repo"], + "environmentId": null, "forceReload": true, }), ); + + assert_eq!( + serde_json::to_value(SkillsListParams { + cwds: vec![PathBuf::from("/repo")], + environment_id: Some("devbox".to_string()), + force_reload: false, + }) + .unwrap(), + json!({ + "cwds": ["/repo"], + "environmentId": "devbox", + }), + ); } #[test] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2bf6035b27..b3076de0a5 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1504,13 +1504,14 @@ Example: $skill-creator Add a new skill for triaging flaky CI and include step-by-step usage. ``` -Use `skills/list` to fetch the available skills (optionally scoped by `cwds`, with `forceReload`). +Use `skills/list` to fetch the available skills (optionally scoped by `cwds`, `environmentId`, and `forceReload`). `skills/list` might reuse a cached skills result per `cwd`; setting `forceReload` to `true` refreshes the result from disk. The server also emits `skills/changed` notifications when watched local skill files change. Treat this as an invalidation signal and re-run `skills/list` with your current params when needed. Use `skills/extraRoots/set` to replace additional standalone skill roots for the current app-server process. These roots use the same layout as other standalone skill roots: each root contains skill directories, and each skill directory contains `SKILL.md`. Missing roots are accepted and load no skills until they exist. This setting is lost when app-server exits. ```json { "method": "skills/list", "id": 25, "params": { + "environmentId": "local", "cwds": ["/Users/me/project", "/Users/me/other-project"], "forceReload": true } } 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 b27476f836..00982ce276 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -499,7 +499,11 @@ impl CatalogRequestProcessor { &self, params: SkillsListParams, ) -> Result { - let SkillsListParams { cwds, force_reload } = params; + let SkillsListParams { + cwds, + environment_id, + force_reload, + } = params; let cwds = if cwds.is_empty() { vec![self.config.cwd.to_path_buf()] } else { @@ -514,13 +518,23 @@ impl CatalogRequestProcessor { let skills_manager = self.thread_manager.skills_manager(); let plugins_manager = self.thread_manager.plugins_manager(); let environment_manager = self.thread_manager.environment_manager(); - let local_fs = environment_manager - .try_local_environment() - .map(|environment| environment.get_filesystem()); + let selected_environment = match environment_id { + Some(environment_id) => { + let environment = environment_manager + .get_environment(&environment_id) + .ok_or_else(|| { + invalid_request(format!( + "unknown skills/list environment id `{environment_id}`" + )) + })?; + Some(environment) + } + None => environment_manager.try_local_environment(), + }; let mut data = futures::stream::iter(cwds.into_iter().enumerate()) .map(|(index, cwd)| { let config = &config; - let local_fs = local_fs.clone(); + let selected_environment = selected_environment.clone(); let plugins_manager = &plugins_manager; let skills_manager = &skills_manager; async move { @@ -541,14 +555,9 @@ impl CatalogRequestProcessor { ); } }; - let local_path_ref = local_fs.clone().map(|file_system| { - codex_core::skills::EnvironmentPathRef::new( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - file_system, - cwd_abs.clone(), - ) - }); - let Some(path_ref) = local_path_ref else { + let Some(environment) = selected_environment.as_ref() else { + // Legacy callers without `environmentId` remain local-only; if local is + // unavailable, do not fabricate an unbound skill path. return ( index, codex_app_server_protocol::SkillsListEntry { @@ -558,6 +567,10 @@ impl CatalogRequestProcessor { }, ); }; + let path_ref = codex_core::skills::EnvironmentPathRef::new( + environment.get_filesystem(), + cwd_abs.clone(), + ); let effective_skill_roots = if workspace_codex_plugins_enabled { let plugins_input = config.plugins_config_input(); plugins_manager diff --git a/codex-rs/app-server/src/skills_watcher.rs b/codex-rs/app-server/src/skills_watcher.rs index 9844e8fec4..7bd940963a 100644 --- a/codex-rs/app-server/src/skills_watcher.rs +++ b/codex-rs/app-server/src/skills_watcher.rs @@ -101,11 +101,8 @@ impl SkillsWatcher { return WatchRegistration::default(); } - let root_path_ref = EnvironmentPathRef::new( - environment_selection.environment_id.clone(), - environment.get_filesystem(), - config.cwd.clone(), - ); + let root_path_ref = + EnvironmentPathRef::new(environment.get_filesystem(), config.cwd.clone()); 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; 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 2ad0b2006a..66fc4ee8d7 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -20,6 +20,7 @@ use codex_app_server_protocol::SkillsListResponse; use codex_app_server_protocol::ThreadStartParams; use codex_config::types::AuthCredentialsStoreMode; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -248,6 +249,7 @@ async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result let stale_skills_list_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: true, }) .await?; @@ -298,6 +300,7 @@ async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result let skills_list_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: false, }) .await?; @@ -372,6 +375,7 @@ async fn skills_list_excludes_plugin_skills_when_workspace_codex_plugins_disable let request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![repo_root.path().to_path_buf()], + environment_id: None, force_reload: true, }) .await?; @@ -422,6 +426,7 @@ async fn skills_list_skips_cwd_roots_when_environment_disabled() -> Result<()> { let request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: true, }) .await?; @@ -462,6 +467,7 @@ async fn skills_list_accepts_relative_cwds() -> Result<()> { let request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![relative_cwd.clone()], + environment_id: Some(LOCAL_ENVIRONMENT_ID.to_string()), force_reload: true, }) .await?; @@ -493,6 +499,7 @@ async fn skills_list_preserves_requested_cwd_order() -> Result<()> { first_cwd.path().to_path_buf(), second_cwd.path().to_path_buf(), ], + environment_id: None, force_reload: true, }) .await?; @@ -527,6 +534,7 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> { let first_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: false, }) .await?; @@ -554,6 +562,7 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> { let second_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: false, }) .await?; @@ -574,6 +583,7 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> { let third_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![cwd.path().to_path_buf()], + environment_id: None, force_reload: true, }) .await?; @@ -755,6 +765,7 @@ async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<( let initial_skills_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![codex_home.path().to_path_buf()], + environment_id: None, force_reload: true, }) .await?; @@ -828,6 +839,7 @@ async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<( let updated_skills_request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![codex_home.path().to_path_buf()], + environment_id: None, force_reload: false, }) .await?; diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 98e3448106..d5feedc3dd 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -1060,11 +1060,7 @@ pub(crate) async fn skill_roots_from_layer_stack( cwd: &AbsolutePathBuf, home_dir: Option<&AbsolutePathBuf>, ) -> Vec { - let path_ref = EnvironmentPathRef::new( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - fs, - cwd.clone(), - ); + let path_ref = EnvironmentPathRef::new(fs, cwd.clone()); skill_roots_with_home_dir(&path_ref, config_layer_stack, home_dir, Vec::new(), Vec::new()) .await } diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index 8ea015d030..92b7ec05bd 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -4,7 +4,6 @@ use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; -use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; @@ -145,17 +144,13 @@ fn normalized(path: &Path) -> AbsolutePathBuf { .abs() } -fn env_path(path: AbsolutePathBuf) -> EnvironmentPathRef { - EnvironmentPathRef::new( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::clone(&LOCAL_FS), - path, - ) +fn local_env_path(path: AbsolutePathBuf) -> EnvironmentPathRef { + EnvironmentPathRef::local(path) } fn local_skill_root(path: AbsolutePathBuf, scope: SkillScope) -> SkillRoot { SkillRoot { - path: env_path(path), + path: local_env_path(path), scope, plugin_id: None, plugin_root: None, @@ -864,10 +859,10 @@ interface: let plugin_root_abs = plugin_root.abs(); let outcome = load_skills_from_roots([SkillRoot { - path: env_path(plugin_root.join("skills").abs()), + path: local_env_path(plugin_root.join("skills").abs()), scope: SkillScope::User, plugin_id: Some("twilio-developer-kit@test".to_string()), - plugin_root: Some(env_path(plugin_root_abs.clone())), + plugin_root: Some(local_env_path(plugin_root_abs.clone())), }]) .await; @@ -920,10 +915,10 @@ interface: ); let outcome = load_skills_from_roots([SkillRoot { - path: env_path(plugin_root.join("skills").abs()), + path: local_env_path(plugin_root.join("skills").abs()), scope: SkillScope::User, plugin_id: Some("twilio-developer-kit@test".to_string()), - plugin_root: Some(env_path(plugin_root.abs())), + plugin_root: Some(local_env_path(plugin_root.abs())), }]) .await; @@ -1271,10 +1266,10 @@ async fn namespaces_plugin_skills_using_plugin_name() { .unwrap(); let outcome = load_skills_from_roots([SkillRoot { - path: env_path(plugin_root.join("skills").abs()), + path: local_env_path(plugin_root.join("skills").abs()), scope: SkillScope::User, plugin_id: Some("sample@test".to_string()), - plugin_root: Some(env_path(plugin_root.abs())), + plugin_root: Some(local_env_path(plugin_root.abs())), }]) .await; @@ -1593,13 +1588,13 @@ async fn deduplicates_by_path_preferring_first_root() { let outcome = load_skills_from_roots([ SkillRoot { - path: env_path(root.path().abs()), + path: local_env_path(root.path().abs()), scope: SkillScope::Repo, plugin_id: None, plugin_root: None, }, SkillRoot { - path: env_path(root.path().abs()), + path: local_env_path(root.path().abs()), scope: SkillScope::User, plugin_id: None, plugin_root: None, @@ -1891,7 +1886,7 @@ async fn skill_roots_include_admin_with_lowest_priority() { let codex_home = tempfile::tempdir().expect("tempdir"); let cfg = make_config(&codex_home).await; - let cwd = env_path(cfg.cwd.clone()); + let cwd = local_env_path(cfg.cwd.clone()); let scopes: Vec = super::skill_roots(&cwd, &cfg.config_layer_stack, Vec::new(), Vec::new()) .await diff --git a/codex-rs/core-skills/src/manager_tests.rs b/codex-rs/core-skills/src/manager_tests.rs index 7c78143583..f2caded837 100644 --- a/codex-rs/core-skills/src/manager_tests.rs +++ b/codex-rs/core-skills/src/manager_tests.rs @@ -7,8 +7,6 @@ use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirementsToml; -use codex_exec_server::LOCAL_ENVIRONMENT_ID; -use codex_exec_server::LOCAL_FS; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::PathExt; @@ -18,7 +16,6 @@ use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; use tempfile::TempDir; fn write_user_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) { @@ -89,11 +86,7 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata { } fn skill_root_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef { - EnvironmentPathRef::new( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::clone(&LOCAL_FS), - path, - ) + EnvironmentPathRef::local(path) } fn local_skills_input( diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 8e20d09506..2f64124b83 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -421,8 +421,7 @@ 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 cwd = crate::environment_path( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + let cwd = crate::skills::EnvironmentPathRef::new( Arc::clone(&codex_exec_server::LOCAL_FS), config.cwd.clone(), ); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index da60668e68..aa5784d4fe 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -92,7 +92,6 @@ pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_explicit_skill_mentions; pub(crate) use skills::default_skill_metadata_budget; -pub(crate) use skills::environment_path_ref; pub(crate) use skills::injection; pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 47b65cf207..559d1f3d7c 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1,7 +1,6 @@ use super::input_queue::InputQueue; use super::*; use crate::config::ConstraintError; -use crate::environment_path; use crate::goals::GoalRuntimeState; use crate::skills::SkillError; use crate::state::ActiveTurn; @@ -457,8 +456,7 @@ async fn warm_plugins_and_skills_for_session_init( .as_ref() .and_then(crate::environment_selection::ResolvedTurnEnvironments::primary) .map(|environment| { - environment_path( - environment.environment_id.clone(), + crate::skills::EnvironmentPathRef::new( environment.environment.get_filesystem(), environment.cwd.clone(), ) diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 971c05eb10..a8648fc2f6 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4129,8 +4129,7 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { .skills_for_cwd( &crate::skills_load_input_from_config( &parent_config, - crate::environment_path( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + crate::skills::EnvironmentPathRef::new( Arc::clone(&skill_fs), parent_config.cwd.clone(), ), @@ -4699,8 +4698,7 @@ 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 cwd = crate::environment_path( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + let cwd = crate::skills::EnvironmentPathRef::new( environment.get_filesystem(), per_turn_config.cwd.clone(), ); @@ -6546,8 +6544,7 @@ where .plugins_for_config(&per_turn_config.plugins_config_input()) .await; let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); - let cwd = crate::environment_path( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + let cwd = crate::skills::EnvironmentPathRef::new( environment.get_filesystem(), per_turn_config.cwd.clone(), ); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 5253fe7d3e..b07492ee21 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -1,7 +1,6 @@ use super::*; use crate::SkillLoadOutcome; use crate::config::GhostSnapshotConfig; -use crate::environment_path; use crate::environment_selection::ResolvedTurnEnvironments; use codex_model_provider::SharedModelProvider; use codex_model_provider::create_model_provider; @@ -680,8 +679,7 @@ impl Session { ) .await; let path_ref = primary_turn_environment.map(|turn_environment| { - environment_path( - turn_environment.environment_id.clone(), + crate::skills::EnvironmentPathRef::new( turn_environment.environment.get_filesystem(), turn_environment.cwd.clone(), ) diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 86aaf004ea..6b3bbce309 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -5,11 +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_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginSkillRoot; -use std::sync::Arc; pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; @@ -49,14 +47,6 @@ pub(crate) fn skills_load_input_from_config( ) } -pub(crate) fn environment_path( - environment_id: String, - file_system: Arc, - path: AbsolutePathBuf, -) -> EnvironmentPathRef { - EnvironmentPathRef::new(environment_id, file_system, path) -} - pub(crate) async fn maybe_emit_implicit_skill_invocation( sess: &Session, turn_context: &TurnContext, diff --git a/codex-rs/exec-server/src/environment_path.rs b/codex-rs/exec-server/src/environment_path.rs index 64d9f327d7..2378a4545f 100644 --- a/codex-rs/exec-server/src/environment_path.rs +++ b/codex-rs/exec-server/src/environment_path.rs @@ -10,47 +10,29 @@ use codex_utils_absolute_path::AbsolutePathBuf; use crate::ExecutorFileSystem; use crate::FileMetadata; use crate::FileSystemSandboxContext; -use crate::LOCAL_ENVIRONMENT_ID; use crate::LOCAL_FS; use crate::ReadDirectoryEntry; -/// Binds an absolute path to the executor filesystem and environment that owns it. +/// Binds an absolute path to the executor filesystem that owns it. #[derive(Clone)] pub struct EnvironmentPathRef { - environment_id: String, file_system: Arc, path: AbsolutePathBuf, } impl EnvironmentPathRef { - pub fn new( - environment_id: String, - file_system: Arc, - path: AbsolutePathBuf, - ) -> Self { - Self { - environment_id, - file_system, - path, - } + pub fn new(file_system: Arc, path: AbsolutePathBuf) -> Self { + Self { file_system, path } } pub fn local(path: AbsolutePathBuf) -> Self { - Self::new( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::clone(&LOCAL_FS), - path, - ) + Self::new(Arc::clone(&LOCAL_FS), path) } pub fn path(&self) -> &AbsolutePathBuf { &self.path } - pub fn environment_id(&self) -> &str { - &self.environment_id - } - pub fn file_system(&self) -> Arc { Arc::clone(&self.file_system) } @@ -87,19 +69,13 @@ impl EnvironmentPathRef { } pub fn with_path(&self, path: AbsolutePathBuf) -> Self { - Self::new( - self.environment_id.clone(), - Arc::clone(&self.file_system), - path, - ) + Self::new(Arc::clone(&self.file_system), path) } } impl PartialEq for EnvironmentPathRef { fn eq(&self, other: &Self) -> bool { - self.environment_id == other.environment_id - && Arc::ptr_eq(&self.file_system, &other.file_system) - && self.path == other.path + Arc::ptr_eq(&self.file_system, &other.file_system) && self.path == other.path } } @@ -107,7 +83,6 @@ impl Eq for EnvironmentPathRef {} impl Hash for EnvironmentPathRef { fn hash(&self, state: &mut H) { - self.environment_id.hash(state); (Arc::as_ptr(&self.file_system) as *const () as usize).hash(state); self.path.hash(state); } @@ -116,7 +91,6 @@ impl Hash for EnvironmentPathRef { impl fmt::Debug for EnvironmentPathRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("EnvironmentPathRef") - .field("environment_id", &self.environment_id) .field("path", &self.path) .finish() } @@ -272,8 +246,7 @@ mod tests { async fn environment_path_ref_forwards_sandbox_to_file_system_methods() { let path = std::env::temp_dir().join("skills/demo").abs(); let file_system = Arc::new(RecordingFileSystem::default()); - let path_ref = - EnvironmentPathRef::new("local".to_string(), file_system.clone(), path.clone()); + let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone()); let sandbox = restricted_sandbox(); assert_eq!( @@ -330,26 +303,23 @@ mod tests { } #[test] - fn environment_path_ref_equality_and_hash_include_environment_id_and_file_system_identity() { + fn environment_path_ref_equality_and_hash_include_file_system_identity() { let path = std::env::temp_dir().join("skills/demo").abs(); let file_system = Arc::new(RecordingFileSystem::default()); let same_file_system: Arc = file_system.clone(); let different_file_system: Arc = Arc::new(RecordingFileSystem::default()); - let left = - EnvironmentPathRef::new("local".to_string(), same_file_system.clone(), path.clone()); - let same = EnvironmentPathRef::new("local".to_string(), same_file_system, path.clone()); - let different_env = - EnvironmentPathRef::new("remote".to_string(), file_system.clone(), path.clone()); - let different_fs = - EnvironmentPathRef::new("local".to_string(), different_file_system, path); + let left = EnvironmentPathRef::new(same_file_system.clone(), path.clone()); + let same = EnvironmentPathRef::new(same_file_system, path.clone()); + let different_path = EnvironmentPathRef::new(file_system.clone(), path.parent().unwrap()); + let different_fs = EnvironmentPathRef::new(different_file_system, path); assert_eq!(left, same); - assert_ne!(left, different_env); + assert_ne!(left, different_path); assert_ne!(left, different_fs); - let set = HashSet::from([left, same, different_env, different_fs]); + let set = HashSet::from([left, same, different_path, different_fs]); assert_eq!(set.len(), 3); } } diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index d90f511d6f..06485f21fd 100644 --- a/codex-rs/tui/src/app/background_requests.rs +++ b/codex-rs/tui/src/app/background_requests.rs @@ -674,6 +674,7 @@ pub(super) async fn fetch_skills_list( request_id, params: SkillsListParams { cwds: vec![cwd], + environment_id: None, force_reload: true, }, }) diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index a403dc232c..345a86698f 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -621,6 +621,7 @@ impl App { app_server .skills_list(codex_app_server_protocol::SkillsListParams { cwds: cwds.clone(), + environment_id: None, force_reload: *force_reload, }) .await,