fix(wallet): safe upgrade backups with time-based retention and guarded file deletion

CHANGELOG.md

@@ -27,6 +27,31 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   Allowing 3.x needs an upstream change in `dashpay/rust-dashcore` first. A TODO
   in `Cargo.toml` marks the re-check.
 
+- **Safer guidance for wallet data this version cannot open**: the message no
+  longer tells you to remove your local wallet data, which could have deleted
+  the `secrets` folder along with keys no recovery phrase can restore. It now
+  asks you to write down your recovery phrases and imported keys in the version
+  you used before, close every running instance, then set aside the `.sqlite`
+  files together with their matching `-wal` and `-shm` files and keep the
+  `secrets` folder. Imported private keys are listed separately from recovery phrases.
+  A database temporarily held by another session now asks you to close that
+  session and try again instead of reporting incompatible data.
+
+- **Upgrade backups no longer pile up or outlive deleted data**: a failed
+  database upgrade no longer leaves a new backup on every attempt. Each
+  database keeps at most one upgrade backup, and it is deleted when you remove
+  a wallet or identity or clear a network's data. An upgrade that can never
+  succeed now shows its message instead of being retried silently.
+  Retention and deletion cover upstream pre-migration snapshots too. Cleanup
+  rejects links and non-files, reports pruning failures, and retries incomplete
+  identity backup deletion at the next startup.
+  Cleanup also removes unfinished snapshots left by a crash and runs when
+  removing a wallet without an available backend. Temporary storage and memory
+  failures remain retryable; permission and invalid-input failures surface directly.
+  Concurrent sessions now protect active snapshots from cleanup, and staged
+  validation preserves retry options and guidance for temporary resource failures.
+  One lock now covers the complete open, upgrade, backup, and retention cycle.
+
 ### Added
 
 - Migration tests also replay public user/DPNS and Evonode identities serialized

docs/ai-design/2026-09-10-platform-pin/upgrade-notes.md

@@ -60,7 +60,15 @@ typed identity rows remain in the backup, while their opaque local metadata is
 preserved. An ambiguous or malformed saved identity roster fails closed.
 The encrypted seed vault is not migrated or rewritten.
 
-Keep the retained backups. Downgrading does not automatically reverse the
+The bridge takes its backup only after the converted copy validates, just before
+the rebuild, and keeps at most one backup per database. Retention includes upstream
+`backups/auto/pre-migration-<database>-*.db` snapshots and runs before and after
+storage opens, including failed opens. Pruning errors stop the operation; backup
+candidates must have the exact database-specific name format and be regular files,
+not symbolic links (or hard links on Unix). Identity cleanup retains its pending
+manifest until backup deletion succeeds, so startup retries incomplete cleanup.
+Deleting a wallet or an identity, or clearing a network's data, also deletes these backups, because they
+copy wallet and identity history. Downgrading does not automatically reverse the
 database conversion; recovery requires the corresponding backup and the prior
 application version. Validation uses synthetic upstream fixtures, not a user's
 real profile.

src/backend_task/error.rs

@@ -188,15 +188,27 @@ pub enum TaskError {
     /// An identity-funding account was derived but could not be saved. Saving
     /// it is what lets a restart find the account again, so the operation is
     /// stopped rather than left able to strand a funding lock the app could no
-    /// longer spend. The technical cause lives in `Debug` and the logs.
+    /// longer spend. Covers non-retryable store failures (constraint, fatal);
+    /// retryable ones use [`Self::IdentityFundingAccountPersistBusy`].
     #[error(
-        "Your wallet could not save the account this payment needs. Check that your disk is not full, then try again."
+        "Your wallet could not save the information it needs for this payment. Restart the application and try again."
     )]
     IdentityFundingAccountPersistFailed {
         #[source]
         source: Box<platform_wallet::changeset::PersistenceError>,
     },
 
+    /// Like [`Self::IdentityFundingAccountPersistFailed`], but the store kept
+    /// reporting a retryable condition until the retry budget ran out. Upstream
+    /// classifies a busy database and a full disk alike, so the copy names both.
+    #[error(
+        "Your wallet could not save the information it needs for this payment because its data is busy or the disk is full. Close any other copy of Dash Evo Tool, make sure there is free disk space, and try again."
+    )]
+    IdentityFundingAccountPersistBusy {
+        #[source]
+        source: Box<platform_wallet::changeset::PersistenceError>,
+    },
+
     /// Single-key wallets are not supported in this version. Their data is
     /// preserved; HD (recovery-phrase) wallets remain fully functional.
     #[error(
@@ -423,6 +435,15 @@ pub enum TaskError {
         source: platform_wallet_storage::WalletStorageError,
     },
 
+    /// Another process currently owns the wallet database write lock.
+    #[error(
+        "Your wallet data is open in another Dash Evo Tool window or command-line session. Close it and try again."
+    )]
+    WalletStorageInUse {
+        #[source]
+        source: platform_wallet_storage::WalletStorageError,
+    },
+
     /// A pinned-PR wallet database could not be upgraded without losing data.
     #[error(transparent)]
     PlatformDatabaseUpgrade {
@@ -473,11 +494,11 @@ pub enum TaskError {
     ///
     /// The migration diagnostic is preserved through the `#[source]` chain for
     /// logs and the `Debug` view; it is kept out of the user-facing `Display`
-    /// copy. On the active development branch the storage layout changed in an
-    /// incompatible way, so the practical action is to remove the local wallet
-    /// data and let the app recreate it (see `docs/kv-keys.md`).
+    /// copy. The copy names only the rebuildable `.sqlite` stores and tells the
+    /// user to keep the `secrets` folder: the vault holds imported and
+    /// masternode keys that no recovery phrase can re-derive.
     #[error(
-        "Your wallet data is not compatible with this version of the app and cannot be opened. Remove the local wallet data so the app can create it fresh, then restart."
+        "Your wallet data was saved by an app version that this version cannot open, so it was left unchanged. Your recovery phrases and keys are still stored on this device. First open the app version you used before and write down the recovery phrase of every wallet. Separately, write down each imported private key. Close all Dash Evo Tool windows and command-line sessions. Then move each .sqlite file together with its matching .sqlite-wal and .sqlite-shm files, if present, out of the app data folder, leave the secrets folder where it is, and start this app again."
     )]
     WalletDataIncompatible {
         #[source]
@@ -1240,7 +1261,7 @@ pub enum TaskError {
     /// The identity and its private keys are gone, but at least one optional
     /// owner-scoped sidecar could not be removed.
     #[error(
-        "The identity was removed, but some DashPay or token-list data may still be stored on this device. The app will keep trying to clear this local data automatically."
+        "The identity was removed, but some local identity data or upgrade backups may still be stored on this device. The app will keep trying to clear this local data automatically."
     )]
     IdentitySidecarCleanupIncomplete,
 
@@ -2833,7 +2854,7 @@ impl TaskError {
 
     /// Map a wallet-storage open failure to the right user-facing variant.
     ///
-    /// Three storage failures get honest, distinct copy; everything else keeps
+    /// Four storage failures get honest, distinct copy; everything else keeps
     /// the generic disk/IO message:
     ///
     /// - A forward-version database (written by a newer build, schema beyond
@@ -2843,14 +2864,19 @@ impl TaskError {
     /// - A divergent migration history (e.g. a database written under an
     ///   earlier, incompatible storage layout that this build's migrations
     ///   cannot reconcile) is surfaced as [`Self::WalletDataIncompatible`] so
-    ///   the banner tells the user to remove the local wallet data — freeing
-    ///   disk space or restarting never resolves a structural mismatch.
+    ///   the banner tells the user to back up keys and set the databases aside
+    ///   while keeping the vault — freeing disk space or restarting never
+    ///   resolves a structural mismatch.
+    /// - A migration blocked by another SQLite writer is surfaced as
+    ///   [`Self::WalletStorageInUse`] so the user can close the competing
+    ///   process and retry instead of following incompatible-data recovery.
     /// - Every other storage failure keeps the generic disk/IO copy via
     ///   [`Self::WalletStorage`].
     ///
     /// Discrimination is on the typed upstream variant
     /// (`WalletStorageError::SchemaVersionUnsupported` /
-    /// `WalletStorageError::Migration`), never on its `Display` text.
+    /// `WalletStorageError::Migration`) and the typed migration source chain,
+    /// never on `Display` text.
     pub fn from_wallet_storage_open_error(
         source: platform_wallet_storage::WalletStorageError,
     ) -> Self {
@@ -2862,13 +2888,40 @@ impl TaskError {
                 found,
                 max_supported,
             },
+            other @ platform_wallet_storage::WalletStorageError::Migration(_)
+                if Self::wallet_storage_error_is_in_use(&other) =>
+            {
+                Self::WalletStorageInUse { source: other }
+            }
             other @ platform_wallet_storage::WalletStorageError::Migration(_) => {
                 Self::WalletDataIncompatible { source: other }
             }
             other => Self::WalletStorage { source: other },
         }
     }
 
+    fn wallet_storage_error_is_in_use(
+        source: &platform_wallet_storage::WalletStorageError,
+    ) -> bool {
+        let mut cause: Option<&(dyn std::error::Error + 'static)> = Some(source);
+        while let Some(error) = cause {
+            if error
+                .downcast_ref::<rusqlite::Error>()
+                .and_then(rusqlite::Error::sqlite_error_code)
+                .is_some_and(|code| {
+                    matches!(
+                        code,
+                        rusqlite::ErrorCode::DatabaseBusy | rusqlite::ErrorCode::DatabaseLocked
+                    )
+                })
+            {
+                return true;
+            }
+            cause = error.source();
+        }
+        false
+    }
+
     /// Returns `true` when this is a [`Self::SecretStore`] open failure caused
     /// specifically by a vault passphrase mismatch
     /// ([`SecretStoreError::WrongPassphrase`]).
@@ -5811,10 +5864,45 @@ mod tests {
             .expect_err("divergent checksum must abort")
     }
 
+    fn busy_migration_error() -> refinery::Error {
+        use refinery::{Migration, Runner};
+
+        let dir = tempfile::tempdir().expect("temporary directory");
+        let path = dir.path().join("wallet.sqlite");
+        let writer = rusqlite::Connection::open(&path).expect("writer connection");
+        writer
+            .execute_batch("BEGIN IMMEDIATE; CREATE TABLE held (id INTEGER);")
+            .expect("hold write lock");
+        let mut contender = rusqlite::Connection::open(&path).expect("contender connection");
+        contender
+            .busy_timeout(std::time::Duration::from_millis(1))
+            .expect("short deterministic timeout");
+        let migration = Migration::unapplied("V1__init", "CREATE TABLE data (id INTEGER);")
+            .expect("valid migration");
+        Runner::new(&[migration])
+            .run(&mut contender)
+            .expect_err("held writer must make the migration busy")
+    }
+
+    #[test]
+    fn busy_migration_error_maps_to_retryable_wallet_storage_message() {
+        let upstream =
+            platform_wallet_storage::WalletStorageError::Migration(busy_migration_error());
+        let err = TaskError::from_wallet_storage_open_error(upstream);
+        assert!(
+            matches!(err, TaskError::WalletStorageInUse { .. }),
+            "Expected WalletStorageInUse, got: {err:?}"
+        );
+        let message = err.to_string();
+        assert!(message.contains("Close") && message.contains("try again"));
+        assert!(!message.contains("incompatible"));
+        assert!(std::error::Error::source(&err).is_some());
+    }
+
     /// A divergent migration history (database written under an
     /// incompatible storage layout) maps to the dedicated
-    /// `WalletDataIncompatible` variant. Its `Display` tells the user to
-    /// remove the local wallet data, NOT the misleading "free disk space" copy.
+    /// `WalletDataIncompatible` variant. Its `Display` must never lead the user
+    /// to destroy the secrets vault, and must not blame disk space.
     #[test]
     fn migration_error_maps_to_wallet_data_incompatible() {
         let upstream =
@@ -5826,9 +5914,18 @@ mod tests {
         );
 
         let msg = err.to_string();
+        assert!(msg.contains(".sqlite-wal") && msg.contains(".sqlite-shm"));
+        assert!(msg.contains("Close all") && msg.contains("write down each imported private key"));
+        assert!(
+            msg.contains("write down the recovery phrase")
+                && msg.contains(".sqlite")
+                && msg.contains("leave the secrets folder where it is"),
+            "Expected scoped, vault-preserving guidance, got: {msg}"
+        );
+        let lower = msg.to_lowercase();
         assert!(
-            msg.contains("not compatible") && msg.contains("Remove"),
-            "Expected incompatibility guidance, got: {msg}"
+            !lower.contains("remove") && !lower.contains("delete"),
+            "Incompatible-data guidance must not tell the user to delete anything, got: {msg}"
         );
         assert!(
             !msg.contains("disk space"),

src/backend_task/identity/remove_identity.rs

@@ -149,6 +149,32 @@ mod tests {
             .any(|key_id| view.get(&MAIN, *key_id).unwrap().is_some())
     }
 
+    /// Upgrade backups copy the network database with every identity in it, so a
+    /// removed identity must not survive in them.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn removing_an_identity_deletes_upgrade_backups() {
+        let staged = stage_identity_with_vaulted_keys(HIGH, MEDIUM).await;
+        let database = crate::wallet_backend::wallet_database_path(
+            staged.ctx.data_dir(),
+            staged.ctx.network(),
+        );
+        let backup = database.with_file_name(format!(
+            "{}.platform-67d4ef3-backup-fixture.sqlite",
+            database.file_name().unwrap().to_string_lossy()
+        ));
+        std::fs::write(&backup, b"old identity rows").unwrap();
+
+        staged
+            .ctx
+            .remove_identity(staged.id)
+            .expect("the identity must be removed");
+
+        assert!(
+            !backup.exists(),
+            "the upgrade backup must be deleted together with the identity"
+        );
+    }
+
     /// A removal that fails before the identity is delisted did not happen:
     /// the identity is still on every screen, its keys are still on the
     /// device, and the button that started this still works. Reporting it as a

src/backend_task/mod.rs

@@ -230,14 +230,16 @@ fn identity_load_ticket(task: &BackendTask) -> Option<(Identifier, IdentityLoadT
 }
 
 /// Whether a wallet-backend build error is terminal (storage written by a
-/// newer/incompatible app build). These must surface their actionable
-/// message instead of being logged-and-discarded as a transient deferral
-/// (F50); every other init error is retried by the cold-boot bridge.
+/// newer/incompatible app build, or a compatibility upgrade that fails the
+/// same way on every attempt). These must surface their actionable message
+/// instead of being logged-and-discarded as a transient deferral (F50); every
+/// other init error is retried by the cold-boot bridge.
 pub(crate) fn is_terminal_storage_open_error(error: &TaskError) -> bool {
-    matches!(
-        error,
-        TaskError::WalletDataTooNew { .. } | TaskError::WalletDataIncompatible { .. }
-    )
+    match error {
+        TaskError::WalletDataTooNew { .. } | TaskError::WalletDataIncompatible { .. } => true,
+        TaskError::PlatformDatabaseUpgrade { source } => !source.is_retryable(),
+        _ => false,
+    }
 }
 
 /// Information about fees for a platform state transition.
@@ -2331,6 +2333,24 @@ mod tests {
                 )),
             }
         ));
+        use crate::wallet_backend::platform_compatibility::UpgradeError;
+        for source in [
+            UpgradeError::Unrecognized,
+            UpgradeError::IdentityRoster { source: None },
+            UpgradeError::Verification,
+            UpgradeError::TypedValidation(Box::new(std::io::Error::other("fixture"))),
+        ] {
+            assert!(
+                is_terminal_storage_open_error(&TaskError::PlatformDatabaseUpgrade { source }),
+                "a deterministic upgrade failure must surface instead of re-running"
+            );
+        }
+        // A user-clearable upgrade failure keeps offering a retry.
+        assert!(!is_terminal_storage_open_error(
+            &TaskError::PlatformDatabaseUpgrade {
+                source: UpgradeError::from(std::io::Error::from(std::io::ErrorKind::StorageFull)),
+            }
+        ));
         // A transient pre-wire state must NOT be treated as terminal.
         assert!(!is_terminal_storage_open_error(
             &TaskError::WalletBackendNotYetWired