super-productivity/eslint-local-rules
Johannes Millan 610ca64894
fix(sync): harden file-based .bak recovery, split gap detection & conflict counts (#8857)
* 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.
2026-07-08 16:45:16 +02:00
..
rules fix(sync): harden file-based .bak recovery, split gap detection & conflict counts (#8857) 2026-07-08 16:45:16 +02:00
index.js fix(sync): harden file-based .bak recovery, split gap detection & conflict counts (#8857) 2026-07-08 16:45:16 +02:00
run-specs.js docs(sync): consolidate sync docs + enforce the contributor model 2026-05-15 16:51:50 +02:00