Skip to content

feat(settings): expose the interactive terminal theme picker - #13036

Closed
teamleaderleo wants to merge 2 commits into
manaflow-ai:mainfrom
teamleaderleo:feature/expose-terminal-theme-picker
Closed

teamleaderleo wants to merge 2 commits into
manaflow-ai:mainfrom
teamleaderleo:feature/expose-terminal-theme-picker

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Reviewer summary

Adds an interactive terminal theme picker in Settings, with preview, search, and the existing theme storage path.

What changed

  • add a Terminal Theme row to Settings with a Choose… action
  • present the existing interactive cmux themes picker in a temporary terminal tab
  • reuse the bundled CLI and Ghostty picker for theme discovery, search/filtering, live preview, light/dark/both targeting, apply, cancel, and persistence
  • close the temporary picker tab automatically when the picker exits

This revives the user-facing idea from #699 against the current theme implementation. The picker itself now already exists in cmux/Ghostty, so this PR only exposes that existing path instead of carrying a second SwiftUI theme catalog and persistence flow.

Implementation

Settings calls a new SettingsHostActions.openTerminalThemePicker() host action. The host opens a focused terminal tab in the current workspace and runs the app-bundled CLI as:

<cmux.app>/Contents/Resources/bin/cmux themes; exit

That keeps cmux themes as the single entry point and lets the current Ghostty picker own all theme behavior.

Testing

The change is intentionally narrow and relies on the existing cmux themes picker and terminal-surface creation paths. The new Settings button has a stable accessibility identifier: SettingsTerminalThemePickerButton.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds a Terminal Theme row to Settings that opens the existing interactive cmux themes picker in a temporary, focused terminal tab, so users no longer need to run the CLI by hand. The tab closes automatically when the picker exits.

The picker reuses the bundled cmux CLI and Ghostty theme picker, so search, filtering, live preview, light/dark targeting, apply, cancel, and persistence all match the CLI experience. Hosts without the bundled CLI fall back to a no-op instead of surfacing a broken action.

Written for commit 620a7c4. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added a Terminal Theme setting with an option to open the interactive theme picker.
    • The picker opens in a focused terminal pane when the required terminal workspace is available.
    • Hosts without terminal theme picker support remain unaffected.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e59e4cac-e85a-4b70-a48e-ec7b940f5339

📥 Commits

Reviewing files that changed from the base of the PR and between 4c67b4d and 482ec50.

📒 Files selected for processing (3)
  • Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swift
  • Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/TerminalSection.swift
  • Sources/HostSettingsActions.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The settings UI adds a Terminal Theme row. The host action contract and implementation open cmux themes; exit in a focused terminal pane after validating required prerequisites.

Changes

Terminal Theme Picker

Layer / File(s) Summary
Settings entry point
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swift, Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/TerminalSection.swift
The settings host contract exposes openTerminalThemePicker(). The Terminal section adds a localized theme row and button that calls the action. Package-only hosts use the default no-op implementation.
Host picker execution
Sources/HostSettingsActions.swift
The host action validates the bundled cmux executable and selected workspace, creates a focused pane that runs cmux themes; exit, and logs or beeps when prerequisites or pane creation fail.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: austinywang

Sequence Diagram(s)

sequenceDiagram
  participant Settings
  participant HostSettingsActions
  participant TerminalPane
  participant CmuxCLI
  Settings->>HostSettingsActions: openTerminalThemePicker()
  HostSettingsActions->>TerminalPane: Create focused pane
  TerminalPane->>CmuxCLI: Run cmux themes then exit
  CmuxCLI-->>Settings: Display theme picker
Loading

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 errors, 2 warnings)

Check name Status Explanation Resolution
Cmux Swift Logging ❌ Error The PR adds two production diagnostics in Sources/HostSettingsActions.swift at the new openTerminalThemePicker() path. They use hostSettingsLogger, but the file-scoped declaration remains `priva… Declare the logger as nonisolated private let hostSettingsLogger = Logger(...). Do not publish the full bundle path or arbitrary error description. Use a fixed diagnostic, or log those dynamic values with explicit private redaction such a…
Cmux User-Facing Error Privacy ❌ Error The new Settings action creates a terminal pane with <bundled cmux> themes; exit, so it newly exposes the existing CLI's stdout/stderr as user-facing command output. In the reviewed base and head, `… Use cmux-level, generic error copy for the Settings-launched picker. Do not forward Ghostty/helper names, environment or config paths, raw helper stderr, raw upstream errors, errno text, or exit details to the terminal. Keep detailed diagno…
Cmux Full Internationalization ❌ Error The PR adds three user-facing Settings strings with localization APIs: settings.terminal.theme (“Terminal Theme”), settings.terminal.theme.subtitle (“Browse Ghostty themes with live preview, inclu… Add all three keys to Resources/Localizable.xcstrings. Provide real translated values, including English, for every existing catalog locale: ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th…
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description explains the feature and implementation, but it does not follow the required template. It lacks the Demo Video section, Review Trigger block, and Checklist, and the Testing section doe… Add the required Demo Video, Review Trigger, and Checklist sections. Complete the Testing section with specific test steps and manual verification results. Mark each applicable checklist item.
✅ Passed checks (20 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request does not change Cloud terminal creation or persistent transport. Its new action validates the bundled local cmux executable and calls the existing `SurfacePaneFactory.makeTerm…
Cmux Swift Actor Isolation ✅ Passed PASS. The diff adds a synchronous action to the existing explicitly @MainActor SettingsHostActions protocol and its default extension. TerminalSection and HostSettingsActions were already `@Ma…
Cmux Swift Blocking Runtime ✅ Passed The pull request adds no prohibited blocking or timing primitive. The new openTerminalThemePicker() method is @MainActor code that checks the bundled CLI, creates a terminal pane, and focuses it. …
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only Settings UI and HostSettingsActions theme-picker integration. The added action launches the bundled cmux themes terminal command and does not add or move any `b…
Cmux Expensive Synchronous Load ✅ Passed PASS — The PR adds a main-actor settings action that validates the bundled CLI and launches a terminal pane through SurfacePaneFactory; it adds no agent-history loader, transcript/JSONL parsing, dir…
Cmux Cache Substitution Correctness ✅ Passed PASS: The reviewed diff only adds a Settings action, a protocol/default implementation, and terminal-pane creation. It does not replace an authoritative read with a cache in a persistence, history, un…
Cmux No Hacky Sleeps ✅ Passed PASS. The authoritative pull-request diff changes only three Swift files. The custom rule applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts; Swift timing and blocking code …
Cmux Algorithmic Complexity ✅ Passed The PR adds one button action, one protocol requirement with a no-op default, and a host method that performs prerequisite checks and delegates to existing pane-creation/focus APIs. The changed code i…
Cmux Swift Concurrency ✅ Passed PASS. The PR adds no legacy async pattern covered by the check. The added host action is synchronous and uses no Dispatch queues, Combine state, completion handlers, or fire-and-forget Task. `SurfaceP…
Cmux Swift @Concurrent ✅ Passed PASS. The authoritative diff adds only synchronous APIs and a synchronous Settings button action. SettingsHostActions, TerminalSection, and HostSettingsActions are @MainActor; the new `openTer…
Cmux Swift Package Boundaries ✅ Passed PASS. The diff adds a SwiftUI settings row and a SettingsHostActions callback in the CmuxSettingsUI package. The app-target implementation is only lifecycle and AppKit/Ghostty terminal glue: it us…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The reviewed range changes only three Swift source files. It does not change a Package.swift manifest, any Package.resolved, .gitignore, or Xcode project/package-reference file. `Packages/…
Cmux Swiftui State Layout ✅ Passed PASS: The SwiftUI diff only adds a SettingsCardRow with a Button action in TerminalSection. It adds no ObservableObject, @Published, @StateObject, @EnvironmentObject, GeometryReader, l…
Cmux Architecture Rethink ✅ Passed PASS. The authoritative diff adds one Settings button routed through SettingsHostActions.openTerminalThemePicker() and one @MainActor host implementation. The implementation uses the existing `Sur…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR adds a Settings button and calls SurfacePaneFactory.makeTerminalPane with .workspace(id: workspace.id, placement: .tab). Terminal panes and tabs are explicitly allowed. The diff adds …
Cmux Source Artifacts ✅ Passed The pull request changes only three existing Swift source files. The diff contains 69 added source lines, with no logs, media, temporary directories, caches, build output, dependency checkout, or broa…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR adds no test or debug seam in production Swift source. The changed Sources files add openTerminalThemePicker() as a product settings action, its package default no-op, and a Settings button…
Title check ✅ Passed The title clearly and concisely describes the main change: exposing the interactive terminal theme picker in Settings.
Full details: Cmux Swift Logging

Explanation

The PR adds two production diagnostics in Sources/HostSettingsActions.swift at the new openTerminalThemePicker() path. They use hostSettingsLogger, but the file-scoped declaration remains private let hostSettingsLogger in a @MainActor file instead of the required nonisolated private let form. The diagnostics also mark cliURL.path and String(describing: error) as .public; the bundle path can contain a user's home-directory name, and the error detail is not explicitly sanitized. These are changed logging paths, not pre-existing unused logging debt.

Resolution

Declare the logger as nonisolated private let hostSettingsLogger = Logger(...). Do not publish the full bundle path or arbitrary error description. Use a fixed diagnostic, or log those dynamic values with explicit private redaction such as .private(mask: .hash).

Full details: Cmux User-Facing Error Privacy

Explanation

The new Settings action creates a terminal pane with &lt;bundled cmux&gt; themes; exit, so it newly exposes the existing CLI's stdout/stderr as user-facing command output. In the reviewed base and head, CLI/CMUXCLI+Themes.swift is unchanged, but its interactive path can emit Bundled Ghostty theme picker helper not found, wrap raw launch errors, and report raw signal/exit/errno details; the CLI top level writes these errors to standard error. The pull request activates this path from Settings without sanitizing those messages. Ghostty is an upstream implementation name, and the new Settings subtitle also directly says Browse Ghostty themes. No user-configured vendor exception applies.

Resolution

Use cmux-level, generic error copy for the Settings-launched picker. Do not forward Ghostty/helper names, environment or config paths, raw helper stderr, raw upstream errors, errno text, or exit details to the terminal. Keep detailed diagnostics only in sanitized internal logs. Replace the new subtitle with vendor-neutral wording such as Browse terminal themes with live preview, including separate light and dark choices.

Full details: Cmux Full Internationalization

Explanation

The PR adds three user-facing Settings strings with localization APIs: settings.terminal.theme (“Terminal Theme”), settings.terminal.theme.subtitle (“Browse Ghostty themes with live preview, including separate light and dark choices.”), and settings.terminal.theme.choose (“Choose…”). However, neither the base nor head contains these keys in Resources/Localizable.xcstrings, and the PR does not modify any catalog. The app catalog currently covers ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant. The host command and logger text are not user-facing localization violations.

Resolution

Add all three keys to Resources/Localizable.xcstrings. Provide real translated values, including English, for every existing catalog locale: ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant. Keep the existing String(localized:defaultValue:) calls aligned with those catalog keys.

Full details: Description check

Explanation

The description explains the feature and implementation, but it does not follow the required template. It lacks the Demo Video section, Review Trigger block, and Checklist, and the Testing section does not state how the change was tested or what was verified manually.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

Review follow-up in 620a7c4:

  • removed the three new localization keys by reusing the existing localized settings.app.theme (Theme) and settings.browser.import.choose (Choose…) strings; the vendor-specific subtitle is gone;
  • Settings-launched cmux themes now keeps the interactive stdout/TTY path but redirects raw CLI/helper stderr away from the user-facing terminal;
  • the two new host diagnostics are fixed messages with no bundle path or raw error text, and the shared logger is explicitly nonisolated.

Direct cmux themes CLI behavior is unchanged. This is scoped to the native Settings exposure path.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Local UI verification — September 19, 2026

Tested 620a7c4452556289d9f4f4f5f38cd07313e023fd.

  • Tagged app build passed; the bundled CLI enumerated 463 themes.
  • Settings → Terminal → Theme → Choose opens the interactive picker in a temporary terminal tab.
  • /dracula filters to Dracula and Dracula+ and updates the preview.
  • Escape leaves search; a second Escape cancels, closes the temporary tab, and returns focus to the original surface.
  • A before/after filesystem comparison confirmed cancellation restored the original absent tagged Ghostty configuration. No saved theme was changed.

Apply/persistence was not exercised in this pass.

Built as an isolated tagged Debug app. The only local overlay was the two build-script files from #12973 to enable CMUX_DEV_BACKEND_MODE=local; application and Settings source matched the commit above.

Screenshots

Settings entry point

Settings entry point

Filtered theme preview

Filtered theme preview

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Replaced by #13224: same commits, head branch moved into the org.

@teamleaderleo
teamleaderleo deleted the feature/expose-terminal-theme-picker branch September 23, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant