From 28ee7aade3afaf47efb19f14b962ccdc49d5518a Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 17:32:06 -0500 Subject: [PATCH 1/8] fix(mcp): preserve scoped memory URL paths Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 8 +- tests/mcp/test_project_context_scoped_key.py | 93 ++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 tests/mcp/test_project_context_scoped_key.py diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index b8a23bdc7..95a20afba 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1407,7 +1407,13 @@ async def resolve_project_and_path( ) resolved = ProjectResolveResponse.model_validate(response.json()) except ToolError as exc: - if "project not found" not in str(exc).lower(): + error_message = str(exc).lower() + project_route_missing = "project not found" in error_message + project_route_hidden_by_scope = ( + "does not have access to this project" in error_message + and not strict_project_routing + ) + if not project_route_missing and not project_route_hidden_by_scope: raise if strict_project_routing: # Trigger: a mutating tool supplied a memory URL whose leading diff --git a/tests/mcp/test_project_context_scoped_key.py b/tests/mcp/test_project_context_scoped_key.py new file mode 100644 index 000000000..5acc533c2 --- /dev/null +++ b/tests/mcp/test_project_context_scoped_key.py @@ -0,0 +1,93 @@ +"""Project-context regressions for project-scoped cloud credentials.""" + +from typing import Any, cast + +import pytest +from mcp.server.fastmcp.exceptions import ToolError + +from basic_memory.mcp.project_context import resolve_project_and_path +from basic_memory.schemas.project_info import ProjectItem + + +class ContextState: + """Minimal FastMCP state surface used by project routing.""" + + def __init__(self) -> None: + self.state: dict[str, object] = {} + + async def get_state(self, key: str) -> object | None: + return self.state.get(key) + + async def set_state(self, key: str, value: object) -> None: + self.state[key] = value + + +@pytest.mark.asyncio +async def test_read_route_falls_back_when_project_prefix_is_hidden_by_scope( + config_manager, + monkeypatch, +) -> None: + """A note directory must not become a forbidden cross-project lookup.""" + config = config_manager.load_config() + config.permalinks_include_project = True + config_manager.save_config(config) + + context = ContextState() + active_project = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="ci-acceptance", + path="/tmp/ci-acceptance", + is_default=False, + ) + await context.set_state("active_project", active_project.model_dump()) + + async def reject_directory_as_project(*args: object, **kwargs: object) -> None: + raise ToolError("This API key does not have access to this project") + + monkeypatch.setattr( + "basic_memory.mcp.tools.utils.call_post", + reject_directory_as_project, + ) + + resolved_project, resolved_path, is_memory_url = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://acceptance/mcp/cloud-smoke", + project="ci-acceptance", + context=cast(Any, context), + ) + + assert resolved_project == active_project + assert resolved_path == "ci-acceptance/acceptance/mcp/cloud-smoke" + assert is_memory_url is True + + +@pytest.mark.asyncio +async def test_mutating_route_keeps_scope_rejection_fail_closed( + config_manager, + monkeypatch, +) -> None: + """Strict mutation routing must not reinterpret a forbidden route as a path.""" + context = ContextState() + active_project = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="ci-acceptance", + path="/tmp/ci-acceptance", + is_default=False, + ) + await context.set_state("active_project", active_project.model_dump()) + + async def reject_project_route(*args: object, **kwargs: object) -> None: + raise ToolError("This API key does not have access to this project") + + monkeypatch.setattr("basic_memory.mcp.tools.utils.call_post", reject_project_route) + + with pytest.raises(ToolError, match="does not have access"): + await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://other-project/cloud-smoke", + project="ci-acceptance", + context=cast(Any, context), + strict_project_routing=True, + ) From ec8430932f4ddc7a00c2cdce023bc220f8a0413c Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 17:36:27 -0500 Subject: [PATCH 2/8] test(mcp): require cached scoped project Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 2 ++ tests/mcp/test_project_context_scoped_key.py | 22 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 95a20afba..5a89343ee 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1411,6 +1411,8 @@ async def resolve_project_and_path( project_route_missing = "project not found" in error_message project_route_hidden_by_scope = ( "does not have access to this project" in error_message + and cached_project is not None + and _project_matches_identifier(cached_project, project) and not strict_project_routing ) if not project_route_missing and not project_route_hidden_by_scope: diff --git a/tests/mcp/test_project_context_scoped_key.py b/tests/mcp/test_project_context_scoped_key.py index 5acc533c2..7e990f109 100644 --- a/tests/mcp/test_project_context_scoped_key.py +++ b/tests/mcp/test_project_context_scoped_key.py @@ -91,3 +91,25 @@ async def reject_project_route(*args: object, **kwargs: object) -> None: context=cast(Any, context), strict_project_routing=True, ) + + +@pytest.mark.asyncio +async def test_read_route_requires_an_authorized_cached_project( + config_manager, + monkeypatch, +) -> None: + """An access denial without an established project must still propagate.""" + context = ContextState() + + async def reject_project_route(*args: object, **kwargs: object) -> None: + raise ToolError("This API key does not have access to this project") + + monkeypatch.setattr("basic_memory.mcp.tools.utils.call_post", reject_project_route) + + with pytest.raises(ToolError, match="does not have access"): + await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://other-project/cloud-smoke", + project="ci-acceptance", + context=cast(Any, context), + ) From 7154e8b152a8ddca7d1d257bae58cc76921b543b Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 17:41:28 -0500 Subject: [PATCH 3/8] fix(mcp): keep note mutations strict Signed-off-by: phernandez --- src/basic_memory/mcp/tools/delete_note.py | 1 + src/basic_memory/mcp/tools/move_note.py | 6 ++- .../tools/test_mutating_memory_url_routing.py | 51 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/mcp/tools/test_mutating_memory_url_routing.py diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 1887e0d93..2cce57ca2 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -302,6 +302,7 @@ async def delete_note( identifier, active_project.name, context, + strict_project_routing=True, ) # Handle directory deletes diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index e778c1083..b0961f80a 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -668,7 +668,11 @@ async def move_note( # same way the sibling tools do. # Outcome: move_note resolves memory:// URLs identically to read/edit/delete. source_project, resolved_identifier, _ = await resolve_project_and_path( - client, identifier, active_project.name, context + client, + identifier, + active_project.name, + context, + strict_project_routing=True, ) # Trigger: a memory:// identifier whose project prefix resolves to a DIFFERENT project diff --git a/tests/mcp/tools/test_mutating_memory_url_routing.py b/tests/mcp/tools/test_mutating_memory_url_routing.py new file mode 100644 index 000000000..937f52def --- /dev/null +++ b/tests/mcp/tools/test_mutating_memory_url_routing.py @@ -0,0 +1,51 @@ +"""Fail-closed memory URL routing for mutating MCP tools.""" + +import importlib + +import pytest +from mcp.server.fastmcp.exceptions import ToolError + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("module_name", "tool_name", "tool_args"), + [ + ( + "basic_memory.mcp.tools.delete_note", + "delete_note", + {"identifier": "memory://other-project/note"}, + ), + ( + "basic_memory.mcp.tools.move_note", + "move_note", + { + "identifier": "memory://other-project/note", + "destination_path": "archive/note.md", + }, + ), + ], +) +async def test_mutating_tools_require_strict_project_routing( + client, + test_project, + monkeypatch, + module_name: str, + tool_name: str, + tool_args: dict[str, str], +) -> None: + """Scope-hidden project prefixes must stop before any mutation client call.""" + tool_module = importlib.import_module(module_name) + + async def reject_scope_hidden_route(*args: object, **kwargs: object) -> None: + assert kwargs["strict_project_routing"] is True + raise ToolError("This API key does not have access to this project") + + monkeypatch.setattr( + tool_module, + "resolve_project_and_path", + reject_scope_hidden_route, + ) + + tool = getattr(tool_module, tool_name) + with pytest.raises(ToolError, match="does not have access"): + await tool(project=test_project.name, **tool_args) From 5047b65aaba9a41ee1dac0d331e2c680bdec0b35 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 17:49:57 -0500 Subject: [PATCH 4/8] fix(mcp): route directory moves strictly Signed-off-by: phernandez --- src/basic_memory/mcp/tools/move_note.py | 133 ++++++++++-------- .../tools/test_mutating_memory_url_routing.py | 46 +++++- 2 files changed, 122 insertions(+), 57 deletions(-) diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index b0961f80a..5f07962d9 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -9,9 +9,42 @@ from mcp.server.fastmcp.exceptions import ToolError from pydantic import AliasChoices, Field +from basic_memory.config import ConfigManager from basic_memory.mcp.server import mcp from basic_memory.mcp.project_context import get_project_client, resolve_project_and_path -from basic_memory.utils import validate_project_path +from basic_memory.schemas.project_info import ProjectItem +from basic_memory.utils import ( + generate_permalink, + normalize_project_reference, + validate_project_path, +) +from basic_memory.workspace_context import current_workspace_permalink_context + + +def _directory_path_for_move( + resolved_identifier: str, + active_project: ProjectItem, + *, + include_project_prefix: bool, +) -> str: + """Return the project-relative source directory expected by the move API.""" + directory = normalize_project_reference(resolved_identifier).strip("/") + project_permalink = active_project.permalink + + route_prefixes: list[str] = [] + workspace_context = current_workspace_permalink_context() + if workspace_context and workspace_context.should_prefix_permalinks: + route_prefixes.append( + f"{generate_permalink(workspace_context.workspace_slug)}/{project_permalink}" + ) + if include_project_prefix: + route_prefixes.append(project_permalink) + + for route_prefix in route_prefixes: + if directory.startswith(f"{route_prefix}/"): + return directory.removeprefix(f"{route_prefix}/") + + return directory async def _detect_cross_project_move_attempt( @@ -561,15 +594,52 @@ async def move_note( move_note("{identifier}", "notes/{destination_path.split("/")[-1] if "/" in destination_path else destination_path}") ```""" - # Handle directory moves - if is_directory: - # Import here to avoid circular import - from basic_memory.mcp.clients import KnowledgeClient + # Resolve every source before branching so file and directory mutations share + # the same fail-closed project boundary. + from basic_memory.mcp.clients import KnowledgeClient + + knowledge_client = KnowledgeClient(client, active_project.external_id) + source_project, resolved_identifier, is_memory_url = await resolve_project_and_path( + client, + identifier, + active_project.name, + context, + strict_project_routing=True, + ) - knowledge_client = KnowledgeClient(client, active_project.external_id) + # move_note only supports moves inside the active project. + if source_project.external_id != active_project.external_id: + logger.info( + f"Move rejected: source '{identifier}' resolves to project " + f"'{source_project.name}', not the active project '{active_project.name}'" + ) + if output_format == "json": + return { + "moved": False, + "title": None, + "permalink": None, + "file_path": None, + "source": identifier, + "destination": destination_path, + "error": "CROSS_PROJECT_MOVE_NOT_SUPPORTED", + } + return _format_cross_project_error_response( + identifier, destination_path, active_project.name, source_project.name + ) + # Handle directory moves + if is_directory: try: - result = await knowledge_client.move_directory(identifier, destination_path) + source_directory = ( + _directory_path_for_move( + resolved_identifier, + active_project, + include_project_prefix=ConfigManager().config.permalinks_include_project, + ) + if is_memory_url + else resolved_identifier + ) + result = await knowledge_client.move_directory(source_directory, destination_path) if output_format == "json": return { "moved": result.failed_moves == 0, @@ -653,55 +723,6 @@ async def move_note( move_note("path/to/file.md", "{destination_path}/file.md") ```""" - # Import here to avoid circular import - from basic_memory.mcp.clients import KnowledgeClient - - # Use typed KnowledgeClient for API calls - knowledge_client = KnowledgeClient(client, active_project.external_id) - - # --- Normalize the identifier for memory:// URLs --- - # Trigger: caller passed a "memory://..." URL (the docstring advertises this, and - # read_note/edit_note/delete_note all accept it). - # Why: resolve_entity / the /resolve endpoint expect a bare permalink or title; - # the literal "memory://" scheme prefix is not stripped by link resolution and - # 404s. resolve_project_and_path strips the prefix and normalizes the path the - # same way the sibling tools do. - # Outcome: move_note resolves memory:// URLs identically to read/edit/delete. - source_project, resolved_identifier, _ = await resolve_project_and_path( - client, - identifier, - active_project.name, - context, - strict_project_routing=True, - ) - - # Trigger: a memory:// identifier whose project prefix resolves to a DIFFERENT project - # than the one move_note is operating on (e.g. "memory://other-project/..."). - # Why: get_project_client already bound knowledge_client to the active project, so the - # resolve_entity below runs against the active project regardless of the URL's - # project. Honoring the cross-project prefix would misroute (path normalized for - # the other project, looked up in the active one); move_note cannot move across - # projects. - # Outcome: reject up front with the cross-project guidance instead of misrouting. - if source_project.external_id != active_project.external_id: - logger.info( - f"Move rejected: source '{identifier}' resolves to project " - f"'{source_project.name}', not the active project '{active_project.name}'" - ) - if output_format == "json": - return { - "moved": False, - "title": None, - "permalink": None, - "file_path": None, - "source": identifier, - "destination": destination_path, - "error": "CROSS_PROJECT_MOVE_NOT_SUPPORTED", - } - return _format_cross_project_error_response( - identifier, destination_path, active_project.name, source_project.name - ) - # Resolve once and reuse the entity ID across extension validation and move. source_ext = "md" # Default to .md if we can't determine source extension resolved_entity_id: str | None = None diff --git a/tests/mcp/tools/test_mutating_memory_url_routing.py b/tests/mcp/tools/test_mutating_memory_url_routing.py index 937f52def..0090f23c9 100644 --- a/tests/mcp/tools/test_mutating_memory_url_routing.py +++ b/tests/mcp/tools/test_mutating_memory_url_routing.py @@ -5,6 +5,10 @@ import pytest from mcp.server.fastmcp.exceptions import ToolError +from basic_memory.mcp.tools.move_note import move_note +from basic_memory.mcp.tools.read_note import read_note +from basic_memory.mcp.tools.write_note import write_note + @pytest.mark.asyncio @pytest.mark.parametrize( @@ -23,6 +27,15 @@ "destination_path": "archive/note.md", }, ), + ( + "basic_memory.mcp.tools.move_note", + "move_note", + { + "identifier": "memory://other-project/directory", + "destination_path": "archive/directory", + "is_directory": True, + }, + ), ], ) async def test_mutating_tools_require_strict_project_routing( @@ -31,7 +44,7 @@ async def test_mutating_tools_require_strict_project_routing( monkeypatch, module_name: str, tool_name: str, - tool_args: dict[str, str], + tool_args: dict[str, str | bool], ) -> None: """Scope-hidden project prefixes must stop before any mutation client call.""" tool_module = importlib.import_module(module_name) @@ -49,3 +62,34 @@ async def reject_scope_hidden_route(*args: object, **kwargs: object) -> None: tool = getattr(tool_module, tool_name) with pytest.raises(ToolError, match="does not have access"): await tool(project=test_project.name, **tool_args) + + +@pytest.mark.asyncio +async def test_directory_move_normalizes_project_memory_url(client, test_project) -> None: + """Directory moves pass a project-relative source path to the mutation API.""" + await write_note( + project=test_project.name, + title="Strict Directory Move", + directory="strict-directory-source", + content="# Strict Directory Move\nMove this note with a memory URL.", + ) + + result = await move_note( + project=test_project.name, + identifier=f"memory://{test_project.name}/strict-directory-source", + destination_path="strict-directory-destination", + is_directory=True, + output_format="json", + ) + + assert isinstance(result, dict) + assert result["moved"] is True + assert result["total_files"] == 1 + assert result["successful_moves"] == 1 + assert result["failed_moves"] == 0 + + moved_note = await read_note( + "strict-directory-destination/strict-directory-move", + project=test_project.name, + ) + assert "Move this note with a memory URL." in moved_note From 6bf8181648fd41ce62463845f7a86c7950394293 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 17:58:26 -0500 Subject: [PATCH 5/8] fix(mcp): preserve existing path mutations Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 19 +++++---- src/basic_memory/mcp/tools/delete_note.py | 1 + src/basic_memory/mcp/tools/move_note.py | 1 + tests/mcp/test_project_context_scoped_key.py | 39 +++++++++++++++++++ .../tools/test_mutating_memory_url_routing.py | 1 + 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 5a89343ee..012fad932 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1298,6 +1298,7 @@ async def resolve_project_and_path( headers: HeaderTypes | None = None, *, strict_project_routing: bool = False, + allow_missing_project_fallback: bool = False, ) -> tuple[ProjectItem, str, bool]: """Resolve project and normalized path for memory:// identifiers. @@ -1305,6 +1306,10 @@ async def resolve_project_and_path( strict_project_routing: Reject a memory URL whose leading project-like segment cannot be resolved. Mutating tools use this to prevent a failed route from falling back to the active project. + allow_missing_project_fallback: When strict routing is enabled, still + allow a genuinely missing project prefix to be treated as an active- + project path. This is safe only for mutations that require an existing + target and cannot create content. Returns: Tuple of (active_project, normalized_path, is_memory_url) @@ -1417,13 +1422,13 @@ async def resolve_project_and_path( ) if not project_route_missing and not project_route_hidden_by_scope: raise - if strict_project_routing: - # Trigger: a mutating tool supplied a memory URL whose leading - # project-like segment did not resolve. - # Why: falling back would reinterpret the full route as a path - # in the active project, allowing append/prepend to create a - # phantom note in the wrong project (#1066). - # Outcome: stop before entity resolution or file creation. + if strict_project_routing and not ( + project_route_missing and allow_missing_project_fallback + ): + # Mutations that can create content must not reinterpret a + # missing project route as an active-project path (#1066). + # Existing-target mutations may opt into that legacy path + # fallback, while scope-hidden routes always fail above. raise UnresolvedProjectRouteError(identifier, project_prefix) from exc else: resolved_project = await resolve_project_parameter(project_prefix, context=context) diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 2cce57ca2..306ce19f6 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -303,6 +303,7 @@ async def delete_note( active_project.name, context, strict_project_routing=True, + allow_missing_project_fallback=True, ) # Handle directory deletes diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index 5f07962d9..29260e4be 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -605,6 +605,7 @@ async def move_note( active_project.name, context, strict_project_routing=True, + allow_missing_project_fallback=True, ) # move_note only supports moves inside the active project. diff --git a/tests/mcp/test_project_context_scoped_key.py b/tests/mcp/test_project_context_scoped_key.py index 7e990f109..5ae9430cd 100644 --- a/tests/mcp/test_project_context_scoped_key.py +++ b/tests/mcp/test_project_context_scoped_key.py @@ -93,6 +93,45 @@ async def reject_project_route(*args: object, **kwargs: object) -> None: ) +@pytest.mark.asyncio +async def test_existing_target_mutation_can_fallback_for_missing_project( + config_manager, + monkeypatch, +) -> None: + """A missing project prefix may still identify a path for non-creating mutations.""" + config = config_manager.load_config() + config.permalinks_include_project = True + config_manager.save_config(config) + + context = ContextState() + active_project = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="ci-acceptance", + path="/tmp/ci-acceptance", + is_default=False, + ) + await context.set_state("active_project", active_project.model_dump()) + + async def reject_missing_project(*args: object, **kwargs: object) -> None: + raise ToolError("Project not found: 'src'") + + monkeypatch.setattr("basic_memory.mcp.tools.utils.call_post", reject_missing_project) + + resolved_project, resolved_path, is_memory_url = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://src/existing-note", + project="ci-acceptance", + context=cast(Any, context), + strict_project_routing=True, + allow_missing_project_fallback=True, + ) + + assert resolved_project == active_project + assert resolved_path == "ci-acceptance/src/existing-note" + assert is_memory_url is True + + @pytest.mark.asyncio async def test_read_route_requires_an_authorized_cached_project( config_manager, diff --git a/tests/mcp/tools/test_mutating_memory_url_routing.py b/tests/mcp/tools/test_mutating_memory_url_routing.py index 0090f23c9..0d5b3fa99 100644 --- a/tests/mcp/tools/test_mutating_memory_url_routing.py +++ b/tests/mcp/tools/test_mutating_memory_url_routing.py @@ -51,6 +51,7 @@ async def test_mutating_tools_require_strict_project_routing( async def reject_scope_hidden_route(*args: object, **kwargs: object) -> None: assert kwargs["strict_project_routing"] is True + assert kwargs["allow_missing_project_fallback"] is True raise ToolError("This API key does not have access to this project") monkeypatch.setattr( From cec709bbddec8e9d8ae5a808a529e3e1d3f7f0eb Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 18:10:41 -0500 Subject: [PATCH 6/8] fix(mcp): reject empty directory mutations Signed-off-by: phernandez --- src/basic_memory/mcp/tools/delete_note.py | 13 +++++-- src/basic_memory/mcp/tools/move_note.py | 14 +++++++- .../tools/test_mutating_memory_url_routing.py | 34 +++++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 306ce19f6..49d3b6445 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -325,7 +325,7 @@ async def delete_note( result = await knowledge_client.delete_directory(directory_identifier) if output_format == "json": response = { - "deleted": result.failed_deletes == 0, + "deleted": result.total_files > 0 and result.failed_deletes == 0, "is_directory": True, "identifier": identifier, "total_files": result.total_files, @@ -334,13 +334,22 @@ async def delete_note( "deleted_files": result.deleted_files, "errors": [error.model_dump() for error in result.errors], } - if result.failed_deletes > 0: + if result.total_files == 0: + response["error"] = "Directory not found or empty: no files matched" + elif result.failed_deletes > 0: response["error"] = ( "Directory delete incomplete: " f"{result.failed_deletes} of {result.total_files} file(s) failed" ) return response + if result.total_files == 0: + return f"""# Directory Delete Failed - No Files Found + +No files matched directory `{identifier}`. + +""" + # Build success message for directory delete result_lines = [ "# Directory Deleted Successfully", diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index 29260e4be..e97a43a32 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -643,7 +643,7 @@ async def move_note( result = await knowledge_client.move_directory(source_directory, destination_path) if output_format == "json": return { - "moved": result.failed_moves == 0, + "moved": result.total_files > 0 and result.failed_moves == 0, "title": None, "permalink": None, "file_path": None, @@ -653,8 +653,20 @@ async def move_note( "total_files": result.total_files, "successful_moves": result.successful_moves, "failed_moves": result.failed_moves, + **( + {"error": "Directory not found or empty: no files matched"} + if result.total_files == 0 + else {} + ), } + if result.total_files == 0: + return f"""# Directory Move Failed - No Files Found + +No files matched source directory `{identifier}`. + +""" + # Build success message for directory move result_lines = [ "# Directory Moved Successfully", diff --git a/tests/mcp/tools/test_mutating_memory_url_routing.py b/tests/mcp/tools/test_mutating_memory_url_routing.py index 0d5b3fa99..eccf1514e 100644 --- a/tests/mcp/tools/test_mutating_memory_url_routing.py +++ b/tests/mcp/tools/test_mutating_memory_url_routing.py @@ -5,6 +5,7 @@ import pytest from mcp.server.fastmcp.exceptions import ToolError +from basic_memory.mcp.tools.delete_note import delete_note from basic_memory.mcp.tools.move_note import move_note from basic_memory.mcp.tools.read_note import read_note from basic_memory.mcp.tools.write_note import write_note @@ -94,3 +95,36 @@ async def test_directory_move_normalizes_project_memory_url(client, test_project project=test_project.name, ) assert "Move this note with a memory URL." in moved_note + + +@pytest.mark.asyncio +async def test_missing_directory_delete_is_not_reported_as_success(client, test_project) -> None: + """A missing-route path fallback must still require files to delete.""" + result = await delete_note( + project=test_project.name, + identifier="memory://missing-directory-route/missing-child", + is_directory=True, + output_format="json", + ) + + assert isinstance(result, dict) + assert result["deleted"] is False + assert result["total_files"] == 0 + assert result["error"] == "Directory not found or empty: no files matched" + + +@pytest.mark.asyncio +async def test_missing_directory_move_is_not_reported_as_success(client, test_project) -> None: + """A missing-route path fallback must still require files to move.""" + result = await move_note( + project=test_project.name, + identifier="memory://missing-directory-route/missing-child", + destination_path="archive/missing-child", + is_directory=True, + output_format="json", + ) + + assert isinstance(result, dict) + assert result["moved"] is False + assert result["total_files"] == 0 + assert result["error"] == "Directory not found or empty: no files matched" From 47d9764bd6e515aa199536ae8ae1a50c56b783ce Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 18:18:11 -0500 Subject: [PATCH 7/8] fix(mcp): preserve empty directory messages Signed-off-by: phernandez --- src/basic_memory/mcp/tools/delete_note.py | 3 ++- src/basic_memory/mcp/tools/move_note.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 49d3b6445..07f6f32a1 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -346,7 +346,8 @@ async def delete_note( if result.total_files == 0: return f"""# Directory Delete Failed - No Files Found -No files matched directory `{identifier}`. +No files found for directory `{identifier}`. +Total files: 0. """ diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index e97a43a32..96dbfc3ef 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -663,7 +663,8 @@ async def move_note( if result.total_files == 0: return f"""# Directory Move Failed - No Files Found -No files matched source directory `{identifier}`. +No files found for source directory `{identifier}`. +Total files: 0. """ From ddd20b80b099bd8d9fd956747c439508b5994965 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Jul 2026 18:24:20 -0500 Subject: [PATCH 8/8] fix(mcp): preserve move project context Signed-off-by: phernandez --- src/basic_memory/mcp/project_context.py | 7 ++- src/basic_memory/mcp/tools/move_note.py | 1 + tests/mcp/test_project_context_scoped_key.py | 48 +++++++++++++++++++ .../tools/test_mutating_memory_url_routing.py | 2 + 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/basic_memory/mcp/project_context.py b/src/basic_memory/mcp/project_context.py index 012fad932..2c5c7801a 100644 --- a/src/basic_memory/mcp/project_context.py +++ b/src/basic_memory/mcp/project_context.py @@ -1299,6 +1299,7 @@ async def resolve_project_and_path( *, strict_project_routing: bool = False, allow_missing_project_fallback: bool = False, + cache_resolved_project: bool = True, ) -> tuple[ProjectItem, str, bool]: """Resolve project and normalized path for memory:// identifiers. @@ -1310,6 +1311,9 @@ async def resolve_project_and_path( allow a genuinely missing project prefix to be treated as an active- project path. This is safe only for mutations that require an existing target and cannot create content. + cache_resolved_project: Persist a project resolved from the memory URL in + MCP context. Set this to false for validation-only routing that may + reject a resolved cross-project source. Returns: Tuple of (active_project, normalized_path, is_memory_url) @@ -1446,7 +1450,8 @@ async def resolve_project_and_path( path=resolved.path, is_default=resolved.is_default, ) - await _set_cached_active_project(context, active_project) + if cache_resolved_project: + await _set_cached_active_project(context, active_project) resolved_path = _canonical_memory_path_for_active_route( active_project, diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index 96dbfc3ef..7e9afcea3 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -606,6 +606,7 @@ async def move_note( context, strict_project_routing=True, allow_missing_project_fallback=True, + cache_resolved_project=False, ) # move_note only supports moves inside the active project. diff --git a/tests/mcp/test_project_context_scoped_key.py b/tests/mcp/test_project_context_scoped_key.py index 5ae9430cd..bb037bc91 100644 --- a/tests/mcp/test_project_context_scoped_key.py +++ b/tests/mcp/test_project_context_scoped_key.py @@ -132,6 +132,54 @@ async def reject_missing_project(*args: object, **kwargs: object) -> None: assert is_memory_url is True +@pytest.mark.asyncio +async def test_validation_only_route_preserves_cached_active_project( + config_manager, + monkeypatch, +) -> None: + """A rejected cross-project mutation must not change subsequent routing state.""" + context = ContextState() + active_project = ProjectItem( + id=1, + external_id="11111111-1111-1111-1111-111111111111", + name="ci-acceptance", + path="/tmp/ci-acceptance", + is_default=False, + ) + await context.set_state("active_project", active_project.model_dump()) + + class ResolvedProjectResponse: + def json(self) -> dict[str, object]: + return { + "external_id": "22222222-2222-2222-2222-222222222222", + "project_id": 2, + "name": "other-project", + "permalink": "other-project", + "path": "/tmp/other-project", + "is_active": True, + "is_default": False, + "resolution_method": "name", + } + + async def resolve_other_project(*args: object, **kwargs: object) -> ResolvedProjectResponse: + return ResolvedProjectResponse() + + monkeypatch.setattr("basic_memory.mcp.tools.utils.call_post", resolve_other_project) + + resolved_project, _, _ = await resolve_project_and_path( + client=cast(Any, None), + identifier="memory://other-project/source-directory", + project="ci-acceptance", + context=cast(Any, context), + strict_project_routing=True, + allow_missing_project_fallback=True, + cache_resolved_project=False, + ) + + assert resolved_project.name == "other-project" + assert context.state["active_project"] == active_project.model_dump() + + @pytest.mark.asyncio async def test_read_route_requires_an_authorized_cached_project( config_manager, diff --git a/tests/mcp/tools/test_mutating_memory_url_routing.py b/tests/mcp/tools/test_mutating_memory_url_routing.py index eccf1514e..6ef3772e1 100644 --- a/tests/mcp/tools/test_mutating_memory_url_routing.py +++ b/tests/mcp/tools/test_mutating_memory_url_routing.py @@ -53,6 +53,8 @@ async def test_mutating_tools_require_strict_project_routing( async def reject_scope_hidden_route(*args: object, **kwargs: object) -> None: assert kwargs["strict_project_routing"] is True assert kwargs["allow_missing_project_fallback"] is True + if tool_name == "move_note": + assert kwargs["cache_resolved_project"] is False raise ToolError("This API key does not have access to this project") monkeypatch.setattr(