diff --git a/docs/plans/2026-07-14-ios-testflight-master-builds.md b/docs/plans/2026-07-14-ios-testflight-master-builds.md new file mode 100644 index 0000000000..df559d8802 --- /dev/null +++ b/docs/plans/2026-07-14-ios-testflight-master-builds.md @@ -0,0 +1,206 @@ +# Implementation plan: Internal TestFlight builds from `master` + +**Status:** Plan only · **Date:** 2026-07-14 +**Difficulty:** Moderate. Allow about one engineering day split across two small +changes, plus Apple processing and tester verification time. + +## Outcome and scope + +Use the existing iOS signing/archive workflow to send the newest eligible `master` +state to an internal TestFlight group. Rapid pushes are coalesced: one beta may run +and only the newest superseding beta waits. This intentionally does not produce one +TestFlight build for every intermediate SHA. + +Production behavior stays event-based: final `v*` tag pushes submit the existing iOS +release for review, prerelease tag pushes remain upload-only, and manual dispatch can +never submit to production. Master builds are marked **TestFlight Internal Only**, so +Apple cannot offer them to external testers or customers. + +Out of scope: external TestFlight, app changes, new dependencies, and the existing +iOS/macOS App Store review-submission race. There is no new app-wide upload queue; +the internal-only beta path neither edits App Store metadata nor opens a review. + +## Routing contract + +| Event | Export | Fastlane lane | Production submit | +| -------------------------------------- | ------------------------ | ------------- | ----------------- | +| Push to `master` | TestFlight Internal Only | `ios beta` | Never | +| Push of final `v*` tag | App Store Connect | `ios release` | Yes | +| Push of prerelease `v*` tag | App Store Connect | `ios release` | No | +| Manual dispatch, default `beta` mode | TestFlight Internal Only | `ios beta` | Never | +| Manual dispatch, `release-upload-only` | App Store Connect | `ios release` | Never | + +The production guard must include the event type, not only the ref: + +```yaml +SUBMIT_FOR_REVIEW: >- + ${{ github.event_name == 'push' + && startsWith(github.ref, 'refs/tags/v') + && !contains(github.ref, '-') }} +``` + +Manual beta dispatch is allowed only from `master`. The explicit +`release-upload-only` mode preserves the workflow's current manual upload capability +without inheriting its final-tag submission footgun. + +## Phase 0: Apple and version prerequisites + +- Create one internal tester group, add the intended App Store Connect users, and + enable automatic distribution. Accept that eligible tag uploads will also reach + this group because automatic distribution is app/group configuration, not + Git-ref-aware. +- Create a dedicated App Store Connect **team** API key with the Developer role for + beta uploads. Store it as `ASC_*` secrets in an `internal-testflight` GitHub + environment restricted to `master`, with no reviewer gate. A team key cannot be + app-scoped; the separate lower-role key reduces privilege and credential reuse but + still has team-wide upload access. +- Keep the existing App Manager key on the release route. Record secret names, roles, + and certificate/profile expiry only—never key or tester content. +- On the pinned Xcode 26.2 runner, confirm `xcodebuild -help` supports the + `testFlightInternalTestingOnly` export option and that the current provisioning + profile can export that distribution type. +- Validate the build-number examples below with `agvtool`, the archived IPA, and + Apple before enabling unattended builds. + +## Increment A: Add a safe manual beta path + +Do not add the `master` push trigger yet. + +### Fastlane + +Add `ios beta` to `fastlane/Fastfile`. It requires `IPA_PATH` and the existing +`ASC_*` contract, calls `upload_to_testflight`, and sets these options explicitly: + +```ruby +skip_submission: true +skip_waiting_for_build_processing: false +distribute_external: false +submit_beta_review: false +wait_processing_timeout_duration: 1800 +``` + +Do not pass tester groups, changelog, or notification options. App Store Connect's +automatic group owns internal distribution. A processing timeout means “upload +succeeded, processing unknown”; inspect App Store Connect before retrying. Never +re-upload the same IPA blindly. + +### Workflow and versioning + +Refactor `.github/workflows/build-ios.yml` into one build job and conditional beta +and release upload jobs: + +- Add `workflow_dispatch.mode` with `beta` as the safe default and + `release-upload-only` as the other choice. +- Set top-level `permissions: contents: read`. +- Export beta runs with `testFlightInternalTestingOnly: true`; tag and manual release + uploads keep the current App Store Connect export. +- Preserve the exact stripped `package.json` marketing version on release routes. +- For betas, set `CFBundleShortVersionString` to + `incrementPatch(max(stripPrerelease(package.version), highest stable vX.Y.Z tag))`. + Fetch tags and implement the strict three-integer comparison without a dependency. + This future train must be greater than the latest approved iOS version, including + immediately after a release. +- Set `CFBundleVersion` exactly to + `$(date -u +%Y%m%d%H%M).${GITHUB_RUN_NUMBER}.${GITHUB_RUN_ATTEMPT}` on every route. + This remains above the old timestamp values and separates same-minute runs and + reruns. +- Verify both values in every relevant target and in the exported IPA. Also confirm + whether `VERSIONING_SYSTEM = apple-generic` must be added for reliable `agvtool` + behavior. +- Move certificate/profile installation to immediately before export; the archive is + already unsigned. Reuse of the shared Apple Distribution certificate remains an + accepted residual risk on every master build. +- Pass exactly one IPA between jobs using pinned artifact actions, a run/attempt- + specific name, `if-no-files-found: error`, a clean download directory, and one-day + retention for beta artifacts. The upload jobs must fail unless exactly one regular + `.ipa` exists. +- Bind only the beta upload job to `internal-testflight`; keep release credentials on + the existing release path. Never enable verbose fastlane output. + +Add workflow-level concurrency for the complete beta build/upload lifecycle: beta +runs share a fixed group with `cancel-in-progress: false` and `queue: single`, keeping +the running run and only the newest pending run. Tag and manual release runs use a +run/attempt-specific group, so a master push cannot coalesce them. + +### Static verification + +- `ruby -c fastlane/Fastfile` +- `bundle exec fastlane lanes` on the macOS runner +- `npx prettier --check .github/workflows/build-ios.yml` +- `git diff --check` +- Exercise the version calculation with an old timestamp build, two same-minute + runs, a rerun, prerelease package versions, and a stable tag newer than the package. +- Review every routing-table case and confirm the `master` push trigger is absent. +- Confirm no dependency or secret content was added. + +## Live gate between increments + +Merge Increment A, then manually dispatch `beta` from `master` once. Do not proceed +until all of the following are true: + +- Apple accepts and finishes processing the expected version/build pair. +- App Store Connect shows the **Internal** indicator and does not allow external or + customer distribution. +- The automatic internal group receives it and one tester can install and launch it. +- No external Beta App Review, App Review submission, App Store version, or release + metadata is created or changed. +- Logs and artifacts expose no signing or API-key material. + +Record only the Actions URL, non-sensitive version/build pair, duration, and result. + +## Increment B: Enable `master` and document operations + +Add the branch trigger while retaining the existing tag trigger: + +```yaml +push: + branches: [master] + tags: ['v*'] +``` + +Update `docs/build-and-publish-notes.md`, `docs/apple-release-automation.md`, and +`.github/SECURITY-SETUP.md` with the routing table, internal-only boundary, +credentials, version/build formulas, coalescing, timeout recovery, and rollback. + +After merge, confirm one normal `master` push uploads successfully. Start three beta +dispatches close together and verify the active run completes, the middle pending +run is superseded, and the newest pending run proceeds. A failed Apple +upload/processing result must leave Actions red. + +The next tagged release is a follow-up observation, not an activation blocker: + +- verify the release lane and submission behavior are unchanged; +- expect the automatic internal group to receive the eligible release build; and +- after it reaches Ready for Distribution, verify the next master beta advances to + the following patch train and Apple accepts it. + +## Cost, rollback, and risks + +Each beta that survives coalescing consumes roughly 10–15 macOS runner minutes plus +one signed IPA upload. Track `accepted beta runs × duration` and artifact storage for +the first week. Keep one-day beta artifact retention; consider a proven-safe +docs-only path filter later only if cost/noise is material. + +Rollback is to remove the `master` branch trigger while retaining manual beta mode. +If a distributed build must stop, use App Store Connect **Expire Build**; disabling +automatic group distribution affects future assignment but does not retract an +installed build. Keep the new version/build scheme after Apple has accepted it. + +| Risk | Mitigation | +| ----------------------------------------------------- | ------------------------------------------------------------------------------------------- | +| Beta reaches external testing or production | Internal-only export plus fail-closed beta lane | +| Post-release betas are rejected as an old version | Future patch train based on package and stable tags; mandatory post-release check | +| Rapid pushes create cost and TestFlight noise | Workflow-wide beta coalescing and one-day artifacts | +| Manual/tag routing submits the wrong build | Explicit modes, event-type production guard, routing-table review | +| Production credentials gain exposure | Dedicated Developer beta key; release key stays on release job; protected master/CODEOWNERS | +| Signing identity gains exposure on every master build | Install only at export, restrict workflow changes, accept and document certificate reuse | +| Apple processing times out after upload | Inspect App Store Connect first; rebuild with a new number only when retry is required | + +## References to recheck when implementing + +- [Apple: Distributing beta builds](https://developer.apple.com/documentation/xcode/distributing-your-app-for-beta-testing-and-releases) +- [Apple: Internal testers and internal-only builds](https://developer.apple.com/help/app-store-connect/test-a-beta-version/add-internal-testers) +- [Apple: Build/version identifiers](https://developer.apple.com/documentation/bundleresources/information-property-list/cfbundleversion) +- [Apple: Marketing-version identifiers](https://developer.apple.com/documentation/bundleresources/information-property-list/cfbundleshortversionstring) +- [fastlane: `upload_to_testflight`](https://docs.fastlane.tools/actions/upload_to_testflight/) +- [GitHub Actions: concurrency](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency) diff --git a/docs/plans/2026-07-14-microsoft-365-calendar-provider.md b/docs/plans/2026-07-14-microsoft-365-calendar-provider.md new file mode 100644 index 0000000000..c60822a42c --- /dev/null +++ b/docs/plans/2026-07-14-microsoft-365-calendar-provider.md @@ -0,0 +1,703 @@ +# Microsoft 365 Calendar provider (Outlook without an iCal URL) + +Status: **proposed, revised after repository audit and adversarial Codex review** · +2026-07-14 + +## Decision summary + +This is feasible, but it is not a mechanical rename of the Google Calendar plugin. +Microsoft Graph event mapping is the easy half. The harder half is operating a +multitenant Microsoft Entra application reliably in university tenants and closing +several generic plugin-host gaps around OAuth, polling, throttling, all-day dates, and +device-local credentials. + +The smallest responsible product is a bundled, **read-only, Electron-only Microsoft +365 Calendar provider for work/school accounts**. It should let a user authenticate +without an iCal URL, select their own calendars, see events in Schedule/Planner, +manually turn an event into a task, and open the original event in Outlook. + +Do not begin the production implementation until the Phase 0 tenant gate succeeds. +Code cannot bypass a university's consent, Conditional Access, or enterprise-app +policy. + +### Difficulty and estimate + +| Outcome | Estimate | Confidence | +| ----------------------------------------------------------- | ----------------------------------------: | --------------------------------- | +| Disposable Graph/tenant feasibility spike | 1–2 engineering days | Medium; tenant policy is external | +| Production Electron MVP, including host hardening and tests | 17–26 additional focused engineering days | Medium | +| Total engineering effort | About 4–6 weeks for one engineer | Medium | +| Microsoft publisher/admin approval | Not included | Unbounded external elapsed time | + +A demo that only logs in and lists events could be built in a few days. Calling that +production-ready would hide the main risks found in review: unsupported mobile flows +currently start OAuth, all calendar providers can be polled at the fastest provider's +cadence, linked tasks cause per-task Graph requests, refresh-token persistence is not +transactional, and synced calendar IDs can meet credentials for a different local +account. + +## User problem + +Some universities require Outlook/Microsoft 365 but disable calendar publication, so +the existing URL-based iCal integration cannot be used. The requested workflow is +personal planning, not team calendar management: + +1. Sign in with a university Microsoft 365 account. +2. Select one or more personal calendars. +3. See upcoming events beside tasks when planning a realistic day. +4. Convert an event into a task when useful. +5. Open the source event in Outlook. + +This fits Super Productivity's deep-work scope as an optional integration. It must be +quiet by default, read-only, least-privilege, and safe when offline or disconnected. + +## What can be reused + +The repository already has most of the structural pieces: + +- `packages/plugin-dev/google-calendar-provider/` demonstrates a bundled OAuth + agenda provider, dynamic calendar selection, recurrence expansion through a remote + API, event-to-task mapping, and provider-local tests. +- `src/app/plugins/oauth/` supplies authorization-code + PKCE flows, Electron + loopback callbacks, token refresh, and local IndexedDB token storage. +- `packages/plugin-api/src/issue-provider-types.ts` already represents agenda events + with start, duration, all-day, due-time, and source URL fields. +- `src/app/features/calendar-integration/` already combines plugin events with iCal + events and exposes task creation and source-link actions. +- `packages/plugin-dev/scripts/build-all.js`, `src/app/plugins/plugin.service.ts`, and + `electron/bundled-plugin-ids.test.cjs` define the bundled-plugin build and reserved-ID + path. + +The provider should extend these building blocks. It should not introduce a second +calendar framework, a Microsoft SDK, a backend token broker, or a new root dependency. + +## Corrections made during the double-check + +The initial outline was too optimistic in the following ways. These are requirements, +not optional polish: + +- Missing mobile client IDs do **not** currently disable native OAuth. The host needs + an explicit additive platform-capability contract. +- Agenda refresh currently uses the minimum interval across all calendar providers. + A one-minute Google provider can therefore make a five-minute Microsoft provider + call Graph every minute. +- Agenda-view configurations hide the auto-poll setting while the default remains on. + Imported plugin-calendar tasks can consequently generate one `getById` request per + task on every issue-poll cycle. +- `/me/calendars` can expose locally represented shared/delegated calendars. The MVP + must filter to calendars owned by the signed-in mailbox instead of merely hiding a + label in the UI. +- Agenda loading and issue search are independent host paths. A claim of “local + search” requires an explicitly keyed provider cache and in-flight deduplication. +- Refreshed token persistence must finish before a new access token is returned, and + a late refresh must not resurrect credentials after disconnect or overwrite a newer + login. +- Transient refresh failures and terminal reauthentication failures need different + state transitions. A 429, timeout, offline error, or 5xx must not delete a usable + refresh token. +- Microsoft requires clients to respect `Retry-After`. A small typed error extension + is preferable to blind sleeps or exposing every response header as a permanent + plugin API. +- Calendar selection is synced, while OAuth credentials are local and keyed once per + plugin. The provider must detect calendar IDs from a different account and explain + that all configurations on one device share one Microsoft login. +- An all-day event needs an explicit `dueDay` date string. Reconstructing it from a + UTC millisecond timestamp can shift it by a day when mailbox and device time zones + differ. +- Event metadata is cached unencrypted in local storage. Authentication changes must + purge the affected provider's cache, while transient offline failures may retain it. +- Plugin translations require `i18n.languages: ["en"]`, `i18n/en.json`, build copying, + and `PluginAPI.translate`; copying the current Google scaffold literally would miss + those requirements. + +## MVP scope + +### Included + +- Microsoft 365 work/school accounts in the global Microsoft cloud. +- Super Productivity Electron builds on Windows, macOS, and Linux. +- One Microsoft account per plugin per device. +- Up to 10 calendars owned by the signed-in mailbox. +- A fixed event window from 7 days before local today through 28 days after local + today. +- Single events, recurring occurrences/exceptions, and multi-day/all-day events. +- Schedule/Planner display, bounded title search over the provider cache, manual task + creation, and opening the Outlook web link. +- Read-only delegated permission `Calendars.ReadBasic` plus `offline_access`. +- Stale cached agenda data during transient offline/server failures, clearly + distinguishable from a reconnect-required state. + +### Explicitly excluded + +- Creating, editing, moving, completing, or deleting Outlook events. +- Time-block write-back and Google feature parity. +- Event bodies/notes, attachments, extensions, attendees, or meeting chat data. +- Shared, delegated, group, room, or resource calendars. +- Personal Outlook.com accounts, guest-only accounts, and national/sovereign clouds. +- Web, Android, and iOS support. +- Multiple Microsoft accounts on one device. +- Automatic backlog import. +- Automatic refresh of imported tasks. The task is a manually created planning + snapshot with a source link; this avoids an unbounded per-task Graph polling path. + +The exclusions should appear in the setup copy and documentation, not only in code +comments. + +## Architecture + +```text +Microsoft Entra authorization (system browser + PKCE) + | + v +device-local OAuth token store (one account per plugin, never synced) + | + v +Microsoft provider -> validated Graph client -> bounded in-memory event cache + | | + | +-> local title search / getById reuse + v +existing issue-provider agenda contract + | + v +calendar integration cache -> Schedule/Planner -> manual task snapshot +``` + +Provider configuration, including selected calendar IDs, remains part of synced issue +provider state. Tokens and the provider's event cache remain device-local. On each +device, the selected IDs must be reconciled against the calendars returned for that +device's connected account before Graph event calls begin. + +## Fixed contracts and limits + +These values make “bounded” testable and avoid adding settings before a real workflow +requires them: + +| Concern | MVP rule | +| ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Tenant endpoint | `organizations` authorization/token endpoints | +| Delegated scopes | `offline_access https://graph.microsoft.com/Calendars.ReadBasic` | +| Calendar ownership | Compare each calendar owner to the default calendar's owner; if ownership is missing/ambiguous, expose only the default calendar and fail closed for the rest | +| Selected calendars | 1 required, 10 maximum | +| Event window | Local start-of-day −7 days through local start-of-day +28 days, sent as explicit-offset instants | +| Graph page size | `$top=100` | +| Pagination | At most 5 pages per calendar and 2,000 mapped events total | +| Calendar request concurrency | 2 | +| Request timeout | 30 seconds | +| Agenda cadence | 5 minutes per Microsoft provider; manual refresh may bypass the due time | +| Overlap | One in-flight fetch per account/config cache key; later automatic ticks reuse it | +| Search | Case-insensitive title search, 50 results maximum, over the same bounded cache | +| Retry without `Retry-After` | At most 2 retries using approximately 1 s and 2 s exponential delay plus jitter | +| Retry with `Retry-After` | Do not retry before the supplied time; retry once only when the wait is at most 30 s, otherwise stop and retain stale data | +| Timed event identity | Composite, reversible encoding of case-sensitive calendar ID + immutable event ID | +| All-day identity/date | Preserve `YYYY-MM-DD` start and exclusive end dates separately from numeric schedule instants | +| Content limits | Validate response shapes; cap IDs/URLs/titles to documented local constants before mapping | + +If a cap is reached, fail that refresh with a safe localized “calendar result limit +reached” error and keep the last complete provider snapshot. Do not silently replace a +complete cache with a truncated one. + +### Failure semantics + +- **Offline, timeout, 429, or 5xx:** keep tokens and the last complete agenda cache; + retry only within the table's budget. +- **One selected calendar returns 403/404:** fail the refresh and retain the last + complete snapshot; setup/test-connection identifies the inaccessible calendar and + requires reselection. This is intentionally all-or-nothing for the MVP because the + host cache is provider-wide, not per remote calendar. +- **401:** force-refresh the access token once and retry the Graph request once. A + second 401, `invalid_grant`, `interaction_required`, or a claims challenge becomes + reconnect-required and purges the affected provider's cached event metadata. +- **Malformed Graph data:** reject the malformed item. If rejected items or paging + limits make the result incomplete, reject the refresh and keep the prior complete + snapshot. +- **No previous cache:** show no events and a localized actionable error; never invent + an empty successful result for an authentication or completeness failure. +- **Account/config mismatch:** make no `calendarView` calls. Ask the user to reselect + calendars for the locally connected account. + +## External feasibility gate (Phase 0) + +This phase deliberately precedes repo implementation. + +1. Create a Super Productivity-owned, multitenant public-client Entra registration + limited to accounts in organizational directories. Do not add a client secret. +2. Register the Electron loopback redirect + `http://127.0.0.1:/` through the Entra + application manifest. Microsoft does not treat a literal `127.0.0.1` port as + interchangeable, so the exact URI matters. +3. Configure only `Calendars.ReadBasic`; request `offline_access` in the OAuth flow. +4. Record whether the project can satisfy Microsoft publisher-verification + prerequisites. Many education tenants restrict unverified multitenant apps even + when a delegated permission is normally user-consentable. +5. Prove authorization, PKCE token exchange, refresh, `/me/calendars`, and one + `calendarView` call with: + - an ordinary Microsoft 365 tenant; + - a representative restrictive university tenant, ideally the reporting user's; + - consent denied and admin-approval-required paths. +6. Verify the chosen fixed port on all three desktop OSes. Confirm that a port + collision produces a clear actionable failure rather than a timeout. +7. Verify that Graph responses expose the fields required under + `Calendars.ReadBasic`: calendar owner/default flags, event subject/start/end, + `isAllDay`, cancellation/response status, immutable ID, and `webLink`. + +**Go:** at least one representative university user can consent, or the university has +a realistic documented admin-approval route; the app registration can be operated and +published by the project; all required fields are available at read-basic scope. + +**No-go:** representative tenants categorically block the app with no workable approval +path, publisher requirements cannot be met, or required event/ownership fields demand +a broader permission the project is unwilling to request. In that case, answer the +user honestly; no client implementation can override the policy. + +The spike should leave a short evidence note with tenant type, requested scopes, +redirect used, success/failure category, and redacted screenshots/errors. Never commit +tokens, tenant IDs, user addresses, authorization codes, or client secrets. + +## Dependency order + +```text +Phase 0 tenant gate + -> host OAuth safety + -> host HTTP/polling/calendar contracts + -> provider Graph slices + -> cross-tenant/manual verification + -> docs and pilot +``` + +Stop at each checkpoint if the preceding contract cannot be made reliable without a +larger architectural change. + +## Ordered implementation tasks + +Each task is intentionally reviewable on its own. File lists are expected touch points, +not permission to broaden the task. + +### 1. Add an explicit OAuth platform-capability contract + +**Depends on:** Phase 0 green. + +Add an optional, backward-compatible `supportedPlatforms` field to +`OAuthFlowConfig`. Existing plugins that omit it keep current behavior; the Microsoft +provider declares Electron only. Enforce the field before preparing a redirect server +or opening a browser. + +Likely files: + +- `packages/plugin-api/src/types.ts` +- `src/app/plugins/oauth/resolve-effective-oauth-config.util.ts` +- `src/app/plugins/oauth/resolve-effective-oauth-config.util.spec.ts` +- `src/app/plugins/oauth/plugin-oauth-bridge.service.ts` +- `src/app/plugins/oauth/plugin-oauth-bridge.service.spec.ts` + +Acceptance: web/native attempts fail before any OAuth side effect; Google behavior is +unchanged. Verify with targeted specs and `npm run checkFile` for every changed TypeScript +file. Estimate: 0.5 day. + +### 2. Make unsupported-platform UX generic + +**Depends on:** Task 1. + +Replace the web-only availability check in the provider dialog with the new platform +contract and a localized desktop-only explanation. + +Likely files: + +- `src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts` +- `src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.html` +- `src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.spec.ts` +- `src/assets/i18n/en.json` + +Acceptance: unsupported builds show a disabled connect action and never start OAuth; +Electron remains connectable. Estimate: 0.5 day. + +### 3. Make token refresh an awaited, generation-guarded transaction + +**Depends on:** Task 1. + +Define internal refresh outcomes for success, transient failure, and terminal +reauthentication. On success, replace a rotated refresh token (or retain the old one if +none is returned), persist the full new token set, and only then return the access +token. Increment a per-plugin generation on connect/disconnect; a late refresh from an +older generation must be discarded and must not write to memory or IndexedDB. Deduplicate +concurrent refreshes within the same generation. + +Expose an additive force-refresh option for the one-retry-on-401 path and emit a +plugin-session-changed event after connect, disconnect, or terminal invalidation. +Transient network/429/5xx errors retain credentials. + +Likely files: + +- `src/app/plugins/oauth/plugin-oauth.model.ts` +- `src/app/plugins/oauth/plugin-oauth.service.ts` +- `src/app/plugins/oauth/plugin-oauth.service.spec.ts` +- `src/app/plugins/oauth/plugin-oauth-bridge.service.ts` +- `src/app/plugins/oauth/plugin-oauth-bridge.service.spec.ts` + +Acceptance: tests cover rotation, no rotation, persistence failure, refresh +deduplication, disconnect during refresh, reconnect/account switch during refresh, +restart restoration, transient failure, terminal failure, and forced refresh. No token +or response body is logged. Estimate: 2–3 days. + +### 4. Harden the Electron loopback callback + +**Depends on:** Phase 0's exact redirect. + +At `PLUGIN_OAUTH_START`, extract the expected state and callback path from the validated +authorization URL. The loopback server must ignore unrelated paths and wrong-state +requests without marking the flow handled or closing. Close only after a matching +callback, timeout, explicit cancellation, or startup error. Keep the existing clear +`EADDRINUSE` error for the fixed port. + +Likely files: + +- `electron/plugin-oauth.ts` +- a small pure callback-validation helper and focused test beside it + +Acceptance: wrong path/state cannot consume the real callback; correct error and code +callbacks complete once; collision and timeout clean up. Estimate: 0.5–1 day. + +### 5. Add a narrow typed plugin HTTP error contract + +**Depends on:** none after Phase 0; land before Graph retry logic. + +Add an optional typed error shape containing only normalized `status`, +`retryAfterMs`, and a stable error category. Parse both seconds and HTTP-date forms of +`Retry-After`. Do not expose arbitrary headers and do not change successful response +shapes. Keep the addition backward-compatible for existing plugins. + +Likely files: + +- `packages/plugin-api/src/issue-provider-types.ts` +- `src/app/plugins/issue-provider/plugin-issue-provider.model.ts` +- `src/app/plugins/issue-provider/plugin-http.service.ts` +- `src/app/plugins/issue-provider/plugin-http.service.spec.ts` + +Acceptance: Electron and supported native paths produce the same safe error fields for +401/403/404/429/5xx/timeout; existing consumers still receive their expected data. +Estimate: 1–1.5 days. + +### 6. Enforce per-provider agenda cadence and no overlap + +**Depends on:** none; required before enabling the provider. + +The combined calendar timer may continue waking at the smallest configured interval, +but it must call only providers that are due. Track last attempt/success and an in-flight +promise per provider ID. A manual refresh marks providers due; an automatic tick never +starts a second request for a provider already in flight. + +Likely files: + +- `src/app/features/calendar-integration/calendar-integration.service.ts` +- `src/app/features/calendar-integration/calendar-integration.service.spec.ts` + +Acceptance: with Google at one minute and Microsoft at five, Microsoft is called once +per five minutes; slow requests never overlap; enable/disable and provider removal clear +cadence state. Estimate: 1–1.5 days. + +### 7. Add a default-auto-poll capability for agenda providers + +**Depends on:** none; additive public contract. + +Add `defaultAutoPoll?: boolean` to the issue-provider manifest plumbing. Preserve the +current default for existing plugins; Microsoft sets it to false. This prevents hidden +agenda-view defaults from starting per-linked-task Graph polling. + +Likely contract/plumbing files: + +- `packages/plugin-api/src/issue-provider-types.ts` +- `src/app/plugins/issue-provider/plugin-issue-provider.model.ts` +- `src/app/plugins/issue-provider/plugin-issue-provider-registry.service.ts` +- `src/app/plugins/issue-provider/plugin-issue-provider-registry.service.spec.ts` +- `src/app/plugins/plugin-bridge.service.ts` + +Then apply it in the provider setup model and cover it in the dialog spec. Acceptance: +new Microsoft configurations store `isAutoPoll: false`; Google and existing providers +retain current defaults. Estimate: 0.5–1 day. + +### 8. Preserve date-only due dates and canonical URLs through the agenda contract + +**Depends on:** contract review checkpoint. + +Add optional `dueDay?: string` to `PluginSearchResult` and +`CalendarIntegrationEvent`. Preserve `dueDay` and `url` when mapping plugin agenda +results. Validate `dueDay` as `YYYY-MM-DD`; the issue adapter must prefer it over +deriving a date from `start` milliseconds. Event opening prefers the canonical HTTPS +URL and falls back to `getIssueLink`. + +Likely files, split into two small commits if needed: + +- `packages/plugin-api/src/issue-provider-types.ts` +- `src/app/features/calendar-integration/calendar-integration.model.ts` +- `src/app/features/calendar-integration/calendar-integration.service.ts` and spec +- `src/app/plugins/issue-provider/plugin-issue-provider-adapter.service.ts` and spec +- `src/app/features/calendar-integration/calendar-event-actions.service.ts` and spec + +Acceptance: mailbox/device timezone differences never shift all-day task dates; +canonical URL, fallback URL, malformed URL, and ordinary iCal behavior are covered. +Estimate: 1–2 days. + +### 9. Purge only authentication-invalid calendar cache entries + +**Depends on:** Tasks 3 and 8. + +Consume the OAuth session-change event in calendar integration. Remove in-memory and +local-storage entries belonging to provider configurations registered by that plugin +on disconnect, account replacement, or terminal invalidation. Retain the last complete +snapshot for transient offline/429/5xx failures. + +Likely files: + +- `src/app/features/calendar-integration/calendar-integration.service.ts` +- `src/app/features/calendar-integration/calendar-integration.service.spec.ts` +- OAuth event definitions/specs from Task 3 if not already complete + +Acceptance: old account titles/links are not visible after disconnect or reconnect; +offline startup can still show the prior account's cache only while that same OAuth +session remains valid. Estimate: 1 day. + +**Checkpoint A:** run all targeted OAuth, plugin HTTP, calendar integration, issue +adapter, provider-dialog, and Google Calendar tests. Review every additive public type +before starting the Microsoft package. If these changes need a breaking plugin API, +stop and write an architecture decision instead. + +### 10. Scaffold the provider with real i18n + +**Depends on:** Checkpoint A. + +Create `packages/plugin-dev/microsoft-calendar-provider/` with the Google provider's +Vitest/esbuild shape, but follow the current plugin i18n contract rather than copying +Google's hard-coded labels. + +Required assets: + +- permanent manifest ID `microsoft-calendar-provider`; +- `i18n.languages: ["en"]` and `i18n/en.json`; +- a build script that copies manifest, icon, and English translations; +- `PluginAPI.translate` for every user-facing provider string; +- OAuth and HTTP permissions only; no node execution; +- `useAgendaView: true`, five-minute agenda interval, + `defaultAutoAddToBacklog: false`, and `defaultAutoPoll: false`; +- no web/mobile client IDs and `supportedPlatforms: ["electron"]`. + +Likely package/tooling files: `package.json`, `package-lock.json`, `tsconfig.json`, +`vitest.config.ts`, and `scripts/build.js`. Keep runtime code dependency-free; scoped +build/test packages mirror the existing plugin. Estimate: 1 day. + +### 11. Implement pure Graph boundary parsing and mapping + +**Depends on:** Task 10. + +Create small typed modules for config validation, Graph response validation, composite +IDs, URL allowlisting, and date mapping. Treat every Graph response and `nextLink` as +untrusted. Every bearer-authenticated absolute URL must be HTTPS with hostname exactly +`graph.microsoft.com`; reject credentials, alternate ports, lookalike suffixes, and +redirect-derived hosts. + +Mapping rules: + +- request `Prefer: IdType="ImmutableId"` on every event request; +- use a reversible calendar-ID + event-ID composite key and preserve case; +- filter cancelled events and, by calm default, events the user declined; +- use a localized “Untitled event” fallback for an empty subject; +- timed values become real instants using the supplied Graph timezone/offset; +- all-day start/end retain date strings, with end exclusive; numeric schedule instants + are constructed from local date boundaries solely for display; +- multi-day duration uses local date boundaries so 23/25-hour DST days still occupy the + correct calendar days; +- accept only safe HTTPS Outlook `webLink` values. + +Acceptance: pure tests cover invalid shapes, oversized fields, hostile `nextLink`, ID +case, recurrence instances/exceptions, missing titles, timed timezone offsets, mailbox +timezone different from device timezone, travel, DST, and multi-day all-day events. +Estimate: 2 days. + +### 12. Connect OAuth and load only owned calendars + +**Depends on:** Tasks 3, 4, 10, and 11. + +Configure authorization-code + PKCE against the `organizations` endpoints with the +Phase 0 client ID and redirect. Load `/me/calendars` after connection. Establish the +mailbox owner from the default calendar, filter calendars to the same normalized owner, +and fail closed as specified when owner data is absent. Do not persist or log the owner +address. + +The multi-select is required and capped at 10. Before saving/testing, reconcile synced +selected IDs with the locally returned owned-calendar IDs. If none or only some match, +show an account-mismatch/reselection error and make no event calls. Setup copy must say +that disconnect/reconnect affects every Microsoft Calendar configuration on the device. + +Acceptance: ordinary own calendars appear; shared/delegated calendars do not; a config +synced from the same account works; a config synced from a different account fails +safely. Estimate: 1–1.5 days. + +### 13. Implement the bounded event fetch and provider cache + +**Depends on:** Tasks 5, 6, 11, and 12. + +Fetch each selected calendar's `calendarView` using the fixed window and numeric limits. +Follow only validated `@odata.nextLink` values. Limit concurrency, enforce one in-flight +promise per cache key, and implement the fixed failure/retry semantics above. + +Use a session-memory cache keyed by a one-way in-memory account-owner fingerprint plus +the sorted selected calendar IDs and window. Never persist the fingerprint. Before the +first agenda load, search may populate the same cache once; afterward `searchIssues` +filters it locally. Clear it on connect, disconnect, selection change, or OAuth terminal +invalidation. + +Acceptance: tests prove pagination caps, total-event cap, 429 with both Retry-After +forms, long Retry-After abort, 5xx backoff, timeout, 401 force-refresh-once, terminal +reconnect, no overlapping fetches, cache isolation, and all-or-nothing retention of the +last complete snapshot. Estimate: 2–3 days. + +### 14. Register the read-only provider definition + +**Depends on:** Task 13. + +Implement the mandatory issue-provider methods: + +- `getHeaders` obtains the current OAuth access token; +- `testConnection` validates account, selection, and one bounded Graph call; +- `getNewIssuesForBacklog` returns the agenda window; +- `searchIssues` searches the same cache, fetching once only if needed; +- `getById` reuses a fresh cache entry or performs one bounded direct lookup; +- `getIssueLink` uses a cached canonical link with a documented work/school fallback; +- `issueDisplay` shows only non-sensitive basic fields. + +Do not register `createIssue`, `updateIssue`, `deleteIssue`, comments, time-block +methods, or push field mappings. Do not include Graph event bodies in mapped objects. + +Acceptance: a static/spy test proves no write HTTP method or write provider hook exists; +task creation gets title, `dueWithTime` or `dueDay`, duration/time estimate, and source +link as appropriate. Estimate: 1–1.5 days. + +### 15. Bundle and reserve the permanent plugin ID + +**Depends on:** Tasks 10 and 14. + +Register build/copy and discovery atomically: + +- `packages/plugin-dev/scripts/build-all.js` +- `src/app/plugins/plugin.service.ts` +- `electron/bundled-plugin-ids.test.cjs` (verification; change only if the test itself + needs no new behavior) + +Acceptance: built assets include `manifest.json`, `plugin.js`, `icon.svg`, and +`i18n/en.json`; the asset path and reserved manifest ID cannot drift. Estimate: 0.5 day. + +### 16. Document privacy, platform, and tenant limitations + +**Depends on:** working implementation. + +Update user documentation in the same feature PR: + +- `docs/wiki/3.07-Issue-Integration-Comparison.md` +- `docs/wiki/4.24-Integrations.md` +- `docs/wiki/3.05-Web-App-vs-Desktop.md` +- `docs/wiki/3.06-User-Data.md` + +Document exact scopes, read-only behavior, Electron-only support, one local account, +synced selection versus unsynced credentials, university admin approval, and storage: +OAuth tokens in local-only IndexedDB plus basic event metadata/source URLs in the +existing unencrypted calendar local-storage cache. State when each cache is retained or +purged. Estimate: 0.5–1 day. + +### 17. End-to-end verification and pilot + +**Depends on:** all implementation tasks. + +Run: + +- `npm run checkFile ` for every changed `.ts` or `.scss` file; +- targeted root specs for OAuth, plugin HTTP, calendar integration, provider dialog, + issue adapter, and Google Calendar regression; +- Microsoft plugin `npm test`, `npm run typecheck`, and `npm run build`; +- `node --test electron/bundled-plugin-ids.test.cjs`; +- `npm run plugins:build` and a production build appropriate to the release branch. + +Manual matrix: + +- Windows, macOS, and Linux Electron; +- ordinary tenant and representative university tenant; +- first consent, denied consent, admin approval required, expired access token, rotated + refresh token, offline refresh, revoked grant, and reconnect to another account; +- one and ten calendars, same-account synced config, different-account synced config; +- timed, recurring, exception, cancelled, declined, all-day, multi-day, DST, and + mailbox/device timezone mismatch; +- one-minute Google plus five-minute Microsoft cadence; +- disconnect/account switch cache purge and offline stale-cache retention; +- port collision and wrong-path/state loopback requests. + +Pilot with the reporting user before general release. A successful pilot means they can +connect without an iCal URL, select their university calendars, plan from the agenda, +create a task snapshot, and open the Outlook event without broader permissions. +Estimate: 1.5–2 days plus user availability. + +## Security and privacy acceptance criteria + +- No client secret or tenant-specific identifier is committed. +- Only `offline_access` and delegated `Calendars.ReadBasic` are requested. +- Every OAuth flow uses PKCE and state; the loopback listener accepts only the expected + path/state on `127.0.0.1`. +- Bearer tokens are sent only to exact HTTPS Microsoft Graph hosts. +- Redirects and `nextLink` values cannot move a bearer request to another host. +- Graph response values are shape/length validated at the boundary and rendered only + through normal escaped Angular/plugin form paths. +- Logs contain only safe categories/status/counts and opaque internal provider IDs; + never tokens, codes, email addresses, tenant IDs, titles, bodies, event URLs, or raw + Graph error payloads. +- OAuth credentials are local-only and never enter synced `pluginConfig`. +- Event bodies, attendees, and attachments are neither requested nor cached. +- Account replacement, disconnect, and terminal authentication failure purge affected + cached event metadata. +- Automatic and manual request paths share concurrency, pagination, retry, and timeout + bounds. +- No new root dependency is introduced. + +## Release criteria + +Ship only when all are true: + +- Phase 0 passed for a representative education tenant. +- The Entra app registration has a named long-term owner and publisher-verification + decision. +- Unsupported platforms cannot start OAuth. +- Refresh rotation survives restart and cannot race disconnect/reconnect. +- Microsoft calls remain on their own cadence even beside faster providers. +- Imported tasks do not trigger automatic per-task Graph polling. +- Shared/delegated calendars are absent from selection. +- All-day dates remain stable across mailbox/device timezone differences. +- Throttling honors Retry-After and all request/page/event limits are tested. +- Cache retention/purge behavior is tested and documented. +- Existing Google Calendar behavior remains green. +- The university pilot succeeds without broader scopes. + +## Deferred follow-ups + +Consider these only after real demand and a separate design review: + +- Android/iOS support with dedicated Entra redirect/client configuration. +- Web support, including the 24-hour SPA refresh-token lifetime and CORS/reauth design. +- Outlook.com consumer accounts and sovereign-cloud endpoint sets. +- Shared/delegated calendars with explicit permission and ownership UX. +- Multiple accounts per device, which requires changing plugin-global OAuth storage. +- Event write operations and time-block synchronization, which require + `Calendars.ReadWrite`, conflict semantics, and a much larger trust surface. +- Delta queries or change notifications if measured Graph volume justifies the added + state and lifecycle complexity. + +## Official references checked + +- [Microsoft identity platform authorization-code flow with PKCE](https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow) +- [Redirect URI restrictions and loopback rules](https://learn.microsoft.com/en-us/entra/identity-platform/reply-url) +- [Refresh-token replacement and lifetimes](https://learn.microsoft.com/en-us/entra/identity-platform/refresh-tokens) +- [`Calendars.ReadBasic` delegated permission](https://learn.microsoft.com/en-us/graph/permissions-reference#calendarsreadbasic) +- [Tenant user-consent configuration](https://learn.microsoft.com/en-us/entra/identity/enterprise-apps/configure-user-consent) +- [Publisher verification](https://learn.microsoft.com/en-us/entra/identity-platform/publisher-verification-overview) +- [List calendars](https://learn.microsoft.com/en-us/graph/api/user-list-calendars?view=graph-rest-1.0) +- [Shared and delegated Outlook calendars](https://learn.microsoft.com/en-us/graph/outlook-get-shared-events-calendars) +- [Calendar view and recurrence expansion](https://learn.microsoft.com/en-us/graph/api/calendar-list-calendarview?view=graph-rest-1.0) +- [Outlook immutable IDs](https://learn.microsoft.com/en-us/graph/outlook-immutable-id) +- [Microsoft Graph event resource and `webLink`](https://learn.microsoft.com/en-us/graph/api/resources/event?view=graph-rest-1.0) +- [Get an event](https://learn.microsoft.com/en-us/graph/api/event-get?view=graph-rest-1.0) +- [Microsoft Graph throttling and Retry-After](https://learn.microsoft.com/en-us/graph/throttling) +- [Claims challenges and Conditional Access](https://learn.microsoft.com/en-us/entra/identity-platform/claims-challenge) diff --git a/docs/wiki/3.08-Sync-Integration-Comparison.md b/docs/wiki/3.08-Sync-Integration-Comparison.md index 03f22eec29..1dc96706ad 100644 --- a/docs/wiki/3.08-Sync-Integration-Comparison.md +++ b/docs/wiki/3.08-Sync-Integration-Comparison.md @@ -16,7 +16,7 @@ Sync providers **synchronize your full Super Productivity data** (tasks, project | WebDAV | All | URL, username, password | Yes (client-side key) | ownCloud and other WebDAV servers | | Dropbox | All | OAuth 2.0 (no password stored) | Yes (client-side key) | Cloud backup, cross-device sync | | SuperSync (beta) | All | URL, username/password or token | Yes (E2E, server-supported) | Dedicated sync server, self-hosted or hosted | -| Local file | Desktop only | None (folder path) | Yes (client-side key) | Local or network folder backup | +| Local file | Desktop only | None (folder path) | Yes (client-side key) | Single-device local or network-folder backup | **Note:** SuperSync is very new and is still in beta. Prefer Nextcloud, WebDAV, Dropbox, or local file for production use until SuperSync is stable. @@ -30,6 +30,7 @@ Sync providers **synchronize your full Super Productivity data** (tasks, project - **Configuration:** Server URL, username, optional separate login name/email, app password, and sync folder path. The username is used in `/remote.php/dav/files//`; the optional login name/email is only for servers that authenticate with a different value. - **Encryption:** Optional client-side encryption; you set an encryption key that is not sent to the server. - **Platform:** Available on desktop (Electron), web, and mobile. In the browser, CORS can block WebDAV requests; the app may recommend the desktop version for reliable sync. +- **Concurrent writes:** Servers that provide strong ETags get atomic conflict protection. Servers with weak or missing ETags use a best-effort content check; avoid syncing the same folder from multiple devices at exactly the same time on those servers. ### WebDAV @@ -59,6 +60,7 @@ Sync providers **synchronize your full Super Productivity data** (tasks, project - **Configuration:** Folder path only; no remote account. - **Encryption:** Optional client-side encryption with a key you set. - **Platform:** **Desktop (Electron) only.** Not available in the web app or mobile, because those environments cannot reliably access arbitrary local or network paths. +- **Important limitation:** Use this as a single-device backup target. Do not mirror the folder between devices with Syncthing, Resilio, cloud-drive clients, or similar tools: local filesystem access cannot atomically coordinate concurrent writers, so simultaneous syncs can overwrite each other. Use Nextcloud, WebDAV, Dropbox, or SuperSync for multi-device sync. ## Choosing a Provider @@ -66,9 +68,9 @@ Sync providers **synchronize your full Super Productivity data** (tasks, project - **WebDAV** — Use if you have ownCloud or another WebDAV server, or if you need to provide the full WebDAV base URL yourself. - **Dropbox** — Use for cloud-backed sync without running your own server; OAuth keeps account credentials out of the app config. - **SuperSync (beta)** — Use when you want a dedicated sync service and optional server-side E2E encryption. Bear in mind it is very new and still in beta. -- **Local file** — Use on desktop when you want sync to a local or network folder (e.g. backup to an external drive or NAS) without using the cloud. +- **Local file** — Use on desktop for a single-device backup to a local or network folder (e.g. an external drive or NAS), not for a folder mirrored between devices. -Conflict resolution (e.g. when two devices change the same data) is the same for all providers: the app uses last-write-wins or prompts you when appropriate. See [[4.23-Managing-Your-Data]]. +Logical conflict resolution (e.g. when two devices change the same data) uses the same operation rules for all providers. Transport guarantees still differ: Local file is single-writer only, and WebDAV atomic write protection depends on strong ETag support. See [[4.23-Managing-Your-Data]]. ## Related diff --git a/packages/sync-providers/src/file-based-sync-data.ts b/packages/sync-providers/src/file-based-sync-data.ts index aa19f69886..69be3a264d 100644 --- a/packages/sync-providers/src/file-based-sync-data.ts +++ b/packages/sync-providers/src/file-based-sync-data.ts @@ -116,6 +116,16 @@ export interface FileBasedOpsFile< recentOps: SyncFileCompactOp[]; oldestOpSyncVersion?: number; snapshotRef: FileBasedSnapshotRef; + /** + * Present only while converting a legacy v2 file. Readers must finish or + * retry the migration before treating this ops file as committed. Keeping + * the complete candidate ops payload in the commit-point file makes recovery + * possible after a crash on either side of the legacy tombstone write. + */ + migration?: { + status: 'pending'; + legacyRev: string; + }; } /** diff --git a/packages/sync-providers/src/file-based/local-file/local-file-sync-base.ts b/packages/sync-providers/src/file-based/local-file/local-file-sync-base.ts index 96a69d8571..504a390197 100644 --- a/packages/sync-providers/src/file-based/local-file/local-file-sync-base.ts +++ b/packages/sync-providers/src/file-based/local-file/local-file-sync-base.ts @@ -128,10 +128,16 @@ export abstract class LocalFileSyncBase implements FileSyncProvider< }); try { - if (!isForceOverwrite && revToMatch) { + // Best-effort conditional write. The FileAdapter abstraction has no + // cross-process compare-and-swap primitive, so this cannot close the + // read→write race on a network/cloud-mirrored folder. It still rejects + // stale updates, vanished targets, and create collisions observed before + // the write. Local file sync is therefore documented as single-writer / + // backup-only rather than a multi-device transport. + if (!isForceOverwrite) { try { const existingFile = await this.downloadFile(targetPath); - if (existingFile.rev !== revToMatch) { + if (revToMatch === null || existingFile.rev !== revToMatch) { this.logger.critical( `${LocalFileSyncBase.LB}.${this.uploadFile.name}() rev mismatch`, { targetPath }, @@ -139,7 +145,11 @@ export abstract class LocalFileSyncBase implements FileSyncProvider< throw new UploadRevToMatchMismatchAPIError(); } } catch (err) { - if (!(err instanceof RemoteFileNotFoundAPIError)) { + if (err instanceof RemoteFileNotFoundAPIError) { + if (revToMatch !== null) { + throw new UploadRevToMatchMismatchAPIError(); + } + } else { throw err; } } diff --git a/packages/sync-providers/src/file-based/webdav/webdav-api.ts b/packages/sync-providers/src/file-based/webdav/webdav-api.ts index e83e69060e..814809ab6c 100644 --- a/packages/sync-providers/src/file-based/webdav/webdav-api.ts +++ b/packages/sync-providers/src/file-based/webdav/webdav-api.ts @@ -15,6 +15,15 @@ import type { WebDavHttpAdapter, WebDavHttpResponse } from './webdav-http-adapte import { FileMeta, WebdavXmlParser } from './webdav-xml-parser'; import type { WebdavPrivateCfg } from './webdav.model'; +/** + * RFC 7232 strong entity-tag: a quoted string of `etagc` chars only. The class + * excludes CR/LF, all control chars, the inner quote, and DEL, so a value from + * this pattern is safe to place verbatim in an `If-Match` header (no header + * splitting). Weak tags (`W/"..."`) intentionally do not match — they cannot + * drive a conditional write, so callers fall back to the content-hash check. + */ +const STRONG_ETAG_RE = /^"[\x21\x23-\x7e\x80-\xff]*"$/; + export interface WebdavApiDeps { logger: SyncLogger; /** @@ -39,6 +48,31 @@ export class WebdavApi { return computeContentRev(data); } + /** + * Returns an RFC-style strong entity tag from a response header. Weak or + * malformed values cannot safely drive `If-Match`, so callers fall back to a + * content hash and the legacy best-effort check instead. + */ + private _readStrongEtag(headers: Record): string | undefined { + const entry = Object.entries(headers).find(([name]) => name.toLowerCase() === 'etag'); + const etag = entry?.[1]?.trim(); + return etag && STRONG_ETAG_RE.test(etag) ? etag : undefined; + } + + private _isStrongEtag(value: string): boolean { + return STRONG_ETAG_RE.test(value); + } + + private _isHttpStatus(error: unknown, status: number): boolean { + return error instanceof HttpNotOkAPIError && error.response?.status === status; + } + + private _remoteChanged(path: string): RemoteFileChangedUnexpectedly { + return new RemoteFileChangedUnexpectedly( + `File ${path} no longer matches the revision downloaded before this upload.`, + ); + } + // ============================== // File Operations // ============================== @@ -155,7 +189,7 @@ export class WebdavApi { const hash = await this._computeContentHash(response.data); return { - rev: hash, + rev: this._readStrongEtag(response.headers) ?? hash, dataStr: response.data, }; } catch (e) { @@ -193,8 +227,13 @@ export class WebdavApi { const expectedHash = await this._computeContentHash(data); try { - // Application-level conflict detection: download current file and compare hash - if (!isForceOverwrite && expectedRev) { + const strongExpectedRev = + expectedRev && this._isStrongEtag(expectedRev) ? expectedRev : undefined; + + // Servers without a strong ETag retain the legacy content-hash check. This + // detects stale writers but cannot close the GET→PUT race; strong ETags do + // close it through the HTTP precondition attached to the PUT below. + if (!isForceOverwrite && expectedRev && !strongExpectedRev) { try { const currentResponse = await this._makeRequest({ url: fullPath, @@ -202,21 +241,24 @@ export class WebdavApi { }); const currentHash = await this._computeContentHash(currentResponse.data); if (currentHash !== expectedRev) { - throw new RemoteFileChangedUnexpectedly( - `File ${path} was modified on remote (expected rev: ${expectedRev}, got: ${currentHash})`, - ); + throw this._remoteChanged(path); } } catch (e) { - // 404 means file doesn't exist yet — safe to proceed with upload - if (!(e instanceof RemoteFileNotFoundAPIError)) { - throw e; - } + // A revision was supplied, so disappearance is itself a conflicting + // remote change. Proceeding would silently recreate over that change. + if (e instanceof RemoteFileNotFoundAPIError) throw this._remoteChanged(path); + throw e; } } const headers: Record = { [WebDavHttpHeader.CONTENT_TYPE]: 'application/octet-stream', }; + if (!isForceOverwrite && strongExpectedRev) { + headers[WebDavHttpHeader.IF_MATCH] = strongExpectedRev; + } else if (!isForceOverwrite && expectedRev === null) { + headers[WebDavHttpHeader.IF_NONE_MATCH] = '*'; + } // Try to upload the file try { @@ -227,6 +269,12 @@ export class WebdavApi { headers, }); } catch (uploadError) { + if (this._isHttpStatus(uploadError, WebDavHttpStatus.PRECONDITION_FAILED)) { + throw this._remoteChanged(path); + } + if (uploadError instanceof RemoteFileNotFoundAPIError && expectedRev !== null) { + throw this._remoteChanged(path); + } if ( // 404 on upload indicates the directory does not exist (Nextcloud) uploadError instanceof RemoteFileNotFoundAPIError || @@ -252,6 +300,9 @@ export class WebdavApi { headers, }); } catch (retryError) { + if (this._isHttpStatus(retryError, WebDavHttpStatus.PRECONDITION_FAILED)) { + throw this._remoteChanged(path); + } if ( retryError instanceof HttpNotOkAPIError && retryError.response && @@ -277,8 +328,8 @@ export class WebdavApi { } } - const verifiedHash = await this._verifyUpload(path, fullPath, expectedHash); - return { rev: verifiedHash }; + const verifiedRev = await this._verifyUpload(path, fullPath, expectedHash); + return { rev: verifiedRev }; } catch (e) { this._deps.logger.critical(`${WebdavApi.L}.upload() error`, errorMeta(e, { path })); throw e; @@ -333,7 +384,7 @@ export class WebdavApi { `sync cycle will re-download and reconcile.`, ); } - return remoteHash; + return this._readStrongEtag(remoteResponse.headers) ?? remoteHash; } async remove(path: string): Promise { diff --git a/packages/sync-providers/src/file-based/webdav/webdav.const.ts b/packages/sync-providers/src/file-based/webdav/webdav.const.ts index 2f0cb1871c..b2a6ad3e6a 100644 --- a/packages/sync-providers/src/file-based/webdav/webdav.const.ts +++ b/packages/sync-providers/src/file-based/webdav/webdav.const.ts @@ -12,6 +12,7 @@ export const WebDavHttpStatus = { NOT_FOUND: 404, METHOD_NOT_ALLOWED: 405, CONFLICT: 409, + PRECONDITION_FAILED: 412, TOO_MANY_REQUESTS: 429, INTERNAL_SERVER_ERROR: 500, } as const; @@ -36,4 +37,6 @@ export const WebDavHttpHeader = { CONTENT_TYPE: 'Content-Type', CONTENT_LENGTH: 'Content-Length', DEPTH: 'Depth', + IF_MATCH: 'If-Match', + IF_NONE_MATCH: 'If-None-Match', } as const; diff --git a/packages/sync-providers/src/provider-types.ts b/packages/sync-providers/src/provider-types.ts index e0a40e9353..8c6ef16dbd 100644 --- a/packages/sync-providers/src/provider-types.ts +++ b/packages/sync-providers/src/provider-types.ts @@ -54,6 +54,14 @@ export interface FileSyncProvider< getFileRev(targetPath: string, localRev: string | null): Promise; downloadFile(targetPath: string): Promise; + /** + * Conditionally replaces a file when `revToMatch` is a revision returned by a + * prior read. A `null` revision means "create only if absent"; force overwrite + * bypasses the condition. Network providers should enforce the comparison in + * the storage service itself. Providers backed by an API without atomic CAS + * may only offer a documented best-effort check and must not be presented as + * safe for concurrent multi-device writers. + */ uploadFile( targetPath: string, dataStr: string, @@ -135,6 +143,14 @@ export interface SuperSyncOpDownloadResponse extends OpDownloadResponseBase { export interface FileSnapshotOpDownloadResponse extends OpDownloadResponseBase { snapshotState?: unknown; + /** Last modification time recorded by the remote snapshot/ops file. */ + remoteLastModified?: number; + /** + * Operation ids whose effects are already represented by `snapshotState`. + * Operations returned alongside a snapshot but absent from this list must be + * applied on top of the snapshot before the download cursor is committed. + */ + snapshotAppliedOpIds?: string[]; } export type OpDownloadResponse = diff --git a/packages/sync-providers/tests/file-based/local-file/local-file-sync-base.spec.ts b/packages/sync-providers/tests/file-based/local-file/local-file-sync-base.spec.ts index edadb3d22f..aff6a1aa75 100644 --- a/packages/sync-providers/tests/file-based/local-file/local-file-sync-base.spec.ts +++ b/packages/sync-providers/tests/file-based/local-file/local-file-sync-base.spec.ts @@ -182,6 +182,25 @@ describe('LocalFileSyncBase', () => { expect(fileAdapter.getFile('/test/sync/sync-data.json')).toBe('force content'); }); + it('rejects a best-effort create when the target already exists', async () => { + const fileAdapter = new MockFileAdapter(); + const provider = new TestableLocalFileSync(fileAdapter); + fileAdapter.setFile('/test/sync/sync-data.json', 'other client content'); + + await expect( + provider.uploadFile('sync-data.json', 'mine', null), + ).rejects.toBeInstanceOf(UploadRevToMatchMismatchAPIError); + expect(fileAdapter.getFile('/test/sync/sync-data.json')).toBe('other client content'); + }); + + it('rejects when a revision-matched target has disappeared', async () => { + const provider = new TestableLocalFileSync(new MockFileAdapter()); + + await expect( + provider.uploadFile('sync-data.json', 'mine', 'previous-rev'), + ).rejects.toBeInstanceOf(UploadRevToMatchMismatchAPIError); + }); + it('preserves NoRevAPIError identity if hashing returns an empty rev', async () => { vi.mocked(md5).mockResolvedValueOnce(''); const fileAdapter = new MockFileAdapter(); diff --git a/packages/sync-providers/tests/file-based/webdav/webdav-api.spec.ts b/packages/sync-providers/tests/file-based/webdav/webdav-api.spec.ts index 77292a50bd..cc493d6608 100644 --- a/packages/sync-providers/tests/file-based/webdav/webdav-api.spec.ts +++ b/packages/sync-providers/tests/file-based/webdav/webdav-api.spec.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { md5 as md5HashWasm } from 'hash-wasm'; import { NOOP_SYNC_LOGGER, type SyncLogger } from '@sp/sync-core'; import { WebdavApi } from '../../../src/file-based/webdav/webdav-api'; +import { WebDavHttpHeader } from '../../../src/file-based/webdav/webdav.const'; import type { WebDavHttpAdapter, WebDavHttpResponse, @@ -71,9 +72,13 @@ const makeApi = ( httpAdapter: adapter as unknown as WebDavHttpAdapter, }); -const okResponse = (data: string, status = 200): WebDavHttpResponse => ({ +const okResponse = ( + data: string, + status = 200, + headers: Record = {}, +): WebDavHttpResponse => ({ status, - headers: {}, + headers, data, }); @@ -113,6 +118,28 @@ describe('WebdavApi', () => { expect(r.rev).toBe(await md5HashWasm('hello world')); }); + it('uses a strong ETag as the download revision', async () => { + const adapter = makeAdapter(); + adapter.request.mockResolvedValue( + okResponse('hello world', 200, { ETag: '"strong-rev"' }), + ); + + const result = await makeApi(adapter).download({ path: 'op-1.json' }); + + expect(result.rev).toBe('"strong-rev"'); + }); + + it('does not trust a weak ETag as an atomic revision', async () => { + const adapter = makeAdapter(); + adapter.request.mockResolvedValue( + okResponse('hello world', 200, { etag: 'W/"weak-rev"' }), + ); + + const result = await makeApi(adapter).download({ path: 'op-1.json' }); + + expect(result.rev).toBe(await md5HashWasm('hello world')); + }); + it('throws EmptyRemoteBodySPError on empty body', async () => { const adapter = makeAdapter(); adapter.request.mockResolvedValue(okResponse('')); @@ -163,6 +190,63 @@ describe('WebdavApi', () => { expect(adapter.request).toHaveBeenCalledTimes(3); }); + it('uses If-Match atomically when the expected revision is a strong ETag', async () => { + const adapter = makeAdapter(); + const data = 'updated body'; + adapter.request.mockResolvedValueOnce(okResponse('', 204)); + adapter.request.mockResolvedValueOnce(okResponse(data, 200, { etag: '"new-rev"' })); + + const result = await makeApi(adapter).upload({ + path: 'op-1.json', + data, + expectedRev: '"old-rev"', + }); + + expect(result.rev).toBe('"new-rev"'); + expect(adapter.request).toHaveBeenCalledTimes(2); + expect(adapter.request.mock.calls[0]?.[0]).toMatchObject({ + method: 'PUT', + headers: expect.objectContaining({ + [WebDavHttpHeader.IF_MATCH]: '"old-rev"', + }), + }); + }); + + it('uses If-None-Match for an atomic create', async () => { + const adapter = makeAdapter(); + const data = 'new body'; + adapter.request.mockResolvedValueOnce(okResponse('', 201)); + adapter.request.mockResolvedValueOnce(okResponse(data)); + + await makeApi(adapter).upload({ + path: 'op-1.json', + data, + expectedRev: null, + }); + + expect(adapter.request.mock.calls[0]?.[0]).toMatchObject({ + method: 'PUT', + headers: expect.objectContaining({ + [WebDavHttpHeader.IF_NONE_MATCH]: '*', + }), + }); + }); + + it('maps a failed HTTP precondition to RemoteFileChangedUnexpectedly', async () => { + const adapter = makeAdapter(); + adapter.request.mockRejectedValueOnce( + new HttpNotOkAPIError(new Response('', { status: 412 })), + ); + + await expect( + makeApi(adapter).upload({ + path: 'op-1.json', + data: 'mine', + expectedRev: '"stale-rev"', + }), + ).rejects.toBeInstanceOf(RemoteFileChangedUnexpectedly); + }); + it('throws RemoteFileChangedUnexpectedly when remote hash drift detected', async () => { const adapter = makeAdapter(); // GET returns body whose hash differs from expectedRev @@ -177,22 +261,18 @@ describe('WebdavApi', () => { ).rejects.toBeInstanceOf(RemoteFileChangedUnexpectedly); }); - it('proceeds on 404 conditional GET (file does not exist yet)', async () => { + it('rejects when a hash-matched file disappeared before upload', async () => { const adapter = makeAdapter(); - const data = 'new content'; adapter.request.mockRejectedValueOnce(new RemoteFileNotFoundAPIError('op-1.json')); - // PUT succeeds - adapter.request.mockResolvedValueOnce(okResponse('', 201)); - // verify GET - adapter.request.mockResolvedValueOnce(okResponse(data)); - const r = await makeApi(adapter).upload({ - path: 'op-1.json', - data, - expectedRev: 'something', - }); - - expect(r.rev).toBe(await md5HashWasm(data)); + await expect( + makeApi(adapter).upload({ + path: 'op-1.json', + data: 'new content', + expectedRev: 'legacy-content-hash', + }), + ).rejects.toBeInstanceOf(RemoteFileChangedUnexpectedly); + expect(adapter.request).toHaveBeenCalledTimes(1); }); it('throws RemoteFileChangedUnexpectedly when verify-after-upload hash mismatches', async () => { diff --git a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.html b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.html index 638f0a9b4f..1e29f71caf 100644 --- a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.html +++ b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.html @@ -16,10 +16,18 @@ {{ T.F.SYNC.D_CONFLICT.REMOTE | translate }} - {{ remote.lastUpdate | localeDate: 'shortDate' : undefined : locale() }} + @if (remote.lastUpdate !== null) { + {{ remote.lastUpdate | localeDate: 'shortDate' : undefined : locale() }} + } @else { + {{ T.F.SYNC.D_CONFLICT.CHANGES_UNKNOWN | translate }} + } - {{ remote.lastUpdate | shortTime }} + {{ + remote.lastUpdate !== null + ? (remote.lastUpdate | shortTime) + : (T.F.SYNC.D_CONFLICT.CHANGES_UNKNOWN | translate) + }} {{ diff --git a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.spec.ts b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.spec.ts index 84685e3e2f..a3038f8933 100644 --- a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.spec.ts +++ b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.spec.ts @@ -13,11 +13,13 @@ const buildConflictData = (overrides: { remoteVectorClock?: VectorClock; lastSyncedVectorClock?: VectorClock | null; localUnsyncedOpsCount?: number; + remoteLastUpdate?: number | null; }): ConflictData => ({ reason: ConflictReason.NoLastSync, localUnsyncedOpsCount: overrides.localUnsyncedOpsCount, remote: { - lastUpdate: 1000, + lastUpdate: + overrides.remoteLastUpdate === undefined ? 1000 : overrides.remoteLastUpdate, lastUpdateAction: 'Remote data', revMap: {}, crossModelVersion: 1, @@ -63,6 +65,23 @@ describe('DialogSyncConflictComponent', () => { }).compileComponents(); }); + describe('timestamp highlighting', () => { + it('does not present an unknown remote timestamp as the newest write', () => { + const component = createComponent(buildConflictData({ remoteLastUpdate: null })); + + expect(component.isHighlightRemote).toBe(false); + expect(component.isHighlightLocal).toBe(false); + }); + + it('does not recommend either side when timestamps are equal', () => { + const data = buildConflictData({ remoteLastUpdate: 2000 }); + const component = createComponent(data); + + expect(component.isHighlightRemote).toBeFalse(); + expect(component.isHighlightLocal).toBeFalse(); + }); + }); + describe('getChangeCount()', () => { it('(a) returns correct per-client delta when lastSyncedVectorClock is present', () => { const component = createComponent( @@ -91,6 +110,23 @@ describe('DialogSyncConflictComponent', () => { // Bug SPAP-7: previously summed the whole clock (3000 / 1400). Must be null now. expect(component.localChangeCount).toBeNull(); expect(component.remoteChangeCount).toBeNull(); + expect(component.isHighlightLocalChanges).toBeFalse(); + expect(component.isHighlightRemoteChanges).toBeFalse(); + }); + + it('does not recommend either side when known change counts are equal', () => { + const component = createComponent( + buildConflictData({ + localVectorClock: { clientA: 10, clientB: 5 }, + remoteVectorClock: { clientA: 3, clientB: 12 }, + lastSyncedVectorClock: { clientA: 3, clientB: 5 }, + }), + ); + + expect(component.localChangeCount).toBe(7); + expect(component.remoteChangeCount).toBe(7); + expect(component.isHighlightLocalChanges).toBeFalse(); + expect(component.isHighlightRemoteChanges).toBeFalse(); }); it('shows the exact pending-op count when the clock delta under-counts (compaction fold)', () => { diff --git a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.ts b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.ts index 0df592749e..73357c4b19 100644 --- a/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.ts +++ b/src/app/imex/sync/dialog-sync-conflict/dialog-sync-conflict.component.ts @@ -59,14 +59,22 @@ export class DialogSyncConflictComponent { remote = this.data.remote; local = this.data.local; - isHighlightRemote = this.remote.lastUpdate >= this.local.lastUpdate; - isHighlightLocal = !this.isHighlightRemote; + isHighlightRemote = + this.remote.lastUpdate !== null && this.remote.lastUpdate > this.local.lastUpdate; + isHighlightLocal = + this.remote.lastUpdate !== null && this.local.lastUpdate > this.remote.lastUpdate; remoteChangeCount = this.getChangeCount('remote'); localChangeCount = this.getLocalChangeCount(); - isHighlightRemoteChanges = (this.remoteChangeCount ?? 0) > (this.localChangeCount ?? 0); - isHighlightLocalChanges = !this.isHighlightRemoteChanges; + isHighlightRemoteChanges = + this.remoteChangeCount !== null && + this.localChangeCount !== null && + this.remoteChangeCount > this.localChangeCount; + isHighlightLocalChanges = + this.remoteChangeCount !== null && + this.localChangeCount !== null && + this.localChangeCount > this.remoteChangeCount; constructor() { this._matDialogRef.disableClose = true; diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts index fac9800aee..ef7ae48906 100644 --- a/src/app/imex/sync/sync-wrapper.service.spec.ts +++ b/src/app/imex/sync/sync-wrapper.service.spec.ts @@ -25,6 +25,7 @@ import { NetworkUnavailableSPError, OperationIntegrityError, PotentialCorsError, + ConflictData, SyncProviderId, SyncStatus, } from '../../op-log/sync-exports'; @@ -1707,6 +1708,28 @@ describe('SyncWrapperService', () => { expect(mockMatDialog.open).toHaveBeenCalled(); }); + it('uses the remote file timestamp in the conflict dialog', async () => { + const remoteLastModified = 1_720_000_000_000; + const conflictError = new LocalDataConflictError( + 3, + { tasks: [{ id: 'remote-task' }] }, + { clientB: 5 }, + null, + remoteLastModified, + ); + mockSyncService.downloadRemoteOps.and.rejectWith(conflictError); + mockMatDialog.open.and.returnValue({ + afterClosed: () => of(undefined), + } as any); + + await service.sync(); + + const dialogConfig = mockMatDialog.open.calls.mostRecent().args[1] as { + data: ConflictData; + }; + expect(dialogConfig.data.remote.lastUpdate).toBe(remoteLastModified); + }); + it('should call forceUploadLocalState when user chooses USE_LOCAL', async () => { const conflictError = new LocalDataConflictError( 2, diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 34205dbb76..d5294a6ed6 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -1324,7 +1324,7 @@ export class SyncWrapperService { const conflictData: ConflictData = { reason: ConflictReason.NoLastSync, remote: { - lastUpdate: Date.now(), // Remote snapshot doesn't have a timestamp, use now + lastUpdate: error.remoteLastModified ?? null, lastUpdateAction: 'Remote data', revMap: {}, crossModelVersion: 1, diff --git a/src/app/op-log/core/errors/sync-errors.ts b/src/app/op-log/core/errors/sync-errors.ts index 9d6f347646..faf06e65aa 100644 --- a/src/app/op-log/core/errors/sync-errors.ts +++ b/src/app/op-log/core/errors/sync-errors.ts @@ -81,6 +81,8 @@ export class LocalDataConflictError extends Error { // display heuristic, not an exact "unsynced" figure. `null` for genuinely-fresh // clients that have never synced (SPAP-7). public readonly lastSyncedVectorClock?: Record | null, + /** Actual `lastModified` recorded by the downloaded remote file. */ + public readonly remoteLastModified?: number, ) { super(`Local data conflict: ${unsyncedCount} unsynced changes would be lost`); } diff --git a/src/app/op-log/core/types/sync-results.types.ts b/src/app/op-log/core/types/sync-results.types.ts index 09b76f3de1..91060ce802 100644 --- a/src/app/op-log/core/types/sync-results.types.ts +++ b/src/app/op-log/core/types/sync-results.types.ts @@ -90,6 +90,14 @@ export interface FileSnapshotDownloadResult extends DownloadResultBase { * Contains the complete application state for bootstrapping a new client. */ snapshotState?: unknown; + /** Last modification time recorded by the remote snapshot/ops file. */ + remoteLastModified?: number; + /** + * Operation ids already represented by `snapshotState`. When omitted, all + * returned operations are treated as snapshot-included for compatibility + * with file providers that expose a fully current snapshot. + */ + snapshotAppliedOpIds?: string[]; } export type DownloadResult = diff --git a/src/app/op-log/core/types/sync.types.ts b/src/app/op-log/core/types/sync.types.ts index 97b70f33a5..964cf3bb8c 100644 --- a/src/app/op-log/core/types/sync.types.ts +++ b/src/app/op-log/core/types/sync.types.ts @@ -139,7 +139,9 @@ interface MainModelData { [modelId: string]: ModelBase; } -interface RemoteMeta extends MetaFileBase { +interface RemoteMeta extends Omit { + /** Null when the remote format does not provide a trustworthy timestamp. */ + lastUpdate: number | null; mainModelData: MainModelData; isFullData?: boolean; } diff --git a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.spec.ts b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.spec.ts index 9fa6d590e3..968c67c23c 100644 --- a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.spec.ts +++ b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.spec.ts @@ -986,6 +986,7 @@ describe('FileBasedSyncAdapterService', () => { Promise.resolve({ dataStr: addPrefix(first), rev: 'rev-1' }), ); await adapter.downloadOps(1); + await adapter.setLastServerSeq(5); // Writer made ops 6..10 (client never saw them), took a snapshot compacting // 1..10 into `state` and reset recentOps, then made op 11. The file's clock @@ -1008,6 +1009,64 @@ describe('FileBasedSyncAdapterService', () => { // suppression). expect(result.gapDetected).toBe(true); }); + + it('does not commit reset metadata when snapshot application is cancelled', async () => { + const compactOp = ( + id: string, + clock: Record, + syncVersion: number, + ): FileBasedSyncData['recentOps'][number] => ({ + id, + c: 'remote-client', + a: 'HA', + o: 'ADD', + e: 'TASK', + d: id, + v: clock, + t: Date.now(), + s: 1, + p: {}, + sv: syncVersion, + }); + + const committed = createMockSyncData({ + syncVersion: 5, + vectorClock: { original: 5 }, + recentOps: [compactOp('op-5', { original: 5 }, 5)], + }); + mockProvider.downloadFile.and.returnValue( + Promise.resolve({ dataStr: addPrefix(committed), rev: 'rev-5' }), + ); + await adapter.downloadOps(0); + await adapter.setLastServerSeq(5); + + const cancelledReset = createMockSyncData({ + syncVersion: 1, + vectorClock: { reset: 1 }, + recentOps: [], + state: { tasks: [{ id: 'remote-reset' }] }, + }); + mockProvider.downloadFile.and.returnValue( + Promise.resolve({ dataStr: addPrefix(cancelledReset), rev: 'rev-reset' }), + ); + expect((await adapter.downloadOps(5, 'local-client')).gapDetected).toBe(true); + await adapter.downloadOps(0, 'local-client'); + // No setLastServerSeq: the user cancelled applying the remote snapshot. + + const remoteAfterCancel = createMockSyncData({ + syncVersion: 2, + vectorClock: { reset: 2 }, + recentOps: [compactOp('op-after-reset', { reset: 2 }, 2)], + state: { tasks: [{ id: 'remote-reset' }] }, + }); + mockProvider.downloadFile.and.returnValue( + Promise.resolve({ dataStr: addPrefix(remoteAfterCancel), rev: 'rev-reset-2' }), + ); + + const result = await adapter.downloadOps(5, 'local-client'); + + expect(result.gapDetected).toBe(true); + }); }); it('should set seq counter to syncVersion after snapshot upload', async () => { @@ -2055,6 +2114,7 @@ describe('FileBasedSyncAdapterService', () => { Promise.resolve({ dataStr: addPrefix(first), rev: 'rev-1' }), ); await adapter.downloadOps(1); + await adapter.setLastServerSeq(5); // Second download: syncVersion regressed 5 -> 2 (would normally look like a // reset), but the causal vector clock is IDENTICAL, so nothing was actually @@ -2085,6 +2145,7 @@ describe('FileBasedSyncAdapterService', () => { Promise.resolve({ dataStr: addPrefix(first), rev: 'rev-1' }), ); await adapter.downloadOps(1); + await adapter.setLastServerSeq(5); const second = createMockSyncData({ syncVersion: 2, @@ -2108,6 +2169,7 @@ describe('FileBasedSyncAdapterService', () => { Promise.resolve({ dataStr: addPrefix(first), rev: 'rev-1' }), ); await adapter.downloadOps(1); + await adapter.setLastServerSeq(5); const second = createMockSyncData({ syncVersion: 2, @@ -3009,6 +3071,39 @@ describe('FileBasedSyncAdapterService', () => { expect(mockStateSnapshotService.getStateSnapshot).not.toHaveBeenCalled(); }); + it('(a1) identifies the split ops already represented by the snapshot', async () => { + const snapshotClock = { client1: 4 }; + const remoteLastModified = 1_720_000_000_000; + const opsFile = makeOpsFile({ + syncVersion: 5, + vectorClock: { client1: 5 }, + lastModified: remoteLastModified, + recentOps: [ + makeCompactOp({ id: 'op-in-snapshot', sv: 4, v: snapshotClock }), + makeCompactOp({ id: 'op-after-snapshot', sv: 5, v: { client1: 5 } }), + ], + snapshotRef: { syncVersion: 4, vectorClock: snapshotClock }, + }); + routeDownloads({ + [C.OPS_FILE]: addPrefix(opsFile, 3), + [C.STATE_FILE]: addPrefix( + makeStateFile({ syncVersion: 4, vectorClock: snapshotClock }), + 3, + ), + }); + + const result = await adapter.downloadOps(0, 'client2'); + + expect(result.snapshotState).toBeDefined(); + expect( + (result as { snapshotAppliedOpIds?: string[] }).snapshotAppliedOpIds, + ).toEqual(['op-in-snapshot']); + expect(result.snapshotVectorClock).toEqual(snapshotClock); + expect((result as { remoteLastModified?: number }).remoteLastModified).toBe( + remoteLastModified, + ); + }); + // (a2) Regression: the split ops buffer floor is SPLIT_COMPACTION_THRESHOLD // (1000), so it routinely exceeds DOWNLOAD_PAGE_SIZE (500). A behind client // must receive the NEWEST ops in a single page — the old code returned only @@ -3086,6 +3181,7 @@ describe('FileBasedSyncAdapterService', () => { ), }); await adapter.downloadOps(5, 'client2'); + await adapter.setLastServerSeq(5); // A strictly-ahead client snapshot-reset the folder (compacting ops this // client never saw into the snapshot) and then made one more edit. @@ -3111,6 +3207,68 @@ describe('FileBasedSyncAdapterService', () => { expect(result.snapshotState).toBeDefined(); }); + it('(a5) split download keeps reset metadata pending when snapshot application is cancelled', async () => { + routeDownloads({ + [C.OPS_FILE]: addPrefix( + makeOpsFile({ + syncVersion: 5, + vectorClock: { original: 5 }, + recentOps: [makeCompactOp({ id: 'op-5', sv: 5, v: { original: 5 } })], + }), + 3, + ), + }); + await adapter.downloadOps(0, 'local-client'); + await adapter.setLastServerSeq(5); + + const resetClock = { reset: 1 }; + routeDownloads({ + [C.OPS_FILE]: addPrefix( + makeOpsFile({ + syncVersion: 1, + vectorClock: resetClock, + recentOps: [], + snapshotRef: { syncVersion: 1, vectorClock: resetClock }, + }), + 3, + ), + [C.STATE_FILE]: addPrefix( + makeStateFile({ syncVersion: 1, vectorClock: resetClock }), + 3, + ), + }); + expect((await adapter.downloadOps(5, 'local-client')).gapDetected).toBe(true); + await adapter.downloadOps(0, 'local-client'); + // No setLastServerSeq: the user cancelled applying the remote snapshot. + + const advancedClock = { reset: 2 }; + routeDownloads({ + [C.OPS_FILE]: addPrefix( + makeOpsFile({ + syncVersion: 2, + vectorClock: advancedClock, + recentOps: [ + makeCompactOp({ + id: 'op-after-reset', + sv: 2, + v: advancedClock, + }), + ], + snapshotRef: { syncVersion: 1, vectorClock: resetClock }, + }), + 3, + ), + [C.STATE_FILE]: addPrefix( + makeStateFile({ syncVersion: 1, vectorClock: resetClock }), + 3, + ), + }); + + const result = await adapter.downloadOps(5, 'local-client'); + + expect(result.gapDetected).toBe(true); + }); + // (b) compaction triggers when the buffer exceeds MAX_RECENT_OPS, writing // state THEN ops. it('(b) compaction past MAX_RECENT_OPS writes sync-state.json BEFORE sync-ops.json', async () => { @@ -3235,21 +3393,29 @@ describe('FileBasedSyncAdapterService', () => { await adapter.uploadOps([createMockSyncOp()], 'client1'); const paths = uploadedPaths(); - // state written before ops before the tombstone. + // A conditional pending marker is acquired first. State is then written, + // followed by the legacy tombstone and the finalized ops commit point. const stateIdx = paths.indexOf(C.STATE_FILE); - const opsIdx = paths.indexOf(C.OPS_FILE); + const pendingOpsIdx = paths.indexOf(C.OPS_FILE); const tombIdx = paths.indexOf(C.SYNC_FILE); + const finalizedOpsIdx = paths.findIndex( + (path, index) => path === C.OPS_FILE && index > tombIdx, + ); expect(stateIdx).toBeGreaterThanOrEqual(0); - expect(opsIdx).toBeGreaterThanOrEqual(0); + expect(pendingOpsIdx).toBeGreaterThanOrEqual(0); expect(tombIdx).toBeGreaterThanOrEqual(0); - expect(stateIdx).toBeLessThan(opsIdx); - expect(opsIdx).toBeLessThan(tombIdx); + expect(finalizedOpsIdx).toBeGreaterThanOrEqual(0); + expect(pendingOpsIdx).toBeLessThan(stateIdx); + expect(stateIdx).toBeLessThan(tombIdx); + expect(tombIdx).toBeLessThan(finalizedOpsIdx); // sync-data.json overwritten with a v3 split tombstone (never removed). expect(mockProvider.removeFile).not.toHaveBeenCalled(); const tombCall = mockProvider.uploadFile.calls .allArgs() .find((a) => a[0] === C.SYNC_FILE); + expect(tombCall![2]).toBe(`${C.SYNC_FILE}-rev`); + expect(tombCall![3]).toBe(false); const tomb = parseWithPrefix(tombCall![1] as string) as unknown as { version: number; format: string; @@ -3273,6 +3439,179 @@ describe('FileBasedSyncAdapterService', () => { expect(bak.version).toBe(C.SPLIT_FILE_VERSION); }); + it('(e2) resumes a pending migration marker after restart before appending ops', async () => { + const legacy = createMockSyncData({ + syncVersion: 7, + vectorClock: { client1: 7 }, + recentOps: [makeCompactOp({ id: 'legacy-op', sv: 7 })], + state: { tasks: ['legacy'] }, + }); + const pendingOps = makeOpsFile({ + syncVersion: 7, + vectorClock: { client1: 7 }, + recentOps: legacy.recentOps, + snapshotRef: { syncVersion: 7, vectorClock: { client1: 7 } }, + migration: { + status: 'pending', + legacyRev: `${C.SYNC_FILE}-rev`, + }, + }); + routeDownloads({ + [C.SYNC_FILE]: addPrefix(legacy, 2), + [C.OPS_FILE]: addPrefix(pendingOps, 3), + [C.STATE_FILE]: addPrefix( + makeStateFile({ + syncVersion: 7, + vectorClock: { client1: 7 }, + state: { tasks: ['legacy'] }, + }), + 3, + ), + }); + + await adapter.uploadOps([createMockSyncOp()], 'client1'); + + const paths = uploadedPaths(); + expect(paths).toContain(C.SYNC_FILE); + const opsUploads = mockProvider.uploadFile.calls + .allArgs() + .filter((args) => args[0] === C.OPS_FILE); + expect(opsUploads.length).toBeGreaterThanOrEqual(2); + const finalized = parseWithPrefix( + opsUploads[opsUploads.length - 1][1] as string, + ) as unknown as FileBasedOpsFile; + expect(finalized.migration).toBeUndefined(); + expect(finalized.recentOps.some((op) => op.id === 'legacy-op')).toBe(true); + }); + + it('(e2b) resumes a pending migration during the next download cycle', async () => { + const clock = { client1: 7 }; + const legacy = createMockSyncData({ + syncVersion: 7, + vectorClock: clock, + state: { tasks: ['legacy'] }, + }); + const pendingOps = makeOpsFile({ + syncVersion: 7, + vectorClock: clock, + snapshotRef: { syncVersion: 7, vectorClock: clock }, + migration: { + status: 'pending', + legacyRev: `${C.SYNC_FILE}-rev`, + }, + }); + routeDownloads({ + [C.SYNC_FILE]: addPrefix(legacy, 2), + [C.OPS_FILE]: addPrefix(pendingOps, 3), + [C.STATE_FILE]: addPrefix( + makeStateFile({ + syncVersion: 7, + vectorClock: clock, + state: { tasks: ['legacy'] }, + }), + 3, + ), + }); + + const result = await adapter.downloadOps(0, 'client2'); + + expect(result.snapshotState).toBeDefined(); + expect(uploadedPaths()).toContain(C.SYNC_FILE); + const finalizedMarker = mockProvider.uploadFile.calls + .allArgs() + .filter((args) => args[0] === C.OPS_FILE) + .map((args) => parseWithPrefix(args[1] as string) as unknown as FileBasedOpsFile) + .find((opsFile) => opsFile.migration === undefined); + expect(finalizedMarker).toBeDefined(); + }); + + it('(e3) retries migration from a newer legacy revision instead of tombstoning over it', async () => { + const legacyV7 = createMockSyncData({ + syncVersion: 7, + vectorClock: { legacy: 7 }, + recentOps: [makeCompactOp({ id: 'legacy-v7', sv: 7, v: { legacy: 7 } })], + state: { tasks: ['v7'] }, + }); + const legacyV8 = createMockSyncData({ + syncVersion: 8, + vectorClock: { legacy: 8 }, + recentOps: [makeCompactOp({ id: 'legacy-v8', sv: 8, v: { legacy: 8 } })], + state: { tasks: ['v8'] }, + }); + let legacyData: unknown = legacyV7; + let legacyRev = 'legacy-rev-7'; + let stateData: unknown; + let stateRev = 'state-rev-0'; + let opsData: unknown; + let opsRev: string | undefined; + let revCounter = 0; + let firstTombstoneAttempt = true; + + mockProvider.downloadFile.and.callFake(async (path: string) => { + if (path === C.SYNC_FILE) { + return { dataStr: addPrefix(legacyData, 2), rev: legacyRev }; + } + if (path === C.OPS_FILE) { + if (!opsData || !opsRev) throw new RemoteFileNotFoundAPIError(path); + return { dataStr: addPrefix(opsData, 3), rev: opsRev }; + } + if (path === C.STATE_FILE && stateData) { + return { dataStr: addPrefix(stateData, 3), rev: stateRev }; + } + throw new RemoteFileNotFoundAPIError(path); + }); + mockProvider.uploadFile.and.callFake( + async ( + path: string, + dataStr: string, + revToMatch: string | null, + isForceOverwrite?: boolean, + ) => { + if (path === C.STATE_FILE) { + stateData = parseWithPrefix(dataStr); + stateRev = `state-rev-${++revCounter}`; + return { rev: stateRev }; + } + if (path === C.OPS_FILE) { + if ( + (!opsRev && revToMatch !== null) || + (opsRev && revToMatch !== opsRev && !isForceOverwrite) + ) { + throw new UploadRevToMatchMismatchAPIError(); + } + opsData = parseWithPrefix(dataStr); + opsRev = `ops-rev-${++revCounter}`; + return { rev: opsRev }; + } + if (path === C.SYNC_FILE) { + if (firstTombstoneAttempt) { + firstTombstoneAttempt = false; + // A legacy writer wins the race after the migrator's first read. + legacyData = legacyV8; + legacyRev = 'legacy-rev-8'; + if (!isForceOverwrite) { + throw new UploadRevToMatchMismatchAPIError(); + } + } + legacyData = parseWithPrefix(dataStr); + legacyRev = `legacy-rev-${++revCounter}`; + return { rev: legacyRev }; + } + return { rev: `${path}-rev-${++revCounter}` }; + }, + ); + + await adapter.uploadOps([createMockSyncOp()], 'client1'); + + const finalState = stateData as FileBasedStateFile; + const finalOps = opsData as FileBasedOpsFile; + expect(finalState.syncVersion).toBe(8); + expect((finalState.state as { tasks: string[] }).tasks).toEqual(['v8']); + expect(finalOps.migration).toBeUndefined(); + expect(finalOps.recentOps.some((op) => op.id === 'legacy-v8')).toBe(true); + expect(finalOps.recentOps.some((op) => op.id === 'op-123')).toBe(true); + }); + // (f) setting-OFF client seeing the tombstone raises the actionable notice // and does NOT upload/diverge. it('(f) OFF client hitting a split tombstone surfaces the enable-setting notice and does not upload', async () => { diff --git a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts index 66c4df7a67..8cd82e253f 100644 --- a/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts +++ b/src/app/op-log/sync-providers/file-based/file-based-sync-adapter.service.ts @@ -102,6 +102,9 @@ export class FileBasedSyncAdapterService { /** Expected sync version for optimistic locking, keyed by provider+user */ private _expectedSyncVersions = new Map(); + /** Downloaded version awaiting confirmation that its snapshot/ops were applied. */ + private _pendingExpectedSyncVersions = new Map(); + /** * SPAP-9: last-seen remote vector clock per provider+user. Used to tell a * benign (cosmetic) syncVersion reset apart from a genuine one: if the file's @@ -110,6 +113,9 @@ export class FileBasedSyncAdapterService { */ private _lastSeenVectorClocks = new Map(); + /** Downloaded causal clock awaiting the same durable-apply confirmation. */ + private _pendingVectorClocks = new Map(); + /** Local sequence counters (simulates server seq for file-based) */ private _localSeqCounters = new Map(); @@ -376,6 +382,16 @@ export class FileBasedSyncAdapterService { setLastServerSeq: async (seq: number): Promise => { this._localSeqCounters.set(providerKey, seq); + const pendingExpectedVersion = this._pendingExpectedSyncVersions.get(providerKey); + if (pendingExpectedVersion !== undefined) { + this._expectedSyncVersions.set(providerKey, pendingExpectedVersion); + this._pendingExpectedSyncVersions.delete(providerKey); + } + const pendingVectorClock = this._pendingVectorClocks.get(providerKey); + if (pendingVectorClock !== undefined) { + this._lastSeenVectorClocks.set(providerKey, pendingVectorClock); + this._pendingVectorClocks.delete(providerKey); + } // SPAP-10: the caller invokes this only after the downloaded ops are // durably applied, so it's the correct point to promote the rev staged in // _downloadOps to last-seen and persist it — same ordering as the seq @@ -1020,11 +1036,11 @@ export class FileBasedSyncAdapterService { ); } - // Update expected version for next upload - this._expectedSyncVersions.set(providerKey, syncData.syncVersion); - // SPAP-9: remember this file's causal clock so the next download can decide - // whether a syncVersion regression is cosmetic or a genuine reset. - this._lastSeenVectorClocks.set(providerKey, syncData.vectorClock); + // Stage the remote baseline until the caller confirms that the downloaded + // snapshot/ops were durably applied. A cancelled conflict dialog must leave + // the committed baseline intact so a later reset still triggers a gap. + this._pendingExpectedSyncVersions.set(providerKey, syncData.syncVersion); + this._pendingVectorClocks.set(providerKey, syncData.vectorClock); this._stageOrDropDownloadedRev(providerKey, rev, recoveredFromBackup); // Filter ops using operation IDs instead of synthetic seq numbers. @@ -1109,6 +1125,7 @@ export class FileBasedSyncAdapterService { hasMore, latestSeq, snapshotVectorClock: syncData.vectorClock, + remoteLastModified: syncData.lastModified, // Signal gap detection when sync version was reset or snapshot was replaced. // This triggers the download service to re-download from seq 0, which will include // the snapshotState for proper state replacement. @@ -1116,6 +1133,9 @@ export class FileBasedSyncAdapterService { // Include full state snapshot for fresh downloads (sinceSeq === 0) // This allows new clients to bootstrap with complete state, not just recent ops ...(snapshotStateWithArchives ? { snapshotState: snapshotStateWithArchives } : {}), + ...(snapshotStateWithArchives + ? { snapshotAppliedOpIds: limitedOps.map(({ op }) => op.id) } + : {}), }; } @@ -1267,6 +1287,8 @@ export class FileBasedSyncAdapterService { this._lastSeenRevs.delete(providerKey); this._pendingRevs.delete(providerKey); this._lastSeenVectorClocks.delete(providerKey); + this._pendingExpectedSyncVersions.delete(providerKey); + this._pendingVectorClocks.delete(providerKey); this._clearCachedSyncData(providerKey); this._clearCachedOpsData(providerKey); this._persistState(); @@ -1302,6 +1324,10 @@ export class FileBasedSyncAdapterService { ); } + private _isPendingSplitMigration(data: FileBasedOpsFile): boolean { + return data.migration?.status === 'pending' && !!data.migration.legacyRev; + } + /** Surfaces the actionable "turn on Surgical sync" notice (non-fatal if no snack). */ private _notifySplitFormatDetected(): void { this._injector @@ -1526,6 +1552,7 @@ export class FileBasedSyncAdapterService { provider: FileSyncProvider, cfg: EncryptAndCompressCfg, encryptKey: string | undefined, + expectedLegacyRev?: string, ): Promise { const tombstone: FileBasedSplitTombstone = { version: FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, @@ -1550,64 +1577,30 @@ export class FileBasedSyncAdapterService { // clients are still caught by the ops-file probe in _downloadSyncFile) than // a tombstone with a resurrectable v2 .bak beside it. // - // Retry caveat: at the _uploadSnapshotSplit call site a throw fails the - // whole snapshot upload, which the user/caller retries. At the MIGRATION - // call site the split files are already committed before this runs, and - // migration is only re-entered while sync-ops.json is missing — so a - // failure here leaves a live v2 sync-data.json until the next snapshot - // upload rewrites the tombstone. Accepted residual (needs a transient PUT - // failure right after two successful PUTs); a periodic tombstone re-assert - // during split compaction would close it if it ever shows up in practice. + // Migration passes the exact downloaded legacy rev, making the primary + // tombstone conditional. A mismatch leaves the pending ops marker intact; + // the recovery loop imports the newer v2 payload and retries. Explicit + // snapshot replacement omits the rev and intentionally force-overwrites. await provider.uploadFile(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE, encoded, null, true); // Overwrite the legacy single file in place (never remove it). - await provider.uploadFile(FILE_BASED_SYNC_CONSTANTS.SYNC_FILE, encoded, null, true); + await provider.uploadFile( + FILE_BASED_SYNC_CONSTANTS.SYNC_FILE, + encoded, + expectedLegacyRev ?? null, + expectedLegacyRev === undefined, + ); } - /** - * One-way migration of an existing single-file v2 `sync-data.json` to the split - * format. Writes `sync-state.json` FIRST, then `sync-ops.json` (the commit - * point), then the v3 tombstone over `sync-data.json` and neutralizes `.bak`. - * Returns the freshly-written ops file (+ rev), or null when there is no legacy - * file to migrate (truly fresh folder). - */ - private async _maybeMigrateLegacyToSplit( + private async _writePendingSplitMigration( provider: FileSyncProvider, cfg: EncryptAndCompressCfg, encryptKey: string | undefined, + legacy: FileBasedSyncData, + legacyRev: string, clientId: string, - ): Promise<{ data: FileBasedOpsFile; rev: string } | null> { - let legacy: FileBasedSyncData; - try { - const r = await this._downloadSyncFile(provider, cfg, encryptKey); - legacy = r.data; - } catch (e) { - if (e instanceof RemoteFileNotFoundAPIError) return null; // truly fresh - // Already a tombstone but the ops file is missing (migration crashed before - // the ops write, or ops file was deleted): treat as fresh split — there is - // no v2 payload left to preserve. - if (e instanceof SplitSyncFormatDetectedError) return null; - throw e; // legacy pfapi (__meta_) etc. must propagate - } - - OpLog.warn( - 'FileBasedSyncAdapter: migrating legacy single-file sync-data.json to split format', - ); + opsRevToMatch: string | null, + ): Promise<{ data: FileBasedOpsFile; rev: string }> { const schemaVersion = legacy.schemaVersion ?? 1; - - const stateData: FileBasedStateFile = { - version: FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, - syncVersion: legacy.syncVersion, - schemaVersion, - vectorClock: legacy.vectorClock, - lastModified: Date.now(), - clientId, - state: legacy.state, - archiveYoung: legacy.archiveYoung, - archiveOld: legacy.archiveOld, - }; - // 1) state file FIRST - const stateRev = await this._writeStateFile(provider, cfg, encryptKey, stateData); - const opsData: FileBasedOpsFile = { version: FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, syncVersion: legacy.syncVersion, @@ -1620,30 +1613,241 @@ export class FileBasedSyncAdapterService { snapshotRef: { syncVersion: legacy.syncVersion, vectorClock: legacy.vectorClock, - rev: stateRev, }, + migration: { status: 'pending', legacyRev }, }; - // 2) ops file (commit point) — force-write to establish the new format - const opsEncoded = await this._encryptAndCompressHandler.compressAndEncryptData( + const encoded = await this._encryptAndCompressHandler.compressAndEncryptData( cfg, encryptKey, opsData, FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, ); this._assertUploadDataNotEmpty( - opsEncoded, - 'FileBasedSyncAdapter._maybeMigrateLegacyToSplit', + encoded, + 'FileBasedSyncAdapter._writePendingSplitMigration', ); - const opsRes = await provider.uploadFile( + const result = await provider.uploadFile( FILE_BASED_SYNC_CONSTANTS.OPS_FILE, - opsEncoded, - null, - true, + encoded, + opsRevToMatch, + false, ); - // 3) tombstone over sync-data.json + neutralize .bak - await this._writeTombstoneAndNeutralizeBak(provider, cfg, encryptKey); + return { data: opsData, rev: result.rev }; + } - return { data: opsData, rev: opsRes.rev }; + private _buildSplitMigrationState( + legacy: FileBasedSyncData, + clientId: string, + ): FileBasedStateFile { + return { + version: FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, + syncVersion: legacy.syncVersion, + schemaVersion: legacy.schemaVersion ?? 1, + vectorClock: legacy.vectorClock, + lastModified: Date.now(), + clientId, + state: legacy.state, + archiveYoung: legacy.archiveYoung, + archiveOld: legacy.archiveOld, + }; + } + + private async _finalizeSplitMigrationMarker( + provider: FileSyncProvider, + cfg: EncryptAndCompressCfg, + encryptKey: string | undefined, + pending: FileBasedOpsFile, + pendingRev: string, + ): Promise<{ data: FileBasedOpsFile; rev: string }> { + const finalized: FileBasedOpsFile = { ...pending }; + delete finalized.migration; + const encoded = await this._encryptAndCompressHandler.compressAndEncryptData( + cfg, + encryptKey, + finalized, + FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION, + ); + this._assertUploadDataNotEmpty( + encoded, + 'FileBasedSyncAdapter._finalizeSplitMigrationMarker', + ); + const result = await provider.uploadFile( + FILE_BASED_SYNC_CONSTANTS.OPS_FILE, + encoded, + pendingRev, + false, + ); + return { data: finalized, rev: result.rev }; + } + + /** + * Completes or repairs a migration whose ops commit point is still marked + * pending. The marker survives crashes and contains the full candidate ops + * payload. A conditional tombstone either wins against the exact legacy rev + * used to build it or fails and causes the newer v2 payload to be re-imported. + */ + private async _resumePendingSplitMigration( + provider: FileSyncProvider, + cfg: EncryptAndCompressCfg, + encryptKey: string | undefined, + initialPending: FileBasedOpsFile, + initialPendingRev: string, + ): Promise<{ data: FileBasedOpsFile; rev: string }> { + let current = { data: initialPending, rev: initialPendingRev }; + const maxAttempts = 1 + this._MAX_UPLOAD_RETRIES; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + if (!this._isPendingSplitMigration(current.data)) return current; + + let legacy: { data: FileBasedSyncData; rev: string } | undefined; + try { + legacy = await this._downloadSyncFile(provider, cfg, encryptKey); + } catch (e) { + if (e instanceof SplitSyncFormatDetectedError) { + try { + return await this._finalizeSplitMigrationMarker( + provider, + cfg, + encryptKey, + current.data, + current.rev, + ); + } catch (finalizeError) { + if (!(finalizeError instanceof UploadRevToMatchMismatchAPIError)) { + throw finalizeError; + } + current = await this._downloadOpsFile(provider, cfg, encryptKey); + continue; + } + } + throw e; + } + + if (legacy.rev !== current.data.migration?.legacyRev) { + try { + current = await this._writePendingSplitMigration( + provider, + cfg, + encryptKey, + legacy.data, + legacy.rev, + current.data.clientId, + current.rev, + ); + } catch (refreshError) { + if (!(refreshError instanceof UploadRevToMatchMismatchAPIError)) { + throw refreshError; + } + current = await this._downloadOpsFile(provider, cfg, encryptKey); + } + continue; + } + + // The pending marker is the write permission. Only after winning its + // conditional create/update may a migrator touch sync-state.json; this + // prevents a stale losing migrator from overwriting a newer state file. + // Rewriting here also repairs a crash after the marker write but before + // the state write. The final marker records the resulting snapshot rev. + const stateRev = await this._writeStateFile( + provider, + cfg, + encryptKey, + this._buildSplitMigrationState(legacy.data, current.data.clientId), + ); + current = { + data: { + ...current.data, + snapshotRef: { ...current.data.snapshotRef, rev: stateRev }, + }, + rev: current.rev, + }; + + try { + await this._writeTombstoneAndNeutralizeBak(provider, cfg, encryptKey, legacy.rev); + } catch (tombstoneError) { + if (!(tombstoneError instanceof UploadRevToMatchMismatchAPIError)) { + throw tombstoneError; + } + // A legacy writer won the conditional race. Re-read and rebuild the + // pending split files from that newer v2 revision on the next attempt. + continue; + } + + try { + return await this._finalizeSplitMigrationMarker( + provider, + cfg, + encryptKey, + current.data, + current.rev, + ); + } catch (finalizeError) { + if (!(finalizeError instanceof UploadRevToMatchMismatchAPIError)) { + throw finalizeError; + } + current = await this._downloadOpsFile(provider, cfg, encryptKey); + } + } + + throw new UploadRevToMatchMismatchAPIError( + 'FileBasedSyncAdapter: split migration did not stabilize after repeated concurrent writes.', + ); + } + + /** + * One-way migration of an existing single-file v2 `sync-data.json` to the split + * format. Conditionally creates a pending `sync-ops.json` marker before + * writing `sync-state.json`, so losing migrators cannot overwrite state. Only + * a matching legacy rev can be tombstoned; the marker is finalized afterward + * and can resume either side of a crash. Returns the finalized ops file (+ + * rev), or null for a truly fresh folder. + */ + private async _maybeMigrateLegacyToSplit( + provider: FileSyncProvider, + cfg: EncryptAndCompressCfg, + encryptKey: string | undefined, + clientId: string, + ): Promise<{ data: FileBasedOpsFile; rev: string } | null> { + let legacy: { data: FileBasedSyncData; rev: string }; + try { + legacy = await this._downloadSyncFile(provider, cfg, encryptKey); + } catch (e) { + if (e instanceof RemoteFileNotFoundAPIError) return null; // truly fresh + // Already a tombstone but the ops file is missing (migration crashed before + // the ops write, or ops file was deleted): treat as fresh split — there is + // no v2 payload left to preserve. + if (e instanceof SplitSyncFormatDetectedError) return null; + throw e; // legacy pfapi (__meta_) etc. must propagate + } + + OpLog.warn( + 'FileBasedSyncAdapter: migrating legacy single-file sync-data.json to split format', + ); + let pending: { data: FileBasedOpsFile; rev: string }; + try { + pending = await this._writePendingSplitMigration( + provider, + cfg, + encryptKey, + legacy.data, + legacy.rev, + clientId, + null, + ); + } catch (e) { + if (!(e instanceof UploadRevToMatchMismatchAPIError)) throw e; + // Another split client created the marker first. Join its recoverable + // migration instead of overwriting the commit point. + pending = await this._downloadOpsFile(provider, cfg, encryptKey); + } + + return this._resumePendingSplitMigration( + provider, + cfg, + encryptKey, + pending.data, + pending.rev, + ); } /** @@ -1751,6 +1955,18 @@ export class FileBasedSyncAdapterService { } } + if (opsFile && opsRev && this._isPendingSplitMigration(opsFile)) { + const resumed = await this._resumePendingSplitMigration( + provider, + cfg, + encryptKey, + opsFile, + opsRev, + ); + opsFile = resumed.data; + opsRev = resumed.rev; + } + if (ops.length === 0 && opsFile) { return { results: [], latestSeq: this._localSeqCounters.get(providerKey) || 0 }; } @@ -1962,6 +2178,19 @@ export class FileBasedSyncAdapterService { } } + if (this._isPendingSplitMigration(opsFile)) { + const resumed = await this._resumePendingSplitMigration( + provider, + cfg, + encryptKey, + opsFile, + opsRev, + ); + opsFile = resumed.data; + opsRev = resumed.rev; + this._setCachedOpsData(providerKey, opsFile, opsRev); + } + // Gap detection on the ops file (mirrors the single-file logic). const previousExpectedVersion = this._expectedSyncVersions.get(providerKey) ?? 0; const syncVersionRegressed = @@ -1996,20 +2225,32 @@ export class FileBasedSyncAdapterService { opsFile.oldestOpSyncVersion > sinceSeq + 1; let needsGapDetection = versionWasReset || snapshotReplacement || partialTrimGap; - this._expectedSyncVersions.set(providerKey, opsFile.syncVersion); - this._lastSeenVectorClocks.set(providerKey, opsFile.vectorClock); + this._pendingExpectedSyncVersions.set(providerKey, opsFile.syncVersion); + this._pendingVectorClocks.set(providerKey, opsFile.vectorClock); this._stageOrDropDownloadedRev(providerKey, opsRev, recoveredFromBackup); - this._persistState(); const isForceFromZero = sinceSeq === 0; const filteredOps: ServerSyncOperation[] = []; + const snapshotAppliedOpIds: string[] = []; opsFile.recentOps.forEach((compactOp, index) => { if (excludeClient && compactOp.c === excludeClient) return; + const op = this._compactToSyncOp(compactOp); filteredOps.push({ serverSeq: index + 1, - op: this._compactToSyncOp(compactOp), + op, receivedAt: compactOp.t, }); + // An op belongs to the snapshot (record-as-applied, do not replay) iff it + // is at or below the snapshot's syncVersion. `sv === undefined` marks a + // legacy op carried over by the split migration: those predate per-op + // versioning and are always fully contained in the migration snapshot, so + // they are snapshot-included too. This is safe because _validateSnapshotRef + // requires the loaded snapshot's clock to be EQUAL to snapshotRef, so the + // boundary always matches what was hydrated; any op newer than the snapshot + // carries an sv > snapshotRef.syncVersion and is reprocessed. + if (compactOp.sv === undefined || compactOp.sv <= opsFile.snapshotRef.syncVersion) { + snapshotAppliedOpIds.push(op.id); + } }); // Whole bounded ops buffer in one page (hasMore=false) — see the single-file // _downloadOps note. File-based providers have no server cursor, so there is no @@ -2022,9 +2263,11 @@ export class FileBasedSyncAdapterService { const latestSeq = opsFile.syncVersion; let snapshotStateWithArchives: Record | undefined; + let snapshot: FileBasedStateFile | undefined; if (isForceFromZero || needsGapDetection) { const snap = await this._loadValidatedSnapshot(provider, cfg, encryptKey, opsFile); if (snap) { + snapshot = snap; snapshotStateWithArchives = { ...(snap.state as Record), ...(snap.archiveYoung ? { archiveYoung: snap.archiveYoung } : {}), @@ -2040,9 +2283,11 @@ export class FileBasedSyncAdapterService { ops: limitedOps, hasMore, latestSeq, - snapshotVectorClock: opsFile.vectorClock, + snapshotVectorClock: snapshot?.vectorClock ?? opsFile.vectorClock, + remoteLastModified: opsFile.lastModified, gapDetected: needsGapDetection, ...(snapshotStateWithArchives ? { snapshotState: snapshotStateWithArchives } : {}), + ...(snapshotStateWithArchives ? { snapshotAppliedOpIds } : {}), }; } @@ -2080,10 +2325,14 @@ export class FileBasedSyncAdapterService { hasMore: false, latestSeq: data.syncVersion, snapshotVectorClock: data.vectorClock, + remoteLastModified: data.lastModified, gapDetected: sinceSeq > 0, ...(snapshotStateWithArchives ? { snapshotState: snapshotStateWithArchives } : {}), + ...(snapshotStateWithArchives + ? { snapshotAppliedOpIds: filteredOps.map(({ op }) => op.id) } + : {}), }; } catch (e) { // Fresh folder, or a tombstone with no ops file yet — nothing to apply. @@ -2303,6 +2552,11 @@ export class FileBasedSyncAdapterService { private _commitLastSeenRev(providerKey: string, rev: string): void { this._lastSeenRevs.set(providerKey, rev); this._pendingRevs.delete(providerKey); + // A successful write establishes a newer authoritative remote baseline. + // Discard metadata staged by any preceding download in this cycle so a later + // cursor update cannot promote stale pre-write values over it. + this._pendingExpectedSyncVersions.delete(providerKey); + this._pendingVectorClocks.delete(providerKey); } /** diff --git a/src/app/op-log/sync/operation-log-download.service.spec.ts b/src/app/op-log/sync/operation-log-download.service.spec.ts index d33c80c4be..9ae3da3ff3 100644 --- a/src/app/op-log/sync/operation-log-download.service.spec.ts +++ b/src/app/op-log/sync/operation-log-download.service.spec.ts @@ -1129,6 +1129,49 @@ describe('OperationLogDownloadService', () => { expect(result.snapshotVectorClock).toEqual(snapshotClock); }); + it('should propagate the snapshot operation boundary for file providers', async () => { + const remoteLastModified = 1_720_000_000_000; + mockApiProvider.providerMode = 'fileSnapshotOps'; + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 5, + snapshotState: { tasks: [] }, + snapshotAppliedOpIds: ['op-in-snapshot'], + remoteLastModified, + }), + ); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.providerMode).toBe('fileSnapshotOps'); + if (result.success && result.providerMode === 'fileSnapshotOps') { + expect(result.snapshotAppliedOpIds).toEqual(['op-in-snapshot']); + expect(result.remoteLastModified).toBe(remoteLastModified); + } + }); + + it('should omit a malformed remote last-modified timestamp', async () => { + mockApiProvider.providerMode = 'fileSnapshotOps'; + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 5, + snapshotState: { tasks: [] }, + remoteLastModified: 'not-a-timestamp' as unknown as number, + }), + ); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.providerMode).toBe('fileSnapshotOps'); + if (result.success && result.providerMode === 'fileSnapshotOps') { + expect(result.remoteLastModified).toBeUndefined(); + } + }); + it('should return snapshotVectorClock even when no new ops', async () => { const snapshotClock = { clientA: 10, clientB: 5 }; mockApiProvider.downloadOps.and.returnValue( diff --git a/src/app/op-log/sync/operation-log-download.service.ts b/src/app/op-log/sync/operation-log-download.service.ts index 9b8b29e59b..64d9185a65 100644 --- a/src/app/op-log/sync/operation-log-download.service.ts +++ b/src/app/op-log/sync/operation-log-download.service.ts @@ -112,6 +112,8 @@ export class OperationLogDownloadService implements OnDestroy { let finalLatestSeq = 0; let snapshotVectorClock: import('../core/operation.types').VectorClock | undefined; let snapshotState: unknown | undefined; + let snapshotAppliedOpIds: string[] | undefined; + let remoteLastModified: number | undefined; // Track encryption state of downloaded operations for detecting encryption config mismatch. // When another client disables encryption, all downloaded ops will be unencrypted. // We track this BEFORE decryption to detect the server's actual encryption state. @@ -203,6 +205,18 @@ export class OperationLogDownloadService implements OnDestroy { response.snapshotState ) { snapshotState = response.snapshotState; + snapshotAppliedOpIds = + 'snapshotAppliedOpIds' in response + ? response.snapshotAppliedOpIds + : undefined; + const receivedLastModified = + 'remoteLastModified' in response ? response.remoteLastModified : undefined; + remoteLastModified = + typeof receivedLastModified === 'number' && + Number.isFinite(receivedLastModified) && + receivedLastModified >= 0 + ? receivedLastModified + : undefined; OpLog.normal( 'OperationLogDownloadService: Received snapshotState for fresh download bootstrap', ); @@ -225,6 +239,8 @@ export class OperationLogDownloadService implements OnDestroy { allOpClocks.length = 0; // Clear clocks too snapshotVectorClock = undefined; // Clear snapshot clock to capture fresh one after reset snapshotState = undefined; // Clear snapshot state to capture fresh one after reset + snapshotAppliedOpIds = undefined; // Clear snapshot boundary with the stale state + remoteLastModified = undefined; // Clear timestamp belonging to the stale state sawAnyOps = false; // Reset encryption tracking sawEncryptedOp = false; @@ -500,6 +516,8 @@ export class OperationLogDownloadService implements OnDestroy { providerMode: 'fileSnapshotOps', // Include snapshot state for file-based sync fresh downloads ...(snapshotState ? { snapshotState } : {}), + ...(snapshotAppliedOpIds ? { snapshotAppliedOpIds } : {}), + ...(remoteLastModified !== undefined ? { remoteLastModified } : {}), }; } diff --git a/src/app/op-log/sync/operation-log-sync.service.spec.ts b/src/app/op-log/sync/operation-log-sync.service.spec.ts index 606dade8da..3a737277c1 100644 --- a/src/app/op-log/sync/operation-log-sync.service.spec.ts +++ b/src/app/op-log/sync/operation-log-sync.service.spec.ts @@ -2039,6 +2039,80 @@ describe('OperationLogSyncService', () => { ]); }); + it('applies split-file operations newer than the downloaded snapshot before advancing the cursor', async () => { + const snapshotIncludedOp: Operation = { + id: 'snapshot-included-op', + clientId: 'client-B', + actionType: 'test' as ActionType, + opType: OpType.Update, + entityType: 'TASK', + entityId: 'task-in-snapshot', + payload: { title: 'Already in snapshot' }, + vectorClock: { clientB: 4 }, + timestamp: 4, + schemaVersion: 1, + }; + const postSnapshotOp: Operation = { + id: 'post-snapshot-op', + clientId: 'client-B', + actionType: 'test' as ActionType, + opType: OpType.Create, + entityType: 'TASK', + entityId: 'task-after-snapshot', + payload: { title: 'Created after snapshot' }, + vectorClock: { clientB: 5 }, + timestamp: 5, + schemaVersion: 1, + }; + const syncHydrationServiceSpy = TestBed.inject( + SyncHydrationService, + ) as jasmine.SpyObj; + syncHydrationServiceSpy.hydrateFromRemoteSync.and.resolveTo(); + downloadServiceSpy.downloadRemoteOps.and.resolveTo({ + newOps: [snapshotIncludedOp, postSnapshotOp], + needsFullStateUpload: false, + success: true, + providerMode: 'fileSnapshotOps', + failedFileCount: 0, + snapshotState: { task: { ids: ['task-in-snapshot'] } }, + snapshotVectorClock: { clientB: 4 }, + snapshotAppliedOpIds: [snapshotIncludedOp.id], + latestServerSeq: 5, + } as unknown as DownloadResult); + const callOrder: string[] = []; + remoteOpsProcessingServiceSpy.processRemoteOps.and.callFake(async () => { + callOrder.push('processRemoteOps'); + return { + localWinOpsCreated: 0, + allOpsFilteredBySyncImport: false, + filteredOpCount: 0, + isLocalUnsyncedImport: false, + blockedByIncompatibleOp: false, + }; + }); + const setLastServerSeqSpy = jasmine + .createSpy('setLastServerSeq') + .and.callFake(async () => { + callOrder.push('setLastServerSeq'); + }); + const mockProvider = { + supportsOperationSync: true, + setLastServerSeq: setLastServerSeqSpy, + } as unknown as OperationSyncCapable; + + await service.downloadRemoteOps(mockProvider); + + expect(opLogStoreSpy.appendBatchSkipDuplicates).toHaveBeenCalledWith( + [snapshotIncludedOp], + 'remote', + ); + expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledOnceWith( + [postSnapshotOp], + {}, + ); + expect(callOrder).toEqual(['processRemoteOps', 'setLastServerSeq']); + }); + it('does NOT throw LocalDataConflictError when store + pending ops contain only example tasks (#7985)', async () => { opLogStoreSpy.getUnsynced.and.returnValue( Promise.resolve([exampleCreateEntry('ex-task-1')]), @@ -2188,6 +2262,7 @@ describe('OperationLogSyncService', () => { const remoteSnapshot = { tasks: [{ id: 'remote-task' }] }; const remoteVectorClock = { clientB: 5, clientC: 3 }; + const remoteLastModified = 1_720_000_000_000; downloadServiceSpy.downloadRemoteOps.and.returnValue( Promise.resolve({ @@ -2200,6 +2275,7 @@ describe('OperationLogSyncService', () => { failedFileCount: 0, snapshotState: remoteSnapshot, snapshotVectorClock: remoteVectorClock, + remoteLastModified, }), ); @@ -2217,6 +2293,7 @@ describe('OperationLogSyncService', () => { expect(conflictError.unsyncedCount).toBe(2); expect(conflictError.remoteSnapshotState).toEqual(remoteSnapshot); expect(conflictError.remoteVectorClock).toEqual(remoteVectorClock); + expect(conflictError.remoteLastModified).toBe(remoteLastModified); } }); diff --git a/src/app/op-log/sync/operation-log-sync.service.ts b/src/app/op-log/sync/operation-log-sync.service.ts index 60990eefc4..b43b982610 100644 --- a/src/app/op-log/sync/operation-log-sync.service.ts +++ b/src/app/op-log/sync/operation-log-sync.service.ts @@ -747,6 +747,7 @@ export class OperationLogSyncService { result.snapshotState as Record, result.snapshotVectorClock, lastSyncedVectorClock, + result.remoteLastModified, ); } @@ -764,6 +765,7 @@ export class OperationLogSyncService { result.snapshotState as Record, result.snapshotVectorClock, lastSyncedVectorClock, + result.remoteLastModified, ); } @@ -806,6 +808,7 @@ export class OperationLogSyncService { result.snapshotState as Record, result.snapshotVectorClock, null, + result.remoteLastModified, ); } @@ -848,14 +851,26 @@ export class OperationLogSyncService { // so the next attempt can retry. await this._discardStartupOps(startupOpIdsToDiscard); - // CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration. - // File-based providers return ALL recentOps on every download, relying on - // getAppliedOpIds() (from IndexedDB) to filter already-applied ops. - // Without writing these ops, they bypass the filter on the next sync cycle - // and get applied again, duplicating entities. - if (result.newOps.length > 0) { + // Single-file snapshots are current through every returned recent op. Split + // snapshots can lag behind sync-ops.json, so only the explicitly-listed + // snapshot ops may be recorded as already applied; the remaining suffix + // must run through normal remote-op processing before the cursor advances. + // An absent list keeps the legacy single-file contract (all ops included). + const snapshotAppliedOpIds = result.snapshotAppliedOpIds + ? new Set(result.snapshotAppliedOpIds) + : null; + const snapshotIncludedOps = snapshotAppliedOpIds + ? result.newOps.filter((op) => snapshotAppliedOpIds.has(op.id)) + : result.newOps; + const postSnapshotOps = snapshotAppliedOpIds + ? result.newOps.filter((op) => !snapshotAppliedOpIds.has(op.id)) + : []; + + // Record operations already represented by the snapshot so future whole- + // file downloads deduplicate them without replaying their effects twice. + if (snapshotIncludedOps.length > 0) { const appendResult = await this.opLogStore.appendBatchSkipDuplicates( - result.newOps, + snapshotIncludedOps, 'remote', ); OpLog.normal( @@ -867,6 +882,17 @@ export class OperationLogSyncService { ); } + if (postSnapshotOps.length > 0) { + const processResult = await this._processRemoteOpsWithStartupCleanup( + postSnapshotOps, + undefined, + [], + ); + if (processResult.blockedByIncompatibleOp) { + return { kind: 'blocked_incompatible' }; + } + } + // Persist lastServerSeq after hydration if (result.latestServerSeq !== undefined) { await syncProvider.setLastServerSeq(result.latestServerSeq); diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 8037f7fcc3..3324dff136 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -1293,8 +1293,8 @@ "LOCAL": "Local", "LOCAL_REMOTE": "Local -> Remote", "NEVER": "Never", - "OVERWRITE_WARNING": "WARNING: The {{targetName}} data contains approximately {{targetChanges}} changes while the {{sourceName}} data only has {{sourceChanges}} changes. Are you sure you want to overwrite the {{targetName}} data with the {{sourceName}} version? This action cannot be undone.", - "OVERWRITE_WARNING_UNKNOWN": "WARNING: This device cannot determine how many unsynced changes the {{targetName}} data has (it has no record of a previous sync). Overwriting the {{targetName}} data with the {{sourceName}} version may discard changes. Are you sure? This action cannot be undone.", + "OVERWRITE_WARNING": "WARNING: The {{targetName}} data contains approximately {{targetChanges}} changes while the {{sourceName}} data only has {{sourceChanges}} changes. Are you sure you want to overwrite the {{targetName}} data with the {{sourceName}} version?", + "OVERWRITE_WARNING_UNKNOWN": "WARNING: This device cannot determine how many unsynced changes the {{targetName}} data has (it has no record of a previous sync). Overwriting the {{targetName}} data with the {{sourceName}} version may discard changes. Are you sure?", "REMOTE": "Remote", "RESULT": "Result", "TEXT": "

Update from remote. Both local and remote data seem to be modified.

",