mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(sandbox): scope ignore patterns to the search root in glob/grep (#5667)
* fix(sandbox): scope ignore patterns to the search root in glob/grep Rebuilt on top of current main so the changelog entry lands in the Unreleased (2.2.0) Fixed section rather than the now-released 2.1.0 block. * test(sandbox): cover the include_dirs glob branch at an ignored search root The file.list_path branch only ran against a non-ignored tree, so a regression there would go unnoticed. Two cases now exercise it with an ignored root: the requested root keeps its contents, and ignored descendants stay hidden.
This commit is contained in:
parent
9b17bef630
commit
e1ec4c6e43
11
CHANGELOG.md
11
CHANGELOG.md
@ -1730,6 +1730,16 @@ This release closes that milestone with **301 merged pull requests**.
|
||||
`uv-lock-check` hook. The script now refreshes the lock with `uv lock` and exits
|
||||
before editing anything when `uv` is missing, instead of leaving a half-bumped
|
||||
working tree behind. Only the root package's version line moves. ([#5859])
|
||||
- **sandbox:** Stop `glob` and `grep` from returning nothing when the search
|
||||
root — or one of its ancestors — matches an ignore pattern such as `build`,
|
||||
`dist`, `logs`, `node_modules`, `coverage` or `target`. The remote sandboxes
|
||||
applied `should_ignore_path` to the absolute path, which tests every segment,
|
||||
so one ignored name anywhere up the tree hid the whole result and the agent
|
||||
was told "no matches" for a directory `ls` had just listed. Ignore patterns
|
||||
are now applied to the path relative to the search root, as `list_dir` already
|
||||
did: an ignored name still hides its own descendants, but searching an
|
||||
ignored root — or a path below an ignored ancestor — returns its contents.
|
||||
([#5667])
|
||||
|
||||
- **sandbox:** The temporary sandbox lease acquired by the HTTP upload route is
|
||||
now released. In remote/provisioner deployments,
|
||||
@ -7414,6 +7424,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#5662]: https://github.com/bytedance/deer-flow/pull/5662
|
||||
[#5663]: https://github.com/bytedance/deer-flow/pull/5663
|
||||
[#5664]: https://github.com/bytedance/deer-flow/pull/5664
|
||||
[#5667]: https://github.com/bytedance/deer-flow/pull/5667
|
||||
[#5669]: https://github.com/bytedance/deer-flow/pull/5669
|
||||
[#5673]: https://github.com/bytedance/deer-flow/pull/5673
|
||||
[#5676]: https://github.com/bytedance/deer-flow/pull/5676
|
||||
|
||||
@ -1522,6 +1522,12 @@
|
||||
器现在通过共享的 `file_signature.read_config_with_signature()`
|
||||
helper 只读取一次,并对返回的字节精确签名;竞态编辑最多
|
||||
只多付出一次重载。([#5848])
|
||||
- **沙箱:** `glob` 与 `grep` 不再在搜索根目录(或其任一祖先目录)命中忽略模式(如 `build`、
|
||||
`dist`、`logs`、`node_modules`、`coverage`、`target`)时返回空结果。此前各远端沙箱把
|
||||
`should_ignore_path` 用在**绝对路径**上,而该函数会检查路径的每一段,于是树上任意位置的忽略名
|
||||
都会把整份结果隐藏,智能体刚 `ls` 出来的目录却被搜索告知“无匹配”。现在忽略模式按**搜索根的
|
||||
相对路径**生效,与 `list_dir` 已有的做法一致:忽略名依然隐藏自己的子孙目录,但以被忽略目录
|
||||
为根、或位于被忽略祖先之下的搜索会正常返回其内容。([#5667])
|
||||
|
||||
- **sandbox:** HTTP 上传路由获取的临时 sandbox 租约现在会释放。在
|
||||
remote/provisioner 部署中,`POST /api/threads/{id}/uploads` 会临时获取线
|
||||
@ -6164,6 +6170,7 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#5662]: https://github.com/bytedance/deer-flow/pull/5662
|
||||
[#5663]: https://github.com/bytedance/deer-flow/pull/5663
|
||||
[#5664]: https://github.com/bytedance/deer-flow/pull/5664
|
||||
[#5667]: https://github.com/bytedance/deer-flow/pull/5667
|
||||
[#5669]: https://github.com/bytedance/deer-flow/pull/5669
|
||||
[#5673]: https://github.com/bytedance/deer-flow/pull/5673
|
||||
[#5676]: https://github.com/bytedance/deer-flow/pull/5676
|
||||
|
||||
@ -13,7 +13,7 @@ from agent_sandbox.core.api_error import ApiError
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
from deerflow.sandbox.remote_list_dir import parse_remote_list_dir_output, remote_list_dir_command
|
||||
from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path_under_root, truncate_line
|
||||
|
||||
from .backend import sandbox_http_trust_env
|
||||
|
||||
@ -1165,22 +1165,22 @@ class AioSandbox(Sandbox):
|
||||
raise
|
||||
|
||||
def glob(self, path: str, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]:
|
||||
root_path = path.rstrip("/") or "/"
|
||||
if not include_dirs:
|
||||
result = self._client.file.find_files(path=path, glob=pattern)
|
||||
files = result.data.files if result.data and result.data.files else []
|
||||
filtered = [file_path for file_path in files if not should_ignore_path(file_path)]
|
||||
filtered = [file_path for file_path in files if not should_ignore_path_under_root(file_path, root_path)]
|
||||
truncated = len(filtered) > max_results
|
||||
return filtered[:max_results], truncated
|
||||
|
||||
result = self._client.file.list_path(path=path, recursive=True, show_hidden=False)
|
||||
entries = result.data.files if result.data and result.data.files else []
|
||||
matches: list[str] = []
|
||||
root_path = path.rstrip("/") or "/"
|
||||
root_prefix = root_path if root_path == "/" else f"{root_path}/"
|
||||
for entry in entries:
|
||||
if entry.path != root_path and not entry.path.startswith(root_prefix):
|
||||
continue
|
||||
if should_ignore_path(entry.path):
|
||||
if should_ignore_path_under_root(entry.path, root_path):
|
||||
continue
|
||||
rel_path = entry.path[len(root_path) :].lstrip("/")
|
||||
if path_matches(pattern, rel_path):
|
||||
@ -1232,7 +1232,7 @@ class AioSandbox(Sandbox):
|
||||
truncated = bool(data and data.truncated)
|
||||
for match in provider_matches:
|
||||
file_path = match.file
|
||||
if should_ignore_path(file_path):
|
||||
if should_ignore_path_under_root(file_path, root):
|
||||
continue
|
||||
if file_path == root:
|
||||
rel_path = file_path.rsplit("/", 1)[-1]
|
||||
|
||||
@ -28,7 +28,7 @@ from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
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
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path_under_root, truncate_line
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
@ -322,7 +322,7 @@ class BoxliteBox(Sandbox):
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)):
|
||||
continue
|
||||
if should_ignore_path(entry):
|
||||
if should_ignore_path_under_root(entry, root):
|
||||
continue
|
||||
rel_path = entry[len(root) :].lstrip("/")
|
||||
if not rel_path:
|
||||
@ -381,7 +381,7 @@ class BoxliteBox(Sandbox):
|
||||
line_number = int(line_no_str)
|
||||
except ValueError:
|
||||
continue
|
||||
if should_ignore_path(file_path):
|
||||
if should_ignore_path_under_root(file_path, root):
|
||||
continue
|
||||
if glob is not None:
|
||||
# Match the caller's real directory scope: a pattern like
|
||||
|
||||
@ -14,7 +14,7 @@ from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
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
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path_under_root, truncate_line
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from deerflow.community.e2b_sandbox.e2b_sandbox_provider import MountUploadResult
|
||||
@ -433,7 +433,7 @@ class E2BSandbox(Sandbox):
|
||||
continue
|
||||
if entry != root and not entry.startswith(root_prefix):
|
||||
continue
|
||||
if should_ignore_path(entry):
|
||||
if should_ignore_path_under_root(entry, root):
|
||||
continue
|
||||
rel_path = entry[len(root) :].lstrip("/")
|
||||
if not rel_path:
|
||||
@ -515,7 +515,7 @@ class E2BSandbox(Sandbox):
|
||||
line_number = int(line_no_str)
|
||||
except ValueError:
|
||||
continue
|
||||
if should_ignore_path(file_path):
|
||||
if should_ignore_path_under_root(file_path, root):
|
||||
continue
|
||||
if glob is not None:
|
||||
# Restrict to the caller's real directory scope -- the
|
||||
|
||||
@ -15,7 +15,7 @@ from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
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
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path_under_root, truncate_line
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
@ -357,7 +357,7 @@ class OpenSandboxSandbox(Sandbox):
|
||||
root_prefix = root if root == "/" else f"{root}/"
|
||||
for entry in output.text.splitlines():
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)) or should_ignore_path(entry):
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)) or should_ignore_path_under_root(entry, root):
|
||||
continue
|
||||
relative = entry[len(root) :].lstrip("/")
|
||||
if relative and path_matches(pattern, relative):
|
||||
@ -416,7 +416,7 @@ class OpenSandboxSandbox(Sandbox):
|
||||
line_number = int(line_number_text)
|
||||
except ValueError:
|
||||
continue
|
||||
if should_ignore_path(file_path):
|
||||
if should_ignore_path_under_root(file_path, root):
|
||||
continue
|
||||
if glob is not None:
|
||||
if file_path != root and not file_path.startswith(root_prefix):
|
||||
|
||||
@ -32,7 +32,7 @@ from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
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
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path, truncate_line
|
||||
from deerflow.sandbox.search import GrepMatch, path_matches, should_ignore_path_under_root, truncate_line
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable, Iterator
|
||||
@ -420,7 +420,7 @@ class TenkiSandbox(Sandbox):
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)):
|
||||
continue
|
||||
if should_ignore_path(entry):
|
||||
if should_ignore_path_under_root(entry, root):
|
||||
continue
|
||||
rel_path = entry[len(root) :].lstrip("/")
|
||||
if not rel_path:
|
||||
@ -481,7 +481,7 @@ class TenkiSandbox(Sandbox):
|
||||
line_number = int(line_no_str)
|
||||
except ValueError:
|
||||
continue
|
||||
if should_ignore_path(file_path):
|
||||
if should_ignore_path_under_root(file_path, root):
|
||||
continue
|
||||
if glob is not None:
|
||||
# Match the caller's real directory scope: a pattern like
|
||||
|
||||
@ -104,6 +104,31 @@ def _match_segments(pattern_parts: tuple[str, ...], path_parts: tuple[str, ...])
|
||||
return len(path_parts) in reachable
|
||||
|
||||
|
||||
def should_ignore_path_under_root(path: str, root: str) -> bool:
|
||||
"""Apply :func:`should_ignore_path` to ``path`` as seen from ``root``.
|
||||
|
||||
Ignore patterns are scoped to the search root, the way ``list_dir`` already
|
||||
applies them: an ignored name hides its own descendants, so searching
|
||||
``build`` -- or any path below an ignored ancestor -- still returns its
|
||||
contents. Testing the absolute path instead silently hides every result as
|
||||
soon as the search root, or one of its ancestors, is itself an ignored
|
||||
name, so ``glob``/``grep`` reported "no matches" for a directory the local
|
||||
walk lists without any trouble.
|
||||
"""
|
||||
root_clean = root.rstrip("/")
|
||||
if not root_clean:
|
||||
root_clean = "/"
|
||||
if path == root_clean:
|
||||
# The caller asked for this exact path. There is nothing relative to
|
||||
# test, and an ignored name must not hide what was explicitly
|
||||
# requested (a single-file ``grep`` hits this on every match).
|
||||
return False
|
||||
prefix = "/" if root_clean == "/" else f"{root_clean}/"
|
||||
if path.startswith(prefix):
|
||||
return should_ignore_path(path[len(prefix) :])
|
||||
return should_ignore_path(path)
|
||||
|
||||
|
||||
def path_matches(pattern: str, rel_path: str) -> bool:
|
||||
"""Match ``rel_path`` (relative to the search root) against a glob pattern.
|
||||
|
||||
|
||||
@ -137,6 +137,24 @@ def tree(tmp_path):
|
||||
return root
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def ignored_name_tree(tmp_path):
|
||||
"""A tree whose root *and* one nested directory carry an ignored name.
|
||||
|
||||
``build`` is in ``IGNORE_PATTERNS``; ``node_modules`` is ignored one level
|
||||
below it. ``list_dir`` already scopes ignore patterns to the listing root,
|
||||
so an ignored name may not hide the root that was explicitly requested,
|
||||
while an ignored name inside the tree keeps hiding its descendants.
|
||||
"""
|
||||
root = tmp_path / "build"
|
||||
(root / "node_modules" / "dep").mkdir(parents=True)
|
||||
(root / "workspace").mkdir()
|
||||
(root / "notes.txt").write_text("header\nneedle build\n", encoding="utf-8")
|
||||
(root / "workspace" / "report.txt").write_text("needle nested\n", encoding="utf-8")
|
||||
(root / "node_modules" / "dep" / "lib.js").write_text("needle hidden\n", encoding="utf-8")
|
||||
return root
|
||||
|
||||
|
||||
def _relative_paths(paths, root):
|
||||
# Directory suffixes and inclusion of the requested root are legitimate
|
||||
# provider presentation differences; names and descendants must agree.
|
||||
@ -167,6 +185,40 @@ def test_glob_can_include_directories(provider, tree):
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_search_root_at_an_ignored_name_keeps_its_contents(provider, ignored_name_tree):
|
||||
assert _relative_paths(provider.sandbox.list_dir(str(ignored_name_tree)), ignored_name_tree) == {"notes.txt", "workspace", "workspace/report.txt"}
|
||||
matches, truncated = provider.sandbox.glob(str(ignored_name_tree), "**/*.txt")
|
||||
assert _relative_paths(matches, ignored_name_tree) == {"notes.txt", "workspace/report.txt"}
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_search_root_below_an_ignored_name_keeps_its_contents(provider, ignored_name_tree):
|
||||
root = ignored_name_tree / "workspace"
|
||||
matches, truncated = provider.sandbox.glob(str(root), "**/*.txt")
|
||||
assert _relative_paths(matches, root) == {"report.txt"}
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_search_root_at_an_ignored_name_still_hides_ignored_descendants(provider, ignored_name_tree):
|
||||
assert provider.sandbox.glob(str(ignored_name_tree), "**/*.js") == ([], False)
|
||||
matches, truncated = provider.sandbox.grep(str(ignored_name_tree), "needle")
|
||||
assert {(Path(m.path).relative_to(ignored_name_tree).as_posix(), m.line_number) for m in matches} == {
|
||||
("notes.txt", 2),
|
||||
("workspace/report.txt", 1),
|
||||
}
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_glob_with_directories_at_an_ignored_name_keeps_its_contents(provider, ignored_name_tree):
|
||||
matches, truncated = provider.sandbox.glob(str(ignored_name_tree), "**/workspace", include_dirs=True)
|
||||
assert _relative_paths(matches, ignored_name_tree) == {"workspace"}
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_glob_with_directories_at_an_ignored_name_still_hides_ignored_descendants(provider, ignored_name_tree):
|
||||
assert provider.sandbox.glob(str(ignored_name_tree), "**/*.js", include_dirs=True) == ([], False)
|
||||
|
||||
|
||||
def test_grep_preserves_line_numbers_and_ignores_descendants(provider, tree):
|
||||
matches, truncated = provider.sandbox.grep(str(tree), "needle", case_sensitive=True)
|
||||
assert {(Path(m.path).relative_to(tree).as_posix(), m.line_number, m.line) for m in matches} == {
|
||||
|
||||
56
backend/tests/test_should_ignore_path_under_root.py
Normal file
56
backend/tests/test_should_ignore_path_under_root.py
Normal file
@ -0,0 +1,56 @@
|
||||
"""Ignore patterns are scoped to the search root, not to the absolute path.
|
||||
|
||||
``list_dir`` already applies ``IGNORE_PATTERNS`` relative to the listing root:
|
||||
explicitly listing ``build`` -- or any path below an ignored ancestor -- returns
|
||||
its contents. ``glob``/``grep`` used to test the *absolute* path, so a search
|
||||
rooted at (or under) an ignored name returned nothing on every remote sandbox
|
||||
while the local walk listed the very same files without any trouble.
|
||||
|
||||
These are the unit tests for the helper itself; the end-to-end cases that drive
|
||||
the real provider ``list_dir``/``glob``/``grep`` for every provider live in
|
||||
``test_sandbox_search_contract.py``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from deerflow.sandbox.search import should_ignore_path, should_ignore_path_under_root
|
||||
|
||||
|
||||
def test_ignored_root_keeps_its_contents() -> None:
|
||||
"""`glob build/**` must not come back empty just because the root is named ``build``."""
|
||||
path = "/data/build/app.js"
|
||||
assert should_ignore_path(path) is True # what the old absolute check did
|
||||
assert should_ignore_path_under_root(path, "/data/build") is False
|
||||
|
||||
|
||||
def test_ignored_ancestor_of_the_root_hides_nothing() -> None:
|
||||
assert should_ignore_path_under_root("/data/build/workspace/report.txt", "/data/build/workspace") is False
|
||||
|
||||
|
||||
def test_ignored_descendant_of_the_root_is_still_hidden() -> None:
|
||||
assert should_ignore_path_under_root("/data/node_modules", "/data") is True
|
||||
assert should_ignore_path_under_root("/data/node_modules/pkg/index.js", "/data") is True
|
||||
|
||||
|
||||
def test_ignored_descendant_below_an_ignored_root_is_hidden() -> None:
|
||||
"""An ignored root still hides its own descendants, as ``list_dir`` does."""
|
||||
assert should_ignore_path_under_root("/data/build/node_modules/pkg/index.js", "/data/build") is True
|
||||
|
||||
|
||||
def test_the_requested_root_itself_is_never_hidden() -> None:
|
||||
"""A single-file ``grep`` hits this on every match: ``file_path == root``."""
|
||||
assert should_ignore_path_under_root("/data/logs", "/data/logs") is False
|
||||
assert should_ignore_path_under_root("/data/build/notes.txt", "/data/build/notes.txt") is False
|
||||
|
||||
|
||||
def test_path_outside_the_root_keeps_the_absolute_check() -> None:
|
||||
assert should_ignore_path_under_root("/other/node_modules/x.js", "/data") is True
|
||||
|
||||
|
||||
def test_root_is_the_filesystem_root() -> None:
|
||||
assert should_ignore_path_under_root("/node_modules/x.js", "/") is True
|
||||
assert should_ignore_path_under_root("/home/a.py", "/") is False
|
||||
|
||||
|
||||
def test_trailing_slash_on_the_root_is_ignored() -> None:
|
||||
assert should_ignore_path_under_root("/data/build/app.js", "/data/build/") is False
|
||||
Loading…
x
Reference in New Issue
Block a user