Skip to content

Fix terminal Cmd+V clipboard payload handling - #1305

Merged
lawrencecchen merged 2 commits into
mainfrom
task-cmd-v-paste-shortcut
Mar 13, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
task-cmd-v-paste-shortcut

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Mar 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add regression tests for HTML-only, attachment-only HTML, and mixed text-plus-image clipboard payloads used by terminal paste
  • extract plain text from HTML/RTF/RTFD payloads, but treat attachment-only rich text as image fallback input instead of dropping the paste
  • write existing public.png clipboard bytes directly so browser image pastes avoid the extra image re-encode on the paste path

Testing

  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-task-cmd-v-paste-shortcut-unit -only-testing:cmuxTests/GhosttyPasteboardHelperTests test

Task

  • Cmd+V can fail or hitch in some terminal states with browser-style clipboard payloads, even though Ctrl+V still pastes.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@vercel

vercel Bot commented Mar 13, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 13, 2026 11:45am

@coderabbitai

coderabbitai Bot commented Mar 13, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Added two new functions to handle Command-key editing actions (cut/copy/paste/selectAll/undo/redo) through the responder chain, with mainMenu fallback. Modified the key equivalent routing to attempt these actions before delegating to the main menu. Added comprehensive tests for paste scenarios.

Changes

Cohort / File(s) Summary
Command-Key Editing Action Handling
Sources/AppDelegate.swift
Introduced standardCommandEditingAction() to identify editing actions from Command key combinations and performCommandEditingAction() to execute them via the responder chain with optional fallback. Extended shouldRouteCommandEquivalentDirectlyToMainMenu to attempt these actions before main menu handling. Added safe optional chaining for mainMenu access and diagnostic logging.
Test Coverage
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Added CommandEquivalentResponderFallbackTests with two test cases validating Cmd+V routing through terminal responders and direct paste invocation. Included helper class PasteProbeGhosttyView and window/menu setup utilities.

Sequence Diagram

sequenceDiagram
    participant Event as NSEvent (Cmd+V)
    participant AppDelegate
    participant SCE as standardCommandEditingAction
    participant PCA as performCommandEditingAction
    participant Responder as Responder Chain
    participant MainMenu as NSApp.mainMenu
    
    Event->>AppDelegate: shouldRouteCommandEquivalentDirectlyToMainMenu
    AppDelegate->>SCE: Detect editing action from key combo
    SCE-->>AppDelegate: Return Selector (paste:)
    
    alt Action detected
        AppDelegate->>PCA: Perform action on first responder
        PCA->>Responder: Attempt on responder chain
        
        alt Responder handles it
            Responder-->>PCA: Success
            PCA-->>AppDelegate: Return true
        else Responder fails
            PCA->>Responder: Attempt on fallback responder
            
            alt Fallback handles it
                Responder-->>PCA: Success
                PCA-->>AppDelegate: Return true
            else All responders fail
                PCA-->>AppDelegate: Return false
            end
        end
    else No action
        AppDelegate->>MainMenu: performKeyEquivalent
        MainMenu-->>AppDelegate: Result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #562: Implements paste(_:) behavior in GhosttyTerminalView, which is the responder-chain endpoint that this PR's new performCommandEditingAction() invokes.
  • PR #792: Modifies AppDelegate's keyboard dispatch logic in overlapping areas of command-equivalent routing.
  • PR #717: Changes command-key routing decisions in AppDelegate and NSWindow for menu vs. responder chain vs. terminal forwarding.

Poem

🐰 A rabbit's delight in Command-key flow,
Where paste now finds its responder below!
No menu delays when a chain can reply,
Cut, copy, undo—let commands quickly fly! ✨
Tests ensure each keystroke lands true and right,
Editing actions now shine oh-so-bright! 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing a responder fallback mechanism for terminal Cmd+V handling.
Description check ✅ Passed The description includes a detailed summary and testing instructions, but lacks demo video, bot review requests, and a complete checklist as specified in the template.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch task-cmd-v-paste-shortcut
📝 Coding Plan
  • Generate coding plan for human review comments

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 2 files

@greptile-apps

greptile-apps Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a regression where Cmd+V could fail to paste in terminal (Ghostty) focused states because the main-menu bypass path had no fallback for when NSApp.mainMenu.performKeyEquivalent returned false. The fix adds two free functions — standardCommandEditingAction (maps well-known Cmd editing chords to selectors) and performCommandEditingAction (dispatches via the responder chain with an optional fallback) — and wires them into the existing shouldRouteCommandEquivalentDirectlyToMainMenu bypass block in the swizzled NSWindow.performKeyEquivalent.

Key changes:

  • standardCommandEditingAction(_:) maps Cmd+X/C/V/A/Z/Y and Cmd+Shift+Z to NSText / NSUndoManager selectors, returning nil for any unrecognised chord so the fall-through path is unaffected.
  • performCommandEditingAction(_:startingAt:fallbackResponder:) calls NSResponder.tryToPerform on the first responder (which walks the responder chain), then on a fallback responder only when the two objects differ — preventing double-dispatch when both point to the same GhosttyNSView.
  • The integration test uses a minimal main menu that intentionally has no paste item, confirms window.performKeyEquivalent returns true, and checks pasteInvocationCount == 1 on the probe view.

Confidence Score: 4/5

  • Safe to merge; logic is narrow, well-tested, and the only observable behavioural change is correctly routing a small set of standard editing shortcuts that were previously dropped.
  • The responder-chain dispatch is standard AppKit, the identity guard prevents double-invocation, and both the unit and integration tests cover the fix's critical path. The one mild flag is that Cmd+Y → redo: is an uncommon macOS mapping and could shadow a terminal keybinding if a GhosttyNSView subclass ever handles redo:, but the current codebase shows no such handler and the action is only attempted after the menu declines it.
  • No files require special attention.

Important Files Changed

Filename Overview
Sources/AppDelegate.swift Adds standardCommandEditingAction to map Cmd+X/C/V/A/Z/Y/Shift+Z to standard editing selectors, and performCommandEditingAction to dispatch via the responder chain with an optional fallback. Integrates both into the window key-equivalent bypass path so that when the main menu doesn't consume a Cmd keystroke in terminal-focused windows, editing actions are attempted before falling through to the original dispatch.
cmuxTests/CmuxWebViewKeyEquivalentTests.swift Adds CommandEquivalentResponderFallbackTests with two tests: an integration test verifying that Cmd+V reaches a GhosttyNSView paste responder when the main menu has no paste item, and a unit test verifying performCommandEditingAction dispatches paste without double-invoking when startingAt === fallbackResponder.

Sequence Diagram

sequenceDiagram
    participant User as User (Cmd+V)
    participant Window as NSWindow<br/>performKeyEquivalent (swizzled)
    participant Menu as NSApp.mainMenu<br/>performKeyEquivalent
    participant SCA as standardCommandEditingAction()
    participant PCA as performCommandEditingAction()
    participant FR as self.firstResponder<br/>(responder chain)
    participant GV as firstResponderGhosttyView<br/>(GhosttyNSView)
    participant Orig as cmux_performKeyEquivalent<br/>(original NSWindow path)

    User->>Window: keyDown Cmd+V
    Window->>Window: cmuxOwningGhosttyView(firstResponder)?
    Note over Window: firstResponderGhosttyView != nil → bypass SwiftUI path
    Window->>Window: shouldRouteCommandEquivalentDirectlyToMainMenu? → true
    Window->>Menu: performKeyEquivalent(Cmd+V)
    Menu-->>Window: false (no paste item in menu)
    Window->>SCA: standardCommandEditingAction(event)
    SCA-->>Window: #selector(NSText.paste(_:))
    Window->>PCA: performCommandEditingAction(paste:, startingAt: FR, fallbackResponder: GV)
    PCA->>FR: tryToPerform(paste:, with: nil)
    Note over FR: walks responder chain
    FR->>GV: paste(nil)
    GV-->>PCA: true
    PCA-->>Window: true
    Window-->>User: return true (event consumed)

    Note over Window,Orig: If no editing action matched OR tryToPerform returned false:<br/>fall through to cmux_performKeyEquivalent (original dispatch)
Loading

Last reviewed commit: 6bb936d

@lawrencecchen
lawrencecchen force-pushed the task-cmd-v-paste-shortcut branch from 6bb936d to 4d4f5d3 Compare March 13, 2026 11:44
@lawrencecchen lawrencecchen changed the title Fix terminal Cmd+V responder fallback Fix terminal Cmd+V clipboard payload handling Mar 13, 2026
@lawrencecchen
lawrencecchen merged commit e94daa0 into main Mar 13, 2026
11 checks passed
@lawrencecchen
lawrencecchen deleted the task-cmd-v-paste-shortcut branch March 13, 2026 11:46
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
* Add clipboard payload regression tests

* Fix terminal clipboard payload handling
@coderabbitai coderabbitai Bot mentioned this pull request Apr 10, 2026
4 of 6 tasks

This branch was successfully deployed

1 active deployment
Preview — 4d4f5d3b Deployed Mar 13, 2026 by vercel[bot]
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.

1 participant