Skip to content

Fix top-bar hover crash on Sonoma and Sequoia - #550

Closed
dragonhuntr wants to merge 2 commits into
manaflow-ai:mainfrom
dragonhuntr:fix/topbar-hover-crash-sonoma-sequoia
Closed

dragonhuntr wants to merge 2 commits into
manaflow-ai:mainfrom
dragonhuntr:fix/topbar-hover-crash-sonoma-sequoia

Conversation

@dragonhuntr

@dragonhuntr dragonhuntr commented Feb 26, 2026 •

Copy link
Copy Markdown

Summary

  • Fix crash caused by WindowDragHandleView capturing mouseEntered, mouseExited, appKitDefined, and systemDefined events that should be deferred — these tracking/system events were incorrectly handled as click-like events on Sonoma and Sequoia
  • Fix build error by marking enqueueLaunchServicesRegistrationWork as nonisolated since it only dispatches to a background queue

Test plan

  • Verified build succeeds locally
  • Tested hover interactions on top bar — no crash
  • Verify on both Sonoma and Sequoia

🤖 Generated with Claude Code

dragonhuntr and others added 2 commits February 26, 2026 08:21
Defer titlebar drag-handle hit capture for passive hover/system events and extend regression tests for those event types.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lated

The static method only dispatches to a background queue and doesn't
access main-actor state, so it can safely be nonisolated.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 26, 2026 13:56
@vercel

vercel Bot commented Feb 26, 2026

Copy link
Copy Markdown

@dragonhuntr is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a macOS crash related to titlebar drag-handle hit-testing on newer macOS versions by ensuring hover/tracking/system events don’t get treated like click events, and resolves a Swift concurrency build issue around Launch Services registration scheduling.

Changes:

  • Update drag-handle hit-capture deferral logic to ignore mouseEntered, mouseExited, appKitDefined, and systemDefined events.
  • Add unit tests asserting the drag handle does not capture hits for those passive/system event types.
  • Mark enqueueLaunchServicesRegistrationWork as nonisolated so it can be used as a scheduler without main-actor isolation issues.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cmuxTests/CmuxWebViewKeyEquivalentTests.swift Extends hit-test coverage to ensure passive/system event types don’t trigger drag-handle capture.
Sources/WindowDragHandleView.swift Expands the set of event types that should defer drag-handle hit capture (prevents hover/system-event crash).
Sources/AppDelegate.swift Adjusts Launch Services scheduling helper to be nonisolated to fix concurrency/build constraints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andrewcummings

Copy link
Copy Markdown

Nice — I was working on this same crash this morning. I filed #490 yesterday with a detailed breakdown after digging into the root cause. The deferral expansion here looks correct for the tracking-area events (scenario 2 in the issue), but there's a third crash scenario this doesn't cover:

Layout-triggered hitTest with stale currentEvent — When clicking the "+" button to create a new tab, SwiftUI runs a layout pass that calls DraggableView.hitTest directly (no sendEvent on the stack). NSApp.currentEvent is a stale .leftMouseDown from the click, so it passes the deferral check and falls through to isWindowDragSuppressed(window: dragHandleView.window), which accesses view properties during layout → exclusive access violation.

Two additional hardening suggestions:

  1. Reorder the deferral check to run before isWindowDragSuppressed(window: dragHandleView.window). Currently the deferral runs after accessing dragHandleView.window, which can itself trigger the violation for events that should be deferred. Moving it to the top of windowDragHandleShouldCaptureHit ensures no view properties are touched for passive events.

  2. Add an event-dispatch guard — a windowDragHandleIsInEventDispatch flag set in cmux_sendEvent and checked at the top of DraggableView.hitTest. When hitTest is called outside of sendEvent (i.e., directly by SwiftUI during layout), return nil immediately. This catches the stale-currentEvent scenario completely:

// WindowDragHandleView.swift
var windowDragHandleIsInEventDispatch = false

// In DraggableView.hitTest:
override func hitTest(_ point: NSPoint) -> NSView? {
    guard windowDragHandleIsInEventDispatch else { return nil }
    // ... existing logic
}

// AppDelegate.swift, in cmux_sendEvent:
let wasInEventDispatch = windowDragHandleIsInEventDispatch
windowDragHandleIsInEventDispatch = true
defer { windowDragHandleIsInEventDispatch = wasInEventDispatch }

The drag handle still works for real mouse-button events since those always go through sendEvent → hitTest. Full analysis in #490 — happy to put up a follow-up PR with these if you'd prefer.

@lawrencecchen

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thanks for this! The titlebar hover crash fix (hitTest now bails out for anything but left-mouse-down) landed on main in #736. You opened this first, so you got there first. Closing since main covers it now.

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.

5 participants