Author SHA1 Message Date
jaap-jan 05c56f20a4 Let a closed terminal take its own status line with it
The strip along the bottom is one line for the whole window, and connecting
writes to it, so "Connected to prod-db." outlived the tab it was about: close
prod-db, look at the terminal beside it, and the bar was still reporting on a
session that is no longer there.

A connection's own lines are now owned by the attempt that wrote them, and
closing a tab clears the line only when that is what the bar is holding.
Ownership rather than a blanket clear on close, because the alternative loses
the sentences this bar exists for — a failed save or a refused sync is what the
design deleted the bar's other four fields to make room for, and closing a
terminal is no answer to one.
2026-08-10 11:41:55 +02:00
10 changed files with 170 additions and 507 deletions
-13
View File
@@ -388,19 +388,6 @@ agent of our own plus ProxyJump covers the real use cases.
**The SSH suite pulls `linuxserver/openssh-server` from Docker Hub**, which is rate-limited for
unauthenticated pulls. If CI starts failing on image pulls rather than on tests, that is why.
**That suite has an intermittent `The connection was closed by the remote host`**, on whichever test
connects first, within tens of milliseconds. Seen in CI and reproducible locally. *Mitigated, not
solved:* `SshServerFixture` now raises sshd's `MaxStartups` from its compiled-in `10:30:100`, which
refuses connections at random past ten unauthenticated ones in flight — reachable because xUnit runs
test classes in parallel and most of them connect. The fixture comment carries the full argument and
is explicit that the cure is unproven.
**And the reason it is unproven is a measurement trap worth not falling into twice.** Docker
throughput on the Windows development machine swings enough to swamp the effect: the identical
unmodified suite ran 85/85 clean and, an hour later, failed 13 runs out of 15. Any before/after flake
comparison taken there is noise. Measure this class of thing in CI, or make the server say why —
raise sshd's `LogLevel`, disable Ryuk so the container outlives the run, and read `docker logs`.
**MSIX packaging is ruled out, not merely deprioritised.** A packaged app runs WebView2 in an
AppContainer where loopback connections are blocked without a `CheckNetIsolation` exemption. The
terminal data plane *is* a loopback WebSocket, so MSIX would break the product outright. Velopack
@@ -1029,48 +1029,6 @@
</ComboBox.ItemTemplate>
</ComboBox>
<!--
Making a credential without leaving the host, as on the desktop and on the same reasoning: the
moment one is wanted is while deciding how a host authenticates, and this head has no keychain
editor for credentials at all — so without this a phone could bind a host to a credential but
never make one. Writes to the keychain the instant ADD is pressed, exactly as the new-tag box
below does and for the same reason: a host can only name an id that exists.
-->
<Button Classes="secondary" Content="+ NEW CREDENTIAL" HorizontalAlignment="Left"
MinHeight="40" Padding="14,0"
IsVisible="{Binding !IsAddingEditorCredential}"
Command="{Binding BeginEditorCredentialCommand}" />
<Border CornerRadius="12" Background="{StaticResource Field}"
BorderBrush="{StaticResource Border}" BorderThickness="1" Padding="12"
IsVisible="{Binding IsAddingEditorCredential}">
<StackPanel Spacing="8">
<TextBlock Classes="label" Text="NEW CREDENTIAL" />
<TextBox Classes="field" Text="{Binding EditorNewCredentialLabel}"
PlaceholderText="name" />
<!--
Optional, and what makes a credential its own item: one account on twenty machines is
rotated in one place. Left blank, this host's own username is used.
-->
<TextBox Classes="field" Text="{Binding EditorNewCredentialUsername}"
PlaceholderText="username (blank: this host's own)" />
<TextBox Classes="field secret" Text="{Binding EditorNewCredentialPassword}"
PlaceholderText="password" />
<TextBox Classes="field" Text="{Binding EditorNewCredentialNotes}"
PlaceholderText="notes" />
<TextBlock Classes="detail" TextWrapping="Wrap"
Text="Added to the keychain as soon as you press ADD, so it stays even if you leave this host without saving." />
<Grid ColumnDefinitions="*,8,*">
<Button Grid.Column="0" Classes="primary" Content="ADD" MinHeight="44"
HorizontalAlignment="Stretch" HorizontalContentAlignment="Center"
Command="{Binding AddEditorCredentialCommand}" />
<Button Grid.Column="2" Classes="secondary" Content="CANCEL" MinHeight="44"
HorizontalAlignment="Stretch" HorizontalContentAlignment="Center"
Command="{Binding CancelEditorCredentialCommand}" />
</Grid>
</StackPanel>
</Border>
<!--
◆ WHICH VAULT THIS HOST WILL LIVE IN. Drawn only while adding and only where there is more than
one vault that can be written to, exactly as on the desktop — an existing host's vault is not a
@@ -584,56 +584,6 @@
</ComboBox.ItemTemplate>
</ComboBox>
<!--
Making a credential without leaving the host. The moment one is wanted is this one: somebody
is deciding how a host authenticates and finds the password is not in the keychain yet, and
sending them to the other screen to add it would lose the half-typed host they are standing
in. Same argument as the new-tag box further down, same immediate write, same honest
consequence — the credential stays if this editor is cancelled, because a host can only name
an id that exists.
A button beside the picker rather than an entry inside it. Every row of that list is a
binding the host can have; "make a new one" is an action, and as an entry it would sit in the
box afterwards describing a state no host can be in.
-->
<Button Classes="ghost" Content="+ NEW CREDENTIAL" HorizontalAlignment="Left"
FontSize="10.5" Height="28" Padding="10,0"
IsVisible="{Binding !IsAddingEditorCredential}"
Command="{Binding BeginEditorCredentialCommand}"
ToolTip.Tip="Adds a credential to the keychain and binds this host to it" />
<Border CornerRadius="12" Background="{StaticResource Field}"
BorderBrush="{StaticResource Border}" BorderThickness="1" Padding="12"
IsVisible="{Binding IsAddingEditorCredential}">
<StackPanel Spacing="6">
<TextBlock Classes="label" Text="NEW CREDENTIAL" FontSize="10" />
<TextBox Text="{Binding EditorNewCredentialLabel}" PlaceholderText="name" Height="36" />
<!--
Optional, and what makes a credential worth being its own item: one account on twenty
machines is rotated in one place. Left blank, this host's own username is used.
-->
<TextBox Text="{Binding EditorNewCredentialUsername}" Height="36"
PlaceholderText="username (blank: use this host's own)" />
<!-- Masked, on the reasoning the keychain's own password box carries. -->
<TextBox Text="{Binding EditorNewCredentialPassword}" PlaceholderText="password"
PasswordChar="•" Height="36">
<TextBox.KeyBindings>
<KeyBinding Gesture="Enter" Command="{Binding AddEditorCredentialCommand}" />
</TextBox.KeyBindings>
</TextBox>
<TextBox Text="{Binding EditorNewCredentialNotes}" PlaceholderText="notes"
AcceptsReturn="True" Height="44" TextWrapping="Wrap" />
<TextBlock Classes="hint" FontSize="10.5" TextWrapping="Wrap"
Text="Added to the keychain as soon as you press ADD, so it stays even if you cancel this host. Renaming and deleting are on the keychain screen." />
<StackPanel Orientation="Horizontal" Spacing="6">
<Button Classes="accent" Content="ADD"
Command="{Binding AddEditorCredentialCommand}" />
<Button Classes="ghost" Content="CANCEL"
Command="{Binding CancelEditorCredentialCommand}" />
</StackPanel>
</StackPanel>
</Border>
<!--
◆ THE RELAY CARD, restyled to the mock's nested-card shape — radius 12, a checkbox with the
title beside it rather than under it — but NOT to the mock's copy. The sentence stays
@@ -25,6 +25,14 @@
channel this application has for saying that a save failed, that a sync was refused, that a merge picked
a winner. The design is a mock-up of a working afternoon and has nowhere for a sentence like that to go;
dropping the bar would have meant dropping the sentence, or repeating it on six screens.
── One line, several terminals ────────────────────────────────────────────────────────────────────────
One bar for a window that now holds any number of tabs, and connecting writes to it — so "Connected to
prod-db." outlived the tab it was about, and closing prod-db left that sentence sitting under whichever
terminal the user looked at next. A connection's own lines are now owned by the attempt that wrote them
and are cleared when its tab is closed; everything else the vault says stays, because closing a terminal
is not an answer to a failed save. See VaultViewModel.ForgetConnectionStatus and
MainWindowViewModel.CloseTabAsync.
-->
<Border Height="24" Background="{StaticResource Chrome}"
@@ -1924,9 +1924,18 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
/// Closes one terminal, ending its shell.
/// </summary>
/// <remarks>
/// <para>
/// This is the one thing in the application that deliberately ends a session, which is why it is a tab's
/// close button and not a menu item: closing the window somebody's job is running in should take exactly
/// as much intent as it looks like it does. Locking does not do this, and neither does anything else.
/// </para>
/// <para>
/// <b>It takes the status line with it, when that line is this tab's.</b> The strip along the bottom is
/// one line for the whole window — see <c>StatusBar.axaml</c> — and connecting writes to it, so closing
/// prod-db used to leave "Connected to prod-db." reporting on a session that is gone, over whichever
/// terminal the user looked at next. Only the sentence this attempt wrote goes; anything the vault has
/// said since stays, because a failed save is not something closing a terminal answers.
/// </para>
/// </remarks>
[RelayCommand]
private async Task CloseTabAsync(TerminalTabViewModel tab)
@@ -1960,6 +1969,9 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
RaiseTabState();
// Only this tab's own sentence, and only if that is what the bar currently holds — see the remark.
Vault?.ForgetConnectionStatus(tab.AttemptId);
// Explicitly, and not left to the selection having moved. Closing a tab that was not the selected one
// changes nothing about the selection, so OnSelectedTabChanged does not run — and the host whose
// terminal just went would keep a lit dot until something else happened to move the selection.
@@ -3400,7 +3412,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
/// </remarks>
private void OnVaultConnectionStarting(object? sender, ConnectionAttemptEventArgs e)
{
var tab = new TerminalTabViewModel(e.Label, e.Address);
var tab = new TerminalTabViewModel(e.Label, e.Address) { AttemptId = e.AttemptId };
attempts[e.AttemptId] = tab;
AdoptTab(tab);
@@ -3429,6 +3441,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
// dropping it would leave a shell running with nothing in the window naming it.
var adopted = new TerminalTabViewModel(e.SessionId, e.Label, e.Address)
{
AttemptId = e.AttemptId,
StartedAt = clock.GetUtcNow(),
Cipher = NullIfEmpty(e.Cipher),
HostKeyAlgorithm = NullIfEmpty(e.HostKeyAlgorithm),
@@ -84,6 +84,18 @@ internal sealed partial class TerminalTabViewModel : ObservableObject
/// </remarks>
internal uint SessionId { get; private set; }
/// <summary>
/// The connection attempt this tab was opened for, or <see cref="Guid.Empty"/> for a tab nothing dialled.
/// </summary>
/// <remarks>
/// Carried only so that closing this tab can take the vault's status line with it when the line is about
/// this connection — see <c>VaultViewModel.ForgetConnectionStatus</c>. It is the attempt id and not the
/// session id because the message can be written before a session exists ("Connecting to prod-db…") and
/// after one has stopped existing, and because the two failure sentences a tab can end on never have a
/// session id at all.
/// </remarks>
internal Guid AttemptId { get; init; }
internal string Label { get; }
/// <summary>The account and endpoint, for the pane header and the status bar.</summary>
@@ -2130,6 +2130,60 @@ internal sealed partial class VaultViewModel(
[ObservableProperty]
private string status = string.Empty;
/// <summary>
/// The connection <see cref="Status"/> is currently about, or <see cref="Guid.Empty"/> when it is about
/// something else — a save, a sync, a refusal that has nothing to do with a terminal.
/// </summary>
/// <remarks>
/// <para>
/// The status line is one line for the whole window, and most of what goes through it is about the vault,
/// which is one thing however many terminals are open. A connection is not: "Connected to prod-db." is
/// about one tab, and it used to outlive that tab — close prod-db, look at the terminal beside it, and the
/// strip along the bottom was still reporting on a session that is no longer there.
/// </para>
/// <para>
/// Ownership rather than a blanket clear on every close, because the alternative loses the sentences this
/// bar exists for: a failed save or a refused sync is what the design deleted the bar's other four fields
/// to make room for, and closing a terminal is no reason to take one away.
/// </para>
/// <para>
/// Reset by <see cref="OnStatusChanged"/> on every write, so the only messages that carry an owner are the
/// ones <see cref="SayAbout"/> writes. Anything assigning <see cref="Status"/> in the ordinary way says,
/// by doing so, that its message belongs to no tab.
/// </para>
/// </remarks>
private Guid statusAttemptId;
partial void OnStatusChanged(string value) => statusAttemptId = Guid.Empty;
/// <summary>Writes a status line that belongs to one connection attempt.</summary>
/// <remarks>
/// After the assignment, not before: <see cref="OnStatusChanged"/> clears the owner on every write, which
/// is what makes "belongs to nobody" the default rather than something each of the other status writes
/// would have to remember to say.
/// </remarks>
private void SayAbout(ConnectionAttemptEventArgs attempt, string message)
{
Status = message;
statusAttemptId = attempt.AttemptId;
}
/// <summary>
/// Clears the status line if what it is saying is about this connection, and leaves it alone otherwise.
/// </summary>
/// <remarks>
/// Called by the shell when a tab is closed — see <c>MainWindowViewModel.CloseTabAsync</c>. Cleared to
/// empty rather than replaced with "Closed prod-db.": the tab going is the report, it is the report the
/// user just made happen, and a bar that answers every close with a sentence is a bar people stop reading.
/// </remarks>
internal void ForgetConnectionStatus(Guid attemptId)
{
if (attemptId != Guid.Empty && statusAttemptId == attemptId)
{
Status = string.Empty;
}
}
[ObservableProperty]
private int pendingChanges;
@@ -3086,37 +3140,6 @@ internal sealed partial class VaultViewModel(
[ObservableProperty]
private AuthenticationChoice? editorSelectedAuthentication;
// ---- Making a credential from inside the host editor ----
// A fifth set of editor fields, and deliberately not the keychain screen's four. Sharing them would put
// IsEditingCredential — which AVaultEditorIsInTheWay asks about — true while the user is on the Hosts
// screen, and the whole Vault screen would refuse to open an editor with a sentence naming a form on
// another screen. That is the exact failure AHostEditorIsInTheWay was split out to end; see its remarks.
/// <summary>Whether the host editor is showing its own new-credential form.</summary>
[ObservableProperty]
private bool isAddingEditorCredential;
/// <summary>The name in the host editor's new-credential form.</summary>
[ObservableProperty]
private string editorNewCredentialLabel = string.Empty;
/// <inheritdoc cref="CredentialEditorUsername" path="/remarks" />
[ObservableProperty]
private string editorNewCredentialUsername = string.Empty;
/// <remarks>
/// Holds a password for as long as the form is open, on the same terms the keychain's box does — see
/// <see cref="CredentialEditorPassword"/>. Cleared by every path that closes this form, including the
/// ones that close the host editor around it, so a password typed here cannot outlive the form and
/// reappear behind the next host somebody edits.
/// </remarks>
[ObservableProperty]
private string editorNewCredentialPassword = string.Empty;
/// <summary>Free text, as the keychain's own editor takes.</summary>
[ObservableProperty]
private string editorNewCredentialNotes = string.Empty;
/// <summary>What the group picker offers: "no group", then every group of the chosen vault.</summary>
/// <inheritdoc cref="EditorAuthenticationChoices" path="/remarks" />
internal ObservableCollection<GroupChoice> EditorGroupChoices { get; } = [];
@@ -3388,121 +3411,6 @@ internal sealed partial class VaultViewModel(
OnPropertyChanged(nameof(HasTagChoices));
}
/// <summary>
/// Opens the host editor's own new-credential form.
/// </summary>
/// <remarks>
/// A button beside the picker rather than an entry inside it. Every row of that list is a binding the
/// host can have — see <see cref="AuthenticationChoice"/> — and "make a new one" is an action, not a
/// binding: as an entry it would sit in the box afterwards describing a state no host can be in, and
/// cancelling the form would leave the picker showing it.
/// </remarks>
[RelayCommand]
private void BeginEditorCredential()
{
ClearEditorCredentialForm();
IsAddingEditorCredential = true;
Status = "Adding a credential for this host.";
}
/// <summary>Abandons the form, clearing the password out of it.</summary>
[RelayCommand]
private void CancelEditorCredential()
{
ClearEditorCredentialForm();
Status = string.Empty;
}
/// <summary>Closes the form and drops what was typed into it, the password included.</summary>
private void ClearEditorCredentialForm()
{
IsAddingEditorCredential = false;
EditorNewCredentialLabel = string.Empty;
EditorNewCredentialUsername = string.Empty;
EditorNewCredentialPassword = string.Empty;
EditorNewCredentialNotes = string.Empty;
}
/// <summary>
/// Creates a credential from the host editor's form and binds the host being edited to it.
/// </summary>
/// <remarks>
/// <para>
/// The same reasoning <see cref="AddEditorTagAsync"/> gives, and for the same moment: somebody is
/// choosing how a host authenticates and finds the password they want is not in the keychain yet.
/// Sending them to the other screen to make one would lose the half-typed host they were standing in.
/// </para>
/// <para>
/// <b>It writes to the keychain immediately, unlike every other field in this editor.</b> A credential
/// is a shared item with an id and a host can only name an id that exists, so there is nothing to defer.
/// Cancelling the host edit therefore leaves the credential behind — honest rather than hidden, and the
/// bargain a tag already makes here.
/// </para>
/// <para>
/// <b>A name that already exists is duplicated rather than reused, which is where this deliberately
/// parts from the tag path.</b> Two tags called "staging" are the same intention spelled twice; two
/// credentials called "root" are two different passwords, and quietly binding the host to the one that
/// happened to be there already would authenticate it as an account the user never chose. A duplicate
/// label in the picker is a smaller problem than a silent wrong password.
/// </para>
/// <para>
/// Into <see cref="editingHostVaultId"/>, not the standing target: the credential belongs wherever the
/// host is being sealed, so everybody who can read the host can read what it authenticates with. That is
/// stricter than the tag path — which files into the active vault and is recorded as a gap — and it can
/// be, because the picker here lists credentials from every readable vault rather than one.
/// </para>
/// </remarks>
[RelayCommand]
private async Task AddEditorCredentialAsync(CancellationToken cancellationToken)
{
var credential = new CredentialSecret
{
Label = EditorNewCredentialLabel.Trim(),
// Not trimmed. A password of spaces is a password — CredentialSecret.TryValidate says so — and
// trimming one here would lock somebody out of a host over a tidiness opinion.
Password = EditorNewCredentialPassword,
Username = string.IsNullOrWhiteSpace(EditorNewCredentialUsername)
? null
: EditorNewCredentialUsername.Trim(),
// Untrimmed and unnormalised past blank-is-absent, as the keychain's editor writes it: free text
// is the user's to lay out, and its leading indent is theirs rather than this form's to correct.
Notes = string.IsNullOrWhiteSpace(EditorNewCredentialNotes) ? null : EditorNewCredentialNotes,
};
if (!credential.TryValidate(out var reason))
{
Status = reason;
return;
}
await RunAsync(
"Saving…",
async () =>
{
var entityId = await session.Credentials
.CreateAsync(editingHostVaultId, credential, cancellationToken)
.ConfigureAwait(true);
// Before the reload, not after it. RefreshOpenEditors rebuilds this picker and then restores
// it from whatever this property says, so writing the binding here is what survives the pass
// — and by the time it is read, ReloadCredentialsAsync has put the matching entry in the
// list for it to land on.
EditorSelectedAuthentication =
AuthenticationChoice.ForCredential(entityId, credential.Label);
ClearEditorCredentialForm();
await ReloadAsync(cancellationToken).ConfigureAwait(true);
Status = $"Added '{credential.Label}' and bound this host to it. "
+ "Save the host to keep the binding.";
}).ConfigureAwait(true);
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
/// <summary>
/// The paths pinned on the host being edited, in the order QUICK ACCESS draws them.
/// </summary>
@@ -7238,9 +7146,6 @@ internal sealed partial class VaultViewModel(
EditorPinnedPaths.Clear();
EditorNewPin = string.Empty;
// Closed rather than carried over, and it holds a password — see EditorNewCredentialPassword.
ClearEditorCredentialForm();
// Before the group picker, because a group belongs to one vault and the picker is that vault's.
BuildEditorVaultChoices(editingHostVaultId);
@@ -7326,9 +7231,6 @@ internal sealed partial class VaultViewModel(
EditorNewTag = string.Empty;
BuildTagChoices();
// As in NewHost, and for the password it can be holding.
ClearEditorCredentialForm();
LoadEditorPinnedPaths(row.Host.PinnedPaths);
BuildEditorVaultChoices(editingHostVaultId);
@@ -8189,10 +8091,6 @@ internal sealed partial class VaultViewModel(
{
IsEditing = false;
editingEntityId = null;
// The form goes with the editor it lives in, password and all. A credential already added through it
// stays in the keychain — see AddEditorCredentialAsync — but what was still being typed does not.
ClearEditorCredentialForm();
Status = string.Empty;
}
@@ -8226,10 +8124,6 @@ internal sealed partial class VaultViewModel(
}
IsEditing = false;
// As CancelEdit does, for the same password.
ClearEditorCredentialForm();
await ReloadAsync(cancellationToken).ConfigureAwait(true);
SelectedHost = Hosts.FirstOrDefault(row => row.EntityId == editingEntityId);
@@ -10927,7 +10821,7 @@ internal sealed partial class VaultViewModel(
Dialled(target, authentication));
ConnectionStarting?.Invoke(this, attempt);
Status = $"Connecting to {target.Label}…";
SayAbout(attempt, $"Connecting to {target.Label}…");
await OpenSessionAsync(attempt, target, authentication, cancellationToken).ConfigureAwait(true);
}
@@ -11202,7 +11096,9 @@ internal sealed partial class VaultViewModel(
/// </remarks>
private void Abandon(ConnectionAttemptEventArgs attempt, string reason)
{
Status = reason;
// Owned by the attempt, unlike Answer's own line below: this is the case where the tab stays, carrying
// the same reason, so closing that tab is exactly the moment the line stops being about anything.
SayAbout(attempt, reason);
ConnectionFailed?.Invoke(
this,
@@ -11257,7 +11153,7 @@ internal sealed partial class VaultViewModel(
// only record that machine was reached at all.
connectionLog?.Identify(sessionId, target.Label, target.HostId);
Status = $"Connected to {target.Label}.";
SayAbout(attempt, $"Connected to {target.Label}.");
// The session the workspace just opened is the only place the negotiated cipher and host-key
// algorithm live — an SshConnectionRequest asks for neither and gets no say in either — so they are
@@ -11288,7 +11184,8 @@ internal sealed partial class VaultViewModel(
// screen it is typed on says so.
if (target.Row is { } row)
{
await RememberTypedPasswordAsync(row, authentication, cancellationToken).ConfigureAwait(true);
await RememberTypedPasswordAsync(attempt, row, authentication, cancellationToken)
.ConfigureAwait(true);
}
}
@@ -11320,6 +11217,7 @@ internal sealed partial class VaultViewModel(
/// </para>
/// </remarks>
private async Task RememberTypedPasswordAsync(
ConnectionAttemptEventArgs attempt,
HostRowViewModel row,
HostAuthentication authentication,
CancellationToken cancellationToken)
@@ -11335,8 +11233,10 @@ internal sealed partial class VaultViewModel(
if (row.IsReadOnly)
{
Status = $"Connected to {row.Label}. Its password was not saved: this host was written by a "
+ "newer version of DodoSSH, and binding a credential would re-encode it.";
SayAbout(
attempt,
$"Connected to {row.Label}. Its password was not saved: this host was written by a newer "
+ "version of DodoSSH, and binding a credential would re-encode it.");
return;
}
@@ -11360,7 +11260,9 @@ internal sealed partial class VaultViewModel(
}
catch (Exception exception)
{
Status = $"Connected to {row.Label}, but its password could not be saved: {exception.Message}";
SayAbout(
attempt,
$"Connected to {row.Label}, but its password could not be saved: {exception.Message}");
return;
}
@@ -11371,8 +11273,10 @@ internal sealed partial class VaultViewModel(
await ReloadAsync(cancellationToken).ConfigureAwait(true);
Status = $"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it "
+ "will not be asked for again.";
SayAbout(
attempt,
$"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it will "
+ "not be asked for again.");
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
@@ -189,25 +189,6 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
await MeasureDrawerAsync(faults => faults.ShouldBeEmpty());
}
/// <remarks>
/// The editor with its new-credential card showing, which the test above never draws: the card is
/// collapsed until somebody presses + NEW CREDENTIAL, so nothing else in this suite measures the three
/// boxes, the paragraph of hint text and the two buttons it adds inside the section that already holds
/// the authentication picker. A card that only appears on a click is exactly the shape that escapes a
/// harness driven by the default state.
/// </remarks>
[Fact]
public async Task TheHostDrawerFitsWithTheNewCredentialFormOpen()
{
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.IsAddingEditorCredential.ShouldBeTrue("there is nothing to measure otherwise");
await MeasureDrawerAsync(faults => faults.ShouldBeEmpty());
}
/// <remarks>
/// The other editor, and it is in this control for the first time: the desktop's group editor used to be
/// a bar across the foot of the hosts screen, where it competed with the grid for the same column. Its
+62 -155
View File
@@ -1254,6 +1254,68 @@ public sealed class ShellFlowTests : IAsyncLifetime
shell.IsTerminalShowing.ShouldBeTrue();
}
/// <remarks>
/// The status line is one line for the whole window, so a sentence about a session that has been closed is
/// a sentence the user reads over some other terminal.
/// </remarks>
[Fact]
public async Task ClosingATab_TakesItsOwnStatusLineWithIt()
{
var vault = await ReadyToConnectAsync();
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
await vault.ConnectCommand.ExecuteAsync(null);
vault.Status.ShouldContain("Connected", Case.Insensitive);
await shell.CloseTabCommand.ExecuteAsync(shell.Tabs[0]);
vault.Status.ShouldBeEmpty();
}
/// <remarks>
/// The other half, and the reason closing does not simply blank the bar: everything this application has
/// to say about a save, a sync or a refusal goes through the same line — see <c>StatusBar.axaml</c> — and
/// closing a terminal answers none of it.
/// </remarks>
[Fact]
public async Task ClosingATab_LeavesAStatusLineThatIsAboutSomethingElse()
{
var vault = await ReadyToConnectAsync();
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
await vault.ConnectCommand.ExecuteAsync(null);
vault.Status = "The keychain could not be saved.";
await shell.CloseTabCommand.ExecuteAsync(shell.Tabs[0]);
vault.Status.ShouldBe("The keychain could not be saved.");
}
/// <remarks>
/// Two sessions to the same host, which is the case a label could not tell apart: the line belongs to the
/// attempt that wrote it, so closing the other one leaves it alone.
/// </remarks>
[Fact]
public async Task ClosingATab_LeavesTheLineAnotherTabWrote()
{
var vault = await ReadyToConnectAsync();
await using var renderer = await FakeRenderer.AttachAsync(workspace, Token);
await vault.ConnectCommand.ExecuteAsync(null);
var first = shell.Tabs[0];
// The second connection's own line is what the bar holds now.
await vault.ConnectCommand.ExecuteAsync(null);
var reported = vault.Status;
await shell.CloseTabCommand.ExecuteAsync(first);
vault.Status.ShouldBe(reported);
}
// ---- Connecting, while it is still happening ----
//
// A handshake is a network round trip and no longer holds the vault while it runs, so there is a stretch
@@ -2980,161 +3042,6 @@ public sealed class ShellFlowTests : IAsyncLifetime
vault.Hosts[0].Host.CredentialId.ShouldBeNull();
}
/// <remarks>
/// The moment a credential is wanted is the moment somebody is choosing how a host authenticates and
/// finds it is not in the keychain yet, so the host editor makes one. Selecting it has to survive the
/// reload the write triggers, which is the part that needs a test: the refill rebuilds the picker from
/// the vault and restores it from the editor's own selection, so the binding is written before the
/// reload rather than after it.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_BindsTheHostToIt()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.IsAddingEditorCredential.ShouldBeTrue();
vault.EditorNewCredentialLabel = "pg-primary";
vault.EditorNewCredentialUsername = "postgres";
vault.EditorNewCredentialPassword = "s3cret";
vault.EditorNewCredentialNotes = "rotated quarterly";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
var credential = vault.Credentials.ShouldHaveSingleItem();
credential.Credential.Username.ShouldBe("postgres");
credential.Credential.Notes.ShouldBe("rotated quarterly");
vault.IsAddingEditorCredential.ShouldBeFalse("the form closes once the credential is in the keychain");
vault.EditorNewCredentialPassword.ShouldBeEmpty("the form must not go on holding the password");
vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId.ShouldBe(
credential.EntityId,
"the picker has to land on the credential that was just made, through the reload");
await vault.SaveHostCommand.ExecuteAsync(null);
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBe(credential.EntityId);
vault.Hosts[0].Host.SshKeyId.ShouldBeNull();
}
/// <remarks>
/// The honest consequence of writing immediately, and the same one the new-tag box already carries: a
/// credential is a shared item with an id, the host can only name an id that exists, so the credential
/// was never part of the host to begin with. What was still being typed is a different matter — that
/// includes a password, and it goes with the editor it was typed into.
/// </remarks>
[Fact]
public async Task CancellingTheHostEditor_KeepsTheCredentialItMade_AndDropsWhatWasStillBeingTyped()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "pg-primary";
vault.EditorNewCredentialPassword = "s3cret";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
// A second one, opened and left half-typed.
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "half";
vault.EditorNewCredentialPassword = "typed-but-never-added";
vault.EditorNewCredentialNotes = "half a thought";
vault.CancelEditCommand.Execute(null);
vault.Credentials.ShouldHaveSingleItem().Label.ShouldBe("pg-primary");
vault.IsAddingEditorCredential.ShouldBeFalse();
vault.EditorNewCredentialLabel.ShouldBeEmpty();
vault.EditorNewCredentialNotes.ShouldBeEmpty();
vault.EditorNewCredentialPassword.ShouldBeEmpty(
"a password typed into an abandoned form must not survive behind the next host");
vault.Hosts.ShouldHaveSingleItem().Host.CredentialId.ShouldBeNull(
"the binding itself was never saved");
}
/// <remarks>
/// Why this form has fields of its own rather than reusing the keychain screen's four.
/// <c>IsEditingCredential</c> is what <c>AVaultEditorIsInTheWay</c> asks about, so sharing it would make
/// the whole Vault screen refuse to open an editor, with a sentence naming a form the user cannot see
/// on a screen they are not looking at. That is the exact failure the guard was split in two to end.
/// </remarks>
[Fact]
public async Task TheHostEditorsCredentialForm_DoesNotBlockTheKeychainsOwnEditors()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.NewCredentialCommand.Execute(null);
vault.IsEditingCredential.ShouldBeTrue(
"the keychain's editor lives on another screen and opens regardless");
}
/// <remarks>
/// Where this deliberately parts from the new-tag box beside it, which offers an existing tag rather
/// than repeating it. Two tags called "staging" are one intention spelled twice; two credentials called
/// "root" are two different passwords, and quietly binding the host to whichever was there already
/// would authenticate it as an account nobody chose.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_UnderANameAlreadyTaken_IsASecondCredential()
{
var vault = await ReadyToConnectAsync();
await AddCredentialAsync(vault, "root", password: "first");
var first = vault.Credentials.ShouldHaveSingleItem().EntityId;
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "root";
vault.EditorNewCredentialPassword = "second";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
vault.Credentials.Count.ShouldBe(2);
vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId.ShouldNotBe(
first,
"binding to the credential that happened to share the name would be the wrong password");
}
/// <remarks>
/// The same refusal <c>CredentialSecret.TryValidate</c> gives the keychain's editor, reaching the user
/// here rather than producing an item that looks usable and fails at the handshake.
/// </remarks>
[Fact]
public async Task ACredentialMadeInTheHostEditor_WithNoPassword_IsRefused()
{
var vault = await ReadyToConnectAsync();
vault.SelectedHost = vault.Hosts[0];
vault.EditSelectedHostCommand.Execute(null);
vault.BeginEditorCredentialCommand.Execute(null);
vault.EditorNewCredentialLabel = "pg-primary";
await vault.AddEditorCredentialCommand.ExecuteAsync(null);
vault.Credentials.ShouldBeEmpty();
vault.IsAddingEditorCredential.ShouldBeTrue("the form stays open on what it refused");
vault.Status.ShouldContain("password");
}
/// <remarks>
/// Tags reach the same editor by a different route — the keychain screen rather than the box under the
/// chips — and a chip that only appeared on the next open would send the user round the same detour.
@@ -112,61 +112,6 @@ public sealed class SshServerFixture : IAsyncLifetime
/// this image as hardening, not as a behaviour worth reproducing: nothing else here opens a channel of
/// any kind, so allowing it changes what exactly one suite can do and what none of the others see.
/// </para>
/// <para>
/// ◆ <b><c>MaxStartups</c> is raised here too, against a flake this suite has and that this change is
/// a mitigation for rather than a proven cure.</b> The distinction is stated because the evidence
/// stops short of the claim, and a later reader deserves to know which.
/// </para>
/// <para>
/// What is established: sshd's compiled-in default is <c>10:30:100</c> — past ten
/// <em>unauthenticated</em> connections in flight it refuses new ones at random, thirty percent of the
/// time, rising to always at a hundred — and the image ships the line commented out, so that default
/// was what ran. xUnit runs test classes in parallel and most classes here open a connection, so ten
/// in flight is reachable in the opening seconds. A refused connection presents to the client as
/// <c>SshConnectionException: The connection was closed by the remote host</c> within tens of
/// milliseconds, on whichever test connects at the wrong moment — which is exactly the observed
/// failure, seen in CI and reproduced locally.
/// </para>
/// <para>
/// What is <em>not</em> established is that this limit is the only cause, because the flake rate could
/// not be measured reliably. On the development machine the identical unmodified suite ran 85/85 clean
/// and, an hour later, failed 13 runs out of 15 — Docker throughput on that host swings far enough to
/// swamp the effect being measured. Any before/after comparison taken there is noise, and two were,
/// before that was noticed.
/// </para>
/// <para>
/// It is committed anyway, on the narrower argument that it is right regardless: a connection throttle
/// is hardening this suite has no interest in reproducing. It exists to test an SSH client, not to
/// survive a rate limit, and a test server that drops connections at random is a bad test server
/// whether or not it is the cause of this particular flake.
/// </para>
/// <para>
/// <b>Not fixed by serialising the suite</b>, which would have hidden it and cost the parallelism, and
/// not by retrying the connect, which would have made the client's own reconnect behaviour untestable
/// by burying it in the fixture. The limit is a property of a hardened server that this suite has no
/// interest in reproducing — it exists to test an SSH client, not to survive a throttle.
/// </para>
/// <para>
/// Replaced in place rather than appended, because sshd_config takes the <em>first</em> value it finds
/// for a keyword: an appended line would be dead the day the image ships an uncommented one of its own.
/// </para>
/// <para>
/// ◆ <b>The reload window is the other candidate, and it is deliberately not guarded against.</b>
/// <c>SIGHUP</c> makes sshd close its listeners and re-execute itself, and <c>pkill</c> returns when
/// the signal is delivered rather than when that has finished — so in principle a connection made
/// immediately afterwards is refused, producing this same exception. A wait that opened connections
/// until the server answered with its banner three times running was written, and then removed: it
/// could not be shown to change anything either, and a fixture carrying two unproven fixes for one
/// symptom is worse than one, because the next person has to disprove both.
/// </para>
/// <para>
/// If this flake returns, that is the next thing to try. Two things to know before trying it: the two
/// causes are indistinguishable from the client, so a fix can only be judged by a repeat run and never
/// by whether the next run passes — and the repeat run has to happen somewhere with stable Docker
/// throughput, which the development machine is not. Better still, make sshd say why: raise its
/// <c>LogLevel</c> here, disable Ryuk so the container outlives the run, and read
/// <c>docker logs</c>. A <c>MaxStartups</c> refusal names itself there; a reload does not.
/// </para>
/// </remarks>
private async Task AllowTcpForwardingAsync()
{
@@ -174,16 +119,14 @@ public sealed class SshServerFixture : IAsyncLifetime
"sh",
"-c",
"sed -i 's/^AllowTcpForwarding no/AllowTcpForwarding yes/' /config/sshd/sshd_config"
+ " && sed -i 's/^#*MaxStartups .*/MaxStartups 200/' /config/sshd/sshd_config"
+ " && pkill -HUP sshd",
]);
if (result.ExitCode != 0)
{
throw new InvalidOperationException(
$"Could not reconfigure the test server: {result.Stderr}");
$"Could not enable TCP forwarding on the test server: {result.Stderr}");
}
}
/// <summary>