Fix stale terminal foreground after theme switch - #3852
Merged
Merged
Conversation
The appearance-change path currently reaches each terminal surface after the app Ghostty config reload, but the per-surface sequence still only refreshes the host background and redraws. This regression pins the required ordering before the production fix: soft surface config update, host background refresh, then visual refresh. Constraint: Issue 3851 requires a two-commit regression-test-then-fix structure Confidence: high Scope-risk: narrow Directive: Keep app-level Ghostty config reload and per-surface redraw as one ordered sequence Tested: git diff --check Not-tested: Local XCTest/build per user instruction; test is intentionally red until the follow-up fix commit
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a GhosttySurfaceConfigurationRefresh helper to orchestrate per-surface Ghostty configuration reload, background refresh, and visual refresh after app-level config changes. It integrates this into AppDelegate.refreshTerminalSurfacesAfterGhosttyConfigReload so terminal surfaces receive updated palette colors during appearance switches. ChangesSurface Configuration Refresh Orchestration
Sequence Diagram(s)sequenceDiagram
participant AppDelegate
participant GhosttySurfaceConfigurationRefresh
participant GhosttyApp
participant HostedView
participant TerminalSurface
AppDelegate->>GhosttySurfaceConfigurationRefresh: applyAfterAppConfigReload(liveSurface?, source, reloadCB, refreshHostBG, forceRefreshCB)
alt surface exists
GhosttySurfaceConfigurationRefresh->>GhosttyApp: reloadSurfaceConfiguration(surface, true, source)
end
GhosttySurfaceConfigurationRefresh->>HostedView: refreshHostBackground()
GhosttySurfaceConfigurationRefresh->>TerminalSurface: forceRefresh(forceRefreshReason)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 SummaryFixes stale terminal foreground colors after theme switches by soft-reloading each live terminal surface's configuration before the existing hosted-background refresh and force-redraw on app-level Ghostty config reload.
Confidence Score: 5/5Safe to merge; the change is a focused, well-tested fix with no regressions introduced. The fix is narrow and correct: the new liveSurfaceForGhosttyAccess guard prevents calling into a freed Ghostty surface pointer, the three-step sequence is enforced in the right order, and both the happy path and the nil-surface path are covered by regression tests. forceRefresh already guards internally against a nil surface. No new timing dependencies, flags, or singletons are introduced. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "Keep terminal palettes fresh after appea..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,18 @@ | |||
| @MainActor | |||
| enum GhosttySurfaceConfigurationRefresh { | |||
| static let forceRefreshReason = "appDelegate.refreshAfterGhosttyConfigReload" | |||
Contributor
There was a problem hiding this comment.
forceRefreshReason is a pure immutable string constant, but because the enclosing enum is @MainActor, this static let is implicitly main-actor-isolated. Any call site outside a @MainActor context (e.g., a nonisolated logging helper or a background diagnostic) would need to await MainActor.run { ... } just to read a plain string. The actor isolation rule flags value-only utilities that should be nonisolated to avoid unnecessary main-actor coupling.
Suggested change
| static let forceRefreshReason = "appDelegate.refreshAfterGhosttyConfigReload" | |
| nonisolated static let forceRefreshReason = "appDelegate.refreshAfterGhosttyConfigReload" |
Rule Used: Flag new or materially worsened Swift 6 actor isol... (source)
Contributor
Author
There was a problem hiding this comment.
Fixed by marking forceRefreshReason nonisolated so reading the plain string does not require MainActor isolation.
— Claude Code
The app-level Ghostty config reload now pushes the already-loaded app config into each live terminal surface with a soft surface reload before repainting. That matches the direct GHOSTTY_ACTION_RELOAD_CONFIG ordering and prevents dark-theme foreground palettes from being reused after a dark-to-light appearance switch. Constraint: Use soft reload so surfaces reuse the app config already loaded by the appearance path Rejected: Force redraw only | redraw uses stale per-surface palette state Confidence: high Scope-risk: narrow Directive: Do not separate per-surface config update from the subsequent redraw on app-level config reloads Tested: git diff --check Not-tested: Local XCTest/build per user instruction; CI will run the regression
austinywang
force-pushed
the
issue-3851-theme-switch-stale-fg
branch
from
May 11, 2026 10:03
11d694f to
ecaf2e1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced May 12, 2026
Closed
This branch was successfully deployed
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
1 participant
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.
Summary
Testing
Note
Medium Risk
Touches terminal surface refresh behavior during Ghostty config reload; incorrect ordering or nil-surface handling could cause missed updates or visual regressions across all open terminals.
Overview
Fixes stale terminal colors after theme/config reload by pushing the reloaded Ghostty config into each live terminal surface before repainting.
Introduces
GhosttySurfaceConfigurationRefresh.applyAfterAppConfigReloadto enforce the sequence soft per-surface config reload → hosted background refresh → forced surface redraw, and updatesAppDelegate.refreshTerminalSurfacesAfterGhosttyConfigReloadto use a live-surface guard.Adds unit tests verifying the call order and the behavior when the surface is unavailable.
Reviewed by Cursor Bugbot for commit ecaf2e1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #3851 by soft-reloading each live terminal surface with the app’s reloaded Ghostty config before repaint to prevent stale foreground colors after theme switches. The surface update, host background refresh, and redraw now run in one ordered pass.
liveSurfaceForGhosttyAccess.Written for commit ecaf2e1. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests