mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(mcp): parse workspace-qualified permalinks in edit_note
edit_note used detect_project_from_memory_url_prefix gated on
`startswith("memory://")`, so plain workspace-qualified identifiers
like `team-slug/project/path/note` were never routed to the correct
workspace project, causing append/prepend to create stray notes in
the default project.
Switch to detect_project_from_identifier_prefix (no memory:// guard),
matching read_note. Also fix the auto-create fallback to derive the
note title/directory from entity_identifier (workspace prefix already
stripped by resolve_project_and_path) rather than the raw identifier.
Closes #810
Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
This commit is contained in:
@@ -9,7 +9,7 @@ from pydantic import AliasChoices, Field
|
||||
|
||||
from basic_memory.config import ConfigManager
|
||||
from basic_memory.mcp.project_context import (
|
||||
detect_project_from_memory_url_prefix,
|
||||
detect_project_from_identifier_prefix,
|
||||
get_project_client,
|
||||
add_project_metadata,
|
||||
resolve_project_and_path,
|
||||
@@ -288,13 +288,14 @@ 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
|
||||
# Trigger: identifier starts with memory:// and no explicit project/project_id 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 project_id is None and identifier.strip().startswith("memory://"):
|
||||
detected = await detect_project_from_memory_url_prefix(
|
||||
# Detect project from identifier prefix before routing.
|
||||
# Trigger: no explicit project/project_id was provided
|
||||
# Why: handles both memory:// URLs and workspace-qualified plain permalinks
|
||||
# (e.g. "basic-memory-xxx/project/path/note") so edits route to the right
|
||||
# workspace instead of the default project
|
||||
# Outcome: project is set from the identifier prefix, routing goes to the correct project
|
||||
if project is None and project_id is None:
|
||||
detected = await detect_project_from_identifier_prefix(
|
||||
identifier,
|
||||
ConfigManager().config,
|
||||
context=context,
|
||||
@@ -377,7 +378,12 @@ async def edit_note(
|
||||
is_not_found = "entity not found" in error_msg or "not found" in error_msg
|
||||
|
||||
if is_not_found and operation in ("append", "prepend"):
|
||||
title, directory = _parse_identifier_to_title_and_directory(identifier)
|
||||
# Use the resolved path (workspace prefix already stripped) so the
|
||||
# note is created at the clean project-relative path, not the
|
||||
# workspace-qualified form that was passed as the raw identifier.
|
||||
title, directory = _parse_identifier_to_title_and_directory(
|
||||
entity_identifier
|
||||
)
|
||||
|
||||
# Validate directory path (same security check as write_note)
|
||||
project_path = active_project.home
|
||||
|
||||
@@ -829,15 +829,15 @@ async def test_edit_note_workspace_qualified_memory_url_keeps_complete_permalink
|
||||
|
||||
@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'.
|
||||
|
||||
A plain path like 'research/note' should not be misrouted to a project
|
||||
named 'research' — the 'research' segment is a directory, not a project.
|
||||
detect_project_from_identifier_prefix is called, but returns None for a plain
|
||||
non-workspace-qualified path so the default project is used.
|
||||
"""
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.edit_note.detect_project_from_memory_url_prefix"
|
||||
"basic_memory.mcp.tools.edit_note.detect_project_from_identifier_prefix",
|
||||
return_value=None,
|
||||
) as mock_detect:
|
||||
# Use a plain path (no memory:// prefix) — detection should not be called
|
||||
await edit_note(
|
||||
identifier="test/some-note",
|
||||
operation="append",
|
||||
@@ -845,14 +845,14 @@ async def test_edit_note_skips_detection_for_plain_path(client, test_project):
|
||||
project=None,
|
||||
)
|
||||
|
||||
mock_detect.assert_not_called()
|
||||
mock_detect.assert_called_once()
|
||||
|
||||
|
||||
@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_memory_url_prefix"
|
||||
"basic_memory.mcp.tools.edit_note.detect_project_from_identifier_prefix"
|
||||
) as mock_detect:
|
||||
await edit_note(
|
||||
identifier=f"memory://{test_project.name}/test/some-note",
|
||||
@@ -884,7 +884,7 @@ async def test_edit_note_skips_detection_when_project_id_provided(
|
||||
import importlib
|
||||
|
||||
edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
|
||||
monkeypatch.setattr(edit_note_module, "detect_project_from_memory_url_prefix", fail_if_called)
|
||||
monkeypatch.setattr(edit_note_module, "detect_project_from_identifier_prefix", fail_if_called)
|
||||
|
||||
result = await edit_note(
|
||||
identifier=f"memory://{test_project.name}/test/project-id-memory-url-edit",
|
||||
@@ -896,3 +896,45 @@ async def test_edit_note_skips_detection_when_project_id_provided(
|
||||
assert isinstance(result, str)
|
||||
assert "Edited note (append)" in result
|
||||
assert f"project: {test_project.name}" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_note_workspace_qualified_plain_permalink_routes_correctly(
|
||||
client,
|
||||
test_project,
|
||||
):
|
||||
"""edit_note must route workspace-qualified plain permalinks to the right project.
|
||||
|
||||
This is the bug from issue #810: passing a workspace-qualified identifier like
|
||||
'team-slug/project-name/path/note' (no memory:// prefix) would bypass project
|
||||
detection and create a stray note in the default project instead of editing
|
||||
the existing note in the target workspace project.
|
||||
"""
|
||||
from basic_memory.workspace_context import workspace_permalink_context
|
||||
|
||||
workspace_slug = "team-acme"
|
||||
qualified_identifier = f"{workspace_slug}/{test_project.name}/docs/ws-plain-note"
|
||||
|
||||
with workspace_permalink_context(workspace_slug=workspace_slug, workspace_type="organization"):
|
||||
await write_note(
|
||||
project=test_project.name,
|
||||
title="Ws Plain Note",
|
||||
directory="docs",
|
||||
content="# Ws Plain Note\nOriginal content.",
|
||||
)
|
||||
|
||||
# Simulate cloud detection: detect_project_from_identifier_prefix resolves the
|
||||
# workspace slug to the correct project name so routing goes to test_project.
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.edit_note.detect_project_from_identifier_prefix",
|
||||
return_value=test_project.name,
|
||||
):
|
||||
result = await edit_note(
|
||||
identifier=qualified_identifier,
|
||||
operation="append",
|
||||
content="\nAppended via plain workspace-qualified permalink.",
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "Edited note (append)" in result
|
||||
assert f"project: {test_project.name}" in result
|
||||
|
||||
Reference in New Issue
Block a user