[P1 / data-integrity] Sync engine cursor race on parallel reconnect push (V6 CRDT repro)

[h4yfans]

⚠️ Severity: P1 — silent data-divergence risk on multi-device offline workflows.
This is not just a flaky e2e test. See Production impact below.

Production impact

Scenarios that reproduce this in the real product

Any two devices that:

1. Both go offline (weak wifi, plane/metro, conference network, router restart)
2. Each creates new notes / tasks / etc. locally while offline
3. Both come back online at roughly the same time (auto-reconnect fires in parallel)

Concrete likely user flows:

• Laptop + phone both drop wifi → user captures notes on both → wifi returns → both devices auto-sync simultaneously.
• Home router restart → desktop + laptop rejoin the network together.
• User takes two devices from airplane mode → wakes both within a few seconds of each other.

User-visible failure mode — silent

• No error. No toast. No sync-failed indicator. Sync engine reports idle + pendingCount: 0.
• The note created on device B simply never appears on device A (or vice versa, asymmetric).
• User on device A thinks "did I forget to write that down?" or "did I lose my note?" → data-loss perception even though the data is safe on device B.
• Trust erosion is proportionally higher than the actual data state because the app lied about sync being complete.

Self-heal? Currently appears no.

Theoretical self-heal path: engine.ts runs periodicPull every 60s (setInterval(() => this.pullCoordinator.periodicPull(), 60_000)). If that pull asked "changes since LAST_CURSOR" it should re-fetch the skipped item on a subsequent tick.

Empirical reality from V6 reproducer: V6 tested with expect.poll({ timeout: 180_000, intervals: [500, 2000, 5000] }) wrapping a syncBothAndWait (3 manual fullSync triggers per cycle). Over 180 seconds this fires ~50+ manual fullSync cycles on each device. Convergence still does not happen. The cursor skip is persistent, not just delayed.

This means a real user hitting this bug may need another write on device B (which shifts B's own cursor and makes the next pull cross the gap) before device A catches up. If the user never writes to B again, the note may never propagate.

Observability

• No telemetry today captures "peer-to-peer propagation success rate" after a reconnect — we can't measure how often this bites users.
• Before the engine fix, worth adding a metric: on each /sync/changes response, record (last_known_peer_cursor - my_cursor) gap. A non-zero sustained gap after a round-trip is the smoking gun.

Summary

When two devices go offline, create new records locally, and reconnect in parallel, one side deterministically misses the other side's record on the first post-reconnect pull. The test body-crdt-coverage-variants.e2e.ts V6 is a faithful reproducer. It's skipped on CI in #272 pending an engine-level fix.

Fingerprint: cursor race, not flake

• Failure is asymmetric and stable within a run: always noteB on A never converges while noteA on B does (or vice versa depending on push ordering).
• Six prior fix(e2e): stabilize ... commits tried test-layer workarounds (longer polls, bigger timeouts, more sync round-trips, waitForCrdtQueueIdle bracketing). None held under CI xvfb timing.
• Locally the test usually passes in ~20s because the second syncBothAndWait round happens to fire at a moment the cursor gap is already covered; under CI load the gap persists.
• Local failure rate is non-zero (~30% in my sync-in-poll refactor run). CI at ~100% is just a timing amplifier; same bug, more exposure.

Root cause (traced in apps/desktop/src/main/sync/engine.ts)

On parallel reconnect, both devices fire handleNetworkChange({online: true}) → scheduleSync(() => this.reconnectSync(...)) → fullSync() → pull → seed → push.

1. Server serializes the two pushes: A's noteA lands at cursor N, B's noteB lands at cursor N+1.
2. Each device's PushCoordinator advances its local LAST_CURSOR to the cursor returned for its own push (N for A, N+1 for B).
3. Subsequent pull asks the server for "changes since my cursor". A asks > N, B asks > N+1.
4. A's pull > N should return noteB. But if the auto-reconnect fullSync on A ran its pull BEFORE B's push landed, then A already did its pull and won't do another until the next timer tick (60s periodicPull in engine.ts).
5. When the test then calls triggerSync(A) via syncAndWait, the engine enters a fresh fullSync. The cursor is still at N from A's own push. Pull > N should return noteB. But if A's previous fullSync already advanced the cursor past N (e.g., to max_server_cursor at the end of a pull batch), the next pull asks > max_server_cursor and sees nothing.

The exact off-by-one depends on PullCoordinator's cursor-advancement policy after each pull page. The practical outcome: one-tick gap in the cursor range [N, N+1] that no single device owns for pull purposes.

Proposed fixes (pick one, both are contracts)

Option A — Client-side: don't advance LAST_CURSOR past own push

Change PushCoordinator (and/or wherever SYNC_STATE_KEYS.LAST_CURSOR is updated on push ack) to set LAST_CURSOR = max(LAST_CURSOR, server_returned_cursor - 1). Keep one tick of overlap so the next pull window guarantees visibility of concurrent peers' items. Client-only change. Downside: next pull returns own push → needs client-side dedup by id (probably already present in item handlers).

Option B — Server-side: inclusive cursor on /sync/changes

Change the sync server's /sync/changes endpoint to interpret cursor as "strictly greater than or equal to" instead of "strictly greater than". Combined with client dedup, this guarantees no gap regardless of client-side cursor advancement.

Reproducer

# Ensure Electron ABI is built
bash apps/desktop/scripts/ensure-native.sh electron
npx electron-vite build

# Un-skip V6 locally (remove the CI check) then run it repeatedly
for i in 1 2 3 4 5; do
  TZ=UTC npx playwright test tests/e2e/body-crdt-coverage-variants.e2e.ts:565 \
    --config config/playwright.config.ts --reporter=list --workers=1 --retries=0
done

Under CI xvfb simulation (TZ=UTC CI=1 + un-skipping the test), failure rate is ~100% with asymmetric direction.

Acceptance

• Either Option A or B landed
• V6 un-skipped in body-crdt-coverage-variants.e2e.ts (remove the test.skip(!!process.env.CI, ...) line)
• V6 passes 5 consecutive CI runs on shard 1/3
• Regression test: a new deterministic two-device parallel-create variant added (not relying on luck of scheduling)
• Telemetry: add cursor-gap metric on /sync/changes response so prod incidence is measurable

References

• PR fix(e2e): replace wall-clock waits with product-emitted signals #271 — test primitives + hasNoteOnDevice hook + calendar fixes
• PR fix(e2e): skip V6 CRDT on CI — sync engine cursor race #272 — this CI skip with the pointer here
• apps/desktop/src/main/sync/engine.ts — reconnectSync, scheduleSync, handleNetworkChange
• apps/desktop/src/main/sync/engine/full-sync-runner.ts — pull → seed → push ordering
• apps/desktop/.claude/CLAUDE.md memory notes on "CRDT Sign-Out/Sign-In Fix (2026-02-27)" describe a related ordering fix