fix: resolve the terminal Copy guard through the Command-aware keyboard layout (#10872)

[aliyansajid]

Fixes #10872.

Problem

On the macOS Dvorak - QWERTY ⌘ layout, pressing ⌘C in a terminal pane with no selection types a stray j into the pty instead of doing nothing.

consumeUnavailableCopyMenuAction exists to swallow exactly this: when nothing is selected AppKit disables Edit ▸ Copy, the main menu declines the chord, and the guard keeps it a native no-op so the failed Copy binding never reaches Ghostty's terminal-input path.

The guard resolved the pressed character with KeyboardLayout.normalizedCharacters(for:), which returns charactersIgnoringModifiers — the character the key produces with no modifiers. AppKit matches menu key equivalents through the layout's Command table instead. On a Command-swapped layout the two disagree:

Layout	Physical key	charactersIgnoringModifiers	What AppKit's menu matched
Dvorak - QWERTY ⌘	C	j	c → Copy
Dvorak - QWERTY ⌘	I	c	i

So the guard missed the real ⌘C (the j leaks into the pty — the reported bug), and it also fired on ⌘I, silently swallowing that chord. Both directions come from the same comparison.

Fix

Resolve the guard the same way AppKit resolved the menu: ask the layout for the character with the event's Command flag applied, via KeyboardLayout.character(forKeyCode:modifierFlags:), which already translates in shortcut mode and is documented to preserve command-aware layouts. The previous charactersIgnoringModifiers path stays as the fallback when the layout has no mapping, so behavior is unchanged on US-QWERTY and on the non-Latin input sources normalizedCharacters was added for.

Plain Dvorak (no Command table swap) keeps matching Copy on the physical I key, which is correct — that really is where ⌘C lives on that layout, and it is what the menu matched.

Tests

Per CLAUDE.md, split into two commits so CI shows the test catching the bug:

1. test: — adds coverage; the two Dvorak-QWERTY ⌘ cases fail.
2. fix: — the one-branch behavior change; all cases pass.

The character check had to move into GhosttyNSView.isStandardCopyMenuKeyEquivalent(_:layoutCharacterProvider:) to be testable, because consumeUnavailableCopyMenuAction returns early without a live Ghostty surface, so the gate can't be observed through it headlessly. The injectable provider mirrors the existing KeyboardShortcutSettings.matches(event:layoutCharacterProvider:) pattern and means no keyboard layout has to be installed on the test host. Commit 1 keeps the extracted logic byte-for-byte equivalent to the old inline check, so the behavior change is isolated to commit 2.

Seven cases in WindowKeyDownReplayGuardTests: both Dvorak-QWERTY ⌘ directions, plain Dvorak, US-QWERTY, a non-Latin (Hangul) input source, ⌘V, and ⌘⇧C.

✔ dvorakQwertyCommandCopyKeyMatchesTheCopyKeyEquivalent
✔ dvorakQwertyCommandDoesNotMatchCopyOnTheDvorakCKey
✔ usQwertyCopyKeyMatchesTheCopyKeyEquivalent
✔ plainDvorakCopyKeyMatchesTheCopyKeyEquivalent
✔ nonLatinInputSourceStillMatchesTheCopyKeyEquivalent
✔ otherCommandKeysDoNotMatchTheCopyKeyEquivalent
✔ copyKeyWithExtraModifiersDoesNotMatchTheCopyKeyEquivalent

No user-facing strings changed, so no localization audit was required.

Two unrelated problems I hit on main, not touched here

Both reproduce on e87efeafec with no local changes, and I left both alone to keep this PR to one concern — happy to open issues or follow-up PRs:

1. The cmuxTests bundle does not compile: cmuxTests/CLISSHPTYAttachProbeReplyRegressionTests.swift:140 fails with "the compiler is unable to type-check this expression in reasonable time" on a six-way string concatenation. I broke that expression up locally (uncommitted, reverted) purely to run the suite. This looks related to main: cmuxTests bundle fails to compile (MobilePairingConnectionTransitionTests vs MobilePairingModel after #12754) #12827.
2. terminalHostedEditableResponderKeepsLocalUndo fails on undoCallCount == 1. It fails identically before and after this change and never reaches the Copy guard.

---

Summary by cubic

Fixes the terminal Copy guard on the macOS Dvorak - QWERTY ⌘ layout so pressing ⌘C with no selection no longer types a stray "j" into the pty. The guard previously compared charactersIgnoringModifiers, which disagrees with the character AppKit's menu matched on Command-swapped layouts; it now resolves the key through the layout's Command table, matching how AppKit declined the Copy chord.

• The guard no longer swallows ⌘I on that layout, which it was doing before.
• US-QWERTY, plain Dvorak, and non-Latin input sources behave exactly as before.
• Adds seven tests covering both Dvorak-QWERTY ⌘ directions, plain Dvorak, US-QWERTY, Hangul input, ⌘V, and ⌘⇧C.

^{Written for commit d8d1f72. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Improved Copy keyboard shortcut detection across different keyboard layouts, including Dvorak-QWERTY, plain Dvorak, non-Latin layouts, and standard US QWERTY.
  • Ensured only the standard Command-C shortcut triggers Copy, while combinations with extra modifiers or other keys do not.

• Tests

  • Added regression coverage for keyboard-layout-specific Copy shortcut behavior.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

Warning

Review limit reached

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6b870b1a-2e7f-40b9-920b-de990414ef0f

📥 Commits

Reviewing files that changed from the base of the PR and between 127cc93 and d8d1f72.

📒 Files selected for processing (2)

• Sources/GhosttyTerminalView.swift
• cmuxTests/WindowKeyDownReplayGuardTests.swift

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e8242e2b-f877-41ff-aaf6-7d605702c8e8

📥 Commits

Reviewing files that changed from the base of the PR and between 6565b59 and 127cc93.

📒 Files selected for processing (2)

• Sources/GhosttyTerminalView.swift
• cmuxTests/WindowKeyDownReplayGuardTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The Copy menu guard now uses Command-aware keyboard layout resolution. Regression tests cover Dvorak-QWERTY, Dvorak, US-QWERTY, non-Latin input, non-Copy keys, and extra modifiers.

Changes

Copy key equivalent resolution

Layer / File(s)	Summary
Command-aware Copy resolution Sources/GhosttyTerminalView.swift, cmuxTests/WindowKeyDownReplayGuardTests.swift	Adds the layout-aware Copy key equivalent helper and tests keyboard-layout, modifier, and fallback behavior.
Copy guard integration Sources/GhosttyTerminalView.swift	Updates consumeUnavailableCopyMenuAction(_:) to use the new helper.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: austinywang

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 1 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing Copy guard resolution through the Command-aware keyboard layout.
Description check	✅ Passed	The description is detailed and relevant. It explains the problem, fix, testing, regression coverage, and unrelated test issues. The template checklist, review-trigger block, and demo video are not in…
Linked Issues check	✅ Passed	The changes satisfy issue #10872. GhosttyNSView.isStandardCopyMenuKeyEquivalent requires a key-down event with only Command, then resolves the physical key with `KeyboardLayout.character(...modifier…
Out of Scope Changes check	✅ Passed	The changes stay within issue #10872. The new helper isolates the Copy-key check, and the injectable provider supports focused regression tests. The test additions cover the linked issue and the requi…
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS: The custom check is not applicable to this pull request. The authoritative diff changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The implemen…
Cmux Swift Actor Isolation	✅ Passed	PASS. The production diff only adds a key-equivalent helper and delegates the existing GhosttyNSView event path to it. It introduces no value model, async service protocol, Sendable reference type…
Cmux Swift Blocking Runtime	✅ Passed	PASS — The production diff adds only synchronous keyboard-layout and modifier-flag resolution, then delegates the existing Copy guard to that helper. The test additions use deterministic event/provide…
Cmux Browser Automation Off-Main	✅ Passed	PASS. The PR changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The diff adds keyboard-layout Copy-key detection and regression tests. It adds no `br…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The review-range diff changes only Sources/GhosttyTerminalView.swift and tests. The production change adds keyboard-layout character resolution and delegates consumeUnavailableCopyMenuAction…
Cmux Cache Substitution Correctness	✅ Passed	PASS: The PR changes only Swift event/key-equivalent handling and tests. It replaces an inline keyboard-layout character check with a Command-aware layout provider; it does not replace a fresh read wi…
Cmux No Hacky Sleeps	✅ Passed	PASS. The pull request changes only two .swift files: Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The custom check applies to non-Swift TypeScript, JavaS…
Cmux Algorithmic Complexity	✅ Passed	PASS: The production diff adds a per-key-event character check in Sources/GhosttyTerminalView.swift:5719-5747. Its allSatisfy calls inspect the single key-equivalent string, not a scalable collect…
Cmux Swift Concurrency	✅ Passed	PASS — The pull request adds only synchronous key-equivalent resolution and synchronous XCTest coverage. The changed production method performs modifier checks and layout-character lookup; it adds no …
Cmux Swift @Concurrent	✅ Passed	PASS. The reviewed diff adds only synchronous Swift code: GhosttyNSView.isStandardCopyMenuKeyEquivalent and synchronous test/provider helpers. It introduces no nonisolated async, @concurrent, as…
Cmux Swift Package Boundaries	✅ Passed	PASS — the production change is a small AppKit/Ghostty menu guard in GhosttyNSView, which the boundary rules explicitly allow to remain in the app target. isStandardCopyMenuKeyEquivalent accepts `…
Cmux Swiftpm Lockfiles	✅ Passed	PASS. The authoritative PR diff changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. It contains no Package.swift, Package.resolved, .gitignore, …
Cmux Swift Logging	✅ Passed	The pull request changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The production change adds keyboard-layout resolution and changes the Copy guard;…
Cmux User-Facing Error Privacy	✅ Passed	The production diff only adds keyboard-layout predicate logic and delegates the existing Copy guard to it. It adds no user-facing errors, alerts, command output, recovery copy, or diagnostic payloads.…
Cmux Full Internationalization	✅ Passed	PASS: The pull request changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The production change adds keyboard-equivalent logic and developer comments…
Cmux Swiftui State Layout	✅ Passed	PASS: The pull request changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The production change adds a static helper to GhosttyNSView, an NSView …
Cmux Architecture Rethink	✅ Passed	PASS. The diff is a small local keyboard-layout correctness fix. It keeps ownership in GhosttyNSView, replaces the existing inline guard on the same AppDelegate path, and states the invariant: mat…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS. The PR changes only keyboard-event handling in GhosttyNSView and adds copy-key tests. It introduces or materially changes no user-visible NSWindow, NSPanel, NSWindowController, SwiftUI `…
Cmux Source Artifacts	✅ Passed	PASS: The authoritative diff changes only Sources/GhosttyTerminalView.swift and cmuxTests/WindowKeyDownReplayGuardTests.swift. The changes are hand-written Swift source and regression tests for th…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The production change does not add a prohibited test or debug seam. Sources/GhosttyTerminalView.swift adds isStandardCopyMenuKeyEquivalent, which implements production Copy-key behavior and is cal…

Full details: Docstring Coverage

Explanation

Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 1 files. (1 skipped: 1 too large.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.}

[teamleaderleo]

Merged, thank you @aliyansajid! Cmd+C on Command-swapped layouts like Dvorak no longer types a stray "j" into the terminal :D

[github-actions]

Merge receipt for d8d1f727f2: every check was green at merge (13 verified; 13 skipped by policy). Full suite runs on main after merge.