Fix remaining MCP JSON output review issues

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

Author: phernandez

src/basic_memory/mcp/tools/delete_note.py

@@ -143,7 +143,7 @@ def _format_delete_error_response(project: str, error_message: str, identifier:
 - Check what notes exist: `list_directory("{project}", "/")`
 
 ## Need help?
-If the note should be deleted but the operation keeps failing, send a message to support@basicmemory.com."""
+If the note should be deleted but the operation keeps failing, send a message to support@basicmachines.co."""
 
 
 @mcp.tool(description="Delete a note or directory by title, permalink, or path")

src/basic_memory/mcp/tools/edit_note.py

@@ -288,7 +288,7 @@ async def edit_note(
                 for obs in result.observations:
                     categories[obs.category] = categories.get(obs.category, 0) + 1
 
-                summary.append("\\n## Observations")
+                summary.append("\n## Observations")
                 for category, count in sorted(categories.items()):
                     summary.append(f"- {category}: {count}")
 
@@ -299,7 +299,7 @@ async def edit_note(
                 unresolved = sum(1 for r in result.relations if not r.to_id)
                 resolved = len(result.relations) - unresolved
 
-                summary.append("\\n## Relations")
+                summary.append("\n## Relations")
                 summary.append(f"- Resolved: {resolved}")
                 if unresolved:
                     summary.append(f"- Unresolved: {unresolved}")

src/basic_memory/mcp/tools/move_note.py

@@ -568,17 +568,25 @@ async def move_note(
         # Use typed KnowledgeClient for API calls
         knowledge_client = KnowledgeClient(client, active_project.external_id)
 
-        # Get the source entity information for extension validation
+        # Resolve once and reuse the entity ID across extension validation and move.
         source_ext = "md"  # Default to .md if we can't determine source extension
+        resolved_entity_id: str | None = None
+        source_entity = None
+
+        async def _ensure_resolved_entity_id() -> str:
+            """Resolve and cache the source entity ID for the duration of this move."""
+            nonlocal resolved_entity_id
+            if resolved_entity_id is None:
+                resolved_entity_id = await knowledge_client.resolve_entity(identifier)
+            return resolved_entity_id
+
         try:
-            # Resolve identifier to entity ID
-            entity_id = await knowledge_client.resolve_entity(identifier)
-            # Fetch source entity information to get the current file extension
-            source_entity = await knowledge_client.get_entity(entity_id)
+            resolved_entity_id = await _ensure_resolved_entity_id()
+            source_entity = await knowledge_client.get_entity(resolved_entity_id)
             if "." in source_entity.file_path:
                 source_ext = source_entity.file_path.split(".")[-1]
         except Exception as e:
-            # If we can't fetch the source entity, default to .md extension
+            # If we can't fetch source metadata, continue with extension defaults.
             logger.debug(f"Could not fetch source entity for extension check: {e}")
 
         # Validate that destination path includes a file extension
@@ -612,14 +620,15 @@ async def move_note(
                 All examples in Basic Memory expect file extensions to be explicitly provided.
                 """).strip()
 
-        # Get the source entity to check its file extension
-        try:
-            # Resolve identifier to entity ID (might already be cached from above)
-            entity_id = await knowledge_client.resolve_entity(identifier)
-            # Fetch source entity information
-            source_entity = await knowledge_client.get_entity(entity_id)
+        # Validate extension consistency when source metadata is available.
+        if source_entity is None:
+            try:
+                resolved_entity_id = await _ensure_resolved_entity_id()
+                source_entity = await knowledge_client.get_entity(resolved_entity_id)
+            except Exception as e:
+                logger.debug(f"Could not fetch source entity for extension check: {e}")
 
-            # Extract file extensions
+        if source_entity is not None:
             source_ext = (
                 source_entity.file_path.split(".")[-1] if "." in source_entity.file_path else ""
             )
@@ -656,17 +665,13 @@ async def move_note(
                     move_note("{identifier}", "{destination_path.rsplit(".", 1)[0]}.{source_ext}")
                     ```
                     """).strip()
-        except Exception as e:
-            # If we can't fetch the source entity, log it but continue
-            # This might happen if the identifier is not yet resolved
-            logger.debug(f"Could not fetch source entity for extension check: {e}")
 
         try:
-            # Resolve identifier to entity ID for the move operation
-            entity_id = await knowledge_client.resolve_entity(identifier)
+            # Resolve identifier only if earlier checks could not.
+            resolved_entity_id = await _ensure_resolved_entity_id()
 
             # Call the move API using KnowledgeClient
-            result = await knowledge_client.move_entity(entity_id, destination_path)
+            result = await knowledge_client.move_entity(resolved_entity_id, destination_path)
             if output_format == "json":
                 return {
                     "moved": True,

src/basic_memory/mcp/tools/project_management.py

@@ -43,6 +43,8 @@ async def list_memory_projects(
                     "name": project.name,
                     "path": project.path,
                     "is_default": project.is_default,
+                    # Reserved for forward-compatible cloud/private project metadata.
+                    # Local project list responses do not currently provide these values.
                     "is_private": False,
                     "display_name": None,
                 }
@@ -154,7 +156,7 @@ async def create_memory_project(
                 f"Project Details:\n"
                 f"• Name: {existing_match.name}\n"
                 f"• Path: {existing_match.path}\n"
-                f"{'• Set as default project\\n' if is_default else ''}"
+                f"{'• Set as default project\n' if is_default else ''}"
                 "\nProject is already available for use in tool calls.\n"
             )
 

src/basic_memory/mcp/tools/read_note.py

@@ -192,32 +192,33 @@ def _empty_json_payload() -> dict:
                 "frontmatter": None,
             }
 
-        def _search_results(payload: object) -> list:
+        def _search_results(payload: object) -> list[dict]:
             if isinstance(payload, dict):
                 results = payload.get("results")
                 return results if isinstance(results, list) else []
-            if hasattr(payload, "results"):
-                results = getattr(payload, "results")
-                return results if isinstance(results, list) else []
+            model_dump = getattr(payload, "model_dump", None)
+            if callable(model_dump):
+                dumped = model_dump()
+                if isinstance(dumped, dict):
+                    results = dumped.get("results")
+                    return results if isinstance(results, list) else []
             return []
 
         def _result_title(item: object) -> str:
-            if isinstance(item, dict):
-                return str(item.get("title") or "")
-            return str(getattr(item, "title", "") or "")
+            if not isinstance(item, dict):
+                return ""
+            return str(item.get("title") or "")
 
         def _result_permalink(item: object) -> Optional[str]:
-            if isinstance(item, dict):
-                value = item.get("permalink")
-                return str(value) if value else None
-            value = getattr(item, "permalink", None)
+            if not isinstance(item, dict):
+                return None
+            value = item.get("permalink")
             return str(value) if value else None
 
         def _result_file_path(item: object) -> Optional[str]:
-            if isinstance(item, dict):
-                value = item.get("file_path")
-                return str(value) if value else None
-            value = getattr(item, "file_path", None)
+            if not isinstance(item, dict):
+                return None
+            value = item.get("file_path")
             return str(value) if value else None
 
         try:

src/basic_memory/mcp/tools/recent_activity.py

@@ -80,7 +80,7 @@ async def recent_activity(
                 hierarchy above. If unknown, use list_memory_projects() to discover
                 available projects.
         output_format: "text" returns human-readable summary text. "json" returns
-            a flat list of recent entity items.
+            a flat list of recent items.
         context: Optional FastMCP context for performance caching.
 
     Returns:
@@ -335,6 +335,7 @@ def _extract_recent_rows(
     for result in activity_data.results:
         primary = result.primary_result
         row = {
+            "type": primary.type,
             "title": primary.title,
             "permalink": primary.permalink,
             "file_path": primary.file_path,

test-int/mcp/test_output_format_json_integration.py

@@ -128,7 +128,9 @@ async def test_recent_activity_json_output(mcp_server, app, test_project):
         assert isinstance(payload, list)
         assert any(item.get("title") == "JSON Integration Recent" for item in payload)
         for item in payload:
-            assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+            assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(
+                item.keys()
+            )
 
 
 @pytest.mark.asyncio
@@ -171,8 +173,11 @@ async def test_recent_activity_json_output_for_relation_and_observation_types(
         relation_payload = _json_content(relation_result)
         assert isinstance(relation_payload, list)
         assert relation_payload
+        assert all(item.get("type") == "relation" for item in relation_payload)
         for item in relation_payload:
-            assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+            assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(
+                item.keys()
+            )
 
         observation_result = await client.call_tool(
             "recent_activity",
@@ -186,8 +191,11 @@ async def test_recent_activity_json_output_for_relation_and_observation_types(
         observation_payload = _json_content(observation_result)
         assert isinstance(observation_payload, list)
         assert observation_payload
+        assert all(item.get("type") == "observation" for item in observation_payload)
         for item in observation_payload:
-            assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+            assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(
+                item.keys()
+            )
 
 
 @pytest.mark.asyncio

tests/mcp/test_tool_json_output_modes.py

@@ -146,7 +146,7 @@ async def test_recent_activity_text_and_json_modes(app, test_project):
     assert isinstance(json_result, list)
     assert any(item.get("title") == "Mode Activity Note" for item in json_result)
     for item in json_result:
-        assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+        assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(item.keys())
 
 
 @pytest.mark.asyncio
@@ -176,8 +176,9 @@ async def test_recent_activity_json_preserves_relation_and_observation_types(app
     )
     assert isinstance(relation_json, list)
     assert relation_json
+    assert all(item.get("type") == "relation" for item in relation_json)
     for item in relation_json:
-        assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+        assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(item.keys())
 
     observation_json = await recent_activity.fn(
         project=test_project.name,
@@ -187,8 +188,9 @@ async def test_recent_activity_json_preserves_relation_and_observation_types(app
     )
     assert isinstance(observation_json, list)
     assert observation_json
+    assert all(item.get("type") == "observation" for item in observation_json)
     for item in observation_json:
-        assert set(["title", "permalink", "file_path", "created_at"]).issubset(item.keys())
+        assert set(["type", "title", "permalink", "file_path", "created_at"]).issubset(item.keys())
 
 
 @pytest.mark.asyncio
@@ -225,6 +227,25 @@ async def test_list_and_create_project_text_and_json_modes(app, test_project, tm
     assert create_json_again["created"] is False
     assert create_json_again["already_exists"] is True
 
+    default_project_name = "mode-default-project"
+    default_project_path = str(
+        tmp_path.parent / (tmp_path.name + "-projects") / "mode-default-project"
+    )
+    await create_memory_project.fn(
+        project_name=default_project_name,
+        project_path=default_project_path,
+        set_default=True,
+        output_format="text",
+    )
+
+    default_text_again = await create_memory_project.fn(
+        project_name=default_project_name,
+        project_path=default_project_path,
+        output_format="text",
+    )
+    assert "Set as default project\\n" not in default_text_again
+    assert "Set as default project\n" in default_text_again
+
 
 @pytest.mark.asyncio
 async def test_delete_note_text_and_json_modes(app, test_project):