From 6b13a12c010ac3177351b36c798bbc8f8cb4a435 Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 9 Mar 2026 21:33:09 -0500 Subject: [PATCH 1/2] fix: coerce string params to list/dict in MCP tools (#656) Some MCP clients (Claude Desktop, others) serialize list/dict arguments as JSON strings instead of native types. Pydantic's validate_call rejects these with validation errors. Add BeforeValidator coercion via coerce_list/coerce_dict helpers to affected parameters in search_notes, write_note, and canvas tools. Co-Authored-By: Claude Opus 4.6 Signed-off-by: phernandez --- src/basic_memory/mcp/tools/canvas.py | 8 +- src/basic_memory/mcp/tools/search.py | 14 +- src/basic_memory/mcp/tools/write_note.py | 7 +- src/basic_memory/utils.py | 33 ++++- .../mcp/test_string_params_integration.py | 132 ++++++++++++++++++ tests/test_coerce.py | 58 ++++++++ 6 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 test-int/mcp/test_string_params_integration.py create mode 100644 tests/test_coerce.py diff --git a/src/basic_memory/mcp/tools/canvas.py b/src/basic_memory/mcp/tools/canvas.py index 26e94093..f60b4e76 100644 --- a/src/basic_memory/mcp/tools/canvas.py +++ b/src/basic_memory/mcp/tools/canvas.py @@ -4,12 +4,14 @@ """ import json -from typing import Dict, List, Any, Optional +from typing import Annotated, Dict, List, Any, Optional from loguru import logger from fastmcp import Context +from pydantic import BeforeValidator from basic_memory.mcp.project_context import get_project_client +from basic_memory.utils import coerce_list from basic_memory.mcp.server import mcp from basic_memory.mcp.tools.utils import call_put, call_post, resolve_entity_id @@ -19,8 +21,8 @@ annotations={"destructiveHint": False, "idempotentHint": True, "openWorldHint": False}, ) async def canvas( - nodes: List[Dict[str, Any]], - edges: List[Dict[str, Any]], + nodes: Annotated[List[Dict[str, Any]], BeforeValidator(coerce_list)], + edges: Annotated[List[Dict[str, Any]], BeforeValidator(coerce_list)], title: str, directory: str, project: Optional[str] = None, diff --git a/src/basic_memory/mcp/tools/search.py b/src/basic_memory/mcp/tools/search.py index 9ea90276..01d38723 100644 --- a/src/basic_memory/mcp/tools/search.py +++ b/src/basic_memory/mcp/tools/search.py @@ -6,8 +6,10 @@ from loguru import logger from fastmcp import Context +from pydantic import BeforeValidator from basic_memory.config import ConfigManager +from basic_memory.utils import coerce_dict, coerce_list from basic_memory.mcp.container import get_container from basic_memory.mcp.project_context import ( detect_project_from_url_prefix, @@ -307,18 +309,26 @@ async def search_notes( output_format: Literal["text", "json"] = "text", note_types: Annotated[ List[str] | None, + BeforeValidator(coerce_list), "Filter by the 'type' field in note frontmatter (e.g. 'note', 'chapter', 'person'). " "Case-insensitive.", ] = None, entity_types: Annotated[ List[str] | None, + BeforeValidator(coerce_list), "Filter by knowledge graph item type: 'entity' (whole notes), 'observation', or " "'relation'. Defaults to 'entity'. Do NOT pass schema/frontmatter types like " "'Chapter' here — use note_types instead.", ] = None, after_date: Optional[str] = None, - metadata_filters: Optional[Dict[str, Any]] = None, - tags: Optional[List[str]] = None, + metadata_filters: Annotated[ + Dict[str, Any] | None, + BeforeValidator(coerce_dict), + ] = None, + tags: Annotated[ + List[str] | None, + BeforeValidator(coerce_list), + ] = None, status: Optional[str] = None, min_similarity: Optional[float] = None, context: Context | None = None, diff --git a/src/basic_memory/mcp/tools/write_note.py b/src/basic_memory/mcp/tools/write_note.py index c3ab22df..a051fd18 100644 --- a/src/basic_memory/mcp/tools/write_note.py +++ b/src/basic_memory/mcp/tools/write_note.py @@ -1,16 +1,17 @@ """Write note tool for Basic Memory MCP server.""" import textwrap -from typing import List, Union, Optional, Literal +from typing import Annotated, List, Union, Optional, Literal from loguru import logger +from pydantic import BeforeValidator from basic_memory.config import ConfigManager from basic_memory.mcp.project_context import get_project_client, add_project_metadata from basic_memory.mcp.server import mcp from fastmcp import Context from basic_memory.schemas.base import Entity -from basic_memory.utils import parse_tags, validate_project_path +from basic_memory.utils import coerce_dict, parse_tags, validate_project_path # Define TagType as a Union that can accept either a string or a list of strings or None TagType = Union[List[str], str, None] @@ -28,7 +29,7 @@ async def write_note( workspace: Optional[str] = None, tags: list[str] | str | None = None, note_type: str = "note", - metadata: dict | None = None, + metadata: Annotated[dict | None, BeforeValidator(coerce_dict)] = None, overwrite: bool | None = None, output_format: Literal["text", "json"] = "text", context: Context | None = None, diff --git a/src/basic_memory/utils.py b/src/basic_memory/utils.py index 174f770c..ed4a8117 100644 --- a/src/basic_memory/utils.py +++ b/src/basic_memory/utils.py @@ -1,5 +1,6 @@ """Utility functions for basic-memory.""" +import json import os import logging @@ -7,7 +8,7 @@ import sys from datetime import datetime, timezone from pathlib import Path -from typing import Protocol, Union, runtime_checkable, List, Optional +from typing import Any, Protocol, Union, runtime_checkable, List, Optional from loguru import logger from unidecode import unidecode @@ -356,6 +357,36 @@ def parse_tags(tags: Union[List[str], str, None]) -> List[str]: return [] +def coerce_list(v: Any) -> Any: + """Coerce string input to list for MCP clients that serialize lists as strings.""" + if v is None: + return v + if isinstance(v, str): + try: + parsed = json.loads(v) + if isinstance(parsed, list): + return parsed + except (json.JSONDecodeError, TypeError): + pass + # Single string value — wrap in a list + return [v] + return v + + +def coerce_dict(v: Any) -> Any: + """Coerce string input to dict for MCP clients that serialize dicts as strings.""" + if v is None: + return v + if isinstance(v, str): + try: + parsed = json.loads(v) + if isinstance(parsed, dict): + return parsed + except (json.JSONDecodeError, TypeError): + pass + return v + + def normalize_newlines(multiline: str) -> str: """Replace any \r\n, \r, or \n with the native newline. diff --git a/test-int/mcp/test_string_params_integration.py b/test-int/mcp/test_string_params_integration.py new file mode 100644 index 00000000..d3d5d932 --- /dev/null +++ b/test-int/mcp/test_string_params_integration.py @@ -0,0 +1,132 @@ +"""Integration tests for MCP tools accepting string-serialized list/dict params. + +Goes through the full FastMCP Client → validate_call → tool function path, +which is where Pydantic rejects strings for list/dict params. +""" + +import pytest +from fastmcp import Client + + +@pytest.mark.asyncio +async def test_search_notes_entity_types_as_string(mcp_server, app, test_project): + """search_notes should accept entity_types as a JSON string via MCP protocol.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Entity Type Coerce Test", + "directory": "test", + "content": "# Test\nContent for entity type coercion", + }, + ) + + # MCP client sends entity_types as a string + result = await client.call_tool( + "search_notes", + { + "project": test_project.name, + "query": "coercion", + "entity_types": '["entity"]', + }, + ) + text = result.content[0].text + assert "Search Failed" not in text + + +@pytest.mark.asyncio +async def test_search_notes_note_types_as_string(mcp_server, app, test_project): + """search_notes should accept note_types as a JSON string via MCP protocol.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Note Type Coerce Test", + "directory": "test", + "content": "# Test\nContent for note type coercion", + }, + ) + + result = await client.call_tool( + "search_notes", + { + "project": test_project.name, + "query": "coercion", + "note_types": '["note"]', + }, + ) + text = result.content[0].text + assert "Search Failed" not in text + + +@pytest.mark.asyncio +async def test_search_notes_tags_as_string(mcp_server, app, test_project): + """search_notes should accept tags as a JSON string via MCP protocol.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Tags Coerce Test", + "directory": "test", + "content": "# Test\nTagged content for coercion", + "tags": "alpha", + }, + ) + + result = await client.call_tool( + "search_notes", + { + "project": test_project.name, + "query": "tagged", + "tags": '["alpha"]', + }, + ) + text = result.content[0].text + assert "Search Failed" not in text + + +@pytest.mark.asyncio +async def test_search_notes_metadata_filters_as_string(mcp_server, app, test_project): + """search_notes should accept metadata_filters as a JSON string via MCP protocol.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Metadata Coerce Test", + "directory": "test", + "content": "# Test\nMetadata content for coercion", + }, + ) + + result = await client.call_tool( + "search_notes", + { + "project": test_project.name, + "query": "metadata", + "metadata_filters": '{"type": "note"}', + }, + ) + text = result.content[0].text + assert "Search Failed" not in text + + +@pytest.mark.asyncio +async def test_write_note_metadata_as_string(mcp_server, app, test_project): + """write_note should accept metadata as a JSON string via MCP protocol.""" + async with Client(mcp_server) as client: + result = await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "String Metadata Note", + "directory": "test", + "content": "# Test\nWith string metadata", + "metadata": '{"priority": "high"}', + }, + ) + text = result.content[0].text + assert "Created note" in text or "Updated note" in text diff --git a/tests/test_coerce.py b/tests/test_coerce.py new file mode 100644 index 00000000..3adc68ee --- /dev/null +++ b/tests/test_coerce.py @@ -0,0 +1,58 @@ +"""Tests for coerce_list and coerce_dict utility functions. + +These must fail until the helpers are implemented in utils.py. +""" + + +from basic_memory.utils import coerce_list, coerce_dict + + +class TestCoerceList: + """Tests for coerce_list.""" + + def test_none_passthrough(self): + assert coerce_list(None) is None + + def test_native_list_passthrough(self): + assert coerce_list(["a", "b"]) == ["a", "b"] + + def test_json_array_string(self): + assert coerce_list('["entity", "observation"]') == ["entity", "observation"] + + def test_single_string_wrapped(self): + assert coerce_list("entity") == ["entity"] + + def test_non_json_string_wrapped(self): + assert coerce_list("not-json") == ["not-json"] + + def test_json_object_string_wrapped(self): + """A JSON object string is not a list, so wrap it.""" + assert coerce_list('{"key": "val"}') == ['{"key": "val"}'] + + def test_int_passthrough(self): + """Non-string, non-None values pass through unchanged.""" + assert coerce_list(42) == 42 + + +class TestCoerceDict: + """Tests for coerce_dict.""" + + def test_none_passthrough(self): + assert coerce_dict(None) is None + + def test_native_dict_passthrough(self): + assert coerce_dict({"k": "v"}) == {"k": "v"} + + def test_json_object_string(self): + assert coerce_dict('{"status": "draft"}') == {"status": "draft"} + + def test_non_json_string_passthrough(self): + """Non-parseable strings pass through (Pydantic will reject them).""" + assert coerce_dict("not-json") == "not-json" + + def test_json_array_string_passthrough(self): + """A JSON array string is not a dict, so pass through.""" + assert coerce_dict('["a", "b"]') == '["a", "b"]' + + def test_int_passthrough(self): + assert coerce_dict(42) == 42 From d76a5ae8979f0e890e4b91ef1c1606a584fbcae9 Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 9 Mar 2026 21:44:35 -0500 Subject: [PATCH 2/2] test: add canvas integration test for string param coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback — canvas nodes/edges coercion was untested through the FastMCP client path. Co-Authored-By: Claude Opus 4.6 Signed-off-by: phernandez --- .../mcp/test_string_params_integration.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test-int/mcp/test_string_params_integration.py b/test-int/mcp/test_string_params_integration.py index d3d5d932..0684381e 100644 --- a/test-int/mcp/test_string_params_integration.py +++ b/test-int/mcp/test_string_params_integration.py @@ -130,3 +130,38 @@ async def test_write_note_metadata_as_string(mcp_server, app, test_project): ) text = result.content[0].text assert "Created note" in text or "Updated note" in text + + +@pytest.mark.asyncio +async def test_canvas_nodes_edges_as_string(mcp_server, app, test_project): + """canvas should accept nodes and edges as JSON strings via MCP protocol.""" + import json + + nodes = [ + { + "id": "n1", + "type": "text", + "text": "Hello", + "x": 0, + "y": 0, + "width": 200, + "height": 100, + } + ] + edges = [ + {"id": "e1", "fromNode": "n1", "toNode": "n1", "label": "self"} + ] + + async with Client(mcp_server) as client: + result = await client.call_tool( + "canvas", + { + "project": test_project.name, + "title": "Coerce Canvas Test", + "directory": "test", + "nodes": json.dumps(nodes), + "edges": json.dumps(edges), + }, + ) + text = result.content[0].text + assert "Created" in text or "Updated" in text