Skip to content

Commit d7faeb7

Browse files
phernandezclaude
andcommitted
feat: Add destination_folder parameter to move_note tool
Allows callers to move a note into a folder while preserving its original filename — no need for a separate read_note round-trip to extract the basename. - destination_folder is mutually exclusive with destination_path - Rejected for directory moves (is_directory=True) - Uses Path().name and PureWindowsPath().as_posix() for cross-platform compat - Validates resolved path against path traversal attacks - Includes formatting fixes in promo.py and test_analytics.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent deef89a commit d7faeb7

3 files changed

Lines changed: 417 additions & 1 deletion

File tree

src/basic_memory/mcp/tools/move_note.py

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Move note tool for Basic Memory MCP server."""
22

3+
from pathlib import Path, PureWindowsPath
34
from textwrap import dedent
45
from typing import Optional, Literal
56

@@ -345,7 +346,8 @@ def _format_move_error_response(error_message: str, identifier: str, destination
345346
)
346347
async def move_note(
347348
identifier: str,
348-
destination_path: str,
349+
destination_path: str = "",
350+
destination_folder: Optional[str] = None,
349351
is_directory: bool = False,
350352
project: Optional[str] = None,
351353
workspace: Optional[str] = None,
@@ -365,6 +367,9 @@ async def move_note(
365367
Use search_notes() or list_directory() first to find the correct path if uncertain.
366368
destination_path: For files: new path relative to project root (e.g., "work/meetings/note.md")
367369
For directories: new directory path (e.g., "archive/docs")
370+
Mutually exclusive with destination_folder.
371+
destination_folder: Move the note into this folder, preserving the original filename.
372+
Mutually exclusive with destination_path. Only for single-file moves.
368373
is_directory: If True, moves an entire directory and all its contents.
369374
When True, identifier and destination_path should be directory paths
370375
(without file extensions). Defaults to False.
@@ -385,6 +390,9 @@ async def move_note(
385390
# Move by exact permalink
386391
move_note("my-note-permalink", "archive/old-notes/my-note.md")
387392
393+
# Move note to archive folder (filename preserved automatically)
394+
move_note("my-note", destination_folder="archive")
395+
388396
# Move with complex path structure
389397
move_note("experiments/ml-results", "archive/2025/ml-experiments.md")
390398
@@ -415,6 +423,57 @@ async def move_note(
415423
- Re-indexes the entity for search
416424
- Maintains all observations and relations
417425
"""
426+
# --- Parameter Validation ---
427+
# Trigger: both destination_path and destination_folder provided
428+
# Why: they are mutually exclusive — one specifies full path, the other just the folder
429+
# Outcome: early error before any entity resolution or API calls
430+
if destination_folder and destination_path:
431+
error_msg = (
432+
"Cannot specify both destination_path and destination_folder. Use one or the other."
433+
)
434+
if output_format == "json":
435+
return {
436+
"moved": False,
437+
"title": None,
438+
"permalink": None,
439+
"file_path": None,
440+
"source": identifier,
441+
"destination": None,
442+
"error": "MUTUALLY_EXCLUSIVE_PARAMS",
443+
}
444+
return f"# Move Failed - Invalid Parameters\n\n{error_msg}"
445+
446+
if not destination_folder and not destination_path:
447+
error_msg = "Either destination_path or destination_folder must be provided."
448+
if output_format == "json":
449+
return {
450+
"moved": False,
451+
"title": None,
452+
"permalink": None,
453+
"file_path": None,
454+
"source": identifier,
455+
"destination": None,
456+
"error": "MISSING_DESTINATION",
457+
}
458+
return f"# Move Failed - Missing Destination\n\n{error_msg}"
459+
460+
# Trigger: destination_folder used with is_directory=True
461+
# Why: destination_folder preserves a single file's name — meaningless for directory moves
462+
if destination_folder and is_directory:
463+
error_msg = (
464+
"destination_folder is only supported for single-file moves, not directory moves."
465+
)
466+
if output_format == "json":
467+
return {
468+
"moved": False,
469+
"title": None,
470+
"permalink": None,
471+
"file_path": None,
472+
"source": identifier,
473+
"destination": None,
474+
"error": "DESTINATION_FOLDER_NOT_FOR_DIRECTORIES",
475+
}
476+
return f"# Move Failed - Invalid Parameters\n\n{error_msg}"
418477
async with get_project_client(project, workspace, context) as (client, active_project):
419478
logger.debug(
420479
f"Moving {'directory' if is_directory else 'note'}: {identifier} to {destination_path} in project: {active_project.name}"
@@ -589,6 +648,66 @@ async def _ensure_resolved_entity_id() -> str:
589648
# If we can't fetch source metadata, continue with extension defaults.
590649
logger.debug(f"Could not fetch source entity for extension check: {e}")
591650

651+
# --- Resolve destination_folder into destination_path ---
652+
# Trigger: caller passed destination_folder instead of destination_path
653+
# Why: extract the original filename from the resolved entity so callers
654+
# don't need a separate read_note round-trip
655+
# Outcome: destination_path is set to folder/original-filename.ext
656+
if destination_folder is not None:
657+
if source_entity is None:
658+
error_msg = (
659+
f"Could not resolve source entity '{identifier}' to extract filename "
660+
f"for destination_folder. Use destination_path with an explicit filename instead."
661+
)
662+
if output_format == "json":
663+
return {
664+
"moved": False,
665+
"title": None,
666+
"permalink": None,
667+
"file_path": None,
668+
"source": identifier,
669+
"destination": None,
670+
"error": "ENTITY_RESOLUTION_FAILED",
671+
}
672+
return f"# Move Failed - Entity Resolution Failed\n\n{error_msg}"
673+
674+
source_filename = Path(source_entity.file_path).name
675+
# Normalize backslashes to forward slashes for Windows compatibility,
676+
# then strip leading/trailing separators
677+
folder = PureWindowsPath(destination_folder).as_posix().strip("/")
678+
destination_path = f"{folder}/{source_filename}" if folder else source_filename
679+
680+
# Validate resolved path to prevent path traversal via destination_folder
681+
if not validate_project_path(destination_path, project_path):
682+
logger.warning(
683+
"Attempted path traversal attack blocked via destination_folder",
684+
destination_folder=destination_folder,
685+
project=active_project.name,
686+
)
687+
if output_format == "json":
688+
return {
689+
"moved": False,
690+
"title": None,
691+
"permalink": None,
692+
"file_path": None,
693+
"source": identifier,
694+
"destination": destination_path,
695+
"error": "SECURITY_VALIDATION_ERROR",
696+
}
697+
return f"""# Move Failed - Security Validation Error
698+
699+
The destination folder '{destination_folder}' is not allowed - paths must stay within project boundaries.
700+
701+
## Valid folder examples:
702+
- `notes`
703+
- `projects/2025`
704+
- `archive/old-notes`
705+
706+
## Try again with a safe folder:
707+
```
708+
move_note("{identifier}", destination_folder="notes")
709+
```"""
710+
592711
# Validate that destination path includes a file extension
593712
if "." not in destination_path or not destination_path.split(".")[-1]:
594713
logger.warning(f"Move failed - no file extension provided: {destination_path}")

test-int/mcp/test_move_note_integration.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,3 +639,80 @@ async def test_move_note_normal_moves_still_work(mcp_server, app, test_project):
639639

640640
content = read_result.content[0].text
641641
assert "This should move normally" in content
642+
643+
644+
@pytest.mark.asyncio
645+
async def test_move_note_with_destination_folder(mcp_server, app, test_project):
646+
"""Test moving a note using destination_folder to preserve the original filename."""
647+
648+
async with Client(mcp_server) as client:
649+
# Create a note to move
650+
await client.call_tool(
651+
"write_note",
652+
{
653+
"project": test_project.name,
654+
"title": "Folder Move Integration",
655+
"directory": "source",
656+
"content": "# Folder Move Integration\n\nTesting destination_folder parameter.",
657+
"tags": "test,folder-move",
658+
},
659+
)
660+
661+
# Move using destination_folder (filename preserved automatically)
662+
move_result = await client.call_tool(
663+
"move_note",
664+
{
665+
"project": test_project.name,
666+
"identifier": "Folder Move Integration",
667+
"destination_folder": "archive/2025",
668+
},
669+
)
670+
671+
# Should return successful move message
672+
assert len(move_result.content) == 1
673+
move_text = move_result.content[0].text
674+
assert "✅ Note moved successfully" in move_text
675+
assert "Folder Move Integration" in move_text
676+
677+
# Verify the note can be read from its new location (original filename preserved)
678+
read_result = await client.call_tool(
679+
"read_note",
680+
{
681+
"project": test_project.name,
682+
"identifier": "archive/2025/folder-move-integration",
683+
},
684+
)
685+
686+
content = read_result.content[0].text
687+
assert "Testing destination_folder parameter" in content
688+
689+
# Verify the original location no longer works
690+
read_original = await client.call_tool(
691+
"read_note",
692+
{
693+
"project": test_project.name,
694+
"identifier": "source/folder-move-integration",
695+
},
696+
)
697+
assert "Note Not Found" in read_original.content[0].text
698+
699+
700+
@pytest.mark.asyncio
701+
async def test_move_note_destination_folder_mutually_exclusive(mcp_server, app, test_project):
702+
"""Test that providing both destination_path and destination_folder returns an error."""
703+
704+
async with Client(mcp_server) as client:
705+
move_result = await client.call_tool(
706+
"move_note",
707+
{
708+
"project": test_project.name,
709+
"identifier": "some-note",
710+
"destination_path": "target/note.md",
711+
"destination_folder": "target",
712+
},
713+
)
714+
715+
assert len(move_result.content) == 1
716+
error_text = move_result.content[0].text
717+
assert "# Move Failed - Invalid Parameters" in error_text
718+
assert "Cannot specify both" in error_text

0 commit comments

Comments
 (0)