Replace SkillsManager with SkillsService (#28705)

## Why

Host skill discovery was still exposed as a manager even though it is a
process-owned service shared by sessions, the app-server catalog, and
file-watcher invalidation. The skills extension also consumed an ad hoc
loaded-skills wrapper instead of a named immutable snapshot.

## What changed

- replace `SkillsManager` with concrete `SkillsService`
- make the service cache and return immutable `HostSkillsSnapshot`
values
- migrate the skills extension host provider to the snapshot boundary
- migrate app-server catalog, watcher, and invalidation paths to the
service

This keeps the service limited to host discovery, caching, roots, and
invalidation. Catalog rendering and invocation remain extension
responsibilities for the next stacked change.
This commit is contained in:
jif authored and GitHub committed 2026-06-17 17:01:06 +02:00
1 parent 5935a90619
commit 0318381762
34 files changed
+244 -235

No files matched your search

+1 -1
View File
@@ -379,7 +379,7 @@ impl MessageProcessor {
thread_manager thread_manager
.plugins_manager() .plugins_manager()
.set_analytics_events_client(analytics_events_client.clone()); .set_analytics_events_client(analytics_events_client.clone());
let skills_watcher = SkillsWatcher::new(thread_manager.skills_manager(), outgoing.clone()); let skills_watcher = SkillsWatcher::new(thread_manager.skills_service(), outgoing.clone());
let pending_thread_unloads = Arc::new(Mutex::new(HashSet::new())); let pending_thread_unloads = Arc::new(Mutex::new(HashSet::new()));
let thread_watch_manager = let thread_watch_manager =
@@ -219,7 +219,7 @@ impl AccountRequestProcessor {
) { ) {
tokio::spawn(async move { tokio::spawn(async move {
thread_manager.plugins_manager().clear_cache(); thread_manager.plugins_manager().clear_cache();
thread_manager.skills_manager().clear_cache(); thread_manager.skills_service().clear_cache();
if thread_manager.list_thread_ids().await.is_empty() { if thread_manager.list_thread_ids().await.is_empty() {
return; return;
} }
@@ -511,7 +511,7 @@ impl CatalogRequestProcessor {
let workspace_codex_plugins_enabled = self let workspace_codex_plugins_enabled = self
.workspace_codex_plugins_enabled(&config, auth.as_ref()) .workspace_codex_plugins_enabled(&config, auth.as_ref())
.await; .await;
let skills_manager = self.thread_manager.skills_manager(); let skills_service = self.thread_manager.skills_service();
let plugins_manager = self.thread_manager.plugins_manager(); let plugins_manager = self.thread_manager.plugins_manager();
let fs = self let fs = self
.thread_manager .thread_manager
@@ -523,7 +523,7 @@ impl CatalogRequestProcessor {
let config = &config; let config = &config;
let fs = fs.clone(); let fs = fs.clone();
let plugins_manager = &plugins_manager; let plugins_manager = &plugins_manager;
let skills_manager = &skills_manager; let skills_service = &skills_service;
async move { async move {
let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await { let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await {
Ok(resolved) => resolved, Ok(resolved) => resolved,
@@ -559,9 +559,10 @@ impl CatalogRequestProcessor {
config_layer_stack, config_layer_stack,
config.bundled_skills_enabled(), config.bundled_skills_enabled(),
); );
let outcome = skills_manager let snapshot = skills_service
.skills_for_cwd(&skills_input, force_reload, fs) .snapshot_for_cwd(&skills_input, force_reload, fs)
.await; .await;
let outcome = snapshot.outcome();
let errors = errors_to_info(&outcome.errors); let errors = errors_to_info(&outcome.errors);
let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths); let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths);
( (
@@ -590,7 +591,7 @@ impl CatalogRequestProcessor {
self.skills_watcher self.skills_watcher
.register_runtime_extra_roots(&extra_roots); .register_runtime_extra_roots(&extra_roots);
self.thread_manager self.thread_manager
.skills_manager() .skills_service()
.set_extra_roots(extra_roots); .set_extra_roots(extra_roots);
self.outgoing self.outgoing
.send_server_notification(ServerNotification::SkillsChanged( .send_server_notification(ServerNotification::SkillsChanged(
@@ -703,7 +704,7 @@ impl CatalogRequestProcessor {
.await .await
.map(|()| { .map(|()| {
self.thread_manager.plugins_manager().clear_cache(); self.thread_manager.plugins_manager().clear_cache();
self.thread_manager.skills_manager().clear_cache(); self.thread_manager.skills_service().clear_cache();
SkillsConfigWriteResponse { SkillsConfigWriteResponse {
effective_enabled: enabled, effective_enabled: enabled,
} }
@@ -169,7 +169,7 @@ impl ConfigRequestProcessor {
pub(crate) async fn handle_config_mutation(&self) { pub(crate) async fn handle_config_mutation(&self) {
self.thread_manager.plugins_manager().clear_cache(); self.thread_manager.plugins_manager().clear_cache();
self.thread_manager.skills_manager().clear_cache(); self.thread_manager.skills_service().clear_cache();
} }
async fn handle_config_mutation_result<T>( async fn handle_config_mutation_result<T>(
@@ -319,7 +319,7 @@ impl ExternalAgentConfigRequestProcessor {
completed_item_results.extend(background_item_results); completed_item_results.extend(background_item_results);
if has_plugin_imports { if has_plugin_imports {
thread_manager.plugins_manager().clear_cache(); thread_manager.plugins_manager().clear_cache();
thread_manager.skills_manager().clear_cache(); thread_manager.skills_service().clear_cache();
} }
send_completed_import_notification( send_completed_import_notification(
&outgoing, &outgoing,
@@ -474,7 +474,7 @@ impl PluginRequestProcessor {
) { ) {
tokio::spawn(async move { tokio::spawn(async move {
thread_manager.plugins_manager().clear_cache(); thread_manager.plugins_manager().clear_cache();
thread_manager.skills_manager().clear_cache(); thread_manager.skills_service().clear_cache();
if thread_manager.list_thread_ids().await.is_empty() { if thread_manager.list_thread_ids().await.is_empty() {
return; return;
} }
@@ -484,7 +484,7 @@ impl PluginRequestProcessor {
fn clear_plugin_related_caches(&self) { fn clear_plugin_related_caches(&self) {
self.thread_manager.plugins_manager().clear_cache(); self.thread_manager.plugins_manager().clear_cache();
self.thread_manager.skills_manager().clear_cache(); self.thread_manager.skills_service().clear_cache();
} }
async fn load_latest_config( async fn load_latest_config(
+6 -6
View File
@@ -8,7 +8,7 @@ use codex_app_server_protocol::SkillsChangedNotification;
use codex_core::ThreadManager; use codex_core::ThreadManager;
use codex_core::config::Config; use codex_core::config::Config;
use codex_core::skills::SkillsLoadInput; use codex_core::skills::SkillsLoadInput;
use codex_core::skills::SkillsManager; use codex_core::skills::SkillsService;
use codex_file_watcher::FileWatcher; use codex_file_watcher::FileWatcher;
use codex_file_watcher::FileWatcherSubscriber; use codex_file_watcher::FileWatcherSubscriber;
use codex_file_watcher::Receiver; use codex_file_watcher::Receiver;
@@ -35,7 +35,7 @@ pub(crate) struct SkillsWatcher {
impl SkillsWatcher { impl SkillsWatcher {
pub(crate) fn new( pub(crate) fn new(
skills_manager: Arc<SkillsManager>, skills_service: Arc<SkillsService>,
outgoing: Arc<OutgoingMessageSender>, outgoing: Arc<OutgoingMessageSender>,
) -> Arc<Self> { ) -> Arc<Self> {
let file_watcher = match FileWatcher::new() { let file_watcher = match FileWatcher::new() {
@@ -48,7 +48,7 @@ impl SkillsWatcher {
let (subscriber, rx) = file_watcher.add_subscriber(); let (subscriber, rx) = file_watcher.add_subscriber();
let shutdown_token = CancellationToken::new(); let shutdown_token = CancellationToken::new();
let shutdown_drop_guard = shutdown_token.clone().drop_guard(); let shutdown_drop_guard = shutdown_token.clone().drop_guard();
Self::spawn_event_loop(rx, skills_manager, outgoing, shutdown_token.child_token()); Self::spawn_event_loop(rx, skills_service, outgoing, shutdown_token.child_token());
Arc::new(Self { Arc::new(Self {
subscriber, subscriber,
runtime_extra_roots_registration: Mutex::new(WatchRegistration::default()), runtime_extra_roots_registration: Mutex::new(WatchRegistration::default()),
@@ -110,7 +110,7 @@ impl SkillsWatcher {
config.bundled_skills_enabled(), config.bundled_skills_enabled(),
); );
let roots = thread_manager let roots = thread_manager
.skills_manager() .skills_service()
.skill_roots_for_config(&skills_input, Some(environment.get_filesystem())) .skill_roots_for_config(&skills_input, Some(environment.get_filesystem()))
.await .await
.into_iter() .into_iter()
@@ -124,7 +124,7 @@ impl SkillsWatcher {
fn spawn_event_loop( fn spawn_event_loop(
rx: Receiver, rx: Receiver,
skills_manager: Arc<SkillsManager>, skills_service: Arc<SkillsService>,
outgoing: Arc<OutgoingMessageSender>, outgoing: Arc<OutgoingMessageSender>,
shutdown_token: CancellationToken, shutdown_token: CancellationToken,
) { ) {
@@ -142,7 +142,7 @@ impl SkillsWatcher {
if event.is_none() { if event.is_none() {
break; break;
} }
skills_manager.clear_cache(); skills_service.clear_cache();
outgoing outgoing
.send_server_notification(ServerNotification::SkillsChanged( .send_server_notification(ServerNotification::SkillsChanged(
SkillsChangedNotification {}, SkillsChangedNotification {},
+1 -1
View File
@@ -45,7 +45,7 @@ pub use codex_core::config::ThreadStoreConfig;
pub use codex_core::config::find_codex_home; pub use codex_core::config::find_codex_home;
pub use codex_core::init_state_db; pub use codex_core::init_state_db;
pub use codex_core::resolve_installation_id; pub use codex_core::resolve_installation_id;
pub use codex_core::skills::SkillsManager; pub use codex_core::skills::SkillsService;
pub use codex_core::thread_store_from_config; pub use codex_core::thread_store_from_config;
pub use codex_exec_server::EnvironmentManager; pub use codex_exec_server::EnvironmentManager;
pub use codex_exec_server::ExecServerError; pub use codex_exec_server::ExecServerError;
+4 -4
View File
@@ -2,20 +2,18 @@ pub mod config_rules;
pub mod injection; pub mod injection;
pub(crate) mod invocation_utils; pub(crate) mod invocation_utils;
pub mod loader; pub mod loader;
pub mod manager;
mod mention_counts; mod mention_counts;
pub mod model; pub mod model;
pub mod remote; pub mod remote;
pub mod render; pub mod render;
pub mod service;
mod skill_instructions; mod skill_instructions;
pub mod system; pub mod system;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes; pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub use invocation_utils::detect_implicit_skill_invocation_for_command; pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use manager::SkillsLoadInput;
pub use manager::SkillsManager;
pub use mention_counts::build_skill_name_counts; pub use mention_counts::build_skill_name_counts;
pub use model::HostLoadedSkills; pub use model::HostSkillsSnapshot;
pub use model::SkillError; pub use model::SkillError;
pub use model::SkillLoadOutcome; pub use model::SkillLoadOutcome;
pub use model::SkillMetadata; pub use model::SkillMetadata;
@@ -31,4 +29,6 @@ pub use render::SkillRenderReport;
pub use render::build_available_skills; pub use render::build_available_skills;
pub use render::default_skill_metadata_budget; pub use render::default_skill_metadata_budget;
pub use render::render_available_skills_body; pub use render::render_available_skills_body;
pub use service::SkillsLoadInput;
pub use service::SkillsService;
pub use skill_instructions::SkillInstructions; pub use skill_instructions::SkillInstructions;
+4 -4
View File
@@ -132,14 +132,14 @@ impl SkillLoadOutcome {
} }
} }
/// Host-loaded skills for one turn, including the filesystem mapping needed to /// Immutable snapshot of host-owned skills and the filesystem mapping needed
/// read skill bodies through the environment that loaded them. /// to read each skill through the environment that discovered it.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct HostLoadedSkills { pub struct HostSkillsSnapshot {
outcome: Arc<SkillLoadOutcome>, outcome: Arc<SkillLoadOutcome>,
} }
impl HostLoadedSkills { impl HostSkillsSnapshot {
pub fn new(outcome: Arc<SkillLoadOutcome>) -> Self { pub fn new(outcome: Arc<SkillLoadOutcome>) -> Self {
Self { outcome } Self { outcome }
} }
@@ -13,6 +13,7 @@ use tracing::info;
use tracing::instrument; use tracing::instrument;
use tracing::warn; use tracing::warn;
use crate::HostSkillsSnapshot;
use crate::SkillLoadOutcome; use crate::SkillLoadOutcome;
use crate::build_implicit_skill_path_indexes; use crate::build_implicit_skill_path_indexes;
use crate::config_rules::SkillConfigRules; use crate::config_rules::SkillConfigRules;
@@ -49,15 +50,18 @@ impl SkillsLoadInput {
} }
} }
pub struct SkillsManager { /// Owns host skill discovery, immutable snapshots, cache invalidation, and extra roots.
///
/// Source-specific model exposure remains the responsibility of the skills extension.
pub struct SkillsService {
codex_home: AbsolutePathBuf, codex_home: AbsolutePathBuf,
restriction_product: Option<Product>, restriction_product: Option<Product>,
extra_roots: RwLock<Vec<AbsolutePathBuf>>, extra_roots: RwLock<Vec<AbsolutePathBuf>>,
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>, cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, HostSkillsSnapshot>>,
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>, cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, HostSkillsSnapshot>>,
} }
impl SkillsManager { impl SkillsService {
pub fn new(codex_home: AbsolutePathBuf, bundled_skills_enabled: bool) -> Self { pub fn new(codex_home: AbsolutePathBuf, bundled_skills_enabled: bool) -> Self {
Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex)) Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex))
} }
@@ -67,7 +71,7 @@ impl SkillsManager {
bundled_skills_enabled: bool, bundled_skills_enabled: bool,
restriction_product: Option<Product>, restriction_product: Option<Product>,
) -> Self { ) -> Self {
let manager = Self { let service = Self {
codex_home, codex_home,
restriction_product, restriction_product,
extra_roots: RwLock::new(Vec::new()), extra_roots: RwLock::new(Vec::new()),
@@ -77,11 +81,11 @@ impl SkillsManager {
if !bundled_skills_enabled { if !bundled_skills_enabled {
// The loader caches bundled skills under `skills/.system`. Clearing that directory is // The loader caches bundled skills under `skills/.system`. Clearing that directory is
// best-effort cleanup; root selection still enforces the config even if removal fails. // best-effort cleanup; root selection still enforces the config even if removal fails.
uninstall_system_skills(&manager.codex_home); uninstall_system_skills(&service.codex_home);
} else if let Err(err) = install_system_skills(&manager.codex_home) { } else if let Err(err) = install_system_skills(&service.codex_home) {
tracing::error!("failed to install system skills: {err}"); tracing::error!("failed to install system skills: {err}");
} }
manager service
} }
pub fn set_extra_roots(&self, extra_roots: Vec<AbsolutePathBuf>) { pub fn set_extra_roots(&self, extra_roots: Vec<AbsolutePathBuf>) {
@@ -102,25 +106,27 @@ impl SkillsManager {
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen /// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
/// to share a directory. /// to share a directory.
#[instrument(level = "trace", skip_all)] #[instrument(level = "trace", skip_all)]
pub async fn skills_for_config( pub async fn snapshot_for_config(
&self, &self,
input: &SkillsLoadInput, input: &SkillsLoadInput,
fs: Option<Arc<dyn ExecutorFileSystem>>, fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome { ) -> HostSkillsSnapshot {
let roots = self.skill_roots_for_config(input, fs).await; let roots = self.skill_roots_for_config(input, fs).await;
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let cache_key = config_skills_cache_key(&roots, &skill_config_rules); let cache_key = config_skills_cache_key(&roots, &skill_config_rules);
if let Some(outcome) = self.cached_outcome_for_config(&cache_key) { if let Some(snapshot) = self.cached_snapshot_for_config(&cache_key) {
return outcome; return snapshot;
} }
let outcome = self.build_skill_outcome(roots, &skill_config_rules).await; let snapshot = HostSkillsSnapshot::new(Arc::new(
self.build_skill_outcome(roots, &skill_config_rules).await,
));
let mut cache = self let mut cache = self
.cache_by_config .cache_by_config
.write() .write()
.unwrap_or_else(std::sync::PoisonError::into_inner); .unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(cache_key, outcome.clone()); cache.insert(cache_key, snapshot.clone());
outcome snapshot
} }
pub async fn skill_roots_for_config( pub async fn skill_roots_for_config(
@@ -142,18 +148,18 @@ impl SkillsManager {
roots roots
} }
pub async fn skills_for_cwd( pub async fn snapshot_for_cwd(
&self, &self,
input: &SkillsLoadInput, input: &SkillsLoadInput,
force_reload: bool, force_reload: bool,
fs: Option<Arc<dyn ExecutorFileSystem>>, fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> SkillLoadOutcome { ) -> HostSkillsSnapshot {
let use_cwd_cache = fs.is_some(); let use_cwd_cache = fs.is_some();
if use_cwd_cache if use_cwd_cache
&& !force_reload && !force_reload
&& let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd) && let Some(snapshot) = self.cached_snapshot_for_cwd(&input.cwd)
{ {
return outcome; return snapshot;
} }
let mut roots = skill_roots( let mut roots = skill_roots(
@@ -168,15 +174,17 @@ impl SkillsManager {
roots.retain(|root| root.scope != SkillScope::System); roots.retain(|root| root.scope != SkillScope::System);
} }
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let outcome = self.build_skill_outcome(roots, &skill_config_rules).await; let snapshot = HostSkillsSnapshot::new(Arc::new(
self.build_skill_outcome(roots, &skill_config_rules).await,
));
if use_cwd_cache { if use_cwd_cache {
let mut cache = self let mut cache = self
.cache_by_cwd .cache_by_cwd
.write() .write()
.unwrap_or_else(std::sync::PoisonError::into_inner); .unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(input.cwd.clone(), outcome.clone()); cache.insert(input.cwd.clone(), snapshot.clone());
} }
outcome snapshot
} }
#[instrument(level = "trace", skip_all)] #[instrument(level = "trace", skip_all)]
@@ -216,17 +224,17 @@ impl SkillsManager {
info!("skills cache cleared ({cleared} entries)"); info!("skills cache cleared ({cleared} entries)");
} }
fn cached_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<SkillLoadOutcome> { fn cached_snapshot_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option<HostSkillsSnapshot> {
match self.cache_by_cwd.read() { match self.cache_by_cwd.read() {
Ok(cache) => cache.get(cwd).cloned(), Ok(cache) => cache.get(cwd).cloned(),
Err(err) => err.into_inner().get(cwd).cloned(), Err(err) => err.into_inner().get(cwd).cloned(),
} }
} }
fn cached_outcome_for_config( fn cached_snapshot_for_config(
&self, &self,
cache_key: &ConfigSkillsCacheKey, cache_key: &ConfigSkillsCacheKey,
) -> Option<SkillLoadOutcome> { ) -> Option<HostSkillsSnapshot> {
match self.cache_by_config.read() { match self.cache_by_config.read() {
Ok(cache) => cache.get(cache_key).cloned(), Ok(cache) => cache.get(cache_key).cloned(),
Err(err) => err.into_inner().get(cache_key).cloned(), Err(err) => err.into_inner().get(cache_key).cloned(),
@@ -303,5 +311,5 @@ fn finalize_skill_outcome(
} }
#[cfg(test)] #[cfg(test)]
#[path = "manager_tests.rs"] #[path = "service_tests.rs"]
mod tests; mod tests;
@@ -159,7 +159,7 @@ enabled = {enabled}
} }
async fn skills_for_config_with_stack( async fn skills_for_config_with_stack(
skills_manager: &SkillsManager, skills_service: &SkillsService,
cwd: &TempDir, cwd: &TempDir,
config_layer_stack: &ConfigLayerStack, config_layer_stack: &ConfigLayerStack,
effective_skill_roots: &[PluginSkillRoot], effective_skill_roots: &[PluginSkillRoot],
@@ -170,9 +170,11 @@ async fn skills_for_config_with_stack(
config_layer_stack.clone(), config_layer_stack.clone(),
bundled_skills_enabled_from_stack(config_layer_stack), bundled_skills_enabled_from_stack(config_layer_stack),
); );
skills_manager skills_service
.skills_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS))) .snapshot_for_config(&skills_input, Some(Arc::clone(&LOCAL_FS)))
.await .await
.outcome()
.clone()
} }
#[test] #[test]
@@ -183,7 +185,7 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n") fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n")
.expect("write stale system skill"); .expect("write stale system skill");
let _skills_manager = SkillsManager::new( let _skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ false, /*bundled_skills_enabled*/ false,
); );
@@ -199,14 +201,14 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, ""); let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
write_user_skill(&codex_home, "a", "skill-a", "from a"); write_user_skill(&codex_home, "a", "skill-a", "from a");
let outcome1 = let outcome1 =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert!( assert!(
outcome1.skills.iter().any(|s| s.name == "skill-a"), outcome1.skills.iter().any(|s| s.name == "skill-a"),
"expected skill-a to be discovered" "expected skill-a to be discovered"
@@ -216,7 +218,7 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
// entry because the effective skill config is unchanged. // entry because the effective skill config is unchanged.
write_user_skill(&codex_home, "b", "skill-b", "from b"); write_user_skill(&codex_home, "b", "skill-b", "from b");
let outcome2 = let outcome2 =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert_eq!(outcome2.errors, outcome1.errors); assert_eq!(outcome2.errors, outcome1.errors);
assert_eq!(outcome2.skills, outcome1.skills); assert_eq!(outcome2.skills, outcome1.skills);
} }
@@ -227,7 +229,7 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
let cwd = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir"); let extra_root = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, ""); let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
@@ -238,13 +240,14 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
config_layer_stack.clone(), config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack), bundled_skills_enabled_from_stack(&config_layer_stack),
); );
let empty_outcome = skills_manager let empty_snapshot = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&skills_input, &skills_input,
/*force_reload*/ false, /*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let empty_outcome = empty_snapshot.outcome();
assert!( assert!(
empty_outcome empty_outcome
.skills .skills
@@ -260,15 +263,16 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
"---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n", "---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n",
) )
.expect("write skill"); .expect("write skill");
skills_manager.set_extra_roots(vec![extra_skills_root.abs()]); skills_service.set_extra_roots(vec![extra_skills_root.abs()]);
let runtime_outcome = skills_manager let runtime_snapshot = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&skills_input, &skills_input,
/*force_reload*/ false, /*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let runtime_outcome = runtime_snapshot.outcome();
assert!( assert!(
runtime_outcome runtime_outcome
.skills .skills
@@ -276,14 +280,15 @@ async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
.any(|skill| skill.name == "runtime-skill") .any(|skill| skill.name == "runtime-skill")
); );
skills_manager.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]); skills_service.set_extra_roots(vec![extra_root.path().join("missing-skills").abs()]);
let replaced_outcome = skills_manager let replaced_snapshot = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&skills_input, &skills_input,
/*force_reload*/ false, /*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let replaced_outcome = replaced_snapshot.outcome();
assert_eq!(replaced_outcome.errors, Vec::new()); assert_eq!(replaced_outcome.errors, Vec::new());
assert!( assert!(
replaced_outcome replaced_outcome
@@ -299,13 +304,13 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() {
let cwd = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir"); let extra_root = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, ""); let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
let empty_outcome = let empty_outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert!( assert!(
empty_outcome empty_outcome
.skills .skills
@@ -321,10 +326,10 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() {
"---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n", "---\nname: runtime-skill\ndescription: runtime skill\n---\n\n# Body\n",
) )
.expect("write skill"); .expect("write skill");
skills_manager.set_extra_roots(vec![extra_skills_root.abs()]); skills_service.set_extra_roots(vec![extra_skills_root.abs()]);
let runtime_outcome = let runtime_outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert!( assert!(
runtime_outcome runtime_outcome
.skills .skills
@@ -332,9 +337,9 @@ async fn set_extra_roots_applies_to_config_loads_and_empty_clears() {
.any(|skill| skill.name == "runtime-skill") .any(|skill| skill.name == "runtime-skill")
); );
skills_manager.set_extra_roots(Vec::new()); skills_service.set_extra_roots(Vec::new());
let cleared_outcome = let cleared_outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert!( assert!(
cleared_outcome cleared_outcome
.skills .skills
@@ -360,13 +365,13 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
&name_toggle_config("sample:sample-search", /*enabled*/ false), &name_toggle_config("sample:sample-search", /*enabled*/ false),
); );
let plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test"); let plugin_skill_root = plugin_skill_root_for_skill_path(&skill_path, "test-plugin@test");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
let outcome = skills_for_config_with_stack( let outcome = skills_for_config_with_stack(
&skills_manager, &skills_service,
&cwd, &cwd,
&config_layer_stack, &config_layer_stack,
&[plugin_skill_root], &[plugin_skill_root],
@@ -427,18 +432,19 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
config_layer_stack.clone(), config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack), bundled_skills_enabled_from_stack(&config_layer_stack),
); );
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
let outcome = skills_manager let snapshot = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&skills_input, &skills_input,
/*force_reload*/ true, /*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let outcome = snapshot.outcome();
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -490,14 +496,15 @@ async fn skills_for_cwd_without_fs_skips_repo_roots() {
config_layer_stack.clone(), config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack), bundled_skills_enabled_from_stack(&config_layer_stack),
); );
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
let outcome = skills_manager let snapshot = skills_service
.skills_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None) .snapshot_for_cwd(&skills_input, /*force_reload*/ true, /*fs*/ None)
.await; .await;
let outcome = snapshot.outcome();
assert!( assert!(
outcome.errors.is_empty(), outcome.errors.is_empty(),
@@ -525,7 +532,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
) )
.expect("write bundled skill"); .expect("write bundled skill");
let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n"); let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ false, /*bundled_skills_enabled*/ false,
); );
@@ -540,7 +547,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
.expect("rewrite bundled skill"); .expect("rewrite bundled skill");
let outcome = let outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
assert!( assert!(
outcome outcome
.skills .skills
@@ -560,24 +567,25 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir");
let config_layer_stack = config_stack(&codex_home, ""); let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]).await; let _ = skills_for_config_with_stack(&skills_service, &cwd, &config_layer_stack, &[]).await;
let base_input = SkillsLoadInput::new( let base_input = SkillsLoadInput::new(
cwd.path().abs(), cwd.path().abs(),
Vec::new(), Vec::new(),
config_layer_stack.clone(), config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack), bundled_skills_enabled_from_stack(&config_layer_stack),
); );
let outcome_a = skills_manager let snapshot_a = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&base_input, &base_input,
/*force_reload*/ false, /*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let outcome_a = snapshot_a.outcome();
assert!( assert!(
outcome_a outcome_a
.skills .skills
@@ -587,13 +595,14 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
write_user_skill(&codex_home, "late", "late-skill", "added after cache"); write_user_skill(&codex_home, "late", "late-skill", "added after cache");
let outcome_b = skills_manager let snapshot_b = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&base_input, &base_input,
/*force_reload*/ false, /*force_reload*/ false,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let outcome_b = snapshot_b.outcome();
assert!( assert!(
outcome_b outcome_b
.skills .skills
@@ -601,13 +610,14 @@ async fn skills_for_cwd_uses_cached_result_until_force_reload() {
.all(|skill| skill.name != "late-skill") .all(|skill| skill.name != "late-skill")
); );
let outcome_reloaded = skills_manager let snapshot_reloaded = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&base_input, &base_input,
/*force_reload*/ true, /*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let outcome_reloaded = snapshot_reloaded.outcome();
assert!( assert!(
outcome_reloaded outcome_reloaded
.skills .skills
@@ -775,7 +785,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
let parent_stack = config_stack(&codex_home, &disabled_skill_config); let parent_stack = config_stack(&codex_home, &disabled_skill_config);
let child_stack = let child_stack =
config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config); config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config);
let skills_manager = SkillsManager::new( let skills_service = SkillsService::new(
codex_home.path().abs(), codex_home.path().abs(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
); );
@@ -786,13 +796,14 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
bundled_skills_enabled_from_stack(&parent_stack), bundled_skills_enabled_from_stack(&parent_stack),
); );
let parent_outcome = skills_manager let parent_snapshot = skills_service
.skills_for_cwd( .snapshot_for_cwd(
&parent_input, &parent_input,
/*force_reload*/ true, /*force_reload*/ true,
Some(Arc::clone(&LOCAL_FS)), Some(Arc::clone(&LOCAL_FS)),
) )
.await; .await;
let parent_outcome = parent_snapshot.outcome();
let parent_skill = parent_outcome let parent_skill = parent_outcome
.skills .skills
.iter() .iter()
@@ -801,7 +812,7 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false); assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false);
let child_outcome = let child_outcome =
skills_for_config_with_stack(&skills_manager, &cwd, &child_stack, &[]).await; skills_for_config_with_stack(&skills_service, &cwd, &child_stack, &[]).await;
let child_skill = child_outcome let child_skill = child_outcome
.skills .skills
.iter() .iter()
+6 -5
View File
@@ -1,5 +1,5 @@
use super::*; use super::*;
use crate::SkillsManager; use crate::SkillsService;
use crate::config::ConfigBuilder; use crate::config::ConfigBuilder;
use crate::skills_load_input_from_config; use crate::skills_load_input_from_config;
use codex_config::ConfigLayerStackOrdering; use codex_config::ConfigLayerStackOrdering;
@@ -416,18 +416,19 @@ enabled = false
.expect("custom role should apply"); .expect("custom role should apply");
let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf()));
let skills_manager = let skills_service =
SkillsManager::new(home.path().abs(), /*bundled_skills_enabled*/ true); SkillsService::new(home.path().abs(), /*bundled_skills_enabled*/ true);
let plugins_input = config.plugins_config_input(); let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input = skills_load_input_from_config(&config, effective_skill_roots); let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
let outcome = skills_manager let snapshot = skills_service
.skills_for_config( .snapshot_for_config(
&skills_input, &skills_input,
Some(Arc::clone(&codex_exec_server::LOCAL_FS)), Some(Arc::clone(&codex_exec_server::LOCAL_FS)),
) )
.await; .await;
let outcome = snapshot.outcome();
let skill = outcome let skill = outcome
.skills .skills
.iter() .iter()
+1 -1
View File
@@ -94,7 +94,7 @@ pub(crate) async fn run_codex_thread_interactive(
.services .services
.turn_environments .turn_environments
.environment_manager(), .environment_manager(),
skills_manager: Arc::clone(&parent_session.services.skills_manager), skills_service: Arc::clone(&parent_session.services.skills_service),
plugins_manager: Arc::clone(&parent_session.services.plugins_manager), plugins_manager: Arc::clone(&parent_session.services.plugins_manager),
mcp_manager: Arc::clone(&parent_session.services.mcp_manager), mcp_manager: Arc::clone(&parent_session.services.mcp_manager),
extensions: Arc::clone(&parent_session.services.extensions), extensions: Arc::clone(&parent_session.services.extensions),
+1 -1
View File
@@ -3874,7 +3874,7 @@ impl Config {
} }
pub fn bundled_skills_enabled(&self) -> bool { pub fn bundled_skills_enabled(&self) -> bool {
crate::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack) crate::skills::service::bundled_skills_enabled_from_stack(&self.config_layer_stack)
} }
} }
+1 -1
View File
@@ -1678,7 +1678,7 @@ async fn guardian_review_request_layout_matches_model_visible_request_snapshot()
"---\nname: {GUARDIAN_SKILL_NAME}\ndescription: Guardian skill injection probe.\n---\n\n{GUARDIAN_SKILL_BODY_PROBE}\n" "---\nname: {GUARDIAN_SKILL_NAME}\ndescription: Guardian skill injection probe.\n---\n\n{GUARDIAN_SKILL_BODY_PROBE}\n"
), ),
)?; )?;
session.services.skills_manager.clear_cache(); session.services.skills_service.clear_cache();
turn.config = Arc::clone(&config); turn.config = Arc::clone(&config);
turn.provider = create_model_provider(config.model_provider.clone(), turn.auth_manager.clone()); turn.provider = create_model_provider(config.model_provider.clone(), turn.auth_manager.clone());
let session = Arc::new(session); let session = Arc::new(session);
+1 -3
View File
@@ -87,16 +87,14 @@ mod session_prefix;
mod session_startup_prewarm; mod session_startup_prewarm;
pub mod skills; pub mod skills;
pub(crate) use skills::SkillInjections; pub(crate) use skills::SkillInjections;
pub(crate) use skills::SkillLoadOutcome;
pub(crate) use skills::SkillMetadata; pub(crate) use skills::SkillMetadata;
pub(crate) use skills::SkillsManager; pub(crate) use skills::SkillsService;
pub(crate) use skills::build_available_skills; pub(crate) use skills::build_available_skills;
pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_injections;
pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::build_skill_name_counts;
pub(crate) use skills::collect_explicit_skill_mentions; pub(crate) use skills::collect_explicit_skill_mentions;
pub(crate) use skills::default_skill_metadata_budget; pub(crate) use skills::default_skill_metadata_budget;
pub(crate) use skills::injection; pub(crate) use skills::injection;
pub(crate) use skills::manager;
pub(crate) use skills::maybe_emit_implicit_skill_invocation; pub(crate) use skills::maybe_emit_implicit_skill_invocation;
pub(crate) use skills::skills_load_input_from_config; pub(crate) use skills::skills_load_input_from_config;
mod stream_events_utils; mod stream_events_utils;
+8 -8
View File
@@ -291,11 +291,9 @@ pub(crate) struct PreviousTurnSettings {
pub(crate) realtime_active: Option<bool>, pub(crate) realtime_active: Option<bool>,
} }
#[cfg(test)]
use crate::SkillLoadOutcome;
#[cfg(test)] #[cfg(test)]
use crate::SkillMetadata; use crate::SkillMetadata;
use crate::SkillsManager; use crate::SkillsService;
use crate::agents_md::load_project_instructions; use crate::agents_md::load_project_instructions;
use crate::exec_policy::ExecPolicyUpdateError; use crate::exec_policy::ExecPolicyUpdateError;
use crate::guardian::GuardianReviewSessionManager; use crate::guardian::GuardianReviewSessionManager;
@@ -304,6 +302,8 @@ use crate::network_policy_decision::execpolicy_network_rule_amendment;
use crate::rollout::map_session_init_error; use crate::rollout::map_session_init_error;
use crate::session_startup_prewarm::SessionStartupPrewarmHandle; use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
use crate::shell; use crate::shell;
#[cfg(test)]
use crate::skills::SkillLoadOutcome;
use crate::state::AutoCompactWindowSnapshot; use crate::state::AutoCompactWindowSnapshot;
use crate::state::PendingRequestPermissions; use crate::state::PendingRequestPermissions;
use crate::state::SessionServices; use crate::state::SessionServices;
@@ -409,7 +409,7 @@ pub(crate) struct CodexSpawnArgs {
pub(crate) auth_manager: Arc<AuthManager>, pub(crate) auth_manager: Arc<AuthManager>,
pub(crate) models_manager: SharedModelsManager, pub(crate) models_manager: SharedModelsManager,
pub(crate) environment_manager: Arc<EnvironmentManager>, pub(crate) environment_manager: Arc<EnvironmentManager>,
pub(crate) skills_manager: Arc<SkillsManager>, pub(crate) skills_service: Arc<SkillsService>,
pub(crate) plugins_manager: Arc<PluginsManager>, pub(crate) plugins_manager: Arc<PluginsManager>,
pub(crate) mcp_manager: Arc<McpManager>, pub(crate) mcp_manager: Arc<McpManager>,
pub(crate) extensions: Arc<codex_extension_api::ExtensionRegistry<crate::config::Config>>, pub(crate) extensions: Arc<codex_extension_api::ExtensionRegistry<crate::config::Config>>,
@@ -495,7 +495,7 @@ impl Codex {
auth_manager, auth_manager,
models_manager, models_manager,
environment_manager, environment_manager,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions, extensions,
@@ -651,7 +651,7 @@ impl Codex {
agent_status_tx.clone(), agent_status_tx.clone(),
conversation_history, conversation_history,
session_source_clone, session_source_clone,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager.clone(), mcp_manager.clone(),
extensions, extensions,
@@ -1507,7 +1507,7 @@ impl Session {
(previous_config, new_config, config) (previous_config, new_config, config)
}; };
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref()); self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
self.services.skills_manager.clear_cache(); self.services.skills_service.clear_cache();
self.services.plugins_manager.clear_cache(); self.services.plugins_manager.clear_cache();
let environments = self.services.turn_environments.snapshot().await; let environments = self.services.turn_environments.snapshot().await;
let hooks = build_hooks_for_config( let hooks = build_hooks_for_config(
@@ -2969,7 +2969,7 @@ impl Session {
} }
if turn_context.config.include_skill_instructions { if turn_context.config.include_skill_instructions {
let available_skills = build_available_skills( let available_skills = build_available_skills(
&turn_context.turn_skills.outcome, turn_context.turn_skills.snapshot.outcome(),
default_skill_metadata_budget(turn_context.model_info.context_window), default_skill_metadata_budget(turn_context.model_info.context_window),
SkillRenderSideEffects::ThreadStart { SkillRenderSideEffects::ThreadStart {
session_telemetry: &self.services.session_telemetry, session_telemetry: &self.services.session_telemetry,
+2 -5
View File
@@ -1,5 +1,4 @@
use super::*; use super::*;
use codex_core_skills::HostLoadedSkills;
use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicBool;
/// Spawn a review thread using the given prompt. /// Spawn a review thread using the given prompt.
@@ -100,9 +99,7 @@ pub(super) async fn spawn_review_thread(
let extension_data = Arc::new(codex_extension_api::ExtensionData::new( let extension_data = Arc::new(codex_extension_api::ExtensionData::new(
review_turn_id.clone(), review_turn_id.clone(),
)); ));
extension_data.insert(HostLoadedSkills::new( extension_data.insert(parent_turn_context.turn_skills.snapshot.clone());
parent_turn_context.turn_skills.outcome.clone(),
));
let review_turn_context = TurnContext { let review_turn_context = TurnContext {
sub_id: review_turn_id.clone(), sub_id: review_turn_id.clone(),
@@ -138,7 +135,7 @@ pub(super) async fn spawn_review_thread(
dynamic_tools: parent_turn_context.dynamic_tools.clone(), dynamic_tools: parent_turn_context.dynamic_tools.clone(),
turn_metadata_state, turn_metadata_state,
extension_data, extension_data,
turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.outcome.clone()), turn_skills: TurnSkillsContext::new(parent_turn_context.turn_skills.snapshot.clone()),
turn_timing_state: Arc::new(TurnTimingState::default()), turn_timing_state: Arc::new(TurnTimingState::default()),
terminal_error: Arc::new(Mutex::new(None)), terminal_error: Arc::new(Mutex::new(None)),
server_model_warning_emitted: AtomicBool::new(false), server_model_warning_emitted: AtomicBool::new(false),
+8 -6
View File
@@ -438,7 +438,7 @@ pub(crate) struct AppServerClientMetadata {
async fn warm_plugins_and_skills_for_session_init( async fn warm_plugins_and_skills_for_session_init(
config: Arc<Config>, config: Arc<Config>,
plugins_manager: Arc<PluginsManager>, plugins_manager: Arc<PluginsManager>,
skills_manager: Arc<SkillsManager>, skills_service: Arc<SkillsService>,
turn_environments: &TurnEnvironmentSnapshot, turn_environments: &TurnEnvironmentSnapshot,
) -> Vec<SkillError> { ) -> Vec<SkillError> {
let fs = turn_environments.primary_filesystem(); let fs = turn_environments.primary_filesystem();
@@ -446,10 +446,12 @@ async fn warm_plugins_and_skills_for_session_init(
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots(); let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots); let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots);
skills_manager skills_service
.skills_for_config(&skills_input, fs) .snapshot_for_config(&skills_input, fs)
.await .await
.outcome()
.errors .errors
.clone()
} }
impl Session { impl Session {
@@ -477,7 +479,7 @@ impl Session {
agent_status: watch::Sender<AgentStatus>, agent_status: watch::Sender<AgentStatus>,
initial_history: InitialHistory, initial_history: InitialHistory,
session_source: SessionSource, session_source: SessionSource,
skills_manager: Arc<SkillsManager>, skills_service: Arc<SkillsService>,
plugins_manager: Arc<PluginsManager>, plugins_manager: Arc<PluginsManager>,
mcp_manager: Arc<McpManager>, mcp_manager: Arc<McpManager>,
extensions: Arc<codex_extension_api::ExtensionRegistry<crate::config::Config>>, extensions: Arc<codex_extension_api::ExtensionRegistry<crate::config::Config>>,
@@ -828,7 +830,7 @@ impl Session {
let plugin_skill_errors = warm_plugins_and_skills_for_session_init( let plugin_skill_errors = warm_plugins_and_skills_for_session_init(
Arc::clone(&config), Arc::clone(&config),
Arc::clone(&plugins_manager), Arc::clone(&plugins_manager),
Arc::clone(&skills_manager), Arc::clone(&skills_service),
&resolved_environments, &resolved_environments,
) )
.instrument(info_span!( .instrument(info_span!(
@@ -999,7 +1001,7 @@ impl Session {
guardian_rejections: Mutex::new(HashMap::new()), guardian_rejections: Mutex::new(HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(), runtime_handle: tokio::runtime::Handle::current(),
skills_manager, skills_service,
plugins_manager: Arc::clone(&plugins_manager), plugins_manager: Arc::clone(&plugins_manager),
mcp_manager: Arc::clone(&mcp_manager), mcp_manager: Arc::clone(&mcp_manager),
extensions, extensions,
+34 -31
View File
@@ -24,6 +24,7 @@ use codex_config::RequirementSource;
use codex_config::Sourced; use codex_config::Sourced;
use codex_config::loader::project_trust_key; use codex_config::loader::project_trust_key;
use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDisabledTool;
use codex_core_skills::HostSkillsSnapshot;
use core_test_support::test_codex::local_selections; use core_test_support::test_codex::local_selections;
use codex_features::Feature; use codex_features::Feature;
@@ -4434,15 +4435,16 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
.default_environment() .default_environment()
.map(|environment| environment.get_filesystem()) .map(|environment| environment.get_filesystem())
.unwrap_or_else(|| std::sync::Arc::clone(&codex_exec_server::LOCAL_FS)); .unwrap_or_else(|| std::sync::Arc::clone(&codex_exec_server::LOCAL_FS));
let parent_outcome = session let parent_snapshot = session
.services .services
.skills_manager .skills_service
.skills_for_cwd( .snapshot_for_cwd(
&crate::skills_load_input_from_config(&parent_config, Vec::new()), &crate::skills_load_input_from_config(&parent_config, Vec::new()),
/*force_reload*/ true, /*force_reload*/ true,
Some(Arc::clone(&skill_fs)), Some(Arc::clone(&skill_fs)),
) )
.await; .await;
let parent_outcome = parent_snapshot.outcome();
let parent_skill = parent_outcome let parent_skill = parent_outcome
.skills .skills
.iter() .iter()
@@ -4488,13 +4490,18 @@ enabled = false
.await; .await;
let child_skill = child_turn let child_skill = child_turn
.turn_skills .turn_skills
.outcome .snapshot
.outcome()
.skills .skills
.iter() .iter()
.find(|skill| skill.name == "demo-skill") .find(|skill| skill.name == "demo-skill")
.expect("demo skill should be discovered"); .expect("demo skill should be discovered");
assert_eq!( assert_eq!(
child_turn.turn_skills.outcome.is_skill_enabled(child_skill), child_turn
.turn_skills
.snapshot
.outcome()
.is_skill_enabled(child_skill),
false false
); );
} }
@@ -4825,7 +4832,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -4842,7 +4849,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
agent_status_tx, agent_status_tx,
InitialHistory::New, InitialHistory::New,
SessionSource::Exec, SessionSource::Exec,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
@@ -4961,7 +4968,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
); );
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -5000,7 +5007,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
guardian_rejections: Mutex::new(std::collections::HashMap::new()), guardian_rejections: Mutex::new(std::collections::HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(), runtime_handle: tokio::runtime::Handle::current(),
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
@@ -5045,12 +5052,10 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
let skills_input = let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skill_fs = environment.get_filesystem(); let skill_fs = environment.get_filesystem();
let skills_outcome = Arc::new( let skills_snapshot = services
services .skills_service
.skills_manager .snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) .await;
.await,
);
let turn_context = Session::make_turn_context( let turn_context = Session::make_turn_context(
thread_id, thread_id,
SessionId::from(thread_id), SessionId::from(thread_id),
@@ -5069,7 +5074,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
resolved_turn_environments, resolved_turn_environments,
session_configuration.cwd().clone(), session_configuration.cwd().clone(),
"turn_id".to_string(), "turn_id".to_string(),
skills_outcome, skills_snapshot,
); );
let session = Session { let session = Session {
@@ -5173,7 +5178,7 @@ async fn make_session_with_config_and_rx(
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -5191,7 +5196,7 @@ async fn make_session_with_config_and_rx(
agent_status_tx, agent_status_tx,
InitialHistory::New, InitialHistory::New,
SessionSource::Exec, SessionSource::Exec,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
@@ -5277,7 +5282,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -5295,7 +5300,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
agent_status_tx, agent_status_tx,
initial_history, initial_history,
session_source, session_source,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
@@ -7006,7 +7011,7 @@ where
); );
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -7045,7 +7050,7 @@ where
guardian_rejections: Mutex::new(std::collections::HashMap::new()), guardian_rejections: Mutex::new(std::collections::HashMap::new()),
guardian_rejection_circuit_breaker: Mutex::new(Default::default()), guardian_rejection_circuit_breaker: Mutex::new(Default::default()),
runtime_handle: tokio::runtime::Handle::current(), runtime_handle: tokio::runtime::Handle::current(),
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()), extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
@@ -7090,12 +7095,10 @@ where
let skills_input = let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skill_fs = environment.get_filesystem(); let skill_fs = environment.get_filesystem();
let skills_outcome = Arc::new( let skills_snapshot = services
services .skills_service
.skills_manager .snapshot_for_config(&skills_input, Some(Arc::clone(&skill_fs)))
.skills_for_config(&skills_input, Some(Arc::clone(&skill_fs))) .await;
.await,
);
let turn_context = Arc::new(Session::make_turn_context( let turn_context = Arc::new(Session::make_turn_context(
thread_id, thread_id,
SessionId::from(thread_id), SessionId::from(thread_id),
@@ -7114,7 +7117,7 @@ where
resolved_turn_environments, resolved_turn_environments,
session_configuration.cwd().clone(), session_configuration.cwd().clone(),
"turn_id".to_string(), "turn_id".to_string(),
skills_outcome, skills_snapshot,
)); ));
let session = Arc::new(Session { let session = Arc::new(Session {
@@ -7781,7 +7784,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget()
}, },
]; ];
turn_context.model_info.context_window = Some(100); turn_context.model_info.context_window = Some(100);
turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome)));
let initial_context = session.build_initial_context(&turn_context).await; let initial_context = session.build_initial_context(&turn_context).await;
let developer_texts = developer_input_texts(&initial_context); let developer_texts = developer_input_texts(&initial_context);
@@ -7932,7 +7935,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
}, },
]; ];
turn_context.model_info.context_window = Some(100); turn_context.model_info.context_window = Some(100);
turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome)));
let _ = session.build_initial_context(&turn_context).await; let _ = session.build_initial_context(&turn_context).await;
let warning_event = timeout(Duration::from_secs(1), rx.recv()) let warning_event = timeout(Duration::from_secs(1), rx.recv())
@@ -694,7 +694,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
config.model_provider.clone(), config.model_provider.clone(),
); );
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
let skills_manager = Arc::new(SkillsManager::new( let skills_service = Arc::new(SkillsService::new(
config.codex_home.clone(), config.codex_home.clone(),
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
)); ));
@@ -711,7 +711,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
auth_manager, auth_manager,
models_manager, models_manager,
environment_manager: Arc::new(EnvironmentManager::default_for_tests()), environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions: codex_extension_api::empty_extension_registry(), extensions: codex_extension_api::empty_extension_registry(),
+1 -1
View File
@@ -528,7 +528,7 @@ async fn build_skills_and_plugins(
} else { } else {
Vec::new() Vec::new()
}; };
let skills_outcome = turn_context.turn_skills.outcome.as_ref(); let skills_outcome = turn_context.turn_skills.snapshot.outcome();
let connector_slug_counts = build_connector_slug_counts(&available_connectors); let connector_slug_counts = build_connector_slug_counts(&available_connectors);
let extension_injection_items = let extension_injection_items =
build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token) build_extension_turn_input_items(sess, turn_context, &user_input, cancellation_token)
+13 -15
View File
@@ -1,9 +1,8 @@
use super::*; use super::*;
use crate::SkillLoadOutcome;
use crate::agents_md::LoadedAgentsMd; use crate::agents_md::LoadedAgentsMd;
use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::shell_snapshot::ShellSnapshotFile; use crate::shell_snapshot::ShellSnapshotFile;
use codex_core_skills::HostLoadedSkills; use codex_core_skills::HostSkillsSnapshot;
use codex_file_system::FileSystemSandboxContext; use codex_file_system::FileSystemSandboxContext;
use codex_model_provider::SharedModelProvider; use codex_model_provider::SharedModelProvider;
use codex_model_provider::create_model_provider; use codex_model_provider::create_model_provider;
@@ -26,14 +25,14 @@ use tracing::instrument;
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub(crate) struct TurnSkillsContext { pub(crate) struct TurnSkillsContext {
pub(crate) outcome: Arc<SkillLoadOutcome>, pub(crate) snapshot: HostSkillsSnapshot,
pub(crate) implicit_invocation_seen_skills: Arc<Mutex<HashSet<String>>>, pub(crate) implicit_invocation_seen_skills: Arc<Mutex<HashSet<String>>>,
} }
impl TurnSkillsContext { impl TurnSkillsContext {
pub(crate) fn new(outcome: Arc<SkillLoadOutcome>) -> Self { pub(crate) fn new(snapshot: HostSkillsSnapshot) -> Self {
Self { Self {
outcome, snapshot,
implicit_invocation_seen_skills: Arc::new(Mutex::new(HashSet::new())), implicit_invocation_seen_skills: Arc::new(Mutex::new(HashSet::new())),
} }
} }
@@ -488,7 +487,7 @@ impl Session {
environments: TurnEnvironmentSnapshot, environments: TurnEnvironmentSnapshot,
cwd: AbsolutePathBuf, cwd: AbsolutePathBuf,
sub_id: String, sub_id: String,
skills_outcome: Arc<SkillLoadOutcome>, skills_snapshot: HostSkillsSnapshot,
) -> TurnContext { ) -> TurnContext {
let reasoning_effort = session_configuration.collaboration_mode.reasoning_effort(); let reasoning_effort = session_configuration.collaboration_mode.reasoning_effort();
let reasoning_summary = session_configuration let reasoning_summary = session_configuration
@@ -531,7 +530,7 @@ impl Session {
)); ));
let (current_date, timezone) = local_time_context(); let (current_date, timezone) = local_time_context();
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone())); let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone()));
extension_data.insert(HostLoadedSkills::new(Arc::clone(&skills_outcome))); extension_data.insert(skills_snapshot.clone());
TurnContext { TurnContext {
sub_id, sub_id,
trace_id: current_span_trace_id(), trace_id: current_span_trace_id(),
@@ -569,7 +568,7 @@ impl Session {
dynamic_tools: session_configuration.dynamic_tools.clone(), dynamic_tools: session_configuration.dynamic_tools.clone(),
turn_metadata_state, turn_metadata_state,
extension_data, extension_data,
turn_skills: TurnSkillsContext::new(skills_outcome), turn_skills: TurnSkillsContext::new(skills_snapshot),
turn_timing_state: Arc::new(TurnTimingState::default()), turn_timing_state: Arc::new(TurnTimingState::default()),
terminal_error: Arc::new(Mutex::new(None)), terminal_error: Arc::new(Mutex::new(None)),
server_model_warning_emitted: AtomicBool::new(false), server_model_warning_emitted: AtomicBool::new(false),
@@ -726,12 +725,11 @@ impl Session {
let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let fs = primary_turn_environment let fs = primary_turn_environment
.map(|turn_environment| turn_environment.environment.get_filesystem()); .map(|turn_environment| turn_environment.environment.get_filesystem());
let skills_outcome = Arc::new( let skills_snapshot = self
self.services .services
.skills_manager .skills_service
.skills_for_config(&skills_input, fs) .snapshot_for_config(&skills_input, fs)
.await, .await;
);
let mut turn_context: TurnContext = Self::make_turn_context( let mut turn_context: TurnContext = Self::make_turn_context(
self.thread_id(), self.thread_id(),
self.session_id(), self.session_id(),
@@ -759,7 +757,7 @@ impl Session {
turn_environments, turn_environments,
cwd, cwd,
sub_id, sub_id,
skills_outcome, skills_snapshot,
); );
turn_context.realtime_active = self.conversation.running_state().await.is_some(); turn_context.realtime_active = self.conversation.running_state().await.is_some();
+3 -3
View File
@@ -14,7 +14,7 @@ pub use codex_core_skills::SkillMetadata;
pub use codex_core_skills::SkillPolicy; pub use codex_core_skills::SkillPolicy;
pub use codex_core_skills::SkillRenderReport; pub use codex_core_skills::SkillRenderReport;
pub use codex_core_skills::SkillsLoadInput; pub use codex_core_skills::SkillsLoadInput;
pub use codex_core_skills::SkillsManager; pub use codex_core_skills::SkillsService;
pub use codex_core_skills::build_available_skills; pub use codex_core_skills::build_available_skills;
pub use codex_core_skills::build_skill_name_counts; pub use codex_core_skills::build_skill_name_counts;
pub use codex_core_skills::config_rules; pub use codex_core_skills::config_rules;
@@ -26,11 +26,11 @@ pub use codex_core_skills::injection::SkillInjections;
pub use codex_core_skills::injection::build_skill_injections; pub use codex_core_skills::injection::build_skill_injections;
pub use codex_core_skills::injection::collect_explicit_skill_mentions; pub use codex_core_skills::injection::collect_explicit_skill_mentions;
pub use codex_core_skills::loader; pub use codex_core_skills::loader;
pub use codex_core_skills::manager;
pub use codex_core_skills::model; pub use codex_core_skills::model;
pub use codex_core_skills::remote; pub use codex_core_skills::remote;
pub use codex_core_skills::render; pub use codex_core_skills::render;
pub use codex_core_skills::render::SkillRenderSideEffects; pub use codex_core_skills::render::SkillRenderSideEffects;
pub use codex_core_skills::service;
pub use codex_core_skills::system; pub use codex_core_skills::system;
pub(crate) fn skills_load_input_from_config( pub(crate) fn skills_load_input_from_config(
@@ -52,7 +52,7 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation(
workdir: &AbsolutePathBuf, workdir: &AbsolutePathBuf,
) { ) {
let Some(candidate) = detect_implicit_skill_invocation_for_command( let Some(candidate) = detect_implicit_skill_invocation_for_command(
turn_context.turn_skills.outcome.as_ref(), turn_context.turn_skills.snapshot.outcome(),
command, command,
workdir, workdir,
) else { ) else {
+2 -2
View File
@@ -1,7 +1,7 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
use crate::SkillsManager; use crate::SkillsService;
use crate::agent::AgentControl; use crate::agent::AgentControl;
use crate::attestation::AttestationProvider; use crate::attestation::AttestationProvider;
use crate::client::ModelClient; use crate::client::ModelClient;
@@ -61,7 +61,7 @@ pub(crate) struct SessionServices {
pub(crate) guardian_rejections: Mutex<HashMap<String, GuardianRejection>>, pub(crate) guardian_rejections: Mutex<HashMap<String, GuardianRejection>>,
pub(crate) guardian_rejection_circuit_breaker: Mutex<GuardianRejectionCircuitBreaker>, pub(crate) guardian_rejection_circuit_breaker: Mutex<GuardianRejectionCircuitBreaker>,
pub(crate) runtime_handle: Handle, pub(crate) runtime_handle: Handle,
pub(crate) skills_manager: Arc<SkillsManager>, pub(crate) skills_service: Arc<SkillsService>,
pub(crate) plugins_manager: Arc<PluginsManager>, pub(crate) plugins_manager: Arc<PluginsManager>,
pub(crate) mcp_manager: Arc<McpManager>, pub(crate) mcp_manager: Arc<McpManager>,
pub(crate) extensions: Arc<ExtensionRegistry<crate::config::Config>>, pub(crate) extensions: Arc<ExtensionRegistry<crate::config::Config>>,
+9 -9
View File
@@ -1,4 +1,4 @@
use crate::SkillsManager; use crate::SkillsService;
use crate::agent::AgentControl; use crate::agent::AgentControl;
use crate::attestation::AttestationProvider; use crate::attestation::AttestationProvider;
use crate::codex_thread::CodexThread; use crate::codex_thread::CodexThread;
@@ -207,7 +207,7 @@ pub(crate) struct ThreadManagerState {
auth_manager: Arc<AuthManager>, auth_manager: Arc<AuthManager>,
models_manager: SharedModelsManager, models_manager: SharedModelsManager,
environment_manager: Arc<EnvironmentManager>, environment_manager: Arc<EnvironmentManager>,
skills_manager: Arc<SkillsManager>, skills_service: Arc<SkillsService>,
plugins_manager: Arc<PluginsManager>, plugins_manager: Arc<PluginsManager>,
mcp_manager: Arc<McpManager>, mcp_manager: Arc<McpManager>,
extensions: Arc<ExtensionRegistry<Config>>, extensions: Arc<ExtensionRegistry<Config>>,
@@ -281,7 +281,7 @@ impl ThreadManager {
Arc::clone(&plugins_manager), Arc::clone(&plugins_manager),
Arc::clone(&extensions), Arc::clone(&extensions),
)); ));
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( let skills_service = Arc::new(SkillsService::new_with_restriction_product(
codex_home, codex_home,
config.bundled_skills_enabled(), config.bundled_skills_enabled(),
restriction_product, restriction_product,
@@ -292,7 +292,7 @@ impl ThreadManager {
thread_created_tx, thread_created_tx,
models_manager: build_models_manager(config, auth_manager.clone()), models_manager: build_models_manager(config, auth_manager.clone()),
environment_manager, environment_manager,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions, extensions,
@@ -373,7 +373,7 @@ impl ThreadManager {
auth_manager.get_api_auth_mode(), auth_manager.get_api_auth_mode(),
)); ));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( let skills_service = Arc::new(SkillsService::new_with_restriction_product(
skills_codex_home, skills_codex_home,
/*bundled_skills_enabled*/ true, /*bundled_skills_enabled*/ true,
restriction_product, restriction_product,
@@ -395,7 +395,7 @@ impl ThreadManager {
models_manager: create_model_provider(provider, Some(auth_manager.clone())) models_manager: create_model_provider(provider, Some(auth_manager.clone()))
.models_manager(codex_home, /*config_model_catalog*/ None), .models_manager(codex_home, /*config_model_catalog*/ None),
environment_manager, environment_manager,
skills_manager, skills_service,
plugins_manager, plugins_manager,
mcp_manager, mcp_manager,
extensions: empty_extension_registry(), extensions: empty_extension_registry(),
@@ -424,8 +424,8 @@ impl ThreadManager {
self.state.auth_manager.clone() self.state.auth_manager.clone()
} }
pub fn skills_manager(&self) -> Arc<SkillsManager> { pub fn skills_service(&self) -> Arc<SkillsService> {
self.state.skills_manager.clone() self.state.skills_service.clone()
} }
pub fn plugins_manager(&self) -> Arc<PluginsManager> { pub fn plugins_manager(&self) -> Arc<PluginsManager> {
@@ -1406,7 +1406,7 @@ impl ThreadManagerState {
auth_manager, auth_manager,
models_manager: Arc::clone(&self.models_manager), models_manager: Arc::clone(&self.models_manager),
environment_manager: Arc::clone(&self.environment_manager), environment_manager: Arc::clone(&self.environment_manager),
skills_manager: Arc::clone(&self.skills_manager), skills_service: Arc::clone(&self.skills_service),
plugins_manager: Arc::clone(&self.plugins_manager), plugins_manager: Arc::clone(&self.plugins_manager),
mcp_manager: Arc::clone(&self.mcp_manager), mcp_manager: Arc::clone(&self.mcp_manager),
extensions: Arc::clone(&self.extensions), extensions: Arc::clone(&self.extensions),
+9 -14
View File
@@ -1,6 +1,6 @@
use std::sync::Arc; use std::sync::Arc;
use codex_core_skills::HostLoadedSkills; use codex_core_skills::HostSkillsSnapshot;
use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_extension_api::ConfigContributor; use codex_extension_api::ConfigContributor;
@@ -121,7 +121,7 @@ where
SkillListQuery { SkillListQuery {
turn_id: thread_store.level_id().to_string(), turn_id: thread_store.level_id().to_string(),
executor_roots: thread_state.selected_roots().to_vec(), executor_roots: thread_state.selected_roots().to_vec(),
host: None, host_snapshot: None,
include_host_skills: false, include_host_skills: false,
include_bundled_skills: config.bundled_skills_enabled, include_bundled_skills: config.bundled_skills_enabled,
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
@@ -184,11 +184,11 @@ where
}; };
let config = thread_state.config(); let config = thread_state.config();
let host_loaded_skills = turn_store.get::<HostLoadedSkills>(); let host_snapshot = turn_store.get::<HostSkillsSnapshot>();
let query = SkillListQuery { let query = SkillListQuery {
turn_id: input.turn_id.clone(), turn_id: input.turn_id.clone(),
executor_roots: thread_state.selected_roots().to_vec(), executor_roots: thread_state.selected_roots().to_vec(),
host: host_loaded_skills.clone(), host_snapshot: host_snapshot.clone(),
include_host_skills: true, include_host_skills: true,
include_bundled_skills: config.bundled_skills_enabled, include_bundled_skills: config.bundled_skills_enabled,
include_orchestrator_skills: thread_state.orchestrator_skills_enabled(), include_orchestrator_skills: thread_state.orchestrator_skills_enabled(),
@@ -217,12 +217,7 @@ where
let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default(); let mut injected_host_skill_prompts = InjectedHostSkillPrompts::default();
for entry in &selected_entries { for entry in &selected_entries {
match self match self
.read_main_prompt( .read_main_prompt(entry, host_snapshot.clone(), session_store, &thread_state)
entry,
host_loaded_skills.clone(),
session_store,
&thread_state,
)
.await .await
{ {
Ok(read_result) => { Ok(read_result) => {
@@ -259,12 +254,12 @@ where
} }
} }
if let Some(host_loaded_skills) = &host_loaded_skills { if let Some(host_snapshot) = &host_snapshot {
for entry in selected_entries for entry in selected_entries
.iter() .iter()
.filter(|entry| entry.authority.kind != SkillSourceKind::Host) .filter(|entry| entry.authority.kind != SkillSourceKind::Host)
{ {
for host_skill in host_loaded_skills for host_skill in host_snapshot
.outcome() .outcome()
.skills .skills
.iter() .iter()
@@ -319,7 +314,7 @@ impl<C> SkillsExtension<C> {
async fn read_main_prompt( async fn read_main_prompt(
&self, &self,
entry: &SkillCatalogEntry, entry: &SkillCatalogEntry,
host_loaded_skills: Option<Arc<HostLoadedSkills>>, host_snapshot: Option<Arc<HostSkillsSnapshot>>,
session_store: &ExtensionData, session_store: &ExtensionData,
thread_state: &SkillsThreadState, thread_state: &SkillsThreadState,
) -> Result<SkillReadResult, String> { ) -> Result<SkillReadResult, String> {
@@ -330,7 +325,7 @@ impl<C> SkillsExtension<C> {
authority: entry.authority.clone(), authority: entry.authority.clone(),
package: entry.id.clone(), package: entry.id.clone(),
resource: entry.main_prompt.clone(), resource: entry.main_prompt.clone(),
host: host_loaded_skills, host_snapshot,
mcp_resources: session_store.get::<McpResourceClient>(), mcp_resources: session_store.get::<McpResourceClient>(),
}, },
) )
+3 -3
View File
@@ -6,7 +6,7 @@ mod executor;
mod host; mod host;
mod orchestrator; mod orchestrator;
use codex_core_skills::HostLoadedSkills; use codex_core_skills::HostSkillsSnapshot;
use codex_mcp::McpResourceClient; use codex_mcp::McpResourceClient;
use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::capabilities::SelectedCapabilityRoot;
@@ -26,7 +26,7 @@ pub use orchestrator::OrchestratorSkillProvider;
pub struct SkillListQuery { pub struct SkillListQuery {
pub turn_id: String, pub turn_id: String,
pub executor_roots: Vec<SelectedCapabilityRoot>, pub executor_roots: Vec<SelectedCapabilityRoot>,
pub host: Option<Arc<HostLoadedSkills>>, pub host_snapshot: Option<Arc<HostSkillsSnapshot>>,
pub include_host_skills: bool, pub include_host_skills: bool,
pub include_bundled_skills: bool, pub include_bundled_skills: bool,
pub include_orchestrator_skills: bool, pub include_orchestrator_skills: bool,
@@ -38,7 +38,7 @@ pub struct SkillReadRequest {
pub authority: SkillAuthority, pub authority: SkillAuthority,
pub package: SkillPackageId, pub package: SkillPackageId,
pub resource: SkillResourceId, pub resource: SkillResourceId,
pub host: Option<Arc<HostLoadedSkills>>, pub host_snapshot: Option<Arc<HostSkillsSnapshot>>,
pub mcp_resources: Option<Arc<McpResourceClient>>, pub mcp_resources: Option<Arc<McpResourceClient>>,
} }
+15 -20
View File
@@ -18,12 +18,10 @@ use crate::provider::SkillSearchRequest;
const HOST_AUTHORITY_ID: &str = "host"; const HOST_AUTHORITY_ID: &str = "host";
/// Host-owned skill provider backed by the already-loaded turn skills. /// Host-owned skill provider backed by an immutable service snapshot.
/// ///
/// The provider intentionally does not reload or cache host skills. Core owns /// Discovery and caching belong to `SkillsService`; this provider only maps a
/// skill loading, including plugin roots, runtime extra roots, and the primary /// snapshot into the authority-aware catalog/read contract.
/// environment filesystem. This adapter only maps that loaded outcome into the
/// skills-extension catalog/read contract.
#[derive(Clone, Default)] #[derive(Clone, Default)]
pub struct HostSkillProvider; pub struct HostSkillProvider;
@@ -36,24 +34,24 @@ impl HostSkillProvider {
impl SkillProvider for HostSkillProvider { impl SkillProvider for HostSkillProvider {
fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> { fn list(&self, query: SkillListQuery) -> SkillProviderFuture<'_, SkillCatalog> {
Box::pin(async move { Box::pin(async move {
let Some(host_loaded_skills) = query.host else { let Some(host_snapshot) = query.host_snapshot else {
return Err(SkillProviderError::new( return Err(SkillProviderError::new(
"host skill provider requires loaded host skills", "host skill provider requires a host skills snapshot",
)); ));
}; };
Ok(catalog_from_outcome(host_loaded_skills.outcome())) Ok(catalog_from_outcome(host_snapshot.outcome()))
}) })
} }
fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> { fn read(&self, request: SkillReadRequest) -> SkillProviderFuture<'_, SkillReadResult> {
Box::pin(async move { Box::pin(async move {
let Some(host_loaded_skills) = request.host else { let Some(host_snapshot) = request.host_snapshot else {
return Err(SkillProviderError::new( return Err(SkillProviderError::new(
"host skill provider requires loaded host skills", "host skill provider requires a host skills snapshot",
)); ));
}; };
let Some(skill) = host_loaded_skills.outcome().skills.iter().find(|skill| { let Some(skill) = host_snapshot.outcome().skills.iter().find(|skill| {
let skill_path = skill.path_to_skills_md.to_string_lossy(); let skill_path = skill.path_to_skills_md.to_string_lossy();
skill_path == request.resource.as_str() skill_path == request.resource.as_str()
|| skill_path.replace('\\', "/") == request.resource.as_str() || skill_path.replace('\\', "/") == request.resource.as_str()
@@ -64,15 +62,12 @@ impl SkillProvider for HostSkillProvider {
))); )));
}; };
let contents = host_loaded_skills let contents = host_snapshot.read_skill_text(skill).await.map_err(|err| {
.read_skill_text(skill) SkillProviderError::new(format!(
.await "failed to read host skill resource {}: {err}",
.map_err(|err| { request.resource.as_str()
SkillProviderError::new(format!( ))
"failed to read host skill resource {}: {err}", })?;
request.resource.as_str()
))
})?;
Ok(SkillReadResult { Ok(SkillReadResult {
resource: request.resource, resource: request.resource,
+1 -1
View File
@@ -68,7 +68,7 @@ impl SkillToolContext {
self.providers.list_orchestrator_for_turn(SkillListQuery { self.providers.list_orchestrator_for_turn(SkillListQuery {
turn_id: turn_id.to_string(), turn_id: turn_id.to_string(),
executor_roots: Vec::new(), executor_roots: Vec::new(),
host: None, host_snapshot: None,
include_host_skills: false, include_host_skills: false,
include_bundled_skills: false, include_bundled_skills: false,
include_orchestrator_skills: true, include_orchestrator_skills: true,
+1 -1
View File
@@ -82,7 +82,7 @@ impl ToolExecutor<ToolCall> for ReadTool {
authority, authority,
package: SkillPackageId(args.package), package: SkillPackageId(args.package),
resource: requested_resource.clone(), resource: requested_resource.clone(),
host: None, host_snapshot: None,
mcp_resources: self.context.mcp_resources.clone(), mcp_resources: self.context.mcp_resources.clone(),
}, },
) )
@@ -3,7 +3,7 @@ use std::sync::Arc;
use std::sync::atomic::AtomicUsize; use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use codex_core_skills::HostLoadedSkills; use codex_core_skills::HostSkillsSnapshot;
use codex_core_skills::loader::SkillRoot; use codex_core_skills::loader::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots; use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::CopyOptions; use codex_exec_server::CopyOptions;
@@ -208,7 +208,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
skill.path_to_skills_md, skill.path_to_skills_md,
canonical_root.join("skill/SKILL.md") canonical_root.join("skill/SKILL.md")
); );
let loaded = HostLoadedSkills::new(Arc::new(outcome)); let loaded = HostSkillsSnapshot::new(Arc::new(outcome));
assert_eq!( assert_eq!(
loaded.read_skill_text(&skill).await.expect("skill body"), loaded.read_skill_text(&skill).await.expect("skill body"),
SKILL_CONTENTS SKILL_CONTENTS
@@ -242,7 +242,7 @@ async fn selected_root_id_distinguishes_identical_executor_paths() {
}, },
}) })
.collect(), .collect(),
host: None, host_snapshot: None,
include_host_skills: false, include_host_skills: false,
include_bundled_skills: true, include_bundled_skills: true,
include_orchestrator_skills: false, include_orchestrator_skills: false,
@@ -4,7 +4,7 @@ use std::sync::Mutex;
use std::sync::atomic::AtomicUsize; use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use codex_core_skills::HostLoadedSkills; use codex_core_skills::HostSkillsSnapshot;
use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
use codex_core_skills::SkillLoadOutcome; use codex_core_skills::SkillLoadOutcome;
@@ -52,7 +52,7 @@ const DEMO_SKILL_CONTENTS: &str =
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n"; "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n";
#[tokio::test] #[tokio::test]
async fn installed_extension_uses_host_loaded_skills() -> TestResult { async fn installed_extension_uses_host_service_snapshot() -> TestResult {
let codex_home = test_codex_home(); let codex_home = test_codex_home();
let skill_path = codex_home.join("skills").join("demo").join("SKILL.md"); let skill_path = codex_home.join("skills").join("demo").join("SKILL.md");
std::fs::create_dir_all( std::fs::create_dir_all(
@@ -97,7 +97,7 @@ async fn installed_extension_uses_host_loaded_skills() -> TestResult {
let loaded_skills = Arc::new(outcome); let loaded_skills = Arc::new(outcome);
let skill_prompt_path = skill_path_string.replace('\\', "/"); let skill_prompt_path = skill_path_string.replace('\\', "/");
let turn_store = ExtensionData::new("turn-1"); let turn_store = ExtensionData::new("turn-1");
turn_store.insert(HostLoadedSkills::new(Arc::clone(&loaded_skills))); turn_store.insert(HostSkillsSnapshot::new(Arc::clone(&loaded_skills)));
let fragments = registry.turn_input_contributors()[0] let fragments = registry.turn_input_contributors()[0]
.contribute( .contribute(