From 8fb44f1e48440ddad308c6749838ed24a7fce290 Mon Sep 17 00:00:00 2001 From: "Zhengxu (Joshua) Jin" <33077399+Jinzhengxu@users.noreply.github.com> Date: Sat, 10 Oct 2026 07:51:46 +0800 Subject: [PATCH] fix(sandbox): number remote read_file line ranges like LocalSandbox (#6585) * fix(sandbox): number remote read_file line ranges like LocalSandbox E2B, BoxLite, Tenki and OpenSandbox sliced ranged reads with str.splitlines(), which also ends lines at a bare "\r", \f, \v, \x1c-\x1e, \x85, \u2028 and \u2029. LocalSandbox.read_file, count_file_lines and the read_file truncation marker end lines only at "\n", so on those providers a ranged read of such a file returned different lines, and following a truncation marker's start_line went back into text already shown. Add split_file_lines next to count_file_lines and use it in the four providers. * test(sandbox): separate remote and host read paths --------- Co-authored-by: Willem Jiang --- README.md | 2 + .../harness/deerflow/community/boxlite/box.py | 3 +- .../community/e2b_sandbox/e2b_sandbox.py | 3 +- .../deerflow/community/opensandbox/sandbox.py | 3 +- .../deerflow/community/tenki/sandbox.py | 3 +- .../harness/deerflow/sandbox/AGENTS.md | 2 +- .../deerflow/sandbox/read_file_contract.py | 18 ++++ backend/tests/AGENTS.md | 6 ++ .../test_remote_read_file_line_numbers.py | 101 ++++++++++++++++++ 9 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 backend/tests/test_remote_read_file_line_numbers.py diff --git a/README.md b/README.md index a5b77ff59f..5d7bedcd20 100644 --- a/README.md +++ b/README.md @@ -2097,6 +2097,8 @@ Each task gets its own execution environment with a full filesystem view — ski The read-before-write gate ties each read mark to that `read_file` call's result, including custom tools returning multi-message `Command` updates. An unrelated result cannot authorize a write after a failed read or hide a successful read. +Ranged `read_file` calls count lines the same way on every sandbox provider: a line ends only at a newline. On E2B, BoxLite, Tenki, and OpenSandbox, a file with bare carriage returns (such as a saved progress log), form feeds, or Unicode line separators therefore returns the same lines as on the local sandbox, and the `start_line` a truncated read suggests points at the next unread line. + Concurrent reads and writes to the same file share a gate across synchronous and asynchronous tool calls. Async callers waiting for that gate do not occupy worker threads needed to finish the current read or write. Cancelling a waiting call leaves the current operation running; a call that already started file inspection still waits for that work to finish before releasing its gate. The built-in `grep` tool searches either one text file or all matching text files below a directory, so an agent can search an uploaded document directly without first broadening the request to the entire uploads directory. diff --git a/backend/packages/harness/deerflow/community/boxlite/box.py b/backend/packages/harness/deerflow/community/boxlite/box.py index d86dcb10ed..274d4e8ac2 100644 --- a/backend/packages/harness/deerflow/community/boxlite/box.py +++ b/backend/packages/harness/deerflow/community/boxlite/box.py @@ -25,6 +25,7 @@ import threading from typing import TYPE_CHECKING, TypeVar from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.read_file_contract import split_file_lines from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env @@ -230,7 +231,7 @@ class BoxliteBox(Sandbox): content = r.stdout or "" if start_line is None and end_line is None: return content - lines = content.splitlines() + lines = split_file_lines(content) # Clamp like LocalSandbox.read_file: a negative start would otherwise # wrap around through Python's negative-index slicing. start = max(start_line or 1, 1) 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 426ef2bae1..28ac559944 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py @@ -11,6 +11,7 @@ from e2b import CommandExitException, FileNotFoundException from e2b_code_interpreter import Sandbox as E2BClientSandbox from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.read_file_contract import split_file_lines from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env @@ -239,7 +240,7 @@ class E2BSandbox(Sandbox): 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() + lines = split_file_lines(text) # Clamp like LocalSandbox.read_file: a negative start would otherwise # wrap around through Python's negative-index slicing. start = max(start_line or 1, 1) diff --git a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py index a3aec4936a..0279d24118 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py +++ b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py @@ -12,6 +12,7 @@ from datetime import timedelta from typing import TYPE_CHECKING, Any from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.read_file_contract import split_file_lines from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env @@ -271,7 +272,7 @@ class OpenSandboxSandbox(Sandbox): return f"Error: {exc}" if start_line is None and end_line is None: return content or "" - lines = (content or "").splitlines() + lines = split_file_lines(content or "") # Clamp like LocalSandbox.read_file: a negative start would otherwise # wrap around through Python's negative-index slicing. start = max(start_line or 1, 1) diff --git a/backend/packages/harness/deerflow/community/tenki/sandbox.py b/backend/packages/harness/deerflow/community/tenki/sandbox.py index 2b41ce88da..bcd8c2d575 100644 --- a/backend/packages/harness/deerflow/community/tenki/sandbox.py +++ b/backend/packages/harness/deerflow/community/tenki/sandbox.py @@ -29,6 +29,7 @@ import threading from typing import TYPE_CHECKING, Any, TypeVar from deerflow.config.paths import VIRTUAL_PATH_PREFIX +from deerflow.sandbox.read_file_contract import split_file_lines from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command from deerflow.sandbox.remote_search import parse_remote_search_output, remote_search_command from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env @@ -299,7 +300,7 @@ class TenkiSandbox(Sandbox): return f"Error: {e}" if start_line is None and end_line is None: return content - lines = (content or "").splitlines() + lines = split_file_lines(content or "") # Clamp like LocalSandbox.read_file: a negative start would otherwise # wrap around through Python's negative-index slicing instead of # reading from the first line. diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 3eac775285..3e59b43a60 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -116,6 +116,6 @@ symlinks, and visible depth/size bounds. - `ls` - Directory listing (tree format, max 2 levels). Remote commands precheck root existence and emit `__DF_FIND_STATUS__:missing`; `find` status 1 is always an incomplete-traversal `OSError`. `AioSandbox.list_dir` parses only `completed`/`None` on #5634's selected generation: `hard_timeout` raises `TimeoutError` and keeps that generation reusable; an ambiguous timeout or status raises `OSError`, replays nothing, and fences that generation. Missing-session 404s drop the stale id without replay. Persistent/scoped shells fence all `httpx.TransportError`s before cleanup. - `glob` - Find files or directories below a root directory with bounded results - `grep` - Search one text file or recursively search a directory, with optional glob filtering and bounded line-level results -- `read_file` - Read file contents with optional line range; full reads keep stored line endings (Local `read_file`/`grep` read `newline="\n"`, writes `newline=""`), ranged reads join with `\n` +- `read_file` - Read file contents with optional line range; full reads keep stored line endings (Local `read_file`/`grep` read `newline="\n"`, writes `newline=""`), ranged reads join with `\n`; remote providers slice with `split_file_lines`, not `splitlines()` - `write_file` - Write/append to files, creates directories; overwrites by default and exposes the `append` argument in the model-facing schema for end-of-file writes; subject to the read-before-write gate when `read_before_write.enabled` (see Middleware Chain) - `str_replace` - Substring replacement (single or all occurrences); `_match_line_endings` gives `old_str`/`new_str` a CRLF file's endings; same-path serialization is scoped to `(sandbox.id, path)` so isolated sandboxes do not contend on identical virtual paths inside one process; subject to the read-before-write gate when `read_before_write.enabled` (see Middleware Chain) diff --git a/backend/packages/harness/deerflow/sandbox/read_file_contract.py b/backend/packages/harness/deerflow/sandbox/read_file_contract.py index dbbcd5aa55..be770ca344 100644 --- a/backend/packages/harness/deerflow/sandbox/read_file_contract.py +++ b/backend/packages/harness/deerflow/sandbox/read_file_contract.py @@ -35,3 +35,21 @@ def count_file_lines(content: str) -> int: if not content: return 0 return content.count("\n") + (1 if not content.endswith("\n") else 0) + + +def split_file_lines(content: str) -> list[str]: + """Split file content into the lines a ranged ``read_file`` numbers. + + Lines end only at ``"\\n"``, as in ``LocalSandbox.read_file`` and + :func:`count_file_lines`, so ``len(split_file_lines(c)) == count_file_lines(c)``. + A trailing ``"\\r"`` is dropped from each line, as ``LocalSandbox`` does for + CRLF files. Remote providers that slice fetched content use this instead of + ``splitlines()``, which also ends lines at a bare ``"\\r"``, ``\\f``, ``\\v``, + ``\\x1c``-``\\x1e``, ``\\x85``, ``\\u2028`` and ``\\u2029``. + """ + if not content: + return [] + lines = content.split("\n") + if lines[-1] == "": + lines.pop() + return [line.rstrip("\r") for line in lines] diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 78bd5ae7c4..6bc54aa519 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -109,6 +109,12 @@ the production PATH lookup, including a directory containing spaces. Seed fake app credentials so auth completion reaches the CLI instead of returning early, and keep fixture filesystem work behind `asyncio.to_thread`. +## Remote read fixtures + +Remote ranged-read fixtures pass POSIX sandbox paths to adapters and map those +paths to native temporary files in fake transports. LocalSandbox keeps the native +host path; do not relax remote path validation to accommodate Windows fixtures. + ## Shared sandbox search contracts `test_sandbox_search_contract.py` runs shared `ls`/`glob`/`grep` scenarios through diff --git a/backend/tests/test_remote_read_file_line_numbers.py b/backend/tests/test_remote_read_file_line_numbers.py new file mode 100644 index 0000000000..349190dd36 --- /dev/null +++ b/backend/tests/test_remote_read_file_line_numbers.py @@ -0,0 +1,101 @@ +"""Ranged read_file numbers lines the same way on every sandbox provider. + +``LocalSandbox.read_file``, ``count_file_lines`` and read_file's truncation +marker end lines only at ``"\\n"``. The remote providers slice the content they +fetched themselves, so they must use the same rule: ``str.splitlines()`` would +also end lines at a bare ``"\\r"``, ``\\f``, ``\\x85`` or ``\\u2028``, and a +continuation such as ``start_line=3`` would then return an earlier part of the +file than the one the marker pointed at. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from deerflow.sandbox.local.local_sandbox import LocalSandbox +from deerflow.sandbox.read_file_contract import READ_FILE_TRUNCATION_PREFIX, count_file_lines +from deerflow.sandbox.tools import _truncate_read_file_output + +# Four lines by the "\n"-only rule; splitlines() sees eight. +_CONTENT = "progress 10%\rprogress 100%\npage one\fpage two\nhead\u2028tail\r\nnext\x85line\n" +_RANGES = [(None, None), (2, 3), (3, None), (None, 2), (4, 4), (5, None)] +REMOTE_PROVIDERS = ("e2b", "boxlite", "tenki", "opensandbox") +_CONTINUE = re.compile(r"Continue with start_line=(\d+)") + + +def _remote_sandbox(name: str, monkeypatch: pytest.MonkeyPatch, remote_files: dict[str, Path]): + """Map POSIX sandbox paths to native host files in the fake transport.""" + + def read_text(path: str) -> str: + return remote_files[path].read_bytes().decode("utf-8") + + if name == "e2b": + from deerflow.community.e2b_sandbox.e2b_sandbox import E2BSandbox + + return E2BSandbox("line-contract", SimpleNamespace(files=SimpleNamespace(read=lambda path, **_kwargs: read_text(path)))) + if name == "boxlite": + from deerflow.community.boxlite.box import BoxliteBox + + sandbox = BoxliteBox("line-contract", SimpleNamespace(), run=None) + monkeypatch.setattr(sandbox, "_exec", lambda *argv: SimpleNamespace(exit_code=0, stdout=read_text(argv[-1]), stderr="")) + return sandbox + if name == "tenki": + from deerflow.community.tenki.sandbox import TenkiSandbox + + sandbox = TenkiSandbox("line-contract", SimpleNamespace()) + monkeypatch.setattr(sandbox, "_fs_op", lambda op: op(SimpleNamespace(read_text=read_text))) + return sandbox + if name == "opensandbox": + from deerflow.community.opensandbox.sandbox import OpenSandboxSandbox + + sandbox = OpenSandboxSandbox("line-contract", SimpleNamespace(), run_command_opts_cls=SimpleNamespace) + monkeypatch.setattr(sandbox, "_file_op", lambda operation: operation(SimpleNamespace(read_file=lambda path, **_kwargs: read_text(path)))) + return sandbox + raise AssertionError(f"Unregistered provider: {name}") + + +@pytest.mark.parametrize("provider", REMOTE_PROVIDERS) +def test_remote_ranged_read_matches_local_sandbox_line_numbers(provider, tmp_path, monkeypatch): + path = tmp_path / "mixed-separators.log" + path.write_bytes(_CONTENT.encode("utf-8")) + local = LocalSandbox("line-contract") + remote_path = f"/workspace/{path.name}" + remote = _remote_sandbox(provider, monkeypatch, {remote_path: path}) + + for start_line, end_line in _RANGES: + expected = local.read_file(str(path), start_line=start_line, end_line=end_line) + assert remote.read_file(remote_path, start_line=start_line, end_line=end_line) == expected, (start_line, end_line) + + # The line read_file's truncation marker would name after line 2. + assert remote.read_file(remote_path, start_line=3, end_line=3) == "head\u2028tail" + + +@pytest.mark.parametrize("provider", REMOTE_PROVIDERS) +def test_remote_read_continues_at_the_line_a_truncation_marker_names(provider, tmp_path, monkeypatch): + """A saved progress log: every line carries a bare carriage return.""" + path = tmp_path / "progress.log" + content = "".join(f"step {i:03d} 0%\rstep {i:03d} 100%\n" for i in range(1, 201)) + path.write_bytes(content.encode("utf-8")) + remote_path = f"/workspace/{path.name}" + remote = _remote_sandbox(provider, monkeypatch, {remote_path: path}) + + truncated = _truncate_read_file_output(remote.read_file(remote_path), 1000) + kept = truncated[: truncated.index(READ_FILE_TRUNCATION_PREFIX)] + start_line = int(_CONTINUE.search(truncated).group(1)) + first_unread = content[len(kept) :].split("\n", 1)[0] + + assert remote.read_file(remote_path, start_line=start_line, end_line=start_line) == first_unread + + +@pytest.mark.parametrize( + "content", + ["", "\n", "one", "one\n", "one\n\n", "a\rb\n", "a\r\nb", "a\fb\vc\x1cd\x85e\u2028f\u2029g\n", "trailing cr\r"], +) +def test_split_file_lines_agrees_with_count_file_lines(content): + from deerflow.sandbox.read_file_contract import split_file_lines + + assert len(split_file_lines(content)) == count_file_lines(content)