Make skills follow model-step state

This commit is contained in:
jif-oai
2026-06-25 13:05:05 +01:00
parent 0a8b48c651
commit 6059eb428a
32 changed files with 1390 additions and 1262 deletions
+3 -7
View File
@@ -36,7 +36,6 @@ pub(crate) struct ThreadExtensionDependencies {
pub(crate) thread_manager: Weak<ThreadManager>,
pub(crate) goal_service: Arc<GoalService>,
pub(crate) environment_manager: Arc<EnvironmentManager>,
pub(crate) executor_skill_provider: Arc<dyn codex_skills_extension::SkillProvider>,
/// Process-scoped persistence backend for extensions that need stored thread history.
pub(crate) thread_store: Arc<dyn ThreadStore>,
}
@@ -56,7 +55,6 @@ where
thread_manager,
goal_service,
environment_manager,
executor_skill_provider,
thread_store: _thread_store,
} = dependencies;
let mut builder = ExtensionRegistryBuilder::<Config>::with_event_sink(event_sink);
@@ -79,11 +77,9 @@ where
codex_image_generation_extension::install(&mut builder, auth_manager, |config: &Config| {
Some(config.codex_home.clone())
});
let skill_providers = codex_skills_extension::SkillProviders::new()
.with_executor_provider(executor_skill_provider)
.with_orchestrator_provider(Arc::new(
codex_skills_extension::OrchestratorSkillProvider::new(),
));
let skill_providers = codex_skills_extension::SkillProviders::new().with_orchestrator_provider(
Arc::new(codex_skills_extension::OrchestratorSkillProvider::new()),
);
codex_skills_extension::install_with_providers(
&mut builder,
skill_providers,
-7
View File
@@ -216,12 +216,6 @@ mod tests {
.expect("refresh tests require state db");
let thread_store = thread_store_from_config(&good_config, Some(state_db.clone()));
let environment_manager = Arc::new(EnvironmentManager::default_for_tests());
let executor_skill_provider: Arc<dyn codex_skills_extension::SkillProvider> = Arc::new(
codex_skills_extension::ExecutorSkillProvider::new_with_restriction_product(
Arc::clone(&environment_manager),
SessionSource::Exec.restriction_product(),
),
);
let thread_manager = Arc::new_cyclic(|thread_manager| {
ThreadManager::new(
&good_config,
@@ -238,7 +232,6 @@ mod tests {
thread_manager: thread_manager.clone(),
goal_service: Arc::new(codex_goal_extension::GoalService::new()),
environment_manager: Arc::clone(&environment_manager),
executor_skill_provider: Arc::clone(&executor_skill_provider),
thread_store: Arc::clone(&thread_store),
},
),
@@ -337,13 +337,6 @@ impl MessageProcessor {
let thread_store = codex_core::thread_store_from_config(config.as_ref(), state_db.clone());
let environment_manager_for_requests = Arc::clone(&environment_manager);
let environment_manager_for_extensions = Arc::clone(&environment_manager);
let restriction_product = session_source.restriction_product();
let executor_skill_provider: Arc<dyn codex_skills_extension::SkillProvider> = Arc::new(
codex_skills_extension::ExecutorSkillProvider::new_with_restriction_product(
Arc::clone(&environment_manager_for_extensions),
restriction_product,
),
);
let goal_service = Arc::new(GoalService::new());
let thread_manager = Arc::new_cyclic(|thread_manager| {
ThreadManager::new(
@@ -364,7 +357,6 @@ impl MessageProcessor {
thread_manager: thread_manager.clone(),
goal_service: Arc::clone(&goal_service),
environment_manager: Arc::clone(&environment_manager_for_extensions),
executor_skill_provider: Arc::clone(&executor_skill_provider),
thread_store: Arc::clone(&thread_store),
},
),
+29 -8
View File
@@ -33,7 +33,13 @@ pub struct McpResourceReadResult {
/// snapshot, so calls automatically use replacements installed during startup and refresh.
#[derive(Clone)]
pub struct McpResourceClient {
manager: Arc<ArcSwap<McpConnectionManager>>,
manager: ResourceManager,
}
#[derive(Clone)]
enum ResourceManager {
Live(Arc<ArcSwap<McpConnectionManager>>),
Snapshot(Arc<McpConnectionManager>),
}
/// Opaque identity for the manager currently used by an MCP resource client.
@@ -59,19 +65,36 @@ impl std::fmt::Debug for McpResourceClient {
impl McpResourceClient {
/// Creates a resource client backed by the session's replaceable MCP manager.
pub fn new(manager: Arc<ArcSwap<McpConnectionManager>>) -> Self {
Self { manager }
Self {
manager: ResourceManager::Live(manager),
}
}
/// Pins resource calls to the manager generation that is current now.
pub fn snapshot(&self) -> Self {
Self {
manager: ResourceManager::Snapshot(self.manager_snapshot()),
}
}
/// Returns the exact manager generation used by this client right now.
pub fn manager_snapshot(&self) -> Arc<McpConnectionManager> {
match &self.manager {
ResourceManager::Live(manager) => manager.load_full(),
ResourceManager::Snapshot(manager) => Arc::clone(manager),
}
}
/// Returns an identity that changes whenever the published manager changes.
pub fn cache_key(&self) -> McpResourceClientCacheKey {
McpResourceClientCacheKey(Arc::downgrade(&self.manager.load_full()))
McpResourceClientCacheKey(Arc::downgrade(&self.manager_snapshot()))
}
/// Returns whether the current manager contains the named server.
///
/// This does not wait for server startup or imply that startup succeeded.
pub async fn has_server(&self, server: &str) -> bool {
self.manager.load_full().contains_server(server)
self.manager_snapshot().contains_server(server)
}
/// Lists one resource page from the named server.
@@ -83,8 +106,7 @@ impl McpResourceClient {
let params =
cursor.map(|cursor| PaginatedRequestParams::default().with_cursor(Some(cursor)));
let result = self
.manager
.load_full()
.manager_snapshot()
.list_resources(server, params)
.await?;
let resources = result
@@ -101,8 +123,7 @@ impl McpResourceClient {
/// Reads one resource from the named server.
pub async fn read_resource(&self, server: &str, uri: &str) -> Result<McpResourceReadResult> {
let result = self
.manager
.load_full()
.manager_snapshot()
.read_resource(server, ReadResourceRequestParams::new(uri.to_string()))
.await?;
let contents = result
+5
View File
@@ -9,6 +9,7 @@ pub mod render;
mod root_loader;
pub mod runtime;
mod runtime_selection;
mod runtime_snapshot;
pub mod service;
mod skill_instructions;
pub mod system;
@@ -36,6 +37,10 @@ pub use render::default_skill_metadata_budget;
pub use render::render_available_skills_body;
pub use root_loader::PluginSkillSnapshots;
pub use runtime_selection::collect_runtime_skill_mentions;
pub use runtime_snapshot::RuntimeSkillInjection;
pub use runtime_snapshot::RuntimeSkillInjections;
pub use runtime_snapshot::SkillInjectionIdentity;
pub use runtime_snapshot::SkillsSnapshot;
pub use service::SkillsLoadInput;
pub use service::SkillsService;
pub use skill_instructions::SkillInstructions;
+4
View File
@@ -148,6 +148,10 @@ impl HostSkillsSnapshot {
self.outcome.as_ref()
}
pub(crate) fn outcome_arc(&self) -> Arc<SkillLoadOutcome> {
Arc::clone(&self.outcome)
}
pub async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result<String> {
let fs = self
.outcome
+48 -10
View File
@@ -1,4 +1,5 @@
use std::any::Any;
use std::collections::HashMap;
use std::fmt;
use std::future::Future;
use std::hash::Hash;
@@ -306,7 +307,7 @@ pub trait SkillSource: Send + Sync {
#[derive(Clone)]
struct RegisteredSkillSource {
label: String,
source: Arc<dyn SkillSource>,
source: Arc<dyn Fn() -> Arc<dyn SkillSource> + Send + Sync>,
}
/// Bound skill sources used to build and read one runtime catalog.
@@ -321,6 +322,18 @@ impl SkillSources {
}
pub fn with_source(mut self, label: impl Into<String>, source: Arc<dyn SkillSource>) -> Self {
self.sources.push(RegisteredSkillSource {
label: label.into(),
source: Arc::new(move || Arc::clone(&source)),
});
self
}
pub fn with_source_factory(
mut self,
label: impl Into<String>,
source: Arc<dyn Fn() -> Arc<dyn SkillSource> + Send + Sync>,
) -> Self {
self.sources.push(RegisteredSkillSource {
label: label.into(),
source,
@@ -328,28 +341,53 @@ impl SkillSources {
self
}
pub fn extend(&mut self, other: Self) {
self.sources.extend(other.sources);
}
pub async fn list(&self) -> SkillCatalog {
self.list_with_sources().await.0
}
pub(crate) async fn list_with_sources(
&self,
) -> (
SkillCatalog,
HashMap<(SkillAuthority, SkillPackageId), Arc<dyn SkillSource>>,
) {
let mut catalog = SkillCatalog::default();
let mut sources = HashMap::new();
for source in &self.sources {
match source.source.list().await {
Ok(source_catalog) => catalog.extend(source_catalog),
let bound_source = (source.source)();
match bound_source.list().await {
Ok(source_catalog) => {
catalog.warnings.extend(source_catalog.warnings);
for entry in source_catalog.entries {
let key = (entry.authority.clone(), entry.id.clone());
if let std::collections::hash_map::Entry::Vacant(route) = sources.entry(key)
{
route.insert(Arc::clone(&bound_source));
catalog.entries.push(entry);
}
}
}
Err(err) => catalog.warnings.push(format!(
"{} skills unavailable: {}",
source.label, err.message
)),
}
}
catalog
(catalog, sources)
}
pub async fn list_kind(&self, kind: &SkillSourceKind) -> SkillSourceResult<SkillCatalog> {
let mut catalog = SkillCatalog::default();
for source in self
.sources
.iter()
.filter(|source| source.source.kind() == *kind)
{
let source_catalog = source.source.list().await.map_err(|err| {
for source in self.sources.iter() {
let bound_source = (source.source)();
if bound_source.kind() != *kind {
continue;
}
let source_catalog = bound_source.list().await.map_err(|err| {
SkillSourceError::new(format!(
"{} skills unavailable: {}",
source.label, err.message
@@ -0,0 +1,483 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::sync::Arc;
use codex_context_fragments::ContextualUserFragment;
use codex_exec_server::ResolvedSelectedCapabilityRoot;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::protocol::Product;
use codex_protocol::user_input::UserInput;
use crate::AvailableSkills;
use crate::HostSkillsSnapshot;
use crate::SkillInstructions;
use crate::SkillMetadata;
use crate::collect_runtime_skill_mentions;
use crate::default_skill_metadata_budget;
use crate::loader::EnvironmentSkillMetadata;
use crate::loader::load_environment_skills_from_root;
use crate::render::SkillRenderSideEffects;
use crate::render::build_available_skills_from_catalog;
use crate::runtime::SkillAuthority;
use crate::runtime::SkillCatalog;
use crate::runtime::SkillCatalogEntry;
use crate::runtime::SkillPackageId;
use crate::runtime::SkillReadRequest;
use crate::runtime::SkillReadResult;
use crate::runtime::SkillResourceId;
use crate::runtime::SkillSource;
use crate::runtime::SkillSourceError;
use crate::runtime::SkillSourceFuture;
use crate::runtime::SkillSourceIdentity;
use crate::runtime::SkillSourceKind;
use crate::runtime::SkillSources;
const MAX_INJECTIONS_PER_STEP: usize = 8;
const MAX_INJECTION_BYTES_PER_STEP: usize = 32 * 1024;
const HOST_AUTHORITY_ID: &str = "host";
type EntryKey = (SkillAuthority, SkillPackageId);
/// Identity of one injected package and the runtime owner that supplied it.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct SkillInjectionIdentity {
source: SkillSourceIdentity,
authority: SkillAuthority,
package: SkillPackageId,
}
#[derive(Debug)]
pub struct RuntimeSkillInjection {
pub identity: SkillInjectionIdentity,
pub entry: SkillCatalogEntry,
pub instructions: SkillInstructions,
}
#[derive(Debug, Default)]
pub struct RuntimeSkillInjections {
pub items: Vec<RuntimeSkillInjection>,
pub warnings: Vec<String>,
}
/// One immutable, authority-aware skill view used by a model sampling step.
#[derive(Clone)]
pub struct SkillsSnapshot {
host: HostSkillsSnapshot,
catalog: Arc<SkillCatalog>,
sources: Arc<HashMap<EntryKey, Arc<dyn SkillSource>>>,
available: Arc<Option<AvailableSkills>>,
warnings: Arc<Vec<String>>,
}
impl SkillsSnapshot {
pub fn from_host(host: HostSkillsSnapshot, context_window: Option<i64>) -> Self {
let source: Arc<dyn SkillSource> = Arc::new(HostSkillSource::new(host.clone()));
let catalog = host_catalog(&host);
let source_by_entry = catalog
.entries
.iter()
.map(|entry| {
(
(entry.authority.clone(), entry.id.clone()),
Arc::clone(&source),
)
})
.collect();
Self::new(host, catalog, source_by_entry, context_window)
}
pub async fn load(
host: HostSkillsSnapshot,
executor_roots: &[ResolvedSelectedCapabilityRoot],
extra_sources: Option<&SkillSources>,
restriction_product: Option<Product>,
context_window: Option<i64>,
) -> Self {
let mut sources =
SkillSources::new().with_source("host", Arc::new(HostSkillSource::new(host.clone())));
for root in executor_roots {
sources = sources.with_source(
format!("executor root `{}`", root.selected_root().id),
Arc::new(ExecutorSkillSource::new(root.clone(), restriction_product)),
);
}
if let Some(extra_sources) = extra_sources {
sources.extend(extra_sources.clone());
}
let (catalog, source_by_entry) = sources.list_with_sources().await;
Self::new(host, catalog, source_by_entry, context_window)
}
fn new(
host: HostSkillsSnapshot,
catalog: SkillCatalog,
source_by_entry: HashMap<EntryKey, Arc<dyn SkillSource>>,
context_window: Option<i64>,
) -> Self {
let available = build_available_skills_from_catalog(
&catalog,
Some(host.outcome()),
default_skill_metadata_budget(context_window),
SkillRenderSideEffects::None,
);
let mut warnings = catalog.warnings.clone();
if let Some(warning) = available
.as_ref()
.and_then(|available| available.warning_message.clone())
{
warnings.push(warning);
}
Self {
host,
catalog: Arc::new(catalog),
sources: Arc::new(source_by_entry),
available: Arc::new(available),
warnings: Arc::new(warnings),
}
}
pub fn available(&self) -> Option<&AvailableSkills> {
self.available.as_ref().as_ref()
}
pub fn warnings(&self) -> &[String] {
self.warnings.as_ref()
}
pub fn skill_name_counts_lower(&self) -> HashMap<String, usize> {
let mut counts = HashMap::new();
for entry in self.catalog.entries.iter().filter(|entry| entry.enabled) {
*counts.entry(entry.name.to_ascii_lowercase()).or_default() += 1;
}
counts
}
pub fn host_skill(&self, entry: &SkillCatalogEntry) -> Option<&SkillMetadata> {
if entry.authority.kind != SkillSourceKind::Host {
return None;
}
self.host
.outcome()
.skills
.iter()
.find(|skill| skill.path_to_skills_md.to_string_lossy() == entry.main_prompt.as_str())
}
pub async fn injections(
&self,
input: &[UserInput],
plain_name_conflicts: &HashSet<String>,
already_injected: &HashSet<SkillInjectionIdentity>,
already_injected_names: &HashSet<String>,
) -> RuntimeSkillInjections {
let selected = collect_runtime_skill_mentions(input, &self.catalog, plain_name_conflicts);
let mut result = RuntimeSkillInjections::default();
let mut injected_names = already_injected_names.clone();
let mut remaining_bytes = MAX_INJECTION_BYTES_PER_STEP;
let mut injection_limit_reached = false;
for entry in &selected {
let key = (entry.authority.clone(), entry.id.clone());
let Some(source) = self.sources.get(&key) else {
result.warnings.push(format!(
"Failed to load skill `{}`: its runtime source is unavailable.",
entry.name
));
continue;
};
let identity = SkillInjectionIdentity {
source: source.identity(),
authority: entry.authority.clone(),
package: entry.id.clone(),
};
if already_injected.contains(&identity) {
continue;
}
let replaces_previous = injected_names.contains(&entry.name);
if result.items.len() == MAX_INJECTIONS_PER_STEP {
injection_limit_reached = true;
break;
}
let request = SkillReadRequest {
authority: entry.authority.clone(),
package: entry.id.clone(),
resource: entry.main_prompt.clone(),
};
match source.read(request).await {
Ok(read) => {
let Some((instructions, truncated)) =
SkillInstructions::from_runtime_with_total_limit(
entry,
&read.contents,
remaining_bytes,
replaces_previous,
)
else {
result.warnings.push(
"Additional selected skills were not loaded because this step reached the 32 KB skill instruction limit."
.to_string(),
);
break;
};
let instruction_bytes = instructions.render().len();
remaining_bytes = remaining_bytes.saturating_sub(instruction_bytes);
if truncated {
result.warnings.push(format!(
"Skill `{}` exceeded the step's instruction limit and was truncated.",
entry.name
));
}
result.items.push(RuntimeSkillInjection {
identity,
entry: entry.clone(),
instructions,
});
injected_names.insert(entry.name.clone());
}
Err(err) => result
.warnings
.push(format!("Failed to load skill `{}`: {err}", entry.name)),
}
}
if injection_limit_reached {
result.warnings.push(format!(
"Only the first {MAX_INJECTIONS_PER_STEP} selected skills were loaded for this step."
));
}
result
}
}
impl fmt::Debug for SkillsSnapshot {
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter
.debug_struct("SkillsSnapshot")
.field("catalog", &self.catalog)
.field("available", &self.available)
.finish_non_exhaustive()
}
}
struct HostSkillSource {
snapshot: HostSkillsSnapshot,
identity: SkillSourceIdentity,
}
impl HostSkillSource {
fn new(snapshot: HostSkillsSnapshot) -> Self {
Self {
identity: SkillSourceIdentity::from_owner(snapshot.outcome_arc()),
snapshot,
}
}
}
impl SkillSource for HostSkillSource {
fn kind(&self) -> SkillSourceKind {
SkillSourceKind::Host
}
fn identity(&self) -> SkillSourceIdentity {
self.identity.clone()
}
fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> {
Box::pin(async move { Ok(host_catalog(&self.snapshot)) })
}
fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> {
Box::pin(async move {
if request.authority != SkillAuthority::new(SkillSourceKind::Host, HOST_AUTHORITY_ID) {
return Err(SkillSourceError::new("host skill authority does not match"));
}
let Some(skill) = self.snapshot.outcome().skills.iter().find(|skill| {
skill.path_to_skills_md.to_string_lossy() == request.resource.as_str()
}) else {
return Err(SkillSourceError::new(format!(
"host skill resource is not loaded: {}",
request.resource.as_str()
)));
};
let contents = self.snapshot.read_skill_text(skill).await.map_err(|err| {
SkillSourceError::new(format!(
"failed to read host skill resource {}: {err}",
request.resource.as_str()
))
})?;
Ok(SkillReadResult {
resource: request.resource,
contents,
})
})
}
}
fn host_catalog(snapshot: &HostSkillsSnapshot) -> SkillCatalog {
let outcome = snapshot.outcome();
let mut catalog = SkillCatalog {
warnings: outcome
.errors
.iter()
.map(|error| {
format!(
"Failed to load skill at {}: {}",
error.path.display(),
error.message
)
})
.collect(),
..Default::default()
};
for (skill, enabled) in outcome.skills_with_enabled() {
let path = skill.path_to_skills_md.to_string_lossy().into_owned();
let mut entry = SkillCatalogEntry::new(
SkillPackageId(path.clone()),
SkillAuthority::new(SkillSourceKind::Host, HOST_AUTHORITY_ID),
skill.name.clone(),
skill.description.clone(),
SkillResourceId::new(path.clone()),
)
.with_short_description(skill.short_description.clone())
.with_display_path(path.replace('\\', "/"))
.with_dependencies(skill.dependencies.clone());
if !enabled {
entry = entry.disabled();
}
if !skill.allows_implicit_invocation() {
entry = entry.hidden_from_prompt();
}
catalog.push_entry(entry);
}
catalog
}
struct ExecutorSkillSource {
root: ResolvedSelectedCapabilityRoot,
authority: SkillAuthority,
restriction_product: Option<Product>,
identity: SkillSourceIdentity,
}
impl ExecutorSkillSource {
fn new(root: ResolvedSelectedCapabilityRoot, restriction_product: Option<Product>) -> Self {
Self {
authority: SkillAuthority::new(
SkillSourceKind::Executor,
root.selected_root().id.clone(),
),
identity: SkillSourceIdentity::from_owner(Arc::clone(root.environment())),
root,
restriction_product,
}
}
}
impl SkillSource for ExecutorSkillSource {
fn kind(&self) -> SkillSourceKind {
SkillSourceKind::Executor
}
fn identity(&self) -> SkillSourceIdentity {
self.identity.clone()
}
fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> {
Box::pin(async move {
let CapabilityRootLocation::Environment {
environment_id,
path,
} = &self.root.selected_root().location;
let outcome = load_environment_skills_from_root(
self.root.file_system().as_ref(),
path,
self.restriction_product,
)
.await;
let mut catalog = SkillCatalog {
warnings: outcome.warnings,
..Default::default()
};
for skill in outcome.skills {
catalog.push_entry(executor_catalog_entry(
skill,
self.authority.clone(),
&self.root.selected_root().id,
environment_id,
));
}
Ok(catalog)
})
}
fn read(&self, request: SkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> {
Box::pin(async move {
if request.authority != self.authority || request.package.0 != request.resource.as_str()
{
return Err(SkillSourceError::new(
"executor skill resource does not match its captured source",
));
}
let CapabilityRootLocation::Environment { environment_id, .. } =
&self.root.selected_root().location;
let Some((resource_environment, path)) = request.resource.environment_path() else {
return Err(SkillSourceError::new(
"executor skill resource has no environment path",
));
};
if resource_environment != environment_id {
return Err(SkillSourceError::new(
"executor skill resource belongs to a different environment",
));
}
let contents = self
.root
.file_system()
.read_file_text(path, /*sandbox*/ None)
.await
.map_err(|err| {
SkillSourceError::new(format!(
"failed to read executor skill resource {}: {err}",
request.resource.as_str()
))
})?;
Ok(SkillReadResult {
resource: request.resource,
contents,
})
})
}
}
fn executor_catalog_entry(
skill: EnvironmentSkillMetadata,
authority: SkillAuthority,
root_id: &str,
environment_id: &str,
) -> SkillCatalogEntry {
let prompt_visible = skill.allows_implicit_invocation();
let path = skill.path_to_skills_md.inferred_native_path_string();
let display_path = format!(
"skill://{root_id}/{}",
path.replace('\\', "/").trim_start_matches('/')
);
let entry = SkillCatalogEntry::new(
SkillPackageId(display_path.clone()),
authority,
skill.name,
skill.description,
SkillResourceId::environment(
display_path.clone(),
environment_id,
skill.path_to_skills_md,
),
)
.with_short_description(skill.short_description)
.with_display_path(display_path)
.with_dependencies(skill.dependencies);
if prompt_visible {
entry
} else {
entry.hidden_from_prompt()
}
}
+33 -1
View File
@@ -6,12 +6,14 @@ use crate::runtime::SkillCatalogEntry;
const MAX_SKILL_NAME_BYTES: usize = 256;
const MAX_SKILL_PATH_BYTES: usize = 1_024;
const MAX_SKILL_BODY_BYTES: usize = 8_000;
const REPLACEMENT_NOTICE: &str = "<replacement_notice>These instructions replace the previously provided instructions for this skill.</replacement_notice>\n";
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillInstructions {
name: String,
path: String,
contents: String,
replaces_previous: bool,
}
impl From<&SkillInjection> for SkillInstructions {
@@ -20,6 +22,7 @@ impl From<&SkillInjection> for SkillInstructions {
name: skill.name.clone(),
path: skill.path.clone(),
contents: skill.contents.clone(),
replaces_previous: false,
}
}
}
@@ -33,10 +36,34 @@ impl SkillInstructions {
name: truncate_utf8(&entry.name, MAX_SKILL_NAME_BYTES).0,
path: truncate_utf8(entry.rendered_path(), MAX_SKILL_PATH_BYTES).0,
contents,
replaces_previous: false,
},
truncated,
)
}
pub(crate) fn from_runtime_with_total_limit(
entry: &SkillCatalogEntry,
contents: &str,
max_total_bytes: usize,
replaces_previous: bool,
) -> Option<(Self, bool)> {
let name = truncate_utf8(&entry.name, MAX_SKILL_NAME_BYTES).0;
let path = truncate_utf8(entry.rendered_path(), MAX_SKILL_PATH_BYTES).0;
let mut instructions = Self {
name,
path,
contents: String::new(),
replaces_previous,
};
let envelope_bytes = instructions.render().len();
let max_body_bytes = max_total_bytes
.checked_sub(envelope_bytes)?
.min(MAX_SKILL_BODY_BYTES);
let (contents, truncated) = truncate_utf8(contents, max_body_bytes);
instructions.contents = contents;
Some((instructions, truncated))
}
}
fn truncate_utf8(value: &str, max_bytes: usize) -> (String, bool) {
@@ -61,8 +88,13 @@ impl ContextualUserFragment for SkillInstructions {
}
fn body(&self) -> String {
let replacement_notice = if self.replaces_previous {
REPLACEMENT_NOTICE
} else {
""
};
format!(
"\n<name>{}</name>\n<path>{}</path>\n{}\n",
"\n<name>{}</name>\n<path>{}</path>\n{replacement_notice}{}\n",
self.name, self.path, self.contents
)
}
@@ -1,50 +0,0 @@
use codex_core_skills::AvailableSkills;
use codex_core_skills::render_available_skills_body;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
use super::ContextualUserFragment;
/// Model-context fragment describing the skills available to Codex.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AvailableSkillsInstructions {
skill_root_lines: Vec<String>,
skill_lines: Vec<String>,
}
impl AvailableSkillsInstructions {
/// Creates a skills context fragment from pre-rendered catalog lines.
pub fn from_skill_lines(skill_lines: Vec<String>) -> Self {
Self {
skill_root_lines: Vec::new(),
skill_lines,
}
}
}
impl From<AvailableSkills> for AvailableSkillsInstructions {
fn from(available_skills: AvailableSkills) -> Self {
Self {
skill_root_lines: available_skills.skill_root_lines,
skill_lines: available_skills.skill_lines,
}
}
}
impl ContextualUserFragment for AvailableSkillsInstructions {
fn role(&self) -> &'static str {
"developer"
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
fn type_markers() -> (&'static str, &'static str) {
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG)
}
fn body(&self) -> String {
render_available_skills_body(&self.skill_root_lines, &self.skill_lines)
}
}
-2
View File
@@ -3,7 +3,6 @@
mod approved_command_prefix_saved;
mod apps_instructions;
mod available_plugins_instructions;
mod available_skills_instructions;
mod collaboration_mode_instructions;
mod contextual_user_message;
mod current_time_reminder;
@@ -37,7 +36,6 @@ pub(crate) mod world_state;
pub(crate) use approved_command_prefix_saved::ApprovedCommandPrefixSaved;
pub(crate) use apps_instructions::AppsInstructions;
pub(crate) use available_plugins_instructions::AvailablePluginsInstructions;
pub use available_skills_instructions::AvailableSkillsInstructions;
pub(crate) use codex_context_fragments::AdditionalContextDeveloperFragment;
pub(crate) use codex_context_fragments::AdditionalContextUserFragment;
pub use codex_context_fragments::ContextualUserFragment;
@@ -1,5 +1,6 @@
mod agents_md;
mod environment;
mod skills;
use crate::context::ContextualUserFragment;
use codex_protocol::models::ContentItem;
@@ -14,6 +15,7 @@ use std::fmt;
pub(crate) use agents_md::AgentsMdState;
pub(crate) use environment::EnvironmentsState;
pub(crate) use skills::SkillsState;
trait ErasedWorldStateSection: Send + Sync {
fn snapshot(&self) -> Option<Value>;
@@ -0,0 +1,138 @@
use codex_core_skills::SkillsSnapshot;
use codex_core_skills::render_available_skills_body;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
use serde::Deserialize;
use serde::Serialize;
use super::PreviousSectionState;
use super::WorldStateSection;
use crate::context::ContextualUserFragment;
const REPLACEMENT_NOTICE: &str =
"This complete skills list replaces the previously provided skills list.";
const REMOVAL_NOTICE: &str = "The previously provided skills list no longer applies.";
#[derive(Clone, Debug, Default)]
pub(crate) struct SkillsState {
skill_root_lines: Vec<String>,
skill_lines: Vec<String>,
}
#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq, Serialize)]
pub(crate) struct SkillsStateSnapshot {
skill_root_lines: Vec<String>,
skill_lines: Vec<String>,
}
impl SkillsState {
pub(crate) fn new(skills: &SkillsSnapshot, enabled: bool) -> Self {
if !enabled {
return Self::default();
}
let Some(available) = skills.available() else {
return Self::default();
};
Self {
skill_root_lines: available.skill_root_lines.clone(),
skill_lines: available.skill_lines.clone(),
}
}
fn has_catalog(&self) -> bool {
!self.skill_lines.is_empty()
}
}
impl WorldStateSection for SkillsState {
const ID: &'static str = "skills";
type Snapshot = SkillsStateSnapshot;
fn snapshot(&self) -> Self::Snapshot {
SkillsStateSnapshot {
skill_root_lines: self.skill_root_lines.clone(),
skill_lines: self.skill_lines.clone(),
}
}
fn matches_legacy_fragment(role: &str, text: &str) -> bool {
let text = text.trim();
role == "developer"
&& text.starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG)
&& text.ends_with(SKILLS_INSTRUCTIONS_CLOSE_TAG)
}
fn render_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> Option<Box<dyn ContextualUserFragment>> {
let current = self.snapshot();
if matches!(previous, PreviousSectionState::Known(previous) if previous == &current) {
return None;
}
let previous_had_catalog = match previous {
PreviousSectionState::Known(previous) => !previous.skill_lines.is_empty(),
PreviousSectionState::Unknown => true,
PreviousSectionState::Absent => false,
};
if !self.has_catalog() && !previous_had_catalog {
return None;
}
Some(Box::new(SkillsStateFragment {
skill_root_lines: self.skill_root_lines.clone(),
skill_lines: self.skill_lines.clone(),
include_policy: !previous_had_catalog,
notice: if !self.has_catalog() {
Some(REMOVAL_NOTICE)
} else if previous_had_catalog {
Some(REPLACEMENT_NOTICE)
} else {
None
},
}))
}
}
struct SkillsStateFragment {
skill_root_lines: Vec<String>,
skill_lines: Vec<String>,
include_policy: bool,
notice: Option<&'static str>,
}
impl ContextualUserFragment for SkillsStateFragment {
fn role(&self) -> &'static str {
"developer"
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
fn type_markers() -> (&'static str, &'static str) {
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG)
}
fn body(&self) -> String {
let catalog = (!self.skill_lines.is_empty()).then(|| {
if self.include_policy {
render_available_skills_body(&self.skill_root_lines, &self.skill_lines)
} else {
let mut lines = vec!["## Skills".to_string()];
if !self.skill_root_lines.is_empty() {
lines.push("### Skill roots".to_string());
lines.extend(self.skill_root_lines.iter().cloned());
}
lines.push("### Available skills".to_string());
lines.extend(self.skill_lines.iter().cloned());
format!("\n{}\n", lines.join("\n"))
}
});
match (self.notice, catalog) {
(Some(notice), Some(catalog)) => format!("\n{notice}\n{catalog}"),
(Some(notice), None) => format!("\n{notice}\n"),
(None, Some(catalog)) => catalog,
(None, None) => String::new(),
}
}
}
-8
View File
@@ -78,7 +78,6 @@ pub(crate) mod prompt_debug;
pub use prompt_debug::build_prompt_input;
pub(crate) mod mentions {
pub(crate) use crate::plugins::build_connector_slug_counts;
pub(crate) use crate::plugins::build_skill_name_counts;
pub(crate) use crate::plugins::collect_explicit_app_ids;
pub(crate) use crate::plugins::collect_explicit_plugin_mentions;
pub(crate) use crate::plugins::collect_tool_mentions_from_messages;
@@ -88,14 +87,7 @@ pub mod sandboxing;
mod session_prefix;
mod session_startup_prewarm;
pub mod skills;
pub(crate) use skills::SkillInjections;
pub(crate) use skills::SkillMetadata;
pub(crate) use skills::SkillsService;
pub(crate) use skills::build_available_skills;
pub(crate) use skills::build_skill_injections;
pub(crate) use skills::build_skill_name_counts;
pub(crate) use skills::collect_explicit_skill_mentions;
pub(crate) use skills::default_skill_metadata_budget;
pub(crate) use skills::injection;
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
pub(crate) use skills::skills_load_input_from_config;
+19 -17
View File
@@ -15,10 +15,10 @@ use codex_rmcp_client::perform_oauth_login;
use tokio_util::sync::CancellationToken;
use tracing::warn;
use crate::SkillMetadata;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::skills::model::SkillToolDependency;
use codex_core_skills::runtime::SkillCatalogEntry;
use codex_mcp::ElicitationReviewerHandle;
use codex_mcp::McpOAuthLoginSupport;
use codex_mcp::McpPermissionPromptAutoApproveContext;
@@ -35,13 +35,13 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
sess: &Session,
turn_context: &TurnContext,
cancellation_token: &CancellationToken,
mentioned_skills: &[SkillMetadata],
mentioned_skills: &[SkillCatalogEntry],
elicitation_reviewer: Option<ElicitationReviewerHandle>,
) {
) -> bool {
let originator_value = originator().value;
if !is_first_party_originator(originator_value.as_str()) {
// Only support first-party clients for now.
return;
return false;
}
let config = turn_context.config.clone();
@@ -50,24 +50,24 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
.features
.enabled(codex_features::Feature::SkillMcpDependencyInstall)
{
return;
return false;
}
let installed = sess.runtime_mcp_servers(config.as_ref()).await;
let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed);
if missing.is_empty() {
return;
return false;
}
let unprompted_missing = filter_prompted_mcp_dependencies(sess, &missing).await;
if unprompted_missing.is_empty() {
return;
return false;
}
if should_install_mcp_dependencies(sess, turn_context, &unprompted_missing, cancellation_token)
.await
{
maybe_install_mcp_dependencies(
return maybe_install_mcp_dependencies(
sess,
turn_context,
config.as_ref(),
@@ -76,35 +76,36 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies(
)
.await;
}
false
}
pub(crate) async fn maybe_install_mcp_dependencies(
sess: &Session,
turn_context: &TurnContext,
config: &crate::config::Config,
mentioned_skills: &[SkillMetadata],
mentioned_skills: &[SkillCatalogEntry],
elicitation_reviewer: Option<ElicitationReviewerHandle>,
) {
) -> bool {
if mentioned_skills.is_empty()
|| !config
.features
.enabled(codex_features::Feature::SkillMcpDependencyInstall)
{
return;
return false;
}
let codex_home = config.codex_home.clone();
let installed = sess.runtime_mcp_servers(config).await;
let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed);
if missing.is_empty() {
return;
return false;
}
let mut servers = match load_global_mcp_servers(&codex_home).await {
Ok(servers) => servers,
Err(err) => {
warn!("failed to load MCP servers while installing skill dependencies: {err}");
return;
return false;
}
};
@@ -120,7 +121,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
}
if !updated {
return;
return false;
}
if let Err(err) = ConfigEditsBuilder::new(&codex_home)
@@ -129,7 +130,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
.await
{
warn!("failed to persist MCP dependencies for mentioned skills: {err}");
return;
return false;
}
for (name, server_config) in added {
@@ -197,7 +198,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
}
if let Err(err) = refresh_config.mcp_servers.set(configured_servers) {
warn!("failed to refresh MCP dependencies for mentioned skills: {err}");
return;
return false;
}
let refresh_servers = sess.runtime_mcp_servers(&refresh_config).await;
sess.refresh_mcp_servers_now(
@@ -208,6 +209,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
elicitation_reviewer,
)
.await;
true
}
async fn should_install_mcp_dependencies(
@@ -416,7 +418,7 @@ fn mcp_dependency_to_server_config(
}
fn collect_missing_mcp_dependencies(
mentioned_skills: &[SkillMetadata],
mentioned_skills: &[SkillCatalogEntry],
installed: &HashMap<String, McpServerConfig>,
) -> HashMap<String, McpServerConfig> {
let mut missing = HashMap::new();
-2
View File
@@ -102,8 +102,6 @@ pub(crate) fn collect_explicit_plugin_mentions(
.collect()
}
pub(crate) use crate::build_skill_name_counts;
pub(crate) fn build_connector_slug_counts(
connectors: &[connectors::AppInfo],
) -> HashMap<String, usize> {
-1
View File
@@ -12,7 +12,6 @@ pub(crate) use injection::build_plugin_injections;
pub(crate) use render::render_explicit_plugin_instructions;
pub(crate) use mentions::build_connector_slug_counts;
pub(crate) use mentions::build_skill_name_counts;
pub(crate) use mentions::collect_explicit_app_ids;
pub(crate) use mentions::collect_explicit_plugin_mentions;
pub(crate) use mentions::collect_tool_mentions_from_messages;
+17 -29
View File
@@ -15,7 +15,6 @@ use crate::agent::AgentStatus;
use crate::agent::agent_status_from_event;
use crate::agent::status::is_final;
use crate::attestation::AttestationProvider;
use crate::build_available_skills;
use crate::compact;
use crate::config::ManagedFeatures;
use crate::config::resolve_tool_suggest_config_from_layer_stack;
@@ -23,7 +22,6 @@ use crate::connectors;
use crate::context::ApprovedCommandPrefixSaved;
use crate::context::AppsInstructions;
use crate::context::AvailablePluginsInstructions;
use crate::context::AvailableSkillsInstructions;
use crate::context::CollaborationModeInstructions;
use crate::context::ContextualUserFragment;
use crate::context::MultiAgentModeInstructions;
@@ -33,7 +31,6 @@ use crate::context::PersonalitySpecInstructions;
use crate::context::RecommendedPluginsInstructions;
use crate::context::world_state::WorldState;
use crate::current_time::TimeProvider;
use crate::default_skill_metadata_budget;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::exec_policy::ExecPolicyManager;
use crate::image_preparation::prepare_response_items;
@@ -42,7 +39,6 @@ use crate::realtime_conversation::RealtimeConversationManager;
use crate::session::step_context::StepContext;
use crate::session::turn_context::TurnEnvironment;
use crate::session_prefix::format_inter_agent_completion_message;
use crate::skills::SkillRenderSideEffects;
use crate::skills_load_input_from_config;
use crate::turn_metadata::TurnMetadataState;
use crate::turn_timing::now_unix_timestamp_ms;
@@ -303,8 +299,6 @@ pub(crate) struct PreviousTurnSettings {
pub(crate) realtime_active: Option<bool>,
}
#[cfg(test)]
use crate::SkillMetadata;
use crate::SkillsService;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::guardian::GuardianReviewSessionManager;
@@ -315,6 +309,8 @@ use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
use crate::shell;
#[cfg(test)]
use crate::skills::SkillLoadOutcome;
#[cfg(test)]
use crate::skills::SkillMetadata;
use crate::state::AutoCompactWindowIds;
use crate::state::AutoCompactWindowSnapshot;
use crate::state::PendingRequestPermissions;
@@ -2834,10 +2830,25 @@ impl Session {
.environment_manager()
.resolve_selected_capability_roots(&self.services.selected_capability_roots)
.await;
let extra_skill_sources = self
.services
.thread_extension_data
.get::<codex_core_skills::runtime::SkillSources>();
let skills = Arc::new(
codex_core_skills::SkillsSnapshot::load(
turn_context.turn_skills.snapshot.clone(),
&selected_capability_roots,
extra_skill_sources.as_deref(),
turn_context.session_source.restriction_product(),
turn_context.model_context_window(),
)
.await,
);
Arc::new(StepContext::new(
turn_context,
environments,
selected_capability_roots,
skills,
loaded_agents_md,
))
}
@@ -3209,29 +3220,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
@@ -3,6 +3,7 @@ use std::sync::Arc;
use crate::agents_md::LoadedAgentsMd;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::session::turn_context::TurnContext;
use codex_core_skills::SkillsSnapshot;
use codex_exec_server::ResolvedSelectedCapabilityRoot;
/// Request-scoped state that may change between model sampling requests.
@@ -12,6 +13,8 @@ pub(crate) struct StepContext {
pub(crate) environments: TurnEnvironmentSnapshot,
/// Capability roots bound to ready environments in this exact step.
pub(crate) selected_capability_roots: Vec<ResolvedSelectedCapabilityRoot>,
/// Complete skill catalog and exact read routes captured for this step.
pub(crate) skills: Arc<SkillsSnapshot>,
/// The canonical AGENTS.md value observed with this environment snapshot.
pub(crate) loaded_agents_md: Option<Arc<LoadedAgentsMd>>,
}
@@ -21,12 +24,14 @@ impl StepContext {
turn: Arc<TurnContext>,
environments: TurnEnvironmentSnapshot,
selected_capability_roots: Vec<ResolvedSelectedCapabilityRoot>,
skills: Arc<SkillsSnapshot>,
loaded_agents_md: Option<Arc<LoadedAgentsMd>>,
) -> Self {
Self {
turn,
environments,
selected_capability_roots,
skills,
loaded_agents_md,
}
}
+6 -56
View File
@@ -13,6 +13,7 @@ use crate::session::step_context::StepContext;
use crate::shell::default_user_shell;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills::SkillRenderSideEffects;
use crate::skills::build_available_skills;
use crate::skills::render::SkillMetadataBudget;
use crate::test_support::models_manager_with_provider;
use crate::tools::format_exec_output_str;
@@ -187,10 +188,15 @@ use std::time::Duration as StdDuration;
impl StepContext {
pub(crate) fn for_test(turn: Arc<TurnContext>) -> Arc<Self> {
let environments = turn.environments.clone();
let skills = Arc::new(codex_core_skills::SkillsSnapshot::from_host(
turn.turn_skills.snapshot.clone(),
turn.model_context_window(),
));
Arc::new(Self::new(
turn,
environments,
Vec::new(),
skills,
/*loaded_agents_md*/ None,
))
}
@@ -8451,62 +8457,6 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o
);
}
#[tokio::test]
async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_builds() {
let (session, turn_context, rx) = make_session_and_context_with_rx().await;
let mut turn_context = Arc::into_inner(turn_context).expect("sole thread settings owner");
let mut outcome = SkillLoadOutcome::default();
outcome.skills = vec![
SkillMetadata {
name: "admin-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(),
scope: SkillScope::Admin,
plugin_id: None,
},
SkillMetadata {
name: "repo-skill".to_string(),
description: "desc".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
},
];
turn_context.model_info.context_window = Some(100);
turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome)));
let turn_context = Arc::new(turn_context);
let _ = build_initial_context(&session, &turn_context).await;
let warning_event = timeout(Duration::from_secs(1), rx.recv())
.await
.expect("warning event should arrive")
.expect("warning event should be readable");
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
));
let _ = build_initial_context(&session, &turn_context).await;
let warning_event = timeout(Duration::from_secs(1), rx.recv())
.await
.expect("warning event should arrive on repeated build")
.expect("warning event should be readable");
assert!(matches!(
warning_event.msg,
EventMsg::Warning(WarningEvent { message })
if message == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list."
));
}
#[tokio::test]
async fn handle_output_item_done_records_image_save_history_message() {
let (session, turn_context) = make_session_and_context().await;
+187 -97
View File
@@ -4,12 +4,9 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::Ordering;
use crate::SkillInjections;
use crate::build_skill_injections;
use crate::client::ModelClientSession;
use crate::client_common::Prompt;
use crate::client_common::ResponseEvent;
use crate::collect_explicit_skill_mentions;
use crate::compact::InitialContextInjection;
use crate::compact::run_inline_auto_compact_task;
use crate::compact::should_use_remote_compact_task;
@@ -30,7 +27,6 @@ use crate::injection::tool_kind_for_path;
use crate::mcp_skill_dependencies::maybe_prompt_and_install_mcp_dependencies;
use crate::mcp_tool_exposure::build_mcp_tool_exposure;
use crate::mentions::build_connector_slug_counts;
use crate::mentions::build_skill_name_counts;
use crate::mentions::collect_explicit_app_ids;
use crate::mentions::collect_explicit_plugin_mentions;
use crate::mentions::collect_tool_mentions_from_messages;
@@ -71,11 +67,12 @@ use codex_analytics::AppInvocation;
use codex_analytics::CompactionPhase;
use codex_analytics::CompactionReason;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::TurnResolvedConfigFact;
use codex_analytics::build_track_events_context;
use codex_async_utils::OrCancelExt;
use codex_core_plugins::RecommendedPluginCandidatesInput;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_core_skills::SkillInjectionIdentity;
use codex_extension_api::TurnInputContext;
use codex_extension_api::TurnInputEnvironment;
use codex_features::Feature;
@@ -171,19 +168,28 @@ pub(crate) async fn run_turn(
.record_context_updates_and_set_reference_context_item(first_step_context.as_ref())
.await;
let Some((injection_items, explicitly_enabled_connectors)) =
build_skills_and_plugins(&sess, turn_context.as_ref(), &input, &cancellation_token).await
else {
return Ok(None);
};
if run_pending_session_start_hooks(&sess, &turn_context).await {
return Ok(None);
}
let mut can_drain_pending_input = input.is_empty();
if run_hooks_and_record_inputs(&sess, &turn_context, &input).await {
let (accepted_input, should_stop) =
run_hooks_and_record_inputs(&sess, &turn_context, &input).await;
if should_stop {
return Ok(None);
}
let mut accepted_user_input = user_input_from_turn_input(&accepted_input);
let Some((injection_items, explicitly_enabled_connectors, available_connectors)) =
build_skills_and_plugins(
&sess,
turn_context.as_ref(),
&accepted_input,
&cancellation_token,
)
.await
else {
return Ok(None);
};
sess.merge_connector_selection(explicitly_enabled_connectors.clone())
.await;
@@ -217,6 +223,7 @@ pub(crate) async fn run_turn(
// 2. After auto-compact, when model/tool continuation needs to resume before any steer.
let mut next_step_context = Some(first_step_context);
let mut skill_injection_state = SkillInjectionState::default();
loop {
// Note that pending_input would be something like a message the user
// submitted through the UI while the model was running. Though the UI
@@ -227,9 +234,12 @@ pub(crate) async fn run_turn(
Vec::new()
};
if run_hooks_and_record_inputs(&sess, &turn_context, &pending_input).await {
let (accepted_pending_input, should_stop) =
run_hooks_and_record_inputs(&sess, &turn_context, &pending_input).await;
if should_stop {
break;
}
accepted_user_input.extend(user_input_from_turn_input(&accepted_pending_input));
let window_id = sess.current_window_id().await;
super::rollout_budget::maybe_record_reminder(
@@ -252,15 +262,19 @@ pub(crate) async fn run_turn(
)
.await?;
if turn_context
.config
.features
.enabled(Feature::DeferredExecutor)
{
world_state = sess
.record_step_world_state_if_changed(&world_state, step_context.as_ref())
.await;
}
world_state = sess
.record_step_world_state_if_changed(&world_state, step_context.as_ref())
.await;
let _mcp_refreshed = record_skill_injections(
&sess,
turn_context.as_ref(),
step_context.as_ref(),
&accepted_user_input,
&available_connectors,
&cancellation_token,
&mut skill_injection_state,
)
.await;
// Construct the input that we will send to the model.
let sampling_request_input: Vec<ResponseItem> = async {
@@ -477,9 +491,10 @@ async fn run_hooks_and_record_inputs(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
input: &[TurnInput],
) -> bool {
) -> (Vec<TurnInput>, bool) {
let mut blocked_input = false;
let mut accepted_user_input = false;
let mut accepted_input = Vec::with_capacity(input.len());
for input_item in input {
let hook_outcome = inspect_pending_input(sess, turn_context, input_item).await;
if hook_outcome.should_stop {
@@ -496,9 +511,147 @@ async fn run_hooks_and_record_inputs(
hook_outcome.additional_contexts,
)
.await;
accepted_input.push(input_item.clone());
}
}
blocked_input && !accepted_user_input
(accepted_input, blocked_input && !accepted_user_input)
}
fn user_input_from_turn_input(input: &[TurnInput]) -> Vec<UserInput> {
input
.iter()
.filter_map(|item| match item {
TurnInput::UserInput { content, .. } => Some(content.as_slice()),
TurnInput::ResponseItem(_) | TurnInput::InterAgentCommunication(_) => None,
})
.flatten()
.cloned()
.collect()
}
#[derive(Default)]
struct SkillInjectionState {
injected: HashSet<SkillInjectionIdentity>,
injected_names: HashSet<String>,
emitted_warnings: HashSet<String>,
}
async fn record_skill_injections(
sess: &Arc<Session>,
turn_context: &TurnContext,
step_context: &StepContext,
input: &[UserInput],
available_connectors: &[connectors::AppInfo],
cancellation_token: &CancellationToken,
state: &mut SkillInjectionState,
) -> bool {
if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) {
return false;
}
for warning in step_context.skills.warnings() {
emit_skill_warning_once(
sess,
turn_context,
warning.clone(),
&mut state.emitted_warnings,
)
.await;
}
let plain_name_conflicts = build_connector_slug_counts(available_connectors)
.into_keys()
.map(|slug| slug.to_ascii_lowercase())
.collect();
let injections = step_context
.skills
.injections(
input,
&plain_name_conflicts,
&state.injected,
&state.injected_names,
)
.await;
for warning in injections.warnings {
emit_skill_warning_once(sess, turn_context, warning, &mut state.emitted_warnings).await;
}
if injections.items.is_empty() {
return false;
}
let selected_entries = injections
.items
.iter()
.map(|injection| injection.entry.clone())
.collect::<Vec<_>>();
let mcp_refreshed = maybe_prompt_and_install_mcp_dependencies(
sess,
turn_context,
cancellation_token,
&selected_entries,
Some(sess.mcp_elicitation_reviewer()),
)
.await;
let items = injections
.items
.iter()
.map(|injection| ContextualUserFragment::into(injection.instructions.clone()))
.collect::<Vec<ResponseItem>>();
let connector_ids = collect_explicit_app_ids_from_skill_items(
&items,
available_connectors,
&step_context.skills.skill_name_counts_lower(),
);
sess.merge_connector_selection(connector_ids).await;
let skill_invocations = injections
.items
.iter()
.filter_map(|injection| {
step_context
.skills
.host_skill(&injection.entry)
.map(|skill| SkillInvocation {
skill_name: skill.name.clone(),
skill_scope: skill.scope,
skill_path: skill.path_to_skills_md.to_path_buf(),
plugin_id: skill.plugin_id.clone(),
invocation_type: InvocationType::Explicit,
})
})
.collect::<Vec<_>>();
sess.services
.analytics_events_client
.track_skill_invocations(
build_track_events_context(
turn_context.model_info.slug.clone(),
sess.thread_id.to_string(),
turn_context.sub_id.clone(),
turn_context.originator.clone(),
),
skill_invocations,
);
for injection in injections.items {
turn_context.session_telemetry.counter(
"codex.skill.injected",
/*inc*/ 1,
&[("status", "ok"), ("skill", injection.entry.name.as_str())],
);
state.injected_names.insert(injection.entry.name.clone());
state.injected.insert(injection.identity);
}
sess.record_conversation_items(turn_context, &items).await;
mcp_refreshed
}
async fn emit_skill_warning_once(
sess: &Session,
turn_context: &TurnContext,
message: String,
emitted: &mut HashSet<String>,
) {
if emitted.insert(message.clone()) {
sess.send_event(turn_context, EventMsg::Warning(WarningEvent { message }))
.await;
}
}
#[instrument(level = "trace", skip_all)]
@@ -507,22 +660,14 @@ async fn build_skills_and_plugins(
turn_context: &TurnContext,
input: &[TurnInput],
cancellation_token: &CancellationToken,
) -> Option<(Vec<ResponseItem>, HashSet<String>)> {
) -> Option<(Vec<ResponseItem>, HashSet<String>, Vec<connectors::AppInfo>)> {
// Guardian input embeds the parent transcript as untrusted evidence. Do not interpret skill or
// plugin mentions from that generated prompt as requests to inject additional instructions.
if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) {
return Some((Vec::new(), HashSet::new()));
return Some((Vec::new(), HashSet::new(), Vec::new()));
}
let user_input = input
.iter()
.filter_map(|item| match item {
TurnInput::UserInput { content, .. } => Some(content.as_slice()),
TurnInput::ResponseItem(_) | TurnInput::InterAgentCommunication(_) => None,
})
.flatten()
.cloned()
.collect::<Vec<_>>();
let user_input = user_input_from_turn_input(input);
let tracking = build_track_events_context(
turn_context.model_info.slug.clone(),
sess.thread_id.to_string(),
@@ -572,61 +717,12 @@ async fn build_skills_and_plugins(
} else {
Vec::new()
};
let skills_outcome = turn_context.turn_skills.snapshot.outcome();
let connector_slug_counts = build_connector_slug_counts(&available_connectors);
let extension_injection_items =
build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token)
.await?;
let skill_name_counts_lower =
build_skill_name_counts(&skills_outcome.skills, &skills_outcome.disabled_paths).1;
let mentioned_skills = collect_explicit_skill_mentions(
&user_input,
&skills_outcome.skills,
&skills_outcome.disabled_paths,
&connector_slug_counts,
);
maybe_prompt_and_install_mcp_dependencies(
sess,
turn_context,
cancellation_token,
&mentioned_skills,
Some(sess.mcp_elicitation_reviewer()),
)
.await;
let injected_host_skill_prompts = turn_context
.extension_data
.get::<InjectedHostSkillPrompts>();
let SkillInjections {
items: skill_injections,
warnings: skill_warnings,
} = build_skill_injections(
&mentioned_skills,
Some(skills_outcome),
Some(&turn_context.session_telemetry),
&sess.services.analytics_events_client,
tracking.clone(),
)
.await;
for message in skill_warnings {
sess.send_event(turn_context, EventMsg::Warning(WarningEvent { message }))
.await;
}
let skill_items: Vec<ResponseItem> = skill_injections
.iter()
.map(|skill| ContextualUserFragment::into(crate::context::SkillInstructions::from(skill)))
.collect();
let skill_connector_ids = collect_explicit_app_ids_from_skill_items(
&skill_items,
&available_connectors,
&skill_name_counts_lower,
);
let plugin_items =
build_plugin_injections(&mentioned_plugins, &mcp_tools, &available_connectors);
let mut explicitly_enabled_connectors = collect_explicit_app_ids(&user_input);
explicitly_enabled_connectors.extend(skill_connector_ids);
let explicitly_enabled_connectors = collect_explicit_app_ids(&user_input);
let connector_names_by_id = available_connectors
.iter()
.map(|connector| (connector.id.as_str(), connector.name.as_str()))
@@ -656,19 +752,13 @@ async fn build_skills_and_plugins(
}
}
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);
let mut injection_items = plugin_items;
injection_items.extend(extension_injection_items);
Some((injection_items, explicitly_enabled_connectors))
Some((
injection_items,
explicitly_enabled_connectors,
available_connectors,
))
}
#[tracing::instrument(
+5
View File
@@ -2,6 +2,7 @@ use super::session::Session;
use super::step_context::StepContext;
use crate::context::world_state::AgentsMdState;
use crate::context::world_state::EnvironmentsState;
use crate::context::world_state::SkillsState;
use crate::context::world_state::WorldState;
impl Session {
@@ -25,6 +26,10 @@ impl Session {
let mut world_state = WorldState::default();
world_state.add_section(AgentsMdState::new(step_context.loaded_agents_md.as_deref()));
world_state.add_section(SkillsState::new(
step_context.skills.as_ref(),
turn_context.config.include_skill_instructions,
));
if turn_context.config.include_environment_context {
world_state.add_section(
EnvironmentsState::from_turn_context_with_environments(
@@ -685,11 +685,16 @@ async fn environment_tools_follow_the_step_context() {
turn.model_info.apply_patch_tool_type = Some(ApplyPatchToolType::Freeform);
let environments = turn.environments.clone();
let skills = Arc::new(codex_core_skills::SkillsSnapshot::from_host(
turn.turn_skills.snapshot.clone(),
turn.model_context_window(),
));
turn.environments.turn_environments.clear();
let step_context = Arc::new(StepContext::new(
Arc::new(turn),
environments,
Vec::new(),
skills,
/*loaded_agents_md*/ None,
));
+9 -1
View File
@@ -26,6 +26,7 @@ use codex_core::thread_store_from_config;
use codex_exec_server::CreateDirectoryOptions;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::RemoveOptions;
use codex_extension_api::ExtensionDataInit;
use codex_extension_api::ExtensionRegistry;
use codex_extension_api::LoadUserInstructionsFuture;
use codex_extension_api::UserInstructionsProvider;
@@ -287,6 +288,7 @@ pub struct TestCodexBuilder {
user_shell_override: Option<Shell>,
exec_server_url: Option<String>,
extensions: Arc<ExtensionRegistry<Config>>,
thread_extension_init: ExtensionDataInit,
user_instructions_provider: Option<Arc<dyn UserInstructionsProvider>>,
supports_openai_form_elicitation: bool,
external_time_provider: Option<Arc<dyn TimeProvider>>,
@@ -378,6 +380,11 @@ impl TestCodexBuilder {
self
}
pub fn with_thread_extension_init(mut self, thread_extension_init: ExtensionDataInit) -> Self {
self.thread_extension_init = thread_extension_init;
self
}
pub fn with_user_instructions_provider(
mut self,
provider: Arc<dyn UserInstructionsProvider>,
@@ -665,7 +672,7 @@ impl TestCodexBuilder {
metrics_service_name: None,
parent_trace: None,
environments,
thread_extension_init: Default::default(),
thread_extension_init: std::mem::take(&mut self.thread_extension_init),
supports_openai_form_elicitation: self.supports_openai_form_elicitation,
}),
)
@@ -1214,6 +1221,7 @@ pub fn test_codex() -> TestCodexBuilder {
user_shell_override: None,
exec_server_url: None,
extensions: empty_extension_registry(),
thread_extension_init: ExtensionDataInit::new(),
user_instructions_provider: None,
supports_openai_form_elicitation: false,
external_time_provider: None,
+194
View File
@@ -11,7 +11,10 @@ use codex_exec_server::FileSystemSandboxContext;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
use codex_exec_server::RemoveOptions;
use codex_extension_api::ExtensionDataInit;
use codex_features::Feature;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
@@ -468,6 +471,54 @@ async fn serve_environment_with_agents_md(
}
}
async fn serve_environment_with_skill(
listener: TcpListener,
skill_path: PathUri,
skill_contents: String,
attach: tokio::sync::oneshot::Receiver<()>,
mut shutdown: tokio::sync::oneshot::Receiver<()>,
) -> usize {
let mut websocket = accept_initialized_exec_server(listener).await;
attach.await.expect("attach signal");
send_environment_info(&mut websocket).await;
let mut reads = 0;
let skill_path_string = skill_path.to_string();
loop {
let request = tokio::select! {
request = read_exec_server_json(&mut websocket) => request,
_ = &mut shutdown => return reads,
};
let response = match request["method"].as_str() {
Some("fs/walk") => json!({
"id": request["id"],
"result": {
"entries": [{ "path": skill_path, "kind": "file" }],
"errors": [],
"truncated": false,
}
}),
Some("fs/getMetadata") => json!({
"id": request["id"],
"error": { "code": -32004, "message": "not found" }
}),
Some("fs/readFile")
if request["params"]["path"].as_str() == Some(skill_path_string.as_str()) =>
{
reads += 1;
json!({
"id": request["id"],
"result": { "dataBase64": BASE64_STANDARD.encode(&skill_contents) }
})
}
method => panic!("unexpected exec-server request: {method:?}"),
};
websocket
.send(Message::Text(response.to_string().into()))
.await
.expect("filesystem response");
}
}
fn tool_names(body: &Value) -> Vec<String> {
body["tools"]
.as_array()
@@ -730,6 +781,149 @@ async fn deferred_executor_loads_agents_md_when_environment_becomes_ready() -> R
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn deferred_executor_replaces_a_colliding_host_skill_for_the_next_step() -> Result<()> {
const HOST_BODY: &str = "HOST_SKILL_BODY_MARKER";
const EXECUTOR_BODY: &str = "EXECUTOR_SKILL_BODY_MARKER";
const SKILL_REPLACEMENT_NOTICE: &str =
"These instructions replace the previously provided instructions for this skill.";
let root_path = PathUri::from_host_native_path("/remote-capability")?;
let skill_path = root_path.join("skills/deploy/SKILL.md")?;
let skill_contents = format!(
"---\nname: deploy\ndescription: Deploy through the ready executor.\n---\n\n{EXECUTOR_BODY}\n"
);
let mut thread_extension_init = ExtensionDataInit::new();
thread_extension_init.insert(vec![SelectedCapabilityRoot {
id: "remote-capability@1".to_string(),
location: CapabilityRootLocation::Environment {
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
path: root_path,
},
}]);
let listener = TcpListener::bind("127.0.0.1:0").await?;
let server = start_mock_server().await;
let response_mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(
"wait-1",
"wait_for_environment",
&json!({ "environment_id": REMOTE_ENVIRONMENT_ID }).to_string(),
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-2", "done"),
ev_completed("resp-2"),
]),
],
)
.await;
let mut builder = test_codex()
.with_exec_server_url(format!("ws://{}", listener.local_addr()?))
.with_thread_extension_init(thread_extension_init)
.with_pre_build_hook(|home| {
let skill_dir = home.join("skills/deploy");
std::fs::create_dir_all(&skill_dir).expect("create host skill directory");
std::fs::write(
skill_dir.join("SKILL.md"),
format!(
"---\nname: deploy\ndescription: Deploy from the host.\n---\n\n{HOST_BODY}\n"
),
)
.expect("write host skill");
})
.with_config(|config| {
config.project_doc_max_bytes = 0;
assert!(config.features.enable(Feature::DeferredExecutor).is_ok());
});
let (attach_tx, attach_rx) = tokio::sync::oneshot::channel();
let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel();
let exec_server = tokio::spawn(serve_environment_with_skill(
listener,
skill_path,
skill_contents,
attach_rx,
shutdown_rx,
));
let test = timeout(Duration::from_secs(5), builder.build(&server))
.await
.context("thread startup should not wait for the remote environment")??;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "Use $deploy after the environment is ready".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: Default::default(),
})
.await?;
wait_for_response_request_count(&response_mock, /*expected_count*/ 1).await;
attach_tx.send(()).expect("attach environment");
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
shutdown_tx.send(()).expect("stop exec server");
assert_eq!(exec_server.await?, 2);
let requests = response_mock.requests();
assert_eq!(requests.len(), 2);
assert_eq!(skill_body_occurrences(&requests[0], HOST_BODY), 1);
assert_eq!(skill_body_occurrences(&requests[0], EXECUTOR_BODY), 0);
assert_eq!(skill_body_occurrences(&requests[1], HOST_BODY), 1);
assert_eq!(skill_body_occurrences(&requests[1], EXECUTOR_BODY), 1);
assert_eq!(environment_skill_occurrences(&requests[0], "deploy"), 0);
assert_eq!(environment_skill_occurrences(&requests[1], "deploy"), 1);
let first_user_context = requests[0].message_input_texts("user");
let host_skill = first_user_context
.iter()
.find(|text| text.starts_with("<skill>") && text.contains(HOST_BODY))
.expect("first request should contain host skill instructions");
assert!(!host_skill.contains(SKILL_REPLACEMENT_NOTICE));
let second_user_context = requests[1].message_input_texts("user");
let executor_skill = second_user_context
.iter()
.find(|text| text.starts_with("<skill>") && text.contains(EXECUTOR_BODY))
.expect("second request should contain executor skill instructions");
assert!(executor_skill.contains(SKILL_REPLACEMENT_NOTICE));
let second_developer_context = requests[1].message_input_texts("developer");
let replacement_catalog = second_developer_context
.iter()
.find(|text| text.contains("This complete skills list replaces"))
.expect("second request should contain the replacement skill catalog");
assert!(!replacement_catalog.contains("### How to use skills"));
Ok(())
}
fn skill_body_occurrences(request: &ResponsesRequest, contents: &str) -> usize {
request
.message_input_texts("user")
.iter()
.filter(|text| text.starts_with("<skill>") && text.contains(contents))
.count()
}
fn environment_skill_occurrences(request: &ResponsesRequest, name: &str) -> usize {
request
.message_input_texts("developer")
.iter()
.filter(|text| {
text.starts_with("<skills_instructions>")
&& text.contains(&format!("- {name}:"))
&& text.contains("(environment resource:")
})
.count()
}
fn agents_md_occurrences(request: &ResponsesRequest, contents: &str) -> usize {
request
.message_input_texts("user")
+20 -266
View File
@@ -1,49 +1,24 @@
use std::collections::HashSet;
use std::sync::Arc;
use codex_core_skills::HostSkillsSnapshot;
use codex_core_skills::SkillInstructions;
use codex_core_skills::collect_runtime_skill_mentions;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_extension_api::ConfigContributor;
use codex_extension_api::ContextContributor;
use codex_extension_api::ContextualUserFragment;
use codex_extension_api::ExtensionData;
use codex_extension_api::ExtensionEventSink;
use codex_extension_api::ExtensionFuture;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_extension_api::PromptFragment;
use codex_extension_api::ThreadLifecycleContributor;
use codex_extension_api::ThreadStartInput;
use codex_extension_api::ToolCall;
use codex_extension_api::ToolContributor;
use codex_extension_api::ToolExecutor;
use codex_extension_api::TurnInputContext;
use codex_extension_api::TurnInputContributor;
use codex_mcp::McpResourceClient;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::WarningEvent;
use crate::SkillsExtensionConfig;
use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillReadResult;
use crate::catalog::SkillSourceKind;
use crate::provider::HostSkillProvider;
use crate::provider::SkillListQuery;
use crate::provider::SkillReadRequest;
use crate::render::available_skills_fragment;
use crate::sources::SkillProviders;
use crate::state::SkillsThreadState;
use crate::state::SkillsTurnState;
use crate::tools::skill_tools;
struct SkillsExtension<C> {
providers: SkillProviders,
event_sink: Arc<dyn ExtensionEventSink>,
config_from_host: Arc<dyn Fn(&C) -> SkillsExtensionConfig + Send + Sync>,
}
@@ -53,20 +28,23 @@ where
{
fn on_thread_start<'a>(&'a self, input: ThreadStartInput<'a, C>) -> ExtensionFuture<'a, ()> {
Box::pin(async move {
let selected_roots = input
.thread_store
.get::<Vec<SelectedCapabilityRoot>>()
.map(|selected_roots| selected_roots.as_ref().clone())
.unwrap_or_default();
let orchestrator_skills_available = !input
.environments
.iter()
.any(|environment| environment.environment_id == LOCAL_ENVIRONMENT_ID);
input.thread_store.insert(SkillsThreadState::new(
(self.config_from_host)(input.config),
selected_roots,
orchestrator_skills_available,
));
let thread_state = input.thread_store.get_or_init(|| {
SkillsThreadState::new(
(self.config_from_host)(input.config),
orchestrator_skills_available,
)
});
thread_state.set_config((self.config_from_host)(input.config));
input
.thread_store
.insert(self.providers.orchestrator_sources_for_thread(
thread_state,
input.session_store.get::<McpResourceClient>(),
));
})
}
}
@@ -77,7 +55,7 @@ where
{
fn on_config_changed(
&self,
_session_store: &ExtensionData,
session_store: &ExtensionData,
thread_store: &ExtensionData,
_previous_config: &C,
new_config: &C,
@@ -87,60 +65,16 @@ where
state.set_config(next_config);
} else {
let orchestrator_skills_available = true;
thread_store.insert(SkillsThreadState::new(
next_config,
Vec::new(),
orchestrator_skills_available,
let thread_state = thread_store
.get_or_init(|| SkillsThreadState::new(next_config, orchestrator_skills_available));
thread_store.insert(self.providers.orchestrator_sources_for_thread(
thread_state,
session_store.get::<McpResourceClient>(),
));
}
}
}
impl<C> ContextContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn contribute_thread_context<'a>(
&'a self,
session_store: &'a ExtensionData,
thread_store: &'a ExtensionData,
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Vec<PromptFragment>> + Send + 'a>> {
Box::pin(async move {
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
return Vec::new();
};
let config = thread_state.config();
if !config.include_instructions {
return Vec::new();
}
let catalog = self
.list_skills(
SkillListQuery {
turn_id: thread_store.level_id().to_string(),
executor_roots: thread_state.selected_roots().to_vec(),
host_snapshot: None,
include_host_skills: false,
include_bundled_skills: config.bundled_skills_enabled,
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
mcp_resources: session_store.get::<McpResourceClient>(),
},
&thread_state,
)
.await;
for warning in &catalog.warnings {
self.emit_warning(thread_store.level_id(), warning.clone());
}
let Some((fragment, warning)) = available_skills_fragment(&catalog) else {
return Vec::new();
};
if let Some(warning) = warning {
self.emit_warning(thread_store.level_id(), warning);
}
vec![PromptFragment::developer_capability(fragment.render())]
})
}
}
impl<C> ToolContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
@@ -167,190 +101,13 @@ where
}
}
impl<C> TurnInputContributor for SkillsExtension<C>
where
C: Send + Sync + 'static,
{
fn contribute<'a>(
&'a self,
input: TurnInputContext,
session_store: &'a ExtensionData,
thread_store: &'a ExtensionData,
turn_store: &'a ExtensionData,
) -> ExtensionFuture<'a, Vec<Box<dyn ContextualUserFragment + Send>>> {
Box::pin(async move {
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
return Vec::new();
};
let config = thread_state.config();
let host_snapshot = turn_store.get::<HostSkillsSnapshot>();
let query = SkillListQuery {
turn_id: input.turn_id.clone(),
executor_roots: thread_state.selected_roots().to_vec(),
host_snapshot: host_snapshot.clone(),
include_host_skills: true,
include_bundled_skills: config.bundled_skills_enabled,
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
mcp_resources: session_store.get::<McpResourceClient>(),
};
let catalog = self.list_skills(query, &thread_state).await;
for warning in &catalog.warnings {
self.emit_warning(&input.turn_id, warning.clone());
}
let plain_name_conflicts = HashSet::new();
let selected_entries =
collect_runtime_skill_mentions(&input.user_input, &catalog, &plain_name_conflicts);
let mut warnings = catalog.warnings.clone();
let mut fragments: Vec<Box<dyn ContextualUserFragment + Send>> = 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, warning)) = available_skills_fragment(&turn_catalog) {
if let Some(warning) = warning {
self.emit_warning(&input.turn_id, warning.clone());
warnings.push(warning);
}
fragments.push(Box::new(fragment));
}
}
let mut main_prompts_injected = false;
let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default();
for entry in &selected_entries {
match self
.read_main_prompt(entry, host_snapshot.clone(), session_store, &thread_state)
.await
{
Ok(read_result) => {
let (fragment, truncated) =
SkillInstructions::from_runtime(entry, &read_result.contents);
if truncated {
let warning = format!(
"Skill `{}` exceeded the main prompt context limit and was truncated.",
entry.name
);
self.emit_warning(&input.turn_id, warning.clone());
warnings.push(warning);
}
fragments.push(Box::new(fragment));
main_prompts_injected = true;
if entry.authority.kind == SkillSourceKind::Host {
injected_host_skill_prompts.insert_path(entry.main_prompt.as_str());
}
}
Err(message) => {
let warning = format!("Failed to load skill `{}`: {message}", entry.name);
self.emit_warning(&input.turn_id, warning.clone());
warnings.push(warning);
}
}
}
if let Some(host_snapshot) = &host_snapshot {
for entry in selected_entries
.iter()
.filter(|entry| entry.authority.kind != SkillSourceKind::Host)
{
for host_skill in host_snapshot
.outcome()
.skills
.iter()
.filter(|host_skill| host_skill.name == entry.name)
{
injected_host_skill_prompts
.insert_path(host_skill.path_to_skills_md.to_string_lossy());
}
}
}
turn_store.insert(SkillsTurnState {
catalog,
selected_entries,
warnings,
main_prompts_injected,
});
if !injected_host_skill_prompts.is_empty() {
turn_store.insert(injected_host_skill_prompts);
}
fragments
})
}
}
impl<C> SkillsExtension<C> {
#[tracing::instrument(level = "trace", skip_all)]
async fn list_skills(
&self,
mut query: SkillListQuery,
thread_state: &SkillsThreadState,
) -> SkillCatalog {
let include_orchestrator_skills = query.include_orchestrator_skills;
let orchestrator_query = query.clone();
let mcp_resources = orchestrator_query.mcp_resources.clone();
query.include_orchestrator_skills = false;
let mut catalog = self.providers.list_for_turn(query).await;
if include_orchestrator_skills {
let orchestrator_catalog = thread_state
.orchestrator_catalog_snapshot(
mcp_resources.as_deref(),
self.providers
.list_orchestrator_for_turn(orchestrator_query),
)
.await;
catalog.extend(orchestrator_catalog);
}
catalog
}
#[tracing::instrument(level = "trace", skip_all, fields(skill = %entry.name))]
async fn read_main_prompt(
&self,
entry: &SkillCatalogEntry,
host_snapshot: Option<Arc<HostSkillsSnapshot>>,
session_store: &ExtensionData,
thread_state: &SkillsThreadState,
) -> Result<SkillReadResult, String> {
thread_state
.read_skill(
&self.providers,
SkillReadRequest {
authority: entry.authority.clone(),
package: entry.id.clone(),
resource: entry.main_prompt.clone(),
host_snapshot,
mcp_resources: session_store.get::<McpResourceClient>(),
},
)
.await
.map_err(|err| err.message)
}
fn emit_warning(&self, turn_id: &str, message: String) {
self.event_sink.emit(Event {
id: turn_id.to_string(),
msg: EventMsg::Warning(WarningEvent { message }),
});
}
}
pub fn install<C>(
registry: &mut ExtensionRegistryBuilder<C>,
config_from_host: impl Fn(&C) -> SkillsExtensionConfig + Send + Sync + 'static,
) where
C: Send + Sync + 'static,
{
install_with_providers(
registry,
SkillProviders::new().with_host_provider(Arc::new(HostSkillProvider::new())),
config_from_host,
);
install_with_providers(registry, SkillProviders::new(), config_from_host);
}
pub fn install_with_providers<C>(
@@ -362,12 +119,9 @@ pub fn install_with_providers<C>(
{
let extension = Arc::new(SkillsExtension {
providers,
event_sink: registry.event_sink(),
config_from_host: Arc::new(config_from_host),
});
registry.thread_lifecycle_contributor(extension.clone());
registry.config_contributor(extension.clone());
registry.prompt_contributor(extension.clone());
registry.turn_input_contributor(extension.clone());
registry.tool_contributor(extension);
}
-38
View File
@@ -1,38 +0,0 @@
use codex_core_skills::AvailableSkills;
use codex_core_skills::render_available_skills_body;
use codex_extension_api::ContextualUserFragment;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) struct AvailableSkillsInstructions {
skill_root_lines: Vec<String>,
skill_lines: Vec<String>,
}
impl From<AvailableSkills> for AvailableSkillsInstructions {
fn from(available: AvailableSkills) -> Self {
Self {
skill_root_lines: available.skill_root_lines,
skill_lines: available.skill_lines,
}
}
}
impl ContextualUserFragment for AvailableSkillsInstructions {
fn role(&self) -> &'static str {
"developer"
}
fn markers(&self) -> (&'static str, &'static str) {
Self::type_markers()
}
fn type_markers() -> (&'static str, &'static str) {
(SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG)
}
fn body(&self) -> String {
render_available_skills_body(&self.skill_root_lines, &self.skill_lines)
}
}
-1
View File
@@ -1,7 +1,6 @@
pub mod catalog;
mod config;
mod extension;
mod fragments;
pub mod provider;
mod render;
mod sources;
-24
View File
@@ -1,34 +1,10 @@
use std::borrow::Cow;
use codex_core_skills::SkillRenderSideEffects;
use codex_core_skills::build_available_skills_from_catalog;
use codex_core_skills::default_skill_metadata_budget;
use codex_utils_string::take_bytes_at_char_boundary;
use crate::catalog::SkillCatalog;
use crate::fragments::AvailableSkillsInstructions;
const MAX_CATALOG_SKILL_DESCRIPTION_CHARS: usize = 1_024;
const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "...";
#[tracing::instrument(
level = "trace",
skip_all,
fields(catalog_entry_count = catalog.entries.len())
)]
pub(crate) fn available_skills_fragment(
catalog: &SkillCatalog,
) -> Option<(AvailableSkillsInstructions, Option<String>)> {
let available = build_available_skills_from_catalog(
catalog,
/*host_outcome*/ None,
default_skill_metadata_budget(/*context_window*/ None),
SkillRenderSideEffects::None,
)?;
let warning = available.warning_message.clone();
Some((available.into(), warning))
}
pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, str> {
if description
.char_indices()
+86 -4
View File
@@ -18,6 +18,7 @@ use crate::provider::SkillListQuery;
use crate::provider::SkillProvider;
use crate::provider::SkillReadRequest;
use crate::provider::SkillSearchRequest;
use crate::state::SkillsThreadState;
#[derive(Clone)]
pub struct SkillProviderSource {
@@ -128,10 +129,6 @@ impl SkillProviders {
.any(|source| source.kind == SkillSourceKind::Orchestrator)
}
pub(crate) async fn list_for_turn(&self, query: SkillListQuery) -> SkillCatalog {
self.sources_for_turn(query).list().await
}
pub(crate) async fn list_orchestrator_for_turn(
&self,
query: SkillListQuery,
@@ -141,6 +138,50 @@ impl SkillProviders {
.await
}
pub(crate) fn orchestrator_sources_for_thread(
&self,
thread_state: Arc<SkillsThreadState>,
mcp_resources: Option<Arc<McpResourceClient>>,
) -> SkillSources {
self.sources
.iter()
.filter(|source| source.kind == SkillSourceKind::Orchestrator)
.fold(SkillSources::new(), |sources, provider| {
let provider = provider.clone();
let thread_state = Arc::clone(&thread_state);
let mcp_resources = mcp_resources.clone();
sources.with_source_factory(
"orchestrator",
Arc::new(move || {
let mcp_resources = mcp_resources
.as_ref()
.map(|client| Arc::new(client.snapshot()));
let identity = mcp_resources
.as_ref()
.map(|client| {
SkillSourceIdentity::from_owner(client.manager_snapshot())
})
.unwrap_or_else(|| provider.identity.clone());
let query = SkillListQuery {
turn_id: String::new(),
executor_roots: Vec::new(),
host_snapshot: None,
include_host_skills: false,
include_bundled_skills: false,
include_orchestrator_skills: true,
mcp_resources: mcp_resources.clone(),
};
Arc::new(CachedOrchestratorSource {
inner: provider.bind(query),
identity,
thread_state: Arc::clone(&thread_state),
mcp_resources,
}) as Arc<dyn SkillSource>
}),
)
})
}
fn sources_for_turn(&self, query: SkillListQuery) -> SkillSources {
self.sources
.iter()
@@ -199,6 +240,47 @@ impl SkillProviders {
}
}
struct CachedOrchestratorSource {
inner: Arc<dyn SkillSource>,
identity: SkillSourceIdentity,
thread_state: Arc<SkillsThreadState>,
mcp_resources: Option<Arc<McpResourceClient>>,
}
impl SkillSource for CachedOrchestratorSource {
fn kind(&self) -> SkillSourceKind {
SkillSourceKind::Orchestrator
}
fn identity(&self) -> SkillSourceIdentity {
self.identity.clone()
}
fn list(&self) -> SkillSourceFuture<'_, SkillCatalog> {
Box::pin(async move {
if !self.thread_state.orchestrator_skills_enabled() {
return Ok(SkillCatalog::default());
}
Ok(self
.thread_state
.orchestrator_catalog_snapshot(self.mcp_resources.as_deref(), self.inner.list())
.await)
})
}
fn read(&self, request: RuntimeSkillReadRequest) -> SkillSourceFuture<'_, SkillReadResult> {
Box::pin(async move {
self.thread_state
.read_orchestrator_source(
self.inner.as_ref(),
request,
self.mcp_resources.as_deref(),
)
.await
})
}
}
struct BoundSkillProvider {
kind: SkillSourceKind,
provider: Arc<dyn SkillProvider>,
+43 -33
View File
@@ -3,15 +3,15 @@ use std::future::Future;
use std::sync::Arc;
use std::sync::Mutex;
use codex_core_skills::runtime::SkillReadRequest as RuntimeSkillReadRequest;
use codex_core_skills::runtime::SkillSource;
use codex_mcp::McpResourceClient;
use codex_mcp::McpResourceClientCacheKey;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use tokio::sync::OnceCell;
use crate::SkillsExtensionConfig;
use crate::catalog::SkillAuthority;
use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillPackageId;
use crate::catalog::SkillProviderError;
use crate::catalog::SkillProviderResult;
@@ -26,20 +26,14 @@ const MAX_CACHED_ORCHESTRATOR_CONTENT_BYTES: usize = 8 * 1024 * 1024;
pub(crate) struct SkillsThreadState {
config: Mutex<SkillsExtensionConfig>,
selected_roots: Vec<SelectedCapabilityRoot>,
orchestrator_skills_available: bool,
orchestrator_cache: Mutex<Option<Arc<OrchestratorGenerationCache>>>,
}
impl SkillsThreadState {
pub(crate) fn new(
config: SkillsExtensionConfig,
selected_roots: Vec<SelectedCapabilityRoot>,
orchestrator_skills_available: bool,
) -> Self {
pub(crate) fn new(config: SkillsExtensionConfig, orchestrator_skills_available: bool) -> Self {
Self {
config: Mutex::new(config),
selected_roots,
orchestrator_skills_available,
orchestrator_cache: Mutex::new(None),
}
@@ -59,10 +53,6 @@ impl SkillsThreadState {
.unwrap_or_else(std::sync::PoisonError::into_inner) = config;
}
pub(crate) fn selected_roots(&self) -> &[SelectedCapabilityRoot] {
&self.selected_roots
}
pub(crate) fn orchestrator_skills_enabled(&self) -> bool {
self.orchestrator_skills_available && self.config().orchestrator_skills_enabled
}
@@ -72,16 +62,18 @@ impl SkillsThreadState {
mcp_resources: Option<&McpResourceClient>,
initialize: impl Future<Output = Result<SkillCatalog, SkillProviderError>> + Send,
) -> SkillCatalog {
self.orchestrator_cache(mcp_resources)
.catalog
.get_or_init(|| async {
initialize.await.unwrap_or_else(|err| SkillCatalog {
warnings: vec![err.message],
..Default::default()
})
})
.await
.clone()
let cache = self.orchestrator_cache(mcp_resources);
if let Some(catalog) = cache.catalog.get() {
return catalog.clone();
}
let catalog = initialize.await.unwrap_or_else(|err| SkillCatalog {
warnings: vec![err.message],
..Default::default()
});
if !catalog.entries.is_empty() && catalog.warnings.is_empty() {
let _ = cache.catalog.set(catalog.clone());
}
catalog
}
pub(crate) async fn read_skill(
@@ -93,8 +85,34 @@ impl SkillsThreadState {
return providers.read(request).await;
}
let cache = self.orchestrator_cache(request.mcp_resources.as_deref());
let cache_key = SkillReadCacheKey::from(&request);
let mcp_resources = request.mcp_resources.clone();
self.read_orchestrator_cached(cache_key, mcp_resources.as_deref(), providers.read(request))
.await
}
pub(crate) async fn read_orchestrator_source(
&self,
source: &dyn SkillSource,
request: RuntimeSkillReadRequest,
mcp_resources: Option<&McpResourceClient>,
) -> SkillProviderResult<SkillReadResult> {
let cache_key = SkillReadCacheKey {
authority: request.authority.clone(),
package: request.package.clone(),
resource: request.resource.clone(),
};
self.read_orchestrator_cached(cache_key, mcp_resources, source.read(request))
.await
}
async fn read_orchestrator_cached(
&self,
cache_key: SkillReadCacheKey,
mcp_resources: Option<&McpResourceClient>,
read: impl Future<Output = SkillProviderResult<SkillReadResult>> + Send,
) -> SkillProviderResult<SkillReadResult> {
let cache = self.orchestrator_cache(mcp_resources);
if let Some(result) = cache
.resources
.lock()
@@ -104,7 +122,7 @@ impl SkillsThreadState {
return Ok(result);
}
let result = providers.read(request).await?;
let result = read.await?;
if result.resource != cache_key.resource {
return Ok(result);
}
@@ -196,11 +214,3 @@ impl OrchestratorResourceCache {
result
}
}
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) struct SkillsTurnState {
pub(crate) catalog: SkillCatalog,
pub(crate) selected_entries: Vec<SkillCatalogEntry>,
pub(crate) warnings: Vec<String>,
pub(crate) main_prompts_injected: bool,
}
+49 -592
View File
@@ -1,34 +1,17 @@
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
use codex_core_skills::HostSkillsSnapshot;
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
use codex_core_skills::SkillLoadOutcome;
use codex_core_skills::SkillMetadata;
use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_core_skills::runtime::SkillSources;
use codex_extension_api::ConversationHistory;
use codex_extension_api::ExtensionData;
use codex_extension_api::ExtensionEventSink;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_extension_api::NoopTurnItemEmitter;
use codex_extension_api::ThreadStartInput;
use codex_extension_api::ToolCall;
use codex_extension_api::ToolPayload;
use codex_extension_api::TurnInputContext;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillScope;
use codex_protocol::protocol::TruncationPolicy;
use codex_protocol::user_input::UserInput;
use codex_skills_extension::SkillProviders;
use codex_skills_extension::SkillsExtensionConfig;
use codex_skills_extension::catalog::SkillAuthority;
@@ -40,314 +23,30 @@ use codex_skills_extension::catalog::SkillReadResult;
use codex_skills_extension::catalog::SkillResourceId;
use codex_skills_extension::catalog::SkillSearchResult;
use codex_skills_extension::catalog::SkillSourceKind;
use codex_skills_extension::install;
use codex_skills_extension::install_with_providers;
use codex_skills_extension::provider::SkillListQuery;
use codex_skills_extension::provider::SkillProvider;
use codex_skills_extension::provider::SkillProviderFuture;
use codex_skills_extension::provider::SkillReadRequest;
use codex_skills_extension::provider::SkillSearchRequest;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
type TestResult = Result<(), Box<dyn std::error::Error>>;
static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0);
const DEMO_SKILL_CONTENTS: &str =
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n";
#[tokio::test]
async fn installed_extension_uses_host_service_snapshot() -> TestResult {
let codex_home = test_codex_home();
let skill_path = codex_home.join("skills").join("demo").join("SKILL.md");
std::fs::create_dir_all(
skill_path
.parent()
.ok_or("skill path should have a parent")?,
)?;
std::fs::write(&skill_path, DEMO_SKILL_CONTENTS)?;
let config = default_config();
let mut builder = ExtensionRegistryBuilder::new();
install(&mut builder, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
let session_source = SessionSource::Cli;
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let skill_path = AbsolutePathBuf::try_from(skill_path)?;
let skill_path_string = skill_path.to_string_lossy().into_owned();
let mut outcome = SkillLoadOutcome::default();
outcome.skills.push(SkillMetadata {
name: "demo".to_string(),
description: "Demo skill.".to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: skill_path,
scope: SkillScope::User,
plugin_id: None,
});
let loaded_skills = Arc::new(outcome);
let skill_prompt_path = skill_path_string.replace('\\', "/");
let turn_store = ExtensionData::new("turn-1");
turn_store.insert(HostSkillsSnapshot::new(Arc::clone(&loaded_skills)));
let fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: "turn-1".to_string(),
user_input: vec![UserInput::Text {
text: "$demo".to_string(),
text_elements: Vec::new(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&turn_store,
)
.await;
let expected_catalog = format!(
"{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n### How to use skills\n{SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"
);
let expected_skill = format!(
"<skill>\n<name>demo</name>\n<path>{skill_prompt_path}</path>\n{DEMO_SKILL_CONTENTS}\n</skill>"
);
assert_eq!(
vec![("developer", expected_catalog), ("user", expected_skill),],
fragments
.iter()
.map(|fragment| (fragment.role(), fragment.render()))
.collect::<Vec<_>>()
);
let injected_host_skill_prompts = turn_store
.get::<InjectedHostSkillPrompts>()
.ok_or("host skill prompt marker should be set")?;
assert!(injected_host_skill_prompts.contains_path(&skill_path_string));
std::fs::remove_dir_all(codex_home)?;
Ok(())
}
#[tokio::test]
async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_input() -> TestResult
{
let read_requests = Arc::new(Mutex::new(Vec::new()));
let executor_provider = Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![test_entry(
SkillSourceKind::Executor,
"env-1",
"executor/lint-fix",
"lint-fix/SKILL.md",
)],
warnings: Vec::new(),
},
read_requests: Arc::clone(&read_requests),
list_calls: None,
fail_first_list: false,
});
let providers = SkillProviders::new().with_executor_provider(executor_provider);
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
thread_store.insert(vec![SelectedCapabilityRoot {
id: "lint-fix".to_string(),
location: CapabilityRootLocation::Environment {
environment_id: "env-1".to_string(),
path: PathUri::parse("file:///skills/lint-fix").expect("skill root URI"),
},
}]);
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let prompt_fragments = registry.context_contributors()[0]
.contribute_thread_context(&session_store, &thread_store)
.await;
assert_eq!(1, prompt_fragments.len());
assert!(
prompt_fragments[0]
.text()
.starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG)
);
assert!(prompt_fragments[0].text().contains("lint-fix"));
assert!(
prompt_fragments[0]
.text()
.contains("(environment resource: skill://executor/lint-fix/SKILL.md)")
);
let turn_store = ExtensionData::new("turn-1");
let fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: "turn-1".to_string(),
user_input: vec![UserInput::Text {
text: "$lint-fix please".to_string(),
text_elements: Vec::new(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&turn_store,
)
.await;
assert_eq!(1, fragments.len());
assert_eq!("user", fragments[0].role());
assert!(fragments[0].render().contains("<name>lint-fix</name>"));
assert!(fragments[0].render().contains("# Lint Fix"));
assert_eq!(
vec![(
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
SkillPackageId("executor/lint-fix".to_string()),
SkillResourceId::new("lint-fix/SKILL.md"),
)],
read_request_keys(&read_requests)
);
let rebuilt_prompt_fragments = registry.context_contributors()[0]
.contribute_thread_context(&session_store, &thread_store)
.await;
assert_eq!(1, rebuilt_prompt_fragments.len());
assert!(rebuilt_prompt_fragments[0].text().contains("lint-fix"));
let next_turn_store = ExtensionData::new("turn-2");
let next_fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: "turn-2".to_string(),
user_input: vec![UserInput::Text {
text: "no skill this time".to_string(),
text_elements: Vec::new(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&next_turn_store,
)
.await;
assert!(next_fragments.is_empty());
Ok(())
}
#[tokio::test]
async fn default_context_truncates_catalog_descriptions() -> TestResult {
let description = "x".repeat(1_025);
let mut entry = test_entry(
SkillSourceKind::Orchestrator,
"codex_apps",
"orchestrator/long-description",
"skill://orchestrator/long-description/SKILL.md",
);
entry.description = description.clone();
let providers =
SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![entry],
warnings: Vec::new(),
},
read_requests: Arc::new(Mutex::new(Vec::new())),
list_calls: None,
fail_first_list: false,
}));
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let fragments = registry.context_contributors()[0]
.contribute_thread_context(&session_store, &thread_store)
.await;
assert_eq!(1, fragments.len());
let rendered = fragments[0].text();
assert!(rendered.contains(&("x".repeat(1_021) + "...")));
assert!(!rendered.contains(&"x".repeat(1_024)));
assert!(!rendered.contains(&description));
Ok(())
}
#[tokio::test]
async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult {
let description = "x".repeat(1_025);
let mut entry = test_entry(
SkillSourceKind::Orchestrator,
"codex_apps",
"orchestrator/long-description",
"skill://orchestrator/long-description/SKILL.md",
);
let mut entry = test_entry("orchestrator/long-description");
entry.description = description.clone();
let providers =
SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![entry],
warnings: Vec::new(),
},
read_requests: Arc::new(Mutex::new(Vec::new())),
list_calls: None,
fail_first_list: false,
}));
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let (registry, session_store, thread_store) = start_extension(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![entry],
warnings: Vec::new(),
},
list_calls: None,
fail_first_list: false,
})
.await;
let tools = registry.tool_contributors()[0].tools(&session_store, &thread_store);
let list_tool = tools
@@ -379,207 +78,52 @@ async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResu
assert_eq!(rendered_description, "x".repeat(1_021) + "...");
assert_ne!(rendered_description, description);
Ok(())
}
#[tokio::test]
async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult {
async fn transient_orchestrator_failure_is_not_cached() -> TestResult {
let list_calls = Arc::new(AtomicUsize::new(0));
let providers =
SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![test_entry(
SkillSourceKind::Orchestrator,
"codex_apps",
"orchestrator/first",
"skill://orchestrator/first/SKILL.md",
)],
warnings: Vec::new(),
},
read_requests: Arc::new(Mutex::new(Vec::new())),
list_calls: Some(Arc::clone(&list_calls)),
fail_first_list: true,
}));
let (event_tx, event_rx) = std::sync::mpsc::channel();
let mut builder =
ExtensionRegistryBuilder::with_event_sink(Arc::new(ChannelEventSink(event_tx)));
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let (_registry, _session_store, thread_store) = start_extension(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![test_entry("orchestrator/first")],
warnings: Vec::new(),
},
list_calls: Some(Arc::clone(&list_calls)),
fail_first_list: true,
})
.await;
let sources = thread_store
.get::<SkillSources>()
.ok_or("orchestrator skill source should be registered")?;
let initial_fragments = registry.context_contributors()[0]
.contribute_thread_context(&session_store, &thread_store)
.await;
assert!(initial_fragments.is_empty());
let EventMsg::Warning(warning) = event_rx.try_recv()?.msg else {
panic!("expected warning event");
};
assert_eq!(
warning.message,
"orchestrator skills unavailable: temporary orchestrator failure"
);
for turn_id in ["turn-1", "turn-2"] {
let fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: turn_id.to_string(),
user_input: vec![UserInput::Text {
text: "$first".to_string(),
text_elements: Vec::new(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&ExtensionData::new(turn_id),
)
.await;
assert!(fragments.is_empty());
}
assert_eq!(1, list_calls.load(Ordering::Relaxed));
let first = sources.list().await;
let second = sources.list().await;
assert!(first.entries.is_empty());
assert_eq!(first.warnings.len(), 1);
assert_eq!(second.entries, vec![test_entry("orchestrator/first")]);
assert_eq!(list_calls.load(Ordering::Relaxed), 2);
Ok(())
}
#[tokio::test]
async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> TestResult {
let read_requests = Arc::new(Mutex::new(Vec::new()));
let root_a_locator = "skill://root-a/shared/lint-fix/SKILL.md";
let root_b_locator = "skill://root-b/shared/lint-fix/SKILL.md";
let executor_provider = Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: [("root-a", root_a_locator), ("root-b", root_b_locator)]
.into_iter()
.map(|(root_id, locator)| {
SkillCatalogEntry::new(
SkillPackageId(locator.to_string()),
SkillAuthority::new(SkillSourceKind::Executor, root_id),
"lint-fix",
"Fix lint errors.",
SkillResourceId::new(locator),
)
.with_display_path(locator)
})
.collect(),
warnings: Vec::new(),
},
read_requests: Arc::clone(&read_requests),
list_calls: None,
fail_first_list: false,
});
let providers = SkillProviders::new().with_executor_provider(executor_provider);
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
thread_store.insert(
[("root-a", "/skills/root-a"), ("root-b", "/skills/root-b")]
.into_iter()
.map(|(id, path)| SelectedCapabilityRoot {
id: id.to_string(),
location: CapabilityRootLocation::Environment {
environment_id: "env-1".to_string(),
path: PathUri::parse(&format!("file://{path}")).expect("skill root URI"),
},
})
.collect::<Vec<_>>(),
);
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
session_store: &session_store,
thread_store: &thread_store,
})
.await;
let fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: "turn-1".to_string(),
user_input: vec![UserInput::Mention {
name: "lint-fix".to_string(),
path: root_b_locator.to_string(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&ExtensionData::new("turn-1"),
)
.await;
assert_eq!(1, fragments.len());
assert!(fragments[0].render().contains(root_b_locator));
assert_eq!(
vec![(
SkillAuthority::new(SkillSourceKind::Executor, "root-b"),
SkillPackageId(root_b_locator.to_string()),
SkillResourceId::new(root_b_locator),
)],
read_request_keys(&read_requests)
);
Ok(())
}
#[tokio::test]
async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
let read_requests = Arc::new(Mutex::new(Vec::new()));
let provider = Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries: vec![
test_entry(
SkillSourceKind::Host,
"host",
"host/visible-skill",
"visible-skill/SKILL.md",
),
test_entry(
SkillSourceKind::Host,
"host",
"host/hidden-skill",
"hidden-skill/SKILL.md",
)
.hidden_from_prompt(),
],
warnings: Vec::new(),
},
read_requests: Arc::clone(&read_requests),
list_calls: None,
fail_first_list: false,
});
let providers = SkillProviders::new().with_host_provider(provider);
async fn start_extension(
provider: StaticSkillProvider,
) -> (
codex_extension_api::ExtensionRegistry<TestConfig>,
ExtensionData,
ExtensionData,
) {
let providers = SkillProviders::new().with_orchestrator_provider(Arc::new(provider));
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
let session_store = ExtensionData::new("session");
let thread_store = ExtensionData::new("thread");
let session_source = SessionSource::Cli;
let config = default_config();
registry.thread_lifecycle_contributors()[0]
.on_thread_start(ThreadStartInput {
config: &config,
config: &TestConfig,
session_source: &session_source,
persistent_thread_state_available: true,
environments: &[],
@@ -587,55 +131,16 @@ async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
thread_store: &thread_store,
})
.await;
let fragments = registry.turn_input_contributors()[0]
.contribute(
TurnInputContext {
turn_id: "turn-1".to_string(),
user_input: vec![UserInput::Text {
text: "$hidden-skill".to_string(),
text_elements: Vec::new(),
}],
environments: Vec::new(),
},
&session_store,
&thread_store,
&ExtensionData::new("turn-1"),
)
.await;
assert_eq!(2, fragments.len());
assert!(fragments[0].render().contains("visible-skill"));
assert!(!fragments[0].render().contains("hidden-skill"));
assert!(fragments[1].render().contains("<name>hidden-skill</name>"));
assert_eq!(
vec![(
SkillAuthority::new(SkillSourceKind::Host, "host"),
SkillPackageId("host/hidden-skill".to_string()),
SkillResourceId::new("hidden-skill/SKILL.md"),
)],
read_request_keys(&read_requests)
);
Ok(())
(registry, session_store, thread_store)
}
#[derive(Clone)]
struct StaticSkillProvider {
catalog: SkillCatalog,
read_requests: Arc<Mutex<Vec<SkillReadRequest>>>,
list_calls: Option<Arc<AtomicUsize>>,
fail_first_list: bool,
}
struct ChannelEventSink(std::sync::mpsc::Sender<Event>);
impl ExtensionEventSink for ChannelEventSink {
fn emit(&self, event: Event) {
let _ = self.0.send(event);
}
}
impl SkillProvider for StaticSkillProvider {
fn list(&self, _query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> {
let list_call = self
@@ -654,15 +159,10 @@ impl SkillProvider for StaticSkillProvider {
}
fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> {
let read_requests = Arc::clone(&self.read_requests);
Box::pin(async move {
read_requests
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.push(request.clone());
Ok(SkillReadResult {
resource: request.resource,
contents: "# Lint Fix\n\nRun the formatter.".to_string(),
contents: "# Skill".to_string(),
})
})
}
@@ -672,67 +172,24 @@ impl SkillProvider for StaticSkillProvider {
}
}
fn test_entry(
kind: SkillSourceKind,
authority_id: &str,
package_id: &str,
main_prompt: &str,
) -> SkillCatalogEntry {
let name = package_id.rsplit('/').next().unwrap_or(package_id);
fn test_entry(package_id: &str) -> SkillCatalogEntry {
SkillCatalogEntry::new(
SkillPackageId(package_id.to_string()),
SkillAuthority::new(kind, authority_id),
name,
SkillAuthority::new(SkillSourceKind::Orchestrator, "codex_apps"),
package_id.rsplit('/').next().unwrap_or(package_id),
"Fix lint errors.",
SkillResourceId::new(main_prompt),
SkillResourceId::new(format!("skill://{package_id}/SKILL.md")),
)
.with_display_path(format!("skill://{package_id}/SKILL.md"))
}
#[derive(Clone, Debug, Eq, PartialEq)]
struct TestConfig {
include_instructions: bool,
bundled_skills_enabled: bool,
orchestrator_skills_enabled: bool,
}
#[derive(Clone, Debug)]
struct TestConfig;
fn default_config() -> TestConfig {
TestConfig {
fn skills_extension_config(_config: &TestConfig) -> SkillsExtensionConfig {
SkillsExtensionConfig {
include_instructions: true,
bundled_skills_enabled: true,
orchestrator_skills_enabled: true,
}
}
fn skills_extension_config(config: &TestConfig) -> SkillsExtensionConfig {
SkillsExtensionConfig {
include_instructions: config.include_instructions,
bundled_skills_enabled: config.bundled_skills_enabled,
orchestrator_skills_enabled: config.orchestrator_skills_enabled,
}
}
fn test_codex_home() -> PathBuf {
let id = NEXT_CODEX_HOME_ID.fetch_add(1, Ordering::Relaxed);
std::env::temp_dir().join(format!(
"codex-skills-extension-test-{}-{id}",
std::process::id(),
))
}
fn read_request_keys(
requests: &Arc<Mutex<Vec<SkillReadRequest>>>,
) -> Vec<(SkillAuthority, SkillPackageId, SkillResourceId)> {
requests
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.iter()
.map(|request| {
(
request.authority.clone(),
request.package.clone(),
request.resource.clone(),
)
})
.collect()
}