diff --git a/docs/sync-and-op-log/conflict-journal-and-review.md b/docs/sync-and-op-log/conflict-journal-and-review.md index 62753f365c..de81845a3c 100644 --- a/docs/sync-and-op-log/conflict-journal-and-review.md +++ b/docs/sync-and-op-log/conflict-journal-and-review.md @@ -99,20 +99,34 @@ kept by synthesizing a single merged UPDATE op. Eligibility - neither side has a DELETE op, and the plan is not an archive plan; - neither side has opaque ops (their changes could not be carried into the - synthesized entity — merging would silently drop them and the two clients - would synthesize DIFFERENT entities); + synthesized delta — merging would silently drop them and the two clients + would synthesize DIFFERENT results); - both sides changed at least one real (non-noise) field; -- the two sides' non-noise changed-field sets are disjoint. +- the two sides' non-noise changed-field sets are disjoint; +- the entity has only ONE conflict in this batch. `detectConflicts` emits one + conflict per remote op with no per-entity aggregation, so an entity with ≥2 + concurrent remote ops would synthesize multiple merged ops whose clocks + dominate one another — a dominated sibling can be superseded and its field + silently dropped. Such entities fall back to whole-entity LWW (honest refusal; + per-entity aggregation into one op is a possible future improvement). **Convergence contract:** both clients must synthesize the byte-identical -merged entity regardless of which one performs the merge. Each client starts -from its own current state (`base + ownChanges`) and overlays the other side's -non-noise fields (disjoint, so nothing is clobbered); noise fields both sides -changed resolve via a deterministic `(timestamp, clientId)` tiebreak. +merged **changes delta** regardless of which one performs the merge. The delta +is the union of both sides' non-noise fields (disjoint, so nothing is clobbered) +plus the noise fields either side changed, resolved via a deterministic +`(timestamp, clientId)` tiebreak. Crucially the delta is derived ONLY from the +two sides' ops — **not** from either client's current entity snapshot. A +full-entity snapshot would drag along fields NEITHER side touched; if such an +untouched field momentarily differs between the two clients (an ordinary +staggered-sync race — e.g. one client already applied a third device's edit the +other has not), the two snapshots would differ, tie under LWW at the identical +`max(timestamp)`, and diverge PERMANENTLY. See `synthesizeMergedChanges`. **Atomicity / no-re-merge contract:** the merged resolution is exactly ONE new -UPDATE op with a **flat full-entity payload**, layered on top of both sides' -history like a normal edit — there is no history rewind. Because the payload is +UPDATE op carrying a **flat PARTIAL delta** (only the changed fields), layered +on top of both sides' history like a normal edit — there is no history rewind. +`lwwUpdateMetaReducer` applies it via `updateOne` (a shallow merge), so fields +outside the delta keep their own values on each client. Because the payload is flat (not `{ changes }`-shaped), `extractUpdateChanges` yields `{}` for it, so a merged op can never itself become disjoint-merge eligible: merges do not cascade or re-merge on later syncs. Merged resolutions are journaled with @@ -133,9 +147,15 @@ Per-entry actions (`SyncConflictUiService`): action — the same action a manual edit dispatches — so the operation-capture meta-reducer turns it into a synced op that propagates everywhere. No history rewind; a flip is a brand-new edit on top of current state. Before applying, - a **stale guard** compares the entity's current values to the journaled - winner values and asks for confirmation if the entity was edited after the - conflict resolved. + a **stale guard** asks for confirmation if the entity was edited after the + conflict resolved. It checks every field the flip will actually write: a + winner-changed field whose current value diverged from the journaled winner + value, OR a **loser-only** field (one the flip writes but the winner never + changed, so it is invisible to the winner values) whose current value is not + already what the flip would write. Without the loser-only check the guard is + blind to exactly the fields flip overwrites, and a post-resolution edit there + would be silently lost. The bulk flip path shows no dialog, so it **skips** + stale entries entirely rather than overwriting them. **Flip capability is deliberately narrow** (`canFlip`); everything else returns `unsupported`, keeps the entry `unreviewed`, and shows an error snack — diff --git a/src/app/op-log/sync/conflict-disjoint-merge.util.ts b/src/app/op-log/sync/conflict-disjoint-merge.util.ts index 3a40adc142..ac2d6003f8 100644 --- a/src/app/op-log/sync/conflict-disjoint-merge.util.ts +++ b/src/app/op-log/sync/conflict-disjoint-merge.util.ts @@ -7,9 +7,9 @@ * both sides' changed fields. * * No Angular, no I/O — deterministic, so the merge decision and the synthesized - * entity are unit-testable in isolation. Determinism is the whole point: both - * clients must arrive at the byte-identical merged entity regardless of which - * one performs the merge (see `synthesizeMergedEntity`). + * changes delta are unit-testable in isolation. Determinism is the whole point: + * both clients must arrive at the byte-identical merged delta regardless of + * which one performs the merge (see `synthesizeMergedChanges`). */ import { OpType } from '../core/operation.types'; @@ -157,34 +157,45 @@ export const isDisjointMergeEligible = (params: { }; /** - * Synthesizes the merged entity — the SINGLE source of truth both clients must - * converge on. + * Synthesizes the merged CHANGES DELTA — the union of both sides' changed + * fields, applied on top of each client's current entity by `updateOne` (a + * shallow MERGE, not a replace). This is the SINGLE source of truth both clients + * must converge on. * - * `currentEntity` is THIS client's current entity state, i.e. `base + localChanges`. - * We overlay the OTHER side's non-noise changed fields (guaranteed disjoint from - * local's, so nothing local is clobbered), then resolve every noise field either - * side changed via the deterministic `(timestamp, clientId)` tiebreak. + * IMPORTANT — why a delta and NOT a full-entity snapshot: the delta is derived + * purely from the two conflicting sides' ops, so both clients compute the + * byte-identical map regardless of the rest of their entity state. A full-entity + * snapshot (`{...currentEntity}`) would drag along fields NEITHER side touched; + * if such an untouched field momentarily differs between the two clients (an + * ordinary staggered-sync race — e.g. one client already applied a third + * device's edit the other has not), the two synthesized snapshots differ, tie + * under LWW at the identical `max(timestamp)`, and diverge PERMANENTLY. Carrying + * only the changed fields makes the merged ops identical and leaves every + * untouched field to its own op/LWW. * - * Convergence: client A starts from `base+A` and overlays B's fields; client B - * starts from `base+B` and overlays A's fields. For every non-noise field the - * value is the same (disjoint sets → each field owned by exactly one side); for - * every unchanged field the value is `base` on both; for every noise field both - * pick the same global tiebreak winner. Therefore `mergedA === mergedB`. + * Convergence: for every non-noise field the value is the same (disjoint sets → + * each field owned by exactly one side); for every noise field both pick the + * same global `(timestamp, clientId)` tiebreak winner. Therefore the delta is + * identical on both clients. */ -export const synthesizeMergedEntity = ( - currentEntity: Record, +export const synthesizeMergedChanges = ( localChanges: Record, remoteChanges: Record, localMeta: MergeSideMeta, remoteMeta: MergeSideMeta, ): Record => { - const merged: Record = { ...currentEntity }; + const changes: Record = {}; - // Overlay the remote side's real (non-noise) fields. Local's real fields are - // already present in `currentEntity` and are disjoint from these. + // Union of both sides' real (non-noise) fields. The two sets are guaranteed + // disjoint (isDisjointMergeEligible), so neither overwrites the other. + for (const [key, value] of Object.entries(localChanges)) { + if (!NOISE_FIELDS.has(key)) { + changes[key] = value; + } + } for (const [key, value] of Object.entries(remoteChanges)) { if (!NOISE_FIELDS.has(key)) { - merged[key] = value; + changes[key] = value; } } @@ -200,15 +211,15 @@ export const synthesizeMergedEntity = ( const localHas = field in localChanges; const remoteHas = field in remoteChanges; if (localHas && remoteHas) { - merged[field] = winner === 'local' ? localChanges[field] : remoteChanges[field]; + changes[field] = winner === 'local' ? localChanges[field] : remoteChanges[field]; } else if (localHas) { - merged[field] = localChanges[field]; + changes[field] = localChanges[field]; } else { - merged[field] = remoteChanges[field]; + changes[field] = remoteChanges[field]; } } - return merged; + return changes; }; /** diff --git a/src/app/op-log/sync/conflict-resolution.disjoint-merge.spec.ts b/src/app/op-log/sync/conflict-resolution.disjoint-merge.spec.ts index a87e46e813..7c9d04ef8a 100644 --- a/src/app/op-log/sync/conflict-resolution.disjoint-merge.spec.ts +++ b/src/app/op-log/sync/conflict-resolution.disjoint-merge.spec.ts @@ -18,7 +18,7 @@ import { VectorClockComparison, } from '../../core/util/vector-clock'; import { - synthesizeMergedEntity, + synthesizeMergedChanges, isDisjointMergeEligible, } from './conflict-disjoint-merge.util'; @@ -220,6 +220,90 @@ describe('ConflictResolutionService — SPAP-14 disjoint-field merge', () => { expect(result.localWinOpsCreated).toBe(1); }); + // ── (a3) SPAP-14 fix: partial-delta merged op, no un-conflicted ride-along ── + it('(a3) synthesizes a partial-delta merged op that excludes un-conflicted fields', async () => { + // Current entity carries a field NEITHER side touched (timeSpentOnDay). A + // full-entity snapshot would embed it and diverge across clients whose + // current state differs (staggered third-device sync); the delta must carry + // ONLY the two sides' changed fields. + mockStore.select.and.returnValue( + of({ + id: 'task-1', + title: 'Local title', + notes: 'base notes', + // An un-conflicted field present in current state (value shape is + // irrelevant — the delta must not read current state at all). + timeSpentOnDay: {}, + }), + ); + const localOp = op({ + id: 'local-1', + clientId: 'A', + vectorClock: { A: 1 }, + timestamp: 2000, + payload: { task: { id: 'task-1', changes: { title: 'Local title' } } }, + }); + const remoteOp = op({ + id: 'remote-1', + clientId: 'B', + vectorClock: { B: 1 }, + timestamp: 1000, + payload: { task: { id: 'task-1', changes: { notes: 'Remote notes' } } }, + }); + + await service.autoResolveConflictsLWW([conflictOf([localOp], [remoteOp])]); + + const merged = mergedOpArgs(); + expect(merged).toBeDefined(); + const payload = merged!.payload as Record; + expect(payload['title']).toBe('Local title'); + expect(payload['notes']).toBe('Remote notes'); + // The un-conflicted field must NOT ride along in the synthesized op. + expect('timeSpentOnDay' in payload).toBe(false); + }); + + // ── (a4) SPAP-14 fix: refuse merge when the entity has >1 conflict this batch + it('(a4) refuses disjoint-merge for an entity with multiple conflicts (falls back to LWW)', async () => { + mockStore.select.and.returnValue( + of({ id: 'task-1', title: 'base', notes: 'base', timeEstimate: 5 }), + ); + const localEst = op({ + id: 'local-est', + clientId: 'A', + vectorClock: { A: 1 }, + timestamp: 3000, + payload: { task: { id: 'task-1', changes: { timeEstimate: 9 } } }, + }); + const remoteTitle = op({ + id: 'remote-title', + clientId: 'B', + vectorClock: { B: 1 }, + timestamp: 3100, + payload: { task: { id: 'task-1', changes: { title: 'B title' } } }, + }); + const remoteNotes = op({ + id: 'remote-notes', + clientId: 'B', + vectorClock: { B: 2 }, + timestamp: 3200, + payload: { task: { id: 'task-1', changes: { notes: 'B notes' } } }, + }); + + // detectConflicts emits one conflict per remote op → two conflicts, same + // entity. Merging each independently would let the clock-dominating sibling + // silently drop the other's field, falsely journaled as "kept both". + await service.autoResolveConflictsLWW([ + conflictOf([localEst], [remoteTitle]), + conflictOf([localEst], [remoteNotes]), + ]); + + // No merged op was synthesized; both conflicts fell back to whole-entity LWW. + const entries = await journal.list('history'); + expect(entries.length).toBeGreaterThan(0); + expect(entries.every((e) => e.winner !== 'merged')).toBe(true); + expect((await journal.list('unreviewed')).length).toBeGreaterThan(0); + }); + // ── (b) title vs title → LWW unchanged ───────────────────────────────────── it('(b) leaves same-field (title-vs-title) conflicts to LWW (journal unreviewed)', async () => { mockStore.select.and.returnValue(of({ id: 'task-1', title: 'Local title' })); @@ -376,27 +460,22 @@ describe('ConflictResolutionService — SPAP-14 disjoint-field merge', () => { // ── (e) two-client convergence ───────────────────────────────────────────── describe('(e) two-client convergence', () => { - const base = { id: 'task-1', title: 'base', notes: 'base', modified: 100 }; // side1 authored by client A; side2 authored by client B. const side1Changes = { title: 'A-title', modified: 1500 }; const side2Changes = { notes: 'B-notes', modified: 1600 }; const side1Meta = { timestamp: 1500, clientId: 'A' }; const side2Meta = { timestamp: 1600, clientId: 'B' }; - const currentA = { ...base, ...side1Changes }; - const currentB = { ...base, ...side2Changes }; - it('both clients synthesize the byte-identical merged entity (either ordering)', () => { + it('both clients synthesize the byte-identical merged DELTA (either ordering)', () => { // Client A: local = side1, remote = side2. - const mergedA = synthesizeMergedEntity( - currentA, + const mergedA = synthesizeMergedChanges( side1Changes, side2Changes, side1Meta, side2Meta, ); // Client B: local = side2, remote = side1 (mirror). - const mergedB = synthesizeMergedEntity( - currentB, + const mergedB = synthesizeMergedChanges( side2Changes, side1Changes, side2Meta, @@ -404,15 +483,37 @@ describe('ConflictResolutionService — SPAP-14 disjoint-field merge', () => { ); expect(mergedA).toEqual(mergedB); - // Explicit expected entity: both real fields kept; noise → newer (side2). + // Explicit expected delta: both real fields kept; noise → newer (side2). + // No `id` and NO untouched fields — the delta carries ONLY changed fields. expect(mergedA).toEqual({ - id: 'task-1', title: 'A-title', notes: 'B-notes', modified: 1600, }); }); + it("the merged DELTA is independent of each client's divergent current state (SPAP-14 divergence fix)", () => { + // The two clients' current entities differ on an UN-conflicted field + // (timeSpentOnDay) — e.g. one already applied a third device's edit the + // other has not. A full-entity snapshot would drag that field along and + // diverge forever; the delta is derived only from the two sides' ops, so + // it is identical regardless. Neither delta may contain timeSpentOnDay. + const mergedA = synthesizeMergedChanges( + side1Changes, + side2Changes, + side1Meta, + side2Meta, + ); + const mergedB = synthesizeMergedChanges( + side2Changes, + side1Changes, + side2Meta, + side1Meta, + ); + expect(mergedA).toEqual(mergedB); + expect('timeSpentOnDay' in mergedA).toBe(false); + }); + it('both merged clocks dominate BOTH original ops', () => { const clockSide1 = { clientA: 2 }; const clockSide2 = { clientB: 2 }; diff --git a/src/app/op-log/sync/conflict-resolution.service.ts b/src/app/op-log/sync/conflict-resolution.service.ts index 755235a5d3..cd7982660e 100644 --- a/src/app/op-log/sync/conflict-resolution.service.ts +++ b/src/app/op-log/sync/conflict-resolution.service.ts @@ -70,7 +70,7 @@ import { buildConflictJournalEntry } from './conflict-journal-emission.util'; import { isDisjointMergeEligible, mergeChangedFields, - synthesizeMergedEntity, + synthesizeMergedChanges, } from './conflict-disjoint-merge.util'; /** @@ -748,13 +748,39 @@ export class ConflictResolutionService { toEntityKey(entityType as EntityType, entityId), }); + // SPAP-14 hardening: disjoint-merge is only safe for a SINGLE remote op per + // entity per batch. detectConflicts emits one conflict per remote op with no + // per-entity aggregation, so an entity with ≥2 concurrent remote ops (e.g. + // one device edited title then notes offline) would synthesize multiple + // merged ops for the same entity; their clocks dominate one another, so a + // dominated sibling can be superseded and its field silently dropped — + // falsely journaled as a successful "kept both" merge. Refuse the merge for + // any entity with >1 conflict this batch and fall back to whole-entity LWW + // (baseline behaviour, no false merge). Per-entity aggregation into one op is + // a possible future improvement; refusal is the safe floor. + const conflictCountByEntity = new Map(); + for (const plan of plans) { + const key = toEntityKey( + plan.conflict.entityType as EntityType, + plan.conflict.entityId, + ); + conflictCountByEntity.set(key, (conflictCountByEntity.get(key) ?? 0) + 1); + } + for (const plan of plans) { // SPAP-14: BEFORE the whole-entity LWW plan, try a disjoint-field merge — // when both sides edited the same entity but DIFFERENT real fields, keep - // BOTH instead of discarding the loser. Delete/archive and same-field - // (overlapping) conflicts are NOT eligible and fall through to the exact - // LWW + SPAP-13 path below, byte-unchanged. - const mergedOp = await this._tryCreateDisjointMergeOp(plan); + // BOTH instead of discarding the loser. Delete/archive, same-field + // (overlapping), and multi-remote-op-per-entity conflicts are NOT eligible + // and fall through to the exact LWW + SPAP-13 path below, byte-unchanged. + const entityKey = toEntityKey( + plan.conflict.entityType as EntityType, + plan.conflict.entityId, + ); + const mergedOp = + (conflictCountByEntity.get(entityKey) ?? 0) > 1 + ? undefined + : await this._tryCreateDisjointMergeOp(plan); if (mergedOp) { mergedResolutions.push({ conflict: plan.conflict, mergedOp }); await this._journalMergedResolution(plan); @@ -860,11 +886,14 @@ export class ConflictResolutionService { * path unchanged. * * The merged op is deterministic and CONVERGENT: both clients synthesize the - * byte-identical merged entity (union of both sides' disjoint real fields, with - * noise fields resolved by a deterministic `(timestamp, clientId)` tiebreak — - * see `synthesizeMergedEntity`) and a vector clock that DOMINATES both sides - * (via `mergeAndIncrementClocks`, mirroring `_createLocalWinUpdateOp`). The op - * uses the standard LWW Update action type and the max timestamp across both + * byte-identical merged CHANGES DELTA (union of both sides' disjoint real + * fields, with noise fields resolved by a deterministic `(timestamp, clientId)` + * tiebreak — see `synthesizeMergedChanges`) and a vector clock that DOMINATES + * both sides (via `mergeAndIncrementClocks`, mirroring `_createLocalWinUpdateOp`). + * The op carries a PARTIAL delta (not a full-entity snapshot), so untouched + * fields that momentarily differ between the two clients can't ride along and + * diverge; `lwwUpdateMetaReducer` applies it via `updateOne` (a shallow merge). + * It uses the standard LWW Update action type and the max timestamp across both * sides, so when two independently-synthesized merged ops meet they carry * identical payloads and resolve by ordinary LWW — never re-merging. * @@ -916,8 +945,14 @@ export class ConflictResolutionService { const localTs = Math.max(...conflict.localOps.map((op) => op.timestamp)); const remoteTs = Math.max(...conflict.remoteOps.map((op) => op.timestamp)); - const mergedEntity = synthesizeMergedEntity( - currentEntityState as Record, + // The merged op carries ONLY the union of both sides' changed fields (a + // partial delta), NOT a full-entity snapshot of `currentEntityState`. The + // delta is derived purely from the two sides' ops, so both clients compute + // the byte-identical map — a full snapshot would drag along untouched fields + // that can differ between clients under staggered sync and diverge forever. + // The lwwUpdateMetaReducer applies it via `updateOne` (a shallow merge), so + // fields outside the delta keep their own values. See `synthesizeMergedChanges`. + const mergedChanges = synthesizeMergedChanges( localChanges, remoteChanges, { timestamp: localTs, clientId: conflict.localOps[0]?.clientId ?? clientId }, @@ -939,7 +974,7 @@ export class ConflictResolutionService { return this.createLWWUpdateOp( conflict.entityType, conflict.entityId, - mergedEntity, + mergedChanges, clientId, newClock, mergedTimestamp, diff --git a/src/app/op-log/sync/sync-conflict-ui.service.spec.ts b/src/app/op-log/sync/sync-conflict-ui.service.spec.ts index b83b6d424c..fb5b22084a 100644 --- a/src/app/op-log/sync/sync-conflict-ui.service.spec.ts +++ b/src/app/op-log/sync/sync-conflict-ui.service.spec.ts @@ -133,6 +133,123 @@ describe('SyncConflictUiService', () => { expect((await journal.getEntry('e1'))?.status).toBe('unreviewed'); }); + it('getStaleState() flags stale when a LOSER-ONLY field was edited after resolution', async () => { + // Overlapping-field LWW conflict: loser changed title+notes, winner changed + // only title (remote won). `notes` is a loser-only field — winnerChangesFor + // cannot see it, so the guard was blind to a post-resolution notes edit and + // flip would silently overwrite it. getStaleState must now detect it. + const entry = makeEntry({ + id: 'lo1', + fieldDiffs: [ + { + field: 'title', + localVal: 'Local title', + remoteVal: 'Remote title', + localChanged: true, + remoteChanged: true, + pickedSide: 'remote', + }, + { + field: 'notes', + localVal: 'Loser notes', + remoteVal: undefined, + localChanged: true, + remoteChanged: false, + pickedSide: 'remote', + }, + ], + }); + // Winner field (title) unchanged, but notes was edited to a value that is + // neither the kept value nor what flip would write. + store.overrideSelector(selectTaskById, { + id: 'task-1', + title: 'Remote title', + notes: 'USER-EDIT', + } as Task); + store.refreshState(); + + const stale = await service.getStaleState(entry); + expect(stale.isStale).toBe(true); + }); + + it('flip() shows the stale confirm for a post-resolution loser-only edit (previously silent)', async () => { + const entry = makeEntry({ + id: 'lo2', + fieldDiffs: [ + { + field: 'title', + localVal: 'Local title', + remoteVal: 'Remote title', + localChanged: true, + remoteChanged: true, + pickedSide: 'remote', + }, + { + field: 'notes', + localVal: 'Loser notes', + remoteVal: undefined, + localChanged: true, + remoteChanged: false, + pickedSide: 'remote', + }, + ], + }); + await journal.record(entry); + store.overrideSelector(selectTaskById, { + id: 'task-1', + title: 'Remote title', + notes: 'USER-EDIT', + } as Task); + store.refreshState(); + setDialogResult(false); // user cancels → flip must NOT silently overwrite + + const result = await service.flip(entry); + + expect(matDialog.open).toHaveBeenCalled(); + expect(result).toBe('cancelled'); + expect(dispatchSpy).not.toHaveBeenCalled(); + }); + + it('flipAllToSide() skips stale entries (never silently overwrites in bulk)', async () => { + // Remote-won entry whose loser-only notes field diverged after resolution. + // Bulk flip-to-local must SKIP it (leave unreviewed), not silently overwrite + // the newer notes — the bulk path shows no per-entry confirm. + const entry = makeEntry({ + id: 'bulk1', + winner: 'remote', + fieldDiffs: [ + { + field: 'title', + localVal: 'Local title', + remoteVal: 'Remote title', + localChanged: true, + remoteChanged: true, + pickedSide: 'remote', + }, + { + field: 'notes', + localVal: 'Loser notes', + remoteVal: undefined, + localChanged: true, + remoteChanged: false, + pickedSide: 'remote', + }, + ], + }); + await journal.record(entry); + store.overrideSelector(selectTaskById, { + id: 'task-1', + title: 'Remote title', + notes: 'USER-EDIT', + } as Task); + store.refreshState(); + + await service.flipAllToSide([entry], 'local'); + + expect(dispatchSpy).not.toHaveBeenCalled(); + expect((await journal.getEntry('bulk1'))?.status).toBe('unreviewed'); + }); + it('flip() suppresses short-syntax parsing on the re-applied title', async () => { // The canonical flip is a rename conflict → changes is exactly { title }, // which is precisely the shape shortSyntax$ re-parses in replace mode. A diff --git a/src/app/op-log/sync/sync-conflict-ui.service.ts b/src/app/op-log/sync/sync-conflict-ui.service.ts index 7f246bf42a..f723d238aa 100644 --- a/src/app/op-log/sync/sync-conflict-ui.service.ts +++ b/src/app/op-log/sync/sync-conflict-ui.service.ts @@ -193,8 +193,12 @@ export class SyncConflictUiService { /** * Bulk FLIP toward one side: applies to rows where that side LOST (so flipping * makes it win). `side='local'` targets remote-won entries; `side='remote'` - * targets local-won entries. Merged entries are never touched. The per-entry - * stale confirm is skipped for the bulk path (explicit power action). + * targets local-won entries. Merged entries are never touched. + * + * Stale entries are SKIPPED (left unreviewed), never silently flipped: a bulk + * action must not overwrite an edit made after the conflict resolved. Because + * the bulk path shows no per-entry dialog, we cannot ask — so we refuse the + * risky ones and leave them for per-entry flip (which surfaces the confirm). */ async flipAllToSide( entries: readonly ConflictJournalEntry[], @@ -207,6 +211,10 @@ export class SyncConflictUiService { entry.winner === loserIsSide && this.canFlip(entry) ) { + const { isStale } = await this.getStaleState(entry); + if (isStale) { + continue; + } await this.flip(entry, { skipStaleConfirm: true }); } } @@ -222,11 +230,25 @@ export class SyncConflictUiService { if (!current) { return { isStale: false, current: undefined }; } + // A field the WINNER changed diverged from its kept value → entity edited + // since resolution. const winnerVals = winnerChangesFor(entry); - const isStale = Object.keys(winnerVals).some( + const winnerStale = Object.keys(winnerVals).some( (field) => !this._valueEquals(current[field], winnerVals[field]), ); - return { isStale, current }; + // Loser-only fields: FLIP WILL write these, but the winner never changed + // them, so `winnerChangesFor` cannot see them — leaving getStaleState blind + // to exactly the fields flip overwrites. If the current value is not already + // the value flip would write, a post-resolution edit lives there and flip + // would silently destroy it, so force the stale confirm. (Overlaps with + // winner-changed fields, e.g. a both-changed `title`, are already covered + // above; only loser-ONLY fields need this extra pass.) + const flipVals = loserChangesFor(entry); + const loserOnlyStale = Object.keys(flipVals).some( + (field) => + !(field in winnerVals) && !this._valueEquals(current[field], flipVals[field]), + ); + return { isStale: winnerStale || loserOnlyStale, current }; } private _buildUpdateAction(