Forward Cmd+Up / Cmd+Down to browser pane (#4635)

[austinywang]

Summary

Fixes #4635.

Browser-focused vertical document navigation now gets routed through the existing trusted keyDown forwarding path instead of falling into the command-equivalent/menu fallback. The change is intentionally narrow: browser content owns plain arrows plus command-only Cmd+Up/Cmd+Down, while modified cmux shortcuts such as Cmd+Option+Arrow stay out of the browser forwarding path.

Routing change

• Extended shouldDispatchBrowserArrowViaFirstResponderKeyDown to return true for browser-focused Cmd+Up and Cmd+Down.
• Kept the existing first-responder ownership boundary in NSWindow.cmux_performKeyEquivalent: the browser pane gets these keys only when the focused responder belongs to the WKWebView.
• Left Cmd+Left/Cmd+Right and Cmd+Option+Arrow outside the forced browser route so back/forward/editor/menu behavior and cmux pane-focus shortcuts are not broadened by this fix.

Re-verified combos

Code-level routing coverage:

• Plain Up/Down/Left/Right still route to browser keyDown when the WKWebView is first responder.
• Cmd+Up and Cmd+Down now route to browser keyDown when the WKWebView is first responder.
• Cmd+Left and Cmd+Right are not newly force-forwarded by this change.
• Cmd+Option+Up and Cmd+Option+Down are not force-forwarded, preserving the cmux pane-focus shortcut family.
• Browser marked-text composition still prevents forced arrow forwarding.
• Non-browser first responder still prevents browser arrow forwarding.

Runtime Google Docs verification/video was not produced in this workspace because the task explicitly reserves dev builds and launches for HQ.

Tests

• Added a failing regression test first in commit 0b29e8bc1.
• Added the fix in commit b43445b07.
• Added symmetric Cmd+Option+Up negative coverage in commit fd8c1b18d after Greptile feedback.
• Not run locally per task instruction; CI will run the test suite.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Enhanced keyboard navigation support for Command+Up and Command+Down arrow key combinations to ensure proper routing and prevent conflicts with other shortcuts.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Canceled		May 23, 2026 5:41am
cmux-staging	Building	Preview, Comment	May 23, 2026 5:41am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1cac2918-3485-4fe2-8ea6-d2045f1b4e59

📥 Commits

Reviewing files that changed from the base of the PR and between f2a39ff and fd8c1b1.

📒 Files selected for processing (2)

• Sources/App/ShortcutRoutingSupport.swift
• cmuxTests/BrowserArrowKeyForwardingTests.swift

---

📝 Walkthrough

Walkthrough

This PR enables Cmd+Up and Cmd+Down to reach the browser pane in cmux by modifying arrow key routing logic. The routing function now dispatches command-modified vertical arrow keys via the browser's first responder instead of blocking them. Corresponding test changes verify the behavior and ensure other modifier combinations remain unaffected.

Changes

Command+vertical arrow key routing

Layer / File(s)	Summary
Command+vertical arrow routing and test coverage Sources/App/ShortcutRoutingSupport.swift, cmuxTests/BrowserArrowKeyForwardingTests.swift	shouldDispatchBrowserArrowViaFirstResponderKeyDown now returns true for command-only modifiers with Up/Down keys (keyCodes 125/126), narrowly scoped to avoid stealing other shortcuts. New test verifies Cmd+vertical arrows dispatch via first responder when browser owns focus; expanded negative test ensures Cmd and Cmd+Option combinations with other keys still don't force arrow forwarding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• #4635: This PR directly implements the fix for the issue where Cmd+Up/Cmd+Down don't reach the browser pane, enabling Google Docs and other web apps to handle document navigation shortcuts.

Poem

🐰 A rabbit hopped through arrow keys with glee,
Adding Cmd to Up and Down with decree,
"Let Google Docs scroll free," it did say,
Routing with care through the browser's first way! 🎯

🚥 Pre-merge checks | ✅ 16 | ❌ 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 (16 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: forwarding Cmd+Up/Cmd+Down keyboard shortcuts to the browser pane, which directly addresses the issue #4635.
Description check	✅ Passed	The PR description includes all major template sections: a comprehensive Summary explaining the routing change and scope, detailed Re-verified combos section covering code-level coverage, and a Tests section documenting the test additions and approach.
Linked Issues check	✅ Passed	The PR fully addresses issue #4635's objectives: Cmd+Up/Down are now routed as trusted key events to the WKWebView when it's the focused responder, and other arrow combos are preserved as intended.
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped to the stated objective: only Cmd+Up/Down routing is modified; Cmd+Left/Right and Cmd+Option+Arrow are intentionally left unchanged, and marked-text guards are preserved.
Cmux Swift Actor Isolation	✅ Passed	Utility functions and immutable value types with no global mutable state, no Sendable/Codable conformances. Safe MainActor.assumeIsolated usage.
Cmux Swift Blocking Runtime	✅ Passed	Production changes are pure logic functions for keyboard routing with no blocking/timing synchronization (no sleep, semaphores, waits, locks, or delayed dispatch). Test-only deterministic assertions.
Cmux No Hacky Sleeps	✅ Passed	PR modifies only Swift files (.swift), which are explicitly excluded from this check. Swift timing rules are covered by a separate rule, not this one.
Cmux Swift Concurrency	✅ Passed	PR introduces only synchronous code: modified shouldDispatchBrowserArrowViaFirstResponderKeyDown (pure logic) and XCTest assertions. No legacy async patterns introduced.
Cmux Swift @Concurrent	✅ Passed	No async/concurrent violations found. All functions are synchronous pure utilities. The one DispatchQueue.main.async call is intentional UI-bound work on the main thread, which is allowed.
Cmux Swift File And Package Boundaries	✅ Passed	Focused bug fix adding ~5 lines to 813-line ShortcutRoutingSupport.swift, far below 250-line failure threshold; maintains single keyboard-routing responsibility with test coverage.
Cmux Swift Logging	✅ Passed	No logging violations found. Changes contain no print, debugPrint, dump, NSLog, file logging, Logger declarations, or data exposure.
Cmux User-Facing Error Privacy	✅ Passed	PR contains only keyboard routing logic in production code and test assertions; no user-facing errors, alerts, or sensitive information exposed.
Cmux Full Internationalization	✅ Passed	No user-facing text changes. Modified comment is developer-only, and test file changes are explicitly allowed per i18n rules.
Cmux Swiftui State Layout	✅ Passed	Both changed files are pure AppKit/Foundation code with no SwiftUI imports or patterns. No state decorators, views, or SwiftUI state violations found.
Cmux Architecture Rethink	✅ Passed	Small pure-logic correctness fix with clear owner (shouldDispatchBrowserArrowViaFirstResponderKeyDown) and narrow invariant. No timing constructs, state leaks, or architectural debt introduced.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR modifies keyboard routing logic and adds test-only fixtures. No window instantiations or window management changes; auxiliary window linting script passes.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-4635-browser-cmd-arrow-keys

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR fixes issue #4635 by routing Cmd+Up and Cmd+Down through the same trusted keyDown forwarding path used for plain arrow keys when a WKWebView is first responder, instead of letting them fall through to the command-equivalent/menu path where web editors cannot intercept them.

• Production change: shouldDispatchBrowserArrowViaFirstResponderKeyDown now returns true for normalizedFlags == [.command] when keyCode is 125 (Down) or 126 (Up), in addition to the existing plain-arrow pass-through.
• Intentional exclusions: Cmd+Left/Cmd+Right (keyCodes 123/124) and all Cmd+Option+Arrow combinations are explicitly left outside the forwarding path, preserving back/forward navigation and cmux pane-focus shortcuts.
• Test coverage: A new positive test verifies both vertical command-arrow keys are forwarded when the browser is first responder; negative tests cover Cmd+Left, Cmd+Right, Cmd+Option+Down, and Cmd+Option+Up.

Confidence Score: 5/5

Safe to merge — the change is narrow, deterministic, and fully guarded by existing first-responder and marked-text checks.

The routing addition is a single exact-equality predicate on modifier flags and two keyCode values; all pre-existing guards remain intact, the positive and negative test coverage matches every exclusion boundary called out in the PR description, and the excluded shortcuts (Cmd+Left/Right, Cmd+Option+Arrow) are independently verified.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/App/ShortcutRoutingSupport.swift	Extends shouldDispatchBrowserArrowViaFirstResponderKeyDown to return true for Cmd+Up/Down (keyCodes 126/125 with only .command modifier); all existing guards preserved.
cmuxTests/BrowserArrowKeyForwardingTests.swift	Adds positive test for Cmd+Up/Down routing and updates negative tests to cover Cmd+Left, Cmd+Right, Cmd+Option+Up, and Cmd+Option+Down.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[keyDown / performKeyEquivalent] --> B{firstResponderIsBrowser?}
    B -- No --> Z[Default AppKit routing]
    B -- Yes --> C{firstResponderHasMarkedText?}
    C -- Yes --> Z
    C -- No --> D{keyCode in 123..126?}
    D -- No --> Z
    D -- Yes --> E[Normalize flags strip numericPad/function/capsLock]
    E --> F{normalizedFlags empty?}
    F -- Yes --> G[Forward via keyDown plain arrows]
    F -- No --> H{normalizedFlags == .command AND keyCode == 125 or 126?}
    H -- Yes --> I[Forward via keyDown Cmd+Up / Cmd+Down NEW]
    H -- No --> Z

Loading

_{Reviews (2): Last reviewed commit: "test: cover Cmd Option Up browser routin..." | Re-trigger Greptile}