diff --git a/backend/packages/harness/deerflow/persistence/agents/file.py b/backend/packages/harness/deerflow/persistence/agents/file.py index 349dd25865..94a132d0e0 100644 --- a/backend/packages/harness/deerflow/persistence/agents/file.py +++ b/backend/packages/harness/deerflow/persistence/agents/file.py @@ -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 diff --git a/backend/packages/harness/deerflow/persistence/managed_subagents/file.py b/backend/packages/harness/deerflow/persistence/managed_subagents/file.py index 3ab7e95c24..ba10f4c77e 100644 --- a/backend/packages/harness/deerflow/persistence/managed_subagents/file.py +++ b/backend/packages/harness/deerflow/persistence/managed_subagents/file.py @@ -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: diff --git a/backend/tests/test_atomic_cleanup_census.py b/backend/tests/test_atomic_cleanup_census.py index 37a6c3fc31..b647b96b0b 100644 --- a/backend/tests/test_atomic_cleanup_census.py +++ b/backend/tests/test_atomic_cleanup_census.py @@ -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()] == []