Skip to content

port upstream #7875: downstream origin marker to stop silent pull drops - #21

Open
artemlitch wants to merge 2 commits into
readwise-rxdb15from
port-7875-downstream-origin-marker
Open

port upstream #7875: downstream origin marker to stop silent pull drops#21
artemlitch wants to merge 2 commits into
readwise-rxdb15from
port-7875-downstream-origin-marker

Conversation

@artemlitch

@artemlitch artemlitch commented Aug 20, 2026

Copy link
Copy Markdown

Problem

When a pulled document's fork state differs from its assumed-master snapshot, the downstream assumes a local write is pending and skips applying the pull. The checkpoint still advances. If the divergence was actually caused by a crash between the fork write and the meta write (or a lost meta write), no push ever comes: the document is wedged, and every future server update for it is dropped silently. This is upstream bug pubkey#7804, fixed in 17.0.0 by pubkey#7875. Our fork is 15.27.0, so we port the fix.

Fix

When the downstream writes a pulled document into the fork, it stamps _meta.o = { _rev, hash } in the same atomic write: the revision height this write produces, plus the replication identifier hash. A later local edit bumps the revision height and voids the marker. The field name and shape match upstream 17.x exactly, so databases stay wire-compatible on this field.

At the skip site, a valid marker (height matches the doc's current revision, hash matches this replication) proves the fork copy is untouched server output, so the divergence can only be a lost meta write. The skip is bypassed: the pull applies and the meta repairs. A voided or absent marker keeps today's behavior, so pending local edits stay protected. Upstream's skipStoringPullMeta clause is dropped; that option does not exist in 15.x.

Not ported: the conflicts.ts half

Upstream pubkey#7875 also removes the isEqual short-circuit in resolveConflictError, so an equal-conflict still resolves and writes the assumed-master meta. That is expressible in 17.x because the conflict handler has separate isEqual() and resolve() methods. In 15.x the handler is one function whose equal output carries no documentData, so there is nothing to write; any port must invent the resolution. The obvious adaptation (substitute realMasterState on equality) works but costs one extra master write per equal-conflict and breaks the protocol test asserting the master stays at revision 1- when both sides insert identical documents. Upstream's own version of that test passes only because their throwing test-handler suppresses all writes, not because the invariant holds.

Consequence: one wedge class remains open in this fork. A document present with identical content on both sides before replication starts never gets an assumed-master row: the downstream skips it (no assumed master), the push classifies it as a conflict, the handler reports equality, and no meta is ever written. The marker cannot help because the fork copy was never written by the downstream. This class needs a follow-up (15.x-native fix or the adaptation with the protocol test updated), tracked with the retroactive-repair work.

Verification

  • Ported upstream's two marker regression tests into test/unit/replication.test.ts (the third upstream test exercises the unported conflicts.ts half and is omitted). Test 1 fails on the pre-port source with the wedge ('FirstUpdate' served instead of 'SecondUpdate') and passes with the port; test 2 is a guard for the still-skipped case and passes on both.
  • replication.test.ts + replication-protocol.test.ts + conflict-handling: 53 passing, 0 failing on DEFAULT_STORAGE=memory; graphql/websocket replication suites 72 passing. Full replication.test.ts with --no-bail additionally shows one pre-existing attachment-test failure that reproduces identically on readwise-rxdb15 (bootstrap artifact of running the file standalone; the suite's bail: true config hid it).
  • Verified end to end on an iOS simulator against the Bookwise app with the local build: a doc with a stale meta row and no possible push (below the upstream checkpoint) wedges permanently on the current pin and heals on this branch, with the marker re-stamped to the new revision height by the downstream write. A real local status edit through the UI bumps the revision and voids the marker, and its pull protection is unchanged.
  • A fresh transpile from the committed source reproduces the committed dist with zero diff.

One shared-with-upstream caveat: with keepMeta and a master-supplied _rev, the predicted marker height can be wrong; the marker then never matches and behavior degrades to the pre-port skip. Fails safe; we do not use keepMeta.

Second commit is the regenerated dist (consumers install prebuilt output from GitHub).

artemlitch and others added 2 commits August 20, 2026 15:31
…se conflict skip

The downstream skips a pulled document when the fork state differs from the assumed master state, on the assumption that a local write is waiting for the upstream to resolve it. A lost meta write or a crash between the fork write and the meta write produces the same difference without any local write, so the document is skipped on every later pull while the checkpoint keeps advancing.

The downstream now records the origin of its own fork writes in _meta.o (identifier hash plus the revision height it is about to write). When fork and assumed master differ but the marker matches the current fork revision, the difference came from the downstream itself, so the skip is bypassed and the pulled document is applied. A local write bumps the revision height and voids the marker, so real local writes are still protected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@artemlitch

Copy link
Copy Markdown
Author

Follow-up on the "one wedge class remains open" paragraph: the class is real at the protocol level but turns out to be unreachable for Bookwise tracked_books, proven by a vitest repro against this branch (readwiseio/rekindled@c7deac0c4c, collectionSyncer.pullApplyLedger.test.ts).

The reason is an accidental shape mismatch in the app's sync config. The push modifier sends wire-shaped docs (booleans, convertBooleanIntegersToBooleans, collectionSyncer.ts:174-177) while the push handler transforms response docs to client shape (integers, collectionSyncer.ts:169-171). At replication-resolve-conflict the handler therefore compares is_sample: false against is_sample: 0 and never returns isEqual: true — the "equal -> do nothing" short-circuit that creates the wedge is dead code on this path. Every first-push conflict resolves master-wins (same content, invisible) and the resolution writes the assumed-master row, so the doc self-heals immediately.

The rekindled test is written as a tripwire: if the push shapes are ever aligned, it fails and the wedge class opens up. The unported conflicts.ts half of upstream pubkey#7875 is effectively a no-op for Bookwise as long as that mismatch stands.

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