From 25407756c3ee1b1e67735e4523b08dc8817390f1 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Wed, 12 Aug 2026 10:34:36 +0200 Subject: [PATCH] Look for a newer build the moment the application starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass of the update loop waited two minutes. Every pass after it came six hours apart, which is the right interval for a product that ships rarely — but the delay in front of the first one quietly excluded a whole way of using this application. A client opened to reach one host and closed again is over before the two minutes are. Used that way, it never checks at all: not once, not slowly, never. That is precisely the machine ADR 0011 names as the real cost of distributing outside a store — quietly a year behind — and the galling part is that the mechanism to fix it was switched on the whole time and simply never reached. The delay's own argument is recorded in the diff it is being removed from, and it was not a bad one: nothing anybody does in their first two minutes depends on an update, and launch is already contending for the network with a schema migration, a resumed sign-in and a first sync, at the one moment somebody is watching the window. What it weighed was the cost of checking early against the benefit of checking early. It never weighed the cost of not checking at all. ◆ THE YIELD IS WHAT KEEPS THIS OFF THE LAUNCH PATH, AND IT IS NOT DECORATION. Start() is called from MainWindowViewModel.StartAsync ahead of the migration, so an inline first pass would run whatever the channel does before its own first await — Velopack reads the install layout from disk — between the user and their window. Yielding hands the rest of launch back and puts the check in a later turn, which is the same moment in every sense anybody can perceive and none of the cost. So the answer to the delay's argument is not that it was wrong; it is that a yield buys most of what two minutes bought. Task.Yield takes no token where Task.Delay did, so the loop body now observes cancellation at its head. Without that, an application closed during launch spends its last moment asking a release channel about a build it will not run. Two things deliberately not changed. The AUTOMATIC UPDATE CHECKS preference still gates the pass — "on start" means every start, not regardless of what the user asked for, and that setting is already on by default. And the data cost is unchanged rather than merely acceptable: a check is a few hundred bytes and the download only follows if something newer exists, so this moves the same traffic earlier without adding any. That matters most on the phone, where the same loop runs against AndroidUpdateChannel. TheFirstPassRunsAtStart_RatherThanOnADelay drives the real loop rather than CheckOnceAsync, which is the one thing that file otherwise avoids — and here it is the point, because the claim is about when the pass happens rather than what it does. It waits on the pass and not on a clock, so there is nothing to be flaky about: a regression that puts a delay back does not fail on a margin, it spins until the suite's own cancellation ends it. DisposingStopsTheLoop keeps its assertion and gains a note that it is now a race rather than a formality. §16.7 of the manual checks gains the sentence that reopening the application does what CHECK NOW does. It is the step somebody following that section would otherwise discover by accident. 447 App tests and 153 layout tests pass. --- docs/manual-checks.md | 4 +- .../ViewModels/UpdateViewModel.cs | 36 ++++++++++++----- .../UpdateFlowTests.cs | 40 +++++++++++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/docs/manual-checks.md b/docs/manual-checks.md index ea206ea..79da807 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -2423,7 +2423,9 @@ script warns rather than failing when that is legitimate, which is the first rel ### 16.7 The update arrives, and the restart lands in it · **the whole point of the work** With v0.1.0 installed and running, a vault unlocked, a host change made, and **a terminal open**, publish -v0.1.1 (`-Upload`). Then press CHECK NOW on Settings → General rather than waiting six hours. +v0.1.1 (`-Upload`). Then press CHECK NOW on Settings → General rather than waiting six hours. Closing and +reopening the application does the same thing without the button: the first pass of the loop runs at launch, +so a client started after a release finds it without anybody asking. **Pass:** the progress bar moves, the banner appears above the status bar, and — the part to actually watch — the terminal **reflows cleanly rather than being sliced**, with the remote seeing the smaller row count. diff --git a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs index 880924f..6df44ae 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs @@ -74,16 +74,6 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab /// private static readonly TimeSpan CheckInterval = TimeSpan.FromHours(6); - /// How long to wait before the first pass. - /// - /// A delay, where VaultViewModel's sync loop runs a pass immediately. The difference is what the - /// user is waiting for: a vault edited on another machine should be current by the time they have - /// finished reading the list, whereas nothing anybody does in their first two minutes depends on an - /// update. Launch is already contending for the network and the CPU with a schema migration, a resumed - /// sign-in and a first sync, at the one moment somebody is watching the window. - /// - private static readonly TimeSpan FirstCheckDelay = TimeSpan.FromMinutes(2); - private readonly IUpdateChannel updates; private readonly ClientSettingsStore settings; private readonly TimeProvider clock; @@ -242,16 +232,40 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab loop = RunCheckLoopAsync(lifetime.Token); } + /// + /// + /// The first pass runs at launch, with no delay in front of it. It used to wait two minutes, on + /// the argument that nothing anybody does in their first two minutes depends on an update and launch is + /// already contending for the network with a schema migration, a resumed sign-in and a first sync. What + /// that argument leaves out is the run that is over before the two minutes are: a client opened to reach + /// one host and closed again never checks at all, and a machine used that way is exactly the one ADR + /// 0011 warns about — quietly a year behind, with the mechanism to fix it switched on and never reached. + /// Every start now asks. + /// + /// + /// The yield is what keeps that off the launch path. is called from + /// MainWindowViewModel.StartAsync before the migration, so running the pass inline would put + /// whatever the channel does before its own first await — Velopack reads the install layout from disk — + /// between the user and their window. Yielding hands the rest of the launch back and lets the check run + /// in a later turn, which is the same moment in every sense that matters and none of the cost. + /// + /// private async Task RunCheckLoopAsync(CancellationToken cancellationToken) { try { - await Task.Delay(FirstCheckDelay, clock, cancellationToken).ConfigureAwait(true); + await Task.Yield(); using var timer = new PeriodicTimer(CheckInterval, clock); do { + // Task.Yield takes no token, unlike the delay it replaced, so a shutdown that lands while + // the loop is waiting to be handed back the thread has to be observed here rather than + // only at the next tick. Otherwise an application closed during launch spends its last + // moment asking a release channel about a build it is not going to run. + cancellationToken.ThrowIfCancellationRequested(); + await CheckOnceAsync(cancellationToken).ConfigureAwait(true); } while (await timer.WaitForNextTickAsync(cancellationToken).ConfigureAwait(true)); diff --git a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs index d47bb73..cee898d 100644 --- a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs @@ -403,6 +403,46 @@ public sealed class UpdateFlowTests : IDisposable channel.Checks.ShouldBe(1); } + /// + /// + /// The loop rather than CheckOnceAsync, which is the one thing the rest of this file avoids + /// driving — and here it is the whole point, because the claim is about when the first pass happens + /// rather than about what it does. The first pass used to wait two minutes, which meant a client opened + /// to reach one host and closed again never asked at all. + /// + /// + /// It waits on the pass and not on a clock, so there is nothing here to be flaky about: a regression + /// that puts a delay back in front of the loop does not fail on a margin, it spins until the suite's own + /// cancellation ends it. + /// + /// + [Fact] + public async Task TheFirstPassRunsAtStart_RatherThanOnADelay() + { + channel.Available = new AvailableUpdate("1.3.0"); + + var updates = Build(); + await using var _ = updates.ConfigureAwait(false); + + updates.Start(); + + while (updates.State is not UpdateState.Ready) + { + Token.ThrowIfCancellationRequested(); + + await Task.Yield(); + } + + channel.Checks.ShouldBe(1); + updates.ReadyVersion.ShouldBe("1.3.0"); + } + + /// + /// Started and disposed with nothing in between, which since the first pass stopped waiting two minutes + /// is a race rather than a formality: the loop may be anywhere between its yield and a finished check + /// when the cancellation lands. What is asserted is what matters either way — that disposing returns, + /// rather than waiting on a pass that will never be allowed to finish. + /// [Fact] public async Task DisposingStopsTheLoop() {