Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions codex-rs/core-skills/src/injection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,32 @@ pub struct SkillInjection {
pub contents: String,
}

/// Host skill prompts that have already been injected by an extension for this
/// turn.
///
/// Core uses this to keep the legacy skill-injection path from sending the same
/// host `SKILL.md` body again while the skills extension is being wired in.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct InjectedHostSkillPrompts {
paths: HashSet<String>,
}

impl InjectedHostSkillPrompts {
pub fn insert_path(&mut self, path: impl Into<String>) {
let path = path.into();
self.paths.insert(normalize_host_skill_path(&path));
self.paths.insert(path);
}

pub fn is_empty(&self) -> bool {
self.paths.is_empty()
}

pub fn contains_path(&self, path: &str) -> bool {
self.paths.contains(path) || self.paths.contains(&normalize_host_skill_path(path))
}
}

pub async fn build_skill_injections(
mentioned_skills: &[SkillMetadata],
loaded_skills: Option<&SkillLoadOutcome>,
Expand Down Expand Up @@ -85,6 +111,10 @@ pub async fn build_skill_injections(
result
}

fn normalize_host_skill_path(path: &str) -> String {
normalize_skill_path(path).replace('\\', "/")
}

fn emit_skill_injected_metric(
otel: Option<&SessionTelemetry>,
skill: &SkillMetadata,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core-skills/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use manager::SkillsLoadInput;
pub use manager::SkillsManager;
pub use mention_counts::build_skill_name_counts;
pub use model::HostLoadedSkills;
pub use model::SkillError;
pub use model::SkillLoadOutcome;
pub use model::SkillMetadata;
Expand Down
32 changes: 30 additions & 2 deletions codex-rs/core-skills/src/model.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::io;
use std::sync::Arc;

use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
Expand All @@ -23,7 +25,7 @@ pub struct SkillMetadata {
}

impl SkillMetadata {
fn allow_implicit_invocation(&self) -> bool {
pub fn allows_implicit_invocation(&self) -> bool {
self.policy
.as_ref()
.and_then(|policy| policy.allow_implicit_invocation)
Expand Down Expand Up @@ -103,7 +105,7 @@ impl SkillLoadOutcome {
}

pub fn is_skill_allowed_for_implicit_invocation(&self, skill: &SkillMetadata) -> bool {
self.is_skill_enabled(skill) && skill.allow_implicit_invocation()
self.is_skill_enabled(skill) && skill.allows_implicit_invocation()
}

pub fn allowed_skills_for_implicit_invocation(&self) -> Vec<SkillMetadata> {
Expand All @@ -129,6 +131,32 @@ impl SkillLoadOutcome {
}
}

/// Host-loaded skills for one turn, including the filesystem mapping needed to
/// read skill bodies through the environment that loaded them.
#[derive(Debug, Clone)]
pub struct HostLoadedSkills {
outcome: Arc<SkillLoadOutcome>,
}

impl HostLoadedSkills {
pub fn new(outcome: Arc<SkillLoadOutcome>) -> Self {
Self { outcome }
}

pub fn outcome(&self) -> &SkillLoadOutcome {
self.outcome.as_ref()
}

pub async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result<String> {
let fs = self
.outcome
.file_system_for_skill(skill)
.unwrap_or_else(|| Arc::clone(&LOCAL_FS));
fs.read_file_text(&skill.path_to_skills_md, /*sandbox*/ None)
.await
}
}

#[derive(Clone, Default)]
pub(crate) struct SkillFileSystemsByPath {
values: Arc<HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>>,
Expand Down
10 changes: 9 additions & 1 deletion codex-rs/core/src/session/review.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::*;
use codex_core_skills::HostLoadedSkills;
use codex_protocol::openai_models::ToolMode;
use std::sync::atomic::AtomicBool;

Expand Down Expand Up @@ -100,6 +101,13 @@ pub(super) async fn spawn_review_thread(
parent_turn_context.network.is_some(),
));

let extension_data = Arc::new(codex_extension_api::ExtensionData::new(
review_turn_id.clone(),
));
extension_data.insert(HostLoadedSkills::new(
parent_turn_context.turn_skills.outcome.clone(),
));

let review_turn_context = TurnContext {
sub_id: review_turn_id.clone(),
trace_id: current_span_trace_id(),
Expand Down Expand Up @@ -143,7 +151,7 @@ pub(super) async fn spawn_review_thread(
dynamic_tools: parent_turn_context.dynamic_tools.clone(),
truncation_policy: model_info.truncation_policy.into(),
turn_metadata_state,
extension_data: Arc::new(codex_extension_api::ExtensionData::new(review_turn_id)),
extension_data,
turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()),
turn_timing_state: Arc::new(TurnTimingState::default()),
server_model_warning_emitted: AtomicBool::new(false),
Expand Down
15 changes: 14 additions & 1 deletion codex-rs/core/src/session/turn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use codex_analytics::InvocationType;
use codex_analytics::TurnResolvedConfigFact;
use codex_analytics::build_track_events_context;
use codex_async_utils::OrCancelExt;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_extension_api::TurnInputContext;
use codex_extension_api::TurnInputEnvironment;
use codex_features::Feature;
Expand Down Expand Up @@ -535,6 +536,9 @@ async fn build_skills_and_plugins(
)
.await;

let injected_host_skill_prompts = turn_context
.extension_data
.get::<InjectedHostSkillPrompts>();
let SkillInjections {
items: skill_injections,
warnings: skill_warnings,
Expand Down Expand Up @@ -591,7 +595,16 @@ async fn build_skills_and_plugins(
.track_plugin_used(tracking.clone(), plugin);
}

let mut injection_items = skill_items;
let mut injection_items: Vec<ResponseItem> = match injected_host_skill_prompts {
Some(injected_host_skill_prompts) => skill_injections
.iter()
.filter(|skill| !injected_host_skill_prompts.contains_path(&skill.path))
.map(|skill| {
ContextualUserFragment::into(crate::context::SkillInstructions::from(skill))
})
.collect(),
None => skill_items,
};
injection_items.extend(plugin_items);
injection_items.extend(extension_injection_items);
Some((injection_items, explicitly_enabled_connectors))
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/session/turn_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::*;
use crate::SkillLoadOutcome;
use crate::config::GhostSnapshotConfig;
use crate::environment_selection::ResolvedTurnEnvironments;
use codex_core_skills::HostLoadedSkills;
use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider;
use codex_protocol::SessionId;
Expand Down Expand Up @@ -530,6 +531,7 @@ impl Session {
));
let (current_date, timezone) = local_time_context();
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone()));
extension_data.insert(HostLoadedSkills::new(Arc::clone(&skills_outcome)));
TurnContext {
sub_id,
trace_id: current_span_trace_id(),
Expand Down
1 change: 0 additions & 1 deletion codex-rs/ext/extension-api/src/contributors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ pub trait TurnLifecycleContributor: Send + Sync {
async fn on_turn_error(&self, _input: TurnErrorInput<'_>) {}
}

/// WARNING: DO NOT USE YET
/// Extension contribution that can add turn-local model input.
///
/// Implementations should resolve only the model-visible input they own and
Expand Down
29 changes: 26 additions & 3 deletions codex-rs/ext/skills/src/extension.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::sync::Arc;

use codex_core::config::Config;
use codex_core_skills::HostLoadedSkills;
use codex_core_skills::SkillInstructions;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_core_skills::injection::SkillInjection;
use codex_extension_api::ConfigContributor;
use codex_extension_api::ContextualUserFragment;
Expand All @@ -20,6 +22,7 @@ use crate::catalog::SkillAuthority;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillReadResult;
use crate::catalog::SkillSourceKind;
use crate::provider::HostSkillProvider;
use crate::provider::SkillListQuery;
use crate::provider::SkillReadRequest;
use crate::render::available_skills_fragment;
Expand Down Expand Up @@ -78,6 +81,7 @@ impl TurnInputContributor for SkillsExtension {
};

let config = thread_state.config();
let host_loaded_skills = turn_store.get::<HostLoadedSkills>();
let query = SkillListQuery {
turn_id: input.turn_id.clone(),
executor_authorities: input
Expand All @@ -90,6 +94,7 @@ impl TurnInputContributor for SkillsExtension {
)
})
.collect(),
host: host_loaded_skills.clone(),
include_host_skills: true,
include_bundled_skills: config.bundled_skills_enabled,
include_remote_skills: true,
Expand All @@ -109,8 +114,12 @@ impl TurnInputContributor for SkillsExtension {

let mut warnings = catalog.warnings.clone();
let mut main_prompts_injected = false;
let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default();
for entry in &selected_entries {
match self.read_main_prompt(entry).await {
match self
.read_main_prompt(entry, host_loaded_skills.clone())
.await
Comment thread
jif-oai marked this conversation as resolved.
{
Ok(read_result) => {
let (contents, truncated) =
truncate_main_prompt_contents(read_result.contents.as_str());
Expand All @@ -129,6 +138,9 @@ impl TurnInputContributor for SkillsExtension {
};
fragments.push(Box::new(SkillInstructions::from(&injection)));
main_prompts_injected = true;
if entry.authority.kind == SkillSourceKind::Host {
injected_host_skill_prompts.insert_path(entry.main_prompt.0.clone());
}
}
Err(message) => {
let warning = format!("Failed to load skill `{}`: {message}", entry.name);
Expand All @@ -144,18 +156,26 @@ impl TurnInputContributor for SkillsExtension {
warnings,
main_prompts_injected,
});
if !injected_host_skill_prompts.is_empty() {
turn_store.insert(injected_host_skill_prompts);
}

fragments
}
}

impl SkillsExtension {
async fn read_main_prompt(&self, entry: &SkillCatalogEntry) -> Result<SkillReadResult, String> {
async fn read_main_prompt(
&self,
entry: &SkillCatalogEntry,
host_loaded_skills: Option<Arc<HostLoadedSkills>>,
) -> Result<SkillReadResult, String> {
self.providers
.read(SkillReadRequest {
authority: entry.authority.clone(),
package: entry.id.clone(),
resource: entry.main_prompt.clone(),
host: host_loaded_skills,
})
.await
.map_err(|err| err.message)
Expand All @@ -170,7 +190,10 @@ impl SkillsExtension {
}

pub fn install(registry: &mut ExtensionRegistryBuilder<Config>) {
install_with_providers(registry, SkillProviders::default());
install_with_providers(
registry,
SkillProviders::new().with_host_provider(Arc::new(HostSkillProvider::new())),
Comment thread
jif-oai marked this conversation as resolved.
Comment thread
jif-oai marked this conversation as resolved.
Comment thread
jif-oai marked this conversation as resolved.
);
}

pub fn install_with_providers(
Expand Down
1 change: 1 addition & 0 deletions codex-rs/ext/skills/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ mod state;

pub use extension::install;
pub use extension::install_with_providers;
pub use provider::HostSkillProvider;
pub use sources::SkillProviderSource;
pub use sources::SkillProviders;
13 changes: 11 additions & 2 deletions codex-rs/ext/skills/src/provider.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;

mod host;

use codex_core_skills::HostLoadedSkills;

use crate::catalog::SkillAuthority;
use crate::catalog::SkillCatalog;
Expand All @@ -9,20 +14,24 @@ use crate::catalog::SkillReadResult;
use crate::catalog::SkillResourceId;
use crate::catalog::SkillSearchResult;

#[derive(Clone, Debug, PartialEq, Eq)]
pub use host::HostSkillProvider;

#[derive(Clone, Debug)]
pub struct SkillListQuery {
pub turn_id: String,
pub executor_authorities: Vec<SkillAuthority>,
pub host: Option<Arc<HostLoadedSkills>>,
pub include_host_skills: bool,
pub include_bundled_skills: bool,
pub include_remote_skills: bool,
}

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug)]
pub struct SkillReadRequest {
pub authority: SkillAuthority,
pub package: SkillPackageId,
pub resource: SkillResourceId,
pub host: Option<Arc<HostLoadedSkills>>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down
Loading
Loading