mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(e2b): preserve trailing whitespace in filenames and survive mtime overflow (#4861)
* fix(e2b): preserve trailing whitespace in filenames and survive mtime overflow _sync_outputs_to_host iterated the NUL-delimited find output with entry.strip() on each record. NUL already guarantees record boundaries, so the strip is redundant and harmful: a filename that legitimately ends in whitespace (e.g. "report ") had its trailing space trimmed, pointing host_path at the wrong file and recording a manifest key that can never match — the file was re-downloaded on every release. The same host-write block wrapped only os.utime in the outer except OSError, but os.utime raises OverflowError (not an OSError) when the ns value is out of range (a far-future remote mtime, e.g. `touch -d '99999 years'`). That escaped the loop, skipping the manifest write and forcing a full re-download next release. Wrap os.utime in its own (OSError, OverflowError) so only the timestamp restoration is dropped; the file is still written and the manifest still updated. * test(e2b): rely on monkeypatch cleanup --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
0a3c04ebcc
commit
38440949c6
@ -2016,7 +2016,10 @@ class E2BSandboxProvider(SandboxProvider):
|
||||
if time.monotonic() >= deadline:
|
||||
truncated_reason = f"time budget {self._SYNC_DEADLINE_SECONDS}s"
|
||||
break
|
||||
entry = entry.strip()
|
||||
# NUL already delimits records, so do NOT strip: a filename that
|
||||
# legitimately ends in whitespace (e.g. "report ") would have its
|
||||
# trailing space trimmed here, pointing host_path at the wrong
|
||||
# file and recording a manifest key that never matches again.
|
||||
if not entry:
|
||||
continue
|
||||
try:
|
||||
@ -2096,7 +2099,21 @@ class E2BSandboxProvider(SandboxProvider):
|
||||
host_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp_path = host_path.with_name(host_path.name + ".e2bsync.tmp")
|
||||
tmp_path.write_bytes(data)
|
||||
os.utime(tmp_path, ns=(remote_mtime_ns, remote_mtime_ns))
|
||||
# os.utime rejects ns values outside roughly +/-2^63; a remote
|
||||
# mtime far in the future (e.g. `touch -d "99999 years" file`)
|
||||
# raises OverflowError, which is not an OSError and would
|
||||
# escape the outer except below, skipping the manifest write
|
||||
# and forcing a full re-download next release. Drop only the
|
||||
# timestamp restoration in that case — the file is still
|
||||
# written correctly.
|
||||
try:
|
||||
os.utime(tmp_path, ns=(remote_mtime_ns, remote_mtime_ns))
|
||||
except (OSError, OverflowError):
|
||||
logger.debug(
|
||||
"e2b sync: skipped mtime restoration for %s (ns=%d)",
|
||||
host_path,
|
||||
remote_mtime_ns,
|
||||
)
|
||||
tmp_path.replace(host_path)
|
||||
host_stat = host_path.stat()
|
||||
manifest[manifest_key] = {
|
||||
|
||||
@ -2173,6 +2173,54 @@ def test_sync_outputs_to_host_removes_manifest_entries_for_deleted_files(monkeyp
|
||||
assert set(manifest["files"]) == {"outputs/live.txt"}
|
||||
|
||||
|
||||
def test_sync_outputs_to_host_preserves_trailing_space_in_filename(monkeypatch, tmp_path):
|
||||
p = _make_provider()
|
||||
_setup_paths(monkeypatch, tmp_path)
|
||||
# "report " (trailing space) is a legal Linux filename; the NUL-delimited
|
||||
# listing preserves it, but a .strip() on each entry would truncate it.
|
||||
listing = "5\t2.000000000\t/home/user/outputs/report \x00"
|
||||
files = FakeFilesAPI(store={"/home/user/outputs/report ": b"hello"})
|
||||
cmds = FakeCommandsAPI([SimpleNamespace(stdout=listing, stderr="", exit_code=0)])
|
||||
client = FakeClient(commands=cmds, files=files)
|
||||
sb = _make_sandbox(client, sandbox_id="sb-sync-space")
|
||||
|
||||
p._sync_outputs_to_host(sb, thread_id="t1", user_id="u1")
|
||||
|
||||
expected = Paths(base_dir=tmp_path).thread_dir("t1", user_id="u1") / "user-data" / "outputs" / "report "
|
||||
assert expected.exists()
|
||||
assert expected.read_bytes() == b"hello"
|
||||
|
||||
|
||||
def test_sync_outputs_to_host_skips_mtime_restoration_on_overflow(monkeypatch, tmp_path):
|
||||
p = _make_provider()
|
||||
_setup_paths(monkeypatch, tmp_path)
|
||||
# os.utime raises OverflowError (not OSError) when the ns value is out of
|
||||
# range; the exact threshold is platform-dependent (macOS clamps, Linux
|
||||
# raises), so force the failure deterministically and assert the file is
|
||||
# still written and the manifest still updated.
|
||||
listing = "5\t1720000000.1234567890\t/home/user/outputs/far-future.txt\x00"
|
||||
files = FakeFilesAPI(store={"/home/user/outputs/far-future.txt": b"hello"})
|
||||
cmds = FakeCommandsAPI([SimpleNamespace(stdout=listing, stderr="", exit_code=0)])
|
||||
client = FakeClient(commands=cmds, files=files)
|
||||
sb = _make_sandbox(client, sandbox_id="sb-sync-overflow")
|
||||
|
||||
e2b_provider_mod = importlib.import_module("deerflow.community.e2b_sandbox.e2b_sandbox_provider")
|
||||
|
||||
def _raise_overflow(path, times=None, ns=None):
|
||||
raise OverflowError("timestamp out of range")
|
||||
|
||||
monkeypatch.setattr(e2b_provider_mod.os, "utime", _raise_overflow)
|
||||
|
||||
p._sync_outputs_to_host(sb, thread_id="t1", user_id="u1")
|
||||
|
||||
paths = Paths(base_dir=tmp_path).thread_dir("t1", user_id="u1")
|
||||
target = paths / "user-data" / "outputs" / "far-future.txt"
|
||||
assert target.exists()
|
||||
assert target.read_bytes() == b"hello"
|
||||
manifest = json.loads((paths / ".e2b-output-sync.json").read_text(encoding="utf-8"))
|
||||
assert manifest["files"]["outputs/far-future.txt"]["remote_size"] == 5
|
||||
|
||||
|
||||
def test_sync_outputs_to_host_discards_manifest_from_another_sandbox(monkeypatch, tmp_path):
|
||||
p = _make_provider()
|
||||
_setup_paths(monkeypatch, tmp_path)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user