fix(file-sync): hash the uploaded snapshot instead of mutable host files
This commit is contained in:
@@ -280,8 +280,8 @@ class TestConcurrency:
|
||||
for path in sorted(tmp_path.glob("*.png"))
|
||||
]
|
||||
|
||||
def upload(host_path, _remote_path):
|
||||
if host_path == str(new_file):
|
||||
def upload(_host_path, remote_path):
|
||||
if remote_path == f"/root/.hermes/cache/images/{new_file.name}":
|
||||
upload_started.set()
|
||||
sync_back_transport_started.wait(timeout=1.0)
|
||||
release_upload.set()
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
"""Upload hashes must describe the bytes received by the remote sandbox."""
|
||||
|
||||
import hashlib
|
||||
import tarfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.environments.file_sync import FileSyncManager
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bulk", [False, True])
|
||||
@pytest.mark.parametrize("edit_before_read", [False, True])
|
||||
def test_host_save_during_upload_survives_unchanged_remote(
|
||||
tmp_path, monkeypatch, bulk, edit_before_read,
|
||||
):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home"))
|
||||
host = tmp_path / "home" / "skills" / "example" / "SKILL.md"
|
||||
host.parent.mkdir(parents=True)
|
||||
host.write_bytes(b"original skill")
|
||||
remote_path = "/root/.hermes/skills/example/SKILL.md"
|
||||
remote = tmp_path / "remote.md"
|
||||
|
||||
def upload(source, destination):
|
||||
assert destination == remote_path
|
||||
if edit_before_read:
|
||||
host.write_bytes(b"saved while upload is starting")
|
||||
remote.write_bytes(Path(source).read_bytes())
|
||||
host.write_bytes(b"new local version saved before upload acknowledgement")
|
||||
|
||||
def download(destination):
|
||||
with tarfile.open(destination, "w") as archive:
|
||||
archive.add(remote, arcname=remote_path.lstrip("/"))
|
||||
|
||||
manager = FileSyncManager(
|
||||
get_files_fn=lambda: [(str(host), remote_path)],
|
||||
upload_fn=upload,
|
||||
bulk_upload_fn=(lambda files: [upload(*pair) for pair in files]) if bulk else None,
|
||||
delete_fn=lambda paths: None,
|
||||
bulk_download_fn=download,
|
||||
)
|
||||
manager.sync(force=True)
|
||||
saved = host.read_bytes()
|
||||
assert manager._pushed_hashes[remote_path] == hashlib.sha256(remote.read_bytes()).hexdigest()
|
||||
manager.sync_back()
|
||||
assert host.read_bytes() == saved
|
||||
|
||||
# A subsequent cycle still detects and uploads the newer local version.
|
||||
manager._upload_fn = lambda source, destination: remote.write_bytes(Path(source).read_bytes())
|
||||
manager._bulk_upload_fn = None
|
||||
manager.sync(force=True)
|
||||
assert remote.read_bytes() == saved
|
||||
@@ -162,10 +162,19 @@ class FileSyncManager:
|
||||
prev_files = dict(self._synced_files)
|
||||
prev_hashes = dict(self._pushed_hashes)
|
||||
try:
|
||||
self._push(to_upload, to_delete)
|
||||
# Hash and upload the same bytes: the original may be saved while
|
||||
# the transport is reading it or waiting for remote acknowledgement.
|
||||
with tempfile.TemporaryDirectory(prefix="hermes-sync-push-") as staging:
|
||||
staged_files = []
|
||||
pushed_hashes = {}
|
||||
for index, (host_path, remote_path) in enumerate(to_upload):
|
||||
staged_path = os.path.join(staging, str(index))
|
||||
shutil.copy2(host_path, staged_path)
|
||||
pushed_hashes[remote_path] = _sha256_file(staged_path)
|
||||
staged_files.append((staged_path, remote_path))
|
||||
self._push(staged_files, to_delete)
|
||||
# Commit (all succeeded).
|
||||
for host_path, remote_path in to_upload:
|
||||
self._pushed_hashes[remote_path] = _sha256_file(host_path)
|
||||
self._pushed_hashes.update(pushed_hashes)
|
||||
for p in to_delete:
|
||||
new_files.pop(p, None)
|
||||
self._pushed_hashes.pop(p, None)
|
||||
|
||||
Reference in New Issue
Block a user