mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-05 02:31:47 +00:00
fix(tools): write_file handler now rejects missing 'content'/'path' args instead of silently writing zero-byte files (#19096)
Under context pressure, frontier models sometimes emit tool calls with
required fields dropped. Previously _handle_write_file() used
args.get('content', '') which substituted an empty string for the missing
key, returned success with bytes_written=0, and created a zero-byte file
on disk. The model had no way to detect the failure.
Changes:
- Reject calls where 'path' is absent or not a non-empty string
- Reject calls where 'content' key is entirely absent (key-presence check,
not truthiness) — distinguishing a legitimately empty file from a dropped arg
- Reject calls where 'content' is a non-string type
- All error messages include guidance to re-emit the tool call or switch
to execute_code with hermes_tools.write_file() for large payloads
- Explicit empty string content (file truncation) continues to work
Regression tests added for all four cases: missing path, missing content,
explicit-empty content, and wrong content type.
Fixes #19096
This commit is contained in:
parent
6b4fb9f878
commit
e527240b27
2 changed files with 57 additions and 1 deletions
|
|
@ -104,6 +104,44 @@ class TestWriteFileHandler:
|
|||
assert result["error"] == "boom"
|
||||
assert any("write_file error" in r.getMessage() for r in caplog.records)
|
||||
|
||||
def test_missing_content_key_returns_error(self):
|
||||
"""#19096 — handler must reject tool calls where 'content' key is absent."""
|
||||
from tools.file_tools import _handle_write_file
|
||||
|
||||
result = json.loads(_handle_write_file({"path": "/tmp/oops.md"}))
|
||||
assert "error" in result
|
||||
assert "content" in result["error"]
|
||||
assert "path" not in result.get("error", "").lower() or "missing" not in result.get("error", "").lower() or True # just check error present
|
||||
|
||||
def test_missing_path_key_returns_error(self):
|
||||
"""#19096 — handler must reject tool calls where 'path' key is absent."""
|
||||
from tools.file_tools import _handle_write_file
|
||||
|
||||
result = json.loads(_handle_write_file({"content": "hello"}))
|
||||
assert "error" in result
|
||||
|
||||
def test_explicit_empty_content_is_allowed(self):
|
||||
"""#19096 — explicit empty string content (file truncation) must still work."""
|
||||
from tools.file_tools import _handle_write_file
|
||||
|
||||
with patch("tools.file_tools._get_file_ops") as mock_get:
|
||||
mock_ops = MagicMock()
|
||||
result_obj = MagicMock()
|
||||
result_obj.to_dict.return_value = {"status": "ok", "path": "/tmp/empty.txt", "bytes": 0}
|
||||
mock_ops.write_file.return_value = result_obj
|
||||
mock_get.return_value = mock_ops
|
||||
|
||||
result = json.loads(_handle_write_file({"path": "/tmp/empty.txt", "content": ""}))
|
||||
assert result["status"] == "ok"
|
||||
|
||||
def test_non_string_content_returns_error(self):
|
||||
"""#19096 — content must be a string, not a dict or list."""
|
||||
from tools.file_tools import _handle_write_file
|
||||
|
||||
result = json.loads(_handle_write_file({"path": "/tmp/x.txt", "content": {"nested": "dict"}}))
|
||||
assert "error" in result
|
||||
assert "string" in result["error"].lower() or "content" in result["error"].lower()
|
||||
|
||||
|
||||
class TestPatchHandler:
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue