Fix CLI commands briefly stealing focus#2464
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThese changes add focus suppression guards to prevent CLI socket commands from stealing focus. When socket command activation suppression is enabled and focus mutations are disallowed, window activation and ordering operations are bypassed or early-return, while submodule dependencies are also updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: c778946c75
ℹ️ 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".
| !TerminalController.socketCommandAllowsInAppFocusMutations() { | ||
| setActiveMainWindow(window) | ||
| return true | ||
| } | ||
| bringToFront(window) |
There was a problem hiding this comment.
This change routes socket-suppressed focus-intent commands (for example window.focus / workspace.select) through bringToFront, which now performs full app activation. In the suppressed path this used to avoid AppKit activation, so running these commands while the user is in another macOS app can steal OS focus again. That regresses the socket focus policy and makes automation intrusive in normal desktop use.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR fixes CLI commands briefly stealing macOS focus by consistently enforcing the socket focus-intent policy across Key changes:
Process note: Per the regression test commit policy in Confidence Score: 5/5Safe to merge — all remaining findings are P2 observations about side-effect scope and a process policy note, none block correctness. The core logic is sound: the new guard conditions correctly prevent non-focus socket commands from calling AppKit activation helpers. All TerminalController activation paths are properly gated. The only open items are a P2 observation about Sources/AppDelegate.swift — confirm Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI Socket Command arrives] --> B{withSocketCommandPolicy\npushes suppressed=true\nallowsFocus=?}
B --> C{Is command in\nfocusIntentWhitelist?}
C -- Yes --> D[allowsFocus = true]
C -- No --> E[allowsFocus = false]
D --> F[focusMainWindow called]
E --> G[focusMainWindow called]
F --> H{suppressed &&\n!allowsFocus?}
H -- No --> I[bringToFront\nsetActiveMainWindow\ndeminiaturize\nmakeKeyAndOrderFront\nNSApp.activate]
H -- Yes --> J[setActiveMainWindow only\nreturn — no AppKit activation]
G --> K{suppressed &&\n!allowsFocus?}
K -- No --> I
K -- Yes --> J
style J fill:#f9a,stroke:#c00
style I fill:#9fa,stroke:#060
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
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:4993">
P2: Suppressed socket commands now mutate active in-app window routing even when focus mutations are disallowed.</violation>
</file>
<file name="Sources/TerminalController.swift">
<violation number="1" location="Sources/TerminalController.swift:10460">
P2: socketCommandAllowsInAppFocusMutations() relies on a global shared stack across all socket threads, so concurrent commands can read another command’s focus policy and incorrectly allow/suppress focus changes in newly gated paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| if TerminalController.shouldSuppressSocketCommandActivation(), | ||
| !TerminalController.socketCommandAllowsInAppFocusMutations() { | ||
| setActiveMainWindow(window) |
There was a problem hiding this comment.
P2: Suppressed socket commands now mutate active in-app window routing even when focus mutations are disallowed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/AppDelegate.swift, line 4993:
<comment>Suppressed socket commands now mutate active in-app window routing even when focus mutations are disallowed.</comment>
<file context>
@@ -4988,14 +4988,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
- }
+ if TerminalController.shouldSuppressSocketCommandActivation(),
+ !TerminalController.socketCommandAllowsInAppFocusMutations() {
+ setActiveMainWindow(window)
return true
}
</file context>
| } | ||
| NSApp.activate(ignoringOtherApps: true) | ||
| window.makeKeyAndOrderFront(nil) | ||
| if socketCommandAllowsInAppFocusMutations() { |
There was a problem hiding this comment.
P2: socketCommandAllowsInAppFocusMutations() relies on a global shared stack across all socket threads, so concurrent commands can read another command’s focus policy and incorrectly allow/suppress focus changes in newly gated paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/TerminalController.swift, line 10460:
<comment>socketCommandAllowsInAppFocusMutations() relies on a global shared stack across all socket threads, so concurrent commands can read another command’s focus policy and incorrectly allow/suppress focus changes in newly gated paths.</comment>
<file context>
@@ -10457,8 +10457,10 @@ class TerminalController {
}
- NSApp.activate(ignoringOtherApps: true)
- window.makeKeyAndOrderFront(nil)
+ if socketCommandAllowsInAppFocusMutations() {
+ NSApp.activate(ignoringOtherApps: true)
+ window.makeKeyAndOrderFront(nil)
</file context>
2 participants
Summary
AppDelegateso non-focus CLI commands cannot invoke AppKit activation/fronting helpersTerminalControlleractivation paths forsettings.open,feedback.open,debug.type, and synthetic input through the same focus-intent whitelistTesting
./scripts/reload.sh --tag cli-focus-stealCloses #140.
Summary by cubic
Stops CLI commands from briefly stealing focus by enforcing the socket focus policy and blocking unintended app/window activation. Addresses #140.
AppDelegate:bringToFront/focusMainWindowearly-exit under suppression; update active main window without activating UI.settings.open,feedback.open,debug.type, synthetic input, and shortcut simulation behindsocketCommandAllowsInAppFocusMutations; usev2FocusAllowedfor V2 activation flags; skipNSApp.activate/makeKeyAndOrderFrontwhen disallowed.simulate_shortcut,settings.open,feedback.open, anddebug.type.Written for commit 5642bb1. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Chores