From fae73a4f0d0385aa9b83d399b19f3fa89e3235c0 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 00:58:58 -0500 Subject: [PATCH 1/6] fix(core): tolerate disappearing guarded files Signed-off-by: phernandez --- src/basic_memory/runtime/note_file_guards.py | 7 +++++- tests/runtime/test_note_file_guard_races.py | 25 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 tests/runtime/test_note_file_guard_races.py diff --git a/src/basic_memory/runtime/note_file_guards.py b/src/basic_memory/runtime/note_file_guards.py index 1612df2d8..f9d954d33 100644 --- a/src/basic_memory/runtime/note_file_guards.py +++ b/src/basic_memory/runtime/note_file_guards.py @@ -61,7 +61,12 @@ async def read_runtime_file_checksum( """Return the current runtime file checksum, or None when absent.""" if not await reader.exists(file_path): return None - return await reader.compute_checksum(file_path) + try: + return await reader.compute_checksum(file_path) + except FileNotFoundError: + # Object stores cannot make exists-plus-read atomic. A concurrent delete + # after the probe has the same domain meaning as an initially absent file. + return None def runtime_file_conflict( diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py new file mode 100644 index 000000000..cec55d240 --- /dev/null +++ b/tests/runtime/test_note_file_guard_races.py @@ -0,0 +1,25 @@ +"""Race regressions for portable note-file concurrency guards.""" + +from basic_memory.runtime.note_file_guards import read_runtime_file_checksum +from basic_memory.runtime.storage import RuntimeFileChecksum, RuntimeFilePath + + +class _DisappearingChecksumReader: + """Model an object deleted after the existence probe succeeds.""" + + async def exists(self, path: RuntimeFilePath) -> bool: + del path + return True + + async def compute_checksum(self, path: RuntimeFilePath) -> RuntimeFileChecksum: + raise FileNotFoundError(path) + + +async def test_checksum_read_treats_post_probe_deletion_as_absent() -> None: + """A disappearing object should not poison a durable materialization retry.""" + checksum = await read_runtime_file_checksum( + _DisappearingChecksumReader(), + "notes/disappeared.md", + ) + + assert checksum is None From 3a9ecaf4c9f976854a0540b9c36dd3b31684a0a9 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 01:05:38 -0500 Subject: [PATCH 2/6] fix(core): preserve missing checksum outcome Signed-off-by: phernandez --- src/basic_memory/services/file_service.py | 4 +++ tests/runtime/test_note_file_guard_races.py | 29 +++++++++------------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/basic_memory/services/file_service.py b/src/basic_memory/services/file_service.py index f301cbb00..d77b62d42 100644 --- a/src/basic_memory/services/file_service.py +++ b/src/basic_memory/services/file_service.py @@ -569,6 +569,10 @@ async def compute_checksum(self, path: FilePath) -> str: return hasher.hexdigest() + except FileNotFoundError: + # Runtime guards distinguish a concurrently deleted file from a + # checksum failure; preserve that domain outcome at the adapter boundary. + raise except Exception as e: # pragma: no cover logger.error("Failed to compute checksum", path=str(full_path), error=str(e)) raise FileError(f"Failed to compute checksum for {path}: {e}") diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py index cec55d240..e53c5ba3c 100644 --- a/tests/runtime/test_note_file_guard_races.py +++ b/tests/runtime/test_note_file_guard_races.py @@ -1,25 +1,20 @@ """Race regressions for portable note-file concurrency guards.""" -from basic_memory.runtime.note_file_guards import read_runtime_file_checksum -from basic_memory.runtime.storage import RuntimeFileChecksum, RuntimeFilePath - - -class _DisappearingChecksumReader: - """Model an object deleted after the existence probe succeeds.""" +from pathlib import Path +from unittest.mock import AsyncMock, patch - async def exists(self, path: RuntimeFilePath) -> bool: - del path - return True - - async def compute_checksum(self, path: RuntimeFilePath) -> RuntimeFileChecksum: - raise FileNotFoundError(path) +from basic_memory.index.note_content_materialization import LocalNoteContentStorage +from basic_memory.runtime.note_file_guards import read_runtime_file_checksum +from basic_memory.services.file_service import FileService -async def test_checksum_read_treats_post_probe_deletion_as_absent() -> None: +async def test_checksum_read_treats_post_probe_deletion_as_absent(tmp_path: Path) -> None: """A disappearing object should not poison a durable materialization retry.""" - checksum = await read_runtime_file_checksum( - _DisappearingChecksumReader(), - "notes/disappeared.md", - ) + file_service = FileService(tmp_path) + storage = LocalNoteContentStorage(file_service) + + # The file disappears after storage reports it present but before checksum I/O. + with patch.object(file_service, "exists", AsyncMock(return_value=True)): + checksum = await read_runtime_file_checksum(storage, "notes/disappeared.md") assert checksum is None From 5dbdf9551484f3aa3a29dbc02f47c7c1be8997c2 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 01:15:53 -0500 Subject: [PATCH 3/6] fix(core): preserve checksum service contract Signed-off-by: phernandez --- .../index/note_content_materialization.py | 9 ++++++++- src/basic_memory/services/file_service.py | 4 ---- tests/runtime/test_note_file_guard_races.py | 11 +++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/basic_memory/index/note_content_materialization.py b/src/basic_memory/index/note_content_materialization.py index ffad2893c..e92a0fbf0 100644 --- a/src/basic_memory/index/note_content_materialization.py +++ b/src/basic_memory/index/note_content_materialization.py @@ -532,7 +532,14 @@ async def exists(self, path: RuntimeFilePath) -> bool: return await self.file_service.exists(path) async def compute_checksum(self, path: RuntimeFilePath) -> RuntimeFileChecksum: - return await self.file_service.compute_checksum(path) + try: + return await self.file_service.compute_checksum(path) + except file_utils.FileError as exc: + if isinstance(exc.__context__, FileNotFoundError): + # The general FileService contract reports I/O failures as FileError, + # while runtime guards need concurrent deletion as an absent file. + raise FileNotFoundError(path) from exc + raise async def delete_file(self, path: RuntimeFilePath) -> None: await self.file_service.delete_file(path) diff --git a/src/basic_memory/services/file_service.py b/src/basic_memory/services/file_service.py index d77b62d42..f301cbb00 100644 --- a/src/basic_memory/services/file_service.py +++ b/src/basic_memory/services/file_service.py @@ -569,10 +569,6 @@ async def compute_checksum(self, path: FilePath) -> str: return hasher.hexdigest() - except FileNotFoundError: - # Runtime guards distinguish a concurrently deleted file from a - # checksum failure; preserve that domain outcome at the adapter boundary. - raise except Exception as e: # pragma: no cover logger.error("Failed to compute checksum", path=str(full_path), error=str(e)) raise FileError(f"Failed to compute checksum for {path}: {e}") diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py index e53c5ba3c..dccd0f145 100644 --- a/tests/runtime/test_note_file_guard_races.py +++ b/tests/runtime/test_note_file_guard_races.py @@ -3,6 +3,9 @@ from pathlib import Path from unittest.mock import AsyncMock, patch +import pytest + +from basic_memory.file_utils import FileError from basic_memory.index.note_content_materialization import LocalNoteContentStorage from basic_memory.runtime.note_file_guards import read_runtime_file_checksum from basic_memory.services.file_service import FileService @@ -18,3 +21,11 @@ async def test_checksum_read_treats_post_probe_deletion_as_absent(tmp_path: Path checksum = await read_runtime_file_checksum(storage, "notes/disappeared.md") assert checksum is None + + +async def test_direct_checksum_preserves_file_service_error_contract(tmp_path: Path) -> None: + """Direct callers still receive FileError when the checksum source is absent.""" + file_service = FileService(tmp_path) + + with pytest.raises(FileError): + await file_service.compute_checksum("notes/disappeared.md") From 88e13811f67d08cd27eddd78ff8970b8b26a557f Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 01:22:25 -0500 Subject: [PATCH 4/6] fix(core): cover directory cleanup checksum race Signed-off-by: phernandez --- src/basic_memory/index/local_notes.py | 9 ++++++++- tests/runtime/test_note_file_guard_races.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/basic_memory/index/local_notes.py b/src/basic_memory/index/local_notes.py index 32bdbbe30..9358f1b19 100644 --- a/src/basic_memory/index/local_notes.py +++ b/src/basic_memory/index/local_notes.py @@ -180,7 +180,14 @@ async def exists(self, path: RuntimeFilePath) -> bool: return await self.file_service.exists(path) async def compute_checksum(self, path: RuntimeFilePath) -> RuntimeFileChecksum: - return await self.file_service.compute_checksum(path) + try: + return await self.file_service.compute_checksum(path) + except FileError as exc: + if isinstance(exc.__context__, FileNotFoundError): + # Directory cleanup uses the portable guard, where deletion after + # the existence probe is the same absent-file outcome. + raise FileNotFoundError(path) from exc + raise async def delete_file_if_unchanged( self, diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py index dccd0f145..c726179dd 100644 --- a/tests/runtime/test_note_file_guard_races.py +++ b/tests/runtime/test_note_file_guard_races.py @@ -6,6 +6,7 @@ import pytest from basic_memory.file_utils import FileError +from basic_memory.index.local_notes import LocalNoteFileDeleteStorage from basic_memory.index.note_content_materialization import LocalNoteContentStorage from basic_memory.runtime.note_file_guards import read_runtime_file_checksum from basic_memory.services.file_service import FileService @@ -29,3 +30,16 @@ async def test_direct_checksum_preserves_file_service_error_contract(tmp_path: P with pytest.raises(FileError): await file_service.compute_checksum("notes/disappeared.md") + + +async def test_directory_delete_checksum_treats_post_probe_deletion_as_absent( + tmp_path: Path, +) -> None: + """Directory cleanup should converge when its target disappears before checksum I/O.""" + file_service = FileService(tmp_path) + storage = LocalNoteFileDeleteStorage(file_service) + + with patch.object(file_service, "exists", AsyncMock(return_value=True)): + checksum = await read_runtime_file_checksum(storage, "notes/disappeared.md") + + assert checksum is None From 14dc261fdcc5b74c584c17092f87667a9cef08fd Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 01:28:43 -0500 Subject: [PATCH 5/6] Handle delete checksum disappearance race Signed-off-by: phernandez --- src/basic_memory/index/local_notes.py | 7 +++- tests/runtime/test_note_file_guard_races.py | 36 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/basic_memory/index/local_notes.py b/src/basic_memory/index/local_notes.py index 9358f1b19..b4f59714b 100644 --- a/src/basic_memory/index/local_notes.py +++ b/src/basic_memory/index/local_notes.py @@ -200,7 +200,12 @@ async def delete_file_if_unchanged( # guarantee portable: never delete an object that no longer matches (basic-memory-cloud#1618). if not await self.file_service.exists(path): return False - if await self.file_service.compute_checksum(path) != expected_checksum: + try: + actual_checksum = await self.compute_checksum(path) + except FileNotFoundError: + # Disappearance after the final existence probe is another safe no-delete outcome. + return False + if actual_checksum != expected_checksum: return False await self.file_service.delete_file(path) return True diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py index c726179dd..5928f0353 100644 --- a/tests/runtime/test_note_file_guard_races.py +++ b/tests/runtime/test_note_file_guard_races.py @@ -8,6 +8,8 @@ from basic_memory.file_utils import FileError from basic_memory.index.local_notes import LocalNoteFileDeleteStorage from basic_memory.index.note_content_materialization import LocalNoteContentStorage +from basic_memory.indexing.note_file_delete_runner import run_note_file_delete +from basic_memory.runtime.cleanup import RuntimeDeleteStatus, RuntimeNoteFileDeleteJobRequest from basic_memory.runtime.note_file_guards import read_runtime_file_checksum from basic_memory.services.file_service import FileService @@ -24,6 +26,40 @@ async def test_checksum_read_treats_post_probe_deletion_as_absent(tmp_path: Path assert checksum is None +async def test_directory_delete_converges_when_file_disappears_before_delete( + tmp_path: Path, +) -> None: + """The final guarded checksum should treat a vanished target as a safe no-delete.""" + file_service = FileService(tmp_path) + file_path = "notes/disappeared.md" + await file_service.write_file(file_path, "# Disappearing note\n") + accepted_checksum = await file_service.compute_checksum(file_path) + original_compute_checksum = file_service.compute_checksum + checksum_calls = 0 + + async def delete_before_final_checksum(path: str) -> str: + nonlocal checksum_calls + checksum_calls += 1 + if checksum_calls == 2: + (tmp_path / path).unlink() + return await original_compute_checksum(path) + + with patch.object(file_service, "compute_checksum", side_effect=delete_before_final_checksum): + result = await run_note_file_delete( + RuntimeNoteFileDeleteJobRequest( + project_id=101, + entity_id=42, + file_path=file_path, + file_checksum=accepted_checksum, + ), + storage=LocalNoteFileDeleteStorage(file_service), + ) + + assert result.status == RuntimeDeleteStatus.skipped + assert result.reason == f"file changed before delete: {file_path}" + assert not (tmp_path / file_path).exists() + + async def test_direct_checksum_preserves_file_service_error_contract(tmp_path: Path) -> None: """Direct callers still receive FileError when the checksum source is absent.""" file_service = FileService(tmp_path) From aeb09978d49109a2406848cd1dae15853021182d Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 30 Aug 2026 01:35:08 -0500 Subject: [PATCH 6/6] Handle note delete checksum disappearance race Signed-off-by: phernandez --- .../index/note_content_materialization.py | 7 +++- tests/runtime/test_note_file_guard_races.py | 34 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/basic_memory/index/note_content_materialization.py b/src/basic_memory/index/note_content_materialization.py index e92a0fbf0..c5a2052c8 100644 --- a/src/basic_memory/index/note_content_materialization.py +++ b/src/basic_memory/index/note_content_materialization.py @@ -555,7 +555,12 @@ async def delete_file_if_unchanged( # atomic precondition; the race is negligible on a filesystem. if not await self.file_service.exists(path): return False - if await self.file_service.compute_checksum(path) != expected_checksum: + try: + actual_checksum = await self.compute_checksum(path) + except FileNotFoundError: + # Disappearance after the final existence probe is another safe no-delete outcome. + return False + if actual_checksum != expected_checksum: return False await self.file_service.delete_file(path) return True diff --git a/tests/runtime/test_note_file_guard_races.py b/tests/runtime/test_note_file_guard_races.py index 5928f0353..f0f055848 100644 --- a/tests/runtime/test_note_file_guard_races.py +++ b/tests/runtime/test_note_file_guard_races.py @@ -60,6 +60,40 @@ async def delete_before_final_checksum(path: str) -> str: assert not (tmp_path / file_path).exists() +async def test_note_delete_converges_when_file_disappears_before_delete( + tmp_path: Path, +) -> None: + """Ordinary note cleanup should share the safe final-checksum outcome.""" + file_service = FileService(tmp_path) + file_path = "notes/disappeared.md" + await file_service.write_file(file_path, "# Disappearing note\n") + accepted_checksum = await file_service.compute_checksum(file_path) + original_compute_checksum = file_service.compute_checksum + checksum_calls = 0 + + async def delete_before_final_checksum(path: str) -> str: + nonlocal checksum_calls + checksum_calls += 1 + if checksum_calls == 2: + (tmp_path / path).unlink() + return await original_compute_checksum(path) + + with patch.object(file_service, "compute_checksum", side_effect=delete_before_final_checksum): + result = await run_note_file_delete( + RuntimeNoteFileDeleteJobRequest( + project_id=101, + entity_id=42, + file_path=file_path, + file_checksum=accepted_checksum, + ), + storage=LocalNoteContentStorage(file_service), + ) + + assert result.status == RuntimeDeleteStatus.skipped + assert result.reason == f"file changed before delete: {file_path}" + assert not (tmp_path / file_path).exists() + + async def test_direct_checksum_preserves_file_service_error_contract(tmp_path: Path) -> None: """Direct callers still receive FileError when the checksum source is absent.""" file_service = FileService(tmp_path)