Skip to content

feat(core): relay self-supersede on stale base + db_version provenance - #1144

Merged
phernandez merged 2 commits into
mainfrom
feat-1589-self-write
Jul 23, 2026
Merged

feat(core): relay self-supersede on stale base + db_version provenance#1144
phernandez merged 2 commits into
mainfrom
feat-1589-self-write

Conversation

@phernandez

Copy link
Copy Markdown
Member

Phase E of the basic-memory-cloud#1589 collaboration persistence redesign (design direction and plan captured on that issue). Cloud consumes this via a rev bump once merged.

Why

Lost-ack wedge (production, 2026-07-23): a collaboration-relay persist timed out client-side after committing on the server. The accepted row then held the relay's own write while the relay's recorded base was one version behind, so every subsequent persist 409'd against the relay's own committed version - zero successful persists for 10+ minutes while the user typed, with an erroneous "note changed elsewhere" conflict toast. Client-side echo heuristics cannot close this class: the client, by construction, does not know about writes whose acks were lost.

Provenance for index events: cloud's redesign carries {db_version, writer} on live updates so the relay can decide echo-vs-out-of-band by version arithmetic. Core already parses the object's bm-* metadata during observed-object indexing but did not expose db_version on the result.

What Changed

  1. Relay self-supersede (indexing/accepted_note_mutation_runner.py): the base-checksum precondition accepts a stale base iff BOTH the current note_content.last_source AND the request source are collaboration_relay. The relay superseding its own prior write is never a real conflict: the live Y.Doc is the merge of everything the relay ever persisted. Scope guards:
    • Foreign current writers (mcp/api/web_v2/s3_webhook) keep full guarded semantics - new test pins this.
    • The deleted-entity base-checksum 409 is untouched (a relay write to a deleted note still rejects).
    • The db_version CAS and its concurrent-write 409 are untouched.
    • New constant NOTE_SOURCE_COLLABORATION_RELAY in runtime/note_object_metadata.py.
  2. db_version provenance (runtime/note_object_metadata.py, indexing/models.py): RuntimeNoteObjectProvenance parses bm-db-version; threads through IndexedFileLiveUpdatePlan -> IndexFileJobResult -> IndexFileNoteLiveUpdatePlan. Trusted-branch only, exactly like the actor fields, and withheld for superseded content the same way content_checksum is (a stale version must not invite consumers to reconcile to it).

Testing

  • New: test_run_accepted_note_update_accepts_relay_self_supersede_on_stale_base (the wedge replay: relay-owned current version + stale relay base -> accepted, db_version bumps) and test_run_accepted_note_update_relay_stale_base_still_rejects_foreign_writes (mcp-owned current version + stale relay base -> 409, nothing persisted).
  • Provenance: existing trusted-metadata tests extended with db_version (parse, plan threading, current-path result, runner preservation).
  • pytest tests/indexing tests/test_runtime -> 479 passed; just lint clean; just typecheck -> only the pre-existing asyncio deprecation warnings.

Risks

  • The self-supersede rule intentionally weakens the precondition for exactly one writer pair (relay over relay). A second relay instance during a deploy window can supersede the first's write; both derive from the same shared Redis Y.Doc, so this converges rather than losing data, and every accepted write remains a Tigris object version.

🤖 Generated with Claude Code

https://claude.ai/code/session_013wFpjrd126rVe6VisiYVDD

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00534e941f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

actor_kind=provenance.actor_kind,
actor_name=provenance.actor_name,
live_update_source=provenance.source,
db_version=provenance.db_version,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify db_version against accepted state before publishing

When an accepted write advances db_version without changing the markdown (for example a relay retry/no-op save with the same db_checksum), an older materialized object still has the same bm-file-checksum, so this checksum-only trust check treats the object's stale bm-db-version as current and publishes it to live-update consumers. Since the cloud relay is expected to use db_version arithmetic for echo-vs-out-of-band decisions, please compare the metadata version with the current NoteContent.db_version (or omit it unless that version is verified), not just with the content checksum.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scenario is real, but the staleness has one direction that fails safe: an object's metadata version is the version of the write that produced that object, so it can LAG the accepted row (content-identical accept bumps db_version without a new materialization) but can never exceed it. A lagging version rides identical content, so the consumer's version arithmetic reads it as an echo and skips - exactly the right outcome for identical bytes. The cloud reconciler additionally treats event fields as a pre-filter only: any apply path goes through the authoritative /read, whose response carries the DB row's current db_version and is the decision-grade value.

Threading NoteContent.db_version verification into the index/materialized-source contracts would add a tenant-DB read to a path that deliberately works from object state alone, to guard a direction that cannot mislead. Encoded the hint-vs-ground-truth contract in the code comment at the plan site instead (4993e71). Happy to revisit if a consumer ever wants to treat event versions as authoritative - that would be the contract change to block.

phernandez added a commit that referenced this pull request Jul 23, 2026
… plan

Object metadata versions can lag the accepted row (content-identical accepts
bump db_version without a new materialization) but can never exceed it, so a
stale event version reads as an echo of identical content downstream - the
correct skip. Authoritative reads carry the DB row's version and remain the
decision-grade value (PR #1144 review).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wFpjrd126rVe6VisiYVDD
@phernandez phernandez changed the title feat(indexing): relay self-supersede on stale base + db_version provenance feat(core): relay self-supersede on stale base + db_version provenance Jul 23, 2026
phernandez and others added 2 commits July 23, 2026 13:12
…nance (#1589)

Two changes for the basic-memory-cloud #1589 collab persistence redesign:

1. Relay self-supersede: the accepted-note base-checksum precondition now
   accepts a stale base when BOTH the current accepted version's last_source
   and the incoming request source are collaboration_relay. A relay persist
   that times out client-side after committing leaves the relay's recorded
   base one version behind its own write; rejecting that wedges every
   subsequent store in a 409 loop (observed live in cloud production
   2026-07-23: zero persists for 10+ minutes while the user typed). The
   relay's next snapshot always supersedes its own prior write - the live
   Y.Doc is the merge of everything the relay ever persisted. Foreign
   writers keep full guarded semantics; the deleted-entity 409 also stays.

2. db_version provenance on index results: RuntimeNoteObjectProvenance now
   parses bm-db-version alongside actor/source; it threads through
   IndexedFileLiveUpdatePlan, IndexFileJobResult, and
   IndexFileNoteLiveUpdatePlan (withheld for superseded content, like
   content_checksum) so cloud's index-completion live updates can carry the
   monotonic version. Consumers then decide echo-vs-out-of-band by version
   arithmetic instead of checksum guessing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wFpjrd126rVe6VisiYVDD
Signed-off-by: phernandez <paul@basicmachines.co>
… plan

Object metadata versions can lag the accepted row (content-identical accepts
bump db_version without a new materialization) but can never exceed it, so a
stale event version reads as an echo of identical content downstream - the
correct skip. Authoritative reads carry the DB row's version and remain the
decision-grade value (PR #1144 review).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wFpjrd126rVe6VisiYVDD
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez
phernandez force-pushed the feat-1589-self-write branch from 4993e71 to c6099e6 Compare July 23, 2026 18:12
@phernandez
phernandez merged commit fc622ff into main Jul 23, 2026
22 checks passed
@phernandez
phernandez deleted the feat-1589-self-write branch July 23, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant