mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(extensions): publish a snapshot restore atomically (#6366)
* fix(extensions): publish a snapshot restore atomically _FileSnapshot.restore() rolled a failed extension install back with Path.write_bytes, which opens with O_TRUNC: if the process died during the rollback, pyproject.toml / uv.lock / config.yaml were left half written and the failed install took the checkout with it. Publish through a temporary file plus os.replace, the pattern the rest of the codebase already uses. * fix(extensions): preserve snapshot permissions on rollback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
3725ba1bfb
commit
03a8ad3071
@ -1641,8 +1641,12 @@ Management commands bootstrap the checkout environment without the extension gro
|
||||
when an installed extension's remote source or managed snapshot has become unavailable,
|
||||
while a fresh checkout can still create the non-extension environment from the existing lock. The
|
||||
manager itself owns the subsequent locked dependency transaction.
|
||||
Mutations for one checkout are serialized through a process lock. The initial manager
|
||||
surface is create/remove rather than in-place upgrade: to change an installed source, save
|
||||
Mutations for one checkout are serialized through a process lock.
|
||||
Failed extension operations restore dependency files and configuration atomically with
|
||||
their original file permissions, preserving access for a Gateway that reads them through
|
||||
group permissions.
|
||||
The initial manager surface is create/remove rather than in-place upgrade: to change an
|
||||
installed source, save
|
||||
its private `plugins[].config`, remove it, reinstall the new pin, and restore that config.
|
||||
|
||||
Local-directory installs are copied into
|
||||
|
||||
@ -43,6 +43,10 @@ case rather than reviving a record whose package declaration may already be gone
|
||||
cancellation skips the recovery sync entirely — the declarations are already restored and
|
||||
the next locked startup sync reconciles the environment, whereas blocking an interrupt on a
|
||||
full dependency resolve invites a second interrupt that escapes the handler mid-transaction.
|
||||
File snapshots capture the original permission bits alongside the bytes; rollback applies
|
||||
that mode to a temporary file before atomically replacing the destination, including when
|
||||
restoring `config.yaml` after a failed removal. A failed write, chmod, or replace leaves the
|
||||
current destination untouched and removes the temporary file.
|
||||
Package mutation is deferred from environment mutation: after `uv add/remove`
|
||||
updates the declaration and lock, one `uv sync --locked --all-packages` preserves the same
|
||||
config-/environment-detected optional extras as normal startup. All three uv calls pin the
|
||||
|
||||
@ -9,6 +9,7 @@ import os
|
||||
import re
|
||||
import shlex
|
||||
import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
@ -96,20 +97,46 @@ class ConfiguredExtension:
|
||||
required: bool
|
||||
|
||||
|
||||
def _write_bytes_atomically(path: Path, content: bytes, mode: int | None) -> None:
|
||||
"""Publish *content* at *path* so an interrupted restore cannot truncate it.
|
||||
|
||||
``Path.write_bytes`` opens with ``O_TRUNC``, so a rollback that dies midway
|
||||
leaves pyproject.toml / uv.lock / config.yaml half written — the failed
|
||||
install would then take the checkout with it. The rest of the codebase
|
||||
already publishes through a temporary file plus ``os.replace``.
|
||||
"""
|
||||
fd, temporary = tempfile.mkstemp(dir=path.parent, prefix=f".{path.name}.", suffix=".tmp")
|
||||
try:
|
||||
with os.fdopen(fd, "wb") as handle:
|
||||
handle.write(content)
|
||||
handle.flush()
|
||||
if mode is not None:
|
||||
os.chmod(temporary, mode)
|
||||
os.fsync(handle.fileno())
|
||||
os.replace(temporary, path)
|
||||
except BaseException:
|
||||
Path(temporary).unlink(missing_ok=True)
|
||||
raise
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class _FileSnapshot:
|
||||
path: Path
|
||||
content: bytes | None
|
||||
mode: int | None
|
||||
|
||||
@classmethod
|
||||
def capture(cls, path: Path) -> _FileSnapshot:
|
||||
return cls(path, path.read_bytes() if path.exists() else None)
|
||||
if not path.exists():
|
||||
return cls(path, None, None)
|
||||
with path.open("rb") as handle:
|
||||
return cls(path, handle.read(), stat.S_IMODE(os.fstat(handle.fileno()).st_mode))
|
||||
|
||||
def restore(self) -> None:
|
||||
if self.content is None:
|
||||
self.path.unlink(missing_ok=True)
|
||||
else:
|
||||
self.path.write_bytes(self.content)
|
||||
_write_bytes_atomically(self.path, self.content, self.mode)
|
||||
|
||||
|
||||
def _read_optional_bytes(path: Path) -> bytes | None:
|
||||
|
||||
107
backend/tests/test_extension_snapshot_restore.py
Normal file
107
backend/tests/test_extension_snapshot_restore.py
Normal file
@ -0,0 +1,107 @@
|
||||
# SPDX-License-Identifier: MIT
|
||||
"""A failed extension install must not corrupt the files it rolls back."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import stat
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.extensions.manager import _FileSnapshot
|
||||
|
||||
|
||||
def test_restore_keeps_the_previous_content_when_the_publish_fails(tmp_path, monkeypatch):
|
||||
target = tmp_path / "pyproject.toml"
|
||||
target.write_text("original\n", encoding="utf-8")
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
|
||||
# The install failed, so the file was rewritten; now the rollback runs.
|
||||
target.write_text("half written by the failed install", encoding="utf-8")
|
||||
|
||||
def _boom(*args, **kwargs):
|
||||
raise OSError("device full")
|
||||
|
||||
monkeypatch.setattr(os, "replace", _boom)
|
||||
|
||||
with pytest.raises(OSError):
|
||||
snapshot.restore()
|
||||
|
||||
# write_bytes would have truncated the file before failing, taking the
|
||||
# checkout with it; publishing through a temporary file cannot.
|
||||
assert target.read_text(encoding="utf-8") == "half written by the failed install"
|
||||
|
||||
|
||||
def test_restore_still_replaces_the_file(tmp_path):
|
||||
target = tmp_path / "config.yaml"
|
||||
target.write_text("original\n", encoding="utf-8")
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
|
||||
target.write_text("changed\n", encoding="utf-8")
|
||||
snapshot.restore()
|
||||
|
||||
assert target.read_text(encoding="utf-8") == "original\n"
|
||||
assert list(tmp_path.glob(".*tmp")) == []
|
||||
|
||||
|
||||
def test_restore_removes_a_file_that_did_not_exist(tmp_path):
|
||||
target = tmp_path / "uv.lock"
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
target.write_text("created by the failed install\n", encoding="utf-8")
|
||||
|
||||
snapshot.restore()
|
||||
|
||||
assert not target.exists()
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "posix", reason="POSIX file permissions")
|
||||
@pytest.mark.parametrize("mode", [0o644, 0o640, 0o600])
|
||||
def test_restore_preserves_the_captured_permissions(tmp_path, mode):
|
||||
target = tmp_path / "config.yaml"
|
||||
target.write_text("original\n", encoding="utf-8")
|
||||
target.chmod(mode)
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
|
||||
target.write_text("changed\n", encoding="utf-8")
|
||||
target.chmod(0o660)
|
||||
snapshot.restore()
|
||||
|
||||
assert target.read_text(encoding="utf-8") == "original\n"
|
||||
assert stat.S_IMODE(target.stat().st_mode) == mode
|
||||
assert list(tmp_path.glob(".*tmp")) == []
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "posix", reason="POSIX file permissions")
|
||||
def test_restore_recreates_a_deleted_file_with_its_captured_permissions(tmp_path):
|
||||
target = tmp_path / "uv.lock"
|
||||
target.write_text("original\n", encoding="utf-8")
|
||||
target.chmod(0o644)
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
target.unlink()
|
||||
|
||||
snapshot.restore()
|
||||
|
||||
assert target.read_text(encoding="utf-8") == "original\n"
|
||||
assert stat.S_IMODE(target.stat().st_mode) == 0o644
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "posix", reason="POSIX file permissions")
|
||||
def test_restore_keeps_the_current_file_when_setting_permissions_fails(tmp_path, monkeypatch):
|
||||
target = tmp_path / "config.yaml"
|
||||
target.write_text("original\n", encoding="utf-8")
|
||||
target.chmod(0o640)
|
||||
snapshot = _FileSnapshot.capture(target)
|
||||
target.write_text("changed\n", encoding="utf-8")
|
||||
target.chmod(0o660)
|
||||
|
||||
def _boom(*args, **kwargs):
|
||||
raise OSError("cannot set permissions")
|
||||
|
||||
monkeypatch.setattr(os, "chmod", _boom)
|
||||
|
||||
with pytest.raises(OSError, match="cannot set permissions"):
|
||||
snapshot.restore()
|
||||
|
||||
assert target.read_text(encoding="utf-8") == "changed\n"
|
||||
assert stat.S_IMODE(target.stat().st_mode) == 0o660
|
||||
assert list(tmp_path.glob(".*tmp")) == []
|
||||
Loading…
x
Reference in New Issue
Block a user