Skip to content

Commit fd848a3

Browse files
phernandeztonydziclaude
authored
fix(mcp): report the accepted checksum from write_note and edit_note (#1619)
Signed-off-by: tonydzi <194927794+tonydzi@users.noreply.github.com> Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: tonydzi <194927794+tonydzi@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9265bdb commit fd848a3

7 files changed

Lines changed: 177 additions & 16 deletions

File tree

‎src/basic_memory/mcp/clients/knowledge.py‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,18 @@ async def write_note(self, note: Entity, *, overwrite: bool) -> WriteNoteRespons
7575
)
7676
return write_note_response_adapter.validate_json(response.content)
7777

78-
async def create_entity(self, entity_data: dict[str, Any]) -> EntityResponse:
78+
async def create_entity(self, entity_data: dict[str, Any]) -> EntityResponseV2:
7979
"""Create a new entity.
8080
8181
Args:
8282
entity_data: Entity data including title, content, folder, etc.
8383
8484
Returns:
85-
EntityResponse with created entity details
85+
EntityResponseV2 with created entity details. The endpoint's
86+
response_model is EntityResponseV2 (it carries file_checksum /
87+
db_checksum, not the legacy EntityResponse.checksum key), so the
88+
response must be parsed as that type or those fields silently
89+
default to None (#1586).
8690
8791
Raises:
8892
ToolError: If the request fails
@@ -102,21 +106,22 @@ async def create_entity(self, entity_data: dict[str, Any]) -> EntityResponse:
102106
operation="create_entity",
103107
path_template="/v2/projects/{project_id}/knowledge/entities",
104108
)
105-
return EntityResponse.model_validate(response.json())
109+
return EntityResponseV2.model_validate(response.json())
106110

107111
async def update_entity(
108112
self,
109113
entity_id: str,
110114
entity_data: dict[str, Any],
111-
) -> EntityResponse:
115+
) -> EntityResponseV2:
112116
"""Update an existing entity (full replacement).
113117
114118
Args:
115119
entity_id: Entity external_id (UUID)
116120
entity_data: Complete entity data for replacement
117121
118122
Returns:
119-
EntityResponse with updated entity details
123+
EntityResponseV2 with updated entity details. See create_entity
124+
for why this must be EntityResponseV2, not EntityResponse (#1586).
120125
121126
Raises:
122127
ToolError: If the request fails
@@ -136,7 +141,7 @@ async def update_entity(
136141
operation="update_entity",
137142
path_template="/v2/projects/{project_id}/knowledge/entities/{entity_id}",
138143
)
139-
return EntityResponse.model_validate(response.json())
144+
return EntityResponseV2.model_validate(response.json())
140145

141146
async def get_entity(
142147
self,
@@ -215,15 +220,16 @@ async def patch_entity(
215220
self,
216221
entity_id: str,
217222
patch_data: dict[str, Any],
218-
) -> EntityResponse:
223+
) -> EntityResponseV2:
219224
"""Partially update an entity.
220225
221226
Args:
222227
entity_id: Entity external_id (UUID)
223228
patch_data: Partial entity data to update
224229
225230
Returns:
226-
EntityResponse with updated entity details
231+
EntityResponseV2 with updated entity details. See create_entity
232+
for why this must be EntityResponseV2, not EntityResponse (#1586).
227233
228234
Raises:
229235
ToolError: If the request fails
@@ -243,7 +249,7 @@ async def patch_entity(
243249
operation="patch_entity",
244250
path_template="/v2/projects/{project_id}/knowledge/entities/{entity_id}",
245251
)
246-
return EntityResponse.model_validate(response.json())
252+
return EntityResponseV2.model_validate(response.json())
247253

248254
async def delete_entity(self, entity_id: str) -> DeleteEntitiesResponse:
249255
"""Delete an entity.

‎src/basic_memory/mcp/tools/edit_note.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
from basic_memory.mcp.server import mcp
3333
from basic_memory.mcp.tools.utils import _extract_response_data, _response_detail_text
3434
from basic_memory.schemas.base import Entity
35-
from basic_memory.schemas.response import EntityResponse
35+
from basic_memory.schemas.v2.entity import EntityResponseV2
3636
from basic_memory.services.link_resolver import (
3737
detect_project_from_workspace_identifier_prefix,
3838
is_workspace_qualified_plain_identifier,
@@ -664,7 +664,7 @@ async def edit_note(
664664

665665
file_created = False
666666
entity_id = ""
667-
result: EntityResponse | None = None
667+
result: EntityResponseV2 | None = None
668668

669669
# Try to resolve the entity; for append/prepend, create it if not found
670670
try:
@@ -799,13 +799,18 @@ async def edit_note(
799799
# --- Format response ---
800800
# result is always set: either by create_entity (auto-create) or patch_entity (edit)
801801
assert result is not None
802+
# Report the accepted revision's checksum: it is recorded at accept
803+
# time and is what checksum-guarded edits compare base_checksum
804+
# against. file_checksum arrives later from deferred materialization
805+
# and can drift from it (#1586).
806+
checksum = result.db_checksum
802807
if file_created:
803808
summary = [
804809
f"# Created note ({operation})",
805810
f"project: {active_project.name}",
806811
f"file_path: {result.file_path}",
807812
f"permalink: {result.permalink}",
808-
f"checksum: {result.checksum[:8] if result.checksum else 'unknown'}",
813+
f"checksum: {checksum[:8] if checksum else 'unknown'}",
809814
"fileCreated: true",
810815
]
811816
lines_added = len(content.split("\n"))
@@ -816,7 +821,7 @@ async def edit_note(
816821
f"project: {active_project.name}",
817822
f"file_path: {result.file_path}",
818823
f"permalink: {result.permalink}",
819-
f"checksum: {result.checksum[:8] if result.checksum else 'unknown'}",
824+
f"checksum: {checksum[:8] if checksum else 'unknown'}",
820825
]
821826

822827
# Add operation-specific details
@@ -872,7 +877,7 @@ async def edit_note(
872877
"title": result.title,
873878
"permalink": result.permalink,
874879
"file_path": result.file_path,
875-
"checksum": result.checksum,
880+
"checksum": checksum,
876881
"operation": operation,
877882
"fileCreated": file_created,
878883
}

‎src/basic_memory/mcp/tools/write_note.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,12 +549,20 @@ async def write_note(
549549
for note in similar_notes
550550
]
551551

552+
# Report the accepted revision's checksum. It is recorded
553+
# synchronously at accept time, and it is the value checksum-guarded
554+
# edits compare base_checksum against. file_checksum is filled in
555+
# later by deferred materialization and can drift if the file is
556+
# edited outside Basic Memory, so it is not what a caller should
557+
# echo back (#1586).
558+
checksum = result.db_checksum
559+
552560
summary = [
553561
f"# {action} note",
554562
f"project: {active_project.name}",
555563
f"file_path: {result.file_path}",
556564
f"permalink: {response_permalink}",
557-
f"checksum: {result.file_checksum[:8] if result.file_checksum else 'unknown'}",
565+
f"checksum: {checksum[:8] if checksum else 'unknown'}",
558566
]
559567

560568
# Count observations by category
@@ -602,7 +610,7 @@ async def write_note(
602610
"title": result.title,
603611
"permalink": response_permalink,
604612
"file_path": result.file_path,
605-
"checksum": result.file_checksum,
613+
"checksum": checksum,
606614
"action": action.lower(),
607615
"similar_notes": [dataclasses.asdict(note) for note in similar_notes],
608616
}

‎test-int/mcp/test_edit_note_integration.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,45 @@ async def test_edit_note_append_operation(mcp_server, app, test_project):
6262
assert "This content was appended." in content
6363

6464

65+
@pytest.mark.asyncio
66+
async def test_edit_note_reports_real_checksum(mcp_server, app, test_project):
67+
"""edit_note must report the persisted checksum, not a permanent "unknown" (#1586).
68+
69+
A real SHA-256 checksum is computed and stored for every edit; the MCP
70+
response must surface it instead of always falling back to "unknown".
71+
"""
72+
73+
async with Client(mcp_server) as client:
74+
await client.call_tool(
75+
"write_note",
76+
{
77+
"project": test_project.name,
78+
"title": "Checksum Probe",
79+
"directory": "probe",
80+
"content": "# Checksum Probe\n\nOriginal content.",
81+
},
82+
)
83+
84+
edit_result = await client.call_tool(
85+
"edit_note",
86+
{
87+
"project": test_project.name,
88+
"identifier": "Checksum Probe",
89+
"operation": "append",
90+
"content": "\n\nAppended content.",
91+
},
92+
)
93+
94+
edit_text = edit_result.content[0].text
95+
assert "checksum: unknown" not in edit_text
96+
checksum_line = next(
97+
line for line in edit_text.splitlines() if line.startswith("checksum: ")
98+
)
99+
reported_checksum = checksum_line.removeprefix("checksum: ")
100+
assert len(reported_checksum) == 8
101+
assert all(c in "0123456789abcdef" for c in reported_checksum)
102+
103+
65104
@pytest.mark.asyncio
66105
async def test_edit_note_prepend_operation(mcp_server, app, test_project):
67106
"""Test prepending content to an existing note."""

‎tests/mcp/clients/test_clients.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ async def test_create_entity(self, monkeypatch):
3030
"""Test create_entity calls correct endpoint."""
3131

3232
mock_response = MagicMock()
33+
# The entity routes return EntityResponseV2, which always carries these ids.
3334
mock_response.json.return_value = {
35+
"id": 1,
36+
"external_id": "entity-uuid-123",
37+
"db_checksum": "d" * 64,
3438
"permalink": "test",
3539
"title": "Test",
3640
"file_path": "test.md",
@@ -53,13 +57,18 @@ async def mock_call_post(client, url, **kwargs):
5357
client = KnowledgeClient(mock_http, "proj-123")
5458
result = await client.create_entity({"title": "Test"})
5559
assert result.title == "Test"
60+
assert result.db_checksum == "d" * 64
5661

5762
@pytest.mark.asyncio
5863
async def test_update_entity(self, monkeypatch):
5964
"""Test update_entity calls correct endpoint without fast query params."""
6065

6166
mock_response = MagicMock()
67+
# The entity routes return EntityResponseV2, which always carries these ids.
6268
mock_response.json.return_value = {
69+
"id": 1,
70+
"external_id": "entity-uuid-123",
71+
"db_checksum": "d" * 64,
6372
"permalink": "test",
6473
"title": "Test",
6574
"file_path": "test.md",
@@ -82,13 +91,18 @@ async def mock_call_put(client, url, **kwargs):
8291
client = KnowledgeClient(mock_http, "proj-123")
8392
result = await client.update_entity("entity-123", {"title": "Test"})
8493
assert result.title == "Test"
94+
assert result.db_checksum == "d" * 64
8595

8696
@pytest.mark.asyncio
8797
async def test_patch_entity(self, monkeypatch):
8898
"""Test patch_entity calls correct endpoint without fast query params."""
8999

90100
mock_response = MagicMock()
101+
# The entity routes return EntityResponseV2, which always carries these ids.
91102
mock_response.json.return_value = {
103+
"id": 1,
104+
"external_id": "entity-uuid-123",
105+
"db_checksum": "d" * 64,
92106
"permalink": "test",
93107
"title": "Test",
94108
"file_path": "test.md",
@@ -111,6 +125,7 @@ async def mock_call_patch(client, url, **kwargs):
111125
client = KnowledgeClient(mock_http, "proj-123")
112126
result = await client.patch_entity("entity-123", {"operation": "append"})
113127
assert result.title == "Test"
128+
assert result.db_checksum == "d" * 64
114129

115130
@pytest.mark.asyncio
116131
async def test_resolve_entity(self, monkeypatch):

‎tests/mcp/test_tool_edit_note.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,3 +1699,42 @@ async def test_edit_note_append_traversal_identifier_json_error(client, test_pro
16991699
assert isinstance(result, dict)
17001700
assert result["error"] == "SECURITY_VALIDATION_ERROR"
17011701
assert result["fileCreated"] is False
1702+
1703+
1704+
@pytest.mark.asyncio
1705+
@pytest.mark.parametrize("file_checksum", [None, "c" * 64])
1706+
async def test_edit_note_reports_the_accepted_db_checksum(
1707+
monkeypatch, app, test_project, file_checksum
1708+
):
1709+
"""edit_note reports db_checksum, the value edit preconditions compare (#1586).
1710+
1711+
The PATCH response is replaced to inject a not-yet-materialized file and a
1712+
drifted file checksum, which the inline test environment never produces.
1713+
"""
1714+
await write_note(
1715+
project=test_project.name,
1716+
title="Deferred Edit Note",
1717+
directory="test",
1718+
content="# Deferred Edit Note\n\nOriginal body.",
1719+
)
1720+
1721+
db_checksum = "b" * 64 # a real, already-persisted SHA-256 hex digest
1722+
real_patch_entity = KnowledgeClient.patch_entity
1723+
1724+
async def fake_patch_entity(self, entity_id, patch_data):
1725+
result = await real_patch_entity(self, entity_id, patch_data)
1726+
return result.model_copy(
1727+
update={"file_checksum": file_checksum, "db_checksum": db_checksum}
1728+
)
1729+
1730+
monkeypatch.setattr(KnowledgeClient, "patch_entity", fake_patch_entity)
1731+
1732+
result = await edit_note(
1733+
project=test_project.name,
1734+
identifier="Deferred Edit Note",
1735+
operation="append",
1736+
content="\n\nAppended body.",
1737+
)
1738+
1739+
assert "checksum: unknown" not in result
1740+
assert f"checksum: {db_checksum[:8]}" in result

‎tests/mcp/test_tool_write_note.py‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Tests for note tools that exercise the full stack with SQLite."""
22

3+
from datetime import datetime, timezone
34
from textwrap import dedent
45
from typing import Any
56

@@ -9,6 +10,7 @@
910
from basic_memory import db
1011
from basic_memory import config as config_module
1112
from basic_memory.mcp import clients as clients_module
13+
from basic_memory.mcp.clients import KnowledgeClient
1214
from basic_memory.mcp.tools import write_note, read_note, delete_note
1315
from basic_memory.mcp.tools.write_note import (
1416
SIMILAR_NOTES_LIMIT,
@@ -19,6 +21,8 @@
1921
)
2022
from basic_memory.repository.relation_repository import RelationRepository
2123
from basic_memory.schemas.search import SearchItemType, SearchResponse, SearchResult
24+
from basic_memory.schemas.v2.entity import EntityResponseV2
25+
from basic_memory.schemas.v2.note_write import NoteCreated
2226
from basic_memory.workspace_context import workspace_permalink_context
2327

2428

@@ -229,6 +233,51 @@ async def test_write_note_no_tags(app, test_project):
229233
assert expected in content
230234

231235

236+
@pytest.mark.asyncio
237+
@pytest.mark.parametrize("file_checksum", [None, "c" * 64])
238+
async def test_write_note_reports_the_accepted_db_checksum(
239+
monkeypatch, app, test_project, file_checksum
240+
):
241+
"""write_note reports db_checksum, the value edit preconditions compare (#1586).
242+
243+
file_checksum is None until deferred materialization runs, and can differ
244+
from db_checksum after an outside edit; either way the response must carry
245+
the accepted revision's checksum. The client call is replaced to inject
246+
those two file states, which the inline test environment never produces.
247+
"""
248+
now = datetime.now(timezone.utc)
249+
db_checksum = "a" * 64 # a real, already-persisted SHA-256 hex digest
250+
251+
async def fake_write_note(self, note, *, overwrite):
252+
entity = EntityResponseV2(
253+
external_id="11111111-1111-1111-1111-111111111111",
254+
id=1,
255+
title=note.title,
256+
note_type=note.note_type,
257+
permalink=f"{test_project.name}/{note.directory}/{note.title.lower()}",
258+
file_path=f"{note.directory}/{note.title}.md",
259+
created_at=now,
260+
updated_at=now,
261+
db_version=1,
262+
db_checksum=db_checksum,
263+
file_version=None,
264+
file_checksum=file_checksum,
265+
)
266+
return NoteCreated(entity=entity)
267+
268+
monkeypatch.setattr(KnowledgeClient, "write_note", fake_write_note)
269+
270+
result = await write_note(
271+
project=test_project.name,
272+
title="Deferred Materialization Note",
273+
directory="test",
274+
content="# Deferred Materialization Note\n\nBody.",
275+
)
276+
277+
assert "checksum: unknown" not in result
278+
assert f"checksum: {db_checksum[:8]}" in result
279+
280+
232281
@pytest.mark.asyncio
233282
async def test_write_note_update_existing(app, test_project):
234283
"""Test creating a new note.

0 commit comments

Comments
 (0)