Keep update popover primary buttons legible when the popover is not key by teamleaderleo · Pull Request #19161 · manaflow-ai/cmux

teamleaderleo · 2026-10-11T00:56:01Z

Summary

The update popover's "Install and Relaunch" button showed as an empty gray box with no label (reported on 0.64.25, macOS 15.3.1, dark appearance; the screenshot shows Skip and Later fine and a blank gray bezel where the prominent button should be).

The popover usually opens without becoming the key window (the pill's main window keeps key), so SwiftUI renders its .borderedProminent buttons in the inactive control state. AppKit then draws the gray inactive bezel under the prominent label and the button all but disappears. This is the same failure the Cloud enablement buttons hit and fixed with cloudProminentButtonStyle() (#17127). Nothing in the popover or its NSHostingController overrides tint, foreground color, or color scheme.

This adds updatePrimaryButtonStyle() in CmuxUpdaterUI (.borderedProminent plus controlActiveState = .key) and uses it for every prominent button in the popover: Install and Relaunch (update available and detected background update), Allow (auto-update permission), Restart Now, and Download Latest Version (error view). The buttons now keep their accent bezel and white label whether or not the popover window is key.

Fixes #18687

Testing

Added UpdatePrimaryButtonStyleTests (CmuxUpdaterUI package tests): hosts a probe under an explicit .inactive control state in an offscreen window and checks the primary style hands its content .key, with a control test that the probe sees .inactive without the style. Runs in CI's swift-package lane; not run locally (no local Swift test runs on this machine).
Not verified visually in a running build. The repo's PR-media tours (dogfood/scenarios) have no scenario that opens the update popover, so there is no CI screenshot of it; adding one would need driving the updater into the update-available state and is left out of this PR.

Changelog

Fixed: The update popover's Install and Relaunch button no longer shows as a blank gray box

Proof

CI only; no tour covers the update popover.

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by cubic

Fixes the update popover's "Install and Relaunch" button rendering as a blank gray box when the popover is not the key window.

The fix adds updatePrimaryButtonStyle() in CmuxUpdaterUI, which applies .borderedProminent and forces the .key control active state, matching the treatment already used for Cloud enablement buttons. All prominent buttons in the popover (Install and Relaunch, Allow, Restart Now, Download Latest Version) now use this style so their accent bezel and label stay visible regardless of window focus state.

Added UpdatePrimaryButtonStyleTests covering the key-state override and a control probe confirming the inactive state without the style.
Visual verification was not performed in a running build; no CI tour covers the update popover.

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

Note

Low Risk
UI-only SwiftUI button styling in the updater popover with targeted unit tests; no auth, data, or update logic changes.

Overview
Fixes #18687: primary actions in the update popover (e.g. Install and Relaunch) no longer render as a blank gray box when the popover window is not key.

Adds updatePrimaryButtonStyle()—.borderedProminent with controlActiveState forced to .key so accent and label stay visible in the inactive-window case (same idea as Cloud’s cloudProminentButtonStyle). UpdatePopoverView and UpdateErrorView swap .buttonStyle(.borderedProminent) for this helper on Install and Relaunch, Allow, Restart Now, and Download Latest Version.

UpdatePrimaryButtonStyleTests assert the modifier overrides an explicit .inactive environment; a control test confirms inactive state is observed without the style.

^{Reviewed by Cursor Bugbot for commit de0044a. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

UI Improvements
- Download, install, allow, and restart buttons in update prompts now share consistent primary-button styling.
- The styling maintains a consistent appearance when the update window is inactive.
Tests
- Added coverage verifying how primary-button styling behaves in active and inactive window states.

The update popover usually opens without becoming the key window, so its .borderedProminent actions render in the inactive control state: AppKit draws the gray inactive bezel under the prominent label and "Install and Relaunch" shows as an empty gray box. Force the key control state on the popover's primary actions (Install and Relaunch, Allow, Restart Now, Download Latest Version), as the Cloud enablement buttons already do. Fixes #18687 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-11T00:56:18Z

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

coderabbitai · 2026-10-11T00:56:47Z

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: e05e2467-ce92-4834-b179-79d5ce8323fb

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd1fe7 and de0044a.

📒 Files selected for processing (4)

Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/UpdateErrorView.swift
Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/UpdatePopoverView.swift
Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/View+UpdatePrimaryButton.swift
Packages/macOS/CmuxUpdaterUI/Tests/CmuxUpdaterUITests/UpdatePrimaryButtonStyleTests.swift

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

📝 Walkthrough

Walkthrough

The updater UI adds a shared primary button style that sets controlActiveState to .key. The download, install, allow, and restart buttons use this style. Tests verify the active-state value with and without the style.

Changes

Updater button styling

Layer / File(s)	Summary
Define and test the primary button style `Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/View+UpdatePrimaryButton.swift`, `Packages/macOS/CmuxUpdaterUI/Tests/CmuxUpdaterUITests/UpdatePrimaryButtonStyleTests.swift`	Adds a `View` extension that applies `.borderedProminent` and sets `controlActiveState` to `.key`. Tests compare the environment value when the style is applied and when it is absent.
Apply style to updater buttons `Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/UpdateErrorView.swift`, `Packages/macOS/CmuxUpdaterUI/Sources/CmuxUpdaterUI/UpdatePopoverView.swift`	Replaces `.borderedProminent` with `updatePrimaryButtonStyle()` on the download, install, allow, and restart buttons. Button actions and keyboard shortcuts remain unchanged.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to de004

The updater buttons are consistently covered by the new key-state style, with no concrete merge-blocking issue established.

Important

Pre-merge checks failed

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

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Cmux Swiftui State Layout		The new `ControlActiveStateProbe` mutates external state during SwiftUI rendering at `UpdatePrimaryButtonStyleTests.swift:71`: `body` executes `recorder.states.append(controlActiveState)`. This is a r…	Keep `ControlActiveStateProbe.body` pure. Record the control state from an explicit lifecycle callback such as `onAppear`/`onChange`, or from an `NSViewRepresentable` update callback, instead of appending to `recorder.states` during `body` …
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (23 passed)

Check name	Status	Explanation
Title check		The title clearly identifies the main user-visible change: keeping update popover primary buttons legible when the popover is not key.
Description check		The description includes Summary, Testing, Changelog, and Proof sections. It explains the problem, implementation, test coverage, CI status, and lack of visual verification. The repository Checklist s…
Linked Issues check		Issue `#18687` requires a clear label on the update popup button. The PR adds `updatePrimaryButtonStyle()` with `.borderedProminent` and `.controlActiveState` forced to `.key`. It applies this style to …
Out of Scope Changes check		The changed source files only update prominent-button styling in the update popover and error view. The new tests directly verify that styling behavior. These changes support the fix for issue `#18687` …
Cmux Cloud Persistent Session And Early Input		PASS: The custom check applies to Cloud terminal creation and transport changes. This PR changes only CmuxUpdaterUI update buttons, adds `updatePrimaryButtonStyle()`, and adds focused SwiftUI tests. T…
Cmux Swift Actor Isolation		The production diff only replaces `.buttonStyle(.borderedProminent)` with a SwiftUI `View` helper and adds `updatePrimaryButtonStyle()`, which composes `.borderedProminent` with `\.environment(\.contr…
Cmux Swift Blocking Runtime		The production diff adds only a SwiftUI button-style helper and replaces button styles; it introduces no semaphores, blocking waits, sleeps, delayed dispatch, polling, main-queue sync, or locks. The o…
Cmux Browser Automation Off-Main		The pull request changes only CmuxUpdaterUI SwiftUI views, a button-style helper, and its tests. The authoritative diff changes no browser socket automation files, commands, worker routing, WebKit cal…
Cmux Expensive Synchronous Load		PASS. The reviewed diff changes only SwiftUI button styling, adds the `updatePrimaryButtonStyle()` environment helper, replaces `.borderedProminent` at update-action call sites, and adds focused UI te…
Cmux Cache Substitution Correctness		The diff changes only SwiftUI button styling and related tests. It does not replace an authoritative read with a cached or opportunistic value, and it does not modify persistence, history, undo, or sn…
Cmux No Hacky Sleeps		PASS: The pull request changes only SwiftUI source and Swift tests. It contains no TypeScript, JavaScript, shell, or build/runtime-script changes covered by this check. The test-only bounded run-loop …
Cmux Algorithmic Complexity		The pull request does not introduce an algorithmic-complexity violation. Production changes only replace button modifiers and add `updatePrimaryButtonStyle()`, which applies `.borderedProminent` and o…
Cmux Swift Concurrency		The diff introduces no flagged legacy async pattern. Production changes only add a SwiftUI button-style extension and replace `.buttonStyle(.borderedProminent)` calls. The test uses `@MainActor` and b…
Cmux Swift `@Concurrent`		The diff adds only the synchronous `updatePrimaryButtonStyle()` view modifier and synchronous UI test helpers. It introduces no `nonisolated async`, `@concurrent`, async call site, or CPU/file/network…
Cmux Swift Package Boundaries		PASS — The diff adds a small SwiftUI view helper inside the existing `Packages/macOS/CmuxUpdaterUI` SwiftPM target and applies it to update-popover buttons. This is UI-only behavior with no independen…
Cmux Swiftpm Lockfiles		PASS: The PR changes only CmuxUpdaterUI source and test files. `Packages/macOS/CmuxUpdaterUI/Package.swift` is identical at the base and head, so no SwiftPM dependency or pin changed. No package `.git…
Cmux Swift Logging		The Swift diff adds no print, debugPrint, dump, NSLog, ad hoc file/stdout logging, Logger declaration, or sensitive-data logging. The production changes only apply a button style, and the added test r…
Cmux User-Facing Error Privacy		PASS — The changed production path is the macOS update popover and error-view download button, but the diff changes only button styling and control state. Existing labels remain unchanged, and no user…
Cmux Full Internationalization		The production diff changes only button styling and adds developer comments; it does not add or materially change user-facing text. Existing button labels remain in their pre-existing localized `Strin…
Cmux Architecture Rethink		The diff is a small, local SwiftUI correctness fix. It adds one shared `updatePrimaryButtonStyle()` helper that applies `.borderedProminent` and forces `.controlActiveState` to `.key`, then uses that …
Cmux Swift Auxiliary Window Close Shortcuts		PASS. The diff changes SwiftUI button styling in `UpdatePopoverView.swift` and `UpdateErrorView.swift`, and adds a view modifier. It does not add or materially change a user-visible `NSWindow`, `NSPan…
Cmux Source Artifacts		All four changed paths are intentional Swift source or test files under the existing CmuxUpdaterUI source and test directories. The diff adds a hand-written button-style helper and focused UI tests, a…
Cmux No Test Or Debug Seam In Production Source		The production diff adds `View.updatePrimaryButtonStyle()`, which implements real update-popover behavior by applying `.borderedProminent` and forcing `.key` control state. It does not use a test/debu…

Full details: Cmux Swiftui State Layout

Explanation

The new ControlActiveStateProbe mutates external state during SwiftUI rendering at UpdatePrimaryButtonStyleTests.swift:71: body executes recorder.states.append(controlActiveState). This is a render-time state write introduced by the diff. The production button-style changes do not introduce the other listed patterns.

Resolution

Keep ControlActiveStateProbe.body pure. Record the control state from an explicit lifecycle callback such as onAppear/onChange, or from an NSViewRepresentable update callback, instead of appending to recorder.states during body evaluation.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

🛠️ Fix failing CI checks 💡

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

github-actions · 2026-10-11T01:16:09Z

Seen on other PRs too (likely flaky): NewMachineSheetLayoutTests.swift:237, NewMachineSheetLayoutTests.swift:156 also failed on other PRs before this one; check whether it is this PR's before re-running.

CI failed on de0044a943 (run 38100103647 attempt 2): 1 code.

Job	Verdict	Why
macos / app-host unit tests (changed suites)	code	a test failed

Failures:

seen on other PRs NewMachineSheetLayoutTests.swift:237 long base machine names keep every control inside the sheet: Expectation failed: (basePopUp → nil) != nil (job)
seen on other PRs NewMachineSheetLayoutTests.swift:156 the Base, Size, and Network pop-ups share a leading edge at their own widths: Expectation failed: popUps.first { $0.itemTitles.contains { $0.hasPrefix("cmux-devbox-") } } → nil (job)

Matched log lines

macos / app-host unit tests (changed suites): ✘ Test "long base machine names keep every control inside the sheet" recorded an issue with 1 argument layout → .grid at NewMachineSheetLayoutTests.swift:237:9: Expectation failed: (basePopUp → nil) != nil

Not re-run automatically: macos / app-host unit tests (changed suites) is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

github-actions · 2026-10-11T01:16:34Z

CI fast guards failed on de0044a943 (https://github.com/manaflow-ai/cmux/actions/runs/38100103647). It does not block the merge; a red guard merged into main breaks it for every open PR.
The log named no failed step; see the run.

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally. This comment is updated in place on each push.

Navigation Menu

Repository navigation

Keep update popover primary buttons legible when the popover is not key - #19161

Keep update popover primary buttons legible when the popover is not key#19161
teamleaderleo wants to merge 1 commit into
mainfrom
fix/issue-18687-update-button-label

teamleaderleo commented Oct 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading

Walkthrough

Changes

Pre-merge checks failed

Uh oh!

github-actions Bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

teamleaderleo commented Oct 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Changelog

Proof

Summary by cubic

Summary by CodeRabbit

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Pre-merge checks failed

❌ Failed checks (1 error, 1 warning)

Uh oh!

github-actions Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

teamleaderleo commented Oct 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 11, 2026 •

edited

Loading

github-actions Bot commented Oct 11, 2026 •

edited

Loading