Repository navigation
Fix Cmd+Tab activation ordering (#744) - #766
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded tracking for the last active main window and implemented helper methods to determine and bring the appropriate main window to the front when the application is activated, integrating this logic with app activation events and window focus notifications. Changes
Sequence DiagramsequenceDiagram
participant User
participant System as macOS System
participant AppDelegate
participant WindowManager as Window Manager
participant MainWindow as Main Window
User->>System: Press Cmd+Tab to switch to app
System->>AppDelegate: applicationDidBecomeActive()
AppDelegate->>WindowManager: bringMainWindowToFrontOnActivationIfNeeded()
WindowManager->>WindowManager: frontmostKnownMainWindow()
WindowManager->>WindowManager: Check lastActiveMainWindow, then keyWindow
WindowManager-->>MainWindow: Identify appropriate main window
MainWindow->>MainWindow: orderFrontRegardless()
MainWindow->>User: Window brought to foreground
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AppDelegate.swift (1)
1707-1728:⚠️ Potential issue | 🔴 CriticalMove activation fronting out of unread-notification guards.
At Line 1727,
bringMainWindowToFrontOnActivationIfNeeded()only runs when unread notifications exist. In normal activations (no unread), the method exits early and never restores front/key ordering, which can leave the Cmd+Tab issue unresolved.💡 Proposed fix
func applicationDidBecomeActive(_ notification: Notification) { sentryBreadcrumb("app.didBecomeActive", category: "lifecycle", data: [ "tabCount": tabManager?.tabs.count ?? 0 ]) @@ if TelemetrySettings.enabledForCurrentLaunch && !isRunningUnderXCTest(env) { PostHogAnalytics.shared.trackDailyActive(reason: "didBecomeActive") PostHogAnalytics.shared.trackHourlyActive(reason: "didBecomeActive") } + bringMainWindowToFrontOnActivationIfNeeded() + guard let tabManager, let notificationStore else { return } guard let tabId = tabManager.selectedTabId else { return } let surfaceId = tabManager.focusedSurfaceId(for: tabId) guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId) else { return } @@ if let surfaceId, let tab = tabManager.tabs.first(where: { $0.id == tabId }) { tab.triggerNotificationFocusFlash(panelId: surfaceId, requiresSplit: false, shouldFocus: false) } notificationStore.markRead(forTabId: tabId, surfaceId: surfaceId) - bringMainWindowToFrontOnActivationIfNeeded() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 1707 - 1728, In applicationDidBecomeActive(_:), bringMainWindowToFrontOnActivationIfNeeded() is currently invoked after the unread-notification guards so it only runs when notifications exist; move the call so it runs for every activation (e.g., place a call to bringMainWindowToFrontOnActivationIfNeeded() immediately before the guard sequence that checks tabManager/notificationStore/tabId or at the top of the notification-handling block) so that bringMainWindowToFrontOnActivationIfNeeded() is executed regardless of notificationStore.hasUnreadNotification(forTabId:surfaceId:); keep the existing notification handling (tab.triggerNotificationFocusFlash and notificationStore.markRead) unchanged.
🧹 Nitpick comments (1)
Sources/AppDelegate.swift (1)
6790-6792: Track only main-terminal windows inlastActiveMainWindow(optional cleanup).At Line 6791, every key window updates
lastActiveMainWindow. Consider gating this to main terminal windows so the field remains semantically precise and deterministic.♻️ Suggested tweak
) { [weak self] note in guard let self, let window = note.object as? NSWindow else { return } - self.lastActiveMainWindow = window + if self.isMainTerminalWindow(window) { + self.lastActiveMainWindow = window + } self.setActiveMainWindow(window) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 6790 - 6792, The handler currently sets lastActiveMainWindow for every key window; restrict it to only main terminal windows by adding a predicate check (e.g. create a helper isMainTerminalWindow(_ window: NSWindow) that verifies whatever makes a window a "main terminal" — class/type of windowController, window.identifier, or its root view controller) and only call self.lastActiveMainWindow = window and self.setActiveMainWindow(window) when that predicate returns true; update the notification handling code around lastActiveMainWindow and setActiveMainWindow to use this helper so the field remains semantically precise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 1707-1728: In applicationDidBecomeActive(_:),
bringMainWindowToFrontOnActivationIfNeeded() is currently invoked after the
unread-notification guards so it only runs when notifications exist; move the
call so it runs for every activation (e.g., place a call to
bringMainWindowToFrontOnActivationIfNeeded() immediately before the guard
sequence that checks tabManager/notificationStore/tabId or at the top of the
notification-handling block) so that
bringMainWindowToFrontOnActivationIfNeeded() is executed regardless of
notificationStore.hasUnreadNotification(forTabId:surfaceId:); keep the existing
notification handling (tab.triggerNotificationFocusFlash and
notificationStore.markRead) unchanged.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 6790-6792: The handler currently sets lastActiveMainWindow for
every key window; restrict it to only main terminal windows by adding a
predicate check (e.g. create a helper isMainTerminalWindow(_ window: NSWindow)
that verifies whatever makes a window a "main terminal" — class/type of
windowController, window.identifier, or its root view controller) and only call
self.lastActiveMainWindow = window and self.setActiveMainWindow(window) when
that predicate returns true; update the notification handling code around
lastActiveMainWindow and setActiveMainWindow to use this helper so the field
remains semantically precise.
Greptile SummaryImproves Cmd+Tab window activation by setting regular activation policy on launch and tracking the last active main terminal window. When the app is activated (e.g., via Cmd+Tab or clicking a notification), the implementation now restores the most recently active main window using a fallback hierarchy: last active window → key window → main window → active tab manager's window → first visible window → any window.
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: 8b389e5 |
| queue: .main | ||
| ) { [weak self] note in | ||
| guard let self, let window = note.object as? NSWindow else { return } | ||
| self.lastActiveMainWindow = window |
There was a problem hiding this comment.
lastActiveMainWindow is set for any window that becomes key, not just main terminal windows. If a non-main window (settings, about, etc.) becomes key, lastActiveMainWindow will point to it, though frontmostKnownMainWindow() filters it out later.
| self.lastActiveMainWindow = window | |
| if isMainTerminalWindow(window) { | |
| self.lastActiveMainWindow = window | |
| } |
…#744) (manaflow-ai#766)" (manaflow-ai#827) This reverts commit 30671a5.
* Revert "Fix Cmd+Tab activation ordering for cmux windows (manaflow-ai#744) (manaflow-ai#766)" This reverts commit 30671a5. * Fix debug Ghostty theme loading fallback
…#744) (manaflow-ai#766)" This reverts commit 30671a5.
…#744) (manaflow-ai#766)" This reverts commit 708b34f.
Summary
Closes Cmd+tab doesn't always work #744
Summary by CodeRabbit
Release Notes