Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/basic_memory/mcp/tools/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from pydantic import AliasChoices, BeforeValidator, Field

from basic_memory.config import ConfigManager, has_cloud_credentials
from basic_memory.utils import build_canonical_permalink, coerce_dict, coerce_list
from basic_memory.utils import build_canonical_permalink, coerce_dict, coerce_list, parse_tags
from basic_memory.mcp.async_client import (
_explicit_routing,
_force_local_mode,
Expand Down Expand Up @@ -678,7 +678,10 @@ async def search_notes(
] = None,
tags: Annotated[
List[str] | None,
BeforeValidator(coerce_list),
# Use parse_tags (not coerce_list) so a bare comma string like "alpha,beta"
# splits into ["alpha", "beta"], matching the `tag:` query shorthand and
# write_note's tag convention. See #910.
BeforeValidator(parse_tags),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-list tag values instead of stringifying them

When tags receives an invalid JSON value such as an object or number, parse_tags falls through to parse_tags(str(tags)), so the MCP argument validator now accepts malformed filters as a literal tag instead of failing validation. The previous coerce_list path left non-string/non-list values for Pydantic to reject, so bad client payloads surfaced immediately; now they become successful searches with confusing no-result behavior. Use a search-specific parser that only accepts str, list[str], or None before splitting comma strings.

Useful? React with 👍 / 👎.

] = None,
status: Optional[str] = None,
min_similarity: Annotated[
Expand Down
52 changes: 52 additions & 0 deletions test-int/mcp/test_search_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,3 +497,55 @@ async def test_search_case_insensitive(mcp_server, app, test_project):

result_text = search_result.content[0].text
assert "Machine Learning Guide" in result_text, f"Failed for search term: {search_term}"


@pytest.mark.asyncio
async def test_tags_param_vs_tag_query_comma_consistency(mcp_server, app, test_project):
"""The `tags=` parameter must split comma-separated strings like the `tag:` shorthand.

Regression test for #910: `search_notes(tags="alpha,beta")` previously coerced the
bare string into the single literal tag `["alpha,beta"]` (matching nothing), while
the `tag:alpha,beta` query shorthand splits on commas. Both paths must agree.
"""

async with Client(mcp_server) as client:
# Note tagged alpha + beta
await client.call_tool(
"write_note",
{
"project": test_project.name,
"title": "Tag Shorthand Note",
"directory": "tag-shorthand",
"content": "# Tag Shorthand Note\n\nTagShorthandToken body",
"tags": ["alpha", "beta"],
},
)

# Path A: tag: query shorthand with comma list -> splits, matches
via_query = await client.call_tool(
"search_notes",
{
"project": test_project.name,
"query": "tag:alpha,beta",
"search_type": "text",
},
)
query_hit = "Tag Shorthand Note" in via_query.content[0].text

# Path B: tags= parameter with the SAME comma string
via_param = await client.call_tool(
"search_notes",
{
"project": test_project.name,
"query": "TagShorthandToken",
"search_type": "text",
"tags": "alpha,beta",
},
)
param_hit = "Tag Shorthand Note" in via_param.content[0].text

assert query_hit, "tag: query shorthand should match (sanity)"
assert param_hit == query_hit, (
"tags='alpha,beta' param must behave like the tag: shorthand "
f"(both split commas). query_hit={query_hit} param_hit={param_hit}"
)