Merge branch 'main' into the group's move and its deletion question

Main took the group's EDIT and DELETE off the GROUPS heading while this branch
was adding a MOVE beside them, so the conflict was about the same six pixels
from both directions. Main's answer wins outright, and it is the better one for
the reason its own message gives: a button beside a heading has no card under a
pointer to mean, and had to work its subject out from the selection or from the
trail. Moving a group had that problem worst of all — the thing it takes with it
is everything on the shelf, and "which shelf" is not a question a button there
could answer plainly.

So the MOVE button is gone and the menu entry it was drawn beside is the whole
of it. That entry was already in this branch, above the separator DELETE sits
below, and it needed no change: the card menu selects whatever was right-clicked
before it runs anything, which is exactly the aiming a group move wants.

Three things went with the button. ShowsGroupActions, which main deleted because
hiding buttons was all it did, and which this branch had extended to hide them
for the move panel as well. CanMoveGroupTarget, which existed to answer whether
that button was worth drawing — CanMoveSelectedHost stays, because the phone
really does leave the host's MOVE out rather than offer a refusal, and a menu
whose entries came and went would be a menu whose items move. And the two test
assertions that read them, which were describing the button rather than the
behaviour; what they were guarding is that the two panels never share the
moment, and IsConfirmingGroupDeletion says that directly.

The move panel and the deletion question both keep their place under the
heading, which is where the buttons were and is now simply where that section
puts things. They still exclude each other, by disarming rather than by a
visibility flag: MoveGroup clears a pending deletion and DeleteGroup folds the
move panel away.

Manual checks 3.3 was rewritten by main for the menu and by this branch for the
tick, and now says both; 3.3a is new and walks a two-level shelf across a vault
boundary, which is the half of this feature no headless test can watch land.

tests/DodoSSH.Client.App.Tests/FakeVaultEventStream.cs

@@ -0,0 +1,49 @@
+using System.Threading.Channels;
+using DodoSSH.Client.Api;
+using DodoSSH.Contracts;
+
+namespace DodoSSH.Client.App.Tests;
+
+/// <summary>
+/// A server's push channel, driven by a test rather than by a socket.
+/// </summary>
+/// <remarks>
+/// The real <c>VaultEventStream</c> is a reconnection policy wrapped round a WebSocket, and none of
+/// that is what the shell's behaviour depends on: what the shell does with a notice is the same
+/// whether it arrived over a healthy socket, after four reconnections, or from this. Driving it by
+/// hand is what makes "the loop synchronised because it was told to, not because a minute passed" a
+/// test that finishes in milliseconds and cannot flake.
+/// </remarks>
+internal sealed class FakeVaultEventStream : IVaultEventStream
+{
+    private readonly Channel<VaultEvent> notices = Channel.CreateUnbounded<VaultEvent>();
+
+    /// <inheritdoc />
+    public bool IsConnected => true;
+
+    /// <summary>How many times the shell has waited on this. Proves the loop is watching at all.</summary>
+    internal int Reads { get; private set; }
+
+    /// <summary>Delivers a notice, as a server would.</summary>
+    internal void Push(Guid vaultId, long sequence = 1) =>
+        notices.Writer.TryWrite(
+            new VaultEvent(VaultEventKinds.VaultChanged, vaultId, sequence));
+
+    /// <summary>Delivers the notice that says the caller's vault list has changed.</summary>
+    internal void PushAccessChanged() =>
+        notices.Writer.TryWrite(new VaultEvent(VaultEventKinds.VaultsChanged));
+
+    /// <inheritdoc />
+    public ValueTask<VaultEvent> ReadAsync(CancellationToken cancellationToken)
+    {
+        Reads++;
+
+        return notices.Reader.ReadAsync(cancellationToken);
+    }
+
+    /// <inheritdoc />
+    public bool TryRead(out VaultEvent notice) => notices.Reader.TryRead(out notice!);
+
+    /// <inheritdoc />
+    public void Dispose() => notices.Writer.TryComplete();
+}

tests/DodoSSH.Client.App.Tests/FakeVaultServer.cs

@@ -39,6 +39,16 @@ internal sealed partial class FakeVaultServer : IVaultServer, IAccountApi, ISync
 
     internal int PushCount { get; private set; }
 
+    /// <summary>
+    /// How many delta reads this server has served.
+    /// </summary>
+    /// <remarks>
+    /// The one observable a synchronisation pass always produces. <see cref="PushCount"/> only moves when
+    /// there is something queued, so a test asking "did a pass run" — which is what the push channel's
+    /// whole purpose comes down to — has to count pulls.
+    /// </remarks>
+    internal int PullCount { get; private set; }
+
     internal bool IsEnrolled => statement is not null;
 
     /// <summary>
@@ -99,6 +109,19 @@ internal sealed partial class FakeVaultServer : IVaultServer, IAccountApi, ISync
     /// <inheritdoc />
     public IKeyBindingAuthorizer KeyBinding => this;
 
+    /// <summary>
+    /// The push channel, which a test drives by hand.
+    /// </summary>
+    /// <remarks>
+    /// A real queue rather than an idle stand-in, because the behaviour worth covering here is the one
+    /// the socket exists for: a notice arriving makes the background loop synchronise without waiting
+    /// out its minute. See <see cref="FakeVaultEventStream.Push"/>.
+    /// </remarks>
+    internal FakeVaultEventStream Notices { get; } = new();
+
+    /// <inheritdoc />
+    public IVaultEventStream Events => Notices;
+
     /// <inheritdoc />
     public SyncOptions SyncOptions => SyncOptions.Default;
 
@@ -238,6 +261,8 @@ internal sealed partial class FakeVaultServer : IVaultServer, IAccountApi, ISync
         SyncPullRequest request,
         CancellationToken cancellationToken)
     {
+        PullCount++;
+
         if (SyncFailure is { } failure)
         {
             return Task.FromException<SyncPullResponse>(failure);

tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs

@@ -523,6 +523,90 @@ public sealed class ShellFlowTests : IAsyncLifetime
         vault.Status.ShouldContain("bad day");
     }
 
+    /// <summary>
+    /// The whole point of the push channel: a pass that did not wait for the minute.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// The timing is what makes this an assertion rather than a hope. The background timer is a full
+    /// minute and the wait below gives up in ten seconds, so a pull that arrives can only have been
+    /// caused by the notice — there is no interval at which the timer could have produced it.
+    /// </para>
+    /// <para>
+    /// The vault id in the notice is arbitrary, and deliberately so: a pass synchronises every vault
+    /// this session can reach, so the loop reads the notice as "there is something to fetch" and never
+    /// as "fetch this one". A test that seeded a real id would imply a targeting this does not do.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task APushedNotice_SynchronisesWithoutWaitingForTheTimer()
+    {
+        await UnlockedAsync();
+
+        // The unlock starts the loop, whose first act is a pass; waited out so the count below is a
+        // baseline rather than a race with it.
+        await EventuallyAsync(
+            () => server.PullCount > 0,
+            "the pass on open should have run");
+
+        var before = server.PullCount;
+
+        server.Notices.Push(Guid.CreateVersion7());
+
+        await EventuallyAsync(
+            () => server.PullCount > before,
+            "a notice should have woken the loop long before the one-minute timer");
+    }
+
+    /// <remarks>
+    /// The half that is easy to get wrong. The loop selects between two waits, and both have to survive
+    /// losing: <c>PeriodicTimer</c> throws if a second wait is started while one is outstanding, and an
+    /// abandoned channel read stays registered and swallows the next notice written. Either defect
+    /// leaves the first notice working and every one after it silently lost, which is why one notice is
+    /// not enough to prove this.
+    /// </remarks>
+    [Fact]
+    public async Task NoticesKeepWakingTheLoop_NotJustTheFirst()
+    {
+        await UnlockedAsync();
+
+        await EventuallyAsync(() => server.PullCount > 0, "the pass on open should have run");
+
+        for (var round = 1; round <= 3; round++)
+        {
+            var before = server.PullCount;
+
+            server.Notices.Push(Guid.CreateVersion7());
+
+            await EventuallyAsync(
+                () => server.PullCount > before,
+                $"notice {round} should have woken the loop as the first one did");
+        }
+    }
+
+    /// <summary>Waits for something a background loop is expected to do, or fails saying what.</summary>
+    /// <remarks>
+    /// Polled rather than signalled because the thing under test is a loop nobody hands a completion
+    /// source to. The bound is generous — this is not measuring latency, only proving that the timer
+    /// cannot be what caused the result.
+    /// </remarks>
+    private static async Task EventuallyAsync(Func<bool> condition, string because)
+    {
+        var deadline = TimeProvider.System.GetUtcNow().AddSeconds(10);
+
+        while (TimeProvider.System.GetUtcNow() < deadline)
+        {
+            if (condition())
+            {
+                return;
+            }
+
+            await Task.Delay(TimeSpan.FromMilliseconds(20), Token);
+        }
+
+        throw new ShouldAssertException(because);
+    }
+
     /// <remarks>
     /// <para>
     /// The page's own <c>term.focus()</c> focuses the textarea inside the document, which does nothing

tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs

@@ -912,13 +912,12 @@ public sealed class VaultSharingTests : IAsyncLifetime
         production.VaultId.ShouldNotBe(sharedVaultId, "this test is meaningless with both in one vault");
         production.Group.ParentId.ShouldNotBeNull("it was nested, which is what has to stay behind");
 
+        // As the card's menu does before it runs the command; see HostsScreen.OnGroupContextRequested.
         vault.SelectedGroup = production;
-        vault.CanMoveGroupTarget.ShouldBeTrue("there is a second vault this session can write to");
 
         vault.MoveGroupCommand.Execute(null);
 
         vault.IsMovingGroup.ShouldBeTrue(vault.Status);
-        vault.ShowsGroupActions.ShouldBeFalse("the panel takes the row of buttons rather than sharing it");
         vault.MoveGroupVaultChoices.ShouldNotContain(choice => choice.VaultId == production.VaultId);
 
         vault.SelectedMoveGroupVault =
@@ -956,7 +955,9 @@ public sealed class VaultSharingTests : IAsyncLifetime
 
     /// <remarks>
     /// The move is refused where it would have nowhere to go, by the command rather than by an empty picker
-    /// — the same answer <c>MoveHostCommand</c> gives one level down.
+    /// — the same answer <c>MoveHostCommand</c> gives one level down, and the only place the question is
+    /// asked. The menu entry is drawn either way, because a menu whose items came and went would be a menu
+    /// whose items move.
     /// </remarks>
     [Fact]
     public async Task MovingAGroupWithNowhereToMoveIt_SaysSoRatherThanOpeningAnEmptyPicker()
@@ -973,8 +974,6 @@ public sealed class VaultSharingTests : IAsyncLifetime
 
         vault.SelectedGroup = vault.Groups.ShouldHaveSingleItem();
 
-        vault.CanMoveGroupTarget.ShouldBeFalse("the personal vault is the only one there is");
-
         vault.MoveGroupCommand.Execute(null);
 
         vault.IsMovingGroup.ShouldBeFalse();