Skip to content

Support tmux-style bare-key chord leaders - #3311

Open
robinjoseph08 wants to merge 16 commits into
manaflow-ai:mainfrom
robinjoseph08:bare-key-chord-leaders
Open

robinjoseph08 wants to merge 16 commits into
manaflow-ai:mainfrom
robinjoseph08:bare-key-chord-leaders

Conversation

@robinjoseph08

@robinjoseph08 robinjoseph08 commented Apr 29, 2026 •

Copy link
Copy Markdown

Summary

Enables tmux-style bare-key chord leaders (e.g. ` then d) configured via ~/.config/cmux/settings.json. Pressing the leader twice in a row sends one literal copy of the character to the focused terminal, mimicking tmux's default send-prefix behavior. Single-stroke bare-key bindings remain rejected.

Builds on chord support added in #2528.

{
  "shortcuts": {
    "bindings": {
      "splitRight": ["`", "d"],
      "focusLeft":  ["`", "h"],
      "focusRight": ["`", "l"],
      "focusUp":    ["`", "k"],
      "focusDown":  ["`", "j"],
      "toggleSplitZoom": ["`", "z"]
    }
  }
}

Partially addresses #1450 (tmux-style keybindings inside cmux) and #1711 (Space as a bindable key — usable now as a chord first stroke; single-stroke space still rejected).

What changed

  • Sources/AppDelegate.swift — narrowed the bare-key early-return guard in handleCustomShortcut so bare-key keyDown events reach the chord-arming code path when at least one configured shortcut has a bare-key chord prefix. Added a cached hasConfiguredBareKeyChordPrefix() flag (recomputed only when shortcuts change) so the typing-latency-sensitive path stays allocation-free. Added sendLiteralChordPrefixToFocusedSurface and the implicit <leader><leader> dispatch at the tail of handleCustomShortcut (after every matchConfiguredShortcut so explicit user bindings always win).
  • Sources/KeyboardShortcutSettings.swift — relaxed StoredShortcut.parseConfig(strokes:) to accept bare-key first strokes only when the shortcut is a chord (single-stroke bare keys remain rejected). Original spec assumed the parser already accepted bare keys; it didn't.
  • Sources/GhosttyTerminalView.swift — #if DEBUG-gated test seam in TerminalSurface.sendText (zero release cost).
  • Recorder UI unchanged — chord shortcuts are settings.json-only per Support chorded keyboard shortcuts #2528, so no recorder changes.

Tests

5 new tests in cmuxTests/AppDelegateShortcutRoutingTests.swift, plus all 7 existing chord regression tests still pass:

  • testBareKeyChordPrefixArmsAndSplitsOnSecondKey
  • testBareKeyChordMismatchDoesNotConsumeSecondKey — locks in Q1=a (eat prefix, pass second key on mismatch)
  • testBareKeyChordDoubleTapSendsLiteralToFocusedTerminal
  • testBareKeyChordDoubleTapWithExplicitBindingFiresActionInsteadOfLiteral
  • testSettingsFileBareKeyChordDispatchesSplitRight — end-to-end through the file store, asserts splitRight actually executes (panel count + 1)

Test plan

  • cmux-unit suite: 13 chord-related tests pass via xcodebuild ... -scheme cmux-unit ... test
  • Manual smoke in tagged Debug build with [" ","d"]-style bindings — split / focus / zoom / double-tap-literal / mismatch-passthrough all behave as expected

Spec/plan

  • Spec: docs/superpowers/specs/2026-04-29-bare-key-chord-leader-design.md
  • Plan: docs/superpowers/plans/2026-04-29-bare-key-chord-leader.md

Known follow-ups (not in this PR)

  • Cache invalidation: the cached bare-key flag refreshes on KeyboardShortcutSettings.didChangeNotification (covers settings.json — the user-facing path). It does NOT yet refresh on cmuxConfigStore.loadedActions changes (socket-configured custom shortcuts). Pre-existing pattern; separate follow-up.
  • Pane resize actions (resizeLeft/Right/Up/Down) — explicitly out of scope per the spec.
  • Recorder UX for shifted-key second strokes is unintuitive (shift+5 instead of %). Pre-existing in Support chorded keyboard shortcuts #2528; not in this PR.

Summary by cubic

Adds tmux-style bare-key chord leaders (e.g. then d) configurable via~/.config/cmux/settings.json`. Double-tap the leader to send a single literal to the focused terminal; single-stroke bare keys are still rejected. Partially addresses #1450 and #1711.

  • New Features

    • Allow a bare key as the first stroke of two-key chords in settings.json (e.g. [" ","d"]).
    • <leader><leader> sends the literal leader unless that exact chord is explicitly bound; uses event.characters so layout and Caps Lock are respected.
    • Shortcut routing checks for configured bare-key leaders on each event across all windows and only bypasses the no-modifiers early-return when present.
    • Docs updated to cover bare-key leaders, the double-tap behavior, and explicit <leader><leader> precedence.
  • Bug Fixes

    • Parser accepts a bare-key first stroke only for two-stroke chords; single-stroke bare keys remain invalid.

Written for commit 96fafe9. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • New Features

    • Added support for bare-key (modifier-less) keyboard chord leaders for simpler configurations
    • Implemented implicit double-tap behavior: pressing a bare-key leader twice forwards it as text to the active terminal
    • Supports explicit leader+leader chord binding to override default double-tap behavior
  • Documentation

    • Enhanced keyboard shortcut documentation with bare-key chord usage and double-tap mechanics explanations
    • Added Japanese and English translations for new chord shortcut behaviors

Spec for letting users bind tmux-style bare-key leaders (e.g. backtick) as chord prefixes, with implicit double-tap-to-send-literal.
The early-return guard in handleCustomShortcut bails on bare-key
events when no chord is armed, which prevents a tmux-style bare-key
leader (e.g. backtick) from ever arming. Narrow the guard to skip
only when no configured shortcut has a bare-key chord prefix.

Fixes the failing testBareKeyChordPrefixArmsAndSplitsOnSecondKey.
When a bare-key chord prefix is armed and the user presses the same
key again with no modifiers and no configured chord binding matched,
forward the prefix character to the focused Ghostty surface. This
matches tmux's default send-prefix behavior, requires no
configuration, and lets users with a bare-key leader still type the
literal character by double-tapping.
Assert first responder identity after makeFirstResponder so a focus
redirect produces a clear failure instead of a confusing empty
captured array. Drop unused manager binding.
Regression test: when a user binds `<prefix><prefix>` to an action (e.g.
splitRight), the configured-chord dispatch fires and the implicit
literal-send must not run. Verifies the existing dispatch order in
handleCustomShortcut where matchConfiguredShortcut runs before the
double-tap-literal block.
StoredShortcut.parseConfig(strokes:) was rejecting bare-key first strokes
unconditionally. Allow them when the binding is a two-stroke chord so that
settings.json entries like `"splitRight": ["\`", "d"]` are parsed and
dispatched correctly.

Adds testSettingsFileBareKeyChordDispatchesSplitRight which verifies the
full parser→file-store→routing path for a bare-key chord leader.
Add chordsRuleBareKey and chordsRuleDoubleTap translation keys to both
en.json and ja.json, wire them into the keyboard-shortcuts docs page,
and add an equivalent prose paragraph to the configuration docs page.
Recompute only when configured shortcuts change instead of allocating
arrays and JSON-decoding UserDefaults on every keystroke. Addresses
the typing-latency-sensitive paths policy in CLAUDE.md.
The previous test only asserted both events were consumed; a future
refactor that consumes the chord without dispatching would have
silently slipped through. Mirror testPerformSplitShortcutSplitsFocusedTerminalSurfaceWhenSelectedWorkspaceIsStale
to assert the actual panel-count change.
@vercel

vercel Bot commented Apr 29, 2026

Copy link
Copy Markdown

@robinjoseph08 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 Apr 29, 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: 2c652bc4-098f-4bf8-870c-17ed85dc275f

📥 Commits

Reviewing files that changed from the base of the PR and between fa7772d and 96fafe9.

📒 Files selected for processing (4)
  • Sources/AppDelegate.swift
  • cmuxTests/AppDelegateShortcutRoutingTests.swift
  • web/messages/en.json
  • web/messages/ja.json
✅ Files skipped from review due to trivial changes (2)
  • web/messages/ja.json
  • web/messages/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmuxTests/AppDelegateShortcutRoutingTests.swift

📝 Walkthrough

Walkthrough

Implements bare-key chord leader support for keyboard shortcuts. AppDelegate detects bare-key chord prefixes, prevents early event discarding, and introduces an implicit <leader><leader> fallback that sends the leader character as text when the same bare key is pressed twice. Updates parsing logic to permit bare-key first strokes in two-stroke chords. Adds test instrumentation and comprehensive test coverage with documentation updates.

Changes

Cohort / File(s) Summary
Core Chord Leader Implementation
Sources/AppDelegate.swift
Adds detection logic for bare-key chord leaders, prevents early-return event discarding for armed chords, introduces helper to resolve focused terminal and send leader character, and implements implicit <leader><leader> fallback behavior when the same bare key is pressed consecutively without a matching explicit chord binding.
Shortcut Parsing & Configuration
Sources/KeyboardShortcutSettings.swift
Modifies parsing logic to reject bare-key first strokes only for single-stroke shortcuts; permits bare-key leader strokes (no modifiers) as the first stroke in two-stroke chords while preserving strict modifier requirements for one-stroke shortcuts.
Test Instrumentation
Sources/GhosttyTerminalView.swift
Introduces DEBUG-only debugLastSendTextRecorder closure in TerminalSurface to capture literal text passed to sendText() for unit test validation without requiring a live Ghostty surface.
Test Coverage
cmuxTests/AppDelegateShortcutRoutingTests.swift
Adds comprehensive XCTest suite validating bare-key chord routing: tests cover arming on bare-key leader, matching second keystrokes, mismatched rejection, double-tap implicit character forwarding to terminal, and explicit leader+leader binding precedence using both shortcut action verification and debugLastSendTextRecorder inspection.
Documentation & i18n
web/app/[locale]/docs/configuration/page.tsx, web/app/[locale]/docs/keyboard-shortcuts/page.tsx, web/messages/en.json, web/messages/ja.json
Adds explanatory text for chord leader strokes describing bare-key acceptance in two-stroke chords, rejection rules for single-stroke bare-key bindings, double-tap "leader armed" behavior, and explicit binding precedence.

Sequence Diagram

sequenceDiagram
    actor User
    participant AppDelegate as AppDelegate<br/>(handleCustomShortcut)
    participant TerminalSurface as TerminalSurface<br/>(focused)
    
    User->>AppDelegate: Press bare-key leader (e.g., backtick)
    AppDelegate->>AppDelegate: Detect bare-key leader configured<br/>Arm chord state
    AppDelegate->>AppDelegate: Return (consume event)
    
    User->>AppDelegate: Press same bare-key again<br/>(no explicit chord binding)
    AppDelegate->>AppDelegate: Check if armed + same key<br/>No explicit binding match
    AppDelegate->>TerminalSurface: sendText(leader character)
    AppDelegate->>AppDelegate: Return (consume event)
    TerminalSurface-->>User: Display literal character
    
    User->>AppDelegate: Press bare-key leader<br/>then different key
    AppDelegate->>AppDelegate: Detect bare-key leader<br/>Arm chord state
    AppDelegate->>AppDelegate: Next key detected<br/>Lookup explicit chord binding
    alt Binding Found
        AppDelegate->>AppDelegate: Dispatch bound action
    else No Binding
        AppDelegate->>TerminalSurface: sendText(leader)
        AppDelegate->>AppDelegate: Pass second key through
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A leader hops, then hops again,
One keystroke held, the second then—
A backtick blooms on screen, so sweet,
Chords now bare, the dance complete! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% 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 and concisely summarizes the main feature: support for tmux-style bare-key chord leaders. It is specific, clear, and directly reflects the primary change in the changeset.
Description check ✅ Passed The description is comprehensive, covering all required template sections: detailed summary of what changed and why, thorough testing section with specific test names, actual test results, and manual verification. Additional context includes design specs and known follow-ups, exceeding the template baseline.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

@greptile-apps

greptile-apps Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds tmux-style bare-key chord leaders (e.g. ` then d) to cmux by narrowing the early-return guard in handleCustomShortcut, adding a cached bare-key prefix flag, relaxing the StoredShortcut parser to accept bare-key first strokes for chords, and implementing the implicit double-tap-to-literal behavior.

  • P1: hasConfiguredBareKeyChordPrefixCache is only recomputed in refreshConfiguredShortcutChordActions(), which fires on KeyboardShortcutSettings.didChangeNotification. Bare-key chords configured via socket (cmuxConfigStore.loadedActions) leave the cache false, silently dropping all prefix keystrokes at the early-return guard until the next settings.json change forces a recompute. This is new breakage introduced here, not covered by the acknowledged pre-existing pattern.

Confidence Score: 3/5

Safe to merge for settings.json users; socket-configured bare-key chords are broken until a follow-up lands.

One P1 defect: bare-key chord prefixes added via socket commands silently fail because the cache is only invalidated on settings.json changes. The settings.json path (the primary user-facing scenario) works correctly and is well-tested. The socket path breakage is new behavior introduced by this PR, not a pre-existing limitation.

Sources/AppDelegate.swift — specifically the recomputeHasConfiguredBareKeyChordPrefix / refreshConfiguredShortcutChordActions cache invalidation site.

Important Files Changed

Filename Overview
Sources/AppDelegate.swift Core routing change: early-return guard narrowed for bare-key events, cached hasConfiguredBareKeyChordPrefixCache flag added, sendLiteralChordPrefixToFocusedSurface and implicit double-tap dispatch added. Cache invalidation is incomplete for socket-configured shortcuts (P1).
Sources/KeyboardShortcutSettings.swift Parser relaxed to allow bare-key first stroke for two-stroke chords only; single-stroke bare-key rejection preserved. Logic is correct.
Sources/GhosttyTerminalView.swift Minimal #if DEBUG-gated test seam added to TerminalSurface.sendText; zero release-build impact.
cmuxTests/AppDelegateShortcutRoutingTests.swift Five new tests added covering arming, mismatch passthrough, double-tap literal, explicit binding precedence, and settings.json end-to-end. testBareKeyChordPrefixArmsAndSplitsOnSecondKey does not assert that the action actually executes despite the name implying it.
web/app/[locale]/docs/keyboard-shortcuts/page.tsx Two new localized list items added for bare-key leader rules; correctly wired via i18n keys.
web/app/[locale]/docs/configuration/page.tsx New prose paragraph added after existing chord documentation explaining bare-key leader syntax and double-tap literal behavior.
web/messages/en.json Two new i18n keys added for bare-key leader and double-tap rules; content is accurate and complete.
web/messages/ja.json Japanese translations added for both new i18n keys; matches the English semantic content.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant H as handleCustomShortcut
    participant C as hasConfiguredBareKeyChordPrefixCache
    participant A as armConfiguredShortcutChordIfNeeded
    participant M as matchConfiguredShortcut
    participant S as sendLiteralChordPrefixToFocusedSurface
    participant T as TerminalSurface

    U->>H: keyDown "`" (no modifiers)
    H->>C: hasConfiguredBareKeyChordPrefix()
    C-->>H: true (bare-key chord configured)
    H->>A: armConfiguredShortcutChordIfNeeded(event)
    A-->>H: prefix "`" armed
    H-->>U: return true (event consumed)

    U->>H: keyDown "d" (chord second stroke)
    H->>M: matchConfiguredShortcut(.splitRight)
    M-->>H: match, execute splitRight
    H-->>U: return true (action dispatched)

    Note over U,T: Double-tap path
    U->>H: keyDown "`" (prefix armed, no match)
    H->>M: matchConfiguredShortcut (all actions)
    M-->>H: no match
    H->>S: sendLiteralChordPrefixToFocusedSurface(prefix, event)
    S->>T: sendText("`")
    S-->>H: true
    H-->>U: return true (literal forwarded)
Loading

Reviews (1): Last reviewed commit: "Assert splitRight side effect in setting..." | Re-trigger Greptile

Comment thread Sources/AppDelegate.swift Outdated
Comment on lines +9979 to +9999
hasConfiguredBareKeyChordPrefixCache = recomputeHasConfiguredBareKeyChordPrefix()
}

private func recomputeHasConfiguredBareKeyChordPrefix() -> Bool {
let context = preferredRegisteredMainWindowContext()
let configuredShortcuts = configuredCmuxShortcutActions(for: context)
.compactMap(\.shortcut)
for action in configuredShortcutChordActions {
let shortcut = KeyboardShortcutSettings.shortcut(for: action)
guard shortcut.hasChord else { continue }
if shortcut.firstStroke.modifierFlags.isEmpty {
return true
}
}
for shortcut in configuredShortcuts {
guard shortcut.hasChord else { continue }
if shortcut.firstStroke.modifierFlags.isEmpty {
return true
}
}
return false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cache not invalidated for socket-configured bare-key chord prefixes

recomputeHasConfiguredBareKeyChordPrefix() is only called from refreshConfiguredShortcutChordActions(), which fires on KeyboardShortcutSettings.didChangeNotification (i.e., settings.json changes). If a bare-key chord leader is added via a socket command (cmuxConfigStore.loadedActions), hasConfiguredBareKeyChordPrefixCache stays false. Every bare-key event will still hit the early-return guard in handleCustomShortcut and return false, silently discarding the prefix event and making the socket-configured bare-key chord non-functional until the next settings.json write triggers a refresh.

The PR description acknowledges this but frames it as a pre-existing pattern; however, the observable breakage (bare-key chord configured via socket does not arm) is new behavior introduced here, not pre-existing.

Comment on lines +318 to +332
}
""".write(to: settingsFileURL, atomically: true, encoding: .utf8)

KeyboardShortcutSettings.settingsFileStore = KeyboardShortcutSettingsFileStore(
primaryPath: settingsFileURL.path,
fallbackPath: nil,
startWatching: false
)

window.makeKeyAndOrderFront(nil)
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))

guard let terminalView = surfaceView(in: focusedPanel.hostedView) else {
XCTFail("Expected a GhosttyNSView inside the focused panel's hosted view")
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Test name implies split fires, but no panel-count assertion

testBareKeyChordPrefixArmsAndSplitsOnSecondKey only asserts that both events return true (consumed). The "splits" claim in the test name — and the assertion message "Second stroke after a bare-key chord prefix must dispatch the bound action" — is not actually verified; there is no XCTAssertEqual(workspace.panels.count, initialCount + 1, …). If the routing consumes the event without dispatching splitRight, this test still passes. testSettingsFileBareKeyChordDispatchesSplitRight (which does check panel count) covers the end-to-end path, but for the withTemporaryShortcut path the assertion gap means regression coverage is weaker than the name suggests.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🧹 Nitpick comments (4)
web/messages/en.json (1)

441-441: Double-tap tmux send-prefix docs look consistent; optionally mention explicit chord precedence.

Line 441 matches the intended “consume first keystroke, send literal on second” behavior. Since the implementation also ensures explicit <leader><leader> chord bindings take precedence over implicit behavior, consider adding a short clause like “If you explicitly bind the leader+leader chord, that configured binding wins.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/messages/en.json` at line 441, Update the "chordsRuleDoubleTap" message
to note that an explicit leader+leader chord binding takes precedence over the
implicit double-tap send behavior; edit the value for the "chordsRuleDoubleTap"
key to append a short clause such as "If you explicitly bind the leader+leader
chord, that configured binding wins" so users know explicit chord bindings
override the default double-tap behavior.
Sources/AppDelegate.swift (1)

5287-5300: Send the actual second-tap text, not the normalized shortcut token.

Line 5299 forwards prefix.key, which is the stored shortcut representation. That can differ from what the user actually typed on the second tap (for example with Caps Lock or layout-dependent bare keys), so <leader><leader> can send the wrong character to the terminal. Prefer event.characters, with prefix.key only as a fallback.

Possible fix
     private func sendLiteralChordPrefixToFocusedSurface(
         prefix: ShortcutStroke,
         event: NSEvent
     ) -> Bool {
@@
         guard let ghosttyView = cmuxOwningGhosttyView(for: responder),
               let surface = ghosttyView.terminalSurface else {
             return false
         }
-        surface.sendText(prefix.key)
+        let literal =
+            (event.characters?.isEmpty == false ? event.characters : nil)
+            ?? (event.charactersIgnoringModifiers?.isEmpty == false ? event.charactersIgnoringModifiers : nil)
+            ?? prefix.key
+        surface.sendText(literal)
         return true
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 5287 - 5300, The
sendLiteralChordPrefixToFocusedSurface function currently forwards prefix.key to
the terminal which is the stored shortcut token; change the send to use the
actual characters from the NSEvent (use event.characters, falling back to
prefix.key if nil/empty) so the real second-tap text (respecting Caps
Lock/layout) is delivered to surface.sendText; keep the existing ghosttyView and
surface lookup (cmuxOwningGhosttyView, terminalSurface) and only replace the
argument passed to surface.sendText.
cmuxTests/AppDelegateShortcutRoutingTests.swift (2)

972-1047: Explicit-binding precedence test should also prove the action fired

Right now this only proves “no literal was sent.” It doesn’t prove the explicit ` + ` chord actually dispatched splitRight. Add a panel-count assertion so this cannot pass on unrelated consumption paths.

Suggested tightening
-        withTemporaryShortcut(action: .splitRight, shortcut: shortcut) {
+        let initialPanelCount = workspace.panels.count
+        withTemporaryShortcut(action: .splitRight, shortcut: shortcut) {
@@
 `#if` DEBUG
             XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: prefixEvent))
             XCTAssertTrue(
                 appDelegate.debugHandleCustomShortcut(event: secondEvent),
                 "Configured `+` chord must dispatch the action"
             )
             XCTAssertEqual(captured, [], "Explicit binding must suppress the implicit literal-send")
 `#else`
             XCTFail("debugHandleCustomShortcut is only available in DEBUG")
 `#endif`
         }
+        RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
+        XCTAssertEqual(
+            workspace.panels.count,
+            initialPanelCount + 1,
+            "Explicit `+` binding should execute splitRight"
+        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/AppDelegateShortcutRoutingTests.swift` around lines 972 - 1047, The
test currently only asserts no literal was sent but doesn't verify the explicit
.splitRight action ran; capture the workspace's panel count (or another reliable
indicator of a split) before invoking the chord in
testBareKeyChordDoubleTapWithExplicitBindingFiresActionInsteadOfLiteral and
assert that the panel count increased (or the expected split result exists)
after the two backtick events inside the withTemporaryShortcut block (use the
same workspace/selectedWorkspace and focusedPanel references and the .splitRight
action) so the test proves the action was dispatched rather than just
suppressing literal-send.

771-833: Assert the split side effect, not only event consumption

This test can pass even if splitRight didn’t run, because it only checks debugHandleCustomShortcut(...) booleans and then discards manager on Line 832. Please assert panel-count delta to prove the action executed.

Suggested tightening
-        withTemporaryShortcut(action: .splitRight, shortcut: shortcut) {
+        let initialPanelCount = manager.selectedWorkspace?.panels.count ?? 0
+        withTemporaryShortcut(action: .splitRight, shortcut: shortcut) {
             guard let prefixEvent = makeKeyDownEvent(
                 key: "`",
                 modifiers: [],
                 keyCode: 50,
                 windowNumber: window.windowNumber
@@
 `#if` DEBUG
             XCTAssertTrue(
                 appDelegate.debugHandleCustomShortcut(event: prefixEvent),
                 "Bare-key chord prefix must be consumed so the terminal does not receive `"
             )
             XCTAssertTrue(
                 appDelegate.debugHandleCustomShortcut(event: actionEvent),
                 "Second stroke after a bare-key chord prefix must dispatch the bound action"
             )
 `#else`
             XCTFail("debugHandleCustomShortcut is only available in DEBUG")
 `#endif`
         }
-        _ = manager
+        RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
+        XCTAssertEqual(
+            manager.selectedWorkspace?.panels.count,
+            initialPanelCount + 1,
+            "Bare-key chord should perform splitRight on second stroke"
+        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/AppDelegateShortcutRoutingTests.swift` around lines 771 - 833, The
test currently only asserts that appDelegate.debugHandleCustomShortcut returned
true but never verifies the split side-effect; capture the panel/tab count from
the TabManager (manager) before firing the prefix/action events and assert the
expected delta after the events to prove splitRight executed (e.g., record let
beforeCount = manager.panelCount or similar accessor, then after handling both
events assert manager.panelCount == beforeCount + 1); keep using the existing
test function testBareKeyChordPrefixArmsAndSplitsOnSecondKey and the existing
withTemporaryShortcut/appDelegate.debugHandleCustomShortcut flow but replace the
discarded reference to manager with an actual before/after assertion to validate
the split action occurred.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/superpowers/plans/2026-04-29-bare-key-chord-leader.md`:
- Around line 19-21: Update the plan text to reflect the actual shipped parser
change: replace the claim that "No parser change needed" with a short note that
StoredShortcut.parseConfig(strokes:) now treats bare-key first strokes in
multi-stroke chords differently (see StoredShortcut.parseConfig(strokes:) and
ShortcutStroke.parseConfig(_:)), and mention that the parser recognizes
backtick/grave tokens as before; also adjust the paragraph about the recorder
(KeyboardShortcutSettings.swift) to clarify that bareKeyNotAllowed still blocks
single-stroke recording but does not prevent bare-key first strokes when parsed
from settings.json. Ensure the plan references the functions
StoredShortcut.parseConfig(strokes:), ShortcutStroke.parseConfig(_:), and the
bareKeyNotAllowed rejection behavior so readers can locate the implemented
behavior.

In `@Sources/AppDelegate.swift`:
- Around line 9979-10000: The cache hasConfiguredBareKeyChordPrefixCache is
being recomputed only against preferredRegisteredMainWindowContext() which
misses bare-key leaders in other live MainWindowContext instances; update
recomputeHasConfiguredBareKeyChordPrefix() to iterate all live registered
MainWindowContext instances and their cmuxConfigStore-backed
configuredCmuxShortcutActions and configuredShortcutChordActions (instead of
using preferredRegisteredMainWindowContext()), checking each
shortcut.firstStroke.modifierFlags.isEmpty for chord prefixes, and return true
if any match; also ensure the cache is invalidated/recomputed when any
MainWindowContext or its cmuxConfigStore changes so window-scoped config updates
correctly refresh the cache.

---

Nitpick comments:
In `@cmuxTests/AppDelegateShortcutRoutingTests.swift`:
- Around line 972-1047: The test currently only asserts no literal was sent but
doesn't verify the explicit .splitRight action ran; capture the workspace's
panel count (or another reliable indicator of a split) before invoking the chord
in testBareKeyChordDoubleTapWithExplicitBindingFiresActionInsteadOfLiteral and
assert that the panel count increased (or the expected split result exists)
after the two backtick events inside the withTemporaryShortcut block (use the
same workspace/selectedWorkspace and focusedPanel references and the .splitRight
action) so the test proves the action was dispatched rather than just
suppressing literal-send.
- Around line 771-833: The test currently only asserts that
appDelegate.debugHandleCustomShortcut returned true but never verifies the split
side-effect; capture the panel/tab count from the TabManager (manager) before
firing the prefix/action events and assert the expected delta after the events
to prove splitRight executed (e.g., record let beforeCount = manager.panelCount
or similar accessor, then after handling both events assert manager.panelCount
== beforeCount + 1); keep using the existing test function
testBareKeyChordPrefixArmsAndSplitsOnSecondKey and the existing
withTemporaryShortcut/appDelegate.debugHandleCustomShortcut flow but replace the
discarded reference to manager with an actual before/after assertion to validate
the split action occurred.

In `@Sources/AppDelegate.swift`:
- Around line 5287-5300: The sendLiteralChordPrefixToFocusedSurface function
currently forwards prefix.key to the terminal which is the stored shortcut
token; change the send to use the actual characters from the NSEvent (use
event.characters, falling back to prefix.key if nil/empty) so the real
second-tap text (respecting Caps Lock/layout) is delivered to surface.sendText;
keep the existing ghosttyView and surface lookup (cmuxOwningGhosttyView,
terminalSurface) and only replace the argument passed to surface.sendText.

In `@web/messages/en.json`:
- Line 441: Update the "chordsRuleDoubleTap" message to note that an explicit
leader+leader chord binding takes precedence over the implicit double-tap send
behavior; edit the value for the "chordsRuleDoubleTap" key to append a short
clause such as "If you explicitly bind the leader+leader chord, that configured
binding wins" so users know explicit chord bindings override the default
double-tap behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b180a0fd-daa5-4b88-a593-1b8a09b3512d

📥 Commits

Reviewing files that changed from the base of the PR and between e181f99 and fa7772d.

📒 Files selected for processing (10)
  • Sources/AppDelegate.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/KeyboardShortcutSettings.swift
  • cmuxTests/AppDelegateShortcutRoutingTests.swift
  • docs/superpowers/plans/2026-04-29-bare-key-chord-leader.md
  • docs/superpowers/specs/2026-04-29-bare-key-chord-leader-design.md
  • web/app/[locale]/docs/configuration/page.tsx
  • web/app/[locale]/docs/keyboard-shortcuts/page.tsx
  • web/messages/en.json
  • web/messages/ja.json

Comment thread docs/superpowers/plans/2026-04-29-bare-key-chord-leader.md Outdated
Comment thread Sources/AppDelegate.swift Outdated
- Recompute hasConfiguredBareKeyChordPrefix across all live
  MainWindowContext instances on every call instead of caching. Caching
  only inspected preferredRegisteredMainWindowContext(), silently missing
  any window whose cmuxConfigStore had bare-key chords but wasn't the
  preferred context at refresh time. The no-cache approach eliminates
  the need for per-context Combine subscriptions (and their teardown on
  context removal): shortcutActions() filters a typically-single-digit
  loadedActions list; in the common no-bare-key case the built-in loop
  short-circuits on the first mismatch and returns false quickly.
- Forward event.characters (with charactersIgnoringModifiers /
  prefix.key fallback) instead of the normalized prefix.key so Caps
  Lock and layout-dependent leaders deliver the actually-typed character.
- chordsRuleDoubleTap mentions that an explicit <leader><leader> user
  binding takes precedence over the implicit literal-send behavior.
Add focused-surface setup and panel-count assertions to
testBareKeyChordPrefixArmsAndSplitsOnSecondKey and
testBareKeyChordDoubleTapWithExplicitBindingFiresActionInsteadOfLiteral,
mirroring the pattern already used by
testSettingsFileBareKeyChordDispatchesSplitRight.
@teamleaderleo teamleaderleo added S3: minor Wrong behavior with a workaround area: input Keyboard, shortcuts, IME, mouse, clipboard and paste labels Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: input Keyboard, shortcuts, IME, mouse, clipboard and paste S3: minor Wrong behavior with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants