Merge branch 'claude/pipeline-curl-not-found-97e70f'

docs/adr/0014-android-updates.md

@@ -84,6 +84,18 @@ losing the local cache, the outbox and the device key.
    turned this application on in the unknown-sources screen. Two deliberate answers, neither to a screen
    DodoSSH controls.
 
+   ◆ "Asks Android to ask" is one step longer than it sounds, and reading it as one step is why this
+   installed nothing at all from the day it shipped until 2026-08-05. An application holding only
+   `REQUEST_INSTALL_PACKAGES` gets no verdict back from committing a session: what the platform returns
+   first is `STATUS_PENDING_USER_ACTION`, carrying the confirmation activity in `Intent.EXTRA_INTENT` for
+   the application to start. **Android does not draw the dialogue on its own.** The commit succeeded, the
+   session staged, and nobody was ever asked anything. The receiver that starts it is
+   `InstallSessionReceiver`; the intent naming it has to be explicit, since a mutable pending intent
+   wrapping an implicit one is refused outright from API 34. Check 17.5 asks for exactly the right thing
+   — "Android's own installer appears naming the package" — so nothing needed rewording; it wants an
+   older build installed and a newer one published to run at all, and on the evidence it was never run
+   against a real pair.
+
 7. **Applying does not end the process, and the shell had to learn that.** On Windows, applying replaces
    the files and restarts, so the shell disposes the vault first — that is what zeroes the identity keys,
    the vault keys and the cache key. On Android the install is a *request* and the answer may be no, so

docs/manual-checks.md

@@ -2091,6 +2091,14 @@ around being broken. An `INSTALL_FAILED_UPDATE_INCOMPATIBLE` means the two build
 keys — on the nightly channel that means the committed keystore changed, and on the release channel it means
 the wrong keystore was used.
 
+◆ Two failures worth naming separately, because both look like "the button does nothing" and neither is a
+signing problem. **The application closes when INSTALL is pressed** — that is an exception escaping the
+command handler, and the message it should have shown is now the Failed line on this screen. **Nothing
+happens at all, and the application carries on** — Android was asked to install and nobody started the
+confirmation it handed back; see `InstallSessionReceiver` and the ◆ note in ADR 0014 rule 6. This check is
+the only thing in the project that can catch either, which is the argument for running it on a real pair of
+builds rather than reasoning about it.
+
 ### 17.6 Declining leaves a working session · **the one that would be missed**
 
 Repeat 17.5 to the point where Android's installer is on screen, with a terminal open and the vault

src/DodoSSH.Client.Android/Platform/AndroidUpdateChannel.cs

@@ -298,6 +298,19 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
     /// and a grant on every launch, all so the installer can read a file this application already has
     /// open — where a session hands it the bytes directly.
     /// </para>
+    /// <para>
+    /// ◆ <b>The pending intent handed to <c>Commit</c> is not a notification, and treating it as one is
+    /// why this silently installed nothing.</b> An application holding <c>REQUEST_INSTALL_PACKAGES</c>
+    /// rather than the privileged <c>INSTALL_PACKAGES</c> never gets a decision back from a commit: what
+    /// the platform sends first is <c>STATUS_PENDING_USER_ACTION</c>, carrying — in
+    /// <c>Intent.EXTRA_INTENT</c> — the activity that draws the confirmation. Android does not draw it on
+    /// its own. Something has to receive that broadcast and start it, and for the life of this feature
+    /// nothing did: the intent named a bare action string with no receiver behind it, so the session was
+    /// written, committed, and left staged forever while the user watched nothing happen. See
+    /// <see cref="InstallSessionReceiver"/>, which is that something, and note that the intent naming it
+    /// must be explicit — a mutable pending intent wrapping an implicit one is refused outright from
+    /// API 34, which is where this build's target sits.
+    /// </para>
     /// </remarks>
     public void ApplyAndRestart(AvailableUpdate update)
     {
@@ -318,7 +331,19 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
             return;
         }
 
-        var installer = packages.PackageInstaller;
+        HandToTheInstaller(packages.PackageInstaller, path);
+    }
+
+    /// <summary>Writes the fetched APK into a session and commits it.</summary>
+    /// <remarks>
+    /// The session is abandoned on any failure, which is not tidiness. One that is created and neither
+    /// committed nor abandoned stays staged, holding the space its <c>SetSize</c> reserved, and Android
+    /// caps how many an application may have open at once — so a fault that repeats, which is exactly
+    /// what a broken updater is, ends up unable to create a session at all. That would be a second and
+    /// wholly unrelated symptom for the same cause, and the more confusing of the two.
+    /// </remarks>
+    private void HandToTheInstaller(PackageInstaller installer, string path)
+    {
         var parameters = new PackageInstaller.SessionParams(PackageInstallMode.FullInstall);
         var length = new FileInfo(path).Length;
 
@@ -326,8 +351,10 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
 
         var id = installer.CreateSession(parameters);
 
-        using (var session = installer.OpenSession(id))
+        try
         {
+            using var session = installer.OpenSession(id);
+
             using (var destination = session.OpenWrite(InstallSession, 0, length))
             using (var source = File.OpenRead(path))
             {
@@ -339,20 +366,41 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
                 session.Fsync(destination);
             }
 
-            // A pending intent is how the platform reports what the user decided, and one is required
-            // whether or not anything listens. Nothing here does: the two outcomes are this process being
-            // replaced and this process carrying on, and both are already visible without being told.
-            // Mutable is required from API 31 — the installer fills the result in — and does not exist
+            // Mutable is required from API 31 — the installer fills the status in — and does not exist
             // below it, where every pending intent is mutable and naming the flag will not compile
             // against the older platform. minSdk here is 28, so both cases are real.
             var flags = OperatingSystem.IsAndroidVersionAtLeast(31)
                 ? PendingIntentFlags.Mutable | PendingIntentFlags.UpdateCurrent
                 : PendingIntentFlags.UpdateCurrent;
 
-            var callback = PendingIntent.GetBroadcast(context, 0, new Intent(InstallSession), flags);
+            // Named at the receiver class rather than carrying an action string, and both halves of that
+            // matter. An explicit intent is the only kind a mutable pending intent may wrap from API 34,
+            // so the implicit one that was here throws before commit is even reached on any current
+            // phone; and a broadcast with a receiver behind it is what gets the confirmation drawn at
+            // all. See the remarks above.
+            var callback = PendingIntent.GetBroadcast(
+                context,
+                0,
+                new Intent(context, typeof(InstallSessionReceiver)),
+                flags);
 
             session.Commit(callback!.IntentSender!);
         }
+        catch
+        {
+            try
+            {
+                installer.AbandonSession(id);
+            }
+            catch (Java.Lang.Throwable)
+            {
+                // The session could not be abandoned either. Whatever went wrong first is the useful
+                // half of that, so it is the one allowed to propagate — a cleanup failure thrown from
+                // here would replace the cause with a consequence.
+            }
+
+            throw;
+        }
     }
 
     /// <summary>Which release this build's channel reads.</summary>
@@ -432,6 +480,93 @@ internal sealed class AndroidUpdateChannel : IUpdateChannel
     }
 }
 
+/// <summary>
+/// Starts the confirmation Android hands back when an install session is committed.
+/// </summary>
+/// <remarks>
+/// <para>
+/// <b>This is the whole of "the platform draws the confirmation".</b> It does, but only once asked: a
+/// commit by an application with <c>REQUEST_INSTALL_PACKAGES</c> answers
+/// <c>STATUS_PENDING_USER_ACTION</c> and puts the activity that draws the dialogue in
+/// <c>Intent.EXTRA_INTENT</c>, for the application to start. Nothing else in the system will start it.
+/// Without a receiver the commit still succeeds, the session still stages, and the update simply never
+/// arrives — which is precisely how this shipped.
+/// </para>
+/// <para>
+/// Every other status is ignored on purpose, and that part of the original reasoning survives: the two
+/// outcomes worth knowing are this process being replaced and this process carrying on, and both are
+/// already visible without being told. A failure the user caused by declining is not an error, and a
+/// failure Android caused says so in its own dialogue.
+/// </para>
+/// <para>
+/// Not exported. The only sender is the pending intent this application handed to its own commit, which
+/// the system delivers under this application's identity — so nothing outside needs to reach it, and an
+/// exported receiver that starts an activity out of an extra is a component anybody could use to launch
+/// an arbitrary screen with this application's package on it.
+/// </para>
+/// </remarks>
+[BroadcastReceiver(Enabled = true, Exported = false)]
+internal sealed class InstallSessionReceiver : BroadcastReceiver
+{
+    /// <inheritdoc />
+    public override void OnReceive(Context? context, Intent? intent)
+    {
+        if (context is null || intent is null)
+        {
+            return;
+        }
+
+        // Defaulted to a failure rather than to the pending value, so a broadcast arriving without a
+        // status is treated as nothing to do instead of as a reason to go looking for an intent.
+        var status = intent.GetIntExtra(
+            PackageInstaller.ExtraStatus,
+            (int)PackageInstallStatus.Failure);
+
+        if (status != (int)PackageInstallStatus.PendingUserAction)
+        {
+            return;
+        }
+
+        if (ConfirmationIn(intent) is not { } confirmation)
+        {
+            return;
+        }
+
+        // The activity if there is one, and the application context otherwise with a task of its own —
+        // the same choice, for the same reason, as SendToTheUnknownSourcesScreen above. A receiver's own
+        // context cannot start an activity without the flag, and the update loop can perfectly well have
+        // committed this while the application was backgrounded.
+        if (PhoneEnvironment.CurrentActivity is { } activity)
+        {
+            activity.StartActivity(confirmation);
+
+            return;
+        }
+
+        confirmation.AddFlags(ActivityFlags.NewTask);
+        context.StartActivity(confirmation);
+    }
+
+    /// <summary>The activity Android wants started, read the way the running platform allows.</summary>
+    /// <remarks>
+    /// The untyped overload is obsolete from API 33 and the typed one does not exist below it, so both
+    /// are here behind the guard the analyser reads. The same shape as the pending-intent flags in
+    /// <see cref="AndroidUpdateChannel"/>, and for the same reason: minSdk is 28, so both branches run
+    /// on phones this ships to.
+    /// </remarks>
+    private static Intent? ConfirmationIn(Intent intent)
+    {
+        if (OperatingSystem.IsAndroidVersionAtLeast(33))
+        {
+            return intent.GetParcelableExtra(
+                Intent.ExtraIntent,
+                Java.Lang.Class.FromType(typeof(Intent))) as Intent;
+        }
+
+        return intent.GetParcelableExtra(Intent.ExtraIntent) as Intent;
+    }
+}
+
 /// <summary>
 /// Picks the update channel this copy of the phone head gets.
 /// </summary>

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

@@ -397,7 +397,22 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
             return;
         }
 
-        await restart(update).ConfigureAwait(true);
+        // ◆ Reported rather than allowed to escape, and the phone is why. Where applying ends the process
+        // a throw here is nearly unreachable; where it does not — see ApplyingEndsTheProcess — the whole
+        // point is that this returns and the application carries on, so an exception has somewhere to go:
+        // out of a command handler, past the dispatcher, and into the process. What that looks like to
+        // somebody holding the phone is pressing INSTALL and having the application close, with nothing
+        // installed and nothing said. Failed and the message is the same answer CheckNowAsync gives, and
+        // it is the difference between a bug that describes itself and one that presents as a crash.
+        try
+        {
+            await restart(update).ConfigureAwait(true);
+        }
+        catch (Exception exception) when (exception is not OutOfMemoryException)
+        {
+            State = UpdateState.Failed;
+            Status = exception.Message;
+        }
     }
 
     [RelayCommand]

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

@@ -29,6 +29,14 @@ internal sealed class FakeUpdateChannel : IUpdateChannel
     /// <summary>When set, downloading throws it.</summary>
     internal Exception? DownloadFailure { get; set; }
 
+    /// <summary>When set, applying throws it — after counting, since the call was still made.</summary>
+    /// <remarks>
+    /// The phone's install can fail on the spot rather than by ending the process: a session the platform
+    /// refuses, or an unknown-sources answer that changed under it. What that must not do is escape a
+    /// command handler, so this is what lets a test hold the view model to reporting it.
+    /// </remarks>
+    internal Exception? RestartFailure { get; set; }
+
     /// <summary>When set, a download waits on it before completing.</summary>
     internal TaskCompletionSource? HoldDownload { get; set; }
 
@@ -97,5 +105,10 @@ internal sealed class FakeUpdateChannel : IUpdateChannel
     {
         Restarts++;
         RestartedWith = update;
+
+        if (RestartFailure is { } failure)
+        {
+            throw failure;
+        }
     }
 }

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

@@ -154,6 +154,31 @@ public sealed class UpdateFlowTests : IDisposable
         channel.RestartedWith!.Version.ShouldBe("1.3.0");
     }
 
+    /// <remarks>
+    /// ◆ The regression for an install that closed the application instead of installing anything. On the
+    /// phone applying is a request that returns, so a failure in it throws on the spot rather than being
+    /// cut short by the process ending — and with nothing catching it, it left a command handler, passed
+    /// the dispatcher and took the process with it. Pressing INSTALL closed the application, said nothing,
+    /// and installed nothing, which reads as anything but a bug in the updater.
+    /// </remarks>
+    [Fact]
+    public async Task AnInstallThatFails_IsReportedRatherThanThrown()
+    {
+        channel.Available = new AvailableUpdate("1.3.0");
+        channel.ApplyingEndsTheProcess = false;
+        channel.RestartFailure = new InvalidOperationException("The installer refused the session.");
+
+        var updates = Build();
+
+        await updates.CheckOnceAsync(Token);
+
+        // Not Should.ThrowAsync: the assertion is that this does not throw at all.
+        await updates.RestartNowCommand.ExecuteAsync(null);
+
+        updates.State.ShouldBe(UpdateState.Failed);
+        updates.Status.ShouldBe("The installer refused the session.");
+    }
+
     [Fact]
     public async Task RestartingWithNothingReady_DoesNothing()
     {