Improve clipboard image pasting and upload performance over SSH

ModernTests/ConductorUploadTests.swift

@@ -0,0 +1,87 @@
+import XCTest
+@testable import iTerm2SharedARC
+
+@MainActor
+final class ConductorUploadTests: XCTestCase {
+    func testScreenshotUsesThirtyOneRequestsAndPreservesBytes() async throws {
+        let data = Data((0..<(490 * 1024)).map { UInt8(truncatingIfNeeded: $0) })
+        var chunks = [Data]()
+        var progress = 0
+        try await ConductorUpload.send(data, checkCancellation: {}) { chunk in
+            chunks.append(chunk)
+        } didTransfer: { count in
+            progress += count
+        }
+        XCTAssertEqual(chunks.count, 31)
+        XCTAssertTrue(chunks.allSatisfy { $0.count <= 16 * 1024 })
+        XCTAssertEqual(chunks.reduce(into: Data()) { $0.append($1) }, data)
+        XCTAssertEqual(progress, data.count)
+    }
+
+    func testCancellationDuringFinalAppendDoesNotFinishUpload() async {
+        var cancelled = false
+        var progress = 0
+        do {
+            try await ConductorUpload.send(Data([1, 2, 3]), checkCancellation: {
+                if cancelled { throw CancellationError() }
+            }) { _ in
+                cancelled = true
+            } didTransfer: { progress += $0 }
+            XCTFail("Cancellation during the final append must reach the cleanup path")
+        } catch is CancellationError {
+            XCTAssertEqual(progress, 0)
+        } catch {
+            XCTFail("Unexpected error: \(error)")
+        }
+    }
+
+    func testCancellationBetweenChunksDoesNotSendAnotherRequest() async {
+        var cancelled = false
+        var requests = 0
+        do {
+            try await ConductorUpload.send(Data(count: 128 * 1024), checkCancellation: {
+                if cancelled { throw CancellationError() }
+            }) { _ in
+                requests += 1
+            } didTransfer: { _ in
+                cancelled = true
+            }
+            XCTFail("Expected cancellation")
+        } catch is CancellationError {
+            XCTAssertEqual(requests, 1)
+        } catch {
+            XCTFail("Unexpected error: \(error)")
+        }
+    }
+
+    func testAppendFailureDoesNotReportProgressOrSendAnotherChunk() async {
+        var requests = 0
+        var progress = 0
+        do {
+            try await ConductorUpload.send(Data(count: 128 * 1024), checkCancellation: {}) { _ in
+                requests += 1
+                throw CocoaError(.fileWriteUnknown)
+            } didTransfer: { progress += $0 }
+            XCTFail("Expected append error")
+        } catch {
+            XCTAssertEqual(requests, 1)
+            XCTAssertEqual(progress, 0)
+        }
+    }
+
+    func testEmptyUploadStillChecksCancellation() async {
+        do {
+            try await ConductorUpload.send(Data(), checkCancellation: {
+                throw CancellationError()
+            }) { _ in
+                XCTFail("Empty upload must not append")
+            } didTransfer: { _ in
+                XCTFail("Empty upload must not report progress")
+            }
+            XCTFail("Expected cancellation")
+        } catch is CancellationError {
+        } catch {
+            XCTFail("Unexpected error: \(error)")
+        }
+    }
+}

ModernTests/NonTextPasteImageTests.swift

@@ -0,0 +1,52 @@
+import AppKit
+import XCTest
+@testable import iTerm2SharedARC
+
+final class NonTextPasteImageTests: XCTestCase {
+    func testTIFFScreenshotBecomesPNGWithoutChangingDimensions() throws {
+        let bitmap = try XCTUnwrap(NSBitmapImageRep(bitmapDataPlanes: nil,
+                                                   pixelsWide: 3,
+                                                   pixelsHigh: 2,
+                                                   bitsPerSample: 8,
+                                                   samplesPerPixel: 4,
+                                                   hasAlpha: true,
+                                                   isPlanar: false,
+                                                   colorSpaceName: .deviceRGB,
+                                                   bytesPerRow: 0,
+                                                   bitsPerPixel: 0))
+        let pixels = try XCTUnwrap(bitmap.bitmapData)
+        for x in 0..<3 {
+            for y in 0..<2 {
+                let offset = y * bitmap.bytesPerRow + x * 4
+                pixels[offset] = 255
+                pixels[offset + 1] = 0
+                pixels[offset + 2] = 0
+                pixels[offset + 3] = 255
+            }
+        }
+        let tiff = try XCTUnwrap(bitmap.representation(using: .tiff, properties: [:]))
+        let result = try XCTUnwrap(iTermNonTextPasteHelper.imageForFile(tiff, fileExtension: "tiff"))
+        XCTAssertEqual(result.fileExtension, "png")
+        XCTAssertEqual(Array(result.data.prefix(8)), [137, 80, 78, 71, 13, 10, 26, 10])
+        let decoded = try XCTUnwrap(NSBitmapImageRep(data: result.data))
+        XCTAssertEqual(decoded.pixelsWide, 3)
+        XCTAssertEqual(decoded.pixelsHigh, 2)
+        var pixel = [UInt](repeating: 0, count: 4)
+        decoded.getPixel(&pixel, atX: 1, y: 1)
+        XCTAssertEqual(pixel, [255, 0, 0, 255])
+    }
+
+    func testSupportedFormatsPreserveOriginalBytes() throws {
+        // Existing compressed images should not be recompressed or lose animation.
+        let original = Data([1, 2, 3, 4])
+        for ext in ["PNG", "JPEG", "jpg", "gif", "webp"] {
+            let result = try XCTUnwrap(iTermNonTextPasteHelper.imageForFile(original, fileExtension: ext))
+            XCTAssertEqual(result.data, original)
+            XCTAssertEqual(result.fileExtension, ext.lowercased())
+        }
+    }
+
+    func testInvalidTIFFDoesNotBecomeAnImagePath() {
+        XCTAssertNil(iTermNonTextPasteHelper.imageForFile(Data([1, 2, 3]), fileExtension: "tiff"))
+    }
+}

docs/notes-3.7.txt

@@ -592,6 +592,14 @@ Shell Integration Improvements:
   It's automatically in your path.
 
 SSH Integration Improvements:
+- Speed up uploads with larger transfer chunks.
+- Fix cancelled uploads leaving temporary files
+  or getting stuck while finishing the transfer.
+- Upload and paste image paths through tmux
+  integration using the existing SSH connection.
+- Convert pasted TIFF screenshots to PNG and use
+  unique filenames for remote image uploads.
+- Do not paste paths after cancelling an upload.
 - Better passphrase handling for SCP and
   private keys. Wrong passphrases now retry
   instead of falling through to an empty

iTerm2.xcodeproj/project.pbxproj

@@ -28,6 +28,7 @@
 		0B05CA460519B96060210386 /* AIChatWireLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 88578347A529CFE749B6F433 /* AIChatWireLogger.swift */; };
 		0BC580AF4D2AB9C3E952CB01 /* ShuffleDeck.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46A873D7F2EFCF8441303D1 /* ShuffleDeck.swift */; };
 		0C026664CE3F52A755A333FF /* OrchestrationMentionRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FB54728D976B5A56B887ED5 /* OrchestrationMentionRenderer.swift */; };
+		0C1B7AF329EFE71B706E946C /* ConductorUpload.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E9FF32118246D33BBE5EF45 /* ConductorUpload.swift */; };
 		0C345F530667B5539D6243B5 /* CompanionPriorityOutbox.swift in Sources */ = {isa = PBXBuildFile; fileRef = 751FAD7033C9A3DC31ABE2F3 /* CompanionPriorityOutbox.swift */; };
 		0C8691424C40627F505DDE94 /* PTYSessionPeerPort.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8721B9515977EC8CB513CDAA /* PTYSessionPeerPort.swift */; };
 		0C9EC47197770F9F682B6F8D /* RemoteCommandToolProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E7BADF8FB7DD7E1B851F797 /* RemoteCommandToolProvider.swift */; };
@@ -6812,6 +6813,7 @@
 		6D70E7DE7E81B39DC29CC27B /* VT100PromptKind.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = VT100PromptKind.h; sourceTree = "<group>"; };
 		6E2F6A9024607073D194DA5A /* iTermConfigGenerationTracker.m */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.objc; path = iTermConfigGenerationTracker.m; sourceTree = "<group>"; };
 		6E8BB76193334BC38C90EC4C /* mul */ = {isa = PBXFileReference; lastKnownFileType = text.json.xcstrings; name = mul; path = mul.lproj/InstantReplay.xcstrings; sourceTree = "<group>"; };
+		6E9FF32118246D33BBE5EF45 /* ConductorUpload.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ConductorUpload.swift; sourceTree = "<group>"; };
 		6EFD41B9F2C6F53723CDF88B /* iTermArrangementKeys.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = iTermArrangementKeys.h; sourceTree = "<group>"; };
 		6F6AD8136D2E4D2EB86E9ABC /* mul */ = {isa = PBXFileReference; lastKnownFileType = text.json.xcstrings; name = mul; path = mul.lproj/AITerm.xcstrings; sourceTree = "<group>"; };
 		6F87CB3CBFBF0D0E8781513C /* VT100StringConversionConfig.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = VT100StringConversionConfig.h; sourceTree = "<group>"; };
@@ -12795,6 +12797,7 @@
 				A6069F352DF79A5100EE6CC7 /* SystemFolderIconProvider.swift */,
 				A6EC8096282EBFB000493544 /* TarJob.swift */,
 				A1B19743799E1009AC26E7F4 /* Conductor+IT2.swift */,
+				6E9FF32118246D33BBE5EF45 /* ConductorUpload.swift */,
 			);
 			path = SSH;
 			sourceTree = "<group>";
@@ -22419,6 +22422,7 @@
 				DDC6A04E4BD124950415EFEA /* iTermHDREngager.swift in Sources */,
 				FB47A51FE147AA2BDA2FFFD4 /* SendCompanionNotificationBuiltInFunction.swift in Sources */,
 				3566BF95446653BE5BBB96EB /* iTermSettingsTruncationChecker.swift in Sources */,
+				0C1B7AF329EFE71B706E946C /* ConductorUpload.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

sources/Localizable.xcstrings

@@ -97480,6 +97480,17 @@
         }
       }
     },
+    "NonTextPaste.CouldNotEncodeImage" : {
+      "comment" : "Error when clipboard image data cannot be converted to a PNG file",
+      "localizations" : {
+        "en" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "Could not convert the clipboard image to PNG."
+          }
+        }
+      }
+    },
     "NonTextPaste.CouldNotCreateTempDir" : {
       "comment" : "Error when a temporary directory cannot be created",
       "extractionState" : "stale",

sources/PTYSession/PTYSession+Private.h

@@ -87,6 +87,7 @@ TriggerDelegate> {
     AITermControllerObjC *_aiterm;
     iTermNonTextPasteHelper *_nonTextPasteHelper;
     TransferrableFile *_uploadAndPasteTransfer;  // Current upload for "upload and paste path" feature
+    NSUUID *_uploadAndPasteIdentifier;
 }
 
 @property(nonatomic, retain) Interval *currentMarkOrNotePosition;

sources/PTYSession/PTYSession.m

@@ -1176,6 +1176,7 @@ - (void)dealloc {
     [_pasteHelper release];
     _nonTextPasteHelper.delegate = nil;
     [_nonTextPasteHelper release];
+    [_uploadAndPasteIdentifier release];
     [_backgroundImage release];
     [_antiIdleTimer invalidate];
     [_cadenceController release];
@@ -15211,10 +15212,11 @@ - (void)uploadFiles:(NSArray *)localFilenames toPath:(SCPPath *)destinationPath
         path.path = [destinationPath.path stringByAppendingPathComponent:filename];
         DLog(@"Will upload local:%@ to remote:%@", file, path.path);
 
-        if ([_conductor canTransferFilesTo:path]) {
+        iTermConductor *conductor = [self nonTextPasteConductor];
+        if ([conductor canTransferFilesTo:path]) {
             DLog(@"Using conductor for upload");
-            [_conductor uploadFile:file to:path];
-            break;
+            [conductor uploadFile:file to:path];
+            continue;
         }
         DLog(@"Using SCPFile for upload");
         SCPFile *scpFile = [[[SCPFile alloc] init] autorelease];
@@ -20436,23 +20438,36 @@ - (NSWindow *)nonTextPasteHelperWindow:(iTermNonTextPasteHelper *)sender {
 }
 
 - (BOOL)nonTextPasteHelperCanUpload:(iTermNonTextPasteHelper *)sender {
-    // Can upload if we have SSH integration (conductor) in framing mode, or if
-    // shell integration detected we're on a remote host.
-    DLog(@"nonTextPasteHelperCanUpload: conductor=%@ framing=%@ currentHost=%@ isLocalhost=%@",
-         self.conductor, @(self.conductor.framing), self.currentHost, @(self.currentHost.isLocalhost));
-    if (self.conductor.framing) {
-        DLog(@"Can upload via conductor");
-        return YES;
+    return [self scpPathForCurrentRemoteHost] != nil;
+}
+
+// A control-mode pane has no conductor of its own. Its gateway owns the SSH
+// connection, but pasted input must still go to this pane.
+- (iTermConductor *)nonTextPasteConductor {
+    if (_conductor) {
+        return _conductor.framing ? _conductor : nil;
+    }
+    PTYSession *gateway = self.tmuxGatewaySession;
+    if (self.isTmuxClient && !_tmuxController.serverIsLocal && gateway != self) {
+        return [gateway nonTextPasteConductor];
     }
-    BOOL canUpload = self.currentHost != nil && !self.currentHost.isLocalhost;
-    DLog(@"canUpload=%@", @(canUpload));
-    return canUpload;
+    return nil;
 }
 
 // Returns an SCPPath for the current remote host and working directory, or nil if not available.
 - (SCPPath *)scpPathForCurrentRemoteHost {
     id<VT100RemoteHostReading> remoteHost = self.currentHost;
+    iTermConductor *conductor = [self nonTextPasteConductor];
     DLog(@"scpPathForCurrentRemoteHost: remoteHost=%@", remoteHost);
+    // SSH integration knows the destination even before shell integration has
+    // reported a host or working directory (including inside ordinary tmux).
+    if (!remoteHost && conductor) {
+        SCPPath *path = [[[SCPPath alloc] init] autorelease];
+        path.hostname = conductor.sshIdentity.hostname;
+        path.username = conductor.sshIdentity.username;
+        path.path = conductor.homeDirectory;
+        return path.path.length ? path : nil;
+    }
     if (!remoteHost || !remoteHost.username || !remoteHost.hostname) {
         DLog(@"No remote host or missing username/hostname");
         return nil;
@@ -20462,7 +20477,13 @@ - (SCPPath *)scpPathForCurrentRemoteHost {
         return nil;
     }
     NSString *workingDirectory = self.variablesScope.path;
-    if (!workingDirectory) {
+    SCPPath *identityPath = [[[SCPPath alloc] init] autorelease];
+    identityPath.hostname = remoteHost.hostname;
+    identityPath.username = remoteHost.username;
+    if (!workingDirectory.length && [conductor canTransferFilesTo:identityPath]) {
+        workingDirectory = conductor.homeDirectory;
+    }
+    if (!workingDirectory.length) {
         DLog(@"No working directory");
         return nil;
     }
@@ -20517,6 +20538,10 @@ - (void)nonTextPasteHelper:(iTermNonTextPasteHelper *)sender uploadFileAndPasteP
 
     // Show upload indicator
     __weak __typeof(self) weakSelf = self;
+    NSUUID *identifier = [NSUUID UUID];
+    [_uploadAndPasteIdentifier release];
+    _uploadAndPasteIdentifier = [identifier retain];
+    iTermConductor *conductor = [self nonTextPasteConductor];
     [_view showUploadIndicatorWithFilename:filename onCancel:^{
         DLog(@"Upload cancelled by user");
         [weakSelf cancelUploadAndPaste];
@@ -20527,13 +20552,20 @@ - (void)nonTextPasteHelper:(iTermNonTextPasteHelper *)sender uploadFileAndPasteP
         DLog(@"Upload completed: success=%d error=%@", success, error);
         __strong __typeof(self) strongSelf = [[weakSelf retain] autorelease];
 
-        if (!strongSelf) {
+        if (!strongSelf || strongSelf->_uploadAndPasteIdentifier != identifier) {
             return;
         }
+        NSString *actualPath = [strongSelf->_uploadAndPasteTransfer isKindOfClass:ConductorFileTransfer.class] ?
+            [strongSelf->_uploadAndPasteTransfer destination] : remotePath;
         strongSelf->_uploadAndPasteTransfer = nil;
+        [strongSelf->_uploadAndPasteIdentifier release];
+        strongSelf->_uploadAndPasteIdentifier = nil;
         [strongSelf.view hideUploadIndicator];
-        if (success) {
-            NSString *escapedPath = [remotePath quotedStringForPaste];
+        SCPPath *currentPath = [strongSelf scpPathForCurrentRemoteHost];
+        BOOL sameHost = [currentPath.hostname isEqualToString:scpPath.hostname] &&
+            (currentPath.username == scpPath.username || [currentPath.username isEqualToString:scpPath.username]);
+        if (success && !strongSelf.exited && sameHost && [strongSelf nonTextPasteConductor] == conductor) {
+            NSString *escapedPath = [actualPath quotedStringForPaste];
             DLog(@"Pasting escaped path: %@", escapedPath);
             [strongSelf pasteString:escapedPath flags:0];
         } else {
@@ -20543,6 +20575,8 @@ - (void)nonTextPasteHelper:(iTermNonTextPasteHelper *)sender uploadFileAndPasteP
 }
 
 - (void)cancelUploadAndPaste {
+    [_uploadAndPasteIdentifier release];
+    _uploadAndPasteIdentifier = nil;
     if (_uploadAndPasteTransfer) {
         DLog(@"Cancelling upload and paste transfer");
         [_uploadAndPasteTransfer stop];
@@ -20562,9 +20596,10 @@ - (TransferrableFile *)uploadFile:(NSString *)localPath toPath:(SCPPath *)destin
     path.path = [destinationPath.path stringByAppendingPathComponent:filename];
     DLog(@"Will upload local:%@ to remote:%@", localPath, path.path);
 
-    if ([_conductor canTransferFilesTo:path]) {
+    iTermConductor *conductor = [self nonTextPasteConductor];
+    if ([conductor canTransferFilesTo:path]) {
         DLog(@"Using conductor for upload with completion");
-        return [_conductor uploadFile:localPath to:path withCompletion:completion];
+        return [conductor uploadFile:localPath to:path withCompletion:completion];
     }
 
     DLog(@"Using SCPFile for upload with completion");