mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Keep marketplace upgrade state out of config (#39595)
## What changed - Stop writing `last_updated` and `last_revision` to marketplace entries in `config.toml`; keep the activated revision in `.codex-marketplace-install.json` instead. - Use installed marketplace metadata to detect up-to-date checkouts. - Snapshot the installed marketplace before activation and roll back a stale upgrade if another installation changed the destination concurrently. ## Testing - Verify marketplace add and upgrade operations leave `config.toml` unchanged. - Cover restoring a newer concurrently installed marketplace when a stale activation is rejected. GitOrigin-RevId: 10942c3fc7c6c3f68b7d5953e8c1b5e86bf71866
This commit is contained in:
committed by
copyberry
parent
1bfabb21fe
commit
8f4a48a6ad
@@ -18,8 +18,6 @@ const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
|
||||
fn configured_marketplace_update() -> MarketplaceConfigUpdate<'static> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
|
||||
@@ -69,12 +69,9 @@ fn commit_marketplace_marker(root: &Path, marker: &str) -> Result<String> {
|
||||
|
||||
fn configured_git_marketplace_update<'a>(
|
||||
source: &'a str,
|
||||
last_revision: Option<&'a str>,
|
||||
ref_name: Option<&'a str>,
|
||||
) -> MarketplaceConfigUpdate<'a> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision,
|
||||
source_type: "git",
|
||||
source,
|
||||
ref_name,
|
||||
@@ -84,8 +81,6 @@ fn configured_git_marketplace_update<'a>(
|
||||
|
||||
fn configured_local_marketplace_update(source: &str) -> MarketplaceConfigUpdate<'_> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "local",
|
||||
source,
|
||||
ref_name: None,
|
||||
@@ -97,14 +92,13 @@ fn record_git_marketplace(
|
||||
codex_home: &Path,
|
||||
marketplace_name: &str,
|
||||
source: &Path,
|
||||
last_revision: &str,
|
||||
ref_name: Option<&str>,
|
||||
) -> Result<()> {
|
||||
let source = source.display().to_string();
|
||||
record_user_marketplace(
|
||||
codex_home,
|
||||
marketplace_name,
|
||||
&configured_git_marketplace_update(&source, Some(last_revision), ref_name),
|
||||
&configured_git_marketplace_update(&source, ref_name),
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -148,25 +142,24 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let debug_source = TempDir::new()?;
|
||||
let tools_source = TempDir::new()?;
|
||||
let debug_old_revision = init_marketplace_repo(debug_source.path(), "debug", "debug old")?;
|
||||
let tools_old_revision = init_marketplace_repo(tools_source.path(), "tools", "tools old")?;
|
||||
init_marketplace_repo(debug_source.path(), "debug", "debug old")?;
|
||||
init_marketplace_repo(tools_source.path(), "tools", "tools old")?;
|
||||
let debug_new_revision = commit_marketplace_marker(debug_source.path(), "debug new")?;
|
||||
let tools_new_revision = commit_marketplace_marker(tools_source.path(), "tools new")?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
debug_source.path(),
|
||||
&debug_old_revision,
|
||||
Some(&debug_new_revision),
|
||||
)?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
tools_source.path(),
|
||||
&tools_old_revision,
|
||||
Some(&tools_new_revision),
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
let config_before = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
@@ -195,8 +188,19 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
"tools new"
|
||||
);
|
||||
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
|
||||
assert!(config.contains(&debug_new_revision));
|
||||
assert!(config.contains(&tools_new_revision));
|
||||
assert_eq!(config, config_before);
|
||||
for (root, expected_revision) in [
|
||||
(debug_root, debug_new_revision),
|
||||
(tools_root, tools_new_revision),
|
||||
] {
|
||||
let metadata: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(
|
||||
root.as_path().join(".codex-marketplace-install.json"),
|
||||
)?)?;
|
||||
assert_eq!(
|
||||
metadata.get("revision").and_then(serde_json::Value::as_str),
|
||||
Some(expected_revision.as_str())
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -206,7 +210,7 @@ async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configu
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
let plugin_source = TempDir::new()?;
|
||||
let old_revision = init_marketplace_repo(source.path(), "trusted", "old")?;
|
||||
init_marketplace_repo(source.path(), "trusted", "old")?;
|
||||
|
||||
init_marketplace_repo(plugin_source.path(), "plugin", "plugin")?;
|
||||
let plugin_root = plugin_source.path().join(".codex-plugin");
|
||||
@@ -237,7 +241,6 @@ async fn automatic_upgrade_isolates_git_while_explicit_install_preserves_configu
|
||||
codex_home.path(),
|
||||
"trusted",
|
||||
source.path(),
|
||||
&old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
@@ -340,26 +343,21 @@ async fn marketplace_upgrade_named_marketplace_only() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let debug_source = TempDir::new()?;
|
||||
let tools_source = TempDir::new()?;
|
||||
let debug_old_revision = init_marketplace_repo(debug_source.path(), "debug", "debug old")?;
|
||||
let tools_old_revision = init_marketplace_repo(tools_source.path(), "tools", "tools old")?;
|
||||
init_marketplace_repo(debug_source.path(), "debug", "debug old")?;
|
||||
init_marketplace_repo(tools_source.path(), "tools", "tools old")?;
|
||||
commit_marketplace_marker(debug_source.path(), "debug new")?;
|
||||
commit_marketplace_marker(tools_source.path(), "tools new")?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
debug_source.path(),
|
||||
&debug_old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
let tools_source_alias = "manual:tools";
|
||||
record_user_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
&configured_git_marketplace_update(
|
||||
tools_source_alias,
|
||||
Some(&tools_old_revision),
|
||||
/*ref_name*/ None,
|
||||
),
|
||||
&configured_git_marketplace_update(tools_source_alias, /*ref_name*/ None),
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
let tools_rewrite = format!("url.{}.insteadOf", tools_source.path().display());
|
||||
@@ -402,13 +400,12 @@ async fn marketplace_upgrade_named_marketplace_only() -> Result<()> {
|
||||
async fn marketplace_upgrade_returns_empty_roots_when_already_up_to_date() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let source = TempDir::new()?;
|
||||
let old_revision = init_marketplace_repo(source.path(), "debug", "debug old")?;
|
||||
init_marketplace_repo(source.path(), "debug", "debug old")?;
|
||||
commit_marketplace_marker(source.path(), "debug new")?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
source.path(),
|
||||
&old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
|
||||
@@ -17,8 +17,6 @@ fn codex_command(codex_home: &Path) -> Result<assert_cmd::Command> {
|
||||
|
||||
fn configured_marketplace_update() -> MarketplaceConfigUpdate<'static> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
|
||||
@@ -36,8 +36,6 @@ fn codex_command_in(codex_home: &Path, current_dir: &Path) -> Result<assert_cmd:
|
||||
|
||||
fn configured_local_marketplace(source: &str) -> MarketplaceConfigUpdate<'_> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-05-06T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "local",
|
||||
source,
|
||||
ref_name: None,
|
||||
@@ -381,8 +379,6 @@ async fn marketplace_list_json_includes_configured_git_marketplace_source() -> R
|
||||
write_plugins_enabled_config(codex_home.path())?;
|
||||
write_marketplace_source(&marketplace_root)?;
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: "2026-06-04T08:39:49Z",
|
||||
last_revision: Some("abc123"),
|
||||
source_type: "git",
|
||||
source: "https://example.com/acme/agent-skills.git",
|
||||
ref_name: None,
|
||||
@@ -430,8 +426,6 @@ async fn marketplace_list_json_keys_configured_source_by_root() -> Result<()> {
|
||||
write_marketplace_source(home.path())?;
|
||||
write_marketplace_source(&marketplace_root)?;
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: "2026-06-04T08:39:49Z",
|
||||
last_revision: Some("abc123"),
|
||||
source_type: "git",
|
||||
source: "https://example.com/acme/agent-skills.git",
|
||||
ref_name: None,
|
||||
@@ -679,8 +673,6 @@ async fn plugin_list_json_includes_configured_git_marketplace_source() -> Result
|
||||
write_plugins_enabled_config(codex_home.path())?;
|
||||
write_marketplace_source(&marketplace_root)?;
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: "2026-06-04T08:39:49Z",
|
||||
last_revision: Some("abc123"),
|
||||
source_type: "git",
|
||||
source: "https://example.com/acme/agent-skills.git",
|
||||
ref_name: None,
|
||||
|
||||
@@ -11,8 +11,6 @@ use toml_edit::value;
|
||||
use crate::CONFIG_TOML_FILE;
|
||||
|
||||
pub struct MarketplaceConfigUpdate<'a> {
|
||||
pub last_updated: &'a str,
|
||||
pub last_revision: Option<&'a str>,
|
||||
pub source_type: &'a str,
|
||||
pub source: &'a str,
|
||||
pub ref_name: Option<&'a str>,
|
||||
@@ -100,10 +98,6 @@ fn upsert_marketplace(
|
||||
};
|
||||
let mut entry = TomlTable::new();
|
||||
entry.set_implicit(false);
|
||||
entry["last_updated"] = value(update.last_updated.to_string());
|
||||
if let Some(last_revision) = update.last_revision {
|
||||
entry["last_revision"] = value(last_revision.to_string());
|
||||
}
|
||||
entry["source_type"] = value(update.source_type.to_string());
|
||||
entry["source"] = value(update.source.to_string());
|
||||
if let Some(ref_name) = update.ref_name {
|
||||
@@ -186,12 +180,45 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn record_user_marketplace_omits_runtime_update_metadata() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let update = MarketplaceConfigUpdate {
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
sparse_paths: &[],
|
||||
};
|
||||
|
||||
record_user_marketplace(codex_home.path(), "debug", &update).unwrap();
|
||||
|
||||
let config: toml::Value =
|
||||
toml::from_str(&fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).unwrap())
|
||||
.unwrap();
|
||||
let marketplace = config
|
||||
.get("marketplaces")
|
||||
.and_then(|marketplaces| marketplaces.get("debug"))
|
||||
.expect("marketplace declaration");
|
||||
assert_eq!(
|
||||
marketplace.get("source_type").and_then(toml::Value::as_str),
|
||||
Some("git")
|
||||
);
|
||||
assert_eq!(
|
||||
marketplace.get("source").and_then(toml::Value::as_str),
|
||||
Some("https://github.com/owner/repo.git")
|
||||
);
|
||||
assert_eq!(
|
||||
marketplace.get("ref").and_then(toml::Value::as_str),
|
||||
Some("main")
|
||||
);
|
||||
assert!(marketplace.get("last_updated").is_none());
|
||||
assert!(marketplace.get("last_revision").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_user_marketplace_removes_requested_entry() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
@@ -227,8 +254,6 @@ mod tests {
|
||||
fn remove_user_marketplace_config_reports_case_mismatch() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
|
||||
@@ -9,8 +9,6 @@ use std::fs;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(super) struct MarketplaceInstallMetadata {
|
||||
@@ -35,10 +33,7 @@ pub(super) fn record_added_marketplace_entry(
|
||||
install_metadata: &MarketplaceInstallMetadata,
|
||||
) -> Result<(), MarketplaceAddError> {
|
||||
let source = install_metadata.config_source();
|
||||
let timestamp = utc_timestamp_now()?;
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: ×tamp,
|
||||
last_revision: None,
|
||||
source_type: install_metadata.config_source_type(),
|
||||
source: &source,
|
||||
ref_name: install_metadata.ref_name(),
|
||||
@@ -204,59 +199,12 @@ fn config_sparse_paths(marketplace: &toml::Value) -> Vec<String> {
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn utc_timestamp_now() -> Result<String, MarketplaceAddError> {
|
||||
let duration = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.map_err(|err| {
|
||||
MarketplaceAddError::Internal(format!("system clock is before Unix epoch: {err}"))
|
||||
})?;
|
||||
Ok(format_utc_timestamp(duration.as_secs() as i64))
|
||||
}
|
||||
|
||||
fn format_utc_timestamp(seconds_since_epoch: i64) -> String {
|
||||
const SECONDS_PER_DAY: i64 = 86_400;
|
||||
let days = seconds_since_epoch.div_euclid(SECONDS_PER_DAY);
|
||||
let seconds_of_day = seconds_since_epoch.rem_euclid(SECONDS_PER_DAY);
|
||||
let (year, month, day) = civil_from_days(days);
|
||||
let hour = seconds_of_day / 3_600;
|
||||
let minute = (seconds_of_day % 3_600) / 60;
|
||||
let second = seconds_of_day % 60;
|
||||
format!("{year:04}-{month:02}-{day:02}T{hour:02}:{minute:02}:{second:02}Z")
|
||||
}
|
||||
|
||||
fn civil_from_days(days_since_epoch: i64) -> (i64, i64, i64) {
|
||||
let days = days_since_epoch + 719_468;
|
||||
let era = if days >= 0 { days } else { days - 146_096 } / 146_097;
|
||||
let day_of_era = days - era * 146_097;
|
||||
let year_of_era =
|
||||
(day_of_era - day_of_era / 1_460 + day_of_era / 36_524 - day_of_era / 146_096) / 365;
|
||||
let mut year = year_of_era + era * 400;
|
||||
let day_of_year = day_of_era - (365 * year_of_era + year_of_era / 4 - year_of_era / 100);
|
||||
let month_prime = (5 * day_of_year + 2) / 153;
|
||||
let day = day_of_year - (153 * month_prime + 2) / 5 + 1;
|
||||
let month = month_prime + if month_prime < 10 { 3 } else { -9 };
|
||||
year += if month <= 2 { 1 } else { 0 };
|
||||
(year, month, day)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn utc_timestamp_formats_unix_epoch_as_rfc3339_utc() {
|
||||
assert_eq!(
|
||||
format_utc_timestamp(/*seconds_since_epoch*/ 0),
|
||||
"1970-01-01T00:00:00Z"
|
||||
);
|
||||
assert_eq!(
|
||||
format_utc_timestamp(/*seconds_since_epoch*/ 1_775_779_200),
|
||||
"2026-04-10T00:00:00Z"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn installed_marketplace_root_for_source_propagates_config_read_errors() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
|
||||
@@ -119,8 +119,6 @@ mod tests {
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
&MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
@@ -180,8 +178,6 @@ mod tests {
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
&MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
@@ -243,8 +239,6 @@ mod tests {
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
&MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision: None,
|
||||
source_type: "git",
|
||||
source: "https://github.com/owner/repo.git",
|
||||
ref_name: Some("main"),
|
||||
|
||||
@@ -3,6 +3,7 @@ mod git;
|
||||
|
||||
use self::activation::activate_marketplace_root;
|
||||
use self::activation::installed_marketplace_metadata_matches;
|
||||
use self::activation::read_installed_marketplace_snapshot;
|
||||
use self::activation::write_installed_marketplace_metadata;
|
||||
use self::git::clone_git_source;
|
||||
use self::git::git_remote_revision;
|
||||
@@ -14,8 +15,6 @@ use crate::marketplace_policy::MarketplacePolicy;
|
||||
use crate::marketplace_policy::validate_marketplace_name_for_add;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::MarketplaceConfigUpdate;
|
||||
use codex_config::record_user_marketplace;
|
||||
use codex_config::types::MarketplaceConfig;
|
||||
use codex_config::types::MarketplaceSourceType;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
@@ -44,7 +43,6 @@ struct ConfiguredGitMarketplace {
|
||||
source: String,
|
||||
ref_name: Option<String>,
|
||||
sparse_paths: Vec<String>,
|
||||
last_revision: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
@@ -200,7 +198,7 @@ fn parse_configured_git_marketplace(
|
||||
.map_err(|err| format!("invalid configured Git marketplace: {err}"))?;
|
||||
let MarketplaceConfig {
|
||||
last_updated: _,
|
||||
last_revision,
|
||||
last_revision: _,
|
||||
source_type,
|
||||
source,
|
||||
ref_name,
|
||||
@@ -216,7 +214,6 @@ fn parse_configured_git_marketplace(
|
||||
source,
|
||||
ref_name,
|
||||
sparse_paths: sparse_paths.unwrap_or_default(),
|
||||
last_revision,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -243,10 +240,10 @@ fn upgrade_configured_git_marketplace(
|
||||
mode,
|
||||
)?;
|
||||
let destination = install_root.join(&marketplace.name);
|
||||
let previous_snapshot = read_installed_marketplace_snapshot(&destination, &marketplace.name);
|
||||
if validate_marketplace_root(&destination)
|
||||
.is_ok_and(|marketplace_name| marketplace_name == marketplace.name)
|
||||
&& marketplace.last_revision.as_deref() == Some(remote_revision.as_str())
|
||||
&& installed_marketplace_metadata_matches(&destination, marketplace, &remote_revision)
|
||||
&& installed_marketplace_metadata_matches(&previous_snapshot, marketplace, &remote_revision)
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
@@ -286,24 +283,8 @@ fn upgrade_configured_git_marketplace(
|
||||
));
|
||||
}
|
||||
write_installed_marketplace_metadata(staged_dir.path(), marketplace, &activated_revision)?;
|
||||
|
||||
let last_updated = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true);
|
||||
let update = MarketplaceConfigUpdate {
|
||||
last_updated: &last_updated,
|
||||
last_revision: Some(&activated_revision),
|
||||
source_type: "git",
|
||||
source: &marketplace.source,
|
||||
ref_name: marketplace.ref_name.as_deref(),
|
||||
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}",
|
||||
marketplace.name
|
||||
)
|
||||
})
|
||||
activate_marketplace_root(&destination, staged_dir, &previous_snapshot, || {
|
||||
ensure_configured_git_marketplace_unchanged(codex_home, marketplace)
|
||||
})?;
|
||||
|
||||
AbsolutePathBuf::try_from(destination)
|
||||
|
||||
@@ -19,27 +19,58 @@ struct InstalledMarketplaceMetadata {
|
||||
revision: String,
|
||||
}
|
||||
|
||||
pub(super) fn installed_marketplace_metadata_matches(
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(super) struct InstalledMarketplaceSnapshot {
|
||||
marketplace_name: String,
|
||||
exists: bool,
|
||||
metadata: Option<InstalledMarketplaceMetadata>,
|
||||
}
|
||||
|
||||
pub(super) fn read_installed_marketplace_snapshot(
|
||||
root: &Path,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
) -> bool {
|
||||
marketplace_name: &str,
|
||||
) -> InstalledMarketplaceSnapshot {
|
||||
if !root.exists() {
|
||||
return InstalledMarketplaceSnapshot {
|
||||
marketplace_name: marketplace_name.to_string(),
|
||||
exists: false,
|
||||
metadata: None,
|
||||
};
|
||||
}
|
||||
let metadata = match std::fs::read_to_string(installed_marketplace_metadata_path(root)) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(_) => return false,
|
||||
Err(_) => {
|
||||
return InstalledMarketplaceSnapshot {
|
||||
marketplace_name: marketplace_name.to_string(),
|
||||
exists: true,
|
||||
metadata: None,
|
||||
};
|
||||
}
|
||||
};
|
||||
let metadata = match serde_json::from_str::<InstalledMarketplaceMetadata>(&metadata) {
|
||||
Ok(metadata) => metadata,
|
||||
Ok(metadata) => Some(metadata),
|
||||
Err(err) => {
|
||||
warn!(
|
||||
marketplace = marketplace.name,
|
||||
marketplace = marketplace_name,
|
||||
error = %err,
|
||||
"failed to parse activated marketplace metadata"
|
||||
);
|
||||
return false;
|
||||
None
|
||||
}
|
||||
};
|
||||
metadata == installed_marketplace_metadata(marketplace, revision)
|
||||
InstalledMarketplaceSnapshot {
|
||||
marketplace_name: marketplace_name.to_string(),
|
||||
exists: true,
|
||||
metadata,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn installed_marketplace_metadata_matches(
|
||||
snapshot: &InstalledMarketplaceSnapshot,
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
) -> bool {
|
||||
snapshot.metadata.as_ref() == Some(&installed_marketplace_metadata(marketplace, revision))
|
||||
}
|
||||
|
||||
pub(super) fn write_installed_marketplace_metadata(
|
||||
@@ -57,6 +88,7 @@ pub(super) fn write_installed_marketplace_metadata(
|
||||
pub(super) fn activate_marketplace_root(
|
||||
destination: &Path,
|
||||
staged_dir: TempDir,
|
||||
previous_snapshot: &InstalledMarketplaceSnapshot,
|
||||
after_activate: impl FnOnce() -> Result<(), String>,
|
||||
) -> Result<(), String> {
|
||||
let staged_root = staged_dir.path();
|
||||
@@ -109,7 +141,18 @@ pub(super) fn activate_marketplace_root(
|
||||
};
|
||||
}
|
||||
|
||||
if let Err(err) = after_activate() {
|
||||
let activation_result = if read_installed_marketplace_snapshot(
|
||||
&backup_root,
|
||||
&previous_snapshot.marketplace_name,
|
||||
) != *previous_snapshot
|
||||
{
|
||||
Err(installed_marketplace_snapshot_changed_error(
|
||||
previous_snapshot,
|
||||
))
|
||||
} else {
|
||||
after_activate()
|
||||
};
|
||||
if let Err(err) = activation_result {
|
||||
let remove_result = std::fs::remove_dir_all(destination);
|
||||
let rollback_result =
|
||||
remove_result.and_then(|()| std::fs::rename(&backup_root, destination));
|
||||
@@ -135,7 +178,14 @@ pub(super) fn activate_marketplace_root(
|
||||
destination.display()
|
||||
)
|
||||
})?;
|
||||
if let Err(err) = after_activate() {
|
||||
let activation_result = if previous_snapshot.exists {
|
||||
Err(installed_marketplace_snapshot_changed_error(
|
||||
previous_snapshot,
|
||||
))
|
||||
} else {
|
||||
after_activate()
|
||||
};
|
||||
if let Err(err) = activation_result {
|
||||
let remove_result = std::fs::remove_dir_all(destination);
|
||||
return match remove_result {
|
||||
Ok(()) => Err(err),
|
||||
@@ -149,6 +199,13 @@ pub(super) fn activate_marketplace_root(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn installed_marketplace_snapshot_changed_error(snapshot: &InstalledMarketplaceSnapshot) -> String {
|
||||
format!(
|
||||
"installed marketplace `{}` changed while auto-upgrade was in flight",
|
||||
snapshot.marketplace_name
|
||||
)
|
||||
}
|
||||
|
||||
fn installed_marketplace_metadata(
|
||||
marketplace: &ConfiguredGitMarketplace,
|
||||
revision: &str,
|
||||
|
||||
@@ -38,7 +38,6 @@ last_revision = "abc123"
|
||||
source: "https://github.com/example/good.git".to_string(),
|
||||
ref_name: Some("main".to_string()),
|
||||
sparse_paths: vec!["plugins".to_string()],
|
||||
last_revision: Some("abc123".to_string()),
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -87,6 +86,10 @@ source = {good_url:?}
|
||||
.expect("installed marketplace root")
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).unwrap(),
|
||||
config
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -241,7 +244,6 @@ ref = "missing-ref"
|
||||
source: raw_source,
|
||||
ref_name: Some("missing-ref".to_string()),
|
||||
sparse_paths: Vec::new(),
|
||||
last_revision: None,
|
||||
};
|
||||
let normalized_source = MarketplaceSource::Git {
|
||||
url: normalized_url,
|
||||
@@ -285,7 +287,6 @@ fn up_to_date_fast_path_validates_marketplace_name() {
|
||||
source: missing_source.clone(),
|
||||
ref_name: Some(REVISION.to_string()),
|
||||
sparse_paths: Vec::new(),
|
||||
last_revision: Some(REVISION.to_string()),
|
||||
};
|
||||
super::activation::write_installed_marketplace_metadata(&destination, &marketplace, REVISION)
|
||||
.expect("write installed marketplace metadata");
|
||||
@@ -306,6 +307,79 @@ fn up_to_date_fast_path_validates_marketplace_name() {
|
||||
assert!(err.contains("git clone marketplace source failed"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_activation_restores_newer_concurrently_installed_marketplace() {
|
||||
const INITIAL_REVISION: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
|
||||
const STALE_REVISION: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
|
||||
const NEWER_REVISION: &str = "cccccccccccccccccccccccccccccccccccccccc";
|
||||
|
||||
let codex_home = TempDir::new().expect("create Codex home");
|
||||
let install_root = marketplace_install_root(codex_home.path());
|
||||
let destination = install_root.join("good");
|
||||
std::fs::create_dir_all(&destination).expect("create installed marketplace root");
|
||||
let marketplace = ConfiguredGitMarketplace {
|
||||
name: "good".to_string(),
|
||||
source: "https://github.com/example/good.git".to_string(),
|
||||
ref_name: Some("main".to_string()),
|
||||
sparse_paths: Vec::new(),
|
||||
};
|
||||
super::activation::write_installed_marketplace_metadata(
|
||||
&destination,
|
||||
&marketplace,
|
||||
INITIAL_REVISION,
|
||||
)
|
||||
.expect("write initial installed marketplace metadata");
|
||||
let previous_snapshot =
|
||||
super::activation::read_installed_marketplace_snapshot(&destination, &marketplace.name);
|
||||
|
||||
std::fs::write(destination.join("marker.txt"), "newer snapshot")
|
||||
.expect("write newer installed marketplace snapshot");
|
||||
super::activation::write_installed_marketplace_metadata(
|
||||
&destination,
|
||||
&marketplace,
|
||||
NEWER_REVISION,
|
||||
)
|
||||
.expect("write newer installed marketplace metadata");
|
||||
|
||||
let staged_dir = tempfile::Builder::new()
|
||||
.prefix("marketplace-upgrade-")
|
||||
.tempdir_in(&install_root)
|
||||
.expect("create stale upgrade staging directory");
|
||||
std::fs::write(staged_dir.path().join("marker.txt"), "stale snapshot")
|
||||
.expect("write stale staged marketplace snapshot");
|
||||
super::activation::write_installed_marketplace_metadata(
|
||||
staged_dir.path(),
|
||||
&marketplace,
|
||||
STALE_REVISION,
|
||||
)
|
||||
.expect("write stale staged marketplace metadata");
|
||||
|
||||
let err = super::activation::activate_marketplace_root(
|
||||
&destination,
|
||||
staged_dir,
|
||||
&previous_snapshot,
|
||||
|| Ok(()),
|
||||
)
|
||||
.expect_err("stale upgrade must not replace a newer installed snapshot");
|
||||
|
||||
assert_eq!(
|
||||
err,
|
||||
"installed marketplace `good` changed while auto-upgrade was in flight"
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(destination.join("marker.txt"))
|
||||
.expect("read restored marketplace snapshot"),
|
||||
"newer snapshot"
|
||||
);
|
||||
let restored_snapshot =
|
||||
super::activation::read_installed_marketplace_snapshot(&destination, &marketplace.name);
|
||||
assert!(super::activation::installed_marketplace_metadata_matches(
|
||||
&restored_snapshot,
|
||||
&marketplace,
|
||||
NEWER_REVISION
|
||||
));
|
||||
}
|
||||
|
||||
fn config_layer_stack(codex_home: &Path, config: &str) -> ConfigLayerStack {
|
||||
let config_file =
|
||||
AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE)).expect("absolute config path");
|
||||
|
||||
Reference in New Issue
Block a user