Fix the viewer model items from the review

src/DiffEngineViewer.Tests/AttachedViewerTests.cs

@@ -571,4 +571,53 @@ public void Dispose()
         {
         }
     }
+
+    /// <summary>
+    /// A tracked file the owner lists that this side cannot open. The listing is the same from one
+    /// pump to the next, so the entry should be too: rebuilt anyway, it replaced the one on screen
+    /// every 200ms and closed the reader's open menu with it.
+    /// </summary>
+    [Test]
+    public async Task AnUnreadableTrackedFileLeavesTheMenuOpen()
+    {
+        var file = Path.Combine(Path.GetTempPath(), $"AttachedViewerTests_{Guid.NewGuid():N}.verified.txt");
+        await File.WriteAllTextAsync(file, "locked away");
+        try
+        {
+            using var holder = new FileStream(file, FileMode.Open, FileAccess.Read, FileShare.None);
+            if (!ViewerServer.TryBind(0, out var server))
+            {
+                throw new("Could not bind an ephemeral port.");
+            }
+
+            using (server)
+            using (var cancel = new CancelSource())
+            {
+                _ = server.Listen(
+                    _ => ViewerResponse.Listing(
+                        [],
+                        deletes: [new(TrackedKeys.ForDelete(file), "Extra.verified.txt", null, file)]),
+                    cancel.Token);
+                var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows));
+                var link = new OwnerLink(host, server.Port);
+
+                await Assert.That(link.Pump()).IsTrue();
+                var first = host.State.Queue.Single();
+                await Assert.That(first.Warning).Contains("Could not read");
+                var row = QueueProjection.Rows(host.State).ToList().FindIndex(_ => _.Kind == QueueRowKind.Entry);
+                host.Mutate(_ => ViewerSession.OpenMenu(_, row));
+                await Assert.That(host.State.Menu).IsNotNull();
+
+                link.Pump();
+
+                await Assert.That(host.State.Menu).IsNotNull();
+                await Assert.That(host.State.Queue.Single()).IsSameReferenceAs(first);
+                await cancel.CancelAsync();
+            }
+        }
+        finally
+        {
+            File.Delete(file);
+        }
+    }
 }

src/DiffEngineViewer.Tests/CollapseTests.cs

@@ -178,4 +178,46 @@ static List<string> Labels(SessionState state) =>
         QueueProjection.Rows(state)
             .Select(_ => _.Label)
             .ToList();
+
+    /// <summary>
+    /// The entry being read is accepted, and the queue closes up: the index it leaves behind names
+    /// the first entry of the folded group after it. Nothing in the list would be highlighted while
+    /// the panes and Accept went on acting on an entry nobody could see.
+    /// </summary>
+    [Test]
+    public async Task Accepting_the_entry_being_read_does_not_select_one_under_a_fold()
+    {
+        var state = ViewerSession.ToggleGroup(Fixtures.Inline(solutionA1, solutionA2, solutionB1, solutionB2), solutionB);
+        state = ViewerSession.SelectKey(state, KeyOf(solutionA2));
+        await Assert.That(QueueProjection.VisibleEntries(state)).Contains(state.Selected);
+
+        var accepted = ViewerSession.Apply(state, CommandKind.Accept, Fixtures.Applied);
+
+        await Assert.That(QueueProjection.VisibleEntries(accepted)).Contains(accepted.Selected);
+    }
+
+    /// <summary>
+    /// The same for a window attached to someone else's queue, whose listing no longer has the
+    /// entry being read - accepted from the tray menu, say.
+    /// </summary>
+    [Test]
+    public async Task A_listing_without_the_entry_being_read_does_not_select_one_under_a_fold()
+    {
+        var state = ViewerSession.ToggleGroup(Fixtures.Attached(Fixtures.Pending(solutionA1, solutionA2, solutionB1, solutionB2)), solutionB);
+        state = ViewerSession.SelectKey(state, KeyOf(solutionA2));
+        await Assert.That(QueueProjection.VisibleEntries(state)).Contains(state.Selected);
+
+        var synced = ViewerSession.Sync(state, Fixtures.Pending(solutionA1, solutionB1, solutionB2), [], null);
+
+        await Assert.That(QueueProjection.VisibleEntries(synced)).Contains(synced.Selected);
+    }
+
+    const string solutionB = "solution|SolutionB";
+    static readonly InlinePatch solutionA1 = Fixtures.Patch(Fixtures.SolutionFile("SolutionA", "Tests", "ATests.cs"), 10);
+    static readonly InlinePatch solutionA2 = Fixtures.Patch(Fixtures.SolutionFile("SolutionA", "Tests", "ATests.cs"), 30);
+    static readonly InlinePatch solutionB1 = Fixtures.Patch(Fixtures.SolutionFile("SolutionB", "Tests", "BTests.cs"), 10);
+    static readonly InlinePatch solutionB2 = Fixtures.Patch(Fixtures.SolutionFile("SolutionB", "Tests", "BTests.cs"), 30);
+
+    static string KeyOf(InlinePatch patch) =>
+        QueueEntry.KeyForInline(patch.SourceFile, patch.LineHint);
 }

src/DiffEngineViewer.Tests/EntryMenuTests.cs

@@ -0,0 +1,175 @@
+/// <summary>
+/// An entry's context menu acts on the entry selected when one of its items is clicked, so the
+/// selection moving while the menu is open retargets it: Discard, clicked in a menu opened on one
+/// entry, discarded another. Nothing the reader did has to happen for that - a focus from the tray
+/// or an IDE, or a test re-sending a snapshot, moves the selection.
+/// </summary>
+public class EntryMenuTests
+{
+    /// <summary>
+    /// A menu opened on A, then a wire Focus naming B - the tray menu or an IDE asking for it -
+    /// through the real handler, then Discard.
+    /// </summary>
+    [Test]
+    public async Task A_wire_focus_does_not_retarget_an_open_entry_menu()
+    {
+        var a = Fixtures.Patch("A.cs", 1);
+        var b = Fixtures.Patch("B.cs", 2);
+        var host = new SessionHost(Fixtures.Inline(a, b));
+        var handler = new MessageHandler(host, Fixtures.Applied, _ =>
+        {
+        });
+        host.Mutate(_ => ViewerSession.OpenMenu(_, VisibleRowOf(_, _ => _.EntryIndex == 0)));
+        await Assert.That(host.State.Current!.Key).IsEqualTo(Key(a));
+        var discard = MenuIndex(host.State, "Discard");
+
+        var focus = handler.Handle(new(ViewerVerb.Focus, Key(b)));
+        await Assert.That(focus.Ok).IsTrue();
+
+        host.Mutate(_ => ViewerProgram.Apply(_, Click(discard), link: null, new NoWindow()));
+
+        await Assert.That(host.State.Queue.Select(_ => _.Key)).Contains(Key(b));
+    }
+
+    /// <summary>
+    /// Attached, with nothing but a test process involved. B's test fails again with the same
+    /// content: a tray owner folds it into the same entry and stashes a focus on it, which its next
+    /// full listing carries. The window syncs - the same entries, so the open menu is kept - and
+    /// then selects B, under the menu opened on A.
+    /// </summary>
+    [Test]
+    public async Task A_focus_riding_the_owners_listing_does_not_retarget_an_open_entry_menu()
+    {
+        var a = Fixtures.Patch("A.cs", 1);
+        var b = Fixtures.Patch("B.cs", 2);
+        using var owner = new TrayLikeOwner(Fixtures.Pending(a, b));
+        var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows));
+        var link = new OwnerLink(host, owner.Port);
+        await Assert.That(link.Pump()).IsTrue();
+        host.Mutate(_ => ViewerSession.OpenMenu(_, VisibleRowOf(_, _ => _.EntryIndex == 0)));
+        await Assert.That(host.State.Current!.Key).IsEqualTo(Key(a));
+        var discard = MenuIndex(host.State, "Discard");
+
+        owner.StashFocus(Key(b));
+        await Assert.That(link.Pump()).IsTrue();
+
+        host.Mutate(_ => ViewerProgram.Apply(_, Click(discard), link, new NoWindow()));
+        link.Pump();
+
+        await Assert.That(owner.Discarded).DoesNotContain(Key(b));
+    }
+
+    static string Key(InlinePatch patch) =>
+        QueueEntry.KeyForInline(patch.SourceFile, patch.LineHint);
+
+    static int VisibleRowOf(SessionState state, Func<QueueItem, bool> match) =>
+        QueueProjection.Visible(state, ScreenBuilder.BodyRows(state), out _).ToList().FindIndex(_ => match(_));
+
+    static int MenuIndex(SessionState state, string label) =>
+        state.Menu!.Items.ToList().FindIndex(_ => _.Label == label);
+
+    static ViewerInput Click(int menuItem) =>
+        new(CommandKind.None, -1, -1, 0, false, Fixtures.Columns, Fixtures.Rows)
+        {
+            ClickedMenuItem = menuItem
+        };
+
+    sealed class NoWindow : IViewerWindow
+    {
+        public bool Present(Screen screen) =>
+            true;
+
+        public ViewerInput Poll() =>
+            default;
+
+        public void SetHidden(bool hidden)
+        {
+        }
+
+        public void Focus()
+        {
+        }
+
+        public void SetClipboard(string text)
+        {
+        }
+
+        public bool Capture(Screen screen, int width, int height, string pngPath) =>
+            false;
+
+        public void Dispose()
+        {
+        }
+    }
+
+    /// <summary>
+    /// A queue owner answering the way OwnedInlineHost does where it matters here: a full listing
+    /// of a fixed queue, carrying a stashed window command once. Discards are recorded rather than
+    /// carried out.
+    /// </summary>
+    sealed class TrayLikeOwner : IDisposable
+    {
+        readonly CancelSource cancel = new();
+        readonly ViewerServer server;
+        readonly Task listening;
+        readonly List<ViewerResponseItem> items;
+        string? focus;
+
+        public TrayLikeOwner(InlineQueue queue)
+        {
+            items = ViewerListing.Items(queue.Items, withPatches: true);
+            if (!ViewerServer.TryBind(0, out var bound))
+            {
+                throw new("Could not bind an ephemeral port.");
+            }
+
+            server = bound;
+            listening = server.Listen(Handle, cancel.Token);
+        }
+
+        public int Port => server.Port;
+
+        public List<string?> Discarded { get; } = [];
+
+        public void StashFocus(string key) =>
+            Interlocked.Exchange(ref focus, key);
+
+        ViewerResponse Handle(ViewerMessage message)
+        {
+            if (message.Verb == ViewerVerb.ListFull)
+            {
+                if (Interlocked.Exchange(ref focus, null) is { } key)
+                {
+                    return ViewerResponse.Listing(items, WindowCommand.Focus, key);
+                }
+
+                return ViewerResponse.Listing(items);
+            }
+
+            if (message.Verb == ViewerVerb.Discard)
+            {
+                lock (Discarded)
+                {
+                    Discarded.Add(message.Key);
+                }
+            }
+
+            return ViewerResponse.Success();
+        }
+
+        public void Dispose()
+        {
+            cancel.Cancel();
+            server.Dispose();
+            try
+            {
+                listening.Wait(TimeSpan.FromSeconds(5));
+            }
+            catch (AggregateException)
+            {
+            }
+
+            cancel.Dispose();
+        }
+    }
+}

src/DiffEngineViewer.Tests/FileSideTests.cs

@@ -109,4 +109,27 @@ static byte[] Png()
         bytes[23] = 600 & 0xFF;
         return bytes;
     }
+
+    /// <summary>
+    /// What a reader met for a bitmap with the one height Math.Abs throws on: FileSide.Read's
+    /// catch-all made the side unreadable, with the exception's message as its warning and no
+    /// stamp, so it was read again on every pass.
+    /// </summary>
+    [Test]
+    public async Task ABmpWithTheMinimumHeightIsReadAsABmp()
+    {
+        var bytes = new byte[54];
+        "BM"u8.CopyTo(bytes);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(2), bytes.Length);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(14), 40);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(18), 64);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(22), int.MinValue);
+        var path = Write("MinimumHeight.received.bmp", bytes);
+
+        var side = FileSide.Read(path);
+
+        await Assert.That(side.Warning).IsNull();
+        await Assert.That(side.Stamp).IsNotNull();
+        await Assert.That(side.Image!.Value.Header!.Value.Format).IsEqualTo(ImageFormat.Bmp);
+    }
 }

src/DiffEngineViewer.Tests/HeldDeleteTests.cs

@@ -66,4 +66,62 @@ static SessionState Conflicted() =>
                 Fixtures.Patch(content: "eight", framework: "net8.0"),
                 Fixtures.Patch(content: "nine", framework: "net9.0")),
             Fixtures.Delete());
+
+    /// <summary>
+    /// B was accepted on its own earlier and refused, so it carries a status. "Accept all in
+    /// SolutionA" then writes A's only snapshot, and carries out A's delete: whether a group holds
+    /// its deletes is about what that group's own accepts did, not about statuses other accepts
+    /// left elsewhere in the queue.
+    /// </summary>
+    [Test]
+    public async Task An_earlier_failure_in_another_solution_does_not_hold_this_solutions_deletes()
+    {
+        var a = Fixtures.Patch(Fixtures.SolutionFile("SolutionA", "Tests", "ATests.cs"), 10);
+        var b = Fixtures.Patch(Fixtures.SolutionFile("SolutionB", "Tests", "BTests.cs"), 10);
+        var state = ViewerSession.EnqueueTracked(Fixtures.Inline(a, b), Fixtures.Delete(solution: "SolutionA"));
+        state = ViewerSession.SelectKey(state, QueueEntry.KeyForInline(b.SourceFile, b.LineHint));
+        state = ViewerSession.Apply(
+            state,
+            CommandKind.Accept,
+            Fixtures.Applying(InlineApplyResult.Failed("Failed to write: BTests.cs")));
+        await Assert.That(state.Queue.Single(_ => _.Key == QueueEntry.KeyForInline(b.SourceFile, b.LineHint)).Status).IsNotNull();
+
+        var deleted = new List<string>();
+        var actions = Fixtures.Applied with
+        {
+            DeleteFile = _ => deleted.Add(_)
+        };
+        var visible = QueueProjection.Visible(state, ScreenBuilder.BodyRows(state), out _).ToList();
+        state = ViewerSession.OpenMenu(state, visible.FindIndex(_ => _.GroupName == "SolutionA"));
+        await Assert.That(state.Menu!.Items[1].Label).IsEqualTo("Accept all in SolutionA");
+
+        var swept = ViewerSession.Apply(state, CommandKind.AcceptGroup, actions);
+
+        await Assert.That(deleted).IsEquivalentTo(["code/extra.verified.txt"]);
+        await Assert.That(swept.Queue.Where(_ => _.Kind == QueueEntryKind.Delete)).IsEmpty();
+    }
+
+    /// <summary>
+    /// And one of the group's own that failed, rather than going stale, holds them: it is still in
+    /// the queue, unwritten.
+    /// </summary>
+    [Test]
+    public async Task A_failure_in_this_solution_holds_its_deletes()
+    {
+        var a = Fixtures.Patch(Fixtures.SolutionFile("SolutionA", "Tests", "ATests.cs"), 10);
+        var b = Fixtures.Patch(Fixtures.SolutionFile("SolutionB", "Tests", "BTests.cs"), 10);
+        var state = ViewerSession.EnqueueTracked(Fixtures.Inline(a, b), Fixtures.Delete(solution: "SolutionA"));
+        var deleted = new List<string>();
+        var actions = Fixtures.Applying(InlineApplyResult.Failed("Failed to write: ATests.cs")) with
+        {
+            DeleteFile = _ => deleted.Add(_)
+        };
+        var visible = QueueProjection.Visible(state, ScreenBuilder.BodyRows(state), out _).ToList();
+        state = ViewerSession.OpenMenu(state, visible.FindIndex(_ => _.GroupName == "SolutionA"));
+
+        var swept = ViewerSession.Apply(state, CommandKind.AcceptGroup, actions);
+
+        await Assert.That(deleted).IsEmpty();
+        await Assert.That(swept.Queue.Single(_ => _.Kind == QueueEntryKind.Delete).Status).IsNotNull();
+    }
 }

src/DiffEngineViewer.Tests/ImageHeaderTests.cs

@@ -152,4 +152,23 @@ static byte[] Ico(int width, int height)
         bytes[7] = (byte) (height == 256 ? 0 : height);
         return bytes;
     }
+
+    /// <summary>
+    /// The one height with no positive counterpart. Math.Abs throws on it, and the side it was read
+    /// for came back unreadable with that exception's message as its warning.
+    /// </summary>
+    [Test]
+    public async Task ABmpWithTheMinimumHeightIsStillABmp()
+    {
+        var bytes = new byte[54];
+        "BM"u8.CopyTo(bytes);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(14), 40);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(18), 64);
+        BinaryPrimitives.WriteInt32LittleEndian(bytes.AsSpan(22), int.MinValue);
+
+        await Assert.That(ImageHeader.TryRead(bytes, out var header)).IsTrue();
+        await Assert.That(header.Format).IsEqualTo(ImageFormat.Bmp);
+        await Assert.That(header.Width).IsEqualTo(64);
+        await Assert.That(header.Height).IsEqualTo(0);
+    }
 }