fix(sync): file-based provider atomicity & conflict UX (#8960) (#9004)

* docs(plugins): add microsoft 365 calendar provider plan

* docs(plugins): add microsoft 365 calendar provider plan

* docs(ios): plan internal testflight builds

* fix(sync): prevent file-based sync data loss

Preserve post-snapshot operations and stage remote baselines until durable apply.

Use conditional writes and a resumable marker for provider concurrency and legacy migration. Report trustworthy remote timestamps and document transport limits.

Refs #8960

* fix(sync): avoid biased conflict recommendations

Highlight a side only when its timestamp or known change count is strictly greater, leaving unknown and tied metadata neutral.

* test(sync): assert options arg on split-file processRemoteOps

The split-file snapshot path now routes post-snapshot ops through
_processRemoteOpsWithStartupCleanup, which calls processRemoteOps with an
(empty) options object. Update the assertion to match the 2-arg call so the
unit suite passes.

* refactor(sync): dedupe strong-ETag regex, document snapshot-op boundary

- Extract the duplicated RFC 7232 strong-entity-tag pattern into a single
  STRONG_ETAG_RE constant with a comment noting why the char class is safe to
  interpolate into an If-Match header (no CR/LF header splitting).
- Explain why sv === undefined ops are classified as snapshot-included: they are
  legacy migration ops fully contained in the snapshot, and _validateSnapshotRef
  enforces a clock-EQUAL boundary. No behavior change.
This commit is contained in:
Johannes Millan 2026-07-14 19:23:24 +02:00 committed by GitHub
parent 91a1f0eda9
commit 2864a39c85
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 2072 additions and 128 deletions

View file

@ -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 1015 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)

View file

@ -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 | 12 engineering days | Medium; tenant policy is external |
| Production Electron MVP, including host hardening and tests | 1726 additional focused engineering days | Medium |
| Total engineering effort | About 46 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:<fixed-high-port>/<fixed-callback-path>` 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: 23 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.51 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: 11.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: 11.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.51 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: 12 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: 11.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: 23 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: 11.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.51 day.
### 17. End-to-end verification and pilot
**Depends on:** all implementation tasks.
Run:
- `npm run checkFile <filepath>` 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.52 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)

View file

@ -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/<username>/`; 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

View file

@ -116,6 +116,16 @@ export interface FileBasedOpsFile<
recentOps: SyncFileCompactOp<TCompactOperation>[];
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;
};
}
/**

View file

@ -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;
}
}

View file

@ -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, string>): 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<string, string> = {
[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<void> {

View file

@ -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;

View file

@ -54,6 +54,14 @@ export interface FileSyncProvider<
getFileRev(targetPath: string, localRev: string | null): Promise<FileRevResponse>;
downloadFile(targetPath: string): Promise<FileDownloadResponse>;
/**
* 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 =

View file

@ -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();

View file

@ -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<string, string> = {},
): 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 () => {

View file

@ -16,10 +16,18 @@
<tr [class.isHighlight]="isHighlightRemote">
<td>{{ T.F.SYNC.D_CONFLICT.REMOTE | translate }}</td>
<td [attr.data-label]="T.F.SYNC.D_CONFLICT.DATE | 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 }}
}
</td>
<td [attr.data-label]="T.F.SYNC.D_CONFLICT.TIME | translate">
{{ remote.lastUpdate | shortTime }}
{{
remote.lastUpdate !== null
? (remote.lastUpdate | shortTime)
: (T.F.SYNC.D_CONFLICT.CHANGES_UNKNOWN | translate)
}}
</td>
<td [attr.data-label]="T.F.SYNC.D_CONFLICT.CHANGES | translate">
<span [class.isHighlight]="isHighlightRemoteChanges">{{

View file

@ -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)', () => {

View file

@ -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;

View file

@ -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,

View file

@ -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,

View file

@ -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<string, number> | null,
/** Actual `lastModified` recorded by the downloaded remote file. */
public readonly remoteLastModified?: number,
) {
super(`Local data conflict: ${unsyncedCount} unsynced changes would be lost`);
}

View file

@ -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 =

View file

@ -139,7 +139,9 @@ interface MainModelData {
[modelId: string]: ModelBase;
}
interface RemoteMeta extends MetaFileBase {
interface RemoteMeta extends Omit<MetaFileBase, 'lastUpdate'> {
/** Null when the remote format does not provide a trustworthy timestamp. */
lastUpdate: number | null;
mainModelData: MainModelData;
isFullData?: boolean;
}

View file

@ -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<string, number>,
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 () => {

View file

@ -102,6 +102,9 @@ export class FileBasedSyncAdapterService {
/** Expected sync version for optimistic locking, keyed by provider+user */
private _expectedSyncVersions = new Map<string, number>();
/** Downloaded version awaiting confirmation that its snapshot/ops were applied. */
private _pendingExpectedSyncVersions = new Map<string, number>();
/**
* 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<string, VectorClock>();
/** Downloaded causal clock awaiting the same durable-apply confirmation. */
private _pendingVectorClocks = new Map<string, VectorClock>();
/** Local sequence counters (simulates server seq for file-based) */
private _localSeqCounters = new Map<string, number>();
@ -376,6 +382,16 @@ export class FileBasedSyncAdapterService {
setLastServerSeq: async (seq: number): Promise<void> => {
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<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
expectedLegacyRev?: string,
): Promise<void> {
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<SyncProviderId>,
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<SyncProviderId>,
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<SyncProviderId>,
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<SyncProviderId>,
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<string, unknown> | 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<string, unknown>),
...(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);
}
/**

View file

@ -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(

View file

@ -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 } : {}),
};
}

View file

@ -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<SyncHydrationService>;
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);
}
});

View file

@ -747,6 +747,7 @@ export class OperationLogSyncService {
result.snapshotState as Record<string, unknown>,
result.snapshotVectorClock,
lastSyncedVectorClock,
result.remoteLastModified,
);
}
@ -764,6 +765,7 @@ export class OperationLogSyncService {
result.snapshotState as Record<string, unknown>,
result.snapshotVectorClock,
lastSyncedVectorClock,
result.remoteLastModified,
);
}
@ -806,6 +808,7 @@ export class OperationLogSyncService {
result.snapshotState as Record<string, unknown>,
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);

View file

@ -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": "<p>Update from remote. Both local and remote data seem to be modified.</p>",