fix(plugins): keep version upgrades additive

codex-rs/core-plugins/Cargo.toml

@@ -32,6 +32,7 @@ chrono = { workspace = true }
 dirs = { workspace = true }
 flate2 = { workspace = true }
 reqwest = { workspace = true }
+semver = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
 tar = { workspace = true }

codex-rs/core-plugins/src/store.rs

@@ -4,8 +4,10 @@ use codex_plugin::PluginId;
 use codex_plugin::validate_plugin_segment;
 use codex_utils_absolute_path::AbsolutePathBuf;
 use codex_utils_plugins::find_plugin_manifest_path;
+use semver::Version;
 use serde::Deserialize;
 use serde_json::Value as JsonValue;
+use std::cmp::Ordering;
 use std::fs;
 use std::io;
 use std::path::Path;
@@ -75,7 +77,7 @@ impl PluginStore {
             })
             .filter(|version| validate_plugin_version_segment(version).is_ok())
             .collect::<Vec<_>>();
-        discovered_versions.sort_unstable();
+        discovered_versions.sort_unstable_by(|left, right| compare_plugin_versions(left, right));
         if discovered_versions.is_empty() {
             None
         } else if discovered_versions
@@ -286,6 +288,15 @@ fn replace_plugin_root_atomically(
     let staged_version_root = staged_root.join(plugin_version);
     copy_dir_recursive(source, &staged_version_root)?;
 
+    let target_version_root = target_root.join(plugin_version);
+    if target_root.exists() && !target_version_root.exists() {
+        fs::rename(&staged_version_root, &target_version_root).map_err(|err| {
+            PluginStoreError::io("failed to activate updated plugin cache version", err)
+        })?;
+        remove_old_plugin_versions(target_root, plugin_version)?;
+        return Ok(());
+    }
+
     if target_root.exists() {
         let backup_dir = tempfile::Builder::new()
             .prefix("plugin-backup-")
@@ -322,6 +333,52 @@ fn replace_plugin_root_atomically(
     Ok(())
 }
 
+fn remove_old_plugin_versions(
+    target_root: &Path,
+    plugin_version: &str,
+) -> Result<(), PluginStoreError> {
+    let Ok(entries) = fs::read_dir(target_root) else {
+        return Ok(());
+    };
+
+    for entry in entries.filter_map(Result::ok) {
+        let Ok(file_type) = entry.file_type() else {
+            continue;
+        };
+        if !file_type.is_dir() {
+            continue;
+        }
+        let Ok(version) = entry.file_name().into_string() else {
+            continue;
+        };
+        if version == plugin_version || validate_plugin_version_segment(&version).is_err() {
+            continue;
+        }
+
+        if fs::remove_dir_all(entry.path()).is_err()
+            && old_plugin_version_would_stay_active(&version, plugin_version)
+        {
+            return Err(PluginStoreError::Invalid(format!(
+                "failed to activate updated plugin cache version `{plugin_version}` while `{version}` remains active"
+            )));
+        }
+    }
+
+    Ok(())
+}
+
+fn old_plugin_version_would_stay_active(old_version: &str, new_version: &str) -> bool {
+    old_version == DEFAULT_PLUGIN_VERSION
+        || compare_plugin_versions(old_version, new_version).is_gt()
+}
+
+fn compare_plugin_versions(left: &str, right: &str) -> Ordering {
+    match (Version::parse(left), Version::parse(right)) {
+        (Ok(left), Ok(right)) => left.cmp(&right),
+        _ => left.cmp(right),
+    }
+}
+
 fn copy_dir_recursive(source: &Path, target: &Path) -> Result<(), PluginStoreError> {
     fs::create_dir_all(target)
         .map_err(|err| PluginStoreError::io("failed to create plugin target directory", err))?;

codex-rs/core-plugins/src/store_tests.rs

@@ -247,7 +247,7 @@ fn active_plugin_version_prefers_default_local_version_when_multiple_versions_ex
 }
 
 #[test]
-fn active_plugin_version_returns_last_sorted_version_when_default_is_missing() {
+fn active_plugin_version_returns_latest_version_when_default_is_missing() {
     let tmp = tempdir().unwrap();
     write_plugin(
         &tmp.path().join("plugins/cache/debug"),
@@ -268,6 +268,76 @@ fn active_plugin_version_returns_last_sorted_version_when_default_is_missing() {
     );
 }
 
+#[test]
+fn active_plugin_version_compares_semver_versions_semantically() {
+    let tmp = tempdir().unwrap();
+    write_plugin(
+        &tmp.path().join("plugins/cache/debug"),
+        "sample-plugin/9.0.0",
+        "sample-plugin",
+    );
+    write_plugin(
+        &tmp.path().join("plugins/cache/debug"),
+        "sample-plugin/10.0.0",
+        "sample-plugin",
+    );
+    let store = PluginStore::new(tmp.path().to_path_buf());
+    let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
+
+    assert_eq!(
+        store.active_plugin_version(&plugin_id),
+        Some("10.0.0".to_string())
+    );
+}
+
+#[test]
+fn install_with_new_version_keeps_existing_plugin_root_and_prunes_old_versions() {
+    let tmp = tempdir().unwrap();
+    let store = PluginStore::new(tmp.path().to_path_buf());
+    let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap();
+
+    write_plugin_with_version(tmp.path(), "v1", "sample-plugin", Some("1.0.0"));
+    store
+        .install(
+            AbsolutePathBuf::try_from(tmp.path().join("v1")).unwrap(),
+            plugin_id.clone(),
+        )
+        .unwrap();
+
+    write_plugin_with_version(tmp.path(), "v2", "sample-plugin", Some("2.0.0"));
+    store
+        .install(
+            AbsolutePathBuf::try_from(tmp.path().join("v2")).unwrap(),
+            plugin_id.clone(),
+        )
+        .unwrap();
+
+    assert_eq!(
+        store.active_plugin_version(&plugin_id),
+        Some("2.0.0".to_string())
+    );
+    assert!(
+        tmp.path()
+            .join("plugins/cache/debug/sample-plugin/2.0.0")
+            .is_dir()
+    );
+    assert!(
+        !tmp.path()
+            .join("plugins/cache/debug/sample-plugin/1.0.0")
+            .exists()
+    );
+}
+
+#[test]
+fn old_plugin_version_would_stay_active_for_local_or_later_versions() {
+    assert!(old_plugin_version_would_stay_active(
+        DEFAULT_PLUGIN_VERSION,
+        "1.0.0"
+    ));
+    assert!(old_plugin_version_would_stay_active("10.0.0", "9.0.0"));
+    assert!(!old_plugin_version_would_stay_active("1.0.0", "2.0.0"));
+}
+
 #[test]
 fn plugin_root_rejects_path_separators_in_key_segments() {
     let err = PluginId::parse("../../etc@debug").unwrap_err();