From 3389fa554e953d07a12a34f5681aae46f17958f8 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 3 Jun 2026 13:32:55 +0200 Subject: [PATCH] skills: resolve per-turn catalogs from turn input context (#26106) ## Why The skills extension needs the resolved turn environments to build a real per-turn `SkillListQuery`. The previous `TurnLifecycleContributor` hook only had a turn id, so it could only seed a placeholder query and never carry the executor authorities that executor-scoped skill routing will need. Moving catalog resolution onto `TurnInputContributor` puts the skills extension on the same turn-preparation path that already has the environment ids and working directories for the submitted turn, while keeping the actual prompt injection work for follow-up changes. ## What changed - switch `ext/skills` from `TurnLifecycleContributor` to `TurnInputContributor` - build `executor_authorities` from `TurnInputContext.environments` and pass them through `SkillListQuery` - keep storing the resolved catalog in `SkillsTurnState`, but drop the placeholder query helper that no longer matches the real data flow - update the extension TODOs to reflect that per-turn catalog resolution now happens in the turn-input contributor, and that prompt/context injection still needs to move later ## Testing - Not run locally. --- codex-rs/Cargo.lock | 1 + .../src/additional_context.rs | 4 +- codex-rs/context-fragments/src/fragment.rs | 19 ++++-- .../core-skills/src/skill_instructions.rs | 2 +- .../context/approved_command_prefix_saved.rs | 2 +- .../core/src/context/apps_instructions.rs | 2 +- .../context/available_plugins_instructions.rs | 2 +- .../context/available_skills_instructions.rs | 2 +- .../collaboration_mode_instructions.rs | 2 +- .../core/src/context/environment_context.rs | 2 +- .../guardian_followup_review_reminder.rs | 2 +- .../src/context/hook_additional_context.rs | 2 +- .../context/image_generation_instructions.rs | 2 +- .../src/context/internal_model_context.rs | 2 +- ...legacy_apply_patch_exec_command_warning.rs | 2 +- .../context/legacy_model_mismatch_warning.rs | 2 +- ...gacy_unified_exec_process_limit_warning.rs | 2 +- .../src/context/model_switch_instructions.rs | 2 +- .../core/src/context/network_rule_saved.rs | 2 +- .../context/personality_spec_instructions.rs | 2 +- .../core/src/context/plugin_instructions.rs | 2 +- .../src/context/realtime_end_instructions.rs | 2 +- .../context/realtime_start_instructions.rs | 2 +- .../realtime_start_with_instructions.rs | 2 +- .../core/src/context/subagent_notification.rs | 2 +- codex-rs/core/src/context/turn_aborted.rs | 2 +- .../core/src/context/user_instructions.rs | 2 +- .../core/src/context/user_shell_command.rs | 2 +- codex-rs/core/src/session/turn.rs | 8 ++- codex-rs/ext/extension-api/Cargo.toml | 1 + .../ext/extension-api/src/contributors.rs | 6 +- codex-rs/ext/extension-api/src/lib.rs | 1 + codex-rs/ext/skills/src/extension.rs | 63 +++++++++++-------- codex-rs/ext/skills/src/provider.rs | 11 ---- .../prompts/src/permissions_instructions.rs | 2 +- 35 files changed, 92 insertions(+), 74 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f7e6ed5de..0b599b419 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2863,6 +2863,7 @@ name = "codex-extension-api" version = "0.0.0" dependencies = [ "async-trait", + "codex-context-fragments", "codex-protocol", "codex-tools", ] diff --git a/codex-rs/context-fragments/src/additional_context.rs b/codex-rs/context-fragments/src/additional_context.rs index 4d08b5178..d1c5147d2 100644 --- a/codex-rs/context-fragments/src/additional_context.rs +++ b/codex-rs/context-fragments/src/additional_context.rs @@ -19,7 +19,7 @@ impl AdditionalContextUserFragment { } impl ContextualUserFragment for AdditionalContextUserFragment { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } @@ -65,7 +65,7 @@ impl AdditionalContextDeveloperFragment { } impl ContextualUserFragment for AdditionalContextDeveloperFragment { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/context-fragments/src/fragment.rs b/codex-rs/context-fragments/src/fragment.rs index f41cbb63e..bd0e06772 100644 --- a/codex-rs/context-fragments/src/fragment.rs +++ b/codex-rs/context-fragments/src/fragment.rs @@ -44,9 +44,7 @@ impl FragmentRegistration for FragmentRegistrationPro /// in which case the default helpers render only the body and never match /// arbitrary text. pub trait ContextualUserFragment { - fn role() -> &'static str - where - Self: Sized; + fn role(&self) -> &'static str; fn markers(&self) -> (&'static str, &'static str); @@ -80,7 +78,18 @@ pub trait ContextualUserFragment { { ResponseItem::Message { id: None, - role: Self::role().to_string(), + role: self.role().to_string(), + content: vec![ContentItem::InputText { + text: self.render(), + }], + phase: None, + } + } + + fn into_boxed_response_item(self: Box) -> ResponseItem { + ResponseItem::Message { + id: None, + role: self.role().to_string(), content: vec![ContentItem::InputText { text: self.render(), }], @@ -93,7 +102,7 @@ pub trait ContextualUserFragment { Self: Sized, { ResponseInputItem::Message { - role: Self::role().to_string(), + role: self.role().to_string(), content: vec![ContentItem::InputText { text: self.render(), }], diff --git a/codex-rs/core-skills/src/skill_instructions.rs b/codex-rs/core-skills/src/skill_instructions.rs index 47dd0d021..b2a002d90 100644 --- a/codex-rs/core-skills/src/skill_instructions.rs +++ b/codex-rs/core-skills/src/skill_instructions.rs @@ -20,7 +20,7 @@ impl From<&SkillInjection> for SkillInstructions { } impl ContextualUserFragment for SkillInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/approved_command_prefix_saved.rs b/codex-rs/core/src/context/approved_command_prefix_saved.rs index 532a2494c..3686741d1 100644 --- a/codex-rs/core/src/context/approved_command_prefix_saved.rs +++ b/codex-rs/core/src/context/approved_command_prefix_saved.rs @@ -14,7 +14,7 @@ impl ApprovedCommandPrefixSaved { } impl ContextualUserFragment for ApprovedCommandPrefixSaved { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/apps_instructions.rs b/codex-rs/core/src/context/apps_instructions.rs index 9a475dc31..f88ee4967 100644 --- a/codex-rs/core/src/context/apps_instructions.rs +++ b/codex-rs/core/src/context/apps_instructions.rs @@ -18,7 +18,7 @@ impl AppsInstructions { } impl ContextualUserFragment for AppsInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/available_plugins_instructions.rs b/codex-rs/core/src/context/available_plugins_instructions.rs index e7f560f6a..b2c12c759 100644 --- a/codex-rs/core/src/context/available_plugins_instructions.rs +++ b/codex-rs/core/src/context/available_plugins_instructions.rs @@ -22,7 +22,7 @@ impl AvailablePluginsInstructions { } impl ContextualUserFragment for AvailablePluginsInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/available_skills_instructions.rs b/codex-rs/core/src/context/available_skills_instructions.rs index e63d63767..f7921072b 100644 --- a/codex-rs/core/src/context/available_skills_instructions.rs +++ b/codex-rs/core/src/context/available_skills_instructions.rs @@ -21,7 +21,7 @@ impl From for AvailableSkillsInstructions { } impl ContextualUserFragment for AvailableSkillsInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/collaboration_mode_instructions.rs b/codex-rs/core/src/context/collaboration_mode_instructions.rs index 16e3d3819..f7b0423d8 100644 --- a/codex-rs/core/src/context/collaboration_mode_instructions.rs +++ b/codex-rs/core/src/context/collaboration_mode_instructions.rs @@ -22,7 +22,7 @@ impl CollaborationModeInstructions { } impl ContextualUserFragment for CollaborationModeInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index 051147b7c..aaaa2b62b 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -523,7 +523,7 @@ fn workspace_roots_from_turn_context_item( } impl ContextualUserFragment for EnvironmentContext { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/guardian_followup_review_reminder.rs b/codex-rs/core/src/context/guardian_followup_review_reminder.rs index 4de750629..cb3569c67 100644 --- a/codex-rs/core/src/context/guardian_followup_review_reminder.rs +++ b/codex-rs/core/src/context/guardian_followup_review_reminder.rs @@ -4,7 +4,7 @@ use super::ContextualUserFragment; pub(crate) struct GuardianFollowupReviewReminder; impl ContextualUserFragment for GuardianFollowupReviewReminder { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/hook_additional_context.rs b/codex-rs/core/src/context/hook_additional_context.rs index 109426166..bd2346844 100644 --- a/codex-rs/core/src/context/hook_additional_context.rs +++ b/codex-rs/core/src/context/hook_additional_context.rs @@ -12,7 +12,7 @@ impl HookAdditionalContext { } impl ContextualUserFragment for HookAdditionalContext { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/image_generation_instructions.rs b/codex-rs/core/src/context/image_generation_instructions.rs index 7ddb231b8..52ce1ca4e 100644 --- a/codex-rs/core/src/context/image_generation_instructions.rs +++ b/codex-rs/core/src/context/image_generation_instructions.rs @@ -17,7 +17,7 @@ impl ImageGenerationInstructions { } impl ContextualUserFragment for ImageGenerationInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/internal_model_context.rs b/codex-rs/core/src/context/internal_model_context.rs index a25e77a82..cbba3ecd7 100644 --- a/codex-rs/core/src/context/internal_model_context.rs +++ b/codex-rs/core/src/context/internal_model_context.rs @@ -76,7 +76,7 @@ impl InternalModelContextFragment { } impl ContextualUserFragment for InternalModelContextFragment { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs b/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs index b9ecbf617..c764a8838 100644 --- a/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs +++ b/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs @@ -5,7 +5,7 @@ use super::ContextualUserFragment; pub(crate) struct LegacyApplyPatchExecCommandWarning; impl ContextualUserFragment for LegacyApplyPatchExecCommandWarning { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/legacy_model_mismatch_warning.rs b/codex-rs/core/src/context/legacy_model_mismatch_warning.rs index 9b769e11e..d713993c9 100644 --- a/codex-rs/core/src/context/legacy_model_mismatch_warning.rs +++ b/codex-rs/core/src/context/legacy_model_mismatch_warning.rs @@ -5,7 +5,7 @@ use super::ContextualUserFragment; pub(crate) struct LegacyModelMismatchWarning; impl ContextualUserFragment for LegacyModelMismatchWarning { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs b/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs index 54f1a40e3..59fe03a2d 100644 --- a/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs +++ b/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs @@ -5,7 +5,7 @@ use super::ContextualUserFragment; pub(crate) struct LegacyUnifiedExecProcessLimitWarning; impl ContextualUserFragment for LegacyUnifiedExecProcessLimitWarning { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/model_switch_instructions.rs b/codex-rs/core/src/context/model_switch_instructions.rs index f15dcd22e..b1bdebbc9 100644 --- a/codex-rs/core/src/context/model_switch_instructions.rs +++ b/codex-rs/core/src/context/model_switch_instructions.rs @@ -14,7 +14,7 @@ impl ModelSwitchInstructions { } impl ContextualUserFragment for ModelSwitchInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/network_rule_saved.rs b/codex-rs/core/src/context/network_rule_saved.rs index 5b45dbb56..482602700 100644 --- a/codex-rs/core/src/context/network_rule_saved.rs +++ b/codex-rs/core/src/context/network_rule_saved.rs @@ -18,7 +18,7 @@ impl NetworkRuleSaved { } impl ContextualUserFragment for NetworkRuleSaved { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/personality_spec_instructions.rs b/codex-rs/core/src/context/personality_spec_instructions.rs index 590ebde80..f584486d0 100644 --- a/codex-rs/core/src/context/personality_spec_instructions.rs +++ b/codex-rs/core/src/context/personality_spec_instructions.rs @@ -12,7 +12,7 @@ impl PersonalitySpecInstructions { } impl ContextualUserFragment for PersonalitySpecInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/plugin_instructions.rs b/codex-rs/core/src/context/plugin_instructions.rs index 8fbccbfa1..be2ac8ec0 100644 --- a/codex-rs/core/src/context/plugin_instructions.rs +++ b/codex-rs/core/src/context/plugin_instructions.rs @@ -12,7 +12,7 @@ impl PluginInstructions { } impl ContextualUserFragment for PluginInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/realtime_end_instructions.rs b/codex-rs/core/src/context/realtime_end_instructions.rs index 39d58c9ae..b4225dfb4 100644 --- a/codex-rs/core/src/context/realtime_end_instructions.rs +++ b/codex-rs/core/src/context/realtime_end_instructions.rs @@ -17,7 +17,7 @@ impl RealtimeEndInstructions { } impl ContextualUserFragment for RealtimeEndInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/realtime_start_instructions.rs b/codex-rs/core/src/context/realtime_start_instructions.rs index af01c184f..074f1a978 100644 --- a/codex-rs/core/src/context/realtime_start_instructions.rs +++ b/codex-rs/core/src/context/realtime_start_instructions.rs @@ -7,7 +7,7 @@ use codex_protocol::protocol::REALTIME_CONVERSATION_OPEN_TAG; pub(crate) struct RealtimeStartInstructions; impl ContextualUserFragment for RealtimeStartInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/realtime_start_with_instructions.rs b/codex-rs/core/src/context/realtime_start_with_instructions.rs index ee242f906..a61130969 100644 --- a/codex-rs/core/src/context/realtime_start_with_instructions.rs +++ b/codex-rs/core/src/context/realtime_start_with_instructions.rs @@ -16,7 +16,7 @@ impl RealtimeStartWithInstructions { } impl ContextualUserFragment for RealtimeStartWithInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" } diff --git a/codex-rs/core/src/context/subagent_notification.rs b/codex-rs/core/src/context/subagent_notification.rs index 0c1358f33..6d92b976b 100644 --- a/codex-rs/core/src/context/subagent_notification.rs +++ b/codex-rs/core/src/context/subagent_notification.rs @@ -18,7 +18,7 @@ impl SubagentNotification { } impl ContextualUserFragment for SubagentNotification { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/turn_aborted.rs b/codex-rs/core/src/context/turn_aborted.rs index 6dc669728..c2ef156b3 100644 --- a/codex-rs/core/src/context/turn_aborted.rs +++ b/codex-rs/core/src/context/turn_aborted.rs @@ -17,7 +17,7 @@ impl TurnAborted { } impl ContextualUserFragment for TurnAborted { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/user_instructions.rs b/codex-rs/core/src/context/user_instructions.rs index 1b3fa4029..a387376f1 100644 --- a/codex-rs/core/src/context/user_instructions.rs +++ b/codex-rs/core/src/context/user_instructions.rs @@ -7,7 +7,7 @@ pub(crate) struct UserInstructions { } impl ContextualUserFragment for UserInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/context/user_shell_command.rs b/codex-rs/core/src/context/user_shell_command.rs index cf5a3337d..377342e50 100644 --- a/codex-rs/core/src/context/user_shell_command.rs +++ b/codex-rs/core/src/context/user_shell_command.rs @@ -27,7 +27,7 @@ impl UserShellCommand { } impl ContextualUserFragment for UserShellCommand { - fn role() -> &'static str { + fn role(&self) -> &'static str { "user" } diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index c6a3155b7..e301a9c04 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -628,7 +628,7 @@ async fn build_extension_turn_input_items( let mut items = Vec::new(); for contributor in contributors { - let contributed_items = contributor + let contributed_fragments = contributor .contribute( input.clone(), &sess.services.session_extension_data, @@ -638,7 +638,11 @@ async fn build_extension_turn_input_items( .or_cancel(cancellation_token) .await .ok()?; - items.extend(contributed_items); + items.extend( + contributed_fragments + .into_iter() + .map(ContextualUserFragment::into_boxed_response_item), + ); } Some(items) diff --git a/codex-rs/ext/extension-api/Cargo.toml b/codex-rs/ext/extension-api/Cargo.toml index 15f6e99fa..85c7d8f98 100644 --- a/codex-rs/ext/extension-api/Cargo.toml +++ b/codex-rs/ext/extension-api/Cargo.toml @@ -15,5 +15,6 @@ workspace = true [dependencies] async-trait = { workspace = true } +codex-context-fragments = { workspace = true } codex-protocol = { workspace = true } codex-tools = { workspace = true } diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index 98bc395a2..14ccb6890 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -1,8 +1,8 @@ use std::future::Future; use std::sync::Arc; +use codex_context_fragments::ContextualUserFragment; use codex_protocol::items::TurnItem; -use codex_protocol::models::ResponseItem; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::TokenUsageInfo; use codex_tools::ToolCall; @@ -98,14 +98,14 @@ pub trait TurnLifecycleContributor: Send + Sync { /// host, not in this input. #[async_trait::async_trait] pub trait TurnInputContributor: Send + Sync { - /// Returns additional model input items for one submitted turn. + /// Returns additional contextual fragments for one submitted turn. async fn contribute( &self, input: TurnInputContext, session_store: &ExtensionData, thread_store: &ExtensionData, turn_store: &ExtensionData, - ) -> Vec; + ) -> Vec>; } /// Contributor for host-owned configuration changes. diff --git a/codex-rs/ext/extension-api/src/lib.rs b/codex-rs/ext/extension-api/src/lib.rs index 870e331fb..7fa60c0fe 100644 --- a/codex-rs/ext/extension-api/src/lib.rs +++ b/codex-rs/ext/extension-api/src/lib.rs @@ -10,6 +10,7 @@ pub use capabilities::NoopExtensionEventSink; pub use capabilities::NoopResponseItemInjector; pub use capabilities::ResponseItemInjectionFuture; pub use capabilities::ResponseItemInjector; +pub use codex_context_fragments::ContextualUserFragment; pub use codex_protocol::models::ResponseItem; pub use codex_tools::ConversationHistory; pub use codex_tools::ExtensionTurnItem; diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index bf91b60de..adbaa8ffe 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -5,14 +5,17 @@ use std::sync::Arc; use codex_core::config::Config; use codex_extension_api::ConfigContributor; use codex_extension_api::ContextContributor; +use codex_extension_api::ContextualUserFragment; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::PromptFragment; use codex_extension_api::ThreadLifecycleContributor; use codex_extension_api::ThreadStartInput; -use codex_extension_api::TurnLifecycleContributor; -use codex_extension_api::TurnStartInput; +use codex_extension_api::TurnInputContext; +use codex_extension_api::TurnInputContributor; +use crate::catalog::SkillAuthority; +use crate::catalog::SkillSourceKind; use crate::provider::SkillListQuery; use crate::providers::SkillProviders; use crate::state::SkillsExtensionConfig; @@ -29,8 +32,8 @@ impl ThreadLifecycleContributor for SkillsExtension { // TODO(skills-extension): this is only the thread-level config snapshot. // Skills are loaded per turn today because cwd, plugin roots, config // layers, and the primary environment filesystem can change between - // turns. The real migration needs a turn-preparation hook before model - // input construction, not just thread startup. + // turns. The TurnInputContributor below owns per-turn catalog + // resolution. input .thread_store .insert(SkillsExtensionConfig::from_config(input.config)); @@ -66,42 +69,50 @@ impl ContextContributor for SkillsExtension { } // TODO(skills-extension): render the available-skills developer - // block from the merged per-turn SkillCatalog. This should - // preserve the existing bounded metadata budget, root aliasing, - // warning behavior, and telemetry side effects. + // block only if the final model needs thread-level guidance that + // does not depend on the per-turn SkillCatalog. // // TODO(skills-extension): avoid using raw PromptFragment strings // for final skills context if the extension API grows typed // contextual fragments. Existing skill blocks are typed so resume // and history filtering can recognize them reliably. - // - // TODO(skills-extension): ContextContributor currently cannot see - // the turn_store, so it cannot read the per-turn catalog seeded by - // the turn provider path below. This is the main extension-api gap - // to close before skills can move out of codex-core. Vec::new() }) } } #[async_trait::async_trait] -impl TurnLifecycleContributor for SkillsExtension { - async fn on_turn_start(&self, input: TurnStartInput<'_>) { - // TODO(skills-extension): replace this lifecycle callback with a real - // turn-input contributor in codex-extension-api. This placeholder only - // demonstrates where provider aggregation belongs; it cannot resolve - // real skills because this hook does not receive cwd, executor - // selections, effective plugins/materialized plugin skill roots, - // connector slug counts, user input, cancellation, analytics, or a - // response-item output channel. - let query = SkillListQuery::placeholder_for_turn(input.turn_id); +impl TurnInputContributor for SkillsExtension { + async fn contribute( + &self, + input: TurnInputContext, + _session_store: &ExtensionData, + _thread_store: &ExtensionData, + turn_store: &ExtensionData, + ) -> Vec> { + let executor_authorities = input + .environments + .iter() + .map(|environment| { + SkillAuthority::new( + SkillSourceKind::Executor, + environment.environment_id.clone(), + ) + }) + .collect(); + let query = SkillListQuery { + turn_id: input.turn_id, + executor_authorities, + include_host_skills: true, + include_remote_skills: true, + }; let catalog = self .providers .list_for_turn(query) .await .unwrap_or_default(); - input.turn_store.insert(SkillsTurnState { + turn_store.insert(SkillsTurnState { catalog, entrypoints_injected: false, }); @@ -115,7 +126,9 @@ impl TurnLifecycleContributor for SkillsExtension { // // TODO(skills-extension): move explicit $skill mention resolution, // SKILL.md reads, skill body injection, and MCP dependency prompting - // out of codex-core's turn assembly once that hook exists. + // out of codex-core's turn assembly once provider implementations + // are ready to preserve behavior. + Vec::new() } } @@ -136,5 +149,5 @@ pub fn install(registry: &mut ExtensionRegistryBuilder) { registry.thread_lifecycle_contributor(extension.clone()); registry.config_contributor(extension.clone()); registry.prompt_contributor(extension.clone()); - registry.turn_lifecycle_contributor(extension); + registry.turn_input_contributor(extension); } diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs index 730adb51b..b6f240752 100644 --- a/codex-rs/ext/skills/src/provider.rs +++ b/codex-rs/ext/skills/src/provider.rs @@ -16,17 +16,6 @@ pub struct SkillListQuery { pub include_remote_skills: bool, } -impl SkillListQuery { - pub(crate) fn placeholder_for_turn(turn_id: &str) -> Self { - Self { - turn_id: turn_id.to_string(), - executor_authorities: Vec::new(), - include_host_skills: true, - include_remote_skills: true, - } - } -} - #[derive(Clone, Debug, PartialEq, Eq)] pub struct SkillReadRequest { pub authority: SkillAuthority, diff --git a/codex-rs/prompts/src/permissions_instructions.rs b/codex-rs/prompts/src/permissions_instructions.rs index 46c3e865d..f360a987d 100644 --- a/codex-rs/prompts/src/permissions_instructions.rs +++ b/codex-rs/prompts/src/permissions_instructions.rs @@ -143,7 +143,7 @@ impl PermissionsInstructions { } impl ContextualUserFragment for PermissionsInstructions { - fn role() -> &'static str { + fn role(&self) -> &'static str { "developer" }