* 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.
* 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.
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.
- cross-env 7→10 (CLI-compatible, Node 20+ engine — already met)
- eslint 9→10 + @eslint/js 9→10 (all active plugins support v10;
fix local rule require-hydration-guard to use context.sourceCode
instead of removed context.getSourceCode())
- jasmine-core 5→6 (forbidDuplicateNames default flipped; no duplicates
in our suite — verified by running the full test:once)
- typia 11→12 (public API surface unchanged; validation specs pass)
Lint, build and the typia/auto-fix specs verified green. The 5 pre-existing
failures in immediate-upload.service.spec.ts are unrelated — also fail on
master with the same package.json that has been shipping.
Deferred from this round: stylelint 16→17 (paired migration with
@csstools/stylelint-formatter-github 1→2, separate work),
electron-dl 3→4 (forces full electron-main CJS→ESM migration),
marked 17→18 (ngx-markdown@21.2.0 peer-locks marked at ^17).
* 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>
- Change local-rules/require-hydration-guard from warn to error
- Change local-rules/require-entity-registry from warn to error
- Improve require-hydration-guard rule to skip effects with { dispatch: false }
since they only perform side effects (audio, UI) and never dispatch actions
- Add skipWhileApplyingRemoteOps() guard to autoShowOverlay$ effect
- Add skipWhileApplyingRemoteOps() guard to triggerIdleWhenEnabled$ effect
- Add eslint-disable comments to test files that intentionally test
unknown entity type handling
- Rename operator to be more descriptive of actual behavior
- Add deprecated alias for backwards compatibility
- Remove inner guards from effects - inject() only works during class
initialization, not inside switchMap callbacks at runtime
- Keep outer guards only (at createEffect level)
- Update ESLint rule to accept both names
- Add comprehensive unit tests for the operator (6 tests)
Key insight: The skipWhileApplyingRemoteOps() operator uses inject()
internally, which only works within Angular's injection context (during
class field initialization when createEffect() is called). Using it
inside switchMap callbacks causes NG0203 errors at runtime.
Also: currentTaskId$ is local UI state that doesn't sync between devices.
The operation log syncs entities (tasks, projects, tags), not UI state.
Therefore distinctUntilChanged() is sufficient for filtering spurious
emissions on inner observables.
Create central entity registry to eliminate scattered configuration
across 7+ files. This reduces technical debt and makes adding new
entity types easier.
Changes:
- Create entity-registry.ts with ENTITY_CONFIGS containing all 17
entity types with their adapters, selectors, and dependencies
- Document storage patterns (adapter, singleton, map, array, virtual)
- Migrate lww-update.meta-reducer.ts to use registry
- Migrate dependency-resolver.service.ts to use registry
- Migrate conflict-resolution.service.ts to use registry (removes
80-line switch statement)
- Migrate validate-operation-payload.ts to use getAllPayloadKeys()
- Add ESLint rule require-entity-registry to detect missing entity
types and typos in entityType properties
Net reduction: 112 lines of code
Code review improvements addressing critical and high priority issues:
Archive Handler:
- Rollback BOTH archiveYoung and archiveOld on flush failure
- Prevents data loss when partial write occurs
Cache Invalidation:
- Add _unsyncedCache invalidation in deleteOpsWhere
- Prevents stale data when deleted ops include unsynced operations
Simple Counter:
- Extract _getCounterValue helper to reduce code duplication
- Use selectSimpleCounterById (O(1)) instead of selectAllSimpleCounters+find (O(n))
- Update tests to properly mock both selectors
Operation Log Sync:
- Add infinite loop prevention when force download returns no clocks
- Add GREATER_THAN corruption detection (treats as CONCURRENT to be safe)
ESLint Hydration Guard Rule:
- Fix combineLatest detection at root level vs nested in operator callbacks
- Add comprehensive test suite (17 test cases)
E2E Tests:
- Fix flaky reminders-schedule-page tests (tasks disappear after scheduling)
Add local-rules/require-hydration-guard ESLint rule that warns when
selector-based NgRx effects lack hydration guards (skipDuringSync() or
isApplyingRemoteOps()).
The rule correctly identifies:
- Effects that START with this.store.select() as primary source
- Does NOT flag selectors in withLatestFrom (secondary sources)
- Does NOT flag selectors inside operator callbacks (already guarded)
This prevents duplicate operations during sync replay where selector-based
effects would fire on intermediate states.
Install eslint-plugin-local-rules to enable the rule.
