fix(sync): harden file-based .bak recovery, split gap detection & conflict counts (#8857)

* fix(sync): harden file-based .bak recovery and split gap detection

Post-merge review of the SPAP-8/9/10/11 series found five defects in the
file-based sync adapter; all fixed here with regression tests proven
red/green against the previous code. Design cross-checked by a 7-reviewer
multi-agent pass; its findings are folded in.

- Split gap detection suppressed a syncVersion reset when the remote
  clock was EQUAL OR GREATER_THAN the last-seen clock — the exact bug the
  SPAP-9 review follow-up removed from the single-file path. A dominating
  client's snapshot reset (which compacts ops this client never saw) was
  treated as cosmetic, skipping snapshot hydration and silently
  diverging. Now EQUAL only, matching the single-file path.

- .bak recovery staged the CORRUPT primary's rev, which setLastServerSeq
  then promoted to _lastSeenRevs: every later poll's SPAP-10 pre-check
  read "unchanged" and skipped the re-download while the upload path (no
  .bak recovery) kept failing on the corrupt primary — sync wedged until
  another client rewrote the file. A recovery download now never
  stages/promotes the rev (each poll re-recovers and re-seeds the heal
  cache), and every site that rewrites _lastSeenRevs drops any stale
  staged rev via the shared _commitLastSeenRev.

- Snapshot uploads (force-upload / "Use Local" / E2EE re-encryption) left
  the pre-snapshot .bak behind. After a password rotation the stale
  old-key .bak was silently "recovered" by a still-old-key client
  (suppressing its wrong-password prompt) and heal-uploaded back over the
  new-key primary, reverting the rotation. Snapshot uploads now write the
  same payload to .bak FIRST, then the primary (_forceUploadWithBakFirst;
  deliberately FATAL — aborting pre-primary leaves the remote consistent
  for a retry). Applies to sync-data.json.bak and sync-ops.json.bak;
  sync-state.json.bak is deliberately exempt: its adoption is
  ref-validated (EQUAL clock vs snapshotRef), so a stale copy is inert,
  and it must keep serving the compaction crash window it exists for.

- Recovery additionally refuses a PLAINTEXT .bak when encryption is
  expected: decoding trusts the file's own prefix flags, so a plaintext
  .bak decodes even under a wrong/rotated key — the same
  wrong-password-suppression class via mode (rather than key) mismatch.

- The split migration wrote the v3 tombstone BEFORE neutralizing the
  legacy .bak (best-effort): a crash between the writes left a live v2
  .bak that an OFF client's recovery would resurrect over the tombstone,
  forking the folder. Neutralize-first, fatal. Residual: a step-3 failure
  after the migration's ops-file commit leaves a live v2 file until the
  next snapshot upload (documented at the call site).

- sync-ops.json — the hot file, rewritten on every op-bearing sync — had
  no backup at all, so a torn write wedged split sync until a manual
  force-upload. It now gets the same backup-before-overwrite + recovery +
  heal treatment as the single-file format. deleteAllData deletes the
  split files BEFORE the tombstone and treats source-of-truth deletion
  failures as errors (success:false) — it previously left every split
  file behind and reported success.

The three backup/recover pairs are collapsed into shared _writeBakFile /
_readBakFile helpers (the EQUAL||GREATER_THAN divergence above is exactly
the copy-drift failure mode this prevents); .bak file names move into
FILE_BASED_SYNC_CONSTANTS as remote-format surface.

* fix(sync): show the exact pending-op count in the conflict dialog

Compaction can fold still-unsynced ops into the snapshot baseline clock,
so the dialog's vector-clock delta could report "0 changes" right next to
"N local changes pending" — and the false 0 skipped the secondary
USE_REMOTE overwrite confirmation. The dialog now prefers the EXACT
pending-op count carried on LocalDataConflictError (new optional
ConflictData.localUnsyncedOpsCount): it is precisely "what USE_REMOTE
would discard", so both the displayed count and the >= 20-difference
confirmation threshold work from a truthful figure; the clock delta
remains the fallback when no measured count is supplied.
Display/confirmation-only — no clock or op-log semantics touched. Also
asserts the explicit-null lastSyncedVectorClock contract at the
fresh-client conflict throw sites (test gap from SPAP-7).

* chore(lint): enforce tx-handle-only access in op-log transactions

The SQLite op-log adapter serializes every entry point through a
per-connection FIFO queue; awaiting an adapter method inside a
.transaction() callback enqueues behind the transaction's own slot and
silently deadlocks all op-log persistence. The port contract documents
the precondition and #8849 promised a lint rule — this adds it
(no-adapter-in-tx, scoped to src/app/op-log) with RuleTester specs.
Matching is rename-proof: it flags access on the SAME receiver the
transaction was opened on (plain identifiers and any `this.<field>`), so
it does not depend on the field being named `_adapter`; known heuristic
gaps (extracted callbacks, aliasing, method indirection) are documented.
Also corrects the port doc: IndexedDB serializes only overlapping-scope
transactions; SQLite provides the stronger whole-connection exclusion.
This commit is contained in:
Johannes Millan 2026-07-08 16:45:16 +02:00 committed by GitHub
parent 1810412015
commit 610ca64894
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 1192 additions and 213 deletions

View file

@ -14,4 +14,5 @@ module.exports = {
'require-entity-registry': require('./rules/require-entity-registry'),
'no-actions-in-effects': require('./rules/no-actions-in-effects'),
'no-multi-entity-effect': require('./rules/no-multi-entity-effect'),
'no-adapter-in-tx': require('./rules/no-adapter-in-tx'),
};

View file

@ -0,0 +1,134 @@
/**
* ESLint rule: no-adapter-in-tx
*
* Code inside an `adapter.transaction(async (tx) => { ... })` callback MUST use
* only the `tx` handle. The SQLite backend (`SqliteOpLogAdapter`) serializes
* every public entry point through a per-connection FIFO queue; a transaction
* holds one queue slot for its whole BEGINCOMMIT. Awaiting any adapter method
* inside the callback (`this._adapter.get(...)` instead of `tx.get(...)`)
* enqueues behind the very slot the callback runs in and silently deadlocks all
* op-log persistence. A runtime guard cannot enforce this (a legal concurrent
* call and an illegal re-entrant one are indistinguishable at the queue), so
* enforcement lives here. See `SqliteOpLogAdapter._serialize()` in
* src/app/op-log/persistence/sqlite-op-log-adapter.ts.
*
* Heuristic (deliberately simple, low-false-positive):
* - Inside any function passed as an argument to a `.transaction(...)` call
* (nested functions included), flag member access on the SAME receiver the
* transaction was opened on both plain identifiers (`dest.put(...)` inside
* `dest.transaction(...)` in op-log-backend-migration.ts) and `this.<field>`
* receivers (`this.<anyField>.get(...)` inside
* `this.<anyField>.transaction(...)`), so the rule survives field renames.
* Access to a DIFFERENT receiver (e.g. `source.get(...)` inside
* `dest.transaction`) stays legal that is a different connection.
* - Additionally flag `this._adapter` / `this.adapter` (the conventional shared
* adapter fields in this dir) inside ANY tx callback, since a second adapter
* instance over the same connection shares the queue.
*
* Known gaps (accepted heuristic, not proof): a callback EXTRACTED to a named
* function/method and passed by reference is not analyzed; aliasing
* (`const a = this._adapter`) and adapter access hidden behind a method call
* inside the callback are invisible; identifier matching is name-based, not
* scope-aware.
*
* Scoped to src/app/op-log/** via eslint.config.js.
*/
module.exports = {
meta: {
type: 'problem',
docs: {
description:
'Inside a .transaction() callback use only the tx handle; adapter methods enqueue behind this transactions queue slot and deadlock',
category: 'Possible Errors',
recommended: true,
},
messages: {
noAdapterInTx:
'Do not call adapter methods inside a `.transaction()` callback — use the `tx` handle. On the SQLite backend every adapter entry point enqueues behind this transactions own FIFO queue slot and deadlocks op-log persistence. See SqliteOpLogAdapter._serialize().',
},
schema: [],
},
create(context) {
// One entry per enclosing `.transaction(...)` callback we are lexically
// inside; holds the receiver identifier name (or null for `this.*`-based
// receivers, which the this-check below covers).
const txCallbackStack = [];
const isThisAdapterAccess = (node) =>
node.type === 'MemberExpression' &&
!node.computed &&
node.object.type === 'ThisExpression' &&
(node.property.name === '_adapter' || node.property.name === 'adapter');
const isTransactionCallbackArg = (fnNode) => {
const parent = fnNode.parent;
return (
parent &&
parent.type === 'CallExpression' &&
parent.arguments.includes(fnNode) &&
parent.callee.type === 'MemberExpression' &&
!parent.callee.computed &&
parent.callee.property.name === 'transaction'
);
};
// Stable key for a transaction receiver: `id:<name>` for plain identifiers
// (`dest.transaction(...)`), `this:<prop>` for `this.<prop>.transaction(...)`
// — so the rule stays correct if the adapter field is ever renamed.
const receiverKeyOf = (node) => {
if (node.type === 'Identifier') return `id:${node.name}`;
if (
node.type === 'MemberExpression' &&
!node.computed &&
node.object.type === 'ThisExpression'
) {
return `this:${node.property.name}`;
}
return null;
};
const enterFunction = (node) => {
if (!isTransactionCallbackArg(node)) return;
txCallbackStack.push({
fnNode: node,
receiverKey: receiverKeyOf(node.parent.callee.object),
});
};
const exitFunction = (node) => {
const top = txCallbackStack[txCallbackStack.length - 1];
if (top && top.fnNode === node) {
txCallbackStack.pop();
}
};
return {
FunctionExpression: enterFunction,
'FunctionExpression:exit': exitFunction,
ArrowFunctionExpression: enterFunction,
'ArrowFunctionExpression:exit': exitFunction,
MemberExpression(node) {
if (txCallbackStack.length === 0) return;
// `this._adapter.<x>` / `this.adapter.<x>` — the conventional shared
// adapter fields in this dir; flagged inside ANY tx callback because a
// second adapter instance still shares the per-connection queue.
if (isThisAdapterAccess(node.object)) {
context.report({ node, messageId: 'noAdapterInTx' });
return;
}
// `<recv>.<x>` where <recv> is the SAME receiver the enclosing
// `.transaction(...)` was called on — rename-proof: covers `dest.put()`
// inside `dest.transaction(...)` AND `this.<anyField>.get()` inside
// `this.<anyField>.transaction(...)`. Access to a DIFFERENT receiver
// (e.g. `source.get()` inside `dest`'s transaction) stays legal — that
// is a different connection.
const key = receiverKeyOf(node.object);
if (key !== null && txCallbackStack.some((ctx) => ctx.receiverKey === key)) {
context.report({ node, messageId: 'noAdapterInTx' });
}
},
};
},
};

View file

@ -0,0 +1,197 @@
/**
* Tests for no-adapter-in-tx ESLint rule
*/
const { RuleTester } = require('eslint');
const rule = require('./no-adapter-in-tx');
const ruleTester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: 'module',
},
});
ruleTester.run('no-adapter-in-tx', rule, {
valid: [
// The blessed pattern: only the tx handle inside the callback.
{
code: `
class Store {
async replace() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
await tx.clear('ops');
await tx.add('ops', { seq: 1 });
const all = await tx.getAll('ops');
return all.length;
});
}
}
`,
},
// Adapter methods OUTSIDE any transaction callback are the normal API.
{
code: `
class Store {
async load() {
const entry = await this._adapter.get('ops', 1);
await this._adapter.put('meta', entry, 'k');
}
}
`,
},
// Adapter use after the transaction call completed — not inside the callback.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
await tx.add('ops', { seq: 1 });
});
await this._adapter.get('ops', 1);
}
}
`,
},
// A DIFFERENT adapter identifier inside the callback is a different
// connection (op-log-backend-migration.ts: source reads inside dest's tx
// would be legal; only dest re-entry deadlocks).
{
code: `
const migrate = async (source, dest) => {
await dest.transaction(['ops'], 'readwrite', async (tx) => {
const row = await source.get('ops', 1);
await tx.put('ops', row, 1);
});
};
`,
},
// `.transaction` on the native IDB handle inside the adapter impl itself
// uses the returned tx object, not the adapter.
{
code: `
class IdbAdapter {
iterate(store) {
const tx = this._database.transaction(store, 'readwrite');
return tx.done;
}
}
`,
},
],
invalid: [
// The core footgun: awaiting an adapter method inside its own tx callback.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
await this._adapter.get('ops', 1);
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Rename-proof: ANY \`this.<field>\` receiver is matched against its own
// transaction — the rule must not depend on the field being named _adapter.
{
code: `
class Store {
async run() {
await this._opLogDb.transaction(['ops'], 'readwrite', async (tx) => {
await this._opLogDb.get('ops', 1);
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Alternate field name `this.adapter`.
{
code: `
class Store {
async run() {
await this.adapter.transaction(['ops'], 'readwrite', async (tx) => {
await this.adapter.put('ops', {}, 1);
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Nested arrow function inside the callback still deadlocks.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
const helper = async () => this._adapter.count('ops');
await helper();
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Nested (non-arrow) async function expression inside the callback.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async function (tx) {
const helper = async function () {
return this._adapter.getAll('ops');
};
await helper();
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Identifier receiver (migration style): re-entering `dest` inside
// `dest.transaction` deadlocks.
{
code: `
const migrate = async (source, dest) => {
await dest.transaction(['ops'], 'readwrite', async (tx) => {
await dest.put('ops', {}, 1);
});
};
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Nested transaction on the same adapter is itself re-entry.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
await this._adapter.transaction(['meta'], 'readwrite', async (tx2) => {
await tx2.put('meta', {}, 'k');
});
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }],
},
// Two violations in one callback are each reported.
{
code: `
class Store {
async run() {
await this._adapter.transaction(['ops'], 'readwrite', async (tx) => {
await this._adapter.delete('ops', 1);
await this._adapter.add('ops', { seq: 2 });
});
}
}
`,
errors: [{ messageId: 'noAdapterInTx' }, { messageId: 'noAdapterInTx' }],
},
],
});
// eslint-disable-next-line no-console
console.log('no-adapter-in-tx: all RuleTester cases passed');

View file

@ -224,17 +224,26 @@ module.exports = tseslint.config(
'local-rules/no-multi-entity-effect': 'warn',
},
},
// Op-log persistence: inside an adapter.transaction() callback only the tx
// handle may be used — adapter methods enqueue behind the transaction's own
// FIFO queue slot on the SQLite backend and deadlock (see
// SqliteOpLogAdapter._serialize()).
{
files: ['src/app/op-log/**/*.ts'],
plugins: {
'local-rules': localRules,
},
rules: {
'local-rules/no-adapter-in-tx': 'error',
},
},
// App code must route logging through Log/SyncLog/OpLog/... helpers.
// Direct console.* calls bypass the exportable log history users attach
// to bug reports. The Log implementation itself, tests, and benchmarks
// (which intentionally dump timing numbers to stdout) are exempt.
{
files: ['src/app/**/*.ts'],
ignores: [
'src/app/**/*.spec.ts',
'src/app/**/*.benchmark.ts',
'src/app/core/log.ts',
],
ignores: ['src/app/**/*.spec.ts', 'src/app/**/*.benchmark.ts', 'src/app/core/log.ts'],
rules: {
'no-console': 'error',
},

View file

@ -167,6 +167,10 @@ export const FILE_BASED_SYNC_CONSTANTS = {
// sync setting). See FileBasedOpsFile / FileBasedStateFile / FileBasedSplitTombstone.
OPS_FILE: 'sync-ops.json',
STATE_FILE: 'sync-state.json',
// SPAP-8-style recovery artifacts for the split files. Remote-format surface:
// old and new clients must agree on these names forever.
OPS_BACKUP_FILE: 'sync-ops.json.bak',
STATE_BACKUP_FILE: 'sync-state.json.bak',
SPLIT_FILE_VERSION: 3 as const,
// Post-compaction RETAINED size for the split ops file (≈ MAX_RECENT_OPS/2).
// NOTE: this is the trim target, NOT the trigger — a recompaction fires when the

View file

@ -12,8 +12,10 @@ const buildConflictData = (overrides: {
localVectorClock?: VectorClock;
remoteVectorClock?: VectorClock;
lastSyncedVectorClock?: VectorClock | null;
localUnsyncedOpsCount?: number;
}): ConflictData => ({
reason: ConflictReason.NoLastSync,
localUnsyncedOpsCount: overrides.localUnsyncedOpsCount,
remote: {
lastUpdate: 1000,
lastUpdateAction: 'Remote data',
@ -91,6 +93,50 @@ describe('DialogSyncConflictComponent', () => {
expect(component.remoteChangeCount).toBeNull();
});
it('shows the exact pending-op count when the clock delta under-counts (compaction fold)', () => {
// Compaction folded the unsynced ops into the last-synced clock, so the
// per-client delta computes 0 even though 3 ops are known to be pending.
// The measured count is what USE_REMOTE would discard — show it.
const component = createComponent(
buildConflictData({
localVectorClock: { clientA: 5 },
remoteVectorClock: { clientA: 5, clientB: 2 },
lastSyncedVectorClock: { clientA: 5 },
localUnsyncedOpsCount: 3,
}),
);
expect(component.localChangeCount).toBe(3);
// The remote count still comes from the clock delta.
expect(component.remoteChangeCount).toBe(2);
});
it('prefers the measured pending-op count over the clock delta whenever provided', () => {
const component = createComponent(
buildConflictData({
localVectorClock: { clientA: 10 },
remoteVectorClock: { clientA: 3 },
lastSyncedVectorClock: { clientA: 3 },
localUnsyncedOpsCount: 3,
}),
);
// delta = 7, but 3 is the exact figure for "what USE_REMOTE discards".
expect(component.localChangeCount).toBe(3);
});
it('falls back to the clock delta when no pending-op count is provided', () => {
const component = createComponent(
buildConflictData({
localVectorClock: { clientA: 10 },
remoteVectorClock: { clientA: 3 },
lastSyncedVectorClock: { clientA: 3 },
}),
);
expect(component.localChangeCount).toBe(7);
});
it('returns null when vector clocks are entirely absent', () => {
const component = createComponent(
buildConflictData({
@ -150,5 +196,32 @@ describe('DialogSyncConflictComponent', () => {
).getConfirmationMessage('USE_REMOTE');
expect(msg).toContain(T.F.SYNC.D_CONFLICT.OVERWRITE_WARNING_UNKNOWN);
});
it('confirms USE_REMOTE from the exact pending count when the clock delta reads 0', () => {
// Compaction folded 25 unsynced ops into the baseline clock: the delta
// computed local=0, remote=0 and shouldConfirmOverwrite('USE_REMOTE')
// returned false — no secondary confirmation despite 25 real unsynced
// local changes about to be discarded. The measured count restores the
// designed >= 20-difference confirmation.
const component = createComponent(
buildConflictData({
localVectorClock: { clientA: 5 },
remoteVectorClock: { clientA: 5 },
lastSyncedVectorClock: { clientA: 5 },
localUnsyncedOpsCount: 25,
}),
);
expect(component.localChangeCount).toBe(25);
const typed = component as unknown as {
shouldConfirmOverwrite(r: string): boolean;
getConfirmationMessage(r: string): string;
};
expect(typed.shouldConfirmOverwrite('USE_REMOTE')).toBe(true);
// The counted (not count-free) warning is used — the figure is exact.
expect(typed.getConfirmationMessage('USE_REMOTE')).toContain(
T.F.SYNC.D_CONFLICT.OVERWRITE_WARNING,
);
});
});
});

View file

@ -57,7 +57,7 @@ export class DialogSyncConflictComponent {
isHighlightLocal = !this.isHighlightRemote;
remoteChangeCount = this.getChangeCount('remote');
localChangeCount = this.getChangeCount('local');
localChangeCount = this.getLocalChangeCount();
isHighlightRemoteChanges = (this.remoteChangeCount ?? 0) > (this.localChangeCount ?? 0);
isHighlightLocalChanges = !this.isHighlightRemoteChanges;
@ -156,9 +156,22 @@ export class DialogSyncConflictComponent {
return changeCount;
}
/**
* Local change count. Prefers the EXACT pending-op count measured from the
* op log over the vector-clock delta: compaction can fold still-unsynced ops
* into the last-synced baseline clock, so the delta can under-count real
* pending changes (e.g. report 0 while N unsynced ops exist which also
* wrongly skipped the secondary overwrite confirmation). The measured count
* is precisely "what USE_REMOTE would discard", so it is the decision-relevant
* figure; the delta remains the fallback for producers that don't supply it.
*/
private getLocalChangeCount(): number | null {
return this.data.localUnsyncedOpsCount ?? this.getChangeCount('local');
}
private shouldConfirmOverwrite(resolution: DialogConflictResolutionResult): boolean {
const remoteChanges = this.getChangeCount('remote');
const localChanges = this.getChangeCount('local');
const remoteChanges = this.remoteChangeCount;
const localChanges = this.localChangeCount;
// If we cannot quantify the changes (fresh client, no last-synced
// baseline), still show the confirmation — overwriting could discard real
@ -181,8 +194,8 @@ export class DialogSyncConflictComponent {
}
private getConfirmationMessage(resolution: DialogConflictResolutionResult): string {
const remoteChanges = this.getChangeCount('remote');
const localChanges = this.getChangeCount('local');
const remoteChanges = this.remoteChangeCount;
const localChanges = this.localChangeCount;
const [sourceName, targetName] =
resolution === 'USE_REMOTE' ? ['remote', 'local'] : ['local', 'remote'];

View file

@ -1249,6 +1249,7 @@ export class SyncWrapperService {
vectorClock: localClock,
lastSyncedVectorClock: error.lastSyncedVectorClock ?? null,
},
localUnsyncedOpsCount: error.unsyncedCount,
};
SyncLog.log(

View file

@ -153,6 +153,15 @@ export interface ConflictData {
reason: ConflictReason;
remote: RemoteMeta;
local: LocalMeta;
/**
* Exact number of unsynced local ops known at conflict time (from
* LocalDataConflictError.unsyncedCount). The conflict dialog prefers it over
* the vector-clock delta as the local change count: compaction can fold
* still-unsynced ops into the last-synced baseline clock, so the delta can
* under-count real pending local changes, while this is precisely what
* USE_REMOTE would discard.
*/
localUnsyncedOpsCount?: number;
additional?: unknown;
}

View file

@ -137,11 +137,13 @@ export interface OpLogTx {
*
* Concurrency: callers may issue operations concurrently the op-log does
* (capture append, archive write and compaction overlap). Transactions are
* mutually exclusive per database, and no bare operation may interleave into an
* open transaction. IndexedDB provides this natively (independent transactions);
* a backend over a single connection with no nested transactions (SQLite) MUST
* serialize internally to honor it. Callers therefore never need to lock around
* these methods themselves.
* mutually exclusive per overlapping store scope, and no bare operation may
* interleave into an open transaction. IndexedDB provides this natively
* (transactions with overlapping scopes are serialized; disjoint-scope ones may
* run concurrently); a backend over a single connection with no nested
* transactions (SQLite) MUST serialize internally and provides the stronger
* whole-connection exclusion via its FIFO queue. Callers therefore never need
* to lock around these methods themselves.
*/
export interface OpLogDbAdapter {
/**

View file

@ -861,11 +861,22 @@ describe('FileBasedSyncAdapterService', () => {
expect(uploadedData.recentOps).toEqual([]); // Fresh start
});
it('should upload snapshot directly without backup (snapshots replace state completely)', async () => {
it('writes the SAME snapshot payload to .bak BEFORE the primary (rotation-safe replace)', async () => {
// A snapshot replaces the remote wholesale, so the pre-snapshot .bak must
// not survive it: after an E2EE password rotation a stale old-key .bak
// would be silently "recovered" by a still-old-key client (suppressing its
// wrong-password prompt) and heal-uploaded back over the new-key primary.
mockProvider.downloadFile.and.throwError(
new RemoteFileNotFoundAPIError('sync-data.json'),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-1' }));
const uploads: { path: string; data: string }[] = [];
mockProvider.uploadFile.and.callFake(
async (path: string, data: string, _rev: string | null, force?: boolean) => {
uploads.push({ path, data });
expect(force).toBe(true);
return { rev: `${path}-rev` };
},
);
await adapter.uploadSnapshot(
{},
@ -877,20 +888,17 @@ describe('FileBasedSyncAdapterService', () => {
'test-op-id-2', // opId
);
// Should upload to main sync file with force overwrite (no backup created)
expect(mockProvider.uploadFile).toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
jasmine.any(String),
null,
true, // Force overwrite - snapshots replace state completely
);
// Should NOT create backup file
expect(mockProvider.uploadFile).not.toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
jasmine.any(String),
jasmine.anything(),
jasmine.anything(),
);
const paths = uploads.map((u) => u.path);
const bakIdx = paths.indexOf(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE);
const mainIdx = paths.indexOf(FILE_BASED_SYNC_CONSTANTS.SYNC_FILE);
expect(bakIdx).toBeGreaterThanOrEqual(0);
expect(mainIdx).toBeGreaterThanOrEqual(0);
// .bak first, then the primary — a crash in between leaves a valid old
// primary + already-refreshed .bak (nothing recoverable, nothing stale).
expect(bakIdx).toBeLessThan(mainIdx);
// Identical payload: the .bak is the snapshot itself, not the pre-snapshot
// remote it deliberately replaces.
expect(uploads[bakIdx].data).toBe(uploads[mainIdx].data);
});
describe('uploadSnapshot gap detection', () => {
@ -2331,6 +2339,80 @@ describe('FileBasedSyncAdapterService', () => {
expect(mockSnackService.open).toHaveBeenCalled();
});
it('(b) recovers from .bak when the primary fails to DECRYPT (real DecryptError)', async () => {
// Same shape as the DecompressError case, driven through a REAL decrypt
// failure: the adapter holds a key, the primary claims encryption but its
// body is garbage. Safe against key rotation because uploadSnapshot
// refreshes .bak with the same payload/key as the primary, so a
// primary/.bak key mismatch cannot exist on a healthy remote.
const encAdapter = service.createAdapter(mockProvider, mockCfg, 'test-password');
const backupData = createMockSyncData({
syncVersion: 5,
recentOps: [compactOp('recovered-from-decrypt-failure') as never],
});
const undecryptableMain =
getSyncFilePrefix({ isCompress: false, isEncrypt: true, modelVersion: 2 }) +
'bm90LXZhbGlkLWNpcGhlcnRleHQ=';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({
dataStr: addPrefix(backupData),
rev: 'bak-rev-decrypt',
});
}
return Promise.resolve({ dataStr: undecryptableMain, rev: 'corrupt-enc-rev' });
});
const result = await encAdapter.downloadOps(0);
expect(result.ops.length).toBe(1);
expect(result.ops[0].op.id).toBe('recovered-from-decrypt-failure');
expect(result.latestSeq).toBe(5);
expect(mockSnackService.open).toHaveBeenCalled();
});
it('(b) refuses a PLAINTEXT .bak when encryption is expected (key-suppression vector)', async () => {
// A plaintext .bak decodes via its own prefix flags even under a
// wrong/rotated key. Accepting it would suppress the wrong-password
// dialog and let the heal upload clobber the encrypted primary — the
// same class as the E2EE-rotation revert. Recovery must refuse it and
// surface the ORIGINAL DecryptError instead.
const encAdapter = service.createAdapter(
mockProvider,
{ isEncrypt: true, isCompress: false },
'test-password',
);
const backupData = createMockSyncData({ syncVersion: 5 });
const undecryptableMain =
getSyncFilePrefix({ isCompress: false, isEncrypt: true, modelVersion: 2 }) +
'bm90LXZhbGlkLWNpcGhlcnRleHQ=';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
// addPrefix uses mockCfg (plaintext) → a plaintext-prefixed .bak.
return Promise.resolve({ dataStr: addPrefix(backupData), rev: 'plain-bak' });
}
return Promise.resolve({ dataStr: undecryptableMain, rev: 'corrupt-enc-rev' });
});
await expectAsync(encAdapter.downloadOps(0)).toBeRejected();
expect(mockSnackService.open).not.toHaveBeenCalled();
});
it('(b) rethrows the ORIGINAL error when both primary and .bak are undecodable', async () => {
const garbage =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 2 }) +
'not-valid-gzip';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ dataStr: garbage, rev: 'bak-corrupt-too' });
}
return Promise.resolve({ dataStr: garbage, rev: 'main-corrupt' });
});
await expectAsync(adapter.downloadOps(0)).toBeRejected();
expect(mockSnackService.open).not.toHaveBeenCalled();
});
it('(b) after recovery, the next upload heals the corrupt primary via ITS rev (no revToMatch pollution)', async () => {
// Regression for the self-perpetuating degraded state: recovery must seed the
// cache with the CORRUPT PRIMARY rev, not the .bak rev, so the follow-up
@ -2759,6 +2841,47 @@ describe('FileBasedSyncAdapterService', () => {
expect(mockProvider.downloadFile).toHaveBeenCalledTimes(1);
expect(result.snapshotState).toBeDefined();
});
it('(g) .bak recovery never promotes the corrupt primary rev to last-seen (heal cannot wedge)', async () => {
// Regression: recovery serves the CORRUPT primary's rev (for the in-cycle
// heal), and setLastServerSeq promotes staged revs. If the corrupt rev were
// promoted, every later poll's pre-check would read "unchanged" and skip the
// full download while the upload path (no .bak recovery) keeps failing on the
// corrupt primary — sync wedged until another client rewrites the file.
const CORRUPT_MAIN_REV = 'corrupt-main-rev-wedge';
const backupData = createMockSyncData({ syncVersion: 2, recentOps: [] });
const undecodableMain =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 2 }) +
'not-valid-gzip';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ dataStr: addPrefix(backupData), rev: 'bak-rev' });
}
return Promise.resolve({ dataStr: undecodableMain, rev: CORRUPT_MAIN_REV });
});
// Recovery download (nothing new to apply) + the caller persisting the
// cursor — the promotion point that previously committed the corrupt rev.
await adapter.downloadOps(2);
await adapter.setLastServerSeq(2);
crossPollBoundary();
mockProvider.downloadFile.calls.reset();
// The remote is still the same corrupt file.
mockProvider.getFileRev.and.callFake(async (path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.SYNC_FILE) {
return { rev: CORRUPT_MAIN_REV };
}
throw new RemoteFileNotFoundAPIError(path);
});
const result = await adapter.downloadOps(2);
// Must NOT short-circuit on the corrupt rev: the full download re-recovers
// and re-seeds the heal cache so a later upload can still heal the primary.
expect(mockProvider.downloadFile).toHaveBeenCalled();
expect(result.latestSeq).toBe(2);
});
});
// ═══════════════════════════════════════════════════════════════════════════
@ -2841,10 +2964,11 @@ describe('FileBasedSyncAdapterService', () => {
// Never builds a snapshot on the op-only path.
expect(mockStateSnapshotService.getStateSnapshot).not.toHaveBeenCalled();
// Every upload targeted the ops file only.
// Every upload targeted the ops file or its SPAP-8 backup — never the
// snapshot (sync-state.json) or legacy (sync-data.json) files.
const paths = uploadedPaths();
expect(paths.length).toBeGreaterThan(0);
paths.forEach((p) => expect(p).toBe(C.OPS_FILE));
expect(paths).toContain(C.OPS_FILE);
paths.forEach((p) => expect([C.OPS_FILE, C.OPS_BACKUP_FILE]).toContain(p as never));
// Only the ops file was downloaded.
mockProvider.downloadFile.calls.allArgs().forEach((args) => {
expect(args[0]).toBe(C.OPS_FILE);
@ -2929,6 +3053,50 @@ describe('FileBasedSyncAdapterService', () => {
expect(result.snapshotState).toBeDefined();
});
// (a4) Same review follow-up as the single-file path: a strictly-dominating
// (GREATER_THAN) remote clock is NOT proof this client received the ops that
// a snapshot reset compacted into sync-state.json, so a syncVersion
// regression must flag a gap. The split fork originally suppressed on
// EQUAL || GREATER_THAN and silently diverged.
it('(a4) split download flags a gap when a dominating-clock reset compacted ops the client had not yet seen', async () => {
// First download establishes expected syncVersion (5) + last-seen clock.
routeDownloads({
[C.OPS_FILE]: addPrefix(
makeOpsFile({
syncVersion: 5,
vectorClock: { client1: 5 },
recentOps: [makeCompactOp({ sv: 5, v: { client1: 5 } })],
oldestOpSyncVersion: 5,
}),
3,
),
});
await adapter.downloadOps(5, 'client2');
// A strictly-ahead client snapshot-reset the folder (compacting ops this
// client never saw into the snapshot) and then made one more edit.
const stateFile = makeStateFile({
syncVersion: 1,
vectorClock: { client1: 6 },
});
const opsFile = makeOpsFile({
syncVersion: 2,
vectorClock: { client1: 7 }, // GREATER_THAN last-seen {client1: 5}
recentOps: [makeCompactOp({ id: 'op-post-reset', sv: 2, v: { client1: 7 } })],
oldestOpSyncVersion: 2,
snapshotRef: { syncVersion: 1, vectorClock: { client1: 6 }, rev: 'state-rev-x' },
});
routeDownloads({
[C.OPS_FILE]: addPrefix(opsFile, 3),
[C.STATE_FILE]: addPrefix(stateFile, 3),
});
const result = await adapter.downloadOps(5, 'client2');
expect(result.gapDetected).toBe(true);
expect(result.snapshotState).toBeDefined();
});
// (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 () => {
@ -2999,7 +3167,7 @@ describe('FileBasedSyncAdapterService', () => {
}),
3,
),
[C.STATE_FILE + '.bak']: addPrefix(
[C.STATE_BACKUP_FILE]: addPrefix(
makeStateFile({
syncVersion: 1,
vectorClock: { client1: 1 },
@ -3075,11 +3243,16 @@ describe('FileBasedSyncAdapterService', () => {
expect(tomb.version).toBe(C.SPLIT_FILE_VERSION);
expect(tomb.format).toBe(C.SPLIT_TOMBSTONE_FORMAT);
// .bak neutralized to a v3 tombstone too.
// .bak neutralized to a v3 tombstone too — and BEFORE the tombstone write,
// so a crash in between leaves a valid v2 primary (nothing stale to
// recover) instead of a tombstone + live v2 .bak that an OFF client's
// SPAP-8 recovery would resurrect over the tombstone.
const bakIdx = paths.indexOf(C.BACKUP_FILE);
expect(bakIdx).toBeGreaterThanOrEqual(0);
expect(bakIdx).toBeLessThan(tombIdx);
const bakCall = mockProvider.uploadFile.calls
.allArgs()
.find((a) => a[0] === C.BACKUP_FILE);
expect(bakCall).toBeDefined();
const bak = parseWithPrefix(bakCall![1] as string) as unknown as {
version: number;
};
@ -3193,5 +3366,150 @@ describe('FileBasedSyncAdapterService', () => {
expect(downloaded).toContain(C.OPS_FILE);
expect(downloaded).toContain(C.STATE_FILE);
});
it('(i) op upload backs up the current sync-ops.json to .bak BEFORE overwriting it', async () => {
// sync-ops.json is the hot file (rewritten on every op-bearing sync) — it
// gets the same SPAP-8 backup-before-overwrite as the single-file format.
const existing = makeOpsFile({
syncVersion: 3,
recentOps: [makeCompactOp({ id: 'op-existing' })],
});
routeDownloads({ [C.OPS_FILE]: addPrefix(existing, 3) });
await adapter.uploadOps([createMockSyncOp()], 'client1');
const paths = uploadedPaths();
const bakIdx = paths.indexOf(C.OPS_BACKUP_FILE);
const opsIdx = paths.lastIndexOf(C.OPS_FILE);
expect(bakIdx).toBeGreaterThanOrEqual(0);
expect(bakIdx).toBeLessThan(opsIdx);
// The .bak preserves the PRE-overwrite remote (syncVersion 3, not the new 4).
const bakCall = mockProvider.uploadFile.calls
.allArgs()
.find((a) => a[0] === C.OPS_BACKUP_FILE);
const bak = parseWithPrefix(bakCall![1] as string) as unknown as FileBasedOpsFile;
expect(bak.syncVersion).toBe(3);
});
it('(i) recovers a corrupt sync-ops.json from .bak and heals it via ITS rev', async () => {
const bakOps = makeOpsFile({
syncVersion: 4,
recentOps: [makeCompactOp({ id: 'op-from-bak' })],
});
const garbage =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 3 }) +
'not-valid-gzip';
mockProvider.downloadFile.and.callFake(async (path: string) => {
if (path === C.OPS_FILE) return { dataStr: garbage, rev: 'corrupt-ops-rev' };
if (path === C.OPS_BACKUP_FILE) {
return { dataStr: addPrefix(bakOps, 3), rev: 'ops-bak-rev' };
}
throw new RemoteFileNotFoundAPIError(path);
});
const result = await adapter.downloadOps(1, 'client2');
expect(result.ops.some((o) => o.op.id === 'op-from-bak')).toBe(true);
expect(mockSnackService.open).toHaveBeenCalled();
// The next upload heals the corrupt primary via a conditional PUT matching
// ITS rev (not the .bak rev, which would mismatch forever).
const opsRevToMatch: (string | null)[] = [];
mockProvider.uploadFile.and.callFake(
async (path: string, _d: string, revToMatch: string | null) => {
if (path === C.OPS_FILE) opsRevToMatch.push(revToMatch);
return { rev: `${path}-newrev` };
},
);
await adapter.uploadOps([createMockSyncOp()], 'client1');
expect(opsRevToMatch).toContain('corrupt-ops-rev');
expect(opsRevToMatch).not.toContain('ops-bak-rev');
});
it('(i) split .bak recovery never promotes the corrupt ops rev to last-seen (heal cannot wedge)', async () => {
mockProvider.id = SyncProviderId.Dropbox;
adapter = service.createAdapter(mockProvider, mockCfg, mockEncryptKey);
const bakOps = makeOpsFile({ syncVersion: 4, recentOps: [] });
const garbage =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 3 }) +
'not-valid-gzip';
mockProvider.downloadFile.and.callFake(async (path: string) => {
if (path === C.OPS_FILE) return { dataStr: garbage, rev: 'corrupt-ops-rev' };
if (path === C.OPS_BACKUP_FILE) {
return { dataStr: addPrefix(bakOps, 3), rev: 'ops-bak-rev' };
}
throw new RemoteFileNotFoundAPIError(path);
});
await adapter.downloadOps(1, 'client2');
await adapter.setLastServerSeq(4); // the promotion point
const lastSeen = (service as unknown as { _lastSeenRevs: Map<string, string> })
._lastSeenRevs;
expect(lastSeen.get('Dropbox')).toBeUndefined();
});
it('(i) snapshot upload refreshes the OPS .bak (not the ref-validated state .bak) before the primary', async () => {
// Rotation-safe replace: sync-ops.json.bak feeds UNVALIDATED recovery, so
// a pre-snapshot (possibly rotated-key) copy must not survive a snapshot
// upload. sync-state.json.bak is deliberately NOT refreshed — its adoption
// is ref-validated (EQUAL clock vs snapshotRef), so a stale copy is inert,
// and it must keep serving the compaction crash window it was made for.
routeDownloads({});
await adapter.uploadSnapshot(
{},
'client1',
'recovery',
{ client1: 1 },
1,
undefined,
'op-id-split-snap',
);
const paths = uploadedPaths();
const oBak = paths.indexOf(C.OPS_BACKUP_FILE);
const oMain = paths.indexOf(C.OPS_FILE);
expect(oBak).toBeGreaterThanOrEqual(0);
expect(oMain).toBeGreaterThanOrEqual(0);
expect(oBak).toBeLessThan(oMain);
// Identical payloads: the ops .bak IS the fresh snapshot-reset ops file.
const argAt = (i: number): string =>
mockProvider.uploadFile.calls.allArgs()[i][1] as string;
expect(argAt(oBak)).toBe(argAt(oMain));
// State .bak untouched.
expect(paths).not.toContain(C.STATE_BACKUP_FILE);
expect(paths).toContain(C.STATE_FILE);
});
it('(i) deleteAllData removes the split files (BEFORE the tombstone) and their .baks', async () => {
mockProvider.removeFile.and.returnValue(Promise.resolve(undefined as never));
await adapter.deleteAllData();
const removed = mockProvider.removeFile.calls.allArgs().map((a) => a[0] as string);
expect(removed).toContain(C.OPS_FILE);
expect(removed).toContain(C.STATE_FILE);
expect(removed).toContain(C.OPS_BACKUP_FILE);
expect(removed).toContain(C.STATE_BACKUP_FILE);
// Split source-of-truth files go before sync-data.json: a partial failure
// must not leave a deleted tombstone next to live split data (an OFF
// client would fresh-start a v2 file against it).
expect(removed.indexOf(C.OPS_FILE)).toBeLessThan(removed.indexOf(C.SYNC_FILE));
expect(removed.indexOf(C.STATE_FILE)).toBeLessThan(removed.indexOf(C.SYNC_FILE));
});
it('(i) deleteAllData FAILS when a split source-of-truth file cannot be deleted', async () => {
// Reporting success while sync-ops.json still holds user data on the
// remote would be a lie — only .bak/lock artifacts are best-effort.
mockProvider.removeFile.and.callFake(async (path: string) => {
if (path === C.OPS_FILE) throw new Error('server error');
return undefined as never;
});
const res = await adapter.deleteAllData();
expect(res.success).toBe(false);
});
});
});

View file

@ -11,6 +11,7 @@ import {
RestorePointType,
} from '../provider.interface';
import { EncryptAndCompressHandlerService } from '../../encryption/encrypt-and-compress-handler.service';
import { extractSyncFileStateFromPrefix } from '../../util/sync-file-prefix';
import { EncryptAndCompressCfg } from '../../core/types/sync.types';
import {
Operation,
@ -699,10 +700,17 @@ export class FileBasedSyncAdapterService {
// Step 2.5: Backup-before-overwrite (two-phase write). Copy the CURRENT remote
// content to sync-data.json.bak before overwriting the primary file, so an
// interrupted/corrupt write can be recovered on the next download. Non-fatal:
// a provider without copy support must still be able to sync.
// interrupted/corrupt write can be recovered on the next download. We re-encode
// the already-in-hand `currentData` rather than issuing another GET.
if (fileExists && currentData) {
await this._backupCurrentRemote(provider, cfg, encryptKey, currentData);
await this._writeBakFile(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
currentData,
FILE_BASED_SYNC_CONSTANTS.FILE_VERSION,
);
}
// Step 3: Upload conditionally; on rev mismatch re-download and retry
@ -721,7 +729,7 @@ export class FileBasedSyncAdapterService {
// SPAP-10: the remote is now exactly what we just uploaded, so its rev is the
// fresh last-seen rev. Recording it lets the next poll short-circuit if no
// other client writes in between. Persisted via _persistState() below.
this._lastSeenRevs.set(providerKey, finalRev);
this._commitLastSeenRev(providerKey, finalRev);
// Use finalSyncVersion (NOT mergedOps.length) to match download behavior.
// mergedOps.length is the total ops count, which can be much larger than syncVersion
@ -832,6 +840,7 @@ export class FileBasedSyncAdapterService {
let syncData: FileBasedSyncData;
let rev: string;
let recoveredFromBackup = false;
try {
const result = await this._downloadSyncFile(provider, cfg, encryptKey);
syncData = result.data;
@ -857,8 +866,15 @@ export class FileBasedSyncAdapterService {
if (this._isRecoverableCorruption(e)) {
// Primary sync-data.json is corrupt/empty/unparseable (e.g. interrupted
// write). Try to recover from the .bak artifact before failing.
const recovered = await this._recoverFromBackup(provider, cfg, encryptKey);
const recovered = await this._readBakFile<FileBasedSyncData>(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.FILE_VERSION,
);
if (recovered) {
recoveredFromBackup = true;
OpLog.warn(
'FileBasedSyncAdapter: Primary sync file unreadable; recovered from backup',
);
@ -873,16 +889,7 @@ export class FileBasedSyncAdapterService {
(e as { primaryRev?: string } | null)?.primaryRev ?? recovered.rev;
rev = primaryRev;
this._setCachedSyncData(providerKey, syncData, rev);
// De-dup the user-facing notice: only surface it the first time we see a
// given corrupt primary rev, so a download-only client (which cannot heal
// the file itself) does not re-toast on every sync cycle.
if (this._lastRecoveredCorruptRev.get(providerKey) !== primaryRev) {
this._lastRecoveredCorruptRev.set(providerKey, primaryRev);
// Lazy resolve — absent in some unit harnesses (returns null → no notice).
this._injector
.get(SnackService, null)
?.open({ msg: T.F.SYNC.S.SYNC_DATA_RECOVERED_FROM_BACKUP });
}
this._notifyRecoveredCorruptPrimaryOnce(providerKey, primaryRev);
} else {
throw e;
}
@ -1005,13 +1012,7 @@ export class FileBasedSyncAdapterService {
// 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);
// SPAP-10 (review follow-up): stage this file's rev as PENDING rather than
// committing it to `_lastSeenRevs` here. It is promoted to last-seen (and
// persisted) only once the caller confirms the ops were durably applied, via
// setLastServerSeq — the same ordering as the seq cursor. Committing it eagerly
// here would let a throw/crash between download and apply strand a rev ahead of
// un-applied ops, so the next poll's cheap pre-check would skip them for good.
this._pendingRevs.set(providerKey, rev);
this._stageOrDropDownloadedRev(providerKey, rev, recoveredFromBackup);
// Filter ops using operation IDs instead of synthetic seq numbers.
// Synthetic seq numbers based on array indices shift when the array is trimmed,
@ -1168,7 +1169,6 @@ export class FileBasedSyncAdapterService {
recentOps: [], // Fresh start - no recent ops
};
// Upload snapshot (no backup - snapshots replace state completely)
const uploadData = await this._encryptAndCompressHandler.compressAndEncryptData(
cfg,
encryptKey,
@ -1176,11 +1176,11 @@ export class FileBasedSyncAdapterService {
FILE_BASED_SYNC_CONSTANTS.FILE_VERSION,
);
this._assertUploadDataNotEmpty(uploadData, 'FileBasedSyncAdapter._uploadSnapshot');
const snapshotUploadRes = await provider.uploadFile(
const snapshotUploadRes = await this._forceUploadWithBakFirst(
provider,
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
uploadData,
null,
true,
);
// Reset local state
@ -1188,7 +1188,7 @@ export class FileBasedSyncAdapterService {
this._localSeqCounters.set(providerKey, newSyncVersion);
// SPAP-10: record the rev of the snapshot we just wrote as the last-seen rev
// so the next poll can skip a redundant full download.
this._lastSeenRevs.set(providerKey, snapshotUploadRes.rev);
this._commitLastSeenRev(providerKey, snapshotUploadRes.rev);
this._persistState();
OpLog.warn(
@ -1213,30 +1213,40 @@ export class FileBasedSyncAdapterService {
OpLog.normal('FileBasedSyncAdapter: Deleting all sync data');
try {
// Delete main sync file — re-throw real errors (not "file not found")
try {
await provider.removeFile(FILE_BASED_SYNC_CONSTANTS.SYNC_FILE);
} catch (e) {
if (!(e instanceof RemoteFileNotFoundAPIError)) {
throw e;
// Source-of-truth files: a failed deletion MUST fail the whole operation
// (outer catch → success:false) — reporting success while user data
// remains on the remote would be a lie. The split files go FIRST so a
// partial failure can't leave a deleted tombstone next to live split data
// (an OFF client would then fresh-start a v2 file against it). "Not
// found" is fine — most users are on the single-file format.
for (const dataFile of [
FILE_BASED_SYNC_CONSTANTS.OPS_FILE,
FILE_BASED_SYNC_CONSTANTS.STATE_FILE,
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
]) {
try {
await provider.removeFile(dataFile);
} catch (e) {
if (!(e instanceof RemoteFileNotFoundAPIError)) {
throw e;
}
}
}
// Delete backup file
try {
await provider.removeFile(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE);
} catch (e) {
if (!(e instanceof RemoteFileNotFoundAPIError)) {
OpLog.warn('FileBasedSyncAdapter: Unexpected error deleting backup file', e);
}
}
// Delete migration lock if exists
try {
await provider.removeFile(FILE_BASED_SYNC_CONSTANTS.MIGRATION_LOCK_FILE);
} catch (e) {
if (!(e instanceof RemoteFileNotFoundAPIError)) {
OpLog.warn('FileBasedSyncAdapter: Unexpected error deleting migration lock', e);
// Disposable artifacts: best-effort — leaving one behind degrades nothing
// (recovery only triggers on a corrupt primary, never on a missing one).
for (const artifact of [
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.OPS_BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.STATE_BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.MIGRATION_LOCK_FILE,
]) {
try {
await provider.removeFile(artifact);
} catch (e) {
if (!(e instanceof RemoteFileNotFoundAPIError)) {
OpLog.warn(`FileBasedSyncAdapter: Unexpected error deleting ${artifact}`, e);
}
}
}
@ -1246,7 +1256,10 @@ export class FileBasedSyncAdapterService {
// SPAP-10: drop the last-seen rev so a stale value can't drive a false
// "nothing new" short-circuit after the remote file has been deleted.
this._lastSeenRevs.delete(providerKey);
this._pendingRevs.delete(providerKey);
this._lastSeenVectorClocks.delete(providerKey);
this._clearCachedSyncData(providerKey);
this._clearCachedOpsData(providerKey);
this._persistState();
return { success: true };
@ -1260,9 +1273,6 @@ export class FileBasedSyncAdapterService {
// SPAP-11: SPLIT-FILE ("SURGICAL SYNC") FORMAT (opt-in)
// ═══════════════════════════════════════════════════════════════════════════
/** State-file backup artifact name (SPAP-8-style recovery for snapshotRef mismatch). */
private readonly _STATE_BACKUP_FILE = FILE_BASED_SYNC_CONSTANTS.STATE_FILE + '.bak';
/**
* Whether the opt-in split-file sync setting is ON. Resolved lazily via the
* injector (like SnackService) so the single-file unit harness which does not
@ -1321,19 +1331,25 @@ export class FileBasedSyncAdapterService {
encryptKey: string | undefined,
): Promise<{ data: FileBasedOpsFile; rev: string }> {
const response = await provider.downloadFile(FILE_BASED_SYNC_CONSTANTS.OPS_FILE);
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedOpsFile>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION) {
throw new SyncDataCorruptedError(
`Unsupported ops-file version: ${data.version} (expected ${FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION})`,
FILE_BASED_SYNC_CONSTANTS.OPS_FILE,
);
try {
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedOpsFile>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION) {
throw new SyncDataCorruptedError(
`Unsupported ops-file version: ${data.version} (expected ${FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION})`,
FILE_BASED_SYNC_CONSTANTS.OPS_FILE,
);
}
return { data, rev: response.rev };
} catch (decodeErr) {
// Annotate the corrupt file's rev so the .bak recovery path can seed the
// heal cache with IT (mirrors _downloadSyncFile).
throw this._annotatePrimaryRev(decodeErr, response.rev);
}
return { data, rev: response.rev };
}
/** Downloads + decodes `sync-state.json` and validates its version. */
@ -1391,20 +1407,22 @@ export class FileBasedSyncAdapterService {
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
): Promise<void> {
let current: { data: FileBasedStateFile };
try {
const current = await this._downloadStateFile(provider, cfg, encryptKey);
const backupData = await this._encryptAndCompressHandler.compressAndEncryptData(
cfg,
encryptKey,
current.data,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
if (!backupData || backupData.trim().length === 0) return;
await provider.uploadFile(this._STATE_BACKUP_FILE, backupData, null, true);
current = await this._downloadStateFile(provider, cfg, encryptKey);
} catch (e) {
// Non-fatal — e.g. first compaction has no existing state file to back up.
OpLog.normal('FileBasedSyncAdapter: state-file backup skipped (non-fatal)', e);
return;
}
await this._writeBakFile(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.STATE_BACKUP_FILE,
current.data,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
}
/** Recovers the snapshot from `sync-state.json.bak` (null if unusable). */
@ -1413,20 +1431,14 @@ export class FileBasedSyncAdapterService {
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
): Promise<FileBasedStateFile | null> {
try {
const response = await provider.downloadFile(this._STATE_BACKUP_FILE);
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedStateFile>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION) return null;
return data;
} catch (e) {
OpLog.warn('FileBasedSyncAdapter: state backup recovery failed', e);
return null;
}
const recovered = await this._readBakFile<FileBasedStateFile>(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.STATE_BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
return recovered?.data ?? null;
}
/** True when a downloaded snapshot matches the ops file's snapshotRef. */
@ -1495,9 +1507,10 @@ export class FileBasedSyncAdapterService {
}
/**
* Overwrites `sync-data.json` with a v3 split tombstone (NEVER deletes it) and
* neutralizes `sync-data.json.bak` so an old client's SPAP-8 `.bak` recovery
* cannot resurrect a v2 file and diverge.
* Neutralizes `sync-data.json.bak` and then overwrites `sync-data.json` with a
* v3 split tombstone (NEVER deletes it), so an old client's SPAP-8 `.bak`
* recovery cannot resurrect a v2 file and diverge. Order matters see the
* inline comment.
*/
private async _writeTombstoneAndNeutralizeBak(
provider: FileSyncProvider<SyncProviderId>,
@ -1516,23 +1529,28 @@ export class FileBasedSyncAdapterService {
tombstone,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
// Neutralize the legacy .bak FIRST, then the tombstone. In the old order
// (tombstone first, .bak best-effort) a crash/failure between the two left a
// live v2 .bak next to the tombstone: an OFF client reading the tombstone
// gets SyncDataCorruptedError → SPAP-8 recovery adopts the v2 .bak → its
// next upload heals v2 back OVER the tombstone, forking the folder into two
// sync worlds. Neutralize-first is crash-safe (a crash in between leaves a
// valid v2 primary + tombstone .bak — nothing recoverable, nothing stale)
// and the failure is deliberately FATAL: better a missing tombstone (OFF
// 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.
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);
// Neutralize the legacy .bak (non-fatal): a leftover v2 .bak could otherwise
// let an old client recover + re-upload v2.
try {
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
encoded,
null,
true,
);
} catch (e) {
OpLog.warn(
'FileBasedSyncAdapter: failed to neutralize legacy .bak during migration (non-fatal)',
e,
);
}
}
/**
@ -1804,6 +1822,21 @@ export class FileBasedSyncAdapterService {
snapshotRef,
};
// Backup-before-overwrite (SPAP-8, same as the single-file path): preserve
// the CURRENT remote ops file in .bak so an interrupted/corrupt write of the
// hot file (rewritten on every op-bearing sync — the highest torn-write
// exposure of any sync file) can be recovered on the next download.
if (opsFile) {
await this._writeBakFile(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.OPS_BACKUP_FILE,
opsFile,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
}
// Commit point.
const { finalRev } = await this._uploadOpsFileWithMismatchFallback(
provider,
@ -1815,7 +1848,7 @@ export class FileBasedSyncAdapterService {
this._clearCachedOpsData(providerKey);
this._expectedSyncVersions.set(providerKey, newSyncVersion);
this._lastSeenRevs.set(providerKey, finalRev);
this._commitLastSeenRev(providerKey, finalRev);
this._lastSeenVectorClocks.set(providerKey, mergedClock);
this._persistState();
@ -1879,6 +1912,7 @@ export class FileBasedSyncAdapterService {
let opsFile: FileBasedOpsFile;
let opsRev: string;
let recoveredFromBackup = false;
try {
const r = await this._downloadOpsFile(provider, cfg, encryptKey);
opsFile = r.data;
@ -1890,7 +1924,30 @@ export class FileBasedSyncAdapterService {
// (migration WRITES happen on the upload path).
return this._tryLegacyReadOnlyDownload(provider, cfg, encryptKey, sinceSeq);
}
throw e;
if (this._isRecoverableCorruption(e)) {
// Corrupt/torn sync-ops.json (the hot file, rewritten every op-bearing
// sync): recover from .bak — same SPAP-8 semantics as the single-file
// path, including seeding the heal cache with the CORRUPT primary's rev
// so this cycle's upload conditionally matches and heals it.
const recovered = await this._readBakFile<FileBasedOpsFile>(
provider,
cfg,
encryptKey,
FILE_BASED_SYNC_CONSTANTS.OPS_BACKUP_FILE,
FILE_BASED_SYNC_CONSTANTS.SPLIT_FILE_VERSION,
);
if (!recovered) throw e;
recoveredFromBackup = true;
OpLog.warn(
'FileBasedSyncAdapter: Primary ops file unreadable; recovered from backup',
);
opsFile = recovered.data;
opsRev = (e as { primaryRev?: string } | null)?.primaryRev ?? recovered.rev;
this._setCachedOpsData(providerKey, opsFile, opsRev);
this._notifyRecoveredCorruptPrimaryOnce(providerKey, opsRev);
} else {
throw e;
}
}
// Gap detection on the ops file (mirrors the single-file logic).
@ -1901,7 +1958,12 @@ export class FileBasedSyncAdapterService {
const lastSeenClock = this._lastSeenVectorClocks.get(providerKey);
if (syncVersionRegressed && lastSeenClock) {
const cmp = compareVectorClocks(opsFile.vectorClock, lastSeenClock);
if (cmp === 'EQUAL' || cmp === 'GREATER_THAN') {
// EQUAL only — same rationale as the single-file path above: GREATER_THAN
// proves the writer did strictly more work, NOT that this client received
// the intervening ops. A dominating client's snapshot reset compacts ops
// this client never saw into sync-state.json; suppressing the reset here
// would skip the snapshot hydration and silently diverge.
if (cmp === 'EQUAL') {
versionWasReset = false;
}
}
@ -1924,12 +1986,7 @@ export class FileBasedSyncAdapterService {
this._expectedSyncVersions.set(providerKey, opsFile.syncVersion);
this._lastSeenVectorClocks.set(providerKey, opsFile.vectorClock);
// SPAP-10 crash-safety (review follow-up): stage the ops-file rev as PENDING,
// mirroring the single-file download path. It is promoted to _lastSeenRevs only
// in setLastServerSeq, after the caller has durably applied the ops. Committing
// it eagerly here would let a crash before apply strand the rev ahead of
// un-applied ops, so the next poll's precheck would skip them for good.
this._pendingRevs.set(providerKey, opsRev);
this._stageOrDropDownloadedRev(providerKey, opsRev, recoveredFromBackup);
this._persistState();
const isForceFromZero = sinceSeq === 0;
@ -2052,6 +2109,11 @@ export class FileBasedSyncAdapterService {
clock,
schemaVersion,
);
// sync-state.json.bak is deliberately NOT refreshed here: its adoption is
// ref-validated (_loadValidatedSnapshot requires an EQUAL clock against the
// ops file's snapshotRef), so a stale — even rotated-key — state .bak is
// inert, and it must keep serving the COMPACTION crash window (state
// written, ops not yet) it was backed up for.
const stateRev = await this._writeStateFile(provider, cfg, encryptKey, stateData);
const opsData: FileBasedOpsFile = {
@ -2074,17 +2136,17 @@ export class FileBasedSyncAdapterService {
opsEncoded,
'FileBasedSyncAdapter._uploadSnapshotSplit',
);
const opsRes = await provider.uploadFile(
const opsRes = await this._forceUploadWithBakFirst(
provider,
FILE_BASED_SYNC_CONSTANTS.OPS_FILE,
FILE_BASED_SYNC_CONSTANTS.OPS_BACKUP_FILE,
opsEncoded,
null,
true,
);
await this._writeTombstoneAndNeutralizeBak(provider, cfg, encryptKey);
this._expectedSyncVersions.set(providerKey, newSyncVersion);
this._localSeqCounters.set(providerKey, newSyncVersion);
this._lastSeenRevs.set(providerKey, opsRes.rev);
this._commitLastSeenRev(providerKey, opsRes.rev);
this._lastSeenVectorClocks.set(providerKey, clock);
this._clearCachedOpsData(providerKey);
this._persistState();
@ -2106,53 +2168,190 @@ export class FileBasedSyncAdapterService {
}
/**
* Writes the CURRENT remote content to sync-data.json.bak before the primary
* file is overwritten (two-phase write). We re-encode the already-in-hand
* `currentData` from `_getCurrentSyncState()` rather than issuing another GET.
* Writes `data` to a `.bak` recovery artifact before its primary file is
* overwritten (two-phase write).
*
* MUST be non-fatal: a provider that cannot write the backup (e.g. no copy
* support, transient failure) must still be able to sync. The backup is a
* recovery artifact losing it only degrades recoverability, never sync.
*
* Force-overwrite is used here because .bak is a disposable recovery artifact,
* NOT the source-of-truth sync file the lost-update guarantee on
* sync-data.json is unaffected.
* NOT a source-of-truth sync file the lost-update guarantee on the primary
* (conditional PUT) is unaffected.
*/
private async _backupCurrentRemote(
private async _writeBakFile<T>(
provider: FileSyncProvider<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
currentData: FileBasedSyncData,
bakPath: string,
data: T,
fileVersion: number,
): Promise<void> {
try {
const backupData = await this._encryptAndCompressHandler.compressAndEncryptData(
cfg,
encryptKey,
currentData,
FILE_BASED_SYNC_CONSTANTS.FILE_VERSION,
data,
fileVersion,
);
if (!backupData || backupData.trim().length === 0) {
OpLog.warn(
'FileBasedSyncAdapter: Skipping backup — encoded backup data was empty',
`FileBasedSyncAdapter: Skipping ${bakPath} — encoded backup data was empty`,
);
return;
}
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
backupData,
null,
true,
);
OpLog.normal('FileBasedSyncAdapter: Wrote backup before overwrite');
await provider.uploadFile(bakPath, backupData, null, true);
OpLog.normal(`FileBasedSyncAdapter: Wrote ${bakPath} before overwrite`);
} catch (e) {
// Non-fatal by design — proceed with the primary upload regardless.
OpLog.warn(
'FileBasedSyncAdapter: Backup-before-overwrite failed (non-fatal), proceeding',
`FileBasedSyncAdapter: ${bakPath} write failed (non-fatal), proceeding`,
e,
);
}
}
/**
* Downloads + decodes a `.bak` recovery artifact. Returns null when the backup
* is missing, undecodable, mode-mismatched, or has an unsupported version the
* caller then surfaces its ORIGINAL corruption error.
*/
private async _readBakFile<T extends { version: number }>(
provider: FileSyncProvider<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
bakPath: string,
expectedVersion: number,
): Promise<{ data: T; rev: string } | null> {
try {
const response = await provider.downloadFile(bakPath);
// Security: when encryption is expected, refuse a PLAINTEXT .bak. Decoding
// trusts the file's own prefix flags, so a plaintext .bak decodes even
// under a wrong/rotated key — silently suppressing the wrong-password
// dialog and letting the heal upload clobber the encrypted primary (same
// class as the E2EE-rotation revert).
if (
cfg.isEncrypt &&
!extractSyncFileStateFromPrefix(response.dataStr).isEncrypted
) {
OpLog.warn(
`FileBasedSyncAdapter: ${bakPath} is plaintext but encryption is expected — refusing recovery`,
);
return null;
}
const data = await this._encryptAndCompressHandler.decompressAndDecryptData<T>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== expectedVersion) {
OpLog.warn(
`FileBasedSyncAdapter: ${bakPath} has unsupported version ${data.version}; cannot recover`,
);
return null;
}
return { data, rev: response.rev };
} catch (e) {
OpLog.warn(`FileBasedSyncAdapter: ${bakPath} recovery failed`, e);
return null;
}
}
/**
* Force-writes the SAME payload to the `.bak` FIRST, then the primary. Used by
* snapshot uploads (force-upload / "Use Local" / E2EE re-encryption): a
* snapshot replaces the remote wholesale, so the pre-snapshot .bak must not
* survive it a stale .bak encrypted with a ROTATED-AWAY key would otherwise
* be silently "recovered" by a still-old-key client (suppressing its
* wrong-password prompt) and heal-uploaded back over the new-key primary,
* reverting the rotation. Unlike the op-path backup, the .bak write here is
* deliberately FATAL: leaving the stale artifact behind breaks the snapshot's
* replace-everything contract, and aborting BEFORE the primary write leaves the
* remote fully consistent for a retry. (.bak-first also means a crash between
* the writes leaves a valid old primary + new .bak nothing stale to recover.)
*/
private async _forceUploadWithBakFirst(
provider: FileSyncProvider<SyncProviderId>,
primaryPath: string,
bakPath: string,
encoded: string,
): Promise<{ rev: string }> {
await provider.uploadFile(bakPath, encoded, null, true);
return provider.uploadFile(primaryPath, encoded, null, true);
}
/**
* Commits a freshly-written remote rev as last-seen and drops any rev still
* staged by an earlier download whose apply never completed (e.g. a conflict
* dialog aborted it) promoting a stale staged rev later would clobber the
* fresh one and re-open the SPAP-10 precheck to a wrong "unchanged" answer.
*/
private _commitLastSeenRev(providerKey: string, rev: string): void {
this._lastSeenRevs.set(providerKey, rev);
this._pendingRevs.delete(providerKey);
}
/**
* SPAP-10 (review follow-up): stage a downloaded file's rev as PENDING rather
* than committing it to `_lastSeenRevs`. It is promoted to last-seen (and
* persisted) only once the caller confirms the ops were durably applied, via
* setLastServerSeq the same ordering as the seq cursor. Committing it eagerly
* would let a throw/crash between download and apply strand a rev ahead of
* un-applied ops, so the next poll's cheap pre-check would skip them for good.
*
* EXCEPTION .bak recovery: `rev` is then the CORRUPT primary's rev (kept in
* the sync-cycle cache so this cycle's upload can heal it via a matching
* conditional overwrite). It must NEVER reach `_lastSeenRevs`: promoting it
* would make every later poll's pre-check read "unchanged" and skip the full
* download, while the upload path (which has no .bak recovery) keeps failing
* on the corrupt primary wedging sync until another client rewrites the
* file. Dropping both entries instead forces each poll to re-download,
* re-recover, and re-seed the heal cache until the primary is actually healed.
*/
private _stageOrDropDownloadedRev(
providerKey: string,
rev: string,
recoveredFromBackup: boolean,
): void {
if (recoveredFromBackup) {
this._pendingRevs.delete(providerKey);
this._lastSeenRevs.delete(providerKey);
} else {
this._pendingRevs.set(providerKey, rev);
}
}
/**
* De-dups the user-facing "recovered from backup" notice per corrupt primary
* rev, so a download-only client (which cannot heal the file itself) does not
* re-toast on every sync cycle.
*/
private _notifyRecoveredCorruptPrimaryOnce(
providerKey: string,
corruptRev: string,
): void {
if (this._lastRecoveredCorruptRev.get(providerKey) !== corruptRev) {
this._lastRecoveredCorruptRev.set(providerKey, corruptRev);
// Lazy resolve — absent in some unit harnesses (returns null → no notice).
this._injector
.get(SnackService, null)
?.open({ msg: T.F.SYNC.S.SYNC_DATA_RECOVERED_FROM_BACKUP });
}
}
/**
* Annotates a decode error with the corrupt primary file's rev so the .bak
* recovery path can seed the heal cache with IT (not the .bak rev) and heal
* the primary via a matching conditional overwrite. Returns the error for
* rethrowing.
*/
private _annotatePrimaryRev(e: unknown, rev: string): unknown {
if (e && typeof e === 'object' && !('primaryRev' in e)) {
(e as { primaryRev?: string }).primaryRev = rev;
}
return e;
}
/**
* Whether a download error indicates a corrupt/empty/unparseable primary file
* that we should attempt to recover from the .bak artifact. Missing files are
@ -2168,43 +2367,18 @@ export class FileBasedSyncAdapterService {
// decrypt/decompress stage rather than JSON parse. Without these, .bak
// recovery silently no-ops for exactly the users who enable encryption or
// compression — the interrupted-write case this feature targets. Safe to
// include: if the .bak is also undecodable, _recoverFromBackup returns null
// and the original error still surfaces.
// include: if the .bak is also undecodable, _readBakFile returns null
// and the original error still surfaces. DecryptError is only safe here
// because snapshot uploads refresh .bak with the same payload/key as the
// primary (see _forceUploadWithBakFirst) — a primary/.bak KEY mismatch
// cannot exist on a remote whose snapshots were all written by fixed
// clients. Mixed-fleet caveat: a pre-fix client's snapshot still leaves a
// stale-key .bak behind until its first op-bearing sync refreshes it.
e instanceof DecryptError ||
e instanceof DecompressError
);
}
/**
* Attempts to recover sync data from sync-data.json.bak. Returns null if the
* backup is missing, unreadable, or has an unsupported version.
*/
private async _recoverFromBackup(
provider: FileSyncProvider<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
): Promise<{ data: FileBasedSyncData; rev: string } | null> {
try {
const response = await provider.downloadFile(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE);
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedSyncData>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== FILE_BASED_SYNC_CONSTANTS.FILE_VERSION) {
OpLog.warn(
`FileBasedSyncAdapter: Backup has unsupported version ${data.version}; cannot recover`,
);
return null;
}
return { data, rev: response.rev };
} catch (e) {
OpLog.warn('FileBasedSyncAdapter: Backup recovery failed', e);
return null;
}
}
/**
* Downloads and decrypts the sync file.
*
@ -2280,15 +2454,9 @@ export class FileBasedSyncAdapterService {
if (decodeErr instanceof SplitSyncFormatDetectedError) {
throw decodeErr;
}
// Annotate the corrupt primary file's current rev onto the error so the
// download-time recovery path can seed the cache with IT (not the .bak rev)
// and heal sync-data.json via a matching conditional overwrite, instead of
// mismatching forever and re-recovering every cycle. We already hold the rev
// here (from the download above), so no extra request is needed.
if (decodeErr && typeof decodeErr === 'object' && !('primaryRev' in decodeErr)) {
(decodeErr as { primaryRev?: string }).primaryRev = response.rev;
}
throw decodeErr;
// We already hold the corrupt primary's rev (from the download above), so
// no extra request is needed to enable the heal-via-conditional-overwrite.
throw this._annotatePrimaryRev(decodeErr, response.rev);
}
// SPAP-11 (Q4): a valid v2 sync-data.json can still coexist with a

View file

@ -2924,6 +2924,56 @@ describe('OperationLogSyncService', () => {
expect(conflictError.unsyncedCount).toBe(0); // No unsynced ops
expect(conflictError.remoteSnapshotState).toEqual(remoteSnapshot);
expect(conflictError.remoteVectorClock).toEqual(remoteVectorClock);
// Fresh client has no prior sync — the last-synced baseline must be
// explicitly null (SPAP-7), not undefined or a clock.
expect(conflictError.lastSyncedVectorClock).toBeNull();
}
});
it('passes a null last-synced clock when fresh client with store data receives remote ops (no snapshot)', async () => {
// Covers the second fresh-client throw site: incremental ops without a
// snapshotState. The error must carry an explicit null baseline (SPAP-7).
stateSnapshotServiceSpy.getStateSnapshot.and.returnValue({
task: { ids: ['task-1'] },
project: { ids: [INBOX_PROJECT.id] },
tag: { ids: [TODAY_TAG.id] },
note: { ids: [] },
} as any);
downloadServiceSpy.downloadRemoteOps.and.resolveTo({
newOps: [
{
id: 'remote-op-1',
clientId: 'clientB',
actionType: 'test' as ActionType,
opType: OpType.Update,
entityType: 'TASK',
entityId: 'task-1',
payload: {},
vectorClock: { clientB: 1 },
timestamp: Date.now(),
schemaVersion: 1,
},
],
needsFullStateUpload: false,
success: true,
providerMode: 'superSyncOps',
failedFileCount: 0,
latestServerSeq: 1,
});
const mockProvider = {
supportsOperationSync: true,
} as any;
try {
await service.downloadRemoteOps(mockProvider);
fail('Expected LocalDataConflictError to be thrown');
} catch (error) {
expect(error).toBeInstanceOf(LocalDataConflictError);
const conflictError = error as LocalDataConflictError;
expect(conflictError.unsyncedCount).toBe(0);
expect(conflictError.lastSyncedVectorClock).toBeNull();
}
});