* 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.
- 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.
* 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.
