Fix cross-window theme background gating after jump-to-unread by lawrencecchen · Pull Request #861 · manaflow-ai/cmux

lawrencecchen · 2026-03-04T10:04:04Z

Summary

Fix a window-background/theme mismatch that can occur when jumping to unread activity across windows (Cmd+Shift+U).

GhosttyNSView.applyWindowBackgroundIfActive() previously gated updates using the globally active TabManager selection, which can be stale during cross-window workspace transitions.

This change makes gating selection-aware per owning window:

resolve the owning tab manager from the surface tab id
prefer owning-window selectedTabId when available
fall back to active manager selection only when owning context is unavailable

Regression tests

Added WindowBackgroundSelectionGateTests for:

owning-window selection precedence
rejection when owning selection differs
active-selection fallback when owning context is missing
permissive behavior when selection context is unavailable

Validation

xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' build
xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -only-testing:cmuxTests/WindowBackgroundSelectionGateTests test
scripts/codex-review.sh /Users/lawrencechen/fun/cmuxterm-hq/worktrees/task-workspace-theme-reconciliation (0 findings)

Summary by cubic

Fixes a window background/theme mismatch when jumping to unread across windows. Gating now prefers the owning window’s selected tab, allows temporary nil selections, and is centralized in a helper for clearer logic and tests.

Bug Fixes
- Gate background per owning window: resolve owning manager from surface tab id, prefer owning selection; if no owning manager, fall back to global active selection; allow when selection is temporarily nil and when no context is available; logic centralized in GhosttyNSView.shouldApplyWindowBackground.
- Added unit tests for owning-window precedence, mismatch rejection, nil-selection allowance, active-selection fallback, and no-context cases.

^{Written for commit 18bc760. Summary will update on new commits.}

Summary by CodeRabbit

Tests
- Added comprehensive test coverage for window background selection logic to ensure correct background application across different window/tab selection scenarios.
Refactor
- Centralized and simplified the decision logic that determines when a window surface background is applied, improving reliability and maintainability without changing user-facing behavior.

vercel · 2026-03-04T10:04:09Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Mar 4, 2026 10:36am

coderabbitai · 2026-03-04T10:04:26Z

📝 Walkthrough

Walkthrough

A new static helper method shouldApplyWindowBackground is added to GhosttyNSView that determines window-background applicability by prioritizing an owning window's selected tab ID over the active selected tab ID. applyWindowBackgroundIfActive is refactored to delegate its gating decision to this helper; tests added.

Changes

Cohort / File(s)	Summary
Window Background Logic Extraction `Sources/GhosttyTerminalView.swift`	Added `static func shouldApplyWindowBackground(surfaceTabId: UUID?, owningManagerExists: Bool, owningSelectedTabId: UUID?, activeSelectedTabId: UUID?) -> Bool`. Refactored `applyWindowBackgroundIfActive` to use this helper for gating the window-background application, preserving existing color/opacity and state updates.
Background Selection Gate Tests `cmuxTests/GhosttyConfigTests.swift`	Added `WindowBackgroundSelectionGateTests` verifying helper behavior across scenarios: prioritizing owningSelectedTabId, rejecting mismatched IDs, allowing when owningManagerExists but owningSelectedTabId is nil, falling back to activeSelectedTabId when no owning manager, and handling nil surfaceTabId cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I peeked inside tabs to see who wins the hue,
The owner gets first dibs, then the active comes through.
A helper hopped in to settle the spat,
Tests gave a nod — now the choice is exact. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main change: fixing cross-window theme background gating after a specific workflow event (jump-to-unread), which aligns directly with the PR's core objective of resolving stale theme updates during cross-window workspace transitions.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch task-workspace-theme-reconciliation

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

greptile-apps · 2026-03-04T10:06:13Z

Greptile Summary

Fixed race condition where background updates during cross-window workspace transitions (e.g., Cmd+Shift+U jump-to-unread) could use stale global tab manager state, causing theme mismatches.

Refactored applyWindowBackgroundIfActive() to prioritize owning window's selectedTabId over global active manager
Extracted selection gating logic into testable static method shouldApplyWindowBackground()
Added comprehensive test suite (WindowBackgroundSelectionGateTests) covering selection precedence, fallback behavior, and edge cases

Confidence Score: 5/5

This PR is safe to merge with minimal risk
Focused bug fix with clear logic, comprehensive test coverage (4 test cases covering all branches), proper fallback handling, and follows Swift best practices
No files require special attention

Important Files Changed

Filename	Overview
Sources/GhosttyTerminalView.swift	Added window-aware background gating logic with proper fallback hierarchy: owning window selection → active selection → permissive default
cmuxTests/GhosttyConfigTests.swift	Added comprehensive test suite covering all selection precedence scenarios and edge cases

_{Last reviewed commit: 68aed1e}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

cmuxTests/GhosttyConfigTests.swift (1)

707-722: Consider one extra edge-case assertion for nil surfaceTabId with non-nil selections.

You already assert permissive behavior with no selection context; adding this variant would fully lock in the early-return contract when surfaceTabId is absent.

➕ Suggested test addition

     XCTAssertTrue(
         GhosttyNSView.shouldApplyWindowBackground(
             surfaceTabId: nil,
             owningSelectedTabId: nil,
             activeSelectedTabId: nil
         )
     )
+    XCTAssertTrue(
+        GhosttyNSView.shouldApplyWindowBackground(
+            surfaceTabId: nil,
+            owningSelectedTabId: UUID(),
+            activeSelectedTabId: UUID()
+        )
+    )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/GhosttyConfigTests.swift` around lines 707 - 722, Add a third
assertion to testShouldApplyWindowBackgroundAllowsWhenNoSelectionContext that
verifies GhosttyNSView.shouldApplyWindowBackground returns true when
surfaceTabId is nil but owningSelectedTabId and/or activeSelectedTabId are
non-nil; specifically call
GhosttyNSView.shouldApplyWindowBackground(surfaceTabId: nil,
owningSelectedTabId: UUID(), activeSelectedTabId: nil) and optionally
surfaceTabId: nil, owningSelectedTabId: nil, activeSelectedTabId: UUID(),
asserting XCTAssertTrue for each to lock in the early-return behavior when
surfaceTabId is absent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2998-3005: The current flatMap collapses "no owning tab manager"
and "owning tab manager exists but its selectedTabId is nil" into the same nil
owningSelectedTabId; change the lookup to first get the owning manager (e.g. let
owningTabManager = tabId.flatMap { appDelegate?.tabManagerFor(tabId: $0) }) and
then derive owningSelectedTabId = owningTabManager?.selectedTabId, and pass both
owningSelectedTabId and an explicit owningManagerExists flag (owningTabManager
!= nil) into shouldApplyWindowBackground (update the shouldApplyWindowBackground
signature/call to accept the new flag) so the logic can distinguish missing
manager vs nil selection when deciding same-window updates.

---

Nitpick comments:
In `@cmuxTests/GhosttyConfigTests.swift`:
- Around line 707-722: Add a third assertion to
testShouldApplyWindowBackgroundAllowsWhenNoSelectionContext that verifies
GhosttyNSView.shouldApplyWindowBackground returns true when surfaceTabId is nil
but owningSelectedTabId and/or activeSelectedTabId are non-nil; specifically
call GhosttyNSView.shouldApplyWindowBackground(surfaceTabId: nil,
owningSelectedTabId: UUID(), activeSelectedTabId: nil) and optionally
surfaceTabId: nil, owningSelectedTabId: nil, activeSelectedTabId: UUID(),
asserting XCTAssertTrue for each to lock in the early-return behavior when
surfaceTabId is absent.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 32ebba68-01c8-45d9-951d-58e7bf271d5c

📥 Commits

Reviewing files that changed from the base of the PR and between f5de515 and 68aed1e.

📒 Files selected for processing (2)

Sources/GhosttyTerminalView.swift
cmuxTests/GhosttyConfigTests.swift

cubic-dev-ai

No issues found across 2 files

lawrencecchen · 2026-03-04T10:25:45Z

Addressed CodeRabbit feedback in f303a8e:

owning-manager presence is now handled explicitly in background gating
active-manager fallback only applies when owning manager is missing
tests extended for (a) owning-manager-present + nil owning selection and (b) nil surfaceTabId with non-nil selection context

Validation rerun:

xcodebuild cmux Debug build: succeeded
WindowBackgroundSelectionGateTests: 5 passed
codex review (uncommitted): 0 findings

…ow-ai#861) * Fix cross-window theme background gating * Handle owning-manager nil-selection theme edge case * Simplify window background gating helper

Fix cross-window theme background gating

68aed1e

vercel Bot deployed to Preview March 4, 2026 10:04 View deployment

coderabbitai Bot reviewed Mar 4, 2026

View reviewed changes

Comment thread Sources/GhosttyTerminalView.swift

cubic-dev-ai Bot reviewed Mar 4, 2026

View reviewed changes

Handle owning-manager nil-selection theme edge case

f303a8e

vercel Bot deployed to Preview March 4, 2026 10:26 View deployment

Simplify window background gating helper

18bc760

vercel Bot deployed to Preview March 4, 2026 10:36 View deployment

lawrencecchen merged commit 79cfe2d into main Mar 4, 2026
12 checks passed

lawrencecchen deleted the task-workspace-theme-reconciliation branch March 4, 2026 10:48

This was referenced Mar 5, 2026

Fix split zoom visibility sync to prevent black browser pane #929

Closed

Fix blank terminal pane when notification fires on another tab #1155

Closed

This was referenced Mar 13, 2026

Share last-surface close flow between X and Cmd+W #1346

Merged

fix: apply background-opacity and background-blur to terminal rendering area (#879) #1858

Merged

coderabbitai Bot mentioned this pull request Mar 30, 2026

Sync ghostty with upstream (1,150 commits) #2374

Closed

4 tasks

coderabbitai Bot mentioned this pull request Apr 26, 2026

Refactor window backdrop policies #3166

Merged

Navigation Menu

Repository navigation

Fix cross-window theme background gating after jump-to-unread - #861

Fix cross-window theme background gating after jump-to-unread#861
lawrencecchen merged 3 commits into
mainfrom
task-workspace-theme-reconciliation

lawrencecchen commented Mar 4, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

vercel Bot commented Mar 4, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Mar 4, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

lawrencecchen commented Mar 4, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lawrencecchen commented Mar 4, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Regression tests

Validation

Summary by cubic

Summary by CodeRabbit

Uh oh!

vercel Bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

lawrencecchen commented Mar 4, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lawrencecchen commented Mar 4, 2026 •

edited by cubic-dev-ai Bot

Loading

vercel Bot commented Mar 4, 2026 •

edited

Loading

coderabbitai Bot commented Mar 4, 2026 •

edited

Loading