mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: coerce string params to list/dict in MCP tools (#657)
Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -4,12 +4,14 @@ This tool creates Obsidian canvas files (.canvas) using the JSON Canvas 1.0 spec
|
||||
"""
|
||||
|
||||
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 @@ from basic_memory.mcp.tools.utils import call_put, call_post, resolve_entity_id
|
||||
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,
|
||||
|
||||
@@ -6,8 +6,10 @@ from typing import Annotated, List, Optional, Dict, Any, Literal
|
||||
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""Utility functions for basic-memory."""
|
||||
|
||||
import json
|
||||
import os
|
||||
|
||||
import logging
|
||||
@@ -7,7 +8,7 @@ import re
|
||||
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
|
||||
@@ -385,6 +386,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.
|
||||
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
"""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
|
||||
|
||||
|
||||
@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
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user