diff --git a/src/basic_memory/cli/commands/tool.py b/src/basic_memory/cli/commands/tool.py index 0eb96aa4..db901ef6 100644 --- a/src/basic_memory/cli/commands/tool.py +++ b/src/basic_memory/cli/commands/tool.py @@ -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 @@ -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 --- @@ -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, diff --git a/src/basic_memory/mcp/tools/delete_note.py b/src/basic_memory/mcp/tools/delete_note.py index b7be9354..348d4957 100644 --- a/src/basic_memory/mcp/tools/delete_note.py +++ b/src/basic_memory/mcp/tools/delete_note.py @@ -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 = [ diff --git a/tests/cli/test_cli_tool_json_output.py b/tests/cli/test_cli_tool_json_output.py index d5e5e183..f32c7dfc 100644 --- a/tests/cli/test_cli_tool_json_output.py +++ b/tests/cli/test_cli_tool_json_output.py @@ -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}, @@ -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 --- diff --git a/tests/mcp/test_tool_json_output_modes.py b/tests/mcp/test_tool_json_output_modes.py index 069534b5..06960aac 100644 --- a/tests/mcp/test_tool_json_output_modes.py +++ b/tests/mcp/test_tool_json_output_modes.py @@ -18,6 +18,7 @@ recent_activity, write_note, ) +from basic_memory.schemas.response import DirectoryDeleteError, DirectoryDeleteResult @pytest.mark.asyncio @@ -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(