From 7c12ff39e1ce9f7523959487b7364e223319c3b4 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 14 Mar 2026 15:41:35 -0500 Subject: [PATCH 1/2] fix(mcp): add project detection from memory:// URLs in edit_note and delete_note edit_note and delete_note were missing detect_project_from_url_prefix() which other tools (read_note, build_context, search_notes, read_content) already use. This caused incorrect cloud routing when using memory:// URLs without an explicit project parameter in multi-project setups. Fixes #664 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: phernandez --- src/basic_memory/mcp/tools/delete_note.py | 9 ++++++++- src/basic_memory/mcp/tools/edit_note.py | 13 ++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 7da89622..56b1882c 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -5,7 +5,8 @@ from fastmcp import Context from mcp.server.fastmcp.exceptions import ToolError -from basic_memory.mcp.project_context import get_project_client +from basic_memory.config import ConfigManager +from basic_memory.mcp.project_context import detect_project_from_url_prefix, get_project_client from basic_memory.mcp.server import mcp @@ -222,6 +223,12 @@ async def delete_note( with suggestions for finding the correct identifier, including search commands and alternative formats to try. """ + # Detect project from memory URL prefix before routing + if project is None: + detected = detect_project_from_url_prefix(identifier, ConfigManager().config) + if detected: + project = detected + async with get_project_client(project, workspace, context) as (client, active_project): logger.debug( f"Deleting {'directory' if is_directory else 'note'}: {identifier} in project: {active_project.name}" diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index 505a39f5..09c384f8 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -5,7 +5,12 @@ from loguru import logger from fastmcp import Context -from basic_memory.mcp.project_context import get_project_client, add_project_metadata +from basic_memory.config import ConfigManager +from basic_memory.mcp.project_context import ( + detect_project_from_url_prefix, + get_project_client, + add_project_metadata, +) from basic_memory.mcp.server import mcp from basic_memory.schemas.base import Entity from basic_memory.schemas.response import EntityResponse @@ -255,6 +260,12 @@ async def edit_note( # Resolve effective default: allow MCP clients to send null for optional int field effective_replacements = expected_replacements if expected_replacements is not None else 1 + # Detect project from memory URL prefix before routing + if project is None: + detected = detect_project_from_url_prefix(identifier, ConfigManager().config) + if detected: + project = detected + async with get_project_client(project, workspace, context) as (client, active_project): logger.info("MCP tool call", tool="edit_note", identifier=identifier, operation=operation) From c5530194c1f7bf324dc2ea82deb3027ae94b2970 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 14 Mar 2026 18:13:11 -0500 Subject: [PATCH 2/2] fix: gate URL detection to memory:// prefix and add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address code review feedback on PR #668: 1. Gate detect_project_from_url_prefix() to only trigger for memory:// identifiers. Plain paths like "research/note" could previously misroute to a project named "research" — now only explicit memory:// URLs trigger project detection for these destructive/mutating tools. 2. Add 6 tests covering: - Detection works with memory:// URLs (edit_note + delete_note) - Detection is NOT called for plain path identifiers - Detection is NOT called when project is explicitly provided Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: phernandez --- src/basic_memory/mcp/tools/delete_note.py | 6 ++- src/basic_memory/mcp/tools/edit_note.py | 6 ++- tests/mcp/test_tool_delete_note.py | 55 +++++++++++++++++++++ tests/mcp/test_tool_edit_note.py | 59 +++++++++++++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index 56b1882c..fa4f70c3 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -224,7 +224,11 @@ async def delete_note( commands and alternative formats to try. """ # Detect project from memory URL prefix before routing - if project is None: + # Trigger: identifier starts with memory:// and no explicit project was provided + # Why: only gate on memory:// to avoid misrouting plain paths like "research/note" + # where "research" is a directory, not a project name + # Outcome: project is set from the URL prefix, routing goes to the correct project + if project is None and identifier.strip().startswith("memory://"): detected = detect_project_from_url_prefix(identifier, ConfigManager().config) if detected: project = detected diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index 09c384f8..8b9c726c 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -261,7 +261,11 @@ async def edit_note( effective_replacements = expected_replacements if expected_replacements is not None else 1 # Detect project from memory URL prefix before routing - if project is None: + # Trigger: identifier starts with memory:// and no explicit project was provided + # Why: only gate on memory:// to avoid misrouting plain paths like "research/note" + # where "research" is a directory, not a project name + # Outcome: project is set from the URL prefix, routing goes to the correct project + if project is None and identifier.strip().startswith("memory://"): detected = detect_project_from_url_prefix(identifier, ConfigManager().config) if detected: project = detected diff --git a/tests/mcp/test_tool_delete_note.py b/tests/mcp/test_tool_delete_note.py index f8647771..89f17a0b 100644 --- a/tests/mcp/test_tool_delete_note.py +++ b/tests/mcp/test_tool_delete_note.py @@ -1,5 +1,7 @@ """Tests for delete_note MCP tool.""" +from unittest.mock import patch + import pytest from basic_memory.mcp.tools.delete_note import delete_note, _format_delete_error_response @@ -120,3 +122,56 @@ async def test_delete_note_rejects_fuzzy_match(client, test_project): # Verify the existing note was NOT deleted content = await read_note("Delete Target Note", project=test_project.name) assert "Should not be deleted" in content + + +@pytest.mark.asyncio +async def test_delete_note_detects_project_from_memory_url(client, test_project): + """delete_note should detect project from memory:// URL prefix when project=None.""" + # Create a note to delete + await write_note( + project=test_project.name, + title="Delete URL Note", + directory="test", + content="# Delete URL Note\nContent to delete.", + ) + + # Delete using memory:// URL with project=None — should auto-detect project + # The note may or may not be found (depends on URL resolution), but the key + # assertion is that routing goes to the correct project + result = await delete_note( + identifier=f"memory://{test_project.name}/test/delete-url-note", + project=None, + ) + + # Result is True (deleted) or False (not found by that URL) — either is acceptable. + # The important thing is it didn't error and routed to the correct project. + assert isinstance(result, bool) + + +@pytest.mark.asyncio +async def test_delete_note_skips_detection_for_plain_path(client, test_project): + """delete_note should NOT call detect_project_from_url_prefix for plain path identifiers. + + A plain path like 'research/note' should not be misrouted to a project + named 'research' — the 'research' segment is a directory, not a project. + """ + with patch("basic_memory.mcp.tools.delete_note.detect_project_from_url_prefix") as mock_detect: + # Use a plain path (no memory:// prefix) — detection should not be called + await delete_note( + identifier="test/nonexistent-note", + project=None, + ) + + mock_detect.assert_not_called() + + +@pytest.mark.asyncio +async def test_delete_note_skips_detection_when_project_provided(client, test_project): + """delete_note should skip URL detection when project is explicitly provided.""" + with patch("basic_memory.mcp.tools.delete_note.detect_project_from_url_prefix") as mock_detect: + await delete_note( + identifier=f"memory://{test_project.name}/test/some-note", + project=test_project.name, + ) + + mock_detect.assert_not_called() diff --git a/tests/mcp/test_tool_edit_note.py b/tests/mcp/test_tool_edit_note.py index 70bf59ef..fc03cd0e 100644 --- a/tests/mcp/test_tool_edit_note.py +++ b/tests/mcp/test_tool_edit_note.py @@ -1,5 +1,6 @@ """Tests for the edit_note MCP tool.""" +from unittest.mock import patch import pytest @@ -770,3 +771,61 @@ async def test_edit_note_insert_before_section_not_found(client, test_project): assert isinstance(result, str) assert "# Edit Failed" in result + + +@pytest.mark.asyncio +async def test_edit_note_detects_project_from_memory_url(client, test_project): + """edit_note should detect project from memory:// URL prefix when project=None.""" + # Create a note first + await write_note( + project=test_project.name, + title="URL Detection Note", + directory="test", + content="# URL Detection Note\nOriginal content.", + ) + + # Edit using memory:// URL with project=None — should auto-detect project + # The memory URL uses the permalink (which includes project prefix) + result = await edit_note( + identifier=f"memory://{test_project.name}/test/url-detection-note", + operation="append", + content="\nAppended via memory URL.", + project=None, + ) + + assert isinstance(result, str) + # Should route to the correct project and succeed (either edit or create) + assert f"project: {test_project.name}" in result + + +@pytest.mark.asyncio +async def test_edit_note_skips_detection_for_plain_path(client, test_project): + """edit_note should NOT call detect_project_from_url_prefix for plain path identifiers. + + A plain path like 'research/note' should not be misrouted to a project + named 'research' — the 'research' segment is a directory, not a project. + """ + with patch("basic_memory.mcp.tools.edit_note.detect_project_from_url_prefix") as mock_detect: + # Use a plain path (no memory:// prefix) — detection should not be called + await edit_note( + identifier="test/some-note", + operation="append", + content="content", + project=None, + ) + + mock_detect.assert_not_called() + + +@pytest.mark.asyncio +async def test_edit_note_skips_detection_when_project_provided(client, test_project): + """edit_note should skip URL detection when project is explicitly provided.""" + with patch("basic_memory.mcp.tools.edit_note.detect_project_from_url_prefix") as mock_detect: + await edit_note( + identifier=f"memory://{test_project.name}/test/some-note", + operation="append", + content="content", + project=test_project.name, + ) + + mock_detect.assert_not_called()