From 0f2321f2444c4e2e1b67b746ba916580fcd8db82 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 15 Jul 2026 20:51:03 +0000 Subject: [PATCH] Refresh host skill catalogs through world state (#33425) ## Why Host skills can change after a thread starts. Subsequent turns need the updated catalog without reinjecting an unchanged catalog on every turn. ## What changed - Register the host skill provider in app-server and project its catalog through a dedicated World State section. - Render host skills with the existing metadata budget and usage instructions, including skill-root aliases when available. - Suppress the legacy host catalog and contextual fragment when World State owns the catalog, while preserving host skills for explicit and shadow selection. ## Testing Added an app-server test that installs a host skill at runtime and verifies the catalog updates once and remains stable on the following turn. GitOrigin-RevId: 4cacc6b2258fc3cab8499fbdf9aa6ab0324ec10d --- codex-rs/app-server/src/extensions.rs | 3 +- .../app-server/tests/suite/v2/host_skills.rs | 186 ++++++++++++++++++ codex-rs/app-server/tests/suite/v2/mod.rs | 1 + codex-rs/core-skills/src/injection.rs | 8 + codex-rs/core/src/session/mod.rs | 9 +- codex-rs/ext/skills/src/extension.rs | 55 ++++-- codex-rs/ext/skills/src/fragments.rs | 30 ++- codex-rs/ext/skills/src/sources.rs | 11 ++ codex-rs/ext/skills/src/world_state.rs | 81 ++++++++ .../ext/skills/tests/implicit_invocation.rs | 5 +- 10 files changed, 366 insertions(+), 23 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/v2/host_skills.rs diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index 31af1d6507..86258e6620 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -83,7 +83,8 @@ where .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_and_metrics( &mut builder, skill_providers, diff --git a/codex-rs/app-server/tests/suite/v2/host_skills.rs b/codex-rs/app-server/tests/suite/v2/host_skills.rs new file mode 100644 index 0000000000..e06ba6d043 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/host_skills.rs @@ -0,0 +1,186 @@ +use std::time::Duration; + +use anyhow::Result; +use app_test_support::TestAppServer; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SkillsExtraRootsSetParams; +use codex_app_server_protocol::SkillsExtraRootsSetResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::UserInput; +use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::responses; +use core_test_support::skip_if_remote; +use pretty_assertions::assert_eq; +use tempfile::TempDir; +use tokio::time::timeout; + +const READ_TIMEOUT: Duration = Duration::from_secs(30); +const INITIAL_SKILL_DESCRIPTION: &str = "INITIAL_HOST_SKILL_DESCRIPTION"; +const RUNTIME_SKILL_DESCRIPTION: &str = "RUNTIME_HOST_SKILL_DESCRIPTION"; + +#[tokio::test] +async fn host_skill_catalog_refreshes_once_when_skills_change() -> Result<()> { + skip_if_remote!( + Ok(()), + "host-local skill changes are not visible to remote executors" + ); + + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_sequence( + &server, + (1..=3) + .map(|index| { + let response_id = format!("resp-{index}"); + let message_id = format!("msg-{index}"); + responses::sse(vec![ + responses::ev_response_created(&response_id), + responses::ev_assistant_message(&message_id, "Done"), + responses::ev_completed(&response_id), + ]) + }) + .collect(), + ) + .await; + + let codex_home = TempDir::new()?; + let extra_root = TempDir::new()?; + let extra_skills_root = extra_root.path().join("skills"); + write_skill( + &extra_skills_root, + "initial-host-skill", + INITIAL_SKILL_DESCRIPTION, + )?; + std::fs::write( + codex_home.path().join("config.toml"), + format!( + r#" +model = "mock-model" +approval_policy = "never" +sandbox_mode = "read-only" +model_provider = "mock_provider" + +[skills] +include_instructions = true + +[skills.bundled] +enabled = false + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"#, + server.uri() + ), + )?; + + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build() + .await?; + timeout(READ_TIMEOUT, app_server.initialize()).await??; + + let request_id = app_server + .send_thread_start_request_with_auto_env(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let response: JSONRPCResponse = timeout( + READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(response)?; + + set_extra_roots(&mut app_server, &extra_skills_root).await?; + run_turn(&mut app_server, &thread.id, "Initial catalog").await?; + + write_skill( + &extra_skills_root, + "runtime-host-skill", + RUNTIME_SKILL_DESCRIPTION, + )?; + set_extra_roots(&mut app_server, &extra_skills_root).await?; + run_turn(&mut app_server, &thread.id, "After install").await?; + run_turn(&mut app_server, &thread.id, "Unchanged follow-up").await?; + + let requests = response_mock.requests(); + assert_eq!(3, requests.len()); + let marker_counts = |marker| { + requests + .iter() + .map(|request| { + request + .message_input_texts("developer") + .iter() + .map(|text| text.matches(marker).count()) + .sum::() + }) + .collect::>() + }; + assert_eq!(vec![1, 2, 2], marker_counts(INITIAL_SKILL_DESCRIPTION)); + assert_eq!(vec![0, 1, 1], marker_counts(RUNTIME_SKILL_DESCRIPTION)); + + Ok(()) +} + +fn write_skill(root: &std::path::Path, name: &str, description: &str) -> Result<()> { + let skill_dir = root.join(name); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n"), + )?; + Ok(()) +} + +async fn set_extra_roots(app_server: &mut TestAppServer, root: &std::path::Path) -> Result<()> { + let request_id = app_server + .send_skills_extra_roots_set_request(SkillsExtraRootsSetParams { + extra_roots: vec![AbsolutePathBuf::from_absolute_path(root)?], + }) + .await?; + let response: JSONRPCResponse = timeout( + READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let _: SkillsExtraRootsSetResponse = to_response(response)?; + timeout( + READ_TIMEOUT, + app_server.read_stream_until_notification_message("skills/changed"), + ) + .await??; + Ok(()) +} + +async fn run_turn(app_server: &mut TestAppServer, thread_id: &str, prompt: &str) -> Result<()> { + let request_id = app_server + .send_turn_start_request(TurnStartParams { + thread_id: thread_id.to_string(), + input: vec![UserInput::Text { + text: prompt.to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + timeout( + READ_TIMEOUT, + app_server.read_stream_until_notification_message("turn/completed"), + ) + .await??; + Ok(()) +} diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index f25a9118a6..ffb1e26c86 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -26,6 +26,7 @@ mod experimental_feature_list; mod external_agent_config; mod fs; mod hooks_list; +mod host_skills; mod imagegen_extension; mod initialize; mod marketplace_add; diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 201e7959fb..3452f94e25 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -39,6 +39,14 @@ pub struct InjectedHostSkillPrompts { paths: HashSet, } +/// Marks a turn whose skills extension projects the host skill catalog through +/// WorldState. +/// +/// Core uses this to keep its legacy thread-start catalog from duplicating the +/// extension-owned catalog. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct HostSkillsCatalogInWorldState; + impl InjectedHostSkillPrompts { pub fn insert_path(&mut self, path: impl Into) { let path = path.into(); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 941be63427..3107955ec4 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -56,6 +56,7 @@ use codex_analytics::TurnCodexErrorFact; use codex_config::types::AuthKeyringBackendKind; use codex_config::types::OAuthCredentialsStoreMode; use codex_connectors::connector_runtime_context_key; +use codex_core_skills::injection::HostSkillsCatalogInWorldState; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; use codex_extension_api::ExtensionDataInit; @@ -3257,6 +3258,10 @@ impl Session { } } if turn_context.config.include_skill_instructions { + let host_catalog_in_world_state = turn_context + .extension_data + .get::() + .is_some(); let available_skills = build_available_skills( turn_context.turn_skills.snapshot.outcome(), default_skill_metadata_budget(turn_context.model_info.context_window), @@ -3279,7 +3284,9 @@ impl Session { }) .await; } - developer_sections.push(skills_instructions.render()); + if !host_catalog_in_world_state { + developer_sections.push(skills_instructions.render()); + } } } let loaded_plugins = self diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index 8e877ac8a5..2b0ca8307a 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -1,6 +1,8 @@ use std::sync::Arc; use codex_core_skills::HostSkillsSnapshot; +use codex_core_skills::default_skill_metadata_budget; +use codex_core_skills::injection::HostSkillsCatalogInWorldState; use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_extension_api::ConfigContributor; @@ -38,7 +40,6 @@ use crate::catalog::SkillSourceKind; use crate::fragments::SkillInstructions; use crate::provider::HostSkillProvider; use crate::provider::SkillListQuery; -use crate::provider::SkillProvider; use crate::provider::SkillReadRequest; use crate::render::MAX_SKILL_NAME_BYTES; use crate::render::MAX_SKILL_PATH_BYTES; @@ -53,6 +54,7 @@ use crate::state::SkillsThreadState; use crate::state::SkillsTurnState; use crate::tools::skill_tools; use crate::world_state::executor_skills_world_state_section; +use crate::world_state::host_skills_world_state_section; struct SkillsExtension { providers: SkillProviders, @@ -173,15 +175,31 @@ where input .turn_store .insert(ExecutorSkillsStepState(catalog.clone())); - let include_usage = input - .thread_store - .get::() + let model_info = input.thread_store.get::(); + let include_usage = model_info + .as_deref() .is_some_and(|model_info| model_info.include_skills_usage_instructions); - vec![executor_skills_world_state_section( + let mut sections = vec![executor_skills_world_state_section( &catalog, config.include_instructions, include_usage, - )] + )]; + if let Some(host_snapshot) = input.turn_store.get::() + && self.providers.has_host_provider() + { + input.turn_store.insert(HostSkillsCatalogInWorldState); + sections.push(host_skills_world_state_section( + &host_snapshot, + config.include_instructions, + include_usage, + default_skill_metadata_budget( + model_info + .as_deref() + .and_then(|model_info| model_info.context_window), + ), + )); + } + sections }) } } @@ -257,33 +275,32 @@ where let config = thread_state.config(); let host_snapshot = turn_store.get::(); + let host_catalog_in_world_state = + turn_store.get::().is_some(); let query = SkillListQuery { turn_id: input.turn_id.clone(), executor_roots: Vec::new(), host_snapshot: host_snapshot.clone(), - include_host_skills: true, + include_host_skills: !host_catalog_in_world_state, include_bundled_skills: config.bundled_skills_enabled, include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), mcp_resources: session_store.get::(), }; let host_query = query.clone(); - let mut catalog = self.list_skills(query, &thread_state).await; - if let Some(executor_skills) = turn_store.get::() { - catalog.extend(executor_skills.0.clone()); - } + let mut catalog = turn_store + .get::() + .map(|executor_skills| executor_skills.0.clone()) + .unwrap_or_default(); + catalog.extend(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 shadow_selection_turn = if config.shadow_selection_enabled { - // App-server leaves host discovery in core, so its configured providers omit the - // host snapshot. Extend only the experiment catalog to keep rendered context intact. let mut shadow_catalog = catalog.clone(); - if host_snapshot.is_some() - && let Ok(host_catalog) = HostSkillProvider::new().list(host_query).await - { - shadow_catalog.extend(host_catalog); + if host_catalog_in_world_state && host_snapshot.is_some() { + shadow_catalog.extend(self.providers.list_host_for_turn(host_query).await); } Some( self.shadow_selection @@ -295,7 +312,9 @@ where thread_state .replace_shadow_selection_turn(input.turn_id.clone(), shadow_selection_turn); let mut fragments: Vec> = Vec::new(); - if config.include_instructions { + if config.include_instructions + && turn_store.get::().is_none() + { let mut turn_catalog = catalog.clone(); turn_catalog.entries.retain(|entry| { entry.authority.kind != SkillSourceKind::Executor diff --git a/codex-rs/ext/skills/src/fragments.rs b/codex-rs/ext/skills/src/fragments.rs index 6769ffbcb1..7f0e063da3 100644 --- a/codex-rs/ext/skills/src/fragments.rs +++ b/codex-rs/ext/skills/src/fragments.rs @@ -1,4 +1,6 @@ +use codex_core_skills::AvailableSkills; use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; +use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ALIASES; use codex_core_skills::render_available_skills_body; use codex_extension_api::ContextualUserFragment; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; @@ -6,6 +8,7 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct AvailableSkillsInstructions { + skill_root_lines: Vec, skill_lines: Vec, } @@ -18,7 +21,30 @@ impl AvailableSkillsInstructions { skill_lines.push("### How to use skills".to_string()); skill_lines.push(SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS.to_string()); } - Self { skill_lines } + Self { + skill_root_lines: Vec::new(), + skill_lines, + } + } + + pub(crate) fn from_available_skills( + available: AvailableSkills, + include_skills_usage_instructions: bool, + ) -> Self { + let mut skill_lines = available.skill_lines; + if include_skills_usage_instructions { + skill_lines.push("### How to use skills".to_string()); + let instructions = if available.skill_root_lines.is_empty() { + SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS + } else { + SKILLS_HOW_TO_USE_WITH_ALIASES + }; + skill_lines.push(instructions.to_string()); + } + Self { + skill_root_lines: available.skill_root_lines, + skill_lines, + } } } @@ -36,7 +62,7 @@ impl ContextualUserFragment for AvailableSkillsInstructions { } fn body(&self) -> String { - render_available_skills_body(&[], &self.skill_lines) + render_available_skills_body(&self.skill_root_lines, &self.skill_lines) } } diff --git a/codex-rs/ext/skills/src/sources.rs b/codex-rs/ext/skills/src/sources.rs index a75a047e09..dc7ec24ac3 100644 --- a/codex-rs/ext/skills/src/sources.rs +++ b/codex-rs/ext/skills/src/sources.rs @@ -107,6 +107,12 @@ impl SkillProviders { .any(|source| source.kind == SkillSourceKind::Orchestrator) } + pub(crate) fn has_host_provider(&self) -> bool { + self.sources + .iter() + .any(|source| source.kind == SkillSourceKind::Host) + } + pub(crate) async fn list_for_turn(&self, query: SkillListQuery) -> SkillCatalog { self.list_matching(&query, |source| source.should_list(&query)) .await @@ -140,6 +146,11 @@ impl SkillProviders { .await } + pub(crate) async fn list_host_for_turn(&self, query: SkillListQuery) -> SkillCatalog { + self.list_matching(&query, |source| source.kind == SkillSourceKind::Host) + .await + } + async fn list_matching( &self, query: &SkillListQuery, diff --git a/codex-rs/ext/skills/src/world_state.rs b/codex-rs/ext/skills/src/world_state.rs index 1084bff6d5..80a1b00777 100644 --- a/codex-rs/ext/skills/src/world_state.rs +++ b/codex-rs/ext/skills/src/world_state.rs @@ -1,3 +1,7 @@ +use codex_core_skills::HostSkillsSnapshot; +use codex_core_skills::SkillMetadataBudget; +use codex_core_skills::build_available_skills; +use codex_core_skills::render::SkillRenderSideEffects; use codex_extension_api::ContextualUserFragment; use codex_extension_api::PreviousWorldStateSection; use codex_extension_api::RenderedWorldStateFragment; @@ -7,12 +11,19 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use serde_json::json; use crate::catalog::SkillCatalog; +use crate::fragments::AvailableSkillsInstructions; use crate::render::available_skills_fragment; pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills"; +pub(crate) const HOST_SKILLS_WORLD_STATE_ID: &str = "host_skills"; +const MAX_HOST_SKILLS_METADATA_CHARS: usize = 8_000; +const MAX_HOST_SKILLS_METADATA_TOKENS: usize = 4_000; const NO_EXECUTOR_SKILLS_BODY: &str = "\n## Skills update\nNo selected-environment skills are currently available.\n"; const HIDDEN_EXECUTOR_SKILLS_BODY: &str = "\n## Skills update\nSelected-environment skills are not listed automatically. Explicit skill mentions can still be resolved when available.\n"; +const NO_HOST_SKILLS_BODY: &str = + "\n## Host skills update\nNo host skills are currently available.\n"; +const HIDDEN_HOST_SKILLS_BODY: &str = "\n## Host skills update\nHost skills are not listed automatically. Explicit skill mentions can still be resolved when available.\n"; pub(crate) fn executor_skills_world_state_section( catalog: &SkillCatalog, @@ -70,3 +81,73 @@ pub(crate) fn executor_skills_world_state_section( None => contribution, } } + +pub(crate) fn host_skills_world_state_section( + host_snapshot: &HostSkillsSnapshot, + include_instructions: bool, + include_skills_usage_instructions: bool, + metadata_budget: SkillMetadataBudget, +) -> WorldStateSectionContribution { + let outcome = host_snapshot.outcome(); + let metadata_budget = match metadata_budget { + SkillMetadataBudget::Tokens(limit) => { + SkillMetadataBudget::Tokens(limit.min(MAX_HOST_SKILLS_METADATA_TOKENS)) + } + SkillMetadataBudget::Characters(limit) => { + SkillMetadataBudget::Characters(limit.min(MAX_HOST_SKILLS_METADATA_CHARS)) + } + }; + let available = if include_instructions { + build_available_skills(outcome, metadata_budget, SkillRenderSideEffects::None) + } else { + None + }; + let body = available.map(|available| { + AvailableSkillsInstructions::from_available_skills( + available, + include_skills_usage_instructions, + ) + .body() + }); + let snapshot = json!({ + "body": body, + "includeInstructions": include_instructions, + }); + let retained_fragment = body + .as_ref() + .map(|body| format!("{SKILLS_INSTRUCTIONS_OPEN_TAG}{body}{SKILLS_INSTRUCTIONS_CLOSE_TAG}")); + + let contribution = + WorldStateSectionContribution::new(HOST_SKILLS_WORLD_STATE_ID, snapshot, move |previous| { + let previous_is_absent = matches!(&previous, PreviousWorldStateSection::Absent); + if let PreviousWorldStateSection::Known(previous) = &previous { + let previous_body = previous.get("body").and_then(serde_json::Value::as_str); + let previous_include_instructions = previous + .get("includeInstructions") + .and_then(serde_json::Value::as_bool); + if previous_body == body.as_deref() + && previous_include_instructions == Some(include_instructions) + { + return None; + } + } + + let body = match body.as_deref() { + Some(body) => body, + None if previous_is_absent => return None, + None if !include_instructions => HIDDEN_HOST_SKILLS_BODY, + None => NO_HOST_SKILLS_BODY, + }; + Some(RenderedWorldStateFragment::new( + "developer", + (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG), + body, + )) + }); + match retained_fragment { + Some(fragment) => contribution.with_retained_fragment_matcher(move |role, text| { + role == "developer" && text.contains(&fragment) + }), + None => contribution, + } +} diff --git a/codex-rs/ext/skills/tests/implicit_invocation.rs b/codex-rs/ext/skills/tests/implicit_invocation.rs index 43b986753e..001c5cc83b 100644 --- a/codex-rs/ext/skills/tests/implicit_invocation.rs +++ b/codex-rs/ext/skills/tests/implicit_invocation.rs @@ -25,6 +25,7 @@ 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::HostSkillProvider; use codex_skills_extension::SkillProviders; use codex_skills_extension::SkillsExtensionConfig; use codex_skills_extension::catalog::SkillAuthority; @@ -226,7 +227,9 @@ async fn shadow_selection_uses_host_snapshot_and_excludes_executor_candidates() let mut builder = ExtensionRegistryBuilder::<()>::new(); install_with_providers_and_metrics( &mut builder, - SkillProviders::new().with_executor_provider(Arc::new(ExecutorProvider)), + SkillProviders::new() + .with_executor_provider(Arc::new(ExecutorProvider)) + .with_host_provider(Arc::new(HostSkillProvider::new())), Some(metrics.clone()), |_| SkillsExtensionConfig { include_instructions: true,