Skip to content

Commit d2bbb05

Browse files
phernandezclaude
andcommitted
fix(sync): stop scan move batches deleting concurrently created destination entities
Scan change planning only pairs a move with a destination that had no DB row at snapshot time, so an entity found at the destination during apply was created concurrently (e.g. an accepted write_note whose content may not be materialized yet). apply_project_index_move_batch deleted that entity unconditionally and re-pointed the stale source entity onto the path, destroying the accepted content. Scan runtimes now opt into verify_replaced_move_targets on the maintenance store: a replacement row is only deleted when its checksum equals the moved entity's indexed checksum — the checksum the move was planned against — which proves it merely indexes the moved bytes (a racing event index of the new path, a safe dedupe). Any other replacement drops that move from the batch with a warning; nothing is deleted or re-pointed, and the next scan reconciles the stale source path as deleted-or-present. Dropped source paths are reported on the batch result and move run. The unconditional-replace branch stays the default because the watcher flow legitimately moves onto an existing indexed file (mv over an indexed path) where the pre-existing destination row must be replaced; cloud scan runtimes also keep current behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 67aac01 commit d2bbb05

4 files changed

Lines changed: 352 additions & 16 deletions

File tree

src/basic_memory/index/local_project.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,10 @@ def runtime_from_dependencies(
590590
delete_path_verifier=LocalProjectIndexDeletePathVerifier(
591591
file_service=dependencies.file_service,
592592
),
593+
# Scan-planned moves guarantee the destination had no DB row at
594+
# snapshot time, so a replacement row found at apply time is a
595+
# concurrent creation that must be checksum-verified before deletion.
596+
verify_replaced_move_targets=True,
593597
)
594598
return LocalProjectIndexRuntime(
595599
observed_file_source=LocalProjectIndexObservedFileSource(

src/basic_memory/indexing/project_index_maintenance.py

Lines changed: 87 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from typing import Protocol
88

99
from loguru import logger
10-
from sqlalchemy import bindparam, case, column, delete, select, table, text, update
10+
from sqlalchemy import RowMapping, bindparam, case, column, delete, select, table, text, update
1111
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
1212

1313
from basic_memory import db
@@ -169,6 +169,7 @@ class ProjectIndexMoveBatchResult:
169169
replaced_entity_ids: frozenset[int] = frozenset()
170170
relation_cleanup_entity_ids: frozenset[int] = frozenset()
171171
missing_paths: tuple[str, ...] = ()
172+
dropped_move_paths: tuple[str, ...] = ()
172173

173174

174175
@dataclass(frozen=True, slots=True)
@@ -198,6 +199,15 @@ def missing_paths(self) -> tuple[str, ...]:
198199
missing_path for record in self.records for missing_path in record.result.missing_paths
199200
)
200201

202+
@property
203+
def dropped_move_paths(self) -> tuple[str, ...]:
204+
"""Return every move source path dropped because its destination changed."""
205+
return tuple(
206+
dropped_path
207+
for record in self.records
208+
for dropped_path in record.result.dropped_move_paths
209+
)
210+
201211

202212
@dataclass(frozen=True, slots=True)
203213
class ProjectIndexDeleteBatch:
@@ -451,6 +461,16 @@ class RepositoryProjectIndexMaintenanceStore:
451461
project_id: ProjectId
452462
move_content_updater: ProjectIndexMoveContentUpdater | None = None
453463
delete_path_verifier: ProjectIndexDeletePathVerifier = TrustPlannedProjectIndexDeleteVerifier()
464+
# Trigger: an entity occupies a move destination at apply time.
465+
# Why: scan change planning only pairs moves with paths that had no DB row
466+
# at snapshot time, so a row found there was created concurrently and
467+
# may carry accepted-but-unmaterialized content; the watcher flow, by
468+
# contrast, legitimately moves onto an existing indexed file and must
469+
# keep replacing it unconditionally.
470+
# Outcome: scan runtimes set this True so a replacement is only deleted
471+
# when its checksum proves it indexes the moved bytes; mismatches
472+
# drop the move for the next scan to reconcile.
473+
verify_replaced_move_targets: bool = False
454474

455475
async def apply_project_index_move_batch(
456476
self,
@@ -466,20 +486,79 @@ async def apply_project_index_move_batch(
466486

467487
async with db.scoped_session(self.session_maker) as session:
468488
existing_paths_result = await session.execute(
469-
select(Entity.id, Entity.file_path, Entity.permalink).where(
489+
select(Entity.id, Entity.file_path, Entity.permalink, Entity.checksum).where(
470490
Entity.project_id == self.project_id,
471491
Entity.file_path.in_(old_paths),
472492
)
473493
)
474-
target_rows = existing_paths_result.mappings().all()
494+
target_rows = list(existing_paths_result.mappings().all())
495+
replaced_entity_ids: frozenset[int] = frozenset()
496+
relation_cleanup_entity_ids: frozenset[int] = frozenset()
497+
dropped_move_paths: tuple[str, ...] = ()
498+
content_updates_by_entity_id: dict[int, ProjectIndexMovedFileContentUpdate] = {}
499+
replacement_rows: list[RowMapping] = []
500+
501+
if target_rows:
502+
new_paths = tuple(
503+
sorted({target_paths_by_old_path[str(row["file_path"])] for row in target_rows})
504+
)
505+
replacement_result = await session.execute(
506+
select(Entity.id, Entity.file_path, Entity.checksum).where(
507+
Entity.project_id == self.project_id,
508+
Entity.file_path.in_(new_paths),
509+
Entity.id.not_in(tuple(int(row["id"]) for row in target_rows)),
510+
)
511+
)
512+
replacement_rows = list(replacement_result.mappings().all())
513+
514+
if self.verify_replaced_move_targets and replacement_rows:
515+
old_path_by_new_path = {
516+
target_paths_by_old_path[str(row["file_path"])]: str(row["file_path"])
517+
for row in target_rows
518+
}
519+
# The move was planned by matching the destination file's checksum
520+
# to the source entity's indexed checksum, so that checksum is the
521+
# only content a replacement row may legitimately index.
522+
expected_checksum_by_new_path = {
523+
target_paths_by_old_path[str(row["file_path"])]: row["checksum"]
524+
for row in target_rows
525+
}
526+
verified_replacement_rows: list[RowMapping] = []
527+
dropped_new_paths: set[str] = set()
528+
for replacement_row in replacement_rows:
529+
replacement_path = str(replacement_row["file_path"])
530+
expected_checksum = expected_checksum_by_new_path.get(replacement_path)
531+
if (
532+
expected_checksum is not None
533+
and replacement_row["checksum"] == expected_checksum
534+
):
535+
verified_replacement_rows.append(replacement_row)
536+
continue
537+
dropped_new_paths.add(replacement_path)
538+
logger.warning(
539+
"Dropping planned move: destination holds a concurrently created entity",
540+
old_path=old_path_by_new_path.get(replacement_path),
541+
new_path=replacement_path,
542+
)
543+
replacement_rows = verified_replacement_rows
544+
if dropped_new_paths:
545+
dropped_move_paths = tuple(
546+
sorted(
547+
old_path_by_new_path[new_path] for new_path in dropped_new_paths
548+
)
549+
)
550+
target_rows = [
551+
row
552+
for row in target_rows
553+
if target_paths_by_old_path[str(row["file_path"])]
554+
not in dropped_new_paths
555+
]
556+
475557
updated_old_paths = frozenset(str(row["file_path"]) for row in target_rows)
476558
target_paths_by_entity_id = {
477559
int(row["id"]): target_paths_by_old_path[str(row["file_path"])]
478560
for row in target_rows
479561
}
480-
replaced_entity_ids: frozenset[int] = frozenset()
481-
relation_cleanup_entity_ids: frozenset[int] = frozenset()
482-
content_updates_by_entity_id: dict[int, ProjectIndexMovedFileContentUpdate] = {}
483562
if self.move_content_updater is not None:
484563
for row in target_rows:
485564
entity_id = int(row["id"])
@@ -499,15 +578,6 @@ async def apply_project_index_move_batch(
499578
content_updates_by_entity_id[entity_id] = content_update
500579

501580
if updated_old_paths:
502-
new_paths = tuple(sorted(set(target_paths_by_entity_id.values())))
503-
replacement_result = await session.execute(
504-
select(Entity.id, Entity.file_path).where(
505-
Entity.project_id == self.project_id,
506-
Entity.file_path.in_(new_paths),
507-
Entity.id.not_in(tuple(target_paths_by_entity_id)),
508-
)
509-
)
510-
replacement_rows = replacement_result.mappings().all()
511581
replaced_entity_ids = frozenset(int(row["id"]) for row in replacement_rows)
512582
relation_cleanup_entity_ids = await delete_project_index_entities(
513583
session,
@@ -620,13 +690,15 @@ async def apply_project_index_move_batch(
620690
move_target.old_path
621691
for move_target in move_batch.targets
622692
if move_target.old_path not in updated_old_paths
693+
and move_target.old_path not in dropped_move_paths
623694
)
624695
return ProjectIndexMoveBatchResult(
625696
updated_files=len(updated_old_paths),
626697
moved_entity_ids=frozenset(target_paths_by_entity_id),
627698
replaced_entity_ids=replaced_entity_ids,
628699
relation_cleanup_entity_ids=relation_cleanup_entity_ids,
629700
missing_paths=missing_paths,
701+
dropped_move_paths=dropped_move_paths,
630702
)
631703

632704
async def apply_project_index_delete_batch(

tests/index/test_local_project_index.py

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@
4444
IndexedEntity,
4545
IndexingBatchResult,
4646
)
47-
from basic_memory.indexing.project_index_coordinator import ProjectIndexObservedFileSource
47+
from basic_memory.indexing.project_index_coordinator import (
48+
ProjectIndexChangeDetector,
49+
ProjectIndexObservedFileSource,
50+
)
4851
from basic_memory.indexing.project_index_maintenance import (
4952
ProjectIndexDeleteRun,
5053
ProjectIndexMoveRun,
@@ -56,6 +59,7 @@
5659
UnresolvedRelation,
5760
)
5861
from basic_memory.models import Entity, Project
62+
from basic_memory.repository import EntityRepository
5963
from basic_memory.repository.note_content_repository import NoteContentRepository
6064
from basic_memory.runtime.jobs import (
6165
RuntimeIndexFileBatchJobRequest,
@@ -1407,6 +1411,134 @@ async def test_local_project_index_treats_rename_over_existing_path_as_modify_an
14071411
assert results[0].file_path == "note.md"
14081412

14091413

1414+
@dataclass(slots=True)
1415+
class ConcurrentDestinationWriteChangeDetector:
1416+
"""After planning, simulate an accepted write_note landing at a move destination."""
1417+
1418+
detector: ProjectIndexChangeDetector
1419+
session_maker: async_sessionmaker[AsyncSession]
1420+
entity_repository: EntityRepository
1421+
destination_file: Path
1422+
destination_relative: str
1423+
accepted_content: bytes
1424+
permalink: str
1425+
1426+
async def detect_all_changes(
1427+
self,
1428+
storage_files: Mapping[str, RuntimeObservedIndexFile],
1429+
) -> ChangeReport:
1430+
report = await self.detector.detect_all_changes(storage_files)
1431+
self.destination_file.write_bytes(self.accepted_content)
1432+
entity = Entity(
1433+
permalink=self.permalink,
1434+
title="Accepted",
1435+
note_type="note",
1436+
file_path=self.destination_relative,
1437+
checksum=sha256(self.accepted_content).hexdigest(),
1438+
content_type="text/markdown",
1439+
created_at=datetime.now(timezone.utc),
1440+
updated_at=datetime.now(timezone.utc),
1441+
)
1442+
async with db.scoped_session(self.session_maker) as session:
1443+
await self.entity_repository.add(session, entity)
1444+
return report
1445+
1446+
1447+
async def test_local_project_index_move_spares_concurrently_created_destination_entity(
1448+
test_project: Project,
1449+
project_config,
1450+
entity_repository,
1451+
session_maker: async_sessionmaker[AsyncSession],
1452+
config_manager,
1453+
monkeypatch,
1454+
) -> None:
1455+
"""A planned move must not destroy an entity created at its destination mid-run."""
1456+
del config_manager
1457+
1458+
source_path = project_config.home / "notes" / "move-race.md"
1459+
destination_path = project_config.home / "archive" / "move-race.md"
1460+
source_path.parent.mkdir(parents=True, exist_ok=True)
1461+
destination_path.parent.mkdir(parents=True, exist_ok=True)
1462+
moved_content = b"# Move Race\n\nOriginal content that moves.\n"
1463+
source_path.write_bytes(moved_content)
1464+
1465+
first = await run_local_project_index_for_project(
1466+
test_project,
1467+
runtime_factory=LocalProjectIndexRuntimeFactory(batch_size=10),
1468+
force_full=True,
1469+
)
1470+
assert first.enqueued_files == 1
1471+
1472+
async with db.scoped_session(session_maker) as session:
1473+
source_before = await entity_repository.get_by_file_path(session, "notes/move-race.md")
1474+
assert source_before is not None
1475+
1476+
source_path.rename(destination_path)
1477+
1478+
accepted_content = b"# Accepted\n\nConcurrently accepted note content.\n"
1479+
runtime_factory = LocalProjectIndexRuntimeFactory(batch_size=10)
1480+
runtime = await runtime_factory.runtime_for_project(test_project)
1481+
racing_runtime = LocalProjectIndexRuntime(
1482+
observed_file_source=runtime.observed_file_source,
1483+
change_detector=ConcurrentDestinationWriteChangeDetector(
1484+
detector=runtime.change_detector,
1485+
session_maker=session_maker,
1486+
entity_repository=entity_repository,
1487+
destination_file=destination_path,
1488+
destination_relative="archive/move-race.md",
1489+
accepted_content=accepted_content,
1490+
permalink=f"{test_project.permalink}/archive/move-race-accepted",
1491+
),
1492+
maintenance_runner=runtime.maintenance_runner,
1493+
moved_entity_search_refresher=runtime.moved_entity_search_refresher,
1494+
batch_enqueuer=runtime.batch_enqueuer,
1495+
completion_relation_runtime=runtime.completion_relation_runtime,
1496+
batch_size=runtime.batch_size,
1497+
)
1498+
1499+
second = await run_local_project_index(
1500+
RuntimeProjectIndexJobRequest(
1501+
project=ProjectRuntimeReference.from_project(test_project),
1502+
),
1503+
runtime=racing_runtime,
1504+
)
1505+
1506+
# The planned move was dropped: nothing moved and nothing was deleted.
1507+
assert second.moved_files == 0
1508+
assert second.deleted_files == 0
1509+
1510+
async with db.scoped_session(session_maker) as session:
1511+
destination_entity = await entity_repository.get_by_file_path(
1512+
session, "archive/move-race.md"
1513+
)
1514+
stale_source = await entity_repository.get_by_file_path(session, "notes/move-race.md")
1515+
1516+
assert destination_entity is not None
1517+
assert destination_entity.checksum == sha256(accepted_content).hexdigest()
1518+
assert destination_path.read_bytes() == accepted_content
1519+
# The stale source row is left for the next scan to reconcile as deleted.
1520+
assert stale_source is not None
1521+
1522+
third = await run_local_project_index_for_project(
1523+
test_project,
1524+
runtime_factory=LocalProjectIndexRuntimeFactory(batch_size=10),
1525+
)
1526+
1527+
assert third.deleted_files == 1
1528+
1529+
async with db.scoped_session(session_maker) as session:
1530+
reconciled_source = await entity_repository.get_by_file_path(
1531+
session, "notes/move-race.md"
1532+
)
1533+
surviving_destination = await entity_repository.get_by_file_path(
1534+
session, "archive/move-race.md"
1535+
)
1536+
1537+
assert reconciled_source is None
1538+
assert surviving_destination is not None
1539+
assert surviving_destination.id == destination_entity.id
1540+
1541+
14101542
async def test_local_project_index_move_repairs_observation_search_permalinks(
14111543
test_project: Project,
14121544
project_config,

0 commit comments

Comments
 (0)