From fbad63975fbb2a304dd59a91bf31170e050fb1e4 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 18 Oct 2026 10:07:56 -0500 Subject: [PATCH 1/3] fix(mcp): name other-workspace projects in POSIX refusals and ls / (#1778) A hosted session that can reach several workspaces only listed its own workspace's projects. A path whose first segment named a project in another workspace was refused with the local mounts only, so an agent retrying from the message could never find the spelling that works. - The refusal now names the workspace holding a same-named project and both spellings that reach it ('//' or project_id). It still refuses: an unqualified first segment never routes to another workspace (#1421). - When nothing matches, the refusal says how other workspaces' projects are addressed and points to list_memory_projects. - `ls /` adds the accessible workspace slugs and a note. They are not rows, since a bare slug is not a routable path. - Only cloud-routed sessions consult workspaces, so local refusals and root listings make no new control-plane calls. - ls(1), cat(1) and find(1) document the workspace-qualified form. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KjsDUcHANBa17M84sKxFG3 Signed-off-by: phernandez --- src/basic_memory/man/man1/cat(1).md | 7 ++ src/basic_memory/man/man1/find(1).md | 7 ++ src/basic_memory/man/man1/ls(1).md | 11 +++ src/basic_memory/mcp/project_context.py | 84 ++++++++++++++++++- src/basic_memory/mcp/tools/posix_tools.py | 18 ++++- tests/mcp/test_project_path_routing.py | 98 +++++++++++++++++++++-- 6 files changed, 214 insertions(+), 11 deletions(-) diff --git a/src/basic_memory/man/man1/cat(1).md b/src/basic_memory/man/man1/cat(1).md index d2076c271..4a195a97b 100644 --- a/src/basic_memory/man/man1/cat(1).md +++ b/src/basic_memory/man/man1/cat(1).md @@ -33,6 +33,13 @@ duplicate), or truncated to an approximate token budget (`--max-tokens`). `--lines` and `--section` cannot be combined. A section response includes start_line and end_line for follow-up `--lines` reads. +Projects in another cloud workspace are not among the session's own +projects. Address them as `//`, or pass +`--project-id` (the MCP tool's `project_id`). A bare `/` never +reaches another workspace; when its first segment names a project there, the +refusal names the workspace and both spellings that work. +`list_memory_projects` (`bm project list`) lists every workspace's projects. + On a TTY the note renders as formatted Markdown; `--plain` writes the raw content to stdout (slice details go to stderr); `--json`, or piped output, emits the structured payload with slice metadata (start_line, end_line, diff --git a/src/basic_memory/man/man1/find(1).md b/src/basic_memory/man/man1/find(1).md index c8605ef13..85aab88ed 100644 --- a/src/basic_memory/man/man1/find(1).md +++ b/src/basic_memory/man/man1/find(1).md @@ -58,6 +58,13 @@ directory named `" specs "` is addressed by that exact spelling. routing prefix, a mount point rather than a subtree, and scopes to that project's root. +Projects in another cloud workspace are not among the session's own +projects. Address them as `//`, or pass +`--project-id` (the MCP tool's `project_id`). A bare `/` never +reaches another workspace; when its first segment names a project there, the +refusal names the workspace and both spellings that work. +`list_memory_projects` (`bm project list`) lists every workspace's projects. + `--fields` is the SELECT to the predicates' WHERE: each hit comes back as the note's identity — title, permalink, file path, external id, last-updated — plus a `fields` object carrying the named frontmatter values, so a filtered set can diff --git a/src/basic_memory/man/man1/ls(1).md b/src/basic_memory/man/man1/ls(1).md index c5d7cdced..97bf76667 100644 --- a/src/basic_memory/man/man1/ls(1).md +++ b/src/basic_memory/man/man1/ls(1).md @@ -31,6 +31,17 @@ pagination and totals. `bm ls` lists files inside one project; the unrelated `bm project ls` lists projects. +The MCP `ls` tool lists the session's projects as mount points at `/`. A cloud +session that can reach several workspaces also gets their slugs and a note on +reaching their projects. + +Projects in another cloud workspace are not among the session's own +projects. Address them as `//`, or pass +`--project-id` (the MCP tool's `project_id`). A bare `/` never +reaches another workspace; when its first segment names a project there, the +refusal names the workspace and both spellings that work. +`list_memory_projects` (`bm project list`) lists every workspace's projects. + ## OPTIONS - **--page** (default: 1) — Page number (1-indexed) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 1fcde4214..a4c43bf08 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1628,6 +1628,84 @@ def _agreed_route_project( return None +OTHER_WORKSPACES_HINT = ( + "projects in other workspaces are addressed as '//' " + "or by project_id; list_memory_projects lists them" +) + + +async def other_workspace_slugs(context: Optional[Context] = None) -> tuple[str, ...]: + """Slugs of the accessible workspaces, for a cloud session that can see several. + + The ``ls /`` mount view lists this session's own projects only, so without this + an agent reading the root takes it for the whole world (#1778). Every slug + returned is a working '//' prefix, the session's own + included. Empty for a session that does not route to the cloud, so a local + root listing never calls the control plane, and for a single-workspace account. + """ + if not _cloud_workspace_discovery_available(ConfigManager().config): + return () + workspaces = await get_available_workspaces(context=context) + if len(workspaces) < 2: + return () + return tuple(sorted(workspace.slug for workspace in workspaces)) + + +async def _other_workspace_hint( + first_segment: str, + addressable: tuple[AddressableProject, ...], + config: BasicMemoryConfig, + context: Optional[Context] = None, +) -> str | None: + """Teach the spelling that reaches a project the refusal could not route to (#1778). + + The refusal lists this session's mounts, and a project in another accessible + workspace is never one of them, so an agent retrying from the message alone + could not find the spelling that works. This names it instead. + + It only reads the workspace index; it never routes. An unqualified first segment + must not reach another workspace on its own, or an ordinary project-relative + path reads another workspace's same-named project (#1421). The caller has to + choose the qualified spelling or the project_id this hint gives. + + Returns None for a session that does not route to the cloud, or for an account + with a single workspace, so a local refusal reads as it always has. A local + session holding cloud credentials is left out on purpose: the hint would add a + control-plane call to its refusal path, and an unreachable control plane would + then replace the refusal with a network error. + """ + if not _cloud_workspace_discovery_available(config): + return None + try: + index = await _ensure_workspace_project_index(context=context) + except ValueError as exc: + if any(error in str(exc).lower() for error in _WORKSPACE_DISCOVERY_FALLBACK_ERRORS): + return None + raise + + # Trigger: the refused segment spells a project in another accessible workspace. + # Why: that project is reachable, only not by this spelling; the session's own + # mounts are excluded because the mount table already declined the segment. + # Outcome: name each match with both spellings that reach it. + own_project_ids = {project.external_id for project in addressable if project.external_id} + permalink = generate_permalink(first_segment) if first_segment else "" + matches = [ + entry + for entry in index.entries_by_permalink.get(permalink, ()) + if entry.project.external_id not in own_project_ids + ] + if matches: + return "; ".join( + f"project '{first_segment}' is in workspace '{entry.workspace.slug}' — use " + f"'{entry.workspace.slug}/{entry.project.permalink}/' or " + f"project_id='{entry.project.external_id}'" + for entry in matches + ) + if len(index.workspaces) > 1: + return OTHER_WORKSPACES_HINT + return None + + async def resolve_project_path_route( path: str, *, @@ -1797,9 +1875,9 @@ async def resolve_project_path_route( 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)}" - ) + message = f"{subject} — active projects: {_addressable_project_prefixes(addressable)}" + hint = await _other_workspace_hint(first_segment, addressable, config, context=context) + raise UnqualifiedPathRefusedError(f"{message}. {hint}" if hint else message) # A session that addresses at most one project has no ambiguity to protect # against, so unqualified input keeps today's default resolution. diff --git a/src/basic_memory/mcp/tools/posix_tools.py b/src/basic_memory/mcp/tools/posix_tools.py index 73b27928e..6193a09a6 100644 --- a/src/basic_memory/mcp/tools/posix_tools.py +++ b/src/basic_memory/mcp/tools/posix_tools.py @@ -48,9 +48,11 @@ from basic_memory.mcp.async_client import is_factory_mode from basic_memory.mcp.note_reads import read_note_json_by_external_id from basic_memory.mcp.project_context import ( + OTHER_WORKSPACES_HINT, ProjectPathRoute, addressable_projects, get_project_client, + other_workspace_slugs, resolve_project_path_route, ) from basic_memory.mcp.server import POSIX_TOOLS_TAG, mcp, set_posix_tools_visibility @@ -472,6 +474,10 @@ async def _project_mount_listing( — 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. + + A cloud session that can see several workspaces also gets their slugs and a + note on reaching them (#1778). They are not listed as rows: a bare slug is not + a routable path, and every row here must be one. """ rows = [ DirectoryNode( @@ -489,8 +495,16 @@ async def _project_mount_listing( page_size=page_size, total=len(rows), has_more=start + page_size < len(rows), - ) - return listing.model_dump(mode="json") + ).model_dump(mode="json") + workspaces = await other_workspace_slugs(context=context) + if workspaces: + listing["workspaces"] = list(workspaces) + listing["note"] = ( + "the projects above are this session's workspace; " + + OTHER_WORKSPACES_HINT + + f" (workspace slugs: {', '.join(workspaces)})" + ) + return listing @mcp.tool( diff --git a/tests/mcp/test_project_path_routing.py b/tests/mcp/test_project_path_routing.py index 452f639fb..05369942e 100644 --- a/tests/mcp/test_project_path_routing.py +++ b/tests/mcp/test_project_path_routing.py @@ -28,6 +28,7 @@ ProjectPathRoute, ProjectPrefixConflictError, AmbiguousMountError, + OTHER_WORKSPACES_HINT, UnqualifiedPathRefusedError, _agreed_route_project, detect_project_from_identifier_prefix, @@ -1644,8 +1645,11 @@ async def test_cloud_unqualified_first_segment_never_reaches_another_workspace( """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.""" + the other tenant's 'notes' and read it. It must refuse instead (#1421). + + The refusal still teaches the way there (#1778): it names the workspace that + holds 'notes' and the two spellings that reach it on purpose. It never offers + the bare 'notes/' prefix, which is the spelling that just refused.""" cross_workspace_session( session_projects=("research", "engineering"), default_projects=("notes",), @@ -1654,9 +1658,89 @@ async def test_cloud_unqualified_first_segment_never_reaches_another_workspace( 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/") + assert str(excinfo.value) == ( + "no project 'notes' — active projects: engineering/, research/. " + "project 'notes' is in workspace 'acme' — use 'acme/notes/' or " + f"project_id='{_DEFAULT_NOTES_ID}'" + ) + + +@pytest.mark.asyncio +async def test_cloud_hinted_spellings_reach_the_other_workspace_project(cross_workspace_session): + """Both spellings the refusal teaches have to route to the project it named, + or the hint would trade one dead end for another (#1778).""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + qualified = await resolve_project_path_route("acme/notes/foo", project=None, project_id=None) + by_id = await resolve_project_path_route("foo", project=None, project_id=_DEFAULT_NOTES_ID) + + assert qualified.project_id == _DEFAULT_NOTES_ID + assert qualified.path == "foo" + assert by_id.project_id == _DEFAULT_NOTES_ID + + +@pytest.mark.asyncio +async def test_cloud_refusal_points_to_other_workspaces_when_no_project_matches( + cross_workspace_session, +): + """A first segment that names no project anywhere still learns that other + workspaces exist and how to address them, since the mounts listed are only + this workspace's (#1778).""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("My Workspace/notes/foo", project=None, project_id=None) + + assert str(excinfo.value) == ( + "no project 'My Workspace' — active projects: engineering/, research/. " + + OTHER_WORKSPACES_HINT + ) + + +@pytest.mark.asyncio +async def test_cloud_root_listing_names_the_other_workspaces(cross_workspace_session): + """`ls /` lists only this workspace's projects, so it also says that other + workspaces exist and how to reach them (#1778). They are not rows: a bare + slug is not a routable path, and every row must be one.""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + mounts = await ls() + + assert [node["name"] for node in mounts["nodes"]] == ["engineering", "research"] + assert mounts["workspaces"] == ["acme", "beta"] + assert OTHER_WORKSPACES_HINT in mounts["note"] + assert "acme, beta" in mounts["note"] + + +@pytest.mark.asyncio +async def test_cloud_single_workspace_root_listing_adds_nothing(cloud_session): + cloud_session("research", "engineering") + + mounts = await ls() + + assert "workspaces" not in mounts + assert "note" not in mounts + + +@pytest.mark.asyncio +async def test_cloud_single_workspace_refusal_adds_no_hint(cloud_session): + """With one workspace there is nowhere else to point, so the refusal reads as + it always has.""" + cloud_session("research", "engineering") + + 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/, research/" @pytest.mark.asyncio @@ -1687,7 +1771,9 @@ async def test_cloud_empty_route_segment_is_not_a_workspace_route(cross_workspac 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/") + assert str(excinfo.value) == ( + "no project 'acme' — active projects: engineering/, research/. " + OTHER_WORKSPACES_HINT + ) @pytest.mark.asyncio From 6595f5649bcd3b38aa147522e784db7bd24f90f9 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 18 Oct 2026 11:19:52 -0500 Subject: [PATCH 2/3] fix(mcp): match other-workspace hints by longest prefix; keep ls / on discovery failure Review follow-ups for #1780. - The refusal hint matched only the first path segment, so with 'Research' beside 'Research/2026' it pointed 'research/2026/foo' at the shorter project. It now uses split_project_permalink_prefix, the splitter routing uses, and names the remainder in the spelling. - A control-plane failure while listing workspaces turned a working `ls /` into an error. The slugs only annotate the root, so it logs and lists the mounts without them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KjsDUcHANBa17M84sKxFG3 Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 52 +++++++++++++++++-------- tests/mcp/test_project_path_routing.py | 48 ++++++++++++++++++++++- 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index a4c43bf08..40079bd4a 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1645,14 +1645,26 @@ async def other_workspace_slugs(context: Optional[Context] = None) -> tuple[str, """ if not _cloud_workspace_discovery_available(ConfigManager().config): return () - workspaces = await get_available_workspaces(context=context) + # Deferred: ToolError lives in FastMCP's runtime, which must not load at CLI startup (#886). + from fastmcp.exceptions import ToolError + + try: + workspaces = await get_available_workspaces(context=context) + except ToolError as exc: + # Trigger: the control plane timed out or failed while the session's own + # project listing already succeeded. + # Why: the slugs only annotate the root listing; losing them must not turn a + # working `ls /` into an error. + # Outcome: the root lists its mounts without the workspace note. + logger.warning(f"Workspace discovery failed; listing mounts without it: {exc}") + return () if len(workspaces) < 2: return () return tuple(sorted(workspace.slug for workspace in workspaces)) async def _other_workspace_hint( - first_segment: str, + candidate: str, addressable: tuple[AddressableProject, ...], config: BasicMemoryConfig, context: Optional[Context] = None, @@ -1683,23 +1695,31 @@ async def _other_workspace_hint( return None raise - # Trigger: the refused segment spells a project in another accessible workspace. - # Why: that project is reachable, only not by this spelling; the session's own - # mounts are excluded because the mount table already declined the segment. + # Trigger: the refused path's leading segments spell a project in another + # accessible workspace. + # Why: that project is reachable, only not by this spelling. The match uses the + # same longest-prefix splitter as routing, so 'research/2026/foo' names + # 'Research/2026', not a shorter 'Research' beside it. The session's own mounts + # are excluded because the mount table already declined these segments. # Outcome: name each match with both spellings that reach it. own_project_ids = {project.external_id for project in addressable if project.external_id} - permalink = generate_permalink(first_segment) if first_segment else "" - matches = [ - entry - for entry in index.entries_by_permalink.get(permalink, ()) - if entry.project.external_id not in own_project_ids - ] - if matches: + other_entries = { + permalink: tuple( + entry for entry in entries if entry.project.external_id not in own_project_ids + ) + for permalink, entries in index.entries_by_permalink.items() + } + claimed = _split_project_permalink_prefix( + candidate, [permalink for permalink, entries in other_entries.items() if entries] + ) + if claimed is not None: + permalink, remainder = claimed + path = remainder or "" return "; ".join( - f"project '{first_segment}' is in workspace '{entry.workspace.slug}' — use " - f"'{entry.workspace.slug}/{entry.project.permalink}/' or " + f"project '{entry.project.name}' is in workspace '{entry.workspace.slug}' — use " + f"'{entry.workspace.slug}/{entry.project.permalink}/{path}' or " f"project_id='{entry.project.external_id}'" - for entry in matches + for entry in other_entries[permalink] ) if len(index.workspaces) > 1: return OTHER_WORKSPACES_HINT @@ -1876,7 +1896,7 @@ async def resolve_project_path_route( first_segment = candidate.split("/", 1)[0] if candidate else "" subject = f"no project '{first_segment}'" if first_segment else "no project specified" message = f"{subject} — active projects: {_addressable_project_prefixes(addressable)}" - hint = await _other_workspace_hint(first_segment, addressable, config, context=context) + hint = await _other_workspace_hint(candidate, addressable, config, context=context) raise UnqualifiedPathRefusedError(f"{message}. {hint}" if hint else message) # A session that addresses at most one project has no ambiguity to protect diff --git a/tests/mcp/test_project_path_routing.py b/tests/mcp/test_project_path_routing.py index 05369942e..636686fc7 100644 --- a/tests/mcp/test_project_path_routing.py +++ b/tests/mcp/test_project_path_routing.py @@ -17,6 +17,7 @@ from typing import AsyncIterator, Optional import pytest +from fastmcp.exceptions import ToolError import basic_memory.mcp.async_client as async_client import basic_memory.mcp.project_context as project_context @@ -1356,6 +1357,8 @@ async def test_cloud_project_listing_is_fetched_once_per_request(cloud_session): _SESSION_RESEARCH_ID = "33333333-3333-3333-3333-333333333333" _SESSION_ENGINEERING_ID = "44444444-4444-4444-4444-444444444444" _DEFAULT_NOTES_ID = "55555555-5555-5555-5555-555555555555" +_DEFAULT_RESEARCH_ID = "66666666-6666-6666-6666-666666666666" +_DEFAULT_RESEARCH_2026_ID = "77777777-7777-7777-7777-777777777777" # 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 @@ -1367,6 +1370,8 @@ async def test_cloud_project_listing_is_fetched_once_per_request(cloud_session): ("session-tenant", "engineering"): _SESSION_ENGINEERING_ID, ("default-tenant", "docs"): _DEFAULT_DOCS_ID, ("default-tenant", "notes"): _DEFAULT_NOTES_ID, + ("default-tenant", "Research"): _DEFAULT_RESEARCH_ID, + ("default-tenant", "Research/2026"): _DEFAULT_RESEARCH_2026_ID, } @@ -1660,7 +1665,7 @@ async def test_cloud_unqualified_first_segment_never_reaches_another_workspace( assert str(excinfo.value) == ( "no project 'notes' — active projects: engineering/, research/. " - "project 'notes' is in workspace 'acme' — use 'acme/notes/' or " + "project 'notes' is in workspace 'acme' — use 'acme/notes/foo' or " f"project_id='{_DEFAULT_NOTES_ID}'" ) @@ -1682,6 +1687,25 @@ async def test_cloud_hinted_spellings_reach_the_other_workspace_project(cross_wo assert by_id.project_id == _DEFAULT_NOTES_ID +@pytest.mark.asyncio +async def test_cloud_refusal_names_the_longest_matching_project(cross_workspace_session): + """Routing takes the longest project permalink, so the hint must too: with + 'Research' beside 'Research/2026', 'research/2026/foo' names 'Research/2026'. + Pointing at 'Research' would teach a route into the wrong project (#1780 review).""" + cross_workspace_session( + session_projects=("docs", "engineering"), + default_projects=("Research", "Research/2026"), + ) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("research/2026/foo", project=None, project_id=None) + + assert str(excinfo.value).endswith( + "project 'Research/2026' is in workspace 'acme' — use 'acme/research/2026/foo' or " + f"project_id='{_DEFAULT_RESEARCH_2026_ID}'" + ) + + @pytest.mark.asyncio async def test_cloud_refusal_points_to_other_workspaces_when_no_project_matches( cross_workspace_session, @@ -1721,6 +1745,28 @@ async def test_cloud_root_listing_names_the_other_workspaces(cross_workspace_ses assert "acme, beta" in mounts["note"] +@pytest.mark.asyncio +async def test_cloud_root_listing_survives_failed_workspace_discovery( + cross_workspace_session, monkeypatch +): + """The workspace slugs only annotate `ls /`; a control-plane failure must leave + the session's own mounts listed (#1780 review).""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + async def failing_workspaces(context=None): + raise ToolError("control plane unavailable") + + monkeypatch.setattr(project_context, "get_available_workspaces", failing_workspaces) + + mounts = await ls() + + assert [node["name"] for node in mounts["nodes"]] == ["engineering", "research"] + assert "workspaces" not in mounts + + @pytest.mark.asyncio async def test_cloud_single_workspace_root_listing_adds_nothing(cloud_session): cloud_session("research", "engineering") From 4b2dc7e9f8a3bf0a6f5121df60264f0261cc99a2 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 18 Oct 2026 12:23:23 -0500 Subject: [PATCH 3/3] fix(mcp): keep POSIX refusals when discovery fails; skip mount-shadowed hints - catch ToolError from workspace discovery in the other-workspace hint, so the refusal still lists this session's mounts - offer only project_id when a mount would claim the qualified spelling Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KjsDUcHANBa17M84sKxFG3 Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 40 ++++++++++++++++++-- tests/mcp/test_project_path_routing.py | 49 +++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 40079bd4a..c737346b0 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1663,6 +1663,31 @@ async def other_workspace_slugs(context: Optional[Context] = None) -> tuple[str, return tuple(sorted(workspace.slug for workspace in workspaces)) +def _hint_for_other_workspace_project( + entry: WorkspaceProjectEntry, + path: str, + addressable: tuple[AddressableProject, ...], +) -> str: + """Name the spellings that reach one project in another workspace. + + The qualified spelling is offered only when routing would actually honor it. + Mounts claim leading segments first, so when a mount's permalink equals the + workspace slug, 'acme/docs/foo' reads 'docs/foo' inside the mount 'acme' instead + of project 'docs' in workspace 'acme'. The project_id cannot be shadowed, so it + is always offered. + """ + qualified = f"{entry.workspace.slug}/{entry.project.permalink}/{path}" + try: + shadowed = _claim_mount_prefix(qualified, addressable) is not None + except AmbiguousMountError: + shadowed = True + by_id = f"project_id='{entry.project.external_id}'" + where = f"project '{entry.project.name}' is in workspace '{entry.workspace.slug}'" + if shadowed: + return f"{where} — use {by_id} with path '{path}'" + return f"{where} — use '{qualified}' or {by_id}" + + async def _other_workspace_hint( candidate: str, addressable: tuple[AddressableProject, ...], @@ -1688,12 +1713,23 @@ async def _other_workspace_hint( """ if not _cloud_workspace_discovery_available(config): return None + # Deferred: ToolError lives in FastMCP's runtime, which must not load at CLI startup (#886). + from fastmcp.exceptions import ToolError + try: index = await _ensure_workspace_project_index(context=context) except ValueError as exc: if any(error in str(exc).lower() for error in _WORKSPACE_DISCOVERY_FALLBACK_ERRORS): return None raise + except ToolError as exc: + # Trigger: the control plane timed out or failed while the session's own + # project listing already succeeded. + # Why: the hint only enriches the refusal; losing it must not replace the + # refusal with an unrelated tool failure. + # Outcome: the refusal still lists this session's mounts, without the hint. + logger.warning(f"Workspace discovery failed; refusing without a hint: {exc}") + return None # Trigger: the refused path's leading segments spell a project in another # accessible workspace. @@ -1716,9 +1752,7 @@ async def _other_workspace_hint( permalink, remainder = claimed path = remainder or "" return "; ".join( - f"project '{entry.project.name}' is in workspace '{entry.workspace.slug}' — use " - f"'{entry.workspace.slug}/{entry.project.permalink}/{path}' or " - f"project_id='{entry.project.external_id}'" + _hint_for_other_workspace_project(entry, path, addressable) for entry in other_entries[permalink] ) if len(index.workspaces) > 1: diff --git a/tests/mcp/test_project_path_routing.py b/tests/mcp/test_project_path_routing.py index 636686fc7..5f0efdab2 100644 --- a/tests/mcp/test_project_path_routing.py +++ b/tests/mcp/test_project_path_routing.py @@ -1359,6 +1359,7 @@ async def test_cloud_project_listing_is_fetched_once_per_request(cloud_session): _DEFAULT_NOTES_ID = "55555555-5555-5555-5555-555555555555" _DEFAULT_RESEARCH_ID = "66666666-6666-6666-6666-666666666666" _DEFAULT_RESEARCH_2026_ID = "77777777-7777-7777-7777-777777777777" +_SESSION_ACME_ID = "88888888-8888-8888-8888-888888888888" # 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 @@ -1368,6 +1369,7 @@ async def test_cloud_project_listing_is_fetched_once_per_request(cloud_session): ("session-tenant", "docs"): _SESSION_DOCS_ID, ("session-tenant", "research"): _SESSION_RESEARCH_ID, ("session-tenant", "engineering"): _SESSION_ENGINEERING_ID, + ("session-tenant", "acme"): _SESSION_ACME_ID, ("default-tenant", "docs"): _DEFAULT_DOCS_ID, ("default-tenant", "notes"): _DEFAULT_NOTES_ID, ("default-tenant", "Research"): _DEFAULT_RESEARCH_ID, @@ -1706,6 +1708,53 @@ async def test_cloud_refusal_names_the_longest_matching_project(cross_workspace_ ) +@pytest.mark.asyncio +async def test_cloud_refusal_omits_a_qualified_spelling_a_mount_would_claim( + cross_workspace_session, +): + """A mount named like the other workspace's slug claims the qualified spelling + first: 'acme/notes/foo' reads 'notes/foo' inside mount 'acme'. The hint then + offers only the project_id, which no mount can shadow (#1780 review).""" + cross_workspace_session( + session_projects=("acme", "engineering"), + default_projects=("notes",), + ) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("notes/foo", project=None, project_id=None) + + assert str(excinfo.value).endswith( + f"project 'notes' is in workspace 'acme' — use project_id='{_DEFAULT_NOTES_ID}' " + "with path 'foo'" + ) + by_id = await resolve_project_path_route("foo", project=None, project_id=_DEFAULT_NOTES_ID) + assert by_id.project_id == _DEFAULT_NOTES_ID + + +@pytest.mark.asyncio +async def test_cloud_refusal_survives_failed_workspace_discovery( + cross_workspace_session, monkeypatch +): + """The hint only enriches the refusal; a control-plane failure must still + refuse with this session's mounts, not surface an unrelated error (#1780 review). + A single segment never reaches the workspace-qualified rule, so the hint is + its only discovery call.""" + cross_workspace_session( + session_projects=("research", "engineering"), + default_projects=("notes",), + ) + + async def failing_workspaces(context=None): + raise ToolError("control plane unavailable") + + monkeypatch.setattr(project_context, "get_available_workspaces", failing_workspaces) + + with pytest.raises(UnqualifiedPathRefusedError) as excinfo: + await resolve_project_path_route("notes", project=None, project_id=None) + + assert str(excinfo.value) == "no project 'notes' — active projects: engineering/, research/" + + @pytest.mark.asyncio async def test_cloud_refusal_points_to_other_workspaces_when_no_project_matches( cross_workspace_session,