Skip to content

Commit 3ba3a95

Browse files
phernandezclaude
andauthored
fix(mcp): stop move_note reporting false success across boundaries (#904)
Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1667cdc commit 3ba3a95

3 files changed

Lines changed: 467 additions & 26 deletions

File tree

src/basic_memory/mcp/tools/move_note.py

Lines changed: 103 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,44 @@ async def _detect_cross_project_move_attempt(
3737
project_list = await project_client.list_projects()
3838
project_names = [p.name.lower() for p in project_list.projects]
3939

40-
# Check if destination path contains any project names
4140
dest_lower = destination_path.lower()
42-
path_parts = dest_lower.split("/")
43-
44-
# Look for project names in the destination path
45-
for part in path_parts:
46-
if part in project_names and part != current_project.lower():
47-
# Found a different project name in the path
41+
path_parts = [part for part in dest_lower.split("/") if part]
42+
43+
# --- Detection 1: leading segment is a known project name ---
44+
# Trigger: the first path segment matches a different project's name.
45+
# Why: a routing-style destination like "other-project/file.md" expresses an
46+
# intent to move into another project, which move_note cannot do — it
47+
# would silently create a same-project nested folder instead.
48+
# Outcome: reject with cross-project guidance rather than fake success.
49+
if path_parts:
50+
leading = path_parts[0]
51+
if leading in project_names and leading != current_project.lower():
4852
matching_project = next(
49-
p.name for p in project_list.projects if p.name.lower() == part
53+
p.name for p in project_list.projects if p.name.lower() == leading
5054
)
5155
return _format_cross_project_error_response(
5256
identifier, destination_path, current_project, matching_project
5357
)
5458

59+
# --- Detection 2: workspace/projects/<x> shaped destination ---
60+
# Trigger: the destination has the exact cloud workspace layout
61+
# "<workspace>/projects/<x>/..." — a single leading workspace segment,
62+
# then literal "projects", then at least one more segment.
63+
# Why: this is the canonical cross-workspace routing shape from #881. It slips
64+
# past name-based detection because the workspace/project names need not
65+
# match any locally known project.
66+
# Outcome: reject with cross-project guidance. The check is pinned to index 1 so
67+
# legitimate nested folders that merely contain a "projects" segment
68+
# (e.g. "notes/projects/my-project/note.md" or a top-level
69+
# "projects/2025/...") are NOT flagged as cross-project moves.
70+
# In "<workspace>/projects/<x>/...", <x> (index 2) is the project being targeted,
71+
# so report that as the target project — not the workspace at index 0 — so the
72+
# guidance points the user at the real project name to write into.
73+
if len(path_parts) >= 3 and path_parts[1] == "projects":
74+
return _format_cross_project_error_response(
75+
identifier, destination_path, current_project, path_parts[2]
76+
)
77+
5578
# No other cross-project patterns detected
5679

5780
except Exception as e:
@@ -633,24 +656,6 @@ async def move_note(
633656
move_note("path/to/file.md", "{destination_path}/file.md")
634657
```"""
635658

636-
# Check for potential cross-project move attempts (file moves only)
637-
cross_project_error = await _detect_cross_project_move_attempt(
638-
client, identifier, destination_path, active_project.name
639-
)
640-
if cross_project_error:
641-
logger.info(f"Detected cross-project move attempt: {identifier} -> {destination_path}")
642-
if output_format == "json":
643-
return {
644-
"moved": False,
645-
"title": None,
646-
"permalink": None,
647-
"file_path": None,
648-
"source": identifier,
649-
"destination": destination_path,
650-
"error": "CROSS_PROJECT_MOVE_NOT_SUPPORTED",
651-
}
652-
return cross_project_error
653-
654659
# Import here to avoid circular import
655660
from basic_memory.mcp.clients import KnowledgeClient
656661

@@ -756,6 +761,31 @@ async def _ensure_resolved_entity_id() -> str:
756761
move_note("{identifier}", destination_folder="notes")
757762
```"""
758763

764+
# --- Cross-boundary intent guard (file moves only) ---
765+
# Trigger: destination_path now holds the real combined target, whether it came
766+
# from destination_path or was resolved from destination_folder above.
767+
# Why: detection must run AFTER folder resolution — running it earlier (when a
768+
# caller used destination_folder) saw an empty destination_path and skipped
769+
# entirely (#881 Gap 3).
770+
# Outcome: a cross-workspace/cross-project routing destination is rejected with
771+
# guidance instead of silently degrading to a same-project nested folder.
772+
cross_project_error = await _detect_cross_project_move_attempt(
773+
client, identifier, destination_path, active_project.name
774+
)
775+
if cross_project_error:
776+
logger.info(f"Detected cross-project move attempt: {identifier} -> {destination_path}")
777+
if output_format == "json":
778+
return {
779+
"moved": False,
780+
"title": None,
781+
"permalink": None,
782+
"file_path": None,
783+
"source": identifier,
784+
"destination": destination_path,
785+
"error": "CROSS_PROJECT_MOVE_NOT_SUPPORTED",
786+
}
787+
return cross_project_error
788+
759789
# Validate that destination path includes a file extension
760790
if "." not in destination_path or not destination_path.split(".")[-1]:
761791
logger.warning(f"Move failed - no file extension provided: {destination_path}")
@@ -839,6 +869,53 @@ async def _ensure_resolved_entity_id() -> str:
839869

840870
# Call the move API using KnowledgeClient
841871
result = await knowledge_client.move_entity(resolved_entity_id, destination_path)
872+
873+
# --- Outcome validation (honest success backstop) ---
874+
# Trigger: the resulting file_path differs from the destination the caller
875+
# requested.
876+
# Why: move_entity stores the path relative to the *current* project root, so
877+
# a cross-boundary intent silently degrades into a same-project nested
878+
# folder. The old code reported "✅ moved successfully" regardless,
879+
# misleading the agent (#881 Gap 2). This is the robust backstop behind
880+
# the up-front detection: any divergence the caller did not ask for
881+
# surfaces as a failure rather than a fake success.
882+
# Outcome: report failure with the actual landing path instead of "✅".
883+
normalized_requested = PureWindowsPath(destination_path).as_posix().strip("/")
884+
normalized_actual = PureWindowsPath(result.file_path).as_posix().strip("/")
885+
if normalized_actual != normalized_requested:
886+
logger.warning(
887+
f"Move outcome diverged from intent: requested={destination_path} "
888+
f"actual={result.file_path}"
889+
)
890+
if output_format == "json":
891+
return {
892+
"moved": False,
893+
"title": result.title,
894+
"permalink": result.permalink,
895+
"file_path": result.file_path,
896+
"source": identifier,
897+
"destination": destination_path,
898+
"error": "MOVE_OUTCOME_MISMATCH",
899+
}
900+
return dedent(f"""
901+
# Move Failed - Unexpected Result Location
902+
903+
The move of '{identifier}' did not land at the requested destination.
904+
905+
**Requested:** `{destination_path}`
906+
**Actual:** `{result.file_path}`
907+
908+
This usually means the destination referenced a different
909+
workspace/project, which move_note cannot do — notes can only be
910+
moved within the same project. To move content between projects:
911+
912+
```
913+
read_note("{identifier}")
914+
write_note("Title", "content", "folder", project="target-project")
915+
delete_note("{identifier}", project="{active_project.name}")
916+
```
917+
""").strip()
918+
842919
if output_format == "json":
843920
return {
844921
"moved": True,

0 commit comments

Comments
 (0)