Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions src/basic_memory/cli/commands/tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from basic_memory.cli.commands.command_utils import run_with_cleanup
from basic_memory.cli.commands.routing import force_routing, validate_routing_flags
from basic_memory.mcp.tools import build_context as mcp_build_context
from basic_memory.mcp.tools import delete_note as mcp_delete_note
from basic_memory.mcp.tools import edit_note as mcp_edit_note
from basic_memory.mcp.tools import list_memory_projects as mcp_list_projects
from basic_memory.mcp.tools import list_workspaces as mcp_list_workspaces
Expand All @@ -40,6 +41,26 @@ def _print_json(result: Any) -> None:
print(json.dumps(result, indent=2, ensure_ascii=True, default=str))


def _delete_note_failure_message(result: dict[str, Any]) -> str | None:
"""Return the CLI failure message for delete-note JSON results, if any."""
error = result.get("error")
if error:
return str(error)

failed_deletes = result.get("failed_deletes")
# Trigger: directory deletion can partially fail without raising from the service.
# Why: cleanup scripts need a non-zero exit when files remain undeleted.
# Outcome: the CLI fails even if older MCP JSON did not include an error field.
if (
result.get("is_directory") is True
and isinstance(failed_deletes, int)
and failed_deletes > 0
):
return f"Directory delete incomplete: {failed_deletes} file(s) failed"

return None


# --- Commands ---


Expand Down Expand Up @@ -178,6 +199,66 @@ def read_note(
raise


@tool_app.command("delete-note")
def delete_note(
identifier: str,
is_directory: bool = typer.Option(
False, "--is-directory", help="Delete a directory instead of a single note"
),
project: Annotated[
Optional[str],
typer.Option(help="The project to use. If not provided, the default project will be used."),
] = None,
project_id: Annotated[
Optional[str],
typer.Option(
"--project-id",
help="Project external_id (UUID). Takes precedence over --project; use to disambiguate same-named projects across cloud workspaces.",
),
] = None,
local: bool = typer.Option(
False, "--local", help="Force local API routing (ignore cloud mode)"
),
cloud: bool = typer.Option(False, "--cloud", help="Force cloud API routing"),
) -> None:
"""Delete a note or directory from the knowledge base.

Examples:

bm tool delete-note notes/old-draft
bm tool delete-note docs/archive --is-directory
"""
try:
validate_routing_flags(local, cloud)

with force_routing(local=local, cloud=cloud):
result = run_with_cleanup(
mcp_delete_note(
identifier=identifier,
is_directory=is_directory,
project=project,
project_id=project_id,
output_format="json",
)
)

if isinstance(result, dict):
failure_message = _delete_note_failure_message(result)
if failure_message:
typer.echo(f"Error: {failure_message}", err=True)
raise typer.Exit(1)

_print_json(result)
except ValueError as e:
typer.echo(f"Error: {e}", err=True)
raise typer.Exit(1)
except Exception as e: # pragma: no cover
if not isinstance(e, typer.Exit):
typer.echo(f"Error during delete_note: {e}", err=True)
raise typer.Exit(1)
raise


@tool_app.command()
def edit_note(
identifier: str,
Expand Down
10 changes: 9 additions & 1 deletion src/basic_memory/mcp/tools/delete_note.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,22 @@ async def delete_note(
)
result = await knowledge_client.delete_directory(directory_identifier)
if output_format == "json":
return {
response = {
"deleted": result.failed_deletes == 0,
"is_directory": True,
"identifier": identifier,
"total_files": result.total_files,
"successful_deletes": result.successful_deletes,
"failed_deletes": result.failed_deletes,
"deleted_files": result.deleted_files,
"errors": [error.model_dump() for error in result.errors],
}
if result.failed_deletes > 0:
response["error"] = (
"Directory delete incomplete: "
f"{result.failed_deletes} of {result.total_files} file(s) failed"
)
return response

# Build success message for directory delete
result_lines = [
Expand Down
147 changes: 147 additions & 0 deletions tests/cli/test_cli_tool_json_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@
"operation": "append",
}

DELETE_NOTE_RESULT = {
"deleted": True,
"is_directory": False,
"title": "Test Note",
"permalink": "notes/test-note",
"file_path": "notes/Test Note.md",
}

DELETE_DIRECTORY_RESULT = {
"deleted": True,
"is_directory": True,
"directory": "notes/archive",
"deleted_count": 3,
}

BUILD_CONTEXT_RESULT = {
"results": [],
"metadata": {"uri": "test/topic", "depth": 1},
Expand Down Expand Up @@ -215,6 +230,138 @@ def test_read_note_include_frontmatter(mock_mcp_read):
assert mock_mcp_read.call_args.kwargs["include_frontmatter"] is True


# --- delete-note ---


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value=DELETE_NOTE_RESULT,
)
def test_delete_note_json_output(mock_mcp_delete: AsyncMock) -> None:
"""delete-note outputs valid JSON from MCP tool."""
result = runner.invoke(
cli_app,
["tool", "delete-note", "test-note"],
)

assert result.exit_code == 0, f"CLI failed: {result.output}"
data = json.loads(result.output)
assert data["deleted"] is True
assert data["permalink"] == "notes/test-note"
mock_mcp_delete.assert_called_once()
assert mock_mcp_delete.call_args.kwargs["output_format"] == "json"
assert mock_mcp_delete.call_args.kwargs["is_directory"] is False


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value=DELETE_DIRECTORY_RESULT,
)
def test_delete_note_directory_flag(mock_mcp_delete: AsyncMock) -> None:
"""delete-note --is-directory passes directory mode to MCP."""
result = runner.invoke(
cli_app,
["tool", "delete-note", "notes/archive", "--is-directory"],
)

assert result.exit_code == 0, f"CLI failed: {result.output}"
data = json.loads(result.output)
assert data["is_directory"] is True
assert mock_mcp_delete.call_args.kwargs["is_directory"] is True


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value={
"deleted": False,
"is_directory": False,
"identifier": "missing-note",
"error": None,
},
)
def test_delete_note_not_found_outputs_json(mock_mcp_delete: AsyncMock) -> None:
"""delete-note treats not-found JSON without an error as a successful command."""
result = runner.invoke(
cli_app,
["tool", "delete-note", "missing-note"],
)

assert result.exit_code == 0, f"CLI failed: {result.output}"
data = json.loads(result.output)
assert data["deleted"] is False
assert data["identifier"] == "missing-note"
assert mock_mcp_delete.call_args.kwargs["output_format"] == "json"


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value={
"deleted": False,
"is_directory": False,
"identifier": "test-note",
"error": "Delete failed",
},
)
def test_delete_note_error_response(mock_mcp_delete: AsyncMock) -> None:
"""delete-note exits with code 1 when MCP tool returns an error field."""
result = runner.invoke(
cli_app,
["tool", "delete-note", "test-note"],
)

assert result.exit_code == 1
assert "Error: Delete failed" in result.output
assert mock_mcp_delete.call_args.kwargs["output_format"] == "json"


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value={
"deleted": False,
"is_directory": True,
"identifier": "notes/archive",
"total_files": 3,
"successful_deletes": 2,
"failed_deletes": 1,
"errors": [{"path": "notes/archive/locked.md", "error": "permission denied"}],
},
)
def test_delete_note_directory_partial_failure_exits_nonzero(
mock_mcp_delete: AsyncMock,
) -> None:
"""delete-note --is-directory exits 1 when any directory file remains undeleted."""
result = runner.invoke(
cli_app,
["tool", "delete-note", "notes/archive", "--is-directory"],
)

assert result.exit_code == 1
assert "Error: Directory delete incomplete: 1 file(s) failed" in result.output
assert mock_mcp_delete.call_args.kwargs["output_format"] == "json"


@patch(
"basic_memory.cli.commands.tool.mcp_delete_note",
new_callable=AsyncMock,
return_value=DELETE_NOTE_RESULT,
)
def test_delete_note_project_id_passthrough(mock_mcp_delete: AsyncMock) -> None:
"""--project-id forwards to the MCP tool's project_id parameter."""
uuid = "11111111-1111-1111-1111-111111111111"
result = runner.invoke(
cli_app,
["tool", "delete-note", "test-note", "--project-id", uuid],
)

assert result.exit_code == 0, f"CLI failed: {result.output}"
assert mock_mcp_delete.call_args.kwargs["project_id"] == uuid


# --- edit-note ---


Expand Down
35 changes: 35 additions & 0 deletions tests/mcp/test_tool_json_output_modes.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
recent_activity,
write_note,
)
from basic_memory.schemas.response import DirectoryDeleteError, DirectoryDeleteResult


@pytest.mark.asyncio
Expand Down Expand Up @@ -326,6 +327,40 @@ async def mock_delete_directory(self, directory: str):
assert "simulated directory delete failure" in json_delete["error"]


@pytest.mark.asyncio
async def test_delete_directory_json_mode_reports_partial_delete_failure(
app, test_project, monkeypatch
):
async def mock_delete_directory(self, directory: str):
return DirectoryDeleteResult(
total_files=2,
successful_deletes=1,
failed_deletes=1,
deleted_files=["mode-tests/deleted.md"],
errors=[
DirectoryDeleteError(
path="mode-tests/locked.md",
error="permission denied",
)
],
)

monkeypatch.setattr(KnowledgeClient, "delete_directory", mock_delete_directory)

json_delete = await delete_note(
identifier="mode-tests",
is_directory=True,
project=test_project.name,
output_format="json",
)
assert isinstance(json_delete, dict)
assert json_delete["deleted"] is False
assert json_delete["failed_deletes"] == 1
assert json_delete["deleted_files"] == ["mode-tests/deleted.md"]
assert json_delete["errors"] == [{"path": "mode-tests/locked.md", "error": "permission denied"}]
assert "Directory delete incomplete" in json_delete["error"]


@pytest.mark.asyncio
async def test_move_note_text_and_json_modes(app, test_project):
await write_note(
Expand Down