diff --git a/tests/tools/test_file_read_guards.py b/tests/tools/test_file_read_guards.py index 7bba5bb00b..375236446a 100644 --- a/tests/tools/test_file_read_guards.py +++ b/tests/tools/test_file_read_guards.py @@ -20,6 +20,7 @@ from tools.file_tools import ( reset_file_dedup, _is_blocked_device, _invalidate_dedup_for_path, + _READ_DEDUP_STATUS_MESSAGE, _get_max_read_chars, _DEFAULT_MAX_READ_CHARS, _read_tracker, @@ -163,7 +164,7 @@ class TestFileDedup(unittest.TestCase): @patch("tools.file_tools._get_file_ops") def test_second_read_returns_dedup_stub(self, mock_ops): - """Second read of same file+range returns dedup stub.""" + """Second read of same file+range returns non-content dedup status.""" mock_ops.return_value = _make_fake_ops( content="line one\nline two\n", file_size=20, ) @@ -174,7 +175,27 @@ class TestFileDedup(unittest.TestCase): # Second read — should get dedup stub r2 = json.loads(read_file_tool(self._tmpfile, task_id="dup")) self.assertTrue(r2.get("dedup"), "Second read should return dedup stub") - self.assertIn("unchanged", r2.get("content", "")) + self.assertEqual(r2.get("status"), "unchanged") + self.assertIn("unchanged", r2.get("message", "")) + self.assertFalse(r2.get("content_returned")) + self.assertNotIn("content", r2) + + @patch("tools.file_tools._get_file_ops") + def test_write_rejects_internal_read_status_text(self, mock_ops): + """write_file must not persist internal read_file status text.""" + fake = MagicMock() + fake.write_file = MagicMock() + mock_ops.return_value = fake + + result = json.loads(write_file_tool( + self._tmpfile, + _READ_DEDUP_STATUS_MESSAGE, + task_id="guard", + )) + + self.assertIn("error", result) + self.assertIn("internal read_file status text", result["error"]) + fake.write_file.assert_not_called() @patch("tools.file_tools._get_file_ops") def test_modified_file_not_deduped(self, mock_ops): diff --git a/tools/file_tools.py b/tools/file_tools.py index 5c399bb588..91f097322d 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -214,6 +214,11 @@ _read_tracker: dict = {} _READ_HISTORY_CAP = 500 # set; used only by get_read_files_summary _DEDUP_CAP = 1000 # dict; skip-identical-reread guard _READ_TIMESTAMPS_CAP = 1000 # dict; external-edit detection for write/patch +_READ_DEDUP_STATUS_MESSAGE = ( + "File unchanged since last read. The content from " + "the earlier read_file result in this conversation is " + "still current — refer to that instead of re-reading." +) def _cap_read_tracker_data(task_data: dict) -> None: @@ -258,6 +263,13 @@ def _cap_read_tracker_data(task_data: dict) -> None: break +def _is_internal_file_status_text(content: str) -> bool: + """Return True when content is an internal file-tool status, not file bytes.""" + if not isinstance(content, str): + return False + return content.strip() == _READ_DEDUP_STATUS_MESSAGE + + def _get_file_ops(task_id: str = "default") -> ShellFileOperations: """Get or create ShellFileOperations for a terminal environment. @@ -451,13 +463,11 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = current_mtime = os.path.getmtime(resolved_str) if current_mtime == cached_mtime: return json.dumps({ - "content": ( - "File unchanged since last read. The content from " - "the earlier read_file result in this conversation is " - "still current — refer to that instead of re-reading." - ), + "status": "unchanged", + "message": _READ_DEDUP_STATUS_MESSAGE, "path": path, "dedup": True, + "content_returned": False, }, ensure_ascii=False) except OSError: pass # stat failed — fall through to full read @@ -702,6 +712,11 @@ def write_file_tool(path: str, content: str, task_id: str = "default") -> str: sensitive_err = _check_sensitive_path(path, task_id) if sensitive_err: return tool_error(sensitive_err) + if _is_internal_file_status_text(content): + return tool_error( + "Refusing to write internal read_file status text as file content. " + "Re-read the file or reconstruct the intended file contents before writing." + ) try: # Resolve once for the registry lock + stale check. Failures here # fall back to the legacy path — write proceeds, per-task staleness