Propagate remote plugin IDs to skill metadata (#35261)

## What changed

- Carry a plugin's local and remote identities together from plugin loading into
  `SkillMetadata`.
- Resolve remote IDs from the installed-plugin snapshot when available, falling
  back to persisted install metadata only when no snapshot exists.
- Include plugin identity in skill cache keys so identity changes refresh cached
  skill metadata.

## Testing

- Cover snapshot and persisted identity resolution, local marketplace isolation,
  cached skill refreshes, and propagation through plugin skill snapshots.

GitOrigin-RevId: aabeeb631a43361fe817358ace7f1ea8ba5db708
This commit is contained in:
jameswt-oai
2026-07-24 23:36:11 +00:00
committed by copyberry
parent 32329b289d
commit 07fd04abb1
28 changed files with 550 additions and 170 deletions
+1 -1
View File
@@ -115,7 +115,7 @@ impl SkillsWatcher {
.await
.into_iter()
// Plugin roots are invalidated by plugin lifecycle operations.
.filter(|root| root.plugin_id.is_none())
.filter(|root| root.plugin_identity.is_none())
.map(|root| WatchPath {
path: root.path.into_path_buf(),
recursive: true,
+1
View File
@@ -17,6 +17,7 @@ mod provider;
pub mod remote;
pub mod remote_bundle;
pub mod remote_legacy;
mod remote_plugin_id_resolver;
mod script_attribution;
pub mod startup_sync;
pub mod store;
+64 -13
View File
@@ -15,6 +15,8 @@ use crate::marketplace_policy::configured_plugins_from_stack;
use crate::npm_source::materialize_npm_plugin_source;
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
use crate::remote::RemoteInstalledPlugin;
use crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot;
use crate::remote_plugin_id_resolver::RemotePluginIdResolver;
use crate::store::PluginStore;
use crate::store::plugin_version_for_source;
use crate::store::plugin_version_for_source_with_fallback_manifest;
@@ -45,6 +47,7 @@ use codex_protocol::protocol::SkillScope;
use codex_skills::SkillConfigRules;
use codex_skills::SkillMetadata;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::SkillDiscoveryMode;
use codex_utils_plugins::find_plugin_manifest_path;
use serde_json::Value as JsonValue;
@@ -78,6 +81,7 @@ enum PluginLoadScope<'a> {
restriction_product: Option<Product>,
skill_config_rules: &'a SkillConfigRules,
plugin_skill_snapshots: Option<&'a PluginSkillSnapshots>,
remote_plugin_id_resolver: &'a RemotePluginIdResolver,
root_scan_slots: Arc<Semaphore>,
},
HooksOnly,
@@ -117,7 +121,7 @@ pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin<McpServerConfig>])
#[instrument(level = "trace", skip_all)]
pub(crate) async fn load_plugins_from_layer_stack(
config_layer_stack: &ConfigLayerStack,
extra_plugins: HashMap<String, PluginConfig>,
remote_installed_plugins_snapshot: RemoteInstalledPluginsSnapshot,
store: &PluginStore,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
restriction_product: Option<Product>,
@@ -125,6 +129,10 @@ pub(crate) async fn load_plugins_from_layer_stack(
root_scan_slots: Arc<Semaphore>,
) -> Vec<LoadedPlugin<McpServerConfig>> {
let skill_config_rules = skill_config_rules_from_stack(config_layer_stack);
let RemoteInstalledPluginsSnapshot {
configs: extra_plugins,
remote_plugin_id_resolver,
} = remote_installed_plugins_snapshot;
load_plugins_from_layer_stack_with_scope(
config_layer_stack,
extra_plugins,
@@ -134,6 +142,7 @@ pub(crate) async fn load_plugins_from_layer_stack(
restriction_product,
skill_config_rules: &skill_config_rules,
plugin_skill_snapshots,
remote_plugin_id_resolver: &remote_plugin_id_resolver,
root_scan_slots,
},
)
@@ -753,18 +762,20 @@ async fn load_plugin(
scope: &PluginLoadScope<'_>,
) -> LoadedPlugin<McpServerConfig> {
let plugin_id = PluginId::parse(&config_name);
let active_plugin_root = plugin_id
let active_plugin_installation = plugin_id
.as_ref()
.ok()
.and_then(|plugin_id| store.active_plugin_root(plugin_id));
let root = active_plugin_root
.clone()
.and_then(|plugin_id| store.active_plugin_installation(plugin_id));
let root = active_plugin_installation
.as_ref()
.map(|installation| installation.root.clone())
.unwrap_or_else(|| match &plugin_id {
Ok(plugin_id) => store.plugin_base_root(plugin_id),
Err(_) => store.root().clone(),
});
let mut loaded_plugin = LoadedPlugin {
config_name,
remote_plugin_id: None,
manifest_name: None,
plugin_namespace: None,
manifest_description: None,
@@ -784,13 +795,13 @@ async fn load_plugin(
return loaded_plugin;
}
let (loaded_plugin_id, plugin_root) = match plugin_id {
let (loaded_plugin_id, installation) = match plugin_id {
Ok(plugin_id) => {
let Some(plugin_root) = active_plugin_root else {
let Some(installation) = active_plugin_installation else {
loaded_plugin.error = Some("plugin is not installed".to_string());
return loaded_plugin;
};
(plugin_id, plugin_root)
(plugin_id, installation)
}
Err(err) => {
loaded_plugin.error = Some(err.to_string());
@@ -798,6 +809,16 @@ async fn load_plugin(
}
};
loaded_plugin.remote_plugin_id = match scope {
PluginLoadScope::AllCapabilities {
remote_plugin_id_resolver,
..
} => remote_plugin_id_resolver.remote_plugin_id_for_installation(&installation),
PluginLoadScope::HooksOnly => None,
};
let plugin_root = installation.root;
if !plugin_root.as_path().is_dir() {
loaded_plugin.error = Some("path does not exist or is not a directory".to_string());
return loaded_plugin;
@@ -815,14 +836,19 @@ async fn load_plugin(
restriction_product,
skill_config_rules,
plugin_skill_snapshots,
remote_plugin_id_resolver: _,
root_scan_slots,
} => {
loaded_plugin.manifest_name = Some(manifest.display_name().to_string());
loaded_plugin.manifest_description = manifest.description.clone();
loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths);
let resolved_skills = load_plugin_skills(
let plugin_identity = PluginIdentity {
plugin_id: loaded_plugin_id.as_key(),
remote_plugin_id: loaded_plugin.remote_plugin_id.clone(),
};
let resolved_skills = load_plugin_skills_with_identity(
&plugin_root,
&loaded_plugin_id,
&plugin_identity,
&manifest,
*restriction_product,
skill_config_rules,
@@ -926,10 +952,35 @@ pub async fn load_plugin_skills(
skill_config_rules: &SkillConfigRules,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
root_scan_slots: Arc<Semaphore>,
) -> ResolvedPluginSkills {
let plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
remote_plugin_id: None,
};
load_plugin_skills_with_identity(
plugin_root,
&plugin_identity,
manifest,
restriction_product,
skill_config_rules,
plugin_skill_snapshots,
root_scan_slots,
)
.await
}
pub(crate) async fn load_plugin_skills_with_identity(
plugin_root: &AbsolutePathBuf,
plugin_identity: &PluginIdentity,
manifest: &PluginManifest,
restriction_product: Option<Product>,
skill_config_rules: &SkillConfigRules,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
root_scan_slots: Arc<Semaphore>,
) -> ResolvedPluginSkills {
load_plugin_skill_inventory(
plugin_root,
plugin_id,
plugin_identity,
manifest,
restriction_product,
plugin_skill_snapshots,
@@ -941,7 +992,7 @@ pub async fn load_plugin_skills(
pub(crate) async fn load_plugin_skill_inventory(
plugin_root: &AbsolutePathBuf,
plugin_id: &PluginId,
plugin_identity: &PluginIdentity,
manifest: &PluginManifest,
restriction_product: Option<Product>,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
@@ -953,7 +1004,7 @@ pub(crate) async fn load_plugin_skill_inventory(
path,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some(plugin_id.as_key()),
plugin_identity: Some(plugin_identity.clone()),
plugin_namespace: Some(manifest.name.clone()),
plugin_root: Some(plugin_root.clone()),
discovery_mode: SkillDiscoveryMode::Recursive,
+1 -1
View File
@@ -159,7 +159,7 @@ enabled = true
let full = load_plugins_from_layer_stack(
&stack,
HashMap::new(),
RemoteInstalledPluginsSnapshot::default(),
&store,
/*plugin_skill_snapshots*/ None,
Some(Product::Codex),
+41 -31
View File
@@ -10,7 +10,7 @@ use crate::loader::load_plugin_apps_from_manifest;
use crate::loader::load_plugin_hooks;
use crate::loader::load_plugin_hooks_from_layer_stack;
use crate::loader::load_plugin_mcp_servers_from_manifest;
use crate::loader::load_plugin_skills;
use crate::loader::load_plugin_skills_with_identity;
use crate::loader::load_plugins_from_layer_stack;
use crate::loader::log_plugin_load_errors;
use crate::loader::materialize_marketplace_plugin_source;
@@ -51,6 +51,9 @@ use crate::remote::RemotePluginScope;
use crate::remote::RemotePluginServiceConfig;
use crate::remote_legacy::RemotePluginFetchError;
use crate::remote_legacy::RemotePluginMutationError;
use crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot;
use crate::remote_plugin_id_resolver::RemotePluginIdResolver;
use crate::remote_plugin_id_resolver::persisted_remote_plugin_id_for_installation;
use crate::startup_sync::curated_plugins_api_marketplace_path;
use crate::startup_sync::curated_plugins_repo_path;
use crate::startup_sync::read_curated_plugins_sha;
@@ -91,6 +94,7 @@ use codex_tools::DiscoverablePluginInfo;
use codex_tools::DiscoverableTool;
use codex_tools::filter_request_plugin_install_discoverable_tools_for_client;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::PluginSkillRoot;
use std::collections::BTreeSet;
use std::collections::HashMap;
@@ -643,7 +647,7 @@ impl PluginsManager {
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.remote_installed_plugins_snapshot(),
&self.store,
Some(&plugin_skill_snapshots),
self.restriction_product,
@@ -729,7 +733,7 @@ impl PluginsManager {
}
let plugins = load_plugins_from_layer_stack(
config_layer_stack,
self.remote_installed_plugin_configs(),
self.remote_installed_plugins_snapshot(),
&self.store,
/*plugin_skill_snapshots*/ None,
self.restriction_product,
@@ -818,37 +822,39 @@ impl PluginsManager {
remote_installed_plugins_to_config(plugins, &self.store)
}
fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option<String> {
let cached_remote_plugin_id = {
let cache = match self.remote_installed_plugins_cache.read() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
cache.as_ref().and_then(|plugins| {
plugins.iter().find_map(|plugin| {
(plugin.name == plugin_id.plugin_name
&& plugin.marketplace_name == plugin_id.marketplace_name)
.then(|| plugin.id.clone())
})
})
fn remote_installed_plugins_snapshot(&self) -> RemoteInstalledPluginsSnapshot {
let cache = match self.remote_installed_plugins_cache.read() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
let Some(plugins) = cache.as_ref() else {
return RemoteInstalledPluginsSnapshot::default();
};
if cached_remote_plugin_id.is_some() {
return cached_remote_plugin_id;
}
match self.store.remote_plugin_id(plugin_id) {
Ok(remote_plugin_id) => remote_plugin_id,
Err(err) => {
tracing::warn!(
plugin_id = %plugin_id.as_key(),
error = %err,
"failed to read persisted remote plugin identity"
);
None
}
RemoteInstalledPluginsSnapshot {
configs: remote_installed_plugins_to_config(plugins, &self.store),
remote_plugin_id_resolver: RemotePluginIdResolver::new(plugins),
}
}
fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option<String> {
let cache = match self.remote_installed_plugins_cache.read() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
if let Some(plugins) = cache.as_ref() {
return plugins.iter().find_map(|plugin| {
(plugin.name == plugin_id.plugin_name
&& plugin.marketplace_name == plugin_id.marketplace_name)
.then(|| plugin.id.clone())
});
}
drop(cache);
let installation = self.store.active_plugin_installation(plugin_id)?;
persisted_remote_plugin_id_for_installation(&installation)
}
pub async fn telemetry_metadata_for_installed_plugin(
&self,
plugin_id: &PluginId,
@@ -1958,9 +1964,13 @@ impl PluginsManager {
manifest.interface.clone(),
marketplace_category,
);
let resolved_skills = load_plugin_skills(
let plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
remote_plugin_id: self.remote_plugin_id_for(&plugin_id),
};
let resolved_skills = load_plugin_skills_with_identity(
&source_path,
&plugin_id,
&plugin_identity,
&manifest,
self.restriction_product,
&codex_core_skills::config_rules::skill_config_rules_from_stack(
+113 -44
View File
@@ -861,6 +861,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
outcome.plugins(),
vec![LoadedPlugin {
config_name: "sample@test".to_string(),
remote_plugin_id: None,
manifest_name: Some("sample".to_string()),
plugin_namespace: Some("sample".to_string()),
manifest_description: Some(
@@ -1139,6 +1140,26 @@ async fn installed_plugin_telemetry_metadata_resolves_persisted_remote_identity(
);
}
#[test]
fn plugin_telemetry_ignores_local_marketplace_sidecars() {
let codex_home = TempDir::new().unwrap();
write_cached_plugin(codex_home.path(), "test", "sample");
let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse");
PluginStore::new(codex_home.path().to_path_buf())
.write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample")
.expect("persist remote plugin id");
let manager = PluginsManager::new(codex_home.path().to_path_buf());
assert_eq!(
manager.telemetry_metadata_for_plugin_id(&plugin_id),
PluginTelemetryMetadata {
plugin_id: Some(plugin_id),
remote_plugin_id: None,
capability_summary: None,
}
);
}
#[tokio::test]
async fn installed_plugin_telemetry_metadata_prefers_remote_snapshot_identity() {
let codex_home = TempDir::new().unwrap();
@@ -2292,6 +2313,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions
outcome.plugins(),
vec![LoadedPlugin {
config_name: "sample@test".to_string(),
remote_plugin_id: None,
manifest_name: None,
plugin_namespace: None,
manifest_description: None,
@@ -2462,6 +2484,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
};
let plugin = |config_name: &str, dir_name: &str, manifest_name: &str| LoadedPlugin {
config_name: config_name.to_string(),
remote_plugin_id: None,
manifest_name: Some(manifest_name.to_string()),
plugin_namespace: Some(
config_name
@@ -2648,52 +2671,98 @@ async fn plugin_cache_ignores_unrelated_session_overrides() {
}
#[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),
);
async fn skill_snapshots_resolve_remote_plugin_identity_from_authoritative_source() {
let mut duplicate_plugin = remote_installed_plugin("sample");
duplicate_plugin.id = "plugins~Plugin_duplicate".to_string();
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");
for (installed_plugins, expected_remote_plugin_id) in [
(None, Some("plugins~Plugin_persisted")),
(Some(Vec::new()), None),
(
Some(vec![remote_installed_plugin("sample"), duplicate_plugin]),
Some("plugins~Plugin_sample"),
),
] {
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/openai-curated-remote/sample/local");
write_plugin(
codex_home
.path()
.join("plugins/cache/openai-curated-remote")
.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),
r#"[features]
plugins = true
remote_plugin = true
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;
[plugins."sample@openai-curated-remote"]
enabled = true
"#,
);
assert_eq!(
cached
.outcome()
.skills
.iter()
.map(|skill| skill.description.as_str())
.collect::<Vec<_>>(),
vec!["first"]
);
let plugin_id =
PluginId::parse("sample@openai-curated-remote").expect("remote plugin id should parse");
PluginStore::new(codex_home.path().to_path_buf())
.write_remote_plugin_id(&plugin_id, "plugins~Plugin_persisted")
.expect("persist remote plugin id");
let config = load_config(codex_home.path(), codex_home.path()).await;
let manager = PluginsManager::new(codex_home.path().to_path_buf());
if let Some(installed_plugins) = installed_plugins {
manager.write_remote_installed_plugins_cache(installed_plugins);
}
let plugin_outcome = manager.plugins_for_config(&config).await;
assert_eq!(
manager
.telemetry_metadata_for_plugin_id(&plugin_id)
.remote_plugin_id
.as_deref(),
expected_remote_plugin_id
);
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(manager.plugin_skill_snapshots_for_config(&config));
let skills_service =
SkillsService::new(codex_home_abs, /*bundled_skills_enabled*/ false);
let snapshot = skills_service
.snapshot_for_config(&skills_input, /*fs*/ None)
.await;
assert_eq!(
snapshot
.outcome()
.skills
.iter()
.map(|skill| {
(
skill.description.as_str(),
skill.plugin_id.as_deref(),
skill.remote_plugin_id.as_deref(),
)
})
.collect::<Vec<_>>(),
vec![(
"first",
Some("sample@openai-curated-remote"),
expected_remote_plugin_id,
)]
);
}
}
#[test]
@@ -6030,7 +6099,7 @@ async fn load_plugins_ignores_project_config_files() {
let plugins = load_plugins_from_layer_stack(
&stack,
std::collections::HashMap::new(),
crate::remote_plugin_id_resolver::RemoteInstalledPluginsSnapshot::default(),
&PluginStore::new(codex_home.path().to_path_buf()),
/*plugin_skill_snapshots*/ None,
Some(Product::Codex),
+1 -1
View File
@@ -482,7 +482,7 @@ impl RemotePluginScope {
}
}
fn from_marketplace_name(name: &str) -> Option<Self> {
pub(crate) fn from_marketplace_name(name: &str) -> Option<Self> {
match name {
REMOTE_GLOBAL_MARKETPLACE_NAME => Some(Self::Global),
REMOTE_CREATED_BY_ME_MARKETPLACE_NAME => Some(Self::User),
@@ -0,0 +1,65 @@
use crate::remote::RemoteInstalledPlugin;
use crate::remote::RemotePluginScope;
use crate::store::ActivePluginInstallation;
use codex_config::types::PluginConfig;
use codex_plugin::PluginId;
use std::collections::HashMap;
use tracing::warn;
#[derive(Default)]
pub(crate) struct RemoteInstalledPluginsSnapshot {
pub(crate) configs: HashMap<String, PluginConfig>,
pub(crate) remote_plugin_id_resolver: RemotePluginIdResolver,
}
#[derive(Default)]
pub(crate) struct RemotePluginIdResolver {
snapshot_ids: Option<HashMap<PluginId, String>>,
}
impl RemotePluginIdResolver {
pub(crate) fn new(plugins: &[RemoteInstalledPlugin]) -> Self {
let mut snapshot_ids = HashMap::with_capacity(plugins.len());
for plugin in plugins {
let Ok(plugin_id) = PluginId::new(plugin.name.clone(), plugin.marketplace_name.clone())
else {
continue;
};
snapshot_ids
.entry(plugin_id)
.or_insert_with(|| plugin.id.clone());
}
Self {
snapshot_ids: Some(snapshot_ids),
}
}
pub(crate) fn remote_plugin_id_for_installation(
&self,
installation: &ActivePluginInstallation,
) -> Option<String> {
if let Some(snapshot_ids) = &self.snapshot_ids {
return snapshot_ids.get(&installation.plugin_id).cloned();
}
persisted_remote_plugin_id_for_installation(installation)
}
}
pub(crate) fn persisted_remote_plugin_id_for_installation(
installation: &ActivePluginInstallation,
) -> Option<String> {
RemotePluginScope::from_marketplace_name(&installation.plugin_id.marketplace_name)?;
match installation.persisted_remote_plugin_id() {
Ok(remote_plugin_id) => remote_plugin_id,
Err(err) => {
warn!(
plugin_id = %installation.plugin_id.as_key(),
error = %err,
"failed to read persisted remote plugin identity"
);
None
}
}
}
@@ -23,6 +23,7 @@ fn path(path: &Path) -> AbsolutePathBuf {
fn loaded_plugin(config_name: &str, root: &Path, enabled: bool) -> LoadedPlugin {
LoadedPlugin {
config_name: config_name.to_string(),
remote_plugin_id: None,
manifest_name: None,
plugin_namespace: None,
manifest_description: None,
+56 -32
View File
@@ -43,6 +43,48 @@ pub struct PluginStore {
data_root: AbsolutePathBuf,
}
pub(crate) struct ActivePluginInstallation {
pub(crate) plugin_id: PluginId,
pub(crate) root: AbsolutePathBuf,
remote_plugin_install_metadata_path: AbsolutePathBuf,
}
impl ActivePluginInstallation {
pub(crate) fn persisted_remote_plugin_id(&self) -> Result<Option<String>, PluginStoreError> {
let contents = match fs::read_to_string(self.remote_plugin_install_metadata_path.as_path())
{
Ok(contents) => contents,
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
Err(err) => {
return Err(PluginStoreError::io(
"failed to read remote plugin install metadata",
err,
));
}
};
let metadata: RemotePluginInstallMetadata =
serde_json::from_str(&contents).map_err(|err| {
PluginStoreError::Invalid(format!(
"failed to parse remote plugin install metadata: {err}"
))
})?;
if metadata.schema_version != REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION {
return Err(PluginStoreError::Invalid(format!(
"unsupported remote plugin install metadata schema version: {}",
metadata.schema_version
)));
}
let remote_plugin_id = metadata.remote_plugin_id.trim();
if remote_plugin_id.is_empty() {
return Err(PluginStoreError::Invalid(
"invalid remote plugin install metadata: remote plugin id must not be blank"
.to_string(),
));
}
Ok(Some(remote_plugin_id.to_string()))
}
}
#[derive(Clone, Copy)]
enum InstallManifest<'a> {
OnDisk,
@@ -124,6 +166,18 @@ impl PluginStore {
.map(|plugin_version| self.plugin_root(plugin_id, &plugin_version))
}
pub(crate) fn active_plugin_installation(
&self,
plugin_id: &PluginId,
) -> Option<ActivePluginInstallation> {
Some(ActivePluginInstallation {
plugin_id: plugin_id.clone(),
root: self.active_plugin_root(plugin_id)?,
remote_plugin_install_metadata_path: self
.remote_plugin_install_metadata_path(plugin_id),
})
}
pub fn is_installed(&self, plugin_id: &PluginId) -> bool {
self.active_plugin_version(plugin_id).is_some()
}
@@ -132,40 +186,10 @@ impl PluginStore {
&self,
plugin_id: &PluginId,
) -> Result<Option<String>, PluginStoreError> {
if !self.is_installed(plugin_id) {
let Some(installation) = self.active_plugin_installation(plugin_id) else {
return Ok(None);
}
let path = self.remote_plugin_install_metadata_path(plugin_id);
let contents = match fs::read_to_string(path.as_path()) {
Ok(contents) => contents,
Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
Err(err) => {
return Err(PluginStoreError::io(
"failed to read remote plugin install metadata",
err,
));
}
};
let metadata: RemotePluginInstallMetadata =
serde_json::from_str(&contents).map_err(|err| {
PluginStoreError::Invalid(format!(
"failed to parse remote plugin install metadata: {err}"
))
})?;
if metadata.schema_version != REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION {
return Err(PluginStoreError::Invalid(format!(
"unsupported remote plugin install metadata schema version: {}",
metadata.schema_version
)));
}
let remote_plugin_id = metadata.remote_plugin_id.trim();
if remote_plugin_id.is_empty() {
return Err(PluginStoreError::Invalid(
"invalid remote plugin install metadata: remote plugin id must not be blank"
.to_string(),
));
}
Ok(Some(remote_plugin_id.to_string()))
installation.persisted_remote_plugin_id()
}
pub fn write_remote_plugin_id(
@@ -11,6 +11,7 @@ use codex_plugin::prompt_safe_plugin_description;
use codex_protocol::auth::AuthMode;
use codex_protocol::protocol::Product;
use codex_skills::SkillConfigRules;
use codex_utils_plugins::PluginIdentity;
use tokio::sync::Semaphore;
use crate::app_mcp_routing::apply_app_mcp_routing_policy;
@@ -218,9 +219,13 @@ 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 plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
remote_plugin_id: None,
};
let skill_inventory = load_plugin_skill_inventory(
plugin_root,
&plugin_id,
&plugin_identity,
&manifest,
restriction_product,
/*plugin_skill_snapshots*/ None,
@@ -17,6 +17,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
path_to_skills_md: test_path_buf(path).abs(),
scope: codex_protocol::protocol::SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}
}
@@ -22,6 +22,7 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata {
path_to_skills_md: skill_doc_path,
scope: codex_protocol::protocol::SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}
}
+15 -12
View File
@@ -30,6 +30,7 @@ use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::PluginSkillRoot;
use codex_utils_plugins::SkillDiscoveryMode;
use dirs::home_dir;
@@ -195,7 +196,7 @@ pub struct SkillRoot {
pub path: AbsolutePathBuf,
pub scope: SkillScope,
pub file_system: Arc<dyn ExecutorFileSystem>,
pub plugin_id: Option<String>,
pub plugin_identity: Option<PluginIdentity>,
pub plugin_namespace: Option<String>,
pub plugin_root: Option<AbsolutePathBuf>,
pub discovery_mode: SkillDiscoveryMode,
@@ -269,7 +270,7 @@ async fn skill_roots_with_home_dir(
path: root.path,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some(root.plugin_id),
plugin_identity: Some(root.plugin_identity),
plugin_namespace: Some(root.plugin_namespace),
plugin_root: Some(root.plugin_root),
discovery_mode: root.discovery_mode,
@@ -278,7 +279,7 @@ async fn skill_roots_with_home_dir(
path,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -310,7 +311,7 @@ fn skill_roots_from_layer_stack_inner(
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::Repo,
file_system: Arc::clone(repo_fs),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -324,7 +325,7 @@ fn skill_roots_from_layer_stack_inner(
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -336,7 +337,7 @@ fn skill_roots_from_layer_stack_inner(
path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -349,7 +350,7 @@ fn skill_roots_from_layer_stack_inner(
path: system_cache_root_dir(&config_folder),
scope: SkillScope::System,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -362,7 +363,7 @@ fn skill_roots_from_layer_stack_inner(
path: config_folder.join(SKILLS_DIR_NAME),
scope: SkillScope::Admin,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -408,7 +409,7 @@ async fn repo_agents_skill_roots(
path: agents_skills,
scope: SkillScope::Repo,
file_system: Arc::clone(&fs),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -530,6 +531,7 @@ async fn load_skills_under_root(
outcome: &mut SkillLoadOutcome,
) {
let fs = skill_root.file_system.as_ref();
let plugin_identity = skill_root.plugin_identity.as_ref();
let plugin_root = match skill_root.plugin_root.as_ref() {
Some(plugin_root) => Some(canonicalize_for_skill_identity(fs, plugin_root).await),
None => None,
@@ -659,7 +661,7 @@ async fn load_skills_under_root(
&skill.path,
&skill.path_uri,
skill_root.scope,
skill_root.plugin_id.as_deref(),
plugin_identity,
plugin_root,
)
.await
@@ -696,7 +698,7 @@ async fn parse_skill_file(
path: &AbsolutePathBuf,
path_uri: &PathUri,
scope: SkillScope,
plugin_id: Option<&str>,
plugin_identity: Option<&PluginIdentity>,
plugin_root: Option<&AbsolutePathBuf>,
) -> Result<SkillMetadata, SkillParseError> {
let metadata_path = path_uri
@@ -734,7 +736,8 @@ async fn parse_skill_file(
policy,
path_to_skills_md: path.clone(),
scope,
plugin_id: plugin_id.map(str::to_string),
plugin_id: plugin_identity.map(|identity| identity.plugin_id.clone()),
remote_plugin_id: plugin_identity.and_then(|identity| identity.remote_plugin_id.clone()),
})
}
+73 -15
View File
@@ -505,6 +505,7 @@ async fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<()
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
@@ -571,7 +572,7 @@ async fn load_user_skills_root(root: &Path) -> SkillLoadOutcome {
path: root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -593,6 +594,7 @@ fn expected_user_skill(path: &Path, name: &str, description: &str) -> SkillMetad
path_to_skills_md: normalized(path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}
}
@@ -680,6 +682,7 @@ async fn loads_skill_dependencies_metadata_from_yaml() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -736,6 +739,7 @@ interface:
path_to_skills_md: normalized(skill_path.as_path()),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -890,6 +894,7 @@ async fn accepts_icon_paths_under_assets_dir() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -931,6 +936,7 @@ async fn ignores_invalid_brand_color() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -985,6 +991,7 @@ async fn ignores_default_prompt_over_max_length() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1027,6 +1034,7 @@ async fn drops_interface_when_icons_are_invalid() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1059,7 +1067,10 @@ interface:
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "twilio-developer-kit@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: None,
plugin_root: Some(plugin_root_abs.clone()),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1094,6 +1105,7 @@ interface:
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: Some("twilio-developer-kit@test".to_string()),
remote_plugin_id: None,
}]
);
}
@@ -1122,7 +1134,10 @@ interface:
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("twilio-developer-kit@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "twilio-developer-kit@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: None,
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1149,6 +1164,7 @@ interface:
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: Some("twilio-developer-kit@test".to_string()),
remote_plugin_id: None,
}]
);
}
@@ -1194,6 +1210,7 @@ async fn loads_skills_via_symlinked_subdir_for_user_scope() {
path_to_skills_md: normalized(&shared_skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1280,6 +1297,7 @@ async fn does_not_loop_on_symlink_cycle_for_user_scope() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1300,7 +1318,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() {
path: admin_root.path().abs(),
scope: SkillScope::Admin,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1327,6 +1345,7 @@ async fn loads_skills_via_symlinked_subdir_for_admin_scope() {
path_to_skills_md: normalized(&shared_skill_path),
scope: SkillScope::Admin,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1367,6 +1386,7 @@ async fn loads_skills_via_symlinked_subdir_for_repo_scope() {
path_to_skills_md: normalized(&linked_skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1388,7 +1408,7 @@ async fn system_scope_ignores_symlinked_subdir() {
path: system_root.abs(),
scope: SkillScope::System,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1428,7 +1448,7 @@ async fn respects_max_scan_depth_for_user_scope() {
path: skills_root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1455,6 +1475,7 @@ async fn respects_max_scan_depth_for_user_scope() {
path_to_skills_md: normalized(&within_depth_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1483,6 +1504,7 @@ async fn loads_valid_skill() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1516,6 +1538,7 @@ async fn falls_back_to_directory_name_when_skill_name_is_missing() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1541,7 +1564,10 @@ async fn namespaces_plugin_skills_using_provided_namespace() {
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "sample@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: Some("sample".to_string()),
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1568,6 +1594,7 @@ async fn namespaces_plugin_skills_using_provided_namespace() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: Some("sample@test".to_string()),
remote_plugin_id: None,
}]
);
}
@@ -1790,7 +1817,10 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "sample@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: Some(plugin_name.clone()),
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1817,6 +1847,7 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: Some("sample@test".to_string()),
remote_plugin_id: None,
}]
);
}
@@ -1841,7 +1872,10 @@ async fn plugin_skill_name_length_limit_rejects_overlong_qualified_name() {
path: plugin_root.join("skills").abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("sample@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "sample@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: Some(plugin_name.clone()),
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -1873,7 +1907,10 @@ async fn direct_child_discovery_ignores_nested_skills() {
path: skills_root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("plugin@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "plugin@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: Some("plugin".to_string()),
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::DirectChildren,
@@ -1896,6 +1933,7 @@ async fn direct_child_discovery_ignores_nested_skills() {
path_to_skills_md: normalized(&direct),
scope: SkillScope::User,
plugin_id: Some("plugin@test".to_string()),
remote_plugin_id: None,
}]
);
}
@@ -1917,7 +1955,10 @@ async fn direct_child_discovery_skips_skills_resolving_outside_plugin_root() {
path: skills_root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: Some("plugin@test".to_string()),
plugin_identity: Some(PluginIdentity {
plugin_id: "plugin@test".to_string(),
remote_plugin_id: None,
}),
plugin_namespace: Some("plugin".to_string()),
plugin_root: Some(plugin_root.abs()),
discovery_mode: SkillDiscoveryMode::DirectChildren,
@@ -1958,6 +1999,7 @@ async fn loads_short_description_from_metadata() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -1990,6 +2032,7 @@ async fn loads_unquoted_description_containing_colon_space() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2022,6 +2065,7 @@ async fn loads_unquoted_short_description_containing_colon_space_and_apostrophe(
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2054,6 +2098,7 @@ async fn loads_unrecognized_frontmatter_fields_that_need_quotes() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2086,6 +2131,7 @@ async fn preserves_block_scalar_body_while_repairing_other_fields() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2203,6 +2249,7 @@ async fn loads_skills_from_repo_root() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2239,6 +2286,7 @@ async fn loads_skills_from_agents_dir_without_codex_dir() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2293,6 +2341,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() {
path_to_skills_md: normalized(&nested_skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "root-skill".to_string(),
@@ -2304,6 +2353,7 @@ async fn loads_skills_from_all_codex_dirs_under_project_root() {
path_to_skills_md: normalized(&root_skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
]
);
@@ -2461,7 +2511,7 @@ async fn merges_root_results_in_input_order_when_scans_finish_out_of_order() {
SkillScope::User
},
file_system: Arc::clone(&root_file_system),
plugin_id: None,
plugin_identity: None,
plugin_namespace: Some("test".to_string()),
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -2527,7 +2577,7 @@ async fn skill_root_scans_wait_for_shared_capacity() {
path: root.abs(),
scope: SkillScope::Repo,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: Some("test".to_string()),
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -2580,6 +2630,7 @@ async fn loads_skills_from_codex_dir_when_not_git_repo() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2596,7 +2647,7 @@ async fn deduplicates_by_path_preferring_first_root() {
path: root.path().abs(),
scope: SkillScope::Repo,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -2605,7 +2656,7 @@ async fn deduplicates_by_path_preferring_first_root() {
path: root.path().abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -2633,6 +2684,7 @@ async fn deduplicates_by_path_preferring_first_root() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2675,6 +2727,7 @@ async fn keeps_duplicate_names_from_repo_and_user() {
path_to_skills_md: normalized(&repo_skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "dupe-skill".to_string(),
@@ -2686,6 +2739,7 @@ async fn keeps_duplicate_names_from_repo_and_user() {
path_to_skills_md: normalized(&user_skill_path),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
},
]
);
@@ -2749,6 +2803,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() {
path_to_skills_md: first_path,
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "dupe-skill".to_string(),
@@ -2760,6 +2815,7 @@ async fn keeps_duplicate_names_from_nested_codex_dirs() {
path_to_skills_md: second_path,
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
]
);
@@ -2832,6 +2888,7 @@ async fn loads_skills_when_cwd_is_file_in_repo() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
@@ -2891,6 +2948,7 @@ async fn loads_skills_from_system_cache_when_present() {
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::System,
plugin_id: None,
remote_plugin_id: None,
}]
);
}
+1
View File
@@ -939,6 +939,7 @@ mod tests {
path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(),
scope,
plugin_id: None,
remote_plugin_id: None,
}
}
+3 -3
View File
@@ -56,14 +56,14 @@ where
.await
.unwrap_or_else(|_| unreachable!());
let cache_key = match (
root.plugin_id.clone(),
root.plugin_identity.clone(),
root.plugin_namespace.clone(),
root.plugin_root.clone(),
) {
(Some(plugin_id), Some(plugin_namespace), Some(plugin_root)) => {
(Some(plugin_identity), Some(plugin_namespace), Some(plugin_root)) => {
Some(PluginSkillRoot {
path: root.path.clone(),
plugin_id,
plugin_identity,
plugin_namespace,
plugin_root,
discovery_mode: root.discovery_mode,
+15 -6
View File
@@ -8,6 +8,7 @@ use codex_exec_server::ExecutorFileSystem;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::PluginSkillRoot;
use tokio::sync::Semaphore;
use tracing::info;
@@ -280,10 +281,18 @@ impl SkillsService {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<(AbsolutePathBuf, u8, Option<String>, Option<String>)>,
roots: Vec<ConfigSkillRootCacheKey>,
skill_config_rules: SkillConfigRules,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillRootCacheKey {
path: AbsolutePathBuf,
scope_rank: u8,
plugin_identity: Option<PluginIdentity>,
plugin_namespace: Option<String>,
}
pub fn bundled_skills_enabled_from_stack(
config_layer_stack: &codex_config::ConfigLayerStack,
) -> bool {
@@ -320,12 +329,12 @@ fn config_skills_cache_key(
SkillScope::System => 2,
SkillScope::Admin => 3,
};
(
root.path.clone(),
ConfigSkillRootCacheKey {
path: root.path.clone(),
scope_rank,
root.plugin_id.clone(),
root.plugin_namespace.clone(),
)
plugin_identity: root.plugin_identity.clone(),
plugin_namespace: root.plugin_namespace.clone(),
}
})
.collect(),
skill_config_rules: skill_config_rules.clone(),
+53 -1
View File
@@ -11,6 +11,7 @@ use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::PathExt;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::PluginSkillRoot;
use codex_utils_plugins::SkillDiscoveryMode;
use pretty_assertions::assert_eq;
@@ -70,7 +71,10 @@ fn plugin_skill_root_for_skill_path(
.expect("plugin skills root should live under a plugin root");
PluginSkillRoot {
path: skills_root.abs(),
plugin_id: plugin_id.to_string(),
plugin_identity: PluginIdentity {
plugin_id: plugin_id.to_string(),
remote_plugin_id: None,
},
plugin_namespace: plugin_namespace.to_string(),
plugin_root: plugin_root.abs(),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -91,6 +95,7 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
.expect("skill path should canonicalize"),
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
}
}
@@ -230,6 +235,53 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
assert_eq!(outcome2.skills, outcome1.skills);
}
#[tokio::test]
async fn skills_for_config_refreshes_cache_when_remote_plugin_id_changes() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let skill_path = write_plugin_skill(
&codex_home,
"test",
"sample",
"sample-search",
"sample-search",
"search sample data",
);
let config_layer_stack = config_stack(&codex_home, "");
let mut plugin_skill_root =
plugin_skill_root_for_skill_path(&skill_path, "sample@test", "sample");
let skills_service = SkillsService::new(
codex_home.path().abs(),
/*bundled_skills_enabled*/ true,
);
skills_for_config_with_stack(
&skills_service,
&cwd,
&config_layer_stack,
&[plugin_skill_root.clone()],
)
.await;
plugin_skill_root.plugin_identity.remote_plugin_id = Some("plugins~Plugin_sample".to_string());
let refreshed = skills_for_config_with_stack(
&skills_service,
&cwd,
&config_layer_stack,
&[plugin_skill_root],
)
.await;
assert_eq!(
refreshed
.skills
.iter()
.find(|skill| skill.name == "sample:sample-search")
.and_then(|skill| skill.remote_plugin_id.as_deref()),
Some("plugins~Plugin_sample")
);
}
#[tokio::test]
async fn set_extra_roots_replaces_runtime_roots_and_clears_cache() {
let codex_home = tempfile::tempdir().expect("tempdir");
@@ -469,7 +469,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
path: host_root.abs(),
scope: SkillScope::User,
file_system,
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: codex_utils_plugins::SkillDiscoveryMode::Recursive,
@@ -504,6 +504,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
path_to_skills_md: first_skill_path,
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "linked:second".to_string(),
@@ -515,6 +516,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
path_to_skills_md: second_skill_path,
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
},
]
);
+7
View File
@@ -8921,6 +8921,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget()
path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(),
scope: SkillScope::Admin,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "repo-skill".to_string(),
@@ -8932,6 +8933,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget()
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
];
turn_context.model_info.context_window = Some(100);
@@ -8969,6 +8971,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() {
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
}];
let rendered = build_available_skills(
&outcome,
@@ -9014,6 +9017,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o
path_to_skills_md: test_path_buf("/tmp/alpha-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
};
let beta = SkillMetadata {
name: "beta-skill".to_string(),
@@ -9025,6 +9029,7 @@ fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_o
path_to_skills_md: test_path_buf("/tmp/beta-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
};
let minimum_skill_line_cost = |skill: &SkillMetadata| {
let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/");
@@ -9073,6 +9078,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(),
scope: SkillScope::Admin,
plugin_id: None,
remote_plugin_id: None,
},
SkillMetadata {
name: "repo-skill".to_string(),
@@ -9084,6 +9090,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_buil
path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(),
scope: SkillScope::Repo,
plugin_id: None,
remote_plugin_id: None,
},
];
turn_context.model_info.context_window = Some(100);
@@ -36,7 +36,7 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dy
path: root.clone(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: Default::default(),
@@ -209,7 +209,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
canonical_root: PathUri::from_abs_path(&canonical_root),
has_plugin_manifest: false,
}),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: Default::default(),
@@ -109,6 +109,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult {
path_to_skills_md: skill_path,
scope: SkillScope::User,
plugin_id: None,
remote_plugin_id: None,
});
let loaded_skills = Arc::new(outcome);
let skill_prompt_path = skill_path_string.replace('\\', "/");
@@ -1069,7 +1070,7 @@ async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResul
path: root,
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_identity: None,
plugin_namespace: None,
plugin_root: None,
discovery_mode: Default::default(),
+14 -3
View File
@@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::collections::HashSet;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginIdentity;
use codex_utils_plugins::PluginSkillRoot;
use codex_utils_plugins::SkillDiscoveryMode;
@@ -17,6 +18,7 @@ const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024;
#[derive(Debug, Clone, PartialEq)]
pub struct LoadedPlugin<M> {
pub config_name: String,
pub remote_plugin_id: Option<String>,
pub manifest_name: Option<String>,
pub plugin_namespace: Option<String>,
pub manifest_description: Option<String>,
@@ -135,7 +137,10 @@ impl<M: Clone> PluginLoadOutcome<M> {
if seen_paths.insert(path.clone()) {
skill_roots.push(PluginSkillRoot {
path: path.clone(),
plugin_id: plugin.config_name.clone(),
plugin_identity: PluginIdentity {
plugin_id: plugin.config_name.clone(),
remote_plugin_id: plugin.remote_plugin_id.clone(),
},
plugin_namespace: plugin_namespace.clone(),
plugin_root: plugin.root.clone(),
discovery_mode: SkillDiscoveryMode::Recursive,
@@ -224,6 +229,7 @@ mod tests {
fn loaded_plugin(config_name: &str, skill_roots: Vec<AbsolutePathBuf>) -> LoadedPlugin<()> {
LoadedPlugin {
config_name: config_name.to_string(),
remote_plugin_id: None,
manifest_name: None,
plugin_namespace: Some(
config_name
@@ -248,8 +254,10 @@ mod tests {
#[test]
fn effective_plugin_skill_roots_preserves_first_plugin_for_shared_root() {
let shared_root = test_path("shared-skills");
let mut first_plugin = loaded_plugin("zeta@test", vec![shared_root.clone()]);
first_plugin.remote_plugin_id = Some("plugins~Plugin_zeta".to_string());
let outcome = PluginLoadOutcome::from_plugins(vec![
loaded_plugin("zeta@test", vec![shared_root.clone()]),
first_plugin,
loaded_plugin("alpha@test", vec![shared_root.clone()]),
]);
@@ -257,7 +265,10 @@ mod tests {
outcome.effective_plugin_skill_roots(),
vec![PluginSkillRoot {
path: shared_root,
plugin_id: "zeta@test".to_string(),
plugin_identity: PluginIdentity {
plugin_id: "zeta@test".to_string(),
remote_plugin_id: Some("plugins~Plugin_zeta".to_string()),
},
plugin_namespace: "zeta".to_string(),
plugin_root: test_path("zeta@test"),
discovery_mode: SkillDiscoveryMode::Recursive,
+1 -1
View File
@@ -6,7 +6,7 @@ pub enum PluginIdError {
Invalid(String),
}
#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct PluginId {
pub plugin_name: String,
pub marketplace_name: String,
+1
View File
@@ -16,6 +16,7 @@ pub struct SkillMetadata {
pub path_to_skills_md: AbsolutePathBuf,
pub scope: SkillScope,
pub plugin_id: Option<String>,
pub remote_plugin_id: Option<String>,
}
impl SkillMetadata {
+8 -1
View File
@@ -26,10 +26,17 @@ pub enum SkillDiscoveryMode {
DirectChildren,
}
/// The local identifier and optional remote identifier for a plugin.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct PluginIdentity {
pub plugin_id: String,
pub remote_plugin_id: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct PluginSkillRoot {
pub path: AbsolutePathBuf,
pub plugin_id: String,
pub plugin_identity: PluginIdentity,
pub plugin_namespace: String,
pub plugin_root: AbsolutePathBuf,
pub discovery_mode: SkillDiscoveryMode,