Repository navigation
Settings store imports appearance at init instead of live-applying it (restores 70bcbda20e) - #8913
Merged
austinywang merged 1 commit intoJul 27, 2026
Conversation
70bcbda decoupled settings appearance replay from Ghostty reload: the file store imports appearanceMode into UserDefaults and the app-lifecycle observer owns live application, so the store cannot re-enter Ghostty while this singleton initializes. e788e56 (Agent Hibernation) re-added the pre-decoupling machinery verbatim in a rebase, without touching the tests that pin the decoupled contract. This restores the decoupling, and the startup suite now silences the global appearance-defaults observer so it measures only what the store itself does.
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesManaged settings refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR restores lifecycle-owned appearance application.
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified. Startup still reads and applies the appearance imported into UserDefaults, while post-launch file reloads write through UserDefaults with the lifecycle observer active to update both application and terminal appearance. Important Files Changed
Reviews (1): Last reviewed commit: "settings: import appearance at store ini..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Launch the app with an appearance mode in cmux.json and the settings file store applies it live while the store singleton is still initializing — reaching appearance application and terminal-theme synchronization from inside init, which is exactly the re-entrancy 70bcbda removed in May. The store is supposed to only import the value into UserDefaults; the app-lifecycle observer owns live application.
What happened: 70bcbda ("fix: decouple settings appearance replay from Ghostty reload") shipped the decoupling and added KeyboardShortcutSettingsFileStoreStartupTests to pin it. Six days later, e788e56 ("Add Agent Hibernation") re-added the entire pre-decoupling machinery verbatim inside KeyboardShortcutSettingsFileStore.swift — the
appearanceEnvironmentfield, thesynchronizeManagedAppearanceTerminalThemethreading, and both live-apply branches — without touching those tests. The diff shape reads as a rebase that resolved against the older version of the file; nothing in the hibernation feature uses the re-added machinery, and no other file references it. The pinning suite has been red since (18 of 24 tests), which dispatch-only CI never surfaced.This restores the decoupled shape on today's file: the appearance key is guarded out of managed-default side effects at the source, both live-apply branches are gone, and the dead parameter chain comes out end to end. Newer code that grew around the clobber (the legacy warn-before-quit parameters, the language-override branch, today's
reload()return type) is preserved untouched.One test-side change rides along: the startup suite's mock records every live apply, including the global appearance-defaults observer's legitimate reaction to the UserDefaults writes the store makes while importing. The suite now stops that observer in setUp and restarts it in tearDown, so it measures only what the store itself does.
Verified on this branch, each suite alone against a fresh test build: KeyboardShortcutSettingsFileStoreStartupTests 24/24 green (18 failures on main), and AppearanceSettingsTests — the suite pinning the observer half of the contract — 24/24 green.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Stop live-applying appearance during settings store initialization. The store now only imports
appearanceModeinto UserDefaults; the app lifecycle observer applies it after launch to prevent re-entrancy.Bug Fixes
KeyboardShortcutSettingsFileStoreinit/reload and guarded the appearance key out of managed-default side effects.Refactors
appearanceEnvironment, appearance/terminal-theme sync code, and thesynchronizeManagedAppearanceTerminalThemeplumbing; simplified side-effect types and method signatures.Written for commit 61fde69. Summary will update on new commits.
Summary by CodeRabbit