From 608d7889a33401c68f6b76f6b6df54970b9e6f3a Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 13:58:30 +0000 Subject: [PATCH 1/2] fix(sftp): auto-create remote parent dir on upload and fix mkdir v4+ type check - SFTPClient.upload now catches FileNotFoundError from both the native put() path and the BytesIO fallback path; on failure it calls sftp.makedirs(parent, exist_ok=True) and retries, so uploading to a path like '/audiobooks/01 - Harry Potter...' no longer raises SFTPNoSuchFile when the remote directory does not yet exist. - SFTPClient.mkdir verification now checks the SFTP v4+ `type` attribute in addition to `permissions`, matching the same pattern used by stat() and chdir(). Servers that return type=2 (directory) but omit permission bits would previously raise a spurious RuntimeError after a successful mkdir. Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/protocols.py | 48 ++++++++++++++----- tests/test_protocols.py | 92 ++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 12 deletions(-) diff --git a/src/portkeydrop/protocols.py b/src/portkeydrop/protocols.py index d12388f..bcff25a 100644 --- a/src/portkeydrop/protocols.py +++ b/src/portkeydrop/protocols.py @@ -1428,6 +1428,8 @@ def upload( sftp = self._ensure_connected() local_path = getattr(local_file, "name", None) + remote_parent = str(PurePosixPath(remote_path).parent) + if isinstance(local_path, str) and os.path.isabs(local_path): # asyncssh native put() — pipelined writes with progress reporting total = os.path.getsize(local_path) @@ -1440,7 +1442,14 @@ async def _upload(): def handler(srcpath, dstpath, copied, total_bytes): callback(copied, total_bytes) - await sftp.put(local_path, remote_path, progress_handler=handler) + try: + await sftp.put(local_path, remote_path, progress_handler=handler) + except FileNotFoundError: + logger.debug( + "Remote parent %r does not exist; creating with makedirs", remote_parent + ) + await sftp.makedirs(remote_parent, exist_ok=True) + await sftp.put(local_path, remote_path, progress_handler=handler) self._run(_upload()) else: @@ -1450,16 +1459,27 @@ def handler(srcpath, dstpath, copied, total_bytes): local_file.seek(0) async def _upload(): - async with sftp.open(remote_path, "wb") as wf: - transferred = 0 - while True: - chunk = local_file.read(8192) - if not chunk: - break - await wf.write(chunk) - transferred += len(chunk) - if callback: - callback(transferred, total) + async def _write(): + async with sftp.open(remote_path, "wb") as wf: + transferred = 0 + while True: + chunk = local_file.read(8192) + if not chunk: + break + await wf.write(chunk) + transferred += len(chunk) + if callback: + callback(transferred, total) + + try: + await _write() + except FileNotFoundError: + logger.debug( + "Remote parent %r does not exist; creating with makedirs", remote_parent + ) + await sftp.makedirs(remote_parent, exist_ok=True) + local_file.seek(0) + await _write() self._run(_upload()) @@ -1505,7 +1525,11 @@ def mkdir(self, path: str) -> None: sftp = self._ensure_connected() self._run(sftp.mkdir(path)) attrs = self._run(sftp.stat(path)) - if not attrs.permissions or not stat.S_ISDIR(attrs.permissions): + mode = attrs.permissions + is_dir = bool(mode and stat.S_ISDIR(mode)) + if not is_dir and getattr(attrs, "type", None) == _SFTP_TYPE_DIRECTORY: + is_dir = True + if not is_dir: raise RuntimeError(f"Remote mkdir verification failed for {path}.") def rename(self, old_path: str, new_path: str) -> None: diff --git a/tests/test_protocols.py b/tests/test_protocols.py index f17dae0..dcab661 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -1613,6 +1613,77 @@ def test_upload_raises_when_remote_size_mismatch(self, mock_connect): with pytest.raises(RuntimeError, match="verification failed"): client.upload(io.BytesIO(b"data"), "/remote.txt") + @patch("asyncssh.connect", new_callable=AsyncMock) + def test_upload_bytesio_creates_remote_dir_on_file_not_found(self, mock_connect): + """BytesIO upload retries after makedirs when parent directory is missing.""" + mock_conn = AsyncMock() + mock_sftp = AsyncMock() + mock_sftp.realpath.return_value = "/" + mock_conn.start_sftp_client.return_value = mock_sftp + mock_connect.return_value = mock_conn + + calls: list[str] = [] + + mock_write_file = AsyncMock() + mock_open_cm = MagicMock() + mock_open_cm.__aenter__ = AsyncMock(return_value=mock_write_file) + mock_open_cm.__aexit__ = AsyncMock(return_value=False) + + def fake_open(path, mode): + calls.append("open") + if len(calls) == 1: + raise FileNotFoundError("No such file or directory") + return mock_open_cm + + mock_sftp.open = fake_open + + stat_attrs = MagicMock() + stat_attrs.size = 4 + mock_sftp.stat.return_value = stat_attrs + + client = SFTPClient(ConnectionInfo(protocol=Protocol.SFTP, host="example.com")) + client.connect() + + client.upload(io.BytesIO(b"data"), "/new/dir/remote.txt") + + mock_sftp.makedirs.assert_awaited_once_with("/new/dir", exist_ok=True) + assert calls.count("open") == 2 + + @patch("os.path.getsize", return_value=4) + @patch("asyncssh.connect", new_callable=AsyncMock) + def test_upload_native_creates_remote_dir_on_file_not_found(self, mock_connect, _mock_getsize): + """Native put() upload retries after makedirs when parent directory is missing.""" + mock_conn = AsyncMock() + mock_sftp = AsyncMock() + mock_sftp.realpath.return_value = "/" + mock_conn.start_sftp_client.return_value = mock_sftp + mock_connect.return_value = mock_conn + + put_calls: list[int] = [] + + async def fake_put(localpath, remotepath, *, progress_handler=None, **kwargs): + put_calls.append(1) + if len(put_calls) == 1: + raise FileNotFoundError("No such file or directory") + + mock_sftp.put = AsyncMock(side_effect=fake_put) + + stat_attrs = MagicMock() + stat_attrs.size = 4 + mock_sftp.stat.return_value = stat_attrs + + client = SFTPClient(ConnectionInfo(protocol=Protocol.SFTP, host="example.com")) + client.connect() + + mock_file = MagicMock() + mock_file.name = "/tmp/upload.bin" + mock_file.close = MagicMock() + + client.upload(mock_file, "/new/dir/remote.bin") + + mock_sftp.makedirs.assert_awaited_once_with("/new/dir", exist_ok=True) + assert len(put_calls) == 2 + @patch("asyncssh.connect", new_callable=AsyncMock) def test_mkdir_raises_when_created_path_is_not_directory(self, mock_connect): mock_conn = AsyncMock() @@ -1632,6 +1703,27 @@ def test_mkdir_raises_when_created_path_is_not_directory(self, mock_connect): with pytest.raises(RuntimeError, match="verification failed"): client.mkdir("/not-a-dir") + @patch("asyncssh.connect", new_callable=AsyncMock) + def test_mkdir_succeeds_when_type_indicates_directory_without_permissions(self, mock_connect): + """mkdir verification accepts SFTP v4+ servers that return type=dir but no permissions.""" + mock_conn = AsyncMock() + mock_sftp = AsyncMock() + mock_sftp.realpath.return_value = "/" + mock_conn.start_sftp_client.return_value = mock_sftp + mock_connect.return_value = mock_conn + + stat_attrs = MagicMock() + stat_attrs.permissions = None + stat_attrs.type = 2 # _SFTP_TYPE_DIRECTORY + mock_sftp.stat.return_value = stat_attrs + + client = SFTPClient(ConnectionInfo(protocol=Protocol.SFTP, host="example.com")) + client.connect() + + # Should not raise + client.mkdir("/new-dir") + mock_sftp.mkdir.assert_awaited_once_with("/new-dir") + @patch("asyncssh.connect", new_callable=AsyncMock) def test_delete_raises_when_remote_stat_succeeds(self, mock_connect): mock_conn = AsyncMock() From 392c69b32949e818e8ed284d30f4ad7aa3914714 Mon Sep 17 00:00:00 2001 From: Orinks Date: Sun, 22 Mar 2026 13:58:47 +0000 Subject: [PATCH 2/2] fix(transfer): recursive upload now creates top-level destination directory _run_recursive_upload collected only subdirectories into dirs_to_create, stopping the walk when remote_parent == job.destination. The top-level destination folder itself was never created, so mkdir calls for its children would fail and subsequent file uploads would error out. Fix: seed dirs_to_create with job.destination so it is always created first (sorted order guarantees parent dirs precede their children). Co-Authored-By: Claude Sonnet 4.6 --- src/portkeydrop/services/transfer_service.py | 4 +-- tests/test_transfer_service.py | 38 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/portkeydrop/services/transfer_service.py b/src/portkeydrop/services/transfer_service.py index b6d9654..1d1b439 100644 --- a/src/portkeydrop/services/transfer_service.py +++ b/src/portkeydrop/services/transfer_service.py @@ -396,8 +396,8 @@ def _run_recursive_upload(self, job: TransferJob) -> None: self._update_progress(job) self._post_event() - # Create directories - dirs_to_create: set[str] = set() + # Create directories — including the top-level destination folder itself + dirs_to_create: set[str] = {job.destination} for _, remote_file, _ in file_queue: remote_parent = os.path.dirname(remote_file).replace("\\", "/") while remote_parent and remote_parent != job.destination: diff --git a/tests/test_transfer_service.py b/tests/test_transfer_service.py index 5b5cb48..5211764 100644 --- a/tests/test_transfer_service.py +++ b/tests/test_transfer_service.py @@ -334,6 +334,44 @@ def test_recursive_upload_creates_remote_dirs_and_uploads(self, tmp_path): assert job.status == TransferStatus.COMPLETE assert mock_client.upload.call_count == 2 + def test_recursive_upload_creates_top_level_destination_dir(self, tmp_path): + """The top-level destination folder must be created before uploading files into it.""" + src_dir = tmp_path / "my_folder" + src_dir.mkdir() + (src_dir / "file.txt").write_text("hello") + + mkdir_calls: list[str] = [] + mock_client = MagicMock() + mock_client.upload.side_effect = lambda fh, dest, callback=None: None + mock_client.mkdir.side_effect = lambda d: mkdir_calls.append(d) + + svc = TransferService(notify_window=None) + job = svc.submit_upload(mock_client, str(src_dir), "/remote/my_folder", recursive=True) + _wait_for_terminal(job) + + assert job.status == TransferStatus.COMPLETE + assert "/remote/my_folder" in mkdir_calls + + def test_recursive_upload_creates_nested_dirs_in_order(self, tmp_path): + """Nested subdirectories are created in sorted (parent-before-child) order.""" + src_dir = tmp_path / "tree" + src_dir.mkdir() + sub = src_dir / "sub" + sub.mkdir() + (sub / "file.txt").write_text("data") + + mkdir_calls: list[str] = [] + mock_client = MagicMock() + mock_client.upload.side_effect = lambda fh, dest, callback=None: None + mock_client.mkdir.side_effect = lambda d: mkdir_calls.append(d) + + svc = TransferService(notify_window=None) + job = svc.submit_upload(mock_client, str(src_dir), "/remote/tree", recursive=True) + _wait_for_terminal(job) + + assert job.status == TransferStatus.COMPLETE + assert mkdir_calls.index("/remote/tree") < mkdir_calls.index("/remote/tree/sub") + def test_recursive_upload_cancel_mid_transfer(self, tmp_path): src_dir = tmp_path / "cancel_dir" src_dir.mkdir()