Skip to content

Fix drag-handle crash on launch from stale foreign-window events - #620

Merged
lawrencecchen merged 1 commit into
mainfrom
issue-490-drag-handle-crash
Feb 27, 2026
Merged

lawrencecchen merged 1 commit into
mainfrom
issue-490-drag-handle-crash

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Feb 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add window-identity check to windowDragHandleShouldCaptureHit so stale leftMouseDown events from other apps (Finder, Dock) during launch don't trigger the SwiftUI hierarchy walk while initial layout is mutating (exclusive-access violation)
  • Add NSLock to WindowDragHandleBreadcrumbLimiter for thread safety
  • Add regression test for nil/foreign/matching eventWindow scenarios
  • Fix shouldApplyImmediateHostedStateUpdate test call sites (removed stale hostWindowAttached: param)

Closes #490

Test plan

  • Launch app with mouse hovering over window area (was crash scenario)
  • Launch from Dock, Spotlight, terminal
  • Verify drag-handle still works for normal clicks after launch
  • Run unit tests: xcodebuild test -only-testing:cmuxTests

Summary by CodeRabbit

  • Bug Fixes

    • Improved thread-safety and memory management in window drag operations to enhance stability.
    • Enhanced hit-testing accuracy for drag capture across multiple window scenarios.
  • Tests

    • Updated tests to validate window-aware drag behavior and reentrancy handling.

Add window-identity check to windowDragHandleShouldCaptureHit so stale
leftMouseDown events from other apps (Finder, Dock) during launch don't
trigger the SwiftUI hierarchy walk while initial layout is mutating.
Add NSLock to breadcrumb limiter for thread safety. Update existing
tests to pass eventWindow for window-attached drag handles.
@vercel

vercel Bot commented Feb 27, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Building Building Preview, Comment Feb 27, 2026 7:20am

@coderabbitai

coderabbitai Bot commented Feb 27, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e20d692 and 03fb930.

📒 Files selected for processing (2)
  • Sources/WindowDragHandleView.swift
  • cmuxTests/CmuxWebViewKeyEquivalentTests.swift

📝 Walkthrough

Walkthrough

The changes add thread safety to drag-handle event processing via NSLock and refactor hit-test logic to be window-aware. A method is renamed to windowDragHandleShouldResolveActiveHitCapture with new parameters for event and drag-handle windows, and call-sites are updated accordingly. Cache pruning is extended, and tests are updated to exercise the new window-scoped behavior.

Changes

Cohort / File(s) Summary
Thread safety and hit-test refactoring
Sources/WindowDragHandleView.swift
Added NSLock for thread-safe access to lastEmissionByKey dictionary; extended pruning logic for stale entries when map exceeds 128 items. Renamed windowDragHandleShouldDeferHitCapture(for:) to windowDragHandleShouldResolveActiveHitCapture(for:eventWindow:dragHandleWindow:) with new window-identity and event-type guards. Updated all call-sites to pass eventWindow and dragHandleWindow parameters; adjusted internal references to use dragHandleWindow for consistency in suppression and depth checks.
Test signature alignment
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Updated test calls to pass eventWindow: NSWindow? parameter to windowDragHandleShouldCaptureHit. Adjusted assertions to verify window-aware reentrant hit resolution and nested window scenarios. Removed hostWindowAttached: Bool parameter expectations from shouldApplyImmediateHostedStateUpdate calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through locks so tight,
Where windows dance and events ignite,
No more crashes in the race—
Thread-safe guards now hold their place! 🔒✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: preventing drag-handle crashes caused by stale foreign-window events, which aligns with the primary objective from issue #490.
Linked Issues check ✅ Passed All code objectives from issue #490 are addressed: window-identity check in windowDragHandleShouldCaptureHit [#490], NSLock for thread safety in breadcrumb limiter [#490], and regression tests for nil/foreign/matching eventWindow scenarios [#490].
Out of Scope Changes check ✅ Passed All changes directly support the linked issue objectives: hit-test refactoring for window identity checks, thread-safe breadcrumb limiting with NSLock, stale entry pruning, test updates including hostWindowAttached parameter removal, and new eventWindow parameter added consistently across call sites.

✏️ 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 issue-490-drag-handle-crash

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

@greptile-apps

greptile-apps Bot commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes crash during app launch when mouse hovers over window area. The core issue was stale leftMouseDown events from external apps (Finder, Dock) triggering SwiftUI hierarchy traversal while initial layout was mutating, causing exclusive-access violations.

Key changes:

  • Added window-identity check in windowDragHandleShouldResolveActiveHitCapture to filter out foreign-window events before hierarchy walk
  • Added NSLock to WindowDragHandleBreadcrumbLimiter.shouldEmit for thread-safe dictionary access
  • Added comprehensive regression test covering nil/foreign/matching event window scenarios
  • Cleaned up stale hostWindowAttached: parameter from test call sites

The fix is narrowly scoped and preserves existing behavior for legitimate same-window events while safely rejecting stale cross-app events during launch.

Confidence Score: 5/5

  • Safe to merge - well-tested crash fix with clear scope
  • The changes are defensive (preventing crashes), well-tested with regression coverage, and use proper synchronization primitives. The window-identity check is a sound approach to filter stale events without affecting normal operation.
  • No files require special attention

Important Files Changed

Filename Overview
Sources/WindowDragHandleView.swift Added window-identity check to prevent stale foreign-window events from triggering SwiftUI hierarchy walk during launch, plus NSLock for thread-safe breadcrumb throttling
cmuxTests/CmuxWebViewKeyEquivalentTests.swift Added regression test for foreign-window event handling and fixed stale test call sites by removing obsolete hostWindowAttached parameter

Last reviewed commit: 03fb930

@lawrencecchen
lawrencecchen merged commit 2202044 into main Feb 27, 2026
8 checks passed
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…aflow-ai#490) (manaflow-ai#620)

Add window-identity check to windowDragHandleShouldCaptureHit so stale
leftMouseDown events from other apps (Finder, Dock) during launch don't
trigger the SwiftUI hierarchy walk while initial layout is mutating.
Add NSLock to breadcrumb limiter for thread safety. Update existing
tests to pass eventWindow for window-attached drag handles.

This branch was successfully deployed

1 active deployment
Preview — 03fb9309 Deployed Feb 27, 2026 by vercel[bot]
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.

Crash on launch: exclusive access violation in windowDragHandleShouldCaptureHit

1 participant