mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: accept null for expected_replacements in edit_note (#606)
MCP clients may send explicit `null` for unused optional fields. `expected_replacements: int = 1` caused FastMCP's JSON Schema validation to reject null before the function body ran. Changed to `Optional[int] = None` with an effective default resolved inside the function body. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -135,7 +135,7 @@ async def edit_note(
|
||||
workspace: Optional[str] = None,
|
||||
section: Optional[str] = None,
|
||||
find_text: Optional[str] = None,
|
||||
expected_replacements: int = 1,
|
||||
expected_replacements: Optional[int] = None,
|
||||
output_format: Literal["text", "json"] = "text",
|
||||
context: Context | None = None,
|
||||
) -> str | dict:
|
||||
@@ -216,6 +216,9 @@ async def edit_note(
|
||||
search_notes() first to find the correct identifier. The tool provides detailed
|
||||
error messages with suggestions if operations fail.
|
||||
"""
|
||||
# Resolve effective default: allow MCP clients to send null for optional int field
|
||||
effective_replacements = expected_replacements if expected_replacements is not None else 1
|
||||
|
||||
async with get_project_client(project, workspace, context) as (client, active_project):
|
||||
logger.info("MCP tool call", tool="edit_note", identifier=identifier, operation=operation)
|
||||
|
||||
@@ -254,8 +257,8 @@ async def edit_note(
|
||||
edit_data["section"] = section
|
||||
if find_text:
|
||||
edit_data["find_text"] = find_text
|
||||
if expected_replacements != 1: # Only send if different from default
|
||||
edit_data["expected_replacements"] = str(expected_replacements)
|
||||
if effective_replacements != 1: # Only send if different from default
|
||||
edit_data["expected_replacements"] = str(effective_replacements)
|
||||
|
||||
# Call the PATCH endpoint
|
||||
result = await knowledge_client.patch_entity(entity_id, edit_data, fast=False)
|
||||
@@ -339,5 +342,5 @@ async def edit_note(
|
||||
"error": str(e),
|
||||
}
|
||||
return _format_error_response(
|
||||
str(e), operation, identifier, find_text, expected_replacements, active_project.name
|
||||
str(e), operation, identifier, find_text, effective_replacements, active_project.name
|
||||
)
|
||||
|
||||
@@ -414,6 +414,39 @@ async def test_edit_note_find_replace_empty_find_text(client, test_project):
|
||||
# Should contain helpful guidance about the error
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_note_append_with_null_optional_fields(client, test_project):
|
||||
"""Regression test: MCP clients may send explicit null for unused optional fields.
|
||||
|
||||
When an MCP client sends find_text=None, section=None, expected_replacements=None
|
||||
for an append operation, the tool should accept them without validation errors.
|
||||
"""
|
||||
# Create initial note
|
||||
await write_note(
|
||||
project=test_project.name,
|
||||
title="Null Fields Test",
|
||||
directory="test",
|
||||
content="# Null Fields Test\nOriginal content.",
|
||||
)
|
||||
|
||||
# Call edit_note with explicit None for all optional fields (simulates MCP null)
|
||||
result = await edit_note(
|
||||
project=test_project.name,
|
||||
identifier="test/null-fields-test",
|
||||
operation="append",
|
||||
content="\nAppended content.",
|
||||
find_text=None,
|
||||
section=None,
|
||||
expected_replacements=None,
|
||||
)
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "Edited note (append)" in result
|
||||
assert f"project: {test_project.name}" in result
|
||||
assert "file_path: test/Null Fields Test.md" in result
|
||||
assert f"[Session: Using project '{test_project.name}']" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_note_preserves_permalink_when_frontmatter_missing(client, test_project):
|
||||
"""Test that editing a note preserves the permalink when frontmatter doesn't contain one.
|
||||
|
||||
Reference in New Issue
Block a user