Repository navigation
iOS: support arbitrary terminal themes - #6664
Conversation
The iOS terminal hardcoded the Monokai palette in two places in GhosttyRuntime (the per-process config and the on-disk default config), plus the SwiftUI letterbox chrome and the input accessory bar. Introduce a shared TerminalTheme value type in CMUXMobileCore (bg, fg, cursor, cursor-text, selection, and the 16 ANSI palette colors as hex strings, Codable/Equatable/Sendable) with a built-in Monokai default and a ghostty config directive generator. TerminalThemeStore holds the active theme process-wide; GhosttyRuntime builds its config from it and exposes setTheme(_:), and the terminal chrome reads the same store so it blends with any theme. Invalid or incomplete themes fall back to Monokai. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a shared terminal theme model and store, includes theme data in host status payloads, decodes it on iOS, and propagates theme-driven colors through runtime, shell, and SwiftUI surface updates. ChangesTerminal Theme System
Estimated code review effort: 4 (Complex) | ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ 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 |
Greptile SummaryThis PR removes four Monokai hardcodes in the iOS terminal and replaces them with a
Confidence Score: 5/5Safe to merge. The concurrency model is correct throughout: GhosttyRuntime is @MainActor-isolated, TerminalThemeStore is @mainactor, and the deduplication guards in applyLiveThemeIfRunning prevent redundant config rebuilds. The implementation correctly threads the theme from Mac config → wire → iOS store → live runtime recolor without any actor isolation violations. The fallback to Monokai is applied at every boundary (decode error, nil theme, invalid palette) so a bad payload cannot leave the terminal in a broken state. The two observations flagged are minor style issues (a double-optional flatten and a redundant in-place write during theme refresh) with no behavioral impact. GhosttySurfaceView.swift — the refreshThemeColors method calls applyBackgroundColorFromConfig after setting the same values, which is a harmless double-write but worth cleaning up. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Mac as Mac (MobileHostService)
participant Wire as mobile.host.status JSON
participant RPC as MobileHostStatusResponse (iOS)
participant Shell as MobileShellComposite
participant Store as TerminalThemeStore (@MainActor)
participant SwiftUI as WorkspaceDetailView (SwiftUI)
participant Rep as GhosttySurfaceRepresentable
participant Runtime as GhosttyRuntime (@MainActor)
participant Surface as GhosttySurfaceView
Mac->>Wire: publicStatusPayload() with theme (GhosttyConfig.load())
Wire->>RPC: decode leniently (try? — bad theme → nil, not a decode failure)
RPC->>Shell: applyTerminalTheme(payload.theme)
Shell->>Store: TerminalThemeStore.set(theme) → validatedOrDefault()
Shell->>Shell: "if current != previous → terminalThemeGeneration &+= 1"
Shell->>SwiftUI: "@Observable change triggers body re-render"
SwiftUI->>Rep: updateUIView(themeGeneration: N)
Rep->>Runtime: applyLiveThemeIfRunning()
Runtime->>Store: "read current (if current == lastAppliedTheme → no-op)"
Runtime->>Runtime: rebuildConfigFromStore()
Runtime->>Surface: ghostty_surface_update_config (all registered surfaces)
Runtime->>Surface: refreshAllSurfacesForThemeChange()
Surface->>Surface: refreshThemeColors() → backgroundColor, inputProxy, cursor overlay
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Mac as Mac (MobileHostService)
participant Wire as mobile.host.status JSON
participant RPC as MobileHostStatusResponse (iOS)
participant Shell as MobileShellComposite
participant Store as TerminalThemeStore (@MainActor)
participant SwiftUI as WorkspaceDetailView (SwiftUI)
participant Rep as GhosttySurfaceRepresentable
participant Runtime as GhosttyRuntime (@MainActor)
participant Surface as GhosttySurfaceView
Mac->>Wire: publicStatusPayload() with theme (GhosttyConfig.load())
Wire->>RPC: decode leniently (try? — bad theme → nil, not a decode failure)
RPC->>Shell: applyTerminalTheme(payload.theme)
Shell->>Store: TerminalThemeStore.set(theme) → validatedOrDefault()
Shell->>Shell: "if current != previous → terminalThemeGeneration &+= 1"
Shell->>SwiftUI: "@Observable change triggers body re-render"
SwiftUI->>Rep: updateUIView(themeGeneration: N)
Rep->>Runtime: applyLiveThemeIfRunning()
Runtime->>Store: "read current (if current == lastAppliedTheme → no-op)"
Runtime->>Runtime: rebuildConfigFromStore()
Runtime->>Surface: ghostty_surface_update_config (all registered surfaces)
Runtime->>Surface: refreshAllSurfacesForThemeChange()
Surface->>Surface: refreshThemeColors() → backgroundColor, inputProxy, cursor overlay
Reviews (7): Last reviewed commit: "ci: refresh Swift file-length budget aft..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| nonisolated(unsafe) private static var storage: TerminalTheme = .monokai | ||
| private static let lock = NSLock() | ||
|
|
||
| /// The active theme, defaulting to ``TerminalTheme/monokai``. | ||
| public static var current: TerminalTheme { | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| return storage | ||
| } | ||
|
|
||
| /// Sets the active theme. An invalid or `nil` theme falls back to Monokai so | ||
| /// the terminal always renders with a complete palette. | ||
| public static func set(_ theme: TerminalTheme?) { | ||
| let resolved = theme?.validatedOrDefault() ?? .monokai | ||
| lock.lock() | ||
| storage = resolved | ||
| lock.unlock() | ||
| } |
There was a problem hiding this comment.
NSLock where actor isolation would be the correct shape
TerminalThemeStore uses nonisolated(unsafe) + NSLock — a pattern the project's concurrency rules flag as a blocking primitive that should instead be expressed with actor isolation. An actor would also give Swift's type system full insight into the isolation boundary, instead of relying on the manual nonisolated(unsafe) escape hatch that bypasses strict-concurrency checking. The only practical obstacle is that current needs to be synchronous to work in string interpolation inside GhosttyRuntime; a nonisolated stored-property approach on an actor (reading an @unchecked Sendable snapshot) or a global actor with a synchronous getter keeps callsites identical while staying within Swift 6's isolation model.
Rule Used: Flag new blocking or timing-based synchronization ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| public static func set(_ theme: TerminalTheme?) { | ||
| let resolved = theme?.validatedOrDefault() ?? .monokai | ||
| lock.lock() | ||
| storage = resolved | ||
| lock.unlock() | ||
| } |
There was a problem hiding this comment.
current guards the unlock with defer to be exception-safe, but set(_:) does not. While a plain struct assignment can't throw today, the inconsistency is fragile — a future refactor that adds any work between lock.lock() and lock.unlock() would silently deadlock if it throws. Align both paths with defer.
| public static func set(_ theme: TerminalTheme?) { | |
| let resolved = theme?.validatedOrDefault() ?? .monokai | |
| lock.lock() | |
| storage = resolved | |
| lock.unlock() | |
| } | |
| public static func set(_ theme: TerminalTheme?) { | |
| let resolved = theme?.validatedOrDefault() ?? .monokai | |
| lock.lock() | |
| defer { lock.unlock() } | |
| storage = resolved | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalTheme.swift`:
- Around line 54-60: The isValid property currently permits colors in
non-canonical formats (like bare "rrggbb" without the # prefix) because
rgbComponents() accepts both formats, but the documented contract specifies
colors should be stored as `#RRGGBB` hex strings. To enforce the canonical format,
modify the isValid property to validate that all colors (background, foreground,
cursor, selectionBackground, selectionForeground, cursorText, and palette
entries) match the pattern `#RRGGBB` (with the hash prefix) before passing them to
rgbComponents(). Additionally, normalize any color input in the initializer by
ensuring all color values are prefixed with # if missing, so that
ghosttyColorDirectives emits the correct canonical format and the contract is
consistently enforced throughout the codebase.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalThemeStore.swift`:
- Around line 11-28: The TerminalThemeStore currently uses manual NSLock
synchronization on nonisolated(unsafe) static storage for thread safety, but all
call sites are main-thread-bound, so this is unnecessary complexity. Replace the
manual locking approach by applying `@MainActor` isolation to the
TerminalThemeStore class itself. Remove the nonisolated(unsafe) annotation and
NSLock declaration, eliminate the lock.lock() and lock.unlock() calls from both
the current getter and set method, and rely on `@MainActor` to provide Swift 6
concurrency safety at the class level instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef89dcdf-50d4-4538-83da-6a4e1e5eb156
📒 Files selected for processing (6)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalTheme.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalThemeStore.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TerminalThemeTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPalette.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttyRuntime.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift
| public var isValid: Bool { | ||
| guard palette.count == Self.paletteCount else { return false } | ||
| var colors = [background, foreground, cursor, selectionBackground, selectionForeground] | ||
| colors.append(contentsOf: palette) | ||
| if let cursorText { colors.append(cursorText) } | ||
| return colors.allSatisfy { Self.rgbComponents($0) != nil } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does Ghostty configuration require a leading "#" for hex colors in directives like background = ...andpalette = N=..., or are bare RRGGBB values valid?
💡 Result:
In Ghostty configuration, a leading "#" is optional for hex color values. Both #RRGGBB and bare RRGGBB formats are valid for directives such as background, foreground, and palette [1][2][3]. Specifically, documentation for directives like background = ... and palette = N=... explicitly states that the color component can be specified as either a hex code (with or without the hash, e.g., #RRGGBB or RRGGBB) or a named X11 color [1][2][3]. For the palette directive, while the hex color value allows the optional "#", the palette index N supports decimal, binary (0b), octal (0o), and hexadecimal (0x) formats [2][3][4].
Citations:
- 1: https://github.com/ghostty-org/ghostty/blob/main/src/config/Config.zig
- 2: https://man.archlinux.org/man/ghostty.5.en
- 3: https://ghostty.org/docs/config/reference
- 4: https://man.archlinux.org/man/ghostty.1.txt
🏁 Script executed:
find . -name "TerminalTheme.swift" -path "*CMUXMobileCore*" | head -1Repository: manaflow-ai/cmux
Length of output: 230
🏁 Script executed:
cat -n "Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalTheme.swift" | head -100Repository: manaflow-ai/cmux
Length of output: 5458
Enforce canonical #RRGGBB format in isValid and during emission.
Lines 9–10 and 14–25 document that colors are stored as #rrggbb hex strings. However, rgbComponents() accepts bare rrggbb (line 62), so isValid (line 59) permits it, and ghosttyColorDirectives (lines 80–94) emits the original format unchanged. A theme with background: "ff8000" passes validation and produces background = ff8000, breaking the stated #rrggbb contract and risking drift at integration boundaries. Normalize to canonical #rrggbb by validating that colors match the #RRGGBB pattern and normalizing input in the initializer or during storage.
Proposed fix
+ private static func isCanonicalHex(_ value: String?) -> Bool {
+ guard let value, value.count == 7, value.first == "#" else { return false }
+ return Int(value.dropFirst(), radix: 16) != nil
+ }
/// Whether every color string parses and the palette has exactly 16 entries.
public var isValid: Bool {
guard palette.count == Self.paletteCount else { return false }
var colors = [background, foreground, cursor, selectionBackground, selectionForeground]
colors.append(contentsOf: palette)
if let cursorText { colors.append(cursorText) }
- return colors.allSatisfy { Self.rgbComponents($0) != nil }
+ return colors.allSatisfy { Self.isCanonicalHex($0) }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TerminalTheme.swift`
around lines 54 - 60, The isValid property currently permits colors in
non-canonical formats (like bare "rrggbb" without the # prefix) because
rgbComponents() accepts both formats, but the documented contract specifies
colors should be stored as `#RRGGBB` hex strings. To enforce the canonical format,
modify the isValid property to validate that all colors (background, foreground,
cursor, selectionBackground, selectionForeground, cursorText, and palette
entries) match the pattern `#RRGGBB` (with the hash prefix) before passing them to
rgbComponents(). Additionally, normalize any color input in the initializer by
ensuring all color values are prefixed with # if missing, so that
ghosttyColorDirectives emits the correct canonical format and the contract is
consistently enforced throughout the codebase.
Source: Learnings
…apply) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The local surface background (area behind/around cells + letterbox + input accessory bar) was sourced from the once-built singleton ghostty config, so a theme change left it on the old color while cells repainted. Source the local background from TerminalThemeStore and rebuild the live runtime config on a theme-generation change (ghostty_app_update_config + ghostty_surface_update_config) so the renderer's defaults and palette follow the new theme without a remount. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swift`:
- Around line 91-94: The theme refresh is being triggered from each
GhosttySurfaceRepresentable instance, causing repeated global rebuilds for a
single themeGeneration change. Move the call to
GhosttyRuntime.applyLiveThemeIfRunning into a single process-wide apply path, or
add an applied-generation guard inside GhosttyRuntime/shared theme handling so
only one refresh happens per generation. Update the GhosttySurfaceRepresentable
coordinator flow and lastAppliedThemeGeneration bookkeeping so mounted views do
not each reapply the same global theme refresh.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`:
- Around line 323-325: The accessory bar setup in TerminalInputTextView should
not keep using hard-coded light/dark control tints when the background changes
with the active theme. Update the accessory control styling near
accessoryBarBackgroundView and the dismiss button configuration to derive
icon/text colors from the current theme foreground or choose a contrasting
variant based on background luminance, so the controls remain readable across
light and dark themes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a199259e-c76f-497e-b5d0-f1a523f0e3b5
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttyRuntime.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift
| if themeGeneration > 0 { | ||
| GhosttyRuntime.applyLiveThemeIfRunning() | ||
| } | ||
| context.coordinator.lastAppliedThemeGeneration = themeGeneration |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Apply the global theme refresh once per generation, not once per view.
GhosttyRuntime.applyLiveThemeIfRunning() already rebuilds the shared config and refreshes all registered surfaces. Calling it from each representable instance means one themeGeneration bump triggers one full global refresh per mounted view, and the first mount after a theme change can rebuild twice because GhosttyRuntime.shared() has already read the current store. Move this to a single process-wide apply point, or add a runtime-level applied-generation guard, so one theme change produces one batch update.
As per path instructions, apply .github/review-bot-rules/algorithmic-complexity.md: “flag nested full-collection scans, per-target rescans for batch actions”.
Also applies to: 113-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swift`
around lines 91 - 94, The theme refresh is being triggered from each
GhosttySurfaceRepresentable instance, causing repeated global rebuilds for a
single themeGeneration change. Move the call to
GhosttyRuntime.applyLiveThemeIfRunning into a single process-wide apply path, or
add an applied-generation guard inside GhosttyRuntime/shared theme handling so
only one refresh happens per generation. Update the GhosttySurfaceRepresentable
coordinator flow and lastAppliedThemeGeneration bookkeeping so mounted views do
not each reapply the same global theme refresh.
Source: Path instructions
| backgroundView.backgroundColor = Self.themeBarColor | ||
| backgroundView.translatesAutoresizingMaskIntoConstraints = false | ||
| self.accessoryBarBackgroundView = backgroundView |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the accessory controls choose a contrasting foreground.
This bar now inherits any terminal background, but the controls in the same block still assume a dark surface (for example, the dismiss button’s fixed light-gray tint a few lines below). Light themes will leave at least that affordance below usable contrast. Please derive the toolbar icon/text colors from the active theme foreground, or switch between light/dark variants based on the background luminance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`
around lines 323 - 325, The accessory bar setup in TerminalInputTextView should
not keep using hard-coded light/dark control tints when the background changes
with the active theme. Update the accessory control styling near
accessoryBarBackgroundView and the dismiss button configuration to derive
icon/text colors from the current theme foreground or choose a contrasting
variant based on background luminance, so the controls remain readable across
light and dark themes.
…emes # Conflicts: # Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift
…dedup, cursor-text, MainActor) Fixes from Cursor Bugbot, CodeRabbit, and Greptile on the arbitrary-themes PR: - MobileHostStatusResponse: decode `theme` leniently (`try?`). A present-but- malformed theme no longer fails the whole host-status decode, which had forced raw-bytes transport and skipped capability/identity adoption over a cosmetic field. - GhosttyRuntime.applyLiveThemeIfRunning: dedupe by theme value. One themeGeneration bump fires updateUIView on every mounted representable; the rebuild + refresh-all now runs once per theme value instead of once per view. Seeded from init so the first mount after a change doesn't rebuild a config that already matches. - MobileHostTerminalTheme (Mac producer): only serialize cursor-text when the config actually parsed a `cursor-text` directive (hasParsedCursorTextColor). Otherwise the phone emitted an explicit cursor-text the Mac never had and mis-colored the cursor label; nil lets the phone derive contrast like the Mac. - TerminalThemeStore: @mainactor instead of nonisolated(unsafe) + NSLock. All producers/consumers are main-actor; the compiler now proves it. TerminalPalette marked @mainactor to match. - scheduleHostIdentityAdoptionIfNeeded: also applyTerminalTheme on the full- timeout recovery path, so a theme skipped by the 750ms probe timeout still adopts. applyTerminalTheme is idempotent. - TerminalTheme.ghosttyColorDirectives: emit canonical `#rrggbb` (normalize bare `rrggbb` input) so the stored contract holds at the ghostty boundary. - Refreshed the stale terminalThemeGeneration doc comment (live-recolor, not remount). Tests: new canonical-hex emission test and malformed-theme-tolerance decode test; existing CMUXMobileCore (146) and CmuxMobileRPC (68) suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
The arbitrary-themes work grows several already-large iOS files (GhosttyRuntime, GhosttySurfaceView, MobileShellComposite, TerminalInputTextView, MobileHostService) and newly tracks GhosttySurfaceRepresentable past 500 lines. This is real feature debt (live recolor + theme sync), not gratuitous bloat; splitting these god-files is a separate refactor. Refresh the budget to current counts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift">
<violation number="1" location="Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift:2998">
P2: Theme can be applied from a stale status response after reconnect, which can recolor the active terminal/chrome to the wrong host theme. This happens because `applyTerminalTheme(payload.theme)` runs after an `await` without revalidating `remoteClient === client`; adding that guard before applying theme keeps stale responses inert.</violation>
</file>
<file name="Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPalette.swift">
<violation number="1" location="Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPalette.swift:23">
P3: This fallback branch is dead for current usage, because `TerminalThemeStore.current` is always validated/Monokai and `background`/`foreground` always parse. Keeping an unreachable `.black` path also conflicts with the Monokai-fallback policy used elsewhere, which can mislead future maintenance.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // only bumps the generation on a real change), so re-applying here is | ||
| // free in the common case and keeps the phone's colors in sync with | ||
| // the Mac even when the probe could not. | ||
| self.applyTerminalTheme(payload.theme) |
There was a problem hiding this comment.
P2: Theme can be applied from a stale status response after reconnect, which can recolor the active terminal/chrome to the wrong host theme. This happens because applyTerminalTheme(payload.theme) runs after an await without revalidating remoteClient === client; adding that guard before applying theme keeps stale responses inert.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift, line 2998:
<comment>Theme can be applied from a stale status response after reconnect, which can recolor the active terminal/chrome to the wrong host theme. This happens because `applyTerminalTheme(payload.theme)` runs after an `await` without revalidating `remoteClient === client`; adding that guard before applying theme keeps stale responses inert.</comment>
<file context>
@@ -2965,6 +2988,14 @@ public final class MobileShellComposite: MobileTerminalOutputSinking {
+ // only bumps the generation on a real change), so re-applying here is
+ // free in the common case and keeps the phone's colors in sync with
+ // the Mac even when the probe could not.
+ self.applyTerminalTheme(payload.theme)
await self.applyHostReportedIdentity(
client: client,
</file context>
| /// Dimmed terminal foreground (`#c8c8c0`). | ||
| static let dimForeground = Color(red: 0xc8 / 255.0, green: 0xc8 / 255.0, blue: 0xc0 / 255.0) | ||
| private static func color(_ hex: String) -> Color { | ||
| guard let rgb = TerminalTheme.rgbComponents(hex) else { return .black } |
There was a problem hiding this comment.
P3: This fallback branch is dead for current usage, because TerminalThemeStore.current is always validated/Monokai and background/foreground always parse. Keeping an unreachable .black path also conflicts with the Monokai-fallback policy used elsewhere, which can mislead future maintenance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPalette.swift, line 23:
<comment>This fallback branch is dead for current usage, because `TerminalThemeStore.current` is always validated/Monokai and `background`/`foreground` always parse. Keeping an unreachable `.black` path also conflicts with the Monokai-fallback policy used elsewhere, which can mislead future maintenance.</comment>
<file context>
@@ -1,17 +1,30 @@
- /// Dimmed terminal foreground (`#c8c8c0`).
- static let dimForeground = Color(red: 0xc8 / 255.0, green: 0xc8 / 255.0, blue: 0xc0 / 255.0)
+ private static func color(_ hex: String) -> Color {
+ guard let rgb = TerminalTheme.rgbComponents(hex) else { return .black }
+ return Color(
+ red: Double(rgb.red) / 255.0,
</file context>
TerminalThemeStore and TerminalPalette are new caseless static-only types the package-conventions lint (namespace-enum/namespace-type) flags after the main rebase. TerminalPalette becomes an internal namespace struct (not an enum, not public, so neither rule applies). TerminalThemeStore stays a process-wide theme singleton (public), converted enum->struct with a reviewed inline lint:allow namespace-type justification: it holds one global rendering resource, not dependency-bearing logic that belongs on an instantiated value. No call-site changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…emes # Conflicts: # .github/swift-file-length-budget.tsv
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 68707c9. Configure here.
| // color overrides into a complete effective palette; the phone applies | ||
| // it so its embedded terminal renders with the Mac's colors instead of | ||
| // the built-in Monokai default. | ||
| let theme = TerminalTheme(ghosttyConfig: GhosttyConfig.load()) |
There was a problem hiding this comment.
Status theme uses stale config cache
Medium Severity
publicStatusPayload builds the wire theme from GhosttyConfig.load() with the default cached path. If the Mac terminal palette changed but the load cache was not invalidated yet, the next mobile.host.status response can advertise colors that no longer match the Mac’s effective terminal theme.
Reviewed by Cursor Bugbot for commit 68707c9. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 64-66: Mark the new SwiftUI state owned by WorkspaceDetailView as
private by updating the declarations of ignoredChatSessionRefreshKey,
ignoredChatSessionRefreshID, and ignoredChatSessionRefreshTask to use private
`@State`. Keep the change limited to these three properties in WorkspaceDetailView
so their state remains internally managed and not externally writable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1c3a6f9-d9b0-4d15-8256-e19c910e5a9d
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftcmux.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift`:
- Around line 64-66: Mark the new SwiftUI state owned by WorkspaceDetailView as
private by updating the declarations of ignoredChatSessionRefreshKey,
ignoredChatSessionRefreshID, and ignoredChatSessionRefreshTask to use private
`@State`. Keep the change limited to these three properties in WorkspaceDetailView
so their state remains internally managed and not externally writable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1c3a6f9-d9b0-4d15-8256-e19c910e5a9d
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swiftcmux.xcodeproj/project.pbxproj
🛑 Comments failed to post (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift (1)
64-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Mark new
@Statepropertiesprivate.
ignoredChatSessionRefreshKey,ignoredChatSessionRefreshID, andignoredChatSessionRefreshTaskshould beprivateper SwiftUI convention (state owned by the view shouldn't be externally writable).🔧 Proposed fix
- `@State` var ignoredChatSessionRefreshKey: String? - `@State` var ignoredChatSessionRefreshID: UUID? - `@State` var ignoredChatSessionRefreshTask: Task<[ChatSessionDescriptor]?, Never>? + `@State` private var ignoredChatSessionRefreshKey: String? + `@State` private var ignoredChatSessionRefreshID: UUID? + `@State` private var ignoredChatSessionRefreshTask: Task<[ChatSessionDescriptor]?, Never>?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.`@State` private var ignoredChatSessionRefreshKey: String? `@State` private var ignoredChatSessionRefreshID: UUID? `@State` private var ignoredChatSessionRefreshTask: Task<[ChatSessionDescriptor]?, Never>?🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 66-66: Prefer empty collection over optional collection
(discouraged_optional_collection)
[Warning] 64-64: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 65-65: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 66-66: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift` around lines 64 - 66, Mark the new SwiftUI state owned by WorkspaceDetailView as private by updating the declarations of ignoredChatSessionRefreshKey, ignoredChatSessionRefreshID, and ignoredChatSessionRefreshTask to use private `@State`. Keep the change limited to these three properties in WorkspaceDetailView so their state remains internally managed and not externally writable.Source: Linters/SAST tools
…ng, themes) Delta: #7318/#7319 mouse cursor-shape + right/middle drag forwarding, #7320 ghostty upstream, #6664 iOS themes, #7196 iOS scroll, #7257/#7307 web, #7329 CLI suggestions, #7222 codex resume update-suppression. Conflicts resolved keeping HEAD structure: - GhosttyTerminalView.swift: took main's viewDidEndLiveResize + resetCursorRects (cursor-shape #7318; ghosttyMouseShape/ghosttyMouseCursor already in HEAD). - Mobile/MobileHostService.swift: union (kept HEAD sharedRequestActivity + main publicStatusPayload theme payload #6664). - project.pbxproj: union-dedup, normalized; test-wiring green. - swift-file-length-budget.tsv regenerated. #7222 (codex update-check suppression) partially landed: the CMUXAgentLaunch package part (AgentResumeArgv.codexUpdateCheckSuppressionOverride) auto-merged and is present; the app-target files (+CodexUpdateCheck.swift + test) extend SurfaceResumeCommandCanonicalizer, a type HEAD dissolved into the CmuxWorkspaces package, so they can't compile and the suppression was never wired (call site was in the HEAD-deleted +PortableAgentExecutable). Removed those two files; re-wire recorded in merge-deferred-gaps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The union kept main's publicStatusPayload (mobile.host.status theme, #6664) which references mobileHostCapabilities, but MobileHostService+Capabilities.swift (which defines it) was not brought into the merge (new-in-main file, anomalously not auto-added). Restored it from origin/main and wired it into pbxproj (Mobile group, bare path). Its deps (TerminalTheme, mobileHostJSONObject) are present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
The iOS terminal only rendered the Monokai theme, hardcoded in four places: the two ghostty config builders in
GhosttyRuntime(the per-process config and the on-disk default config), the SwiftUI letterbox/toolbar chrome (TerminalPalette), and the input accessory bar fill. This makes iOS support arbitrary themes instead.A new
TerminalThemevalue type lives in the sharedCMUXMobileCorepackage: background, foreground, cursor, cursor-text, selection background/foreground, and the 16 ANSI palette colors, all as#rrggbbhex strings. It isCodable,Equatable, andSendable(no UIKit/AppKit), so a theme can be produced anywhere, decoded from JSON, and consumed by the embedded libghostty runtime on iOS. It carries a built-in.monokai, anisValidcheck, avalidatedOrDefault()fallback, and aghosttyColorDirectivesgenerator that emits the matching ghostty config lines.TerminalThemeStoreholds the active theme process-wide behind a lock.GhosttyRuntimebuilds its config fromTerminalThemeStore.currentand exposessetTheme(_:)/currentTheme; callsetThemebefore the runtime is first built. The terminal chrome (TerminalPalette, the input bar) reads the same store so it blends with whatever theme is active rather than a fixed color. Anynil, invalid, or incomplete (not exactly 16 palette entries) theme falls back to Monokai, so the terminal always renders with a complete palette.This reuses the existing color shape already on the wire (the render-grid
Stylecolors and OSC color fields are the same#rrggbbhex strings) instead of inventing a parallel format, and keeps Monokai as the default so behavior is unchanged until a theme is supplied.Testing
swift testonPackages/Shared/CMUXMobileCorepasses (119 tests, 10 suites). NewTerminalThemeTestscover: Monokai validity, JSON encode/decode round-trip, decoding an arbitrary (Solarized-style) theme from JSON, invalid/short-palette fallback to Monokai, hex parsing edge cases, ghostty directive generation (including conditional cursor-text), and the store set/fallback behavior.The iOS-only packages (
CmuxMobileTerminal,CmuxMobileShellUI) depend on UIKit and theGhosttyKit.xcframeworkbinary, which is not built in this worktree, so they were not compiled locally. The changed iOS files use only the publicTerminalTheme/TerminalThemeStoreAPIs that compile cleanly in theCMUXMobileCorepackage build, and parse clean against the iOS SDK. Full Xcode build verification is left to CI.Issues
None. Plain-text feature task, no associated GitHub issue.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches mobile host RPC, ghostty runtime config updates, and several iOS UI paths; malformed themes are isolated, but live config refresh affects all mounted surfaces.
Overview
Adds a shared
TerminalThememodel andTerminalThemeStoreso iOS is no longer hardcoded to Monokai. The Macmobile.host.statuspayload now includes the effective theme fromGhosttyConfig.load(); iOS decodes it leniently (bad theme does not break status) and applies it on connect and on the slower identity recovery path.MobileShellComposite.applyTerminalThemeupdates the store and bumpsterminalThemeGenerationonly when colors actually change.GhosttySurfaceRepresentablereacts to that counter and callsGhosttyRuntime.applyLiveThemeIfRunning()to rebuild ghostty config and refresh surfaces in place, preserving scrollback. SwiftUI/UIKit chrome (TerminalPalette, input accessory bar, surface backgrounds) follows the same store. Mac→phone mapping omitscursorTextunless the Mac config explicitly set it.Reviewed by Cursor Bugbot for commit 68707c9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds end-to-end support for arbitrary terminal themes on iOS and syncs the Mac’s effective theme to the phone. Theme changes now live‑recolor the terminal and UI without remounting, so scrollback is preserved.
New Features
mobile.host.status; built fromGhosttyConfig.load()and serialized viaMobileHostTerminalThemeto matchTerminalTheme.themeinMobileHostStatusResponseand applies it viaMobileShellComposite.applyTerminalTheme(_:), which updates@MainActorTerminalThemeStoreand bumpsterminalThemeGenerationonly on real changes; also reapplies on the full-timeout status recovery path.GhosttySurfaceRepresentablewatchesthemeGenerationand callsGhosttyRuntime.applyLiveThemeIfRunning(), which rebuilds the runtime config (ghostty_app_update_config/ghostty_surface_update_config) and updates all surfaces once per theme value;GhosttySurfaceView,TerminalInputTextView, andTerminalPaletteread fromTerminalThemeStoreto update backgrounds and chrome in place.Bug Fixes
themeobjects; falls back to Monokai without breaking transport/identity.cursorTextwhen explicitly set on the Mac.TerminalThemeStore/TerminalPalette@MainActorto satisfy namespace-type lint.Written for commit 68707c9. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests