agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] Reuse parsed plugin skills during session startup (#28844)

Browse Source 
## Summary

- Preserve raw plugin skill-root snapshots in the matching loaded-plugin
cache entry, keyed by the effective plugin root identity including
namespace.
- Pass those snapshots through `SkillsLoadInput` as an optional preload,
so session startup reuses plugin parsing while ordinary skill loads pass
`None`.
- Keep plugin skill loading cohesive: the existing loaders accept the
optional snapshots directly, and uncached or marketplace-detail paths do
not create a cache.

## Why

Plugin discovery already parses plugin skills to determine available
capabilities. Cold session startup then scanned and parsed the same
roots again while building the skills snapshot.

This solves the same duplicate-work problem as #28623 while keeping
ownership narrow: `PluginsManager` creates and owns
`PluginSkillSnapshots` only for its loaded-plugin cache entry;
`SkillsService` consumes an optional clone. Entry replacement or
clearing naturally drops the snapshots, with no separate generation,
capacity policy, or watcher coupling.

## Validation

- `cargo clippy -p codex-core-skills --all-targets -- -D warnings`
- `just test -p codex-core-plugins
skills_service_reuses_skills_parsed_during_plugin_load`
- `just test -p codex-core-skills
namespaces_plugin_skills_using_provided_namespace`
- `just fmt`
This commit is contained in:
xl-openai
2026-06-18 16:45:58 -07:00
committed by GitHub
parent 346d2c163f
commit e83b7841b0
17 changed files with 527 additions and 211 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/loader.rs
+18 -4
View File
@@ -21,6 +21,7 @@ use codex_config::HooksFile;
use codex_config::types::McpServerConfig;
use codex_config::types::PluginConfig;
use codex_config::types::PluginMcpServerConfig;
use codex_core_skills::PluginSkillSnapshots;
use codex_core_skills::SkillMetadata;
use codex_core_skills::config_rules::SkillConfigRules;
use codex_core_skills::config_rules::resolve_disabled_skill_paths;
@@ -74,6 +75,7 @@ enum PluginLoadScope<'a> {
AllCapabilities {
restriction_product: Option<Product>,
skill_config_rules: &'a SkillConfigRules,
plugin_skill_snapshots: Option<&'a PluginSkillSnapshots>,
},
HooksOnly,
}
@@ -115,6 +117,7 @@ pub(crate) async fn load_plugins_from_layer_stack(
config_layer_stack: &ConfigLayerStack,
extra_plugins: HashMap<String, PluginConfig>,
store: &PluginStore,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
restriction_product: Option<Product>,
prefer_remote_curated_conflicts: bool,
) -> Vec<LoadedPlugin<McpServerConfig>> {
@@ -127,6 +130,7 @@ pub(crate) async fn load_plugins_from_layer_stack(
PluginLoadScope::AllCapabilities {
restriction_product,
skill_config_rules: &skill_config_rules,
plugin_skill_snapshots,
},
)
.await
@@ -745,6 +749,7 @@ async fn load_plugin(
PluginLoadScope::AllCapabilities {
restriction_product,
skill_config_rules,
plugin_skill_snapshots,
} => {
loaded_plugin.manifest_name = Some(manifest.display_name().to_string());
loaded_plugin.manifest_description = manifest.description.clone();
@@ -755,6 +760,7 @@ async fn load_plugin(
&manifest,
*restriction_product,
skill_config_rules,
*plugin_skill_snapshots,
)
.await;
let has_enabled_skills = resolved_skills.has_enabled_skills();
@@ -851,10 +857,17 @@ pub async fn load_plugin_skills(
manifest: &PluginManifest,
restriction_product: Option<Product>,
skill_config_rules: &SkillConfigRules,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
) -> ResolvedPluginSkills {
load_plugin_skill_inventory(plugin_root, plugin_id, manifest, restriction_product)
.await
.resolve(skill_config_rules)
load_plugin_skill_inventory(
plugin_root,
plugin_id,
manifest,
restriction_product,
plugin_skill_snapshots,
)
.await
.resolve(skill_config_rules)
}
pub(crate) async fn load_plugin_skill_inventory(
@@ -862,6 +875,7 @@ pub(crate) async fn load_plugin_skill_inventory(
plugin_id: &PluginId,
manifest: &PluginManifest,
restriction_product: Option<Product>,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
) -> PluginSkillInventory {
let roots = plugin_skill_roots(plugin_root, &manifest.paths)
.into_iter()
@@ -874,7 +888,7 @@ pub(crate) async fn load_plugin_skill_inventory(
plugin_root: Some(plugin_root.clone()),
})
.collect::<Vec<_>>();
let outcome = load_skills_from_roots(roots).await;
let outcome = load_skills_from_roots(roots, plugin_skill_snapshots).await;
let had_errors = !outcome.errors.is_empty();
let skills = outcome
.skills
codex-rs/core-plugins/src/loader_tests.rs
+1
View File
@@ -160,6 +160,7 @@ enabled = true
&stack,
HashMap::new(),
&store,
/*plugin_skill_snapshots*/ None,
Some(Product::Codex),
/*prefer_remote_curated_conflicts*/ false,
)
codex-rs/core-plugins/src/manager.rs
+54 -20
View File
@@ -61,6 +61,7 @@ use codex_config::set_user_plugin_enabled;
use codex_config::types::PluginConfig;
use codex_config::types::ToolSuggestDisabledTool;
use codex_config::types::ToolSuggestDiscoverableType;
use codex_core_skills::PluginSkillSnapshots;
use codex_core_skills::SkillMetadata;
use codex_core_skills::config_rules::SkillConfigRules;
use codex_core_skills::config_rules::skill_config_rules_from_stack;
@@ -370,6 +371,7 @@ pub struct PluginsManager {
struct LoadedPluginsCacheEntry {
key: PluginLoadCacheKey,
plugins: Vec<LoadedPlugin>,
plugin_skill_snapshots: PluginSkillSnapshots,
}
#[derive(Default)]
@@ -385,6 +387,16 @@ struct PluginLoadCacheKey {
remote_plugin_enabled: bool,
}
impl PluginLoadCacheKey {
fn from_config(config: &PluginsConfigInput) -> Self {
Self {
configured_plugins: configured_plugins_from_stack(&config.config_layer_stack),
skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack),
remote_plugin_enabled: config.remote_plugin_enabled,
}
}
}
impl PluginsManager {
pub fn new(codex_home: PathBuf) -> Self {
Self::new_with_options(codex_home, Some(Product::Codex), /*auth_mode*/ None)
@@ -470,6 +482,24 @@ impl PluginsManager {
.await
}
/// Returns skill snapshots parsed while loading the matching plugin cache entry.
pub fn plugin_skill_snapshots_for_config(
&self,
config: &PluginsConfigInput,
) -> Option<PluginSkillSnapshots> {
if !config.plugins_enabled {
return None;
}
let key = PluginLoadCacheKey::from_config(config);
self.loaded_plugins_cache
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.entry
.as_ref()
.filter(|cached| cached.key == key)
.map(|cached| cached.plugin_skill_snapshots.clone())
}
#[instrument(
name = "plugins_for_config",
level = "info",
@@ -489,11 +519,7 @@ impl PluginsManager {
return PluginLoadOutcome::default();
}
let cache_key = PluginLoadCacheKey {
configured_plugins: configured_plugins_from_stack(&config.config_layer_stack),
skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack),
remote_plugin_enabled: config.remote_plugin_enabled,
};
let cache_key = PluginLoadCacheKey::from_config(config);
if !force_reload && let Some(plugins) = self.cached_loaded_plugins(&cache_key) {
return self.resolve_loaded_plugins_for_auth(plugins);
}
@@ -506,16 +532,23 @@ impl PluginsManager {
return self.resolve_loaded_plugins_for_auth(plugins);
}
let cache_generation = self.loaded_plugins_cache_generation();
let plugin_skill_snapshots = PluginSkillSnapshots::for_plugin_load();
let plugins = load_plugins_from_layer_stack(
&config.config_layer_stack,
self.remote_installed_plugin_configs(),
&self.store,
Some(&plugin_skill_snapshots),
self.restriction_product,
config.remote_plugin_enabled,
)
.await;
log_plugin_load_errors(&plugins);
self.cache_loaded_plugins_if_current(cache_generation, cache_key, plugins.clone());
self.cache_loaded_plugins_if_current(
cache_generation,
cache_key,
plugins.clone(),
plugin_skill_snapshots,
);
self.resolve_loaded_plugins_for_auth(plugins)
}
@@ -589,6 +622,7 @@ impl PluginsManager {
config_layer_stack,
self.remote_installed_plugin_configs(),
&self.store,
/*plugin_skill_snapshots*/ None,
self.restriction_product,
config.remote_plugin_enabled,
)
@@ -626,19 +660,13 @@ impl PluginsManager {
}
fn cached_loaded_plugins(&self, key: &PluginLoadCacheKey) -> Option<Vec<LoadedPlugin>> {
match self.loaded_plugins_cache.read() {
Ok(cache) => cache
.entry
.as_ref()
.filter(|cached| cached.key == *key)
.map(|cached| cached.plugins.clone()),
Err(err) => err
.into_inner()
.entry
.as_ref()
.filter(|cached| cached.key == *key)
.map(|cached| cached.plugins.clone()),
}
self.loaded_plugins_cache
.read()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.entry
.as_ref()
.filter(|cached| cached.key == *key)
.map(|cached| cached.plugins.clone())
}
fn loaded_plugins_cache_generation(&self) -> u64 {
@@ -653,13 +681,18 @@ impl PluginsManager {
generation: u64,
key: PluginLoadCacheKey,
plugins: Vec<LoadedPlugin>,
plugin_skill_snapshots: PluginSkillSnapshots,
) {
let mut cache = match self.loaded_plugins_cache.write() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
if cache.generation == generation {
cache.entry = Some(LoadedPluginsCacheEntry { key, plugins });
cache.entry = Some(LoadedPluginsCacheEntry {
key,
plugins,
plugin_skill_snapshots,
});
}
}
@@ -1659,6 +1692,7 @@ impl PluginsManager {
&codex_core_skills::config_rules::skill_config_rules_from_stack(
&config.config_layer_stack,
),
/*plugin_skill_snapshots*/ None,
)
.await;
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
codex-rs/core-plugins/src/manager_tests.rs
+60 -1
View File
@@ -37,6 +37,9 @@ use codex_config::McpServerConfig;
use codex_config::McpServerOAuthConfig;
use codex_config::McpServerToolConfig;
use codex_config::types::McpServerTransportConfig;
use codex_core_skills::PluginSkillSnapshots;
use codex_core_skills::SkillsLoadInput;
use codex_core_skills::SkillsService;
use codex_core_skills::config_rules::SkillConfigRules;
use codex_login::CodexAuth;
use codex_plugin::AppDeclaration;
@@ -1476,6 +1479,7 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() {
&manifest,
/*restriction_product*/ None,
&SkillConfigRules::default(),
/*plugin_skill_snapshots*/ None,
)
.await;
@@ -2039,6 +2043,55 @@ async fn plugin_cache_ignores_unrelated_session_overrides() {
assert_eq!(second.plugins()[0].mcp_servers.len(), 1);
}
#[tokio::test]
async fn skills_service_reuses_skills_parsed_during_plugin_load() {
let codex_home = TempDir::new().unwrap();
let codex_home_abs = codex_home.path().to_path_buf().abs();
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_plugin(
codex_home.path().join("plugins/cache/test").as_path(),
"sample/local",
"sample",
);
let skill_path = plugin_root.join("skills/SKILL.md");
write_file(&skill_path, "---\nname: search\ndescription: first\n---\n");
write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
&plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true),
);
let config = load_config(codex_home.path(), codex_home.path()).await;
let manager = PluginsManager::new(codex_home.path().to_path_buf());
let plugin_outcome = manager.plugins_for_config(&config).await;
let plugin_skill_snapshots = manager.plugin_skill_snapshots_for_config(&config);
write_file(&skill_path, "---\nname: search\ndescription: second\n---\n");
let skills_input = SkillsLoadInput::new(
codex_home_abs.clone(),
plugin_outcome.effective_plugin_skill_roots(),
config.config_layer_stack.clone(),
/*bundled_skills_enabled*/ false,
)
.with_plugin_skill_snapshots(plugin_skill_snapshots);
let skills_service = SkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false);
let cached = skills_service
.snapshot_for_config(&skills_input, /*fs*/ None)
.await;
assert_eq!(
cached
.outcome()
.skills
.iter()
.map(|skill| skill.description.as_str())
.collect::<Vec<_>>(),
vec!["first"]
);
}
#[test]
fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() {
let codex_home = TempDir::new().unwrap();
@@ -2051,7 +2104,12 @@ fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() {
let stale_generation = manager.loaded_plugins_cache_generation();
manager.clear_loaded_plugins_cache();
manager.cache_loaded_plugins_if_current(stale_generation, cache_key.clone(), Vec::new());
manager.cache_loaded_plugins_if_current(
stale_generation,
cache_key.clone(),
Vec::new(),
PluginSkillSnapshots::for_plugin_load(),
);
assert_eq!(manager.cached_loaded_plugins(&cache_key), None);
}
@@ -5164,6 +5222,7 @@ async fn load_plugins_ignores_project_config_files() {
&stack,
std::collections::HashMap::new(),
&PluginStore::new(codex_home.path().to_path_buf()),
/*plugin_skill_snapshots*/ None,
Some(Product::Codex),
/*prefer_remote_curated_conflicts*/ false,
)
codex-rs/core-plugins/src/tool_suggest_metadata.rs
+8 -2
View File
@@ -210,8 +210,14 @@ async fn load_plugin_metadata(
}
let manifest = load_plugin_manifest(plugin_root.as_path())
.ok_or_else(|| "missing or invalid plugin.json".to_string())?;
let skill_inventory =
load_plugin_skill_inventory(plugin_root, &plugin_id, &manifest, restriction_product).await;
let skill_inventory = load_plugin_skill_inventory(
plugin_root,
&plugin_id,
&manifest,
restriction_product,
/*plugin_skill_snapshots*/ None,
)
.await;
let mut mcp_server_names =
load_plugin_mcp_servers(plugin_root.as_path(), /*auth_mode*/ None)
.await