The client pruned its durable vector clock with uploader-only protection,
so once 21+ client ids accumulated after a full-state import, the import
author's low-counter entry was evicted. Every subsequent local op then
permanently failed the sync-import filter's knows-import-counter rescue
and was dropped as CONCURRENT on every peer — the client-side ceiling of
the server fix in #9089.
- client limitVectorClockSize wrapper now takes a preserve list, matching
the shared implementation
- calculateRemoteClockMerge (remote merge + reducer checkpoint) preserves
the latest full-state author; an in-batch full-state op supersedes the
stored one, and the checkpoint resolves the author inside its
transaction so a just-rejected import cannot name the protected author
- snapshot save, compaction, hydrator restore, and the sync-hydration
file-snapshot bootstrap protect the author on their durable-clock paths
- docs: add the missing calculateRemoteClockMerge prune site, correct the
stale RepairOperationService rows (repair ships the full clock), and
reword the sync-core pruning comment to the real invariant
SuperSync E2EE (AES-256-GCM) authenticates only op.payload; envelope
fields including op.entityIds travel as plaintext. The two [TASK] LWW
Update meta-reducers trusted meta.entityIds (copied from that envelope)
to decide which tasks to relocate, so a compromised sync server could
inject victim ids and relocate arbitrary tasks. GHSA-8pxh-mgc7-gp3g.
Carry the deterministic move footprint (#9001) inside the encrypted,
authenticated payload as LwwUpdatePayload.projectMoveFootprint:
- createLWWUpdateOp writes the footprint to both op.entityIds (the
server still needs plaintext ids for its indexed conflict detection
and cannot read the ciphertext) and payload.projectMoveFootprint.
- convertOpToAction surfaces the authenticated copy onto
meta.moveFootprint (mirrors the recreatesEntityAfterDelete pattern).
- Both LWW-TASK reducers (project repair + section membership) read
meta.moveFootprint via a shared parseMoveFootprint helper and no
longer trust meta.entityIds. Legacy ops without the field fall back
to receiving-state repair.
- getTaskProjectMoveEntityIds (footprint re-derivation during conflict
resolution, fed remote ops) reads the authenticated payload instead
of op.entityIds, so a tampered remote envelope cannot be laundered
into a freshly-authenticated merged op. Covers the disjoint-merge,
local-win, and superseded-op callers via one choke point.
Additive optional field: forward/backward compatible, no crypto or
envelope-version change. The DELETE/archive (bulk-archive-filter) and
conflict-detection (get-op-entity-ids) envelope reads are suppression-
only, not relocation, and remain for the durable AAD hardening.
* fix(sync-core): break whole-entity LWW timestamp ties by clientId
An exact-millisecond timestamp tie on the same field of one entity fell
to remote-wins with no tiebreak. Because local/remote are swapped between
devices, each kept the other's value and diverged permanently.
Fall back to a deterministic larger-clientId compare on the tie (mirrors
noiseTiebreakSide), routing a local tie-win through the existing
localWinOperationKind: 'update' path so the compensating op preserves the
local value and dominates the loser's clock.
* refactor(sync-core): tidy LWW tiebreak helper + strengthen tie tests
Multi-review follow-up (no behavior change):
- drop unreachable `?? ops[0]` fallback and the unused generic in
winningClientId; correct the doc comment's "mirrors" overstatement.
- rewrite the mislabeled service-level tie test that never exercised the
tiebreak and add the local-clientId-larger direction (compensating op).
* test(sync): cover LWW client-ID tie-win over a concurrent remote DELETE
The tiebreak reaches _createLocalWinUpdateOp's delete-recreation branch via
a tie (not just a newer timestamp); assert the entity is still recreated.
deleteProject cascade-deletes a project's tasks, notes, sections, repeat
config, and archive data in one reducer pass. When that op lost an LWW
conflict to a concurrent project edit, only the PROJECT entity was
reversed: every client resurrected an empty project and the winning
client's status-blind hydration replay cascaded its tasks away after a
restart (live state != post-restart replay).
Rather than recreate every cascaded entity (payload scales with project
size and cannot restore every side effect safely), give schema-v4
deleteProject operations explicit delete-wins precedence:
- new deleteProject actions carry a shared PROJECT_DELETE_WINS_MARKER; the
shared LWW planner accepts a host-supplied delete-wins classifier. A
marked remote delete is applied regardless of timestamps; a marked local
delete is replaced with one op whose vector clock dominates both sides.
- historical unmarked (schema-v3) deletions keep timestamp-based LWW; the
absence of the marker (never added by the no-op v3->v4 migration) is the
real discriminator, and a schema v3->v4 barrier (mirroring v2->v3) makes
older clients block on the newer-schema gate instead of mis-resolving.
Delete-wins plans reuse the archive-win resolution pipeline, so they
inherit its atomic persistence and losing-op rejection, and disjoint
merge leaves them untouched (the delete must win the whole entity).
Hardening from multi-agent review:
- union allTaskIds/noteIds across multiple concurrent marked deletes for
the same project, so a single replacement cannot leave orphan tasks on
clients that only receive it (the task reducer removes by allTaskIds).
- gate the classifier on the AUTHENTICATED payload projectId matching the
plaintext entityId, so a tampered/replayed delete retargeted onto a live
entity cannot silently drop a concurrent edit.
- guard a null/undefined delete payload in the classifier instead of
throwing and wedging the conflict pass.
- pin the server's legacy-misc conflict alias to the fixed v1->v2 split
boundary, not CURRENT_SCHEMA_VERSION, so this bump does not fabricate
false GLOBAL_CONFIG:misc/tasks conflicts during rollout.
- bind the marker with a shared const (compiler-checked on producer and
consumer) and rename _isArchivePlan -> _isWholeEntityWinPlan.
Documents the policy as ARCHITECTURE-DECISIONS.md #7.
Addresses #8997.
* chore: add project-scoped Angular MCP
* chore: update npm for release-age policy
* fix(sync): preserve LWW outcomes across clients
Distinguish replacement snapshots from partial merge operations, protect device-local sync configuration, recreate winning deletes, and resolve every conflicted entity in bulk operations.
Fixes#8956
* fix(sync): harden mixed LWW conflict replay
Preserve unaffected remote and local bulk intents, keep device-local sync settings during replacement replay, and gate replacement semantics behind schema v3.
* fix(sync): recover subtask subtree and harden LWW replay follow-ups
Follow-up hardening for #8956 after multi-agent review:
- Recreate a locally-winning parent's subtasks when a remote bulk delete
is a mixed winner. The full remote delete is applied (cascade-deleting
the parent's subtasks via handleDeleteTasks) but only the parent had a
compensation op, so the subtree was silently lost across devices.
- extractUpdateChanges: scan array-valued payload props instead of guessing
`${payloadKey}s`, so irregular bulk keys (e.g. taskUpdates) no longer
return {} and drop a remote winner's changes.
- Degrade gracefully instead of throwing when a remote update wins over a
local delete with no reconstructable base entity, matching the
single-entity path (a permanent sync wedge is worse than the bounded
divergence it already accepts).
- Remove dead code (unused `deleting` set + zero-caller wrapper), use the
Set-based scoped-bulk-delete filter, type meta via LwwUpdateMode, and
restore the withLocalOnlySyncSettings rationale comment.
Adds regression tests for the subtree-recovery and irregular-bulk-key paths.
* fix(sync): preserve conflict outcomes during replay
Persist replacement LWW operations and bulk-delete snapshots so reconstructed state matches the result applied live. Bump the op-log DB version to prevent older clients from opening the incompatible schema.
* fix(sync): persist conflict outcomes atomically
Write remote losers, local compensations, and final remote winners in one IndexedDB transaction so crashes cannot expose a partial replay order. Add real-store coverage for live/replay equivalence and transaction rollback.
* fix(sync): preserve multi-entity conflict recovery
* fix(sync): close review gaps in multi-entity conflict recovery
- recreate a winning parent's subtasks when a remote DELETE loses
outright (single-entity or all-local-win bulk), so clients that
applied the delete and status-blind hydration replay converge
- apply the combined resolution batch in durable seq order so a
pending row reused from a prior failed attempt replays identically
live and after a crash
- restamp converted remote updates carrying the v3 replacement
envelope to the current schema version
- strip the virtual TODAY tag from LWW task payloads and shallow-merge
patch-mode singleton payloads instead of replacing feature state
- pin the server snapshot fast-path spec to CURRENT_SCHEMA_VERSION
(fixes the CI failure from the v2-to-v3 bump)
* test(sync): pin outright-losing delete convergence across clients
Three-way real-reducer/real-store convergence for the pure-loser path
(live == restart replay == originating client), covering both the
same-batch recreate exemption and the cross-batch recreate path.
Verified to fail against the pre-fix service.
* fix(sync): reject disjoint merges for multi-entity ops
Scope conflict field extraction and journal titles to the actual entity. Fall back to whole-op LWW whenever either side is multi-entity so sibling updates are never falsely reported as preserved.\n\nFixes #8944.
* fix(sync): harden multi-entity conflict resolution
A committed reducer must never be durable without its vector clock, and
a merged clock must never be durable without its reducer checkpoint —
either mismatch lets the next local operation be causally older than
state already visible in NgRx after a crash.
- markArchivePending + separate mergeRemoteOpClocks are replaced by one
markReducersCommittedAndMergeClocks transaction (ops + vector_clock);
the clock math is extracted into a pure calculateRemoteClockMerge so
the standalone merge path keeps identical full-state-reset semantics.
- The sync-core RemoteOperationApplyStorePort gains an
onRemoteClocksDurable hook so deferred local actions drain exactly
when clocks are durable, not merely when ops were applied; a
checkpoint rejection can no longer mask the primary apply error.
- Conflict resolution's local-wins path writes remote losers and rebased
local compensations in one appendMixedSourceBatchSkipDuplicates
transaction, so synthetic ops cannot reuse or regress the client
counter.
- DB_VERSION 7 -> 8 as a deliberate downgrade barrier: released v7
readers only understand 'failed' and would silently overlook
outstanding 'archive_pending' work.
op-log suite and sync-core suite cover the checkpoint rollback, atomic
clock merge, mixed-source ordering and drain failure matrix.
* 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>
- USE_REMOTE: fix guaranteed self-deadlock — flushPendingWrites() was
called while already holding the non-reentrant sp_op_log lock its own
Phase 2 re-acquires, so every "Use Server Data" hung 30s and aborted.
New OperationWriteFlushService.flushThenRunExclusive owns the bounded
flush->lock->recheck barrier; ServerMigrationService reuses it, which
also bounds its previously unbounded snapshot-cutoff retry loop.
- USE_REMOTE: make the rebuild crash-resumable. A raw_rebuild_incomplete
META marker is set atomically with the baseline replacement and cleared
after the replay commits; the next sync redoes the raw rebuild instead
of the normal download (which excludes own ops server-side and would
silently lose the un-replayed suffix). The resume keeps the first
attempt's pre-replace backup instead of overwriting the single slot
with the partial baseline.
- Deferred actions: serialize overlapping drains (two concurrent drains
could mint two ops for one user intent), stop the drain at the first
transient failure instead of persisting successors out of order (the
older edit would win LWW everywhere via clock inversion), abandon
permanently invalid actions after one attempt (typed
PermanentDeferredWriteError) instead of retrying + snacking on every
sync forever, and latch the buffer-size warnings to once per stuck
window (previously a blocking dev dialog per action past 100); drop
the no-op 5000 "hard cap" tier.
- writeOperation: post-append bookkeeping failures no longer propagate
into the deferred retry loop, which re-appended the same action under
a fresh op id (double-apply of additive payloads).
- remote-apply: full-state cleanup retains every full-state op of the
current batch, so an archive_pending import can no longer be deleted
before its archive retry (startup replay would lose a change already
visible at runtime).
- Hydration: sanitize a malformed stored schemaVersion per-op instead of
failing the whole boot into recovery; strict parsing (floor now 1,
matching the server contract) stays on the receive/upload paths.
- Server: request fingerprints are computed lazily — only when a dedup
entry exists, and otherwise after the rate-limit/pre-quota gates but
before uploadOps mutates the parsed ops. Fixes the pre-gate hashing of
full payloads (authenticated DoS-cost regression) and repairs three
sync-fixes retry tests to model true identical-body retries.
- Delete dead code: failed-op-ids util (+spec) and MAX_VERSION_SKIP (the
removed forward-compat band); fix the stale clock-merge parity comment.
sync-core 210/210, shared-schema 50/50, super-sync-server 825/825, and
all touched Angular specs pass.
Make remote replay and authoritative state replacement crash-resumable and atomic. Preserve pending changes until piggyback processing commits, and stop cleanly on incompatible operations. Validate sync identities and schemas while strengthening request deduplication and privacy-safe diagnostics.
Address findings from a full sync-system review (blockers + high/medium):
- USE_REMOTE ("Use Server Data") is now a true download-first rebuild:
fetches the complete server history (incl. own/already-known ops via a
raw-download mode) and validates it before any local mutation; aborts
untouched on download failure, empty remote, or newer-schema ops.
- Widen the incoming full-state conflict gate to treat all user work as
meaningful (not just TASK/PROJECT/TAG/NOTE CUD); fix the piggyback race
by judging against a pre-upload pending snapshot.
- Stop advancing the server cursor past ops blocked by schema/migration
failures so they are retried after an app update; block any op from a
newer schema version (no forward-compat band).
- Retry of failed remote ops runs archive side effects only, never
re-dispatching reducers whose effects already committed (fixes additive
double-apply of time-tracking/counter deltas across hydration+retry).
- Use local monotonic seq (not lexical UUIDv7) to decide which ops are
covered by an uploaded snapshot, preventing dropped unsynced ops under
clock rollback.
- Server: include entityIds in duplicate-operation equality.
- Capture buffer no longer silently drops accepted actions at 100 (warns
to reload); drop only past a 5000 pathological cap.
Adds regression coverage for each fix. sync-core 209/209,
super-sync-server 817/817, and all touched Angular specs pass.
* feat(sync-core): drop shared-schema vector-clock compat re-export and app enum
- Remove the shared-schema → sync-core compatibility re-export of
vector-clock types/functions; retarget app files
(vector-clock.ts,operation-log.const.ts) and the server (sync.types.ts)
to import from @sp/sync-core directly
- Add @sp/sync-core to super-sync-server/package.json deps (it was
load-bearing through the re-export)
- Convert VectorClockComparison from a bare type to an as const object +
derived type in sync-core,drop the app-side enum copy and the as cast
- Update spec imports and pa ckage-boundaries.md
* docs: remove stale shared-schema vector-clock references
- Update comments in vector-clocks.md, client vector-clock.ts, and
server sync.types.ts to reference @sp/sync-core directly
- Remove @sp/sync-core dependency from shared-schema/package.json
- Regenerate package-lock.json to reflect the removed edge
* docs(sync): fix orphaned VectorClockComparison comment
The comment block describing VectorClockComparison was left dangling
above no declaration after the app-side enum was removed, and still
claimed 'Uses enum for client-side ergonomics'. Move it above the
re-export it documents and correct the wording.
---------
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
* refactor: remove dead SyncStateCorruptedError, compat exports, and 0-byte sync-providers barrel #8328
* docs(sync): drop stale SyncStateCorruptedError/fail-fast references (#8328)
The fail-fast dependency-resolution subsystem (DependencyResolverService +
SyncStateCorruptedError throw) was removed earlier; OperationApplierService now
bulk-dispatches ops in causal arrival order and returns a failedOp for the caller
to re-validate/retry. Update the sync architecture docs, archive-operations
diagram, and user-data wiki to match. Completes the dead-code cleanup for #8396.
---------
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
* refactor(sync): tighten extracted package surfaces
Combined polish from the post-extraction review:
- sync-core: strip NgRx-shaped types from EntityConfig/EntityRegistry;
expose host extensions via generic param. Move StateSelector,
PropsStateSelector, SelectByIdFactory, SelectById, EntityUpdateLike,
EntityAdapterLike to a new app-side entity-registry-host.types.ts.
- sync-core: mark OpType.SyncImport/BackupImport/Repair as @deprecated;
hosts should use createFullStateOpTypeHelpers().
- sync-providers: resolve provider.types.ts vs provider-types.ts
duplication; inline implementation into the dashed canonical name.
- sync-providers: drop unused root barrel and "." export; consumers
already use focused subpath barrels (/dropbox, /webdav, etc.).
- sync-providers: replace wildcard "@sp/sync-providers/*" tsconfig path
alias with 11 explicit subpath entries matching package.json exports;
deep-internal imports now fail at typecheck.
- sync-providers: move @sp/sync-core from dependencies to
peerDependencies (kept in devDependencies for tests).
- both packages: add composite: true to enable project references;
introduce tsconfig.build.json overlay so tsup DTS bundler still works.
- gitignore: ignore **/*.tsbuildinfo composite outputs.
* refactor(sync-core): prune 47 unused barrel exports
Removes exports with zero consumers outside the package. Source files
are unchanged; only the public barrel is trimmed. Covers compression
helper classes, sync-file-prefix error/config types, replay coordinator
internals, remote-apply result types, upload/download planning option
and plan types, ports misc, conflict-resolution helper types, and
sync-import-filter decision types.
* refactor(sync-core): drop unused encryption migration path
decryptWithMigration and DecryptResult had no host consumer; they
exposed a structural-migration entry point ("here is your ciphertext
re-encrypted under Argon2id") that nothing in the codebase reads. The
side-channel setLegacyKdfWarningHandler — which IS used — stays.
encryptWithDerivedKey/decryptWithDerivedKey lose their export keyword
and remain as module-internal helpers; encrypt/decrypt/encryptBatch/
decryptBatch still call them. Wire format and legacy-fallback semantics
are unchanged, so existing ciphertext continues to decrypt.
Test imports for compression and sync-file-prefix specs now go via
their source files instead of the trimmed barrel.
* fix(sync-providers): bound dropbox token refresh to single retry; share md5 rev helper
The five hand-rolled token-refresh blocks in Dropbox.{getFileRev,
downloadFile, uploadFile, removeFile, listFiles} recursed on themselves
after refresh. If the post-refresh call still saw a token error (real
case: the refresh token itself was revoked), the recursion would not
terminate. Consolidated into a single _withTokenRefresh helper that
attempts the call, refreshes once on a token error, retries once, then
lets the outer 401 classifier surface AuthFailSPError.
Same log message, same _isTokenError discriminator, same refresh call.
Same five sites still apply their post-call non-token error mapping
(NoRev, InvalidData, RemoteFileNotFound, path-not-found swallow, etc.).
Also extracts md5 content-rev computation duplicated between
LocalFileSyncBase._getLocalRev and WebdavApi._computeContentHash into a
shared file-based/content-rev.ts; both call sites preserve their own
error wrapping at the boundary.
* refactor(sync): split oversized super-sync and conflict-resolution
sync-providers: extract request-ID hashing from super-sync.ts (1017 ->
918 lines) into a new request-id.ts. The helpers were free functions
already in disguise (none referenced this), so the move is mechanical.
HTTP plumbing (_doWebFetch/_doNativeFetch/_fetchApi*) stays as private
methods — it transitively touches 12 instance members and would need
either a wide context object or a separate http-client collaborator
class to extract cleanly. Left as a follow-up.
sync-core: split conflict-resolution.ts into three cohesive files:
- entity-frontier.ts now owns buildEntityFrontier and
adjustForClockCorruption (per-entity vector-clock domain).
- extractEntityFromPayload and extractUpdateChanges move to
operation.types.ts next to the existing extractActionPayload.
- conflict-resolution.ts keeps deep-equality, LWW planning,
partitioning, and identical-conflict detection.
Public barrel exports unchanged; tests now import the moved symbols
from their new homes.
* refactor(sync-core): drop redundant OperationStorePort
OperationStorePort overlapped with RemoteOperationApplyStorePort on the
two state-transition methods (markSynced/markApplied,
markRejected/markFailed) and had zero non-structural consumers — the
only implementer was OperationLogStoreService, which already exposes
the three methods as its own public surface. Removing the port leaves
the service contract intact and removes the verb-pair confusion noted
in the post-extraction review.
Spec contract test still drives the same state transitions; only the
local typing of the test fixture changes from the deleted interface to
Pick<OperationLogStoreService, ...>.
* refactor(sync-providers): decouple SuperSync provider from SP-specific host
Two coupling leaks the package shouldn't carry:
1. SUPER_SYNC_DEFAULT_BASE_URL was an implicit fallback inside
SuperSyncProvider — an SP-specific URL baked into a "framework-
agnostic" package. Make defaultBaseUrl a required SuperSyncDeps
field; the host factory supplies the SP default. The constant stays
exported as a suggested default for hosts targeting the SP-hosted
server.
2. Consumers that wanted the WebSocket path had to do
`provider as unknown as SuperSyncProvider` to call
getWebSocketParams. Introduce SuperSyncWebSocketAccess interface +
isSuperSyncWebSocketAccess structural guard; SuperSyncProvider
implements it. sync-wrapper.service drops its cast in favor of the
guard.
super-sync-restore.service still casts to SuperSyncProvider for the
restore path — same pattern would solve it, but out of scope here.
* test(sync-providers): extract shared test helpers and prefer barrels
Adds tests/helpers/sync-logger.ts and tests/helpers/credential-store.ts
to centralize the noopLogger and CredentialStore mocks that were copy-
pasted across 8 spec files. createStatefulCredentialStore covers the
"load/upsert/clear with state" cases; createMockCredentialStore covers
bare vi.fn() ports. Spec sites that needed a unique mockResolvedValue
chain it after the helper, preserving behavior 1:1.
Also migrates 5 spec files from deep ../src/<file> paths to the
matching sub-barrel (../src/webdav, /http, /super-sync, /platform) for
symbols already exported there. No new barrel exports added — internal
types (WebDavHttpAdapter, WebdavApi, DropboxApi, etc.) stay on deep
paths because they are intentionally not part of the public surface.
super-sync.spec.ts keeps its own credential/logger mocks (special
__asPort wrapper and vi.spyOn against the live NOOP_SYNC_LOGGER) that
the generic helpers cannot reproduce without bloat.
* test(sync): pin vector-clock pruning, error-meta privacy, and sync-import edges
Fills three test gaps surfaced by the post-extraction review:
- vector-clock pruning correctness across clocks: 4 cases pinning that
pruning legitimately flips GREATER_THAN to CONCURRENT/LESS_THAN when
the dropped keys are still present in the comparison clock. This is
the documented behavior (compareVectorClocks is intentionally not
pruning-aware); the protocol handles flips server-side via the
rejected-ops retry loop. preserveClientIds case also covered.
- error-meta privacy boundary: 22 new cases covering urlPathOnly (strip
query/fragment/userinfo, preserve host+path+port, leave non-URLs
intact) and errorMeta (no leakage of headers, response bodies, OAuth
tokens, signed-URL params, user emails, or attached error fields).
Real negative assertions (.not.toContain), not shape checks.
- sync-import-filter edge cases: 8 cases covering empty clocks on
either side, op clock listing the import client at 0, same-client
with equal counter (pinning the strict-greater-than boundary), and
different-client knowledge above the import counter.
sync-core 195 -> 207 tests, sync-providers 319 -> 341 tests; no
production code changed.
* style(sync-core): format sync-file-prefix.spec import line
* fix(sync): address package review feedback
Findings from a multi-agent review of the recent sync extraction:
- Seven error classes in @sp/sync-providers shipped with a leading
space in `name`, and `UploadRevToMatchMismatchAPIError` was further
truncated to ' UploadRevToMatchMismatchAP'. Consumers use instanceof
so runtime behavior was preserved, but stack traces, error envelopes,
and structured-log meta carried the broken names. Added a regression
test asserting instance.name matches ErrCtor.name for all 14 classes.
- @sp/sync-core decryptBatch JSDoc claimed Argon2 errors are never
silently masked as legacy fallbacks, contradicting the actual
catch-and-decryptLegacy path. The fallback is part of the public
wire-format contract; rewrote the comment to match the implementation
and reference the module-level wire-format spec.
- SuperSyncEncryptionToggleService.enable/disableEncryption promised
"Clear cache on success" but never called clearSessionKeyCache().
Added the call on the success path in both methods, matching the
pattern used by EncryptionPasswordChangeService and
FileBasedEncryptionService.
- Removed packages/sync-core/tests/ports.spec.ts — 57 LOC of
vi.fn().mockResolvedValue type-assertion tests with no production
code under test. Port shapes are enforced at the host via
`implements` clauses with real behavioral coverage in sibling specs.
- super-sync: keep the 75s abort timer alive across response.text() on
non-OK responses so a stalled error body still triggers AbortError.
- decryptBatch: hold derived keys in a batch-local map. Previously
Phase 3 read from the LRU session cache, which could evict entries
mid-batch when the input contained more unique salts than the cache
could hold (SESSION_DECRYPT_CACHE_MAX_SIZE = 100), crashing on the
non-null assertion. Adds a 120-item regression test.
- session cache: replace the 32-bit djb2 password identifier with a
length-prefixed full-password key (injective). A djb2 collision
silently returned a key derived from a different password, producing
undecryptable ciphertext on subsequent encrypts.
- docs: bless salt-per-session-per-password semantics explicitly in the
encryption.ts JSDoc and the sync-core README so future readers and
tests know IV uniqueness — not salt uniqueness — is the AES-GCM
invariant being preserved.
* refactor(sync): extract framework-agnostic sync types into @sp/sync-core
Stand up packages/sync-core/ as the new home for sync types and
constants that have no Angular/NgRx coupling. This is the thin first
slice of separating sync engine, configuration, and provider concerns
into distinct packages.
Files moved (sources now live in packages/sync-core/src/):
- core/operation.types.ts
- core/action-types.enum.ts
- core/lww-update-action-types.ts
- core/sync-state-corrupted.error.ts
- core/types/apply.types.ts
- sync-providers/provider.const.ts
- util/entity-key.util.ts
Original paths in src/app/op-log/ keep working as thin re-export stubs
so existing callers don't change. op-log/sync-exports.ts now sources
provider.const exports directly from @sp/sync-core.
Files with transitive Angular dependencies (encryption, sync-errors,
provider.interface, vector-clock util) stay in the app for now — moving
them requires introducing a logger port and is part of the next slice.
* refactor(sync): keep @sp/sync-core domain-agnostic
The first slice landed too much Super Productivity-specific content in
@sp/sync-core. The lib should expose generic sync primitives; the host
app supplies the SP-specific config.
Pulled out of the lib (now app-side only):
- ActionType enum (NgRx action strings for SP features)
- ENTITY_TYPES / EntityType union (SP domain entities)
- SyncImportReason union (SP import flows)
- RepairSummary / RepairPayload (SP repair shape)
- WrappedFullStatePayload + appDataComplete helpers (SP wire format)
- SyncProviderId / SyncStatus / ConflictReason / OAUTH_SYNC_PROVIDERS
/ REMOTE_FILE_CONTENT_PREFIX / PRIVATE_CFG_PREFIX (SP providers/keys)
- The @sp/shared-schema dep (also SP-coupled)
Generic-ized in the lib:
- Operation.actionType: string and Operation.entityType: string (lib
carries opaque strings; host narrows in app code)
- Operation.syncImportReason removed; app extends Operation with it
- VectorClock now defined locally as Record<string, number>
- LWW helpers replaced by createLwwUpdateActionTypeHelpers(entityTypes)
factory; the app instantiates it with SP's ENTITY_TYPES
- entity-key.util uses string for entityType
App stubs now redeclare the SP-narrowed Operation, EntityChange,
EntityConflict, ConflictResult, MultiEntityPayload, ApplyOperationsResult
on top of the lib generic types via Omit-and-extend, and re-host all the
SP-specific helpers (WrappedFullStatePayload, extractFullStateFromPayload,
assertValidFullStatePayload, RepairSummary, RepairPayload).
Also added @sp/sync-core to src/tsconfig.spec.json paths so the spec
build uses the source, not the dist (matching shared-schema).
* docs(sync): add @sp/sync-core extraction plan
Roadmap for carving the sync engine out of src/app/op-log/ into a
reusable, framework-agnostic AND domain-agnostic @sp/sync-core package
plus a sibling @sp/sync-providers.
Documents:
- The three-concern split (engine / config / providers) target
- The domain rule: nothing SP-specific lands in the lib (ActionType,
ENTITY_TYPES, SyncImportReason, RepairPayload, SyncProviderId, the
appDataComplete wire format, @sp/shared-schema all stay app-side)
- PR 1 (landed): generic primitives only; app stubs preserve every
pre-existing call site via Omit-and-extend
- PR 2: SyncLogger port + parameterize entity-registry
- PR 3a: pure algorithmic core (vector-clock client wrapper, conflict
detection, op validation, encryption, sync-errors) — needs only the
SyncLogger port
- PR 3b: orchestrators behind ports (OperationStorePort,
ActionDispatchPort, ConflictUiPort, SyncConfigPort) — the high-risk
step where the app/lib boundary becomes load-bearing
- PR 4: lift providers into @sp/sync-providers
- PR 5: ESLint boundary rule
* refactor(sync): address sync-core extraction review
* docs(sync): refine extraction plan follow-ups
---------
Co-authored-by: Claude <noreply@anthropic.com>
