fix(core): give the move-orphan gate a content-checksum source - #1160
Merged
Conversation
The move-vacate gate (#1601) compares a path's vacate marker against the current object's checksum via the checker's single current_checksum_source. That source also drives the freshness decision, so it is locked to whatever domain the store indexes by. On local that is the SHA-256 content checksum everywhere, so the gate works. A store whose freshness domain differs from note content — Basic Memory Cloud indexes by S3 ETag while note content (and therefore the marker) is SHA-256 — cannot satisfy both: the marker is content-domain, the freshness checksum is ETag-domain, so the gate would never match a real orphan and would instead retire the marker and re-import the ghost. Add an optional move_orphan_checksum_source used only for the marker comparison (retire + tombstone paths). It defaults to None, in which case the gate falls back to current_checksum and behavior is unchanged for local and any single-domain store. The gap-(a) moved-entity comparison stays on current_checksum because both sides are the entity/freshness domain. Cloud will pass a SHA-256 (bm-file-checksum) source here; see basic-memory-cloud#1601 / PR #1613. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joe P <joe@basicmemory.com>
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The move-vacate gate added in #1152 (basic-memory-cloud#1601) compares a path's vacate marker against the current object's checksum using the checker's single
current_checksum_source. That same source drives the freshness decision, so it is effectively locked to whatever checksum domain the store indexes by.Change
Add an optional
move_orphan_checksum_source: CurrentFileChecksumSource | None = NonetoFileIndexChecker, used only for the marker comparison in_apply_move_orphan_gate(the retire and tombstone paths). When it isNonethe gate falls back tocurrent_checksum, so behavior is unchanged for local and any single-domain store. The gap-(a) moved-entity comparison keeps usingcurrent_checksumbecause both sides are the entity/freshness domain.Cloud will pass a SHA-256 (
bm-file-checksumobject metadata) source here.Risk
Backward-compatible and opt-in: the new field defaults to
None. No existing caller passes it, so local indexing is byte-for-byte unchanged (verified: the 22 existing checker tests pass untouched).Testing
ty+ruffclean. Addedtest_checker_gate_uses_content_checksum_source_when_domains_differ: with a freshness checksum (ETag) deliberately different from the marker's content checksum, the gate consults the content source, suppresses the leftover, and does not retire the marker. 23 checker tests pass.Companion: basic-memory-cloud#1613.
🤖 Generated with Claude Code