Merge branch 'claude/sleepy-chebyshev-cda68d'

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

@@ -0,0 +1,107 @@
+using System.Globalization;
+using System.Net.WebSockets;
+using System.Text.RegularExpressions;
+using DodoSSH.Client.Terminal;
+
+namespace DodoSSH.Client.App.Tests;
+
+/// <summary>
+/// Stands in for the terminal page, so the connect path can be exercised without a WebView.
+/// </summary>
+/// <remarks>
+/// <para>
+/// It attaches the way the real renderer does rather than reaching for the workspace's internals:
+/// fetch the served page, read the token and socket URL the host substituted into it, then open the
+/// WebSocket with the same two subprotocols. Anything cheaper — handing it the token directly — would
+/// stop testing the part of the handshake that has actually been got wrong before.
+/// </para>
+/// <para>
+/// Attaching is what completes <c>TerminalWorkspace.WaitForRendererAsync</c>, and that await is the
+/// real gate on a first connection: the data plane drops frames when nothing is attached rather than
+/// queueing them, so a session opened before this exists would lose its <c>SessionOpened</c> frame.
+/// </para>
+/// </remarks>
+internal sealed partial class FakeRenderer : IAsyncDisposable
+{
+    private readonly ClientWebSocket socket;
+
+    private FakeRenderer(ClientWebSocket socket) => this.socket = socket;
+
+    /// <summary>Fetches the page and attaches a socket, as the real renderer would.</summary>
+    internal static async Task<FakeRenderer> AttachAsync(
+        TerminalWorkspace workspace,
+        CancellationToken cancellationToken)
+    {
+        using var http = new HttpClient();
+
+        var page = await http
+            .GetStringAsync(workspace.PageUrl, cancellationToken)
+            .ConfigureAwait(false);
+
+        var token = Attribute(page, "data-token");
+        var socketUrl = Attribute(page, "data-socket");
+
+        var attached = new ClientWebSocket();
+
+        attached.Options.AddSubProtocol(TerminalDataPlane.SubProtocol);
+        attached.Options.AddSubProtocol($"token.{token}");
+
+        // The listener requires the page's own origin, which is what makes a page in the user's
+        // browser unable to reach this socket.
+        attached.Options.SetRequestHeader(
+            "Origin",
+            string.Create(
+                CultureInfo.InvariantCulture,
+                $"{workspace.PageUrl.Scheme}://{workspace.PageUrl.Authority}"));
+
+        try
+        {
+            await attached.ConnectAsync(new Uri(socketUrl), cancellationToken).ConfigureAwait(false);
+        }
+        catch
+        {
+            attached.Dispose();
+            throw;
+        }
+
+        var renderer = new FakeRenderer(attached);
+
+        await workspace.WaitForRendererAsync().ConfigureAwait(false);
+
+        return renderer;
+    }
+
+    /// <inheritdoc />
+    public async ValueTask DisposeAsync()
+    {
+        if (socket.State == WebSocketState.Open)
+        {
+            try
+            {
+                await socket
+                    .CloseAsync(WebSocketCloseStatus.NormalClosure, null, CancellationToken.None)
+                    .ConfigureAwait(false);
+            }
+            catch (WebSocketException)
+            {
+                // The host may have gone first; there is nothing to salvage either way.
+            }
+        }
+
+        socket.Dispose();
+    }
+
+    private static string Attribute(string page, string name)
+    {
+        var match = AttributeValue(name).Match(page);
+
+        return match.Success
+            ? match.Groups[1].Value
+            : throw new InvalidOperationException(
+                $"The served page carried no {name}. The host substitutes it at serve time, so "
+                + $"either the placeholder is missing from the test's page asset or substitution broke.");
+    }
+
+    private static Regex AttributeValue(string name) =>
+        new($"{Regex.Escape(name)}=\"([^\"]*)\"", RegexOptions.None, TimeSpan.FromSeconds(1));
+}

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

@@ -0,0 +1,99 @@
+using DodoSSH.Client.Ssh;
+
+namespace DodoSSH.Client.App.Tests;
+
+/// <summary>
+/// An SSH stack that connects to nothing.
+/// </summary>
+/// <remarks>
+/// The shell suite is about what the view models do, and the real factory would need a reachable
+/// sshd — which <c>DodoSSH.Client.Ssh.Tests</c> already covers against a container. What this makes
+/// testable is everything the connect path does <em>around</em> the connection.
+/// </remarks>
+internal sealed class FakeSshConnectionFactory : ISshConnectionFactory
+{
+    /// <summary>Thrown instead of connecting, when set. Used for the host-key paths.</summary>
+    internal Exception? Failure { get; set; }
+
+    /// <summary>Requests this factory was asked for, in order.</summary>
+    internal List<SshConnectionRequest> Requests { get; } = [];
+
+    /// <inheritdoc />
+    public Task<ISshConnection> ConnectAsync(
+        SshConnectionRequest request,
+        CancellationToken cancellationToken)
+    {
+        Requests.Add(request);
+
+        return Failure is { } failure
+            ? Task.FromException<ISshConnection>(failure)
+            : Task.FromResult<ISshConnection>(new FakeSshConnection(request));
+    }
+}
+
+internal sealed class FakeSshConnection(SshConnectionRequest request) : ISshConnection
+{
+    /// <inheritdoc />
+    public bool IsConnected { get; private set; } = true;
+
+    /// <inheritdoc />
+    public HostKeyPresentation HostKey { get; } =
+        new(request.Host, request.Port, "ssh-ed25519", "SHA256:fake");
+
+    /// <inheritdoc />
+    public Task<ISshShellSession> OpenShellAsync(
+        TerminalSize size,
+        CancellationToken cancellationToken) =>
+        Task.FromResult<ISshShellSession>(new FakeSshShellSession());
+
+    /// <inheritdoc />
+    public ValueTask DisposeAsync()
+    {
+        IsConnected = false;
+        return ValueTask.CompletedTask;
+    }
+}
+
+/// <summary>A shell that is open, silent and never closes on its own.</summary>
+/// <remarks>
+/// <see cref="ReadAsync"/> blocks rather than returning 0. Returning 0 means the remote closed the
+/// channel, which would end the session the moment it was opened and make the test assert against a
+/// connection that had already gone.
+/// </remarks>
+internal sealed class FakeSshShellSession : ISshShellSession
+{
+    private readonly CancellationTokenSource closed = new();
+
+    /// <inheritdoc />
+    public bool IsOpen { get; private set; } = true;
+
+    /// <inheritdoc />
+    public async ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken)
+    {
+        using var linked = CancellationTokenSource.CreateLinkedTokenSource(
+            cancellationToken, closed.Token);
+
+        await Task.Delay(Timeout.InfiniteTimeSpan, linked.Token).ConfigureAwait(false);
+
+        return 0;
+    }
+
+    /// <inheritdoc />
+    public ValueTask WriteAsync(ReadOnlyMemory<byte> data, CancellationToken cancellationToken) =>
+        ValueTask.CompletedTask;
+
+    /// <inheritdoc />
+    public void Resize(TerminalSize size)
+    {
+    }
+
+    /// <inheritdoc />
+    public async ValueTask DisposeAsync()
+    {
+        IsOpen = false;
+
+        await closed.CancelAsync().ConfigureAwait(false);
+
+        closed.Dispose();
+    }
+}

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

@@ -29,6 +29,13 @@ public sealed class ShellFlowTests : IAsyncLifetime
 
     private readonly FakeVaultServer server = new();
 
+    /// <remarks>
+    /// The real factory would need a reachable sshd, which <c>DodoSSH.Client.Ssh.Tests</c> covers against
+    /// a container. Nothing in this suite connected before, so substituting it costs no coverage and makes
+    /// the connect path reachable.
+    /// </remarks>
+    private readonly FakeSshConnectionFactory ssh = new();
+
     private int signInAttempts;
 
     private string directory = null!;
@@ -51,15 +58,26 @@ public sealed class ShellFlowTests : IAsyncLifetime
         // In-memory assets rather than the application's Avalonia-resource provider, which reads the
         // resource system at construction and needs an initialised toolkit. This is what
         // ITerminalAssetProvider is for; nothing in this suite renders anything.
+        //
+        // The page carries the same two placeholders the real one does, because FakeRenderer attaches by
+        // reading them back out of the served page rather than by being handed the token.
         workspace = new TerminalWorkspace(
             new InMemoryTerminalAssetProvider(
                 new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)
                 {
-                    ["/terminal"] = new("text/html; charset=utf-8", "<!doctype html>"u8.ToArray()),
+                    ["/terminal"] = new(
+                        "text/html; charset=utf-8",
+                        System.Text.Encoding.UTF8.GetBytes(
+                            $"<!doctype html><div data-token=\"{TerminalDataPlane.TokenPlaceholder}\" "
+                            + $"data-socket=\"{TerminalDataPlane.SocketUrlPlaceholder}\"></div>")),
                 }),
-            new SshNetConnectionFactory(knownHosts),
+            ssh,
             TimeProvider.System);
 
+        // Started, as the application does immediately after composing it. Without the accept loop the
+        // page is never served, so nothing could attach a renderer.
+        workspace.Start();
+
         shell = new MainWindowViewModel(
             paths,
             caches,
@@ -372,6 +390,95 @@ public sealed class ShellFlowTests : IAsyncLifetime
         vault.Status.ShouldContain("bad day");
     }
 
+    /// <remarks>
+    /// <para>
+    /// The page's own <c>term.focus()</c> focuses the textarea inside the document, which does nothing
+    /// while the window's keyboard focus is still on the Connect button — so the first keystrokes of a
+    /// session went to the shell's UI rather than the remote shell, and the terminal had to be clicked
+    /// first. The view hands the control focus when this fires; see <c>NativeKeyboardFocus</c> for why an
+    /// ordinary <c>Focus()</c> call is enough in that direction and not in the other.
+    /// </para>
+    /// <para>
+    /// What this covers is the plumbing that carries the fix: that the raise is on the success path and
+    /// happens once per session, and that the shell forwards it. Deleting the raise outright is already a
+    /// build error — the event would be unused, and warnings are errors — but moving it, which is the
+    /// likelier mistake, is not. It does <em>not</em> cover the focus call itself: that needs a native
+    /// window, and headless Avalonia has none, which is exactly why this class of defect has escaped
+    /// tests here before. Measured separately in a harness; see docs/platform-flags.md.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task ConnectingAsksTheViewToFocusTheTerminal()
+    {
+        var vault = await ReadyToConnectAsync();
+
+        await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
+
+        var requests = 0;
+        shell.TerminalSessionOpened += (_, _) => requests++;
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        vault.Status.ShouldContain("Connected", Case.Insensitive);
+        requests.ShouldBe(1);
+
+        // Again, on a second session. This is why it is an event and not a bound flag: a boolean that was
+        // already true would not move focus to the terminal the user just opened.
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        requests.ShouldBe(2);
+    }
+
+    /// <remarks>
+    /// Focus must not be taken on a failure. A host-key prompt needs the keyboard on the prompt's own
+    /// buttons, and taking it into a terminal that has no session would strand the decision.
+    /// </remarks>
+    [Fact]
+    public async Task AFailedConnect_DoesNotAskForTheTerminalToBeFocused()
+    {
+        var vault = await ReadyToConnectAsync();
+
+        await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
+
+        ssh.Failure = new SshHostKeyUnknownException(
+            new HostKeyPresentation("db.internal", 22, "ssh-ed25519", "SHA256:unknown"));
+
+        var requests = 0;
+        shell.TerminalSessionOpened += (_, _) => requests++;
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        vault.HasPendingHostKey.ShouldBeTrue();
+        requests.ShouldBe(0);
+    }
+
+    /// <remarks>
+    /// The shell stops forwarding once the vault is gone. Dropping the detach half of that would compile
+    /// and pass every other test, while leaving a discarded vault able to move focus in a locked window.
+    /// </remarks>
+    [Fact]
+    public async Task LockingStopsTheShellForwardingFocusRequests()
+    {
+        var vault = await ReadyToConnectAsync();
+
+        await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
+
+        var requests = 0;
+        shell.TerminalSessionOpened += (_, _) => requests++;
+
+        await vault.ConnectCommand.ExecuteAsync(null);
+        requests.ShouldBe(1);
+
+        await shell.LockCommand.ExecuteAsync(null);
+
+        shell.Vault.ShouldBeNull();
+
+        // The discarded vault is detached, so even a late raise from it reaches nobody.
+        await vault.ConnectCommand.ExecuteAsync(null);
+
+        requests.ShouldBe(1);
+    }
+
     [Fact]
     public async Task AnInvalidHost_IsRefusedWithAReason()
     {
@@ -569,4 +676,17 @@ public sealed class ShellFlowTests : IAsyncLifetime
 
         await vault.SaveHostCommand.ExecuteAsync(null);
     }
+
+    /// <summary>An unlocked vault with one selected host and a renderer attached.</summary>
+    private async Task<VaultViewModel> ReadyToConnectAsync()
+    {
+        await UnlockedAsync();
+
+        var vault = shell.Vault!;
+
+        await AddHostAsync(vault, "prod-db");
+        vault.SelectedHost = vault.Hosts[0];
+
+        return vault;
+    }
 }