fix(cli): report missing notes with a failing exit status (#1508)

Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

src/basic_memory/cli/commands/tool.py

@@ -770,14 +770,11 @@ def read_note(
                 )
             )
 
-        # 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"):
+        # A missing note may carry useful suggestions. Render those in the
+        # requested mode before exiting non-zero; other failures keep the
+        # existing JSON diagnostic path.
+        not_found = isinstance(result, dict) and result.get("error") == "NOTE_NOT_FOUND"
+        if isinstance(result, dict) and result.get("error") and not not_found:
             typer.echo(f"Error: {result['error']}", err=True)
             _print_json(result)
             raise typer.Exit(1)
@@ -801,9 +798,12 @@ def read_note(
             else:
                 console.print(Text(text))
         elif mode == "plain":
-            _plain_read_note(result, include_frontmatter=include_frontmatter)
+            _plain_read_note(result, include_frontmatter=include_frontmatter and not not_found)
         else:
             _display_read_note(result, include_frontmatter=include_frontmatter)
+        if not_found:
+            typer.echo(f"Error: Note not found: {identifier}", err=True)
+            raise typer.Exit(1)
     except ValueError as e:
         typer.echo(f"Error: {e}", err=True)
         raise typer.Exit(1)

src/basic_memory/man/man3/read-note(3).md

@@ -37,10 +37,13 @@ bm tool read-note IDENTIFIER [--project NAME | --project-id UUID]
 
 Returns the raw markdown of a note. The identifier is resolved through a
 cascade: direct permalink lookup, then exact title match, then full-text
-search. If nothing matches exactly, read-note returns guidance text instead
-of an error: a ranked list of related notes, each with a copy-pasteable
+search. If nothing matches exactly, MCP text mode returns guidance:
+a ranked list of related notes, each with a copy-pasteable
 `read_note()` call, plus suggested `search_notes()` and `write_note()` next
-steps. A miss is a navigable dead end, not an exception.
+steps. JSON mode returns null note fields plus `error: "NOTE_NOT_FOUND"`,
+a message naming the identifier, and `related_results` when available.
+The CLI exits with status 1 for a missing note in every output mode, retaining
+suggestions in JSON, plain, and Rich output. An existing empty note still succeeds.
 
 Accepted identifier forms (all verified):
 
@@ -56,7 +59,7 @@ Accepted identifier forms (all verified):
 - **project_id** (string | null, optional, default: None) — Project external_id (UUID). Prefer this over `project` when known — it routes to the exact project regardless of name collisions across cloud workspaces. Takes precedence over `project`. Get from list_memory_projects().
 - **page** (integer, optional, default: 1) — Page of fallback-search results to use when the identifier does not resolve to a note directly (default: 1). A direct or exact-title match returns the note content — page/page_size never chunk the note itself, and the title-match lookup pages through fixed-size pages of title results until an exact match is found or results are exhausted, regardless of page or page_size. Aliases: page_number.
 - **page_size** (integer, optional, default: 10) — Number of fallback-search results per page (default: 10). When no match is found, this caps how many related-note suggestions are listed. Aliases: limit, per_page.
-- **output_format** (string, optional, default: "text") — "text" returns markdown content or guidance text. "json" returns a structured object with title/permalink/file_path/content/frontmatter.
+- **output_format** (string, optional, default: "text") — "text" returns markdown content or guidance text. "json" returns a structured object with title/permalink/file_path/content/frontmatter. Unresolved notes carry error="NOTE_NOT_FOUND" and a message, with related_results when suggestions are available.
 - **include_frontmatter** (boolean, optional, default: False) — For unsliced JSON reads, include opening YAML in content; parsed frontmatter is returned either way. Explicit line ranges are never stripped. CLI: --frontmatter (--include-frontmatter is a deprecated alias).
 - **start_line** (integer | null, optional, default: None) — First document line to read (1-based, inclusive). Defaults to 1 when only end_line is given. Line scans count the full Markdown, including frontmatter, matching cat's default line coordinates.
 - **end_line** (integer | null, optional, default: None) — Last document line to read (inclusive); omitted means EOF. Out-of-file ranges return empty content; invalid/reversed ranges fail. With either bound, text output is numbered and JSON carries coordinates, has_more, and next_start_line/next_end_line. include_frontmatter does not strip an explicitly addressed range. Edits between calls may shift lines.
@@ -102,7 +105,7 @@ bm tool read-note runbook --start-line 120 --end-line 180 --plain
 
 ## EXAMPLES
 
-A miss returns suggestions, not an error (run against the dev project):
+A miss in MCP text mode returns suggestions (run against the dev project):
 
 ```
 read_note("xyzzy definitely missing note", project="dev")

src/basic_memory/mcp/tools/read_note.py

@@ -8,6 +8,8 @@
 
 from loguru import logger
 from fastmcp import Context
+from fastmcp.exceptions import ToolError
+from httpx import HTTPStatusError
 from pydantic import AliasChoices, Field
 
 from basic_memory.config import ConfigManager
@@ -130,6 +132,8 @@ async def read_note(
             Aliases: limit, per_page.
         output_format: "text" returns markdown content or guidance text.
             "json" returns a structured object with title/permalink/file_path/content/frontmatter.
+            Unresolved notes carry error="NOTE_NOT_FOUND" and a message, with
+            related_results when suggestions are available.
         include_frontmatter: For unsliced JSON reads, include opening YAML in content;
             parsed frontmatter is returned either way. Explicit line ranges are never
             stripped. CLI: --frontmatter (--include-frontmatter is a deprecated alias).
@@ -290,13 +294,15 @@ async def _read_resolved_note(entity_id: str) -> str | dict[str, Any]:
                     "next_end_line": min(total, last + width) if last < total else None,
                 }
 
-            def _empty_json_payload() -> dict[str, Any]:
+            def _not_found_json_payload() -> dict[str, Any]:
                 return {
                     "title": None,
                     "permalink": None,
                     "file_path": None,
                     "content": None,
                     "frontmatter": None,
+                    "error": "NOTE_NOT_FOUND",
+                    "message": f"Note not found: {identifier}",
                 }
 
             def _search_results(payload: object) -> list[dict[str, object]]:
@@ -345,7 +351,13 @@ async def _search_candidates(
                     output_format="json",
                     context=context,
                 )
-                return cast(dict[str, object], response) if isinstance(response, dict) else {}
+                # JSON searches return a dict even when empty. Text here is a
+                # formatted search failure, not evidence that the note is absent.
+                if not isinstance(response, dict):
+                    if output_format == "json" or line_scan:
+                        raise RuntimeError(f"Fallback search failed: {response}")
+                    return {}
+                return cast(dict[str, object], response)
 
             def _result_title(item: dict[str, object]) -> str:
                 return str(item.get("title") or "")
@@ -365,11 +377,47 @@ def _result_external_id(item: dict[str, object]) -> str | None:
             if output_format == "json" or line_scan:
                 exact_external_id = _exact_external_id(entity_path)
                 if exact_external_id is not None:
-                    return await _read_resolved_note(exact_external_id)
+                    try:
+                        return await _read_resolved_note(exact_external_id)
+                    except ToolError as error:
+                        cause = error.__cause__
+                        # Only the entity GET proves this UUID is absent. A 404
+                        # from its resource fallback is a failed content read.
+                        if (
+                            isinstance(cause, HTTPStatusError)
+                            and cause.response.status_code == 404
+                            and cause.request.url.path.endswith(
+                                f"/knowledge/entities/{exact_external_id}"
+                            )
+                        ):
+                            if line_scan:
+                                # The slice endpoint also uses 404 for an existing
+                                # non-Markdown entity. Confirm absence without slice
+                                # parameters before classifying this error as missing.
+                                try:
+                                    await knowledge_client.get_entity(exact_external_id)
+                                except ToolError as lookup_error:
+                                    lookup_cause = lookup_error.__cause__
+                                    if (
+                                        not isinstance(lookup_cause, HTTPStatusError)
+                                        or lookup_cause.response.status_code != 404
+                                    ):
+                                        raise
+                                else:
+                                    raise error
+                            if output_format == "json":
+                                return _not_found_json_payload()
+                            return format_not_found_message(active_project.name, identifier)
+                        raise
 
                 try:
                     entity_id = await knowledge_client.resolve_entity(entity_path, strict=True)
-                except Exception as error:  # pragma: no cover
+                except ToolError as error:
+                    cause = error.__cause__
+                    # Search is a recovery for a confirmed lookup miss, not for
+                    # unavailable or unauthorized resolution services.
+                    if not isinstance(cause, HTTPStatusError) or cause.response.status_code != 404:
+                        raise
                     logger.info(f"Direct lookup failed for '{entity_path}': {error}")
                 else:
                     logger.info(
@@ -431,22 +479,16 @@ def _result_external_id(item: dict[str, object]) -> str | None:
                     break
 
             if result is not None and (output_format == "json" or line_scan):
-                try:
-                    entity_id = _result_external_id(result)
-                    if entity_id is None and _result_permalink(result) is not None:
-                        entity_id = await knowledge_client.resolve_entity(
-                            _result_permalink(result) or "", strict=True
-                        )
-                    if entity_id is not None:
-                        logger.info(
-                            f"Found note by exact title search: {_result_permalink(result)}"
-                        )
-                        return await _read_resolved_note(entity_id)
-                except Exception as error:  # pragma: no cover
-                    logger.info(
-                        "Failed to fetch content for found title match "
-                        f"{_result_permalink(result)}: {error}"
+                # An exact candidate identifies a note; retrieval failures must surface
+                # as operational errors instead of suggesting that it is missing.
+                entity_id = _result_external_id(result)
+                if entity_id is None and _result_permalink(result) is not None:
+                    entity_id = await knowledge_client.resolve_entity(
+                        _result_permalink(result) or "", strict=True
                     )
+                if entity_id is not None:
+                    logger.info(f"Found note by exact title search: {_result_permalink(result)}")
+                    return await _read_resolved_note(entity_id)
             elif result is not None and _result_permalink(result):
                 try:
                     entity_id = await knowledge_client.resolve_entity(
@@ -472,13 +514,13 @@ def _result_external_id(item: dict[str, object]) -> str | None:
             text_candidates = _search_results(text_results)
             if not text_candidates:
                 if output_format == "json":
-                    return _empty_json_payload()
+                    return _not_found_json_payload()
                 return format_not_found_message(active_project.name, identifier)
             # The fallback search is paginated server-side to page_size, so list
             # the whole returned page instead of a hardcoded cap — otherwise the
             # caller's page_size would be silently ignored past the cap.
             if output_format == "json":
-                payload = _empty_json_payload()
+                payload = _not_found_json_payload()
                 payload["related_results"] = [
                     {
                         "title": _result_title(result),

test-int/cli/test_cli_tool_delete_note_integration.py

@@ -36,14 +36,19 @@ def _write_note(
     return json.loads(result.stdout)
 
 
-def _read_note(identifier: str, *, project: str | None = None) -> dict[str, Any]:
+def _read_note(
+    identifier: str, *, project: str | None = None, missing: bool = False
+) -> dict[str, Any]:
     args = ["tool", "read-note", identifier]
     if project is not None:
         args.extend(["--project", project])
 
     result = runner.invoke(cli_app, args)
-    assert result.exit_code == 0, result.output
-    return json.loads(result.stdout)
+    assert result.exit_code == (1 if missing else 0), result.output
+    payload = json.loads(result.stdout)
+    if missing:
+        assert payload["error"] == "NOTE_NOT_FOUND"
+    return payload
 
 
 def _delete_note(
@@ -114,7 +119,7 @@ def test_delete_note_removes_file_database_record_and_search_result(
     }
     assert not note_path.exists()
 
-    missing = _read_note(note["permalink"])
+    missing = _read_note(note["permalink"], missing=True)
     assert missing["title"] is None
     assert missing["permalink"] is None
     assert missing["content"] is None
@@ -177,7 +182,7 @@ def test_delete_note_project_id_takes_precedence_over_wrong_project_name(
     assert exit_code == 0, output
     assert payload["deleted"] is True
     assert payload["title"] == "CLI Delete By Project ID"
-    assert _read_note(note["permalink"])["title"] is None
+    assert _read_note(note["permalink"], missing=True)["title"] is None
 
 
 def test_delete_note_memory_url_detects_project_from_identifier(
@@ -197,7 +202,7 @@ def test_delete_note_memory_url_detects_project_from_identifier(
     assert exit_code == 0, output
     assert payload["deleted"] is True
     assert payload["permalink"] == note["permalink"]
-    assert _read_note(note["permalink"], project=test_project.name)["title"] is None
+    assert _read_note(note["permalink"], project=test_project.name, missing=True)["title"] is None
 
 
 def test_delete_directory_removes_nested_files_database_records_and_search_results(
@@ -238,7 +243,7 @@ def test_delete_directory_removes_nested_files_database_records_and_search_resul
     assert not any(path.exists() for path in note_paths)
 
     for note in notes:
-        assert _read_note(note["permalink"])["title"] is None
+        assert _read_note(note["permalink"], missing=True)["title"] is None
 
     search = _search_notes("CLI Delete Directory", mode_flag="--title")
     assert search["total"] == 0

test-int/cli/test_cli_tool_json_failure_integration.py

@@ -6,28 +6,81 @@
 
 import json
 
+import pytest
 from typer.testing import CliRunner
 
 from basic_memory.cli.main import app as cli_app
 
 runner = CliRunner()
 
 
-def test_read_note_not_found(app, app_config, test_project, config_manager):
-    """read-note with non-existent identifier returns JSON with null fields."""
+@pytest.mark.parametrize(
+    "identifier", ["nonexistent-note-that-does-not-exist", "22222222-2222-4222-8222-222222222222"]
+)
+def test_read_note_not_found(app, app_config, test_project, config_manager, identifier):
+    """A missing note remains machine-readable but must not report success."""
     result = runner.invoke(
         cli_app,
-        ["tool", "read-note", "nonexistent-note-that-does-not-exist"],
+        ["tool", "read-note", identifier],
     )
 
-    assert result.exit_code == 0
+    assert result.exit_code == 1
     data = json.loads(result.stdout)
-    # MCP tool returns a valid JSON payload with null fields for not-found
+    assert data["error"] == "NOTE_NOT_FOUND"
+    assert data["message"] == f"Note not found: {identifier}"
+    assert "Note not found" in result.stderr
     assert data["title"] is None
     assert data["permalink"] is None
     assert data["content"] is None
 
 
+@pytest.mark.parametrize("mode", ["piped", "json", "plain", "rich"])
+@pytest.mark.parametrize("related", [False, True])
+def test_read_after_delete_fails_in_every_mode(
+    app, app_config, test_project, config_manager, monkeypatch, mode, related
+):
+    """The actual write/delete/read flow must fail even when search offers alternatives."""
+    monkeypatch.setattr("basic_memory.cli.commands.tool._use_rich", lambda: mode == "rich")
+    title = "Missingneedle"
+    created = runner.invoke(
+        cli_app,
+        ["tool", "write-note", "--title", title, "--folder", "test", "--content", "original"],
+    )
+    assert created.exit_code == 0, created.output
+    deleted = runner.invoke(cli_app, ["tool", "delete-note", title])
+    assert deleted.exit_code == 0, deleted.output
+    if related:
+        alternative = runner.invoke(
+            cli_app,
+            [
+                "tool",
+                "write-note",
+                "--title",
+                "Alternative",
+                "--folder",
+                "test",
+                "--content",
+                f"This mentions {title} but is a different note.",
+            ],
+        )
+        assert alternative.exit_code == 0, alternative.output
+
+    flags = [f"--{mode}"] if mode in {"json", "plain"} else []
+    result = runner.invoke(cli_app, ["tool", "read-note", title, "--frontmatter", *flags])
+    assert result.exit_code == 1, result.output
+    assert f"Note not found: {title}" in result.stderr
+    if mode in {"piped", "json"}:
+        payload = json.loads(result.stdout)
+        assert payload["error"] == "NOTE_NOT_FOUND"
+        assert payload["content"] is None
+        if related:
+            assert payload["related_results"][0]["title"] == "Alternative"
+    else:
+        assert "Note not found" in result.stdout
+        if related:
+            assert "Alternative" in result.stdout
+
+
 def test_write_note_missing_content(app, app_config, test_project, config_manager):
     """write-note without content or stdin returns error exit code."""
     result = runner.invoke(