From 30a58462192386cb57924d670c23412855776c96 Mon Sep 17 00:00:00 2001 From: Maz Benoscar Date: Sun, 10 May 2026 11:09:03 -0400 Subject: [PATCH] fix(tools): make write_file append discoverable in model-facing schema (#2843) * fix: make tool argument behavior discoverable The write_file tool already supported append=false by default with append=true for end-of-file writes, but the parsed docstring did not describe append in the model-facing schema. This records the overwrite default and append path in the tool description, adds resilient schema regression coverage, and keeps backend sandbox docs aligned. The regression now also checks that every public parameter in the existing tool schema test matrix has a description. Enabling docstring parsing on setup_agent and update_agent fills the two existing gaps with their existing Args docs instead of duplicating descriptions elsewhere. Constraint: Issue #2831 asks for a small docstring/schema discoverability fix without changing runtime file-writing behavior Rejected: Changing write_file defaults | would alter existing overwrite semantics and broaden the fix beyond schema discoverability Rejected: Exact phrase assertions | too brittle for future docstring rewording while testing the same behavior Confidence: high Scope-risk: narrow Directive: Keep model-facing tool parameters documented through parsed docstrings or equivalent schema descriptions Tested: cd backend && uv run pytest tests/test_setup_agent_tool.py tests/test_update_agent_tool.py tests/test_tool_args_schema_no_pydantic_warning.py tests/test_sandbox_tools_security.py::test_str_replace_and_append_on_same_path_should_preserve_both_updates -q Tested: cd backend && uv run ruff check packages/harness/deerflow/sandbox/tools.py packages/harness/deerflow/tools/builtins/setup_agent_tool.py packages/harness/deerflow/tools/builtins/update_agent_tool.py tests/test_tool_args_schema_no_pydantic_warning.py Not-tested: Full backend test suite Co-authored-by: OmX * Fix the lint error --------- Co-authored-by: OmX Co-authored-by: Willem Jiang --- backend/CLAUDE.md | 2 +- backend/README.md | 2 +- .../packages/harness/deerflow/sandbox/tools.py | 3 ++- .../deerflow/tools/builtins/setup_agent_tool.py | 2 +- .../tools/builtins/update_agent_tool.py | 2 +- ...test_tool_args_schema_no_pydantic_warning.py | 17 +++++++++++++++++ 6 files changed, 23 insertions(+), 5 deletions(-) diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index d03aeefd8..99922a61e 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -243,7 +243,7 @@ Proxied through nginx: `/api/langgraph/*` → LangGraph, all other `/api/*` → - `bash` - Execute commands with path translation and error handling - `ls` - Directory listing (tree format, max 2 levels) - `read_file` - Read file contents with optional line range -- `write_file` - Write/append to files, creates directories +- `write_file` - Write/append to files, creates directories; overwrites by default and exposes the `append` argument in the model-facing schema for end-of-file writes - `str_replace` - Substring replacement (single or all occurrences); same-path serialization is scoped to `(sandbox.id, path)` so isolated sandboxes do not contend on identical virtual paths inside one process ### Subagent System (`packages/harness/deerflow/subagents/`) diff --git a/backend/README.md b/backend/README.md index 0e2d966ee..6295eba22 100644 --- a/backend/README.md +++ b/backend/README.md @@ -79,7 +79,7 @@ Per-thread isolated execution with virtual path translation: - **Skills path**: `/mnt/skills` → `deer-flow/skills/` directory - **Skills loading**: Recursively discovers nested `SKILL.md` files under `skills/{public,custom}` and preserves nested container paths - **File-write safety**: `str_replace` serializes read-modify-write per `(sandbox.id, path)` so isolated sandboxes keep concurrency even when virtual paths match -- **Tools**: `bash`, `ls`, `read_file`, `write_file`, `str_replace` (`bash` is disabled by default when using `LocalSandboxProvider`; use `AioSandboxProvider` for isolated shell access) +- **Tools**: `bash`, `ls`, `read_file`, `write_file`, `str_replace` (`write_file` overwrites by default and exposes `append` for end-of-file writes; `bash` is disabled by default when using `LocalSandboxProvider`; use `AioSandboxProvider` for isolated shell access) ### Subagent System diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index a20004a8a..7c746b1aa 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -1499,12 +1499,13 @@ def write_file_tool( content: str, append: bool = False, ) -> str: - """Write text content to a file. + """Write text content to a file. By default this overwrites the target file; set append to true to add content to the end without replacing existing content. Args: description: Explain why you are writing to this file in short words. ALWAYS PROVIDE THIS PARAMETER FIRST. path: The **absolute** path to the file to write to. ALWAYS PROVIDE THIS PARAMETER SECOND. content: The content to write to the file. ALWAYS PROVIDE THIS PARAMETER THIRD. + append: Whether to append content to the end of the file instead of overwriting it. Defaults to false. """ try: sandbox = ensure_sandbox_initialized(runtime) 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 97929ad56..2f796b005 100644 --- a/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/setup_agent_tool.py @@ -20,7 +20,7 @@ def _get_runtime_user_id(runtime: Runtime) -> str: return get_effective_user_id() -@tool +@tool(parse_docstring=True) def setup_agent( soul: str, description: str, 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 90d951859..b2dc8ca72 100644 --- a/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py @@ -67,7 +67,7 @@ def _cleanup_temps(temps: list[Path]) -> None: logger.debug("Failed to clean up temp file %s", tmp, exc_info=True) -@tool +@tool(parse_docstring=True) def update_agent( runtime: Runtime, soul: str | None = None, diff --git a/backend/tests/test_tool_args_schema_no_pydantic_warning.py b/backend/tests/test_tool_args_schema_no_pydantic_warning.py index 037771b3e..6da56347f 100644 --- a/backend/tests/test_tool_args_schema_no_pydantic_warning.py +++ b/backend/tests/test_tool_args_schema_no_pydantic_warning.py @@ -89,3 +89,20 @@ def test_tool_args_schema_does_not_emit_pydantic_context_warning(tool_obj, extra pydantic_warnings = [w for w in caught if "PydanticSerializationUnexpectedValue" in str(w.message)] assert not pydantic_warnings, f"{tool_obj.name} args_schema.model_dump() emitted Pydantic context serialization warnings: {[str(w.message) for w in pydantic_warnings]}" + + +def test_write_file_append_is_discoverable_in_tool_schema() -> None: + """``append`` must be visible and described in the model-facing tool schema.""" + assert "append" in write_file_tool.description + + append_field = write_file_tool.tool_call_schema.model_fields["append"] + assert append_field.default is False + assert append_field.description + assert "append" in append_field.description + + +@pytest.mark.parametrize("tool_obj", [case[0] for case in _TOOL_CASES], ids=[case[0].name for case in _TOOL_CASES]) +def test_model_facing_tool_parameters_have_descriptions(tool_obj) -> None: + """Every model-facing tool parameter should explain when and how to use it.""" + missing_descriptions = [field_name for field_name, field in tool_obj.tool_call_schema.model_fields.items() if not field.description] + assert missing_descriptions == [], f"{tool_obj.name} has model-facing parameters without descriptions: {missing_descriptions}. Add an Args: section to the tool's docstring and ensure @tool(parse_docstring=True) is set."