diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index acfd762e..22d38e95 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,4 +36,4 @@ jobs: run: uv run ruff check nanobot --select F401,F841 - name: Run tests - run: uv run pytest tests/ --ignore=tests/channels/test_matrix_channel.py \ No newline at end of file + run: uv run pytest tests/ diff --git a/nanobot/agent/tools/filesystem.py b/nanobot/agent/tools/filesystem.py index cd542d00..1f3afd34 100644 --- a/nanobot/agent/tools/filesystem.py +++ b/nanobot/agent/tools/filesystem.py @@ -2,6 +2,7 @@ import difflib import mimetypes +import os from dataclasses import dataclass from pathlib import Path from typing import Any @@ -178,7 +179,6 @@ class ReadFileTool(_FsTool): # Read dedup: same path + offset + limit + unchanged mtime → stub # Always check for external modifications before dedup - import os entry = file_state._state.get(str(fp.resolve())) try: current_mtime = os.path.getmtime(fp) @@ -218,6 +218,10 @@ class ReadFileTool(_FsTool): return build_image_content_blocks(raw, mime, str(fp), f"(Image file: {path})") return f"Error: Cannot read binary file {path} (MIME: {mime or 'unknown'}). Only UTF-8 text and images are supported." + # Normalize CRLF -> LF before line-splitting. Primarily a Windows + # concern (git checkouts with autocrlf, editors saving CRLF) but + # applied on all platforms so downstream StrReplace/Grep behavior + # is consistent regardless of where the file was written. text_content = text_content.replace("\r\n", "\n") all_lines = text_content.splitlines() diff --git a/tests/tools/test_read_enhancements.py b/tests/tools/test_read_enhancements.py index c16bb437..0be12370 100644 --- a/tests/tools/test_read_enhancements.py +++ b/tests/tools/test_read_enhancements.py @@ -1,5 +1,6 @@ """Tests for ReadFileTool enhancements: description fix, read dedup, PDF support, device blacklist.""" +import os import sys import pytest @@ -181,3 +182,67 @@ class TestReadDeviceBlacklist: result = await tool.execute(path=str(link)) assert "Error" in result assert "blocked" in result.lower() or "device" in result.lower() + + +# --------------------------------------------------------------------------- +# file_state: mtime-unchanged / content-changed fallback +# --------------------------------------------------------------------------- +# On filesystems with coarse mtime resolution (NTFS ~100ms, FAT 2s) a fast +# write-after-read can leave mtime unchanged. The content-hash fallback is +# what protects against stale-read warnings being false-negative on those +# platforms. Lock that behavior down here so nobody reverts it silently. + +class TestFileStateHashFallback: + + def test_check_read_warns_when_content_changed_but_mtime_same(self, tmp_path): + f = tmp_path / "data.txt" + f.write_text("original", encoding="utf-8") + file_state.record_read(f) + original_mtime = os.path.getmtime(f) + + f.write_text("modified", encoding="utf-8") + os.utime(f, (original_mtime, original_mtime)) + assert os.path.getmtime(f) == original_mtime + + warning = file_state.check_read(f) + assert warning is not None + assert "modified" in warning.lower() + + def test_check_read_passes_when_content_and_mtime_unchanged(self, tmp_path): + f = tmp_path / "data.txt" + f.write_text("stable", encoding="utf-8") + file_state.record_read(f) + + assert file_state.check_read(f) is None + + +# --------------------------------------------------------------------------- +# Line-ending normalization +# --------------------------------------------------------------------------- +# ReadFileTool normalizes CRLF -> LF before line-splitting. This primarily +# helps Windows users whose checkouts carry CRLF line endings and whose +# subsequent StrReplace edits would otherwise miss on `\r` boundaries. The +# normalization applies on all platforms; these tests lock that in so the +# behavior is intentional and discoverable, not accidental. + +class TestReadFileLineEndingNormalization: + + @pytest.fixture() + def tool(self, tmp_path): + return ReadFileTool(workspace=tmp_path) + + @pytest.mark.asyncio + async def test_crlf_is_normalized_to_lf(self, tool, tmp_path): + f = tmp_path / "crlf.txt" + f.write_bytes(b"alpha\r\nbeta\r\ngamma\r\n") + result = await tool.execute(path=str(f)) + assert "\r" not in result + assert "alpha" in result and "beta" in result and "gamma" in result + + @pytest.mark.asyncio + async def test_lf_only_is_preserved(self, tool, tmp_path): + f = tmp_path / "lf.txt" + f.write_bytes(b"alpha\nbeta\ngamma\n") + result = await tool.execute(path=str(f)) + assert "\r" not in result + assert "alpha" in result and "beta" in result and "gamma" in result diff --git a/tests/tools/test_tool_validation.py b/tests/tools/test_tool_validation.py index c90f866c..73e3b4f2 100644 --- a/tests/tools/test_tool_validation.py +++ b/tests/tools/test_tool_validation.py @@ -1,3 +1,4 @@ +import shlex import subprocess import sys from typing import Any @@ -547,13 +548,20 @@ async def test_exec_always_returns_exit_code() -> None: async def test_exec_head_tail_truncation(tmp_path) -> None: """Long output should preserve both head and tail.""" tool = ExecTool() - # Generate output that exceeds _MAX_OUTPUT (10_000 chars) - # Use a temp script file to avoid Windows command line quote parsing issues + # Generate output that exceeds _MAX_OUTPUT (10_000 chars). + # Use a temp script file so the output-generating logic lives in a file + # (Windows cmd.exe has finicky rules for quoting `-c` payloads with + # embedded newlines). ExecTool runs via create_subprocess_shell, so we + # must quote *both* the interpreter path and the script path — tmp_path + # on some CI runners and on many local Windows installs contains spaces + # (e.g. C:\Users\John Doe\AppData\...) which would otherwise break the + # shell's argv split. script_file = tmp_path / "gen_output.py" script_file.write_text("print('A' * 6000 + chr(10) + 'B' * 6000)", encoding="utf-8") - # On Windows, cmd.exe handles quotes differently. Use the path directly - # without additional quotes since the temp path shouldn't have spaces. - command = f"{sys.executable} {script_file}" + if sys.platform == "win32": + command = subprocess.list2cmdline([sys.executable, str(script_file)]) + else: + command = f"{shlex.quote(sys.executable)} {shlex.quote(str(script_file))}" result = await tool.execute(command=command) assert "chars truncated" in result # Head portion should start with As