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

Merge branch 'claude/delete-confirmations-becf4a'
ci / build and test (push) Failing after 2s
Details

Browse Source
This commit is contained in:
jaap-jan
2026-07-31 11:52:27 +02:00
parent 9608d73747 91438fb382
commit 1292084af9
11 changed files with 979 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/DodoSSH.Client.App/ViewModels/TransfersViewModel.cs
+101 -9
View File
@@ -174,6 +174,40 @@ internal sealed partial class TransferRowViewModel(TransferSnapshot snapshot) :
}
}
/// <summary>
/// Something on the host that has been asked about and not yet agreed to.
/// </summary>
/// <remarks>
/// <para>
/// The one deletion in this application that nothing can walk back. A vault item is a tombstone against a
/// copy the server still holds until the pass lands; a file on somebody's host is bytes, and this screen
/// has no wastebasket to put them in.
/// </para>
/// <para>
/// It carries the full path rather than only the name, because the name is the half that does not identify
/// anything: <c>config</c> in the directory that was showing a moment ago and <c>config</c> in the one
/// showing now look identical in a confirmation, and only one of them is the file somebody meant.
/// </para>
/// </remarks>
/// <param name="Name">What the row was called.</param>
/// <param name="FullPath">Where it is, which is what the question actually promises to delete.</param>
/// <param name="IsDirectory">Whether it is a directory, which the host treats differently.</param>
internal sealed record RemoteDeletionRequest(string Name, string FullPath, bool IsDirectory)
{
/// <summary>The question, naming the kind because the two behave differently.</summary>
internal string Question => IsDirectory
? $"Delete the directory '{Name}' on the host?"
: $"Delete '{Name}' on the host?";
/// <summary>What it costs, which is everything: there is no copy here and no undo there.</summary>
internal string Consequence => IsDirectory
? "It is removed on the host itself. The host refuses a directory that still has anything in it, so "
+ "this either removes an empty one or fails — and if it goes, it is gone: nothing here keeps a "
+ "copy and there is no undo."
: "It is removed on the host itself. Nothing here keeps a copy, the folder on this machine is not "
+ "touched, and there is no undo.";
}
/// <summary>
/// The transfers screen: a host, two directory panes, and the queue between them.
/// </summary>
@@ -284,6 +318,20 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
internal bool HasRemoteEntries => RemoteEntries.Count > 0;
/// <summary>The deletion on the host that has been asked about, or null when none has.</summary>
[ObservableProperty]
private RemoteDeletionRequest? pendingRemoteDeletion;
internal bool IsConfirmingRemoteDeletion => PendingRemoteDeletion is not null;
/// <summary>Whether the pane's DELETE is live.</summary>
/// <remarks>
/// Off while its own question is up, so a second press cannot arm a second one behind the card — and
/// disabled rather than hidden, because this button sits in a row of three and a gap where it was would
/// move UP and REFRESH out from under the pointer.
/// </remarks>
internal bool CanDeleteRemote => IsConnected && !IsConfirmingRemoteDeletion;
// ---- The local pane ----
[ObservableProperty]
@@ -672,15 +720,16 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
}
/// <summary>
/// Deletes the chosen remote file, or an empty directory.
/// Asks whether the chosen remote file, or empty directory, should go.
/// </summary>
/// <remarks>
/// Not recursive, and the refusal comes from the server rather than from a check here — see
/// <c>ISftpSession.DeleteAsync</c>. It is offered because the queue refuses to overwrite: without a way
/// to remove what is in the way, "that file is already there" would be a dead end.
/// Deleting on the host is offered because the queue refuses to overwrite: without a way to remove what
/// is in the way, "that file is already there" would be a dead end. It is asked about first because of
/// what it is — the only thing this application destroys that neither the server nor this machine has a
/// copy of.
/// </remarks>
[RelayCommand]
private async Task DeleteRemoteAsync(CancellationToken cancellationToken)
private void DeleteRemote()
{
if (SelectedRemoteEntry is not { } row)
{
@@ -688,18 +737,44 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
return;
}
PendingRemoteDeletion = new RemoteDeletionRequest(row.Name, row.FullPath, !row.IsFile);
}
/// <summary>
/// Deletes what was agreed to.
/// </summary>
/// <remarks>
/// Not recursive, and the refusal comes from the server rather than from a check here — see
/// <c>ISftpSession.DeleteAsync</c>. It acts on the path the question named rather than on the selection,
/// which is what makes the question a promise: nothing between asking and answering can point it
/// somewhere else.
/// </remarks>
[RelayCommand]
private async Task ConfirmDeleteRemoteAsync(CancellationToken cancellationToken)
{
if (PendingRemoteDeletion is not { } request)
{
return;
}
PendingRemoteDeletion = null;
await RunAsync(
$"Deleting {row.Name}…",
$"Deleting {request.Name}…",
async () =>
{
await RequireSession().DeleteAsync(row.FullPath, cancellationToken).ConfigureAwait(true);
await RequireSession().DeleteAsync(request.FullPath, cancellationToken).ConfigureAwait(true);
await ListRemoteAsync(RemotePath, cancellationToken).ConfigureAwait(true);
Status = $"Deleted {row.Name}.";
Status = $"Deleted {request.Name}.";
}).ConfigureAwait(true);
}
/// <summary>Thinks better of it.</summary>
[RelayCommand]
private void CancelDeleteRemote() => PendingRemoteDeletion = null;
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
@@ -919,11 +994,28 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
{
OnPropertyChanged(nameof(CanDownload));
OnPropertyChanged(nameof(CanUpload));
OnPropertyChanged(nameof(CanDeleteRemote));
}
partial void OnSelectedRemoteEntryChanged(RemoteEntryRowViewModel? value) =>
/// <remarks>
/// Any change to the selection takes the question away, which is stricter than the vault's rule and can
/// afford to be: this list is refilled only by a navigation or a refresh somebody asked for, so there is
/// no background pass to pull a card out from under a reader. Listing and disconnecting both null the
/// selection, so this one hook covers all three ways the answer could stop being about what was asked.
/// </remarks>
partial void OnSelectedRemoteEntryChanged(RemoteEntryRowViewModel? value)
{
OnPropertyChanged(nameof(CanDownload));
PendingRemoteDeletion = null;
}
partial void OnPendingRemoteDeletionChanged(RemoteDeletionRequest? value)
{
OnPropertyChanged(nameof(IsConfirmingRemoteDeletion));
OnPropertyChanged(nameof(CanDeleteRemote));
}
partial void OnSelectedLocalEntryChanged(LocalEntryRowViewModel? value) =>
OnPropertyChanged(nameof(CanUpload));
src/DodoSSH.Client.App/ViewModels/VaultViewModel.cs
+302 -18
View File
@@ -422,6 +422,55 @@ internal sealed record VaultItemRowViewModel(
internal bool HasBadge => Badge.Length > 0;
}
/// <summary>Which list a deletion that has been asked for is aimed at.</summary>
internal enum DeletionTarget
{
/// <summary>A host, from the sidebar beside the terminal.</summary>
Host,
/// <summary>An SSH key, from the vault screen.</summary>
Key,
/// <summary>A stored password, from the vault screen.</summary>
Credential,
}
/// <summary>
/// A deletion that has been asked for and not yet agreed to.
/// </summary>
/// <remarks>
/// <para>
/// A state rather than a dialog, on the same reasoning as the sign-out confirmation — see
/// <c>MainWindowViewModel.IsConfirmingSignOut</c>. What makes it worth having at all is that the sentences
/// below are <em>computed</em>: how many hosts authenticate with the key about to go, whether a terminal is
/// open on the host about to go, and whether this machine can push the tombstone yet. A confirmation that
/// only said "are you sure?" would be a click to train people out of.
/// </para>
/// <para>
/// It carries the item's id rather than pointing at the selection, so that whatever moves the selection
/// between the question and the answer — a background sync, a filter, a click in the list — cannot turn an
/// agreement about one item into the deletion of another.
/// </para>
/// </remarks>
/// <param name="Target">Which list to delete from.</param>
/// <param name="EntityId">The item the question is about.</param>
/// <param name="Question">The question itself, naming the item.</param>
/// <param name="Consequence">Where it goes, and how far.</param>
/// <param name="Usage">
/// What is riding on this particular item — hosts that authenticate with it, a terminal open on it — or
/// empty when nothing is. The line that changes the answer, as opposed to the one every deletion shares.
/// </param>
internal sealed record DeletionRequest(
DeletionTarget Target,
Guid EntityId,
string Question,
string Consequence,
string Usage)
{
/// <summary>Whether anything depends on the item, which is the line worth reading twice.</summary>
internal bool HasUsage => Usage.Length > 0;
}
/// <summary>
/// Gets this machine online, if it can be.
/// </summary>
@@ -857,6 +906,32 @@ internal sealed partial class VaultViewModel(
/// <summary>The credential being edited, or null when creating.</summary>
private Guid? editingCredentialId;
// ---- Deleting ----
/// <summary>The deletion that has been asked for, or null when nothing has been.</summary>
/// <remarks>
/// One at a time, and one for all three kinds. Two armed deletions cannot be told apart by a user
/// looking at two cards, and this application only ever has one selected item per screen to aim a
/// question at.
/// </remarks>
[ObservableProperty]
private DeletionRequest? pendingDeletion;
internal bool IsConfirmingDeletion => PendingDeletion is not null;
/// <summary>Whether the sidebar's row of host buttons is showing.</summary>
/// <remarks>
/// Its own property because the markup cannot express <c>!IsEditing &amp;&amp; !IsConfirmingDeletion</c>,
/// and because both halves are the same rule: the question about deleting a host takes the place of the
/// buttons that asked it, so that DELETE cannot be pressed a second time while its own confirmation is
/// on screen.
/// </remarks>
internal bool ShowsHostActions => !IsEditing && !IsConfirmingDeletion;
/// <summary>Whether the vault screen's Edit and Delete are showing.</summary>
/// <inheritdoc cref="ShowsHostActions" />
internal bool ShowsItemActions => SelectedItemIsEditable && !IsConfirmingDeletion;
// ---- Connecting ----
/// <remarks>
@@ -1068,7 +1143,7 @@ internal sealed partial class VaultViewModel(
/// <returns>How many keys would not decrypt.</returns>
/// <remarks>
/// Unlike the host list, the selection is <em>not</em> defaulted to the first row: it is what
/// <see cref="DeleteKeyAsync" /> acts on, and a list that picked a row on every background sync would aim
/// <see cref="DeleteKey" /> aims at, and a list that picked a row on every background sync would point
/// that button at a key nobody chose.
/// </remarks>
private async Task<int> ReloadKeysAsync(CancellationToken cancellationToken)
@@ -1094,9 +1169,9 @@ internal sealed partial class VaultViewModel(
/// <returns>How many credentials would not decrypt.</returns>
/// <remarks>
/// An existing selection survives a reload and a reload never invents one, which is the same pair of rules
/// as the key list and matters more here. <see cref="DeleteCredentialAsync" /> acts on the selection, so a
/// list that fell back to its first row would put a one-click deletion of somebody's password behind a
/// button they never aimed.
/// as the key list and matters more here. <see cref="DeleteCredential" /> reads the selection, so a list
/// that fell back to its first row would point the deletion — and the question in front of it — at a
/// password nobody chose.
/// </remarks>
private async Task<int> ReloadCredentialsAsync(CancellationToken cancellationToken)
{
@@ -1475,26 +1550,23 @@ internal sealed partial class VaultViewModel(
}
}
/// <summary>Deletes whatever the selected row is.</summary>
/// <summary>Asks about deleting whatever the selected row is.</summary>
/// <remarks>
/// Pins are not deleted from here even though they can be. Withdrawing trust applies to an endpoint
/// rather than to a row — every pin for the address goes — and calling that "delete" beside two buttons
/// that remove exactly one item would misdescribe it. It has its own button, named for what it does.
/// </remarks>
[RelayCommand]
private async Task DeleteSelectedItemAsync()
private void DeleteSelectedItem()
{
switch (SelectedVaultItem?.Kind)
{
// Null rather than a token, and deliberately: a [RelayCommand] over a method whose only
// parameter is a CancellationToken generates ExecuteAsync(object? parameter) that ignores the
// argument and supplies a token from its own source. Passing one would read as plumbing.
case VaultItemKind.Key:
await DeleteKeyCommand.ExecuteAsync(null).ConfigureAwait(true);
DeleteKeyCommand.Execute(null);
break;
case VaultItemKind.Credential:
await DeleteCredentialCommand.ExecuteAsync(null).ConfigureAwait(true);
DeleteCredentialCommand.Execute(null);
break;
default:
@@ -1560,15 +1632,42 @@ internal sealed partial class VaultViewModel(
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
/// <summary>Queues a tombstone for the selected host.</summary>
/// <summary>Asks whether the selected host should go.</summary>
/// <remarks>
/// A terminal already open on the host is disclosed rather than prevented, because deleting a host does
/// not close one — a session outlives the row that opened it, exactly as it outlives a lock. Somebody
/// deleting a machine they are still working on should know that is what they have done.
/// </remarks>
[RelayCommand]
private async Task DeleteHostAsync(CancellationToken cancellationToken)
private void DeleteHost()
{
if (SelectedHost is not { } row)
{
return;
}
PendingDeletion = new DeletionRequest(
DeletionTarget.Host,
row.EntityId,
$"Delete the host '{row.Label}'?",
HowFarADeletionGoes("The host and everything saved about it"),
row.IsConnected
? "A terminal is open on this host. It stays open — deleting the host does not close it, and "
+ "nothing will reopen it afterwards."
: string.Empty);
}
/// <summary>Queues a tombstone for the host that was agreed to.</summary>
private async Task DeleteHostNowAsync(Guid entityId, CancellationToken cancellationToken)
{
if (Hosts.FirstOrDefault(row => row.EntityId == entityId) is not { } row)
{
// Gone between the question and the answer — a sync that pulled somebody else's deletion is the
// realistic way. Saying so beats a silent no-op under a card that has just been agreed to.
Status = "That host is no longer here, so nothing was deleted.";
return;
}
await RunAsync(
"Deleting…",
async () =>
@@ -1687,15 +1786,39 @@ internal sealed partial class VaultViewModel(
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
/// <summary>Queues a tombstone for the selected key.</summary>
/// <summary>Asks whether the selected key should go.</summary>
/// <remarks>
/// The private key is the thing this vault holds that is least likely to exist anywhere else, which is
/// why the question says so. What it does not say is that the key is gone from the machines it was
/// installed on: deleting it here removes this vault's copy, and the <c>authorized_keys</c> file on a
/// server is not something this application has ever written to.
/// </remarks>
[RelayCommand]
private async Task DeleteKeyAsync(CancellationToken cancellationToken)
private void DeleteKey()
{
if (SelectedKey is not { } row)
{
return;
}
PendingDeletion = new DeletionRequest(
DeletionTarget.Key,
row.EntityId,
$"Delete the SSH key '{row.Label}'?",
HowFarADeletionGoes("The private key, its passphrase and everything saved with them")
+ " If this key is not on disk anywhere else, this is the only copy.",
HostsBoundTo(host => host.SshKeyId, row.EntityId));
}
/// <summary>Queues a tombstone for the key that was agreed to.</summary>
private async Task DeleteKeyNowAsync(Guid entityId, CancellationToken cancellationToken)
{
if (Keys.FirstOrDefault(row => row.EntityId == entityId) is not { } row)
{
Status = "That key is no longer here, so nothing was deleted.";
return;
}
await RunAsync(
"Deleting…",
async () =>
@@ -1812,15 +1935,32 @@ internal sealed partial class VaultViewModel(
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
/// <summary>Queues a tombstone for the selected credential.</summary>
/// <summary>Asks whether the selected credential should go.</summary>
[RelayCommand]
private async Task DeleteCredentialAsync(CancellationToken cancellationToken)
private void DeleteCredential()
{
if (SelectedCredential is not { } row)
{
return;
}
PendingDeletion = new DeletionRequest(
DeletionTarget.Credential,
row.EntityId,
$"Delete the password '{row.Label}'?",
HowFarADeletionGoes("The password and the account saved with it"),
HostsBoundTo(host => host.CredentialId, row.EntityId));
}
/// <summary>Queues a tombstone for the credential that was agreed to.</summary>
private async Task DeleteCredentialNowAsync(Guid entityId, CancellationToken cancellationToken)
{
if (Credentials.FirstOrDefault(row => row.EntityId == entityId) is not { } row)
{
Status = "That password is no longer here, so nothing was deleted.";
return;
}
await RunAsync(
"Deleting…",
async () =>
@@ -1836,6 +1976,92 @@ internal sealed partial class VaultViewModel(
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
/// <summary>Carries out the deletion that was asked about.</summary>
/// <remarks>
/// Disarmed before the work rather than after it, so that the card goes the moment it is answered and a
/// second press during a slow round trip has nothing left to agree to.
/// </remarks>
[RelayCommand]
private async Task ConfirmDeleteAsync(CancellationToken cancellationToken)
{
if (PendingDeletion is not { } request)
{
return;
}
PendingDeletion = null;
switch (request.Target)
{
case DeletionTarget.Host:
await DeleteHostNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true);
break;
case DeletionTarget.Key:
await DeleteKeyNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true);
break;
case DeletionTarget.Credential:
await DeleteCredentialNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true);
break;
default:
break;
}
}
/// <summary>Thinks better of it.</summary>
[RelayCommand]
private void CancelDelete() => PendingDeletion = null;
/// <summary>
/// Where a deleted item goes, and how far.
/// </summary>
/// <remarks>
/// The offline branch is the same distinction saving makes, and it matters more here: a tombstone that
/// has not been pushed is a deletion the other machines have not heard about, and somebody deleting a
/// credential because it leaked should be told which of those two they have just done.
/// </remarks>
private string HowFarADeletionGoes(string what) => connection() is null
? $"{what} goes from this machine now, and from your other machines once this one is online again. "
+ "There is no undo."
: $"{what} goes from this machine now, and from your other machines at the next synchronisation. "
+ "There is no undo.";
/// <summary>
/// What the hosts that authenticate with an item would be left with, or nothing when none do.
/// </summary>
/// <remarks>
/// Counted rather than warned about in general terms. The number is the difference between a sentence
/// somebody reads and one they click past, and what happens next is worth stating exactly: a host bound
/// to something the vault no longer has is refused at connect time rather than quietly falling back to a
/// typed password — see <see cref="TryBuildAuthentication" />.
/// </remarks>
private string HostsBoundTo(Func<HostSecret, Guid?> binding, Guid entityId)
{
var bound = Hosts
.Where(row => binding(row.Host) == entityId)
.Select(row => row.Label)
.ToArray();
if (bound.Length == 0)
{
return string.Empty;
}
// Three names and a count past that, because this is read in a 244-pixel column and a vault with
// twenty hosts on one key would otherwise put a paragraph of names where a warning should be.
var named = bound.Length <= 3
? string.Join(", ", bound)
: $"{string.Join(", ", bound.Take(3))} and {bound.Length - 3} more";
return bound.Length == 1
? $"{named} authenticates with it, and will refuse to connect rather than fall back to a typed "
+ "password."
: $"{bound.Length} hosts authenticate with it — {named} — and will refuse to connect rather than "
+ "fall back to a typed password.";
}
/// <summary>
/// Withdraws trust from the selected pin's endpoint.
/// </summary>
@@ -2577,6 +2803,33 @@ internal sealed partial class VaultViewModel(
{
OnPropertyChanged(nameof(SelectedHostAsksForAPassword));
OnPropertyChanged(nameof(SelectedHostAuthenticationNote));
DisarmIfAimedElsewhere(DeletionTarget.Host, value?.EntityId);
}
partial void OnPendingDeletionChanged(DeletionRequest? value)
{
OnPropertyChanged(nameof(IsConfirmingDeletion));
OnPropertyChanged(nameof(ShowsHostActions));
OnPropertyChanged(nameof(ShowsItemActions));
}
/// <summary>
/// Takes the question away when the selection it was asked about has moved on.
/// </summary>
/// <remarks>
/// Compared by entity id rather than by row, and that is the whole point of the method. A reload
/// replaces every row object in the list, so a background pass a minute after the question would
/// otherwise take the card away from under somebody still reading it — while a click onto a different
/// item, which is the case that actually needs handling, leaves an armed deletion pointing at something
/// nobody is looking at any more.
/// </remarks>
private void DisarmIfAimedElsewhere(DeletionTarget target, Guid? entityId)
{
if (PendingDeletion is { } request && request.Target == target && request.EntityId != entityId)
{
PendingDeletion = null;
}
}
/// <remarks>
@@ -2664,6 +2917,11 @@ internal sealed partial class VaultViewModel(
OnPropertyChanged(nameof(SelectedItemIsEditable));
OnPropertyChanged(nameof(SelectedItemIsPin));
OnPropertyChanged(nameof(SelectedDetailHeading));
OnPropertyChanged(nameof(ShowsItemActions));
// Both kinds this table can delete, because one selection covers both lists.
DisarmIfAimedElsewhere(DeletionTarget.Key, value?.EntityId);
DisarmIfAimedElsewhere(DeletionTarget.Credential, value?.EntityId);
switch (value?.Kind)
{
@@ -2714,8 +2972,34 @@ internal sealed partial class VaultViewModel(
/// flips back in every path that closes one, so this notification always observes the pair in a
/// consistent state.
/// </remarks>
partial void OnIsEditingChanged(bool value) =>
partial void OnIsEditingChanged(bool value)
{
OnPropertyChanged(nameof(CanForgetHostKey));
OnPropertyChanged(nameof(ShowsHostActions));
DisarmOnceAnEditorIsOpen(value);
}
partial void OnIsEditingKeyChanged(bool value) => DisarmOnceAnEditorIsOpen(value);
partial void OnIsEditingCredentialChanged(bool value) => DisarmOnceAnEditorIsOpen(value);
/// <summary>
/// Takes the question away when an editor opens over the pane it was asked in.
/// </summary>
/// <remarks>
/// The sidebar's confirmation replaces the buttons that could open the host editor, so that half cannot
/// happen; the vault screen's Add buttons stay on screen beside the detail pane, so that half can. One
/// rule for both, rather than a guard on the three commands that would have to be remembered by the
/// fourth.
/// </remarks>
private void DisarmOnceAnEditorIsOpen(bool opened)
{
if (opened)
{
PendingDeletion = null;
}
}
partial void OnPendingHostKeyChanged(HostKeyPresentation? value) =>
OnPropertyChanged(nameof(HasPendingHostKey));