super-productivity/e2e/tests/sync/webdav-surgical-sync.spec.ts
Johannes Millan b50d5f6d96
fix(sync): dedupe surgical-sync retries and keep the import author through pruning (#9089)
* fix(sync): deduplicate surgical sync retries

Persist split-file configuration and acknowledge operation IDs already committed remotely after a lost upload response.

* fix(sync): preserve import author during clock pruning

Keep the active causal full-state author in oversized stored clocks and reuse the stored protected IDs when classifying response-loss retries.

* test(sync): harden supersync failure coverage

Exercise real upload endpoints and exact operation IDs across response loss, validation failures, schema blockers, full-state boundaries, vector pruning, and concurrent edits.

* fix(sync): heal a corrupt primary on duplicate-only uploads

The .bak recovery path caches the CORRUPT primary's rev precisely so this
cycle's conditional overwrite repairs sync-ops.json. The duplicate-retry
short-circuit read that cache and returned before the write, leaving the
primary corrupt whenever the recovered buffer already held every pending
op. Flag the recovered entry and let those uploads fall through.

Also stop synthesising a serverSeq for ops already in the buffer: the
field is optional, the upload and download paths number ops differently,
and a mixed batch could hand two ops the same value.

* perf(sync): look the full-state author up once per upload

Batch upload is off by default, so the guarded batch path was not the one
serving production: the serial path queries the causal full-state author
per op inside a single transaction, and a clock of 21-50 entries passes
validation and trips the guard on every one of them. Memoize the author
per transaction and resolve it lazily, so only an op whose clock actually
overflows pays, and only once. This also retires the batch pre-scan and
its loop-carried author, leaving both paths on one mechanism.

Report the lookup through ProcessOperationResult so the upload summary
stops under-reporting round-trips, and record why reconstructing the
stored protected set loosens id-collision detection.

* test(sync): wait for the committed title in renameTask

renameTask blurred the textarea, slept 300ms and returned without ever
checking the rename landed. Blur -> dispatch -> re-render outruns that
delay on a loaded machine, so a following sync uploads without the rename
op and the caller asserts against a task that was never renamed — which
is what supersync 3.1 hits on CI but never locally.

Wait for the new title instead, mirroring markTaskDone's done-state wait
and the e2e no-waitForTimeout rule.

* test(sync): dispatch focus so renameTask actually commits

renameTask relied on el.focus() to emit a focus event, but these tests
drive two clients as separate pages and only one page can hold focus, so
on CI the event often never fires. TaskTitleComponent then keeps
_isFocused=false, and resetToLastExternalValueTrigger resets tmpValue to
the stored title on the next task-object emission. Blur therefore
computes wasChanged=false, task.component skips update(), and the rename
is silently dropped without ever becoming an op.

That is supersync 3.1: client A's rename lives only in tmpValue, A syncs
and uploads nothing, B uploads its done op, A downloads it, the task ref
changes and the title reverts to the original — exactly the state the CI
artifact captured. A real user always has real focus, so the app itself
is unaffected.

Dispatch focus explicitly, mirroring the synthetic input/blur already
used here. Also correct the previous commit's claim: the toBeVisible
wait matches tmpValue, a component-local signal rendered in both
template branches, so it never observed the committed title and could
not have fixed this.

* docs: revert incidental prettier reformat of unrelated docs

The master merge ran prettier across files it pulled in, reformatting
three documents this branch has no business touching: markdown table
cell padding plus *emphasis* -> _emphasis_, with no content change.
handover.md documents two unrelated branches entirely.

Restores them to master. vector-clocks.md keeps its edits — those are
this branch's own and describe the pruning protection.

* refactor(sync): drop the full-state author lookup's roundtrip accounting

resolveFullStateAuthor memoizes per transaction, so the lookup it counts
fires at most once per upload — the plumbing existed to report a number
that is always 0 or 1, on a log line already counting dozens. The memo
and its own accounting cancelled out.

Removing didQuery lets resolveFullStateAuthor return string | undefined
and getPruneProtectedIds return string[], instead of both carrying a
tuple purely to feed the counter.

uploadDbRoundtrips and the batch path's own counter are untouched.

* test(sync): assert the committed store title in renameTask

The focus dispatch did not fix supersync 3.1 — the shard failed again
with an identical snapshot (original title, done, rename gone), so that
diagnosis was wrong. Stop guessing at the trigger and make the helper
able to observe the thing in question.

task-title renders tmpValue, a component-local signal, in BOTH its
editing and idle branches. Every DOM assertion here therefore matches as
soon as the synthetic input event fires, whether or not an op was ever
captured — which is why two rounds of "wait for the title" changed
nothing. Read the store instead, via the __e2eTestHelpers.store hook the
timeSpent helper already uses.

This is a diagnostic as much as a fix: it splits the two remaining
explanations. If renameTask now fails, the rename never becomes an op
and the bug is in how the test drives the edit. If it passes and 3.1
still fails at the merge assertion, the op is captured and lost during
sync — a real defect, and the test is right to fail.

* test(sync): move the 3.1 disjoint-merge rewrite out to #9095

3.1 was the last red shard, and it turned out to be right: the
store-backed renameTask passes, so the rename IS captured as an op, and
the test still fails at the merge assertion — the op is committed and
then lost during sync. Filed as #9095.

That bug is pre-existing and cannot be reached by anything in this PR:
the file-based adapter is not used by SuperSync, and the server-side
author memo only engages for clocks over 20 entries where this test
carries about three. 3.1 is also the only test here that exercises
neither of this PR's fixes — it races a title change against a
move-to-done, which is conflict resolution, not retry dedup or clock
pruning. So it moves to #9095 rather than holding verified sync fixes
red.

The rest of the hardening stays: the fault injections whose globs never
matched a real endpoint, the schema-mismatch test that asserted nothing,
and the compaction suite that called an endpoint which never existed are
what actually cover the fixes here.

Restoring the old 3.1 puts a misleading test back, so it now carries a
comment saying why it proves little and where the real one lives. The
strengthened version is kept on test/issue-9095-disjoint-merge-repro.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-07-17 00:46:01 +02:00

231 lines
8.5 KiB
TypeScript

import { test, expect } from '../../fixtures/webdav.fixture';
import type { APIRequestContext, Page } from '@playwright/test';
import { SyncPage } from '../../pages/sync.page';
import { WorkViewPage } from '../../pages/work-view.page';
import {
closeContextsSafely,
createSyncFolder,
generateSyncFolderName,
setupSyncClient,
waitForSyncComplete,
WEBDAV_CONFIG_TEMPLATE,
} from '../../utils/sync-helpers';
import { waitForAppReady, waitForStatePersistence } from '../../utils/waits';
interface SurgicalOpsFile {
recentOps: Array<{ id: string }>;
}
const readSurgicalOpsFile = async (
request: APIRequestContext,
url: string,
authorization: string,
): Promise<SurgicalOpsFile> => {
const response = await request.get(url, {
headers: { Authorization: authorization },
});
expect(response.ok()).toBe(true);
const encoded = await response.text();
const prefixEnd = encoded.indexOf('__');
if (prefixEnd < 0) {
throw new Error('Surgical ops file is missing its format prefix');
}
return JSON.parse(encoded.slice(prefixEnd + 2)) as SurgicalOpsFile;
};
const getLocalOperationState = async (
page: Page,
operationId: string,
): Promise<{ syncedAt?: number; rejectedAt?: number } | undefined> =>
page.evaluate(async (id) => {
const db = await new Promise<IDBDatabase>((resolve, reject) => {
const openRequest = indexedDB.open('SUP_OPS');
openRequest.onsuccess = (): void => resolve(openRequest.result);
openRequest.onerror = (): void => reject(openRequest.error);
});
try {
return await new Promise<{ syncedAt?: number; rejectedAt?: number } | undefined>(
(resolve, reject) => {
const tx = db.transaction('ops', 'readonly');
const getRequest = tx.objectStore('ops').index('byId').get(id);
getRequest.onsuccess = (): void =>
resolve(
getRequest.result as { syncedAt?: number; rejectedAt?: number } | undefined,
);
getRequest.onerror = (): void => reject(getRequest.error);
},
);
} finally {
db.close();
}
}, operationId);
test.describe('@webdav @surgical WebDAV Surgical sync', () => {
test('survives a committed ops response loss and restart', async ({
browser,
baseURL,
request,
}) => {
test.slow();
const appUrl = baseURL || 'http://localhost:4242';
const folderName = generateSyncFolderName('e2e-surgical');
const folderUrl = `${WEBDAV_CONFIG_TEMPLATE.baseUrl}${folderName}/DEV/`;
const config = {
...WEBDAV_CONFIG_TEMPLATE,
syncFolderPath: `/${folderName}`,
isUseSplitSyncFiles: true,
};
const authorization =
'Basic ' +
Buffer.from(
`${WEBDAV_CONFIG_TEMPLATE.username}:${WEBDAV_CONFIG_TEMPLATE.password}`,
).toString('base64');
await createSyncFolder(request, folderName);
let clientA: Awaited<ReturnType<typeof setupSyncClient>> | null = null;
let clientB: Awaited<ReturnType<typeof setupSyncClient>> | null = null;
try {
clientA = await setupSyncClient(browser, appUrl);
clientB = await setupSyncClient(browser, appUrl);
const syncA = new SyncPage(clientA.page);
const syncB = new SyncPage(clientB.page);
const workViewA = new WorkViewPage(clientA.page);
const workViewB = new WorkViewPage(clientB.page);
await syncA.setupWebdavSync(config);
const taskA = `Surgical-A-${folderName}`;
await workViewA.addTask(taskA);
await waitForStatePersistence(clientA.page);
await syncA.triggerSync();
await waitForSyncComplete(clientA.page, syncA);
const opsFile = await request.get(`${folderUrl}sync-ops.json`, {
headers: { Authorization: authorization },
});
const stateFile = await request.get(`${folderUrl}sync-state.json`, {
headers: { Authorization: authorization },
});
expect(opsFile.ok()).toBe(true);
expect(stateFile.ok()).toBe(true);
await syncB.setupWebdavSync(config);
await syncB.triggerSync();
await waitForSyncComplete(clientB.page, syncB);
await expect(clientB.page.locator('task', { hasText: taskA })).toBeVisible();
const baselineOps = await readSurgicalOpsFile(
request,
`${folderUrl}sync-ops.json`,
authorization,
);
const baselineOpIds = new Set(
baselineOps.recentOps.map((operation) => operation.id),
);
let requestPhase: 'fault' | 'restart' = 'fault';
const faultRequests: string[] = [];
const restartRequests: string[] = [];
let committedOpsWrite = false;
let responseDropped = false;
let restartOpsWrites = 0;
const recordWebDavRequest = (url: string): void => {
if (url.startsWith(folderUrl)) {
const requests = requestPhase === 'fault' ? faultRequests : restartRequests;
requests.push(new URL(url).pathname);
}
};
const requestListener = (webDavRequest: { url(): string }): void =>
recordWebDavRequest(webDavRequest.url());
clientB.page.on('request', requestListener);
await clientB.page.route('**/sync-ops.json', async (route) => {
if (route.request().method() === 'PUT') {
if (requestPhase === 'fault') {
if (!committedOpsWrite) {
const response = await route.fetch();
expect(response.ok()).toBe(true);
committedOpsWrite = true;
}
await route.abort('failed');
responseDropped = true;
return;
}
restartOpsWrites++;
}
await route.continue();
});
const taskB = `Surgical-B-${folderName}`;
await workViewB.addTask(taskB);
await waitForStatePersistence(clientB.page);
await syncB.triggerSync();
await expect.poll(() => responseDropped).toBe(true);
await syncB.syncSpinner.waitFor({ state: 'hidden', timeout: 20000 });
expect(committedOpsWrite).toBe(true);
expect(faultRequests.some((path) => path.endsWith('/sync-ops.json'))).toBe(true);
expect(faultRequests.some((path) => path.includes('/sync-state'))).toBe(false);
const committedOps = await readSurgicalOpsFile(
request,
`${folderUrl}sync-ops.json`,
authorization,
);
const newlyCommittedIds = committedOps.recentOps
.map((operation) => operation.id)
.filter((id) => !baselineOpIds.has(id));
expect(newlyCommittedIds).toHaveLength(1);
const committedOperationId = newlyCommittedIds[0];
expect(
(await getLocalOperationState(clientB.page, committedOperationId))?.syncedAt,
'the lost response must leave the committed operation pending',
).toBeUndefined();
// The ops PUT committed remotely, but its response never reached the
// client. Reload before retrying to exercise persisted cursor/revision
// recovery rather than only an in-memory retry.
requestPhase = 'restart';
await clientB.page.reload({ waitUntil: 'domcontentloaded' });
await waitForAppReady(clientB.page);
await syncB.triggerSync();
await waitForSyncComplete(clientB.page, syncB);
await expect
.poll(
async () =>
(await getLocalOperationState(clientB.page, committedOperationId))?.syncedAt,
)
.not.toBeUndefined();
await expect(clientB.page.locator('task', { hasText: taskB })).toHaveCount(1);
expect(restartRequests.some((path) => path.endsWith('/sync-ops.json'))).toBe(true);
expect(restartRequests.some((path) => path.includes('/sync-state'))).toBe(false);
expect(restartRequests.some((path) => path.endsWith('/sync-data.json'))).toBe(
false,
);
expect(restartOpsWrites).toBe(0);
const recoveredOps = await readSurgicalOpsFile(
request,
`${folderUrl}sync-ops.json`,
authorization,
);
expect(
recoveredOps.recentOps.filter(
(operation) => operation.id === committedOperationId,
),
).toHaveLength(1);
await syncA.triggerSync();
await waitForSyncComplete(clientA.page, syncA);
await expect(clientA.page.locator('task', { hasText: taskB })).toBeVisible();
await expect(clientB.page.locator('task', { hasText: taskA })).toBeVisible();
await expect(clientB.page.locator('task', { hasText: taskB })).toBeVisible();
clientB.page.off('request', requestListener);
} finally {
if (clientB) {
await clientB.page.unroute('**/sync-ops.json').catch(() => {});
}
await closeContextsSafely(clientA?.context, clientB?.context);
}
});
});