Suppress browser editing shortcut replay

[austinywang]

Summary

• Follow-up to Fix Slack composer Cmd+C in browser panes #4126 after review feedback.
• Make the window-level browser document-editing preflight terminal after dispatching Cmd+C/Cmd+X/Cmd+A into the web view, matching the existing Find shortcut no-replay invariant.
• Add window-path regression coverage for focused browser child responders when WebKit declines and when both WebKit and the menu decline.

Tests

• Not run locally per repository and task instructions. CI should run the required checks.

---

Note

Medium Risk
Changes global NSWindow.performKeyEquivalent routing for browser editing shortcuts, which can affect copy/cut/select-all behavior and menu fallback across the app. Risk is mitigated by added regression tests covering focused child responders and menu-decline scenarios.

Overview
Prevents double-dispatch/replay of browser document-editing command equivalents (e.g. Cmd+C) when they are preflighted through a focused CmuxWebView from the window-level performKeyEquivalent path.

After forwarding the shortcut into the web view, the window preflight now always consumes the event (matching the existing Find-family behavior) to avoid falling through and re-triggering WebKit.

Adds window-path unit tests to verify: (1) main-menu fallback still occurs when WebKit declines, and (2) no second web-content replay occurs when both WebKit and the menu decline.

^{Reviewed by Cursor Bugbot for commit 0d5eaa0. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Prevents duplicate dispatch of editing shortcuts (Cmd+C/Cmd+X/Cmd+A) by making the window preflight terminal after sending the event to CmuxWebView, matching the Find shortcut behavior. Fixes Slack Cmd+C copy replays in embedded browsers; connects to Linear issue 4123.

• Bug Fixes
  • Window preflight always returns after forwarding to the web view, preventing a second WebKit replay; main-menu fallback runs inside CmuxWebView.performKeyEquivalent when WebKit declines.
  • Added tests for focused browser child: falls back to main menu when WebKit declines; no second replay when both WebKit and the menu decline.

^{Written for commit 0d5eaa0. Summary will update on new commits. Review in cubic}

Summary by CodeRabbit

• Bug Fixes

  • Improved keyboard shortcut handling in browser contexts to ensure proper fallback to menu actions when web content doesn't handle shortcuts, preventing unnecessary replay of the same key combinations in specific scenarios.

• Tests

  • Added test coverage for keyboard shortcut routing behavior in focused browser windows, including validation of common shortcuts like Cmd+C and their menu interaction.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 18, 2026 6:21am
cmux-staging	Building	Preview, Comment	May 18, 2026 6:21am

[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: e85bae4c-e470-4cc1-987d-711b90f9aea4

📥 Commits

Reviewing files that changed from the base of the PR and between 50b985b and 0d5eaa0.

📒 Files selected for processing (2)

• Sources/AppDelegate.swift
• cmuxTests/BrowserConfigTests.swift

---

📝 Walkthrough

Walkthrough

This PR modifies browser key-equivalent handling in AppDelegate to suppress replay when focused web content declines a shortcut, and adds comprehensive test coverage for the Cmd+C routing behavior in both fallback-to-menu and suppression scenarios.

Changes

Browser Key-Equivalent Suppression

Layer / File(s)	Summary
Browser shortcut suppression behavior Sources/AppDelegate.swift	Modified browser document-editing shortcut path to unconditionally return true and suppress replay when performKeyEquivalent(with:) returns false, changing from previous fallthrough behavior.
Key routing test coverage cmuxTests/BrowserConfigTests.swift	Added two new tests verifying Cmd+C routing when focused browser child web content returns false: one confirms menu fallback occurs and action is invoked; the other confirms suppression occurs and menu action is uninvoked.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• manaflow-ai/cmux#1305: Both PRs modify Sources/AppDelegate.swift key-equivalent routing in the responder-chain path and add companion Cmd shortcut routing tests with similar suppression/redirection logic.
• manaflow-ai/cmux#3398: Both PRs modify AppKit key-equivalent routing in Sources/AppDelegate.swift to change fallback behavior based on handler return values and add tests asserting menu action invocation.
• manaflow-ai/cmux#2356: Both PRs modify the same cmux_performKeyEquivalent routing logic to control whether unclaimed shortcuts fall through to menu handling when web view returns false.

Poem

A rabbit hops through shortcut paths so clear,
When web views decline, we suppress with cheer! 🐰
No replay here—just silence and grace,
And tests to keep chaos out of this place.

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

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preventing replay of browser editing shortcuts, which matches the core fix in the changeset.
Description check	✅ Passed	The description includes a summary section explaining what changed and why, and a testing section stating tests were added and CI will run checks. However, the testing section lacks details on what was verified manually.
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.
Cmux Swift Actor Isolation	✅ Passed	No Swift 6 actor isolation issues found. All changes within @MainActor AppDelegate. No new types, protocols, or implicit MainActor declarations. Tests properly annotated. No isolation debt introduced.
Cmux Swift Blocking Runtime	✅ Passed	No blocking/timing primitives introduced. Production code changes control flow only. Test additions use allowed @MainActor scaffolding, not blocking patterns.
Cmux No Hacky Sleeps	✅ Passed	Check not applicable. PR modifies only Swift files. The 'runtime-no-hacky-sleeps' rule applies to TypeScript, JavaScript, shell, and non-Swift scripts, excluding Swift code.
Cmux Swift Concurrency	✅ Passed	PR introduces no legacy async patterns. Changes are synchronous keyboard event routing in AppDelegate and standard XCTest cases. @MainActor on tests is required AppKit/XCTest boundary.
Cmux Swift @Concurrent	✅ Passed	No @concurrent violations found. Changes are synchronous keyboard handling with properly annotated @MainActor test methods. No async, concurrent misuse, or boundary issues detected.
Cmux Swift File And Package Boundaries	✅ Passed	Net +1 line AppDelegate (existing oversized file, touched incidentally) and +108 test lines. Changes are AppKit glue within AppDelegate's core responsibility. Complies with allowed categories.
Cmux Swift Logging	✅ Passed	Logging added uses cmuxDebugLog guarded by #if DEBUG, complying with swift-logging.md rules. No prohibited logging methods in production code. No secrets exposed.
Cmux User-Facing Error Privacy	✅ Passed	No user-facing error messages, alerts, or privacy violations detected. Debug logging is properly guarded by #if DEBUG and only appears in debug builds, not production.
Cmux Swiftui State Layout	✅ Passed	PR contains only AppKit-based changes. No SwiftUI views, state declarations, or state management patterns introduced. Changes are not subject to swiftui-state-layout rules.
Cmux Architecture Rethink	✅ Passed	Small correctness fix preventing duplicate WebKit exposure. Window-level handler now terminal after forwarding to CmuxWebView. Clear invariant, single owner, no timing repairs/observers/side channels.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	The PR changes keyboard shortcut routing in AppDelegate. Test fixtures in BrowserConfigTests are allowed by rule.

_{✏️ 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-4123-slack-cmd-c-copy

---

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

Makes the window-level browser document-editing shortcut preflight (Cmd+C/Cmd+X/Cmd+A) terminal after forwarding to CmuxWebView, matching the existing Find-shortcut invariant; the main-menu fallback now runs inside CmuxWebView.performKeyEquivalent rather than on a second pass through the normal NSWindow path.

• Sources/AppDelegate.swift: Removes the conditional if result { return true } guard and replaces it with an unconditional return true, with a comment explaining that CmuxWebView.performKeyEquivalent already runs the menu fallback before returning; the debug log retains the result binding to report whether WebKit or the menu resolved the shortcut.
• cmuxTests/BrowserConfigTests.swift: Adds two @MainActor window-path regression tests — one verifying menu fallback when a focused browser child declines, and one confirming that no second WebKit replay occurs when both WebKit and an empty menu decline.

Confidence Score: 5/5

Safe to merge — the one-line production change matches the pre-existing Find-shortcut invariant exactly, and the new tests exercise both new window-path branches directly.

The production delta is a targeted removal of a conditional guard that previously allowed the window dispatch path to replay WebKit; the unconditional return true mirrors the identical pattern already in place for the Find command. CmuxWebView.performKeyEquivalent handles the menu fallback internally, and existing tests validate that invariant. The two new window-path tests exercise both outcomes (menu handles, menu misses) and neither the production nor test changes introduce new state, new singletons, or new timing dependencies.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/AppDelegate.swift	Single-block change: unconditional return true replaces the conditional if result guard, matching the Find-shortcut no-replay invariant; debug log still reports result via the retained let result binding.
cmuxTests/BrowserConfigTests.swift	Adds two @MainActor window-path regression tests covering focused-browser-child scenarios: menu-fallback when WebKit declines, and replay suppression when both WebKit and the menu decline.

Sequence Diagram

sequenceDiagram
    participant AppKit as NSWindow<br/>(swizzled)
    participant CmuxWV as CmuxWebView<br/>performKeyEquivalent
    participant WKWebKit as WKWebView<br/>(WebKit)
    participant Menu as NSApp.mainMenu

    Note over AppKit: Cmd+C pressed, focused<br/>browser child responder

    AppKit->>CmuxWV: performKeyEquivalent(Cmd+C)
    CmuxWV->>WKWebKit: forward Cmd+C

    alt WebKit handles it
        WKWebKit-->>CmuxWV: true
        CmuxWV-->>AppKit: true
        Note over AppKit: return true (suppressed)
    else WebKit declines
        WKWebKit-->>CmuxWV: false
        CmuxWV->>Menu: performKeyEquivalent(Cmd+C)
        alt Menu handles it
            Menu-->>CmuxWV: true
            CmuxWV-->>AppKit: true
        else Menu declines
            Menu-->>CmuxWV: false
            CmuxWV-->>AppKit: false
        end
        Note over AppKit: return true regardless<br/>(no WebKit replay)
    end

Loading

_{Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}