diff --git a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py index 6a82b82695..fd34ab73a3 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py @@ -678,6 +678,7 @@ def _build_self_update_section(agent_name: str | None) -> str: Rules: - Always pass the FULL replacement text for `soul` (no patch semantics). Start from your current SOUL above and apply the user's edits. - Only pass the fields that should change. Omit the others to preserve them. +- Never pass literal strings like `"null"`, `"none"`, or `"undefined"` for unchanged fields. - Pass `skills=[]` to disable all skills, or omit `skills` to keep the existing whitelist. - After `update_agent` returns successfully, tell the user the change is persisted and will take effect on the next turn. diff --git a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py index 18500a2481..ddb96ae35d 100644 --- a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py @@ -17,12 +17,13 @@ import logging import tempfile from pathlib import Path -from typing import Any +from typing import Annotated, Any import yaml from langchain_core.messages import ToolMessage from langchain_core.tools import tool from langgraph.types import Command +from pydantic import BeforeValidator from deerflow.config.agents_config import load_agent_config, validate_agent_name from deerflow.config.app_config import get_app_config @@ -32,6 +33,8 @@ logger = logging.getLogger(__name__) +_NULLISH_STRINGS = frozenset({"null", "none", "undefined"}) + def _stage_temp(path: Path, text: str) -> Path: """Write ``text`` into a sibling temp file and return its path. @@ -67,14 +70,26 @@ def _cleanup_temps(temps: list[Path]) -> None: logger.debug("Failed to clean up temp file %s", tmp, exc_info=True) +def _is_nullish_string(value: object) -> bool: + return isinstance(value, str) and value.strip().lower() in _NULLISH_STRINGS + + +def _normalize_nullish_string(value: object) -> object: + return None if _is_nullish_string(value) else value + + +OptionalText = Annotated[str | None, BeforeValidator(_normalize_nullish_string)] +OptionalStringList = Annotated[list[str] | None, BeforeValidator(_normalize_nullish_string)] + + @tool(parse_docstring=True) def update_agent( runtime: Runtime, - soul: str | None = None, - description: str | None = None, - skills: list[str] | None = None, - tool_groups: list[str] | None = None, - model: str | None = None, + soul: OptionalText = None, + description: OptionalText = None, + skills: OptionalStringList = None, + tool_groups: OptionalStringList = None, + model: OptionalText = None, ) -> Command: """Persist updates to the current custom agent's SOUL.md and config.yaml. @@ -86,7 +101,9 @@ def update_agent( semantics, so always start from the current SOUL and apply your edits. Pass ``skills=[]`` to disable all skills for this agent. Omit ``skills`` - entirely to keep the existing whitelist. + entirely to keep the existing whitelist. Do not pass literal strings like + ``"null"`` / ``"none"`` / ``"undefined"`` for unchanged fields; omit those + fields instead. Args: soul: Optional full replacement SOUL.md content. @@ -104,10 +121,10 @@ def update_agent( agent_name_raw: str | None = runtime.context.get("agent_name") if runtime.context else None def _err(message: str) -> Command: - return Command(update={"messages": [ToolMessage(content=f"Error: {message}", tool_call_id=tool_call_id)]}) + return Command(update={"messages": [ToolMessage(content=f"Error: {message}", tool_call_id=tool_call_id, status="error")]}) if soul is None and description is None and skills is None and tool_groups is None and model is None: - return _err("No fields provided. Pass at least one of: soul, description, skills, tool_groups, model.") + return _err('No fields provided. Pass at least one of: soul, description, skills, tool_groups, model. Omit unchanged fields instead of passing null-like strings such as "null", "none", or "undefined".') try: agent_name = validate_agent_name(agent_name_raw) diff --git a/backend/tests/test_lead_agent_prompt.py b/backend/tests/test_lead_agent_prompt.py index 28c8518e59..a03fa02b5d 100644 --- a/backend/tests/test_lead_agent_prompt.py +++ b/backend/tests/test_lead_agent_prompt.py @@ -30,6 +30,7 @@ def test_build_self_update_section_present_for_custom_agent(): assert "" in section assert "my-agent" in section assert "update_agent" in section + assert '"null"' in section def test_build_custom_mounts_section_returns_empty_when_no_mounts(monkeypatch): diff --git a/backend/tests/test_update_agent_tool.py b/backend/tests/test_update_agent_tool.py index 3cef11252d..9eaf1c1a48 100644 --- a/backend/tests/test_update_agent_tool.py +++ b/backend/tests/test_update_agent_tool.py @@ -15,6 +15,7 @@ import pytest import yaml +from langchain.tools import ToolRuntime from deerflow.config.agents_config import AgentConfig from deerflow.tools.builtins.update_agent_tool import update_agent @@ -31,6 +32,18 @@ def _runtime(agent_name: str | None = "test-agent", tool_call_id: str = "call_1" return _DummyRuntime(context={"agent_name": agent_name} if agent_name is not None else {}, tool_call_id=tool_call_id) +def _tool_runtime(agent_name: str | None = "test-agent", tool_call_id: str = "call_1") -> ToolRuntime: + return ToolRuntime( + state={"sandbox": {"sandbox_id": "local"}, "thread_data": {}}, + context={"agent_name": agent_name} if agent_name is not None else {}, + config={"configurable": {"thread_id": "thread-1"}}, + stream_writer=lambda _: None, + tools=[], + tool_call_id=tool_call_id, + store=None, + ) + + def _make_paths_mock(tmp_path: Path) -> MagicMock: paths = MagicMock() paths.base_dir = tmp_path @@ -115,6 +128,7 @@ def test_update_agent_requires_at_least_one_field(tmp_path, patched_paths): msg = result.update["messages"][0] assert "No fields provided" in msg.content + assert msg.status == "error" def test_update_agent_rejects_unknown_model(tmp_path, patched_paths, stub_app_config): @@ -141,6 +155,65 @@ def test_update_agent_accepts_known_model(tmp_path, patched_paths, stub_app_conf assert "model" in result.update["messages"][0].content +def test_update_agent_treats_nullish_optional_text_as_omitted(tmp_path, patched_paths): + """Models sometimes pass literal "null" strings while trying to omit fields. + + Treat those as omitted for optional text fields so they do not get persisted + as a model name or SOUL.md content and feed repeated update_agent retries. + """ + agent_dir = _seed_agent(tmp_path, description="old desc", soul="old soul") + + result = update_agent.invoke( + { + "runtime": _tool_runtime(), + "soul": "null", + "description": "none", + "model": "undefined", + } + ) + + msg = result.update["messages"][0] + assert "No fields provided" in msg.content + assert msg.status == "error" + + cfg = yaml.safe_load((agent_dir / "config.yaml").read_text()) + assert cfg["description"] == "old desc" + assert "model" not in cfg + assert (agent_dir / "SOUL.md").read_text() == "old soul" + + +def test_update_agent_rejects_string_list_fields(tmp_path, patched_paths): + """skills/tool_groups must be real arrays; string placeholders are invalid.""" + agent_dir = _seed_agent(tmp_path, skills=["existing"]) + + assert update_agent.args_schema is not None + with pytest.raises(ValueError, match="skills"): + update_agent.args_schema.model_validate({"skills": "alpha,beta"}) + + cfg = yaml.safe_load((agent_dir / "config.yaml").read_text()) + assert cfg["skills"] == ["existing"] + + +def test_update_agent_treats_nullish_string_list_fields_as_omitted(tmp_path, patched_paths): + agent_dir = _seed_agent(tmp_path, skills=["existing"]) + + result = update_agent.invoke( + { + "runtime": _tool_runtime(), + "skills": "null", + "tool_groups": "none", + } + ) + + msg = result.update["messages"][0] + assert "No fields provided" in msg.content + assert msg.status == "error" + + cfg = yaml.safe_load((agent_dir / "config.yaml").read_text()) + assert cfg["skills"] == ["existing"] + assert "tool_groups" not in cfg + + # --- Partial update tests ---