Let the desktop client replace itself, and give the repository one version

Packaging for Windows, and the updater that only exists once something is
packaged. Velopack, win-x64, fed from the project's own forge — never from the
deployment a client signs in to, which is ADR 0011 rule 2 carried over
unchanged and is why the feed address is a constant in the code rather than a
setting. See docs/adr/0012-desktop-distribution-and-updates.md.

**Nothing is ever installed while somebody is using it.** A newer build is found
on a six-hourly pass, downloaded in the background, and then waits — for a
restart the user presses, or for the next launch they were going to do anyway.
That is a policy rather than caution: this application argues at length that
locking keeps shells running, because a lock that destroyed work would stop
being used, and a restart does not keep them. Having taught that, it owes the
user the choice at the one moment it stops being true, and the sentence saying
so counts the shells it would close.

**The version is now derived from the v* tag**, by MinVer, for everything. There
was no version before this — no property anywhere, so every assembly reported
the SDK's 1.0.0 and the API served that string as its serverVersion to every
client that asked. The tag was already the version of record for the container
image; this makes it the version of record full stop. MinVer's failure mode is
answering plausibly rather than failing, and here a wrong version is a client
that never updates, so it is guarded twice: fetch-depth 0 on every checkout, and
a step that fails a tag build when the tag and the computed version disagree.

**The pack id is DodoSSH.Desktop and not DodoSSH**, which is the one decision
here that would have destroyed data. Velopack installs to %LOCALAPPDATA%\<packId>
and removes that whole directory on uninstall, and %LOCALAPPDATA%\DodoSSH is
where ClientPaths keeps the encrypted cache, the outbox of changes not yet
pushed, and the device key. The obvious id would have had the uninstaller
silently delete work the server has never seen — the thing the application
refuses to do without a counted confirmation. Velopack's own advice to move user
data to roaming %APPDATA% is declined for the reason ClientPaths already gives.

**Releases are cut by a person, and CI gains no job that could.** The tempting
argument is that a forge write token is not a signing key. It does not survive
contact with what the token does: Velopack clients trust their feed and do not
verify a package signature when they apply one, so whoever can write a release
can ship an update every install runs. That is the capability ADR 0011 rule 1
puts on a machine which is not a runner, reached through a different door. The
mechanical objection — vpk needs Windows and the runners are Linux — is the
smaller of the two and is recorded beside it, because somebody will fix one and
believe they are done.

Unsigned for now, deliberately and with the cost stated where a user reads it:
SmartScreen warns once per person, on Setup.exe, because Mark-of-the-Web is
applied by the browser that downloaded it. In-app updates are fetched by the
application and applied from a local file, and never trip it.

The banner is a fourth row of the window rather than an overlay. Anything drawn
in the terminal's rectangle is sliced by the native child window that composites
above it — the defect this window has shipped once — and a sibling row is the
arrangement TitleBar and StatusBar already prove works.

----

Three defects surfaced on the way, none of them in the feature being built.

**A settings key absent from the file came back as the CLR default, not the
declared one.** The JSON source generator builds a record through a synthesised
parameterised constructor and assigns every property from its argument array, so
a property initializer runs and is then overwritten by a default for anything the
file did not contain. A settings.json of {} read back a font size of 0, clamped
up to the 8px floor rather than the 13px the renderer draws at. It could not bite
while there was one setting, because that setting was written on every save and
so was never absent; adding a second would have turned automatic update checks
off for every existing profile, silently, the opposite of the documented default.
Reflection-based deserialisation of the same JSON answers correctly, which is why
every way of checking it by hand agrees except the one that ships. The defaults
now live on the constructor parameters, which is the only place the generator
reads them from.

**Declaring a RuntimeIdentifier on the desktop head broke the server's image
build.** It is the obvious way to let a self-contained publish restore under
locked mode, and it writes a net10.0/win-x64 target into the lock file of every
project the head references transitively — including DodoSSH.Contracts and
DodoSSH.Crypto, which the API builds too. The Dockerfile restores those with no
RID and fails NU1004. Found by running docker build rather than by reading. The
RID stays out of the committed state; the two commands that need one ask for it
unlocked, and the release script puts the lock files back.

**A Docker ARG named VERSION silently sets MSBuild's Version.** An ARG is an
environment variable for the rest of the stage, MSBuild reads environment
variables as properties, and property names are case-insensitive. With the
workflow passing main-<short sha> on a main build the publish died with
NETSDK1018 pointing at DodoSSH.Contracts, a project nobody had touched. The build
stage's argument is ASSEMBLY_VERSION now, empty except on a tag build.

All three are in docs/platform-flags.md, which is where the next person will look.

----

Verified: the whole solution builds and restores locked; 289 shell, 93 layout and
54 session tests pass, including the regression test for the settings defect and
a measurement of the banner at the window's minimum width. vpk pack runs end to
end and reports "Verified VelopackApp.Run()" against Program.Main. The API image
builds correctly both as a main build and as a tag build, carrying 1.0.0 and
0.1.0 respectively.

Not verified, and it needs a published release to be: installing, updating and
uninstalling on a real machine. That is Phase 15 of docs/manual-checks.md, and
the pack id and the WebView2 profile fix are reasoned and commented but only
proved by walking it. Two things to watch at the first upload — the reverse
proxy's body-size limit for a 64 MB asset, and whether vpk upload gitea is happy
with Gitea 1.27.1.
This commit is contained in:
2026-08-04 17:04:41 +02:00
parent 176df67861
commit 6728a0a597
66 changed files with 3190 additions and 44 deletions
+59 -3
View File
@@ -19,8 +19,38 @@ namespace DodoSSH.Client.Session;
/// belongs in the cache, behind the passphrase.
/// </para>
/// </remarks>
public sealed record ClientSettings
/// <param name="TerminalFontSize">The terminal font size, in CSS pixels.</param>
/// <param name="AutomaticUpdateChecks">
/// Whether this machine looks for a newer build on its own. See the remarks on the property.
/// </param>
public sealed record ClientSettings(
int TerminalFontSize = ClientSettings.DefaultTerminalFontSize,
bool AutomaticUpdateChecks = true)
{
/*
A positional record, and the defaults live on the parameters rather than on property initializers.
That is not a style choice it is the only place System.Text.Json will read them from, and getting
it wrong fails silently.
What the source generator emits for a record is an ObjectWithParameterizedConstructorCreator: it
treats the init-only properties as constructor arguments and builds the object as
new ClientSettings() { TerminalFontSize = (int)args[0], AutomaticUpdateChecks = (bool)args[1] }
so a property initializer does run and is then immediately overwritten by args, which for a member
absent from the JSON is the CLR default. Measured before this was fixed: a settings file of {} read
back TerminalFontSize 0, clamped to the 8px floor rather than the 13px the renderer draws at, and
AutomaticUpdateChecks false. Reflection-based deserialisation of the same JSON answers 13 and true,
which is what makes it so easy to miss. Setting GenerationMode to Metadata does not help either; it
was tried.
Constructor parameter defaults are carried into the generated metadata, so absence resolves to the
value declared here. ASettingAbsentFromTheFile_ComesBackAsItsDeclaredDefault fails without this.
The type is still used exactly as before new ClientSettings(), and `with` expressions because
every parameter is optional.
*/
/// <summary>The size a terminal draws at with nothing stored.</summary>
/// <remarks>
/// Matches the renderer's own default, and has to: the page creates panes at its own constant until
@@ -43,8 +73,34 @@ public sealed record ClientSettings
/// </remarks>
public const int MaximumTerminalFontSize = 32;
/// <summary>The terminal font size, in CSS pixels.</summary>
public int TerminalFontSize { get; init; } = DefaultTerminalFontSize;
/*
AutomaticUpdateChecks: why it is on by default, and the three neighbours it deliberately does not
have. Prose rather than XML doc because the property is declared in the parameter list above, and a
param tag is the wrong shape for several paragraphs.
On by default. A client that quietly runs a year behind is the failure ADR 0011 names as the real
cost of distributing outside a store, and the desktop has the mechanism to avoid it so the
default should use it. It costs little to leave on, because a check downloads and never installs:
nothing happens to a running application except a line offering a restart.
Three neighbours this deliberately does not have, because each would be a mistake worth
naming rather than an omission.
No feed address. Where builds come from is a constant in the code, and ADR 0011 rule 2 is
why: an operator who could point this at themselves could pin a chosen user to a build with a known
hole. This file is plaintext and hand-editable, which is exactly what makes it the wrong home for
that value.
No last-checked timestamp. The rule above about nothing secret is not only about secrets
a record of when this machine last contacted the project's forge is a small fact about a person
that this file does not otherwise carry. It also buys little: the check runs on a timer measured
from launch, so any machine that runs at all is current within hours. What the screen shows is
remembered for the session and no longer.
No channel switch. There is one release channel. A toggle offering a second would be a
preference with nothing behind it, which is the thing the preferences screen's own opening comment
warns against.
*/
/// <summary>Brings a value inside the range this type will store.</summary>
public static int ClampTerminalFontSize(int pixels) =>
+166
View File
@@ -0,0 +1,166 @@
using System.Reflection;
namespace DodoSSH.Client.Session;
/// <summary>
/// A build newer than the one running.
/// </summary>
/// <remarks>
/// A version string and nothing else, so that whatever the update framework hands back never crosses this
/// seam. The implementation keeps its own richer answer privately and matches on the version when it is
/// handed one of these back; everything above only ever needs the number to print.
/// </remarks>
/// <param name="Version">What the newer build calls itself.</param>
public sealed record AvailableUpdate(string Version);
/// <summary>
/// Where newer builds of this client come from.
/// </summary>
/// <remarks>
/// <para>
/// <b>The release channel is the project's own, and never the deployment the client is signed in to.</b>
/// That is ADR 0011 rule 2 — see <c>docs/adr/0011-android-distribution.md</c> — and it is a security
/// property rather than a preference:
/// an operator who could answer the update check could pin a chosen user to a build with a known hole, by
/// withholding the answer, without holding any key at all. The implementation's feed address is therefore
/// a constant and not a setting, and this interface has no member that would let a caller name one.
/// </para>
/// <para>
/// <b>An interface rather than a delegate</b>, which is a deliberate departure from the shell's habit —
/// <c>SignInHandler</c>, <c>resume</c> and <c>copyToClipboard</c> are all delegates so that the view models
/// stay drivable by a test with no network. That shape fits a single call. This is four operations, an
/// availability question and an order they have to happen in, and four delegates in a constructor is worse
/// than one interface. <see cref="IDeviceKeyStore"/> is the same shape in the same assembly and settled the
/// same trade.
/// </para>
/// <para>
/// Nothing here throws for the ordinary failures. A forge that cannot be reached is a laptop on a train,
/// not a fault, and the caller's answer is to try again later.
/// </para>
/// </remarks>
public interface IUpdateChannel
{
/// <summary>Whether this copy of the client is one that can replace itself.</summary>
/// <remarks>
/// False for a build run from a source checkout, for a copy somebody unzipped by hand, and for every
/// platform this has not been built for. Asked before offering anything, because an offer that cannot
/// be carried out is worse than no offer.
/// </remarks>
bool IsSupported { get; }
/// <summary>What this build calls itself.</summary>
/// <remarks>
/// Answered even when <see cref="IsSupported"/> is false. Which version is running is a fact worth
/// printing on a build that cannot update itself — arguably more so, since somebody will have to
/// replace it by hand.
/// </remarks>
string CurrentVersion { get; }
/// <summary>Asks the release channel whether there is anything newer.</summary>
/// <returns>
/// The newer build, or <see langword="null"/> if this one is current. Null is also the honest answer
/// when the channel cannot be reached at all: the caller does the same thing either way, and an
/// unreachable forge is not a state a user can act on.
/// </returns>
Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken);
/// <summary>Fetches an update a previous <see cref="CheckAsync"/> found.</summary>
/// <param name="update">The update to fetch. Must be one this source returned.</param>
/// <param name="progress">Reports percentage complete.</param>
/// <param name="cancellationToken">Cancellation token.</param>
Task DownloadAsync(AvailableUpdate update, IProgress<int> progress, CancellationToken cancellationToken);
/// <summary>
/// Swaps this build for the downloaded one and starts it.
/// </summary>
/// <remarks>
/// <b>This does not return.</b> It is the reason the whole capability is behind an interface rather
/// than being three lines inline in the view model: a test cannot drive a method that ends the process,
/// and the single most important thing to be able to assert about this feature is that a downloaded
/// update is <em>never</em> applied unless somebody asked for it. A fake that records the call instead
/// of making it is what makes that assertion writable.
/// </remarks>
/// <param name="update">The update to apply. Must be one that has been downloaded.</param>
void ApplyAndRestart(AvailableUpdate update);
}
/// <summary>
/// A copy of the client that cannot replace itself.
/// </summary>
/// <remarks>
/// <para>
/// Four different situations resolve to this one object, and they are deliberately not distinguished: a
/// build started from a source checkout, a copy extracted by hand from an archive, a desktop platform this
/// has not been packaged for, and the phone — whose distribution is settled separately by ADR 0011 and has
/// no updater at all. All four mean the same thing. <em>This copy was not installed by anything that knows
/// how to replace it</em>, and saying so is different from pretending otherwise.
/// </para>
/// <para>
/// A null object rather than a nullable field on the view model, following
/// <see cref="UnavailableDeviceKeyStore"/>: the caller then has one shape to write against, and "no
/// updater here" is a thing the interface can express rather than a case every call site has to remember.
/// </para>
/// </remarks>
public sealed class UnavailableUpdateChannel : IUpdateChannel
{
/// <inheritdoc />
public bool IsSupported => false;
/// <inheritdoc />
public string CurrentVersion => ClientVersion.Current;
/// <inheritdoc />
public Task<AvailableUpdate?> CheckAsync(CancellationToken cancellationToken) =>
Task.FromResult<AvailableUpdate?>(null);
/// <inheritdoc />
public Task DownloadAsync(
AvailableUpdate update,
IProgress<int> progress,
CancellationToken cancellationToken) =>
throw new NotSupportedException(
"This copy of DodoSSH cannot replace itself. Check IsSupported before offering an update.");
/// <inheritdoc />
public void ApplyAndRestart(AvailableUpdate update) =>
throw new NotSupportedException(
"This copy of DodoSSH cannot replace itself. Check IsSupported before offering an update.");
}
/// <summary>
/// What this build calls itself.
/// </summary>
/// <remarks>
/// <para>
/// Read from the assembly rather than from the update framework, so that it answers on a build that has no
/// updater — a source checkout, or the phone. The version a user is shown should not depend on whether the
/// thing that could replace it happens to be present.
/// </para>
/// <para>
/// The informational version is the one that carries the whole number, including any prerelease part;
/// <c>AssemblyVersion</c> is <c>major.0.0.0</c> by MinVer's design and would print <c>0.0.0</c> for every
/// 0.x build. The <c>+sha</c> build metadata is trimmed because it is for a machine, and this string is
/// read aloud in support conversations.
/// </para>
/// </remarks>
public static class ClientVersion
{
/// <summary>The running build's version, without build metadata.</summary>
public static string Current { get; } = Read();
private static string Read()
{
var informational = (Assembly.GetEntryAssembly() ?? typeof(ClientVersion).Assembly)
.GetCustomAttribute<AssemblyInformationalVersionAttribute>()
?.InformationalVersion;
if (string.IsNullOrWhiteSpace(informational))
{
return "0.0.0";
}
var plus = informational.IndexOf('+', StringComparison.Ordinal);
return plus < 0 ? informational : informational[..plus];
}
}
@@ -14,6 +14,12 @@
"resolved": "5.6.0",
"contentHash": "Kcobt3pnOdO0A+6CKiMHZdTEluJpsfxiV20axtZdmfBQnDmiWTKPJADlgAfdTuKNAnVarrkJa0UEGwuOo91muw=="
},
"MinVer": {
"type": "Direct",
"requested": "[7.0.0, )",
"resolved": "7.0.0",
"contentHash": "2lMTCQl5bGP4iv0JNkockPnyllC6eHLz+CoK2ICvalvHod+exXSxueu9hq+zNkU7bZBJf8wMfeRC/Edn8AGmEg=="
},
"Microsoft.Data.Sqlite.Core": {
"type": "Transitive",
"resolved": "10.0.10",