teknium1
faa188bf52
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.
313 lines
12 KiB
Python
313 lines
12 KiB
Python
"""Tests for edge cases in tools/file_operations.py.
|
|
|
|
Covers:
|
|
- ``_is_likely_binary()`` content-analysis branch (dead-code removal regression guard)
|
|
- ``_check_lint()`` robustness against file paths containing curly braces
|
|
"""
|
|
|
|
import pytest
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
from tools.file_operations import ShellFileOperations, _parse_search_context_line
|
|
|
|
|
|
# =========================================================================
|
|
# _is_likely_binary edge cases
|
|
# =========================================================================
|
|
|
|
|
|
class TestIsLikelyBinary:
|
|
"""Verify content-analysis logic after dead-code removal."""
|
|
|
|
@pytest.fixture()
|
|
def ops(self):
|
|
return ShellFileOperations.__new__(ShellFileOperations)
|
|
|
|
def test_binary_extension_returns_true(self, ops):
|
|
"""Known binary extensions should short-circuit without content analysis."""
|
|
assert ops._is_likely_binary("image.png") is True
|
|
assert ops._is_likely_binary("archive.tar.gz", content_sample="hello") is True
|
|
|
|
def test_text_content_returns_false(self, ops):
|
|
"""Normal printable text should not be classified as binary."""
|
|
sample = "Hello, world!\nThis is a normal text file.\n"
|
|
assert ops._is_likely_binary("unknown.xyz", content_sample=sample) is False
|
|
|
|
|
|
def test_just_above_threshold(self, ops):
|
|
"""301/1000 = 30.1% non-printable → should be binary."""
|
|
sample = "\x00" * 301 + "a" * 699
|
|
assert ops._is_likely_binary("data.xyz", content_sample=sample) is True
|
|
|
|
def test_tabs_and_newlines_excluded(self, ops):
|
|
"""Tabs, carriage returns, and newlines should not count as non-printable."""
|
|
sample = "\t" * 400 + "\n" * 300 + "\r" * 200 + "a" * 100
|
|
assert ops._is_likely_binary("file.txt", content_sample=sample) is False
|
|
|
|
def test_content_sample_longer_than_1000(self, ops):
|
|
"""Only the first 1000 characters should be analysed."""
|
|
# First 1000 chars: 200 NUL + 800 printable = 20% → not binary
|
|
# Remaining 1000 chars: all NUL → ignored by [:1000] slice
|
|
sample = "\x00" * 200 + "a" * 800 + "\x00" * 1000
|
|
assert ops._is_likely_binary("file.xyz", content_sample=sample) is False
|
|
|
|
|
|
# =========================================================================
|
|
# _check_lint edge cases
|
|
# =========================================================================
|
|
|
|
|
|
class TestCheckLintBracePaths:
|
|
"""Verify _check_lint handles file paths with curly braces safely.
|
|
|
|
Uses ``.js`` to exercise the shell-linter path since ``.py`` now goes
|
|
through the in-process ast.parse linter (see TestCheckLintInproc).
|
|
"""
|
|
|
|
@pytest.fixture()
|
|
def ops(self):
|
|
obj = ShellFileOperations.__new__(ShellFileOperations)
|
|
obj._command_cache = {}
|
|
return obj
|
|
|
|
def test_normal_path(self, ops):
|
|
"""Normal path without braces should work as before."""
|
|
with patch.object(ops, "_has_command", return_value=True), \
|
|
patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(exit_code=0, stdout="")
|
|
result = ops._check_lint("/tmp/test_file.js")
|
|
|
|
assert result.success is True
|
|
# Verify the command was built correctly
|
|
cmd_arg = mock_exec.call_args[0][0]
|
|
assert "'/tmp/test_file.js'" in cmd_arg
|
|
|
|
def test_path_with_curly_braces(self, ops):
|
|
"""Path containing ``{`` and ``}`` must not raise KeyError/ValueError."""
|
|
with patch.object(ops, "_has_command", return_value=True), \
|
|
patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(exit_code=0, stdout="")
|
|
# This would raise KeyError with .format() but works with .replace()
|
|
result = ops._check_lint("/tmp/{test}_file.js")
|
|
|
|
assert result.success is True
|
|
cmd_arg = mock_exec.call_args[0][0]
|
|
assert "{test}" in cmd_arg
|
|
|
|
def test_path_with_nested_braces(self, ops):
|
|
"""Path with complex brace patterns like ``{{var}}`` should be safe."""
|
|
with patch.object(ops, "_has_command", return_value=True), \
|
|
patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(exit_code=0, stdout="")
|
|
result = ops._check_lint("/tmp/{{var}}.js")
|
|
|
|
assert result.success is True
|
|
|
|
def test_unsupported_extension_skipped(self, ops):
|
|
"""Extensions without a linter should return a skipped result."""
|
|
result = ops._check_lint("/tmp/file.unknown_ext")
|
|
assert result.skipped is True
|
|
|
|
def test_missing_linter_skipped(self, ops):
|
|
"""When the linter binary is not installed, skip gracefully."""
|
|
with patch.object(ops, "_has_command", return_value=False):
|
|
result = ops._check_lint("/tmp/test.js")
|
|
assert result.skipped is True
|
|
|
|
def test_lint_failure_returns_output(self, ops):
|
|
"""When the linter exits non-zero, result should capture output."""
|
|
with patch.object(ops, "_has_command", return_value=True), \
|
|
patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(
|
|
exit_code=1,
|
|
stdout="SyntaxError: invalid syntax",
|
|
)
|
|
result = ops._check_lint("/tmp/bad.js")
|
|
|
|
assert result.success is False
|
|
assert "SyntaxError" in result.output
|
|
|
|
|
|
class TestCheckLintInproc:
|
|
"""Verify in-process linters (.py via ast.parse, .json, .yaml, .toml).
|
|
|
|
These bypass the shell linter table entirely and parse content
|
|
directly in Python — no subprocess, no toolchain dependency.
|
|
"""
|
|
|
|
@pytest.fixture()
|
|
def ops(self):
|
|
obj = ShellFileOperations.__new__(ShellFileOperations)
|
|
obj._command_cache = {}
|
|
return obj
|
|
|
|
def test_python_inproc_clean(self, ops):
|
|
"""Valid Python content passes in-process ast.parse."""
|
|
result = ops._check_lint("/tmp/ok.py", content="x = 1\n")
|
|
assert result.success is True
|
|
assert not result.skipped
|
|
assert result.output == ""
|
|
|
|
|
|
def test_json_inproc_clean(self, ops):
|
|
result = ops._check_lint("/tmp/a.json", content='{"a": 1}')
|
|
assert result.success is True
|
|
|
|
|
|
def test_toml_inproc_error(self, ops):
|
|
result = ops._check_lint("/tmp/b.toml", content='[section\nk = "v"')
|
|
assert result.success is False
|
|
assert "TOMLDecodeError" in result.output
|
|
|
|
|
|
class TestCheckLintDelta:
|
|
"""Verify _check_lint_delta() filters pre-existing errors from post-edit output."""
|
|
|
|
@pytest.fixture()
|
|
def ops(self):
|
|
obj = ShellFileOperations.__new__(ShellFileOperations)
|
|
obj._command_cache = {}
|
|
return obj
|
|
|
|
def test_clean_post_no_pre_lint(self, ops):
|
|
"""Hot path: post-write is clean, pre-lint should be skipped entirely."""
|
|
with patch.object(ops, "_check_lint", wraps=ops._check_lint) as wrapped:
|
|
r = ops._check_lint_delta("/tmp/a.py", pre_content="x = 0\n", post_content="x = 1\n")
|
|
# Post-lint called exactly once (clean), pre-lint never called.
|
|
assert wrapped.call_count == 1
|
|
assert r.success is True
|
|
|
|
|
|
def test_pre_existing_remains_flagged_but_not_new(self, ops):
|
|
"""Single-error parsers (ast) may miss that post is OK — be cautious."""
|
|
# Pre has line-1 error, post keeps it (and doesn't add anything new)
|
|
pre = 'def a(:\n pass\n'
|
|
post = 'def a(:\n pass\n\nprint(42)\n' # still line 1 broken
|
|
r = ops._check_lint_delta("/tmp/d.py", pre_content=pre, post_content=post)
|
|
# File is still broken — don't lie and claim success — but flag it as pre-existing
|
|
assert r.success is False
|
|
assert "pre-existing" in (r.message or "").lower()
|
|
|
|
|
|
# =========================================================================
|
|
# Pagination bounds
|
|
# =========================================================================
|
|
|
|
|
|
class TestPaginationBounds:
|
|
"""Invalid pagination inputs should not leak into shell commands."""
|
|
|
|
def test_read_file_clamps_offset_and_limit_before_building_sed_range(self):
|
|
env = MagicMock()
|
|
env.cwd = "/tmp"
|
|
ops = ShellFileOperations(env)
|
|
commands = []
|
|
|
|
def fake_exec(command, *args, **kwargs):
|
|
commands.append(command)
|
|
if command.startswith("if [ -f ") or command.startswith("wc -c"):
|
|
return MagicMock(exit_code=0, stdout="12")
|
|
if command.startswith("head -c"):
|
|
return MagicMock(exit_code=0, stdout="line1\nline2\n")
|
|
if command.startswith("sed -n"):
|
|
return MagicMock(exit_code=0, stdout="line1\n")
|
|
if command.startswith("wc -l"):
|
|
return MagicMock(exit_code=0, stdout="2")
|
|
return MagicMock(exit_code=0, stdout="")
|
|
|
|
with patch.object(ops, "_exec", side_effect=fake_exec):
|
|
result = ops.read_file("notes.txt", offset=0, limit=0)
|
|
|
|
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' | cut -b1-8001"]
|
|
|
|
def test_search_clamps_offset_and_limit_before_building_head_pipeline(self):
|
|
env = MagicMock()
|
|
env.cwd = "/tmp"
|
|
ops = ShellFileOperations(env)
|
|
commands = []
|
|
|
|
def fake_exec(command, *args, **kwargs):
|
|
commands.append(command)
|
|
if command.startswith("test -e"):
|
|
return MagicMock(exit_code=0, stdout="exists")
|
|
if command.startswith("rg --files"):
|
|
return MagicMock(exit_code=0, stdout="a.py\n")
|
|
return MagicMock(exit_code=0, stdout="")
|
|
|
|
with patch.object(ops, "_has_command", side_effect=lambda cmd: cmd == "rg"), \
|
|
patch.object(ops, "_exec", side_effect=fake_exec):
|
|
result = ops.search("*.py", target="files", path=".", offset=-4, limit=-2)
|
|
|
|
assert result.files == ["a.py"]
|
|
rg_commands = [cmd for cmd in commands if cmd.startswith("rg --files")]
|
|
assert rg_commands
|
|
assert "| head -n 1" in rg_commands[0]
|
|
|
|
|
|
# =========================================================================
|
|
# Search context parsing
|
|
# =========================================================================
|
|
|
|
|
|
class TestSearchContextParsing:
|
|
def test_search_with_grep_uses_extended_regex(self):
|
|
env = MagicMock()
|
|
env.cwd = "/tmp"
|
|
ops = ShellFileOperations(env)
|
|
|
|
with patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(
|
|
exit_code=0,
|
|
stdout="./first.txt:1:foo\n./second.txt:1:bar\n",
|
|
)
|
|
result = ops._search_with_grep(
|
|
"foo|bar",
|
|
path=".",
|
|
file_glob=None,
|
|
limit=10,
|
|
offset=0,
|
|
output_mode="content",
|
|
context=0,
|
|
)
|
|
|
|
cmd_arg = mock_exec.call_args[0][0]
|
|
assert cmd_arg.startswith("set -o pipefail; grep -rnHE ")
|
|
assert result.error is None
|
|
assert result.total_count == 2
|
|
assert [match.content for match in result.matches] == ["foo", "bar"]
|
|
|
|
def test_parse_search_context_line_prefers_rightmost_numeric_separator(self):
|
|
parsed = _parse_search_context_line("dir/file-12-name.py-8-context here")
|
|
|
|
assert parsed == ("dir/file-12-name.py", 8, "context here")
|
|
|
|
|
|
def test_search_with_grep_context_handles_filename_with_dash_digits(self):
|
|
env = MagicMock()
|
|
env.cwd = "/tmp"
|
|
ops = ShellFileOperations(env)
|
|
|
|
with patch.object(ops, "_exec") as mock_exec:
|
|
mock_exec.return_value = MagicMock(
|
|
exit_code=0,
|
|
stdout="dir/file-12-name.py-8-context here\n",
|
|
)
|
|
result = ops._search_with_grep(
|
|
"needle",
|
|
path=".",
|
|
file_glob=None,
|
|
limit=10,
|
|
offset=0,
|
|
output_mode="content",
|
|
context=1,
|
|
)
|
|
|
|
assert result.error is None
|
|
assert result.total_count == 1
|
|
assert result.matches[0].path == "dir/file-12-name.py"
|
|
assert result.matches[0].line_number == 8
|
|
assert result.matches[0].content == "context here"
|