fix(sync): stop the per-cycle resync after another device deleted attachments the phone still listed (#1136)

Author: dongdongbh

docs/release-notes/unreleased.md

@@ -5,3 +5,5 @@ Changes collected after `v1.2.7` and before the next version tag.
 ## Full Change List
 
 - Sync: leaving the app mid-sync (switching to another app on a phone, or a background sync hitting its deadline) could leave the shared sync lock behind on Dropbox and WebDAV, and every device then reported "Remote sync is temporarily reserved by mindwtr-mobile" and waited up to five minutes before syncing again. The abort cancelled the request that removes the lock. Lock requests on desktop and mobile now finish independently of the abort, so an interrupted cycle still releases its lock. The "Sync follow-up scheduled" log line also now reports the delay that actually applies instead of only the pacing delay. (from a v1.2.7 device log)
+
+- Sync: a phone could keep syncing every second or two with no changes, warning "syncConflictDiscarded" for the same attachments on every cycle, after another device had deleted attachments the phone still listed. Two causes are fixed. The app kept its own older copy of a task whenever only its attachments had changed, and then wrote that copy back over what sync had just stored. And two devices that hold the same records in a different order were treated as different, so every cycle uploaded again and a self-hosted server answered with a merge each time. The order of records no longer counts as a change, and attachment-only changes now replace the in-memory task. (#1136)

packages/core/src/store-helpers.test.ts

@@ -20,7 +20,7 @@ import {
     reuseSettingsIfEquivalent,
     selectFocusedCount,
 } from './store-helpers';
-import type { Project, Section, Task } from './types';
+import type { Attachment, Project, Section, Task } from './types';
 import type { SaveBaseState } from './store-types';
 
 const createTask = (
@@ -1021,3 +1021,36 @@ describe('persist', () => {
         expect(debouncedSave).not.toHaveBeenCalled();
     });
 });
+
+describe('reconcileEntityCollection attachments (#1136)', () => {
+    const attachment = (overrides: Partial<Attachment> = {}): Attachment => ({
+        id: 'att-1',
+        kind: 'file' as const,
+        title: 'scan.pdf',
+        uri: 'file:///attachments/att-1.pdf',
+        createdAt: '2026-01-01T00:00:00.000Z',
+        updatedAt: '2026-01-01T00:00:00.000Z',
+        ...overrides,
+    });
+
+    it('replaces an owner whose attachments changed under an unchanged revision tuple', () => {
+        const existing = createTask('t1', 'project-1', 0, { attachments: [attachment()] });
+        const incoming = createTask('t1', 'project-1', 0, {
+            attachments: [attachment({ deletedAt: '2026-01-01T00:00:00.000Z' })],
+        });
+
+        expect(hasSameEntityIdentity(existing, incoming)).toBe(false);
+        const result = reconcileEntityCollection([existing], buildEntityMap([existing]), [incoming]);
+        expect(result.items[0]).toBe(incoming);
+    });
+
+    it('still reuses an owner whose attachments are equal by content', () => {
+        const existing = createTask('t1', 'project-1', 0, { attachments: [attachment()] });
+        const incoming = createTask('t1', 'project-1', 0, { attachments: [attachment()] });
+
+        expect(hasSameEntityIdentity(existing, incoming)).toBe(true);
+        const result = reconcileEntityCollection([existing], buildEntityMap([existing]), [incoming]);
+        expect(result.items).toEqual([existing]);
+        expect(result.items[0]).toBe(existing);
+    });
+});

packages/core/src/store-helpers.ts

@@ -10,7 +10,7 @@ import { isTaskActionable, isTaskFinished } from './task-status';
 import { safeParseDate } from './date';
 import { filterNotDeleted } from './sync-helpers';
 import { nextRevision, normalizeRevision } from './sync-revision';
-import type { AiSettings, AppData, Area, Person, Project, Section, Task, TaskStatus } from './types';
+import type { AiSettings, AppData, Area, Attachment, Person, Project, Section, Task, TaskStatus } from './types';
 import { generateUUID as uuidv4 } from './uuid';
 import type { DerivedState, SaveBaseState, TaskStore } from './store-types';
 
@@ -511,12 +511,33 @@ export const reuseArrayIfShallowEqual = <T>(previous: T[], next: T[]): T[] => (
         : next
 );
 
+// Attachments carry their own per-record LWW (deletedAt, cloudKey, localStatus,
+// contentRev) and a merge can change them WITHOUT touching the owner's revision
+// tuple. Reusing the existing owner object on that tuple alone kept a task's
+// pre-merge attachments alive in the store; the post-load persist then wrote
+// them back over what the sync cycle had just stored, every cycle (#1136).
+const haveSameAttachments = (left?: Attachment[], right?: Attachment[]): boolean => {
+    if (left === right) return true;
+    const leftItems = left ?? [];
+    const rightItems = right ?? [];
+    if (leftItems.length !== rightItems.length) return false;
+    for (let index = 0; index < leftItems.length; index += 1) {
+        if (leftItems[index] === rightItems[index]) continue;
+        if (JSON.stringify(leftItems[index]) !== JSON.stringify(rightItems[index])) return false;
+    }
+    return true;
+};
+
 export const hasSameEntityIdentity = <T extends EntityWithRevision>(existing: T, incoming: T): boolean => (
     existing.updatedAt === incoming.updatedAt
     && normalizeRevision(existing.rev) === normalizeRevision(incoming.rev)
     && existing.revBy === incoming.revBy
     && existing.deletedAt === incoming.deletedAt
     && existing.purgedAt === incoming.purgedAt
+    && haveSameAttachments(
+        (existing as { attachments?: Attachment[] }).attachments,
+        (incoming as { attachments?: Attachment[] }).attachments,
+    )
 );
 
 export const reconcileEntityCollection = <T extends EntityWithRevision>(

packages/core/src/sync-helpers.test.ts

@@ -4,6 +4,7 @@ import {
     assertNoPendingAttachmentUploads,
     computeCoveredSettingsFingerprint,
     computeSyncChangeFingerprint,
+    computeStableValueFingerprint,
     computeSyncPayloadFingerprint,
     findPendingAttachmentUploads,
     hasPendingSyncSideEffects,
@@ -1042,3 +1043,55 @@ describe('sync-helpers computeSyncPayloadFingerprint', () => {
         expect(computeSyncPayloadFingerprint(data)).toBe(computeSyncPayloadFingerprint(data));
     });
 });
+
+describe('sync comparison ignores the order of id-keyed lists (#1136)', () => {
+    const attachment = (id: string): Attachment => ({
+        id,
+        kind: 'file',
+        title: `${id}.pdf`,
+        uri: '',
+        createdAt: '2026-01-01T00:00:00.000Z',
+        updatedAt: '2026-01-01T00:00:00.000Z',
+    });
+    const task = (id: string, attachments: Attachment[]) => ({
+        id,
+        title: id,
+        status: 'inbox' as const,
+        tags: [],
+        contexts: [],
+        createdAt: '2026-01-01T00:00:00.000Z',
+        updatedAt: '2026-01-01T00:00:00.000Z',
+        attachments,
+    });
+    const doc = (tasks: ReturnType<typeof task>[]): AppData => ({
+        tasks: tasks as unknown as AppData['tasks'],
+        projects: [],
+        sections: [],
+        areas: [],
+        people: [],
+        settings: {},
+    });
+
+    it('treats a merge that listed the same records local-first on each device as equal', () => {
+        const phone = doc([task('t2', [attachment('a2'), attachment('a1')]), task('t1', [])]);
+        const server = doc([task('t1', []), task('t2', [attachment('a1'), attachment('a2')])]);
+
+        expect(areSyncPayloadsEqual(phone, server)).toBe(true);
+        expect(computeStableValueFingerprint(phone)).toBe(computeStableValueFingerprint(server));
+        expect(computeSyncPayloadFingerprint(phone)).toBe(computeSyncPayloadFingerprint(server));
+    });
+
+    it('still sees a content difference behind a reorder', () => {
+        const phone = doc([task('t2', [attachment('a2'), attachment('a1')])]);
+        const server = doc([task('t2', [attachment('a1'), { ...attachment('a2'), deletedAt: '2026-01-02T00:00:00.000Z' }])]);
+
+        expect(areSyncPayloadsEqual(phone, server)).toBe(false);
+    });
+
+    it('keeps primitive lists positional', () => {
+        expect(areSyncPayloadsEqual(
+            { ...doc([]), settings: { contexts: ['a', 'b'] } as AppData['settings'] },
+            { ...doc([]), settings: { contexts: ['b', 'a'] } as AppData['settings'] },
+        )).toBe(false);
+    });
+});

packages/core/src/sync-helpers.ts

@@ -315,9 +315,24 @@ export const sanitizeAppDataForRemote = (data: AppData): AppData => {
     };
 };
 
+const isIdKeyed = (item: unknown): item is { id: string } => (
+    !!item && typeof item === 'object' && typeof (item as { id?: unknown }).id === 'string'
+);
+
+// Lists of id-keyed records (entities, attachments, checklist items) compare
+// by content, not position. A merge emits the local side's order first, so two
+// devices that added records concurrently hold the same set in different
+// orders forever; comparing positionally made every fingerprint differ, every
+// cycle upload, and the self-hosted server report a merge each time (#1136).
+// Position carries no meaning for these lists: ordering lives in explicit
+// order fields, and every reorder bumps the owner's updatedAt anyway.
 const normalizeForSyncComparison = (value: unknown): unknown => {
     if (Array.isArray(value)) {
-        return value.map((item) => normalizeForSyncComparison(item));
+        const items = value.map((item) => normalizeForSyncComparison(item));
+        if (items.length > 1 && items.every(isIdKeyed)) {
+            items.sort((left, right) => (left.id < right.id ? -1 : left.id > right.id ? 1 : 0));
+        }
+        return items;
     }
     if (value && typeof value === 'object') {
         const record = value as Record<string, unknown>;
@@ -352,7 +367,7 @@ const hashStableSyncJson = (value: string): string => {
 
 export const computeStableValueFingerprint = (value: unknown): string => {
     const json = toStableSyncJson(value);
-    return `stable-v1:${json.length}:${hashStableSyncJson(json)}`;
+    return `stable-v2:${json.length}:${hashStableSyncJson(json)}`;
 };
 
 export const computeSyncPayloadFingerprint = (data: AppData): string =>