Commit graph

10 commits

Author SHA1 Message Date
Johannes Millan
011a92585d
docs(sync): add sync simplification roadmap (#9062)
* docs(sync): add simplification roadmap

* docs(sync): strengthen simplification roadmap

* docs(sync): simplify sync implementation plan

Prioritize deletion, atomic single-tab ownership, and full-sync trigger consolidation while deferring compatibility removals and speculative abstractions.

* docs(sync): make simplification roadmap compatibility-safe

Narrow deletion work behind persisted-data, provider-eligibility, delivery, and request-cost gates. Keep schema readers and separate unrelated startup and maintenance correctness projects.

* docs(sync): fold code-verified review findings into plan

* docs(sync): align simplification plan with latest changes

* docs(sync): correct trigger-branch claim and add footprint estimate

* docs: cap services at 1000 LOC

* chore(lint): warn when a service exceeds 1000 lines

* chore(lint): raise service size cap to 1200 lines

* docs(sync): re-align simplification plan to master 6fefd741c5

* docs(sync): fold #9054 footprint auth into plan contracts

Master advanced 3 commits past the plan's 6fefd741c5 baseline. Exactly
one touches a task surface: #9054 (authenticated LWW project-move
footprint, GHSA-8pxh) landed in conflict-resolution.service.ts — the
file Task 6 carves up.

Contract 14 documented only #9045's decrypt-path check and emphasised
the weaker half ("synthetic ops are intentionally exempt; that exemption
must survive the disjoint-merge removal"). Read alone, that could bless
an op.entityIds read while removing disjoint-merge, reopening the vector
one merge removed. #9045's exemption sentence is still accurate at
master, so 14 was incomplete rather than wrong.

Name the invariant #9054 established: getTaskProjectMoveEntityIds
re-derives footprints from the authenticated payload.projectMoveFootprint
(LWW) / actionPayload.projectMoveSubTaskIds + actionPayload.task.id (raw
TASK_SHARED_UPDATE), never the plaintext envelope, and that one choke
point serves the disjoint-merge, local-win and superseded-op callers.

Also fence #9054 in §4 and re-align the baseline to 7ef7e69e96 (#9052
orphan-task navigation and #9056 locale touch no task surface).

* chore(lint): make the service size cap enforce on new services

'warn' could not enforce what AGENTS.md asserts. `ng lint` runs the
@angular-eslint builder, whose maxWarnings defaults to -1 and is not
overridden in angular.json, so warnings never fail a build: a new
1300-line service merged green. The documented flip-to-'error' gate
("once every *.service.ts is under the cap") needed
conflict-resolution.service.ts to go 3934 -> 1200, i.e. the whole
7-task simplification plan, so it would realistically never flip.

Ratchet instead: 'error' on **/*.service.ts, with the 8 pre-existing
offenders grandfathered to non-failing warnings at their real size, so
the debt stays visible and the list can only shrink.

Verified: a new over-cap service errors (exit 1); all 8 grandfathered
warn (exit 0); full `npm run lint:ts` is exit 0 with exactly 8 warnings
and 0 errors, which proves the list is complete — a missed offender
would fail the build. Every >1200-line @Injectable in the repo is named
*.service.ts, so the glob has no coverage hole, and .service.spec.ts is
exempt (checked against the 7797-line conflict-resolution spec).

Also correct AGENTS.md "1200 LOC": max-lines defaults to
skipComments/skipBlankLines false, so the cap counts physical lines.
2026-07-16 13:42:38 +02:00
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
Johannes Millan
cbb4e28ec4
build(lint): enforce Log helpers over console.* in src/app (#8239)
* build(lint): enforce Log helpers over console.* in src/app

Adds an ESLint no-console rule scoped to src/app (exempting log.ts,
*.spec.ts, and *.benchmark.ts) and migrates the remaining console.*
call sites to use Log / TaskLog / SyncLog / IssueLog / PluginLog so
their output is captured in the exportable log history users attach to
bug reports.

* test: spy on Log helpers instead of console after no-console migration

* fix(trello): keep username out of exportable log history

* test: derive today expectation from mocked DateService date
2026-06-10 13:08:17 +02:00
Johannes Millan
42e6626b76 docs(sync): consolidate sync docs + enforce the contributor model
Collapse the sprawling, partly-stale docs/sync-and-op-log/ tree into a
small authoritative set and make the sync-correctness invariant
partly lint-enforced instead of convention-only.

Docs:
- Delete superseded/duplicate/provably-stale design, plan, and
  background-research docs (quick-reference, the architecture-diagrams
  monolith, the "Hybrid Manifest" docs describing code that does not
  exist, completed long-term plans, LLM-synthesis analyses).
- Salvage load-bearing decision history into the surviving docs before
  deletion: rejected-alternatives rationale -> operation-log-architecture
  ("Why this architecture"); vector-clock pruning incident history ->
  vector-clocks.md; archive-payload optimization -> architecture E.7.
- Add contributor-sync-model.md as the single-invariant entry point
  (one user intent = one op; replayed/remote ops must not re-trigger
  effects), with a decision table mapping to the enforcing linters.
- Repoint external/internal cross-refs; add CONTRIBUTING.md + CLAUDE.md
  pointers; record the migration in a dated docs/plans/ design doc.

Enforcement (new eslint-local-rules):
- no-actions-in-effects (error): effects must inject LOCAL_ACTIONS /
  ALL_ACTIONS, never the raw @ngrx/effects Actions stream.
- no-multi-entity-effect (warn, heuristic): flags a literal returned
  array of >=2 action-creator calls; docstring + valid-case specs pin
  exactly which shapes are and are not detected.
- run-specs.js runner wired into `npm run lint` via test:lint-rules;
  refuses to run under test-framework globals and counts RuleTester.run
  invocations so a spec that asserts nothing fails instead of passing.
- Correct the ALL_ACTIONS JSDoc in local-actions.token.ts to match
  reality (archive-operation-handler uses LOCAL_ACTIONS).

Reviewed via parallel multi-agent review; findings W1/W2/W4 and a
dangling doc anchor addressed.
2026-05-15 16:51:50 +02:00
Johannes Millan
5fa8260bb8 refactor(sync): finish package extraction polish 2026-05-13 14:11:15 +02:00
Johannes Millan
fb940c1d4a Merge branch 'master' into feat/our-latest-super-sync-server-update
Resolves conflicts in supersync after master's parallel work:
- Schema: adopt master's 4-col entity_sequence_index + partial restore-point
  index migration. Drops our out-of-band index hack — master's migration runs
  CREATE INDEX CONCURRENTLY directly.
- deploy.sh: take master's simpler 169-line version; keep our hardening
  (set -euo pipefail, guarded ${VAR:-}, diagnostic pipeline || true).
- snapshot.service.ts: keep our EncryptedOpsNotSupportedError, cache stale-
  overwrite guard (updateMany+create+P2002), invalidate corrupt cache,
  async gzip helpers, _resolveExpectedFirstSeq leading-gap acceptance,
  full-state reset semantics, Buffer.byteLength replay size check. Adopt
  master's REPLAY_OPERATION_SELECT, ReplayOperationRow type, and
  assertContiguousReplayBatch module-level helper.
- compressed-body-parser.ts: keep our invalid-json reason, single base64
  decode, and shared gzip.ts helper. Adopt master's improved
  isDecompressedPayloadTooLargeError check (covers 'Cannot create a Buffer
  larger than').
- operation-download.service.ts: take master's latestSeq==0 fast-path and
  conditional minSeq aggregate.
- local-rest-api-handler.service.ts: take master's shared
  isTaskInToday + isTodayWithOffset (offset-aware) over our private method.
- tests: align spec mocks with upsert + cacheSnapshotIfReplayable +
  REPLAY_OPERATION_SELECT, take master's migration-sql.spec.ts.

587/587 supersync tests pass; checkFile clean on all touched .ts files.
2026-05-12 20:40:26 +02:00
Johannes Millan
a97a15457b refactor(sync-providers): scaffold provider package 2026-05-12 19:14:12 +02:00
Johannes Millan
b56c997874 refactor(sync): add sync-core boundary contracts 2026-05-11 10:09:15 +02:00
Johannes Millan
19796204f3
Add hydration guard for selector-based NgRx effects (#6426)
* fix(sync): guard unprotected selector-based effects against sync replay

Add skipWhileApplyingRemoteOps() guards to selector-based effects that
were missing hydration protection, preventing unwanted side effects
during sync/hydration replay:

- tag.effects.ts: cleanupNullTasksForTaskList$ (hidden dispatch via
  tagService.updateTag inside tap)
- reminder-countdown.effects.ts: reminderCountdownBanner$ (banner flash)
- voice-reminder.effects.ts: dominaMode$ (unwanted TTS)
- task-ui.effects.ts: timeEstimateExceeded$ and
  timeEstimateExceededDismissBanner$ (unwanted notifications/banners)

Also enable the existing require-hydration-guard ESLint rule in
eslint.config.js (scoped to *.effects.ts files) to prevent future
regressions. The rule was already written but never wired into the
linting pipeline.

https://claude.ai/code/session_01WcAdr12nsvLdAjubLx1ZLf

* fix: address PR review feedback for selector effects guards

- Fix require-hydration-guard.spec.js for ESLint v9 (parserOptions → languageOptions)
- Upgrade require-hydration-guard severity from warn to error
- Restore require-entity-registry rule (also lost in ESLint v9 migration)
- Remove unrelated package-lock.json noise

https://claude.ai/code/session_01WcAdr12nsvLdAjubLx1ZLf

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-02-08 15:18:21 +01:00
Johannes Millan
e6da7ced37 chore(deps): upgrade ESLint to v9 with flat config
- Upgrade eslint from v8 to v9.39.2
- Upgrade typescript-eslint to v8.52.0 (unified package)
- Add angular-eslint package for flat config support
- Migrate from .eslintrc.json to eslint.config.js (flat config)
- Update package configs for sync-md, automations, boilerplate-solid-js
- Remove unused eslint-disable directives
- Fix lint errors (empty interface, template eqeqeq)

BREAKING: Requires Node.js 18.18+ (ESLint 9 requirement)
2026-01-10 16:08:11 +01:00