fix(mcp): close out the #952 manual verification findings (#981)

Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hernandez
2026-06-11 17:59:35 -05:00
committed by GitHub
parent a148e72f56
commit 1ad3a350ad
17 changed files with 600 additions and 51 deletions
+1
View File
@@ -147,6 +147,7 @@ async def to_graph_context(
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,
+3
View File
@@ -83,8 +83,11 @@ def app_callback(
# 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",
+8 -1
View File
@@ -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)
+23 -2
View File
@@ -10,7 +10,7 @@ from httpx import ASGITransport, AsyncClient, Timeout
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 <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:
+73 -5
View File
@@ -54,6 +54,14 @@ _workspace_provider: Optional[Callable[[], Awaitable[list[WorkspaceInfo]]]] = No
_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}"
)
+4 -1
View File
@@ -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] = [
@@ -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
@@ -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 @@ class SearchIndexRow:
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,
+3
View File
@@ -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[
+38 -2
View File
@@ -18,6 +18,7 @@ from basic_memory.repository.search_repository import SearchRepository, SearchIn
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 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
from_id,
to_id,
relation_type,
to_name,
content,
category,
entity_id,
@@ -550,7 +582,7 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
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 @@ class ContextService:
from_id,
to_id,
relation_type,
to_name,
content,
category,
entity_id,
@@ -686,7 +722,7 @@ class ContextService:
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
""")