mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
479067ea8c
commit
8fb44f1e48
@ -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.
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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]
|
||||
|
||||
@ -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
|
||||
|
||||
101
backend/tests/test_remote_read_file_line_numbers.py
Normal file
101
backend/tests/test_remote_read_file_line_numbers.py
Normal file
@ -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)
|
||||
Loading…
x
Reference in New Issue
Block a user