feat(file-sync): sync remote changes back to host on teardown#8189
Closed
kshitijk4poor wants to merge 1 commit into
Closed
feat(file-sync): sync remote changes back to host on teardown#8189kshitijk4poor wants to merge 1 commit into
kshitijk4poor wants to merge 1 commit into
Conversation
Salvage of PR NousResearch#8018 by @alt-glitch onto current main. On sandbox teardown, FileSyncManager now downloads the remote .hermes/ directory, diffs against SHA-256 hashes of what was originally pushed, and applies only changed files back to the host. Core (tools/environments/file_sync.py): - sync_back(): orchestrates download -> unpack -> diff -> apply with: - Retry with exponential backoff (3 attempts, 2s/4s/8s) - SIGINT trap + defer (prevents partial writes on Ctrl-C) - fcntl.flock serialization (concurrent gateway sandboxes) - Last-write-wins conflict resolution with warning - New remote files pulled back via _infer_host_path prefix matching Backends: - SSH: _ssh_bulk_download — tar cf - piped over SSH - Modal: _modal_bulk_download — exec tar cf - -> proc.stdout.read - Daytona: _daytona_bulk_download — exec tar cf -> SDK download_file - All three call sync_back() at the top of cleanup() Fixes applied during salvage (vs original PR NousResearch#8018): | # | Issue | Fix | |---|-------|-----| | C1 | import fcntl unconditional — crashes Windows | try/except with fallback; _sync_back_locked skips locking when fcntl=None | | W1 | assert for runtime guard (stripped by -O) | Replaced with proper if/raise RuntimeError | | W2 | O(n*m) from _get_files_fn() called per file | Cache mapping once at start of _sync_back_impl, pass to resolve/infer | | W3 | Dead BulkDownloadFn imports in 3 backends | Removed unused imports | | W4 | Modal hardcodes root/.hermes, no explanation | Added docstring comment explaining Modal always runs as root | | S1 | SHA-256 computed for new files where pushed_hash=None | Skip hashing when pushed_hash is None (comparison always False) | | S2 | Daytona /tmp/.hermes_sync.tar never cleaned up | Added rm -f after download (best-effort) | Tests: 49 passing (17 new: _infer_host_path edge cases, SIGINT main/worker thread, Windows fcntl=None fallback, Daytona tar cleanup). Based on NousResearch#8018 by @alt-glitch.
4 tasks
Contributor
|
Merged via #11291. Your commit was cherry-picked onto current main with authorship preserved (commit d64446e). Thanks for the thorough salvage of #8018 — every review item from the previous round was addressed and the 17 extra tests you added caught real edge cases. Follow-up hardening on top: PID-suffix on Daytona temp path, 2 GiB size cap, 'nothing previously pushed' guard, and lifecycle ordering fix in Daytona cleanup(). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of PR #8018 by @alt-glitch onto current main. Implements Phase 2 of the bulk file sync spec — on sandbox teardown,
FileSyncManagerdownloads the remote.hermes/directory, diffs against SHA-256 hashes of what was originally pushed, and applies only changed files back to the host.Depends on #8014 (bulk upload — already merged as base).
Changes
Core (
tools/environments/file_sync.py)sync_back(): orchestrates download → unpack → diff → apply with:fcntl.flockserialization (concurrent gateway sandboxes)_infer_host_pathprefix matchingBackends
_ssh_bulk_download—tar cf -piped over SSH_modal_bulk_download—exec tar cf -→proc.stdout.read_daytona_bulk_download—exec tar cf→ SDKdownload_filesync_back()at the top ofcleanup()Fixes applied during salvage (vs original PR #8018)
import fcntlunconditional — crashes on Windowstry/except ImportErrorwith fallback;_sync_back_lockedskips locking whenfcntl is Noneassertused for runtime guard (stripped bypython -O)if/raise RuntimeError_get_files_fn()called per file in inner loop_sync_back_impl, pass to_resolve_host_path/_infer_host_pathBulkDownloadFnimports in SSH/Modal/Daytonaroot/.hermeswith no explanationpushed_hash=Nonepushed_hash is None(comparison is always False)/tmp/.hermes_sync.tarnever cleaned up on remoterm -fafter download (best-effort cleanup)Tests
49 passing (17 new tests added during salvage):
_infer_host_pathedge cases: no match, partial prefix false match, correct matchfcntl=Noneskips file locking without errorrm -fcalled after downloadBased on #8018 by @alt-glitch.