Skip to content

fix: keep main windows movable for Swish#5001

Open
fairusage wants to merge 1 commit into
manaflow-ai:mainfrom
fairusage:fix-swish-window-movability
Open

fix: keep main windows movable for Swish#5001
fairusage wants to merge 1 commit into
manaflow-ai:mainfrom
fairusage:fix-swish-window-movability

Conversation

@fairusage
Copy link
Copy Markdown

@fairusage fairusage commented May 29, 2026

Summary

  • keep cmux main windows isMovable so Accessibility-based window managers like Swish can attach titlebar gestures
  • keep isMovableByWindowBackground = false so app content does not become an implicit drag region
  • update the drag behavior unit test to preserve this compatibility

Fixes #1383.

Testing

  • Not run: local machine only has Command Line Tools selected, so xcodebuild fails with tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.


Summary by cubic

Keep main windows movable so Swish and other Accessibility-based window managers can attach titlebar gestures. Background drags stay disabled; only explicit titlebar zones start drags. Fixes #1383.

  • Bug Fixes
    • Set isMovable = true and keep isMovableByWindowBackground = false in main window config.
    • Updated drag behavior test to expect a movable window while preserving explicit drag zones and temporary overrides.

Written for commit 79effca. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Fixed window drag behavior: windows are now properly movable while maintaining correct dragging for interactive elements like tabs and folders.

Review Change Stack

@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b45789d2-24a5-4dd9-889c-eb6cc9079492

📥 Commits

Reviewing files that changed from the base of the PR and between 9a1a7d1 and 79effca.

📒 Files selected for processing (2)
  • Sources/App/CmuxMainWindow.swift
  • cmuxTests/WindowAndDragTests.swift

📝 Walkthrough

Walkthrough

This PR enables window movability in CmuxMainWindow by setting isMovable = true instead of false, while keeping isMovableByWindowBackground = false. The configuration change is validated by a rewritten test that confirms the window remains movable after temporary movability helper operations.

Changes

Window movability for accessibility and external window managers

Layer / File(s) Summary
Window movability configuration and validation
Sources/App/CmuxMainWindow.swift, cmuxTests/WindowAndDragTests.swift
configureCmuxMainWindowDragBehavior(_:) switches isMovable from false to true to enable external window managers (like Swish) to detect the title bar and perform window management gestures, while isMovableByWindowBackground remains false to prevent unintended content drags. Updated comments clarify that the window is now recognizable as movable for Accessibility-based window managers. The test is rewritten to validate this new contract and confirm the window remains movable after withTemporaryWindowMovableEnabled.

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • manaflow-ai/cmux#4290: Adjusts the same window-movability/suppression contract to fix pane-tab drag routing.

Poem

A window stands tall, now ready to move,
Swish gestures flow in their mystical groove,
True to the spirit of titles set free,
While drags of the content respectfully be,
🪟✨ Accessibility wins today!

🚥 Pre-merge checks | ✅ 17 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: enabling window movability in cmux main windows for Swish compatibility.
Description check ✅ Passed The description covers the key changes, issue reference, and testing status, but does not include testing details, demo video, or checklist items as specified in the template.
Linked Issues check ✅ Passed The PR addresses issue #1383 by implementing suggested fix #3: setting isMovable = true by default while keeping isMovableByWindowBackground = false and updating tests accordingly.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the Swish compatibility issue: window movability configuration and related test updates, with no unrelated modifications.
Cmux Swift Actor Isolation ✅ Passed Production code changes only modify window boolean properties in an existing @MainActor-annotated function; no new types, protocols, or actor isolation violations introduced.
Cmux Swift Blocking Runtime ✅ Passed Production code change in CmuxMainWindow.swift only sets window properties (isMovable=true, isMovableByWindowBackground=false) with no blocking/timing primitives. Test changes are test-only.
Cmux No Hacky Sleeps ✅ Passed Check scope is TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. PR contains only Swift code (CmuxMainWindow.swift, WindowAndDragTests.swift), explicitly excluded from this check.
Cmux Algorithmic Complexity ✅ Passed No algorithmic complexity violations found. Changes are O(1) operations (property assignments) or test-only code with no collection iterations or nested scans.
Cmux Swift Concurrency ✅ Passed Changes introduce no legacy async patterns (DispatchQueue.global, Combine, completion handlers, fire-and-forget Tasks). Uses AppKit callbacks and @MainActor appropriately.
Cmux Swift @Concurrent ✅ Passed No @concurrent annotation violations found. Changes are synchronous functions marked with @MainActor that only set simple window properties, requiring no @concurrent annotations.
Cmux Swift File And Package Boundaries ✅ Passed Focused Swish bug fix: +6 lines to 101-line AppKit glue file, +11 lines to test file. No new oversized files, no mixed responsibilities, inherent app-target code.
Cmux Swift Logging ✅ Passed PR contains no logging violations: production code has no print/debugPrint/dump/NSLog/Logger, test code only uses XCTest assertions.
Cmux User-Facing Error Privacy ✅ Passed Changes only include developer comments and test code. No user-facing errors, sensitive data, credentials, or prohibited information exposed.
Cmux Full Internationalization ✅ Passed PR changes are limited to developer-only comments in CmuxMainWindow.swift and test-only code in WindowAndDragTests.swift. No user-facing text requiring localization was added or modified.
Cmux Swiftui State Layout ✅ Passed Changes are AppKit-only NSWindow configuration with no new SwiftUI state patterns, layout issues, or render-time mutations.
Cmux Architecture Rethink ✅ Passed PR contains straightforward correctness fix with clear state ownership (NSWindow.isMovable), explicit invariant documentation, and no timing/dispatch patterns or architectural debt.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR modifies drag behavior configuration in CmuxMainWindow.swift and adds a test-only fixture window in cmuxTests, which is explicitly allowed by the rule's permitted cases.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR changes configureCmuxMainWindowDragBehavior to set isMovable = true (previously false) while keeping isMovableByWindowBackground = false, so Accessibility-based window managers like Swish can recognize cmux windows as movable and attach titlebar gestures without enabling background-drag on app content.

  • The existing mouseDownCanMoveWindow = false override on MainWindowHostingView already prevents any view-initiated titlebar drag, so the new isMovable = true baseline exposes only the accessibility metadata Swish needs and does not unlock a new user-visible drag path.
  • The move-suppression sequence (beginWindowMoveSuppressionSequence / finishWindowMoveSuppressionSequence) correctly saves true as previousMovableState and restores it after tab and folder drags, preserving the Swish-compatible state end-to-end.
  • The unit test is updated to reflect the new contract: withTemporaryWindowMovableEnabled now returns true (a no-op on an already-movable window) and the post-call isMovable assertion flips to XCTAssertTrue.

Confidence Score: 5/5

Safe to merge. The one-line production change is well-scoped and the existing move-suppression and mouseDownCanMoveWindow mechanisms already prevent unintended window movement.

The change is minimal: a single boolean flip in configureCmuxMainWindowDragBehavior. The three complementary guards — isMovableByWindowBackground = false, mouseDownCanMoveWindow = false on NSHostingView, and the beginWindowMoveSuppressionSequence / finishWindowMoveSuppressionSequence pair — correctly preserve the tab-safe drag contract with the new true baseline. The test is updated to match and directly covers all mutated assertions.

No files require special attention.

Important Files Changed

Filename Overview
Sources/App/CmuxMainWindow.swift Flips isMovable from false to true while keeping isMovableByWindowBackground = false; adds an explanatory comment. The invariant is sound: MouseDownCanMoveWindow = false on NSHostingView already blocks native titlebar dragging, and the move-suppression sequence correctly saves/restores the new true baseline around tab and folder drags.
cmuxTests/WindowAndDragTests.swift Test updated to match new contract: asserts isMovable == true after configureCmuxMainWindowDragBehavior, updates withTemporaryWindowMovableEnabled expectations from false to true, and adds an explicit assertion for isMovableByWindowBackground == false.

Reviews (1): Last reviewed commit: "fix: keep main windows movable for Swish" | Re-trigger Greptile

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.

Swish (window management app) gestures don't work on cmux windows

1 participant