Skip to content

Resolve a separate sidebar's content against its own backdrop - #14841

Merged
teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
stoptypingnow:sidebar-follows-app-appearance
Sep 27, 2026
Merged

teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
stoptypingnow:sidebar-follows-app-appearance

Conversation

@stoptypingnow

@stoptypingnow stoptypingnow commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

With the app set to Light and a dark terminal theme, inactive rows in the left sidebar now draw dark text on the light sidebar material instead of white. Fixes #14840.

A sidebar that doesn't share the terminal backdrop draws an NSVisualEffectView material that inherits the app appearance, but since #10149 its content followed the terminal-derived scheme. WindowAppearanceSnapshot now takes the ambient scheme (the resolver passes settings.colorScheme, which ContentView fills from AppearanceSettings.effectiveColorScheme(for: appearanceMode, …)) and uses it for sidebarContentColorScheme and the sidebar tint scheme unless unifySurfaceBackdrops is on. Unified sidebars, chromeColorScheme, and the rest of the chrome keep the terminal authority. Snapshots built without an ambient scheme keep the old behavior, so direct WindowAppearanceSnapshot(...) callers are unchanged.

The tint scheme lives in the shared sidebarSettings, so the right sidebar's material tint variant follows the same rule; its content scheme is untouched.

Evidence

  • 64423f01d4 adds sidebarSchemeFollowsTheSurfaceItIsDrawnOn (unified/separate × terminal/ambient disagreement). swift test --package-path Packages/macOS/CmuxAppKitSupportUI on that commit: 54 tests, 4 issues, all in the separate-sidebar cases.
  • dcabd785d6 is the fix; it also updates two existing resolver tests that asserted the separate sidebar follows the terminal. Same command: 54 tests pass.
  • python3 scripts/verify-local.py --affected upstream/main --swift-changed upstream/main: 3/3 checks pass.
  • Tagged Debug build (CMUX_DEV_BACKEND_MODE=local), Light, theme = light:0x96f: on main the inactive row title is white on the light material; with this branch it's dark. Dark mode renders as before.
  • Not run: the cmuxTests app-host suites. WindowAppearanceSnapshotTests builds snapshots without an ambient scheme, so it should keep the old behavior; CI will confirm.

Related: #10509 fixes group-header labels in the SwiftUI sidebar and doesn't touch this path. #13353 proposes separating UI appearance from terminal themes more broadly.

Summary by CodeRabbit

  • Bug Fixes
    • Sidebar colors now follow the ambient appearance when the sidebar and terminal use separate backdrops. When their backdrops are unified, the sidebar follows the terminal’s resolved color scheme. This keeps sidebar styling consistent with its display surface, including when the sidebar and terminal have different appearance settings.

A sidebar with its own material backdrop resolves that material against
the app appearance, so its content must use the same scheme when an
opaque terminal theme disagrees. A sidebar sharing the terminal backdrop
keeps following the terminal. Fails on main: the separate-sidebar cases
resolve to the terminal scheme.
Since manaflow-ai#10149 sidebar content follows the terminal-derived scheme, but a
sidebar that doesn't share the terminal backdrop draws an
NSVisualEffectView material that inherits the app appearance. With a
Light app setting and a dark terminal theme, inactive workspace rows
drew white text on the light material.

WindowAppearanceSnapshot now takes the ambient scheme and derives
sidebarContentColorScheme and the sidebar tint scheme from it unless
unifySurfaceBackdrops is on; unified sidebars and all other chrome keep
the terminal authority. Snapshots built without an ambient scheme keep
the previous behavior.
@stoptypingnow

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document v2.2 and I hereby sign the CLA

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for opening your first cmux pull request!

We're a small team and the outside-PR queue is long, so a reply can take a while — sometimes longer than we'd like. If this one goes quiet and you'd like eyes on it, comment here and we'll pick it up.

A few things that help:

  • The PR template source has a commented-out "Review Trigger" block of review-bot mentions; it does not show in the rendered description. Pasting it as a comment after your latest commit is the quickest way to get automated review.
  • If the CLA check asks, reply with the sentence it gives you.
  • The verification ladder shows which checks fit your change. Say in the description which ones you ran.
  • If we end up fixing the same problem another way, we'll credit you with a Co-authored-by trailer and link the fix here.

github-actions Bot added a commit that referenced this pull request Sep 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@stoptypingnow
stoptypingnow force-pushed the sidebar-follows-app-appearance branch from a509d35 to dcabd78 Compare September 26, 2026 14:58
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: f2bb8d41-50bc-4b4d-82da-77a3fc321678

📥 Commits

Reviewing files that changed from the base of the PR and between 2123231 and dcabd78.

📒 Files selected for processing (3)
  • Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Appearance/WindowAppearanceResolver.swift
  • Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Appearance/WindowAppearanceSnapshot.swift
  • Packages/macOS/CmuxAppKitSupportUI/Tests/CmuxAppKitSupportUITests/WindowChrome/Appearance/WindowAppearanceResolverTests.swift

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


📝 Walkthrough

Walkthrough

The snapshot now selects the sidebar scheme based on backdrop unification. Separate sidebars use the ambient app scheme when available; unified backdrops use the resolved terminal scheme. The resolver passes the ambient scheme, and tests cover both configurations.

Changes

Sidebar appearance

Layer / File(s) Summary
Select the sidebar scheme
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Appearance/WindowAppearanceSnapshot.swift
The snapshot stores a sidebar-specific scheme. It uses the resolved terminal scheme for unified backdrops and the ambient scheme for separate backdrops when provided.
Pass ambient scheme and verify behavior
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Appearance/WindowAppearanceResolver.swift, Packages/macOS/CmuxAppKitSupportUI/Tests/CmuxAppKitSupportUITests/WindowChrome/Appearance/WindowAppearanceResolverTests.swift
The resolver passes the settings color scheme as the ambient scheme. Tests check sidebar schemes for separate and unified backdrops.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to dcabd

The sidebar scheme change matches the reported appearance behavior. No actionable merge blocker is established; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to dcabd

The change affects how sidebars choose text and tint colors. The reviewed paths lead to visual rendering, with no identified new security exposure. Compatibility for callers that do not supply an ambient scheme is preserved.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The observed exposure is window sidebar appearance, including the shared material tint choice for left and right sidebar backdrops. No independently attackable service or data-store scope is established by this path.

Trust Boundaries and Controls

  • observed — The resolver injects the ambient appearance from settings. A caller that omits an ambient scheme instead retains the terminal-derived fallback; the examined path uses either choice for rendering policy.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
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 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed PR #14841 satisfies the coding requirements in directly linked issue #14840. WindowAppearanceResolver.current(settings:) passes settings.colorScheme as ambientColorScheme. `WindowAppearanceSnaps…
Out of Scope Changes check ✅ Passed The reviewed changes stay within issue #14840. The snapshot property, resolver wiring, compatibility fallback, comments, and regression tests all support sidebar scheme selection and tint behavior. No…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The PR changes only window appearance resolution and related tests in three WindowChrome/Appearance files. The diff introduces no Cloud terminal creation, cmux-tui client, transport, PTY rea…
Cmux Swift Actor Isolation ✅ Passed PASS. The production diff only adds an immutable ColorScheme value, an optional initializer value, and synchronous scheme selection in WindowAppearanceSnapshot, plus passes settings.colorScheme …
Cmux Swift Blocking Runtime ✅ Passed PASS: The PR changes appearance-scheme selection only. The production diff adds ambientColorScheme, sidebarColorScheme, and deterministic scheme assignment. It adds no semaphore, blocking wait, sl…
Cmux Browser Automation Off-Main ✅ Passed PASS. The scoped diff changes only WindowAppearanceResolver, WindowAppearanceSnapshot, and their appearance tests. It adds ambient/sidebar color-scheme resolution and does not change browser socket co…
Cmux Expensive Synchronous Load ✅ Passed PASS. The pull request changes only appearance-scheme resolution and related tests. The production additions pass ambientColorScheme, compute sidebarScheme, and assign color-scheme values. The dif…
Cmux Cache Substitution Correctness ✅ Passed PASS. The diff does not replace a fresh persistence, history, undo, or durable snapshot read with a cache. WindowAppearanceSnapshot is a transient render-pass value. The new ambient scheme has a col…
Cmux No Hacky Sleeps ✅ Passed PASS: The pull request changes only three Swift files. The rule explicitly applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts, and excludes Swift timing and blocking primiti…
Cmux Algorithmic Complexity ✅ Passed PASS: The production diff adds only scalar scheme selection and propagation. WindowAppearanceSnapshot uses one conditional expression with optional fallback, and WindowAppearanceResolver passes on…
Cmux Swift Concurrency ✅ Passed The diff only changes synchronous appearance resolution, snapshot initialization, and tests. Added lines contain no Dispatch queues/groups, Combine state, completion-handler APIs, or fire-and-forget T…
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only synchronous appearance resolution, snapshot initialization, and tests. The review-scoped diff adds no async, nonisolated, @concurrent, Task, or await code…
Cmux Swift Package Boundaries ✅ Passed PASS: The production changes are confined to the existing CmuxAppKitSupportUI SwiftPM target, not an app-target root Sources/ path. WindowAppearanceSnapshot and WindowAppearanceResolver are Ap…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The pull request changes only three Swift source/test files under Packages/macOS/CmuxAppKitSupportUI. It does not change any Package.swift, Package.resolved, .gitignore, workflow, or Xco…
Cmux Swift Logging ✅ Passed The pull request adds no logging. The changed production files only adjust sidebar color-scheme resolution and snapshot state. The added lines contain no print, debugPrint, dump, NSLog, file/s…
Cmux User-Facing Error Privacy ✅ Passed PASS — The production diff changes sidebar color-scheme resolution only. It adds no user-facing error, alert, command output, API error body, or recovery copy. The changed scheme reaches sidebar UI th…
Cmux Full Internationalization ✅ Passed The PR changes only Swift appearance-resolution logic, developer documentation comments, and tests. It adds no user-facing text, localization keys, string catalogs, locale files, web messages, or meta…
Cmux Swiftui State Layout ✅ Passed PASS. The diff changes WindowAppearanceSnapshot value resolution and resolver tests only. It adds no ObservableObject, @Published, @Observable, geometry measurement, lazy/list row store refere…
Cmux Architecture Rethink ✅ Passed PASS. The diff is a small value-snapshot correctness fix. WindowAppearanceResolver supplies settings.colorScheme to WindowAppearanceSnapshot, and the initializer derives one sidebarScheme from…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR changes only appearance resolver/snapshot value types and appearance tests. The authoritative diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, cmux.* ident…
Cmux Source Artifacts ✅ Passed The diff changes only three intentional Swift product and test files under Sources/.../Appearance and Tests/.../Appearance. No logs, screenshots, recordings, caches, build output, temporary direct…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The production diff changes only appearance resolution in WindowAppearanceResolver.swift and WindowAppearanceSnapshot.swift. It adds ambientColorScheme, sidebarColorScheme, and sidebar s…
Title check ✅ Passed The title clearly describes the main change: separate sidebar content resolves against its own backdrop.
Description check ✅ Passed The description provides a detailed problem statement, implementation summary, test results, manual verification, and known limitations. It omits the template headings, Demo Video or screenshots, and …
  • 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.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 27, 2026 11:42
@teamleaderleo
teamleaderleo merged commit b66e365 into manaflow-ai:main Sep 27, 2026
68 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for dcabd785d6: every check was green at merge (20 verified; 17 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 27, 2026
f5c179f iOS: fix the test failures that keep iOS CI red on main (manaflow-ai#14803)
8685bf5 Hold update relaunch while agents are mid-turn (manaflow-ai#14969)
dc90332 Keep CLI socket-discovery tests off the host's real cmux (manaflow-ai#14919)
dd3c91b docs: shorten root agent instructions and link existing procedures (manaflow-ai#14998)
8c744df Rename edits inline or in the palette, never in an alert (manaflow-ai#14986)
9ae4383 Calmer chrome motion: appear instantly, fade out only, no overshoot (manaflow-ai#14984)
6510f56 Write opencode config JSON without escaping slashes (cmux 7140) (manaflow-ai#14805)
ab5e7da ci: stop catch-up merges from failing the CLA check (manaflow-ai#14913)
52c8f41 Add cmux session move for Claude sessions (manaflow-ai#14959)
36785b1 Hide decorative Settings sidebar icons from VoiceOver (manaflow-ai#14989)
4c7158c Label the sound preview button and fix mistranslated action verbs (manaflow-ai#14983)
e704a77 Bound untracked paths stored in last-turn diff baselines (manaflow-ai#14980)
f073df1 Fix remote Files sidebar for names that change under NFD (manaflow-ai#14978)
5c68499 Bump bonsplit: mouse wheel scrolls the overflowed tab strip (manaflow-ai#14985)
9466dcb Keep agent resume bindings through the update-relaunch save (manaflow-ai#14971)
ef8b037 docs: take release notes from a Changelog section in each PR instead of CHANGELOG.md edits (manaflow-ai#14934)
6eddfd7 ci: skip the delta diff when main moved further than the pull request (manaflow-ai#14987)
fefcec7 ci: attribute red PR runs to the machine or the code, re-run machine failures once (manaflow-ai#14977)
c185deb Accept file drops on remote tmux mirror panes (manaflow-ai#14981)
90773c7 test: make CmuxSidebarGit probe waits event-driven (manaflow-ai#14973)
1f2dbfe ci: skip the scheduled Blacksmith cache warmers while owned pools serve PRs (manaflow-ai#14827)
2850651 docs: add a guide to customizing cmux's look (manaflow-ai#14850)
b66e365 Resolve a separate sidebar's content against its own backdrop (manaflow-ai#14841)
88a9360 UI tests: one labelled frame per action, built in CI; scripts/ui-test (manaflow-ai#14966)
20cfa78 fix(omo): resolve relative file refs in the shadow config without double-loading OpenCode config (manaflow-ai#14935)
f0e964c ci: make the aggregate app-host product the default, layers opt-in (manaflow-ai#14975)
52dce98 ci: run and register the machine-failure test (manaflow-ai#14972)
7bf48bc ci: route compile admission by kept-build distance across minis (manaflow-ai#14949)
44fa3f5 Offer cmux in Open With for Markdown, source, and text files (manaflow-ai#14968)
45c2d66 Replay the Claude session id of agents in cmux ssh (cmux-tui) panes (manaflow-ai#14906)
b4c1b31 Label icon-only chrome buttons and localize project panel text (manaflow-ai#14926)
14a6909 seed prefetch: keep the seed adopt would pick, of any seeded width (manaflow-ai#14944)
19e73d2 ci: self-calibrating warm-distance compile estimates (manaflow-ai#14932)
fa98d86 ci: redispatch focused runs the Mac failed before any test started (manaflow-ai#14963)
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.

Light app appearance + dark terminal theme: sidebar rows render white text on the light sidebar material (since 0.64.23)

2 participants