From faa188bf52dc59e4cfcb8e6a0701456fb623b6d0 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:45:56 -0700 Subject: [PATCH] feat(file-ops): clamp oversized lines in the shell pipeline before transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ShellFileOperations.read_file previously ran sed -n '{off},{end}p' bare, so a file with one pathological line (e.g. a 50MB+ minified bundle on a single line) shipped the entire line across the exec transport before Python's per-line clamp (_add_line_numbers, MAX_LINE_LENGTH=2000) could trim it. read_file now pipes through 'cut -b1-{4*max_line_length+1}' so the shell bounds every line to 8001 bytes before the bytes ever reach Python. UTF-8 finding: GNU 'cut -c' is byte-based despite its name (verified: cutting a line of 2-byte 'é' at -c8004 splits a codepoint, leaving a bare 0xC3 lead byte). The transport decodes with errors='replace', so a split codepoint becomes U+FFFD rather than raising — but a clamp of max_line_length+1 BYTES would deliver under max_line_length CHARS for multibyte text, so the Python clamp would never fire and truncation would be silent. Using 4*max_line_length+1 bytes (UTF-8 max 4 bytes/codepoint) guarantees any line longer than max_line_length chars still decodes to more than max_line_length chars, so len(line) > max_line_length always triggers the existing '... [truncated]' suffix, and any boundary U+FFFD lands past char max_line_length where the clamp removes it — verified empirically with fixtures ('é'*4001 splits at the byte boundary yet the result contains no U+FFFD and ends with the truncated suffix). 'cut -b' is used explicitly to document the byte semantics. cut (unlike sed -n p) always newline-terminates its output, which would grow a phantom empty final line on files without a trailing newline; the final-page path now probes the last byte (tail -c 1 | wc -l) and strips the artifact. read_file_raw is untouched: it is documented as no-per-line-truncation. Benchmark (50MB single-line fixture, /usr/bin/time -v, median of 3): before: 191.1 MB peak RSS, 1260 ms wall after: 97.8 MB peak RSS, 490 ms wall Correctness identical in both arms: monster line returns the clamped 2000-char form + '... [truncated]', offset=2 returns the trailing normal lines intact. Tests: 153 passed, 0 failed, 4 skipped across the file-ops suites plus a new tests/tools/test_read_shell_line_clamp.py pinning the monster-line clamp, offset-past-monster reads, no-trailing-newline preservation, both UTF-8 boundary cases, and read_file_raw's exemption. Two existing mocks asserting the exact sed command string were updated for the pipeline. --- tests/tools/test_file_operations.py | 2 +- .../tools/test_file_operations_edge_cases.py | 2 +- tests/tools/test_read_shell_line_clamp.py | 126 ++++++++++++++++++ tools/file_operations.py | 43 +++++- 4 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 tests/tools/test_read_shell_line_clamp.py diff --git a/tests/tools/test_file_operations.py b/tests/tools/test_file_operations.py index 59746b7901..4ecdfeb268 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -331,7 +331,7 @@ class TestShellFileOpsHelpers: "else exit 1; fi" ) assert commands[1] == "head -c 1000 '/c/Users/alice/notes.txt' 2>/dev/null | base64" - assert commands[2] == "sed -n '1,2000p' '/c/Users/alice/notes.txt'" + assert commands[2] == "sed -n '1,2000p' '/c/Users/alice/notes.txt' | cut -b1-8001" assert commands[3] == "wc -l < '/c/Users/alice/notes.txt'" def test_is_likely_binary_by_extension(self, file_ops): diff --git a/tests/tools/test_file_operations_edge_cases.py b/tests/tools/test_file_operations_edge_cases.py index ee73e9df0d..0865801911 100644 --- a/tests/tools/test_file_operations_edge_cases.py +++ b/tests/tools/test_file_operations_edge_cases.py @@ -221,7 +221,7 @@ class TestPaginationBounds: assert result.error is None assert "1|line1" in result.content sed_commands = [cmd for cmd in commands if cmd.startswith("sed -n")] - assert sed_commands == ["sed -n '1,1p' 'notes.txt'"] + assert sed_commands == ["sed -n '1,1p' 'notes.txt' | cut -b1-8001"] def test_search_clamps_offset_and_limit_before_building_head_pipeline(self): env = MagicMock() diff --git a/tests/tools/test_read_shell_line_clamp.py b/tests/tools/test_read_shell_line_clamp.py new file mode 100644 index 0000000000..1d10b3b4d3 --- /dev/null +++ b/tests/tools/test_read_shell_line_clamp.py @@ -0,0 +1,126 @@ +"""Shell-pipeline per-line clamp in ShellFileOperations.read_file. + +A file with one pathological line (e.g. a 400MB minified bundle on a single +line) used to cross the exec transport in full: ``sed -n`` emitted the whole +line and Python only clamped it afterwards. read_file now pipes through +``cut -b1-{4*max_line_length+1}`` so the shell bounds each line before the +bytes reach Python. + +Byte-vs-char note: GNU ``cut -c`` is byte-based, so the clamp can split a +multibyte UTF-8 codepoint at the boundary. The transport decodes with +errors="replace", turning a split into U+FFFD; the budget of +4*max_line_length+1 bytes guarantees at least max_line_length+1 decoded chars +survive for any over-long line, so the Python clamp always fires, always +appends "... [truncated]", and always removes any boundary U+FFFD (it can +only appear past char max_line_length). +""" + +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..")) + +from tools.environments.local import LocalEnvironment +from tools.file_operations import ShellFileOperations +from tools.tool_output_limits import get_max_line_length + + +@pytest.fixture +def ops(): + return ShellFileOperations(LocalEnvironment()) + + +def test_monster_line_clamped_in_shell(tmp_path, ops): + """A single multi-MB line comes back clamped, with the truncated suffix.""" + max_len = get_max_line_length() + monster = tmp_path / "monster.txt" + with open(monster, "w") as f: + f.write("x" * 5_000_000) + f.write("\n") + f.write("after\n") + + result = ops.read_file(str(monster)) + assert result.error is None + lines = result.content.split("\n") + assert lines[0] == "1|" + "x" * max_len + "... [truncated]" + assert lines[1] == "2|after" + # Sanity: the whole result stays tiny relative to the 5MB line. + assert len(result.content) < max_len + 200 + + +def test_offset_past_monster_returns_normal_lines(tmp_path, ops): + monster = tmp_path / "monster.txt" + with open(monster, "w") as f: + f.write("y" * 1_000_000 + "\n") + for i in range(5): + f.write(f"normal line {i}\n") + + result = ops.read_file(str(monster), offset=2) + assert result.error is None + assert result.content.split("\n")[:5] == [ + f"{i + 2}|normal line {i}" for i in range(5) + ] + + +def test_normal_multiline_read_unchanged(tmp_path, ops): + p = tmp_path / "plain.txt" + p.write_text("alpha\nbeta\ngamma\n") + result = ops.read_file(str(p)) + assert result.error is None + assert result.content.startswith("1|alpha\n2|beta\n3|gamma") + + +def test_no_trailing_newline_preserved(tmp_path, ops): + """cut newline-terminates its output; read_file must strip the artifact.""" + p = tmp_path / "nonl.txt" + p.write_text("a\nb") + result = ops.read_file(str(p)) + assert result.content == "1|a\n2|b" + + +def test_utf8_multibyte_boundary_no_mojibake(tmp_path, ops): + """Byte clamp may split a codepoint; no U+FFFD may reach the result.""" + max_len = get_max_line_length() + p = tmp_path / "mb.txt" + with open(p, "w", encoding="utf-8") as f: + # 2-byte chars sized so the byte clamp (4*max_len+1) lands mid-char. + f.write("é" * (2 * max_len + 1) + "\n") + f.write("second\n") + + result = ops.read_file(str(p)) + assert result.error is None + lines = result.content.split("\n") + assert lines[0] == "1|" + "é" * max_len + "... [truncated]" + assert "\ufffd" not in result.content + assert lines[1] == "2|second" + + +def test_multibyte_line_over_limit_still_marked_truncated(tmp_path, ops): + """A multibyte line just over the char limit must still get the suffix. + + This is the case a max_line_length+1 BYTE clamp would silently break: + 2001 chars of 'é' are 4002 bytes, so a 2001-byte clamp would deliver + ~1000 chars and the Python clamp would never fire. + """ + max_len = get_max_line_length() + p = tmp_path / "just_over.txt" + with open(p, "w", encoding="utf-8") as f: + f.write("é" * (max_len + 1) + "\n") + + result = ops.read_file(str(p)) + assert result.error is None + first = result.content.split("\n")[0] + assert first == "1|" + "é" * max_len + "... [truncated]" + + +def test_read_file_raw_not_clamped(tmp_path, ops): + """read_file_raw is documented as no-per-line-truncation — verify.""" + max_len = get_max_line_length() + p = tmp_path / "long_raw.txt" + long_line = "z" * (10 * max_len) + p.write_text(long_line + "\n") + result = ops.read_file_raw(str(p)) + assert result.error is None + assert result.content == long_line + "\n" diff --git a/tools/file_operations.py b/tools/file_operations.py index 2b6c2a8560..d3347a4f90 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -1342,9 +1342,37 @@ class ShellFileOperations(FileOperations): error="Binary file - cannot display as text. Use appropriate tools to handle this file type." ) - # Read with pagination using sed + # Read with pagination using sed, clamping each line to a byte + # budget IN THE SHELL so a pathological single-line file (e.g. one + # 400MB minified line) never crosses the exec transport. The Python + # clamp in _add_line_numbers still runs afterwards; the shell clamp + # only bounds what reaches it. + # + # Why 4*max_line_length + 1 bytes (not max_line_length + 1): + # ``cut -c`` on GNU coreutils is byte-based despite its name, and a + # byte clamp can split a multibyte UTF-8 codepoint at the boundary. + # The transport decodes with errors="replace", so a split codepoint + # becomes U+FFFD rather than an exception — but a clamp of + # max_line_length+1 BYTES yields far fewer CHARS than + # max_line_length for multibyte text, so the Python clamp would + # never fire and truncation would be silent (no "... [truncated]" + # suffix). UTF-8 codepoints are at most 4 bytes, so any line whose + # first max_line_length chars survive occupies at most + # 4*max_line_length bytes; keeping one byte more guarantees that + # every line longer than max_line_length chars still decodes to + # more than max_line_length chars, which triggers the existing + # Python-side clamp (len(line) > max_line_length) and its + # "... [truncated]" suffix. Any U+FFFD from a boundary split lands + # beyond char max_line_length and is always removed by that clamp, + # so mojibake is never visible. ``cut -b`` is used explicitly to + # document the byte semantics. + from tools.tool_output_limits import get_max_line_length + line_clamp_bytes = 4 * get_max_line_length() + 1 end_line = offset + limit - 1 - read_cmd = f"sed -n '{offset},{end_line}p' {self._escape_shell_arg(path)}" + read_cmd = ( + f"sed -n '{offset},{end_line}p' {self._escape_shell_arg(path)}" + f" | cut -b1-{line_clamp_bytes}" + ) read_result = self._exec(read_cmd) if read_result.exit_code != 0: @@ -1371,6 +1399,17 @@ class ShellFileOperations(FileOperations): if truncated: hint = f"Use offset={end_line + 1} to continue reading (showing {offset}-{end_line} of {total_lines} lines)" + # ``cut`` (unlike sed -n p) always newline-terminates its output, + # so a file whose final line has no trailing newline would grow a + # phantom empty last line. Only possible when this page reaches the + # file's final line; probe the last byte and strip the artifact. + if not truncated and read_output.endswith('\n'): + tail_cmd = f"tail -c 1 {self._escape_shell_arg(path)} | wc -l" + tail_result = self._exec(tail_cmd) + tail_output = _strip_terminal_fence_leaks(tail_result.stdout) + if tail_result.exit_code == 0 and tail_output.strip() == "0": + read_output = read_output[:-1] + # Ambiguous-silence guards: an empty content string is # indistinguishable, from inside the model, from a broken tool — # it re-reads, widens the window, tries another path. Name the