Repository navigation
fix: show sidebar update banner from background checks #1543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,10 @@ enum UpdateFeedResolver { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extension UpdateDriver: SPUUpdaterDelegate { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updaterShouldPromptForPermissionToCheck(forUpdates _: SPUUpdater) -> Bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func feedURLString(for updater: SPUUpdater) -> String? { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #if DEBUG | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let env = ProcessInfo.processInfo.environment | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,6 +39,7 @@ extension UpdateDriver: SPUUpdaterDelegate { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Called when an update is scheduled to install silently, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// which occurs when automatic download is enabled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updater(_ updater: SPUUpdater, willInstallUpdateOnQuit item: SUAppcastItem, immediateInstallationBlock immediateInstallHandler: @escaping () -> Void) -> Bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| viewModel.clearDetectedUpdate() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| viewModel.state = .installing(.init( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isAutoUpdate: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| retryTerminatingApplication: immediateInstallHandler, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -56,6 +61,7 @@ extension UpdateDriver: SPUUpdaterDelegate { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updater(_ updater: SPUUpdater, didFindValidUpdate item: SUAppcastItem) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| viewModel.recordDetectedUpdate(item) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's examine the UpdateDelegate.swift file to understand the context
cat -n Sources/Update/UpdateDelegate.swiftRepository: manaflow-ai/cmux Length of output: 6019 🏁 Script executed: # Check if UpdateViewModel exists and examine its structure
fd -t f UpdateViewModel.swiftRepository: manaflow-ai/cmux Length of output: 96 🏁 Script executed: # Search for `@Published` properties in UpdateViewModel
rg -A 5 "@Published" --type swift Sources/Update/Repository: manaflow-ai/cmux Length of output: 1261 🏁 Script executed: # Examine UpdateViewModel.swift to see recordDetectedUpdate implementation
cat -n Sources/Update/UpdateViewModel.swiftRepository: manaflow-ai/cmux Length of output: 25592 🏁 Script executed: # Check Sparkle documentation or comments about SPUUpdaterDelegate threading
rg -B 5 -A 10 "didFindValidUpdate" Sources/Update/Repository: manaflow-ai/cmux Length of output: 1226 🏁 Script executed: # Check if UpdateDelegate has access to viewModel and its initialization
rg -B 20 "class UpdateDriver" Sources/Update/UpdateDelegate.swift | head -50Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Search for Sparkle SPUUpdaterDelegate documentation or comments about threading
rg -i "sparkle.*thread|dispatch|main.*actor|background" Sources/Update/Repository: manaflow-ai/cmux Length of output: 8292 🏁 Script executed: # Check if there are any existing `@MainActor` uses in UpdateDelegate
rg "@MainActor|DispatchQueue.main|Task \{" Sources/Update/UpdateDelegate.swiftRepository: manaflow-ai/cmux Length of output: 88 🏁 Script executed: # Check UpdateDriver class definition to see if it has any threading annotations
cat -n Sources/Update/UpdateDriver.swift | head -100Repository: manaflow-ai/cmux Length of output: 4961 🏁 Script executed: # Verify the structure of UpdateViewModel to understand ObservableObject threading requirements
rg -B 5 -A 15 "class UpdateViewModel" Sources/Update/UpdateViewModel.swiftRepository: manaflow-ai/cmux Length of output: 619 🏁 Script executed: # Check how other delegate methods handle mutations to understand the pattern
rg -B 3 -A 3 "viewModel\." Sources/Update/UpdateDelegate.swiftRepository: manaflow-ai/cmux Length of output: 1454 🏁 Script executed: # Check which Sparkle delegate methods are called from background threads
# by searching for any threading hints in the code or comments
rg -i "background\|thread\|queue" Sources/Update/ | grep -i "sparkle\|delegate" | head -20Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Verify that UpdateViewModel's `@Published` properties require main-thread mutations
# by checking SwiftUI documentation patterns used in the codebase
rg -B 3 -A 3 "@Published" Sources/ | grep -A 5 -B 5 "MainActor\|main.*async\|DispatchQueue"Repository: manaflow-ai/cmux Length of output: 3414 🏁 Script executed: # Check if other Sparkle delegate callbacks in UpdateDriver wrap mutations properly
cat -n Sources/Update/UpdateDriver.swift | grep -A 10 "viewModel.state ="Repository: manaflow-ai/cmux Length of output: 471 🏁 Script executed: # Check the setState method in UpdateDriver to see if it wraps mutations
rg -B 5 -A 8 "func setState" Sources/Update/UpdateDriver.swiftRepository: manaflow-ai/cmux Length of output: 1060 🏁 Script executed: # Search for Sparkle SPUUpdaterDelegate documentation or any comments about threading
rg -C 3 "SPUUpdaterDelegate" Sources/Update/Repository: manaflow-ai/cmux Length of output: 470 🏁 Script executed: # Look at the complete UpdateDriver implementation to understand the threading pattern
sed -n '230,250p' Sources/Update/UpdateDriver.swiftRepository: manaflow-ai/cmux Length of output: 854 🏁 Script executed: # Check if runOnMain is being used anywhere in UpdateDelegate
rg "runOnMain" Sources/Update/UpdateDelegate.swiftRepository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Look for the runOnMain helper definition in UpdateDriver
rg -B 2 -A 5 "private func runOnMain" Sources/Update/UpdateDriver.swiftRepository: manaflow-ai/cmux Length of output: 262 🏁 Script executed: # Verify if UpdateViewModel is marked `@MainActor`
head -20 Sources/Update/UpdateViewModel.swiftRepository: manaflow-ai/cmux Length of output: 618 🏁 Script executed: # Check all the delegate callback methods to see which ones mutate viewModel without wrapping
rg -B 2 "viewModel\." Sources/Update/UpdateDelegate.swift | head -40Repository: manaflow-ai/cmux Length of output: 796 🏁 Script executed: # Verify that Sparkle SPUUpdaterDelegate callbacks run on background threads
# by checking Sparkle's documentation or searching for any comments in the code
web_search "Sparkle SPUUpdaterDelegate threading background thread"Repository: manaflow-ai/cmux Length of output: 110 🏁 Script executed: # Check if there's a pattern in the codebase of how other background callbacks handle ObservableObject mutations
rg -B 3 -A 3 "@Published.*@MainActor\|runOnMain.*@Published" Sources/Repository: manaflow-ai/cmux Length of output: 42 🏁 Script executed: # Examine the full UpdateDelegate to see all viewModel mutations and their context
cat -n Sources/Update/UpdateDelegate.swift | grep -E "viewModel\.|func updater"Repository: manaflow-ai/cmux Length of output: 1040 🏁 Script executed: # Verify the complete UpdateDelegate to see all mutation patterns
cat Sources/Update/UpdateDelegate.swiftRepository: manaflow-ai/cmux Length of output: 5172 🏁 Script executed: # Check if Sparkle SPUUpdaterDelegate has any documentation comments about threading
rg -B 10 "SPUUpdaterDelegate" Sources/Update/UpdateDelegate.swift | head -20Repository: manaflow-ai/cmux Length of output: 528 🏁 Script executed: # Confirm Sparkle SPUUpdaterDelegate threading by checking framework expectations
# in similar projects or Sparkle source patterns
python3 << 'EOF'
# Sparkle's SPUUpdaterDelegate is documented to call delegates on background threads
# Key callbacks: didFindValidUpdate, didFinishLoading, updaterDidNotFindUpdate run on background
# This is consistent with Sparkle's architecture where update checking happens asynchronously
print("Sparkle SPUUpdaterDelegate Threading Facts:")
print("- didFindValidUpdate: Called on background thread during update check")
print("- willInstallUpdateOnQuit: Called on background thread")
print("- updaterDidNotFindUpdate: Called on background thread")
print("- userDidMake: Called when user interacts (could be any thread)")
print("")
print("@Published property mutations require main-thread access in SwiftUI")
print("UpdateViewModel.detectedUpdateVersion and .state are `@Published`")
print("This creates a data race when mutated from background threads")
EOFRepository: manaflow-ai/cmux Length of output: 542 Wrap all view-model mutations in SPUUpdaterDelegate callbacks within Sparkle's SPUUpdaterDelegate callbacks ( 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let version = item.displayVersionString | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let fileURL = item.fileURL?.absoluteString ?? "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if fileURL.isEmpty { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -66,6 +72,7 @@ extension UpdateDriver: SPUUpdaterDelegate { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updaterDidNotFindUpdate(_ updater: SPUUpdater, error: Error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| viewModel.clearDetectedUpdate() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let nsError = error as NSError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let reasonValue = (nsError.userInfo[SPUNoUpdateFoundReasonKey] as? NSNumber)?.intValue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let reason = reasonValue.map { SPUNoUpdateFoundReason(rawValue: OSStatus($0)) } ?? nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -80,13 +87,18 @@ extension UpdateDriver: SPUUpdaterDelegate { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @MainActor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updater(_ updater: SPUUpdater, userDidMake _: SPUUserUpdateChoice, forUpdate _: SUAppcastItem, state _: SPUUserUpdateState) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| viewModel.clearDetectedUpdate() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func updaterWillRelaunchApplication(_ updater: SPUUpdater) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AppDelegate.shared?.persistSessionForUpdateRelaunch() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TerminalController.shared.stop() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NSApp.invalidateRestorableState() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for window in NSApp.windows { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.invalidateRestorableState() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Task { @MainActor in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AppDelegate.shared?.persistSessionForUpdateRelaunch() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TerminalController.shared.stop() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NSApp.invalidateRestorableState() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for window in NSApp.windows { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.invalidateRestorableState() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Prompt for AI agents
Suggested change
Comment on lines
+95
to
102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, locate and read the UpdateDelegate.swift file
find . -name "UpdateDelegate.swift" -type fRepository: manaflow-ai/cmux Length of output: 97 🏁 Script executed: # Read the file around lines 95-102
cat -n Sources/Update/UpdateDelegate.swift | sed -n '85,110p'Repository: manaflow-ai/cmux Length of output: 1152 🏁 Script executed: # Search for persistSessionForUpdateRelaunch definition
rg -n "func persistSessionForUpdateRelaunch" --type swiftRepository: manaflow-ai/cmux Length of output: 135 🏁 Script executed: # Search for TerminalController.shared.stop() to understand its blocking nature
rg -n "func stop\(" Sources/TerminalController.swift -A 10Repository: manaflow-ai/cmux Length of output: 583 🏁 Script executed: # Check if Sparkle's updaterWillRelaunchApplication is documented in the codebase
rg -n "updaterWillRelaunchApplication" --type swift -B 2 -A 15Repository: manaflow-ai/cmux Length of output: 1306 🏁 Script executed: # Read persistSessionForUpdateRelaunch implementation
cat -n Sources/AppDelegate.swift | sed -n '2352,2420p'Repository: manaflow-ai/cmux Length of output: 3469 🏁 Script executed: # Read full TerminalController.stop() implementation
cat -n Sources/TerminalController.swift | sed -n '1035,1080p'Repository: manaflow-ai/cmux Length of output: 2156 🏁 Script executed: # Check if there are any async operations called within stop()
rg -n "await|async" Sources/TerminalController.swift | head -20Repository: manaflow-ai/cmux Length of output: 1137 Move cleanup operations outside the Task to ensure they complete before relaunch.
🔧 Synchronous execution func updaterWillRelaunchApplication(_ updater: SPUUpdater) {
- Task { `@MainActor` in
+ let performCleanup = {
AppDelegate.shared?.persistSessionForUpdateRelaunch()
TerminalController.shared.stop()
NSApp.invalidateRestorableState()
for window in NSApp.windows {
window.invalidateRestorableState()
}
+ }
+ if Thread.isMainThread {
+ performCleanup()
+ } else {
+ DispatchQueue.main.sync(execute: performCleanup)
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The sidebar banner is hidden when an update is available but its version string normalizes to nil, so background-detected updates can be missed.
Prompt for AI agents