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

Move the shelf as well as what is on it, and ask what a deletion takes

Browse Source 
Two things about a group, and they turn out to be the same argument twice.

**A group can be moved to another vault, and it takes everything on it.** The
host's move shipped last week and stopped one level too low: moving twenty
machines into a shared vault meant twenty trips through a menu, and each one
arrived stripped of the group it had been filed under, so the shelf had to be
rebuilt by hand on the other side. Moving the shelf is what people were
attempting. MOVE sits between EDIT and DELETE over the group cards, and on the
card's own menu above the separator DELETE is below — the same place, and the
same reasoning, as the host pane's ⋯ entry.

**The whole subtree goes, and taking less was never coherent.** A group's
children are items of the vault it is leaving, so a parent moved alone leaves
them naming a tombstone and they surface as roots in the vault the user has just
emptied: half a shelf here and half there, from one gesture that said "move
this". The hosts are the same argument and are the half the request was about.

The groups go first, top down, and the hosts last. Each item is re-sealed under
the destination's key and takes a new id — VaultItemRepository.MoveAsync, which
HostGroupRepository now exposes — so nothing pointing at a group can be written
until that group has landed and its new id is known, and a child's parent must
already be over there. What an interruption leaves is therefore hosts still in
the vault they started in, under UNGROUPED: visible, and re-movable. The reverse
order would leave hosts in the destination filed under nothing.

The parent stays behind and the tags are dropped, which is the host move's rule
one level up: both are items of the vault being left, so a reference carried
across would resolve on the machine that moved it and dangle for everybody else
in the destination. The moved group arrives at the top level, and the panel says
so before the press rather than the status line saying it after. Keys and
passwords are kept — those genuinely resolve across vaults, and clearing them
would take a working host and make one that cannot connect — and any now outside
the destination is named, because that is precisely what the other members of it
will not be able to resolve.

Refused as a whole where anything under the group was written by a newer client,
rather than skipped item by item: a move that left behind what it could not
re-encode would file some of the shelf in one vault and the rest in the other,
which is the state this exists to prevent. Refused with a host editor open, as
the drop gesture is, because it rewrites hosts. And the walk carries a visited
set, for the reason every walk over this tree does: a group that is its own
parent — which two offline clients can build and no editor was ever shown —
would otherwise be appended to the move list for as long as there was memory.

**Deleting a group now asks what should become of the hosts under it, and that
reverses a decision this repository had written down.** The deletion did not
touch them: the reference was left dangling, the list resolved it to nothing,
and the machines turned up under UNGROUPED. That was right for one of the two
things people delete a group for and wrong for the other — a heading being tidied
away should leave its machines alone, and a project that has been decommissioned
is a shelf and everything on it — and nothing in the code can tell which of the
two it is looking at. So it is asked.

A tick rather than a pair of options, because the two answers are not equally
weighted: keeping the hosts is recoverable and deleting them is not, so the safe
answer is the one that needs no decision. It is off on every question, including
the one that disarms it, or a tick left standing would destroy the next group's
machines on the strength of a decision about the last one's.

Once the deletion knows which hosts it means, leaving them naming something that
has gone is a state kept for no reason, so the unticked answer writes too: N
hosts with the reference cleared, where the ticked one writes N tombstones. That
is the N writes HostGroupRepository refuses to hide behind a DeleteAsync
overload, made where somebody asked for them and where the count is on screen
first. The nested groups take the deleted group's place in the tree rather than
being orphaned to the top level. A read-only host is skipped, counted and named,
because unfiling it would re-encode a payload this build cannot represent — and
the cost of skipping is a dangling id, which every reader here already survives.

**Both are the desktop's alone**, and that is not an omission. The phone draws
groups as headings in the host list and has never had a way to delete or move
one; the two panels take the row of buttons over the group cards, and there is
no such row on a 360dp screen to take.

One test had to change its premise rather than its assertion.
EditingAHostWhoseGroupIsGone built its dangling reference by deleting the group,
which now unfiles instead — so it imports a host naming an id nothing resolves,
which is what a group deleted on another machine actually looks like and is the
only way that state still arises. The picker's placeholder is still needed and
still covered.

Four places said an item could not be moved between vaults. Two were about a
group and were true when written; the other two were left stale by the host's
move. All four now say what is true, including the design gaps document, where
the chevron beside the vault name stays undrawn for the reason it already had.
This commit is contained in:
jaap-jan
2026-08-04 17:04:06 +02:00
parent 742f65c204
commit a86731ee08
10 changed files with 1125 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs
+130 -21
View File
@@ -3801,13 +3801,20 @@ public sealed class ShellFlowTests : IAsyncLifetime
}
/// <remarks>
/// Deleting a group deliberately does not rewrite the hosts in it — one delete would otherwise become N
/// writes, N outbox rows and N chances to merge against a change nobody made — so those hosts keep an id
/// that resolves to nothing. "The group is gone" and "this host is in no group" have to look the same,
/// because to the person reading the list they are the same thing.
/// <para>
/// Deleting a group leaves the machines under it alone <em>and</em> stops them naming it. It used to do
/// only the first: the reference was left dangling and the list resolved it to nothing, which looked
/// identical and cost no writes. The tick is what changed that — a deletion that can take the hosts with
/// it has to be a deletion that knows which hosts it means, and once it knows, leaving them holding the
/// id of something that has gone is a state kept for no reason.
/// </para>
/// <para>
/// The tick is deliberately not touched here, which is the point of the assertions: the default answer
/// is the one that keeps the machines.
/// </para>
/// </remarks>
[Fact]
public async Task DeletingAGroup_LeavesItsHostsUnderTheUngroupedHeading()
public async Task DeletingAGroup_UnfilesItsHostsRatherThanLeavingThemNamingIt()
{
await UnlockedAsync();
var vault = shell.Vault!;
@@ -3816,32 +3823,136 @@ public sealed class ShellFlowTests : IAsyncLifetime
await AddGroupAsync(vault, "production");
await FileAsync(vault, "prod-db", "production");
var groupId = vault.Groups.Single().EntityId;
vault.SelectedGroup = vault.Groups.Single();
vault.DeleteGroupCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull().Usage
var question = vault.PendingDeletion.ShouldNotBeNull();
question.Usage
.ShouldContain("1 host", Case.Sensitive, "the count is what makes the question worth reading");
question.HasChoice.ShouldBeTrue("a group with a host under it has a second question");
vault.DeletionTakesTheHostsToo.ShouldBeFalse("the safe answer is the one nobody has to choose");
// The pass that follows every write on this screen reports what it moved and supersedes the
// confirmation, for a deletion as much as for a save — so it is made to fail, and what the sentence
// says is asserted in the state where somebody actually reads it.
server.SyncFailure = new HttpRequestException("The server is having a bad day.");
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Groups.ShouldBeEmpty();
vault.HasGroups.ShouldBeFalse();
// The host keeps the id, which is what makes this cheap; the list is what resolves it to nothing.
vault.Hosts.Single().Host.GroupId.ShouldBe(groupId);
vault.Hosts.Single().Host.GroupId.ShouldBeNull(vault.Status);
vault.Hosts.Single().GroupLabel.ShouldBeEmpty();
vault.SidebarRows.ShouldAllBe(row => row is HostRowViewModel);
// The card says the same thing the phone's list does: nothing. An id nobody can name is drawn as no
// group rather than as a GUID on a chip.
vault.Hosts.Single().GroupLabel.ShouldBeEmpty();
vault.Status.ShouldContain("UNGROUPED", Case.Sensitive);
}
/// <remarks>
/// The picker keeps a placeholder entry for a group the vault no longer has, exactly as the
/// The other answer, and the reason the question is asked at all: a group is sometimes a heading being
/// tidied away and sometimes a project that has been decommissioned, and nothing in the view model can
/// tell which of the two it is looking at.
/// </remarks>
[Fact]
public async Task DeletingAGroupWithTheTickSet_TakesItsHostsWithIt()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddHostAsync(vault, "prod-web");
await AddGroupAsync(vault, "production");
await FileAsync(vault, "prod-db", "production");
vault.SelectedGroup = vault.Groups.Single();
vault.DeleteGroupCommand.Execute(null);
vault.PendingDeletion.ShouldNotBeNull().Choice.ShouldContain("host");
vault.DeletionTakesTheHostsToo = true;
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Groups.ShouldBeEmpty();
// Only the machine that was filed under it. A deletion aimed at a heading must not reach the hosts
// that were never on it.
vault.Hosts.Select(row => row.Label).ShouldBe(["prod-web"], vault.Status);
}
/// <remarks>
/// The answer is not carried from one question to the next. A tick left standing would delete the next
/// group's machines on the strength of a decision about the last one's, and there is no undo on either
/// side of that.
/// </remarks>
[Fact]
public async Task AskingAboutASecondGroup_StartsFromKeepingItsHosts()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddGroupAsync(vault, "production");
await AddGroupAsync(vault, "staging");
await FileAsync(vault, "prod-db", "production");
vault.SelectedGroup = vault.Groups.Single(
row => string.Equals(row.Label, "production", StringComparison.Ordinal));
vault.DeleteGroupCommand.Execute(null);
vault.DeletionTakesTheHostsToo = true;
vault.CancelDeleteCommand.Execute(null);
vault.SelectedGroup = vault.Groups.Single(
row => string.Equals(row.Label, "staging", StringComparison.Ordinal));
vault.DeleteGroupCommand.Execute(null);
vault.DeletionTakesTheHostsToo.ShouldBeFalse("every question starts from keeping the machines");
}
/// <remarks>
/// Unfiling rewrites every host under the heading, so it is refused with a host editor open for the
/// reason a drop onto a group card is: rewriting the saved host under a half-typed edit of it would be a
/// save nobody asked for, and one they could then not cancel. Deleting a single host is not refused,
/// because that one writes nothing to a form anybody is looking at.
/// </remarks>
[Fact]
public async Task DeletingAGroupWhileTheHostEditorIsOpen_IsRefused()
{
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddGroupAsync(vault, "production");
await FileAsync(vault, "prod-db", "production");
vault.SelectedHost = vault.Hosts.Single();
vault.EditSelectedHostCommand.Execute(null);
vault.EditorLabel = "half-typed";
vault.SelectedGroup = vault.Groups.Single();
vault.DeleteGroupCommand.Execute(null);
vault.PendingDeletion.ShouldBeNull("the question was never put");
vault.IsEditing.ShouldBeTrue("and the edit is still there to finish");
vault.Status.ShouldContain("editing");
}
/// <remarks>
/// <para>
/// The picker keeps a placeholder entry for a group the vault does not have, exactly as the
/// authentication picker does for a deleted key. Without it the picker would open on "No group" and
/// somebody editing the host's port would unfile it by saving.
/// </para>
/// <para>
/// The dangling id is imported rather than produced by deleting the group, and that is a consequence of
/// the change above rather than a contrivance: a group deleted <em>here</em> now unfiles its hosts on the
/// way out, so the only way a host still names one is that the group went on another machine and this
/// client has yet to be told — which is exactly what a host arriving with an id nothing resolves is.
/// </para>
/// </remarks>
[Fact]
public async Task EditingAHostWhoseGroupIsGone_DoesNotUnfileItBySaving()
@@ -3849,15 +3960,13 @@ public sealed class ShellFlowTests : IAsyncLifetime
await UnlockedAsync();
var vault = shell.Vault!;
await AddHostAsync(vault, "prod-db");
await AddGroupAsync(vault, "production");
await FileAsync(vault, "prod-db", "production");
var groupId = Guid.CreateVersion7();
var groupId = vault.Groups.Single().EntityId;
await vault.ImportHostsAsync(
[new HostSecret { Label = "prod-db", Hostname = "db.internal", GroupId = groupId }],
Token);
vault.SelectedGroup = vault.Groups.Single();
vault.DeleteGroupCommand.Execute(null);
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Groups.ShouldBeEmpty("nothing in this keychain answers to that id");
vault.SelectedHost = vault.Hosts.Single();
vault.EditSelectedHostCommand.Execute(null);
tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs
+149
View File
@@ -875,6 +875,155 @@ public sealed class VaultSharingTests : IAsyncLifetime
.EntityId.ShouldBeNull("only 'no parent' is left once the group is going somewhere else");
}
/// <remarks>
/// <para>
/// Moving the shelf rather than what is on it, which is the operation people were attempting one host at
/// a time: a group cannot go anywhere alone, because the machines filed under it and the groups nested
/// inside it are items of the vault it is leaving. All of them are re-sealed under the destination's key
/// and all of them take new ids, so what this asserts is not only that they arrived but that the tree
/// arrived — the child is still under the parent, and the host is still under the child, through two
/// levels of ids that were rewritten on the way across.
/// </para>
/// <para>
/// The parent the moved group was nested under is asserted <em>gone</em>, and that is the honest half. A
/// parent belongs to the vault it is in, so carrying the reference would leave everybody else in the
/// destination looking at a group hanging from nothing. It arrives at the top level and the sentence
/// says so.
/// </para>
/// </remarks>
[Fact]
public async Task MovingAGroupToAnotherVault_TakesItsHostsAndItsNestedGroupsWithIt()
{
await UnlockedAsync();
var vaults = shell.Vaults;
await CreateVaultAsync(vaults, "Platform secrets");
var vault = shell.Vault!;
var sharedVaultId = vaults.SelectedVault!.VaultId;
await vault.LoadAsync(Token);
await SeedNestedShelfAsync(vault);
var production = Named(vault, "production");
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");
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 =
vault.MoveGroupVaultChoices.Single(choice => choice.VaultId == sharedVaultId);
// The pass that follows every write on this screen is made to fail, so that the move's own sentence
// is still on the status line to be read — the same arrangement, and for the same reason, as the
// host's move test above.
server.SyncFailure = new IOException("The server is not answering.");
await vault.ConfirmMoveGroupCommand.ExecuteAsync(null);
var moved = Named(vault, "production");
var nested = Named(vault, "web");
moved.VaultId.ShouldBe(sharedVaultId, vault.Status);
moved.EntityId.ShouldNotBe(production.EntityId, "an id belongs to one vault");
moved.Group.ParentId.ShouldBeNull("a parent belongs to the vault the group came from");
nested.VaultId.ShouldBe(sharedVaultId, "a group inside it cannot be left in the other vault");
nested.Group.ParentId.ShouldBe(moved.EntityId, "and it is still nested under the group it was in");
var host = vault.Hosts.Single(row => string.Equals(row.Label, "prod-db", StringComparison.Ordinal));
host.VaultId.ShouldBe(sharedVaultId, "the hosts came with the shelf");
host.Host.GroupId.ShouldBe(nested.EntityId, "and are still filed where they were");
// The group it was nested under is the one thing that stayed, and it stayed where it was.
Named(vault, "estate").VaultId.ShouldBe(production.VaultId);
vault.SelectedGroup?.EntityId.ShouldBe(moved.EntityId, "the buttons follow the group they moved");
vault.Status.ShouldContain("Platform secrets");
vault.Status.ShouldContain("top level");
}
/// <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.
/// </remarks>
[Fact]
public async Task MovingAGroupWithNowhereToMoveIt_SaysSoRatherThanOpeningAnEmptyPicker()
{
await UnlockedAsync();
var vault = shell.Vault!;
await vault.LoadAsync(Token);
vault.NewGroupCommand.Execute(null);
vault.GroupEditorLabel = "production";
await vault.SaveGroupCommand.ExecuteAsync(null);
vault.SelectedGroup = vault.Groups.ShouldHaveSingleItem();
vault.CanMoveGroupTarget.ShouldBeFalse("the personal vault is the only one there is");
vault.MoveGroupCommand.Execute(null);
vault.IsMovingGroup.ShouldBeFalse();
vault.MoveGroupVaultChoices.ShouldBeEmpty();
vault.Status.ShouldContain("only vault you can write to");
}
/// <summary>The group card with a given name, re-found because every row is replaced on every reload.</summary>
private static HostGroupRowViewModel Named(VaultViewModel vault, string label) =>
vault.Groups.Single(row => string.Equals(row.Label, label, StringComparison.Ordinal));
/// <summary>
/// Builds estate › production › web in the personal vault, with prod-db on the innermost shelf.
/// </summary>
/// <remarks>
/// Three levels, because two would not tell a subtree that was walked from one that was assumed a single
/// level deep — the middle group is the one that has to arrive with a rewritten parent and a rewritten
/// child at once.
/// </remarks>
private static async Task SeedNestedShelfAsync(VaultViewModel vault)
{
await AddGroupAsync(vault, "estate", under: null);
await AddGroupAsync(vault, "production", under: "estate");
await AddGroupAsync(vault, "web", under: "production");
vault.NewHostCommand.Execute(null);
vault.EditorLabel = "prod-db";
vault.EditorHostname = "db.internal";
vault.EditorSelectedGroup = vault.EditorGroupChoices.Single(
choice => string.Equals(choice.Label, "web", StringComparison.Ordinal));
await vault.SaveHostCommand.ExecuteAsync(null);
}
/// <summary>Adds a group, optionally nested under one already there.</summary>
private static async Task AddGroupAsync(VaultViewModel vault, string label, string? under)
{
vault.NewGroupCommand.Execute(null);
vault.GroupEditorLabel = label;
if (under is not null)
{
vault.GroupEditorSelectedParent = vault.GroupEditorParentChoices.Single(
choice => string.Equals(choice.Label, under, StringComparison.Ordinal));
}
await vault.SaveGroupCommand.ExecuteAsync(null);
}
/// <remarks>
/// <para>
/// Dragging a host card onto a group card is the one gesture that files a host without opening its