Repository navigation
Fix three terminal clipboard copy failures (Cmd+C swallowed on agent TUI panes, empty writes clearing the clipboard, VT-export capture race) - #8064
Conversation
The one-shot standardClipboardWriteCapture armed by readTerminalTextFromVTExportForSnapshot (session snapshots, mobile replay, agent naming) diverted the NEXT standard-clipboard write from any source. A user copy (Cmd+C, copy-on-select, or copy mode) racing an in-flight VT export was captured instead of reaching NSPasteboard, silently leaving the clipboard empty. The capture now carries a predicate and only consumes writes that match it; non-matching writes pass through to the real pasteboard with the capture left armed. The VT-export caller matches only a single absolute path (or file URL) to an existing file — the shape write_screen_file / write_active_file produce — so user copies can no longer be swallowed. Also: check the previously ignored setString result (retry once and log on failure, instead of leaving the clipboard cleared with nothing written), log pass-through writes for diagnosis, and inject the standard pasteboard so tests cover the write path without touching the real general pasteboard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
…ard writes Two more clipboard failures found while dogfooding on a Claude Code (agent TUI) pane, where output redraws continuously: 1. validateUserInterfaceItem gated Copy on ghostty_surface_has_selection, which can report false while the runtime still holds a live selection under constant TUI redraw (the selection highlight stays visible and copy-on-select still produces the full text). The disabled menu item then swallows Cmd+C at key-equivalent dispatch, so the runtime's own copy binding never runs. Repro fingerprint: Cmd+C only worked while the Edit menu was held open (menu tracking lets the key fall through to the surface). Copy is now enabled whenever a surface exists; copying with no selection remains a harmless no-op. 2. writeString cleared the pasteboard before writing even for an empty payload (e.g. copy-on-select firing after the selection was already invalidated), silently destroying whatever the user last copied — paste turns up empty and clipboard managers record nothing. Empty standard writes are now ignored and logged. Also adds DEBUG-level cmuxDebugLog probes on the ghostty write-clipboard callback, mirroring the existing read-side logging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
|
@a05031113 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughClipboard capture now filters writes by payload, preserves unrelated clipboard writes, supports injected pasteboards, and validates VT export paths before capture. Ghostty clipboard diagnostics and copy command availability were also updated. ChangesClipboard capture and export handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalController
participant TerminalPasteboardService
participant NSPasteboard
TerminalController->>TerminalPasteboardService: arm capture with exported-path predicate
TerminalPasteboardService->>NSPasteboard: pass through unrelated clipboard write
TerminalPasteboardService->>TerminalController: return matching exported file path
TerminalPasteboardService->>NSPasteboard: write later clipboard payload
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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 several terminal clipboard failures. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "Reject overlapping clipboard captures in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if armed.accepts(string) { | ||
| standardClipboardWriteCaptureLock.lock() | ||
| if standardClipboardWriteCapture === armed { | ||
| standardClipboardWriteCapture = nil | ||
| } | ||
| standardClipboardWriteCaptureLock.unlock() | ||
| armed.capture(string) | ||
| return |
There was a problem hiding this comment.
Two matching clipboard callbacks can both read the same armed capture and pass accepts before either clears the slot. The callback that loses the identity check still calls armed.capture and returns, so it overwrites the first result and swallows its own write; concurrent VT exports can therefore return the wrong path while neither write reaches the pasteboard.
| // leaving the clipboard empty. Re-declare and retry once so the | ||
| // failure is at least not silent data loss. | ||
| pasteboard.clearContents() | ||
| let retried = pasteboard.setString(string, forType: .string) |
There was a problem hiding this comment.
If another clipboard callback writes successfully after this callback's first setString fails, the retry calls clearContents again and deletes that newer value. Concurrent terminal copies can therefore leave the clipboard empty or replace the most recent copy with the older retry.
| var isDirectory = ObjCBool(false) | ||
| return fileManager.fileExists(atPath: path, isDirectory: &isDirectory) | ||
| && !isDirectory.boolValue |
There was a problem hiding this comment.
The predicate accepts any existing absolute file, not only the file created by the active VT export. If a user copies /etc/hosts or another existing path while capture is armed, that copy is swallowed and the snapshot reads the unrelated file as terminal output instead of waiting for the export's write.
| var isDirectory = ObjCBool(false) | ||
| return fileManager.fileExists(atPath: path, isDirectory: &isDirectory) | ||
| && !isDirectory.boolValue |
There was a problem hiding this comment.
This predicate runs synchronously inside the Ghostty clipboard callback. An absolute path on a slow network or FUSE volume makes fileExists block terminal runtime processing while capture is armed, causing input or rendering stalls; the callback should perform only a cheap export-owned shape check and defer filesystem validation.
Rule Used: Flag production Swift that reads, decodes, or scan... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift`:
- Around line 130-139: Update the matching branch in TerminalPasteboardService’s
standard clipboard write handling so armed.capture(string) and the early return
occur only after this thread successfully clears standardClipboardWriteCapture
under the lock. If the identity check fails because another thread or the
disarming defer won the race, unlock and continue to the normal pasteboard write
path without swallowing the value.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eaa32b1e-6883-4441-81c1-ecf135644e5a
📒 Files selected for processing (6)
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swiftSources/GhosttyTerminalView.swiftSources/TerminalController.swiftcmuxTests/SessionPersistenceTests.swift
…-string export predicate - writeString: only the write that atomically clears the one-shot slot captures; a concurrent matching write that loses the claim race falls through to the pasteboard instead of being swallowed (Greptile P1, CodeRabbit critical). - Retry after a failed setString only runs while changeCount still matches our clearContents, so it can never clobber a newer concurrent write (Greptile P1). - isPlausibleExportedScreenPath: replaced the fileExists check with a pure string prefix check against the process temporary directory (accepting both /var and /private/var spellings) — no filesystem I/O inside the ghostty runtime callback, and existing absolute paths a user copies (e.g. /etc/hosts) no longer match (Greptile P1 + P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
|
Addressed the automated review findings in 8eb7394:
🤖 Generated with Claude Code |
| ) -> String? { | ||
| let capture = ClipboardWriteCapture(accepts: predicate) | ||
| standardClipboardWriteCaptureLock.lock() | ||
| standardClipboardWriteCapture = capture |
There was a problem hiding this comment.
nil, while the second can read the first pane's export. The atomic claim in writeString prevents two callbacks from consuming one capture, but it does not prevent separate capture operations from stealing each other's writes. Captures need export-specific ownership, or overlapping operations must be serialized or rejected.
…rites Arming a capture while another is in flight replaced the process-wide slot: with predicate-scoped captures, the first operation's export write could satisfy the second capture (both predicates accept export paths), handing pane A's export to pane B's snapshot while pane A got nil. captureNextStandardClipboardWrite now returns nil without arming when the slot is already held. Every caller already treats nil as "fall back to a non-capture read", so an overlapping export degrades gracefully instead of cross-delivering content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
|
Addressed the follow-up finding in 8f81791:
The earlier inline comments on 🤖 Generated with Claude Code |
|
Bumped into the same issue. Can this get merged? |
|
@bogdanovich Thanks for confirming — sorry you hit it too. Quick workaround until this lands: after selecting, hold the Edit menu open and press ⌘C (a disabled Copy menu item otherwise swallows the key equivalent before the terminal's own copy binding can run), or enable copy-on-select in settings. Branch is refreshed against latest main (f440d86, no conflicts; the pasteboard files had no upstream changes) and re-verified locally: GhosttyKit + app build green, clipboard capture tests pass. Ready for maintainer review / CI approval. 🤖 Generated with Claude Code |
Upstream replaced the single-string clipboard write path with writeRepresentations (all textual MIME variants on one pasteboard item). The predicate-gated capture claim, empty-payload guard, and changeCount-guarded retry now live in writeRepresentations — matched against the preferred plain-text representation — with writeString forwarding as upstream intends. The retry rebuilds the pasteboard item, since an item cannot be attached twice. Both test suites are kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
ghostty_surface_has_selection can report false while the runtime still holds a live selection, for example under constant TUI redraw in agent panes. Gating the Copy menu item on it greyed Copy out and swallowed Cmd+C. Enable Copy whenever a surface exists; copying with no selection is a no-op. Salvaged from #8064. Co-authored-by: a05031113 <a05031113@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for this, @a05031113! Your Cmd+C fix (keeping Copy enabled so it works in agent TUI panes when Ghostty misses the selection) just landed in #14557 with you as co-author. The other two pieces here were covered by the #8838 pasteboard rewrite, so I'll close this one. Really appreciate it :D |
3 participants
Summary
Copying terminal text failed intermittently — and on agent TUI panes (Claude Code) almost always. Dogfooding with DEBUG probes on a live Claude Code pane isolated three independent bugs that stack:
1. Cmd+C swallowed by stale selection validation (main user-facing failure)
On a pane whose TUI redraws continuously,
ghostty_surface_has_selectionreportsfalsewhile the runtime still holds a live selection — the highlight stays visible and copy-on-select still produces the full text.validateUserInterfaceItemgated Copy on that flag, so the Edit ▸ Copy item goes disabled, and a disabled menu item swallows ⌘C at key-equivalent dispatch — the event never reaches the surface, so the runtime's owncopy_to_clipboardbinding can't run either.Repro fingerprint: select text in a pane running Claude Code → Copy is greyed in the Edit menu and ⌘C is a silent no-op — but holding the Edit menu open and pressing ⌘C copies fine (menu tracking lets the key fall through to the surface).
Fix: enable Copy whenever a surface exists. Copying with no selection remains a harmless no-op.
2. Empty clipboard writes destroy the clipboard
TerminalPasteboardService.writeStringranclearContents()before writing even when the payload was empty (e.g. copy-on-select firing after the selection was already invalidated by a redraw). The user's previous clipboard content is silently destroyed — paste turns up empty and clipboard managers record nothing. Empty standard writes are now ignored (and logged).3. VT-export clipboard capture swallows concurrent user copies
The one-shot
standardClipboardWriteCapturearmed byreadTerminalTextFromVTExportForSnapshot(session snapshots, mobile replay, agent naming — all frequent on agent panes) diverted the next standard-clipboard write from any source. A user copy racing an in-flight VT export was captured instead of reachingNSPasteboard. The capture now carries a predicate and only consumes writes that match the export payload shape (a single absolute path to an existing file); non-matching writes pass through with the capture left armed. Also checks the previously ignoredsetStringresult (retry once + log), and injects the standard pasteboard so tests cover the write path without touching the real general pasteboard.Tests
CmuxTerminalTests: non-matching writes pass through an armed capture and leave it armed; empty writes don't clear existing content; standard writes land on the injected pasteboard after the capture disarms.cmuxTests/SessionPersistenceTests:isPlausibleExportedScreenPathaccepts an existing file path / file URL and rejects user text, missing paths, and directories.Note on the two-commit red/green policy: the regression tests exercise seams introduced by the fixes themselves (
matching:predicate, injected standard pasteboard), so a tests-only first commit would not compile. Happy to restructure where feasible if preferred.Localization audit
No user-facing strings added or changed (code comments and DEBUG-only
cmuxDebugLogprobes only), so noLocalizable.xcstrings/ web message catalog changes are needed.Verification
Built with
./scripts/reload.sh --tag fix-clipboardand dogfooded on a live Claude Code pane on macOS 26.5: select → ⌘C → paste now works with the Edit menu closed; copy-on-select content survives subsequent selection-clear events; package syntax verified and behavior confirmed by the reporting user across multiple rounds.🤖 Generated with Claude Code
https://claude.ai/code/session_01SwGyYRt9QjDUhimcp2xB7E
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes three clipboard copy failures in terminal panes, especially fast‑redrawing agent TUIs. Cmd+C now works reliably, VT‑export no longer swallows user copies, empty writes don’t wipe the clipboard, and overlapping captures are rejected; fixes now live in
writeRepresentationsand cover all text formats.writeRepresentations(match on preferredtext/plain); inject the standard pasteboard in tests.Written for commit 3b24426. Summary will update on new commits.
Summary by CodeRabbit