mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Support marketplace plugin manifest fallback (#28789)
## Summary Support marketplace plugins whose source directory does not include a discoverable plugin manifest. Metadata-rich `marketplace.json` entries now act as fallback plugin manifests for listing, local detail reads, install, and non-curated cache refresh. The fallback preserves marketplace-entry plugin fields wholesale, then adds the small Codex-facing compatibility bridge for presentation metadata. A real source `plugin.json` always wins when present. ## Details - Capture flattened marketplace-entry fields into `MarketplacePluginManifestFallback`, preserving fields such as `version`, `description`, `skills`, `mcpServers`, `apps`, `hooks`, `agents`, `commands`, `strict`, `author`, and future manifest fields without a per-field translation list. - Bridge Claude-style top-level `displayName`, `author.name`, `homepage`, and marketplace `category` into Codex's nested `interface` fields only when the nested values are absent. - Treat fallback metadata as installable only when the marketplace entry contributes metadata beyond bare `name` and `source`; existing missing-manifest behavior remains for metadata-free entries. - Read local plugin details from the already parsed fallback manifest, including fallback-declared app and MCP paths, instead of rereading only an on-disk manifest. - Pass fallback contents into `PluginStore`, which validates them and injects `.codex-plugin/plugin.json` into Store's existing atomic copy. Local marketplace source directories are never mutated, and the fallback path no longer needs an additional staging directory. - Keep Git source materialization unchanged; Git clones still use the existing marketplace source staging area before Store installation.
This commit is contained in:
committed by
GitHub
Unverified
parent
dce673905a
commit
772c5c5195
@@ -7,12 +7,14 @@ use crate::manifest::PluginManifestMcpServers;
|
||||
use crate::manifest::PluginManifestPaths;
|
||||
use crate::manifest::load_plugin_manifest;
|
||||
use crate::marketplace::MarketplacePluginSource;
|
||||
use crate::marketplace::find_marketplace_plugin;
|
||||
use crate::marketplace::list_marketplaces;
|
||||
use crate::marketplace::load_marketplace;
|
||||
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
use crate::store::PluginStore;
|
||||
use crate::store::plugin_version_for_source;
|
||||
use crate::store::plugin_version_for_source_with_fallback_manifest;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::HooksFile;
|
||||
@@ -460,7 +462,7 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
let store = PluginStore::try_new(codex_home.to_path_buf()).map_err(|err| err.to_string())?;
|
||||
let marketplace_outcome = list_marketplaces(additional_roots)
|
||||
.map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?;
|
||||
let mut plugin_sources = HashMap::<String, MarketplacePluginSource>::new();
|
||||
let mut plugin_sources = HashMap::<String, (MarketplacePluginSource, Option<String>)>::new();
|
||||
|
||||
for marketplace in marketplace_outcome.marketplaces {
|
||||
if is_openai_curated_marketplace_name(&marketplace.name) {
|
||||
@@ -489,14 +491,31 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
continue;
|
||||
}
|
||||
|
||||
plugin_sources.insert(plugin_key, plugin.source);
|
||||
let manifest_fallback = find_marketplace_plugin(&marketplace.path, &plugin.name)
|
||||
.map(|resolved| {
|
||||
resolved
|
||||
.manifest_fallback
|
||||
.contents_if_has_metadata()
|
||||
.map(str::to_string)
|
||||
})
|
||||
.unwrap_or_else(|err| {
|
||||
warn!(
|
||||
plugin = plugin.name,
|
||||
marketplace = marketplace.name,
|
||||
error = %err,
|
||||
"failed to resolve marketplace plugin manifest fallback during cache refresh"
|
||||
);
|
||||
None
|
||||
});
|
||||
plugin_sources.insert(plugin_key, (plugin.source, manifest_fallback));
|
||||
}
|
||||
}
|
||||
|
||||
let mut cache_refreshed = false;
|
||||
for plugin_id in configured_non_curated_plugin_ids {
|
||||
let plugin_key = plugin_id.as_key();
|
||||
let Some(source) = plugin_sources.get(&plugin_key).cloned() else {
|
||||
let Some((source, manifest_fallback_contents)) = plugin_sources.get(&plugin_key).cloned()
|
||||
else {
|
||||
warn!(
|
||||
plugin = plugin_id.plugin_name,
|
||||
marketplace = plugin_id.marketplace_name,
|
||||
@@ -509,8 +528,14 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
format!("failed to materialize plugin source for {plugin_key}: {err}")
|
||||
})?;
|
||||
let source_path = materialized.path.clone();
|
||||
let plugin_version = plugin_version_for_source(source_path.as_path())
|
||||
.map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?;
|
||||
let plugin_version = match manifest_fallback_contents.as_deref() {
|
||||
Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest(
|
||||
source_path.as_path(),
|
||||
manifest_contents,
|
||||
),
|
||||
None => plugin_version_for_source(source_path.as_path()),
|
||||
}
|
||||
.map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?;
|
||||
|
||||
if mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str())
|
||||
@@ -518,9 +543,16 @@ fn refresh_non_curated_plugin_cache_with_mode(
|
||||
continue;
|
||||
}
|
||||
|
||||
store
|
||||
.install_with_version(source_path, plugin_id.clone(), plugin_version)
|
||||
.map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?;
|
||||
match manifest_fallback_contents.as_deref() {
|
||||
Some(manifest_contents) => store.install_with_version_and_fallback_manifest(
|
||||
source_path,
|
||||
plugin_id.clone(),
|
||||
plugin_version,
|
||||
manifest_contents,
|
||||
),
|
||||
None => store.install_with_version(source_path, plugin_id.clone(), plugin_version),
|
||||
}
|
||||
.map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?;
|
||||
cache_refreshed = true;
|
||||
}
|
||||
|
||||
@@ -901,15 +933,22 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec<AbsolutePathBuf> {
|
||||
|
||||
pub async fn load_plugin_apps(plugin_root: &Path) -> Vec<AppDeclaration> {
|
||||
if let Some(manifest) = load_plugin_manifest(plugin_root) {
|
||||
return load_apps_from_paths(
|
||||
plugin_root,
|
||||
plugin_app_config_paths(plugin_root, &manifest.paths),
|
||||
)
|
||||
.await;
|
||||
return load_plugin_apps_from_manifest(plugin_root, &manifest.paths).await;
|
||||
}
|
||||
load_apps_from_paths(plugin_root, default_app_config_paths(plugin_root)).await
|
||||
}
|
||||
|
||||
pub(crate) async fn load_plugin_apps_from_manifest(
|
||||
plugin_root: &Path,
|
||||
manifest_paths: &PluginManifestPaths,
|
||||
) -> Vec<AppDeclaration> {
|
||||
load_apps_from_paths(
|
||||
plugin_root,
|
||||
plugin_app_config_paths(plugin_root, manifest_paths),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub fn plugin_app_declarations_from_value(value: &JsonValue) -> Vec<AppDeclaration> {
|
||||
let Ok(parsed) = serde_json::from_value::<PluginAppFile>(value.clone()) else {
|
||||
return Vec::new();
|
||||
@@ -1181,7 +1220,7 @@ async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap<String,
|
||||
.await
|
||||
}
|
||||
|
||||
async fn load_plugin_mcp_servers_from_manifest(
|
||||
pub(crate) async fn load_plugin_mcp_servers_from_manifest(
|
||||
plugin_root: &Path,
|
||||
manifest_paths: &PluginManifestPaths,
|
||||
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
|
||||
|
||||
@@ -7,10 +7,10 @@ use crate::loader::PluginHookLoadOutcome;
|
||||
use crate::loader::configured_curated_plugin_ids_from_codex_home;
|
||||
use crate::loader::curated_plugin_cache_version;
|
||||
use crate::loader::installed_plugin_telemetry_metadata;
|
||||
use crate::loader::load_plugin_apps;
|
||||
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;
|
||||
use crate::loader::load_plugin_mcp_servers_from_manifest;
|
||||
use crate::loader::load_plugin_skills;
|
||||
use crate::loader::load_plugins_from_layer_stack;
|
||||
use crate::loader::log_plugin_load_errors;
|
||||
@@ -27,6 +27,7 @@ use crate::marketplace::MarketplaceInterface;
|
||||
use crate::marketplace::MarketplaceListError;
|
||||
use crate::marketplace::MarketplaceListOutcome;
|
||||
use crate::marketplace::MarketplacePluginAuthPolicy;
|
||||
use crate::marketplace::MarketplacePluginManifestFallback;
|
||||
use crate::marketplace::MarketplacePluginPolicy;
|
||||
use crate::marketplace::MarketplacePluginSource;
|
||||
use crate::marketplace::ResolvedMarketplacePlugin;
|
||||
@@ -315,6 +316,7 @@ pub struct ConfiguredMarketplacePlugin {
|
||||
pub policy: MarketplacePluginPolicy,
|
||||
pub interface: Option<PluginManifestInterface>,
|
||||
pub keywords: Vec<String>,
|
||||
pub manifest_fallback: Option<MarketplacePluginManifestFallback>,
|
||||
pub installed: bool,
|
||||
pub enabled: bool,
|
||||
}
|
||||
@@ -1256,15 +1258,32 @@ impl PluginsManager {
|
||||
};
|
||||
let store = self.store.clone();
|
||||
let codex_home = self.codex_home.clone();
|
||||
let manifest_fallback_contents = resolved
|
||||
.manifest_fallback
|
||||
.contents_if_has_metadata()
|
||||
.map(str::to_string);
|
||||
let result: StorePluginInstallResult = tokio::task::spawn_blocking(move || {
|
||||
let materialized =
|
||||
materialize_marketplace_plugin_source(codex_home.as_path(), &resolved.source)
|
||||
.map_err(PluginStoreError::Invalid)?;
|
||||
let source_path = materialized.path;
|
||||
if let Some(plugin_version) = plugin_version {
|
||||
store.install_with_version(source_path, resolved.plugin_id, plugin_version)
|
||||
} else {
|
||||
store.install(source_path, resolved.plugin_id)
|
||||
match (plugin_version, manifest_fallback_contents.as_deref()) {
|
||||
(Some(plugin_version), Some(manifest_contents)) => store
|
||||
.install_with_version_and_fallback_manifest(
|
||||
source_path,
|
||||
resolved.plugin_id,
|
||||
plugin_version,
|
||||
manifest_contents,
|
||||
),
|
||||
(Some(plugin_version), None) => {
|
||||
store.install_with_version(source_path, resolved.plugin_id, plugin_version)
|
||||
}
|
||||
(None, Some(manifest_contents)) => store.install_with_fallback_manifest(
|
||||
source_path,
|
||||
resolved.plugin_id,
|
||||
manifest_contents,
|
||||
),
|
||||
(None, None) => store.install(source_path, resolved.plugin_id),
|
||||
}
|
||||
})
|
||||
.await
|
||||
@@ -1394,6 +1413,7 @@ impl PluginsManager {
|
||||
let enabled = enabled_plugins.contains(&plugin_key);
|
||||
let mut interface = plugin.interface;
|
||||
let mut local_version = plugin.local_version;
|
||||
let manifest_fallback = plugin.manifest_fallback.clone();
|
||||
if installed
|
||||
&& matches!(&plugin.source, MarketplacePluginSource::Git { .. })
|
||||
&& let Some(plugin_id) = plugin_id.as_ref()
|
||||
@@ -1424,6 +1444,7 @@ impl PluginsManager {
|
||||
policy: plugin.policy,
|
||||
keywords: plugin.keywords,
|
||||
interface,
|
||||
manifest_fallback,
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
@@ -1491,6 +1512,10 @@ impl PluginsManager {
|
||||
|
||||
let marketplace_name = plugin.plugin_id.marketplace_name.clone();
|
||||
let plugin_key = plugin.plugin_id.as_key();
|
||||
let manifest_fallback = plugin
|
||||
.manifest_fallback
|
||||
.contents_if_has_metadata()
|
||||
.map(|_| plugin.manifest_fallback.clone());
|
||||
let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config);
|
||||
let installed = installed_plugins.contains(&plugin_key);
|
||||
let installed_version = if installed {
|
||||
@@ -1518,6 +1543,7 @@ impl PluginsManager {
|
||||
.as_ref()
|
||||
.map(|manifest| manifest.keywords.clone())
|
||||
.unwrap_or_default(),
|
||||
manifest_fallback,
|
||||
installed,
|
||||
enabled: enabled_plugins.contains(&plugin_key),
|
||||
},
|
||||
@@ -1604,9 +1630,18 @@ impl PluginsManager {
|
||||
"path does not exist or is not a directory".to_string(),
|
||||
));
|
||||
}
|
||||
let manifest = load_plugin_manifest(source_path.as_path()).ok_or_else(|| {
|
||||
MarketplaceError::InvalidPlugin("missing or invalid plugin.json".to_string())
|
||||
})?;
|
||||
let manifest =
|
||||
if codex_utils_plugins::find_plugin_manifest_path(source_path.as_path()).is_some() {
|
||||
load_plugin_manifest(source_path.as_path())
|
||||
} else {
|
||||
plugin
|
||||
.manifest_fallback
|
||||
.as_ref()
|
||||
.and_then(|fallback| fallback.parse_for_plugin_root(source_path.as_path()))
|
||||
}
|
||||
.ok_or_else(|| {
|
||||
MarketplaceError::InvalidPlugin("missing or invalid plugin.json".to_string())
|
||||
})?;
|
||||
let description = manifest.description.clone();
|
||||
let marketplace_category = plugin
|
||||
.interface
|
||||
@@ -1637,8 +1672,14 @@ impl PluginsManager {
|
||||
})
|
||||
.collect();
|
||||
let auth_mode = self.auth_mode();
|
||||
let mut app_declarations = load_plugin_apps(source_path.as_path()).await;
|
||||
let mut mcp_servers = load_plugin_mcp_servers(source_path.as_path(), auth_mode).await;
|
||||
let mut app_declarations =
|
||||
load_plugin_apps_from_manifest(source_path.as_path(), &manifest.paths).await;
|
||||
let mut mcp_servers = load_plugin_mcp_servers_from_manifest(
|
||||
source_path.as_path(),
|
||||
&manifest.paths,
|
||||
/*plugin_policy*/ None,
|
||||
)
|
||||
.await;
|
||||
if auth_mode.is_some() {
|
||||
apply_app_mcp_routing_policy(
|
||||
&mut app_declarations,
|
||||
|
||||
@@ -2241,6 +2241,98 @@ async fn install_plugin_uses_manifest_version_for_non_curated_plugins() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin_json() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
let plugin_root = repo_root.join("plugins/quality-review");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
fs::create_dir_all(plugin_root.join("skills/thermo-nuclear-code-quality-review")).unwrap();
|
||||
fs::write(
|
||||
plugin_root.join("skills/thermo-nuclear-code-quality-review/SKILL.md"),
|
||||
"review skill",
|
||||
)
|
||||
.unwrap();
|
||||
fs::write(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "quality-review",
|
||||
"description": "Strict code quality review focused on maintainability.",
|
||||
"source": "./plugins/quality-review",
|
||||
"author": {
|
||||
"name": "Byron Grogan"
|
||||
},
|
||||
"skills": [
|
||||
"./skills/thermo-nuclear-code-quality-review"
|
||||
],
|
||||
"category": "code-review"
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let result = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.install_plugin(PluginInstallRequest {
|
||||
plugin_name: "quality-review".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let installed_path = tmp.path().join("plugins/cache/debug/quality-review/local");
|
||||
assert_eq!(
|
||||
result,
|
||||
PluginInstallOutcome {
|
||||
plugin_id: PluginId::new("quality-review".to_string(), "debug".to_string()).unwrap(),
|
||||
plugin_version: "local".to_string(),
|
||||
installed_path: AbsolutePathBuf::try_from(installed_path.clone()).unwrap(),
|
||||
auth_policy: MarketplacePluginAuthPolicy::OnInstall,
|
||||
}
|
||||
);
|
||||
assert!(!plugin_root.join(".codex-plugin/plugin.json").exists());
|
||||
assert!(
|
||||
!tmp.path()
|
||||
.join("plugins/.marketplace-plugin-source-staging")
|
||||
.exists()
|
||||
);
|
||||
|
||||
let manifest = crate::manifest::load_plugin_manifest(&installed_path).unwrap();
|
||||
assert_eq!(manifest.name, "quality-review");
|
||||
assert_eq!(
|
||||
manifest.description.as_deref(),
|
||||
Some("Strict code quality review focused on maintainability.")
|
||||
);
|
||||
assert_eq!(
|
||||
manifest.paths.skills,
|
||||
vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
installed_path.join("skills/thermo-nuclear-code-quality-review")
|
||||
)
|
||||
.unwrap()
|
||||
]
|
||||
);
|
||||
let interface = manifest.interface.expect("fallback interface");
|
||||
assert_eq!(interface.developer_name.as_deref(), Some("Byron Grogan"));
|
||||
assert_eq!(interface.category.as_deref(), Some("code-review"));
|
||||
let fallback_json: serde_json::Value = serde_json::from_str(
|
||||
&fs::read_to_string(installed_path.join(".codex-plugin/plugin.json")).unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
fallback_json["author"],
|
||||
serde_json::json!({ "name": "Byron Grogan" })
|
||||
);
|
||||
assert_eq!(fallback_json["category"], "code-review");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_plugin_supports_git_subdir_marketplace_sources() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -2484,6 +2576,7 @@ enabled = false
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: true,
|
||||
enabled: true,
|
||||
},
|
||||
@@ -2503,6 +2596,7 @@ enabled = false
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: true,
|
||||
enabled: false,
|
||||
},
|
||||
@@ -2632,6 +2726,7 @@ plugins = true
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: false,
|
||||
}]
|
||||
@@ -2768,6 +2863,142 @@ plugins = true
|
||||
assert!(api_key_outcome.plugin.apps.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_plugin_for_config_uses_marketplace_manifest_fallback_paths_for_local_source() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
let plugin_root = repo_root.join("sample-plugin");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
write_file(
|
||||
&repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "sample-plugin",
|
||||
"source": "./sample-plugin",
|
||||
"apps": "./config/custom.app.json",
|
||||
"mcpServers": {
|
||||
"sample-mcp": {
|
||||
"command": "sample-mcp"
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join("config/custom.app.json"),
|
||||
r#"{"apps":{"sample-app":{"id":"connector_sample"}}}"#,
|
||||
);
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
"#,
|
||||
);
|
||||
|
||||
let config = load_config(tmp.path(), &repo_root).await;
|
||||
let manager = PluginsManager::new(tmp.path().to_path_buf());
|
||||
let outcome = manager
|
||||
.read_plugin_for_config(
|
||||
&config,
|
||||
&PluginReadRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
outcome.plugin.apps,
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.plugin.mcp_server_names,
|
||||
vec!["sample-mcp".to_string()]
|
||||
);
|
||||
|
||||
let listed_plugin = manager
|
||||
.list_marketplaces_for_config(
|
||||
&config,
|
||||
&[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()],
|
||||
/*include_openai_curated*/ false,
|
||||
)
|
||||
.unwrap()
|
||||
.marketplaces
|
||||
.into_iter()
|
||||
.find(|marketplace| marketplace.name == "debug")
|
||||
.unwrap()
|
||||
.plugins
|
||||
.into_iter()
|
||||
.find(|plugin| plugin.name == "sample-plugin")
|
||||
.unwrap();
|
||||
let listed_detail = manager
|
||||
.read_plugin_detail_for_marketplace_plugin(&config, "debug", listed_plugin)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
listed_detail.apps,
|
||||
vec![AppConnectorId("connector_sample".to_string())]
|
||||
);
|
||||
assert_eq!(
|
||||
listed_detail.mcp_server_names,
|
||||
vec!["sample-mcp".to_string()]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_plugin_for_config_does_not_fallback_from_invalid_plugin_manifest() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
let plugin_root = repo_root.join("sample-plugin");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
write_file(
|
||||
&repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "sample-plugin",
|
||||
"source": "./sample-plugin",
|
||||
"description": "Fallback metadata"
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
write_file(&plugin_root.join(".codex-plugin/plugin.json"), "{");
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
"#,
|
||||
);
|
||||
|
||||
let config = load_config(tmp.path(), &repo_root).await;
|
||||
let err = PluginsManager::new(tmp.path().to_path_buf())
|
||||
.read_plugin_for_config(
|
||||
&config,
|
||||
&PluginReadRequest {
|
||||
plugin_name: "sample-plugin".to_string(),
|
||||
marketplace_path: AbsolutePathBuf::try_from(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
)
|
||||
.unwrap(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.to_string(), "missing or invalid plugin.json");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_plugin_for_config_uses_user_layer_skill_settings_only() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -3150,8 +3381,11 @@ enabled = true
|
||||
.find(|marketplace| marketplace.name == "debug")
|
||||
.expect("debug marketplace should be listed");
|
||||
|
||||
let mut plugins = marketplace.plugins;
|
||||
assert!(plugins[0].manifest_fallback.is_some());
|
||||
plugins[0].manifest_fallback = None;
|
||||
assert_eq!(
|
||||
marketplace.plugins,
|
||||
plugins,
|
||||
vec![ConfiguredMarketplacePlugin {
|
||||
id: "toolkit@debug".to_string(),
|
||||
name: "toolkit".to_string(),
|
||||
@@ -3186,6 +3420,7 @@ enabled = true
|
||||
..Default::default()
|
||||
}),
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: true,
|
||||
enabled: true,
|
||||
}]
|
||||
@@ -3266,6 +3501,7 @@ plugins = true
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: false,
|
||||
}],
|
||||
@@ -3388,6 +3624,7 @@ plugins = true
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: false,
|
||||
}],
|
||||
@@ -3815,6 +4052,7 @@ enabled = false
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: true,
|
||||
}]
|
||||
@@ -3847,6 +4085,7 @@ enabled = false
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: false,
|
||||
}]
|
||||
@@ -3937,6 +4176,7 @@ enabled = true
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
installed: false,
|
||||
enabled: true,
|
||||
}],
|
||||
|
||||
@@ -8,6 +8,7 @@ use codex_plugin::PluginIdError;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Map as JsonMap;
|
||||
use serde_json::Value as JsonValue;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
@@ -29,6 +30,7 @@ pub struct ResolvedMarketplacePlugin {
|
||||
pub policy: MarketplacePluginPolicy,
|
||||
pub interface: Option<PluginManifestInterface>,
|
||||
pub manifest: Option<crate::manifest::PluginManifest>,
|
||||
pub manifest_fallback: MarketplacePluginManifestFallback,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -56,6 +58,57 @@ pub struct MarketplaceInterface {
|
||||
pub display_name: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct MarketplacePluginManifestFallback {
|
||||
contents: String,
|
||||
has_metadata: bool,
|
||||
}
|
||||
|
||||
impl MarketplacePluginManifestFallback {
|
||||
pub fn contents(&self) -> &str {
|
||||
&self.contents
|
||||
}
|
||||
|
||||
pub(crate) fn contents_if_has_metadata(&self) -> Option<&str> {
|
||||
self.has_metadata.then_some(self.contents())
|
||||
}
|
||||
|
||||
pub(crate) fn parse_for_plugin_root(
|
||||
&self,
|
||||
plugin_root: &Path,
|
||||
) -> Option<crate::manifest::PluginManifest> {
|
||||
crate::manifest::parse_plugin_manifest(
|
||||
plugin_root,
|
||||
&fallback_plugin_manifest_path(plugin_root),
|
||||
&self.contents,
|
||||
)
|
||||
.ok()
|
||||
}
|
||||
|
||||
pub(crate) fn parse_for_listing(&self) -> Option<crate::manifest::PluginManifest> {
|
||||
// Git sources have no plugin root before install. Parse against a synthetic absolute root,
|
||||
// then discard path-bearing fields so listings expose metadata only.
|
||||
let mut manifest = crate::manifest::parse_plugin_manifest(
|
||||
Path::new("/"),
|
||||
Path::new("/.codex-plugin/plugin.json"),
|
||||
&self.contents,
|
||||
)
|
||||
.ok()?;
|
||||
manifest.paths = crate::manifest::PluginManifestPaths {
|
||||
skills: Vec::new(),
|
||||
mcp_servers: None,
|
||||
apps: None,
|
||||
hooks: None,
|
||||
};
|
||||
if let Some(interface) = manifest.interface.as_mut() {
|
||||
interface.composer_icon = None;
|
||||
interface.logo = None;
|
||||
interface.screenshots.clear();
|
||||
}
|
||||
Some(manifest)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct MarketplacePlugin {
|
||||
pub name: String,
|
||||
@@ -64,6 +117,7 @@ pub struct MarketplacePlugin {
|
||||
pub policy: MarketplacePluginPolicy,
|
||||
pub interface: Option<PluginManifestInterface>,
|
||||
pub keywords: Vec<String>,
|
||||
pub manifest_fallback: Option<MarketplacePluginManifestFallback>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
@@ -313,6 +367,10 @@ pub fn load_marketplace(path: &AbsolutePathBuf) -> Result<Marketplace, Marketpla
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
|
||||
let manifest_fallback = plugin
|
||||
.manifest_fallback
|
||||
.contents_if_has_metadata()
|
||||
.map(|_| plugin.manifest_fallback.clone());
|
||||
let local_version = plugin
|
||||
.manifest
|
||||
.as_ref()
|
||||
@@ -329,6 +387,7 @@ pub fn load_marketplace(path: &AbsolutePathBuf) -> Result<Marketplace, Marketpla
|
||||
policy: plugin.policy,
|
||||
interface: plugin.interface,
|
||||
keywords,
|
||||
manifest_fallback,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -434,13 +493,27 @@ fn resolve_marketplace_plugin_entry(
|
||||
source,
|
||||
policy,
|
||||
category,
|
||||
manifest_fields,
|
||||
} = plugin;
|
||||
let Some(source) = resolve_supported_plugin_source(marketplace_path, &name, source) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let manifest_fallback =
|
||||
marketplace_plugin_manifest_fallback(&name, category.as_deref(), &manifest_fields);
|
||||
|
||||
let manifest = match &source {
|
||||
MarketplacePluginSource::Local { path } => load_plugin_manifest(path.as_path()),
|
||||
MarketplacePluginSource::Local { path } => {
|
||||
if codex_utils_plugins::find_plugin_manifest_path(path.as_path()).is_some() {
|
||||
load_plugin_manifest(path.as_path())
|
||||
} else if manifest_fallback.has_metadata {
|
||||
manifest_fallback.parse_for_plugin_root(path.as_path())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
MarketplacePluginSource::Git { .. } if manifest_fallback.has_metadata => {
|
||||
manifest_fallback.parse_for_listing()
|
||||
}
|
||||
MarketplacePluginSource::Git { .. } => None,
|
||||
};
|
||||
let interface = plugin_interface_with_marketplace_category(
|
||||
@@ -462,6 +535,7 @@ fn resolve_marketplace_plugin_entry(
|
||||
},
|
||||
interface,
|
||||
manifest,
|
||||
manifest_fallback,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -765,6 +839,9 @@ struct RawMarketplaceManifestPlugin {
|
||||
policy: RawMarketplaceManifestPluginPolicy,
|
||||
#[serde(default)]
|
||||
category: Option<String>,
|
||||
#[serde(default)]
|
||||
#[serde(flatten)]
|
||||
manifest_fields: JsonMap<String, JsonValue>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)]
|
||||
@@ -822,6 +899,85 @@ fn resolve_marketplace_interface(
|
||||
}
|
||||
}
|
||||
|
||||
fn fallback_plugin_manifest_path(plugin_root: &Path) -> PathBuf {
|
||||
plugin_root.join(".codex-plugin/plugin.json")
|
||||
}
|
||||
|
||||
fn marketplace_plugin_manifest_fallback(
|
||||
name: &str,
|
||||
category: Option<&str>,
|
||||
manifest_fields: &JsonMap<String, JsonValue>,
|
||||
) -> MarketplacePluginManifestFallback {
|
||||
let mut manifest = manifest_fields.clone();
|
||||
manifest.insert("name".to_string(), JsonValue::String(name.to_string()));
|
||||
if let Some(category) = category {
|
||||
manifest.insert(
|
||||
"category".to_string(),
|
||||
JsonValue::String(category.to_string()),
|
||||
);
|
||||
}
|
||||
if let Some(interface) = plugin_manifest_interface(manifest_fields, category) {
|
||||
manifest.insert("interface".to_string(), interface);
|
||||
}
|
||||
|
||||
let contents = serde_json::to_string_pretty(&JsonValue::Object(manifest))
|
||||
.unwrap_or_else(|_| format!(r#"{{"name":"{name}"}}"#));
|
||||
MarketplacePluginManifestFallback {
|
||||
contents,
|
||||
has_metadata: !manifest_fields.is_empty() || category.is_some(),
|
||||
}
|
||||
}
|
||||
|
||||
fn plugin_manifest_interface(
|
||||
fields: &JsonMap<String, JsonValue>,
|
||||
category: Option<&str>,
|
||||
) -> Option<JsonValue> {
|
||||
let mut interface = fields
|
||||
.get("interface")
|
||||
.and_then(JsonValue::as_object)
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
|
||||
if !interface.contains_key("displayName")
|
||||
&& let Some(display_name) = fields.get("displayName").and_then(JsonValue::as_str)
|
||||
{
|
||||
interface.insert(
|
||||
"displayName".to_string(),
|
||||
JsonValue::String(display_name.to_string()),
|
||||
);
|
||||
}
|
||||
if !interface.contains_key("developerName")
|
||||
&& let Some(author_name) = fields
|
||||
.get("author")
|
||||
.and_then(|author| author.get("name"))
|
||||
.and_then(JsonValue::as_str)
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
{
|
||||
interface.insert(
|
||||
"developerName".to_string(),
|
||||
JsonValue::String(author_name.to_string()),
|
||||
);
|
||||
}
|
||||
if !interface.contains_key("websiteUrl")
|
||||
&& !interface.contains_key("websiteURL")
|
||||
&& let Some(homepage) = fields.get("homepage").and_then(JsonValue::as_str)
|
||||
{
|
||||
interface.insert(
|
||||
"websiteUrl".to_string(),
|
||||
JsonValue::String(homepage.to_string()),
|
||||
);
|
||||
}
|
||||
if let Some(category) = category.map(str::trim).filter(|value| !value.is_empty()) {
|
||||
interface.insert(
|
||||
"category".to_string(),
|
||||
JsonValue::String(category.to_string()),
|
||||
);
|
||||
}
|
||||
|
||||
(!interface.is_empty()).then_some(JsonValue::Object(interface))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "marketplace_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -20,6 +20,17 @@ fn write_alternate_plugin_manifest(plugin_root: &Path, contents: &str) {
|
||||
fs::write(manifest_path, contents).unwrap();
|
||||
}
|
||||
|
||||
fn minimal_manifest_fallback(name: &str) -> MarketplacePluginManifestFallback {
|
||||
MarketplacePluginManifestFallback {
|
||||
contents: format!(
|
||||
r#"{{
|
||||
"name": "{name}"
|
||||
}}"#
|
||||
),
|
||||
has_metadata: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
let tmp = tempdir().unwrap();
|
||||
@@ -65,6 +76,7 @@ fn find_marketplace_plugin_finds_repo_marketplace_plugin() {
|
||||
},
|
||||
interface: None,
|
||||
manifest: None,
|
||||
manifest_fallback: minimal_manifest_fallback("local-plugin"),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -108,6 +120,7 @@ fn find_marketplace_plugin_supports_alternate_layout_and_string_local_source() {
|
||||
},
|
||||
interface: None,
|
||||
manifest: None,
|
||||
manifest_fallback: minimal_manifest_fallback("string-source-plugin"),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -162,10 +175,219 @@ fn find_marketplace_plugin_supports_git_subdir_sources() {
|
||||
},
|
||||
interface: None,
|
||||
manifest: None,
|
||||
manifest_fallback: minimal_manifest_fallback("remote-plugin"),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_marketplace_plugin_builds_manifest_fallback_from_entry() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
let plugin_root = repo_root.join("plugins/quality-review");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
fs::create_dir_all(plugin_root.join("skills/thermo-nuclear-code-quality-review")).unwrap();
|
||||
fs::create_dir_all(plugin_root.join("skills/second-review")).unwrap();
|
||||
fs::write(
|
||||
repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r##"{
|
||||
"name": "team-marketplace",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "quality-review",
|
||||
"version": "1.2.3",
|
||||
"description": "Strict code quality review focused on maintainability.",
|
||||
"displayName": "Quality Review",
|
||||
"source": "./plugins/quality-review",
|
||||
"author": {
|
||||
"name": "Byron Grogan"
|
||||
},
|
||||
"homepage": "https://example.com/quality",
|
||||
"repository": "https://github.com/example/quality-review",
|
||||
"license": "MIT",
|
||||
"skills": [
|
||||
"./skills/thermo-nuclear-code-quality-review",
|
||||
"./skills/second-review"
|
||||
],
|
||||
"commands": ["./commands/review.md"],
|
||||
"mcpServers": {
|
||||
"review": {
|
||||
"type": "stdio",
|
||||
"command": "review-mcp"
|
||||
}
|
||||
},
|
||||
"apps": "./apps/app.json",
|
||||
"hooks": ["./hooks/session.json"],
|
||||
"agents": [
|
||||
"./agents/thermo-nuclear-code-quality-review.md"
|
||||
],
|
||||
"category": "code-review",
|
||||
"keywords": ["quality", "review"],
|
||||
"strict": false,
|
||||
"interface": {
|
||||
"shortDescription": "Interface short description.",
|
||||
"longDescription": "Runs strict reviews focused on maintainability and boundaries.",
|
||||
"category": "interface-category",
|
||||
"capabilities": ["review", "quality"],
|
||||
"privacyPolicyURL": "https://example.com/privacy",
|
||||
"termsOfServiceUrl": "https://example.com/terms",
|
||||
"defaultPrompt": [
|
||||
"Review this change",
|
||||
"Find structural issues"
|
||||
],
|
||||
"brandColor": "#00AAFF",
|
||||
"composerIcon": "./assets/icon.svg",
|
||||
"logo": "./assets/logo.png",
|
||||
"screenshots": ["./assets/shot.png"]
|
||||
}
|
||||
}
|
||||
]
|
||||
}"##,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let resolved = find_marketplace_plugin(
|
||||
&AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(),
|
||||
"quality-review",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let manifest = resolved.manifest.as_ref().expect("fallback manifest");
|
||||
assert_eq!(manifest.name, "quality-review");
|
||||
assert_eq!(manifest.version.as_deref(), Some("1.2.3"));
|
||||
assert_eq!(
|
||||
manifest.description.as_deref(),
|
||||
Some("Strict code quality review focused on maintainability.")
|
||||
);
|
||||
assert_eq!(
|
||||
manifest.paths.skills,
|
||||
vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
plugin_root.join("skills/thermo-nuclear-code-quality-review")
|
||||
)
|
||||
.unwrap(),
|
||||
AbsolutePathBuf::try_from(plugin_root.join("skills/second-review")).unwrap(),
|
||||
]
|
||||
);
|
||||
let Some(crate::manifest::PluginManifestMcpServers::Object(mcp_servers)) =
|
||||
manifest.paths.mcp_servers.as_ref()
|
||||
else {
|
||||
panic!("fallback mcpServers should be inline");
|
||||
};
|
||||
assert_eq!(
|
||||
serde_json::from_str::<JsonValue>(mcp_servers).unwrap(),
|
||||
serde_json::json!({
|
||||
"review": {
|
||||
"type": "stdio",
|
||||
"command": "review-mcp"
|
||||
}
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
manifest.paths.apps.as_ref(),
|
||||
Some(&AbsolutePathBuf::try_from(plugin_root.join("apps/app.json")).unwrap())
|
||||
);
|
||||
assert_eq!(
|
||||
manifest.paths.hooks.as_ref(),
|
||||
Some(&crate::manifest::PluginManifestHooks::Paths(vec![
|
||||
AbsolutePathBuf::try_from(plugin_root.join("hooks/session.json")).unwrap()
|
||||
]))
|
||||
);
|
||||
assert_eq!(manifest.keywords, vec!["quality", "review"]);
|
||||
let interface = manifest.interface.as_ref().expect("fallback interface");
|
||||
assert_eq!(
|
||||
interface,
|
||||
&PluginManifestInterface {
|
||||
display_name: Some("Quality Review".to_string()),
|
||||
short_description: Some("Interface short description.".to_string()),
|
||||
long_description: Some(
|
||||
"Runs strict reviews focused on maintainability and boundaries.".to_string()
|
||||
),
|
||||
developer_name: Some("Byron Grogan".to_string()),
|
||||
category: Some("code-review".to_string()),
|
||||
capabilities: vec!["review".to_string(), "quality".to_string()],
|
||||
website_url: Some("https://example.com/quality".to_string()),
|
||||
privacy_policy_url: Some("https://example.com/privacy".to_string()),
|
||||
terms_of_service_url: Some("https://example.com/terms".to_string()),
|
||||
default_prompt: Some(vec![
|
||||
"Review this change".to_string(),
|
||||
"Find structural issues".to_string()
|
||||
]),
|
||||
brand_color: Some("#00AAFF".to_string()),
|
||||
composer_icon: Some(
|
||||
AbsolutePathBuf::try_from(plugin_root.join("assets/icon.svg")).unwrap()
|
||||
),
|
||||
logo: Some(AbsolutePathBuf::try_from(plugin_root.join("assets/logo.png")).unwrap()),
|
||||
screenshots: vec![
|
||||
AbsolutePathBuf::try_from(plugin_root.join("assets/shot.png")).unwrap()
|
||||
],
|
||||
}
|
||||
);
|
||||
|
||||
let fallback_json: JsonValue =
|
||||
serde_json::from_str(resolved.manifest_fallback.contents()).unwrap();
|
||||
assert_eq!(
|
||||
fallback_json["skills"],
|
||||
serde_json::json!([
|
||||
"./skills/thermo-nuclear-code-quality-review",
|
||||
"./skills/second-review"
|
||||
])
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["mcpServers"],
|
||||
serde_json::json!({
|
||||
"review": {
|
||||
"type": "stdio",
|
||||
"command": "review-mcp"
|
||||
}
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["displayName"],
|
||||
JsonValue::String("Quality Review".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["interface"]["websiteUrl"],
|
||||
JsonValue::String("https://example.com/quality".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["interface"]["privacyPolicyURL"],
|
||||
JsonValue::String("https://example.com/privacy".to_string())
|
||||
);
|
||||
assert!(fallback_json["interface"].get("privacyPolicyUrl").is_none());
|
||||
assert_eq!(
|
||||
fallback_json["author"],
|
||||
serde_json::json!({ "name": "Byron Grogan" })
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["agents"],
|
||||
serde_json::json!(["./agents/thermo-nuclear-code-quality-review.md"])
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["commands"],
|
||||
serde_json::json!(["./commands/review.md"])
|
||||
);
|
||||
assert_eq!(fallback_json["strict"], JsonValue::Bool(false));
|
||||
assert_eq!(
|
||||
fallback_json["homepage"],
|
||||
JsonValue::String("https://example.com/quality".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["repository"],
|
||||
JsonValue::String("https://github.com/example/quality-review".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["license"],
|
||||
JsonValue::String("MIT".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
fallback_json["category"],
|
||||
JsonValue::String("code-review".to_string())
|
||||
);
|
||||
assert!(resolved.manifest_fallback.has_metadata);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_marketplace_plugin_normalizes_github_shorthand_with_dot_git_suffix() {
|
||||
let tmp = tempdir().unwrap();
|
||||
@@ -415,6 +637,7 @@ fn list_marketplaces_supports_alternate_manifest_layout() {
|
||||
screenshots: Vec::new(),
|
||||
}),
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -500,6 +723,7 @@ fn list_marketplaces_supports_repo_root_local_plugin_sources() {
|
||||
screenshots: Vec::new(),
|
||||
}),
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -552,6 +776,7 @@ fn list_marketplaces_includes_plugins_without_discoverable_manifest() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -659,6 +884,7 @@ fn list_marketplaces_supports_explicit_api_marketplace_manifest_path() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -750,6 +976,7 @@ fn list_marketplaces_returns_home_and_repo_marketplaces() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
MarketplacePlugin {
|
||||
name: "home-only".to_string(),
|
||||
@@ -764,6 +991,7 @@ fn list_marketplaces_returns_home_and_repo_marketplaces() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -787,6 +1015,7 @@ fn list_marketplaces_returns_home_and_repo_marketplaces() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
MarketplacePlugin {
|
||||
name: "repo-only".to_string(),
|
||||
@@ -801,6 +1030,7 @@ fn list_marketplaces_returns_home_and_repo_marketplaces() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
],
|
||||
},
|
||||
@@ -880,6 +1110,7 @@ fn list_marketplaces_keeps_distinct_entries_for_same_name() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
},
|
||||
Marketplace {
|
||||
@@ -899,6 +1130,7 @@ fn list_marketplaces_keeps_distinct_entries_for_same_name() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
},
|
||||
]
|
||||
@@ -974,6 +1206,7 @@ fn list_marketplaces_dedupes_multiple_roots_in_same_repo() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -1138,6 +1371,7 @@ fn list_marketplaces_skips_plugins_with_invalid_names_but_keeps_marketplace() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
}],
|
||||
}]
|
||||
);
|
||||
@@ -1220,6 +1454,9 @@ fn list_marketplaces_keeps_remote_and_local_plugin_sources() {
|
||||
},
|
||||
{
|
||||
"name": "git-subdir-plugin",
|
||||
"version": "1.2.3",
|
||||
"displayName": "Git Subdir Plugin",
|
||||
"keywords": ["git", "remote"],
|
||||
"source": {
|
||||
"source": "git-subdir",
|
||||
"url": "owner/repo",
|
||||
@@ -1240,8 +1477,11 @@ fn list_marketplaces_keeps_remote_and_local_plugin_sources() {
|
||||
.marketplaces;
|
||||
|
||||
assert_eq!(marketplaces.len(), 1);
|
||||
let mut plugins = marketplaces[0].plugins.clone();
|
||||
assert!(plugins[2].manifest_fallback.is_some());
|
||||
plugins[2].manifest_fallback = None;
|
||||
assert_eq!(
|
||||
marketplaces[0].plugins,
|
||||
plugins,
|
||||
vec![
|
||||
MarketplacePlugin {
|
||||
name: "local-plugin".to_string(),
|
||||
@@ -1257,6 +1497,7 @@ fn list_marketplaces_keeps_remote_and_local_plugin_sources() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
MarketplacePlugin {
|
||||
name: "url-plugin".to_string(),
|
||||
@@ -1274,10 +1515,11 @@ fn list_marketplaces_keeps_remote_and_local_plugin_sources() {
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
manifest_fallback: None,
|
||||
},
|
||||
MarketplacePlugin {
|
||||
name: "git-subdir-plugin".to_string(),
|
||||
local_version: None,
|
||||
local_version: Some("1.2.3".to_string()),
|
||||
source: MarketplacePluginSource::Git {
|
||||
url: "https://github.com/owner/repo.git".to_string(),
|
||||
path: Some("plugins/example".to_string()),
|
||||
@@ -1289,8 +1531,12 @@ fn list_marketplaces_keeps_remote_and_local_plugin_sources() {
|
||||
authentication: MarketplacePluginAuthPolicy::OnInstall,
|
||||
products: None,
|
||||
},
|
||||
interface: None,
|
||||
keywords: Vec::new(),
|
||||
interface: Some(PluginManifestInterface {
|
||||
display_name: Some("Git Subdir Plugin".to_string()),
|
||||
..Default::default()
|
||||
}),
|
||||
keywords: vec!["git".to_string(), "remote".to_string()],
|
||||
manifest_fallback: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use crate::manifest::PluginManifest;
|
||||
use crate::manifest::load_plugin_manifest;
|
||||
use crate::manifest::parse_plugin_manifest;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -30,6 +31,12 @@ pub struct PluginStore {
|
||||
data_root: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum InstallManifest<'a> {
|
||||
OnDisk,
|
||||
Fallback(&'a str),
|
||||
}
|
||||
|
||||
impl PluginStore {
|
||||
pub fn new(codex_home: PathBuf) -> Self {
|
||||
Self::try_new(codex_home)
|
||||
@@ -104,8 +111,20 @@ impl PluginStore {
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
let plugin_version = plugin_version_for_source(source_path.as_path())?;
|
||||
self.install_with_version(source_path, plugin_id, plugin_version)
|
||||
self.install_with_manifest(source_path, plugin_id, InstallManifest::OnDisk)
|
||||
}
|
||||
|
||||
pub(crate) fn install_with_fallback_manifest(
|
||||
&self,
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
manifest_contents: &str,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
self.install_with_manifest(
|
||||
source_path,
|
||||
plugin_id,
|
||||
InstallManifest::Fallback(manifest_contents),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn install_with_version(
|
||||
@@ -113,6 +132,47 @@ impl PluginStore {
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
plugin_version: String,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
self.install_with_version_and_manifest(
|
||||
source_path,
|
||||
plugin_id,
|
||||
plugin_version,
|
||||
InstallManifest::OnDisk,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn install_with_version_and_fallback_manifest(
|
||||
&self,
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
plugin_version: String,
|
||||
manifest_contents: &str,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
self.install_with_version_and_manifest(
|
||||
source_path,
|
||||
plugin_id,
|
||||
plugin_version,
|
||||
InstallManifest::Fallback(manifest_contents),
|
||||
)
|
||||
}
|
||||
|
||||
fn install_with_manifest(
|
||||
&self,
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
let manifest = resolve_install_manifest(source_path.as_path(), manifest);
|
||||
let plugin_version = plugin_version_for_install_manifest(source_path.as_path(), manifest)?;
|
||||
self.install_with_version_and_manifest(source_path, plugin_id, plugin_version, manifest)
|
||||
}
|
||||
|
||||
fn install_with_version_and_manifest(
|
||||
&self,
|
||||
source_path: AbsolutePathBuf,
|
||||
plugin_id: PluginId,
|
||||
plugin_version: String,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<PluginInstallResult, PluginStoreError> {
|
||||
if !source_path.as_path().is_dir() {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
@@ -121,7 +181,8 @@ impl PluginStore {
|
||||
)));
|
||||
}
|
||||
|
||||
let plugin_name = plugin_name_for_source(source_path.as_path())?;
|
||||
let manifest = resolve_install_manifest(source_path.as_path(), manifest);
|
||||
let plugin_name = plugin_name_for_source(source_path.as_path(), manifest)?;
|
||||
if plugin_name != plugin_id.plugin_name {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
"plugin.json name `{plugin_name}` does not match marketplace plugin name `{}`",
|
||||
@@ -134,6 +195,7 @@ impl PluginStore {
|
||||
source_path.as_path(),
|
||||
self.plugin_base_root(&plugin_id).as_path(),
|
||||
&plugin_version,
|
||||
manifest,
|
||||
)?;
|
||||
|
||||
Ok(PluginInstallResult {
|
||||
@@ -168,7 +230,37 @@ impl PluginStoreError {
|
||||
}
|
||||
|
||||
pub fn plugin_version_for_source(source_path: &Path) -> Result<String, PluginStoreError> {
|
||||
let plugin_version = plugin_manifest_version_for_source(source_path)?
|
||||
plugin_version_for_install_manifest(source_path, InstallManifest::OnDisk)
|
||||
}
|
||||
|
||||
pub(crate) fn plugin_version_for_source_with_fallback_manifest(
|
||||
source_path: &Path,
|
||||
manifest_contents: &str,
|
||||
) -> Result<String, PluginStoreError> {
|
||||
let manifest =
|
||||
resolve_install_manifest(source_path, InstallManifest::Fallback(manifest_contents));
|
||||
plugin_version_for_install_manifest(source_path, manifest)
|
||||
}
|
||||
|
||||
fn resolve_install_manifest<'a>(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'a>,
|
||||
) -> InstallManifest<'a> {
|
||||
// A real plugin manifest always wins. The fallback only fills the gap for marketplace
|
||||
// sources that cannot be changed in place because they may be user-owned directories.
|
||||
match manifest {
|
||||
InstallManifest::Fallback(_) if find_plugin_manifest_path(source_path).is_some() => {
|
||||
InstallManifest::OnDisk
|
||||
}
|
||||
manifest => manifest,
|
||||
}
|
||||
}
|
||||
|
||||
fn plugin_version_for_install_manifest(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<String, PluginStoreError> {
|
||||
let plugin_version = plugin_manifest_version_for_source(source_path, manifest)?
|
||||
.unwrap_or_else(|| DEFAULT_PLUGIN_VERSION.to_string());
|
||||
validate_plugin_version_segment(&plugin_version).map_err(PluginStoreError::Invalid)?;
|
||||
Ok(plugin_version)
|
||||
@@ -193,9 +285,20 @@ pub fn validate_plugin_version_segment(plugin_version: &str) -> Result<(), Strin
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn plugin_manifest_for_source(source_path: &Path) -> Result<PluginManifest, PluginStoreError> {
|
||||
load_plugin_manifest(source_path)
|
||||
.ok_or_else(|| PluginStoreError::Invalid("missing or invalid plugin.json".to_string()))
|
||||
fn plugin_manifest_for_source(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<PluginManifest, PluginStoreError> {
|
||||
match manifest {
|
||||
InstallManifest::OnDisk => load_plugin_manifest(source_path)
|
||||
.ok_or_else(|| PluginStoreError::Invalid("missing or invalid plugin.json".to_string())),
|
||||
InstallManifest::Fallback(contents) => parse_plugin_manifest(
|
||||
source_path,
|
||||
&source_path.join(".codex-plugin/plugin.json"),
|
||||
contents,
|
||||
)
|
||||
.map_err(|err| PluginStoreError::Invalid(format!("failed to parse plugin.json: {err}"))),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
@@ -207,12 +310,17 @@ struct RawPluginManifestVersion {
|
||||
|
||||
fn plugin_manifest_version_for_source(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<Option<String>, PluginStoreError> {
|
||||
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.json", err))?;
|
||||
let contents = match manifest {
|
||||
InstallManifest::OnDisk => {
|
||||
let manifest_path = find_plugin_manifest_path(source_path)
|
||||
.ok_or_else(|| PluginStoreError::Invalid("missing plugin.json".to_string()))?;
|
||||
fs::read_to_string(&manifest_path)
|
||||
.map_err(|err| PluginStoreError::io("failed to read plugin.json", err))?
|
||||
}
|
||||
InstallManifest::Fallback(contents) => contents.to_string(),
|
||||
};
|
||||
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 {
|
||||
@@ -232,8 +340,11 @@ fn plugin_manifest_version_for_source(
|
||||
Ok(Some(version.to_string()))
|
||||
}
|
||||
|
||||
fn plugin_name_for_source(source_path: &Path) -> Result<String, PluginStoreError> {
|
||||
let manifest = plugin_manifest_for_source(source_path)?;
|
||||
fn plugin_name_for_source(
|
||||
source_path: &Path,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<String, PluginStoreError> {
|
||||
let manifest = plugin_manifest_for_source(source_path, manifest)?;
|
||||
|
||||
let plugin_name = manifest.name;
|
||||
validate_plugin_segment(&plugin_name, "plugin name")
|
||||
@@ -261,6 +372,7 @@ fn replace_plugin_root_atomically(
|
||||
source: &Path,
|
||||
target_root: &Path,
|
||||
plugin_version: &str,
|
||||
manifest: InstallManifest<'_>,
|
||||
) -> Result<(), PluginStoreError> {
|
||||
let Some(parent) = target_root.parent() else {
|
||||
return Err(PluginStoreError::Invalid(format!(
|
||||
@@ -287,6 +399,21 @@ fn replace_plugin_root_atomically(
|
||||
let staged_root = staged_dir.path().join(plugin_dir_name);
|
||||
let staged_version_root = staged_root.join(plugin_version);
|
||||
copy_dir_recursive(source, &staged_version_root)?;
|
||||
if let InstallManifest::Fallback(contents) = manifest {
|
||||
// Inject the generated manifest into Store's existing atomic copy so install does not
|
||||
// mutate the original source or require a second staging directory.
|
||||
let manifest_path = staged_version_root.join(".codex-plugin/plugin.json");
|
||||
let Some(manifest_parent) = manifest_path.parent() else {
|
||||
return Err(PluginStoreError::Invalid(
|
||||
"plugin manifest path has no parent".to_string(),
|
||||
));
|
||||
};
|
||||
fs::create_dir_all(manifest_parent).map_err(|err| {
|
||||
PluginStoreError::io("failed to create plugin manifest directory", err)
|
||||
})?;
|
||||
fs::write(&manifest_path, contents)
|
||||
.map_err(|err| PluginStoreError::io("failed to write fallback plugin manifest", err))?;
|
||||
}
|
||||
|
||||
let target_version_root = target_root.join(plugin_version);
|
||||
if target_root.exists() && !target_version_root.exists() {
|
||||
|
||||
@@ -192,7 +192,7 @@ fn install_with_version_uses_requested_cache_version() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_uses_manifest_version_when_present() {
|
||||
fn install_prefers_on_disk_manifest_version_over_fallback() {
|
||||
let tmp = tempdir().unwrap();
|
||||
write_plugin_with_version(
|
||||
tmp.path(),
|
||||
@@ -203,9 +203,10 @@ fn install_uses_manifest_version_when_present() {
|
||||
let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
let result = PluginStore::new(tmp.path().to_path_buf())
|
||||
.install(
|
||||
.install_with_fallback_manifest(
|
||||
AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(),
|
||||
plugin_id.clone(),
|
||||
r#"{"name":"sample-plugin","version":"9.9.9"}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user