feat: add auto updater #117

deansheather · 2025-06-04T15:51:55Z

Closes: #122

Adds an auto updater utility using NetSparkle.

Most of the UI matches the way it's done in NetSparkle for the built-in UIs (I referenced the Avalonia and WPF ones mostly), including most of the copy and implementation (incl. quirks). I did this in case we ever want to contribute this upstream. The only major difference is how I handle the release notes stuff.

For release notes, I use the <description> tag to generate HTML for the web view. There's a hardcoded """template""" in the source code and a copy of sindresorhus/github-markdown-css in the repo that gets included as an asset in the binary. This matches what we do on macOS but diverges from the way NetSparkle does release notes rendering using <sparkle:releaseNotesUrl> (or something like that).

Adds a button to the right click context menu on the tray icon for checking for updates
Adds custom UIs for all parts of the updater
Implements "action" notifications with a click handler into UserNotifier
Adds Serilog output to the debug output window in VS in DEBUG builds

TODO:

App cast generation script
Release pipeline modifications
Generate secret key and add to repo secrets
Publish initial 0.5.0 version manually
Set final security and update source related settings in NetSparkle

Follow up:

Integrate into the settings UI, add controls for whether it should automatically check for updates etc.

App/Services/UpdateController.cs

spikecurtis · 2025-06-09T09:26:10Z

App/Views/TrayWindow.xaml.cs

        TrayIcon.OpenCommand = Tray_OpenCommand;
+        TrayIcon.CheckForUpdatesCommand = Tray_CheckForUpdatesCommand;


Ideally we wouldn't include this if it's administratively disabled.

Clicking it will show a popup explaining that it's disabled by an administrator, which I think is good enough for now. I'll file a ticket to revisit this

spikecurtis · 2025-06-09T10:15:13Z

App/Services/UpdateController.cs

+
+    bool IUIFactory.HideReleaseNotes { get; set; }
+    bool IUIFactory.HideSkipButton { get; set; }
+    bool IUIFactory.HideRemindMeLaterButton { get; set; }


why are these bools IUIFactor instead of just normal public bool?

Is there some reason we only want them accessed via the interface?

I just like the pattern, discovered it while looking at some of the NetSparkle code

App/Views/UpdaterCheckingForUpdatesWindow.xaml

spikecurtis · 2025-06-09T10:44:32Z

scripts/Update-AppCast.ps1

+#          -outputAppCastSignaturePath <path>
+param (
+    [Parameter(Mandatory = $true)]
+    [ValidatePattern("^v\d+\.\d+\.\d+$")]


if we are going to enforce that the tag follows this format, then why not just compute it from the version? Seems like a gnarly bug if they didn't match versions.

spikecurtis · 2025-06-09T11:08:20Z

App/Services/UserNotifier.cs

+            return;
+        }
+
+        var dispatcherQueue = ((App)Application.Current).TrayWindow?.DispatcherQueue;


I don't think we want it to be the UserNotifier's business to figure out which thread the activation needs to run on. The Sparkle stuff needs to be on the UI, but other actions might not need to be.

Getting onto the correct thread should be the job of the callback itself.

I think that most (if not all) callbacks will want to run on the UI thread so they can e.g. spawn a window, update fields on the view model.

Either way we have to do the workaround to use the TrayWindow's DispatcherQueue in this case, so I think this avoids us having to have this workaround in multiple places in the future.

I still think it shouldn't be in UserNotifier but if you're going to insist, then the DispatcherQueue needs to be passed in at startup in some reasonably portable way. Doing it like this will make this impossible to unit test.

spikecurtis · 2025-06-09T11:10:08Z

App/Services/UserNotifier.cs

+            .AddText(message)
+            .AddArgument(CoderNotificationId, id)
+            .BuildNotification();
+        ActionHandlers[id] = action;


Doesn't this end up leaking handlers for every notification we send that doesn't get clicked?

I think we need to define some notification types, and allow a single handler per type. It could be dynamically registered at runtime, or just hard-coded handlers per type in App.xaml.cs (and then UserNotifier doesn't even need to worry about activations, just sending notifications).

That's fair, but I'd prefer this to be handled at the UserNotifier level then in App.xaml.cs. Ideally the IUserNotifier interface has a method called HandleActivation that takes a custom data type for ease of testing.

And I guess each handler would either be in a dictionary or registered once at startup.

ibetitsmike · 2025-06-11T10:04:26Z

.github/workflows/release.yaml

+        $newAppCastSignaturePath = $newAppCastPath + ".signature"
+        & ./scripts/Update-AppCast.ps1 `
+          -tag "${{ github.ref_name }}" `
+          -version "${{ steps.version.outputs.VERSION }}" `


considering the update will fail if we don't "bump" the version number (ie. previous is lower than new) - do we want to check this here within the script?

ibetitsmike · 2025-06-11T10:08:17Z

App/App.xaml.cs

+                //
+                // HACK: This isn't super robust, but the security risk is
+                //       minor anyway. Malicious apps running as the user could
+                //       likely override this setting by altering the memory of


One way to remove this risk is by always treating registry as a source of truth instead of relying on the in-memory value to block this.

ibetitsmike · 2025-06-11T10:09:35Z

App/Controls/TrayIcon.xaml

@@ -32,7 +32,7 @@
                        <XamlUICommand
                            Label="Show Window"
                            Description="Show Window"
-                            Command="{x:Bind OpenCommand}">
+                            Command="{x:Bind OpenCommand, Mode=OneWay}">


The command shouldn't need a direction on the bind IIRC

ibetitsmike · 2025-06-11T10:10:07Z

App/Controls/TrayIcon.xaml

+                        <XamlUICommand
+                            Label="Check for Updates"
+                            Description="Check for Updates"
+                            Command="{x:Bind CheckForUpdatesCommand, Mode=OneWay}">


Suggested change

Command="{x:Bind CheckForUpdatesCommand, Mode=OneWay}">

Command="{x:Bind CheckForUpdatesCommand}">

ibetitsmike · 2025-06-11T10:10:40Z

App/Controls/TrayIcon.xaml

                <MenuFlyoutItem>
                    <MenuFlyoutItem.Command>
                        <XamlUICommand
                            Label="Exit"
                            Description="Exit"
-                            Command="{x:Bind ExitCommand}">
+                            Command="{x:Bind ExitCommand, Mode=OneWay}">


Suggested change

Command="{x:Bind ExitCommand, Mode=OneWay}">

Command="{x:Bind ExitCommand}">

ibetitsmike · 2025-06-11T10:40:44Z

App/Views/UpdaterDownloadProgressWindow.xaml.cs

+
+        InitializeComponent();
+        TitleBarIcon.SetTitlebarIcon(this);
+        SystemBackdrop = new DesktopAcrylicBackdrop();


Suggested change

SystemBackdrop = new DesktopAcrylicBackdrop();

ibetitsmike · 2025-06-11T10:43:28Z

App/Views/UpdaterDownloadProgressWindow.xaml.cs

+
+    public UpdaterDownloadProgressViewModel ViewModel;
+
+    private bool _didCallDownloadProcessCompletedHandler;


Suggested change

private bool _didCallDownloadProcessCompletedHandler;

private bool _downloadProcessCompletedInvoked;

just a nitpick
having an auxilary verb in the property name sounds weird imho

ibetitsmike · 2025-06-11T10:43:54Z

App/Views/UpdaterUpdateAvailableWindow.xaml

+    MinWidth="600" MinHeight="500">
+
+    <Window.SystemBackdrop>
+        <DesktopAcrylicBackdrop />


Suggested change

<DesktopAcrylicBackdrop />

<MicaBackdrop />

ibetitsmike · 2025-06-11T10:44:02Z

App/Views/UpdaterUpdateAvailableWindow.xaml.cs

+
+        InitializeComponent();
+        TitleBarIcon.SetTitlebarIcon(this);
+        SystemBackdrop = new DesktopAcrylicBackdrop();


Suggested change

SystemBackdrop = new DesktopAcrylicBackdrop();

ibetitsmike · 2025-06-11T10:46:40Z

App/Views/UpdaterUpdateAvailableWindow.xaml.cs

+
+    void IUpdateAvailable.Close()
+    {
+        UserRespondedToUpdateCheck(UpdateAvailableResult.None); // the Avalonia UI does this "just in case"


why do we reference the Avalonia UI in the comment?

deansheather added 3 commits June 4, 2025 17:58

feat: add auto updater

2f8f3a1

small changes

c92ba0b

Merge branch 'main' into dean/updater

5072e24

deansheather requested review from spikecurtis and ibetitsmike June 4, 2025 15:51

deansheather added 6 commits June 5, 2025 21:04

chore: appcast generation

26706bb

improvements

199a436

improvements

e0867e4

use real URL

6ea247d

cache control

f3e5a67

actions fix

7d35551

spikecurtis reviewed Jun 9, 2025

View reviewed changes

ibetitsmike requested changes Jun 11, 2025

View reviewed changes

		TrayIcon.OpenCommand = Tray_OpenCommand;
		TrayIcon.CheckForUpdatesCommand = Tray_CheckForUpdatesCommand;

	Command="{x:Bind CheckForUpdatesCommand, Mode=OneWay}">
	Command="{x:Bind CheckForUpdatesCommand}">

	Command="{x:Bind ExitCommand, Mode=OneWay}">
	Command="{x:Bind ExitCommand}">


		public UpdaterDownloadProgressViewModel ViewModel;

		private bool _didCallDownloadProcessCompletedHandler;

	private bool _didCallDownloadProcessCompletedHandler;
	private bool _downloadProcessCompletedInvoked;

feat: add auto updater #117

Are you sure you want to change the base?

feat: add auto updater #117

Uh oh!

Conversation

deansheather commented Jun 4, 2025 • edited by ibetitsmike Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

Follow up:

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deansheather commented Jun 4, 2025 •

edited by ibetitsmike

Loading