feat(web): add a rebindable thread.interrupt keybinding - #5669
feat(web): add a rebindable thread.interrupt keybinding#5669ChristmasSun wants to merge 6 commits into
Conversation
A key can be written more than one way: parseKeybindingShortcut normalizes "esc" to "escape" and "space" to " ". The settings UI renders a stored rule back using the alias, so editing a rule bound to Escape sent a replace target of "esc" for a rule persisted as "escape". Rule comparison used raw string equality, so the target never matched, the original rule survived, and the command ended up bound twice. Compare the parsed shortcut when the raw keys differ. No default binding uses an aliased key today, which is why this has gone unnoticed.
Stopping a running turn required clicking the stop button; there was no keyboard path and nothing to bind. Register thread.interrupt as a keybinding command, defaulting to Escape, so it appears in Settings as "Thread: Interrupt" and can be rebound. The shortcut is only claimed while a turn is running, so Escape keeps its existing behavior everywhere else, and the default is scoped to !terminalFocus so it still reaches the terminal.
|
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:
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 |
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new rebindable You can customize Macroscope's approvability policy. Learn more. |
…lays Review follow-ups: ignore key auto-repeat so holding the shortcut cannot spray concurrent interrupt requests at the same turn, skip the shortcut while the command palette or stash menu is open so those keep their dismiss behavior, and exclude the model picker via the default binding's when clause.
…binding The listener ran in the capture phase and stopped propagation, so its allowlist of two overlays was the only thing standing between the shortcut and every other Escape-dismissible surface — dialogs, menus, selects — which could neither dismiss nor be enumerated exhaustively. Listen in the bubble phase instead and skip when the event was already consumed: by the time the event reaches window, every overlay handler has run, and a dismiss marks it via preventDefault or stops it from arriving at all. The palette/stash checks stay as backstops for dismiss handlers that do not mark the event.
| event.preventDefault(); | ||
| onInterrupt(); | ||
| }; | ||
| window.addEventListener("keydown", handler); |
There was a problem hiding this comment.
🟠 High chat/ChatComposer.tsx:2317
The thread.interrupt listener on window runs in bubble phase, but ChatView's type-to-focus handler runs in capture phase on the same window. When thread.interrupt is rebound to a bare printable key (e.g. x) and focus is on non-interactive chat content, the capture-phase handler calls shouldTypeToFocusComposer, inserts the key into the composer, and calls stopPropagation — so the bubble-phase interrupt handler never fires and the running turn is not interrupted. The interrupt binding needs to be resolved before type-to-focus can consume printable shortcuts, or otherwise excluded from that path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 2317:
The `thread.interrupt` listener on `window` runs in bubble phase, but `ChatView`'s type-to-focus handler runs in capture phase on the same `window`. When `thread.interrupt` is rebound to a bare printable key (e.g. `x`) and focus is on non-interactive chat content, the capture-phase handler calls `shouldTypeToFocusComposer`, inserts the key into the composer, and calls `stopPropagation` — so the bubble-phase interrupt handler never fires and the running turn is not interrupted. The interrupt binding needs to be resolved before type-to-focus can consume printable shortcuts, or otherwise excluded from that path.
There was a problem hiding this comment.
Valid. Fixed in b00ab8c: ChatView's handler now resolves the shortcut before the type-to-focus branch, and type-to-focus only claims keys that resolve to no binding. An explicit binding on a bare printable key wins consistently — for every command, not just thread.interrupt — and the event then falls through the dispatch chain untouched to the bubble-phase interrupt listener. One deliberate trade-off: such a binding now always claims its key, so it no longer types into the composer even while the thread is idle. That matches how every modifier-chord binding already behaves, and the alternative — key types when idle, interrupts when running — seemed worse. Shipped defaults are all modifier chords, so nothing changes without a custom binding.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ChatView's capture-phase handler inserted bare printable keys into the composer before resolving the shortcut, so a command bound to a printable key (for example thread.interrupt rebound to a letter) was shadowed whenever focus sat on neutral chat content — the key typed instead of dispatching, and bubble-phase listeners never saw the event. Resolve the command first and let type-to-focus claim only keys that resolve to no binding. An explicit binding on a bare printable key now wins consistently, matching how every modifier-based binding already behaves. Shipped defaults are all modifier chords, so nothing changes without a custom binding.
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 b00ab8c. Configure here.
Escape closing the expanded image viewer during a running turn also interrupted the turn: both listeners sit on window bubble, the dialog mounts later so it registers later, and the interrupt handler had already seen the event before the dialog marked it consumed. Ordering among same-phase listeners cannot answer "is an overlay open", so ask the DOM: share the floating-layer selector that type-to-focus already uses (moved to ChatView.logic, with role=dialog added for ad-hoc dialogs like the image viewer) and skip the shortcut while any floating layer is present. This replaces the stash-menu allowlist check — the popover matches the selector directly.
Closes #5667. Stacked on #5668 — rebinding the default Escape shortcut duplicates the rule without that fix, so this branch contains that commit too. Reviewing #5668 first leaves this diff at just the feature (5 files, +60).
What changed
thread.interruptis registered as a keybinding command, defaulting to Escape. It appears in Settings → Keybindings as "Thread: Interrupt" and can be rebound like any other shortcut.Why
Stopping a running turn required clicking the square stop button — there was no keyboard path, and no command to bind. Codex and Claude Code both stop on Escape, so the reflex already exists.
#4298 did this as a hard-coded Escape in the composer and was closed. This registers a command instead, so the key is configurable rather than baked in.
Behavior
!terminalFocus, so Escape still reaches the terminal.shouldInterruptRunningThreadFromKeybinding) with unit tests; the listener follows the existingcomposer.stashpattern inChatComposer.commandLabelderives "Thread: Interrupt" from the command id, so no label map changes were needed.Rebinding back to Escape
The recorder uses bare Escape as its cancel action (
KeybindingsSettings.tsx:807), matching how Codex behaves, so Escape is not recordable as a shortcut. Reset-to-default is the way back to it after rebinding, which works because Escape ships as this command's default.Verification
apps/webcomposer-logic: 43 pass, including 3 new casespackages/shared+ settings suites: 422 passapps/serverkeybindings: 23 passvp lintclean, typecheck clean acrossweb,contracts,shared,servernote: video here is for this fix + the fix on #5668 as if not when you change the stop thread keybind from escape to something else it duplicates which is an unrelated bug that #5668 fixes.
Screen.Recording.2026-08-07.at.5.20.36.PM.mov
Checklist
Note
Medium Risk
Touches global keyboard routing (capture vs bubble) and default Escape behavior during active turns; changes are scoped with overlay checks and idle-thread fall-through, plus a server-side keybinding persistence fix.
Overview
Introduces
thread.interruptas a first-class keybinding (defaultescape, when!terminalFocus && !modelPickerOpen) so users can stop a running turn from the keyboard and rebind it in Settings.ChatComposerlistens on windowkeydown(bubble phase), resolves the binding, and callsonInterruptonly while the thread phase isrunning—ignoring repeat events, consumed keys, the command palette, and any open floating layer via sharedOPEN_FLOATING_LAYER_SELECTOR(dialogs, menus,[role="dialog"], etc.).ChatViewtype-to-focus now runs only when the key resolves to no command, so printable keys explicitly bound (e.g. to interrupt) are not swallowed in capture phase.Server
isSameKeybindingRulecompares keys through parsed/canonical encoding so alias spellings (esc/escape) match when replacing rules from the settings UI.Reviewed by Cursor Bugbot for commit 0c3b692. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add rebindable
thread.interruptkeybinding defaulting to Escapethread.interruptto the keybinding contracts and sets a default binding ofEscape(when terminal and model picker are not focused) inpackages/shared/src/keybindings.ts.ChatComposervia a windowkeydownbubble-phase handler that callsonInterruptwhen the thread is running, guarded against floating layers and command palette.ChatViewso it no longer intercepts keys that have an explicit keybinding — commands are resolved first and type-to-focus only activates when none match.isSameKeybindingRuleinkeybindings.tsto canonicalize keys before comparison, treating alias spellings likeesc/escapeas equivalent.Macroscope summarized 0c3b692.