Integrate app updater with UI by gzdunek · Pull Request #57491 · gravitational/teleport

gzdunek · 2025-08-04T09:58:25Z

This PR adds a dialog to check for updates and integrates it with the updater service (the widget will be added in the next PR).

This first commit is most important one. It implements updater methods that will be used by the UI. Unfortunately, I found some bugs in electron-updater, so I had to add extra logic to resolve them. I've left comments in AppUpdater.ts to explain what wasn't working and how those issues were addressed. If anything is unclear, please let me know.

While testing, I also realized that the UI in the multi-cluster scenario may be too complex when we show errors/warning about unreachable clusters (although it seemed fine in Storybook to me). I'll address this separately by reducing alerts and making inline errors more prominent.

How to test it (on macOS)

I prepared two dev builds with different versions (19.0.0-dev.connect.updates.1 and 19.0.0-dev.connect.updates.2) to test the updates.

Globally set the base URL env var: launchctl setenv TELEPORT_CDN_BASE_URL https://cdn.cloud.gravitational.io.
Set your cluster client tools version: tctl autoupdate client-tools target 19.0.0-dev.connect.updates.2.
Download 19.0.0-dev.connect.updates.1 build from https://cdn.cloud.gravitational.io/Teleport%20Connect-19.0.0-dev.connect.updates.1.dmg.
Move the app to Applications and start it -> App menu -> Check for updates.

…agingCluster` and `quitAndInstall`

…clicking it

…s it, trim the checksum value before validating it

ravicious

I managed to look through the new commits and the old ones until "Expose new APIs through Electron IPC", I'll try to finish the review by the end of this day.

ravicious

I'll do some light QA tomorrow as so far I've mostly just read the code.

ravicious · 2025-08-06T16:03:09Z

+    // after quitting the app.
+    // We may revisit this behavior in the future. For example, if we introduce
+    // a UI notification indicating that there's an update to be installed.
+    await this.checkForUpdates({ noAutoDownload: true });


Is there any way we can create tests for this? It feels quite brittle, as in we don't directly discard any updates here but rather we depend on this.checkForUpdates doing this somewhere underneath when called with noAutoDownload. It'd be at least nice to cover that it in fact does this when noAutoDownload is used.

Is it possible to mock/fake autoUpdater? The logic in this file is already rather complex and we just don't have any tests for it, meaning that even this edge case here would need to be covered by the test plan.

I thought it would a pain to mock autoUpdater, but it wasn't that bad!
I added tests for updates auto download and discarding them when the latest check returns a new update or no update, as this part feels the most brittle.

ravicious · 2025-08-07T07:13:45Z

Improvement for the future: It'd be nice if after downloading the update there was a button to abort the update.

I was considering it, but I was afraid of making auto updates too complex. Also, I don't think I know any app/system that would allow it 😅
Do you see a specific scenario where this would be useful?

My original comment might have been a result of a developer brain. I was testing the feature and it downloaded an update that I didn't really need, but I couldn't skip it once it's been downloaded.

But this got me thinking: the newest version of my editor is currently broken. The updater fortunately offers a "Skip this update" feature (I think it's supported OOTB in one of the popular macOS updaters). What if we release a bugged version of Connect? The cluster admin would probably need to turn off auto-updates, but why would they need to do that if there's a problem only with Connect and not tsh? Or say the bug affects only a specific platform. As the end user, can I skip it somehow? Is setting an env var the only available workaround?

For the CLI tools, the official workaround is the env var:

An environment variable TELEPORT_TOOLS_VERSION can be used as an emergency workaround for a known issue, pinning to a specific version in CI/CD, for debugging, or for manual updates.

The same one will be available in Connect. I know it's a bit inconvenient for desktop apps, but I also feel it's acceptable for the updater v1.
I can open an issue to add a UI option in the future that will allow users to skip an update or maybe even revert to a previous version.

…Logout`

ravicious · 2025-08-07T15:31:59Z

My original comment might have been a result of a developer brain. I was testing the feature and it downloaded an update that I didn't really need, but I couldn't skip it once it's been downloaded.

But this got me thinking: the newest version of my editor is currently broken. The updater fortunately offers a "Skip this update" feature (I think it's supported OOTB in one of the popular macOS updaters). What if we release a bugged version of Connect? The cluster admin would probably need to turn off auto-updates, but why would they need to do that if there's a problem only with Connect and not tsh? Or say the bug affects only a specific platform. As the end user, can I skip it somehow? Is setting an env var the only available workaround?

backport-bot-workflows · 2025-08-08T08:13:35Z

@gzdunek See the table below for backport results.

Branch	Result
branch/v16	Failed
branch/v17	Failed
branch/v18	Failed

* Implement `checkForUpdates`, `download`, `cancelDownload`, `changeManagingCluster` and `quitAndInstall` * Expose new APIs through Electron IPC * Add app updater context to store updater state * Add `AppUpdates` dialog * Switch "Download" button to "Starting Download..." immediately after clicking it * Clear managing cluster on logout * Return correct error in `GetClusterVersions` * Do not prepend `https://` to file URL, DefaultBaseURL already contains it, trim the checksum value before validating it * Only try to download an update if it's available * Remove separate `will-quit` handler * Clarify comment * Invert condition in `preventInstallingOutdatedUpdates` * Add comments * Move `closeAllConnections()` out of try catch * Add specialized functions to serialize/deserialize errors * Add tests * Leave TODO * Do not wait for `maybeRemoveAppUpdatesManagingCluster` in `useClusterLogout` * Hide app updater for .tar.gz which is not supported * Add missing mock (cherry picked from commit 13aa750)

* Integrate app updater with UI (#57491) * Implement `checkForUpdates`, `download`, `cancelDownload`, `changeManagingCluster` and `quitAndInstall` * Expose new APIs through Electron IPC * Add app updater context to store updater state * Add `AppUpdates` dialog * Switch "Download" button to "Starting Download..." immediately after clicking it * Clear managing cluster on logout * Return correct error in `GetClusterVersions` * Do not prepend `https://` to file URL, DefaultBaseURL already contains it, trim the checksum value before validating it * Only try to download an update if it's available * Remove separate `will-quit` handler * Clarify comment * Invert condition in `preventInstallingOutdatedUpdates` * Add comments * Move `closeAllConnections()` out of try catch * Add specialized functions to serialize/deserialize errors * Add tests * Leave TODO * Do not wait for `maybeRemoveAppUpdatesManagingCluster` in `useClusterLogout` * Hide app updater for .tar.gz which is not supported * Add missing mock (cherry picked from commit 13aa750) * Show update widget in login dialog (#57695) * Add app updates widget to login dialog * Adjust compatibility warning to app updates * Simplify widget error state, do not show full error message * Make the widget take up 100% width both in login modal an in story * Fix LocalWithPasswordless story * Use `alignItems="stretch"` * Use `SpaceProps` instead of redefining `mx` manually * Infer type * Display buttons in the bottom of Alert * Avoid moving content when scrollbar appears * Destructure `Promise.all` result * Add comment --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> (cherry picked from commit c59a4e6) * Simplify auto updates multi-cluster view (#57709) * Simplify auto updates multi-cluster view * Improve labels * Fix tests (cherry picked from commit b574045) * Connect: Force focus after a successful SSO login * Move login functions from ClustersService to useClusterLogin * Remove no longer used GetResourcesParams * Remove clusters/types.ts * Force focus on Connect after successful SSO login * Update SSO prompt after login but before sync * Add test for SSO prompts * Use `Promise.withResolvers` Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com> * Update wait-for-sync prompt message --------- Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com> * Hardcode the lowest supported Connect versions that support managed updates (#58348) * Provide the lowest supported app versions for managed updates * Deserialize errors in the UI code, not in preload Otherwise, the error name is lost. * Bump version in tests * Lint * Show exact version in error message * Do not hide UnsupportedVersionError * Fix margin * Make alert take 100% width (cherry picked from commit d7e1352) --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

gzdunek added 5 commits August 4, 2025 11:52

Implement checkForUpdates, download, cancelDownload, `changeMan…

14881cb

…agingCluster` and `quitAndInstall`

Expose new APIs through Electron IPC

ff9d146

Add app updater context to store updater state

ea28b0c

Add AppUpdates dialog

c003811

Switch "Download" button to "Starting Download..." immediately after …

ff491da

…clicking it

gzdunek added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 backport/branch/v18 labels Aug 4, 2025

github-actions bot requested review from bl-nero and ravicious August 4, 2025 09:58

github-actions bot added size/md ui labels Aug 4, 2025

gzdunek commented Aug 4, 2025

View reviewed changes

Comment thread web/packages/teleterm/src/ui/AppUpdater/AppUpdaterContext.tsx

gzdunek requested review from ryanclark and removed request for bl-nero August 4, 2025 10:00

gzdunek added 3 commits August 4, 2025 12:12

Clear managing cluster on logout

e37acbf

Return correct error in GetClusterVersions

8f1d85a

Do not prepend https:// to file URL, DefaultBaseURL already contain…

5ca1de6

…s it, trim the checksum value before validating it

gzdunek force-pushed the gzdunek/integrate-app-updater-with-ui branch from 0ef7892 to 5ca1de6 Compare August 4, 2025 10:16

ravicious reviewed Aug 4, 2025

View reviewed changes

ravicious self-requested a review August 4, 2025 16:28

gzdunek added 3 commits August 5, 2025 12:03

Only try to download an update if it's available

7a0cabf

Remove separate will-quit handler

1aa750a

Clarify comment

36b137c

ryanclark approved these changes Aug 5, 2025

View reviewed changes

Comment thread web/packages/teleterm/src/services/appUpdater/appUpdater.ts Outdated

ravicious reviewed Aug 6, 2025

View reviewed changes

ravicious self-requested a review August 6, 2025 08:51

gzdunek added 2 commits August 6, 2025 11:32

Invert condition in preventInstallingOutdatedUpdates

437b7dd

Add comments

b25644e

gzdunek added 2 commits August 6, 2025 11:32

Move closeAllConnections() out of try catch

274b839

Add specialized functions to serialize/deserialize errors

511a37d

ravicious reviewed Aug 6, 2025

View reviewed changes

ravicious self-requested a review August 6, 2025 16:18

ravicious reviewed Aug 7, 2025

View reviewed changes

gzdunek added 3 commits August 7, 2025 15:58

Add tests

3463f1b

Leave TODO

158a47c

Do not wait for maybeRemoveAppUpdatesManagingCluster in `useCluster…

b76744e

…Logout`

gzdunek requested a review from ravicious August 7, 2025 14:15

Hide app updater for .tar.gz which is not supported

cc21c5f

ravicious approved these changes Aug 7, 2025

View reviewed changes

Add missing mock

b41eda3

gzdunek enabled auto-merge August 7, 2025 16:21

gzdunek added this pull request to the merge queue Aug 8, 2025

Merged via the queue into master with commit 13aa750 Aug 8, 2025
44 of 45 checks passed

gzdunek deleted the gzdunek/integrate-app-updater-with-ui branch August 8, 2025 08:11

This was referenced Aug 22, 2025

[v18] Show app updater in UI #58260

Merged

[v17] Show app updater in UI #58261

Merged

Conversation

gzdunek commented Aug 4, 2025

How to test it (on macOS)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ravicious left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ravicious left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ravicious Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

gzdunek Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ravicious Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

gzdunek Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

ravicious Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

gzdunek Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ravicious Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

backport-bot-workflows bot commented Aug 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants