diff --git a/CHANGELOG.md b/CHANGELOG.md index a53634749..cc000c6df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ hydrated only from projects in scope. Every search result, on both routes, now carries `project_id` and `project_external_id`. +- **#1558**: `search_notes(search_all_projects=True)` runs one scoped query per database + instead of one search per project, so a local vault with many projects is one + query, and each cloud workspace is one query, with results attributed to their + project by the server rather than by which request they came back on. A new + `projects` parameter searches a chosen subset by name or external id; an unknown + name is an error. Cross-database results are still merged by score, one failing + database is skipped with a warning and an inexact total, and a retryable outage + or a server too old to attribute its results fails the whole page. + - **#1512**: Word, PowerPoint, and CSV files get the same sidecar Markdown note a PDF gets. `bm import document ` indexes the project, extracts the file, and writes `..md` next to it plus a run note under diff --git a/src/basic_memory/man/man3/search-notes(3).md b/src/basic_memory/man/man3/search-notes(3).md index 83804f61e..f252de77d 100644 --- a/src/basic_memory/man/man3/search-notes(3).md +++ b/src/basic_memory/man/man3/search-notes(3).md @@ -21,7 +21,7 @@ MCP: ``` search_notes(query=None, project=None, project_id=None, - search_all_projects=False, page=1, page_size=10, + search_all_projects=False, projects=None, page=1, page_size=10, search_type=None, output_format="text", note_types=None, entity_types=None, categories=None, after_date=None, metadata_filters=None, tags=None, status=None, @@ -88,6 +88,7 @@ It is not a hard token budget: titles and metadata can still be large. - **project** (string | null, optional, default: None) — Project name to search in. Optional - server will resolve using hierarchy. If unknown, use list_memory_projects() to discover available projects. - **project_id** (string | null, optional, default: None) — Project external_id (UUID). Prefer this over `project` when known — it routes to the exact project regardless of name collisions across cloud workspaces. Takes precedence over `project`. Get from list_memory_projects(). - **search_all_projects** (boolean, optional, default: False) — Optional opt-in to search every accessible project. Ignored when `project` or `project_id` is supplied. +- **projects** (array | null, optional, default: None) — Optional list of project names or external ids to search together. Names are matched exactly as list_memory_projects() reports them (cloud projects by their workspace-qualified name). Ignored when `project` or `project_id` is supplied; an unknown name is an error rather than a silent skip. - **page** (integer, optional, default: 1) — The page number of results to return (default 1). Aliases: page_number. - **page_size** (integer, optional, default: 10) — The number of results to return per page (default 10). Aliases: limit, per_page. - **search_type** (string | null, optional, default: None) — Type of search to perform, one of: "text", "title", "permalink", "vector", "semantic", "hybrid". Default is dynamic: "hybrid" when semantic search is enabled, otherwise "text". diff --git a/src/basic_memory/mcp/clients/__init__.py b/src/basic_memory/mcp/clients/__init__.py index 303ff9f67..28190add2 100644 --- a/src/basic_memory/mcp/clients/__init__.py +++ b/src/basic_memory/mcp/clients/__init__.py @@ -12,7 +12,7 @@ """ from basic_memory.mcp.clients.knowledge import KnowledgeClient -from basic_memory.mcp.clients.search import SearchClient +from basic_memory.mcp.clients.search import ScopedSearchClient, SearchClient from basic_memory.mcp.clients.memory import MemoryClient from basic_memory.mcp.clients.directory import DirectoryClient from basic_memory.mcp.clients.resource import ResourceClient @@ -22,6 +22,7 @@ __all__ = [ "KnowledgeClient", + "ScopedSearchClient", "SearchClient", "MemoryClient", "DirectoryClient", diff --git a/src/basic_memory/mcp/clients/search.py b/src/basic_memory/mcp/clients/search.py index 08886178f..932e964e5 100644 --- a/src/basic_memory/mcp/clients/search.py +++ b/src/basic_memory/mcp/clients/search.py @@ -1,8 +1,10 @@ -"""Typed client for search API operations. +"""Typed clients for search API operations. -Encapsulates all /v2/projects/{project_id}/search/* endpoints. +``SearchClient`` covers /v2/projects/{project_id}/search/*; ``ScopedSearchClient`` +covers QUERY /v2/search/, one search over an explicit set of projects. """ +from collections.abc import Mapping, Sequence from typing import Any from httpx import AsyncClient @@ -19,6 +21,71 @@ _TEMPORAL_QUERY_FIELDS = ("valid_at", "valid_overlaps", "time_kind") +def _confirm_temporal_filter_applied(query: Mapping[str, Any], payload: Mapping[str, Any]) -> None: + """Refuse a response that does not confirm a requested valid-time filter ran. + + Trigger: this request carried a valid-time filter but the response does not + confirm the server ran it. + Why: SearchQuery ignores unknown fields, so a server predating SPEC-82 accepts + the request and returns results that look filtered. A valid-time query + excludes undated sources; unfiltered results include them, and the caller + would have no way to tell. + Outcome: fail loudly instead of returning a wrong answer that reads as right. + """ + if any(query.get(field) for field in _TEMPORAL_QUERY_FIELDS) and ( + payload.get("temporal_applied") is not True + ): + raise ValueError( + "The search API did not apply the requested valid-time filter " + "(no temporal_applied confirmation in the response). The server is " + "likely older than this client; upgrade it or drop valid_at / " + "valid_overlaps / time_kind from the query." + ) + + +class ScopedSearchClient: + """Typed client for ``QUERY /v2/search/``: one search over an explicit set of projects. + + ``project_ids`` are the internal ids the project list reports; every id must + belong to the database the HTTP client is routed to. An empty set answers no rows. + """ + + def __init__(self, http_client: AsyncClient): + self.http_client = http_client + + async def search( + self, + query: dict[str, Any], + *, + project_ids: Sequence[int], + page: int = 1, + page_size: int = 10, + ) -> SearchResponse: + """Search the named projects; results carry ``project_id`` and ``project_external_id``.""" + from basic_memory.mcp.tools.utils import call_query + + with logfire.span( + "mcp.client.search.search_scope", + client_name="search", + operation="search_scope", + project_count=len(project_ids), + page=page, + page_size=page_size, + ): + response = await call_query( + self.http_client, + "/v2/search/", + json={**query, "project_ids": list(project_ids)}, + params={"page": page, "page_size": page_size}, + client_name="search", + operation="search_scope", + path_template="/v2/search/", + ) + payload = response.json() + _confirm_temporal_filter_applied(query, payload) + return SearchResponse.model_validate(payload) + + class SearchClient: """Typed client for search operations. @@ -92,21 +159,5 @@ async def search( retrieval_mode = query.get("retrieval_mode", SearchRetrievalMode.FTS) payload["total_is_exact"] = retrieval_mode == SearchRetrievalMode.FTS - # Trigger: this request carried a valid-time filter but the response does not - # confirm the server ran it. - # Why: SearchQuery ignores unknown fields, so a server predating SPEC-82 accepts - # the request and returns results that look filtered. A valid-time query - # excludes undated sources; unfiltered results include them, and the caller - # would have no way to tell. - # Outcome: fail loudly instead of returning a wrong answer that reads as right. - if any(query.get(field) for field in _TEMPORAL_QUERY_FIELDS) and ( - payload.get("temporal_applied") is not True - ): - raise ValueError( - "The search API did not apply the requested valid-time filter " - "(no temporal_applied confirmation in the response). The server is " - "likely older than this client; upgrade it or drop valid_at / " - "valid_overlaps / time_kind from the query." - ) - + _confirm_temporal_filter_applied(query, payload) return SearchResponse.model_validate(payload) diff --git a/src/basic_memory/mcp/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py index e59bbee78..b9bd53407 100644 --- a/src/basic_memory/mcp/tools/project_management.py +++ b/src/basic_memory/mcp/tools/project_management.py @@ -124,6 +124,7 @@ def _merge_projects( { "name": name, "external_id": external_id, + "id": proj.id if proj else None, "path": path, "local_path": local_path, "cloud_path": cloud_path, @@ -258,6 +259,7 @@ def _merge_workspace_projects( { "name": cloud_proj.name, "external_id": cloud_proj.external_id, + "id": cloud_proj.id, "path": local_path or cloud_path, "local_path": local_path, "cloud_path": cloud_path, @@ -283,6 +285,7 @@ def _merge_workspace_projects( { "name": project.name, "external_id": project.external_id, + "id": project.id, "path": project.path, "local_path": project.path, "cloud_path": None, diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py index 75f75f409..e02da891f 100644 --- a/src/basic_memory/mcp/tools/search.py +++ b/src/basic_memory/mcp/tools/search.py @@ -1,6 +1,7 @@ """Search tools for Basic Memory MCP server.""" import re +from dataclasses import dataclass from textwrap import dedent from typing import Annotated, List, Optional, Dict, Any, Literal, cast from uuid import UUID @@ -11,7 +12,7 @@ from fastmcp import Context from pydantic import AliasChoices, BeforeValidator, Field -from basic_memory.config import ConfigManager, has_cloud_credentials +from basic_memory.config import ConfigManager from basic_memory.utils import ( build_canonical_permalink, coerce_dict, @@ -19,11 +20,7 @@ parse_tags, strict_search_tags, ) -from basic_memory.mcp.async_client import ( - _explicit_routing, - _force_local_mode, - is_factory_mode, -) +from basic_memory.mcp.async_client import get_client from basic_memory.mcp.container import get_container from basic_memory.mcp.index_readiness import project_index_required from basic_memory.mcp.project_context import ( @@ -33,6 +30,7 @@ ) from basic_memory.mcp.server import mcp from basic_memory.schemas.base import normalize_note_type +from basic_memory.schemas.project_info import ProjectItem from basic_memory.schemas.search import ( SearchItemType, SearchQuery, @@ -43,6 +41,118 @@ from basic_memory.temporal import TemporalQualifierError, parse_temporal_filter _SERVICE_UNAVAILABLE_HEADING = "# Search Failed - Service Temporarily Unavailable" +_NO_SEARCH_CRITERIA_MESSAGE = ( + "# No Search Criteria\n\n" + "Please provide at least one of: `query`, `metadata_filters`, " + "`tags`, `status`, `note_types`, `entity_types`, `categories`, " + "`after_date`, `valid_at`, `valid_overlaps`, or `time_kind`." +) +# Alias common column/model names to their frontmatter key equivalents. Users often +# pass "note_type" (the entity model column) when the frontmatter field is "type". +_METADATA_KEY_ALIASES = {"note_type": "type"} +_VALID_SEARCH_TYPES = ("hybrid", "permalink", "semantic", "text", "title", "vector") + + +def _build_search_query( + *, + query: str | None, + search_type: str, + note_types: list[str], + entity_types: list[str], + categories: list[str], + after_date: str | None, + metadata_filters: dict[str, Any] | None, + tags: list[str] | None, + status: str | None, + min_similarity: float | None, + valid_at: str | None, + valid_overlaps: str | None, + time_kind: str | None, +) -> SearchQuery | None: + """Map tool parameters onto one ``SearchQuery``; ``None`` when nothing narrows the search. + + Shared by the project search and the all-projects search so both ask the API + the same question for the same parameters. + """ + search_query = SearchQuery() + + # Only map search_type to query fields when there is an actual query string. + # When query is None/empty, skip the search mode block: filters-only path. + effective_query = (query or "").strip() + if effective_query: + if search_type == "text": + search_query.text = effective_query + search_query.retrieval_mode = SearchRetrievalMode.FTS + elif search_type in ("vector", "semantic"): + search_query.text = effective_query + search_query.retrieval_mode = SearchRetrievalMode.VECTOR + elif search_type == "hybrid": + search_query.text = effective_query + search_query.retrieval_mode = SearchRetrievalMode.HYBRID + elif search_type == "title": + search_query.title = effective_query + elif search_type == "permalink" and "*" in effective_query: + search_query.permalink_match = effective_query + elif search_type == "permalink": + search_query.permalink = effective_query + else: + raise ValueError( + f"Invalid search_type '{search_type}'. " + f"Valid options: {', '.join(_VALID_SEARCH_TYPES)}" + ) + + # Add optional filters if provided (empty lists are treated as no filter) + if entity_types: + search_query.entity_types = [SearchItemType(t) for t in entity_types] + if categories: + search_query.categories = categories + if note_types: + search_query.note_types = note_types + if after_date: + search_query.after_date = after_date + if metadata_filters: + search_query.metadata_filters = { + _METADATA_KEY_ALIASES.get(key, key): value for key, value in metadata_filters.items() + } + if tags: + search_query.tags = tags + if status: + search_query.status = status + if min_similarity is not None: + search_query.min_similarity = min_similarity + # Presence, not truthiness, for the same reason as everywhere else on this path: + # these are assigned after construction, so the model's own blank guard never + # runs here, and a blank has already been refused by the tool. + if valid_at is not None: + search_query.valid_at = valid_at + if valid_overlaps is not None: + search_query.valid_overlaps = valid_overlaps + if time_kind is not None: + search_query.time_kind = time_kind + + if search_query.no_criteria(): + return None + + # Default to entity-level results to avoid returning individual + # observations/relations as separate search results (see issue #31). + # Applied after no_criteria() so that the implicit default doesn't + # mask a truly empty search request. + if not search_query.entity_types: + # Trigger: a category or valid-time filter was supplied without an + # explicit entity_types. + # Why: both only exist on observations. Categories live on observation + # rows, and temporal assertions are projected against an + # observation's (type, id). Defaulting to "entity" would AND either + # filter against entity rows and return nothing, defeating the + # whole query. + # Outcome: scope the implicit default to observations so + # search_notes(categories=[...]) and search_notes(valid_at=...) + # return the matching bullets. + if search_query.categories or search_query.has_temporal_filter(): + search_query.entity_types = [SearchItemType("observation")] + else: + search_query.entity_types = [SearchItemType("entity")] + return search_query def _compact_search_response(response: SearchResponse) -> SearchResponse: @@ -474,8 +584,28 @@ def _matches_constrained_project(project: dict[str, Any], constrained_project: o return constrained_project in candidates -def _search_project_refs(projects_payload: object) -> list[dict[str, str | None]]: - """Extract project routing refs for optional account-scoped search.""" +@dataclass(frozen=True) +class SearchProjectRef: + """One project an all-projects search may read, and the database it lives in. + + ``name`` is the routable spelling (``workspace/project`` for a cloud project), + which is also the prefix results are qualified with. ``workspace_tenant_id`` is + ``None`` for the local database; every cloud workspace is its own database. + """ + + name: str + external_id: str + id: int + workspace_tenant_id: str | None + path: str + + @property + def bare_name(self) -> str: + return self.name.rsplit("/", 1)[-1] + + +def _search_project_refs(projects_payload: object) -> list[SearchProjectRef]: + """Extract the projects an all-projects search can read from the project list.""" if not isinstance(projects_payload, dict): return [] @@ -484,8 +614,8 @@ def _search_project_refs(projects_payload: object) -> list[dict[str, str | None] if not isinstance(projects, list): return [] - refs: list[dict[str, str | None]] = [] - seen: set[tuple[str | None, str | None]] = set() + refs: list[SearchProjectRef] = [] + seen: set[str] = set() constrained_project = payload.get("constrained_project") for item in projects: if not isinstance(item, dict) or not _matches_constrained_project( @@ -494,42 +624,64 @@ def _search_project_refs(projects_payload: object) -> list[dict[str, str | None] continue project = item.get("qualified_name") or item.get("name") - project_name = project if isinstance(project, str) and project.strip() else None - project_id = _valid_project_id(item.get("external_id")) - if project_name is None and project_id is None: + external_id = _valid_project_id(item.get("external_id")) + internal_id = item.get("id") + # A scoped search addresses a project by its id and attributes hits by its + # external id; a list row missing either cannot be searched this way. + if ( + not isinstance(project, str) + or not project.strip() + or external_id is None + or not isinstance(internal_id, int) + or isinstance(internal_id, bool) + ): continue - - key = (project_name, project_id) - if key in seen: + if external_id in seen: continue - seen.add(key) - refs.append({"project": project_name, "project_id": project_id}) + seen.add(external_id) + tenant = item.get("workspace_tenant_id") + refs.append( + SearchProjectRef( + name=project, + external_id=external_id, + id=internal_id, + workspace_tenant_id=tenant if isinstance(tenant, str) and tenant else None, + path=str(item.get("path") or ""), + ) + ) return refs -async def _load_search_project_refs(context: Context | None = None) -> list[dict[str, str | None]]: +def _select_project_refs( + refs: list[SearchProjectRef], projects: list[str] +) -> list[SearchProjectRef]: + """Keep the projects a caller named, by name, workspace-qualified name, or external id.""" + selected: list[SearchProjectRef] = [] + unknown: list[str] = [] + for requested in projects: + token = requested.strip() + matches = [ref for ref in refs if token in (ref.name, ref.bare_name, ref.external_id)] + if not matches: + unknown.append(token) + continue + for ref in matches: + if ref not in selected: + selected.append(ref) + if unknown: + available = ", ".join(sorted(ref.name for ref in refs)) or "none" + raise ValueError( + f"Unknown project(s): {', '.join(unknown)}. Available projects: {available}" + ) + return selected + + +async def _load_search_project_refs(context: Context | None = None) -> list[SearchProjectRef]: """Load accessible projects for search_all_projects without coupling the wrapper tool.""" from basic_memory.mcp.tools.project_management import list_memory_projects return _search_project_refs(await list_memory_projects(output_format="json", context=context)) -def _raw_results_from_search_payload( - results: SearchResponse | list[SearchResult | dict[str, Any]] | dict[str, Any], -) -> list[SearchResult | dict[str, Any]]: - """Return the result list from any search_notes JSON-compatible payload.""" - if isinstance(results, SearchResponse): - return list(results.results) - if isinstance(results, dict): - nested_results = results.get("results") - return ( - cast(list[SearchResult | dict[str, Any]], nested_results) - if isinstance(nested_results, list) - else [] - ) - return list(results) - - def _result_score(result: SearchResult | dict[str, Any]) -> float: """Return a comparable search score for merged project results.""" if isinstance(result, SearchResult): @@ -561,51 +713,57 @@ def _qualify_permalink_for_project(permalink: object, project: str | None) -> ob ) -def _qualify_results_for_project( - results: list[SearchResult | dict[str, Any]], - project_ref: dict[str, str | None], +def _qualify_result_for_project( + result: SearchResult, + project: str, *, compact: bool = False, +) -> dict[str, Any]: + """Attach the searched workspace/project prefix to one result's permalink.""" + result_data = result.model_dump() + if compact and result_data.get("type") == SearchItemType.OBSERVATION: + # This is an exact file read target, not a generated permalink: + # retain its extension, spaces, and case under the local project or + # workspace/project route. + result_data["permalink"] = f"{project.strip('/')}/{result_data['file_path'].lstrip('/')}" + else: + result_data["permalink"] = _qualify_permalink_for_project( + result_data.get("permalink"), project + ) + return result_data + + +def _attribute_results( + results: list[SearchResult], + refs: list[SearchProjectRef], + *, + compact: bool, ) -> list[dict[str, Any]]: - """Attach the searched workspace/project prefix to each result permalink.""" - qualified: list[dict[str, Any]] = [] + """Qualify each hit's permalink with the project the server says it came from.""" + refs_by_external_id = {ref.external_id: ref for ref in refs} + attributed: list[dict[str, Any]] = [] for result in results: - if isinstance(result, SearchResult): - result_data = result.model_dump() - else: - result_data = dict(result) - project = project_ref.get("project") - if compact and result_data.get("type") == SearchItemType.OBSERVATION and project: - # This is an exact file read target, not a generated permalink: - # retain its extension, spaces, and case under the local project or - # workspace/project route. - result_data["permalink"] = ( - f"{project.strip('/')}/{result_data['file_path'].lstrip('/')}" + ref = refs_by_external_id.get(result.project_external_id or "") + if ref is None: + raise ValueError( + "The search API returned a result it did not attribute to a project in " + "the requested scope. The server is likely older than this client; " + "upgrade it before searching across projects." ) - else: - result_data["permalink"] = _qualify_permalink_for_project( - result_data.get("permalink"), project - ) - qualified.append(result_data) - return qualified - + attributed.append(_qualify_result_for_project(result, ref.name, compact=compact)) + return attributed -def _result_total(results: dict[str, Any], raw_results: list[SearchResult | dict[str, Any]]) -> int: - """Return the best available total for a per-project search payload.""" - total = results.get("total") - if isinstance(total, int) and total > 0: - return total - return len(raw_results) + (1 if results.get("has_more") is True else 0) +def _database_label(tenant_id: str | None, refs: list[SearchProjectRef]) -> str: + """Name one searched database for logs and error responses.""" + if tenant_id is None: + return "local projects" + return f"workspace {refs[0].name.split('/', 1)[0]}" -def _result_total_is_exact(results: dict[str, Any]) -> bool: - """Return whether a per-project payload explicitly guarantees an exact total.""" - return results.get("total_is_exact") is True - -def _project_ref_label(project_ref: dict[str, str | None]) -> str: - """Return a stable log label for a project search ref.""" - return project_ref.get("project") or project_ref.get("project_id") or "" +def _project_item(ref: SearchProjectRef) -> ProjectItem: + """The project shape the readiness check reads (external id and name).""" + return ProjectItem(id=ref.id, external_id=ref.external_id, name=ref.bare_name, path=ref.path) async def _search_all_projects( @@ -628,20 +786,24 @@ async def _search_all_projects( time_kind: str | None, context: Context | None, compact: bool = False, + projects: list[str] | None = None, ) -> dict[str, Any] | str: - """Search every accessible project when the caller explicitly opts in.""" + """Search every accessible project, one query per database. + + Projects that share a database are searched with one scoped query, so within that + database the answer is one ranking rather than per-project pages merged after the + fact. The local database is one group and each cloud workspace is another; only + the merge across databases happens in this process, by score. + """ requested_page = max(page, 1) requested_page_size = max(page_size, 1) - # Each per-project call runs through search_notes -> SearchClient, which refuses a - # response that does not confirm the filter ran. So a project either honored the - # valid-time filter or was dropped with a warning below; the merged answer never - # silently mixes filtered and unfiltered rows. The filter itself is already known to - # be well formed -- search_notes parses it before reaching here -- which is what - # makes "dropped with a warning" mean an unavailable project and nothing else. # Presence, not truthiness: a blank value is refused by `parse_temporal_filter` # before any project is searched, so anything not None is a real question here. temporal_requested = valid_at is not None or valid_overlaps is not None or time_kind is not None project_refs = await _load_search_project_refs(context=context) + if projects: + project_refs = _select_project_refs(project_refs, projects) + scope_label = ", ".join(ref.name for ref in project_refs) if projects else "all projects" if not project_refs: response = SearchResponse( results=[], @@ -654,110 +816,110 @@ async def _search_all_projects( ) if output_format == "json": return response.model_dump(mode="json", exclude_none=True) - return _format_search_markdown(response, "all projects", query) + return _format_search_markdown(response, scope_label, query) + + effective_search_type = search_type or _default_search_type() + search_query = _build_search_query( + query=query, + search_type=effective_search_type, + note_types=note_types, + entity_types=entity_types, + categories=categories, + after_date=after_date, + metadata_filters=metadata_filters, + tags=tags, + status=status, + min_similarity=min_similarity, + valid_at=valid_at, + valid_overlaps=valid_overlaps, + time_kind=time_kind, + ) + if search_query is None: + return _NO_SEARCH_CRITERIA_MESSAGE + query_payload = search_query.model_dump() - per_project_page_size = requested_page * requested_page_size + # Import here to avoid circular import (tools -> clients -> utils -> tools) + from basic_memory.mcp.clients import ScopedSearchClient + + databases: dict[str | None, list[SearchProjectRef]] = {} + for ref in project_refs: + databases.setdefault(ref.workspace_tenant_id, []).append(ref) + + # Each database answers the whole prefix this page needs, so the merge across + # databases can slice it without a second round of requests. + per_database_page_size = requested_page * requested_page_size merged_results: list[dict[str, Any]] = [] total = 0 total_is_exact = True - any_project_has_more = False - # How many projects actually answered. A leg that fails is skipped with a warning, - # so without this the caller cannot tell "no note matched" from "nothing ran". - projects_answered = 0 + any_database_has_more = False + # How many databases actually answered. A database that fails is skipped with a + # warning, so without this the caller cannot tell "no note matched" from + # "nothing ran". + databases_answered = 0 + failures: list[str] = [] query_hint: str | None = None - # Trigger: caller asked for an account-wide search. - # Why: project_id (external UUID) routes through the cloud v2 API path, - # which 401s on local installs because there's no JWT to present. - # Project names route through the local-ASGI path and work for both - # backends — cloud disambiguates names via the workspace/project - # qualified_name already baked into project_ref["project"]. - # Outcome: forward project_id only when the same signals get_project_client - # uses to pick a cloud route are present. Mirrors the cloud_available - # composite in project_context.get_project_client (single source of - # truth for "can we route to cloud?"). - config = ConfigManager().config - use_cloud_routing = ( - is_factory_mode() - or (_explicit_routing() and not _force_local_mode()) - or has_cloud_credentials(config) - ) - - for project_ref in project_refs: - recursive_project_id = project_ref["project_id"] if use_cloud_routing else None + for tenant_id, refs in databases.items(): + label = _database_label(tenant_id, refs) try: - results = await search_notes( - query=query, - project=project_ref["project"], - project_id=recursive_project_id, - page=1, - page_size=per_project_page_size, - search_type=search_type, - output_format="json", - note_types=note_types or None, - entity_types=entity_types or None, - categories=categories or None, - after_date=after_date, - metadata_filters=metadata_filters, - tags=tags, - status=status, - min_similarity=min_similarity, - valid_at=valid_at, - valid_overlaps=valid_overlaps, - time_kind=time_kind, - search_all_projects=False, - context=context, - # Project qualification below must run after compact replaces - # observation excerpt permalinks with their owning file paths. - compact=compact, - ) + async with get_client(workspace=tenant_id) as client: + response = await ScopedSearchClient(client).search( + query_payload, + project_ids=[ref.id for ref in refs], + page=1, + page_size=per_database_page_size, + ) + if not response.results: + # An empty page is a trustworthy miss only after an index pass. + for ref in refs: + guidance = await project_index_required(client, _project_item(ref)) + if guidance is not None: + return guidance except Exception as exc: - logger.warning( - f"Multi-project search failed for project {_project_ref_label(project_ref)}: {exc}" - ) + if _is_service_unavailable_error(exc): + return _format_service_unavailable_response(label, str(exc), query or "") + logger.warning(f"Multi-project search failed for {label}: {exc}") + failures.append(f"{label}: {exc}") total_is_exact = False continue - if isinstance(results, str): - if results.startswith(_SERVICE_UNAVAILABLE_HEADING): - return results - if not results.startswith("# Search Failed"): - return results - logger.warning( - "Multi-project search failed for project " - f"{_project_ref_label(project_ref)}: {results}" - ) - total_is_exact = False - continue - - projects_answered += 1 - if isinstance(results.get("query_hint"), str): - query_hint = results["query_hint"] - raw_results = _raw_results_from_search_payload(results) - total += _result_total(results, raw_results) - total_is_exact = total_is_exact and _result_total_is_exact(results) - any_project_has_more = any_project_has_more or results.get("has_more") is True - merged_results.extend( - _qualify_results_for_project(raw_results, project_ref, compact=compact) + databases_answered += 1 + if compact: + response = _compact_search_response(response) + if response.query_hint: + query_hint = response.query_hint + total += ( + response.total + if response.total > 0 + else len(response.results) + (1 if response.has_more else 0) ) - - # Trigger: a valid-time filter was requested and not one project answered. - # Why: each leg confirms the filter through SearchClient or is refused by it, and a - # refusal is caught above, logged, and skipped -- so a fleet of servers predating - # SPEC-82 drops every leg and arrives here indistinguishable from "no note matched". - # Claiming `temporal_applied` on that would confirm a filter that ran nowhere, which - # is the version skew the client's own check exists to make loud. + total_is_exact = total_is_exact and response.total_is_exact + any_database_has_more = any_database_has_more or response.has_more + merged_results.extend(_attribute_results(response.results, refs, compact=compact)) + + # Trigger: a valid-time filter was requested and not one database answered. + # Why: each database confirms the filter through the client or is refused by it, + # and a refusal is caught above, logged, and skipped -- so a fleet of servers + # predating SPEC-82 drops every database and arrives here indistinguishable from + # "no note matched". Claiming `temporal_applied` on that would confirm a filter + # that ran nowhere, which is the version skew the client's own check exists to + # make loud. # Outcome: the skew is propagated as one error naming it, rather than returning an # empty result wearing the shape of a successful filtered search. - if temporal_requested and projects_answered == 0: + if temporal_requested and databases_answered == 0: raise ValueError( "No project applied the requested valid-time filter: every project was " "skipped, so the filter ran nowhere and an empty result would not mean " "'no matches'. The servers are likely older than this client; upgrade them " "or drop valid_at / valid_overlaps / time_kind from the query." ) + if databases_answered == 0: + # Every database failed. That is a failed search, not an empty one. + return _format_search_error_response( + scope_label, "; ".join(failures), query or "", effective_search_type + ) - # Each project owns retrieval and optional reranking behind its typed API client. + # Each database owns retrieval and optional reranking behind its typed API client. # The MCP process only merges returned scores; it must not instantiate repository # providers with local credentials for content fetched through another route. sorted_results = sorted(merged_results, key=_result_score, reverse=True) @@ -767,17 +929,17 @@ async def _search_all_projects( response = SearchResponse.model_validate( { "results": paged_results, - # Only propagate query guidance when every project answered and the - # aggregate is empty; one empty leg must not label a successful search. + # Only propagate query guidance when every database answered and the + # aggregate is empty; one empty database must not label a successful search. "query_hint": query_hint - if requested_page == 1 and not merged_results and projects_answered == len(project_refs) + if requested_page == 1 and not merged_results and databases_answered == len(databases) else None, "current_page": requested_page, "page_size": requested_page_size, "total": total, "total_is_exact": total_is_exact, - "has_more": any_project_has_more or total > end or len(sorted_results) > end, - # Confirmed only because a project answered: `projects_answered` is + "has_more": any_database_has_more or total > end or len(sorted_results) > end, + # Confirmed only because a database answered: `databases_answered` is # non-zero here for any temporal query, guarded immediately above. "temporal_applied": True if temporal_requested else None, } @@ -785,7 +947,7 @@ async def _search_all_projects( if output_format == "json": return response.model_dump(mode="json", exclude_none=True) - return _format_search_markdown(response, "all projects", query) + return _format_search_markdown(response, scope_label, query) @mcp.tool( @@ -817,6 +979,7 @@ async def search_notes( validation_alias=AliasChoices("search_all_projects", "all_projects"), ), ] = False, + projects: Optional[List[str]] = None, # `offset` is intentionally NOT aliased to `page`: offset is item-indexed # (skip N items) while page is 1-indexed page-number. Direct aliasing would # silently return the wrong slice. @@ -941,8 +1104,8 @@ async def search_notes( Project Resolution: Server resolves projects in this order: Single Project Mode → project parameter → default project. If project unknown, use list_memory_projects() or recent_activity() first. - Set search_all_projects=True to search every accessible project; this is opt-in because it - performs one search per project. + Set search_all_projects=True to search every accessible project, or pass projects=[...] to + search a chosen subset. Either runs one scoped query per database the projects live in. ## Search Syntax Examples @@ -1083,6 +1246,10 @@ async def search_notes( workspaces. Takes precedence over `project`. Get from list_memory_projects(). search_all_projects: Optional opt-in to search every accessible project. Ignored when `project` or `project_id` is supplied. + projects: Optional list of project names or external ids to search together. Names + are matched exactly as list_memory_projects() reports them (cloud projects + by their workspace-qualified name). Ignored when `project` or `project_id` + is supplied; an unknown name is an error rather than a silent skip. page: The page number of results to return (default 1). Aliases: page_number. page_size: The number of results to return per page (default 10). @@ -1236,6 +1403,7 @@ async def search_notes( # Outcome: comma-split/list normalization applies on every path; parse_str_list is # idempotent, so MCP-validated input passes through unchanged. note_types = parse_str_list(note_types) if note_types is not None else [] + projects = parse_str_list(projects) if projects is not None else None entity_types = parse_str_list(entity_types) if entity_types is not None else [] categories = parse_str_list(categories) if categories is not None else [] @@ -1290,7 +1458,7 @@ async def search_notes( # already provide a concrete project route. # Why: multi-project fan-out can be slow, so default search remains project-scoped. # Outcome: run one normal search per accessible project and merge ranked results. - if search_all_projects and project is None and project_id is None: + if (search_all_projects or projects) and project is None and project_id is None: all_projects_result = await _search_all_projects( query=query, page=page, @@ -1310,6 +1478,7 @@ async def search_notes( time_kind=time_kind, context=context, compact=compact, + projects=projects, ) return all_projects_result @@ -1365,104 +1534,24 @@ async def search_notes( effective_search_type = "permalink" try: - # Create a SearchQuery object based on the parameters - search_query = SearchQuery() - - # Only map search_type to query fields when there is an actual query string. - # When query is None/empty, skip the search mode block — filters-only path. + search_query = _build_search_query( + query=query, + search_type=effective_search_type, + note_types=note_types, + entity_types=entity_types, + categories=categories, + after_date=after_date, + metadata_filters=metadata_filters, + tags=tags, + status=status, + min_similarity=min_similarity, + valid_at=valid_at, + valid_overlaps=valid_overlaps, + time_kind=time_kind, + ) + if search_query is None: + return _NO_SEARCH_CRITERIA_MESSAGE effective_query = (query or "").strip() - if effective_query: - valid_search_types = { - "text", - "title", - "permalink", - "vector", - "semantic", - "hybrid", - } - if effective_search_type == "text": - search_query.text = effective_query - search_query.retrieval_mode = SearchRetrievalMode.FTS - elif effective_search_type in ("vector", "semantic"): - search_query.text = effective_query - search_query.retrieval_mode = SearchRetrievalMode.VECTOR - elif effective_search_type == "hybrid": - search_query.text = effective_query - search_query.retrieval_mode = SearchRetrievalMode.HYBRID - elif effective_search_type == "title": - search_query.title = effective_query - elif effective_search_type == "permalink" and "*" in effective_query: - search_query.permalink_match = effective_query - elif effective_search_type == "permalink": - search_query.permalink = effective_query - else: - raise ValueError( - f"Invalid search_type '{effective_search_type}'. " - f"Valid options: {', '.join(sorted(valid_search_types))}" - ) - - # Add optional filters if provided (empty lists are treated as no filter) - if entity_types: - search_query.entity_types = [SearchItemType(t) for t in entity_types] - if categories: - search_query.categories = categories - if note_types: - search_query.note_types = note_types - if after_date: - search_query.after_date = after_date - if metadata_filters: - # Alias common column/model names to their frontmatter key equivalents. - # Users often pass "note_type" (the entity model column) when the - # frontmatter field is actually "type". - _METADATA_KEY_ALIASES = {"note_type": "type"} - metadata_filters = { - _METADATA_KEY_ALIASES.get(k, k): v for k, v in metadata_filters.items() - } - search_query.metadata_filters = metadata_filters - if tags: - search_query.tags = tags - if status: - search_query.status = status - if min_similarity is not None: - search_query.min_similarity = min_similarity - # Presence, not truthiness, for the same reason as everywhere else on - # this path: these are assigned after construction, so the model's own - # blank guard never runs here, and a blank has already been refused above. - if valid_at is not None: - search_query.valid_at = valid_at - if valid_overlaps is not None: - search_query.valid_overlaps = valid_overlaps - if time_kind is not None: - search_query.time_kind = time_kind - - # Reject searches with no criteria at all - if search_query.no_criteria(): - return ( - "# No Search Criteria\n\n" - "Please provide at least one of: `query`, `metadata_filters`, " - "`tags`, `status`, `note_types`, `entity_types`, `categories`, " - "`after_date`, `valid_at`, `valid_overlaps`, or `time_kind`." - ) - - # Default to entity-level results to avoid returning individual - # observations/relations as separate search results (see issue #31). - # Applied after no_criteria() so that the implicit default doesn't - # mask a truly empty search request. - if not search_query.entity_types: - # Trigger: a category or valid-time filter was supplied without an - # explicit entity_types. - # Why: both only exist on observations — categories live on observation - # rows, and temporal assertions are projected against an - # observation's (type, id). Defaulting to "entity" would AND either - # filter against entity rows and return nothing, defeating the - # whole query. - # Outcome: scope the implicit default to observations so - # search_notes(categories=[...]) and search_notes(valid_at=...) - # return the matching bullets. - if search_query.categories or search_query.has_temporal_filter(): - search_query.entity_types = [SearchItemType("observation")] - else: - search_query.entity_types = [SearchItemType("entity")] logger.debug( f"Search request: project={active_project.name} " diff --git a/tests/mcp/test_tool_contracts.py b/tests/mcp/test_tool_contracts.py index 43b828995..f6693eae3 100644 --- a/tests/mcp/test_tool_contracts.py +++ b/tests/mcp/test_tool_contracts.py @@ -136,6 +136,7 @@ "project", "project_id", "search_all_projects", + "projects", "page", "page_size", "search_type", diff --git a/tests/mcp/test_tool_search_temporal.py b/tests/mcp/test_tool_search_temporal.py index c5a5b388d..45a380adb 100644 --- a/tests/mcp/test_tool_search_temporal.py +++ b/tests/mcp/test_tool_search_temporal.py @@ -11,12 +11,14 @@ """ import inspect +import sys from typing import Any +from unittest.mock import AsyncMock import pytest from basic_memory.mcp.tools import write_note -from basic_memory.mcp.tools.search import search_notes +from basic_memory.mcp.tools.search import SearchProjectRef, search_notes # The spec's worked example, verbatim: one note, two decisions, adjacent half-open # effective windows meeting at the July 27 cutover. @@ -474,17 +476,37 @@ async def test_all_projects_search_propagates_a_filter_no_project_could_apply( """ await _write_cache_layer_note(test_project.name) - import sys - # `basic_memory.mcp.tools.search` is shadowed by a `search` function exported # from the package, so reach the module itself rather than that name. search_module = sys.modules["basic_memory.mcp.tools.search"] - async def refuse_every_leg(*args: Any, **kwargs: Any) -> str: - # What a per-project leg looks like once SearchClient rejects the response. - return "# Search Failed\n\nThe search API did not apply the requested valid-time filter" + monkeypatch.setattr( + search_module, + "_load_search_project_refs", + AsyncMock( + return_value=[ + SearchProjectRef( + name=test_project.name, + external_id=test_project.external_id, + id=test_project.id, + workspace_tenant_id=None, + path=test_project.path, + ) + ] + ), + ) + + class RefusingScopedSearchClient: + # What a database looks like once the client rejects its response. + def __init__(self, client: Any) -> None: + pass + + async def search(self, *args: Any, **kwargs: Any) -> Any: + raise ValueError("The search API did not apply the requested valid-time filter") - monkeypatch.setattr(search_module, "search_notes", refuse_every_leg) + monkeypatch.setattr( + sys.modules["basic_memory.mcp.clients"], "ScopedSearchClient", RefusingScopedSearchClient + ) with pytest.raises(ValueError, match="No project applied the requested valid-time filter"): await search_module._search_all_projects( diff --git a/tests/mcp/tools/test_cjk_search_guidance.py b/tests/mcp/tools/test_cjk_search_guidance.py index 6507fd10b..3da88ee67 100644 --- a/tests/mcp/tools/test_cjk_search_guidance.py +++ b/tests/mcp/tools/test_cjk_search_guidance.py @@ -1,51 +1,87 @@ """Account search carries query guidance only for a complete, empty first page.""" import importlib +from contextlib import asynccontextmanager from typing import Literal from unittest.mock import AsyncMock import pytest +from basic_memory.mcp.tools.search import SearchProjectRef +from basic_memory.schemas.search import SearchItemType, SearchResponse, SearchResult + +ONE = SearchProjectRef( + name="one", + external_id="11111111-1111-1111-1111-111111111111", + id=1, + workspace_tenant_id=None, + path="/one", +) +TWO = SearchProjectRef( + name="two", + external_id="22222222-2222-2222-2222-222222222222", + id=2, + workspace_tenant_id="tenant-two", + path="/two", +) + @pytest.mark.asyncio @pytest.mark.parametrize("case", ["empty", "hit", "failed", "later"]) @pytest.mark.parametrize("output_format", ["text", "json"]) async def test_fanout_query_hint(monkeypatch, case: str, output_format: Literal["text", "json"]): search = importlib.import_module("basic_memory.mcp.tools.search") - monkeypatch.setattr( - search, - "_load_search_project_refs", - AsyncMock( - return_value=[ - {"project": "one", "project_id": None}, - {"project": "two", "project_id": None}, - ] - ), - ) - empty = { - "results": [], - "total": 0, - "total_is_exact": True, - "query_hint": "Try a shorter word from your query.", - } - second: dict[str, object] | str = empty - if case == "hit": - second = { - "results": [ - { - "title": "Match", - "type": "entity", - "score": 1.0, - "file_path": "note.md", - "permalink": "note", - } - ], - "total": 1, - "total_is_exact": True, - } - elif case == "failed": - second = "# Search Failed - Access Error" - monkeypatch.setattr(search, "search_notes", AsyncMock(side_effect=[empty, second])) + clients = importlib.import_module("basic_memory.mcp.clients") + monkeypatch.setattr(search, "_load_search_project_refs", AsyncMock(return_value=[ONE, TWO])) + monkeypatch.setattr(search, "project_index_required", AsyncMock(return_value=None)) + + @asynccontextmanager + async def fake_get_client(project_name=None, workspace=None): + yield {"workspace": workspace} + + def empty(page: int, page_size: int) -> SearchResponse: + return SearchResponse( + results=[], + current_page=page, + page_size=page_size, + total=0, + total_is_exact=True, + query_hint="Try a shorter word from your query.", + ) + + class StubScopedSearchClient: + def __init__(self, client): + self.workspace = client["workspace"] + + async def search(self, payload, *, project_ids, page, page_size): + # The first database (local) is always empty; the second database varies. + if self.workspace is None: + return empty(page, page_size) + if case == "hit": + return SearchResponse( + results=[ + SearchResult( + title="Match", + type=SearchItemType.ENTITY, + score=1.0, + file_path="note.md", + permalink="note", + project_id=TWO.id, + project_external_id=TWO.external_id, + ) + ], + current_page=page, + page_size=page_size, + total=1, + total_is_exact=True, + ) + if case == "failed": + raise RuntimeError("access denied") + return empty(page, page_size) + + monkeypatch.setattr(search, "get_client", fake_get_client) + monkeypatch.setattr(clients, "ScopedSearchClient", StubScopedSearchClient) + result = await search._search_all_projects( query="雾凇拼音", page=2 if case == "later" else 1, diff --git a/tests/mcp/tools/test_search_notes_multi_project.py b/tests/mcp/tools/test_search_notes_multi_project.py index 69a616cf0..27541c55c 100644 --- a/tests/mcp/tools/test_search_notes_multi_project.py +++ b/tests/mcp/tools/test_search_notes_multi_project.py @@ -1,116 +1,158 @@ -"""Tests for optional multi-project search_notes behavior.""" +"""Optional multi-project search_notes behavior: one scoped query per database.""" -from contextlib import asynccontextmanager import importlib +from contextlib import asynccontextmanager +from typing import Any from unittest.mock import AsyncMock -from httpx import HTTPStatusError, Request, Response -from fastmcp.exceptions import ToolError import pytest +from fastmcp.exceptions import ToolError +from httpx import HTTPStatusError, Request, Response +from basic_memory.mcp.tools.search import SearchProjectRef from basic_memory.schemas.search import SearchItemType, SearchResponse, SearchResult +PERSONAL = SearchProjectRef( + name="personal/main", + external_id="11111111-1111-1111-1111-111111111111", + id=11, + workspace_tenant_id="tenant-personal", + path="/personal/main", +) +TEAM = SearchProjectRef( + name="team-paul/main", + external_id="22222222-2222-2222-2222-222222222222", + id=22, + workspace_tenant_id="tenant-team", + path="/team/main", +) +ALPHA = SearchProjectRef( + name="alpha", + external_id="33333333-3333-3333-3333-333333333333", + id=3, + workspace_tenant_id=None, + path="/alpha", +) +BETA = SearchProjectRef( + name="beta", + external_id="44444444-4444-4444-4444-444444444444", + id=4, + workspace_tenant_id=None, + path="/beta", +) + + +def _result( + ref: SearchProjectRef, + *, + title: str, + score: float, + observation: bool = False, + permalink: str = "notes/example", + file_path: str = "/notes/example.md", +) -> SearchResult: + return SearchResult( + title=title, + permalink=permalink, + content="MCP content", + type=SearchItemType.OBSERVATION if observation else SearchItemType.ENTITY, + category="fact" if observation else None, + score=score, + file_path=file_path, + project_id=ref.id, + project_external_id=ref.external_id, + ) + -def _stub_routing_mode(monkeypatch, *, cloud: bool) -> None: - """Pin the three cloud-route signals search.py reads. +def _install_scoped_search( + monkeypatch: pytest.MonkeyPatch, + refs: list[SearchProjectRef], + answer, +) -> tuple[list[str | None], list[dict[str, Any]]]: + """Route the all-projects search into a stub that records each database's call. - `_search_all_projects` only forwards project_id (external UUID) when a - cloud route is available. The composite mirrors get_project_client: - factory mode OR explicit --cloud OR has_cloud_credentials. Tests stub - all three so a dev box with OAuth tokens on disk can't bleed into the - local-mode case. + ``answer(workspace, project_ids, page, page_size)`` returns the SearchResponse for + one database, or raises to model a failed database. """ + clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search") - monkeypatch.setattr(search_mod, "is_factory_mode", lambda: False) - monkeypatch.setattr(search_mod, "_explicit_routing", lambda: cloud) - monkeypatch.setattr(search_mod, "_force_local_mode", lambda: False) - monkeypatch.setattr(search_mod, "has_cloud_credentials", lambda config: cloud) + workspaces: list[str | None] = [] + calls: list[dict[str, Any]] = [] + async def fake_load_search_project_refs(context=None): + return refs -@pytest.fixture -def cloud_routing(monkeypatch): - """Force the cloud-routing path for multi-project search tests.""" - _stub_routing_mode(monkeypatch, cloud=True) + @asynccontextmanager + async def fake_get_client(project_name=None, workspace=None): + workspaces.append(workspace) + yield {"workspace": workspace} + + class StubScopedSearchClient: + def __init__(self, client): + self.workspace = client["workspace"] + + async def search(self, payload, *, project_ids, page, page_size): + calls.append( + { + "workspace": self.workspace, + "project_ids": list(project_ids), + "page": page, + "page_size": page_size, + "payload": payload, + } + ) + return answer(self.workspace, list(project_ids), page, page_size) + + monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) + monkeypatch.setattr(search_mod, "get_client", fake_get_client) + monkeypatch.setattr(clients_mod, "ScopedSearchClient", StubScopedSearchClient) + monkeypatch.setattr(search_mod, "project_index_required", AsyncMock(return_value=None)) + return workspaces, calls -@pytest.fixture -def local_routing(monkeypatch): - """Force the local-routing path for multi-project search tests.""" - _stub_routing_mode(monkeypatch, cloud=False) +def _page(results: list[SearchResult], page: int, page_size: int, **fields: Any) -> SearchResponse: + return SearchResponse( + results=results, + current_page=page, + page_size=page_size, + total=fields.pop("total", len(results)), + **fields, + ) @pytest.mark.asyncio @pytest.mark.parametrize("compact", [False, True]) @pytest.mark.parametrize("observations", [False, True]) -async def test_search_notes_search_all_projects_qualifies_result_permalinks( - monkeypatch, cloud_routing, compact, observations +async def test_each_workspace_is_one_query_and_results_stay_routable( + monkeypatch, compact, observations ): - """Multi-project search belongs to search_notes and keeps result ids routable.""" - clients_mod = importlib.import_module("basic_memory.mcp.clients") + """Two cloud workspaces are two databases: one scoped call each, merged by score.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") - project_refs = [ - { - "project": "personal/main", - "project_id": "11111111-1111-1111-1111-111111111111", - }, - { - "project": "team-paul/main", - "project_id": "22222222-2222-2222-2222-222222222222", - }, - ] - searched_projects: list[tuple[str | None, str | None]] = [] - - async def fake_load_search_project_refs(context=None): - return project_refs - - class StubProject: - def __init__(self, name: str | None, external_id: str | None): - self.name = name or "main" - self.external_id = external_id or "local-main" - - @asynccontextmanager - async def fake_get_project_client(project=None, context=None, project_id=None): - searched_projects.append((project, project_id)) - yield object(), StubProject(project, project_id) - - async def fake_resolve_project_and_path(client, identifier, project=None, context=None): - return StubProject(project, None), identifier, False - - class MockSearchClient: - def __init__(self, client, project_id): - self.project_id = project_id + def answer(workspace, project_ids, page, page_size): + ref, title, score = ( + (PERSONAL, "Personal MCP Test Note", 0.5) + if workspace == PERSONAL.workspace_tenant_id + else (TEAM, "Team MCP Test Note", 0.9) + ) + return _page( + [ + _result( + ref, + title=title, + score=score, + observation=observations, + permalink="main/tests/mcp-test-note", + file_path="tests/Exact Note.md" + if observations + else "/main/tests/mcp-test-note.md", + ) + ], + page, + page_size, + ) - async def search(self, payload, page, page_size): - if self.project_id == "11111111-1111-1111-1111-111111111111": - title = "Personal MCP Test Note" - score = 0.5 - else: - title = "Team MCP Test Note" - score = 0.9 - return SearchResponse( - results=[ - SearchResult( - title=title, - permalink="main/tests/mcp-test-note", - content="MCP content", - type=SearchItemType.OBSERVATION if observations else SearchItemType.ENTITY, - category="fact" if observations else None, - score=score, - file_path="tests/Exact Note.md" - if observations - else "/main/tests/mcp-test-note.md", - ) - ], - current_page=page, - page_size=page_size, - total=1, - ) - - monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) - monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) - monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) + workspaces, calls = _install_scoped_search(monkeypatch, [PERSONAL, TEAM], answer) result = await search_mod.search_notes( query="MCP Test Note", @@ -120,9 +162,10 @@ async def search(self, payload, page, page_size): ) assert isinstance(result, dict) - assert searched_projects == [ - ("personal/main", "11111111-1111-1111-1111-111111111111"), - ("team-paul/main", "22222222-2222-2222-2222-222222222222"), + assert workspaces == [PERSONAL.workspace_tenant_id, TEAM.workspace_tenant_id] + assert [(call["workspace"], call["project_ids"]) for call in calls] == [ + (PERSONAL.workspace_tenant_id, [PERSONAL.id]), + (TEAM.workspace_tenant_id, [TEAM.id]), ] path = "tests/Exact Note.md" if compact and observations else "tests/mcp-test-note" assert [item["permalink"] for item in result["results"]] == [ @@ -131,11 +174,101 @@ async def search(self, payload, page, page_size): ] if compact: assert all("content" not in item for item in result["results"]) + assert result["total"] == 2 + assert result["total_is_exact"] is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize("compact_observations", [False, True]) +async def test_local_projects_share_one_scoped_query(monkeypatch, compact_observations): + """Projects in the local database are one query, and every hit names its project.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + + def answer(workspace, project_ids, page, page_size): + return _page( + [ + _result( + ref, title=f"Note in {ref.name}", score=0.5, observation=compact_observations + ) + for ref in (ALPHA, BETA) + ], + page, + page_size, + ) + + workspaces, calls = _install_scoped_search(monkeypatch, [ALPHA, BETA], answer) + + result = await search_mod.search_notes( + query="anything", + search_all_projects=True, + output_format="json", + compact=compact_observations, + ) + + assert isinstance(result, dict) + assert workspaces == [None] + assert [(call["workspace"], call["project_ids"]) for call in calls] == [ + (None, [ALPHA.id, BETA.id]) + ] + assert result["total"] == 2 assert result["total_is_exact"] is True + if compact_observations: + assert [item["permalink"] for item in result["results"]] == [ + "alpha/notes/example.md", + "beta/notes/example.md", + ] + else: + assert [item["permalink"] for item in result["results"]] == [ + "notes/example", + "notes/example", + ] + assert [item["project_external_id"] for item in result["results"]] == [ + ALPHA.external_id, + BETA.external_id, + ] + + +@pytest.mark.asyncio +async def test_each_database_answers_the_whole_prefix_the_page_needs(monkeypatch): + """Page two of five asks every database for its top ten, then slices the merge.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + + def answer(workspace, project_ids, page, page_size): + ref = PERSONAL if workspace == PERSONAL.workspace_tenant_id else TEAM + # Distinct scores across databases so the merge order is unambiguous. + base = 0.9 if ref is TEAM else 0.85 + return _page( + [ + _result(ref, title=f"{ref.name} {index}", score=base - index * 0.1) + for index in range(6) + ], + page, + page_size, + total=6, + ) + + _workspaces, calls = _install_scoped_search(monkeypatch, [PERSONAL, TEAM], answer) + + result = await search_mod.search_notes( + query="notes", search_all_projects=True, output_format="json", page=2, page_size=5 + ) + + assert isinstance(result, dict) + assert [(call["page"], call["page_size"]) for call in calls] == [(1, 10), (1, 10)] + assert [item["title"] for item in result["results"]] == [ + "personal/main 2", + "team-paul/main 3", + "personal/main 3", + "team-paul/main 4", + "personal/main 4", + ] + assert result["current_page"] == 2 + assert result["total"] == 12 + assert result["has_more"] is True @pytest.mark.asyncio -async def test_search_notes_multi_project_search_is_opt_in(monkeypatch): +async def test_multi_project_search_is_opt_in(monkeypatch): """Default search_notes calls stay scoped to the resolved project.""" clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search") @@ -168,7 +301,6 @@ async def search(self, payload, page, page_size): monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) - monkeypatch.setattr(search_mod, "project_index_required", AsyncMock(return_value=None)) result = await search_mod.search_notes(query="MCP Test Note", output_format="json") @@ -179,9 +311,7 @@ async def search(self, payload, page, page_size): @pytest.mark.asyncio -async def test_search_notes_search_all_projects_with_no_refs_returns_empty_all_projects( - monkeypatch, -): +async def test_no_accessible_projects_is_an_empty_all_projects_answer(monkeypatch): """Explicit all-project search must not silently fall back to one project.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") @@ -189,12 +319,12 @@ async def fake_load_search_project_refs(context=None): return [] @asynccontextmanager - async def fail_get_project_client(*args, **kwargs): - raise AssertionError("search_all_projects=True should not fall back to scoped search") + async def fail_get_client(*args, **kwargs): + raise AssertionError("search_all_projects=True should not query without projects") yield monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fail_get_project_client) + monkeypatch.setattr(search_mod, "get_client", fail_get_client) result = await search_mod.search_notes( query="MCP Test Note", @@ -202,7 +332,6 @@ async def fail_get_project_client(*args, **kwargs): output_format="json", ) - assert isinstance(result, dict) assert result == { "results": [], "current_page": 1, @@ -214,40 +343,11 @@ async def fail_get_project_client(*args, **kwargs): @pytest.mark.asyncio -async def test_search_notes_search_all_projects_continues_after_project_failure( - monkeypatch, cloud_routing -): - """One failing project should not discard successful all-project search results.""" - clients_mod = importlib.import_module("basic_memory.mcp.clients") +async def test_a_failing_database_is_skipped_and_the_total_becomes_inexact(monkeypatch): + """One failing workspace should not discard the other's results.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") - - project_refs = [ - { - "project": "personal/main", - "project_id": "11111111-1111-1111-1111-111111111111", - }, - { - "project": "team-paul/main", - "project_id": "22222222-2222-2222-2222-222222222222", - }, - ] warnings: list[str] = [] - async def fake_load_search_project_refs(context=None): - return project_refs - - class StubProject: - def __init__(self, name: str | None, external_id: str | None): - self.name = name or "main" - self.external_id = external_id or "local-main" - - @asynccontextmanager - async def fake_get_project_client(project=None, context=None, project_id=None): - yield object(), StubProject(project, project_id) - - async def fake_resolve_project_and_path(client, identifier, project=None, context=None): - return StubProject(project, None), identifier, False - class FakeLogger: def debug(self, *args, **kwargs): pass @@ -258,34 +358,15 @@ def error(self, *args, **kwargs): def warning(self, message, *args, **kwargs): warnings.append(str(message)) - class MockSearchClient: - def __init__(self, client, project_id): - self.project_id = project_id - - async def search(self, payload, page, page_size): - if self.project_id == "22222222-2222-2222-2222-222222222222": - raise RuntimeError("team index unavailable") - return SearchResponse( - results=[ - SearchResult( - title="Personal MCP Test Note", - permalink="main/tests/mcp-test-note", - content="MCP content", - type=SearchItemType.ENTITY, - score=0.5, - file_path="/main/tests/mcp-test-note.md", - ) - ], - current_page=page, - page_size=page_size, - total=1, - ) + def answer(workspace, project_ids, page, page_size): + if workspace == TEAM.workspace_tenant_id: + raise RuntimeError("team index unavailable") + return _page( + [_result(PERSONAL, title="Personal MCP Test Note", score=0.5)], page, page_size + ) - monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) - monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) + _install_scoped_search(monkeypatch, [PERSONAL, TEAM], answer) monkeypatch.setattr(search_mod, "logger", FakeLogger()) - monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) result = await search_mod.search_notes( query="MCP Test Note", @@ -294,84 +375,50 @@ async def search(self, payload, page, page_size): ) assert isinstance(result, dict) - assert [item["permalink"] for item in result["results"]] == [ - "personal/main/tests/mcp-test-note", - ] + assert [item["permalink"] for item in result["results"]] == ["personal/main/notes/example"] assert result["total"] == 1 assert result["total_is_exact"] is False - assert any("team-paul/main" in warning for warning in warnings) + assert any("workspace team-paul" in warning for warning in warnings) assert any("team index unavailable" in warning for warning in warnings) @pytest.mark.asyncio -async def test_search_notes_search_all_projects_propagates_retryable_service_outage( - monkeypatch, cloud_routing -): - """A retryable project outage must fail the merged page instead of returning a partial one.""" - clients_mod = importlib.import_module("basic_memory.mcp.clients") +async def test_every_database_failing_is_a_failed_search_not_an_empty_one(monkeypatch): + """With one database, its failure is the whole answer; do not dress it as no matches.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") - project_refs = [ - { - "project": "personal/main", - "project_id": "11111111-1111-1111-1111-111111111111", - }, - { - "project": "team-paul/main", - "project_id": "22222222-2222-2222-2222-222222222222", - }, - ] - async def fake_load_search_project_refs(context=None): - return project_refs + def answer(workspace, project_ids, page, page_size): + raise RuntimeError("local index unavailable") - class StubProject: - def __init__(self, name: str | None, external_id: str | None): - self.name = name or "main" - self.external_id = external_id or "local-main" + _install_scoped_search(monkeypatch, [ALPHA, BETA], answer) - @asynccontextmanager - async def fake_get_project_client(project=None, context=None, project_id=None): - yield object(), StubProject(project, project_id) + result = await search_mod.search_notes( + query="MCP Test Note", search_all_projects=True, output_format="json" + ) - async def fake_resolve_project_and_path(client, identifier, project=None, context=None): - return StubProject(project, None), identifier, False + assert isinstance(result, str) + assert result.startswith("# Search Failed") + assert "local index unavailable" in result - class MockSearchClient: - def __init__(self, client, project_id): - self.project_id = project_id - async def search(self, payload, page, page_size): - if self.project_id == "22222222-2222-2222-2222-222222222222": - request = Request("POST", "https://api.example/search") - response = Response( - 503, - request=request, - json={"detail": "Reranker temporarily unavailable"}, - ) - try: - response.raise_for_status() - except HTTPStatusError as exc: - raise ToolError("Reranker temporarily unavailable") from exc - return SearchResponse( - results=[ - SearchResult( - title="Personal result", - permalink="main/personal-result", - content="MCP content", - type=SearchItemType.ENTITY, - score=0.5, - file_path="/main/personal-result.md", - ) - ], - current_page=page, - page_size=page_size, - total=1, +@pytest.mark.asyncio +async def test_a_retryable_service_outage_fails_the_merged_page(monkeypatch): + """A retryable outage must fail the merged page instead of returning a partial one.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + + def answer(workspace, project_ids, page, page_size): + if workspace == TEAM.workspace_tenant_id: + request = Request("QUERY", "https://api.example/v2/search/") + response = Response( + 503, request=request, json={"detail": "Reranker temporarily unavailable"} ) + try: + response.raise_for_status() + except HTTPStatusError as exc: + raise ToolError("Reranker temporarily unavailable") from exc + return _page([_result(PERSONAL, title="Personal result", score=0.5)], page, page_size) - monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) - monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) - monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) + _install_scoped_search(monkeypatch, [PERSONAL, TEAM], answer) result = await search_mod.search_notes( query="MCP Test Note", @@ -384,92 +431,118 @@ async def search(self, payload, page, page_size): @pytest.mark.asyncio -@pytest.mark.parametrize("compact_observations", [False, True]) -async def test_search_notes_search_all_projects_local_omits_project_id( - monkeypatch, local_routing, compact_observations -): - """Without a cloud route, fan-out must address each project by name only. - - project_id (external UUID) routes through the cloud v2 API path, which - returns 401 on local installs because there's no JWT to present. Local - fan-out has to fall back to the name-routed path so each per-project - search actually returns results instead of silently failing. - """ - clients_mod = importlib.import_module("basic_memory.mcp.clients") +async def test_an_empty_answer_checks_every_project_in_the_database_for_an_index(monkeypatch): + """An empty scoped page is only a miss once each project has been indexed.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") - project_refs = [ - { - "project": "alpha", - "project_id": "11111111-1111-1111-1111-111111111111", - }, - { - "project": "beta", - "project_id": "22222222-2222-2222-2222-222222222222", - }, - ] - searched_projects: list[tuple[str | None, str | None]] = [] + def answer(workspace, project_ids, page, page_size): + return _page([], page, page_size, total=0) - async def fake_load_search_project_refs(context=None): - return project_refs + _install_scoped_search(monkeypatch, [ALPHA, BETA], answer) + checked: list[str] = [] - class StubProject: - def __init__(self, name: str | None, external_id: str | None): - self.name = name or "main" - self.external_id = external_id or "local-main" + async def readiness(client, project): + checked.append(project.name) + return "# Project Index Required\n\nProject 'beta'" if project.name == "beta" else None - @asynccontextmanager - async def fake_get_project_client(project=None, context=None, project_id=None): - searched_projects.append((project, project_id)) - yield object(), StubProject(project, project_id) + monkeypatch.setattr(search_mod, "project_index_required", readiness) - async def fake_resolve_project_and_path(client, identifier, project=None, context=None): - return StubProject(project, None), identifier, False + result = await search_mod.search_notes( + query="missing", search_all_projects=True, output_format="text" + ) - class MockSearchClient: - def __init__(self, client, project_id): - self.project_id = project_id + assert isinstance(result, str) + assert result.startswith("# Project Index Required") + assert checked == ["alpha", "beta"] - async def search(self, payload, page, page_size): - return SearchResponse( - results=[ - SearchResult( - title=f"Note in {self.project_id or 'local'}", - permalink="notes/example", - content="", - type=SearchItemType.OBSERVATION - if compact_observations - else SearchItemType.ENTITY, - score=0.5, - file_path="/notes/example.md", - ) - ], - current_page=page, - page_size=page_size, - total=1, - ) - monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) - monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) - monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) +@pytest.mark.asyncio +async def test_projects_selects_a_subset_across_databases(monkeypatch): + """Naming projects searches only those, grouped by the database each lives in.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + + def answer(workspace, project_ids, page, page_size): + ref = TEAM if workspace == TEAM.workspace_tenant_id else BETA + return _page([_result(ref, title=ref.name, score=0.5)], page, page_size) + + _workspaces, calls = _install_scoped_search(monkeypatch, [PERSONAL, TEAM, ALPHA, BETA], answer) result = await search_mod.search_notes( - query="anything", - search_all_projects=True, - output_format="json", - compact=compact_observations, + query="notes", projects=["beta", TEAM.external_id], output_format="json" ) assert isinstance(result, dict) - assert searched_projects == [("alpha", None), ("beta", None)], ( - "Local fan-out must omit project_id so the recursive search_notes calls " - "take the name-routed path." + assert [(call["workspace"], call["project_ids"]) for call in calls] == [ + (None, [BETA.id]), + (TEAM.workspace_tenant_id, [TEAM.id]), + ] + assert {item["title"] for item in result["results"]} == {"beta", "team-paul/main"} + + +@pytest.mark.asyncio +async def test_an_unknown_project_name_is_refused_before_any_query(monkeypatch): + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + _workspaces, calls = _install_scoped_search( + monkeypatch, [ALPHA, BETA], lambda *args: pytest.fail("must not query") ) - assert result["total"] == 2 - assert result["total_is_exact"] is True - if compact_observations: - assert [item["permalink"] for item in result["results"]] == [ - "alpha/notes/example.md", - "beta/notes/example.md", - ] + + with pytest.raises(ValueError, match="Unknown project\\(s\\): gamma"): + await search_mod.search_notes(query="notes", projects=["gamma"], output_format="json") + + assert calls == [] + + +@pytest.mark.asyncio +async def test_a_hit_the_server_does_not_attribute_is_a_version_skew_error(monkeypatch): + """A result without project identity cannot be qualified; say so instead of guessing.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + + def answer(workspace, project_ids, page, page_size): + stray = _result(ALPHA, title="stray", score=0.5).model_copy( + update={"project_external_id": None} + ) + return _page([stray], page, page_size) + + _install_scoped_search(monkeypatch, [ALPHA, BETA], answer) + + # Like the valid-time skew above, this is a version mismatch, not a search miss: + # it is raised as an error rather than returned as an empty or partial page. + with pytest.raises(ValueError, match="did not attribute"): + await search_mod.search_notes(query="notes", search_all_projects=True, output_format="json") + + +def test_project_refs_need_an_id_an_external_id_and_a_name(): + """List rows a scoped search cannot address or attribute are left out.""" + search_mod = importlib.import_module("basic_memory.mcp.tools.search") + payload = { + "projects": [ + {"name": "alpha", "external_id": ALPHA.external_id, "id": 3, "path": "/alpha"}, + {"name": "no-id", "external_id": BETA.external_id}, + {"name": "no-external-id", "id": 5}, + {"name": "bool-id", "external_id": PERSONAL.external_id, "id": True}, + { + "name": "main", + "qualified_name": "team-paul/main", + "external_id": TEAM.external_id, + "id": 22, + "workspace_tenant_id": "tenant-team", + "path": "/team/main", + }, + ], + "constrained_project": None, + } + + refs = search_mod._search_project_refs(payload) + + assert refs == [ + SearchProjectRef( + name="alpha", + external_id=ALPHA.external_id, + id=3, + workspace_tenant_id=None, + path="/alpha", + ), + TEAM, + ] + assert search_mod._search_project_refs({"projects": "nope"}) == [] + assert search_mod._search_project_refs(None) == [] diff --git a/tests/mcp/tools/test_search_notes_multi_project_temporal.py b/tests/mcp/tools/test_search_notes_multi_project_temporal.py index 85fe04963..b0431d2b6 100644 --- a/tests/mcp/tools/test_search_notes_multi_project_temporal.py +++ b/tests/mcp/tools/test_search_notes_multi_project_temporal.py @@ -1,7 +1,7 @@ -"""All-projects search must carry the valid-time filter into every project (SPEC-82). +"""All-projects search must carry the valid-time filter into every database (SPEC-82). `_search_all_projects` re-declares the whole filter surface in its own signature and then -calls `search_notes` once per project. A filter that is not repeated there is dropped for +runs one scoped query per database. A filter that is not repeated there is dropped for every project at once, and the merged answer would quietly mix filtered and unfiltered rows -- the worst shape this failure can take, because the result still looks like an answer. @@ -10,52 +10,49 @@ import importlib from contextlib import asynccontextmanager from typing import Any +from unittest.mock import AsyncMock import pytest +from basic_memory.mcp.tools.search import SearchProjectRef from basic_memory.schemas.search import SearchItemType, SearchResponse, SearchResult PROJECT_REFS = [ - {"project": "personal/main", "project_id": "11111111-1111-1111-1111-111111111111"}, - {"project": "team-paul/main", "project_id": "22222222-2222-2222-2222-222222222222"}, + SearchProjectRef( + name="personal/main", + external_id="11111111-1111-1111-1111-111111111111", + id=11, + workspace_tenant_id="tenant-personal", + path="/personal/main", + ), + SearchProjectRef( + name="team-paul/main", + external_id="22222222-2222-2222-2222-222222222222", + id=22, + workspace_tenant_id="tenant-team", + path="/team/main", + ), ] -@pytest.fixture -def cloud_routing(monkeypatch): - """Pin the routing signals so project ids are forwarded deterministically.""" - search_mod = importlib.import_module("basic_memory.mcp.tools.search") - monkeypatch.setattr(search_mod, "is_factory_mode", lambda: False) - monkeypatch.setattr(search_mod, "_explicit_routing", lambda: True) - monkeypatch.setattr(search_mod, "_force_local_mode", lambda: False) - monkeypatch.setattr(search_mod, "has_cloud_credentials", lambda config: True) - - def _install_stub_client(monkeypatch, payloads: list[dict[str, Any]], refs) -> None: - """Route every per-project search into a stub that records its query payload.""" + """Route every database's scoped search into a stub that records its query payload.""" clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search") - - class StubProject: - def __init__(self, name: str | None, external_id: str | None): - self.name = name or "main" - self.external_id = external_id or "local-main" + refs_by_tenant = {ref.workspace_tenant_id: ref for ref in refs} @asynccontextmanager - async def fake_get_project_client(project=None, context=None, project_id=None): - yield object(), StubProject(project, project_id) - - async def fake_resolve_project_and_path(client, identifier, project=None, context=None): - return StubProject(project, None), identifier, False + async def fake_get_client(project_name=None, workspace=None): + yield {"workspace": workspace} async def fake_load_search_project_refs(context=None): return refs - class MockSearchClient: - def __init__(self, client, project_id): - self.project_id = project_id + class StubScopedSearchClient: + def __init__(self, client): + self.ref = refs_by_tenant[client["workspace"]] - async def search(self, payload, page, page_size): + async def search(self, payload, *, project_ids, page, page_size): payloads.append(payload) return SearchResponse( results=[ @@ -66,6 +63,8 @@ async def search(self, payload, page, page_size): type=SearchItemType.OBSERVATION, score=0.5, file_path="/main/decisions/cache-layer.md", + project_id=self.ref.id, + project_external_id=self.ref.external_id, ) ], current_page=page, @@ -75,14 +74,14 @@ async def search(self, payload, page, page_size): ) monkeypatch.setattr(search_mod, "_load_search_project_refs", fake_load_search_project_refs) - monkeypatch.setattr(search_mod, "get_project_client", fake_get_project_client) - monkeypatch.setattr(search_mod, "resolve_project_and_path", fake_resolve_project_and_path) - monkeypatch.setattr(clients_mod, "SearchClient", MockSearchClient) + monkeypatch.setattr(search_mod, "get_client", fake_get_client) + monkeypatch.setattr(clients_mod, "ScopedSearchClient", StubScopedSearchClient) + monkeypatch.setattr(search_mod, "project_index_required", AsyncMock(return_value=None)) @pytest.mark.asyncio -async def test_all_projects_search_forwards_the_valid_time_filter(monkeypatch, cloud_routing): - """Every project is asked the same valid-time question, not just the first.""" +async def test_all_projects_search_forwards_the_valid_time_filter(monkeypatch): + """Every database is asked the same valid-time question, not just the first.""" search_mod = importlib.import_module("basic_memory.mcp.tools.search") payloads: list[dict[str, Any]] = [] _install_stub_client(monkeypatch, payloads, PROJECT_REFS) @@ -101,12 +100,12 @@ async def test_all_projects_search_forwards_the_valid_time_filter(monkeypatch, c assert payload["valid_at"] == "2026-07-28" assert payload["time_kind"] == "effective" assert payload["valid_overlaps"] is None - # Every leg confirmed it ran the filter, so the merged answer confirms it too. + # Every database confirmed it ran the filter, so the merged answer confirms it too. assert result["temporal_applied"] is True @pytest.mark.asyncio -async def test_all_projects_search_forwards_an_overlap_filter(monkeypatch, cloud_routing): +async def test_all_projects_search_forwards_an_overlap_filter(monkeypatch): payloads: list[dict[str, Any]] = [] _install_stub_client(monkeypatch, payloads, PROJECT_REFS) search_mod = importlib.import_module("basic_memory.mcp.tools.search") @@ -125,7 +124,7 @@ async def test_all_projects_search_forwards_an_overlap_filter(monkeypatch, cloud @pytest.mark.asyncio -async def test_all_projects_search_without_a_filter_claims_nothing(monkeypatch, cloud_routing): +async def test_all_projects_search_without_a_filter_claims_nothing(monkeypatch): """An ordinary all-projects search stays exactly the payload it always was.""" payloads: list[dict[str, Any]] = [] _install_stub_client(monkeypatch, payloads, PROJECT_REFS) @@ -150,9 +149,8 @@ async def test_all_projects_search_without_a_filter_claims_nothing(monkeypatch, ], ) @pytest.mark.asyncio -async def test_a_malformed_filter_is_refused_before_any_project_is_searched( +async def test_a_malformed_filter_is_refused_before_any_database_is_searched( monkeypatch, - cloud_routing, valid_at: str | None, valid_overlaps: str | None, time_kind: str | None, @@ -160,12 +158,10 @@ async def test_a_malformed_filter_is_refused_before_any_project_is_searched( ): """A typo must read as an error, never as an all-projects search with no matches. - Each per-project leg turns the API's 400 into a `# Search Failed` string, which the - fan-out cannot tell from a project being unavailable: it logs it and skips on. With - every project skipped the merged answer is an empty success that still reports - `temporal_applied`, so a mistyped filter would come back as the plausible-looking - "no matches found" for a question that never ran anywhere. Client-side validation is - the only layer that can tell the two apart, so it runs once, before the fan-out. + A database that refuses the filter is logged and skipped, so with every database + skipped the merged answer would be an empty success that still reports + `temporal_applied`. Client-side validation is the only layer that can tell a typo + from an unavailable database, so it runs once, before any query. """ payloads: list[dict[str, Any]] = [] _install_stub_client(monkeypatch, payloads, PROJECT_REFS) @@ -185,9 +181,7 @@ async def test_a_malformed_filter_is_refused_before_any_project_is_searched( @pytest.mark.asyncio -async def test_all_projects_search_with_no_projects_still_confirms_the_filter( - monkeypatch, cloud_routing -): +async def test_all_projects_search_with_no_projects_still_confirms_the_filter(monkeypatch): """Zero projects is an empty answer to the valid-time question, not an unfiltered one.""" payloads: list[dict[str, Any]] = [] _install_stub_client(monkeypatch, payloads, [])