error-meta.spec.ts was the only test importing via the package
self-reference subpath @sp/sync-providers/log, which exports maps to
./dist/log.mjs — requiring a tsup build that 'npm test' never runs, so
vitest could not resolve it. Import ../../src/log like the other 15
test files and drop the now-dead tsconfig.spec path mapping that only
existed to satisfy that import's typecheck.
* fix(sync): guard fresh-client first sync against data-loss trap & decrypt-race
A genuinely-fresh client seeds example tasks before its first sync, so on first
connect to a populated, encrypted SuperSync dataset it hit a SYNC_IMPORT conflict
dialog whose USE_LOCAL would overwrite the real remote, plus a red
DecryptNoPasswordError.
- A1: SyncImportConflictGateService flags isNeverSynced (\!hasSyncedOps) on the
incoming-import dialog; the dialog guards the destructive USE_LOCAL with a
confirm and focuses the scenario-appropriate safe button via cdkFocusInitial.
- B: OperationLogDownloadService logs 'encrypted ops, no key' quietly only for a
genuinely-fresh client (never synced AND no local encryption flag); it stays
loud (dropped-credential signature) for already-synced clients and for a wiped
op store whose config still flags encryption on, via the new optional provider
method SuperSync.isEncryptionEnabled().
- D: SuperSync.isReady() returns false while isEncryptionEnabled && \!encryptKey,
keeping a self-inconsistent encrypted config out of auto-sync; shares the
_isEncryptionHalfConfigured predicate with getEncryptKey().
Tests: super-sync isReady/isEncryptionEnabled, conflict-gate isNeverSynced,
dialog confirm-guard + focus-by-scenario, download severity branches, and a
sync-service end-to-end first-sync-trap guard.
Plan/rationale: docs/plans/2026-06-03-fresh-client-first-sync-data-loss-trap.md
* docs(sync): add fresh-client first-sync data-loss-trap plan
Documents the shared root cause (example tasks seeded pre-first-sync), the A1/B/D
fixes, and the accepted Fix C residual (example-task pollution onto non-encrypted
accounts) with the rationale for deferring an identity-based cleanup.
* fix(sync): capture never-synced guard pre-download for piggyback SYNC_IMPORT
The SYNC_IMPORT conflict gate's never-synced guard (which blocks a
fresh client from force-overwriting a populated remote via USE_LOCAL)
was computed from a live hasSyncedOps() read on the piggyback-upload
path. By the time that gate runs, the same sync has already persisted
downloaded ops with syncedAt and marked accepted uploads synced, so the
flag flips to "has synced" mid-cycle and the guard disarms itself —
re-opening the data-loss trap on the piggyback path.
Capture the never-synced snapshot once at sync-cycle start, before
download, and thread it through downloadRemoteOps()/uploadPendingOps()
into the gate. The gate falls back to a live read only for standalone
callers (download gate is safe live: nothing is persisted until
processRemoteOps).
Also stop the sync-wrapper catch from re-logging the expected
DecryptNoPasswordError at error level — the download/upload service
already logs it at the right severity, so re-logging undid the
fresh-client onboarding-prompt quieting.
Tests: gate honors a caller-provided isNeverSynced without consulting
live history; piggyback path flags isNeverSynced=true even when the
upload marks ops synced; wrapper threads the pre-download snapshot.
Refresh the plan doc's stale test counts and document the fix (§11).
iOS reads a stale sync-data.json on every WebDAV sync: URLSession's
default configuration caches GET responses, so the client never sees
remote changes and — because the pre-upload conflict check re-reads the
same cached body — blindly overwrites newer remote data. Android (OkHttp,
no cache) and web (fetch `cache: 'no-store'`) are unaffected, which is why
this is iOS-only.
- iOS WebDavHttpPlugin: disable the HTTP cache (urlCache=nil +
reloadIgnoringLocalCacheData) so sync reads always hit the network.
- WebDavHttpAdapter: send `Cache-Control: no-cache, no-store` + `Pragma`
on the native request path, covering an upstream proxy/CDN cache too.
- Log allowlisted, non-sensitive cache-relevant response headers so a
future log capture localises a stale read (client cache vs proxy).
Fixes#7144
WebdavBaseProvider.clearAuthCredentials() blanked the user-typed
userName/password on a single recoverable AuthFailSPError /
MissingCredentialsSPError. The transient-retry tolerance in
sync-wrapper is SuperSync-only, so a one-off 401 on WebDAV/Nextcloud
irreversibly erased the stored connection settings with no undo.
A WebDAV password is a user-typed, often-irrecoverable secret, not a
refreshable OAuth token. Remove the WebDAV/Nextcloud override so
ProviderManager's existing guard makes the auth-error clear path a
no-op for them; Dropbox/SuperSync keep their token clearing. The
actionable re-configure snackbar still fires unchanged. Document the
optional-hook contract on SyncProviderBase.
Add regression tests for the provider hook (incl. asserting WebDAV
exposes no clearAuthCredentials) and a legacy WebDAV credential
migration repro harness (the migration itself was sound).
Fixes#7616. Related: #7619 (exported logs leak task content).
testConnection() did a PROPFIND only on the configured sync folder.
On first-time setup that folder does not exist yet (created lazily on
first upload), so the server returned 404 and every correctly-configured
new Nextcloud/WebDAV user saw "Connection test failed".
Probe the WebDAV base root instead: it is reachable and auth-checked for
any valid server/credentials, while a wrong username/base path (404) or
bad password (401) still correctly fails. Add regression tests covering
the base-root probe, wrong-base 404, and the bad-credentials safety
invariant.
Address review nits on 058f92e972: anchor the retryable-upload regex
to "unable to resolve host" so future SuperSync op-graph rejection
strings (e.g. "Unable to resolve parent revision") can't be flipped
from permanent to retryable; rename RETRY_BACKOFF_MS to
RETRY_BACKOFF_BASE_MS (it is the linear base, not the total); clarify
the JSDoc that 4.5s is the inter-attempt sleep budget, not wall-clock.
Android UnknownHostException after Doze-mode resume produced a raw
"Unable to resolve host" snackbar because the post-retry error message
didn't match isRetryableUploadError's patterns, so it skipped the
NetworkUnavailableSPError wrap. Bump the native HTTP retry backoff from
1s/2s to 1.5s/3s (~4.5s budget) to better tolerate post-Doze DNS
staleness, and add the Android phrasing to both transient-error checks
so the error now surfaces as the translated network-warning snackbar.
* refactor(sync): tighten extracted package surfaces
Combined polish from the post-extraction review:
- sync-core: strip NgRx-shaped types from EntityConfig/EntityRegistry;
expose host extensions via generic param. Move StateSelector,
PropsStateSelector, SelectByIdFactory, SelectById, EntityUpdateLike,
EntityAdapterLike to a new app-side entity-registry-host.types.ts.
- sync-core: mark OpType.SyncImport/BackupImport/Repair as @deprecated;
hosts should use createFullStateOpTypeHelpers().
- sync-providers: resolve provider.types.ts vs provider-types.ts
duplication; inline implementation into the dashed canonical name.
- sync-providers: drop unused root barrel and "." export; consumers
already use focused subpath barrels (/dropbox, /webdav, etc.).
- sync-providers: replace wildcard "@sp/sync-providers/*" tsconfig path
alias with 11 explicit subpath entries matching package.json exports;
deep-internal imports now fail at typecheck.
- sync-providers: move @sp/sync-core from dependencies to
peerDependencies (kept in devDependencies for tests).
- both packages: add composite: true to enable project references;
introduce tsconfig.build.json overlay so tsup DTS bundler still works.
- gitignore: ignore **/*.tsbuildinfo composite outputs.
* refactor(sync-core): prune 47 unused barrel exports
Removes exports with zero consumers outside the package. Source files
are unchanged; only the public barrel is trimmed. Covers compression
helper classes, sync-file-prefix error/config types, replay coordinator
internals, remote-apply result types, upload/download planning option
and plan types, ports misc, conflict-resolution helper types, and
sync-import-filter decision types.
* refactor(sync-core): drop unused encryption migration path
decryptWithMigration and DecryptResult had no host consumer; they
exposed a structural-migration entry point ("here is your ciphertext
re-encrypted under Argon2id") that nothing in the codebase reads. The
side-channel setLegacyKdfWarningHandler — which IS used — stays.
encryptWithDerivedKey/decryptWithDerivedKey lose their export keyword
and remain as module-internal helpers; encrypt/decrypt/encryptBatch/
decryptBatch still call them. Wire format and legacy-fallback semantics
are unchanged, so existing ciphertext continues to decrypt.
Test imports for compression and sync-file-prefix specs now go via
their source files instead of the trimmed barrel.
* fix(sync-providers): bound dropbox token refresh to single retry; share md5 rev helper
The five hand-rolled token-refresh blocks in Dropbox.{getFileRev,
downloadFile, uploadFile, removeFile, listFiles} recursed on themselves
after refresh. If the post-refresh call still saw a token error (real
case: the refresh token itself was revoked), the recursion would not
terminate. Consolidated into a single _withTokenRefresh helper that
attempts the call, refreshes once on a token error, retries once, then
lets the outer 401 classifier surface AuthFailSPError.
Same log message, same _isTokenError discriminator, same refresh call.
Same five sites still apply their post-call non-token error mapping
(NoRev, InvalidData, RemoteFileNotFound, path-not-found swallow, etc.).
Also extracts md5 content-rev computation duplicated between
LocalFileSyncBase._getLocalRev and WebdavApi._computeContentHash into a
shared file-based/content-rev.ts; both call sites preserve their own
error wrapping at the boundary.
* refactor(sync): split oversized super-sync and conflict-resolution
sync-providers: extract request-ID hashing from super-sync.ts (1017 ->
918 lines) into a new request-id.ts. The helpers were free functions
already in disguise (none referenced this), so the move is mechanical.
HTTP plumbing (_doWebFetch/_doNativeFetch/_fetchApi*) stays as private
methods — it transitively touches 12 instance members and would need
either a wide context object or a separate http-client collaborator
class to extract cleanly. Left as a follow-up.
sync-core: split conflict-resolution.ts into three cohesive files:
- entity-frontier.ts now owns buildEntityFrontier and
adjustForClockCorruption (per-entity vector-clock domain).
- extractEntityFromPayload and extractUpdateChanges move to
operation.types.ts next to the existing extractActionPayload.
- conflict-resolution.ts keeps deep-equality, LWW planning,
partitioning, and identical-conflict detection.
Public barrel exports unchanged; tests now import the moved symbols
from their new homes.
* refactor(sync-core): drop redundant OperationStorePort
OperationStorePort overlapped with RemoteOperationApplyStorePort on the
two state-transition methods (markSynced/markApplied,
markRejected/markFailed) and had zero non-structural consumers — the
only implementer was OperationLogStoreService, which already exposes
the three methods as its own public surface. Removing the port leaves
the service contract intact and removes the verb-pair confusion noted
in the post-extraction review.
Spec contract test still drives the same state transitions; only the
local typing of the test fixture changes from the deleted interface to
Pick<OperationLogStoreService, ...>.
* refactor(sync-providers): decouple SuperSync provider from SP-specific host
Two coupling leaks the package shouldn't carry:
1. SUPER_SYNC_DEFAULT_BASE_URL was an implicit fallback inside
SuperSyncProvider — an SP-specific URL baked into a "framework-
agnostic" package. Make defaultBaseUrl a required SuperSyncDeps
field; the host factory supplies the SP default. The constant stays
exported as a suggested default for hosts targeting the SP-hosted
server.
2. Consumers that wanted the WebSocket path had to do
`provider as unknown as SuperSyncProvider` to call
getWebSocketParams. Introduce SuperSyncWebSocketAccess interface +
isSuperSyncWebSocketAccess structural guard; SuperSyncProvider
implements it. sync-wrapper.service drops its cast in favor of the
guard.
super-sync-restore.service still casts to SuperSyncProvider for the
restore path — same pattern would solve it, but out of scope here.
* test(sync-providers): extract shared test helpers and prefer barrels
Adds tests/helpers/sync-logger.ts and tests/helpers/credential-store.ts
to centralize the noopLogger and CredentialStore mocks that were copy-
pasted across 8 spec files. createStatefulCredentialStore covers the
"load/upsert/clear with state" cases; createMockCredentialStore covers
bare vi.fn() ports. Spec sites that needed a unique mockResolvedValue
chain it after the helper, preserving behavior 1:1.
Also migrates 5 spec files from deep ../src/<file> paths to the
matching sub-barrel (../src/webdav, /http, /super-sync, /platform) for
symbols already exported there. No new barrel exports added — internal
types (WebDavHttpAdapter, WebdavApi, DropboxApi, etc.) stay on deep
paths because they are intentionally not part of the public surface.
super-sync.spec.ts keeps its own credential/logger mocks (special
__asPort wrapper and vi.spyOn against the live NOOP_SYNC_LOGGER) that
the generic helpers cannot reproduce without bloat.
* test(sync): pin vector-clock pruning, error-meta privacy, and sync-import edges
Fills three test gaps surfaced by the post-extraction review:
- vector-clock pruning correctness across clocks: 4 cases pinning that
pruning legitimately flips GREATER_THAN to CONCURRENT/LESS_THAN when
the dropped keys are still present in the comparison clock. This is
the documented behavior (compareVectorClocks is intentionally not
pruning-aware); the protocol handles flips server-side via the
rejected-ops retry loop. preserveClientIds case also covered.
- error-meta privacy boundary: 22 new cases covering urlPathOnly (strip
query/fragment/userinfo, preserve host+path+port, leave non-URLs
intact) and errorMeta (no leakage of headers, response bodies, OAuth
tokens, signed-URL params, user emails, or attached error fields).
Real negative assertions (.not.toContain), not shape checks.
- sync-import-filter edge cases: 8 cases covering empty clocks on
either side, op clock listing the import client at 0, same-client
with equal counter (pinning the strict-greater-than boundary), and
different-client knowledge above the import counter.
sync-core 195 -> 207 tests, sync-providers 319 -> 341 tests; no
production code changed.
* style(sync-core): format sync-file-prefix.spec import line
* fix(sync): address package review feedback
Findings from a multi-agent review of the recent sync extraction:
- Seven error classes in @sp/sync-providers shipped with a leading
space in `name`, and `UploadRevToMatchMismatchAPIError` was further
truncated to ' UploadRevToMatchMismatchAP'. Consumers use instanceof
so runtime behavior was preserved, but stack traces, error envelopes,
and structured-log meta carried the broken names. Added a regression
test asserting instance.name matches ErrCtor.name for all 14 classes.
- @sp/sync-core decryptBatch JSDoc claimed Argon2 errors are never
silently masked as legacy fallbacks, contradicting the actual
catch-and-decryptLegacy path. The fallback is part of the public
wire-format contract; rewrote the comment to match the implementation
and reference the module-level wire-format spec.
- SuperSyncEncryptionToggleService.enable/disableEncryption promised
"Clear cache on success" but never called clearSessionKeyCache().
Added the call on the success path in both methods, matching the
pattern used by EncryptionPasswordChangeService and
FileBasedEncryptionService.
- Removed packages/sync-core/tests/ports.spec.ts — 57 LOC of
vi.fn().mockResolvedValue type-assertion tests with no production
code under test. Port shapes are enforced at the host via
`implements` clauses with real behavioral coverage in sibling specs.
Two related improvements to snapshot/full-state handling:
- Persist the incoming op's vector clock as `user_sync_state.latest_full_state_vector_clock`
so downloads can skip the expensive aggregate scan when the snapshot clock is
still valid; falls back to the legacy aggregate when missing or invalid.
- Treat retried snapshot uploads (same opId) as idempotent successes instead of
409/DUPLICATE_OPERATION so a dropped response doesn't push clients into the
download-and-merge path. Per-namespace request dedup keeps ops and snapshot
caches isolated.
Adds a 2-retry / 1s+2s backoff loop to the browser/Electron SuperSync
request path, mirroring the existing native retry. A new typed
NetworkUnavailableSPError replaces the previous string-shape contract
between provider and wrapper: the provider throws the typed error from
both web-retry exhaustion and native-failure handler, and the wrapper
matches via instanceof to show a transient WARNING snackbar
(F.SYNC.S.NETWORK_ERROR) without flipping into a hard ERROR state.
Drops the wrapper-side regex classifier (and its defensive
^HTTP\s+\d{3}\b filter), the now-unused isTransientNetworkError alias,
and dedupes the predicate call inside the web-retry catch. The single
remaining call site for the broad regex (operation-log-upload) now
imports isRetryableUploadError directly under its honest name.
Tests cover the 1s/2s cadence, retry-then-success, retry-exhaustion via
the typed error, and negative paths (AbortError, HTTP-status 5xx,
AuthFailSPError must NOT retry). The error class is added to the
cross-module identity safety net.
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).
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.
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.
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).
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).
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.
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
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`.
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).
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).
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).
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.
Lift Dropbox, DropboxApi, and DropboxFileMetadata into @sp/sync-providers
behind a small set of new injected ports:
- ProviderPlatformInfo — { isNativePlatform, isAndroidWebView, isIosNative }
readonly booleans; replaces direct Capacitor.isNativePlatform / IS_IOS_NATIVE
reads inside the provider.
- WebFetchFactory — callable type () => fetch; lazy resolution preserves the
iOS workaround where Capacitor patches window.fetch asynchronously.
- NativeHttpExecutor (already in slice 4) gains a maxRetries option so
getTokensFromAuthCode can unify with the regular retry path while still
being one-shot for one-time auth-code exchanges.
App-side, dropbox.ts collapses to a 38-line factory function
createDropboxProvider that wires OP_LOG_SYNC_LOGGER + APP_PROVIDER_PLATFORM_INFO +
APP_WEB_FETCH + SyncCredentialStore + CapacitorHttp.request into DropboxDeps and
returns the package Dropbox class directly. sync-providers.factory.ts updated.
Privacy work folded in:
- A1: dropbox.ts:191 no longer logs raw r.data on malformed download.
- A3: every catch-site SyncLog.critical(..., e) replaced with structured
toSyncLogError(e) + curated SyncLogMeta; URLs scrubbed to host + pathname.
- B3.2: error constructors receive relative targetPath, never the joined
basePath + targetPath.
- B3.3: AuthFailSPError no longer carries raw responseData.
Native-platform routing specs that were previously skipped under Jasmine
(Capacitor.request un-mockable) are now un-skipped under Vitest with the
injected NativeHttpExecutor mock. Adds 33 package tests (70 -> 103).
tryCatchInlineAsync removed: the sole call site inlines a defensive
response.json().catch(...) instead. Last consumer of src/app/util/
try-catch-inline.ts gone, so the util is deleted.
DropboxFileMetadata moves with the provider; src/app/imex/sync/dropbox/
dropbox.model.ts deleted (no other consumers).
Lift 12 provider-shared error classes (AuthFailSPError, InvalidDataSPError,
HttpNotOkAPIError, NoRevAPIError, RemoteFileNotFoundAPIError,
MissingCredentialsSPError, MissingRefreshTokenAPIError,
TooManyRequestsAPIError, UploadRevToMatchMismatchAPIError,
PotentialCorsError, RemoteFileChangedUnexpectedly, EmptyRemoteBodySPError)
plus AdditionalLogErrorBase and extractErrorMessage into
@sp/sync-providers. App-side sync-errors.ts becomes a re-export shim
so existing call sites and instanceof checks keep working.
The moved AdditionalLogErrorBase drops its constructor-time
OP_LOG_SYNC_LOGGER.log side effect (Option A from the slice design):
privacy responsibility shifts entirely to catch-site logging via the
injected SyncLogger port. A new app-side identity spec asserts the
constructor identity is preserved across import paths so future bundler
or tsconfig drift can't silently break instanceof catches.
HttpNotOkAPIError splits its parsed body excerpt off .message onto a
new opt-in .detail field; getErrorTxt forwards .detail to UI surfaces
so user-visible toasts remain unchanged while privacy-aware logger
paths see only "HTTP <status> <statusText>".
TooManyRequestsAPIError's constructor is narrowed to accept only
{ status, retryAfter?, path? } — closing a latent bearer-token leak
where Dropbox's _handleErrorResponse passed the raw Authorization
header through additionalLog. Callers in dropbox-api and
webdav-http-adapter updated accordingly.
Package gains "sideEffects": false to unlock tree-shaking through the
barrel for consumers that import only error classes.
Slice design and round-2 multi-review findings documented in
docs/plans/2026-05-12-pr5-dropbox-slice.md.
Move executeNativeRequestWithRetry and isTransientNetworkError into
@sp/sync-providers behind an injected executor + SyncLogger port. The
package version is platform-agnostic; the app-side shim wires
CapacitorHttp and OP_LOG_SYNC_LOGGER through so existing positional
callers (DropboxApi, SuperSync) keep working unchanged.
The retry policy (2 retries, transient network errors only) is
preserved. Retry log entries now flow as safe SyncLogMeta primitives
(url, attempt, errorName, errorCode) instead of the raw error object,
aligning with the package's privacy-aware logger contract.
