Repository navigation
Raise all cmux windows on auth callback, Settings on top - #3044
lawrencecchen wants to merge 4 commits into
Conversation
Previously application(_:open:) only routed the cmux://auth-callback URL to AuthManager.handleCallbackURL. That flipped the Settings row's UI state but left the rest of the app buried — the browser remained frontmost, and any workspace windows stayed behind whatever app the user was in before clicking "Return to cmux". After handleCallbackURL resolves we now: 1. NSApp.activate(ignoringOtherApps: true) to bring cmux to the foreground. 2. Iterate NSApp.orderedWindows.reversed(), calling orderFront on each non-Settings visible window so their relative z-order is preserved (back windows stay behind front windows). 3. makeKeyAndOrderFront on the Settings window so the just-signed-in UI lands on top.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAppDelegate.application(_:open:) now activates the app synchronously before processing auth-callback URLs, invokes AuthManager.shared.handleCallbackURL(url) for each URL, and after each callback completion re-activates the app and reorders visible windows, ensuring Settings is brought to key/front if visible. (34 words) Changes
Sequence Diagram(s)sequenceDiagram
participant System as System (open URL)
participant App as AppDelegate
participant Auth as AuthManager
participant Win as WindowManager (NSApp / NSRunningApplication)
System->>App: application(_:open: URLs)
App->>Win: focusAppForAuthCallback()\n(NSApp.activate + current.activate)
loop for each auth-callback URL
App->>Auth: handleCallbackURL(url) (async)
Auth-->>App: completion (success/failure)
App->>Win: raiseWindowsAfterAuthCallback()\n(activate + reorder visible windows, bring Settings key)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes a window-focus regression after the Stack Auth sign-in callback: The window-ordering logic is correct — snapshotting Confidence Score: 5/5Safe to merge — logic is correct, all findings are P2 style suggestions. The window z-order algorithm is sound, the Settings window nil/invisible cases are handled correctly, and @mainactor ensures no concurrency issues. The two P2 notes (style inconsistency with NSApplication.shared vs NSApp, and focus-on-error behavior) don't block correctness. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant AppDelegate
participant AuthManager
participant NSApp
Browser->>AppDelegate: application(_:open:) cmux://auth-callback
AppDelegate->>AppDelegate: filter authCallbacks
AppDelegate->>AuthManager: handleCallbackURL(url) [async]
AuthManager-->>AppDelegate: success / throws
AppDelegate->>AppDelegate: focusAppAfterAuthCallback()
AppDelegate->>NSApp: activate(ignoringOtherApps: true)
loop visible.reversed() excluding settingsWindow
AppDelegate->>NSApp: orderFront(nil) — workspace windows (back to front)
end
AppDelegate->>NSApp: makeKeyAndOrderFront(nil) — Settings window
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // cmux workspace windows buried behind other apps. Activate the | ||
| // app, re-front every visible cmux window in back-to-front order | ||
| // to preserve relative z-order, then raise Settings on top. | ||
| NSApplication.shared.activate(ignoringOtherApps: true) |
There was a problem hiding this comment.
NSApplication.shared vs NSApp style inconsistency
The rest of the file (lines 3585 and 7258) uses NSApp.activate(ignoringOtherApps: true); this new call uses NSApplication.shared. They're equivalent, but the inconsistency is minor noise.
| NSApplication.shared.activate(ignoringOtherApps: true) | |
| NSApp.activate(ignoringOtherApps: true) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cb77c1cdb
ℹ️ 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".
| } catch { | ||
| NSLog("auth.callback failed: %@", "\(error)") | ||
| } | ||
| self.focusAppAfterAuthCallback() |
There was a problem hiding this comment.
Call to focusAppAfterAuthCallback() currently runs even when handleCallbackURL throws, so expired/invalid cmux://auth-callback links still activate the app and raise all windows. In practice this creates unexpected focus stealing for failed sign-in attempts (or any stray callback URL) and is not tied to a successful auth state update; moving the focus call into the success path avoids that regression.
Useful? React with 👍 / 👎.
Previous attempt still left workspace windows buried because NSApp.activate(ignoringOtherApps:) is cooperative on macOS 14+ and only guarantees the key window comes forward. Switched to NSRunningApplication.current.activate(options: [.activateAllWindows]) which is the supported API for "bring every window of this app along," and upgraded the per-window call to orderFrontRegardless so the re-ordering takes effect even if cooperative activation hasn't completed yet.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 6783-6799: Add DEBUG-only dlog() calls in
focusAppAfterAuthCallback() to trace the focus-repair sequence: wrap logs in `#if`
DEBUG / `#endif` and emit messages before/after
NSApplication.shared.activate(ignoringOtherApps:), log the list of visible
windows (from visible variable) and each window being ordered via
window.orderFront(nil), and log when SettingsWindowController.shared.window is
made key/front; use clear messages referencing the function name and window
identities to aid debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| private func focusAppAfterAuthCallback() { | ||
| // When the sign-in deeplink returns through the browser only the | ||
| // Settings window gets re-fronted by default, leaving any regular | ||
| // cmux workspace windows buried behind other apps. Activate the | ||
| // app, re-front every visible cmux window in back-to-front order | ||
| // to preserve relative z-order, then raise Settings on top. | ||
| NSApplication.shared.activate(ignoringOtherApps: true) | ||
|
|
||
| let settingsWindow = SettingsWindowController.shared.window | ||
| let visible = NSApp.orderedWindows.filter { $0.isVisible } | ||
| for window in visible.reversed() where window !== settingsWindow { | ||
| window.orderFront(nil) | ||
| } | ||
| if let settingsWindow, settingsWindow.isVisible { | ||
| settingsWindow.makeKeyAndOrderFront(nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add DEBUG logging for the auth focus repair path.
This helper mutates app/window focus but doesn’t emit a focus debug event, which makes auth-callback focus regressions harder to diagnose.
Proposed logging addition
private func focusAppAfterAuthCallback() {
+#if DEBUG
+ let beforeKey = NSApp.keyWindow?.windowNumber ?? -1
+ let beforeMain = NSApp.mainWindow?.windowNumber ?? -1
+ dlog(
+ "auth.callback.focus.begin keyWin=\(beforeKey) mainWin=\(beforeMain) " +
+ "ordered=\(NSApp.orderedWindows.map { "\($0.windowNumber):\($0.identifier?.rawValue ?? "nil"):\($0.isVisible ? 1 : 0)" }.joined(separator: ","))"
+ )
+#endif
// When the sign-in deeplink returns through the browser only the
// Settings window gets re-fronted by default, leaving any regular
// cmux workspace windows buried behind other apps. Activate the
// app, re-front every visible cmux window in back-to-front order
// to preserve relative z-order, then raise Settings on top.
NSApplication.shared.activate(ignoringOtherApps: true)
let settingsWindow = SettingsWindowController.shared.window
let visible = NSApp.orderedWindows.filter { $0.isVisible }
for window in visible.reversed() where window !== settingsWindow {
window.orderFront(nil)
}
if let settingsWindow, settingsWindow.isVisible {
settingsWindow.makeKeyAndOrderFront(nil)
}
+#if DEBUG
+ dlog(
+ "auth.callback.focus.end keyWin=\(NSApp.keyWindow?.windowNumber ?? -1) " +
+ "mainWin=\(NSApp.mainWindow?.windowNumber ?? -1) settingsVisible=\((settingsWindow?.isVisible == true) ? 1 : 0)"
+ )
+#endif
}As per coding guidelines, “All debug events (keys, mouse, focus, splits, tabs) must be logged to the unified debug log using the dlog() function and wrapped in #if DEBUG / #endif preprocessor directives.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private func focusAppAfterAuthCallback() { | |
| // When the sign-in deeplink returns through the browser only the | |
| // Settings window gets re-fronted by default, leaving any regular | |
| // cmux workspace windows buried behind other apps. Activate the | |
| // app, re-front every visible cmux window in back-to-front order | |
| // to preserve relative z-order, then raise Settings on top. | |
| NSApplication.shared.activate(ignoringOtherApps: true) | |
| let settingsWindow = SettingsWindowController.shared.window | |
| let visible = NSApp.orderedWindows.filter { $0.isVisible } | |
| for window in visible.reversed() where window !== settingsWindow { | |
| window.orderFront(nil) | |
| } | |
| if let settingsWindow, settingsWindow.isVisible { | |
| settingsWindow.makeKeyAndOrderFront(nil) | |
| } | |
| } | |
| private func focusAppAfterAuthCallback() { | |
| `#if` DEBUG | |
| let beforeKey = NSApp.keyWindow?.windowNumber ?? -1 | |
| let beforeMain = NSApp.mainWindow?.windowNumber ?? -1 | |
| dlog( | |
| "auth.callback.focus.begin keyWin=\(beforeKey) mainWin=\(beforeMain) " + | |
| "ordered=\(NSApp.orderedWindows.map { "\($0.windowNumber):\($0.identifier?.rawValue ?? "nil"):\($0.isVisible ? 1 : 0)" }.joined(separator: ","))" | |
| ) | |
| `#endif` | |
| // When the sign-in deeplink returns through the browser only the | |
| // Settings window gets re-fronted by default, leaving any regular | |
| // cmux workspace windows buried behind other apps. Activate the | |
| // app, re-front every visible cmux window in back-to-front order | |
| // to preserve relative z-order, then raise Settings on top. | |
| NSApplication.shared.activate(ignoringOtherApps: true) | |
| let settingsWindow = SettingsWindowController.shared.window | |
| let visible = NSApp.orderedWindows.filter { $0.isVisible } | |
| for window in visible.reversed() where window !== settingsWindow { | |
| window.orderFront(nil) | |
| } | |
| if let settingsWindow, settingsWindow.isVisible { | |
| settingsWindow.makeKeyAndOrderFront(nil) | |
| } | |
| `#if` DEBUG | |
| dlog( | |
| "auth.callback.focus.end keyWin=\(NSApp.keyWindow?.windowNumber ?? -1) " + | |
| "mainWin=\(NSApp.mainWindow?.windowNumber ?? -1) settingsVisible=\((settingsWindow?.isVisible == true) ? 1 : 0)" | |
| ) | |
| `#endif` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 6783 - 6799, Add DEBUG-only dlog()
calls in focusAppAfterAuthCallback() to trace the focus-repair sequence: wrap
logs in `#if` DEBUG / `#endif` and emit messages before/after
NSApplication.shared.activate(ignoringOtherApps:), log the list of visible
windows (from visible variable) and each window being ordered via
window.orderFront(nil), and log when SettingsWindowController.shared.window is
made key/front; use clear messages referencing the function name and window
identities to aid debugging.
My Bash-driven repro surfaced the actual reason Settings came up
alone. On macOS 14+, NSApp.activate() and NSRunningApplication.
current.activate(options:) are cooperative: the OS only grants
them if the call fires synchronously while the user-event context
is still active. The original code routed activation through a
Task { @mainactor in ... }, which hops to a later run loop tick
and loses that context, so activation silently no-op'd and only
the subsequent makeKeyAndOrderFront on the Settings window
managed to pull cmux to the foreground.
Split the handler into two phases:
- focusAppForAuthCallback() fires synchronously inside
application(_:open:), within the URL-event frame, so activation
is granted.
- raiseWindowsAfterAuthCallback() runs after the async token
exchange and re-orders workspace windows back-to-front with
makeKeyAndOrderFront on the first (forces activation as a
secondary guarantee) + orderFrontRegardless on the rest, then
makeKeyAndOrderFront on Settings.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
6793-6824:⚠️ Potential issue | 🟡 MinorAdd DEBUG logging around the auth focus repair path.
These helpers mutate app/window focus but still don’t emit focus diagnostics, making regressions in this path hard to debug.
🔎 Proposed logging addition
private func focusAppForAuthCallback() { +#if DEBUG + dlog( + "auth.callback.focus.sync.begin " + + "key={\(debugWindowToken(NSApp.keyWindow))} main={\(debugWindowToken(NSApp.mainWindow))}" + ) +#endif NSApp.activate() NSRunningApplication.current.activate(options: [.activateAllWindows]) +#if DEBUG + dlog( + "auth.callback.focus.sync.end active=\(NSApp.isActive ? 1 : 0) " + + "key={\(debugWindowToken(NSApp.keyWindow))} main={\(debugWindowToken(NSApp.mainWindow))}" + ) +#endif } @@ private func raiseWindowsAfterAuthCallback() { +#if DEBUG + dlog( + "auth.callback.raise.begin " + + "key={\(debugWindowToken(NSApp.keyWindow))} main={\(debugWindowToken(NSApp.mainWindow))}" + ) +#endif // Second activate to catch the case where the activation request @@ let settingsWindow = SettingsWindowController.shared.window let visible = NSApp.orderedWindows.filter { $0.isVisible } +#if DEBUG + dlog( + "auth.callback.raise.visible " + + visible.map { "{\(debugWindowToken($0))}" }.joined(separator: " ") + ) +#endif @@ let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow } for (idx, window) in workspaceWindows.enumerated() { +#if DEBUG + dlog("auth.callback.raise.workspace idx=\(idx) window={\(debugWindowToken(window))}") +#endif if idx == 0 { window.makeKeyAndOrderFront(nil) } else { window.orderFrontRegardless() } } if let settingsWindow, settingsWindow.isVisible { +#if DEBUG + dlog("auth.callback.raise.settings window={\(debugWindowToken(settingsWindow))}") +#endif settingsWindow.makeKeyAndOrderFront(nil) } +#if DEBUG + dlog( + "auth.callback.raise.end active=\(NSApp.isActive ? 1 : 0) " + + "key={\(debugWindowToken(NSApp.keyWindow))} main={\(debugWindowToken(NSApp.mainWindow))}" + ) +#endif }As per coding guidelines, “All debug events (keys, mouse, focus, splits, tabs) must be logged to the unified debug log using the
dlog()function and wrapped in#if DEBUG/#endifpreprocessor directives.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 6793 - 6824, Add debug logging around the auth focus repair path: inside focusAppForAuthCallback() and at the start and key steps of raiseWindowsAfterAuthCallback() (before/after the NSApp.activate()/NSRunningApplication.current.activate calls, before iterating workspaceWindows and when making the first window key, and before/after bringing settingsWindow forward) call dlog() with concise context messages; wrap all dlog() calls in `#if` DEBUG / `#endif` to comply with guidelines and reference SettingsWindowController.shared.window when logging which window is being focused. Ensure messages identify the method name and the window (or lack thereof) so focus transitions can be traced in the unified debug log.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 6813-6822: The current logic reverses NSApp.orderedWindows then
treats the first element as the frontmost, making the originally backmost window
key; change the selection/ordering so the original frontmost becomes key.
Specifically, adjust how workspaceWindows is built (stop using
visible.reversed() or instead pick the last element as the key) so that in the
loop over workspaceWindows the element handled when idx == 0 is the original
frontmost window; keep the rest using orderFrontRegardless(), and continue to
skip settingsWindow (refer to visible, workspaceWindows, settingsWindow,
makeKeyAndOrderFront, orderFrontRegardless).
---
Duplicate comments:
In `@Sources/AppDelegate.swift`:
- Around line 6793-6824: Add debug logging around the auth focus repair path:
inside focusAppForAuthCallback() and at the start and key steps of
raiseWindowsAfterAuthCallback() (before/after the
NSApp.activate()/NSRunningApplication.current.activate calls, before iterating
workspaceWindows and when making the first window key, and before/after bringing
settingsWindow forward) call dlog() with concise context messages; wrap all
dlog() calls in `#if` DEBUG / `#endif` to comply with guidelines and reference
SettingsWindowController.shared.window when logging which window is being
focused. Ensure messages identify the method name and the window (or lack
thereof) so focus transitions can be traced in the unified debug log.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow } | ||
| for (idx, window) in workspaceWindows.enumerated() { | ||
| if idx == 0 { | ||
| window.makeKeyAndOrderFront(nil) | ||
| } else { | ||
| window.orderFrontRegardless() | ||
| } | ||
| } | ||
| if let settingsWindow, settingsWindow.isVisible { | ||
| settingsWindow.makeKeyAndOrderFront(nil) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd Sources && sed -n '6800,6835p' AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 1726
🏁 Script executed:
rg "didBecomeKeyNotification" --context 5 -A 5Repository: manaflow-ai/cmux
Length of output: 3840
🏁 Script executed:
rg "let visible" Sources/AppDelegate.swift -B 3 -A 3Repository: manaflow-ai/cmux
Length of output: 2980
Make the originally frontmost workspace key, not the backmost one.
NSApp.orderedWindows returns windows front-to-back. After visible.reversed(), the loop's first iteration (idx == 0) makes workspaceWindows[0]—the originally backmost window—the key window via makeKeyAndOrderFront(). The didBecomeKeyNotification observer then calls setActiveMainWindow(), leaving the app context pointing to the wrong workspace. Later orderFrontRegardless() calls restore visual z-order but don't fix the active context state.
🐛 Proposed fix
let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow }
- for (idx, window) in workspaceWindows.enumerated() {
- if idx == 0 {
- window.makeKeyAndOrderFront(nil)
- } else {
- window.orderFrontRegardless()
- }
+ for window in workspaceWindows {
+ window.orderFrontRegardless()
+ }
+ if let frontmostWorkspaceWindow = workspaceWindows.last {
+ frontmostWorkspaceWindow.makeKeyAndOrderFront(nil)
}
if let settingsWindow, settingsWindow.isVisible {
settingsWindow.makeKeyAndOrderFront(nil)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow } | |
| for (idx, window) in workspaceWindows.enumerated() { | |
| if idx == 0 { | |
| window.makeKeyAndOrderFront(nil) | |
| } else { | |
| window.orderFrontRegardless() | |
| } | |
| } | |
| if let settingsWindow, settingsWindow.isVisible { | |
| settingsWindow.makeKeyAndOrderFront(nil) | |
| let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow } | |
| for window in workspaceWindows { | |
| window.orderFrontRegardless() | |
| } | |
| if let frontmostWorkspaceWindow = workspaceWindows.last { | |
| frontmostWorkspaceWindow.makeKeyAndOrderFront(nil) | |
| } | |
| if let settingsWindow, settingsWindow.isVisible { | |
| settingsWindow.makeKeyAndOrderFront(nil) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 6813 - 6822, The current logic
reverses NSApp.orderedWindows then treats the first element as the frontmost,
making the originally backmost window key; change the selection/ordering so the
original frontmost becomes key. Specifically, adjust how workspaceWindows is
built (stop using visible.reversed() or instead pick the last element as the
key) so that in the loop over workspaceWindows the element handled when idx == 0
is the original frontmost window; keep the rest using orderFrontRegardless(),
and continue to skip settingsWindow (refer to visible, workspaceWindows,
settingsWindow, makeKeyAndOrderFront, orderFrontRegardless).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
6806-6816:⚠️ Potential issue | 🟠 MajorKeep the originally frontmost workspace as the active context.
visible.reversed()makesidx == 0the originally backmost workspace, so Line 6810 can make the wrong main window key and updateAppDelegate.tabManagerto that workspace before Settings is raised. Prefer ordering all workspaces, then make the original frontmost workspace key before raising Settings. This was already flagged in an earlier review.🐛 Proposed fix
let settingsWindow = SettingsWindowController.shared.window let visible = NSApp.orderedWindows.filter { $0.isVisible } let workspaceWindows = visible.reversed().filter { $0 !== settingsWindow } - for (idx, window) in workspaceWindows.enumerated() { - if idx == 0 { - window.makeKeyAndOrderFront(nil) - } else { - window.orderFrontRegardless() - } + for window in workspaceWindows { + window.orderFrontRegardless() + } + if let frontmostWorkspaceWindow = workspaceWindows.last { + frontmostWorkspaceWindow.makeKeyAndOrderFront(nil) } if let settingsWindow, settingsWindow.isVisible { settingsWindow.makeKeyAndOrderFront(nil) }Verify the AppKit ordering assumption if needed:
What order does NSApplication.orderedWindows return in AppKit, and does NSWindow.makeKeyAndOrderFront trigger didBecomeKey notifications?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 6806 - 6816, The current logic reverses NSApp.orderedWindows so idx==0 becomes the original backmost window, causing the wrong workspace to be made key and assigned to AppDelegate.tabManager; change the sequence in the block handling visible/workspaceWindows so you do not reverse the visible array: build workspaceWindows from visible.filter { $0 !== settingsWindow } in original order, call orderFrontRegardless() on all but the original frontmost, then explicitly call makeKeyAndOrderFront(nil) on the original frontmost window (and update AppDelegate.tabManager after that) before handling settingsWindow.makeKeyAndOrderFront(nil); reference the visible, workspaceWindows, settingsWindow variables and the makeKeyAndOrderFront/orderFrontRegardless calls when implementing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/AppDelegate.swift`:
- Around line 6806-6816: The current logic reverses NSApp.orderedWindows so
idx==0 becomes the original backmost window, causing the wrong workspace to be
made key and assigned to AppDelegate.tabManager; change the sequence in the
block handling visible/workspaceWindows so you do not reverse the visible array:
build workspaceWindows from visible.filter { $0 !== settingsWindow } in original
order, call orderFrontRegardless() on all but the original frontmost, then
explicitly call makeKeyAndOrderFront(nil) on the original frontmost window (and
update AppDelegate.tabManager after that) before handling
settingsWindow.makeKeyAndOrderFront(nil); reference the visible,
workspaceWindows, settingsWindow variables and the
makeKeyAndOrderFront/orderFrontRegardless calls when implementing.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:6812">
P2: Using `orderFrontRegardless()` here can leave the top workspace window non-key when Settings is not visible, causing keyboard focus to remain on a different window.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if idx == 0 { | ||
| window.makeKeyAndOrderFront(nil) | ||
| } else { | ||
| window.orderFrontRegardless() |
There was a problem hiding this comment.
P2: Using orderFrontRegardless() here can leave the top workspace window non-key when Settings is not visible, causing keyboard focus to remain on a different window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/AppDelegate.swift, line 6812:
<comment>Using `orderFrontRegardless()` here can leave the top workspace window non-key when Settings is not visible, causing keyboard focus to remain on a different window.</comment>
<file context>
@@ -6780,18 +6788,29 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
+ if idx == 0 {
+ window.makeKeyAndOrderFront(nil)
+ } else {
+ window.orderFrontRegardless()
+ }
}
</file context>
| window.orderFrontRegardless() | |
| window.makeKeyAndOrderFront(nil) |
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Summary
After completing the Stack Auth sign-in and clicking "Return to cmux" on the browser handoff page, only the Settings window got focused. Any workspace windows stayed buried behind whatever app was in front before — the flow surfaced the Settings row UI update, but the rest of cmux was lost.
AppDelegate.application(_:open:)now callsfocusAppAfterAuthCallback()oncehandleCallbackURLresolves:NSApp.activate(ignoringOtherApps: true)brings cmux to the foreground.NSApp.orderedWindows.reversed(), callingorderFront(nil)on each visible non-Settings window so their relative z-order is preserved.makeKeyAndOrderFront(nil)on the Settings window so the just-signed-in row lands on top.Test plan
Summary by cubic
Synchronously activate cmux on the Stack Auth callback, then restore all windows with workspace order preserved and Settings on top. Fixes cases where only Settings came forward and other windows stayed behind other apps.
application(_:open:)usingNSApp.activate()andNSRunningApplication.current.activate(options: [.activateAllWindows])so macOS 14+ grants focus.handleCallbackURL, re-order visible windows back-to-front (makeKeyAndOrderFrontfirst workspace,orderFrontRegardlessothers) and make Settings key and front.Written for commit 249c4a6. Summary will update on new commits.
Summary by CodeRabbit