fix(persistence): discard the staged temp when the write itself fails (#7504)

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
llb555 2026-10-27 11:47:28 +08:00 committed by GitHub
parent bfd599e840
commit 68c9fce5c4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 67 additions and 7 deletions

View File

@ -246,12 +246,10 @@ class FileAgentStore(AgentStore):
try:
if config is not None:
config_text = yaml.dump(config, default_flow_style=False, allow_unicode=True, sort_keys=False)
config_tmp = _stage_temp(agent_dir / "config.yaml", config_text)
staged.append(config_tmp)
config_tmp = _stage_temp(agent_dir / "config.yaml", config_text, staged)
pending.append((config_tmp, agent_dir / "config.yaml"))
if soul is not None:
soul_tmp = _stage_temp(agent_dir / SOUL_FILENAME, soul)
staged.append(soul_tmp)
soul_tmp = _stage_temp(agent_dir / SOUL_FILENAME, soul, staged)
pending.append((soul_tmp, agent_dir / SOUL_FILENAME))
for tmp, target in pending:
tmp.replace(target)
@ -263,10 +261,14 @@ class FileAgentStore(AgentStore):
cleanup_failed_atomic_write(tmp)
def _stage_temp(target: Path, text: str) -> Path:
def _stage_temp(target: Path, text: str, staged: list[Path]) -> Path:
# newline="" keeps the operator's/agent's bytes: the default text write
# translates LF into os.linesep, so SOUL.md and config.yaml landed with
# CRLF on Windows while every reader saw LF.
with tempfile.NamedTemporaryFile("w", encoding="utf-8", newline="", dir=target.parent, suffix=".tmp", delete=False) as f:
# Register the path before writing: a failing ``write`` must still reach
# the caller's cleanup, or the staged file leaks with no attempt and no log.
path = Path(f.name)
staged.append(path)
f.write(text)
return Path(f.name)
return path

View File

@ -90,10 +90,12 @@ class FileManagedSubagentStore(ManagedSubagentStore):
tmp_path: Path | None = None
try:
with tempfile.NamedTemporaryFile("w", encoding="utf-8", newline="", dir=path.parent, suffix=".tmp", delete=False) as handle:
# Record the staged path before writing: a failing ``write``/``fsync``
# must still reach the cleanup below, or the temp file leaks silently.
tmp_path = Path(handle.name)
handle.write(payload)
handle.flush()
os.fsync(handle.fileno())
tmp_path = Path(handle.name)
os.replace(tmp_path, path)
tmp_path = None
finally:

View File

@ -69,3 +69,59 @@ def test_agent_store_write_keeps_the_publish_error_when_cleanup_fails(tmp_path,
with pytest.raises(OSError, match="replace failed"):
FileAgentStore._write(agent_dir, {"name": "planner"}, "# SOUL")
def test_managed_subagent_write_removes_the_temp_when_the_write_fails(tmp_path, monkeypatch):
"""A failure before the publish still has to discard the staged file."""
from deerflow.persistence.managed_subagents import file as store_module
from deerflow.persistence.managed_subagents.base import ManagedSubagentDefinition
definition = ManagedSubagentDefinition(
name="planner",
description="plans",
system_prompt="plan things",
)
def _failing_fsync(*_args, **_kwargs):
raise OSError("fsync failed")
monkeypatch.setattr(store_module.os, "fsync", _failing_fsync)
with pytest.raises(OSError, match="fsync failed"):
store_module.FileManagedSubagentStore._atomic_write(tmp_path / "planner.json", definition)
assert [p.name for p in tmp_path.iterdir()] == []
def test_agent_store_write_removes_the_temp_when_the_write_fails(tmp_path, monkeypatch):
"""The staging helper must register its path before writing the payload."""
from deerflow.persistence.agents import file as store_module
agent_dir = tmp_path / "agents"
agent_dir.mkdir()
real_named_tempfile = store_module.tempfile.NamedTemporaryFile
class _FailingWrite:
"""A real staged file whose payload write fails."""
def __init__(self, *args, **kwargs):
self._inner = real_named_tempfile(*args, **kwargs)
self.name = self._inner.name
def __enter__(self):
self._inner.__enter__()
return self
def __exit__(self, *exc_info):
return self._inner.__exit__(*exc_info)
def write(self, *_args, **_kwargs):
raise OSError("write failed")
monkeypatch.setattr(store_module.tempfile, "NamedTemporaryFile", _FailingWrite)
with pytest.raises(OSError, match="write failed"):
store_module.FileAgentStore._write(agent_dir, {"name": "planner"}, "# SOUL")
assert [p.name for p in agent_dir.iterdir()] == []