A v18.14 snapshot (schema v2) lacks globalConfig.idle.isSuppressIdleDuring
FocusMode, which #8965 added as a required field without its own schema bump.
Upgrading bumps the snapshot onto the v2->v4 migration path, whose validation
gate — uniquely, the only validator that runs on the RAW snapshot before the
loadAllData reducer can backfill defaults, and the only one that is fatal
rather than repair-or-tolerate — rejects it. Hydration then aborts, recovery
refuses because a snapshot still exists, and the app boots to an empty store,
deterministically, every launch.
Two fixes:
- v2->v3 migration backfills the opt-in default (false) when the field is not
already a boolean (never clobbering a real user choice), so the migrated
snapshot validates and a clean v4 snapshot persists immediately.
- The migration-path state-validation gate is now non-fatal: on failure it
rolls the on-disk cache back to the pre-migration backup (never persisting an
unvalidated snapshot) but returns the migrated snapshot for reducer-healed
hydration, instead of throwing into disaster recovery. Genuinely corrupt
state is still caught at Checkpoint C and not persisted. Metadata-validation
failures remain fatal.
Both paths are sabotage-tested to fail without their fix. A known follow-up
(noted in code) can persist a fresh snapshot after any migration-then-valid
hydration so the safety-net path converges in one boot instead of re-migrating
each launch for a not-yet-backfilled field.
A bare CREATE INDEX CONCURRENTLY aborted mid-build (the in-image step
timeout firing at its 1800s default when a raised MIGRATION_TIMEOUT was
not forwarded, an external stop, or OOM) leaves the migration failed and
an INVALID index of the target name, wedging the deploy. Deployed images
reported only a bare "prisma migrate deploy failed (exit 143)" with no
recovery steps, so operators had no way forward.
- deploy.sh: forward MIGRATE_STEP_TIMEOUT into the migrator so a large
MIGRATION_TIMEOUT is not silently capped at the image default (1800s)
and kill a slow CREATE INDEX CONCURRENTLY early (the root cause).
- migrate-deploy.sh: normalize BusyBox `timeout` 143 -> 124 so a step
timeout hits the timeout branch, not the generic one.
- migrate-deploy.sh: emit_interrupted_recovery_hint() prints copy-paste
recovery (drop the INVALID index, roll the record back, re-run) for an
interrupted CONCURRENTLY build, from BOTH the 124 timeout branch (where
a normalized 143 -- the incident's own signal -- lands) and the generic
non-gate branch (OOM/137), plus the existing P3009 re-run path. Every
path prints guidance only and NEVER auto-resolves a bare CREATE.
- env.example: document the now-forwarded MIGRATE_STEP_TIMEOUT knob.
- tests: cover 143 normalization, the incident's 143->124 timeout-branch
recovery, the P3009 and non-gate bare-create recovery, and the
forwarded step timeout.
The bare CREATE stays bare and fail-loud on purpose: 20260613000001
ships in v18.11.0-v18.14.0 and is applied natively on healthy deploys,
so converting it to a drop-then-create shape would change its checksum
and break every DB that already applied it.
Recovery for the current wedged deploy (20260613000001):
DROP INDEX CONCURRENTLY IF EXISTS "operations_entity_ids_gin";
prisma migrate resolve --rolled-back \
20260613000001_add_operation_entity_ids_gin_index
then re-run the deploy with the step timeout raised enough for the GIN
build (and clear any idle-in-transaction blocker first).
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
* fix(sync): deduplicate surgical sync retries
Persist split-file configuration and acknowledge operation IDs already committed remotely after a lost upload response.
* fix(sync): preserve import author during clock pruning
Keep the active causal full-state author in oversized stored clocks and reuse the stored protected IDs when classifying response-loss retries.
* test(sync): harden supersync failure coverage
Exercise real upload endpoints and exact operation IDs across response loss, validation failures, schema blockers, full-state boundaries, vector pruning, and concurrent edits.
* fix(sync): heal a corrupt primary on duplicate-only uploads
The .bak recovery path caches the CORRUPT primary's rev precisely so this
cycle's conditional overwrite repairs sync-ops.json. The duplicate-retry
short-circuit read that cache and returned before the write, leaving the
primary corrupt whenever the recovered buffer already held every pending
op. Flag the recovered entry and let those uploads fall through.
Also stop synthesising a serverSeq for ops already in the buffer: the
field is optional, the upload and download paths number ops differently,
and a mixed batch could hand two ops the same value.
* perf(sync): look the full-state author up once per upload
Batch upload is off by default, so the guarded batch path was not the one
serving production: the serial path queries the causal full-state author
per op inside a single transaction, and a clock of 21-50 entries passes
validation and trips the guard on every one of them. Memoize the author
per transaction and resolve it lazily, so only an op whose clock actually
overflows pays, and only once. This also retires the batch pre-scan and
its loop-carried author, leaving both paths on one mechanism.
Report the lookup through ProcessOperationResult so the upload summary
stops under-reporting round-trips, and record why reconstructing the
stored protected set loosens id-collision detection.
* test(sync): wait for the committed title in renameTask
renameTask blurred the textarea, slept 300ms and returned without ever
checking the rename landed. Blur -> dispatch -> re-render outruns that
delay on a loaded machine, so a following sync uploads without the rename
op and the caller asserts against a task that was never renamed — which
is what supersync 3.1 hits on CI but never locally.
Wait for the new title instead, mirroring markTaskDone's done-state wait
and the e2e no-waitForTimeout rule.
* test(sync): dispatch focus so renameTask actually commits
renameTask relied on el.focus() to emit a focus event, but these tests
drive two clients as separate pages and only one page can hold focus, so
on CI the event often never fires. TaskTitleComponent then keeps
_isFocused=false, and resetToLastExternalValueTrigger resets tmpValue to
the stored title on the next task-object emission. Blur therefore
computes wasChanged=false, task.component skips update(), and the rename
is silently dropped without ever becoming an op.
That is supersync 3.1: client A's rename lives only in tmpValue, A syncs
and uploads nothing, B uploads its done op, A downloads it, the task ref
changes and the title reverts to the original — exactly the state the CI
artifact captured. A real user always has real focus, so the app itself
is unaffected.
Dispatch focus explicitly, mirroring the synthetic input/blur already
used here. Also correct the previous commit's claim: the toBeVisible
wait matches tmpValue, a component-local signal rendered in both
template branches, so it never observed the committed title and could
not have fixed this.
* docs: revert incidental prettier reformat of unrelated docs
The master merge ran prettier across files it pulled in, reformatting
three documents this branch has no business touching: markdown table
cell padding plus *emphasis* -> _emphasis_, with no content change.
handover.md documents two unrelated branches entirely.
Restores them to master. vector-clocks.md keeps its edits — those are
this branch's own and describe the pruning protection.
* refactor(sync): drop the full-state author lookup's roundtrip accounting
resolveFullStateAuthor memoizes per transaction, so the lookup it counts
fires at most once per upload — the plumbing existed to report a number
that is always 0 or 1, on a log line already counting dozens. The memo
and its own accounting cancelled out.
Removing didQuery lets resolveFullStateAuthor return string | undefined
and getPruneProtectedIds return string[], instead of both carrying a
tuple purely to feed the counter.
uploadDbRoundtrips and the batch path's own counter are untouched.
* test(sync): assert the committed store title in renameTask
The focus dispatch did not fix supersync 3.1 — the shard failed again
with an identical snapshot (original title, done, rename gone), so that
diagnosis was wrong. Stop guessing at the trigger and make the helper
able to observe the thing in question.
task-title renders tmpValue, a component-local signal, in BOTH its
editing and idle branches. Every DOM assertion here therefore matches as
soon as the synthetic input event fires, whether or not an op was ever
captured — which is why two rounds of "wait for the title" changed
nothing. Read the store instead, via the __e2eTestHelpers.store hook the
timeSpent helper already uses.
This is a diagnostic as much as a fix: it splits the two remaining
explanations. If renameTask now fails, the rename never becomes an op
and the bug is in how the test drives the edit. If it passes and 3.1
still fails at the merge assertion, the op is captured and lost during
sync — a real defect, and the test is right to fail.
* test(sync): move the 3.1 disjoint-merge rewrite out to #9095
3.1 was the last red shard, and it turned out to be right: the
store-backed renameTask passes, so the rename IS captured as an op, and
the test still fails at the merge assertion — the op is committed and
then lost during sync. Filed as #9095.
That bug is pre-existing and cannot be reached by anything in this PR:
the file-based adapter is not used by SuperSync, and the server-side
author memo only engages for clocks over 20 entries where this test
carries about three. 3.1 is also the only test here that exercises
neither of this PR's fixes — it races a title change against a
move-to-done, which is conflict resolution, not retry dedup or clock
pruning. So it moves to #9095 rather than holding verified sync fixes
red.
The rest of the hardening stays: the fault injections whose globs never
matched a real endpoint, the schema-mismatch test that asserted nothing,
and the compaction suite that called an endpoint which never existed are
what actually cover the fixes here.
Restoring the old 3.1 puts a misleading test back, so it now carries a
comment saying why it proves little and where the real one lives. The
strengthened version is kept on test/issue-9095-disjoint-merge-repro.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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.
* test(sync): reproduce file-based compaction stranded ops pointer (#9040)
Concurrent split-file compactions can leave the committed sync-ops.json
referencing a snapshot present in neither sync-state.json nor its .bak: the
snapshot is force-written unconditionally while only the ops file is gated, so
the loser of the ops race can clobber the winner's snapshot after backing up an
older generation.
Adds a failing integration test that drives two interleaved compactions against
the stateful MockFileProvider and asserts a fresh client can still hydrate the
committed generation. Fails today (unrecoverable gap); passes once the snapshot
is written under a generation-unique, immutable name.
* fix(sync): write split-compaction snapshots to immutable files (#9040)
Concurrent split-file compactions could strand the winning ops pointer: the
snapshot was force-written to the fixed sync-state.json, so the loser of the
conditional ops-commit race could clobber the winner's snapshot, leaving the
committed sync-ops.json referencing a snapshot absent from both sync-state.json
and its .bak — an unrecoverable gap for a fresh client.
Write the compaction snapshot to a generation+client-unique immutable file
(sync-state__<syncVersion>__<clientId>.json) recorded in snapshotRef.file. A
concurrent compactor writes a DIFFERENT file, so the winning pointer can never
be clobbered. Readers prefer snapshotRef.file and fall back to sync-state.json /
.bak (kept dual-written for pre-#9040 clients). The superseded snapshot is
GC'd O(1) after each commit; a losing compactor's same-generation orphan is a
rare, bounded leak (documented, with a listFiles-prune upgrade path).
Also fixes MockFileProvider to model create-if-absent (If-None-Match: *) so the
fresh-folder concurrent-compaction path is faithfully gated in tests.
Tests: 3 concurrent-compaction integration scenarios (harmful/benign interleave,
fresh-folder race) + 2 unit tests pinning the snapshot resolution order. Full
op-log suite (3427) and sync-providers package (404) green.
* fix(sync): reclaim orphaned snapshot when compaction loses the commit race (#9040)
A concurrent compactor that writes its immutable snapshot but then loses the
conditional ops-commit left that snapshot orphaned — no committed ops file ever
referenced it, and the O(1) predecessor-GC never reclaimed it (same generation).
The losing compactor now deletes the snapshot it just wrote when its commit
throws, eliminating the leak at its only realistic source. Only a crash between
the snapshot write and the commit/cleanup can still orphan a file (rare crash
window; listFiles-prune remains the documented backstop).
Refactors the GC helper into a single-file _removeGenStateFile primitive used by
both the post-commit predecessor delete and the new failure-path cleanup.
Tests assert the loser's orphan is gone after both the near-cap and fresh-folder
concurrent-loss scenarios. Full op-log suite (3427) green.
* fix(sync): only reclaim orphaned snapshot on confirmed rev-mismatch (#9040)
The loser-orphan cleanup deleted the just-written immutable snapshot on ANY
commit failure. A rev-mismatch is a confirmed server rejection (ops did not
commit), but a network/5xx error is ambiguous — the ops PUT may have landed and
committed, in which case the snapshot is still referenced and deleting it would
strand a reader. Restrict the cleanup to UploadRevToMatchMismatchAPIError.
Adds a test asserting the immutable snapshot survives a non-mismatch commit
failure. Full op-log suite (3428) green.
* fix(sync): use a random suffix, not clientId, in immutable snapshot names (#9040)
Filenames are not encrypted, so embedding the clientId in
sync-state__<syncVersion>__<clientId>.json leaked device count/platform and a
per-device counter to the remote — a metadata downgrade for E2EE file-based
users. Replace the clientId with an opaque random suffix, which still gives two
concurrent compactors distinct files. A collision is astronomically unlikely and
self-heals (the reader validates loaded content against snapshotRef, so a wrong
file fails validation and falls back to sync-state.json/.bak). syncVersion stays
in the name for legible ordering and a future listFiles-prune.
Tests now assert on the count of immutable snapshot files (and a captured name)
rather than hardcoding the now-random filenames. Full op-log suite (3428) green.
* fix(supersync): exclude legacy REPAIR from retention cleanup pruning
The daily-cleanup fallback (used when `latestFullStateSeq` is absent, i.e.
legacy/pre-marker installs) selected the pruning boundary with a raw
`opType: { in: [SYNC_IMPORT, BACKUP_IMPORT, REPAIR] }` filter that includes
legacy REPAIR rows (`repairBaseServerSeq` NULL). Such a repair carries no
causal base cursor proving its state is current as of its seq, so pruning
history behind it can permanently drop ops committed between its logical
base and its seq for any device replaying from before it.
#8971 migrated five full-state queries to `CAUSAL_FULL_STATE_OPERATION_WHERE`
but missed this one — the single query whose result directly authorizes a
DELETE. Route it through the same causal-only predicate; a legacy-repair-only
user then yields `protectedFromSeq === null` and is skipped (no deletion),
the safe default already implemented below.
Adds a regression test whose mock `findFirst` honours the causal
`repairBaseServerSeq` predicate.
* fix(supersync): gate misc→tasks conflict alias on the split boundary
`detectConflictForEntity` and `prefetchLatestEntityOpsForBatch` looked up
the legacy `GLOBAL_CONFIG:misc` alias for an incoming `tasks` write with
`schemaVersion < CURRENT_SCHEMA_VERSION`. Once v4 shipped, that aliases
post-split v2/v3 misc writes — disjoint from `tasks` — and fabricates false
`CONFLICT_CONCURRENT` rejections of tasks-settings writes for multi-device
users.
Gate both read-side lookups on the fixed `MISC_TASKS_SPLIT_SCHEMA_VERSION`
(2), matching the `isLegacyMiscConfigOperation` incoming gate and the
warning comment it already carries. Drops the now-unused
`CURRENT_SCHEMA_VERSION` import.
Adds real-PostgreSQL integration coverage for both changed lookups
(`detectConflictForEntity` and the batch `prefetchLatestEntityOpsForBatch`):
a post-split v2/v3 misc write must not alias, a legacy v1 one still must.
* fix(sync): re-upload LWW local-win ops after a WS-triggered download
A WS-triggered download that resolves a conflict against pending local ops
appends LWW local-win replacement ops straight to the op-log, bypassing the
capture effect. Unlike `ImmediateUploadService` and the main sync loop, this
path had no follow-up upload, so the preserved edit sat unsynced until the
next user edit or periodic sync — an unbounded window for manual-sync-only
users.
Re-upload when `localWinOpsCreated > 0`, mirroring the other two paths, and
surface the same terminal outcomes `ImmediateUploadService` does — permanent
rejection / rejected-full-state baseline → ERROR, mandatory-but-missing key →
UNKNOWN_OR_CHANGED — so a preserved edit that fails to converge is not left
silent (the WS path never claims IN_SYNC). Single follow-up only, matching the
sibling side channel.
* fix(supersync): validate the primary latestFullStateSeq marker as causal before pruning
The scheduled retention cleanup trusted `state.latestFullStateSeq` as a pruning
boundary whenever it was `<= lastSnapshotSeq`, with no check that the marked op is
a causal full-state operation. The earlier fix (37bf818) hardened only the
fallback query used when the marker is absent.
Installs upgraded from before #8973 can carry a `latestFullStateSeq` set from a
legacy REPAIR (repairBaseServerSeq NULL) through the old isFullStateOpType gate,
and that migration shipped no backfill to clear stale markers. Trusting such a
marker prunes history behind a repair the replay path deliberately refuses as a
boundary (LegacyRepairReplayUnsupportedError) — permanent, cross-device loss.
Validate the marked op against CAUSAL_FULL_STATE_OPERATION_WHERE before it can
authorize a DELETE; a stale marker drops to the (now causal-only) fallback query
or the user is skipped. Updates the happy-path test to a causal boundary, adds a
stale-marker regression test, and teaches the mock findFirst to honour an exact
serverSeq predicate.
* test(e2e): de-flake USE_REMOTE crash-resume reload
`page.reload()` defaulted to waiting for the `load` event, which can never
fire while an active SuperSync WebSocket/sync connection keeps the page
"loading" — so the reload timed out at 30s (observed flake:
`page.reload: Timeout 30000ms exceeded`). Three sibling sync specs already
document this hang and work around it with close()+newPage(), but that drops
sessionStorage, which this test asserts on across the reload.
Wait only for `domcontentloaded` (with 60s headroom) instead; `waitForAppReady`
— which itself only needs `domcontentloaded` — remains the real readiness gate,
and the reload still preserves sessionStorage.
* 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.
* docs(plugins): add microsoft 365 calendar provider plan
* docs(plugins): add microsoft 365 calendar provider plan
* docs(ios): plan internal testflight builds
* fix(sync): prevent file-based sync data loss
Preserve post-snapshot operations and stage remote baselines until durable apply.
Use conditional writes and a resumable marker for provider concurrency and legacy migration. Report trustworthy remote timestamps and document transport limits.
Refs #8960
* fix(sync): avoid biased conflict recommendations
Highlight a side only when its timestamp or known change count is strictly greater, leaving unknown and tied metadata neutral.
* test(sync): assert options arg on split-file processRemoteOps
The split-file snapshot path now routes post-snapshot ops through
_processRemoteOpsWithStartupCleanup, which calls processRemoteOps with an
(empty) options object. Update the assertion to match the 2-arg call so the
unit suite passes.
* refactor(sync): dedupe strong-ETag regex, document snapshot-op boundary
- Extract the duplicated RFC 7232 strong-entity-tag pattern into a single
STRONG_ETAG_RE constant with a comment noting why the char class is safe to
interpolate into an If-Match header (no CR/LF header splitting).
- Explain why sv === undefined ops are classified as snapshot-included: they are
legacy migration ops fully contained in the snapshot, and _validateSnapshotRef
enforces a clock-EQUAL boundary. No behavior change.
* 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): prevent destructive full-state sync races
Use causal ordering and server-sequence preconditions for full-state operations, surface snapshot rejection, deduplicate migrations, and retain queued WebSocket downloads.\n\nFixes #8959
* fix(sync): block uploads after rejected imports
Keep incremental operations behind a durable barrier when a local import or restore is rejected. Release the barrier only after a newer full-state snapshot is accepted, and surface the blocked state instead of reporting sync success.
* fix(sync): harden causal repair recovery
Persist repair base cursors across the client, provider, and server so stale snapshots are rejected before quota or history mutation and legacy repairs cannot poison restore state.
Atomically rebase stale repairs after downloading the missing suffix, preserve rejected-import barriers and WebSocket watermarks, and cover PostgreSQL serialization.
* fix(sync): harden repair recovery edge cases
Block dependent uploads after any rejected full-state operation, reset negotiated capabilities across provider config changes, and bound WebSocket download retries.
Update sync test doubles for causal repairs and the expanded duplicate-operation identity.
* fix(sync): honor sync import conflict outcomes
Propagate nested conflict cancellation through rejected-op handling so it cannot trigger automatic merges or consume the retry budget.
Report force-local resolution failures when the clean-slate upload is blocked, rejected, or accepts no operations.
* fix(sync): harden force overwrite recovery
Verify the exact force-upload operation, preserve clean-slate retries, and roll back rejected replacements. Keep unresolved work retryable without reporting a successful sync.
* test(sync): cover clean-slate rollback in postgres
* fix(sync): make task replay values deterministic
Capture logical dates and timestamps in task actions and backfill legacy operations. Carry per-day task totals so own-operation replay is idempotent while foreign time remains additive.
Fixes#8957
* fix(sync): make time snapshots replay-safe
Exclude pending task-time batches from op-log and file-sync snapshots so their later additive operations cannot overlap snapshot state. Preserve concurrent direct credits and normalize legacy replay dates deterministically.
Fixes#8957
* fix(sync): align snapshots with queued task time
Exclude accumulator and in-flight task-time deltas from operation-log snapshots so later delta operations cannot double-count them. Capture file and direct-upload snapshots at the same operation-log boundary, and flush or clear queued time around destructive state replacement.
* fix(tasks): flush queued time at task boundaries
Persist queued timer deltas before absolute short-syntax edits, and clear them whenever project, schedule, or repeat cleanup deletes the owning task. This prevents stale batches from recreating or overwriting removed task time.
* fix(sync): preserve concurrent task time deltas
Treat concurrent task-time batches as commuting updates on both client and server, while retaining causal stale-operation checks. Reject malformed identities, dates, durations, and unsafe timestamps before replay or persistence.
* test(sync): cover task time snapshot replay
Exercise seeded snapshot and restart invariants plus a real three-client SuperSync convergence path with the initial time inside the snapshot.
* fix(sync): select action type for legacy conflicts
* 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
* fix(sync): preserve offline operations and harden auth
Accept operations from long-offline clients without rewriting their timestamps, and retain a replayable full-state base during cleanup. Neutralize registration account discovery and suppress repeated login and recovery token emails with atomic claims.
Fixes#8961
* fix(sync): harden auth and retention edge cases
Make unauthenticated auth responses neutral across delivery and resend failures, isolate WebAuthn ceremonies, and consume login and recovery tokens atomically. Add monitored handling for histories without a replay base and strengthen regression coverage.
Refs #8961
* perf(sync): reduce cleanup replay-base queries
Reuse the maintained full-state sequence marker during retention cleanup while preserving a query fallback for legacy and stale-marker rows. Clarify no-base warnings and strengthen exact auth-response and recovery transaction tests.
Refs #8961
* fix(sync): reject non-integer op timestamps before BigInt persistence
A non-integer or non-finite client timestamp passed the schema
(timestamp is z.number(), not .int()) and threw at BigInt() during
upload, aborting the whole batch as an unstructured 500. Reject it in
validateOp as a per-op INVALID_TIMESTAMP instead. Op age stays
unbounded so long-offline backlogs are still accepted.
Refs #8961
- 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).
The misc-to-tasks migration spread the transformed legacy misc values
over an already-populated tasks section, so a stale v1 misc copy could
overwrite settings a v2 client had since changed. Flip the merge order
(existing tasks section wins) and drop the already-migrated early
return, which made the outcome depend on which duplicate arrived first.
- A request repeating one operation id no longer double-charges quota or
wedges the batch: the first occurrence reserves the id and later
siblings are terminally rejected (DUPLICATE_OPERATION for an exact
retry, INVALID_OP_ID otherwise), in both batch and serial paths.
- Clean-slate snapshot uploads become durably idempotent: the request
cache is process-local and expiring, so the client-supplied opId is
now required (contract superRefine) and checked against the stored
operation inside the per-user lock before any data is deleted. An
exact retry returns the original serverSeq; a colliding opId is
rejected without touching existing data.
- Audit logging validates id/entityType/opType/clientId against the
known-safe charsets before embedding them in log lines, and the six
duplicated reject-audit blocks collapse into one rejectedUploadResult
helper; the ops handler logs rejected error codes instead of whole
operation objects.
Two conflict-detection blind spots let concurrent edits slip through
without a conflict:
- An op carrying both a scalar entityId and an entityIds array only
checked the array; the scalar and array sets are now unioned for
detection while getStoredEntityIds keeps the historical storage
normalization (scalar in entity_id, arrays in entity_ids).
- Histories written before schema v2 keep migrated task settings under
GLOBAL_CONFIG:misc. Incoming GLOBAL_CONFIG:tasks writes now consult
the legacy misc row as an alias (newest of the two wins, compared by
serverSeq), and legacy misc ops check the tasks key per-entity. Works
for encrypted payloads since only row metadata is consulted.
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.
- 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.
* 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(sync): harden file-based .bak recovery and split gap detection
Post-merge review of the SPAP-8/9/10/11 series found five defects in the
file-based sync adapter; all fixed here with regression tests proven
red/green against the previous code. Design cross-checked by a 7-reviewer
multi-agent pass; its findings are folded in.
- Split gap detection suppressed a syncVersion reset when the remote
clock was EQUAL OR GREATER_THAN the last-seen clock — the exact bug the
SPAP-9 review follow-up removed from the single-file path. A dominating
client's snapshot reset (which compacts ops this client never saw) was
treated as cosmetic, skipping snapshot hydration and silently
diverging. Now EQUAL only, matching the single-file path.
- .bak recovery staged the CORRUPT primary's rev, which setLastServerSeq
then promoted to _lastSeenRevs: every later poll's SPAP-10 pre-check
read "unchanged" and skipped the re-download while the upload path (no
.bak recovery) kept failing on the corrupt primary — sync wedged until
another client rewrote the file. A recovery download now never
stages/promotes the rev (each poll re-recovers and re-seeds the heal
cache), and every site that rewrites _lastSeenRevs drops any stale
staged rev via the shared _commitLastSeenRev.
- Snapshot uploads (force-upload / "Use Local" / E2EE re-encryption) left
the pre-snapshot .bak behind. After a password rotation the stale
old-key .bak was silently "recovered" by a still-old-key client
(suppressing its wrong-password prompt) and heal-uploaded back over the
new-key primary, reverting the rotation. Snapshot uploads now write the
same payload to .bak FIRST, then the primary (_forceUploadWithBakFirst;
deliberately FATAL — aborting pre-primary leaves the remote consistent
for a retry). Applies to sync-data.json.bak and sync-ops.json.bak;
sync-state.json.bak is deliberately exempt: its adoption is
ref-validated (EQUAL clock vs snapshotRef), so a stale copy is inert,
and it must keep serving the compaction crash window it exists for.
- Recovery additionally refuses a PLAINTEXT .bak when encryption is
expected: decoding trusts the file's own prefix flags, so a plaintext
.bak decodes even under a wrong/rotated key — the same
wrong-password-suppression class via mode (rather than key) mismatch.
- The split migration wrote the v3 tombstone BEFORE neutralizing the
legacy .bak (best-effort): a crash between the writes left a live v2
.bak that an OFF client's recovery would resurrect over the tombstone,
forking the folder. Neutralize-first, fatal. Residual: a step-3 failure
after the migration's ops-file commit leaves a live v2 file until the
next snapshot upload (documented at the call site).
- sync-ops.json — the hot file, rewritten on every op-bearing sync — had
no backup at all, so a torn write wedged split sync until a manual
force-upload. It now gets the same backup-before-overwrite + recovery +
heal treatment as the single-file format. deleteAllData deletes the
split files BEFORE the tombstone and treats source-of-truth deletion
failures as errors (success:false) — it previously left every split
file behind and reported success.
The three backup/recover pairs are collapsed into shared _writeBakFile /
_readBakFile helpers (the EQUAL||GREATER_THAN divergence above is exactly
the copy-drift failure mode this prevents); .bak file names move into
FILE_BASED_SYNC_CONSTANTS as remote-format surface.
* fix(sync): show the exact pending-op count in the conflict dialog
Compaction can fold still-unsynced ops into the snapshot baseline clock,
so the dialog's vector-clock delta could report "0 changes" right next to
"N local changes pending" — and the false 0 skipped the secondary
USE_REMOTE overwrite confirmation. The dialog now prefers the EXACT
pending-op count carried on LocalDataConflictError (new optional
ConflictData.localUnsyncedOpsCount): it is precisely "what USE_REMOTE
would discard", so both the displayed count and the >= 20-difference
confirmation threshold work from a truthful figure; the clock delta
remains the fallback when no measured count is supplied.
Display/confirmation-only — no clock or op-log semantics touched. Also
asserts the explicit-null lastSyncedVectorClock contract at the
fresh-client conflict throw sites (test gap from SPAP-7).
* chore(lint): enforce tx-handle-only access in op-log transactions
The SQLite op-log adapter serializes every entry point through a
per-connection FIFO queue; awaiting an adapter method inside a
.transaction() callback enqueues behind the transaction's own slot and
silently deadlocks all op-log persistence. The port contract documents
the precondition and #8849 promised a lint rule — this adds it
(no-adapter-in-tx, scoped to src/app/op-log) with RuleTester specs.
Matching is rename-proof: it flags access on the SAME receiver the
transaction was opened on (plain identifiers and any `this.<field>`), so
it does not depend on the field being named `_adapter`; known heuristic
gaps (extracted callbacks, aliasing, method indirection) are documented.
Also corrects the port doc: IndexedDB serializes only overlapping-scope
transactions; SQLite provides the stronger whole-connection exclusion.
* feat(sync): SPAP-11 opt-in split-file sync format for O(delta) syncs
Splits the single sync-data.json into a small always-read/written sync-ops.json
(the commit point) plus a sync-state.json snapshot rewritten only on
compaction / force-upload / gap-repair / migration, so a normal sync transfers
O(delta) instead of O(dataset). Opt-in via the isUseSplitSyncFiles setting; the
legacy file is left as a v3 tombstone so old clients hard-stop rather than
silently diverge.
Review follow-ups:
- Recompaction triggers at combinedOps > MAX_RECENT_OPS (2000) and trims to
SPLIT_COMPACTION_THRESHOLD (1000), so it no longer rebuilds the snapshot on
every op-bearing sync once the folder crosses 1000 (test b2).
- Split download stages the ops-file rev to _pendingRevs (not _lastSeenRevs),
promoted only in setLastServerSeq after durable apply — mirrors the single-file
crash-safety path (test g).
- Split rev-precheck is gated on sinceSeq > 0, so a forceFromSeq0 download
(USE_REMOTE / rehydrate) always fetches ops + snapshot (test h).
Stacked on SPAP-9 (#8787, merged) and SPAP-10 (#8795): the split path reuses
their per-provider causality (_lastSeenVectorClocks / compareVectorClocks) and
rev (_lastSeenRevs / _pendingRevs) infrastructure.
SPAP-11
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sync): return whole file-based ops buffer per download page
The file-based download paths (single-file _downloadOps and split-format
_downloadOpsSplit) truncated recentOps to the caller's page size
(DOWNLOAD_PAGE_SIZE = 500) via slice(0, limit), but they return ops by array
index and ignore sinceSeq. The caller (operation-log-download.service) loops on
`hasMore`, advancing sinceSeq by the returned index-based serverSeq — so when the
buffer exceeds the page size the adapter kept re-returning the SAME oldest slice.
A behind client never received its newest ops and the loop spun until it bailed
(MAX_DOWNLOAD_ITERATIONS / in-memory cap), leaving the sync stuck (success:false)
and unable to converge.
This is the common case for the split format, whose ops buffer floor is
SPLIT_COMPACTION_THRESHOLD (1000) — always above DOWNLOAD_PAGE_SIZE. The
single-file path shares it, latent since MAX_RECENT_OPS was raised 500 -> 2000
(before, buffer == page, so hasMore never tripped).
File-based providers have no server-side cursor (they re-download the whole file
each call), so cross-call pagination cannot advance and truncation just drops the
newest ops. The recentOps buffer is already bounded by MAX_RECENT_OPS, so return
it WHOLE with hasMore=false and let the caller's appliedOpIds dedup decide what is
actually new. Returning everything (rather than slicing to a cap) also converges
safely if a future higher-MAX_RECENT_OPS client writes an over-cap buffer, instead
of re-entering the same stranding loop.
Documents the `limit` divergence on the OperationSyncCapable.downloadOps
interface. Regression tests cover a 600-op buffer with page size 500 on both the
split and single-file paths (newest ops delivered, hasMore=false).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(sync): drop redundant recentOps-length clause from gap detection (SPAP-33)
The file-based gap detector treated a trimming gap as real only when the ops
buffer was also at its cap:
oldestOpSyncVersion > sinceSeq + 1 && recentOps.length >= <cap>
The length clause is redundant for correctness — syncVersion is contiguous and
every bump carries at least one op, so oldestOpSyncVersion > sinceSeq + 1 already
proves the op at sinceSeq+1 existed and was trimmed away, and it never
false-positives. But the clause caused a false NEGATIVE: a buffer trimmed at a
SMALLER floor than the current cap — a legacy buffer written by an old client
with a lower MAX_RECENT_OPS, or (in the split format) a buffer trimmed to
SPLIT_COMPACTION_THRESHOLD — has a genuine gap while holding fewer ops than the
cap, so the gap was suppressed and the behind client applied ops without the
snapshot base and silently diverged.
Drop the clause in both the single-file (>= MAX_RECENT_OPS) and split
(>= SPLIT_COMPACTION_THRESHOLD) paths, so a behind client correctly falls back to
the snapshot. Updates the single-file test that asserted the suppressed behavior
and adds a split-path regression (short trimmed buffer -> gap + snapshot load).
Previously deferred as SPAP-33; pulled in as it prevents silent cross-client
divergence and sits in the same download-correctness path as the pagination fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Johannes Millan <johannes.millan@gmail.com>
Use vector clocks to resolve file-based sync conflicts by causality instead
of always surfacing the binary USE_LOCAL/USE_REMOTE dialog:
- keep-local when local strictly dominates the remote snapshot
- apply-snapshot (no dialog) when the snapshot strictly dominates local
- dialog when clocks are concurrent and can't be auto-resolved
Cosmetic syncVersion-reset suppression is gated on the last-seen vector
clock and treated as cosmetic ONLY when the clocks are EQUAL. A GREATER_THAN
remote clock proves the writer did more work, not that this client received
the intervening ops (a snapshot can compact ops we never saw), so it now
conservatively triggers a seq-0 resync rather than being silently skipped.
CONCURRENT-snapshot auto-merge defaults OFF and, when enabled, only merges
if the retained recent ops provably bridge the whole gap to the snapshot
clock (local ⊔ recentOps ⊒ snapshot); otherwise it falls back to the
user-recoverable dialog instead of replaying only recentOps and silently
dropping compacted-only entities. Re-enabling default-on is gated on
multi-client E2E validation (SPAP-34).
SPAP-9
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(plugin): add generic request capability to PluginAPI
* feat(plugin): gate PluginAPI.request behind manifest allowedHosts
PluginAPI.request is otherwise reachable to any host the shared SSRF
filter does not block. Add a manifest-declared, host-enforced allowlist
so a plugin's outbound reach is explicit and reviewable at install.
- PluginManifest.allowedHosts: exact hostnames the plugin may reach.
- PluginBridgeService enforces it before the shared HTTP/SSRF layer:
exact host, case-insensitive, trailing-dot tolerant, port-agnostic;
userinfo tricks resolved via URL parsing; fail-closed (empty/omitted
allowedHosts disables request entirely).
- validatePluginManifest rejects malformed allowedHosts at install.
Companion tests: bridge enforcement (reject non-declared host,
fail-closed on undefined/empty, case-insensitive + trailing-dot,
userinfo-trick blocked) and manifest validation.
* style(plugin): fix prettier formatting flagged by CI in request API
The generic-request commit slipped past checkFile on three files; CI
prettier flagged the multi-line request() signature and a long spec
assertion string. Formatting only, no behavior change.
* feat(plugin): require "http" permission for PluginAPI.request
Network egress becomes an explicit, opt-in capability (like nodeExecution):
request now requires "http" in the manifest permissions in addition to a
matching allowedHosts entry. Missing either is fail-closed. Enforced
host-side (before the shared SSRF layer) on both the iframe and
Function-sandbox paths; the "http" permission gets a human-readable line in
the plugin security info.
Tests: fail-closed without the "http" permission.
* feat(plugin): surface plugin network reach in the plugin-management UI
Render the manifest allowedHosts as a chip-set beside permissions/hooks
(with a count in the collapsible title), so a plugin's outbound network
reach is reviewable in-app instead of only in the raw manifest.
Tests: allowedHosts shown in the collapsible title, omitted when none.
* feat(plugin): block redirects on PluginAPI.request (SSRF-via-redirect)
Only the initial request URL is allowlist/SSRF-checked, but HttpClient/XHR
auto-follows 3xx — so a declared host could 302 to a private/metadata IP and
return internal content to the plugin. Execute the request path via fetch with
redirect:"error" so any redirect is refused instead of chased.
- New PluginHttpHelperOpts.blockRedirects (default false); PluginBridge sets it
for request. Issue-provider HTTP is untouched (still HttpClient).
- fetch executor preserves the HttpClient contract callers depend on: query
params, timeout via AbortController (covering the body read, not just headers),
text/json parsing, and non-2xx rejecting with .status + .error.
- Only plain objects/arrays are JSON-serialized; FormData/Blob/URLSearchParams/
ArrayBuffer(View) pass through, and Content-Type: application/json is added
only for the JSON case (HttpClient parity).
- Web/desktop only: on native, Capacitor patches fetch and ignores
redirect/signal, so native falls back to HttpClient (documented limitation).
- Blocks all redirects incl. benign same-host ones; manual per-hop re-validation
is not possible on the web (cross-origin Location is opaque). Documented.
Tests: redirect refused, .status/.error parity, SSRF still pre-checked,
body pass-through, params + responseType:text, real-timer timeout, and native
falls back to HttpClient.
* fix(plugin): gate allowedHosts UI on the "http" capability
The plugin-management panel showed a "Network access" section (and title count)
whenever allowedHosts was non-empty, regardless of permissions — advertising
reach for a capability the bridge rejects without the "http" permission. Gate the
section, the chip loop, and the title count on a getNetworkReachHosts() helper
(hosts only when permissions includes "http"). Addresses review on #8721.
* docs(plugin): note _requestNoRedirect bypasses NetworkRetryInterceptorService
The fetch path skips Angular HTTP_INTERCEPTORS, so PluginAPI.request GETs lose the
single status-0 retry the HttpClient paths keep. Inherent to redirect:"error"
(XHR/HttpClient cannot block redirects). Flagged in review on #8721.
File-based providers (WebDAV, Dropbox, OneDrive, Nextcloud, LocalFile)
derived "should I encrypt?" from key presence alone, so a silently
dropped credential-store key made the next sync upload the full
sync-data.json in plaintext with no error — same class as GHSA-9v8x,
which only covered SuperSync.
Root cause: there was no durable, per-provider record of encryption
intent. The global sync.isEncryptionEnabled flag is shared across
providers and re-derived from key presence in the settings form, so it
is unreliable in both directions (stale-true after a SuperSync→file
switch; flipped false by a routine save once the key is gone).
Fix — persist intent per provider, enforce at one boundary, recover
uniformly:
- Store isEncryptionEnabled in each file-based provider's privateCfg,
written atomically with the key on enable/disable (closes the
non-atomic disable window) and backfilled from the key present at
save time / first sync for pre-fix configs. Read everywhere as
`isEncryptionEnabled ?? !!encryptKey`.
- WrappedProviderService now derives the adapter's isEncrypt from this
per-provider intent, not the global flag — so a stale global flag no
longer blocks legitimate plaintext sync and privateCfg writes keep the
adapter cache correct.
- The upload service fails closed for file-based intent-on-but-key-gone
via a new adapter hook (isEncryptionKeyMissing), throwing
EncryptNoPasswordError BEFORE either upload loop — never plaintext,
and never the permanent markRejected the snapshot path would apply.
- SyncWrapperService routes EncryptNoPasswordError to the enter-password
recovery dialog on the main sync, force-upload and USE_LOCAL conflict
paths (previously only main sync; the others dead-ended in a snack).
- The encrypt handler keeps throwing on isEncrypt-without-key as a
last-line backstop.
Residual: a pre-fix config whose key is dropped before intent is ever
recorded (never synced/saved after upgrade) cannot be distinguished from
"never encrypted" and is not retroactively protected — no record exists.
Regression tests per layer: adapter hooks, upload-boundary guard,
per-provider intent persistence, form derivation/preservation, and
conflict-path recovery routing.
* fix(sync): never upload plaintext ops for E2EE-mandatory providers
SuperSync's first-time setup ran an initial sync (dialog-sync-cfg save ->
sync(true)) BEFORE the user chose an encryption password, so all local ops
(incl. issue-provider credentials) were uploaded to the server in cleartext.
Completing setup deleted them; aborting left them stored indefinitely, breaking
the E2EE promise (GHSA-9v8x-68pf-p5x7).
Add an optional `isEncryptionMandatory` capability to OperationSyncCapable
(true for SuperSync) and refuse to upload in the op-log upload path while no
usable key is configured. Downloads still run (merge-first) and the encryption-
enable flow performs the first, encrypted upload, so the setup flow and prompt
are unchanged. File-based providers, where unencrypted sync is a legitimate
user choice, leave the flag unset.
* fix(sync): fail closed on plaintext snapshot for E2EE-mandatory providers
Multi-review hardening for GHSA-9v8x-68pf-p5x7. The op-upload guard closes the
reported leak, but SnapshotUploadService.deleteAndReuploadWithNewEncryption could
still push a plaintext snapshot for SuperSync on two adjacent paths: the (today
UI-unreachable) disable-encryption flow, and a keyless import declaring
isEncryptionEnabled:true. Reject an unencrypted snapshot for an encryption-
mandatory provider before any destructive deleteAllData, so the "never transmit
plaintext" invariant holds regardless of caller.
Also lower the mandatory-encryption upload-skip log from warn to normal: it is an
expected by-design skip during the pre-encryption setup window and would
otherwise fire on every auto-sync cycle.
* fix(sync): consolidate op-log into the encryption-enable snapshot
Follow-up to the GHSA-9v8x-68pf-p5x7 upload guard. With the guard, first-time
SuperSync setup leaves the whole local history unsynced until encryption is
enabled; the enable-snapshot then represents that full state, but the ops were
re-uploaded incrementally on top of it on the next sync (redundant server op-log
bloat, and previously untested at whole-history volume).
deleteAndReuploadWithNewEncryption now captures the pending ops the snapshot
subsumes (before the destructive delete, under runWithSyncBlocked + a modal
dialog so the set is stable) and marks them synced after a successful upload —
mirroring planRegularOpsAfterFullStateUpload in the op-log upload path, which
this direct snapshot upload bypasses. Also fixes the same latent redundancy in
the enable-from-settings-with-pending-ops flow.
Adds a multi-client e2e: local task history exists before first-time encrypted
setup, then a second client with the same password receives exactly those tasks
with no duplicates, conflicts, or errors.
* ci(e2e): add optional grep input to manual SuperSync e2e dispatch
* fix(sync): capture subsumed ops before state snapshot to prevent mark-synced data loss
deleteAndReuploadWithNewEncryption captured getUnsynced() AFTER the full-state
snapshot, so an op created in that window was marked synced yet absent from the
snapshot — silently lost. Capture the unsynced set first: every marked-synced op
is then guaranteed present in the snapshot, and a concurrent op arriving after
the capture is left unsynced and re-uploaded next sync (idempotent by op id).
* test(sync): mock WebCrypto so mandatory-encryption guard test reaches the guard
The 'enabling without a usable key' case passed isEncryptionEnabled: true but did
not mock WebCrypto, so the availability check threw WebCryptoNotAvailableError in
non-secure CI before the /unencrypted snapshot/ guard under test.
* feat(plugins): enable OAuth-based issue-provider plugins
Generic, provider-agnostic plugin-framework hooks so an issue-provider plugin
that needs an exact OAuth redirect can work without any built-in code:
- OAuthFlowConfig.redirectUri: a plugin may declare an exact pre-registered
callback; the host uses it for both the authorize request and token exchange.
- The Electron loopback honors a plugin-requested fixed port and rejects with a
clear message when that port is already in use.
- Apply user-supplied clientId/clientSecret/redirectUri overrides onto a
plugin's oauthConfig (bring-your-own OAuth app).
- Fix: merging a partial pluginConfig update no longer drops omitted keys and
deep-merges nested objects (e.g. twoWaySync).
Split out of the Basecamp community-plugin work per PR #8507 feedback; contains
no provider-specific code.
* fix(plugins): address #8546 review
- validate OAuthFlowConfig.redirectUri per platform (loopback / same-origin /
app scheme) and fail fast instead of hanging; restrict desktop loopback to 127.0.0.1
- warn when a client secret is dropped on web/native (bring-your-own credentials)
- validate the IPC loopback port to [1024,65535], register the error handler before
listen(), and close the failed server
- merge pluginConfig via generic recursion instead of a hardcoded twoWaySync case
- nits: named token-store imports; fix stale prepareRedirectUri comment
- tests: redirectUri validation, the web client-secret warning, and generic merge
* fix(plugins): address #8546 round-2 review
- loopback error handler calls cleanupServer() so a post-listen runtime error
doesn't leave the server ref / 5-min timer dangling
- share OAUTH_LOOPBACK_PORT_{MIN,MAX} between the renderer and Electron main so
the bounds never drift; reject out-of-range (incl. 0/80/443) redirect ports early
- drop _getElectronLoopbackPort and parse the already-validated redirectUri once
- document that a bring-your-own clientSecret syncs via pluginConfig (override boundary)
- skip __proto__/constructor/prototype keys in the pluginConfig merge (defense-in-depth)
- test: prototype-pollution guard
* fix(plugins): address #8546 round-3 review
- reject native redirectUri overrides outright (closes CodeQL
js/incomplete-url-scheme-check) via a pure, per-platform validateOAuthRedirectUri
util (electron loopback / native reject / web same-origin)
- gate bring-your-own OAuth credentials to the desktop loopback flow and warn
(instead of silently dropping clientId) when set on web/native
- namespace BYO under pluginConfig.oauthOverrides (was flat keys); document the
convention on OAuthFlowConfig (public plugin API)
- shallow top-level pluginConfig merge: drop the deep recursion + proto guard;
nested objects (e.g. twoWaySync) are replaced wholesale, matching callers
- pin the web redirect to /assets/oauth-callback.html via a shared constant so a
same-origin wrong-path URI fails fast; note the desktop 127.0.0.1-only rule
- companion tests for each
* fix(plugins): strip desktop redirectUri on web/native OAuth flows
A plugin-declared redirectUri is the desktop loopback override; keeping it on
the web/native branches made a web/native-capable plugin throw at connect time
(the loopback URI fails web/native redirectUri validation). Strip it on those
branches so prepareRedirectUri falls through to the platform default. Document
redirectUri as desktop-only in the plugin API and fix a misleading test name.
* refactor(plugins): extract resolveEffectiveOAuthConfig and harden native fallthrough
Move the platform client/secret/redirectUri selection out of the bridge into a
pure, parameterized util so every branch is unit-testable (the IS_* platform
consts are module-level and cannot be mocked in karma). Also strip clientSecret
and redirectUri on the native fall-through — a native platform where the plugin
ships no matching client id — keeping both strictly desktop-only.
Optional hardening on top of the redirectUri fix; safe to drop independently.
* feat(plugins): add local-only secret storage API for plugins
Add setSecret/getSecret/deleteSecret to the plugin API, backed by a
dedicated 'sup-plugin-secrets' IndexedDB that is never part of Super
Productivity's sync, exports, or backups (mirrors the existing OAuth
token store). Secrets are namespaced per plugin and purged on uninstall.
Unblocks credential-using plugins (e.g. IMAP mailbox -> task) that must
not put passwords in persistDataSynced or synced issue-provider config.
Also purge plugin OAuth tokens on uninstall (best-effort, alongside the
secret purge) — they previously leaked past uninstall.
Refs #7511
* fix(plugins): purge plugin secrets and OAuth tokens on cache clear
clearUploadedPluginsFromMemory (the 'Clear plugin cache' action) wiped
plugin code and persisted nodeExecution consent (#8512 Phase 2) but left
secrets and OAuth tokens in their dedicated stores. A same-id re-upload
after a cache clear has no existingState, so the re-upload purge never
fires and the new plugin could inherit the previous plugin's credentials
— the same id-reuse gap #8512 closed for consent. Purge both here too,
best-effort and idempotent, mirroring the per-plugin uninstall purge.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* fix(sync): verify file-based upload size to catch truncated writes (#8604)
Dropbox and OneDrive enforce no end-to-end integrity, so a truncated upload
(a cut-short body silently accepted) lands a partial gzip/JSON that fails to
decode on every later download until the file is deleted. The #7300 post-upload
verification was WebDAV-only.
Add a shared assertUploadedSizeMatches() helper: compare the stored byte size
(already in the upload response, no extra request) against the bytes sent, but
only for pure-ASCII payloads so the comparison is transport-encoding independent
— avoiding false positives on the native CapacitorHttp path, where a wrong
byte-count assumption would silently re-upload every cycle. Compressed/encrypted
payloads are base64 (ASCII), covering the reported case. On mismatch raise
UploadRevToMatchMismatchAPIError, the error the upload path already surfaces.
This detects and fails the sync loudly; it does not repair the remote.
Wired into Dropbox and OneDrive uploads. WebDAV keeps its stronger content-hash
re-read check.
* perf(sync): detect ASCII without encoding in upload-size check (#8604)
The upload-size guard called TextEncoder.encode(data).length purely to detect
whether the payload is pure-ASCII (byte count === char count), allocating a
full multi-MB Uint8Array on every Dropbox/OneDrive upload — including the
non-ASCII skip path, which is the default-config common case. Detect non-ASCII
with a regex instead and use data.length directly; behaviour is identical
(byteLen === data.length iff pure ASCII) with no allocation and an early exit on
the first non-ASCII unit.
Also add OneDrive upload-size wiring tests (match + ASCII truncation), which the
app-side spec previously exercised only on the skip path.
Surfaced by a second multi-agent review.
