diff --git a/design/mockups/drop-safety-review/01-minimal-light.html b/design/mockups/drop-safety-review/01-minimal-light.html new file mode 100644 index 0000000..63063bf --- /dev/null +++ b/design/mockups/drop-safety-review/01-minimal-light.html @@ -0,0 +1,30 @@ + + +Drop safety review - Minimal light + +
+

Drop safety state

Conflict-aware copy and move operations keep Undo honest when a destination already existed.

+
Documentscopy target+
+
Imagessort by rule
+
DW
+
Archivemove target
+
Scriptsopen with
+
Could not undo completelyreport.txt existed before this drop; the original file was left protected.
+
diff --git a/design/mockups/drop-safety-review/02-dark-pro.html b/design/mockups/drop-safety-review/02-dark-pro.html new file mode 100644 index 0000000..a30d24a --- /dev/null +++ b/design/mockups/drop-safety-review/02-dark-pro.html @@ -0,0 +1,35 @@ + + +Drop safety review - Dark pro + +
+
+

Safety review

+
DropMove
+
DestinationDownloads
+
Conflict1 existing
+
Undo resultPartial
+
+
Imagesrule target
+
Downloads activereport.txt already exists
+
DW
+
Archivemove target
+
Scriptsopen with
+
Undo needs attentionDropwheel protected the pre-existing destination and rolled back only tracked files.
+
diff --git a/design/mockups/drop-safety-review/03-high-contrast-a11y.html b/design/mockups/drop-safety-review/03-high-contrast-a11y.html new file mode 100644 index 0000000..848226a --- /dev/null +++ b/design/mockups/drop-safety-review/03-high-contrast-a11y.html @@ -0,0 +1,27 @@ + + +Drop safety review - High contrast + +
+

Incomplete undo

The status is explicit when a destination file existed before the drop and cannot be safely removed.

+
DocumentsCopy target
+
ArchiveMove target
+
DW
+
SorterRules active
+
EditorOpen with
+
Could not undo completelyreport.txt was present before the drop. Dropwheel did not delete that file.
+
diff --git a/design/mockups/drop-safety-review/04-playful-rounded.html b/design/mockups/drop-safety-review/04-playful-rounded.html new file mode 100644 index 0000000..9752963 --- /dev/null +++ b/design/mockups/drop-safety-review/04-playful-rounded.html @@ -0,0 +1,29 @@ + + +Drop safety review - Soft rounded + +
+

Drop with confidence

Undo copy/move keeps track of files that existed before the drop and marks the rollback as partial.

+
Picturessort photos
+
Downloadscopy here+
+
DW
+
Archivemove here
+
Editoropen with
+
Could not undo completelyOne existing destination was protected.
+
diff --git a/design/mockups/drop-safety-review/APPROVED b/design/mockups/drop-safety-review/APPROVED new file mode 100644 index 0000000..0a017b6 --- /dev/null +++ b/design/mockups/drop-safety-review/APPROVED @@ -0,0 +1 @@ +2026-07-09 fix/product-safety-review approved 01-minimal-light.html for drop safety and incomplete undo states diff --git a/design/mockups/drop-safety-review/NOTES.md b/design/mockups/drop-safety-review/NOTES.md new file mode 100644 index 0000000..7fbdf27 --- /dev/null +++ b/design/mockups/drop-safety-review/NOTES.md @@ -0,0 +1,12 @@ +# Mockups - drop-safety-review + +DESIGN stage for the GUI feature "drop-safety-review". Rule: >=4 stylistically distinct +mockups plus approval before GUI implementation. + +## Variants +- `01-minimal-light.html` - restrained overlay with an incomplete undo toast. +- `02-dark-pro.html` - operational side panel showing destination conflict state. +- `03-high-contrast-a11y.html` - high-contrast rollback status. +- `04-playful-rounded.html` - softer bubble layout with partial undo notification. + +Approved direction: `01-minimal-light.html`. diff --git a/src/Dropwheel/Services/FileOps.cs b/src/Dropwheel/Services/FileOps.cs index 17f388b..102f54e 100644 --- a/src/Dropwheel/Services/FileOps.cs +++ b/src/Dropwheel/Services/FileOps.cs @@ -28,9 +28,17 @@ private struct SHFILEOPSTRUCT [DllImport("shell32.dll", CharSet = CharSet.Unicode)] private static extern int SHFileOperation(ref SHFILEOPSTRUCT op); + public static string[] DestinationConflicts(IEnumerable files, string destFolder) + { + return files + .Select(f => Path.Combine(destFolder, Path.GetFileName(f))) + .Where(p => File.Exists(p) || Directory.Exists(p)) + .ToArray(); + } + /// Copy or move files into destFolder. When silent (used by the folder watcher for - /// auto-sort) the shell shows no progress window, no error UI and no conflict prompt — collisions - /// are auto-renamed — so background sorting never interrupts the user with dialogs. + /// auto-sort) the shell shows no progress window, no error UI and no conflict prompt. Callers + /// that need no-overwrite behavior must preflight with DestinationConflicts first. public static bool Execute(IEnumerable files, string destFolder, DropAction action, bool silent = false) { var list = files.ToArray(); diff --git a/src/Dropwheel/Services/TargetStore.cs b/src/Dropwheel/Services/TargetStore.cs index 47e8eb8..f8fe923 100644 --- a/src/Dropwheel/Services/TargetStore.cs +++ b/src/Dropwheel/Services/TargetStore.cs @@ -8,6 +8,7 @@ namespace Dropwheel.Services; public static class TargetStore { public static AppConfig Config { get; private set; } = new(); + internal static string? DirOverride { get; set; } /// Raised after the config is written to disk. The folder watcher listens to this to /// re-sync its FileSystemWatchers when targets or their Watch flag change. @@ -18,7 +19,7 @@ public static class TargetStore public static IEnumerable AllTargets => Config.Targets.SelectMany(t => t.IsGroup ? (IEnumerable)t.Children! : new[] { t }); - public static string Dir => Path.Combine( + public static string Dir => DirOverride ?? Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "Dropwheel"); public static string FilePath => Path.Combine(Dir, "config.json"); @@ -45,7 +46,12 @@ public static void Load() catch (IOException ex) { ErrorLog.Write("Config is unreadable; backing it up and recreating defaults", ex); shouldBackup = true; } catch (UnauthorizedAccessException ex) { ErrorLog.Write("Config is unreadable; backing it up and recreating defaults", ex); shouldBackup = true; } } - if (shouldBackup) BackupBadConfig(DateTime.Now); + if (shouldBackup && !BackupBadConfig(DateTime.Now)) + { + Config = Defaults(); + ErrorLog.Write("Settings file could not be backed up; using defaults in memory without overwriting it."); + return; + } Config = Defaults(); Save(); } @@ -56,17 +62,22 @@ internal static string BackupPath(DateTime now) return Path.Combine(Dir, $"config.bad.{stamp}.json"); } - private static void BackupBadConfig(DateTime now) + private static bool BackupBadConfig(DateTime now) { try { - if (!File.Exists(FilePath)) return; + if (!File.Exists(FilePath)) return true; var backup = BackupPath(now); for (int i = 2; File.Exists(backup); i++) backup = Path.Combine(Dir, $"config.bad.{now:yyyyMMdd_HHmmss}.{i}.json"); File.Copy(FilePath, backup); + return true; + } + catch (Exception ex) + { + ErrorLog.Write("Failed to back up bad config", ex); + return false; } - catch (Exception ex) { ErrorLog.Write("Failed to back up bad config", ex); } } /// Writes via a temp file then renames it: if the process is killed mid-write, the diff --git a/src/Dropwheel/Services/WatcherService.cs b/src/Dropwheel/Services/WatcherService.cs index 75e9c40..013b299 100644 --- a/src/Dropwheel/Services/WatcherService.cs +++ b/src/Dropwheel/Services/WatcherService.cs @@ -179,6 +179,12 @@ private void SortOne(Entry entry, string file) // Create the destination folder first: otherwise SHFileOperation moving a single file // to a non-existent path treats the last segment as a new file name, not a folder. Directory.CreateDirectory(folder); + var conflicts = FileOps.DestinationConflicts(files, folder); + if (conflicts.Length > 0) + { + ErrorLog.Write($"Auto-sort skipped '{file}' because destination already exists: '{conflicts[0]}'"); + continue; + } if (FileOps.Execute(files, folder, DropAction.Move, silent: true)) _ui.InvokeAsync(() => QueueToast(files.Count)); // coalesce the toast on the UI thread else diff --git a/src/Dropwheel/UI/OverlayWindow.Dnd.cs b/src/Dropwheel/UI/OverlayWindow.Dnd.cs index b00335c..02f7d92 100644 --- a/src/Dropwheel/UI/OverlayWindow.Dnd.cs +++ b/src/Dropwheel/UI/OverlayWindow.Dnd.cs @@ -77,9 +77,9 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) return; } var act = Resolve(t, e); - bool hadCollision = FileOps.HasDestinationCollision(files, dest); + var op = BuildOpBefore(act, files, dest); bool ok = FileOps.Execute(files, dest, act); - if (ok) RememberOpIfUnambiguous(act, files, dest, hadCollision); + if (ok) RememberOp(op); ShowToast(ok ? $"{(act == DropAction.Move ? "➜ Moved" : "⧉ Copied")}: {files.Length} item(s) → {t.Name}" : "Operation was not completed", ok); @@ -90,7 +90,7 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) if (saved.Length > 0) { if (t.IsSorter) SortSavedVirtuals(t, saved); - else RememberOpIfUnambiguous(DropAction.Copy, saved, dest, hadCollision: false); + else RememberOp(BuildCreatedCopyOp(saved, dest)); } ShowToast(saved.Length > 0 ? $"⧉ Saved: {saved.Length} item(s) → {t.Name}" @@ -102,7 +102,7 @@ private void OnBubbleDropCore(TargetItem t, DragEventArgs e) if (saved is { } path) { if (t.IsSorter) SortSavedVirtuals(t, new[] { path }); - else RememberOpIfUnambiguous(DropAction.Copy, new[] { path }, dest, hadCollision: false); + else RememberOp(BuildCreatedCopyOp(new[] { path }, dest)); } ShowToast(saved != null ? $"≡ Saved text → {System.IO.Path.GetFileName(saved)}" diff --git a/src/Dropwheel/UI/OverlayWindow.Sort.cs b/src/Dropwheel/UI/OverlayWindow.Sort.cs index 722e6d7..5064785 100644 --- a/src/Dropwheel/UI/OverlayWindow.Sort.cs +++ b/src/Dropwheel/UI/OverlayWindow.Sort.cs @@ -12,15 +12,16 @@ private void DropSorted(TargetItem t, string[] files, DropAction act) { var plan = SortService.Plan(t, files); bool ok = true; - var ops = new List<(DropAction, string[], string, bool)>(); + var ops = new List(); foreach (var (folder, group) in plan) { Directory.CreateDirectory(folder); - bool hadCollision = FileOps.HasDestinationCollision(group, folder); - if (FileOps.Execute(group, folder, act)) ops.Add((act, group.ToArray(), folder, hadCollision)); + var sources = group.ToArray(); + var op = BuildOpBefore(act, sources, folder); + if (FileOps.Execute(sources, folder, act)) ops.Add(op); else ok = false; } - if (ops.Count > 0) RememberOpsIfUnambiguous(ops); + if (ops.Count > 0) RememberOps(ops); ShowToast(ok ? $"⇅ Sorted: {files.Length} item(s) → {t.Name}" : "Sorting was not completed", ops.Count > 0); @@ -31,17 +32,17 @@ private void DropSorted(TargetItem t, string[] files, DropAction act) private void SortSavedVirtuals(TargetItem t, string[] saved) { var plan = SortService.Plan(t, saved); - var ops = new List<(DropAction, string[], string, bool)>(); + var ops = new List(); string root = IOPath.GetFullPath(t.Path).TrimEnd('\\'); foreach (var (folder, group) in plan) { if (IOPath.GetFullPath(folder).TrimEnd('\\') == root) - { ops.Add((DropAction.Copy, group.ToArray(), folder, false)); continue; } + { ops.Add(BuildCreatedCopyOp(group.ToArray(), folder)); continue; } Directory.CreateDirectory(folder); - bool hadCollision = FileOps.HasDestinationCollision(group, folder); - if (FileOps.Execute(group, folder, DropAction.Move)) - ops.Add((DropAction.Copy, group.ToArray(), folder, hadCollision)); + var sources = group.ToArray(); + if (FileOps.Execute(sources, folder, DropAction.Move)) + ops.Add(BuildCreatedCopyOp(sources, folder)); } - if (ops.Count > 0) RememberOpsIfUnambiguous(ops); + if (ops.Count > 0) RememberOps(ops); } } diff --git a/src/Dropwheel/UI/OverlayWindow.Undo.cs b/src/Dropwheel/UI/OverlayWindow.Undo.cs index bc6fdd8..14c7ef5 100644 --- a/src/Dropwheel/UI/OverlayWindow.Undo.cs +++ b/src/Dropwheel/UI/OverlayWindow.Undo.cs @@ -8,20 +8,26 @@ namespace Dropwheel.UI; public partial class OverlayWindow { + internal readonly record struct FileOp( + DropAction Act, + string[] Sources, + string Dest, + string[] ExistingDestinations); + // One drop operation may consist of several moves (sorter). - private readonly List<(DropAction Act, string[] Sources, string Dest)> _lastOps = new(); + private readonly List _lastOps = new(); - private void RememberOpIfUnambiguous(DropAction act, string[] sources, string dest, bool hadCollision) - { - _lastOps.Clear(); - if (!hadCollision) _lastOps.Add((act, sources, dest)); - } + private void RememberOp(FileOp op) + { _lastOps.Clear(); _lastOps.Add(op); } - private void RememberOpsIfUnambiguous(IEnumerable<(DropAction Act, string[] Sources, string Dest, bool HadCollision)> ops) - { - _lastOps.Clear(); - _lastOps.AddRange(ops.Where(op => !op.HadCollision).Select(op => (op.Act, op.Sources, op.Dest))); - } + private void RememberOps(IEnumerable ops) + { _lastOps.Clear(); _lastOps.AddRange(ops); } + + internal static FileOp BuildOpBefore(DropAction act, string[] sources, string dest) => + new(act, sources, dest, FileOps.DestinationConflicts(sources, dest)); + + internal static FileOp BuildCreatedCopyOp(string[] sources, string dest) => + new(DropAction.Copy, sources, dest, Array.Empty()); private void OnUndoClick(object sender, MouseButtonEventArgs e) { @@ -40,14 +46,24 @@ private void Undo() ShowToast(ok ? "↩ Undone" : "Could not undo completely"); } - private static bool UndoOne((DropAction Act, string[] Sources, string Dest) op) + internal static string[] CopyUndoTargets(FileOp op) { - bool ok = true; + var protectedPaths = new HashSet( + op.ExistingDestinations.Select(p => IOPath.GetFullPath(p)), + StringComparer.OrdinalIgnoreCase); + return op.Sources + .Select(s => IOPath.Combine(op.Dest, IOPath.GetFileName(s))) + .Where(p => !protectedPaths.Contains(IOPath.GetFullPath(p))) + .Where(p => File.Exists(p) || Directory.Exists(p)) + .ToArray(); + } + + internal static bool UndoOne(FileOp op) + { + bool ok = op.ExistingDestinations.Length == 0; if (op.Act == DropAction.Copy) { - var copies = op.Sources - .Select(s => IOPath.Combine(op.Dest, IOPath.GetFileName(s))) - .Where(p => File.Exists(p) || Directory.Exists(p)).ToArray(); + var copies = CopyUndoTargets(op); if (copies.Length > 0) ok = FileOps.Delete(copies); } else @@ -55,7 +71,8 @@ private static bool UndoOne((DropAction Act, string[] Sources, string Dest) op) foreach (var src in op.Sources) { var dst = IOPath.Combine(op.Dest, IOPath.GetFileName(src)); - if ((File.Exists(dst) || Directory.Exists(dst)) + if (!op.ExistingDestinations.Contains(dst, StringComparer.OrdinalIgnoreCase) + && (File.Exists(dst) || Directory.Exists(dst)) && IOPath.GetDirectoryName(src) is { Length: > 0 } dir) ok &= FileOps.Execute(new[] { dst }, dir, DropAction.Move); } diff --git a/tests/Dropwheel.Tests/AppConfigTests.cs b/tests/Dropwheel.Tests/AppConfigTests.cs index da4af27..a57ee8c 100644 --- a/tests/Dropwheel.Tests/AppConfigTests.cs +++ b/tests/Dropwheel.Tests/AppConfigTests.cs @@ -4,8 +4,22 @@ namespace Dropwheel.Tests; -public class AppConfigTests +public class AppConfigTests : IDisposable { + private readonly string _root = Path.Combine(Path.GetTempPath(), "dw_config_" + Guid.NewGuid().ToString("N")); + + public AppConfigTests() + { + Directory.CreateDirectory(_root); + TargetStore.DirOverride = _root; + } + + public void Dispose() + { + TargetStore.DirOverride = null; + try { Directory.Delete(_root, true); } catch (DirectoryNotFoundException) { } + } + [Fact] public void Default_open_animation_preserves_existing_pop_behavior() { @@ -27,6 +41,37 @@ public void Bad_config_backup_path_is_timestamped_json() { var path = TargetStore.BackupPath(new DateTime(2026, 7, 8, 18, 30, 5)); - Assert.EndsWith(Path.Combine("Dropwheel", "config.bad.20260708_183005.json"), path); + Assert.Equal(Path.Combine(_root, "config.bad.20260708_183005.json"), path); + } + + [Fact] + public void Load_backs_up_corrupt_config_before_recreating_defaults() + { + var configPath = Path.Combine(_root, "config.json"); + File.WriteAllText(configPath, "{ not valid json"); + + TargetStore.Load(); + + var backup = Assert.Single(Directory.GetFiles(_root, "config.bad.*.json")); + Assert.Equal("{ not valid json", File.ReadAllText(backup)); + Assert.NotEmpty(TargetStore.Config.Targets); + Assert.Contains("\"Targets\"", File.ReadAllText(configPath)); + } + + [Fact] + public void Load_does_not_overwrite_config_when_backup_fails() + { + var configPath = Path.Combine(_root, "config.json"); + const string original = "{ locked config"; + File.WriteAllText(configPath, original); + + using (new FileStream(configPath, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) + { + TargetStore.Load(); + Assert.NotEmpty(TargetStore.Config.Targets); + } + + Assert.Equal(original, File.ReadAllText(configPath)); + Assert.Empty(Directory.GetFiles(_root, "config.bad.*.json")); } } diff --git a/tests/Dropwheel.Tests/FileOpsTests.cs b/tests/Dropwheel.Tests/FileOpsTests.cs index 3f84bbb..ac606ba 100644 --- a/tests/Dropwheel.Tests/FileOpsTests.cs +++ b/tests/Dropwheel.Tests/FileOpsTests.cs @@ -44,4 +44,27 @@ public void Destination_collision_ignores_non_colliding_names() } finally { Directory.Delete(root, true); } } + + [Fact] + public void DestinationConflicts_reports_existing_destination_names() + { + var root = Path.Combine(Path.GetTempPath(), "dw_fileops_" + Guid.NewGuid().ToString("N")); + var dest = Path.Combine(root, "dest"); + var src = Path.Combine(root, "src", "report.txt"); + Directory.CreateDirectory(Path.GetDirectoryName(src)!); + Directory.CreateDirectory(dest); + File.WriteAllText(src, "new"); + var existing = Path.Combine(dest, "report.txt"); + File.WriteAllText(existing, "old"); + try + { + var conflicts = FileOps.DestinationConflicts(new[] { src }, dest); + + Assert.Equal(new[] { existing }, conflicts); + } + finally + { + try { Directory.Delete(root, true); } catch (DirectoryNotFoundException) { } + } + } } diff --git a/tests/Dropwheel.Tests/OverlayUndoTests.cs b/tests/Dropwheel.Tests/OverlayUndoTests.cs new file mode 100644 index 0000000..f7e3c02 --- /dev/null +++ b/tests/Dropwheel.Tests/OverlayUndoTests.cs @@ -0,0 +1,75 @@ +using System.IO; +using Dropwheel.Models; +using Dropwheel.UI; + +namespace Dropwheel.Tests; + +public sealed class OverlayUndoTests : IDisposable +{ + private readonly string _root = Path.Combine(Path.GetTempPath(), "dw_undo_" + Guid.NewGuid().ToString("N")); + + public OverlayUndoTests() => Directory.CreateDirectory(_root); + public void Dispose() { try { Directory.Delete(_root, true); } catch (DirectoryNotFoundException) { } } + + [Fact] + public void CopyUndoTargets_skips_destination_that_existed_before_copy() + { + var src = Path.Combine(_root, "src", "report.txt"); + var dest = Path.Combine(_root, "dest"); + Directory.CreateDirectory(Path.GetDirectoryName(src)!); + Directory.CreateDirectory(dest); + File.WriteAllText(src, "new"); + var existing = Path.Combine(dest, "report.txt"); + File.WriteAllText(existing, "old"); + var op = OverlayWindow.BuildOpBefore(DropAction.Copy, new[] { src }, dest); + + var targets = OverlayWindow.CopyUndoTargets(op); + + Assert.Empty(targets); + } + + [Fact] + public void CopyUndoTargets_returns_created_copy_when_no_prior_conflict_exists() + { + var src = Path.Combine(_root, "src", "report.txt"); + var dest = Path.Combine(_root, "dest"); + Directory.CreateDirectory(Path.GetDirectoryName(src)!); + Directory.CreateDirectory(dest); + File.WriteAllText(src, "new"); + var op = OverlayWindow.BuildOpBefore(DropAction.Copy, new[] { src }, dest); + var copy = Path.Combine(dest, "report.txt"); + File.WriteAllText(copy, "new"); + + var targets = OverlayWindow.CopyUndoTargets(op); + + Assert.Equal(new[] { copy }, targets); + } + + [Fact] + public void UndoOne_copy_reports_incomplete_when_destination_preexisted() + { + var src = Path.Combine(_root, "src", "report.txt"); + var dest = Path.Combine(_root, "dest"); + Directory.CreateDirectory(Path.GetDirectoryName(src)!); + Directory.CreateDirectory(dest); + File.WriteAllText(src, "new"); + File.WriteAllText(Path.Combine(dest, "report.txt"), "old"); + var op = OverlayWindow.BuildOpBefore(DropAction.Copy, new[] { src }, dest); + + Assert.False(OverlayWindow.UndoOne(op)); + } + + [Fact] + public void UndoOne_move_reports_incomplete_when_destination_preexisted() + { + var src = Path.Combine(_root, "src", "report.txt"); + var dest = Path.Combine(_root, "dest"); + Directory.CreateDirectory(Path.GetDirectoryName(src)!); + Directory.CreateDirectory(dest); + File.WriteAllText(src, "new"); + File.WriteAllText(Path.Combine(dest, "report.txt"), "old"); + var op = OverlayWindow.BuildOpBefore(DropAction.Move, new[] { src }, dest); + + Assert.False(OverlayWindow.UndoOne(op)); + } +} diff --git a/tests/Dropwheel.Tests/SortServiceTests.cs b/tests/Dropwheel.Tests/SortServiceTests.cs index d0502c2..3203197 100644 --- a/tests/Dropwheel.Tests/SortServiceTests.cs +++ b/tests/Dropwheel.Tests/SortServiceTests.cs @@ -179,6 +179,19 @@ public void Catastrophic_regex_times_out_and_later_rules_can_match() Assert.True(sw.Elapsed < TimeSpan.FromSeconds(5)); } + [Fact] + public void Slow_regex_rule_times_out_and_lets_file_fall_through() + { + var file = MakeFile(new string('a', 40) + "!.txt"); + var t = Sorter(_root, + Rule("Slow", ConditionField.NameRegex, CompareOp.Matches, "^(a+)+$"), + Rule("Text", ConditionField.Extension, CompareOp.In, "txt")); + + var plan = SortService.Plan(t, new[] { file }); + + Assert.Contains(file, plan[Path.Combine(_root, "Text")]); + } + [Fact] public void MatchedRuleIndex_distinguishes_rules_with_the_same_destination() {