From 5fc9fe0411ad4412d2d472fb42d37622ae5ba46f Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sun, 10 May 2026 23:23:36 +0200 Subject: [PATCH] refactor(sync): extract framework-agnostic sync types into @sp/sync-core (#7546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(sync): extract framework-agnostic sync types into @sp/sync-core Stand up packages/sync-core/ as the new home for sync types and constants that have no Angular/NgRx coupling. This is the thin first slice of separating sync engine, configuration, and provider concerns into distinct packages. Files moved (sources now live in packages/sync-core/src/): - core/operation.types.ts - core/action-types.enum.ts - core/lww-update-action-types.ts - core/sync-state-corrupted.error.ts - core/types/apply.types.ts - sync-providers/provider.const.ts - util/entity-key.util.ts Original paths in src/app/op-log/ keep working as thin re-export stubs so existing callers don't change. op-log/sync-exports.ts now sources provider.const exports directly from @sp/sync-core. Files with transitive Angular dependencies (encryption, sync-errors, provider.interface, vector-clock util) stay in the app for now — moving them requires introducing a logger port and is part of the next slice. * refactor(sync): keep @sp/sync-core domain-agnostic The first slice landed too much Super Productivity-specific content in @sp/sync-core. The lib should expose generic sync primitives; the host app supplies the SP-specific config. Pulled out of the lib (now app-side only): - ActionType enum (NgRx action strings for SP features) - ENTITY_TYPES / EntityType union (SP domain entities) - SyncImportReason union (SP import flows) - RepairSummary / RepairPayload (SP repair shape) - WrappedFullStatePayload + appDataComplete helpers (SP wire format) - SyncProviderId / SyncStatus / ConflictReason / OAUTH_SYNC_PROVIDERS / REMOTE_FILE_CONTENT_PREFIX / PRIVATE_CFG_PREFIX (SP providers/keys) - The @sp/shared-schema dep (also SP-coupled) Generic-ized in the lib: - Operation.actionType: string and Operation.entityType: string (lib carries opaque strings; host narrows in app code) - Operation.syncImportReason removed; app extends Operation with it - VectorClock now defined locally as Record - LWW helpers replaced by createLwwUpdateActionTypeHelpers(entityTypes) factory; the app instantiates it with SP's ENTITY_TYPES - entity-key.util uses string for entityType App stubs now redeclare the SP-narrowed Operation, EntityChange, EntityConflict, ConflictResult, MultiEntityPayload, ApplyOperationsResult on top of the lib generic types via Omit-and-extend, and re-host all the SP-specific helpers (WrappedFullStatePayload, extractFullStateFromPayload, assertValidFullStatePayload, RepairSummary, RepairPayload). Also added @sp/sync-core to src/tsconfig.spec.json paths so the spec build uses the source, not the dist (matching shared-schema). * docs(sync): add @sp/sync-core extraction plan Roadmap for carving the sync engine out of src/app/op-log/ into a reusable, framework-agnostic AND domain-agnostic @sp/sync-core package plus a sibling @sp/sync-providers. Documents: - The three-concern split (engine / config / providers) target - The domain rule: nothing SP-specific lands in the lib (ActionType, ENTITY_TYPES, SyncImportReason, RepairPayload, SyncProviderId, the appDataComplete wire format, @sp/shared-schema all stay app-side) - PR 1 (landed): generic primitives only; app stubs preserve every pre-existing call site via Omit-and-extend - PR 2: SyncLogger port + parameterize entity-registry - PR 3a: pure algorithmic core (vector-clock client wrapper, conflict detection, op validation, encryption, sync-errors) — needs only the SyncLogger port - PR 3b: orchestrators behind ports (OperationStorePort, ActionDispatchPort, ConflictUiPort, SyncConfigPort) — the high-risk step where the app/lib boundary becomes load-bearing - PR 4: lift providers into @sp/sync-providers - PR 5: ESLint boundary rule * refactor(sync): address sync-core extraction review * docs(sync): refine extraction plan follow-ups --------- Co-authored-by: Claude --- .../sync-core-extraction-plan.md | 566 ++++++++++++++++++ package-lock.json | 12 + package.json | 3 +- packages/build-packages.js | 9 + packages/sync-core/.gitignore | 2 + packages/sync-core/package.json | 28 + packages/sync-core/src/apply.types.ts | 30 + packages/sync-core/src/entity-key.util.ts | 27 + packages/sync-core/src/index.ts | 31 + .../sync-core/src/lww-update-action-types.ts | 45 ++ packages/sync-core/src/operation.types.ts | 231 +++++++ .../src/sync-state-corrupted.error.ts | 48 ++ packages/sync-core/tsconfig.json | 21 + packages/sync-core/tsup.config.ts | 9 + .../op-log/core/lww-update-action-types.ts | 28 +- src/app/op-log/core/operation.types.ts | 320 ++-------- .../op-log/core/sync-state-corrupted.error.ts | 49 +- src/app/op-log/core/types/apply.types.ts | 31 +- src/app/op-log/util/entity-key.util.ts | 31 +- src/tsconfig.spec.json | 3 +- tsconfig.base.json | 3 +- 21 files changed, 1158 insertions(+), 369 deletions(-) create mode 100644 docs/long-term-plans/sync-core-extraction-plan.md create mode 100644 packages/sync-core/.gitignore create mode 100644 packages/sync-core/package.json create mode 100644 packages/sync-core/src/apply.types.ts create mode 100644 packages/sync-core/src/entity-key.util.ts create mode 100644 packages/sync-core/src/index.ts create mode 100644 packages/sync-core/src/lww-update-action-types.ts create mode 100644 packages/sync-core/src/operation.types.ts create mode 100644 packages/sync-core/src/sync-state-corrupted.error.ts create mode 100644 packages/sync-core/tsconfig.json create mode 100644 packages/sync-core/tsup.config.ts diff --git a/docs/long-term-plans/sync-core-extraction-plan.md b/docs/long-term-plans/sync-core-extraction-plan.md new file mode 100644 index 0000000000..77450a1dcb --- /dev/null +++ b/docs/long-term-plans/sync-core-extraction-plan.md @@ -0,0 +1,566 @@ +# `@sp/sync-core` Extraction Plan + +> **Status: In progress - PR 1 is under review in #7546** + +**Goal:** Carve the sync engine out of `src/app/op-log/` into a reusable, +framework-agnostic, **domain-agnostic** `@sp/sync-core` package, plus a sibling +`@sp/sync-providers` package for bundled provider implementations. + +## Context + +The sync frontend lives in `src/app/op-log/` (the older `src/app/pfapi/` is +legacy and out of scope). It already organizes itself by concern (`core`, +`sync`, `apply`, `capture`, `persistence`, `encryption`, `validation`, `util`, +`model`, `sync-providers`), but the boundary is convention-only: the engine +reaches into NgRx state, `core/entity-registry.ts` hardcodes imports from 15+ +feature reducers, and providers and engine code intermix freely. + +The eventual target is a **three-concern split**: + +1. **Sync logic / engine** - operation orchestration, vector clocks, conflict + resolution, persistence interfaces. Framework-agnostic and domain-agnostic. +2. **Configuration** - entity registry, model config, app-specific wiring, + action-type enums, entity-type unions, repair payload shapes, provider lists. + Lives in the app. +3. **Provider implementations** - SuperSync, Dropbox, WebDAV, LocalFile. + Pluggable, and talking to the engine through stable interfaces. + +## Domain Rule + +Anything that names a Super Productivity domain object, enum value, or wire +convention belongs in the app, not in `@sp/sync-core`. The lib carries +`actionType` and `entityType` as plain `string`; the app narrows via +`Omit`-and-extend on top of the lib's generic `Operation`. + +App-only forever: + +- **`ActionType` enum** - host-app action catalog, not lib content. +- **`ENTITY_TYPES` / `EntityType` union** - TASK, PROJECT, TAG, METRIC, BOARD, + etc. are SP's domain. Lib uses `string`; app narrows. +- **`SyncImportReason` union** - SP's specific import flows. +- **`RepairSummary`, `RepairPayload`** - SP's repair-output shape. +- **`WrappedFullStatePayload` + `extractFullStateFromPayload` + + `assertValidFullStatePayload`** - the `appDataComplete` wrapper and the + `['task','project','tag','globalConfig']` key-presence check are SP wire + format. +- **`SyncProviderId`, `OAUTH_SYNC_PROVIDERS`, `REMOTE_FILE_CONTENT_PREFIX`, + `PRIVATE_CFG_PREFIX`** - SP's bundled providers and SP-flavored storage + prefixes. +- **`@sp/shared-schema`** - that package is SP-coupled today, so + `@sp/sync-core` must not depend on it. + +Where the lib needs host-specific enumerations, it exposes a factory or config +object and the app supplies values at composition time. The current LWW helper +factory is the model to follow. + +## Recommendations From PR #7546 Review + +These adjustments should happen before the extraction proceeds beyond the thin +first slice: + +1. **Move boundary enforcement up.** Add ESLint/package-boundary checks in the + next PR, not at the end. Once `packages/sync-core/` exists, accidental + imports from Angular, NgRx, `src/app`, or `@sp/shared-schema` should fail + immediately. +2. **Single-source vector-clock algorithms.** The client currently delegates + comparison/merge/prune behavior to `@sp/shared-schema` for client/server + parity. Before moving vector-clock code, pick one owner for + compare/merge/prune and have the other package/server import or re-export it. + Do not duplicate the algorithms. +3. **Treat full-state operation classification as configuration.** PR 1 keeps + `OpType.SyncImport`, `OpType.BackupImport`, and `OpType.Repair` in the + generic package for compatibility. Before the engine becomes reusable, make + full-state operation classification configurable or explicitly document those + op types as host-defined strings. +4. **Do not move `OperationApplierService` wholesale.** It currently coordinates + NgRx bulk dispatch, hydration windows, archive side effects, and deferred + local actions. Extract a small core replay contract/state machine first, + leaving the Angular/SP choreography in the app until the port boundary has + proven itself. +5. **Make logger metadata privacy-safe.** `CLAUDE.md` forbids logging user + content into exportable logs. The `SyncLogger` port should make this explicit + by accepting only safe, structured metadata and documenting that payloads/full + entities must not be logged. +6. **Add package tests before moving algorithms.** `@sp/sync-core` can start + with build-only checks, but PR 3a should first introduce the package test + runner and then port algorithm specs. +7. **Keep provider extraction separate.** Do not let `@sp/sync-core` learn + provider IDs, file prefixes, OAuth behavior, credential storage, or bundled + provider lists. + +## PR 1 - Thin First Slice (#7546) + +Stand up `packages/sync-core/` with pieces that are framework-agnostic and +mostly domain-agnostic. No behavior change. Establishes the import boundary and +the `@sp/sync-core` alias so later PRs work against a real package boundary. + +### Goals + +- Create `packages/sync-core/` mirroring the existing package shape. +- Move only generic primitives and helpers. +- Move only framework-agnostic code: no `@Injectable`, no `inject()`, no NgRx, + no Angular Material. +- Keep existing `src/app/op-log/` call sites working through stubs at the + original paths. +- Keep `ActionType`, provider constants, full-state payload wrappers, repair + payload shapes, and import reasons app-side. +- Avoid behavior changes. + +### Current Contents + +Source: `packages/sync-core/src/`. All exports come through `index.ts`. + +**Operation primitives** (`operation.types.ts`): + +- `OpType` enum. +- `Operation` with `actionType: string` and `entityType: string`. +- `OperationLogEntry`, `EntityConflict`, `ConflictResult`, `EntityChange`, + `MultiEntityPayload`. +- `VectorClock = Record`. +- `FULL_STATE_OP_TYPES`, `isFullStateOpType`, `isMultiEntityPayload`, + `extractActionPayload`. + +**LWW factory** (`lww-update-action-types.ts`): + +- `createLwwUpdateActionTypeHelpers(entityTypes)` returns + `LWW_UPDATE_ACTION_TYPES`, `isLwwUpdateActionType`, `getLwwEntityType`, and + `toLwwUpdateActionType`. +- The app instantiates it once with `ENTITY_TYPES`. + +**Apply types** (`apply.types.ts`): + +- `ApplyOperationsResult`, `ApplyOperationsOptions` over the lib's generic + `Operation`. + +**Utilities**: + +- `toEntityKey`, `parseEntityKey`. +- `SyncStateCorruptedError`. + +### App Stubs + +Each previously-public symbol path keeps working via thin shims: + +- `src/app/op-log/core/operation.types.ts` re-exports generic symbols and + redeclares SP-narrowed `Operation`, `OperationLogEntry`, `EntityChange`, + `EntityConflict`, `ConflictResult`, and `MultiEntityPayload`. +- `src/app/op-log/core/types/apply.types.ts` redeclares app-narrowed apply + result/options types. +- `src/app/op-log/core/lww-update-action-types.ts` instantiates the LWW helper + factory with `ENTITY_TYPES`. +- `src/app/op-log/core/sync-state-corrupted.error.ts` re-exports from the + package. +- `src/app/op-log/util/entity-key.util.ts` delegates to the package while + preserving the app's `EntityType`-narrowed API. +- `src/app/op-log/core/action-types.enum.ts` stays full source in the app. +- `src/app/op-log/sync-providers/provider.const.ts` stays full source in the app. + +### PR 1 Follow-Ups Before Merge + +- Update the PR description if it still says `action-types.enum.ts` or + `provider.const.ts` moved into `@sp/sync-core`; the code correctly keeps them + app-side. +- Fix comments that imply `sync-core` depends on `shared-schema`. The build may + run after `shared-schema`, but the package dependency direction must remain + absent. +- Decide whether `FULL_STATE_OP_TYPES` is acceptable compatibility debt for PR 1 + or whether it should already become app-configurable. + +### Verification + +1. `cd packages/sync-core && npx tsup` - package builds clean. +2. `npx tsc -p src/tsconfig.app.json --noEmit` - app type-checks. +3. `npm run checkFile` on every touched `.ts` file. +4. `npm test` or scoped op-log specs. +5. App boot plus manual sync smoke: sync round-trip, conflict round-trip, + encryption toggle. +6. SuperSync E2E when the branch is ready for merge. +7. Boundary check returns nothing: + + ```bash + grep -r "from '@angular\\|from '@ngrx\\|from '@sp/shared-schema\\|src/app" packages/sync-core/src/ + ``` + +--- + +## PR 2 - Boundary Guardrails, Entity Registry Types, Logger Port + +This replaces the original late ESLint PR. Boundary guardrails should land +immediately after the package exists. + +### Goals + +1. **Add package boundary enforcement.** Lint `packages/sync-core/**` and reject + imports from Angular, NgRx, `src/app`, and `@sp/shared-schema`. +2. **Entity registry as config.** Move abstract registry types into + `@sp/sync-core`; keep SP feature imports and registry construction in the app. +3. **Logger port.** Define a privacy-aware `SyncLogger` interface in + `@sp/sync-core` so moveable files can drop direct `OpLog` imports. + +### Boundary Enforcement + +- Update `eslint.config.js` so `packages/sync-core/**` is linted. +- Add `no-restricted-imports` for: + - `@angular/*` + - `@ngrx/*` + - `@sp/shared-schema` + - `src/app/*` and relative app imports such as `../../src/app/*` +- Keep package exceptions explicit for packages that cannot yet be linted. +- Add the same rule for `packages/sync-providers/**` once that package exists. + +### Entity Registry Types + +Define `EntityConfig` / `EntityRegistry` types in +`@sp/sync-core/src/entity-registry.types.ts`, but make the shape reflect the +current registry, not a simplified example. + +Required storage patterns: + +```ts +type EntityStoragePattern = 'adapter' | 'singleton' | 'map' | 'array' | 'virtual'; +``` + +Guidelines: + +- Registry keys are `string`; the app narrows them to `EntityType`. +- Selectors are structural function types; the package must not import NgRx + selector types. +- Adapter support is structural, not `@ngrx/entity`-typed. Include only the + methods actually consumed by op-log code. +- Include `payloadKey`, `featureName`, `mapKey`, and `arrayKey` if current + consumers need them. +- Keep `SINGLETON_ENTITY_ID` generic if it remains engine-relevant; otherwise + keep it in the app. + +App-side changes: + +- Replace the hardcoded exported registry with `buildEntityRegistry()` in + `src/app/op-log/core/entity-registry.ts`. +- Provide an `ENTITY_REGISTRY` injection token in app code for services that + should stop importing the registry singleton directly. +- Keep all feature reducer/selector imports in the app. + +### Logger Port + +Define `SyncLogger` in the lib: + +```ts +export type SyncLogMeta = Record; + +export interface SyncLogger { + log(message: string, meta?: SyncLogMeta): void; + error(message: string, error?: unknown, meta?: SyncLogMeta): void; + err(message: string, error?: unknown, meta?: SyncLogMeta): void; + normal(message: string, meta?: SyncLogMeta): void; + verbose(message: string, meta?: SyncLogMeta): void; + info(message: string, meta?: SyncLogMeta): void; + warn(message: string, meta?: SyncLogMeta): void; + critical(message: string, meta?: SyncLogMeta): void; + debug(message: string, meta?: SyncLogMeta): void; +} +``` + +Also provide a `NOOP_SYNC_LOGGER` for tests and package defaults. + +Keep both `error()` and `err()` initially because current movable code uses both +`OpLog` spellings. If a follow-up PR normalizes calls to one spelling, do that +explicitly in the same PR instead of silently shrinking the port surface. + +Privacy rule: logger metadata must not include full entities, operation payloads, +task titles, note text, raw provider responses, credentials, or encryption +material. IDs, counts, op IDs, action strings, entity types, and error names are +acceptable. + +### What This Unlocks + +After this PR, files blocked only by `OpLog` can move without creating a package +dependency on app logging: + +- `op-log/encryption/` +- `op-log/core/errors/sync-errors.ts` +- `op-log/util/sync-file-prefix.ts` + +### Verification + +- `npm run lint` proves package boundary rules are active. +- Add and revert one deliberately-bad package import to prove the rule fails. +- `npm test` for registry-related specs. +- App boot + sync round-trip. +- Manual log export flow: sync/encryption events still appear and do not expose + user content. + +--- + +## PR 3a - Vector-Clock Ownership and Package Test Harness + +Do this before moving more algorithms. Vector-clock parity is load-bearing for +sync correctness. + +### Goals + +1. Pick the single source of truth for vector-clock compare/merge/prune logic. +2. Add a package test runner for `@sp/sync-core`. +3. Port existing vector-clock tests before changing call sites. + +### Preferred Direction + +Decide the dependency direction before PR 3a moves code. The preferred outcome +is that `@sp/sync-core` owns generic vector-clock algorithms: + +- `compareVectorClocks` +- `mergeVectorClocks` +- `limitVectorClockSize` +- `MAX_VECTOR_CLOCK_SIZE` +- validation/sanitization helpers if they are shared by client/server + +This is acceptable only if current server/shared consumers can depend on +`@sp/sync-core` without creating a bad package direction or build cycle. In that +case, update build order so `sync-core` is available before those consumers, or +make the server consume `@sp/sync-core` directly. + +If that dependency direction is awkward, create a tiny leaf package such as +`@sp/vector-clock` and have both `@sp/sync-core` and server/shared code consume +it. Do not make `@sp/sync-core` depend on `@sp/shared-schema`; the important +constraint is one implementation, not two copies. + +### Migration Notes + +- Preserve client null/undefined wrapper behavior exactly. +- Preserve `MAX_VECTOR_CLOCK_SIZE = 20`. +- Preserve server ordering: conflict detection first, pruning before storage. +- Replace the RxJS prune `Subject` with a callback/event hook at the package + boundary. +- Route logging through `SyncLogger`. + +### Verification + +- Port `src/app/core/util/vector-clock.spec.ts` into package tests where possible. +- Keep app wrapper specs for integration behavior and import compatibility. +- Run package tests, app op-log specs, and boundary grep. + +--- + +## PR 3b - Pure Algorithmic Core + +Move framework-agnostic, stateless sync algorithms. These should only need typed +inputs and the logger port. + +### What Moves + +- Conflict detection and LWW resolution algorithms from + `op-log/sync/conflict-resolution.service.ts`. +- Filtering/partitioning helpers that operate on `OperationLogEntry[]`. +- Pure op merge helpers currently scattered across `remote-ops-processing.service.ts` + and `operation-log-sync.service.ts`. +- Pure operation payload validation from `op-log/validation/`, as long as it + does not import app schemas or NgRx selectors. +- Encryption/compression utilities once `OpLog` is removed. +- `sync-errors.ts` and `sync-file-prefix.ts` if they are generic after + logger/config cleanup. + +### What Stays App-Side + +- Anything that calls `Store.dispatch()` or `Store.select()`. +- `OperationLogStoreService` and IndexedDB implementation details. +- UI services: dialogs, snacks, Angular Material. +- Effects, meta-reducers, and `LOCAL_ACTIONS` wiring. +- App schema validation tied to SP model shape. +- Full-state payload wrappers and SP repair payloads. + +### Verification + +- Package test suite for moved algorithms. +- Full app `npm test` for integration through stubs. +- Boundary grep stays empty. +- Manual sync round-trip, encryption toggle, and conflict scenario. + +--- + +## PR 4a - Port Contracts Only + +Introduce orchestration ports without moving the orchestrators yet. This reduces +the risk of the later service moves. + +### Ports + +- `OperationStorePort` - abstract over op-log persistence. Method names use + `Operation` / `OperationLogEntry` only. +- `ActionDispatchPort` - abstract over dispatching replay actions. Takes generic + action objects and must preserve `meta` exactly. +- `RemoteApplyWindowPort` - abstracts `HydrationStateService` behavior: start + remote apply, end remote apply, post-sync cooldown. +- `DeferredLocalActionsPort` - abstracts + `OperationLogEffects.processDeferredActions()`. +- `ArchiveSideEffectPort` - abstracts archive-specific IndexedDB handling for + remote operations. +- `ConflictUiPort` - app dialog/snack adapter. Reasons are strings at the + package boundary. +- `SyncConfigPort` - app adapter around NgRx config selectors. Provider IDs are + strings at the package boundary. +- `RepairPort` only if truly needed, and with generic shapes. + +### Why Split This Out + +`OperationApplierService` is not just replay logic. It currently coordinates: + +- bulk NgRx dispatch, +- the required event-loop yield after dispatch, +- remote apply windows and cooldowns, +- archive side effects, +- `remoteArchiveDataApplied`, +- deferred local action processing. + +Those behaviors should first be represented as ports and tested while the +service remains app-side. + +### Verification + +- Adapter specs prove app services satisfy the ports. +- Existing app sync specs still pass. +- Add contract tests for action `meta` preservation and bulk-dispatch yield + behavior. + +--- + +## PR 4b - Move Small Orchestration Units Behind Ports + +Move only orchestration code whose dependencies are already represented by ports +and whose behavior can be tested without Angular. + +### Candidate Moves + +- Upload batching/retry logic from `OperationLogUploadService` if provider and + store access are ported. +- Remote op processing state machine if applying, marking, and validation are all + ports. +- Pure parts of download/upload decision logic. + +### Keep App-Side Until Proven Safe + +- The Angular `OperationApplierService` shell. +- `bulkApplyOperations` action and meta-reducer wiring. +- `HydrationStateService` implementation. +- `ArchiveOperationHandler` implementation. +- Effects using `inject(LOCAL_ACTIONS)`. +- UI-coupled conflict/import/download services. + +### Verification + +- Package orchestration tests. +- App adapter tests. +- Full app unit tests. +- SuperSync scenarios focused on concurrency, fresh-client bootstrap, server + migration, and import conflicts. + +--- + +## PR 4c - Revisit `OperationApplierService` + +Only after 4a/4b are stable, decide whether any part of +`OperationApplierService` belongs in `@sp/sync-core`. + +Acceptable extraction: + +- a small generic replay coordinator that calls ports in a strict order; +- contract tests for yielding, failure reporting, archive side-effect ordering, + and deferred-action flush timing. + +Likely app-side permanently: + +- NgRx action construction and `bulkApplyOperations`, +- Angular `Injector` usage, +- `remoteArchiveDataApplied`, +- hydration-state implementation, +- archive handler implementation. + +Hard requirements from `CLAUDE.md`: + +- remote operations must not trigger normal effects; +- selector-based effects must remain guarded by the sync window; +- bulk dispatch must yield after the dispatch; +- remote archive side effects must still run; +- deferred local actions must be processed after remote apply finishes. + +--- + +## PR 5 - Lift Providers Into `@sp/sync-providers` + +Pull bundled providers out of `src/app/op-log/sync-providers/` so engine, +providers, and app wiring each live in their own package. + +### What Moves + +- `op-log/sync-providers/super-sync/` +- `op-log/sync-providers/file-based/dropbox/` +- `op-log/sync-providers/file-based/webdav/` including Nextcloud-specific code +- `op-log/sync-providers/file-based/local-file/`, with Electron APIs behind an + app-provided port +- provider registry/factory logic that does not read NgRx state directly + +### What Stays App-Side + +- `SyncProviderId` and bundled provider lists. +- Credential-store Angular service implementation. +- OAuth callback routing. +- Provider config UI/dialogs. +- Electron bridge implementation. +- Any code reading `selectSyncConfig` directly. + +### Provider Package Rules + +- Provider IDs inside the package are string constants, not the app's + `SyncProviderId` enum. +- Credential storage is an interface. +- HTTP should use `fetch` or an injected HTTP port, not Angular `HttpClient`. +- Provider package must not import `@sp/sync-core` internals beyond public + ports/types. + +### Verification + +- Per-provider unit specs. +- E2E sync round-trip per provider: Dropbox, WebDAV, LocalFile, SuperSync. +- Fresh-client bootstrap for file-based providers. +- Electron-gated LocalFile path smoke test. + +--- + +## PR 6 - Final Boundary Hardening + +This is now a final audit rather than the first boundary rule. + +### Goals + +- Extend the boundary rules to `packages/sync-providers/**`. +- Audit package manifests for accidental runtime deps. +- Audit public exports for SP names and app-only concepts. +- Add a small architecture note that explains the package boundaries and allowed + dependency direction. + +### Verification + +- `npm run lint`. +- Boundary grep for both packages. +- Package builds from a clean install. +- Full app unit tests and selected sync E2E. + +--- + +## Summary Timeline + +| PR | Scope | Risk | Notes | +| ------ | ---------------------------------------------------------- | ----------- | -------------------------------------- | +| **1** | Stand up `@sp/sync-core` with generic primitives and stubs | Low | Current PR #7546 | +| **2** | Boundary lint, registry types, privacy-aware logger port | Medium | Moves guardrails earlier | +| **3a** | Vector-clock ownership and package test harness | Medium | Prevents algorithm drift | +| **3b** | Pure algorithmic core | Medium | No Angular/NgRx/IndexedDB | +| **4a** | Port contracts only | Medium | Keeps orchestrators app-side | +| **4b** | Move small orchestration units behind ports | High | Incremental state-machine extraction | +| **4c** | Revisit `OperationApplierService` extraction | High | Extract only if the boundary is proven | +| **5** | Lift providers into `@sp/sync-providers` | Medium-High | Provider deps stay out of core | +| **6** | Final boundary hardening and architecture note | Low | Audit and lock down | + +After the final PR, `@sp/sync-core` should be the domain-agnostic sync engine +and abstractions, `@sp/sync-providers` should contain bundled provider +implementations, and `src/app/op-log/` should contain SP-specific wiring: NgRx +adapters, dialog ports, entity-registry composition, `ActionType`, `EntityType`, +`SyncImportReason`, `SyncProviderId`, repair shapes, and full-state wire format. diff --git a/package-lock.json b/package-lock.json index ddb0bf5c4c..f24960f21c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9563,6 +9563,10 @@ "resolved": "packages/shared-schema", "link": true }, + "node_modules/@sp/sync-core": { + "resolved": "packages/sync-core", + "link": true + }, "node_modules/@standard-schema/spec": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@standard-schema/spec/-/spec-1.1.0.tgz", @@ -29754,6 +29758,14 @@ "url": "https://github.com/sponsors/colinhacks" } }, + "packages/sync-core": { + "name": "@sp/sync-core", + "version": "1.0.0", + "devDependencies": { + "tsup": "^8.0.0", + "typescript": "^5.0.0" + } + }, "packages/vite-plugin": { "name": "@super-productivity/vite-plugin", "version": "1.0.0", diff --git a/package.json b/package.json index ad2c4cb527..d65910b586 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "assemble:android:prod": "cd android && ./gradlew assembleRelease && cd ..", "assemble:android:stage": "cd android && ./gradlew assembleDebug && cd ..", "prebuild": "npm run env && node ./tools/git-version.js && npm run build:packages", - "prepare": "husky && ts-patch install && npm run env && npm run shared-schema:build && npm run plugin-api:build", + "prepare": "husky && ts-patch install && npm run env && npm run shared-schema:build && npm run sync-core:build && npm run plugin-api:build", "build": "npm run buildAllElectron:noTests:prod", "build:packages": "node ./packages/build-packages.js", "buildAllElectron:noTests:prod": "npm run lint && npm run buildFrontend:prod:es6 && npm run electron:build", @@ -155,6 +155,7 @@ "clean:translations": "node ./tools/clean-translations.js", "plugin-api:build": "cd packages/plugin-api && npm run build", "shared-schema:build": "cd packages/shared-schema && npm run build", + "sync-core:build": "cd packages/sync-core && npm run build", "plugin-api:build:watch": "cd packages/plugin-api && npm run build:watch", "vite-plugin:build": "cd packages/vite-plugin && npm run build", "plugins:build": "npm run plugin-api:build && npm run vite-plugin:build && cd packages/plugin-dev && npm run build:all", diff --git a/packages/build-packages.js b/packages/build-packages.js index e3a85b6903..42d4b80942 100755 --- a/packages/build-packages.js +++ b/packages/build-packages.js @@ -46,6 +46,15 @@ async function getPlugins() { skipCopy: true, }); + // Build sync-core after shared-schema to keep package build order stable. + // sync-core must not import shared-schema; the sync core stays domain-agnostic. + plugins.push({ + name: 'sync-core', + path: 'packages/sync-core', + buildCommand: 'npm run build', + skipCopy: true, + }); + // Add plugin-api as it's a dependency for plugins plugins.push({ name: 'plugin-api', diff --git a/packages/sync-core/.gitignore b/packages/sync-core/.gitignore new file mode 100644 index 0000000000..1eae0cf670 --- /dev/null +++ b/packages/sync-core/.gitignore @@ -0,0 +1,2 @@ +dist/ +node_modules/ diff --git a/packages/sync-core/package.json b/packages/sync-core/package.json new file mode 100644 index 0000000000..7b91788446 --- /dev/null +++ b/packages/sync-core/package.json @@ -0,0 +1,28 @@ +{ + "name": "@sp/sync-core", + "version": "1.0.0", + "description": "Framework-agnostic core types and utilities for Super Productivity sync", + "main": "dist/index.js", + "module": "dist/index.mjs", + "types": "dist/index.d.ts", + "exports": { + ".": { + "import": { + "types": "./dist/index.d.mts", + "default": "./dist/index.mjs" + }, + "require": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + } + } + }, + "scripts": { + "build": "tsup", + "build:tsc": "tsc" + }, + "devDependencies": { + "tsup": "^8.0.0", + "typescript": "^5.0.0" + } +} diff --git a/packages/sync-core/src/apply.types.ts b/packages/sync-core/src/apply.types.ts new file mode 100644 index 0000000000..4c2b03ee93 --- /dev/null +++ b/packages/sync-core/src/apply.types.ts @@ -0,0 +1,30 @@ +import { Operation } from './operation.types'; + +/** + * Result of applying a batch of operations. + * + * Allows callers to handle partial success scenarios where some operations + * were applied before an error occurred. + */ +export interface ApplyOperationsResult { + /** Operations that were successfully applied. */ + appliedOps: Operation[]; + + /** + * If an error occurred, this contains the failed operation and the error. + * Operations after this one in the batch were NOT applied. + */ + failedOp?: { + op: Operation; + error: Error; + }; +} + +export interface ApplyOperationsOptions { + /** + * When true, skip side effects that would normally fire on first application + * (e.g. writing to archive storage) — used when replaying already-persisted + * local operations during hydration. + */ + isLocalHydration?: boolean; +} diff --git a/packages/sync-core/src/entity-key.util.ts b/packages/sync-core/src/entity-key.util.ts new file mode 100644 index 0000000000..52e44ee530 --- /dev/null +++ b/packages/sync-core/src/entity-key.util.ts @@ -0,0 +1,27 @@ +/** + * Creates a unique key for an entity by combining its type and ID. + * Used for indexing and looking up entities across the operation log system. + * + * @param entityType The type of the entity (e.g., 'TASK', 'PROJECT') + * @param entityId The unique ID of the entity + * @returns A composite key in the format "ENTITY_TYPE:entityId" + */ +export const toEntityKey = (entityType: string, entityId: string): string => + `${entityType}:${entityId}`; + +/** + * Parses an entity key back into its components. + * + * @param key The composite entity key + * @returns An object containing entityType and entityId + */ +export const parseEntityKey = (key: string): { entityType: string; entityId: string } => { + const colonIndex = key.indexOf(':'); + if (colonIndex === -1) { + throw new Error(`Invalid entity key format: ${key}`); + } + return { + entityType: key.substring(0, colonIndex), + entityId: key.substring(colonIndex + 1), + }; +}; diff --git a/packages/sync-core/src/index.ts b/packages/sync-core/src/index.ts new file mode 100644 index 0000000000..2ec2466fa6 --- /dev/null +++ b/packages/sync-core/src/index.ts @@ -0,0 +1,31 @@ +// Operation log primitives — the generic, app-agnostic core of the sync engine. +export { + OpType, + FULL_STATE_OP_TYPES, + isFullStateOpType, + isMultiEntityPayload, + extractActionPayload, +} from './operation.types'; +export type { + VectorClock, + Operation, + OperationLogEntry, + EntityConflict, + ConflictResult, + EntityChange, + MultiEntityPayload, +} from './operation.types'; + +// LWW (Last-Writer-Wins) update action-type helpers — factory parameterized by +// the host application's entity-type list, so the lib stays domain-agnostic. +export { createLwwUpdateActionTypeHelpers } from './lww-update-action-types'; +export type { LwwUpdateActionTypeHelpers } from './lww-update-action-types'; + +// Apply-operation result and option types. +export type { ApplyOperationsResult, ApplyOperationsOptions } from './apply.types'; + +// Entity key encoding helpers. +export { toEntityKey, parseEntityKey } from './entity-key.util'; + +// Sync state corruption error. +export { SyncStateCorruptedError } from './sync-state-corrupted.error'; diff --git a/packages/sync-core/src/lww-update-action-types.ts b/packages/sync-core/src/lww-update-action-types.ts new file mode 100644 index 0000000000..9810a094c2 --- /dev/null +++ b/packages/sync-core/src/lww-update-action-types.ts @@ -0,0 +1,45 @@ +/** + * Last-Writer-Wins update action type helpers. + * + * The lib doesn't know which entity types exist in the host app, so the helpers + * are constructed via a factory that takes the host's entity-type list. + */ + +const LWW_UPDATE_SUFFIX = '] LWW Update'; + +export interface LwwUpdateActionTypeHelpers { + /** Set of all "; + /** True if `actionType` is a LWW update for one of the registered entity types. */ + isLwwUpdateActionType(actionType: string): boolean; + /** Reverse lookup: which entity type is targeted by this LWW action-type string? */ + getLwwEntityType(actionType: string): TEntityType | undefined; + /** Build the LWW update action-type string for a given entity type. */ + toLwwUpdateActionType(entityType: TEntityType): string; +} + +/** + * Build LWW helpers for the given list of entity types. + * + * @example + * const lww = createLwwUpdateActionTypeHelpers(['TASK', 'PROJECT'] as const); + * lww.toLwwUpdateActionType('TASK'); // '[TASK] LWW Update' + */ +export const createLwwUpdateActionTypeHelpers = ( + entityTypes: readonly TEntityType[], +): LwwUpdateActionTypeHelpers => { + const LWW_UPDATE_ACTION_TYPES: ReadonlySet = new Set( + entityTypes.map((et) => `[${et}${LWW_UPDATE_SUFFIX}`), + ); + + const LWW_ENTITY_MAP: ReadonlyMap = new Map( + entityTypes.map((et) => [`[${et}${LWW_UPDATE_SUFFIX}`, et]), + ); + + return { + LWW_UPDATE_ACTION_TYPES, + isLwwUpdateActionType: (actionType) => LWW_UPDATE_ACTION_TYPES.has(actionType), + getLwwEntityType: (actionType) => LWW_ENTITY_MAP.get(actionType), + toLwwUpdateActionType: (entityType) => `[${entityType}${LWW_UPDATE_SUFFIX}`, + }; +}; diff --git a/packages/sync-core/src/operation.types.ts b/packages/sync-core/src/operation.types.ts new file mode 100644 index 0000000000..1c7d716097 --- /dev/null +++ b/packages/sync-core/src/operation.types.ts @@ -0,0 +1,231 @@ +/** + * Vector clock data structure. + * Maps client IDs to their respective monotonically-increasing clock values. + * + * Defined locally so the lib has no dependency on application-specific schema. + */ +export type VectorClock = Record; + +export enum OpType { + Create = 'CRT', + Update = 'UPD', + Delete = 'DEL', + Move = 'MOV', // For list reordering + Batch = 'BATCH', // For bulk operations (import, mass update) + /** Replaces entire app state from remote sync. All concurrent ops are discarded. */ + SyncImport = 'SYNC_IMPORT', + /** Replaces entire app state from backup file. All concurrent ops are discarded. */ + BackupImport = 'BACKUP_IMPORT', + /** Auto-repair operation containing full repaired state. */ + Repair = 'REPAIR', +} + +export interface Operation { + /** + * Unique identifier for the operation. + * Should be a UUID v7 (time-ordered) to allow for rough chronological sorting + * even without vector clocks, which aids in debugging and indexing. + */ + id: string; + + // ACTION MAPPING + /** + * The action type string this operation replays (e.g., '[Task] Update'). + * Carried as an opaque string here; the host app's action-type enum is the + * source of truth for valid values. + */ + actionType: string; + + /** + * High-level operation category (Create, Update, Delete, etc.). + * Used for broad logic handling like persistence strategies or conflict resolution rules. + */ + opType: OpType; + + // SCOPE + /** + * The type of the data model entity being modified. + * Carried as an opaque string here; the host app supplies the valid set. + */ + entityType: string; + + /** + * The specific ID of the entity being modified. + * Use '*' for singleton entities like global config. + */ + entityId?: string; + + /** + * List of entity IDs for batch operations that affect multiple items simultaneously. + */ + entityIds?: string[]; + + // DATA + /** + * The actual data change associated with the operation. + * - For 'CRT', this is the full object. + * - For 'UPD', this is a partial object (changeset). + * - For 'DEL', this might be empty or a tombstone. + */ + payload: unknown; + + // CAUSALITY & ORDERING + /** + * The ID of the device/client that generated this operation. + * Essential for vector clock management and identifying the source of changes. + */ + clientId: string; + + /** + * Represents the causal state of the world AFTER this operation was applied. + * Used to detect concurrency: if two ops have unordered vector clocks, they are concurrent. + */ + vectorClock: VectorClock; + + /** + * Wall clock time (epoch ms) from the **originating** device. + * Used as a tie-breaker for concurrent operations (Last-Write-Wins logic). + */ + timestamp: number; + + // META + /** + * The schema version of the data at the time of operation creation. + * Allows the system to migrate or transform payloads if the data structure changes in the future. + */ + schemaVersion: number; +} + +export interface OperationLogEntry { + /** + * Local, monotonic auto-increment integer (IndexedDB primary key). + * Strictly orders operations as they arrived or were generated on THIS specific device. + */ + seq: number; + + /** + * The operation data itself (the synchronized part). + */ + op: Operation; + + /** + * Local timestamp (epoch ms) indicating when this operation was written to the LOCAL database. + * Used strictly for local maintenance, such as garbage collection (compaction). + */ + appliedAt: number; + + /** + * Origin of the operation: + * - 'local': Generated by this device. + * - 'remote': Received from the sync server. + */ + source: 'local' | 'remote'; + + /** + * Timestamp (epoch ms) when this operation was successfully acknowledged by the remote server. + * Null/undefined if the operation is still pending upload. + */ + syncedAt?: number; + + /** + * Timestamp (epoch ms) if the operation was rejected during conflict resolution. + * Effectively marks the operation as "dead" but kept for history/debugging. + */ + rejectedAt?: number; + + /** + * For remote ops only: tracks whether the op was successfully applied to local state. + * Used for crash recovery: on startup, any 'pending' or 'failed' remote ops are re-dispatched. + */ + applicationStatus?: 'pending' | 'applied' | 'failed'; + + /** + * For 'failed' ops: number of retry attempts. + */ + retryCount?: number; +} + +export interface EntityConflict { + entityType: string; + entityId: string; + localOps: Operation[]; + remoteOps: Operation[]; + suggestedResolution: 'local' | 'remote' | 'merge' | 'manual'; + mergedPayload?: unknown; +} + +export interface ConflictResult { + nonConflicting: Operation[]; + conflicts: EntityConflict[]; +} + +// ============================================================================= +// FULL-STATE OPERATION HELPERS +// ============================================================================= + +/** + * OpTypes that contain full application state in their payload. + */ +export const FULL_STATE_OP_TYPES = new Set([ + OpType.SyncImport, + OpType.BackupImport, + OpType.Repair, +]); + +/** + * Type guard to check if an operation is a full-state operation. + */ +export const isFullStateOpType = (opType: OpType | string): boolean => + FULL_STATE_OP_TYPES.has(opType as OpType); + +// ============================================================================= +// MULTI-ENTITY OPERATIONS +// ============================================================================= + +/** + * Represents a single entity change within a multi-entity operation. + */ +export interface EntityChange { + entityType: string; + entityId: string; + opType: OpType; + /** + * The actual changes: + * - For Create: Full entity object + * - For Update: Partial object with only changed fields + * - For Delete: Minimal tombstone + */ + changes: unknown; +} + +/** + * Payload wrapper for multi-entity operations. + * Contains the original action payload plus all entity changes computed from state diff. + */ +export interface MultiEntityPayload { + actionPayload: Record; + entityChanges: EntityChange[]; +} + +/** + * Type guard to check if a payload is a multi-entity payload. + */ +export const isMultiEntityPayload = (payload: unknown): payload is MultiEntityPayload => { + return ( + typeof payload === 'object' && + payload !== null && + 'entityChanges' in payload && + Array.isArray((payload as MultiEntityPayload).entityChanges) + ); +}; + +/** + * Extracts the action payload from an operation payload. + * Handles both multi-entity payloads (new format) and legacy payloads. + */ +export const extractActionPayload = (payload: unknown): Record => { + if (isMultiEntityPayload(payload)) { + return payload.actionPayload; + } + return payload as Record; +}; diff --git a/packages/sync-core/src/sync-state-corrupted.error.ts b/packages/sync-core/src/sync-state-corrupted.error.ts new file mode 100644 index 0000000000..2b44e36229 --- /dev/null +++ b/packages/sync-core/src/sync-state-corrupted.error.ts @@ -0,0 +1,48 @@ +/** + * Error thrown when the sync state is corrupted and a full re-sync is required. + * + * ## Design Philosophy: Fail Fast, Re-sync Clean + * + * When an operation cannot be applied due to missing dependencies, rather than + * attempting complex retry logic with queues, we fail immediately and signal + * that a full re-sync is needed. + * + * ### Why This Approach? + * + * 1. **Simplicity**: No retry queues, no failed operation tracking, no pruning logic + * 2. **Correctness**: A full re-sync guarantees consistent state + * 3. **Debuggability**: Clear error with context about what went wrong + * 4. **User Experience**: Better to re-sync cleanly than live with subtle inconsistencies + * + * ### When Is This Thrown? + * + * - When applying remote operations and a hard dependency is missing + * - This indicates either: + * - Operations arrived out of order (protocol issue) + * - Local state is corrupted + * - A bug in dependency tracking + * + * ### How Should Callers Handle This? + * + * The host application should catch this error, mark the affected operations as + * failed according to its persistence model, and trigger a full re-sync to + * restore consistent state. + */ +export class SyncStateCorruptedError extends Error { + constructor( + message: string, + public readonly context: { + opId: string; + actionType: string; + missingDependencies: string[]; + }, + ) { + super(message); + this.name = 'SyncStateCorruptedError'; + + // Maintains proper stack trace for where error was thrown (V8 engines) + if (Error.captureStackTrace) { + Error.captureStackTrace(this, SyncStateCorruptedError); + } + } +} diff --git a/packages/sync-core/tsconfig.json b/packages/sync-core/tsconfig.json new file mode 100644 index 0000000000..cabc0b830d --- /dev/null +++ b/packages/sync-core/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "preserve", + "moduleResolution": "bundler", + "lib": ["ES2022"], + "declaration": true, + "declarationMap": true, + "sourceMap": true, + "outDir": "./dist", + "rootDir": "./src", + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "resolveJsonModule": true, + "isolatedModules": true + }, + "include": ["src/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/sync-core/tsup.config.ts b/packages/sync-core/tsup.config.ts new file mode 100644 index 0000000000..e2f0728676 --- /dev/null +++ b/packages/sync-core/tsup.config.ts @@ -0,0 +1,9 @@ +import { defineConfig } from 'tsup'; + +export default defineConfig({ + entry: ['src/index.ts'], + format: ['esm', 'cjs'], + dts: true, + sourcemap: true, + clean: true, +}); diff --git a/src/app/op-log/core/lww-update-action-types.ts b/src/app/op-log/core/lww-update-action-types.ts index 24a569a94d..b361c47d16 100644 --- a/src/app/op-log/core/lww-update-action-types.ts +++ b/src/app/op-log/core/lww-update-action-types.ts @@ -1,20 +1,16 @@ -import { ENTITY_TYPES, EntityType, ActionType } from './operation.types'; +// LWW helper instance for Super Productivity. The lib at @sp/sync-core exposes +// a factory; the app supplies its own entity-type list (ENTITY_TYPES) so the lib +// stays domain-agnostic. -const LWW_UPDATE_SUFFIX = '] LWW Update'; +import { createLwwUpdateActionTypeHelpers } from '@sp/sync-core'; +import { ENTITY_TYPES } from '@sp/shared-schema'; +import type { EntityType } from './operation.types'; +import type { ActionType } from './action-types.enum'; -export const LWW_UPDATE_ACTION_TYPES: ReadonlySet = new Set( - ENTITY_TYPES.map((et) => `[${et}${LWW_UPDATE_SUFFIX}`), -); - -const LWW_ENTITY_MAP: ReadonlyMap = new Map( - ENTITY_TYPES.map((et) => [`[${et}${LWW_UPDATE_SUFFIX}`, et]), -); - -export const isLwwUpdateActionType = (actionType: string): boolean => - LWW_UPDATE_ACTION_TYPES.has(actionType); - -export const getLwwEntityType = (actionType: string): EntityType | undefined => - LWW_ENTITY_MAP.get(actionType); +const helpers = createLwwUpdateActionTypeHelpers(ENTITY_TYPES); +export const LWW_UPDATE_ACTION_TYPES = helpers.LWW_UPDATE_ACTION_TYPES; +export const isLwwUpdateActionType = helpers.isLwwUpdateActionType; +export const getLwwEntityType = helpers.getLwwEntityType; export const toLwwUpdateActionType = (entityType: EntityType): ActionType => - `[${entityType}${LWW_UPDATE_SUFFIX}` as ActionType; + helpers.toLwwUpdateActionType(entityType) as ActionType; diff --git a/src/app/op-log/core/operation.types.ts b/src/app/op-log/core/operation.types.ts index d28245c121..bbb84ad0d7 100644 --- a/src/app/op-log/core/operation.types.ts +++ b/src/app/op-log/core/operation.types.ts @@ -1,25 +1,34 @@ -import { VectorClock } from '../../core/util/vector-clock'; -import { ActionType } from './action-types.enum'; -import { EntityType as SharedEntityType, ENTITY_TYPES } from '@sp/shared-schema'; -export { VectorClock, ActionType, ENTITY_TYPES }; +// Generic, framework-agnostic primitives come from @sp/sync-core. +// Super Productivity-specific types (entity-type union, app action-type enum, +// sync-import reasons, repair payloads, full-state wrapper) live alongside this +// file in the app. -export enum OpType { - Create = 'CRT', - Update = 'UPD', - Delete = 'DEL', - Move = 'MOV', // For list reordering - Batch = 'BATCH', // For bulk operations (import, mass update) - /** Replaces entire app state from remote sync. All concurrent ops are discarded. See CLAUDE.md #12. */ - SyncImport = 'SYNC_IMPORT', - /** Replaces entire app state from backup file. All concurrent ops are discarded. See CLAUDE.md #12. */ - BackupImport = 'BACKUP_IMPORT', - /** Auto-repair operation containing full repaired state. */ - Repair = 'REPAIR', -} +import type { + VectorClock, + Operation as LibOperation, + OperationLogEntry as LibOperationLogEntry, + EntityChange as LibEntityChange, + EntityConflict as LibEntityConflict, + ConflictResult as LibConflictResult, + MultiEntityPayload as LibMultiEntityPayload, +} from '@sp/sync-core'; +import type { EntityType as SharedEntityType } from '@sp/shared-schema'; +import { ENTITY_TYPES } from '@sp/shared-schema'; +import { ActionType } from './action-types.enum'; + +export { + OpType, + FULL_STATE_OP_TYPES, + isFullStateOpType, + extractActionPayload, +} from '@sp/sync-core'; +import { isMultiEntityPayload as libIsMultiEntityPayload } from '@sp/sync-core'; +export type { VectorClock }; +export { ENTITY_TYPES, ActionType }; /** - * Entity type - identifies the kind of data entity being operated on. - * Re-exported from @sp/shared-schema to ensure client/server consistency. + * Entity type — Super Productivity's domain set, sourced from `@sp/shared-schema` + * so client and server agree on the union. */ export type EntityType = SharedEntityType; @@ -35,83 +44,14 @@ export type SyncImportReason = | 'SERVER_MIGRATION' | 'REPAIR'; -export interface Operation { - /** - * Unique identifier for the operation. - * Should be a UUID v7 (time-ordered) to allow for rough chronological sorting - * even without vector clocks, which aids in debugging and indexing. - */ - id: string; - - // ACTION MAPPING - /** - * The specific NgRx Action type (e.g., '[Task] Update'). - * Used to replay the operation against the store during application or testing. - * Must be a value from the ActionType enum to ensure type safety. - */ +/** + * Super Productivity's narrowed Operation type: tightens `actionType` and + * `entityType` to the app's enums and adds the optional `syncImportReason` + * field carried on full-state ops. + */ +export interface Operation extends Omit { actionType: ActionType; - - /** - * High-level operation category (Create, Update, Delete, etc.). - * Used for broad logic handling like persistence strategies or conflict resolution rules. - */ - opType: OpType; - - // SCOPE - /** - * The type of the data model entity being modified (e.g., 'TASK', 'PROJECT'). - */ entityType: EntityType; - - /** - * The specific ID of the entity being modified. - * Use '*' for singleton entities like global config. - */ - entityId?: string; - - /** - * List of entity IDs for batch operations that affect multiple items simultaneously. - */ - entityIds?: string[]; - - // DATA - /** - * The actual data change associated with the operation. - * - For 'CRT', this is the full object. - * - For 'UPD', this is a partial object (changeset). - * - For 'DEL', this might be empty or a tombstone. - * Validated by Typia at runtime. - */ - payload: unknown; - - // CAUSALITY & ORDERING - /** - * The ID of the device/client that generated this operation. - * Essential for vector clock management and identifying the source of changes. - */ - clientId: string; - - /** - * Represents the causal state of the world AFTER this operation was applied. - * Used to detect concurrency: if two ops have unordered vector clocks, they are concurrent. - * This is the primary mechanism for ensuring consistency across distributed devices. - */ - vectorClock: VectorClock; - - /** - * Wall clock time (epoch ms) from the **originating** device. - * - Used as a tie-breaker for concurrent operations (Last-Write-Wins logic). - * - NOT used for garbage collection or local maintenance. - */ - timestamp: number; - - // META - /** - * The schema version of the data at the time of operation creation. - * Allows the system to migrate or transform payloads if the data structure changes in the future. - */ - schemaVersion: number; - /** * Optional reason for full-state operations (SYNC_IMPORT, BACKUP_IMPORT, REPAIR). * Used in the conflict dialog to explain why the import was created. @@ -120,76 +60,43 @@ export interface Operation { syncImportReason?: SyncImportReason; } -export interface OperationLogEntry { - /** - * Local, monotonic auto-increment integer (IndexedDB primary key). - * Strictly orders operations as they arrived or were generated on THIS specific device. - */ - seq: number; - - /** - * The operation data itself (the synchronized part). - */ +export interface OperationLogEntry extends Omit { op: Operation; - - /** - * Local timestamp (epoch ms) indicating when this operation was written to the LOCAL database. - * - **Usage:** Strictly for local maintenance, such as garbage collection (compaction). - * - **Compaction:** Old entries are deleted based on `Date.now() - appliedAt > Retention`. - * - **Sync:** This value is NOT synchronized and implies nothing about the global order of events. - */ - appliedAt: number; - - /** - * Origin of the operation: - * - 'local': Generated by this device. - * - 'remote': Received from the sync server. - */ - source: 'local' | 'remote'; - - /** - * Timestamp (epoch ms) when this operation was successfully acknowledged by the remote server. - * - Null/Undefined if the operation is still pending upload. - * - Used to determine which operations are safe to compact (must be synced first). - */ - syncedAt?: number; - - /** - * Timestamp (epoch ms) if the operation was rejected during conflict resolution. - * Effectively marks the operation as "dead" but kept for history/debugging. - */ - rejectedAt?: number; - - /** - * For remote ops only: tracks whether the op was successfully applied to the local NgRx store. - * - 'pending': Stored in DB but not yet dispatched to state (e.g., during initial download). - * - 'applied': Successfully dispatched to NgRx. - * - 'failed': Attempted to apply but failed (e.g., missing dependency). Will be retried on startup. - * Used for crash recovery: on startup, any 'pending' or 'failed' remote ops are re-dispatched. - */ - applicationStatus?: 'pending' | 'applied' | 'failed'; - - /** - * For 'failed' ops: number of retry attempts. - * After MAX_CONFLICT_RETRY_ATTEMPTS, the op is marked as rejected. - */ - retryCount?: number; } -export interface EntityConflict { +export interface EntityChange extends Omit { entityType: EntityType; - entityId: string; - localOps: Operation[]; // Local ops affecting this entity - remoteOps: Operation[]; // Remote ops affecting the same entity - suggestedResolution: 'local' | 'remote' | 'merge' | 'manual'; - mergedPayload?: unknown; // If auto-mergeable } -export interface ConflictResult { +export interface EntityConflict extends Omit< + LibEntityConflict, + 'entityType' | 'localOps' | 'remoteOps' +> { + entityType: EntityType; + localOps: Operation[]; + remoteOps: Operation[]; +} + +export interface ConflictResult extends Omit< + LibConflictResult, + 'nonConflicting' | 'conflicts' +> { nonConflicting: Operation[]; conflicts: EntityConflict[]; } +export interface MultiEntityPayload extends Omit { + entityChanges: EntityChange[]; +} + +/** + * SP-narrowed type guard that mirrors `@sp/sync-core`'s `isMultiEntityPayload` + * but narrows to the app's `MultiEntityPayload` (with the SP entity-type union). + * The runtime check is identical; only the inferred type changes. + */ +export const isMultiEntityPayload = (payload: unknown): payload is MultiEntityPayload => + libIsMultiEntityPayload(payload); + /** * Minimal summary of repairs performed, used in REPAIR operation payload. * Keeps repair log lightweight while providing debugging info. @@ -212,26 +119,6 @@ export interface RepairPayload { repairSummary: RepairSummary; } -// ============================================================================= -// FULL-STATE OPERATION PAYLOADS -// ============================================================================= - -/** - * OpTypes that contain full application state in their payload. - * Used for type guards and validation. - */ -export const FULL_STATE_OP_TYPES = new Set([ - OpType.SyncImport, - OpType.BackupImport, - OpType.Repair, -]); - -/** - * Type guard to check if an operation is a full-state operation. - */ -export const isFullStateOpType = (opType: OpType | string): boolean => - FULL_STATE_OP_TYPES.has(opType as OpType); - /** * Legacy wrapper format for full-state payloads. * Some older code wrapped the state in { appDataComplete: ... }. @@ -256,12 +143,6 @@ export const isWrappedFullStatePayload = ( /** * Extracts the raw application state from a full-state operation payload. * Handles both wrapped ({ appDataComplete: ... }) and unwrapped formats. - * - * IMPORTANT: This should be used when uploading snapshots to ensure - * consistent format in sync files. - * - * @param payload - The operation payload (wrapped or unwrapped) - * @returns The raw application state */ export const extractFullStateFromPayload = ( payload: unknown, @@ -269,17 +150,12 @@ export const extractFullStateFromPayload = ( if (isWrappedFullStatePayload(payload)) { return payload.appDataComplete; } - // Unwrapped format - payload IS the state return payload as Record; }; /** * Validates that a full-state payload has the expected structure. * Throws an error if the payload is malformed. - * - * @param payload - The payload to validate - * @param context - Description of where this is being called from (for error messages) - * @throws Error if payload is not a valid full-state payload */ export const assertValidFullStatePayload: ( payload: unknown, @@ -293,7 +169,6 @@ export const assertValidFullStatePayload: ( ); } - // Check for expected top-level properties (not exhaustive, just key ones) const expectedKeys = ['task', 'project', 'tag', 'globalConfig']; const hasExpectedKeys = expectedKeys.some((key) => key in state); @@ -305,76 +180,3 @@ export const assertValidFullStatePayload: ( ); } }; - -// ============================================================================= -// MULTI-ENTITY OPERATIONS -// ============================================================================= - -/** - * Represents a single entity change within a multi-entity operation. - * Captures the exact changes made to one entity as part of an atomic operation. - */ -export interface EntityChange { - /** - * The type of entity being changed. - */ - entityType: EntityType; - - /** - * The ID of the entity being changed. - */ - entityId: string; - - /** - * The type of change (Create, Update, Delete). - */ - opType: OpType; - - /** - * The actual changes: - * - For Create: Full entity object - * - For Update: Partial object with only changed fields - * - For Delete: Minimal tombstone { id: string } - */ - changes: unknown; -} - -/** - * Payload wrapper for multi-entity operations. - * Contains the original action payload plus all entity changes computed from state diff. - */ -export interface MultiEntityPayload { - /** - * The original action payload (for replaying the action on remote clients). - */ - actionPayload: Record; - - /** - * All entity changes that resulted from this action. - * Computed by diffing state before and after the action. - */ - entityChanges: EntityChange[]; -} - -/** - * Type guard to check if a payload is a multi-entity payload. - */ -export const isMultiEntityPayload = (payload: unknown): payload is MultiEntityPayload => { - return ( - typeof payload === 'object' && - payload !== null && - 'entityChanges' in payload && - Array.isArray((payload as MultiEntityPayload).entityChanges) - ); -}; - -/** - * Extracts the action payload from an operation payload. - * Handles both multi-entity payloads (new format) and legacy payloads. - */ -export const extractActionPayload = (payload: unknown): Record => { - if (isMultiEntityPayload(payload)) { - return payload.actionPayload; - } - return payload as Record; -}; diff --git a/src/app/op-log/core/sync-state-corrupted.error.ts b/src/app/op-log/core/sync-state-corrupted.error.ts index 23fd9312b6..1683e8fcf0 100644 --- a/src/app/op-log/core/sync-state-corrupted.error.ts +++ b/src/app/op-log/core/sync-state-corrupted.error.ts @@ -1,47 +1,2 @@ -/** - * Error thrown when the sync state is corrupted and a full re-sync is required. - * - * ## Design Philosophy: Fail Fast, Re-sync Clean - * - * When an operation cannot be applied due to missing dependencies, rather than - * attempting complex retry logic with queues, we fail immediately and signal - * that a full re-sync is needed. - * - * ### Why This Approach? - * - * 1. **Simplicity**: No retry queues, no failed operation tracking, no pruning logic - * 2. **Correctness**: A full re-sync guarantees consistent state - * 3. **Debuggability**: Clear error with context about what went wrong - * 4. **User Experience**: Better to re-sync cleanly than live with subtle inconsistencies - * - * ### When Is This Thrown? - * - * - When applying remote operations and a hard dependency is missing - * - This indicates either: - * - Operations arrived out of order (protocol issue) - * - Local state is corrupted - * - A bug in dependency tracking - * - * ### How Should Callers Handle This? - * - * The caller (OperationLogSyncService) catches this error, marks the operations - * as failed, and should trigger a full re-sync to restore consistent state. - */ -export class SyncStateCorruptedError extends Error { - constructor( - message: string, - public readonly context: { - opId: string; - actionType: string; - missingDependencies: string[]; - }, - ) { - super(message); - this.name = 'SyncStateCorruptedError'; - - // Maintains proper stack trace for where error was thrown (V8 engines) - if (Error.captureStackTrace) { - Error.captureStackTrace(this, SyncStateCorruptedError); - } - } -} +// Re-exported from @sp/sync-core. Source of truth lives in packages/sync-core/src/sync-state-corrupted.error.ts. +export { SyncStateCorruptedError } from '@sp/sync-core'; diff --git a/src/app/op-log/core/types/apply.types.ts b/src/app/op-log/core/types/apply.types.ts index 24f74ff029..2e05ac3840 100644 --- a/src/app/op-log/core/types/apply.types.ts +++ b/src/app/op-log/core/types/apply.types.ts @@ -1,36 +1,23 @@ -import { Operation } from '../operation.types'; +// App-narrowed apply types. The lib's @sp/sync-core ships generic versions; the +// app uses its own Operation type so callers see the SP-narrowed entityType / +// actionType unions and the syncImportReason field. + +import type { Operation } from '../operation.types'; -/** - * Result of applying operations to the NgRx store. - * - * This allows callers to handle partial success scenarios where some operations - * were applied before an error occurred. - */ export interface ApplyOperationsResult { - /** - * Operations that were successfully applied to the NgRx store. - * These ops have already been dispatched and should be marked as applied. - */ + /** Operations that were successfully applied. */ appliedOps: Operation[]; - - /** - * If an error occurred, this contains the failed operation and the error. - * Operations after this one in the batch were NOT applied. - */ failedOp?: { op: Operation; error: Error; }; } -/** - * Options for applying operations to the NgRx store. - */ export interface ApplyOperationsOptions { /** - * When true, skip archive handling (already persisted from original execution). - * Use ONLY for local hydration where operations are replaying - * previously validated local operations from SUP_OPS. + * When true, skip side effects that would normally fire on first application + * (e.g. writing to archive storage) — used when replaying already-persisted + * local operations during hydration. */ isLocalHydration?: boolean; } diff --git a/src/app/op-log/util/entity-key.util.ts b/src/app/op-log/util/entity-key.util.ts index c039b8c8b0..4a10099a00 100644 --- a/src/app/op-log/util/entity-key.util.ts +++ b/src/app/op-log/util/entity-key.util.ts @@ -1,31 +1,18 @@ -import { EntityType } from '../core/operation.types'; +import { + parseEntityKey as parseLibEntityKey, + toEntityKey as toLibEntityKey, +} from '@sp/sync-core'; +import type { EntityType } from '../core/operation.types'; -/** - * Creates a unique key for an entity by combining its type and ID. - * Used for indexing and looking up entities across the operation log system. - * - * @param entityType The type of the entity (e.g., 'TASK', 'PROJECT') - * @param entityId The unique ID of the entity - * @returns A composite key in the format "ENTITY_TYPE:entityId" - */ export const toEntityKey = (entityType: EntityType, entityId: string): string => - `${entityType}:${entityId}`; + toLibEntityKey(entityType, entityId); -/** - * Parses an entity key back into its components. - * - * @param key The composite entity key - * @returns An object containing entityType and entityId - */ export const parseEntityKey = ( key: string, ): { entityType: EntityType; entityId: string } => { - const colonIndex = key.indexOf(':'); - if (colonIndex === -1) { - throw new Error(`Invalid entity key format: ${key}`); - } + const parsed = parseLibEntityKey(key); return { - entityType: key.substring(0, colonIndex) as EntityType, - entityId: key.substring(colonIndex + 1), + entityType: parsed.entityType as EntityType, + entityId: parsed.entityId, }; }; diff --git a/src/tsconfig.spec.json b/src/tsconfig.spec.json index ce86a563b3..b200e1872a 100644 --- a/src/tsconfig.spec.json +++ b/src/tsconfig.spec.json @@ -6,7 +6,8 @@ "types": ["jasmine", "node"], "paths": { "@super-productivity/plugin-api": ["packages/plugin-api/src/index.ts"], - "@sp/shared-schema": ["packages/shared-schema/src/index.ts"] + "@sp/shared-schema": ["packages/shared-schema/src/index.ts"], + "@sp/sync-core": ["packages/sync-core/src/index.ts"] } }, "files": ["test.ts", "polyfills.ts"], diff --git a/tsconfig.base.json b/tsconfig.base.json index 1bd99a88f5..8f498cc4a7 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -25,7 +25,8 @@ "lib": ["es2022", "dom"], "paths": { "@super-productivity/plugin-api": ["packages/plugin-api/src/index.ts"], - "@sp/shared-schema": ["packages/shared-schema/src/index.ts"] + "@sp/shared-schema": ["packages/shared-schema/src/index.ts"], + "@sp/sync-core": ["packages/sync-core/src/index.ts"] }, "plugins": [ {