diff --git a/backend/packages/harness/deerflow/tools/AGENTS.md b/backend/packages/harness/deerflow/tools/AGENTS.md index feff4bdb12..a898a0e550 100644 --- a/backend/packages/harness/deerflow/tools/AGENTS.md +++ b/backend/packages/harness/deerflow/tools/AGENTS.md @@ -44,7 +44,7 @@ Only standalone tool discovery without a model falls back to the base profile. - `present_files` - Make output files visible to user (only `/mnt/user-data/outputs`); virtual paths use `resolve_runtime_user_id(runtime)` so validation resolves the same user-scoped outputs directory established by `ThreadDataMiddleware` - `ask_clarification` - Request clarification (intercepted by ClarificationMiddleware, which preserves text fallback and adds `artifact.human_input` for Web UI Human Input Cards). Beyond free text and single choice, the request-side v2 protocol supports `fields` (structured form card collecting several values at once; field types: text/textarea/number/select/multi_select/checkbox/date, validated and normalized server-side in the middleware — invalid entries are dropped, unknown types degrade to `text`; a standalone multi-select question is a one-field form). Replies stay on the v1 response protocol (`text`/`option`): the form card submits a readable text summary - `view_image` - Read image bytes for vision-capable models; both sync and async entry points require `sandbox:execute` before any host or sandbox read. With blob storage enabled, validated bytes are persisted as `kind="viewed-image"` and the checkpoint carries a JSON-safe ref beside the compatibility host path; store initialization/write failures return a generic tool error. Blob-backed model reads validate the ref against the separate image metadata before use and fall through to validated compatibility paths on any store failure. Live sandbox bytes remain the fallback for the same sandbox generation, replacement-sandbox recovery uses only SHA-256-verified synchronized host bytes, and async tool invocation drains blocking reads before cancellation may release the sandbox lease - - `setup_agent` - Bootstrap-only: persist a custom agent's `SOUL.md` and `config.yaml`. Re-bootstrapping preserves the owner's existing `display_name`. Bound only when `is_bootstrap=True`. + - `setup_agent` - Bootstrap-only: persist a custom agent's `SOUL.md` and `config.yaml`. Re-bootstrapping preserves the owner's existing `display_name`. Bound only when `is_bootstrap=True`. The default agent's global `SOUL.md` is published through a staged temporary plus `os.replace`, like the agent store's per-agent write, so an interrupted setup keeps the previous persona file instead of leaving it truncated. - `update_agent` - Custom-agent-only: persist self-updates to the current agent's `SOUL.md` / `config.yaml` from inside a normal chat (partial update + atomic write). Bound when `agent_name` is set and `is_bootstrap=False`. 4. **Subagent tool** (if enabled): - `task` - Delegate to subagent (`prompt`, `subagent_type`, optional `acceptance_criteria`, and an optional model-visible `description` used only as a short progress label). Execution never depends on `description`; lifecycle display falls back to `prompt` when a provider omits it. Subagent reports are self-reports: the docstring directs the lead to expect `[rN]` receipt citations and verifiable handles while `verification.receipts_enabled` (and explicitly qualifies that disabled receipts mean no citations and no citation verdict), to read the delegation ledger's citation cross-check as execution evidence only, and to attach `acceptance_criteria` for objectively checkable outcomes (canonical forms `file: exists|non-empty`, `file_written:`, `tests_passed:`); criteria are handed to the executor and appended to the subagent's task message as untrusted data (see `subagents/report_contract.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 151081807c..925edb8dd9 100644 --- a/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py @@ -1,4 +1,8 @@ import logging +import os +import stat +import tempfile +from pathlib import Path from langchain_core.messages import ToolMessage from langchain_core.tools import tool @@ -14,6 +18,39 @@ from deerflow.tools.types import Runtime logger = logging.getLogger(__name__) +def _publish_text(target: Path, text: str) -> None: + """Publish *text* at *target* so no reader ever observes a half-written file. + + A truncating write destroys the previous file before the new content + lands, so an interrupted write leaves a truncated SOUL.md — the persona + file the prompt builder reads on every run — instead of the old one. The + custom-agent store already publishes the same file through a staged + temporary plus ``os.replace``; the default agent's copy must not be the + one that can tear. + + The temporary carries the target's mode (0644 for a first publish) before + the replace, so a re-publish keeps the mode an operator set on the file, + and one try/finally spans staging and publishing so a failure in either + phase cannot orphan the temporary in ``base_dir``. + """ + try: + mode = stat.S_IMODE(os.stat(target).st_mode) + except OSError: + mode = 0o644 + temporary: Path | None = None + try: + with tempfile.NamedTemporaryFile("w", encoding="utf-8", dir=target.parent, suffix=".tmp", delete=False) as handle: + temporary = Path(handle.name) + handle.write(text) + handle.flush() + os.chmod(temporary, mode) + os.fsync(handle.fileno()) + temporary.replace(target) + finally: + if temporary is not None: + temporary.unlink(missing_ok=True) + + @tool(parse_docstring=True) def setup_agent( soul: str, @@ -80,7 +117,7 @@ def setup_agent( # regardless of the agent-storage backend. paths = get_paths() paths.base_dir.mkdir(parents=True, exist_ok=True) - (paths.base_dir / SOUL_FILENAME).write_text(soul, encoding="utf-8") + _publish_text(paths.base_dir / SOUL_FILENAME, soul) logger.info(f"[agent_creator] Created agent '{agent_name}'") return Command( diff --git a/backend/tests/test_setup_agent_tool.py b/backend/tests/test_setup_agent_tool.py index dc2f03f675..180d937f86 100644 --- a/backend/tests/test_setup_agent_tool.py +++ b/backend/tests/test_setup_agent_tool.py @@ -2,6 +2,9 @@ from __future__ import annotations +import os +import stat +import tempfile from pathlib import Path from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -203,3 +206,135 @@ class TestSetupAgentEmptySoulGuard: _call_setup_agent(tmp_path, soul=" ", description="desc") assert existing_soul.read_text(encoding="utf-8") == "original per-agent soul" + + +class TestSetupAgentGlobalSoulAtomicPublish: + """The default agent's SOUL.md must never be observable half written.""" + + def _write_global_soul(self, tmp_path: Path, soul: str): + with patch("deerflow.tools.builtins.setup_agent_tool.get_paths", return_value=_make_paths_mock(tmp_path)), patch("deerflow.config.agents_config.get_paths", return_value=_make_paths_mock(tmp_path)): + return setup_agent.func( + soul=soul, + description="desc", + runtime=_DummyRuntime(context={"agent_name": None}, tool_call_id="tool-global"), + ) + + def test_publishes_full_soul_without_leaving_temps(self, tmp_path: Path): + result = self._write_global_soul(tmp_path, "# Persona\n\nBe kind.") + + soul_path = tmp_path / "SOUL.md" + assert soul_path.read_text(encoding="utf-8") == "# Persona\n\nBe kind." + assert list(tmp_path.glob("*.tmp")) == [] + assert "created successfully" in result.update["messages"][0].content + + def test_failed_publish_keeps_the_previous_soul(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + soul_path = tmp_path / "SOUL.md" + soul_path.write_text("original global soul", encoding="utf-8") + + def _boom(*args, **kwargs): + raise OSError("read-only filesystem") + + monkeypatch.setattr(os, "replace", _boom) + result = self._write_global_soul(tmp_path, "replacement soul") + + # A truncating write would have emptied SOUL.md before failing, leaving + # every later run with a persona file missing its instructions; + # publishing through a temporary file cannot. + assert soul_path.read_text(encoding="utf-8") == "original global soul" + assert list(tmp_path.glob("*.tmp")) == [] + assert "Error" in result.update["messages"][0].content + + def test_truncating_write_would_have_destroyed_it(self, tmp_path: Path): + """Control: the previous implementation loses the content in the same failure.""" + soul_path = tmp_path / "SOUL.md" + soul_path.write_text("original global soul", encoding="utf-8") + + try: + with open(soul_path, "w", encoding="utf-8") as handle: + handle.write("replacement soul") + raise OSError("read-only filesystem") + except OSError: + pass + + assert soul_path.read_text(encoding="utf-8") != "original global soul" + + def test_publish_sets_the_temp_mode_before_the_replace(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + soul_path = tmp_path / "SOUL.md" + soul_path.write_text("original global soul", encoding="utf-8") + chmod_calls: list[tuple[Path, int]] = [] + real_chmod = os.chmod + + def spy_chmod(path, mode): + chmod_calls.append((Path(path), mode)) + real_chmod(path, mode) + + monkeypatch.setattr(os, "chmod", spy_chmod) + self._write_global_soul(tmp_path, "replacement soul") + + # The staged temporary — not the published file — carries the target's + # mode into the replace, so the published file never takes mkstemp's + # 0600 and a re-publish keeps a mode an operator set. + assert chmod_calls, "the temporary must be chmod'ed before the replace" + temporary, mode = chmod_calls[0] + assert temporary.name.endswith(".tmp") + assert mode == stat.S_IMODE(os.stat(soul_path).st_mode) + + def test_staging_failure_leaves_no_temporary(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + soul_path = tmp_path / "SOUL.md" + soul_path.write_text("original global soul", encoding="utf-8") + real_named_temporary_file = tempfile.NamedTemporaryFile + + class _FailingStagingHandle: + """Stands in for the staging handle: the write fails mid-flight (ENOSPC).""" + + def __init__(self, *args, **kwargs): + self._handle = real_named_temporary_file(*args, **kwargs) + self.name = self._handle.name + + def write(self, data): + raise OSError("No space left on device") + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + self._handle.close() + return False + + monkeypatch.setattr(tempfile, "NamedTemporaryFile", _FailingStagingHandle) + result = self._write_global_soul(tmp_path, "replacement soul") + + # The staging failure happens before the replace, so the previous file + # survives and the delete=False temporary is cleaned up rather than + # orphaned in base_dir. + assert soul_path.read_text(encoding="utf-8") == "original global soul" + assert list(tmp_path.glob("*.tmp")) == [] + assert "Error" in result.update["messages"][0].content + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits; Windows chmod only carries the read-only flag") +class TestSetupAgentGlobalSoulPublishMode: + """The published SOUL.md keeps a real mode, never mkstemp's 0600.""" + + def _write_global_soul(self, tmp_path: Path, soul: str): + with patch("deerflow.tools.builtins.setup_agent_tool.get_paths", return_value=_make_paths_mock(tmp_path)), patch("deerflow.config.agents_config.get_paths", return_value=_make_paths_mock(tmp_path)): + return setup_agent.func( + soul=soul, + description="desc", + runtime=_DummyRuntime(context={"agent_name": None}, tool_call_id="tool-global"), + ) + + def test_first_publish_is_world_readable(self, tmp_path: Path): + self._write_global_soul(tmp_path, "# Persona") + + assert stat.S_IMODE(os.stat(tmp_path / "SOUL.md").st_mode) == 0o644 + + def test_republish_preserves_the_operators_mode(self, tmp_path: Path): + soul_path = tmp_path / "SOUL.md" + soul_path.write_text("original global soul", encoding="utf-8") + os.chmod(soul_path, 0o640) + + self._write_global_soul(tmp_path, "replacement soul") + + assert soul_path.read_text(encoding="utf-8") == "replacement soul" + assert stat.S_IMODE(os.stat(soul_path).st_mode) == 0o640