fix(cli): align bm tool commands with MCP (error exits, overwrite, category, default)

Fixes confirmed CLI/MCP parity bugs found by the integration-test bug hunt:

- #1 / #5: `bm tool write-note` exited 0 on an error/conflict JSON result
  (e.g. NOTE_ALREADY_EXISTS). Added the same error-field guard the sibling
  commands (delete-note/edit-note/search-notes) use so a blocked/failed write
  exits non-zero. The conflict JSON is still printed to stdout for tooling.
- #6: `bm tool read-note` exited 0 on a SECURITY_VALIDATION_ERROR
  (path traversal). Added the same error-field guard; genuine not-found
  (null fields, no error key) still exits 0.
- #2: `bm tool write-note` had no --overwrite flag though MCP write_note
  supports overwrite=True. Added --overwrite and forward it to the tool.
- #4: `bm tool search-notes` lacked a --category filter though MCP
  search_notes has a categories param. Added repeatable --category and
  forward categories= to the tool.
- #3: `bm tool recent-activity` default --page-size was 50; changed to 10
  to match the MCP recent_activity default and the other CLI tool commands.

All five fixes live in src/basic_memory/cli/commands/tool.py. Regression
tests added under test-int/bughunt_fixes/ come from the integration bug hunt
(real CliRunner -> CLI -> MCP tool -> ASGI API -> DB/files, no mocks).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

src/basic_memory/cli/commands/tool.py

@@ -100,6 +100,11 @@ def write_note(
             help="Project external_id (UUID). Takes precedence over --project; use to disambiguate same-named projects across cloud workspaces.",
         ),
     ] = None,
+    overwrite: bool = typer.Option(
+        False,
+        "--overwrite",
+        help="Replace an existing note on conflict (matches MCP write_note overwrite=True)",
+    ),
     local: bool = typer.Option(
         False, "--local", help="Force local API routing (ignore cloud mode)"
     ),
@@ -112,6 +117,7 @@ def write_note(
     bm tool write-note --title "My Note" --folder "notes" --content "Note content"
     bm tool write-note --title "My Guide" --folder "notes" --content "..." --type guide
     echo "content" | bm tool write-note --title "My Note" --folder "notes"
+    bm tool write-note --title "My Note" --folder "notes" --overwrite
     bm tool write-note --title "My Note" --folder "notes" --local
     """
     try:
@@ -144,9 +150,22 @@ def write_note(
                     project_id=project_id,
                     tags=tags,
                     note_type=note_type,
+                    overwrite=overwrite,
                     output_format="json",
                 )
             )
+
+        # MCP tool returns an error field on failure in JSON mode (e.g.
+        # NOTE_ALREADY_EXISTS on a blocked overwrite, SECURITY_VALIDATION_ERROR).
+        # Trigger: result carries a non-empty `error`.
+        # Why: parity with delete-note/edit-note/search-notes so exit-code-driven
+        #      scripts detect a failed/blocked write instead of seeing exit 0.
+        # Outcome: print the error to stderr and exit non-zero.
+        if isinstance(result, dict) and result.get("error"):
+            typer.echo(f"Error: {result['error']}", err=True)
+            _print_json(result)
+            raise typer.Exit(1)
+
         _print_json(result)
     except ValueError as e:
         typer.echo(f"Error: {e}", err=True)
@@ -200,6 +219,19 @@ def read_note(
                     output_format="json",
                 )
             )
+
+        # MCP tool returns an error field on failure in JSON mode (e.g.
+        # SECURITY_VALIDATION_ERROR on a path-traversal identifier). A genuine
+        # not-found returns null fields with no `error` key, so it still exits 0.
+        # Trigger: result carries a non-empty `error`.
+        # Why: parity with edit-note/delete-note/search-notes so a blocked read
+        #      surfaces a non-zero exit instead of looking like success.
+        # Outcome: print the error to stderr and exit non-zero.
+        if isinstance(result, dict) and result.get("error"):
+            typer.echo(f"Error: {result['error']}", err=True)
+            _print_json(result)
+            raise typer.Exit(1)
+
         _print_json(result)
     except ValueError as e:
         typer.echo(f"Error: {e}", err=True)
@@ -417,7 +449,9 @@ def recent_activity(
         "7d", "--timeframe", help="Timeframe filter (e.g., '7d', '1 week')"
     ),
     page: int = typer.Option(1, "--page", help="Page number for pagination"),
-    page_size: int = typer.Option(50, "--page-size", help="Number of results per page"),
+    # Match the MCP recent_activity default (page_size=10) so identical default
+    # invocations return the same number of rows from CLI and MCP.
+    page_size: int = typer.Option(10, "--page-size", help="Number of results per page"),
     project: Annotated[
         Optional[str],
         typer.Option(help="The project to use. If not provided, the default project will be used."),
@@ -502,6 +536,16 @@ def search_notes(
             help="Filter by search item type: entity, observation, relation (repeatable)",
         ),
     ] = None,
+    categories: Annotated[
+        Optional[List[str]],
+        typer.Option(
+            "--category",
+            help=(
+                "Filter observation results to exact categories (repeatable); "
+                "pair with --entity-type observation"
+            ),
+        ),
+    ] = None,
     meta: Annotated[
         Optional[List[str]],
         typer.Option("--meta", help="Filter by frontmatter key=value (repeatable)"),
@@ -536,6 +580,7 @@ def search_notes(
     bm tool search-notes --permalink "specs/*"
     bm tool search-notes --tag python --tag async
     bm tool search-notes --meta status=draft
+    bm tool search-notes "auth" --entity-type observation --category requirement
     """
     try:
         validate_routing_flags(local, cloud)
@@ -601,6 +646,7 @@ def search_notes(
                     page_size=page_size,
                     note_types=note_types,
                     entity_types=entity_types,
+                    categories=categories,
                     metadata_filters=metadata_filters,
                     tags=tags,
                     status=status,

test-int/bughunt_fixes/test_cli_tool_read_note_security_error.py

@@ -0,0 +1,50 @@
+"""Bug hunt regression test (#6): `bm tool read-note` exit code on a
+path-traversal SECURITY_VALIDATION_ERROR.
+
+The MCP read_note tool detects path-traversal identifiers and returns
+{"error": "SECURITY_VALIDATION_ERROR", ...}. Every other wrapped tool command
+exits non-zero on an error payload; read-note used to print the payload and
+exit 0. These integration tests assert read-note now matches its siblings.
+"""
+
+import pytest
+from typer.testing import CliRunner
+
+from basic_memory.cli.main import app as cli_app
+from basic_memory.mcp.tools import read_note as mcp_read_note
+
+runner = CliRunner()
+
+TRAVERSAL_IDENTIFIER = "../../../../etc/passwd"
+
+
+@pytest.mark.asyncio
+async def test_read_note_security_error_mcp_emits_error_field(
+    app, app_config, test_project, config_manager
+):
+    """MCP read_note JSON flags the path traversal with a SECURITY_VALIDATION_ERROR."""
+    result = await mcp_read_note(
+        identifier=TRAVERSAL_IDENTIFIER,
+        project=test_project.name,
+        output_format="json",
+    )
+    assert isinstance(result, dict)
+    assert result.get("error") == "SECURITY_VALIDATION_ERROR"
+
+
+def test_read_note_security_error_cli_exit_code_matches_other_tools(
+    app, app_config, test_project, config_manager
+):
+    """CLI read-note must not exit 0 when the MCP payload carries an error."""
+    result = runner.invoke(
+        cli_app,
+        ["tool", "read-note", TRAVERSAL_IDENTIFIER, "--project", test_project.name],
+    )
+
+    combined = result.stdout
+    assert "SECURITY_VALIDATION_ERROR" in combined, combined
+
+    assert result.exit_code != 0, (
+        f"read-note exited {result.exit_code} on a SECURITY_VALIDATION_ERROR; "
+        "other tool commands exit non-zero on error payloads"
+    )

test-int/bughunt_fixes/test_cli_tool_recent_activity_page_size.py

@@ -0,0 +1,76 @@
+"""Bug hunt regression test (#3): `bm tool recent-activity` page_size default.
+
+The MCP recent_activity tool defaults page_size=10; the CLI wrapper used to
+default to 50. Because page_size becomes the SQL LIMIT for the query, identical
+default invocations returned a different number of rows from CLI vs MCP. This
+integration test proves the CLI default now matches the MCP default of 10.
+"""
+
+import json
+
+from typer.testing import CliRunner
+
+from basic_memory.cli.main import app as cli_app
+
+runner = CliRunner()
+
+MCP_DEFAULT_PAGE_SIZE = 10
+
+
+def _write_note(title: str, folder: str, content: str) -> None:
+    result = runner.invoke(
+        cli_app,
+        [
+            "tool",
+            "write-note",
+            "--title",
+            title,
+            "--folder",
+            folder,
+            "--content",
+            content,
+        ],
+    )
+    assert result.exit_code == 0, result.output
+
+
+def test_recent_activity_default_page_size_matches_mcp(
+    app, app_config, test_project, config_manager, monkeypatch
+):
+    """CLI recent-activity default page_size must match the MCP tool default (10)."""
+    monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", test_project.name)
+
+    for i in range(15):
+        _write_note(
+            f"Parity Note {i:02d}",
+            "parity-recent",
+            f"# Parity Note {i:02d}\n\nUnique body token PARITY{i:02d}.",
+        )
+
+    mcp_default_result = runner.invoke(
+        cli_app,
+        [
+            "tool",
+            "recent-activity",
+            "--project",
+            test_project.name,
+            "--page-size",
+            str(MCP_DEFAULT_PAGE_SIZE),
+        ],
+    )
+    assert mcp_default_result.exit_code == 0, mcp_default_result.output
+    mcp_default_rows = json.loads(mcp_default_result.stdout)
+    assert len(mcp_default_rows) == MCP_DEFAULT_PAGE_SIZE
+
+    cli_default_result = runner.invoke(
+        cli_app,
+        ["tool", "recent-activity", "--project", test_project.name],
+    )
+    assert cli_default_result.exit_code == 0, cli_default_result.output
+    cli_default_rows = json.loads(cli_default_result.stdout)
+
+    assert len(cli_default_rows) == MCP_DEFAULT_PAGE_SIZE, (
+        f"CLI recent-activity default returned {len(cli_default_rows)} rows but "
+        f"the MCP tool default (page_size={MCP_DEFAULT_PAGE_SIZE}) returns "
+        f"{len(mcp_default_rows)}. Default page_size diverges (CLI=50 vs MCP=10)."
+    )

test-int/bughunt_fixes/test_cli_tool_search_notes_category.py

@@ -0,0 +1,73 @@
+"""Bug hunt regression test (#4): `bm tool search-notes` --category filter.
+
+The MCP search_notes tool exposes a `categories` parameter for exact-match
+observation-category filtering. The CLI wrapper had no equivalent flag. This
+integration test asserts the CLI now exposes `--category` and that it filters
+observation results to the requested category exactly.
+"""
+
+import json
+
+from typer.testing import CliRunner
+
+from basic_memory.cli.main import app as cli_app
+
+runner = CliRunner()
+
+
+def _write_note(title: str, folder: str, content: str) -> dict:
+    result = runner.invoke(
+        cli_app,
+        [
+            "tool",
+            "write-note",
+            "--title",
+            title,
+            "--folder",
+            folder,
+            "--content",
+            content,
+        ],
+    )
+    assert result.exit_code == 0, result.output
+    return json.loads(result.stdout)
+
+
+def test_search_notes_exposes_category_filter(app, app_config, test_project, config_manager):
+    """CLI search-notes should expose --category like the MCP `categories` param."""
+    _write_note(
+        "Category Filter Note",
+        "parity-category",
+        "# Category Filter Note\n\n"
+        "## Observations\n"
+        "- [requirement] system must authenticate users CATTOKEN\n"
+        "- [decision] use OAuth for auth CATTOKEN\n",
+    )
+
+    result = runner.invoke(
+        cli_app,
+        [
+            "tool",
+            "search-notes",
+            "CATTOKEN",
+            "--project",
+            test_project.name,
+            "--entity-type",
+            "observation",
+            "--category",
+            "requirement",
+        ],
+    )
+
+    assert result.exit_code == 0, (
+        "`--category` filter is not supported by the CLI search-notes command "
+        "even though the MCP search_notes tool documents a `categories` param. "
+        f"exit_code={result.exit_code} output={result.output}"
+    )
+
+    payload = json.loads(result.stdout)
+    categories = {r.get("category") for r in payload.get("results", []) if r.get("category")}
+    assert categories == {"requirement"}, (
+        "--category requirement should return only requirement observations, "
+        f"got categories={categories}"
+    )