Skip to content

Fix unbound Cmd+Shift+key combos being silently swallowed - #1959

Merged
austinywang merged 1 commit into
manaflow-ai:mainfrom
che-3:fix/cmd-shift-key-swallowed
Mar 22, 2026
Merged

austinywang merged 1 commit into
manaflow-ai:mainfrom
che-3:fix/cmd-shift-key-swallowed

Conversation

@che-3

@che-3 che-3 commented Mar 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Unbound Cmd+Shift+key combos (e.g. Cmd+Shift+K with no binding) were silently consumed and never reached the terminal via kitty keyboard protocol
  • Root cause: the performKeyEquivalent redispatch path used event.characters which returns empty string for Cmd-modified keys
  • Changed to event.charactersIgnoringModifiers which returns the actual key character (e.g. "k"), while modifiers are preserved in modifierFlags

Why bound keys worked

Bound Cmd+Shift keys go directly through keyDown (line ~5077), bypassing the redispatch path entirely. Only unbound keys hit the two-pass redispatch where event.characters was used.

Test plan

  • Press Cmd+Shift+K (unbound) in Neovim with kitty protocol → key event received
  • Press Cmd+Shift+J (bound to write_screen_file) → still works as before
  • Press Cmd+N, Cmd+D, etc. (bound cmux shortcuts) → unchanged
  • Test on non-US keyboard layout (AZERTY, etc.) → correct character sent

Fixes #1718

🤖 Generated with Claude Code


Summary by cubic

Fixes unbound Cmd+Shift+key combos being swallowed on macOS so the keys reach the terminal via the kitty keyboard protocol. Fixes #1718 while leaving existing keybindings unchanged.

  • Bug Fixes
    • Root cause: event.characters returns an empty string for Cmd‑modified keys in the performKeyEquivalent redispatch path.
    • Change: use event.charactersIgnoringModifiers to get the base character while preserving modifierFlags.

Written for commit 1ce751f. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed keyboard event routing to correctly handle modifier key combinations, improving input consistency when processing complex key sequences.

Use charactersIgnoringModifiers instead of characters when redispatching
Cmd-modified key events in performKeyEquivalent. Cmd-modified keys don't
produce text characters, so event.characters returns an empty string for
Cmd+Shift combos, preventing Ghostty from encoding them as kitty protocol
sequences. charactersIgnoringModifiers returns the actual key character
(e.g. "k" for Cmd+Shift+K) while modifiers are preserved in modifierFlags.

Fixes manaflow-ai#1718

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 22, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Mar 22, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 23ec2bf0-5dd3-4f17-a985-9e13ed4746eb

📥 Commits

Reviewing files that changed from the base of the PR and between e0e0e35 and 1ce751f.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift

📝 Walkthrough

Walkthrough

A single-line modification in the AppKit key-event redispatch logic changes the fallback character extraction from event.characters to event.charactersIgnoringModifiers, affecting how the key equivalent is matched when suppressing and reusing previously handled Cmd-modified key events via the two-pass timestamp mechanism.

Changes

Cohort / File(s) Summary
Key-event character extraction
Sources/GhosttyTerminalView.swift
Modified the equivalent string derivation in the two-pass timestamp mechanism to use event.charactersIgnoringModifiers instead of event.characters, improving key matching accuracy for Cmd-modified keys regardless of modifier state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A modifierless path, the key does take,
Ignoring shifts and commands awake,
Through AppKit's maze, the keystroke flies,
No longer swallowed—unbound and wise! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main bug fix: resolving the issue where unbound Cmd+Shift+key combinations were being silently consumed without reaching the terminal.
Description check ✅ Passed The PR description comprehensively covers the summary, root cause, why bound keys worked, and includes a detailed test plan matching the required template sections.
Linked Issues check ✅ Passed The code change directly addresses issue #1718 by replacing event.characters with event.charactersIgnoringModifiers in performKeyEquivalent, ensuring unbound Cmd+Shift+key combinations are forwarded to the terminal via kitty protocol.
Out of Scope Changes check ✅ Passed The single-line change is narrowly scoped to fix the specific bug in the key-equivalent redispatch logic and introduces no out-of-scope modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@greptile-apps

greptile-apps Bot commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a one-character bug in the two-pass performKeyEquivalent redispatch path: unbound Cmd+Shift+<key> combos were silently consumed because event.characters returns an empty string for Command-modified keys on macOS, causing the reconstructed finalEvent to carry an empty character string and reach keyDown in a state the kitty keyboard protocol couldn't encode correctly. Replacing it with event.charactersIgnoringModifiers (which returns the base key character regardless of Command) ensures the key is forwarded properly.

  • Root cause confirmed correct: NSEvent.characters is documented to return empty for Cmd-modified keys; charactersIgnoringModifiers is the standard workaround.
  • Reconstructed event is consistent: modifierFlags (including .command and .shift) are already preserved on finalEvent, so using the base character for both characters and charactersIgnoringModifiers fields is safe — downstream kitty encoding reads modifier flags separately.
  • No regression risk for bound keys: bound Cmd+Shift keys reach keyDown via the early-exit path at line 5077, completely bypassing this redispatch code.
  • Non-US layouts: charactersIgnoringModifiers respects the active keyboard layout while stripping Command, making the fix layout-agnostic.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, well-reasoned one-line fix with no regressions on the bound-key path.
  • The change targets a single, clearly-identified macOS API behaviour: NSEvent.characters always returns an empty string for Command-modified events, while charactersIgnoringModifiers returns the base character. The fix is the canonical workaround. Bound keys are unaffected because they exit performKeyEquivalent before reaching the changed line. The reconstructed event preserves all modifier flags, so kitty keyboard protocol encoding remains correct. No new logic, no new state, no new API surface.
  • No files require special attention.

Important Files Changed

Filename Overview
Sources/GhosttyTerminalView.swift One-line fix in the two-pass performKeyEquivalent redispatch path: swaps event.characters (always empty for Cmd-modified keys on macOS) for event.charactersIgnoringModifiers so that unbound Cmd+Shift+key combos produce a non-empty character string and are forwarded to keyDown instead of being silently swallowed.

Sequence Diagram

sequenceDiagram
    participant AppKit
    participant PKE as performKeyEquivalent
    participant KD as keyDown

    Note over AppKit,KD: Unbound Cmd+Shift+Key (no cmux binding)

    AppKit->>PKE: Pass 1
    PKE->>PKE: is_binding check returns nil
    PKE->>PKE: lastPerformKeyEvent is nil, store timestamp T
    PKE-->>AppKit: false

    AppKit->>PKE: Pass 2 (same event, timestamp T)
    PKE->>PKE: lastPerformKeyEvent matches timestamp T
    PKE->>PKE: Bug: event.characters was empty for Cmd-modified keys
    PKE->>PKE: Fix: event.charactersIgnoringModifiers returns base character
    PKE->>PKE: Build finalEvent with base char and original modifierFlags
    PKE->>KD: keyDown(finalEvent)
    KD-->>PKE: encodes key via kitty keyboard protocol
    PKE-->>AppKit: true
Loading

Reviews (1): Last reviewed commit: "Fix Cmd+Shift+key combos being swallowed..." | Re-trigger Greptile

@austinywang
austinywang merged commit eb248a1 into manaflow-ai:main Mar 22, 2026
4 of 5 checks passed
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…i#1959)

Use charactersIgnoringModifiers instead of characters when redispatching
Cmd-modified key events in performKeyEquivalent. Cmd-modified keys don't
produce text characters, so event.characters returns an empty string for
Cmd+Shift combos, preventing Ghostty from encoding them as kitty protocol
sequences. charactersIgnoringModifiers returns the actual key character
(e.g. "k" for Cmd+Shift+K) while modifiers are preserved in modifierFlags.

Fixes manaflow-ai#1718

Co-authored-by: CHE-3 <schumannzheng@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unbound Cmd+Shift+<key> key combinations are silently swallowed, they never reach terminal/kitty protocol

2 participants