fix(install): fix force reinstall cleanup and resume file corruption

Author: QaidVoid

.github/workflows/ci.yaml

@@ -65,7 +65,7 @@ jobs:
         uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7  #v5.5.1
         with:
           files: lcov.info
-          fail_ci_if_error: true
+          fail_ci_if_error: false
           token: ${{ secrets.CODECOV_TOKEN }}
 
   cargo-machete:

crates/soar-cli/src/install.rs

@@ -20,6 +20,7 @@ use soar_core::{
     package::{
         install::{InstallMarker, InstallTarget, PackageInstaller},
         query::PackageQuery,
+        update::remove_old_versions,
         url::UrlPackage,
     },
     SoarResult,
@@ -260,7 +261,9 @@ fn resolve_packages(
                 }
             }
 
-            let existing_install = installed_packages.into_iter().next();
+            let existing_install = installed_pkg
+                .cloned()
+                .or_else(|| installed_packages.into_iter().next());
 
             install_targets.push(InstallTarget {
                 package: url_pkg.to_package(),
@@ -977,7 +980,8 @@ async fn spawn_installation_task(
     let ctx = ctx.clone();
 
     tokio::spawn(async move {
-        let result = install_single_package(&ctx, &target, progress_callback, core_db).await;
+        let result =
+            install_single_package(&ctx, &target, progress_callback, core_db.clone()).await;
 
         match result {
             Ok((install_dir, symlinks)) => {
@@ -987,12 +991,16 @@ async fn spawn_installation_task(
                     .insert(idx, (install_dir, symlinks));
                 installed_count.fetch_add(1, Ordering::Relaxed);
                 total_pb.inc(1);
+
+                let _ = remove_old_versions(&target.package, &core_db, false);
             }
             Err(err) => {
                 match err {
                     SoarError::Warning(err) => {
                         let mut warnings = ctx.warnings.lock().unwrap();
                         warnings.push(err);
+
+                        let _ = remove_old_versions(&target.package, &core_db, false);
                     }
                     _ => {
                         let mut errors = ctx.errors.lock().unwrap();

crates/soar-cli/src/update.rs

@@ -360,7 +360,7 @@ async fn spawn_update_task(
                     warnings.push(err);
 
                     if !keep {
-                        let _ = remove_old_versions(&target.package, &diesel_db);
+                        let _ = remove_old_versions(&target.package, &diesel_db, false);
                     }
                 }
                 _ => {
@@ -373,7 +373,7 @@ async fn spawn_update_task(
             total_pb.inc(1);
 
             if !keep {
-                let _ = remove_old_versions(&target.package, &diesel_db);
+                let _ = remove_old_versions(&target.package, &diesel_db, false);
             }
         }
 

crates/soar-core/src/package/install.rs

@@ -67,7 +67,7 @@ pub struct PackageInstaller {
     globs: Vec<String>,
 }
 
-#[derive(Clone, Default)]
+#[derive(Clone, Default, Debug)]
 pub struct InstallTarget {
     pub package: Package,
     pub existing_install: Option<crate::database::models::InstalledPackage>,
@@ -100,9 +100,34 @@ impl PackageInstaller {
         );
         let profile = get_config().default_profile.clone();
 
-        let needs_new_record = match &target.existing_install {
-            None => true,
-            Some(existing) => existing.version != package.version,
+        // Check if there's a pending install for this exact version we can resume
+        let has_pending = db.with_conn(|conn| {
+            CoreRepository::has_pending_install(
+                conn,
+                &package.pkg_id,
+                &package.pkg_name,
+                &package.repo_name,
+                &package.version,
+            )
+        })?;
+
+        trace!(
+            pkg_id = package.pkg_id,
+            pkg_name = package.pkg_name,
+            repo_name = package.repo_name,
+            version = package.version,
+            has_pending = has_pending,
+            "checking for pending install"
+        );
+
+        let needs_new_record = if has_pending {
+            trace!("resuming existing pending install");
+            false
+        } else {
+            match &target.existing_install {
+                None => true,
+                Some(existing) => existing.version != package.version || existing.is_installed,
+            }
         };
 
         if needs_new_record {
@@ -119,6 +144,17 @@ impl PackageInstaller {
             let installed_path = install_dir.to_string_lossy();
             let installed_date = Utc::now().format("%Y-%m-%d %H:%M:%S").to_string();
 
+            // Clean up any orphaned pending installs (different versions) before creating new record
+            let orphaned_paths = db.with_conn(|conn| {
+                CoreRepository::delete_pending_installs(conn, pkg_id, pkg_name, repo_name)
+            })?;
+            for path in orphaned_paths {
+                let path = std::path::Path::new(&path);
+                if path.exists() {
+                    fs::remove_dir_all(path).ok();
+                }
+            }
+
             let new_package = NewInstalledPackage {
                 repo_name,
                 pkg_id,
@@ -351,6 +387,7 @@ impl PackageInstaller {
         let with_pkg_id = self.with_pkg_id;
         let installed_date = Utc::now().format("%Y-%m-%d %H:%M:%S").to_string();
 
+        let installed_path = self.install_dir.to_string_lossy();
         let record_id: Option<i32> = self.db.with_conn(|conn| {
             CoreRepository::record_installation(
                 conn,
@@ -363,6 +400,7 @@ impl PackageInstaller {
                 with_pkg_id,
                 checksum,
                 &installed_date,
+                &installed_path,
             )
         })?;
 

crates/soar-core/src/package/update.rs

@@ -12,7 +12,8 @@ use crate::{
 ///
 /// This function finds all installed versions of the package (by pkg_id, pkg_name, repo_name)
 /// that are older than the current version and removes them from disk and database.
-pub fn remove_old_versions(package: &Package, db: &DieselDatabase) -> SoarResult<()> {
+/// If `force` is true, removes pinned packages too. Otherwise only unpinned packages.
+pub fn remove_old_versions(package: &Package, db: &DieselDatabase, force: bool) -> SoarResult<()> {
     let Package {
         pkg_id,
         pkg_name,
@@ -21,7 +22,7 @@ pub fn remove_old_versions(package: &Package, db: &DieselDatabase) -> SoarResult
     } = package;
 
     let old_packages = db.with_conn(|conn| {
-        CoreRepository::get_old_package_paths(conn, pkg_id, pkg_name, repo_name)
+        CoreRepository::get_old_package_paths(conn, pkg_id, pkg_name, repo_name, force)
     })?;
 
     for (_id, installed_path) in &old_packages {
@@ -32,7 +33,9 @@ pub fn remove_old_versions(package: &Package, db: &DieselDatabase) -> SoarResult
         }
     }
 
-    db.with_conn(|conn| CoreRepository::delete_old_packages(conn, pkg_id, pkg_name, repo_name))?;
+    db.with_conn(|conn| {
+        CoreRepository::delete_old_packages(conn, pkg_id, pkg_name, repo_name, force)
+    })?;
 
     Ok(())
 }

crates/soar-db/src/repository/core.rs

@@ -1,6 +1,6 @@
 //! Core database repository for installed packages.
 
-use diesel::prelude::*;
+use diesel::{prelude::*, sql_types::Bool, sqlite::Sqlite};
 
 use crate::{
     models::{
@@ -366,6 +366,7 @@ impl CoreRepository {
     }
 
     /// Updates an installed package after successful installation.
+    /// Only updates the record with is_installed=false (the newly created one).
     #[allow(clippy::too_many_arguments)]
     pub fn record_installation(
         conn: &mut SqliteConnection,
@@ -378,14 +379,16 @@ impl CoreRepository {
         with_pkg_id: bool,
         checksum: Option<&str>,
         installed_date: &str,
+        installed_path: &str,
     ) -> QueryResult<Option<i32>> {
         let provides = provides.map(|v| serde_json::to_value(v).unwrap_or_default());
         diesel::update(
             packages::table
                 .filter(packages::repo_name.eq(repo_name))
                 .filter(packages::pkg_name.eq(pkg_name))
                 .filter(packages::pkg_id.eq(pkg_id))
-                .filter(packages::version.eq(version)),
+                .filter(packages::version.eq(version))
+                .filter(packages::is_installed.eq(false)),
         )
         .set((
             packages::size.eq(size),
@@ -394,6 +397,7 @@ impl CoreRepository {
             packages::provides.eq(provides),
             packages::with_pkg_id.eq(with_pkg_id),
             packages::checksum.eq(checksum),
+            packages::installed_path.eq(installed_path),
         ))
         .returning(packages::id)
         .get_result(conn)
@@ -459,6 +463,54 @@ impl CoreRepository {
         diesel::delete(packages::table.filter(packages::id.eq(id))).execute(conn)
     }
 
+    /// Checks if a pending install (is_installed=false) exists for a specific package version.
+    /// Used to check if we can resume a partial install.
+    pub fn has_pending_install(
+        conn: &mut SqliteConnection,
+        pkg_id: &str,
+        pkg_name: &str,
+        repo_name: &str,
+        version: &str,
+    ) -> QueryResult<bool> {
+        let count: i64 = packages::table
+            .filter(packages::pkg_id.eq(pkg_id))
+            .filter(packages::pkg_name.eq(pkg_name))
+            .filter(packages::repo_name.eq(repo_name))
+            .filter(packages::version.eq(version))
+            .filter(packages::is_installed.eq(false))
+            .count()
+            .get_result(conn)?;
+        Ok(count > 0)
+    }
+
+    /// Deletes pending (is_installed=false) records for a package and returns their paths.
+    /// Used to clean up orphaned partial installs before starting a new install.
+    pub fn delete_pending_installs(
+        conn: &mut SqliteConnection,
+        pkg_id: &str,
+        pkg_name: &str,
+        repo_name: &str,
+    ) -> QueryResult<Vec<String>> {
+        let paths: Vec<String> = packages::table
+            .filter(packages::pkg_id.eq(pkg_id))
+            .filter(packages::pkg_name.eq(pkg_name))
+            .filter(packages::repo_name.eq(repo_name))
+            .filter(packages::is_installed.eq(false))
+            .select(packages::installed_path)
+            .load(conn)?;
+
+        diesel::delete(
+            packages::table
+                .filter(packages::pkg_id.eq(pkg_id))
+                .filter(packages::pkg_name.eq(pkg_name))
+                .filter(packages::repo_name.eq(repo_name))
+                .filter(packages::is_installed.eq(false)),
+        )
+        .execute(conn)?;
+
+        Ok(paths)
+    }
+
     /// Gets the portable package configuration for a package.
     pub fn get_portable(
         conn: &mut SqliteConnection,
@@ -525,13 +577,15 @@ impl CoreRepository {
             .execute(conn)
     }
 
-    /// Gets old package versions (all except the newest unpinned one) for cleanup.
+    /// Gets old package versions (all except the newest one) for cleanup.
     /// Returns the installed paths of packages to remove.
+    /// If `force` is true, includes pinned packages. Otherwise only unpinned packages.
     pub fn get_old_package_paths(
         conn: &mut SqliteConnection,
         pkg_id: &str,
         pkg_name: &str,
         repo_name: &str,
+        force: bool,
     ) -> QueryResult<Vec<(i32, String)>> {
         let latest: Option<(i32, String)> = packages::table
             .filter(packages::pkg_id.eq(pkg_id))
@@ -546,23 +600,33 @@ impl CoreRepository {
             return Ok(Vec::new());
         };
 
-        packages::table
+        let query = packages::table
             .filter(packages::pkg_id.eq(pkg_id))
             .filter(packages::pkg_name.eq(pkg_name))
             .filter(packages::repo_name.eq(repo_name))
-            .filter(packages::pinned.eq(false))
             .filter(packages::id.ne(latest_id))
             .filter(packages::installed_path.ne(&latest_path))
+            .into_boxed();
+
+        let query = if force {
+            query
+        } else {
+            query.filter(packages::pinned.eq(false))
+        };
+
+        query
             .select((packages::id, packages::installed_path))
             .load(conn)
     }
 
-    /// Deletes old package versions (all except the newest unpinned one).
+    /// Deletes old package versions (all except the newest one).
+    /// If `force` is true, deletes pinned packages too. Otherwise only unpinned packages.
     pub fn delete_old_packages(
         conn: &mut SqliteConnection,
         pkg_id: &str,
         pkg_name: &str,
         repo_name: &str,
+        force: bool,
     ) -> QueryResult<usize> {
         let latest_id: Option<i32> = packages::table
             .filter(packages::pkg_id.eq(pkg_id))
@@ -577,15 +641,21 @@ impl CoreRepository {
             return Ok(0);
         };
 
-        diesel::delete(
-            packages::table
-                .filter(packages::pkg_id.eq(pkg_id))
-                .filter(packages::pkg_name.eq(pkg_name))
-                .filter(packages::repo_name.eq(repo_name))
-                .filter(packages::pinned.eq(false))
-                .filter(packages::id.ne(latest_id)),
-        )
-        .execute(conn)
+        let pinned_filter: Box<dyn BoxableExpression<packages::table, Sqlite, SqlType = Bool>> =
+            if force {
+                Box::new(diesel::dsl::sql::<Bool>("TRUE"))
+            } else {
+                Box::new(packages::pinned.eq(false))
+            };
+
+        let query = packages::table
+            .filter(packages::pkg_id.eq(pkg_id))
+            .filter(packages::pkg_name.eq(pkg_name))
+            .filter(packages::repo_name.eq(repo_name))
+            .filter(packages::id.ne(latest_id))
+            .filter(pinned_filter);
+
+        diesel::delete(query).execute(conn)
     }
 
     /// Unlinks all packages with a given name except those matching pkg_id and checksum.