diff --git a/src/basic_memory/api/v2/utils.py b/src/basic_memory/api/v2/utils.py index db96b8b1..cd0ccd74 100644 --- a/src/basic_memory/api/v2/utils.py +++ b/src/basic_memory/api/v2/utils.py @@ -147,6 +147,7 @@ def to_summary( from_entity_id=item.from_id, from_entity_external_id=from_ext_id, to_entity=to_title, + to_name=item.to_name, to_entity_id=item.to_id, to_entity_external_id=to_ext_id, created_at=item.created_at, diff --git a/src/basic_memory/cli/app.py b/src/basic_memory/cli/app.py index 13ef9aff..ad38cd7a 100644 --- a/src/basic_memory/cli/app.py +++ b/src/basic_memory/cli/app.py @@ -83,8 +83,11 @@ def _post_command_messages() -> None: # Skip for 'mcp' command - it has its own lifespan that handles initialization # Skip for API-using commands (status, sync, etc.) - they handle initialization via deps.py # Skip for 'reset' command - it manages its own database lifecycle + # Skip for 'man' - it only copies packaged files; a broken local database + # must not block installing the offline docs skip_init_commands = { "doctor", + "man", "mcp", "status", "sync", diff --git a/src/basic_memory/cli/commands/status.py b/src/basic_memory/cli/commands/status.py index 6f4317db..773218d2 100644 --- a/src/basic_memory/cli/commands/status.py +++ b/src/basic_memory/cli/commands/status.py @@ -190,9 +190,16 @@ async def run_status( if sync_report.total == 0: return project_item.name, sync_report if time.monotonic() >= deadline: + # Why the hint: indexing is done by the sync coordinator, which + # only runs inside a live server (bm mcp / hosted API). In a + # CLI-only session nothing will ever drain the pending count, + # so this wait cannot succeed — point at the command that + # actually indexes (#959). raise StatusTimeout( f"Timed out after {timeout:g}s waiting for '{project_item.name}' " - f"to finish indexing ({sync_report.total} pending change(s) remaining)." + f"to finish indexing ({sync_report.total} pending change(s) remaining). " + f"If no Basic Memory server is running, pending changes are never " + f"indexed — run 'bm reindex --project {project_item.name}' instead." ) await asyncio.sleep(poll_interval) diff --git a/src/basic_memory/mcp/async_client.py b/src/basic_memory/mcp/async_client.py index cf957e89..5b959a5a 100644 --- a/src/basic_memory/mcp/async_client.py +++ b/src/basic_memory/mcp/async_client.py @@ -10,7 +10,7 @@ from loguru import logger import logfire -from basic_memory.config import ConfigManager, ProjectMode +from basic_memory.config import ConfigManager, ProjectMode, has_cloud_credentials if TYPE_CHECKING: from sqlalchemy.ext.asyncio import AsyncEngine, AsyncSession, async_sessionmaker @@ -368,7 +368,8 @@ async def get_client( 1. Factory injection. 2. Explicit routing flags (--local/--cloud). 3. Per-project mode routing when project_name is provided. - 4. Local ASGI transport by default. + 4. Cloud routing when a workspace selector is provided. + 5. Local ASGI transport by default. """ if _client_factory: async with _client_factory(workspace=workspace) as client: @@ -428,6 +429,26 @@ async def get_client( yield client return + # --- Workspace-selector routing --- + # Trigger: caller passed a cloud workspace selector and nothing above routed. + # Why: a workspace names a cloud tenant — silently serving the request from + # the local ASGI app sent writes to the wrong destination (#954: a cloud + # project create either failed on the cloud-style path or landed locally). + # Outcome: route to the cloud proxy when credentials exist; without + # credentials fail fast instead of pretending the operation succeeded. + if workspace is not None: + if not has_cloud_credentials(config): + raise RuntimeError( + f"A cloud workspace was requested ('{workspace}') but no cloud " + "credentials were found. Run 'bm cloud login' or " + "'bm cloud set-key ' first, or omit the workspace selector " + "for a local operation." + ) + logger.debug(f"Workspace selector '{workspace}' provided - using cloud proxy client") + async with _cloud_client(config, timeout, workspace=workspace) as client: + yield client + return + # --- Default fallback --- logger.debug("Default routing - using ASGI client for local Basic Memory API") async with _asgi_client(timeout) as client: diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 212b6048..f47bd3ef 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -54,6 +54,14 @@ _WORKSPACE_PROJECT_INDEX_STATE_KEY = "workspace_project_index" +class WorkspaceProjectLookupMiss(ValueError): + """A project was absent from the workspace index (as opposed to ambiguous). + + Misses are retried once against a freshly rebuilt index, because the + session cache may simply predate an out-of-band project creation (#956). + """ + + @dataclass(frozen=True) class WorkspaceProjectEntry: """A cloud project resolved together with the workspace that owns it.""" @@ -460,6 +468,17 @@ def _canonical_memory_path_for_workspace( # Outcome: lookups preserve the complete workspace/project canonical permalink. if not normalized_remainder: normalized_remainder = project_permalink + + # Same index-form rule as _canonical_memory_path_for_active_route (#957): + # without an active workspace permalink context, stored permalinks are + # project-qualified and a workspace-prefixed pattern cannot match. + if "*" in normalized_remainder and current_workspace_permalink_context() is None: + return build_qualified_permalink_reference( + project_permalink, + normalized_remainder, + include_project=True, + ) + return build_qualified_permalink_reference( project_permalink, normalized_remainder, @@ -477,6 +496,24 @@ def _canonical_memory_path_for_active_route( ) -> str: """Return the canonical permalink path for the currently routed project/workspace.""" project_prefix = active_project.permalink + + # Trigger: the path contains a glob wildcard (folder/*) and no server-side + # workspace permalink context is active. + # Why: patterns match raw against the search index, so they must mirror the + # stored permalink form. The contextvar is what qualified permalinks at + # write time — when it is absent, stored rows are project-qualified and a + # workspace prefix (from the client's cached_workspace display state) + # guarantees zero matches (#957). Direct lookups keep full qualification + # because the link resolver understands it; patterns have no fallback. + # Outcome: without the contextvar, qualify patterns with the project prefix + # only; with it, fall through to normal workspace canonicalization. + if "*" in path and current_workspace_permalink_context() is None: + if not include_project: + return path + if path == project_prefix or path.startswith(f"{project_prefix}/"): + return path + return f"{project_prefix}/{path}" + workspace_remainder = path if include_project and (path == project_prefix or path.startswith(f"{project_prefix}/")): # Trigger: the memory URL already names the active project root/prefix @@ -722,9 +759,15 @@ async def _fetch_workspace_project_entries( async def _ensure_workspace_project_index( context: Optional[Context] = None, + *, + force_refresh: bool = False, ) -> WorkspaceProjectIndex: - """Build or load the session-local workspace/project lookup index.""" - if context: + """Build or load the session-local workspace/project lookup index. + + force_refresh bypasses the cached index and rebuilds from discovery — + used by resolve_workspace_project_identifier when a lookup misses (#956). + """ + if context and not force_refresh: cached_raw = await context.get_state(_WORKSPACE_PROJECT_INDEX_STATE_KEY) cached_index = _workspace_project_index_from_state(cached_raw) if cached_index is not None: @@ -865,7 +908,32 @@ async def resolve_workspace_project_identifier( ) -> WorkspaceProjectEntry: """Resolve a project by external_id (UUID), qualified name, or unqualified name.""" index = await _ensure_workspace_project_index(context=context) + try: + return await _resolve_workspace_project_from_index(index, project, context) + except WorkspaceProjectLookupMiss: + # Trigger: the lookup missed the session-cached index. + # Why: a miss is exactly the signal the cache may be stale — projects + # created out-of-band (CLI, a teammate in a shared workspace) post-date + # the index built at session start (#956). + # Outcome: rebuild the index once and retry; a second miss is authoritative + # and its error (with the refreshed project list) propagates. + logger.info( + f"Workspace project lookup missed for '{project}'; refreshing index and retrying" + ) + refreshed = await _ensure_workspace_project_index(context=context, force_refresh=True) + return await _resolve_workspace_project_from_index(refreshed, project, context) + + +async def _resolve_workspace_project_from_index( + index: WorkspaceProjectIndex, + project: str, + context: Optional[Context] = None, +) -> WorkspaceProjectEntry: + """Resolve a project against one concrete index snapshot. + Raises WorkspaceProjectLookupMiss for absent projects (retryable via index + refresh) and plain ValueError for ambiguity, which a refresh cannot fix. + """ # Fast path: direct lookup by external_id when the identifier is a UUID # Canonicalize via str(UUID(...)) so uppercase, brace-wrapped, or urn:uuid forms # all hash to the same lowercase-hyphenated key as the stored external_ids. @@ -895,7 +963,7 @@ async def resolve_workspace_project_identifier( failed_workspace.tenant_id == workspace.tenant_id for failed_workspace in index.failed_workspaces ): - raise ValueError( + raise WorkspaceProjectLookupMiss( f"Projects for workspace '{workspace.name}' ({workspace.slug}) " "could not be loaded. Retry after workspace discovery recovers." ) @@ -904,7 +972,7 @@ async def resolve_workspace_project_identifier( for entry in index.entries if entry.workspace.tenant_id == workspace.tenant_id ) - raise ValueError( + raise WorkspaceProjectLookupMiss( f"Project '{project_identifier}' was not found in workspace " f"'{workspace.name}' ({workspace.slug}). Available projects: {available}" ) @@ -929,7 +997,7 @@ async def resolve_workspace_project_identifier( "retry or use a qualified project from an indexed workspace." ) available = ", ".join(entry.qualified_name for entry in index.entries) - raise ValueError( + raise WorkspaceProjectLookupMiss( f"Project '{project}' was not found in indexed cloud workspaces. " f"Available projects: {available}.{failed_note}" ) diff --git a/src/basic_memory/mcp/tools/build_context.py b/src/basic_memory/mcp/tools/build_context.py index ad9bc213..e6c3da0b 100644 --- a/src/basic_memory/mcp/tools/build_context.py +++ b/src/basic_memory/mcp/tools/build_context.py @@ -54,7 +54,10 @@ def _format_entity_block(result: ContextResult) -> str: lines.append("") lines.append("### Relations") for rel in relation_items: - lines.append(f"- {rel.relation_type} [[{rel.to_entity}]]") + # Unresolved forward references have no resolved entity yet; fall back + # to the literal target text instead of rendering [[None]] (#955) + target = rel.to_entity or rel.to_name + lines.append(f"- {rel.relation_type} [[{target}]]") # --- Related entities (non-relation related results) --- related_entities: list[EntitySummary | ObservationSummary] = [ diff --git a/src/basic_memory/mcp/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py index cd022906..fd14d565 100644 --- a/src/basic_memory/mcp/tools/project_management.py +++ b/src/basic_memory/mcp/tools/project_management.py @@ -477,10 +477,14 @@ async def _resolve_workspace_routing( if workspace is None: return None - explicit_cloud_routing = _explicit_routing() and not _force_local_mode() + forced_local = _explicit_routing() and _force_local_mode() config = ConfigManager().config + # Resolve whenever credentials make workspace discovery possible — not only + # under explicit --cloud. A workspace selector implies cloud routing + # (get_client routes it to the cloud proxy, #954), and the transport needs + # the tenant id in X-Workspace-ID, not a slug or display name. should_resolve_workspace = is_factory_mode() or ( - explicit_cloud_routing and has_cloud_credentials(config) + has_cloud_credentials(config) and not forced_local ) if not should_resolve_workspace: return workspace @@ -521,8 +525,9 @@ async def create_memory_project( workspace: Optional cloud workspace selector to create the project in. Slug is preferred for AI callers, but tenant_id and unique name are also accepted. When omitted, the connection's default workspace is used. Discover values - via `list_workspaces`. In local mode the selector is passed through - without slug resolution. + via `list_workspaces`. A workspace selector implies cloud routing: + without cloud credentials the call fails fast instead of silently + creating a local project (#954). output_format: "text" returns the existing human-readable result text. "json" returns structured project creation metadata. context: Optional FastMCP context for progress/status logging. @@ -660,8 +665,9 @@ async def delete_project( workspace: Optional cloud workspace selector to delete the project from. Slug is preferred for AI callers, but tenant_id and unique name are also accepted. When omitted, the connection's default workspace is - used. In local mode the selector is passed through without slug - resolution, matching create_memory_project behavior. + used. A workspace selector implies cloud routing: without cloud + credentials the call fails fast, matching create_memory_project + behavior (#954). Returns: Confirmation message about project deletion diff --git a/src/basic_memory/repository/search_index_row.py b/src/basic_memory/repository/search_index_row.py index a883b720..9ffe6a73 100644 --- a/src/basic_memory/repository/search_index_row.py +++ b/src/basic_memory/repository/search_index_row.py @@ -36,6 +36,7 @@ class SearchIndexRow: category: Optional[str] = None # observations from_id: Optional[int] = None # relations to_id: Optional[int] = None # relations + to_name: Optional[str] = None # relations: literal target text, set even when unresolved relation_type: Optional[str] = None # relations # Matched chunk text from vector search (the actual content that matched the query) @@ -94,6 +95,7 @@ def to_insert(self, serialize_json: bool = True): else self.metadata, "from_id": self.from_id, "to_id": self.to_id, + "to_name": self.to_name, "relation_type": self.relation_type, "entity_id": self.entity_id, "category": self.category, diff --git a/src/basic_memory/schemas/memory.py b/src/basic_memory/schemas/memory.py index a0af709c..838121fc 100644 --- a/src/basic_memory/schemas/memory.py +++ b/src/basic_memory/schemas/memory.py @@ -155,6 +155,9 @@ class RelationSummary(BaseModel): from_entity_id: Optional[int] = None from_entity_external_id: Optional[str] = None to_entity: Optional[str] = None + # Literal target text from the markdown; present even when the relation is + # an unresolved forward reference (to_entity is None until the target exists) + to_name: Optional[str] = None to_entity_id: Optional[int] = None to_entity_external_id: Optional[str] = None created_at: Annotated[ diff --git a/src/basic_memory/services/context_service.py b/src/basic_memory/services/context_service.py index a27b595f..c1bb60b0 100644 --- a/src/basic_memory/services/context_service.py +++ b/src/basic_memory/services/context_service.py @@ -18,6 +18,7 @@ from basic_memory.schemas.memory import MemoryUrl, memory_url_path from basic_memory.schemas.search import SearchItemType from basic_memory.utils import generate_permalink +from basic_memory.workspace_context import workspace_slug_for_canonical_permalinks if TYPE_CHECKING: from basic_memory.services.link_resolver import LinkResolver @@ -36,6 +37,7 @@ class ContextResultRow: from_id: Optional[int] = None to_id: Optional[int] = None relation_type: Optional[str] = None + to_name: Optional[str] = None content: Optional[str] = None category: Optional[str] = None entity_id: Optional[int] = None @@ -143,6 +145,29 @@ async def build_context( primary = await self.search_repository.search( permalink_match=normalized_path, limit=fetch_limit, offset=offset ) + + # Trigger: a workspace-qualified pattern matched nothing while a + # workspace permalink context is active. + # Why: rows written before workspace canonicalization (or via + # clients that didn't forward workspace headers) store + # project-qualified permalinks; a workspace-prefixed pattern + # can never match those legacy rows (#957). + # Outcome: retry once with the workspace prefix stripped so the + # pattern matches the index form the rows actually carry. + if not primary: + workspace_slug = workspace_slug_for_canonical_permalinks() + ws_prefix = f"{workspace_slug}/" if workspace_slug else None + if ws_prefix and normalized_path.startswith(ws_prefix): + fallback_path = normalized_path.removeprefix(ws_prefix) + logger.debug( + f"Pattern search fallback without workspace prefix: " + f"'{fallback_path}'" + ) + primary = await self.search_repository.search( + permalink_match=fallback_path, + limit=fetch_limit, + offset=offset, + ) else: normalized_path = generate_permalink(path, split_extension=False) logger.debug(f"Direct lookup for '{normalized_path}'") @@ -392,6 +417,7 @@ async def find_related( from_id=row.from_id, to_id=row.to_id, relation_type=row.relation_type, + to_name=row.to_name, content=row.content, category=row.category, entity_id=row.entity_id, @@ -426,6 +452,7 @@ def _build_postgres_query( # pragma: no cover CAST(NULL AS INTEGER) as from_id, CAST(NULL AS INTEGER) as to_id, CAST(NULL AS TEXT) as relation_type, + CAST(NULL AS TEXT) as to_name, CAST(NULL AS TEXT) as content, CAST(NULL AS TEXT) as category, CAST(NULL AS INTEGER) as entity_id, @@ -478,6 +505,10 @@ def _build_postgres_query( # pragma: no cover WHEN step_type = 1 THEN r.relation_type ELSE NULL END as relation_type, + CASE + WHEN step_type = 1 THEN r.to_name + ELSE NULL + END as to_name, CAST(NULL AS TEXT) as content, CAST(NULL AS TEXT) as category, CAST(NULL AS INTEGER) as entity_id, @@ -541,6 +572,7 @@ def _build_postgres_query( # pragma: no cover from_id, to_id, relation_type, + to_name, content, category, entity_id, @@ -550,7 +582,7 @@ def _build_postgres_query( # pragma: no cover FROM entity_graph WHERE depth > 0 GROUP BY type, id, title, permalink, file_path, from_id, to_id, - relation_type, content, category, entity_id, root_id, created_at + relation_type, to_name, content, category, entity_id, root_id, created_at ORDER BY depth, type, id LIMIT :max_results """) @@ -579,6 +611,7 @@ def _build_sqlite_query( NULL as from_id, NULL as to_id, NULL as relation_type, + NULL as to_name, NULL as content, NULL as category, NULL as entity_id, @@ -606,6 +639,7 @@ def _build_sqlite_query( r.from_id, r.to_id, r.relation_type, + r.to_name, NULL as content, NULL as category, NULL as entity_id, @@ -644,6 +678,7 @@ def _build_sqlite_query( NULL as from_id, NULL as to_id, NULL as relation_type, + NULL as to_name, NULL as content, NULL as category, NULL as entity_id, @@ -677,6 +712,7 @@ def _build_sqlite_query( from_id, to_id, relation_type, + to_name, content, category, entity_id, @@ -686,7 +722,7 @@ def _build_sqlite_query( FROM entity_graph WHERE depth > 0 GROUP BY type, id, title, permalink, file_path, from_id, to_id, - relation_type, content, category, entity_id, root_id, created_at + relation_type, to_name, content, category, entity_id, root_id, created_at ORDER BY depth, type, id LIMIT :max_results """) diff --git a/test-int/mcp/test_project_management_integration.py b/test-int/mcp/test_project_management_integration.py index b663ab61..46bb29ef 100644 --- a/test-int/mcp/test_project_management_integration.py +++ b/test-int/mcp/test_project_management_integration.py @@ -591,36 +591,35 @@ async def test_nested_project_paths_rejected(mcp_server, app, test_project, tmp_ @pytest.mark.asyncio -async def test_create_project_accepts_workspace_in_local_mode( +async def test_create_project_workspace_without_credentials_fails_fast( mcp_server, app, test_project, tmp_path ): - """Passing workspace via the MCP wire is accepted by the tool schema and - does not break the local create path. + """A workspace selector without cloud credentials fails fast — no local create (#954). - In local mode there is no cloud factory installed, so workspace is a no-op: - the request lands on the ASGI transport which has no workspace concept. This - test guards the schema so a future change can't accidentally drop the parameter. + The previous contract treated workspace as a local no-op, which was the bug: + a caller asking for a cloud team workspace got a silent local project. The + tool schema still accepts the parameter (this test exercises it over the + MCP wire), but the create must error with auth guidance and leave no + project behind. """ + from fastmcp.exceptions import ToolError async with Client(mcp_server) as client: - create_result = await client.call_tool( - "create_memory_project", - { - "project_name": "ws-local-test", - "project_path": str( - tmp_path.parent / (tmp_path.name + "-projects") / "project-ws-local-test" - ), - "workspace": "team-paul", - }, - ) - - assert len(create_result.content) == 1 - create_text = create_result.content[0].text # pyright: ignore [reportAttributeAccessIssue] - assert "✓" in create_text - assert "ws-local-test" in create_text + with pytest.raises(ToolError, match="cloud workspace was requested"): + await client.call_tool( + "create_memory_project", + { + "project_name": "ws-local-test", + "project_path": str( + tmp_path.parent / (tmp_path.name + "-projects") / "project-ws-local-test" + ), + "workspace": "team-paul", + }, + ) + # The failed create must not leave a local project behind list_result = await client.call_tool("list_memory_projects", {}) - assert "ws-local-test" in list_result.content[0].text # pyright: ignore [reportAttributeAccessIssue] + assert "ws-local-test" not in list_result.content[0].text # pyright: ignore [reportAttributeAccessIssue] @pytest.mark.asyncio diff --git a/tests/cli/test_man_command.py b/tests/cli/test_man_command.py index 0088b6f0..c6882e9a 100644 --- a/tests/cli/test_man_command.py +++ b/tests/cli/test_man_command.py @@ -75,3 +75,21 @@ def fake_run(*args, **kwargs): assert result.exit_code == 0, result.output assert "not on your manpath" not in _flattened(result.output) + + +def test_man_install_skips_app_initialization(tmp_path, monkeypatch): + """man install must not touch the database (PR #971 review). + + Installing offline docs only copies packaged files; a locked or broken + local database must not block it, so `man` is in skip_init_commands. + """ + import basic_memory.services.initialization as init_module + + def explode(*args, **kwargs): + raise AssertionError("ensure_initialization must not run for `bm man`") + + monkeypatch.setattr(init_module, "ensure_initialization", explode) + result = runner.invoke(app, ["man", "install", "--dir", str(tmp_path)]) + + assert result.exit_code == 0, result.output + assert (tmp_path / "man1" / "bm.1").exists() diff --git a/tests/cli/test_status_wait_timeout.py b/tests/cli/test_status_wait_timeout.py new file mode 100644 index 00000000..561b1723 --- /dev/null +++ b/tests/cli/test_status_wait_timeout.py @@ -0,0 +1,55 @@ +"""Tests for the `bm status --wait` timeout guidance (#959).""" + +import pytest + +import basic_memory.cli.commands.status as status_module +from basic_memory.cli.commands.status import StatusTimeout, run_status +from basic_memory.schemas import SyncReportResponse +from basic_memory.schemas.project_info import ProjectItem + + +@pytest.mark.asyncio +async def test_status_wait_timeout_points_at_reindex(monkeypatch, config_manager): + """The timeout error must hand the user the command that actually indexes. + + In CLI-only sessions no sync coordinator runs, so pending changes never + drain and --wait always times out; without the hint the dead end looks + like a hung indexer (#959). + """ + project_item = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="scratch", + path="/tmp/scratch", + is_default=True, + ) + pending_report = SyncReportResponse(new={"notes/seed.md"}, total=1) + + class FakeProjectClient: + def __init__(self, client): + pass + + async def get_status(self, external_id): + return pending_report + + class FakeClientContext: + async def __aenter__(self): + return object() + + async def __aexit__(self, *args): + return False + + async def fake_get_active_project(client, project, context): + return project_item + + monkeypatch.setattr(status_module, "get_client", lambda **kwargs: FakeClientContext()) + monkeypatch.setattr(status_module, "get_active_project", fake_get_active_project) + monkeypatch.setattr(status_module, "ProjectClient", FakeProjectClient) + + with pytest.raises(StatusTimeout) as exc_info: + await run_status(project="scratch", wait=True, timeout=0.01, poll_interval=0.001) + + message = str(exc_info.value) + assert "Timed out" in message + assert "bm reindex --project scratch" in message + assert "no Basic Memory server is running" in message diff --git a/tests/mcp/test_async_client_modes.py b/tests/mcp/test_async_client_modes.py index a557b407..70235fd0 100644 --- a/tests/mcp/test_async_client_modes.py +++ b/tests/mcp/test_async_client_modes.py @@ -618,3 +618,42 @@ async def test_get_cloud_control_plane_client_raises_without_credentials(config_ with pytest.raises(RuntimeError, match="Cloud routing requested but no credentials found"): async with get_cloud_control_plane_client(): pass + + +@pytest.mark.asyncio +async def test_get_client_workspace_selector_routes_to_cloud(config_manager): + """A bare workspace selector routes to the cloud proxy, not local ASGI (#954). + + This is the create_memory_project(workspace=...) path on a local MCP + server: no factory, no explicit flags, no project_name — the selector + alone must imply cloud routing. + """ + cfg = config_manager.load_config() + cfg.cloud_host = "https://cloud.example.test" + cfg.cloud_api_key = "bmc_test_key_123" + config_manager.save_config(cfg) + + async with get_client(workspace="tenant-123") as client: + assert str(client.base_url).rstrip("/") == "https://cloud.example.test/proxy" + assert client.headers.get("X-Workspace-ID") == "tenant-123" + + +@pytest.mark.asyncio +async def test_get_client_workspace_selector_without_credentials_fails_fast( + config_manager, monkeypatch +): + """No credentials + workspace selector must raise, never fall back to local (#954). + + The silent local fallback was the bug: a cloud project create either + failed on a cloud-style path or silently created a local project. + """ + cfg = config_manager.load_config() + cfg.cloud_host = "https://cloud.example.test" + cfg.cloud_api_key = None + cfg.cloud_client_id = "cid" + cfg.cloud_domain = "https://auth.example.test" + config_manager.save_config(cfg) + + with pytest.raises(RuntimeError, match="cloud workspace was requested"): + async with get_client(workspace="team-slug"): + pass diff --git a/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index b8493165..7081bd72 100644 --- a/tests/mcp/test_project_context.py +++ b/tests/mcp/test_project_context.py @@ -679,7 +679,7 @@ async def test_resolve_workspace_project_identifier_handles_qualified_and_collis ) index = _build_workspace_project_index((personal, acme), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -701,7 +701,7 @@ def _patch_index(monkeypatch, workspaces, entries): index = _build_workspace_project_index(workspaces, entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -895,7 +895,7 @@ async def test_detect_project_from_memory_url_prefix_resolves_workspace_slug(mon ) index = _build_workspace_project_index((personal, team), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1026,7 +1026,7 @@ async def test_detect_project_from_identifier_prefix_resolves_workspace_with_loc ), ) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1081,7 +1081,7 @@ async def test_resolve_workspace_qualified_memory_url_ignores_workspace_project_ ) index = _build_workspace_project_index((workspace,), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1121,7 +1121,7 @@ async def test_resolve_workspace_qualified_memory_url_fails_on_duplicate_project ) index = _build_workspace_project_index((team,), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1162,7 +1162,7 @@ async def test_resolve_workspace_qualified_memory_url_uses_personal_canonical_pa ) index = _build_workspace_project_index((personal,), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1204,7 +1204,7 @@ async def test_resolve_workspace_qualified_memory_url_keeps_org_canonical_path_w ) index = _build_workspace_project_index((team,), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1318,7 +1318,7 @@ async def test_resolve_workspace_project_identifier_uses_active_workspace_for_du ) index = _build_workspace_project_index((personal, acme), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1374,7 +1374,7 @@ async def test_resolve_workspace_project_identifier_resolves_by_external_id(monk ) index = _build_workspace_project_index((personal, acme), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1411,7 +1411,7 @@ async def test_resolve_workspace_project_identifier_normalizes_uuid_forms(monkey ) index = _build_workspace_project_index((workspace,), entries) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) @@ -1666,7 +1666,7 @@ async def test_get_project_client_with_cloud_project_id_routes_to_workspace_with (WorkspaceProjectEntry(workspace=personal, project=cloud_project),), ) - async def fake_index(context=None): + async def fake_index(context=None, force_refresh=False): return index get_client_calls: list[dict[str, str | None]] = [] @@ -3153,3 +3153,142 @@ async def fail_control_plane() -> AsyncIterator[Any]: # pragma: no cover finally: # Restore original factory to avoid polluting other tests async_client._client_factory = original_factory + + +@pytest.mark.asyncio +async def test_resolver_refreshes_index_on_miss(monkeypatch): + """A stale index miss triggers one rebuild and the retry resolves (#956). + + Mirrors the field failure: a teammate (or the CLI) creates a project after + the session index was built; project_id routing must find it without a + session restart. + """ + import basic_memory.mcp.project_context as project_context + from basic_memory.mcp.project_context import ( + WorkspaceProjectEntry, + WorkspaceProjectLookupMiss, + _build_workspace_project_index, + resolve_workspace_project_identifier, + ) + + team = _workspace( + tenant_id="team-tenant", + workspace_type="organization", + slug="team", + name="Team", + role="owner", + is_default=True, + ) + old_entry = WorkspaceProjectEntry( + workspace=team, + project=_project("Existing", id=1, external_id="11111111-1111-4111-8111-111111111111"), + ) + new_entry = WorkspaceProjectEntry( + workspace=team, + project=_project("Manual", id=2, external_id="22222222-2222-4222-8222-222222222222"), + ) + stale = _build_workspace_project_index((team,), (old_entry,)) + fresh = _build_workspace_project_index((team,), (old_entry, new_entry)) + + calls = [] + + async def fake_index(context=None, force_refresh=False): + calls.append(force_refresh) + return fresh if force_refresh else stale + + monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) + + # By external_id (the project_id routing path that failed in the field) + resolved = await resolve_workspace_project_identifier( + "22222222-2222-4222-8222-222222222222" + ) + assert resolved.project.permalink == "manual" + assert calls == [False, True] + + # By name, same refresh-and-retry + calls.clear() + resolved = await resolve_workspace_project_identifier("manual") + assert resolved.project.permalink == "manual" + assert calls == [False, True] + + # A project absent from the fresh index too: exactly one refresh, then the + # authoritative miss propagates + calls.clear() + with pytest.raises(WorkspaceProjectLookupMiss): + await resolve_workspace_project_identifier("never-existed") + assert calls == [False, True] + + # A hit on the cached index never triggers a rebuild + calls.clear() + resolved = await resolve_workspace_project_identifier("existing") + assert resolved.project.permalink == "existing" + assert calls == [False] + + +@pytest.mark.asyncio +async def test_resolve_project_and_path_keeps_patterns_project_qualified( + config_manager, + monkeypatch, +): + """Glob patterns are qualified with the project prefix only, never the workspace (#957). + + The search index stores project-qualified permalinks (manual/man3/...), so a + workspace-qualified pattern (/manual/man3*) can never match anything. + Direct URLs keep workspace qualification (the link resolver handles them); + patterns have no resolver fallback and must match the index form. + """ + from mcp.server.fastmcp.exceptions import ToolError + + from basic_memory.mcp.project_context import resolve_project_and_path + from basic_memory.schemas.project_info import ProjectItem + + config = config_manager.load_config() + config.permalinks_include_project = True + config_manager.save_config(config) + + context = ContextState() + cached_project = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="manual", + path="/tmp/manual", + is_default=False, + ) + team_workspace = _workspace( + tenant_id="team-tenant", + workspace_type="organization", + slug="team-paul", + name="Team Paul", + role="editor", + ) + await context.set_state("active_project", cached_project.model_dump()) + await context.set_state("active_workspace", team_workspace.model_dump()) + + async def fake_call_post(*args, **kwargs): + raise ToolError("project not found") + + monkeypatch.setattr("basic_memory.mcp.tools.utils.call_post", fake_call_post) + + # Bare pattern: project prefix only — no workspace slug + _, resolved_path, _ = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://man3/*", + context=ctx(context), + ) + assert resolved_path == "manual/man3/*" + + # Workspace-qualified pattern URL: workspace stripped down to index form + _, resolved_path, _ = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://team-paul/manual/man3/*", + context=ctx(context), + ) + assert resolved_path == "manual/man3/*" + + # Direct URLs keep the full workspace-qualified canonical path + _, resolved_path, _ = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://man3/write-note-3", + context=ctx(context), + ) + assert resolved_path == "team-paul/manual/man3/write-note-3" diff --git a/tests/mcp/test_tool_build_context.py b/tests/mcp/test_tool_build_context.py index 921f6e5c..c2c71039 100644 --- a/tests/mcp/test_tool_build_context.py +++ b/tests/mcp/test_tool_build_context.py @@ -254,3 +254,53 @@ async def test_build_context_markdown_not_found(client, test_project): assert isinstance(result, str) assert "No results found" in result assert test_project.name in result + + +def test_format_entity_block_renders_unresolved_relations_by_name(): + """Unresolved forward references render their target text, not [[None]] (#955).""" + from datetime import UTC, datetime + + from basic_memory.mcp.tools.build_context import _format_entity_block + from basic_memory.schemas.memory import ( + ContextResult, + EntitySummary, + RelationSummary, + ) + + now = datetime.now(UTC) + page = EntitySummary( + external_id="entity-1", + entity_id=1, + title="write-note(3)", + permalink="man3/write-note-3", + content="# write-note(3)", + file_path="man3/write-note-3.md", + created_at=now, + ) + unresolved = RelationSummary( + title="see_also: edit-note(3)", + file_path="man3/write-note-3.md", + permalink="man3/write-note-3/see-also/edit-note-3", + relation_type="see_also", + from_entity="write-note(3)", + to_entity=None, + to_name="edit-note(3)", + created_at=now, + ) + resolved = RelationSummary( + title="see_also: bm-note(5)", + file_path="man3/write-note-3.md", + permalink="man3/write-note-3/see-also/bm-note-5", + relation_type="see_also", + from_entity="write-note(3)", + to_entity="bm-note(5)", + to_name="bm-note(5)", + created_at=now, + ) + block = _format_entity_block( + ContextResult(primary_result=page, observations=[], related_results=[unresolved, resolved]) + ) + + assert "- see_also [[edit-note(3)]]" in block + assert "- see_also [[bm-note(5)]]" in block + assert "[[None]]" not in block diff --git a/tests/services/test_context_service.py b/tests/services/test_context_service.py index b56f40f2..0b584207 100644 --- a/tests/services/test_context_service.py +++ b/tests/services/test_context_service.py @@ -645,3 +645,102 @@ async def test_build_context_without_link_resolver( url = memory_url.validate_strings("memory://Root") context_result = await service.build_context(url) assert context_result.metadata.primary_count == 0 + + +@pytest.mark.asyncio +async def test_find_related_carries_to_name_for_unresolved_relations(session_maker, app_config): + """Relation rows expose to_name so unresolved forward refs render by name (#955). + + A forward reference (to_id NULL) previously surfaced with no usable target + text — build_context printed [[None]] even though the markdown named the + target. The context query must select to_name for both resolved and + unresolved relation rows. + """ + from basic_memory.repository.entity_repository import EntityRepository + from basic_memory.repository.observation_repository import ObservationRepository + from basic_memory.repository.sqlite_search_repository import SQLiteSearchRepository + from basic_memory.repository.postgres_search_repository import PostgresSearchRepository + from basic_memory.config import DatabaseBackend + from basic_memory import db + + async with db.scoped_session(session_maker) as db_session: + project = Project(name="forward-ref-project", path="/forward-ref") + db_session.add(project) + await db_session.flush() + + source = Entity( + title="write-note(3)", + note_type="document", + content_type="text/markdown", + project_id=project.id, + permalink="man3/write-note-3", + file_path="man3/write-note-3.md", + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + resolved_target = Entity( + title="bm-note(5)", + note_type="document", + content_type="text/markdown", + project_id=project.id, + permalink="man5/bm-note-5", + file_path="man5/bm-note-5.md", + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + db_session.add_all([source, resolved_target]) + await db_session.flush() + + resolved = Relation( + project_id=project.id, + from_id=source.id, + to_id=resolved_target.id, + to_name="bm-note(5)", + relation_type="see_also", + ) + # Forward reference: the target page does not exist yet + unresolved = Relation( + project_id=project.id, + from_id=source.id, + to_id=None, + to_name="edit-note(3)", + relation_type="see_also", + ) + db_session.add_all([resolved, unresolved]) + await db_session.commit() + + if app_config.database_backend == DatabaseBackend.POSTGRES: + search_repo = PostgresSearchRepository(session_maker, project.id) + else: + search_repo = SQLiteSearchRepository(session_maker, project.id) + entity_repo = EntityRepository(session_maker, project.id) + obs_repo = ObservationRepository(session_maker, project.id) + context_service = ContextService(search_repo, entity_repo, obs_repo) + + related = await context_service.find_related([("entity", source.id)], max_depth=2) + relation_rows = {r.to_name: r for r in related if r.type == "relation"} + + assert "edit-note(3)" in relation_rows, "unresolved relation row missing to_name" + assert relation_rows["edit-note(3)"].to_id is None + assert relation_rows["bm-note(5)"].to_name == "bm-note(5)" + + +@pytest.mark.asyncio +async def test_pattern_search_falls_back_for_legacy_unqualified_rows(context_service, test_graph): + """Workspace-qualified patterns fall back to the project form for legacy rows (#957). + + Rows written before workspace canonicalization (or via clients that did not + forward workspace headers) store project-qualified permalinks. A pattern + canonicalized under an active workspace context would otherwise match + nothing — the field failure that opened the issue. + """ + from basic_memory.workspace_context import workspace_permalink_context + + # test_graph rows are stored without any workspace prefix (legacy form). + # Query with a workspace-qualified pattern under an active context. + with workspace_permalink_context(workspace_slug="team-paul", workspace_type="organization"): + context = await context_service.build_context("memory://team-paul/test-project/test/*") + + permalinks = {result.primary_result.permalink for result in context.results} + assert permalinks, "fallback did not match legacy rows" + assert all(p and p.startswith("test-project/test/") for p in permalinks), permalinks