diff --git a/src/basic_memory/cli/commands/posix.py b/src/basic_memory/cli/commands/posix.py index 6586d3113..8f38634d4 100644 --- a/src/basic_memory/cli/commands/posix.py +++ b/src/basic_memory/cli/commands/posix.py @@ -46,6 +46,7 @@ _validate_output_flags, console, ) + from basic_memory.schemas.directory import DEFAULT_DIRECTORY_PAGE_SIZE # MCP tool functions are imported inside each command: importing @@ -358,10 +359,16 @@ def _add_tree_branches(branch: Tree, entries: dict[str, _TreeEntry]) -> None: _add_tree_branches(child, entry.children) -def _display_tree(result: dict[str, Any], path: str) -> None: - """Render find results as a Rich tree rooted at the search path.""" - entries = _build_tree(list(result.get("nodes", [])), path) - tree = Tree(f"[bold cyan]{markup_escape(path)}[/bold cyan]") +def _display_tree(result: dict[str, Any], label: str, root: str) -> None: + """Render find results as a Rich tree rooted at the search path. + + ``label`` is the caller's spelling of the root; ``root`` is the routed + project-relative path the node paths actually start with — a qualified + '/dir' input strips its project prefix in the shared tool layer, + so the two differ exactly when the input carried a project prefix (#1415). + """ + entries = _build_tree(list(result.get("nodes", [])), root) + tree = Tree(f"[bold cyan]{markup_escape(label)}[/bold cyan]") if not entries: tree.add("[dim]empty[/dim]") _add_tree_branches(tree, entries) @@ -379,10 +386,14 @@ def _print_plain_tree_level(entries: dict[str, _TreeEntry], depth: int) -> None: _print_plain_tree_level(entry.children, depth + 1) -def _plain_tree(result: dict[str, Any], path: str) -> None: - """Render the tree as two-space-indented lines; pagination note on stderr.""" - print(path) - _print_plain_tree_level(_build_tree(list(result.get("nodes", [])), path), depth=1) +def _plain_tree(result: dict[str, Any], label: str, root: str) -> None: + """Render the tree as two-space-indented lines; pagination note on stderr. + + ``label``/``root`` split as in ``_display_tree``: print the caller's + spelling, strip the routed project-relative root from node paths. + """ + print(label) + _print_plain_tree_level(_build_tree(list(result.get("nodes", [])), root), depth=1) if result.get("has_more") is True: print(f"… more entries (page {result.get('page', 1)}; use --page)", file=sys.stderr) @@ -811,16 +822,20 @@ def tree( # tree is a client-side recombination of find: the same flat listing, with # the hierarchy rebuilt from directory paths for display, so its JSON # contract is exactly find's payload. - from basic_memory.mcp.tools import find as mcp_find + from basic_memory.mcp.tools.posix_tools import find_listing from fastmcp.exceptions import ToolError try: validate_routing_flags(local, cloud) _validate_output_flags(json_output, plain) + # find returns the listing and the root its node paths are relative to + # from one resolution. Resolving here as well cost a second project-list + # round trip on every cloud call, because a CLI invocation has no + # FastMCP context for the per-request cache to live in (#1421). with force_routing(local=local, cloud=cloud): - result = run_with_cleanup( - mcp_find( + result, root = run_with_cleanup( + find_listing( path, name=name, depth=depth, @@ -834,9 +849,9 @@ def tree( if mode == "json": _print_json(result) elif mode == "plain": - _plain_tree(result, path) + _plain_tree(result, path, root) else: - _display_tree(result, path) + _display_tree(result, path, root) except (ValueError, ToolError) as e: typer.echo(f"Error: {e}", err=True) raise typer.Exit(1) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 3aa6e137e..fbad0a89e 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -13,7 +13,9 @@ from __future__ import annotations import asyncio +import os from contextlib import asynccontextmanager, nullcontext +from dataclasses import dataclass from typing import ( TYPE_CHECKING, AsyncIterator, @@ -61,10 +63,10 @@ identifier_path as _identifier_path, project_matches_identifier as _project_matches_identifier, split_project_prefix as _split_project_prefix, + is_workspace_route_shaped as _is_workspace_route_shaped, + split_project_permalink_prefix as _split_project_permalink_prefix, split_qualified_project_identifier as _split_qualified_project_identifier_impl, - split_workspace_identifier_segments as _split_workspace_identifier_segments, - split_workspace_memory_url_segments as _split_workspace_memory_url_segments, - unqualified_project_identifier as _unqualified_project_identifier, + split_workspace_slug_prefix as _split_workspace_slug_prefix, ) from basic_memory.mcp.workspace_project_index import ( WORKSPACE_PROJECT_INDEX_STATE_KEY as _WORKSPACE_PROJECT_INDEX_STATE_KEY, @@ -239,6 +241,14 @@ def _workspace_identifier_discovery_available( if _cloud_workspace_discovery_available(config): return True + # Identifier detection serves read_note and search, which have no mount + # table and no refusal rule, so only the unmistakable three-segment form may + # reach for the network here. + return _local_cloud_discovery_allowed(config) and _is_workspace_route_shaped(identifier) + + +def _local_cloud_discovery_allowed(config: BasicMemoryConfig) -> bool: + """Return True when a locally routed session may still consult cloud discovery.""" from basic_memory.mcp.async_client import ( _explicit_routing, _force_local_mode, @@ -247,10 +257,7 @@ def _workspace_identifier_discovery_available( if _explicit_routing() and _force_local_mode(): return False - return ( - has_cloud_credentials(config) - and _split_workspace_identifier_segments(identifier) is not None - ) + return has_cloud_credentials(config) async def resolve_workspace_qualified_memory_url( @@ -258,32 +265,48 @@ async def resolve_workspace_qualified_memory_url( context: Optional[Context] = None, ) -> WorkspaceMemoryUrlResolution | None: """Resolve a workspace-qualified memory URL against accessible workspaces.""" - segments = _split_workspace_memory_url_segments(identifier) - if segments is None: + if not identifier.strip().startswith("memory://"): return None - - return await _resolve_workspace_segments(identifier, segments, context=context) + return await resolve_workspace_qualified_identifier(identifier, context=context) async def resolve_workspace_qualified_identifier( identifier: str, context: Optional[Context] = None, ) -> WorkspaceMemoryUrlResolution | None: - """Resolve a workspace-qualified permalink or memory URL against accessible workspaces.""" - segments = _split_workspace_identifier_segments(identifier) - if segments is None: + """Resolve a workspace-qualified permalink or memory URL against accessible workspaces. + + A path is required after the project. That is what keeps + 'memory://main/notes' readable as project 'main' with note 'notes': a + workspace route has to name something *inside* the project, or the same + string would have two readings and the project-prefix resolver would lose + the ones it has always owned. The posix resolver takes the pathless form + (a project root is a legitimate thing to list) and says so at its own call + site. + """ + resolved = await _resolve_workspace_route(identifier, context=context) + if resolved is None or not resolved[1]: return None - - return await _resolve_workspace_segments(identifier, segments, context=context) + return resolved[0] -async def _resolve_workspace_segments( +async def _resolve_workspace_route( identifier: str, - segments: tuple[str, str, str], context: Optional[Context] = None, -) -> WorkspaceMemoryUrlResolution | None: - """Resolve parsed workspace/project/path segments against accessible workspaces.""" - workspace_slug, project_identifier, remainder = segments +) -> tuple[WorkspaceMemoryUrlResolution, str] | None: + """Resolve '/[/]' to its project and remaining path. + + The project half is matched by ``split_project_permalink_prefix`` against + the projects of *that* workspace, so a project whose permalink spans + several segments ('Research/2026') is reachable and the remainder always + comes from the same match that chose the project — the two can never + disagree about how many segments were consumed. + """ + slug_split = _split_workspace_slug_prefix(identifier) + if slug_split is None: + return None + workspace_slug, rest = slug_split + index = await _ensure_workspace_project_index(context=context) workspace = next( (item for item in index.workspaces if item.slug.casefold() == workspace_slug.casefold()), @@ -292,13 +315,23 @@ async def _resolve_workspace_segments( if workspace is None: return None - project_permalink = generate_permalink(project_identifier) - matches = [ - entry - for entry in index.entries_by_permalink.get(project_permalink, ()) - if entry.workspace.tenant_id == workspace.tenant_id - ] - if not matches: + entries_by_permalink: dict[str, WorkspaceProjectEntry] = {} + for entry in index.entries: + if entry.workspace.tenant_id != workspace.tenant_id: + continue + collision = entries_by_permalink.setdefault(entry.project.permalink, entry) + if collision is not entry: + details = ", ".join( + f"{item.qualified_name} ({item.project.external_id})" for item in (collision, entry) + ) + raise ValueError( + f"Project '{entry.project.permalink}' matched multiple projects in workspace " + f"'{workspace.name}' ({workspace.slug}). Project permalinks must be unique. " + f"Matches: {details}" + ) + + claimed = _split_project_permalink_prefix(rest, entries_by_permalink) + if claimed is None: if any( failed_workspace.tenant_id == workspace.tenant_id for failed_workspace in index.failed_workspaces @@ -308,32 +341,24 @@ async def _resolve_workspace_segments( "could not be loaded. Retry after workspace discovery recovers." ) - # Trigger: first segment matches a workspace slug but the second does not - # match a project in that workspace. - # Why: workspace-qualified URLs require both route segments to match; otherwise + # Trigger: first segment matches a workspace slug but nothing after it + # matches a project in that workspace. + # Why: workspace-qualified routes require both halves to match; otherwise # existing project-prefixed URLs like `memory://main/notes/foo` can collide # with a workspace slug named `main`. # Outcome: treat this as not workspace-qualified and let the caller use # the existing project-prefix/default-project resolver. return None - if len(matches) > 1: - details = ", ".join( - f"{entry.qualified_name} ({entry.project.external_id})" for entry in matches - ) - raise ValueError( - f"Project '{project_identifier}' matched multiple projects in workspace " - f"'{workspace.name}' ({workspace.slug}). Project permalinks must be unique. " - f"Matches: {details}" - ) - entry = matches[0] + project_permalink, remainder = claimed + entry = entries_by_permalink[project_permalink] canonical_path = _canonical_memory_path_for_workspace( workspace_slug=entry.workspace.slug, workspace_type=entry.workspace.workspace_type, project_permalink=entry.project.permalink, remainder=remainder, ) - return WorkspaceMemoryUrlResolution(entry=entry, canonical_path=canonical_path) + return WorkspaceMemoryUrlResolution(entry=entry, canonical_path=canonical_path), remainder async def get_available_workspaces(context: Optional[Context] = None) -> list[WorkspaceInfo]: @@ -800,7 +825,11 @@ async def resolve_project_and_path( workspace_context = current_workspace_permalink_context() if workspace_context and project: workspace_prefix = generate_permalink(workspace_context.workspace_slug) - project_permalink = generate_permalink(_unqualified_project_identifier(project)) + # Strip only this workspace's own slug. Guessing that the first + # segment of `project` is a workspace mangles a project whose name + # contains '/' ('Research/2026' -> '2026'), and the workspace here + # is known, so nothing has to be inferred. + project_permalink = generate_permalink(project).removeprefix(f"{workspace_prefix}/") qualified_prefix = f"{workspace_prefix}/{project_permalink}" if normalized_path == qualified_prefix or normalized_path.startswith( f"{qualified_prefix}/" @@ -926,6 +955,17 @@ async def detect_project_from_memory_url_prefix( return await detect_project_from_identifier_prefix(identifier, config, context=context) +# Workspace discovery is best-effort for prefix detection: an identifier that +# names no reachable workspace/project simply stays unrouted, because it may not +# have meant a workspace at all. Anything outside this set is a real failure and +# propagates. +_WORKSPACE_DISCOVERY_FALLBACK_ERRORS = ( + "not found", + "no accessible workspaces", + "unable to discover", +) + + async def detect_project_from_identifier_prefix( identifier: str, config: BasicMemoryConfig, @@ -945,11 +985,6 @@ async def detect_project_from_identifier_prefix( return None if _workspace_identifier_discovery_available(identifier, config): - workspace_discovery_fallback_errors = ( - "not found", - "no accessible workspaces", - "unable to discover", - ) try: workspace_resolution = await resolve_workspace_qualified_identifier( identifier, @@ -957,7 +992,7 @@ async def detect_project_from_identifier_prefix( ) except ValueError as exc: message = str(exc).lower() - if any(error in message for error in workspace_discovery_fallback_errors): + if any(error in message for error in _WORKSPACE_DISCOVERY_FALLBACK_ERRORS): return None raise @@ -975,7 +1010,7 @@ async def detect_project_from_identifier_prefix( ) except ValueError as exc: message = str(exc).lower() - if any(error in message for error in workspace_discovery_fallback_errors): + if any(error in message for error in _WORKSPACE_DISCOVERY_FALLBACK_ERRORS): return None raise @@ -984,6 +1019,537 @@ async def detect_project_from_identifier_prefix( return None +# --- Project-qualified path routing (POSIX tools, #1415) --- +# Projects are mount points: '/path' inputs route to that project so +# tool inputs accept exactly the prefixed identifiers tool outputs and stored +# permalinks produce. One resolver serves the MCP posix tools and, through +# them, the CLI verbs. +# +# The mount table and the routing table are one set (addressable_projects), so +# every mount `ls /` advertises is reachable by name and an unqualified +# reference in a many-project workspace refuses instead of picking a default +# (#1421). +# +# Precedence: the mount table wins the leading path segments, ahead of +# workspace-qualified '//' parsing. A project permalink +# can also be an accessible workspace's slug, and only one reading of '/...' +# can win. Mount-wins is what the advertised list promises — a name `ls /` shows +# must address that mount, or we advertise a name that resolves somewhere else, +# which is worse than not advertising it at all. "Segments", plural: a project +# name may contain '/', so its advertised permalink can span more than one. +# +# Route versus path is inherently ambiguous, so the answer is a precedence +# order, not a parser. 'acme/docs/foo' is a well-formed workspace route AND a +# well-formed folder path inside the caller's own project; no analysis of the +# string, and no amount of knowing which projects exist, recovers which the +# caller meant. One question settles it: does the call already say which project +# it means? +# +# 1. An explicit project (param or env constraint) says so. The remaining path +# is inside that project and nothing reroutes it. A prefix naming a +# *different* addressable mount still conflicts rather than being silently +# preferred — that is a contradiction in one call, not an ambiguity. +# 2. Otherwise the leading segments are read as a route, but only when they +# name an accessible workspace AND a project inside it. A path that merely +# looks workspace-shaped ('notes/2026/foo') matches nothing and stays +# relative on its own. +# +# Rule 2 deliberately does not depend on how many projects the session mounts. +# It did briefly, to keep a coincidentally workspace-shaped relative path at +# home in a one-mount session; that made the qualified paths this layer *itself* +# emits unroutable there, since replaying one without the project argument read +# the sole mount's same-named path instead. Both readings are a silent +# wrong-project read, so the tie breaks on whose string it is: the emitted +# canonical form is ours and must route back, while a user-typed path that +# collides with a real workspace and project has an explicit, documented fix in +# rule 1. Coincidence loses to the address we published. +# +# Rule 3 (mounts) sits above all of this: a name `ls /` advertises always +# addresses that mount. +# +# The cost of that choice, stated plainly: when a project's permalink equals an +# accessible workspace's slug, that workspace's OTHER projects lose their +# qualified path spelling. With '/team' advertised as a mount, 'team/docs/x' is +# project 'team', path 'docs/x' — never workspace 'team', project 'docs'. Those +# projects stay addressable through the project param (project='team/docs' with +# the project-relative path 'x'), which is the escape hatch the prefix-conflict +# message already teaches, so nothing becomes unreachable — only differently +# spelled. + + +@dataclass(frozen=True) +class ProjectPathRoute: + """Effective routing for one posix call: project params + project-relative path. + + ``stripped=False`` means the input carried no recognized project prefix: + ``path`` is the caller's input byte-for-byte and ``project`` is the + explicit value that was passed (or None, meaning the existing default + resolution chain applies — only reachable when the session addresses at + most one project; several addressable projects refuse instead). + ``stripped=True`` means a project prefix was recognized: ``project`` + is the canonical config name (or workspace-qualified name) and ``path`` is + the remainder with no leading slash, "" meaning the project root. + + ``project_id`` is the effective external_id for the call — the caller's own + when they passed one, otherwise the id of the advertised mount that claimed + the prefix. Callers pass both fields to ``get_project_client`` verbatim; the + id is what keeps a mount bound to the workspace that advertised it, since a + bare project name can name a different project in another accessible + workspace (#1421). + """ + + project: Optional[str] + path: str + stripped: bool + project_id: Optional[str] = None + + +class ProjectPrefixConflictError(ValueError): + """Explicit project param and the path's project prefix name different projects.""" + + +class UnqualifiedPathRefusedError(ValueError): + """Unqualified input matched no project in a workspace that addresses several.""" + + +class AmbiguousMountError(ValueError): + """Two addressable projects share one permalink, so a path names both.""" + + +@dataclass(frozen=True) +class AddressableProject: + """One project this session can both advertise and route to. + + ``name`` is the routing identifier handed to ``get_project_client`` and + ``permalink`` is the path prefix agents copy out of tool output. A cloud + session also carries ``external_id``: project names are unique only inside + one workspace, so the UUID is the only identifier that pins a mount to the + workspace whose listing advertised it. A locally routed session reads its + mounts from config, which holds no UUIDs, and has no second workspace to be + confused with, so ``external_id`` is None there. + """ + + name: str + permalink: str + external_id: Optional[str] = None + + +# The session's own project listing, memoized for one MCP request: routing and +# the mount view ask the same question, so a request pays for it at most once. +_SESSION_PROJECT_LIST_STATE_KEY = "session_project_list" + + +def _session_routes_to_cloud() -> bool: + """Return True when this session's project-less client is a tenant route. + + Mirrors ``get_client()``'s own decision for a call that names no project: + factory injection first (the hosted MCP server), then an explicit --cloud + flag. Everything else is the local ASGI app, whose mount table is the local + config. The distinction matters because BasicMemoryConfig always + materializes a placeholder 'main' project, so a non-empty ``config.projects`` + proves nothing about a cloud session's real projects. + """ + from basic_memory.mcp.async_client import ( + _explicit_routing, + _force_local_mode, + is_factory_mode, + ) + + return is_factory_mode() or (_explicit_routing() and not _force_local_mode()) + + +async def _session_project_list(context: Optional[Context] = None) -> ProjectList: + """List the projects reachable through this session's own route.""" + if context: + cached_raw = await context.get_state(_SESSION_PROJECT_LIST_STATE_KEY) + if isinstance(cached_raw, dict): + return ProjectList.model_validate(cached_raw) + + # Deferred imports to avoid circular dependency with the client modules. + from basic_memory.mcp.async_client import get_client + from basic_memory.mcp.clients import ProjectClient + + async with get_client() as client: + project_list = await ProjectClient(client).list_projects() + + if context: + await context.set_state(_SESSION_PROJECT_LIST_STATE_KEY, project_list.model_dump()) + return project_list + + +async def addressable_projects( + context: Optional[Context] = None, +) -> tuple[AddressableProject, ...]: + """Return every project this session can address, sorted by name. + + One source answers two questions that must never disagree: which mounts + ``ls /`` advertises, and which first path segment names a project. When + they came from different sources, a cloud session could advertise + ``/research`` at the root and then fail to recognize ``research/notes/x``, + silently routing it to a default project instead (#1421). + + A locally routed session's config IS its mount table, so it answers with no + network call. A cloud session's projects live in the tenant database and + its local config holds only the placeholder 'main' entry, so the session's + own project listing answers instead — the same call the mount view has + always made to render the root. + """ + if _session_routes_to_cloud(): + project_list = await _session_project_list(context=context) + projects = ( + AddressableProject( + name=item.name, + permalink=item.permalink, + external_id=item.external_id, + ) + for item in project_list.projects + ) + else: + projects = ( + AddressableProject(name=name, permalink=generate_permalink(name)) + for name in ConfigManager().config.projects + ) + return tuple(sorted(projects, key=lambda project: project.name)) + + +def _addressable_project_prefixes(projects: tuple[AddressableProject, ...]) -> str: + """Render addressable projects as copyable '/' prefixes.""" + return ", ".join(f"{permalink}/" for permalink in sorted(item.permalink for item in projects)) + + +def _claim_mount_prefix( + candidate: str, + projects: tuple[AddressableProject, ...], +) -> tuple[AddressableProject, str] | None: + """Return the mount whose permalink claims the candidate's leading segments. + + The mount table is one candidate set among several, so the matching itself + lives in ``split_project_permalink_prefix``; this only maps the winning + permalink back to the project that owns it. + """ + # Two addressable projects can share a permalink ('My Docs' beside + # 'my-docs'). add_project refuses that now, so only a config written before + # the check, or edited by hand, still holds one. Record the pair rather than + # rejecting the table: an ambiguity is only a problem for the paths that + # actually resolve to it, and failing at build time failed every route in + # the session — including the exact-name escape hatch the error recommends, + # which does not go through this lookup at all. + by_permalink: dict[str, AddressableProject] = {} + collisions: dict[str, AddressableProject] = {} + for project in projects: + first = by_permalink.setdefault(project.permalink, project) + if first is not project: + collisions.setdefault(project.permalink, project) + + claimed = _split_project_permalink_prefix(candidate, by_permalink) + if claimed is None: + return None + permalink, remainder = claimed + + # This path names the duplicated permalink, so it genuinely names two + # projects. Refuse it — silently picking one would read that project's + # content under the other's name — and name both so the caller can act. + duplicate = collisions.get(permalink) + if duplicate is not None: + raise AmbiguousMountError( + f"projects '{by_permalink[permalink].name}' and '{duplicate.name}' share the " + f"permalink '{permalink}', so that path names both. Rename one, or pass " + "project= with the exact name." + ) + + return by_permalink[permalink], remainder + + +async def _detect_workspace_qualified_route( + candidate: str, + config: BasicMemoryConfig, + context: Optional[Context] = None, +) -> tuple[str, str, str] | None: + """Resolve an explicitly qualified '/[/]' candidate. + + Returns the qualified project identifier, the project-relative remainder, + and the resolved project's external_id, or None when the candidate does not + spell a reachable workspace route. + + Both segments must match — the first an accessible workspace slug, the + second a project inside *that* workspace — so this never reaches a project + the caller did not name. Resolving only the first segment and searching + every accessible workspace for it is what let an ordinary project-relative + path ('notes/foo', where this session's workspace has no 'notes') route + into another tenant's same-named project (#1421). An unqualified first + segment now falls through to the refusal below, which names the mounts this + session can actually address. + + Workspace-qualified memory URLs require three segments so that + 'memory://main/notes' stays readable as project 'main'. A posix path only + reaches here after the mount table declined its leading segments, so nothing + addressable can be meant by it and the two-segment form unambiguously names + that project's root — without which 'ls acme/docs/notes' resolved while + 'ls acme/docs' (that same project's root) had no spelling at all. + """ + if _split_workspace_slug_prefix(candidate) is None: + return None + # The pathless root gets the same permission as the path form. The + # three-segment requirement in _workspace_identifier_discovery_available + # guards *identifier* detection, where a two-segment string is more likely + # an ordinary relative path than a workspace route. Here that judgement has + # already been made: the mount table declined these segments, no project was + # named, and several are addressable, so an unqualified path refuses anyway + # (see the precedence order). Keeping the stricter gate only made + # 'acme/docs/note' resolve while 'acme/docs' — that project's own root — + # refused, in a locally routed session holding cloud credentials. + if not _cloud_workspace_discovery_available(config) and not _local_cloud_discovery_allowed( + config + ): + return None + + try: + resolved = await _resolve_workspace_route(candidate, context=context) + except ValueError as exc: + if any(error in str(exc).lower() for error in _WORKSPACE_DISCOVERY_FALLBACK_ERRORS): + return None + raise + + if resolved is None: + return None + # Unlike the memory-URL caller, the pathless form is kept: a project root is + # a legitimate thing to list. The remainder comes from the same match that + # chose the project, so the route and the path it leaves behind can never + # disagree about how many segments were consumed. The external_id rides + # along for the same reason the mount table's does: this parse already knows + # the exact entry, and handing on only the qualified *name* would let it be + # re-resolved against a different workspace that happens to hold a project + # literally named '/'. + resolution, remainder = resolved + return resolution.project_identifier, remainder, resolution.entry.project.external_id + + +def _workspace_qualifies(qualified: str, bare: str) -> bool: + """True when ``qualified`` is ``bare`` with exactly one workspace slug in front. + + Asking "is this identifier workspace-qualified?" of a single string is not + answerable — a project name may contain '/', so 'Research/2026' and + 'acme/docs' have the same shape. Comparing two spellings of the *same* + project is answerable without any candidate set, because a workspace slug is + exactly one segment: the qualified spelling is the bare one plus exactly one + leading segment. That is the whole rule, and it is why 'acme/Research/2026' + qualifies 'Research/2026' while it does not qualify a project named '2026'. + """ + qualified_permalink = generate_permalink(qualified) + bare_permalink = generate_permalink(bare) + return qualified_permalink.endswith(f"/{bare_permalink}") and ( + qualified_permalink.count("/") - bare_permalink.count("/") == 1 + ) + + +def _agreed_route_project( + detected: str, + explicit: str, + addressable: tuple[AddressableProject, ...], +) -> str | None: + """The project both spellings name, or None when they name different projects. + + Identity first, spelling only as a last resort. When ``explicit`` names a + project this session addresses, the question is settled without looking at + the strings at all: the two either name the same addressable project or two + different ones, and two different ones are a conflict. Deciding that by + shape instead read 'team/docs' as a workspace-qualified spelling of 'docs' + and served the wrong project, silently, in a config where both exist. + + The shape comparison survives for exactly one case: ``explicit`` names no + addressable project, so it addresses another workspace. There is no local + identity to compare against — resolving it would take the workspace + discovery that rule 1 deliberately does not run when a project was named — + and a workspace slug is exactly one segment, so the qualified spelling is + the bare one plus exactly one leading segment. That is a genuine fallback + for identities this session cannot resolve, not a shortcut around ones it + can. + + Returns the more-qualified spelling, so an explicit '/' + outlives a bare prefix match: a local project can shadow a same-named + project in another workspace, and dropping the explicitly named workspace + would silently reroute the call to the local shadow. + """ + if generate_permalink(detected) == generate_permalink(explicit): + return detected + + # Both name projects this session can address, and they are not the same + # one — a contradiction in a single call, whatever their spellings suggest. + explicit_permalink = generate_permalink(explicit) + if any(project.permalink == explicit_permalink for project in addressable): + return None + + if _workspace_qualifies(explicit, detected): + return explicit + if _workspace_qualifies(detected, explicit): + return detected + return None + + +async def resolve_project_path_route( + path: str, + *, + project: Optional[str], + project_id: Optional[str], + context: Optional[Context] = None, +) -> ProjectPathRoute: + """Resolve a posix tool's path/identifier into an effective project route. + + First-segment project resolution (#1415), in order: + + 1. ``project_id`` (UUID) bypasses parsing entirely — comparing a path + prefix against a UUID would need an API round-trip. + 2. An explicit project (the ``BASIC_MEMORY_MCP_PROJECT`` constraint, else + the ``project`` param) wins: an agreeing path prefix is stripped, a + disagreeing one raises ProjectPrefixConflictError — never silently + preferring either. Agreement keeps the more-qualified spelling: an + explicit '/' outlives a bare local prefix match. + 3. Otherwise leading segments naming an addressable project route there + with the remainder as the project-relative path. + 4. Otherwise an explicitly workspace-qualified '/[/]' + spelling routes to that project in that workspace — those projects belong + to workspaces this session's own route does not list, so they never appear + in the mount table rule 3 reads. Both segments must match, so an + unqualified first segment never reaches another workspace. With no path it + names that project's root, the same way a bare mount name does. + 5. Otherwise, when the session addresses more than one project, raise + UnqualifiedPathRefusedError instead of silently defaulting; a session + that addresses at most one project keeps today's default resolution. + + Rules 3 and 5 read the set from ``addressable_projects`` — the same set + ``ls /`` advertises — so a project can never be listed at the root and then + go unrecognized as a path prefix (#1421). Rule 3 deliberately precedes rule + 4; see the mount-precedence note above this section for the collision that + ordering resolves and the spelling it costs. + """ + if project_id is not None: + return ProjectPathRoute(project=project, path=path, stripped=False, project_id=project_id) + + # The env constraint is ProjectResolver's priority 1, so it participates in + # agree/strip and conflict exactly like the param it outranks. + explicit = os.environ.get("BASIC_MEMORY_MCP_PROJECT") or project + config = ConfigManager().config + candidate = normalize_project_reference(_identifier_path(path)).strip("/") + + detected: Optional[str] = None + remainder = "" + mount_project_id: Optional[str] = None + addressable: tuple[AddressableProject, ...] | None = None + + # --- Rule 3: the advertised mount table claims the leading segments --- + # Trigger: the input carries a leading segment at all — a bare mount name + # ('ls research') or a path under one. + # Why: this set is what `ls /` advertises, and an advertised name that + # resolves somewhere else is worse than one never advertised. The workspace + # parse below would take '//...' first, so a project whose + # permalink is also an accessible workspace slug would hand 'team/docs/x' + # to project 'docs' in workspace 'team' instead of the mount named 'team' + # — reading another project's data under an advertised name. + # Outcome: a leading segment matching an addressable project routes there + # with the remainder as the project-relative path, and workspace discovery + # is never consulted for it. A local session pays nothing (its config is + # its mount table); a cloud session pays one per-request memoized listing. + if candidate: + addressable = await addressable_projects(context=context) + claimed = _claim_mount_prefix(candidate, addressable) + if claimed is not None: + mount, remainder = claimed + detected = mount.name + mount_project_id = mount.external_id + + # --- Rule 3b: the explicit project's own permalink, spelled as the prefix --- + # Trigger: a project was named, no mount claimed the segments, and the path + # begins with that same project's permalink. + # Why: rule 2 already strips a prefix that agrees with the explicit project; + # that only worked for spellings the mount table recognizes, so a + # workspace-qualified project reached it solely through rule 4's discovery + # — which the precedence order now (correctly) declines to run when a + # project was named. The agreement is decidable right here without any + # network: it is the caller's own two spellings of one project. + # Outcome: cat("acme/docs/foo", project="acme/docs") reads 'foo', and a path + # from a routed ls("acme/docs") round-trips with the project param set. + if detected is None and explicit is not None: + explicit_claim = _split_project_permalink_prefix(candidate, (generate_permalink(explicit),)) + if explicit_claim is not None: + detected, remainder = explicit, explicit_claim[1] + + # --- Rule 4: explicitly workspace-qualified spellings for everything else --- + # Trigger: no advertised mount claimed the leading segments, the input has + # more than one segment, and no project was named. + # Why: '/[/]' addresses projects in workspaces this + # session's own route does not list, so they are absent from the mount + # table above and would otherwise be unreachable. But 'acme/docs/foo' is + # equally a well-formed folder path inside the caller's own project, and + # nothing in the string or the project set recovers which was meant — see + # the route-versus-path precedence note above this section for why the + # two conditions on this line are the whole answer. + # Outcome: only a route naming BOTH an accessible workspace and a project + # inside it resolves here. An unqualified first segment falls through to + # the refusal below instead of being searched for across every accessible + # workspace — that search read another tenant's same-named project under + # an ordinary project-relative path (#1421). + if detected is None and "/" in candidate and explicit is None: + qualified = await _detect_workspace_qualified_route(candidate, config, context=context) + if qualified is not None: + detected, remainder, mount_project_id = qualified + + if explicit is not None: + if detected is None: + return ProjectPathRoute( + project=_canonicalize_project_name(explicit, config), path=path, stripped=False + ) + routed = _agreed_route_project(detected, explicit, addressable or ()) + if routed is not None: + # Trigger: the explicit spelling is workspace-qualified while the + # path prefix matched an unqualified local config name. + # Why: the explicitly named workspace must survive, or the call + # silently reroutes to a same-named local shadow. + # Outcome: when the explicit spelling wins it also drops the mount + # id that names this session's own workspace; every other + # agreement keeps the detected (canonical) spelling and stays + # bound to the mount that matched. + prefer_explicit = routed is explicit + return ProjectPathRoute( + project=_canonicalize_project_name(routed, config), + path=remainder, + stripped=True, + project_id=None if prefer_explicit else mount_project_id, + ) + raise ProjectPrefixConflictError( + f"path names project '{detected}' but project '{explicit}' was passed — " + f"use '{detected}/' alone, or project='{explicit}' with a " + "project-relative path" + ) + + if detected is not None: + return ProjectPathRoute( + project=_canonicalize_project_name(detected, config), + path=remainder, + stripped=True, + project_id=mount_project_id, + ) + + # Trigger: no explicit project, no recognized prefix, several addressable projects. + # Why: the stateless server would otherwise fall back to a default project — + # the measured multi-project failure (#1415) this refusal removes. In a + # team workspace that default is one shared mutable is_default flag, so a + # teammate flipping it silently redirects this call, writes included; the + # refusal is what keeps unqualified references from depending on it (#1421). + # Outcome: a self-teaching error listing every project in copyable prefix form. + if addressable is None: + addressable = await addressable_projects(context=context) + if len(addressable) > 1: + first_segment = candidate.split("/", 1)[0] if candidate else "" + subject = f"no project '{first_segment}'" if first_segment else "no project specified" + raise UnqualifiedPathRefusedError( + f"{subject} — active projects: {_addressable_project_prefixes(addressable)}" + ) + + # A session that addresses at most one project has no ambiguity to protect + # against, so unqualified input keeps today's default resolution. + return ProjectPathRoute(project=None, path=path, stripped=False) + + @asynccontextmanager async def get_project_client( project: Optional[str] = None, @@ -1144,10 +1710,13 @@ async def get_project_client( active_ws: WorkspaceInfo | None = None resolved_entry: WorkspaceProjectEntry | None = None workspace_id: str - project_for_api = _unqualified_project_identifier(resolved_project) if project_entry and project_entry.workspace_id: - # Per-project config stores the cloud tenant id directly + # Per-project config stores the cloud tenant id directly. The + # identifier came out of config.projects, so it is the project name + # verbatim — splitting a workspace off it would mangle a name that + # legitimately contains '/'. + project_for_api = resolved_project workspace_id = project_entry.workspace_id active_ws = await _workspace_metadata_by_tenant_id(workspace_id, context=context) else: diff --git a/src/basic_memory/mcp/project_context_identifiers.py b/src/basic_memory/mcp/project_context_identifiers.py index af1e33154..24ed01bdc 100644 --- a/src/basic_memory/mcp/project_context_identifiers.py +++ b/src/basic_memory/mcp/project_context_identifiers.py @@ -3,7 +3,7 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Optional +from typing import Iterable, Optional from basic_memory.config import BasicMemoryConfig from basic_memory.mcp.workspace_project_index import WorkspaceProjectEntry @@ -50,6 +50,15 @@ def canonicalize_project_name( if project_name is None: return None + # Exact before fuzzy, the same ordering the v2 project router and the + # workspace index use. It matters when two configured names share a + # permalink: matching by permalink alone would answer a caller who named one + # of them exactly with whichever the config happened to list first — the + # other project — which is precisely the escape hatch the ambiguity error + # tells them to use. + if project_name in config.projects: + return project_name + requested_permalink = generate_permalink(project_name) for configured_name in config.projects: if generate_permalink(configured_name) == requested_permalink: @@ -69,7 +78,16 @@ def project_matches_identifier(project_item: ProjectItem, identifier: Optional[s def split_qualified_project_identifier(identifier: str) -> tuple[str | None, str]: - """Split ``/`` identifiers for cloud routing.""" + """Split ``/`` identifiers for cloud routing. + + This guesses: a project name may contain '/', so 'acme/docs' and + 'Research/2026' are the same shape and no string can say which is which. + Call it only after an exact whole-identifier lookup has already missed, so + the guess is a fallback rather than the first reading — see + ``resolve_workspace_project_from_index`` and the v2 project router, which + both resolve exact-first for that reason. To compare two spellings of the + same project, use ``_workspace_qualifies`` instead, which needs no guess. + """ cleaned = identifier.strip() if "/" not in cleaned: return None, cleaned @@ -79,35 +97,77 @@ def split_qualified_project_identifier(identifier: str) -> tuple[str | None, str return workspace_slug, project_identifier -def unqualified_project_identifier(identifier: str) -> str: - """Return the project segment from an optional qualified identifier.""" - _, project_identifier = split_qualified_project_identifier(identifier) - return project_identifier - - def identifier_path(identifier: str) -> str: """Return the routable path portion of a raw identifier or memory URL.""" stripped = identifier.strip() return memory_url_path(stripped) if stripped.startswith("memory://") else stripped -def split_workspace_identifier_segments(identifier: str) -> tuple[str, str, str] | None: - """Split ``//`` identifiers into route segments.""" - normalized = normalize_project_reference(identifier_path(identifier)).strip("/") - parts = normalized.split("/", 2) - if len(parts) != 3: +def split_project_permalink_prefix( + path: str, + permalinks: Iterable[str], +) -> tuple[str, str] | None: + """Split a path into the project permalink it names and the remainder. + + This is the *only* way to turn a path-shaped input into a project plus a + project-relative path, and it deliberately takes the candidate set as an + argument: a project name may contain '/', and ``generate_permalink`` + preserves it, so how many leading segments a project consumes is a fact + about the known projects, never about the string. Callers that split on + the first '/' instead kept rediscovering that — for mounts (#1421), then + again for workspace routes. + + The longest permalink wins, which is also the only reading that can be + right when one project's permalink prefixes another's ('research' beside + 'research/2026'). Returns None when no permalink claims the leading + segments; the remainder has no leading slash and is "" at the root. + """ + segments = normalize_project_reference(identifier_path(path)).strip("/").split("/") + # An empty interior segment ('a//b') names nothing. Matching around it would + # repair a malformed path into a route the caller did not write. + if not all(segments): return None - workspace_slug, project_identifier, remainder = parts - if not workspace_slug or not project_identifier or not remainder: + + claimed: tuple[str, str] | None = None + claimed_depth = 0 + for permalink in permalinks: + depth = permalink.count("/") + 1 + if depth > len(segments) or depth <= claimed_depth: + continue + if generate_permalink("/".join(segments[:depth])) != permalink: + continue + claimed = (permalink, "/".join(segments[depth:])) + claimed_depth = depth + return claimed + + +def split_workspace_slug_prefix(identifier: str) -> tuple[str, str] | None: + """Split ``/`` — the workspace half of a qualified route. + + A workspace slug is always one segment, so this parse is pure shape. What + follows it is a project permalink plus a path, which only + ``split_project_permalink_prefix`` can separate. + """ + normalized = normalize_project_reference(identifier_path(identifier)).strip("/") + workspace_slug, _, rest = normalized.partition("/") + # A rest starting with '/' means the project segment is empty ('acme//notes'). + # The caller wrote nothing there, so this is not a workspace route. + if not workspace_slug or not rest or rest.startswith("/"): return None - return workspace_slug, project_identifier, remainder + return workspace_slug, rest -def split_workspace_memory_url_segments(identifier: str) -> tuple[str, str, str] | None: - """Split ``memory:////`` into route segments.""" - if not identifier.strip().startswith("memory://"): - return None - return split_workspace_identifier_segments(identifier) +def is_workspace_route_shaped(identifier: str) -> bool: + """True when an identifier has enough segments to spell workspace/project/path. + + A shape question only — it gates whether workspace discovery may be + consulted at all, and deliberately says nothing about where the project + ends. Three segments is the unmistakable form: fewer could equally be a + project-relative path in the session's own project. + """ + normalized = normalize_project_reference(identifier_path(identifier)).strip("/") + parts = normalized.split("/", 2) + return len(parts) == 3 and all(parts) def canonical_memory_path_for_workspace( @@ -203,11 +263,16 @@ def detect_project_from_url_prefix( """Return the local config project matching a memory URL path prefix.""" path = memory_url_path(identifier) if identifier.strip().startswith("memory://") else identifier normalized = normalize_project_reference(path) + # The '*' guard belongs to the glob case: a globbed first segment is search + # input, not a project prefix, and must not be matched against any project. prefix, _ = split_project_prefix(normalized) if prefix is None: return None - prefix_permalink = generate_permalink(prefix) - for project_name in config.projects: - if generate_permalink(project_name) == prefix_permalink: - return project_name - return None + + by_permalink = {generate_permalink(name): name for name in config.projects} + claimed = split_project_permalink_prefix(normalized, by_permalink) + # A prefix with no remainder names the project itself, not a path in it; + # split_project_prefix already rejected that shape above. + if claimed is None or not claimed[1]: + return None + return by_permalink[claimed[0]] diff --git a/src/basic_memory/mcp/tools/posix_tools.py b/src/basic_memory/mcp/tools/posix_tools.py index b40b8157f..d905a1361 100644 --- a/src/basic_memory/mcp/tools/posix_tools.py +++ b/src/basic_memory/mcp/tools/posix_tools.py @@ -6,8 +6,29 @@ ``basic_memory.mcp.server`` flips their visibility from the ``enable_posix_tools`` config flag at lifespan startup, so no tool body ever checks config itself. + +Projects are mount points (#1415): when no ``project``/``project_id`` param is +given, a path or identifier whose first segment names an addressable project +routes there, with the remainder as the project-relative path — inputs accept +exactly the '/path' identifiers tool outputs produce. An explicit +project param plus an agreeing prefix strips the prefix; a disagreeing one +refuses naming both. Where more than one project is addressable — several local +projects, or a cloud workspace holding several — an unrecognized first segment +refuses with the project list rather than silently defaulting (#1421); the +mount view and the resolver read one list, so anything ``ls "/"`` advertises is +addressable by name. The resolver answers with both routing fields — the project +and its external_id — and every tool hands that pair to ``get_project_client`` +verbatim, which is what keeps a cloud mount bound to the workspace whose listing +advertised it. + +Collision rule: the project always wins over a same-named top-level folder in +the default project, so that folder is only reachable unqualified when a single +project is addressable (where there is no ambiguity); the qualified +'/folder/...' form always reaches it. ``man`` is excluded — its +``project`` param names the manual project, not a data project. """ +import os from typing import Any, Optional from fastmcp import Context @@ -17,13 +38,101 @@ from basic_memory.man import bundled_pages, find_page, parse_page_ref, render_index from basic_memory.mcp.container import get_container from basic_memory.mcp.note_reads import read_note_json_by_external_id -from basic_memory.mcp.project_context import get_project_client +from basic_memory.mcp.project_context import ( + ProjectPathRoute, + addressable_projects, + get_project_client, + resolve_project_path_route, +) from basic_memory.mcp.server import POSIX_TOOLS_TAG, mcp, set_posix_tools_visibility from basic_memory.schemas.directory import ( DEFAULT_DIRECTORY_PAGE_SIZE, MAX_DIRECTORY_PAGE_SIZE, + DirectoryListResponse, + DirectoryNode, ) from basic_memory.schemas.search import SearchItemType, SearchQuery, SearchRetrievalMode +from basic_memory.utils import generate_permalink + +# --- Round-trip coherence --- +# A path a routed verb returns must be a path the resolver accepts. When a call +# addresses its project in the path ('ls research/notes'), the project prefix is +# stripped before the project-scoped API sees it, so that API answers in +# project-relative paths — '/notes'. Handing those back unchanged breaks the +# navigation loop the mount model promises: feeding '/notes' into `ls` refuses as +# unqualified, or worse, opens a *different* project that happens to be mounted +# as 'notes'. +# +# One rule decides *whether* and *with what* (_route_prefix); each response +# schema says *where*. That split is deliberate. Rewriting by key name anywhere +# in the payload also rewrote a note's own frontmatter when the author happened +# to use a `file_path:` key, so a routed `cat` returned frontmatter that +# disagreed with both its own `content` and the stored file. Transport metadata +# and note content are different things that can spell a key the same way, and +# only position tells them apart. + + +def _route_prefix(route: ProjectPathRoute) -> str | None: + """The prefix a routed response must re-attach, or None if nothing was stripped. + + None means the caller addressed the project some other way (an explicit + param, or a single-project session), so the project-relative paths it gets + back are already the ones it can feed back. + """ + if not route.stripped or route.project is None: + return None + # The permalink form is the spelling `ls "/"` advertises and the resolver + # normalizes to, so it round-trips for display names too ('My Research'). + return generate_permalink(route.project) + + +def _requalified_path(value: str, prefix: str) -> str: + """Re-attach a stripped project prefix, preserving the field's slash shape.""" + if value.startswith("/"): + return f"/{prefix}" if value == "/" else f"/{prefix}{value}" + return f"{prefix}/{value}" if value else prefix + + +def qualify_note_paths(payload: dict[str, Any], route: ProjectPathRoute) -> dict[str, Any]: + """Re-qualify a note payload's transport path. + + ``file_path`` is the note's address and is re-qualified. ``frontmatter`` is + the note's own YAML — canonical content that must come back byte for byte, + even when it carries keys named like transport fields — and ``permalink`` is + an identity with its own canonical form; neither is touched. + """ + prefix = _route_prefix(route) + if prefix is None: + return payload + return {**payload, "file_path": _requalified_path(payload["file_path"], prefix)} + + +def _requalified_directory_node(node: dict[str, Any], prefix: str) -> dict[str, Any]: + """Re-qualify one DirectoryNode's addressing fields, and its children.""" + requalified = dict(node) + directory_path = node.get("directory_path") + if isinstance(directory_path, str): + requalified["directory_path"] = _requalified_path(directory_path, prefix) + # file_path is Optional on DirectoryNode: directory rows carry no file. + file_path = node.get("file_path") + if isinstance(file_path, str): + requalified["file_path"] = _requalified_path(file_path, prefix) + children = node.get("children") + if children: + requalified["children"] = [_requalified_directory_node(child, prefix) for child in children] + return requalified + + +def qualify_listing_paths(payload: dict[str, Any], route: ProjectPathRoute) -> dict[str, Any]: + """Re-qualify a directory listing's node addressing fields.""" + prefix = _route_prefix(route) + if prefix is None: + return payload + return { + **payload, + "nodes": [_requalified_directory_node(node, prefix) for node in payload["nodes"]], + } + # The manual project holds the non-bundled manual pages as ordinary notes; # `man` falls back to it for page reads and searches it in query mode. @@ -38,7 +147,7 @@ @mcp.tool( title="Cat", - description="Print a note's content.", + description="Print a note's content. Accepts '/path' identifiers.", tags={POSIX_TOOLS_TAG, "notes"}, annotations={ "title": "Cat", @@ -61,7 +170,8 @@ async def cat( """Print a note's content, optionally sliced by line range, section, or token budget. Args: - identifier: Note title, permalink, or memory:// URL (resolved exactly). + identifier: Note title, permalink, memory:// URL, or '/path' + identifier (resolved exactly). start_line: First line to include (1-indexed, inclusive). end_line: Last line to include (inclusive). Defaults to the last line. section: Heading to slice to: "Decisions", path form "Auth/Decisions" @@ -124,7 +234,15 @@ async def cat( if server_side_slice and (start_line is not None or end_line is not None): lines_param = f"{start_line or 1}-{'' if end_line is None else end_line}" - async with get_project_client(project, context=context, project_id=project_id) as ( + # '/path' identifiers route to their project; route.path is the + # identifier unchanged when no prefix was recognized. + route = await resolve_project_path_route( + identifier, project=project, project_id=project_id, context=context + ) + if route.stripped and not route.path: + raise ValueError(f"cat: '{identifier}' names a project, not a note") + + async with get_project_client(route.project, context=context, project_id=route.project_id) as ( client, active_project, ): @@ -132,7 +250,7 @@ async def cat( from basic_memory.mcp.clients import KnowledgeClient, ResourceClient knowledge_client = KnowledgeClient(client, active_project.external_id) - entity_id = await knowledge_client.resolve_entity(identifier, strict=True) + entity_id = await knowledge_client.resolve_entity(route.path, strict=True) payload: dict[str, Any] = dict( await read_note_json_by_external_id( knowledge_client=knowledge_client, @@ -146,7 +264,7 @@ async def cat( ) if server_side_slice or (start_line is None and end_line is None): - return payload + return qualify_note_paths(payload, route) lines = str(payload["content"]).splitlines() total_lines = len(lines) @@ -156,7 +274,7 @@ async def cat( payload["start_line"] = first payload["end_line"] = last payload["total_lines"] = total_lines - return payload + return qualify_note_paths(payload, route) def _grep_retrieval_mode(literal: bool) -> SearchRetrievalMode: @@ -173,7 +291,7 @@ def _grep_retrieval_mode(literal: bool) -> SearchRetrievalMode: @mcp.tool( title="Grep", - description="Search note content for a pattern.", + description="Search note content for a pattern. Requires 'project' when several are addressable.", tags={POSIX_TOOLS_TAG, "search"}, annotations={ "title": "Grep", @@ -198,7 +316,7 @@ async def grep( literal: Force literal full-text matching instead of semantic search. page: Page number (1-indexed). page_size: Results per page. - project: Project name. Optional - the server resolves the default. + project: Project name. Required when more than one project is addressable. project_id: Project external_id (UUID); takes precedence over `project`. context: Optional FastMCP context. @@ -212,12 +330,19 @@ async def grep( if page_size < 1: raise ValueError(f"page_size must be >= 1, got {page_size}") + # grep's pattern is never parsed as a path — search text like "error/timeout" + # must not be mistaken for a mount. Routing participates for the refusal rule + # only: unqualified multi-project calls fail loudly instead of defaulting. + route = await resolve_project_path_route( + "", project=project, project_id=project_id, context=context + ) + query = SearchQuery( text=pattern, retrieval_mode=_grep_retrieval_mode(literal), entity_types=[SearchItemType.ENTITY], ) - async with get_project_client(project, context=context, project_id=project_id) as ( + async with get_project_client(route.project, context=context, project_id=route.project_id) as ( client, active_project, ): @@ -229,9 +354,39 @@ async def grep( return response.model_dump(mode="json", exclude_none=True) +async def _project_mount_listing( + *, page: int, page_size: int, context: Context | None +) -> dict[str, Any]: + """Render the addressable projects as directory entries (the mount-point view). + + Sources ``addressable_projects`` — the same set the path resolver routes by + — so every mount advertised here is reachable as '/path' (#1421). + Each row's ``directory_path`` is the copyable '/' prefix form, and + the set already arrives sorted by project name. + """ + rows = [ + DirectoryNode( + name=item.name, + directory_path=f"/{item.permalink}", + permalink=item.permalink, + type="directory", + ) + for item in await addressable_projects(context=context) + ] + start = (page - 1) * page_size + listing = DirectoryListResponse( + nodes=rows[start : start + page_size], + page=page, + page_size=page_size, + total=len(rows), + has_more=start + page_size < len(rows), + ) + return listing.model_dump(mode="json") + + @mcp.tool( title="Ls", - description="List one directory level.", + description="List one directory level. '/' lists projects; paths accept '/path'.", tags={POSIX_TOOLS_TAG, "navigation"}, annotations={ "title": "Ls", @@ -251,10 +406,14 @@ async def ls( """List the immediate contents of one directory. Args: - path: Directory path to list (default: project root). + path: Directory path to list. '/' (the default) with no project param + lists the active projects as mount points; '/path' routes + into that project. page: Page number (1-indexed). page_size: Nodes per page. - project: Project name. Optional - the server resolves the default. + project: Project name. Optional - '/' lists projects; qualified paths + route themselves; other unqualified paths refuse when several + projects are addressable. project_id: Project external_id (UUID); takes precedence over `project`. context: Optional FastMCP context. @@ -268,7 +427,24 @@ async def ls( if page_size > MAX_DIRECTORY_PAGE_SIZE: raise ValueError(f"page_size must be <= {MAX_DIRECTORY_PAGE_SIZE}, got {page_size}") - async with get_project_client(project, context=context, project_id=project_id) as ( + # Trigger: bare root with no project addressed (param, UUID, or env constraint). + # Why: the mount-point view puts project discovery in-band — ls "/" shows the + # mount table, ls "" shows that project's root (#1415). + # Outcome: list the active projects as directory entries; no project client. + if ( + project is None + and project_id is None + and not os.environ.get("BASIC_MEMORY_MCP_PROJECT") + and not path.strip().strip("/") + ): + return await _project_mount_listing(page=page, page_size=page_size, context=context) + + route = await resolve_project_path_route( + path, project=project, project_id=project_id, context=context + ) + list_path = f"/{route.path}" if route.stripped else path + + async with get_project_client(route.project, context=context, project_id=route.project_id) as ( client, active_project, ): @@ -276,13 +452,82 @@ async def ls( from basic_memory.mcp.clients import DirectoryClient directory_client = DirectoryClient(client, active_project.external_id) - listing = await directory_client.list(path, depth=1, page=page, page_size=page_size) - return listing.model_dump(mode="json") + listing = await directory_client.list(list_path, depth=1, page=page, page_size=page_size) + return qualify_listing_paths(listing.model_dump(mode="json"), route) + + +def routed_listing_root(path: str, route: ProjectPathRoute) -> str: + """The directory the returned paths are relative to, in *their* address space. + + Returned paths carry the project prefix whenever the call put it in the + path, so the root a caller strips to rebuild a hierarchy must carry it too — + and in the permalink spelling the payload uses, not the caller's ('My + Research' vs 'my-research'). + """ + if not route.stripped or route.project is None: + return path + prefix = generate_permalink(route.project) + return f"{prefix}/{route.path}" if route.path else prefix + + +async def find_listing( + path: str = "/", + *, + name: Optional[str] = None, + depth: int = _MAX_FIND_DEPTH, + page: int = 1, + page_size: int = DEFAULT_DIRECTORY_PAGE_SIZE, + project: Optional[str] = None, + project_id: Optional[str] = None, + context: Context | None = None, +) -> tuple[dict[str, Any], str]: + """find's body: the listing, plus the root its paths are relative to. + + `bm tree` needs both halves — the listing to render and the root to strip — + and resolving the path twice to get them cost a second project-list round + trip on every cloud CLI call, since CLI calls carry no FastMCP context for + the per-request cache to live in. One resolve now answers both. + """ + if depth < 1 or depth > _MAX_FIND_DEPTH: + raise ValueError(f"depth must be between 1 and {_MAX_FIND_DEPTH}, got {depth}") + if page < 1: + raise ValueError(f"page must be >= 1, got {page}") + if page_size < 1: + raise ValueError(f"page_size must be >= 1, got {page_size}") + if page_size > MAX_DIRECTORY_PAGE_SIZE: + raise ValueError(f"page_size must be <= {MAX_DIRECTORY_PAGE_SIZE}, got {page_size}") + + # The directory API is project-scoped, so cross-project find does not exist: + # find "/" with no project in a multi-project config refuses, teaching the + # per-project '/path' form instead. + route = await resolve_project_path_route( + path, project=project, project_id=project_id, context=context + ) + list_path = f"/{route.path}" if route.stripped else path + + async with get_project_client(route.project, context=context, project_id=route.project_id) as ( + client, + active_project, + ): + # Import here to avoid circular import + from basic_memory.mcp.clients import DirectoryClient + + directory_client = DirectoryClient(client, active_project.external_id) + listing = await directory_client.list( + list_path, + depth=depth, + file_name_glob=name, + page=page, + page_size=page_size, + ) + payload = qualify_listing_paths(listing.model_dump(mode="json"), route) + + return payload, routed_listing_root(path, route) @mcp.tool( title="Find", - description="Recursively list files matching a name glob.", + description="Recursively list files matching a name glob. Paths accept '/path'.", tags={POSIX_TOOLS_TAG, "navigation"}, annotations={ "title": "Find", @@ -304,48 +549,36 @@ async def find( """Recursively list files under a directory, optionally filtered by name glob. Args: - path: Directory to start from (default: project root). + path: Directory to start from (default: project root). '/path' + routes into that project. name: File-name glob to match, e.g. "*.md". None matches everything. depth: How many levels to recurse (1-10, default: 10). page: Page number (1-indexed). page_size: Nodes per page. - project: Project name. Optional - the server resolves the default. + project: Project name. Optional - qualified paths route themselves; + unqualified paths refuse when several projects are addressable. project_id: Project external_id (UUID); takes precedence over `project`. context: Optional FastMCP context. Returns: The directory listing as JSON: nodes, pagination, and totals. """ - if depth < 1 or depth > _MAX_FIND_DEPTH: - raise ValueError(f"depth must be between 1 and {_MAX_FIND_DEPTH}, got {depth}") - if page < 1: - raise ValueError(f"page must be >= 1, got {page}") - if page_size < 1: - raise ValueError(f"page_size must be >= 1, got {page_size}") - if page_size > MAX_DIRECTORY_PAGE_SIZE: - raise ValueError(f"page_size must be <= {MAX_DIRECTORY_PAGE_SIZE}, got {page_size}") - - async with get_project_client(project, context=context, project_id=project_id) as ( - client, - active_project, - ): - # Import here to avoid circular import - from basic_memory.mcp.clients import DirectoryClient - - directory_client = DirectoryClient(client, active_project.external_id) - listing = await directory_client.list( - path, - depth=depth, - file_name_glob=name, - page=page, - page_size=page_size, - ) - return listing.model_dump(mode="json") + listing, _ = await find_listing( + path, + name=name, + depth=depth, + page=page, + page_size=page_size, + project=project, + project_id=project_id, + context=context, + ) + return listing @mcp.tool( title="Tail", - description="Show recently changed notes.", + description="Show recently changed notes. Requires 'project' when several are addressable.", tags={POSIX_TOOLS_TAG, "navigation", "notes"}, annotations={ "title": "Tail", @@ -366,7 +599,7 @@ async def tail( Args: timeframe: Time window, e.g. "7d", "yesterday", "2 days ago". lines: Maximum number of rows to return (1-100). - project: Project name. Optional - the server resolves the default. + project: Project name. Required when more than one project is addressable. project_id: Project external_id (UUID); takes precedence over `project`. context: Optional FastMCP context. @@ -378,7 +611,13 @@ async def tail( if lines > _MAX_TAIL_LINES: raise ValueError(f"lines must be <= {_MAX_TAIL_LINES}, got {lines}") - async with get_project_client(project, context=context, project_id=project_id) as ( + # tail has no path to carry a project prefix, so routing participates for + # the refusal rule only: unqualified multi-project calls fail loudly. + route = await resolve_project_path_route( + "", project=project, project_id=project_id, context=context + ) + + async with get_project_client(route.project, context=context, project_id=route.project_id) as ( client, active_project, ): diff --git a/src/basic_memory/mcp/workspace_project_index.py b/src/basic_memory/mcp/workspace_project_index.py index f40c780f9..fddfffe13 100644 --- a/src/basic_memory/mcp/workspace_project_index.py +++ b/src/basic_memory/mcp/workspace_project_index.py @@ -214,11 +214,20 @@ def format_qualified_choices(entries: Sequence[WorkspaceProjectEntry]) -> str: return " or ".join(entry.qualified_name for entry in entries) -def match_workspace_identifier( +def find_workspace_identifier( workspaces: tuple[WorkspaceInfo, ...], workspace_identifier: str, -) -> WorkspaceInfo: - """Resolve a qualified route segment by slug, tenant_id, then display name.""" +) -> WorkspaceInfo | None: + """Resolve a qualified route segment by slug, tenant_id, then display name. + + Returns None when nothing matches, so a caller probing whether a segment + *is* a workspace can ask without catching. The precedence lives here only: + slugs win globally over tenant ids, which win over display names, so one + workspace's display name never shadows another's slug. A second matcher + that walked the list once and took the first field to hit would decide by + iteration order instead, and did — it sent a route naming a slug-owned + workspace to a whole-permalink project in a third workspace entirely. + """ slug_matches = [ workspace for workspace in workspaces @@ -247,7 +256,19 @@ def match_workspace_identifier( if name_matches: return name_matches[0] - available = ", ".join(workspace.slug for workspace in workspaces) + return None + + +def match_workspace_identifier( + workspaces: tuple[WorkspaceInfo, ...], + workspace_identifier: str, +) -> WorkspaceInfo: + """Resolve a qualified route segment, failing when nothing matches.""" + workspace = find_workspace_identifier(workspaces, workspace_identifier) + if workspace is not None: + return workspace + + available = ", ".join(item.slug for item in workspaces) raise ValueError( f"Workspace '{workspace_identifier}' was not found by slug, tenant_id, or name. " f"Available workspace slugs: {available}" @@ -270,7 +291,28 @@ async def resolve_workspace_project_from_index( from basic_memory.mcp.project_context_identifiers import split_qualified_project_identifier + # '/' and a project literally named 'acme/docs' are the + # same shape, and only the index can tell them apart — so try both readings, + # qualified first. Qualified wins because it is the spelling this module's + # own disambiguation errors teach, and because it names a workspace + # explicitly: reading it as some other workspace's whole permalink runs the + # call against a tenant the caller did not name. The whole-permalink reading + # is the fallback, which is what makes a slash-bearing project name routable + # at all rather than failing with "Workspace 'Research' was not found". workspace_identifier, project_identifier = split_qualified_project_identifier(project) + whole_permalink = generate_permalink(project) + qualified_workspace = ( + find_workspace_identifier(index.workspaces, workspace_identifier) + if workspace_identifier + else None + ) + qualified_hit = qualified_workspace is not None and any( + entry.workspace.tenant_id == qualified_workspace.tenant_id + for entry in index.entries_by_permalink.get(generate_permalink(project_identifier), ()) + ) + if not qualified_hit and whole_permalink in index.entries_by_permalink: + workspace_identifier, project_identifier = None, project + project_permalink = generate_permalink(project_identifier) if workspace_identifier: diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index 336744238..f8bff9b9c 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -223,8 +223,27 @@ async def add_project( set_default: Whether to set this project as the default Raises: - ValueError: If the project already exists or path collides with existing project + ValueError: If the project already exists, the name has no permalink, + or the path collides with an existing project """ + # Trigger: a name whose permalink has an empty segment — pure punctuation + # or emoji ('!!!', '💥') reduce to "", and a leading slash ('/foo') + # leaves an empty first segment. + # Why: the permalink is the project's address, and the resolver matches + # it segment by segment against a path whose leading slashes are + # already stripped. An empty segment means no path can ever match it: + # '' advertises at the root as '/', indistinguishable from every other + # such project, and '/foo' advertises '//foo' and cannot be entered. + # Either way the mount view lists something unaddressable (#1421). + # Outcome: refused at the one boundary that creates projects, so an + # unaddressable mount cannot exist rather than being handled downstream. + if not all(generate_permalink(name).split("/")): + raise ValueError( + f"Project name '{name}' has no usable permalink. Names need at least one " + "letter, digit, or CJK character in every path segment, and may not start " + "with '/', so the project has an address." + ) + # If project_root is set, constrain all projects to that directory project_root = self.config_manager.config.project_root sanitized_name = None @@ -251,6 +270,24 @@ async def add_project( async with db.scoped_session(self.session_maker) as session: existing_projects = await self.repository.find_all(session, use_load_options=False) + + # Trigger: a different name that normalizes to an existing project's + # permalink ('My Docs' beside 'my-docs'). + # Why: the permalink is the address, so two of them are one address + # for two projects. The mount view advertises both at the same + # path and the resolver can only pick one, leaving the other + # unreachable and its paths reading the wrong project's content. + # Outcome: refused here, where the second one would be created. + name_permalink = generate_permalink(name) + for existing in existing_projects: + if existing.name != name and generate_permalink(existing.name) == name_permalink: + raise ValueError( + f"Project name '{name}' has the same permalink as existing project " + f"'{existing.name}' ('{name_permalink}'). Project permalinks are " + "addresses and must be unique; choose a name that differs by more " + "than case, spacing, or punctuation." + ) + if project_root: # Check for case-insensitive path collisions with existing projects for existing in existing_projects: diff --git a/tests/cli/test_cli_posix_verbs.py b/tests/cli/test_cli_posix_verbs.py index 03dda4601..e209ce55b 100644 --- a/tests/cli/test_cli_posix_verbs.py +++ b/tests/cli/test_cli_posix_verbs.py @@ -12,13 +12,18 @@ import json import os -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest from fastmcp.exceptions import ToolError from typer.testing import CliRunner from basic_memory.cli.main import app as cli_app +from basic_memory.config_models import ProjectEntry +from basic_memory.mcp.project_context import ( + ProjectPrefixConflictError, + UnqualifiedPathRefusedError, +) runner = CliRunner() @@ -139,6 +144,23 @@ def _dir_node(**overrides): LS_RESULT_MORE = {**LS_RESULT, "has_more": True} +# The mount-point view (#1415): ls "/" with no project addressed returns the +# active projects as bare directory nodes whose directory_path is the copyable +# '/' prefix form. Same DirectoryListResponse contract as any listing, +# so the existing renderers must handle it untouched. +LS_MOUNT_RESULT = { + "nodes": [ + _dir_node(name="main", directory_path="/main", permalink="main", type="directory"), + _dir_node( + name="research", directory_path="/research", permalink="research", type="directory" + ), + ], + "page": 1, + "page_size": 10, + "total": 2, + "has_more": False, +} + FIND_RESULT = { "nodes": [ _dir_node(name="specs", directory_path="/specs", type="directory"), @@ -256,16 +278,33 @@ def _assert_not_json(output: str) -> None: json.loads(output) -# Every verb with its patch target and a realistic payload. head shares cat's -# tool and payload; tree shares find's — their JSON contracts are identical. +# Every verb with its patch target, the payload it must print, and what the +# patched callable returns. head shares cat's tool and payload; tree's JSON +# contract is still find's, but it calls find_listing, which hands back the +# listing and the root its paths are relative to from one resolution (#1421). +FIND_LISTING_TARGET = "basic_memory.mcp.tools.posix_tools.find_listing" VERB_CASES = [ - pytest.param(["cat", "specs/search"], "basic_memory.mcp.tools.cat", CAT_RESULT, id="cat"), - pytest.param(["head", "specs/search"], "basic_memory.mcp.tools.cat", CAT_RESULT, id="head"), - pytest.param(["grep", "ranking"], "basic_memory.mcp.tools.grep", GREP_RESULT, id="grep"), - pytest.param(["ls", "/specs"], "basic_memory.mcp.tools.ls", LS_RESULT, id="ls"), - pytest.param(["find", "/specs"], "basic_memory.mcp.tools.find", FIND_RESULT, id="find"), - pytest.param(["tail"], "basic_memory.mcp.tools.tail", TAIL_RESULT, id="tail"), - pytest.param(["tree", "/specs"], "basic_memory.mcp.tools.find", TREE_RESULT, id="tree"), + pytest.param( + ["cat", "specs/search"], "basic_memory.mcp.tools.cat", CAT_RESULT, CAT_RESULT, id="cat" + ), + pytest.param( + ["head", "specs/search"], "basic_memory.mcp.tools.cat", CAT_RESULT, CAT_RESULT, id="head" + ), + pytest.param( + ["grep", "ranking"], "basic_memory.mcp.tools.grep", GREP_RESULT, GREP_RESULT, id="grep" + ), + pytest.param(["ls", "/specs"], "basic_memory.mcp.tools.ls", LS_RESULT, LS_RESULT, id="ls"), + pytest.param( + ["find", "/specs"], "basic_memory.mcp.tools.find", FIND_RESULT, FIND_RESULT, id="find" + ), + pytest.param(["tail"], "basic_memory.mcp.tools.tail", TAIL_RESULT, TAIL_RESULT, id="tail"), + pytest.param( + ["tree", "/specs"], + FIND_LISTING_TARGET, + TREE_RESULT, + (TREE_RESULT, "/specs"), + id="tree", + ), ] @@ -274,10 +313,10 @@ def _assert_not_json(output: str) -> None: # --------------------------------------------------------------------------- -@pytest.mark.parametrize(("args", "target", "payload"), VERB_CASES) -def test_verb_non_tty_outputs_tool_payload_verbatim(args, target, payload): +@pytest.mark.parametrize(("args", "target", "payload", "tool_return"), VERB_CASES) +def test_verb_non_tty_outputs_tool_payload_verbatim(args, target, payload, tool_return): """Piped output is the MCP tool's return value, unchanged (auto-JSON).""" - with patch(target, new_callable=AsyncMock, return_value=payload) as mock_tool: + with patch(target, new_callable=AsyncMock, return_value=tool_return) as mock_tool: result = _invoke(args) assert result.exit_code == 0, result.output @@ -285,10 +324,10 @@ def test_verb_non_tty_outputs_tool_payload_verbatim(args, target, payload): mock_tool.assert_called_once() -@pytest.mark.parametrize(("args", "target", "payload"), VERB_CASES) -def test_verb_json_flag_overrides_tty(args, target, payload): +@pytest.mark.parametrize(("args", "target", "payload", "tool_return"), VERB_CASES) +def test_verb_json_flag_overrides_tty(args, target, payload, tool_return): """--json wins over the interactive renderer on a TTY.""" - with patch(target, new_callable=AsyncMock, return_value=payload): + with patch(target, new_callable=AsyncMock, return_value=tool_return): result = _tty_invoke([*args, "--json"]) assert result.exit_code == 0, result.output @@ -598,6 +637,29 @@ def test_ls_defaults_and_paging_passthrough(mock_ls): assert mock_ls.call_args.kwargs["page_size"] == 50 +@patch("basic_memory.mcp.tools.ls", new_callable=AsyncMock, return_value=LS_MOUNT_RESULT) +def test_ls_plain_mount_view_prints_copyable_prefixes(mock_ls): + """bm ls with no project emits one '//' line per mount, so the + prefix form the routing rules teach is copyable straight from the output.""" + result = _tty_invoke(["ls", "--plain"]) + + assert result.exit_code == 0, result.output + assert result.stdout == "/main/\n/research/\n" + + +@patch("basic_memory.mcp.tools.ls", new_callable=AsyncMock, return_value=LS_MOUNT_RESULT) +def test_ls_rich_mount_view_renders_bare_directory_nodes(mock_ls): + """Mount rows carry no title/updated_at; the rich renderer shows them fine.""" + result = _tty_invoke(["ls"]) + + flat = _flattened(result.output) + assert result.exit_code == 0, result.output + _assert_not_json(result.output) + assert "main/" in result.output + assert "research/" in result.output + assert "total 2" in flat + + # --------------------------------------------------------------------------- # find # --------------------------------------------------------------------------- @@ -692,7 +754,7 @@ def test_tail_lines_and_timeframe_passthrough(mock_tail): # --------------------------------------------------------------------------- -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT, "/")) def test_tree_plain_rebuilds_nesting_from_flat_nodes(mock_find): """The API page is flat; nesting is rebuilt from directory_path segments, including a synthesized parent for files whose directory node was filtered @@ -703,7 +765,7 @@ def test_tree_plain_rebuilds_nesting_from_flat_nodes(mock_find): assert result.stdout == "/\n specs/\n search.md\n auth/\n deep.md\n" -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT, "/")) def test_tree_rich_output(mock_find): result = _tty_invoke(["tree"]) @@ -714,7 +776,7 @@ def test_tree_rich_output(mock_find): assert "deep.md" in result.output -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_ROOTED_RESULT) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_ROOTED_RESULT, "/specs")) def test_tree_rooted_path_skips_the_root_node(mock_find): """The search root's own node must not render as its own child.""" result = _tty_invoke(["tree", "/specs", "--plain"]) @@ -723,11 +785,7 @@ def test_tree_rooted_path_skips_the_root_node(mock_find): assert result.stdout == "/specs\n intro.md\n" -@patch( - "basic_memory.mcp.tools.find", - new_callable=AsyncMock, - return_value=TREE_DIR_AFTER_FILE_RESULT, -) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_DIR_AFTER_FILE_RESULT, "/")) def test_tree_directory_node_after_synthesized_intermediate(mock_find): """A directory listed after a file already implied it stays one directory.""" result = _tty_invoke(["tree", "--plain"]) @@ -736,7 +794,7 @@ def test_tree_directory_node_after_synthesized_intermediate(mock_find): assert result.stdout == "/\n specs/\n search.md\n" -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT_MORE) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT_MORE, "/")) def test_tree_rich_reports_more_entries(mock_find): result = _tty_invoke(["tree"]) @@ -744,7 +802,7 @@ def test_tree_rich_reports_more_entries(mock_find): assert "more entries" in _flattened(result.output) -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT_MORE) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT_MORE, "/")) def test_tree_plain_more_entries_note_goes_to_stderr(mock_find): result = _tty_invoke(["tree", "--plain"]) @@ -753,7 +811,7 @@ def test_tree_plain_more_entries_note_goes_to_stderr(mock_find): assert "more entries" in result.stderr -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT_EMPTY) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT_EMPTY, "/")) def test_tree_rich_empty(mock_find): result = _tty_invoke(["tree"]) @@ -761,7 +819,7 @@ def test_tree_rich_empty(mock_find): assert "empty" in result.output -@patch("basic_memory.mcp.tools.find", new_callable=AsyncMock, return_value=TREE_RESULT) +@patch(FIND_LISTING_TARGET, new_callable=AsyncMock, return_value=(TREE_RESULT, "/")) def test_tree_passes_find_arguments_through(mock_find): result = _invoke(["tree", "/specs", "--name", "*.md", "--depth", "2"]) @@ -771,13 +829,73 @@ def test_tree_passes_find_arguments_through(mock_find): assert mock_find.call_args.kwargs["depth"] == 2 +# A qualified tree root gets back node paths that carry the project prefix, and +# the root to strip carries it too — both from the one resolution (#1421). The +# --project spelling is the other addressing frame: project-relative throughout. +TREE_QUALIFIED_RESULT = { + "nodes": [ + _dir_node(name="notes", directory_path="/second-project/notes", type="directory"), + _dir_node( + name="foo.md", + file_path="second-project/notes/foo.md", + directory_path="/second-project/notes/foo.md", + ), + ], + "page": 1, + "page_size": 10, + "total": 2, + "has_more": False, +} +TREE_RELATIVE_RESULT = { + **TREE_QUALIFIED_RESULT, + "nodes": [ + _dir_node(name="notes", directory_path="/notes", type="directory"), + _dir_node(name="foo.md", file_path="notes/foo.md", directory_path="/notes/foo.md"), + ], +} + + +@patch( + FIND_LISTING_TARGET, + new_callable=AsyncMock, + side_effect=[ + (TREE_QUALIFIED_RESULT, "second-project/notes"), + (TREE_RELATIVE_RESULT, "/notes"), + ], +) +def test_tree_qualified_path_matches_project_flag_hierarchy( + mock_find, config_manager, tmp_path_factory +): + """'bm tree /dir' and 'bm tree /dir --project ' are the + same call by rule 2, so they must render the same hierarchy — each stripping + the root in its own addressing frame, or the project segment duplicates + under the root (#1415).""" + config = config_manager.load_config() + config.projects["second-project"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("second-project-cli")) + ) + config_manager.save_config(config) + + qualified = _tty_invoke(["tree", "second-project/notes", "--plain"]) + flagged = _tty_invoke(["tree", "/notes", "--project", "second-project", "--plain"]) + + assert qualified.exit_code == 0, qualified.output + assert flagged.exit_code == 0, flagged.output + assert qualified.stdout == "second-project/notes\n foo.md\n" + # Only the printed root label may differ between the equivalent spellings. + assert qualified.stdout.splitlines()[1:] == flagged.stdout.splitlines()[1:] + # The tool still receives the caller's spelling; routing stays in the tool layer. + assert mock_find.call_args_list[0].args == ("second-project/notes",) + assert mock_find.call_count == 2 + + # --------------------------------------------------------------------------- # Errors and routing (shared command scaffold, exercised per verb) # --------------------------------------------------------------------------- -@pytest.mark.parametrize(("args", "target", "payload"), VERB_CASES) -def test_verb_tool_error_exits_nonzero(args, target, payload): +@pytest.mark.parametrize(("args", "target", "payload", "tool_return"), VERB_CASES) +def test_verb_tool_error_exits_nonzero(args, target, payload, tool_return): """A strict-resolve miss (ToolError) becomes stderr + exit 1, per verb.""" with patch(target, new_callable=AsyncMock, side_effect=ToolError("Entity not found: nope")): result = _invoke(args) @@ -799,6 +917,41 @@ def test_verb_value_error_exits_nonzero(mock_cat): assert "Error: start_line must be >= 1" in result.output +@patch( + "basic_memory.mcp.tools.ls", + new_callable=AsyncMock, + side_effect=UnqualifiedPathRefusedError( + "no project 'notes' — active projects: main/, research/" + ), +) +def test_verb_refusal_reaches_stderr_with_exit_1(mock_ls): + """The multi-project refusal (#1415) is a ValueError subclass, so the + existing verb error mapping delivers its self-teaching message unchanged.""" + result = _invoke(["ls", "/notes"]) + + assert result.exit_code == 1 + assert "Error: no project 'notes' — active projects: main/, research/" in result.stderr + + +@patch( + "basic_memory.mcp.tools.cat", + new_callable=AsyncMock, + side_effect=ProjectPrefixConflictError( + "path names project 'research' but project 'main' was passed — use " + "'research/' alone, or project='main' with a project-relative path" + ), +) +def test_project_flag_with_conflicting_prefix_exits_nonzero(mock_cat): + """--project plus a qualified path passes both through verbatim; the tool + layer owns the conflict decision and the CLI just reports it.""" + result = _invoke(["cat", "research/notes/foo", "--project", "main"]) + + assert result.exit_code == 1 + assert mock_cat.call_args.args == ("research/notes/foo",) + assert mock_cat.call_args.kwargs["project"] == "main" + assert "Error: path names project 'research' but project 'main' was passed" in result.stderr + + @patch("basic_memory.mcp.tools.cat", new_callable=AsyncMock, return_value=CAT_RESULT) def test_local_and_cloud_together_errors(mock_cat): result = _invoke(["cat", "specs/search", "--local", "--cloud"]) @@ -822,3 +975,28 @@ async def capture(*args, **kwargs): assert result.exit_code == 0, result.output assert seen["force_local"] == "true" + + +@patch( + FIND_LISTING_TARGET, + new_callable=AsyncMock, + return_value=(TREE_QUALIFIED_RESULT, "second-project/notes"), +) +def test_tree_routes_the_path_exactly_once(mock_find): + """tree gets the listing and the root from one resolution, so it must not + resolve the path itself. + + A CLI invocation carries no FastMCP context, so the per-request project-list + cache has nowhere to live: a second resolution is a second project-list + round trip on every cloud call, and for a workspace-qualified path a second + workspace/project index build (#1421). Patching the resolver to explode is + the check — if tree still reaches for it, this fails loudly. + """ + boom = Mock(side_effect=AssertionError("tree resolved the path a second time")) + with patch("basic_memory.mcp.project_context.resolve_project_path_route", boom): + result = _tty_invoke(["tree", "second-project/notes", "--plain"]) + + assert result.exit_code == 0, result.output + assert result.stdout == "second-project/notes\n foo.md\n" + mock_find.assert_called_once() + boom.assert_not_called() diff --git a/tests/mcp/conftest.py b/tests/mcp/conftest.py index 7d53b946c..40577b239 100644 --- a/tests/mcp/conftest.py +++ b/tests/mcp/conftest.py @@ -9,8 +9,12 @@ from fastmcp import FastMCP from httpx import AsyncClient, ASGITransport +from basic_memory import db from basic_memory.api.app import app as fastapi_app +from basic_memory.config_models import ProjectEntry from basic_memory.deps import get_engine_factory, get_app_config +from basic_memory.models.project import Project +from basic_memory.repository.project_repository import ProjectRepository from basic_memory.services.search_service import SearchService from basic_memory.mcp.server import mcp as mcp_server @@ -90,3 +94,33 @@ def test_entity_data(): async def init_search_index(search_service: SearchService): """Initialize search index. Request this fixture explicitly in tests that need it.""" await search_service.init_search_index() + + +@pytest_asyncio.fixture +async def second_project(config_manager, engine_factory, tmp_path_factory) -> Project: + """A second active project (DB row + config entry) for multi-project routing tests. + + Both halves matter: the API resolves projects from the database, while the + MCP client layer routes by the local config — a project absent from config + routes CLOUD by default and dies without credentials. The path lives outside + config_home because test-project's path IS config_home; nesting one project + inside another would make its files ambiguous between the two. + """ + project_path = tmp_path_factory.mktemp("second-project-home") + _, session_maker = engine_factory + async with db.scoped_session(session_maker) as session: + project = await ProjectRepository().create( + session, + { + "name": "second-project", + "description": "Second project for multi-project routing tests", + "path": str(project_path), + "is_active": True, + "is_default": False, + }, + ) + + config = config_manager.load_config() + config.projects["second-project"] = ProjectEntry(path=str(project_path)) + config_manager.save_config(config) + return project diff --git a/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index 431b4ffa3..4a98fb6ff 100644 --- a/tests/mcp/test_project_context.py +++ b/tests/mcp/test_project_context.py @@ -1055,6 +1055,70 @@ async def fake_index(context=None, force_refresh=False): ) +@pytest.mark.asyncio +async def test_detect_project_from_identifier_prefix_falls_back_to_bare_project_name( + monkeypatch, +): + """An identifier that is not workspace-qualified still resolves its first + segment by searching every accessible workspace for a project of that name. + + This is the legacy detector read_note and search share. The posix path + resolver deliberately does NOT use it: a bare first segment that names no + addressable mount must refuse rather than reach another workspace's + same-named project (#1421), so its rule 4 parses only fully qualified + '/[/]' routes. Pinned here so the behavior these + two tools still depend on is exercised on its own terms. + """ + import basic_memory.mcp.project_context as project_context + from basic_memory.config import BasicMemoryConfig + from basic_memory.mcp.project_context import ( + WorkspaceProjectEntry, + _build_workspace_project_index, + detect_project_from_identifier_prefix, + ) + + personal = _workspace( + tenant_id="personal-tenant", + workspace_type="personal", + slug="personal", + name="Personal", + role="owner", + is_default=True, + ) + index = _build_workspace_project_index( + (personal,), + ( + WorkspaceProjectEntry( + workspace=personal, + project=_project("notes", id=1, external_id="personal-notes-id"), + ), + ), + ) + + async def fake_index(context=None, force_refresh=False): + return index + + monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index) + # A hosted (factory) session is what makes workspace discovery available to + # an identifier that is not itself workspace-qualified. + monkeypatch.setattr("basic_memory.mcp.async_client.is_factory_mode", lambda: True) + + # BasicMemoryConfig always materializes a placeholder 'main' entry, so the + # local-prefix check ahead of the fallback sees a config that names no 'notes'. + config = BasicMemoryConfig(projects={}, cloud_api_key="bmc_test123") + + # 'notes/...' names no workspace, so the qualified parse declines and the + # bare first segment is resolved across workspaces instead. + assert ( + await detect_project_from_identifier_prefix("notes/to-do-list", config) == "personal/notes" + ) + + # A first segment that names no project anywhere is an ordinary path, not a + # route: the lookup miss stays best-effort and leaves the identifier unrouted + # rather than becoming the caller's error. + assert await detect_project_from_identifier_prefix("absent/to-do-list", config) is None + + @pytest.mark.asyncio async def test_resolve_workspace_qualified_memory_url_ignores_workspace_project_miss( monkeypatch, diff --git a/tests/mcp/test_project_path_routing.py b/tests/mcp/test_project_path_routing.py new file mode 100644 index 000000000..1f3d0411b --- /dev/null +++ b/tests/mcp/test_project_path_routing.py @@ -0,0 +1,1580 @@ +"""Direct unit tests for resolve_project_path_route (#1415, #1421). + +The resolver is the single routing seam for the posix tools: '/path' +inputs route by first segment, an explicit project must agree with a path +prefix, and unqualified input refuses instead of defaulting whenever more than +one project is addressable. The local-config tests drive the function +branch-by-branch against configs written through the test ConfigManager; no API +client is involved there because local-config matching never leaves the +process. The cloud section at the bottom stands up a factory-mode session, +where the project list comes from the tenant instead. +""" + +import re +from contextlib import asynccontextmanager +from dataclasses import dataclass +from typing import AsyncIterator, Optional + +import pytest + +import basic_memory.mcp.async_client as async_client +import basic_memory.mcp.project_context as project_context +from basic_memory.config_models import ProjectEntry +from basic_memory.mcp.project_context import ( + AddressableProject, + ProjectPathRoute, + ProjectPrefixConflictError, + AmbiguousMountError, + UnqualifiedPathRefusedError, + _agreed_route_project, + resolve_project_path_route, + resolve_workspace_project_identifier, + resolve_workspace_qualified_identifier, + resolve_workspace_qualified_memory_url, +) +from basic_memory.mcp.project_context_identifiers import ( + split_project_permalink_prefix, +) +from basic_memory.mcp.tools import grep, ls +from basic_memory.mcp.workspace_project_index import ( + WorkspaceProjectEntry, + WorkspaceProjectIndex, + build_workspace_project_index, + resolve_workspace_project_from_index, +) +from basic_memory.schemas.cloud import WorkspaceInfo +from basic_memory.schemas.project_info import ProjectItem, ProjectList +from basic_memory.utils import generate_permalink +from tests.mcp.conftest import ContextState, ctx + + +@pytest.fixture(autouse=True) +def clean_project_env(monkeypatch): + """The env constraint acts as an explicit project; clear it by default.""" + monkeypatch.delenv("BASIC_MEMORY_MCP_PROJECT", raising=False) + + +@pytest.fixture +def multi_project_config(config_manager, tmp_path_factory): + """Three local projects, one with a spaced display name for permalink tests.""" + config = config_manager.load_config() + config.projects["second-project"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("second-project-config")) + ) + config.projects["My Research"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("my-research-config")) + ) + config_manager.save_config(config) + return config_manager + + +@pytest.fixture +def empty_project_config(config_manager): + """A config with no projects at all (cloud-only local client).""" + config = config_manager.load_config() + config.projects = {} + config_manager.save_config(config) + return config_manager + + +# --- project_id passthrough --- + + +@pytest.mark.asyncio +async def test_project_id_bypasses_prefix_parsing(multi_project_config): + """project_id routes by external UUID, so even a conflicting-looking prefix + is never examined — documented limitation, mirroring read_note. The route + echoes the caller's id so tool call sites can pass the route alone.""" + route = await resolve_project_path_route( + "second-project/notes/foo", + project="test-project", + project_id="11111111-1111-1111-1111-111111111111", + ) + + assert route == ProjectPathRoute( + project="test-project", + path="second-project/notes/foo", + stripped=False, + project_id="11111111-1111-1111-1111-111111111111", + ) + + +# --- rule 1: first-segment routing --- + + +@pytest.mark.asyncio +async def test_first_segment_routes_with_remainder(multi_project_config): + route = await resolve_project_path_route( + "second-project/notes/foo", project=None, project_id=None + ) + + assert route == ProjectPathRoute(project="second-project", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_single_segment_project_names_project_root(multi_project_config): + """A bare project name routes with an empty remainder — ls 'second-project' + lists that project's root.""" + route = await resolve_project_path_route("second-project", project=None, project_id=None) + + assert route == ProjectPathRoute(project="second-project", path="", stripped=True) + + +@pytest.mark.asyncio +async def test_memory_url_prefix_routes(multi_project_config): + route = await resolve_project_path_route( + "memory://second-project/notes/foo", project=None, project_id=None + ) + + assert route == ProjectPathRoute(project="second-project", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_namespace_syntax_routes(multi_project_config): + """'project::note' normalizes to path syntax before prefix detection.""" + route = await resolve_project_path_route( + "second-project::notes/foo", project=None, project_id=None + ) + + assert route == ProjectPathRoute(project="second-project", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_display_name_matches_by_permalink(multi_project_config): + """The permalink form routes to the canonical config name, spaces and all.""" + route = await resolve_project_path_route("my-research/notes/foo", project=None, project_id=None) + + assert route == ProjectPathRoute(project="My Research", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_multi_segment_project_permalink_routes(config_manager, tmp_path_factory): + """Project names may contain '/', and generate_permalink keeps it, so a + mount's permalink can span several segments. Comparing only the first + segment advertised '/research/2026' at the root and then could not enter it; + the longest matching permalink wins, so the nested name beats its own + prefix.""" + config = config_manager.load_config() + config.projects["Research"] = ProjectEntry(path=str(tmp_path_factory.mktemp("research"))) + config.projects["Research/2026"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("research-2026")) + ) + config_manager.save_config(config) + + root = await resolve_project_path_route("research/2026", project=None, project_id=None) + assert root == ProjectPathRoute(project="Research/2026", path="", stripped=True) + + nested = await resolve_project_path_route( + "research/2026/notes/x", project=None, project_id=None + ) + assert nested == ProjectPathRoute(project="Research/2026", path="notes/x", stripped=True) + + # The one-segment mount still claims paths its longer sibling does not. + shallow = await resolve_project_path_route("research/notes/x", project=None, project_id=None) + assert shallow == ProjectPathRoute(project="Research", path="notes/x", stripped=True) + + +@pytest.mark.asyncio +async def test_multi_segment_mount_agrees_with_explicit_workspace_spelling( + config_manager, tmp_path_factory +): + """The explicit-workspace escape hatch has to survive slash-bearing names. + + With mount 'Research/2026' detected and project='acme/Research/2026' passed, + inferring qualification from the first slash read the detected mount as + workspace 'Research' plus project '2026', so two agreeing spellings were + rejected as a conflict. Segment count settles it without guessing. + """ + config = config_manager.load_config() + config.projects["Research/2026"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("research-2026-agree")) + ) + config_manager.save_config(config) + + route = await resolve_project_path_route( + "research/2026/notes/x", project="acme/Research/2026", project_id=None + ) + + # The explicitly named workspace survives, and the mount id goes with it. + assert route == ProjectPathRoute( + project="acme/Research/2026", path="notes/x", stripped=True, project_id=None + ) + + +@pytest.mark.asyncio +async def test_colliding_mount_permalinks_refuse_rather_than_pick(config_manager, tmp_path_factory): + """Two names that normalize to one permalink are one address for two + projects, and the resolver must not choose. + + add_project refuses this now, so only a config written before that check or + edited by hand can reach it — but silently keeping whichever sorted last + read that project's content under the other's name, which is worse than a + loud failure naming both. + """ + config = config_manager.load_config() + config.projects["My Docs"] = ProjectEntry(path=str(tmp_path_factory.mktemp("my-docs-a"))) + config.projects["my-docs"] = ProjectEntry(path=str(tmp_path_factory.mktemp("my-docs-b"))) + config_manager.save_config(config) + + with pytest.raises(AmbiguousMountError) as excinfo: + await resolve_project_path_route("my-docs/notes", project=None, project_id=None) + + assert "My Docs" in str(excinfo.value) + assert "my-docs" in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_colliding_mounts_only_fail_the_paths_that_name_them( + config_manager, tmp_path_factory +): + """An ambiguity fails the calls that depend on it, and no others. + + Raising while building the lookup table rejected every non-empty path in the + session, so an unrelated project became unreachable and the exact-name + escape hatch the error itself recommends did not work — that call never goes + through the ambiguous lookup at all. + """ + config = config_manager.load_config() + config.projects["My Docs"] = ProjectEntry(path=str(tmp_path_factory.mktemp("dup-a"))) + config.projects["my-docs"] = ProjectEntry(path=str(tmp_path_factory.mktemp("dup-b"))) + config.projects["Other"] = ProjectEntry(path=str(tmp_path_factory.mktemp("dup-other"))) + config_manager.save_config(config) + + # A path claimed by an unrelated mount is unaffected. + unrelated = await resolve_project_path_route("other/note", project=None, project_id=None) + assert unrelated == ProjectPathRoute(project="Other", path="note", stripped=True) + + # So is the escape hatch, for the unrelated project... + hatch = await resolve_project_path_route("note", project="Other", project_id=None) + assert hatch == ProjectPathRoute(project="Other", path="note", stripped=False) + + # ...and for either side of the collision, which naming exactly must reach. + for name in ("My Docs", "my-docs"): + route = await resolve_project_path_route("note", project=name, project_id=None) + assert route == ProjectPathRoute(project=name, path="note", stripped=False) + + # The path that genuinely names both still refuses, naming both. + with pytest.raises(AmbiguousMountError) as excinfo: + await resolve_project_path_route("my-docs/note", project=None, project_id=None) + assert "My Docs" in str(excinfo.value) + assert "my-docs" in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_sibling_slash_bearing_project_conflicts_with_its_prefix( + config_manager, tmp_path_factory +): + """'docs' and 'team/docs' are two projects, not two spellings of one. + + The path names the 'team/docs' mount while the caller named 'docs', so this + is the ordinary prefix conflict — but the shape heuristic read the extra + leading segment as a workspace qualifier, made them agree, and discarded the + explicit selection to read the other project instead. + """ + config = config_manager.load_config() + config.projects["docs"] = ProjectEntry(path=str(tmp_path_factory.mktemp("docs-sibling"))) + config.projects["team/docs"] = ProjectEntry( + path=str(tmp_path_factory.mktemp("team-docs-sibling")) + ) + config_manager.save_config(config) + + with pytest.raises(ProjectPrefixConflictError) as excinfo: + await resolve_project_path_route("team/docs/note", project="docs", project_id=None) + + assert "team/docs" in str(excinfo.value) + assert "'docs' was passed" in str(excinfo.value) + + # Each project is still reachable by naming it and giving a relative path. + for name in ("docs", "team/docs"): + route = await resolve_project_path_route(f"{name}/note", project=name, project_id=None) + assert route == ProjectPathRoute(project=name, path="note", stripped=True) + + +@pytest.mark.asyncio +async def test_glob_first_segment_never_routes(multi_project_config): + """split_project_prefix's '*' guard: a glob first segment is search input, + not a mount — it falls through to the multi-project refusal.""" + with pytest.raises(UnqualifiedPathRefusedError, match=re.escape("no project 'second-*'")): + await resolve_project_path_route("second-*/notes/foo", project=None, project_id=None) + + +# --- rule 2: explicit project agree/strip/conflict --- + + +@pytest.mark.asyncio +async def test_explicit_project_canonicalized_when_no_prefix(multi_project_config): + """No recognized prefix: the path passes through untouched and the explicit + project is canonicalized to its config spelling.""" + route = await resolve_project_path_route("notes/foo", project="my-research", project_id=None) + + assert route == ProjectPathRoute(project="My Research", path="notes/foo", stripped=False) + + +@pytest.mark.asyncio +async def test_agreeing_prefix_strips_with_explicit_project(multi_project_config): + route = await resolve_project_path_route( + "my-research/notes/foo", project="My Research", project_id=None + ) + + assert route == ProjectPathRoute(project="My Research", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_conflicting_prefix_raises_naming_both(multi_project_config): + """The conflict message names both projects and offers both spellings.""" + with pytest.raises(ProjectPrefixConflictError) as excinfo: + await resolve_project_path_route( + "second-project/notes/foo", project="My Research", project_id=None + ) + + assert str(excinfo.value) == ( + "path names project 'second-project' but project 'My Research' was passed — " + "use 'second-project/' alone, or project='My Research' with a " + "project-relative path" + ) + + +# --- workspace-qualified spellings --- +# The agreement helper is a pure function; driving the qualified spellings +# through it directly avoids standing up cloud workspace discovery. The +# remainder is no longer derived separately — it comes from the same parse that +# matched the route, so the qualified-path tests below pin it end to end. + + +def test_split_project_permalink_prefix_matches_whole_permalinks_longest_first(): + """The one place a path becomes (project, project-relative path). It takes + the candidate set precisely because how many segments a project consumes is + a fact about the known projects, not about the string.""" + permalinks = ["research", "research/2026"] + + assert split_project_permalink_prefix("research/2026/notes", permalinks) == ( + "research/2026", + "notes", + ) + assert split_project_permalink_prefix("research/notes", permalinks) == ("research", "notes") + assert split_project_permalink_prefix("research/2026", permalinks) == ("research/2026", "") + assert split_project_permalink_prefix("engineering/notes", permalinks) is None + # Spelling is normalized per segment, so display names match their permalink. + assert split_project_permalink_prefix("My Research/notes", ["my-research"]) == ( + "my-research", + "notes", + ) + + +@pytest.mark.asyncio +async def test_workspace_resolvers_reject_non_routes_without_discovery(): + """Both entry points refuse shapes that cannot be a workspace route before + touching discovery — no workspace index is built for a plain memory URL or a + single-segment identifier, which is what keeps read_note and search off the + cloud round trip.""" + assert await resolve_workspace_qualified_memory_url("second-project/notes/x") is None + assert await resolve_workspace_qualified_identifier("single-segment") is None + + +def test_agreed_route_project_across_mixed_qualification(): + """A workspace-qualified spelling agrees with the unqualified spelling of + the same project, in either direction, and the more-qualified one carries + the route; different projects never agree. + + This is the fallback path, reached only when the explicit spelling names no + project this session addresses — so there is no identity to compare and the + shapes are all there is. Then segment count decides, not the mere presence + of a slash: a workspace slug is exactly one segment, so 'acme/Research/2026' + qualifies the project 'Research/2026' while it does not qualify a project + named '2026'. + """ + outside = () + + assert _agreed_route_project("research", "other/research", outside) == "other/research" + assert _agreed_route_project("other/research", "research", outside) == "other/research" + assert _agreed_route_project("second-project", "other/research", outside) is None + + # Slash-bearing project names: the escape hatch has to keep working. + assert ( + _agreed_route_project("Research/2026", "acme/Research/2026", outside) + == "acme/Research/2026" + ) + # ...without agreeing with a different project that merely shares a tail. + assert _agreed_route_project("2026", "acme/Research/2026", outside) is None + # A mount named after a workspace still conflicts with that workspace route. + assert _agreed_route_project("team", "team/docs", outside) is None + + +def test_agreed_route_project_prefers_identity_over_shape(): + """When the explicit spelling names a project this session addresses, the + answer comes from identity and the shapes are never consulted. + + 'team/docs' beside 'docs' are two real projects, not one project spelled two + ways. Reading the extra leading segment as a workspace qualifier made them + agree, so an explicit project='docs' was silently discarded and the call read + the other project instead of conflicting. + """ + addressable = ( + AddressableProject(name="docs", permalink="docs"), + AddressableProject(name="team/docs", permalink="team/docs"), + ) + + assert _agreed_route_project("team/docs", "docs", addressable) is None + assert _agreed_route_project("docs", "team/docs", addressable) is None + + # The same project under two spellings still agrees, by permalink equality. + assert _agreed_route_project("Team/Docs", "team/docs", addressable) == "Team/Docs" + + # With no such project addressable, the shape fallback still applies. + assert _agreed_route_project("team/docs", "docs", ()) == "team/docs" + + +@pytest.mark.asyncio +async def test_explicit_workspace_qualified_project_survives_local_prefix_agreement( + multi_project_config, +): + """An explicit '/' param carries the route even when the + path prefix matches a same-named local project — the explicitly named + workspace is never silently swapped for the local shadow.""" + route = await resolve_project_path_route( + "second-project/notes/foo", project="other/second-project", project_id=None + ) + + assert route == ProjectPathRoute( + project="other/second-project", path="notes/foo", stripped=True + ) + + +# --- rule 4: multi-project refusal messages --- + + +@pytest.mark.asyncio +async def test_unqualified_path_refusal_lists_projects_sorted(multi_project_config): + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("notes/foo", project=None, project_id=None) + + assert str(excinfo.value) == ( + "no project 'notes' — active projects: my-research/, second-project/, test-project/" + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("path", ["", "/"]) +async def test_empty_input_refuses_with_no_project_specified(multi_project_config, path): + """grep/tail (no path) and a bare root land here in multi-project configs.""" + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route(path, project=None, project_id=None) + + assert str(excinfo.value) == ( + "no project specified — active projects: my-research/, second-project/, test-project/" + ) + + +# --- single-project and empty-config passthrough --- + + +@pytest.mark.asyncio +@pytest.mark.parametrize("path", ["notes/foo", "test/root", "", "some-title"]) +async def test_single_project_input_passes_through_unchanged(config_manager, path): + """One configured project keeps today's default resolution; 'test' is not + falsely stripped as a prefix of 'test-project' (permalink comparison).""" + route = await resolve_project_path_route(path, project=None, project_id=None) + + assert route == ProjectPathRoute(project=None, path=path, stripped=False) + + +@pytest.mark.asyncio +async def test_empty_config_passes_through(empty_project_config): + """An emptied config still materializes the placeholder 'main' project, so a + locally routed session addresses exactly one project and keeps today's + default resolution — there is nothing to disambiguate.""" + assert list(empty_project_config.config.projects) == ["main"] + + route = await resolve_project_path_route("anything/x", project=None, project_id=None) + + assert route == ProjectPathRoute(project=None, path="anything/x", stripped=False) + + +# --- env constraint as effective explicit project --- + + +@pytest.mark.asyncio +async def test_env_constraint_agreeing_prefix_strips(multi_project_config, monkeypatch): + monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", "second-project") + + route = await resolve_project_path_route( + "second-project/notes/foo", project=None, project_id=None + ) + + assert route == ProjectPathRoute(project="second-project", path="notes/foo", stripped=True) + + +@pytest.mark.asyncio +async def test_env_constraint_conflicting_prefix_refuses(multi_project_config, monkeypatch): + monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", "test-project") + + with pytest.raises(ProjectPrefixConflictError, match="but project 'test-project' was passed"): + await resolve_project_path_route("second-project/notes/foo", project=None, project_id=None) + + +@pytest.mark.asyncio +async def test_env_constraint_prevents_refusal_for_empty_input(multi_project_config, monkeypatch): + """The constraint is an explicit project: grep/tail-style empty input routes + to it instead of refusing.""" + monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", "second-project") + + route = await resolve_project_path_route("", project=None, project_id=None) + + assert route == ProjectPathRoute(project="second-project", path="", stripped=False) + + +# --- cloud/factory sessions (#1421) --- +# The hosted MCP server installs a client factory and keeps project state in the +# tenant database, not in config: BasicMemoryConfig always materializes a +# placeholder 'main' entry, so local config proves nothing about a cloud +# session's projects. These tests stand up that shape and pin the invariant +# that ties the mount view to the resolver — everything `ls "/"` advertises must +# be addressable — plus the refusal that keeps an unqualified reference from +# landing on whichever project a teammate last flagged as default. + + +@dataclass +class _CloudSession: + """What a stood-up cloud session exposes to a test.""" + + projects: tuple[ProjectItem, ...] + listings: list[ProjectList] + + +def _routes_to_project(route: ProjectPathRoute, permalink: str) -> bool: + """True when a route landed on the project the mount view advertises. + + Cloud routes may come back workspace-qualified ('team/research'), so accept + either the bare permalink or that permalink behind exactly one workspace + segment — the same rule the resolver uses to compare two spellings. + """ + assert route.project is not None + routed = generate_permalink(route.project) + return routed == permalink or ( + routed.endswith(f"/{permalink}") and routed.count("/") - permalink.count("/") == 1 + ) + + +@pytest.fixture +def cloud_session(monkeypatch, config_manager): + """Build a factory-mode session whose workspace holds the named projects. + + Both the mount view and workspace discovery read the same project listing + the tenant serves, exactly as they do in production: the injected factory + client answers `get_client()` for the mount view and `get_client(workspace=)` + for the workspace index. Every listing call is recorded so tests can pin how + often routing pays for it. + """ + + def build(*names: str, default: Optional[str] = None) -> _CloudSession: + config = config_manager.load_config() + # The cloud nulls its config cache so default_project reads as None; + # projects={} still round-trips back as the placeholder 'main' entry. + config.projects = {} + config.default_project = None + config_manager.save_config(config) + + projects = tuple( + ProjectItem( + id=index + 1, + external_id=f"{generate_permalink(name)}-external-id", + name=name, + path=f"/app/data/{generate_permalink(name)}", + is_default=name == default, + ) + for index, name in enumerate(names) + ) + project_list = ProjectList(projects=list(projects), default_project=default) + workspace = WorkspaceInfo( + tenant_id="team-tenant", + workspace_type="organization", + slug="team", + name="Team", + role="editor", + is_default=True, + ) + + listings: list[ProjectList] = [] + + @asynccontextmanager + async def fake_get_client(*args, **kwargs) -> AsyncIterator[object]: + yield object() + + async def fake_list_projects(self) -> ProjectList: + listings.append(project_list) + return project_list + + async def fake_get_available_workspaces(context=None) -> list[WorkspaceInfo]: + return [workspace] + + monkeypatch.setattr(async_client, "is_factory_mode", lambda: True) + monkeypatch.setattr(async_client, "get_client", fake_get_client) + monkeypatch.setattr( + "basic_memory.mcp.clients.project.ProjectClient.list_projects", + fake_list_projects, + ) + monkeypatch.setattr( + project_context, + "get_available_workspaces", + fake_get_available_workspaces, + ) + return _CloudSession(projects=projects, listings=listings) + + return build + + +@pytest.mark.asyncio +async def test_cloud_workspace_refuses_unqualified_path(cloud_session): + """The reported failure (#1421): with only a placeholder config entry to + enumerate, an unqualified path used to route to a default project. It now + refuses, naming every addressable project in copyable prefix form.""" + cloud_session("research", "engineering", "personal-notes") + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("notes/foo", project=None, project_id=None) + + assert str(excinfo.value) == ( + "no project 'notes' — active projects: engineering/, personal-notes/, research/" + ) + + +@pytest.mark.asyncio +async def test_cloud_workspace_refuses_pathless_call_through_the_tool(cloud_session): + """grep/tail carry no path to qualify, so the refusal is the tool's answer: + a silent search of one project out of several is the bug being removed.""" + cloud_session("research", "engineering") + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await grep("needle") + + assert str(excinfo.value) == ("no project specified — active projects: engineering/, research/") + + +@pytest.mark.asyncio +async def test_cloud_qualified_path_routes_to_that_project(cloud_session): + """A qualified '/notes/x' routes to that exact project by the mount + name the root advertises, with the remainder as the project-relative path. + + The mount table is already scoped to this session's own route, so it is the + authority on that first segment; `ls "research"` has always routed by the + bare name, and the path form now agrees with it instead of qualifying the + tenant only when a slash happened to be present. + """ + cloud_session("research", "engineering", "personal-notes") + + route = await resolve_project_path_route("research/notes/x", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="research", + path="notes/x", + stripped=True, + project_id="research-external-id", + ) + assert _routes_to_project(route, "research") + + +@pytest.mark.asyncio +async def test_cloud_workspace_qualified_path_without_mount_collision_still_routes(cloud_session): + """No project is named 'team', so the workspace slug is free to claim the + first segment: '//' resolves through workspace + discovery, qualified name and all. Mount precedence takes nothing away from + the spellings that address other workspaces.""" + cloud_session("research", "engineering") + + route = await resolve_project_path_route("team/research/notes/x", project=None, project_id=None) + + # The route carries the resolved entry's id, so nothing re-resolves the + # qualified name against a workspace that did not answer it. + assert route == ProjectPathRoute( + project="team/research", + path="notes/x", + stripped=True, + project_id="research-external-id", + ) + + +@pytest.mark.asyncio +async def test_local_cloud_session_lists_a_workspace_project_root( + multi_project_config, monkeypatch +): + """The pathless root gets the same discovery permission as the path form. + + In a locally routed session holding cloud credentials, 'acme/docs/note' + resolved through workspace discovery while 'acme/docs' — that same project's + root, and the thing `ls` needs to enter it — refused. The stricter + three-segment gate belongs to identifier detection (read_note, search), + which has no mount table to decline first and no refusal rule behind it. + """ + workspace = WorkspaceInfo( + tenant_id="acme-tenant", + workspace_type="organization", + slug="acme", + name="Acme", + role="editor", + is_default=True, + ) + + async def fake_workspaces(context=None) -> list[WorkspaceInfo]: + return [workspace] + + async def fake_entries(ws, context=None): + return ( + WorkspaceProjectEntry( + workspace=ws, + project=ProjectItem( + id=1, + external_id="99999999-9999-9999-9999-999999999999", + name="docs", + path="/app/data/docs", + ), + ), + ) + + monkeypatch.setattr(project_context, "get_available_workspaces", fake_workspaces) + monkeypatch.setattr(project_context, "_fetch_workspace_project_entries", fake_entries) + monkeypatch.setattr(project_context, "has_cloud_credentials", lambda config: True) + + root = await resolve_project_path_route("acme/docs", project=None, project_id=None) + nested = await resolve_project_path_route("acme/docs/note", project=None, project_id=None) + + assert root == ProjectPathRoute( + project="acme/docs", + path="", + stripped=True, + project_id="99999999-9999-9999-9999-999999999999", + ) + assert nested.project == "acme/docs" + assert nested.path == "note" + + +@pytest.mark.asyncio +async def test_cloud_workspace_qualified_project_root_routes(cloud_session): + """'/' with no path names that project's root, exactly as + a bare mount name does. Only the three-segment form used to resolve, so a + cross-workspace project could be listed into ('acme/docs/notes') but its own + root ('acme/docs') had no spelling at all — it fell through to the + multi-project refusal.""" + cloud_session("research", "engineering") + + route = await resolve_project_path_route("team/research", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="team/research", path="", stripped=True, project_id="research-external-id" + ) + + +@pytest.mark.asyncio +async def test_cloud_workspace_route_matches_multi_segment_project_permalink(cloud_session): + """A workspace route splits project from path by matching that workspace's + project permalinks, not by taking one segment. The mount table learned this + first; the workspace parser rediscovered the same wrong assumption, so both + now go through split_project_permalink_prefix and neither can drift.""" + cloud_session("Research/2026", "research") + + nested = await resolve_project_path_route( + "team/research/2026/notes", project=None, project_id=None + ) + assert nested == ProjectPathRoute( + project="team/research/2026", + path="notes", + stripped=True, + project_id="research/2026-external-id", + ) + + root = await resolve_project_path_route("team/research/2026", project=None, project_id=None) + assert root == ProjectPathRoute( + project="team/research/2026", + path="", + stripped=True, + project_id="research/2026-external-id", + ) + + # The shorter sibling still claims its own paths — longest match, not first. + sibling = await resolve_project_path_route("team/research/notes", project=None, project_id=None) + assert sibling == ProjectPathRoute( + project="team/research", + path="notes", + stripped=True, + project_id="research-external-id", + ) + + +@pytest.mark.asyncio +async def test_cloud_workspace_project_root_falls_through_without_workspaces( + cloud_session, monkeypatch +): + """Workspace discovery stays best-effort for prefix detection: with no + reachable workspace, '/' is simply not a route and lands + on the ordinary refusal. Input that shaped like a workspace route may never + have meant one, so a discovery failure must not become the caller's error.""" + cloud_session("research", "engineering") + + async def no_workspaces(context=None) -> list[WorkspaceInfo]: + return [] + + monkeypatch.setattr(project_context, "get_available_workspaces", no_workspaces) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("acme/docs", project=None, project_id=None) + + assert str(excinfo.value) == "no project 'acme' — active projects: engineering/, research/" + + +@pytest.mark.asyncio +async def test_cloud_slash_bearing_project_resolves_by_its_own_name(cloud_session): + """Resolving a project identifier tries the whole name before reading its + first segment as a workspace. + + 'Research/2026' and 'acme/docs' are the same shape, so a slash-bearing + project name used to be unroutable — the lookup took 'Research' for a + workspace and failed with "Workspace 'Research' was not found". The v2 + project router already resolved exact-first for this reason; the index now + matches it. + """ + cloud_session("Research/2026", "engineering") + + entry = await resolve_workspace_project_identifier("Research/2026") + + assert entry.project.name == "Research/2026" + assert entry.qualified_name == "team/research/2026" + + # The workspace-qualified spelling still resolves through the split. + qualified = await resolve_workspace_project_identifier("team/Research/2026") + assert qualified.project.name == "Research/2026" + + +@pytest.mark.asyncio +async def test_cloud_mount_wins_over_colliding_workspace_slug(cloud_session): + """The collision: 'team' is both an advertised mount and this workspace's + slug, and that workspace also holds a project 'docs'. The mount wins the + first segment, so 'team/docs/x' is project 'team', path 'docs/x' — never + project 'docs' in workspace 'team'. Attempting workspace-qualified parsing + first served another project's data under an advertised name.""" + cloud_session("team", "docs") + + route = await resolve_project_path_route("team/docs/x", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="team", path="docs/x", stripped=True, project_id="team-external-id" + ) + + +@pytest.mark.asyncio +async def test_cloud_slug_collision_shadowed_project_stays_addressable(cloud_session): + """The documented cost of mount-wins, pinned: while a mount named 'team' + exists, project 'docs' in workspace 'team' loses its qualified *path* + spelling. It stays addressable through the project param — project-relative, + or with an agreeing prefix — and the shadowed spelling conflicts loudly + rather than resolving either way.""" + cloud_session("team", "docs") + + relative = await resolve_project_path_route("x", project="team/docs", project_id=None) + assert relative == ProjectPathRoute(project="team/docs", path="x", stripped=False) + + agreeing = await resolve_project_path_route("docs/x", project="team/docs", project_id=None) + assert agreeing == ProjectPathRoute(project="team/docs", path="x", stripped=True) + + with pytest.raises(ProjectPrefixConflictError, match="path names project 'team'"): + await resolve_project_path_route("team/docs/x", project="team/docs", project_id=None) + + +@pytest.mark.asyncio +async def test_cloud_every_advertised_mount_is_addressable_under_slug_collision(cloud_session): + """The round-trip invariant survives the collision, which is the point of + mount precedence: a mount named after the workspace slug still routes to + itself, and so does every other mount beside it.""" + cloud_session("team", "docs") + + mounts = await ls() + + assert [node["name"] for node in mounts["nodes"]] == ["docs", "team"] + for node in mounts["nodes"]: + permalink = node["permalink"] + + external_id = f"{permalink}-external-id" + + root_route = await resolve_project_path_route(permalink, project=None, project_id=None) + assert root_route == ProjectPathRoute( + project=node["name"], path="", stripped=True, project_id=external_id + ) + + path_route = await resolve_project_path_route( + f"{permalink}/notes/x", project=None, project_id=None + ) + assert path_route == ProjectPathRoute( + project=node["name"], path="notes/x", stripped=True, project_id=external_id + ) + + +@pytest.mark.asyncio +async def test_cloud_every_advertised_mount_is_addressable(cloud_session): + """Round trip over the advertised list: `ls "/"` and the resolver read one + source, so every mount the root advertises routes — as a bare mount name + and as a path under it. A mount that listed but did not route is exactly + the mismatch #1421 reported.""" + cloud_session("research", "engineering", "personal-notes") + + mounts = await ls() + + assert [node["name"] for node in mounts["nodes"]] == [ + "engineering", + "personal-notes", + "research", + ] + for node in mounts["nodes"]: + permalink = node["permalink"] + assert node["directory_path"] == f"/{permalink}" + + root_route = await resolve_project_path_route(permalink, project=None, project_id=None) + assert root_route.stripped is True + assert root_route.path == "" + assert _routes_to_project(root_route, permalink) + + path_route = await resolve_project_path_route( + f"{permalink}/notes/x", project=None, project_id=None + ) + assert path_route.stripped is True + assert path_route.path == "notes/x" + assert _routes_to_project(path_route, permalink) + + +@pytest.mark.asyncio +async def test_cloud_single_project_workspace_resolves_unqualified(cloud_session): + """One project in the workspace means no ambiguity to protect against, so + unqualified references keep resolving unchanged.""" + cloud_session("research") + + route = await resolve_project_path_route("notes/foo", project=None, project_id=None) + + assert route == ProjectPathRoute(project=None, path="notes/foo", stripped=False) + + +@pytest.mark.asyncio +async def test_cloud_refusal_does_not_consult_the_default_flag(cloud_session): + """is_default is one shared mutable flag in a team workspace, so it must not + decide where an unqualified call lands: a workspace whose default points at + 'alpha' still refuses while 'beta' and 'gamma' exist.""" + session = cloud_session("alpha", "beta", "gamma", default="alpha") + + assert [project.name for project in session.projects if project.is_default] == ["alpha"] + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("", project=None, project_id=None) + + # The flagged project is listed as one choice among equals, never selected. + assert str(excinfo.value) == "no project specified — active projects: alpha/, beta/, gamma/" + + # And it hijacks nothing that named a project: qualified input still routes + # where the caller said, not to whatever carries the flag. + route = await resolve_project_path_route("beta/notes/x", project=None, project_id=None) + + assert _routes_to_project(route, "beta") + + +@pytest.mark.asyncio +async def test_cloud_project_listing_is_fetched_once_per_request(cloud_session): + """The cost of knowing the workspace's projects: one listing per MCP + request, memoized in context state, no matter how many resolutions the + request makes. Without a context (CLI calls) each resolution pays.""" + session = cloud_session("research", "engineering") + context = ContextState() + + first = await resolve_project_path_route( + "research", project=None, project_id=None, context=ctx(context) + ) + assert first.stripped is True + assert len(session.listings) == 1 + + session.listings.clear() + second = await resolve_project_path_route( + "engineering", project=None, project_id=None, context=ctx(context) + ) + + assert second.stripped is True + assert session.listings == [] + + +# --- mounts stay bound to the workspace that advertised them (#1421) --- +# A hosted session's own route is one tenant, but workspace discovery reaches +# every workspace the account can see, and project names are unique only inside +# one of them. These tests stand up that shape: two accessible workspaces, the +# session bound to the non-default one, and the same project permalink in both. + +_SESSION_DOCS_ID = "11111111-1111-1111-1111-111111111111" +_DEFAULT_DOCS_ID = "22222222-2222-2222-2222-222222222222" +_SESSION_RESEARCH_ID = "33333333-3333-3333-3333-333333333333" +_SESSION_ENGINEERING_ID = "44444444-4444-4444-4444-444444444444" +_DEFAULT_NOTES_ID = "55555555-5555-5555-5555-555555555555" + +# Every (workspace, project) pair gets its own id: the same project name living +# in two workspaces is the whole point of this section, and the id is what pins +# a mount to the workspace that advertised it. UUID-shaped because the index +# looks an external_id up as a UUID before falling back to a name search. +_WORKSPACE_PROJECT_IDS = { + ("session-tenant", "docs"): _SESSION_DOCS_ID, + ("session-tenant", "research"): _SESSION_RESEARCH_ID, + ("session-tenant", "engineering"): _SESSION_ENGINEERING_ID, + ("default-tenant", "docs"): _DEFAULT_DOCS_ID, + ("default-tenant", "notes"): _DEFAULT_NOTES_ID, +} + + +@dataclass +class _FakeHttpClient: + """Stands in for the routed client, carrying only the workspace selector.""" + + workspace: Optional[str] + + +def _tenant_listing(tenant_id: str, names: tuple[str, ...]) -> ProjectList: + """Build one tenant's project listing, the first name carrying is_default.""" + return ProjectList( + projects=[ + ProjectItem( + id=index + 1, + external_id=_WORKSPACE_PROJECT_IDS[(tenant_id, name)], + name=name, + path=f"/app/data/{generate_permalink(name)}", + is_default=index == 0, + ) + for index, name in enumerate(names) + ], + default_project=names[0], + ) + + +@pytest.fixture +def cross_workspace_session(monkeypatch, config_manager): + """Build a factory session on a non-default workspace beside the default one. + + ``get_client()`` with no selector is the session's own tenant — what the + hosted server hands a call that names no workspace, and therefore what the + mount view lists. ``get_client(workspace=...)`` is how the workspace index + reaches each accessible tenant, so the two answer with different projects. + """ + + def build( + *, + failed_tenant: Optional[str] = None, + session_projects: tuple[str, ...] = ("docs",), + default_projects: tuple[str, ...] = ("docs",), + ) -> tuple[WorkspaceInfo, WorkspaceInfo]: + config = config_manager.load_config() + config.projects = {} + config.default_project = None + config_manager.save_config(config) + + session_workspace = WorkspaceInfo( + tenant_id="session-tenant", + workspace_type="organization", + slug="beta", + name="Beta", + role="editor", + is_default=False, + ) + default_workspace = WorkspaceInfo( + tenant_id="default-tenant", + workspace_type="personal", + slug="acme", + name="Acme", + role="owner", + is_default=True, + ) + listings = { + "session-tenant": _tenant_listing("session-tenant", session_projects), + "default-tenant": _tenant_listing("default-tenant", default_projects), + } + + @asynccontextmanager + async def fake_get_client(*args, **kwargs) -> AsyncIterator[object]: + yield _FakeHttpClient(workspace=kwargs.get("workspace")) + + async def fake_list_projects(self) -> ProjectList: + tenant = self.http_client.workspace or session_workspace.tenant_id + if tenant == failed_tenant: + raise RuntimeError(f"tenant {tenant} is unavailable") + return listings[tenant] + + async def fake_get_available_workspaces(context=None) -> list[WorkspaceInfo]: + return [session_workspace, default_workspace] + + monkeypatch.setattr(async_client, "is_factory_mode", lambda: True) + monkeypatch.setattr(async_client, "get_client", fake_get_client) + monkeypatch.setattr( + "basic_memory.mcp.clients.project.ProjectClient.list_projects", + fake_list_projects, + ) + monkeypatch.setattr( + project_context, + "get_available_workspaces", + fake_get_available_workspaces, + ) + return session_workspace, default_workspace + + return build + + +@pytest.mark.asyncio +async def test_cloud_mount_routes_by_the_id_that_names_its_workspace(cross_workspace_session): + """The mount view lists this session's own tenant, so a mount it advertises + has to route there. Carrying only the bare name let the cross-workspace + index re-resolve 'docs' by the is_default flag: on a first call — before any + workspace is cached — `cat("docs/x")` read the default workspace's 'docs' + under a name the session workspace advertised.""" + session_workspace, default_workspace = cross_workspace_session() + + route = await resolve_project_path_route("docs/x", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="docs", path="x", stripped=True, project_id=_SESSION_DOCS_ID + ) + + # The id is what the shared index consumes, and it is the half that decides: + # by id the route lands in the session's workspace, while the bare name + # still falls through to whichever workspace carries the default flag. + by_id = await resolve_workspace_project_identifier(route.project_id or "") + by_name = await resolve_workspace_project_identifier("docs") + + assert by_id.workspace.tenant_id == session_workspace.tenant_id + assert by_name.workspace.tenant_id == default_workspace.tenant_id + + +@pytest.mark.asyncio +async def test_emitted_cross_workspace_path_replays_with_one_mount(cross_workspace_session): + """A path this layer emits must route back through the same session. + + `ls("docs", project="acme/docs")` strips the agreeing prefix and requalifies + its children as 'acme/docs/...'. Replaying one without the project argument + used to hit the mount-count gate, skip workspace parsing, and read the sole + mount's same-named path in the *other* tenant — a navigation path returned + for one workspace silently reading another. + """ + cross_workspace_session(session_projects=("docs",), default_projects=("docs",)) + + original = await resolve_project_path_route("docs", project="acme/docs", project_id=None) + assert original.project == "acme/docs" + + replay = await resolve_project_path_route("acme/docs/notes", project=None, project_id=None) + + assert replay == ProjectPathRoute( + project="acme/docs", path="notes", stripped=True, project_id=_DEFAULT_DOCS_ID + ) + + +@pytest.mark.asyncio +async def test_workspace_shaped_path_routes_even_with_one_mount(cross_workspace_session): + """Route versus path, decided by the precedence order rather than a parse. + + An unqualified path whose leading segments name a real accessible workspace + and a real project inside it is read as a route, whatever the session + mounts. This briefly depended on the mount count, to keep a coincidentally + workspace-shaped relative path at home in a one-mount session — but that + made the qualified paths this layer itself emits unroutable there, and both + readings are a silent wrong-project read. The tie breaks on whose string it + is: the emitted canonical form is ours and must route back; a user-typed + collision has the explicit fix asserted in the test below. + """ + cross_workspace_session(session_projects=("research",), default_projects=("docs",)) + + route = await resolve_project_path_route("acme/docs/foo", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="acme/docs", path="foo", stripped=True, project_id=_DEFAULT_DOCS_ID + ) + + # A path that only looks workspace-shaped matches no workspace and stays put. + coincidence = await resolve_project_path_route("notes/2026/foo", project=None, project_id=None) + assert coincidence == ProjectPathRoute(project=None, path="notes/2026/foo", stripped=False) + + +@pytest.mark.asyncio +async def test_named_project_strips_its_own_qualified_prefix(cross_workspace_session): + """Rule 2 — an agreeing prefix strips — has to hold for the qualified + spelling too, and it is decidable without any network. + + Only rule 4's discovery used to recognize a workspace-qualified project in + the path, and the precedence order now declines to run that when a project + was named. So the caller's own two spellings of one project stopped + agreeing: 'foo' was read as 'acme/docs/foo'. It also broke the round trip, + since a routed ls("acme/docs") returns exactly this prefixed form. + """ + cross_workspace_session(session_projects=("research",), default_projects=("docs",)) + + route = await resolve_project_path_route("acme/docs/foo", project="acme/docs", project_id=None) + + assert route == ProjectPathRoute(project="acme/docs", path="foo", stripped=True) + + # A prefix that is NOT the named project still stays part of the path. + unrelated = await resolve_project_path_route( + "acme/docs/foo", project="research", project_id=None + ) + assert unrelated.path == "acme/docs/foo" + + +@pytest.mark.asyncio +async def test_named_project_keeps_the_rest_of_the_path_inside_it(cross_workspace_session): + """An explicit project settles route-versus-path: the caller said which + project they mean, so the remaining path is inside it and nothing reroutes. + + This spelling previously raised a prefix conflict, which left the reported + bug with no workaround at all — the path could neither stay home on its own + nor be pinned there. + """ + cross_workspace_session(session_projects=("research",), default_projects=("docs",)) + + route = await resolve_project_path_route("acme/docs/foo", project="research", project_id=None) + + assert route == ProjectPathRoute(project="research", path="acme/docs/foo", stripped=False) + + +@pytest.mark.asyncio +async def test_workspace_route_still_wins_when_the_path_could_not_resolve( + cross_workspace_session, +): + """The other half of the order: with several projects mounted, an + unqualified path refuses anyway, so reading the leading segments as a route + is the only way the input can mean anything. Cross-workspace addressing must + not be lost to the fix above.""" + cross_workspace_session( + session_projects=("research", "engineering"), default_projects=("docs",) + ) + + route = await resolve_project_path_route("acme/docs/foo", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="acme/docs", path="foo", stripped=True, project_id=_DEFAULT_DOCS_ID + ) + + # And an unaddressable name still refuses rather than defaulting. + with pytest.raises(UnqualifiedPathRefusedError): + await resolve_project_path_route("nope/nothing/x", project=None, project_id=None) + + +@pytest.mark.asyncio +async def test_cloud_explicit_qualified_project_drops_the_mount_id(cross_workspace_session): + """The escape hatch keeps working: an explicit '/' names + a workspace of its own, so it must not inherit the mount's id and be routed + back to this session's tenant.""" + cross_workspace_session() + + route = await resolve_project_path_route("docs/x", project="acme/docs", project_id=None) + + assert route == ProjectPathRoute(project="acme/docs", path="x", stripped=True, project_id=None) + + +@pytest.mark.asyncio +async def test_cloud_workspace_project_root_surfaces_a_failed_workspace(cross_workspace_session): + """A '/' root whose workspace could not be listed is a + real failure, not an unrecognized path: it must say so rather than fall + through to a refusal that claims the project does not exist. + + Two mounted projects, because workspace routes are only parsed when an + unqualified path could not resolve anyway — see the precedence order. + """ + cross_workspace_session( + failed_tenant="default-tenant", + session_projects=("research", "engineering"), + ) + + with pytest.raises(ValueError, match="could not be loaded"): + await resolve_project_path_route("acme/docs", project=None, project_id=None) + + +# --- an unqualified first segment never leaves this session's workspace (#1421) --- +# The companion to mount-id binding above. That one covers a name present in +# both workspaces; these cover a name present only in the *other* one, where +# there is no mount to claim the segment and the workspace fallback used to +# resolve the bare name across every accessible workspace. + + +@pytest.mark.asyncio +async def test_cloud_unqualified_first_segment_never_reaches_another_workspace( + cross_workspace_session, +): + """The leak: this session's workspace has no 'notes', another accessible + workspace does, and `cat("notes/foo")` is an ordinary project-relative path. + Resolving the bare first segment against every accessible workspace found + the other tenant's 'notes' and read it. It must refuse instead, naming only + the mounts this session can actually address.""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("notes/foo", project=None, project_id=None) + + # The other workspace's project is named nowhere in the refusal — it was + # never addressable from here, so advertising it would teach a wrong route. + assert str(excinfo.value) == ("no project 'notes' — active projects: engineering/, research/") + + +@pytest.mark.asyncio +async def test_cloud_unqualified_first_segment_stays_local_in_a_single_project_workspace( + cross_workspace_session, +): + """Same shape, one project in this workspace: rule 5 has no ambiguity to + refuse, so the path stays unstripped for the ordinary default resolution. + The point is where it does *not* go — a lone mount must not make the + cross-workspace name lookup the tiebreaker.""" + cross_workspace_session(session_projects=("research",), default_projects=("notes",)) + + route = await resolve_project_path_route("notes/foo", project=None, project_id=None) + + assert route == ProjectPathRoute(project=None, path="notes/foo", stripped=False) + + +@pytest.mark.asyncio +async def test_cloud_empty_route_segment_is_not_a_workspace_route(cross_workspace_session): + """An empty segment names nothing, so 'acme//notes' is not the qualified + spelling of anything even though 'acme' is a real workspace slug. It refuses + rather than being repaired into a route the caller did not write.""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("acme//notes", project=None, project_id=None) + + assert str(excinfo.value) == ("no project 'acme' — active projects: engineering/, research/") + + +@pytest.mark.asyncio +async def test_cloud_other_workspace_stays_reachable_when_qualified(cross_workspace_session): + """Refuse-don't-default takes no address away: naming the workspace is how a + caller reaches it deliberately, as a path and through the project param, and + both spellings still land on the other tenant's project.""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + qualified_path = await resolve_project_path_route( + "acme/notes/foo", project=None, project_id=None + ) + assert qualified_path == ProjectPathRoute( + project="acme/notes", path="foo", stripped=True, project_id=_DEFAULT_NOTES_ID + ) + + root = await resolve_project_path_route("acme/notes", project=None, project_id=None) + assert root == ProjectPathRoute( + project="acme/notes", path="", stripped=True, project_id=_DEFAULT_NOTES_ID + ) + + explicit = await resolve_project_path_route("foo", project="acme/notes", project_id=None) + assert explicit == ProjectPathRoute(project="acme/notes", path="foo", stripped=False) + + +# --- a qualified name must reach the workspace it names (#1421) --- + + +def _index_with(*entries: tuple[WorkspaceInfo, str, str]) -> WorkspaceProjectIndex: + """Build a workspace index from (workspace, project name, external_id) rows.""" + by_tenant: dict[str, WorkspaceInfo] = {} + for workspace, _, _ in entries: + by_tenant.setdefault(workspace.tenant_id, workspace) + workspaces = tuple(by_tenant.values()) + return build_workspace_project_index( + workspaces, + tuple( + WorkspaceProjectEntry( + workspace=workspace, + project=ProjectItem( + id=index + 1, + external_id=external_id, + name=name, + path=f"/app/data/{generate_permalink(name)}", + ), + ) + for index, (workspace, name, external_id) in enumerate(entries) + ), + ) + + +@pytest.mark.asyncio +async def test_slug_precedence_survives_the_whole_permalink_probe(): + """One workspace's display name must not shadow another's slug. + + match_workspace_identifier gives slugs global precedence. The probe that + decides whether the qualified reading resolves had its own inline matcher + that took the first field to hit in iteration order, so a display-name + workspace won, the qualified reading looked like a miss, and the request + fell through to an unrelated whole-permalink project in a third workspace — + a cross-workspace read from a route that named a real slug. + """ + named_foo = WorkspaceInfo( + tenant_id="t1", + workspace_type="organization", + slug="wsone", + name="foo", + role="editor", + is_default=False, + ) + slug_foo = WorkspaceInfo( + tenant_id="t2", + workspace_type="organization", + slug="foo", + name="Second", + role="editor", + is_default=False, + ) + third = WorkspaceInfo( + tenant_id="t3", + workspace_type="organization", + slug="third", + name="Third", + role="editor", + is_default=False, + ) + index = build_workspace_project_index( + (named_foo, slug_foo, third), + ( + WorkspaceProjectEntry( + workspace=slug_foo, + project=ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="docs", + path="/app/docs", + ), + ), + WorkspaceProjectEntry( + workspace=third, + project=ProjectItem( + id=2, + external_id="22222222-2222-2222-2222-222222222222", + name="foo/docs", + path="/app/foo-docs", + ), + ), + ), + ) + + entry = await resolve_workspace_project_from_index(index, "foo/docs") + + assert entry.workspace.slug == "foo" + assert entry.project.name == "docs" + + +@pytest.mark.asyncio +async def test_qualified_name_beats_a_colliding_whole_permalink(): + """'/' and a project literally named 'acme/docs' are the + same shape, so the index tries both readings — qualified first. + + Preferring the whole permalink sent a route that explicitly named workspace + 'acme' to a *different* tenant that happened to hold a project called + 'acme/docs'. The fallback still exists: it is what makes a slash-bearing + name routable when its first segment names no workspace at all. + """ + acme = WorkspaceInfo( + tenant_id="acme-tenant", + workspace_type="organization", + slug="acme", + name="Acme", + role="editor", + is_default=True, + ) + beta = WorkspaceInfo( + tenant_id="beta-tenant", + workspace_type="organization", + slug="beta", + name="Beta", + role="editor", + is_default=False, + ) + index = _index_with( + (acme, "docs", "11111111-1111-1111-1111-111111111111"), + (beta, "acme/docs", "22222222-2222-2222-2222-222222222222"), + (beta, "Research/2026", "33333333-3333-3333-3333-333333333333"), + ) + + qualified = await resolve_workspace_project_from_index(index, "acme/docs") + assert qualified.workspace.slug == "acme" + assert qualified.project.name == "docs" + + # The fallback: no workspace is named 'Research', so the whole permalink wins. + slash_bearing = await resolve_workspace_project_from_index(index, "Research/2026") + assert slash_bearing.workspace.slug == "beta" + assert slash_bearing.project.name == "Research/2026" + + # And the collided project is still reachable by the id that names it exactly. + by_id = await resolve_workspace_project_from_index( + index, "22222222-2222-2222-2222-222222222222" + ) + assert by_id.project.name == "acme/docs" + + +@pytest.mark.asyncio +async def test_empty_session_workspace_still_reaches_a_named_workspace(monkeypatch, config_manager): + """Route-versus-path protects *exactly* one addressable project, not none. + + A factory session whose connection-time workspace holds no projects has no + project for a relative path to resolve inside, and an empty mount table + cannot refuse on the caller's behalf either — so 'acme/docs/note' fell + through to project=None and tried the empty workspace. With nothing to take + away, the explicitly named workspace wins. + """ + config = config_manager.load_config() + config.projects = {} + config.default_project = None + config_manager.save_config(config) + + empty_ws = WorkspaceInfo( + tenant_id="empty-tenant", + workspace_type="organization", + slug="beta", + name="Beta", + role="editor", + is_default=False, + ) + acme = WorkspaceInfo( + tenant_id="acme-tenant", + workspace_type="organization", + slug="acme", + name="Acme", + role="editor", + is_default=True, + ) + + @asynccontextmanager + async def fake_get_client(*a, **k): + yield object() + + async def fake_list_projects(self): + return ProjectList(projects=[], default_project=None) # session workspace is EMPTY + + async def fake_workspaces(context=None): + return [empty_ws, acme] + + async def fake_entries(ws, context=None): + if ws.tenant_id != "acme-tenant": + return () + return ( + WorkspaceProjectEntry( + workspace=ws, + project=ProjectItem( + id=1, + external_id="77777777-7777-7777-7777-777777777777", + name="docs", + path="/app/docs", + ), + ), + ) + + monkeypatch.setattr(async_client, "is_factory_mode", lambda: True) + monkeypatch.setattr(async_client, "get_client", fake_get_client) + monkeypatch.setattr( + "basic_memory.mcp.clients.project.ProjectClient.list_projects", fake_list_projects + ) + monkeypatch.setattr(project_context, "get_available_workspaces", fake_workspaces) + monkeypatch.setattr(project_context, "_fetch_workspace_project_entries", fake_entries) + + route = await resolve_project_path_route("acme/docs/note", project=None, project_id=None) + + assert route == ProjectPathRoute( + project="acme/docs", + path="note", + stripped=True, + project_id="77777777-7777-7777-7777-777777777777", + ) diff --git a/tests/mcp/test_tool_posix.py b/tests/mcp/test_tool_posix.py index f63f0724a..f6684b54d 100644 --- a/tests/mcp/test_tool_posix.py +++ b/tests/mcp/test_tool_posix.py @@ -5,15 +5,28 @@ assert on the JSON shapes the canonical `output_format="json"` paths produce. """ +from pathlib import Path from types import SimpleNamespace import pytest +import yaml from fastmcp.exceptions import ToolError import basic_memory.mcp.tools.posix_tools as posix_tools +from basic_memory.mcp.project_context import ( + ProjectPrefixConflictError, + UnqualifiedPathRefusedError, +) from basic_memory.mcp.tools import cat, find, grep, ls, man, tail, write_note from basic_memory.schemas.search import SearchRetrievalMode + +@pytest.fixture +def no_project_constraint(monkeypatch): + """Clear the env project constraint so unqualified routing paths are reachable.""" + monkeypatch.delenv("BASIC_MEMORY_MCP_PROJECT", raising=False) + + # --- cat --- @@ -392,6 +405,8 @@ async def test_grep_rejects_bad_arguments(kwargs, message): @pytest.mark.asyncio async def test_ls_root_listing(client, test_graph, test_project): + # New contract (#1415): with no project addressed, ls "/" lists projects as + # mount points — this test pins the project-scoped case via project=. result = await ls(project=test_project.name) assert result["total"] == 1 @@ -417,6 +432,7 @@ async def test_ls_directory_contents(client, test_graph, test_project): @pytest.mark.asyncio async def test_ls_empty_project(client, test_project): + # Project-scoped case (#1415): project= bypasses the mount-point view. result = await ls(project=test_project.name) assert result["total"] == 0 @@ -614,3 +630,445 @@ async def test_man_query_missing_manual_project_raises(client, test_project): # silently searching the wrong project. with pytest.raises(RuntimeError, match="no credentials found"): await man(query="anything") + + +# --- project-qualified routing (#1415) --- +# Projects are mount points: '/path' inputs route to that project, an +# explicit project must agree with a path prefix, and multi-project configs +# refuse unqualified input with the active project list instead of defaulting. + + +# -- ls "/" mount-point view -- + + +@pytest.mark.asyncio +async def test_ls_root_lists_projects_as_mount_points( + client, test_project, second_project, no_project_constraint +): + """ls "/" with no project addressed is the mount table, sorted by name.""" + result = await ls() + + assert result["total"] == 2 + assert result["has_more"] is False + assert [node["name"] for node in result["nodes"]] == ["second-project", "test-project"] + for node in result["nodes"]: + assert node["type"] == "directory" + # directory_path is the copyable '/' prefix form. + assert node["directory_path"] == f"/{node['permalink']}" + + +@pytest.mark.asyncio +async def test_ls_root_mount_view_in_single_project_config( + client, test_graph, test_project, no_project_constraint +): + """The mount view is unconditional (#1415): even a single-project config + lists the mount table at "/" so in-band discovery is uniform.""" + result = await ls() + + assert result["total"] == 1 + assert result["nodes"][0]["name"] == "test-project" + assert result["nodes"][0]["directory_path"] == "/test-project" + assert result["nodes"][0]["type"] == "directory" + + +@pytest.mark.asyncio +async def test_ls_mount_view_paginates_over_project_rows( + client, test_project, second_project, no_project_constraint +): + first = await ls(page=1, page_size=1) + last = await ls(page=2, page_size=1) + + assert first["total"] == 2 + assert first["has_more"] is True + assert [node["name"] for node in first["nodes"]] == ["second-project"] + assert [node["name"] for node in last["nodes"]] == ["test-project"] + assert last["has_more"] is False + + +# -- rule 1: first-segment routing per verb -- + + +@pytest.mark.asyncio +async def test_ls_single_segment_project_name_lists_its_root( + client, test_project, second_project, no_project_constraint +): + await write_note( + title="Second Root Note", + directory="notes", + content="# Second Root Note\n\nsecond project content", + project="second-project", + ) + + result = await ls("second-project") + + names = {node["name"] for node in result["nodes"]} + assert names == {"notes"} + + +@pytest.mark.asyncio +async def test_ls_qualified_path_routes_into_project_directory( + client, test_project, second_project, no_project_constraint +): + await write_note( + title="Second Dir Note", + directory="notes", + content="# Second Dir Note", + project="second-project", + ) + + result = await ls("/second-project/notes") + + names = {node["name"] for node in result["nodes"]} + assert names == {"Second Dir Note.md"} + + +@pytest.mark.asyncio +async def test_find_qualified_path_routes_to_project( + client, test_project, second_project, no_project_constraint +): + await write_note( + title="Second Find Note", + directory="notes", + content="# Second Find Note", + project="second-project", + ) + + result = await find("second-project", name="*.md") + + names = {node["name"] for node in result["nodes"]} + assert names == {"Second Find Note.md"} + + +@pytest.mark.asyncio +async def test_cat_qualified_identifier_equals_explicit_project_read( + client, test_graph, test_project, second_project, no_project_constraint +): + """'test-project/test/root' with no project param reads the same note as + project='test-project' + 'test/root'. + + The payloads are not identical, and deliberately so: each answers in the + addressing frame its caller used, so the file_path it hands back is one the + same call shape accepts again. Content is what must match. + """ + qualified = await cat("test-project/test/root") + explicit = await cat("test/root", project=test_project.name) + + assert qualified["title"] == explicit["title"] == "Root" + assert qualified["content"] == explicit["content"] + + # Each frame's file_path round-trips in that same frame. + assert qualified["file_path"] == f"test-project/{explicit['file_path']}" + assert (await cat(qualified["file_path"]))["title"] == "Root" + assert (await cat(explicit["file_path"], project=test_project.name))["title"] == "Root" + + +@pytest.mark.asyncio +async def test_tool_output_permalink_round_trips_into_cat( + client, test_project, second_project, no_project_constraint +): + """Round trip: stored permalinks are already project-qualified, so tail's + output is a valid cat identifier with no project param anywhere — and the + mount view's prefix is that permalink's first segment.""" + await write_note( + title="Round Trip", + directory="notes", + content="# Round Trip\n\nround trip body", + project="second-project", + ) + + rows = await tail(project="second-project") + permalink = next(row["permalink"] for row in rows if row["title"] == "Round Trip") + assert permalink == "second-project/notes/round-trip" + + mounts = await ls() + mount_prefixes = {node["directory_path"] for node in mounts["nodes"]} + assert f"/{permalink.split('/', 1)[0]}" in mount_prefixes + + result = await cat(permalink) + + assert result["title"] == "Round Trip" + assert "round trip body" in result["content"] + + +# -- rule 2: explicit project + prefix agree/conflict -- + + +@pytest.mark.asyncio +async def test_explicit_project_with_agreeing_prefix_strips( + client, test_graph, test_project, second_project, no_project_constraint +): + qualified = await ls("/test-project/test", project=test_project.name) + relative = await ls("/test", project=test_project.name) + + # Same listing, each in its caller's addressing frame (see cat's twin above). + assert qualified["total"] == relative["total"] == 5 + assert [node["name"] for node in qualified["nodes"]] == [ + node["name"] for node in relative["nodes"] + ] + assert [node["file_path"] for node in qualified["nodes"]] == [ + f"test-project/{node['file_path']}" for node in relative["nodes"] + ] + + +@pytest.mark.asyncio +async def test_explicit_project_with_conflicting_prefix_refuses( + client, test_project, second_project, no_project_constraint +): + """A disagreeing prefix is never silently resolved either way.""" + with pytest.raises( + ProjectPrefixConflictError, + match="path names project 'second-project' but project 'test-project' was passed", + ): + await cat("second-project/notes/foo", project=test_project.name) + + +# -- rule 4: multi-project unqualified refusal, self-teaching message -- + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("verb", "kwargs", "subject"), + [ + ("cat", {"identifier": "notes/foo"}, "no project 'notes'"), + ("ls", {"path": "/notes"}, "no project 'notes'"), + ("find", {"path": "/x"}, "no project 'x'"), + ("grep", {"pattern": "needle"}, "no project specified"), + ("tail", {}, "no project specified"), + ], +) +async def test_unqualified_input_refuses_in_multi_project_config( + client, test_project, second_project, no_project_constraint, verb, kwargs, subject +): + """Each verb refuses rather than silently defaulting, listing every active + project in copyable prefix form.""" + tool = {"cat": cat, "ls": ls, "find": find, "grep": grep, "tail": tail}[verb] + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await tool(**kwargs) + + assert str(excinfo.value) == (f"{subject} — active projects: second-project/, test-project/") + + +@pytest.mark.asyncio +async def test_grep_argument_validation_precedes_refusal( + client, test_project, second_project, no_project_constraint +): + """Bad-argument errors keep firing before any routing decision.""" + with pytest.raises(ValueError, match="pattern must not be empty"): + await grep("") + + +# -- rule 5: single-project passthrough and near-collisions -- + + +@pytest.mark.asyncio +async def test_single_project_unqualified_paths_pass_through( + client, test_graph, test_project, no_project_constraint +): + """One configured project keeps today's ergonomics: unqualified paths route + to the default project, and 'test' is not falsely stripped as a prefix of + 'test-project' (permalink comparison, not startswith).""" + listing = await ls("/test") + assert listing["total"] == 5 + + note = await cat("test/root") + assert note["title"] == "Root" + + rows = await tail() + assert {row["title"] for row in rows} >= {"Root"} + + found = await grep("Root", literal=True) + assert found["results"] + + +@pytest.mark.asyncio +async def test_project_named_folder_is_reachable_double_qualified( + client, test_project, second_project, no_project_constraint +): + """Collision rule: the project wins the first segment, so a top-level folder + named like its own project is addressed by double-qualifying.""" + await write_note( + title="Shadowed", + directory="second-project", + content="# Shadowed\n\nshadowed body", + project="second-project", + ) + + listing = await ls("second-project/second-project") + assert {node["name"] for node in listing["nodes"]} == {"Shadowed.md"} + + result = await cat("second-project/second-project/shadowed") + assert result["title"] == "Shadowed" + + +@pytest.mark.asyncio +async def test_cat_bare_project_name_is_an_error( + client, test_project, second_project, no_project_constraint +): + with pytest.raises(ValueError, match="names a project, not a note"): + await cat("second-project") + + +# -- env constraint (BASIC_MEMORY_MCP_PROJECT) -- + + +@pytest.mark.asyncio +async def test_env_constraint_counts_as_explicit_project( + client, test_graph, test_project, second_project, monkeypatch +): + """The env constraint participates exactly like the project param: no + refusal, agreeing prefixes strip, disagreeing prefixes conflict, and + ls "/" lists the constrained project's root, not the mount table.""" + monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", test_project.name) + + rows = await tail() + assert {row["title"] for row in rows} >= {"Root"} + + stripped = await ls("/test-project/test") + assert stripped["total"] == 5 + + constrained_root = await ls() + assert {node["name"] for node in constrained_root["nodes"]} == {"test"} + + with pytest.raises(ProjectPrefixConflictError): + await cat("second-project/notes/foo") + + +# -- project_id passthrough -- + + +@pytest.mark.asyncio +async def test_project_id_routes_without_prefix_parsing( + client, test_graph, test_project, second_project, no_project_constraint +): + """project_id routes by external UUID and bypasses prefix parsing entirely, + so a multi-project config needs no qualification.""" + result = await cat("test/root", project_id=test_project.external_id) + + assert result["title"] == "Root" + + +# -- round-trip coherence: a returned path is an accepted path (#1421) -- +# The property belongs to the routing layer, not to any one verb, so these tests +# enumerate the verbs rather than naming them. A seventh posix verb that accepts +# a path has to answer for the property here before it can ship. + + +def _path_accepting_posix_verbs() -> set[str]: + """Posix verbs whose first parameter is a routable path or identifier. + + Derived from the tools themselves so a new one cannot slip past the round-trip + tests below by simply not being listed. + """ + import inspect + + verbs = {} + for verb in (cat, grep, ls, find, tail, man): + fn = getattr(verb, "fn", verb) + first = next(iter(inspect.signature(fn).parameters), None) + if first in {"path", "identifier"}: + verbs[fn.__name__] = verb + return set(verbs) + + +def test_path_accepting_verbs_are_the_ones_covered_below(): + """Pins the set the round-trip tests cover. Adding a path-accepting verb + fails here, which is the prompt to give it the same guarantee — the class is + closed by this enumeration, not by every author remembering.""" + assert _path_accepting_posix_verbs() == {"cat", "ls", "find"} + + +@pytest.mark.asyncio +async def test_ls_returned_paths_route_back_to_the_same_project( + client, test_project, second_project, no_project_constraint +): + """A qualified `ls` advertises child paths; feeding one back must reach the + same project. Returning the API's project-relative '/notes' refused as + unqualified — or, with a project mounted as 'notes', opened that one.""" + await write_note( + title="Second Root Note", + directory="notes", + content="# Second Root Note", + project="second-project", + ) + + listing = await ls("second-project") + child = next(node for node in listing["nodes"] if node["name"] == "notes") + assert child["directory_path"] == "/second-project/notes" + + # The advertised path is accepted verbatim, with no project param. + nested = await ls(child["directory_path"]) + assert {node["name"] for node in nested["nodes"]} == {"Second Root Note.md"} + + +@pytest.mark.asyncio +async def test_find_returned_paths_route_back_to_the_same_project( + client, test_project, second_project, no_project_constraint +): + """find advertises both directory_path and file_path; each must address the + project the call routed to, so `cat` and `ls` accept them unchanged.""" + await write_note( + title="Second Root Note", + directory="notes", + content="# Second Root Note", + project="second-project", + ) + + listing = await find("second-project") + file_node = next(node for node in listing["nodes"] if node["type"] == "file") + dir_node = next(node for node in listing["nodes"] if node["type"] == "directory") + + assert file_node["file_path"].startswith("second-project/") + assert dir_node["directory_path"].startswith("/second-project") + + assert (await cat(file_node["file_path"]))["title"] == "Second Root Note" + assert (await ls(dir_node["directory_path"]))["total"] >= 1 + + +@pytest.mark.asyncio +async def test_unrouted_listings_keep_project_relative_paths( + client, test_graph, test_project, second_project, no_project_constraint +): + """The prefix goes back only when the call put it in the path. An explicit + project param is a different addressing frame: those paths are fed back with + the same param, so re-prefixing them would break that round trip instead.""" + listing = await ls("/test", project=test_project.name) + + assert all(not node["file_path"].startswith("test-project/") for node in listing["nodes"]) + assert (await ls("/test", project=test_project.name))["total"] == listing["total"] + + +@pytest.mark.asyncio +async def test_routed_cat_never_rewrites_note_frontmatter( + client, test_project, second_project, no_project_constraint +): + """Re-qualification touches transport metadata, never note content. + + Frontmatter is the author's own YAML and may legitimately carry keys spelled + like transport fields. Rewriting by key name anywhere in the payload turned + `file_path: imports/source.md` into `second-project/imports/source.md`, so a + routed read returned frontmatter that disagreed with both its own `content` + and the file on disk. Position, not spelling, separates the two. + """ + await write_note( + title="Imported Note", + directory="notes", + content="# Imported Note\n\nbody", + project="second-project", + metadata={"file_path": "imports/source.md", "directory_path": "/imports"}, + ) + + payload = await cat("second-project/notes/imported-note") + + # The stored file is the authority: parse its frontmatter block and require + # the response to reproduce it exactly. + stored = (Path(second_project.path) / "notes" / "Imported Note.md").read_text() + _, _, rest = stored.partition("---\n") + block, _, _ = rest.partition("\n---") + assert yaml.safe_load(block) == payload["frontmatter"] + assert payload["frontmatter"]["file_path"] == "imports/source.md" + assert payload["frontmatter"]["directory_path"] == "/imports" + + # The transport path is still qualified, so the round trip holds. + assert payload["file_path"] == "second-project/notes/Imported Note.md" + assert (await cat(payload["file_path"]))["title"] == "Imported Note" diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index b22d6d21b..a273b3317 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -221,6 +221,42 @@ async def test_get_project_info(project_service: ProjectService, test_graph, tes assert isinstance(info.system, SystemStatus) +@pytest.mark.asyncio +@pytest.mark.parametrize("name", ["💥", "!!!", "---", "/", "/foo", "/a/b"]) +async def test_add_project_rejects_names_without_a_permalink( + project_service: ProjectService, name: str +): + """A project's permalink is its address, and generate_permalink reduces pure + punctuation or emoji to "". + + The resolver matches a permalink segment by segment against a path whose + leading slashes are already stripped, so any empty segment is unmatchable: + "" advertises at the root as '/', indistinguishable from every other such + project, and '/foo' advertises '//foo' and cannot be entered. Either breaks + the rule that anything the mount view lists is addressable, so both are + refused where projects are created. + """ + with tempfile.TemporaryDirectory() as temp_dir: + with pytest.raises(ValueError, match="no usable permalink"): + await project_service.add_project(name, temp_dir) + + +@pytest.mark.asyncio +async def test_add_project_rejects_a_colliding_permalink( + project_service: ProjectService, test_project +): + """Permalinks are addresses, so two projects cannot share one. + + 'My Docs' beside 'my-docs' advertised both mounts at the same path and the + resolver could only pick one, leaving the other unreachable and its paths + reading the wrong project's content. + """ + with tempfile.TemporaryDirectory() as temp_dir: + colliding = test_project.name.upper().replace("-", " ") + with pytest.raises(ValueError, match="same permalink as existing project"): + await project_service.add_project(colliding, temp_dir) + + @pytest.mark.asyncio async def test_add_project_async(project_service: ProjectService): """Test adding a project with the updated async method."""