Fix three terminal clipboard copy failures (Cmd+C swallowed on agent TUI panes, empty writes clearing the clipboard, VT-export capture race)

Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift

@@ -1,6 +1,7 @@
 public import AppKit
 public import CmuxTerminalCore
 public import GhosttyKit
+internal import os
 
 /// The terminal's pasteboard capability: clipboard reads and writes for the
 /// ghostty runtime, plus materialization of pasteboard images into owned
@@ -23,13 +24,23 @@ public import GhosttyKit
 /// temp-file set and the one-shot write capture), the sanctioned shape for
 /// state shared with synchronous callbacks.
 public final class TerminalPasteboardService: Sendable {
-    /// One-shot interception slot for ``captureNextStandardClipboardWrite(_:)``.
+    /// One-shot interception slot for ``captureNextStandardClipboardWrite(matching:_:)``.
     final class ClipboardWriteCapture: Sendable {
         private let lock = NSLock()
         // SAFETY: guarded by `lock`; written by the runtime's write-clipboard
         // callback thread and read by the capturing caller.
         nonisolated(unsafe) private var capturedValue: String?
 
+        /// Predicate deciding whether a standard-clipboard write belongs to
+        /// this capture. Writes it rejects pass through to the real
+        /// pasteboard and leave the capture armed, so an unrelated write
+        /// (e.g. a user copy racing a VT export) is not swallowed.
+        let accepts: @Sendable (String) -> Bool
+
+        init(accepts: @escaping @Sendable (String) -> Bool) {
+            self.accepts = accepts
+        }
+
         /// Stores the diverted clipboard string.
         func capture(_ value: String) {
             lock.lock()
@@ -57,6 +68,16 @@ public final class TerminalPasteboardService: Sendable {
     // ghostty runtime threads.
     nonisolated(unsafe) private let selectionPasteboard: NSPasteboard
 
+    // SAFETY: immutable reference; same argument as `selectionPasteboard`.
+    // Injectable so tests can exercise standard-location writes without
+    // touching the real general pasteboard.
+    nonisolated(unsafe) private let standardPasteboard: NSPasteboard
+
+    private static let logger = Logger(
+        subsystem: "com.cmuxterm.app",
+        category: "terminal.pasteboard"
+    )
+
     /// The directory that owned temporary image files are written into.
     let temporaryDirectory: URL
 
@@ -74,11 +95,19 @@ public final class TerminalPasteboardService: Sendable {
 
     /// Creates the process's pasteboard service.
     ///
-    /// - Parameter temporaryDirectory: Destination for owned temporary image
-    ///   files. Tests inject a scratch directory; the app uses the user's
-    ///   temporary directory.
-    public init(temporaryDirectory: URL = FileManager.default.temporaryDirectory) {
+    /// - Parameters:
+    ///   - temporaryDirectory: Destination for owned temporary image files.
+    ///     Tests inject a scratch directory; the app uses the user's
+    ///     temporary directory.
+    ///   - standardPasteboard: The pasteboard backing the standard clipboard
+    ///     location. Tests inject a scratch pasteboard; the app uses
+    ///     `NSPasteboard.general`.
+    public init(
+        temporaryDirectory: URL = FileManager.default.temporaryDirectory,
+        standardPasteboard: NSPasteboard = .general
+    ) {
         self.temporaryDirectory = temporaryDirectory
+        self.standardPasteboard = standardPasteboard
         self.selectionPasteboard = NSPasteboard(
             name: NSPasteboard.Name("com.mitchellh.ghostty.selection")
         )
@@ -88,43 +117,90 @@ public final class TerminalPasteboardService: Sendable {
 extension TerminalPasteboardService: TerminalClipboardWriting {
     /// Publishes all textual representations as one pasteboard item, honoring
     /// an armed one-shot capture for the standard location.
+    ///
+    /// An armed capture only consumes writes its predicate accepts (matched
+    /// against the preferred plain-text representation); any other
+    /// standard-location write (e.g. a user copy racing a VT export) passes
+    /// through to the real pasteboard with the capture left armed.
     public func writeRepresentations(
         _ representations: [TerminalClipboardRepresentation],
         to location: ghostty_clipboard_e
     ) {
         guard !representations.isEmpty else { return }
+        let preferredValue = representations.first(where: {
+            normalizedTerminalClipboardMIMEType($0.mimeType) == "text/plain"
+        })?.string ?? representations[0].string
 
         if location == GHOSTTY_CLIPBOARD_STANDARD {
-            var capture: ClipboardWriteCapture?
             standardClipboardWriteCaptureLock.lock()
-            capture = standardClipboardWriteCapture
-            if capture != nil {
-                standardClipboardWriteCapture = nil
-            }
+            let armed = standardClipboardWriteCapture
             standardClipboardWriteCaptureLock.unlock()
 
-            if let capture {
-                let value = representations.first(where: {
-                    normalizedTerminalClipboardMIMEType($0.mimeType) == "text/plain"
-                })?.string ?? representations[0].string
-                capture.capture(value)
-                return
+            if let armed {
+                if armed.accepts(preferredValue) {
+                    // Claim the one-shot slot atomically: only the write that
+                    // actually clears it may capture. A concurrent matching
+                    // write that loses this race falls through to the real
+                    // pasteboard instead of overwriting the captured value
+                    // and being swallowed.
+                    standardClipboardWriteCaptureLock.lock()
+                    let claimed = standardClipboardWriteCapture === armed
+                    if claimed {
+                        standardClipboardWriteCapture = nil
+                    }
+                    standardClipboardWriteCaptureLock.unlock()
+                    if claimed {
+                        armed.capture(preferredValue)
+                        return
+                    }
+                } else {
+                    Self.logger.info(
+                        "standard write passed through armed capture (length \(preferredValue.count, privacy: .public))"
+                    )
+                }
             }
         }
 
+        // An empty payload (e.g. copy-on-select firing after a TUI redraw
+        // already invalidated the selection) must not clear the clipboard:
+        // clearContents-then-write-nothing silently destroys whatever the
+        // user last copied.
+        guard !preferredValue.isEmpty else {
+            Self.logger.info("ignored empty clipboard write")
+            return
+        }
+
         guard let pasteboard = pasteboard(for: location) else { return }
-        let item = NSPasteboardItem()
-        var writtenTypes = Set<NSPasteboard.PasteboardType>()
-        for representation in representations {
-            let type = terminalPasteboardType(forMIMEType: representation.mimeType)
-            guard writtenTypes.insert(type).inserted else { continue }
-            _ = item.setString(
-                representation.string,
-                forType: type
+        // A pasteboard item can only be attached to one pasteboard, so the
+        // retry path below needs a freshly built copy.
+        let makeItem: () -> NSPasteboardItem = {
+            let item = NSPasteboardItem()
+            var writtenTypes = Set<NSPasteboard.PasteboardType>()
+            for representation in representations {
+                let type = terminalPasteboardType(forMIMEType: representation.mimeType)
+                guard writtenTypes.insert(type).inserted else { continue }
+                _ = item.setString(
+                    representation.string,
+                    forType: type
+                )
+            }
+            return item
+        }
+        let clearedChangeCount = pasteboard.clearContents()
+        if !pasteboard.writeObjects([makeItem()]) {
+            // A contended pasteboard can reject the write after clearContents,
+            // leaving the clipboard empty. Retry once so the failure is not
+            // silent data loss — but only while nothing else has written in
+            // the meantime, so the retry never clobbers a newer value.
+            var retried = false
+            if pasteboard.changeCount == clearedChangeCount {
+                pasteboard.clearContents()
+                retried = pasteboard.writeObjects([makeItem()])
+            }
+            Self.logger.error(
+                "pasteboard write failed (length \(preferredValue.count, privacy: .public)), retry \(retried ? "succeeded" : "skipped-or-failed", privacy: .public)"
             )
         }
-        pasteboard.clearContents()
-        _ = pasteboard.writeObjects([item])
     }
 
     /// Writes a string to the given ghostty clipboard location, honoring an
@@ -136,14 +212,39 @@ extension TerminalPasteboardService: TerminalClipboardWriting {
         )
     }
 
-    /// Arms a one-shot diversion of the next standard-clipboard write that
-    /// happens while `action` runs, returning the diverted string.
+    /// Arms a one-shot diversion of the next matching standard-clipboard
+    /// write that happens while `action` runs, returning the diverted string.
+    ///
+    /// - Parameters:
+    ///   - predicate: Decides whether a given standard-clipboard write is the
+    ///     one this capture is waiting for. Non-matching writes reach the
+    ///     real pasteboard and leave the capture armed, so unrelated writers
+    ///     (a user copy, another surface) are not swallowed. Pass a predicate
+    ///     as narrow as the expected payload allows — e.g. "an existing file
+    ///     under the temporary directory" for a VT screen export.
+    ///   - action: The operation expected to trigger the write.
+    ///
+    /// Returns `nil` without running a capture when another capture is
+    /// already in flight: replacing the armed slot would let one operation's
+    /// write satisfy the other's capture (both predicates accept export
+    /// paths), handing the wrong content to the wrong caller. Callers
+    /// already treat `nil` as "fall back to a non-capture read".
     @discardableResult
-    public func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String? {
-        let capture = ClipboardWriteCapture()
+    public func captureNextStandardClipboardWrite(
+        matching predicate: @escaping @Sendable (String) -> Bool = { _ in true },
+        _ action: () -> Bool
+    ) -> String? {
+        let capture = ClipboardWriteCapture(accepts: predicate)
         standardClipboardWriteCaptureLock.lock()
-        standardClipboardWriteCapture = capture
+        let alreadyArmed = standardClipboardWriteCapture != nil
+        if !alreadyArmed {
+            standardClipboardWriteCapture = capture
+        }
         standardClipboardWriteCaptureLock.unlock()
+        guard !alreadyArmed else {
+            Self.logger.info("clipboard capture rejected: another capture is in flight")
+            return nil
+        }
 
         defer {
             standardClipboardWriteCaptureLock.lock()
@@ -186,7 +287,7 @@ extension TerminalPasteboardService {
     public func pasteboard(for location: ghostty_clipboard_e) -> NSPasteboard? {
         switch location {
         case GHOSTTY_CLIPBOARD_STANDARD:
-            return .general
+            return standardPasteboard
         case GHOSTTY_CLIPBOARD_SELECTION:
             return selectionPasteboard
         default:

Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift

@@ -139,6 +139,67 @@ struct ClipboardWriteCaptureTests {
         #expect(service.hasString(for: GHOSTTY_CLIPBOARD_SELECTION))
     }
 
+    @Test func nonMatchingWritePassesThroughAndLeavesCaptureArmed() throws {
+        let scratchDir = try makeScratchDirectory()
+        defer { try? FileManager.default.removeItem(at: scratchDir) }
+        let scratch = ScratchPasteboard()
+        let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard)
+        let exportFile = scratchDir.appendingPathComponent("export.vt")
+        try Data("screen".utf8).write(to: exportFile)
+        let userCopy = "user copied text \(UUID().uuidString)"
+
+        let captured = service.captureNextStandardClipboardWrite(
+            matching: { $0 == exportFile.path }
+        ) {
+            // A user copy racing the export must reach the pasteboard...
+            service.writeString(userCopy, to: GHOSTTY_CLIPBOARD_STANDARD)
+            // ...and the capture must stay armed for the export's own write.
+            service.writeString(exportFile.path, to: GHOSTTY_CLIPBOARD_STANDARD)
+            return true
+        }
+
+        #expect(captured == exportFile.path)
+        #expect(scratch.pasteboard.string(forType: .string) == userCopy)
+    }
+
+    @Test func overlappingCaptureIsRejectedAndDoesNotStealTheFirstCapturesWrite() {
+        let service = TerminalPasteboardService()
+        let outer = service.captureNextStandardClipboardWrite { () -> Bool in
+            // A second capture while one is in flight must be rejected...
+            let inner = service.captureNextStandardClipboardWrite { true }
+            #expect(inner == nil)
+            // ...and the original capture still owns the next matching write.
+            service.writeString("outer-value", to: GHOSTTY_CLIPBOARD_STANDARD)
+            return true
+        }
+        #expect(outer == "outer-value")
+    }
+
+    @Test func emptyStandardWriteDoesNotClearExistingClipboardContent() {
+        let scratch = ScratchPasteboard()
+        let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard)
+        let marker = "existing-content-\(UUID().uuidString)"
+        service.writeString(marker, to: GHOSTTY_CLIPBOARD_STANDARD)
+
+        // An empty payload (e.g. copy-on-select firing after the selection
+        // was already invalidated) must be a no-op, not a destructive clear.
+        service.writeString("", to: GHOSTTY_CLIPBOARD_STANDARD)
+
+        #expect(scratch.pasteboard.string(forType: .string) == marker)
+    }
+
+    @Test func standardWritesLandOnInjectedPasteboardAfterCaptureDisarms() {
+        let scratch = ScratchPasteboard()
+        let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard)
+        service.captureNextStandardClipboardWrite {
+            service.writeString("diverted", to: GHOSTTY_CLIPBOARD_STANDARD)
+            return true
+        }
+        let marker = "after-capture-\(UUID().uuidString)"
+        service.writeString(marker, to: GHOSTTY_CLIPBOARD_STANDARD)
+        #expect(scratch.pasteboard.string(forType: .string) == marker)
+    }
+
     @Test func mixedRepresentationsShareOnePasteboardItem() {
         let service = TerminalPasteboardService()
         let marker = "mixed-\(UUID().uuidString)"

Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swift

@@ -23,14 +23,36 @@ public protocol TerminalClipboardWriting: AnyObject, Sendable {
     /// Writes a string to the given ghostty clipboard location.
     ///
     /// When a one-shot capture is armed via
-    /// ``captureNextStandardClipboardWrite(_:)``, a standard-location write is
-    /// diverted into the capture instead of the system pasteboard.
+    /// ``captureNextStandardClipboardWrite(matching:_:)``, a standard-location
+    /// write the capture's predicate accepts is diverted into the capture
+    /// instead of the system pasteboard; a write it rejects reaches the
+    /// pasteboard and leaves the capture armed.
     func writeString(_ string: String, to location: ghostty_clipboard_e)
 
-    /// Arms a one-shot diversion of the next standard-clipboard write that
-    /// happens while `action` runs, returning the diverted string.
+    /// Arms a one-shot diversion of the next matching standard-clipboard
+    /// write that happens while `action` runs, returning the diverted string.
     ///
-    /// Returns `nil` when `action` reports failure or no write occurred.
+    /// `predicate` decides whether a given write is the one this capture is
+    /// waiting for; non-matching writes (e.g. a concurrent user copy) pass
+    /// through to the real pasteboard un-swallowed.
+    ///
+    /// Returns `nil` when `action` reports failure, no matching write
+    /// occurred, or another capture is already in flight (overlapping
+    /// captures are rejected rather than allowed to steal each other's
+    /// writes; callers treat `nil` as "fall back to a non-capture read").
+    @discardableResult
+    func captureNextStandardClipboardWrite(
+        matching predicate: @escaping @Sendable (String) -> Bool,
+        _ action: () -> Bool
+    ) -> String?
+}
+
+extension TerminalClipboardWriting {
+    /// Arms a one-shot diversion of the next standard-clipboard write,
+    /// accepting any payload. Prefer the `matching:` variant with the
+    /// narrowest predicate the expected payload allows.
     @discardableResult
-    func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String?
+    public func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String? {
+        captureNextStandardClipboardWrite(matching: { _ in true }, action)
+    }
 }

Sources/GhosttyTerminalView.swift

@@ -877,7 +877,15 @@ class GhosttyApp {
             }
         }
         runtimeConfig.write_clipboard_cb = { _, location, content, len, _ in
-            guard let content = content, len > 0 else { return }
+            guard let content = content, len > 0 else {
+                #if DEBUG
+                cmuxDebugLog("terminal.clipboard.write EMPTY payload location=\(location.rawValue) len=\(len)")
+                #endif
+                return
+            }
+            #if DEBUG
+            cmuxDebugLog("terminal.clipboard.write location=\(location.rawValue) items=\(len)")
+            #endif
             let buffer = UnsafeBufferPointer(start: content, count: Int(len))
             let decoder = TerminalClipboardRepresentationDecoder()
 
@@ -4643,8 +4651,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
     func validateUserInterfaceItem(_ item: NSValidatedUserInterfaceItem) -> Bool {
         switch item.action {
         case #selector(copy(_:)):
-            guard let surface = surface else { return false }
-            return hasCopyableTerminalSelection(surface: surface)
+            // Enabled whenever a surface exists, not gated on
+            // ghostty_surface_has_selection: that flag can report false while
+            // the runtime still holds a live selection (e.g. under constant
+            // TUI redraw), and a disabled menu item swallows Cmd+C before the
+            // runtime's own copy binding can handle it. Copy with no
+            // selection is a harmless no-op.
+            return surface != nil
         case #selector(paste(_:)):
             return GhosttyApp.terminalPasteboard.hasString(for: GHOSTTY_CLIPBOARD_STANDARD)
         case #selector(pasteAsPlainText(_:)):