From 622dcc369c9ae859c0520e52cf92cca499e20cd9 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 19 Oct 2026 01:00:44 +0200 Subject: [PATCH 01/13] fix(backup): write archives through a partial file and prune only after a success An export or automatic backup wrote straight to the destination, which truncated it first. A manual export over an existing archive that failed halfway destroyed the copy the user had, and an automatic backup cut short left an archive-shaped file that retention then kept as the newest backup while deleting the oldest good one. write_archive now goes through waveflow_core::atomic_write: the archive is written to .partial, synced, and renamed over the target only once complete; any failure removes the partial and leaves the target as it was. Retention runs only for a profile whose archive was written, never counts an unfinished archive, and removes one left by a crash once it is a day old. The pre-reset safety backup drops its own .part dance, which the shared writer now does. --- docs/features/ui.md | 4 +- src-tauri/crates/app/src/backup.rs | 115 +++++++++- .../crates/app/src/commands/maintenance.rs | 25 +-- .../crates/app/src/commands/profile_io.rs | 30 ++- src-tauri/crates/core/src/atomic_write.rs | 212 ++++++++++++++++++ src-tauri/crates/core/src/lib.rs | 1 + 6 files changed, 353 insertions(+), 34 deletions(-) create mode 100644 src-tauri/crates/core/src/atomic_write.rs diff --git a/docs/features/ui.md b/docs/features/ui.md index 91196f8b3..10630dcb9 100644 --- a/docs/features/ui.md +++ b/docs/features/ui.md @@ -602,7 +602,7 @@ Per-profile isolated database (libraries, playlists, settings, play history); sh [`commands/profile_io.rs`](../../src-tauri/crates/app/src/commands/profile_io.rs) packages a profile into a single `.waveflow` (zip) file containing `manifest.json` + `data.db` + the per-profile `artwork/` directory. Settings → Storage and backups → Profile backups exposes both buttons. -- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. +- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to `.partial`, flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. - **Import:** always allocates a fresh profile row — never overwrites — under the archive's profile name, or `Name (2)`, `Name (3)`… when a profile already holds it (#767): `profile.name` is UNIQUE and restoring a backup of a profile that still exists is the ordinary case, so the name is taken by construction. It retries on the violation itself rather than checking first. The command returns the new id **and** that name, the success message shows the name, and Settings refreshes the profile list so the selector it points at actually lists the profile. It then extracts the archive under `profiles//`. Failures roll the row back so a half-imported profile doesn't survive the error. Before the sqlx migrator runs, [`normalise_migration_checksums`](../../src-tauri/crates/app/src/commands/profile_io.rs) rewrites `_sqlx_migrations.checksum` for every version present in both the archive and the local migrator — older builds checked out migration files with CRLF endings (Windows `core.autocrlf=true` + no `.gitattributes` lock) so their stored SHA-384 differs from the same SQL re-hashed today, even though the DDL is identical. A `.gitattributes` at repo root now pins `*.sql` / `*.rs` / `*.ts` / etc. to LF so future archives stay byte-stable. Once normalised, the new pool is opened once so any pending sqlx migrations replay before the user switches to it. An archive whose `_sqlx_migrations` lists a version unknown to the local migrator is rejected — that means the export came from a newer build. - **Feedback and failures:** an archive carries the artwork cache, so a large library takes minutes; the card shows an "Exporting…" / "Importing…" line while the buttons are greyed, since greyed buttons alone read as a hang. Both commands log their error at `warn` (`profile export failed` / `profile import failed`) — the UI can only show one generic sentence, and until #767 the cause was written nowhere. - **Out of scope:** the shared `app.db` (Last.fm key, Discord opt-in, `network.offline_mode`) belongs to the install, not the profile. @@ -615,7 +615,7 @@ Opt-in scheduled mirror of the manual export so the user's playlists / likes / r - **Config** lives in `app_setting` (install-wide, not per-profile): `backup.enabled` (bool, default OFF), `backup.interval_days` (1-90, default 7), `backup.folder` (string; empty = default `/waveflow/backups/`), `backup.retention` (1-50, default 5 — per profile), `backup.last_run_at` (epoch ms). - **Loop** is a single tokio task started once at boot ([`spawn_backup_loop`](../../src-tauri/crates/app/src/backup.rs)). When disabled, parks on a `tokio::sync::Notify` (zero cost) until the user toggles. When enabled, computes the next deadline as `last_run_at + interval_days * 86_400_000` and uses `tokio::select!` between a sleep and the same `Notify` so config changes wake it without waiting for the old sleep to expire. -- **Pass** ([`run_one_backup`](../../src-tauri/crates/app/src/backup.rs)) iterates every row in `profile`, calls the shared [`profile_io::write_archive`](../../src-tauri/crates/app/src/commands/profile_io.rs) (pub-crate-ified from the manual-export path so the two stay bit-compatible), and applies retention per profile (`-*.waveflow` sorted by mtime, oldest beyond `retention` deleted). The active profile gets a `PRAGMA wal_checkpoint(TRUNCATE)` first; inactive profiles are already cold on disk (the pool ran a checkpoint at switch / shutdown). +- **Pass** ([`run_one_backup`](../../src-tauri/crates/app/src/backup.rs)) iterates every row in `profile`, calls the shared [`profile_io::write_archive`](../../src-tauri/crates/app/src/commands/profile_io.rs) (pub-crate-ified from the manual-export path so the two stay bit-compatible), and applies retention per profile (`-*.waveflow` sorted by mtime, oldest beyond `retention` deleted) — **only after that profile's archive was written**. Retention is "keep the newest N", and pruning after a failed write deleted the oldest good backup on every failed run. An unfinished `*.waveflow.partial` is never counted as a backup; one left by a crash is removed once it is a day old, an age no write in progress reaches since every write touches it. The active profile gets a `PRAGMA wal_checkpoint(TRUNCATE)` first; inactive profiles are already cold on disk (the pool ran a checkpoint at switch / shutdown). - **Failure isolation:** per-profile errors are logged but don't abort the pass — one corrupt profile shouldn't block backups of the healthy ones. - **Commands** in [`commands/backup.rs`](../../src-tauri/crates/app/src/commands/backup.rs): `get_backup_config`, `set_backup_config` (also signals the loop), `run_backup_now`. UI is [`BackupCard`](../../src/components/views/settings/BackupCard.tsx) in Settings → Storage and backups → Profile backups right after the manual export/import. diff --git a/src-tauri/crates/app/src/backup.rs b/src-tauri/crates/app/src/backup.rs index e1f07f449..9ce6d66a2 100644 --- a/src-tauri/crates/app/src/backup.rs +++ b/src-tauri/crates/app/src/backup.rs @@ -424,7 +424,7 @@ pub async fn run_one_backup( .await .map_err(|e| AppError::Other(format!("backup task join: {e}")))?; - match result { + let written = match result { Ok(()) => { tracing::info!(profile_id, target = %target.display(), "auto backup ok"); created.push(target.to_string_lossy().to_string()); @@ -435,18 +435,28 @@ pub async fn run_one_backup( if attempted_with_cache { bundle_metadata_artwork = false; } + true } Err(err) => { tracing::warn!(profile_id, ?err, "auto backup failed"); failed += 1; + false } - } + }; // Prune old archives for this profile. List `-*.waveflow` // in the folder, sort by mtime descending, keep the first // `retention` entries. - if let Err(err) = prune_old_backups(&folder, &safe, config.retention as usize).await { - tracing::warn!(?err, "auto backup retention sweep failed"); + // + // Only after an archive was written. Retention is "keep the + // newest N", and a pass that added nothing has no new archive + // to make room for: pruning anyway deleted the oldest good + // backup on every failed run, so a profile that kept failing + // was slowly left with none. + if written { + if let Err(err) = prune_old_backups(&folder, &safe, config.retention as usize).await { + tracing::warn!(?err, "auto backup retention sweep failed"); + } } done += 1; @@ -475,6 +485,37 @@ pub async fn run_one_backup( }) } +/// How old an unfinished archive has to be before retention removes it. +/// Far longer than any backup takes to write, so it is never one in +/// progress — another pass can be running beside this one. +const STALE_PARTIAL: Duration = Duration::from_secs(24 * 60 * 60); + +/// What a file in the backup folder is to the retention sweep of one +/// profile. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BackupFile { + /// A finished archive of this profile: counted, oldest pruned. + Archive, + /// An archive of this profile still being written, or one whose + /// write never finished. Never counted as a backup. + Partial, + /// Anything else, left alone. + Other, +} + +fn classify_backup_file(name: &str, prefix: &str) -> BackupFile { + if !name.starts_with(prefix) { + return BackupFile::Other; + } + if name.ends_with(".waveflow") { + return BackupFile::Archive; + } + match name.strip_suffix(waveflow_core::atomic_write::PARTIAL_SUFFIX) { + Some(archive) if archive.ends_with(".waveflow") => BackupFile::Partial, + _ => BackupFile::Other, + } +} + async fn prune_old_backups( folder: &std::path::Path, name_prefix: &str, @@ -494,20 +535,35 @@ async fn prune_old_backups( // nothing to prune. Not an error from the user's perspective. Err(_) => return Ok(0), }; + let now = std::time::SystemTime::now(); for entry in read_dir.flatten() { let path = entry.path(); - let Some(stem) = path - .file_name() - .and_then(|s| s.to_str()) - .filter(|s| s.starts_with(&prefix_owned) && s.ends_with(".waveflow")) - else { + let Some(name) = path.file_name().and_then(|s| s.to_str()) else { continue; }; - let _ = stem; + let kind = classify_backup_file(name, &prefix_owned); + if kind == BackupFile::Other { + continue; + } let mtime = entry .metadata() .and_then(|m| m.modified()) .unwrap_or(std::time::SystemTime::UNIX_EPOCH); + if kind == BackupFile::Partial { + // Never counted as a backup: it is an archive that did + // not finish. One whose writer is gone (a crash, a + // power cut) would otherwise sit in the folder for good, + // at the size of a whole archive. A partial still being + // written is touched by every write, so it never looks + // this old. + let stale = now + .duration_since(mtime) + .is_ok_and(|age| age > STALE_PARTIAL); + if stale && std::fs::remove_file(&path).is_ok() { + tracing::debug!(path = %path.display(), "removed an unfinished backup"); + } + continue; + } entries.push((path, mtime)); } entries.sort_by_key(|e| std::cmp::Reverse(e.1)); // newest first @@ -662,3 +718,42 @@ pub fn spawn_backup_loop(handle: AppHandle, backup_handle: BackupHandle) { } }); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_finished_archive_of_the_profile_is_counted() { + assert_eq!( + classify_backup_file("Main-2026-10-19T10-00-00.waveflow", "Main-"), + BackupFile::Archive + ); + } + + /// An unfinished archive must never count toward retention: kept as + /// the newest, it pushed the oldest good backup out. + #[test] + fn an_unfinished_archive_is_not_a_backup() { + assert_eq!( + classify_backup_file("Main-2026-10-19T10-00-00.waveflow.partial", "Main-"), + BackupFile::Partial + ); + } + + #[test] + fn other_files_are_left_alone() { + assert_eq!( + classify_backup_file("Other-2026-10-19T10-00-00.waveflow", "Main-"), + BackupFile::Other + ); + assert_eq!( + classify_backup_file("Main-notes.txt.partial", "Main-"), + BackupFile::Other + ); + assert_eq!( + classify_backup_file("Main-notes.txt", "Main-"), + BackupFile::Other + ); + } +} diff --git a/src-tauri/crates/app/src/commands/maintenance.rs b/src-tauri/crates/app/src/commands/maintenance.rs index dfdb9def5..2f8c80f2f 100644 --- a/src-tauri/crates/app/src/commands/maintenance.rs +++ b/src-tauri/crates/app/src/commands/maintenance.rs @@ -474,7 +474,6 @@ async fn safety_backup_all_profiles(state: &AppState) -> Option { "pre-reset-{}-{profile_id}-{stamp}.waveflow", sanitize_filename(&profile_name) )); - let tmp_target = final_target.with_extension("part"); let snapshot_db = final_target.with_extension("dbtmp"); // 1. Snapshot the db with `VACUUM INTO` instead of raw-copying the @@ -511,16 +510,16 @@ async fn safety_backup_all_profiles(state: &AppState) -> Option { exported_at: Utc::now().to_rfc3339(), }; - // 2. Archive the SNAPSHOT (not the live db) + artwork into a temp - // `.part` sibling, renamed to the final name only on success so - // a failed/interrupted write never leaves a truncated file that - // looks like a valid backup (same dir → atomic rename). + // 2. Archive the SNAPSHOT (not the live db) + artwork. + // `write_archive` goes through a `.partial` sibling renamed to + // the final name only on success, so a failed/interrupted write + // never leaves a truncated file that looks like a valid backup. let write_res = tokio::task::spawn_blocking({ - let tmp_target = tmp_target.clone(); + let final_target = final_target.clone(); let snapshot_db = snapshot_db.clone(); move || { write_archive( - &tmp_target, + &final_target, &profile_dir, &snapshot_db, &artwork_dir, @@ -535,22 +534,13 @@ async fn safety_backup_all_profiles(state: &AppState) -> Option { let _ = std::fs::remove_file(&snapshot_db); match write_res { - Ok(Ok(())) => { - match std::fs::rename(&tmp_target, &final_target) { - Ok(()) => written += 1, - Err(err) => { - tracing::warn!(?err, profile_id, "pre-reset safety backup: could not finalize archive; discarding partial"); - let _ = std::fs::remove_file(&tmp_target); - } - } - } + Ok(Ok(())) => written += 1, Ok(Err(err)) => { tracing::warn!( ?err, profile_id, "pre-reset safety backup: archive failed for profile" ); - let _ = std::fs::remove_file(&tmp_target); } Err(err) => { tracing::warn!( @@ -558,7 +548,6 @@ async fn safety_backup_all_profiles(state: &AppState) -> Option { profile_id, "pre-reset safety backup: task join failed for profile" ); - let _ = std::fs::remove_file(&tmp_target); } } } diff --git a/src-tauri/crates/app/src/commands/profile_io.rs b/src-tauri/crates/app/src/commands/profile_io.rs index 984ba15e6..ffd5f7a67 100644 --- a/src-tauri/crates/app/src/commands/profile_io.rs +++ b/src-tauri/crates/app/src/commands/profile_io.rs @@ -353,6 +353,12 @@ async fn cleanup_partial_profile(state: &AppState, profile_id: i64) { // is bundled under `metadata_artwork/**` so a restore on another machine // doesn't have to re-fetch every cover. When `None`, the archive stays // lean and the cache will be rebuilt lazily after restore. +// +// Written to `.partial` and renamed over `target` only once the +// archive is complete. Writing in place truncated the destination first, +// so an export over an existing archive that failed halfway destroyed +// the copy the user already had, and an automatic backup cut short left +// an archive-shaped file that retention then kept as the newest backup. pub(crate) fn write_archive( target: &Path, profile_dir: &Path, @@ -361,10 +367,26 @@ pub(crate) fn write_archive( metadata_artwork_dir: Option<&Path>, manifest: &ArchiveManifest, ) -> AppResult<()> { - if let Some(parent) = target.parent() { - std::fs::create_dir_all(parent)?; - } - let file = File::create(target)?; + waveflow_core::atomic_write::write_with(target, |file| { + write_archive_into( + file, + profile_dir, + db_path, + artwork_dir, + metadata_artwork_dir, + manifest, + ) + }) +} + +fn write_archive_into( + file: &mut File, + profile_dir: &Path, + db_path: &Path, + artwork_dir: &Path, + metadata_artwork_dir: Option<&Path>, + manifest: &ArchiveManifest, +) -> AppResult<()> { let mut zip = ZipWriter::new(file); let opts: SimpleFileOptions = SimpleFileOptions::default() diff --git a/src-tauri/crates/core/src/atomic_write.rs b/src-tauri/crates/core/src/atomic_write.rs new file mode 100644 index 000000000..aac51dd71 --- /dev/null +++ b/src-tauri/crates/core/src/atomic_write.rs @@ -0,0 +1,212 @@ +//! Replace a file only once its new content is complete. +//! +//! For files WaveFlow writes whole — a profile archive, a playlist +//! export — where the previous copy is worth more than a half-written +//! new one. Writing straight to the destination truncates it first, so +//! a full disk, a share that drops or a crash halfway leaves the user +//! with neither: an export over last week's archive destroys last +//! week's archive, and an automatic backup cut short sits in the backup +//! folder looking like the newest good one. +//! +//! Here the bytes go to `.partial` beside the destination, reach +//! the disk, and only then replace it in a single `rename`. Any failure +//! removes the partial and leaves the destination as it was. +//! +//! Not for audio files: those go through [`crate::tagio`], which has to +//! keep the original's inode where it can and copies it before writing. + +use std::fs::File; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; + +/// Appended to the destination's file name while it is being written. +/// Anything that lists a folder for finished files (the backup +/// retention, for one) must not take a name ending in it as one. +pub const PARTIAL_SUFFIX: &str = ".partial"; + +/// `.partial`, in the destination's directory. +/// +/// Beside it, never in the system temp directory: `rename` is only +/// atomic within a filesystem, and an export folder is routinely on a +/// different one than `/tmp`. +pub fn partial_path(target: &Path) -> PathBuf { + let mut name = target + .file_name() + .map(|n| n.to_os_string()) + .unwrap_or_default(); + name.push(PARTIAL_SUFFIX); + target.with_file_name(name) +} + +/// Removes the partial unless it was renamed into place, so every early +/// return between creating it and committing it cleans up after itself. +struct Partial { + path: PathBuf, + committed: bool, +} + +impl Drop for Partial { + fn drop(&mut self) { + if !self.committed { + let _ = std::fs::remove_file(&self.path); + } + } +} + +/// Write `target` through a sibling partial file. +/// +/// `body` writes the complete new content into the handle it is given +/// (opened for reading and writing, empty, positioned at the start). The +/// content is flushed to the disk, then renamed over `target`. When +/// `body` or any step after it fails, `target` is untouched — still +/// absent, or still holding what it held before — and the partial is +/// gone. +/// +/// The parent directory is created if missing. Blocking: call it from a +/// blocking thread, never an async worker. +pub fn write_with(target: &Path, body: impl FnOnce(&mut File) -> Result) -> Result +where + E: From, +{ + let directory = target + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + std::fs::create_dir_all(directory).map_err(E::from)?; + + let mut partial = Partial { + path: partial_path(target), + committed: false, + }; + // `create` rather than `create_new`: a partial already there is the + // leftover of a write that crashed before it could clean up, and the + // name is ours, so it is simply started over. + let value = { + let mut file = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(true) + .open(&partial.path) + .map_err(E::from)?; + let value = body(&mut file)?; + file.flush().map_err(E::from)?; + // Before the rename, not after: once renamed, the file is the + // one the user will restore from, and a rename that survives a + // power cut over content that did not would be the very archive + // this exists to prevent. + file.sync_all().map_err(E::from)?; + value + // The handle closes here: Windows refuses to rename a file that + // is still open. + }; + + std::fs::rename(&partial.path, target).map_err(E::from)?; + partial.committed = true; + // Durability on top of a replacement that has already happened, so + // its failure is not reported as a failed write. + let _ = sync_directory(directory); + Ok(value) +} + +/// [`write_with`] for content already in memory. +pub fn write_bytes(target: &Path, bytes: &[u8]) -> io::Result<()> { + write_with(target, |file| file.write_all(bytes)) +} + +/// Make the rename durable on Unix, where it is a separate write to the +/// directory that a crash can lose. Windows has no directory handle to +/// sync, and opening one fails there, so this is Unix-only. +fn sync_directory(directory: &Path) -> io::Result<()> { + #[cfg(unix)] + { + File::open(directory)?.sync_all() + } + #[cfg(not(unix))] + { + let _ = directory; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_partial_sits_beside_the_target() { + let target = Path::new("/backups/Main-2026.waveflow"); + assert_eq!( + partial_path(target), + Path::new("/backups/Main-2026.waveflow.partial") + ); + } + + #[test] + fn a_complete_write_replaces_the_target_and_leaves_nothing_behind() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("list.m3u8"); + std::fs::write(&target, b"old").unwrap(); + + write_bytes(&target, b"new").unwrap(); + + assert_eq!(std::fs::read(&target).unwrap(), b"new"); + assert!(!partial_path(&target).exists()); + } + + /// The case this exists for: a write that fails halfway must not + /// have truncated the copy the user already had. + #[test] + fn a_failed_write_keeps_the_previous_content() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("archive.waveflow"); + std::fs::write(&target, b"last week's archive").unwrap(); + + let result: io::Result<()> = write_with(&target, |file| { + file.write_all(b"half of a new one")?; + Err(io::Error::other("disk full")) + }); + + assert!(result.is_err()); + assert_eq!(std::fs::read(&target).unwrap(), b"last week's archive"); + assert!(!partial_path(&target).exists()); + } + + /// And with nothing there before, a failure leaves nothing at all + /// -- not an archive-shaped file a listing would take for a backup. + #[test] + fn a_failed_first_write_leaves_no_target() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("archive.waveflow"); + + let result: io::Result<()> = write_with(&target, |file| { + file.write_all(b"partial")?; + Err(io::Error::other("cancelled")) + }); + + assert!(result.is_err()); + assert!(!target.exists()); + assert!(!partial_path(&target).exists()); + } + + #[test] + fn a_leftover_partial_is_started_over() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("list.m3u8"); + std::fs::write(partial_path(&target), b"crashed write, much longer").unwrap(); + + write_bytes(&target, b"fresh").unwrap(); + + assert_eq!(std::fs::read(&target).unwrap(), b"fresh"); + } + + #[test] + fn a_missing_parent_directory_is_created() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("new").join("folder").join("list.m3u8"); + + write_bytes(&target, b"x").unwrap(); + + assert_eq!(std::fs::read(&target).unwrap(), b"x"); + } +} diff --git a/src-tauri/crates/core/src/lib.rs b/src-tauri/crates/core/src/lib.rs index 5528f8d05..63068062f 100644 --- a/src-tauri/crates/core/src/lib.rs +++ b/src-tauri/crates/core/src/lib.rs @@ -13,6 +13,7 @@ pub mod album_playback; pub mod analysis; pub mod artwork; +pub mod atomic_write; pub mod audio_format; pub mod domain; pub mod error; From 401eeec6213a401812e9f43bfc1840bfd019f31a Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 19 Oct 2026 01:02:23 +0200 Subject: [PATCH 02/13] fix(playlist): write the m3u export through a partial file export_playlist_m3u wrote the destination in place, so overwriting an existing playlist file that failed halfway left it truncated. It now goes through the same partial-then-rename writer as the profile archives, on the blocking pool rather than an async worker, since the destination can be a network share. --- docs/features/playlists.md | 2 +- src-tauri/crates/app/src/commands/playlist.rs | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/features/playlists.md b/docs/features/playlists.md index cbf2801ec..bb3fce676 100644 --- a/docs/features/playlists.md +++ b/docs/features/playlists.md @@ -31,7 +31,7 @@ Drag-and-drop is implemented with `@dnd-kit` over a virtualised list (`@tanstack ## M3U import / export -`export_playlist_m3u` writes a UTF-8 `.m3u8` with `#EXTINF:, - ` headers and absolute file paths. The importer accepts both `.m3u` and `.m3u8` (foobar2000, VLC, Rekordbox, hand-written sets) and matches against the active library: +`export_playlist_m3u` writes a UTF-8 `.m3u8` with `#EXTINF:<duration>,<artist> - <title>` headers and absolute file paths, through a `.partial` sibling renamed over the destination once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so overwriting an existing playlist file that fails halfway leaves the old one intact rather than truncated. The importer accepts both `.m3u` and `.m3u8` (foobar2000, VLC, Rekordbox, hand-written sets) and matches against the active library: 1. **Canonical-path match** — strip Windows `\\?\` prefix, lowercase the drive letter, normalise separators. 2. **Basename fallback** — if path-match fails, try the basename. This recovers playlists exported before a library reorganisation. diff --git a/src-tauri/crates/app/src/commands/playlist.rs b/src-tauri/crates/app/src/commands/playlist.rs index 9b9fbc4e6..fd115acfc 100644 --- a/src-tauri/crates/app/src/commands/playlist.rs +++ b/src-tauri/crates/app/src/commands/playlist.rs @@ -825,14 +825,21 @@ pub async fn export_playlist_m3u( out.push('\n'); } + // Through a partial file renamed into place, so overwriting an + // existing playlist file that fails halfway (a full disk, a share + // that drops) leaves the old one intact rather than truncated. On + // the blocking pool: the destination is wherever the user picked, + // network shares included, and the parent may need creating. let dest = std::path::PathBuf::from(&dest_path); - if let Some(parent) = dest.parent() { - if !parent.as_os_str().is_empty() { - std::fs::create_dir_all(parent) - .map_err(|e| AppError::Other(format!("create parent dir: {e}")))?; - } + { + let dest = dest.clone(); + tokio::task::spawn_blocking(move || { + waveflow_core::atomic_write::write_bytes(&dest, out.as_bytes()) + }) + .await + .map_err(|e| AppError::Other(format!("write m3u join: {e}")))? + .map_err(|e| AppError::Other(format!("write m3u file: {e}")))?; } - std::fs::write(&dest, out).map_err(|e| AppError::Other(format!("write m3u file: {e}")))?; tracing::info!( playlist_id, From c91ebe8e565b50cafed84a4c7994238d7e26c52a Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 01:14:11 +0200 Subject: [PATCH 03/13] fix(watcher): scan on lost-events signals and re-arm folders missing at launch The event filter kept only create, modify and remove, so the backend's own "events were lost, rescan" signal (an inotify queue overflow, an FSEvents must-scan: EventKind::Other with the rescan flag) was dropped, and the large copy that produces it was never picked up. It now asks for the same debounced scan of the folder as any other change. Errors from the backend were dropped silently; they are now logged, at most once a minute per folder with a count of the ones in between. A watched folder that could not be armed at launch (a drive not plugged in yet, a share that mounts late) was logged once and left unwatched until restart. restore_from_db and the toggle now go through watch_or_wait, which keeps a placeholder in the watcher map and retries every 60 s, then scans the folder once it is back. The placeholder is dropped like a watcher, so removing the folder, turning watching off or switching profile ends the retries, and a retry that lost that race drops the watcher it built instead of installing it. --- docs/features/library.md | 2 +- src-tauri/crates/app/src/watcher.rs | 358 ++++++++++++++++++++++------ 2 files changed, 286 insertions(+), 74 deletions(-) diff --git a/docs/features/library.md b/docs/features/library.md index eddd74eb1..640d0f453 100644 --- a/docs/features/library.md +++ b/docs/features/library.md @@ -10,7 +10,7 @@ The library is a per-profile SQLite database (`<root>/profiles/<id>/data.db`) ke - **Several genres per track** (#729) — every genre item is read (`get_strings(ItemKey::Genre)`, never `genre()`, which is the first one): lofty already turns a multi-value ID3v2.4 `TCON`, repeated Vorbis `GENRE` comments and several MP4 `©gen` into one item each. [`split_genres`](../../src-tauri/crates/core/src/scanner/canonical.rs) then splits each value on `;`, NUL and `\` — taggers that write one field join genres with `;`, and some write the `\\` a tag editor shows between multiple values into the file as text ("Alternative\\Rock") — trims them, and drops a name the library would file under one already listed (same `canonical_name`), keeping the tag's order. **Not on `/` or `,`**: they occur inside genre names people use ("Hip-Hop/Rap", "Folk, World, & Country"). Each genre is one `track_genre` row. A library scanned before #729 holds one genre per track — the first value, or a "Rock; Jazz" kept whole — and the fast path never reopens a file whose mtime and size still match, so each folder gets **one forced re-read**, marked `profile_setting['scan.genre_backfill_done.<folder>']` like the custom-tag pass: the unchanged-file branch compares the file's genres with the stored ones and relinks them when they differ. A stopped scan does not set the marker. A file that fails extraction holds it back, since a lock is usually transient, but for three scans at most (`scan.genre_backfill_attempts.<folder>`, as the custom-tag pass counts), so a permanently unreadable file does not re-read the folder forever. The genre row such a scan leaves without tracks stays in `genre` (a smart-playlist rule may name it) and drops out of every listing, which only ever counts genres with an available track. - **Folder cover fallback** — when a track carries no embedded picture, the scanner inspects its parent directory for a sidecar image with one of the canonical stems (`cover`, `folder`, `front`, `albumart`, `album`, `artwork`) and an extension the thumbnail pipeline can decode (`jpg`/`jpeg`, `png`, `webp`, `bmp`, `gif`, `tiff`). The first match — by stem priority, not alphabetical — is hash-addressed into the shared `artwork/` dir like an embedded picture. The provenance is tagged `source = 'folder'` in the `artwork` table so a future cleanup job can distinguish it from `'embedded'`, `'deezer'`, or `'user'` entries. Covers common CD-rip / lossless layouts where the artwork sits beside the audio files. - **Audio quality** — sample rate, bitrate, channel count, bit depth and codec are captured at scan time. Hi-Res badges (≥ 24-bit, ≥ 44.1 kHz) light up automatically on covers and rows. -- **Watch folders** — [`notify 8`](https://crates.io/crates/notify) drives a per-folder filesystem watcher with debounced rescans so files dropped into a watched directory appear without a manual refresh. Deletions flag rows `is_available = 0` rather than purging them, so play history, ratings and playlist memberships survive a reorganisation. +- **Watch folders** — [`notify 8`](https://crates.io/crates/notify) drives a per-folder filesystem watcher with debounced rescans so files dropped into a watched directory appear without a manual refresh. Deletions flag rows `is_available = 0` rather than purging them, so play history, ratings and playlist memberships survive a reorganisation. Three things the [watcher](../../src-tauri/crates/app/src/watcher.rs) does not let fall through: the backend's own "events were lost" signal (an inotify queue overflow, an FSEvents must-scan — `EventKind::Other` with the rescan flag, usually naming no path) asks for the same debounced scan of the folder as any change, since it is exactly what a large copy produces; errors the backend reports are logged, at most once a minute per folder with a count of the ones in between; and a watched folder that cannot be armed — a drive not plugged in at launch, a share that mounts late — is retried every 60 s, then scanned once when it is back, instead of staying unwatched until the next restart. The retry is a placeholder in the same map as the watchers, so removing the folder, turning watching off or switching profile stops it the same way. - **The reserved directory** — `.waveflow/` inside a library folder is WaveFlow's own, and both walkers leave it alone ([`scanner::reserved`](../../src-tauri/crates/core/src/scanner/reserved.rs), issue #695). It has to be said explicitly, for three reasons that each had to be checked rather than assumed: `mp4` is in `AUDIO_EXTENSIONS`, legitimately, since an `.mp4` usually holds AAC; `is_scannable_audio` takes it **on sight**, because only `ogg` / `oga` pay for a header read, so a Canvas clip with no audio stream at all would pass; and the walk had no directory filter whatsoever, so the leading dot would have bought nothing — that is a display convention, not something a file walker obeys. `collect_scannable_files` prunes the branch as `filter_entry` meets it, so the whole subtree is skipped rather than each file in it refused, and the fs watcher drops an event whose **every** path is inside it, so caching a clip does not cost a folder rescan (an event touching one of ours and one of theirs is a library change). The check is rooted, not run over every component of an absolute path: somebody whose music lives under a directory of that name would otherwise have an empty library and nothing to explain it — and for the same reason the walk never prunes its own root. - **Live progress** — `scan_folder_inner` emits a `scan:progress` event (throttled to every 25 files, plus one initial and one final tick) carrying `current` / `total`, the running `added` / `updated` / `skipped` / `errors` tallies, `cancelled`, and `current_dir` — the parent directory of the file just processed, `None` on the initial and final ticks. The moving bar itself belongs to the [task status bar](ui.md#long-running-tasks) since #601, which lists every long operation in one place; [`ScanProgressToast`](../../src/components/common/ScanProgressToast.tsx) (mounted once in `AppLayout`) shows the **outcome**, which is the part a status bar cannot — its row vanishes when the task ends, and “412 added, 3 errors” is what is worth reading. The toast ignores non-terminal events, so a rescan starting underneath cannot blank a summary the user has not finished reading. A scan the user stopped reports as stopped rather than as a completed walk, and `current` then carries the files actually reached rather than the folder total (issue #430, #601). **Listing the folder reports too**: on a network share the walk alone can take minutes, and the task row used to sit at zero with no folder named and a stop button nobody looked at until the list was complete. `collect_scannable_files_with` now hands the row the count of audio files found so far (no total yet — the status bar shows the count alone) and the directory it is in, and checks the stop flag every 32 entries; a stop there returns no list at all, since the scan treats the list as the folder's contents and would sweep every track it had not reached yet. diff --git a/src-tauri/crates/app/src/watcher.rs b/src-tauri/crates/app/src/watcher.rs index e8ebc6d2d..6f087f306 100644 --- a/src-tauri/crates/app/src/watcher.rs +++ b/src-tauri/crates/app/src/watcher.rs @@ -18,11 +18,17 @@ //! shutdown channel signals the debounce task to exit. //! - Profile switches must call [`WatcherManager::unwatch_all`] then //! [`WatcherManager::restore_from_db`] with the new pool. +//! - A folder that cannot be armed — usually a drive that is not plugged +//! in yet — is not given up on: [`WatcherManager::watch_or_wait`] +//! keeps a placeholder for it and tries again every +//! [`REARM_INTERVAL`], then scans it once it is back. The placeholder +//! is dropped exactly like a watcher, so unwatching or switching +//! profile stops the retries too. use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::time::{Duration, Instant}; use notify::{Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; use serde::Serialize; @@ -40,6 +46,17 @@ use crate::state::AppState; /// events as the OS finalizes each file) yields a single scan. const DEBOUNCE: Duration = Duration::from_millis(1500); +/// How often a watched folder that could not be armed is tried again. +/// A drive plugged in after launch is picked up within this long; +/// shorter would only spend more stat calls on a drive that is absent +/// for days. +const REARM_INTERVAL: Duration = Duration::from_secs(60); + +/// Shortest gap between two logged errors from one folder's watcher. An +/// error can repeat for every event the backend fails to deliver, and a +/// log flooded with the same line hides everything else in it. +const ERROR_LOG_INTERVAL: Duration = Duration::from_secs(60); + /// Tauri event fired after a watcher-driven rescan completes. The /// frontend should refetch `list_libraries` (which feeds the /// "library.updated_at" reactivity used by the views). @@ -55,13 +72,15 @@ struct LibraryRescannedPayload { } /// Per-folder state held inside [`WatcherManager`]. Dropping this -/// struct stops both the OS-level watcher and the debounce task. +/// struct stops both the OS-level watcher and the debounce task — or, +/// for a folder still waiting to be armed, its retry task. struct FolderHandle { /// Held only so the underlying notify thread stays alive — never - /// read after construction. - _watcher: RecommendedWatcher, - /// Signals the debounce task to exit. Sender is dropped on - /// removal; the task's `recv()` returns `Err(_)` and bails. + /// read after construction. `None` while the folder is waiting to + /// be armed (see [`WatcherManager::watch_or_wait`]). + _watcher: Option<RecommendedWatcher>, + /// Signals the debounce task (or the retry task) to exit. Sender is + /// dropped on removal; the task's receiver resolves and it bails. _shutdown: oneshot::Sender<()>, } @@ -83,57 +102,7 @@ impl WatcherManager { /// Start watching `path` for `folder_id`. If a watcher already /// existed for this id (e.g. after a path edit) it's dropped first. pub fn watch(&self, folder_id: i64, library_id: i64, path: PathBuf) -> AppResult<()> { - if !path.is_dir() { - return Err(AppError::Other(format!( - "watch path is not a directory: {}", - path.display() - ))); - } - - // Bridge the sync notify callback into an async tokio channel. - // Bounded would risk dropping events under load — unbounded is - // fine because the debounce stage collapses bursts to a single - // scan anyway. - let (event_tx, event_rx) = mpsc::unbounded_channel::<()>(); - let event_tx_for_cb = event_tx.clone(); - let watcher_path = path.clone(); - let reserved_root = path.clone(); - - let mut watcher = notify::recommended_watcher(move |res: notify::Result<Event>| { - let Ok(event) = res else { return }; - if !is_relevant_event(&event) { - return; - } - if is_our_own_media(&reserved_root, &event) { - return; - } - // Best-effort send — the debounce task may be tearing down. - let _ = event_tx_for_cb.send(()); - }) - .map_err(|e| AppError::Other(format!("notify watcher init: {e}")))?; - - watcher - .watch(&watcher_path, RecursiveMode::Recursive) - .map_err(|e| { - AppError::Other(format!("notify watch {}: {e}", watcher_path.display())) - })?; - - let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); - let app = self.app.clone(); - tauri::async_runtime::spawn(debounce_loop( - event_rx, - shutdown_rx, - app, - folder_id, - library_id, - event_tx, - )); - - let handle = FolderHandle { - _watcher: watcher, - _shutdown: shutdown_tx, - }; - + let (handle, _scan) = arm(&self.app, folder_id, library_id, &path)?; let mut map = self.inner.lock().expect("watcher map poisoned"); // Replacing drops the previous handle, which stops its watcher // and signals its debounce task — exactly what we want when @@ -143,6 +112,46 @@ impl WatcherManager { Ok(()) } + /// [`Self::watch`], and when the folder cannot be armed, keep trying. + /// + /// For the callers that act on the user's standing choice rather + /// than on a click: the folders restored at launch and the toggle. + /// The usual reason a watched folder cannot be armed is that it is + /// not there yet — an external drive plugged in after launch, a + /// share that mounts late — and giving up on it then meant it went + /// unwatched until the next restart, with nothing to say so. + pub fn watch_or_wait(&self, folder_id: i64, library_id: i64, path: PathBuf) { + let Err(err) = self.watch(folder_id, library_id, path.clone()) else { + return; + }; + tracing::warn!( + folder_id, + path = %path.display(), + %err, + "folder watcher not armed; trying again every {}s", + REARM_INTERVAL.as_secs() + ); + let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); + // In the map like an armed watcher, so `unwatch`, `unwatch_all` + // and a later `watch` of the same folder all end the retries the + // same way they stop a watcher: by dropping this handle. + self.inner.lock().expect("watcher map poisoned").insert( + folder_id, + FolderHandle { + _watcher: None, + _shutdown: shutdown_tx, + }, + ); + tauri::async_runtime::spawn(rearm_loop( + self.app.clone(), + Arc::clone(&self.inner), + shutdown_rx, + folder_id, + library_id, + path, + )); + } + /// Stop watching `folder_id`. No-op when the folder isn't watched. pub fn unwatch(&self, folder_id: i64) { let mut map = self.inner.lock().expect("watcher map poisoned"); @@ -162,9 +171,10 @@ impl WatcherManager { } /// Boot-time hydration: query every `is_watched=1` folder in the - /// active profile and arm a watcher for each one. Failures are - /// logged per-folder so a missing path on one folder doesn't - /// disable watching everywhere. + /// active profile and arm a watcher for each one. A folder that + /// cannot be armed is logged and retried on its own (see + /// [`Self::watch_or_wait`]), so a missing path on one folder neither + /// disables watching everywhere nor stays unwatched for the session. pub async fn restore_from_db(&self, pool: &SqlitePool) -> AppResult<()> { let rows: Vec<(i64, i64, String)> = sqlx::query_as("SELECT id, library_id, path FROM library_folder WHERE is_watched = 1") @@ -172,22 +182,189 @@ impl WatcherManager { .await?; for (folder_id, library_id, path) in rows { - if let Err(err) = self.watch(folder_id, library_id, PathBuf::from(&path)) { - tracing::warn!(folder_id, path, %err, "failed to restore watcher"); - } + self.watch_or_wait(folder_id, library_id, PathBuf::from(&path)); } Ok(()) } } +/// Build the OS watcher for `path` and the debounce task behind it. +/// +/// Returns the handle to keep in the map, and a sender that asks the +/// debounce task for a scan exactly as an event would. +fn arm( + app: &AppHandle, + folder_id: i64, + library_id: i64, + path: &Path, +) -> AppResult<(FolderHandle, UnboundedSender<()>)> { + if !path.is_dir() { + return Err(AppError::Other(format!( + "watch path is not a directory: {}", + path.display() + ))); + } + + // Bridge the sync notify callback into an async tokio channel. + // Bounded would risk dropping events under load — unbounded is + // fine because the debounce stage collapses bursts to a single + // scan anyway. + let (event_tx, event_rx) = mpsc::unbounded_channel::<()>(); + let event_tx_for_cb = event_tx.clone(); + let reserved_root = path.to_path_buf(); + let mut errors = ErrorThrottle::default(); + + let mut watcher = notify::recommended_watcher(move |res: notify::Result<Event>| { + let event = match res { + Ok(event) => event, + Err(err) => { + // Logged, not acted on: an error does not say which + // changes it cost, and lost events are reported as a + // rescan event instead (see `is_relevant_event`). + if let Some(suppressed) = errors.admit(Instant::now()) { + tracing::warn!(folder_id, %err, suppressed, "folder watcher error"); + } + return; + } + }; + if !is_relevant_event(&event) { + return; + } + if is_our_own_media(&reserved_root, &event) { + return; + } + // Best-effort send — the debounce task may be tearing down. + let _ = event_tx_for_cb.send(()); + }) + .map_err(|e| AppError::Other(format!("notify watcher init: {e}")))?; + + watcher + .watch(path, RecursiveMode::Recursive) + .map_err(|e| AppError::Other(format!("notify watch {}: {e}", path.display())))?; + + let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); + tauri::async_runtime::spawn(debounce_loop( + event_rx, + shutdown_rx, + app.clone(), + folder_id, + library_id, + event_tx.clone(), + )); + + Ok(( + FolderHandle { + _watcher: Some(watcher), + _shutdown: shutdown_tx, + }, + event_tx, + )) +} + +/// Retry task for a watched folder that could not be armed. +/// +/// Every [`REARM_INTERVAL`] it looks for the folder; once it is there +/// and armed, the watcher takes the placeholder's place in the map and +/// one scan is asked for. Changes made while the folder was away — the +/// whole drive's worth, if it comes back with new music on it — never +/// produced an event, so without that scan they would wait for the next +/// one. +/// +/// Ends when its placeholder is dropped: the folder was unwatched or +/// removed, the profile switched, or a later `watch` replaced it. +async fn rearm_loop( + app: AppHandle, + inner: Arc<Mutex<HashMap<i64, FolderHandle>>>, + mut shutdown_rx: oneshot::Receiver<()>, + folder_id: i64, + library_id: i64, + path: PathBuf, +) { + loop { + tokio::select! { + biased; + _ = &mut shutdown_rx => return, + _ = tokio::time::sleep(REARM_INTERVAL) => {} + } + // Through tokio's blocking pool: a share that went away can take + // the filesystem's whole timeout to answer a stat. + let present = tokio::fs::metadata(&path) + .await + .is_ok_and(|meta| meta.is_dir()); + if !present { + continue; + } + let (handle, scan) = match arm(&app, folder_id, library_id, &path) { + Ok(armed) => armed, + Err(err) => { + tracing::debug!(folder_id, %err, "folder is back but could not be armed yet"); + continue; + } + }; + { + let mut map = inner.lock().expect("watcher map poisoned"); + // Only if the placeholder is still ours. Every removal from + // the map happens under this lock and drops the sender with + // it, so a closed channel here means the folder was + // unwatched or replaced meanwhile — and the watcher just + // built is dropped rather than resurrecting it, possibly + // under another profile's folder of the same id. + if !matches!( + shutdown_rx.try_recv(), + Err(oneshot::error::TryRecvError::Empty) + ) { + return; + } + map.insert(folder_id, handle); + } + tracing::info!(folder_id, library_id, path = %path.display(), "watched folder is back; watching it again"); + let _ = scan.send(()); + return; + } +} + /// Filter notify events down to the ones that can change the library. /// We ignore Access (read-only, never mutates content) and /// AccessKind metadata-only changes. +/// +/// Plus the backend's own "events were lost, look again" signal: an +/// inotify queue that overflowed, an FSEvents stream that says a +/// subtree must be rescanned. It comes as `EventKind::Other` with the +/// rescan flag, usually naming no path, and it is precisely what a large +/// copy into the folder produces — dropping it meant that copy was never +/// picked up. It asks for the same debounced scan of the whole folder +/// any other event does. fn is_relevant_event(event: &Event) -> bool { - matches!( - event.kind, - EventKind::Create(_) | EventKind::Modify(_) | EventKind::Remove(_) - ) + event.need_rescan() + || matches!( + event.kind, + EventKind::Create(_) | EventKind::Modify(_) | EventKind::Remove(_) + ) +} + +/// Rate limit for the errors one folder's watcher reports. +#[derive(Default)] +struct ErrorThrottle { + last_logged: Option<Instant>, + suppressed: u64, +} + +impl ErrorThrottle { + /// `Some(n)` when the error arriving at `now` should be logged, `n` + /// being how many were swallowed since the last one that was, so + /// the log still says how often it happened. + fn admit(&mut self, now: Instant) -> Option<u64> { + match self.last_logged { + Some(last) if now.duration_since(last) < ERROR_LOG_INTERVAL => { + self.suppressed += 1; + None + } + _ => { + self.last_logged = Some(now); + Some(std::mem::take(&mut self.suppressed)) + } + } + } } /// Whether every path this event names is media WaveFlow itself keeps in @@ -336,11 +513,10 @@ pub async fn apply_toggle( "library_folder {folder_id} not found" ))); }; - if let Err(err) = manager.watch(folder_id, library_id, PathBuf::from(&path)) { - // Surface the error but the DB flag below is still - // updated so the UI reflects the user's intent. - tracing::warn!(folder_id, path, %err, "watch start failed"); - } + // A folder that cannot be armed right now is logged and retried + // until it can; the DB flag below is still updated so the UI + // reflects the user's intent. + manager.watch_or_wait(folder_id, library_id, PathBuf::from(&path)); } else { manager.unwatch(folder_id); } @@ -356,7 +532,7 @@ pub async fn apply_toggle( #[cfg(test)] mod tests { use super::*; - use notify::event::CreateKind; + use notify::event::{AccessKind, CreateKind, Flag}; fn event(paths: &[&str]) -> Event { let mut event = Event::new(EventKind::Create(CreateKind::Any)); @@ -393,4 +569,40 @@ mod tests { fn an_event_without_paths_is_kept() { assert!(!is_our_own_media(Path::new("/music"), &event(&[]))); } + + /// The backend saying it lost events (inotify overflow, FSEvents + /// must-scan) is the one event that most needs a scan: it is what a + /// large copy produces. + #[test] + fn a_lost_events_signal_asks_for_a_scan() { + let rescan = Event::new(EventKind::Other).set_flag(Flag::Rescan); + assert!(is_relevant_event(&rescan)); + assert!(!is_our_own_media(Path::new("/music"), &rescan)); + } + + #[test] + fn reads_and_unflagged_others_do_not() { + assert!(!is_relevant_event(&Event::new(EventKind::Access( + AccessKind::Any + )))); + assert!(!is_relevant_event(&Event::new(EventKind::Other))); + assert!(is_relevant_event(&event(&["/music/a.flac"]))); + } + + #[test] + fn errors_are_logged_once_a_minute_with_a_count() { + let mut throttle = ErrorThrottle::default(); + let start = Instant::now(); + assert_eq!(throttle.admit(start), Some(0)); + assert_eq!(throttle.admit(start + Duration::from_secs(1)), None); + assert_eq!(throttle.admit(start + Duration::from_secs(30)), None); + assert_eq!( + throttle.admit(start + ERROR_LOG_INTERVAL + Duration::from_secs(1)), + Some(2) + ); + assert_eq!( + throttle.admit(start + ERROR_LOG_INTERVAL + Duration::from_secs(2)), + None + ); + } } From 46325953480c6606a6de216d46724152626b998c Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 01:23:28 +0200 Subject: [PATCH 04/13] fix(edit): refuse a tag save over a file another program changed The Track Properties form writes the title, numbers and year back on every save, from the values the database held when the dialog opened. A tag another program wrote while the dialog was open was silently reverted. The dialog now reads the file's stamp (size and modification time, as an opaque string from waveflow_core::tagio::file_stamp) each time it loads the track and hands it back with the save. update_track_tags checks it before pausing playback and again just before the write, and refuses with AppError::FileChangedOnDisk when the file moved; it returns the new stamp so the dialog's own saves never trip it. The refusal shows its own message with a "Reload from file" action, which rescans the track's library folder (reread_track_folder) and refills the form, since reloading from the database alone would show the same stale values. New strings are added to all 17 locales. Callers that send only the fields the user chose (the batch editor, the tag fetcher) are unchanged. --- docs/features/library.md | 2 +- src-tauri/crates/app/src/commands/edit.rs | 120 +++++++++++++++++- src-tauri/crates/app/src/error.rs | 9 ++ src-tauri/crates/app/src/lib.rs | 2 + src-tauri/crates/core/src/tagio.rs | 67 ++++++++++ .../common/TrackPropertiesModal.tsx | 106 +++++++++++++++- src/i18n/locales/ar.json | 2 + src/i18n/locales/de.json | 2 + src/i18n/locales/en.json | 2 + src/i18n/locales/es.json | 2 + src/i18n/locales/fr.json | 2 + src/i18n/locales/hi.json | 2 + src/i18n/locales/id.json | 2 + src/i18n/locales/it.json | 2 + src/i18n/locales/ja.json | 2 + src/i18n/locales/ko.json | 2 + src/i18n/locales/nl.json | 2 + src/i18n/locales/pt-BR.json | 2 + src/i18n/locales/pt.json | 2 + src/i18n/locales/ru.json | 2 + src/i18n/locales/tr.json | 2 + src/i18n/locales/zh-CN.json | 2 + src/i18n/locales/zh-TW.json | 2 + src/lib/tauri/track.ts | 49 ++++++- 24 files changed, 374 insertions(+), 15 deletions(-) diff --git a/docs/features/library.md b/docs/features/library.md index 640d0f453..c45ff8ce5 100644 --- a/docs/features/library.md +++ b/docs/features/library.md @@ -157,7 +157,7 @@ Tag edits go through the same funnel: [`edit.rs`](../../src-tauri/crates/app/src - **Single-click play** — with `ui.single_click_play` on, the first click of a row plays it; the second click of a double-click (`detail > 1`) and the row's `dblclick` stand down, so a double-click out of habit is one play instead of three. Either way, playing a row clears the selection its own click made (#897): with double-click play the first click selects the row, and the blue selection used to outrank the green "now playing" on that very row. The library, album and playlist tables all do this; playing a selection from the action bar is a separate path and keeps it. - **A-Z navigator** — letter rail on the artists tab, NFD-normalised so accents (É → E, Ñ → N) bucket correctly. - **Multi-select** — ctrl/shift across rows with a floating action bar (Play / Add to queue / Add to playlist / Remove) anchored to the bottom of the viewport. **Ctrl+A** (Cmd+A) selects every local row of the list on screen — the tracks tab, an opened folder, an inventory category, an album, a playlist, once that list has loaded (right after navigating, the previous one is still in state) — ([`useSelectAllShortcut`](../../src/hooks/useSelectAllShortcut.ts)) — and leaves the combo to a focused field or an open dialog. The selection bar shows on the tracks tab, in an opened folder and in an inventory category, and the selection clears when the tab, the folder or the inventory category changes. Each row's number gives way to a **checkbox** ([`RowSelectCheckbox`](../../src/components/common/RowSelectCheckbox.tsx)) on keyboard focus (it is hidden by opacity, so it stays in the tab order) and on every row once a selection has been _asked for_ — a Ctrl or Shift click, or Ctrl+A (`useMultiSelect().checkboxMode`). A plain click only highlights its row, which keeps its number: switching the whole list to boxes on the way to a double-click play is what [#859](https://github.com/InstaZDLL/WaveFlow/issues/859) reported. The box no longer shows on hover, in either click mode ([#867](https://github.com/InstaZDLL/WaveFlow/issues/867)): it sits where the number is, so it caught clicks meant for the row, and it put a checkbox under every pass of the mouse. Each box is named after its row for screen readers; its click and Space never reach the row, which would play it or reset the selection. A row clicked with the mouse never shows a focus ring, whatever key is pressed next — Escape, a digit, or a desktop shortcut such as Super+Q under Hyprland ([#866](https://github.com/InstaZDLL/WaveFlow/issues/866)): [`inputModality`](../../src/lib/inputModality.ts) marks focus that arrives from a pointer, and `app.css` keeps a marked element ringless; focus reached from the keyboard keeps its ring. Server rows have no rowid to select and keep their number. -- **Track Properties dialog** — foobar2000-style modal with the full tag set, audio specs, analysis results, file path and a Show in Explorer button. The read-only view shows every tag the form edits (track and disc number on two rows — the old "1 / 37" read as track 1 of 37 — and the genre), and **Edit** sits in the header next to Close rather than in the footer a scroll away. In the form, artist, album and genre are [`TagCombobox`](../../src/components/common/TagCombobox.tsx) fields: click or type and a list opens with what the library already holds. Genres are few and loaded whole, then filtered as you type (accent- and case-insensitive); artists and albums can run to thousands, so they come from the backend's `search_artists` / `search_albums` (bounded, prefix-first, the top bar's search) after a 150 ms pause, cached per query for the life of the dialog and shown as returned, since that search matches a normalised form the list would not. For the genre, the list adds the common genres of [`genrePresets.ts`](../../src/lib/genrePresets.ts) (the ID3v1 / Winamp list tag editors propose, trimmed, plus a few recent genres) — anything typed is still taken as is. A pick replaces the genre rather than adding one: the save stores one genre, so completing a `;`-separated list would create a genre named after the whole list. The list is portalled (the metadata section clips with `overflow: hidden`) and focus never leaves the input, so Escape closes the list first and the dialog second. **What a save sends, and what it changes.** Title, numbers and year always; artist, album and genre only when the field differs from what the track holds. The artist field is filled from `get_track_artists`, the credits joined with `"; "` — the only separator `split_artist_name` splits on — not from the row's display `artist_name`, which is joined with `", "`: echoing that back merged a multi-artist track's credits into one artist named "A, B", in the database and in the file, on any save, a title-only one included. Artists, albums and genres are found by a canonical key that ignores case, punctuation and spacing, so "The Wall" typed over "the wall" lands on the same row; the save then **respells** that row (`respell` in `edit.rs`), since nothing else ever renamed it and the fix showed in the file alone. Only on an explicit edit: the scanner leaves names as they were first seen, or two files spelling an artist differently would rename it back and forth on every scan. The save also recomputes the year of the album the track left and the one it joined (`refresh_album_years`, shared with the folder scan), which the header otherwise kept until the next scan of that folder. +- **Track Properties dialog** — foobar2000-style modal with the full tag set, audio specs, analysis results, file path and a Show in Explorer button. The read-only view shows every tag the form edits (track and disc number on two rows — the old "1 / 37" read as track 1 of 37 — and the genre), and **Edit** sits in the header next to Close rather than in the footer a scroll away. In the form, artist, album and genre are [`TagCombobox`](../../src/components/common/TagCombobox.tsx) fields: click or type and a list opens with what the library already holds. Genres are few and loaded whole, then filtered as you type (accent- and case-insensitive); artists and albums can run to thousands, so they come from the backend's `search_artists` / `search_albums` (bounded, prefix-first, the top bar's search) after a 150 ms pause, cached per query for the life of the dialog and shown as returned, since that search matches a normalised form the list would not. For the genre, the list adds the common genres of [`genrePresets.ts`](../../src/lib/genrePresets.ts) (the ID3v1 / Winamp list tag editors propose, trimmed, plus a few recent genres) — anything typed is still taken as is. A pick replaces the genre rather than adding one: the save stores one genre, so completing a `;`-separated list would create a genre named after the whole list. The list is portalled (the metadata section clips with `overflow: hidden`) and focus never leaves the input, so Escape closes the list first and the dialog second. **What a save sends, and what it changes.** Title, numbers and year always; artist, album and genre only when the field differs from what the track holds. The artist field is filled from `get_track_artists`, the credits joined with `"; "` — the only separator `split_artist_name` splits on — not from the row's display `artist_name`, which is joined with `", "`: echoing that back merged a multi-artist track's credits into one artist named "A, B", in the database and in the file, on any save, a title-only one included. Artists, albums and genres are found by a canonical key that ignores case, punctuation and spacing, so "The Wall" typed over "the wall" lands on the same row; the save then **respells** that row (`respell` in `edit.rs`), since nothing else ever renamed it and the fix showed in the file alone. Only on an explicit edit: the scanner leaves names as they were first seen, or two files spelling an artist differently would rename it back and forth on every scan. The save also recomputes the year of the album the track left and the one it joined (`refresh_album_years`, shared with the folder scan), which the header otherwise kept until the next scan of that folder. **A file changed by another program while the dialog is open is not overwritten.** Since the title, numbers and year always go back, a tag written by another tagger in the meantime would have been silently reverted to the values the dialog showed. The dialog reads the file's stamp (`get_track_file_stamp`: size and modification time, the pair the scanner's fast path trusts, as an opaque string so the nanoseconds survive JSON) each time it loads the track, and hands it back with the save; `update_track_tags` checks it before pausing playback and again just before the write, and refuses with `AppError::FileChangedOnDisk` when the file moved. The save returns the new stamp, so the dialog's own writes — and the rating and cover, which refresh the track — never trip it. The refusal names the cause and offers **Reload from file**, which rescans the track's library folder (`reread_track_folder`, the ordinary scan, so unchanged files cost a stat) and refills the form from the result: reloading from the database alone would show the same stale values with a fresh stamp. A change made _before_ the dialog opened and not yet scanned is outside what the stamp can see; a watched folder catches that one. The batch editor and the tag fetcher send only the fields the user chose, so they carry no stamp. - **POPM ratings** — 5-star with half-steps, round-tripped to the file's tag. Edit surfaces: inline `StarRating` in the library track list, integer-star submenu in the right-click `TrackContextMenu` (any view), full half-star widget in the `TrackPropertiesModal`. The backend command `set_track_rating` writes the POPM frame back to the file (binary `<email>\0<rating><counter>` for ID3v2, text `RATING=0-100` for Vorbis / MP4 / APE), updates `track.rating` in the DB, then emits `track:updated` so every open view re-fetches without polling. The write goes through `edit::patch_file` like every other tag edit (#644), so a `.dsf` gets its POPM frame through the DSF writer rather than stopping at the database. **ID3v2 ratings are written as the POPM frame itself**, on the concrete tag, and that is not a stylistic choice: lofty 0.25 changed what a POPM looks like on its generic tag — it arrives as `<provider>|<stars>|<counter>` through a whole-star `StarRating` rather than as the raw frame body its own documentation still describes — so a 0-255 value in half-star steps put into the generic tag is dropped on the way back to the file, silently, and a plain number or a raw body produces no frame at all (measured). The scanner reads both shapes for the same reason: reading only the old one is why an MP3's rating came back as nothing after that bump. `.dff` is still refused, with the reason. WaveFlow does not read a rating back out of a DSD file — the DSD extractor lifts title / artist / album / year and no more — so the database stays the copy this app trusts; what the write buys is the rating travelling with the file. Smart playlists expose this as the `rating_min` rule — see [smart-playlists.md](smart-playlists.md#custom-smart-playlists-recursive-boolean-rule-tree). - **Lightbox** — double-click any cover or artist photo to view full-size with keyboard navigation. diff --git a/src-tauri/crates/app/src/commands/edit.rs b/src-tauri/crates/app/src/commands/edit.rs index 746dd3d85..5547fcacc 100644 --- a/src-tauri/crates/app/src/commands/edit.rs +++ b/src-tauri/crates/app/src/commands/edit.rs @@ -130,6 +130,87 @@ fn validate_edit(edit: &TrackEdit) -> AppResult<()> { Ok(()) } +/// The file's stamp ([`waveflow_core::tagio::file_stamp`]) as the tag +/// editor opens, for it to hand back with its save. `None` when the file +/// cannot be reached, in which case the save is not checked against +/// anything and fails on its own when it tries to write. +#[tauri::command] +pub async fn get_track_file_stamp( + state: tauri::State<'_, AppState>, + track_id: i64, +) -> AppResult<Option<String>> { + let pool = state.require_profile_pool().await?; + let path: Option<String> = sqlx::query_scalar("SELECT file_path FROM track WHERE id = ?") + .bind(track_id) + .fetch_optional(&*pool) + .await?; + let path = path.ok_or_else(|| AppError::Other(format!("track {track_id} not found")))?; + tokio::task::spawn_blocking(move || { + waveflow_core::tagio::file_stamp(std::path::Path::new(&path)).ok() + }) + .await + .map_err(|e| AppError::Other(format!("file stamp join: {e}"))) +} + +/// Refuse to write over a file that changed since the editor read it. +/// +/// The dialog's form holds the values the library had when it opened, +/// and its save writes the title, the numbers and the year back whether +/// or not the user touched them — so if another tagger rewrote the file +/// meanwhile, saving would undo that program's change without a word. +fn ensure_unchanged(path: &std::path::Path, opened_at: &str) -> AppResult<()> { + if waveflow_core::tagio::file_stamp(path)? == opened_at { + Ok(()) + } else { + Err(AppError::FileChangedOnDisk) + } +} + +/// Re-read the folder holding `track_id`, so the library catches up with +/// a file another program changed — what the tag editor's "reload" +/// does after a save was refused by [`AppError::FileChangedOnDisk`]. +/// +/// The whole folder, through the ordinary scan, rather than a one-file +/// path of its own: the scanner is the only code that turns a file into +/// rows (albums, credits, genres, artwork), and its fast path skips every +/// file whose size and time have not moved, so the cost is a walk of the +/// folder plus the files that did change — the same scan the folder +/// watcher would have run. +#[tauri::command] +pub async fn reread_track_folder( + state: tauri::State<'_, AppState>, + app: AppHandle, + track_id: i64, +) -> AppResult<()> { + let pool = state.require_profile_pool().await?; + let profile_id = state.require_profile_id().await?; + let folder_id: Option<Option<i64>> = + sqlx::query_scalar("SELECT folder_id FROM track WHERE id = ?") + .bind(track_id) + .fetch_optional(&*pool) + .await?; + let folder_id = folder_id + .flatten() + .ok_or_else(|| AppError::Other(format!("track {track_id} is in no library folder")))?; + let artwork_dir = state.paths.profile_artwork_dir(profile_id); + crate::commands::scan::scan_folder_inner(&pool, &artwork_dir, folder_id, Some(&app), false) + .await?; + // Same follow-ups as a manual folder scan: ship the sync ops it + // queued, and let every view refetch. + state.drain.notify(); + let _ = app.emit("track:updated", track_id); + let _ = app.emit("library:rescanned", ()); + Ok(()) +} + +/// Write `edit` into the track's file, then bring the database in line. +/// +/// `file_stamp` is what [`get_track_file_stamp`] returned when the +/// editor opened. When given, the write is refused with +/// [`AppError::FileChangedOnDisk`] if the file no longer matches it. +/// Callers that send only the fields the user changed (the tag fetcher) +/// leave it out. Returns the file's stamp after the write, so the editor +/// can save again without being refused by its own change. #[tauri::command] pub async fn update_track_tags( state: tauri::State<'_, AppState>, @@ -137,7 +218,8 @@ pub async fn update_track_tags( app: AppHandle, track_id: i64, edit: TrackEdit, -) -> AppResult<()> { + file_stamp: Option<String>, +) -> AppResult<Option<String>> { validate_edit(&edit)?; let pool = state.require_profile_pool().await?; @@ -154,6 +236,15 @@ pub async fn update_track_tags( let row = row.ok_or_else(|| AppError::Other(format!("track {track_id} not found")))?; let path = std::path::PathBuf::from(&row.file_path); + // 1b. A save that will be refused should not pause the music first, + // so the stamp is checked before step 2 as well as at the write. + if let Some(opened_at) = file_stamp.clone() { + let path = path.clone(); + tokio::task::spawn_blocking(move || ensure_unchanged(&path, &opened_at)) + .await + .map_err(|e| AppError::Other(format!("file stamp join: {e}")))??; + } + // 2. If the engine is playing this track, pause before opening. // Releases the read handle that would otherwise refuse our write // open on Windows. Resume is the user's call — see module doc. @@ -176,13 +267,22 @@ pub async fn update_track_tags( // On a blocking thread, not here: this reaches the disk, and on the // network share the whole of #590 is about it can take seconds. An // async worker parked on it is one fewer for every other command. + // The stamp is checked again here, as close to the write as it gets: + // the pause above can take its time, and a tagger saving during it + // is exactly the case being guarded. { let path = path.clone(); let edit = edit.clone(); - tokio::task::spawn_blocking(move || write_tags_to_file(&path, &edit)) - .await - .map_err(|e| AppError::Other(format!("tag write join: {e}")))? - .map_err(|e| AppError::Other(format!("tag write failed: {e}")))?; + let file_stamp = file_stamp.clone(); + tokio::task::spawn_blocking(move || -> AppResult<()> { + if let Some(opened_at) = &file_stamp { + ensure_unchanged(&path, opened_at)?; + } + write_tags_to_file(&path, &edit) + .map_err(|e| AppError::Other(format!("tag write failed: {e}"))) + }) + .await + .map_err(|e| AppError::Other(format!("tag write join: {e}")))??; } // 3b. The file has changed on disk; recompute its hash so the @@ -211,7 +311,15 @@ pub async fn update_track_tags( let _ = app.emit("track:updated", track_id); let _ = app.emit("library:rescanned", ()); let _ = app.emit("player:queue-changed", ()); - Ok(()) + + // The file as this write left it, which is what the next save from + // the same dialog has to match. Best-effort: the tags are written + // and the database is in line, so failing the command here would + // report a save that happened as one that did not. + let stamp = tokio::task::spawn_blocking(move || waveflow_core::tagio::file_stamp(&path).ok()) + .await + .unwrap_or(None); + Ok(stamp) } /// Per-track result of a batch update. Surfaced to the frontend so it diff --git a/src-tauri/crates/app/src/error.rs b/src-tauri/crates/app/src/error.rs index ffacc3fff..202ce8ba3 100644 --- a/src-tauri/crates/app/src/error.rs +++ b/src-tauri/crates/app/src/error.rs @@ -112,6 +112,15 @@ pub enum AppError { #[error("zip error: {0}")] Zip(#[from] zip::result::ZipError), + /// The tag editor's save found the file no longer the one its dialog + /// read: another program wrote it in the meantime. Refused rather + /// than overwriting that change with the values the dialog showed. + /// The frontend recognises this message (`FILE_CHANGED_ON_DISK` in + /// `src/lib/tauri/track.ts`) and offers to reload, so the wording is + /// part of the contract. + #[error("the file changed on disk since the editor opened it")] + FileChangedOnDisk, + #[error("{0}")] Other(String), } diff --git a/src-tauri/crates/app/src/lib.rs b/src-tauri/crates/app/src/lib.rs index 0fe02584a..5fdce98f3 100644 --- a/src-tauri/crates/app/src/lib.rs +++ b/src-tauri/crates/app/src/lib.rs @@ -883,6 +883,8 @@ pub fn run() { commands::track::search_tracks_advanced, commands::edit::update_track_tags, commands::edit::update_tracks_batch, + commands::edit::get_track_file_stamp, + commands::edit::reread_track_folder, commands::edit::update_track_cover, commands::track::toggle_like_track, commands::track::list_liked_track_ids, diff --git a/src-tauri/crates/core/src/tagio.rs b/src-tauri/crates/core/src/tagio.rs index 2006a940a..56937ff6c 100644 --- a/src-tauri/crates/core/src/tagio.rs +++ b/src-tauri/crates/core/src/tagio.rs @@ -391,6 +391,73 @@ fn rename_over(from: &Path, to: &Path) -> io::Result<()> { } } +/// A cheap fingerprint of `path` as it is on disk: its size and its +/// modification time, to the precision the filesystem keeps. +/// +/// What the tag editor reads when it opens and checks again before it +/// writes. Its form shows the values the library holds, and a save +/// writes them all back, so a file another tagger rewrote in between +/// would quietly lose that program's change. The pair is the one the +/// scanner's fast path trusts to say a file has not changed, and it +/// costs a `stat` rather than a read of the file. +/// +/// Opaque on purpose: it is only ever compared for equality, and a +/// string carries nanoseconds intact through JSON, where a number would +/// be rounded to what a double holds. +pub fn file_stamp(path: &Path) -> io::Result<String> { + let meta = std::fs::metadata(path)?; + let modified = meta.modified()?; + let nanos = match modified.duration_since(std::time::UNIX_EPOCH) { + Ok(after) => after.as_nanos() as i128, + // A file dated before 1970 is unusual, not invalid. + Err(before) => -(before.duration().as_nanos() as i128), + }; + Ok(format!("{}:{nanos}", meta.len())) +} + +#[cfg(test)] +mod stamp_tests { + use super::*; + + #[test] + fn the_stamp_holds_while_the_file_does_not_change() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a.flac"); + std::fs::write(&path, b"fLaC").unwrap(); + assert_eq!(file_stamp(&path).unwrap(), file_stamp(&path).unwrap()); + } + + #[test] + fn a_rewrite_changes_the_stamp() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a.flac"); + std::fs::write(&path, b"fLaC").unwrap(); + let before = file_stamp(&path).unwrap(); + std::fs::write(&path, b"fLaC with a longer tag").unwrap(); + assert_ne!(file_stamp(&path).unwrap(), before); + } + + /// Same size, new content: what a tagger editing one field in the + /// tag's padding produces. Only the time can tell it apart. + #[test] + fn a_same_size_rewrite_changes_it_through_the_time() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a.flac"); + std::fs::write(&path, b"fLaC").unwrap(); + let before = file_stamp(&path).unwrap(); + let file = std::fs::File::options().write(true).open(&path).unwrap(); + let later = std::fs::metadata(&path).unwrap().modified().unwrap() + Duration::from_secs(5); + file.set_modified(later).unwrap(); + assert_ne!(file_stamp(&path).unwrap(), before); + } + + #[test] + fn a_missing_file_has_no_stamp() { + let dir = tempfile::tempdir().unwrap(); + assert!(file_stamp(&dir.path().join("gone.flac")).is_err()); + } +} + /// The ID3v2 tag already on disk, as a span of bytes at the head of the /// file (#590). #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/src/components/common/TrackPropertiesModal.tsx b/src/components/common/TrackPropertiesModal.tsx index 6ba20434a..148f55bf5 100644 --- a/src/components/common/TrackPropertiesModal.tsx +++ b/src/components/common/TrackPropertiesModal.tsx @@ -6,6 +6,7 @@ import { ExternalLink, ImageUp, Pencil, + RotateCcw, Save, Sparkles, TriangleAlert, @@ -18,7 +19,10 @@ import { formatDuration, getTrack, getTrackArtists, + getTrackFileStamp, getTrackGenres, + isFileChangedOnDisk, + rereadTrackFolder, setTrackRating, updateTrackCover, updateTrackTags, @@ -129,6 +133,16 @@ export function TrackPropertiesModal({ // can't tag — and the dialog used to swallow all of them into a // console line while looking like nothing had happened. const [saveError, setSaveError] = useState<string | null>(null); + // The file's stamp (size + modification time) as of the values the + // dialog shows, handed back with the save. The form writes the title, + // numbers and year back whether or not they were touched, so a file + // another tagger rewrote while the dialog was open would lose that + // change; with the stamp, the backend refuses the save instead. + // `null` until loaded, and then the save goes unchecked. + const [fileStamp, setFileStamp] = useState<string | null>(null); + // Set when a save was refused for that reason: the error then offers + // to reload the values from the file instead of repeating the save. + const [fileChanged, setFileChanged] = useState(false); // Genres as they stand in the database, loaded alongside the track. // `null` means "not loaded yet", which is what gates the save below. const [genres, setGenres] = useState<string[] | null>(null); @@ -189,6 +203,25 @@ export function TrackPropertiesModal({ }; }, [track]); + // Re-read with every refresh of the track, which is what the form is + // hydrated from: a save of our own, a rating or a new cover all + // rewrite the file and emit `track:updated`, and the stamp has to move + // with them or the next save would be refused by our own change. + useEffect(() => { + if (!track) return; + let cancelled = false; + getTrackFileStamp(track.id) + .then((stamp) => { + if (!cancelled) setFileStamp(stamp); + }) + .catch((err) => + console.error("[TrackProperties] get_track_file_stamp failed", err), + ); + return () => { + cancelled = true; + }; + }, [track]); + // What the library already holds, offered while editing so a value is // picked rather than retyped — a typo in an artist name makes a second // artist. A failed lookup only means fewer suggestions. @@ -340,6 +373,7 @@ export function TrackPropertiesModal({ if (!track || saving) return; setSaving(true); setSaveError(null); + setFileChanged(false); try { const edit: TrackEdit = { title: form.title, @@ -384,7 +418,11 @@ export function TrackPropertiesModal({ if (sendingArtist || album !== (track.album_title ?? "").trim()) { edit.album = album; } - await updateTrackTags(track.id, edit); + const stamp = await updateTrackTags(track.id, edit, fileStamp); + // Straight away rather than through the refresh `track:updated` + // triggers: a second save made before that lands must match the + // file this one left. + setFileStamp(stamp); // Only when we sent one — otherwise this would record a genre we // never wrote, and the *next* save would act on it. setGenreTouched(false); @@ -421,6 +459,50 @@ export function TrackPropertiesModal({ // surrounding views listen to and react to (re-fetch the row). } catch (err) { console.error("[TrackProperties] update_track_tags failed", err); + setFileChanged(isFileChangedOnDisk(err)); + setSaveError(String(err)); + } finally { + setSaving(false); + } + }; + + // After a save refused because the file changed: rescan its folder so + // the library holds what the other program wrote, then refill the + // dialog from it. Reloading from the database alone would show the + // same stale values with a fresh stamp, and the next save would + // overwrite the change after all. The user's unsaved input is dropped, + // which is the point: it was typed over values that no longer exist. + const handleReload = async () => { + if (!track || saving) return; + setSaving(true); + try { + await rereadTrackFolder(track.id); + const [fresh, genreRows, artistRows] = await Promise.all([ + getTrack(track.id), + getTrackGenres(track.id), + getTrackArtists(track.id), + ]); + setGenres(genreRows); + setGenreTouched(false); + setArtists(artistRows.length > 0 ? artistRows : null); + // The hydration below carries the genre over and reads the + // credits from their ref, so both are put in place here. + setForm((prev) => ({ + ...prev, + genre: genreRows.join("; "), + artist: + artistRows.length > 0 + ? artistRows.join("; ") + : (fresh?.artist_name ?? ""), + })); + // A new `track` re-runs the hydration (which also leaves edit + // mode) and the stamp fetch. + if (fresh) setTrack(fresh); + setSaveError(null); + setFileChanged(false); + } catch (err) { + console.error("[TrackProperties] reread_track_folder failed", err); + setFileChanged(false); setSaveError(String(err)); } finally { setSaving(false); @@ -437,6 +519,7 @@ export function TrackPropertiesModal({ if (!path) return; setSaving(true); setSaveError(null); + setFileChanged(false); await updateTrackCover(track.id, path); // The backend's `track:updated` event triggers a refetch in the // surrounding views; the modal itself stays open on the same @@ -861,9 +944,24 @@ export function TrackPropertiesModal({ className="mx-6 mb-4 flex items-start gap-2 rounded-xl border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-700 dark:border-red-900/60 dark:bg-red-950/40 dark:text-red-300" > <TriangleAlert size={16} className="mt-0.5 shrink-0" /> - <span className="min-w-0 break-words"> - {t("trackProperties.saveFailed", { reason: saveError })} - </span> + <div className="min-w-0 flex-1"> + <span className="break-words"> + {fileChanged + ? t("trackProperties.fileChanged") + : t("trackProperties.saveFailed", { reason: saveError })} + </span> + {fileChanged && ( + <button + type="button" + onClick={handleReload} + disabled={saving} + className="mt-2 inline-flex items-center gap-1.5 rounded-lg border border-red-300 bg-white px-2.5 py-1 text-xs font-medium text-red-700 hover:bg-red-100 disabled:opacity-50 dark:border-red-800 dark:bg-red-950/60 dark:text-red-200 dark:hover:bg-red-900/50 transition-colors" + > + <RotateCcw size={12} /> + <span>{t("trackProperties.reloadFromFile")}</span> + </button> + )} + </div> </div> )} diff --git a/src/i18n/locales/ar.json b/src/i18n/locales/ar.json index 4286e6ae7..50961e2cd 100644 --- a/src/i18n/locales/ar.json +++ b/src/i18n/locales/ar.json @@ -1387,6 +1387,8 @@ "save": "حفظ", "saving": "جارٍ الحفظ…", "saveFailed": "تعذّر الحفظ: {{reason}}", + "fileChanged": "عدّل برنامج آخر الملف منذ فتح هذه النافذة. أعد تحميل قيمه قبل الحفظ حتى لا تُستبدل تلك التغييرات.", + "reloadFromFile": "إعادة التحميل من الملف", "fields": { "title": "العنوان", "artist": "الفنان (الفنانون)", diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 27d32a223..528b95827 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -1238,6 +1238,8 @@ "save": "Speichern", "saving": "Wird gespeichert…", "saveFailed": "Speichern fehlgeschlagen: {{reason}}", + "fileChanged": "Die Datei wurde seit dem Öffnen dieses Fensters von einem anderen Programm geändert. Lade ihre Werte neu, bevor du speicherst, damit diese Änderungen nicht überschrieben werden.", + "reloadFromFile": "Aus der Datei neu laden", "fields": { "title": "Titel", "artist": "Künstler", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 80891f4b5..f1894fcdc 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -1238,6 +1238,8 @@ "save": "Save", "saving": "Saving…", "saveFailed": "Couldn't save: {{reason}}", + "fileChanged": "The file was changed by another program since this window opened. Reload its values before saving, so those changes are not overwritten.", + "reloadFromFile": "Reload from file", "fields": { "title": "Title", "artist": "Artist(s)", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index feda09f5e..c76632b2a 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -1281,6 +1281,8 @@ "save": "Guardar", "saving": "Guardando…", "saveFailed": "No se pudo guardar: {{reason}}", + "fileChanged": "Otro programa ha modificado el archivo desde que se abrió esta ventana. Vuelve a cargar sus valores antes de guardar para no sobrescribir esos cambios.", + "reloadFromFile": "Volver a cargar desde el archivo", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index a9ec73dae..425b61a2b 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -1281,6 +1281,8 @@ "save": "Enregistrer", "saving": "Enregistrement…", "saveFailed": "Échec de l'enregistrement : {{reason}}", + "fileChanged": "Le fichier a été modifié par un autre programme depuis l'ouverture de cette fenêtre. Rechargez ses valeurs avant d'enregistrer, pour ne pas écraser ces changements.", + "reloadFromFile": "Recharger depuis le fichier", "fields": { "title": "Titre", "artist": "Artiste(s)", diff --git a/src/i18n/locales/hi.json b/src/i18n/locales/hi.json index 230cd256d..877f1e2a7 100644 --- a/src/i18n/locales/hi.json +++ b/src/i18n/locales/hi.json @@ -1238,6 +1238,8 @@ "save": "सहेजें", "saving": "सहेज रहा है…", "saveFailed": "सहेजा नहीं जा सका: {{reason}}", + "fileChanged": "यह विंडो खुलने के बाद किसी दूसरे प्रोग्राम ने फ़ाइल बदल दी है। उन बदलावों पर ओवरराइट न हो, इसलिए सहेजने से पहले उसके मान फिर से लोड करें।", + "reloadFromFile": "फ़ाइल से फिर से लोड करें", "fields": { "title": "शीर्षक", "artist": "कलाकार", diff --git a/src/i18n/locales/id.json b/src/i18n/locales/id.json index 11e013566..332d48ff9 100644 --- a/src/i18n/locales/id.json +++ b/src/i18n/locales/id.json @@ -1236,6 +1236,8 @@ "save": "Simpan", "saving": "Menyimpan…", "saveFailed": "Gagal menyimpan: {{reason}}", + "fileChanged": "Berkas telah diubah oleh program lain sejak jendela ini dibuka. Muat ulang nilainya sebelum menyimpan agar perubahan tersebut tidak tertimpa.", + "reloadFromFile": "Muat ulang dari berkas", "fields": { "title": "Judul", "artist": "Artis", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index 83c9599e0..249904b5d 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -1281,6 +1281,8 @@ "save": "Salva", "saving": "Salvataggio…", "saveFailed": "Salvataggio non riuscito: {{reason}}", + "fileChanged": "Il file è stato modificato da un altro programma dopo l'apertura di questa finestra. Ricarica i suoi valori prima di salvare, per non sovrascrivere quelle modifiche.", + "reloadFromFile": "Ricarica dal file", "fields": { "title": "Titolo", "artist": "Artista/i", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 540f9278d..982d71358 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -1236,6 +1236,8 @@ "save": "保存", "saving": "保存中…", "saveFailed": "保存できませんでした：{{reason}}", + "fileChanged": "このウィンドウを開いた後に、別のプログラムがファイルを変更しました。その変更を上書きしないよう、保存する前に値を再読み込みしてください。", + "reloadFromFile": "ファイルから再読み込み", "fields": { "title": "タイトル", "artist": "アーティスト", diff --git a/src/i18n/locales/ko.json b/src/i18n/locales/ko.json index 8e8489b05..4d2b9a9f6 100644 --- a/src/i18n/locales/ko.json +++ b/src/i18n/locales/ko.json @@ -1236,6 +1236,8 @@ "save": "저장", "saving": "저장 중…", "saveFailed": "저장하지 못했습니다: {{reason}}", + "fileChanged": "이 창을 연 뒤 다른 프로그램이 파일을 변경했습니다. 그 변경 사항을 덮어쓰지 않도록 저장하기 전에 값을 다시 불러오세요.", + "reloadFromFile": "파일에서 다시 불러오기", "fields": { "title": "제목", "artist": "아티스트", diff --git a/src/i18n/locales/nl.json b/src/i18n/locales/nl.json index 127d73227..dd04b6999 100644 --- a/src/i18n/locales/nl.json +++ b/src/i18n/locales/nl.json @@ -1238,6 +1238,8 @@ "save": "Opslaan", "saving": "Opslaan…", "saveFailed": "Opslaan mislukt: {{reason}}", + "fileChanged": "Het bestand is door een ander programma gewijzigd sinds dit venster is geopend. Laad de waarden opnieuw voordat je opslaat, zodat die wijzigingen niet worden overschreven.", + "reloadFromFile": "Opnieuw laden uit bestand", "fields": { "title": "Titel", "artist": "Artiest(en)", diff --git a/src/i18n/locales/pt-BR.json b/src/i18n/locales/pt-BR.json index a686eb825..ff7b6498f 100644 --- a/src/i18n/locales/pt-BR.json +++ b/src/i18n/locales/pt-BR.json @@ -1282,6 +1282,8 @@ "save": "Salvar", "saving": "Salvando…", "saveFailed": "Não foi possível salvar: {{reason}}", + "fileChanged": "O arquivo foi alterado por outro programa depois que esta janela foi aberta. Recarregue os valores antes de salvar para não sobrescrever essas alterações.", + "reloadFromFile": "Recarregar do arquivo", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/pt.json b/src/i18n/locales/pt.json index 0e544f7e5..b3ee79df2 100644 --- a/src/i18n/locales/pt.json +++ b/src/i18n/locales/pt.json @@ -1282,6 +1282,8 @@ "save": "Guardar", "saving": "A guardar…", "saveFailed": "Não foi possível guardar: {{reason}}", + "fileChanged": "O ficheiro foi alterado por outro programa desde que esta janela foi aberta. Recarregue os valores antes de guardar, para não substituir essas alterações.", + "reloadFromFile": "Recarregar do ficheiro", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index cf18d822f..7e59ffcf6 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -1328,6 +1328,8 @@ "save": "Сохранить", "saving": "Сохранение…", "saveFailed": "Не удалось сохранить: {{reason}}", + "fileChanged": "После открытия этого окна файл был изменён другой программой. Перезагрузите значения перед сохранением, чтобы не затереть эти изменения.", + "reloadFromFile": "Перезагрузить из файла", "fields": { "title": "Название", "artist": "Исполнитель(и)", diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index f45c4dce9..8b9894ea4 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -1238,6 +1238,8 @@ "save": "Kaydet", "saving": "Kaydediliyor…", "saveFailed": "Kaydedilemedi: {{reason}}", + "fileChanged": "Bu pencere açıldıktan sonra dosya başka bir program tarafından değiştirildi. Bu değişikliklerin üzerine yazmamak için kaydetmeden önce değerleri yeniden yükleyin.", + "reloadFromFile": "Dosyadan yeniden yükle", "fields": { "title": "Başlık", "artist": "Sanatçı(lar)", diff --git a/src/i18n/locales/zh-CN.json b/src/i18n/locales/zh-CN.json index d845f0a6f..b769d3337 100644 --- a/src/i18n/locales/zh-CN.json +++ b/src/i18n/locales/zh-CN.json @@ -1236,6 +1236,8 @@ "save": "保存", "saving": "保存中…", "saveFailed": "保存失败：{{reason}}", + "fileChanged": "打开此窗口后，文件已被其他程序修改。请在保存前重新加载其值，以免覆盖这些更改。", + "reloadFromFile": "从文件重新加载", "fields": { "title": "标题", "artist": "艺人", diff --git a/src/i18n/locales/zh-TW.json b/src/i18n/locales/zh-TW.json index 18ed2559a..cbade6be8 100644 --- a/src/i18n/locales/zh-TW.json +++ b/src/i18n/locales/zh-TW.json @@ -1236,6 +1236,8 @@ "save": "儲存", "saving": "儲存中…", "saveFailed": "儲存失敗：{{reason}}", + "fileChanged": "開啟此視窗後，檔案已被其他程式修改。請在儲存前重新載入其值，以免覆寫這些變更。", + "reloadFromFile": "從檔案重新載入", "fields": { "title": "標題", "artist": "藝人", diff --git a/src/lib/tauri/track.ts b/src/lib/tauri/track.ts index 9bf3c516a..1109f341c 100644 --- a/src/lib/tauri/track.ts +++ b/src/lib/tauri/track.ts @@ -277,7 +277,7 @@ export async function listLikedTracks(): Promise<Track[]> { * (not transmitted) means "leave this field untouched"; an empty * string means "clear this field" where the format allows it. * - * `artist` accepts a comma-separated multi-artist string ("A, B"), + * `artist` accepts a `"; "`-separated multi-artist string ("A; B"), * which the backend splits via the same parser the scanner uses. */ export interface TrackEdit { @@ -290,12 +290,53 @@ export interface TrackEdit { genre?: string | null; } -/** Persist the edited tags both to the audio file and to the database. */ +/** + * Persist the edited tags both to the audio file and to the database. + * + * `fileStamp` is what {@link getTrackFileStamp} returned when the editor + * opened: given, the backend refuses the write when the file changed + * since (see {@link isFileChangedOnDisk}). Resolves to the file's stamp + * after the write, for the editor's next save. + */ export function updateTrackTags( trackId: number, edit: TrackEdit, -): Promise<void> { - return invoke<void>("update_track_tags", { trackId, edit }); + fileStamp?: string | null, +): Promise<string | null> { + return invoke<string | null>("update_track_tags", { + trackId, + edit, + fileStamp: fileStamp ?? null, + }); +} + +/** + * An opaque fingerprint of the track's file (size + modification time), + * or `null` when the file cannot be reached. Only ever compared for + * equality, by the backend. + */ +export function getTrackFileStamp(trackId: number): Promise<string | null> { + return invoke<string | null>("get_track_file_stamp", { trackId }); +} + +/** + * Rescan the library folder holding the track, so the database catches + * up with a file another program changed. Unchanged files are skipped by + * the scanner's fast path. + */ +export function rereadTrackFolder(trackId: number): Promise<void> { + return invoke<void>("reread_track_folder", { trackId }); +} + +/** + * The backend's `AppError::FileChangedOnDisk` message: a save refused + * because another program wrote the file after the editor opened it. + */ +export const FILE_CHANGED_ON_DISK = "the file changed on disk"; + +/** Whether `err` is a save refused by {@link FILE_CHANGED_ON_DISK}. */ +export function isFileChangedOnDisk(err: unknown): boolean { + return String(err).includes(FILE_CHANGED_ON_DISK); } /** Summary returned by `updateTracksBatch`. */ From 9f95714ee58b357ed270f3ef2f80a2c100dd8b81 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 01:54:03 +0200 Subject: [PATCH 05/13] fix(backup): give each atomic write its own partial, sweep stale ones after every attempt Two writes to one destination shared the predictable .partial name and truncated each other's file. Each write now creates a partial of its own exclusively (process id and counter in the name, extension kept last so a listing still knows an unfinished archive). Unfinished archives are swept after every backup attempt, failed or not; retention of finished ones still waits for a successful write. --- docs/features/playlists.md | 2 +- docs/features/ui.md | 4 +- src-tauri/crates/app/src/backup.rs | 20 +++- src-tauri/crates/core/src/atomic_write.rs | 128 ++++++++++++++++------ 4 files changed, 114 insertions(+), 40 deletions(-) diff --git a/docs/features/playlists.md b/docs/features/playlists.md index bb3fce676..055bc095e 100644 --- a/docs/features/playlists.md +++ b/docs/features/playlists.md @@ -31,7 +31,7 @@ Drag-and-drop is implemented with `@dnd-kit` over a virtualised list (`@tanstack ## M3U import / export -`export_playlist_m3u` writes a UTF-8 `.m3u8` with `#EXTINF:<duration>,<artist> - <title>` headers and absolute file paths, through a `.partial` sibling renamed over the destination once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so overwriting an existing playlist file that fails halfway leaves the old one intact rather than truncated. The importer accepts both `.m3u` and `.m3u8` (foobar2000, VLC, Rekordbox, hand-written sets) and matches against the active library: +`export_playlist_m3u` writes a UTF-8 `.m3u8` with `#EXTINF:<duration>,<artist> - <title>` headers and absolute file paths, through a partial sibling of its own (created exclusively) renamed over the destination once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so overwriting an existing playlist file that fails halfway leaves the old one intact rather than truncated. The importer accepts both `.m3u` and `.m3u8` (foobar2000, VLC, Rekordbox, hand-written sets) and matches against the active library: 1. **Canonical-path match** — strip Windows `\\?\` prefix, lowercase the drive letter, normalise separators. 2. **Basename fallback** — if path-match fails, try the basename. This recovers playlists exported before a library reorganisation. diff --git a/docs/features/ui.md b/docs/features/ui.md index 10630dcb9..bd4ebd9f7 100644 --- a/docs/features/ui.md +++ b/docs/features/ui.md @@ -602,7 +602,7 @@ Per-profile isolated database (libraries, playlists, settings, play history); sh [`commands/profile_io.rs`](../../src-tauri/crates/app/src/commands/profile_io.rs) packages a profile into a single `.waveflow` (zip) file containing `manifest.json` + `data.db` + the per-profile `artwork/` directory. Settings → Storage and backups → Profile backups exposes both buttons. -- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to `<target>.partial`, flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. +- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to a partial file of its own beside the target (`<stem>.<process>-<n>.waveflow.partial`, created exclusively, so two writes to one destination never share it), flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. - **Import:** always allocates a fresh profile row — never overwrites — under the archive's profile name, or `Name (2)`, `Name (3)`… when a profile already holds it (#767): `profile.name` is UNIQUE and restoring a backup of a profile that still exists is the ordinary case, so the name is taken by construction. It retries on the violation itself rather than checking first. The command returns the new id **and** that name, the success message shows the name, and Settings refreshes the profile list so the selector it points at actually lists the profile. It then extracts the archive under `profiles/<new_id>/`. Failures roll the row back so a half-imported profile doesn't survive the error. Before the sqlx migrator runs, [`normalise_migration_checksums`](../../src-tauri/crates/app/src/commands/profile_io.rs) rewrites `_sqlx_migrations.checksum` for every version present in both the archive and the local migrator — older builds checked out migration files with CRLF endings (Windows `core.autocrlf=true` + no `.gitattributes` lock) so their stored SHA-384 differs from the same SQL re-hashed today, even though the DDL is identical. A `.gitattributes` at repo root now pins `*.sql` / `*.rs` / `*.ts` / etc. to LF so future archives stay byte-stable. Once normalised, the new pool is opened once so any pending sqlx migrations replay before the user switches to it. An archive whose `_sqlx_migrations` lists a version unknown to the local migrator is rejected — that means the export came from a newer build. - **Feedback and failures:** an archive carries the artwork cache, so a large library takes minutes; the card shows an "Exporting…" / "Importing…" line while the buttons are greyed, since greyed buttons alone read as a hang. Both commands log their error at `warn` (`profile export failed` / `profile import failed`) — the UI can only show one generic sentence, and until #767 the cause was written nowhere. - **Out of scope:** the shared `app.db` (Last.fm key, Discord opt-in, `network.offline_mode`) belongs to the install, not the profile. @@ -615,7 +615,7 @@ Opt-in scheduled mirror of the manual export so the user's playlists / likes / r - **Config** lives in `app_setting` (install-wide, not per-profile): `backup.enabled` (bool, default OFF), `backup.interval_days` (1-90, default 7), `backup.folder` (string; empty = default `<app_data>/waveflow/backups/`), `backup.retention` (1-50, default 5 — per profile), `backup.last_run_at` (epoch ms). - **Loop** is a single tokio task started once at boot ([`spawn_backup_loop`](../../src-tauri/crates/app/src/backup.rs)). When disabled, parks on a `tokio::sync::Notify` (zero cost) until the user toggles. When enabled, computes the next deadline as `last_run_at + interval_days * 86_400_000` and uses `tokio::select!` between a sleep and the same `Notify` so config changes wake it without waiting for the old sleep to expire. -- **Pass** ([`run_one_backup`](../../src-tauri/crates/app/src/backup.rs)) iterates every row in `profile`, calls the shared [`profile_io::write_archive`](../../src-tauri/crates/app/src/commands/profile_io.rs) (pub-crate-ified from the manual-export path so the two stay bit-compatible), and applies retention per profile (`<sanitized-name>-*.waveflow` sorted by mtime, oldest beyond `retention` deleted) — **only after that profile's archive was written**. Retention is "keep the newest N", and pruning after a failed write deleted the oldest good backup on every failed run. An unfinished `*.waveflow.partial` is never counted as a backup; one left by a crash is removed once it is a day old, an age no write in progress reaches since every write touches it. The active profile gets a `PRAGMA wal_checkpoint(TRUNCATE)` first; inactive profiles are already cold on disk (the pool ran a checkpoint at switch / shutdown). +- **Pass** ([`run_one_backup`](../../src-tauri/crates/app/src/backup.rs)) iterates every row in `profile`, calls the shared [`profile_io::write_archive`](../../src-tauri/crates/app/src/commands/profile_io.rs) (pub-crate-ified from the manual-export path so the two stay bit-compatible), and applies retention per profile (`<sanitized-name>-*.waveflow` sorted by mtime, oldest beyond `retention` deleted) — **only after that profile's archive was written**. Retention is "keep the newest N", and pruning after a failed write deleted the oldest good backup on every failed run. Unfinished archives are swept after every attempt, failed or not, since a profile that keeps failing is the one that leaves them. An unfinished `*.waveflow.partial` is never counted as a backup; one left by a crash is removed once it is a day old, an age no write in progress reaches since every write touches it. The active profile gets a `PRAGMA wal_checkpoint(TRUNCATE)` first; inactive profiles are already cold on disk (the pool ran a checkpoint at switch / shutdown). - **Failure isolation:** per-profile errors are logged but don't abort the pass — one corrupt profile shouldn't block backups of the healthy ones. - **Commands** in [`commands/backup.rs`](../../src-tauri/crates/app/src/commands/backup.rs): `get_backup_config`, `set_backup_config` (also signals the loop), `run_backup_now`. UI is [`BackupCard`](../../src/components/views/settings/BackupCard.tsx) in Settings → Storage and backups → Profile backups right after the manual export/import. diff --git a/src-tauri/crates/app/src/backup.rs b/src-tauri/crates/app/src/backup.rs index 9ce6d66a2..51913e9bb 100644 --- a/src-tauri/crates/app/src/backup.rs +++ b/src-tauri/crates/app/src/backup.rs @@ -453,10 +453,12 @@ pub async fn run_one_backup( // to make room for: pruning anyway deleted the oldest good // backup on every failed run, so a profile that kept failing // was slowly left with none. - if written { - if let Err(err) = prune_old_backups(&folder, &safe, config.retention as usize).await { - tracing::warn!(?err, "auto backup retention sweep failed"); - } + // + // Unfinished archives left by a crash are swept either way: a + // profile that keeps failing is exactly the one that leaves them. + let keep = written.then_some(config.retention as usize); + if let Err(err) = prune_old_backups(&folder, &safe, keep).await { + tracing::warn!(?err, "auto backup retention sweep failed"); } done += 1; @@ -516,10 +518,13 @@ fn classify_backup_file(name: &str, prefix: &str) -> BackupFile { } } +/// Sweep the backup folder for one profile: remove its stale unfinished +/// archives, and when `keep` is given, every finished archive past the +/// newest `keep`. async fn prune_old_backups( folder: &std::path::Path, name_prefix: &str, - keep: usize, + keep: Option<usize>, ) -> AppResult<()> { let prefix = format!("{name_prefix}-"); let folder = folder.to_path_buf(); @@ -566,6 +571,9 @@ async fn prune_old_backups( } entries.push((path, mtime)); } + let Some(keep) = keep else { + return Ok(0); + }; entries.sort_by_key(|e| std::cmp::Reverse(e.1)); // newest first let mut removed = 0; for (path, _) in entries.into_iter().skip(keep) { @@ -736,7 +744,7 @@ mod tests { #[test] fn an_unfinished_archive_is_not_a_backup() { assert_eq!( - classify_backup_file("Main-2026-10-19T10-00-00.waveflow.partial", "Main-"), + classify_backup_file("Main-2026-10-19T10-00-00.4242-1.waveflow.partial", "Main-"), BackupFile::Partial ); } diff --git a/src-tauri/crates/core/src/atomic_write.rs b/src-tauri/crates/core/src/atomic_write.rs index aac51dd71..224e7df0b 100644 --- a/src-tauri/crates/core/src/atomic_write.rs +++ b/src-tauri/crates/core/src/atomic_write.rs @@ -8,36 +8,89 @@ //! week's archive, and an automatic backup cut short sits in the backup //! folder looking like the newest good one. //! -//! Here the bytes go to `<name>.partial` beside the destination, reach +//! Here the bytes go to a partial file beside the destination, reach //! the disk, and only then replace it in a single `rename`. Any failure -//! removes the partial and leaves the destination as it was. +//! removes the partial and leaves the destination as it was. Each write +//! gets a partial of its own, created exclusively: two writes aimed at +//! the same destination (an export run while a backup writes there) +//! would otherwise truncate each other's half-written file. //! //! Not for audio files: those go through [`crate::tagio`], which has to //! keep the original's inode where it can and copies it before writing. +use std::ffi::OsString; use std::fs::File; use std::io::{self, Write}; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; /// Appended to the destination's file name while it is being written. /// Anything that lists a folder for finished files (the backup /// retention, for one) must not take a name ending in it as one. pub const PARTIAL_SUFFIX: &str = ".partial"; -/// `<destination file name>.partial`, in the destination's directory. +/// The partial for one write of `target`: `<stem>.<token>.<extension>` +/// followed by [`PARTIAL_SUFFIX`], or `<name>.<token>` and the suffix for +/// a name without an extension, in the destination's directory. /// -/// Beside it, never in the system temp directory: `rename` is only +/// The extension stays last before the suffix so a folder listing can +/// still tell what the partial would have become: the backup retention +/// knows `….waveflow.partial` as an unfinished archive. Beside the +/// destination, never in the system temp directory: `rename` is only /// atomic within a filesystem, and an export folder is routinely on a /// different one than `/tmp`. -pub fn partial_path(target: &Path) -> PathBuf { - let mut name = target - .file_name() - .map(|n| n.to_os_string()) - .unwrap_or_default(); +pub fn partial_path(target: &Path, token: &str) -> PathBuf { + let mut name = OsString::new(); + match (target.file_stem(), target.extension()) { + (Some(stem), Some(extension)) => { + name.push(stem); + name.push("."); + name.push(token); + name.push("."); + name.push(extension); + } + _ => { + name.push(target.file_name().unwrap_or_default()); + name.push("."); + name.push(token); + } + } name.push(PARTIAL_SUFFIX); target.with_file_name(name) } +/// A token no other write of this process, or of another process, uses: +/// the process id and a counter. +fn next_token() -> String { + static NEXT: AtomicU64 = AtomicU64::new(0); + format!( + "{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + ) +} + +/// Create a partial no one else holds. A name already taken (a leftover +/// from a crashed process that had the same id) moves on to the next +/// token rather than writing over it. +fn create_partial(target: &Path) -> io::Result<(PathBuf, File)> { + let mut last_err = None; + for _ in 0..16 { + let path = partial_path(target, &next_token()); + match std::fs::OpenOptions::new() + .read(true) + .write(true) + .create_new(true) + .open(&path) + { + Ok(file) => return Ok((path, file)), + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => last_err = Some(err), + Err(err) => return Err(err), + } + } + Err(last_err.unwrap_or_else(|| io::Error::other("no free partial name"))) +} + /// Removes the partial unless it was renamed into place, so every early /// return between creating it and committing it cleans up after itself. struct Partial { @@ -74,21 +127,12 @@ where .unwrap_or_else(|| Path::new(".")); std::fs::create_dir_all(directory).map_err(E::from)?; + let (path, mut file) = create_partial(target).map_err(E::from)?; let mut partial = Partial { - path: partial_path(target), + path, committed: false, }; - // `create` rather than `create_new`: a partial already there is the - // leftover of a write that crashed before it could clean up, and the - // name is ours, so it is simply started over. let value = { - let mut file = std::fs::OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(true) - .open(&partial.path) - .map_err(E::from)?; let value = body(&mut file)?; file.flush().map_err(E::from)?; // Before the rename, not after: once renamed, the file is the @@ -96,9 +140,10 @@ where // power cut over content that did not would be the very archive // this exists to prevent. file.sync_all().map_err(E::from)?; - value - // The handle closes here: Windows refuses to rename a file that + // Closed before the rename: Windows refuses to rename a file that // is still open. + drop(file); + value }; std::fs::rename(&partial.path, target).map_err(E::from)?; @@ -133,13 +178,30 @@ fn sync_directory(directory: &Path) -> io::Result<()> { mod tests { use super::*; + /// Partials left in `dir`, whatever their token. + fn partials(dir: &Path) -> usize { + std::fs::read_dir(dir) + .unwrap() + .flatten() + .filter(|e| e.file_name().to_string_lossy().ends_with(PARTIAL_SUFFIX)) + .count() + } + #[test] - fn the_partial_sits_beside_the_target() { - let target = Path::new("/backups/Main-2026.waveflow"); + fn the_partial_sits_beside_the_target_and_keeps_its_extension_last() { assert_eq!( - partial_path(target), - Path::new("/backups/Main-2026.waveflow.partial") + partial_path(Path::new("/backups/Main-2026.waveflow"), "7-1"), + Path::new("/backups/Main-2026.7-1.waveflow.partial") ); + assert_eq!( + partial_path(Path::new("/exports/list"), "7-2"), + Path::new("/exports/list.7-2.partial") + ); + } + + #[test] + fn every_write_gets_its_own_partial() { + assert_ne!(next_token(), next_token()); } #[test] @@ -151,7 +213,7 @@ mod tests { write_bytes(&target, b"new").unwrap(); assert_eq!(std::fs::read(&target).unwrap(), b"new"); - assert!(!partial_path(&target).exists()); + assert_eq!(partials(dir.path()), 0); } /// The case this exists for: a write that fails halfway must not @@ -169,7 +231,7 @@ mod tests { assert!(result.is_err()); assert_eq!(std::fs::read(&target).unwrap(), b"last week's archive"); - assert!(!partial_path(&target).exists()); + assert_eq!(partials(dir.path()), 0); } /// And with nothing there before, a failure leaves nothing at all @@ -186,18 +248,22 @@ mod tests { assert!(result.is_err()); assert!(!target.exists()); - assert!(!partial_path(&target).exists()); + assert_eq!(partials(dir.path()), 0); } + /// A partial someone else holds is neither truncated nor removed: it + /// may be another write in progress. #[test] - fn a_leftover_partial_is_started_over() { + fn another_partial_of_the_same_target_is_left_alone() { let dir = tempfile::tempdir().unwrap(); let target = dir.path().join("list.m3u8"); - std::fs::write(partial_path(&target), b"crashed write, much longer").unwrap(); + let other = partial_path(&target, "1-999"); + std::fs::write(&other, b"someone else's write").unwrap(); write_bytes(&target, b"fresh").unwrap(); assert_eq!(std::fs::read(&target).unwrap(), b"fresh"); + assert_eq!(std::fs::read(&other).unwrap(), b"someone else's write"); } #[test] From b1b35bba993780282eaaf52d86b71671c0d97a63 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 01:54:03 +0200 Subject: [PATCH 06/13] fix(watcher): arm a returning folder on the blocking pool --- src-tauri/crates/app/src/watcher.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src-tauri/crates/app/src/watcher.rs b/src-tauri/crates/app/src/watcher.rs index 6f087f306..421fab95e 100644 --- a/src-tauri/crates/app/src/watcher.rs +++ b/src-tauri/crates/app/src/watcher.rs @@ -294,12 +294,24 @@ async fn rearm_loop( if !present { continue; } - let (handle, scan) = match arm(&app, folder_id, library_id, &path) { - Ok(armed) => armed, - Err(err) => { + // On the blocking pool too: arming stats the folder and registers + // a recursive watch, one per directory under inotify, which on a + // large collection is not a runtime worker's job. + let armed = { + let app = app.clone(); + let path = path.clone(); + tokio::task::spawn_blocking(move || arm(&app, folder_id, library_id, &path)).await + }; + let (handle, scan) = match armed { + Ok(Ok(armed)) => armed, + Ok(Err(err)) => { tracing::debug!(folder_id, %err, "folder is back but could not be armed yet"); continue; } + Err(err) => { + tracing::warn!(folder_id, %err, "arming the folder watcher panicked"); + continue; + } }; { let mut map = inner.lock().expect("watcher map poisoned"); From 0370a691e35fc8bcc82d5b68254ebc7f3bfa61ef Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 01:54:04 +0200 Subject: [PATCH 07/13] fix(edit): save only with the stamp read alongside the form Save stays disabled until the file stamp loaded with the form's values is there, and a failed load says so. Fetching one at the click would already include the change it exists to catch. A failed reload keeps its button, and the stamp's doc names the one rewrite it cannot see. --- src-tauri/crates/core/src/tagio.rs | 6 ++++++ .../common/TrackPropertiesModal.tsx | 20 ++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src-tauri/crates/core/src/tagio.rs b/src-tauri/crates/core/src/tagio.rs index 56937ff6c..2db286b52 100644 --- a/src-tauri/crates/core/src/tagio.rs +++ b/src-tauri/crates/core/src/tagio.rs @@ -404,6 +404,12 @@ fn rename_over(from: &Path, to: &Path) -> io::Result<()> { /// Opaque on purpose: it is only ever compared for equality, and a /// string carries nanoseconds intact through JSON, where a number would /// be rounded to what a double holds. +/// +/// What it does not catch: a rewrite that keeps both the size and the +/// modification time, which a tagger has to do on purpose (restoring the +/// time after a same-size edit). Comparing content would catch it, at the +/// cost of reading the whole file twice per save; a dialog that guards +/// against an ordinary second tagger does not need that. pub fn file_stamp(path: &Path) -> io::Result<String> { let meta = std::fs::metadata(path)?; let modified = meta.modified()?; diff --git a/src/components/common/TrackPropertiesModal.tsx b/src/components/common/TrackPropertiesModal.tsx index 148f55bf5..de11a5800 100644 --- a/src/components/common/TrackPropertiesModal.tsx +++ b/src/components/common/TrackPropertiesModal.tsx @@ -138,7 +138,10 @@ export function TrackPropertiesModal({ // numbers and year back whether or not they were touched, so a file // another tagger rewrote while the dialog was open would lose that // change; with the stamp, the backend refuses the save instead. - // `null` until loaded, and then the save goes unchecked. + // `null` until loaded, and Save stays disabled until then: the stamp + // has to be the one read with the values the form shows. Taken at the + // click instead, it would already include the change it is meant to + // catch. const [fileStamp, setFileStamp] = useState<string | null>(null); // Set when a save was refused for that reason: the error then offers // to reload the values from the file instead of repeating the save. @@ -214,9 +217,11 @@ export function TrackPropertiesModal({ .then((stamp) => { if (!cancelled) setFileStamp(stamp); }) - .catch((err) => - console.error("[TrackProperties] get_track_file_stamp failed", err), - ); + .catch((err) => { + console.error("[TrackProperties] get_track_file_stamp failed", err); + // Said, not just logged: it is why Save stays disabled. + if (!cancelled) setSaveError(String(err)); + }); return () => { cancelled = true; }; @@ -370,7 +375,7 @@ export function TrackPropertiesModal({ }; const handleSave = async () => { - if (!track || saving) return; + if (!track || saving || fileStamp == null) return; setSaving(true); setSaveError(null); setFileChanged(false); @@ -502,7 +507,8 @@ export function TrackPropertiesModal({ setFileChanged(false); } catch (err) { console.error("[TrackProperties] reread_track_folder failed", err); - setFileChanged(false); + // `fileChanged` stays set: the file is still the changed one, and + // the reload button has to stay there to be tried again. setSaveError(String(err)); } finally { setSaving(false); @@ -979,7 +985,7 @@ export function TrackPropertiesModal({ <button type="button" onClick={handleSave} - disabled={saving} + disabled={saving || fileStamp == null} className="inline-flex items-center gap-2 px-4 py-2 rounded-xl text-sm font-medium bg-emerald-500 text-white hover:bg-emerald-600 transition-colors disabled:opacity-50" > <Save size={14} /> From dba91e5de060dea9a7f690f9f7776182dc3facd9 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 02:33:56 +0200 Subject: [PATCH 08/13] fix(backup): keep the destination's group and mode on a replaced file The partial was created with default rights and renamed over the target, so an archive made private came back readable by others. On Unix the partial now takes the destination's group, then its mode (owner bits only when the group cannot be matched), before a byte is written. A new destination keeps the defaults a plain create gives. --- docs/features/ui.md | 2 +- src-tauri/crates/core/src/atomic_write.rs | 41 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/docs/features/ui.md b/docs/features/ui.md index bd4ebd9f7..365f3d335 100644 --- a/docs/features/ui.md +++ b/docs/features/ui.md @@ -602,7 +602,7 @@ Per-profile isolated database (libraries, playlists, settings, play history); sh [`commands/profile_io.rs`](../../src-tauri/crates/app/src/commands/profile_io.rs) packages a profile into a single `.waveflow` (zip) file containing `manifest.json` + `data.db` + the per-profile `artwork/` directory. Settings → Storage and backups → Profile backups exposes both buttons. -- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to a partial file of its own beside the target (`<stem>.<process>-<n>.waveflow.partial`, created exclusively, so two writes to one destination never share it), flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. +- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to a partial file of its own beside the target (`<stem>.<process>-<n>.waveflow.partial`, created exclusively, so two writes to one destination never share it; on Unix it takes the destination's group and mode before a byte is written, so a private archive stays private), flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. - **Import:** always allocates a fresh profile row — never overwrites — under the archive's profile name, or `Name (2)`, `Name (3)`… when a profile already holds it (#767): `profile.name` is UNIQUE and restoring a backup of a profile that still exists is the ordinary case, so the name is taken by construction. It retries on the violation itself rather than checking first. The command returns the new id **and** that name, the success message shows the name, and Settings refreshes the profile list so the selector it points at actually lists the profile. It then extracts the archive under `profiles/<new_id>/`. Failures roll the row back so a half-imported profile doesn't survive the error. Before the sqlx migrator runs, [`normalise_migration_checksums`](../../src-tauri/crates/app/src/commands/profile_io.rs) rewrites `_sqlx_migrations.checksum` for every version present in both the archive and the local migrator — older builds checked out migration files with CRLF endings (Windows `core.autocrlf=true` + no `.gitattributes` lock) so their stored SHA-384 differs from the same SQL re-hashed today, even though the DDL is identical. A `.gitattributes` at repo root now pins `*.sql` / `*.rs` / `*.ts` / etc. to LF so future archives stay byte-stable. Once normalised, the new pool is opened once so any pending sqlx migrations replay before the user switches to it. An archive whose `_sqlx_migrations` lists a version unknown to the local migrator is rejected — that means the export came from a newer build. - **Feedback and failures:** an archive carries the artwork cache, so a large library takes minutes; the card shows an "Exporting…" / "Importing…" line while the buttons are greyed, since greyed buttons alone read as a hang. Both commands log their error at `warn` (`profile export failed` / `profile import failed`) — the UI can only show one generic sentence, and until #767 the cause was written nowhere. - **Out of scope:** the shared `app.db` (Last.fm key, Discord opt-in, `network.offline_mode`) belongs to the install, not the profile. diff --git a/src-tauri/crates/core/src/atomic_write.rs b/src-tauri/crates/core/src/atomic_write.rs index 224e7df0b..d189c5387 100644 --- a/src-tauri/crates/core/src/atomic_write.rs +++ b/src-tauri/crates/core/src/atomic_write.rs @@ -132,6 +132,30 @@ where path, committed: false, }; + // Replacing a file must not change who can read it: an archive the + // user made private would otherwise come back with default rights + // after the rename. So the destination's mode goes on the partial + // before a byte is written, and the content is never more exposed + // than the file it replaces. A new destination keeps the defaults a + // plain create would have given it. Unix only: on Windows the partial + // takes the folder's inherited ACL, as the destination did, and the + // one mode bit there (read-only) would stop the write itself. + #[cfg(unix)] + if let Ok(meta) = std::fs::metadata(target) { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + // Group bits only mean the same thing for the same group: a `640` + // archive of the "music" group, copied onto a partial owned by the + // user's own group, would let that other group read it. So the + // partial takes the destination's group first, and when that is + // refused (a group the user is not in), it keeps the owner's bits + // alone. + let same_group = std::os::unix::fs::fchown(&file, None, Some(meta.gid())).is_ok(); + // Permission and special bits only, not the file type. + let mode = meta.permissions().mode() & 0o7777; + let mode = if same_group { mode } else { mode & 0o700 }; + file.set_permissions(std::fs::Permissions::from_mode(mode)) + .map_err(E::from)?; + } let value = { let value = body(&mut file)?; file.flush().map_err(E::from)?; @@ -266,6 +290,23 @@ mod tests { assert_eq!(std::fs::read(&other).unwrap(), b"someone else's write"); } + /// A replaced file keeps the permissions it had, so a private archive + /// does not come back readable by everyone. + #[cfg(unix)] + #[test] + fn a_replaced_file_keeps_its_permissions() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("private.waveflow"); + std::fs::write(&target, b"old").unwrap(); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o600)).unwrap(); + + write_bytes(&target, b"new").unwrap(); + + let mode = std::fs::metadata(&target).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } + #[test] fn a_missing_parent_directory_is_created() { let dir = tempfile::tempdir().unwrap(); From 3ba3556e1b95b5eb4aec9f4d7b2e70afa923d00a Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 02:33:57 +0200 Subject: [PATCH 09/13] fix(edit): explain an unreadable file and offer to reload it When the file cannot be stamped (moved, deleted, unreadable), Save stayed disabled with no word as to why. The dialog now says so and offers Reload from file, which rescans it and refills the form before stamping again: a fresh stamp over the old values would let a save overwrite a change made while the file was away. --- .../common/TrackPropertiesModal.tsx | 45 +++++++++++++++++-- src/i18n/locales/ar.json | 1 + src/i18n/locales/de.json | 1 + src/i18n/locales/en.json | 1 + src/i18n/locales/es.json | 1 + src/i18n/locales/fr.json | 1 + src/i18n/locales/hi.json | 1 + src/i18n/locales/id.json | 1 + src/i18n/locales/it.json | 1 + src/i18n/locales/ja.json | 1 + src/i18n/locales/ko.json | 1 + src/i18n/locales/nl.json | 1 + src/i18n/locales/pt-BR.json | 1 + src/i18n/locales/pt.json | 1 + src/i18n/locales/ru.json | 1 + src/i18n/locales/tr.json | 1 + src/i18n/locales/zh-CN.json | 1 + src/i18n/locales/zh-TW.json | 1 + 18 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/components/common/TrackPropertiesModal.tsx b/src/components/common/TrackPropertiesModal.tsx index de11a5800..9fea12b94 100644 --- a/src/components/common/TrackPropertiesModal.tsx +++ b/src/components/common/TrackPropertiesModal.tsx @@ -143,6 +143,12 @@ export function TrackPropertiesModal({ // click instead, it would already include the change it is meant to // catch. const [fileStamp, setFileStamp] = useState<string | null>(null); + // The stamp could not be read: the file is gone, moved or unreadable. + // Save stays disabled, and the dialog says why and offers to reload. + // Reload, not just re-stamp: a file that comes back may have been + // retagged meanwhile, and a fresh stamp over the form's old values + // would let the save overwrite that change after all. + const [stampUnavailable, setStampUnavailable] = useState(false); // Set when a save was refused for that reason: the error then offers // to reload the values from the file instead of repeating the save. const [fileChanged, setFileChanged] = useState(false); @@ -215,12 +221,16 @@ export function TrackPropertiesModal({ let cancelled = false; getTrackFileStamp(track.id) .then((stamp) => { - if (!cancelled) setFileStamp(stamp); + if (cancelled) return; + setFileStamp(stamp); + setStampUnavailable(stamp == null); }) .catch((err) => { console.error("[TrackProperties] get_track_file_stamp failed", err); - // Said, not just logged: it is why Save stays disabled. - if (!cancelled) setSaveError(String(err)); + if (!cancelled) { + setFileStamp(null); + setStampUnavailable(true); + } }); return () => { cancelled = true; @@ -503,6 +513,12 @@ export function TrackPropertiesModal({ // A new `track` re-runs the hydration (which also leaves edit // mode) and the stamp fetch. if (fresh) setTrack(fresh); + // Read again here as well: when the row comes back unchanged (or + // not at all), `track` does not move, and the warning would stay + // on the old answer. + const stamp = await getTrackFileStamp(track.id).catch(() => null); + setFileStamp(stamp); + setStampUnavailable(stamp == null); setSaveError(null); setFileChanged(false); } catch (err) { @@ -944,6 +960,29 @@ export function TrackPropertiesModal({ </Section> </div> + {editing && stampUnavailable && ( + <div + role="status" + className="mx-6 mb-4 flex items-start gap-2 rounded-xl border border-amber-200 bg-amber-50 px-3 py-2 text-sm text-amber-800 dark:border-amber-900/60 dark:bg-amber-950/40 dark:text-amber-200" + > + <TriangleAlert size={16} className="mt-0.5 shrink-0" /> + <div className="min-w-0 flex-1"> + <span className="break-words"> + {t("trackProperties.stampUnavailable")} + </span> + <button + type="button" + onClick={handleReload} + disabled={saving} + className="mt-2 inline-flex items-center gap-1.5 rounded-lg border border-amber-300 bg-white px-2.5 py-1 text-xs font-medium text-amber-800 hover:bg-amber-100 disabled:opacity-50 dark:border-amber-800 dark:bg-amber-950/60 dark:text-amber-200 dark:hover:bg-amber-900/50 transition-colors" + > + <RotateCcw size={12} /> + <span>{t("trackProperties.reloadFromFile")}</span> + </button> + </div> + </div> + )} + {saveError && ( <div role="alert" diff --git a/src/i18n/locales/ar.json b/src/i18n/locales/ar.json index 50961e2cd..0a789c9e6 100644 --- a/src/i18n/locales/ar.json +++ b/src/i18n/locales/ar.json @@ -1389,6 +1389,7 @@ "saveFailed": "تعذّر الحفظ: {{reason}}", "fileChanged": "عدّل برنامج آخر الملف منذ فتح هذه النافذة. أعد تحميل قيمه قبل الحفظ حتى لا تُستبدل تلك التغييرات.", "reloadFromFile": "إعادة التحميل من الملف", + "stampUnavailable": "تتعذّر قراءة الملف على القرص: ربما نُقل أو حُذف أو أصبح غير قابل للوصول. سيبقى الحفظ معطّلًا حتى تصبح قراءته ممكنة.", "fields": { "title": "العنوان", "artist": "الفنان (الفنانون)", diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 528b95827..13d9d2076 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -1240,6 +1240,7 @@ "saveFailed": "Speichern fehlgeschlagen: {{reason}}", "fileChanged": "Die Datei wurde seit dem Öffnen dieses Fensters von einem anderen Programm geändert. Lade ihre Werte neu, bevor du speicherst, damit diese Änderungen nicht überschrieben werden.", "reloadFromFile": "Aus der Datei neu laden", + "stampUnavailable": "Die Datei kann auf dem Datenträger nicht gelesen werden: Sie wurde möglicherweise verschoben, gelöscht oder ist nicht zugänglich. Speichern bleibt deaktiviert, bis sie lesbar ist.", "fields": { "title": "Titel", "artist": "Künstler", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index f1894fcdc..99ea01ed3 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -1240,6 +1240,7 @@ "saveFailed": "Couldn't save: {{reason}}", "fileChanged": "The file was changed by another program since this window opened. Reload its values before saving, so those changes are not overwritten.", "reloadFromFile": "Reload from file", + "stampUnavailable": "The file can't be read on disk: it may have been moved, deleted or made inaccessible. Saving stays disabled until it can be read.", "fields": { "title": "Title", "artist": "Artist(s)", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index c76632b2a..451e37e43 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -1283,6 +1283,7 @@ "saveFailed": "No se pudo guardar: {{reason}}", "fileChanged": "Otro programa ha modificado el archivo desde que se abrió esta ventana. Vuelve a cargar sus valores antes de guardar para no sobrescribir esos cambios.", "reloadFromFile": "Volver a cargar desde el archivo", + "stampUnavailable": "No se puede leer el archivo en el disco: puede que se haya movido, eliminado o que no sea accesible. Guardar seguirá desactivado hasta que se pueda leer.", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 425b61a2b..fb6a0b6c7 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -1283,6 +1283,7 @@ "saveFailed": "Échec de l'enregistrement : {{reason}}", "fileChanged": "Le fichier a été modifié par un autre programme depuis l'ouverture de cette fenêtre. Rechargez ses valeurs avant d'enregistrer, pour ne pas écraser ces changements.", "reloadFromFile": "Recharger depuis le fichier", + "stampUnavailable": "Impossible de lire le fichier sur le disque : il a peut-être été déplacé, supprimé ou rendu inaccessible. L'enregistrement reste désactivé tant qu'il n'est pas lisible.", "fields": { "title": "Titre", "artist": "Artiste(s)", diff --git a/src/i18n/locales/hi.json b/src/i18n/locales/hi.json index 877f1e2a7..ab922611e 100644 --- a/src/i18n/locales/hi.json +++ b/src/i18n/locales/hi.json @@ -1240,6 +1240,7 @@ "saveFailed": "सहेजा नहीं जा सका: {{reason}}", "fileChanged": "यह विंडो खुलने के बाद किसी दूसरे प्रोग्राम ने फ़ाइल बदल दी है। उन बदलावों पर ओवरराइट न हो, इसलिए सहेजने से पहले उसके मान फिर से लोड करें।", "reloadFromFile": "फ़ाइल से फिर से लोड करें", + "stampUnavailable": "डिस्क पर फ़ाइल पढ़ी नहीं जा सकती: हो सकता है उसे हटा दिया गया हो, कहीं और ले जाया गया हो या वह पहुँच से बाहर हो। जब तक वह पढ़ी नहीं जा सकती, सहेजना बंद रहेगा।", "fields": { "title": "शीर्षक", "artist": "कलाकार", diff --git a/src/i18n/locales/id.json b/src/i18n/locales/id.json index 332d48ff9..cca1b77dc 100644 --- a/src/i18n/locales/id.json +++ b/src/i18n/locales/id.json @@ -1238,6 +1238,7 @@ "saveFailed": "Gagal menyimpan: {{reason}}", "fileChanged": "Berkas telah diubah oleh program lain sejak jendela ini dibuka. Muat ulang nilainya sebelum menyimpan agar perubahan tersebut tidak tertimpa.", "reloadFromFile": "Muat ulang dari berkas", + "stampUnavailable": "File tidak dapat dibaca di disk: mungkin telah dipindahkan, dihapus, atau tidak dapat diakses. Penyimpanan tetap nonaktif sampai file dapat dibaca.", "fields": { "title": "Judul", "artist": "Artis", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index 249904b5d..d5e1cf5c7 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -1283,6 +1283,7 @@ "saveFailed": "Salvataggio non riuscito: {{reason}}", "fileChanged": "Il file è stato modificato da un altro programma dopo l'apertura di questa finestra. Ricarica i suoi valori prima di salvare, per non sovrascrivere quelle modifiche.", "reloadFromFile": "Ricarica dal file", + "stampUnavailable": "Impossibile leggere il file su disco: potrebbe essere stato spostato, eliminato o reso inaccessibile. Il salvataggio resta disattivato finché non è leggibile.", "fields": { "title": "Titolo", "artist": "Artista/i", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 982d71358..bd52ff3f0 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -1238,6 +1238,7 @@ "saveFailed": "保存できませんでした：{{reason}}", "fileChanged": "このウィンドウを開いた後に、別のプログラムがファイルを変更しました。その変更を上書きしないよう、保存する前に値を再読み込みしてください。", "reloadFromFile": "ファイルから再読み込み", + "stampUnavailable": "ディスク上のファイルを読み込めません。移動・削除されたか、アクセスできなくなった可能性があります。読み込めるようになるまで保存は無効です。", "fields": { "title": "タイトル", "artist": "アーティスト", diff --git a/src/i18n/locales/ko.json b/src/i18n/locales/ko.json index 4d2b9a9f6..f8c1a33ac 100644 --- a/src/i18n/locales/ko.json +++ b/src/i18n/locales/ko.json @@ -1238,6 +1238,7 @@ "saveFailed": "저장하지 못했습니다: {{reason}}", "fileChanged": "이 창을 연 뒤 다른 프로그램이 파일을 변경했습니다. 그 변경 사항을 덮어쓰지 않도록 저장하기 전에 값을 다시 불러오세요.", "reloadFromFile": "파일에서 다시 불러오기", + "stampUnavailable": "디스크에서 파일을 읽을 수 없습니다. 이동되었거나 삭제되었거나 접근할 수 없게 되었을 수 있습니다. 읽을 수 있을 때까지 저장이 비활성화됩니다.", "fields": { "title": "제목", "artist": "아티스트", diff --git a/src/i18n/locales/nl.json b/src/i18n/locales/nl.json index dd04b6999..cdeb88d07 100644 --- a/src/i18n/locales/nl.json +++ b/src/i18n/locales/nl.json @@ -1240,6 +1240,7 @@ "saveFailed": "Opslaan mislukt: {{reason}}", "fileChanged": "Het bestand is door een ander programma gewijzigd sinds dit venster is geopend. Laad de waarden opnieuw voordat je opslaat, zodat die wijzigingen niet worden overschreven.", "reloadFromFile": "Opnieuw laden uit bestand", + "stampUnavailable": "Het bestand kan niet van de schijf worden gelezen: het is mogelijk verplaatst, verwijderd of ontoegankelijk gemaakt. Opslaan blijft uitgeschakeld tot het leesbaar is.", "fields": { "title": "Titel", "artist": "Artiest(en)", diff --git a/src/i18n/locales/pt-BR.json b/src/i18n/locales/pt-BR.json index ff7b6498f..792aab203 100644 --- a/src/i18n/locales/pt-BR.json +++ b/src/i18n/locales/pt-BR.json @@ -1284,6 +1284,7 @@ "saveFailed": "Não foi possível salvar: {{reason}}", "fileChanged": "O arquivo foi alterado por outro programa depois que esta janela foi aberta. Recarregue os valores antes de salvar para não sobrescrever essas alterações.", "reloadFromFile": "Recarregar do arquivo", + "stampUnavailable": "Não é possível ler o arquivo no disco: ele pode ter sido movido, excluído ou ficado inacessível. Salvar fica desativado até que ele possa ser lido.", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/pt.json b/src/i18n/locales/pt.json index b3ee79df2..aaf9980c0 100644 --- a/src/i18n/locales/pt.json +++ b/src/i18n/locales/pt.json @@ -1284,6 +1284,7 @@ "saveFailed": "Não foi possível guardar: {{reason}}", "fileChanged": "O ficheiro foi alterado por outro programa desde que esta janela foi aberta. Recarregue os valores antes de guardar, para não substituir essas alterações.", "reloadFromFile": "Recarregar do ficheiro", + "stampUnavailable": "Não é possível ler o ficheiro no disco: pode ter sido movido, eliminado ou tornado inacessível. Guardar fica desativado até que possa ser lido.", "fields": { "title": "Título", "artist": "Artista(s)", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index 7e59ffcf6..d8aecc927 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -1330,6 +1330,7 @@ "saveFailed": "Не удалось сохранить: {{reason}}", "fileChanged": "После открытия этого окна файл был изменён другой программой. Перезагрузите значения перед сохранением, чтобы не затереть эти изменения.", "reloadFromFile": "Перезагрузить из файла", + "stampUnavailable": "Не удаётся прочитать файл на диске: возможно, он перемещён, удалён или недоступен. Сохранение отключено, пока файл не станет доступен для чтения.", "fields": { "title": "Название", "artist": "Исполнитель(и)", diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index 8b9894ea4..af1b55e26 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -1240,6 +1240,7 @@ "saveFailed": "Kaydedilemedi: {{reason}}", "fileChanged": "Bu pencere açıldıktan sonra dosya başka bir program tarafından değiştirildi. Bu değişikliklerin üzerine yazmamak için kaydetmeden önce değerleri yeniden yükleyin.", "reloadFromFile": "Dosyadan yeniden yükle", + "stampUnavailable": "Dosya diskte okunamıyor: taşınmış, silinmiş ya da erişilemez hâle gelmiş olabilir. Okunabilir olana kadar kaydetme devre dışı kalır.", "fields": { "title": "Başlık", "artist": "Sanatçı(lar)", diff --git a/src/i18n/locales/zh-CN.json b/src/i18n/locales/zh-CN.json index b769d3337..dc4b60d73 100644 --- a/src/i18n/locales/zh-CN.json +++ b/src/i18n/locales/zh-CN.json @@ -1238,6 +1238,7 @@ "saveFailed": "保存失败：{{reason}}", "fileChanged": "打开此窗口后，文件已被其他程序修改。请在保存前重新加载其值，以免覆盖这些更改。", "reloadFromFile": "从文件重新加载", + "stampUnavailable": "无法读取磁盘上的文件：它可能已被移动、删除或无法访问。在文件可读之前，保存将保持禁用。", "fields": { "title": "标题", "artist": "艺人", diff --git a/src/i18n/locales/zh-TW.json b/src/i18n/locales/zh-TW.json index cbade6be8..7cacec1c6 100644 --- a/src/i18n/locales/zh-TW.json +++ b/src/i18n/locales/zh-TW.json @@ -1238,6 +1238,7 @@ "saveFailed": "儲存失敗：{{reason}}", "fileChanged": "開啟此視窗後，檔案已被其他程式修改。請在儲存前重新載入其值，以免覆寫這些變更。", "reloadFromFile": "從檔案重新載入", + "stampUnavailable": "無法讀取磁碟上的檔案：它可能已被移動、刪除或無法存取。在檔案可讀取之前，儲存將維持停用。", "fields": { "title": "標題", "artist": "藝人", From 0665ec5b09209633ef8a8315337efcf4737011b9 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 02:57:34 +0200 Subject: [PATCH 10/13] fix(backup): replace an existing file with ReplaceFileW on Windows A rename over the destination left the partial's inherited ACL in place of the one the user set, where rewriting in place had kept it. ReplaceFileW keeps the ACL and attributes, and fails rather than replacing without them. The replaced file goes to a backup name of ours during the swap, so the one failure that moves it away without putting the new file in place can move it back, or name where it is. --- docs/features/ui.md | 2 +- src-tauri/Cargo.lock | 1 + src-tauri/crates/core/Cargo.toml | 6 ++ src-tauri/crates/core/src/atomic_write.rs | 80 ++++++++++++++++++++++- 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/docs/features/ui.md b/docs/features/ui.md index 365f3d335..3edecf0af 100644 --- a/docs/features/ui.md +++ b/docs/features/ui.md @@ -602,7 +602,7 @@ Per-profile isolated database (libraries, playlists, settings, play history); sh [`commands/profile_io.rs`](../../src-tauri/crates/app/src/commands/profile_io.rs) packages a profile into a single `.waveflow` (zip) file containing `manifest.json` + `data.db` + the per-profile `artwork/` directory. Settings → Storage and backups → Profile backups exposes both buttons. -- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to a partial file of its own beside the target (`<stem>.<process>-<n>.waveflow.partial`, created exclusively, so two writes to one destination never share it; on Unix it takes the destination's group and mode before a byte is written, so a private archive stays private), flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. +- **Export:** the active-profile path runs `PRAGMA wal_checkpoint(TRUNCATE)` first so the bundled DB captures every committed page (otherwise a busy WAL would leave the archive holding a partial snapshot). The CPU-bound zip work runs on `tokio::task::spawn_blocking`. The archive is written to a partial file of its own beside the target (`<stem>.<process>-<n>.waveflow.partial`, created exclusively, so two writes to one destination never share it; on Unix it takes the destination's group and mode before a byte is written, and on Windows an existing destination is replaced with `ReplaceFileW`, which keeps its ACL and attributes, so a private archive stays private), flushed to disk and renamed over the destination only once complete ([`atomic_write`](../../src-tauri/crates/core/src/atomic_write.rs)), so an export that fails halfway — full disk, a share that drops — leaves an existing archive at that path as it was instead of truncated, and leaves no archive-shaped file behind when there was none. The automatic backup and the pre-reset safety copy share the same writer. - **Import:** always allocates a fresh profile row — never overwrites — under the archive's profile name, or `Name (2)`, `Name (3)`… when a profile already holds it (#767): `profile.name` is UNIQUE and restoring a backup of a profile that still exists is the ordinary case, so the name is taken by construction. It retries on the violation itself rather than checking first. The command returns the new id **and** that name, the success message shows the name, and Settings refreshes the profile list so the selector it points at actually lists the profile. It then extracts the archive under `profiles/<new_id>/`. Failures roll the row back so a half-imported profile doesn't survive the error. Before the sqlx migrator runs, [`normalise_migration_checksums`](../../src-tauri/crates/app/src/commands/profile_io.rs) rewrites `_sqlx_migrations.checksum` for every version present in both the archive and the local migrator — older builds checked out migration files with CRLF endings (Windows `core.autocrlf=true` + no `.gitattributes` lock) so their stored SHA-384 differs from the same SQL re-hashed today, even though the DDL is identical. A `.gitattributes` at repo root now pins `*.sql` / `*.rs` / `*.ts` / etc. to LF so future archives stay byte-stable. Once normalised, the new pool is opened once so any pending sqlx migrations replay before the user switches to it. An archive whose `_sqlx_migrations` lists a version unknown to the local migrator is rejected — that means the export came from a newer build. - **Feedback and failures:** an archive carries the artwork cache, so a large library takes minutes; the card shows an "Exporting…" / "Importing…" line while the buttons are greyed, since greyed buttons alone read as a hang. Both commands log their error at `warn` (`profile export failed` / `profile import failed`) — the UI can only show one generic sentence, and until #767 the cause was written nowhere. - **Out of scope:** the shared `app.db` (Last.fm key, Discord opt-in, `network.offline_mode`) belongs to the install, not the profile. diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 57ad782ed..1fac24423 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -7858,6 +7858,7 @@ dependencies = [ "wasmtime", "wasmtime-wasi", "waveflow-plugin-sdk", + "windows-sys 0.61.2", ] [[package]] diff --git a/src-tauri/crates/core/Cargo.toml b/src-tauri/crates/core/Cargo.toml index c5c9d1ee4..12210ee0f 100644 --- a/src-tauri/crates/core/Cargo.toml +++ b/src-tauri/crates/core/Cargo.toml @@ -223,6 +223,12 @@ wasmtime-wasi = { version = "49", default-features = false, optional = true } pinyin = { version = "0.11.0", default-features = false, features = ["plain"] } unicode-normalization = { version = "0.1.25", optional = true } +# `ReplaceFileW`, so an atomic write keeps the replaced file's ACL the way +# rewriting it in place did (`atomic_write`). Already in the lockfile +# through other crates. +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Storage_FileSystem"] } + [dev-dependencies] # Sandboxed scratch directories for the smart-playlist cover render # tests. Same usage shape as the app crate's existing dev-dep. diff --git a/src-tauri/crates/core/src/atomic_write.rs b/src-tauri/crates/core/src/atomic_write.rs index d189c5387..7e55249f4 100644 --- a/src-tauri/crates/core/src/atomic_write.rs +++ b/src-tauri/crates/core/src/atomic_write.rs @@ -170,7 +170,7 @@ where value }; - std::fs::rename(&partial.path, target).map_err(E::from)?; + replace(&partial.path, target).map_err(E::from)?; partial.committed = true; // Durability on top of a replacement that has already happened, so // its failure is not reported as a failed write. @@ -178,6 +178,84 @@ where Ok(value) } +/// Move the finished partial over `target`. +/// +/// On Windows an existing destination is replaced with `ReplaceFileW`, +/// which keeps its ACL and attributes the way rewriting it in place did. +/// A plain rename would leave the partial's inherited ACL in their place, +/// and an explicit one the user set would be gone. When they cannot be +/// kept, the write fails rather than replacing the file without them. A +/// destination that does not exist is renamed into place. +/// +/// The replaced file goes to a backup name of ours during the swap. On +/// one failure, `ERROR_UNABLE_TO_MOVE_REPLACEMENT_2`, Windows has already +/// moved it there without putting the new file in its place: it is moved +/// back, and if even that fails the error says where it is, so neither +/// copy is lost to the cleanup. The backup is a partial by name, so a +/// leftover is swept like one. +#[cfg(windows)] +fn replace(partial: &Path, target: &Path) -> io::Result<()> { + use std::os::windows::ffi::OsStrExt; + use windows_sys::Win32::Foundation::{ + ERROR_FILE_NOT_FOUND, ERROR_UNABLE_TO_MOVE_REPLACEMENT_2, + }; + use windows_sys::Win32::Storage::FileSystem::ReplaceFileW; + + if std::fs::symlink_metadata(target).is_err() { + return std::fs::rename(partial, target); + } + let wide = |path: &Path| -> Vec<u16> { + path.as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect() + }; + let backup = partial_path(target, &format!("{}-replaced", next_token())); + let replaced = wide(target); + let replacement = wide(partial); + let backup_name = wide(&backup); + // SAFETY: the three paths are NUL-terminated UTF-16 buffers that + // outlive the call; the two reserved pointers are null and the flags + // 0, as the API documents. + let ok = unsafe { + ReplaceFileW( + replaced.as_ptr(), + replacement.as_ptr(), + backup_name.as_ptr(), + 0, + std::ptr::null(), + std::ptr::null(), + ) + }; + if ok != 0 { + let _ = std::fs::remove_file(&backup); + return Ok(()); + } + let err = io::Error::last_os_error(); + match err.raw_os_error().map(|code| code as u32) { + // The destination went away between the check and the call. + Some(ERROR_FILE_NOT_FOUND) => std::fs::rename(partial, target), + // The old file is at `backup` and the new one is not in place. + Some(ERROR_UNABLE_TO_MOVE_REPLACEMENT_2) => match std::fs::rename(&backup, target) { + Ok(()) => Err(err), + Err(_) => Err(io::Error::other(format!( + "{err}; the previous file is at {}", + backup.display() + ))), + }, + // Nothing was moved; a backup is only made once the swap starts. + _ => { + let _ = std::fs::remove_file(&backup); + Err(err) + } + } +} + +#[cfg(not(windows))] +fn replace(partial: &Path, target: &Path) -> io::Result<()> { + std::fs::rename(partial, target) +} + /// [`write_with`] for content already in memory. pub fn write_bytes(target: &Path, bytes: &[u8]) -> io::Result<()> { write_with(target, |file| file.write_all(bytes)) From a0fd659837c16930c0661ecc37d875854d6e7684 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 02:57:35 +0200 Subject: [PATCH 11/13] fix(edit): keep a reload's stamp only when the file held still The reload stamps the file before and after rereading it, before it applies anything. When the two differ, or either is missing, nothing is kept and Save stays off until a reload finds the file still. The stamp it validated is handed to the refresh it triggers rather than read again, so a write landing in between cannot be stamped over older values. --- .../common/TrackPropertiesModal.tsx | 45 +++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/components/common/TrackPropertiesModal.tsx b/src/components/common/TrackPropertiesModal.tsx index 9fea12b94..1690c8268 100644 --- a/src/components/common/TrackPropertiesModal.tsx +++ b/src/components/common/TrackPropertiesModal.tsx @@ -149,6 +149,11 @@ export function TrackPropertiesModal({ // retagged meanwhile, and a fresh stamp over the form's old values // would let the save overwrite that change after all. const [stampUnavailable, setStampUnavailable] = useState(false); + // The stamp a reload validated against the values it put in the form. + // The reload changes `track`, which re-runs the stamp fetch below; that + // fetch must hand this one back rather than read the file again, or a + // write landing in between would be stamped over the older values. + const reloadedStampRef = useRef<string | null>(null); // Set when a save was refused for that reason: the error then offers // to reload the values from the file instead of repeating the save. const [fileChanged, setFileChanged] = useState(false); @@ -218,6 +223,13 @@ export function TrackPropertiesModal({ // with them or the next save would be refused by our own change. useEffect(() => { if (!track) return; + const reloaded = reloadedStampRef.current; + if (reloaded != null) { + reloadedStampRef.current = null; + setFileStamp(reloaded); + setStampUnavailable(false); + return; + } let cancelled = false; getTrackFileStamp(track.id) .then((stamp) => { @@ -491,12 +503,30 @@ export function TrackPropertiesModal({ if (!track || saving) return; setSaving(true); try { + // Stamped on both sides of the reread, and the second time before + // anything is applied: a write landing while it runs would leave the + // form with values older than the file, and once `track` changes, + // its effect fetches a fresh stamp that would let Save overwrite + // that write. When the two differ, nothing of this reload is kept. + const before = await getTrackFileStamp(track.id).catch(() => null); await rereadTrackFolder(track.id); const [fresh, genreRows, artistRows] = await Promise.all([ getTrack(track.id), getTrackGenres(track.id), getTrackArtists(track.id), ]); + const after = await getTrackFileStamp(track.id).catch(() => null); + setStampUnavailable(after == null); + if (before == null || after == null || before !== after) { + // Moved during the reload, or unreadable: no stamp, so no save, + // until a reload that finds the file still. + setFileStamp(null); + if (after != null) { + setFileChanged(true); + setSaveError(t("trackProperties.fileChanged")); + } + return; + } setGenres(genreRows); setGenreTouched(false); setArtists(artistRows.length > 0 ? artistRows : null); @@ -511,14 +541,13 @@ export function TrackPropertiesModal({ : (fresh?.artist_name ?? ""), })); // A new `track` re-runs the hydration (which also leaves edit - // mode) and the stamp fetch. - if (fresh) setTrack(fresh); - // Read again here as well: when the row comes back unchanged (or - // not at all), `track` does not move, and the warning would stay - // on the old answer. - const stamp = await getTrackFileStamp(track.id).catch(() => null); - setFileStamp(stamp); - setStampUnavailable(stamp == null); + // mode) and the stamp fetch; the stamp is set here as well, for a + // row that comes back unchanged and so does not move `track`. + if (fresh) { + reloadedStampRef.current = after; + setTrack(fresh); + } + setFileStamp(after); setSaveError(null); setFileChanged(false); } catch (err) { From d156ce89bc0e05ceb72e2704bfcc56fb3d3647b5 Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 03:24:12 +0200 Subject: [PATCH 12/13] fix(backup): give a replace's backup a name the sweep leaves alone The file a Windows replace moves aside was named like a partial, so if a failed swap left it there, the backup sweep would delete it a day later, though it could be the only copy of the old archive. It now ends in .replaced, which nothing sweeps. --- src-tauri/crates/core/src/atomic_write.rs | 54 ++++++++++++++++++----- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src-tauri/crates/core/src/atomic_write.rs b/src-tauri/crates/core/src/atomic_write.rs index 7e55249f4..372239890 100644 --- a/src-tauri/crates/core/src/atomic_write.rs +++ b/src-tauri/crates/core/src/atomic_write.rs @@ -40,6 +40,20 @@ pub const PARTIAL_SUFFIX: &str = ".partial"; /// atomic within a filesystem, and an export folder is routinely on a /// different one than `/tmp`. pub fn partial_path(target: &Path, token: &str) -> PathBuf { + sibling_path(target, token, PARTIAL_SUFFIX) +} + +/// Where the file a Windows replace moves out of the way goes for the +/// length of the swap: like [`partial_path`], but with this suffix, which +/// nothing sweeps. Should the swap fail in a way that leaves the old file +/// there, it may be the only copy of it, and it must not be taken for an +/// unfinished write and deleted a day later. +#[cfg(any(windows, test))] +pub const REPLACED_SUFFIX: &str = ".replaced"; + +/// `<stem>.<token>.<extension><suffix>`, or `<name>.<token><suffix>` for +/// a name without an extension, in the target's directory. +fn sibling_path(target: &Path, token: &str, suffix: &str) -> PathBuf { let mut name = OsString::new(); match (target.file_stem(), target.extension()) { (Some(stem), Some(extension)) => { @@ -55,7 +69,7 @@ pub fn partial_path(target: &Path, token: &str) -> PathBuf { name.push(token); } } - name.push(PARTIAL_SUFFIX); + name.push(suffix); target.with_file_name(name) } @@ -187,12 +201,13 @@ where /// kept, the write fails rather than replacing the file without them. A /// destination that does not exist is renamed into place. /// -/// The replaced file goes to a backup name of ours during the swap. On -/// one failure, `ERROR_UNABLE_TO_MOVE_REPLACEMENT_2`, Windows has already -/// moved it there without putting the new file in its place: it is moved -/// back, and if even that fails the error says where it is, so neither -/// copy is lost to the cleanup. The backup is a partial by name, so a -/// leftover is swept like one. +/// The replaced file goes to a backup name of ours during the swap +/// ([`REPLACED_SUFFIX`]). On one failure, +/// `ERROR_UNABLE_TO_MOVE_REPLACEMENT_2`, Windows has already moved it +/// there without putting the new file in its place: it is moved back, and +/// if even that fails the error says where it is, so neither copy is lost +/// to the cleanup. Nothing sweeps that name, since it may then hold the +/// only copy of the old file. #[cfg(windows)] fn replace(partial: &Path, target: &Path) -> io::Result<()> { use std::os::windows::ffi::OsStrExt; @@ -210,7 +225,7 @@ fn replace(partial: &Path, target: &Path) -> io::Result<()> { .chain(std::iter::once(0)) .collect() }; - let backup = partial_path(target, &format!("{}-replaced", next_token())); + let backup = sibling_path(target, &next_token(), REPLACED_SUFFIX); let replaced = wide(target); let replacement = wide(partial); let backup_name = wide(&backup); @@ -280,12 +295,15 @@ fn sync_directory(directory: &Path) -> io::Result<()> { mod tests { use super::*; - /// Partials left in `dir`, whatever their token. + /// Partials, and a Windows replace's backups, left in `dir`. fn partials(dir: &Path) -> usize { std::fs::read_dir(dir) .unwrap() .flatten() - .filter(|e| e.file_name().to_string_lossy().ends_with(PARTIAL_SUFFIX)) + .filter(|e| { + let name = e.file_name().to_string_lossy().into_owned(); + name.ends_with(PARTIAL_SUFFIX) || name.ends_with(REPLACED_SUFFIX) + }) .count() } @@ -301,6 +319,22 @@ mod tests { ); } + /// A Windows replace's backup must not be swept as an unfinished + /// write: it can hold the only copy of the old file. + #[test] + fn the_replaced_backup_is_not_a_partial() { + let backup = sibling_path( + Path::new("/backups/Main-2026.waveflow"), + "7-3", + REPLACED_SUFFIX, + ); + assert_eq!( + backup, + Path::new("/backups/Main-2026.7-3.waveflow.replaced") + ); + assert!(!backup.to_string_lossy().ends_with(PARTIAL_SUFFIX)); + } + #[test] fn every_write_gets_its_own_partial() { assert_ne!(next_token(), next_token()); From e20fd86a71bface7af68751b0748c5eec98602dc Mon Sep 17 00:00:00 2001 From: InstaZDLL <github.105mh@8shield.net> Date: Mon, 19 Oct 2026 03:24:12 +0200 Subject: [PATCH 13/13] fix(edit): keep save blocked after a conflict until a clean reload Once the file is known to differ from the form, no stamp is restored, not by a fetch already in flight nor by one a later track:updated starts, and a cover change keeps the notice and its Reload button. Only a reload whose two stamps agree lifts it. --- .../common/TrackPropertiesModal.tsx | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/components/common/TrackPropertiesModal.tsx b/src/components/common/TrackPropertiesModal.tsx index 1690c8268..d5570bae5 100644 --- a/src/components/common/TrackPropertiesModal.tsx +++ b/src/components/common/TrackPropertiesModal.tsx @@ -154,6 +154,12 @@ export function TrackPropertiesModal({ // fetch must hand this one back rather than read the file again, or a // write landing in between would be stamped over the older values. const reloadedStampRef = useRef<string | null>(null); + // Set once the file is known to differ from the form (a save refused, + // or a reload that saw the file move). Until a reload whose two stamps + // agree clears it, no stamp is restored, not by a fetch already in + // flight nor by one a later `track:updated` starts, since those would + // re-enable Save over the stale values. + const staleRef = useRef(false); // Set when a save was refused for that reason: the error then offers // to reload the values from the file instead of repeating the save. const [fileChanged, setFileChanged] = useState(false); @@ -234,8 +240,8 @@ export function TrackPropertiesModal({ getTrackFileStamp(track.id) .then((stamp) => { if (cancelled) return; - setFileStamp(stamp); setStampUnavailable(stamp == null); + setFileStamp(staleRef.current ? null : stamp); }) .catch((err) => { console.error("[TrackProperties] get_track_file_stamp failed", err); @@ -486,7 +492,12 @@ export function TrackPropertiesModal({ // surrounding views listen to and react to (re-fetch the row). } catch (err) { console.error("[TrackProperties] update_track_tags failed", err); - setFileChanged(isFileChangedOnDisk(err)); + const changed = isFileChangedOnDisk(err); + setFileChanged(changed); + if (changed) { + staleRef.current = true; + setFileStamp(null); + } setSaveError(String(err)); } finally { setSaving(false); @@ -520,6 +531,7 @@ export function TrackPropertiesModal({ if (before == null || after == null || before !== after) { // Moved during the reload, or unreadable: no stamp, so no save, // until a reload that finds the file still. + staleRef.current = true; setFileStamp(null); if (after != null) { setFileChanged(true); @@ -527,6 +539,7 @@ export function TrackPropertiesModal({ } return; } + staleRef.current = false; setGenres(genreRows); setGenreTouched(false); setArtists(artistRows.length > 0 ? artistRows : null); @@ -569,8 +582,12 @@ export function TrackPropertiesModal({ ); if (!path) return; setSaving(true); - setSaveError(null); - setFileChanged(false); + // While the form is known to be stale, its notice and Reload stay: + // they are the only way back to a Save that works. + if (!staleRef.current) { + setSaveError(null); + setFileChanged(false); + } await updateTrackCover(track.id, path); // The backend's `track:updated` event triggers a refetch in the // surrounding views; the modal itself stays open on the same