fix(search): cap path_prefixes at the MCP tool; widen path filter tests
Round 2 review follow-ups: - Add Field(max_length=20) to the nc_semantic_search path_prefixes param so an LLM client can't build an unbounded OR-filter (mirrors the cap the Astrolabe PHP controller applies on the UI path). - Note in normalize_path_prefixes that the two-pass collect-then-strip is deliberate (the `if path_prefix:` guard is truthy for whitespace-only input; the strip pass is what drops it). - Tests: exercise build_base_filter_conditions with 3 folders (guards the list comprehension) and parametrize the no-path case over None, empty list, and blank-only inputs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
cd243ed6c3
commit
ea108140ab
@@ -217,6 +217,9 @@ def normalize_path_prefixes(
|
||||
if path_prefixes:
|
||||
raw.extend(path_prefixes)
|
||||
|
||||
# Two-pass on purpose: the ``if path_prefix:`` guard above is truthy for a
|
||||
# whitespace-only string like ``" "``, so the strip-and-drop pass below is
|
||||
# what actually removes it — collecting first keeps the dedup order stable.
|
||||
seen: set[str] = set()
|
||||
cleaned: list[str] = []
|
||||
for value in raw:
|
||||
|
||||
@@ -102,11 +102,13 @@ def configure_semantic_tools(mcp: FastMCP):
|
||||
path_prefixes: Annotated[
|
||||
list[str] | None,
|
||||
Field(
|
||||
max_length=20,
|
||||
description=(
|
||||
"Restrict to files under any of these folders/paths "
|
||||
"(e.g. ['/Projects/Reports', '/Shared/Specs']). Folders are "
|
||||
"OR-ed together. Matches the file_path of indexed files "
|
||||
"only, so setting it implicitly limits results to files. "
|
||||
"Capped at 20 folders to bound the OR-filter width. "
|
||||
"None or empty = no path filter."
|
||||
),
|
||||
),
|
||||
|
||||
@@ -265,11 +265,23 @@ class TestBuildBaseFilterConditions:
|
||||
assert match.text == prefix
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_no_path_condition_when_prefix_absent(self) -> None:
|
||||
conditions = build_base_filter_conditions("alice", None, path_prefix=None)
|
||||
@pytest.mark.parametrize(
|
||||
"kwargs",
|
||||
[
|
||||
{"path_prefix": None},
|
||||
{"path_prefixes": None},
|
||||
{"path_prefixes": []},
|
||||
{"path_prefix": " ", "path_prefixes": ["", " "]},
|
||||
],
|
||||
)
|
||||
def test_no_path_condition_when_prefix_absent(self, kwargs) -> None:
|
||||
# No folder filter (None, empty list, or blank-only) must add neither a
|
||||
# flat file_path condition nor a nested path OR.
|
||||
conditions = build_base_filter_conditions("alice", None, **kwargs)
|
||||
assert not any(
|
||||
isinstance(c, FieldCondition) and c.key == "file_path" for c in conditions
|
||||
)
|
||||
assert self._path_should_texts(conditions) is None
|
||||
|
||||
@staticmethod
|
||||
def _path_should_texts(conditions) -> set[str] | None:
|
||||
@@ -292,12 +304,18 @@ class TestBuildBaseFilterConditions:
|
||||
|
||||
@pytest.mark.unit
|
||||
def test_multiple_path_prefixes_or_in_nested_should(self) -> None:
|
||||
# Two+ folders must OR together: a single nested Filter(should=[...]) is
|
||||
# appended (not two must conditions, which would AND and match nothing).
|
||||
# 3+ folders must OR together: a single nested Filter(should=[...]) is
|
||||
# appended (not separate must conditions, which would AND and match
|
||||
# nothing). 3 folders also guards the list comprehension against an
|
||||
# off-by-one.
|
||||
conditions = build_base_filter_conditions(
|
||||
"alice", None, path_prefixes=["/Projects", "/Archive"]
|
||||
"alice", None, path_prefixes=["/Projects", "/Archive", "/Shared"]
|
||||
)
|
||||
assert self._path_should_texts(conditions) == {"/Projects", "/Archive"}
|
||||
assert self._path_should_texts(conditions) == {
|
||||
"/Projects",
|
||||
"/Archive",
|
||||
"/Shared",
|
||||
}
|
||||
# No bare file_path FieldCondition in must for the multi-folder case.
|
||||
assert not any(
|
||||
isinstance(c, FieldCondition) and c.key == "file_path" for c in conditions
|
||||
|
||||
Reference in New Issue
Block a user