Allow Command-Escape keybindings#4307
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
github-actions
Bot
added
vouch:unvouched
ApprovabilityVerdict: Needs human review This PR introduces new capability for Command-Escape keybindings, involving global shortcut registration on macOS and new IPC channels for keyboard event forwarding. The runtime behavior changes to the keybinding capture system warrant human review. You can customize Macroscope's approvability policy. Learn more. |
![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: d9d2d6ae96
ℹ️ 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".
| const captureInput = nativeKeybindingCaptureInput(input); | ||
| if (captureInput) { | ||
| event.preventDefault(); | ||
| wc.send(NATIVE_KEYBINDING_CAPTURE_CHANNEL, captureInput); |
There was a problem hiding this comment.
When a preview webview owns focus, this sends the synthetic Escape keydown into the guest page, but T3 app shortcuts are resolved by the parent renderer's window keydown listener (apps/web/src/routes/_chat.tsx) and the existing preview shortcut bridge forwards app shortcuts with mainWindow.value.webContents.sendInputEvent. As a result, a user-assigned mod+esc command such as preview refresh/toggle will not run while focus is inside the preview, even though the commit explicitly tries to support focused preview webviews; route this captured chord to the main window as an app shortcut rather than only to wc.
Useful? React with 👍 / 👎.
| input: Pick<Input, "type" | "key" | "meta" | "control" | "alt" | "shift">, | ||
| ): NativeKeybindingCaptureInput | null { | ||
| const key = input.key.toLowerCase(); | ||
| if (input.type !== "keyDown" || (key !== "escape" && key !== "esc") || !input.meta) { |
There was a problem hiding this comment.
On non-Mac platforms mod is derived from ctrlKey in keybindingFromKeyboardEvent, but both desktop before-input-event paths call this helper, which only accepts input.meta. In the Windows/Linux scenario where Ctrl+Escape needs the native bridge instead of a normal renderer keydown, the helper returns null, while the only synthesized chord would be Meta+Escape and record/run as meta+esc rather than the newly allowed mod+esc; pass the platform or accept control for non-Darwin callers.
Useful? React with 👍 / 👎.
| if (Electron.globalShortcut.isRegistered(MACOS_MOD_ESCAPE_ACCELERATOR)) { | ||
| return; | ||
| } | ||
| const registered = Electron.globalShortcut.register(MACOS_MOD_ESCAPE_ACCELERATOR, () => { |
There was a problem hiding this comment.
On macOS this registers only the exact Command+Escape accelerator and the callback always emits the MACOS_MOD_ESCAPE_INPUT payload with ctrl/alt/shift false. The recorder now accepts and serializes extra modifiers on Escape, so Cmd+Shift+Esc/Cmd+Option+Esc keybindings can be saved but never captured or triggered on the platform that needs this native bridge; register and dispatch each modifier variant, or capture the actual flags, instead of hard-coding the unmodified chord.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 70f663e. Configure here.
| ctrlKey: false, | ||
| altKey: false, | ||
| shiftKey: false, | ||
| }; |
There was a problem hiding this comment.
macOS shortcut drops modifier keys
Medium Severity
On macOS, the global shortcut for Command+Escape always sends a fixed input payload that incorrectly sets shiftKey, altKey, and ctrlKey to false. This means any additional modifiers pressed with Command+Escape are ignored, leading to incorrect keybinding capture compared to other platforms.
Reviewed by Cursor Bugbot for commit 70f663e. Configure here.
| const focusedWebContents = Electron.webContents.getFocusedWebContents(); | ||
| if (focusedWebContents && !focusedWebContents.isDestroyed()) { | ||
| focusedWebContents.send(NATIVE_KEYBINDING_CAPTURE_CHANNEL, MACOS_MOD_ESCAPE_INPUT); | ||
| } |
There was a problem hiding this comment.
Cmd+Escape triggers bare Escape handlers
Medium Severity
While the main window is focused on macOS, every physical Command+Escape chord registers a global shortcut and forwards a synthetic keydown whose key is Escape. Several UI layers treat any Escape key as cancel/back (settings navigation, chat selection clear, preview pick teardown) without checking modifiers, so Command+Escape outside keybinding capture can trigger those actions unintentionally.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 70f663e. Configure here.
1 participant
Summary
Screen.Recording.2026-07-22.at.4.30.09.PM.mov
mod+escwhile keeping bare Escape as the recorder cancel actionRoot cause
macOS consumes a physical Command-Escape chord before Chromium emits the renderer keyboard event. The keybinding recorder therefore stayed blank even though its web-level normalization supported
mod+esc.Validation
vp test run apps/web/src/components/settings/KeybindingsSettings.logic.test.ts apps/desktop/src/keybindings/NativeKeybindingCapture.test.ts apps/desktop/src/preview/Manager.test.ts apps/desktop/src/window/DesktopWindow.test.ts(56 tests)vp lintfor all changed filesvp run typecheckinapps/desktopvp run typecheckinapps/webmod+escin T3 Code DevNote
Allow Command-Escape to be captured and recorded as a keybinding
Command+Escapeshortcut while the main window is focused and forwards it to the focused WebContents via a newNATIVE_KEYBINDING_CAPTURE_CHANNELIPC channel; unregisters on blur or close.Meta+Escapeviabefore-input-eventon the window's WebContents, prevents default, and sends the same payload.keydownEscape event with modifier flags.mod+Escapeis now recorded as a valid shortcut; bare Escape still cancels capture.Macroscope summarized 70f663e.
Note
Medium Risk
Touches global shortcuts and low-level keyboard handling across main window and preview webviews; focus/blur registration limits macOS side effects but incorrect lifecycle could still steal or miss chords.
Overview
Command-Escape can now be recorded as
mod+escin keybinding settings on desktop, where macOS previously swallowed the chord before the renderer saw it.A new
desktop:native-keybinding-captureIPC path normalizes meta+Escape in the main process, prevents default where needed, and preloads re-inject a synthetickeydownon the focused element. On macOS,Command+Escapeis registered viaglobalShortcutonly while the main window is focused and unregistered on blur/close; on other platforms the main window usesbefore-input-event. Preview webviews get the same intercept inPreviewManager.In KeybindingsSettings, capture runs
keybindingFromKeyboardEventbefore treating Escape as cancel, so modified Escape becomes a shortcut while bare Escape still exits recording.Reviewed by Cursor Bugbot for commit 70f663e. Bugbot is set up for automated code reviews on this repo. Configure here.