Validate required args in file and todo tools
Cluster 19096 · agent. Data as of
3 open items. Fix merged in #19284. Check it covers your case.
The short answer
- Known?
- Yes. 12 related items grouped here, 2 issues already closed.
- Fix event
- A fix was merged to main in PR #19284. Check it covers your case. No release data is tracked.
- Tracker
- The maintainers' tracker marks this
partially resolved, deciding PRs #19284, #133864. That is separate from the fix event. - Still open
- 1 issue and 2 PRs.
Next step: read #19284, then check whether your report is one of the 1 still open.
Read #19284Pick up a Lab taskThese issues and fixes address missing or empty argument validation across file tools (read_file, write_file) and the todo tool, where absent or empty inputs were silently accepted and produced corrupt or zero-byte output. The changes add upfront rejection of invalid arguments instead of silent substitution or misleading errors.
Open issues1
- Issue#31791Bug: read_file produces confusing error when called with empty/missing path
Open PRs2
Fixes on record
-
#19284 fix(tools): write_file rejects missing 'content'/'path' instead of creating zero-byte files
files:
tests/tools/test_file_tools.py,tools/file_tools.pyShow the diff
-
tests/tools/test_file_tools.py@@ -104,6 +104,44 @@ def test_unexpected_exception_still_logs_error(self, mock_get, caplog): 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") -
tools/file_tools.py@@ -1097,7 +1097,25 @@ def _handle_read_file(args, **kw): def _handle_write_file(args, **kw): tid = kw.get("task_id") or "default" - return write_file_tool(path=args.get("path", ""), content=args.get("content", ""), task_id=tid) + if not args.get("path") or not isinstance(args.get("path"), str): + return tool_error( + "write_file: missing required field 'path'. Re-emit the tool call with " + "both 'path' and 'content' set." + ) + if "content" not in args: + return tool_error( + "write_file: missing required field 'content'. The tool call included a " + "path but no content argument — this is almost always a dropped-arg bug " + "under context pressure. Re-emit the tool call with the full content " + "payload, or use execute_code with hermes_tools.write_file() for very " + "large files." + ) + if not isinstance(args["content"], str): + return tool_error( + f"write_file: 'content' must be a string, got " + f"{type(args['content']).__name__}." + ) + return write_file_tool(path=args["path"], content=args["content"], task_id=tid) def _handle_patch(args, **kw):
-
-
#133864 fix(todo): reject empty content instead of substituting '(no description)'
files:
tests/tools/test_todo_tool.py,tools/todo_tool.py
5 PRs closed without merging
From the close comments: duplicate 3 · superseded by another PR 1 · other 1
-
#19097 fix(tools): guard write_file against missing required args · other →
e527240,#19096 -
#19119 fix(tools): write_file rejects missing 'content'/'path' instead of creating zero-byte files (#19096) · duplicate →
#19097 -
#31878 fix(file-tools): reject empty read_file path · duplicate →
#31801,#31791 - #44498 fix(todo): reject empty content instead of substituting (no description) (#44496) · superseded by another PR
-
#83712 fix(todo): reject empty content instead of silent placeholder · duplicate →
#44498,#44496
Closed issues · 2
How this cluster was grouped
-
Link evidence (
partly structural links): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text. - Members outside that structural core may belong to a different underlying problem.
-
Pairwise link density
0.15: the share of item pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36.1.00means every pair is linked.
Pick up a Lab task
Check whether a fix covers the open issues here, with your own agent. Bring back evidence for maintainers.
Open the Lab