mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(plugins): tabulate plugin list output (#23727)
## Summary - render `codex plugin list` as one table per marketplace with the marketplace manifest path shown above each table - surface the installed plugin version in the CLI output by threading `installed_version` through marketplace listing state - narrow the system-root exemption so only known bundled/runtime marketplaces skip missing-manifest failures, and keep `VERSION` empty for cached-but-unconfigured plugins ## Rationale The plugin list UX was hard to scan as a flat list and did not show which installed version was active. This change makes the CLI output easier to read in the real multi-marketplace case, keeps the plugin path visible, fixes the Sapphire regression where bundled/runtime marketplace roots were blocking `plugin list`, and addresses the two review findings that came out of the follow-up deep review. ## Key Decisions - kept the CLI output grouped per marketplace instead of one global table so the marketplace path can live with the rows it owns - kept `VERSION` as the installed version, which means it is empty until a plugin is actually installed - handled the bundled/runtime regression in the CLI snapshot validation path rather than widening app-server protocol or changing marketplace loading behavior - narrowed the exemption to known system marketplace names plus expected system paths, so user-configured marketplaces under those directories still fail loudly - gated `installed_version` on actual installed state so `VERSION` cannot show stale cache state for `not installed` rows ## Validation - `just fmt` - Sapphire: `cargo test -p codex-cli --test plugin_cli` (`14 passed; 0 failed`) - Sapphire smoke test: bundled/runtime roots still work - `cargo run -q -p codex-cli -- plugin add sample@debug` - `cargo run -q -p codex-cli -- plugin list` - verified the bundled/runtime-root scenario no longer errors and shows the expected marketplace table output - Sapphire smoke test: custom marketplace under bundled path still errors - verified `failed to load configured marketplace snapshot(s)` for `custom-marketplace` - Sapphire smoke test: cached-but-unconfigured plugin hides version - verified `sample@debug not installed` renders with an empty `VERSION` column ## Sample Output ```text /tmp/custom-marketplace/plugin.json NAME VERSION STATUS DESCRIPTION sample@debug 1.0.0 enabled Debug sample plugin other@local not installed Local development plugin ```
This commit is contained in:
Casey Chow
@@ -263,6 +263,7 @@ pub struct ConfiguredMarketplacePlugin {
|
||||
pub id: String,
|
||||
pub name: String,
|
||||
pub local_version: Option<String>,
|
||||
pub installed_version: Option<String>,
|
||||
pub source: MarketplacePluginSource,
|
||||
pub policy: MarketplacePluginPolicy,
|
||||
pub interface: Option<PluginManifestInterface>,
|
||||
@@ -1225,15 +1226,21 @@ impl PluginsManager {
|
||||
if !self.restriction_product_matches(plugin.policy.products.as_deref()) {
|
||||
return None;
|
||||
}
|
||||
let plugin_id =
|
||||
PluginId::new(plugin.name.clone(), marketplace_name.clone()).ok();
|
||||
let installed = installed_plugins.contains(&plugin_key);
|
||||
let installed_version = installed.then_some(()).and_then(|_| {
|
||||
plugin_id
|
||||
.as_ref()
|
||||
.and_then(|plugin_id| self.store.active_plugin_version(plugin_id))
|
||||
});
|
||||
let enabled = enabled_plugins.contains(&plugin_key);
|
||||
let mut interface = plugin.interface;
|
||||
let mut local_version = plugin.local_version;
|
||||
if installed
|
||||
&& matches!(&plugin.source, MarketplacePluginSource::Git { .. })
|
||||
&& let Ok(plugin_id) =
|
||||
PluginId::new(plugin.name.clone(), marketplace_name.clone())
|
||||
&& let Some(plugin_root) = self.store.active_plugin_root(&plugin_id)
|
||||
&& let Some(plugin_id) = plugin_id.as_ref()
|
||||
&& let Some(plugin_root) = self.store.active_plugin_root(plugin_id)
|
||||
&& let Some(manifest) = load_plugin_manifest(plugin_root.as_path())
|
||||
{
|
||||
local_version = manifest.version.clone();
|
||||
@@ -1251,6 +1258,7 @@ impl PluginsManager {
|
||||
// plugin entries from duplicate marketplace files intentionally
|
||||
// resolve to the first discovered source.
|
||||
id: plugin_key,
|
||||
installed_version,
|
||||
installed,
|
||||
enabled,
|
||||
name: plugin.name,
|
||||
@@ -1298,6 +1306,12 @@ impl PluginsManager {
|
||||
let marketplace_name = plugin.plugin_id.marketplace_name.clone();
|
||||
let plugin_key = plugin.plugin_id.as_key();
|
||||
let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config);
|
||||
let installed = installed_plugins.contains(&plugin_key);
|
||||
let installed_version = if installed {
|
||||
self.store.active_plugin_version(&plugin.plugin_id)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let plugin = self
|
||||
.read_plugin_detail_for_marketplace_plugin(
|
||||
config,
|
||||
@@ -1309,6 +1323,7 @@ impl PluginsManager {
|
||||
.manifest
|
||||
.as_ref()
|
||||
.and_then(|manifest| manifest.version.clone()),
|
||||
installed_version,
|
||||
source: plugin.source,
|
||||
policy: plugin.policy,
|
||||
interface: plugin.interface,
|
||||
@@ -1317,7 +1332,7 @@ impl PluginsManager {
|
||||
.as_ref()
|
||||
.map(|manifest| manifest.keywords.clone())
|
||||
.unwrap_or_default(),
|
||||
installed: installed_plugins.contains(&plugin_key),
|
||||
installed,
|
||||
enabled: enabled_plugins.contains(&plugin_key),
|
||||
},
|
||||
)
|
||||
|
||||
@@ -1599,6 +1599,7 @@ enabled = false
|
||||
id: "enabled-plugin@debug".to_string(),
|
||||
name: "enabled-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: Some("local".to_string()),
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo/enabled-plugin"))
|
||||
.unwrap(),
|
||||
@@ -1617,6 +1618,7 @@ enabled = false
|
||||
id: "disabled-plugin@debug".to_string(),
|
||||
name: "disabled-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: Some("local".to_string()),
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo/disabled-plugin"),)
|
||||
.unwrap(),
|
||||
@@ -1738,6 +1740,7 @@ plugins = true
|
||||
id: "default-plugin@debug".to_string(),
|
||||
name: "default-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: None,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo/default-plugin")).unwrap(),
|
||||
},
|
||||
@@ -2170,6 +2173,7 @@ enabled = true
|
||||
id: "toolkit@debug".to_string(),
|
||||
name: "toolkit".to_string(),
|
||||
local_version: None,
|
||||
installed_version: Some("local".to_string()),
|
||||
source: MarketplacePluginSource::Git {
|
||||
url: missing_remote_repo_url,
|
||||
path: Some("plugins/toolkit".to_string()),
|
||||
@@ -2287,6 +2291,7 @@ plugins = true
|
||||
id: "linear@openai-curated".to_string(),
|
||||
name: "linear".to_string(),
|
||||
local_version: None,
|
||||
installed_version: None,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(curated_root.join("plugins/linear")).unwrap(),
|
||||
},
|
||||
@@ -2582,6 +2587,7 @@ enabled = false
|
||||
id: "dup-plugin@debug".to_string(),
|
||||
name: "dup-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: None,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo-a/from-a")).unwrap(),
|
||||
},
|
||||
@@ -2613,6 +2619,7 @@ enabled = false
|
||||
id: "b-only-plugin@debug".to_string(),
|
||||
name: "b-only-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: None,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo-b/from-b-only")).unwrap(),
|
||||
},
|
||||
@@ -2698,6 +2705,7 @@ enabled = true
|
||||
id: "sample-plugin@debug".to_string(),
|
||||
name: "sample-plugin".to_string(),
|
||||
local_version: None,
|
||||
installed_version: None,
|
||||
source: MarketplacePluginSource::Local {
|
||||
path: AbsolutePathBuf::try_from(tmp.path().join("repo/sample-plugin")).unwrap(),
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user