fix(agents): harden update_agent null-like args (#3237)

* fix(agents): harden update_agent null-like args

* docs: mention undefined null-like update args

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/packages/harness/deerflow/agents/lead_agent/prompt.py

@@ -678,6 +678,7 @@ SOUL.md or config.yaml — those write into a temporary sandbox/tool workspace a
 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.
 </self_update>

backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py

@@ -17,12 +17,13 @@ from __future__ import annotations
 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 @@ from deerflow.tools.types import Runtime
 
 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)

backend/tests/test_lead_agent_prompt.py

@@ -30,6 +30,7 @@ def test_build_self_update_section_present_for_custom_agent():
     assert "<self_update>" 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):

backend/tests/test_update_agent_tool.py

@@ -15,6 +15,7 @@ from unittest.mock import MagicMock, patch
 
 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 ---