Repository navigation
Fix OpenCode bracketed paste fallback in terminal - #2971
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWindow-level command-key routing now defers to the focused Ghostty view after an app-menu miss; the view prepares its surface before paste actions and consumes key-equivalents when appropriate. Tests add runtime swizzles and probes to verify single-invocation paste behavior and surface-recreation semantics. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Window as NSWindow
participant PB as Pasteboard
participant View as GhosttyNSView
participant Surface as Surface
User->>Window: press Cmd+V / Cmd+Shift+V
Window->>Window: detect paste-equivalent (modifiers + key)
Window->>Window: attempt mainMenu.performKeyEquivalent() -> miss
Window->>PB: query for GHOSTTY_CLIPBOARD_STANDARD
PB-->>Window: has data
Window->>View: focused -> performKeyEquivalentAfterMenuMiss(event)
View->>View: prepareSurfaceForPaste(reason)
View->>Surface: ensureSurfaceReadyForInput()
alt surface ready
View->>View: paste() / pasteAsPlainText()
View-->>Window: handled (return true)
else surface missing
View->>View: requestInputRecoveryAfterSurfaceMiss(reason)
View-->>Window: not handled -> fallback routing
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes a regression where Confidence Score: 5/5Safe to merge — the paste fallback is logically correct and cannot cause double-paste; the only finding is a test organization style concern. All remaining findings are P2 style/organization. The fix itself is tight: the new block is gated on ghosttyView focus, exact paste-command keycode (9 = V key), and pasteboard content, so it can't fire spuriously. The main-menu path still runs first; the fallback only activates on a miss. The two-commit structure (test first, fix second) matches the CLAUDE.md regression test policy. No data loss, no security concern, no double-paste risk. cmuxTests/CJKIMEInputTests.swift — new test is in a misnamed class/file, purely organizational. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[cmux_performKeyEquivalent called] --> B{firstResponderGhosttyView != nil?}
B -- No --> G[cmux_performKeyEquivalent original]
B -- Yes --> C{IME composing + no Cmd?}
C -- Yes --> D[Delegate to original path]
C -- No --> E{Has .command flag?}
E -- No --> F[ghosttyView.performKeyEquivalent]
E -- Yes --> H{shouldRouteToMainMenu?}
H -- Yes, menu exists --> I[mainMenu.performKeyEquivalent]
I -- consumed --> J[return true]
I -- missed --> K{NEW: paste command + pasteboard has content?}
H -- No or no menu --> K
K -- Yes --> L[ghosttyView.paste / pasteAsPlainText]
L --> M[return true — bracketed paste preserved]
K -- No --> G
G --> N[return result]
Reviews (1): Last reviewed commit: "fix: preserve terminal paste when menu s..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 1691-1696: The global paste hook ghosttyPasteActionHook is being
used for this test and may be invoked by other terminals; change the hook
closure to only increment pasteInvocationCount when the provided terminal view
matches this test's surfaceView (similar to the IME swizzle tests). Keep
previousPasteHook/defer as-is, and scope the check inside the closure to guard
on surfaceView so only paste events for this specific terminal view affect
pasteInvocationCount.
- Around line 1725-1735: The test currently calls
window.performKeyEquivalent(with: event) and then asserts pasteInvocationCount
and forwardedCommandVCount without guaranteeing hostedTerminal.surface stays
alive; wrap the performKeyEquivalent call and the subsequent assertions in
withExtendedLifetime(hostedTerminal.surface) so the surface isn't deallocated in
optimized builds—specifically, keep hostedTerminal.surface alive while invoking
window.performKeyEquivalent(with: event) and while asserting on
pasteInvocationCount and forwardedCommandVCount.
🪄 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: afd45ac1-b9e8-4ecd-aa08-c975beb6d0ff
📒 Files selected for processing (2)
Sources/AppDelegate.swiftcmuxTests/CJKIMEInputTests.swift
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cmuxTests/CJKIMEInputTests.swift (2)
1725-1735:⚠️ Potential issue | 🟡 MinorKeep the terminal surface alive through the Cmd+V assertions.
This test extracts
windowandsurfaceView, then no longer useshostedTerminal.surface; optimized builds can release it before theperformKeyEquivalentpath completes.🧪 Proposed fix
- XCTAssertTrue(window.performKeyEquivalent(with: event)) - XCTAssertEqual( - pasteInvocationCount, - 1, - "Cmd+V should still invoke the terminal paste action even if the window main-menu fast path misses" - ) - XCTAssertEqual( - forwardedCommandVCount, - 0, - "Cmd+V should not fall back to Ghostty keyDown when the terminal paste action is available" - ) + withExtendedLifetime(hostedTerminal.surface) { + XCTAssertTrue(window.performKeyEquivalent(with: event)) + XCTAssertEqual( + pasteInvocationCount, + 1, + "Cmd+V should still invoke the terminal paste action even if the window main-menu fast path misses" + ) + XCTAssertEqual( + forwardedCommandVCount, + 0, + "Cmd+V should not fall back to Ghostty keyDown when the terminal paste action is available" + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/CJKIMEInputTests.swift` around lines 1725 - 1735, The test releases hostedTerminal.surface before the Cmd+V assertions, allowing optimized builds to drop the terminal surface mid-path; retain a strong reference to the terminal surface (e.g., assign hostedTerminal.surface to a local variable) before calling window.performKeyEquivalent(with:) and hold that variable through the XCTAssertEqual checks so the terminal surface remains alive while evaluating pasteInvocationCount and forwardedCommandVCount; update the test to capture the surface into a local constant prior to performKeyEquivalent to ensure the surface isn't deallocated during the assertions.
1691-1696:⚠️ Potential issue | 🟡 MinorScope the global paste hook to this test’s terminal view.
ghosttyPasteActionHookis global after swizzling, so an unrelated terminal paste during either test can increment the local counter.🧪 Proposed fix
- ghosttyPasteActionHook = { _, _ in + ghosttyPasteActionHook = { candidateView, _ in + guard candidateView === surfaceView else { return } pasteInvocationCount += 1 }Also applies to: 1760-1765
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/CJKIMEInputTests.swift` around lines 1691 - 1696, The global ghosttyPasteActionHook is being set for the whole process, so unrelated terminal paste events can increment pasteInvocationCount; instead, wrap the new hook to only increment when the pasted terminal matches this test's terminal view (e.g., check the hook's terminalView parameter against the test's TerminalView / terminal variable) while still preserving and calling previousPasteHook, and apply the same scoped-hook pattern to the other occurrence around the second test; restore the original hook in the defer as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 13-14: The tests only swizzle and observe GhosttyNSView.paste(_:)
but must also cover the pasteAsPlainText(_:) fallback; add a parallel swizzle
and hook (mirror the existing ghosttyPasteActionSwizzled and
ghosttyPasteActionHook logic and the installGhosttyPasteActionSwizzle()
implementation) for pasteAsPlainText(_:), expose a
ghosttyPasteAsPlainTextActionHook or similar, and update the test cases to
assert that invoking the Cmd+Shift+V key-equivalent routes through
pasteAsPlainText(_:) (i.e., install the new swizzle before the test and verify
the hook is called when simulating Cmd+Shift+V).
- Around line 1789-1798: The test must also assert that the recovered-surface
path did not forward the Cmd+V to keyDown: after calling
window.performKeyEquivalent(with: event) and asserting pasteInvocationCount == 1
and hostedTerminal.surface.surface is non-nil, add the same check used in the
prior test to verify forwardedCommandVCount == 0 (or the observer/assert that
inspects forwardedCommandVCount) to ensure the released/recreated surface did
not forward the command to keyDown; update the test that references
window.performKeyEquivalent, pasteInvocationCount,
hostedTerminal.surface.surface to include this additional assertion/observer.
---
Duplicate comments:
In `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 1725-1735: The test releases hostedTerminal.surface before the
Cmd+V assertions, allowing optimized builds to drop the terminal surface
mid-path; retain a strong reference to the terminal surface (e.g., assign
hostedTerminal.surface to a local variable) before calling
window.performKeyEquivalent(with:) and hold that variable through the
XCTAssertEqual checks so the terminal surface remains alive while evaluating
pasteInvocationCount and forwardedCommandVCount; update the test to capture the
surface into a local constant prior to performKeyEquivalent to ensure the
surface isn't deallocated during the assertions.
- Around line 1691-1696: The global ghosttyPasteActionHook is being set for the
whole process, so unrelated terminal paste events can increment
pasteInvocationCount; instead, wrap the new hook to only increment when the
pasted terminal matches this test's terminal view (e.g., check the hook's
terminalView parameter against the test's TerminalView / terminal variable)
while still preserving and calling previousPasteHook, and apply the same
scoped-hook pattern to the other occurrence around the second test; restore the
original hook in the defer as before.
🪄 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: ab1487cb-75b6-456a-b6d4-53d951d25703
📒 Files selected for processing (3)
Sources/AppDelegate.swiftSources/GhosttyTerminalView.swiftcmuxTests/CJKIMEInputTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:14541">
P1: The direct Cmd+V fallback is no longer always consumed; when `prepareSurfaceForPaste` returns false, the event falls through to normal keyDown routing, which can reintroduce the paste-shortcut miss behavior this block is intended to prevent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmuxTests/CJKIMEInputTests.swift (1)
1709-1951: Add a negative test for the empty-clipboard fallback gate.The new tests cover the text-present path, but the fallback is supposed to consume Cmd+V/Cmd+Shift+V only when clipboard text exists. A no-string pasteboard case would catch regressions where the window fallback consumes paste shortcuts and blocks normal routing even though there is nothing to paste.
🧪 Suggested coverage direction
+ func testCommandVWithEmptyPasteboardDoesNotInvokeTerminalPasteWhenMainMenuMisses() throws { + installGhosttyPasteActionSwizzle() + + let hostedTerminal = try makeHostedTerminalWindow() + let terminalSurface = hostedTerminal.surface + let window = hostedTerminal.window + let surfaceView = hostedTerminal.surfaceView + defer { window.orderOut(nil) } + + window.makeFirstResponder(surfaceView) + + let previousMainMenu = NSApp.mainMenu + NSApp.mainMenu = installUnrelatedMainMenu() + defer { NSApp.mainMenu = previousMainMenu } + + let pasteboard = NSPasteboard.general + let pasteboardSnapshot = snapshotPasteboardItems(pasteboard) + defer { restorePasteboardItems(pasteboardSnapshot, to: pasteboard) } + pasteboard.clearContents() + + var pasteInvocationCount = 0 + let previousPasteHook = ghosttyPasteActionHook + ghosttyPasteActionHook = { candidateView, sender in + previousPasteHook?(candidateView, sender) + guard candidateView === surfaceView else { return } + pasteInvocationCount += 1 + } + defer { ghosttyPasteActionHook = previousPasteHook } + + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "v", + charactersIgnoringModifiers: "v", + isARepeat: false, + keyCode: 9 + ) else { + XCTFail("Failed to construct Cmd+V event") + return + } + + withExtendedLifetime(terminalSurface) { + XCTAssertFalse(window.performKeyEquivalent(with: event)) + XCTAssertEqual(pasteInvocationCount, 0) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/CJKIMEInputTests.swift` around lines 1709 - 1951, Add a negative test that mirrors the existing Cmd+V/Cmd+Shift+V tests but leaves the NSPasteboard empty to exercise the “empty-clipboard fallback gate”: create a test (e.g., testCommandVPasteDoesNotInvokeTerminalFallbackWhenClipboardEmpty) that installs installGhosttyPasteActionSwizzle(), makes the hosted terminal and surfaceView first responder, replaces NSApp.mainMenu with installUnrelatedMainMenu(), clears the pasteboard (no setString call), installs a ghosttyPasteActionHook / ghosttyPasteAsPlainTextActionHook and a GhosttyNSView.debugGhosttySurfaceKeyEventObserver, synthesize the Cmd+V and Cmd+Shift+V events using NSEvent.keyEvent and call window.performKeyEquivalent(with:), then assert that pasteInvocationCount and pasteAsPlainTextInvocationCount remain 0 while forwardedCommandVCount increments (i.e., the fallback does not consume the shortcut when clipboard has no text); use the same helper names (ghosttyPasteActionHook, ghosttyPasteAsPlainTextActionHook, GhosttyNSView.debugGhosttySurfaceKeyEventObserver, window.performKeyEquivalent) so the test is consistent with the other cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/CJKIMEInputTests.swift`:
- Around line 1709-1951: Add a negative test that mirrors the existing
Cmd+V/Cmd+Shift+V tests but leaves the NSPasteboard empty to exercise the
“empty-clipboard fallback gate”: create a test (e.g.,
testCommandVPasteDoesNotInvokeTerminalFallbackWhenClipboardEmpty) that installs
installGhosttyPasteActionSwizzle(), makes the hosted terminal and surfaceView
first responder, replaces NSApp.mainMenu with installUnrelatedMainMenu(), clears
the pasteboard (no setString call), installs a ghosttyPasteActionHook /
ghosttyPasteAsPlainTextActionHook and a
GhosttyNSView.debugGhosttySurfaceKeyEventObserver, synthesize the Cmd+V and
Cmd+Shift+V events using NSEvent.keyEvent and call
window.performKeyEquivalent(with:), then assert that pasteInvocationCount and
pasteAsPlainTextInvocationCount remain 0 while forwardedCommandVCount increments
(i.e., the fallback does not consume the shortcut when clipboard has no text);
use the same helper names (ghosttyPasteActionHook,
ghosttyPasteAsPlainTextActionHook,
GhosttyNSView.debugGhosttySurfaceKeyEventObserver, window.performKeyEquivalent)
so the test is consistent with the other cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e3e0331f-c065-4aea-9d69-cf04d70e3459
📒 Files selected for processing (1)
cmuxTests/CJKIMEInputTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
6532-6542:⚠️ Potential issue | 🟠 MajorAdd guard for Cmd+V and Cmd+Shift+V before
keyDownfallback to prevent double-firing paste throughinterpretKeyEvents.When
shouldRetryMainMenuis false, the menu has already been tried and missed. CallingkeyDown(with:)unconditionally will invokeinterpretKeyEventsandghostty_surface_keyon the same key event, recreating the double-fire issue the comment at line 6528–6531 warns against. No AppDelegate hardware-V fallback intercepts these shortcuts before this branch, so they reachkeyDownwhen the Edit menu lacks a paste item. The proposed guard correctly invokespaste(_:)orpasteAsPlainText(_:)directly afterprepareSurfaceForPaste, matching the behavior that Edit > Paste would have performed.Proposed guard for hardware paste shortcuts before keyDown fallback
if shouldRetryMainMenu && isConsumed && !isAll && keySequence.isEmpty && keyTables.isEmpty { if let menu = NSApp.mainMenu, menu.performKeyEquivalent(with: event) { return true } } + if !shouldRetryMainMenu, + event.keyCode == UInt16(kVK_ANSI_V), + (flags == [.command] || flags == [.command, .shift]), + GhosttyPasteboardHelper.hasString(for: GHOSTTY_CLIPBOARD_STANDARD) { + let isPlainTextPaste = flags == [.command, .shift] + guard prepareSurfaceForPaste( + reason: isPlainTextPaste + ? "pasteAsPlainText.keyEquivalent.missingSurface" + : "paste.keyEquivalent.missingSurface" + ) else { + return false + } + if isPlainTextPaste { + pasteAsPlainText(nil) + } else { + paste(nil) + } + return true + } + // For performable bindings where the menu didn't handle the event, // fall through to keyDown so Ghostty can perform the action directly // (e.g. paste when no menu item exists). keyDown(with: event)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 6532 - 6542, The fallback branch currently calls keyDown(with:) unconditionally which causes interpretKeyEvents and ghostty_surface_key to double-fire paste for hardware Cmd+V/Cmd+Shift+V; add a guard that detects the hardware paste shortcuts (Cmd+V and Cmd+Shift+V) before calling keyDown: if detected, call prepareSurfaceForPaste() then invoke paste(_:) for Cmd+V or pasteAsPlainText(_:) for Cmd+Shift+V and return true, otherwise proceed to keyDown(with: event) as before; update logic near shouldRetryMainMenu and ensure you reference keyDown(with:), prepareSurfaceForPaste(), paste(_:), pasteAsPlainText(_:), interpretKeyEvents and ghostty_surface_key in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/AppDelegateShortcutRoutingTests.swift`:
- Around line 14-32: Test only exercises the "Ghostty handled" branch because
GhosttyCommandEquivalentProbeView defaults performAfterMenuMissResult = true and
is never toggled; add a complementary test that sets
probe.performAfterMenuMissResult = false, triggers the same key-equivalent flow,
and asserts that window-level handlers were invoked by checking pasteCallCount
and pasteAsPlainTextCallCount on GhosttyCommandEquivalentProbeView (the class
overrides performKeyEquivalentAfterMenuMiss(with:), paste(_:), and
pasteAsPlainText(_:), so locate and reuse those symbols to flip the boolean and
assert the fallback behavior).
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 6532-6542: The fallback branch currently calls keyDown(with:)
unconditionally which causes interpretKeyEvents and ghostty_surface_key to
double-fire paste for hardware Cmd+V/Cmd+Shift+V; add a guard that detects the
hardware paste shortcuts (Cmd+V and Cmd+Shift+V) before calling keyDown: if
detected, call prepareSurfaceForPaste() then invoke paste(_:) for Cmd+V or
pasteAsPlainText(_:) for Cmd+Shift+V and return true, otherwise proceed to
keyDown(with: event) as before; update logic near shouldRetryMainMenu and ensure
you reference keyDown(with:), prepareSurfaceForPaste(), paste(_:),
pasteAsPlainText(_:), interpretKeyEvents and ghostty_surface_key in the change.
🪄 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: 134b95d4-fcec-4340-993f-90a6c3e4a3d1
📒 Files selected for processing (3)
Sources/AppDelegate.swiftSources/GhosttyTerminalView.swiftcmuxTests/AppDelegateShortcutRoutingTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/AppDelegate.swift
| private final class GhosttyCommandEquivalentProbeView: GhosttyNSView { | ||
| var afterMenuMissCallCount = 0 | ||
| var pasteCallCount = 0 | ||
| var pasteAsPlainTextCallCount = 0 | ||
| var performAfterMenuMissResult = true | ||
|
|
||
| override func performKeyEquivalentAfterMenuMiss(with event: NSEvent) -> Bool { | ||
| afterMenuMissCallCount += 1 | ||
| return performAfterMenuMissResult | ||
| } | ||
|
|
||
| override func paste(_ sender: Any?) { | ||
| pasteCallCount += 1 | ||
| } | ||
|
|
||
| override func pasteAsPlainText(_ sender: Any?) { | ||
| pasteAsPlainTextCallCount += 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect GhosttyNSView initializers and any init-time side effects that could
# make direct instantiation from a test fixture unsafe.
ast-grep --pattern $'class GhosttyNSView: $_ {
$$$
}'
# Find all initializers of GhosttyNSView to see what runs on init(frame:).
rg -nP --type=swift -C5 '\bclass\s+GhosttyNSView\b|override\s+init\s*\(frame\b|required\s+init\?\(coder\b'
# Check for any override of performKeyEquivalentAfterMenuMiss to confirm the
# method exists on GhosttyNSView (and see its default behavior).
rg -nP --type=swift -C3 '\bperformKeyEquivalentAfterMenuMiss\s*\('Repository: manaflow-ai/cmux
Length of output: 33790
🏁 Script executed:
#!/bin/bash
# Read the full GhosttyNSView init implementations and setup method
sed -n '5390,5450p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 3160
🏁 Script executed:
#!/bin/bash
# Find installEventMonitor and updateTrackingAreas implementations
rg -n "private func installEventMonitor|func updateTrackingAreas" Sources/GhosttyTerminalView.swift -A 20Repository: manaflow-ai/cmux
Length of output: 2270
🏁 Script executed:
#!/bin/bash
# Check if there's deinit cleanup for the event monitor
rg -n "deinit" Sources/GhosttyTerminalView.swift -A 15 | head -80Repository: manaflow-ai/cmux
Length of output: 3699
🏁 Script executed:
#!/bin/bash
# Check GhosttyNSView deinit to see if event monitor is cleaned up
sed -n '5131,5180p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 1947
🏁 Script executed:
#!/bin/bash
# Search for event monitor cleanup
rg -n "eventMonitor" Sources/GhosttyTerminalView.swift | grep -E "(nil|removeMonitor|removeLocalMonitor)"Repository: manaflow-ai/cmux
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check where eventMonitor is removed and the overall cleanup pattern
rg -n "eventMonitor\s*=" Sources/GhosttyTerminalView.swift -B2 -A2Repository: manaflow-ai/cmux
Length of output: 355
🏁 Script executed:
#!/bin/bash
# Check what class/context the eventMonitor removal is in
sed -n '8350,8365p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 702
Test only exercises the "Ghostty handled" branch.
The probe defaults performAfterMenuMissResult = true and is never toggled, so the window routing is only verified for the case where Ghostty's binding resolution claims the event. The complementary path — performKeyEquivalentAfterMenuMiss returning false, causing the window-level V fallback to invoke paste(_:) / pasteAsPlainText(_:) — is not asserted in this file. Per the PR summary, that positive path is covered elsewhere (paste-action swizzling, pasteboard snapshot/restore), so this test's narrow "don't preempt Ghostty" assertion is appropriate on its own.
GhosttyCommandEquivalentProbeView(frame:) directly instantiates GhosttyNSView rather than using the surfaceView(in:) lookup elsewhere in the file. This is safe: setup() registers a global scroll-wheel event monitor (cleaned up properly in deinit), sets up tracking areas, and registers drag types—none of which require a Ghostty surface to be present. The probe view has no surface assigned and doesn't access self.surface, so there's no risk of crashes or state leakage into subsequent tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmuxTests/AppDelegateShortcutRoutingTests.swift` around lines 14 - 32, Test
only exercises the "Ghostty handled" branch because
GhosttyCommandEquivalentProbeView defaults performAfterMenuMissResult = true and
is never toggled; add a complementary test that sets
probe.performAfterMenuMissResult = false, triggers the same key-equivalent flow,
and asserts that window-level handlers were invoked by checking pasteCallCount
and pasteAsPlainTextCallCount on GhosttyCommandEquivalentProbeView (the class
overrides performKeyEquivalentAfterMenuMiss(with:), paste(_:), and
pasteAsPlainText(_:), so locate and reuse those symbols to flip the boolean and
assert the fallback behavior).
|
Hi, thanks for the fix! I'm on v0.63.2 and experiencing the single-character paste issue in OpenCode. This PR is merged but the latest nightly on GitHub Releases is still from Feb 15 (commit 3698f13), which predates this fix. Any chance a new nightly build could be published so we can pick it up? Thanks! |
v0.63.1 |
…ode-paste-first-char Fix OpenCode bracketed paste fallback in terminal
This branch was successfully deployed
4 participants
Summary
Cmd+Vwhen the window-level main-menu bypass missesCmd+Vfall through to generic keyDown routingCloses #2966
Summary by cubic
Fixes Cmd+V/Cmd+Shift+V so a missed main‑menu path first runs Ghostty’s keybinding resolution, then falls back to terminal paste, preserving bracketed paste for TUIs like OpenCode. Also recreates the terminal surface before consuming the shortcut so paste works after a transient surface release.
Written for commit a90bc54. Summary will update on new commits.
Summary by CodeRabbit
Note
Medium Risk
Adjusts macOS key-equivalent routing for terminal-focused windows, which can affect global/menu shortcuts and paste behavior across layouts; covered by new regression tests but still touches input handling paths.
Overview
Fixes a terminal paste regression where Cmd+V / Cmd+Shift+V could bypass the terminal paste actions after the window’s direct-to-menu key-equivalent fast path misses, causing the event to fall through to generic
keyDownand break bracketed paste in TUIs.The window now retries Ghostty’s command-binding resolution after a main-menu miss via
performKeyEquivalentAfterMenuMiss, and the terminal view splitsperformKeyEquivalentinto a main-menu-retry vs post-miss path. Paste actions (paste,pasteAsPlainText) now ensure/recover the underlying surface before executing.Adds targeted regression tests that simulate main-menu misses, swizzle paste actions to assert single invocation, snapshot/restore the pasteboard, and verify surface recreation works when the runtime surface was released.
Reviewed by Cursor Bugbot for commit a90bc54. Bugbot is set up for automated code reviews on this repo. Configure here.