mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: Handle alternate plugin manifest paths (#18182)
Load plugin manifests through a shared discoverable-path helper so manifest reads, installs, and skill names all see the same alternate manifest location.
This commit is contained in:
committed by
GitHub
Unverified
parent
a803790a10
commit
37161bc76e
@@ -462,7 +462,7 @@ async fn load_plugin(
|
||||
}
|
||||
|
||||
let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else {
|
||||
loaded_plugin.error = Some("missing or invalid .codex-plugin/plugin.json".to_string());
|
||||
loaded_plugin.error = Some("missing or invalid plugin.json".to_string());
|
||||
return loaded_plugin;
|
||||
};
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::PLUGIN_MANIFEST_PATH;
|
||||
use codex_utils_plugins::find_plugin_manifest_path;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::fs;
|
||||
@@ -115,10 +115,7 @@ enum RawPluginManifestDefaultPromptEntry {
|
||||
}
|
||||
|
||||
pub 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 manifest_path = find_plugin_manifest_path(plugin_root)?;
|
||||
let contents = fs::read_to_string(&manifest_path).ok()?;
|
||||
match serde_json::from_str::<RawPluginManifest>(&contents) {
|
||||
Ok(manifest) => {
|
||||
@@ -319,11 +316,14 @@ fn resolve_default_prompt_str(plugin_root: &Path, field: &str, prompt: &str) ->
|
||||
}
|
||||
|
||||
fn warn_invalid_default_prompt(plugin_root: &Path, field: &str, message: &str) {
|
||||
let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH);
|
||||
tracing::warn!(
|
||||
path = %manifest_path.display(),
|
||||
"ignoring {field}: {message}"
|
||||
);
|
||||
if let Some(manifest_path) = find_plugin_manifest_path(plugin_root) {
|
||||
tracing::warn!(
|
||||
path = %manifest_path.display(),
|
||||
"ignoring {field}: {message}"
|
||||
);
|
||||
} else {
|
||||
tracing::warn!("ignoring {field}: {message}");
|
||||
}
|
||||
}
|
||||
|
||||
fn json_value_type(value: &JsonValue) -> &'static str {
|
||||
@@ -390,6 +390,8 @@ mod tests {
|
||||
use std::path::Path;
|
||||
use tempfile::tempdir;
|
||||
|
||||
const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json";
|
||||
|
||||
fn write_manifest(plugin_root: &Path, version: Option<&str>, interface: &str) {
|
||||
fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir");
|
||||
let version = version
|
||||
@@ -408,6 +410,13 @@ mod tests {
|
||||
.expect("write manifest");
|
||||
}
|
||||
|
||||
fn write_alternate_plugin_manifest(plugin_root: &Path, contents: &str) {
|
||||
let manifest_path = plugin_root.join(ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH);
|
||||
fs::create_dir_all(manifest_path.parent().expect("manifest parent"))
|
||||
.expect("create manifest dir");
|
||||
fs::write(manifest_path, contents).expect("write manifest");
|
||||
}
|
||||
|
||||
fn load_manifest(plugin_root: &Path) -> PluginManifest {
|
||||
load_plugin_manifest(plugin_root).expect("load plugin manifest")
|
||||
}
|
||||
@@ -506,4 +515,31 @@ mod tests {
|
||||
|
||||
assert_eq!(manifest.version, Some("1.2.3-beta+7".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_manifest_uses_alternate_discoverable_path() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let plugin_root = tmp.path().join("demo-plugin");
|
||||
write_alternate_plugin_manifest(
|
||||
&plugin_root,
|
||||
r#"{
|
||||
"name": "demo-plugin",
|
||||
"version": " 2.0.0 ",
|
||||
"interface": {
|
||||
"displayName": "Fallback Plugin"
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
|
||||
let manifest = load_manifest(&plugin_root);
|
||||
|
||||
assert_eq!(manifest.version, Some("2.0.0".to_string()));
|
||||
assert_eq!(
|
||||
manifest
|
||||
.interface
|
||||
.as_ref()
|
||||
.and_then(|interface| interface.display_name.as_deref()),
|
||||
Some("Fallback Plugin")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -26,8 +26,10 @@ const MARKETPLACE_MANIFEST_RELATIVE_PATHS: &[&str] = &[
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ResolvedMarketplacePlugin {
|
||||
pub plugin_id: PluginId,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub auth_policy: MarketplacePluginAuthPolicy,
|
||||
pub source: MarketplacePluginSource,
|
||||
pub policy: MarketplacePluginPolicy,
|
||||
pub interface: Option<PluginManifestInterface>,
|
||||
pub manifest: Option<crate::manifest::PluginManifest>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -158,12 +160,9 @@ impl MarketplaceError {
|
||||
}
|
||||
}
|
||||
|
||||
// Always read the specified marketplace file from disk so installs see the
|
||||
// latest marketplace.json contents without any in-memory cache invalidation.
|
||||
pub fn resolve_marketplace_plugin(
|
||||
pub fn find_marketplace_plugin(
|
||||
marketplace_path: &AbsolutePathBuf,
|
||||
plugin_name: &str,
|
||||
restriction_product: Option<Product>,
|
||||
) -> Result<ResolvedMarketplacePlugin, MarketplaceError> {
|
||||
let marketplace = load_raw_marketplace_manifest(marketplace_path)?;
|
||||
let marketplace_name = marketplace.name;
|
||||
@@ -173,39 +172,11 @@ pub fn resolve_marketplace_plugin(
|
||||
continue;
|
||||
}
|
||||
|
||||
let RawMarketplaceManifestPlugin {
|
||||
name,
|
||||
source,
|
||||
policy,
|
||||
..
|
||||
} = plugin;
|
||||
let install_policy = policy.installation;
|
||||
let product_allowed = match policy.products.as_deref() {
|
||||
None => true,
|
||||
Some([]) => false,
|
||||
Some(products) => restriction_product
|
||||
.is_some_and(|product| product.matches_product_restriction(products)),
|
||||
};
|
||||
if install_policy == MarketplacePluginInstallPolicy::NotAvailable || !product_allowed {
|
||||
return Err(MarketplaceError::PluginNotAvailable {
|
||||
plugin_name: name,
|
||||
marketplace_name,
|
||||
});
|
||||
if let Some(plugin) =
|
||||
resolve_marketplace_plugin_entry(marketplace_path, &marketplace_name, plugin)?
|
||||
{
|
||||
return Ok(plugin);
|
||||
}
|
||||
|
||||
let Some(source_path) =
|
||||
resolve_supported_plugin_source_path(marketplace_path, &name, source)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
return Ok(ResolvedMarketplacePlugin {
|
||||
plugin_id: PluginId::new(name, marketplace_name).map_err(|err| match err {
|
||||
PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message),
|
||||
})?,
|
||||
source_path,
|
||||
auth_policy: policy.authentication,
|
||||
});
|
||||
}
|
||||
|
||||
Err(MarketplaceError::PluginNotFound {
|
||||
@@ -214,6 +185,31 @@ pub fn resolve_marketplace_plugin(
|
||||
})
|
||||
}
|
||||
|
||||
pub fn find_installable_marketplace_plugin(
|
||||
marketplace_path: &AbsolutePathBuf,
|
||||
plugin_name: &str,
|
||||
restriction_product: Option<Product>,
|
||||
) -> Result<ResolvedMarketplacePlugin, MarketplaceError> {
|
||||
let resolved = find_marketplace_plugin(marketplace_path, plugin_name)?;
|
||||
let product_allowed = match resolved.policy.products.as_deref() {
|
||||
None => true,
|
||||
Some([]) => false,
|
||||
Some(products) => {
|
||||
restriction_product.is_some_and(|product| product.matches_product_restriction(products))
|
||||
}
|
||||
};
|
||||
if resolved.policy.installation == MarketplacePluginInstallPolicy::NotAvailable
|
||||
|| !product_allowed
|
||||
{
|
||||
return Err(MarketplaceError::PluginNotAvailable {
|
||||
plugin_name: resolved.plugin_id.plugin_name,
|
||||
marketplace_name: resolved.plugin_id.marketplace_name,
|
||||
});
|
||||
}
|
||||
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
pub fn list_marketplaces(
|
||||
additional_roots: &[AbsolutePathBuf],
|
||||
) -> Result<MarketplaceListOutcome, MarketplaceError> {
|
||||
@@ -270,36 +266,26 @@ pub fn load_marketplace(path: &AbsolutePathBuf) -> Result<Marketplace, Marketpla
|
||||
let mut plugins = Vec::new();
|
||||
|
||||
for plugin in marketplace.plugins {
|
||||
let RawMarketplaceManifestPlugin {
|
||||
name,
|
||||
source,
|
||||
policy,
|
||||
category,
|
||||
} = plugin;
|
||||
let Some(source_path) = resolve_supported_plugin_source_path(path, &name, source) else {
|
||||
continue;
|
||||
let plugin = match resolve_marketplace_plugin_entry(path, &marketplace.name, plugin) {
|
||||
Ok(Some(plugin)) => plugin,
|
||||
Ok(None) => continue,
|
||||
Err(MarketplaceError::InvalidPlugin(message)) => {
|
||||
warn!(
|
||||
path = %path.display(),
|
||||
marketplace = %marketplace.name,
|
||||
error = %message,
|
||||
"skipping invalid marketplace plugin"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
let source = MarketplacePluginSource::Local {
|
||||
path: source_path.clone(),
|
||||
};
|
||||
let mut interface =
|
||||
load_plugin_manifest(source_path.as_path()).and_then(|manifest| manifest.interface);
|
||||
if let Some(category) = category {
|
||||
// Marketplace taxonomy wins when both sources provide a category.
|
||||
interface
|
||||
.get_or_insert_with(PluginManifestInterface::default)
|
||||
.category = Some(category);
|
||||
}
|
||||
|
||||
plugins.push(MarketplacePlugin {
|
||||
name,
|
||||
source,
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: policy.installation,
|
||||
authentication: policy.authentication,
|
||||
products: policy.products,
|
||||
},
|
||||
interface,
|
||||
name: plugin.plugin_id.plugin_name,
|
||||
source: plugin.source,
|
||||
policy: plugin.policy,
|
||||
interface: plugin.interface,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -389,6 +375,48 @@ fn load_raw_marketplace_manifest(
|
||||
})
|
||||
}
|
||||
|
||||
fn resolve_marketplace_plugin_entry(
|
||||
marketplace_path: &AbsolutePathBuf,
|
||||
marketplace_name: &str,
|
||||
plugin: RawMarketplaceManifestPlugin,
|
||||
) -> Result<Option<ResolvedMarketplacePlugin>, MarketplaceError> {
|
||||
let RawMarketplaceManifestPlugin {
|
||||
name,
|
||||
source,
|
||||
policy,
|
||||
category,
|
||||
} = plugin;
|
||||
let Some(source_path) = resolve_supported_plugin_source_path(marketplace_path, &name, source)
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let manifest = load_plugin_manifest(source_path.as_path());
|
||||
let mut interface = manifest
|
||||
.as_ref()
|
||||
.and_then(|manifest| manifest.interface.clone());
|
||||
if let Some(category) = category {
|
||||
// Marketplace taxonomy wins when both sources provide a category.
|
||||
interface
|
||||
.get_or_insert_with(PluginManifestInterface::default)
|
||||
.category = Some(category);
|
||||
}
|
||||
|
||||
Ok(Some(ResolvedMarketplacePlugin {
|
||||
plugin_id: PluginId::new(name, marketplace_name.to_string()).map_err(|err| match err {
|
||||
PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message),
|
||||
})?,
|
||||
source: MarketplacePluginSource::Local { path: source_path },
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: policy.installation,
|
||||
authentication: policy.authentication,
|
||||
products: policy.products,
|
||||
},
|
||||
interface,
|
||||
manifest,
|
||||
}))
|
||||
}
|
||||
|
||||
fn resolve_supported_plugin_source_path(
|
||||
marketplace_path: &AbsolutePathBuf,
|
||||
plugin_name: &str,
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::path::Path;
|
||||
use tempfile::tempdir;
|
||||
|
||||
const ALTERNATE_MARKETPLACE_RELATIVE_PATH: &str = ".claude-plugin/marketplace.json";
|
||||
const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json";
|
||||
|
||||
fn write_alternate_marketplace(repo_root: &Path, contents: &str) -> AbsolutePathBuf {
|
||||
let marketplace_path = repo_root.join(ALTERNATE_MARKETPLACE_RELATIVE_PATH);
|
||||
@@ -13,8 +14,14 @@ fn write_alternate_marketplace(repo_root: &Path, contents: &str) -> AbsolutePath
|
||||
AbsolutePathBuf::try_from(marketplace_path).unwrap()
|
||||
}
|
||||
|
||||
fn write_alternate_plugin_manifest(plugin_root: &Path, contents: &str) {
|
||||
let manifest_path = plugin_root.join(ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH);
|
||||
fs::create_dir_all(manifest_path.parent().unwrap()).unwrap();
|
||||
fs::write(manifest_path, contents).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
fn find_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -37,10 +44,9 @@ fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let resolved = resolve_marketplace_plugin(
|
||||
let resolved = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"local-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
@@ -49,14 +55,22 @@ fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
ResolvedMarketplacePlugin {
|
||||
plugin_id: PluginId::new("local-plugin".to_string(), "codex-curated".to_string())
|
||||
.unwrap(),
|
||||
source_path: AbsolutePathBuf::try_from(repo_root.join("plugin-1")).unwrap(),
|
||||
auth_policy: MarketplacePluginAuthPolicy::OnInstall,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("plugin-1")).unwrap(),
|
||||
},
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: MarketplacePluginInstallPolicy::Available,
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: None,
|
||||
manifest: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_supports_alternate_layout_and_string_local_source() {
|
||||
fn find_marketplace_plugin_supports_alternate_layout_and_string_local_source() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -73,12 +87,7 @@ fn resolve_marketplace_plugin_supports_alternate_layout_and_string_local_source(
|
||||
}"#,
|
||||
);
|
||||
|
||||
let resolved = resolve_marketplace_plugin(
|
||||
&marketplace_path,
|
||||
"string-source-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap();
|
||||
let resolved = find_marketplace_plugin(&marketplace_path, "string-source-plugin").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolved,
|
||||
@@ -88,15 +97,23 @@ fn resolve_marketplace_plugin_supports_alternate_layout_and_string_local_source(
|
||||
"alternate-marketplace".to_string()
|
||||
)
|
||||
.unwrap(),
|
||||
source_path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin"))
|
||||
.unwrap(),
|
||||
auth_policy: MarketplacePluginAuthPolicy::OnInstall,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin"))
|
||||
.unwrap(),
|
||||
},
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: MarketplacePluginInstallPolicy::Available,
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: None,
|
||||
manifest: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_reports_missing_plugin() {
|
||||
fn find_marketplace_plugin_reports_missing_plugin() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -107,10 +124,9 @@ fn resolve_marketplace_plugin_reports_missing_plugin() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = resolve_marketplace_plugin(
|
||||
let err = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"missing",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
@@ -124,8 +140,18 @@ fn resolve_marketplace_plugin_reports_missing_plugin() {
|
||||
fn list_marketplaces_supports_alternate_manifest_layout() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
let plugin_root = repo_root.join("plugins/string-source-plugin");
|
||||
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
write_alternate_plugin_manifest(
|
||||
&plugin_root,
|
||||
r#"{
|
||||
"name":"string-source-plugin",
|
||||
"interface": {
|
||||
"displayName": "String Source Plugin"
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
let marketplace_path = write_alternate_marketplace(
|
||||
&repo_root,
|
||||
r#"{
|
||||
@@ -163,6 +189,70 @@ fn list_marketplaces_supports_alternate_manifest_layout() {
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: Some(PluginManifestInterface {
|
||||
display_name: Some("String Source Plugin".to_string()),
|
||||
short_description: None,
|
||||
long_description: None,
|
||||
developer_name: None,
|
||||
category: None,
|
||||
capabilities: Vec::new(),
|
||||
website_url: None,
|
||||
privacy_policy_url: None,
|
||||
terms_of_service_url: None,
|
||||
default_prompt: None,
|
||||
brand_color: None,
|
||||
composer_icon: None,
|
||||
logo: None,
|
||||
screenshots: Vec::new(),
|
||||
}),
|
||||
}],
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_marketplaces_includes_plugins_without_discoverable_manifest() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
let marketplace_path = write_alternate_marketplace(
|
||||
&repo_root,
|
||||
r#"{
|
||||
"name": "alternate-marketplace",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "missing-plugin",
|
||||
"source": "./plugins/missing-plugin"
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
|
||||
let marketplaces = list_marketplaces_with_home(
|
||||
&[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()],
|
||||
/*home_dir*/ None,
|
||||
)
|
||||
.unwrap()
|
||||
.marketplaces;
|
||||
|
||||
assert_eq!(
|
||||
marketplaces,
|
||||
vec![Marketplace {
|
||||
name: "alternate-marketplace".to_string(),
|
||||
path: marketplace_path,
|
||||
interface: None,
|
||||
plugins: vec![MarketplacePlugin {
|
||||
name: "missing-plugin".to_string(),
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("plugins/missing-plugin"),)
|
||||
.unwrap(),
|
||||
},
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: MarketplacePluginInstallPolicy::Available,
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: None,
|
||||
}],
|
||||
}]
|
||||
@@ -448,16 +538,17 @@ fn list_marketplaces_keeps_distinct_entries_for_same_name() {
|
||||
]
|
||||
);
|
||||
|
||||
let resolved = resolve_marketplace_plugin(
|
||||
let resolved = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_marketplace).unwrap(),
|
||||
"local-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolved.source_path,
|
||||
AbsolutePathBuf::try_from(repo_root.join("repo-plugin")).unwrap()
|
||||
resolved.source,
|
||||
MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("repo-plugin")).unwrap(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -621,6 +712,67 @@ fn list_marketplaces_skips_invalid_plugins_but_keeps_marketplace() {
|
||||
assert!(marketplaces[1].plugins.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_marketplaces_skips_plugins_with_invalid_names_but_keeps_marketplace() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
fs::write(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "invalid-name-marketplace",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "valid-plugin",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./valid-plugin"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "invalid.plugin",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./invalid-plugin"
|
||||
}
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let marketplaces = list_marketplaces_with_home(
|
||||
&[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()],
|
||||
/*home_dir*/ None,
|
||||
)
|
||||
.unwrap()
|
||||
.marketplaces;
|
||||
|
||||
assert_eq!(
|
||||
marketplaces,
|
||||
vec![Marketplace {
|
||||
name: "invalid-name-marketplace".to_string(),
|
||||
path: AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json"))
|
||||
.unwrap(),
|
||||
interface: None,
|
||||
plugins: vec![MarketplacePlugin {
|
||||
name: "valid-plugin".to_string(),
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("valid-plugin")).unwrap(),
|
||||
},
|
||||
policy: MarketplacePluginPolicy {
|
||||
installation: MarketplacePluginInstallPolicy::Available,
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_marketplaces_reports_marketplace_load_errors() {
|
||||
let tmp = tempdir().unwrap();
|
||||
@@ -940,7 +1092,7 @@ fn list_marketplaces_ignores_plugin_interface_assets_without_dot_slash() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_skips_invalid_local_paths() {
|
||||
fn find_marketplace_plugin_skips_invalid_local_paths() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -962,10 +1114,9 @@ fn resolve_marketplace_plugin_skips_invalid_local_paths() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = resolve_marketplace_plugin(
|
||||
let err = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"local-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
@@ -976,7 +1127,7 @@ fn resolve_marketplace_plugin_skips_invalid_local_paths() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_skips_unsupported_sources() {
|
||||
fn find_marketplace_plugin_skips_unsupported_sources() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -996,8 +1147,7 @@ fn resolve_marketplace_plugin_skips_unsupported_sources() {
|
||||
}"#,
|
||||
);
|
||||
|
||||
let err = resolve_marketplace_plugin(&marketplace_path, "remote-plugin", Some(Product::Codex))
|
||||
.unwrap_err();
|
||||
let err = find_marketplace_plugin(&marketplace_path, "remote-plugin").unwrap_err();
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
@@ -1006,7 +1156,7 @@ fn resolve_marketplace_plugin_skips_unsupported_sources() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_uses_first_duplicate_entry() {
|
||||
fn find_marketplace_plugin_uses_first_duplicate_entry() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -1035,21 +1185,22 @@ fn resolve_marketplace_plugin_uses_first_duplicate_entry() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let resolved = resolve_marketplace_plugin(
|
||||
let resolved = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"local-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolved.source_path,
|
||||
AbsolutePathBuf::try_from(repo_root.join("first")).unwrap()
|
||||
resolved.source,
|
||||
MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(repo_root.join("first")).unwrap(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_rejects_disallowed_product() {
|
||||
fn find_installable_marketplace_plugin_rejects_disallowed_product() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -1074,7 +1225,7 @@ fn resolve_marketplace_plugin_rejects_disallowed_product() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = resolve_marketplace_plugin(
|
||||
let err = find_installable_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"chatgpt-plugin",
|
||||
Some(Product::Atlas),
|
||||
@@ -1088,7 +1239,7 @@ fn resolve_marketplace_plugin_rejects_disallowed_product() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_allows_missing_products_field() {
|
||||
fn find_marketplace_plugin_allows_missing_products_field() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -1111,10 +1262,9 @@ fn resolve_marketplace_plugin_allows_missing_products_field() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let resolved = resolve_marketplace_plugin(
|
||||
let resolved = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"default-plugin",
|
||||
Some(Product::Codex),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
@@ -1122,7 +1272,7 @@ fn resolve_marketplace_plugin_allows_missing_products_field() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_marketplace_plugin_rejects_explicit_empty_products() {
|
||||
fn find_installable_marketplace_plugin_rejects_explicit_empty_products() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
@@ -1147,7 +1297,7 @@ fn resolve_marketplace_plugin_rejects_explicit_empty_products() {
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let err = resolve_marketplace_plugin(
|
||||
let err = find_installable_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"disabled-plugin",
|
||||
Some(Product::Codex),
|
||||
|
||||
@@ -3,7 +3,7 @@ use crate::manifest::load_plugin_manifest;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::PLUGIN_MANIFEST_PATH;
|
||||
use codex_utils_plugins::find_plugin_manifest_path;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::fs;
|
||||
@@ -114,7 +114,7 @@ impl PluginStore {
|
||||
let plugin_name = plugin_name_for_source(source_path.as_path())?;
|
||||
if plugin_name != plugin_id.plugin_name {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin manifest name `{plugin_name}` does not match marketplace plugin name `{}`",
|
||||
"plugin.json name `{plugin_name}` does not match marketplace plugin name `{}`",
|
||||
plugin_id.plugin_name
|
||||
)));
|
||||
}
|
||||
@@ -184,20 +184,8 @@ fn validate_plugin_version_segment(plugin_version: &str) -> Result<(), String> {
|
||||
}
|
||||
|
||||
fn plugin_manifest_for_source(source_path: &Path) -> Result<PluginManifest, PluginStoreError> {
|
||||
let manifest_path = source_path.join(PLUGIN_MANIFEST_PATH);
|
||||
if !manifest_path.is_file() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"missing plugin manifest: {}",
|
||||
manifest_path.display()
|
||||
)));
|
||||
}
|
||||
|
||||
load_plugin_manifest(source_path).ok_or_else(|| {
|
||||
PluginStoreError::Invalid(format!(
|
||||
"missing or invalid plugin manifest: {}",
|
||||
manifest_path.display()
|
||||
))
|
||||
})
|
||||
load_plugin_manifest(source_path)
|
||||
.ok_or_else(|| PluginStoreError::Invalid("missing or invalid plugin.json".to_string()))
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
@@ -210,37 +198,26 @@ struct RawPluginManifestVersion {
|
||||
fn plugin_manifest_version_for_source(
|
||||
source_path: &Path,
|
||||
) -> Result<Option<String>, PluginStoreError> {
|
||||
let manifest_path = source_path.join(PLUGIN_MANIFEST_PATH);
|
||||
if !manifest_path.is_file() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"missing plugin manifest: {}",
|
||||
manifest_path.display()
|
||||
)));
|
||||
}
|
||||
let manifest_path = find_plugin_manifest_path(source_path)
|
||||
.ok_or_else(|| PluginStoreError::Invalid("missing plugin.json".to_string()))?;
|
||||
|
||||
let contents = fs::read_to_string(&manifest_path)
|
||||
.map_err(|err| PluginStoreError::io("failed to read plugin manifest", err))?;
|
||||
let manifest: RawPluginManifestVersion = serde_json::from_str(&contents).map_err(|err| {
|
||||
PluginStoreError::Invalid(format!(
|
||||
"failed to parse plugin manifest {}: {err}",
|
||||
manifest_path.display()
|
||||
))
|
||||
})?;
|
||||
.map_err(|err| PluginStoreError::io("failed to read plugin.json", err))?;
|
||||
let manifest: RawPluginManifestVersion = serde_json::from_str(&contents)
|
||||
.map_err(|err| PluginStoreError::Invalid(format!("failed to parse plugin.json: {err}")))?;
|
||||
let Some(version) = manifest.version else {
|
||||
return Ok(None);
|
||||
};
|
||||
let Some(version) = version.as_str() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"invalid plugin version in manifest {}: expected string",
|
||||
manifest_path.display()
|
||||
)));
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid plugin version in plugin.json: expected string".to_string(),
|
||||
));
|
||||
};
|
||||
let version = version.trim();
|
||||
if version.is_empty() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"invalid plugin version in manifest {}: must not be blank",
|
||||
manifest_path.display()
|
||||
)));
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"invalid plugin version in plugin.json: must not be blank".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(Some(version.to_string()))
|
||||
}
|
||||
|
||||
@@ -173,13 +173,9 @@ fn install_rejects_blank_manifest_version() {
|
||||
.expect_err("blank manifest version should be rejected");
|
||||
let err = err.to_string().replace('\\', "/");
|
||||
|
||||
assert!(
|
||||
err.starts_with("invalid plugin version in manifest "),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
assert!(
|
||||
err.ends_with("sample-plugin/.codex-plugin/plugin.json: must not be blank"),
|
||||
"unexpected error: {err}"
|
||||
assert_eq!(
|
||||
err,
|
||||
"invalid plugin version in plugin.json: must not be blank"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -305,6 +301,6 @@ fn install_rejects_manifest_names_that_do_not_match_marketplace_plugin_name() {
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"plugin manifest name `manifest-name` does not match marketplace plugin name `different-name`"
|
||||
"plugin.json name `manifest-name` does not match marketplace plugin name `different-name`"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user