[codex] Support local marketplace sources (#17756)

## Summary

- Port marketplace source support into the shared core marketplace-add
flow
- Support local marketplace directory sources
- Support direct `marketplace.json` URL sources
- Persist the new source types in config/schema and cover them in CLI
and app-server tests

## Validation

- `cargo test -p codex-core marketplace_add`
- `cargo test -p codex-cli marketplace_add`
- `cargo test -p codex-app-server marketplace_add`
- `just write-config-schema`
- `just fmt`
- `just fix -p codex-core`
- `just fix -p codex-cli`

## Context

Current `main` moved marketplace-add behavior into shared core code and
still assumed only git-backed sources. This change keeps that structure
but restores support for local directories and direct manifest URLs in
the shared path.
This commit is contained in:
xli-oai
2026-04-14 15:58:14 -07:00
committed by GitHub
Unverified
parent 96254a763a
commit 3cc689fb23
9 changed files with 408 additions and 33 deletions
@@ -1,7 +1,11 @@
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::to_response;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::MarketplaceAddParams;
use codex_app_server_protocol::MarketplaceAddResponse;
use codex_app_server_protocol::RequestId;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use tokio::time::Duration;
use tokio::time::timeout;
@@ -9,8 +13,20 @@ use tokio::time::timeout;
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
#[tokio::test]
async fn marketplace_add_rejects_local_directory_source() -> Result<()> {
async fn marketplace_add_local_directory_source() -> Result<()> {
let codex_home = TempDir::new()?;
let source = codex_home.path().join("marketplace");
std::fs::create_dir_all(source.join(".agents/plugins"))?;
std::fs::create_dir_all(source.join("plugins/sample/.codex-plugin"))?;
std::fs::write(
source.join(".agents/plugins/marketplace.json"),
r#"{"name":"debug","plugins":[]}"#,
)?;
std::fs::write(
source.join("plugins/sample/.codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
)?;
std::fs::write(source.join("plugins/sample/marker.txt"), "local ref")?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
@@ -22,19 +38,24 @@ async fn marketplace_add_rejects_local_directory_source() -> Result<()> {
})
.await?;
let err = timeout(
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let MarketplaceAddResponse {
marketplace_name,
installed_root,
already_added,
} = to_response(response)?;
let expected_root = source.canonicalize()?;
assert_eq!(err.error.code, -32600);
assert!(
err.error.message.contains(
"local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo"
),
"unexpected error: {}",
err.error.message
assert_eq!(marketplace_name, "debug");
assert_eq!(installed_root.as_path(), expected_root.as_path());
assert!(!already_added);
assert_eq!(
std::fs::read_to_string(installed_root.as_path().join("plugins/sample/marker.txt"))?,
"local ref"
);
Ok(())
}
+2 -1
View File
@@ -23,7 +23,8 @@ enum MarketplaceSubcommand {
#[derive(Debug, Parser)]
struct AddMarketplaceArgs {
/// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, or SSH URLs.
/// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, SSH URLs,
/// or local marketplace root directories.
source: String,
/// Git ref to check out. Overrides any @ref or #ref suffix in SOURCE.
+35 -9
View File
@@ -1,6 +1,8 @@
use anyhow::Result;
use codex_config::CONFIG_TOML_FILE;
use codex_core::plugins::marketplace_install_root;
use predicates::str::contains;
use pretty_assertions::assert_eq;
use std::path::Path;
use tempfile::TempDir;
@@ -37,7 +39,7 @@ fn write_marketplace_source(source: &Path, marker: &str) -> Result<()> {
}
#[tokio::test]
async fn marketplace_add_rejects_local_directory_source() -> Result<()> {
async fn marketplace_add_local_directory_source() -> Result<()> {
let codex_home = TempDir::new()?;
let source = TempDir::new()?;
write_marketplace_source(source.path(), "local ref")?;
@@ -48,16 +50,40 @@ async fn marketplace_add_rejects_local_directory_source() -> Result<()> {
.current_dir(source_parent)
.args(["marketplace", "add", source_arg.as_str()])
.assert()
.failure()
.stderr(contains(
"local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo",
));
.success();
assert!(
!marketplace_install_root(codex_home.path())
.join("debug")
.exists()
let installed_root = marketplace_install_root(codex_home.path()).join("debug");
assert!(!installed_root.exists());
let config = std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE))?;
let config: toml::Value = toml::from_str(&config)?;
let expected_source = source.path().canonicalize()?.display().to_string();
assert_eq!(
config["marketplaces"]["debug"]["source_type"].as_str(),
Some("local")
);
assert_eq!(
config["marketplaces"]["debug"]["source"].as_str(),
Some(expected_source.as_str())
);
Ok(())
}
#[tokio::test]
async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> {
let codex_home = TempDir::new()?;
let source = TempDir::new()?;
write_marketplace_source(source.path(), "local ref")?;
let manifest_path = source.path().join(".agents/plugins/marketplace.json");
codex_command(codex_home.path())?
.args(["marketplace", "add", manifest_path.to_str().unwrap()])
.assert()
.failure()
.stderr(contains(
"local marketplace source must be a directory, not a file",
));
Ok(())
}
+1
View File
@@ -632,6 +632,7 @@ pub struct MarketplaceConfig {
#[serde(rename_all = "snake_case")]
pub enum MarketplaceSourceType {
Git,
Local,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)]
+2 -1
View File
@@ -810,7 +810,8 @@
},
"MarketplaceSourceType": {
"enum": [
"git"
"git",
"local"
],
"type": "string"
},
@@ -45,7 +45,11 @@ pub(crate) fn installed_marketplace_roots_from_config(
);
return None;
}
let path = default_install_root.join(marketplace_name);
let path = resolve_configured_marketplace_root(
marketplace_name,
marketplace,
&default_install_root,
)?;
path.join(".agents/plugins/marketplace.json")
.is_file()
.then_some(path)
@@ -55,3 +59,18 @@ pub(crate) fn installed_marketplace_roots_from_config(
roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path()));
roots
}
pub(crate) fn resolve_configured_marketplace_root(
marketplace_name: &str,
marketplace: &toml::Value,
default_install_root: &Path,
) -> Option<PathBuf> {
match marketplace.get("source_type").and_then(toml::Value::as_str) {
Some("local") => marketplace
.get("source")
.and_then(toml::Value::as_str)
.filter(|source| !source.is_empty())
.map(PathBuf::from),
_ => Some(default_install_root.join(marketplace_name)),
}
}
+99 -2
View File
@@ -16,10 +16,12 @@ use install::marketplace_staging_root;
use install::replace_marketplace_root;
use install::safe_marketplace_dir_name;
use metadata::MarketplaceInstallMetadata;
use metadata::find_marketplace_root_by_name;
use metadata::installed_marketplace_root_for_source;
use metadata::record_added_marketplace_entry;
use source::MarketplaceSource;
use source::parse_marketplace_source;
use source::stage_marketplace_source;
use source::validate_marketplace_source_root;
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -102,6 +104,33 @@ where
});
}
if let MarketplaceSource::Local { path } = &source {
let marketplace_name = validate_marketplace_source_root(path)?;
if marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME {
return Err(MarketplaceAddError::InvalidRequest(format!(
"marketplace '{OPENAI_CURATED_MARKETPLACE_NAME}' is reserved and cannot be added from {}",
source.display()
)));
}
if find_marketplace_root_by_name(codex_home, &install_root, &marketplace_name)?.is_some() {
return Err(MarketplaceAddError::InvalidRequest(format!(
"marketplace '{marketplace_name}' is already added from a different source; remove it before adding {}",
source.display()
)));
}
record_added_marketplace_entry(codex_home, &marketplace_name, &install_metadata)?;
return Ok(MarketplaceAddOutcome {
marketplace_name,
source_display: source.display(),
installed_root: AbsolutePathBuf::try_from(path.clone()).map_err(|err| {
MarketplaceAddError::Internal(format!(
"failed to resolve installed marketplace root: {err}"
))
})?,
already_added: false,
});
}
let staging_root = marketplace_staging_root(&install_root);
fs::create_dir_all(&staging_root).map_err(|err| {
MarketplaceAddError::Internal(format!(
@@ -120,8 +149,7 @@ where
})?;
let staged_root = staged_root.keep();
let MarketplaceSource::Git { url, ref_name } = &source;
clone_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?;
stage_marketplace_source(&source, &sparse_paths, &staged_root, clone_source)?;
let marketplace_name = validate_marketplace_source_root(&staged_root)?;
if marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME {
@@ -214,6 +242,75 @@ mod tests {
Ok(())
}
#[test]
fn add_marketplace_sync_installs_local_directory_source_and_updates_config() -> Result<()> {
let codex_home = TempDir::new()?;
let source_root = TempDir::new()?;
write_marketplace_source(source_root.path(), "local copy")?;
let result = add_marketplace_sync_with_cloner(
codex_home.path(),
MarketplaceAddRequest {
source: source_root.path().display().to_string(),
ref_name: None,
sparse_paths: Vec::new(),
},
|_url, _ref_name, _sparse_paths, _destination| {
panic!("git cloner should not be called for local marketplace sources")
},
)?;
let expected_source = source_root.path().canonicalize()?.display().to_string();
assert_eq!(result.marketplace_name, "debug");
assert_eq!(result.source_display, expected_source);
assert_eq!(
result.installed_root.as_path(),
source_root.path().canonicalize()?
);
assert!(!result.already_added);
assert!(
!marketplace_install_root(codex_home.path())
.join("debug")
.exists()
);
let config = fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE))?;
assert!(config.contains("[marketplaces.debug]"));
assert!(config.contains("source_type = \"local\""));
assert!(config.contains(&format!("source = \"{expected_source}\"")));
Ok(())
}
#[test]
fn add_marketplace_sync_treats_existing_local_directory_source_as_already_added() -> Result<()>
{
let codex_home = TempDir::new()?;
let source_root = TempDir::new()?;
write_marketplace_source(source_root.path(), "local copy")?;
let request = MarketplaceAddRequest {
source: source_root.path().display().to_string(),
ref_name: None,
sparse_paths: Vec::new(),
};
let first_result = add_marketplace_sync_with_cloner(codex_home.path(), request.clone(), {
|_url, _ref_name, _sparse_paths, _destination| {
panic!("git cloner should not be called for local marketplace sources")
}
})?;
let second_result = add_marketplace_sync_with_cloner(codex_home.path(), request, {
|_url, _ref_name, _sparse_paths, _destination| {
panic!("git cloner should not be called for local marketplace sources")
}
})?;
assert!(!first_result.already_added);
assert!(second_result.already_added);
assert_eq!(second_result.installed_root, first_result.installed_root);
Ok(())
}
fn write_marketplace_source(source: &Path, marker: &str) -> std::io::Result<()> {
fs::create_dir_all(source.join(".agents/plugins"))?;
fs::create_dir_all(source.join("plugins/sample/.codex-plugin"))?;
@@ -1,5 +1,6 @@
use super::MarketplaceAddError;
use super::MarketplaceSource;
use crate::plugins::installed_marketplaces::resolve_configured_marketplace_root;
use crate::plugins::validate_marketplace_root;
use codex_config::CONFIG_TOML_FILE;
use codex_config::MarketplaceConfigUpdate;
@@ -23,6 +24,9 @@ enum InstalledMarketplaceSource {
ref_name: Option<String>,
sparse_paths: Vec<String>,
},
Local {
path: String,
},
}
pub(super) fn record_added_marketplace_entry(
@@ -77,7 +81,11 @@ pub(super) fn installed_marketplace_root_for_source(
if !install_metadata.matches_config(marketplace) {
continue;
}
let root = install_root.join(marketplace_name);
let Some(root) =
resolve_configured_marketplace_root(marketplace_name, marketplace, install_root)
else {
continue;
};
if validate_marketplace_root(&root).is_ok() {
return Ok(Some(root));
}
@@ -86,6 +94,48 @@ pub(super) fn installed_marketplace_root_for_source(
Ok(None)
}
pub(super) fn find_marketplace_root_by_name(
codex_home: &Path,
install_root: &Path,
marketplace_name: &str,
) -> Result<Option<PathBuf>, MarketplaceAddError> {
let config_path = codex_home.join(CONFIG_TOML_FILE);
let config = match fs::read_to_string(&config_path) {
Ok(config) => config,
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None),
Err(err) => {
return Err(MarketplaceAddError::Internal(format!(
"failed to read user config {}: {err}",
config_path.display()
)));
}
};
let config: toml::Value = toml::from_str(&config).map_err(|err| {
MarketplaceAddError::Internal(format!(
"failed to parse user config {}: {err}",
config_path.display()
))
})?;
let Some(marketplace) = config
.get("marketplaces")
.and_then(toml::Value::as_table)
.and_then(|marketplaces| marketplaces.get(marketplace_name))
else {
return Ok(None);
};
let Some(root) =
resolve_configured_marketplace_root(marketplace_name, marketplace, install_root)
else {
return Ok(None);
};
if validate_marketplace_root(&root).is_ok() {
Ok(Some(root))
} else {
Ok(None)
}
}
impl MarketplaceInstallMetadata {
pub(super) fn from_source(source: &MarketplaceSource, sparse_paths: &[String]) -> Self {
let source = match source {
@@ -94,6 +144,9 @@ impl MarketplaceInstallMetadata {
ref_name: ref_name.clone(),
sparse_paths: sparse_paths.to_vec(),
},
MarketplaceSource::Local { path } => InstalledMarketplaceSource::Local {
path: path.display().to_string(),
},
};
Self { source }
}
@@ -101,24 +154,28 @@ impl MarketplaceInstallMetadata {
fn config_source_type(&self) -> &'static str {
match &self.source {
InstalledMarketplaceSource::Git { .. } => "git",
InstalledMarketplaceSource::Local { .. } => "local",
}
}
fn config_source(&self) -> String {
match &self.source {
InstalledMarketplaceSource::Git { url, .. } => url.clone(),
InstalledMarketplaceSource::Local { path } => path.clone(),
}
}
fn ref_name(&self) -> Option<&str> {
match &self.source {
InstalledMarketplaceSource::Git { ref_name, .. } => ref_name.as_deref(),
InstalledMarketplaceSource::Local { .. } => None,
}
}
fn sparse_paths(&self) -> &[String] {
match &self.source {
InstalledMarketplaceSource::Git { sparse_paths, .. } => sparse_paths,
InstalledMarketplaceSource::Local { .. } => &[],
}
}
@@ -227,4 +284,39 @@ mod tests {
"unexpected error: {err}"
);
}
#[test]
fn installed_marketplace_root_for_source_uses_local_source_root() {
let codex_home = TempDir::new().unwrap();
let install_root = codex_home.path().join("marketplaces");
let source_root = codex_home.path().join("source");
fs::create_dir_all(source_root.join(".agents/plugins")).unwrap();
fs::write(
source_root.join(".agents/plugins/marketplace.json"),
r#"{"name":"debug","plugins":[]}"#,
)
.unwrap();
fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
format!(
"[marketplaces.debug]\nsource_type = \"local\"\nsource = \"{}\"\n",
source_root.display()
),
)
.unwrap();
let source = MarketplaceSource::Local {
path: source_root.clone(),
};
let install_metadata = MarketplaceInstallMetadata::from_source(&source, &[]);
let root = installed_marketplace_root_for_source(
codex_home.path(),
&install_root,
&install_metadata,
)
.unwrap();
assert_eq!(root, Some(source_root));
}
}
@@ -2,6 +2,7 @@ use super::MarketplaceAddError;
use crate::plugins::validate_marketplace_root;
use crate::plugins::validate_plugin_segment;
use std::path::Path;
use std::path::PathBuf;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(super) enum MarketplaceSource {
@@ -9,6 +10,9 @@ pub(super) enum MarketplaceSource {
url: String,
ref_name: Option<String>,
},
Local {
path: PathBuf,
},
}
pub(super) fn parse_marketplace_source(
@@ -26,9 +30,18 @@ pub(super) fn parse_marketplace_source(
let ref_name = explicit_ref.or(parsed_ref);
if looks_like_local_path(&base_source) {
return Err(MarketplaceAddError::InvalidRequest(
"local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo".to_string(),
));
if ref_name.is_some() {
return Err(MarketplaceAddError::InvalidRequest(
"--ref is only supported for git marketplace sources".to_string(),
));
}
let path = resolve_local_source_path(&base_source)?;
if path.is_file() {
return Err(MarketplaceAddError::InvalidRequest(
"local marketplace source must be a directory, not a file".to_string(),
));
}
return Ok(MarketplaceSource::Local { path });
}
if is_ssh_git_url(&base_source) || is_git_url(&base_source) {
@@ -50,6 +63,31 @@ pub(super) fn parse_marketplace_source(
)))
}
pub(super) fn stage_marketplace_source<F>(
source: &MarketplaceSource,
sparse_paths: &[String],
staged_root: &Path,
clone_source: F,
) -> Result<(), MarketplaceAddError>
where
F: Fn(&str, Option<&str>, &[String], &Path) -> Result<(), MarketplaceAddError>,
{
if !sparse_paths.is_empty() && !matches!(source, MarketplaceSource::Git { .. }) {
return Err(MarketplaceAddError::InvalidRequest(
"--sparse is only supported for git marketplace sources".to_string(),
));
}
match source {
MarketplaceSource::Git { url, ref_name } => {
clone_source(url, ref_name.as_deref(), sparse_paths, staged_root)
}
MarketplaceSource::Local { .. } => unreachable!(
"local marketplace sources are added without staging a copied install root"
),
}
}
pub(super) fn validate_marketplace_source_root(root: &Path) -> Result<String, MarketplaceAddError> {
let marketplace_name = validate_marketplace_root(root)
.map_err(|err| MarketplaceAddError::InvalidRequest(err.to_string()))?;
@@ -94,6 +132,38 @@ fn looks_like_local_path(source: &str) -> bool {
|| source == ".."
}
fn resolve_local_source_path(source: &str) -> Result<PathBuf, MarketplaceAddError> {
let path = expand_tilde_path(source);
let path = if path.is_absolute() {
path
} else {
std::env::current_dir()
.map_err(|err| {
MarketplaceAddError::Internal(format!(
"failed to read current working directory for local marketplace source: {err}"
))
})?
.join(path)
};
path.canonicalize().map_err(|err| {
MarketplaceAddError::InvalidRequest(format!(
"failed to resolve local marketplace source {}: {err}",
path.display()
))
})
}
fn expand_tilde_path(source: &str) -> PathBuf {
let Some(rest) = source.strip_prefix("~/") else {
return PathBuf::from(source);
};
let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")) else {
return PathBuf::from(source);
};
PathBuf::from(home).join(rest)
}
fn is_ssh_git_url(source: &str) -> bool {
source.starts_with("ssh://") || source.starts_with("git@") && source.contains(':')
}
@@ -126,6 +196,7 @@ impl MarketplaceSource {
Some(ref_name) => format!("{url}#{ref_name}"),
None => url.clone(),
},
Self::Local { path } => path.display().to_string(),
}
}
}
@@ -134,6 +205,7 @@ impl MarketplaceSource {
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
#[test]
fn github_shorthand_parses_ref_suffix() {
@@ -229,12 +301,57 @@ mod tests {
}
#[test]
fn parse_marketplace_source_rejects_local_directory_source() {
let err = parse_marketplace_source("./marketplace", /*explicit_ref*/ None).unwrap_err();
fn local_path_source_parses() {
let source = parse_marketplace_source(".", /*explicit_ref*/ None).unwrap();
assert_eq!(
err.to_string(),
"local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo"
let MarketplaceSource::Local { path } = source else {
panic!("expected local path source");
};
assert!(path.is_absolute());
}
#[test]
fn local_file_source_is_rejected() {
let tempdir = TempDir::new().unwrap();
let file = tempdir.path().join("marketplace.json");
std::fs::write(&file, "{}").unwrap();
let err =
parse_marketplace_source(file.to_str().unwrap(), /*explicit_ref*/ None).unwrap_err();
assert!(
err.to_string()
.contains("local marketplace source must be a directory, not a file"),
"unexpected error: {err}"
);
}
#[test]
fn non_git_sources_reject_ref_override() {
let err = parse_marketplace_source("./marketplace", Some("main".to_string())).unwrap_err();
assert!(
err.to_string()
.contains("--ref is only supported for git marketplace sources"),
"unexpected error: {err}"
);
}
#[test]
fn non_git_sources_reject_sparse_checkout() {
let path = std::env::current_dir().unwrap();
let err = stage_marketplace_source(
&MarketplaceSource::Local { path },
&["plugins/foo".to_string()],
Path::new("/tmp"),
|_url, _ref_name, _sparse_paths, _staged_root| Ok(()),
)
.unwrap_err();
assert!(
err.to_string()
.contains("--sparse is only supported for git marketplace sources"),
"unexpected error: {err}"
);
}