Refactor plugin config and cache path (#13333)

Update config.toml plugin entries to use
<plugin_name>@<marketplace_name> as the key.
Plugin now stays in
[plugins/cache/marketplace-name/plugin-name/$version/]
Clean up the plugin code structure.
Add plugin install functionality (not used yet).
This commit is contained in:
xl-openai 2026-03-03 15:00:18 -08:00 committed by GitHub
parent 041c896509
commit 9b004e2db1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 622 additions and 92 deletions

View file

@ -1107,14 +1107,8 @@
"enabled": {
"default": true,
"type": "boolean"
},
"path": {
"$ref": "#/definitions/AbsolutePathBuf"
}
},
"required": [
"path"
],
"type": "object"
},
"ProjectConfig": {

View file

@ -778,7 +778,6 @@ pub struct SkillConfig {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct PluginConfig {
pub path: AbsolutePathBuf,
#[serde(default = "default_enabled")]
pub enabled: bool,
}

View file

@ -417,7 +417,7 @@ mod tests {
fs::write(path, contents).unwrap();
}
fn plugin_config_toml(plugin_root: &Path) -> String {
fn plugin_config_toml() -> String {
let mut root = toml::map::Map::new();
let mut features = toml::map::Map::new();
@ -425,14 +425,10 @@ mod tests {
root.insert("features".to_string(), Value::Table(features));
let mut plugin = toml::map::Map::new();
plugin.insert(
"path".to_string(),
Value::String(plugin_root.display().to_string()),
);
plugin.insert("enabled".to_string(), Value::Boolean(true));
let mut plugins = toml::map::Map::new();
plugins.insert("sample".to_string(), Value::Table(plugin));
plugins.insert("sample@test".to_string(), Value::Table(plugin));
root.insert("plugins".to_string(), Value::Table(plugins));
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize")
@ -616,7 +612,10 @@ mod tests {
#[tokio::test]
async fn effective_mcp_servers_include_plugins_without_overriding_user_config() {
let codex_home = tempfile::tempdir().expect("tempdir");
let plugin_root = codex_home.path().join("plugin-sample");
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
@ -638,7 +637,7 @@ mod tests {
);
write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
&plugin_config_toml(&plugin_root),
&plugin_config_toml(),
);
let mut config = ConfigBuilder::default()

View file

@ -1,4 +1,14 @@
use super::load_plugin_manifest;
use super::plugin_manifest_name;
use super::store::DEFAULT_PLUGIN_VERSION;
use super::store::PluginId;
use super::store::PluginInstallRequest;
use super::store::PluginInstallResult;
use super::store::PluginStore;
use super::store::PluginStoreError;
use crate::config::Config;
use crate::config::ConfigService;
use crate::config::ConfigServiceError;
use crate::config::ConfigToml;
use crate::config::profile::ConfigProfile;
use crate::config::types::McpServerConfig;
@ -7,10 +17,13 @@ use crate::config_loader::ConfigLayerStack;
use crate::features::Feature;
use crate::features::FeatureOverrides;
use crate::features::Features;
use codex_app_server_protocol::ConfigValueWriteParams;
use codex_app_server_protocol::MergeStrategy;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use serde_json::Map as JsonMap;
use serde_json::Value as JsonValue;
use serde_json::json;
use std::collections::HashMap;
use std::fs;
use std::path::Path;
@ -18,7 +31,6 @@ use std::path::PathBuf;
use std::sync::RwLock;
use tracing::warn;
const PLUGIN_MANIFEST_PATH: &str = ".codex-plugin/plugin.json";
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json";
@ -71,12 +83,16 @@ impl PluginLoadOutcome {
}
pub struct PluginsManager {
codex_home: PathBuf,
store: PluginStore,
cache_by_cwd: RwLock<HashMap<PathBuf, PluginLoadOutcome>>,
}
impl PluginsManager {
pub fn new(_codex_home: PathBuf) -> Self {
pub fn new(codex_home: PathBuf) -> Self {
Self {
codex_home: codex_home.clone(),
store: PluginStore::new(codex_home),
cache_by_cwd: RwLock::new(HashMap::new()),
}
}
@ -104,7 +120,7 @@ impl PluginsManager {
return outcome;
}
let outcome = load_plugins_from_layer_stack(config_layer_stack);
let outcome = load_plugins_from_layer_stack(config_layer_stack, &self.store);
log_plugin_load_errors(&outcome);
let mut cache = match self.cache_by_cwd.write() {
Ok(cache) => cache,
@ -128,6 +144,50 @@ impl PluginsManager {
Err(err) => err.into_inner().get(cwd).cloned(),
}
}
pub async fn install_plugin(
&self,
request: PluginInstallRequest,
) -> Result<PluginInstallResult, PluginInstallError> {
let store = self.store.clone();
let result = tokio::task::spawn_blocking(move || store.install(request))
.await
.map_err(PluginInstallError::join)??;
ConfigService::new_with_defaults(self.codex_home.clone())
.write_value(ConfigValueWriteParams {
key_path: format!("plugins.{}", result.plugin_id.as_key()),
value: json!({
"enabled": true,
}),
merge_strategy: MergeStrategy::Replace,
file_path: None,
expected_version: None,
})
.await
.map(|_| ())
.map_err(PluginInstallError::from)?;
Ok(result)
}
}
#[derive(Debug, thiserror::Error)]
pub enum PluginInstallError {
#[error("{0}")]
Store(#[from] PluginStoreError),
#[error("{0}")]
Config(#[from] ConfigServiceError),
#[error("failed to join plugin install task: {0}")]
Join(#[from] tokio::task::JoinError),
}
impl PluginInstallError {
fn join(source: tokio::task::JoinError) -> Self {
Self::Join(source)
}
}
fn plugins_feature_enabled_from_stack(config_layer_stack: &ConfigLayerStack) -> bool {
@ -160,11 +220,6 @@ fn log_plugin_load_errors(outcome: &PluginLoadOutcome) {
}
}
#[derive(Debug, Default, Deserialize)]
struct PluginManifest {
name: String,
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct PluginMcpFile {
@ -172,7 +227,10 @@ struct PluginMcpFile {
mcp_servers: HashMap<String, JsonValue>,
}
pub fn load_plugins_from_layer_stack(config_layer_stack: &ConfigLayerStack) -> PluginLoadOutcome {
pub(crate) fn load_plugins_from_layer_stack(
config_layer_stack: &ConfigLayerStack,
store: &PluginStore,
) -> PluginLoadOutcome {
let mut configured_plugins: Vec<_> = configured_plugins_from_stack(config_layer_stack)
.into_iter()
.collect();
@ -181,7 +239,7 @@ pub fn load_plugins_from_layer_stack(config_layer_stack: &ConfigLayerStack) -> P
let mut plugins = Vec::with_capacity(configured_plugins.len());
let mut seen_mcp_server_names = HashMap::<String, String>::new();
for (configured_name, plugin) in configured_plugins {
let loaded_plugin = load_plugin(configured_name.clone(), &plugin);
let loaded_plugin = load_plugin(configured_name.clone(), &plugin, store);
for name in loaded_plugin.mcp_servers.keys() {
if let Some(previous_plugin) =
seen_mcp_server_names.insert(name.clone(), configured_name.clone())
@ -226,12 +284,18 @@ fn configured_plugins_from_stack(
}
}
fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin {
let plugin_root = plugin.path.clone();
fn load_plugin(config_name: String, plugin: &PluginConfig, store: &PluginStore) -> LoadedPlugin {
let plugin_version = DEFAULT_PLUGIN_VERSION.to_string();
let plugin_root = PluginId::parse(&config_name)
.map(|plugin_id| store.plugin_root(&plugin_id, &plugin_version));
let root = match &plugin_root {
Ok(plugin_root) => plugin_root.clone(),
Err(_) => store.root().clone(),
};
let mut loaded_plugin = LoadedPlugin {
config_name,
manifest_name: None,
root: plugin_root.clone(),
root,
enabled: plugin.enabled,
skill_roots: Vec::new(),
mcp_servers: HashMap::new(),
@ -242,6 +306,14 @@ fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin {
return loaded_plugin;
}
let plugin_root = match plugin_root {
Ok(plugin_root) => plugin_root,
Err(err) => {
loaded_plugin.error = Some(err.to_string());
return loaded_plugin;
}
};
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;
@ -272,33 +344,6 @@ fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin {
loaded_plugin
}
fn load_plugin_manifest(plugin_root: &Path) -> Option<PluginManifest> {
let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH);
if !manifest_path.is_file() {
return None;
}
let contents = fs::read_to_string(&manifest_path).ok()?;
match serde_json::from_str(&contents) {
Ok(manifest) => Some(manifest),
Err(err) => {
warn!(
path = %manifest_path.display(),
"failed to parse plugin manifest: {err}"
);
None
}
}
}
fn plugin_manifest_name(manifest: &PluginManifest, plugin_root: &Path) -> String {
plugin_root
.file_name()
.and_then(|name| name.to_str())
.filter(|_| manifest.name.trim().is_empty())
.unwrap_or(&manifest.name)
.to_string()
}
fn default_skill_roots(plugin_root: &Path) -> Vec<PathBuf> {
let skills_dir = plugin_root.join(DEFAULT_SKILLS_DIR_NAME);
if skills_dir.is_dir() {
@ -422,6 +467,7 @@ mod tests {
use crate::config::ConfigBuilder;
use crate::config::types::McpServerTransportConfig;
use pretty_assertions::assert_eq;
use std::fs;
use tempfile::TempDir;
use toml::Value;
@ -430,11 +476,20 @@ mod tests {
fs::write(path, contents).unwrap();
}
fn plugin_config_toml(
plugin_root: &Path,
enabled: bool,
plugins_feature_enabled: bool,
) -> String {
fn write_plugin(root: &Path, dir_name: &str, manifest_name: &str) {
let plugin_root = root.join(dir_name);
fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap();
fs::create_dir_all(plugin_root.join("skills")).unwrap();
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
format!(r#"{{"name":"{manifest_name}"}}"#),
)
.unwrap();
fs::write(plugin_root.join("skills/SKILL.md"), "skill").unwrap();
fs::write(plugin_root.join(".mcp.json"), r#"{"mcpServers":{}}"#).unwrap();
}
fn plugin_config_toml(enabled: bool, plugins_feature_enabled: bool) -> String {
let mut root = toml::map::Map::new();
let mut features = toml::map::Map::new();
@ -445,14 +500,10 @@ mod tests {
root.insert("features".to_string(), Value::Table(features));
let mut plugin = toml::map::Map::new();
plugin.insert(
"path".to_string(),
Value::String(plugin_root.display().to_string()),
);
plugin.insert("enabled".to_string(), Value::Boolean(enabled));
let mut plugins = toml::map::Map::new();
plugins.insert("sample".to_string(), Value::Table(plugin));
plugins.insert("sample@test".to_string(), Value::Table(plugin));
root.insert("plugins".to_string(), Value::Table(plugins));
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize")
@ -471,7 +522,10 @@ mod tests {
#[tokio::test]
async fn load_plugins_loads_default_skills_and_mcp_servers() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home.path().join("plugin-sample");
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
@ -497,16 +551,13 @@ mod tests {
}"#,
);
let outcome = load_plugins_from_config(
&plugin_config_toml(&plugin_root, true, true),
codex_home.path(),
)
.await;
let outcome =
load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()).await;
assert_eq!(
outcome.plugins,
vec![LoadedPlugin {
config_name: "sample".to_string(),
config_name: "sample@test".to_string(),
manifest_name: Some("sample".to_string()),
root: AbsolutePathBuf::try_from(plugin_root.clone()).unwrap(),
enabled: true,
@ -544,7 +595,10 @@ mod tests {
#[tokio::test]
async fn load_plugins_preserves_disabled_plugins_without_effective_contributions() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home.path().join("plugin-sample");
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
@ -562,16 +616,13 @@ mod tests {
}"#,
);
let outcome = load_plugins_from_config(
&plugin_config_toml(&plugin_root, false, true),
codex_home.path(),
)
.await;
let outcome =
load_plugins_from_config(&plugin_config_toml(false, true), codex_home.path()).await;
assert_eq!(
outcome.plugins,
vec![LoadedPlugin {
config_name: "sample".to_string(),
config_name: "sample@test".to_string(),
manifest_name: None,
root: AbsolutePathBuf::try_from(plugin_root).unwrap(),
enabled: false,
@ -605,7 +656,10 @@ mod tests {
#[tokio::test]
async fn load_plugins_returns_empty_when_feature_disabled() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home.path().join("plugin-sample");
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
@ -616,12 +670,77 @@ mod tests {
"---\nname: sample-search\ndescription: search sample data\n---\n",
);
let outcome =
load_plugins_from_config(&plugin_config_toml(true, false), codex_home.path()).await;
assert_eq!(outcome, PluginLoadOutcome::default());
}
#[tokio::test]
async fn load_plugins_rejects_invalid_plugin_keys() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
);
let mut root = toml::map::Map::new();
let mut features = toml::map::Map::new();
features.insert("plugins".to_string(), Value::Boolean(true));
root.insert("features".to_string(), Value::Table(features));
let mut plugin = toml::map::Map::new();
plugin.insert("enabled".to_string(), Value::Boolean(true));
let mut plugins = toml::map::Map::new();
plugins.insert("sample".to_string(), Value::Table(plugin));
root.insert("plugins".to_string(), Value::Table(plugins));
let outcome = load_plugins_from_config(
&plugin_config_toml(&plugin_root, true, false),
&toml::to_string(&Value::Table(root)).expect("plugin test config should serialize"),
codex_home.path(),
)
.await;
assert_eq!(outcome, PluginLoadOutcome::default());
assert_eq!(outcome.plugins.len(), 1);
assert_eq!(
outcome.plugins[0].error.as_deref(),
Some("invalid plugin key `sample`; expected <plugin>@<marketplace>")
);
assert!(outcome.effective_skill_roots().is_empty());
assert!(outcome.effective_mcp_servers().is_empty());
}
#[tokio::test]
async fn install_plugin_updates_config_with_relative_path_and_plugin_key() {
let tmp = tempfile::tempdir().unwrap();
write_plugin(tmp.path(), "sample-plugin", "sample-plugin");
let result = PluginsManager::new(tmp.path().to_path_buf())
.install_plugin(PluginInstallRequest {
source_path: tmp.path().join("sample-plugin"),
marketplace_name: None,
})
.await
.unwrap();
let installed_path = tmp.path().join("plugins/cache/debug/sample-plugin/local");
assert_eq!(
result,
PluginInstallResult {
plugin_id: PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap(),
plugin_version: "local".to_string(),
installed_path,
}
);
let config = fs::read_to_string(tmp.path().join("config.toml")).unwrap();
assert!(config.contains(r#"[plugins."sample-plugin@debug"]"#));
assert!(config.contains("enabled = true"));
}
}

View file

@ -0,0 +1,37 @@
use serde::Deserialize;
use std::fs;
use std::path::Path;
pub(crate) const PLUGIN_MANIFEST_PATH: &str = ".codex-plugin/plugin.json";
#[derive(Debug, Default, Deserialize)]
pub(crate) struct PluginManifest {
name: String,
}
pub(crate) fn load_plugin_manifest(plugin_root: &Path) -> Option<PluginManifest> {
let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH);
if !manifest_path.is_file() {
return None;
}
let contents = fs::read_to_string(&manifest_path).ok()?;
match serde_json::from_str(&contents) {
Ok(manifest) => Some(manifest),
Err(err) => {
tracing::warn!(
path = %manifest_path.display(),
"failed to parse plugin manifest: {err}"
);
None
}
}
}
pub(crate) fn plugin_manifest_name(manifest: &PluginManifest, plugin_root: &Path) -> String {
plugin_root
.file_name()
.and_then(|name| name.to_str())
.filter(|_| manifest.name.trim().is_empty())
.unwrap_or(&manifest.name)
.to_string()
}

View file

@ -0,0 +1,14 @@
mod manager;
mod manifest;
mod store;
pub use manager::LoadedPlugin;
pub use manager::PluginInstallError;
pub use manager::PluginLoadOutcome;
pub use manager::PluginsManager;
pub(crate) use manager::plugin_namespace_for_skill_path;
pub(crate) use manifest::load_plugin_manifest;
pub(crate) use manifest::plugin_manifest_name;
pub use store::PluginId;
pub use store::PluginInstallRequest;
pub use store::PluginInstallResult;

View file

@ -0,0 +1,374 @@
use super::load_plugin_manifest;
use super::manifest::PLUGIN_MANIFEST_PATH;
use super::plugin_manifest_name;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::fs;
use std::io;
use std::path::Path;
use std::path::PathBuf;
const DEFAULT_MARKETPLACE_NAME: &str = "debug";
pub(crate) const DEFAULT_PLUGIN_VERSION: &str = "local";
pub(crate) const PLUGINS_CACHE_DIR: &str = "plugins/cache";
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PluginInstallRequest {
pub source_path: PathBuf,
pub marketplace_name: Option<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PluginId {
pub plugin_name: String,
pub marketplace_name: String,
}
impl PluginId {
pub fn new(plugin_name: String, marketplace_name: String) -> Result<Self, PluginStoreError> {
validate_plugin_segment(&plugin_name, "plugin name")
.map_err(PluginStoreError::InvalidPluginKey)?;
validate_plugin_segment(&marketplace_name, "marketplace name")
.map_err(PluginStoreError::InvalidPluginKey)?;
Ok(Self {
plugin_name,
marketplace_name,
})
}
pub fn parse(plugin_key: &str) -> Result<Self, PluginStoreError> {
let Some((plugin_name, marketplace_name)) = plugin_key.rsplit_once('@') else {
return Err(PluginStoreError::InvalidPluginKey(format!(
"invalid plugin key `{plugin_key}`; expected <plugin>@<marketplace>"
)));
};
if plugin_name.is_empty() || marketplace_name.is_empty() {
return Err(PluginStoreError::InvalidPluginKey(format!(
"invalid plugin key `{plugin_key}`; expected <plugin>@<marketplace>"
)));
}
Self::new(plugin_name.to_string(), marketplace_name.to_string()).map_err(|err| match err {
PluginStoreError::InvalidPluginKey(message) => {
PluginStoreError::InvalidPluginKey(format!("{message} in `{plugin_key}`"))
}
other => other,
})
}
pub fn as_key(&self) -> String {
format!("{}@{}", self.plugin_name, self.marketplace_name)
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PluginInstallResult {
pub plugin_id: PluginId,
pub plugin_version: String,
pub installed_path: PathBuf,
}
#[derive(Debug, Clone)]
pub struct PluginStore {
root: AbsolutePathBuf,
}
impl PluginStore {
pub fn new(codex_home: PathBuf) -> Self {
Self {
root: AbsolutePathBuf::try_from(codex_home.join(PLUGINS_CACHE_DIR))
.unwrap_or_else(|err| panic!("plugin cache root should be absolute: {err}")),
}
}
pub fn root(&self) -> &AbsolutePathBuf {
&self.root
}
pub fn plugin_root(&self, plugin_id: &PluginId, plugin_version: &str) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(
self.root
.as_path()
.join(&plugin_id.marketplace_name)
.join(&plugin_id.plugin_name)
.join(plugin_version),
)
.unwrap_or_else(|err| panic!("plugin cache path should resolve to an absolute path: {err}"))
}
pub fn install(
&self,
request: PluginInstallRequest,
) -> Result<PluginInstallResult, PluginStoreError> {
let source_path = request.source_path;
if !source_path.is_dir() {
return Err(PluginStoreError::InvalidPlugin(format!(
"plugin source path is not a directory: {}",
source_path.display()
)));
}
let plugin_name = plugin_name_for_source(&source_path)?;
let marketplace_name = request
.marketplace_name
.filter(|name| !name.trim().is_empty())
.unwrap_or_else(|| DEFAULT_MARKETPLACE_NAME.to_string());
let plugin_version = DEFAULT_PLUGIN_VERSION.to_string();
let plugin_id = match PluginId::new(plugin_name, marketplace_name) {
Ok(plugin_id) => plugin_id,
Err(PluginStoreError::InvalidPluginKey(message)) => {
return Err(PluginStoreError::InvalidPlugin(message));
}
Err(err) => return Err(err),
};
let installed_path = self
.plugin_root(&plugin_id, &plugin_version)
.into_path_buf();
if let Some(parent) = installed_path.parent() {
fs::create_dir_all(parent).map_err(|err| {
PluginStoreError::io("failed to create plugin cache directory", err)
})?;
}
remove_existing_target(&installed_path)?;
copy_dir_recursive(&source_path, &installed_path)?;
Ok(PluginInstallResult {
plugin_id,
plugin_version,
installed_path,
})
}
}
#[derive(Debug, thiserror::Error)]
pub enum PluginStoreError {
#[error("{context}: {source}")]
Io {
context: &'static str,
#[source]
source: io::Error,
},
#[error("{0}")]
InvalidPlugin(String),
#[error("{0}")]
InvalidPluginKey(String),
}
impl PluginStoreError {
fn io(context: &'static str, source: io::Error) -> Self {
Self::Io { context, source }
}
}
fn plugin_name_for_source(source_path: &Path) -> Result<String, PluginStoreError> {
let manifest_path = source_path.join(PLUGIN_MANIFEST_PATH);
if !manifest_path.is_file() {
return Err(PluginStoreError::InvalidPlugin(format!(
"missing plugin manifest: {}",
manifest_path.display()
)));
}
let manifest = load_plugin_manifest(source_path).ok_or_else(|| {
PluginStoreError::InvalidPlugin(format!(
"missing or invalid plugin manifest: {}",
manifest_path.display()
))
})?;
let plugin_name = plugin_manifest_name(&manifest, source_path);
validate_plugin_segment(&plugin_name, "plugin name")
.map_err(PluginStoreError::InvalidPlugin)
.map(|_| plugin_name)
}
fn validate_plugin_segment(segment: &str, kind: &str) -> Result<(), String> {
if segment.is_empty() {
return Err(format!("invalid {kind}: must not be empty"));
}
if !segment
.chars()
.all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_')
{
return Err(format!(
"invalid {kind}: only ASCII letters, digits, `_`, and `-` are allowed"
));
}
Ok(())
}
fn remove_existing_target(path: &Path) -> Result<(), PluginStoreError> {
if !path.exists() {
return Ok(());
}
if path.is_dir() {
fs::remove_dir_all(path).map_err(|err| {
PluginStoreError::io("failed to remove existing plugin cache entry", err)
})
} else {
fs::remove_file(path).map_err(|err| {
PluginStoreError::io("failed to remove existing plugin cache entry", err)
})
}
}
fn copy_dir_recursive(source: &Path, target: &Path) -> Result<(), PluginStoreError> {
fs::create_dir_all(target)
.map_err(|err| PluginStoreError::io("failed to create plugin target directory", err))?;
for entry in fs::read_dir(source)
.map_err(|err| PluginStoreError::io("failed to read plugin source directory", err))?
{
let entry =
entry.map_err(|err| PluginStoreError::io("failed to enumerate plugin source", err))?;
let source_path = entry.path();
let target_path = target.join(entry.file_name());
let file_type = entry
.file_type()
.map_err(|err| PluginStoreError::io("failed to inspect plugin source entry", err))?;
if file_type.is_dir() {
copy_dir_recursive(&source_path, &target_path)?;
} else if file_type.is_file() {
fs::copy(&source_path, &target_path)
.map_err(|err| PluginStoreError::io("failed to copy plugin file", err))?;
}
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
fn write_plugin(root: &Path, dir_name: &str, manifest_name: &str) {
let plugin_root = root.join(dir_name);
fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap();
fs::create_dir_all(plugin_root.join("skills")).unwrap();
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
format!(r#"{{"name":"{manifest_name}"}}"#),
)
.unwrap();
fs::write(plugin_root.join("skills/SKILL.md"), "skill").unwrap();
fs::write(plugin_root.join(".mcp.json"), r#"{"mcpServers":{}}"#).unwrap();
}
#[test]
fn install_copies_plugin_into_default_marketplace() {
let tmp = tempdir().unwrap();
write_plugin(tmp.path(), "sample-plugin", "sample-plugin");
let result = PluginStore::new(tmp.path().to_path_buf())
.install(PluginInstallRequest {
source_path: tmp.path().join("sample-plugin"),
marketplace_name: None,
})
.unwrap();
let installed_path = tmp.path().join("plugins/cache/debug/sample-plugin/local");
assert_eq!(
result,
PluginInstallResult {
plugin_id: PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap(),
plugin_version: "local".to_string(),
installed_path: installed_path.clone(),
}
);
assert!(installed_path.join(".codex-plugin/plugin.json").is_file());
assert!(installed_path.join("skills/SKILL.md").is_file());
}
#[test]
fn install_uses_manifest_name_for_destination_and_key() {
let tmp = tempdir().unwrap();
write_plugin(tmp.path(), "source-dir", "manifest-name");
let result = PluginStore::new(tmp.path().to_path_buf())
.install(PluginInstallRequest {
source_path: tmp.path().join("source-dir"),
marketplace_name: Some("market".to_string()),
})
.unwrap();
assert_eq!(
result,
PluginInstallResult {
plugin_id: PluginId::new("manifest-name".to_string(), "market".to_string())
.unwrap(),
plugin_version: "local".to_string(),
installed_path: tmp.path().join("plugins/cache/market/manifest-name/local"),
}
);
}
#[test]
fn plugin_root_derives_path_from_key_and_version() {
let tmp = tempdir().unwrap();
let store = PluginStore::new(tmp.path().to_path_buf());
let plugin_id = PluginId::new("sample".to_string(), "debug".to_string()).unwrap();
assert_eq!(
store.plugin_root(&plugin_id, "local").as_path(),
tmp.path().join("plugins/cache/debug/sample/local")
);
}
#[test]
fn plugin_root_rejects_path_separators_in_key_segments() {
let err = PluginId::parse("../../etc@debug").unwrap_err();
assert_eq!(
err.to_string(),
"invalid plugin name: only ASCII letters, digits, `_`, and `-` are allowed in `../../etc@debug`"
);
let err = PluginId::parse("sample@../../etc").unwrap_err();
assert_eq!(
err.to_string(),
"invalid marketplace name: only ASCII letters, digits, `_`, and `-` are allowed in `sample@../../etc`"
);
}
#[test]
fn install_rejects_manifest_names_with_path_separators() {
let tmp = tempdir().unwrap();
write_plugin(tmp.path(), "source-dir", "../../etc");
let err = PluginStore::new(tmp.path().to_path_buf())
.install(PluginInstallRequest {
source_path: tmp.path().join("source-dir"),
marketplace_name: None,
})
.unwrap_err();
assert_eq!(
err.to_string(),
"invalid plugin name: only ASCII letters, digits, `_`, and `-` are allowed"
);
}
#[test]
fn install_rejects_marketplace_names_with_path_separators() {
let tmp = tempdir().unwrap();
write_plugin(tmp.path(), "source-dir", "sample-plugin");
let err = PluginStore::new(tmp.path().to_path_buf())
.install(PluginInstallRequest {
source_path: tmp.path().join("source-dir"),
marketplace_name: Some("../../etc".to_string()),
})
.unwrap_err();
assert_eq!(
err.to_string(),
"invalid marketplace name: only ASCII letters, digits, `_`, and `-` are allowed"
);
}
}

View file

@ -24,7 +24,7 @@ use tempfile::TempDir;
use wiremock::MockServer;
fn write_plugin_skill_plugin(home: &TempDir) -> std::path::PathBuf {
let plugin_root = home.path().join("plugins/sample");
let plugin_root = home.path().join("plugins/cache/test/sample/local");
let skill_dir = plugin_root.join("skills/sample-search");
std::fs::create_dir_all(skill_dir.as_path()).expect("create plugin skill dir");
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
@ -40,17 +40,14 @@ fn write_plugin_skill_plugin(home: &TempDir) -> std::path::PathBuf {
.expect("write plugin skill");
std::fs::write(
home.path().join("config.toml"),
format!(
"[features]\nplugins = true\n\n[plugins.sample]\nenabled = true\npath = \"{}\"\n",
plugin_root.display()
),
"[features]\nplugins = true\n\n[plugins.\"sample@test\"]\nenabled = true\n",
)
.expect("write config");
skill_dir.join("SKILL.md")
}
fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
let plugin_root = home.path().join("plugins/sample");
let plugin_root = home.path().join("plugins/cache/test/sample/local");
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
std::fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
@ -72,10 +69,7 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
.expect("write plugin mcp config");
std::fs::write(
home.path().join("config.toml"),
format!(
"[features]\nplugins = true\n\n[plugins.sample]\nenabled = true\npath = \"{}\"\n",
plugin_root.display()
),
"[features]\nplugins = true\n\n[plugins.\"sample@test\"]\nenabled = true\n",
)
.expect("write config");
}