fix(mcp): route edit_note workspace-qualified permalinks (#813)

Signed-off-by: phernandez <paul@basicmachines.co>

src/basic_memory/mcp/tools/edit_note.py

@@ -9,6 +9,7 @@ from pydantic import AliasChoices, Field
 
 from basic_memory.config import ConfigManager
 from basic_memory.mcp.project_context import (
+    _cloud_workspace_discovery_available,
     detect_project_from_memory_url_prefix,
     get_project_client,
     add_project_metadata,
@@ -17,7 +18,11 @@ from basic_memory.mcp.project_context import (
 from basic_memory.mcp.server import mcp
 from basic_memory.schemas.base import Entity
 from basic_memory.schemas.response import EntityResponse
-from basic_memory.utils import validate_project_path
+from basic_memory.services.link_resolver import (
+    detect_project_from_workspace_identifier_prefix,
+    is_workspace_qualified_plain_identifier,
+)
+from basic_memory.utils import normalize_project_reference, validate_project_path
 
 
 def _parse_identifier_to_title_and_directory(identifier: str) -> tuple[str, str]:
@@ -47,6 +52,58 @@ def _parse_identifier_to_title_and_directory(identifier: str) -> tuple[str, str]
     return title, directory
 
 
+def _compose_workspace_project_route(
+    *,
+    workspace: Optional[str],
+    project: Optional[str],
+    project_id: Optional[str],
+) -> Optional[str]:
+    """Return the explicit project route requested by workspace/project args."""
+    if workspace is None:
+        return project
+
+    cleaned_workspace = workspace.strip().strip("/")
+    if not cleaned_workspace:
+        raise ValueError("workspace must not be empty when provided")
+    if "/" in cleaned_workspace:
+        raise ValueError("workspace must be a single workspace slug, name, or tenant_id")
+    if project_id is not None:
+        raise ValueError("workspace cannot be combined with project_id; use project_id alone")
+    if project is None or not project.strip().strip("/"):
+        raise ValueError("workspace requires an explicit project argument")
+
+    cleaned_project = project.strip().strip("/")
+    if "/" in cleaned_project:
+        raise ValueError(
+            "Use either workspace='workspace' with project='project', "
+            "or project='workspace/project', not both"
+        )
+    return f"{cleaned_workspace}/{cleaned_project}"
+
+
+def _format_ambiguous_workspace_identifier_response(
+    *,
+    identifier: str,
+    detected_project: str,
+) -> str:
+    """Format the safe-stop response for ambiguous plain write identifiers."""
+    cleaned_identifier = identifier.strip()
+    normalized_identifier = normalize_project_reference(cleaned_identifier).strip("/")
+    workspace_hint, project_hint, note_identifier = normalized_identifier.split("/", 2)
+
+    return f"""# Edit Failed - Ambiguous Identifier
+
+`{cleaned_identifier}` could refer to a local note path in the active project, or to a note in `{detected_project}`.
+
+Because edit_note changes content, Basic Memory will not infer a workspace route from a plain path.
+
+Retry with one of these explicit routes:
+- `edit_note(identifier="{note_identifier}", project="{detected_project}", operation=..., content=...)`
+- `edit_note(identifier="{note_identifier}", workspace="{workspace_hint}", project="{project_hint}", operation=..., content=...)`
+- `edit_note(identifier="memory://{normalized_identifier}", operation=..., content=...)`
+- `edit_note(identifier="{note_identifier}", project_id="<project external_id>", operation=..., content=...)`"""
+
+
 def _format_error_response(
     error_message: str,
     operation: str,
@@ -181,6 +238,7 @@ async def edit_note(
         ),
     ],
     project: Optional[str] = None,
+    workspace: Optional[str] = None,
     project_id: Optional[str] = None,
     # Section/heading naming varies across tools; accept the descriptive forms.
     section: Annotated[
@@ -224,7 +282,10 @@ async def edit_note(
                   - "insert_after_section": Insert content after a section heading without consuming it (note must exist)
         content: The content to add or use for replacement
         project: Project name to edit in. Optional - server will resolve using hierarchy.
+                Use "workspace/project" to route to a project in a specific cloud workspace.
                 If unknown, use list_memory_projects() to discover available projects.
+        workspace: Workspace slug, name, or tenant_id. When provided with `project`,
+                routes as `workspace/project`. Cannot be combined with `project_id`.
         project_id: Project external_id (UUID). Prefer this over `project` when known —
                 it routes to the exact project regardless of name collisions across cloud
                 workspaces. Takes precedence over `project`. Get from list_memory_projects().
@@ -287,18 +348,52 @@ 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
+    project = _compose_workspace_project_route(
+        workspace=workspace,
+        project=project,
+        project_id=project_id,
+    )
 
-    # 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(
-            identifier,
-            ConfigManager().config,
-            context=context,
-        )
+    # Resolve or reject routable identifier prefixes before selecting a client.
+    # Trigger: no explicit project/project_id was provided.
+    # Why: memory:// URLs are explicit routes, but plain three-segment identifiers
+    #   are ambiguous for a mutating tool.
+    # Outcome: memory:// can route; plain workspace/project/path matches stop with
+    #   guidance instead of silently editing another project.
+    if project is None and project_id is None:
+        config = ConfigManager().config
+        if identifier.strip().startswith("memory://"):
+            detected = await detect_project_from_memory_url_prefix(
+                identifier,
+                config,
+                context=context,
+            )
+        elif _cloud_workspace_discovery_available(
+            config
+        ) and is_workspace_qualified_plain_identifier(identifier):
+            detected = await detect_project_from_workspace_identifier_prefix(
+                identifier,
+                config,
+                context=context,
+            )
+            if detected:
+                if output_format == "json":
+                    return {
+                        "title": None,
+                        "permalink": None,
+                        "file_path": None,
+                        "checksum": None,
+                        "operation": operation,
+                        "fileCreated": False,
+                        "error": "AMBIGUOUS_IDENTIFIER",
+                        "project": detected,
+                    }
+                return _format_ambiguous_workspace_identifier_response(
+                    identifier=identifier,
+                    detected_project=detected,
+                )
+        else:
+            detected = None
         if detected:
             project = detected
 

src/basic_memory/services/link_resolver.py

@@ -1,7 +1,7 @@
-"""Service for resolving markdown links to permalinks."""
+"""Service and helpers for resolving markdown links and permalink-like identifiers."""
 
 import uuid as uuid_mod
-from typing import Optional, Tuple, Dict
+from typing import Any, Optional, Tuple, Dict
 
 from loguru import logger
 
@@ -20,6 +20,42 @@ from basic_memory.utils import (
 from basic_memory.workspace_context import current_workspace_permalink_context
 
 
+def is_workspace_qualified_plain_identifier(identifier: str) -> bool:
+    """Return True for plain ``<workspace>/<project>/<path>`` identifiers."""
+    stripped = identifier.strip()
+    if stripped.startswith("memory://"):
+        return False
+
+    normalized = normalize_project_reference(stripped).strip("/")
+    return len(normalized.split("/", 2)) == 3
+
+
+async def detect_project_from_workspace_identifier_prefix(
+    identifier: str,
+    config: BasicMemoryConfig,
+    context: Any | None = None,
+) -> Optional[str]:
+    """Resolve a project route from a plain workspace-qualified identifier."""
+    if not is_workspace_qualified_plain_identifier(identifier):
+        return None
+
+    from basic_memory.mcp.project_context import (
+        _cloud_workspace_discovery_available,
+        resolve_workspace_qualified_identifier,
+    )
+
+    if not _cloud_workspace_discovery_available(config):
+        return None
+
+    workspace_resolution = await resolve_workspace_qualified_identifier(
+        identifier,
+        context=context,
+    )
+    if workspace_resolution is None:
+        return None
+    return workspace_resolution.project_identifier
+
+
 class LinkResolver:
     """Service for resolving markdown links to permalinks.
 

tests/mcp/test_tool_contracts.py

@@ -37,6 +37,7 @@ EXPECTED_TOOL_SIGNATURES: dict[str, list[str]] = {
         "operation",
         "content",
         "project",
+        "workspace",
         "project_id",
         "section",
         "find_text",

tests/mcp/test_tool_edit_note.py

@@ -9,6 +9,68 @@ from basic_memory.mcp.tools.read_note import read_note
 from basic_memory.mcp.tools.write_note import write_note
 
 
+def test_edit_note_workspace_project_route_helper():
+    """workspace/project routing should be explicit and deterministic."""
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+
+    assert (
+        edit_note_module._compose_workspace_project_route(
+            workspace=None,
+            project="docs/setup",
+            project_id=None,
+        )
+        == "docs/setup"
+    )
+    assert (
+        edit_note_module._compose_workspace_project_route(
+            workspace="docs",
+            project="setup",
+            project_id=None,
+        )
+        == "docs/setup"
+    )
+
+
+@pytest.mark.parametrize(
+    ("route_kwargs", "message"),
+    [
+        (
+            {"workspace": " ", "project": "setup", "project_id": None},
+            "workspace must not be empty",
+        ),
+        (
+            {"workspace": "docs/setup", "project": "setup", "project_id": None},
+            "workspace must be a single workspace",
+        ),
+        (
+            {"workspace": "docs", "project": "setup", "project_id": "project-id"},
+            "workspace cannot be combined with project_id",
+        ),
+        (
+            {"workspace": "docs", "project": None, "project_id": None},
+            "workspace requires an explicit project",
+        ),
+        (
+            {"workspace": "docs", "project": "setup/install", "project_id": None},
+            "not both",
+        ),
+    ],
+)
+def test_edit_note_workspace_project_route_helper_rejects_invalid_inputs(
+    route_kwargs,
+    message,
+):
+    """Ambiguous workspace/project argument combinations should fail before routing."""
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+
+    with pytest.raises(ValueError, match=message):
+        edit_note_module._compose_workspace_project_route(**route_kwargs)
+
+
 @pytest.mark.asyncio
 async def test_edit_note_append_operation(client, test_project):
     """Test appending content to an existing note."""
@@ -827,6 +889,221 @@ async def test_edit_note_workspace_qualified_memory_url_keeps_complete_permalink
     assert f"permalink: {expected_permalink}" in result
 
 
+@pytest.mark.asyncio
+async def test_edit_note_workspace_qualified_plain_permalink_requires_explicit_route(
+    monkeypatch,
+    test_project,
+):
+    """Plain workspace-qualified write identifiers should stop before mutating."""
+    import importlib
+    from contextlib import asynccontextmanager
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+    workspace_slug = "team-acme"
+    qualified_identifier = f"{workspace_slug}/{test_project.name}/team/plain-edit-note"
+    detected_identifiers: list[str] = []
+
+    async def detect_workspace_project(identifier, config, context=None):
+        detected_identifiers.append(identifier)
+        return f"{workspace_slug}/{test_project.name}"
+
+    @asynccontextmanager
+    async def fail_if_called(*args, **kwargs):
+        raise AssertionError("ambiguous plain identifiers should not select a project client")
+        yield
+
+    monkeypatch.setattr(
+        edit_note_module,
+        "_cloud_workspace_discovery_available",
+        lambda config: True,
+    )
+    monkeypatch.setattr(
+        edit_note_module,
+        "detect_project_from_workspace_identifier_prefix",
+        detect_workspace_project,
+        raising=False,
+    )
+    monkeypatch.setattr(edit_note_module, "get_project_client", fail_if_called)
+
+    result = await edit_note(
+        identifier=qualified_identifier,
+        operation="append",
+        content="\nAppended via plain workspace-qualified permalink.",
+        project=None,
+    )
+
+    assert detected_identifiers == [qualified_identifier]
+    assert isinstance(result, str)
+    assert "# Edit Failed - Ambiguous Identifier" in result
+    assert f"`{qualified_identifier}` could refer to a local note path" in result
+    assert f'project="{workspace_slug}/{test_project.name}"' in result
+    assert f"memory://{qualified_identifier}" in result
+
+
+@pytest.mark.asyncio
+async def test_edit_note_workspace_qualified_plain_permalink_json_error(
+    monkeypatch,
+    test_project,
+):
+    """Ambiguous plain write identifiers should stay machine-readable in JSON mode."""
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+    workspace_slug = "team-acme"
+    qualified_identifier = f"{workspace_slug}/{test_project.name}/team/plain-edit-note"
+
+    async def detect_workspace_project(identifier, config, context=None):
+        assert identifier == qualified_identifier
+        return f"{workspace_slug}/{test_project.name}"
+
+    monkeypatch.setattr(
+        edit_note_module,
+        "_cloud_workspace_discovery_available",
+        lambda config: True,
+    )
+    monkeypatch.setattr(
+        edit_note_module,
+        "detect_project_from_workspace_identifier_prefix",
+        detect_workspace_project,
+        raising=False,
+    )
+
+    result = await edit_note(
+        identifier=qualified_identifier,
+        operation="append",
+        content="\nAppended via plain workspace-qualified permalink.",
+        output_format="json",
+        project=None,
+    )
+
+    assert isinstance(result, dict)
+    assert result["error"] == "AMBIGUOUS_IDENTIFIER"
+    assert result["project"] == f"{workspace_slug}/{test_project.name}"
+    assert result["fileCreated"] is False
+
+
+@pytest.mark.asyncio
+async def test_edit_note_ambiguous_namespace_identifier_returns_guidance(
+    monkeypatch,
+    test_project,
+):
+    """Namespace-style workspace identifiers should still return ambiguity guidance."""
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+    workspace_slug = "team-acme"
+    identifier = f"{workspace_slug}::{test_project.name}/team/plain-edit-note"
+    normalized_identifier = f"{workspace_slug}/{test_project.name}/team/plain-edit-note"
+
+    async def detect_workspace_project(raw_identifier, config, context=None):
+        assert raw_identifier == identifier
+        return f"{workspace_slug}/{test_project.name}"
+
+    monkeypatch.setattr(
+        edit_note_module,
+        "_cloud_workspace_discovery_available",
+        lambda config: True,
+    )
+    monkeypatch.setattr(
+        edit_note_module,
+        "detect_project_from_workspace_identifier_prefix",
+        detect_workspace_project,
+        raising=False,
+    )
+
+    result = await edit_note(
+        identifier=identifier,
+        operation="append",
+        content="\nAppended via namespace-style plain identifier.",
+        project=None,
+    )
+
+    assert isinstance(result, str)
+    assert "# Edit Failed - Ambiguous Identifier" in result
+    assert f"`{identifier}` could refer to a local note path" in result
+    assert f"memory://{normalized_identifier}" in result
+
+
+@pytest.mark.asyncio
+async def test_edit_note_workspace_project_args_compose_explicit_route(monkeypatch):
+    """workspace plus project should route like project='workspace/project'."""
+    from contextlib import asynccontextmanager
+    from types import SimpleNamespace
+
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+    captured_routes: list[tuple[str | None, str | None]] = []
+
+    @asynccontextmanager
+    async def fake_get_project_client(project, context=None, project_id=None):
+        captured_routes.append((project, project_id))
+        yield object(), SimpleNamespace(name="setup")
+
+    monkeypatch.setattr(edit_note_module, "get_project_client", fake_get_project_client)
+
+    with pytest.raises(ValueError, match="Invalid operation"):
+        await edit_note(
+            identifier="install",
+            operation="invalid",
+            content="content",
+            workspace="docs",
+            project="setup",
+        )
+
+    assert captured_routes == [("docs/setup", None)]
+
+
+@pytest.mark.asyncio
+async def test_edit_note_three_segment_plain_path_stays_local_without_workspace_discovery(
+    monkeypatch,
+    client,
+    test_project,
+):
+    """Three-segment local paths should stay on the active project without discovery."""
+    import importlib
+
+    edit_note_module = importlib.import_module("basic_memory.mcp.tools.edit_note")
+
+    await write_note(
+        project=test_project.name,
+        title="Local Three Segment",
+        directory="folder/subdir",
+        content="# Local Three Segment\nOriginal content.",
+    )
+
+    async def fail_if_called(*args, **kwargs):
+        raise AssertionError("local three-segment paths should not trigger workspace detection")
+
+    monkeypatch.setattr(
+        edit_note_module,
+        "_cloud_workspace_discovery_available",
+        lambda config: False,
+    )
+    monkeypatch.setattr(
+        edit_note_module,
+        "detect_project_from_workspace_identifier_prefix",
+        fail_if_called,
+    )
+
+    result = await edit_note(
+        identifier="folder/subdir/local-three-segment",
+        operation="append",
+        content="\nAppended locally.",
+        project=None,
+    )
+
+    assert isinstance(result, str)
+    assert "Edited note (append)" in result
+    assert f"project: {test_project.name}" in result
+
+    updated = await read_note(
+        identifier="folder/subdir/local-three-segment",
+        project=test_project.name,
+    )
+    assert "Appended locally." in updated
+
+
 @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.
@@ -834,9 +1111,12 @@ async def test_edit_note_skips_detection_for_plain_path(client, test_project):
     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_memory_url_prefix"
-    ) as mock_detect:
+    with (
+        patch("basic_memory.mcp.tools.edit_note.detect_project_from_memory_url_prefix") as mock_url,
+        patch(
+            "basic_memory.mcp.tools.edit_note.detect_project_from_workspace_identifier_prefix"
+        ) as mock_workspace,
+    ):
         # Use a plain path (no memory:// prefix) — detection should not be called
         await edit_note(
             identifier="test/some-note",
@@ -845,15 +1125,19 @@ async def test_edit_note_skips_detection_for_plain_path(client, test_project):
             project=None,
         )
 
-        mock_detect.assert_not_called()
+        mock_url.assert_not_called()
+        mock_workspace.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_memory_url_prefix"
-    ) as mock_detect:
+    with (
+        patch("basic_memory.mcp.tools.edit_note.detect_project_from_memory_url_prefix") as mock_url,
+        patch(
+            "basic_memory.mcp.tools.edit_note.detect_project_from_workspace_identifier_prefix"
+        ) as mock_workspace,
+    ):
         await edit_note(
             identifier=f"memory://{test_project.name}/test/some-note",
             operation="append",
@@ -861,7 +1145,8 @@ async def test_edit_note_skips_detection_when_project_provided(client, test_proj
             project=test_project.name,
         )
 
-        mock_detect.assert_not_called()
+        mock_url.assert_not_called()
+        mock_workspace.assert_not_called()
 
 
 @pytest.mark.asyncio
@@ -885,6 +1170,11 @@ async def test_edit_note_skips_detection_when_project_id_provided(
 
     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_workspace_identifier_prefix",
+        fail_if_called,
+    )
 
     result = await edit_note(
         identifier=f"memory://{test_project.name}/test/project-id-memory-url-edit",

tests/services/test_link_resolver.py

@@ -2,6 +2,7 @@
 
 import uuid
 from datetime import datetime, timezone
+from types import SimpleNamespace
 
 import pytest
 
@@ -118,6 +119,92 @@ def project_prefix(test_entities) -> str:
     return test_entities[0].permalink.split("/", 1)[0]
 
 
+def test_workspace_qualified_plain_identifier_shape_helper_lives_in_link_resolver():
+    """Workspace route shape detection belongs with permalink/link resolution."""
+    from basic_memory.services import link_resolver
+
+    assert link_resolver.is_workspace_qualified_plain_identifier("team-acme/research/note")
+    assert link_resolver.is_workspace_qualified_plain_identifier("team-acme/research/folder/note")
+    assert not link_resolver.is_workspace_qualified_plain_identifier(
+        "memory://team-acme/research/note"
+    )
+    assert not link_resolver.is_workspace_qualified_plain_identifier("research/note")
+
+
+@pytest.mark.asyncio
+async def test_workspace_identifier_project_detection_requires_workspace_shape(
+    monkeypatch,
+    config_manager,
+):
+    """Two-segment project-relative paths should not trigger workspace discovery."""
+    from basic_memory.services import link_resolver
+
+    def fail_if_called(config):
+        raise AssertionError("plain project-relative paths should skip cloud discovery")
+
+    monkeypatch.setattr(
+        "basic_memory.mcp.project_context._cloud_workspace_discovery_available",
+        fail_if_called,
+    )
+
+    detected = await link_resolver.detect_project_from_workspace_identifier_prefix(
+        "research/note",
+        config_manager.config,
+    )
+
+    assert detected is None
+
+
+@pytest.mark.asyncio
+async def test_workspace_identifier_project_detection_skips_without_discovery(
+    monkeypatch,
+    config_manager,
+):
+    """Workspace-shaped paths stay local when cloud workspace discovery is unavailable."""
+    from basic_memory.services import link_resolver
+
+    monkeypatch.setattr(
+        "basic_memory.mcp.project_context._cloud_workspace_discovery_available",
+        lambda config: False,
+    )
+
+    detected = await link_resolver.detect_project_from_workspace_identifier_prefix(
+        "team-acme/research/note",
+        config_manager.config,
+    )
+
+    assert detected is None
+
+
+@pytest.mark.asyncio
+async def test_workspace_identifier_project_detection_returns_project(
+    monkeypatch,
+    config_manager,
+):
+    """Workspace-qualified plain identifiers should return the resolved project route."""
+    from basic_memory.services import link_resolver
+
+    async def resolve_workspace_identifier(identifier, context=None):
+        assert identifier == "team-acme/research/note"
+        return SimpleNamespace(project_identifier="team-acme/research")
+
+    monkeypatch.setattr(
+        "basic_memory.mcp.project_context._cloud_workspace_discovery_available",
+        lambda config: True,
+    )
+    monkeypatch.setattr(
+        "basic_memory.mcp.project_context.resolve_workspace_qualified_identifier",
+        resolve_workspace_identifier,
+    )
+
+    detected = await link_resolver.detect_project_from_workspace_identifier_prefix(
+        "team-acme/research/note",
+        config_manager.config,
+    )
+
+    assert detected == "team-acme/research"
+
+
 @pytest.mark.asyncio
 async def test_exact_permalink_match(link_resolver, test_entities, project_prefix):
     """Test resolving a link that exactly matches a permalink."""