Skip to content

Commit 5bc02c9

Browse files
committed
fix(mcp): honor workspace canonical edge cases
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 4861331 commit 5bc02c9

7 files changed

Lines changed: 194 additions & 17 deletions

File tree

src/basic_memory/mcp/tools/delete_note.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ def _format_delete_error_response(project: str, error_message: str, identifier:
155155
If the note should be deleted but the operation keeps failing, send a message to support@basicmemory.com."""
156156

157157

158-
def _directory_path_for_delete(target_identifier: str, active_project: ProjectItem) -> str:
158+
def _directory_path_for_delete(
159+
target_identifier: str,
160+
active_project: ProjectItem,
161+
*,
162+
include_project_prefix: bool,
163+
) -> str:
159164
"""Return the project-relative directory path expected by the delete API."""
160165
directory = normalize_project_reference(target_identifier).strip("/")
161166
project_permalink = active_project.permalink
@@ -166,7 +171,8 @@ def _directory_path_for_delete(target_identifier: str, active_project: ProjectIt
166171
route_prefixes.append(
167172
f"{generate_permalink(workspace_context.workspace_slug)}/{project_permalink}"
168173
)
169-
route_prefixes.append(project_permalink)
174+
if include_project_prefix:
175+
route_prefixes.append(project_permalink)
170176

171177
for route_prefix in route_prefixes:
172178
if directory.startswith(f"{route_prefix}/"):
@@ -299,7 +305,11 @@ async def delete_note(
299305
# delete_directory filters by project-relative file_path prefixes.
300306
# Outcome: strip only the route prefix before calling the delete API.
301307
directory_identifier = (
302-
_directory_path_for_delete(target_identifier, active_project)
308+
_directory_path_for_delete(
309+
target_identifier,
310+
active_project,
311+
include_project_prefix=ConfigManager().config.permalinks_include_project,
312+
)
303313
if is_memory_url
304314
else target_identifier
305315
)

src/basic_memory/services/entity_service.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,15 @@ async def resolve_permalink(
206206
if self.app_config:
207207
include_project = self.app_config.permalinks_include_project
208208

209+
workspace_permalink = workspace_slug_for_canonical_permalinks()
209210
project_permalink = None
210-
# Trigger: project-prefixed permalinks are enabled
211-
# Why: we need the project slug to build the canonical permalink
212-
# Outcome: fetch and cache the project's permalink
213-
if include_project:
211+
# Trigger: project-prefixed permalinks are enabled, or organization workspace
212+
# context requires a complete workspace/project canonical permalink.
213+
# Why: project slug is the stable middle segment for globally addressable links.
214+
# Outcome: fetch and cache the project's permalink before building the canonical URL.
215+
if include_project or workspace_permalink:
214216
project_permalink = await self._get_project_permalink()
215217

216-
workspace_permalink = workspace_slug_for_canonical_permalinks()
217218
desired_permalink = build_canonical_permalink(
218219
project_permalink,
219220
file_path_str,

src/basic_memory/utils.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -240,30 +240,40 @@ def build_canonical_permalink(
240240
Canonical permalink string.
241241
"""
242242
normalized_path = generate_permalink(file_path)
243-
244-
if not include_project or not project_permalink:
245-
return normalized_path
246-
247-
normalized_project = generate_permalink(project_permalink)
248243
normalized_workspace = generate_permalink(workspace_permalink) if workspace_permalink else None
244+
249245
if normalized_workspace:
246+
if not project_permalink:
247+
raise ValueError("workspace_permalink requires project_permalink")
248+
249+
normalized_project = generate_permalink(project_permalink)
250250
workspace_project_prefix = f"{normalized_workspace}/{normalized_project}"
251251
if normalized_path == workspace_project_prefix or normalized_path.startswith(
252252
f"{workspace_project_prefix}/"
253253
):
254254
return normalized_path
255255

256+
if normalized_path == normalized_project or normalized_path.startswith(
257+
f"{normalized_project}/"
258+
):
259+
project_path = normalized_path
260+
else:
261+
project_path = f"{normalized_project}/{normalized_path}"
262+
263+
return f"{normalized_workspace}/{project_path}"
264+
265+
if not include_project or not project_permalink:
266+
return normalized_path
267+
268+
normalized_project = generate_permalink(project_permalink)
256269
if normalized_path == normalized_project or normalized_path.startswith(
257270
f"{normalized_project}/"
258271
):
259272
project_path = normalized_path
260273
else:
261274
project_path = f"{normalized_project}/{normalized_path}"
262275

263-
if not workspace_permalink:
264-
return project_path
265-
266-
return f"{normalized_workspace}/{project_path}"
276+
return project_path
267277

268278

269279
def setup_logging(

tests/mcp/test_project_context.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,48 @@ async def fake_index(context=None):
829829
assert resolved.canonical_path == "main/notes/foo"
830830

831831

832+
@pytest.mark.asyncio
833+
async def test_resolve_workspace_qualified_memory_url_keeps_org_canonical_path_without_project_prefix(
834+
config_manager,
835+
monkeypatch,
836+
):
837+
import basic_memory.mcp.project_context as project_context
838+
from basic_memory.mcp.project_context import (
839+
WorkspaceProjectEntry,
840+
_build_workspace_project_index,
841+
resolve_workspace_qualified_memory_url,
842+
)
843+
844+
config = config_manager.load_config()
845+
config.permalinks_include_project = False
846+
config_manager.save_config(config)
847+
848+
team = _workspace(
849+
tenant_id="team-tenant",
850+
workspace_type="organization",
851+
slug="team-paul",
852+
name="Team Paul",
853+
role="editor",
854+
)
855+
entries = (
856+
WorkspaceProjectEntry(
857+
workspace=team,
858+
project=_project("main", id=1, external_id="team-main-id"),
859+
),
860+
)
861+
index = _build_workspace_project_index((team,), entries)
862+
863+
async def fake_index(context=None):
864+
return index
865+
866+
monkeypatch.setattr(project_context, "_ensure_workspace_project_index", fake_index)
867+
868+
resolved = await resolve_workspace_qualified_memory_url("memory://team-paul/main/notes/foo")
869+
870+
assert resolved is not None
871+
assert resolved.canonical_path == "team-paul/main/notes/foo"
872+
873+
832874
@pytest.mark.asyncio
833875
async def test_get_project_client_routes_duplicate_project_through_workspace_slug(
834876
config_manager,

tests/mcp/test_tool_delete_note.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,50 @@ async def test_delete_directory_memory_url_strips_project_prefix(client, test_pr
205205
assert result["failed_deletes"] == 0
206206

207207

208+
@pytest.mark.asyncio
209+
async def test_delete_directory_memory_url_keeps_real_project_prefix_when_prefixes_disabled(
210+
client,
211+
test_project,
212+
config_manager,
213+
):
214+
"""When project prefixes are disabled, resolved directory paths are already project-relative."""
215+
config = config_manager.load_config()
216+
config.permalinks_include_project = False
217+
config_manager.save_config(config)
218+
219+
real_directory = f"{test_project.name}/archive"
220+
await write_note(
221+
project=test_project.name,
222+
title="Delete Directory Real Project Prefix",
223+
directory=real_directory,
224+
content="# Delete Directory Real Project Prefix\nDelete target content.",
225+
)
226+
await write_note(
227+
project=test_project.name,
228+
title="Keep Directory Decoy",
229+
directory="archive",
230+
content="# Keep Directory Decoy\nDecoy content.",
231+
)
232+
233+
result = await delete_note(
234+
identifier=f"memory://{test_project.name}/{real_directory}",
235+
is_directory=True,
236+
project=test_project.name,
237+
output_format="json",
238+
)
239+
240+
assert isinstance(result, dict)
241+
assert result["deleted"] is True
242+
assert result["total_files"] == 1
243+
assert result["successful_deletes"] == 1
244+
assert result["failed_deletes"] == 0
245+
246+
deleted = await read_note("Delete Directory Real Project Prefix", project=test_project.name)
247+
assert "Delete target content" not in deleted
248+
decoy = await read_note("Keep Directory Decoy", project=test_project.name)
249+
assert "Decoy content" in decoy
250+
251+
208252
@pytest.mark.asyncio
209253
async def test_delete_directory_workspace_memory_url_strips_route_prefix(client, test_project):
210254
"""Workspace-qualified memory:// directory deletes still hit project-relative files."""

tests/mcp/test_tool_read_note.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,46 @@ async def test_team_workspace_write_stores_complete_canonical_permalink(
328328
assert read_result["content"].strip() == "Team canonical content"
329329

330330

331+
@pytest.mark.asyncio
332+
async def test_team_workspace_write_stores_complete_permalink_when_project_prefixes_disabled(
333+
app,
334+
test_project,
335+
entity_repository,
336+
config_manager,
337+
):
338+
from basic_memory.workspace_context import workspace_permalink_context
339+
340+
config = config_manager.load_config()
341+
config.permalinks_include_project = False
342+
config_manager.save_config(config)
343+
344+
expected_permalink = f"team-paul/{test_project.name}/team/team-no-project-prefix-note"
345+
346+
with workspace_permalink_context(workspace_slug="team-paul", workspace_type="organization"):
347+
write_result = await write_note(
348+
project=test_project.name,
349+
title="Team No Project Prefix Note",
350+
directory="team",
351+
content="Team canonical content without project-prefix config",
352+
)
353+
354+
assert f"permalink: {expected_permalink}" in write_result
355+
356+
stored = await entity_repository.get_by_permalink(expected_permalink)
357+
assert stored is not None
358+
assert stored.permalink == expected_permalink
359+
360+
read_result = await read_note(
361+
f"memory://{expected_permalink}",
362+
project=test_project.name,
363+
output_format="json",
364+
)
365+
366+
assert isinstance(read_result, dict)
367+
assert read_result["permalink"] == expected_permalink
368+
assert read_result["content"].strip() == "Team canonical content without project-prefix config"
369+
370+
331371
@pytest.mark.asyncio
332372
async def test_personal_workspace_write_keeps_project_scoped_permalink(
333373
app,

tests/utils/test_permalink_formatting.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from basic_memory.services import EntityService
99
from basic_memory.sync.sync_service import SyncService
1010
from basic_memory.utils import build_canonical_permalink, generate_permalink
11+
from basic_memory.workspace_context import workspace_permalink_context
1112

1213

1314
async def create_test_file(path: Path, content: str = "test content") -> None:
@@ -148,3 +149,32 @@ def test_build_canonical_permalink_preserves_complete_workspace_prefix():
148149
)
149150
== "team-paul/main/notes/foo"
150151
)
152+
153+
154+
def test_build_canonical_permalink_workspace_prefix_ignores_project_prefix_flag():
155+
"""Organization workspace canonical permalinks always include workspace and project."""
156+
assert (
157+
build_canonical_permalink(
158+
"main",
159+
"notes/foo.md",
160+
include_project=False,
161+
workspace_permalink="team-paul",
162+
)
163+
== "team-paul/main/notes/foo"
164+
)
165+
166+
167+
@pytest.mark.asyncio
168+
async def test_entity_service_workspace_permalink_uses_project_when_prefixes_disabled(
169+
entity_service: EntityService,
170+
project_config: ProjectConfig,
171+
):
172+
"""Workspace note creation uses complete canonical shape even without local project prefixes."""
173+
assert entity_service.app_config is not None
174+
entity_service.app_config.permalinks_include_project = False
175+
176+
with workspace_permalink_context("team-paul", "organization"):
177+
permalink = await entity_service.resolve_permalink("team/no-project-prefix-service.md")
178+
179+
project_permalink = generate_permalink(project_config.name)
180+
assert permalink == f"team-paul/{project_permalink}/team/no-project-prefix-service"

0 commit comments

Comments
 (0)