From ad25b13cae44fd6a18d6c2e21cc1aa5ecfa6a531 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:30:08 -0500 Subject: [PATCH 1/9] fix(cli): skip app initialization for bm man MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #971 review (codex): man install only copies packaged groff files, but as a top-level subcommand it ran ensure_initialization first — a locked or broken local database would block installing the offline docs. Add man to skip_init_commands with a regression test that fails if initialization ever runs for it. Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/cli/app.py | 3 +++ tests/cli/test_man_command.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) 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/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() From ad083212d3d20318130b76e29841062e76512efc Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:35:37 -0500 Subject: [PATCH 2/9] fix(mcp): refresh workspace project index on lookup miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session-local index was built once and never refreshed, so projects created out-of-band (CLI, a teammate in a shared workspace) stayed invisible and unroutable by project_id until session restart — and the error asserted nonexistence of a project that demonstrably existed. A miss is exactly the signal the cache may be stale: tag absent-project errors as WorkspaceProjectLookupMiss (distinct from ambiguity, which a refresh cannot fix), rebuild the index once, and retry. A second miss is authoritative. Hits never trigger rebuilds. This also un-breaks read_note's fuzzy fallback, which re-resolves the active project by external_id through this index. Fixes #956 Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 49 +++++++++++-- tests/mcp/test_project_context.py | 94 +++++++++++++++++++++---- 2 files changed, 126 insertions(+), 17 deletions(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 212b6048..185ace36 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.""" @@ -722,9 +730,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 +879,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 +934,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 +943,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 +968,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/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index b8493165..f24e6aa9 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,73 @@ 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] From 87e3577f19d159034407e310394e3ae1a923f71f Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:40:02 -0500 Subject: [PATCH 3/9] fix(mcp): route workspace selectors to the cloud or fail fast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_client(workspace=...) silently dropped the selector on the default fallback path: a create_memory_project targeting a team workspace from a local MCP server (OAuth-only, no flags, no project route) was served by the local ASGI app — failing on cloud-style paths like '/manual' or, worse, silently creating a local project where the user asked for a shared cloud one. A workspace selector now implies cloud routing: route to the cloud proxy when credentials exist, raise with the auth hint when they don't. Explicit --local and per-project routing still take precedence (priority order is documented in the docstring). _resolve_workspace_routing now resolves slugs/names to tenant ids whenever credentials allow discovery — not only under factory mode or explicit --cloud — so X-Workspace-ID always carries the tenant id. Fixes #954 Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/mcp/async_client.py | 25 +++++++++++- .../mcp/tools/project_management.py | 8 +++- tests/mcp/test_async_client_modes.py | 39 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) 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/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py index cd022906..eda0d4f4 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 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 From cdd6e923c8ace5c62817fd9a351fe2d9dd1903c3 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:46:45 -0500 Subject: [PATCH 4/9] fix(core): carry to_name through context so forward refs render by name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_context printed unresolved forward references as [[None]]: the relation's literal target text (Relation.to_name) was embedded only in the composite row title ('see_also: edit-note(3)'), to_id is NULL until the target exists, and the formatter rendered the resolved-entity title — None. Select to_name as a real column in both context CTEs (SQLite and Postgres), thread it through ContextResultRow, SearchIndexRow, and RelationSummary (additive, optional — JSON consumers gain the field too), and fall back to it in the text formatter. Forward references are a first-class part of the format; they now render as written. Regression tests at the service layer (both backends) and the formatter. Fixes #955 Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/api/v2/utils.py | 1 + src/basic_memory/mcp/tools/build_context.py | 5 +- .../repository/search_index_row.py | 2 + src/basic_memory/schemas/memory.py | 3 + src/basic_memory/services/context_service.py | 16 +++- tests/mcp/test_tool_build_context.py | 50 ++++++++++++ tests/services/test_context_service.py | 80 +++++++++++++++++++ 7 files changed, 154 insertions(+), 3 deletions(-) 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/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/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..a31dc7fa 100644 --- a/src/basic_memory/services/context_service.py +++ b/src/basic_memory/services/context_service.py @@ -36,6 +36,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 @@ -392,6 +393,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 +428,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 +481,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 +548,7 @@ def _build_postgres_query( # pragma: no cover from_id, to_id, relation_type, + to_name, content, category, entity_id, @@ -550,7 +558,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 +587,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 +615,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 +654,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 +688,7 @@ def _build_sqlite_query( from_id, to_id, relation_type, + to_name, content, category, entity_id, @@ -686,7 +698,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/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..f997d86c 100644 --- a/tests/services/test_context_service.py +++ b/tests/services/test_context_service.py @@ -645,3 +645,83 @@ 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)" From 4fb6e2c9e2c3687bc0015859b816a9a967be6932 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:52:46 -0500 Subject: [PATCH 5/9] fix(mcp): qualify glob patterns with project prefix only, not workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit folder/* patterns returned nothing on cloud workspace projects: memory URL canonicalization qualified them as //, but the search index stores project-qualified permalinks without the workspace prefix, so the pattern-match path (which has no link-resolver fallback, unlike direct lookups) could never match a row. Patterns now canonicalize to the index form — project prefix only — in both the active-route and workspace-qualified-URL paths. Direct URLs keep full workspace qualification, which the link resolver understands. Fixes #957 Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 25 +++++++++ tests/mcp/test_project_context.py | 69 +++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 185ace36..7bf9b0de 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -468,6 +468,16 @@ 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): + # patterns match project-qualified permalinks, never workspace-qualified ones. + if "*" in normalized_remainder: + return build_qualified_permalink_reference( + project_permalink, + normalized_remainder, + include_project=True, + ) + return build_qualified_permalink_reference( project_permalink, normalized_remainder, @@ -485,6 +495,21 @@ 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/*). + # Why: pattern lookups match raw against the search index, which stores + # project-qualified permalinks without the workspace prefix — a + # workspace-qualified pattern can never match anything (#957). Direct + # lookups keep full workspace qualification because the link resolver + # understands it; the pattern path has no resolver fallback. + # Outcome: qualify patterns with the project prefix only. + if "*" in path: + 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 diff --git a/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index f24e6aa9..7081bd72 100644 --- a/tests/mcp/test_project_context.py +++ b/tests/mcp/test_project_context.py @@ -3223,3 +3223,72 @@ async def fake_index(context=None, force_refresh=False): 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" From 02687d6b3714bfedb8aa87f26e59302b923384fa Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 16:56:38 -0500 Subject: [PATCH 6/9] fix(cli): point status --wait timeouts at bm reindex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In CLI-only sessions no sync coordinator runs (it lives in the server lifespan), so pending changes never drain and --wait always times out — a dead end that looks like a hung indexer. The timeout error now says so and hands over the command that actually indexes: 'bm reindex --project '. Closes the code half of #959 (the stale 'basic-memory sync' doc references were fixed in #971). Fixes #959 Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/cli/commands/status.py | 9 +++- tests/cli/test_status_wait_timeout.py | 55 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/cli/test_status_wait_timeout.py 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/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 From ed4d94f835609b92cfcf968ed0d8118b9e93caa2 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 17:05:55 -0500 Subject: [PATCH 7/9] test(mcp): pin the new workspace-selector contract in integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_create_project_accepts_workspace_in_local_mode asserted the old behavior — workspace as a local no-op — which is precisely what #954 classifies as the bug (silent local create when a cloud team workspace was requested). The test now pins the new contract: the schema still accepts the parameter over the MCP wire, the create fails fast with auth guidance when no credentials exist, and no local project is left behind. Tool docstrings updated to match. Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- .../mcp/tools/project_management.py | 10 +++-- .../test_project_management_integration.py | 43 +++++++++---------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/basic_memory/mcp/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py index eda0d4f4..fd14d565 100644 --- a/src/basic_memory/mcp/tools/project_management.py +++ b/src/basic_memory/mcp/tools/project_management.py @@ -525,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. @@ -664,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/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 From efe7a086675ae469e8fef8244db727f324b16949 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 17:16:22 -0500 Subject: [PATCH 8/9] fix(mcp): scope pattern project-qualification to contextvar-less routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught an interaction the first #957 cut missed: when the server-side workspace permalink contextvar is active (hosted MCP / factory mode), stored permalinks ARE workspace-qualified, so workspace-qualified patterns are correct there. The stored form follows the contextvar at write time; the client's cached_workspace is display state and must not qualify patterns. Patterns now drop the workspace prefix only when no contextvar is active — exactly the local-stdio-to-cloud case from the field repro. Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 7bf9b0de..f47bd3ef 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -470,8 +470,9 @@ def _canonical_memory_path_for_workspace( normalized_remainder = project_permalink # Same index-form rule as _canonical_memory_path_for_active_route (#957): - # patterns match project-qualified permalinks, never workspace-qualified ones. - if "*" in normalized_remainder: + # 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, @@ -496,14 +497,17 @@ def _canonical_memory_path_for_active_route( """Return the canonical permalink path for the currently routed project/workspace.""" project_prefix = active_project.permalink - # Trigger: the path contains a glob wildcard (folder/*). - # Why: pattern lookups match raw against the search index, which stores - # project-qualified permalinks without the workspace prefix — a - # workspace-qualified pattern can never match anything (#957). Direct - # lookups keep full workspace qualification because the link resolver - # understands it; the pattern path has no resolver fallback. - # Outcome: qualify patterns with the project prefix only. - if "*" in path: + # 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}/"): From 58cbe745e42abe0eab2031cfc95e913e41939fa5 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 11 Jun 2026 17:37:04 -0500 Subject: [PATCH 9/9] fix(core): pattern search falls back past the workspace prefix for legacy rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #981 review (codex): cloud routes run inside the workspace permalink contextvar, so workspace-qualified patterns are correct for rows written under that context — but rows written before workspace canonicalization (or via clients that didn't forward the headers) store project-qualified permalinks, and a qualified pattern can never match them. The client cannot know which form the server stores. Server-side answer: when a wildcard search matches nothing and a workspace permalink context is active, retry once with the workspace prefix stripped. New-consistent data matches on the first attempt; legacy data matches on the fallback. Regression test writes legacy-form rows and queries under an active workspace context. Co-Authored-By: Claude Fable 5 Signed-off-by: phernandez --- src/basic_memory/services/context_service.py | 24 +++++++++++++++++++ tests/services/test_context_service.py | 25 +++++++++++++++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/services/context_service.py b/src/basic_memory/services/context_service.py index a31dc7fa..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 @@ -144,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}'") diff --git a/tests/services/test_context_service.py b/tests/services/test_context_service.py index f997d86c..0b584207 100644 --- a/tests/services/test_context_service.py +++ b/tests/services/test_context_service.py @@ -717,11 +717,30 @@ async def test_find_related_carries_to_name_for_unresolved_relations(session_mak 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 - ) + 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