From 46abf040122803911d0e7126f72646321a9cf5ab Mon Sep 17 00:00:00 2001 From: kewe63 Date: Mon, 13 Apr 2026 14:55:08 +0300 Subject: [PATCH] fix(ssh): handle WinError 1314 symlink failure with shutil.copy2 fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, os.symlink() raises OSError (WinError 1314) unless the process has Administrator rights or Developer Mode is enabled. The SSH bulk-upload staging logic used symlinks to mirror the remote layout before piping through tar; this caused all ssh_bulk_upload tests to fail on Windows. - ssh.py: wrap os.symlink() in try/except OSError and fall back to shutil.copy2() so staging works on every platform. shutil was already imported, no new dependency introduced. - file_sync.py: replace str(Path(remote).parent) with posixpath.dirname(remote) in unique_parent_dirs(). pathlib.Path uses the host separator (\ on Windows), but these paths are sent to a remote Linux host over SSH and must always use forward slashes. - test_ssh_bulk_upload.py: make test_staging_symlinks_mirror_remote_layout platform-agnostic — assert file existence and content instead of os.path.islink() + os.readlink(), since the staged entry may be a copy on Windows. --- tests/tools/test_ssh_bulk_upload.py | 16 ++++++++++++---- tools/environments/file_sync.py | 3 ++- tools/environments/ssh.py | 7 ++++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_ssh_bulk_upload.py b/tests/tools/test_ssh_bulk_upload.py index a2fa82e6c..c7d38f182 100644 --- a/tests/tools/test_ssh_bulk_upload.py +++ b/tests/tools/test_ssh_bulk_upload.py @@ -90,7 +90,12 @@ class TestSSHBulkUpload: assert "/home/testuser/.hermes/credentials" in mkdir_str def test_staging_symlinks_mirror_remote_layout(self, mock_env, tmp_path): - """Symlinks in staging dir should mirror the .hermes-relative layout.""" + """Staged file in staging dir should mirror the remote path structure. + + On platforms where symlinks are available (Linux/macOS) the staged + entry is a symlink; on Windows it may be a regular copy. Either way + the file must exist at the expected path and contain the right data. + """ f1 = tmp_path / "local_a.txt" f1.write_text("content a") @@ -105,11 +110,14 @@ class TestSSHBulkUpload: # Capture the staging dir from -C argument c_idx = cmd.index("-C") staging_dir = cmd[c_idx + 1] - # Check the symlink exists + # Check the staged entry exists at the base-relative path expected = os.path.join(staging_dir, "skills/my_skill.md") staging_paths.append(expected) - assert os.path.islink(expected), f"Expected symlink at {expected}" - assert os.readlink(expected) == os.path.abspath(str(f1)) + # File must exist (either as symlink or copy) + assert os.path.exists(expected), f"Expected staged file at {expected}" + # Content must match the source + with open(expected, "r") as fh: + assert fh.read() == "content a" mock = MagicMock() mock.stdout = MagicMock() diff --git a/tools/environments/file_sync.py b/tools/environments/file_sync.py index 6de78c87b..89f712693 100644 --- a/tools/environments/file_sync.py +++ b/tools/environments/file_sync.py @@ -9,6 +9,7 @@ view) and don't need this. import hashlib import logging import os +import posixpath import shlex import shutil import signal @@ -87,7 +88,7 @@ def quoted_mkdir_command(dirs: list[str]) -> str: def unique_parent_dirs(files: list[tuple[str, str]]) -> list[str]: """Extract sorted unique parent directories from (host, remote) pairs.""" - return sorted({str(Path(remote).parent) for _, remote in files}) + return sorted({posixpath.dirname(remote) for _, remote in files}) def _sha256_file(path: str) -> str: diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 8924d7689..fac9d5d6c 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -179,6 +179,8 @@ class SSHEnvironment(BaseEnvironment): raise RuntimeError(f"remote mkdir failed: {result.stderr.strip()}") # Symlink staging avoids fragile GNU tar --transform rules. + # On Windows, symlink creation requires admin rights or Developer Mode, + # so fall back to copying the file when os.symlink raises OSError. with tempfile.TemporaryDirectory(prefix="hermes-ssh-bulk-") as staging: for host_path, remote_path in files: try: @@ -195,7 +197,10 @@ class SSHEnvironment(BaseEnvironment): staged = os.path.join(staging, rel_remote) os.makedirs(os.path.dirname(staged), exist_ok=True) - os.symlink(os.path.abspath(host_path), staged) + try: + os.symlink(os.path.abspath(host_path), staged) + except OSError: + shutil.copy2(host_path, staged) tar_cmd = ["tar", "-chf", "-", "-C", staging, "."] ssh_cmd = self._build_ssh_command()