Skip to content

Add a default-on toggle for automatic update checks#452

Closed
FuJacob wants to merge 1 commit into
mainfrom
feat/auto-update-toggle
Closed

Add a default-on toggle for automatic update checks#452
FuJacob wants to merge 1 commit into
mainfrom
feat/auto-update-toggle

Conversation

@FuJacob

@FuJacob FuJacob commented May 31, 2026

Copy link
Copy Markdown
Owner

Summary

Cotabby checks for updates on launch and once a day through Sparkle (SUEnableAutomaticChecks is already true in CotabbyInfo.plist), but users had no way to turn that off. This adds an "Automatically Check for Updates" toggle to the About settings pane. It is on by default, matching the existing behavior, and lets a user opt out of all background update activity while still being able to press "Check for Updates" manually.

AppUpdateManager now exposes Sparkle's own automaticallyChecksForUpdates as the single source of truth (a computed proxy, not a second persisted copy), so the toggle and the updater can never drift apart. The once-per-launch background check is now gated on that flag, and Sparkle's daily scheduled check already honors it, so turning the toggle off genuinely stops background checks.

The toggle lives in the About pane because that pane already owns AppUpdateManager and the "Check for Updates" button, so update controls stay together.

Validation

xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build
# ** BUILD SUCCEEDED **

xcodebuild test -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO
# ** TEST SUCCEEDED **  566 tests, 0 failures (4 skipped)

swiftlint lint --strict --quiet Cotabby/Services/Utilities/AppUpdateManager.swift Cotabby/UI/Settings/Panes/AboutPaneView.swift
# exit 0

Manual check: open Settings > About. The "Updates" section shows the toggle on by default. Turning it off persists across relaunch (Sparkle writes SUEnableAutomaticChecks to user defaults) and suppresses the launch background check; "Check for Updates" still works.

Linked issues

None.

Risk / rollout notes

  • Default is unchanged for existing users: automatic checks stay on. The new control only adds the ability to opt out.
  • The preference is Sparkle's own (SUEnableAutomaticChecks), read and written through AppUpdateManager. No new settings key or migration.
  • AppUpdateManager now conforms to ObservableObject (added import Combine) so the About pane can bind a toggle to it. It has no @Published state; the setter calls objectWillChange.send() so a bound toggle re-renders.
  • Scope is auto-check only. Silent auto-download and install (SUAutomaticallyUpdate) stays off, so an available update still prompts before installing, which is the existing behavior.
  • No new files, so project.yml and the generated project are unchanged.

Greptile Summary

This PR adds an "Automatically Check for Updates" toggle to the About settings pane, backed by a computed proxy on AppUpdateManager that reads and writes Sparkle's own SUEnableAutomaticChecks preference directly — avoiding a duplicate persisted copy that could drift out of sync.

  • AppUpdateManager gains ObservableObject conformance and an automaticallyChecksForUpdates computed property whose setter calls objectWillChange.send() before writing to Sparkle, so a bound SwiftUI toggle re-renders correctly.
  • The once-per-launch checkForUpdatesInBackground() call in start() is now gated on the flag, making the toggle a complete opt-out of all background update activity.
  • AboutPaneView changes appUpdateManager to @ObservedObject and adds the toggle via a lightweight Binding helper.

Confidence Score: 4/5

Safe to merge; the toggle correctly proxies Sparkle's own preference with no duplicated state and the launch check is properly gated.

The design is solid — single source of truth, correct objectWillChange ordering, and @ObservedObject usage are all right. The two suggestions are about a missing isStarted guard in the new setter (inconsistent with the existing checkForUpdates() pattern) and a missing .disabled modifier on the toggle, both of which only matter in misconfigured development builds and have no production impact.

No files require special attention for production. The isStarted guard omission in AppUpdateManager and the always-enabled toggle in AboutPaneView are worth a second look if the misconfigured-Sparkle path ever gains more test coverage.

Important Files Changed

Filename Overview
Cotabby/Services/Utilities/AppUpdateManager.swift Adds ObservableObject conformance and an automaticallyChecksForUpdates computed proxy; correctly gates the launch background check on the flag. The setter lacks an isStarted guard (unlike checkForUpdates()), which is harmless in production but inconsistent.
Cotabby/UI/Settings/Panes/AboutPaneView.swift Adds Updates section with an automatic-check toggle bound through a custom Binding. @ObservedObject is the correct attribute here. Toggle has no disabled modifier for the misconfigured-Sparkle case, but this is development-only.

Sequence Diagram

sequenceDiagram
    participant User
    participant AboutPaneView
    participant AppUpdateManager
    participant SPUUpdater
    participant UserDefaults

    User->>AboutPaneView: toggles "Automatically Check for Updates"
    AboutPaneView->>AppUpdateManager: "automaticallyChecksForUpdates = newValue"
    AppUpdateManager->>SPUUpdater: read automaticallyChecksForUpdates (guard check)
    SPUUpdater-->>AppUpdateManager: current value
    AppUpdateManager->>AppUpdateManager: objectWillChange.send()
    AppUpdateManager->>SPUUpdater: "automaticallyChecksForUpdates = newValue"
    SPUUpdater->>UserDefaults: persist SUEnableAutomaticChecks
    AppUpdateManager->>AppUpdateManager: log change
    AboutPaneView-->>User: toggle re-renders (SwiftUI diff)

    Note over AppUpdateManager,SPUUpdater: On next app launch
    AppUpdateManager->>AppUpdateManager: start() called
    AppUpdateManager->>SPUUpdater: startUpdater()
    AppUpdateManager->>SPUUpdater: read automaticallyChecksForUpdates
    SPUUpdater->>UserDefaults: read SUEnableAutomaticChecks
    UserDefaults-->>SPUUpdater: stored value
    alt "automaticallyChecksForUpdates == true"
        AppUpdateManager->>SPUUpdater: checkForUpdatesInBackground()
    else "automaticallyChecksForUpdates == false"
        AppUpdateManager->>AppUpdateManager: log skip
    end
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "Add a default-on toggle to control autom..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Cotabby checks for updates on launch and once a day through Sparkle, with
SUEnableAutomaticChecks already defaulted on in Info.plist, but there was no
way for a user to turn that off. This adds an "Automatically Check for Updates"
toggle to the About settings pane, on by default.

AppUpdateManager now exposes Sparkle's automaticallyChecksForUpdates as the
single source of truth (rather than a second persisted copy) and gates the
launch background check on it, so turning the toggle off fully stops background
update activity while the manual "Check for Updates" button still works.
Comment on lines +91 to +101
var automaticallyChecksForUpdates: Bool {
get { updaterController.updater.automaticallyChecksForUpdates }
set {
guard newValue != updaterController.updater.automaticallyChecksForUpdates else {
return
}
objectWillChange.send()
updaterController.updater.automaticallyChecksForUpdates = newValue
log("Automatic update checks \(newValue ? "enabled" : "disabled") by user preference.")
}
}

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.

P2 checkForUpdates() guards on isStarted and logs a diagnostic when the updater hasn't been started, but automaticallyChecksForUpdates's setter has no equivalent guard. In the (development-only) path where hasUsableConfiguration returns false, Sparkle never starts but the setter is still reachable from the UI — it writes to UserDefaults silently with no indication that the change has no effect this session. Adding the guard keeps the surface area consistent and makes misconfiguration easier to diagnose.

Suggested change
var automaticallyChecksForUpdates: Bool {
get { updaterController.updater.automaticallyChecksForUpdates }
set {
guard newValue != updaterController.updater.automaticallyChecksForUpdates else {
return
}
objectWillChange.send()
updaterController.updater.automaticallyChecksForUpdates = newValue
log("Automatic update checks \(newValue ? "enabled" : "disabled") by user preference.")
}
}
var automaticallyChecksForUpdates: Bool {
get { updaterController.updater.automaticallyChecksForUpdates }
set {
guard isStarted else {
log("Ignoring automatic-checks preference change because the updater has not started.")
return
}
guard newValue != updaterController.updater.automaticallyChecksForUpdates else {
return
}
objectWillChange.send()
updaterController.updater.automaticallyChecksForUpdates = newValue
log("Automatic update checks \(newValue ? "enabled" : "disabled") by user preference.")
}
}

Fix in Codex Fix in Claude Code

Comment on lines +57 to +66
@ViewBuilder
private var updatesRow: some View {
Toggle(isOn: automaticUpdateChecksBinding) {
SettingsRowLabel(
title: "Automatically Check for Updates",
description: "Let Cotabby check for new versions in the background on launch and once " +
"a day. Turn this off to only update when you press Check for Updates."
)
}
}

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.

P2 The toggle is always interactive, even in configurations where Sparkle never starts (i.e. development builds where hasUsableConfiguration is false). Exposing isStarted through AppUpdateManager (or a dedicated isAvailable flag) and wiring it to .disabled(!appUpdateManager.isStarted) on the toggle would prevent a confusing state where the toggle moves but has no observable effect during that session.

Suggested change
@ViewBuilder
private var updatesRow: some View {
Toggle(isOn: automaticUpdateChecksBinding) {
SettingsRowLabel(
title: "Automatically Check for Updates",
description: "Let Cotabby check for new versions in the background on launch and once " +
"a day. Turn this off to only update when you press Check for Updates."
)
}
}
@ViewBuilder
private var updatesRow: some View {
Toggle(isOn: automaticUpdateChecksBinding) {
SettingsRowLabel(
title: "Automatically Check for Updates",
description: "Let Cotabby check for new versions in the background on launch and once " +
"a day. Turn this off to only update when you press Check for Updates."
)
}
.disabled(!appUpdateManager.isStarted)
}

Fix in Codex Fix in Claude Code

@FuJacob FuJacob closed this May 31, 2026
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.

1 participant