super-productivity/packages/super-sync-server/tests/sync-fixes.spec.ts
Johannes Millan eecf33a4e8 fix(sync): remediate multi-agent review findings
- USE_REMOTE: fix guaranteed self-deadlock — flushPendingWrites() was
  called while already holding the non-reentrant sp_op_log lock its own
  Phase 2 re-acquires, so every "Use Server Data" hung 30s and aborted.
  New OperationWriteFlushService.flushThenRunExclusive owns the bounded
  flush->lock->recheck barrier; ServerMigrationService reuses it, which
  also bounds its previously unbounded snapshot-cutoff retry loop.
- USE_REMOTE: make the rebuild crash-resumable. A raw_rebuild_incomplete
  META marker is set atomically with the baseline replacement and cleared
  after the replay commits; the next sync redoes the raw rebuild instead
  of the normal download (which excludes own ops server-side and would
  silently lose the un-replayed suffix). The resume keeps the first
  attempt's pre-replace backup instead of overwriting the single slot
  with the partial baseline.
- Deferred actions: serialize overlapping drains (two concurrent drains
  could mint two ops for one user intent), stop the drain at the first
  transient failure instead of persisting successors out of order (the
  older edit would win LWW everywhere via clock inversion), abandon
  permanently invalid actions after one attempt (typed
  PermanentDeferredWriteError) instead of retrying + snacking on every
  sync forever, and latch the buffer-size warnings to once per stuck
  window (previously a blocking dev dialog per action past 100); drop
  the no-op 5000 "hard cap" tier.
- writeOperation: post-append bookkeeping failures no longer propagate
  into the deferred retry loop, which re-appended the same action under
  a fresh op id (double-apply of additive payloads).
- remote-apply: full-state cleanup retains every full-state op of the
  current batch, so an archive_pending import can no longer be deleted
  before its archive retry (startup replay would lose a change already
  visible at runtime).
- Hydration: sanitize a malformed stored schemaVersion per-op instead of
  failing the whole boot into recovery; strict parsing (floor now 1,
  matching the server contract) stays on the receive/upload paths.
- Server: request fingerprints are computed lazily — only when a dedup
  entry exists, and otherwise after the rate-limit/pre-quota gates but
  before uploadOps mutates the parsed ops. Fixes the pre-gate hashing of
  full payloads (authenticated DoS-cost regression) and repairs three
  sync-fixes retry tests to model true identical-body retries.
- Delete dead code: failed-op-ids util (+spec) and MAX_VERSION_SKIP (the
  removed forward-compat band); fix the stale clock-merge parity comment.

sync-core 210/210, shared-schema 50/50, super-sync-server 825/825, and
all touched Angular specs pass.
2026-07-10 16:39:37 +02:00

636 lines
22 KiB
TypeScript

/**
* Tests for sync system fixes (Issues 1-6)
*
* These tests verify the fixes implemented for the identified sync issues:
* - Issue 1: Request deduplication with piggybacking
* - Issue 3: Encrypted snapshot uploads
* - Issue 4: Encrypted ops blocking restore endpoint
* - Issue 5: Dedup before quota check
* - Issue 6: Vector-clock EQUAL from different client
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import Fastify, { FastifyInstance } from 'fastify';
import { uuidv7 } from 'uuidv7';
// Store for test data
let testOperations: Map<string, any>;
let serverSeqCounter: number;
let requestCache: Map<string, any>;
// Mock the database module with Prisma mocks
vi.mock('../src/db', () => {
const applySelect = (op: any, select?: Record<string, boolean>) => {
if (!op || !select) {
return op;
}
return Object.fromEntries(
Object.entries(select)
.filter(([, shouldSelect]) => shouldSelect)
.map(([key]) => [key, op[key]]),
);
};
const hasUniqueConflict = (row: any) =>
Array.from(testOperations.values()).some(
(op) =>
op.id === row.id ||
(op.userId === row.userId &&
row.serverSeq !== undefined &&
op.serverSeq === row.serverSeq),
);
return {
prisma: {
$transaction: vi.fn().mockImplementation(async (callback: any) => {
const tx = {
operation: {
create: vi.fn().mockImplementation(async (args: any) => {
serverSeqCounter++;
const op = {
...args.data,
serverSeq: serverSeqCounter,
receivedAt: BigInt(Date.now()),
};
testOperations.set(args.data.id, op);
return op;
}),
createMany: vi.fn().mockImplementation(async (args: any) => {
const rows = Array.isArray(args.data) ? args.data : [args.data];
let count = 0;
for (const row of rows) {
if (hasUniqueConflict(row)) {
if (args.skipDuplicates) {
continue;
}
throw new Error('Unique constraint failed');
}
if (row.serverSeq === undefined) {
serverSeqCounter++;
}
testOperations.set(row.id, {
...row,
serverSeq: row.serverSeq ?? serverSeqCounter,
receivedAt: row.receivedAt ?? BigInt(Date.now()),
});
count++;
}
return { count };
}),
findFirst: vi.fn().mockImplementation(async (args: any) => {
// Find by ID
if (args.where?.id) {
return (
applySelect(testOperations.get(args.where.id), args.select) || null
);
}
// Find full-state operation
if (args.where?.opType?.in) {
for (const op of Array.from(testOperations.values()).reverse()) {
if (
args.where.opType.in.includes(op.opType) &&
args.where.userId === op.userId
) {
return applySelect(op, args.select);
}
}
}
return null;
}),
findMany: vi.fn().mockImplementation(async (args: any) => {
const ops = Array.from(testOperations.values());
return ops
.filter((op) => {
if (args.where?.userId !== undefined && args.where.userId !== op.userId)
return false;
if (
args.where?.serverSeq?.gt !== undefined &&
op.serverSeq <= args.where.serverSeq.gt
)
return false;
if (
args.where?.clientId?.not &&
op.clientId === args.where.clientId.not
)
return false;
return true;
})
.sort((a, b) => a.serverSeq - b.serverSeq)
.slice(0, args.take || 500);
}),
count: vi.fn().mockImplementation(async (args: any) => {
const ops = Array.from(testOperations.values());
return ops.filter((op) => {
if (args.where?.userId !== undefined && args.where.userId !== op.userId)
return false;
if (
args.where?.serverSeq?.gt !== undefined &&
op.serverSeq <= args.where.serverSeq.gt
)
return false;
if (
args.where?.serverSeq?.lte !== undefined &&
op.serverSeq > args.where.serverSeq.lte
)
return false;
if (
args.where?.isPayloadEncrypted !== undefined &&
op.isPayloadEncrypted !== args.where.isPayloadEncrypted
)
return false;
return true;
}).length;
}),
aggregate: vi.fn().mockResolvedValue({ _min: { serverSeq: 1 } }),
findUnique: vi.fn().mockImplementation(async (args: any) => {
if (args.where?.id) {
return (
applySelect(testOperations.get(args.where.id), args.select) || null
);
}
return null;
}),
},
userSyncState: {
findUnique: vi.fn().mockResolvedValue({ lastSeq: serverSeqCounter }),
upsert: vi.fn().mockResolvedValue({}),
update: vi.fn().mockImplementation(async (args: any) => {
if (args.data?.lastSeq?.increment !== undefined) {
serverSeqCounter += args.data.lastSeq.increment;
} else if (args.data?.lastSeq?.decrement !== undefined) {
serverSeqCounter -= args.data.lastSeq.decrement;
}
return { lastSeq: serverSeqCounter };
}),
},
syncDevice: {
upsert: vi.fn().mockResolvedValue({}),
count: vi.fn().mockResolvedValue(1),
},
$queryRaw: vi.fn().mockResolvedValue([]),
// Upload transaction writes the storage counter atomically via $executeRaw.
$executeRaw: vi.fn().mockResolvedValue(0),
};
return callback(tx);
}),
operation: {
findFirst: vi.fn(),
findMany: vi.fn().mockImplementation(async (args: any) => {
const ops = Array.from(testOperations.values());
return ops
.filter((op) => {
if (args.where?.userId !== undefined && args.where.userId !== op.userId)
return false;
if (
args.where?.serverSeq?.gt !== undefined &&
op.serverSeq <= args.where.serverSeq.gt
)
return false;
if (args.where?.clientId?.not && op.clientId === args.where.clientId.not)
return false;
return true;
})
.sort((a, b) => a.serverSeq - b.serverSeq)
.slice(0, args.take || 500);
}),
aggregate: vi.fn().mockResolvedValue({ _min: { serverSeq: 1 } }),
findUnique: vi.fn().mockImplementation(async (args: any) => {
if (args.where?.id) {
return applySelect(testOperations.get(args.where.id), args.select) || null;
}
return null;
}),
},
userSyncState: {
findUnique: vi.fn().mockImplementation(async () => ({
lastSeq: serverSeqCounter,
})),
upsert: vi.fn().mockResolvedValue({}),
update: vi.fn().mockResolvedValue({}),
updateMany: vi.fn().mockResolvedValue({ count: 1 }),
create: vi.fn().mockResolvedValue({}),
findMany: vi.fn().mockResolvedValue([]),
},
syncDevice: {
upsert: vi.fn().mockResolvedValue({}),
count: vi.fn().mockResolvedValue(1),
deleteMany: vi.fn().mockResolvedValue({ count: 0 }),
},
user: {
findUnique: vi.fn().mockResolvedValue({
id: 1,
storageQuotaBytes: BigInt(100 * 1024 * 1024),
storageUsedBytes: BigInt(0),
}),
update: vi.fn().mockResolvedValue({}),
},
$queryRaw: vi.fn().mockResolvedValue([{ total: BigInt(0) }]),
},
};
});
// Mock auth
vi.mock('../src/auth', () => ({
verifyToken: vi
.fn()
.mockResolvedValue({ valid: true, userId: 1, email: 'test@test.com' }),
}));
// Import after mocking
import { syncRoutes } from '../src/sync/sync.routes';
import { initSyncService, getSyncService } from '../src/sync/sync.service';
// Helper to create operation
const createOp = (
clientId: string,
overrides: Partial<{
id: string;
actionType: string;
opType: string;
entityType: string;
entityId: string;
payload: unknown;
vectorClock: Record<string, number>;
timestamp: number;
schemaVersion: number;
}> = {},
) => ({
id: uuidv7(),
clientId,
actionType: 'ADD_TASK',
opType: 'CRT',
entityType: 'TASK',
entityId: 'task-1',
payload: { title: 'Test Task' },
vectorClock: {},
timestamp: Date.now(),
schemaVersion: 1,
...overrides,
});
describe('Sync System Fixes', () => {
let app: FastifyInstance;
const authToken = 'mock-token';
beforeEach(async () => {
// Reset test state
testOperations = new Map();
serverSeqCounter = 0;
requestCache = new Map();
vi.clearAllMocks();
// Initialize service
initSyncService();
// Create Fastify app
app = Fastify();
await app.register(syncRoutes, { prefix: '/api/sync' });
await app.ready();
});
afterEach(async () => {
await app.close();
});
// =============================================================================
// Issue 1: Request deduplication with piggybacking
// =============================================================================
describe('Issue 1: Request deduplication with piggybacking', () => {
it('should return fresh newOps on deduplicated request', async () => {
const clientA = 'client-a';
const clientB = 'client-b';
const requestId = 'req-123';
// A true retry resends the IDENTICAL body — the request fingerprint
// treats a same-requestId/different-body request as a cache miss.
const opA = createOp(clientA, { entityId: 'task-a' });
// Step 1: Client A uploads with requestId
const firstResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [opA],
clientId: clientA,
lastKnownServerSeq: 0,
requestId,
},
});
expect(firstResponse.statusCode).toBe(200);
const firstBody = firstResponse.json();
expect(firstBody.results[0].accepted).toBe(true);
const clientASeq = firstBody.latestSeq;
// Step 2: Client B uploads ops
const clientBResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [createOp(clientB, { entityId: 'task-b' })],
clientId: clientB,
},
});
expect(clientBResponse.statusCode).toBe(200);
// Step 3: Client A retries with same requestId but updated lastKnownServerSeq
const retryResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [opA],
clientId: clientA,
lastKnownServerSeq: clientASeq,
requestId,
},
});
expect(retryResponse.statusCode).toBe(200);
const retryBody = retryResponse.json();
// Should be deduplicated
expect(retryBody.deduplicated).toBe(true);
// CRITICAL: Should have piggybacked ops from client B
expect(retryBody.newOps).toBeDefined();
expect(retryBody.newOps.length).toBeGreaterThan(0);
expect(
retryBody.newOps.some(
(op: { op: { clientId: string } }) => op.op.clientId === clientB,
),
).toBe(true);
});
it('should use retry request lastKnownServerSeq, not original', async () => {
const clientA = 'client-a';
const clientB = 'client-b';
const requestId = 'req-456';
const opA = createOp(clientA, { entityId: 'task-1' });
// Client A uploads
await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [opA],
clientId: clientA,
lastKnownServerSeq: 0,
requestId,
},
});
// Client B uploads
await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [createOp(clientB, { entityId: 'task-2' })],
clientId: clientB,
},
});
// Client A retries with lastKnownServerSeq=0
const retryResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [opA],
clientId: clientA,
lastKnownServerSeq: 0,
requestId,
},
});
const body = retryResponse.json();
expect(body.deduplicated).toBe(true);
// Should get client B's ops since lastKnownServerSeq=0
expect(body.newOps?.length).toBeGreaterThan(0);
});
});
// =============================================================================
// Issue 3: Encrypted snapshot uploads
// =============================================================================
describe('Issue 3: Encrypted snapshot uploads', () => {
it('should store isPayloadEncrypted flag from snapshot upload', async () => {
const snapshotResponse = await app.inject({
method: 'POST',
url: '/api/sync/snapshot',
headers: { authorization: `Bearer ${authToken}` },
payload: {
state: 'encrypted-string-here',
clientId: 'test-client',
reason: 'recovery',
vectorClock: { 'test-client': 1 },
schemaVersion: 1,
isPayloadEncrypted: true,
},
});
expect(snapshotResponse.statusCode).toBe(200);
const snapshotBody = snapshotResponse.json();
expect(snapshotBody.accepted).toBe(true);
// Download ops and verify the SYNC_IMPORT has isPayloadEncrypted
const downloadResponse = await app.inject({
method: 'GET',
url: '/api/sync/ops?sinceSeq=0',
headers: { authorization: `Bearer ${authToken}` },
});
const downloadBody = downloadResponse.json();
const syncImportOp = downloadBody.ops.find(
(op: { op: { opType: string } }) => op.op.opType === 'SYNC_IMPORT',
);
expect(syncImportOp).toBeDefined();
expect(syncImportOp.op.isPayloadEncrypted).toBe(true);
});
it('should default isPayloadEncrypted to false when not provided', async () => {
const snapshotResponse = await app.inject({
method: 'POST',
url: '/api/sync/snapshot',
headers: { authorization: `Bearer ${authToken}` },
payload: {
state: { tasks: [] },
clientId: 'test-client',
reason: 'initial',
vectorClock: { 'test-client': 1 },
schemaVersion: 1,
},
});
expect(snapshotResponse.statusCode).toBe(200);
// Download and verify default
const downloadResponse = await app.inject({
method: 'GET',
url: '/api/sync/ops?sinceSeq=0',
headers: { authorization: `Bearer ${authToken}` },
});
const downloadBody = downloadResponse.json();
const syncImportOp = downloadBody.ops.find(
(op: { op: { opType: string } }) => op.op.opType === 'SYNC_IMPORT',
);
expect(syncImportOp.op.isPayloadEncrypted).toBe(false);
});
});
// =============================================================================
// Issue 5: Dedup before quota check
// Tests that deduplication happens before quota check to allow retries
// =============================================================================
describe('Issue 5: Dedup before quota check', () => {
it('should return cached results even when quota would be exceeded on retry', async () => {
const clientId = uuidv7();
const requestId = uuidv7();
const op = createOp(clientId, { entityId: 'task-1' });
// First request with requestId
const firstResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [op],
clientId,
lastKnownServerSeq: 0,
requestId,
},
});
expect(firstResponse.statusCode).toBe(200);
const firstBody = firstResponse.json();
expect(firstBody.results[0].accepted).toBe(true);
// Second request with same requestId (retry) should return cached results
// even if the real quota check would fail (because dedup runs first)
const retryResponse = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [op],
clientId,
lastKnownServerSeq: 0,
requestId,
},
});
expect(retryResponse.statusCode).toBe(200);
const retryBody = retryResponse.json();
expect(retryBody.deduplicated).toBe(true);
expect(retryBody.results[0].accepted).toBe(true);
});
});
// =============================================================================
// Issue 7: serverTime in download response
// =============================================================================
describe('Issue 7: serverTime in download response', () => {
it('should include serverTime in download response for clock drift detection', async () => {
// First upload an operation
await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [createOp('test-client', { entityId: 'task-1' })],
clientId: 'test-client',
},
});
const beforeTime = Date.now();
const downloadResponse = await app.inject({
method: 'GET',
url: '/api/sync/ops?sinceSeq=0',
headers: { authorization: `Bearer ${authToken}` },
});
const afterTime = Date.now();
expect(downloadResponse.statusCode).toBe(200);
const body = downloadResponse.json();
// serverTime should be present and within expected range
expect(body.serverTime).toBeDefined();
expect(typeof body.serverTime).toBe('number');
expect(body.serverTime).toBeGreaterThanOrEqual(beforeTime);
expect(body.serverTime).toBeLessThanOrEqual(afterTime);
});
});
// =============================================================================
// Issue 8: Cleanup should return affected userIds for storage update
// =============================================================================
describe('Issue 8: deleteOldSyncedOpsForAllUsers returns affectedUserIds', () => {
it('should return totalDeleted and affectedUserIds structure', async () => {
const syncService = getSyncService();
const cutoffTime = Date.now() - 50 * 24 * 60 * 60 * 1000; // 50 days ago
// Call the method - with default mock it returns empty since no users have snapshots
const result = await syncService.deleteOldSyncedOpsForAllUsers(cutoffTime);
// Verify return structure
expect(result).toHaveProperty('totalDeleted');
expect(result).toHaveProperty('affectedUserIds');
expect(typeof result.totalDeleted).toBe('number');
expect(Array.isArray(result.affectedUserIds)).toBe(true);
});
it('should return empty affectedUserIds when no operations deleted', async () => {
const syncService = getSyncService();
const cutoffTime = Date.now() - 50 * 24 * 60 * 60 * 1000;
const result = await syncService.deleteOldSyncedOpsForAllUsers(cutoffTime);
expect(result.totalDeleted).toBe(0);
expect(result.affectedUserIds).toHaveLength(0);
});
});
// =============================================================================
// Issue 6: Vector-clock EQUAL from different client
// =============================================================================
describe('Issue 6: Vector-clock EQUAL from different client', () => {
it('should allow operations when no prior operation exists', async () => {
const clientA = uuidv7();
const entityId = 'task-' + uuidv7();
// Client A creates entity - should succeed since no prior op exists
const response1 = await app.inject({
method: 'POST',
url: '/api/sync/ops',
headers: { authorization: `Bearer ${authToken}` },
payload: {
ops: [
{
id: uuidv7(),
clientId: clientA,
actionType: 'AddTask',
opType: 'CRT',
entityType: 'TASK',
entityId,
payload: { title: 'Task' },
vectorClock: { [clientA]: 1 },
timestamp: Date.now(),
schemaVersion: 1,
},
],
clientId: clientA,
lastKnownServerSeq: 0,
},
});
expect(response1.statusCode).toBe(200);
expect(response1.json().results[0].accepted).toBe(true);
});
});
});