DodoTech-Public/DodoSSH
Public Access
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Ask before deleting, and connect a host by double-clicking it

Browse Source 
DELETE on a host, an SSH key, a stored password or a file on the host now puts
a question where the button was, and only answering it deletes anything. It is
a state rather than a dialog, which is the arrangement signing out already had
and for the same reason: this is the moment that has to be able to say what is
about to go before it goes.

What the question says is counted rather than generic, because a confirmation
that only asks whether you are sure is a click to train people out of. A key
names the hosts that authenticate with it and says they will refuse to connect
afterwards rather than falling back to a typed password, which is what the
connect path actually does. A host discloses a terminal open on it, because
deleting the host does not close the session. Every vault deletion says how far
it travels and whether this machine can push the tombstone yet or is queuing
it. Deleting on the host carries the strongest warning of the four on purpose:
everything else here is a tombstone against a copy the server still holds, and
a file on somebody's machine is bytes with nothing behind them — so that one
names the full path, since a bare name identifies nothing.

The armed request carries the item's entity id, so nothing that moves the
selection between the question and the answer can redirect it, and answering
about something that has since gone says so instead of doing nothing quietly.
Disarming compares ids rather than rows, which is the subtle half: a reload
replaces every row object, so the naive rule would have let the pass that runs
every minute take the card away from somebody halfway through reading it.

Forgetting a pinned host key is deliberately still unguarded. It costs one
fingerprint check on the next connection and it is the safe direction to be
wrong in — the dangerous button there is the one that adds trust, and that one
is already a prompt at connect time. Discarding a stopped transfer is likewise
unguarded: it removes a resumable part file and leaves the source alone.

Double-clicking a host in the sidebar connects to it, wired as a gesture in the
control exactly as the transfers screen opens a directory. CONNECT stays, since
it is the button with the password box beside it.

Ten existing delete call sites now go through arm-and-confirm helpers, and
eight new flow tests cover asking first, cancelling, the counted warning,
disarming on a selection change and on an editor opening, surviving a sync, and
the stale-item guard. Three layout tests measure the new shapes — the sidebar
card is the one card in the application a user cannot scroll — and one of them
also asserts the card renders its text, because a card whose compiled bindings
did not resolve would lay out perfectly as empty rows. The double-click test
performs the real gesture and proves it reached the connect command through a
refusal that never touches a network.

dotnet build, dotnet test and dotnet format --verify-no-changes are all clean:
853 tests, including the end-to-end suite against real containers.
This commit is contained in:
jaap-jan
2026-07-31 11:52:13 +02:00
parent 240aadb746
commit 91438fb382
11 changed files with 979 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs
+259 -10
View File
@@ -729,7 +729,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await AddHostAsync(vault, "prod-db");
vault.SelectedHost = vault.Hosts[0];
await vault.DeleteHostCommand.ExecuteAsync(null);
await DeleteSelectedHostAsync(vault);
vault.Hosts.ShouldBeEmpty();
@@ -739,6 +739,219 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.PendingChanges.ShouldBe(0);
}
// ---- The question in front of a deletion ----
/// <remarks>
/// The half that makes the confirmation worth having: pressing DELETE has to change nothing at all. A
/// card that appeared after the item had already gone would be a receipt, not a question.
/// </remarks>
[Fact]
public async Task DeletingAHost_AsksFirstAndChangesNothingUntilItIsAnswered()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
vault.SelectedHost = vault.Hosts[0];
vault.DeleteHostCommand.Execute(null);
var question = vault.PendingDeletion.ShouldNotBeNull();
question.Question.ShouldContain("prod-db", Case.Insensitive);
vault.IsConfirmingDeletion.ShouldBeTrue();
vault.ShowsHostActions.ShouldBeFalse("the buttons are what the question replaces");
vault.Hosts.ShouldHaveSingleItem();
server.LiveRowCount.ShouldBe(1);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Hosts.ShouldBeEmpty();
vault.PendingDeletion.ShouldBeNull("the question goes when it is answered");
vault.ShowsHostActions.ShouldBeTrue();
}
[Fact]
public async Task CancellingADeletion_LeavesTheItemWhereItWas()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy");
vault.SelectedCredential = vault.Credentials[0];
vault.DeleteCredentialCommand.Execute(null);
vault.CancelDeleteCommand.Execute(null);
vault.PendingDeletion.ShouldBeNull();
// And the answer that would have deleted it has nothing left to act on.
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Credentials.ShouldHaveSingleItem();
server.LiveRowCount.ShouldBe(1);
}
/// <remarks>
/// What the question is for. A key that two hosts authenticate with is not the same deletion as one
/// nothing uses, and the hosts do not fall back to a typed password when it goes — they refuse, which is
/// asserted from the connect path's side in
/// <see cref="AHostWhoseKeyHasBeenDeleted_RefusesRatherThanFallingBackToThePassword"/>.
/// </remarks>
[Fact]
public async Task TheQuestionAboutAKey_CountsTheHostsThatAuthenticateWithIt()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddKeyAsync(vault, "deploy");
var keyId = vault.Keys[0].EntityId;
await AddHostAsync(vault, "prod-db");
await AddHostAsync(vault, "prod-web");
await BindKeyAsync(vault, Host(vault, "prod-db"), keyId);
await BindKeyAsync(vault, Host(vault, "prod-web"), keyId);
vault.SelectedKey = vault.Keys[0];
vault.DeleteKeyCommand.Execute(null);
var question = vault.PendingDeletion.ShouldNotBeNull();
question.HasUsage.ShouldBeTrue();
question.Usage.ShouldContain("2 hosts");
question.Usage.ShouldContain("prod-db");
question.Usage.ShouldContain("prod-web");
// And the sentence above it says how far the deletion travels, which needs no host at all.
question.Consequence.ShouldContain("no undo", Case.Insensitive);
}
/// <remarks>
/// A key nothing uses gets no scare line, which is the other half of counting: a warning that appeared
/// every time would say nothing the second time.
/// </remarks>
[Fact]
public async Task TheQuestionAboutAKeyNothingUses_SaysNothingAboutHosts()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddKeyAsync(vault, "spare");
await AddHostAsync(vault, "prod-db");
vault.SelectedKey = vault.Keys[0];
vault.DeleteKeyCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull().HasUsage.ShouldBeFalse();
}
/// <remarks>
/// The failure this guards against is a question answered about something else: arm the deletion, click
/// another row, press the button that is still on screen. The armed item is what the answer acts on, and
/// choosing a different one takes the question away rather than re-aiming it.
/// </remarks>
[Fact]
public async Task ChoosingSomethingElse_TakesTheQuestionAway()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy");
await AddCredentialAsync(vault, "staging deploy");
await vault.LoadAsync(Token);
vault.Section = VaultSection.Credentials;
vault.SelectedVaultItem = vault.VaultItems[0];
vault.DeleteCredentialCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull();
vault.SelectedVaultItem = vault.VaultItems[1];
vault.PendingDeletion.ShouldBeNull();
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Credentials.Count.ShouldBe(2, "nothing was agreed to");
}
/// <remarks>
/// The case the naive rule got wrong. A reload replaces every row object in the list, so disarming on
/// any change of the selected <em>row</em> would let the pass that runs every minute take the card away
/// from somebody halfway through reading it. The entity id is what the rule compares.
/// </remarks>
[Fact]
public async Task ASyncUnderneathAnArmedQuestion_LeavesItAlone()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
vault.SelectedHost = vault.Hosts[0];
vault.DeleteHostCommand.Execute(null);
var armed = vault.PendingDeletion.ShouldNotBeNull();
await vault.SyncCommand.ExecuteAsync(null);
await vault.LoadAsync(Token);
vault.PendingDeletion.ShouldBe(armed);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Hosts.ShouldBeEmpty();
}
/// <remarks>
/// Opening an editor is the other way the pane the question is in stops being about the question: the
/// vault screen's Add buttons stay on screen beside the detail pane, so a password editor can open over
/// an armed deletion. It disarms rather than stacking two forms in a 244-pixel column.
/// </remarks>
[Fact]
public async Task OpeningAnEditor_TakesTheQuestionAway()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddCredentialAsync(vault, "prod deploy");
vault.SelectedCredential = vault.Credentials[0];
vault.DeleteCredentialCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull();
vault.NewCredentialCommand.Execute(null);
vault.IsEditingCredential.ShouldBeTrue();
vault.PendingDeletion.ShouldBeNull();
}
/// <remarks>
/// An answer to a question about something that has since gone — the realistic way being a pass that
/// pulled somebody else's deletion. The reload that brings that news normally moves the selection and
/// takes the question with it; this holds the guard behind that, which is what keeps a stale agreement
/// from being a silent no-op under a card that has just been pressed.
/// </remarks>
[Fact]
public async Task AnsweringAboutSomethingAlreadyGone_SaysSo()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddKeyAsync(vault, "deploy");
vault.SelectedKey = vault.Keys[0];
vault.DeleteKeyCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull();
// Underneath the question, as another machine's deletion would arrive.
await vault.Session.SshKeys.DeleteAsync(
vault.Session.ActiveVaultId, vault.Keys[0].EntityId, Token);
await vault.LoadAsync(Token);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Status.ShouldContain("no longer here");
}
[Fact]
public async Task SyncingWhileOffline_QueuesRatherThanFailing()
{
@@ -1005,7 +1218,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await AddKeyAsync(vault, "deploy");
vault.SelectedKey = vault.Keys[0];
await vault.DeleteKeyCommand.ExecuteAsync(null);
await DeleteSelectedKeyAsync(vault);
vault.Keys.ShouldBeEmpty();
server.LiveRowCount.ShouldBe(0);
@@ -1387,7 +1600,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await BindKeyAsync(vault, vault.Hosts[0], vault.Keys[0].EntityId);
vault.SelectedKey = vault.Keys[0];
await vault.DeleteKeyCommand.ExecuteAsync(null);
await DeleteSelectedKeyAsync(vault);
vault.Keys.ShouldBeEmpty();
vault.SelectedHost = vault.Hosts[0];
@@ -1412,7 +1625,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await BindKeyAsync(vault, vault.Hosts[0], keyId);
vault.SelectedKey = vault.Keys[0];
await vault.DeleteKeyCommand.ExecuteAsync(null);
await DeleteSelectedKeyAsync(vault);
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
@@ -1541,7 +1754,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await AddCredentialAsync(vault, "prod deploy");
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
await DeleteSelectedCredentialAsync(vault);
vault.Credentials.ShouldBeEmpty();
server.LiveRowCount.ShouldBe(0);
@@ -1573,7 +1786,12 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.SelectedCredential.ShouldBeNull();
await vault.DeleteCredentialCommand.ExecuteAsync(null);
// Explicitly rather than through the helper: with nothing selected there is nothing to ask about,
// and the absence of a question is what proves the button found nothing to aim at.
vault.DeleteCredentialCommand.Execute(null);
vault.PendingDeletion.ShouldBeNull();
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Credentials.ShouldHaveSingleItem();
}
@@ -1594,7 +1812,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
// And a kind with nothing in it is left out rather than reported as zero.
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
await DeleteSelectedCredentialAsync(vault);
await vault.LoadAsync(Token);
vault.Status.ShouldBe("1 host(s), 1 key(s) in Personal.");
@@ -1725,7 +1943,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await BindCredentialAsync(vault, vault.Hosts[0], vault.Credentials[0].EntityId);
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
await DeleteSelectedCredentialAsync(vault);
vault.Credentials.ShouldBeEmpty();
vault.SelectedHost = vault.Hosts[0];
@@ -1747,7 +1965,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
await BindCredentialAsync(vault, vault.Hosts[0], credentialId);
vault.SelectedCredential = vault.Credentials[0];
await vault.DeleteCredentialCommand.ExecuteAsync(null);
await DeleteSelectedCredentialAsync(vault);
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
@@ -1900,7 +2118,7 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.KnownHostPins.ShouldHaveSingleItem().IsDialledByAHost.ShouldBeTrue();
vault.SelectedHost = vault.Hosts[0];
await vault.DeleteHostCommand.ExecuteAsync(null);
await DeleteSelectedHostAsync(vault);
vault.KnownHostPins.ShouldHaveSingleItem().IsDialledByAHost.ShouldBeFalse(
"the pin outlives the host, and the list has to admit it");
@@ -2673,6 +2891,37 @@ public sealed class ShellFlowTests : IAsyncLifetime
await vault.SaveKeyCommand.ExecuteAsync(null);
}
/// <summary>The host row with a given name, which the list orders by label rather than by age.</summary>
private static HostRowViewModel Host(VaultViewModel vault, string label) =>
vault.Hosts.Single(row => string.Equals(row.Label, label, StringComparison.Ordinal));
/// <summary>Deletes the selected host: the question, and then the answer to it.</summary>
/// <remarks>
/// Both halves, because both are what deleting anything now takes — arming on its own changes nothing,
/// which is what <c>DeletingAHost_AsksFirstAndChangesNothingUntilItIsAnswered</c> holds it to. Tests
/// about something else go through these three helpers, so the two-step is spelled out in one place
/// rather than in ten.
/// </remarks>
private static async Task DeleteSelectedHostAsync(VaultViewModel vault)
{
vault.DeleteHostCommand.Execute(null);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
}
/// <inheritdoc cref="DeleteSelectedHostAsync" />
private static async Task DeleteSelectedKeyAsync(VaultViewModel vault)
{
vault.DeleteKeyCommand.Execute(null);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
}
/// <inheritdoc cref="DeleteSelectedHostAsync" />
private static async Task DeleteSelectedCredentialAsync(VaultViewModel vault)
{
vault.DeleteCredentialCommand.Execute(null);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
}
/// <summary>Points a host at a key through the editor, the way a user would.</summary>
private static Task BindKeyAsync(VaultViewModel vault, HostRowViewModel host, Guid keyId) =>
BindAsync(vault, host, AuthenticationKind.SshKey, keyId);