Guard the menu and hotkey delete paths

src/DiffEngineTray.Tests/TrackerDeleteTest.cs

@@ -80,6 +80,48 @@ public async Task AcceptSingle_NotEmpty()
         await Assert.That(tracker.TrackingAny).IsTrue();
     }
 
+    /// <summary>
+    /// A delete that cannot be done. The menu and hot key paths called File.Delete straight, so a
+    /// read-only or open verified file threw out of the click handler - onto the UI thread, where
+    /// nothing hooks Application.ThreadException - and the entry had already been untracked, so
+    /// the pending delete went with it.
+    /// </summary>
+    [Test]
+    public async Task AcceptLeavesAnUndeletableFileTracked()
+    {
+        await using var tracker = new RecordingTracker();
+        var tracked = tracker.AddDelete(file1);
+
+        using (File.Open(file1, FileMode.Open, FileAccess.Read, FileShare.None))
+        {
+            tracker.Accept(tracked);
+        }
+
+        await Assert.That(tracker.Deletes).HasSingleItem();
+        await Assert.That(File.Exists(file1)).IsTrue();
+    }
+
+    /// <summary>
+    /// And one bad delete does not stop the rest of Accept all. Unguarded, the throw skipped the
+    /// remaining deletes, every pending move, and the snapshots.
+    /// </summary>
+    [Test]
+    public async Task AcceptAllContinuesPastAnUndeletableFile()
+    {
+        await using var tracker = new RecordingTracker();
+        tracker.AddDelete(file1);
+        tracker.AddDelete(file2);
+
+        using (File.Open(file1, FileMode.Open, FileAccess.Read, FileShare.None))
+        {
+            await tracker.AcceptAll();
+        }
+
+        // The one that could go, went; the one that could not is still pending
+        await Assert.That(File.Exists(file2)).IsFalse();
+        await Assert.That(tracker.Deletes).HasSingleItem();
+    }
+
     public void Dispose()
     {
         File.Delete(file1);

src/DiffEngineTray/Tracker.cs

@@ -404,22 +404,30 @@ public TrackedDelete AddDelete(string file) =>
                 return existing;
             });
 
+    /// <summary>
+    /// Through <see cref="AcceptTracked(TrackedDelete)"/>, which is what the wire path has always
+    /// used: it catches, re-tracks so the delete can be retried, and reports why.
+    /// <para>
+    /// These called File.Delete straight, so a read-only or open verified file threw out of a menu
+    /// click or a hot key - onto the UI thread, where nothing hooks Application.ThreadException -
+    /// and the entry was already untracked by then, so the pending delete was lost with it.
+    /// </para>
+    /// </summary>
     public void Accept(TrackedDelete delete)
     {
-        if (deletes.TryRemove(delete.File, out var removed))
+        var (ok, message) = AcceptTracked(delete);
+        if (!ok &&
+            message != null)
         {
-            File.Delete(removed.File);
+            Log.Error(message);
         }
     }
 
     public void Accept(IEnumerable<TrackedDelete> toAccept)
     {
-        foreach (var delete in toAccept)
+        foreach (var delete in toAccept.ToList())
         {
-            if (deletes.TryRemove(delete.File, out var removed))
-            {
-                File.Delete(removed.File);
-            }
+            Accept(delete);
         }
     }
 
@@ -706,12 +714,13 @@ public Task AcceptAll()
 
     void AcceptAllDeletes()
     {
-        foreach (var delete in deletes.Values)
+        // One at a time, and no Clear afterwards: a delete that fails re-tracks itself, and
+        // clearing would throw that away. Unguarded, the first bad one also took AcceptMoves and
+        // AcceptAllSnapshots with it, so "Accept all" stopped at the first read-only file
+        foreach (var delete in deletes.Values.ToList())
         {
-            File.Delete(delete.File);
+            Accept(delete);
         }
-
-        deletes.Clear();
     }
 
     public ICollection<TrackedDelete> Deletes => deletes.Values;