diff --git a/src/basic_memory/index/local_dependencies.py b/src/basic_memory/index/local_dependencies.py index 11c697a09..c01e97104 100644 --- a/src/basic_memory/index/local_dependencies.py +++ b/src/basic_memory/index/local_dependencies.py @@ -14,7 +14,12 @@ from basic_memory import db from basic_memory.config import BasicMemoryConfig, ConfigManager -from basic_memory.file_utils import FileMetadata, ParseError, remove_frontmatter +from basic_memory.file_utils import ( + FileMetadata, + ParseError, + compute_checksum, + remove_frontmatter, +) from basic_memory.indexing.batch_indexer import BatchIndexer from basic_memory.indexing.file_batch_runner import IndexFileBatchIndexer from basic_memory.indexing.file_index_checking import ( @@ -386,6 +391,7 @@ async def index_markdown_file( title=synced.entity.title, permalink=synced.entity.permalink, checksum=synced.checksum, + content_checksum=await compute_checksum(synced.markdown_content), operation=operation, content_superseded=content_superseded, ) @@ -455,6 +461,8 @@ async def index_regular_file( title=entity.title, permalink=entity.permalink, checksum=indexed.checksum, + # A regular file carries no note provenance to validate. + content_checksum=None, operation=operation, ) diff --git a/src/basic_memory/indexing/file_indexer.py b/src/basic_memory/indexing/file_indexer.py index bd9afd2f2..4d2b6144d 100644 --- a/src/basic_memory/indexing/file_indexer.py +++ b/src/basic_memory/indexing/file_indexer.py @@ -12,6 +12,7 @@ from basic_memory.indexing.file_index_checking import IndexedFileChecksumRow, MoveDetectionEntity from basic_memory import db +from basic_memory.file_utils import compute_checksum from basic_memory.indexing.note_content_reconciliation import ( NoteContentReconciliationAnchor, NoteContentReconciliationResult, @@ -265,6 +266,7 @@ async def index_markdown_file( title=synced.entity.title, permalink=synced.entity.permalink, checksum=synced.checksum, + content_checksum=await compute_checksum(synced.markdown_content), operation=operation, content_superseded=content_superseded, ) diff --git a/src/basic_memory/indexing/index_file_runner.py b/src/basic_memory/indexing/index_file_runner.py index 59f33f222..47f794817 100644 --- a/src/basic_memory/indexing/index_file_runner.py +++ b/src/basic_memory/indexing/index_file_runner.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, field from typing import Protocol, override +from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from basic_memory import db @@ -25,6 +26,7 @@ plan_current_materialized_note_result, plan_indexed_file_live_update_metadata, ) +from basic_memory.models import NoteContent from basic_memory.read_cache import ReadCacheInvalidator, invalidate_cache from basic_memory.runtime.jobs import RuntimeStorageFileIndexMode from basic_memory.runtime.note_object_metadata import RuntimeNoteObjectMetadataMap @@ -125,15 +127,21 @@ async def load_current_materialized_note_entity( file_path, load_relations=False, ) - if entity is None: - return None + if entity is None: + return None + # The markdown the index holds, as a content checksum: what a note object's + # bm-file-checksum must match before its provenance is trusted. + content_checksum = await session.scalar( + select(NoteContent.file_checksum).where(NoteContent.entity_id == entity.id) + ) return CurrentMaterializedNoteEntity.from_fields( entity_id=int(entity.id), external_id=entity.external_id, title=entity.title, permalink=entity.permalink, - checksum=entity.checksum, + storage_checksum=entity.checksum, + content_checksum=content_checksum, file_path=file_path, ) diff --git a/src/basic_memory/indexing/models.py b/src/basic_memory/indexing/models.py index cabfae261..51ec7e5a0 100644 --- a/src/basic_memory/indexing/models.py +++ b/src/basic_memory/indexing/models.py @@ -28,9 +28,8 @@ from basic_memory.runtime.note_object_metadata import ( RuntimeNoteObjectMetadataMap, RuntimeNoteObjectProvenance, - RuntimeStorageObjectChecksumSource, db_version_from_object_metadata, - storage_object_checksum_for_index_match, + file_checksum_from_object_metadata, ) from basic_memory.runtime.storage import ( NoteExternalId, @@ -41,6 +40,7 @@ RuntimeNoteActorKind, RuntimeNoteActorName, RuntimeNoteChangeSource, + RuntimeNoteContentChecksum, StorageEtag, normalize_storage_etag, ) @@ -226,7 +226,12 @@ class FileIndexResult: external_id: str title: str permalink: str | None + # The storage checksum of the bytes indexed (an S3 ETag in cloud, the content + # sha256 locally); recorded as entity.checksum. checksum: str + # sha256 of the markdown indexed, comparable with a note object's bm-file-checksum; + # None for regular (non-note) files. + content_checksum: RuntimeNoteContentChecksum | None operation: FileIndexOperation # The indexed snapshot committed successfully, but a newer accepted note # generation won before derived relations could be published. The next @@ -243,6 +248,7 @@ def from_fields( title: object, permalink: object, checksum: str, + content_checksum: RuntimeNoteContentChecksum | None, operation: FileIndexOperation, content_superseded: bool = False, ) -> FileIndexResult: @@ -271,6 +277,7 @@ def from_fields( file_path=file_path, ), checksum=checksum, + content_checksum=content_checksum, operation=operation, content_superseded=content_superseded, ) @@ -332,9 +339,9 @@ class IndexFileJobResult: # echo-vs-out-of-band by version arithmetic. None when the metadata was # absent or failed checksum validation. db_version: int | None = None - # True when the object carried our own bm-file-checksum metadata that no - # longer matches what this job indexed: a newer own-stack write landed - # mid-job, so this result describes superseded content (issue #1445). + # True when the stored object changed after this job read it: a newer write + # landed mid-job, so this result describes superseded content (issue #1445). + # That write's own storage notification indexes the newer content. content_superseded: bool = False @@ -569,7 +576,10 @@ class CurrentMaterializedNoteEntity: external_id: str title: str permalink: str - checksum: RuntimeFileChecksum | None + # entity.checksum: the storage checksum of the indexed object. + storage_checksum: RuntimeFileChecksum | None + # note_content.file_checksum: sha256 of the markdown the index holds. + content_checksum: RuntimeNoteContentChecksum | None @classmethod def from_fields( @@ -579,7 +589,8 @@ def from_fields( external_id: object, title: object, permalink: object, - checksum: object, + storage_checksum: object, + content_checksum: object, file_path: str, ) -> CurrentMaterializedNoteEntity: """Validate entity fields loaded for a current materialized note. @@ -605,7 +616,8 @@ def from_fields( field_name="permalink", file_path=file_path, ), - checksum=str(checksum) if checksum is not None else None, + storage_checksum=str(storage_checksum) if storage_checksum is not None else None, + content_checksum=str(content_checksum) if content_checksum is not None else None, ) @@ -622,34 +634,24 @@ def _required_current_materialized_note_text( @dataclass(frozen=True, slots=True) class CurrentMaterializedNotePlan: - """Planned current-file result plus checksum diagnostics for adapter logging.""" + """Planned current-file result, or a request to load the indexed entity first.""" job_result: IndexFileJobResult requires_entity: bool = False - object_checksum_source: RuntimeStorageObjectChecksumSource | None = None - object_checksum: RuntimeFileChecksum | None = None - entity_checksum: RuntimeFileChecksum | None = None - source: RuntimeNoteChangeSource | None = None - checksum_matches_entity: bool | None = None @dataclass(frozen=True, slots=True) class IndexedFileLiveUpdatePlan: - """Trusted live-update metadata for one freshly indexed file.""" + """Supersession and trusted provenance for one freshly indexed file. - object_checksum_source: RuntimeStorageObjectChecksumSource - object_checksum: RuntimeFileChecksum - indexed_checksum: RuntimeFileChecksum - checksum_matches_indexed_file: bool - content_superseded: bool = False - metadata_actor_user_profile_id: str | None = None - metadata_actor_kind: str | None = None - metadata_actor_name: str | None = None - metadata_source: RuntimeNoteChangeSource | None = None + Provenance fields are set only when the object's bm-file-checksum matches the + content this job indexed (#1589). + """ + + content_superseded: bool actor_user_profile_id: str | None = None actor_kind: str | None = None actor_name: str | None = None - # Trusted-branch only, like the actor fields (#1589). db_version: int | None = None live_update_source: RuntimeNoteChangeSource | None = None operation: FileIndexOperation | None = None @@ -672,33 +674,22 @@ def plan_current_materialized_note_result( live_update_operation = file_index_operation_from_note_object_metadata(object_metadata) if live_update_operation is None: - return CurrentMaterializedNotePlan( - job_result=current_result, - source=provenance.source, - ) + return CurrentMaterializedNotePlan(job_result=current_result) if entity is None: - return CurrentMaterializedNotePlan( - job_result=current_result, - requires_entity=True, - source=provenance.source, - ) - - selected_checksum = storage_object_checksum_for_index_match( - object_checksum=object_checksum, - object_metadata=object_metadata, + return CurrentMaterializedNotePlan(job_result=current_result, requires_entity=True) + + # Each comparison stays within one kind of checksum. The storage checksums say the + # index holds this object; the content checksums say the object's provenance + # describes the markdown the index holds, so it can be trusted (#1589). + object_content_checksum = file_checksum_from_object_metadata(object_metadata) + provenance_describes_indexed_note = ( + object_checksum == entity.storage_checksum + and object_content_checksum is not None + and object_content_checksum == entity.content_checksum ) - checksum_matches_entity = selected_checksum.checksum == entity.checksum - plan = CurrentMaterializedNotePlan( - job_result=current_result, - object_checksum_source=selected_checksum.source, - object_checksum=selected_checksum.checksum, - entity_checksum=entity.checksum, - source=provenance.source, - checksum_matches_entity=checksum_matches_entity, - ) - if not checksum_matches_entity: - return plan + if not provenance_describes_indexed_note: + return CurrentMaterializedNotePlan(job_result=current_result) return CurrentMaterializedNotePlan( job_result=IndexFileJobResult( @@ -708,7 +699,7 @@ def plan_current_materialized_note_result( note_external_id=entity.external_id, title=entity.title, permalink=entity.permalink, - entity_checksum=entity.checksum, + entity_checksum=entity.storage_checksum, operation=live_update_operation, actor_user_profile_id=provenance.actor_user_profile_id, actor_kind=provenance.actor_kind, @@ -716,11 +707,6 @@ def plan_current_materialized_note_result( live_update_source=provenance.source, db_version=provenance.db_version, ), - object_checksum_source=plan.object_checksum_source, - object_checksum=plan.object_checksum, - entity_checksum=plan.entity_checksum, - source=plan.source, - checksum_matches_entity=True, ) @@ -730,45 +716,25 @@ def plan_indexed_file_live_update_metadata( object_checksum: RuntimeFileChecksum, object_metadata: RuntimeNoteObjectMetadataMap | None, ) -> IndexedFileLiveUpdatePlan: - """Plan trusted actor/source metadata for a freshly indexed file.""" - selected_checksum = storage_object_checksum_for_index_match( - object_checksum=object_checksum, - object_metadata=object_metadata, - ) - provenance = RuntimeNoteObjectProvenance.from_object_metadata(object_metadata) - checksum_matches_indexed_file = selected_checksum.checksum == indexed_file.checksum - # Our own stack stamped the object with a content checksum and it no longer - # matches what this job indexed: a newer own-stack write landed mid-job (its - # webhook job is queued), so this job indexed superseded content. An etag - # mismatch proves nothing (etags never equal content sha256s), so external - # writes are unaffected. - content_superseded = ( - not checksum_matches_indexed_file - and selected_checksum.source == RuntimeStorageObjectChecksumSource.note_file_checksum - ) - plan = IndexedFileLiveUpdatePlan( - object_checksum_source=selected_checksum.source, - object_checksum=selected_checksum.checksum, - indexed_checksum=indexed_file.checksum, - checksum_matches_indexed_file=checksum_matches_indexed_file, - content_superseded=content_superseded, - metadata_actor_user_profile_id=provenance.actor_user_profile_id, - metadata_actor_kind=provenance.actor_kind, - metadata_actor_name=provenance.actor_name, - metadata_source=provenance.source, - ) - if not checksum_matches_indexed_file: - return plan + """Plan supersession and trusted actor/source metadata for a freshly indexed file. + `object_checksum` is the storage checksum of the object as it stands after indexing, + the same kind as `indexed_file.checksum` on every backend. + """ + # A different stored object means a newer write replaced the file after this job read + # it. That write's own storage notification indexes the newer content, whoever wrote it. + if object_checksum != indexed_file.checksum: + return IndexedFileLiveUpdatePlan(content_superseded=True) + + # Object metadata is trusted only when its content checksum names the markdown this + # job indexed (#1589); a sync client can re-upload different bytes with stale metadata. + object_content_checksum = file_checksum_from_object_metadata(object_metadata) + if object_content_checksum is None or object_content_checksum != indexed_file.content_checksum: + return IndexedFileLiveUpdatePlan(content_superseded=False) + + provenance = RuntimeNoteObjectProvenance.from_object_metadata(object_metadata) return IndexedFileLiveUpdatePlan( - object_checksum_source=plan.object_checksum_source, - object_checksum=plan.object_checksum, - indexed_checksum=plan.indexed_checksum, - checksum_matches_indexed_file=True, - metadata_actor_user_profile_id=plan.metadata_actor_user_profile_id, - metadata_actor_kind=plan.metadata_actor_kind, - metadata_actor_name=plan.metadata_actor_name, - metadata_source=plan.metadata_source, + content_superseded=False, actor_user_profile_id=provenance.actor_user_profile_id, actor_kind=provenance.actor_kind, actor_name=provenance.actor_name, diff --git a/src/basic_memory/indexing/note_materialization_runner.py b/src/basic_memory/indexing/note_materialization_runner.py index 3b900ab82..f0bd3b9e8 100644 --- a/src/basic_memory/indexing/note_materialization_runner.py +++ b/src/basic_memory/indexing/note_materialization_runner.py @@ -643,6 +643,11 @@ async def publish_written_file_state( { "mtime": written_file.file_updated_at.timestamp(), "size": len(prepared_write.markdown_content.encode("utf-8")), + # The accepted note was indexed when it was saved, so the stored + # object now matches the index. Recording its storage checksum + # lets the storage notification for this write find the file + # current instead of reading and re-indexing it. + "checksum": written_file.storage_checksum, # The file now holds accepted content, so a sync client's original # is stale. Recognizing it after this would let a restore silently # revert the edit on storage while the index keeps it. diff --git a/src/basic_memory/runtime/note_materialization.py b/src/basic_memory/runtime/note_materialization.py index de64991dc..3bfbd19dd 100644 --- a/src/basic_memory/runtime/note_materialization.py +++ b/src/basic_memory/runtime/note_materialization.py @@ -40,8 +40,13 @@ class RuntimeWrittenFileState: """Object state returned after storage accepts a materialized note write.""" file_path: RuntimeFilePath + # sha256 of the accepted markdown: the note_content file lineage. file_checksum: RuntimeFileChecksum file_updated_at: datetime + # The checksum storage reports for the written object (an S3 ETag in cloud, the + # content sha256 on a local filesystem). Indexing compares this kind against + # entity.checksum, so the publisher records it there. + storage_checksum: RuntimeFileChecksum class RuntimeFileMetadataSource(Protocol): @@ -118,6 +123,8 @@ async def write_prepared_note_to_content_store( file_path=prepared_write.file_path, file_checksum=actual_checksum, file_updated_at=file_metadata.modified_at, + # A local content store's storage checksum is the content sha256. + storage_checksum=actual_checksum, ) # A sync client may have restored the original over indexing's frontmatter rewrite. @@ -148,4 +155,5 @@ async def write_prepared_note_to_content_store( file_path=prepared_write.file_path, file_checksum=file_checksum, file_updated_at=file_metadata.modified_at, + storage_checksum=file_checksum, ) diff --git a/src/basic_memory/runtime/note_object_metadata.py b/src/basic_memory/runtime/note_object_metadata.py index 8e489e7e5..c2886a0d4 100644 --- a/src/basic_memory/runtime/note_object_metadata.py +++ b/src/basic_memory/runtime/note_object_metadata.py @@ -5,13 +5,11 @@ import re from collections.abc import Mapping from dataclasses import dataclass -from enum import StrEnum from typing import Self from uuid import UUID from basic_memory.runtime.storage import ( RuntimeEntityId, - RuntimeFileChecksum, RuntimeNoteActorKind, RuntimeNoteActorName, RuntimeNoteChangeSource, @@ -83,21 +81,6 @@ _MAX_ACTOR_NAME_LENGTH = 120 -class RuntimeStorageObjectChecksumSource(StrEnum): - """Storage checksum source used when matching an indexed file to object metadata.""" - - note_file_checksum = "bm-file-checksum" - storage_etag = "etag" - - -@dataclass(frozen=True, slots=True) -class RuntimeStorageObjectChecksum: - """Checksum selected for comparing an indexed file to a storage object.""" - - checksum: RuntimeFileChecksum - source: RuntimeStorageObjectChecksumSource - - @dataclass(frozen=True, slots=True) class RuntimeNoteObjectMetadata: """Metadata written onto a materialized note object.""" @@ -188,8 +171,12 @@ def actor_name_from_object_metadata( def file_checksum_from_object_metadata( metadata: RuntimeNoteObjectMetadataMap | None, -) -> RuntimeFileChecksum | None: - """Return the Basic Memory content checksum mirrored onto a note object.""" +) -> RuntimeNoteContentChecksum | None: + """Return the sha256 of the note markdown mirrored onto a note object (bm-file-checksum). + + This is a content checksum. It is never comparable with a storage checksum such as + an S3 ETag. + """ if not metadata: return None @@ -201,24 +188,6 @@ def file_checksum_from_object_metadata( return stripped or None -def storage_object_checksum_for_index_match( - *, - object_checksum: RuntimeFileChecksum, - object_metadata: RuntimeNoteObjectMetadataMap | None, -) -> RuntimeStorageObjectChecksum: - """Return the checksum that should match an indexed markdown file.""" - file_checksum = file_checksum_from_object_metadata(object_metadata) - if file_checksum is not None: - return RuntimeStorageObjectChecksum( - checksum=file_checksum, - source=RuntimeStorageObjectChecksumSource.note_file_checksum, - ) - return RuntimeStorageObjectChecksum( - checksum=object_checksum, - source=RuntimeStorageObjectChecksumSource.storage_etag, - ) - - def db_version_from_object_metadata( metadata: RuntimeNoteObjectMetadataMap | None, ) -> RuntimeNoteContentVersion | None: diff --git a/test-int/read_cache/test_materialization_invalidation.py b/test-int/read_cache/test_materialization_invalidation.py index ec9eb6db6..2c2ac9a3c 100644 --- a/test-int/read_cache/test_materialization_invalidation.py +++ b/test-int/read_cache/test_materialization_invalidation.py @@ -67,6 +67,7 @@ async def index_file(self, file_path: str, *, source: str) -> FileIndexResult: title=self.entity.title, permalink=self.entity.permalink, checksum=checksum, + content_checksum=None, operation=FileIndexOperation.updated, ) diff --git a/test-int/test_materialized_note_index_gate.py b/test-int/test_materialized_note_index_gate.py new file mode 100644 index 000000000..c1cf57419 --- /dev/null +++ b/test-int/test_materialized_note_index_gate.py @@ -0,0 +1,106 @@ +"""A materialized note is already indexed: its storage notification must not re-read it.""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest +from sqlalchemy import select + +from basic_memory import db +from basic_memory.indexing.file_index_checking import indexed_checksums_by_path +from basic_memory.indexing.note_materialization_runner import ( + RepositoryNoteMaterializationPublisher, +) +from basic_memory.models import Entity, NoteContent, Project +from basic_memory.repository.entity_repository import EntityRepository +from basic_memory.repository.note_content_repository import NoteContentRepository +from basic_memory.runtime.note_content import ( + RuntimeNoteMaterializationJobRequest, + RuntimeNoteMaterializationStatus, +) +from basic_memory.runtime.note_materialization import ( + RuntimeWrittenFileState, + plan_prepared_note_write, +) + + +@pytest.mark.asyncio +async def test_published_materialization_is_recognized_by_the_index_gate( + engine_factory, + test_project: Project, +) -> None: + """The index gate compares the stored object's checksum with entity.checksum. + + Cloud storage reports an S3 ETag, which never equals the markdown's sha256, so the + publisher must record the written object's storage checksum, not the content one. + """ + _, session_maker = engine_factory + file_path = "notes/materialized.md" + markdown = "# Materialized\n" + async with db.scoped_session(session_maker) as session: + entity = Entity( + project_id=test_project.id, + title="Materialized", + note_type="note", + content_type="text/markdown", + file_path=file_path, + permalink="notes/materialized", + checksum="etag-of-previous-object", + ) + session.add(entity) + await session.flush() + entity_id = entity.id + await NoteContentRepository(project_id=test_project.id).create( + session, + NoteContent( + entity_id=entity_id, + markdown_content=markdown, + db_version=2, + db_checksum="content-sha256-v2", + file_version=1, + file_checksum="content-sha256-v1", + file_write_status="writing", + ), + ) + + request = RuntimeNoteMaterializationJobRequest( + project_id=test_project.id, + entity_id=entity_id, + db_version=2, + db_checksum="content-sha256-v2", + source="web_v2", + ) + prepared_write = plan_prepared_note_write( + request=request, + file_path=file_path, + markdown_content=markdown, + previous_file_checksum="content-sha256-v1", + previous_sync_checksum=None, + attempted_at=datetime(2026, 10, 7, 3, 0, tzinfo=UTC), + ) + written_file = RuntimeWrittenFileState( + file_path=file_path, + file_checksum="content-sha256-v2", + file_updated_at=datetime(2026, 10, 7, 3, 0, 1, tzinfo=UTC), + storage_checksum="etag-of-written-object", + ) + + result = await RepositoryNoteMaterializationPublisher( + session_maker=session_maker + ).publish_written_file_state(request, prepared_write, written_file) + + assert result.status is RuntimeNoteMaterializationStatus.written + async with db.scoped_session(session_maker) as session: + stored_entity = await session.scalar(select(Entity).where(Entity.id == entity_id)) + rows = await EntityRepository(project_id=test_project.id).get_by_file_paths( + session, [file_path] + ) + assert stored_entity is not None + assert stored_entity.checksum == "etag-of-written-object" + assert stored_entity.sync_checksum is None + + # The storage notification for this write carries the written object's ETag. + indexed = indexed_checksums_by_path(rows)[file_path] + assert indexed.recognizes("etag-of-written-object") + assert not indexed.recognizes("content-sha256-v2") diff --git a/test-int/test_note_materialization_lock_order.py b/test-int/test_note_materialization_lock_order.py index b8fcd2ad2..fb7410794 100644 --- a/test-int/test_note_materialization_lock_order.py +++ b/test-int/test_note_materialization_lock_order.py @@ -108,6 +108,7 @@ async def test_materialization_and_accepted_mutation_share_note_content_first_or file_path=file_path, file_checksum="materialized-checksum", file_updated_at=datetime(2026, 8, 5, 1, 1, tzinfo=UTC), + storage_checksum="materialized-etag", ) session_lock = StartedMaterializationLock() publisher = RepositoryNoteMaterializationPublisher( @@ -254,6 +255,7 @@ async def test_publish_cas_loss_never_reverts_newer_accepted_write( file_path=file_path, file_checksum="stale-materialized-checksum", file_updated_at=datetime(2026, 8, 5, 2, 1, tzinfo=UTC), + storage_checksum="stale-materialized-etag", ) session_lock = StartedMaterializationLock() publisher = RepositoryNoteMaterializationPublisher( diff --git a/tests/cloud/test_note_content_materialization.py b/tests/cloud/test_note_content_materialization.py index 91d7915e7..018f5eec9 100644 --- a/tests/cloud/test_note_content_materialization.py +++ b/tests/cloud/test_note_content_materialization.py @@ -65,6 +65,7 @@ async def index_file(self, file_path: str, *, source: str) -> FileIndexResult: title="Test note", permalink="notes/test", checksum="indexed-checksum", + content_checksum=None, operation=FileIndexOperation.updated, ) diff --git a/tests/index/test_inline_storage_event_processor.py b/tests/index/test_inline_storage_event_processor.py index d0d113944..ac2840a30 100644 --- a/tests/index/test_inline_storage_event_processor.py +++ b/tests/index/test_inline_storage_event_processor.py @@ -110,6 +110,7 @@ async def index_file(self, file_path: str, *, source: str) -> FileIndexResult: title="Note 42", permalink="notes/note-42", checksum="checksum-42", + content_checksum=None, operation=FileIndexOperation.created, ) diff --git a/tests/index/test_local_markdown_file_indexer.py b/tests/index/test_local_markdown_file_indexer.py index 5b459db1d..9ccd6b3bf 100644 --- a/tests/index/test_local_markdown_file_indexer.py +++ b/tests/index/test_local_markdown_file_indexer.py @@ -136,6 +136,7 @@ async def test_local_file_indexer_treats_empty_markdown_basename_as_regular_file title=".md", permalink=None, checksum="abc123", + content_checksum=None, operation=FileIndexOperation.created, ) markdown_index = AsyncMock() diff --git a/tests/index/test_local_project_index.py b/tests/index/test_local_project_index.py index 34fc6b986..da57e7faa 100644 --- a/tests/index/test_local_project_index.py +++ b/tests/index/test_local_project_index.py @@ -2527,6 +2527,7 @@ async def index_file(self, file_path: str, *, source: str) -> FileIndexResult: title="Note 99", permalink="notes/note-99", checksum="indexed-checksum", + content_checksum=None, operation=FileIndexOperation.updated, ) diff --git a/tests/indexing/test_file_indexer.py b/tests/indexing/test_file_indexer.py index fc64ca054..3bcdacec0 100644 --- a/tests/indexing/test_file_indexer.py +++ b/tests/indexing/test_file_indexer.py @@ -97,6 +97,7 @@ def _file_indexer( title="Note", permalink="notes/note", checksum=CHECKSUM, + content_checksum=None, operation=FileIndexOperation.updated, ) ) diff --git a/tests/indexing/test_index_file_runner.py b/tests/indexing/test_index_file_runner.py index 74ab047b0..678f5e1d0 100644 --- a/tests/indexing/test_index_file_runner.py +++ b/tests/indexing/test_index_file_runner.py @@ -83,7 +83,17 @@ class FakeRepositoryEntity: external_id = "note-42" title = "Repository Note" permalink = "notes/repository-note" - checksum = "checksum-1" + checksum = "storage-native-etag" + + +class FakeNoteContentSession: + """Answers the loader's note_content.file_checksum lookup.""" + + def __init__(self, content_checksum: str | None) -> None: + self.content_checksum = content_checksum + + async def scalar(self, _statement: object) -> str | None: + return self.content_checksum class FakeEntityRepository: @@ -169,7 +179,10 @@ def indexed_file() -> FileIndexResult: external_id="note-42", title="Test Note", permalink="notes/test-note", - checksum="checksum-1", + # Cloud shape: the storage checksum is the object's ETag, the content + # checksum is the sha256 the object's bm-file-checksum names. + checksum="storage-native-etag", + content_checksum="checksum-1", operation=FileIndexOperation.updated, ) @@ -187,7 +200,7 @@ async def fake_scoped_session( scoped_session_maker: async_sessionmaker[AsyncSession], ) -> AsyncIterator[AsyncSession]: assert scoped_session_maker is session_maker - session = cast(AsyncSession, object()) + session = cast(AsyncSession, FakeNoteContentSession("checksum-1")) sessions.append(session) yield session @@ -205,7 +218,8 @@ async def fake_scoped_session( external_id="note-42", title="Repository Note", permalink="notes/repository-note", - checksum="checksum-1", + storage_checksum="storage-native-etag", + content_checksum="checksum-1", ) assert repository.calls == [(sessions[0], "notes/a.md", False)] @@ -257,7 +271,8 @@ async def test_run_index_file_preserves_current_materialized_note_metadata() -> external_id="note-42", title="Created through MCP", permalink="notes/created-through-mcp", - checksum="checksum-1", + storage_checksum="storage-native-etag", + content_checksum="checksum-1", ) ) file_indexer = FakeFileIndexer() @@ -277,7 +292,7 @@ async def test_run_index_file_preserves_current_materialized_note_metadata() -> note_external_id="note-42", title="Created through MCP", permalink="notes/created-through-mcp", - entity_checksum="checksum-1", + entity_checksum="storage-native-etag", operation=FileIndexOperation.created, actor_user_profile_id="33333333-3333-3333-3333-333333333333", live_update_source="mcp", diff --git a/tests/indexing/test_models.py b/tests/indexing/test_models.py index a7d99ef90..9de9fbdc9 100644 --- a/tests/indexing/test_models.py +++ b/tests/indexing/test_models.py @@ -54,7 +54,6 @@ NOTE_OBJECT_DB_VERSION_METADATA, NOTE_OBJECT_FILE_CHECKSUM_METADATA, NOTE_OBJECT_SOURCE_METADATA, - RuntimeStorageObjectChecksumSource, ) from basic_memory.runtime.projects import ProjectRuntimeReference from basic_memory.runtime.storage import ( @@ -72,6 +71,7 @@ def test_file_index_result_is_a_frozen_success_value(): title="A Note", permalink="notes/a-note", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -89,6 +89,7 @@ def test_file_index_result_from_fields_validates_required_entity_text(): title=" A Note ", permalink=" notes/a-note ", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -99,6 +100,7 @@ def test_file_index_result_from_fields_validates_required_entity_text(): title="A Note", permalink="notes/a-note", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -110,6 +112,7 @@ def test_file_index_result_from_fields_validates_required_entity_text(): title="", permalink="notes/a-note", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -122,6 +125,7 @@ def test_file_index_result_from_fields_validates_optional_permalink_text(): title="A Note", permalink=None, checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -135,6 +139,7 @@ def test_file_index_result_from_fields_validates_optional_permalink_text(): title="A Note", permalink=123, checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -146,6 +151,7 @@ def test_file_index_result_from_fields_validates_optional_permalink_text(): title="A Note", permalink=" ", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.created, ) @@ -217,13 +223,11 @@ def test_index_file_job_result_from_indexed_file_uses_trusted_live_update_plan() title="A Note", permalink="notes/a-note", checksum="checksum-1", + content_checksum="content-1", operation=FileIndexOperation.updated, ) live_update_plan = IndexedFileLiveUpdatePlan( - object_checksum_source=RuntimeStorageObjectChecksumSource.note_file_checksum, - object_checksum="checksum-1", - indexed_checksum="checksum-1", - checksum_matches_indexed_file=True, + content_superseded=False, actor_user_profile_id="33333333-3333-3333-3333-333333333333", actor_kind=NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, actor_name="Claude Code", @@ -691,7 +695,8 @@ def test_current_materialized_note_entity_from_fields_requires_indexed_permalink external_id="note-42", title="A Note", permalink=None, - checksum="checksum-1", + storage_checksum="etag-1", + content_checksum="content-1", file_path="notes/a.md", ) @@ -702,7 +707,8 @@ def test_current_materialized_note_entity_from_fields_validates_identity_text(): external_id=" note-42 ", title=" A Note ", permalink=" notes/a-note ", - checksum="checksum-1", + storage_checksum="etag-1", + content_checksum="content-1", file_path="notes/a.md", ) @@ -711,19 +717,22 @@ def test_current_materialized_note_entity_from_fields_validates_identity_text(): external_id="note-42", title="A Note", permalink="notes/a-note", - checksum="checksum-1", + storage_checksum="etag-1", + content_checksum="content-1", ) - no_checksum = CurrentMaterializedNoteEntity.from_fields( + not_yet_indexed = CurrentMaterializedNoteEntity.from_fields( entity_id=42, external_id="note-42", title="A Note", permalink="notes/a-note", - checksum=None, + storage_checksum=None, + content_checksum=None, file_path="notes/a.md", ) - assert no_checksum.checksum is None + assert not_yet_indexed.storage_checksum is None + assert not_yet_indexed.content_checksum is None with pytest.raises(RuntimeError, match="Current entity for notes/a.md is missing title"): CurrentMaterializedNoteEntity.from_fields( @@ -731,34 +740,44 @@ def test_current_materialized_note_entity_from_fields_validates_identity_text(): external_id="note-42", title=" ", permalink="notes/a-note", - checksum="checksum-1", + storage_checksum="etag-1", + content_checksum="content-1", file_path="notes/a.md", ) -def test_plan_current_materialized_note_result_preserves_trusted_live_update_metadata(): - entity = CurrentMaterializedNoteEntity.from_fields( +# Cloud shape for every planner test below: storage checksums are S3 ETags +# ("etag-*"); bm-file-checksum and indexed content checksums are content sha256s +# ("content-*"). The two kinds never equal each other. +MCP_NOTE_OBJECT_METADATA = { + NOTE_OBJECT_FILE_CHECKSUM_METADATA: "content-1", + NOTE_OBJECT_ACTOR_USER_PROFILE_ID_METADATA: "33333333-3333-3333-3333-333333333333", + NOTE_OBJECT_ACTOR_KIND_METADATA: NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, + NOTE_OBJECT_ACTOR_NAME_METADATA: "Claude Code", + NOTE_OBJECT_SOURCE_METADATA: "mcp", + NOTE_OBJECT_DB_VERSION_METADATA: "1", +} + + +def materialized_entity(*, storage_checksum: str, content_checksum: str | None): + return CurrentMaterializedNoteEntity( entity_id=42, external_id="note-42", title="A Note", permalink="notes/a-note", - checksum="checksum-1", - file_path="notes/a.md", + storage_checksum=storage_checksum, + content_checksum=content_checksum, ) + +def test_plan_current_materialized_note_result_trusts_provenance_of_the_indexed_object(): + """The index holds this object and its bm-file-checksum names the indexed markdown.""" plan = plan_current_materialized_note_result( reason="file already indexed: notes/a.md", file_path="notes/a.md", - object_checksum="storage-native-etag", - object_metadata={ - NOTE_OBJECT_FILE_CHECKSUM_METADATA: "checksum-1", - NOTE_OBJECT_ACTOR_USER_PROFILE_ID_METADATA: ("33333333-3333-3333-3333-333333333333"), - NOTE_OBJECT_ACTOR_KIND_METADATA: NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, - NOTE_OBJECT_ACTOR_NAME_METADATA: "Claude Code", - NOTE_OBJECT_SOURCE_METADATA: "mcp", - NOTE_OBJECT_DB_VERSION_METADATA: "1", - }, - entity=entity, + object_checksum="etag-1", + object_metadata=MCP_NOTE_OBJECT_METADATA, + entity=materialized_entity(storage_checksum="etag-1", content_checksum="content-1"), ) assert plan == CurrentMaterializedNotePlan( @@ -769,7 +788,7 @@ def test_plan_current_materialized_note_result_preserves_trusted_live_update_met note_external_id="note-42", title="A Note", permalink="notes/a-note", - entity_checksum="checksum-1", + entity_checksum="etag-1", operation=FileIndexOperation.created, actor_user_profile_id="33333333-3333-3333-3333-333333333333", actor_kind=NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, @@ -777,33 +796,51 @@ def test_plan_current_materialized_note_result_preserves_trusted_live_update_met live_update_source="mcp", db_version=1, ), - object_checksum_source=RuntimeStorageObjectChecksumSource.note_file_checksum, - object_checksum="checksum-1", - entity_checksum="checksum-1", - source="mcp", - checksum_matches_entity=True, ) -def test_plan_current_materialized_note_result_omits_ambiguous_metadata(): - entity = CurrentMaterializedNoteEntity.from_fields( - entity_id=42, - external_id="note-42", - title="A Note", - permalink="notes/a-note", - checksum="checksum-1", +@pytest.mark.parametrize( + ("storage_checksum", "content_checksum"), + [ + # Stale bm-* metadata: the index holds different markdown than it names. + ("etag-1", "content-2"), + # The index holds a different object than this one. + ("etag-2", "content-1"), + # The note's markdown lineage is not recorded yet. + ("etag-1", None), + ], +) +def test_plan_current_materialized_note_result_withholds_untrusted_provenance( + storage_checksum: str, content_checksum: str | None +): + plan = plan_current_materialized_note_result( + reason="file already indexed: notes/a.md", file_path="notes/a.md", + object_checksum="etag-1", + object_metadata=MCP_NOTE_OBJECT_METADATA, + entity=materialized_entity( + storage_checksum=storage_checksum, content_checksum=content_checksum + ), ) + assert plan == CurrentMaterializedNotePlan( + job_result=IndexFileJobResult( + status=IndexFileJobStatus.current, + reason="file already indexed: notes/a.md", + ), + ) + + +def test_plan_current_materialized_note_result_omits_ambiguous_metadata(): plan = plan_current_materialized_note_result( reason="file already indexed: notes/a.md", file_path="notes/a.md", - object_checksum="storage-native-etag", + object_checksum="etag-1", object_metadata={ - NOTE_OBJECT_FILE_CHECKSUM_METADATA: "checksum-1", + NOTE_OBJECT_FILE_CHECKSUM_METADATA: "content-1", NOTE_OBJECT_SOURCE_METADATA: "mcp", }, - entity=entity, + entity=materialized_entity(storage_checksum="etag-1", content_checksum="content-1"), ) assert plan == CurrentMaterializedNotePlan( @@ -811,7 +848,6 @@ def test_plan_current_materialized_note_result_omits_ambiguous_metadata(): status=IndexFileJobStatus.current, reason="file already indexed: notes/a.md", ), - source="mcp", ) @@ -819,9 +855,9 @@ def test_plan_current_materialized_note_result_requests_entity_when_metadata_is_ plan = plan_current_materialized_note_result( reason="file already indexed: notes/a.md", file_path="notes/a.md", - object_checksum="storage-native-etag", + object_checksum="etag-1", object_metadata={ - NOTE_OBJECT_FILE_CHECKSUM_METADATA: "checksum-1", + NOTE_OBJECT_FILE_CHECKSUM_METADATA: "content-1", NOTE_OBJECT_SOURCE_METADATA: "mcp", NOTE_OBJECT_DB_VERSION_METADATA: "2", }, @@ -834,43 +870,36 @@ def test_plan_current_materialized_note_result_requests_entity_when_metadata_is_ reason="file already indexed: notes/a.md", ), requires_entity=True, - source="mcp", ) -def test_plan_indexed_file_live_update_metadata_preserves_matching_metadata(): - indexed_file = FileIndexResult( +def indexed_note(*, checksum: str = "etag-1", content_checksum: str | None = "content-1"): + return FileIndexResult( file_path="notes/a.md", entity_id=42, external_id="note-42", title="A Note", permalink="notes/a-note", - checksum="checksum-1", + checksum=checksum, + content_checksum=content_checksum, operation=FileIndexOperation.updated, ) + +def test_plan_indexed_file_live_update_metadata_trusts_an_app_written_note(): + """Regression: an ETag-backed note the app wrote is not superseded (plan test D10). + + Comparing bm-file-checksum (a content sha256) with the indexed ETag marked every + such note superseded, which dropped its embeddings and its provenance. + """ plan = plan_indexed_file_live_update_metadata( - indexed_file=indexed_file, - object_checksum="storage-native-etag", - object_metadata={ - NOTE_OBJECT_FILE_CHECKSUM_METADATA: "checksum-1", - NOTE_OBJECT_ACTOR_USER_PROFILE_ID_METADATA: ("33333333-3333-3333-3333-333333333333"), - NOTE_OBJECT_ACTOR_KIND_METADATA: NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, - NOTE_OBJECT_ACTOR_NAME_METADATA: "Claude Code", - NOTE_OBJECT_SOURCE_METADATA: "mcp", - NOTE_OBJECT_DB_VERSION_METADATA: "2", - }, + indexed_file=indexed_note(), + object_checksum="etag-1", + object_metadata={**MCP_NOTE_OBJECT_METADATA, NOTE_OBJECT_DB_VERSION_METADATA: "2"}, ) assert plan == IndexedFileLiveUpdatePlan( - object_checksum_source=RuntimeStorageObjectChecksumSource.note_file_checksum, - object_checksum="checksum-1", - indexed_checksum="checksum-1", - checksum_matches_indexed_file=True, - metadata_actor_user_profile_id="33333333-3333-3333-3333-333333333333", - metadata_actor_kind=NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, - metadata_actor_name="Claude Code", - metadata_source="mcp", + content_superseded=False, actor_user_profile_id="33333333-3333-3333-3333-333333333333", actor_kind=NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, actor_name="Claude Code", @@ -880,73 +909,36 @@ def test_plan_indexed_file_live_update_metadata_preserves_matching_metadata(): ) -def test_plan_indexed_file_live_update_metadata_omits_mismatched_metadata(): - indexed_file = FileIndexResult( - file_path="notes/a.md", - entity_id=42, - external_id="note-42", - title="A Note", - permalink="notes/a-note", - checksum="checksum-1", - operation=FileIndexOperation.updated, - ) - +def test_plan_indexed_file_live_update_metadata_supersedes_a_replaced_object(): + """A newer write replaced the object after this job read it, whoever wrote it.""" plan = plan_indexed_file_live_update_metadata( - indexed_file=indexed_file, - object_checksum="storage-native-etag", - object_metadata={ - NOTE_OBJECT_FILE_CHECKSUM_METADATA: "checksum-2", - NOTE_OBJECT_ACTOR_USER_PROFILE_ID_METADATA: ("33333333-3333-3333-3333-333333333333"), - NOTE_OBJECT_ACTOR_KIND_METADATA: NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, - NOTE_OBJECT_ACTOR_NAME_METADATA: "Claude Code", - NOTE_OBJECT_SOURCE_METADATA: "mcp", - NOTE_OBJECT_DB_VERSION_METADATA: "2", - }, + indexed_file=indexed_note(), + object_checksum="etag-2", + object_metadata=MCP_NOTE_OBJECT_METADATA, ) - assert plan == IndexedFileLiveUpdatePlan( - object_checksum_source=RuntimeStorageObjectChecksumSource.note_file_checksum, - object_checksum="checksum-2", - indexed_checksum="checksum-1", - checksum_matches_indexed_file=False, - # bm-file-checksum mismatch = a newer own-stack write landed mid-job - content_superseded=True, - metadata_actor_user_profile_id="33333333-3333-3333-3333-333333333333", - metadata_actor_kind=NOTE_OBJECT_ACTOR_KIND_MCP_CLIENT, - metadata_actor_name="Claude Code", - metadata_source="mcp", - actor_user_profile_id=None, - actor_kind=None, - actor_name=None, - live_update_source=None, - operation=None, - ) + assert plan == IndexedFileLiveUpdatePlan(content_superseded=True) -def test_plan_indexed_file_live_update_metadata_etag_mismatch_is_not_superseded(): - indexed_file = FileIndexResult( - file_path="notes/a.md", - entity_id=42, - external_id="note-42", - title="A Note", - permalink="notes/a-note", - checksum="checksum-1", - operation=FileIndexOperation.updated, - ) - +@pytest.mark.parametrize( + "object_metadata", + [ + # Stale bm-* metadata re-uploaded on different bytes. + {**MCP_NOTE_OBJECT_METADATA, NOTE_OBJECT_FILE_CHECKSUM_METADATA: "content-2"}, + # A file written outside the app carries no provenance. + {}, + ], +) +def test_plan_indexed_file_live_update_metadata_withholds_untrusted_provenance( + object_metadata: dict[str, str], +): plan = plan_indexed_file_live_update_metadata( - indexed_file=indexed_file, - object_checksum="storage-native-etag", - object_metadata={ - NOTE_OBJECT_SOURCE_METADATA: "mcp", - }, + indexed_file=indexed_note(), + object_checksum="etag-1", + object_metadata=object_metadata, ) - # An etag never equals a content sha256, so an etag-source mismatch proves - # nothing about supersession - external writes must not suppress checksums. - assert plan.object_checksum_source is RuntimeStorageObjectChecksumSource.storage_etag - assert plan.checksum_matches_indexed_file is False - assert plan.content_superseded is False + assert plan == IndexedFileLiveUpdatePlan(content_superseded=False) def test_plan_index_file_note_live_update_superseded_content_omits_checksum(): diff --git a/tests/indexing/test_note_materialization_runner.py b/tests/indexing/test_note_materialization_runner.py index c60624a52..6e1d4abb4 100644 --- a/tests/indexing/test_note_materialization_runner.py +++ b/tests/indexing/test_note_materialization_runner.py @@ -307,6 +307,7 @@ def written_file() -> RuntimeWrittenFileState: file_path="notes/a.md", file_checksum="new-file-sum", file_updated_at=datetime(2026, 6, 18, 14, 18, tzinfo=UTC), + storage_checksum="new-object-etag", ) @@ -379,10 +380,12 @@ async def test_content_store_note_materialization_file_writer_writes_prepared_no content_store=content_store ).write_prepared_note(prepared) + # A local content store reports the content sha256 as its storage checksum. assert written == RuntimeWrittenFileState( file_path="notes/a.md", file_checksum="new-file-sum", file_updated_at=modified_at, + storage_checksum="new-file-sum", ) assert content_store.write_calls == [ ( @@ -694,6 +697,8 @@ async def record_clear_vacate_path( { "mtime": written.file_updated_at.timestamp(), "size": len(b"# A note\n"), + # The storage checksum of the written object, not the content sha256. + "checksum": "new-object-etag", "sync_checksum": None, }, ) @@ -896,6 +901,8 @@ async def record_clear_vacate_path( { "mtime": written.file_updated_at.timestamp(), "size": len(b"# A note\n"), + # The storage checksum of the written object, not the content sha256. + "checksum": "new-object-etag", "sync_checksum": None, }, ) diff --git a/tests/test_runtime.py b/tests/test_runtime.py index e569c31f9..bd8571131 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -30,8 +30,6 @@ RuntimeNoteActorOrigin, RuntimeNoteObjectMetadata, RuntimeNoteObjectProvenance, - RuntimeStorageObjectChecksum, - RuntimeStorageObjectChecksumSource, actor_kind_from_object_metadata, actor_name_from_object_metadata, actor_user_profile_id_from_object_metadata, @@ -39,7 +37,6 @@ file_checksum_from_object_metadata, normalize_actor_name, source_from_object_metadata, - storage_object_checksum_for_index_match, ) from basic_memory.runtime.cleanup import ( RUNTIME_FILE_SNAPSHOT_TIMESTAMP_MATCH_EPSILON_SECONDS, @@ -1154,21 +1151,13 @@ def test_note_actor_origin_excludes_system_writers(self): is None ) - def test_storage_object_checksum_for_index_match_prefers_note_file_checksum(self): - assert storage_object_checksum_for_index_match( - object_checksum="etag-sum", - object_metadata={NOTE_OBJECT_FILE_CHECKSUM_METADATA: " file-sum "}, - ) == RuntimeStorageObjectChecksum( - checksum="file-sum", - source=RuntimeStorageObjectChecksumSource.note_file_checksum, - ) - assert storage_object_checksum_for_index_match( - object_checksum="etag-sum", - object_metadata={}, - ) == RuntimeStorageObjectChecksum( - checksum="etag-sum", - source=RuntimeStorageObjectChecksumSource.storage_etag, + def test_file_checksum_from_object_metadata_reads_the_content_checksum(self): + assert ( + file_checksum_from_object_metadata({NOTE_OBJECT_FILE_CHECKSUM_METADATA: " file-sum "}) + == "file-sum" ) + assert file_checksum_from_object_metadata({}) is None + assert file_checksum_from_object_metadata(None) is None def test_normalize_actor_name_strips_unsafe_characters_and_limits_length(self): assert normalize_actor_name(" Pat\t\n