From 08fdf61516c0b82d506e87a6379464b9ac27c61b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Sat, 11 Jul 2026 23:20:53 +0800 Subject: [PATCH] fix(sandbox): handle one-sided line ranges in read_file (#4078) * fix(sandbox): honor single-bound line ranges in read_file read_file only sliced content when BOTH start_line and end_line were provided, so read_file(path, start_line=100) or read_file(path, end_line=50) silently returned the whole file. Handle one-sided ranges: default the missing bound (start->1, end->EOF), clamp start to 1, and return clear messages when start_line exceeds the file length or start_line > end_line. * address review: guard one-sided end_line <= 0 in read_file Add symmetric end_line guard and run the inverted-range check regardless of which bounds are explicit, so a lone end_line<=0 returns a clean error instead of a negative-index slice. Per @willem-bd review on #4078. Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../harness/deerflow/sandbox/tools.py | 13 ++- backend/tests/test_read_file_line_range.py | 94 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 backend/tests/test_read_file_line_range.py diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index 3be93ebc5..202474605 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -1994,8 +1994,17 @@ def read_file_tool( content = read_current_file_content(runtime, path) if not content: return "(empty)" - if start_line is not None and end_line is not None: - content = "\n".join(content.splitlines()[start_line - 1 : end_line]) + if start_line is not None or end_line is not None: + lines = content.splitlines() + s = max(start_line, 1) if start_line is not None else 1 + e = end_line if end_line is not None else len(lines) + if e < 1: + return "(end_line must be >= 1)" + if s > len(lines): + return "(start_line exceeds file length)" + if s > e: + return "(start_line > end_line — no lines in range)" + content = "\n".join(lines[s - 1 : e]) try: from deerflow.config.app_config import get_app_config diff --git a/backend/tests/test_read_file_line_range.py b/backend/tests/test_read_file_line_range.py new file mode 100644 index 000000000..9c4da1af5 --- /dev/null +++ b/backend/tests/test_read_file_line_range.py @@ -0,0 +1,94 @@ +"""read_file tool line-range handling for one-sided (single-bound) ranges. + +Previously ``read_file`` only sliced when BOTH ``start_line`` and ``end_line`` +were supplied; a lone ``start_line`` (or lone ``end_line``) was silently ignored +and the whole file was returned. These tests pin the one-sided range contract: +tail-from-start, head-to-end, clamping of ``start_line=0``, and clean error +strings for an inverted range or a start beyond EOF (instead of an empty/garbage +slice). +""" + +from pathlib import Path +from types import SimpleNamespace + +from deerflow.sandbox.local.local_sandbox import LocalSandbox +from deerflow.sandbox.tools import read_file_tool + +_FIVE_LINES = "line1\nline2\nline3\nline4\nline5" + + +def _local_runtime(tmp_path: Path) -> SimpleNamespace: + for sub in ("workspace", "uploads", "outputs"): + (tmp_path / sub).mkdir(parents=True, exist_ok=True) + thread_data = { + "workspace_path": str(tmp_path / "workspace"), + "uploads_path": str(tmp_path / "uploads"), + "outputs_path": str(tmp_path / "outputs"), + } + return SimpleNamespace( + state={"sandbox": {"sandbox_id": "local:t1"}, "thread_data": thread_data}, + context={"thread_id": "t1"}, + ) + + +def _read(tmp_path, monkeypatch, **kwargs) -> str: + runtime = _local_runtime(tmp_path) + (tmp_path / "uploads" / "five.txt").write_text(_FIVE_LINES, encoding="utf-8") + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: LocalSandbox("t1")) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None) + return read_file_tool.func( + runtime=runtime, + description="read a line range", + path="/mnt/user-data/uploads/five.txt", + **kwargs, + ) + + +def test_only_start_line_returns_tail_from_that_line(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=3) + assert result == "line3\nline4\nline5" + + +def test_only_end_line_returns_head_up_to_that_line(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, end_line=2) + assert result == "line1\nline2" + + +def test_start_line_zero_is_clamped_to_first_line(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=0) + assert result == _FIVE_LINES + + +def test_start_line_greater_than_end_line_returns_clean_error(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=4, end_line=2) + assert "start_line > end_line" in result + # No garbage slice content leaked into the error. + assert "line4" not in result + + +def test_start_line_beyond_eof_returns_clean_error(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=99) + assert "start_line exceeds file length" in result + + +def test_both_bounds_still_slice_inclusive_range(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=2, end_line=4) + assert result == "line2\nline3\nline4" + + +def test_only_end_line_zero_returns_clean_error(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, end_line=0) + assert "end_line must be >= 1" in result + # No leaked line content in the error. + assert "line1" not in result + + +def test_only_end_line_negative_returns_clean_error(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, end_line=-1) + assert "end_line must be >= 1" in result + assert "line4" not in result + + +def test_end_line_past_eof_clamps_to_last_line(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, end_line=99) + assert result == _FIVE_LINES