diff --git a/README.md b/README.md index 79be25490..6155dabaf 100644 --- a/README.md +++ b/README.md @@ -517,6 +517,10 @@ For Docker development, service startup follows `config.yaml` sandbox mode. In L See the [Sandbox Configuration Guide](backend/docs/CONFIGURATION.md#sandbox) to configure your preferred mode. +Remote directory listings report traversal failures (for example, unreadable +directories) as incomplete results, even when no entries were returned. A +missing start path is reported separately as “Directory not found.” + #### MCP Server In the chat UI, enable **Token Usage → Debug** to inspect generic/MCP tool calls. diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 8dcf6477c..f43f00c39 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -97,7 +97,7 @@ **Sandbox Tools** (in `packages/harness/deerflow/sandbox/tools.py`): - Every sandbox tool keeps a model-visible `description` field for a human-readable progress label, but the field is optional and defaults to an empty string. Tool execution must depend only on its operational arguments; the frontend supplies localized fallback labels when a provider omits `description`. - `bash` - Execute commands with path translation and error handling. For `LocalSandbox` (host bash), output on POSIX and Windows is captured through bounded pipe-drain threads and stdin is `/dev/null`; Windows capture decodes with the platform text encoding and applies universal-newline translation, matching the former `subprocess.run(..., text=True)` behavior for locale-code-page output, Python UTF-8 Mode, CRLF, and bare CR. That translation is Windows-only so the pre-existing POSIX output contract remains byte-decoded without newline rewriting. On POSIX, a backgrounded long-lived process (`server &`) returns immediately instead of blocking the turn on an inherited pipe, while unredirected background output is drained without growing anonymous temp files. Commands that read stdin get immediate EOF. The command runs in its own process group with a wall-clock timeout (`sandbox.bash_command_timeout`, default 600s); on timeout the whole POSIX process group or Windows process tree is killed and the agent gets a notice telling it to background long-lived processes. The shared bash tool description scopes host environment detection to LocalSandbox: start with `uname -s`, follow with `sw_vers` on Darwin, and read Linux host system files only when the active policy permits them. Local path and `file://` rejections provide the same conditional recovery guidance: command-only probes for environment questions, allowed virtual paths otherwise, and no repetition of the rejected path. The description also instructs the model to background long-lived processes (e.g. servers) up front so it doesn't waste the turn waiting on a foreground server. See `LocalSandbox.execute_command`, its platform runners, and `bash_tool`'s docstring. -- `ls` - Directory listing (tree format, max 2 levels) +- `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`, including when no entries were printed. Do not infer a missing path from status 1 alone. - `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 diff --git a/backend/packages/harness/deerflow/sandbox/remote_list_dir.py b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py index 9e2cc1ed9..461c67aff 100644 --- a/backend/packages/harness/deerflow/sandbox/remote_list_dir.py +++ b/backend/packages/harness/deerflow/sandbox/remote_list_dir.py @@ -5,8 +5,9 @@ does not enable ``pipefail``, so the pipeline's exit code is ``head``'s, not ``find``'s. A missing ``find`` binary (127) then looks like an empty listing and becomes ``FileNotFoundError``. -The command below writes ``find``'s own status after the bounded listing so -callers can tell a missing path from a command failure. ``head`` closing the +The command checks root existence before running ``find`` and writes its own +status after the bounded listing, distinguishing missing paths from traversal +failures even when no entries were printed. ``head`` closing the pipe can kill ``find`` with SIGPIPE (141); that is a successful truncation, not an error. """ @@ -16,10 +17,11 @@ from __future__ import annotations import shlex _STATUS_PREFIX = "__DF_FIND_STATUS__:" +_MISSING_ROOT = "missing" _LIST_LIMIT = 500 -# 0 = ok, 1 = find reported a missing start point / tree error, 141 = SIGPIPE -# from head truncating a large listing. -_FIND_OK = (0, 1, 141) +# 0 = ok, 141 = SIGPIPE from head truncating a large listing. +# A missing root has its own marker; status 1 always means traversal failed. +_FIND_OK = (0, 141) def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIMIT) -> str: @@ -33,7 +35,8 @@ def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIM # ``exit`` of that status (126 if the file is missing): the last command # would otherwise be ``rm``, whose 0/1 is not find's status. return ( - f"set +e; _st=/tmp/df_find_$$; " + f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; " + f"_st=/tmp/df_find_$$; " f"{{ find -H {quoted} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null; " f'echo $? > "$_st"; }} | head -n {n}; ' f'st=$(cat "$_st" 2>/dev/null); ' @@ -51,8 +54,8 @@ def parse_remote_list_dir_output( """Parse listing stdout, preferring the find-status marker over pipeline status. Raises: - OSError: Command/client failure (missing binary, invocation error, ...). - FileNotFoundError: ``find`` ran and produced no entries (missing path). + OSError: Command/client failure or an incomplete traversal. + FileNotFoundError: The root is missing or no listable entries exist. """ # find delimits records with "\n" only. splitlines() would also split on # \v, \f, \x1c-\x1e and \x85, which are legal in Linux filenames. Do not @@ -64,6 +67,8 @@ def parse_remote_list_dir_output( find_status: int | None = None if lines and lines[-1].startswith(_STATUS_PREFIX): raw = lines.pop()[len(_STATUS_PREFIX) :] + if raw == _MISSING_ROOT: + raise FileNotFoundError(resolved) try: find_status = int(raw) except ValueError: @@ -73,15 +78,18 @@ def parse_remote_list_dir_output( if find_status is None: # Do not treat a missing marker as success. The process status used to - # be ``rm``'s (0/1, both in _FIND_OK), which reclassified a lost 127 + # be ``rm``'s (0/1), which reclassified a lost 127 # as FileNotFoundError. - if pipeline_exit_code is not None and pipeline_exit_code not in _FIND_OK: + if pipeline_exit_code is not None and pipeline_exit_code not in (*_FIND_OK, 1): raise OSError(f"Failed to list_dir {resolved}: command exited with code {pipeline_exit_code}") raise OSError(f"Failed to list_dir {resolved}: find status marker missing") + + entries = [line for line in lines if line] + if find_status == 1: + raise OSError(f"Failed to list_dir {resolved}: find exited with code 1, usually because some files or directories could not be read; results would be incomplete, so list a narrower path") if find_status not in _FIND_OK: raise OSError(f"Failed to list_dir {resolved}: command exited with code {find_status}") - entries = [line for line in lines if line] if not entries: raise FileNotFoundError(resolved) return entries diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index e7d0e51ed..7ceee79c5 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -620,11 +620,13 @@ class TestListDirSerialization: with pytest.raises(OSError, match="Failed to list directory"): sandbox.list_dir("/test") - def test_list_dir_raises_when_find_returns_no_entries(self, sandbox): - sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="\n__DF_FIND_STATUS__:1\n", exit_code=1))) + @pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)]) + def test_list_dir_classifies_empty_failure(self, sandbox, marker, error): + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output=f"\n__DF_FIND_STATUS__:{marker}\n", exit_code=1))) - with pytest.raises(FileNotFoundError): + with pytest.raises(error) as exc: sandbox.list_dir("/missing") + assert type(exc.value) is error def test_list_dir_raises_oserror_when_result_data_is_none(self, sandbox): sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=None)) diff --git a/backend/tests/test_boxlite_provider.py b/backend/tests/test_boxlite_provider.py index 67bef80c3..0ea75152b 100644 --- a/backend/tests/test_boxlite_provider.py +++ b/backend/tests/test_boxlite_provider.py @@ -1327,19 +1327,21 @@ def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None: assert truncated is False -def test_list_dir_raises_when_find_returns_no_entries() -> None: +@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)]) +def test_list_dir_classifies_empty_failure(marker, error) -> None: class _EmptyBox: async def exec(self, *argv, env=None, timeout=None): - return types.SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1) + return types.SimpleNamespace(stdout=f"\n__DF_FIND_STATUS__:{marker}\n", stderr="", exit_code=1) box = BoxliteBox("box-id", box=_EmptyBox(), run=_fake_run) - with pytest.raises(FileNotFoundError): + with pytest.raises(error) as exc: box.list_dir("/mnt/user-data/workspace") + assert type(exc.value) is error def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path() -> None: - # find exit 1 is "start point absent"; 127 (no binary) must not look missing. + # 127 (no binary) must not look like a missing path. class _MissingBinaryBox: async def exec(self, *argv, env=None, timeout=None): return types.SimpleNamespace(stdout="", stderr="", exit_code=127) diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index 6932b7b2f..7e6dc68e9 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -5228,15 +5228,15 @@ def test_list_dir_raises_when_client_closed(): sb.list_dir("/home/user") -def test_list_dir_raises_when_find_returns_no_entries(): - # `find ... 2>/dev/null` on a missing path yields empty stdout; that is not - # a real empty directory (`find -type d` still prints the directory itself). - listing = SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1) +@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)]) +def test_list_dir_classifies_empty_failure(marker, error): + listing = SimpleNamespace(stdout=f"\n__DF_FIND_STATUS__:{marker}\n", stderr="", exit_code=1) client = FakeClient(commands=FakeCommandsAPI([listing])) sb = _make_sandbox(client) - with pytest.raises(FileNotFoundError): + with pytest.raises(error) as exc: sb.list_dir("/home/user/missing") + assert type(exc.value) is error def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path(): diff --git a/backend/tests/test_opensandbox_provider.py b/backend/tests/test_opensandbox_provider.py index 55a4425c0..4f9d813af 100644 --- a/backend/tests/test_opensandbox_provider.py +++ b/backend/tests/test_opensandbox_provider.py @@ -157,7 +157,7 @@ class _FakeCommands: matches = sorted(path for path in set(paths) if path == root or path.startswith(f"{root}/")) if "__DF_FIND_STATUS__:" in command: status = 0 if matches else 1 - marker = f"__DF_FIND_STATUS__:{status}" + marker = "__DF_FIND_STATUS__:0" if matches else "__DF_FIND_STATUS__:missing" stdout = (*matches, "", marker) if matches else ("", marker) return _execution(stdout=stdout, exit_code=status) return _execution(stdout=tuple(matches)) diff --git a/backend/tests/test_remote_list_dir.py b/backend/tests/test_remote_list_dir.py index 2cbbeef1a..052467b70 100644 --- a/backend/tests/test_remote_list_dir.py +++ b/backend/tests/test_remote_list_dir.py @@ -23,12 +23,23 @@ def test_parse_marker_127_is_command_failure_not_missing_path() -> None: parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) -def test_parse_marker_1_empty_is_missing_path() -> None: - stdout = "\n__DF_FIND_STATUS__:1\n" +def test_parse_missing_marker_is_missing_path() -> None: + stdout = "__DF_FIND_STATUS__:missing\n" with pytest.raises(FileNotFoundError): parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) +def test_parse_marker_1_empty_is_incomplete_failure() -> None: + with pytest.raises(OSError, match="results would be incomplete"): + parse_remote_list_dir_output("\n__DF_FIND_STATUS__:1\n", "/dir", pipeline_exit_code=1) + + +def test_parse_marker_1_with_entries_is_incomplete_failure() -> None: + stdout = "/dir/visible.txt\n\n__DF_FIND_STATUS__:1\n" + with pytest.raises(OSError, match="results would be incomplete"): + parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=1) + + def test_parse_marker_0_returns_listing_and_keeps_trailing_space() -> None: stdout = "/dir/notes.txt \n/dir/sub\n\n__DF_FIND_STATUS__:0\n" assert parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) == [ @@ -57,7 +68,7 @@ def test_parse_falls_back_to_pipeline_exit_without_marker() -> None: @_POSIX_SH def test_parse_without_marker_real_subprocess_status_is_not_always_ok() -> None: - """``rm -f`` exits 0/1, both in _FIND_OK. A real process status with no marker must not become FileNotFoundError.""" + """``rm -f`` exits 0/1. A process status without a marker must not become FileNotFoundError.""" for script in ("exit 0", "exit 1"): proc = subprocess.run(["sh", "-c", script], capture_output=True, text=True, check=False) with pytest.raises(OSError, match="marker missing"): @@ -76,7 +87,7 @@ def test_command_records_find_status_after_head() -> None: assert "head -n 500" in command assert "__DF_FIND_STATUS__:" in command assert command.index("find -H ") < command.index("head -n") - assert command.index("head -n") < command.index("__DF_FIND_STATUS__:") + assert command.index("head -n") < command.rindex("__DF_FIND_STATUS__:") assert 'exit "${st:-126}"' in command @@ -126,7 +137,7 @@ def _write_fake_find(tmp_path, script: str): def test_list_dir_command_surfaces_find_127_not_head_0(tmp_path) -> None: fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n") proc = _run_list_dir_script( - remote_list_dir_command("/dir", 2), + remote_list_dir_command(str(tmp_path), 2), env=_env_with_bin(str(fake_bin)), ) assert proc.returncode == 127 @@ -134,11 +145,23 @@ def test_list_dir_command_surfaces_find_127_not_head_0(tmp_path) -> None: parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) +@_POSIX_SH +def test_list_dir_command_surfaces_partial_find_failure(tmp_path) -> None: + fake_bin = _write_fake_find(tmp_path, '#!/bin/sh\nprintf "/dir/visible.txt\\n"\nexit 1\n') + proc = _run_list_dir_script( + remote_list_dir_command(str(tmp_path), 2), + env=_env_with_bin(str(fake_bin)), + ) + assert proc.returncode == 1 + with pytest.raises(OSError, match="results would be incomplete"): + parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) + + @_POSIX_SH def test_list_dir_command_records_find_127_under_set_e(tmp_path) -> None: fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n") proc = _run_list_dir_script( - "set -e; " + remote_list_dir_command("/dir", 2), + "set -e; " + remote_list_dir_command(str(tmp_path), 2), env=_env_with_bin(str(fake_bin)), ) assert proc.returncode == 127 @@ -151,6 +174,7 @@ def test_list_dir_command_records_find_127_under_set_e(tmp_path) -> None: def test_list_dir_command_missing_path_is_file_not_found(tmp_path) -> None: missing = tmp_path / "no-such-dir" proc = _run_list_dir_script(remote_list_dir_command(str(missing), 2)) + assert proc.stdout.strip() == "__DF_FIND_STATUS__:missing" with pytest.raises(FileNotFoundError): parse_remote_list_dir_output(proc.stdout, str(missing), pipeline_exit_code=proc.returncode) @@ -177,10 +201,21 @@ def test_list_dir_command_head_truncation_is_not_an_error(tmp_path) -> None: fake_find.chmod(fake_find.stat().st_mode | stat.S_IEXEC) proc = _run_list_dir_script( - remote_list_dir_command("/dir", 2), + remote_list_dir_command(str(tmp_path), 2), env=_env_with_bin(str(fake_bin)), ) entries = parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode) assert len(entries) == 500 assert entries[0] == "/dir/f1" assert entries[-1] == "/dir/f500" + + +@_POSIX_SH +def test_list_dir_existing_root_with_no_output_is_incomplete_failure(tmp_path) -> None: + root = tmp_path / "unreadable 'directory" + root.mkdir() + fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 1\n") + proc = _run_list_dir_script(remote_list_dir_command(str(root), 2), env=_env_with_bin(str(fake_bin))) + assert proc.returncode == 1 + with pytest.raises(OSError, match="results would be incomplete"): + parse_remote_list_dir_output(proc.stdout, str(root), pipeline_exit_code=proc.returncode) diff --git a/backend/tests/test_tenki_provider.py b/backend/tests/test_tenki_provider.py index 7a6e8e81e..3472d3c5d 100644 --- a/backend/tests/test_tenki_provider.py +++ b/backend/tests/test_tenki_provider.py @@ -173,7 +173,8 @@ class _FakeSandbox: listing = ("\n".join(hits) + "\n") if hits else "" if "__DF_FIND_STATUS__:" in script: status = 0 if hits else 1 - return _FakeResult(stdout=f"{listing}\n__DF_FIND_STATUS__:{status}\n".encode(), exit_code=status) + marker = "0" if hits else "missing" + return _FakeResult(stdout=f"{listing}\n__DF_FIND_STATUS__:{marker}\n".encode(), exit_code=status) return _FakeResult(stdout=listing.encode()) if script.startswith("grep "): # grep -e 2>/dev/null | head -N