diff --git a/CHANGELOG.md b/CHANGELOG.md index 685999a11..1dc6d7bef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,14 @@ gains `basic-memory-diagnostics(3)`, closing the one gap between the section-3 corpus and the tool registry. +- Notes are readable as MCP resources. Every `memory://` URL Basic Memory hands out + now answers the standard `resources/read`: `memory:///` returns the + note's raw markdown, frontmatter included, with the identifier accepted as a + permalink, title, or file path. Unknown notes point at `search_notes`; binary files + point at `read_content`. `memory://man/...` keeps answering as the manual and + `memory:////info` as project info, whichever template the + server matches first. + - **#610**: The manual ships in the package and is served over MCP. The 21 section-3 pages (one per MCP tool -- `search-notes(3)`, `write-note(3)`, ...) now live in `src/basic_memory/man/man3/` as canonical, portable notes. The MCP server exposes @@ -37,6 +45,13 @@ ### Bug Fixes +- The `memory://{workspace}/{project}/info` resource is now actually readable over + `resources/read`: it returned a Pydantic model, which the resource runtime rejects + (`contents must be str, bytes, or list[ResourceContent]`), so every served read + failed. It returns the validated response as JSON text now. Found by the new + note-resource tests, which read through a real client session instead of calling + the handler directly. + - **#1344**: Deleting a note no longer erases the relations pointing at it. The `relation.to_id` foreign key is now `ON DELETE SET NULL` rather than `ON DELETE CASCADE`, and `Entity.incoming_relations` no longer cascades deletes diff --git a/src/basic_memory/mcp/resources/__init__.py b/src/basic_memory/mcp/resources/__init__.py index ab5cbc0a9..8cbac316d 100644 --- a/src/basic_memory/mcp/resources/__init__.py +++ b/src/basic_memory/mcp/resources/__init__.py @@ -1,6 +1,7 @@ """Bundled MCP resources for Basic Memory.""" from basic_memory.mcp.resources.man import manual_index, manual_page +from basic_memory.mcp.resources.notes import note_resource from basic_memory.mcp.resources.project_info import project_info -__all__ = ["manual_index", "manual_page", "project_info"] +__all__ = ["manual_index", "manual_page", "note_resource", "project_info"] diff --git a/src/basic_memory/mcp/resources/man.py b/src/basic_memory/mcp/resources/man.py index 434eca620..fd6b92ac2 100644 --- a/src/basic_memory/mcp/resources/man.py +++ b/src/basic_memory/mcp/resources/man.py @@ -7,6 +7,7 @@ ``search_notes`` — so an agent's first guess resolves. """ +from fastmcp import Context from fastmcp.exceptions import ResourceError from fastmcp.resources import FileResource from pydantic import AnyUrl @@ -41,17 +42,32 @@ async def manual_index() -> str: ), mime_type="text/markdown", ) -def manual_page(ref: str) -> str: +async def manual_page(ref: str, context: Context | None = None) -> str: try: page_ref = parse_page_ref(ref) except ValueError as error: - raise ResourceError(f"{error}; read {MANUAL_INDEX_URI} for the index") from error - page = find_page(page_ref) - if page is None: - raise ResourceError( + page = None + miss = ResourceError(f"{error}; read {MANUAL_INDEX_URI} for the index") + else: + page = find_page(page_ref) + miss = ResourceError( f"No manual entry for {page_ref.display}; read {MANUAL_INDEX_URI} for the index" ) - return page.read() + if page is not None: + return page.read() + + # This template registers first and wins ties for memory://man/... over the + # notes template, and nothing reserves `man` as a project name — so when no + # page matches, the URI may be a note in a project really named man. + # Deferred import: notes.py imports this module. + from basic_memory.mcp.resources.notes import NoteNotFoundError, read_note_markdown + + try: + return await read_note_markdown(f"man/{ref}", context) + except NoteNotFoundError: + # Neither a page nor a note — the manual's hint is the useful one; an + # operational note failure keeps its own cause instead. + raise miss from None # Concrete resources are what clients list; the template only answers reads. diff --git a/src/basic_memory/mcp/resources/notes.py b/src/basic_memory/mcp/resources/notes.py new file mode 100644 index 000000000..f2c7866ea --- /dev/null +++ b/src/basic_memory/mcp/resources/notes.py @@ -0,0 +1,141 @@ +"""Notes as MCP resources. + +Basic Memory hands out ``memory://`` URLs everywhere — pages, prompts, handoffs, +conversation summaries — so reading one through the standard MCP +``resources/read`` must work too. ``memory://{project}/{path*}`` returns the +note's raw markdown, exactly as it sits on disk, frontmatter included. +""" + +from fastmcp import Context +from fastmcp.exceptions import ResourceError, ToolError + +from basic_memory.config import ConfigManager +from basic_memory.mcp.project_context import ( + detect_project_from_memory_url_prefix, + get_project_client, + resolve_project_and_path, +) +from basic_memory.mcp.resources.man import manual_page +from basic_memory.mcp.resources.project_info import project_info +from basic_memory.mcp.server import mcp +from basic_memory.mcp.tools.utils import call_get, call_post +from basic_memory.utils import generate_permalink + +NOTE_TEMPLATE = "memory://{project}/{path*}" + + +class NoteNotFoundError(ResourceError): + """The identifier resolved to no note — distinct from operational failures. + + Fallback dispatchers (the manual namespace, the /info shape) may only swap + in their own error when the note is confirmed missing; auth, server, and + transport failures must keep their cause. + """ + + +async def _route_for(identifier: str, context: Context | None) -> str | None: + """The project route the URI's prefix names, or None for the default client. + + The canonical prefix detection decides, covering configured local projects + and workspace-qualified cloud routes alike: the client must be opened for + the URI's own project, because a cloud project needs its own transport. + + One refinement: with permalinks_include_project=False a *local* project + match is a directory collision — the active project owns unprefixed + permalinks — so it is dropped. Workspace-qualified cloud routes keep their + workspace/project segments regardless of that flag, so they still route. + """ + config = ConfigManager().config + route = await detect_project_from_memory_url_prefix( + f"memory://{identifier}", config, context=context + ) + if route is None or config.permalinks_include_project: + return route + requested = generate_permalink(route) + for configured_name in config.projects: + if generate_permalink(configured_name) == requested: + return None + return route + + +async def read_note_markdown(identifier: str, context: Context | None) -> str: + """Read one note's raw markdown by its memory:// identifier. + + Routing uses the same semantics as the tools: a leading segment that names a + configured project routes there (with that project's own client — cloud or + local); otherwise — legacy unprefixed permalinks, + permalinks_include_project=False — resolve_project_and_path resolves the + whole path in the active/default project. + """ + try: + route = await _route_for(identifier, context) + async with get_project_client(route, context) as (client, active_project): + target, entity_path, _ = await resolve_project_and_path( + client, f"memory://{identifier}", active_project.name, context + ) + # strict: a resource read returns the addressed document or an error — + # never the fuzzy-search guess the tools use for suggestions. Only this + # call's not-found is a confirmed note miss; a 'Project not found' from + # routing above must surface as the route failure it is. + try: + resolved = await call_post( + client, + f"/v2/projects/{target.external_id}/knowledge/resolve", + json={"identifier": entity_path, "strict": True}, + ) + except ToolError as error: + if "not found" in str(error).lower(): + raise NoteNotFoundError( + f"No note {identifier!r}; search_notes can find the identifier" + ) from error + raise + entity_id = resolved.json()["external_id"] + response = await call_get( + client, f"/v2/projects/{target.external_id}/resource/{entity_id}" + ) + except (ValueError, RuntimeError) as error: + # Routing failed before any read happened (a constrained or unresolvable + # route, or the cloud workspace index consulted without credentials). + raise ResourceError(str(error)) from error + except ToolError as error: + # Routing and content-read failures (a stale project route, auth, server, + # transport) keep their actionable cause; the confirmed note miss is + # mapped where the entity resolver answers, above. + raise ResourceError(str(error)) from error + + content_type = response.headers.get("content-type", "") + # Only text comes back byte-exact; steer binaries to the tool built for them. + if not (content_type.startswith("text/") or content_type == "application/json"): + raise ResourceError( + f"{identifier!r} is {content_type or 'binary'}; use the read_content tool " + "for non-text files" + ) + return response.text + + +@mcp.resource( + uri=NOTE_TEMPLATE, + name="note", + description=( + "A note's raw markdown, addressed by its memory:// URL — " + "memory:///, e.g. memory://research/specs/search-design. " + "The identifier may be a permalink, a title, or a file path in the project." + ), + mime_type="text/markdown", +) +async def note_resource(project: str, path: str, context: Context | None = None) -> str: + """Return the raw markdown of one note.""" + # `man` is the manual's namespace; its template registers first and wins the + # tie, and manual_page itself falls back to a note in a project really named + # man — delegating keeps both templates' answers identical either way. + if project == "man": + return await manual_page(path, context) + + # The {workspace}/{project}/info shape belongs to the project_info resource, + # which itself falls back to a note named .../info — delegating keeps both + # handlers' answers identical whichever template wins the tie. + head, _, tail = path.rpartition("/") + if tail == "info" and head and "/" not in head: + return await project_info(workspace=project, project=head, context=context) + + return await read_note_markdown(f"{project}/{path}", context) diff --git a/src/basic_memory/mcp/resources/project_info.py b/src/basic_memory/mcp/resources/project_info.py index 2a281f2a4..f2a3671da 100644 --- a/src/basic_memory/mcp/resources/project_info.py +++ b/src/basic_memory/mcp/resources/project_info.py @@ -1,6 +1,7 @@ """Project info resource for Basic Memory MCP server.""" from fastmcp import Context +from fastmcp.exceptions import ResourceError, ToolError from loguru import logger from basic_memory.config import ConfigManager, ProjectMode @@ -19,7 +20,7 @@ async def project_info( workspace: str, project: str, context: Context | None = None, -) -> ProjectInfoResponse: +) -> str: """Get comprehensive information about a workspace-qualified Basic Memory project. This resource provides detailed statistics and status information about a @@ -38,7 +39,8 @@ async def project_info( context: Optional FastMCP context for performance caching. Returns: - Detailed project information and statistics. + Detailed project information and statistics as a JSON document — + resources carry text, so the validated response is serialized here. """ logger.info("Getting project info") @@ -57,6 +59,37 @@ async def project_info( ): project_route = configured_project - async with get_project_client(project_route, context) as (client, active_project): - response = await call_get(client, f"/v2/projects/{active_project.external_id}/info") - return ProjectInfoResponse.model_validate(response.json()) + try: + async with get_project_client(project_route, context) as (client, active_project): + response = await call_get(client, f"/v2/projects/{active_project.external_id}/info") + try: + info = ProjectInfoResponse.model_validate(response.json()) + except ValueError as payload_error: + # A reachable route answered with an incompatible payload — a backend + # fault to surface, never a cue for the outer handler to serve a note. + raise ResourceError( + f"Project info for '{project_route}' returned an invalid payload: " + f"{payload_error}" + ) from payload_error + return info.model_dump_json(indent=2) + except (ValueError, RuntimeError, ToolError) as error: + # Trigger: forced-local transports surface an unknown compound route as a + # ToolError rather than ValueError/RuntimeError. + # Why: only a missing project route may fall back to a note; auth, server, + # and transport failures on a real route must keep their cause. + # Outcome: route misses continue into the fallback; other ToolErrors raise. + if isinstance(error, ToolError) and "not found" not in str(error).lower(): + raise + # This template also wins ties for {project}/{directory}/info note URIs + # (precedence between overlapping template matches is undefined), so a + # failed workspace/project route may really be a note whose canonical + # permalink ends in /info. Deferred import: notes.py imports this module. + from basic_memory.mcp.resources.notes import NoteNotFoundError, read_note_markdown + + try: + return await read_note_markdown(f"{workspace}/{project}/info", context) + except NoteNotFoundError: + # Neither a project route nor a note — the route error is the cause. + # An operational note failure (auth, server, transport) propagates + # with its own cause instead. + raise ResourceError(str(error)) from error diff --git a/src/basic_memory/mcp/server.py b/src/basic_memory/mcp/server.py index 62780d6db..1c19aff2e 100644 --- a/src/basic_memory/mcp/server.py +++ b/src/basic_memory/mcp/server.py @@ -224,7 +224,9 @@ async def lifespan(app: FastMCP): "For a fuller guide, read the `memory://ai_assistant_guide` resource. The manual has a " "page for nearly every tool, with verified examples and gotchas: `memory://man` lists " "them, and `memory://man/(3)` (for example `memory://man/search-notes(3)`) is one " - "page — read it before using a tool for the first time. If you have a web or fetch tool " + "page — read it before using a tool for the first time. Any note is readable the same " + "way: its memory:/// URL is a resource returning the raw markdown. If " + "you have a web or fetch tool " "and need current " "documentation, fetch `https://docs.basicmemory.com/llms.txt` first, then fetch only the " "relevant linked `/raw/...md` page." diff --git a/test-int/mcp/test_resources_integration.py b/test-int/mcp/test_resources_integration.py new file mode 100644 index 000000000..6550a509e --- /dev/null +++ b/test-int/mcp/test_resources_integration.py @@ -0,0 +1,86 @@ +"""Integration tests for MCP resources: the manual and notes over resources/read. + +Full flow, no mocks: MCP Client → MCP Server → FastAPI (ASGI) → database. This is +what an actual MCP client does with the `memory://` URIs Basic Memory hands out. +""" + +from typing import Any + +import pytest +from fastmcp import Client + +# The mcp_server fixture registers tools, resources, and prompts. + + +async def read_text(client: Client[Any], uri: str) -> str: + contents = await client.read_resource(uri) + text = getattr(contents[0], "text", None) + assert isinstance(text, str) + return text + + +@pytest.mark.asyncio +async def test_manual_resources_are_listed_and_readable(mcp_server, app): + """The manual index and pages answer resources/list and resources/read.""" + async with Client(mcp_server) as client: + listed = {str(resource.uri) for resource in await client.list_resources()} + assert "memory://man" in listed + assert "memory://man/search-notes(3)" in listed + + index = await read_text(client, "memory://man") + assert index.startswith("# Basic Memory manual") + + # Any common spelling of a page resolves through the template. + page = await read_text(client, "memory://man/search-notes(3)") + by_tool_name = await read_text(client, "memory://man/search_notes") + assert page.startswith("---\ntitle: search-notes(3)\n") + assert by_tool_name == page + + +@pytest.mark.asyncio +async def test_note_is_readable_at_its_memory_uri(mcp_server, app, test_project): + """A note written through the tools reads back as raw markdown via its URI.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Search Design", + "directory": "specs", + "content": ( + "# Search Design\n\n" + "- [decision] notes answer resources/read #mcp\n" + "- relates_to [[Indexing]]\n" + ), + }, + ) + + # Project-prefixed canonical URI. + text = await read_text(client, f"memory://{test_project.name}/specs/search-design") + assert text.startswith("---\n") # raw file: frontmatter included + assert "- [decision] notes answer resources/read #mcp" in text + + # Unprefixed spelling: the first segment is a directory, not a project, + # so routing falls back to the active/default project. + unprefixed = await read_text(client, "memory://specs/search-design") + assert unprefixed == text + + # File-path spelling. + by_path = await read_text(client, f"memory://{test_project.name}/specs/search-design.md") + assert by_path == text + + +@pytest.mark.asyncio +async def test_project_info_uri_reads_over_the_wire(mcp_server, app, test_project): + """The workspace/project/info template serves JSON stats through a real session.""" + async with Client(mcp_server) as client: + info = await read_text(client, f"memory://local/{test_project.permalink}/info") + assert test_project.name in info + + +@pytest.mark.asyncio +async def test_unknown_note_reports_a_missing_note(mcp_server, app, test_project): + """A miss surfaces as an error naming the note, not a fuzzy match or silence.""" + async with Client(mcp_server) as client: + with pytest.raises(Exception, match="No note"): + await client.read_resource(f"memory://{test_project.name}/nope/does-not-exist") diff --git a/tests/mcp/test_man_resources.py b/tests/mcp/test_man_resources.py index a331feadb..c7c32573d 100644 --- a/tests/mcp/test_man_resources.py +++ b/tests/mcp/test_man_resources.py @@ -12,6 +12,7 @@ manual_index, manual_page, ) +import basic_memory.mcp.resources.notes as notes_module from basic_memory.mcp.server import mcp @@ -73,8 +74,26 @@ async def test_tool_name_reaches_the_page_that_documents_it() -> None: assert page.startswith("---\ntitle: chatgpt-fetch(3)\n") -def test_unknown_pages_point_at_the_index() -> None: +@pytest.mark.asyncio +async def test_unknown_pages_point_at_the_index(app, test_project) -> None: + # The miss falls through to a note lookup in a project named man; when that + # is a confirmed miss too, the manual's index hint is the error. with pytest.raises(ResourceError, match="No manual entry for nope; read memory://man"): - manual_page("nope") + await manual_page("nope") with pytest.raises(ResourceError, match="not a manual page reference; read memory://man"): - manual_page("docs/nope") + await manual_page("docs/nope") + + +@pytest.mark.asyncio +async def test_man_template_falls_back_to_a_project_named_man( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The man template registers first and wins ties over the notes template, so + # the note fallback must live here for the served path to reach it. + async def note_read(identifier, context): + assert identifier == "man/guides/setup" + return "note from the man project" + + monkeypatch.setattr(notes_module, "read_note_markdown", note_read) + + assert await _read("memory://man/guides/setup") == "note from the man project" diff --git a/tests/mcp/test_note_resources.py b/tests/mcp/test_note_resources.py new file mode 100644 index 000000000..b5f775d68 --- /dev/null +++ b/tests/mcp/test_note_resources.py @@ -0,0 +1,399 @@ +"""Tests for notes as MCP resources (memory://{project}/{path*}).""" + +from __future__ import annotations + +from importlib import import_module +from types import SimpleNamespace + +import pytest +from fastmcp import Client +from fastmcp.exceptions import ResourceError, ToolError + +import basic_memory.mcp.resources.notes as notes_module +from basic_memory.mcp.resources.notes import NOTE_TEMPLATE, note_resource +from basic_memory.mcp.server import mcp +from basic_memory.mcp.tools import write_note + + +async def _read(uri: str) -> str: + # A real client session: resources/read through the server injects a live + # Context, exactly as production does (mcp.read_resource alone would not). + async with Client(mcp) as session: + contents = await session.read_resource(uri) + text = getattr(contents[0], "text", None) + assert isinstance(text, str) + return text + + +@pytest.mark.asyncio +async def test_note_template_is_registered() -> None: + templates = {str(template.uri_template) for template in await mcp.list_resource_templates()} + + assert NOTE_TEMPLATE in templates + + +@pytest.mark.asyncio +async def test_note_reads_as_raw_markdown(app, test_project) -> None: + await write_note( + title="Resource Read Test", + directory="specs", + content="# Resource Read Test\n\n- [design] notes are resources #mcp\n", + project=test_project.name, + ) + + text = await _read(f"memory://{test_project.permalink}/specs/resource-read-test") + + assert text.startswith("---\n") # the raw file, frontmatter included + assert "- [design] notes are resources #mcp" in text + + +@pytest.mark.asyncio +async def test_unknown_note_and_unknown_project_raise_resource_errors(app, test_project) -> None: + with pytest.raises(ResourceError, match="No note 'test-project/nope/missing'"): + await note_resource(project=test_project.name, path="nope/missing") + # An unknown first segment falls back to the default project (unprefixed + # permalinks) and reports the full identifier as missing there. + with pytest.raises(ResourceError, match="No note"): + await note_resource(project="no-such-project-anywhere", path="anything") + + +@pytest.mark.asyncio +async def test_unprefixed_permalink_reads_in_default_project(app, test_project) -> None: + # With permalinks_include_project=False (or legacy notes) the URI's first + # segment is a directory, not a project; routing must fall back to the + # active/default project with the whole path as the identifier. + await write_note( + title="Roadmap", + directory="docs", + content="# Roadmap\n\nUnprefixed permalink read.\n", + project=test_project.name, + ) + + text = await _read("memory://docs/roadmap") + + assert "Unprefixed permalink read." in text + + +@pytest.mark.asyncio +async def test_project_route_not_found_is_not_a_note_miss( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # A stale configured project (backend answers 'Project not found') must + # surface the route failure — never claim the note itself is missing. + def broken_route(project, context=None, project_id=None): + raise ToolError("Project not found: docs") + + monkeypatch.setattr(notes_module, "get_project_client", broken_route) + with pytest.raises(ResourceError, match="Project not found") as excinfo: + await note_resource(project=test_project.name, path="anything") + assert not isinstance(excinfo.value, notes_module.NoteNotFoundError) + + +@pytest.mark.asyncio +async def test_non_404_failures_keep_their_cause( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + async def failing_resolve(client, url, json=None): + raise ToolError("Authentication required: You need to authenticate to access 'x'") + + monkeypatch.setattr(notes_module, "call_post", failing_resolve) + with pytest.raises(ResourceError, match="Authentication required"): + await note_resource(project=test_project.name, path="anything") + + +@pytest.mark.asyncio +async def test_routing_errors_surface_their_cause( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + async def constrained(client, identifier, project, context): + raise ValueError("Project is constrained to 'other'") + + monkeypatch.setattr(notes_module, "resolve_project_and_path", constrained) + with pytest.raises(ResourceError, match="constrained"): + await note_resource(project=test_project.name, path="anything") + + +@pytest.mark.asyncio +async def test_man_namespace_stays_the_manual(app) -> None: + # Which template a server matches first is not guaranteed, so the notes + # handler must answer memory://man/... exactly as the manual would. + direct = await note_resource(project="man", path="search-notes(3)") + served = await _read("memory://man/search-notes(3)") + + assert direct.startswith("---\ntitle: search-notes(3)\n") + assert served == direct + + +@pytest.mark.asyncio +async def test_client_is_opened_for_the_uris_own_project( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # A cloud-mode project needs its own transport; the client must be routed for + # the URI's project when it is configured, and for the default when it is not. + routes: list[str | None] = [] + real_get_project_client = notes_module.get_project_client + + def recording_get_project_client(project, context=None, project_id=None): + routes.append(project) + return real_get_project_client(project, context, project_id=project_id) + + monkeypatch.setattr(notes_module, "get_project_client", recording_get_project_client) + + await write_note( + title="Routed", + directory="specs", + content="# Routed\n", + project=test_project.name, + ) + await _read(f"memory://{test_project.permalink}/specs/routed") + # A note exists, so this also proves strict resolution: the miss stays a + # miss instead of fuzzy-matching the existing note the way tools would. + with pytest.raises(ResourceError, match="No note"): + await note_resource(project="docs", path="missing-note") + + assert routes[0] == test_project.name # configured segment → its own client + assert routes[1] is None # unconfigured segment → default client, path fallback + + +@pytest.mark.asyncio +async def test_a_project_named_man_is_reachable_behind_the_manual( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + # The manual answers first, but nothing reserves the name: when no page + # matches, the URI falls through to a note in a project really named man. + async def note_read(identifier, context): + assert identifier == "man/guides/setup" + return "note content from the man project" + + monkeypatch.setattr(notes_module, "read_note_markdown", note_read) + assert await note_resource(project="man", path="guides/setup") == ( + "note content from the man project" + ) + + async def note_miss(identifier, context): + raise notes_module.NoteNotFoundError("No note") + + monkeypatch.setattr(notes_module, "read_note_markdown", note_miss) + with pytest.raises(ResourceError, match="read memory://man for the index"): + await note_resource(project="man", path="guides/setup") + + async def note_error(identifier, context): + raise ResourceError("Authentication required: x") + + # An operational note failure keeps its cause instead of the manual's hint. + monkeypatch.setattr(notes_module, "read_note_markdown", note_error) + with pytest.raises(ResourceError, match="Authentication required"): + await note_resource(project="man", path="guides/setup") + + +@pytest.mark.asyncio +async def test_project_info_template_still_answers_info_uris(app, test_project) -> None: + # The three-segment info URI overlaps the notes template; pin that reading it + # through a real session yields project info rather than a missing-note error. + content = await _read(f"memory://local/{test_project.permalink}/info") + + assert test_project.name in content + + +@pytest.mark.asyncio +async def test_info_shaped_uris_delegate_to_project_info(app, test_project) -> None: + # Insurance for the other tie outcome: if this template ever wins the + # {ws}/{proj}/info shape, the reader still gets project info. + direct = await note_resource(project="local", path=f"{test_project.permalink}/info") + + assert test_project.name in direct + + +@pytest.mark.asyncio +async def test_note_actually_named_info_still_reads(app, test_project) -> None: + await write_note( + title="Info", + directory="sub", + content="# Info\n\nA note that happens to be called info.\n", + project=test_project.name, + ) + + # Direct: the delegation routes through project_info, which falls back to + # the note when no such workspace/project pair exists. + direct = await note_resource(project=test_project.name, path="sub/info") + # Served: whichever template wins the 3-segment /info shape, the canonical + # extensionless permalink reads — and so does the file path. + served = await _read(f"memory://{test_project.permalink}/sub/info") + served_md = await _read(f"memory://{test_project.permalink}/sub/info.md") + + assert "A note that happens to be called info." in direct + assert "A note that happens to be called info." in served + assert "A note that happens to be called info." in served_md + + +@pytest.mark.asyncio +async def test_workspace_qualified_uris_route_through_their_project( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # memory://personal/main/docs/report: the canonical prefix detection names + # the workspace-qualified route, and the client must be opened for it — + # with its failures surfacing, not falling back to the default project. + async def detected(identifier, config, context=None): + assert identifier == "memory://personal/main/docs/report" + return "personal/main" + + monkeypatch.setattr(notes_module, "detect_project_from_memory_url_prefix", detected) + routes: list[str | None] = [] + real_get_project_client = notes_module.get_project_client + + def recording(project, context=None, project_id=None): + routes.append(project) + return real_get_project_client(project, context, project_id=project_id) + + monkeypatch.setattr(notes_module, "get_project_client", recording) + with pytest.raises(ResourceError): + await note_resource(project="personal", path="main/docs/report") + + assert routes == ["personal/main"] + + +@pytest.mark.asyncio +async def test_workspace_routes_survive_disabled_project_prefixes( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # permalinks_include_project=False drops only local project-name collisions; + # cloud permalinks stay workspace-qualified regardless of the flag, so a + # detected workspace route must still open that route's client. + class StubConfig: + permalinks_include_project = False + projects = {test_project.name: test_project.path} + + class StubConfigManager: + config = StubConfig() + + monkeypatch.setattr(notes_module, "ConfigManager", StubConfigManager) + + async def detected(identifier, config, context=None): + return "team-paul/main" + + monkeypatch.setattr(notes_module, "detect_project_from_memory_url_prefix", detected) + routes: list[str | None] = [] + real_get_project_client = notes_module.get_project_client + + def recording(project, context=None, project_id=None): + routes.append(project) + return real_get_project_client(project, context, project_id=project_id) + + monkeypatch.setattr(notes_module, "get_project_client", recording) + with pytest.raises(ResourceError): + await note_resource(project="team-paul", path="main/team/note") + + assert routes == ["team-paul/main"] + + +@pytest.mark.asyncio +async def test_unprefixed_permalinks_ignore_project_name_collisions( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # With permalinks_include_project=False, memory://docs/roadmap is the note + # docs/roadmap in the active project even when a project named docs exists. + class StubConfig: + permalinks_include_project = False + projects = {"docs": "/nowhere", test_project.name: test_project.path} + + class StubConfigManager: + config = StubConfig() + + monkeypatch.setattr(notes_module, "ConfigManager", StubConfigManager) + routes: list[str | None] = [] + real_get_project_client = notes_module.get_project_client + + def recording(project, context=None, project_id=None): + routes.append(project) + return real_get_project_client(project, context, project_id=project_id) + + monkeypatch.setattr(notes_module, "get_project_client", recording) + await write_note( + title="Roadmap", + directory="docs", + content="# Roadmap\n\nActive project wins.\n", + project=test_project.name, + ) + + text = await note_resource(project="docs", path="roadmap") + + assert "Active project wins." in text + assert routes == [None] # no pre-routing to the colliding project name + + +@pytest.mark.asyncio +async def test_info_fallback_keeps_operational_note_failures( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + async def note_error(identifier, context): + raise ResourceError("Authentication required: x") + + monkeypatch.setattr(notes_module, "read_note_markdown", note_error) + with pytest.raises(ResourceError, match="Authentication required"): + await notes_module.project_info(workspace="nowhere", project="also-nowhere") + + +@pytest.mark.asyncio +async def test_info_uri_that_is_neither_project_nor_note_reports_the_route( + app, test_project +) -> None: + with pytest.raises(ResourceError): + await notes_module.project_info(workspace="nowhere", project="also-nowhere") + + +@pytest.mark.asyncio +async def test_binary_content_is_steered_to_read_content( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + await write_note( + title="Binary Decoy", + directory="specs", + content="# Binary Decoy\n", + project=test_project.name, + ) + + async def fake_call_get(client, url): + return SimpleNamespace(headers={"content-type": "image/png"}, text="") + + monkeypatch.setattr(notes_module, "call_get", fake_call_get) + + with pytest.raises(ResourceError, match="use the read_content tool"): + await note_resource(project=test_project.name, path="specs/binary-decoy") + + +@pytest.mark.asyncio +async def test_info_fallback_runs_when_forced_local_reports_project_not_found( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + # Forced-local transports surface an unknown compound route as ToolError + # ("Project not found"), not ValueError — the note fallback must still run. + await write_note( + title="Info", + directory="sub", + content="# Info\n\nStill readable under forced-local routing.\n", + project=test_project.name, + ) + project_info_module = import_module("basic_memory.mcp.resources.project_info") + + def missing_route(project, context=None, project_id=None): + raise ToolError(f"Project not found: {project}") + + monkeypatch.setattr(project_info_module, "get_project_client", missing_route) + + text = await notes_module.project_info(workspace=test_project.name, project="sub") + + assert "Still readable under forced-local routing." in text + + +@pytest.mark.asyncio +async def test_info_route_tool_errors_that_are_not_misses_propagate( + app, test_project, monkeypatch: pytest.MonkeyPatch +) -> None: + project_info_module = import_module("basic_memory.mcp.resources.project_info") + + def broken_route(project, context=None, project_id=None): + raise ToolError("Authentication required: x") + + monkeypatch.setattr(project_info_module, "get_project_client", broken_route) + with pytest.raises(ToolError, match="Authentication required"): + await notes_module.project_info(workspace=test_project.name, project="sub") diff --git a/tests/mcp/test_resources.py b/tests/mcp/test_resources.py index 1ca5bd8c3..a767b8ed1 100644 --- a/tests/mcp/test_resources.py +++ b/tests/mcp/test_resources.py @@ -4,11 +4,13 @@ import pytest from fastmcp import Context +from fastmcp.exceptions import ResourceError from httpx import AsyncClient from basic_memory.mcp.prompts.ai_assistant_guide import ai_assistant_guide from basic_memory.mcp.resources.project_info import project_info from basic_memory.mcp.server import mcp +from basic_memory.schemas import ProjectInfoResponse from basic_memory.schemas.project_info import ProjectItem @@ -65,7 +67,9 @@ async def project_client( project_info_module = import_module("basic_memory.mcp.resources.project_info") monkeypatch.setattr(project_info_module, "get_project_client", project_client) - info = await project_info(workspace="personal", project="test-project") + info = ProjectInfoResponse.model_validate_json( + await project_info(workspace="personal", project="test-project") + ) assert selected_route == "personal/test-project" assert info.project_name == test_project.name @@ -74,6 +78,28 @@ async def project_client( @pytest.mark.asyncio async def test_project_info_resource_routes_local_workspace(client, test_project): """The canonical local URI strips its workspace sentinel before local routing.""" - info = await project_info(workspace="local", project=test_project.permalink) + info = ProjectInfoResponse.model_validate_json( + await project_info(workspace="local", project=test_project.permalink) + ) assert info.project_name == test_project.name + + +@pytest.mark.asyncio +async def test_project_info_invalid_payload_surfaces_instead_of_note_fallback( + client, test_project, monkeypatch: pytest.MonkeyPatch +): + """A reachable route with a broken payload is a backend fault, not a note miss.""" + + class FakeResponse: + def json(self): + return {"bogus": True} + + async def fake_call_get(client_, url): + return FakeResponse() + + project_info_module = import_module("basic_memory.mcp.resources.project_info") + monkeypatch.setattr(project_info_module, "call_get", fake_call_get) + + with pytest.raises(ResourceError, match="invalid payload"): + await project_info(workspace="local", project=test_project.permalink)