diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index a8bec6b402..15b4ce2504 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1713,6 +1713,7 @@ mod tests { request_id: request_id(), params: v2::SkillsListParams { cwds: Vec::new(), + environments: 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 70d283994e..6e6ca3bce6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -23,11 +23,23 @@ pub struct SkillsListParams { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub cwds: Vec, + /// Explicit environment/cwd pairs to scan instead of legacy `cwds`. + #[ts(optional = nullable)] + pub environments: 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, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SkillsListEnvironment { + pub environment_id: String, + pub cwd: AbsolutePathBuf, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] 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 90273cd868..28dca548fa 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2604,20 +2604,25 @@ fn skills_list_params_serialization_uses_force_reload() { assert_eq!( serde_json::to_value(SkillsListParams { cwds: Vec::new(), + environments: None, force_reload: false, }) .unwrap(), - json!({}), + json!({ + "environments": null, + }), ); assert_eq!( serde_json::to_value(SkillsListParams { cwds: vec![PathBuf::from("/repo")], + environments: None, force_reload: true, }) .unwrap(), json!({ "cwds": ["/repo"], + "environments": null, "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 9d9542ba6c..d441105f5a 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -13,6 +13,15 @@ pub(crate) struct CatalogRequestProcessor { const SKILLS_LIST_CWD_CONCURRENCY: usize = 5; +struct SkillsListTarget { + path_ref: codex_core::skills::EnvironmentPathRef, +} + +struct ResolvedSkillsListTarget { + primary_cwd: codex_core::skills::EnvironmentPathRef, + config_layer_stack: ConfigLayerStack, +} + fn skills_to_info( skills: &[codex_core::skills::SkillMetadata], disabled_paths: &HashSet, @@ -189,13 +198,22 @@ impl CatalogRequestProcessor { ) -> Result<(AbsolutePathBuf, ConfigLayerStack), String> { let cwd_abs = AbsolutePathBuf::relative_to_current_dir(cwd).map_err(|err| err.to_string())?; + let config_layer_stack = self.load_config_layers_for_cwd(cwd_abs.clone()).await?; + + Ok((cwd_abs, config_layer_stack)) + } + + async fn load_config_layers_for_cwd( + &self, + cwd: AbsolutePathBuf, + ) -> Result { let config_layer_stack = self .config_manager - .load_config_layers_for_cwd(cwd_abs.clone()) + .load_config_layers_for_cwd(cwd) .await .map_err(|err| err.to_string())?; - Ok((cwd_abs, config_layer_stack)) + Ok(config_layer_stack) } async fn load_latest_config( @@ -208,6 +226,175 @@ impl CatalogRequestProcessor { .map_err(|err| internal_error(format!("failed to reload config: {err}"))) } + fn skills_list_targets( + &self, + cwds: Vec, + environments: Option>, + ) -> Result< + Vec>, + JSONRPCErrorError, + > { + if environments.is_some() && !cwds.is_empty() { + return Err(invalid_request( + "skills/list cannot set both `cwds` and `environments`", + )); + } + + let environment_manager = self.thread_manager.environment_manager(); + match environments { + Some(environments) => environments + .into_iter() + .map(|environment| { + let selected_environment = environment_manager + .get_environment(&environment.environment_id) + .ok_or_else(|| { + invalid_request(format!( + "unknown skills/list environment id `{}`", + environment.environment_id + )) + })?; + Ok(Ok(SkillsListTarget { + path_ref: codex_core::skills::EnvironmentPathRef::new( + Some(environment.environment_id), + selected_environment.get_filesystem(), + environment.cwd, + ), + })) + }) + .collect(), + None => { + let cwds = if cwds.is_empty() { + vec![self.config.cwd.to_path_buf()] + } else { + cwds + }; + let environment = environment_manager + .default_environment() + .map(|environment| { + ( + environment_manager + .default_environment_id() + .map(str::to_string), + environment, + ) + }) + .or_else(|| { + environment_manager + .try_local_environment() + .map(|environment| { + ( + Some(codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()), + environment, + ) + }) + }); + Ok(cwds + .into_iter() + .map(|cwd| { + let cwd = match AbsolutePathBuf::relative_to_current_dir(&cwd) { + Ok(cwd) => cwd, + Err(err) => { + return Err(Self::skills_list_error_entry(cwd, err.to_string())); + } + }; + let Some((environment_id, environment)) = environment.as_ref() else { + return Err(Self::skills_list_empty_entry(cwd.to_path_buf())); + }; + Ok(SkillsListTarget { + path_ref: codex_core::skills::EnvironmentPathRef::new( + environment_id.clone(), + environment.get_filesystem(), + cwd, + ), + }) + }) + .collect()) + } + } + } + + async fn resolve_skills_list_target( + &self, + target: SkillsListTarget, + ) -> Result { + let SkillsListTarget { path_ref } = target; + match self + .load_config_layers_for_cwd(path_ref.path().clone()) + .await + { + Ok(config_layer_stack) => Ok(ResolvedSkillsListTarget { + primary_cwd: path_ref, + config_layer_stack, + }), + Err(message) => Err(Self::skills_list_error_entry( + path_ref.path().to_path_buf(), + message, + )), + } + } + + async fn load_skills_list_entry( + &self, + target: ResolvedSkillsListTarget, + config: &Config, + workspace_codex_plugins_enabled: bool, + force_reload: bool, + ) -> codex_app_server_protocol::SkillsListEntry { + let ResolvedSkillsListTarget { + primary_cwd, + config_layer_stack, + } = target; + let cwd = primary_cwd.path().to_path_buf(); + let skill_root_path_ref = Some(primary_cwd.clone()); + let effective_skill_roots = if workspace_codex_plugins_enabled { + let plugins_input = config + .plugins_config_input() + .with_skill_path_ref(skill_root_path_ref.clone()); + self.thread_manager + .plugins_manager() + .effective_skill_roots_for_layer_stack(&config_layer_stack, &plugins_input) + .await + } else { + Vec::new() + }; + let skills_input = codex_core::skills::SkillsLoadInput::new( + Some(primary_cwd), + skill_root_path_ref, + effective_skill_roots, + config_layer_stack, + config.bundled_skills_enabled(), + ); + let outcome = self + .thread_manager + .skills_manager() + .skills_for_cwd(&skills_input, force_reload) + .await; + codex_app_server_protocol::SkillsListEntry { + cwd, + skills: skills_to_info(&outcome.skills, &outcome.disabled_paths), + errors: errors_to_info(&outcome.errors), + } + } + + fn skills_list_empty_entry(cwd: PathBuf) -> codex_app_server_protocol::SkillsListEntry { + codex_app_server_protocol::SkillsListEntry { + cwd, + skills: Vec::new(), + errors: Vec::new(), + } + } + + fn skills_list_error_entry( + cwd: PathBuf, + message: String, + ) -> codex_app_server_protocol::SkillsListEntry { + codex_app_server_protocol::SkillsListEntry { + cwd: cwd.clone(), + skills: Vec::new(), + errors: vec![codex_app_server_protocol::SkillErrorInfo { path: cwd, message }], + } + } + async fn workspace_codex_plugins_enabled( &self, config: &Config, @@ -484,97 +671,37 @@ impl CatalogRequestProcessor { &self, params: SkillsListParams, ) -> Result { - let SkillsListParams { cwds, force_reload } = params; - let cwds = if cwds.is_empty() { - vec![self.config.cwd.to_path_buf()] - } else { - cwds - }; - + let SkillsListParams { + cwds, + environments, + force_reload, + } = params; let config = self.load_latest_config(/*fallback_cwd*/ None).await?; + let targets = self.skills_list_targets(cwds, environments)?; let auth = self.auth_manager.auth().await; let workspace_codex_plugins_enabled = self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await; - 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 default_environment_id = environment_manager - .default_environment_id() - .map(str::to_string); - let fs = environment_manager - .default_environment() - .map(|environment| environment.get_filesystem()); - let local_path_ref = Some(codex_core::skills::EnvironmentPathRef::new( - Some(codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string()), - Arc::clone(&codex_exec_server::LOCAL_FS), - config.cwd.clone(), - )); - let mut data = futures::stream::iter(cwds.into_iter().enumerate()) - .map(|(index, cwd)| { + let mut data = futures::stream::iter(targets.into_iter().enumerate()) + .map(|(index, target)| { let config = &config; - let default_environment_id = default_environment_id.clone(); - let fs = fs.clone(); - let local_path_ref = local_path_ref.clone(); - let plugins_manager = &plugins_manager; - let skills_manager = &skills_manager; async move { - let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await { - Ok(resolved) => resolved, - Err(message) => { - let error_path = cwd.clone(); - return ( - index, - codex_app_server_protocol::SkillsListEntry { - cwd, - skills: Vec::new(), - errors: vec![codex_app_server_protocol::SkillErrorInfo { - path: error_path, - message, - }], - }, - ); - } - }; - let effective_skill_roots = if workspace_codex_plugins_enabled { - let plugins_input = config - .plugins_config_input() - .with_skill_path_ref(local_path_ref.clone()); - plugins_manager - .effective_skill_roots_for_layer_stack( - &config_layer_stack, - &plugins_input, - ) - .await - } else { - Vec::new() - }; - let skills_input = codex_core::skills::SkillsLoadInput::new( - fs.clone().map(|file_system| { - codex_core::skills::EnvironmentPathRef::new( - default_environment_id, - file_system, - cwd_abs.clone(), - ) - }), - local_path_ref, - effective_skill_roots, - config_layer_stack, - config.bundled_skills_enabled(), - ); - let outcome = skills_manager - .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); - ( - index, - codex_app_server_protocol::SkillsListEntry { - cwd, - skills, - errors, + let entry = match target { + Ok(target) => match self.resolve_skills_list_target(target).await { + Ok(target) => { + self.load_skills_list_entry( + target, + config, + workspace_codex_plugins_enabled, + force_reload, + ) + .await + } + Err(entry) => entry, }, - ) + Err(entry) => entry, + }; + (index, entry) } }) .buffer_unordered(SKILLS_LIST_CWD_CONCURRENCY) 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 74100bc19f..602077989c 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -228,6 +228,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()], + environments: None, force_reload: true, }) .await?; @@ -278,6 +279,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()], + environments: None, force_reload: false, }) .await?; @@ -352,6 +354,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()], + environments: None, force_reload: true, }) .await?; @@ -381,7 +384,7 @@ async fn skills_list_excludes_plugin_skills_when_workspace_codex_plugins_disable } #[tokio::test] -async fn skills_list_skips_cwd_roots_when_environment_disabled() -> Result<()> { +async fn skills_list_skips_skill_roots_when_environment_disabled() -> Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; write_skill(&codex_home, "home-skill")?; @@ -402,6 +405,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()], + environments: None, force_reload: true, }) .await?; @@ -415,18 +419,7 @@ async fn skills_list_skips_cwd_roots_when_environment_disabled() -> Result<()> { 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") - ); + assert_eq!(data[0].skills, Vec::new()); Ok(()) } @@ -442,6 +435,7 @@ async fn skills_list_accepts_relative_cwds() -> Result<()> { let request_id = mcp .send_skills_list_request(SkillsListParams { cwds: vec![relative_cwd.clone()], + environments: None, force_reload: true, }) .await?; @@ -473,6 +467,7 @@ async fn skills_list_preserves_requested_cwd_order() -> Result<()> { first_cwd.path().to_path_buf(), second_cwd.path().to_path_buf(), ], + environments: None, force_reload: true, }) .await?; @@ -507,6 +502,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()], + environments: None, force_reload: false, }) .await?; @@ -534,6 +530,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()], + environments: None, force_reload: false, }) .await?; @@ -554,6 +551,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()], + environments: None, force_reload: true, }) .await?; @@ -591,6 +589,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()], + environments: None, force_reload: true, }) .await?; @@ -664,6 +663,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()], + environments: None, force_reload: false, }) .await?; diff --git a/codex-rs/tui/src/app/background_requests.rs b/codex-rs/tui/src/app/background_requests.rs index d90f511d6f..4904b1350c 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], + environments: 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..16a8d1db2a 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(), + environments: None, force_reload: *force_reload, }) .await,