Fix close shortcuts targeting original window

[austinywang]

Summary

• Add a regression test for close shortcuts when key-event window metadata points at the original window while a newer window is focused.
• Fix close shortcut routing so focused-window close actions target the focused main window.

Testing

• Not run locally per repository/task instructions; CI will run the test suite.

Fixes #4614

---

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

---

Note

Medium Risk
Changes global keyboard shortcut routing for close actions (Cmd+W, Cmd+Shift+W, close window), which can affect multi-window behavior and regress edge cases involving auxiliary windows.

Overview
Fixes close-shortcut routing so focused-window close actions prefer NSApp.keyWindow/NSApp.mainWindow over potentially stale NSEvent window metadata, and syncs the active main-window context before executing close operations.

This introduces focused close helpers (mainWindowForFocusedCloseShortcut, tabManagerForFocusedCloseShortcut, auxiliaryWindowForFocusedCloseShortcut) to consistently decide between auxiliary windows (e.g., browser popups) and the focused main window’s TabManager.

Adds regression tests ensuring Cmd+W (close tab/panel) and Cmd+Shift+W (close workspace) act on the currently focused window even when the key event’s windowNumber points at an older window.

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

---

Summary by cubic

Route close shortcuts to the focused window so tabs, workspaces, and windows close in the right place, even when the key event has stale window metadata. Adds stricter regression tests for Cmd+W and Cmd+Shift+W. Fixes #4614.

• Bug Fixes
  • Close Tab/Workspace/Window now prefer the focused keyWindow/mainWindow and resync app context; only fall back to the event’s window.
  • Auxiliary windows (e.g., browser popups) take ownership of close; otherwise actions route to the focused window’s TabManager.

^{Written for commit 12056b5. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• Bug Fixes

  • Cmd+W and Cmd+Shift+W now consistently target the currently focused window (avoids acting on the wrong window when multiple windows/panels are open).
  • Close-Window shortcut now resolves the intended window more reliably and provides a subtle alert if no target is available.

• Tests

  • Added regression tests to ensure close shortcuts continue to target the focused window in stale-window scenarios.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Canceled		May 23, 2026 4:04am
cmux-staging	Building	Preview, Comment	May 23, 2026 4:04am

[coderabbitai]

📝 Walkthrough

Walkthrough

Cmd+W and Cmd+Shift+W close shortcuts are updated to resolve targets by preferring the current focused cmux terminal window (via NSApp.keyWindow/NSApp.mainWindow) and fall back to event-derived metadata only when necessary. Three AppDelegate helpers implement this resolution, three shortcut paths are refactored to use them, and tests validate behavior when event-window metadata is stale.

Changes

Focused window close shortcut routing

Layer / File(s)	Summary
Focused-close shortcut helpers Sources/AppDelegate.swift	New methods mainWindowForFocusedCloseShortcut(event:), tabManagerForFocusedCloseShortcut(event:), and auxiliaryWindowForFocusedCloseShortcut(event:) resolve the correct target for close shortcuts by preferring NSApp's current focused cmux terminal window over event-derived metadata.
Close shortcut routing using focused helpers Sources/AppDelegate.swift	Close Tab, Close Workspace, and Close Window shortcut paths now use the focused-close helpers. Close Tab detects and closes auxiliary windows before falling back to workspace/panel closure. Close Window synchronizes active context to the resolved main window and beeps/returns early if resolution fails.
Stale event-window metadata test coverage cmuxTests/AppDelegateShortcutRoutingTests.swift	Two new test methods plus a shared assertion helper simulate stale AppDelegate tab-manager metadata and verify that Cmd+W and Cmd+Shift+W act on the currently focused window's tab/workspace state, not the stale original window.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• manaflow-ai/cmux#4442: Updates AppDelegate/shortcut-routing test scaffolding and state cleanup in AppDelegateShortcutRoutingTests.swift, overlapping directly on shortcut-routing test infrastructure.

Poem

🐰 I hopped to the front where the key is in view,

Not the old shadow that once I knew.
Cmd+W now listens to whichever is bright,
Focused and ready — it closes the right. ✨

🚥 Pre-merge checks | ✅ 16 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (16 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main fix: addressing close shortcuts that were targeting the original window instead of the focused one.
Linked Issues check	✅ Passed	The PR fully addresses issue #4614 requirements: fixes close-shortcut routing to target the focused window, introduces focused-close helpers, adds regression tests for Cmd+W and Cmd+Shift+W with stale window metadata, and ensures correct window selection logic.
Out of Scope Changes check	✅ Passed	All changes directly address the linked issue #4614: implementation fixes for close-shortcut routing and corresponding regression tests are within scope.
Cmux Swift Actor Isolation	✅ Passed	New private methods added to @MainActor AppDelegate inherit main actor isolation and operate only on UI types; no problematic types or unsafely shared Sendable types.
Cmux Swift Blocking Runtime	✅ Passed	No blocking primitives in production code—only synchronous window resolution helpers. Test code uses RunLoop.main.run(until:), which is allowed test-only scaffolding.
Cmux No Hacky Sleeps	✅ Passed	Rule applies only to TypeScript, JavaScript, shell, or build/runtime scripts. PR modifies only Swift files (AppDelegate.swift), which are explicitly excluded from this check.
Cmux Swift Concurrency	✅ Passed	PR introduces only synchronous code for close shortcut routing; no new DispatchQueue, Task, Combine, or completion-handler patterns detected.
Cmux Swift @Concurrent	✅ Passed	All new Swift functions comply with concurrent-annotation rules: synchronous MainActor-isolated instance methods with no @concurrent or async/await patterns.
Cmux Swift File And Package Boundaries	✅ Passed	Adds 37 net lines to oversized AppDelegate (below 250-line limit). Small helper functions extract existing shortcut logic. Focused bug fix matching allowed case criteria.
Cmux Swift Logging	✅ Passed	PR contains only debug-only cmuxDebugLog calls guarded by #if DEBUG; no violations of swift-logging.md rules for production code.
Cmux User-Facing Error Privacy	✅ Passed	PR adds only private internal logic functions and test code; no new user-facing error messages, alerts, or privacy-violating content detected.
Cmux Full Internationalization	✅ Passed	PR adds routing logic to AppDelegate.swift and tests. No new user-facing text; handlers call existing localized APIs. Tests are exempt from localization requirements.
Cmux Swiftui State Layout	✅ Passed	PR changes AppKit code (AppDelegate.swift) and test files only. No SwiftUI state patterns found: zero @Published, @Observable, @State, GeometryReader, or ObservableObject class definitions.
Cmux Architecture Rethink	✅ Passed	Correctness fix inverting priority from stale event metadata to NSApp.keyWindow/mainWindow; centralizes close-shortcut routing; no timing/blocking patterns; includes regression tests.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR modifies close-shortcut routing in AppDelegate (no new windows) and adds regression tests with test fixtures (cmux.main.* identifiers). Test windows are explicitly allowed per rule.
Description check	✅ Passed	PR description covers the essential summary (what changed and why) and mentions testing approach, but lacks a demo video and complete checklist.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-4614-cmd-w-wrong-window

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR fixes a focus-routing bug where Cmd+W and Cmd+Shift+W could close content in a stale/original window when AppKit key-event window metadata lagged behind the actual focused window. Three focused-close helpers are introduced in AppDelegate (mainWindowForFocusedCloseShortcut, tabManagerForFocusedCloseShortcut, auxiliaryWindowForFocusedCloseShortcut) that prefer keyWindow/mainWindow over the event's window number, falling back to event metadata only when no focused terminal is found. Two targeted regression tests verify that both shortcuts route to the focused window even when the synthesized event carries the original window number.

Confidence Score: 5/5

Safe to merge — the routing change is well-scoped, the priority order (keyWindow → mainWindow → event fallback) is correct for all three close actions, and the regression tests directly cover the stale-metadata scenario that prompted the fix.

The three new helpers correctly prefer the actually focused window over AppKit event metadata. The auxiliary-window check in closeTab is expanded to include keyWindow and mainWindow before the event window, which is strictly better than the old code. The closeWorkspace and closeWindow handlers are simpler and more correct. No incorrect data can reach the close operations through the new paths. The regression tests verify the exact bug scenario end-to-end.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/AppDelegate.swift	Adds three focused-close routing helpers and updates closeTab, closeWorkspace, and closeWindow handlers to prefer the actually focused window over stale event window metadata. Logic is correct: keyWindow → mainWindow → event fallback priority, auxiliary windows checked via cmuxWindowShouldOwnCloseShortcut, and synchronizeActiveMainWindowContext called to realign app context before close actions.
cmuxTests/AppDelegateShortcutRoutingTests.swift	Adds two regression tests (Cmd+W and Cmd+Shift+W) and a shared helper assertCloseShortcutTargetsFocusedWindowWhenEventWindowMetadataIsStale. Properly restores tabManager and UserDefaults in defer, asserts both that the original window is untouched and that the focused window loses the expected tab/workspace.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Close shortcut fired\nCmd+W / Cmd+Shift+W / Cmd+Q] --> B{matchConfiguredShortcut}

    B -->|closeTab| CT_AUX[auxiliaryWindowForFocusedCloseShortcut]
    CT_AUX --> CT_AUX1{keyWindow is\nauxiliary?}
    CT_AUX1 -->|Yes| CLOSE_AUX[performClose on aux window]
    CT_AUX1 -->|No| CT_AUX2{mainWindow is\nauxiliary?}
    CT_AUX2 -->|Yes| CLOSE_AUX
    CT_AUX2 -->|No| CT_AUX3{event window is\nauxiliary?}
    CT_AUX3 -->|Yes| CLOSE_AUX
    CT_AUX3 -->|No| CT_MGR[routedManager from\ntabManagerForFocusedCloseShortcut]
    CT_MGR --> CT_PANEL[closeCurrentPanelWithConfirmation]

    B -->|closeWorkspace| CW_MGR[tabManagerForFocusedCloseShortcut]
    CW_MGR --> CW_CLOSE[closeCurrentWorkspaceWithConfirmation]

    B -->|closeWindow| CW_WIN[mainWindowForFocusedCloseShortcut]
    CW_WIN --> CW_SYNC[synchronizeActiveMainWindowContext]
    CW_SYNC --> CW_CONF[closeWindowWithConfirmation]

    subgraph mainWindowForFocusedCloseShortcut
        MF1{keyWindow is\nmain terminal?}
        MF1 -->|Yes| MF_KEY[return keyWindow]
        MF1 -->|No| MF2{mainWindow is\nmain terminal?}
        MF2 -->|Yes| MF_MAIN[return mainWindow]
        MF2 -->|No| MF_EVT[mainWindowForShortcutEvent - event fallback]
    end

    CT_MGR -.-> MF1
    CW_MGR -.-> MF1
    CW_WIN -.-> MF1

Loading

_{Reviews (2): Last reviewed commit: "test: tighten close shortcut routing ass..." | Re-trigger Greptile}