diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py index 3c67f3ac..e9ddb081 100644 --- a/src/basic_memory/mcp/tools/search.py +++ b/src/basic_memory/mcp/tools/search.py @@ -10,13 +10,8 @@ from loguru import logger 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, coerce_list -from basic_memory.mcp.async_client import ( - _explicit_routing, - _force_local_mode, - is_factory_mode, -) from basic_memory.mcp.container import get_container from basic_memory.mcp.project_context import ( detect_project_from_identifier_prefix, @@ -485,28 +480,21 @@ async def _search_all_projects( any_project_has_more = False # 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) - ) - + # Why: forwarding project_id (external UUID) routes the per-project search + # through get_project_client's UUID branch, which treats unknown + # identifiers as cloud and 401s when local OAuth tokens linger from a + # past `bm cloud login` even though the project itself is local. + # Outcome: route by `project` name (qualified_name like "workspace/project" + # in cloud refs, plain name in local refs) — both backends resolve + # that without UUID lookup. project_id is only used as a fallback when + # a ref unexpectedly has no name to route by. for project_ref in project_refs: - recursive_project_id = project_ref["project_id"] if use_cloud_routing else None + project_name = project_ref["project"] + recursive_project_id = None if project_name else project_ref["project_id"] try: results = await search_notes( query=query, - project=project_ref["project"], + project=project_name, project_id=recursive_project_id, page=1, page_size=per_project_page_size, diff --git a/tests/mcp/tools/test_search_notes_multi_project.py b/tests/mcp/tools/test_search_notes_multi_project.py index bc962aa6..0da07c47 100644 --- a/tests/mcp/tools/test_search_notes_multi_project.py +++ b/tests/mcp/tools/test_search_notes_multi_project.py @@ -8,38 +8,8 @@ import pytest from basic_memory.schemas.search import SearchItemType, SearchResponse, SearchResult -def _stub_routing_mode(monkeypatch, *, cloud: bool) -> None: - """Pin the three cloud-route signals search.py reads. - - `_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. - """ - 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) - - -@pytest.fixture -def cloud_routing(monkeypatch): - """Force the cloud-routing path for multi-project search tests.""" - _stub_routing_mode(monkeypatch, cloud=True) - - -@pytest.fixture -def local_routing(monkeypatch): - """Force the local-routing path for multi-project search tests.""" - _stub_routing_mode(monkeypatch, cloud=False) - - @pytest.mark.asyncio -async def test_search_notes_search_all_projects_qualifies_result_permalinks( - monkeypatch, cloud_routing -): +async def test_search_notes_search_all_projects_qualifies_result_permalinks(monkeypatch): """Multi-project search belongs to search_notes and keeps result ids routable.""" clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search") @@ -62,7 +32,10 @@ async def test_search_notes_search_all_projects_qualifies_result_permalinks( 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" + # When the fan-out routes by name only, project_id is None — keep + # the test stable by falling back to the name so the downstream + # SearchClient still has a stable per-project identifier. + self.external_id = external_id or self.name @asynccontextmanager async def fake_get_project_client(project=None, context=None, project_id=None): @@ -74,10 +47,12 @@ async def test_search_notes_search_all_projects_qualifies_result_permalinks( class MockSearchClient: def __init__(self, client, project_id): + # The fake project supplies the workspace-qualified name as the + # external_id, so each per-project search keys off that. self.project_id = project_id async def search(self, payload, page, page_size): - if self.project_id == "11111111-1111-1111-1111-111111111111": + if self.project_id == "personal/main": title = "Personal MCP Test Note" score = 0.5 else: @@ -111,9 +86,13 @@ async def test_search_notes_search_all_projects_qualifies_result_permalinks( ) assert isinstance(result, dict) + # Fan-out now routes by qualified_name only; project_id is omitted because + # the workspace-qualified name is already unambiguous and UUID routing + # hits get_project_client's cloud branch even for local projects when + # local OAuth credentials are present. assert searched_projects == [ - ("personal/main", "11111111-1111-1111-1111-111111111111"), - ("team-paul/main", "22222222-2222-2222-2222-222222222222"), + ("personal/main", None), + ("team-paul/main", None), ] assert [item["permalink"] for item in result["results"]] == [ "team-paul/main/tests/mcp-test-note", @@ -198,9 +177,7 @@ async def test_search_notes_search_all_projects_with_no_refs_returns_empty_all_p @pytest.mark.asyncio -async def test_search_notes_search_all_projects_continues_after_project_failure( - monkeypatch, cloud_routing -): +async def test_search_notes_search_all_projects_continues_after_project_failure(monkeypatch): """One failing project should not discard successful all-project search results.""" clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search") @@ -223,7 +200,7 @@ async def test_search_notes_search_all_projects_continues_after_project_failure( 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" + self.external_id = external_id or self.name @asynccontextmanager async def fake_get_project_client(project=None, context=None, project_id=None): @@ -244,10 +221,12 @@ async def test_search_notes_search_all_projects_continues_after_project_failure( class MockSearchClient: def __init__(self, client, project_id): + # Fan-out routes by name now, so the stub project reflects the + # workspace-qualified name back as the SearchClient's project_id. self.project_id = project_id async def search(self, payload, page, page_size): - if self.project_id == "22222222-2222-2222-2222-222222222222": + if self.project_id == "team-paul/main": raise RuntimeError("team index unavailable") return SearchResponse( results=[ @@ -287,15 +266,15 @@ async def test_search_notes_search_all_projects_continues_after_project_failure( @pytest.mark.asyncio -async def test_search_notes_search_all_projects_local_omits_project_id( - monkeypatch, local_routing -): - """Without a cloud route, fan-out must address each project by name only. +async def test_search_notes_search_all_projects_omits_project_id(monkeypatch): + """Fan-out must address each project by name, never by external UUID. - 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. + project_id routes through get_project_client's UUID branch, which treats + any unknown identifier as cloud — so when a local install still has OAuth + tokens from a past `bm cloud login`, every per-project recursive call 401s + and the merged result list silently stays empty. Routing by name avoids + that path on both backends; cloud refs disambiguate via the + workspace/project qualified_name already baked into project_ref["project"]. """ clients_mod = importlib.import_module("basic_memory.mcp.clients") search_mod = importlib.import_module("basic_memory.mcp.tools.search")