fix(windows): keep crash-recovery autosaves out of the roaming profile (#5648) (#6412)

* fix(windows): keep crash-recovery autosaves out of the roaming profile (#5648)

* test(windows): cover the local recovery folder and moving old autosaves (#5648)

apps/photocraft/src/app_dirs.rs

@@ -9,6 +9,9 @@
 //!    folder can't be written (read-only medium, Program Files), the app warns and falls back.
 //! 3. The platform convention: macOS `~/Library/Application Support/Photocraft`, Windows
 //!    `%APPDATA%\Photocraft`, Linux `$XDG_CONFIG_HOME/photocraft` or `~/.config/photocraft`.
+//!
+//! One exception: on Windows the platform convention keeps crash-recovery autosaves out of the
+//! roaming profile ([`recovery_dir`]).
 
 use std::ffi::OsString;
 use std::path::{Path, PathBuf};
@@ -60,6 +63,379 @@ pub fn config_dir() -> Option<PathBuf> {
     current().dir.clone()
 }
 
+/// The crash-recovery folder's name.
+const RECOVERY: &str = "Recovery";
+
+/// Where crash-recovery autosaves live, with any left in the settings directory by an earlier
+/// version moved there first ([`adopt_legacy_recovery`]). Call it once, at startup.
+pub fn recovery_dir() -> Option<PathBuf> {
+    let data = current();
+    let dir = resolve_recovery(data, |k| std::env::var_os(k))?;
+    Some(match data.dir.as_ref().map(|d| d.join(RECOVERY)) {
+        Some(legacy) => adopt_legacy_recovery(&legacy, &dir),
+        None => dir,
+    })
+}
+
+/// The recovery folder for `data` (#5648). Autosaves hold whole documents and are rewritten every
+/// few minutes, so on Windows the platform convention puts them in the machine-local
+/// `%LOCALAPPDATA%\Photocraft\Recovery`: the roaming `%APPDATA%` may be synced at sign-in and
+/// sign-out or capped by policy. They stay out of `%TEMP%`, which cleanup tools may empty before
+/// a crashed document is recovered. The override and portable folders keep everything in one
+/// place, and other platforms already keep these folders local.
+pub fn resolve_recovery(data: &DataDir, env: impl Fn(&str) -> Option<OsString>) -> Option<PathBuf> {
+    if cfg!(windows)
+        && data.mode == Mode::Platform
+        && let Some(local) = env("LOCALAPPDATA").filter(|l| !l.is_empty())
+    {
+        return Some(PathBuf::from(local).join("Photocraft").join(RECOVERY));
+    }
+    data.dir.as_ref().map(|d| d.join(RECOVERY))
+}
+
+/// The lock file, beside the recovery folder, that keeps two PhotoCraft processes starting at
+/// once (`--new-instance`, control launches) from moving the same autosaves.
+const MIGRATION_LOCK: &str = "recovery-migration.lock";
+
+/// How long a launch waits for another one's move before going ahead without moving anything.
+const MIGRATION_WAIT: std::time::Duration = std::time::Duration::from_secs(5);
+
+/// Suffix of a copy staged for publishing ([`stage_copy`]): disposable, the original still exists.
+const STAGED: &str = ".moving";
+
+/// Suffix of an original set aside while it is copied to another volume ([`move_across`]): it
+/// may be the only copy, so it is put back, never thrown away, if the move doesn't finish.
+const HELD: &str = ".held";
+
+/// Move autosaves from `legacy` (the recovery folder of versions before #5648) into `dir`, so a
+/// crash just before an update is still offered for recovery. Returns the folder this process
+/// uses for its recovery store. That is `legacy` only when `dir` doesn't exist and no process can
+/// be moving autosaves (the move failed, or its lock can't be made), so nothing deletes what is
+/// written there. A launch that finds another one moving uses `dir`, which the move never
+/// replaces or removes. See [`adopt_locked`] for how they move.
+pub fn adopt_legacy_recovery(legacy: &Path, dir: &Path) -> PathBuf {
+    adopt_legacy_recovery_waiting(legacy, dir, MIGRATION_WAIT)
+}
+
+/// [`adopt_legacy_recovery`], waiting up to `wait` for another process's move.
+fn adopt_legacy_recovery_waiting(legacy: &Path, dir: &Path, wait: std::time::Duration) -> PathBuf {
+    let held_aside = || legacy.parent().is_some_and(|p| !side_entries(p, legacy, HELD).is_empty());
+    if legacy == dir || (!legacy.is_dir() && !held_aside()) {
+        return dir.to_path_buf();
+    }
+    // An empty old folder has nothing to move (`remove_dir` only removes an empty one).
+    if !held_aside() && std::fs::remove_dir(legacy).is_ok() {
+        return dir.to_path_buf();
+    }
+    let unmoved = || if dir.exists() || !legacy.is_dir() { dir.to_path_buf() } else { legacy.to_path_buf() };
+    let Some(parent) = dir.parent() else { return unmoved() };
+    let lock =
+        std::fs::create_dir_all(parent).and_then(|()| std::fs::OpenOptions::new().create(true).truncate(false).write(true).open(parent.join(MIGRATION_LOCK)));
+    let Ok(lock) = lock else {
+        // No lock file means no other process can be moving: the old folder is safe to use.
+        log::warn!("couldn't open the recovery move lock in {}; autosaves stay where they are", parent.display());
+        return unmoved();
+    };
+    let deadline = std::time::Instant::now() + wait;
+    loop {
+        match lock.try_lock() {
+            Ok(()) => break,
+            Err(std::fs::TryLockError::WouldBlock) if std::time::Instant::now() < deadline => std::thread::sleep(std::time::Duration::from_millis(50)),
+            Err(std::fs::TryLockError::WouldBlock) => {
+                // Another process is moving and may yet remove the old folder: never write there.
+                log::info!("another PhotoCraft is moving crash-recovery autosaves; using {}", dir.display());
+                return dir.to_path_buf();
+            }
+            Err(std::fs::TryLockError::Error(e)) => {
+                // Locking fails for every process here alike, so none can be moving.
+                log::warn!("couldn't lock {}: {e}; autosaves stay where they are", parent.join(MIGRATION_LOCK).display());
+                return unmoved();
+            }
+        }
+    }
+    // The lock file stays (removing it would race a process about to lock it); the OS releases
+    // the lock when `lock` is dropped or the process ends.
+    let used = adopt_locked(legacy, dir);
+    if used == legacy {
+        // This process writes its autosaves to the old folder: it keeps the lock for as long as it
+        // runs, so no other launch moves (and removes) the folder under it.
+        KEPT_LOCKS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push(lock);
+    }
+    used
+}
+
+/// Migration locks of the old folders this process uses (see [`adopt_legacy_recovery`]), kept
+/// until it ends.
+static KEPT_LOCKS: std::sync::Mutex<Vec<std::fs::File>> = std::sync::Mutex::new(Vec::new());
+
+/// The move itself, under the migration lock. First, what a process that ended mid-move left is
+/// tidied: originals it set aside are put back, copies it staged are removed. An empty `legacy`
+/// is then just removed. Otherwise the whole folder moves if `dir` doesn't exist yet; when both
+/// exist, each autosave (`<key>.pcraft` bundle and `<key>.json` sidecar, see
+/// `photocraft_format::autosave`) moves as a whole, bundle first, unless `dir` already has either
+/// part, and one that stays is left in `legacy` untouched. Every move is a [`move_path`].
+fn adopt_locked(legacy: &Path, dir: &Path) -> PathBuf {
+    if let Some(parent) = legacy.parent() {
+        for held in side_entries(parent, legacy, HELD) {
+            put_back(&held, legacy);
+        }
+    }
+    // Autosave parts held aside inside the old folder by a per-autosave move.
+    let held_parts: Vec<(PathBuf, String)> = std::fs::read_dir(legacy)
+        .into_iter()
+        .flatten()
+        .flatten()
+        .filter_map(|e| e.file_name().to_str().and_then(|n| side_of(n, HELD)).map(|orig| (e.path(), orig.to_owned())))
+        .collect();
+    for (held, original) in held_parts {
+        put_back(&held, &legacy.join(original));
+    }
+    for folder in [dir.parent(), Some(dir)].into_iter().flatten() {
+        remove_stale_staging(folder);
+    }
+    // Another process may have finished the move while this one waited.
+    if !legacy.is_dir() {
+        return dir.to_path_buf();
+    }
+    if std::fs::remove_dir(legacy).is_ok() {
+        return dir.to_path_buf();
+    }
+    if !dir.exists() {
+        // The common case: the whole folder moves at once.
+        return match move_path(legacy, dir) {
+            Ok(()) => {
+                log::info!("moved crash-recovery autosaves from {} to {}", legacy.display(), dir.display());
+                dir.to_path_buf()
+            }
+            Err(e) => {
+                log::warn!("couldn't move crash-recovery autosaves from {} to {} ({e}); using the old folder", legacy.display(), dir.display());
+                if legacy.is_dir() { legacy.to_path_buf() } else { dir.to_path_buf() }
+            }
+        };
+    }
+    move_entries(legacy, dir);
+    // Only an emptied folder goes; anything that couldn't move stays for a later launch.
+    let _ = std::fs::remove_dir(legacy);
+    dir.to_path_buf()
+}
+
+/// Move each autosave of `from` into `to` (both folders): bundle first, then sidecar, skipped if
+/// `to` has either part. Temp files of an interrupted write (`photocraft_format::atomic`) go
+/// along too, for the recovery store's startup sweep there.
+fn move_entries(from: &Path, to: &Path) {
+    let Ok(entries) = std::fs::read_dir(from) else { return };
+    let names: Vec<String> = entries.flatten().filter_map(|e| e.file_name().to_str().map(str::to_owned)).collect();
+    let keys: std::collections::BTreeSet<&str> = names.iter().filter_map(|n| n.strip_suffix(".pcraft").or_else(|| n.strip_suffix(".json"))).collect();
+    for key in keys {
+        let parts = [format!("{key}.pcraft"), format!("{key}.json")];
+        if parts.iter().any(|p| std::fs::symlink_metadata(to.join(p)).is_ok()) {
+            continue;
+        }
+        // A bundle that moves without its sidecar is still listed (`list_recovery` reads a
+        // committed bundle's manifest); a sidecar must never move without its bundle.
+        for part in &parts {
+            let src = from.join(part);
+            if std::fs::symlink_metadata(&src).is_ok()
+                && let Err(e) = move_path(&src, &to.join(part))
+            {
+                log::warn!("couldn't move {} to {}: {e}", src.display(), to.display());
+                break;
+            }
+        }
+    }
+    for name in names.iter().filter(|n| n.ends_with(".pcsave.tmp")) {
+        let _ = move_path(&from.join(name), &to.join(name));
+    }
+}
+
+/// How deep [`copy_tree`] follows folders: an autosave bundle is a few levels deep.
+const MAX_COPY_DEPTH: u32 = 16;
+
+fn already_exists(path: &Path) -> std::io::Error {
+    std::io::Error::new(std::io::ErrorKind::AlreadyExists, format!("{} already exists", path.display()))
+}
+
+/// A path beside `of`, named `.<name>.<pid>-<n><suffix>`, that no other call uses.
+fn side_path(of: &Path, suffix: &str) -> std::io::Result<PathBuf> {
+    static NEXT: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0);
+    let parent = of.parent().ok_or_else(|| std::io::Error::other(format!("{} has no parent folder", of.display())))?;
+    let name = of.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
+    Ok(parent.join(format!(".{name}.{}-{}{suffix}", std::process::id(), NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed))))
+}
+
+/// The name `side` was made from, if it is a [`side_path`] name with `suffix`.
+fn side_of<'a>(side: &'a str, suffix: &str) -> Option<&'a str> {
+    let rest = side.strip_prefix('.')?.strip_suffix(suffix)?;
+    let (name, ids) = rest.rsplit_once('.')?;
+    let (pid, n) = ids.split_once('-')?;
+    let digits = |s: &str| !s.is_empty() && s.bytes().all(|b| b.is_ascii_digit());
+    (!name.is_empty() && digits(pid) && digits(n)).then_some(name)
+}
+
+/// The side paths with `suffix` in `folder` made from `of`.
+fn side_entries(folder: &Path, of: &Path, suffix: &str) -> Vec<PathBuf> {
+    let Some(name) = of.file_name().and_then(|n| n.to_str()) else { return Vec::new() };
+    std::fs::read_dir(folder)
+        .into_iter()
+        .flatten()
+        .flatten()
+        .filter(|e| e.file_name().to_str().and_then(|n| side_of(n, suffix)) == Some(name))
+        .map(|e| e.path())
+        .collect()
+}
+
+/// Move the file or folder `from` to `to` without ever replacing anything at `to`. On one volume
+/// a folder is renamed (Windows refuses an existing target; elsewhere only an empty folder could
+/// be replaced) and a file is hard-linked, which fails on an existing target, and then unlinked
+/// at `from`. Otherwise (another volume, e.g. a roaming profile redirected to a network share
+/// while `%LOCALAPPDATA%` stays local) it is copied ([`move_across`]).
+fn move_path(from: &Path, to: &Path) -> std::io::Result<()> {
+    if std::fs::symlink_metadata(to).is_ok() {
+        return Err(already_exists(to));
+    }
+    let meta = std::fs::symlink_metadata(from)?;
+    let moved = if meta.is_dir() { std::fs::rename(from, to) } else { std::fs::hard_link(from, to).and_then(|()| std::fs::remove_file(from)) };
+    match moved {
+        Ok(()) => Ok(()),
+        Err(e) if matches!(e.kind(), std::io::ErrorKind::AlreadyExists | std::io::ErrorKind::DirectoryNotEmpty) => Err(e),
+        Err(_) => move_across(from, to),
+    }
+}
+
+/// Move `from` to `to` by copying. `from` is first renamed aside on its own volume
+/// (`.<name>.<pid>-<n>.held`, [`hold`]), so a process still writing to the old path (another
+/// launch that couldn't take the lock, an older PhotoCraft) starts a new `from` there instead of
+/// adding to what is copied, and nothing it writes is removed. The held original is copied to a
+/// staged path ([`stage_copy`]), published without replacing anything ([`publish`]), and only
+/// then removed. If the copy or publish fails, the held original is put back ([`put_back`]).
+fn move_across(from: &Path, to: &Path) -> std::io::Result<()> {
+    let held = hold(from)?;
+    match stage_copy(&held, to).and_then(|staged| publish(&staged, to)) {
+        Ok(()) => {
+            let removed = if std::fs::symlink_metadata(&held)?.is_dir() { std::fs::remove_dir_all(&held) } else { std::fs::remove_file(&held) };
+            if let Err(e) = removed {
+                log::warn!("copied {} to {} but couldn't remove {}: {e}", from.display(), to.display(), held.display());
+            }
+            Ok(())
+        }
+        Err(e) => {
+            put_back(&held, from);
+            Err(e)
+        }
+    }
+}
+
+/// Rename `from` aside on its own volume (see [`move_across`]); returns where it is now.
+fn hold(from: &Path) -> std::io::Result<PathBuf> {
+    let held = side_path(from, HELD)?;
+    std::fs::rename(from, &held)?;
+    Ok(held)
+}
+
+/// Return the held original `held` to `original`. If `original` has been made again meanwhile (a
+/// writer that kept using the old path), the held folder's entries move into it without replacing
+/// anything ([`move_entries`]); whatever can't move stays held, logged, and is tried again under
+/// the next move's lock ([`adopt_locked`] puts back what a process that ended mid-move held).
+/// Nothing held is ever removed with data in it.
+fn put_back(held: &Path, original: &Path) {
+    if std::fs::symlink_metadata(original).is_err() && std::fs::rename(held, original).is_ok() {
+        return;
+    }
+    if held.is_dir() && original.is_dir() {
+        move_entries(held, original);
+        if std::fs::remove_dir(held).is_ok() {
+            return;
+        }
+    }
+    log::warn!("{} is kept aside: {} exists", held.display(), original.display());
+}
+
+/// Copy `from` to a staging path beside `to` that this call creates exclusively, so nothing
+/// another process made is ever written to or cleaned up. On failure the staged copy is removed.
+fn stage_copy(from: &Path, to: &Path) -> std::io::Result<PathBuf> {
+    let staged = side_path(to, STAGED)?;
+    let is_dir = std::fs::symlink_metadata(from)?.is_dir();
+    let copied = if is_dir { copy_tree(from, &staged, 0) } else { copy_file(from, &staged) };
+    if let Err(e) = copied {
+        // `copy_tree` and `copy_file` create `staged` exclusively, and everything under a
+        // fresh folder is new: `AlreadyExists` means `staged` was someone else's to keep.
+        if e.kind() != std::io::ErrorKind::AlreadyExists {
+            let _ = if is_dir { std::fs::remove_dir_all(&staged) } else { std::fs::remove_file(&staged) };
+        }
+        return Err(e);
+    }
+    Ok(staged)
+}
+
+/// Remove copies a process left staged in `folder` when it ended mid-move: disposable, since the
+/// original is still in place or held aside. Called only under the migration lock, which every
+/// staging happens under. Links are never followed.
+fn remove_stale_staging(folder: &Path) {
+    let Ok(entries) = std::fs::read_dir(folder) else { return };
+    for entry in entries.flatten() {
+        if !entry.file_name().to_str().is_some_and(|n| side_of(n, STAGED).is_some()) {
+            continue;
+        }
+        let removed = match entry.file_type() {
+            Ok(t) if t.is_dir() => std::fs::remove_dir_all(entry.path()),
+            Ok(t) if t.is_file() => std::fs::remove_file(entry.path()),
+            _ => continue,
+        };
+        if let Err(e) = removed {
+            log::warn!("couldn't remove the stale staged copy {}: {e}", entry.path().display());
+        }
+    }
+}
+
+/// Give the staged copy its final name `to` without replacing anything there (see [`move_path`]
+/// for the folder and file cases). On failure the staged copy is removed and `to` is untouched.
+fn publish(staged: &Path, to: &Path) -> std::io::Result<()> {
+    let is_dir = std::fs::symlink_metadata(staged)?.is_dir();
+    let published = if std::fs::symlink_metadata(to).is_ok() {
+        Err(already_exists(to))
+    } else if is_dir {
+        std::fs::rename(staged, to)
+    } else {
+        std::fs::hard_link(staged, to).and_then(|()| std::fs::remove_file(staged))
+    };
+    if published.is_err() {
+        let _ = if is_dir { std::fs::remove_dir_all(staged) } else { std::fs::remove_file(staged) };
+    }
+    published
+}
+
+/// Copy the folder `from` to the new folder `to` (created here; an existing `to` is an error):
+/// plain files and folders only (a link or other special file is refused), at most
+/// [`MAX_COPY_DEPTH`] levels.
+fn copy_tree(from: &Path, to: &Path, depth: u32) -> std::io::Result<()> {
+    if depth > MAX_COPY_DEPTH {
+        return Err(std::io::Error::other(format!("{} is nested too deep", from.display())));
+    }
+    std::fs::create_dir(to)?;
+    for entry in std::fs::read_dir(from)? {
+        let entry = entry?;
+        let target = to.join(entry.file_name());
+        if entry.file_type()?.is_dir() {
+            copy_tree(&entry.path(), &target, depth.saturating_add(1))?;
+        } else {
+            copy_file(&entry.path(), &target)?;
+        }
+    }
+    Ok(())
+}
+
+/// Copy one plain file (a link or other special file is refused) to the new file `to` (an
+/// existing `to` is an error), flushed to disk.
+fn copy_file(from: &Path, to: &Path) -> std::io::Result<()> {
+    if !std::fs::symlink_metadata(from)?.file_type().is_file() {
+        return Err(std::io::Error::other(format!("{} is not a plain file", from.display())));
+    }
+    let mut src = std::fs::File::open(from)?;
+    let mut dst = std::fs::OpenOptions::new().write(true).create_new(true).open(to)?;
+    std::io::copy(&mut src, &mut dst)?;
+    dst.sync_all()
+}
+
 /// Resolve the data directory from the environment (`env`) and the executable's folder.
 pub fn resolve(env: impl Fn(&str) -> Option<OsString>, exe_dir: Option<&Path>) -> DataDir {
     if let Some(d) = env("PHOTOCRAFT_CONFIG_DIR").filter(|d| !d.is_empty()) {
@@ -223,6 +599,300 @@ mod tests {
         let _ = std::fs::remove_dir_all(&root);
     }
 
+    /// #5648: on Windows, autosaves go to the machine-local `%LOCALAPPDATA%`, not the roaming
+    /// profile; an override or portable folder keeps them inside itself.
+    #[test]
+    fn autosaves_stay_out_of_the_roaming_profile() {
+        let root = temp("recovery-local");
+        let settings = root.join("appdata").join("Photocraft");
+        let local = root.join("local");
+        let env = env_of(&[("LOCALAPPDATA", &local)]);
+        let platform = DataDir { dir: Some(settings.clone()), mode: Mode::Platform, warning: None };
+        let expected = if cfg!(windows) { local.join("Photocraft").join("Recovery") } else { settings.join("Recovery") };
+        assert_eq!(resolve_recovery(&platform, &env), Some(expected));
+        for mode in [Mode::Override, Mode::Portable] {
+            let d = DataDir { dir: Some(settings.clone()), mode, warning: None };
+            assert_eq!(resolve_recovery(&d, &env), Some(settings.join("Recovery")), "{mode:?}");
+        }
+        // No (or an empty) LOCALAPPDATA: beside the settings, as before.
+        let empty = PathBuf::new();
+        assert_eq!(resolve_recovery(&platform, env_of(&[("LOCALAPPDATA", &empty)])), Some(settings.join("Recovery")));
+        assert_eq!(resolve_recovery(&platform, |_| None), Some(settings.join("Recovery")));
+        let nowhere = DataDir { dir: None, mode: Mode::Platform, warning: None };
+        assert_eq!(resolve_recovery(&nowhere, |_| None), None);
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// An autosave bundle and its sidecar, as `format::Autosaver` writes them.
+    fn autosave(dir: &Path, key: &str) {
+        std::fs::create_dir_all(dir.join(format!("{key}.pcraft"))).unwrap();
+        std::fs::write(dir.join(format!("{key}.pcraft")).join("manifest.json"), key).unwrap();
+        std::fs::write(dir.join(format!("{key}.json")), key).unwrap();
+    }
+
+    #[test]
+    fn autosaves_from_the_old_folder_move_to_the_new_one() {
+        let root = temp("recovery-move");
+        let legacy = root.join("roaming").join("Recovery");
+        let dir = root.join("local").join("Photocraft").join("Recovery");
+        autosave(&legacy, "a");
+        assert_eq!(adopt_legacy_recovery(&legacy, &dir), dir);
+        assert!(!legacy.exists(), "the old folder is gone");
+        assert_eq!(std::fs::read_to_string(dir.join("a.pcraft").join("manifest.json")).unwrap(), "a");
+        assert!(dir.join("a.json").is_file());
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    #[test]
+    fn autosaves_merge_into_an_existing_folder_without_overwriting() {
+        let root = temp("recovery-merge");
+        let legacy = root.join("roaming").join("Recovery");
+        let dir = root.join("local").join("Recovery");
+        autosave(&legacy, "old");
+        autosave(&legacy, "both");
+        // A temp file of an interrupted write moves along, for the recovery store's sweep.
+        let temp_file = ".old.json.4294967294-0.pcsave.tmp";
+        std::fs::write(legacy.join(temp_file), b"partial").unwrap();
+        std::fs::create_dir_all(&dir).unwrap();
+        std::fs::write(dir.join("both.json"), "newer").unwrap();
+        assert_eq!(adopt_legacy_recovery(&legacy, &dir), dir);
+        assert!(dir.join(temp_file).is_file() && !legacy.join(temp_file).exists());
+        assert!(dir.join("old.pcraft").is_dir() && dir.join("old.json").is_file(), "a whole autosave moves");
+        assert_eq!(std::fs::read_to_string(dir.join("both.json")).unwrap(), "newer", "never overwritten");
+        assert!(!dir.join("both.pcraft").exists(), "a clashing autosave isn't split");
+        assert!(legacy.join("both.pcraft").is_dir() && legacy.join("both.json").is_file(), "it stays behind whole");
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    #[test]
+    fn autosaves_that_cannot_move_keep_being_used_where_they_are() {
+        let root = temp("recovery-stuck");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "a");
+        // A file where the new folder's parent should be: nothing can be created there.
+        std::fs::write(root.join("blocked"), b"in the way").unwrap();
+        let dir = root.join("blocked").join("Recovery");
+        assert_eq!(adopt_legacy_recovery(&legacy, &dir), legacy);
+        assert!(legacy.join("a.json").is_file(), "nothing lost");
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    #[test]
+    fn an_empty_old_folder_is_removed_even_where_it_cannot_move() {
+        let root = temp("recovery-empty");
+        let legacy = root.join("roaming").join("Recovery");
+        std::fs::create_dir_all(&legacy).unwrap();
+        std::fs::write(root.join("blocked"), b"in the way").unwrap();
+        let dir = root.join("blocked").join("Recovery");
+        assert_eq!(adopt_legacy_recovery(&legacy, &dir), dir, "nothing to keep listed");
+        assert!(!legacy.exists(), "the empty old folder is gone");
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// Where a rename can't cross volumes (a roaming profile on a network share), an autosave
+    /// is copied whole, nested folders included, and only then removed.
+    #[test]
+    fn a_move_across_volumes_copies_then_removes() {
+        let root = temp("recovery-copy");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "a");
+        std::fs::create_dir_all(legacy.join("a.pcraft").join("tiles")).unwrap();
+        std::fs::write(legacy.join("a.pcraft").join("tiles").join("t0.zst"), b"tile").unwrap();
+        let dir = root.join("local").join("Recovery");
+        std::fs::create_dir_all(root.join("local")).unwrap();
+        move_across(&legacy, &dir).unwrap();
+        assert!(!legacy.exists(), "the original is removed after the copy");
+        assert_eq!(side_leftovers(&root.join("roaming")), 0, "nothing is left held aside");
+        assert_eq!(std::fs::read_to_string(dir.join("a.pcraft").join("manifest.json")).unwrap(), "a");
+        assert_eq!(std::fs::read(dir.join("a.pcraft").join("tiles").join("t0.zst")).unwrap(), b"tile");
+        assert_eq!(std::fs::read_to_string(dir.join("a.json")).unwrap(), "a");
+        // One file on its own, as the per-autosave moves do.
+        move_across(&dir.join("a.json"), &root.join("a.json")).unwrap();
+        assert!(!dir.join("a.json").exists() && root.join("a.json").is_file());
+        assert_eq!(side_leftovers(&dir) + side_leftovers(&root), 0);
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    #[test]
+    fn a_failed_copy_puts_the_original_back_and_leaves_no_partial_copy() {
+        let root = temp("recovery-copy-fail");
+        let legacy = root.join("Recovery");
+        autosave(&legacy, "a");
+        // Deeper than any bundle: the copy gives up part way.
+        let mut deep = legacy.join("a.pcraft");
+        for _ in 0..=MAX_COPY_DEPTH {
+            deep = deep.join("d");
+        }
+        std::fs::create_dir_all(&deep).unwrap();
+        let dir = root.join("Local");
+        assert!(move_across(&legacy, &dir).is_err());
+        assert!(!dir.exists(), "nothing is published");
+        assert_eq!(side_leftovers(&root), 0, "no staged copy, nothing left held aside");
+        assert!(legacy.join("a.json").is_file() && deep.is_dir(), "the original is back in place");
+        // A target that already exists is never copied into or removed.
+        std::fs::create_dir_all(&dir).unwrap();
+        std::fs::write(dir.join("keep"), b"mine").unwrap();
+        assert!(move_across(&legacy.join("a.json"), &dir).is_err());
+        assert_eq!(std::fs::read(dir.join("keep")).unwrap(), b"mine");
+        assert!(legacy.join("a.json").is_file());
+        assert_eq!(side_leftovers(&root) + side_leftovers(&legacy), 0);
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// Staged (`.moving`) and held (`.held`) side paths left in `dir`.
+    fn side_leftovers(dir: &Path) -> usize {
+        std::fs::read_dir(dir)
+            .unwrap()
+            .flatten()
+            .filter(|e| e.file_name().to_str().is_some_and(|n| side_of(n, STAGED).is_some() || side_of(n, HELD).is_some()))
+            .count()
+    }
+
+    /// The target appears after the copy is staged (another process got there first): publishing
+    /// fails, that process's target keeps its contents, the staged copy is removed and the
+    /// original is left as it was. Folder and file alike.
+    #[test]
+    fn a_target_that_appears_while_copying_is_never_replaced_or_removed() {
+        let root = temp("recovery-race");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "a");
+        let local = root.join("local");
+        std::fs::create_dir_all(&local).unwrap();
+        // A whole folder.
+        let dir = local.join("Recovery");
+        let staged = stage_copy(&legacy, &dir).unwrap();
+        autosave(&dir, "theirs");
+        let e = publish(&staged, &dir).unwrap_err();
+        assert_eq!(e.kind(), std::io::ErrorKind::AlreadyExists);
+        assert_eq!(std::fs::read_to_string(dir.join("theirs.json")).unwrap(), "theirs", "their folder is intact");
+        assert!(!dir.join("a.json").exists(), "nothing was copied into it");
+        assert!(!staged.exists() && side_leftovers(&local) == 0, "only the staged copy is removed");
+        assert!(legacy.join("a.json").is_file(), "the original is untouched");
+        // One file.
+        let to = dir.join("a.json");
+        let staged = stage_copy(&legacy.join("a.json"), &to).unwrap();
+        std::fs::write(&to, "theirs").unwrap();
+        assert_eq!(publish(&staged, &to).unwrap_err().kind(), std::io::ErrorKind::AlreadyExists);
+        assert_eq!(std::fs::read_to_string(&to).unwrap(), "theirs");
+        assert!(!staged.exists() && legacy.join("a.json").is_file());
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// A process holds the lock and is copying the old folder (held aside, copy staged) when a
+    /// second launch gives up waiting. The second launch must get the new folder, never the old
+    /// one, and what it writes there survives the first one finishing: the publish refuses the
+    /// now-existing folder and the original is put back, so both autosaves stay (#6412 review).
+    #[test]
+    fn a_launch_that_times_out_never_writes_where_a_move_removes() {
+        let root = temp("recovery-timeout");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "old");
+        let local = root.join("local");
+        std::fs::create_dir_all(&local).unwrap();
+        let dir = local.join("Recovery");
+        // First process: takes the lock, sets the original aside and stages its copy, then pauses.
+        let first = std::fs::OpenOptions::new().create(true).truncate(false).write(true).open(local.join(MIGRATION_LOCK)).unwrap();
+        first.lock().unwrap();
+        let held = hold(&legacy).unwrap();
+        let staged = stage_copy(&held, &dir).unwrap();
+        // Second launch: times out and writes a newer autosave where it was told to.
+        let chosen = adopt_legacy_recovery_waiting(&legacy, &dir, std::time::Duration::ZERO);
+        assert_eq!(chosen, dir, "a launch that couldn't get the lock never uses the old folder");
+        autosave(&chosen, "new");
+        // First process resumes: publish, and on failure put the original back (as `move_across`).
+        assert_eq!(publish(&staged, &dir).unwrap_err().kind(), std::io::ErrorKind::AlreadyExists);
+        put_back(&held, &legacy);
+        drop(first);
+        assert_eq!(std::fs::read_to_string(dir.join("new.json")).unwrap(), "new", "the newer autosave survives");
+        assert_eq!(std::fs::read_to_string(legacy.join("old.json")).unwrap(), "old", "the old one is back in place");
+        assert_eq!(side_leftovers(&local) + side_leftovers(&root.join("roaming")), 0);
+        // The next launch merges them.
+        assert_eq!(adopt_legacy_recovery_waiting(&legacy, &dir, std::time::Duration::ZERO), dir);
+        assert!(dir.join("old.json").is_file() && dir.join("new.json").is_file() && !legacy.exists());
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// A writer that still uses the old path while it is being copied (an older PhotoCraft)
+    /// starts a new folder there; the move removes only the original it held aside.
+    #[test]
+    fn a_write_to_the_old_path_during_a_copy_is_not_removed() {
+        let root = temp("recovery-old-writer");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "old");
+        let local = root.join("local");
+        std::fs::create_dir_all(&local).unwrap();
+        let dir = local.join("Recovery");
+        let held = hold(&legacy).unwrap();
+        let staged = stage_copy(&held, &dir).unwrap();
+        autosave(&legacy, "new");
+        publish(&staged, &dir).unwrap();
+        std::fs::remove_dir_all(&held).unwrap();
+        assert_eq!(std::fs::read_to_string(dir.join("old.json")).unwrap(), "old");
+        assert_eq!(std::fs::read_to_string(legacy.join("new.json")).unwrap(), "new", "the new write is untouched");
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// What a process killed mid-move left is tidied at the next move: a held original is put
+    /// back (it may be the only copy) and moved, a staged copy is removed; other dot-files and
+    /// look-alike names are not touched.
+    #[test]
+    fn a_killed_move_is_tidied_next_time() {
+        let root = temp("recovery-stale");
+        let roaming = root.join("roaming");
+        let legacy = roaming.join("Recovery");
+        autosave(&legacy, "a");
+        let held = hold(&legacy).unwrap();
+        assert!(!legacy.exists() && held.is_dir());
+        let local = root.join("local");
+        let dir = local.join("Recovery");
+        let stale = local.join(".Recovery.4294967294-3.moving");
+        autosave(&stale, "half");
+        let kept = [local.join(".Recovery.moving"), local.join(".keep.1-x.moving"), local.join("notes.txt")];
+        for k in &kept {
+            std::fs::write(k, b"x").unwrap();
+        }
+        assert_eq!(adopt_legacy_recovery_waiting(&legacy, &dir, std::time::Duration::ZERO), dir);
+        assert_eq!(std::fs::read_to_string(dir.join("a.json")).unwrap(), "a", "the held original was put back and moved");
+        assert!(!stale.exists(), "the stale staged copy is gone");
+        assert_eq!(side_leftovers(&roaming), 0);
+        assert!(kept.iter().all(|k| k.is_file()), "only side names are touched");
+        assert_eq!(side_of(".a.json.12-0.moving", STAGED), Some("a.json"));
+        assert!(side_of("a.json.12-0.moving", STAGED).is_none() && side_of(".a.json.-0.moving", STAGED).is_none());
+        assert!(side_of(".a.json.12-0.held", STAGED).is_none() && side_of(".a.json.12-0.held", HELD) == Some("a.json"));
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    /// While another process holds the move lock, a launch moves nothing and uses the new folder;
+    /// once the lock is released, the move happens.
+    #[test]
+    fn a_move_waits_for_another_process_holding_the_lock() {
+        let root = temp("recovery-lock");
+        let legacy = root.join("roaming").join("Recovery");
+        autosave(&legacy, "a");
+        let dir = root.join("local").join("Recovery");
+        std::fs::create_dir_all(root.join("local")).unwrap();
+        let held = std::fs::OpenOptions::new().create(true).truncate(false).write(true).open(root.join("local").join(MIGRATION_LOCK)).unwrap();
+        held.lock().unwrap();
+        assert_eq!(adopt_legacy_recovery_waiting(&legacy, &dir, std::time::Duration::ZERO), dir);
+        assert!(legacy.join("a.json").is_file() && !dir.exists(), "nothing moved under another's lock");
+        drop(held);
+        assert_eq!(adopt_legacy_recovery_waiting(&legacy, &dir, std::time::Duration::ZERO), dir);
+        assert!(dir.join("a.json").is_file() && !legacy.exists());
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
+    #[test]
+    fn no_old_folder_or_the_same_folder_changes_nothing() {
+        let root = temp("recovery-noop");
+        let dir = root.join("Recovery");
+        assert_eq!(adopt_legacy_recovery(&root.join("missing"), &dir), dir);
+        assert!(!dir.exists(), "nothing is created up front");
+        autosave(&dir, "a");
+        assert_eq!(adopt_legacy_recovery(&dir, &dir), dir);
+        assert!(dir.join("a.json").is_file());
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
     #[test]
     fn missing_exe_dir_or_env_never_panics() {
         let d = resolve(|_| None, None);

apps/photocraft/src/services.rs

@@ -264,8 +264,8 @@ fn block_on<T>(future: impl Future<Output = T>) -> T {
 }
 
 /// Per-user settings directory: `PHOTOCRAFT_CONFIG_DIR`, else `<exe dir>/PhotoCraftData` in
-/// portable mode, else the platform convention. Everything the app persists lives under it; see
-/// [`crate::app_dirs`].
+/// portable mode, else the platform convention. Everything the app persists lives under it
+/// except, on Windows, crash-recovery autosaves; see [`crate::app_dirs`].
 pub fn config_dir() -> Option<PathBuf> {
     crate::app_dirs::config_dir()
 }
@@ -280,8 +280,10 @@ pub fn presets_dir() -> Option<PathBuf> {
     config_dir().map(|d| d.join("Presets"))
 }
 
+/// Crash-recovery autosaves: under the settings directory, except on Windows, where they stay
+/// out of the roaming profile (see [`crate::app_dirs::recovery_dir`]).
 fn recovery_dir() -> Option<PathBuf> {
-    config_dir().map(|d| d.join("Recovery"))
+    crate::app_dirs::recovery_dir()
 }
 
 /// Write `bytes` crash-safely (temp file beside the target, fsync, rename, directory fsync; see

docs/control-protocol.md

@@ -246,7 +246,9 @@ log text file. GPU on/off and the GPU tile size apply at the next launch.
 The desktop app stores them in `preferences.json` in the platform config directory (macOS
 `~/Library/Application Support/Photocraft`, Windows `%APPDATA%\Photocraft`, Linux
 `$XDG_CONFIG_HOME/photocraft`; override with `PHOTOCRAFT_CONFIG_DIR`); autosaves go to its
-`Recovery` folder, and the window size and position, the dock width and floating-panel geometry
+`Recovery` folder (on Windows to `%LOCALAPPDATA%\Photocraft\Recovery`, out of the roaming
+profile; autosaves left by earlier versions move there at startup), and the window size and
+position, the dock width and floating-panel geometry
 to `ui.ron` beside it (a file holding values that would crash startup is discarded). In portable mode (a `portable.txt` or `PhotoCraft.portable` file beside the
 executable, as in the Windows portable zip) that directory is `PhotoCraftData` next to the
 executable instead. The web build keeps them in `localStorage`. A save writes only the values

packaging/windows/portable.txt

@@ -2,8 +2,8 @@ PhotoCraft portable mode
 
 While this file sits next to photocraft.exe, PhotoCraft keeps its preferences, brush presets,
 crash-recovery autosaves and startup state in the PhotoCraftData folder beside it, and writes
-nothing to %APPDATA%. Copy the whole folder to move PhotoCraft with its settings.
+nothing to %APPDATA% or %LOCALAPPDATA%. Copy the whole folder to move PhotoCraft with its settings.
 
-Delete this file to use the normal per-user settings folder (%APPDATA%\Photocraft) instead.
-If PhotoCraftData can't be written (for example on a read-only drive), PhotoCraft says so and
-uses %APPDATA%\Photocraft instead.
+Delete this file to use the normal per-user folders instead: %APPDATA%\Photocraft for settings
+and %LOCALAPPDATA%\Photocraft\Recovery for crash-recovery autosaves. If PhotoCraftData can't be
+written (for example on a read-only drive), PhotoCraft says so and uses those folders instead.