diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 844a84ef73..6fd863daf5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2325,6 +2325,7 @@ dependencies = [ "codex-cloud-tasks", "codex-config", "codex-core", + "codex-core-api", "codex-core-plugins", "codex-exec", "codex-exec-server", @@ -2738,6 +2739,7 @@ dependencies = [ "codex-model-provider-info", "codex-models-manager", "codex-protocol", + "codex-skills-extension", "codex-utils-absolute-path", ] @@ -3343,6 +3345,7 @@ dependencies = [ "codex-login", "codex-protocol", "codex-shell-command", + "codex-skills-extension", "codex-utils-absolute-path", "codex-utils-cli", "codex-utils-json-to-toml", @@ -4660,6 +4663,7 @@ dependencies = [ "codex-model-provider-info", "codex-models-manager", "codex-protocol", + "codex-skills-extension", "codex-utils-absolute-path", "codex-utils-cargo-bin", "codex-utils-path-uri", diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index 8a6bc07f3d..c91d5d38ea 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -5,6 +5,7 @@ use codex_analytics::AnalyticsEventsClient; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadGoalUpdatedNotification; +use codex_app_server_protocol::WarningNotification; use codex_core::NewThread; use codex_core::StartThreadOptions; use codex_core::ThreadManager; @@ -77,11 +78,14 @@ where 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); + // Selected executor roots take precedence over ambient host skills when a + // plain-text skill mention has the same name in both authorities. let skill_providers = codex_skills_extension::SkillProviders::new() .with_executor_provider(executor_skill_provider) .with_orchestrator_provider(Arc::new( codex_skills_extension::OrchestratorSkillProvider::new(), - )); + )) + .with_host_provider(Arc::new(codex_skills_extension::HostSkillProvider::new())); codex_skills_extension::install_with_providers( &mut builder, skill_providers, @@ -143,6 +147,23 @@ impl ExtensionEventSink for AppServerExtensionEventSink { .await; }); } + EventMsg::Warning(warning_event) => { + let Ok(thread_id) = ThreadId::from_string(&event.id) else { + tracing::warn!(event_id = %event.id, "dropping extension warning with invalid thread id"); + return; + }; + let outgoing = Arc::clone(&self.outgoing); + tokio::spawn(async move { + outgoing + .send_server_notification(ServerNotification::Warning( + WarningNotification { + thread_id: Some(thread_id.to_string()), + message: warning_event.message, + }, + )) + .await; + }); + } msg => { tracing::debug!(event_id = %event.id, ?msg, "dropping unsupported extension event"); } diff --git a/codex-rs/app-server/tests/suite/v2/executor_skills.rs b/codex-rs/app-server/tests/suite/v2/executor_skills.rs index ede5b20fb8..b6e9edde2c 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -125,12 +125,13 @@ stream_max_retries = 0 .await??; let request = response_mock.single_request(); - assert!( - request - .message_input_texts("developer") - .iter() - .any(|text| text.contains(SKILL_NAME)) - ); + let skills_catalog = request + .message_input_texts("developer") + .into_iter() + .find(|text| text.starts_with("")) + .expect("skills catalog should be model-visible"); + assert!(skills_catalog.contains("Colliding local skill.")); + assert!(skills_catalog.contains("Deploy through the executor.")); let skill_fragments = request .message_input_texts("user") .into_iter() diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 7be1146df5..e1c75198b3 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -32,6 +32,7 @@ codex-cloud-tasks = { path = "../cloud-tasks" } codex-utils-cli = { workspace = true } codex-config = { workspace = true } codex-core = { workspace = true } +codex-core-api = { workspace = true } codex-core-plugins = { workspace = true } codex-home = { workspace = true } codex-exec = { workspace = true } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index f6c5fbfb98..f8153cf3ad 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1969,6 +1969,7 @@ async fn run_debug_prompt_input_command( input, /*state_db*/ None, user_instructions_provider, + codex_core_api::default_extension_registry(), ) .await?; println!("{}", serde_json::to_string_pretty(&prompt_input)?); diff --git a/codex-rs/core-api/Cargo.toml b/codex-rs/core-api/Cargo.toml index f4c89e5640..b215a1abb2 100644 --- a/codex-rs/core-api/Cargo.toml +++ b/codex-rs/core-api/Cargo.toml @@ -27,4 +27,5 @@ codex-login = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } codex-protocol = { workspace = true } +codex-skills-extension = { workspace = true } codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index de3a7de905..5a1c646feb 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -99,3 +99,16 @@ pub use codex_protocol::protocol::TurnEnvironmentSelection; pub use codex_protocol::protocol::W3cTraceContext; pub use codex_protocol::user_input::UserInput; pub use codex_utils_absolute_path::AbsolutePathBuf; + +/// Builds the default extension registry for simple `ThreadManager` hosts. +pub fn default_extension_registry() -> std::sync::Arc> +{ + let mut builder = codex_extension_api::ExtensionRegistryBuilder::new(); + codex_skills_extension::install(&mut builder, |config: &Config| { + codex_skills_extension::SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + } + }); + std::sync::Arc::new(builder.build()) +} diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 1477a299b6..b81371a935 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -24,6 +24,7 @@ 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_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."; +const SKILLS_INTRO_WITH_MIXED_ALIASES: &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 use short paths that can be expanded with the skill roots table, `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_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. - 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. @@ -59,13 +60,49 @@ pub const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The list abov - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. - Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; +const SKILLS_HOW_TO_USE_WITH_MIXED_ALIASES: &str = r###"- Discovery: The list above is the skills available in this session (name + description + source locator). `file` locators use short paths that must be expanded with `### Skill roots`, `environment resource` locators are owned by an execution environment, `orchestrator resource` locators must be accessed through `skills.list` and `skills.read`, and `custom resource` locators use their provider's access mechanism. +- 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): + 1) After deciding to use a skill, the main agent must read its `SKILL.md` completely before taking task actions. For a `file` locator, expand the short path with `### Skill roots`; for an `environment resource`, use the owning environment; for an `orchestrator resource`, call `skills.list` with `{"authority":{"kind":"orchestrator"}}`, select the matching package, and pass its `main_resource` to `skills.read`. If a read is truncated or paginated, continue until EOF. + 2) When `SKILL.md` references another resource, use the same access mechanism. Resolve relative paths against the directory containing an expanded `file` locator. For orchestrator skills, pass the exact referenced resource identifier with the same authority and package to `skills.read`; do not treat `skill://` identifiers as filesystem paths. + 3) If `SKILL.md` points to extra folders such as `references/`, use its routing instructions to identify the resources required for the task. The main agent must read each required instruction or reference file itself before acting on it. Do not delegate reading, summarizing, or interpreting skill instructions to a subagent. Subagents may still perform task work when the selected skill allows it. + 4) For filesystem-backed skills, prefer running or patching provided scripts instead of retyping large code blocks. For orchestrator skills, use `skills.read` and the available tools; do not invent a local path. + 5) Reuse provided assets or templates through the same source access mechanism instead of recreating them. +- Coordination and sequencing: + - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them. + - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why. +- Context hygiene: + - Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file. Do not load unrelated references, scripts, or assets. + - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked. + - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. +- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum SkillCatalogMode { + HostOnly, + Mixed, +} + pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[String]) -> String { + render_available_skills_body_for_mode(skill_root_lines, skill_lines, SkillCatalogMode::HostOnly) +} + +pub fn render_available_skills_body_for_mode( + skill_root_lines: &[String], + skill_lines: &[String], + mode: SkillCatalogMode, +) -> String { let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); if skill_root_lines.is_empty() { lines.push(SKILLS_INTRO_WITH_ABSOLUTE_PATHS.to_string()); } else { - lines.push(SKILLS_INTRO_WITH_ALIASES.to_string()); + let intro = match mode { + SkillCatalogMode::HostOnly => SKILLS_INTRO_WITH_ALIASES, + SkillCatalogMode::Mixed => SKILLS_INTRO_WITH_MIXED_ALIASES, + }; + lines.push(intro.to_string()); lines.push("### Skill roots".to_string()); lines.extend(skill_root_lines.iter().cloned()); } @@ -76,7 +113,10 @@ pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[ let how_to_use = if skill_root_lines.is_empty() { SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS } else { - SKILLS_HOW_TO_USE_WITH_ALIASES + match mode { + SkillCatalogMode::HostOnly => SKILLS_HOW_TO_USE_WITH_ALIASES, + SkillCatalogMode::Mixed => SKILLS_HOW_TO_USE_WITH_MIXED_ALIASES, + } }; lines.push(how_to_use.to_string()); diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index d6eb93dd14..5d3db53ddb 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -109,11 +109,11 @@ impl codex_extension_api::ThreadLifecycleContributor for GuardianMemoryC impl codex_extension_api::ContextContributor for GuardianMemoryContextProbe { fn contribute<'a>( &'a self, - _session_store: &'a codex_extension_api::ExtensionData, - thread_store: &'a codex_extension_api::ExtensionData, + context: codex_extension_api::ContextContributionContext<'a>, ) -> codex_extension_api::ExtensionFuture<'a, Vec> { Box::pin(async move { - if thread_store + if context + .thread_store .get::() .is_some_and(|enabled| enabled.0) { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 0641d7226e..0ae718e045 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -89,11 +89,9 @@ 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/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 0da42d8a4e..c0d18e5af1 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -19,7 +19,6 @@ use crate::session::turn::built_tools; use crate::state_db_bridge::StateDbHandle; use crate::thread_manager::ThreadManager; use crate::thread_manager::thread_store_from_config; -use codex_extension_api::empty_extension_registry; /// Build the model-visible `input` list for a single debug turn. #[doc(hidden)] @@ -28,6 +27,7 @@ pub async fn build_prompt_input( input: Vec, state_db: Option, user_instructions_provider: Arc, + extensions: Arc>, ) -> CodexResult> { config.ephemeral = true; @@ -53,7 +53,7 @@ pub async fn build_prompt_input( .await .map_err(|err| CodexErr::Fatal(err.to_string()))?, ), - empty_extension_registry(), + extensions, user_instructions_provider, /*analytics_events_client*/ None, thread_store, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 0ecc706818..dfb88c89a0 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -16,7 +16,6 @@ use crate::agent::agent_status_from_event; use crate::agent::status::is_final; use crate::agents_md::LoadedAgentsMd; 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; @@ -24,14 +23,12 @@ 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::NetworkRuleSaved; use crate::context::PermissionsInstructions; use crate::context::PersonalitySpecInstructions; use crate::context::RecommendedPluginsInstructions; -use crate::default_skill_metadata_budget; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::exec_policy::ExecPolicyManager; use crate::image_preparation::prepare_response_items; @@ -39,7 +36,6 @@ use crate::parse_turn_item; use crate::realtime_conversation::RealtimeConversationManager; 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; @@ -56,6 +52,7 @@ use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ExtensionDataInit; use codex_extension_api::LoadedUserInstructions; use codex_extension_api::PromptSlot; @@ -2969,29 +2966,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 @@ -3028,10 +3002,12 @@ impl Session { let context_contributors = self.services.extensions.context_contributors().to_vec(); for contributor in context_contributors { for fragment in contributor - .contribute( - &self.services.session_extension_data, - &self.services.thread_extension_data, - ) + .contribute(ContextContributionContext { + session_store: &self.services.session_extension_data, + thread_store: &self.services.thread_extension_data, + turn_store: turn_context.extension_data.as_ref(), + model_context_window: turn_context.model_info.context_window, + }) .await { match fragment.slot() { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 724c35f2e8..8f37adf839 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -11,6 +11,7 @@ use crate::function_tool::FunctionCallError; 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; @@ -24,7 +25,6 @@ use codex_config::RequirementSource; use codex_config::Sourced; use codex_config::loader::project_trust_key; use codex_config::types::ToolSuggestDisabledTool; -use codex_core_skills::HostSkillsSnapshot; use core_test_support::test_codex::local_selections; use codex_features::Feature; @@ -7544,13 +7544,13 @@ struct PromptExtensionTestState; impl codex_extension_api::ContextContributor for PromptExtensionTestContributor { fn contribute<'a>( &'a self, - _session_store: &'a codex_extension_api::ExtensionData, - thread_store: &'a codex_extension_api::ExtensionData, + context: codex_extension_api::ContextContributionContext<'a>, ) -> std::pin::Pin< Box> + Send + 'a>, > { Box::pin(async move { - thread_store + context + .thread_store .get::() .is_some() .then(|| { @@ -7757,54 +7757,6 @@ async fn build_initial_context_omits_default_image_save_location_without_image_h ); } -#[tokio::test] -async fn build_initial_context_trims_skill_metadata_from_context_window_budget() { - let (session, mut turn_context) = make_session_and_context().await; - 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 initial_context = session.build_initial_context(&turn_context).await; - let developer_texts = developer_input_texts(&initial_context); - - assert!( - developer_texts - .iter() - .all(|text| !text.contains("Exceeded skills context budget")), - "expected skill budget warning to stay out of the initial context, got {developer_texts:?}" - ); - assert!( - developer_texts - .iter() - .all(|text| !text.contains("- admin-skill:") && !text.contains("- repo-skill:")), - "expected no skill metadata entries to fit the tiny budget, got {developer_texts:?}" - ); -} - #[test] fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { let session_telemetry = test_session_telemetry_without_metadata(); @@ -7907,61 +7859,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 _ = session.build_initial_context(&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 _ = session.build_initial_context(&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/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 76f06df1ed..fd7051cdb2 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -27,6 +27,7 @@ codex-login = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } codex-protocol = { workspace = true } +codex-skills-extension = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } codex-utils-path-uri = { workspace = true } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index daf6f79b5c..616be02eea 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -25,9 +25,9 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::LoadUserInstructionsFuture; use codex_extension_api::UserInstructionsProvider; -use codex_extension_api::empty_extension_registry; use codex_features::Feature; use codex_home::CodexHomeUserInstructionsProvider; use codex_login::CodexAuth; @@ -1141,11 +1141,22 @@ pub fn test_codex() -> TestCodexBuilder { cloud_config_bundle: None, user_shell_override: None, exec_server_url: None, - extensions: empty_extension_registry(), + extensions: default_test_extensions(), user_instructions_provider: None, } } +fn default_test_extensions() -> Arc> { + let mut builder = ExtensionRegistryBuilder::new(); + codex_skills_extension::install(&mut builder, |config: &Config| { + codex_skills_extension::SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + } + }); + Arc::new(builder.build()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/tests/suite/prompt_debug_tests.rs b/codex-rs/core/tests/suite/prompt_debug_tests.rs index 5883a8564b..74a3203403 100644 --- a/codex-rs/core/tests/suite/prompt_debug_tests.rs +++ b/codex-rs/core/tests/suite/prompt_debug_tests.rs @@ -39,6 +39,7 @@ async fn build_prompt_input_includes_context_and_user_message() -> Result<()> { }], /*state_db*/ None, user_instructions_provider, + codex_extension_api::empty_extension_registry(), ) .await?; diff --git a/codex-rs/ext/extension-api/examples/enabled_extensions.rs b/codex-rs/ext/extension-api/examples/enabled_extensions.rs index 9027c28241..130884dcde 100644 --- a/codex-rs/ext/extension-api/examples/enabled_extensions.rs +++ b/codex-rs/ext/extension-api/examples/enabled_extensions.rs @@ -7,6 +7,7 @@ use std::task::Context; use std::task::Poll; use std::task::Waker; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistryBuilder; use shared_state_extension::recorded_style_contributions; @@ -72,9 +73,19 @@ async fn contribute_prompt( session_store: &ExtensionData, thread_store: &ExtensionData, ) -> Vec { + let turn_store = ExtensionData::new("turn"); let mut fragments = Vec::new(); for contributor in registry.context_contributors() { - fragments.extend(contributor.contribute(session_store, thread_store).await); + fragments.extend( + contributor + .contribute(ContextContributionContext { + session_store, + thread_store, + turn_store: &turn_store, + model_context_window: None, + }) + .await, + ); } fragments } diff --git a/codex-rs/ext/extension-api/examples/enabled_extensions/shared_state_extension.rs b/codex-rs/ext/extension-api/examples/enabled_extensions/shared_state_extension.rs index 531f65b99e..f981056bec 100644 --- a/codex-rs/ext/extension-api/examples/enabled_extensions/shared_state_extension.rs +++ b/codex-rs/ext/extension-api/examples/enabled_extensions/shared_state_extension.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistryBuilder; @@ -19,10 +20,11 @@ struct StyleContributor; impl ContextContributor for StyleContributor { fn contribute<'a>( &'a self, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, + context: ContextContributionContext<'a>, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + let session_store = context.session_store; + let thread_store = context.thread_store; contribution_counts(session_store).record_style(); contribution_counts(thread_store).record_style(); @@ -39,10 +41,11 @@ struct UsageContributor; impl ContextContributor for UsageContributor { fn contribute<'a>( &'a self, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, + context: ContextContributionContext<'a>, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + let session_store = context.session_store; + let thread_store = context.thread_store; contribution_counts(session_store).record_usage(); contribution_counts(thread_store).record_usage(); diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 3a0fee49a9..bafb73074e 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -41,6 +41,15 @@ pub use turn_lifecycle::TurnStopInput; /// Boxed, sendable future returned by asynchronous extension contributors. pub type ExtensionFuture<'a, T> = Pin + Send + 'a>>; +/// Host context available while extensions contribute initial model input. +#[derive(Clone, Copy)] +pub struct ContextContributionContext<'a> { + pub session_store: &'a ExtensionData, + pub thread_store: &'a ExtensionData, + pub turn_store: &'a ExtensionData, + pub model_context_window: Option, +} + /// Extension contribution that resolves runtime MCP servers from host config. /// /// Contributors run in registration order. Later contributions for the same @@ -65,8 +74,7 @@ pub trait McpServerContributor: Send + Sync { pub trait ContextContributor: Send + Sync { fn contribute<'a>( &'a self, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, + context: ContextContributionContext<'a>, ) -> ExtensionFuture<'a, Vec>; } diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index e646a53a52..b46a085951 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -33,6 +33,7 @@ pub use codex_tools::parse_tool_input_schema; pub use codex_tools::parse_tool_input_schema_without_compaction; pub use contributors::ApprovalReviewContributor; pub use contributors::ConfigContributor; +pub use contributors::ContextContributionContext; pub use contributors::ContextContributor; pub use contributors::ExtensionFuture; pub use contributors::McpServerContribution; diff --git a/codex-rs/ext/extension-api/tests/registry.rs b/codex-rs/ext/extension-api/tests/registry.rs index 79773a98cf..c4cc6fcf41 100644 --- a/codex-rs/ext/extension-api/tests/registry.rs +++ b/codex-rs/ext/extension-api/tests/registry.rs @@ -5,6 +5,7 @@ use std::sync::Mutex; use codex_extension_api::ApprovalReviewContributor; use codex_extension_api::ConfigContributor; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ContextContributor; use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; @@ -36,8 +37,7 @@ struct AllContributors; impl ContextContributor for AllContributors { fn contribute<'a>( &'a self, - _session_store: &'a ExtensionData, - _thread_store: &'a ExtensionData, + _context: ContextContributionContext<'a>, ) -> ExtensionFuture<'a, Vec> { Box::pin(std::future::ready(Vec::new())) } @@ -149,8 +149,7 @@ struct NamedContextContributor(&'static str); impl ContextContributor for NamedContextContributor { fn contribute<'a>( &'a self, - _session_store: &'a ExtensionData, - _thread_store: &'a ExtensionData, + _context: ContextContributionContext<'a>, ) -> ExtensionFuture<'a, Vec> { Box::pin(std::future::ready(vec![PromptFragment::developer_policy( self.0, @@ -199,7 +198,16 @@ async fn contributors_preserve_registration_order() { let mut fragments = Vec::new(); for contributor in registry.context_contributors() { - fragments.extend(contributor.contribute(&session_store, &thread_store).await); + fragments.extend( + contributor + .contribute(ContextContributionContext { + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + model_context_window: None, + }) + .await, + ); } let mut item = TurnItem::HookPrompt(HookPromptItem { id: "item".to_string(), diff --git a/codex-rs/ext/memories/src/extension.rs b/codex-rs/ext/memories/src/extension.rs index 23c80a8779..2ef215e091 100644 --- a/codex-rs/ext/memories/src/extension.rs +++ b/codex-rs/ext/memories/src/extension.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use codex_core::config::Config; use codex_extension_api::ConfigContributor; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionFuture; @@ -50,10 +51,10 @@ impl MemoriesExtensionConfig { impl ContextContributor for MemoriesExtension { fn contribute<'a>( &'a self, - _session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, + context: ContextContributionContext<'a>, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + let thread_store = context.thread_store; let Some(config) = thread_store.get::() else { return Vec::new(); }; diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index b78c20c9f1..8b359284e5 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -1,6 +1,7 @@ use std::path::Path; use std::sync::Arc; +use codex_extension_api::ContextContributionContext; use codex_extension_api::ContextContributor; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistryBuilder; @@ -179,9 +180,16 @@ async fn prompt_contribution_uses_memory_summary_when_enabled() { dedicated_tools: false, codex_home: tempdir.path().abs(), }); + let session_store = ExtensionData::new("session"); + let turn_store = ExtensionData::new("turn"); let fragments = extension - .contribute(&ExtensionData::new("session"), &thread_store) + .contribute(ContextContributionContext { + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + model_context_window: None, + }) .await; assert_eq!(fragments.len(), 1); diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index c82ce66423..d16f5ccba7 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -4,6 +4,7 @@ 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::ContextContributionContext; use codex_extension_api::ContextContributor; use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; @@ -105,10 +106,11 @@ where { fn contribute<'a>( &'a self, - session_store: &'a ExtensionData, - thread_store: &'a ExtensionData, + context: ContextContributionContext<'a>, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + let session_store = context.session_store; + let thread_store = context.thread_store; let Some(thread_state) = thread_store.get::() else { return Vec::new(); }; @@ -116,13 +118,14 @@ where if !config.include_instructions { return Vec::new(); } + let host_snapshot = context.turn_store.get::(); let catalog = self .list_skills( SkillListQuery { - turn_id: thread_store.level_id().to_string(), + turn_id: context.turn_store.level_id().to_string(), executor_roots: thread_state.selected_roots().to_vec(), - host_snapshot: None, - include_host_skills: false, + 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::(), @@ -133,10 +136,17 @@ where 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() + let Some((fragment, warning)) = available_skills_fragment( + &catalog, + host_snapshot.as_deref(), + context.model_context_window, + ) else { + return Vec::new(); + }; + if let Some(warning) = warning { + self.emit_warning(thread_store.level_id(), warning); + } + vec![PromptFragment::developer_capability(fragment.render())] }) } } @@ -189,7 +199,7 @@ where turn_id: input.turn_id.clone(), executor_roots: thread_state.selected_roots().to_vec(), host_snapshot: host_snapshot.clone(), - include_host_skills: true, + include_host_skills: false, include_bundled_skills: config.bundled_skills_enabled, include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), mcp_resources: session_store.get::(), @@ -201,17 +211,6 @@ where 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(); diff --git a/codex-rs/ext/skills/src/fragments.rs b/codex-rs/ext/skills/src/fragments.rs index 0a24b48b5a..f8f591745f 100644 --- a/codex-rs/ext/skills/src/fragments.rs +++ b/codex-rs/ext/skills/src/fragments.rs @@ -1,16 +1,41 @@ -use codex_core_skills::render_available_skills_body; +use codex_core_skills::AvailableSkills; +use codex_core_skills::render::SkillCatalogMode; +use codex_core_skills::render::render_available_skills_body_for_mode; 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_root_lines: Vec, skill_lines: Vec, + mode: SkillCatalogMode, } impl AvailableSkillsInstructions { - pub(crate) fn from_skill_lines(skill_lines: Vec) -> Self { - Self { skill_lines } + pub(crate) fn from_skill_lines(skill_lines: Vec, mode: SkillCatalogMode) -> Self { + Self { + skill_root_lines: Vec::new(), + skill_lines, + mode, + } + } + + pub(crate) fn from_available_skills_with_additional_lines( + mut available: AvailableSkills, + additional_lines: Vec, + ) -> Self { + let mode = if additional_lines.is_empty() { + SkillCatalogMode::HostOnly + } else { + SkillCatalogMode::Mixed + }; + available.skill_lines.extend(additional_lines); + Self { + skill_root_lines: available.skill_root_lines, + skill_lines: available.skill_lines, + mode, + } } } @@ -28,7 +53,7 @@ impl ContextualUserFragment for AvailableSkillsInstructions { } fn body(&self) -> String { - render_available_skills_body(&[], &self.skill_lines) + render_available_skills_body_for_mode(&self.skill_root_lines, &self.skill_lines, self.mode) } } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index d913e4161c..94787570fc 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -4,6 +4,11 @@ use crate::catalog::SkillCatalog; use crate::catalog::SkillCatalogEntry; use crate::catalog::SkillSourceKind; use crate::fragments::AvailableSkillsInstructions; +use codex_core_skills::HostSkillsSnapshot; +use codex_core_skills::build_available_skills; +use codex_core_skills::default_skill_metadata_budget; +use codex_core_skills::render::SkillCatalogMode; +use codex_core_skills::render::SkillRenderSideEffects; const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; const MAX_MAIN_PROMPT_BYTES: usize = 8_000; @@ -12,16 +17,76 @@ pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; pub(crate) fn available_skills_fragment( catalog: &SkillCatalog, -) -> Option { + host_snapshot: Option<&HostSkillsSnapshot>, + model_context_window: Option, +) -> Option<(AvailableSkillsInstructions, Option)> { + let visible_entries = catalog + .entries + .iter() + .filter(|entry| entry.enabled && entry.prompt_visible) + .collect::>(); + if visible_entries + .iter() + .any(|entry| entry.authority.kind == SkillSourceKind::Host) + && let Some(host_snapshot) = host_snapshot + { + let external_lines = bounded_skill_lines( + visible_entries + .iter() + .copied() + .filter(|entry| entry.authority.kind != SkillSourceKind::Host), + ); + if let Some(available) = build_available_skills( + host_snapshot.outcome(), + default_skill_metadata_budget(model_context_window), + SkillRenderSideEffects::None, + ) { + let warning = available.warning_message.clone(); + return Some(( + AvailableSkillsInstructions::from_available_skills_with_additional_lines( + available, + external_lines, + ), + warning, + )); + } + if !external_lines.is_empty() { + return Some(( + AvailableSkillsInstructions::from_skill_lines( + external_lines, + SkillCatalogMode::Mixed, + ), + None, + )); + } + return None; + } + + let mode = if visible_entries + .iter() + .any(|entry| entry.authority.kind != SkillSourceKind::Host) + { + SkillCatalogMode::Mixed + } else { + SkillCatalogMode::HostOnly + }; + let skill_lines = bounded_skill_lines(visible_entries); + (!skill_lines.is_empty()).then(|| { + ( + AvailableSkillsInstructions::from_skill_lines(skill_lines, mode), + None, + ) + }) +} + +fn bounded_skill_lines<'a>( + entries: impl IntoIterator, +) -> Vec { 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) - { + for entry in entries { let description = entry .short_description .as_deref() @@ -36,17 +101,13 @@ pub(crate) fn available_skills_fragment( 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)) + skill_lines } fn render_skill_line(entry: &SkillCatalogEntry, description: &str) -> String { diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index cd6f46d9c9..65fd40803d 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -9,7 +9,7 @@ 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::ContextContributionContext; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; @@ -49,10 +49,10 @@ 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"; + "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse [$calendar](app://calendar).\n"; #[tokio::test] -async fn installed_extension_uses_host_service_snapshot() -> TestResult { +async fn installed_extension_catalog_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( @@ -99,44 +99,115 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult { 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, - ) + let prompt_fragments = registry.context_contributors()[0] + .contribute(prompt_context(&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)); + assert_eq!(1, prompt_fragments.len()); + assert_eq!(expected_catalog, prompt_fragments[0].text()); std::fs::remove_dir_all(codex_home)?; Ok(()) } +#[tokio::test] +async fn mixed_catalog_preserves_host_budget_and_external_locator() -> TestResult { + let host_path = + AbsolutePathBuf::try_from(std::env::temp_dir().join("host-demo").join("SKILL.md"))?; + let mut outcome = SkillLoadOutcome::default(); + outcome.skills.push(SkillMetadata { + name: "host-demo".to_string(), + description: "x".repeat(5_000), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: host_path, + scope: SkillScope::User, + plugin_id: None, + }); + let providers = SkillProviders::new() + .with_host_provider(Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![test_entry( + SkillSourceKind::Host, + "host", + "host/host-demo", + "host-demo/SKILL.md", + )], + warnings: Vec::new(), + }, + read_requests: Arc::new(Mutex::new(Vec::new())), + list_calls: None, + fail_first_list: false, + })) + .with_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::new(Mutex::new(Vec::new())), + list_calls: None, + fail_first_list: false, + })); + 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"); + thread_store.insert(vec![SelectedCapabilityRoot { + id: "lint-fix".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "env-1".to_string(), + path: "/skills/lint-fix".to_string(), + }, + }]); + 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 turn_store = ExtensionData::new("turn-1"); + turn_store.insert(HostSkillsSnapshot::new(Arc::new(outcome))); + + let fragments = registry.context_contributors()[0] + .contribute(ContextContributionContext { + session_store: &session_store, + thread_store: &thread_store, + turn_store: &turn_store, + model_context_window: Some(10_000), + }) + .await; + + assert_eq!(1, fragments.len()); + let text = fragments[0].text(); + assert!(text.contains("host-demo")); + assert!(text.contains("environment resource: skill://executor/lint-fix/SKILL.md")); + let EventMsg::Warning(warning) = event_rx.try_recv()?.msg else { + panic!("expected host budget warning"); + }; + assert!(warning.message.contains("2% skills context budget")); + + Ok(()) +} + #[tokio::test] async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_input() -> TestResult { @@ -183,7 +254,11 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in .await; let prompt_fragments = registry.context_contributors()[0] - .contribute(&session_store, &thread_store) + .contribute(prompt_context( + &session_store, + &thread_store, + &ExtensionData::new("initial-turn"), + )) .await; assert_eq!(1, prompt_fragments.len()); assert!( @@ -228,7 +303,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in read_request_keys(&read_requests) ); let rebuilt_prompt_fragments = registry.context_contributors()[0] - .contribute(&session_store, &thread_store) + .contribute(prompt_context(&session_store, &thread_store, &turn_store)) .await; assert_eq!(1, rebuilt_prompt_fragments.len()); assert!(rebuilt_prompt_fragments[0].text().contains("lint-fix")); @@ -294,7 +369,11 @@ async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { .await; let initial_fragments = registry.context_contributors()[0] - .contribute(&session_store, &thread_store) + .contribute(prompt_context( + &session_store, + &thread_store, + &ExtensionData::new("initial-turn"), + )) .await; assert!(initial_fragments.is_empty()); let EventMsg::Warning(warning) = event_rx.try_recv()?.msg else { @@ -422,16 +501,16 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { catalog: SkillCatalog { entries: vec![ test_entry( - SkillSourceKind::Host, - "host", - "host/visible-skill", - "visible-skill/SKILL.md", + SkillSourceKind::Orchestrator, + "codex_apps", + "orchestrator/visible-skill", + "skill://orchestrator/visible-skill/SKILL.md", ), test_entry( - SkillSourceKind::Host, - "host", - "host/hidden-skill", - "hidden-skill/SKILL.md", + SkillSourceKind::Orchestrator, + "codex_apps", + "orchestrator/hidden-skill", + "skill://orchestrator/hidden-skill/SKILL.md", ) .hidden_from_prompt(), ], @@ -441,7 +520,7 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { list_calls: None, fail_first_list: false, }); - let providers = SkillProviders::new().with_host_provider(provider); + let providers = SkillProviders::new().with_orchestrator_provider(provider); let mut builder = ExtensionRegistryBuilder::new(); install_with_providers(&mut builder, providers, skills_extension_config); let registry = builder.build(); @@ -460,6 +539,17 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { }) .await; + let initial_fragments = registry.context_contributors()[0] + .contribute(prompt_context( + &session_store, + &thread_store, + &ExtensionData::new("initial-turn"), + )) + .await; + assert_eq!(1, initial_fragments.len()); + assert!(initial_fragments[0].text().contains("visible-skill")); + assert!(!initial_fragments[0].text().contains("hidden-skill")); + let fragments = registry.turn_input_contributors()[0] .contribute( TurnInputContext { @@ -476,15 +566,13 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult { ) .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!(1, fragments.len()); + assert!(fragments[0].render().contains("hidden-skill")); assert_eq!( vec![( - SkillAuthority::new(SkillSourceKind::Host, "host"), - SkillPackageId("host/hidden-skill".to_string()), - SkillResourceId::new("hidden-skill/SKILL.md"), + SkillAuthority::new(SkillSourceKind::Orchestrator, "codex_apps"), + SkillPackageId("orchestrator/hidden-skill".to_string()), + SkillResourceId::new("skill://orchestrator/hidden-skill/SKILL.md"), )], read_request_keys(&read_requests) ); @@ -605,3 +693,16 @@ fn read_request_keys( }) .collect() } + +fn prompt_context<'a>( + session_store: &'a ExtensionData, + thread_store: &'a ExtensionData, + turn_store: &'a ExtensionData, +) -> ContextContributionContext<'a> { + ContextContributionContext { + session_store, + thread_store, + turn_store, + model_context_window: None, + } +} diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 9d568722e8..013be7e2d9 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -26,6 +26,7 @@ codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } codex-login = { workspace = true } codex-protocol = { workspace = true } +codex-skills-extension = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-json-to-toml = { workspace = true } rmcp = { workspace = true } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index ce19575a9c..073fe1bf73 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -6,12 +6,15 @@ use codex_core::StateDbHandle; use codex_core::ThreadManager; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; -use codex_extension_api::empty_extension_registry; +use codex_extension_api::ExtensionEventSink; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_home::CodexHomeUserInstructionsProvider; use codex_login::AuthManager; use codex_login::default_client::USER_AGENT_SUFFIX; use codex_login::default_client::get_codex_user_agent; use codex_protocol::ThreadId; +use codex_protocol::protocol::Event; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::Submission; use rmcp::model::CallToolRequestParams; @@ -37,6 +40,7 @@ use crate::codex_tool_config::CodexToolCallReplyParam; use crate::codex_tool_config::create_tool_for_codex_tool_call_param; use crate::codex_tool_config::create_tool_for_codex_tool_call_reply_param; use crate::outgoing_message::OutgoingMessageSender; +use crate::outgoing_message::OutgoingNotificationMeta; pub(crate) struct MessageProcessor { outgoing: Arc, @@ -66,12 +70,16 @@ impl MessageProcessor { let user_instructions_provider = Arc::new(CodexHomeUserInstructionsProvider::new( config.codex_home.clone(), )); + let running_requests_id_to_codex_uuid = Arc::new(Mutex::new(HashMap::new())); let thread_manager = Arc::new(ThreadManager::new( config.as_ref(), auth_manager, SessionSource::Mcp, environment_manager, - empty_extension_registry(), + default_extensions( + Arc::clone(&outgoing), + Arc::clone(&running_requests_id_to_codex_uuid), + ), user_instructions_provider, /*analytics_events_client*/ None, codex_core::thread_store_from_config(config.as_ref(), state_db.clone()), @@ -84,7 +92,7 @@ impl MessageProcessor { initialized: false, arg0_paths, thread_manager, - running_requests_id_to_codex_uuid: Arc::new(Mutex::new(HashMap::new())), + running_requests_id_to_codex_uuid, } } @@ -571,3 +579,60 @@ impl MessageProcessor { tracing::info!("notifications/initialized"); } } + +fn default_extensions( + outgoing: Arc, + running_requests: Arc>>, +) -> Arc> { + let event_sink = Arc::new(McpServerExtensionEventSink { + outgoing, + running_requests, + }); + let mut builder = ExtensionRegistryBuilder::with_event_sink(event_sink); + codex_skills_extension::install(&mut builder, |config: &Config| { + codex_skills_extension::SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + } + }); + Arc::new(builder.build()) +} + +struct McpServerExtensionEventSink { + outgoing: Arc, + running_requests: Arc>>, +} + +impl ExtensionEventSink for McpServerExtensionEventSink { + fn emit(&self, event: Event) { + let Ok(thread_id) = ThreadId::from_string(&event.id) else { + tracing::warn!(event_id = %event.id, "dropping extension event with invalid thread id"); + return; + }; + let outgoing = Arc::clone(&self.outgoing); + let running_requests = Arc::clone(&self.running_requests); + tokio::spawn(async move { + let request_id = + running_requests + .lock() + .await + .iter() + .find_map(|(request_id, running_thread_id)| { + (*running_thread_id == thread_id).then(|| request_id.clone()) + }); + outgoing + .send_event_as_notification( + &event, + Some(OutgoingNotificationMeta { + request_id, + thread_id: Some(thread_id), + }), + ) + .await; + }); + } +} + +#[cfg(test)] +#[path = "message_processor_tests.rs"] +mod tests; diff --git a/codex-rs/mcp-server/src/message_processor_tests.rs b/codex-rs/mcp-server/src/message_processor_tests.rs new file mode 100644 index 0000000000..8ceaf013a2 --- /dev/null +++ b/codex-rs/mcp-server/src/message_processor_tests.rs @@ -0,0 +1,50 @@ +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::WarningEvent; +use pretty_assertions::assert_eq; +use serde_json::json; +use tokio::sync::mpsc; + +use super::*; +use crate::outgoing_message::OutgoingMessage; +use crate::outgoing_message::OutgoingNotification; + +#[tokio::test] +async fn extension_event_sink_forwards_warning_with_request_and_thread_metadata() { + let (outgoing_tx, mut outgoing_rx) = mpsc::unbounded_channel(); + let outgoing = Arc::new(OutgoingMessageSender::new(outgoing_tx)); + let thread_id = ThreadId::new(); + let request_id = RequestId::String("request-1".into()); + let running_requests = Arc::new(Mutex::new(HashMap::from([(request_id.clone(), thread_id)]))); + let sink = McpServerExtensionEventSink { + outgoing, + running_requests, + }; + + sink.emit(Event { + id: thread_id.to_string(), + msg: EventMsg::Warning(WarningEvent { + message: "skill warning".to_string(), + }), + }); + + let OutgoingMessage::Notification(OutgoingNotification { method, params }) = + outgoing_rx.recv().await.expect("notification") + else { + panic!("expected extension notification"); + }; + assert_eq!(method, "codex/event"); + assert_eq!( + params, + Some(json!({ + "_meta": { + "requestId": request_id, + "threadId": thread_id, + }, + "id": thread_id.to_string(), + "msg": { + "type": "warning", + "message": "skill warning", + }, + })) + ); +} diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index e2d803ce76..cbda64c605 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -55,7 +55,7 @@ use codex_core_api::UserInput; use codex_core_api::WebSearchMode; use codex_core_api::arg0_dispatch_or_else; use codex_core_api::built_in_model_providers; -use codex_core_api::empty_extension_registry; +use codex_core_api::default_extension_registry; use codex_core_api::find_codex_home; use codex_core_api::init_state_db; use codex_core_api::item_event_to_server_notification; @@ -129,7 +129,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { auth_manager, SessionSource::Exec, environment_manager, - empty_extension_registry(), + default_extension_registry(), user_instructions_provider, /*analytics_events_client*/ None, Arc::clone(&thread_store),