Merge claude/linux-update-check: offer an RPM only to a copy rpm owns, and never read an unknown version as older

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

docs/manual-checks.md

@@ -3538,3 +3538,16 @@ Register a device, sync something, then `sudo dnf remove dodossh`.
 make an uninstall delete a user's un-synced work — the Linux spelling of the mistake ADR 0013 decision 2
 exists to prevent. Files left behind under `/opt/DodoSSH` are files the package did not own, which means
 something wrote into the install directory at runtime and would survive a reinstall.
+
+### 19.13 A copy at the same path that rpm does not own is offered nothing
+
+On a distribution without rpm — Arch, say, with a locally built package that puts the nightly tree at
+`/opt/DodoSSH.Nightly` — open PREFERENCES → UPDATES.
+
+**Pass:** **CHECK NOW** is disabled and the row says this copy was installed outside the `dodossh-nightly`
+RPM and should be updated with the package manager that installed it. `~/.local/share/dodossh.nightly/updates/`
+stays empty however long it runs.
+
+**Failure means:** a banner offering a nightly is the path being taken as proof the RPM is installed, or an
+rpm that cannot answer being read as an old version — the fault this check was written for, which offered
+every nightly as newer and fetched a 56 MB package the machine could never install, every six hours.

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

@@ -127,17 +127,61 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
         this.installedVersion = installedVersion;
         this.handToInstaller = handToInstaller;
 
-        packageName = string.Equals(channel, "nightly", StringComparison.Ordinal)
-            ? "dodossh-nightly"
-            : "dodossh";
+        packageName = PackageFor(channel);
+    }
+
+    /// <summary>The channel for a copy under the RPM's install root, if rpm says the package is there.</summary>
+    internal static IUpdateChannel ForInstall(string channel) =>
+        ForInstall(channel, InstalledRpmVersion, () => new LinuxUpdateChannel(channel));
+
+    /// <summary>
+    /// The channel for a copy under the RPM's install root, with rpm supplied, which is how the tests
+    /// reach it.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <b>The path says where this copy is and only rpm says who put it there.</b>
+    /// <c>UpdateChannels.LinuxChannel</c> has already established the first, and on an RPM distribution
+    /// that is the whole story. It is not on any other: a locally built pacman or dpkg package that mirrors
+    /// the spec puts the same tree at the same path, and this channel then offered every nightly as newer
+    /// than a version nobody could read, fetched a package that machine has no way to install, and did it
+    /// again every six hours.
+    /// </para>
+    /// <para>
+    /// Asked once, at composition, for the reason the rest of this decision is: who installed this copy
+    /// does not change while it runs. It costs one <c>rpm</c> launch at start on a machine whose path
+    /// already matched, and nothing at all anywhere else.
+    /// </para>
+    /// </remarks>
+    /// <param name="channel">The build's channel, which names the package.</param>
+    /// <param name="installedVersion">What rpm says is installed under a package name, or null.</param>
+    /// <param name="whenOwned">The channel to use when rpm does own this copy.</param>
+    internal static IUpdateChannel ForInstall(
+        string channel,
+        Func<string, string?> installedVersion,
+        Func<IUpdateChannel> whenOwned)
+    {
+        ArgumentException.ThrowIfNullOrEmpty(channel);
+        ArgumentNullException.ThrowIfNull(installedVersion);
+        ArgumentNullException.ThrowIfNull(whenOwned);
+
+        var package = PackageFor(channel);
+
+        return installedVersion(package) is null
+            ? new UnavailableUpdateChannel(
+                $"This copy of DodoSSH was installed outside the {package} RPM — rpm has no record of that "
+                + "package here — and an RPM is the only kind of update it can fetch. Update it with the "
+                + "package manager that installed it.")
+            : whenOwned();
     }
 
     /// <inheritdoc />
     /// <remarks>
     /// True unconditionally, because whether this copy is one that can be updated was already decided by
-    /// <see cref="UpdateChannels.ForThisMachine"/> — a build run from a checkout never reaches this class
-    /// at all. The same split Velopack's side uses, where <c>IsInstalled</c> is checked once at
-    /// composition rather than at every call site.
+    /// <see cref="UpdateChannels.ForThisMachine"/> and <see cref="ForInstall(string)"/> — a build run from
+    /// a checkout, or installed by something other than rpm, never reaches this class at all. The same
+    /// split Velopack's side uses, where <c>IsInstalled</c> is checked once at composition rather than at
+    /// every call site.
     /// </remarks>
     public bool IsSupported => true;
 
@@ -184,9 +228,20 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
                 $"The {channel} release's {manifestName} could not be read as a manifest.");
         }
 
+        // ◆ No answer from rpm is a failure, never a version. ForInstall only built this class because
+        // rpm answered, so silence now is a package removed under a running process or an rpm that
+        // stopped working — and any stand-in number would make every published build look newer and
+        // fetch a package for a machine that cannot install it, which is what this once did.
+        if (installedVersion(packageName) is not { } installed)
+        {
+            throw new InvalidOperationException(
+                $"rpm has no record of {packageName}, so there is no installed version to compare the "
+                + $"{channel} release against.");
+        }
+
         // The one place null is returned, and it means what null is documented to mean: this build is
         // current. Compared the way rpm compares, because rpm is what accepts or refuses the install.
-        if (RpmVersion.Compare(manifest.RpmVersion, installedVersion(packageName) ?? "0") <= 0)
+        if (RpmVersion.Compare(manifest.RpmVersion, installed) <= 0)
         {
             return null;
         }
@@ -324,6 +379,12 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
             ? $"{RepositoryApi}/releases/tags/nightly-desktop"
             : $"{RepositoryApi}/releases/latest";
 
+    /// <summary>The package a channel's builds are published as.</summary>
+    private static string PackageFor(string channel) =>
+        string.Equals(channel, "nightly", StringComparison.Ordinal)
+            ? "dodossh-nightly"
+            : "dodossh";
+
     private static string? Asset(LinuxForgeRelease? release, string name) =>
         release?.Assets?.FirstOrDefault(
             asset => string.Equals(asset.Name, name, StringComparison.Ordinal))?.DownloadUrl;
@@ -365,9 +426,11 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel
     /// <c>scripts/ci-linux-rpm.sh</c> performs, and two copies of one rule is a pair that can disagree.
     /// </para>
     /// <para>
-    /// A machine that cannot answer gets null, which the caller reads as <c>0</c> and makes every
-    /// published build look newer. That is the wrong way to fail, chosen deliberately: the failure is then
-    /// an install PackageKit declines rather than a security fix that never arrives.
+    /// A machine that cannot answer gets null — no <c>rpm</c> at all, or one with no such package — and
+    /// null means "rpm does not own this copy", never a version. It used to be read as <c>0</c>, on the
+    /// argument that the worst case was an install PackageKit declined. That holds only where PackageKit
+    /// installs RPMs: on any other distribution it made every published build look newer, forever, and
+    /// fetched a package that machine could not install. See <see cref="ForInstall(string)"/>.
     /// </para>
     /// </remarks>
     private static string? InstalledRpmVersion(string packageName)

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

@@ -66,21 +66,23 @@ internal static class UpdateChannels
         }
     }
 
-    /// <summary>The Linux channel, if this copy is one a package manager put here.</summary>
+    /// <summary>The Linux channel, if this copy is one the RPM put here.</summary>
     /// <remarks>
     /// <para>
     /// <b>The question Velopack answers with <c>IsInstalled</c>, answered by looking at where this process
-    /// lives.</b> The RPM owns <c>/opt/DodoSSH</c> or <c>/opt/DodoSSH.Nightly</c> — the spec puts it there
-    /// and nothing else writes to <c>/opt</c> under those names — so a build running from anywhere else is
-    /// a checkout somebody started, an extracted archive, or a tree copied by hand. None of those can be
-    /// updated by installing a package over them, and offering it would be an offer that cannot be carried
-    /// out.
+    /// lives and then asking rpm.</b> The RPM owns <c>/opt/DodoSSH</c> or <c>/opt/DodoSSH.Nightly</c> — the
+    /// spec puts it there — so a build running from anywhere else is a checkout somebody started, an
+    /// extracted archive, or a tree copied by hand. None of those can be updated by installing a package
+    /// over them, and offering it would be an offer that cannot be carried out.
     /// </para>
     /// <para>
-    /// Checked by path rather than by asking rpm which package owns the executable, which would be the
-    /// more thorough answer and costs a process launch on every start for a distinction nothing acts on
-    /// differently. The install root is a constant in two places — here and the packaging script — and
-    /// they are named in each other's comments.
+    /// <b>The path is necessary and not sufficient.</b> It used to be the whole check, on the argument
+    /// that asking rpm cost a process launch for a distinction nothing acted on differently. Something
+    /// does: a locally built pacman package that mirrors the spec puts the same tree at the same path,
+    /// and that copy was offered every nightly as an RPM it could never install. So a matching path is
+    /// then put to rpm — see <see cref="LinuxUpdateChannel.ForInstall(string)"/> — and the launch is paid
+    /// only where the path already matched. The install root is a constant in two places — here and the
+    /// packaging script — and they are named in each other's comments.
     /// </para>
     /// </remarks>
     [SupportedOSPlatform("linux")]
@@ -91,7 +93,7 @@ internal static class UpdateChannels
         var here = AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar);
 
         return string.Equals(here, root, StringComparison.Ordinal)
-            ? new LinuxUpdateChannel(DesktopChannel.Name)
+            ? LinuxUpdateChannel.ForInstall(DesktopChannel.Name)
             : new UnavailableUpdateChannel();
     }
 }

src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml

@@ -84,7 +84,10 @@
                          IsVisible="{Binding Updates.Status, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
               <TextBlock Classes="settingsrowcaption"
                          Text="This copy of DodoSSH cannot replace itself, so nothing above does anything. That is what a build run from a source checkout looks like, and also what a copy somebody unzipped by hand looks like — it is the installer that registers the update path."
-                         IsVisible="{Binding Updates.IsUnsupported}" />
+                         IsVisible="{Binding Updates.IsUnsupportedForTheUsualReason}" />
+              <TextBlock Classes="settingsrowcaption"
+                         Text="{Binding Updates.UnsupportedReason}"
+                         IsVisible="{Binding Updates.UnsupportedReason, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
             </StackPanel>
           </Border>
 

src/DodoSSH.Client.Session/ClientUpdates.cs

@@ -141,9 +141,32 @@ public interface IUpdateChannel
 /// <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>
+/// <para>
+/// <b>One situation outside those four carries a <see cref="Reason"/>.</b> A Linux copy sitting exactly
+/// where the RPM puts it that no RPM owns — built locally and installed by another distribution's package
+/// manager. It looks installed, so the sentence about checkouts and unzipped copies would describe the
+/// wrong machine, and the person reading it needs telling which updater is theirs.
+/// </para>
 /// </remarks>
 public sealed class UnavailableUpdateChannel : IUpdateChannel
 {
+    /// <summary>Creates one for the situations that need no telling apart.</summary>
+    public UnavailableUpdateChannel()
+    {
+    }
+
+    /// <summary>Creates one whose situation is worth a sentence of its own.</summary>
+    /// <param name="reason">What to tell the user, as a sentence they can act on.</param>
+    public UnavailableUpdateChannel(string reason)
+    {
+        ArgumentException.ThrowIfNullOrWhiteSpace(reason);
+
+        Reason = reason;
+    }
+
+    /// <summary>Why this copy cannot replace itself, where that is not the usual why.</summary>
+    public string? Reason { get; }
+
     /// <inheritdoc />
     public bool IsSupported => false;
 

src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs

@@ -126,6 +126,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
         this.restart = restart;
 
         CurrentVersion = updates.CurrentVersion;
+        UnsupportedReason = (updates as UnavailableUpdateChannel)?.Reason;
         isAutomatic = settings.Read().AutomaticUpdateChecks;
         state = updates.IsSupported ? UpdateState.Idle : UpdateState.Unsupported;
     }
@@ -133,6 +134,16 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
     /// <summary>What this build calls itself.</summary>
     internal string CurrentVersion { get; }
 
+    /// <summary>Why this copy cannot replace itself, when the channel had something particular to say.</summary>
+    /// <remarks>
+    /// Null in the ordinary cases, where the preferences screen's own sentence about checkouts and unzipped
+    /// copies is the right one. See <see cref="UnavailableUpdateChannel"/> for the case where it is not.
+    /// </remarks>
+    internal string? UnsupportedReason { get; }
+
+    /// <summary>Unsupported, with nothing more particular to say than the screen's own sentence.</summary>
+    internal bool IsUnsupportedForTheUsualReason => IsUnsupported && UnsupportedReason is null;
+
     [ObservableProperty]
     private UpdateState state;
 
@@ -248,6 +259,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
     {
         OnPropertyChanged(nameof(IsSupported));
         OnPropertyChanged(nameof(IsUnsupported));
+        OnPropertyChanged(nameof(IsUnsupportedForTheUsualReason));
         OnPropertyChanged(nameof(IsChecking));
         OnPropertyChanged(nameof(IsDownloading));
         OnPropertyChanged(nameof(IsReady));

tests/DodoSSH.Client.App.Tests/LinuxUpdateChannelTests.cs

@@ -86,19 +86,86 @@ public sealed class LinuxUpdateChannelTests : IDisposable
     }
 
     /// <summary>
-    /// A machine that cannot say what it has treats everything as newer.
+    /// A machine that cannot say what it has is told so, and is offered nothing.
     /// </summary>
     /// <remarks>
-    /// The wrong way to fail, chosen deliberately and for the reason the android channel gives: the cost is
-    /// an install the package manager declines, and the cost of the other direction is a security fix that
-    /// never arrives.
+    /// This used to read the silence as <c>0</c> and offer everything, on the argument that the cost was
+    /// an install the package manager declined. On a machine with no rpm at all — a pacman package of the
+    /// same tree at the same path — that was every nightly, every six hours, as a 56 MB download nothing
+    /// could install. Unknown is not older.
     /// </remarks>
     [Fact]
-    public async Task AnUnknownInstalledVersionTreatsEverythingAsNewer()
+    public async Task AnUnknownInstalledVersionThrowsRatherThanOfferingAnything()
     {
-        var channel = Channel(RandomNumberGenerator.GetBytes(16), published: "0.0.1~alpha.0.1-1", installed: null);
+        var handed = new List<string>();
+        var channel = Channel(
+            RandomNumberGenerator.GetBytes(16),
+            published: "0.0.1~alpha.0.200-1",
+            installed: null,
+            handToInstaller: handed.Add);
 
-        (await channel.CheckAsync(Token)).ShouldNotBeNull();
+        await Should.ThrowAsync<InvalidOperationException>(async () => await channel.CheckAsync(Token));
+
+        await Should.ThrowAsync<InvalidOperationException>(
+            async () => await channel.DownloadAsync(
+                new DodoSSH.Client.Session.AvailableUpdate("0.0.1-alpha.0.200"),
+                new Progress<int>(),
+                Token));
+
+        handed.ShouldBeEmpty();
+        Directory.Exists(Path.Combine(directory, "updates")).ShouldBeFalse();
+    }
+
+    /// <summary>
+    /// A copy rpm does not own gets no channel at all, and a sentence saying whose it is to update.
+    /// </summary>
+    /// <remarks>
+    /// The case that matters: the right path and no rpm — a locally built pacman package that mirrors the
+    /// spec. No binary and no such package both arrive here as null.
+    /// </remarks>
+    [Fact]
+    public void ACopyRpmDoesNotOwn_IsUnavailableAndSaysWhy()
+    {
+        var asked = new List<string>();
+
+        var updates = LinuxUpdateChannel.ForInstall(
+            "nightly",
+            package =>
+            {
+                asked.Add(package);
+
+                return null;
+            },
+            () => throw new InvalidOperationException("Built a channel for a copy rpm does not own."));
+
+        asked.ShouldBe(["dodossh-nightly"]);
+        updates.IsSupported.ShouldBeFalse();
+
+        var reason = updates.ShouldBeOfType<DodoSSH.Client.Session.UnavailableUpdateChannel>().Reason;
+
+        reason.ShouldNotBeNull();
+        reason.ShouldContain("outside the dodossh-nightly RPM");
+        reason.ShouldContain("package manager that installed it");
+    }
+
+    [Fact]
+    public void ACopyRpmOwns_GetsTheChannel()
+    {
+        var asked = new List<string>();
+        var owned = new FakeUpdateChannel();
+
+        var updates = LinuxUpdateChannel.ForInstall(
+            "release",
+            package =>
+            {
+                asked.Add(package);
+
+                return "0.0.1-1";
+            },
+            () => owned);
+
+        asked.ShouldBe(["dodossh"]);
+        updates.ShouldBeSameAs(owned);
     }
 
     [Fact]

tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs

@@ -80,6 +80,32 @@ public sealed class UpdateFlowTests : IDisposable
         Build().CurrentVersion.ShouldBe("0.4.2");
     }
 
+    [Fact]
+    public void ABuildThatCannotReplaceItself_ForAReasonOfItsOwn_SaysThatInstead()
+    {
+        var updates = new UpdateViewModel(
+            new UnavailableUpdateChannel("Update it with the package manager that installed it."),
+            new ClientSettingsStore(Paths),
+            TimeProvider.System,
+            () => 0,
+            restart: _ => Task.CompletedTask);
+
+        updates.IsUnsupported.ShouldBeTrue();
+        updates.UnsupportedReason.ShouldBe("Update it with the package manager that installed it.");
+        updates.IsUnsupportedForTheUsualReason.ShouldBeFalse();
+    }
+
+    [Fact]
+    public void ABuildThatCannotReplaceItself_ForTheUsualReasons_LeavesTheScreenItsOwnSentence()
+    {
+        channel.IsSupported = false;
+
+        var updates = Build();
+
+        updates.UnsupportedReason.ShouldBeNull();
+        updates.IsUnsupportedForTheUsualReason.ShouldBeTrue();
+    }
+
     [Fact]
     public async Task ABackgroundPassThatFindsNothing_SaysNothing()
     {