* feat(sync): conflict-journal foundation (observe-only)
Add a device-local IndexedDB conflict journal that records every sync-
conflict auto-resolution so the discarded ("losing") side is preserved
and reviewable later. Foundation subtask for the conflict-review epic;
no UI here, verifiable purely by unit tests.
- New SUP_CONFLICT_JOURNAL IndexedDB store (own DB; never touches the
op-log SUP_OPS schema) + ConflictJournalService with record/query API
(unreviewedCount$, list, markKept, markFlipped, getEntry) and 14-day /
200-entry retention pruning, wired to run on app start via APP_INITIALIZER.
- Pure classifier buildConflictJournalEntry maps each resolution to the
agreed taxonomy (newer/tie/delete-wins/noise/clock-corruption-
suspected; disjoint-merge reserved for the next subtask), capturing the
loser's field values verbatim. NOISE_FIELDS is limited to metadata
timestamps (modified/lastModified/created): the list-ordering arrays
carry membership as well as order, so an overlap on them is surfaced as
a reviewable conflict rather than silently classified as noise.
- Emission is strictly observe-only: journaling runs after the LWW plan is
built, wrapped in try/catch (record() swallows its own errors); clock-
corruption attribution uses a WeakSet side-channel tagged at detection.
The existing conflict-resolution suite stays 138/138 green, proving LWW
picks are unchanged. One-sided / sequential / EQUAL updates never become
conflicts and produce zero journal entries.
SPAP-13
* feat(sync): disjoint-field auto-merge for concurrent edits
When two clients concurrently edit the same entity but different fields
(A changes title, B changes notes), whole-entity LWW previously discarded
one side. Keep both when the non-noise changed-field sets are disjoint.
In _resolveConflictsWithLWW, before LWW picks a winner, each CONCURRENT
conflict is tested for merge eligibility (neither side deleting/archiving;
both changed >=1 real field; non-noise field sets disjoint). If eligible,
synthesize a single merged UPDATE op — the current entity overlaid with
the other side's non-noise fields, noise fields resolved by the greater
(timestamp, clientId) — carrying a vector clock that dominates both sides,
so it propagates through normal sync. The resolution is winner 'merged'
and is journaled reason 'disjoint-merge' / status 'info' (not counted as
unreviewed). Any overlap on a non-noise field, or a delete/archive on
either side, falls through to the existing LWW path unchanged.
Convergence: both clients compute the byte-identical merged entity
(disjoint real fields each owned by one side; noise resolved by the same
global tiebreak) with clocks dominating both originals, so the two
independently-synthesized merged ops carry identical full-entity payloads
and re-resolve to the same state via ordinary LWW without re-merging.
No sync-core/protocol change. Existing conflict-resolution suite stays
138/138; SPAP-13 journal specs stay green.
SPAP-14
* feat(sync): sync conflicts review UI (banner, badge, page, flip)
Builds the conflict-review UI on top of the device-local conflict journal.
- Post-sync summary banner "N conflicts auto-resolved (X remote, Y local
won)" with REVIEW / DISMISS, replacing the bare LWW_CONFLICTS_AUTO_
RESOLVED snack at its emission sites; a persistent badge on the sync
icon bound to unreviewedCount$ (survives banner dismiss).
- New /sync-conflicts page: Unreviewed | History tabs, rows grouped by
entity type with winner + reason chips, expandable per-field diff
(LOCAL vs REMOTE, device name + wall-clock time, winner marked),
per-row KEEP / FLIP and bulk KEEP ALL / FLIP ALL → LOCAL / → REMOTE.
History renders merged auto-merges as per-field chips.
- Flip dispatches a normal entity update with the loser's journaled field
values (syncs like a user edit, no history rewind) and marks the entry
flipped; a stale-flip confirm appears (with the current value shown)
when the entity changed since resolution.
- i18n under F.SYNC.CONFLICT_REVIEW.
Delete-restore and archived-entity flip are surfaced as unsupported for
now (an update op can't recreate an absent entity) — follow-up.
SPAP-15
* fix(sync): count disjoint-merge ops in localWinOpsCreated
autoResolveConflictsLWW returned only newLocalWinOps.length, excluding the
synthesized merged ops appended in STEP 3b. A sync whose only conflicts were
disjoint-field merges returned 0, so the caller (immediate-upload.service.ts)
skipped the immediate re-upload and reported IN_SYNC while the merged op sat
unsynced until a later cycle.
Count mergedResolutions.length too — each merge appends exactly one pending
local op. Mirrors the rejection-handler path (operation-log-sync.service.ts).
Add a regression spec asserting a merge-only conflict returns
localWinOpsCreated: 1 (fails on the old return, passes now).
Also harden the _corruptionSuspectedConflicts WeakSet doc against a future
refactor that clones EntityConflict between detect and resolve.
Addresses review feedback on PR #8874.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sync): address second-pass review (delete-lost, archive guard test, polish)
Second-pass review follow-ups (johannesjo). The MEDIUM localWinOpsCreated
item was already fixed in 23e9a56.
Important:
- delete-lost classification: when a delete LOSES to a concurrent newer edit,
the loser side is a pure Delete op (no field changes), so it fell through to
`noise`/`info` and never surfaced. Add a `delete-lost` reason classified as
`unreviewed` (inverse of `delete-wins`), checked before the noise fallthrough.
+ regression spec.
- archive-vs-disjoint-edit test (d2): an archive is an UPDATE, so eligibility
does NOT reject it — only the `_isArchivePlan` guard does. New test forces
eligibility TRUE and asserts no merged op is synthesized, so a guard
regression now fails a test (previously nothing covered it).
Minor:
- pruneOnStart: wrap in try/catch (log + return 0, matching record()'s
observe-only contract); reset the poisoned `_initPromise` in `_ensureDb` on
open failure so a transient IndexedDB error can't wedge the service.
- sync-conflicts-page.scss: use `--color-success` token + color-mix tint
instead of hardcoded #4caf50 / rgba(76,175,80,…); drop the dead
`--color-warning` fallback (#e6a817 didn't match the real #ff9800 token).
- main-header badge: matBadgeColor warn -> accent (a resolved count is not an
error); move the count announcement to `matBadgeDescription` and give the
button a stable sync-action aria-label (it was null at count 0, leaving the
icon-only button unnamed).
- remove dead i18n key LWW_CONFLICTS_AUTO_RESOLVED (t.const.ts + en.json).
- add direct unit spec for noiseTiebreakSide (incl. equal-timestamp clientId
determinism) and buildMergedFieldDiffs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sync): address third-pass review (flip capability, field presence, opaque ops, profile isolation)
Blocking #3 — delete-lost/delete-wins offered a false-success Flip:
canFlip() is now reason/field-aware (refuses delete-lost/delete-wins,
empty loser changes, and relationship-bearing fields whose bare adapter
update would bypass meta-reducer invariants); flip() guards on it and
only marks an entry flipped when an op was actually dispatched.
Blocking #2 — field absent vs side-set-undefined: fieldDiffs now record
per-side presence (localChanged/remoteChanged); loserChangesFor/
winnerChangesFor only emit fields the side actually changed, so Flip no
longer clears winner-only fields and the stale guard no longer compares
undefined. Legacy entries without flags fall back to value-presence
(exact, since op payloads are pure JSON).
Blocking #1 — non-adapter action payloads: per-op extraction now falls
back to capture-time entityChanges (TIME_TRACKING/syncTimeSpent), and
ops with no readable delta (convertToSubTask & co) are "opaque": never
classified noise/info, preserved verbatim as kind:'action' diffs for
review, excluded from flip/stale computations, and never disjoint-merge
eligible (merging would drop the mutation and diverge the two clients).
Profile isolation — switchProfile clears the conflict journal
(ConflictJournalService.clearAll) so a new profile cannot see the
previous profile's entity data or Flip against the wrong dataset.
Also: replace the as-any selector cast with a typed union-member
narrowing, and add the required docs (sync-and-op-log/
conflict-journal-and-review.md incl. the atomicity/no-re-merge
contract, wiki 3.06 + 4.23).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sync): address fourth-pass review (flip short-syntax, selector throws, restore isolation)
HIGH — a title-only flip matched shortSyntax$'s exact trigger shape, so
#tag/+project/@schedule tokens in the discarded title re-parsed into
cross-entity mutations: the TASK flip now dispatches with
isIgnoreShortSyntax: true (journaled literal value, not user input).
MEDIUM — selectTagById/selectNoteById THROW on a missing entity, so
expanding (or flipping) a TAG/NOTE row whose entity is gone rejected
unhandled before the !current guard: _readCurrentEntity now catches and
returns undefined, and getStaleState/flip degrade gracefully.
MEDIUM — the journal survived backup restores: clearAll() moved to the
BackupService.importCompleteBackup chokepoint, covering every full
dataset replacement (profile switch, JSON import, local-backup restore,
SuperSync restore) instead of only the profile switch.
LOW — ISSUE_PROVIDER's factory selectById is now special-cased (was
rendering the inner selector function as the "current" entity);
schedule/reminder fields (dueDay/dueWithTime/deadline*/reminderId) join
the flip blocklist (renamed FLIP_UNSAFE_FIELDS) since their invariants
live in dedicated flows; loser/winnerChangesFor early-return for merged
entries (their tiebreak diffs carry pickedSide, so the per-diff check
alone did not exclude them).
Docs updated accordingly (dev doc + wiki 3.06/4.23).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sync): refuse flip for remindAt/deadlineRemindAt (reminder-lifecycle)
FLIP_UNSAFE_FIELDS is a deny-list: any Task field not listed is flippable
via a bare updateTask. reminderId was covered, but the sibling reminder-
lifecycle timestamps remindAt and deadlineRemindAt were not — a conflict on
either alone (e.g. concurrent deadline-reminder-lead edits) would pass
canFlip and, when flipped, write the field without scheduling/cancelling the
actual reminder in ReminderService, leaving a dangling/missing notification.
Add both to FLIP_UNSAFE_FIELDS, document the deny-list drift risk, and pin
the reminder/schedule members with per-field canFlip=false spec cases.
* fix(sync): make disjoint-merge convergent + close flip stale-guard blind spot
Two confirmed cross-device data defects found in the SPAP-14 whole-PR review:
1. Full-entity merged op diverges (CRITICAL). The merged op was a full-entity
snapshot of each client's CURRENT state, so any field NEITHER conflicting
side touched rode along. Under ordinary staggered sync (one client already
applied a third device's edit to that field, the other not yet), the two
clients synthesize different snapshots that tie under LWW at the identical
max(timestamp) and diverge PERMANENTLY. Fix: synthesize a PARTIAL delta
(union of the two sides' changed fields only), derived purely from the ops
so both clients compute the byte-identical map; lwwUpdateMetaReducer applies
it via updateOne (shallow merge), leaving untouched fields alone.
synthesizeMergedEntity -> synthesizeMergedChanges.
Also: detectConflicts emits one conflict per remote op with no per-entity
aggregation, so an entity with >=2 concurrent remote ops synthesized multiple
merged ops whose clocks dominate one another -> a dominated sibling's field
is silently dropped, falsely journaled as 'kept both'. Fix: refuse merge for
any entity with >1 conflict this batch and fall back to whole-entity LWW.
2. Flip stale-guard blind to loser-only fields (HIGH). getStaleState only
compared WINNER-changed fields, but flip writes LOSER-changed fields. A
post-resolution edit to a loser-only field was undetected and silently
overwritten. Fix: also flag stale when a loser-only field flip will write
diverged from the value flip would write; bulk flipAllToSide now SKIPS stale
entries instead of silently applying them.
Adds regression specs for the un-conflicted-field ride-along, multi-remote-op
refusal, and loser-only stale detection (per-entry + bulk). Full conflict suite
green (disjoint-merge 12, ui 24, conflict-resolution 138, journal 20, hook 6,
util 14, review-util 13, superseded 42, banner 3, page 6).
* fix(sync): scope flip stale-guard loser-only check to remote-won entries
Re-review of efe66d7 found the new loser-only stale check false-positives on
LOCAL-won conflicts: there the loser is the REMOTE side, whose value was never
applied (current holds the un-recorded base), so current != flipVal is the
NORMAL post-resolution state, not an edit. That made flipAllToSide silently skip
legitimate local-won entries and single flip nag with a spurious confirm.
Scope loserOnlyStale to winner==='remote' (the only case where the loser's
optimistically-applied local value persists, giving a valid unedited baseline).
Remote-won silent-loss detection (the originally-reported defect) is unchanged.
Loser-only fields on LOCAL-won entries remain undetectable without a journaled
post-resolution baseline — documented as a follow-up. +2 regression specs
(local-won no-false-positive: getStaleState + bulk flip).
* fix(sync): restrict disjoint-merge to types with a RECREATE_FALLBACK
The merged op is a partial delta. If it wins over a concurrent DELETE on a
passive-observer client (one that already applied that delete), it reaches
lwwUpdateMetaReducer's addOne recreate branch WITHOUT passing through the
full-entity reconstruction in _convertToLWWUpdatesIfNeeded (that runs only for
conflict winners, not non-conflicting remote ops). For a type without a
RECREATE_FALLBACK (NOTE/METRIC/TASK_REPEAT_CFG/ISSUE_PROVIDER) the bare partial
addOne yields a Typia-invalid entity -> 'Repair failed' dead-end; the parent's
full-snapshot merged op recreated validly, so the partial delta is a regression
there.
Refuse disjoint-merge for fallback-less types (fall back to whole-entity LWW,
whose local-win op carries a full snapshot). Residual: fallback types can still
recreate with DEFAULT_* backfill diverging in that rare 3-device race — the same
bounded limitation already documented in recreate-fallback.const.ts. +regression
spec (NOTE disjoint conflict -> LWW, not merged).
* fix(sync): harden conflict-journal failure and lifecycle paths
Three hardening improvements from the final review pass:
1. Journal disjoint merges only AFTER the merged op is durably appended
(STEP 3b), not at plan time. A 'merged' entry claims both sides were
kept, which is only true once the op is persisted — an append failure
could previously leave a phantom 'kept both' journal entry. +regression
spec (a6): append failure -> no merged journal entry.
2. Extend the never-throw contract to ALL ConflictJournalService methods.
list() is awaited (via maybeShowSummaryBanner) inside
autoResolveConflictsLWW's notification step — i.e. after ops were
already applied — so a transient IndexedDB failure there failed an
otherwise-completed sync. list -> [], getEntry -> undefined,
markKept/markFlipped swallowed (entry stays unreviewed, user retries).
+spec.
3. Recover from abnormal IndexedDB closure: idb's terminated hook now
drops the memoized _db/_initPromise handles so the next call reopens
instead of failing on a dead connection for the rest of the session
(deferred fourth-pass item). +spec.
Plus: reciprocal note in recreate-fallback.const.ts that membership also
opts a type into disjoint-merge, and doc updates for the new contracts.
* test(sync): pin the terminated-hook wiring in the journal recovery spec
The recovery spec called _resetDbHandles() directly, so removing the
terminated: callback from openDB (functionally reverting the force-close
recovery) kept all tests green. Fire the real 'close' event idb listens
for instead (duck-typed FakeEvent for fake-indexeddb) and assert the
handles were cleared. Mutation-verified: deleting the terminated line
now fails this spec.
* fix(sync): clear conflict journal inside the op-log lock on import
Author-review finding: importCompleteBackup released the OPERATION_LOG
lock before clearAll(), leaving a narrow cross-tab window where a
concurrent conflict resolution's fresh post-import journal entry gets
wiped. Clearing inside the lock serializes the clear strictly before any
post-import journaling. (_resetAllLastServerSeqs is journal-independent
local bookkeeping and keeps its persist-first ordering.)
Also documents the merged-op composition residual from the same review:
a merged partial op is not closed under later whole-op LWW composition
in the no-pending-local concurrent-apply path — verified pre-existing
(branch and behavior identical at the pre-SPAP-14 merge-base with plain
user ops), so recorded as a class-level op-log residual rather than
patched here.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
962c5bbeb1