diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index bc5953803ed7..eb151ebe703c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2813,9 +2813,7 @@ name = "codex-core-skills" version = "0.0.0" dependencies = [ "anyhow", - "codex-analytics", "codex-config", - "codex-context-fragments", "codex-exec-server", "codex-login", "codex-model-provider", diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index ed040aa8d9e9..dd8dfd73aff4 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -9,7 +9,6 @@ use codex_core::NewThread; use codex_core::StartThreadOptions; use codex_core::ThreadManager; use codex_core::config::Config; -use codex_exec_server::EnvironmentManager; use codex_extension_api::AgentSpawnFuture; use codex_extension_api::AgentSpawner; use codex_extension_api::ExtensionEventSink; @@ -35,8 +34,6 @@ pub(crate) struct ThreadExtensionDependencies { pub(crate) analytics_events_client: AnalyticsEventsClient, pub(crate) thread_manager: Weak, pub(crate) goal_service: Arc, - pub(crate) environment_manager: Arc, - pub(crate) executor_skill_provider: Arc, /// Process-scoped persistence backend for extensions that need stored thread history. pub(crate) thread_store: Arc, } @@ -55,8 +52,6 @@ where analytics_events_client, thread_manager, goal_service, - environment_manager, - executor_skill_provider, thread_store: _thread_store, } = dependencies; let mut builder = ExtensionRegistryBuilder::::with_event_sink(event_sink); @@ -74,25 +69,13 @@ where codex_guardian::install(&mut builder, guardian_agent_spawner); codex_memories_extension::install(&mut builder, codex_otel::global()); codex_mcp_extension::install(&mut builder); - codex_mcp_extension::install_executor_plugins(&mut builder, environment_manager); codex_web_search_extension::install(&mut builder, auth_manager.clone()); codex_image_generation_extension::install(&mut builder, auth_manager, |config: &Config| { Some(config.codex_home.clone()) }); - let skill_providers = codex_skills_extension::SkillProviders::new() - .with_executor_provider(executor_skill_provider) - .with_orchestrator_provider(Arc::new( - codex_skills_extension::OrchestratorSkillProvider::new(), - )); - codex_skills_extension::install_with_providers( - &mut builder, - skill_providers, - |config: &Config| codex_skills_extension::SkillsExtensionConfig { - include_instructions: config.include_skill_instructions, - bundled_skills_enabled: config.bundled_skills_enabled(), - orchestrator_skills_enabled: config.orchestrator_skills_enabled, - }, - ); + codex_skills_extension::install(&mut builder, |config: &Config| { + config.orchestrator_skills_enabled + }); Arc::new(builder.build()) } diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index 848d201f2ea6..426d8fcc67b6 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -216,12 +216,6 @@ mod tests { .expect("refresh tests require state db"); let thread_store = thread_store_from_config(&good_config, Some(state_db.clone())); let environment_manager = Arc::new(EnvironmentManager::default_for_tests()); - let executor_skill_provider: Arc = Arc::new( - codex_skills_extension::ExecutorSkillProvider::new_with_restriction_product( - Arc::clone(&environment_manager), - SessionSource::Exec.restriction_product(), - ), - ); let thread_manager = Arc::new_cyclic(|thread_manager| { ThreadManager::new( &good_config, @@ -238,7 +232,6 @@ mod tests { thread_manager: thread_manager.clone(), goal_service: Arc::new(codex_goal_extension::GoalService::new()), environment_manager: Arc::clone(&environment_manager), - executor_skill_provider: Arc::clone(&executor_skill_provider), thread_store: Arc::clone(&thread_store), }, ), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 41ecbfe5613e..9ad1b71d6000 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -336,14 +336,6 @@ impl MessageProcessor { // resumed, or forked threads to a different persistence backend/root. let thread_store = codex_core::thread_store_from_config(config.as_ref(), state_db.clone()); let environment_manager_for_requests = Arc::clone(&environment_manager); - let environment_manager_for_extensions = Arc::clone(&environment_manager); - let restriction_product = session_source.restriction_product(); - let executor_skill_provider: Arc = Arc::new( - codex_skills_extension::ExecutorSkillProvider::new_with_restriction_product( - Arc::clone(&environment_manager_for_extensions), - restriction_product, - ), - ); let goal_service = Arc::new(GoalService::new()); let thread_manager = Arc::new_cyclic(|thread_manager| { ThreadManager::new( @@ -363,8 +355,6 @@ impl MessageProcessor { analytics_events_client: analytics_events_client.clone(), thread_manager: thread_manager.clone(), goal_service: Arc::clone(&goal_service), - environment_manager: Arc::clone(&environment_manager_for_extensions), - executor_skill_provider: Arc::clone(&executor_skill_provider), thread_store: Arc::clone(&thread_store), }, ), diff --git a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs index 77b9bfcbf364..bc0a64ef7298 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs +++ b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs @@ -215,6 +215,7 @@ impl ExternalAgentSessionImporter { .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), }, dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: Some(MultiAgentVersion::V1), initial_window_id: uuid::Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/common/rollout.rs b/codex-rs/app-server/tests/common/rollout.rs index ec01dacc47ad..8d2567c010d4 100644 --- a/codex-rs/app-server/tests/common/rollout.rs +++ b/codex-rs/app-server/tests/common/rollout.rs @@ -200,6 +200,7 @@ fn create_fake_rollout_with_source_and_parent_thread_id( model_provider: model_provider.map(str::to_string), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, @@ -288,6 +289,7 @@ pub fn create_fake_rollout_with_text_elements( model_provider: model_provider.map(str::to_string), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/app-server/tests/suite/conversation_summary.rs b/codex-rs/app-server/tests/suite/conversation_summary.rs index 7f964d314722..63ad473261d6 100644 --- a/codex-rs/app-server/tests/suite/conversation_summary.rs +++ b/codex-rs/app-server/tests/suite/conversation_summary.rs @@ -131,6 +131,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() -> originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs index 8fdb38e5ead8..7e0db1b5b3b6 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs @@ -158,6 +158,7 @@ async fn thread_delete_with_non_local_thread_store_does_not_create_local_persist originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 6b427b0d976e..9f677617576e 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -1369,6 +1369,7 @@ async fn seed_pathless_store_thread( originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index ce95f4810c66..533087a11e96 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -2070,6 +2070,7 @@ stream_max_retries = 0 model_provider: Some("mock_provider".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs index f647dc2cfd4f..231ede47b9ba 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs @@ -218,6 +218,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> { originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { diff --git a/codex-rs/codex-mcp/src/resource_client.rs b/codex-rs/codex-mcp/src/resource_client.rs index 4406b81a53c6..6f479c4027d8 100644 --- a/codex-rs/codex-mcp/src/resource_client.rs +++ b/codex-rs/codex-mcp/src/resource_client.rs @@ -33,7 +33,13 @@ pub struct McpResourceReadResult { /// snapshot, so calls automatically use replacements installed during startup and refresh. #[derive(Clone)] pub struct McpResourceClient { - manager: Arc>, + manager: ResourceManager, +} + +#[derive(Clone)] +enum ResourceManager { + Live(Arc>), + Snapshot(Arc), } /// Opaque identity for the manager currently used by an MCP resource client. @@ -59,19 +65,36 @@ impl std::fmt::Debug for McpResourceClient { impl McpResourceClient { /// Creates a resource client backed by the session's replaceable MCP manager. pub fn new(manager: Arc>) -> Self { - Self { manager } + Self { + manager: ResourceManager::Live(manager), + } + } + + /// Pins resource calls to the manager generation that is current now. + pub fn snapshot(&self) -> Self { + Self { + manager: ResourceManager::Snapshot(self.manager_snapshot()), + } + } + + /// Returns the exact manager generation used by this client right now. + pub fn manager_snapshot(&self) -> Arc { + match &self.manager { + ResourceManager::Live(manager) => manager.load_full(), + ResourceManager::Snapshot(manager) => Arc::clone(manager), + } } /// Returns an identity that changes whenever the published manager changes. pub fn cache_key(&self) -> McpResourceClientCacheKey { - McpResourceClientCacheKey(Arc::downgrade(&self.manager.load_full())) + McpResourceClientCacheKey(Arc::downgrade(&self.manager_snapshot())) } /// Returns whether the current manager contains the named server. /// /// This does not wait for server startup or imply that startup succeeded. pub async fn has_server(&self, server: &str) -> bool { - self.manager.load_full().contains_server(server) + self.manager_snapshot().contains_server(server) } /// Lists one resource page from the named server. @@ -83,8 +106,7 @@ impl McpResourceClient { let params = cursor.map(|cursor| PaginatedRequestParams::default().with_cursor(Some(cursor))); let result = self - .manager - .load_full() + .manager_snapshot() .list_resources(server, params) .await?; let resources = result @@ -101,8 +123,7 @@ impl McpResourceClient { /// Reads one resource from the named server. pub async fn read_resource(&self, server: &str, uri: &str) -> Result { let result = self - .manager - .load_full() + .manager_snapshot() .read_resource(server, ReadResourceRequestParams::new(uri.to_string())) .await?; let contents = result diff --git a/codex-rs/connectors/src/snapshot.rs b/codex-rs/connectors/src/snapshot.rs index a07b1ef5894b..92ad48024245 100644 --- a/codex-rs/connectors/src/snapshot.rs +++ b/codex-rs/connectors/src/snapshot.rs @@ -8,7 +8,6 @@ use codex_plugin::PluginCapabilitySummary; /// Connector declarations contributed by one plugin package. #[derive(Clone, Debug, PartialEq, Eq)] pub struct PluginConnectorSource { - plugin_id: String, plugin_display_name: String, connector_ids: Vec, } @@ -16,12 +15,10 @@ pub struct PluginConnectorSource { impl PluginConnectorSource { /// Creates one plugin source from parsed app declarations. pub fn new( - plugin_id: impl Into, plugin_display_name: impl Into, declarations: impl IntoIterator, ) -> Self { Self::from_connector_ids( - plugin_id, plugin_display_name, declarations .into_iter() @@ -31,7 +28,6 @@ impl PluginConnectorSource { /// Creates one plugin source from connector IDs that were already parsed. pub fn from_connector_ids( - plugin_id: impl Into, plugin_display_name: impl Into, connector_ids: impl IntoIterator, ) -> Self { @@ -42,7 +38,6 @@ impl PluginConnectorSource { .filter(|connector_id| seen_connector_ids.insert(connector_id.clone())) .collect(); Self { - plugin_id: plugin_id.into(), plugin_display_name: plugin_display_name.into(), connector_ids, } @@ -105,7 +100,6 @@ impl ConnectorSnapshot { pub fn from_plugin_capability_summaries(summaries: &[PluginCapabilitySummary]) -> Self { Self::from_plugin_sources(summaries.iter().map(|summary| { PluginConnectorSource::from_connector_ids( - summary.config_name.clone(), summary.display_name.clone(), summary.app_connector_ids.clone(), ) diff --git a/codex-rs/connectors/src/snapshot_tests.rs b/codex-rs/connectors/src/snapshot_tests.rs index 3087bc73cdf9..9322f5b07b87 100644 --- a/codex-rs/connectors/src/snapshot_tests.rs +++ b/codex-rs/connectors/src/snapshot_tests.rs @@ -6,14 +6,12 @@ use super::PluginConnectorSource; #[test] fn snapshot_merges_sources_in_order_and_dedupes_provenance() { - let host_source = source("host", "Zulu", &["calendar", "calendar"]); - let host = ConnectorSnapshot::from_plugin_sources([ - source("skills", "Skills only", &[]), - host_source.clone(), - ]); + let host_source = source("Zulu", &["calendar", "calendar"]); + let host = + ConnectorSnapshot::from_plugin_sources([source("Skills only", &[]), host_source.clone()]); let selected = ConnectorSnapshot::from_plugin_sources([ - source("selected-a", "Alpha", &["drive", "calendar"]), - source("selected-b", "Alpha", &["calendar"]), + source("Alpha", &["drive", "calendar"]), + source("Alpha", &["calendar"]), ]); let merged = host.merged_with(&selected); @@ -36,9 +34,8 @@ fn snapshot_merges_sources_in_order_and_dedupes_provenance() { ); } -fn source(id: &str, display_name: &str, connector_ids: &[&str]) -> PluginConnectorSource { +fn source(display_name: &str, connector_ids: &[&str]) -> PluginConnectorSource { PluginConnectorSource::from_connector_ids( - id, display_name, connector_ids .iter() diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index f86ecdf75b8f..d10e556415b0 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -14,9 +14,7 @@ workspace = true [dependencies] anyhow = { workspace = true } -codex-analytics = { workspace = true } codex-config = { workspace = true } -codex-context-fragments = { workspace = true } codex-exec-server = { workspace = true } codex-login = { workspace = true } codex-model-provider = { workspace = true } diff --git a/codex-rs/core-skills/src/executor_runtime.rs b/codex-rs/core-skills/src/executor_runtime.rs new file mode 100644 index 000000000000..3dcd9908f31d --- /dev/null +++ b/codex-rs/core-skills/src/executor_runtime.rs @@ -0,0 +1,248 @@ +use std::sync::Arc; +use std::sync::Mutex; + +use codex_exec_server::ResolvedSelectedCapabilityRoot; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_protocol::protocol::Product; + +use crate::loader::EnvironmentSkillMetadata; +use crate::loader::load_environment_skills_from_root; +use crate::runtime::SkillAuthority; +use crate::runtime::SkillCatalog; +use crate::runtime::SkillCatalogEntry; +use crate::runtime::SkillPackageId; +use crate::runtime::SkillReadRequest; +use crate::runtime::SkillReadResult; +use crate::runtime::SkillResourceId; +use crate::runtime::SkillSource; +use crate::runtime::SkillSourceError; +use crate::runtime::SkillSourceFuture; +use crate::runtime::SkillSourceIdentity; +use crate::runtime::SkillSourceKind; + +/// Session-lifetime cache for catalogs discovered from stable executor capability roots. +/// +/// Cache entries are keyed by the complete selected root and product restriction, not by the +/// process-local [`ResolvedSelectedCapabilityRoot`]. Environment availability therefore controls +/// whether a cached catalog is projected into a model step, but temporarily losing an environment +/// does not invalidate its catalog. A different root identity, environment ID, path, or product +/// restriction produces a cache miss and a new discovery. +/// The entry also owns the source identity used for injection deduplication, so reconnecting the +/// same stable environment does not make an unchanged skill look like a different instruction. +/// +/// There is intentionally no filesystem-based invalidation. Selected environment contents are +/// treated as stable for the lifetime of a session. Dropping this cache at session shutdown is the +/// only operation that invalidates successful entries. Catalogs containing warnings are not +/// cached, so transient discovery failures can recover on a later model step. +#[derive(Default)] +pub struct ExecutorSkillCatalogCache { + entries: Mutex>, +} + +#[derive(Clone)] +pub(crate) struct CachedExecutorSkillCatalog { + key: ExecutorSkillCatalogCacheKey, + catalog: Arc, + identity: SkillSourceIdentity, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct ExecutorSkillCatalogCacheKey { + selected_root: SelectedCapabilityRoot, + restriction_product: Option, +} + +impl ExecutorSkillCatalogCache { + pub(crate) async fn catalog_for_stable_root( + &self, + root: &ResolvedSelectedCapabilityRoot, + restriction_product: Option, + ) -> CachedExecutorSkillCatalog { + let key = ExecutorSkillCatalogCacheKey { + selected_root: root.selected_root().clone(), + restriction_product, + }; + if let Some(cached) = self.catalog(&key) { + return cached; + } + + // Do not hold the cache lock across executor I/O. Concurrent first loads may duplicate + // discovery, but the second check below ensures only one result becomes authoritative. + let identity = SkillSourceIdentity::from_owner(Arc::new(key.clone())); + let source = ExecutorSkillSource::new(root.clone(), restriction_product, identity.clone()); + let discovered = Arc::new(source.load_catalog().await); + let discovered = CachedExecutorSkillCatalog { + key: key.clone(), + catalog: discovered, + identity, + }; + if !discovered.warnings().is_empty() { + return discovered; + } + let mut entries = self + .entries + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(entry) = entries.iter().find(|entry| entry.key == key) { + return entry.clone(); + } + entries.push(discovered.clone()); + discovered + } + + fn catalog(&self, key: &ExecutorSkillCatalogCacheKey) -> Option { + self.entries + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .iter() + .find(|entry| &entry.key == key) + .cloned() + } +} + +impl CachedExecutorSkillCatalog { + pub(crate) fn catalog(&self) -> &SkillCatalog { + self.catalog.as_ref() + } + + pub(crate) fn identity(&self) -> SkillSourceIdentity { + self.identity.clone() + } + + fn warnings(&self) -> &[String] { + &self.catalog.warnings + } +} + +pub(crate) struct ExecutorSkillSource { + root: ResolvedSelectedCapabilityRoot, + authority: SkillAuthority, + restriction_product: Option, + identity: SkillSourceIdentity, +} + +impl ExecutorSkillSource { + pub(crate) fn new( + root: ResolvedSelectedCapabilityRoot, + restriction_product: Option, + identity: SkillSourceIdentity, + ) -> Self { + Self { + authority: SkillAuthority::new( + SkillSourceKind::Executor, + root.selected_root().id.clone(), + ), + identity, + root, + restriction_product, + } + } + + async fn load_catalog(&self) -> SkillCatalog { + let CapabilityRootLocation::Environment { + environment_id, + path, + } = &self.root.selected_root().location; + let outcome = load_environment_skills_from_root( + self.root.file_system().as_ref(), + path, + self.restriction_product, + ) + .await; + let mut catalog = SkillCatalog { + warnings: outcome.warnings, + ..Default::default() + }; + for skill in outcome.skills { + catalog.push_entry(executor_catalog_entry( + skill, + self.authority.clone(), + &self.root.selected_root().id, + environment_id, + )); + } + catalog + } +} + +impl SkillSource for ExecutorSkillSource { + fn identity(&self) -> SkillSourceIdentity { + self.identity.clone() + } + + fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> { + Box::pin(async move { Ok(self.load_catalog().await) }) + } + + fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> { + Box::pin(async move { + if request.authority != self.authority || request.package.0 != request.resource.as_str() + { + return Err(SkillSourceError::new( + "executor skill resource does not match its captured source", + )); + } + let CapabilityRootLocation::Environment { environment_id, .. } = + &self.root.selected_root().location; + let Some((resource_environment, path)) = request.resource.environment_path() else { + return Err(SkillSourceError::new( + "executor skill resource has no environment path", + )); + }; + if resource_environment != environment_id { + return Err(SkillSourceError::new( + "executor skill resource belongs to a different environment", + )); + } + let contents = self + .root + .file_system() + .read_file_text(path, /*sandbox*/ None) + .await + .map_err(|err| { + SkillSourceError::new(format!( + "failed to read executor skill resource {}: {err}", + request.resource.as_str() + )) + })?; + Ok(SkillReadResult { + resource: request.resource, + contents, + }) + }) + } +} + +fn executor_catalog_entry( + skill: EnvironmentSkillMetadata, + authority: SkillAuthority, + root_id: &str, + environment_id: &str, +) -> SkillCatalogEntry { + let prompt_visible = skill.allows_implicit_invocation(); + let path = skill.path_to_skills_md.inferred_native_path_string(); + let display_path = format!( + "skill://{root_id}/{}", + path.replace('\\', "/").trim_start_matches('/') + ); + let entry = SkillCatalogEntry::new( + SkillPackageId(display_path.clone()), + authority, + skill.name, + skill.description, + SkillResourceId::environment( + display_path.clone(), + environment_id, + skill.path_to_skills_md, + ), + ) + .with_short_description(skill.short_description) + .with_display_path(display_path) + .with_dependencies(skill.dependencies); + if prompt_visible { + entry + } else { + entry.hidden_from_prompt() + } +} diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 201e7959fbd5..0fdd8b40a68a 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -1,228 +1,13 @@ -use std::collections::HashMap; use std::collections::HashSet; -use std::sync::Arc; -use crate::SkillLoadOutcome; -use crate::SkillMetadata; -use crate::build_skill_name_counts; -use codex_analytics::AnalyticsEventsClient; -use codex_analytics::InvocationType; -use codex_analytics::SkillInvocation; -use codex_analytics::TrackEventsContext; -use codex_exec_server::LOCAL_FS; -use codex_otel::SessionTelemetry; -use codex_protocol::user_input::UserInput; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::PathUri; use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; -#[derive(Debug, Default)] -pub struct SkillInjections { - pub items: Vec, - pub warnings: Vec, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SkillInjection { - pub name: String, - pub path: String, - pub contents: String, -} - -/// Host skill prompts that have already been injected by an extension for this -/// turn. -/// -/// Core uses this to keep the legacy skill-injection path from sending the same -/// host `SKILL.md` body again while the skills extension is being wired in. -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct InjectedHostSkillPrompts { - paths: HashSet, -} - -impl InjectedHostSkillPrompts { - pub fn insert_path(&mut self, path: impl Into) { - let path = path.into(); - self.paths.insert(normalize_host_skill_path(&path)); - self.paths.insert(path); - } - - pub fn is_empty(&self) -> bool { - self.paths.is_empty() - } - - pub fn contains_path(&self, path: &str) -> bool { - self.paths.contains(path) || self.paths.contains(&normalize_host_skill_path(path)) - } -} - -#[tracing::instrument( - level = "trace", - skip_all, - fields(mentioned_skill_count = mentioned_skills.len()) -)] -pub async fn build_skill_injections( - mentioned_skills: &[SkillMetadata], - loaded_skills: Option<&SkillLoadOutcome>, - otel: Option<&SessionTelemetry>, - analytics_client: &AnalyticsEventsClient, - tracking: TrackEventsContext, -) -> SkillInjections { - if mentioned_skills.is_empty() { - return SkillInjections::default(); - } - - let mut result = SkillInjections { - items: Vec::with_capacity(mentioned_skills.len()), - warnings: Vec::new(), - }; - let mut invocations = Vec::new(); - - for skill in mentioned_skills { - let fs = loaded_skills - .and_then(|outcome| outcome.file_system_for_skill(skill)) - .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - let path = PathUri::from_abs_path(&skill.path_to_skills_md); - match fs.read_file_text(&path, /*sandbox*/ None).await { - Ok(contents) => { - emit_skill_injected_metric(otel, skill, "ok"); - invocations.push(SkillInvocation { - skill_name: skill.name.clone(), - skill_scope: skill.scope, - skill_path: skill.path_to_skills_md.to_path_buf(), - plugin_id: skill.plugin_id.clone(), - invocation_type: InvocationType::Explicit, - }); - result.items.push(SkillInjection { - name: skill.name.clone(), - path: skill.path_to_skills_md.to_string_lossy().into_owned(), - contents, - }); - } - Err(err) => { - emit_skill_injected_metric(otel, skill, "error"); - let message = format!( - "Failed to load skill {name} at {path}: {err:#}", - name = skill.name, - path = skill.path_to_skills_md.display() - ); - result.warnings.push(message); - } - } - } - - analytics_client.track_skill_invocations(tracking, invocations); - - result -} - -fn normalize_host_skill_path(path: &str) -> String { - normalize_skill_path(path).replace('\\', "/") -} - -fn emit_skill_injected_metric( - otel: Option<&SessionTelemetry>, - skill: &SkillMetadata, - status: &str, -) { - let Some(otel) = otel else { - return; - }; - - otel.counter( - "codex.skill.injected", - /*inc*/ 1, - &[("status", status), ("skill", skill.name.as_str())], - ); -} - -/// Collect explicitly mentioned skills from structured and text mentions. -/// -/// Structured `UserInput::Skill` selections are resolved first by path against -/// enabled skills. Text inputs are then scanned to extract `$skill-name` tokens, and we -/// iterate `skills` in their existing order to preserve prior ordering semantics. -/// Explicit links are resolved by path and plain names are only used when the match -/// is unambiguous. -/// -/// Complexity: `O(T + (N_s + N_t) * S)` time, `O(S + M)` space, where: -/// `S` = number of skills, `T` = total text length, `N_s` = number of structured skill inputs, -/// `N_t` = number of text inputs, `M` = max mentions parsed from a single text input. -pub fn collect_explicit_skill_mentions( - inputs: &[UserInput], - skills: &[SkillMetadata], - disabled_paths: &HashSet, - connector_slug_counts: &HashMap, -) -> Vec { - let skill_name_counts = build_skill_name_counts(skills, disabled_paths).0; - - let selection_context = SkillSelectionContext { - skills, - disabled_paths, - skill_name_counts: &skill_name_counts, - connector_slug_counts, - }; - let mut selected: Vec = Vec::new(); - let mut seen_names: HashSet = HashSet::new(); - let mut seen_paths: HashSet = HashSet::new(); - let mut blocked_plain_names: HashSet = HashSet::new(); - - for input in inputs { - if let UserInput::Skill { name, path, .. } = input { - blocked_plain_names.insert(name.clone()); - let Ok(path) = AbsolutePathBuf::relative_to_current_dir(path) else { - continue; - }; - - if selection_context.disabled_paths.contains(&path) || seen_paths.contains(&path) { - continue; - } - - if let Some(skill) = selection_context - .skills - .iter() - .find(|skill| skill.path_to_skills_md == path) - { - seen_paths.insert(skill.path_to_skills_md.clone()); - seen_names.insert(skill.name.clone()); - selected.push(skill.clone()); - } - } - } - - for input in inputs { - if let UserInput::Text { text, .. } = input { - let mentioned_names = extract_tool_mentions(text); - select_skills_from_mentions( - &selection_context, - &blocked_plain_names, - &mentioned_names, - &mut seen_names, - &mut seen_paths, - &mut selected, - ); - } - } - - selected -} - -struct SkillSelectionContext<'a> { - skills: &'a [SkillMetadata], - disabled_paths: &'a HashSet, - skill_name_counts: &'a HashMap, - connector_slug_counts: &'a HashMap, -} - pub struct ToolMentions<'a> { - names: HashSet<&'a str>, paths: HashSet<&'a str>, plain_names: HashSet<&'a str>, } impl<'a> ToolMentions<'a> { - fn is_empty(&self) -> bool { - self.names.is_empty() && self.paths.is_empty() - } - pub fn plain_names(&self) -> impl Iterator + '_ { self.plain_names.iter().copied() } @@ -276,22 +61,16 @@ pub fn plugin_config_name_from_path(path: &str) -> Option<&str> { .filter(|value| !value.is_empty()) } -pub(crate) fn normalize_skill_path(path: &str) -> &str { - path.strip_prefix(SKILL_PATH_PREFIX).unwrap_or(path) -} - /// Extract `$tool-name` mentions from a single text input. /// -/// Supports explicit resource links in the form `[$tool-name](resource path)`. When a -/// resource path is present, it is captured for exact path matching while also tracking -/// the name for fallback matching. +/// Supports explicit resource links in the form `[$tool-name](resource path)`. +/// Linked mentions are selected by their exact path rather than falling back to the name. pub fn extract_tool_mentions(text: &str) -> ToolMentions<'_> { extract_tool_mentions_with_sigil(text, TOOL_MENTION_SIGIL) } pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> { let text_bytes = text.as_bytes(); - let mut mentioned_names: HashSet<&str> = HashSet::new(); let mut mentioned_paths: HashSet<&str> = HashSet::new(); let mut plain_names: HashSet<&str> = HashSet::new(); @@ -303,12 +82,6 @@ pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions parse_linked_tool_mention(text, text_bytes, index, sigil) { if !is_common_env_var(name) { - if !matches!( - tool_kind_for_path(path), - ToolMentionKind::App | ToolMentionKind::Mcp | ToolMentionKind::Plugin - ) { - mentioned_names.insert(name); - } mentioned_paths.insert(path); } index = end_index; @@ -339,97 +112,17 @@ pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions let name = &text[name_start..name_end]; if !is_common_env_var(name) { - mentioned_names.insert(name); plain_names.insert(name); } index = name_end; } ToolMentions { - names: mentioned_names, paths: mentioned_paths, plain_names, } } -/// Select mentioned skills while preserving the order of `skills`. -fn select_skills_from_mentions( - selection_context: &SkillSelectionContext<'_>, - blocked_plain_names: &HashSet, - mentions: &ToolMentions<'_>, - seen_names: &mut HashSet, - seen_paths: &mut HashSet, - selected: &mut Vec, -) { - if mentions.is_empty() { - return; - } - - let mention_skill_paths: HashSet<&str> = mentions - .paths() - .filter(|path| { - !matches!( - tool_kind_for_path(path), - ToolMentionKind::App | ToolMentionKind::Mcp | ToolMentionKind::Plugin - ) - }) - .map(normalize_skill_path) - .collect(); - - for skill in selection_context.skills { - if selection_context - .disabled_paths - .contains(&skill.path_to_skills_md) - || seen_paths.contains(&skill.path_to_skills_md) - { - continue; - } - - let path_str = skill.path_to_skills_md.to_string_lossy(); - if mention_skill_paths.contains(path_str.as_ref()) { - seen_paths.insert(skill.path_to_skills_md.clone()); - seen_names.insert(skill.name.clone()); - selected.push(skill.clone()); - } - } - - for skill in selection_context.skills { - if selection_context - .disabled_paths - .contains(&skill.path_to_skills_md) - || seen_paths.contains(&skill.path_to_skills_md) - { - continue; - } - - if blocked_plain_names.contains(skill.name.as_str()) { - continue; - } - if !mentions.plain_names.contains(skill.name.as_str()) { - continue; - } - - let skill_count = selection_context - .skill_name_counts - .get(skill.name.as_str()) - .copied() - .unwrap_or(0); - let connector_count = selection_context - .connector_slug_counts - .get(&skill.name.to_ascii_lowercase()) - .copied() - .unwrap_or(0); - if skill_count != 1 || connector_count != 0 { - continue; - } - - if seen_names.insert(skill.name.clone()) { - seen_paths.insert(skill.path_to_skills_md.clone()); - selected.push(skill.clone()); - } - } -} - fn parse_linked_tool_mention<'a>( text: &'a str, text_bytes: &[u8], @@ -505,38 +198,6 @@ fn is_common_env_var(name: &str) -> bool { ) } -#[cfg(test)] -fn text_mentions_skill(text: &str, skill_name: &str) -> bool { - if skill_name.is_empty() { - return false; - } - - let text_bytes = text.as_bytes(); - let skill_bytes = skill_name.as_bytes(); - - for (index, byte) in text_bytes.iter().copied().enumerate() { - if byte != b'$' { - continue; - } - - let name_start = index + 1; - let Some(rest) = text_bytes.get(name_start..) else { - continue; - }; - if !rest.starts_with(skill_bytes) { - continue; - } - - let after_index = name_start + skill_bytes.len(); - let after = text_bytes.get(after_index).copied(); - if after.is_none_or(|b| !is_mention_name_char(b)) { - return true; - } - } - - false -} - fn is_mention_name_char(byte: u8) -> bool { matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-' | b':') } diff --git a/codex-rs/core-skills/src/injection_tests.rs b/codex-rs/core-skills/src/injection_tests.rs index 78aa19589527..cb99aba4af73 100644 --- a/codex-rs/core-skills/src/injection_tests.rs +++ b/codex-rs/core-skills/src/injection_tests.rs @@ -1,94 +1,22 @@ use super::*; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_absolute_path::test_support::PathBufExt; -use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; -use std::collections::HashMap; use std::collections::HashSet; -fn make_skill(name: &str, path: &str) -> SkillMetadata { - SkillMetadata { - name: name.to_string(), - description: format!("{name} skill"), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf(path).abs(), - scope: codex_protocol::protocol::SkillScope::User, - plugin_id: None, - } -} - fn set<'a>(items: &'a [&'a str]) -> HashSet<&'a str> { items.iter().copied().collect() } fn assert_mentions(text: &str, expected_names: &[&str], expected_paths: &[&str]) { let mentions = extract_tool_mentions(text); - assert_eq!(mentions.names, set(expected_names)); + assert_eq!(mentions.plain_names, set(expected_names)); assert_eq!(mentions.paths, set(expected_paths)); } -fn linked_skill_mention(name: &str, unix_path: &str) -> String { - format!("[${name}]({})", test_path_buf(unix_path).display()) -} - -fn collect_mentions( - inputs: &[UserInput], - skills: &[SkillMetadata], - disabled_paths: &HashSet, - connector_slug_counts: &HashMap, -) -> Vec { - collect_explicit_skill_mentions(inputs, skills, disabled_paths, connector_slug_counts) -} - -#[test] -fn text_mentions_skill_requires_exact_boundary() { - assert_eq!( - true, - text_mentions_skill("use $notion-research-doc please", "notion-research-doc") - ); - assert_eq!( - true, - text_mentions_skill("($notion-research-doc)", "notion-research-doc") - ); - assert_eq!( - true, - text_mentions_skill("$notion-research-doc.", "notion-research-doc") - ); - assert_eq!( - false, - text_mentions_skill("$notion-research-docs", "notion-research-doc") - ); - assert_eq!( - false, - text_mentions_skill("$notion-research-doc_extra", "notion-research-doc") - ); -} - -#[test] -fn text_mentions_skill_handles_end_boundary_and_near_misses() { - assert_eq!(true, text_mentions_skill("$alpha-skill", "alpha-skill")); - assert_eq!(false, text_mentions_skill("$alpha-skillx", "alpha-skill")); - assert_eq!( - true, - text_mentions_skill("$alpha-skillx and later $alpha-skill ", "alpha-skill") - ); -} - -#[test] -fn text_mentions_skill_handles_many_dollars_without_looping() { - let prefix = "$".repeat(256); - let text = format!("{prefix} not-a-mention"); - assert_eq!(false, text_mentions_skill(&text, "alpha-skill")); -} - #[test] fn extract_tool_mentions_handles_plain_and_linked_mentions() { assert_mentions( "use $alpha and [$beta](/tmp/beta)", - &["alpha", "beta"], + &["alpha"], &["/tmp/beta"], ); } @@ -109,7 +37,7 @@ fn extract_tool_mentions_requires_link_syntax() { #[test] fn extract_tool_mentions_trims_linked_paths_and_allows_spacing() { - assert_mentions("use [$beta] ( /tmp/beta )", &["beta"], &["/tmp/beta"]); + assert_mentions("use [$beta] ( /tmp/beta )", &[], &["/tmp/beta"]); } #[test] @@ -129,230 +57,3 @@ fn extract_tool_mentions_keeps_plugin_skill_namespaces() { &[], ); } - -#[test] -fn collect_explicit_skill_mentions_text_respects_skill_order() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let beta = make_skill("beta-skill", "/tmp/beta"); - let skills = vec![beta.clone(), alpha.clone()]; - let inputs = vec![UserInput::Text { - text: "first $alpha-skill then $beta-skill".to_string(), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - // Text scanning should not change the previous selection ordering semantics. - assert_eq!(selected, vec![beta, alpha]); -} - -#[test] -fn collect_explicit_skill_mentions_prioritizes_structured_inputs() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let beta = make_skill("beta-skill", "/tmp/beta"); - let skills = vec![alpha.clone(), beta.clone()]; - let inputs = vec![ - UserInput::Text { - text: "please run $alpha-skill".to_string(), - text_elements: Vec::new(), - }, - UserInput::Skill { - name: "beta-skill".to_string(), - path: test_path_buf("/tmp/beta"), - }, - ]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, vec![beta, alpha]); -} - -#[test] -fn collect_explicit_skill_mentions_skips_invalid_structured_and_blocks_plain_fallback() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let skills = vec![alpha]; - let inputs = vec![ - UserInput::Text { - text: "please run $alpha-skill".to_string(), - text_elements: Vec::new(), - }, - UserInput::Skill { - name: "alpha-skill".to_string(), - path: test_path_buf("/tmp/missing"), - }, - ]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_skips_disabled_structured_and_blocks_plain_fallback() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let skills = vec![alpha]; - let inputs = vec![ - UserInput::Text { - text: "please run $alpha-skill".to_string(), - text_elements: Vec::new(), - }, - UserInput::Skill { - name: "alpha-skill".to_string(), - path: test_path_buf("/tmp/alpha"), - }, - ]; - let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs()]); - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_dedupes_by_path() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let skills = vec![alpha.clone()]; - let mention = linked_skill_mention("alpha-skill", "/tmp/alpha"); - let inputs = vec![UserInput::Text { - text: format!("use {mention} and {mention}"), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, vec![alpha]); -} - -#[test] -fn collect_explicit_skill_mentions_skips_ambiguous_name() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let beta = make_skill("demo-skill", "/tmp/beta"); - let skills = vec![alpha, beta]; - let inputs = vec![UserInput::Text { - text: "use $demo-skill and again $demo-skill".to_string(), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_prefers_linked_path_over_name() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let beta = make_skill("demo-skill", "/tmp/beta"); - let skills = vec![alpha, beta.clone()]; - let inputs = vec![UserInput::Text { - text: format!( - "use $demo-skill and {}", - linked_skill_mention("demo-skill", "/tmp/beta") - ), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, vec![beta]); -} - -#[test] -fn collect_explicit_skill_mentions_skips_plain_name_when_connector_matches() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let skills = vec![alpha]; - let inputs = vec![UserInput::Text { - text: "use $alpha-skill".to_string(), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::from([("alpha-skill".to_string(), 1)]); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_allows_explicit_path_with_connector_conflict() { - let alpha = make_skill("alpha-skill", "/tmp/alpha"); - let skills = vec![alpha.clone()]; - let inputs = vec![UserInput::Text { - text: format!("use {}", linked_skill_mention("alpha-skill", "/tmp/alpha")), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::from([("alpha-skill".to_string(), 1)]); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, vec![alpha]); -} - -#[test] -fn collect_explicit_skill_mentions_skips_when_linked_path_disabled() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let beta = make_skill("demo-skill", "/tmp/beta"); - let skills = vec![alpha, beta]; - let inputs = vec![UserInput::Text { - text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/alpha")), - text_elements: Vec::new(), - }]; - let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs()]); - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_prefers_resource_path() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let beta = make_skill("demo-skill", "/tmp/beta"); - let skills = vec![alpha, beta.clone()]; - let inputs = vec![UserInput::Text { - text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/beta")), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, vec![beta]); -} - -#[test] -fn collect_explicit_skill_mentions_skips_missing_path_with_no_fallback() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let beta = make_skill("demo-skill", "/tmp/beta"); - let skills = vec![alpha, beta]; - let inputs = vec![UserInput::Text { - text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/missing")), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, Vec::new()); -} - -#[test] -fn collect_explicit_skill_mentions_skips_missing_path_without_fallback() { - let alpha = make_skill("demo-skill", "/tmp/alpha"); - let skills = vec![alpha]; - let inputs = vec![UserInput::Text { - text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/missing")), - text_elements: Vec::new(), - }]; - let connector_counts = HashMap::new(); - - let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - - assert_eq!(selected, Vec::new()); -} diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 53ffcddd0752..1734e2bef06d 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -1,4 +1,5 @@ pub mod config_rules; +mod executor_runtime; pub mod injection; pub(crate) mod invocation_utils; pub mod loader; @@ -7,10 +8,13 @@ pub mod model; pub mod remote; pub mod render; mod root_loader; +pub mod runtime; +mod runtime_selection; +mod runtime_snapshot; pub mod service; -mod skill_instructions; pub mod system; +pub use executor_runtime::ExecutorSkillCatalogCache; pub(crate) use invocation_utils::build_implicit_skill_path_indexes; pub use invocation_utils::detect_implicit_skill_invocation_for_command; pub use mention_counts::build_skill_name_counts; @@ -27,10 +31,16 @@ pub use render::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; pub use render::SKILLS_INTRO_WITH_ALIASES; pub use render::SkillMetadataBudget; pub use render::SkillRenderReport; +pub use render::SkillRenderSideEffects; pub use render::build_available_skills; +pub use render::build_available_skills_from_catalog; pub use render::default_skill_metadata_budget; pub use render::render_available_skills_body; pub use root_loader::PluginSkillSnapshots; +pub use runtime_selection::collect_runtime_skill_mentions; +pub use runtime_snapshot::RuntimeSkillInjection; +pub use runtime_snapshot::RuntimeSkillInjections; +pub use runtime_snapshot::SkillInjectionIdentity; +pub use runtime_snapshot::SkillsSnapshot; pub use service::SkillsLoadInput; pub use service::SkillsService; -pub use skill_instructions::SkillInstructions; diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index 6afbde6cc94b..a7f17eb87735 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -148,6 +148,10 @@ impl HostSkillsSnapshot { self.outcome.as_ref() } + pub(crate) fn outcome_arc(&self) -> Arc { + Arc::clone(&self.outcome) + } + pub async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result { let fs = self .outcome diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 89ea1675d0ee..5cbcae053e93 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -6,6 +6,9 @@ use std::path::Path; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; +use crate::runtime::SkillCatalog; +use crate::runtime::SkillCatalogEntry; +use crate::runtime::SkillSourceKind; use codex_otel::SessionTelemetry; use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; @@ -16,6 +19,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; +const MAX_RUNTIME_SKILL_METADATA_BYTES: usize = 8_000; const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; const MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS: usize = 1_024; const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; @@ -25,9 +29,9 @@ pub const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were s pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT: &str = "Skill descriptions were shortened to fit the 2% skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str = "Exceeded skills context budget. All skill descriptions were removed and"; -pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that can be used. Each entry includes a name, description, and source locator. `file` locators are on the host filesystem, `environment resource` locators are owned by an execution environment, `orchestrator resource` locators are opaque non-filesystem resources, and `custom resource` locators use their provider's access mechanism."; +pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that can be used. Each entry includes a name, description, and source locator. `file` locators are on the host filesystem, `environment resource` locators are owned by an execution environment, and `orchestrator resource` locators are opaque non-filesystem resources."; pub const SKILLS_INTRO_WITH_ALIASES: &str = "A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and a short path that can be expanded into an absolute path using the skill roots table."; -pub const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + source locator). `file` entries live on the host filesystem, `environment resource` entries are owned by their execution environment, `orchestrator resource` entries must be accessed through `skills.list` and `skills.read`, and `custom resource` entries use their provider's access mechanism. +pub const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + source locator). `file` entries live on the host filesystem, `environment resource` entries are owned by their execution environment, and `orchestrator resource` entries must be accessed through `skills.list` and `skills.read`. - Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. - Missing/blocked: If a named skill isn't in the list or its source can't be read, say so briefly and continue with the best fallback. - How to use a skill (progressive disclosure): @@ -202,6 +206,103 @@ pub fn build_available_skills( Some(selected) } +/// Renders one authority-aware runtime catalog with the same budget, report, +/// warnings, aliases, and metrics as host-only skills. +pub fn build_available_skills_from_catalog( + catalog: &SkillCatalog, + host_outcome: Option<&SkillLoadOutcome>, + budget: SkillMetadataBudget, + side_effects: SkillRenderSideEffects<'_>, +) -> Option { + let mut entries = catalog + .entries + .iter() + .filter(|entry| entry.enabled && entry.prompt_visible) + .collect::>(); + if entries.is_empty() { + record_skill_render_side_effects( + side_effects, + /*total_count*/ 0, + /*included_count*/ 0, + /*omitted_count*/ 0, + /*truncated_description_chars*/ 0, + ); + return None; + } + if let Some(outcome) = host_outcome { + let host_paths = + ordered_skills_for_budget(&outcome.allowed_skills_for_implicit_invocation()) + .into_iter() + .map(|skill| { + normalized_path(&skill.path_to_skills_md.to_string_lossy()).into_owned() + }) + .collect::>(); + entries.sort_by(|left, right| { + let host_position = |entry: &SkillCatalogEntry| { + (entry.authority.kind == SkillSourceKind::Host).then(|| { + let path = normalized_path(entry.main_prompt.as_str()); + host_paths + .iter() + .position(|host_path| host_path == path.as_ref()) + .unwrap_or(host_paths.len()) + }) + }; + match (host_position(left), host_position(right)) { + (Some(left), Some(right)) => left.cmp(&right), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => left + .authority + .kind + .to_string() + .cmp(&right.authority.kind.to_string()) + .then_with(|| left.authority.id.cmp(&right.authority.id)) + .then_with(|| left.id.0.cmp(&right.id.0)), + } + }); + } + + let absolute = build_available_skills_from_lines( + entries + .iter() + .map(|entry| SkillLine::from_catalog_entry(entry)) + .collect(), + entries.len(), + budget, + SkillPathAliases::default(), + ) + .and_then(|available| bound_runtime_skill_metadata_bytes(available, budget)); + let should_try_aliases = absolute.as_ref().is_none_or(|absolute| { + absolute.report.omitted_count > 0 || absolute.report.truncated_description_chars > 0 + }); + let aliased = if should_try_aliases + && entries + .iter() + .all(|entry| entry.authority.kind == SkillSourceKind::Host) + { + host_outcome + .and_then(|outcome| { + build_aliased_available_skills_from_catalog(outcome, &entries, budget) + }) + .and_then(|available| bound_runtime_skill_metadata_bytes(available, budget)) + } else { + None + }; + let selected = match (absolute, aliased) { + (Some(absolute), Some(aliased)) + if aliased_render_is_better(&aliased, &absolute, budget) => + { + aliased + } + (Some(absolute), _) => absolute, + (None, Some(aliased)) => aliased, + (None, None) => return None, + }; + + record_available_skills_side_effects(&selected, budget, side_effects); + Some(selected) +} + fn build_available_skills_from_lines( skill_lines: Vec>, total_count: usize, @@ -213,7 +314,58 @@ fn build_available_skills_from_lines( } let (skill_lines, report) = render_skill_lines_from_lines(skill_lines, total_count, budget); - let warning_message = if report.omitted_count > 0 { + let warning_message = skill_metadata_warning(&report, budget); + let available = AvailableSkills { + skill_root_lines: path_aliases.skill_root_lines, + skill_lines, + report, + warning_message, + }; + Some(available) +} + +fn bound_runtime_skill_metadata_bytes( + mut available: AvailableSkills, + budget: SkillMetadataBudget, +) -> Option { + let mut used_bytes = available + .skill_root_lines + .iter() + .map(|line| line.len().saturating_add(1)) + .sum::(); + if used_bytes > MAX_RUNTIME_SKILL_METADATA_BYTES { + return None; + } + + available.skill_lines.retain(|line| { + let line_bytes = line.len().saturating_add(1); + let Some(next_bytes) = used_bytes.checked_add(line_bytes) else { + return false; + }; + if next_bytes > MAX_RUNTIME_SKILL_METADATA_BYTES { + return false; + } + used_bytes = next_bytes; + true + }); + if available.skill_lines.is_empty() { + return None; + } + + available.report.included_count = available.skill_lines.len(); + available.report.omitted_count = available + .report + .total_count + .saturating_sub(available.report.included_count); + available.warning_message = skill_metadata_warning(&available.report, budget); + Some(available) +} + +fn skill_metadata_warning( + report: &SkillRenderReport, + budget: SkillMetadataBudget, +) -> Option { + if report.omitted_count > 0 { let skill_word = if report.omitted_count == 1 { "skill" } else { @@ -243,14 +395,7 @@ fn build_available_skills_from_lines( ) } else { None - }; - let available = AvailableSkills { - skill_root_lines: path_aliases.skill_root_lines, - skill_lines, - report, - warning_message, - }; - Some(available) + } } fn record_available_skills_side_effects( @@ -450,6 +595,7 @@ impl SkillRenderReport { struct SkillLine<'a> { name: &'a str, description: Cow<'a, str>, + locator: &'static str, path: String, } @@ -492,6 +638,29 @@ impl<'a> SkillLine<'a> { Self { name: skill.name.as_str(), description, + locator: "file", + path, + } + } + + fn from_catalog_entry(entry: &'a SkillCatalogEntry) -> Self { + Self::from_catalog_entry_with_path(entry, entry.rendered_path().to_string()) + } + + fn from_catalog_entry_with_path(entry: &'a SkillCatalogEntry, path: String) -> Self { + let locator = match &entry.authority.kind { + SkillSourceKind::Host => "file", + SkillSourceKind::Executor => "environment resource", + SkillSourceKind::Orchestrator => "orchestrator resource", + }; + let description = entry + .short_description + .as_deref() + .unwrap_or(entry.description.as_str()); + Self { + name: &entry.name, + description: truncate_default_context_skill_description(description), + locator, path, } } @@ -525,19 +694,25 @@ impl<'a> SkillLine<'a> { fn render_with_description_chars(&self, description_chars: usize) -> String { if description_chars == 0 { - format!("- {}: (file: {})", self.name, self.path) + format!("- {}: ({}: {})", self.name, self.locator, self.path) } else { let end = self.rendered_description_prefix_len(description_chars); let description = &self.description.as_ref()[..end]; - format!("- {}: {} (file: {})", self.name, description, self.path) + format!( + "- {}: {} ({}: {})", + self.name, description, self.locator, self.path + ) } } fn render_with_description(&self, description: &str) -> String { if description.is_empty() { - format!("- {}: (file: {})", self.name, self.path) + format!("- {}: ({}: {})", self.name, self.locator, self.path) } else { - format!("- {}: {} (file: {})", self.name, description, self.path) + format!( + "- {}: {} ({}: {})", + self.name, description, self.locator, self.path + ) } } } @@ -682,6 +857,57 @@ fn build_aliased_available_skills( build_available_skills_from_lines(skill_lines, skills.len(), adjusted_budget, plan.aliases) } +fn build_aliased_available_skills_from_catalog( + outcome: &SkillLoadOutcome, + entries: &[&SkillCatalogEntry], + budget: SkillMetadataBudget, +) -> Option { + let host_skills = outcome + .allowed_skills_for_implicit_invocation() + .into_iter() + .filter(|skill| { + entries.iter().any(|entry| { + entry.authority.kind == SkillSourceKind::Host + && normalized_path(entry.main_prompt.as_str()) + == normalized_path(&skill.path_to_skills_md.to_string_lossy()) + }) + }) + .collect::>(); + let plan = build_alias_plan(outcome, &host_skills, budget)?; + if plan.table_cost >= budget.limit() { + return None; + } + let adjusted_limit = budget.limit().saturating_sub(plan.table_cost); + let adjusted_budget = match budget { + SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit), + SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit), + }; + let skill_lines = entries + .iter() + .map(|entry| { + let path = host_skills + .iter() + .find(|skill| { + entry.authority.kind == SkillSourceKind::Host + && normalized_path(entry.main_prompt.as_str()) + == normalized_path(&skill.path_to_skills_md.to_string_lossy()) + }) + .map(|skill| render_skill_path_with_aliases(skill, &plan)) + .unwrap_or_else(|| entry.rendered_path().to_string()); + SkillLine::from_catalog_entry_with_path(entry, path) + }) + .collect(); + build_available_skills_from_lines(skill_lines, entries.len(), adjusted_budget, plan.aliases) +} + +fn normalized_path(path: &str) -> Cow<'_, str> { + if path.contains('\\') { + Cow::Owned(path.replace('\\', "/")) + } else { + Cow::Borrowed(path) + } +} + #[derive(Debug, Clone, Default, PartialEq, Eq)] struct SkillPathAliases { skill_root_lines: Vec, @@ -932,6 +1158,9 @@ mod tests { use std::collections::HashMap; use std::sync::Arc; + use crate::runtime::SkillAuthority; + use crate::runtime::SkillPackageId; + use crate::runtime::SkillResourceId; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; @@ -960,6 +1189,46 @@ mod tests { skill } + #[test] + fn catalog_rendering_keeps_a_hard_byte_cap_for_multibyte_metadata() { + let description = "💡".repeat(MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS + 1); + let entries = ["alpha", "beta"] + .into_iter() + .map(|name| { + SkillCatalogEntry::new( + SkillPackageId(name.to_string()), + SkillAuthority::new(SkillSourceKind::Executor, "executor"), + name, + description.clone(), + SkillResourceId::new(format!("{name}/SKILL.md")), + ) + }) + .collect(); + let catalog = SkillCatalog { + entries, + warnings: Vec::new(), + }; + + let rendered = build_available_skills_from_catalog( + &catalog, + /*host_outcome*/ None, + SkillMetadataBudget::Characters(usize::MAX), + SkillRenderSideEffects::None, + ) + .expect("one catalog entry should fit"); + let metadata_bytes = rendered + .skill_root_lines + .iter() + .chain(&rendered.skill_lines) + .map(|line| line.len().saturating_add(1)) + .sum::(); + + assert!(metadata_bytes <= MAX_RUNTIME_SKILL_METADATA_BYTES); + assert_eq!(rendered.report.included_count, 1); + assert_eq!(rendered.report.omitted_count, 1); + assert!(rendered.warning_message.is_some()); + } + fn expected_skill_line(skill: &SkillMetadata, description: &str) -> String { SkillLine::new(skill).render_with_description(description) } diff --git a/codex-rs/ext/skills/src/catalog.rs b/codex-rs/core-skills/src/runtime.rs similarity index 50% rename from codex-rs/ext/skills/src/catalog.rs rename to codex-rs/core-skills/src/runtime.rs index 59977a47cac2..c5925d72072d 100644 --- a/codex-rs/ext/skills/src/catalog.rs +++ b/codex-rs/core-skills/src/runtime.rs @@ -1,6 +1,16 @@ -use codex_core_skills::model::SkillDependencies; +use std::any::Any; +use std::collections::HashMap; +use std::fmt; +use std::future::Future; +use std::hash::Hash; +use std::hash::Hasher; +use std::pin::Pin; +use std::sync::Arc; + use codex_utils_path_uri::PathUri; +use crate::model::SkillDependencies; + /// Source authority that owns a skill package and must be used to read it. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum SkillSourceKind { @@ -11,28 +21,20 @@ pub enum SkillSourceKind { Executor, /// Skills owned by the orchestrator rather than an execution environment. Orchestrator, - /// Extension-private source kind for future providers that do not fit an - /// existing transport category. - Custom(String), } impl SkillSourceKind { - pub fn custom(kind: impl Into) -> Self { - Self::Custom(kind.into()) - } - fn as_str(&self) -> &str { match self { Self::Host => "host", Self::Executor => "executor", Self::Orchestrator => "orchestrator", - Self::Custom(kind) => kind, } } } -impl std::fmt::Display for SkillSourceKind { - fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SkillSourceKind { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { self.as_str().fmt(formatter) } } @@ -91,7 +93,7 @@ impl SkillResourceId { &self.id } - pub(crate) fn environment_path(&self) -> Option<(&str, &PathUri)> { + pub fn environment_path(&self) -> Option<(&str, &PathUri)> { self.environment_path .as_ref() .map(|resource| (resource.environment_id.as_str(), &resource.path)) @@ -166,14 +168,14 @@ impl SkillCatalogEntry { self } - pub(crate) fn rendered_path(&self) -> &str { + pub fn rendered_path(&self) -> &str { self.display_path .as_deref() .unwrap_or_else(|| self.main_prompt.as_str()) } } -/// Merged catalog for one turn. +/// Merged catalog for one model step. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct SkillCatalog { pub entries: Vec, @@ -181,13 +183,6 @@ pub struct SkillCatalog { } impl SkillCatalog { - pub fn extend(&mut self, other: SkillCatalog) { - for entry in other.entries { - self.push_entry(entry); - } - self.warnings.extend(other.warnings); - } - pub fn push_entry(&mut self, entry: SkillCatalogEntry) { if self .entries @@ -201,33 +196,27 @@ impl SkillCatalog { } } -/// Contents returned after resolving a skill resource through its owner. +/// A request to read one resource from the source that listed it. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct SkillReadResult { +pub struct SkillReadRequest { + pub authority: SkillAuthority, + pub package: SkillPackageId, pub resource: SkillResourceId, - pub contents: String, -} - -/// Search results for a package whose files are not readable through ordinary -/// executor filesystem access. -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct SkillSearchResult { - pub matches: Vec, } +/// Contents returned after resolving a skill resource through its owner. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct SkillSearchMatch { +pub struct SkillReadResult { pub resource: SkillResourceId, - pub title: String, - pub snippet: String, + pub contents: String, } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct SkillProviderError { +pub struct SkillSourceError { pub message: String, } -impl SkillProviderError { +impl SkillSourceError { pub fn new(message: impl Into) -> Self { Self { message: message.into(), @@ -235,12 +224,147 @@ impl SkillProviderError { } } -impl std::fmt::Display for SkillProviderError { - fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for SkillSourceError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { self.message.fmt(formatter) } } -impl std::error::Error for SkillProviderError {} +impl std::error::Error for SkillSourceError {} + +pub type SkillSourceResult = Result; + +/// Boxed future used to keep heterogeneous runtime skill sources object-safe. +pub type SkillSourceFuture<'a, T> = Pin> + Send + 'a>>; + +/// Opaque identity of the runtime owner captured by a skill source. +#[derive(Clone)] +pub struct SkillSourceIdentity(Arc); + +impl SkillSourceIdentity { + pub fn from_owner(owner: Arc) -> Self + where + T: Any + Send + Sync, + { + Self(owner) + } +} + +impl fmt::Debug for SkillSourceIdentity { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.debug_tuple("SkillSourceIdentity").finish() + } +} + +impl PartialEq for SkillSourceIdentity { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) + } +} + +impl Eq for SkillSourceIdentity {} + +impl Hash for SkillSourceIdentity { + fn hash(&self, state: &mut H) { + (Arc::as_ptr(&self.0) as *const ()).hash(state); + } +} + +/// One bound source of runtime skills. +/// +/// Implementations capture the authority needed to list and read their skills, +/// such as a host snapshot, one executor filesystem, or an orchestrator client. +/// A resource returned by [`SkillSource::list`] must be read through the same +/// source instead of being converted into an ambient local path. +pub trait SkillSource: Send + Sync { + /// Returns the stable identity of the runtime owner captured by this source. + fn identity(&self) -> SkillSourceIdentity; + + /// Lists the skills available from the authority captured by this source. + fn list(&self) -> SkillSourceFuture<'_, SkillCatalog>; + + /// Reads a resource previously listed by this source. + fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult>; +} + +#[derive(Clone)] +struct RegisteredSkillSource { + label: String, + source: Arc Arc + Send + Sync>, +} + +/// Bound skill sources used to build and read one runtime catalog. +#[derive(Clone, Default)] +pub struct SkillSources { + sources: Vec, +} + +impl SkillSources { + pub fn new() -> Self { + Self::default() + } + + pub fn with_source(mut self, label: impl Into, source: Arc) -> Self { + self.sources.push(RegisteredSkillSource { + label: label.into(), + source: Arc::new(move || Arc::clone(&source)), + }); + self + } + + pub fn with_source_factory( + mut self, + label: impl Into, + source: Arc Arc + Send + Sync>, + ) -> Self { + self.sources.push(RegisteredSkillSource { + label: label.into(), + source, + }); + self + } + + pub fn extend(&mut self, other: Self) { + self.sources.extend(other.sources); + } -pub type SkillProviderResult = Result; + pub(crate) async fn list_with_sources( + &self, + ) -> ( + SkillCatalog, + HashMap<(SkillAuthority, SkillPackageId), Arc>, + ) { + let mut catalog = SkillCatalog::default(); + let mut sources = HashMap::new(); + for source in &self.sources { + let bound_source = (source.source)(); + match bound_source.list().await { + Ok(source_catalog) => { + catalog.warnings.extend(source_catalog.warnings); + for entry in source_catalog.entries { + let key = (entry.authority.clone(), entry.id.clone()); + if let std::collections::hash_map::Entry::Vacant(route) = sources.entry(key) + { + route.insert(Arc::clone(&bound_source)); + catalog.entries.push(entry); + } + } + } + Err(err) => catalog.warnings.push(format!( + "{} skills unavailable: {}", + source.label, err.message + )), + } + } + (catalog, sources) + } +} + +impl fmt::Debug for SkillSources { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_list() + .entries(self.sources.iter().map(|source| &source.label)) + .finish() + } +} diff --git a/codex-rs/core-skills/src/runtime_selection.rs b/codex-rs/core-skills/src/runtime_selection.rs new file mode 100644 index 000000000000..753c02c9091a --- /dev/null +++ b/codex-rs/core-skills/src/runtime_selection.rs @@ -0,0 +1,181 @@ +use std::collections::HashSet; + +use codex_protocol::user_input::UserInput; + +use crate::injection::extract_tool_mentions; +use crate::runtime::SkillAuthority; +use crate::runtime::SkillCatalog; +use crate::runtime::SkillCatalogEntry; +use crate::runtime::SkillPackageId; +use crate::runtime::SkillSourceKind; + +const SKILL_PATH_PREFIX: &str = "skill://"; + +/// Selects explicit skill mentions from one authority-aware runtime catalog. +/// +/// Exact locators win. Plain names prefer executor, then orchestrator, then +/// host skills, and must be unique within the first matching authority kind. +/// Plain names that collide with another tool namespace are ignored. +/// A structured selection blocks a plain-name fallback even when its locator +/// no longer exists. +pub fn collect_runtime_skill_mentions( + inputs: &[UserInput], + catalog: &SkillCatalog, + plain_name_conflicts: &HashSet, +) -> Vec { + let mut selected = Vec::new(); + let mut seen_names = HashSet::new(); + let mut blocked_plain_names = HashSet::new(); + + for input in inputs { + match input { + UserInput::Skill { name, path } => { + blocked_plain_names.insert(name.clone()); + select_by_path( + catalog, + &path.to_string_lossy(), + &mut seen_names, + &mut selected, + ); + } + UserInput::Mention { name, path } if path_is_skill(path) => { + blocked_plain_names.insert(name.clone()); + select_by_path(catalog, path, &mut seen_names, &mut selected); + } + _ => {} + } + } + + for input in inputs { + let UserInput::Text { text, .. } = input else { + continue; + }; + let mentions = extract_tool_mentions(text); + select_text_mentions( + catalog, + &mentions, + &blocked_plain_names, + plain_name_conflicts, + &mut seen_names, + &mut selected, + ); + } + + selected +} + +fn select_text_mentions( + catalog: &SkillCatalog, + mentions: &crate::injection::ToolMentions<'_>, + blocked_plain_names: &HashSet, + plain_name_conflicts: &HashSet, + seen_names: &mut HashSet, + selected: &mut Vec, +) { + let mentioned_paths = mentions + .paths() + .filter(|path| path_is_skill(path)) + .map(normalize_skill_path) + .collect::>(); + for entry in catalog.entries.iter().filter(|entry| entry.enabled) { + if entry_paths(entry) + .into_iter() + .any(|path| mentioned_paths.contains(normalize_skill_path(path))) + { + push_selected(entry, seen_names, selected); + } + } + + let selected_names = mentions + .plain_names() + .filter(|name| !blocked_plain_names.contains(*name)) + .filter(|name| !plain_name_conflicts.contains(&name.to_ascii_lowercase())) + .filter_map(|name| select_by_name(catalog, name)) + .map(SkillCatalogEntryKey::from) + .collect::>(); + for entry in &catalog.entries { + if selected_names.contains(&SkillCatalogEntryKey::from(entry)) { + push_selected(entry, seen_names, selected); + } + } +} + +fn select_by_path( + catalog: &SkillCatalog, + path: &str, + seen_names: &mut HashSet, + selected: &mut Vec, +) { + let path = normalize_skill_path(path); + for entry in catalog.entries.iter().filter(|entry| entry.enabled) { + if entry_paths(entry) + .into_iter() + .any(|candidate| normalize_skill_path(candidate) == path) + { + push_selected(entry, seen_names, selected); + } + } +} + +fn entry_paths(entry: &SkillCatalogEntry) -> [&str; 3] { + [ + entry.main_prompt.as_str(), + entry.id.0.as_str(), + entry.rendered_path(), + ] +} + +fn select_by_name<'a>(catalog: &'a SkillCatalog, name: &str) -> Option<&'a SkillCatalogEntry> { + for kind in [ + SkillSourceKind::Executor, + SkillSourceKind::Orchestrator, + SkillSourceKind::Host, + ] { + let mut matches = catalog + .entries + .iter() + .filter(|entry| entry.enabled && entry.authority.kind == kind && entry.name == name); + let first = matches.next(); + if first.is_some() { + return first.filter(|_| matches.next().is_none()); + } + } + None +} + +fn push_selected( + entry: &SkillCatalogEntry, + seen_names: &mut HashSet, + selected: &mut Vec, +) { + if seen_names.insert(entry.name.clone()) { + selected.push(entry.clone()); + } +} + +fn path_is_skill(path: &str) -> bool { + path.starts_with(SKILL_PATH_PREFIX) + || path + .rsplit(['/', '\\']) + .next() + .is_some_and(|file_name| file_name.eq_ignore_ascii_case("SKILL.md")) +} + +fn normalize_skill_path(path: &str) -> &str { + path.strip_prefix(SKILL_PATH_PREFIX).unwrap_or(path) +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +struct SkillCatalogEntryKey { + authority: SkillAuthority, + package: SkillPackageId, +} + +impl From<&SkillCatalogEntry> for SkillCatalogEntryKey { + fn from(entry: &SkillCatalogEntry) -> Self { + Self { + authority: entry.authority.clone(), + package: entry.id.clone(), + } + } +} diff --git a/codex-rs/core-skills/src/runtime_snapshot.rs b/codex-rs/core-skills/src/runtime_snapshot.rs new file mode 100644 index 000000000000..ebeebc4e1f53 --- /dev/null +++ b/codex-rs/core-skills/src/runtime_snapshot.rs @@ -0,0 +1,371 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::fmt; +use std::sync::Arc; + +use codex_exec_server::ResolvedSelectedCapabilityRoot; +use codex_protocol::protocol::Product; +use codex_protocol::user_input::UserInput; + +use crate::AvailableSkills; +use crate::ExecutorSkillCatalogCache; +use crate::HostSkillsSnapshot; +use crate::SkillMetadata; +use crate::collect_runtime_skill_mentions; +use crate::default_skill_metadata_budget; +use crate::executor_runtime::ExecutorSkillSource; +use crate::render::SkillRenderSideEffects; +use crate::render::build_available_skills_from_catalog; +use crate::runtime::SkillAuthority; +use crate::runtime::SkillCatalog; +use crate::runtime::SkillCatalogEntry; +use crate::runtime::SkillPackageId; +use crate::runtime::SkillReadRequest; +use crate::runtime::SkillReadResult; +use crate::runtime::SkillResourceId; +use crate::runtime::SkillSource; +use crate::runtime::SkillSourceError; +use crate::runtime::SkillSourceFuture; +use crate::runtime::SkillSourceIdentity; +use crate::runtime::SkillSourceKind; +use crate::runtime::SkillSources; + +const HOST_AUTHORITY_ID: &str = "host"; + +type EntryKey = (SkillAuthority, SkillPackageId); + +/// Identity of one injected package and the runtime owner that supplied it. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct SkillInjectionIdentity { + source: SkillSourceIdentity, + authority: SkillAuthority, + package: SkillPackageId, +} + +#[derive(Debug)] +pub struct RuntimeSkillInjection { + pub identity: SkillInjectionIdentity, + pub entry: SkillCatalogEntry, + pub contents: String, +} + +#[derive(Debug, Default)] +pub struct RuntimeSkillInjections { + pub items: Vec, + pub warnings: Vec, +} + +/// One immutable, authority-aware skill view used by a model sampling step. +#[derive(Clone)] +pub struct SkillsSnapshot { + host: HostSkillsSnapshot, + catalog: Arc, + sources: Arc>>, + available: Arc>, + warnings: Arc>, +} + +impl SkillsSnapshot { + pub fn from_host(host: HostSkillsSnapshot, context_window: Option) -> Self { + let source: Arc = Arc::new(HostSkillSource::new(host.clone())); + let catalog = host_catalog(&host); + let source_by_entry = catalog + .entries + .iter() + .map(|entry| { + ( + (entry.authority.clone(), entry.id.clone()), + Arc::clone(&source), + ) + }) + .collect(); + Self::new(host, catalog, source_by_entry, context_window) + } + + pub async fn load( + host: HostSkillsSnapshot, + executor_catalog_cache: &ExecutorSkillCatalogCache, + executor_roots: &[ResolvedSelectedCapabilityRoot], + extra_sources: Option<&SkillSources>, + restriction_product: Option, + context_window: Option, + ) -> Self { + let host_source: Arc = Arc::new(HostSkillSource::new(host.clone())); + let mut catalog = SkillCatalog::default(); + let mut source_by_entry = HashMap::new(); + merge_bound_catalog( + &mut catalog, + &mut source_by_entry, + &host_catalog(&host), + &host_source, + ); + for root in executor_roots { + let cached = executor_catalog_cache + .catalog_for_stable_root(root, restriction_product) + .await; + let executor_source = Arc::new(ExecutorSkillSource::new( + root.clone(), + restriction_product, + cached.identity(), + )); + let source: Arc = executor_source; + merge_bound_catalog( + &mut catalog, + &mut source_by_entry, + cached.catalog(), + &source, + ); + } + if let Some(extra_sources) = extra_sources { + let (extra_catalog, extra_source_by_entry) = extra_sources.list_with_sources().await; + catalog.warnings.extend(extra_catalog.warnings); + for entry in extra_catalog.entries { + let key = (entry.authority.clone(), entry.id.clone()); + if let std::collections::hash_map::Entry::Vacant(route) = + source_by_entry.entry(key.clone()) + && let Some(source) = extra_source_by_entry.get(&key) + { + route.insert(Arc::clone(source)); + catalog.entries.push(entry); + } + } + } + + Self::new(host, catalog, source_by_entry, context_window) + } + + fn new( + host: HostSkillsSnapshot, + catalog: SkillCatalog, + source_by_entry: HashMap>, + context_window: Option, + ) -> Self { + let available = build_available_skills_from_catalog( + &catalog, + Some(host.outcome()), + default_skill_metadata_budget(context_window), + SkillRenderSideEffects::None, + ); + let mut warnings = catalog.warnings.clone(); + if let Some(warning) = available + .as_ref() + .and_then(|available| available.warning_message.clone()) + { + warnings.push(warning); + } + Self { + host, + catalog: Arc::new(catalog), + sources: Arc::new(source_by_entry), + available: Arc::new(available), + warnings: Arc::new(warnings), + } + } + + pub fn available(&self) -> Option<&AvailableSkills> { + self.available.as_ref().as_ref() + } + + pub fn warnings(&self) -> &[String] { + self.warnings.as_ref() + } + + pub fn skill_name_counts_lower(&self) -> HashMap { + let mut counts = HashMap::new(); + for entry in self.catalog.entries.iter().filter(|entry| entry.enabled) { + *counts.entry(entry.name.to_ascii_lowercase()).or_default() += 1; + } + counts + } + + pub fn host_skill(&self, entry: &SkillCatalogEntry) -> Option<&SkillMetadata> { + if entry.authority.kind != SkillSourceKind::Host { + return None; + } + self.host + .outcome() + .skills + .iter() + .find(|skill| skill.path_to_skills_md.to_string_lossy() == entry.main_prompt.as_str()) + } + + pub async fn injections( + &self, + input: &[UserInput], + plain_name_conflicts: &HashSet, + active_skills: &HashMap, + remaining_items: usize, + ) -> RuntimeSkillInjections { + let selected = collect_runtime_skill_mentions(input, &self.catalog, plain_name_conflicts); + let mut result = RuntimeSkillInjections::default(); + for entry in &selected { + let key = (entry.authority.clone(), entry.id.clone()); + let Some(source) = self.sources.get(&key) else { + result.warnings.push(format!( + "Failed to load skill `{}`: its runtime source is unavailable.", + entry.name + )); + continue; + }; + let identity = SkillInjectionIdentity { + source: source.identity(), + authority: entry.authority.clone(), + package: entry.id.clone(), + }; + if active_skills.get(&entry.name) == Some(&identity) { + continue; + } + if result.items.len() == remaining_items { + result.warnings.push(format!( + "Only the first {remaining_items} newly selected skills were loaded because this turn reached its skill instruction limit." + )); + break; + } + let request = SkillReadRequest { + authority: entry.authority.clone(), + package: entry.id.clone(), + resource: entry.main_prompt.clone(), + }; + match source.read(request).await { + Ok(read) => result.items.push(RuntimeSkillInjection { + identity, + entry: entry.clone(), + contents: read.contents, + }), + Err(err) => result + .warnings + .push(format!("Failed to load skill `{}`: {err}", entry.name)), + } + } + result + } + + pub fn contains_injection(&self, identity: &SkillInjectionIdentity) -> bool { + let key = (identity.authority.clone(), identity.package.clone()); + self.catalog + .entries + .iter() + .any(|entry| entry.enabled && entry.authority == key.0 && entry.id == key.1) + && self + .sources + .get(&key) + .is_some_and(|source| source.identity() == identity.source) + } +} + +fn merge_bound_catalog( + catalog: &mut SkillCatalog, + source_by_entry: &mut HashMap>, + source_catalog: &SkillCatalog, + source: &Arc, +) { + catalog + .warnings + .extend(source_catalog.warnings.iter().cloned()); + for entry in &source_catalog.entries { + let key = (entry.authority.clone(), entry.id.clone()); + if let std::collections::hash_map::Entry::Vacant(route) = source_by_entry.entry(key) { + route.insert(Arc::clone(source)); + catalog.entries.push(entry.clone()); + } + } +} + +impl fmt::Debug for SkillsSnapshot { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("SkillsSnapshot") + .field("catalog", &self.catalog) + .field("available", &self.available) + .finish_non_exhaustive() + } +} + +struct HostSkillSource { + snapshot: HostSkillsSnapshot, + identity: SkillSourceIdentity, +} + +impl HostSkillSource { + fn new(snapshot: HostSkillsSnapshot) -> Self { + Self { + identity: SkillSourceIdentity::from_owner(snapshot.outcome_arc()), + snapshot, + } + } +} + +impl SkillSource for HostSkillSource { + fn identity(&self) -> SkillSourceIdentity { + self.identity.clone() + } + + fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> { + Box::pin(async move { Ok(host_catalog(&self.snapshot)) }) + } + + fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> { + Box::pin(async move { + if request.authority != SkillAuthority::new(SkillSourceKind::Host, HOST_AUTHORITY_ID) { + return Err(SkillSourceError::new("host skill authority does not match")); + } + let Some(skill) = self.snapshot.outcome().skills.iter().find(|skill| { + skill.path_to_skills_md.to_string_lossy() == request.resource.as_str() + }) else { + return Err(SkillSourceError::new(format!( + "host skill resource is not loaded: {}", + request.resource.as_str() + ))); + }; + let contents = self.snapshot.read_skill_text(skill).await.map_err(|err| { + SkillSourceError::new(format!( + "failed to read host skill resource {}: {err}", + request.resource.as_str() + )) + })?; + Ok(SkillReadResult { + resource: request.resource, + contents, + }) + }) + } +} + +fn host_catalog(snapshot: &HostSkillsSnapshot) -> SkillCatalog { + let outcome = snapshot.outcome(); + let mut catalog = SkillCatalog { + warnings: outcome + .errors + .iter() + .map(|error| { + format!( + "Failed to load skill at {}: {}", + error.path.display(), + error.message + ) + }) + .collect(), + ..Default::default() + }; + for (skill, enabled) in outcome.skills_with_enabled() { + let path = skill.path_to_skills_md.to_string_lossy().into_owned(); + let mut entry = SkillCatalogEntry::new( + SkillPackageId(path.clone()), + SkillAuthority::new(SkillSourceKind::Host, HOST_AUTHORITY_ID), + skill.name.clone(), + skill.description.clone(), + SkillResourceId::new(path.clone()), + ) + .with_short_description(skill.short_description.clone()) + .with_display_path(path.replace('\\', "/")) + .with_dependencies(skill.dependencies.clone()); + if !enabled { + entry = entry.disabled(); + } + if !skill.allows_implicit_invocation() { + entry = entry.hidden_from_prompt(); + } + catalog.push_entry(entry); + } + catalog +} diff --git a/codex-rs/core-skills/src/skill_instructions.rs b/codex-rs/core-skills/src/skill_instructions.rs deleted file mode 100644 index b2a002d9025b..000000000000 --- a/codex-rs/core-skills/src/skill_instructions.rs +++ /dev/null @@ -1,41 +0,0 @@ -use codex_context_fragments::ContextualUserFragment; - -use crate::injection::SkillInjection; - -#[derive(Debug, Clone, PartialEq)] -pub struct SkillInstructions { - name: String, - path: String, - contents: String, -} - -impl From<&SkillInjection> for SkillInstructions { - fn from(skill: &SkillInjection) -> Self { - Self { - name: skill.name.clone(), - path: skill.path.clone(), - contents: skill.contents.clone(), - } - } -} - -impl ContextualUserFragment for SkillInstructions { - fn role(&self) -> &'static str { - "user" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - ("", "") - } - - fn body(&self) -> String { - format!( - "\n{}\n{}\n{}\n", - self.name, self.path, self.contents - ) - } -} diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index 6919d8c6274d..c4c40577ca4f 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -1,5 +1,6 @@ use super::residency::is_v2_resident_session_source; use super::*; +use codex_extension_api::ExtensionDataInit; const AGENT_NAMES: &str = include_str!("../agent_names.txt"); @@ -433,6 +434,16 @@ impl AgentControl { )) })?; + let selected_capability_roots = parent_history + .items + .iter() + .find_map(|item| { + let RolloutItem::SessionMeta(meta_line) = item else { + return None; + }; + Some(meta_line.meta.selected_capability_roots.clone()) + }) + .unwrap_or_default(); let mut forked_rollout_items = parent_history.items; if let SpawnAgentForkMode::LastNTurns(last_n_turns) = fork_mode { forked_rollout_items = @@ -504,6 +515,8 @@ impl AgentControl { { forked_rollout_items.push(RolloutItem::ResponseItem(subagent_usage_hint_message)); } + let mut thread_extension_init = ExtensionDataInit::new(); + thread_extension_init.insert(selected_capability_roots); state .fork_thread_with_source( @@ -517,6 +530,7 @@ impl AgentControl { inherited_environments, inherited_exec_policy, options.environments.clone(), + thread_extension_init, ) .await } diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index e00a8624f459..de8754ad920f 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -17,6 +17,8 @@ use codex_features::Feature; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_protocol::AgentPath; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::ModeKind; use codex_protocol::models::ContentItem; use codex_protocol::models::MessagePhase; @@ -38,6 +40,7 @@ use codex_thread_store::InMemoryThreadStore; use codex_thread_store::LocalThreadStore; use codex_thread_store::LocalThreadStoreConfig; use codex_thread_store::ThreadStore; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::time::Duration; @@ -1446,7 +1449,33 @@ async fn spawn_agent_fork_last_n_turns_keeps_only_recent_turns() { #[tokio::test] async fn spawn_agent_fork_last_n_turns_drops_parent_startup_prefix_when_under_limit() { let harness = AgentControlHarness::new().await; - let (parent_thread_id, parent_thread) = harness.start_thread().await; + let selected_capability_roots = vec![SelectedCapabilityRoot { + id: "demo@1".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "build".to_string(), + path: PathUri::parse("file:///plugins/demo").expect("plugin root URI"), + }, + }]; + let mut thread_extension_init = ExtensionDataInit::new(); + thread_extension_init.insert(selected_capability_roots.clone()); + let parent = harness + .manager + .start_thread_with_options(StartThreadOptions { + config: harness.config.clone(), + initial_history: InitialHistory::New, + session_source: None, + thread_source: None, + dynamic_tools: Vec::new(), + metrics_service_name: None, + parent_trace: None, + environments: Vec::new(), + thread_extension_init, + supports_openai_form_elicitation: false, + }) + .await + .expect("start parent thread"); + let parent_thread_id = parent.thread_id; + let parent_thread = parent.thread; let startup_turn_context = parent_thread.codex.session.new_default_turn().await; parent_thread .codex @@ -1525,6 +1554,14 @@ async fn spawn_agent_fork_last_n_turns_drops_parent_startup_prefix_when_under_li !history_contains_text(history.raw_items(), "parent startup developer context"), "bounded fork should drop parent startup context even when fewer turns exist than requested" ); + assert_eq!( + &child_thread + .codex + .session + .services + .selected_capability_roots, + &selected_capability_roots + ); assert!( child_thread .codex diff --git a/codex-rs/core/src/context/available_skills_instructions.rs b/codex-rs/core/src/context/available_skills_instructions.rs deleted file mode 100644 index 4e166d436cfc..000000000000 --- a/codex-rs/core/src/context/available_skills_instructions.rs +++ /dev/null @@ -1,50 +0,0 @@ -use codex_core_skills::AvailableSkills; -use codex_core_skills::render_available_skills_body; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; - -use super::ContextualUserFragment; - -/// Model-context fragment describing the skills available to Codex. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct AvailableSkillsInstructions { - skill_root_lines: Vec, - skill_lines: Vec, -} - -impl AvailableSkillsInstructions { - /// Creates a skills context fragment from pre-rendered catalog lines. - pub fn from_skill_lines(skill_lines: Vec) -> Self { - Self { - skill_root_lines: Vec::new(), - skill_lines, - } - } -} - -impl From for AvailableSkillsInstructions { - fn from(available_skills: AvailableSkills) -> Self { - Self { - skill_root_lines: available_skills.skill_root_lines, - skill_lines: available_skills.skill_lines, - } - } -} - -impl ContextualUserFragment for AvailableSkillsInstructions { - fn role(&self) -> &'static str { - "developer" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG) - } - - fn body(&self) -> String { - render_available_skills_body(&self.skill_root_lines, &self.skill_lines) - } -} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index da7db0b37389..1a11c6dfe68f 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -3,7 +3,6 @@ mod approved_command_prefix_saved; mod apps_instructions; mod available_plugins_instructions; -mod available_skills_instructions; mod collaboration_mode_instructions; mod contextual_user_message; mod current_time_reminder; @@ -27,6 +26,7 @@ mod realtime_start_instructions; mod realtime_start_with_instructions; mod recommended_plugins_instructions; mod rollout_budget; +mod skill_instructions; mod subagent_notification; mod token_budget_context; mod turn_aborted; @@ -37,13 +37,11 @@ pub(crate) mod world_state; pub(crate) use approved_command_prefix_saved::ApprovedCommandPrefixSaved; pub(crate) use apps_instructions::AppsInstructions; pub(crate) use available_plugins_instructions::AvailablePluginsInstructions; -pub use available_skills_instructions::AvailableSkillsInstructions; pub(crate) use codex_context_fragments::AdditionalContextDeveloperFragment; pub(crate) use codex_context_fragments::AdditionalContextUserFragment; pub use codex_context_fragments::ContextualUserFragment; pub(crate) use codex_context_fragments::FragmentRegistration; pub(crate) use codex_context_fragments::FragmentRegistrationProxy; -pub(crate) use codex_core_skills::SkillInstructions; pub(crate) use collaboration_mode_instructions::CollaborationModeInstructions; pub(crate) use contextual_user_message::is_contextual_user_fragment; pub(crate) use contextual_user_message::parse_visible_hook_prompt_message; @@ -70,6 +68,7 @@ pub(crate) use realtime_start_instructions::RealtimeStartInstructions; pub(crate) use realtime_start_with_instructions::RealtimeStartWithInstructions; pub(crate) use recommended_plugins_instructions::RecommendedPluginsInstructions; pub(crate) use rollout_budget::RolloutBudgetContext; +pub(crate) use skill_instructions::SkillInstructions; pub(crate) use subagent_notification::SubagentNotification; pub(crate) use token_budget_context::ContextWindowGuidance; pub(crate) use token_budget_context::TokenBudgetContext; diff --git a/codex-rs/core/src/context/skill_instructions.rs b/codex-rs/core/src/context/skill_instructions.rs new file mode 100644 index 000000000000..a884a79b057f --- /dev/null +++ b/codex-rs/core/src/context/skill_instructions.rs @@ -0,0 +1,99 @@ +use codex_core_skills::runtime::SkillCatalogEntry; + +use super::ContextualUserFragment; + +const MAX_SKILL_NAME_BYTES: usize = 256; +const MAX_SKILL_PATH_BYTES: usize = 1_024; +const MAX_SKILL_INSTRUCTIONS_BYTES: usize = 4_000; +const REPLACEMENT_NOTICE: &str = "These instructions replace the previously provided instructions for this skill.\n"; +const UNAVAILABLE_NOTICE: &str = "The previously provided instructions for this skill no longer apply because the skill is unavailable in the current runtime.\n"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SkillInstructions { + name: String, + path: Option, + contents: String, + update: SkillInstructionsUpdate, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SkillInstructionsUpdate { + Initial, + Replacement, + Unavailable, +} + +impl SkillInstructions { + pub(crate) fn from_runtime( + entry: &SkillCatalogEntry, + contents: &str, + max_total_bytes: usize, + replaces_previous: bool, + ) -> Option<(Self, bool)> { + let mut instructions = Self { + name: truncate_utf8(&entry.name, MAX_SKILL_NAME_BYTES).0, + path: Some(truncate_utf8(entry.rendered_path(), MAX_SKILL_PATH_BYTES).0), + contents: String::new(), + update: if replaces_previous { + SkillInstructionsUpdate::Replacement + } else { + SkillInstructionsUpdate::Initial + }, + }; + let envelope_bytes = instructions.render().len(); + let max_body_bytes = max_total_bytes + .min(MAX_SKILL_INSTRUCTIONS_BYTES) + .checked_sub(envelope_bytes)?; + let (contents, truncated) = truncate_utf8(contents, max_body_bytes); + instructions.contents = contents; + Some((instructions, truncated)) + } + + pub(crate) fn unavailable(name: &str) -> Self { + Self { + name: truncate_utf8(name, MAX_SKILL_NAME_BYTES).0, + path: None, + contents: String::new(), + update: SkillInstructionsUpdate::Unavailable, + } + } +} + +fn truncate_utf8(value: &str, max_bytes: usize) -> (String, bool) { + let mut end = value.len().min(max_bytes); + while !value.is_char_boundary(end) { + end = end.saturating_sub(1); + } + (value[..end].to_string(), end < value.len()) +} + +impl ContextualUserFragment for SkillInstructions { + fn role(&self) -> &'static str { + "user" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + ("", "") + } + + fn body(&self) -> String { + let path = self + .path + .as_ref() + .map(|path| format!("{path}\n")) + .unwrap_or_default(); + let notice = match self.update { + SkillInstructionsUpdate::Initial => "", + SkillInstructionsUpdate::Replacement => REPLACEMENT_NOTICE, + SkillInstructionsUpdate::Unavailable => UNAVAILABLE_NOTICE, + }; + format!( + "\n{}\n{path}{notice}{}\n", + self.name, self.contents + ) + } +} diff --git a/codex-rs/core/src/context/world_state/mod.rs b/codex-rs/core/src/context/world_state/mod.rs index 50b9967096d7..954f2dbc4dd3 100644 --- a/codex-rs/core/src/context/world_state/mod.rs +++ b/codex-rs/core/src/context/world_state/mod.rs @@ -1,5 +1,6 @@ mod agents_md; mod environment; +mod skills; use crate::context::ContextualUserFragment; use codex_protocol::models::ContentItem; @@ -14,6 +15,7 @@ use std::fmt; pub(crate) use agents_md::AgentsMdState; pub(crate) use environment::EnvironmentsState; +pub(crate) use skills::SkillsState; trait ErasedWorldStateSection: Send + Sync { fn snapshot(&self) -> Option; diff --git a/codex-rs/core/src/context/world_state/skills.rs b/codex-rs/core/src/context/world_state/skills.rs new file mode 100644 index 000000000000..4347bd69aa7d --- /dev/null +++ b/codex-rs/core/src/context/world_state/skills.rs @@ -0,0 +1,140 @@ +use codex_core_skills::SkillsSnapshot; +use codex_core_skills::render_available_skills_body; +use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; +use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; +use serde::Deserialize; +use serde::Serialize; + +use super::PreviousSectionState; +use super::WorldStateSection; +use crate::context::ContextualUserFragment; + +const REPLACEMENT_NOTICE: &str = "This skills list replaces the previously provided skills list."; +const REMOVAL_NOTICE: &str = "The previously provided skills list no longer applies."; + +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq, Serialize)] +pub(crate) struct SkillsState { + skill_root_lines: Vec, + skill_lines: Vec, + omitted_count: usize, +} + +impl SkillsState { + pub(crate) fn new(skills: &SkillsSnapshot, enabled: bool) -> Self { + if !enabled { + return Self::default(); + } + let Some(available) = skills.available() else { + return Self::default(); + }; + Self { + skill_root_lines: available.skill_root_lines.clone(), + skill_lines: available.skill_lines.clone(), + omitted_count: available.report.omitted_count, + } + } + + fn has_catalog(&self) -> bool { + !self.skill_lines.is_empty() + } +} + +impl WorldStateSection for SkillsState { + const ID: &'static str = "skills"; + type Snapshot = Self; + + fn snapshot(&self) -> Self::Snapshot { + self.clone() + } + + fn matches_legacy_fragment(role: &str, text: &str) -> bool { + let text = text.trim(); + role == "developer" + && text.starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG) + && text.ends_with(SKILLS_INSTRUCTIONS_CLOSE_TAG) + } + + fn render_diff( + &self, + previous: PreviousSectionState<'_, Self::Snapshot>, + ) -> Option> { + let current = self.snapshot(); + if matches!(previous, PreviousSectionState::Known(previous) if previous == ¤t) { + return None; + } + let previous_had_catalog = match previous { + PreviousSectionState::Known(previous) => !previous.skill_lines.is_empty(), + PreviousSectionState::Unknown => true, + PreviousSectionState::Absent => false, + }; + if !self.has_catalog() && !previous_had_catalog { + return None; + } + Some(Box::new(SkillsStateFragment { + skill_root_lines: self.skill_root_lines.clone(), + skill_lines: self.skill_lines.clone(), + omitted_count: self.omitted_count, + include_policy: !previous_had_catalog, + notice: if !self.has_catalog() { + Some(REMOVAL_NOTICE) + } else if previous_had_catalog { + Some(REPLACEMENT_NOTICE) + } else { + None + }, + })) + } +} + +struct SkillsStateFragment { + skill_root_lines: Vec, + skill_lines: Vec, + omitted_count: usize, + include_policy: bool, + notice: Option<&'static str>, +} + +impl ContextualUserFragment for SkillsStateFragment { + fn role(&self) -> &'static str { + "developer" + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG) + } + + fn body(&self) -> String { + let catalog = (!self.skill_lines.is_empty()).then(|| { + if self.include_policy { + render_available_skills_body(&self.skill_root_lines, &self.skill_lines) + } else { + let mut lines = vec!["## Skills".to_string()]; + if !self.skill_root_lines.is_empty() { + lines.push("### Skill roots".to_string()); + lines.extend(self.skill_root_lines.iter().cloned()); + } + lines.push("### Available skills".to_string()); + lines.extend(self.skill_lines.iter().cloned()); + format!("\n{}\n", lines.join("\n")) + } + }).map(|mut catalog| { + if self.omitted_count > 0 { + catalog.push_str(&format!( + "\n{} additional skills are omitted from this model-visible list because of the catalog size limit.\n", + self.omitted_count + )); + } + catalog + }); + match (self.notice, catalog) { + (Some(notice), Some(catalog)) => format!("\n{notice}\n{catalog}"), + (Some(notice), None) => format!("\n{notice}\n"), + (None, Some(catalog)) => catalog, + (None, None) => String::new(), + } + } +} diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index fa86dded559f..ef6a3ec5ea93 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::collections::HashSet; use std::fmt; use std::sync::Arc; @@ -227,6 +228,24 @@ pub(crate) struct TurnEnvironmentSnapshot { } impl TurnEnvironmentSnapshot { + /// Maps each captured environment to its exact ready handle, or `None` when it was starting. + pub(crate) fn captured_environments(&self) -> HashMap>> { + self.turn_environments + .iter() + .map(|environment| { + ( + environment.environment_id.clone(), + Some(Arc::clone(&environment.environment)), + ) + }) + .chain( + self.starting + .iter() + .map(|environment| (environment.selection.environment_id.clone(), None)), + ) + .collect() + } + pub(crate) fn primary(&self) -> Option<&TurnEnvironment> { self.turn_environments.first() } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index ad5f54123fa3..53f170dddf7f 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -78,7 +78,6 @@ pub(crate) mod prompt_debug; pub use prompt_debug::build_prompt_input; pub(crate) mod mentions { pub(crate) use crate::plugins::build_connector_slug_counts; - pub(crate) use crate::plugins::build_skill_name_counts; pub(crate) use crate::plugins::collect_explicit_app_ids; pub(crate) use crate::plugins::collect_explicit_plugin_mentions; pub(crate) use crate::plugins::collect_tool_mentions_from_messages; @@ -88,14 +87,7 @@ pub mod sandboxing; mod session_prefix; mod session_startup_prewarm; pub mod skills; -pub(crate) use skills::SkillInjections; -pub(crate) use skills::SkillMetadata; pub(crate) use skills::SkillsService; -pub(crate) use skills::build_available_skills; -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::injection; pub(crate) use skills::maybe_emit_implicit_skill_invocation; pub(crate) use skills::skills_load_input_from_config; diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 9b2b85aa2d0d..0f8eabc08b78 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -15,10 +15,10 @@ use codex_rmcp_client::perform_oauth_login; use tokio_util::sync::CancellationToken; use tracing::warn; -use crate::SkillMetadata; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::skills::model::SkillToolDependency; +use codex_core_skills::runtime::SkillCatalogEntry; use codex_mcp::ElicitationReviewerHandle; use codex_mcp::McpOAuthLoginSupport; use codex_mcp::McpPermissionPromptAutoApproveContext; @@ -35,13 +35,13 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( sess: &Session, turn_context: &TurnContext, cancellation_token: &CancellationToken, - mentioned_skills: &[SkillMetadata], + mentioned_skills: &[SkillCatalogEntry], elicitation_reviewer: Option, -) { +) -> bool { let originator_value = originator().value; if !is_first_party_originator(originator_value.as_str()) { // Only support first-party clients for now. - return; + return false; } let config = turn_context.config.clone(); @@ -50,24 +50,24 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( .features .enabled(codex_features::Feature::SkillMcpDependencyInstall) { - return; + return false; } let installed = sess.runtime_mcp_servers(config.as_ref()).await; let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed); if missing.is_empty() { - return; + return false; } let unprompted_missing = filter_prompted_mcp_dependencies(sess, &missing).await; if unprompted_missing.is_empty() { - return; + return false; } if should_install_mcp_dependencies(sess, turn_context, &unprompted_missing, cancellation_token) .await { - maybe_install_mcp_dependencies( + return maybe_install_mcp_dependencies( sess, turn_context, config.as_ref(), @@ -76,35 +76,36 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( ) .await; } + false } pub(crate) async fn maybe_install_mcp_dependencies( sess: &Session, turn_context: &TurnContext, config: &crate::config::Config, - mentioned_skills: &[SkillMetadata], + mentioned_skills: &[SkillCatalogEntry], elicitation_reviewer: Option, -) { +) -> bool { if mentioned_skills.is_empty() || !config .features .enabled(codex_features::Feature::SkillMcpDependencyInstall) { - return; + return false; } let codex_home = config.codex_home.clone(); let installed = sess.runtime_mcp_servers(config).await; let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed); if missing.is_empty() { - return; + return false; } let mut servers = match load_global_mcp_servers(&codex_home).await { Ok(servers) => servers, Err(err) => { warn!("failed to load MCP servers while installing skill dependencies: {err}"); - return; + return false; } }; @@ -120,7 +121,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( } if !updated { - return; + return false; } if let Err(err) = ConfigEditsBuilder::new(&codex_home) @@ -129,7 +130,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( .await { warn!("failed to persist MCP dependencies for mentioned skills: {err}"); - return; + return false; } for (name, server_config) in added { @@ -197,7 +198,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( } if let Err(err) = refresh_config.mcp_servers.set(configured_servers) { warn!("failed to refresh MCP dependencies for mentioned skills: {err}"); - return; + return false; } let refresh_servers = sess.runtime_mcp_servers(&refresh_config).await; sess.refresh_mcp_servers_now( @@ -208,6 +209,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( elicitation_reviewer, ) .await; + true } async fn should_install_mcp_dependencies( @@ -416,7 +418,7 @@ fn mcp_dependency_to_server_config( } fn collect_missing_mcp_dependencies( - mentioned_skills: &[SkillMetadata], + mentioned_skills: &[SkillCatalogEntry], installed: &HashMap, ) -> HashMap { let mut missing = HashMap::new(); diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index dd0da327c4d6..396d06564438 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -59,6 +59,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/core/src/plugins/mentions.rs b/codex-rs/core/src/plugins/mentions.rs index a5e94345cb09..016066d86d8d 100644 --- a/codex-rs/core/src/plugins/mentions.rs +++ b/codex-rs/core/src/plugins/mentions.rs @@ -102,8 +102,6 @@ pub(crate) fn collect_explicit_plugin_mentions( .collect() } -pub(crate) use crate::build_skill_name_counts; - pub(crate) fn build_connector_slug_counts( connectors: &[connectors::AppInfo], ) -> HashMap { diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 11e2c338bc39..bdd4cfb4d787 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -12,7 +12,6 @@ pub(crate) use injection::build_plugin_injections; pub(crate) use render::render_explicit_plugin_instructions; pub(crate) use mentions::build_connector_slug_counts; -pub(crate) use mentions::build_skill_name_counts; pub(crate) use mentions::collect_explicit_app_ids; pub(crate) use mentions::collect_explicit_plugin_mentions; pub(crate) use mentions::collect_tool_mentions_from_messages; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index be12e61e085e..95b609dd62c6 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -15,7 +15,6 @@ use crate::agent::AgentStatus; use crate::agent::agent_status_from_event; use crate::agent::status::is_final; use crate::attestation::AttestationProvider; -use crate::build_available_skills; use crate::compact; use crate::config::ManagedFeatures; use crate::config::resolve_tool_suggest_config_from_layer_stack; @@ -23,7 +22,6 @@ use crate::connectors; use crate::context::ApprovedCommandPrefixSaved; use crate::context::AppsInstructions; use crate::context::AvailablePluginsInstructions; -use crate::context::AvailableSkillsInstructions; use crate::context::CollaborationModeInstructions; use crate::context::ContextualUserFragment; use crate::context::MultiAgentModeInstructions; @@ -33,7 +31,6 @@ use crate::context::PersonalitySpecInstructions; use crate::context::RecommendedPluginsInstructions; use crate::context::world_state::WorldState; use crate::current_time::TimeProvider; -use crate::default_skill_metadata_budget; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::exec_policy::ExecPolicyManager; use crate::image_preparation::prepare_response_items; @@ -42,7 +39,6 @@ use crate::realtime_conversation::RealtimeConversationManager; use crate::session::step_context::StepContext; use crate::session::turn_context::TurnEnvironment; use crate::session_prefix::format_inter_agent_completion_message; -use crate::skills::SkillRenderSideEffects; use crate::skills_load_input_from_config; use crate::turn_metadata::TurnMetadataState; use crate::turn_timing::now_unix_timestamp_ms; @@ -303,8 +299,6 @@ pub(crate) struct PreviousTurnSettings { pub(crate) realtime_active: Option, } -#[cfg(test)] -use crate::SkillMetadata; use crate::SkillsService; use crate::exec_policy::ExecPolicyUpdateError; use crate::guardian::GuardianReviewSessionManager; @@ -315,6 +309,8 @@ use crate::session_startup_prewarm::SessionStartupPrewarmHandle; use crate::shell; #[cfg(test)] use crate::skills::SkillLoadOutcome; +#[cfg(test)] +use crate::skills::SkillMetadata; use crate::state::AutoCompactWindowIds; use crate::state::AutoCompactWindowSnapshot; use crate::state::PendingRequestPermissions; @@ -2828,9 +2824,35 @@ impl Session { .await; } let loaded_agents_md = self.services.agents_md_manager.get_loaded().await; + let selected_capability_roots = self + .services + .turn_environments + .environment_manager() + .resolve_selected_capability_roots( + &self.services.selected_capability_roots, + &environments.captured_environments(), + ) + .await; + let extra_skill_sources = self + .services + .thread_extension_data + .get::(); + let skills = Arc::new( + codex_core_skills::SkillsSnapshot::load( + turn_context.turn_skills.snapshot.clone(), + &self.services.executor_skill_catalog_cache, + &selected_capability_roots, + extra_skill_sources.as_deref(), + turn_context.session_source.restriction_product(), + turn_context.model_context_window(), + ) + .await, + ); Arc::new(StepContext::new( turn_context, environments, + selected_capability_roots, + skills, loaded_agents_md, )) } @@ -3202,29 +3224,6 @@ impl Session { developer_sections.push(apps_instructions.render()); } } - if turn_context.config.include_skill_instructions { - let available_skills = build_available_skills( - turn_context.turn_skills.snapshot.outcome(), - default_skill_metadata_budget(turn_context.model_info.context_window), - SkillRenderSideEffects::ThreadStart { - session_telemetry: &self.services.session_telemetry, - }, - ); - if let Some(available_skills) = available_skills { - let warning_message = available_skills.warning_message.clone(); - let skills_instructions = AvailableSkillsInstructions::from(available_skills); - if let Some(warning_message) = warning_message { - self.send_event_raw(Event { - id: String::new(), - msg: EventMsg::Warning(WarningEvent { - message: warning_message, - }), - }) - .await; - } - developer_sections.push(skills_instructions.render()); - } - } let loaded_plugins = self .services .plugins_manager diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index e43a26bbe530..85daa021b58a 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -10,6 +10,7 @@ use crate::state::ActiveTurn; use codex_extension_api::ExtensionDataInit; use codex_login::auth::AgentIdentityAuthPolicy; use codex_protocol::SessionId; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::config_types::ServiceTier; use codex_protocol::permissions::FileSystemPath; @@ -555,6 +556,10 @@ impl Session { config.current_time_reminder.as_ref(), external_time_provider, )?; + let selected_capability_roots = thread_extension_init + .get::>() + .map(|roots| roots.as_ref().clone()) + .unwrap_or_else(|| initial_history.get_selected_capability_roots()); let mcp_thread_init = thread_extension_init.clone(); let thread_extension_data = codex_extension_api::ExtensionData::new_with_init( thread_id.to_string(), @@ -584,6 +589,7 @@ impl Session { text: session_configuration.base_instructions.clone(), }, dynamic_tools: session_configuration.dynamic_tools.clone(), + selected_capability_roots: selected_capability_roots.clone(), multi_agent_version: initial_multi_agent_version, initial_window_id: initial_auto_compact_window_ids .window_id @@ -1034,6 +1040,7 @@ impl Session { guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), skills_service, + executor_skill_catalog_cache: Default::default(), agents_md_manager, plugins_manager: Arc::clone(&plugins_manager), mcp_manager: Arc::clone(&mcp_manager), @@ -1041,6 +1048,7 @@ impl Session { // TODO(jif): extract session to share between sub-agents session_extension_data, thread_extension_data, + selected_capability_roots, mcp_thread_init, supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new( supports_openai_form_elicitation, diff --git a/codex-rs/core/src/session/step_context.rs b/codex-rs/core/src/session/step_context.rs index bf672db6e7eb..d13110e281c7 100644 --- a/codex-rs/core/src/session/step_context.rs +++ b/codex-rs/core/src/session/step_context.rs @@ -3,12 +3,18 @@ use std::sync::Arc; use crate::agents_md::LoadedAgentsMd; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::session::turn_context::TurnContext; +use codex_core_skills::SkillsSnapshot; +use codex_exec_server::ResolvedSelectedCapabilityRoot; /// Request-scoped state that may change between model sampling requests. #[derive(Debug)] pub(crate) struct StepContext { pub(crate) turn: Arc, pub(crate) environments: TurnEnvironmentSnapshot, + /// Capability roots bound to ready environments in this exact step. + pub(crate) selected_capability_roots: Vec, + /// Complete skill catalog and exact read routes captured for this step. + pub(crate) skills: Arc, /// The canonical AGENTS.md value observed with this environment snapshot. pub(crate) loaded_agents_md: Option>, } @@ -17,11 +23,15 @@ impl StepContext { pub(crate) fn new( turn: Arc, environments: TurnEnvironmentSnapshot, + selected_capability_roots: Vec, + skills: Arc, loaded_agents_md: Option>, ) -> Self { Self { turn, environments, + selected_capability_roots, + skills, loaded_agents_md, } } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 4553d54a0c7a..d055277a28e0 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -13,6 +13,7 @@ use crate::session::step_context::StepContext; use crate::shell::default_user_shell; use crate::shell_snapshot::ShellSnapshot; use crate::skills::SkillRenderSideEffects; +use crate::skills::build_available_skills; use crate::skills::render::SkillMetadataBudget; use crate::test_support::models_manager_with_provider; use crate::tools::format_exec_output_str; @@ -187,9 +188,15 @@ use std::time::Duration as StdDuration; impl StepContext { pub(crate) fn for_test(turn: Arc) -> Arc { let environments = turn.environments.clone(); + let skills = Arc::new(codex_core_skills::SkillsSnapshot::from_host( + turn.turn_skills.snapshot.clone(), + turn.model_context_window(), + )); Arc::new(Self::new( turn, environments, + Vec::new(), + skills, /*loaded_agents_md*/ None, )) } @@ -4043,6 +4050,7 @@ async fn attach_thread_persistence(session: &mut Session) -> PathBuf { originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { @@ -5396,6 +5404,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), skills_service, + executor_skill_catalog_cache: Default::default(), agents_md_manager: Arc::new(AgentsMdManager::new(/*user_instructions*/ None)), plugins_manager, mcp_manager, @@ -5404,6 +5413,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { agent_control.session_id().to_string(), ), thread_extension_data: codex_extension_api::ExtensionData::new(thread_id.to_string()), + selected_capability_roots: Vec::new(), mcp_thread_init: codex_extension_api::ExtensionDataInit::default(), supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false), agent_control, @@ -6919,6 +6929,7 @@ async fn shutdown_complete_does_not_append_to_thread_store_after_shutdown() { originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { @@ -7471,6 +7482,7 @@ where guardian_rejection_circuit_breaker: Mutex::new(Default::default()), runtime_handle: tokio::runtime::Handle::current(), skills_service, + executor_skill_catalog_cache: Default::default(), agents_md_manager: Arc::new(AgentsMdManager::new(/*user_instructions*/ None)), plugins_manager, mcp_manager, @@ -7479,6 +7491,7 @@ where agent_control.session_id().to_string(), ), thread_extension_data: codex_extension_api::ExtensionData::new(thread_id.to_string()), + selected_capability_roots: Vec::new(), mcp_thread_init: codex_extension_api::ExtensionDataInit::default(), supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false), agent_control, @@ -8446,62 +8459,6 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o ); } -#[tokio::test] -async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_builds() { - let (session, turn_context, rx) = make_session_and_context_with_rx().await; - let mut turn_context = Arc::into_inner(turn_context).expect("sole thread settings owner"); - let mut outcome = SkillLoadOutcome::default(); - outcome.skills = vec![ - SkillMetadata { - name: "admin-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), - scope: SkillScope::Admin, - plugin_id: None, - }, - SkillMetadata { - name: "repo-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - }, - ]; - turn_context.model_info.context_window = Some(100); - turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome))); - let turn_context = Arc::new(turn_context); - - let _ = build_initial_context(&session, &turn_context).await; - let warning_event = timeout(Duration::from_secs(1), rx.recv()) - .await - .expect("warning event should arrive") - .expect("warning event should be readable"); - assert!(matches!( - warning_event.msg, - EventMsg::Warning(WarningEvent { message }) - if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list." - )); - - let _ = build_initial_context(&session, &turn_context).await; - let warning_event = timeout(Duration::from_secs(1), rx.recv()) - .await - .expect("warning event should arrive on repeated build") - .expect("warning event should be readable"); - assert!(matches!( - warning_event.msg, - EventMsg::Warning(WarningEvent { message }) - if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list." - )); -} - #[tokio::test] async fn handle_output_item_done_records_image_save_history_message() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 9b3588fc1e68..448fb7186966 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -4,12 +4,9 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::Ordering; -use crate::SkillInjections; -use crate::build_skill_injections; use crate::client::ModelClientSession; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; -use crate::collect_explicit_skill_mentions; use crate::compact::InitialContextInjection; use crate::compact::run_inline_auto_compact_task; use crate::compact::should_use_remote_compact_task; @@ -17,6 +14,7 @@ use crate::compact_remote::run_inline_remote_auto_compact_task; use crate::compact_remote_v2::run_inline_remote_auto_compact_task as run_inline_remote_auto_compact_task_v2; use crate::connectors; use crate::context::ContextualUserFragment; +use crate::context::SkillInstructions; use crate::feedback_tags; use crate::hook_runtime::inspect_pending_input; use crate::hook_runtime::record_additional_contexts; @@ -30,7 +28,6 @@ use crate::injection::tool_kind_for_path; use crate::mcp_skill_dependencies::maybe_prompt_and_install_mcp_dependencies; use crate::mcp_tool_exposure::build_mcp_tool_exposure; use crate::mentions::build_connector_slug_counts; -use crate::mentions::build_skill_name_counts; use crate::mentions::collect_explicit_app_ids; use crate::mentions::collect_explicit_plugin_mentions; use crate::mentions::collect_tool_mentions_from_messages; @@ -71,11 +68,12 @@ use codex_analytics::AppInvocation; use codex_analytics::CompactionPhase; use codex_analytics::CompactionReason; use codex_analytics::InvocationType; +use codex_analytics::SkillInvocation; use codex_analytics::TurnResolvedConfigFact; use codex_analytics::build_track_events_context; use codex_async_utils::OrCancelExt; use codex_core_plugins::RecommendedPluginCandidatesInput; -use codex_core_skills::injection::InjectedHostSkillPrompts; +use codex_core_skills::SkillInjectionIdentity; use codex_extension_api::TurnInputContext; use codex_extension_api::TurnInputEnvironment; use codex_features::Feature; @@ -171,19 +169,28 @@ pub(crate) async fn run_turn( .record_context_updates_and_set_reference_context_item(first_step_context.as_ref()) .await; - let Some((injection_items, explicitly_enabled_connectors)) = - build_skills_and_plugins(&sess, turn_context.as_ref(), &input, &cancellation_token).await - else { - return Ok(None); - }; - if run_pending_session_start_hooks(&sess, &turn_context).await { return Ok(None); } let mut can_drain_pending_input = input.is_empty(); - if run_hooks_and_record_inputs(&sess, &turn_context, &input).await { + let (accepted_input, should_stop) = + run_hooks_and_record_inputs(&sess, &turn_context, &input).await; + if should_stop { return Ok(None); } + let mut accepted_user_input = user_input_from_turn_input(&accepted_input); + + let Some((injection_items, explicitly_enabled_connectors, available_connectors)) = + build_skills_and_plugins( + &sess, + turn_context.as_ref(), + &accepted_input, + &cancellation_token, + ) + .await + else { + return Ok(None); + }; sess.merge_connector_selection(explicitly_enabled_connectors.clone()) .await; @@ -217,6 +224,7 @@ pub(crate) async fn run_turn( // 2. After auto-compact, when model/tool continuation needs to resume before any steer. let mut next_step_context = Some(first_step_context); + let mut skill_injection_state = SkillInjectionState::default(); loop { // Note that pending_input would be something like a message the user // submitted through the UI while the model was running. Though the UI @@ -227,9 +235,12 @@ pub(crate) async fn run_turn( Vec::new() }; - if run_hooks_and_record_inputs(&sess, &turn_context, &pending_input).await { + let (accepted_pending_input, should_stop) = + run_hooks_and_record_inputs(&sess, &turn_context, &pending_input).await; + if should_stop { break; } + accepted_user_input.extend(user_input_from_turn_input(&accepted_pending_input)); let window_id = sess.current_window_id().await; super::rollout_budget::maybe_record_reminder( @@ -252,15 +263,19 @@ pub(crate) async fn run_turn( ) .await?; - if turn_context - .config - .features - .enabled(Feature::DeferredExecutor) - { - world_state = sess - .record_step_world_state_if_changed(&world_state, step_context.as_ref()) - .await; - } + world_state = sess + .record_step_world_state_if_changed(&world_state, step_context.as_ref()) + .await; + let _mcp_refreshed = record_skill_injections( + &sess, + turn_context.as_ref(), + step_context.as_ref(), + &accepted_user_input, + &available_connectors, + &cancellation_token, + &mut skill_injection_state, + ) + .await; // Construct the input that we will send to the model. let sampling_request_input: Vec = async { @@ -360,6 +375,7 @@ pub(crate) async fn run_turn( return Ok(None); } can_drain_pending_input = !model_needs_follow_up; + skill_injection_state = SkillInjectionState::default(); continue; } @@ -477,9 +493,10 @@ async fn run_hooks_and_record_inputs( sess: &Arc, turn_context: &Arc, input: &[TurnInput], -) -> bool { +) -> (Vec, bool) { let mut blocked_input = false; let mut accepted_user_input = false; + let mut accepted_input = Vec::with_capacity(input.len()); for input_item in input { let hook_outcome = inspect_pending_input(sess, turn_context, input_item).await; if hook_outcome.should_stop { @@ -496,9 +513,216 @@ async fn run_hooks_and_record_inputs( hook_outcome.additional_contexts, ) .await; + accepted_input.push(input_item.clone()); } } - blocked_input && !accepted_user_input + (accepted_input, blocked_input && !accepted_user_input) +} + +fn user_input_from_turn_input(input: &[TurnInput]) -> Vec { + input + .iter() + .filter_map(|item| match item { + TurnInput::UserInput { content, .. } => Some(content.as_slice()), + TurnInput::ResponseItem(_) | TurnInput::InterAgentCommunication(_) => None, + }) + .flatten() + .cloned() + .collect() +} + +const MAX_SKILL_INJECTIONS_PER_TURN: usize = 8; +const MAX_SKILL_INJECTION_BYTES_PER_TURN: usize = 32 * 1024; + +#[derive(Default)] +struct SkillInjectionState { + active: HashMap, + injected_items: usize, + injected_bytes: usize, + emitted_warnings: HashSet, +} + +async fn record_skill_injections( + sess: &Arc, + turn_context: &TurnContext, + step_context: &StepContext, + input: &[UserInput], + available_connectors: &[connectors::AppInfo], + cancellation_token: &CancellationToken, + state: &mut SkillInjectionState, +) -> bool { + if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) { + return false; + } + for warning in step_context.skills.warnings() { + emit_skill_warning_once( + sess, + turn_context, + warning.clone(), + &mut state.emitted_warnings, + ) + .await; + } + let plain_name_conflicts = build_connector_slug_counts(available_connectors) + .into_keys() + .map(|slug| slug.to_ascii_lowercase()) + .collect(); + let injections = step_context + .skills + .injections( + input, + &plain_name_conflicts, + &state.active, + MAX_SKILL_INJECTIONS_PER_TURN.saturating_sub(state.injected_items), + ) + .await; + for warning in injections.warnings { + emit_skill_warning_once(sess, turn_context, warning, &mut state.emitted_warnings).await; + } + let mut accepted = Vec::new(); + let mut planned_bytes = state.injected_bytes; + for injection in &injections.items { + let remaining_bytes = MAX_SKILL_INJECTION_BYTES_PER_TURN.saturating_sub(planned_bytes); + let Some((instructions, truncated)) = SkillInstructions::from_runtime( + &injection.entry, + &injection.contents, + remaining_bytes, + state.active.contains_key(&injection.entry.name), + ) else { + emit_skill_warning_once( + sess, + turn_context, + "Additional selected skills were not loaded because this turn reached the 32 KB skill instruction limit." + .to_string(), + &mut state.emitted_warnings, + ) + .await; + break; + }; + if truncated { + emit_skill_warning_once( + sess, + turn_context, + format!( + "Skill `{}` exceeded the per-skill instruction limit and was truncated.", + injection.entry.name + ), + &mut state.emitted_warnings, + ) + .await; + } + let rendered_bytes = instructions.render().len(); + planned_bytes = planned_bytes.saturating_add(rendered_bytes); + accepted.push((injection, instructions, rendered_bytes)); + } + + let replacement_names = accepted + .iter() + .map(|(injection, _, _)| injection.entry.name.as_str()) + .collect::>(); + let unavailable_names = state + .active + .iter() + .filter(|(name, identity)| { + !replacement_names.contains(name.as_str()) + && !step_context.skills.contains_injection(identity) + }) + .map(|(name, _)| name.clone()) + .collect::>(); + let mut items = unavailable_names + .iter() + .map(|name| ContextualUserFragment::into(SkillInstructions::unavailable(name))) + .collect::>(); + for name in unavailable_names { + state.active.remove(&name); + } + + if accepted.is_empty() { + if !items.is_empty() { + sess.record_conversation_items(turn_context, &items).await; + } + return false; + } + + let selected_entries = accepted + .iter() + .map(|(injection, _, _)| injection.entry.clone()) + .collect::>(); + let mcp_refreshed = maybe_prompt_and_install_mcp_dependencies( + sess, + turn_context, + cancellation_token, + &selected_entries, + Some(sess.mcp_elicitation_reviewer()), + ) + .await; + + items.extend( + accepted + .iter() + .map(|(_, instructions, _)| ContextualUserFragment::into(instructions.clone())), + ); + let connector_ids = collect_explicit_app_ids_from_skill_items( + &items, + available_connectors, + &step_context.skills.skill_name_counts_lower(), + ); + sess.merge_connector_selection(connector_ids).await; + + let skill_invocations = accepted + .iter() + .filter_map(|(injection, _, _)| { + step_context + .skills + .host_skill(&injection.entry) + .map(|skill| SkillInvocation { + skill_name: skill.name.clone(), + skill_scope: skill.scope, + skill_path: skill.path_to_skills_md.to_path_buf(), + plugin_id: skill.plugin_id.clone(), + invocation_type: InvocationType::Explicit, + }) + }) + .collect::>(); + sess.services + .analytics_events_client + .track_skill_invocations( + build_track_events_context( + turn_context.model_info.slug.clone(), + sess.thread_id.to_string(), + turn_context.sub_id.clone(), + turn_context.originator.clone(), + ), + skill_invocations, + ); + for (injection, _, rendered_bytes) in accepted { + turn_context.session_telemetry.counter( + "codex.skill.injected", + /*inc*/ 1, + &[("status", "ok"), ("skill", injection.entry.name.as_str())], + ); + state.injected_items = state.injected_items.saturating_add(1); + state.injected_bytes = state.injected_bytes.saturating_add(rendered_bytes); + state + .active + .insert(injection.entry.name.clone(), injection.identity.clone()); + } + if !items.is_empty() { + sess.record_conversation_items(turn_context, &items).await; + } + mcp_refreshed +} + +async fn emit_skill_warning_once( + sess: &Session, + turn_context: &TurnContext, + message: String, + emitted: &mut HashSet, +) { + if emitted.insert(message.clone()) { + sess.send_event(turn_context, EventMsg::Warning(WarningEvent { message })) + .await; + } } #[instrument(level = "trace", skip_all)] @@ -507,22 +731,14 @@ async fn build_skills_and_plugins( turn_context: &TurnContext, input: &[TurnInput], cancellation_token: &CancellationToken, -) -> Option<(Vec, HashSet)> { +) -> Option<(Vec, HashSet, Vec)> { // Guardian input embeds the parent transcript as untrusted evidence. Do not interpret skill or // plugin mentions from that generated prompt as requests to inject additional instructions. if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) { - return Some((Vec::new(), HashSet::new())); + return Some((Vec::new(), HashSet::new(), Vec::new())); } - let user_input = input - .iter() - .filter_map(|item| match item { - TurnInput::UserInput { content, .. } => Some(content.as_slice()), - TurnInput::ResponseItem(_) | TurnInput::InterAgentCommunication(_) => None, - }) - .flatten() - .cloned() - .collect::>(); + let user_input = user_input_from_turn_input(input); let tracking = build_track_events_context( turn_context.model_info.slug.clone(), sess.thread_id.to_string(), @@ -572,61 +788,12 @@ async fn build_skills_and_plugins( } else { Vec::new() }; - let skills_outcome = turn_context.turn_skills.snapshot.outcome(); - let connector_slug_counts = build_connector_slug_counts(&available_connectors); let extension_injection_items = build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token) .await?; - let skill_name_counts_lower = - build_skill_name_counts(&skills_outcome.skills, &skills_outcome.disabled_paths).1; - let mentioned_skills = collect_explicit_skill_mentions( - &user_input, - &skills_outcome.skills, - &skills_outcome.disabled_paths, - &connector_slug_counts, - ); - maybe_prompt_and_install_mcp_dependencies( - sess, - turn_context, - cancellation_token, - &mentioned_skills, - Some(sess.mcp_elicitation_reviewer()), - ) - .await; - - let injected_host_skill_prompts = turn_context - .extension_data - .get::(); - let SkillInjections { - items: skill_injections, - warnings: skill_warnings, - } = build_skill_injections( - &mentioned_skills, - Some(skills_outcome), - Some(&turn_context.session_telemetry), - &sess.services.analytics_events_client, - tracking.clone(), - ) - .await; - - for message in skill_warnings { - sess.send_event(turn_context, EventMsg::Warning(WarningEvent { message })) - .await; - } - - let skill_items: Vec = skill_injections - .iter() - .map(|skill| ContextualUserFragment::into(crate::context::SkillInstructions::from(skill))) - .collect(); - let skill_connector_ids = collect_explicit_app_ids_from_skill_items( - &skill_items, - &available_connectors, - &skill_name_counts_lower, - ); let plugin_items = build_plugin_injections(&mentioned_plugins, &mcp_tools, &available_connectors); - let mut explicitly_enabled_connectors = collect_explicit_app_ids(&user_input); - explicitly_enabled_connectors.extend(skill_connector_ids); + let explicitly_enabled_connectors = collect_explicit_app_ids(&user_input); let connector_names_by_id = available_connectors .iter() .map(|connector| (connector.id.as_str(), connector.name.as_str())) @@ -656,19 +823,13 @@ async fn build_skills_and_plugins( } } - let mut injection_items: Vec = match injected_host_skill_prompts { - Some(injected_host_skill_prompts) => skill_injections - .iter() - .filter(|skill| !injected_host_skill_prompts.contains_path(&skill.path)) - .map(|skill| { - ContextualUserFragment::into(crate::context::SkillInstructions::from(skill)) - }) - .collect(), - None => skill_items, - }; - injection_items.extend(plugin_items); + let mut injection_items = plugin_items; injection_items.extend(extension_injection_items); - Some((injection_items, explicitly_enabled_connectors)) + Some(( + injection_items, + explicitly_enabled_connectors, + available_connectors, + )) } #[tracing::instrument( diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index e19e8c2ff43f..b05ccb88b914 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -2,6 +2,7 @@ use super::session::Session; use super::step_context::StepContext; use crate::context::world_state::AgentsMdState; use crate::context::world_state::EnvironmentsState; +use crate::context::world_state::SkillsState; use crate::context::world_state::WorldState; impl Session { @@ -10,6 +11,10 @@ impl Session { step_context: &StepContext, ) -> WorldState { let turn_context = step_context.turn.as_ref(); + tracing::trace!( + selected_capability_root_count = step_context.selected_capability_roots.len(), + "building step world state" + ); let environment_subagents = if turn_context.config.include_environment_context { self.services .agent_control @@ -21,6 +26,10 @@ impl Session { let mut world_state = WorldState::default(); world_state.add_section(AgentsMdState::new(step_context.loaded_agents_md.as_deref())); + world_state.add_section(SkillsState::new( + step_context.skills.as_ref(), + turn_context.config.include_skill_instructions, + )); if turn_context.config.include_environment_context { world_state.add_section( EnvironmentsState::from_turn_context_with_environments( diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 7b5d87b26f7c..0c328fe65cd4 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -22,9 +22,6 @@ pub use codex_core_skills::default_skill_metadata_budget; pub use codex_core_skills::detect_implicit_skill_invocation_for_command; pub use codex_core_skills::filter_skill_load_outcome_for_product; pub use codex_core_skills::injection; -pub use codex_core_skills::injection::SkillInjections; -pub use codex_core_skills::injection::build_skill_injections; -pub use codex_core_skills::injection::collect_explicit_skill_mentions; pub use codex_core_skills::loader; pub use codex_core_skills::model; pub use codex_core_skills::remote; diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index cfed7d90d032..a685a239c206 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -25,6 +25,7 @@ use arc_swap::ArcSwap; use arc_swap::ArcSwapOption; use codex_analytics::AnalyticsEventsClient; use codex_core_plugins::PluginsManager; +use codex_core_skills::ExecutorSkillCatalogCache; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionDataInit; use codex_extension_api::ExtensionRegistry; @@ -33,6 +34,7 @@ use codex_login::AuthManager; use codex_mcp::McpConnectionManager; use codex_models_manager::manager::SharedModelsManager; use codex_otel::SessionTelemetry; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_rollout::state_db::StateDbHandle; use codex_rollout_trace::ThreadTraceContext; use codex_thread_store::LiveThread; @@ -65,6 +67,9 @@ pub(crate) struct SessionServices { pub(crate) guardian_rejection_circuit_breaker: Mutex, pub(crate) runtime_handle: Handle, pub(crate) skills_service: Arc, + /// Catalogs discovered from executor capability roots. The cache is session-scoped because + /// selected environment contents are stable for the session lifetime. + pub(crate) executor_skill_catalog_cache: ExecutorSkillCatalogCache, pub(crate) agents_md_manager: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, @@ -72,6 +77,9 @@ pub(crate) struct SessionServices { pub(crate) session_extension_data: ExtensionData, pub(crate) thread_extension_data: ExtensionData, pub(crate) supports_openai_form_elicitation: AtomicBool, + /// Raw capability selections for this thread. Each model step resolves them against its + /// current executor environments before using them. + pub(crate) selected_capability_roots: Vec, pub(crate) mcp_thread_init: ExtensionDataInit, pub(crate) agent_control: AgentControl, pub(crate) network_proxy: ArcSwapOption, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 98e13c5d67d8..b2add06024fc 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1402,6 +1402,7 @@ impl ThreadManagerState { inherited_environments: Option, inherited_exec_policy: Option>, environments: Option>, + thread_extension_init: ExtensionDataInit, ) -> CodexResult { let environments = environments.unwrap_or_else(|| { default_thread_environment_selections(self.environment_manager.as_ref(), &config.cwd) @@ -1421,7 +1422,7 @@ impl ThreadManagerState { inherited_exec_policy, /*parent_trace*/ None, environments, - /*thread_extension_init*/ ExtensionDataInit::default(), + thread_extension_init, /*supports_openai_form_elicitation*/ false, /*user_shell_override*/ None, )) diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index f1acc2d9799c..0d407e19c5d7 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -20,6 +20,8 @@ use codex_protocol::protocol::AgentMessageEvent; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::InternalSessionSource; use codex_protocol::protocol::ResumedHistory; +use codex_protocol::protocol::SessionMeta; +use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TurnStartedEvent; @@ -610,6 +612,71 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() ); } +#[tokio::test] +async fn selected_capability_roots_round_trip_through_fork() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config().await; + config.codex_home = temp_dir.path().join("codex-home").abs(); + config.cwd = config.codex_home.abs(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let manager = ThreadManager::with_models_provider_and_home_for_tests( + CodexAuth::from_api_key("dummy"), + config.model_provider.clone(), + config.codex_home.to_path_buf(), + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + ); + let selected_roots = vec![SelectedCapabilityRoot { + id: "demo@1".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "build".to_string(), + path: PathUri::parse("file:///plugins/demo").expect("plugin root URI"), + }, + }]; + let inherited = manager + .start_thread_with_options(StartThreadOptions { + config, + initial_history: InitialHistory::Forked(vec![RolloutItem::SessionMeta( + SessionMetaLine { + meta: SessionMeta { + selected_capability_roots: selected_roots.clone(), + ..SessionMeta::default() + }, + git: None, + }, + )]), + session_source: None, + thread_source: None, + dynamic_tools: Vec::new(), + metrics_service_name: None, + parent_trace: None, + environments: Vec::new(), + thread_extension_init: Default::default(), + supports_openai_form_elicitation: false, + }) + .await + .expect("start inherited fork"); + inherited.thread.ensure_rollout_materialized().await; + inherited + .thread + .flush_rollout() + .await + .expect("flush inherited fork"); + let inherited_history = RolloutRecorder::get_rollout_history( + &inherited + .thread + .rollout_path() + .expect("inherited fork rollout path"), + ) + .await + .expect("read inherited fork rollout"); + + assert_eq!( + inherited_history.get_selected_capability_roots(), + selected_roots + ); +} + #[tokio::test] async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { let temp_dir = tempdir().expect("tempdir"); diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index c422cc749e52..54eabec83c6d 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -685,10 +685,16 @@ async fn environment_tools_follow_the_step_context() { turn.model_info.apply_patch_tool_type = Some(ApplyPatchToolType::Freeform); let environments = turn.environments.clone(); + let skills = Arc::new(codex_core_skills::SkillsSnapshot::from_host( + turn.turn_skills.snapshot.clone(), + turn.model_context_window(), + )); turn.environments.turn_environments.clear(); let step_context = Arc::new(StepContext::new( Arc::new(turn), environments, + Vec::new(), + skills, /*loaded_agents_md*/ None, )); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index eb4fa3b39f39..8a4f5cca7dc1 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -26,6 +26,7 @@ use codex_core::thread_store_from_config; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; +use codex_extension_api::ExtensionDataInit; use codex_extension_api::ExtensionRegistry; use codex_extension_api::LoadUserInstructionsFuture; use codex_extension_api::UserInstructionsProvider; @@ -36,6 +37,7 @@ use codex_login::CodexAuth; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::built_in_model_providers; use codex_models_manager::bundled_models_response; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelsResponse; @@ -287,6 +289,7 @@ pub struct TestCodexBuilder { user_shell_override: Option, exec_server_url: Option, extensions: Arc>, + thread_extension_init: ExtensionDataInit, user_instructions_provider: Option>, supports_openai_form_elicitation: bool, external_time_provider: Option>, @@ -378,6 +381,19 @@ impl TestCodexBuilder { self } + pub fn with_thread_extension_init(mut self, thread_extension_init: ExtensionDataInit) -> Self { + self.thread_extension_init = thread_extension_init; + self + } + + pub fn with_selected_capability_roots( + mut self, + selected_capability_roots: Vec, + ) -> Self { + self.thread_extension_init.insert(selected_capability_roots); + self + } + pub fn with_user_instructions_provider( mut self, provider: Arc, @@ -665,7 +681,7 @@ impl TestCodexBuilder { metrics_service_name: None, parent_trace: None, environments, - thread_extension_init: Default::default(), + thread_extension_init: std::mem::take(&mut self.thread_extension_init), supports_openai_form_elicitation: self.supports_openai_form_elicitation, }), ) @@ -1214,6 +1230,7 @@ pub fn test_codex() -> TestCodexBuilder { user_shell_override: None, exec_server_url: None, extensions: empty_extension_registry(), + thread_extension_init: ExtensionDataInit::new(), user_instructions_provider: None, supports_openai_form_elicitation: false, external_time_provider: None, diff --git a/codex-rs/core/tests/suite/personality_migration.rs b/codex-rs/core/tests/suite/personality_migration.rs index ca1d148728f2..3dd0f3c0169e 100644 --- a/codex-rs/core/tests/suite/personality_migration.rs +++ b/codex-rs/core/tests/suite/personality_migration.rs @@ -75,6 +75,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, @@ -127,6 +128,7 @@ async fn write_rollout_with_meta_only(dir: &Path, thread_id: ThreadId) -> io::Re model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 5f8c50349693..d34b8ffa6bd7 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -12,6 +12,8 @@ use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::RemoveOptions; use codex_features::Feature; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; @@ -468,6 +470,58 @@ async fn serve_environment_with_agents_md( } } +async fn serve_environment_with_skill( + listener: TcpListener, + skill_path: PathUri, + skill_contents: String, + attach: tokio::sync::oneshot::Receiver<()>, + mut shutdown: tokio::sync::oneshot::Receiver<()>, +) -> usize { + let mut websocket = accept_initialized_exec_server(listener).await; + attach.await.expect("attach signal"); + send_environment_info(&mut websocket).await; + let mut reads = 0; + let skill_path_string = skill_path.to_string(); + loop { + let request = tokio::select! { + request = read_exec_server_json(&mut websocket) => request, + _ = &mut shutdown => return reads, + }; + let response = match request["method"].as_str() { + Some("fs/canonicalize") => json!({ + "id": request["id"], + "result": { "path": request["params"]["path"] } + }), + Some("fs/walk") => json!({ + "id": request["id"], + "result": { + "entries": [{ "path": skill_path, "kind": "file" }], + "errors": [], + "truncated": false, + } + }), + Some("fs/getMetadata") => json!({ + "id": request["id"], + "error": { "code": -32004, "message": "not found" } + }), + Some("fs/readFile") + if request["params"]["path"].as_str() == Some(skill_path_string.as_str()) => + { + reads += 1; + json!({ + "id": request["id"], + "result": { "dataBase64": BASE64_STANDARD.encode(&skill_contents) } + }) + } + method => panic!("unexpected exec-server request: {method:?}"), + }; + websocket + .send(Message::Text(response.to_string().into())) + .await + .expect("filesystem response"); + } +} + fn tool_names(body: &Value) -> Vec { body["tools"] .as_array() @@ -730,6 +784,148 @@ async fn deferred_executor_loads_agents_md_when_environment_becomes_ready() -> R Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn deferred_executor_reuses_skill_catalog_after_environment_ready() -> Result<()> { + const HOST_BODY: &str = "HOST_SKILL_BODY_MARKER"; + const EXECUTOR_BODY: &str = "EXECUTOR_SKILL_BODY_MARKER"; + const SKILL_REPLACEMENT_NOTICE: &str = + "These instructions replace the previously provided instructions for this skill."; + let root_path = PathUri::from_host_native_path("/remote-capability")?; + let skill_path = root_path.join("skills/deploy/SKILL.md")?; + let skill_contents = format!( + "---\nname: deploy\ndescription: Deploy through the ready executor.\n---\n\n{EXECUTOR_BODY}\n" + ); + let selected_capability_roots = vec![SelectedCapabilityRoot { + id: "remote-capability@1".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + path: root_path, + }, + }]; + + let listener = TcpListener::bind("127.0.0.1:0").await?; + let server = start_mock_server().await; + let response_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + "wait-1", + "wait_for_environment", + &json!({ "environment_id": REMOTE_ENVIRONMENT_ID }).to_string(), + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "done"), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-3", "still done"), + ev_completed("resp-3"), + ]), + ], + ) + .await; + let mut builder = test_codex() + .with_exec_server_url(format!("ws://{}", listener.local_addr()?)) + .with_selected_capability_roots(selected_capability_roots) + .with_pre_build_hook(|home| { + let skill_dir = home.join("skills/deploy"); + std::fs::create_dir_all(&skill_dir).expect("create host skill directory"); + std::fs::write( + skill_dir.join("SKILL.md"), + format!( + "---\nname: deploy\ndescription: Deploy from the host.\n---\n\n{HOST_BODY}\n" + ), + ) + .expect("write host skill"); + }) + .with_config(|config| { + config.project_doc_max_bytes = 0; + assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); + }); + let (attach_tx, attach_rx) = tokio::sync::oneshot::channel(); + let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel(); + let exec_server = tokio::spawn(serve_environment_with_skill( + listener, + skill_path, + skill_contents, + attach_rx, + shutdown_rx, + )); + let test = timeout(Duration::from_secs(5), builder.build(&server)) + .await + .context("thread startup should not wait for the remote environment")??; + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "Use $deploy after the environment is ready".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_response_request_count(&response_mock, /*expected_count*/ 1).await; + attach_tx.send(()).expect("attach environment"); + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "Continue without loading another skill".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + shutdown_tx.send(()).expect("stop exec server"); + assert_eq!(exec_server.await?, 2); + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 3); + let first_user_context = requests[0].message_input_texts("user"); + let host_skill = first_user_context + .iter() + .find(|text| text.starts_with("") && text.contains(HOST_BODY)) + .expect("first request should contain host skill instructions"); + assert!(!host_skill.contains(SKILL_REPLACEMENT_NOTICE)); + assert!( + first_user_context + .iter() + .all(|text| !text.contains(EXECUTOR_BODY)) + ); + let second_user_context = requests[1].message_input_texts("user"); + let executor_skill = second_user_context + .iter() + .find(|text| text.starts_with("") && text.contains(EXECUTOR_BODY)) + .expect("second request should contain executor skill instructions"); + assert!(executor_skill.contains(SKILL_REPLACEMENT_NOTICE)); + let second_developer_context = requests[1].message_input_texts("developer"); + let replacement_catalog = second_developer_context + .iter() + .find(|text| text.contains("This skills list replaces")) + .expect("second request should contain the replacement skill catalog"); + assert!(replacement_catalog.contains("(environment resource:")); + Ok(()) +} + fn agents_md_occurrences(request: &ResponsesRequest, contents: &str) -> usize { request .message_input_texts("user") diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 5eea5a09e716..1c2f08262a8f 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -372,6 +372,7 @@ async fn backfill_scans_existing_rollouts() -> Result<()> { model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 664ef730d0b3..66f738110315 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -54,7 +54,7 @@ pub const CODEX_EXEC_SERVER_NOISE_CHATGPT_ACCOUNT_ID_ENV_VAR: &str = #[derive(Debug)] pub struct EnvironmentManager { default_environment: Option, - environments: RwLock>>, + pub(super) environments: RwLock>>, local_environment: Option>, local_runtime_paths: Option, } @@ -574,6 +574,25 @@ impl Environment { } } + /// Starts the initial connection after an environment is actually selected for use. + pub(crate) fn start_connecting_for_use(environment: &Arc) { + if environment.remote_client.is_none() { + return; + } + let mut startup_task = environment + .startup_task + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if startup_task.is_none() { + let environment = Arc::clone(environment); + *startup_task = Some(AbortOnDropHandle::new(tokio::spawn(async move { + if let Err(error) = environment.wait_until_ready().await { + tracing::debug!(%error, "exec-server environment startup failed"); + } + }))); + } + } + /// Returns whether initial startup has either succeeded or permanently failed. pub fn startup_finished(&self) -> bool { self.remote_client diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 5b2bfa7e558b..92c7f85edfcc 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -22,6 +22,7 @@ mod relay_proto; mod remote; mod remote_file_system; mod remote_process; +mod resolved_capability; mod rpc; mod runtime_paths; mod sandboxed_file_system; @@ -143,6 +144,7 @@ pub use protocol::WriteResponse; pub use protocol::WriteStatus; pub use remote::RemoteEnvironmentConfig; pub use remote::run_remote_environment; +pub use resolved_capability::ResolvedSelectedCapabilityRoot; pub use runtime_paths::ExecServerRuntimePaths; pub use server::DEFAULT_LISTEN_URL; pub use server::ExecServerListenUrlParseError; diff --git a/codex-rs/exec-server/src/resolved_capability.rs b/codex-rs/exec-server/src/resolved_capability.rs new file mode 100644 index 000000000000..2902cf9eeffd --- /dev/null +++ b/codex-rs/exec-server/src/resolved_capability.rs @@ -0,0 +1,114 @@ +use std::collections::HashMap; +use std::fmt; +use std::sync::Arc; + +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; + +use crate::Environment; +use crate::EnvironmentManager; +use crate::ExecutorFileSystem; + +/// A selected capability root paired with its currently ready environment handle. +/// +/// Environment IDs have stable identity and contents. This process-local value must not be +/// persisted: it only keeps the current connection handle alive while one model step uses the +/// stable environment. +#[derive(Clone)] +pub struct ResolvedSelectedCapabilityRoot { + selected_root: SelectedCapabilityRoot, + environment: Arc, +} + +impl ResolvedSelectedCapabilityRoot { + pub fn selected_root(&self) -> &SelectedCapabilityRoot { + &self.selected_root + } + + pub fn environment(&self) -> &Arc { + &self.environment + } + + pub fn file_system(&self) -> Arc { + self.environment.get_filesystem() + } +} + +impl EnvironmentManager { + /// Resolves selected roots whose stable environments are ready for the current model step. + /// + /// Environment identity comes from the selected root's stable environment ID. A ready + /// environment captured for the step carries its exact process-local handle so readiness and + /// execution cannot come from different registry snapshots. Missing, starting, or failed + /// environments are omitted. A lazy environment is started for a later step. + pub async fn resolve_selected_capability_roots( + &self, + selected_roots: &[SelectedCapabilityRoot], + captured_environments: &HashMap>>, + ) -> Vec { + let candidates = { + let environments = self + .environments + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner); + selected_roots + .iter() + .filter_map(|selected_root| { + let CapabilityRootLocation::Environment { environment_id, .. } = + &selected_root.location; + let (environment, already_ready) = + match captured_environments.get(environment_id) { + Some(Some(environment)) => (Arc::clone(environment), true), + Some(None) => return None, + None => (Arc::clone(environments.get(environment_id)?), false), + }; + Some(( + ResolvedSelectedCapabilityRoot { + selected_root: selected_root.clone(), + environment, + }, + already_ready, + )) + }) + .collect::>() + }; + + let mut readiness = HashMap::new(); + for (candidate, already_ready) in &candidates { + let CapabilityRootLocation::Environment { environment_id, .. } = + &candidate.selected_root().location; + if readiness.contains_key(environment_id) { + continue; + } + let environment = candidate.environment(); + let ready = if *already_ready { + true + } else if environment.startup_finished() { + environment.wait_until_ready().await.is_ok() + } else { + Environment::start_connecting_for_use(environment); + false + }; + readiness.insert(environment_id.clone(), ready); + } + + candidates + .into_iter() + .map(|(candidate, _)| candidate) + .filter(|candidate| { + let CapabilityRootLocation::Environment { environment_id, .. } = + &candidate.selected_root().location; + readiness.get(environment_id).copied().unwrap_or(false) + }) + .collect() + } +} + +impl fmt::Debug for ResolvedSelectedCapabilityRoot { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("ResolvedSelectedCapabilityRoot") + .field("selected_root", &self.selected_root) + .finish_non_exhaustive() + } +} diff --git a/codex-rs/exec-server/tests/selected_capability_roots.rs b/codex-rs/exec-server/tests/selected_capability_roots.rs new file mode 100644 index 000000000000..639d652ca85c --- /dev/null +++ b/codex-rs/exec-server/tests/selected_capability_roots.rs @@ -0,0 +1,69 @@ +#![cfg(unix)] + +mod common; + +use std::collections::HashMap; +use std::sync::Arc; + +use codex_exec_server::EnvironmentManager; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_path_uri::PathUri; +use common::exec_server::exec_server; +use pretty_assertions::assert_eq; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn selected_capability_roots_use_captured_handle_after_replacement() -> anyhow::Result<()> { + let mut executor = exec_server().await?; + let manager = EnvironmentManager::without_environments(); + let selected_root = SelectedCapabilityRoot { + id: "demo@1".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "tools".to_string(), + path: PathUri::parse("file:///plugins/demo")?, + }, + }; + + manager.upsert_environment( + "tools".to_string(), + executor.websocket_url().to_string(), + /*connect_timeout*/ None, + )?; + let environment_a = manager + .get_environment("tools") + .expect("executor A should be registered"); + environment_a.wait_until_ready().await?; + + let unavailable = manager + .resolve_selected_capability_roots( + std::slice::from_ref(&selected_root), + &HashMap::from([("tools".to_string(), None)]), + ) + .await; + assert!(unavailable.is_empty()); + + let captured_environments = + HashMap::from([("tools".to_string(), Some(Arc::clone(&environment_a)))]); + // Replace only the process-local handle; the stable environment ID and executor stay the same. + manager.upsert_environment( + "tools".to_string(), + executor.websocket_url().to_string(), + /*connect_timeout*/ None, + )?; + + let available = manager + .resolve_selected_capability_roots( + std::slice::from_ref(&selected_root), + &captured_environments, + ) + .await; + let [resolved] = available.as_slice() else { + anyhow::bail!("selected root should resolve through its stable environment"); + }; + + assert_eq!(resolved.selected_root(), &selected_root); + assert!(Arc::ptr_eq(resolved.environment(), &environment_a)); + + executor.shutdown().await?; + Ok(()) +} diff --git a/codex-rs/ext/skills/src/config.rs b/codex-rs/ext/skills/src/config.rs deleted file mode 100644 index 3d903cafb848..000000000000 --- a/codex-rs/ext/skills/src/config.rs +++ /dev/null @@ -1,10 +0,0 @@ -/// Host-supplied configuration used by the skills extension. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct SkillsExtensionConfig { - /// Whether the available-skills catalog is included in model context. - pub include_instructions: bool, - /// Whether bundled skills are eligible for discovery. - pub bundled_skills_enabled: bool, - /// Whether orchestrator-owned skills are eligible for discovery. - pub orchestrator_skills_enabled: bool, -} diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index de9facd8c2f3..cb6f7bd3eb03 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -1,53 +1,23 @@ use std::sync::Arc; -use codex_core_skills::HostSkillsSnapshot; -use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_extension_api::ConfigContributor; -use codex_extension_api::ContextContributor; -use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; -use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionFuture; use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::PromptFragment; use codex_extension_api::ThreadLifecycleContributor; use codex_extension_api::ThreadStartInput; use codex_extension_api::ToolCall; use codex_extension_api::ToolContributor; use codex_extension_api::ToolExecutor; -use codex_extension_api::TurnInputContext; -use codex_extension_api::TurnInputContributor; use codex_mcp::McpResourceClient; -use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_protocol::protocol::Event; -use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::WarningEvent; -use crate::SkillsExtensionConfig; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillSourceKind; -use crate::fragments::SkillInstructions; -use crate::provider::HostSkillProvider; -use crate::provider::SkillListQuery; -use crate::provider::SkillReadRequest; -use crate::render::MAX_SKILL_NAME_BYTES; -use crate::render::MAX_SKILL_PATH_BYTES; -use crate::render::available_skills_fragment; -use crate::render::truncate_main_prompt_contents; -use crate::render::truncate_utf8_to_bytes; -use crate::selection::collect_explicit_skill_mentions; -use crate::sources::SkillProviders; +use crate::sources::orchestrator_skill_sources; use crate::state::SkillsThreadState; -use crate::state::SkillsTurnState; use crate::tools::skill_tools; struct SkillsExtension { - providers: SkillProviders, - event_sink: Arc, - config_from_host: Arc SkillsExtensionConfig + Send + Sync>, + orchestrator_skills_enabled: Arc bool + Send + Sync>, } impl ThreadLifecycleContributor for SkillsExtension @@ -56,19 +26,21 @@ where { fn on_thread_start<'a>(&'a self, input: ThreadStartInput<'a, C>) -> ExtensionFuture<'a, ()> { Box::pin(async move { - let selected_roots = input - .thread_store - .get::>() - .map(|selected_roots| selected_roots.as_ref().clone()) - .unwrap_or_default(); let orchestrator_skills_available = !input .environments .iter() .any(|environment| environment.environment_id == LOCAL_ENVIRONMENT_ID); - input.thread_store.insert(SkillsThreadState::new( - (self.config_from_host)(input.config), - selected_roots, - orchestrator_skills_available, + let thread_state = input.thread_store.get_or_init(|| { + SkillsThreadState::new( + (self.orchestrator_skills_enabled)(input.config), + orchestrator_skills_available, + ) + }); + thread_state + .set_orchestrator_skills_enabled((self.orchestrator_skills_enabled)(input.config)); + input.thread_store.insert(orchestrator_skill_sources( + thread_state, + input.session_store.get::(), )); }) } @@ -80,67 +52,26 @@ where { fn on_config_changed( &self, - _session_store: &ExtensionData, + session_store: &ExtensionData, thread_store: &ExtensionData, _previous_config: &C, new_config: &C, ) { - let next_config = (self.config_from_host)(new_config); + let enabled = (self.orchestrator_skills_enabled)(new_config); if let Some(state) = thread_store.get::() { - state.set_config(next_config); + state.set_orchestrator_skills_enabled(enabled); } else { let orchestrator_skills_available = true; - thread_store.insert(SkillsThreadState::new( - next_config, - Vec::new(), - orchestrator_skills_available, + let thread_state = thread_store + .get_or_init(|| SkillsThreadState::new(enabled, orchestrator_skills_available)); + thread_store.insert(orchestrator_skill_sources( + thread_state, + session_store.get::(), )); } } } -impl ContextContributor for SkillsExtension -where - C: Send + Sync + 'static, -{ - fn contribute_thread_context<'a>( - &'a self, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, - ) -> std::pin::Pin> + Send + 'a>> { - Box::pin(async move { - let Some(thread_state) = thread_store.get::() else { - return Vec::new(); - }; - let config = thread_state.config(); - if !config.include_instructions { - return Vec::new(); - } - let catalog = self - .list_skills( - SkillListQuery { - turn_id: thread_store.level_id().to_string(), - executor_roots: thread_state.selected_roots().to_vec(), - host_snapshot: None, - include_host_skills: false, - include_bundled_skills: config.bundled_skills_enabled, - include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), - mcp_resources: session_store.get::(), - }, - &thread_state, - ) - .await; - for warning in &catalog.warnings { - self.emit_warning(thread_store.level_id(), warning.clone()); - } - available_skills_fragment(&catalog) - .map(|fragment| PromptFragment::developer_capability(fragment.render())) - .into_iter() - .collect() - }) - } -} - impl ToolContributor for SkillsExtension where C: Send + Sync + 'static, @@ -153,224 +84,24 @@ where let Some(thread_state) = thread_store.get::() else { return Vec::new(); }; - if !self.providers.has_orchestrator_provider() - || !thread_state.orchestrator_skills_enabled() - { + if !thread_state.orchestrator_skills_enabled() { return Vec::new(); } - skill_tools( - self.providers.clone(), - session_store.get::(), - thread_state, - ) - } -} - -impl TurnInputContributor for SkillsExtension -where - C: Send + Sync + 'static, -{ - fn contribute<'a>( - &'a self, - input: TurnInputContext, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, - turn_store: &'a ExtensionData, - ) -> ExtensionFuture<'a, Vec>> { - Box::pin(async move { - let Some(thread_state) = thread_store.get::() else { - return Vec::new(); - }; - - let config = thread_state.config(); - let host_snapshot = turn_store.get::(); - let query = SkillListQuery { - turn_id: input.turn_id.clone(), - executor_roots: thread_state.selected_roots().to_vec(), - host_snapshot: host_snapshot.clone(), - include_host_skills: true, - include_bundled_skills: config.bundled_skills_enabled, - include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), - mcp_resources: session_store.get::(), - }; - let catalog = self.list_skills(query, &thread_state).await; - for warning in &catalog.warnings { - self.emit_warning(&input.turn_id, warning.clone()); - } - - let selected_entries = collect_explicit_skill_mentions(&input.user_input, &catalog); - let mut fragments: Vec> = Vec::new(); - if config.include_instructions { - let mut turn_catalog = catalog.clone(); - turn_catalog.entries.retain(|entry| { - entry.authority.kind != SkillSourceKind::Executor - && entry.authority.kind != SkillSourceKind::Orchestrator - }); - if let Some(fragment) = available_skills_fragment(&turn_catalog) { - fragments.push(Box::new(fragment)); - } - } - - let mut warnings = catalog.warnings.clone(); - let mut main_prompts_injected = false; - let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default(); - for entry in &selected_entries { - match self - .read_main_prompt(entry, host_snapshot.clone(), session_store, &thread_state) - .await - { - Ok(read_result) => { - let (contents, truncated) = - truncate_main_prompt_contents(read_result.contents.as_str()); - if truncated { - let warning = format!( - "Skill `{}` exceeded the main prompt context limit and was truncated.", - entry.name - ); - self.emit_warning(&input.turn_id, warning.clone()); - warnings.push(warning); - } - let fragment = SkillInstructions { - name: truncate_utf8_to_bytes(&entry.name, MAX_SKILL_NAME_BYTES).0, - path: truncate_utf8_to_bytes( - entry.rendered_path(), - MAX_SKILL_PATH_BYTES, - ) - .0, - contents, - }; - fragments.push(Box::new(fragment)); - main_prompts_injected = true; - if entry.authority.kind == SkillSourceKind::Host { - injected_host_skill_prompts.insert_path(entry.main_prompt.as_str()); - } - } - Err(message) => { - let warning = format!("Failed to load skill `{}`: {message}", entry.name); - self.emit_warning(&input.turn_id, warning.clone()); - warnings.push(warning); - } - } - } - - if let Some(host_snapshot) = &host_snapshot { - for entry in selected_entries - .iter() - .filter(|entry| entry.authority.kind != SkillSourceKind::Host) - { - for host_skill in host_snapshot - .outcome() - .skills - .iter() - .filter(|host_skill| host_skill.name == entry.name) - { - injected_host_skill_prompts - .insert_path(host_skill.path_to_skills_md.to_string_lossy()); - } - } - } - - turn_store.insert(SkillsTurnState { - catalog, - selected_entries, - warnings, - main_prompts_injected, - }); - if !injected_host_skill_prompts.is_empty() { - turn_store.insert(injected_host_skill_prompts); - } - - fragments - }) - } -} - -impl SkillsExtension { - #[tracing::instrument(level = "trace", skip_all)] - async fn list_skills( - &self, - mut query: SkillListQuery, - thread_state: &SkillsThreadState, - ) -> SkillCatalog { - let include_orchestrator_skills = query.include_orchestrator_skills; - let orchestrator_query = query.clone(); - let mcp_resources = orchestrator_query.mcp_resources.clone(); - query.include_orchestrator_skills = false; - - let mut catalog = self.providers.list_for_turn(query).await; - if include_orchestrator_skills { - let orchestrator_catalog = thread_state - .orchestrator_catalog_snapshot( - mcp_resources.as_deref(), - self.providers - .list_orchestrator_for_turn(orchestrator_query), - ) - .await; - catalog.extend(orchestrator_catalog); - } - catalog - } - - #[tracing::instrument(level = "trace", skip_all, fields(skill = %entry.name))] - async fn read_main_prompt( - &self, - entry: &SkillCatalogEntry, - host_snapshot: Option>, - session_store: &ExtensionData, - thread_state: &SkillsThreadState, - ) -> Result { - thread_state - .read_skill( - &self.providers, - SkillReadRequest { - authority: entry.authority.clone(), - package: entry.id.clone(), - resource: entry.main_prompt.clone(), - host_snapshot, - mcp_resources: session_store.get::(), - }, - ) - .await - .map_err(|err| err.message) - } - - fn emit_warning(&self, turn_id: &str, message: String) { - self.event_sink.emit(Event { - id: turn_id.to_string(), - msg: EventMsg::Warning(WarningEvent { message }), - }); + skill_tools(session_store.get::(), thread_state) } } pub fn install( registry: &mut ExtensionRegistryBuilder, - config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static, -) where - C: Send + Sync + 'static, -{ - install_with_providers( - registry, - SkillProviders::new().with_host_provider(Arc::new(HostSkillProvider::new())), - config_from_host, - ); -} - -pub fn install_with_providers( - registry: &mut ExtensionRegistryBuilder, - providers: SkillProviders, - config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static, + orchestrator_skills_enabled: impl Fn(&C) -> bool + Send + Sync + 'static, ) where C: Send + Sync + 'static, { let extension = Arc::new(SkillsExtension { - providers, - event_sink: registry.event_sink(), - config_from_host: Arc::new(config_from_host), + orchestrator_skills_enabled: Arc::new(orchestrator_skills_enabled), }); registry.thread_lifecycle_contributor(extension.clone()); registry.config_contributor(extension.clone()); - registry.prompt_contributor(extension.clone()); - registry.turn_input_contributor(extension.clone()); registry.tool_contributor(extension); } diff --git a/codex-rs/ext/skills/src/fragments.rs b/codex-rs/ext/skills/src/fragments.rs deleted file mode 100644 index 0a24b48b5a35..000000000000 --- a/codex-rs/ext/skills/src/fragments.rs +++ /dev/null @@ -1,61 +0,0 @@ -use codex_core_skills::render_available_skills_body; -use codex_extension_api::ContextualUserFragment; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; - -#[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) struct AvailableSkillsInstructions { - skill_lines: Vec, -} - -impl AvailableSkillsInstructions { - pub(crate) fn from_skill_lines(skill_lines: Vec) -> Self { - Self { skill_lines } - } -} - -impl ContextualUserFragment for AvailableSkillsInstructions { - fn role(&self) -> &'static str { - "developer" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG) - } - - fn body(&self) -> String { - render_available_skills_body(&[], &self.skill_lines) - } -} - -#[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) struct SkillInstructions { - pub(crate) name: String, - pub(crate) path: String, - pub(crate) contents: String, -} - -impl ContextualUserFragment for SkillInstructions { - fn role(&self) -> &'static str { - "user" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - ("", "") - } - - fn body(&self) -> String { - let name = &self.name; - let path = &self.path; - let contents = &self.contents; - format!("\n{name}\n{path}\n{contents}\n") - } -} diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 8abdf6a93e67..833562e9b705 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -1,20 +1,7 @@ -pub mod catalog; -mod config; mod extension; -mod fragments; -pub mod provider; mod render; -mod selection; mod sources; mod state; mod tools; -pub use config::SkillsExtensionConfig; pub use extension::install; -pub use extension::install_with_providers; -pub use provider::ExecutorSkillProvider; -pub use provider::HostSkillProvider; -pub use provider::OrchestratorSkillProvider; -pub use provider::SkillProvider; -pub use sources::SkillProviderSource; -pub use sources::SkillProviders; diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs deleted file mode 100644 index 405592e93478..000000000000 --- a/codex-rs/ext/skills/src/provider.rs +++ /dev/null @@ -1,66 +0,0 @@ -use std::future::Future; -use std::pin::Pin; -use std::sync::Arc; - -mod executor; -mod host; -mod orchestrator; - -use codex_core_skills::HostSkillsSnapshot; -use codex_mcp::McpResourceClient; -use codex_protocol::capabilities::SelectedCapabilityRoot; - -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillProviderResult; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillResourceId; -use crate::catalog::SkillSearchResult; - -pub use executor::ExecutorSkillProvider; -pub use host::HostSkillProvider; -pub use orchestrator::OrchestratorSkillProvider; - -#[derive(Clone, Debug)] -pub struct SkillListQuery { - pub turn_id: String, - pub executor_roots: Vec, - pub host_snapshot: Option>, - pub include_host_skills: bool, - pub include_bundled_skills: bool, - pub include_orchestrator_skills: bool, - pub mcp_resources: Option>, -} - -#[derive(Clone, Debug)] -pub struct SkillReadRequest { - pub authority: SkillAuthority, - pub package: SkillPackageId, - pub resource: SkillResourceId, - pub host_snapshot: Option>, - pub mcp_resources: Option>, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct SkillSearchRequest { - pub authority: SkillAuthority, - pub package: SkillPackageId, - pub query: String, -} - -pub type SkillProviderFuture<'a, T> = - Pin> + Send + 'a>>; - -/// Source-specific skill catalog and resource access. -/// -/// Implementations must preserve authority boundaries: a resource listed by a -/// provider must be read or searched through the same provider/authority rather -/// than converted into an ambient local path. -pub trait SkillProvider: Send + Sync { - fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog>; - - fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult>; - - fn search(&self, request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult>; -} diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs deleted file mode 100644 index 9938035e9228..000000000000 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ /dev/null @@ -1,166 +0,0 @@ -use std::sync::Arc; - -use codex_core_skills::loader::EnvironmentSkillMetadata; -use codex_core_skills::loader::load_environment_skills_from_root; -use codex_exec_server::EnvironmentManager; -use codex_protocol::capabilities::CapabilityRootLocation; -use codex_protocol::protocol::Product; -use codex_utils_path_uri::PathConvention; - -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillProviderError; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillResourceId; -use crate::catalog::SkillSearchResult; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillListQuery; -use crate::provider::SkillProvider; -use crate::provider::SkillProviderFuture; -use crate::provider::SkillReadRequest; -use crate::provider::SkillSearchRequest; - -/// Discovers and reads skills through the filesystem owned by an execution environment. -#[derive(Clone, Debug)] -pub struct ExecutorSkillProvider { - environment_manager: Arc, - restriction_product: Option, -} - -impl ExecutorSkillProvider { - pub fn new_with_restriction_product( - environment_manager: Arc, - restriction_product: Option, - ) -> Self { - Self { - environment_manager, - restriction_product, - } - } -} - -impl SkillProvider for ExecutorSkillProvider { - fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { - Box::pin(async move { - let mut catalog = SkillCatalog::default(); - for selected_root in query.executor_roots { - let selected_root_id = selected_root.id; - let CapabilityRootLocation::Environment { - environment_id, - path, - } = selected_root.location; - let authority = - SkillAuthority::new(SkillSourceKind::Executor, selected_root_id.clone()); - let Some(environment) = self.environment_manager.get_environment(&environment_id) - else { - catalog.warnings.push(format!( - "Selected capability root `{selected_root_id}` references unavailable environment `{environment_id}`." - )); - continue; - }; - let file_system = environment.get_filesystem(); - let outcome = load_environment_skills_from_root( - file_system.as_ref(), - &path, - self.restriction_product, - ) - .await; - catalog.warnings.extend(outcome.warnings); - for skill in outcome.skills { - catalog.push_entry(catalog_entry_from_skill( - &skill, - authority.clone(), - &selected_root_id, - &environment_id, - )); - } - } - - Ok(catalog) - }) - } - - fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { - Box::pin(async move { - if request.authority.kind != SkillSourceKind::Executor { - return Err(SkillProviderError::new(format!( - "executor skill provider cannot read {} resources", - request.authority.kind - ))); - } - if request.package.0 != request.resource.as_str() { - return Err(SkillProviderError::new( - "executor skill resource does not match its package", - )); - } - let Some((environment_id, resource_path)) = request.resource.environment_path() else { - return Err(SkillProviderError::new( - "executor skill resource is not bound to an environment", - )); - }; - let Some(environment) = self.environment_manager.get_environment(environment_id) else { - return Err(SkillProviderError::new(format!( - "executor skill resource references unavailable environment `{environment_id}`" - ))); - }; - let contents = environment - .get_filesystem() - .read_file_text(resource_path, /*sandbox*/ None) - .await - .map_err(|err| { - SkillProviderError::new(format!( - "failed to read executor skill resource {}: {err}", - request.resource.as_str() - )) - })?; - - Ok(SkillReadResult { - resource: request.resource, - contents, - }) - }) - } - - fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> { - Box::pin(async { Ok(SkillSearchResult::default()) }) - } -} - -fn catalog_entry_from_skill( - skill: &EnvironmentSkillMetadata, - authority: SkillAuthority, - selected_root_id: &str, - environment_id: &str, -) -> SkillCatalogEntry { - let skill_path = skill.path_to_skills_md.inferred_native_path_string(); - let normalized_path = match skill.path_to_skills_md.infer_path_convention() { - Some(PathConvention::Windows) => skill_path.replace('\\', "/"), - Some(PathConvention::Posix) | None => skill_path, - }; - let display_path = format!( - "skill://{selected_root_id}/{}", - normalized_path.trim_start_matches('/') - ); - let entry = SkillCatalogEntry::new( - SkillPackageId(display_path.clone()), - authority, - skill.name.clone(), - skill.description.clone(), - SkillResourceId::environment( - display_path.clone(), - environment_id, - skill.path_to_skills_md.clone(), - ), - ) - .with_short_description(skill.short_description.clone()) - .with_display_path(display_path) - .with_dependencies(skill.dependencies.clone()); - - if skill.allows_implicit_invocation() { - entry - } else { - entry.hidden_from_prompt() - } -} diff --git a/codex-rs/ext/skills/src/provider/host.rs b/codex-rs/ext/skills/src/provider/host.rs deleted file mode 100644 index 2cee694216dc..000000000000 --- a/codex-rs/ext/skills/src/provider/host.rs +++ /dev/null @@ -1,129 +0,0 @@ -use codex_core_skills::SkillLoadOutcome; -use codex_core_skills::SkillMetadata; - -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillProviderError; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillResourceId; -use crate::catalog::SkillSearchResult; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillListQuery; -use crate::provider::SkillProvider; -use crate::provider::SkillProviderFuture; -use crate::provider::SkillReadRequest; -use crate::provider::SkillSearchRequest; - -const HOST_AUTHORITY_ID: &str = "host"; - -/// Host-owned skill provider backed by an immutable service snapshot. -/// -/// Discovery and caching belong to `SkillsService`; this provider only maps a -/// snapshot into the authority-aware catalog/read contract. -#[derive(Clone, Default)] -pub struct HostSkillProvider; - -impl HostSkillProvider { - pub fn new() -> Self { - Self - } -} - -impl SkillProvider for HostSkillProvider { - fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { - Box::pin(async move { - let Some(host_snapshot) = query.host_snapshot else { - return Err(SkillProviderError::new( - "host skill provider requires a host skills snapshot", - )); - }; - - Ok(catalog_from_outcome(host_snapshot.outcome())) - }) - } - - fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { - Box::pin(async move { - let Some(host_snapshot) = request.host_snapshot else { - return Err(SkillProviderError::new( - "host skill provider requires a host skills snapshot", - )); - }; - let Some(skill) = host_snapshot.outcome().skills.iter().find(|skill| { - let skill_path = skill.path_to_skills_md.to_string_lossy(); - skill_path == request.resource.as_str() - || skill_path.replace('\\', "/") == request.resource.as_str() - }) else { - return Err(SkillProviderError::new(format!( - "host skill resource is not loaded: {}", - request.resource.as_str() - ))); - }; - - let contents = host_snapshot.read_skill_text(skill).await.map_err(|err| { - SkillProviderError::new(format!( - "failed to read host skill resource {}: {err}", - request.resource.as_str() - )) - })?; - - Ok(SkillReadResult { - resource: request.resource, - contents, - }) - }) - } - - fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> { - Box::pin(async { Ok(SkillSearchResult::default()) }) - } -} - -fn catalog_from_outcome(outcome: &SkillLoadOutcome) -> SkillCatalog { - let mut catalog = SkillCatalog { - entries: Vec::new(), - warnings: outcome - .errors - .iter() - .map(|err| { - format!( - "Failed to load skill at {}: {}", - err.path.display(), - err.message - ) - }) - .collect(), - }; - - for (skill, enabled) in outcome.skills_with_enabled() { - catalog.push_entry(catalog_entry_from_skill(skill, enabled)); - } - - catalog -} - -fn catalog_entry_from_skill(skill: &SkillMetadata, enabled: bool) -> SkillCatalogEntry { - let skill_path = skill.path_to_skills_md.to_string_lossy().into_owned(); - let display_path = skill_path.replace('\\', "/"); - let mut entry = SkillCatalogEntry::new( - SkillPackageId(skill_path.clone()), - SkillAuthority::new(SkillSourceKind::Host, HOST_AUTHORITY_ID), - skill.name.clone(), - skill.description.clone(), - SkillResourceId::new(skill_path), - ) - .with_short_description(skill.short_description.clone()) - .with_display_path(display_path) - .with_dependencies(skill.dependencies.clone()); - - if !enabled { - entry = entry.disabled(); - } - if !skill.allows_implicit_invocation() { - entry = entry.hidden_from_prompt(); - } - - entry -} diff --git a/codex-rs/ext/skills/src/provider/orchestrator.rs b/codex-rs/ext/skills/src/provider/orchestrator.rs deleted file mode 100644 index 8d97bf32a1da..000000000000 --- a/codex-rs/ext/skills/src/provider/orchestrator.rs +++ /dev/null @@ -1,331 +0,0 @@ -use std::collections::HashSet; -use std::time::Duration; - -use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; -use codex_protocol::mcp::Resource; -use codex_protocol::mcp::ResourceContent; -use url::Url; - -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillProviderError; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillResourceId; -use crate::catalog::SkillSearchResult; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillListQuery; -use crate::provider::SkillProvider; -use crate::provider::SkillProviderFuture; -use crate::provider::SkillReadRequest; -use crate::provider::SkillSearchRequest; - -const ORCHESTRATOR_SKILL_MIME_TYPE: &str = "mcp/skill"; -const ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT: Duration = Duration::from_secs(10); -const ORCHESTRATOR_SKILL_READ_TIMEOUT: Duration = Duration::from_secs(10); -const MAX_RESOURCE_PAGES: usize = 10; -const MAX_ORCHESTRATOR_SKILLS: usize = 100; -const MAX_SKILL_NAME_CHARS: usize = 64; -const MAX_QUALIFIED_SKILL_NAME_CHARS: usize = 128; -const MAX_SKILL_PACKAGE_URI_CHARS: usize = 1_024; -const MAX_SKILL_RESOURCE_URI_CHARS: usize = 2_048; -const MAX_SKILL_RESOURCE_CONTENT_BYTES: usize = 1024 * 1024; - -/// Discovers and reads skills owned by the orchestrator. -/// -/// The provider uses session-scoped resources without exposing the transport or -/// resource server to callers that configure the skills extension. -#[derive(Clone, Debug, Default)] -pub struct OrchestratorSkillProvider; - -impl OrchestratorSkillProvider { - pub fn new() -> Self { - Self - } -} - -impl SkillProvider for OrchestratorSkillProvider { - fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { - Box::pin(async move { - let Some(client) = query.mcp_resources else { - return Ok(SkillCatalog::default()); - }; - if !client.has_server(CODEX_APPS_MCP_SERVER_NAME).await { - return Ok(SkillCatalog::default()); - } - - let discovery_deadline = - tokio::time::Instant::now() + ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT; - let mut catalog = SkillCatalog::default(); - let mut cursor = None; - let mut seen_cursors = HashSet::new(); - let mut skill_resources_seen = 0usize; - let mut skipped_resources = 0usize; - let mut truncated = false; - let mut completed_pages = 0usize; - - for _ in 0..MAX_RESOURCE_PAGES { - let page = match tokio::time::timeout_at( - discovery_deadline, - client.list_resources(CODEX_APPS_MCP_SERVER_NAME, cursor.clone()), - ) - .await - { - Ok(result) => result.map_err(|err| { - SkillProviderError::new(format!( - "failed to list orchestrator skill resources: {err:#}" - )) - }), - Err(_) => Err(SkillProviderError::new(format!( - "orchestrator skill discovery timed out after {ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT:?}" - ))), - }; - let result = match page { - Ok(result) => result, - Err(err) if completed_pages == 0 => return Err(err), - Err(err) => { - let page_word = if completed_pages == 1 { - "page" - } else { - "pages" - }; - catalog.warnings.push(format!( - "Orchestrator skill discovery stopped after {completed_pages} resource {page_word}: {}", - err.message - )); - cursor = None; - break; - } - }; - completed_pages = completed_pages.saturating_add(1); - - for resource in &result.resources { - if resource.mime_type.as_deref() != Some(ORCHESTRATOR_SKILL_MIME_TYPE) { - continue; - } - if skill_resources_seen >= MAX_ORCHESTRATOR_SKILLS { - truncated = true; - break; - } - skill_resources_seen = skill_resources_seen.saturating_add(1); - match catalog_entry_from_resource(resource) { - Some(entry) => catalog.push_entry(entry), - None => skipped_resources = skipped_resources.saturating_add(1), - } - } - - if truncated { - break; - } - let Some(next_cursor) = result.next_cursor else { - cursor = None; - break; - }; - if !seen_cursors.insert(next_cursor.clone()) { - catalog.warnings.push( - "Orchestrator skill resource pagination returned a duplicate cursor." - .to_string(), - ); - cursor = None; - break; - } - cursor = Some(next_cursor); - } - - if cursor.is_some() || truncated { - catalog.warnings.push(format!( - "Orchestrator skill discovery was truncated at {MAX_ORCHESTRATOR_SKILLS} skills or {MAX_RESOURCE_PAGES} resource pages." - )); - } - if skipped_resources > 0 { - catalog.warnings.push(format!( - "Skipped {skipped_resources} malformed orchestrator skill resources." - )); - } - - Ok(catalog) - }) - } - - fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { - Box::pin(async move { - if request.authority - != SkillAuthority::new(SkillSourceKind::Orchestrator, CODEX_APPS_MCP_SERVER_NAME) - { - return Err(SkillProviderError::new(format!( - "orchestrator skill provider cannot read authority {}", - request.authority.id - ))); - } - if !resource_belongs_to_package(&request.package.0, request.resource.as_str()) { - return Err(SkillProviderError::new( - "orchestrator skill resource does not match its package", - )); - } - - let Some(client) = request.mcp_resources.as_ref() else { - return Err(SkillProviderError::new( - "session MCP resource client is not configured", - )); - }; - let result = tokio::time::timeout( - ORCHESTRATOR_SKILL_READ_TIMEOUT, - client.read_resource(CODEX_APPS_MCP_SERVER_NAME, request.resource.as_str()), - ) - .await - .map_err(|_| { - SkillProviderError::new(format!( - "orchestrator skill read timed out after {ORCHESTRATOR_SKILL_READ_TIMEOUT:?}" - )) - })? - .map_err(|err| { - SkillProviderError::new(format!( - "failed to read orchestrator skill resource {}: {err:#}", - request.resource.as_str() - )) - })?; - let contents = result - .contents - .into_iter() - .find_map(|contents| match contents { - ResourceContent::Text { uri, text, .. } if uri == request.resource.as_str() => { - Some(text) - } - ResourceContent::Text { .. } | ResourceContent::Blob { .. } => None, - }); - let Some(contents) = contents else { - return Err(SkillProviderError::new(format!( - "orchestrator skill resource {} did not return matching text contents", - request.resource.as_str() - ))); - }; - if contents.len() > MAX_SKILL_RESOURCE_CONTENT_BYTES { - return Err(SkillProviderError::new(format!( - "orchestrator skill resource {} exceeds the {MAX_SKILL_RESOURCE_CONTENT_BYTES}-byte read limit", - request.resource.as_str() - ))); - } - - Ok(SkillReadResult { - resource: request.resource, - contents, - }) - }) - } - - fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> { - Box::pin(async { Ok(SkillSearchResult::default()) }) - } -} - -fn catalog_entry_from_resource(resource: &Resource) -> Option { - let uri = validated_skill_uri(resource.uri.as_str(), MAX_SKILL_PACKAGE_URI_CHARS)?; - let meta = resource.meta.as_ref()?.as_object()?; - let skill_name = normalized_label(meta.get("skill_name")?.as_str()?, MAX_SKILL_NAME_CHARS)?; - let name = if meta.get("source").and_then(|value| value.as_str()) == Some("user") { - skill_name - } else { - let plugin_name = - normalized_label(meta.get("plugin_name")?.as_str()?, MAX_SKILL_NAME_CHARS)?; - let qualified_name = format!("{plugin_name}:{skill_name}"); - (qualified_name.chars().count() <= MAX_QUALIFIED_SKILL_NAME_CHARS) - .then_some(qualified_name)? - }; - let description = normalized_description(resource.description.as_deref().unwrap_or_default())?; - let main_prompt = main_prompt_uri(uri); - - Some( - SkillCatalogEntry::new( - SkillPackageId(uri.to_string()), - SkillAuthority::new(SkillSourceKind::Orchestrator, CODEX_APPS_MCP_SERVER_NAME), - name, - description, - SkillResourceId::new(main_prompt), - ) - .with_display_path(uri), - ) -} - -fn validated_skill_uri(uri: &str, max_chars: usize) -> Option<&str> { - validated_skill_url(uri, max_chars).map(|_| uri) -} - -fn validated_skill_url(uri: &str, max_chars: usize) -> Option { - if uri.chars().count() > max_chars - || uri - .chars() - .any(|ch| ch.is_control() || ch.is_whitespace() || matches!(ch, '<' | '>')) - { - return None; - } - - let url = Url::parse(uri).ok()?; - let path_is_valid = url.path_segments().is_some_and(|segments| { - let segments = segments.collect::>(); - !segments.is_empty() && segments.iter().all(|segment| !segment.is_empty()) - }); - (url.scheme() == "skill" - && url.as_str() == uri - && url.host_str().is_some_and(|host| !host.is_empty()) - && url.username().is_empty() - && url.password().is_none() - && url.port().is_none() - && url.query().is_none() - && url.fragment().is_none() - && path_is_valid) - .then_some(url) -} - -fn resource_belongs_to_package(package: &str, resource: &str) -> bool { - let Some(package) = validated_skill_url(package, MAX_SKILL_PACKAGE_URI_CHARS) else { - return false; - }; - let Some(resource) = validated_skill_url(resource, MAX_SKILL_RESOURCE_URI_CHARS) else { - return false; - }; - - let Some(package_segments) = package.path_segments() else { - return false; - }; - let Some(resource_segments) = resource.path_segments() else { - return false; - }; - let package_segments = package_segments.collect::>(); - let resource_segments = resource_segments.collect::>(); - - package.scheme() == resource.scheme() - && package.host_str() == resource.host_str() - && resource_segments.len() > package_segments.len() - && resource_segments.starts_with(&package_segments) -} - -fn normalized_label(value: &str, max_chars: usize) -> Option { - let value = normalized_single_line(value, max_chars)?; - let invalid = value.is_empty() || value.chars().any(|ch| matches!(ch, '&' | '<' | '>')); - (!invalid).then_some(value) -} - -fn normalized_description(value: &str) -> Option { - let value = value.split_whitespace().collect::>().join(" "); - if value.chars().any(char::is_control) { - return None; - } - - Some( - value - .replace('&', "&") - .replace('<', "<") - .replace('>', ">"), - ) -} - -fn normalized_single_line(value: &str, max_chars: usize) -> Option { - let value = value.split_whitespace().collect::>().join(" "); - let valid = value.chars().count() <= max_chars && !value.chars().any(char::is_control); - valid.then_some(value) -} - -fn main_prompt_uri(package_uri: &str) -> String { - format!("{}/SKILL.md", package_uri.trim_end_matches('/')) -} diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index dda32b486409..ebd3c960450e 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -2,62 +2,8 @@ use std::borrow::Cow; use codex_utils_string::take_bytes_at_char_boundary; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillSourceKind; -use crate::fragments::AvailableSkillsInstructions; - -const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; -const MAX_MAIN_PROMPT_BYTES: usize = 8_000; const MAX_CATALOG_SKILL_DESCRIPTION_CHARS: usize = 1_024; const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; -pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; -pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; - -#[tracing::instrument( - level = "trace", - skip_all, - fields(catalog_entry_count = catalog.entries.len()) -)] -pub(crate) fn available_skills_fragment( - catalog: &SkillCatalog, -) -> Option { - let mut total_bytes = 0usize; - let mut omitted = 0usize; - let mut skill_lines = Vec::new(); - - for entry in catalog - .entries - .iter() - .filter(|entry| entry.enabled && entry.prompt_visible) - { - let description = entry - .short_description - .as_deref() - .unwrap_or(entry.description.as_str()); - let description = truncate_catalog_skill_description(description); - let line = render_skill_line(entry, description.as_ref()); - let next_bytes = total_bytes.saturating_add(line.len()); - if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { - omitted = omitted.saturating_add(1); - continue; - } - total_bytes = next_bytes; - skill_lines.push(line); - } - - if skill_lines.is_empty() { - return None; - } - if omitted > 0 { - let skill_word = if omitted == 1 { "skill" } else { "skills" }; - skill_lines.push(format!( - "- {omitted} additional {skill_word} omitted from this bounded skills list." - )); - } - - Some(AvailableSkillsInstructions::from_skill_lines(skill_lines)) -} pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, str> { if description @@ -79,26 +25,6 @@ pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, s Cow::Owned(truncated) } -fn render_skill_line(entry: &SkillCatalogEntry, description: &str) -> String { - let locator_kind = match &entry.authority.kind { - SkillSourceKind::Host => "file", - SkillSourceKind::Executor => "environment resource", - SkillSourceKind::Orchestrator => "orchestrator resource", - SkillSourceKind::Custom(_) => "custom resource", - }; - let name = entry.name.as_str(); - let path = entry.rendered_path(); - if description.is_empty() { - format!("- {name}: ({locator_kind}: {path})") - } else { - format!("- {name}: {description} ({locator_kind}: {path})") - } -} - -pub(crate) fn truncate_main_prompt_contents(contents: &str) -> (String, bool) { - truncate_utf8_to_bytes(contents, MAX_MAIN_PROMPT_BYTES) -} - pub(crate) fn truncate_utf8_to_bytes(contents: &str, max_bytes: usize) -> (String, bool) { let truncated = take_bytes_at_char_boundary(contents, max_bytes); (truncated.to_string(), truncated.len() < contents.len()) diff --git a/codex-rs/ext/skills/src/selection.rs b/codex-rs/ext/skills/src/selection.rs deleted file mode 100644 index a708b18d6c42..000000000000 --- a/codex-rs/ext/skills/src/selection.rs +++ /dev/null @@ -1,137 +0,0 @@ -use std::collections::HashSet; - -use codex_core_skills::injection::extract_tool_mentions; -use codex_protocol::user_input::UserInput; - -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillPackageId; - -const SKILL_PATH_PREFIX: &str = "skill://"; - -#[tracing::instrument( - level = "trace", - skip_all, - fields( - input_count = inputs.len(), - catalog_entry_count = catalog.entries.len() - ) -)] -pub(crate) fn collect_explicit_skill_mentions( - inputs: &[UserInput], - catalog: &SkillCatalog, -) -> Vec { - let mut selected = Vec::new(); - let mut seen = HashSet::new(); - let mut blocked_plain_names = HashSet::new(); - - for input in inputs { - match input { - UserInput::Skill { name, path } => { - blocked_plain_names.insert(name.clone()); - select_by_path(catalog, &path.to_string_lossy(), &mut seen, &mut selected); - } - UserInput::Mention { name, path } if path_is_skill(path) => { - blocked_plain_names.insert(name.clone()); - select_by_path(catalog, path, &mut seen, &mut selected); - } - UserInput::Text { .. } | UserInput::Image { .. } | UserInput::LocalImage { .. } => {} - UserInput::Mention { .. } => {} - _ => {} - } - } - - for input in inputs { - let UserInput::Text { text, .. } = input else { - continue; - }; - - let mentions = extract_tool_mentions(text); - for path in mentions.paths() { - if path_is_skill(path) { - select_by_path( - catalog, - normalize_skill_path(path), - &mut seen, - &mut selected, - ); - } - } - for name in mentions.plain_names() { - if blocked_plain_names.contains(name) { - continue; - } - if let Some(entry) = catalog - .entries - .iter() - .find(|entry| entry.enabled && entry.name == name) - { - push_selected(entry, &mut seen, &mut selected); - } - } - } - - selected -} - -fn select_by_path( - catalog: &SkillCatalog, - path: &str, - seen: &mut HashSet, - selected: &mut Vec, -) { - let normalized_path = normalize_skill_path(path); - for entry in catalog.entries.iter().filter(|entry| entry.enabled) { - if entry_matches_path(entry, normalized_path) { - push_selected(entry, seen, selected); - } - } -} - -fn push_selected( - entry: &SkillCatalogEntry, - seen: &mut HashSet, - selected: &mut Vec, -) { - let key = SkillCatalogEntryKey::from(entry); - if seen.insert(key) { - selected.push(entry.clone()); - } -} - -fn entry_matches_path(entry: &SkillCatalogEntry, path: &str) -> bool { - entry.main_prompt.as_str() == path - || entry.id.0 == path - || entry - .display_path - .as_deref() - .is_some_and(|display_path| normalize_skill_path(display_path) == path) -} - -fn path_is_skill(path: &str) -> bool { - path.starts_with(SKILL_PATH_PREFIX) - || path - .rsplit(['/', '\\']) - .next() - .is_some_and(|file_name| file_name.eq_ignore_ascii_case("SKILL.md")) -} - -fn normalize_skill_path(path: &str) -> &str { - path.strip_prefix(SKILL_PATH_PREFIX).unwrap_or(path) -} - -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -struct SkillCatalogEntryKey { - authority: SkillAuthority, - package: SkillPackageId, -} - -impl From<&SkillCatalogEntry> for SkillCatalogEntryKey { - fn from(entry: &SkillCatalogEntry) -> Self { - Self { - authority: entry.authority.clone(), - package: entry.id.clone(), - } - } -} diff --git a/codex-rs/ext/skills/src/sources.rs b/codex-rs/ext/skills/src/sources.rs index d558c13b3ba3..cd9853e5f60f 100644 --- a/codex-rs/ext/skills/src/sources.rs +++ b/codex-rs/ext/skills/src/sources.rs @@ -1,218 +1,385 @@ -use std::fmt; +use std::collections::HashSet; use std::sync::Arc; +use std::time::Duration; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillProviderError; -use crate::catalog::SkillProviderResult; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillSearchResult; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillListQuery; -use crate::provider::SkillProvider; -use crate::provider::SkillReadRequest; -use crate::provider::SkillSearchRequest; - -#[derive(Clone)] -pub struct SkillProviderSource { - kind: SkillSourceKind, - label: String, - provider: Arc, +use codex_core_skills::runtime::SkillAuthority; +use codex_core_skills::runtime::SkillCatalog; +use codex_core_skills::runtime::SkillCatalogEntry; +use codex_core_skills::runtime::SkillPackageId; +use codex_core_skills::runtime::SkillReadRequest; +use codex_core_skills::runtime::SkillReadResult; +use codex_core_skills::runtime::SkillResourceId; +use codex_core_skills::runtime::SkillSource; +use codex_core_skills::runtime::SkillSourceError; +use codex_core_skills::runtime::SkillSourceFuture; +use codex_core_skills::runtime::SkillSourceIdentity; +use codex_core_skills::runtime::SkillSourceKind; +use codex_core_skills::runtime::SkillSourceResult; +use codex_core_skills::runtime::SkillSources; +use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::McpResourceClient; +use codex_protocol::mcp::Resource; +use codex_protocol::mcp::ResourceContent; +use url::Url; + +use crate::state::SkillsThreadState; + +const ORCHESTRATOR_SKILL_MIME_TYPE: &str = "mcp/skill"; +const ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT: Duration = Duration::from_secs(10); +const ORCHESTRATOR_SKILL_READ_TIMEOUT: Duration = Duration::from_secs(10); +const MAX_RESOURCE_PAGES: usize = 10; +const MAX_ORCHESTRATOR_SKILLS: usize = 100; +const MAX_SKILL_NAME_CHARS: usize = 64; +const MAX_QUALIFIED_SKILL_NAME_CHARS: usize = 128; +const MAX_SKILL_PACKAGE_URI_CHARS: usize = 1_024; +const MAX_SKILL_RESOURCE_URI_CHARS: usize = 2_048; +const MAX_SKILL_RESOURCE_CONTENT_BYTES: usize = 1024 * 1024; + +pub(crate) fn orchestrator_skill_sources( + thread_state: Arc, + mcp_resources: Option>, +) -> SkillSources { + SkillSources::new().with_source_factory( + "orchestrator", + Arc::new(move || { + orchestrator_skill_source(Arc::clone(&thread_state), mcp_resources.clone()) + }), + ) } -impl SkillProviderSource { - pub fn new( - kind: SkillSourceKind, - label: impl Into, - provider: Arc, - ) -> Self { - Self { - kind, - label: label.into(), - provider, - } +pub(crate) fn orchestrator_skill_source( + thread_state: Arc, + mcp_resources: Option>, +) -> Arc { + // Bind listing and reads to the manager generation current when this source is created. + let mcp_resources = mcp_resources.map(|client| Arc::new(client.snapshot())); + let identity = mcp_resources + .as_ref() + .map(|client| SkillSourceIdentity::from_owner(client.manager_snapshot())) + .unwrap_or_else(|| SkillSourceIdentity::from_owner(Arc::clone(&thread_state))); + Arc::new(OrchestratorSkillSource { + identity, + thread_state, + mcp_resources, + }) +} + +struct OrchestratorSkillSource { + identity: SkillSourceIdentity, + thread_state: Arc, + mcp_resources: Option>, +} + +impl SkillSource for OrchestratorSkillSource { + fn identity(&self) -> SkillSourceIdentity { + self.identity.clone() } - pub fn host(label: impl Into, provider: Arc) -> Self { - Self::new(SkillSourceKind::Host, label, provider) + fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> { + Box::pin(async move { + if !self.thread_state.orchestrator_skills_enabled() { + return Ok(SkillCatalog::default()); + } + Ok(self + .thread_state + .orchestrator_catalog_snapshot( + self.mcp_resources.as_deref(), + discover_orchestrator_skills(self.mcp_resources.as_deref()), + ) + .await) + }) } - pub fn executor(label: impl Into, provider: Arc) -> Self { - Self::new(SkillSourceKind::Executor, label, provider) + fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> { + Box::pin(async move { + let read_request = request.clone(); + self.thread_state + .read_orchestrator_resource( + &request, + self.mcp_resources.as_deref(), + read_orchestrator_skill(self.mcp_resources.as_deref(), read_request), + ) + .await + }) } +} - pub fn orchestrator(label: impl Into, provider: Arc) -> Self { - Self::new(SkillSourceKind::Orchestrator, label, provider) +async fn discover_orchestrator_skills( + client: Option<&McpResourceClient>, +) -> SkillSourceResult { + let Some(client) = client else { + return Ok(SkillCatalog::default()); + }; + if !client.has_server(CODEX_APPS_MCP_SERVER_NAME).await { + return Ok(SkillCatalog::default()); } - fn should_list(&self, query: &SkillListQuery) -> bool { - match &self.kind { - SkillSourceKind::Host => query.include_host_skills, - SkillSourceKind::Executor => !query.executor_roots.is_empty(), - SkillSourceKind::Orchestrator => query.include_orchestrator_skills, - SkillSourceKind::Custom(_) => true, + let discovery_deadline = tokio::time::Instant::now() + ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT; + let mut catalog = SkillCatalog::default(); + let mut cursor = None; + let mut seen_cursors = HashSet::new(); + let mut skill_resources_seen = 0usize; + let mut skipped_resources = 0usize; + let mut truncated = false; + let mut completed_pages = 0usize; + + for _ in 0..MAX_RESOURCE_PAGES { + let page = match tokio::time::timeout_at( + discovery_deadline, + client.list_resources(CODEX_APPS_MCP_SERVER_NAME, cursor.clone()), + ) + .await + { + Ok(result) => result.map_err(|err| { + SkillSourceError::new(format!( + "failed to list orchestrator skill resources: {err:#}" + )) + }), + Err(_) => Err(SkillSourceError::new(format!( + "orchestrator skill discovery timed out after {ORCHESTRATOR_SKILL_DISCOVERY_TIMEOUT:?}" + ))), + }; + let result = match page { + Ok(result) => result, + Err(err) if completed_pages == 0 => return Err(err), + Err(err) => { + let page_word = if completed_pages == 1 { + "page" + } else { + "pages" + }; + catalog.warnings.push(format!( + "Orchestrator skill discovery stopped after {completed_pages} resource {page_word}: {}", + err.message + )); + cursor = None; + break; + } + }; + completed_pages = completed_pages.saturating_add(1); + + for resource in &result.resources { + if resource.mime_type.as_deref() != Some(ORCHESTRATOR_SKILL_MIME_TYPE) { + continue; + } + if skill_resources_seen >= MAX_ORCHESTRATOR_SKILLS { + truncated = true; + break; + } + skill_resources_seen = skill_resources_seen.saturating_add(1); + match catalog_entry_from_resource(resource) { + Some(entry) => catalog.push_entry(entry), + None => skipped_resources = skipped_resources.saturating_add(1), + } } - } - fn owns_kind(&self, kind: &SkillSourceKind) -> bool { - &self.kind == kind + if truncated { + break; + } + let Some(next_cursor) = result.next_cursor else { + cursor = None; + break; + }; + if !seen_cursors.insert(next_cursor.clone()) { + catalog.warnings.push( + "Orchestrator skill resource pagination returned a duplicate cursor.".to_string(), + ); + cursor = None; + break; + } + cursor = Some(next_cursor); } -} -impl fmt::Debug for SkillProviderSource { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter - .debug_struct("SkillProviderSource") - .field("kind", &self.kind) - .field("label", &self.label) - .finish() + if cursor.is_some() || truncated { + catalog.warnings.push(format!( + "Orchestrator skill discovery was truncated at {MAX_ORCHESTRATOR_SKILLS} skills or {MAX_RESOURCE_PAGES} resource pages." + )); + } + if skipped_resources > 0 { + catalog.warnings.push(format!( + "Skipped {skipped_resources} malformed orchestrator skill resources." + )); } -} -#[derive(Clone, Default, Debug)] -pub struct SkillProviders { - sources: Vec, + Ok(catalog) } -impl SkillProviders { - pub fn new() -> Self { - Self::default() +async fn read_orchestrator_skill( + client: Option<&McpResourceClient>, + request: SkillReadRequest, +) -> SkillSourceResult { + if request.authority != orchestrator_authority() { + return Err(SkillSourceError::new(format!( + "orchestrator skill source cannot read authority {}", + request.authority.id + ))); } - - pub fn with_provider(mut self, source: SkillProviderSource) -> Self { - self.sources.push(source); - self + if !resource_belongs_to_package(&request.package.0, request.resource.as_str()) { + return Err(SkillSourceError::new( + "orchestrator skill resource does not match its package", + )); } - pub fn with_host_provider(mut self, provider: Arc) -> Self { - self.sources - .push(SkillProviderSource::host("host", provider)); - self - } - - pub fn with_executor_provider(mut self, provider: Arc) -> Self { - self.sources - .push(SkillProviderSource::executor("executor", provider)); - self + let Some(client) = client else { + return Err(SkillSourceError::new( + "session MCP resource client is not configured", + )); + }; + let result = tokio::time::timeout( + ORCHESTRATOR_SKILL_READ_TIMEOUT, + client.read_resource(CODEX_APPS_MCP_SERVER_NAME, request.resource.as_str()), + ) + .await + .map_err(|_| { + SkillSourceError::new(format!( + "orchestrator skill read timed out after {ORCHESTRATOR_SKILL_READ_TIMEOUT:?}" + )) + })? + .map_err(|err| { + SkillSourceError::new(format!( + "failed to read orchestrator skill resource {}: {err:#}", + request.resource.as_str() + )) + })?; + let contents = result + .contents + .into_iter() + .find_map(|contents| match contents { + ResourceContent::Text { uri, text, .. } if uri == request.resource.as_str() => { + Some(text) + } + ResourceContent::Text { .. } | ResourceContent::Blob { .. } => None, + }); + let Some(contents) = contents else { + return Err(SkillSourceError::new(format!( + "orchestrator skill resource {} did not return matching text contents", + request.resource.as_str() + ))); + }; + if contents.len() > MAX_SKILL_RESOURCE_CONTENT_BYTES { + return Err(SkillSourceError::new(format!( + "orchestrator skill resource {} exceeds the {MAX_SKILL_RESOURCE_CONTENT_BYTES}-byte read limit", + request.resource.as_str() + ))); } - pub fn with_orchestrator_provider(mut self, provider: Arc) -> Self { - self.sources - .push(SkillProviderSource::orchestrator("orchestrator", provider)); - self - } + Ok(SkillReadResult { + resource: request.resource, + contents, + }) +} - pub(crate) fn has_orchestrator_provider(&self) -> bool { - self.sources - .iter() - .any(|source| source.kind == SkillSourceKind::Orchestrator) - } +fn catalog_entry_from_resource(resource: &Resource) -> Option { + let uri = validated_skill_uri(resource.uri.as_str(), MAX_SKILL_PACKAGE_URI_CHARS)?; + let meta = resource.meta.as_ref()?.as_object()?; + let skill_name = normalized_label(meta.get("skill_name")?.as_str()?, MAX_SKILL_NAME_CHARS)?; + let name = if meta.get("source").and_then(|value| value.as_str()) == Some("user") { + skill_name + } else { + let plugin_name = + normalized_label(meta.get("plugin_name")?.as_str()?, MAX_SKILL_NAME_CHARS)?; + let qualified_name = format!("{plugin_name}:{skill_name}"); + (qualified_name.chars().count() <= MAX_QUALIFIED_SKILL_NAME_CHARS) + .then_some(qualified_name)? + }; + let description = normalized_description(resource.description.as_deref().unwrap_or_default())?; + let main_prompt = main_prompt_uri(uri); - pub(crate) async fn list_for_turn(&self, query: SkillListQuery) -> SkillCatalog { - self.list_matching(&query, |source| source.should_list(&query)) - .await - } + Some( + SkillCatalogEntry::new( + SkillPackageId(uri.to_string()), + orchestrator_authority(), + name, + description, + SkillResourceId::new(main_prompt), + ) + .with_display_path(uri), + ) +} - pub(crate) async fn list_orchestrator_for_turn( - &self, - query: SkillListQuery, - ) -> SkillProviderResult { - let mut catalog = SkillCatalog::default(); +fn orchestrator_authority() -> SkillAuthority { + SkillAuthority::new(SkillSourceKind::Orchestrator, CODEX_APPS_MCP_SERVER_NAME) +} - for source in self - .sources - .iter() - .filter(|source| source.kind == SkillSourceKind::Orchestrator) - { - let source_catalog = source.provider.list(query.clone()).await.map_err(|err| { - SkillProviderError::new(format!( - "{} skills unavailable: {}", - source.label, err.message - )) - })?; - catalog.extend(source_catalog); - } +fn validated_skill_uri(uri: &str, max_chars: usize) -> Option<&str> { + validated_skill_url(uri, max_chars).map(|_| uri) +} - Ok(catalog) +fn validated_skill_url(uri: &str, max_chars: usize) -> Option { + if uri.chars().count() > max_chars + || uri + .chars() + .any(|ch| ch.is_control() || ch.is_whitespace() || matches!(ch, '<' | '>')) + { + return None; } - async fn list_matching( - &self, - query: &SkillListQuery, - should_list: impl Fn(&SkillProviderSource) -> bool, - ) -> SkillCatalog { - let mut catalog = SkillCatalog::default(); - - for source in self.sources.iter().filter(|source| should_list(source)) { - extend_catalog( - &mut catalog, - source.provider.list(query.clone()).await, - source.label.as_str(), - ); - } + let url = Url::parse(uri).ok()?; + let path_is_valid = url.path_segments().is_some_and(|segments| { + let segments = segments.collect::>(); + !segments.is_empty() && segments.iter().all(|segment| !segment.is_empty()) + }); + (url.scheme() == "skill" + && url.as_str() == uri + && url.host_str().is_some_and(|host| !host.is_empty()) + && url.username().is_empty() + && url.password().is_none() + && url.port().is_none() + && url.query().is_none() + && url.fragment().is_none() + && path_is_valid) + .then_some(url) +} - catalog - } +fn resource_belongs_to_package(package: &str, resource: &str) -> bool { + let Some(package) = validated_skill_url(package, MAX_SKILL_PACKAGE_URI_CHARS) else { + return false; + }; + let Some(resource) = validated_skill_url(resource, MAX_SKILL_RESOURCE_URI_CHARS) else { + return false; + }; - pub(crate) async fn read( - &self, - request: SkillReadRequest, - ) -> Result { - let mut last_error = None; - for source in self - .sources - .iter() - .filter(|source| source.owns_kind(&request.authority.kind)) - { - match source.provider.read(request.clone()).await { - Ok(result) => return Ok(result), - Err(err) => last_error = Some(err), - } - } + let Some(package_segments) = package.path_segments() else { + return false; + }; + let Some(resource_segments) = resource.path_segments() else { + return false; + }; + let package_segments = package_segments.collect::>(); + let resource_segments = resource_segments.collect::>(); - match last_error { - Some(err) => Err(err), - None => Err(SkillProviderError::new(format!( - "{} skill provider is not configured", - request.authority.kind - ))), - } - } + package.scheme() == resource.scheme() + && package.host_str() == resource.host_str() + && resource_segments.len() > package_segments.len() + && resource_segments.starts_with(&package_segments) +} - pub async fn search( - &self, - request: SkillSearchRequest, - ) -> Result { - let mut last_error = None; - for source in self - .sources - .iter() - .filter(|source| source.owns_kind(&request.authority.kind)) - { - match source.provider.search(request.clone()).await { - Ok(result) => return Ok(result), - Err(err) => last_error = Some(err), - } - } +fn normalized_label(value: &str, max_chars: usize) -> Option { + let value = normalized_single_line(value, max_chars)?; + let invalid = value.is_empty() || value.chars().any(|ch| matches!(ch, '&' | '<' | '>')); + (!invalid).then_some(value) +} - match last_error { - Some(err) => Err(err), - None => Err(SkillProviderError::new(format!( - "{} skill provider is not configured", - request.authority.kind - ))), - } +fn normalized_description(value: &str) -> Option { + let value = value.split_whitespace().collect::>().join(" "); + if value.chars().any(char::is_control) { + return None; } + + Some( + value + .replace('&', "&") + .replace('<', "<") + .replace('>', ">"), + ) } -fn extend_catalog( - catalog: &mut SkillCatalog, - result: Result, - label: &str, -) { - match result { - Ok(source_catalog) => catalog.extend(source_catalog), - Err(err) => catalog - .warnings - .push(format!("{label} skills unavailable: {}", err.message)), - } +fn normalized_single_line(value: &str, max_chars: usize) -> Option { + let value = value.split_whitespace().collect::>().join(" "); + let valid = value.chars().count() <= max_chars && !value.chars().any(char::is_control); + valid.then_some(value) +} + +fn main_prompt_uri(package_uri: &str) -> String { + format!("{}/SKILL.md", package_uri.trim_end_matches('/')) } diff --git a/codex-rs/ext/skills/src/state.rs b/codex-rs/ext/skills/src/state.rs index 0b751ebdc2e2..307c8241ae72 100644 --- a/codex-rs/ext/skills/src/state.rs +++ b/codex-rs/ext/skills/src/state.rs @@ -2,99 +2,79 @@ use std::collections::HashMap; use std::future::Future; use std::sync::Arc; use std::sync::Mutex; - +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; + +use codex_core_skills::runtime::SkillAuthority; +use codex_core_skills::runtime::SkillCatalog; +use codex_core_skills::runtime::SkillPackageId; +use codex_core_skills::runtime::SkillReadRequest; +use codex_core_skills::runtime::SkillReadResult; +use codex_core_skills::runtime::SkillResourceId; +use codex_core_skills::runtime::SkillSourceError; +use codex_core_skills::runtime::SkillSourceResult; use codex_mcp::McpResourceClient; use codex_mcp::McpResourceClientCacheKey; -use codex_protocol::capabilities::SelectedCapabilityRoot; use tokio::sync::OnceCell; -use crate::SkillsExtensionConfig; -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillCatalogEntry; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillProviderError; -use crate::catalog::SkillProviderResult; -use crate::catalog::SkillReadResult; -use crate::catalog::SkillResourceId; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillReadRequest; -use crate::sources::SkillProviders; - const MAX_CACHED_ORCHESTRATOR_RESOURCES: usize = 100; const MAX_CACHED_ORCHESTRATOR_CONTENT_BYTES: usize = 8 * 1024 * 1024; pub(crate) struct SkillsThreadState { - config: Mutex, - selected_roots: Vec, + orchestrator_skills_enabled: AtomicBool, orchestrator_skills_available: bool, orchestrator_cache: Mutex>>, } impl SkillsThreadState { pub(crate) fn new( - config: SkillsExtensionConfig, - selected_roots: Vec, + orchestrator_skills_enabled: bool, orchestrator_skills_available: bool, ) -> Self { Self { - config: Mutex::new(config), - selected_roots, + orchestrator_skills_enabled: AtomicBool::new(orchestrator_skills_enabled), orchestrator_skills_available, orchestrator_cache: Mutex::new(None), } } - pub(crate) fn config(&self) -> SkillsExtensionConfig { - self.config - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone() - } - - pub(crate) fn set_config(&self, config: SkillsExtensionConfig) { - *self - .config - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) = config; - } - - pub(crate) fn selected_roots(&self) -> &[SelectedCapabilityRoot] { - &self.selected_roots + pub(crate) fn set_orchestrator_skills_enabled(&self, enabled: bool) { + self.orchestrator_skills_enabled + .store(enabled, Ordering::Relaxed); } pub(crate) fn orchestrator_skills_enabled(&self) -> bool { - self.orchestrator_skills_available && self.config().orchestrator_skills_enabled + self.orchestrator_skills_available + && self.orchestrator_skills_enabled.load(Ordering::Relaxed) } pub(crate) async fn orchestrator_catalog_snapshot( &self, mcp_resources: Option<&McpResourceClient>, - initialize: impl Future> + Send, + initialize: impl Future> + Send, ) -> SkillCatalog { - self.orchestrator_cache(mcp_resources) - .catalog - .get_or_init(|| async { - initialize.await.unwrap_or_else(|err| SkillCatalog { - warnings: vec![err.message], - ..Default::default() - }) - }) - .await - .clone() + let cache = self.orchestrator_cache(mcp_resources); + if let Some(catalog) = cache.catalog.get() { + return catalog.clone(); + } + let catalog = initialize.await.unwrap_or_else(|err| SkillCatalog { + warnings: vec![err.message], + ..Default::default() + }); + if !catalog.entries.is_empty() && catalog.warnings.is_empty() { + let _ = cache.catalog.set(catalog.clone()); + } + catalog } - pub(crate) async fn read_skill( + pub(crate) async fn read_orchestrator_resource( &self, - providers: &SkillProviders, - request: SkillReadRequest, - ) -> SkillProviderResult { - if request.authority.kind != SkillSourceKind::Orchestrator { - return providers.read(request).await; - } - - let cache = self.orchestrator_cache(request.mcp_resources.as_deref()); - let cache_key = SkillReadCacheKey::from(&request); + request: &SkillReadRequest, + mcp_resources: Option<&McpResourceClient>, + read: impl Future> + Send, + ) -> SkillSourceResult { + let cache_key = SkillReadCacheKey::from(request); + let cache = self.orchestrator_cache(mcp_resources); if let Some(result) = cache .resources .lock() @@ -104,7 +84,7 @@ impl SkillsThreadState { return Ok(result); } - let result = providers.read(request).await?; + let result = read.await?; if result.resource != cache_key.resource { return Ok(result); } @@ -196,11 +176,3 @@ impl OrchestratorResourceCache { result } } - -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub(crate) struct SkillsTurnState { - pub(crate) catalog: SkillCatalog, - pub(crate) selected_entries: Vec, - pub(crate) warnings: Vec, - pub(crate) main_prompts_injected: bool, -} diff --git a/codex-rs/ext/skills/src/tools/list.rs b/codex-rs/ext/skills/src/tools/list.rs index e556f9783f06..6433819f48f2 100644 --- a/codex-rs/ext/skills/src/tools/list.rs +++ b/codex-rs/ext/skills/src/tools/list.rs @@ -7,9 +7,9 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use crate::catalog::SkillCatalogEntry; use crate::render::truncate_catalog_skill_description; use crate::render::truncate_utf8_to_bytes; +use codex_core_skills::runtime::SkillCatalogEntry; use super::MAX_HANDLE_BYTES; use super::SkillToolAuthority; @@ -68,7 +68,8 @@ impl ToolExecutor for ListTool { Box::pin(async move { let args: ListArgs = parse_args(&call)?; let authority = args.authority.into_authority(); - let catalog = self.context.catalog(&call.turn_id, args.authority).await; + let source = self.context.source(); + let catalog = self.context.catalog(source.as_ref()).await; let response = ListResponse { skills: catalog .entries diff --git a/codex-rs/ext/skills/src/tools/mod.rs b/codex-rs/ext/skills/src/tools/mod.rs index 0d05064afb89..7e9b319bff82 100644 --- a/codex-rs/ext/skills/src/tools/mod.rs +++ b/codex-rs/ext/skills/src/tools/mod.rs @@ -1,5 +1,9 @@ use std::sync::Arc; +use codex_core_skills::runtime::SkillAuthority; +use codex_core_skills::runtime::SkillCatalog; +use codex_core_skills::runtime::SkillSource; +use codex_core_skills::runtime::SkillSourceKind; use codex_extension_api::FunctionCallError; use codex_extension_api::JsonToolOutput; use codex_extension_api::ResponsesApiTool; @@ -19,11 +23,7 @@ use serde::Deserialize; use serde::Serialize; use serde_json::Value; -use crate::catalog::SkillAuthority; -use crate::catalog::SkillCatalog; -use crate::catalog::SkillSourceKind; -use crate::provider::SkillListQuery; -use crate::sources::SkillProviders; +use crate::sources::orchestrator_skill_source; use crate::state::SkillsThreadState; mod list; @@ -34,13 +34,11 @@ const SKILLS_NAMESPACE: &str = "skills"; const MAX_HANDLE_BYTES: usize = 2_048; pub(crate) fn skill_tools( - providers: SkillProviders, mcp_resources: Option>, thread_state: Arc, ) -> Vec>> { let context = SkillToolContext { - providers, - mcp_resources, + mcp_resources: mcp_resources.map(|client| Arc::new(client.snapshot())), thread_state, }; vec![ @@ -53,30 +51,22 @@ pub(crate) fn skill_tools( #[derive(Clone)] struct SkillToolContext { - providers: SkillProviders, mcp_resources: Option>, thread_state: Arc, } impl SkillToolContext { - async fn catalog(&self, turn_id: &str, authority: SkillToolAuthority) -> SkillCatalog { - match authority { - SkillToolAuthority::Orchestrator => { - self.thread_state - .orchestrator_catalog_snapshot( - self.mcp_resources.as_deref(), - self.providers.list_orchestrator_for_turn(SkillListQuery { - turn_id: turn_id.to_string(), - executor_roots: Vec::new(), - host_snapshot: None, - include_host_skills: false, - include_bundled_skills: false, - include_orchestrator_skills: true, - mcp_resources: self.mcp_resources.clone(), - }), - ) - .await - } + fn source(&self) -> Arc { + orchestrator_skill_source(Arc::clone(&self.thread_state), self.mcp_resources.clone()) + } + + async fn catalog(&self, source: &dyn SkillSource) -> SkillCatalog { + match source.list().await { + Ok(catalog) => catalog, + Err(err) => SkillCatalog { + warnings: vec![err.message], + ..Default::default() + }, } } } diff --git a/codex-rs/ext/skills/src/tools/read.rs b/codex-rs/ext/skills/src/tools/read.rs index 64587cab95b6..ff29e6157992 100644 --- a/codex-rs/ext/skills/src/tools/read.rs +++ b/codex-rs/ext/skills/src/tools/read.rs @@ -1,3 +1,6 @@ +use codex_core_skills::runtime::SkillPackageId; +use codex_core_skills::runtime::SkillReadRequest; +use codex_core_skills::runtime::SkillResourceId; use codex_extension_api::FunctionCallError; use codex_extension_api::ToolCall; use codex_extension_api::ToolExecutor; @@ -8,10 +11,6 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use crate::catalog::SkillPackageId; -use crate::catalog::SkillResourceId; -use crate::provider::SkillReadRequest; - use super::MAX_HANDLE_BYTES; use super::SkillToolAuthority; use super::SkillToolContext; @@ -62,7 +61,8 @@ impl ToolExecutor for ReadTool { validate_handle("package", &args.package, MAX_HANDLE_BYTES)?; validate_handle("resource", &args.resource, MAX_HANDLE_BYTES)?; - let catalog = self.context.catalog(&call.turn_id, args.authority).await; + let source = self.context.source(); + let catalog = self.context.catalog(source.as_ref()).await; let package_is_available = catalog.entries.iter().any(|entry| { entry.enabled && entry.authority == authority && entry.id.0 == args.package }); @@ -73,19 +73,12 @@ impl ToolExecutor for ReadTool { } let requested_resource = SkillResourceId::new(args.resource); - let result = self - .context - .thread_state - .read_skill( - &self.context.providers, - SkillReadRequest { - authority, - package: SkillPackageId(args.package), - resource: requested_resource.clone(), - host_snapshot: None, - mcp_resources: self.context.mcp_resources.clone(), - }, - ) + let result = source + .read(SkillReadRequest { + authority, + package: SkillPackageId(args.package), + resource: requested_resource.clone(), + }) .await .map_err(|err| { tracing::warn!( diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs deleted file mode 100644 index 06bf062eacfb..000000000000 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ /dev/null @@ -1,312 +0,0 @@ -use std::io; -use std::sync::Arc; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering; - -use codex_core_skills::HostSkillsSnapshot; -use codex_core_skills::loader::SkillRoot; -use codex_core_skills::loader::load_skills_from_roots; -use codex_exec_server::CopyOptions; -use codex_exec_server::CreateDirectoryOptions; -use codex_exec_server::EnvironmentManager; -use codex_exec_server::ExecutorFileSystem; -use codex_exec_server::ExecutorFileSystemFuture; -use codex_exec_server::FileMetadata; -use codex_exec_server::FileSystemReadStream; -use codex_exec_server::FileSystemSandboxContext; -use codex_exec_server::ReadDirectoryEntry; -use codex_exec_server::RemoveOptions; -use codex_protocol::capabilities::CapabilityRootLocation; -use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_protocol::protocol::SkillScope; -use codex_skills_extension::ExecutorSkillProvider; -use codex_skills_extension::provider::SkillListQuery; -use codex_skills_extension::provider::SkillProvider; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::PathUri; -use pretty_assertions::assert_eq; - -const SKILL_CONTENTS: &str = - "---\nname: synthetic\ndescription: Synthetic executor skill.\n---\n\nEXECUTOR_ONLY_BODY\n"; -const PLUGIN_MANIFEST: &str = r#"{"name":"synthetic-plugin"}"#; -static NEXT_TEST_ROOT_ID: AtomicUsize = AtomicUsize::new(0); - -struct SyntheticFileSystem { - alias_root: PathUri, - canonical_root: PathUri, - has_plugin_manifest: bool, -} - -impl SyntheticFileSystem { - fn path(&self, relative_path: &str) -> io::Result { - self.canonical_root - .join(relative_path) - .map_err(io::Error::other) - } - - async fn canonicalize(&self, path: &PathUri) -> io::Result { - if path == &self.alias_root { - return Ok(self.canonical_root.clone()); - } - self.metadata(path)?; - Ok(path.clone()) - } - - async fn read_file(&self, path: &PathUri) -> io::Result> { - if path == &self.path("skill/SKILL.md")? { - Ok(SKILL_CONTENTS.as_bytes().to_vec()) - } else if self.has_plugin_manifest && path == &self.path(".claude-plugin/plugin.json")? { - Ok(PLUGIN_MANIFEST.as_bytes().to_vec()) - } else { - Err(io::Error::new(io::ErrorKind::NotFound, "not found")) - } - } - - async fn read_directory(&self, path: &PathUri) -> io::Result> { - if path == &self.canonical_root { - Ok(vec![ReadDirectoryEntry { - file_name: "skill".to_string(), - is_directory: true, - is_file: false, - }]) - } else if path == &self.path("skill")? { - Ok(vec![ReadDirectoryEntry { - file_name: "SKILL.md".to_string(), - is_directory: false, - is_file: true, - }]) - } else { - Err(io::Error::new(io::ErrorKind::NotFound, "not found")) - } - } - - fn metadata(&self, path: &PathUri) -> io::Result { - let skill_dir = self.path("skill")?; - let skill_path = self.path("skill/SKILL.md")?; - let manifest_path = self.path(".claude-plugin/plugin.json")?; - let (is_directory, is_file) = if path == &self.canonical_root || path == &skill_dir { - (true, false) - } else if path == &skill_path || self.has_plugin_manifest && path == &manifest_path { - (false, true) - } else { - return Err(io::Error::new(io::ErrorKind::NotFound, "not found")); - }; - Ok(FileMetadata { - is_directory, - is_file, - is_symlink: false, - size: 0, - created_at_ms: 0, - modified_at_ms: 0, - }) - } -} - -impl ExecutorFileSystem for SyntheticFileSystem { - fn canonicalize<'a>( - &'a self, - path: &'a PathUri, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, PathUri> { - Box::pin(SyntheticFileSystem::canonicalize(self, path)) - } - - fn read_file<'a>( - &'a self, - path: &'a PathUri, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, Vec> { - Box::pin(SyntheticFileSystem::read_file(self, path)) - } - - fn read_file_stream<'a>( - &'a self, - _path: &'a PathUri, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> { - Box::pin(async { - Err(io::Error::new( - io::ErrorKind::Unsupported, - "synthetic filesystem does not support streaming reads", - )) - }) - } - - fn write_file<'a>( - &'a self, - _path: &'a PathUri, - _contents: Vec, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, ()> { - Box::pin(async move { Err(io::Error::new(io::ErrorKind::Unsupported, "read only")) }) - } - - fn create_directory<'a>( - &'a self, - _path: &'a PathUri, - _options: CreateDirectoryOptions, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, ()> { - Box::pin(async move { Err(io::Error::new(io::ErrorKind::Unsupported, "read only")) }) - } - - fn get_metadata<'a>( - &'a self, - path: &'a PathUri, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, FileMetadata> { - Box::pin(async move { self.metadata(path) }) - } - - fn read_directory<'a>( - &'a self, - path: &'a PathUri, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, Vec> { - Box::pin(SyntheticFileSystem::read_directory(self, path)) - } - - fn remove<'a>( - &'a self, - _path: &'a PathUri, - _options: RemoveOptions, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, ()> { - Box::pin(async move { Err(io::Error::new(io::ErrorKind::Unsupported, "read only")) }) - } - - fn copy<'a>( - &'a self, - _source_path: &'a PathUri, - _destination_path: &'a PathUri, - _options: CopyOptions, - _sandbox: Option<&'a FileSystemSandboxContext>, - ) -> ExecutorFileSystemFuture<'a, ()> { - Box::pin(async move { Err(io::Error::new(io::ErrorKind::Unsupported, "read only")) }) - } -} - -#[tokio::test] -async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { - let test_root = - std::env::temp_dir().join(format!("codex-executor-skill-fs-{}", std::process::id())); - let alias_root = AbsolutePathBuf::from_absolute_path_checked(test_root.join("alias")) - .expect("absolute path"); - let canonical_root = AbsolutePathBuf::from_absolute_path_checked(test_root.join("canonical")) - .expect("absolute path"); - assert!(!alias_root.as_path().exists()); - assert!(!canonical_root.as_path().exists()); - - let outcome = load_skills_from_roots( - [SkillRoot { - path: alias_root.clone(), - scope: SkillScope::User, - file_system: Arc::new(SyntheticFileSystem { - alias_root: PathUri::from_abs_path(&alias_root), - canonical_root: PathUri::from_abs_path(&canonical_root), - has_plugin_manifest: false, - }), - plugin_id: None, - plugin_namespace: None, - plugin_root: None, - }], - /*plugin_skill_snapshots*/ None, - ) - .await; - assert_eq!(outcome.errors, Vec::new()); - assert_eq!(outcome.skills.len(), 1); - - let skill = outcome.skills[0].clone(); - assert_eq!(skill.name, "synthetic"); - assert_eq!( - skill.path_to_skills_md, - canonical_root.join("skill/SKILL.md") - ); - let loaded = HostSkillsSnapshot::new(Arc::new(outcome)); - assert_eq!( - loaded.read_skill_text(&skill).await.expect("skill body"), - SKILL_CONTENTS - ); -} - -#[tokio::test] -async fn selected_root_id_distinguishes_identical_executor_paths() { - let root_label = if cfg!(unix) { - r"root\identity" - } else { - "root-identity" - }; - let test_root = create_local_skill_root(root_label).expect("create local skill root"); - let selected_root = test_root.to_string_lossy().into_owned(); - let selected_root = if cfg!(windows) { - selected_root.replace('\\', "/") - } else { - selected_root - }; - let provider = ExecutorSkillProvider::new_with_restriction_product( - Arc::new(EnvironmentManager::default_for_tests()), - /*restriction_product*/ None, - ); - let catalog = provider - .list(SkillListQuery { - turn_id: "turn-1".to_string(), - executor_roots: ["root-a", "root-b"] - .into_iter() - .map(|id| SelectedCapabilityRoot { - id: id.to_string(), - location: CapabilityRootLocation::Environment { - environment_id: "local".to_string(), - path: PathUri::from_host_native_path(&test_root).expect("skill root URI"), - }, - }) - .collect(), - host_snapshot: None, - include_host_skills: false, - include_bundled_skills: true, - include_orchestrator_skills: false, - mcp_resources: None, - }) - .await - .expect("list executor skills"); - - assert_eq!( - catalog - .entries - .iter() - .map(|entry| ( - entry.authority.id.clone(), - entry.display_path.clone().expect("display path"), - )) - .collect::>(), - vec![ - ( - "root-a".to_string(), - format!( - "skill://root-a/{}/skill/SKILL.md", - selected_root.trim_start_matches('/') - ), - ), - ( - "root-b".to_string(), - format!( - "skill://root-b/{}/skill/SKILL.md", - selected_root.trim_start_matches('/') - ), - ), - ] - ); - - std::fs::remove_dir_all(test_root).expect("remove skill directory"); -} - -fn create_local_skill_root(label: &str) -> io::Result { - let id = NEXT_TEST_ROOT_ID.fetch_add(1, Ordering::Relaxed); - let test_root = std::env::temp_dir().join(format!( - "codex-executor-skill-{label}-{}-{id}", - std::process::id() - )); - let skill_dir = test_root.join("skill"); - std::fs::create_dir_all(&skill_dir)?; - std::fs::write(skill_dir.join("SKILL.md"), SKILL_CONTENTS)?; - Ok(test_root) -} diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs deleted file mode 100644 index 761dc34d71b3..000000000000 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ /dev/null @@ -1,738 +0,0 @@ -use std::path::PathBuf; -use std::sync::Arc; -use std::sync::Mutex; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering; - -use codex_core_skills::HostSkillsSnapshot; -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; -use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; -use codex_core_skills::SkillLoadOutcome; -use codex_core_skills::SkillMetadata; -use codex_core_skills::injection::InjectedHostSkillPrompts; -use codex_extension_api::ConversationHistory; -use codex_extension_api::ExtensionData; -use codex_extension_api::ExtensionEventSink; -use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::NoopTurnItemEmitter; -use codex_extension_api::ThreadStartInput; -use codex_extension_api::ToolCall; -use codex_extension_api::ToolPayload; -use codex_extension_api::TurnInputContext; -use codex_protocol::capabilities::CapabilityRootLocation; -use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_protocol::protocol::Event; -use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; -use codex_protocol::protocol::SessionSource; -use codex_protocol::protocol::SkillScope; -use codex_protocol::protocol::TruncationPolicy; -use codex_protocol::user_input::UserInput; -use codex_skills_extension::SkillProviders; -use codex_skills_extension::SkillsExtensionConfig; -use codex_skills_extension::catalog::SkillAuthority; -use codex_skills_extension::catalog::SkillCatalog; -use codex_skills_extension::catalog::SkillCatalogEntry; -use codex_skills_extension::catalog::SkillPackageId; -use codex_skills_extension::catalog::SkillProviderError; -use codex_skills_extension::catalog::SkillReadResult; -use codex_skills_extension::catalog::SkillResourceId; -use codex_skills_extension::catalog::SkillSearchResult; -use codex_skills_extension::catalog::SkillSourceKind; -use codex_skills_extension::install; -use codex_skills_extension::install_with_providers; -use codex_skills_extension::provider::SkillListQuery; -use codex_skills_extension::provider::SkillProvider; -use codex_skills_extension::provider::SkillProviderFuture; -use codex_skills_extension::provider::SkillReadRequest; -use codex_skills_extension::provider::SkillSearchRequest; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::PathUri; -use pretty_assertions::assert_eq; - -type TestResult = Result<(), Box>; - -static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0); -const DEMO_SKILL_CONTENTS: &str = - "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n"; - -#[tokio::test] -async fn installed_extension_uses_host_service_snapshot() -> TestResult { - let codex_home = test_codex_home(); - let skill_path = codex_home.join("skills").join("demo").join("SKILL.md"); - std::fs::create_dir_all( - skill_path - .parent() - .ok_or("skill path should have a parent")?, - )?; - std::fs::write(&skill_path, DEMO_SKILL_CONTENTS)?; - let config = default_config(); - - let mut builder = ExtensionRegistryBuilder::new(); - install(&mut builder, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - let session_source = SessionSource::Cli; - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let skill_path = AbsolutePathBuf::try_from(skill_path)?; - let skill_path_string = skill_path.to_string_lossy().into_owned(); - let mut outcome = SkillLoadOutcome::default(); - outcome.skills.push(SkillMetadata { - name: "demo".to_string(), - description: "Demo skill.".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: skill_path, - scope: SkillScope::User, - plugin_id: None, - }); - let loaded_skills = Arc::new(outcome); - let skill_prompt_path = skill_path_string.replace('\\', "/"); - let turn_store = ExtensionData::new("turn-1"); - turn_store.insert(HostSkillsSnapshot::new(Arc::clone(&loaded_skills))); - - let fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: "turn-1".to_string(), - user_input: vec![UserInput::Text { - text: "$demo".to_string(), - text_elements: Vec::new(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &turn_store, - ) - .await; - - let expected_catalog = format!( - "{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n### How to use skills\n{SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}" - ); - let expected_skill = format!( - "\ndemo\n{skill_prompt_path}\n{DEMO_SKILL_CONTENTS}\n" - ); - assert_eq!( - vec![("developer", expected_catalog), ("user", expected_skill),], - fragments - .iter() - .map(|fragment| (fragment.role(), fragment.render())) - .collect::>() - ); - let injected_host_skill_prompts = turn_store - .get::() - .ok_or("host skill prompt marker should be set")?; - assert!(injected_host_skill_prompts.contains_path(&skill_path_string)); - - std::fs::remove_dir_all(codex_home)?; - Ok(()) -} - -#[tokio::test] -async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_input() -> TestResult -{ - let read_requests = Arc::new(Mutex::new(Vec::new())); - let executor_provider = Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![test_entry( - SkillSourceKind::Executor, - "env-1", - "executor/lint-fix", - "lint-fix/SKILL.md", - )], - warnings: Vec::new(), - }, - read_requests: Arc::clone(&read_requests), - list_calls: None, - fail_first_list: false, - }); - let providers = SkillProviders::new().with_executor_provider(executor_provider); - let mut builder = ExtensionRegistryBuilder::new(); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - thread_store.insert(vec![SelectedCapabilityRoot { - id: "lint-fix".to_string(), - location: CapabilityRootLocation::Environment { - environment_id: "env-1".to_string(), - path: PathUri::parse("file:///skills/lint-fix").expect("skill root URI"), - }, - }]); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let prompt_fragments = registry.context_contributors()[0] - .contribute_thread_context(&session_store, &thread_store) - .await; - assert_eq!(1, prompt_fragments.len()); - assert!( - prompt_fragments[0] - .text() - .starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG) - ); - assert!(prompt_fragments[0].text().contains("lint-fix")); - assert!( - prompt_fragments[0] - .text() - .contains("(environment resource: skill://executor/lint-fix/SKILL.md)") - ); - - let turn_store = ExtensionData::new("turn-1"); - let fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: "turn-1".to_string(), - user_input: vec![UserInput::Text { - text: "$lint-fix please".to_string(), - text_elements: Vec::new(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &turn_store, - ) - .await; - - assert_eq!(1, fragments.len()); - assert_eq!("user", fragments[0].role()); - assert!(fragments[0].render().contains("lint-fix")); - assert!(fragments[0].render().contains("# Lint Fix")); - assert_eq!( - vec![( - SkillAuthority::new(SkillSourceKind::Executor, "env-1"), - SkillPackageId("executor/lint-fix".to_string()), - SkillResourceId::new("lint-fix/SKILL.md"), - )], - read_request_keys(&read_requests) - ); - let rebuilt_prompt_fragments = registry.context_contributors()[0] - .contribute_thread_context(&session_store, &thread_store) - .await; - assert_eq!(1, rebuilt_prompt_fragments.len()); - assert!(rebuilt_prompt_fragments[0].text().contains("lint-fix")); - - let next_turn_store = ExtensionData::new("turn-2"); - let next_fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: "turn-2".to_string(), - user_input: vec![UserInput::Text { - text: "no skill this time".to_string(), - text_elements: Vec::new(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &next_turn_store, - ) - .await; - - assert!(next_fragments.is_empty()); - - Ok(()) -} - -#[tokio::test] -async fn default_context_truncates_catalog_descriptions() -> TestResult { - let description = "x".repeat(1_025); - let mut entry = test_entry( - SkillSourceKind::Orchestrator, - "codex_apps", - "orchestrator/long-description", - "skill://orchestrator/long-description/SKILL.md", - ); - entry.description = description.clone(); - let providers = - SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![entry], - warnings: Vec::new(), - }, - read_requests: Arc::new(Mutex::new(Vec::new())), - list_calls: None, - fail_first_list: false, - })); - let mut builder = ExtensionRegistryBuilder::new(); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let fragments = registry.context_contributors()[0] - .contribute_thread_context(&session_store, &thread_store) - .await; - assert_eq!(1, fragments.len()); - let rendered = fragments[0].text(); - assert!(rendered.contains(&("x".repeat(1_021) + "..."))); - assert!(!rendered.contains(&"x".repeat(1_024))); - assert!(!rendered.contains(&description)); - - Ok(()) -} - -#[tokio::test] -async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult { - let description = "x".repeat(1_025); - let mut entry = test_entry( - SkillSourceKind::Orchestrator, - "codex_apps", - "orchestrator/long-description", - "skill://orchestrator/long-description/SKILL.md", - ); - entry.description = description.clone(); - let providers = - SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![entry], - warnings: Vec::new(), - }, - read_requests: Arc::new(Mutex::new(Vec::new())), - list_calls: None, - fail_first_list: false, - })); - let mut builder = ExtensionRegistryBuilder::new(); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let tools = registry.tool_contributors()[0].tools(&session_store, &thread_store); - let list_tool = tools - .iter() - .find(|tool| tool.tool_name().name == "list") - .ok_or("skills.list tool should be registered")?; - let payload = ToolPayload::Function { - arguments: serde_json::json!({"authority": {"kind": "orchestrator"}}).to_string(), - }; - let output = list_tool - .handle(ToolCall { - turn_id: "turn-1".to_string(), - call_id: "call-1".to_string(), - tool_name: list_tool.tool_name(), - model: "gpt-test".to_string(), - truncation_policy: TruncationPolicy::Bytes(1_024), - conversation_history: ConversationHistory::default(), - turn_item_emitter: Arc::new(NoopTurnItemEmitter), - environments: Vec::new(), - payload: payload.clone(), - }) - .await?; - let response = output - .post_tool_use_response("call-1", &payload) - .ok_or("skills.list should expose structured output")?; - let rendered_description = response["skills"][0]["description"] - .as_str() - .ok_or("skills.list response should include a description")?; - - assert_eq!(rendered_description, "x".repeat(1_021) + "..."); - assert_ne!(rendered_description, description); - - Ok(()) -} - -#[tokio::test] -async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { - let list_calls = Arc::new(AtomicUsize::new(0)); - let providers = - SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![test_entry( - SkillSourceKind::Orchestrator, - "codex_apps", - "orchestrator/first", - "skill://orchestrator/first/SKILL.md", - )], - warnings: Vec::new(), - }, - read_requests: Arc::new(Mutex::new(Vec::new())), - list_calls: Some(Arc::clone(&list_calls)), - fail_first_list: true, - })); - let (event_tx, event_rx) = std::sync::mpsc::channel(); - let mut builder = - ExtensionRegistryBuilder::with_event_sink(Arc::new(ChannelEventSink(event_tx))); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let initial_fragments = registry.context_contributors()[0] - .contribute_thread_context(&session_store, &thread_store) - .await; - assert!(initial_fragments.is_empty()); - let EventMsg::Warning(warning) = event_rx.try_recv()?.msg else { - panic!("expected warning event"); - }; - assert_eq!( - warning.message, - "orchestrator skills unavailable: temporary orchestrator failure" - ); - - for turn_id in ["turn-1", "turn-2"] { - let fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: turn_id.to_string(), - user_input: vec![UserInput::Text { - text: "$first".to_string(), - text_elements: Vec::new(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &ExtensionData::new(turn_id), - ) - .await; - assert!(fragments.is_empty()); - } - assert_eq!(1, list_calls.load(Ordering::Relaxed)); - - Ok(()) -} - -#[tokio::test] -async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> TestResult { - let read_requests = Arc::new(Mutex::new(Vec::new())); - let root_a_locator = "skill://root-a/shared/lint-fix/SKILL.md"; - let root_b_locator = "skill://root-b/shared/lint-fix/SKILL.md"; - let executor_provider = Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: [("root-a", root_a_locator), ("root-b", root_b_locator)] - .into_iter() - .map(|(root_id, locator)| { - SkillCatalogEntry::new( - SkillPackageId(locator.to_string()), - SkillAuthority::new(SkillSourceKind::Executor, root_id), - "lint-fix", - "Fix lint errors.", - SkillResourceId::new(locator), - ) - .with_display_path(locator) - }) - .collect(), - warnings: Vec::new(), - }, - read_requests: Arc::clone(&read_requests), - list_calls: None, - fail_first_list: false, - }); - let providers = SkillProviders::new().with_executor_provider(executor_provider); - let mut builder = ExtensionRegistryBuilder::new(); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - thread_store.insert( - [("root-a", "/skills/root-a"), ("root-b", "/skills/root-b")] - .into_iter() - .map(|(id, path)| SelectedCapabilityRoot { - id: id.to_string(), - location: CapabilityRootLocation::Environment { - environment_id: "env-1".to_string(), - path: PathUri::parse(&format!("file://{path}")).expect("skill root URI"), - }, - }) - .collect::>(), - ); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: "turn-1".to_string(), - user_input: vec![UserInput::Mention { - name: "lint-fix".to_string(), - path: root_b_locator.to_string(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &ExtensionData::new("turn-1"), - ) - .await; - - assert_eq!(1, fragments.len()); - assert!(fragments[0].render().contains(root_b_locator)); - assert_eq!( - vec![( - SkillAuthority::new(SkillSourceKind::Executor, "root-b"), - SkillPackageId(root_b_locator.to_string()), - SkillResourceId::new(root_b_locator), - )], - read_request_keys(&read_requests) - ); - - Ok(()) -} - -#[tokio::test] -async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { - let read_requests = Arc::new(Mutex::new(Vec::new())); - let provider = Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![ - test_entry( - SkillSourceKind::Host, - "host", - "host/visible-skill", - "visible-skill/SKILL.md", - ), - test_entry( - SkillSourceKind::Host, - "host", - "host/hidden-skill", - "hidden-skill/SKILL.md", - ) - .hidden_from_prompt(), - ], - warnings: Vec::new(), - }, - read_requests: Arc::clone(&read_requests), - list_calls: None, - fail_first_list: false, - }); - let providers = SkillProviders::new().with_host_provider(provider); - let mut builder = ExtensionRegistryBuilder::new(); - install_with_providers(&mut builder, providers, skills_extension_config); - let registry = builder.build(); - let session_store = ExtensionData::new("session"); - let thread_store = ExtensionData::new("thread"); - let session_source = SessionSource::Cli; - let config = default_config(); - registry.thread_lifecycle_contributors()[0] - .on_thread_start(ThreadStartInput { - config: &config, - session_source: &session_source, - persistent_thread_state_available: true, - environments: &[], - session_store: &session_store, - thread_store: &thread_store, - }) - .await; - - let fragments = registry.turn_input_contributors()[0] - .contribute( - TurnInputContext { - turn_id: "turn-1".to_string(), - user_input: vec![UserInput::Text { - text: "$hidden-skill".to_string(), - text_elements: Vec::new(), - }], - environments: Vec::new(), - }, - &session_store, - &thread_store, - &ExtensionData::new("turn-1"), - ) - .await; - - assert_eq!(2, fragments.len()); - assert!(fragments[0].render().contains("visible-skill")); - assert!(!fragments[0].render().contains("hidden-skill")); - assert!(fragments[1].render().contains("hidden-skill")); - assert_eq!( - vec![( - SkillAuthority::new(SkillSourceKind::Host, "host"), - SkillPackageId("host/hidden-skill".to_string()), - SkillResourceId::new("hidden-skill/SKILL.md"), - )], - read_request_keys(&read_requests) - ); - - Ok(()) -} - -#[derive(Clone)] -struct StaticSkillProvider { - catalog: SkillCatalog, - read_requests: Arc>>, - list_calls: Option>, - fail_first_list: bool, -} - -struct ChannelEventSink(std::sync::mpsc::Sender); - -impl ExtensionEventSink for ChannelEventSink { - fn emit(&self, event: Event) { - let _ = self.0.send(event); - } -} - -impl SkillProvider for StaticSkillProvider { - fn list(&self, _query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { - let list_call = self - .list_calls - .as_ref() - .map(|list_calls| list_calls.fetch_add(1, Ordering::Relaxed)); - let fail = self.fail_first_list && list_call == Some(0); - let catalog = self.catalog.clone(); - Box::pin(async move { - if fail { - Err(SkillProviderError::new("temporary orchestrator failure")) - } else { - Ok(catalog) - } - }) - } - - fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { - let read_requests = Arc::clone(&self.read_requests); - Box::pin(async move { - read_requests - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .push(request.clone()); - Ok(SkillReadResult { - resource: request.resource, - contents: "# Lint Fix\n\nRun the formatter.".to_string(), - }) - }) - } - - fn search(&self, _request: SkillSearchRequest) -> SkillProviderFuture<'_, SkillSearchResult> { - Box::pin(async { Ok(SkillSearchResult::default()) }) - } -} - -fn test_entry( - kind: SkillSourceKind, - authority_id: &str, - package_id: &str, - main_prompt: &str, -) -> SkillCatalogEntry { - let name = package_id.rsplit('/').next().unwrap_or(package_id); - SkillCatalogEntry::new( - SkillPackageId(package_id.to_string()), - SkillAuthority::new(kind, authority_id), - name, - "Fix lint errors.", - SkillResourceId::new(main_prompt), - ) - .with_display_path(format!("skill://{package_id}/SKILL.md")) -} - -#[derive(Clone, Debug, Eq, PartialEq)] -struct TestConfig { - include_instructions: bool, - bundled_skills_enabled: bool, - orchestrator_skills_enabled: bool, -} - -fn default_config() -> TestConfig { - TestConfig { - include_instructions: true, - bundled_skills_enabled: true, - orchestrator_skills_enabled: true, - } -} - -fn skills_extension_config(config: &TestConfig) -> SkillsExtensionConfig { - SkillsExtensionConfig { - include_instructions: config.include_instructions, - bundled_skills_enabled: config.bundled_skills_enabled, - orchestrator_skills_enabled: config.orchestrator_skills_enabled, - } -} - -fn test_codex_home() -> PathBuf { - let id = NEXT_CODEX_HOME_ID.fetch_add(1, Ordering::Relaxed); - std::env::temp_dir().join(format!( - "codex-skills-extension-test-{}-{id}", - std::process::id(), - )) -} - -fn read_request_keys( - requests: &Arc>>, -) -> Vec<(SkillAuthority, SkillPackageId, SkillResourceId)> { - requests - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .iter() - .map(|request| { - ( - request.authority.clone(), - request.package.clone(), - request.resource.clone(), - ) - }) - .collect() -} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1affdf0d2026..42b01b16f32b 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -19,6 +19,7 @@ use crate::AgentPath; use crate::SessionId; use crate::ThreadId; use crate::approvals::ElicitationRequestEvent; +use crate::capabilities::SelectedCapabilityRoot; use crate::config_types::ApprovalsReviewer; use crate::config_types::CollaborationMode; use crate::config_types::ModeKind; @@ -2570,6 +2571,12 @@ impl InitialHistory { } } + pub fn get_selected_capability_roots(&self) -> Vec { + self.get_session_meta() + .map(|meta| meta.selected_capability_roots.clone()) + .unwrap_or_default() + } + pub fn get_multi_agent_version(&self) -> Option { match self { InitialHistory::New | InitialHistory::Cleared => None, @@ -3003,6 +3010,9 @@ pub struct SessionMeta { skip_serializing_if = "Option::is_none" )] pub dynamic_tools: Option>, + /// Capability roots selected for this thread by the hosting platform. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub selected_capability_roots: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub memory_mode: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -3032,6 +3042,7 @@ impl Default for SessionMeta { model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/compression_tests.rs b/codex-rs/rollout/src/compression_tests.rs index a4db9a965caf..733e6cc5fb61 100644 --- a/codex-rs/rollout/src/compression_tests.rs +++ b/codex-rs/rollout/src/compression_tests.rs @@ -472,6 +472,7 @@ fn write_rollout(path: &std::path::Path, thread_id: ThreadId, message: &str) -> model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/metadata_tests.rs b/codex-rs/rollout/src/metadata_tests.rs index 0744e7780e72..c0aea248c579 100644 --- a/codex-rs/rollout/src/metadata_tests.rs +++ b/codex-rs/rollout/src/metadata_tests.rs @@ -49,6 +49,7 @@ async fn extract_metadata_from_rollout_uses_session_meta() { model_provider: Some("openai".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, @@ -106,6 +107,7 @@ async fn extract_metadata_from_rollout_returns_latest_memory_mode() { model_provider: Some("openai".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, @@ -375,6 +377,7 @@ fn write_rollout_in_sessions_with_cwd( model_provider: Some("test-provider".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/recorder.rs b/codex-rs/rollout/src/recorder.rs index 3908604c7761..58d5c76662d5 100644 --- a/codex-rs/rollout/src/recorder.rs +++ b/codex-rs/rollout/src/recorder.rs @@ -12,6 +12,7 @@ use std::sync::Mutex; use chrono::SecondsFormat; use codex_protocol::SessionId; use codex_protocol::ThreadId; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::BaseInstructions; use serde_json::Value; @@ -91,6 +92,7 @@ pub enum RolloutRecorderParams { originator: String, base_instructions: BaseInstructions, dynamic_tools: Vec, + selected_capability_roots: Vec, multi_agent_version: Option, initial_window_id: Option, }, @@ -182,6 +184,7 @@ impl RolloutRecorderParams { originator, base_instructions, dynamic_tools, + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: None, } @@ -194,6 +197,20 @@ impl RolloutRecorderParams { self } + pub fn with_selected_capability_roots( + mut self, + selected_capability_roots: Vec, + ) -> Self { + if let Self::Create { + selected_capability_roots: roots, + .. + } = &mut self + { + *roots = selected_capability_roots; + } + self + } + pub fn with_multi_agent_version( mut self, multi_agent_version: Option, @@ -732,6 +749,7 @@ impl RolloutRecorder { originator, base_instructions, dynamic_tools, + selected_capability_roots, multi_agent_version, initial_window_id, } => { @@ -769,6 +787,7 @@ impl RolloutRecorder { } else { Some(dynamic_tools) }, + selected_capability_roots, memory_mode: (!config.generate_memories()).then_some("disabled".to_string()), multi_agent_version, context_window: initial_window_id.map(SessionContextWindow::new), diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 4b72ff76730d..71ec252c23e7 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -101,6 +101,7 @@ async fn state_db_init_backfills_before_returning() -> anyhow::Result<()> { model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/session_index_tests.rs b/codex-rs/rollout/src/session_index_tests.rs index 7f57c2ba089b..be39fb06abe6 100644 --- a/codex-rs/rollout/src/session_index_tests.rs +++ b/codex-rs/rollout/src/session_index_tests.rs @@ -41,6 +41,7 @@ fn write_rollout_with_metadata(path: &Path, thread_id: ThreadId) -> std::io::Res model_provider: Some("test-provider".into()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/state_db_tests.rs b/codex-rs/rollout/src/state_db_tests.rs index 5712b65cee22..bb341aa85aaa 100644 --- a/codex-rs/rollout/src/state_db_tests.rs +++ b/codex-rs/rollout/src/state_db_tests.rs @@ -174,6 +174,7 @@ fn write_rollout_with_user_message( model_provider: Some("test-provider".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index e6a3c728ef25..901461cf5666 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -1288,6 +1288,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { model_provider: Some("test-provider".into()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index e5ae07839e5b..81de0cc39f42 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -343,6 +343,7 @@ mod tests { model_provider: Some("openai".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, @@ -535,6 +536,7 @@ mod tests { model_provider: Some("openai".to_string()), base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index d1c2c90762ba..e5ab6fb31d9a 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -2148,6 +2148,7 @@ mod tests { model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: Some("polluted".to_string()), multi_agent_version: None, context_window: None, @@ -2211,6 +2212,7 @@ mod tests { model_provider: None, base_instructions: None, dynamic_tools: None, + selected_capability_roots: Vec::new(), memory_mode: None, multi_agent_version: None, context_window: None, diff --git a/codex-rs/thread-store/src/in_memory.rs b/codex-rs/thread-store/src/in_memory.rs index dfed2ffac297..d64510f223fd 100644 --- a/codex-rs/thread-store/src/in_memory.rs +++ b/codex-rs/thread-store/src/in_memory.rs @@ -126,6 +126,7 @@ mod tests { originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: uuid::Uuid::now_v7().to_string(), metadata: ThreadPersistenceMetadata { @@ -281,6 +282,7 @@ impl InMemoryThreadStore { model_provider: Some(params.metadata.model_provider.clone()), base_instructions: Some(params.base_instructions.clone()), dynamic_tools: (!params.dynamic_tools.is_empty()).then(|| params.dynamic_tools.clone()), + selected_capability_roots: params.selected_capability_roots.clone(), memory_mode: matches!(params.metadata.memory_mode, ThreadMemoryMode::Disabled) .then_some("disabled".to_string()), multi_agent_version: params.multi_agent_version, diff --git a/codex-rs/thread-store/src/local/create_thread.rs b/codex-rs/thread-store/src/local/create_thread.rs index eba6a5a6a99b..451931b4dc43 100644 --- a/codex-rs/thread-store/src/local/create_thread.rs +++ b/codex-rs/thread-store/src/local/create_thread.rs @@ -38,6 +38,7 @@ pub(super) async fn create_thread( params.dynamic_tools, ) .with_session_id(params.session_id) + .with_selected_capability_roots(params.selected_capability_roots) .with_multi_agent_version(params.multi_agent_version) .with_initial_window_id(params.initial_window_id), ) diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs index 51e622fc5bc4..717ae09b6931 100644 --- a/codex-rs/thread-store/src/local/mod.rs +++ b/codex-rs/thread-store/src/local/mod.rs @@ -1133,6 +1133,7 @@ mod tests { originator: "test_originator".to_string(), base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + selected_capability_roots: Vec::new(), multi_agent_version: None, initial_window_id: uuid::Uuid::now_v7().to_string(), metadata: thread_metadata(), diff --git a/codex-rs/thread-store/src/types.rs b/codex-rs/thread-store/src/types.rs index 1f761cea938c..f96a4df31b43 100644 --- a/codex-rs/thread-store/src/types.rs +++ b/codex-rs/thread-store/src/types.rs @@ -5,6 +5,7 @@ use chrono::DateTime; use chrono::Utc; use codex_protocol::SessionId; use codex_protocol::ThreadId; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::BaseInstructions; use codex_protocol::models::PermissionProfile; @@ -85,6 +86,9 @@ pub struct CreateThreadParams { pub base_instructions: BaseInstructions, /// Dynamic tools available to the thread at startup. pub dynamic_tools: Vec, + /// Environment-qualified capability roots selected for this thread. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub selected_capability_roots: Vec, /// Multi-agent runtime selected when the thread was created. pub multi_agent_version: Option, /// Initial context-window identity captured when the thread was created.