Skip to content

feat(apple): Disable the update checker for MDM and App store - #9167

Merged
jamilbk merged 1 commit into
mainfrom
feat/apple/disable-update-checker
May 16, 2025
Merged

jamilbk merged 1 commit into
mainfrom
feat/apple/disable-update-checker

Conversation

@jamilbk

@jamilbk jamilbk commented May 16, 2025

Copy link
Copy Markdown
Member

For App Store installed macOS clients, it doesn't make sense to run an update checker, because the system is managing the updates, and will notify the user if there's an update available for Firezone (the user has configured the system to manage app updates).

Related: #7664
Related: #4505

@jamilbk
jamilbk requested review from Copilot and thomaseizinger and removed request for Copilot May 16, 2025 07:04
@vercel

vercel Bot commented May 16, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 16, 2025 7:04am

import UserNotifications
import Cocoa

@MainActor

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved @MainActor to the class level

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables disabling the built-in update checker for macOS clients installed via the App Store by wiring a new disableUpdateCheck config flag into the existing update flow and detecting App Store installs.

  • Injects Store into UpdateChecker and listens for config changes to start/stop the timer.
  • Adds a new disableUpdateCheck property in Configuration with its default and decoding logic.
  • Removes the App Store–specific branch in downloadURL.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
UpdateNotification.swift Injected Store, combined config and App Store detection to toggle update polling; added stopCheckingForUpdates.
MenuBar.swift Passed store into UpdateChecker initializer.
Configuration.swift Added disableUpdateCheck default, key, and parsing in config.
Comments suppressed due to low confidence (2)

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift:146

  • The App Store branch was removed, so downloadURL() now always points to the website; consider restoring the if BundleHelper.isAppStore() case to direct users from App Store installs to the App Store page.
static func downloadURL() -> URL {

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift:60

  • There are no tests verifying that the update checker starts and stops correctly when disableUpdateCheck or App Store mode changes; consider adding unit tests for handleConfigurationChange behavior.
private func handleConfigurationChange() {


public static let defaultAccountSlug = ""
public static let defaultConnectOnStart = true
public static let defaultDisableUpdateCheck = false

Copilot AI May 16, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new disableUpdateCheck config option should be documented in the public README or API docs so users know how to opt out of the update checker.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that the comments generated by Copilot are getting progressivly worse.

@jamilbk

jamilbk commented May 16, 2025

Copy link
Copy Markdown
Member Author

Tested with disabling and enabling the update check.

@jamilbk
jamilbk added this pull request to the merge queue May 16, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 16, 2025
@jamilbk
jamilbk added this pull request to the merge queue May 16, 2025
Merged via the queue into main with commit 9951e82 May 16, 2025
@jamilbk
jamilbk deleted the feat/apple/disable-update-checker branch May 16, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants