Repository navigation
Conversation
…store init Simulates macOS 26 (build 25D771280a+) where applicationDidFinishLaunching fires before cmuxApp.init() body completes, so isApplicationFinishedLaunching() returns true while CmuxSettingsFileStore is still initializing. The existing guard in AppIconSettings.applyIcon only blocks when the flag is false; on affected macOS 26 builds the flag is already true, so applyIcon proceeds to call startAppearanceObservation → applyIconForCurrentAppearance → NSApplication APIs that crash during App.init() re-entrancy. This test currently FAILS (startObservationCallCount > 0). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…S 26 crash On macOS 26 (build 25D771280a+), applicationDidFinishLaunching fires before cmuxApp.init() body completes because @NSApplicationDelegateAdaptor stored properties are initialized before the init body runs. This causes isApplicationFinishedLaunching() to return true while CmuxSettingsFileStore is still initializing, bypassing the existing guard in AppIconSettings.applyIcon. The result is a deterministic crash at startup: AppIconAppearanceObserver.applyIconForCurrentAppearance() + 648 closure manaflow-ai#3 in static AppIconSettings.Environment.live() CmuxSettingsFileStore.applyManagedUserDefaultsValue(_:for:) CmuxSettingsFileStore.init(...) Add isInitializationComplete flag to CmuxSettingsFileStore. Both applyManagedUserDefaultsValue and restoreUserDefaultsBackup now skip AppIconSettings.applyIcon while the flag is false (i.e. during init). AppDelegate.ensureApplicationIcon() handles the initial icon apply once applicationDidFinishLaunching runs, which is already the correct path. Fixes manaflow-ai#3099 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@KIT64 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a deterministic crash on macOS 26.3.1 (build 25D771280a+) where Confidence Score: 5/5Safe to merge — minimal, targeted fix with a dedicated regression test and no observable behavior change on unaffected code paths. The change is a narrow, two-line guard addition with no effect on post-init behavior. The regression test correctly simulates the macOS 26 re-entrancy condition and validates the fix. Existing tests for the startup-replay and double-replay scenarios continue to pass. No P0/P1 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI Runtime
participant App as cmuxApp.init()
participant Store as CmuxSettingsFileStore.init()
participant AppDelegate as AppDelegate
participant AppIcon as AppIconSettings
SwiftUI->>App: init() begins
Note over App: @NSApplicationDelegateAdaptor stored props init'd first
SwiftUI->>AppDelegate: applicationDidFinishLaunching (macOS 26: fires early)
Note over AppDelegate: isApplicationFinishedLaunching() → true
App->>Store: init() begins
Store->>Store: bootstrapPrimaryTemplateIfNeeded()
Store->>Store: reload() → applyManagedUserDefaultsValue(appIconMode)
Store->>Store: guard isInitializationComplete else break ✅ (skips applyIcon)
Store->>Store: isInitializationComplete = true
Store-->>App: init() complete
AppDelegate->>AppIcon: ensureApplicationIcon() → applyIcon() ✅ (safe path)
Reviews (1): Last reviewed commit: "fix: skip appIcon apply during CmuxSetti..." | Re-trigger Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Thanks for this! The settings-store init no longer applies side effects like the app icon landed on main in #4353. You opened this first, so you got there first. Closing since main covers it now. |
2 participants
Summary
Fixes #3099 — deterministic crash on launch on macOS 26.3.1 (build 25D771280a+).
Root cause: On this build,
applicationDidFinishLaunchingfires beforecmuxApp.init()body completes.@NSApplicationDelegateAdaptorstored properties are initialized before theinit()body, which causes SwiftUI to wire up the delegate (and fireapplicationDidFinishLaunching) before_ = KeyboardShortcutSettings.settingsFileStoreruns. As a result,isApplicationFinishedLaunching()returnstruewhileCmuxSettingsFileStore.init()is still executing, bypassing the existing guard inAppIconSettings.applyIconand crashing at:Fix: Add
isInitializationCompleteflag toCmuxSettingsFileStore. BothapplyManagedUserDefaultsValueandrestoreUserDefaultsBackupnow skipAppIconSettings.applyIconwhileisInitializationComplete == false(i.e. duringinit()).AppDelegate.ensureApplicationIcon()handles the initial icon apply inapplicationDidFinishLaunching, which is already the correct path.Two-commit structure:
testSettingsFileStoreDoesNotApplyAppIconDuringInitEvenIfLaunchIsAlreadyFinished) — simulates macOS 26 by settingisApplicationFinishedLaunching: { true }and asserting nostartAppearanceObservationcalls during store init.isInitializationCompleteflag.Workaround (for users on macOS 26.3.1 until a fix ships)
Create
~/.config/cmux/settings.jsonwith any non-automaticappIconMode:{ "$schema": "https://raw.githubusercontent.com/manaflow-ai/cmux/main/web/data/cmux-settings.schema.json", "appIconMode": "light" }Test plan
testSettingsFileStoreDoesNotApplyAppIconDuringInitEvenIfLaunchIsAlreadyFinishedpasses after the fix committestSettingsFileStoreDoesNotApplyAutomaticAppIconDuringStartupReplayandtestSettingsFileStoreCanReplayAutomaticAppIconSettingTwiceWithoutTouchingAppKitstill passapplicationDidFinishLaunching(viaensureApplicationIcon())appIconModechanges in settings.json still take effect after app is running🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Summary by cubic
Fixes a deterministic crash on launch in macOS 26.3.1 by deferring app icon application during
CmuxSettingsFileStoreinitialization; the initial icon now applies afterapplicationDidFinishLaunching.isInitializationCompleteflag set after the firstreload();applyManagedUserDefaultsValueandrestoreUserDefaultsBackupskipAppIconSettings.applyIconuntil initialization completes.AppDelegate.ensureApplicationIcon()applies the icon after launch.testSettingsFileStoreDoesNotApplyAppIconDuringInitEvenIfLaunchIsAlreadyFinished.Written for commit 7ecb658. Summary will update on new commits.