Stop the updater offering the build it saw first, whatever has been published since

A client two versions behind was offered the older of the two, installed it,
restarted, and was told at once that another update was waiting. Two downloads,
two installs and two restarts to move one step.

The feed was not the problem and could not have been. The nightly release is
deleted and remade on every push to main, so it holds exactly one build: a client
can only ever be offered the newest thing there is. The wrong version came from
this application's own state.

◆ THE PASS STOPPED ASKING, WHICH IS NOT WHAT IT WAS MEANT TO STOP DOING.

    if (!IsAutomatic || State is UpdateState.Downloading or UpdateState.Ready)

Ready means something is downloaded and waiting. The intent behind excluding it is
right — there is no point fetching what has already been fetched — and what it
actually did was skip the check. So from the first thing any process downloaded,
its banner was pinned to that build for the rest of its life, however many newer
ones were published afterwards. On a rolling channel that is the normal case
rather than a corner.

Not re-fetching is still the rule and now lives in FetchAsync, which compares
versions rather than refusing to look: the same build is left alone, a newer one
replaces what was pending. Which is also why FetchAsync answers with what is
*pending* rather than with what the check returned — a client on 1.0 holding 1.1
is told 1.1 by every later check, because 1.1 is still the newest thing on the
feed, so "the check found something" and "there is something new" are different
questions and only the second one is worth a download.

A build already fetched is never discarded, including when a check answers null.
Null means the feed's newest is no newer than the build that is *running*, which
says nothing about the one on disk — that one is still newer, and withdrawing an
offer somebody has already been shown would be the worse answer.

◆ AND A LAST LOOK BEFORE THE PROCESS IS REPLACED, WHICH IS THE HALF THE TIMER
  CANNOT REACH.

The loop asks every six hours. The build that caused this was published minutes
before RESTART NOW was pressed, so no interval anybody would accept would have
caught it. The press now asks once more.

It costs one small request in the ordinary case, because the feed answers with the
build already on disk and nothing is fetched. It costs a download only when there
genuinely is something newer, which is the case where the alternative was two
downloads anyway.

Bounded at ten seconds, and the wait rather than the request: VelopackUpdateChannel
records that Velopack's check takes no token, so cancelling would be observed only
after the call had already returned. Ten seconds bounds a small JSON document
against a network that drops packets rather than refusing them; a refused
connection fails in milliseconds and never reaches it.

Nothing here refuses a restart. An unreachable forge, a look that ran out of time,
a fetch of the newer build that failed halfway — all leave the build on disk
untouched and still newer than the one running, so the restart goes ahead with it.
Hence swallowed rather than reported: Failed is for something a person is waiting
on an answer to, and this person is waiting on a restart they are about to get.

IsRestarting keeps the banner up through it. Everything the strip draws keys off
IsReady and the look moves the state off that, so without it pressing RESTART NOW
made the banner vanish with nothing on screen saying why, for as long as a 120 MB
package takes. It says what it is doing instead, percentage and all.

◆ THE CHANNEL HAD TO LEARN WHICH BUILD IT FETCHED, AS AGAINST WHICH IT LAST FOUND.

Required by both halves above rather than tidied along with them. Matched()
resolved an AvailableUpdate only against `found`, which a check overwrites — and
both changes make this application check while a build is already downloaded. A
check that lands a newer answer and then fails to fetch it, or is given up on
halfway, leaves the shell restarting with the build it already has and Matched()
throwing at it: the offer real, the package on disk real, and the only thing wrong
being that the channel had forgotten which one it was. A slow forge became a
refused restart.

So `fetched` is recorded after a download returns, tried before `found`, and
survives every later check.

Settings → General claimed checks start "a couple of minutes after launch", which
has been untrue since the first pass moved to launch. It now also says the loop
keeps asking while an update is waiting, because that is the behaviour somebody
reading that row would want to know about.

UpdateFlowTests goes from 21 to 28: the pass asks again and fetches once, the
pending build is replaced when the feed moves on, a superseded offer reappears
through a dismissal, the press installs what the last look finds, and the three
ways the last look can fail all still install what was already on disk. Check
16.7a is the manual half, needing two real releases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs

@@ -204,6 +204,25 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel
     /// </remarks>
     private UpdateInfo? found;
 
+    /// <summary>
+    /// The last thing actually fetched, kept separately from the last thing found.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// ◆ <b>Because the two can disagree, and when they do it is an apply that pays for it.</b> A check
+    /// overwrites <see cref="found"/>, and the shell now checks while a build is already downloaded and
+    /// waiting — that is how it replaces a pending build when the feed has moved on, and how it takes one
+    /// last look on the way to a restart. Either of those can land a newer answer in <c>found</c> and then
+    /// fail to fetch it, or be given up on halfway; the shell's answer is to restart with the build it
+    /// already has, and <see cref="Matched"/> resolving only against <c>found</c> would have thrown at it.
+    /// </para>
+    /// <para>
+    /// Which turned a slow forge into a refused restart: the offer on screen was real, the package on disk
+    /// was real, and the only thing wrong was that this class had forgotten which one it was.
+    /// </para>
+    /// </remarks>
+    private UpdateInfo? fetched;
+
     internal VelopackUpdateChannel(UpdateManager manager) => this.manager = manager;
 
     /// <inheritdoc />
@@ -321,7 +340,23 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel
 
         // Velopack reports progress as an Action<int> and the rest of this codebase speaks IProgress<T>,
         // so the adaptation happens here rather than leaking the older shape into the view models.
-        return manager.DownloadUpdatesAsync(Matched(update), progress.Report, cancellationToken);
+        return DownloadAndRememberAsync(Matched(update), progress, cancellationToken);
+    }
+
+    /// <remarks>
+    /// Recorded after the await and not before, so that a download which threw leaves the previous fetch as
+    /// the one this channel can still apply. See <see cref="fetched"/>.
+    /// </remarks>
+    private async Task DownloadAndRememberAsync(
+        UpdateInfo info,
+        IProgress<int> progress,
+        CancellationToken cancellationToken)
+    {
+        await manager
+            .DownloadUpdatesAsync(info, progress.Report, cancellationToken)
+            .ConfigureAwait(false);
+
+        fetched = info;
     }
 
     /// <inheritdoc />
@@ -335,21 +370,39 @@ internal sealed class VelopackUpdateChannel : IUpdateChannel
     }
 
     /// <remarks>
+    /// <para>
     /// The guard exists because the seam narrows <c>UpdateInfo</c> down to a version string, so nothing in
     /// the type system stops a caller inventing one. Every legitimate caller passes back exactly what
-    /// <see cref="CheckAsync"/> returned; a mismatch is a bug in this application rather than anything a
-    /// user did, which is why it throws rather than resolving to some safe-looking default.
+    /// <see cref="CheckAsync"/> returned, or what a download of that then fetched; a mismatch is a bug in
+    /// this application rather than anything a user did, which is why it throws rather than resolving to
+    /// some safe-looking default.
+    /// </para>
+    /// <para>
+    /// What was fetched is tried before what was found, because that is the one an apply names and the one a
+    /// later check cannot take away. See <see cref="fetched"/> for how the two come to disagree.
+    /// </para>
     /// </remarks>
     private UpdateInfo Matched(AvailableUpdate update)
     {
-        if (found is not { } info
-            || !string.Equals(info.TargetFullRelease.Version.ToString(), update.Version, StringComparison.Ordinal))
+        if (Names(fetched, update))
         {
-            throw new InvalidOperationException(
-                $"No update matching {update.Version} has been found by this channel. "
-                + "Call CheckAsync and pass back what it returned.");
+            return fetched!;
         }
 
-        return info;
+        if (Names(found, update))
+        {
+            return found!;
+        }
+
+        throw new InvalidOperationException(
+            $"No update matching {update.Version} has been found or fetched by this channel. "
+            + "Call CheckAsync and pass back what it returned.");
     }
+
+    private static bool Names(UpdateInfo? info, AvailableUpdate update) =>
+        info is not null
+        && string.Equals(
+            info.TargetFullRelease.Version.ToString(),
+            update.Version,
+            StringComparison.Ordinal);
 }