mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): retention pruning, misc→tasks alias boundary & WS local-win re-upload (#9028)
* fix(supersync): exclude legacy REPAIR from retention cleanup pruning
The daily-cleanup fallback (used when `latestFullStateSeq` is absent, i.e.
legacy/pre-marker installs) selected the pruning boundary with a raw
`opType: { in: [SYNC_IMPORT, BACKUP_IMPORT, REPAIR] }` filter that includes
legacy REPAIR rows (`repairBaseServerSeq` NULL). Such a repair carries no
causal base cursor proving its state is current as of its seq, so pruning
history behind it can permanently drop ops committed between its logical
base and its seq for any device replaying from before it.
#8971 migrated five full-state queries to `CAUSAL_FULL_STATE_OPERATION_WHERE`
but missed this one — the single query whose result directly authorizes a
DELETE. Route it through the same causal-only predicate; a legacy-repair-only
user then yields `protectedFromSeq === null` and is skipped (no deletion),
the safe default already implemented below.
Adds a regression test whose mock `findFirst` honours the causal
`repairBaseServerSeq` predicate.
* fix(supersync): gate misc→tasks conflict alias on the split boundary
`detectConflictForEntity` and `prefetchLatestEntityOpsForBatch` looked up
the legacy `GLOBAL_CONFIG:misc` alias for an incoming `tasks` write with
`schemaVersion < CURRENT_SCHEMA_VERSION`. Once v4 shipped, that aliases
post-split v2/v3 misc writes — disjoint from `tasks` — and fabricates false
`CONFLICT_CONCURRENT` rejections of tasks-settings writes for multi-device
users.
Gate both read-side lookups on the fixed `MISC_TASKS_SPLIT_SCHEMA_VERSION`
(2), matching the `isLegacyMiscConfigOperation` incoming gate and the
warning comment it already carries. Drops the now-unused
`CURRENT_SCHEMA_VERSION` import.
Adds real-PostgreSQL integration coverage for both changed lookups
(`detectConflictForEntity` and the batch `prefetchLatestEntityOpsForBatch`):
a post-split v2/v3 misc write must not alias, a legacy v1 one still must.
* fix(sync): re-upload LWW local-win ops after a WS-triggered download
A WS-triggered download that resolves a conflict against pending local ops
appends LWW local-win replacement ops straight to the op-log, bypassing the
capture effect. Unlike `ImmediateUploadService` and the main sync loop, this
path had no follow-up upload, so the preserved edit sat unsynced until the
next user edit or periodic sync — an unbounded window for manual-sync-only
users.
Re-upload when `localWinOpsCreated > 0`, mirroring the other two paths, and
surface the same terminal outcomes `ImmediateUploadService` does — permanent
rejection / rejected-full-state baseline → ERROR, mandatory-but-missing key →
UNKNOWN_OR_CHANGED — so a preserved edit that fails to converge is not left
silent (the WS path never claims IN_SYNC). Single follow-up only, matching the
sibling side channel.
* fix(supersync): validate the primary latestFullStateSeq marker as causal before pruning
The scheduled retention cleanup trusted `state.latestFullStateSeq` as a pruning
boundary whenever it was `<= lastSnapshotSeq`, with no check that the marked op is
a causal full-state operation. The earlier fix (37bf818) hardened only the
fallback query used when the marker is absent.
Installs upgraded from before #8973 can carry a `latestFullStateSeq` set from a
legacy REPAIR (repairBaseServerSeq NULL) through the old isFullStateOpType gate,
and that migration shipped no backfill to clear stale markers. Trusting such a
marker prunes history behind a repair the replay path deliberately refuses as a
boundary (LegacyRepairReplayUnsupportedError) — permanent, cross-device loss.
Validate the marked op against CAUSAL_FULL_STATE_OPERATION_WHERE before it can
authorize a DELETE; a stale marker drops to the (now causal-only) fallback query
or the user is skipped. Updates the happy-path test to a causal boundary, adds a
stale-marker regression test, and teaches the mock findFirst to honour an exact
serverSeq predicate.
* test(e2e): de-flake USE_REMOTE crash-resume reload
`page.reload()` defaulted to waiting for the `load` event, which can never
fire while an active SuperSync WebSocket/sync connection keeps the page
"loading" — so the reload timed out at 30s (observed flake:
`page.reload: Timeout 30000ms exceeded`). Three sibling sync specs already
document this hang and work around it with close()+newPage(), but that drops
sessionStorage, which this test asserts on across the reload.
Wait only for `domcontentloaded` (with 60s headroom) instead; `waitForAppReady`
— which itself only needs `domcontentloaded` — remains the real readiness gate,
and the reload still preserves sessionStorage.
This commit is contained in:
parent
9132ab6722
commit
18262eb1f3
7 changed files with 478 additions and 7 deletions
|
|
@ -97,7 +97,12 @@ test.describe('@supersync USE_REMOTE interrupted rebuild recovery', () => {
|
|||
rebuildCommittedLog: REBUILD_COMMITTED_LOG,
|
||||
},
|
||||
);
|
||||
await clientB.page.reload();
|
||||
// Wait only for `domcontentloaded`, not the default `load`: an active
|
||||
// SuperSync WebSocket/sync connection can keep the page "loading" so the
|
||||
// `load` event never fires and `reload()` times out (flaky). `reload()`
|
||||
// preserves sessionStorage (unlike close()+newPage()), which this test
|
||||
// needs, and `waitForAppReady` below is the real readiness gate.
|
||||
await clientB.page.reload({ waitUntil: 'domcontentloaded', timeout: 60000 });
|
||||
await waitForAppReady(clientB.page);
|
||||
|
||||
// Importing gives B a known local state and creates a full-state local op,
|
||||
|
|
@ -131,7 +136,11 @@ test.describe('@supersync USE_REMOTE interrupted rebuild recovery', () => {
|
|||
|
||||
await clientB.sync.syncImportUseRemoteBtn.click();
|
||||
await crashObserved;
|
||||
await clientB.page.reload();
|
||||
// domcontentloaded (not the default `load`) — see the note on the first
|
||||
// reload above: an active sync connection can block `load` and hang
|
||||
// `reload()`. sessionStorage survives the reload; the assertion below
|
||||
// depends on it.
|
||||
await clientB.page.reload({ waitUntil: 'domcontentloaded', timeout: 60000 });
|
||||
await waitForAppReady(clientB.page);
|
||||
expect(
|
||||
await clientB.page.evaluate(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue