mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Move marketplace add under plugin command (#18116)
## Summary - move the marketplace add CLI from `codex marketplace add` to `codex plugin marketplace add` - keep marketplace config overrides working through the nested plugin command - reject `--sparse` for local marketplace directory sources before the local-source install path bypasses git-source validation ## Validation - `just fmt` - `git diff --check` - `cargo test -p codex-cli` - `cargo test -p codex-core marketplace_add -- --nocapture` - `cargo test -p codex-core install_plugin_updates_config_with_relative_path_and_plugin_key -- --nocapture` - `xli-test-marketplace-cli` local isolated matrix: `T1`, `L1`-`L10`
This commit is contained in:
committed by
GitHub
Unverified
parent
bf6e7e12aa
commit
5818ed6660
@@ -111,8 +111,8 @@ enum Subcommand {
|
||||
/// Manage external MCP servers for Codex.
|
||||
Mcp(McpCli),
|
||||
|
||||
/// Manage plugin marketplaces for Codex.
|
||||
Marketplace(MarketplaceCli),
|
||||
/// Manage Codex plugins.
|
||||
Plugin(PluginCli),
|
||||
|
||||
/// Start Codex as an MCP server (stdio).
|
||||
McpServer,
|
||||
@@ -170,6 +170,21 @@ enum Subcommand {
|
||||
Features(FeaturesCli),
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct PluginCli {
|
||||
#[clap(flatten)]
|
||||
pub config_overrides: CliConfigOverrides,
|
||||
|
||||
#[command(subcommand)]
|
||||
subcommand: PluginSubcommand,
|
||||
}
|
||||
|
||||
#[derive(Debug, clap::Subcommand)]
|
||||
enum PluginSubcommand {
|
||||
/// Manage plugin marketplaces for Codex.
|
||||
Marketplace(MarketplaceCli),
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct CompletionCommand {
|
||||
/// Shell to generate completions for
|
||||
@@ -726,17 +741,23 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
prepend_config_flags(&mut mcp_cli.config_overrides, root_config_overrides.clone());
|
||||
mcp_cli.run().await?;
|
||||
}
|
||||
Some(Subcommand::Marketplace(mut marketplace_cli)) => {
|
||||
Some(Subcommand::Plugin(plugin_cli)) => {
|
||||
reject_remote_mode_for_subcommand(
|
||||
root_remote.as_deref(),
|
||||
root_remote_auth_token_env.as_deref(),
|
||||
"marketplace",
|
||||
"plugin",
|
||||
)?;
|
||||
prepend_config_flags(
|
||||
&mut marketplace_cli.config_overrides,
|
||||
root_config_overrides.clone(),
|
||||
);
|
||||
marketplace_cli.run().await?;
|
||||
let PluginCli {
|
||||
mut config_overrides,
|
||||
subcommand,
|
||||
} = plugin_cli;
|
||||
prepend_config_flags(&mut config_overrides, root_config_overrides.clone());
|
||||
match subcommand {
|
||||
PluginSubcommand::Marketplace(mut marketplace_cli) => {
|
||||
prepend_config_flags(&mut marketplace_cli.config_overrides, config_overrides);
|
||||
marketplace_cli.run().await?;
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(Subcommand::AppServer(app_server_cli)) => {
|
||||
let AppServerCommand {
|
||||
@@ -1690,6 +1711,35 @@ mod tests {
|
||||
assert!(matches!(cli.subcommand, Some(Subcommand::Responses(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_marketplace_add_parses_under_plugin() {
|
||||
let cli =
|
||||
MultitoolCli::try_parse_from(["codex", "plugin", "marketplace", "add", "owner/repo"])
|
||||
.expect("parse");
|
||||
|
||||
assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_marketplace_upgrade_parses_under_plugin() {
|
||||
let cli =
|
||||
MultitoolCli::try_parse_from(["codex", "plugin", "marketplace", "upgrade", "debug"])
|
||||
.expect("parse");
|
||||
|
||||
assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn marketplace_no_longer_parses_at_top_level() {
|
||||
let add_result =
|
||||
MultitoolCli::try_parse_from(["codex", "marketplace", "add", "owner/repo"]);
|
||||
assert!(add_result.is_err());
|
||||
|
||||
let upgrade_result =
|
||||
MultitoolCli::try_parse_from(["codex", "marketplace", "upgrade", "debug"]);
|
||||
assert!(upgrade_result.is_err());
|
||||
}
|
||||
|
||||
fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo {
|
||||
let token_usage = TokenUsage {
|
||||
output_tokens: 2,
|
||||
|
||||
@@ -48,7 +48,7 @@ async fn marketplace_add_local_directory_source() -> Result<()> {
|
||||
|
||||
codex_command(codex_home.path())?
|
||||
.current_dir(source_parent)
|
||||
.args(["marketplace", "add", source_arg.as_str()])
|
||||
.args(["plugin", "marketplace", "add", source_arg.as_str()])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
@@ -78,7 +78,12 @@ async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> {
|
||||
let manifest_path = source.path().join(".agents/plugins/marketplace.json");
|
||||
|
||||
codex_command(codex_home.path())?
|
||||
.args(["marketplace", "add", manifest_path.to_str().unwrap()])
|
||||
.args([
|
||||
"plugin",
|
||||
"marketplace",
|
||||
"add",
|
||||
manifest_path.to_str().unwrap(),
|
||||
])
|
||||
.assert()
|
||||
.failure()
|
||||
.stderr(contains(
|
||||
@@ -87,3 +92,27 @@ async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn marketplace_add_rejects_sparse_for_local_directory_source() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
write_marketplace_source(source.path(), "local ref")?;
|
||||
|
||||
codex_command(codex_home.path())?
|
||||
.args([
|
||||
"plugin",
|
||||
"marketplace",
|
||||
"add",
|
||||
"--sparse",
|
||||
".agents",
|
||||
source.path().to_str().unwrap(),
|
||||
])
|
||||
.assert()
|
||||
.failure()
|
||||
.stderr(contains(
|
||||
"--sparse is only supported for git marketplace sources",
|
||||
));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
use anyhow::Result;
|
||||
use predicates::str::contains;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn codex_command(codex_home: &Path) -> Result<assert_cmd::Command> {
|
||||
let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?);
|
||||
cmd.env("CODEX_HOME", codex_home);
|
||||
Ok(cmd)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn marketplace_upgrade_runs_under_plugin() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
codex_command(codex_home.path())?
|
||||
.args(["plugin", "marketplace", "upgrade"])
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(contains("No configured Git marketplaces to upgrade."));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn marketplace_upgrade_no_longer_runs_at_top_level() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
codex_command(codex_home.path())?
|
||||
.args(["marketplace", "upgrade"])
|
||||
.assert()
|
||||
.failure()
|
||||
.stderr(contains("unexpected argument 'upgrade' found"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -77,6 +77,11 @@ where
|
||||
sparse_paths,
|
||||
} = request;
|
||||
let source = parse_marketplace_source(&source, ref_name)?;
|
||||
if !sparse_paths.is_empty() && !matches!(source, MarketplaceSource::Git { .. }) {
|
||||
return Err(MarketplaceAddError::InvalidRequest(
|
||||
"--sparse is only supported for git marketplace sources".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let install_root = marketplace_install_root(codex_home);
|
||||
fs::create_dir_all(&install_root).map_err(|err| {
|
||||
@@ -287,6 +292,38 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_marketplace_sync_rejects_sparse_checkout_for_local_directory_source() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source_root = TempDir::new()?;
|
||||
write_marketplace_source(source_root.path(), "local copy")?;
|
||||
|
||||
let err = add_marketplace_sync_with_cloner(
|
||||
codex_home.path(),
|
||||
MarketplaceAddRequest {
|
||||
source: source_root.path().display().to_string(),
|
||||
ref_name: None,
|
||||
sparse_paths: vec![".agents".to_string()],
|
||||
},
|
||||
|_url, _ref_name, _sparse_paths, _destination| {
|
||||
panic!("git cloner should not be called for local marketplace sources")
|
||||
},
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"--sparse is only supported for git marketplace sources"
|
||||
);
|
||||
assert!(
|
||||
!codex_home
|
||||
.path()
|
||||
.join(codex_config::CONFIG_TOML_FILE)
|
||||
.exists()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_marketplace_sync_treats_existing_local_directory_source_as_already_added() -> Result<()>
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user