From fbefaa3437e19550fd58cb7058a7f2a87d59329f Mon Sep 17 00:00:00 2001 From: johannesjo Date: Wed, 13 May 2026 00:50:17 +0200 Subject: [PATCH] fix(supersync): port requestId idempotency missed in slice rebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master shipped commit 30da81ea5 ("fix supersync retry idempotency") after this slice's worktree was created from 88961485f, so the package class was missing the deterministic `requestId` on every `uploadOps` payload. Without it the SuperSync server can't recognize a retried upload (e.g. after a network drop between server commit and client receipt) and rejects the replay as duplicate ops instead of returning the cached response/piggybacked ops. Port the four private helpers (`_createOpsUploadRequestId`, `_compactRequestIdPart`, `_hashRequestIdInput`, `_stableJsonStringify` + `_toStableJsonValue`) and the `OPS_UPLOAD_REQUEST_ID_PREFIX` constant into the package class, and add `requestId` to the `uploadOps` JSON payload. All helpers are pure functions with no external deps — no new ports required. Port master's Vitest spec: assert the prefix/length on the existing "uploads operations successfully" test, and add a dedicated "stable requestId across retries and key-reordering" test that exercises five batches and asserts the equivalence/divergence classes (retry → same id, key reorder → same id, content change → different id, batch resize → different id). Bundle: CJS 97.79 KB / ESM 94.99 KB (+2.6 KB from the helpers). Package spec count: 276 (was 275). Caught by codex during post-slice review. --- .../src/super-sync/super-sync.ts | 75 +++++++++++++++++++ .../tests/super-sync/super-sync.spec.ts | 59 +++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/packages/sync-providers/src/super-sync/super-sync.ts b/packages/sync-providers/src/super-sync/super-sync.ts index d74e134bb2..91a58bf03a 100644 --- a/packages/sync-providers/src/super-sync/super-sync.ts +++ b/packages/sync-providers/src/super-sync/super-sync.ts @@ -30,6 +30,9 @@ import { const LAST_SERVER_SEQ_KEY_PREFIX = 'super_sync_last_server_seq_'; +/** Versioned prefix for the deterministic ops-upload `requestId`. */ +const OPS_UPLOAD_REQUEST_ID_PREFIX = 'ops-v1'; + /** 75s allows the server's 60s database timeout plus network/parse buffer. */ const SUPERSYNC_REQUEST_TIMEOUT_MS = 75000; @@ -121,6 +124,77 @@ export class SuperSyncProvider } } + /** + * Deterministic upload-batch identifier for server-side idempotency. + * Lets the SuperSync server recognize a retried upload (e.g. after a + * network drop between server commit and client receipt) and return + * the cached result instead of rejecting as duplicate ops. + * + * The id is derived from `clientId` + a stable hash of the ops + * batch. Reordering nested object keys or repeating the exact same + * batch yields the same id; changing any op's payload (or batch + * size) yields a different id. + */ + private _createOpsUploadRequestId(ops: SyncOperation[], clientId: string): string { + const opIds = ops.map((op) => op.id).join('|'); + let opsFingerprint = opIds; + try { + opsFingerprint = this._stableJsonStringify(ops); + } catch { + opsFingerprint = opIds; + } + const firstOpId = this._compactRequestIdPart(ops[0]?.id ?? 'empty'); + const lastOp = ops.length > 0 ? ops[ops.length - 1] : undefined; + const lastOpId = this._compactRequestIdPart(lastOp?.id ?? 'empty'); + const hash = this._hashRequestIdInput(`${clientId}|${opsFingerprint}`); + return `${OPS_UPLOAD_REQUEST_ID_PREFIX}-${ops.length}-${firstOpId}-${lastOpId}-${hash}`; + } + + private _compactRequestIdPart(id: string): string { + return id.replace(/[^a-zA-Z0-9_-]/g, '').slice(0, 8) || 'x'; + } + + /** + * Two-way FNV-1a-like hash (32-bit FNV + a second mixing pass) so + * the resulting hex is wide enough for batch identification. Pure + * function, no Web Crypto dependency. + */ + private _hashRequestIdInput(input: string): string { + let hashA = 0x811c9dc5; + let hashB = 0x9e3779b9; + + for (let i = 0; i < input.length; i++) { + const code = input.charCodeAt(i); + hashA = Math.imul(hashA ^ code, 16777619); + hashB = Math.imul(hashB + code, 2246822519) ^ (hashB >>> 13); + } + + return `${(hashA >>> 0).toString(36)}${(hashB >>> 0).toString(36)}`; + } + + private _stableJsonStringify(value: unknown): string { + return JSON.stringify(this._toStableJsonValue(value)) ?? 'undefined'; + } + + private _toStableJsonValue(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map((item) => this._toStableJsonValue(item)); + } + + if (value !== null && typeof value === 'object') { + return Object.fromEntries( + Object.keys(value as Record) + .sort() + .map((key) => [ + key, + this._toStableJsonValue((value as Record)[key]), + ]), + ); + } + + return value; + } + // === Operation Sync Implementation === async uploadOps( @@ -138,6 +212,7 @@ export class SuperSyncProvider ops, clientId, lastKnownServerSeq, + requestId: this._createOpsUploadRequestId(ops, clientId), }); if (this.isNativePlatform) { diff --git a/packages/sync-providers/tests/super-sync/super-sync.spec.ts b/packages/sync-providers/tests/super-sync/super-sync.spec.ts index 128eeb2def..0a27f06586 100644 --- a/packages/sync-providers/tests/super-sync/super-sync.spec.ts +++ b/packages/sync-providers/tests/super-sync/super-sync.spec.ts @@ -409,6 +409,65 @@ describe('SuperSyncProvider', () => { const body = JSON.parse(bodyJson); expect(body.ops).toEqual(ops); expect(body.clientId).toBe('client-1'); + expect(body.requestId).toMatch(/^ops-v1-/); + expect(body.requestId.length).toBeLessThanOrEqual(64); + }); + + it('uses a stable requestId across retries and key-reordering, distinct for content/batch changes', async () => { + const { provider, cfgStore, fetchMock } = buildProvider(); + cfgStore.load.mockResolvedValue(testConfig); + fetchMock.mockResolvedValue(okResponse({ results: [], latestSeq: 5 })); + + const firstOp = createMockOperation({ + id: 'op-123', + payload: { top: 'value', nested: { a: 1, b: 2 } }, + vectorClock: { client1: 1, client2: 2 }, + }); + const secondOp = createMockOperation({ id: 'op-456', entityId: 'task-2' }); + const ops = [firstOp, secondOp]; + + // Call 0: original batch + await provider.uploadOps(ops, 'client-1', 2); + // Call 1: same batch, retried with different lastKnownServerSeq + await provider.uploadOps(ops, 'client-1', 4); + // Call 2: same batch with nested keys reordered (stable JSON) + await provider.uploadOps( + [ + { + ...firstOp, + payload: { nested: { b: 2, a: 1 }, top: 'value' }, + vectorClock: { client2: 2, client1: 1 }, + }, + secondOp, + ], + 'client-1', + 4, + ); + // Call 3: same batch but first op's payload content changed + await provider.uploadOps( + [{ ...firstOp, payload: { title: 'Changed Task' } }, secondOp], + 'client-1', + 4, + ); + // Call 4: extended batch (extra op appended) + await provider.uploadOps( + [...ops, createMockOperation({ id: 'op-789', entityId: 'task-3' })], + 'client-1', + 4, + ); + + const bodies = await Promise.all( + [0, 1, 2, 3, 4].map(async (i) => { + const [, options] = fetchMock.mock.calls[i] as [string, RequestInit]; + return JSON.parse(await decompressGzip(options.body as Blob)); + }), + ); + + expect(bodies[1].requestId).toBe(bodies[0].requestId); + expect(bodies[2].requestId).toBe(bodies[0].requestId); + expect(bodies[3].requestId).not.toBe(bodies[0].requestId); + expect(bodies[4].requestId).not.toBe(bodies[0].requestId); + expect(bodies[0].requestId.length).toBeLessThanOrEqual(64); }); it('includes lastKnownServerSeq when provided', async () => {