Commit graph

11 commits

Author SHA1 Message Date
Johannes Millan
57cee868bf
fix(locale): consolidate textLocale, fix planner month label, enforce via lint (#8987) (#9065)
* refactor(locale): collapse inlined textLocale copies into the helper

No behavior change: textLocale() is defined as isoTextLocale() ?? currentLocale(),
which is exactly what these five sites had inlined.

 #9056 was based on master and so could not use the helper #9055 adds — it
re-inlined the expression at focus-session, habit-tracker, worklog,
scheduled-list and scheduled-date-group. Now that both have landed, collapse
them so there is one canonical spelling instead of six.

Caller specs mock textLocale() directly rather than the isoTextLocale/
currentLocale pair, mirroring what each SUT actually calls.

* fix(locale): planner month label followed the browser locale, not the app's

monthLabel passed no locale to toLocaleDateString, so the spelled-out month
followed the *browser's* locale and ignored both the configured date locale
and the UI language: a German browser rendered 'Juli 2026' in an English app.
Same family as #8987 but reachable without the ISO option at all.

Route it through textLocale(), which is the UI language under the ISO option
and currentLocale() otherwise.

The two existing specs computed their expected value with the same undefined
locale, so they mirrored the bug and could never have caught it. They now pin
a fixed app locale, deliberately not the runner's browser locale — otherwise
they would pass either way.

* test(lint): add require-text-locale rule and enforce it over src/app

Guards the #8987 invariant that kept recurring: spelled-out weekday/month
names must be formatted with textLocale(), never currentLocale() (the ISO
option's 'sv' sentinel) and never the implicit browser locale. Three PRs
chased this bug class site-by-site because currentLocale() is the
obvious-looking default at every new call site.

- Resolves `const locale = ...currentLocale()` through the scope chain: that
  is the shape the original bug had in plannedStartDateStr, so a rule matching
  only direct calls would have missed the very bug it exists to prevent.
- Covers new Intl.DateTimeFormat() too — the same trap in constructor form,
  used at ~9 sites. Stays silent on clock times (hour/minute/dayPeriod), which
  must keep currentLocale() so the ISO 24h format survives.
- Specs excluded: computing an expected string against an explicit locale is a
  legitimate test technique; the invariant is about what the product renders.
- Documents its own blind spots (locale threaded through a parameter, reassigned
  variables, non-literal options) and pins them as valid cases, per the
  no-multi-entity-effect convention.

Error severity is safe: textLocale() equals currentLocale() for every non-ISO
option, so for spelled-out names it is never worse. Zero violations remain.

* fix(lint): keep require-text-locale silent on clock-time formats

The rule documented itself as staying silent on clock times, but `dayPeriod`
sat in ALWAYS_SPELLED_OUT and only `.toLocaleTimeString()` was excluded — so
`{ hour, minute, dayPeriod }` via `Intl.DateTimeFormat`/`toLocaleString` did
fire, and its message told the reader to switch to textLocale(). Following
that advice flips the ISO 24h clock to 12h: "13:05" -> "1:05 in the
afternoon". The same holds for any mixed date+time options object:
`{ weekday, hour }` goes from "onsdag 13:05" to "Wednesday 1:05 PM" — a
Swedish name traded for a broken clock, the exact ISO regression this rule
family exists to prevent.

A format that mixes a spelled-out name with a clock has no single correct
locale; it has to be split (names on textLocale(), clock on currentLocale(),
as plannedStartDateStr does), which is more than a one-locale message can
advise. So skip any options object containing `hour`, and say so. This costs
a blind spot on `{ weekday, hour }` — cheaper than confidently wrong advice
at `error` severity.

No call site changes: `dayPeriod`/`era` have zero uses in src/, so this was
latent. All four real bug shapes are still caught; src/app stays at zero
violations.

The two clock-time `valid` cases named dayPeriod in their comments but only
ever tested `{ hour, minute }` in their code, which is how this slipped
through — pin the actual shapes instead.

* fix(lint): catch dateStyle in require-text-locale

The rule missed `dateStyle` entirely, so the canonical #8987 shape walked
straight past it: `toLocaleDateString(currentLocale(), { dateStyle: 'full' })`
renders "onsdag 15 juli 2026" under the sentinel — a spelled-out weekday and
month — without naming weekday or month at all. Zero call sites today, so this
was latent, but guarding call sites that do not exist yet is the rule's whole
job.

`dateStyle` needs its own value set rather than month's: the two invert.
`month: 'short'` is "Jul" (spelled out) but `dateStyle: 'short'` is
"2026-07-15" (numeric), so reusing SPELLED_OUT_VALUES would have flagged
dateStyle:'short' and pushed the reader to route ISO's YYYY-MM-DD through
textLocale() — the mirror of the clock-time trap. Modelled as a per-field map
so the inversion is stated where it can't be conflated, and pinned from both
sides: 'short' as valid, 'full'/'medium'/'long' as invalid. Sabotage-verified —
swapping in month's value set fails the spec.

`timeStyle` joins `hour` as a clock-time field: `{ dateStyle, timeStyle }` is
the mixed date+time case again ("onsdag 15 juli 2026 kl. 13:05" -> "Wednesday,
July 15, 2026 at 1:05 PM"), and it carries no `hour` key for the existing guard
to catch.

Verified: 6/6 real bug shapes flagged, 0/6 false positives on correct usage,
src/app still at zero violations.
2026-07-16 22:33:23 +02:00
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