fix(sync): stop a phantom conflict deleting a record everywhere — 2.6.53

CHANGELOG.md

@@ -1,6 +1,6 @@
 # CHANGELOG
 
-## [2.6.52] - 05.09.2026
+## [2.6.53] - 05.09.2026
 
 **New**
 

app/build.gradle

@@ -21,7 +21,7 @@ apply from: "$projectDir/gradle/libs-task.gradle"
 apply from: "$projectDir/gradle/changelog-task.gradle"
 
 
-def appVersionCode = 52
+def appVersionCode = 53
 def appVersionName = "2.6.${appVersionCode}"
 
 def gitCommitHashProvider = providers.exec {

app/src/androidTest/java/com/pasich/mynotes/db/RoomSyncStoreTest.java

@@ -789,6 +789,83 @@ public void resolveConflict_dropsAConflictTheRecordHasMovedPastInsteadOfApplying
                 .isEqualTo(5_000L);
     }
 
+    @Test
+    public void resolveConflict_dropsARowWhoseWinnerTheRecordHasAlreadyLeftBehind()
+            throws Exception {
+        // The record now equals the row's alternative — another device kept it — so the
+        // pre-selected winner is a version the record has moved past. Offered anyway, it was
+        // applied on a tap; here the winner is a deletion.
+        int noteId = seedNote("loser", "body", null);
+        db.syncMetadataDao().touch(SyncMetadata.RECORD_TYPE_NOTE, noteId, 5_000L);
+        com.pasich.mynotes.data.database.entities.SyncConflictEntity row =
+                noteConflictRow(
+                        "11111111-1111-4111-8111-111111111111", "deleted-winner", 3_000L, 2_000L);
+        row.winnerJson =
+                "{\"type\":\"note\",\"id\":\"11111111-1111-4111-8111-111111111111\","
+                        + "\"updatedAt\":\"1970-01-01T00:00:03Z\",\"deletedAt\":\"1970-01-01T00:00:03Z\","
+                        + "\"payload\":{}}";
+        row.winnerTombstone = true;
+        db.syncConflictDao().insertIgnoringDuplicates(Collections.singletonList(row));
+        long conflictId = db.syncConflictDao().getAll().get(0).id;
+
+        store.resolveConflict(conflictId, SyncResolution.KEEP_WINNER);
+
+        assertThat(db.noteDao().getNoteSync(noteId)).isNotNull();
+        assertThat(db.noteDao().getNoteSync(noteId).getTitle()).isEqualTo("loser");
+        assertThat(db.syncConflictDao().getById(conflictId)).isNull();
+    }
+
+    @Test
+    public void applySnapshot_doesNotRecordThePreferencesBaseUntilTheCommitSucceeded()
+            throws Exception {
+        // Recorded inside the transaction, a base for a version that was then never committed
+        // made the next build publish the local settings over the other device's change with
+        // no conflict.
+        PreferencesAdapter adapter = new PreferencesAdapter();
+        RoomSyncStore preferencesStore = new RoomSyncStore(context, db, adapter.helper);
+        preferencesStore.readState();
+        // A build first, as every sync does: it records the live digest as the baseline, so the
+        // apply below sees unchanged settings rather than a leftover baseline from another test.
+        preferencesStore.buildSnapshot();
+        adapter.succeeds.set(false);
+        db.syncMetadataDao().setVersion(SyncMetadata.RECORD_TYPE_PREFERENCES, 0, 1_000L, null);
+        SyncRecord remote =
+                SyncRecord.live(
+                        SyncRecord.Type.PREFERENCES,
+                        "00000000-0000-4000-8000-000000000000",
+                        java.time.Instant.ofEpochMilli(2_000L),
+                        new com.google.gson.Gson()
+                                .toJsonTree(preferencesWithTheme(3))
+                                .getAsJsonObject());
+
+        try {
+            preferencesStore.applySnapshot(
+                    new SyncSnapshot(Collections.singletonList(remote)), Collections.emptyList());
+            throw new AssertionError("Expected the failed commit to propagate");
+        } catch (IOException expected) {
+            // The journal stays for recovery; the base must not claim the version landed.
+        }
+
+        assertThat(
+                        db.syncMetadataDao()
+                                .getByStableId(
+                                        SyncMetadata.RECORD_TYPE_PREFERENCES,
+                                        "00000000-0000-4000-8000-000000000000")
+                                .syncedVersionId)
+                .isNull();
+
+        adapter.succeeds.set(true);
+        preferencesStore.applySnapshot(
+                new SyncSnapshot(Collections.singletonList(remote)), Collections.emptyList());
+        assertThat(
+                        db.syncMetadataDao()
+                                .getByStableId(
+                                        SyncMetadata.RECORD_TYPE_PREFERENCES,
+                                        "00000000-0000-4000-8000-000000000000")
+                                .syncedVersionId)
+                .isEqualTo(remote.getCanonicalPayloadHash());
+    }
+
     /** A stored note conflict whose winner is titled after its version id. */
     private com.pasich.mynotes.data.database.entities.SyncConflictEntity noteConflictRow(
             String stableId, String winnerVersionId, long winnerUpdatedAt, long loserUpdatedAt) {

app/src/androidTest/java/com/pasich/mynotes/db/TwoPeerStoreConvergenceTest.java

@@ -61,6 +61,66 @@ public void setUp() {
     public void tearDown() {
         a.db.close();
         b.db.close();
+        if (c != null) c.db.close();
+    }
+
+    private Peer c;
+
+    @Test
+    public void aRecordRevivedElsewhereLeavesNoPhantomDeletionOnAPeerThatNeverHeldIt()
+            throws Exception {
+        // B edits, C deletes, B keeps the edit. A — a fresh install that never held the record —
+        // received the deletion-versus-edit conflict along the way. Once the edit arrives on A the
+        // row is meaningless: offered anyway, pre-selected on the deletion, one tap deleted the
+        // record on every device with no conflict raised anywhere.
+        RecordKind kind = new TaskRecord();
+        c = new Peer();
+        kind.create(b, "TaskX");
+        clock.advance();
+        assertClean(b.sync());
+        clock.advance();
+        assertClean(c.sync());
+        clock.advance();
+        assertClean(b.sync());
+
+        kind.edit(b, "TaskX-B");
+        clock.advance();
+        kind.delete(c);
+        clock.advance();
+        assertClean(c.sync());
+        clock.advance();
+        assertThat(b.sync().getStatus()).isEqualTo(SyncState.Status.SUCCESS);
+        assertThat(b.store.getUnresolvedConflicts()).hasSize(1);
+        clock.advance();
+        assertThat(a.sync().getStatus()).isEqualTo(SyncState.Status.SUCCESS);
+
+        // B keeps its edit: the winner was the deletion, so it is the alternative that stays.
+        clock.advance();
+        for (SyncConflictEntity conflict : b.store.getUnresolvedConflicts()) {
+            b.store.resolveConflict(
+                    conflict.id,
+                    conflict.winnerTombstone
+                            ? SyncResolution.KEEP_ALTERNATIVE
+                            : SyncResolution.KEEP_WINNER);
+        }
+        clock.advance();
+        assertClean(b.sync());
+        clock.advance();
+        assertClean(a.sync());
+
+        // The record arrived on A; nothing is left there to tap, and it lives on everywhere.
+        assertThat(a.store.getUnresolvedConflicts()).isEmpty();
+        assertThat(kind.title(a)).isEqualTo("TaskX-B");
+        clock.advance();
+        assertClean(c.sync());
+        assertThat(kind.title(c)).isEqualTo("TaskX-B");
+        clock.advance();
+        assertClean(a.sync());
+        clock.advance();
+        assertClean(b.sync());
+        assertThat(kind.title(b)).isEqualTo("TaskX-B");
+        assertThat(b.store.getUnresolvedConflicts()).isEmpty();
+        assertThat(c.store.getUnresolvedConflicts()).isEmpty();
     }
 
     @Test

app/src/main/java/com/pasich/mynotes/data/sync/RoomSyncStore.java

@@ -308,10 +308,21 @@ private void applySnapshotInternal(
                                                         localId,
                                                         record.getCanonicalPayloadHash());
                                     }
+                                    // A row stored here before this device held the record — a
+                                    // conflict replicated from elsewhere — may name a winner
+                                    // that is no longer the live version. Left open, it was
+                                    // offered pre-selected on that winner, and when the winner
+                                    // was a deletion one tap deleted the record everywhere.
+                                    retireConflictsSupersededBy(record);
                                     transactionFailureInjector.afterRecordApplied(record);
                                     continue;
                                 }
-                                if (metadata == null) continue;
+                                if (metadata == null) {
+                                    // A deletion of a record this device never held: nothing to
+                                    // apply, but an open row for it must still follow the version.
+                                    retireConflictsSupersededBy(record);
+                                    continue;
+                                }
                                 if (record.getType() == SyncRecord.Type.PREFERENCES
                                         && !record.isTombstone()) {
                                     // Decided last, once every other record has been applied:
@@ -414,14 +425,11 @@ private void applySnapshotInternal(
                                                     preferencesMetadata[0].localId,
                                                     stagedPreferencesUpdatedAt,
                                                     null);
-                                    if (preferencesRecord != null) {
-                                        database.syncMetadataDao()
-                                                .setSyncedVersion(
-                                                        preferencesMetadata[0].recordType,
-                                                        preferencesMetadata[0].localId,
-                                                        preferencesRecord
-                                                                .getCanonicalPayloadHash());
-                                    }
+                                    // The synced version is recorded only once the commit below
+                                    // has succeeded: recorded here, a refused or failed commit
+                                    // left the base naming a version that was never applied, and
+                                    // the next build then published the local settings over the
+                                    // other device's change with no conflict.
                                     database.syncPendingPreferencesDao()
                                             .upsert(
                                                     new SyncPendingPreferencesEntity(
@@ -447,18 +455,35 @@ private void applySnapshotInternal(
             throw error.ioException;
         }
         if (deferFinalState || preferencesBaseline[0] != null) {
+            boolean committed = false;
             if (preferencesBaseline[0] != null) {
                 // The journal is only dropped once the adapter reports a durable commit; a
                 // failure here leaves it in place for recoverPendingPreferences and keeps the
                 // sync state retryable rather than claiming success. A commit refused because
                 // the settings moved in the meantime is not a failure: the journal is dropped
                 // and the edit is published by the next build.
-                commitPendingPreferences(
-                        stagedPreferences, stagedPreferencesTarget, preferencesBaseline[0]);
+                committed =
+                        commitPendingPreferences(
+                                stagedPreferences, stagedPreferencesTarget, preferencesBaseline[0]);
             }
+            boolean recordBase = committed && preferencesRecord != null;
             database.runInTransaction(
                     () -> {
                         database.syncPendingPreferencesDao().clear();
+                        if (recordBase) {
+                            SyncMetadataEntity metadata =
+                                    database.syncMetadataDao()
+                                            .getByStableId(
+                                                    SyncMetadata.RECORD_TYPE_PREFERENCES,
+                                                    PREFERENCES_STABLE_ID);
+                            if (metadata != null) {
+                                database.syncMetadataDao()
+                                        .setSyncedVersion(
+                                                metadata.recordType,
+                                                metadata.localId,
+                                                preferencesRecord.getCanonicalPayloadHash());
+                            }
+                        }
                         if (finalState != null)
                             database.syncStateDao().upsert(toEntity(finalState));
                     });
@@ -1095,6 +1120,11 @@ public void resolveConflict(long conflictId, @NonNull SyncResolution resolution)
      * was dropped unsettled instead of offered. Its alternative came back at the next sync as a
      * fresh conflict, was dropped again after the next resolution, and the account never settled.
      * Only content the user actually changed since the conflict was recorded counts.
+     *
+     * <p>The one content that keeps a row alive is the row's own winner. A record that now equals
+     * the row's alternative was switched to it — by a resolution here or on another device — so the
+     * pre-selected winner is a version the record has left behind; offered anyway, and when that
+     * winner was a deletion, one tap deleted the record everywhere.
      */
     private boolean isSuperseded(@NonNull SyncConflictEntity conflict) throws IOException {
         SyncMetadataEntity metadata =
@@ -1104,9 +1134,8 @@ private boolean isSuperseded(@NonNull SyncConflictEntity conflict) throws IOExce
                         <= Math.max(conflict.winnerUpdatedAt, conflict.loserUpdatedAt)) {
             return false;
         }
-        String current = contentDigest(metadata);
-        return !current.equals(contentDigest(conflict.recordType, conflict.winnerJson))
-                && !current.equals(contentDigest(conflict.recordType, conflict.loserJson));
+        return !contentDigest(metadata)
+                .equals(contentDigest(conflict.recordType, conflict.winnerJson));
     }
 
     /** A digest of what the local record says, independent of when it last changed. */

app/src/main/java/com/pasich/mynotes/data/sync/SyncMerger.java

@@ -91,8 +91,13 @@ private void mergeVersions(
                 merged.put(key, local);
                 return;
             }
-            if (base.equals(local.getCanonicalPayloadHash())) {
-                // This device has not moved since it last synchronized; the remote has.
+            if (base.equals(local.getCanonicalPayloadHash())
+                    && !remote.getUpdatedAt().isBefore(local.getUpdatedAt())) {
+                // This device has not moved since it last synchronized; the remote has. Only
+                // forwards, though: a remote older than what this device synchronized is a head
+                // that has gone missing, and following it would revert the newer version on every
+                // unedited device until it existed nowhere. That case falls through to
+                // last-writer-wins below, which keeps the local copy and republishes it.
                 merged.put(key, remote);
                 return;
             }

app/src/test/java/com/pasich/mynotes/data/sync/SyncMergerTest.java

@@ -55,6 +55,21 @@ public void merge_takesTheRemoteWithoutAConflictWhenOnlyTheOtherSideMoved() {
         assertThat(result.getConflicts()).isEmpty();
     }
 
+    @Test
+    public void merge_doesNotFollowARemoteOlderThanWhatItLastSynchronized() {
+        // A head bundle gone missing from Drive makes the remote fall back to an older version.
+        // Taking it because "only the remote moved" reverted the newer copy on every unedited
+        // device until it existed nowhere; last-writer-wins keeps it and republishes it.
+        SyncRecord synced = note(NOTE_ID, TWENTY, "Milk and bread");
+        SyncRecord unchanged = synced.withBaseVersion(synced.getCanonicalPayloadHash());
+        SyncRecord olderRemote = note(NOTE_ID, TEN, "Milk");
+
+        SyncMergeResult result = merger.merge(snapshot(unchanged), snapshot(olderRemote));
+
+        assertThat(result.getMergedSnapshot().getRecords()).containsExactly(unchanged);
+        assertThat(result.getConflicts()).hasSize(1);
+    }
+
     @Test
     public void merge_stillReportsAConflictWhenBothSidesMovedFromTheSameBase() {
         SyncRecord synced = note(NOTE_ID, TEN, "Milk");

app/src/test/java/com/pasich/mynotes/data/sync/TwoPeerConvergenceTest.java

@@ -58,6 +58,55 @@ public void aCategoryDeletedOnOnePeerAndEditedOnTheOtherSettlesAfterOneResolutio
         deletedHereEditedThereSettlesAfterOneResolution(SyncRecord.Type.CATEGORY);
     }
 
+    @Test
+    public void aRecordRevivedElsewhereLeavesNoPhantomDeletionOnAPeerThatNeverHeldIt() {
+        // B edits, C deletes, B keeps the edit. A — a fresh install that never held the record —
+        // received the deletion-versus-edit conflict along the way. Once the edit arrives on A the
+        // row is meaningless: offered anyway, pre-selected on the deletion, one tap deleted the
+        // record on every device with no conflict raised anywhere.
+        SyncRecord.Type type = SyncRecord.Type.TASK;
+        Peer b = new Peer("B", clock, drive);
+        Peer c = new Peer("C", clock, drive);
+        Peer a = new Peer("A", clock, drive);
+        b.create(type, RECORD_ID, "TaskX");
+        clock.advance();
+        assertThat(b.sync().getConflictCount()).isEqualTo(0);
+        clock.advance();
+        assertThat(c.sync().getConflictCount()).isEqualTo(0);
+        clock.advance();
+        assertThat(b.sync().getConflictCount()).isEqualTo(0);
+
+        b.edit(type, RECORD_ID, "TaskX-B");
+        clock.advance();
+        c.delete(type, RECORD_ID);
+        clock.advance();
+        assertThat(c.sync().getConflictCount()).isEqualTo(0);
+        clock.advance();
+        assertThat(b.sync().getStatus()).isEqualTo(SyncState.Status.SUCCESS);
+        assertThat(b.store.pendingConflicts()).hasSize(1);
+        clock.advance();
+        a.sync();
+
+        // B keeps its edit and publishes it.
+        clock.advance();
+        b.store.resolveAllKeepingLive(clock.instant());
+        clock.advance();
+        assertThat(b.sync().getConflictCount()).isEqualTo(0);
+        clock.advance();
+        assertThat(a.sync().getConflictCount()).isEqualTo(0);
+
+        assertThat(a.store.pendingConflicts()).isEmpty();
+        assertThat(a.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B");
+        clock.advance();
+        assertThat(c.sync().getConflictCount()).isEqualTo(0);
+        assertThat(c.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B");
+        clock.advance();
+        assertThat(a.sync().getConflictCount()).isEqualTo(0);
+        clock.advance();
+        assertThat(b.sync().getConflictCount()).isEqualTo(0);
+        assertThat(b.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B");
+    }
+
     @Test
     public void aTaskSyncedBeforeBasesWereRecordedStillSettlesAfterOneResolution() {
         deletedHereEditedThereSettlesAfterOneResolution(SyncRecord.Type.TASK, true);
@@ -251,8 +300,7 @@ void resolveKeepingLive(ConflictRow row, Instant resolvedAt) {
                                                     .isAfter(row.conflict.getLoser().getUpdatedAt())
                                             ? row.conflict.getWinner().getUpdatedAt()
                                             : row.conflict.getLoser().getUpdatedAt())
-                    && !contentDigest(current).equals(contentDigest(row.conflict.getWinner()))
-                    && !contentDigest(current).equals(contentDigest(row.conflict.getLoser()))) {
+                    && !contentDigest(current).equals(contentDigest(row.conflict.getWinner()))) {
                 conflicts.remove(row);
                 return;
             }