mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 01:21:43 +00:00
feat(file-sync): sync remote changes back to host on teardown
Add sync_back() to FileSyncManager — on sandbox cleanup, downloads the remote .hermes/ directory as a tar archive, diffs against SHA-256 hashes of what was originally pushed, and applies only changed files. - SHA-256 content hashing on push for accurate change detection - Retry with exponential backoff (3 attempts, 2s/4s/8s) - SIGINT deferred during sync-back to prevent partial writes - fcntl.flock serialization for concurrent gateway sandboxes - Last-write-wins conflict resolution with logged warnings - New files created on remote are pulled back via path inference - Backend implementations: SSH (tar cf over pipe), Modal (exec tar cf, read stdout), Daytona (exec tar cf, SDK download_file) - Wired into cleanup() for all three backends (runs before ControlMaster close / sandbox terminate / sandbox stop) 28 new tests (10 FSM core + 18 backend-specific), 72 total passing.
This commit is contained in:
parent
27eeea0555
commit
37c478cf2f
6 changed files with 1023 additions and 0 deletions
306
tests/tools/test_file_sync_back.py
Normal file
306
tests/tools/test_file_sync_back.py
Normal file
|
|
@ -0,0 +1,306 @@
|
|||
"""Tests for FileSyncManager.sync_back() — pull remote changes to host."""
|
||||
|
||||
import fcntl
|
||||
import io
|
||||
import logging
|
||||
import os
|
||||
import signal
|
||||
import tarfile
|
||||
import time
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, call, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.environments.file_sync import (
|
||||
FileSyncManager,
|
||||
_sha256_file,
|
||||
_SYNC_BACK_BACKOFF,
|
||||
_SYNC_BACK_MAX_RETRIES,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_tar(files: dict[str, bytes], dest: Path):
|
||||
"""Write a tar archive containing the given arcname->content pairs."""
|
||||
with tarfile.open(dest, "w") as tar:
|
||||
for arcname, content in files.items():
|
||||
info = tarfile.TarInfo(name=arcname)
|
||||
info.size = len(content)
|
||||
tar.addfile(info, io.BytesIO(content))
|
||||
|
||||
|
||||
def _make_download_fn(files: dict[str, bytes]):
|
||||
"""Return a bulk_download_fn that writes a tar of the given files."""
|
||||
def download(dest: Path):
|
||||
_make_tar(files, dest)
|
||||
return download
|
||||
|
||||
|
||||
def _sha256_bytes(data: bytes) -> str:
|
||||
"""Compute SHA-256 hex digest of raw bytes (for test convenience)."""
|
||||
import hashlib
|
||||
return hashlib.sha256(data).hexdigest()
|
||||
|
||||
|
||||
def _write_file(path: Path, content: bytes) -> str:
|
||||
"""Write bytes to *path*, creating parents, and return the string path."""
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(content)
|
||||
return str(path)
|
||||
|
||||
|
||||
def _make_manager(
|
||||
tmp_path: Path,
|
||||
file_mapping: list[tuple[str, str]] | None = None,
|
||||
bulk_download_fn=None,
|
||||
) -> FileSyncManager:
|
||||
"""Create a FileSyncManager wired for testing.
|
||||
|
||||
*file_mapping* is a list of (host_path, remote_path) tuples that
|
||||
``get_files_fn`` returns. If *None* an empty list is used.
|
||||
"""
|
||||
mapping = file_mapping or []
|
||||
return FileSyncManager(
|
||||
get_files_fn=lambda: mapping,
|
||||
upload_fn=MagicMock(),
|
||||
delete_fn=MagicMock(),
|
||||
bulk_download_fn=bulk_download_fn,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSyncBackNoop:
|
||||
"""sync_back() is a no-op when there is no download function."""
|
||||
|
||||
def test_sync_back_noop_without_download_fn(self, tmp_path):
|
||||
mgr = _make_manager(tmp_path, bulk_download_fn=None)
|
||||
# Should return immediately without error
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
# Nothing to assert beyond "no exception raised"
|
||||
|
||||
|
||||
class TestSyncBackNoChanges:
|
||||
"""When all remote files match pushed hashes, nothing is applied."""
|
||||
|
||||
def test_sync_back_no_changes(self, tmp_path):
|
||||
host_file = tmp_path / "host" / "cred.json"
|
||||
host_content = b'{"key": "val"}'
|
||||
_write_file(host_file, host_content)
|
||||
|
||||
remote_path = "/root/.hermes/cred.json"
|
||||
mapping = [(str(host_file), remote_path)]
|
||||
|
||||
# Remote tar contains the same content as was pushed
|
||||
download_fn = _make_download_fn({
|
||||
"root/.hermes/cred.json": host_content,
|
||||
})
|
||||
|
||||
mgr = _make_manager(tmp_path, file_mapping=mapping, bulk_download_fn=download_fn)
|
||||
# Simulate that we already pushed this file with this hash
|
||||
mgr._pushed_hashes[remote_path] = _sha256_bytes(host_content)
|
||||
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
# Host file should be unchanged (same content, same bytes)
|
||||
assert host_file.read_bytes() == host_content
|
||||
|
||||
|
||||
class TestSyncBackAppliesChanged:
|
||||
"""Remote file differs from pushed version -- gets copied to host."""
|
||||
|
||||
def test_sync_back_applies_changed_file(self, tmp_path):
|
||||
host_file = tmp_path / "host" / "skill.py"
|
||||
original_content = b"print('v1')"
|
||||
_write_file(host_file, original_content)
|
||||
|
||||
remote_path = "/root/.hermes/skill.py"
|
||||
mapping = [(str(host_file), remote_path)]
|
||||
|
||||
remote_content = b"print('v2 - edited on remote')"
|
||||
download_fn = _make_download_fn({
|
||||
"root/.hermes/skill.py": remote_content,
|
||||
})
|
||||
|
||||
mgr = _make_manager(tmp_path, file_mapping=mapping, bulk_download_fn=download_fn)
|
||||
mgr._pushed_hashes[remote_path] = _sha256_bytes(original_content)
|
||||
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
assert host_file.read_bytes() == remote_content
|
||||
|
||||
|
||||
class TestSyncBackNewRemoteFile:
|
||||
"""File created on remote (not in _pushed_hashes) is applied via _infer_host_path."""
|
||||
|
||||
def test_sync_back_detects_new_remote_file(self, tmp_path):
|
||||
# Existing mapping gives _infer_host_path a prefix to work with
|
||||
existing_host = tmp_path / "host" / "skills" / "existing.py"
|
||||
_write_file(existing_host, b"existing")
|
||||
mapping = [(str(existing_host), "/root/.hermes/skills/existing.py")]
|
||||
|
||||
# Remote has a NEW file in the same directory that was never pushed
|
||||
new_remote_content = b"# brand new skill created on remote"
|
||||
download_fn = _make_download_fn({
|
||||
"root/.hermes/skills/new_skill.py": new_remote_content,
|
||||
})
|
||||
|
||||
mgr = _make_manager(tmp_path, file_mapping=mapping, bulk_download_fn=download_fn)
|
||||
# No entry in _pushed_hashes for the new file
|
||||
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
# The new file should have been inferred and written to the host
|
||||
expected_host_path = tmp_path / "host" / "skills" / "new_skill.py"
|
||||
assert expected_host_path.exists()
|
||||
assert expected_host_path.read_bytes() == new_remote_content
|
||||
|
||||
|
||||
class TestSyncBackConflict:
|
||||
"""Host AND remote both changed since push -- warning logged, remote wins."""
|
||||
|
||||
def test_sync_back_conflict_warns(self, tmp_path, caplog):
|
||||
host_file = tmp_path / "host" / "config.json"
|
||||
original_content = b'{"v": 1}'
|
||||
_write_file(host_file, original_content)
|
||||
|
||||
remote_path = "/root/.hermes/config.json"
|
||||
mapping = [(str(host_file), remote_path)]
|
||||
|
||||
# Host was modified after push
|
||||
host_file.write_bytes(b'{"v": 2, "host-edit": true}')
|
||||
|
||||
# Remote was also modified
|
||||
remote_content = b'{"v": 3, "remote-edit": true}'
|
||||
download_fn = _make_download_fn({
|
||||
"root/.hermes/config.json": remote_content,
|
||||
})
|
||||
|
||||
mgr = _make_manager(tmp_path, file_mapping=mapping, bulk_download_fn=download_fn)
|
||||
mgr._pushed_hashes[remote_path] = _sha256_bytes(original_content)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="tools.environments.file_sync"):
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
# Conflict warning was logged
|
||||
assert any("conflict" in r.message.lower() for r in caplog.records)
|
||||
|
||||
# Remote version wins (last-write-wins)
|
||||
assert host_file.read_bytes() == remote_content
|
||||
|
||||
|
||||
class TestSyncBackRetries:
|
||||
"""Retry behaviour with exponential backoff."""
|
||||
|
||||
@patch("tools.environments.file_sync.time.sleep")
|
||||
def test_sync_back_retries_on_failure(self, mock_sleep, tmp_path):
|
||||
call_count = 0
|
||||
|
||||
def flaky_download(dest: Path):
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
if call_count < 3:
|
||||
raise RuntimeError(f"network error #{call_count}")
|
||||
# Third attempt succeeds -- write a valid (empty) tar
|
||||
_make_tar({}, dest)
|
||||
|
||||
mgr = _make_manager(tmp_path, bulk_download_fn=flaky_download)
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
assert call_count == 3
|
||||
# Sleep called twice (between attempt 1->2 and 2->3)
|
||||
assert mock_sleep.call_count == 2
|
||||
mock_sleep.assert_any_call(_SYNC_BACK_BACKOFF[0])
|
||||
mock_sleep.assert_any_call(_SYNC_BACK_BACKOFF[1])
|
||||
|
||||
@patch("tools.environments.file_sync.time.sleep")
|
||||
def test_sync_back_all_retries_exhausted(self, mock_sleep, tmp_path, caplog):
|
||||
def always_fail(dest: Path):
|
||||
raise RuntimeError("persistent failure")
|
||||
|
||||
mgr = _make_manager(tmp_path, bulk_download_fn=always_fail)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="tools.environments.file_sync"):
|
||||
# Should NOT raise -- failures are logged, not propagated
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
# All retries were attempted
|
||||
assert mock_sleep.call_count == _SYNC_BACK_MAX_RETRIES - 1
|
||||
|
||||
# Final "all attempts failed" warning was logged
|
||||
assert any("all" in r.message.lower() and "failed" in r.message.lower() for r in caplog.records)
|
||||
|
||||
|
||||
class TestPushedHashesPopulated:
|
||||
"""_pushed_hashes is populated during sync() and cleared on delete."""
|
||||
|
||||
def test_pushed_hashes_populated_on_sync(self, tmp_path):
|
||||
host_file = tmp_path / "data.txt"
|
||||
host_file.write_bytes(b"hello world")
|
||||
|
||||
remote_path = "/root/.hermes/data.txt"
|
||||
mapping = [(str(host_file), remote_path)]
|
||||
|
||||
mgr = FileSyncManager(
|
||||
get_files_fn=lambda: mapping,
|
||||
upload_fn=MagicMock(),
|
||||
delete_fn=MagicMock(),
|
||||
)
|
||||
|
||||
mgr.sync(force=True)
|
||||
|
||||
assert remote_path in mgr._pushed_hashes
|
||||
assert mgr._pushed_hashes[remote_path] == _sha256_file(str(host_file))
|
||||
|
||||
def test_pushed_hashes_cleared_on_delete(self, tmp_path):
|
||||
host_file = tmp_path / "deleteme.txt"
|
||||
host_file.write_bytes(b"to be deleted")
|
||||
|
||||
remote_path = "/root/.hermes/deleteme.txt"
|
||||
mapping = [(str(host_file), remote_path)]
|
||||
current_mapping = list(mapping)
|
||||
|
||||
mgr = FileSyncManager(
|
||||
get_files_fn=lambda: current_mapping,
|
||||
upload_fn=MagicMock(),
|
||||
delete_fn=MagicMock(),
|
||||
)
|
||||
|
||||
# Sync to populate hashes
|
||||
mgr.sync(force=True)
|
||||
assert remote_path in mgr._pushed_hashes
|
||||
|
||||
# Remove the file from the mapping (simulates local deletion)
|
||||
os.unlink(str(host_file))
|
||||
current_mapping.clear()
|
||||
|
||||
mgr.sync(force=True)
|
||||
|
||||
# Hash should be cleaned up
|
||||
assert remote_path not in mgr._pushed_hashes
|
||||
|
||||
|
||||
class TestSyncBackFileLock:
|
||||
"""Verify that fcntl.flock is used during sync-back."""
|
||||
|
||||
@patch("tools.environments.file_sync.fcntl.flock")
|
||||
def test_sync_back_file_lock(self, mock_flock, tmp_path):
|
||||
download_fn = _make_download_fn({})
|
||||
mgr = _make_manager(tmp_path, bulk_download_fn=download_fn)
|
||||
|
||||
mgr.sync_back(hermes_home=tmp_path / ".hermes")
|
||||
|
||||
# flock should have been called at least twice: LOCK_EX to acquire, LOCK_UN to release
|
||||
assert mock_flock.call_count >= 2
|
||||
|
||||
lock_calls = mock_flock.call_args_list
|
||||
lock_ops = [c[0][1] for c in lock_calls]
|
||||
assert fcntl.LOCK_EX in lock_ops
|
||||
assert fcntl.LOCK_UN in lock_ops
|
||||
Loading…
Add table
Add a link
Reference in a new issue