diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py index a3be91863..3a1a8ed07 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py @@ -281,7 +281,12 @@ class AioSandbox(Sandbox): logger.error(f"Failed to execute command with injected env in sandbox: {e}") return f"Error: {e}" - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: """Read the content of a file in the sandbox. Args: @@ -291,7 +296,12 @@ class AioSandbox(Sandbox): The content of the file. """ try: - result = self._client.file.read_file(file=path) + kwargs = {} + if start_line is not None: + kwargs["start_line"] = max(start_line - 1, 0) + if end_line is not None: + kwargs["end_line"] = max(end_line, 0) + result = self._client.file.read_file(file=path, **kwargs) return result.data.content if result.data else "" except Exception as e: logger.error(f"Failed to read file in sandbox: {e}") diff --git a/backend/packages/harness/deerflow/community/boxlite/box.py b/backend/packages/harness/deerflow/community/boxlite/box.py index 534d7e7dd..afdb383f1 100644 --- a/backend/packages/harness/deerflow/community/boxlite/box.py +++ b/backend/packages/harness/deerflow/community/boxlite/box.py @@ -205,7 +205,12 @@ class BoxliteBox(Sandbox): # ── file operations ───────────────────────────────────────────────── - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: resolved = self._resolve_path(path) try: r = self._exec("cat", "--", resolved) @@ -214,7 +219,13 @@ class BoxliteBox(Sandbox): return f"Error: {e}" if r.exit_code not in (0, None): return f"Error: {(r.stderr or '').strip() or 'cannot read file'}" - return r.stdout or "" + content = r.stdout or "" + if start_line is None and end_line is None: + return content + lines = content.splitlines() + start = start_line or 1 + end = end_line if end_line is not None else len(lines) + return "\n".join(lines[start - 1 : end]) def write_file(self, path: str, content: str, append: bool = False) -> None: self._write_bytes(self._resolve_path(path), content.encode("utf-8"), append=append) diff --git a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py index e8607975a..92295d012 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py @@ -205,13 +205,23 @@ class E2BSandbox(Sandbox): logger.warning("e2b sandbox ping raised non-fatal error: %s", e) return True - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: resolved = self._resolve_path(path) try: content = self._client.files.read(resolved) - if isinstance(content, bytes): - return content.decode("utf-8", errors="replace") - return content if content is not None else "" + if start_line is None and end_line is None: + return content.decode("utf-8", errors="replace") if isinstance(content, bytes) else content or "" + text = content.decode("utf-8", errors="replace") if isinstance(content, bytes) else content or "" + lines = text.splitlines() + start = start_line or 1 + end = end_line if end_line is not None else len(lines) + content = "\n".join(lines[start - 1 : end]) + return content except Exception as e: logger.error("Failed to read file %s in e2b sandbox: %s", resolved, e) return f"Error: {e}" diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index aafb1c529..9f70e8a3c 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py @@ -678,11 +678,29 @@ class LocalSandbox(Sandbox): return sorted(result) - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: resolved_path = self._resolve_path(path) + should_slice = start_line is not None or end_line is not None try: with open(resolved_path, encoding="utf-8") as f: - content = f.read() + if not should_slice: + content = f.read() + + start = max(start_line or 1, 1) + if should_slice: + selected: list[str] = [] + for line_number, line in enumerate(f, start=1): + if line_number < start: + continue + if end_line is not None and line_number > end_line: + break + selected.append(line.rstrip("\r\n")) + content = "\n".join(selected) # Only reverse-resolve paths in files that were previously written # by write_file (agent-authored content). User-uploaded files, # external tool output, and other non-agent content should not be diff --git a/backend/packages/harness/deerflow/sandbox/sandbox.py b/backend/packages/harness/deerflow/sandbox/sandbox.py index 5147a9ac2..9ca966d57 100644 --- a/backend/packages/harness/deerflow/sandbox/sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/sandbox.py @@ -91,11 +91,18 @@ class Sandbox(ABC): pass @abstractmethod - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: """Read the content of a file. Args: path: The absolute path of the file to read. + start_line: Optional starting line number (1-indexed, inclusive). + end_line: Optional ending line number (1-indexed, inclusive). Returns: The content of the file. diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index 6a7f6e32b..1ad1379f7 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -2138,29 +2138,44 @@ def read_file_tool( Args: description: Explain why you are reading this file in short words. ALWAYS PROVIDE THIS PARAMETER FIRST. path: The **absolute** path to the file to read. - start_line: Optional starting line number (1-indexed, inclusive). Use with end_line to read a specific range. - end_line: Optional ending line number (1-indexed, inclusive). Use with start_line to read a specific range. + start_line: Optional starting line number (1-indexed, inclusive). Omit to start at the first line. + end_line: Optional ending line number (1-indexed, inclusive). Omit to read through the last line. """ try: # Block access to disabled skill files if _is_disabled_skill_path(path, user_id=resolve_runtime_user_id(runtime)): skill_name = _extract_skill_name_from_skills_path(path) or "unknown" return f"Error: Skill '{skill_name}' is disabled. Access to its files is blocked. Enable the skill in settings before using it." + if start_line is not None and start_line < 1: + return "(start_line must be >= 1)" + effective_start = start_line or 1 + if end_line is not None and end_line < 1: + return "(end_line must be >= 1)" + if end_line is not None and effective_start > end_line: + return "(start_line > end_line — no lines in range)" + requested_path = path - content = read_current_file_content(runtime, path) + sandbox = ensure_sandbox_initialized(runtime) + ensure_thread_directories_exist(runtime) + use_line_range = start_line is not None or end_line is not None + if use_line_range: + if is_local_sandbox(runtime): + thread_data = get_thread_data(runtime) + validate_local_tool_path(path, thread_data, read_only=True) + if _is_skills_path(path): + path = _resolve_skills_path(path) + elif _is_acp_workspace_path(path): + path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data)) + elif not _is_custom_mount_path(path): + path = _resolve_and_validate_user_data_path(path, thread_data) + # Custom mount paths are resolved by LocalSandbox._resolve_path() + content = sandbox.read_file(path, start_line=start_line, end_line=end_line) + else: + content = read_current_file_content(runtime, path) if not content: - return "(empty)" - 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): + if start_line is not None and start_line > 1: 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]) + return "(empty)" try: from deerflow.config.app_config import get_app_config diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index 7aa5aed47..356a00363 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -358,6 +358,20 @@ class TestNoChangeTimeout: assert calls[0].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT +class TestReadFile: + def test_read_file_forwards_requested_line_range(self, sandbox): + sandbox._client.file.read_file = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(content="line 1\nline 2"))) + + result = sandbox.read_file("/mnt/user-data/workspace/huge.log", start_line=1, end_line=10) + + assert result == "line 1\nline 2" + sandbox._client.file.read_file.assert_called_once_with( + file="/mnt/user-data/workspace/huge.log", + start_line=0, + end_line=10, + ) + + class TestConcurrentFileWrites: """Verify file write paths do not lose concurrent updates.""" diff --git a/backend/tests/test_boxlite_provider.py b/backend/tests/test_boxlite_provider.py index c151dd83c..58ac686b8 100644 --- a/backend/tests/test_boxlite_provider.py +++ b/backend/tests/test_boxlite_provider.py @@ -165,6 +165,28 @@ def test_execute_command_forwards_timeout_to_sdk_and_loop_runner() -> None: assert run_timeouts == [5] +def test_read_file_supports_optional_line_ranges(monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[tuple[str, ...]] = [] + + box = BoxliteBox("box-id", box=object(), run=_fake_run) + + def _fake_exec(*argv: str): + calls.append(argv) + return types.SimpleNamespace( + exit_code=0, + stdout="line 1\nline 2\nline 3\nline 4\nline 5", + stderr="", + ) + + monkeypatch.setattr(box, "_exec", _fake_exec) + + assert box.read_file("/mnt/user-data/workspace/range.txt") == "line 1\nline 2\nline 3\nline 4\nline 5" + assert box.read_file("/mnt/user-data/workspace/range.txt", start_line=2, end_line=4) == "line 2\nline 3\nline 4" + assert box.read_file("/mnt/user-data/workspace/range.txt", start_line=4) == "line 4\nline 5" + assert box.read_file("/mnt/user-data/workspace/range.txt", end_line=2) == "line 1\nline 2" + assert all(call == ("cat", "--", "/mnt/user-data/workspace/range.txt") for call in calls) + + def test_grep_always_prints_filename_for_single_file_paths() -> None: fake = _FakeBox(name="box-id") box = BoxliteBox("box-id", box=fake, run=_fake_run) diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index c438ab476..cd3aa865f 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -1739,6 +1739,29 @@ def test_sync_outputs_to_host_is_noop_when_client_closed(): p._sync_outputs_to_host(sb, thread_id="t1", user_id="u1") +def test_read_file_supports_bounded_ranges(): + files = FakeFilesAPI( + store={"/home/user/workspace/range.txt": b"line 1\nline 2\nline 3\nline 4\nline 5"}, + ) + client = FakeClient(files=files) + sb = _make_sandbox(client, sandbox_id="sb-read-range") + + assert sb.read_file("/mnt/user-data/workspace/range.txt") == "line 1\nline 2\nline 3\nline 4\nline 5" + assert sb.read_file("/mnt/user-data/workspace/range.txt", start_line=2, end_line=4) == "line 2\nline 3\nline 4" + assert sb.read_file("/mnt/user-data/workspace/range.txt", start_line=4) == "line 4\nline 5" + assert sb.read_file("/mnt/user-data/workspace/range.txt", end_line=2) == "line 1\nline 2" + + resolved_path = "/home/user/workspace/range.txt" + assert all(path == resolved_path for path, _fmt in files.read_calls), files.read_calls + + +def test_read_file_returns_error_for_missing_file(): + client = FakeClient(files=FakeFilesAPI()) + sb = _make_sandbox(client, sandbox_id="sb-read-missing") + + assert sb.read_file("/mnt/user-data/workspace/missing.txt").startswith("Error:") + + def _outputs_dir(tmp_path): return Paths(base_dir=tmp_path).thread_dir("t1", user_id="u1") / "user-data" / "outputs" diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index f072d6378..a6ae73037 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -797,6 +797,77 @@ class TestLocalSandboxProviderMounts: # The container path should be preserved through roundtrip assert "/mnt/data/config.json" in result + def test_read_file_line_range_streams_without_full_read(self, tmp_path): + """Bounded line reads should stream without slurping the whole file.""" + data_dir = tmp_path / "data" + data_dir.mkdir() + big_file = data_dir / "huge.log" + big_file.write_text("\n".join(f"line {i}" for i in range(1, 2000)), encoding="utf-8") + + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/data", local_path=str(data_dir)), + ], + ) + + class GuardedFile: + def __init__(self, wrapped): + self._wrapped = wrapped + + def __enter__(self): + self._wrapped.__enter__() + return self + + def __exit__(self, exc_type, exc, tb): + return self._wrapped.__exit__(exc_type, exc, tb) + + def __iter__(self): + return self + + def __next__(self): + return next(self._wrapped) + + def read(self, *args, **kwargs): + raise AssertionError("full read() should not be used for ranged reads") + + def __getattr__(self, name): + return getattr(self._wrapped, name) + + import builtins + + real_open = builtins.open + + def guarded_open(file, *args, **kwargs): + handle = real_open(file, *args, **kwargs) + if Path(file) == big_file: + return GuardedFile(handle) + return handle + + with patch("builtins.open", side_effect=guarded_open): + content = sandbox.read_file("/mnt/data/huge.log", start_line=1, end_line=10) + + assert content == "\n".join(f"line {i}" for i in range(1, 11)) + + def test_read_file_single_sided_line_ranges_supported(self, tmp_path): + """LocalSandbox should support partial reads when only one bound is provided.""" + data_dir = tmp_path / "data" + data_dir.mkdir() + (data_dir / "range.txt").write_text( + "\n".join(f"line {i}" for i in range(1, 11)), + encoding="utf-8", + ) + + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/data", local_path=str(data_dir)), + ], + ) + + assert sandbox.read_file("/mnt/data/range.txt", start_line=8) == "line 8\nline 9\nline 10" + assert sandbox.read_file("/mnt/data/range.txt", end_line=3) == "line 1\nline 2\nline 3" + def test_setup_path_mappings_normalizes_container_path_trailing_slash(self, tmp_path): skills_dir = tmp_path / "skills" skills_dir.mkdir() diff --git a/backend/tests/test_read_file_line_range.py b/backend/tests/test_read_file_line_range.py index 9c4da1af5..a29654092 100644 --- a/backend/tests/test_read_file_line_range.py +++ b/backend/tests/test_read_file_line_range.py @@ -3,9 +3,9 @@ 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). +tail-from-start, head-to-end, rejection of non-positive line numbers, and clean +error strings for an inverted range or a start beyond EOF (instead of an +empty/garbage slice). """ from pathlib import Path @@ -54,9 +54,16 @@ def test_only_end_line_returns_head_up_to_that_line(tmp_path, monkeypatch) -> No assert result == "line1\nline2" -def test_start_line_zero_is_clamped_to_first_line(tmp_path, monkeypatch) -> None: +def test_start_line_zero_returns_clean_error(tmp_path, monkeypatch) -> None: result = _read(tmp_path, monkeypatch, start_line=0) - assert result == _FIVE_LINES + assert "start_line must be >= 1" in result + assert "line1" not in result + + +def test_start_line_negative_returns_clean_error(tmp_path, monkeypatch) -> None: + result = _read(tmp_path, monkeypatch, start_line=-1) + assert "start_line must be >= 1" in result + assert "line5" not in result def test_start_line_greater_than_end_line_returns_clean_error(tmp_path, monkeypatch) -> None: diff --git a/backend/tests/test_read_file_tool_binary.py b/backend/tests/test_read_file_tool_binary.py index 31a6f9129..633ec76ba 100644 --- a/backend/tests/test_read_file_tool_binary.py +++ b/backend/tests/test_read_file_tool_binary.py @@ -63,3 +63,83 @@ def test_read_file_tool_text_file_unaffected(tmp_path, monkeypatch) -> None: assert "hello 你好" in result, result assert "binary" not in result.lower(), result + + +def test_read_file_tool_passes_line_range_into_sandbox(monkeypatch) -> None: + captured: dict[str, int | str | None] = {} + + class RangeAwareSandbox: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: + captured["path"] = path + captured["start_line"] = start_line + captured["end_line"] = end_line + return "line 1\nline 2" + + runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"}) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: RangeAwareSandbox()) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None) + + result = read_file_tool.func( + runtime=runtime, + description="read top of file", + path="/mnt/user-data/workspace/huge.log", + start_line=1, + end_line=10, + ) + + assert result == "line 1\nline 2" + assert captured == { + "path": "/mnt/user-data/workspace/huge.log", + "start_line": 1, + "end_line": 10, + } + + +def test_read_file_tool_passes_open_ended_ranges_into_sandbox(monkeypatch) -> None: + calls: list[tuple[int | None, int | None]] = [] + + class RangeAwareSandbox: + def read_file(self, path: str, start_line: int | None = None, end_line: int | None = None) -> str: + calls.append((start_line, end_line)) + return "line 1\nline 2" + + runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"}) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: RangeAwareSandbox()) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None) + + start_only = read_file_tool.func( + runtime=runtime, + description="read tail", + path="/mnt/user-data/workspace/huge.log", + start_line=1, + ) + end_only = read_file_tool.func( + runtime=runtime, + description="read head", + path="/mnt/user-data/workspace/huge.log", + end_line=10, + ) + + assert start_only == "line 1\nline 2" + assert end_only == "line 1\nline 2" + assert calls == [(1, None), (None, 10)] + + +def test_read_file_tool_validates_range_order(monkeypatch) -> None: + runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"}) + monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: object()) + + result = read_file_tool.func( + runtime=runtime, + description="bad order", + path="/mnt/user-data/workspace/huge.log", + start_line=20, + end_line=10, + ) + + assert "start_line > end_line" in result diff --git a/backend/tests/test_sandbox_middleware.py b/backend/tests/test_sandbox_middleware.py index ad8818add..ac6818b8a 100644 --- a/backend/tests/test_sandbox_middleware.py +++ b/backend/tests/test_sandbox_middleware.py @@ -46,7 +46,12 @@ class _SandboxStub(Sandbox): del env, timeout return "OK" - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: return "content" def download_file(self, path: str) -> bytes: diff --git a/backend/tests/test_sandbox_tools_security.py b/backend/tests/test_sandbox_tools_security.py index 59d0bc068..930eda1d4 100644 --- a/backend/tests/test_sandbox_tools_security.py +++ b/backend/tests/test_sandbox_tools_security.py @@ -1245,7 +1245,12 @@ def test_str_replace_parallel_updates_should_preserve_both_edits(monkeypatch) -> self._state_lock = threading.Lock() self._overlap_detected = threading.Event() - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: with self._state_lock: self._active_reads += 1 snapshot = self.content @@ -1308,7 +1313,12 @@ def test_str_replace_parallel_updates_in_isolated_sandboxes_should_not_share_pat self.content = "alpha\nbeta\n" self._shared_state = shared_state - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: state_lock = self._shared_state["state_lock"] with state_lock: active_reads = self._shared_state["active_reads"] @@ -1390,7 +1400,12 @@ def test_str_replace_and_append_on_same_path_should_preserve_both_updates(monkey self.str_replace_has_snapshot = threading.Event() self.append_finished = threading.Event() - def read_file(self, path: str) -> str: + def read_file( + self, + path: str, + start_line: int | None = None, + end_line: int | None = None, + ) -> str: with self.state_lock: snapshot = self.content self.str_replace_has_snapshot.set()