Commit graph

20428 commits

Author SHA1 Message Date
Johannes Millan
e4f9a4e2e5 refactor(sync-providers): extract local file provider 2026-05-13 11:36:19 +02:00
johannesjo
0b597a1f44 Merge branch 'task/on-production-server-8e6dbb' 2026-05-13 02:12:36 +02:00
johannesjo
ca660a0585 fix(supersync): harden production server deploy 2026-05-13 02:10:46 +02:00
johannesjo
058b10033c docs(sync): update extraction plan after supersync slice 2026-05-13 02:04:41 +02:00
johannesjo
e03a4d41bd fix(sync-providers): retry supersync rate limit uploads 2026-05-13 01:52:56 +02:00
johannesjo
3db24a7b50 fix(supersync): harden requestId, 429 surface, and native error logging
Four improvements landed on top of the slice:

1. **Encrypted payloads excluded from requestId fingerprint.**
   AES-GCM uses fresh IVs on every encryption, so the same logical
   op produces different ciphertext each retry. Hashing the raw
   payload would compute a different requestId on every retry,
   defeating server-side idempotency for encryption-enabled
   accounts. `_toRequestIdFingerprintOp` substitutes a fixed
   `'[encrypted-payload]'` sentinel when `op.isPayloadEncrypted`,
   so retries of the same logical op hash to the same id even
   after re-encryption. New spec call (#5/#6) verifies same
   `requestId` for the same op with different ciphertext.

2. **429 retry-delay extraction.** SuperSync's server returns
   Fastify's standard rate-limit shape (`{ statusCode: 429, code:
   'FST_TOO_MANY_REQUESTS', message: 'Rate limit exceeded, retry
   in 5 minutes' }`). `_extractRetryDelayReason` parses the
   `message` field and threads "retry in N minutes/seconds" into
   the thrown `HTTP 429 …` error message so users see actionable
   wait info. Spec updated to assert the full message.

3. **Native logger meta hardened.** `_handleNativeRequestError`
   and `_doWebFetch`'s catch previously logged `error:
   error.message`, which on iOS/Android TLS-cert errors could
   embed the resolved hostname into the exportable log. Replace
   with `_getSafeErrorLogMeta` (uses `toSyncLogError` from
   `@sp/sync-core`) which emits only `errorName` + optional
   `errorCode` — both safe primitives. TLS-cert spec extended to
   assert the logger meta also doesn't leak the hostname.

4. **`unable to connect` added to retryable patterns.** The
   user-facing native rewrite message ("Unable to connect to
   SuperSync server. …") is now recognized as retryable by the
   broad-pattern matcher, so upstream retry loops can retry the
   replacement message itself. Spec added.

Plus minor structural improvements: `SuperSyncHttpStatusError`
now carries `status: number`; the user-facing native message is a
named constant (`RETRYABLE_NATIVE_REQUEST_MESSAGE`); the
non-Error fallback throw at end of `_handleNativeRequestError`
scrubs via `toSyncLogError` too.

Package spec count: 277 (no count delta — existing tests extended).
Bundle: CJS 97.92 KB / ESM 95.12 KB (negligible delta).
2026-05-13 01:28:14 +02:00
johannesjo
b28ff1b571 Merge branch 'master' into task/handover-pr-5-next-slice-supersync-751252
Brings in master's `30da81ea5 fix supersync retry idempotency`,
which landed after this branch was created. The server-side files
merge cleanly: `super-sync-server/src/sync/sync.routes.ts` and
`sync.service.ts` now recognize the new `requestId` field for
idempotent upload replays, with two new server specs.

Two client-side conflicts, both resolved by keeping the branch
version because the slice already ports master's additions to the
package side:

- `src/app/op-log/sync-providers/super-sync/super-sync.ts`
  (content conflict). Master added the requestId helpers
  (`_createOpsUploadRequestId`, `_compactRequestIdPart`,
  `_hashRequestIdInput`, `_stableJsonStringify`,
  `_toStableJsonValue`) and the `OPS_UPLOAD_REQUEST_ID_PREFIX`
  constant to the old class-based file. The branch had replaced
  that file with the `createSuperSyncProvider()` factory shim and
  ported the helpers into the package class in commit fbefaa343.
  Resolution: keep the factory shim (ours).

- `src/app/op-log/sync-providers/super-sync/super-sync.spec.ts`
  (modify/delete). Master added two specs asserting the `requestId`
  format and stability. The branch deleted the file when the
  package's Vitest spec took over. The equivalent specs are already
  in `packages/sync-providers/tests/super-sync/super-sync.spec.ts`
  (committed in fbefaa343). Resolution: keep the deletion (ours).

Post-merge verification:
- `npm run sync-providers:test` → 277/277
- `npm run sync-providers:build` → CJS 97.92 KB / ESM 95.12 KB
- `npm run lint` → clean
- App imex/sync spec sweep: 381/381
2026-05-13 01:07:34 +02:00
johannesjo
a96e446479 refactor(supersync): tag native HTTP-status throws with an internal class
The post-review fix in 8d7e05cce used `error.message.startsWith('HTTP ')`
in `_handleNativeRequestError` to detect our own thrown HTTP errors
(so they propagate unchanged on the non-retryable native path).
The check is functionally correct but brittle — any foreign error
whose message coincidentally begins with "HTTP " would also pass
through, leaking whatever followed.

Replace with a file-local `SuperSyncHttpStatusError extends Error`
class. The web and native non-2xx branches throw this class; the
handler's instanceof check is now robust against foreign-message
collisions. The error class is intentionally NOT exported — its only
consumer is the catch block in `_doNativeFetch`.

Regression spec added: a foreign error with message
`"HTTP request from sync.example.com refused at 10.0.0.5"` gets
scrubbed to the name-only form. The hostname and IP don't leak to
the user-facing surface.

Bundle: CJS 97.92 KB / ESM 95.12 KB (negligible delta from the
class declaration). Package spec count: 277.
2026-05-13 01:02:40 +02:00
johannesjo
fbefaa3437 fix(supersync): port requestId idempotency missed in slice rebase
Master shipped commit 30da81ea5 ("fix supersync retry idempotency")
after this slice's worktree was created from 88961485f, so the
package class was missing the deterministic `requestId` on every
`uploadOps` payload. Without it the SuperSync server can't recognize
a retried upload (e.g. after a network drop between server commit
and client receipt) and rejects the replay as duplicate ops instead
of returning the cached response/piggybacked ops.

Port the four private helpers (`_createOpsUploadRequestId`,
`_compactRequestIdPart`, `_hashRequestIdInput`, `_stableJsonStringify`
+ `_toStableJsonValue`) and the `OPS_UPLOAD_REQUEST_ID_PREFIX`
constant into the package class, and add `requestId` to the
`uploadOps` JSON payload. All helpers are pure functions with no
external deps — no new ports required.

Port master's Vitest spec: assert the prefix/length on the existing
"uploads operations successfully" test, and add a dedicated
"stable requestId across retries and key-reordering" test that
exercises five batches and asserts the equivalence/divergence
classes (retry → same id, key reorder → same id, content change →
different id, batch resize → different id).

Bundle: CJS 97.79 KB / ESM 94.99 KB (+2.6 KB from the helpers).
Package spec count: 276 (was 275).

Caught by codex during post-slice review.
2026-05-13 00:50:17 +02:00
johannesjo
8d7e05cceb fix(supersync): plug residual gaps surfaced by post-slice review
Two post-merge findings from the careful review pass on the
SuperSync slice (PR 7b):

1. **Storage NaN bug.** The app-side `localStorage`-backed
   `SuperSyncStorage.getLastServerSeq` returned `NaN` for an empty-
   string value (`parseInt("", 10) === NaN`), which the package's
   `stored ?? 0` fallback could not catch (nullish coalescing only
   triggers on `null`/`undefined`). The original implementation
   used `stored ? parseInt(stored, 10) : 0` and returned `0` for
   any falsy string. Add an explicit `Number.isFinite` check in
   the adapter so the port contract (returns `number | null`) is
   actually honored. Doesn't happen in practice today — `setItem`
   always writes a stringified int — but a hypothetical regression
   that wrote `""` would silently break sync replay.

2. **`_handleNativeRequestError` residual hostname leak.** The
   non-retryable native path re-threw the original error
   unchanged. Foreign errors from the native HTTP executor (e.g.
   iOS TLS cert errors like "SSL certificate problem: hostname
   mismatch for sync.example.com") can embed the resolved
   hostname in `.message`, which surfaces in user-visible snack
   notifications via `super-sync-restore.service.ts` and
   similar. Scrub to a name-only form
   (`SuperSync native request failed: <Error.name>`) while
   preserving identity for our own thrown errors
   (`AuthFailSPError`, `MissingCredentialsSPError`,
   `HTTP <status>` form). The full message is still captured in
   the structured log meta for diagnostics.

Two new Vitest specs cover the scrub path and the
`AuthFailSPError` identity preservation through the non-retryable
native branch. Package spec count: 275 (was 273).
2026-05-13 00:45:41 +02:00
johannesjo
c40feef6d2 refactor(sync-providers): move SuperSync provider into package
Move SuperSync's provider class, model, and spec from
`src/app/op-log/sync-providers/super-sync/` into
`packages/sync-providers/src/super-sync/` behind the ports
established by slices 5-6 (logger, platform-info, web-fetch,
native-http-executor, credential-store) plus two new ports for this
slice: `SuperSyncStorage` (narrow, three methods for `lastServerSeq`
persistence) and `SuperSyncResponseValidators` (host-side wrapper
that keeps `@sp/shared-schema` out of the package boundary).

Privacy fixes applied inline per multi-review consensus:

- `AuthFailSPError(reason, body)` now only takes the extracted
  reason — body is no longer retained on `additionalLog`.
- `_handleNativeRequestError` drops the parenthesised
  `(${errorMessage})` interpolation from the user-facing message
  to avoid leaking hostname/URL on native-stack errors.
- Timeout `Error.message` drops the `path` interpolation
  (`excludeClient` clientId was leaking into the thrown message).
- `_extractServerErrorReason` caps the extracted server reason at
  80 chars to defend against future server contract drift.
- Thrown non-2xx errors use a fixed `HTTP <status> <statusText> —
<reason?>` form (web) or `HTTP <status> — <reason?>` (native)
  instead of embedding the raw response body in `Error.message`.

JSDoc invariants pinned on `getEncryptKey`, `getWebSocketParams`,
`_cachedServerSeqKey`, and `deleteAllData` response shape.

Compression switched to direct `@sp/sync-core` import (no
`CompressError` wrapping at SuperSync's call sites; the existing
app-side compression-handler still wraps for
`EncryptAndCompressHandlerService`).

Spec converted Jasmine → Vitest. `TestableSuperSyncProvider`
subclass gone — native-platform routing now tested via injected
`platformInfo` + `nativeHttpExecutor` mocks. Adds a new
`privacy regression: no user content in error/log surface`
describe block driven by an HTTP-error path with a body containing
plausible user content (taskId/title/accessToken).

App-side `super-sync.ts` collapses to a 50-line
`createSuperSyncProvider()` factory (no `extraPath` arg —
SuperSync explicitly ignored it). The `SyncProviderId.SuperSync`
enum stays app-side; `AssertSuperSyncId` conditional pins
enum-vs-constant drift at compile time. `super-sync.model.ts`
becomes a re-export shim. `sync-providers.factory.ts` updated.

`super-sync-restore.service.ts` narrows the package's
`RestorePoint<string>` return through the consensus "narrow at the
app shim" decision (single cast at the call boundary).

Bundle: CJS 95.15 KB / ESM 92.34 KB (up from 76.78 / 74.18). Right
in the performance reviewer's 95-100 KB estimate; under the
original 110 KB projection. Tiered barrel split remains a
documented deferral.

Package spec count: 273 (was 197, added 76 SuperSync specs).
All targeted app specs green: sync-wrapper (107), op-log (2513
across the directory), imex/sync (381).
2026-05-13 00:30:19 +02:00
johannesjo
30da81ea5a fix supersync retry idempotency 2026-05-13 00:14:35 +02:00
johannesjo
a2509cb8ac refactor(sync-providers): promote isRetryableUploadError helper
Move the broad-pattern transient-error matcher from
`src/app/op-log/sync/sync-error-utils.ts:96` into
`packages/sync-providers/src/http/retryable-upload-error.ts` as
`isRetryableUploadError`. Distinct from the package's existing
`isTransientNetworkError` at `http/native-http-retry.ts:136`, which
checks native-platform error codes for the CapacitorHttp retry
path; the two coexist because they answer different questions
(retry the native HTTP failure? vs retry the server-returned error
string?).

The name was renamed during promotion (per multi-review consensus)
to avoid the "discriminate on input shape" smell that the
input-typed names (`isTransientErrorMessage` vs
`isTransientNetworkError`) would have produced. The intent-anchored
form makes the call-site decision explicit.

App-side `sync-error-utils.ts` keeps its current export name
(`isTransientNetworkError`) as an alias re-export so
`operation-log-upload.service.ts:31, 166` continues to import
unchanged. No behavior change.

Sets up PR 7b — SuperSync (in the package) imports
`isRetryableUploadError` directly.
2026-05-13 00:07:09 +02:00
johannesjo
3d0ff2aec7 docs(sync-core-plan): record SuperSync slice multi-review consensus
Folds the six-Claude-lens (correctness, security/privacy, architecture,
alternatives, performance, simplicity) review pass into the SuperSync
slice design doc. Closes all 8 open questions in-doc, records new
blockers the original draft undercounted, and trims the body per the
simplicity reviewer's recommendations.

Key revisions:
- Storage port is narrow `SuperSyncStorage` (3 methods), not the
  generic `KeyValueStoragePort` originally proposed. Architecture
  and alternatives both pushed back.
- Promoted helper renamed to `isRetryableUploadError` (intent-
  anchored) to avoid naming collision with package's existing
  `isTransientNetworkError`.
- Factory drops `extraPath` — SuperSync explicitly ignores it.
- Privacy A1 fix uses extracted-reason form (via
  `_extractServerErrorReason`, capped at 80 chars), not blanket
  body-drop — preserves 5xx debug context.

Four new privacy blockers added that the original sweep missed:
- `AuthFailSPError(reason, body)` retains body via
  `AdditionalLogErrorBase.additionalLog` (security + correctness
  both flagged independently — PR 5b did NOT scrub this for
  SuperSync's call site).
- `_handleNativeRequestError` re-throw embeds raw `errorMessage`
  (can leak hostname/URL from native-stack `.message`).
- Timeout `Error.message` embeds `path` with `excludeClient` query
  param (pseudonymous clientId).
- `_extractServerErrorReason` returns server `error` field uncapped.

Dissents recorded: alternatives reviewer preferred pre-split spec
before move, moving CompressError/DecompressError into sync-core
for strict behavior preservation, and barrel split as PR 7d.
2026-05-12 23:58:58 +02:00
johannesjo
6a1f569a09 docs(sync-core-plan): draft SuperSync slice design for multi-review
Captures the SuperSync provider move (slice 7) design surface before
implementation: which files move into @sp/sync-providers, which stay
app-side, which ports get reused from prior slices, which new ports
this slice introduces, the privacy sweep checklist, the suggested
commit shape, and the open questions for parallel multi-review.

Key design calls flagged for review:
- response-validators.ts stays app-side (banned @sp/shared-schema
  import). New SuperSyncResponseValidators port.
- localStorage access becomes a KeyValueStoragePort.
- isTransientNetworkError promoted from app sync-error-utils to the
  package as isTransientErrorMessage (distinct from the package's
  existing native-error-code-aware version).
- Compression imported directly from @sp/sync-core; CompressError
  wrapping dropped at SuperSync call sites (no instanceof catches
  exist).
- Two privacy A1 sites identified: _doNativeFetch:646-648 and
  _doWebFetch:582 embed response body in thrown Error.message; fix
  via fixed status-only message.
- Spec migration kept monolithic for the move (1553 lines), split
  deferred to a follow-up.

Multi-review consensus block left blank for the reviewer pass.
2026-05-12 23:50:12 +02:00
johannesjo
173218c151 docs(sync-core-plan): summarize Sixth Slice and renumber remaining
Folds the WebDAV + Nextcloud slice notes into the long-term plan in
the same format as prior slices and renumbers the Remaining Slice
Plan so SuperSync becomes slice 1 (next) and LocalFile slice 2.

Captures the consensus-driven decisions from the WebDAV slice
(port reuse instead of new `WebDavNativeHttpExecutor`, no-retry
preservation, monolithic spec move, Nextcloud generic widened,
`md5HashSync` → `hash-wasm` async, CORS heuristic tightened,
inline `registerPlugin` dropped) plus follow-up commit history
(namespace/server-format specs, xmldom devDeps move, CORS pattern
broadening).

Records the bundle-size delta (75.77 KB CJS / 73.23 KB ESM after
the slice) and the architectural deferrals (parser O(n) walk,
LocalFile's `md5HashPromise`).

Includes a more concrete plan for the SuperSync slice: port reuse,
response-validators staying app-side because of the
\`@sp/shared-schema\` boundary ban, localStorage storage port,
\`isTransientNetworkError\` promotion path, compression direct
\`@sp/sync-core\` import, and the privacy-sweep starting points.
2026-05-12 23:47:05 +02:00
Johannes Millan
88961485fd fix(supersync): harden deploy database migration 2026-05-12 23:24:13 +02:00
Johannes Millan
0c93e8492c test(sync): speed up slow sync specs 2026-05-12 22:57:00 +02:00
Johannes Millan
e0cc783bba Merge branch 'master' into feat/carefully-review-all-super-sync-related-9daf6d
* master:
  fix(sync): restore InvalidFilePrefixError privacy log at app-shim site
  test(sync-providers): restore WebDAV parser namespace + server-format specs
  refactor(sync-providers): @xmldom/xmldom to devDeps + global DOMParser
  refactor(sync-providers): broaden WebDAV CORS heuristic for real browsers
  refactor(sync-providers): testWebdavConnection helper, drop adapter+api from barrel
  refactor(sync-providers): tighten WebDAV slice after multi-review
  refactor(sync-providers): move WebDAV + Nextcloud providers into package
  docs(sync-core-plan): fold Gemini review dissent into consensus
  refactor(sync-providers): promote shared log helpers
  docs(sync-core-plan): record WebDAV slice multi-review consensus
  docs(sync-core-plan): draft WebDAV slice design for multi-review
  docs(sync-core-plan): summarize Fifth Slice and renumber remaining
  fix(sync): harden supersync server edge cases
  fix reviewed calendar and duplicate edge cases
  test(electron): align TODAY filter spec with isTodayWithOffset impl
  refactor(sync-providers): tighten Dropbox port after review
  refactor(sync-providers): move Dropbox provider into package
  refactor(sync-providers): move provider error classes

# Conflicts:
#	packages/super-sync-server/src/sync/services/operation-download.service.ts
#	packages/super-sync-server/src/sync/sync.routes.ts
2026-05-12 22:35:37 +02:00
Johannes Millan
37aedd1b8b perf(supersync): minimize users row-lock window in uploadOps
W1: move the atomic storage-counter UPDATE to be the LAST statement
before COMMIT in uploadOps's $transaction. PostgreSQL takes a
row-level write lock on `users` at UPDATE and holds it through
COMMIT, so any concurrent uploadOps from the same user blocks on
that row for the rest of the transaction's window. Previously the
UPDATE ran before syncDevice.upsert, widening the lock window
unnecessarily.
2026-05-12 22:26:02 +02:00
Johannes Millan
29da578760 fix(supersync): order cleanup reconcile stalest-first and add C1 regression tests
S1: replace the Fisher-Yates random shuffle in cleanup.ts with a
deterministic orderBy snapshotAt asc at the source. When the affected
user list exceeds the reconcile budget, the most-drifted users are
reconciled first; the freshest tail rolls over to the next pass. No
more Math.random, no probabilistic starvation, and the test stub for
Math.random goes away.

C1 regression tests: add coverage that proves the prototype-pollution
guard works for CRT, UPD, MOV, BATCH single-entity, and BATCH entities
map. Each test asserts state.TASK's prototype is still Object.prototype
and the malicious payload doesn't leak via the prototype chain.
2026-05-12 22:25:18 +02:00
Johannes Millan
8a8ba77cf4 fix(sync): restore InvalidFilePrefixError privacy log at app-shim site
Pre-existing test debt from PR 5a (already on master). The original
`AdditionalLogErrorBase` constructor logged invalid-prefix details
via `OpLog.log` at construction time with a key-summary string
containing `inputLength=N, expectedPrefix="...", ...`. PR 5a moved
the class to `@sp/sync-providers` and intentionally dropped that
constructor-time logging side effect — privacy responsibility
shifts to catch-site logging. The package error class no longer
fires `OpLog.log` at construction.

That left `sync-file-prefix.spec.ts` failing on master: it spies on
`OpLog.log` and asserts the call log contains `'inputLength'`
(without the raw sync payload). The privacy contract is real and
worth keeping; only the responsibility for firing the log call
moved.

Fix the app shim `createInvalidPrefixError` callback to log
structured details via `OpLog.log` before constructing the error.
The callback only ever receives the `details` object built by the
package helper (`expectedPrefix`, `endSeparator`, `inputLength`) —
the raw sync payload never reaches it, so the privacy invariant
holds. The first-arg string inlines the key names so the test's
`calls.allArgs().flat().join('\n')` assertion finds 'inputLength'
even after the second-arg object gets stringified positionally.

Verification
- `npm run test:file src/app/op-log/util/sync-file-prefix.spec.ts`
  → 1/1 SUCCESS.
- `npm run checkFile` on the modified file → clean.
2026-05-12 22:22:42 +02:00
Johannes Millan
15fbf2c140 test(sync-providers): restore WebDAV parser namespace + server-format specs
Multi-review (W4) flagged that the Vitest spec migration dropped a
chunk of namespace / server-quirk coverage that the original Karma
specs protected. The NS-aware `getElementsByTagNameNS('*', name)`
lookup is the only reason mixed-prefix and no-prefix DAV responses
parse uniformly — so a future "let's use getElementsByTagName"
cleanup that breaks Apache mod_dav or ownCloud responses must fail
loudly.

Add 13 specs to webdav-xml-parser.spec.ts (164 → 177 total package
tests). Coverage restored:

Namespace quirks:
- Lowercase `d:` prefix (the standard case)
- No-prefix namespace (Apache mod_dav style)
- Mixed prefixes in one document (DAV: + lp1:, both bound to DAV:)
- ownCloud / Nextcloud custom namespaces (`oc:`, `nc:`) alongside DAV:

Server-specific formats:
- IIS-style multistatus with absolute hrefs (`http://host:port/path`)
- Nginx mod-dav with HTTP/1.0 status line (our `.includes('200 OK')`
  check is version-agnostic by design)
- 207 partial-failure response with mixed 200 / 403 propstat entries
  (only the 200 entry should be parsed)

parseXmlResponseElement null returns:
- Missing href in <response>
- Missing propstat in <response>

HTML response detection (extra cases):
- HTML with explicit charset DOCTYPE declaration
- JSON content NOT flagged as HTML
- Plain XML starting with `<` but not `<html` NOT flagged
- The Nextcloud "There is nothing here, sorry" anti-pattern detected
  mid-string

Test count: 164 → 177 (+13). Full slice coverage now stands at
54 webdav-specific specs in the package (was 53 going into PR 6b).

Other invariants the original Karma specs covered that are NOT
restored here (deferred as lower-priority; will surface via the
remaining slice E2E pass against a real Nextcloud/WebDAV server):
- ETag vs Last-Modified preference / fallback ordering
- Empty content-length defaulting to 0
- Encoded-URL round-trip in href
2026-05-12 22:22:42 +02:00
Johannes Millan
a11f27cf43 refactor(sync-providers): @xmldom/xmldom to devDeps + global DOMParser
Multi-review (W3) flagged that `@xmldom/xmldom` was a runtime
dependency only because the package's vitest env (Node) lacks the
DOM `DOMParser`. Browsers, Electron, and Capacitor WebViews all
provide their own `DOMParser` — so the dep was shipping into the
host's transitive bundle just to support the package's unit tests.

Move it to `devDependencies` and:

- Use `globalThis.DOMParser` at runtime in `webdav-xml-parser.ts`.
  Added a minimal `declare const DOMParser: { new(): {
  parseFromString(text, mimeType): XmlNodeLike } }` so the package's
  strict TS still type-checks without DOM lib pulling in via
  xmldom's `/// <reference lib="dom" />`.
- Polyfill `globalThis.DOMParser` from `@xmldom/xmldom` in a new
  `tests/setup-dom-parser.ts` vitest setup file. Vitest config adds
  it to `setupFiles`.
- Drop the explicit `import { DOMParser } from '@xmldom/xmldom'` from
  the parser source.

Side effect: the package's `tsconfig.json` now sets `lib: ["ES2022",
"DOM"]`. Previously the `/// <reference lib="dom" />` directive
inside `@xmldom/xmldom`'s shipped `index.d.ts` was transparently
pulling DOM lib in for our TS DTS compilation (covering `Response`,
`RequestInit`, `URL`, `URLSearchParams`, `btoa`, etc.). Removing the
xmldom import broke that chain — the package code uses those DOM
globals throughout, so adding `DOM` to `lib` is the explicit fix.

Bundle impact
- Package's own ESM bundle: 73.23 KB (was 70.93 KB; +2.3 KB from
  PR 6b's test-connection helper, not xmldom — xmldom was already
  marked external by tsup since it was a dep, so the package bundle
  never carried it).
- Host bundle savings: `@xmldom/xmldom` no longer transitively
  installed for consumers — roughly 50 KB pre-gzip / ~15 KB gzip
  recovered from app bundles.

Verification
- npm run sync-providers:test: 164/164 (setup file polyfills
  DOMParser correctly; vitest reports 274 ms setup time).
- npm run sync-providers:build: ESM 73.23 KB / CJS 75.77 KB / DTS
  37.13 KB. Confirmed `grep -c xmldom dist/index.mjs == 0`.
- All modified files pass `npm run checkFile`.
2026-05-12 22:22:42 +02:00
Johannes Millan
32e9b911bc refactor(sync-providers): broaden WebDAV CORS heuristic for real browsers
Multi-review (W2) flagged that the tightened heuristic from PR 6b
matched only `message.toLowerCase().includes('cors')`. Chrome's actual
CORS rejection is `"TypeError: Failed to fetch"` and Firefox's is
`"NetworkError when attempting to fetch resource at <url>"` — neither
contains the substring `"cors"`. As written, PotentialCorsError was
effectively dead code for the most common browser environments, and
real CORS misconfigs surfaced as a generic synthetic HTTP 500.

Broaden the match to every signal the major browsers actually emit:
`cors`, `cross-origin`, `opaque`, `failed to fetch`, `load failed`,
`network request failed`, `networkerror when attempting`. The bias is
intentional — WebDAV's most common deployment failure is misconfigured
CORS, so over-attributing offline / DNS to CORS still surfaces an
actionable hint. Only `TypeError` triggers the branch; other error
classes (AbortError, generic Error) fall through to the outer catch.

Privacy is preserved orthogonally: the raw fetch error is no longer
logged anywhere (the broader log meta is structured `errorMeta`), and
`PotentialCorsError(scrubbedUrl)` only ever carries the host+pathname
form of the URL — never the user-supplied URL with userinfo / query.
The Firefox message that embeds the URL on `.message` is therefore
contained.

Pulled the heuristic into a private `_isLikelyCors(error)` method with
the rationale documented in JSDoc so future maintainers see the
intent (and the privacy invariant).

Spec coverage: 6 parameterized cases per real-world emit pattern + a
non-CORS pass-through + a URL-leak privacy assertion. 164/164 tests
green (+7 since the previous run).
2026-05-12 22:22:42 +02:00
Johannes Millan
496559c3ad refactor(sync-providers): testWebdavConnection helper, drop adapter+api from barrel
Multi-review (W1) flagged WebDavHttpAdapter + WebdavApi as leaky from
the package public surface — both were exported solely so the dialog
could compose them for one "Test connection" button, repeating the
dep-wiring the createWebdavProvider factory already encapsulates.

Resolution: add a package-side `testWebdavConnection(cfg, deps)`
helper next to the WebDAV provider, and a thin app-side wrapper that
composes the four app singletons (logger, platform info, web fetch,
native HTTP). The dialog now imports `testWebdavConnection` from the
app shim and the call collapses from 12 lines to 1.

Package public surface drops both classes:

- Before: `WebDavHttpAdapter`, `WebDavHttpAdapterDeps`,
  `WebDavHttpRequest`, `WebDavHttpResponse`, `WebdavApi`, `WebdavApiDeps`.
- After: `testWebdavConnection`, `TestWebdavConnectionDeps`.

The internal classes remain available within the package (still used
by `WebdavBaseProvider` and the test helper); only the barrel surface
shrinks. ESM size effectively unchanged (the dropped exports were
re-exports of internal collaborators).

Verification
- npm run sync-providers:test: 157/157.
- npm run sync-providers:build: clean; ESM 71.71 KB.
- All four touched files pass `npm run checkFile`.
2026-05-12 22:22:41 +02:00
Johannes Millan
de8377a2df refactor(sync-providers): tighten WebDAV slice after multi-review
Six small mechanical follow-ups identified by the multi-review pass on
the WebDAV bulk-move slice. No behavior change beyond log levels and a
single hoisted hash; all sites independently verified.

- **W5 — forward responseType in APP_WEBDAV_NATIVE_HTTP.** The package
  adapter passes `responseType: 'text'` to the executor; the app-side
  adapter was destructuring only url/method/headers/data and silently
  dropping the field. Web fallback returns text today; future native
  impls that honor responseType could otherwise return non-string
  data. Added to the WebDavHttpOptions definitions interface too.

- **W6 — demote testConnection + _createDirectory + 409-persists logs
  from critical to normal.** All three are recoverable / expected
  failure modes during config debugging. Logging them at `critical`
  noises up the exportable log without adding signal. Updated the
  test-connection privacy spec to spy on `normal` as well (privacy
  invariant unchanged: errorMeta-structured at either level).

- **S1 — hoist md5(data) before the PUT block.** `upload()` was
  computing the local payload hash twice (once for the post-PUT
  verify, once before passing to _verifyUpload). With hash-wasm's
  async md5 that's a ~30-80 ms WASM call per ~MB sync file
  saved. The pre-PUT GET (conditional rev check) and the post-PUT
  GET (integrity verify) are still hashed each — those are remote
  payloads.

- **S2 / S3 — pin privacy invariants on HttpNotOkAPIError.** Added
  JSDoc invariants on `.response` and `.detail`: never stream the
  body into a structured logger, never route `.detail` to
  SyncLog (Nextcloud's <s:message> often embeds filenames). No
  behavior change — purely a contract pin so a future maintainer
  doesn't accidentally regress what the slice carefully avoided.

- **S5 — constrain WebdavBaseProvider<T, TPrivateCfg extends
  WebdavCredentialsLike>.** Adds a minimal `userName? / password? /
  baseUrl? / syncFolderPath?` bound. Drops both intersection-type
  casts (`as TPrivateCfg & { … }`) and the `as unknown as
  TPrivateCfg` widening cast in clearAuthCredentials. Both concrete
  cfgs (`WebdavPrivateCfg`, `NextcloudPrivateCfg`) already satisfy
  the bound.

Verification
- npm run sync-providers:test: 157/157.
- All modified files pass `npm run checkFile`.

Follow-ups remaining from the multi-review (will land as separate
commits): W1 (testWebdavConnection helper + barrel cleanup), W2
(CORS heuristic real-world coverage), W3 (@xmldom/xmldom to
devDependencies + global DOMParser), W4 (parser namespace +
server-format spec restoration).
2026-05-12 22:22:41 +02:00
Johannes Millan
914122f134 refactor(sync-providers): move WebDAV + Nextcloud providers into package
Lift WebdavBaseProvider, Webdav, NextcloudProvider, WebdavApi,
WebDavHttpAdapter, and WebdavXmlParser into @sp/sync-providers behind
the existing port surface — no new ports introduced this slice (per
multi-review consensus).

Architecture
- App side: thin createWebdavProvider(extraPath?: string) /
  createNextcloudProvider(extraPath?: string) factories compose
  NativeHttpExecutor / WebFetchFactory / ProviderPlatformInfo /
  SyncCredentialStore deps internally, matching the Dropbox slice
  precedent. sync-providers.factory.ts updated to call the factories.
- Package side: WebdavBaseProvider is generic on a WebdavProviderId
  union (typeof PROVIDER_ID_WEBDAV | typeof PROVIDER_ID_NEXTCLOUD),
  eliminating the four `as unknown as` casts the original code used
  to share its base class across WebDAV and Nextcloud cfgs.
- Native HTTP path stays correct: the app injects an APP_WEBDAV_NATIVE_HTTP
  adapter (in capacitor-webdav-http/app-webdav-native-http.ts) that
  wires Capacitor's WebDavHttp plugin into the existing
  NativeHttpExecutor port. The package's WebDavHttpAdapter selects the
  native path via platformInfo.isNativePlatform; otherwise it goes
  through WebFetchFactory. The inline registerPlugin('WebDavHttp')
  call in the previous adapter is gone — the canonical registration
  in capacitor-webdav-http/index.ts (with the web fallback) is the
  only one now.

Hashing
- md5HashSync (spark-md5) replaced with hash-wasm's async md5 (already
  a package dep used by PKCE). _computeContentHash on WebdavApi is
  now async; ripples through download / upload / verify paths.
- Added @xmldom/xmldom as a package dep so the parser can run under
  vitest's Node test env. The parser uses getElementsByTagNameNS('*',
  name) so it works portably across browser DOMParser and xmldom
  (xmldom does not implement querySelector).

Privacy sweep
- urlPathOnly applied at every URL-bearing error-construction and log
  site in webdav-http-adapter (incl. PotentialCorsError, the new
  HttpNotOkAPIError synthetic 500, RemoteFileNotFoundAPIError).
- errorMeta(e, extra) replaces every raw `SyncLog.error(..., e)` site
  across webdav-api and webdav-http-adapter — ten+ sites converted to
  structured `SyncLogMeta` (errorName / errorCode / safe primitives).
- _buildFullPath now throws InvalidDataSPError with a generic
  "contains '..' or '//'" message instead of generic Error echoing
  the user-supplied path.
- WebdavXmlParser.validateResponseContent's log no longer carries
  `responseSnippet: content.substring(0, 200)` — only `contentLength`
  and operation name go through the logger.
- testConnection retains its user-facing fullUrl + e.message (the
  user is testing their own server config — this is intentional UX,
  not a log), but routes the same error through `errorMeta` for the
  separate structured log line.
- CORS heuristic at webdav-http-adapter.ts:180-219 (40 lines) collapsed
  to a 3-line `TypeError && message.includes('cors')` check. Closes
  the privacy leak (Firefox's NetworkError embeds the request URL)
  and the prior false-positive where plain offline/DNS errors fired
  PotentialCorsError.

Spec migration
- Jasmine specs converted to Vitest:
  - webdav-xml-parser.spec.ts (15 tests, was 40)
  - webdav-http-adapter.spec.ts (11 tests, was 18)
  - webdav-api.spec.ts (18 tests, was 44)
  - webdav-base-provider.spec.ts (9 tests, was 26)
- The TestableWebDavHttpAdapter subclass-override pattern is deleted;
  tests inject the WebDavHttpAdapterDeps (platformInfo, webFetch,
  nativeHttp, logger) directly. Native-routed tests un-skip cleanly.
- The package-level __mocks__/@capacitor/core.ts harness is deleted
  (no longer needed — the package never imports @capacitor/core).

Dialog-sync-cfg
- src/app/imex/sync/dialog-sync-cfg now imports WebdavApi +
  WebDavHttpAdapter from @sp/sync-providers and constructs them with
  app-supplied deps for the "Test connection" UX. The user-facing
  success/error snackbar uses result.fullUrl + result.error
  unchanged.

Verification
- npm run sync-providers:test: 157/157 (was 103; +54 webdav specs).
- npm run sync-providers:build: ESM 70.93 KB / CJS 73.78 KB / DTS
  34.40 KB (was 40/43/25, ~30 KB growth from WebDAV + Nextcloud +
  @xmldom/xmldom).
- npm run lint: clean.
- npm run test:file file-based-sync-adapter.service.spec.ts: 58/58.
- npm run test:file sync-wrapper.service.spec.ts: 107/107.

Slice scope per multi-review consensus
- Open Q1: dropped the proposed WebDavNativeHttpExecutor port —
  reused NativeHttpExecutor with options. Open Q2: hash-wasm async.
  Open Q3: Nextcloud generic widening to union. Open Q4: inline
  registerPlugin dropped. Open Q5: CORS heuristic tightened in-slice.
  Open Q6: no-retry behavior preserved. Open Q7: TestableWebDavHttpAdapter
  deleted. Open Q8: webdav-api.spec kept conceptually monolithic
  (the package-side rewrite is smaller, but no second-file split).
- Documented gemini-dissent decisions in the slice design doc.

Defers (per consensus, not in this slice):
- local-file-sync-base.ts md5HashPromise migration — for the LocalFile
  slice.
- _directoryCreationQueue refactor — works; not premature.
- webdav-api file split into smaller modules — follow-up.

Follow-up testing not yet run by Claude (handover gates):
- Full npm test suite (two timezone variants).
- Full E2E.
- Manual round-trip against a real WebDAV / Nextcloud server (PUT
  with If-Match rev, 412 conflict path, 401 reauth path, 404 fresh-
  client bootstrap).
2026-05-12 22:22:41 +02:00
Johannes Millan
cf4fbd22e3 docs(sync-core-plan): fold Gemini review dissent into consensus
Gemini's review eventually completed after a workspace-sandbox retry
and quota throttling. Its findings broadly affirm the Claude
consensus, with two dissents — both rejected with reasoning:

- Q1 (port reuse): Gemini wanted to keep the new WebDavNativeHttpExecutor
  port "for consistency with NativeHttpExecutor." Rejected — the
  architecture and simplicity reviewers verified by code reading that
  the existing NativeHttpExecutor already supports arbitrary methods,
  responseType: 'text', and maxRetries: 0. Naming consistency is a
  weak argument against actual port duplication.

- Q6 (retry policy): Gemini wanted a 2-attempt + explicit 423 Locked
  retry. Rejected — alternatives and simplicity flagged this as a
  behavior change masquerading as a refactor. Adapter has zero retries
  today; preserving that keeps the slice scope a move. Per-call-site
  maxRetries remains trivially available once we reuse the existing
  port.

Refresh the consensus header from "Claude-only consensus" to reflect
that Gemini did contribute.
2026-05-12 22:22:41 +02:00
Johannes Millan
dc66b235a6 refactor(sync-providers): promote shared log helpers
Move errorMeta(e, extra) and urlPathOnly(url) from the
dropbox-api.ts top-of-file consts into a new
packages/sync-providers/src/log/error-meta.ts module, and re-export
from the package barrel.

Both helpers are about to gain a second consumer in the WebDAV
provider slice — urlPathOnly for scrubbing basePath out of error
URLs at every error-construction and log call site, errorMeta for
building privacy-aware SyncLogMeta from raw catches in the WebDAV
adapter and API.

No behavior change. Package surface adds two named exports
(`errorMeta`, `urlPathOnly`). ESM bundle stays at 40.53 KB (no
detectable change); 103/103 package tests green; tsup + DTS build
clean; all three modified files pass checkFile.

Follow-up: PR 6b moves the WebDAV + Nextcloud providers behind the
existing NativeHttpExecutor port and adopts these helpers during
the privacy sweep.
2026-05-12 22:22:41 +02:00
Johannes Millan
9a00152c22 docs(sync-core-plan): record WebDAV slice multi-review consensus
Add a "Multi-review consensus (2026-05-12)" section between the goal
and the what-moves description, mirroring the Dropbox slice doc's
structure. Captures findings from four Claude reviewers (security,
architecture, alternatives, simplicity). Codex / Copilot / Gemini CLIs
were attempted but failed for environment reasons (sandbox, deny-tool
classifier, quota+workspace limits); noted in the consensus header.

Decisions revised:

- Open question 1: DROP the new WebDavNativeHttpExecutor port. Reuse
  NativeHttpExecutor — the existing port already supports responseType:
  'text', maxRetries: 0, and arbitrary methods (PROPFIND/MKCOL/MOVE).
  Auto-JSON-parse is a property of CapacitorHttp.request, not the port.
  The app injects a different adapter (wired to WebDavHttp plugin) of
  the same port. Commit 2 collapses to "wire app-side adapter."
- Open question 4: Drop inline registerPlugin in this slice — the
  canonical registration in capacitor-webdav-http/index.ts carries the
  web: () => import('./web') fallback; the inline one in
  webdav-http-adapter.ts:31 does not.
- Open question 5: Tighten CORS heuristic in this slice. Collapse the
  string-matching to a ~3-line "TypeError && message.includes('cors')"
  check and replace the ambiguous-error log with toSyncLogError +
  urlPathOnly meta. Closes a privacy leak (Firefox NetworkError embeds
  the URL) and removes 40 lines.
- Open question 6: Preserve no-retry behavior. Under open-question-1's
  port reuse, becomes a per-call-site maxRetries: 0 argument.
- Open question 8: Keep webdav-api.spec.ts monolithic. Match Dropbox
  precedent (~876 lines, single file move).
- Commit shape: Match Dropbox 5a/5b split. PR 6a = log helpers
  promotion; PR 6b = bulk move + adapter wiring + privacy sweep +
  Nextcloud generic widen + md5HashSync → hash-wasm + CORS tighten +
  spec migration.
- Factory shape: createWebdavProvider(extraPath?: string), not (deps).
  Deps are composed internally from app singletons, matching the
  Dropbox precedent at app-side dropbox.ts:31-43.

Decisions affirmed: md5HashSync → hash-wasm async (with one-line
benchmark in PR description), Nextcloud generic widening to union,
delete TestableWebDavHttpAdapter spec harness.

New privacy blockers surfaced (must fix in PR 6b): URL/basePath leak
via _buildFullPath at four error-construction call sites, PROPFIND
response body fed into HttpNotOkAPIError (contains user filenames),
testConnection returning raw e.message, _buildFullPath throwing
generic Error with raw path, A3 sweep undercount (10+ raw-error log
sites), new B3.4 invariant (FileMeta never enters a Log call site),
and a documented package-boundary invariant that response headers
are not logged or attached to errors.

Action items: PR 6a (helpers promotion) → PR 6b (bulk move). The
original "Open questions" section is preserved below the consensus
as a decision-log for review history, in the same style as the
Dropbox slice doc.
2026-05-12 22:22:41 +02:00
Johannes Millan
bd6863ac84 docs(sync-core-plan): draft WebDAV slice design for multi-review
Pre-implementation design doc for PR 5's WebDAV + Nextcloud slice,
mirroring the structure of the Dropbox slice design doc.

Captures the file-move surface (9 source files + specs), the new
WebDavNativeHttpExecutor port shape (callable type, divergent from
NativeHttpExecutor because of Capacitor WebDavHttp plugin transport
differences), the md5HashSync → hash-wasm migration choice, the
errorMeta / urlPathOnly log-helper promotion as a precursor commit,
the privacy A1/A3/B3.x sweep checklist, and the Nextcloud generic-
parameter / cast cleanup option.

Includes eight open questions framed for multi-review (port naming,
md5 strategy, generic parameter, registerPlugin cleanup scope, CORS
heuristic, retry policy, spec migration, file split). Suggested
three-commit shape (helpers promotion → port introduction → bulk
move) keeps each commit independently green.

No code changes; design doc only.
2026-05-12 22:22:41 +02:00
Johannes Millan
745551cacb docs(sync-core-plan): summarize Fifth Slice and renumber remaining
Add a "Current Fifth Slice" section mirroring the Current First through
Fourth Slice summaries, capturing PR 5a (provider error classes), PR 5b
(Dropbox provider proper), and the post-review cleanup pass.

Renumber the Remaining Slice Plan now that the Dropbox slice has shipped:
WebDAV + Nextcloud is now slice 1 (next), SuperSync slice 2, LocalFile
slice 3. The WebDAV entry calls out the WebDavNativeHttpExecutor port
shape, the errorMeta / urlPathOnly log-helper promotion, and the
A1/A3/B3.x privacy sweep carry-over.

Refresh the top-of-doc Status header so the shipped-vs-remaining
inventory matches the slice summaries.
2026-05-12 22:22:41 +02:00
Johannes Millan
5438b87c85 fix(supersync): address multi-review findings (C1, W2, W4, W9, W11, S6)
C1: guard CRT/UPD/MOV/BATCH (and BATCH-entities) replay against
__proto__/constructor/prototype entityIds — bracket-assignment to those
keys invokes the __proto__ setter and swaps the entity map's prototype.
Shares the new isUnsafeEntityKey helper with the full-state branch.

W2: stop deleting storageUsageLocks[userId] in clearForUser. The
identity-guarded finally in runWithStorageUsageLock self-deletes when
the chain drains, and removing the head while a follower is queued lets
a fresh caller see no previous and race the in-flight chain. Replaces
the deprecated assertion with a race regression test.

W4: when _generateSnapshotImpl's regenerated blob exceeds the hard
MAX_SNAPSHOT_SIZE_BYTES ceiling, mirror cacheSnapshot's clear-stale
logic via a race-safe updateMany. Without this an oversize regen left
a poisoned older blob serving GET /snapshot.

W9: reject layered or non-gzip Content-Encoding values with 415 in the
content-type parser instead of falling through to JSON.parse on raw
gzip bytes (which produced a misleading 400 invalid-json).

W11: computeOpStorageBytes now returns { bytes, fallback }; route gate
and uploadOps both Logger.warn the count of unserializable ops (never
the content) so the previously-lost observability is back.

S6: drop the no-op Math.max around estimatedOpStorageBytes for
uncacheable snapshots — stateBytes is always > MAX_SNAPSHOT_SIZE_BYTES
when \!cacheable, so the floor never engaged.
2026-05-12 22:22:14 +02:00
Johannes Millan
0286804fd3 test(supersync): tighten prototype-pollution test invariants
The previous SYNC_IMPORT prototype-pollution regression test asserted
`Object.prototype.polluted === undefined` and `({}).polluted === undefined`.
Both assertions are tautological: `Object.assign({}, {__proto__: x})` uses
[[Set]] on the target, which triggers the `__proto__` setter only on the
target itself — it swaps `state`'s prototype reference, but does NOT
pollute the global `Object.prototype`. The old test therefore passed even
against the buggy implementation it was meant to catch.

Assert the real invariant instead:
- `Object.getPrototypeOf(result) === Object.prototype` (state's prototype
  was not swapped — this is what flips under the buggy code).
- `result.polluted` is undefined via the prototype chain.
- `__proto__` is not an own property on the result.

Verified the new assertions fail against `Object.assign(state, fullState)`
and pass against the current key-by-key copy with a __proto__/constructor/
prototype skip list. Kept the global-pollution checks as defense in depth
and the `finally` cleanup in case of future regressions.
2026-05-12 22:06:37 +02:00
Johannes Millan
63b68b5041 Merge branch 'feat/carefully-review-all-super-sync-related-b22f81'
* feat/carefully-review-all-super-sync-related-b22f81:
  fix(sync): harden supersync server edge cases
2026-05-12 21:55:30 +02:00
Johannes Millan
948d9c7db3 fix(sync): harden supersync server edge cases 2026-05-12 21:53:49 +02:00
Johannes Millan
312dc94da8 fix(supersync): cast fullState entry to Record on full-state replay
TypeScript flagged the state[key] = fullStateRecord[key] assignment from
99fc98a1ab — state is typed Record<string, Record<string, unknown>> while
fullStateRecord[key] is unknown. Cast matches the existing pattern used
elsewhere in replayOpsToState for nested-record writes.
2026-05-12 21:51:34 +02:00
Johannes Millan
30a0809d84 fix(supersync): tighten routes — close quota double-count, snapshot gate, rate-limit, bomb-cap, content-encoding
B3: remove now-redundant post-commit `applyStorageUsageDelta` writes in
the routes. Wave-1 commit 9af17e460e moved the storage-counter write
INTO `uploadOps`'s `$transaction`, so the route adding a second delta
would double-count every accepted op. /ops drops the post-commit
increment entirely; POST /snapshot now only writes `cacheResult.deltaBytes`
(the snapshot-cache portion). The route's `computeOpsStorageBytes` now
delegates to the shared `computeOpStorageBytes` helper in `sync.const.ts`
to guarantee gate-vs-increment parity.

B4: when `preparedSnapshot.cacheable === false`, do not subtract
`previousBytes` from the gate, and use `Math.max(stateBytes,
MAX_SNAPSHOT_SIZE_BYTES)` as the gate value so the worst-case op-row
write cannot slip past on a raw-JSON estimate that doesn't capture
JSONB/TOAST overhead.

B5: GET /snapshot now passes a `maxCacheBytes` cap (the user's remaining
quota) down through `generateSnapshot`. When the freshly compressed
snapshot would exceed the cap, the cache is NOT written but the
in-memory snapshot is still returned to the caller. A user already at
quota can still catch up reads without growing on-disk storage further.
Adds a regression test in `snapshot.service.spec.ts`.

B8: add `rateLimit: { max: 10, timeWindow: '15 minutes' }` to POST
/snapshot to match the other write-heavy operations (DELETE /data,
/restore/:seq). Snapshot uploads are expensive (30MB body,
`prepareSnapshotCache` zlib + JSON.stringify, full-state op replay)
so burst-uploads from one user can pin a worker.

B9: split `MAX_DECOMPRESSED_SIZE` into per-endpoint caps
(30 MB for /ops, 60 MB for /snapshot) so a 10 MB compressed body cannot
expand to 100 MB of JSON.parse work and stall the event loop.

B10: normalize `Content-Encoding` via a shared `isSingleTokenGzipEncoding`
helper so RFC-valid values like `Gzip`, ` gzip `, or `['gzip']` match.
Layered encodings (`gzip, deflate`) are rejected. Replaces all three
strict `=== 'gzip'` comparisons in `sync.routes.ts`.

Updates `sync-compressed-body.routes.spec.ts` for the new post-commit
delta contract: the snapshot route only writes the cache portion now;
the op-row portion is owned by `uploadOps`'s transaction.
2026-05-12 21:49:50 +02:00
Johannes Millan
7d654d0be1 fix(supersync): align snapshot service spec mocks with upsert
Two negative-path generateSnapshot tests declared `update` on the mock tx
and asserted `expect(update).not.toHaveBeenCalled()`. Production calls
`tx.userSyncState.upsert` (not update) inside `_generateSnapshotImpl`, so
the assertion was tautological — `update` is never invoked regardless of
whether the rejection guard ran. Renaming to `upsert` makes the
"no cache write happened" assertion meaningful again.

The module-level `prisma.userSyncState.update` mock stays — production
still calls `update` from `_invalidateCachedSnapshot`, exercised by the
decompression-error test.
2026-05-12 21:36:54 +02:00
Johannes Millan
9af17e460e fix(supersync): sanitize errors, exact full-state quota, atomic counter
- B7: replace leaky `Transaction rolled back: <raw Prisma msg>` per-op
  error with a generic 'Transaction failed - please retry'. The raw
  exception text is still logged via `Logger.error`, but never returned
  to the client (could leak SQL fragments, FK / column names).
- B6: in `deleteOldestRestorePointAndOps`, measure the exact bytes for
  the deleted full-state op (SYNC_IMPORT / BACKUP_IMPORT / REPAIR) row
  via `pg_column_size` BEFORE deletion. Bounded to 0-1 rows per call,
  so it doesn't reintroduce the DoS from scanning every delta op. Delta
  ops keep the cheap `count * APPROX_BYTES_PER_OP` approximation;
  full-state payloads (up to 20MB) no longer undercount by ~20000x and
  leave the counter permanently low if reconcile fails. Docstring on
  APPROX_BYTES_PER_OP clarifies it is ONLY valid for delta cleanup.
- I6: inside `deleteOldSyncedOpsForAllUsers`, call
  `storageQuotaService.markNeedsReconcile(userId)` for each user whose
  cleanup deleted rows. A crash mid-loop now leaves surviving users
  with a forced-reconcile marker so their next request self-heals,
  instead of waiting for the daily pass. TODO left for persisting the
  marker in a DB column for cross-restart / multi-instance safety.
- B3 (service side): write `users.storage_used_bytes` atomically
  inside the upload `$transaction` using `$executeRaw` with the same
  GREATEST(...) clamp pattern as `storage-quota.service.ts`. Shared
  `computeOpStorageBytes` helper keeps the gate and the increment in
  agreement about per-op size. Clean-slate path SETs the counter to
  the new total rather than incrementing onto the just-reset row.
- B2 companion: add `RequestDeduplicationService.clearForUser(userId)`
  and call it from both `deleteAllUserData` and the uploadOps
  clean-slate path so a retry from before the wipe cannot return
  cached results that reference now-deleted state.
2026-05-12 21:36:49 +02:00
Johannes Millan
9764502f86 fix(supersync): use effectiveSinceSeq for Case-2 gap detection
The minSeq-based gap check compared the raw sinceSeq, so a client
requesting from before retention purge would get gapDetected=true
even when the snapshot already supersedes that purged history. Case 3
already used effectiveSinceSeq; this aligns Case 2 so the snapshot
fast-forward suppresses the false positive and avoids triggering an
unnecessary full restore on the client.
2026-05-12 21:36:45 +02:00
Johannes Millan
a2bbe5d0f5 fix(supersync): randomize cleanup reconcile order to prevent user starvation
The deferred-reconcile budget caps at 720 users per pass
(RECONCILE_BUDGET_MS / RECONCILE_INTERVAL_MS = 3.6M / 5K). With a stable
DB iteration order, the first 720 affectedUserIds got reconciled every
daily cleanup and any tail beyond that drifted indefinitely.

Shuffle the affected-user list with Fisher-Yates before slicing the
budget window so each pass picks a different random subset, and include
both numbers in the warn log so operators can spot servers that have
outgrown the per-pass budget.
2026-05-12 21:36:44 +02:00
Johannes Millan
b7332238a7 fix(supersync): clear all per-user quota state on user wipe
clearForUser now also clears forcedReconciles (would force a spurious
extra scan on the next quota check) and storageUsageLocks (orphaned
queued lock chain) alongside the existing inflightReconciles cleanup.

Also document the known byte-counting mismatch between pg_column_size
(TOAST-compressed) and the hot-path Buffer.byteLength(JSON.stringify)
in calculateStorageUsage. The proper fix needs a payload_bytes column
populated at insert time; in-place switching to octet_length(text)
would re-detoast every row and resurrect the disk-I/O DoS.
2026-05-12 21:36:41 +02:00
Johannes Millan
b4812a2b30 fix reviewed calendar and duplicate edge cases 2026-05-12 21:34:02 +02:00
Johannes Millan
99fc98a1ab fix(supersync): harden snapshot cache writes and replay against races and prototype pollution
Address three review findings on snapshot.service.ts:

- B1: _generateSnapshotImpl no longer unconditionally upserts the cache.
  A concurrent upload that bumped lastSnapshotSeq beyond our latestSeq
  could be clobbered. Switched to the same race-safe updateMany+create
  pattern as cacheSnapshot, and pin cacheDelta to 0 when the race is
  lost so the onCacheDelta hook does not over-credit storage.

- I2: Always run _assertCachedSnapshotBaseReplayable before serving a
  cached snapshot, including the startSeq>=latestSeq fast path.
  A legacy build that failed to reject encrypted ops could have
  produced a poisoned cache that the fast path would serve forever.

- I4: replayOpsToState no longer uses Object.assign for full-state op
  payloads. A malicious SYNC_IMPORT/BACKUP_IMPORT/REPAIR payload with
  a __proto__ key would pollute Object.prototype via the [[Set]]
  semantics of Object.assign. Copy keys explicitly and skip
  __proto__/constructor/prototype.

I3 (RepeatableRead transaction spans gzip/replay/gzip and can starve
the DB pool under load) is acknowledged but left for a focused PR —
restructuring requires moving the cache write outside the transaction
and updating multiple integration tests.

Updated unit-test mocks to provide updateMany/create on the
transaction client, and added regression coverage for the prototype-
pollution and fast-path-validation cases.
2026-05-12 21:33:38 +02:00
Johannes Millan
40c354d4ef fix(supersync): scrub merge debris and align deploy defaults
- B11: Remove orphan deploy-db-scalar.mjs and dead RUN_POST_MIGRATION_INDEXES
  references in env.example and README. Rewrite README deploy section to
  match what scripts/deploy.sh actually does (drop false claims about
  empty migration SQL, failed-migration recovery, and optional index builds).
- B12: Default RUN_MIGRATIONS_ON_STARTUP to false in docker-compose.yml so
  the compose, Dockerfile ENV, and env.example agree on the safer default
  (manual deploy.sh runs migrations once before app restart). Update the
  migration-sql.spec assertion to match.
- I9: Wrap prisma migrate deploy in deploy.sh with a MIGRATION_TIMEOUT
  (default 900s). CREATE INDEX CONCURRENTLY can block on long-running
  transactions; on timeout (exit 124) the script now fails loudly with a
  clear error rather than hanging.
2026-05-12 21:26:32 +02:00
Johannes Millan
7a7d80ac24 Merge branch 'feat/handover-pr-5-dropbox-provider-8ae7e1'
* feat/handover-pr-5-dropbox-provider-8ae7e1:
  refactor(sync-providers): tighten Dropbox port after review
  refactor(sync-providers): move Dropbox provider into package
  refactor(sync-providers): move provider error classes
2026-05-12 21:15:31 +02:00
Johannes Millan
1e2f58c867 test(electron): align TODAY filter spec with isTodayWithOffset impl
The master merge swapped dateService.isToday for the pure
isTodayWithOffset utility, but this test still mocked isToday and used
a 1970 timestamp. Use a real today-UTC timestamp safe for both CI
timezones.
2026-05-12 21:11:53 +02:00
Johannes Millan
b80f827f37 refactor(sync-providers): tighten Dropbox port after review
Four cleanups surfaced by the post-merge multi-review pass:

- Drop the dead `export type { NativeHttpResponse }` from the dropbox
  module — the package barrel already re-exports it from the http
  module, so the dropbox-level re-export was unreachable noise.
- Replace the hand-rolled `encodeFormBody` helper with the standard
  `URLSearchParams`. Eight lines and one Object.entries map gone; the
  fetch path now passes URLSearchParams directly (idiomatic BodyInit),
  the CapacitorHttp path uses `.toString()` for the string `data`
  field. Encoding remains spec-correct for `application/x-www-form-
  urlencoded`.
- Convert the runtime `_idCheck` constant in the app shim into a
  pure-type `AssertDropboxId` conditional alias. Same compile-time
  guarantee, smaller surface.
- Inline the redundant `_executeNativeRequestWithRetry` private wrapper
  on `DropboxApi` — only one caller and it just forwarded to the
  package helper.
- Drop the now-unnecessary `as unknown as` step on the credentialStore
  cast in the factory shim; a single `as DropboxDeps['credentialStore']`
  suffices.

Package tests stay at 103/103. tsup build, full lint, and the
identity / file-based-sync-adapter specs all clean.
2026-05-12 21:09:11 +02:00