mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
Fix marketplace auto-upgrade refresh correctness
This commit is contained in:
@@ -102,17 +102,28 @@ struct CachedFeaturedPluginIds {
|
||||
featured_plugin_ids: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
struct NonCuratedCacheRefreshRequest {
|
||||
roots: Vec<AbsolutePathBuf>,
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
enum NonCuratedCacheRefreshMode {
|
||||
IfVersionChanged,
|
||||
ForceReinstall,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct NonCuratedCacheRefreshState {
|
||||
requested_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
last_refreshed_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
requested: Option<NonCuratedCacheRefreshRequest>,
|
||||
last_refreshed: Option<NonCuratedCacheRefreshRequest>,
|
||||
in_flight: bool,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct ConfiguredMarketplaceUpgradeState {
|
||||
in_flight: bool,
|
||||
completed: bool,
|
||||
}
|
||||
|
||||
fn featured_plugin_ids_cache_key(
|
||||
@@ -1111,7 +1122,7 @@ impl PluginsManager {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
if state.completed || state.in_flight {
|
||||
if state.in_flight {
|
||||
return;
|
||||
}
|
||||
state.in_flight = true;
|
||||
@@ -1128,35 +1139,17 @@ impl PluginsManager {
|
||||
.name("plugins-marketplace-auto-upgrade".to_string())
|
||||
.spawn(move || {
|
||||
let outcome = upgrade_configured_git_marketplaces(codex_home.as_path(), &config);
|
||||
let cache_refresh_succeeded = if outcome.upgraded_roots.is_empty() {
|
||||
true
|
||||
} else {
|
||||
match refresh_non_curated_plugin_cache(
|
||||
codex_home.as_path(),
|
||||
if !outcome.upgraded_roots.is_empty() {
|
||||
manager.maybe_start_non_curated_plugin_cache_force_reinstall_for_roots(
|
||||
&outcome.upgraded_roots,
|
||||
) {
|
||||
Ok(cache_refreshed) => {
|
||||
if cache_refreshed {
|
||||
manager.clear_cache();
|
||||
}
|
||||
true
|
||||
}
|
||||
Err(err) => {
|
||||
manager.clear_cache();
|
||||
warn!(
|
||||
"failed to refresh non-curated plugin cache after marketplace auto-upgrade: {err}"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
};
|
||||
);
|
||||
}
|
||||
|
||||
let mut state = match manager.configured_marketplace_upgrade_state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
state.in_flight = false;
|
||||
state.completed = outcome.all_succeeded && cache_refresh_succeeded;
|
||||
})
|
||||
{
|
||||
let mut state = match self.configured_marketplace_upgrade_state.write() {
|
||||
@@ -1171,6 +1164,27 @@ impl PluginsManager {
|
||||
pub fn maybe_start_non_curated_plugin_cache_refresh_for_roots(
|
||||
self: &Arc<Self>,
|
||||
roots: &[AbsolutePathBuf],
|
||||
) {
|
||||
self.maybe_start_non_curated_plugin_cache_refresh_for_roots_with_mode(
|
||||
roots,
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
);
|
||||
}
|
||||
|
||||
fn maybe_start_non_curated_plugin_cache_force_reinstall_for_roots(
|
||||
self: &Arc<Self>,
|
||||
roots: &[AbsolutePathBuf],
|
||||
) {
|
||||
self.maybe_start_non_curated_plugin_cache_refresh_for_roots_with_mode(
|
||||
roots,
|
||||
NonCuratedCacheRefreshMode::ForceReinstall,
|
||||
);
|
||||
}
|
||||
|
||||
fn maybe_start_non_curated_plugin_cache_refresh_for_roots_with_mode(
|
||||
self: &Arc<Self>,
|
||||
roots: &[AbsolutePathBuf],
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
) {
|
||||
let mut roots = roots.to_vec();
|
||||
roots.sort_unstable();
|
||||
@@ -1178,6 +1192,7 @@ impl PluginsManager {
|
||||
if roots.is_empty() {
|
||||
return;
|
||||
}
|
||||
let request = NonCuratedCacheRefreshRequest { roots, mode };
|
||||
|
||||
let should_spawn = {
|
||||
let mut state = match self.non_curated_cache_refresh_state.write() {
|
||||
@@ -1185,13 +1200,25 @@ impl PluginsManager {
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
// Collapse repeated plugin/list requests onto one worker and only queue another pass
|
||||
// when the requested roots set actually changes.
|
||||
if state.requested_roots.as_ref() == Some(&roots)
|
||||
|| (!state.in_flight && state.last_refreshed_roots.as_ref() == Some(&roots))
|
||||
// when the requested roots set actually changes. Forced reinstall requests are not
|
||||
// deduped against the last completed pass because the same marketplace root path can
|
||||
// point at newly activated files after an auto-upgrade.
|
||||
if state.requested.as_ref() == Some(&request)
|
||||
|| (mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& !state.in_flight
|
||||
&& state.last_refreshed.as_ref() == Some(&request))
|
||||
{
|
||||
return;
|
||||
}
|
||||
state.requested_roots = Some(roots);
|
||||
if mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& state.requested.as_ref().is_some_and(|requested| {
|
||||
requested.mode == NonCuratedCacheRefreshMode::ForceReinstall
|
||||
&& requested.roots == request.roots
|
||||
})
|
||||
{
|
||||
return;
|
||||
}
|
||||
state.requested = Some(request);
|
||||
if state.in_flight {
|
||||
false
|
||||
} else {
|
||||
@@ -1213,7 +1240,7 @@ impl PluginsManager {
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
state.in_flight = false;
|
||||
state.requested_roots = None;
|
||||
state.requested = None;
|
||||
warn!("failed to start non-curated plugin cache refresh task: {err}");
|
||||
}
|
||||
}
|
||||
@@ -1267,15 +1294,15 @@ impl PluginsManager {
|
||||
|
||||
fn run_non_curated_plugin_cache_refresh_loop(self: Arc<Self>) {
|
||||
loop {
|
||||
let roots = {
|
||||
let request = {
|
||||
let state = match self.non_curated_cache_refresh_state.read() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
state.requested_roots.clone()
|
||||
state.requested.clone()
|
||||
};
|
||||
|
||||
let Some(roots) = roots else {
|
||||
let Some(request) = request else {
|
||||
let mut state = match self.non_curated_cache_refresh_state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
@@ -1284,30 +1311,33 @@ impl PluginsManager {
|
||||
return;
|
||||
};
|
||||
|
||||
let refreshed =
|
||||
match refresh_non_curated_plugin_cache(self.codex_home.as_path(), &roots) {
|
||||
Ok(cache_refreshed) => {
|
||||
if cache_refreshed {
|
||||
self.clear_cache();
|
||||
}
|
||||
true
|
||||
}
|
||||
Err(err) => {
|
||||
let refreshed = match refresh_non_curated_plugin_cache(
|
||||
self.codex_home.as_path(),
|
||||
&request.roots,
|
||||
request.mode,
|
||||
) {
|
||||
Ok(cache_refreshed) => {
|
||||
if cache_refreshed {
|
||||
self.clear_cache();
|
||||
warn!("failed to refresh non-curated plugin cache: {err}");
|
||||
false
|
||||
}
|
||||
};
|
||||
true
|
||||
}
|
||||
Err(err) => {
|
||||
self.clear_cache();
|
||||
warn!("failed to refresh non-curated plugin cache: {err}");
|
||||
false
|
||||
}
|
||||
};
|
||||
|
||||
let mut state = match self.non_curated_cache_refresh_state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
if refreshed {
|
||||
state.last_refreshed_roots = Some(roots.clone());
|
||||
state.last_refreshed = Some(request.clone());
|
||||
}
|
||||
if state.requested_roots.as_ref() == Some(&roots) {
|
||||
state.requested_roots = None;
|
||||
if state.requested.as_ref() == Some(&request) {
|
||||
state.requested = None;
|
||||
state.in_flight = false;
|
||||
return;
|
||||
}
|
||||
@@ -1557,6 +1587,7 @@ fn refresh_curated_plugin_cache(
|
||||
fn refresh_non_curated_plugin_cache(
|
||||
codex_home: &Path,
|
||||
additional_roots: &[AbsolutePathBuf],
|
||||
mode: NonCuratedCacheRefreshMode,
|
||||
) -> Result<bool, String> {
|
||||
let configured_non_curated_plugin_ids =
|
||||
non_curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home(
|
||||
@@ -1625,7 +1656,9 @@ fn refresh_non_curated_plugin_cache(
|
||||
continue;
|
||||
};
|
||||
|
||||
if store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) {
|
||||
if mode == NonCuratedCacheRefreshMode::IfVersionChanged
|
||||
&& store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -2562,6 +2562,7 @@ enabled = true
|
||||
refresh_non_curated_plugin_cache(
|
||||
tmp.path(),
|
||||
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
)
|
||||
.expect("cache refresh should succeed")
|
||||
);
|
||||
@@ -2614,6 +2615,7 @@ enabled = true
|
||||
refresh_non_curated_plugin_cache(
|
||||
tmp.path(),
|
||||
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
)
|
||||
.expect("cache refresh should reinstall missing configured plugin")
|
||||
);
|
||||
@@ -2667,11 +2669,75 @@ enabled = true
|
||||
!refresh_non_curated_plugin_cache(
|
||||
tmp.path(),
|
||||
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
)
|
||||
.expect("cache refresh should be a no-op when configured plugins are current")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_non_curated_plugin_cache_force_reinstalls_current_local_version() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(repo_root.join(".git")).unwrap();
|
||||
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
|
||||
write_plugin(&repo_root, "sample-plugin", "sample-plugin");
|
||||
fs::write(repo_root.join("sample-plugin/skills/SKILL.md"), "new skill").unwrap();
|
||||
write_file(
|
||||
&repo_root.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "sample-plugin",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./sample-plugin"
|
||||
}
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
write_plugin(
|
||||
&tmp.path().join("plugins/cache/debug"),
|
||||
"sample-plugin/local",
|
||||
"sample-plugin",
|
||||
);
|
||||
fs::write(
|
||||
tmp.path()
|
||||
.join("plugins/cache/debug/sample-plugin/local/skills/SKILL.md"),
|
||||
"old skill",
|
||||
)
|
||||
.unwrap();
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[plugins."sample-plugin@debug"]
|
||||
enabled = true
|
||||
"#,
|
||||
);
|
||||
|
||||
assert!(
|
||||
refresh_non_curated_plugin_cache(
|
||||
tmp.path(),
|
||||
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
|
||||
NonCuratedCacheRefreshMode::ForceReinstall,
|
||||
)
|
||||
.expect("cache refresh should reinstall unchanged local version")
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
fs::read_to_string(
|
||||
tmp.path()
|
||||
.join("plugins/cache/debug/sample-plugin/local/skills/SKILL.md")
|
||||
)
|
||||
.unwrap(),
|
||||
"new skill"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_non_curated_plugin_cache_ignores_invalid_unconfigured_plugin_versions() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -2716,6 +2782,7 @@ enabled = true
|
||||
refresh_non_curated_plugin_cache(
|
||||
tmp.path(),
|
||||
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
|
||||
NonCuratedCacheRefreshMode::IfVersionChanged,
|
||||
)
|
||||
.expect("cache refresh should ignore unrelated invalid plugin manifests")
|
||||
);
|
||||
|
||||
@@ -5,7 +5,11 @@ use codex_config::record_user_marketplace;
|
||||
use codex_config::types::MarketplaceConfig;
|
||||
use codex_config::types::MarketplaceSourceType;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use std::process::Output;
|
||||
use std::process::Stdio;
|
||||
@@ -13,9 +17,11 @@ use std::time::Duration;
|
||||
use tempfile::TempDir;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::Config;
|
||||
|
||||
const MARKETPLACE_UPGRADE_GIT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const MARKETPLACE_INSTALL_METADATA_FILE: &str = ".codex-marketplace-install.json";
|
||||
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
pub(super) struct ConfiguredMarketplaceUpgradeOutcome {
|
||||
@@ -32,6 +38,16 @@ struct ConfiguredGitMarketplace {
|
||||
last_revision: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
struct ActivatedMarketplaceMetadata {
|
||||
source_type: MarketplaceSourceType,
|
||||
source: String,
|
||||
ref_name: Option<String>,
|
||||
sparse_paths: Vec<String>,
|
||||
revision: String,
|
||||
}
|
||||
|
||||
pub(super) fn upgrade_configured_git_marketplaces(
|
||||
codex_home: &Path,
|
||||
config: &Config,
|
||||
@@ -78,7 +94,7 @@ fn configured_git_marketplaces(config: &Config) -> Vec<ConfiguredGitMarketplace>
|
||||
};
|
||||
let marketplaces = match marketplaces_value
|
||||
.clone()
|
||||
.try_into::<std::collections::HashMap<String, MarketplaceConfig>>()
|
||||
.try_into::<HashMap<String, MarketplaceConfig>>()
|
||||
{
|
||||
Ok(marketplaces) => marketplaces,
|
||||
Err(err) => {
|
||||
@@ -89,38 +105,43 @@ fn configured_git_marketplaces(config: &Config) -> Vec<ConfiguredGitMarketplace>
|
||||
|
||||
let mut configured = marketplaces
|
||||
.into_iter()
|
||||
.filter_map(|(name, marketplace)| {
|
||||
let MarketplaceConfig {
|
||||
last_updated: _,
|
||||
last_revision,
|
||||
source_type,
|
||||
source,
|
||||
ref_name,
|
||||
sparse_paths,
|
||||
} = marketplace;
|
||||
if source_type != Some(MarketplaceSourceType::Git) {
|
||||
return None;
|
||||
}
|
||||
let Some(source) = source else {
|
||||
warn!(
|
||||
marketplace = name,
|
||||
"ignoring configured Git marketplace without source"
|
||||
);
|
||||
return None;
|
||||
};
|
||||
Some(ConfiguredGitMarketplace {
|
||||
name,
|
||||
source,
|
||||
ref_name,
|
||||
sparse_paths: sparse_paths.unwrap_or_default(),
|
||||
last_revision,
|
||||
})
|
||||
})
|
||||
.filter_map(|(name, marketplace)| configured_git_marketplace_from_config(name, marketplace))
|
||||
.collect::<Vec<_>>();
|
||||
configured.sort_unstable_by(|left, right| left.name.cmp(&right.name));
|
||||
configured
|
||||
}
|
||||
|
||||
fn configured_git_marketplace_from_config(
|
||||
name: String,
|
||||
marketplace: MarketplaceConfig,
|
||||
) -> Option<ConfiguredGitMarketplace> {
|
||||
let MarketplaceConfig {
|
||||
last_updated: _,
|
||||
last_revision,
|
||||
source_type,
|
||||
source,
|
||||
ref_name,
|
||||
sparse_paths,
|
||||
} = marketplace;
|
||||
if source_type != Some(MarketplaceSourceType::Git) {
|
||||
return None;
|
||||
}
|
||||
let Some(source) = source else {
|
||||
warn!(
|
||||
marketplace = name,
|
||||
"ignoring configured Git marketplace without source"
|
||||
);
|
||||
return None;
|
||||
};
|
||||
Some(ConfiguredGitMarketplace {
|
||||
name,
|
||||
source,
|
||||
ref_name,
|
||||
sparse_paths: sparse_paths.unwrap_or_default(),
|
||||
last_revision,
|
||||
})
|
||||
}
|
||||
|
||||
fn upgrade_configured_git_marketplace(
|
||||
codex_home: &Path,
|
||||
install_root: &Path,
|
||||
@@ -137,6 +158,7 @@ fn upgrade_configured_git_marketplace(
|
||||
.join(".agents/plugins/marketplace.json")
|
||||
.is_file()
|
||||
&& marketplace.last_revision.as_deref() == Some(remote_revision.as_str())
|
||||
&& activated_marketplace_metadata_matches(&destination, marketplace, &remote_revision)
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
@@ -173,6 +195,7 @@ fn upgrade_configured_git_marketplace(
|
||||
marketplace.name
|
||||
));
|
||||
}
|
||||
write_activated_marketplace_metadata(staged_dir.path(), marketplace, &remote_revision)?;
|
||||
|
||||
let last_updated = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true);
|
||||
let update = MarketplaceConfigUpdate {
|
||||
@@ -184,6 +207,7 @@ fn upgrade_configured_git_marketplace(
|
||||
sparse_paths: &marketplace.sparse_paths,
|
||||
};
|
||||
activate_marketplace_root(&destination, staged_dir, || {
|
||||
ensure_configured_git_marketplace_unchanged(codex_home, marketplace)?;
|
||||
record_user_marketplace(codex_home, &marketplace.name, &update).map_err(|err| {
|
||||
format!(
|
||||
"failed to record upgraded marketplace `{}` in user config.toml: {err}",
|
||||
@@ -197,6 +221,113 @@ fn upgrade_configured_git_marketplace(
|
||||
.map_err(|err| format!("upgraded marketplace path is not absolute: {err}"))
|
||||
}
|
||||
|
||||
fn activated_marketplace_metadata_matches(
|
||||
root: &Path,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
) -> bool {
|
||||
let metadata = match std::fs::read_to_string(activated_marketplace_metadata_path(root)) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(_) => return false,
|
||||
};
|
||||
let metadata = match serde_json::from_str::<ActivatedMarketplaceMetadata>(&metadata) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(err) => {
|
||||
warn!(
|
||||
marketplace = marketplace.name,
|
||||
error = %err,
|
||||
"failed to parse activated marketplace metadata"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
metadata == activated_marketplace_metadata(marketplace, revision)
|
||||
}
|
||||
|
||||
fn write_activated_marketplace_metadata(
|
||||
root: &Path,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
) -> Result<(), String> {
|
||||
let metadata = activated_marketplace_metadata(marketplace, revision);
|
||||
let contents = serde_json::to_string_pretty(&metadata)
|
||||
.map_err(|err| format!("failed to serialize activated marketplace metadata: {err}"))?;
|
||||
std::fs::write(activated_marketplace_metadata_path(root), contents)
|
||||
.map_err(|err| format!("failed to write activated marketplace metadata: {err}"))
|
||||
}
|
||||
|
||||
fn activated_marketplace_metadata(
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
) -> ActivatedMarketplaceMetadata {
|
||||
ActivatedMarketplaceMetadata {
|
||||
source_type: MarketplaceSourceType::Git,
|
||||
source: marketplace.source.clone(),
|
||||
ref_name: marketplace.ref_name.clone(),
|
||||
sparse_paths: marketplace.sparse_paths.clone(),
|
||||
revision: revision.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn activated_marketplace_metadata_path(root: &Path) -> PathBuf {
|
||||
root.join(MARKETPLACE_INSTALL_METADATA_FILE)
|
||||
}
|
||||
|
||||
fn ensure_configured_git_marketplace_unchanged(
|
||||
codex_home: &Path,
|
||||
expected: &ConfiguredGitMarketplace,
|
||||
) -> Result<(), String> {
|
||||
let current = read_configured_git_marketplace(codex_home, &expected.name)?;
|
||||
match current {
|
||||
Some(current) if current == *expected => Ok(()),
|
||||
Some(_) => Err(format!(
|
||||
"configured marketplace `{}` changed while auto-upgrade was in flight",
|
||||
expected.name
|
||||
)),
|
||||
None => Err(format!(
|
||||
"configured marketplace `{}` was removed or is no longer a Git marketplace",
|
||||
expected.name
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn read_configured_git_marketplace(
|
||||
codex_home: &Path,
|
||||
marketplace_name: &str,
|
||||
) -> Result<Option<ConfiguredGitMarketplace>, String> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let raw_config = match std::fs::read_to_string(&config_path) {
|
||||
Ok(raw_config) => raw_config,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
|
||||
Err(err) => {
|
||||
return Err(format!(
|
||||
"failed to read user config {} while checking marketplace auto-upgrade: {err}",
|
||||
config_path.display()
|
||||
));
|
||||
}
|
||||
};
|
||||
let config: toml::Value = toml::from_str(&raw_config).map_err(|err| {
|
||||
format!(
|
||||
"failed to parse user config {} while checking marketplace auto-upgrade: {err}",
|
||||
config_path.display()
|
||||
)
|
||||
})?;
|
||||
let Some(marketplaces_value) = config.get("marketplaces") else {
|
||||
return Ok(None);
|
||||
};
|
||||
let mut marketplaces = marketplaces_value
|
||||
.clone()
|
||||
.try_into::<HashMap<String, MarketplaceConfig>>()
|
||||
.map_err(|err| format!("invalid marketplaces config while checking auto-upgrade: {err}"))?;
|
||||
let Some(marketplace) = marketplaces.remove(marketplace_name) else {
|
||||
return Ok(None);
|
||||
};
|
||||
Ok(configured_git_marketplace_from_config(
|
||||
marketplace_name.to_string(),
|
||||
marketplace,
|
||||
))
|
||||
}
|
||||
|
||||
fn git_remote_revision(
|
||||
source: &str,
|
||||
ref_name: Option<&str>,
|
||||
@@ -210,11 +341,7 @@ fn git_remote_revision(
|
||||
|
||||
let ref_name = ref_name.unwrap_or("HEAD");
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new("git")
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("ls-remote")
|
||||
.arg(source)
|
||||
.arg(ref_name),
|
||||
git_command().arg("ls-remote").arg(source).arg(ref_name),
|
||||
"git ls-remote marketplace source",
|
||||
timeout,
|
||||
)?;
|
||||
@@ -249,19 +376,14 @@ fn clone_git_source(
|
||||
) -> Result<(), String> {
|
||||
if sparse_paths.is_empty() {
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new("git")
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("clone")
|
||||
.arg(source)
|
||||
.arg(destination),
|
||||
git_command().arg("clone").arg(source).arg(destination),
|
||||
"git clone marketplace source",
|
||||
timeout,
|
||||
)?;
|
||||
ensure_git_success(&output, "git clone marketplace source")?;
|
||||
if let Some(ref_name) = ref_name {
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new("git")
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
git_command()
|
||||
.arg("-C")
|
||||
.arg(destination)
|
||||
.arg("checkout")
|
||||
@@ -275,8 +397,7 @@ fn clone_git_source(
|
||||
}
|
||||
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new("git")
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
git_command()
|
||||
.arg("clone")
|
||||
.arg("--filter=blob:none")
|
||||
.arg("--no-checkout")
|
||||
@@ -287,9 +408,8 @@ fn clone_git_source(
|
||||
)?;
|
||||
ensure_git_success(&output, "git clone marketplace source")?;
|
||||
|
||||
let mut sparse_checkout = Command::new("git");
|
||||
let mut sparse_checkout = git_command();
|
||||
sparse_checkout
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("-C")
|
||||
.arg(destination)
|
||||
.arg("sparse-checkout")
|
||||
@@ -303,8 +423,7 @@ fn clone_git_source(
|
||||
ensure_git_success(&output, "git sparse-checkout marketplace source")?;
|
||||
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new("git")
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
git_command()
|
||||
.arg("-C")
|
||||
.arg(destination)
|
||||
.arg("checkout")
|
||||
@@ -315,6 +434,14 @@ fn clone_git_source(
|
||||
ensure_git_success(&output, "git checkout marketplace ref")
|
||||
}
|
||||
|
||||
fn git_command() -> Command {
|
||||
let mut command = Command::new("git");
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.env("GIT_TERMINAL_PROMPT", "0");
|
||||
command
|
||||
}
|
||||
|
||||
fn activate_marketplace_root(
|
||||
destination: &Path,
|
||||
staged_dir: TempDir,
|
||||
@@ -522,6 +649,7 @@ mod tests {
|
||||
let revision = git_output(source_repo.path(), &["rev-parse", "HEAD"]);
|
||||
let installed_root = marketplace_install_root(codex_home.path()).join("debug");
|
||||
write_marketplace_repo(&installed_root, "debug", "old");
|
||||
write_installed_metadata(&installed_root, source_repo.path(), None, &[], &revision);
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
&marketplace_config(source_repo.path(), &revision),
|
||||
@@ -543,6 +671,42 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upgrade_configured_git_marketplace_reclones_when_install_metadata_differs() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let source_repo = TempDir::new().unwrap();
|
||||
write_marketplace_repo(source_repo.path(), "debug", "new");
|
||||
init_git_repo(source_repo.path());
|
||||
let revision = git_output(source_repo.path(), &["rev-parse", "HEAD"]);
|
||||
let installed_root = marketplace_install_root(codex_home.path()).join("debug");
|
||||
write_marketplace_repo(&installed_root, "debug", "old");
|
||||
write_installed_metadata(&installed_root, source_repo.path(), None, &[], &revision);
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
&marketplace_config_with_ref(source_repo.path(), &revision, &revision),
|
||||
);
|
||||
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config);
|
||||
|
||||
assert_eq!(
|
||||
outcome,
|
||||
ConfiguredMarketplaceUpgradeOutcome {
|
||||
upgraded_roots: vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
marketplace_install_root(codex_home.path()).join("debug")
|
||||
)
|
||||
.unwrap()
|
||||
],
|
||||
all_succeeded: true,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt")).unwrap(),
|
||||
"new"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upgrade_configured_git_marketplace_keeps_existing_root_on_name_mismatch() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
@@ -599,6 +763,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upgrade_configured_git_marketplace_rolls_back_when_config_changes() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let source_repo = TempDir::new().unwrap();
|
||||
write_marketplace_repo(source_repo.path(), "debug", "new");
|
||||
init_git_repo(source_repo.path());
|
||||
let changed_source_repo = TempDir::new().unwrap();
|
||||
write_marketplace_repo(changed_source_repo.path(), "debug", "changed");
|
||||
init_git_repo(changed_source_repo.path());
|
||||
let installed_root = marketplace_install_root(codex_home.path()).join("debug");
|
||||
write_marketplace_repo(&installed_root, "debug", "old");
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
&marketplace_config(source_repo.path(), "old-revision"),
|
||||
);
|
||||
let config = load_plugins_config(codex_home.path()).await;
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
&marketplace_config(changed_source_repo.path(), "changed-revision"),
|
||||
);
|
||||
|
||||
let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config);
|
||||
|
||||
assert_eq!(
|
||||
outcome,
|
||||
ConfiguredMarketplaceUpgradeOutcome {
|
||||
upgraded_roots: Vec::new(),
|
||||
all_succeeded: false,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt")).unwrap(),
|
||||
"old"
|
||||
);
|
||||
let config = std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).unwrap();
|
||||
assert!(config.contains(&changed_source_repo.path().display().to_string()));
|
||||
assert!(config.contains(r#"last_revision = "changed-revision""#));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upgrade_configured_git_marketplaces_ignores_local_unconfigured_marketplace() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
@@ -649,6 +852,44 @@ source = "{}"
|
||||
)
|
||||
}
|
||||
|
||||
fn marketplace_config_with_ref(
|
||||
source_repo: &Path,
|
||||
last_revision: &str,
|
||||
ref_name: &str,
|
||||
) -> String {
|
||||
format!(
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[marketplaces.debug]
|
||||
last_updated = "2026-04-10T00:00:00Z"
|
||||
last_revision = "{last_revision}"
|
||||
source_type = "git"
|
||||
source = "{}"
|
||||
ref = "{ref_name}"
|
||||
"#,
|
||||
source_repo.display()
|
||||
)
|
||||
}
|
||||
|
||||
fn write_installed_metadata(
|
||||
root: &Path,
|
||||
source_repo: &Path,
|
||||
ref_name: Option<&str>,
|
||||
sparse_paths: &[String],
|
||||
revision: &str,
|
||||
) {
|
||||
let marketplace = ConfiguredGitMarketplace {
|
||||
name: "debug".to_string(),
|
||||
source: source_repo.display().to_string(),
|
||||
ref_name: ref_name.map(str::to_string),
|
||||
sparse_paths: sparse_paths.to_vec(),
|
||||
last_revision: Some(revision.to_string()),
|
||||
};
|
||||
write_activated_marketplace_metadata(root, &marketplace, revision)
|
||||
.expect("metadata should write");
|
||||
}
|
||||
|
||||
fn write_marketplace_repo(root: &Path, marketplace_name: &str, marker: &str) {
|
||||
write_file(
|
||||
&root.join(".agents/plugins/marketplace.json"),
|
||||
|
||||
Reference in New Issue
Block a user