From 19768d73cc236e73f8e039fe6a1213c78ab6b56f Mon Sep 17 00:00:00 2001 From: llb555 <3103086905@qq.com> Date: Mon, 26 Oct 2026 23:33:49 +0800 Subject: [PATCH] fix(setup-agent): keep a failed discard from replacing the publish outcome (#7564) * fix(setup-agent): keep a failed discard from replacing the publish outcome * test(setup-agent): pin the post-publish discard and assert the real temp name --------- Co-authored-by: Willem Jiang --- .../tools/builtins/setup_agent_tool.py | 5 +- .../tests/test_setup_agent_atomic_cleanup.py | 78 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 backend/tests/test_setup_agent_atomic_cleanup.py diff --git a/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py b/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py index 8979bf8e36..3297a4378e 100644 --- a/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py @@ -14,6 +14,7 @@ from deerflow.knowledge_scope import canonicalize_knowledge_scope from deerflow.persistence.agents import get_agent_store from deerflow.runtime.user_context import resolve_runtime_user_id from deerflow.tools.types import Runtime +from deerflow.utils.atomic_write import cleanup_failed_atomic_write logger = logging.getLogger(__name__) @@ -48,7 +49,9 @@ def _publish_text(target: Path, text: str) -> None: temporary.replace(target) finally: if temporary is not None: - temporary.unlink(missing_ok=True) + # The publish outcome is already decided; a temp we cannot discard + # must not replace it with the cleanup error. + cleanup_failed_atomic_write(temporary) @tool(parse_docstring=True) diff --git a/backend/tests/test_setup_agent_atomic_cleanup.py b/backend/tests/test_setup_agent_atomic_cleanup.py new file mode 100644 index 0000000000..1cb91bec18 --- /dev/null +++ b/backend/tests/test_setup_agent_atomic_cleanup.py @@ -0,0 +1,78 @@ +"""A failed discard must not replace the outcome of an agent file publish. + +``_publish_text`` writes an agent's ``SOUL.md``/config through a same-directory +temporary. On Windows a scanner or a peer holding the handle can make the +removal fail after a successful ``os.replace`` — or while another error is +already propagating — and the cleanup error used to win. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + + +@pytest.fixture +def real_unlink(): + return Path.unlink + + +def _fail_unlink_on(monkeypatch, predicate, real_unlink): + def unlink(self, *args, **kwargs): + if predicate(self): + raise PermissionError("temporary unlink failed") + return real_unlink(self, *args, **kwargs) + + monkeypatch.setattr(Path, "unlink", unlink) + + +def test_publish_text_keeps_the_replace_error_when_cleanup_fails(tmp_path, monkeypatch, real_unlink): + from deerflow.tools.builtins import setup_agent_tool as module + + target = tmp_path / "SOUL.md" + target.write_text("original", encoding="utf-8") + + def _failing_replace(self, *args, **kwargs): + raise OSError("replace failed") + + monkeypatch.setattr(Path, "replace", _failing_replace) + _fail_unlink_on(monkeypatch, lambda path: path.suffix == ".tmp", real_unlink) + + with pytest.raises(OSError, match="replace failed"): + module._publish_text(target, "replacement") + + assert target.read_text(encoding="utf-8") == "original" + + +def test_publish_text_still_succeeds_and_leaves_no_temporary(tmp_path, caplog): + from deerflow.tools.builtins import setup_agent_tool as module + + target = tmp_path / "SOUL.md" + target.write_text("original", encoding="utf-8") + caplog.clear() + + with caplog.at_level("WARNING", logger="deerflow.utils.atomic_write"): + module._publish_text(target, "replacement") + + assert target.read_text(encoding="utf-8") == "replacement" + # The staged name comes from ``NamedTemporaryFile`` (``tmpXXXXXXXX.tmp``), so + # assert the whole directory: nothing leaked and nothing else was created. + assert sorted(p.name for p in tmp_path.iterdir()) == ["SOUL.md"] + assert caplog.text == "" + + +def test_publish_text_survives_a_discard_that_fails_after_publishing(tmp_path, monkeypatch, caplog, real_unlink): + """Removal can fail after a successful replace; the publish must still stand.""" + from deerflow.tools.builtins import setup_agent_tool as module + + target = tmp_path / "SOUL.md" + target.write_text("original", encoding="utf-8") + caplog.clear() + _fail_unlink_on(monkeypatch, lambda path: path.suffix == ".tmp", real_unlink) + + with caplog.at_level("WARNING", logger="deerflow.utils.atomic_write"): + module._publish_text(target, "replacement") + + assert target.read_text(encoding="utf-8") == "replacement" + assert "Failed to remove temporary output file" in caplog.text