matBadgeDescription is a string input; binding null relied on
AriaDescriber.describe() silently no-op'ing on falsy values. Verified
hidden-state semantics: matBadgeHidden hides the badge via CSS only and
MatBadge keeps an aria description active regardless, so at count 0 the
description must be empty — '' is the type-correct value MatBadge
explicitly removes the aria-describedby for.
SPAP-38 (deferred LOW from PR #8874 review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- The archive-mutex comment claimed every archive mutation is locked;
compressArchive, the remote loadAllData import and the time-tracking
cleanups still write outside it (tracked in #8941). Name them.
- The merge-journal comment described an impossible skipped-duplicate
path (merged ids are fresh per run); describe the real accepted
window instead: durable merge, crash before journaling, entry stays
absent (observe-only log).
- Delete the unreachable stage-2 intra-batch duplicate pass: stage 1
(validateAndClampBatch) reserves every op id including invalid first
siblings, so validated candidates are unique by construction.
- Delete clearRawRebuildIncomplete: superseded by completeRawRebuild,
which retires the marker atomically with the recovery token; only
specs still called it.
- Clear the conflict journal when a USE_REMOTE rebuild completes — the
documented "cleared whenever the full dataset is replaced" contract
previously had a single caller (backup import), leaving stale badge
counts and review entries describing replaced history.
- _notifyResolutionOutcome: drop the win-count parameters left over
from the removed count snack and gate on resolutions.length.
- Snapshot handler: keep the clean-slate opId invariant local with a
defense-in-depth 400 instead of relying on the contract superRefine
in another package.
- Document that the legacy misc->tasks conflict alias only covers the
per-entity path (GLOBAL_CONFIG writes are single-entity today).
isIgnoreDBLock historically meant "sync already holds the op-log DB
lock", but _runTaskArchiveMutation also treated it as permission to skip
the new TASK_ARCHIVE mutex — so every remote archive side effect except
moveToArchive ran unserialized against locked local mutations, and a
concurrent read-modify-write could silently drop one side's archive
write (the exact race the mutex was added to close).
TASK_ARCHIVE is deliberately separate from OPERATION_LOG, so acquiring
it while sync holds the op-log lock is safe — the remote moveToArchive
path has always done exactly that. The mutex is now unconditional, and
the remote flushYoungToOld handler wraps its two-archive read-modify-
write in the same lock instead of writing through the adapter bare.
The isIgnoreDBLock option is retained as inert API surface; removing
the threading is tracked as a follow-up.
Review of the #8874 x #8900 merge seam found two defects in STEP 3b:
- The synthesized merged op rode in the apply batch, so the reducer
checkpoint's pending-only assertion threw on it: every real disjoint
auto-merge aborted the checkpoint transaction and wedged sync behind
IncompleteRemoteOperationsError until app restart. Checkpoint now
covers only pending-appended remote rows; synthetic local ops are
exempt (their durability contract is the append + upload path).
- appendWithVectorClockUpdate replaced the durable vector clock with a
clock computed only from the conflict's own ops, regressing the
client counter and enabling silent cross-device op drops. Merge
writes now go through appendMixedSourceBatchSkipDuplicates, which
rebases the merged op on the durable clock in the same transaction —
also closing the crash window between the remote originals and their
superseding merged op, and turning duplicate re-appends into skips
instead of ConstraintErrors.
Two regression tests enforce the coordinator's whole-batch reducer-
commit contract and the pending-only checkpoint against the resolution
flow; the journal keeps recording merges only after a durable append.
Same #8874-vintage store mock as the disjoint-merge spec: without the
atomic mixed-source batch method every local-wins flow through
autoResolveConflictsLWW threw before journaling.
The #8874 spec predates the atomic mixed-source batch and reducer
checkpoint on the store port; its two createSpyObj sites lacked the
methods, so every flow through the local-wins write path threw.
The lock-serialization refactor routes internal archive calls through
_updateTasks/_deleteTasks so a held sp_task_archive lock is never
re-acquired; five assertions still spied the public wrappers and never
fired.
Remote archive side effects must be idempotent and immune to concurrent
read-modify-write races, or a retried operation can duplicate or drop
archive rows.
- TaskArchiveService serializes every archive mutation behind a
dedicated sp_task_archive web lock (separate from OPERATION_LOG,
which remote archive handlers already hold non-reentrantly).
- ArchiveService.moveTasksToArchiveAndFlushArchiveIfDue writes tasks
with setMany over a deduplicated sorted id set, so a retry over a
partially written archive converges instead of double-adding.
- TaskService._moveToArchive coalesces concurrent archive calls for the
same ids and persists archive data before dispatching moveToArchive,
so a full-state snapshot cannot acknowledge the op while its archive
write is still in flight.
The pre-replace import backup and the USE_REMOTE rebuild recovery
marker were matched by timestamp only, so a concurrent capture or a
reload between rebuild completion and snack dismissal could clear or
restore the wrong backup.
- Import backups carry an opaque backupId (uuidv7); clearImportBackup,
replaceAllForRawRebuild and applyRemoteReplaceWithSnapshot verify the
expected identity inside their transactions before acting.
- Completing a raw rebuild atomically replaces the incomplete marker
with a durable raw_rebuild_recovery entry, so the "restore previous
data" Undo survives a reload; StartupService re-offers it at boot and
dismissal retires exactly the offered backup.
- SnackService treats a sticky recovery/update action as a single
persistent slot: unrelated transient snacks no longer destroy a
visible Undo, and dismissal awaits the snack's dismissFn.
* refactor(tasks): extract shared task ordering helpers
Extract getReorderedSubTaskIds (membership check + move shared by
moveSubTaskUp/Down/ToTop/ToBottom) and moveValidIdsToFront (shared by
removeTasksFromTodayTag and localRemoveOverdueFromToday) into pure,
tested utils. No behavior change.
Closes#7912
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(tasks): drop getReorderedSubTaskIds, inline check in reorderSubTask
Review feedback (#8926): the moveSubTask* actions already funnel through
the single reorderSubTask helper, so the extraction added no dedup value.
Keep moveValidIdsToFront, which removes real duplication.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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>
Remediate the findings of the 5-agent necessity review:
- USE_REMOTE: a local capture racing the locked rebuild now throws a
typed CaptureRacedRebuildError, and forceDownloadRemoteState retries
phase 2 in-call (bounded, 3 attempts) through the existing
crash-resume branch — raced ops fold into preservedLocalOps and the
already-downloaded history is reused (WS downloads and immediate
uploads stay gated by the marker, so no re-download is needed).
Previously every attempt aborted while e.g. time tracking dispatched
continuously, re-downloading the full history per sync trigger and
churning the Undo snack (now shown on final failure only).
- Hydrator archive retry: pass skipDeferredLocalActions and drain
explicitly with a caught error. A drain throw from the coordinator's
finally used to mask the archive result and escalate out of
hydrateStore() into attemptRecovery(), which can import stale legacy
data over a correctly hydrated store.
- Incomplete-remote gate: run one in-session archive-only retry when
the only blockers are quarantined failed/archive_pending ops, so a
transient archive failure self-heals on the next sync instead of
wedging until app restart. Never attempted while the rebuild marker
is set or reducer-uncommitted pending rows exist.
- Boot recovery: StartupService surfaces the pre-replace backup's
persistent restore snack when a stranded raw_rebuild_incomplete
marker is found, covering users who boot offline or disable sync
after finding the app "emptied" by a mid-rebuild crash.
- Snack correctness: latch the USE_REMOTE newer-schema warning once per
session; guard _notifyBlockedOp and the LWW apply-failure snack with
hasPendingPersistentAction() so they cannot destroy a visible Undo;
drop the useless "Update app" action for below-minimum data.
- Strings: MIGRATION_FAILED / VERSION_UNSUPPORTED now describe the
blocking semantics instead of the removed skip behavior.
- Store: getPendingRemoteOps excludes rejected rows (parity with
getFailedRemoteOps) so a rejected-but-pending row cannot trip the
sync gate for a session.
- Server: compute the upload request fingerprint eagerly after the
rate-limit gate — identical cost to the lazy closure in every path,
minus the memoization machinery; laziness remains in the snapshot
handler where it skips hashing multi-MB states.
op-log suite 3004/3004, super-sync-server 831/831, checkFile clean on
all touched files. Nine regression tests pin the new behavior.
These eslint-disable directives no longer suppress any reported
problems, so ESLint flags them as unused directive warnings. Remove
them to clear the 11 lint warnings.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* chore(ui): removes dead utilities and op-log leftovers [#8260 - Tier A]
* update readme and remove-unused-log-imports.ts
* restore benchmark test and make runnable
* fix(app): hide donation page on macOS
Use the stable macOS bridge instead of the MAS runtime flag and redirect direct donation-page navigation on restricted Apple builds.
* fix(app): harden donation platform gating
Give the restriction a behavior-specific contract, cover platform classification and real router redirects, and correct the platform documentation.
Fresh clients generate startup config and genesis operations before their first sync. Treat those operations as setup state until a prior sync exists, while continuing to protect later config changes and all ordinary user work from authoritative snapshot replacement.
* fix(sync): defense-in-depth vs entityId retarget on encrypted ops
SuperSync E2EE (AES-256-GCM) covers only op.payload; metadata fields
(entityId, opType, actionType, vectorClock, isPayloadEncrypted, ...)
travel as plaintext and are not bound by the auth tag. A malicious/
compromised sync server or MITM could retag an encrypted LWW-update op
with a different entityId, redirecting the authenticated changes onto an
attacker-chosen entity — convertOpToAction() previously trusted the
tampered entityId over the authenticated payload.id (coercing even a
missing payload.id) and only warned.
At the decrypt boundary (where encryption origin is known) verify that
an in-scope LWW op's authenticated payload carries a string id equal to
op.entityId; otherwise fail closed via a new OperationIntegrityError,
distinct from DecryptError so it does not trigger the enter-password
dialog. The gate mirrors convertOpToAction's predicate (alias resolution
+ singleton exclusion) so the two boundaries cannot drift.
Scoped defense-in-depth for GHSA-8pxh-mgc7-gp3g, NOT full integrity.
Still open (durable AAD-envelope fix): plaintext-injection downgrade via
isPayloadEncrypted=false (needs a download-side mandatory-encryption
guard), opType promotion, entityType swap, vectorClock replay. Correct
the overstated integrity claim in the encryption architecture doc.
* fix(sync): reject plaintext ops when SuperSync encryption is mandatory
The isPayloadEncrypted flag is unauthenticated plaintext metadata, so a
compromised SuperSync server or MITM could set it to false and inject a
fully attacker-authored plaintext op — it would skip decryption AND the
payload/metadata integrity check and be applied verbatim (arbitrary op
forgery). This is a strictly more powerful bypass than the ciphertext
entityId retarget closed previously.
assertOpsEncryptedWhenExpected rejects any inbound plaintext op (download
+ piggyback paths) when encryption is enabled. It gates on config INTENT
(isEncryptionMandatory && isEncryptionEnabled()), not key presence, so it
also fails closed in the dropped-credential state (a !!encryptKey gate
would fail open there). Safe with no legacy-data loss: enabling
encryption deletes the server copy and re-uploads everything encrypted,
so no legitimate plaintext op remains; a never-encrypted account
(isEncryptionEnabled()===false) still accepts plaintext. The SuperSync
op-level twin of the file-based GHSA-vrc7 download guard and the
GHSA-9544 upload guard.
Also give OperationIntegrityError a dedicated sync-wrapper branch: fail
closed with a calm translated message instead of the raw GHSA/technical
string.
Follows up the review of GHSA-8pxh-mgc7-gp3g.
Multi-review remediation for the USE_REMOTE rebuild path.
- Crash-resume recovery gap (confirmed by 2 independent reviewers): when an
interrupted-rebuild resume aborts in its download/validate phase (empty or
newer-schema remote, or a persistent download failure), forceDownloadRemoteState
threw before it could offer Undo. The prior attempt had already committed the
destructive baseline, so the pre-replace backup was stranded with no restore
affordance — reading as total data loss. downloadRemoteOps now surfaces the
recovery Undo on a resume abort (deduped via hasPendingPersistentAction so
repeated auto/WS syncs don't respawn it). Covered by two new unit tests.
- SnackService: collapse the redundant dual write of the persistent-action flag
(set in open() AND recomputed in _openSnack) to one authoritative write in
open(); _openSnack keeps only the on-dismiss clear (3-reviewer consensus).
- Document the archive-retry idempotency invariant on ARCHIVE_AFFECTING_ACTION_TYPES:
the hydrator retry re-runs archive side effects with skipReducerDispatch even
after a fully-successful run, so they must stay idempotent (overwrite, never
additive) to avoid double-counting time-tracking/counter deltas.
After a "Use Server Data" replace shows the persistent Undo recovery
snack (#8107), a follow-up routine sync-success snack must not silently
replace it. SnackService now tracks a pending persistent action
(actionStr + duration 0); the header sync() skips its success feedback
while one is showing. Unblocks the committed USE_REMOTE crash-resume e2e
(supersync-use-remote-crash-resume.spec.ts), whose Undo assertion
depends on the recovery snack surviving the sync that resumed the rebuild.
USE_REMOTE's atomic rebuild synthesizes a globalConfig shell from
DEFAULT_GLOBAL_CONFIG (getDefaultMainModelData omits globalConfig), then
re-applies the device's local-only sync settings (provider, isEnabled,
isEncryptionEnabled, syncInterval, isManualSyncOnly) onto the baseline.
Without this, an interrupted rebuild committed a baseline whose sync
config was pure defaults, so the resumed client could lose the provider
and schedule it needs to sync again. Adds a unit test asserting the
device-local settings survive into runRemoteStateReplacement's baseline.
* style(tasks): elevate add-subtask input
* fix(tasks): parse eml files without external dependency
Replace postal-mime with a bounded parser for sender, subject, and unencoded plain-text bodies. Ignore unsupported MIME body formats and document the root dependency policy.
* fix(tasks): isolate and harden eml import
Lazy-load the local parser, reject lossy or unsupported bodies, and store accepted text as literal notes so imported content cannot trigger remote resource loads. Document the title-only fallback and expand regression coverage.
* fix(tasks): decode eml headers and harden import parsing
- Decode RFC 2047 encoded-words in the subject and sender name so
international titles show "Grüße" instead of raw "=?UTF-8?...?=".
- Replace the charset regex with a quote-aware Content-Type parser so a
charset inside another quoted parameter can't be mistaken for the real
one, and ignore RFC 822 header comments (e.g. "7bit (comment)").
- Cap the synced title (300) and body (100k) so a crafted .eml can't
amplify into an oversized op (the literal fence can double the body).
- Document parseEml's intentional MIME omissions to prevent a later
"fix" that reopens the untrusted-body attack surface.
The integration spec still pinned the pre-branch semantics where a
pending GLOBAL_CONFIG op counted as non-meaningful. The widened gate
(79f91e36fe) deliberately treats config changes as meaningful user work
(entity-wide exemptions are unsafe — GLOBAL_CONFIG carries synced
preferences), pinned by the gate unit spec but missed here, so the full
suite failed since that commit. Split the case: example-only pending
stays silent + discardable; config pending now expects the dialog.
- 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 stays on the
receive/upload paths (0 still parses so a below-minimum remote op
surfaces as VERSION_UNSUPPORTED, not a generic migration failure).
- 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.
- 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.
* chore(deps): pin npm version to remove lockfile drift
Contributors on npm 10 (Node 22's bundled default) vs npm 11 reconcile the
app-builder-lib.minimatch override differently, rewriting package-lock.json on
npm install. Pin npm 11.7.0 via the standard packageManager field (Corepack)
and complete the existing volta block so volta users pick it up automatically.
Ref: discussion #8869
* chore: stop tracking .claude/skills symlink (runtime-managed)
The Claude Code runtime scaffolds .claude/ each session and materializes
.claude/skills as a real directory, clobbering the committed symlink
(-> ../.agents/skills). Git then reported a perpetual 'D .claude/skills' in
every worktree/session. Skill sources remain tracked under .agents/skills/;
let the runtime own .claude/skills and ignore it via the existing /.claude/*
rule.
* fix(config): restore day-start offset after operation replay
Reconcile DateService and app-state from config after incremental or full-state bulk replay without re-minting persistent task operations.
Fixes#8890
* fix(sync): replay full-state operations through reducers
Normalize canonical full-state operation types to LOAD_ALL_DATA during replay while preserving their original operation metadata. This lets repair and legacy backup operations install state through the existing reducers.
* feat(plugins): add Todoist import plugin with Import/Export launcher
* fix(plugins): fix cross-chunk subtask loss in todoist-import + hardening
* fix(plugins): clamp non-finite Todoist durations + review polish
* feat(plugins): add Eisenhower priority mapping to Todoist import
Replace the single "map priorities to p1-p3 tags" checkbox with one
mutually-exclusive control (Nothing / p1-p3 tags / Eisenhower matrix).
The new Eisenhower option reuses SP's built-in urgent/important tags by
title (p1 -> urgent+important, p2 -> important, p3 -> urgent, p4 -> none),
so imported tasks populate the Eisenhower Matrix board with no new tag
and no new plugin API. Collapsing Todoist's single priority axis onto the
2-D matrix is opinionated, so it stays opt-in and off by default.
Requested in the review of #8882.
* fix(plugins): harden Todoist import data integrity
* fix(plugins): improve Todoist import feedback
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.
* fix(task): schedule overdue tasks for today via Shift+T and guard stale task focus (#8851)
* fix(planner): only bind data-task-id on planner-task when focusable
The unconditional data-task-id host binding broke the e2e done-confirmation
strategy in e2e/pages/task.page.ts, which selects its wait branch based on
the attribute's presence. Boards render planner-task cards without an inner
<task> element, so the id-based wait could never resolve (#8851).
* refactor(tasks): share overdue predicate across selector and util
The rebase onto #8858 re-inlined the overdue comparison into
selectOverdueTaskIds and dropped the delegation to isTaskOverdue, so the
two overdue definitions (the overdue list vs. the Shift+T "Add to Today"
path) were duplicated and free to drift — the exact footgun #8851 set out
to avoid, and the util's JSDoc still claimed the selector delegated to it.
Extract isTaskOverdueByThreshold as the single source of truth for the
comparison and getLogicalTodayStartMs for the boundary. isTaskOverdue and
selectOverdueTaskIds both route through it, so they cannot drift. The
selector still computes the threshold once per recompute (no per-task date
parsing), preserving #8858's perf posture. No behavior change.
* test(e2e): base markTaskAsDone strategy on <task> host, not attr
planner-task now carries data-task-id in the Planner overdue list (#8851),
so keying the done-confirmation strategy off data-task-id presence would send
a wrapper down the <task> path — document.querySelectorAll('task') finds no
match and the wait hangs 10s. Key it off the element actually being a <task>
instead. No behavior change for real <task> rows; every wrapper keeps the
300ms wrapper path. Verified: worklog-basic (real task) and boards #7498
(planner-task wrapper) both pass.
---------
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
